From 91dfd2ac50cd671b64bbb9569d97f06e8fed8573 Mon Sep 17 00:00:00 2001 From: roboomp Date: Sat, 27 Jun 2026 23:32:56 +0000 Subject: [PATCH] fix(tui): unref'd desktop notifier subprocess so it cannot delay process exit Bun keeps spawned subprocesses referenced by the event loop until they exit, regardless of stdio: "ignore". A slow notify-send or a hung gdbus call (e.g. stalled session-bus activation) could therefore keep omp alive past renderer shutdown after a completion or ask notification. Call .unref() on the spawned child so the event loop can exit immediately. Test updated to assert .unref() is invoked on the spawned subprocess. Re #3685 --- packages/tui/src/desktop-notify.ts | 9 ++++++++- packages/tui/test/desktop-notify.test.ts | 11 ++++++++--- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/packages/tui/src/desktop-notify.ts b/packages/tui/src/desktop-notify.ts index f6d42b942..18a152d39 100644 --- a/packages/tui/src/desktop-notify.ts +++ b/packages/tui/src/desktop-notify.ts @@ -178,12 +178,19 @@ export function sendDesktopNotification(message: string | TerminalNotification): const notifier = resolveDesktopNotifier(); if (!notifier) return; try { - Bun.spawn({ + // `.unref()` lets the event loop exit while the notifier is still running. + // Without it, an unresponsive D-Bus activation (slow `notify-send`, hung + // `gdbus` waiting on a stalled session bus) would keep `omp` alive past + // the renderer's shutdown — a completion toast must never delay process + // exit. Ignored stdio alone does not detach the child from the parent's + // reference count. + const child = Bun.spawn({ cmd: buildDesktopNotifyCommand(notifier, message), stdin: "ignore", stdout: "ignore", stderr: "ignore", }); + child.unref(); } catch { // Best-effort: a failed spawn is silent. } diff --git a/packages/tui/test/desktop-notify.test.ts b/packages/tui/test/desktop-notify.test.ts index 690fbc361..123642c32 100644 --- a/packages/tui/test/desktop-notify.test.ts +++ b/packages/tui/test/desktop-notify.test.ts @@ -173,9 +173,10 @@ describe("sendDesktopNotification", () => { resetDesktopNotifierCache(); }); - it("fires Bun.spawn with the resolved notify-send argv and detached stdio", () => { + it("fires Bun.spawn with the resolved notify-send argv and unref's the child so it never blocks process exit", () => { vi.spyOn(utils, "$which").mockImplementation(name => (name === "notify-send" ? "/usr/bin/notify-send" : null)); - const spawn = vi.spyOn(Bun, "spawn").mockImplementation((..._args: unknown[]) => ({}) as never); + const unref = vi.fn(); + const spawn = vi.spyOn(Bun, "spawn").mockImplementation((..._args: unknown[]) => ({ unref }) as never); sendDesktopNotification({ title: "Session", body: "Complete" }); @@ -198,11 +199,15 @@ describe("sendDesktopNotification", () => { expect(opts.stdin).toBe("ignore"); expect(opts.stdout).toBe("ignore"); expect(opts.stderr).toBe("ignore"); + // `.unref()` is what actually decouples a slow notifier from process exit; + // without it Bun keeps the event loop pinned to the child even with + // stdio: "ignore". + expect(unref).toHaveBeenCalledTimes(1); }); it("is a silent no-op when no notifier binary is installed", () => { vi.spyOn(utils, "$which").mockReturnValue(null); - const spawn = vi.spyOn(Bun, "spawn").mockImplementation((..._args: unknown[]) => ({}) as never); + const spawn = vi.spyOn(Bun, "spawn").mockImplementation((..._args: unknown[]) => ({ unref: vi.fn() }) as never); sendDesktopNotification("ping");