From ec9997b7a6d47ef0c7d7c09d6b9172c5c597a954 Mon Sep 17 00:00:00 2001 From: SpencerJung Date: Fri, 22 May 2026 17:12:14 +0900 Subject: [PATCH] fix(background-agent): keep cleanup error listener active Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- .../background-agent/process-cleanup.test.ts | 28 +++++++++++++++++++ .../background-agent/process-cleanup.ts | 16 +++++++---- 2 files changed, 39 insertions(+), 5 deletions(-) diff --git a/src/features/background-agent/process-cleanup.test.ts b/src/features/background-agent/process-cleanup.test.ts index f8d09be0c..f67456274 100644 --- a/src/features/background-agent/process-cleanup.test.ts +++ b/src/features/background-agent/process-cleanup.test.ts @@ -435,6 +435,34 @@ describe("#given process cleanup registration", () => { } }) + test("#given repeated uncaughtException events #when manager is registered #then listener stays installed and host is not forced to exit", async () => { + const uncaughtExceptionListenersBefore = process.listeners("uncaughtException") + const exitSpy = spyOn(process, "exit").mockImplementation((() => undefined) as never) + const shutdown = mock(() => {}) + const manager = { shutdown } + registeredManagers.push(manager) + __enableScheduledForcedExitForTesting() + + try { + registerManagerForCleanup(manager) + + process.emit("uncaughtException", new Error("first transient MCP failure")) + process.emit("uncaughtException", new Error("second transient MCP failure")) + await flushMicrotasks() + + expect(process.listeners("uncaughtException")).toHaveLength( + uncaughtExceptionListenersBefore.length + 1, + ) + expect(shutdown).not.toHaveBeenCalled() + expect(exitSpy).not.toHaveBeenCalled() + expect(process.exitCode).toBe(0) + } finally { + exitSpy.mockRestore() + __disableScheduledForcedExitForTesting() + process.exitCode = 0 + } + }) + test("#given a manager registered AND process emits 'exit' #then cleanup still runs (signal path remains the real shutdown gate)", () => { const exitListenersBefore = process.listeners("exit") const shutdown = mock(() => {}) diff --git a/src/features/background-agent/process-cleanup.ts b/src/features/background-agent/process-cleanup.ts index 3e2fd0c31..a2af8a4b5 100644 --- a/src/features/background-agent/process-cleanup.ts +++ b/src/features/background-agent/process-cleanup.ts @@ -115,16 +115,22 @@ function registerErrorEvent( // regardless of cause, so cleanup is not skipped when the host genuinely // dies. // - // We still detach the listener before logging so a re-emit from inside - // `log()` (e.g. EPIPE while writing to a broken pipe during shutdown) - // cannot recurse and produce the 100+ GB log explosion that #3856-era - // regressions caused. + // Keep the listener installed after logging. Desktop sidecars can emit more + // than one transient error during MCP startup or provider reconnects; if we + // detach after the first event, the second uncaught exception falls through + // to Node's default process termination path and reproduces the exit-code-1 + // crash from #4128. A local re-entry guard still prevents `log()` failures + // (for example EPIPE while writing during shutdown) from recursing into the + // 100+ GB log explosion that #3856-era regressions caused. + let logging = false const listener = (error: unknown) => { - process.off(signal, listener) + if (logging) return + logging = true log( `[background-agent] ${signal} observed; keeping host alive and skipping cleanup (signal handlers run on real shutdown)`, describeProcessCleanupError(error), ) + logging = false } process.on(signal, listener) return listener