diff --git a/src/renderer/src/components/terminal-pane/terminal-visibility-resume.test.ts b/src/renderer/src/components/terminal-pane/terminal-visibility-resume.test.ts index 21716851899..29d9f27d287 100644 --- a/src/renderer/src/components/terminal-pane/terminal-visibility-resume.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-visibility-resume.test.ts @@ -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 @@ -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) diff --git a/src/renderer/src/components/terminal-pane/terminal-visibility-resume.ts b/src/renderer/src/components/terminal-pane/terminal-visibility-resume.ts index 5892dead9d7..c330efb37e4 100644 --- a/src/renderer/src/components/terminal-pane/terminal-visibility-resume.ts +++ b/src/renderer/src/components/terminal-pane/terminal-visibility-resume.ts @@ -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 diff --git a/src/renderer/src/lib/pane-manager/terminal-linkifier-hover-reset.test.ts b/src/renderer/src/lib/pane-manager/terminal-linkifier-hover-reset.test.ts new file mode 100644 index 00000000000..aca9b7034d5 --- /dev/null +++ b/src/renderer/src/lib/pane-manager/terminal-linkifier-hover-reset.test.ts @@ -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) + }) +}) diff --git a/src/renderer/src/lib/pane-manager/terminal-linkifier-hover-reset.ts b/src/renderer/src/lib/pane-manager/terminal-linkifier-hover-reset.ts new file mode 100644 index 00000000000..2c93f354ac0 --- /dev/null +++ b/src/renderer/src/lib/pane-manager/terminal-linkifier-hover-reset.ts @@ -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 */ + } +} diff --git a/tests/e2e/terminal-link-hover-after-worktree-return.spec.ts b/tests/e2e/terminal-link-hover-after-worktree-return.spec.ts new file mode 100644 index 00000000000..138d29aa38f --- /dev/null +++ b/tests/e2e/terminal-link-hover-after-worktree-return.spec.ts @@ -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 { + 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('.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 { + 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('.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 { + await page.evaluate((tabId) => { + const manager = window.__paneManagers?.get(tabId) + const pane = manager?.getActivePane?.() ?? manager?.getPanes?.()[0] ?? null + const screen = pane?.terminal.element?.querySelector('.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 { + 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 { + 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 + }) + }) +})