fix(desktop): quit cleanly on SIGTERM/SIGINT/SIGHUP so termination isn't misreported as a crash (#2821)

.changeset/suppress-signal-termination-crash-prompt.md

@@ -0,0 +1,5 @@
+---
+"@inkeep/open-knowledge": patch
+---
+
+The desktop app no longer invites you to file a bug report after it was asked to quit by a termination signal (a macOS logout, `killall`, Activity Monitor's "Quit", or a parent process stopping it). These signals are an orderly request to stop, not an app crash, but the main process previously exited without running its clean-quit path — so the dirty-shutdown sentinel was left behind and the next launch misread the session as a crash. The main process now handles SIGTERM/SIGINT/SIGHUP by clearing the sentinel and quitting cleanly. Genuine app crashes still prompt exactly as before, and a crash that produced a crash dump is still detected on the next boot.

packages/desktop/src/main/index.ts

@@ -316,6 +316,7 @@ import { startFirstRunHandshake } from './share-handoff.ts';
 import { checkOutboundUrl, handleShellOpenExternal } from './shell-allowlist.ts';
 import { applyHarvestedAuthSock, harvestShellAuthSock } from './shell-env.ts';
 import { createShowGateRegistry, type ShowGateRegistry } from './show-gate.ts';
+import { installSignalCleanQuit } from './signal-clean-quit.ts';
 import { reclaimProjectSkillsOnProjectOpen, reclaimUserSkillsOnLaunch } from './skill-reclaim.ts';
 import { attachSpellcheckContextMenu } from './spellcheck-context-menu.ts';
 import { popSpellcheckMenu } from './spellcheck-menu.ts';
@@ -5436,6 +5437,17 @@ function bootPrimaryInstance(): void {
   powerMonitor.on('shutdown', () => crashDetection?.noteOsShutdown());
   powerMonitor.on('suspend', () => crashDetection?.noteSuspend());
   powerMonitor.on('resume', () => crashDetection?.noteResume());
+  // Clean-quit on catchable termination signals (SIGTERM/SIGINT/SIGHUP) so an
+  // orderly stop (logout, `killall`, Activity Monitor's "Quit") isn't misread
+  // as a crash next boot. Installed after crashDetection is wired so
+  // `markCleanQuit` is live. Full rationale + SIGKILL-race handling live in
+  // `signal-clean-quit.ts`.
+  installSignalCleanQuit({
+    process,
+    markCleanQuit: () => crashDetection?.markCleanQuit(),
+    quit: () => app.quit(),
+    logger: getLogger('signal-clean-quit'),
+  });
   app.on('child-process-gone', (_event, details) => {
     // Feed the server-exit recorder every Utility death (not just the crash
     // reasons the invitation pipeline acts on) so the bundle can distinguish a

packages/desktop/src/main/signal-clean-quit.test.ts

@@ -0,0 +1,106 @@
+/**
+ * Signal-clean-quit tests: a fake process emitter records the per-signal
+ * handlers the installer registers, and fake `markCleanQuit` / `quit` hooks
+ * record call order. No Electron, no real signals — the whole pipeline is
+ * exercised by emitting into the fake emitter.
+ */
+
+import { describe, expect, test } from 'vitest';
+import { CLEAN_QUIT_SIGNALS, installSignalCleanQuit } from './signal-clean-quit.ts';
+
+interface Rig {
+  emit(signal: NodeJS.Signals): void;
+  registered: NodeJS.Signals[];
+  calls: string[];
+  install(overrides?: { markCleanQuit?: () => void }): void;
+}
+
+function makeRig(): Rig {
+  const handlers = new Map<NodeJS.Signals, Array<() => void>>();
+  const registered: NodeJS.Signals[] = [];
+  const calls: string[] = [];
+  const rig: Rig = {
+    registered,
+    calls,
+    emit(signal) {
+      for (const handler of handlers.get(signal) ?? []) handler();
+    },
+    install(overrides) {
+      installSignalCleanQuit({
+        process: {
+          on(signal, listener) {
+            registered.push(signal);
+            const list = handlers.get(signal) ?? [];
+            list.push(listener);
+            handlers.set(signal, list);
+            return this;
+          },
+        },
+        markCleanQuit:
+          overrides?.markCleanQuit ??
+          (() => {
+            calls.push('markCleanQuit');
+          }),
+        quit: () => {
+          calls.push('quit');
+        },
+        logger: { info: () => {} },
+      });
+    },
+  };
+  return rig;
+}
+
+describe('installSignalCleanQuit', () => {
+  test('registers a handler for each of SIGTERM/SIGINT/SIGHUP by default', () => {
+    const rig = makeRig();
+    rig.install();
+    expect(rig.registered).toEqual([...CLEAN_QUIT_SIGNALS]);
+    expect([...CLEAN_QUIT_SIGNALS]).toEqual(['SIGTERM', 'SIGINT', 'SIGHUP']);
+  });
+
+  test('clears the sentinel before quitting so it is durable against a SIGKILL race', () => {
+    const rig = makeRig();
+    rig.install();
+
+    rig.emit('SIGTERM');
+
+    // markCleanQuit MUST precede quit: app.quit() teardown is async and may be
+    // cut short by a SIGKILL escalation, so the synchronous sentinel clear has
+    // to have already happened.
+    expect(rig.calls).toEqual(['markCleanQuit', 'quit']);
+  });
+
+  test('each catchable signal drives a clean quit', () => {
+    for (const signal of CLEAN_QUIT_SIGNALS) {
+      const rig = makeRig();
+      rig.install();
+      rig.emit(signal);
+      expect(rig.calls).toEqual(['markCleanQuit', 'quit']);
+    }
+  });
+
+  test('fires once — a second signal during teardown is a no-op', () => {
+    const rig = makeRig();
+    rig.install();
+
+    rig.emit('SIGTERM');
+    rig.emit('SIGINT');
+    rig.emit('SIGTERM');
+
+    expect(rig.calls).toEqual(['markCleanQuit', 'quit']);
+  });
+
+  test('a throwing markCleanQuit still lets the quit proceed', () => {
+    const rig = makeRig();
+    rig.install({
+      markCleanQuit: () => {
+        rig.calls.push('markCleanQuit');
+        throw new Error('sentinel unwritable');
+      },
+    });
+
+    expect(() => rig.emit('SIGTERM')).not.toThrow();
+    expect(rig.calls).toEqual(['markCleanQuit', 'quit']);
+  });
+});

packages/desktop/src/main/signal-clean-quit.ts

@@ -0,0 +1,95 @@
+/**
+ * Signal-driven clean-quit for the Electron main process.
+ *
+ * A `SIGTERM` / `SIGINT` / `SIGHUP` is an *orderly* request to stop — the OS
+ * asking apps to quit for a logout, a parent process, `killall`, or Activity
+ * Monitor's "Quit" (as opposed to "Force Quit", which is an uncatchable
+ * `SIGKILL`). Node's default disposition terminates the process WITHOUT running
+ * Electron's `before-quit` -> `will-quit` sequence, so the dirty-shutdown
+ * sentinel is never cleared and the NEXT boot misreports the session as a crash
+ * ("previous session ended without a clean quit"). That is the same
+ * "the environment ended the session, not the app" class the reboot /
+ * OS-shutdown / suspend suppression already covers — a termination signal is
+ * just the one that arrives in-band at teardown time rather than as an
+ * announced power marker.
+ *
+ * These handlers close that gap. On the first such signal we (1) clear the
+ * sentinel synchronously so the record is durable even if a `SIGKILL`
+ * escalation lands mid-teardown (the OS gives a signalled app a short grace
+ * before force-killing it), then (2) drive Electron's normal `app.quit()` so
+ * the full teardown (`will-quit` sentinel clear, PTY reap, logger flush,
+ * owned-server drain on the update path) still runs. The handler fires once —
+ * a second signal arriving while the quit sequence is already in flight is a
+ * no-op rather than a second `app.quit()`.
+ *
+ * A genuine main-process crash is unaffected: it never delivers a signal here,
+ * and the boot-time minidump scan remains the authoritative native-crash signal
+ * (clearing the sentinel does not touch minidumps, so a fresh dump still arms a
+ * dump-driven invitation on the next boot).
+ *
+ * This is deliberately NOT a `process.on('uncaughtException')` handler — see
+ * `process-safety-net.ts` for why one must never be added. A signal handler is
+ * a distinct disposition and does not change Electron's main-process
+ * crash-dialog semantics. It mirrors the established teardown-signal wiring in
+ * the sibling utility processes: `pty-host.ts` `installHostReaping` handles all
+ * three of these signals, and `server-entry.ts` handles SIGTERM and SIGINT.
+ *
+ * Electron-free by construction (the process emitter, clean-quit hook, quit
+ * driver, and logger are all injected) so it is unit-testable without a live
+ * app.
+ */
+
+/** Catchable teardown signals that read as an orderly stop, not an app crash. */
+export const CLEAN_QUIT_SIGNALS: readonly NodeJS.Signals[] = ['SIGTERM', 'SIGINT', 'SIGHUP'];
+
+interface SignalCleanQuitLogger {
+  info(payload: Record<string, unknown>, msg: string): void;
+}
+
+/** Minimal surface this installer needs — `process` satisfies it; tests pass an emitter. */
+interface ProcessSignalEmitter {
+  on(signal: NodeJS.Signals, listener: () => void): unknown;
+}
+
+export interface InstallSignalCleanQuitOpts {
+  /** The process to subscribe on — `process` in production, a fake emitter in tests. */
+  process: ProcessSignalEmitter;
+  /**
+   * Clear the dirty-shutdown sentinel so the next boot doesn't read this
+   * session as a crash. Runs synchronously before `quit()` so it is durable
+   * against a `SIGKILL` escalation racing the async quit teardown.
+   */
+  markCleanQuit: () => void;
+  /** Drive Electron's orderly quit (`before-quit` -> `will-quit` -> exit). */
+  quit: () => void;
+  logger: SignalCleanQuitLogger;
+}
+
+/**
+ * Register clean-quit handlers for the catchable teardown signals. Call once,
+ * during main-process boot, after crash detection is wired (so `markCleanQuit`
+ * is live). The first signal wins; later signals during teardown are no-ops.
+ */
+export function installSignalCleanQuit(opts: InstallSignalCleanQuitOpts): void {
+  let handled = false;
+  const handle = (signal: NodeJS.Signals): void => {
+    if (handled) return;
+    handled = true;
+    opts.logger.info(
+      { event: 'desktop.signal-clean-quit', signal },
+      'received termination signal — quitting cleanly',
+    );
+    // Clear the sentinel first: durable even if the OS escalates to SIGKILL
+    // before `app.quit()`'s async teardown reaches `will-quit`. `markCleanQuit`
+    // is idempotent, so the `will-quit` handler calling it again is a no-op.
+    // A throw here must not skip `quit()` — the signal is a request to stop and
+    // we honor it regardless of sentinel-clear outcome.
+    try {
+      opts.markCleanQuit();
+    } catch {}
+    opts.quit();
+  };
+  for (const signal of CLEAN_QUIT_SIGNALS) {
+    opts.process.on(signal, () => handle(signal));
+  }
+}