From dd68f324d0919cd983aa46d264a9f16b70ca34ac Mon Sep 17 00:00:00 2001 From: MoerAI Date: Mon, 11 May 2026 18:43:40 +0900 Subject: [PATCH] fix(background-agent): add OMO_DISABLE_PROCESS_CLEANUP env opt-out for global handlers (fixes #3856) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently `registerManagerForCleanup` unconditionally installs global `uncaughtException` and `unhandledRejection` listeners that call `process.exit(1)` after cleanup. For users who load the plugin but never run background-agent tasks, these handlers turn transient streaming errors (e.g. undici `UND_ERR_SOCKET` mid-stream resets from `api.githubcopilot.com`) into a full process kill — opencode dies after every flaky response. Add an `OMO_DISABLE_PROCESS_CLEANUP` env var (accepts 1/true/yes/on, case-insensitive) that skips just the error-event registration. Signal handlers (SIGINT/SIGTERM/SIGBREAK/beforeExit/exit) remain installed so graceful shutdown of any in-flight cleanup targets still runs. This is the lowest-risk near-term mitigation suggested in the issue (option #2): users opting in pay the cost of unhandled rejections themselves, but no longer lose their session to a transient socket reset. Verification: 6 new test cases cover env-var precedence (set/unset, truthy/falsy values), signal-handler preservation, and behavior under `uncaughtException`. All 20 tests in process-cleanup.test.ts pass. Typecheck clean. Manual QA confirms env-var detection works end-to-end. --- .../background-agent/process-cleanup.test.ts | 97 +++++++++++++++++++ .../background-agent/process-cleanup.ts | 29 ++++++ 2 files changed, 126 insertions(+) diff --git a/src/features/background-agent/process-cleanup.test.ts b/src/features/background-agent/process-cleanup.test.ts index bcdfa43ce..ebe142c43 100644 --- a/src/features/background-agent/process-cleanup.test.ts +++ b/src/features/background-agent/process-cleanup.test.ts @@ -235,6 +235,103 @@ describe("#given process cleanup registration", () => { }) }) + describe("#given OMO_DISABLE_PROCESS_CLEANUP env var", () => { + let originalEnvValue: string | undefined + + beforeEach(() => { + originalEnvValue = process.env.OMO_DISABLE_PROCESS_CLEANUP + }) + + afterEach(() => { + if (originalEnvValue === undefined) { + delete process.env.OMO_DISABLE_PROCESS_CLEANUP + } else { + process.env.OMO_DISABLE_PROCESS_CLEANUP = originalEnvValue + } + }) + + test("#given env var is set to 1 #when registerManagerForCleanup runs #then uncaughtException handler is NOT registered", () => { + const uncaughtExceptionListenersBefore = process.listeners("uncaughtException") + const unhandledRejectionListenersBefore = process.listeners("unhandledRejection") + process.env.OMO_DISABLE_PROCESS_CLEANUP = "1" + const manager = { shutdown: mock(() => {}) } + registeredManagers.push(manager) + + registerManagerForCleanup(manager) + + expect(process.listeners("uncaughtException")).toHaveLength(uncaughtExceptionListenersBefore.length) + expect(process.listeners("unhandledRejection")).toHaveLength(unhandledRejectionListenersBefore.length) + }) + + test("#given env var is set to true #when registerManagerForCleanup runs #then handlers are NOT registered", () => { + const uncaughtExceptionListenersBefore = process.listeners("uncaughtException") + process.env.OMO_DISABLE_PROCESS_CLEANUP = "true" + const manager = { shutdown: mock(() => {}) } + registeredManagers.push(manager) + + registerManagerForCleanup(manager) + + expect(process.listeners("uncaughtException")).toHaveLength(uncaughtExceptionListenersBefore.length) + }) + + test("#given env var is set to 0 #when registerManagerForCleanup runs #then handlers ARE registered", () => { + const uncaughtExceptionListenersBefore = process.listeners("uncaughtException") + process.env.OMO_DISABLE_PROCESS_CLEANUP = "0" + const manager = { shutdown: mock(() => {}) } + registeredManagers.push(manager) + + registerManagerForCleanup(manager) + + expect(process.listeners("uncaughtException")).toHaveLength(uncaughtExceptionListenersBefore.length + 1) + }) + + test("#given env var is unset #when registerManagerForCleanup runs #then handlers ARE registered", () => { + const uncaughtExceptionListenersBefore = process.listeners("uncaughtException") + delete process.env.OMO_DISABLE_PROCESS_CLEANUP + const manager = { shutdown: mock(() => {}) } + registeredManagers.push(manager) + + registerManagerForCleanup(manager) + + expect(process.listeners("uncaughtException")).toHaveLength(uncaughtExceptionListenersBefore.length + 1) + }) + + test("#given env var is set #when signals fire #then SIGINT/SIGTERM/beforeExit/exit handlers still run cleanup", () => { + const exitListenersBefore = process.listeners("exit") + process.env.OMO_DISABLE_PROCESS_CLEANUP = "yes" + const shutdown = mock(() => {}) + const manager = { shutdown } + registeredManagers.push(manager) + + registerManagerForCleanup(manager) + const exitListener = getNewListener("exit", exitListenersBefore) + exitListener() + + expect(shutdown).toHaveBeenCalledTimes(1) + }) + + test("#given env var is set AND process emits uncaughtException #when event fires #then manager shutdown is NOT invoked by our handler", async () => { + process.env.OMO_DISABLE_PROCESS_CLEANUP = "1" + const exitSpy = spyOn(process, "exit").mockImplementation((() => undefined) as never) + const shutdown = mock(() => {}) + const manager = { shutdown } + registeredManagers.push(manager) + + try { + registerManagerForCleanup(manager) + + // Other listeners on uncaughtException may exist (e.g. node default). + // We assert that OUR handler did not run cleanup. + process.emit("uncaughtException", new Error("boom")) + await flushMicrotasks() + + expect(shutdown).not.toHaveBeenCalled() + } finally { + exitSpy.mockRestore() + } + }) + }) + describe("#given uncaught exception and rejection cleanup", () => { test("#given manager registered AND process emits uncaughtException #when event fires #then manager shuts down before process exits", async () => { const exitSpy = spyOn(process, "exit").mockImplementation((() => undefined) as never) diff --git a/src/features/background-agent/process-cleanup.ts b/src/features/background-agent/process-cleanup.ts index d08890825..220b69879 100644 --- a/src/features/background-agent/process-cleanup.ts +++ b/src/features/background-agent/process-cleanup.ts @@ -3,6 +3,26 @@ import { log } from "../../shared" type ProcessCleanupSignal = NodeJS.Signals | "beforeExit" | "exit" type ProcessCleanupErrorEvent = "uncaughtException" | "unhandledRejection" +/** + * When set to a truthy value (1/true/yes/on), suppresses the global + * uncaughtException / unhandledRejection handlers that force-exit the host + * process. Use this when the plugin is installed but background-agent tasks + * are not actively in use, to avoid OpenCode dying on transient streaming + * errors propagated as unhandled rejections (see issue #3856). + * + * Signal handlers (SIGINT/SIGTERM/SIGBREAK/beforeExit/exit) remain registered + * because they are needed for graceful shutdown of any in-flight cleanup + * targets that were registered before the user noticed the issue. + */ +const PROCESS_CLEANUP_DISABLE_ENV = "OMO_DISABLE_PROCESS_CLEANUP" +const TRUTHY_ENV_VALUES = new Set(["1", "true", "yes", "on"]) + +function isProcessCleanupErrorHandlersDisabled(): boolean { + const raw = process.env[PROCESS_CLEANUP_DISABLE_ENV] + if (!raw) return false + return TRUTHY_ENV_VALUES.has(raw.trim().toLowerCase()) +} + /** @internal test-only seam: prevents process.exitCode from contaminating bun test runner */ let _scheduleForcedExitEnabled = true @@ -116,6 +136,15 @@ export function registerManagerForCleanup(manager: CleanupTarget): void { } registerSignal("beforeExit", false) registerSignal("exit", false) + + if (isProcessCleanupErrorHandlersDisabled()) { + log( + `[background-agent] ${PROCESS_CLEANUP_DISABLE_ENV} is set; skipping global uncaughtException/unhandledRejection handler registration. ` + + "Signal handlers (SIGINT/SIGTERM/beforeExit/exit) remain active.", + ) + return + } + cleanupErrorHandlers.set("uncaughtException", registerErrorEvent("uncaughtException", cleanupAll)) cleanupErrorHandlers.set("unhandledRejection", registerErrorEvent("unhandledRejection", cleanupAll)) }