fix(tui): preserve sessions across gateway reconnects
An attached (dashboard-embedded) Ink TUI whose WebSocket dropped never recovered even though the backend stayed alive, and a spawned gateway that crashed resumed the wrong session. - GatewayClient no longer resets `subscribed` on each transport generation: the renderer drain()s once on mount, so every post-reconnect event (gateway.ready included) stayed buffered forever. - clearReconnect() keeps the attempt counter; it is reset on gateway.ready (and kill()), so backoff actually grows across failed reconnects. - useMainApp's exit handler no longer calls start() for an attached socket closure — GatewayClient owns that reconnect; it only respawns a still-owned child, and plans the resume with the durable stored_session_id (what session.resume takes) instead of the process-local runtime sid. - session.create's stored_session_id is carried into ui state / the active session file so recovery and the exit epilogue target the durable id. - The recovery target is cleared only after resumeById resolves into a live sid, so a second disconnect during setup/history loading keeps it. - Stale-socket identity guards on 'open'/'message'. Salvaged from #111599 (@gustavosmendes) with trims: kept the client-side backoff reconnect for spawned children too (the "keeps trying to reconnect in the background" copy depends on it), kept the RPC-triggered reconnect during backoff, kept the spawn-mode "reply in progress was lost" wording (true for a dead child) and added attached-mode copy in userMessages.ts, dropped the config_warning contract regen and the SessionCreateResponse re-export (local type gains stored_session_id instead). Fixes #111594
This commit is contained in:
@@ -1074,12 +1074,17 @@ describe('createGatewayEventHandler', () => {
|
||||
const appended: Msg[] = []
|
||||
const newSession = vi.fn()
|
||||
const resumeById = vi.fn()
|
||||
const resumed = Promise.withResolvers<void>()
|
||||
const ctx = buildCtx(appended)
|
||||
|
||||
ctx.session.newSession = newSession
|
||||
// Mimic resumeById's synchronous status write so the test proves the
|
||||
// "recovering session…" label is applied *after* (and survives) it.
|
||||
ctx.session.resumeById = resumeById.mockImplementation(() => patchUiState({ status: 'resuming…' }))
|
||||
ctx.session.resumeById = resumeById.mockImplementation(() => {
|
||||
patchUiState({ status: 'resuming…' })
|
||||
|
||||
return resumed.promise.then(() => patchUiState({ sid: 'sess-recovered', status: 'ready' }))
|
||||
})
|
||||
ctx.session.STARTUP_RESUME_ID = ''
|
||||
ctx.session.recoverSidRef = ref<null | string>('sess-crashed')
|
||||
|
||||
@@ -1089,10 +1094,12 @@ describe('createGatewayEventHandler', () => {
|
||||
|
||||
await vi.waitFor(() => expect(resumeById).toHaveBeenCalledWith('sess-crashed'))
|
||||
expect(newSession).not.toHaveBeenCalled()
|
||||
// One-shot: the ref is consumed so a later ordinary restart forges/resumes
|
||||
// per config instead of re-resuming the recovered session.
|
||||
expect(ctx.session.recoverSidRef.current).toBeNull()
|
||||
expect(ctx.session.recoverSidRef.current).toBe('sess-crashed')
|
||||
expect(getUiState().status).toBe('recovering session…')
|
||||
|
||||
resumed.resolve()
|
||||
await vi.waitFor(() => expect(ctx.session.recoverSidRef.current).toBeNull())
|
||||
expect(getUiState().sid).toBe('sess-recovered')
|
||||
})
|
||||
|
||||
it('on gateway.ready with auto_resume on and a recent session, resumes it', async () => {
|
||||
|
||||
@@ -715,15 +715,16 @@ export function createGatewayEventHandler(ctx: GatewayEventHandlerContext): (ev:
|
||||
})
|
||||
.catch((e: unknown) => turnController.pushActivity(`command catalog unavailable: ${rpcErrorMessage(e)}`, 'info'))
|
||||
|
||||
// Crash recovery: a respawn triggered by an unexpected gateway death
|
||||
// resumes the session that was live, not a brand-new one. One-shot — the
|
||||
// ref is cleared so an ordinary later restart still forges/resumes per
|
||||
// config. No startup prompt here (this is mid-session, not a cold boot).
|
||||
// Keep the recovery target until resume succeeds, including across a second
|
||||
// disconnect during setup or history loading. Recovery never resends the prompt.
|
||||
const recoverSid = recoverSidRef?.current
|
||||
|
||||
if (recoverSidRef && recoverSid) {
|
||||
recoverSidRef.current = null
|
||||
resumeById(recoverSid)
|
||||
void resumeById(recoverSid).then(() => {
|
||||
if (getUiState().sid && recoverSidRef.current === recoverSid) {
|
||||
recoverSidRef.current = null
|
||||
}
|
||||
})
|
||||
// After resumeById: it synchronously sets status to 'resuming…' on entry,
|
||||
// so override it here to keep the distinct "recovering" label visible for
|
||||
// the duration of the resume RPC (which later flips status to 'ready').
|
||||
|
||||
@@ -483,12 +483,10 @@ export interface GatewayEventHandlerContext {
|
||||
STARTUP_RESUME_ID: string
|
||||
colsRef: MutableRefObject<number>
|
||||
newSession: (msg?: string, title?: string) => void
|
||||
// Set by useMainApp's exit handler to the session that was live when the
|
||||
// gateway died unexpectedly; consumed once by the next `gateway.ready` so a
|
||||
// respawn resumes that session instead of forging a fresh one.
|
||||
// Session carried across a transport loss or child exit, cleared after resume.
|
||||
recoverSidRef?: MutableRefObject<null | string>
|
||||
resetSession: () => void
|
||||
resumeById: (id: string) => void
|
||||
resumeById: (id: string) => Promise<void>
|
||||
setCatalog: StateSetter<null | SlashCatalog>
|
||||
}
|
||||
submission: {
|
||||
|
||||
@@ -72,6 +72,8 @@ import {
|
||||
BACKEND_RESTARTING,
|
||||
BACKEND_RESTARTING_ACTIVITY,
|
||||
backendGaveUp,
|
||||
CONNECTION_LOST,
|
||||
CONNECTION_LOST_ACTIVITY,
|
||||
lastStderrLine
|
||||
} from './userMessages.js'
|
||||
import { useSessionLifecycle } from './useSessionLifecycle.js'
|
||||
@@ -962,16 +964,31 @@ export function useMainApp(gw: GatewayClient) {
|
||||
|
||||
const exitHandler = (code: null | number) => {
|
||||
turnController.reset()
|
||||
const state = getUiState()
|
||||
const storedSid = state.info?.stored_session_id || null
|
||||
|
||||
// Attached socket closed: the backend (and any live turn) is still there —
|
||||
// GatewayClient owns the backoff reconnect, and the next gateway.ready
|
||||
// resumes the durable session id. Calling start() here would race that
|
||||
// reconnect and reset its backoff.
|
||||
if (gw.attached) {
|
||||
recoverSidRef.current = storedSid ?? recoverSidRef.current
|
||||
patchUiState({ busy: false, compacting: false, sid: null, status: 'reconnecting…' })
|
||||
|
||||
if (state.sid) {
|
||||
turnController.pushActivity(CONNECTION_LOST_ACTIVITY, 'warn')
|
||||
sys(CONNECTION_LOST)
|
||||
}
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
// A still-owned child dying while the TUI is alive is an *unexpected*
|
||||
// death — a user /quit exits Node before this fires, and a replaced child
|
||||
// is identity-skipped in GatewayClient. Rather than stranding a long
|
||||
// session (the user's complaint), respawn the gateway and resume the
|
||||
// persisted session via the next gateway.ready, so a single crash / OOM /
|
||||
// signal doesn't lose their work. planGatewayRecovery bounds the attempts
|
||||
// so a gateway that crash-loops on startup can't spawn-storm, and falls
|
||||
// back to recoverSidRef when sid was already cleared by a prior exit.
|
||||
const plan = planGatewayRecovery(getUiState().sid, recoverSidRef.current, recoveryAtRef.current, Date.now())
|
||||
// death — respawn the gateway and resume the persisted session via the
|
||||
// next gateway.ready. session.resume takes the durable stored id, not the
|
||||
// process-local runtime sid. planGatewayRecovery bounds the attempts so a
|
||||
// crash-looping gateway can't spawn-storm.
|
||||
const plan = planGatewayRecovery(storedSid, recoverSidRef.current, recoveryAtRef.current, Date.now())
|
||||
|
||||
// Clear sid immediately: while the gateway is down, sid-guarded effects
|
||||
// (session.active_list poll, queue drain) would otherwise fire RPCs at a
|
||||
|
||||
@@ -208,13 +208,16 @@ export function useSessionLifecycle(opts: UseSessionLifecycleOptions) {
|
||||
return null
|
||||
}
|
||||
|
||||
const info = r.info ?? null
|
||||
// The durable id lives on the create result; the lazy-create `info` does
|
||||
// not carry it, and session.resume / the exit epilogue need the stored id.
|
||||
const storedSid = r.stored_session_id || r.session_id
|
||||
const info = r.info ? { ...r.info, stored_session_id: storedSid } : null
|
||||
const requestedTitle = title?.trim() ?? ''
|
||||
|
||||
resetSession()
|
||||
setSessionStartedAt(Date.now())
|
||||
|
||||
writeActiveSessionFile(r.session_id)
|
||||
writeActiveSessionFile(storedSid)
|
||||
patchUiState({
|
||||
info,
|
||||
sid: r.session_id,
|
||||
@@ -331,7 +334,7 @@ export function useSessionLifecycle(opts: UseSessionLifecycleOptions) {
|
||||
patchOverlayState({ sessions: false })
|
||||
patchUiState({ status: 'resuming…' })
|
||||
|
||||
rpc<SetupStatusResponse>('setup.status', {}).then(setup => {
|
||||
return rpc<SetupStatusResponse>('setup.status', {}).then(setup => {
|
||||
if (setup?.provider_configured === false) {
|
||||
panel(SETUP_REQUIRED_TITLE, buildSetupRequiredSections())
|
||||
patchUiState({ status: 'setup required' })
|
||||
@@ -341,7 +344,7 @@ export function useSessionLifecycle(opts: UseSessionLifecycleOptions) {
|
||||
|
||||
const previousSid = getUiState().sid
|
||||
|
||||
gw.request<SessionResumeResult>('session.resume', { cols: colsRef.current, session_id: id })
|
||||
return gw.request<SessionResumeResult>('session.resume', { cols: colsRef.current, session_id: id })
|
||||
.then(raw => {
|
||||
const r = asRpcResult<SessionResumeResult>(raw)
|
||||
|
||||
@@ -351,7 +354,10 @@ export function useSessionLifecycle(opts: UseSessionLifecycleOptions) {
|
||||
return patchUiState({ status: 'ready' })
|
||||
}
|
||||
|
||||
const info = r.info ?? null
|
||||
const info = r.info
|
||||
? { ...r.info, stored_session_id: r.info.stored_session_id || r.stored_session_id || r.resumed || id }
|
||||
: null
|
||||
|
||||
const running = Boolean(r.running || r.status === 'working' || r.status === 'waiting')
|
||||
|
||||
resetSession()
|
||||
@@ -360,7 +366,7 @@ export function useSessionLifecycle(opts: UseSessionLifecycleOptions) {
|
||||
const resumed = [...toTranscriptMessages(r.messages), ...liveSessionInflightMessages(r.inflight)]
|
||||
|
||||
setHistoryItems(info ? [introMsg(info), ...resumed] : resumed)
|
||||
writeActiveSessionFile(r.resumed ?? r.session_id)
|
||||
writeActiveSessionFile(info?.stored_session_id || r.resumed || id)
|
||||
patchUiState({
|
||||
busy: running,
|
||||
info,
|
||||
|
||||
@@ -39,6 +39,12 @@ export const BACKEND_RESTARTING =
|
||||
|
||||
export const BACKEND_RESTARTING_ACTIVITY = 'Hermes stopped unexpectedly · restarting…'
|
||||
|
||||
// Attached (dashboard / embedded) mode: only the socket dropped; Hermes and any
|
||||
// reply in progress are still alive on the backend and come back on reconnect.
|
||||
export const CONNECTION_LOST = 'Connection to Hermes lost — reconnecting and reopening your chat…'
|
||||
|
||||
export const CONNECTION_LOST_ACTIVITY = 'connection lost · reconnecting…'
|
||||
|
||||
export const backendGaveUp = (code: null | number, lastLine?: string): string => {
|
||||
const exit = code === null ? '' : ` (exit code ${code})`
|
||||
const detail = detailLine(lastLine)
|
||||
|
||||
@@ -170,9 +170,15 @@ export class GatewayClient extends EventEmitter {
|
||||
})
|
||||
}
|
||||
|
||||
get attached(): boolean {
|
||||
return this.attachUrl !== null
|
||||
}
|
||||
|
||||
private publish(ev: AnyGatewayEvent) {
|
||||
if (ev.type === 'gateway.ready') {
|
||||
this.ready = true
|
||||
this.clearReconnect()
|
||||
this.reconnectAttempts = 0
|
||||
|
||||
if (this.readyTimer) {
|
||||
clearTimeout(this.readyTimer)
|
||||
@@ -276,8 +282,6 @@ export class GatewayClient extends EventEmitter {
|
||||
clearTimeout(this.reconnectTimer)
|
||||
this.reconnectTimer = null
|
||||
}
|
||||
|
||||
this.reconnectAttempts = 0
|
||||
}
|
||||
|
||||
private resetStartupState() {
|
||||
@@ -288,7 +292,9 @@ export class GatewayClient extends EventEmitter {
|
||||
// attached to a discarded child / socket.
|
||||
this.channel.detach(new Error('gateway restarting'))
|
||||
this.ready = false
|
||||
this.subscribed = false
|
||||
// `subscribed` is NOT reset here: the renderer drain()s once on mount, so a
|
||||
// reset would strand every post-reconnect event (gateway.ready included) in
|
||||
// the buffer forever (#111594).
|
||||
// Invalidate any pending deferred drain() flush from a prior transport so
|
||||
// its queued microtask becomes a no-op (it captured the old generation).
|
||||
this.drainGeneration += 1
|
||||
@@ -325,6 +331,7 @@ export class GatewayClient extends EventEmitter {
|
||||
|
||||
private handleTransportExit(code: null | number, reason?: string) {
|
||||
this.clearReadyTimer()
|
||||
this.ready = false
|
||||
this.closeSidecarSocket()
|
||||
this.lifecycle(`[lifecycle] transport exit code=${code ?? 'null'} reason=${reason ?? 'none'}`)
|
||||
this.channel.detach(new Error(reason || `gateway exited${code === null ? '' : ` (${code})`}`))
|
||||
@@ -332,9 +339,10 @@ export class GatewayClient extends EventEmitter {
|
||||
// Self-heal: a dropped transport (real close OR silent drop caught by the
|
||||
// heartbeat) should reconnect instead of stranding the UI on a dead socket
|
||||
// (issue #32997). Intentional shutdown sets `disposed` and skips this.
|
||||
// Schedule before the synchronous 'exit' emission: useMainApp's existing
|
||||
// Schedule before the synchronous 'exit' emission: in spawn mode useMainApp's
|
||||
// recovery subscriber may call start() immediately, and start() cancels this
|
||||
// timer so there is only one recovery owner.
|
||||
// timer so there is only one recovery owner; the attempt counter survives
|
||||
// until gateway.ready so backoff keeps growing across failed restarts.
|
||||
this.scheduleReconnect()
|
||||
|
||||
if (this.subscribed) {
|
||||
@@ -534,12 +542,15 @@ export class GatewayClient extends EventEmitter {
|
||||
ws.addEventListener(
|
||||
'open',
|
||||
() => {
|
||||
if (this.ws !== ws) {
|
||||
return
|
||||
}
|
||||
|
||||
if (!settled) {
|
||||
settled = true
|
||||
resolve()
|
||||
}
|
||||
|
||||
this.clearReconnect()
|
||||
this.connectSidecarMirror()
|
||||
},
|
||||
{ once: true }
|
||||
@@ -577,7 +588,11 @@ export class GatewayClient extends EventEmitter {
|
||||
connectPromise.catch(() => {})
|
||||
this.wsConnectPromise = connectPromise
|
||||
|
||||
ws.addEventListener('message', ev => this.handleWebSocketFrame(ev.data))
|
||||
ws.addEventListener('message', ev => {
|
||||
if (this.ws === ws) {
|
||||
this.handleWebSocketFrame(ev.data)
|
||||
}
|
||||
})
|
||||
ws.addEventListener('close', ev => {
|
||||
// Skip close events from sockets that have already been
|
||||
// replaced — start() / closeGatewaySocket() can swap `this.ws`
|
||||
@@ -618,7 +633,6 @@ export class GatewayClient extends EventEmitter {
|
||||
this.attachUrl = attachUrl
|
||||
this.sidecarUrl = sidecarUrl
|
||||
this.resetStartupState()
|
||||
this.clearReconnect()
|
||||
|
||||
if (this.proc && !this.proc.killed && this.proc.exitCode === null) {
|
||||
this.lifecycle(`[lifecycle] replacing live gateway child ${describeChild(this.proc)}`)
|
||||
@@ -767,6 +781,7 @@ export class GatewayClient extends EventEmitter {
|
||||
kill(reason = 'requested') {
|
||||
this.disposed = true
|
||||
this.clearReconnect()
|
||||
this.reconnectAttempts = 0
|
||||
const proc = this.proc
|
||||
const killed = proc?.kill()
|
||||
|
||||
|
||||
@@ -179,6 +179,9 @@ export interface SystemBatteryResponse {
|
||||
export interface SessionCreateResponse {
|
||||
info?: SessionInfo & { config_warning?: string; credential_warning?: string }
|
||||
session_id: string
|
||||
// Durable id (state.db row) — what session.resume takes; `session_id` is the
|
||||
// process-local runtime handle.
|
||||
stored_session_id?: string
|
||||
}
|
||||
|
||||
export type LiveSessionStatus = 'idle' | 'starting' | 'waiting' | 'working'
|
||||
|
||||
Reference in New Issue
Block a user