desktop: classify expected sidecar shutdowns

apps/desktop/src/main/app-quit.ts

@@ -0,0 +1,23 @@
+/**
+ * Shared "is the app on its way out?" flag for the main process.
+ *
+ * Teardown makes several failures expected rather than exceptional: a sidecar
+ * child dying, a navigation aborting mid-load. Both the sidecar and window code
+ * need the same answer, so the flag lives here instead of being threaded
+ * through either. Listeners are registered at import time (Electron accepts
+ * `app.on` before ready) so no caller can forget to arm it.
+ */
+
+import { app } from "electron";
+
+let quitting = false;
+
+const markQuitting = () => {
+  quitting = true;
+};
+
+app.on("before-quit", markQuitting);
+app.on("will-quit", markQuitting);
+app.on("quit", markQuitting);
+
+export const isAppQuitting = (): boolean => quitting;

apps/desktop/src/main/crash-screen.test.ts

@@ -0,0 +1,26 @@
+/**
+ * The screen is shown for an outside interrupt as well as a real crash — the
+ * server is gone either way — so the one sentence that must not appear when
+ * nothing was sent upstream is pinned here. Telling a user a report was filed
+ * when none was is the visible half of the reported problem.
+ */
+
+import { describe, expect, it } from "@effect/vitest";
+import { sidecarCrashHtml } from "./crash-screen";
+
+const REPORT_CLAIM = "A crash report was sent automatically";
+
+describe("sidecarCrashHtml", () => {
+  it("claims nothing was sent when nothing was", () => {
+    const html = sidecarCrashHtml({ reported: false });
+
+    expect(html).not.toContain(REPORT_CLAIM);
+    // The screen still has to explain the server is gone and offer a way back.
+    expect(html).toContain("The local Executor server stopped unexpectedly");
+    expect(html).toContain("Restart server");
+  });
+
+  it("says a report was sent when one was", () => {
+    expect(sidecarCrashHtml({ reported: true })).toContain(REPORT_CLAIM);
+  });
+});

apps/desktop/src/main/index.ts

@@ -35,6 +35,8 @@ import {
   reportAProblem,
 } from "./diagnostics";
 import { sidecarCrashHtml, startupWindowHtml } from "./crash-screen";
+import { isExpectedNavigationAbort } from "./navigation-errors";
+import { isAppQuitting } from "./app-quit";
 import { replaceSupervisedDaemonForDesktop } from "./supervised-connection";
 import {
   bundledExecutorPath,
@@ -148,6 +150,37 @@ const webUrlForConnection = (conn: SidecarConnection): string => {
 const htmlDataUrl = (html: string): string =>
   `data:text/html;charset=utf-8,${encodeURIComponent(html)}`;
 
+/**
+ * True when a `loadURL` rejection is just Chromium cancelling a navigation
+ * whose window went away — the window closing mid-load is a race we cause, not
+ * a failure to reach the server.
+ */
+const isTeardownNavigationError = (error: unknown, window: BrowserWindow): boolean =>
+  isExpectedNavigationAbort({
+    // oxlint-disable-next-line executor/no-instanceof-error, executor/no-unknown-error-message -- boundary: Electron rejects loadURL with a plain Node Error carrying the net error name
+    message: error instanceof Error ? error.message : String(error),
+    windowDestroyed: window.isDestroyed(),
+    appQuitting: isAppQuitting(),
+  });
+
+/**
+ * Fire-and-forget navigation. The caller has no way to react to a failure, so
+ * an uncaught rejection here would only reach the main-process unhandled
+ * rejection handler; log it instead, quietly when it is teardown noise.
+ */
+const loadWindowUrl = async (window: BrowserWindow, url: string): Promise<void> => {
+  // oxlint-disable-next-line executor/no-try-catch-or-throw -- boundary: Electron navigation rejects; there is no caller left to surface it to
+  try {
+    await window.loadURL(url);
+  } catch (error) {
+    if (isTeardownNavigationError(error, window)) {
+      log.info("Navigation aborted while the window was closing");
+      return;
+    }
+    log.error("Failed to load window URL", error);
+  }
+};
+
 // The supervised daemon (and the desktop sidecar) own this data dir — the same
 // path the CLI's `executor web`/daemon uses, so desktop and CLI share state.
 const DESKTOP_DATA_DIR = join(homedir(), ".executor");
@@ -355,7 +388,7 @@ const armSupervisedMonitor = () => {
         supervisedDaemonDown = false;
         connection = live;
         installBearerAuthHeader(live.baseUrl, live.authToken);
-        if (window) void window.loadURL(webUrlForConnection(live));
+        if (window) void loadWindowUrl(window, webUrlForConnection(live));
       }
     })();
   }, 10_000);
@@ -540,10 +573,13 @@ const showStartupWindow = async (): Promise<void> => {
   }
 };
 
-const showCrashScreen = (window: BrowserWindow | null = liveMainWindow()): void => {
+const showCrashScreen = (
+  window: BrowserWindow | null = liveMainWindow(),
+  options: { readonly reported: boolean } = { reported: errorReportingEnabled },
+): void => {
   if (!window) return;
-  const html = sidecarCrashHtml({ reported: errorReportingEnabled });
-  void window.loadURL(htmlDataUrl(html));
+  const html = sidecarCrashHtml({ reported: options.reported && errorReportingEnabled });
+  void loadWindowUrl(window, htmlDataUrl(html));
 };
 
 const createWindow = async (conn: SidecarConnection) => {
@@ -560,6 +596,12 @@ const createWindow = async (conn: SidecarConnection) => {
     await window.loadURL(webUrlForConnection(conn));
     if (!window.isDestroyed() && !window.isVisible()) window.show();
   } catch (error) {
+    // The window was closed (or the app is quitting) while this navigation was
+    // in flight. Nothing failed — there is just nowhere left to navigate.
+    if (isTeardownNavigationError(error, window)) {
+      log.info("Executor web UI load aborted while the window was closing");
+      return;
+    }
     log.error("Failed to load Executor web UI", error);
     if (!existingWindow) destroyWindow(window);
     // oxlint-disable-next-line executor/no-try-catch-or-throw -- boundary: caller decides whether to fall back or surface startup failure
@@ -1158,8 +1200,11 @@ const boot = async () => {
   // A sidecar that dies under a live window would leave the web UI failing
   // every request with no explanation. Swap in the crash screen — its
   // buttons drive the regular preload bridge (restart / export diagnostics).
-  onUnexpectedSidecarExit(() => {
-    showCrashScreen();
+  onUnexpectedSidecarExit((notice) => {
+    // An expected shutdown (an interrupt aimed at the app) still leaves the web
+    // UI dead, so the screen is the same — it just must not claim a crash
+    // report was sent when none was.
+    showCrashScreen(liveMainWindow(), { reported: notice.reported });
     // A crashing sidecar may be a broken release — quietly stage any
     // available update so the install prompt appears on its own (same
     // self-heal as the fatal startup path).

apps/desktop/src/main/navigation-errors.test.ts

@@ -0,0 +1,40 @@
+import { describe, expect, it } from "@effect/vitest";
+import { isExpectedNavigationAbort } from "./navigation-errors";
+
+const abort = (overrides: Partial<Parameters<typeof isExpectedNavigationAbort>[0]> = {}) =>
+  isExpectedNavigationAbort({
+    message: "ERR_FAILED (-2) loading 'http://127.0.0.1:4789/?_token=redacted'",
+    windowDestroyed: false,
+    appQuitting: false,
+    ...overrides,
+  });
+
+describe("isExpectedNavigationAbort", () => {
+  it("treats ERR_FAILED as expected when the window was destroyed mid-load", () => {
+    expect(abort({ windowDestroyed: true })).toBe(true);
+  });
+
+  it("treats ERR_CONNECTION_RESET as expected while the app is quitting", () => {
+    expect(
+      abort({
+        message: "ERR_CONNECTION_RESET (-101) loading 'http://127.0.0.1:4789/'",
+        appQuitting: true,
+      }),
+    ).toBe(true);
+  });
+
+  it("keeps ERR_FAILED unexpected under a live window", () => {
+    // A load that fails while the window is alive and the app is running is a
+    // genuine "cannot reach the local server" failure and must still surface.
+    expect(abort()).toBe(false);
+  });
+
+  it("keeps an unrelated navigation failure unexpected even during teardown", () => {
+    expect(
+      abort({
+        message: "ERR_CERT_AUTHORITY_INVALID (-202) loading 'https://example.invalid/'",
+        windowDestroyed: true,
+      }),
+    ).toBe(false);
+  });
+});

apps/desktop/src/main/navigation-errors.ts

@@ -0,0 +1,21 @@
+/**
+ * Classification of `BrowserWindow.loadURL` rejections.
+ *
+ * Chromium cancels an in-flight navigation when the window it belongs to is
+ * destroyed, and reports the cancellation as a generic net error (ERR_FAILED
+ * / ERR_ABORTED / ERR_CONNECTION_RESET). That is a shutdown race with no
+ * diagnostic value. The same net errors under a live window mean the local
+ * server genuinely could not be loaded, which must still surface — so this is
+ * a branch, never a blanket ignore.
+ */
+
+const TEARDOWN_NET_ERRORS = /ERR_FAILED|ERR_ABORTED|ERR_CONNECTION_RESET|ERR_CONNECTION_CLOSED/;
+
+export interface NavigationAbortInput {
+  readonly message: string;
+  readonly windowDestroyed: boolean;
+  readonly appQuitting: boolean;
+}
+
+export const isExpectedNavigationAbort = (input: NavigationAbortInput): boolean =>
+  (input.windowDestroyed || input.appQuitting) && TEARDOWN_NET_ERRORS.test(input.message);

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

@@ -0,0 +1,163 @@
+/**
+ * The classifier is pure and tested next door; this covers what `startSidecar`
+ * DOES with each verdict — which is where the reported symptom lived. A child
+ * interrupted from outside must leave the crash channel untouched and tell the
+ * window no report was sent, while a genuine post-boot death must still be
+ * reported. Both halves are driven through the real `startSidecar`, with only
+ * the process spawn and Electron's environment replaced.
+ */
+
+import { describe, expect, it } from "@effect/vitest";
+// oxlint-disable-next-line executor/no-vitest-import -- boundary: vi.mock must come from vitest itself for mock hoisting to resolve
+import { vi } from "vitest";
+import { EventEmitter } from "node:events";
+
+let appQuitting = false;
+const crashReports: string[] = [];
+
+vi.mock("electron", () => ({
+  app: { isPackaged: true, getVersion: () => "1.6.0", on: () => {} },
+}));
+
+vi.mock("electron-log/main.js", () => {
+  const scope = () => ({ info: () => {}, error: () => {}, warn: () => {}, debug: () => {} });
+  return { default: { scope }, scope };
+});
+
+vi.mock("./local-auth", () => ({ loadOrMintLocalAuthToken: () => "test-token" }));
+
+vi.mock("./settings", () => ({ getServerSettings: () => ({ port: 4789 }) }));
+
+vi.mock("./diagnostics", () => ({
+  reportSidecarCrash: (message: string) => {
+    crashReports.push(message);
+  },
+  sidecarCrashReportingEnv: () => ({}),
+}));
+
+vi.mock("./supervised-daemon", () => ({ resolveSupervisedDaemonAttach: () => null }));
+
+vi.mock("./app-quit", () => ({ isAppQuitting: () => appQuitting }));
+
+class FakeChild extends EventEmitter {
+  readonly stdout = new EventEmitter();
+  readonly stderr = new EventEmitter();
+  readonly pid = 4242;
+  exitCode: number | null = null;
+  killed = false;
+  kill() {
+    this.killed = true;
+    return true;
+  }
+}
+
+// The packaged branch of `resolveSidecarCommand` reads Electron's resources
+// path; nothing is executed, since the spawn itself is replaced below.
+(process as { resourcesPath?: string }).resourcesPath ??= "/tmp/executor-test-resources";
+
+let spawned: FakeChild | null = null;
+
+vi.mock("node:child_process", () => ({
+  spawn: () => {
+    spawned = new FakeChild();
+    return spawned;
+  },
+}));
+
+const { startSidecar, onUnexpectedSidecarExit, stopSidecar } = await import("./sidecar");
+
+/** Boot a sidecar to the point `startSidecar` resolves, then hand back the child. */
+const bootedSidecar = async (): Promise<FakeChild> => {
+  const started = startSidecar();
+  // The child announces readiness with the structured stdout sentinel.
+  await Promise.resolve();
+  spawned?.stdout.emit("data", Buffer.from("EXECUTOR_READY:4789\n"));
+  await started;
+  // oxlint-disable-next-line executor/no-non-null-assertion -- the spawn mock always records a child
+  return spawned!;
+};
+
+interface Observed {
+  readonly notices: { readonly reported: boolean }[];
+  readonly reports: string[];
+}
+
+/** Boot, drive one post-boot exit, and report what the app did about it. */
+const exitAfterBoot = async (
+  exit: { readonly code: number | null; readonly signal: NodeJS.Signals | null },
+  options: { readonly quitting?: boolean; readonly stopFirst?: boolean } = {},
+): Promise<Observed> => {
+  appQuitting = false;
+  crashReports.length = 0;
+  const notices: { readonly reported: boolean }[] = [];
+  onUnexpectedSidecarExit((notice) => notices.push(notice));
+
+  const child = await bootedSidecar();
+  child.stderr.emit("data", Buffer.from("some trailing output\n"));
+  if (options.stopFirst) {
+    const stopping = stopSidecar(child as never);
+    child.emit("exit", exit.code, exit.signal);
+    await stopping;
+  } else {
+    appQuitting = options.quitting ?? false;
+    child.emit("exit", exit.code, exit.signal);
+  }
+  await Promise.resolve();
+  return { notices, reports: [...crashReports] };
+};
+
+describe("startSidecar post-boot exit handling", () => {
+  it("does not report an outside interrupt, and tells the window nothing was sent", async () => {
+    // A group-wide Ctrl-C reaches the child because it shares Electron's
+    // process group; Node surfaces it as 128+SIGINT. The server really is gone,
+    // so the window must still be told — but no crash report exists to claim.
+    const { notices, reports } = await exitAfterBoot({ code: 130, signal: null });
+
+    expect(reports).toEqual([]);
+    expect(notices).toEqual([{ reported: false }]);
+  });
+
+  it("does not report a SIGTERM the app did not send", async () => {
+    const { notices, reports } = await exitAfterBoot({ code: null, signal: "SIGTERM" });
+
+    expect(reports).toEqual([]);
+    expect(notices).toEqual([{ reported: false }]);
+  });
+
+  it("still reports a genuine post-boot death and says a report was sent", async () => {
+    const { notices, reports } = await exitAfterBoot({ code: 1, signal: null });
+
+    expect(reports).toHaveLength(1);
+    expect(reports[0]).toContain("code=1");
+    expect(notices).toEqual([{ reported: true }]);
+  });
+
+  it("still reports an abnormal death signal", async () => {
+    const { notices, reports } = await exitAfterBoot({ code: null, signal: "SIGSEGV" });
+
+    expect(reports).toHaveLength(1);
+    expect(notices).toEqual([{ reported: true }]);
+  });
+
+  it("says nothing at all when the app itself stopped the sidecar", async () => {
+    const { notices, reports } = await exitAfterBoot(
+      { code: null, signal: "SIGTERM" },
+      { stopFirst: true },
+    );
+
+    expect(reports).toEqual([]);
+    expect(notices).toEqual([]);
+  });
+
+  it("says nothing at all when the app is quitting", async () => {
+    // Quit tears the tree down; the window is going away, so surfacing a
+    // disconnected screen at that moment would be its own bug.
+    const { notices, reports } = await exitAfterBoot(
+      { code: 130, signal: null },
+      { quitting: true },
+    );
+
+    expect(reports).toEqual([]);
+    expect(notices).toEqual([]);
+  });
+});