Stop reporting clean sidecar shutdowns as desktop crashes

A post-boot sidecar exit was only treated as expected when the desktop
itself called stopSidecar (which sends SIGTERM). A SIGINT reaching the
child any other way — most often a process-group signal racing ahead of
teardown — exited the bun/Effect daemon with code 130 and fell through to
reportSidecarCrash plus the in-window crash screen, despite an empty
stderr tail. That surfaced as a spurious 'Sidecar exited unexpectedly
(code=130 signal=null)' crash report on a healthy shutdown.

Guard the classification two ways:

- Latch quit intent in before-quit (markAppQuitting), synchronously and
  before stopConnection's await, so any sidecar exit during teardown is
  treated as shutdown even if a signal beats the SIGTERM.
- Treat a clean signal-driven exit (code 130/143 with empty stderr) as a
  shutdown, not a crash: still surface the recovery screen, but don't
  report it. Exits carrying stderr, non-signal codes, or a null code
  (hard kill) still report as before.

The decision lives in a pure, electron-free helper so it is unit-tested
in isolation.

Author: RhysSullivan

apps/desktop/src/main/index.ts

@@ -23,6 +23,7 @@ import {
   startSidecar,
   stopSidecar,
   onUnexpectedSidecarExit,
+  markAppQuitting,
   SidecarPortInUseError,
   type SidecarConnection,
 } from "./sidecar";
@@ -888,6 +889,11 @@ if (ensureSingleInstance()) {
   });
 
   app.on("before-quit", async (event) => {
+    // Latch the quit intent first, synchronously: from here on, a sidecar exit
+    // is part of shutdown. A process-group SIGINT can race ahead of the SIGTERM
+    // stopConnection sends below, and without this it would be misreported as a
+    // crash (the Sentry "Sidecar exited unexpectedly (code=130)" false positive).
+    markAppQuitting();
     if (!connection) return;
     // A supervised daemon must keep serving after the app quits — don't stop it,
     // and don't block the quit on teardown we don't need to do.

apps/desktop/src/main/sidecar-exit.ts

@@ -0,0 +1,44 @@
+/**
+ * Pure classification of a sidecar's *post-boot* exit (the child died after it
+ * had already reported ready). Kept free of electron/log imports so the rule is
+ * unit-testable in isolation — `sidecar.ts` owns the I/O around it.
+ *
+ * The trap this guards against: the desktop only marks an exit "expected" when
+ * it stopped the child itself via `stopSidecar` (which sends SIGTERM). A signal
+ * that arrives any other way — most commonly a process-group SIGINT racing
+ * ahead of teardown — would otherwise be filed as a crash, painting the crash
+ * screen over (and Sentry-reporting) what was really a clean shutdown.
+ */
+
+// 128 + SIGINT(2) and 128 + SIGTERM(15): the exit codes a process reports when
+// it shuts down through its own signal handler rather than dying on the raw
+// signal. An Effect `runMain` program interrupted by SIGINT exits 130.
+const SHUTDOWN_SIGNAL_EXIT_CODES: ReadonlySet<number> = new Set([130, 143]);
+
+export type PostBootSidecarExitDecision =
+  /** We asked it to stop, or the app is quitting — say nothing, do nothing. */
+  | { readonly kind: "expected" }
+  /**
+   * A clean signal-driven exit (130/143, no stderr) that bypassed `stopSidecar`.
+   * The web UI is dead so the recovery screen still has to show, but this is a
+   * healthy shutdown and must not be reported as a crash.
+   */
+  | { readonly kind: "recover" }
+  /** A genuine fault: surface recovery UI *and* report upstream. */
+  | { readonly kind: "crash" };
+
+export const classifyPostBootSidecarExit = (input: {
+  /** The child was passed to `stopSidecar` (we initiated this stop). */
+  readonly expected: boolean;
+  /** `before-quit` has fired — the whole app is tearing down. */
+  readonly appQuitting: boolean;
+  readonly code: number | null;
+  readonly stderrTail: string;
+}): PostBootSidecarExitDecision => {
+  if (input.expected || input.appQuitting) return { kind: "expected" };
+  const cleanShutdownSignal =
+    input.code !== null &&
+    SHUTDOWN_SIGNAL_EXIT_CODES.has(input.code) &&
+    input.stderrTail.trim().length === 0;
+  return cleanShutdownSignal ? { kind: "recover" } : { kind: "crash" };
+};

apps/desktop/src/main/sidecar.ts

@@ -24,6 +24,7 @@ import {
 import { loadOrMintLocalAuthToken } from "./local-auth";
 import { getServerSettings } from "./settings";
 import { reportSidecarCrash, sidecarCrashReportingEnv } from "./diagnostics";
+import { classifyPostBootSidecarExit } from "./sidecar-exit";
 import { SERVER_SETTINGS_USERNAME, type DesktopServerSettings } from "../shared/server-settings";
 
 // Sidecar output is echoed to the terminal (visible when Electron is run
@@ -39,6 +40,15 @@ const STDERR_TAIL_LIMIT = 8 * 1024;
 // their exits are expected and must not be reported as crashes.
 const expectedExits = new WeakSet<ChildProcess>();
 
+// Flipped by main/index.ts the moment the app begins quitting. A sidecar exit
+// during teardown is part of shutdown even when a process-group signal (SIGINT)
+// races ahead of stopSidecar's SIGTERM — so once this is set, no post-boot exit
+// is reported as a crash. One-way latch: the process is on its way out.
+let appQuitting = false;
+export const markAppQuitting = (): void => {
+  appQuitting = true;
+};
+
 // Main/index.ts subscribes to swap the dead web UI for the in-window crash
 // screen. A callback (not an import) keeps this module free of window
 // concerns.
@@ -409,13 +419,29 @@ export async function startSidecar(options: StartOptions = {}): Promise<SidecarC
 
     const onExit = (code: number | null, signal: NodeJS.Signals | null) => {
       if (resolved) {
-        // Post-boot exit: expected when we stopped it ourselves (quit,
-        // restart, update); anything else is a sidecar crash under a live
-        // window — log it and report upstream with the stderr tail.
-        if (expectedExits.has(child)) {
+        // Post-boot exit. Expected when we stopped it ourselves (quit, restart,
+        // update) or the app is mid-quit; a clean signal-driven exit that raced
+        // past stopSidecar still needs the recovery screen but isn't a crash;
+        // anything else is a real fault — report it with the stderr tail.
+        const decision = classifyPostBootSidecarExit({
+          expected: expectedExits.has(child),
+          appQuitting,
+          code,
+          stderrTail: stderrBuffer,
+        });
+        if (decision.kind === "expected") {
           sidecarLog.info(`exited (code=${code} signal=${signal})`);
           return;
         }
+        if (decision.kind === "recover") {
+          // A SIGINT/SIGTERM-driven shutdown (code 130/143, no stderr) that
+          // bypassed stopSidecar — e.g. a process-group signal during teardown.
+          // The web UI is dead, so surface the recovery screen, but don't file
+          // a healthy shutdown as a crash.
+          sidecarLog.warn(`exited via shutdown signal (code=${code} signal=${signal})`);
+          unexpectedExitListener?.();
+          return;
+        }
         const message = `Sidecar exited unexpectedly (code=${code} signal=${signal})`;
         sidecarLog.error(message);
         reportSidecarCrash(message, stderrBuffer);

tests/desktop-sidecar-exit.test.ts

@@ -0,0 +1,103 @@
+import { describe, expect, it } from "@effect/vitest";
+import { classifyPostBootSidecarExit } from "../apps/desktop/src/main/sidecar-exit";
+
+// Guards the desktop's post-boot sidecar exit classification — the source of the
+// Sentry "Sidecar exited unexpectedly (code=130 signal=null)" false positive,
+// where a clean SIGINT-driven shutdown that raced past stopSidecar was filed as
+// a crash and painted the crash screen over a healthy teardown.
+describe("classifyPostBootSidecarExit", () => {
+  it("treats a stop we initiated (stopSidecar) as expected", () => {
+    expect(
+      classifyPostBootSidecarExit({
+        expected: true,
+        appQuitting: false,
+        code: 143,
+        stderrTail: "",
+      }),
+    ).toEqual({ kind: "expected" });
+  });
+
+  it("treats any exit during app quit as expected, even a racing SIGINT", () => {
+    // The reported event: code=130 (SIGINT), empty stderr, child not yet handed
+    // to stopSidecar — but the app is quitting, so this is shutdown, not a crash.
+    expect(
+      classifyPostBootSidecarExit({
+        expected: false,
+        appQuitting: true,
+        code: 130,
+        stderrTail: "",
+      }),
+    ).toEqual({ kind: "expected" });
+  });
+
+  it("does NOT report a clean SIGINT exit (130, no stderr) as a crash", () => {
+    // A signal that bypassed stopSidecar while the app is live: recover the UI,
+    // but never report a healthy shutdown to Sentry.
+    expect(
+      classifyPostBootSidecarExit({
+        expected: false,
+        appQuitting: false,
+        code: 130,
+        stderrTail: "",
+      }),
+    ).toEqual({ kind: "recover" });
+  });
+
+  it("does NOT report a clean SIGTERM exit (143, no stderr) as a crash", () => {
+    expect(
+      classifyPostBootSidecarExit({
+        expected: false,
+        appQuitting: false,
+        code: 143,
+        stderrTail: "",
+      }),
+    ).toEqual({ kind: "recover" });
+  });
+
+  it("still reports a real fault: signal exit code carrying stderr", () => {
+    // 130 with a stack trace is an interrupt that caught the daemon mid-error —
+    // that is worth reporting, so it must not be swallowed as a clean shutdown.
+    expect(
+      classifyPostBootSidecarExit({
+        expected: false,
+        appQuitting: false,
+        code: 130,
+        stderrTail: "Error: boom\n  at thing (x.ts:1:1)",
+      }),
+    ).toEqual({ kind: "crash" });
+  });
+
+  it("still reports a non-signal crash (e.g. exit 1)", () => {
+    expect(
+      classifyPostBootSidecarExit({
+        expected: false,
+        appQuitting: false,
+        code: 1,
+        stderrTail: "",
+      }),
+    ).toEqual({ kind: "crash" });
+  });
+
+  it("reports a hard kill (null code) as a crash", () => {
+    // SIGKILL / abnormal death surfaces as code=null — not a clean shutdown.
+    expect(
+      classifyPostBootSidecarExit({
+        expected: false,
+        appQuitting: false,
+        code: null,
+        stderrTail: "",
+      }),
+    ).toEqual({ kind: "crash" });
+  });
+
+  it("ignores whitespace-only stderr when judging a clean shutdown", () => {
+    expect(
+      classifyPostBootSidecarExit({
+        expected: false,
+        appQuitting: false,
+        code: 130,
+        stderrTail: "  \n\t ",
+      }),
+    ).toEqual({ kind: "recover" });
+  });
+});