mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-05-13 03:52:00 +00:00
feat(tui): support attaching to an existing gateway (#21978)
* feat(tui): support attaching to an existing gateway Allow the TUI gateway client to connect via HERMES_TUI_GATEWAY_URL while preserving spawned gateway fallback, and mirror event frames to sidecar feeds so dashboard tool activity remains visible. * review(copilot): redact attach URLs and gate stale transport exits Strip query strings (and any user info) from gateway / sidecar URLs before logging or surfacing them in `gateway.start_timeout`, so attach tokens never leak into the TUI log tail or activity feed. Also gate the spawned-proc and websocket close handlers on transport identity so a stale child or socket cannot clear a freshly-started ready timer or reject newly-issued pending requests during reconnect. * review(copilot): tighten transport restart and shutdown lifecycle Reject any in-flight RPCs in resetStartupState so callers do not hang on promises issued to the previous transport when start() swaps a child or socket. Have kill() explicitly reject pending so attach-mode promises drain after an intentional shutdown, and reattach when HERMES_TUI_GATEWAY_URL rotates between requests instead of silently keeping the old session. Fold the spawned child error path through handleTransportExit so a failed spawn clears the startup timer and emits a single exit event. Also null the websocket reference before calling close so the identity guard correctly tags stale close events on real WebSocket timing. Locks the new behaviors in with regression tests for kill, URL rotation, and stale-pending cleanup. * review(copilot): swallow stray ws connect rejection and isolate test env Attach a no-op catch handler on the websocket connect promise so an unobserved connect-error / early-close rejection cannot surface as an unhandled promise rejection in Node when no request is currently racing the open. Snapshot HERMES_TUI_GATEWAY_URL / HERMES_TUI_SIDECAR_URL in beforeEach and restore them in afterEach so vitest runs that set those env vars beforehand do not get permanently cleared. * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * review(copilot): hoist wire decoder and harden redact fallback Reuse a single module-level TextDecoder for binary websocket frames so high-frequency attach-mode traffic does not allocate one per message. Strengthen the redactUrl fallback so embedded user:pass@ credentials are also masked when the WHATWG URL parser rejects the input, and pin the new behavior with a regression test that drives a malformed bearer URL through the gateway-stderr publish path. * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * review(copilot): force redact fallback path with deterministic fixture Replace the "%zz" user-info fixture, which WHATWG URL actually accepts in recent Node and silently routed the test back through the structured-URL branch, with a port-99999 fixture that the parser rejects across Node versions. Add a pre-flight `expect(() => new URL(fixture)).toThrow()` assertion so a future URL-parser change can never silently bypass `redactUrl()`'s fallback again. * review(copilot): sanitize websocket constructor failures Avoid logging raw WebSocket constructor error messages because some implementations include the full input URL, including token-bearing query strings. Log the redacted gateway or sidecar URL with the error class instead, and add regression coverage for constructor-throw paths on both attach and sidecar sockets. * review(self): restart transport on attach-mode transition Route runtime HERMES_TUI_GATEWAY_URL changes through start() so switching from spawned-gateway mode to attach mode also tears down the previously spawned Python child instead of leaving it alive. Keep the existing fast-fail behavior for pending RPCs. Also make constructor-failure logging fully generic after the redacted URL, avoiding even implementation-specific error class text in the log tail. * review(copilot): use websocket wording for attach close errors When the attached websocket closes, reject pending RPCs with an explicit websocket-closed reason instead of the spawned-process oriented `gateway exited` wording. Add coverage to ensure close code 1011 surfaces as `gateway websocket closed (1011)`. --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This commit is contained in:
brooklyn!
GitHub
parent
9680827078
commit
1997b3baf8
2 changed files with 812 additions and 29 deletions
386
ui-tui/src/__tests__/gatewayClient.test.ts
Normal file
386
ui-tui/src/__tests__/gatewayClient.test.ts
Normal file
|
|
@ -0,0 +1,386 @@
|
|||
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[] = []
|
||||
|
||||
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
|
||||
}
|
||||
|
||||
this.listeners.set(
|
||||
type,
|
||||
entries.filter(entry => entry.callback !== callback)
|
||||
)
|
||||
}
|
||||
|
||||
send(payload: string) {
|
||||
if (this.readyState !== FakeWebSocket.OPEN) {
|
||||
throw new Error('socket not open')
|
||||
}
|
||||
|
||||
this.sent.push(payload)
|
||||
}
|
||||
|
||||
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)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
describe('GatewayClient websocket attach mode', () => {
|
||||
const originalWebSocket = globalThis.WebSocket
|
||||
let originalGatewayUrl: string | undefined
|
||||
let originalSidecarUrl: string | undefined
|
||||
|
||||
beforeEach(() => {
|
||||
originalGatewayUrl = process.env.HERMES_TUI_GATEWAY_URL
|
||||
originalSidecarUrl = process.env.HERMES_TUI_SIDECAR_URL
|
||||
FakeWebSocket.reset()
|
||||
;(globalThis as { WebSocket?: unknown }).WebSocket = FakeWebSocket as unknown as typeof WebSocket
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
if (originalGatewayUrl === undefined) {
|
||||
delete process.env.HERMES_TUI_GATEWAY_URL
|
||||
} else {
|
||||
process.env.HERMES_TUI_GATEWAY_URL = originalGatewayUrl
|
||||
}
|
||||
|
||||
if (originalSidecarUrl === undefined) {
|
||||
delete process.env.HERMES_TUI_SIDECAR_URL
|
||||
} else {
|
||||
process.env.HERMES_TUI_SIDECAR_URL = originalSidecarUrl
|
||||
}
|
||||
|
||||
FakeWebSocket.reset()
|
||||
|
||||
if (originalWebSocket) {
|
||||
globalThis.WebSocket = originalWebSocket
|
||||
} else {
|
||||
delete (globalThis as { WebSocket?: unknown }).WebSocket
|
||||
}
|
||||
})
|
||||
|
||||
it('waits for websocket open and resolves RPC requests', async () => {
|
||||
process.env.HERMES_TUI_GATEWAY_URL = 'ws://gateway.test/api/ws?token=abc'
|
||||
const gw = new GatewayClient()
|
||||
|
||||
gw.start()
|
||||
const gatewaySocket = FakeWebSocket.instances[0]!
|
||||
const req = gw.request<{ ok: boolean }>('session.create', { cols: 80 })
|
||||
|
||||
expect(gatewaySocket.sent).toHaveLength(0)
|
||||
gatewaySocket.open()
|
||||
await vi.waitFor(() => expect(gatewaySocket.sent).toHaveLength(1))
|
||||
|
||||
const frame = JSON.parse(gatewaySocket.sent[0] ?? '{}') as { id: string; method: string }
|
||||
expect(frame.method).toBe('session.create')
|
||||
|
||||
gatewaySocket.message(JSON.stringify({ id: frame.id, jsonrpc: '2.0', result: { ok: true } }))
|
||||
await expect(req).resolves.toEqual({ ok: true })
|
||||
|
||||
gw.kill()
|
||||
})
|
||||
|
||||
it('mirrors event frames to sidecar websocket when configured', async () => {
|
||||
process.env.HERMES_TUI_GATEWAY_URL = 'ws://gateway.test/api/ws?token=abc'
|
||||
process.env.HERMES_TUI_SIDECAR_URL = 'ws://gateway.test/api/pub?token=abc&channel=demo'
|
||||
|
||||
const gw = new GatewayClient()
|
||||
const seen: string[] = []
|
||||
|
||||
gw.on('event', ev => seen.push(ev.type))
|
||||
gw.start()
|
||||
|
||||
const gatewaySocket = FakeWebSocket.instances[0]!
|
||||
gatewaySocket.open()
|
||||
await vi.waitFor(() => expect(FakeWebSocket.instances).toHaveLength(2))
|
||||
|
||||
const sidecarSocket = FakeWebSocket.instances[1]!
|
||||
|
||||
sidecarSocket.open()
|
||||
gw.drain()
|
||||
|
||||
const eventFrame = JSON.stringify({
|
||||
jsonrpc: '2.0',
|
||||
method: 'event',
|
||||
params: { type: 'tool.start', payload: { tool_id: 't1' } }
|
||||
})
|
||||
gatewaySocket.message(eventFrame)
|
||||
|
||||
expect(seen).toContain('tool.start')
|
||||
expect(sidecarSocket.sent).toContain(eventFrame)
|
||||
|
||||
gw.kill()
|
||||
})
|
||||
|
||||
it('emits exit when attached websocket closes', () => {
|
||||
process.env.HERMES_TUI_GATEWAY_URL = 'ws://gateway.test/api/ws?token=abc'
|
||||
const gw = new GatewayClient()
|
||||
const exits: Array<null | number> = []
|
||||
|
||||
gw.on('exit', code => exits.push(code))
|
||||
gw.start()
|
||||
|
||||
const gatewaySocket = FakeWebSocket.instances[0]!
|
||||
|
||||
gatewaySocket.open()
|
||||
gw.drain()
|
||||
gatewaySocket.close(1011)
|
||||
|
||||
expect(exits).toEqual([1011])
|
||||
})
|
||||
|
||||
it('rejects pending RPCs with websocket wording when the attached socket closes', async () => {
|
||||
process.env.HERMES_TUI_GATEWAY_URL = 'ws://gateway.test/api/ws?token=abc'
|
||||
const gw = new GatewayClient()
|
||||
|
||||
gw.start()
|
||||
const gatewaySocket = FakeWebSocket.instances[0]!
|
||||
|
||||
gatewaySocket.open()
|
||||
gw.drain()
|
||||
|
||||
const req = gw.request('session.create', {})
|
||||
await vi.waitFor(() => expect(gatewaySocket.sent.length).toBeGreaterThan(0))
|
||||
|
||||
gatewaySocket.close(1011)
|
||||
|
||||
await expect(req).rejects.toThrow(/gateway websocket closed \(1011\)/)
|
||||
})
|
||||
|
||||
it('rejects pending RPCs when kill() closes the attached websocket', async () => {
|
||||
process.env.HERMES_TUI_GATEWAY_URL = 'ws://gateway.test/api/ws?token=abc'
|
||||
const gw = new GatewayClient()
|
||||
|
||||
gw.start()
|
||||
const gatewaySocket = FakeWebSocket.instances[0]!
|
||||
|
||||
gatewaySocket.open()
|
||||
gw.drain()
|
||||
|
||||
const req = gw.request('session.create', {})
|
||||
await vi.waitFor(() => expect(gatewaySocket.sent.length).toBeGreaterThan(0))
|
||||
|
||||
gw.kill()
|
||||
|
||||
await expect(req).rejects.toThrow(/gateway closed/)
|
||||
})
|
||||
|
||||
it('reattaches when HERMES_TUI_GATEWAY_URL rotates between requests', async () => {
|
||||
process.env.HERMES_TUI_GATEWAY_URL = 'ws://gateway-old.test/api/ws?token=abc'
|
||||
const gw = new GatewayClient()
|
||||
|
||||
gw.start()
|
||||
const firstSocket = FakeWebSocket.instances[0]!
|
||||
|
||||
firstSocket.open()
|
||||
gw.drain()
|
||||
|
||||
const stale = gw.request('session.create', {})
|
||||
await vi.waitFor(() => expect(firstSocket.sent.length).toBeGreaterThan(0))
|
||||
|
||||
process.env.HERMES_TUI_GATEWAY_URL = 'ws://gateway-new.test/api/ws?token=xyz'
|
||||
const next = gw.request('session.create', {})
|
||||
|
||||
await expect(stale).rejects.toThrow(/gateway attach url changed/)
|
||||
await vi.waitFor(() => expect(FakeWebSocket.instances).toHaveLength(2))
|
||||
|
||||
const secondSocket = FakeWebSocket.instances[1]!
|
||||
expect(secondSocket.url).toContain('gateway-new.test')
|
||||
|
||||
secondSocket.open()
|
||||
await vi.waitFor(() => expect(secondSocket.sent.length).toBeGreaterThan(0))
|
||||
|
||||
const frame = JSON.parse(secondSocket.sent[0] ?? '{}') as { id: string }
|
||||
secondSocket.message(JSON.stringify({ id: frame.id, jsonrpc: '2.0', result: { ok: true } }))
|
||||
|
||||
await expect(next).resolves.toEqual({ ok: true })
|
||||
gw.kill()
|
||||
})
|
||||
|
||||
it('redacts query string secrets in attach failure logs and events', () => {
|
||||
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')
|
||||
|
||||
gw.kill()
|
||||
})
|
||||
|
||||
it('redacts attach URL secrets when the WebSocket constructor throws', () => {
|
||||
const secretUrl = 'ws://gateway.test/api/ws?token=hunter2&channel=secret'
|
||||
|
||||
process.env.HERMES_TUI_GATEWAY_URL = secretUrl
|
||||
;(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()
|
||||
|
||||
gw.start()
|
||||
gw.drain()
|
||||
|
||||
const tail = gw.getLogTail(20)
|
||||
expect(tail).not.toContain('hunter2')
|
||||
expect(tail).not.toContain('channel=secret')
|
||||
expect(tail).not.toContain(secretUrl)
|
||||
expect(tail).toContain('ws://gateway.test/api/ws?***')
|
||||
|
||||
gw.kill()
|
||||
})
|
||||
|
||||
it('redacts sidecar URL secrets when the WebSocket constructor throws', async () => {
|
||||
const sidecarUrl = 'ws://gateway.test/api/pub?token=hunter2&channel=secret'
|
||||
|
||||
process.env.HERMES_TUI_GATEWAY_URL = 'ws://gateway.test/api/ws?token=abc'
|
||||
process.env.HERMES_TUI_SIDECAR_URL = sidecarUrl
|
||||
;(globalThis as { WebSocket?: unknown }).WebSocket = class ThrowingSidecarWebSocket extends FakeWebSocket {
|
||||
constructor(url: string) {
|
||||
if (url.includes('/api/pub')) {
|
||||
throw new TypeError(`Invalid URL: ${url}`)
|
||||
}
|
||||
|
||||
super(url)
|
||||
}
|
||||
} as unknown as typeof WebSocket
|
||||
|
||||
const gw = new GatewayClient()
|
||||
|
||||
gw.start()
|
||||
const gatewaySocket = FakeWebSocket.instances[0]!
|
||||
gatewaySocket.open()
|
||||
await vi.waitFor(() => expect(gw.getLogTail(20)).toContain('[sidecar] failed to connect'))
|
||||
|
||||
const tail = gw.getLogTail(20)
|
||||
expect(tail).not.toContain('hunter2')
|
||||
expect(tail).not.toContain('channel=secret')
|
||||
expect(tail).not.toContain(sidecarUrl)
|
||||
expect(tail).toContain('ws://gateway.test/api/pub?***')
|
||||
|
||||
gw.kill()
|
||||
})
|
||||
|
||||
it('redacts user-info credentials even on URLs the WHATWG parser rejects', () => {
|
||||
// Port 99999 is outside the WHATWG URL parser's valid 0–65535
|
||||
// range and survives `.trim()`, so the fixture deterministically
|
||||
// exercises `redactUrl()`'s fallback branch across Node versions.
|
||||
// (An earlier `%zz` user-info fixture did NOT actually throw in
|
||||
// recent Node — WHATWG accepts malformed percent escapes there —
|
||||
// which silently routed the test through the structured-URL path.)
|
||||
const fixture = 'ws://alice:hunter2@gateway.test:99999/api/ws?token=secret'
|
||||
expect(() => new URL(fixture)).toThrow()
|
||||
|
||||
process.env.HERMES_TUI_GATEWAY_URL = fixture
|
||||
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('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')
|
||||
expect(tail).not.toContain('token=secret')
|
||||
|
||||
gw.kill()
|
||||
})
|
||||
})
|
||||
Loading…
Add table
Add a link
Reference in a new issue