fix(cli): persist supervisor give-up crash diagnostics to disk (#13538) (#13908)

Root cause: ServerSupervisor.handleExit()'s give-up branch only printed the
crash summary/log via console.error(). In --tray/--tray-worker mode this
process is launched detached with stdio:"ignore" on Windows and Linux
(bin/cli/tray/detachedTray.mjs buildTrayLaunch()), so that output is
discarded by the OS and nothing ever explained why the tray + gateway
disappeared together.

Fix (Part A only, see plan-file for Part B — a restart-on-failure policy,
out of scope here): best-effort append the crash summary + buffered log to
<DATA_DIR>/server/crash.log, wrapped in try/catch so the write can never
block shutdown. Surface the same file from `omniroute doctor`.

Regression test: tests/unit/issue-13538-tray-crash-diagnostics-lost.test.ts

bin/cli/commands/doctor.mjs

@@ -10,6 +10,7 @@ import { getCliToken, CLI_TOKEN_HEADER } from "../utils/cliToken.mjs";
 import { printHeading } from "../io.mjs";
 import { t } from "../i18n.mjs";
 import { readDatabaseHealth, readEncryptedCredentialSamples } from "../sqlite.mjs";
+import { getCrashLogPath } from "../runtime/processSupervisor.mjs";
 
 const STATIC_SALT = "omniroute-field-encryption-v1";
 const KEY_LENGTH = 32;
@@ -380,6 +381,33 @@ function checkMemory() {
   });
 }
 
+// #13538: surfaces the supervisor's give-up crash record (persisted by
+// ServerSupervisor.persistCrashLog(), bin/cli/runtime/processSupervisor.mjs)
+// so a user whose `--tray` worker died silently (detached, stdio:"ignore")
+// has something concrete `doctor` can point at without needing `--log`.
+function checkCrashLog() {
+  const crashLogPath = getCrashLogPath();
+  if (!fs.existsSync(crashLogPath)) {
+    return ok("Crash log", "No supervisor crash record found", { crashLogPath });
+  }
+
+  try {
+    const stat = fs.statSync(crashLogPath);
+    const contents = fs.readFileSync(crashLogPath, "utf8");
+    const lastEntry = contents.split("\n").filter(Boolean).slice(-6).join("\n");
+    return warn(
+      "Crash log",
+      `Supervisor recorded a give-up crash at ${crashLogPath} (last modified ${stat.mtime.toISOString()})`,
+      { crashLogPath, modifiedAt: stat.mtime.toISOString(), tail: lastEntry }
+    );
+  } catch (error) {
+    return warn("Crash log", `Crash record exists at ${crashLogPath} but could not be read`, {
+      crashLogPath,
+      error: error instanceof Error ? error.message : String(error),
+    });
+  }
+}
+
 async function fetchWithTimeout(url, options = {}) {
   const controller = new AbortController();
   const timeout = setTimeout(() => controller.abort(), CHECK_TIMEOUT_MS);
@@ -579,6 +607,7 @@ export async function collectDoctorChecks(context = {}, options = {}) {
   checks.push(await checkNodeRuntime(rootDir));
   checks.push(await checkNativeBinary(rootDir));
   checks.push(checkMemory());
+  checks.push(checkCrashLog());
 
   if (!options.skipLiveness) {
     checks.push(await checkServerLiveness(options));

bin/cli/runtime/processSupervisor.mjs

@@ -1,7 +1,9 @@
 import { spawn } from "node:child_process";
+import { mkdirSync, appendFileSync } from "node:fs";
 import { dirname, join } from "node:path";
 import { fileURLToPath } from "node:url";
 import { writePidFile, cleanupPidFile, killAllSubprocesses, isPidRunning } from "../utils/pid.mjs";
+import { resolveDataDir } from "../data-dir.mjs";
 import {
   RESTART_RESET_MS,
   DEFAULT_MAX_RESTARTS,
@@ -19,6 +21,13 @@ import {
 
 const CRASH_LOG_LINES = 50;
 
+// #13538: shared path resolver so `omniroute doctor` (bin/cli/commands/doctor.mjs)
+// can surface the same file persistCrashLog() writes, without duplicating the
+// `<DATA_DIR>/server/...` convention from bin/cli/utils/pid.mjs.
+export function getCrashLogPath() {
+  return join(resolveDataDir(), "server", "crash.log");
+}
+
 const PACKAGE_ROOT = join(dirname(fileURLToPath(import.meta.url)), "..", "..", "..");
 // Bun needs the Node-compat polyfill preloaded (#9761). The file ships at the
 // package root via package.json "files" (see scripts/build/pack-artifact-policy.ts)
@@ -164,7 +173,8 @@ export class ServerSupervisor {
     if (aliveMs >= RESTART_RESET_MS) this.restartCount = 0;
 
     if (this.restartCount >= this.maxRestarts) {
-      console.error(`\n⚠ Server crashed ${this.maxRestarts} times in <30s.`);
+      const summary = `Server crashed ${this.maxRestarts} times in <30s.`;
+      console.error(`\n⚠ ${summary}`);
       if (this.onCrashCallback) {
         const action = this.onCrashCallback(this.crashLog);
         if (action === "disable-mitm-and-retry") {
@@ -175,6 +185,14 @@ export class ServerSupervisor {
         }
       }
       this.dumpCrashLog();
+      // #13538: the give-up path used to only console.error() this diagnostic.
+      // In `--tray`/`--tray-worker` mode this process is launched detached with
+      // stdio:"ignore" (bin/cli/tray/detachedTray.mjs buildTrayLaunch()), so
+      // that console output is discarded by the OS and nothing ever explains
+      // why the tray + gateway disappeared together. Best-effort persist a
+      // durable record next to the existing per-service PID file convention
+      // (bin/cli/utils/pid.mjs) so it survives the process exit below.
+      this.persistCrashLog(summary);
       process.exit(exitCode ?? 1);
       return;
     }
@@ -206,6 +224,23 @@ export class ServerSupervisor {
     console.error("--- End crash log ---\n");
   }
 
+  // #13538: best-effort append a durable crash record to
+  // `<DATA_DIR>/server/crash.log`, mirroring the `<DATA_DIR>/<service>/.pid`
+  // layout from bin/cli/utils/pid.mjs. Wrapped in try/catch — this diagnostic
+  // write must NEVER block or fail shutdown (the give-up branch always calls
+  // process.exit() right after this).
+  persistCrashLog(summary) {
+    try {
+      const crashLogPath = getCrashLogPath();
+      mkdirSync(dirname(crashLogPath), { recursive: true });
+      const timestamp = new Date().toISOString();
+      const body = [`[${timestamp}] ${summary}`, ...this.crashLog, ""].join("\n");
+      appendFileSync(crashLogPath, body, "utf8");
+    } catch {
+      // Best-effort only — a diagnostic write failure must not prevent shutdown.
+    }
+  }
+
   stop() {
     this.isShuttingDown = true;
     if (this.child?.pid) {

changelog.d/fixes/13538-tray-crash-diagnostics.md

@@ -0,0 +1 @@
+- **fix(cli):** persist the supervisor's give-up crash record to `<DATA_DIR>/server/crash.log` (surfaced by `omniroute doctor`) instead of only printing it — the console output was discarded when `--tray` mode's detached worker exited, leaving no trace of why the gateway/tray disappeared (#13538) — thanks @ProphetOfDoom-PoD

tests/unit/fixtures/issue-13538-crashing-server.mjs

@@ -0,0 +1,4 @@
+// Fixture used by tests/unit/issue-13538-tray-crash-diagnostics-lost.test.ts:
+// a minimal "server" that always exits non-zero immediately, simulating a
+// gateway process that repeatedly crashes under load.
+process.exit(1);

tests/unit/issue-13538-tray-crash-diagnostics-lost.test.ts

@@ -0,0 +1,102 @@
+import test from "node:test";
+import assert from "node:assert/strict";
+import { existsSync, mkdtempSync, rmSync, readFileSync } from "node:fs";
+import { join, dirname } from "node:path";
+import { tmpdir } from "node:os";
+import { fileURLToPath } from "node:url";
+
+// #13538: "Windows native tray gateway exits silently under concurrent OpenCode
+// requests" — reporter says the tray worker + gateway both disappear with "no
+// graceful shutdown entry" and no diagnostic anywhere.
+//
+// Root cause (bin/cli/runtime/processSupervisor.mjs handleExit(), give-up
+// branch): once the supervised server child crashes >= maxRestarts times
+// within RESTART_RESET_MS, the supervisor dumps its crash log with
+// console.error() only and calls process.exit(). In `--tray`/`--tray-worker`
+// mode this IS the process that owns the tray icon (bin/cli/commands/serve.mjs
+// runWithSupervisor()), and on Windows/Linux that whole process is launched
+// detached with `stdio: "ignore"` (bin/cli/tray/detachedTray.mjs
+// buildTrayLaunch(), already asserted by
+// tests/unit/cli/tray-detached.test.ts:44-61). So every console.error the
+// give-up path writes is discarded by the OS, and nothing ever persists a
+// record of *why* the tray + gateway died — matching the reporter's "no
+// graceful shutdown entry and no corresponding Windows Application crash
+// event".
+//
+// This test proves the underlying, platform-independent defect: the give-up
+// path never persists its crash log anywhere outside the dying process's own
+// (possibly-ignored) stdio. It asserts the desired behavior — a durable
+// per-service crash record next to the existing PID file convention
+// (bin/cli/utils/pid.mjs getServicePidPath: `<DATA_DIR>/<service>/...`) — and
+// fails today because no such write exists.
+
+process.env.PORT = "0"; // waitUntilPortFree no-ops on port 0 (matches cli-process-supervisor.test.ts)
+
+const HERE = dirname(fileURLToPath(import.meta.url));
+const CRASHING_SERVER = join(HERE, "fixtures", "issue-13538-crashing-server.mjs");
+
+test("#13538: supervisor give-up path persists a discoverable crash record (currently lost)", async () => {
+  const dataDir = mkdtempSync(join(tmpdir(), "omniroute-13538-"));
+  const origDataDir = process.env.DATA_DIR;
+  process.env.DATA_DIR = dataDir;
+
+  const { ServerSupervisor } = await import("../../bin/cli/runtime/processSupervisor.mjs");
+
+  const exits: Array<number | undefined> = [];
+  const origExit = process.exit.bind(process);
+  // @ts-ignore — stub so the real give-up exit doesn't kill the test runner,
+  // exactly like tests/unit/cli-process-supervisor.test.ts does.
+  process.exit = (code?: number) => {
+    exits.push(code);
+  };
+
+  const supervisor = new ServerSupervisor({
+    serverPath: CRASHING_SERVER,
+    env: { ...process.env },
+    maxRestarts: 1,
+    memoryLimit: 64,
+  });
+
+  try {
+    supervisor.start();
+
+    // Real child process crashes twice: 1st crash restarts (backoff ~1s), 2nd
+    // crash exhausts maxRestarts=1 and hits the give-up branch.
+    const deadline = Date.now() + 15_000;
+    while (exits.length === 0 && Date.now() < deadline) {
+      await new Promise((r) => setTimeout(r, 100));
+    }
+
+    assert.equal(exits.length, 1, "supervisor must give up exactly once");
+    assert.equal(exits[0], 1, "give-up exit code must be non-zero (matches production behavior)");
+
+    // Desired behavior: the give-up path leaves a durable, discoverable record
+    // of the crash (so a tray worker killed via this path — spawned detached
+    // with stdio:"ignore" per bin/cli/tray/detachedTray.mjs buildTrayLaunch()
+    // — is still diagnosable afterward, e.g. by `omniroute doctor`).
+    const expectedCrashRecord = join(dataDir, "server", "crash.log");
+    assert.equal(
+      existsSync(expectedCrashRecord),
+      true,
+      `expected a persisted crash record at ${expectedCrashRecord} after the supervisor gave up, ` +
+        `but none was written — the only trace (console.error) is discarded when the process is ` +
+        `spawned with stdio:"ignore" (the exact config used for the detached tray worker on ` +
+        "Windows/Linux), leaving zero diagnostic trace of why the gateway/tray disappeared (#13538)"
+    );
+
+    // The record itself must be useful, not just present: it should carry the
+    // buffered crash log lines the give-up branch already prints to console.
+    const contents = readFileSync(expectedCrashRecord, "utf8");
+    assert.match(
+      contents,
+      /crashed 1 times/,
+      "crash record must include the give-up summary line, not just an empty file"
+    );
+  } finally {
+    // @ts-ignore
+    process.exit = origExit;
+    if (origDataDir === undefined) delete process.env.DATA_DIR;
+    else process.env.DATA_DIR = origDataDir;
+    rmSync(dataDir, { recursive: true, force: true });
+  }
+});