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 <flyinhigh@users.noreply.github.com>
This commit is contained in:
co-authored by
flyinhigh
parent
9102d4a588
commit
e2145a5c9c
@@ -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()
|
||||
|
||||
|
||||
@@ -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<string, ListenerEntry[]>()
|
||||
readyState = FakeWebSocket.CONNECTING
|
||||
sent: string[] = []
|
||||
readonly url: string
|
||||
private listeners = new Map<string, ListenerEntry[]>()
|
||||
|
||||
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')
|
||||
|
||||
@@ -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<CommandsCatalogResponse>('commands.catalog', {})
|
||||
.then(r => {
|
||||
if (!r?.pairs) {
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user