mirror of
https://github.com/stablyai/orca.git
synced 2026-10-18 08:03:22 +00:00
fix(terminal): restore clickable links after returning to a worktree
Clicking a terminal link (file path, URL, term_* handle, OSC-8) does nothing after switching to another worktree and back, until the terminal is scrolled a little. xterm's linkifier only re-runs link providers on mousemove when the hovered buffer cell changes vs its cached `_lastBufferCell`. Hiding the terminal surface fires `mouseleave`, which clears the current link but leaves that cache. On reveal the pointer returns to the same cell, so the mousemove handler short-circuits and the link is never re-established; a scroll shifts the buffer position and re-linkifies, which is the "scroll to fix" symptom. Reset the linkifier hover-cell cache when a terminal surface is revealed (resumeTerminalVisibility) so the next mousemove re-evaluates providers. Covers all link types, including file-path links whose geometry click fallback does not compensate after reveal. Verified in real Electron via a new e2e that fails without the reset and passes with it, for both a file-path link and a URL.
This commit is contained in:
@@ -27,6 +27,11 @@ vi.mock('./terminal-webgl-atlas-recovery', () => ({
|
||||
// the terminal-output debounce (which a background stream could otherwise defer).
|
||||
scheduleTabRevealWebglAtlasRecovery: () => scheduleTabRevealWebglAtlasRecovery()
|
||||
}))
|
||||
const resetTerminalLinkifierHoverState = vi.fn()
|
||||
vi.mock('@/lib/pane-manager/terminal-linkifier-hover-reset', () => ({
|
||||
resetTerminalLinkifierHoverState: (terminal: unknown) =>
|
||||
resetTerminalLinkifierHoverState(terminal)
|
||||
}))
|
||||
|
||||
type FakeManager = {
|
||||
getPanes: ReturnType<typeof vi.fn>
|
||||
@@ -90,6 +95,18 @@ describe('resumeTerminalVisibility reveal repaint', () => {
|
||||
)
|
||||
})
|
||||
|
||||
it('resets each pane linkifier hover cache on reveal so links recover without a scroll', () => {
|
||||
const first = { name: 'pane-a' }
|
||||
const second = { name: 'pane-b' }
|
||||
const manager = createManager()
|
||||
manager.getPanes.mockReturnValue([{ terminal: first }, { terminal: second }])
|
||||
|
||||
resumeTerminalVisibility(resumeArgs(manager, false))
|
||||
|
||||
expect(resetTerminalLinkifierHoverState).toHaveBeenCalledWith(first)
|
||||
expect(resetTerminalLinkifierHoverState).toHaveBeenCalledWith(second)
|
||||
})
|
||||
|
||||
it('schedules the repaint after rendering resumes on a heavy reveal', () => {
|
||||
const order: string[] = []
|
||||
const manager = createManager(order)
|
||||
|
||||
@@ -9,6 +9,7 @@ import {
|
||||
enforceTerminalCurrentScrollIntent,
|
||||
syncTerminalScrollIntentFromViewport
|
||||
} from '@/lib/pane-manager/terminal-scroll-intent'
|
||||
import { resetTerminalLinkifierHoverState } from '@/lib/pane-manager/terminal-linkifier-hover-reset'
|
||||
import { fitAndFocusPanes, fitPanes, focusActivePane } from './pane-helpers'
|
||||
import { scheduleTabRevealWebglAtlasRecovery } from './terminal-webgl-atlas-recovery'
|
||||
|
||||
@@ -54,6 +55,12 @@ export function resumeTerminalVisibility({
|
||||
captureViewportPositions,
|
||||
withSuppressedScrollTracking
|
||||
}: ResumeTerminalVisibilityArgs): void {
|
||||
// Why: hiding the surface fired mouseleave, which cleared xterm's current
|
||||
// link but left its hover cell cache; without this reset a link stays dead
|
||||
// until a scroll when the pointer returns to the same cell on reveal.
|
||||
for (const pane of manager.getPanes()) {
|
||||
resetTerminalLinkifierHoverState(pane.terminal)
|
||||
}
|
||||
syncTerminalViewportIntents(manager)
|
||||
// Why: WebGL resume can disturb xterm's viewport bookkeeping before the
|
||||
// post-resume fit runs. Capture numeric viewport positions first; the
|
||||
|
||||
@@ -0,0 +1,33 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import type { Terminal } from '@xterm/xterm'
|
||||
import { resetTerminalLinkifierHoverState } from './terminal-linkifier-hover-reset'
|
||||
|
||||
type FakeLinkifier = { _lastBufferCell?: unknown; _activeLine?: number }
|
||||
|
||||
function createTerminal(linkifier: FakeLinkifier | null | undefined, hasCore = true): Terminal {
|
||||
const core = hasCore ? { linkifier: linkifier ?? undefined } : undefined
|
||||
return { _core: core } as unknown as Terminal
|
||||
}
|
||||
|
||||
describe('resetTerminalLinkifierHoverState', () => {
|
||||
it('clears the hover cell cache and active line so the next mousemove re-linkifies', () => {
|
||||
const linkifier: FakeLinkifier = { _lastBufferCell: { x: 5, y: 5 }, _activeLine: 5 }
|
||||
resetTerminalLinkifierHoverState(createTerminal(linkifier))
|
||||
|
||||
expect(linkifier._lastBufferCell).toBeUndefined()
|
||||
expect(linkifier._activeLine).toBe(-1)
|
||||
})
|
||||
|
||||
it('is a no-op when the linkifier or core internals are unavailable', () => {
|
||||
expect(() => resetTerminalLinkifierHoverState(createTerminal(null))).not.toThrow()
|
||||
expect(() => resetTerminalLinkifierHoverState(createTerminal(undefined, false))).not.toThrow()
|
||||
})
|
||||
|
||||
it('only touches fields that exist so a renamed xterm internal degrades safely', () => {
|
||||
const linkifier: FakeLinkifier = {}
|
||||
resetTerminalLinkifierHoverState(createTerminal(linkifier))
|
||||
|
||||
expect('_lastBufferCell' in linkifier).toBe(false)
|
||||
expect('_activeLine' in linkifier).toBe(false)
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,46 @@
|
||||
import type { Terminal } from '@xterm/xterm'
|
||||
|
||||
type LinkifierHoverCache = {
|
||||
_lastBufferCell?: unknown
|
||||
_activeLine?: number
|
||||
}
|
||||
|
||||
type TerminalCoreWithLinkifier = {
|
||||
_core?: {
|
||||
linkifier?: LinkifierHoverCache
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Force xterm's linkifier to re-run link providers on the next mousemove.
|
||||
*
|
||||
* Why: when a terminal is hidden (worktree/tab switch) the browser fires
|
||||
* `mouseleave`, which clears the linkifier's current link but leaves its
|
||||
* `_lastBufferCell` cache set. On reveal the pointer usually returns to the
|
||||
* same cell, so xterm's mousemove handler short-circuits (position unchanged)
|
||||
* and never re-linkifies — the link (file path, URL, term_* handle, OSC-8)
|
||||
* stays dead until a scroll shifts the buffer position. Clearing the cell/line
|
||||
* cache makes the next mousemove re-evaluate providers so the link and its
|
||||
* hover underline recover without a scroll.
|
||||
*
|
||||
* Reaches into xterm internals (`@xterm/xterm` 6.1.0-beta.287 `Linkifier`)
|
||||
* because there is no public API to invalidate the hover cache. Guarded so a
|
||||
* future xterm build that renames these fields degrades to the pre-fix
|
||||
* behavior (link recovers on the next genuine cell change) instead of throwing.
|
||||
*/
|
||||
export function resetTerminalLinkifierHoverState(terminal: Terminal): void {
|
||||
try {
|
||||
const linkifier = (terminal as unknown as TerminalCoreWithLinkifier)._core?.linkifier
|
||||
if (!linkifier) {
|
||||
return
|
||||
}
|
||||
if ('_lastBufferCell' in linkifier) {
|
||||
linkifier._lastBufferCell = undefined
|
||||
}
|
||||
if ('_activeLine' in linkifier) {
|
||||
linkifier._activeLine = -1
|
||||
}
|
||||
} catch {
|
||||
/* linkifier internals unavailable — link recovers on the next cell change */
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,256 @@
|
||||
import { randomUUID } from 'node:crypto'
|
||||
import { rmSync, writeFileSync } from 'node:fs'
|
||||
import path from 'node:path'
|
||||
import type { Page } from '@stablyai/playwright-test'
|
||||
import { expect, test } from './helpers/orca-app'
|
||||
import {
|
||||
ensureTerminalVisible,
|
||||
getAllWorktreeIds,
|
||||
switchToWorktree,
|
||||
waitForActiveWorktree,
|
||||
waitForSessionReady
|
||||
} from './helpers/store'
|
||||
import {
|
||||
getTerminalContent,
|
||||
sendToTerminal,
|
||||
waitForActivePanePtyId,
|
||||
waitForActiveTerminalManager
|
||||
} from './helpers/terminal'
|
||||
import { waitForPtyShellEcho } from './terminal-pty-readiness'
|
||||
|
||||
/**
|
||||
* Repro for: a clickable terminal link (file path, URL, …) becomes dead after
|
||||
* switching to another worktree and returning, until the user scrolls the
|
||||
* terminal a little.
|
||||
*
|
||||
* xterm's Linkifier only re-runs link providers on mousemove when the hovered
|
||||
* buffer cell changes vs its `_lastBufferCell` cache. Hiding the surface fires
|
||||
* mouseleave (clearing the current link) but leaves that cache, so returning
|
||||
* the pointer to the same cell short-circuits and the link is never
|
||||
* re-established — `currentLink` stays null. File-path links are the worst case
|
||||
* because their geometry click fallback does not compensate after reveal.
|
||||
*/
|
||||
type HoverProbe = { clientX: number; clientY: number; tabId: string }
|
||||
|
||||
async function locateHoverProbe(page: Page, needle: string): Promise<HoverProbe> {
|
||||
return page.evaluate((needle) => {
|
||||
const state = window.__store?.getState()
|
||||
const worktreeId = state?.activeWorktreeId ?? null
|
||||
const tabId =
|
||||
state?.activeTabType === 'terminal'
|
||||
? (state?.activeTabId ?? null)
|
||||
: worktreeId
|
||||
? (state?.activeTabIdByWorktree?.[worktreeId] ?? null)
|
||||
: null
|
||||
const manager = tabId ? window.__paneManagers?.get(tabId) : null
|
||||
const pane = manager?.getActivePane?.() ?? manager?.getPanes?.()[0] ?? null
|
||||
if (!tabId || !pane) {
|
||||
throw new Error('active terminal pane unavailable')
|
||||
}
|
||||
const terminal = pane.terminal
|
||||
const buffer = terminal.buffer.active
|
||||
let hit: { row: number; col: number } | null = null
|
||||
for (let row = 0; row < terminal.rows; row += 1) {
|
||||
const line = buffer.getLine(buffer.viewportY + row)
|
||||
if (!line) {
|
||||
continue
|
||||
}
|
||||
const idx = line.translateToString(true).indexOf(needle)
|
||||
if (idx >= 0) {
|
||||
hit = { row, col: idx }
|
||||
break
|
||||
}
|
||||
}
|
||||
if (!hit) {
|
||||
throw new Error('link text not visible in terminal viewport')
|
||||
}
|
||||
const screen = terminal.element?.querySelector<HTMLElement>('.xterm-screen')
|
||||
if (!screen) {
|
||||
throw new Error('xterm-screen element unavailable')
|
||||
}
|
||||
const rect = screen.getBoundingClientRect()
|
||||
const cellWidth = rect.width / terminal.cols
|
||||
const cellHeight = rect.height / terminal.rows
|
||||
// Aim at the middle of the link text so the pointer lands squarely inside
|
||||
// the link range regardless of rounding.
|
||||
const targetCol = hit.col + Math.floor(needle.length / 2)
|
||||
return {
|
||||
clientX: rect.left + (targetCol + 0.5) * cellWidth,
|
||||
clientY: rect.top + (hit.row + 0.5) * cellHeight,
|
||||
tabId
|
||||
}
|
||||
}, needle)
|
||||
}
|
||||
|
||||
/**
|
||||
* Dispatch a hover mousemove at the probe coordinates and return the text of
|
||||
* the link the linkifier considers active (or null). Callers poll this because
|
||||
* Orca's file-path provider resolves link candidates asynchronously.
|
||||
*/
|
||||
async function hoverAndReadActiveLinkText(page: Page, probe: HoverProbe): Promise<string | null> {
|
||||
await page.evaluate(({ clientX, clientY, tabId }) => {
|
||||
const manager = window.__paneManagers?.get(tabId)
|
||||
const pane = manager?.getActivePane?.() ?? manager?.getPanes?.()[0] ?? null
|
||||
const screen = pane?.terminal.element?.querySelector<HTMLElement>('.xterm-screen')
|
||||
screen?.dispatchEvent(
|
||||
new MouseEvent('mousemove', { bubbles: true, cancelable: true, clientX, clientY })
|
||||
)
|
||||
}, probe)
|
||||
return page.evaluate(({ tabId }) => {
|
||||
const manager = window.__paneManagers?.get(tabId)
|
||||
const pane = manager?.getActivePane?.() ?? manager?.getPanes?.()[0] ?? null
|
||||
const core = pane?.terminal as unknown as
|
||||
| { _core?: { linkifier?: { currentLink?: { link?: { text?: string } } } } }
|
||||
| undefined
|
||||
return core?._core?.linkifier?.currentLink?.link?.text ?? null
|
||||
}, probe)
|
||||
}
|
||||
|
||||
async function dispatchScreenMouseLeave(page: Page, tabId: string): Promise<void> {
|
||||
await page.evaluate((tabId) => {
|
||||
const manager = window.__paneManagers?.get(tabId)
|
||||
const pane = manager?.getActivePane?.() ?? manager?.getPanes?.()[0] ?? null
|
||||
const screen = pane?.terminal.element?.querySelector<HTMLElement>('.xterm-screen')
|
||||
// Mimics the pointer leaving as the surface hides on a worktree switch:
|
||||
// clears the linkifier's current link but keeps its cell cache.
|
||||
screen?.dispatchEvent(new MouseEvent('mouseleave', { bubbles: false, cancelable: true }))
|
||||
}, tabId)
|
||||
}
|
||||
|
||||
async function activeWorktreePath(page: Page): Promise<string> {
|
||||
return page.evaluate(() => {
|
||||
const state = window.__store?.getState()
|
||||
const id = state?.activeWorktreeId
|
||||
return (
|
||||
Object.values(state?.worktreesByRepo ?? {})
|
||||
.flat()
|
||||
.find((w) => w.id === id)?.path ?? ''
|
||||
)
|
||||
})
|
||||
}
|
||||
|
||||
/**
|
||||
* Drive the full repro and assert the link re-establishes on hover after
|
||||
* returning to the worktree. `contains` accommodates the file provider's
|
||||
* display text differing slightly from the raw echoed token.
|
||||
*/
|
||||
async function assertLinkRecoversAfterReturn(
|
||||
page: Page,
|
||||
args: {
|
||||
firstWorktreeId: string
|
||||
secondWorktreeId: string
|
||||
needle: string
|
||||
expectContains: string
|
||||
}
|
||||
): Promise<void> {
|
||||
const probe = await locateHoverProbe(page, args.needle)
|
||||
|
||||
// Baseline: hovering establishes the link before any switch.
|
||||
await expect
|
||||
.poll(() => hoverAndReadActiveLinkText(page, probe), {
|
||||
timeout: 5_000,
|
||||
message: 'baseline hover never established the link'
|
||||
})
|
||||
.toContain(args.expectContains)
|
||||
|
||||
await dispatchScreenMouseLeave(page, probe.tabId)
|
||||
await switchToWorktree(page, args.secondWorktreeId)
|
||||
await waitForActiveTerminalManager(page, 30_000)
|
||||
await page.waitForTimeout(200)
|
||||
|
||||
await switchToWorktree(page, args.firstWorktreeId)
|
||||
await ensureTerminalVisible(page)
|
||||
await waitForActiveTerminalManager(page, 30_000)
|
||||
await page.waitForTimeout(250)
|
||||
|
||||
// Hover the SAME cell without scrolling. Pre-fix this never re-establishes
|
||||
// the link (dead until a scroll); post-fix the reveal reset re-linkifies.
|
||||
await expect
|
||||
.poll(() => hoverAndReadActiveLinkText(page, probe), {
|
||||
timeout: 5_000,
|
||||
message: 'link did not re-establish on hover after returning to the worktree'
|
||||
})
|
||||
.toContain(args.expectContains)
|
||||
}
|
||||
|
||||
test.describe('Terminal link hover after worktree return', () => {
|
||||
test.beforeEach(async ({ orcaPage }) => {
|
||||
await waitForSessionReady(orcaPage)
|
||||
})
|
||||
|
||||
test('re-establishes a file-path link on hover after switching worktrees and back', async ({
|
||||
orcaPage
|
||||
}) => {
|
||||
const firstWorktreeId = await waitForActiveWorktree(orcaPage)
|
||||
const secondWorktreeId = (await getAllWorktreeIds(orcaPage)).find(
|
||||
(id) => id !== firstWorktreeId
|
||||
)
|
||||
test.skip(!secondWorktreeId, 'link-hover repro needs the seeded secondary worktree')
|
||||
if (!secondWorktreeId) {
|
||||
return
|
||||
}
|
||||
|
||||
await ensureTerminalVisible(orcaPage)
|
||||
await waitForActiveTerminalManager(orcaPage, 30_000)
|
||||
const ptyId = await waitForActivePanePtyId(orcaPage)
|
||||
await waitForPtyShellEcho(orcaPage, ptyId, 15_000)
|
||||
|
||||
const worktreePath = await activeWorktreePath(orcaPage)
|
||||
const fileName = `orca-linkfile-${randomUUID().slice(0, 8)}.txt`
|
||||
writeFileSync(path.join(worktreePath, fileName), 'orca file link target\n')
|
||||
const needle = `./${fileName}`
|
||||
|
||||
try {
|
||||
await sendToTerminal(orcaPage, ptyId, `echo ${needle}\r`)
|
||||
await expect
|
||||
.poll(() => getTerminalContent(orcaPage, 4000), {
|
||||
timeout: 10_000,
|
||||
message: 'file-link fixture did not reach the terminal buffer'
|
||||
})
|
||||
.toContain(fileName)
|
||||
|
||||
await assertLinkRecoversAfterReturn(orcaPage, {
|
||||
firstWorktreeId,
|
||||
secondWorktreeId,
|
||||
needle,
|
||||
expectContains: fileName
|
||||
})
|
||||
} finally {
|
||||
rmSync(path.join(worktreePath, fileName), { force: true })
|
||||
}
|
||||
})
|
||||
|
||||
test('re-establishes a URL link on hover after switching worktrees and back', async ({
|
||||
orcaPage
|
||||
}) => {
|
||||
const firstWorktreeId = await waitForActiveWorktree(orcaPage)
|
||||
const secondWorktreeId = (await getAllWorktreeIds(orcaPage)).find(
|
||||
(id) => id !== firstWorktreeId
|
||||
)
|
||||
test.skip(!secondWorktreeId, 'link-hover repro needs the seeded secondary worktree')
|
||||
if (!secondWorktreeId) {
|
||||
return
|
||||
}
|
||||
|
||||
await ensureTerminalVisible(orcaPage)
|
||||
await waitForActiveTerminalManager(orcaPage, 30_000)
|
||||
const ptyId = await waitForActivePanePtyId(orcaPage)
|
||||
await waitForPtyShellEcho(orcaPage, ptyId, 15_000)
|
||||
|
||||
const url = `https://example.com/orca-link-${randomUUID()}`
|
||||
await sendToTerminal(orcaPage, ptyId, `echo ${url}\r`)
|
||||
await expect
|
||||
.poll(() => getTerminalContent(orcaPage, 4000), {
|
||||
timeout: 10_000,
|
||||
message: 'URL fixture did not reach the terminal buffer'
|
||||
})
|
||||
.toContain(url)
|
||||
|
||||
await assertLinkRecoversAfterReturn(orcaPage, {
|
||||
firstWorktreeId,
|
||||
secondWorktreeId,
|
||||
needle: url,
|
||||
expectContains: url
|
||||
})
|
||||
})
|
||||
})
|
||||
Reference in New Issue
Block a user