mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-06-08 08:11:38 +00:00
* 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. `<Link>` only emitted the underlying `<ink-link>` (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 `<Box onClick>` styling; inline link URLs didn't. Fix --- - `Link.tsx`: always emit `<ink-link>` 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 `<Link url="file:///...">`. 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 <url>` 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 <AlternateScreen> 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.
204 lines
5.6 KiB
TypeScript
204 lines
5.6 KiB
TypeScript
import { Stream } from 'stream'
|
|
|
|
import type { ReactNode } from 'react'
|
|
|
|
import { logForDebugging } from '../utils/debug.js'
|
|
|
|
import type { FrameEvent } from './frame.js'
|
|
import Ink, { type Options as InkOptions } from './ink.js'
|
|
import instances from './instances.js'
|
|
|
|
export type RenderOptions = {
|
|
/**
|
|
* Output stream where app will be rendered.
|
|
*
|
|
* @default process.stdout
|
|
*/
|
|
stdout?: NodeJS.WriteStream
|
|
/**
|
|
* Input stream where app will listen for input.
|
|
*
|
|
* @default process.stdin
|
|
*/
|
|
stdin?: NodeJS.ReadStream
|
|
/**
|
|
* Error stream.
|
|
* @default process.stderr
|
|
*/
|
|
stderr?: NodeJS.WriteStream
|
|
/**
|
|
* Configure whether Ink should listen to Ctrl+C keyboard input and exit the app. This is needed in case `process.stdin` is in raw mode, because then Ctrl+C is ignored by default and process is expected to handle it manually.
|
|
*
|
|
* @default true
|
|
*/
|
|
exitOnCtrlC?: boolean
|
|
|
|
/**
|
|
* Patch console methods to ensure console output doesn't mix with Ink output.
|
|
*
|
|
* @default true
|
|
*/
|
|
patchConsole?: boolean
|
|
|
|
/**
|
|
* 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 = {
|
|
/**
|
|
* Replace previous root node with a new one or update props of the current root node.
|
|
*/
|
|
rerender: Ink['render']
|
|
/**
|
|
* Manually unmount the whole Ink app.
|
|
*/
|
|
unmount: Ink['unmount']
|
|
/**
|
|
* Returns a promise, which resolves when app is unmounted.
|
|
*/
|
|
waitUntilExit: Ink['waitUntilExit']
|
|
cleanup: () => void
|
|
}
|
|
|
|
/**
|
|
* A managed Ink root, similar to react-dom's createRoot API.
|
|
* Separates instance creation from rendering so the same root
|
|
* can be reused for multiple sequential screens.
|
|
*/
|
|
export type Root = {
|
|
render: (node: ReactNode) => void
|
|
unmount: () => void
|
|
waitUntilExit: () => Promise<void>
|
|
}
|
|
|
|
export const forceRedraw = (stdout: NodeJS.WriteStream = process.stdout): boolean => {
|
|
const instance = instances.get(stdout)
|
|
|
|
if (!instance) {
|
|
return false
|
|
}
|
|
|
|
instance.forceRedraw()
|
|
|
|
return true
|
|
}
|
|
|
|
/**
|
|
* Mount a component and render the output.
|
|
*/
|
|
export const renderSync = (node: ReactNode, options?: NodeJS.WriteStream | RenderOptions): Instance => {
|
|
const opts = getOptions(options)
|
|
|
|
const inkOptions: InkOptions = {
|
|
stdout: process.stdout,
|
|
stdin: process.stdin,
|
|
stderr: process.stderr,
|
|
exitOnCtrlC: true,
|
|
patchConsole: true,
|
|
...opts
|
|
}
|
|
|
|
const instance: Ink = getInstance(inkOptions.stdout, () => new Ink(inkOptions))
|
|
|
|
instance.render(node)
|
|
|
|
return {
|
|
rerender: instance.render,
|
|
unmount() {
|
|
instance.unmount()
|
|
},
|
|
waitUntilExit: instance.waitUntilExit,
|
|
cleanup: () => instances.delete(inkOptions.stdout)
|
|
}
|
|
}
|
|
|
|
const wrappedRender = async (node: ReactNode, options?: NodeJS.WriteStream | RenderOptions): Promise<Instance> => {
|
|
// Preserve the microtask boundary that `await loadYoga()` used to provide.
|
|
// Without it, the first render fires synchronously before async startup work
|
|
// (e.g. useReplBridge notification state) settles, and the subsequent Static
|
|
// write overwrites scrollback instead of appending below the logo.
|
|
await Promise.resolve()
|
|
const instance = renderSync(node, options)
|
|
logForDebugging(`[render] first ink render: ${Math.round(process.uptime() * 1000)}ms since process start`)
|
|
|
|
return instance
|
|
}
|
|
|
|
export default wrappedRender
|
|
|
|
/**
|
|
* Create an Ink root without rendering anything yet.
|
|
* Like react-dom's createRoot — call root.render() to mount a tree.
|
|
*/
|
|
export async function createRoot({
|
|
stdout = process.stdout,
|
|
stdin = process.stdin,
|
|
stderr = process.stderr,
|
|
exitOnCtrlC = true,
|
|
patchConsole = true,
|
|
onFrame,
|
|
onHyperlinkClick
|
|
}: RenderOptions = {}): Promise<Root> {
|
|
// See wrappedRender — preserve microtask boundary from the old WASM await.
|
|
await Promise.resolve()
|
|
|
|
const instance = new Ink({
|
|
stdout,
|
|
stdin,
|
|
stderr,
|
|
exitOnCtrlC,
|
|
patchConsole,
|
|
onFrame,
|
|
onHyperlinkClick
|
|
})
|
|
|
|
// Register in the instances map so that code that looks up the Ink
|
|
// instance by stdout (e.g. external editor pause/resume) can find it.
|
|
instances.set(stdout, instance)
|
|
|
|
return {
|
|
render: node => instance.render(node),
|
|
unmount: () => instance.unmount(),
|
|
waitUntilExit: () => instance.waitUntilExit()
|
|
}
|
|
}
|
|
|
|
const getOptions = (stdout: NodeJS.WriteStream | RenderOptions | undefined = {}): RenderOptions => {
|
|
if (stdout instanceof Stream) {
|
|
return {
|
|
stdout,
|
|
stdin: process.stdin
|
|
}
|
|
}
|
|
|
|
return stdout
|
|
}
|
|
|
|
const getInstance = (stdout: NodeJS.WriteStream, createInstance: () => Ink): Ink => {
|
|
let instance = instances.get(stdout)
|
|
|
|
if (!instance) {
|
|
instance = createInstance()
|
|
instances.set(stdout, instance)
|
|
}
|
|
|
|
return instance
|
|
}
|