From e2145a5c9cae337fad64ab6cf34fed730c00595b Mon Sep 17 00:00:00 2001 From: Austin Pickett Date: Thu, 11 Jun 2026 19:47:53 -0400 Subject: [PATCH] fix(ui-tui): stabilize embedded dashboard chat gateway (#44528) Cherry-picked from #39840 by @flyinhigh and rebased cleanly on main. - Defer config fetch in createGatewayEventHandler until gateway.ready to avoid render-phase RPC that can mutate transcript state and trigger React error 301 in embedded dashboard PTYs. - Use undici WebSocket fallback when globalThis.WebSocket is unavailable (Node attach mode and sidecar mirror sockets). - Add regression tests for both fixes. Co-authored-by: flyinhigh --- nix/lib.nix | 2 +- package-lock.json | 77 +++++++- ui-tui/package.json | 1 + .../createGatewayEventHandler.test.ts | 16 +- ui-tui/src/__tests__/gatewayClient.test.ts | 187 ++++++++---------- ui-tui/src/app/createGatewayEventHandler.ts | 11 +- ui-tui/src/gatewayClient.ts | 17 +- 7 files changed, 186 insertions(+), 125 deletions(-) diff --git a/nix/lib.nix b/nix/lib.nix index 64546b919dc..7d2fe511a40 100644 --- a/nix/lib.nix +++ b/nix/lib.nix @@ -21,7 +21,7 @@ let # Single npm deps fetch from the workspace root lockfile. # All workspace packages share this derivation. - npmDepsHash = "sha256-jN6rD+vVhTCWz3lFZzlmFYXmcMRPTtYWy3XVSiDYbvM="; + npmDepsHash = "sha256-BfTSh6J2VZ/07tq2DYnKgUViZCgRhW1sC2uj18H65SE="; npmDeps = pkgs.fetchNpmDeps { inherit src; diff --git a/package-lock.json b/package-lock.json index 6045382faf3..018074f3023 100644 --- a/package-lock.json +++ b/package-lock.json @@ -401,6 +401,13 @@ "node": ">=14.17" } }, + "apps/desktop/node_modules/undici-types": { + "version": "7.18.2", + "resolved": "https://registry.npmjs.org/undici-types/-/undici-types-7.18.2.tgz", + "integrity": "sha512-AsuCzffGHJybSaRrmr5eHr81mwJU3kjw6M+uprWvCXiNeN9SOGwQ3Jn8jb8m3Z6izVgknn1R0FTCEAP2QrLY/w==", + "dev": true, + "license": "MIT" + }, "apps/desktop/node_modules/vite": { "version": "8.0.10", "resolved": "https://registry.npmjs.org/vite/-/vite-8.0.10.tgz", @@ -8430,13 +8437,13 @@ "license": "MIT" }, "node_modules/@types/node": { - "version": "24.13.1", - "resolved": "https://registry.npmjs.org/@types/node/-/node-24.13.1.tgz", - "integrity": "sha512-RSpUJGmvsJ1ZeBehQZFhIdpsz+bIpES0nIQXko4Ybq+N+kX6XvOq3Jo+iJ82FWLdblFq85AsMikd3m35jgezYg==", + "version": "24.12.2", + "resolved": "https://registry.npmjs.org/@types/node/-/node-24.12.2.tgz", + "integrity": "sha512-A1sre26ke7HDIuY/M23nd9gfB+nrmhtYyMINbjI1zHJxYteKR6qSMX56FsmjMcDb3SMcjJg5BiRRgOCC/yBD0g==", "devOptional": true, "license": "MIT", "dependencies": { - "undici-types": "~7.18.0" + "undici-types": "~7.16.0" } }, "node_modules/@types/plist": { @@ -11589,6 +11596,25 @@ "@electron/windows-sign": "^1.1.2" } }, + "node_modules/electron-winstaller/node_modules/debug": { + "version": "4.4.3", + "resolved": "https://registry.npmjs.org/debug/-/debug-4.4.3.tgz", + "integrity": "sha512-RGwwWnwQvkVfavKVt22FGLw+xYSdzARwm0ru6DhTVA3umU5hZc28V3kO4stgYryrTlLpuvgI9GiijltAjNbcqA==", + "dev": true, + "license": "MIT", + "peer": true, + "dependencies": { + "ms": "^2.1.3" + }, + "engines": { + "node": ">=6.0" + }, + "peerDependenciesMeta": { + "supports-color": { + "optional": true + } + } + }, "node_modules/electron-winstaller/node_modules/fs-extra": { "version": "7.0.1", "resolved": "https://registry.npmjs.org/fs-extra/-/fs-extra-7.0.1.tgz", @@ -11616,6 +11642,14 @@ "graceful-fs": "^4.1.6" } }, + "node_modules/electron-winstaller/node_modules/ms": { + "version": "2.1.3", + "resolved": "https://registry.npmjs.org/ms/-/ms-2.1.3.tgz", + "integrity": "sha512-6FlzubTLZG3J2a/NVCAleEhjzq5oxgHyaCU9yYXvcLsvoVaHJq/s5xXI6/XXP6tz7R9xAOtHnSO/tXtF3WRTlA==", + "dev": true, + "license": "MIT", + "peer": true + }, "node_modules/electron-winstaller/node_modules/universalify": { "version": "0.1.2", "resolved": "https://registry.npmjs.org/universalify/-/universalify-0.1.2.tgz", @@ -14571,9 +14605,9 @@ } }, "node_modules/joi": { - "version": "18.2.1", - "resolved": "https://registry.npmjs.org/joi/-/joi-18.2.1.tgz", - "integrity": "sha512-2/OKlogiESf2Nh3TFCrRjrr9z1DRHeW0I+KReF67+4J0Ns+8hBtHRmoWAZ2OFU6I5+TWLEe6sVlSdXPjHm5UbQ==", + "version": "18.1.2", + "resolved": "https://registry.npmjs.org/joi/-/joi-18.1.2.tgz", + "integrity": "sha512-rF5MAmps5esSlhCA+N1b6IYHDw9j/btzGaqfgie522jS02Ju/HXBxamlXVlKEHAxoMKQL77HWI8jlqWsFuekZA==", "dev": true, "license": "BSD-3-Clause", "dependencies": { @@ -20341,9 +20375,9 @@ } }, "node_modules/undici-types": { - "version": "7.18.2", - "resolved": "https://registry.npmjs.org/undici-types/-/undici-types-7.18.2.tgz", - "integrity": "sha512-AsuCzffGHJybSaRrmr5eHr81mwJU3kjw6M+uprWvCXiNeN9SOGwQ3Jn8jb8m3Z6izVgknn1R0FTCEAP2QrLY/w==", + "version": "7.16.0", + "resolved": "https://registry.npmjs.org/undici-types/-/undici-types-7.16.0.tgz", + "integrity": "sha512-Zz+aZWSj8LE6zoxD+xrjh4VfkIG8Ya6LvYkZqtUQGJPZjYl53ypCaUwWqo7eI0x66KBGeRo+mlBEkMSeSZ38Nw==", "devOptional": true, "license": "MIT" }, @@ -21426,6 +21460,7 @@ "ink-text-input": "^6.0.0", "nanostores": "^1.2.0", "react": "^19.2.4", + "undici": "^6.25.0", "unicode-animations": "^1.0.3" }, "devDependencies": { @@ -22036,6 +22071,21 @@ "node": ">=14.17" } }, + "ui-tui/node_modules/undici": { + "version": "6.26.0", + "resolved": "https://registry.npmjs.org/undici/-/undici-6.26.0.tgz", + "integrity": "sha512-4yqz8a3n5HmGTlsbADNtr/dJlhkh/55Rq798G6ibiULcXbDtaLpTl1pvdqcbFfeoj3iSi52lePFM7h9H21cw/A==", + "engines": { + "node": ">=18.17" + } + }, + "ui-tui/node_modules/undici-types": { + "version": "7.18.2", + "resolved": "https://registry.npmjs.org/undici-types/-/undici-types-7.18.2.tgz", + "integrity": "sha512-AsuCzffGHJybSaRrmr5eHr81mwJU3kjw6M+uprWvCXiNeN9SOGwQ3Jn8jb8m3Z6izVgknn1R0FTCEAP2QrLY/w==", + "dev": true, + "license": "MIT" + }, "ui-tui/node_modules/wrap-ansi": { "version": "9.0.2", "resolved": "https://registry.npmjs.org/wrap-ansi/-/wrap-ansi-9.0.2.tgz", @@ -22251,6 +22301,13 @@ "engines": { "node": ">=14.17" } + }, + "web/node_modules/undici-types": { + "version": "7.18.2", + "resolved": "https://registry.npmjs.org/undici-types/-/undici-types-7.18.2.tgz", + "integrity": "sha512-AsuCzffGHJybSaRrmr5eHr81mwJU3kjw6M+uprWvCXiNeN9SOGwQ3Jn8jb8m3Z6izVgknn1R0FTCEAP2QrLY/w==", + "dev": true, + "license": "MIT" } } } diff --git a/ui-tui/package.json b/ui-tui/package.json index f6767da678b..c81ccc4e8d0 100644 --- a/ui-tui/package.json +++ b/ui-tui/package.json @@ -22,6 +22,7 @@ "ink-text-input": "^6.0.0", "nanostores": "^1.2.0", "react": "^19.2.4", + "undici": "^6.25.0", "unicode-animations": "^1.0.3" }, "devDependencies": { diff --git a/ui-tui/src/__tests__/createGatewayEventHandler.test.ts b/ui-tui/src/__tests__/createGatewayEventHandler.test.ts index 121b9011c7f..30776e43de1 100644 --- a/ui-tui/src/__tests__/createGatewayEventHandler.test.ts +++ b/ui-tui/src/__tests__/createGatewayEventHandler.test.ts @@ -658,6 +658,17 @@ describe('createGatewayEventHandler', () => { }) }) + it('does not fetch config while constructing the gateway event handler', () => { + const appended: Msg[] = [] + const ctx = buildCtx(appended) + + ctx.gateway.rpc = vi.fn(async () => null) + + createGatewayEventHandler(ctx) + + expect(ctx.gateway.rpc).not.toHaveBeenCalled() + }) + it('on gateway.ready with no STARTUP_RESUME_ID and auto_resume off, forges a new session', async () => { const appended: Msg[] = [] const newSession = vi.fn() @@ -1020,8 +1031,9 @@ describe('createGatewayEventHandler', () => { ) const onEvent = createGatewayEventHandler(ctx) - // Eager config fetch fires at creation; let it resolve before any spawn - // (mirrors real usage — config lands well before the first delegation). + // Config fetch starts once the gateway is ready; let it resolve before any + // spawn (mirrors real usage — config lands well before first delegation). + onEvent({ payload: {}, type: 'gateway.ready' } as any) await Promise.resolve() await Promise.resolve() diff --git a/ui-tui/src/__tests__/gatewayClient.test.ts b/ui-tui/src/__tests__/gatewayClient.test.ts index f1228e56fbe..a872a008ddb 100644 --- a/ui-tui/src/__tests__/gatewayClient.test.ts +++ b/ui-tui/src/__tests__/gatewayClient.test.ts @@ -1,97 +1,103 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import { GatewayClient } from '../gatewayClient.js' - interface ListenerEntry { callback: (event: any) => void once: boolean } -class FakeWebSocket { - static CONNECTING = 0 - static OPEN = 1 - static CLOSING = 2 - static CLOSED = 3 - static instances: FakeWebSocket[] = [] +const { FakeWebSocket } = vi.hoisted(() => { + class FakeWebSocket { + static CONNECTING = 0 + static OPEN = 1 + static CLOSING = 2 + static CLOSED = 3 + static instances: FakeWebSocket[] = [] - readyState = FakeWebSocket.CONNECTING - sent: string[] = [] - readonly url: string - private listeners = new Map() + readyState = FakeWebSocket.CONNECTING + sent: string[] = [] + readonly url: string + private listeners = new Map() - constructor(url: string) { - this.url = url - FakeWebSocket.instances.push(this) - } - - static reset() { - FakeWebSocket.instances = [] - } - - addEventListener(type: string, callback: (event: any) => void, options?: unknown) { - const once = - typeof options === 'object' && - options !== null && - 'once' in options && - Boolean((options as { once?: unknown }).once) - - const entries = this.listeners.get(type) ?? [] - - entries.push({ callback, once }) - this.listeners.set(type, entries) - } - - removeEventListener(type: string, callback: (event: any) => void) { - const entries = this.listeners.get(type) - - if (!entries) { - return + constructor(url: string) { + this.url = url + FakeWebSocket.instances.push(this) } - this.listeners.set( - type, - entries.filter(entry => entry.callback !== callback) - ) - } - - send(payload: string) { - if (this.readyState !== FakeWebSocket.OPEN) { - throw new Error('socket not open') + static reset() { + FakeWebSocket.instances = [] } - this.sent.push(payload) - } + addEventListener(type: string, callback: (event: any) => void, options?: unknown) { + const once = + typeof options === 'object' && + options !== null && + 'once' in options && + Boolean((options as { once?: unknown }).once) - close(code = 1000) { - if (this.readyState === FakeWebSocket.CLOSED) { - return + const entries = this.listeners.get(type) ?? [] + + entries.push({ callback, once }) + this.listeners.set(type, entries) } - this.readyState = FakeWebSocket.CLOSED - this.emit('close', { code }) - } + removeEventListener(type: string, callback: (event: any) => void) { + const entries = this.listeners.get(type) - open() { - this.readyState = FakeWebSocket.OPEN - this.emit('open', {}) - } + if (!entries) { + return + } - message(data: string) { - this.emit('message', { data }) - } + this.listeners.set( + type, + entries.filter(entry => entry.callback !== callback) + ) + } - private emit(type: string, event: any) { - const entries = [...(this.listeners.get(type) ?? [])] + send(payload: string) { + if (this.readyState !== FakeWebSocket.OPEN) { + throw new Error('socket not open') + } - for (const entry of entries) { - entry.callback(event) + this.sent.push(payload) + } - if (entry.once) { - this.removeEventListener(type, entry.callback) + close(code = 1000) { + if (this.readyState === FakeWebSocket.CLOSED) { + return + } + + this.readyState = FakeWebSocket.CLOSED + this.emit('close', { code }) + } + + open() { + this.readyState = FakeWebSocket.OPEN + this.emit('open', {}) + } + + message(data: string) { + this.emit('message', { data }) + } + + private emit(type: string, event: any) { + const entries = [...(this.listeners.get(type) ?? [])] + + for (const entry of entries) { + entry.callback(event) + + if (entry.once) { + this.removeEventListener(type, entry.callback) + } } } } -} + + return { FakeWebSocket } +}) + +vi.mock('undici', () => ({ WebSocket: FakeWebSocket })) + +import { GatewayClient } from '../gatewayClient.js' describe('GatewayClient websocket attach mode', () => { const originalWebSocket = globalThis.WebSocket @@ -269,30 +275,15 @@ describe('GatewayClient websocket attach mode', () => { gw.kill() }) - it('redacts query string secrets in attach failure logs and events', () => { + it('uses the undici WebSocket fallback when global WebSocket is unavailable', () => { process.env.HERMES_TUI_GATEWAY_URL = 'ws://gateway.test/api/ws?token=hunter2&channel=secret' delete (globalThis as { WebSocket?: unknown }).WebSocket const gw = new GatewayClient() - const stderrLines: string[] = [] - gw.on('event', ev => { - if (ev.type === 'gateway.stderr' && typeof ev.payload?.line === 'string') { - stderrLines.push(ev.payload.line) - } - }) gw.start() - gw.drain() - - expect(stderrLines.length).toBeGreaterThan(0) - - for (const line of stderrLines) { - expect(line).not.toContain('hunter2') - expect(line).not.toContain('channel=secret') - } - - expect(gw.getLogTail(20)).not.toContain('hunter2') - expect(gw.getLogTail(20)).not.toContain('channel=secret') + expect(FakeWebSocket.instances).toHaveLength(1) + expect(FakeWebSocket.instances[0]?.url).toBe('ws://gateway.test/api/ws?token=hunter2&channel=secret') gw.kill() }) @@ -363,27 +354,17 @@ describe('GatewayClient websocket attach mode', () => { expect(() => new URL(fixture)).toThrow() process.env.HERMES_TUI_GATEWAY_URL = fixture - delete (globalThis as { WebSocket?: unknown }).WebSocket + ;(globalThis as { WebSocket?: unknown }).WebSocket = class ThrowingWebSocket extends FakeWebSocket { + constructor(url: string) { + throw new TypeError(`Invalid URL: ${url}`) + } + } as unknown as typeof WebSocket const gw = new GatewayClient() - const stderrLines: string[] = [] - gw.on('event', ev => { - if (ev.type === 'gateway.stderr' && typeof ev.payload?.line === 'string') { - stderrLines.push(ev.payload.line) - } - }) gw.start() gw.drain() - expect(stderrLines.length).toBeGreaterThan(0) - - for (const line of stderrLines) { - expect(line).not.toContain('alice') - expect(line).not.toContain('hunter2') - expect(line).not.toContain('token=secret') - } - const tail = gw.getLogTail(20) expect(tail).not.toContain('alice') expect(tail).not.toContain('hunter2') diff --git a/ui-tui/src/app/createGatewayEventHandler.ts b/ui-tui/src/app/createGatewayEventHandler.ts index 7636eb6946f..438fd4f65e5 100644 --- a/ui-tui/src/app/createGatewayEventHandler.ts +++ b/ui-tui/src/app/createGatewayEventHandler.ts @@ -219,11 +219,6 @@ export function createGatewayEventHandler(ctx: GatewayEventHandlerContext): (ev: agentsNudgedThisTurn = false } - // Kick off the config fetch eagerly at handler creation so the flag is - // resolved well before the first delegation of any real session (which - // only happens after gateway.ready + a user turn). - ensureAgentsNudgeConfig() - const refreshDelegationStatus = (force = false) => { const now = Date.now() @@ -312,6 +307,12 @@ export function createGatewayEventHandler(ctx: GatewayEventHandlerContext): (ev: applySkin(skin) } + // Kick off the config fetch once the gateway is actually ready. If handler + // construction does this during React render, a startup transport error can + // report through sys(), mutate transcript state, and trip React's + // "too many re-renders" guard in embedded dashboard PTYs. + ensureAgentsNudgeConfig() + rpc('commands.catalog', {}) .then(r => { if (!r?.pairs) { diff --git a/ui-tui/src/gatewayClient.ts b/ui-tui/src/gatewayClient.ts index 2774d3de0e6..5dfbe880fb1 100644 --- a/ui-tui/src/gatewayClient.ts +++ b/ui-tui/src/gatewayClient.ts @@ -4,6 +4,8 @@ import { existsSync } from 'node:fs' import { delimiter, resolve } from 'node:path' import { createInterface } from 'node:readline' +import { WebSocket as UndiciWebSocket } from 'undici' + import type { GatewayEvent } from './gatewayTypes.js' import { CircularBuffer } from './lib/circularBuffer.js' import { recordParentLifecycle } from './lib/parentLog.js' @@ -19,6 +21,9 @@ const WS_OPEN = 1 const WS_CLOSING = 2 const WS_CLOSED = 3 +const getWebSocketCtor = (): typeof WebSocket => + typeof WebSocket === 'undefined' ? (UndiciWebSocket as unknown as typeof WebSocket) : WebSocket + const truncateLine = (line: string) => line.length > MAX_LOG_LINE_BYTES ? `${line.slice(0, MAX_LOG_LINE_BYTES)}… [truncated ${line.length} bytes]` : line @@ -262,14 +267,16 @@ export class GatewayClient extends EventEmitter { return } - if (typeof WebSocket === 'undefined') { + const WebSocketCtor = getWebSocketCtor() + + if (typeof WebSocketCtor === 'undefined') { this.pushLog(`[sidecar] WebSocket unavailable; skipping mirror to ${redactUrl(this.sidecarUrl)}`) return } try { - const ws = new WebSocket(this.sidecarUrl) + const ws = new WebSocketCtor(this.sidecarUrl) this.sidecarWs = ws ws.addEventListener('close', () => { @@ -402,7 +409,9 @@ export class GatewayClient extends EventEmitter { const safeAttachUrl = redactUrl(attachUrl) this.startReadyTimer('websocket', safeAttachUrl) - if (typeof WebSocket === 'undefined') { + const WebSocketCtor = getWebSocketCtor() + + if (typeof WebSocketCtor === 'undefined') { const line = `[startup] WebSocket API unavailable; cannot attach to ${safeAttachUrl}` this.pushLog(line) @@ -413,7 +422,7 @@ export class GatewayClient extends EventEmitter { } try { - const ws = new WebSocket(attachUrl) + const ws = new WebSocketCtor(attachUrl) let settled = false this.ws = ws