From 08671d877108769e99ce649bd9ea93a861a0b19b Mon Sep 17 00:00:00 2001 From: brooklyn! Date: Wed, 13 May 2026 13:52:10 -0700 Subject: [PATCH] tui: make URLs clickable + hover-highlight in any terminal (#25071) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * tui: make URLs clickable + hover-highlight in any terminal Problem ------- URLs printed by `hermes --tui` were not clickable in basic macOS Terminal.app. Cmd+click did nothing, the cursor didn't change shape — like nothing was detected — even though arrow buttons and other Box onClick handlers worked fine. Root cause ---------- Two layers of dead plumbing: 1. `` only emitted the underlying `` (which carries the hyperlink metadata into the screen buffer) when `supportsHyperlinks()` said yes. On Apple_Terminal that's false, so the per-cell hyperlink field stayed empty, so `Ink.getHyperlinkAt()` had nothing to return on click. The visible underline was just decorative. 2. `Ink.openHyperlink()` calls `this.onHyperlinkClick?.(url)`, but `onHyperlinkClick` was never assigned anywhere in the codebase. The click pipeline (`App.tsx → onOpenHyperlink → Ink.openHyperlink`) ran but bailed silently on the optional chain. Bonus discovery: even when wired up, there was no hover affordance — terminal apps can't change the system mouse cursor, so users had no visual signal that a cell was clickable. Arrow buttons in the chrome worked because they had explicit `` styling; inline link URLs didn't. Fix --- - `Link.tsx`: always emit `` regardless of terminal capability. The renderer's `wrapWithOsc8Link` already gates the actual OSC 8 escape on `supportsHyperlinks()` further down — so terminals that don't understand OSC 8 still don't see the escape, but the screen-buffer metadata (which the click dispatcher reads) is now populated everywhere. - `ink.tsx + root.ts`: add `onHyperlinkClick?: (url: string) => void` to `Options` / `RenderOptions`, wire it to the existing `Ink.onHyperlinkClick` field in the constructor. - `src/lib/openExternalUrl.ts`: small platform-aware opener using `child_process.spawn` with arg-array (no shell) — http(s) only, rejects `file:`, `javascript:`, `data:`, etc., so a hostile model can't trigger arbitrary local handlers via ``. Detached + stdio ignore so closing the TUI doesn't kill the browser and Chrome stderr doesn't leak into the alt screen. - `entry.tsx`: pass `onHyperlinkClick: openExternalUrl` to `ink.render`. - `hyperlinkHover.ts` + Ink hover wiring: track the URL under the pointer in `Ink.hoveredHyperlink`, update it from `dispatchHover`, and inverse- highlight every cell of the matching link in the render-pass overlay (same pattern as `applySearchHighlight`). This is the cursor-hover affordance for clickable links — terminals don't expose cursor shape, so we light up the link itself. - `types/hermes-ink.d.ts`: add `onHyperlinkClick` to the `RenderOptions` shim so consumers (`entry.tsx`) type-check against the new option. Tests ----- - `src/lib/openExternalUrl.test.ts` (15 cases): http(s) accepted; file/js/ data/mailto/ftp/ssh rejected; macOS open(1), Windows cmd.exe start with empty title slot, Linux xdg-open dispatch; shell-metacharacter URLs pass through unmolested as a single argv element; synchronous spawn failure returns false. Verified empirically in Apple Terminal 455.1 (macOS 15.7.3): clicking a URL opens in default browser, hovering inverts the link cells, and moving away clears the highlight. Full TUI suite: 713 passing, 0 type errors. Reverts ------- The earlier attempt that version-gated Apple_Terminal in `supports-hyperlinks.ts` was based on a wrong assumption — Terminal.app silently strips OSC 8 sequences but does not render them as clickable hyperlinks. Reverted to the original allowlist. * tui: address Copilot review — explorer.exe on win32 + comment fixes - openExternalUrl: switch win32 from `cmd.exe /c start` to `explorer.exe`. cmd.exe's `start` builtin reparses the URL through cmd's tokenizer, so `&`, `|`, `^`, `<`, `>` either split the command or get reinterpreted — breaking both the protocol-allowlist safety story AND plain http(s) URLs with `&` in query strings. `explorer.exe ` invokes the registered protocol handler directly with no shell. - openExternalUrl.test.ts: rename the win32 test to reflect the new contract and add two regression tests — one with `&|^<>` metachars, one with the common analytics-URL `&` query-param pattern — both pinned to single-argv-element delivery via explorer.exe. - Link.tsx: fix misleading comment. OSC 8 escapes are emitted unconditionally by the renderer (`wrapWithOsc8Link` in render-node-to-output.ts, `oscLink` in log-update.ts). Non-supporting terminals silently strip the sequence, which is why hover/click affordance has to come from the in-process overlay rather than the terminal's own link rendering. Verified: 715/715 tests pass, type-check + build clean. * tui: address Copilot review #2 — async spawn errors + hover scope + docs 1. openExternalUrl: attach a no-op `'error'` listener on the spawned child BEFORE unref(). spawn() returns a ChildProcess synchronously even when the binary is missing (ENOENT on xdg-open / explorer.exe), unreachable, or otherwise unusable; the failure surfaces later as an 'error' event. An unhandled 'error' on an EventEmitter crashes Node, which would tear down the whole TUI. The listener is a deliberate no-op — we already returned `true` synchronously and the user just doesn't see the browser pop. 2. openExternalUrl.test.ts: add a regression test using a real EventEmitter to simulate the async-error path. Pins both the listener-attached contract and the "doesn't throw on emit" behavior. Was 17/17, now 18/18. 3. ink.tsx dispatchHover: bypass `getHyperlinkAt()` and read `cellAt(...).hyperlink` directly. `getHyperlinkAt` falls back to `findPlainTextUrlAt` for cells without an OSC 8 hyperlink, but the render-pass overlay (`applyHyperlinkHoverHighlight`) only matches on `cell.hyperlink === hoveredUrl` — so plain-text URLs would burn re-renders without ever producing the highlight. Hover is now a strictly 1:1 fit for what the overlay can paint. Plain-text URLs still get the click action via the existing dispatch path. 4. root.ts + ink.tsx doc comments: replace the misleading "typically `open` / `xdg-open` / `start` shell" wording with the actual safe recipe — argv-array spawn into `open` / `xdg-open` / `explorer.exe`, with an explicit warning that `cmd.exe /c start` reparses the URL through cmd's tokenizer and is unsafe + breaks `&`-query URLs. Verified: 716/716 tests pass, type-check + build clean. * tui: address Copilot review #3 — hover damage, alt-screen cleanup, opener allowlist 1. ink.tsx onRender: stop folding steady-state hover into hlActive. hlActive forces a full-screen damage diff so previous-frame inverted cells get re-emitted when the highlight set changes. The transition IS the trigger — enter / leave / change-to-other-link. While the pointer just sits on a link the painted cells don't change and the per-cell diff handles the no-op. Folding the steady state in would burn a full-screen diff on every frame. Added a lastRenderedHoveredHyperlink tracker and gate the hlActive bump on `hovered !== lastRendered`. 2. ink.tsx setAltScreenActive: clear hoveredHyperlink (and the tracker) when toggling alt-screen state. Hover dispatch is alt-screen-gated, so once we leave there's no path to clear it. Without this, remounting would paint a phantom hover from the previous session until the next mouse-move arrived. 3. openExternalUrl.ts openCommand: allowlist linux + the BSD family for xdg-open and return null for everything else (aix, sunos, cygwin, haiku, etc.). Previously the default-fallback always returned xdg-open, which made the caller's `if (!command) return false` dead and yielded a misleading `true` on platforms that probably don't have xdg-open. New tests cover the null path AND the openExternalUrl-returns-false-without-spawning behavior. Verified: 718/718 tests pass, type-check + build clean. * tui: address Copilot review #4 — doc comment accuracy 1. openExternalUrl return-value doc: now lists all three false paths (URL rejected / no opener for platform / synchronous spawn throw) plus a note that async 'error' events still return true because the spawn was attempted. 2. ink.tsx onHyperlinkClick field doc: clarifies the callback receives either an OSC 8 hyperlink OR a plain-text URL detected by findPlainTextUrlAt — App.tsx routes both into the same callback. 3. hyperlinkHover applyHyperlinkHoverHighlight doc: drops the misleading 'caller forces full-frame damage' promise. Caller decides; for hover the current caller only forces full damage on transitions. No behavior change. 718/718 tests pass. * tui: address Copilot review #5 — lint fixes 1. ink.tsx: reorder `./hyperlinkHover.js` import before `./screen.js` to satisfy perfectionist/sort-imports. 2. Link.tsx: drop unused `fallback` parameter destructuring + the trailing `void (null as ...)` dead-statement (would trip no-unused-expressions). Kept `fallback?: ReactNode` on the Props interface as a documented compat shim so existing call sites still compile, with a comment explaining why it's no longer wired up. 3. openExternalUrl.test.ts: replace `typeof import('node:child_process').spawn` inline annotations (forbidden by @typescript-eslint/consistent-type-imports) with a `SpawnLike` type alias backed by a real `import type { spawn as SpawnFn }`. No behavior change. 718/718 tests pass, type-check clean, lint clean on all modified files. --- .../hermes-ink/src/ink/components/Link.tsx | 65 ++---- .../hermes-ink/src/ink/hyperlinkHover.ts | 52 +++++ ui-tui/packages/hermes-ink/src/ink/ink.tsx | 104 ++++++++- ui-tui/packages/hermes-ink/src/ink/root.ts | 22 +- ui-tui/src/entry.tsx | 13 +- ui-tui/src/lib/openExternalUrl.test.ts | 217 ++++++++++++++++++ ui-tui/src/lib/openExternalUrl.ts | 158 +++++++++++++ ui-tui/src/types/hermes-ink.d.ts | 1 + 8 files changed, 587 insertions(+), 45 deletions(-) create mode 100644 ui-tui/packages/hermes-ink/src/ink/hyperlinkHover.ts create mode 100644 ui-tui/src/lib/openExternalUrl.test.ts create mode 100644 ui-tui/src/lib/openExternalUrl.ts diff --git a/ui-tui/packages/hermes-ink/src/ink/components/Link.tsx b/ui-tui/packages/hermes-ink/src/ink/components/Link.tsx index 71c49145589..6020d50bdab 100644 --- a/ui-tui/packages/hermes-ink/src/ink/components/Link.tsx +++ b/ui-tui/packages/hermes-ink/src/ink/components/Link.tsx @@ -1,53 +1,38 @@ import type { ReactNode } from 'react' import React from 'react' -import { c as _c } from 'react/compiler-runtime' - -import { supportsHyperlinks } from '../supports-hyperlinks.js' import Text from './Text.js' export type Props = { readonly children?: ReactNode readonly url: string + // Kept for backwards-compat: prior versions rendered `fallback` instead of + // the linked content on terminals where supportsHyperlinks() was false. We + // now always emit the hyperlink metadata so the in-process click/hover + // dispatcher can act on it regardless of the terminal's own OSC 8 support + // (see comment in the function body), so `fallback` is no longer wired up. + // Leaving the prop on the interface keeps existing call sites compiling. readonly fallback?: ReactNode } -export default function Link(t0: Props) { - const $ = _c(5) - - const { children, url, fallback } = t0 - +export default function Link({ children, url }: Props): React.ReactNode { + // Always emit : the renderer stores `hyperlink` per cell in the + // screen buffer, which the click dispatcher (Ink.getHyperlinkAt → + // onHyperlinkClick) reads on mouseup to open URLs externally. Gating this + // on supportsHyperlinks() broke clicks in Apple Terminal / any terminal + // not on the OSC 8 allowlist — the cell's hyperlink field stayed empty, + // so the click pipeline had nothing to open. + // + // The OSC 8 escape itself is emitted unconditionally by the renderer + // (wrapWithOsc8Link in render-node-to-output.ts, oscLink in log-update.ts). + // Terminals that don't understand OSC 8 silently strip it — including + // Apple Terminal, which is why hover/click affordance has to come from + // the in-process overlay (applyHyperlinkHoverHighlight) and not from the + // terminal's own link rendering. const content = children ?? url - if (supportsHyperlinks()) { - let t1 - - if ($[0] !== content || $[1] !== url) { - t1 = ( - - {content} - - ) - $[0] = content - $[1] = url - $[2] = t1 - } else { - t1 = $[2] - } - - return t1 - } - - const t1 = fallback ?? content - let t2 - - if ($[3] !== t1) { - t2 = {t1} - $[3] = t1 - $[4] = t2 - } else { - t2 = $[4] - } - - return t2 + return ( + + {content} + + ) } -//# sourceMappingURL=data:application/json;charset=utf-8;base64,eyJ2ZXJzaW9uIjozLCJuYW1lcyI6WyJSZWFjdE5vZGUiLCJSZWFjdCIsInN1cHBvcnRzSHlwZXJsaW5rcyIsIlRleHQiLCJQcm9wcyIsImNoaWxkcmVuIiwidXJsIiwiZmFsbGJhY2siLCJMaW5rIiwidDAiLCIkIiwiX2MiLCJjb250ZW50IiwidDEiLCJ0MiJdLCJzb3VyY2VzIjpbIkxpbmsudHN4Il0sInNvdXJjZXNDb250ZW50IjpbImltcG9ydCB0eXBlIHsgUmVhY3ROb2RlIH0gZnJvbSAncmVhY3QnXG5pbXBvcnQgUmVhY3QgZnJvbSAncmVhY3QnXG5pbXBvcnQgeyBzdXBwb3J0c0h5cGVybGlua3MgfSBmcm9tICcuLi9zdXBwb3J0cy1oeXBlcmxpbmtzLmpzJ1xuaW1wb3J0IFRleHQgZnJvbSAnLi9UZXh0LmpzJ1xuXG5leHBvcnQgdHlwZSBQcm9wcyA9IHtcbiAgcmVhZG9ubHkgY2hpbGRyZW4/OiBSZWFjdE5vZGVcbiAgcmVhZG9ubHkgdXJsOiBzdHJpbmdcbiAgcmVhZG9ubHkgZmFsbGJhY2s/OiBSZWFjdE5vZGVcbn1cblxuZXhwb3J0IGRlZmF1bHQgZnVuY3Rpb24gTGluayh7XG4gIGNoaWxkcmVuLFxuICB1cmwsXG4gIGZhbGxiYWNrLFxufTogUHJvcHMpOiBSZWFjdC5SZWFjdE5vZGUge1xuICAvLyBVc2UgY2hpbGRyZW4gaWYgcHJvdmlkZWQsIG90aGVyd2lzZSBkaXNwbGF5IHRoZSBVUkxcbiAgY29uc3QgY29udGVudCA9IGNoaWxkcmVuID8/IHVybFxuXG4gIGlmIChzdXBwb3J0c0h5cGVybGlua3MoKSkge1xuICAgIC8vIFdyYXAgaW4gVGV4dCB0byBlbnN1cmUgd2UncmUgaW4gYSB0ZXh0IGNvbnRleHRcbiAgICAvLyAoaW5rLWxpbmsgaXMgYSB0ZXh0IGVsZW1lbnQgbGlrZSBpbmstdGV4dClcbiAgICByZXR1cm4gKFxuICAgICAgPFRleHQ+XG4gICAgICAgIDxpbmstbGluayBocmVmPXt1cmx9Pntjb250ZW50fTwvaW5rLWxpbms+XG4gICAgICA8L1RleHQ+XG4gICAgKVxuICB9XG5cbiAgcmV0dXJuIDxUZXh0PntmYWxsYmFjayA/PyBjb250ZW50fTwvVGV4dD5cbn1cbiJdLCJtYXBwaW5ncyI6IjtBQUFBLGNBQWNBLFNBQVMsUUFBUSxPQUFPO0FBQ3RDLE9BQU9DLEtBQUssTUFBTSxPQUFPO0FBQ3pCLFNBQVNDLGtCQUFrQixRQUFRLDJCQUEyQjtBQUM5RCxPQUFPQyxJQUFJLE1BQU0sV0FBVztBQUU1QixPQUFPLEtBQUtDLEtBQUssR0FBRztFQUNsQixTQUFTQyxRQUFRLENBQUMsRUFBRUwsU0FBUztFQUM3QixTQUFTTSxHQUFHLEVBQUUsTUFBTTtFQUNwQixTQUFTQyxRQUFRLENBQUMsRUFBRVAsU0FBUztBQUMvQixDQUFDO0FBRUQsZUFBZSxTQUFBUSxLQUFBQyxFQUFBO0VBQUEsTUFBQUMsQ0FBQSxHQUFBQyxFQUFBO0VBQWM7SUFBQU4sUUFBQTtJQUFBQyxHQUFBO0lBQUFDO0VBQUEsSUFBQUUsRUFJckI7RUFFTixNQUFBRyxPQUFBLEdBQWdCUCxRQUFlLElBQWZDLEdBQWU7RUFFL0IsSUFBSUosa0JBQWtCLENBQUMsQ0FBQztJQUFBLElBQUFXLEVBQUE7SUFBQSxJQUFBSCxDQUFBLFFBQUFFLE9BQUEsSUFBQUYsQ0FBQSxRQUFBSixHQUFBO01BSXBCTyxFQUFBLElBQUMsSUFBSSxDQUNILFNBQXlDLENBQXpCUCxJQUFHLENBQUhBLElBQUUsQ0FBQyxDQUFHTSxRQUFNLENBQUUsRUFBOUIsUUFBeUMsQ0FDM0MsRUFGQyxJQUFJLENBRUU7TUFBQUYsQ0FBQSxNQUFBRSxPQUFBO01BQUFGLENBQUEsTUFBQUosR0FBQTtNQUFBSSxDQUFBLE1BQUFHLEVBQUE7SUFBQTtNQUFBQSxFQUFBLEdBQUFILENBQUE7SUFBQTtJQUFBLE9BRlBHLEVBRU87RUFBQTtFQUlHLE1BQUFBLEVBQUEsR0FBQU4sUUFBbUIsSUFBbkJLLE9BQW1CO0VBQUEsSUFBQUUsRUFBQTtFQUFBLElBQUFKLENBQUEsUUFBQUcsRUFBQTtJQUExQkMsRUFBQSxJQUFDLElBQUksQ0FBRSxDQUFBRCxFQUFrQixDQUFFLEVBQTFCLElBQUksQ0FBNkI7SUFBQUgsQ0FBQSxNQUFBRyxFQUFBO0lBQUFILENBQUEsTUFBQUksRUFBQTtFQUFBO0lBQUFBLEVBQUEsR0FBQUosQ0FBQTtFQUFBO0VBQUEsT0FBbENJLEVBQWtDO0FBQUEiLCJpZ25vcmVMaXN0IjpbXX0= diff --git a/ui-tui/packages/hermes-ink/src/ink/hyperlinkHover.ts b/ui-tui/packages/hermes-ink/src/ink/hyperlinkHover.ts new file mode 100644 index 00000000000..92a43eb06ad --- /dev/null +++ b/ui-tui/packages/hermes-ink/src/ink/hyperlinkHover.ts @@ -0,0 +1,52 @@ +import { cellAtIndex, CellWidth, type Screen, setCellStyleId, type StylePool } from './screen.js' + +/** + * Highlight every cell whose OSC 8 hyperlink matches `hoveredUrl` by inverting + * its style. This is the cursor-hover affordance for clickable links: terminal + * applications can't change the system mouse cursor, so we light up the link + * itself when the pointer is over it. Same overlay machinery as + * applySearchHighlight — post-layout, pure SGR, picked up by the diff. + * + * Returns true if any cell was highlighted. The caller decides whether to + * promote that into a full-frame damage request — for hover specifically, + * full damage is only useful on enter/leave/change transitions (so the + * previous frame's inverted cells get re-emitted), not on every steady-state + * frame the pointer sits on the link. + */ +export function applyHyperlinkHoverHighlight( + screen: Screen, + hoveredUrl: string | undefined, + stylePool: StylePool +): boolean { + if (!hoveredUrl) { + return false + } + + const w = screen.width + const height = screen.height + let applied = false + + for (let row = 0; row < height; row++) { + const rowOff = row * w + + for (let col = 0; col < w; col++) { + const cell = cellAtIndex(screen, rowOff + col) + + // Skip SpacerTail — the head cell at col-1 owns the hyperlink, and + // setCellStyleId on the tail would split the styling of a wide-char + // glyph mid-cell. The head's restyle covers both halves. + if (cell.width === CellWidth.SpacerTail) { + continue + } + + if (cell.hyperlink !== hoveredUrl) { + continue + } + + applied = true + setCellStyleId(screen, col, row, stylePool.withInverse(cell.styleId)) + } + } + + return applied +} diff --git a/ui-tui/packages/hermes-ink/src/ink/ink.tsx b/ui-tui/packages/hermes-ink/src/ink/ink.tsx index c4669847e68..8a8603cf573 100644 --- a/ui-tui/packages/hermes-ink/src/ink/ink.tsx +++ b/ui-tui/packages/hermes-ink/src/ink/ink.tsx @@ -24,6 +24,7 @@ import { KeyboardEvent } from './events/keyboard-event.js' import { FocusManager } from './focus.js' import { emptyFrame, type Frame, type FrameEvent } from './frame.js' import { dispatchClick, dispatchHover, dispatchMouse } from './hit-test.js' +import { applyHyperlinkHoverHighlight } from './hyperlinkHover.js' import instances from './instances.js' import { LogUpdate } from './log-update.js' import { nodeCache } from './node-cache.js' @@ -150,6 +151,21 @@ export type Options = { patchConsole: boolean waitUntilExit?: () => Promise onFrame?: (event: FrameEvent) => void + /** + * Called when a click lands on a cell with an OSC 8 hyperlink (or a + * plain-text URL detected by findPlainTextUrlAt). The host is responsible + * for opening the URL — `child_process.spawn` with an argv array (NOT + * shell-mode) to the platform's native opener: `open` on macOS, + * `xdg-open` on Linux/BSD, `explorer.exe` on Windows. Avoid + * `cmd.exe /c start` — `start` is a cmd builtin that reparses the URL + * through cmd's tokenizer (`&` / `|` / `^` / `<` / `>` get split or + * reinterpreted), which both breaks plain URLs with `&` in query + * strings and undermines any caller-side protocol allowlist. Without + * this wired up, links rendered by `` look underlined but do + * nothing on click in any terminal where mouse tracking is on + * (Cmd+click is consumed by the TUI, not Terminal.app). + */ + onHyperlinkClick?: (url: string) => void } export default class Ink { private readonly log: LogUpdate @@ -232,6 +248,19 @@ export default class Ink { // so App.tsx's handleMouseEvent is stateless — dispatchHover diffs // against this set and mutates it in place. private readonly hoveredNodes = new Set() + + // The OSC 8 hyperlink URL under the pointer, or undefined when the cursor + // isn't on a link. Updated from dispatchHover; consumed by the render-pass + // overlay (applyHyperlinkHoverHighlight) to invert link cells under the + // pointer. This is the closest the TUI can get to the desktop's + // cursor-changes-on-hover affordance — terminals don't expose cursor + // shape control to applications. + private hoveredHyperlink: string | undefined = undefined + + // Last value of hoveredHyperlink that we actually painted. Compared in + // onRender so we can scope full-screen damage to enter/leave/change + // transitions, not every steady-state hover frame. + private lastRenderedHoveredHyperlink: string | undefined = undefined // Set by via setAltScreenActive(). Controls the // renderer's cursor.y clamping (keeps cursor in-viewport to avoid // LF-induced scroll when screen.height === terminalRows) and gates @@ -287,6 +316,14 @@ export default class Ink { this.restoreStderr = this.patchStderr() } + // Host-supplied hyperlink-open callback. The mouse-event pipeline + // (App.tsx → onOpenHyperlink → Ink.openHyperlink → onHyperlinkClick) + // is fully wired internally; without this assignment the optional + // chain in openHyperlink() bails silently and clicks on URLs do + // nothing. The field stays writable so tests / debug overlays can + // still rebind it after construction. + this.onHyperlinkClick = options.onHyperlinkClick + this.terminal = { stdout: options.stdout, stderr: options.stderr @@ -769,6 +806,26 @@ export default class Ink { // Position-highlight (below) overlays CURRENT (yellow) on top. hlActive = applySearchHighlight(frame.screen, this.searchHighlightQuery, this.stylePool) + // Hyperlink hover overlay: inverts every cell of the link currently + // under the pointer. Cheap-ish (linear scan of the visible buffer), + // only fires when hoveredHyperlink is set. + // + // hlActive controls full-screen damage (used by selection/search to + // make sure the previous frame's inverted cells get re-diffed when + // the highlight set changes). For hover, the *transition* is what + // needs the full-damage hammer — enter / leave / change-to-other-link. + // During steady-state hover the painted cells don't change and the + // ordinary per-cell diff handles the no-op. Folding the steady-state + // case into hlActive would burn full-screen diffs every frame while + // the pointer just sits on the link. + const hoverApplied = applyHyperlinkHoverHighlight(frame.screen, this.hoveredHyperlink, this.stylePool) + const hoverTransition = this.hoveredHyperlink !== this.lastRenderedHoveredHyperlink + this.lastRenderedHoveredHyperlink = this.hoveredHyperlink + + if (hoverApplied && hoverTransition) { + hlActive = true + } + // Position-based CURRENT: write yellow at positions[currentIdx] + // rowOffset. No scanning — positions came from a prior scan when // the message first mounted. Message-relative + rowOffset = screen. @@ -1182,6 +1239,16 @@ export default class Ink { this.altScreenActive = active this.altScreenMouseTracking = active && mouseTracking + // Hover state is alt-screen-scoped: dispatchHover is gated on + // altScreenActive, so once we leave the alt screen there's no path to + // clear it on our own. Without this reset, remounting + // would render a phantom hover highlight from the previous session + // until the next mouse-move event arrived. Clear both the live value + // and the last-rendered tracker so the next onRender sees no transition + // and no overlay. + this.hoveredHyperlink = undefined + this.lastRenderedHoveredHyperlink = undefined + if (active) { this.resetFramesForAltScreen() } else { @@ -1770,6 +1837,34 @@ export default class Ink { } dispatchHover(this.rootNode, col, row, this.hoveredNodes) + + // Hover affordance for hyperlinks: read the cell at the pointer, store + // its URL (or clear when the pointer leaves a link), and request a + // repaint when the value changes. The render-pass overlay paints the + // highlight; we just track which URL is "hot". + // + // IMPORTANT: bypass getHyperlinkAt() here — its plain-text URL fallback + // (findPlainTextUrlAt) would return URLs for cells whose `cell.hyperlink` + // is undefined, which the overlay (applyHyperlinkHoverHighlight) + // wouldn't match. That'd burn re-renders without ever producing an + // affordance. Read the OSC 8 hyperlink directly off the cell so the + // hover state is a 1:1 fit for what the overlay can paint. The + // plain-text URL fallback still works for clicks; hover is a strictly + // weaker signal and OK to skip on plain-text URLs. + const screen = this.frontFrame.screen + const cell = cellAt(screen, col, row) + let next = cell?.hyperlink + + // SpacerTail (second half of a wide-char / emoji glyph) stores the + // hyperlink on the head cell at col-1. Same logic as getHyperlinkAt. + if (!next && cell?.width === CellWidth.SpacerTail && col > 0) { + next = cellAt(screen, col - 1, row)?.hyperlink + } + + if (next !== this.hoveredHyperlink) { + this.hoveredHyperlink = next + this.scheduleRender() + } } dispatchKeyboardEvent(parsedKey: ParsedKey): void { const target = this.focusManager.activeElement ?? this.rootNode @@ -1814,8 +1909,13 @@ export default class Ink { } /** - * Optional callback fired when clicking an OSC 8 hyperlink in fullscreen - * mode. Set by FullscreenLayout via useLayoutEffect. + * Optional callback fired when clicking a cell that has an associated URL + * in fullscreen mode. `url` may be either an OSC 8 hyperlink (from a + * `` render or external OSC 8 escape that landed in the buffer) or + * a plain-text URL detected on the clicked row by findPlainTextUrlAt + * (App.tsx routes both into the same callback). Set from the host via + * the `onHyperlinkClick` Render/Ink option, or directly on the instance + * for late-bound test scenarios. */ onHyperlinkClick: ((url: string) => void) | undefined diff --git a/ui-tui/packages/hermes-ink/src/ink/root.ts b/ui-tui/packages/hermes-ink/src/ink/root.ts index 1d7af3803b4..41d02d52a0d 100644 --- a/ui-tui/packages/hermes-ink/src/ink/root.ts +++ b/ui-tui/packages/hermes-ink/src/ink/root.ts @@ -44,6 +44,22 @@ export type RenderOptions = { * Called after each frame render with timing and flicker information. */ onFrame?: (event: FrameEvent) => void + + /** + * Called when a click lands on a cell with an OSC 8 hyperlink (or a + * plain-text URL the renderer detects on the same row). The host owns + * the actual open — `child_process.spawn` with an argv array (NOT + * shell-mode) to the platform's native opener: `open` on macOS, + * `xdg-open` on Linux/BSD, `explorer.exe` on Windows. Avoid + * `cmd.exe /c start` — `start` is a cmd builtin that reparses the URL + * through cmd's tokenizer (`&` / `|` / `^` / `<` / `>` get split or + * reinterpreted as command syntax), which both breaks plain URLs with + * `&` in query strings and undermines any protocol allowlist on the + * caller side. Hermes wires this in `entry.tsx`; library users who + * don't pass it will see clickable underline styling but no action on + * click in any terminal where mouse tracking is on. + */ + onHyperlinkClick?: (url: string) => void } export type Instance = { @@ -138,7 +154,8 @@ export async function createRoot({ stderr = process.stderr, exitOnCtrlC = true, patchConsole = true, - onFrame + onFrame, + onHyperlinkClick }: RenderOptions = {}): Promise { // See wrappedRender — preserve microtask boundary from the old WASM await. await Promise.resolve() @@ -149,7 +166,8 @@ export async function createRoot({ stderr, exitOnCtrlC, patchConsole, - onFrame + onFrame, + onHyperlinkClick }) // Register in the instances map so that code that looks up the Ink diff --git a/ui-tui/src/entry.tsx b/ui-tui/src/entry.tsx index cfb0cd2f3f0..bfd56fa19d6 100644 --- a/ui-tui/src/entry.tsx +++ b/ui-tui/src/entry.tsx @@ -9,6 +9,7 @@ import { GatewayClient } from './gatewayClient.js' import { setupGracefulExit } from './lib/gracefulExit.js' import { formatBytes, type HeapDumpResult, performHeapDump } from './lib/memory.js' import { type MemorySnapshot, startMemoryMonitor } from './lib/memoryMonitor.js' +import { openExternalUrl } from './lib/openExternalUrl.js' import { resetTerminalModes } from './lib/terminalModes.js' if (!process.stdin.isTTY) { @@ -85,4 +86,14 @@ const onFrame = } : undefined -ink.render(, { exitOnCtrlC: false, onFrame }) +ink.render(, { + exitOnCtrlC: false, + onFrame, + // Open URLs in the user's default browser when a link cell is clicked. + // The TUI's mouse tracking captures click events before Terminal.app's + // own URL detection can fire, so without this hook clicks on `` + // do nothing in any terminal where mouseTracking is on. + onHyperlinkClick: url => { + openExternalUrl(url) + } +}) diff --git a/ui-tui/src/lib/openExternalUrl.test.ts b/ui-tui/src/lib/openExternalUrl.test.ts new file mode 100644 index 00000000000..3d280da3687 --- /dev/null +++ b/ui-tui/src/lib/openExternalUrl.test.ts @@ -0,0 +1,217 @@ +import type { ChildProcess, spawn as SpawnFn } from 'node:child_process' +import { EventEmitter } from 'node:events' + +import { describe, expect, it, vi } from 'vitest' + +import { openCommand, openExternalUrl, parseSafeUrl } from './openExternalUrl.js' + +type SpawnLike = typeof SpawnFn + +describe('parseSafeUrl', () => { + it('accepts http and https URLs', () => { + expect(parseSafeUrl('https://example.com')?.href).toBe('https://example.com/') + expect(parseSafeUrl('http://example.com/path?q=1')?.href).toBe('http://example.com/path?q=1') + }) + + it('rejects file: URLs (would let a hostile model trigger arbitrary local handlers)', () => { + expect(parseSafeUrl('file:///etc/passwd')).toBeNull() + }) + + it('rejects javascript:, data:, and vbscript: URLs', () => { + expect(parseSafeUrl('javascript:alert(1)')).toBeNull() + expect(parseSafeUrl('data:text/html,')).toBeNull() + expect(parseSafeUrl('vbscript:msgbox')).toBeNull() + }) + + it('rejects mailto:, ftp:, and other non-web protocols', () => { + expect(parseSafeUrl('mailto:test@example.com')).toBeNull() + expect(parseSafeUrl('ftp://example.com')).toBeNull() + expect(parseSafeUrl('ssh://example.com')).toBeNull() + }) + + it('rejects unparseable strings', () => { + expect(parseSafeUrl('not a url')).toBeNull() + expect(parseSafeUrl('')).toBeNull() + }) + + it('rejects non-string inputs defensively', () => { + expect(parseSafeUrl(undefined as unknown as string)).toBeNull() + expect(parseSafeUrl(null as unknown as string)).toBeNull() + expect(parseSafeUrl(123 as unknown as string)).toBeNull() + }) +}) + +describe('openCommand', () => { + it('returns macOS open(1) on darwin', () => { + expect(openCommand('darwin')).toEqual({ command: 'open', args: [] }) + }) + + it('routes through explorer.exe on win32 — not cmd.exe — so URLs with & | ^ < > stay safe', () => { + // win32 must not route through cmd.exe — see comment in openCommand. + // Test pins the contract that we use explorer.exe (non-shell) so URLs + // with `&`/`|`/`^`/`<`/`>` aren't reparsed by cmd's tokenizer. + const cmd = openCommand('win32') + expect(cmd?.command).toBe('explorer.exe') + expect(cmd?.args).toEqual([]) + }) + + it('falls back to xdg-open on linux/bsd', () => { + expect(openCommand('linux')).toEqual({ command: 'xdg-open', args: [] }) + expect(openCommand('freebsd')).toEqual({ command: 'xdg-open', args: [] }) + expect(openCommand('openbsd')).toEqual({ command: 'xdg-open', args: [] }) + }) + + it('returns null for unknown platforms (aix, sunos, cygwin, etc.)', () => { + // Avoid optimistically dispatching xdg-open on platforms where it + // probably isn't installed — the caller's `if (!command) return false` + // path surfaces "no opener" honestly instead. + expect(openCommand('aix')).toBeNull() + expect(openCommand('sunos')).toBeNull() + expect(openCommand('cygwin')).toBeNull() + expect(openCommand('haiku')).toBeNull() + expect(openCommand('')).toBeNull() + }) +}) + +describe('openExternalUrl on unsupported platforms', () => { + it('returns false without spawning when the platform has no known opener', () => { + const spawn = vi.fn() as unknown as SpawnLike + + expect(openExternalUrl('https://example.com/', { spawn, platform: () => 'aix' })).toBe(false) + expect(spawn).not.toHaveBeenCalled() + }) +}) + +describe('openExternalUrl', () => { + // Tracks the most recent fake child so tests can inspect its 'error' + // handlers and emit on it. Use a loose EventEmitter alias rather than + // ChildProcess — the latter's `unref` signature is strictly `() => void` + // and doesn't accept `vi.fn()` without a generic. + type FakeChild = EventEmitter & { unref: () => void } + + function mockSpawn(): { + spawn: SpawnLike + calls: Array<{ command: string; args: readonly string[] }> + lastChild: () => FakeChild | undefined + } { + const calls: Array<{ command: string; args: readonly string[] }> = [] + let lastChild: FakeChild | undefined + + const spawn = vi.fn((command: string, args: readonly string[]) => { + calls.push({ command, args }) + + // Use a real EventEmitter so .once('error', cb) wires up correctly + // and we can synthesize async failures by emitting 'error' from the + // test. The cast is the same one Node uses internally — ChildProcess + // extends EventEmitter. + const child = new EventEmitter() as FakeChild + + child.unref = () => {} + lastChild = child + + return child as unknown as ChildProcess + }) as unknown as SpawnLike + + return { spawn, calls, lastChild: () => lastChild } + } + + it('opens a normal https URL via the platform command', () => { + const { spawn, calls } = mockSpawn() + + expect(openExternalUrl('https://example.com/foo', { spawn, platform: () => 'darwin' })).toBe(true) + expect(calls).toHaveLength(1) + expect(calls[0]!.command).toBe('open') + expect(calls[0]!.args).toEqual(['https://example.com/foo']) + }) + + it('uses xdg-open on linux', () => { + const { spawn, calls } = mockSpawn() + + openExternalUrl('https://example.com/', { spawn, platform: () => 'linux' }) + expect(calls[0]!.command).toBe('xdg-open') + }) + + it('refuses to open file: URLs and does not spawn', () => { + const { spawn, calls } = mockSpawn() + + expect(openExternalUrl('file:///etc/passwd', { spawn, platform: () => 'darwin' })).toBe(false) + expect(calls).toHaveLength(0) + }) + + it('refuses to open javascript: URLs and does not spawn', () => { + const { spawn, calls } = mockSpawn() + + expect(openExternalUrl('javascript:alert(1)', { spawn, platform: () => 'darwin' })).toBe(false) + expect(calls).toHaveLength(0) + }) + + it('passes URLs containing shell metacharacters as plain args (no shell interpolation)', () => { + const { spawn, calls } = mockSpawn() + + // A URL with `; & ` plus URL-encoded backticks. spawn(..., args) without + // shell:true means the OS receives these as a single argv element. + const hostile = 'https://example.com/path%3Bevil%20%26%20rm%20-rf' + + openExternalUrl(hostile, { spawn, platform: () => 'darwin' }) + expect(calls).toHaveLength(1) + expect(calls[0]!.args[calls[0]!.args.length - 1]).toBe(hostile) + }) + + it('on win32, a URL with & | ^ < > is forwarded as a single argv element via explorer.exe', () => { + const { spawn, calls } = mockSpawn() + + // Plain http URL with & in query (very common, e.g. analytics params) + // plus other cmd metacharacters that would split or reinterpret the + // command if win32 routed through cmd.exe /c start. Note that the URL + // parser percent-encodes `<` and `>` (which is fine — encoded forms + // can't be reinterpreted by any shell), but `&`, `|`, `^` survive + // and would tokenize cmd.exe if we ever regressed back to it. + const meta = 'https://example.com/q?a=1&b=2|c^df' + + expect(openExternalUrl(meta, { spawn, platform: () => 'win32' })).toBe(true) + expect(calls).toHaveLength(1) + expect(calls[0]!.command).toBe('explorer.exe') + // The URL must arrive as exactly one argv element — not split on &/|/^/etc. + const forwarded = calls[0]!.args[0]! + expect(calls[0]!.args).toHaveLength(1) + expect(forwarded).toContain('a=1&b=2') + expect(forwarded).toContain('|c^d') + }) + + it('on win32, common http URLs with & query params are forwarded intact', () => { + const { spawn, calls } = mockSpawn() + const url = 'https://example.com/search?q=foo&page=2&utm_source=hermes' + + openExternalUrl(url, { spawn, platform: () => 'win32' }) + expect(calls[0]!.args).toEqual([url]) + }) + + it('returns false on synchronous spawn failure', () => { + const spawn = vi.fn(() => { + throw new Error('ENOENT') + }) as unknown as SpawnLike + + expect(openExternalUrl('https://example.com/', { spawn, platform: () => 'linux' })).toBe(false) + }) + + it('does not crash the host when the spawned process emits an async error', () => { + // Real-world case: `xdg-open` / `explorer.exe` missing on PATH. spawn() + // returns a ChildProcess synchronously, then emits 'error' once the + // exec actually fails. Without a registered 'error' listener, Node + // re-throws the event as an uncaught exception → TUI dies. We attach + // a no-op listener inside openExternalUrl; this test pins that contract. + const { spawn, lastChild } = mockSpawn() + + expect(openExternalUrl('https://example.com/', { spawn, platform: () => 'linux' })).toBe(true) + + const child = lastChild() + expect(child).toBeDefined() + // Must have a listener registered BEFORE we emit, or EventEmitter will + // throw synchronously here (which is exactly the crash we're preventing). + expect(child!.listenerCount('error')).toBeGreaterThan(0) + + // Emit and assert it doesn't throw. If the listener weren't attached, + // this would throw 'Unhandled error' and fail the test. + expect(() => child!.emit('error', new Error('ENOENT: xdg-open not found'))).not.toThrow() + }) +}) diff --git a/ui-tui/src/lib/openExternalUrl.ts b/ui-tui/src/lib/openExternalUrl.ts new file mode 100644 index 00000000000..6c095a8d16f --- /dev/null +++ b/ui-tui/src/lib/openExternalUrl.ts @@ -0,0 +1,158 @@ +import { spawn, type SpawnOptions } from 'node:child_process' +import { platform } from 'node:os' + +/** + * Opens an external URL in the user's default browser/handler. + * + * Wired into the Ink instance via `onHyperlinkClick` in entry.tsx, so any + * mouse click on a `` cell (or a row containing a plain-text URL the + * renderer detected) goes here. Mouse tracking inside the TUI prevents + * Terminal.app's native Cmd+click from firing — the click is captured + * before the terminal application sees it — so we have to handle the open + * ourselves. + * + * Safety: + * - http(s) only. Anything else (`file:`, `data:`, `javascript:`, etc.) is + * rejected — a hostile model could otherwise emit `` + * and trick a click into running an arbitrary local handler. + * - Hostname is parsed via `URL`; only well-formed URLs are forwarded. + * - Spawned via `child_process.spawn` with arg array (no shell), so a URL + * containing shell metacharacters (`;`, `&`, backticks) cannot be + * interpreted as a command. + * + * Returns `true` if the spawn was attempted, `false` if the open could + * not proceed — covers (a) URL rejected by `parseSafeUrl` (non-http(s), + * malformed, etc.), (b) no known opener for the current platform + * (`openCommand` returned null), or (c) `spawn()` threw synchronously + * before the child was created. Async failures after spawn (`'error'` + * event because the binary couldn't exec) still return `true` because + * the spawn was attempted — the no-op error listener absorbs the event + * so the TUI doesn't crash, and the user just doesn't see their browser + * pop. + */ +export function openExternalUrl(rawUrl: string, dependencies: OpenDependencies = {}): boolean { + const url = parseSafeUrl(rawUrl) + + if (!url) { + return false + } + + const spawnFn = dependencies.spawn ?? spawn + const platformId = dependencies.platform?.() ?? platform() + + const command = openCommand(platformId) + + if (!command) { + return false + } + + try { + const child = spawnFn(command.command, [...command.args, url.toString()], { + // Detach so closing the TUI later doesn't kill the browser process, + // and ignore stdio so we don't leak FDs into our raw-mode terminal. + // Without `ignore` here, Chrome's stderr can land in the alt screen. + detached: true, + stdio: 'ignore' + } satisfies SpawnOptions) + + // Async failure path: spawn returns a ChildProcess synchronously even + // when the binary is missing (ENOENT on `xdg-open` / `explorer.exe`), + // unreachable (EACCES), or otherwise unusable — the failure surfaces + // later as an 'error' event. Without a handler, an unhandled 'error' + // on an EventEmitter crashes Node, which would tear down the whole + // TUI. Attach a no-op listener BEFORE unref() so the event has a + // consumer; we already returned `true` synchronously, so the user + // just won't see their browser open — same as if the URL had been + // rejected upstream. + child.once('error', () => { + // Intentional no-op. The TUI keeps running; user gets no browser + // pop, which is the failure mode we promised in the doc comment. + }) + + child.unref() + + return true + } catch { + // spawn can also throw synchronously on argv-validation failures + // (e.g. NUL in the path). Treat it as a no-op rather than crashing. + return false + } +} + +export type OpenDependencies = { + spawn?: typeof spawn + platform?: () => string +} + +/** + * Validate and normalize a URL for opening externally. + * Exported for testing. + */ +export function parseSafeUrl(value: string): null | URL { + if (!value || typeof value !== 'string') { + return null + } + + let parsed: URL + + try { + parsed = new URL(value) + } catch { + return null + } + + // http(s) only — opening file://, data:, javascript:, vbscript:, etc. + // would let a malicious model run a local handler with attacker-controlled + // input on a single click. + if (parsed.protocol !== 'http:' && parsed.protocol !== 'https:') { + return null + } + + // Reject empty or all-whitespace hostnames defensively. URL parsing + // accepts URLs like 'http:///foo' on some Node versions; we don't want + // to forward those to `open`. + if (!parsed.hostname.trim()) { + return null + } + + return parsed +} + +type OpenCommand = { command: string; args: readonly string[] } + +/** + * Per-platform open command. We deliberately avoid `cmd.exe /c start` on + * Windows even though it's the canonical example, because `start` is a cmd + * builtin: the URL string is reparsed by cmd's command-line tokenizer and + * characters like `&`, `|`, `^`, `<`, `>` either break the command or get + * interpreted as additional commands. That undermines the protocol + * allowlist's safety story and also breaks plain http(s) URLs with `&` in + * query strings. `explorer.exe ` is the safe, non-shell alternative — + * it invokes the registered protocol handler for http(s) without going + * through cmd. Linux/BSD use `xdg-open` directly with no shell wrapping. + * + * Returns null for platforms where we don't know a safe opener (e.g. `aix`, + * `sunos`, `cygwin`). The caller's `if (!command) return false` path then + * surfaces "no opener" instead of optimistically trying `xdg-open` on a + * platform that probably doesn't have it. + */ +export function openCommand(platformId: string): OpenCommand | null { + if (platformId === 'darwin') { + return { command: 'open', args: [] } + } + + if (platformId === 'win32') { + return { command: 'explorer.exe', args: [] } + } + + // Linux + the BSD family ship xdg-open via xdg-utils. Everything else + // (aix, sunos, cygwin, haiku, etc.) returns null so openExternalUrl's + // command-not-found fallback fires honestly. + const XDG_OPEN_PLATFORMS = new Set(['linux', 'freebsd', 'openbsd', 'netbsd', 'dragonfly']) + + if (XDG_OPEN_PLATFORMS.has(platformId)) { + return { command: 'xdg-open', args: [] } + } + + return null +} diff --git a/ui-tui/src/types/hermes-ink.d.ts b/ui-tui/src/types/hermes-ink.d.ts index c8038576d3a..b84f843d322 100644 --- a/ui-tui/src/types/hermes-ink.d.ts +++ b/ui-tui/src/types/hermes-ink.d.ts @@ -66,6 +66,7 @@ declare module '@hermes/ink' { readonly exitOnCtrlC?: boolean readonly patchConsole?: boolean readonly onFrame?: (event: FrameEvent) => void + readonly onHyperlinkClick?: (url: string) => void } export type Instance = {