diff --git a/src/renderer/src/lib/pane-manager/pane-lifecycle.test.ts b/src/renderer/src/lib/pane-manager/pane-lifecycle.test.ts index a18e432d065..c69600e8a35 100644 --- a/src/renderer/src/lib/pane-manager/pane-lifecycle.test.ts +++ b/src/renderer/src/lib/pane-manager/pane-lifecycle.test.ts @@ -515,6 +515,7 @@ describe('openTerminal — addon and provider wiring', () => { } }), attachCustomWheelEventHandler: vi.fn(), + onWriteParsed: vi.fn(() => ({ dispose: vi.fn() })), write: vi.fn(() => { events.push('write') }), @@ -606,6 +607,22 @@ describe('openTerminal — addon and provider wiring', () => { expect(pane.arabicShapingJoinerCleanup).toBeNull() }) + // Why: a link streamed under a stationary pointer must re-linkify on the next + // move; openTerminal wires the hover-cache reset and disposePane must detach it. + it('installs the streamed-output linkifier hover reset and disposes it', () => { + const { pane } = createOpenTerminalHarness() + + openTerminal(pane) + const disposable = pane.linkifierHoverResetDisposable + expect(disposable?.dispose).toBeTypeOf('function') + expect(pane.terminal.onWriteParsed).toHaveBeenCalledTimes(1) + + const disposeSpy = vi.spyOn(disposable!, 'dispose') + disposePane(pane, new Map([[pane.id, pane]])) + expect(disposeSpy).toHaveBeenCalledTimes(1) + expect(pane.linkifierHoverResetDisposable).toBeNull() + }) + // Why: the DOM renderer misrenders joined spans (per-character // letter-spacing blowout), so the joiner must only join while this pane's // WebGL addon is live — locked here against the real openTerminal wiring. diff --git a/src/renderer/src/lib/pane-manager/pane-lifecycle.ts b/src/renderer/src/lib/pane-manager/pane-lifecycle.ts index ea938f9a351..59e24471026 100644 --- a/src/renderer/src/lib/pane-manager/pane-lifecycle.ts +++ b/src/renderer/src/lib/pane-manager/pane-lifecycle.ts @@ -9,6 +9,7 @@ import { cancelDeferredScrollRestore } from './pane-scroll' import { activateOrcaTerminalUnicodeProvider } from '../../../../shared/terminal-unicode-provider' import { attachTerminalMouseWheelMultiplier } from './pane-terminal-mouse-wheel' import { attachTerminalScrollIntentTracking } from './terminal-scroll-intent-dom-tracking' +import { installTerminalLinkifierHoverResetOnWrite } from './terminal-linkifier-hover-reset-on-write' import { attachDomRendererFocusClassSync } from './pane-dom-focus-class-sync' import { attachWebgl, cancelPendingWebglRefresh, disposeWebgl } from './pane-webgl-renderer' import { configureLazyArabicShapingJoiner } from './terminal-arabic-shaping-joiner' @@ -56,6 +57,11 @@ export function openTerminal(pane: ManagedPaneInternal): void { xtermContainer, pane.leafId ) + // Why: a link streamed into a visible pane under a stationary pointer would + // otherwise stay un-underlined/un-clickable until the mouse crosses to a new + // line; invalidate the linkifier hover cache when output lands so the next + // pointer move re-linkifies it. + pane.linkifierHoverResetDisposable = installTerminalLinkifierHoverResetOnWrite(terminal) // Activate Orca's Unicode 11 width shim *before* any caller-driven write. CJK / emoji / // ZWJ codepoints get baked into the buffer at the active unicode version on @@ -228,6 +234,8 @@ export function disposePane( pane.focusClassSyncCleanup = null pane.terminalScrollIntentDisposable?.dispose() pane.terminalScrollIntentDisposable = null + pane.linkifierHoverResetDisposable?.dispose() + pane.linkifierHoverResetDisposable = null // Deregister the RTL shaping joiner: terminal.dispose() below does not. try { pane.arabicShapingJoinerCleanup?.() diff --git a/src/renderer/src/lib/pane-manager/pane-manager-types.ts b/src/renderer/src/lib/pane-manager/pane-manager-types.ts index 5f2c3d5fcdb..3face2a5189 100644 --- a/src/renderer/src/lib/pane-manager/pane-manager-types.ts +++ b/src/renderer/src/lib/pane-manager/pane-manager-types.ts @@ -163,6 +163,9 @@ export type ManagedPaneInternal = { focusClassSyncCleanup?: (() => void) | null // Stored so disposePane() can remove user-scroll intent listeners. terminalScrollIntentDisposable?: IDisposable | null + // Stored so disposePane() can detach the streamed-output hover-cache reset + // that keeps freshly printed links linkifiable without a scroll. + linkifierHoverResetDisposable?: IDisposable | null // Stored so disposePane() can deregister the joiner; terminal.dispose() // does not remove registered character joiners. arabicShapingJoinerCleanup?: (() => void) | null diff --git a/src/renderer/src/lib/pane-manager/terminal-linkifier-hover-reset-on-write.test.ts b/src/renderer/src/lib/pane-manager/terminal-linkifier-hover-reset-on-write.test.ts new file mode 100644 index 00000000000..ad812d58f20 --- /dev/null +++ b/src/renderer/src/lib/pane-manager/terminal-linkifier-hover-reset-on-write.test.ts @@ -0,0 +1,159 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { Terminal } from '@xterm/xterm' +import * as hoverReset from './terminal-linkifier-hover-reset' +import { installTerminalLinkifierHoverResetOnWrite } from './terminal-linkifier-hover-reset-on-write' + +// Spy on the reset primitive while keeping its real field-clearing behavior, so +// tests can COUNT invocations — the throttle/coalesce properties are otherwise +// invisible (the reset writes idempotent cache state). +vi.mock('./terminal-linkifier-hover-reset', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + resetTerminalLinkifierHoverState: vi.fn(actual.resetTerminalLinkifierHoverState) + } +}) + +type LinkifierCache = { _lastBufferCell?: unknown; _activeLine?: number; _currentLink?: unknown } + +function createFakeTerminal(): { + terminal: Terminal + emitWriteParsed: () => void + linkifier: LinkifierCache + listenerDisposed: () => boolean +} { + const listeners = new Set<() => void>() + let disposed = false + const linkifier: LinkifierCache = { + _lastBufferCell: { x: 3, y: 4 }, + _activeLine: 4, + _currentLink: undefined + } + const terminal = { + onWriteParsed: (handler: () => void) => { + listeners.add(handler) + return { + dispose: () => { + disposed = true + listeners.delete(handler) + } + } + }, + _core: { linkifier } + } as unknown as Terminal + return { + terminal, + emitWriteParsed: () => listeners.forEach((handler) => handler()), + linkifier, + listenerDisposed: () => disposed + } +} + +const resetSpy = vi.mocked(hoverReset.resetTerminalLinkifierHoverState) + +describe('installTerminalLinkifierHoverResetOnWrite', () => { + beforeEach(() => { + vi.useFakeTimers() + resetSpy.mockClear() + }) + afterEach(() => vi.useRealTimers()) + + it('clears the linkifier hover cache a throttle window after output lands', () => { + const fake = createFakeTerminal() + installTerminalLinkifierHoverResetOnWrite(fake.terminal) + + fake.emitWriteParsed() + // Not reset synchronously — throttled so streaming does not re-query per chunk. + expect(resetSpy).not.toHaveBeenCalled() + expect(fake.linkifier._lastBufferCell).toBeDefined() + + vi.advanceTimersByTime(150) + expect(resetSpy).toHaveBeenCalledTimes(1) + expect(fake.linkifier._lastBufferCell).toBeUndefined() + expect(fake.linkifier._activeLine).toBe(-1) + }) + + it('coalesces a burst of writes into exactly one reset per window', () => { + const fake = createFakeTerminal() + installTerminalLinkifierHoverResetOnWrite(fake.terminal) + + // 20 chunks inside one window must schedule only one reset — not 20. This + // fails if the leading-edge throttle guard is removed. + for (let i = 0; i < 20; i += 1) { + fake.emitWriteParsed() + vi.advanceTimersByTime(5) + } + vi.advanceTimersByTime(150) + expect(resetSpy).toHaveBeenCalledTimes(1) + }) + + it('keeps resetting during continuous streaming instead of starving', () => { + const fake = createFakeTerminal() + installTerminalLinkifierHoverResetOnWrite(fake.terminal) + + // A chunk every 50ms for 500ms. The throttle fires ~every 150ms; a debounce + // (clearTimeout + reschedule per chunk) would never fire while the stream + // continues — that regression is what this guards against. + for (let i = 0; i < 10; i += 1) { + fake.emitWriteParsed() + vi.advanceTimersByTime(50) + } + expect(resetSpy.mock.calls.length).toBeGreaterThanOrEqual(3) + }) + + it('does not disturb an actively hovered link, and resumes once it clears', () => { + const fake = createFakeTerminal() + installTerminalLinkifierHoverResetOnWrite(fake.terminal) + + fake.linkifier._currentLink = { link: 'https://example.com' } + fake.emitWriteParsed() + vi.advanceTimersByTime(150) + // Hovering: the cache is left intact so the underline/tooltip do not flicker. + expect(resetSpy).not.toHaveBeenCalled() + expect(fake.linkifier._lastBufferCell).toBeDefined() + + fake.linkifier._currentLink = undefined + fake.emitWriteParsed() + vi.advanceTimersByTime(150) + expect(resetSpy).toHaveBeenCalledTimes(1) + expect(fake.linkifier._lastBufferCell).toBeUndefined() + }) + + it('does not drop the reset when the stream goes quiet mid-hover', () => { + const fake = createFakeTerminal() + installTerminalLinkifierHoverResetOnWrite(fake.terminal) + + // Last chunk of a burst lands while a link is hovered, then output stops. + fake.linkifier._currentLink = { link: 'https://example.com' } + fake.emitWriteParsed() + // Many windows pass with NO further writes — the pending reset must survive. + vi.advanceTimersByTime(600) + expect(resetSpy).not.toHaveBeenCalled() + + // Once the pointer leaves the link, the retry finally resets — without any + // new output re-arming it. + fake.linkifier._currentLink = undefined + vi.advanceTimersByTime(150) + expect(resetSpy).toHaveBeenCalledTimes(1) + expect(fake.linkifier._lastBufferCell).toBeUndefined() + }) + + it('cancels the pending reset and detaches the listener on dispose', () => { + const fake = createFakeTerminal() + const disposable = installTerminalLinkifierHoverResetOnWrite(fake.terminal) + + fake.emitWriteParsed() + disposable.dispose() + expect(fake.listenerDisposed()).toBe(true) + + vi.advanceTimersByTime(500) + // Disposed before the timer fired: no reset, cache untouched. + expect(resetSpy).not.toHaveBeenCalled() + expect(fake.linkifier._lastBufferCell).toEqual({ x: 3, y: 4 }) + }) + + it('degrades to a no-op when the terminal lacks onWriteParsed', () => { + const terminal = { _core: { linkifier: {} } } as unknown as Terminal + expect(() => installTerminalLinkifierHoverResetOnWrite(terminal).dispose()).not.toThrow() + }) +}) diff --git a/src/renderer/src/lib/pane-manager/terminal-linkifier-hover-reset-on-write.ts b/src/renderer/src/lib/pane-manager/terminal-linkifier-hover-reset-on-write.ts new file mode 100644 index 00000000000..509b0012365 --- /dev/null +++ b/src/renderer/src/lib/pane-manager/terminal-linkifier-hover-reset-on-write.ts @@ -0,0 +1,67 @@ +import type { IDisposable, Terminal } from '@xterm/xterm' +import { + isTerminalLinkifierHoverActive, + resetTerminalLinkifierHoverState +} from './terminal-linkifier-hover-reset' + +// Why: coalesce bursts of streamed output into at most one hover-cache reset +// per window so continuous agent output does not force a provider re-query on +// every parsed chunk. 150ms keeps a freshly printed link responsive to the +// user's next pointer move without measurable churn. +const HOVER_RESET_THROTTLE_MS = 150 + +/** + * Invalidate xterm's linkifier hover cache shortly after streamed output lands. + * + * Why: xterm re-runs link providers only on mousemove when the hovered buffer + * cell changes, and it caches provider replies per line with no content-change + * invalidation ({@link resetTerminalLinkifierHoverState} documents the fields). + * A URL an agent streams into a visible pane under a stationary pointer is + * therefore never underlined — and its native activation stays dead — until the + * pointer crosses to a different line, which is the "click the terminal a few + * times before the link works" symptom. Clearing the cell/line cache when new + * content lands lets the very next pointer move re-linkify the fresh URL. + * + * Sibling of the visibility-resume reset (see terminal-visibility-resume.ts), + * which only covers reveal — not output streaming into an already-visible pane. + */ +export function installTerminalLinkifierHoverResetOnWrite(terminal: Terminal): IDisposable { + // Why: never let this break pane creation if a Terminal stub or a future + // xterm build lacks onWriteParsed — links then recover on the next cell + // change, as they did before this reset existed. + if (typeof terminal.onWriteParsed !== 'function') { + return { dispose: () => undefined } + } + let timer: ReturnType | null = null + const flush = (): void => { + // Why: never invalidate the cache while the user is hovering a link — it + // would clear+re-query the active link (async for file paths), flickering + // its underline/tooltip. Re-arm instead of dropping the pending reset: if + // this was the last chunk of a burst and it appended a link to the hovered + // line, dropping the reset would leave that link dead until a line change. + // The retry performs the reset once the hover ends. (timer stays non-null + // during the retry so a concurrent write does not stack a second timer.) + if (isTerminalLinkifierHoverActive(terminal)) { + timer = setTimeout(flush, HOVER_RESET_THROTTLE_MS) + return + } + timer = null + resetTerminalLinkifierHoverState(terminal) + } + const scheduleReset = (): void => { + if (timer !== null) { + return + } + timer = setTimeout(flush, HOVER_RESET_THROTTLE_MS) + } + const writeParsedDisposable = terminal.onWriteParsed(scheduleReset) + return { + dispose: () => { + if (timer !== null) { + clearTimeout(timer) + timer = null + } + writeParsedDisposable.dispose() + } + } +} 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 index aca9b7034d5..b39b2bbd5d0 100644 --- 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 @@ -1,8 +1,11 @@ import { describe, expect, it } from 'vitest' import type { Terminal } from '@xterm/xterm' -import { resetTerminalLinkifierHoverState } from './terminal-linkifier-hover-reset' +import { + isTerminalLinkifierHoverActive, + resetTerminalLinkifierHoverState +} from './terminal-linkifier-hover-reset' -type FakeLinkifier = { _lastBufferCell?: unknown; _activeLine?: number } +type FakeLinkifier = { _lastBufferCell?: unknown; _activeLine?: number; _currentLink?: unknown } function createTerminal(linkifier: FakeLinkifier | null | undefined, hasCore = true): Terminal { const core = hasCore ? { linkifier: linkifier ?? undefined } : undefined @@ -31,3 +34,18 @@ describe('resetTerminalLinkifierHoverState', () => { expect('_activeLine' in linkifier).toBe(false) }) }) + +describe('isTerminalLinkifierHoverActive', () => { + it('is true only while a link is currently hovered', () => { + expect(isTerminalLinkifierHoverActive(createTerminal({ _currentLink: { link: 'x' } }))).toBe( + true + ) + expect(isTerminalLinkifierHoverActive(createTerminal({ _currentLink: undefined }))).toBe(false) + expect(isTerminalLinkifierHoverActive(createTerminal({}))).toBe(false) + }) + + it('degrades to false when linkifier internals are unavailable', () => { + expect(isTerminalLinkifierHoverActive(createTerminal(null))).toBe(false) + expect(isTerminalLinkifierHoverActive(createTerminal(undefined, false))).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 index 2c93f354ac0..d8c40e4b59c 100644 --- a/src/renderer/src/lib/pane-manager/terminal-linkifier-hover-reset.ts +++ b/src/renderer/src/lib/pane-manager/terminal-linkifier-hover-reset.ts @@ -3,6 +3,9 @@ import type { Terminal } from '@xterm/xterm' type LinkifierHoverCache = { _lastBufferCell?: unknown _activeLine?: number + // Set while xterm is showing a hovered link; cleared on mouseleave / when the + // pointer moves off the link (Linkifier `_clearCurrentLink`). + _currentLink?: unknown } type TerminalCoreWithLinkifier = { @@ -44,3 +47,22 @@ export function resetTerminalLinkifierHoverState(terminal: Terminal): void { /* linkifier internals unavailable — link recovers on the next cell change */ } } + +/** + * True while xterm is actively showing a hovered link. + * + * Why: callers that invalidate the hover cache on a timer (streamed output) + * must skip while a link is hovered — clearing the cache makes the next + * mousemove clear and (for async providers like file paths) re-query the active + * link, flickering its underline/tooltip. Guarded like {@link + * resetTerminalLinkifierHoverState} so a renamed field degrades to "not + * hovering" rather than throwing. + */ +export function isTerminalLinkifierHoverActive(terminal: Terminal): boolean { + try { + const linkifier = (terminal as unknown as TerminalCoreWithLinkifier)._core?.linkifier + return Boolean(linkifier && '_currentLink' in linkifier && linkifier._currentLink) + } catch { + return false + } +}