Skip to content

fix(terminal): re-linkify streamed URLs so links underline/click on first hover - #9320

Merged
nwparker merged 2 commits into
mainfrom
nwparker/term-links-not-clickable
Jul 18, 2026
Merged

nwparker merged 2 commits into
mainfrom
nwparker/term-links-not-clickable

Conversation

@nwparker

@nwparker nwparker commented Jul 18, 2026 •

Copy link
Copy Markdown
Contributor

What & why

Terminal URL links (e.g. the PR links an agent CLI like Claude Code prints) frequently stayed un-underlined and un-clickable until you clicked the terminal a few times. Then they'd "wake up" and become underlined + ⌘/Ctrl+click-able.

ELI5

xterm only draws the underline under a link when your mouse moves over it, and it remembers "this row has no link" from the last time your mouse was there. When an agent prints a link into the row your cursor is already resting on, xterm keeps trusting that stale "no link here" memory — so the link looks like plain text. Wiggling/clicking eventually moves the mouse across a row boundary, which makes xterm look again and finally notice the link.

This PR simply wipes that stale memory whenever new output arrives, so the very next mouse move notices the link. One move instead of several clicks.

Root cause

xterm's Linkifier recomputes a link's hover underline only on mousemove when the hovered buffer cell changes, and caches provider replies per line with no content-change invalidation (onRenderedViewportChange re-linkify is armed only while a link is already hovered). A URL streamed in under a stationary pointer is therefore never linkified — and native click activation stays dead too (it shares the same _currentLink precondition) — until the pointer crosses to a different line.

Orca already had the exact primitive for this — resetTerminalLinkifierHoverState (clears _lastBufferCell/_activeLine) — but it was wired only to visibility-resume (#9061, the "scroll to fix on tab switch" case), never to output streaming into an already-visible pane. That gap is the reported symptom.

The fix

  • New installTerminalLinkifierHoverResetOnWrite: a leading-edge-throttled (150 ms) terminal.onWriteParsed listener that clears the linkifier hover cache when output lands, so the next pointer move re-linkifies the fresh URL. Installed in openTerminal, disposed in disposePane.
  • Active-hover guard (isTerminalLinkifierHoverActive): the reset is skipped while a link is currently hovered (_currentLink set), so an in-progress hover is never cleared + re-queried (which would flicker the underline/tooltip for the async file-path provider). This provably doesn't reintroduce the bug — the reported scenario has the pointer over blank/non-link text, where _currentLink is null.
  • Throttle is leading-edge (not a debounce) on purpose: a debounce would starve during continuous streaming. A test locks this in.

Evidence

Diagnosis verified against the xterm 6.1.0-beta.287 source (Linkifier.ts): underline is mousemove-only; per-line reply cache is invalidated only on cross-line moves/resize; activation requires an existing _currentLink. Mouse-reporting mode / focus / pointer-events were all investigated and ruled out.

Tests (npx vitest run --config config/vitest.config.ts src/renderer/src/lib/pane-manager/ → 564 pass, 1 skipped):

  • openTerminal install + disposePane dispose are asserted against the real subscribe path (not the guarded no-op).
  • Throttle behavior is mutation-verified: removing the throttle guard fails the "one reset per window" test (20 vs 1); a debounce refactor fails the "no starvation during continuous streaming" test.
  • Active-hover guard: reset is skipped while _currentLink is set and resumes once it clears.
  • Guard degrades to a no-op if onWriteParsed/internals are absent.

Adversarial review: 2 rounds of independent fresh reviewers. Round 1 surfaced 3 real issues (silently-untested wiring, throttle undetectable by tests, mid-hover flicker) — all fixed here. Round 2 came back clean.

Perf (hot path — agents stream large volumes): negligible. Output is coalesced by Orca's scheduler and xterm's WriteBuffer fires onWriteParsed at most ~80/s per terminal regardless of throughput; the listener is a timer !== null check + return in the common case, one timer per 150 ms/pane, zero per-event allocation, and clearing the cache does no provideLinks/render work until a real mousemove (verified in xterm source). ≈ <20 µs CPU/s per streaming pane.

tc ✓ · oxlint ✓ · oxfmt ✓.

User-facing regression tradeoffs

Target: zero — and I believe it is zero.

  • No behavior change when idle / not streaming — the reset only fires after output lands.
  • No flicker for hovered links — the active-hover guard skips the reset mid-hover, so underlines/tooltips of a link you're pointing at are never disturbed. This is strictly better than a naive version.
  • No double-opens / no change to click routing — the existing click paths are untouched; this only affects when the hover cache is invalidated.
  • No perf regression on the streaming hot path (numbers above).
  • Fails safe — all xterm-internals access is guarded; if a future xterm renames a field, behavior degrades to exactly today's pre-fix behavior, never a throw/crash.

One honest micro-edge-case (not a regression vs. today): if you are hovering a link on a row and that exact row is rewritten in place to a different link while your pointer stays put, the guard defers the refresh until you move to another row. This is astronomically rare, matches xterm's own pre-existing behavior for in-place same-row rewrites, and self-heals on the next move.

Known separate limitation (out of scope, not introduced here)

URL underlines still come solely from the stock WebLinksAddon, which only understands a single physical row. URLs an agent draws inside a box or hard-wrapped across rows may never underline at all (distinct from this "underlines after interaction" bug). Orca's robust box/wrap-aware scanners are wired only to the click fallback, not to a link provider. Happy to do that as a follow-up.

Test plan

  • pnpm run test on src/renderer/src/lib/pane-manager/ — 564 pass
  • typecheck (web) clean
  • oxlint + oxfmt clean
  • Manual: in a terminal running an agent, have it print a URL under a stationary cursor → move the mouse onto it once → it underlines and ⌘/Ctrl+click opens it (previously needed several clicks).

Made with Orca 🐋

…irst hover

Terminal URL links (e.g. PR links an agent CLI prints) frequently stayed
un-underlined and un-clickable until the user clicked the pane a few times.

Root cause: xterm's Linkifier recomputes a link's underline only on mousemove
when the hovered buffer cell changes, and caches provider replies per line with
no content-change invalidation. A URL streamed in under a stationary pointer is
never linkified — and native activation stays dead (shares the _currentLink
precondition) — until the pointer crosses to a different line.

Orca already had resetTerminalLinkifierHoverState for this, but it was wired
only to visibility-resume (#9061), not to output streamed into a visible pane.

Add installTerminalLinkifierHoverResetOnWrite: a leading-edge-throttled (150ms)
onWriteParsed listener that clears the linkifier hover cache when output lands,
so the next pointer move re-linkifies the fresh URL. A guard skips the reset
while a link is actively hovered (_currentLink set) so an in-progress hover
never clears+re-queries (no underline/tooltip flicker). Installed in
openTerminal, disposed in disposePane.

Co-authored-by: Orca <help@stably.ai>
@coderabbitai

coderabbitai Bot commented Jul 18, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 9a967887-673c-4f4d-8d15-f21b6604c15f

📥 Commits

Reviewing files that changed from the base of the PR and between fb5878a and 3cdbec4.

📒 Files selected for processing (2)
  • src/renderer/src/lib/pane-manager/terminal-linkifier-hover-reset-on-write.test.ts
  • src/renderer/src/lib/pane-manager/terminal-linkifier-hover-reset-on-write.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/renderer/src/lib/pane-manager/terminal-linkifier-hover-reset-on-write.ts
  • src/renderer/src/lib/pane-manager/terminal-linkifier-hover-reset-on-write.test.ts

📝 Walkthrough

Walkthrough

Adds active-link detection for xterm linkifier internals and a throttled onWriteParsed handler that resets hover-cache state during streamed output while preserving active links. Integrates the handler into terminal pane creation and disposal, stores its disposable on managed panes, and adds tests for throttling, continuous writes, hover protection, missing handlers, and lifecycle cleanup.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description is detailed, but it does not follow the required template sections like Summary, Screenshots, AI Review Report, Security Audit, or Notes. Add the missing template sections, especially Summary, Screenshots, Testing checklist, AI Review Report, Security Audit, and Notes.
Docstring Coverage ⚠️ Warning Docstring coverage is 57.14% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title matches the main change: fixing streamed terminal URLs so they become linkified on first hover.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

… hover

Previously flush() dropped the pending reset when it fired while a link was
hovered, relying on a later write to re-arm it. If that was the last chunk of a
burst and it appended a link to the hovered line, the reset was lost and the
link stayed dead until a line change — the same symptom class this fix targets.
Re-arm the throttle instead so the reset runs once the hover ends, even if
output has gone quiet. Timer stays non-null during the retry so a concurrent
write cannot stack a second timer.
@nwparker
nwparker merged commit 403d681 into main Jul 18, 2026
4 checks passed
enrick-dev pushed a commit to enrick-dev/orca that referenced this pull request Oct 15, 2026
…irst hover (stablyai#9320)

Invalidate xterm's linkifier hover cache (throttled) when output streams into a visible pane, so a URL printed under a stationary pointer underlines and becomes Cmd/Ctrl+clickable on the next pointer move instead of after several clicks. Skips while a link is hovered (no flicker) and re-arms so the reset is never dropped mid-hover. Reuses the visibility-resume primitive (stablyai#9061).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant