Fix desktop supervised daemon health flaps

Rhys Sullivan committed 4 months ago

Commit ea930021 parent dcb121d4 files · +115 −64Browse files

Unified · Split

4 files

apps/desktop/src/main/diagnostics.ts +4 −0
apps/desktop/src/main/index.ts +6 −0
apps/desktop/src/main/sidecar.ts +4 −3
e2e/desktop-packaged/supervised-regressions.test.ts +101 −61

apps/desktop/src/main/diagnostics.ts+4−0

 const buildManifest = () => {
   const settings = getServerSettings();
+  const executorLogs = join(app.getPath("home"), ".executor", "logs");
   return {
     generated: new Date().toISOString(),
     app: app.getName(),
     paths: {
       userData: app.getPath("userData"),
       logs: dirname(log.transports.file.getFile().path),
+      executorLogs,
       crashDumps: app.getPath("crashDumps"),
     },
     // The bearer token is never included — it stays in auth.json on the machine.
   const { readFile } = await import("node:fs/promises");
   const logsDir = dirname(log.transports.file.getFile().path);
+  const executorLogsDir = join(app.getPath("home"), ".executor", "logs");
   const entries: ZipEntry[] = [
     ...collectFiles(logsDir, "logs"),
+    ...collectFiles(executorLogsDir, "executor-logs"),
     ...collectFiles(app.getPath("crashDumps"), "crash-dumps"),
   ];

apps/desktop/src/main/index.ts+6−0

 // reconnecting overlay while it's down, and reload once it's back.
 let supervisedMonitorTimer: ReturnType<typeof setInterval> | null = null;
 let supervisedDaemonDown = false;
+let supervisedMonitorMisses = 0;
+const SUPERVISED_MONITOR_MISSES_BEFORE_DOWN = 3;
 const stopSupervisedMonitor = () => {
   if (supervisedMonitorTimer) clearInterval(supervisedMonitorTimer);
   supervisedMonitorTimer = null;
   supervisedDaemonDown = false;
+  supervisedMonitorMisses = 0;
 };
 const armSupervisedMonitor = () => {
       const live = await attachToSupervisedDaemon();
       const window = liveMainWindow();
       if (!live) {
+        supervisedMonitorMisses += 1;
+        if (supervisedMonitorMisses < SUPERVISED_MONITOR_MISSES_BEFORE_DOWN) return;
         if (!supervisedDaemonDown && window) {
           supervisedDaemonDown = true;
           const html = sidecarCrashHtml({ reported: errorReportingEnabled });
         }
         return;
       }
+      supervisedMonitorMisses = 0;
       if (supervisedDaemonDown) {
         supervisedDaemonDown = false;
         connection = live;

apps/desktop/src/main/sidecar.ts+4−3

  * sidecar. Reads `server.json`, confirms the recorded process is alive and the
  * endpoint answers, and returns a child-less `SidecarConnection` flagged
  * `supervisedDaemon: true`. Returns null when no usable supervised daemon is
- * present (the caller then falls back to managed-spawn).
+ * present.
  *
  * Only a `cli-daemon` manifest is treated as supervised — a `desktop-sidecar`
  * manifest belongs to a managed sidecar (ours or another desktop instance) and
   const auth = manifest.connection.auth;
   const authToken = auth && auth.kind === "bearer" ? auth.token : "";
   if (!(await isDaemonReachable(origin))) {
-    sidecarLog.info(`removing stale supervised daemon manifest for unreachable ${origin}`);
+    sidecarLog.warn(
-    removeManifestIfOwnedBy(dataDir, manifest.pid);
+      `supervised daemon at ${origin} (pid ${manifest.pid}) did not answer the health probe; keeping its manifest because the process is still alive`,
+    );
     return null;
   }

e2e/desktop-packaged/supervised-regressions.test.ts+101−61

   await stopProcess(app?.child);
 };
-const waitUntil = async (predicate: () => boolean, timeoutMs: number): Promise<boolean> => {
-  const deadline = Date.now() + timeoutMs;
-  for (;;) {
-    if (predicate()) return true;
-    if (Date.now() >= deadline) return false;
-    await new Promise((resolve) => setTimeout(resolve, 100));
-  }
-};
 const waitForServerConnectionLabel = async (
   page: CdpPage,
   expectedText: string,
   it.skip("Desktop packaged supervised attach security (needs a GUI display and no already-running Executor.app)", () => {});
 } else {
   scenario(
-    "Desktop packaged supervised attach · stale manifest probe does not send the saved bearer",
+    "Desktop packaged supervised attach · a slow live daemon does not look crashed",
     { timeout: 240_000 },
-    Effect.promise(() => runStaleManifestProbe()),
+    Effect.gen(function* () {
+      const runDir = yield* RunDir;
+      yield* Effect.promise(() => runSlowLiveDaemonProbe(runDir));
+    }),
   );
   scenario(
   );
 }
-const runStaleManifestProbe = async () => {
+const writeCliDaemonManifest = (input: {
-  const home = mkdtempSync(join(tmpdir(), "executor-pkg-stale-probe-"));
+  readonly controlDir: string;
+  readonly dataDir: string;
+  readonly origin: string;
+  readonly displayName: string;
+  readonly token: string;
+}): void => {
+  mkdirSync(input.controlDir, { recursive: true });
+  writeFileSync(
+    join(input.controlDir, "server.json"),
+    serializeExecutorLocalServerManifest({
+      version: 1,
+      kind: "cli-daemon",
+      pid: process.pid,
+      startedAt: new Date().toISOString(),
+      dataDir: input.dataDir,
+      scopeDir: input.dataDir,
+      connection: normalizeExecutorServerConnection({
+        origin: input.origin,
+        displayName: input.displayName,
+        auth: { kind: "bearer", token: input.token },
+      }),
+      owner: { client: "cli", version: null, executablePath: null },
+    }),
+    { mode: 0o600 },
+  );
+};
+const runSlowLiveDaemonProbe = async (runDir: string) => {
+  const home = mkdtempSync(join(tmpdir(), "executor-pkg-slow-live-daemon-"));
   const dataDir = join(home, ".executor");
   const controlDir = join(dataDir, "server-control");
   const manifestPath = join(controlDir, "server.json");
-  const token = "stale-manifest-leaked-token";
+  const token = "slow-live-daemon-token";
   const launchdSnapshot = captureLaunchdService();
   const requests: Array<{ readonly url: string; readonly authorization: string | null }> = [];
-  let resolveFirst!: () => void;
+  let healthMode: "ok" | "slow" = "ok";
-  const firstRequest = new Promise<void>((resolve) => {
+  let slowHealthResponses = 0;
-    resolveFirst = resolve;
+  let resolveThirdSlowHealthResponse!: () => void;
+  const thirdSlowHealthResponse = new Promise<void>((resolve) => {
+    resolveThirdSlowHealthResponse = resolve;
   });
   const server = createServer((req: IncomingMessage, res) => {
+    const url = req.url ?? "/";
     requests.push({
-      url: req.url ?? "/",
+      url,
       authorization: req.headers.authorization ?? null,
     });
-    resolveFirst();
+    if (url.startsWith("/api/health")) {
+      if (healthMode === "slow") {
+        setTimeout(() => {
+          slowHealthResponses += 1;
+          if (slowHealthResponses >= 3) resolveThirdSlowHealthResponse();
+          if (res.destroyed || res.writableEnded) return;
+          res.writeHead(200, { "content-type": "text/plain" });
+          res.end("ok");
+        }, 5_000);
+        return;
+      }
+      res.writeHead(200, { "content-type": "text/plain" });
+      res.end("ok");
+      return;
+    }
     res.writeHead(200, { "content-type": "text/html" });
-    res.end("<!doctype html><title>stale daemon</title><body>stale daemon</body>");
+    res.end("<!doctype html><title>Executor</title><body><main>Fake Executor UI</main></body>");
   });
   await new Promise<void>((resolve) => server.listen(0, "127.0.0.1", resolve));
   const port = (server.address() as net.AddressInfo).port;
-  let appProcess: ChildProcess | undefined;
+  let app: PackagedApp | undefined;
-  let appOutput = "";
   try {
-    mkdirSync(controlDir, { recursive: true });
+    writeCliDaemonManifest({
-    writeFileSync(
+      controlDir,
-      join(controlDir, "server.json"),
+      dataDir,
-      serializeExecutorLocalServerManifest({
+      origin: `http://127.0.0.1:${port}`,
-        version: 1,
+      displayName: "Slow live daemon",
-        kind: "cli-daemon",
+      token,
-        pid: process.pid,
-        startedAt: new Date().toISOString(),
-        dataDir,
-        scopeDir: dataDir,
-        connection: normalizeExecutorServerConnection({
-          origin: `http://127.0.0.1:${port}`,
-          displayName: "Stale daemon",
-          auth: { kind: "bearer", token },
-        }),
-        owner: { client: "cli", version: null, executablePath: null },
-      }),
-      { mode: 0o600 },
-    );
-    appProcess = spawn(requireBundle().app, [], {
-      env: packagedAppEnv(home),
-      stdio: ["ignore", "pipe", "pipe"],
     });
-    const collectOutput = (chunk: Buffer) => {
-      appOutput = (appOutput + chunk.toString()).slice(-8_192);
-    };
-    appProcess.stdout?.on("data", collectOutput);
-    appProcess.stderr?.on("data", collectOutput);
-    const probed = await Promise.race([
+    app = await launchPackaged(home);
-      firstRequest.then(() => true),
+    const page = app.cdp;
-      new Promise<false>((resolve) => setTimeout(() => resolve(false), 60_000)),
+    await page.waitForText("Fake Executor UI", 120_000);
-    ]);
+    await page.screenshot(join(runDir, "01-attached-to-fake-daemon.png"));
-    expect(probed, `packaged app probed the stale manifest endpoint\n${appOutput}`).toBe(true);
+    const firstHealthProbe = requests.find((request) => request.url.startsWith("/api/health"));
     expect(
-      requests[0]?.authorization ?? null,
+      firstHealthProbe?.authorization ?? null,
-      "the stale-manifest reachability probe must not disclose the saved bearer",
+      "the supervised-daemon health probe must not disclose the saved bearer",
     ).toBeNull();
-    const manifestRemoved = await waitUntil(() => !existsSync(manifestPath), 15_000);
+    healthMode = "slow";
+    const sawSlowMonitorProbe = await Promise.race([
+      thirdSlowHealthResponse.then(() => true),
+      new Promise<false>((resolve) => setTimeout(() => resolve(false), 60_000)),
+    ]);
+    expect(
+      sawSlowMonitorProbe,
+      "the packaged app should continue probing the supervised daemon after initial attach",
+    ).toBe(true);
+    expect(
+      requests
+        .filter((request) => request.url.startsWith("/api/health"))
+        .map((request) => request.authorization),
+      "supervised-daemon health probes must never carry the saved bearer",
+    ).not.toContain(`Bearer ${token}`);
     expect(
-      manifestRemoved,
+      existsSync(manifestPath),
-      "a live pid with a failed health probe must be removed before desktop falls back",
+      "a live daemon's manifest must survive transient health probe failures",
     ).toBe(true);
+    expect(
+      await page.textPresent("The local Executor server stopped unexpectedly"),
+      "one slow monitor probe should not show the crash screen",
+    ).toBe(false);
+    expect(
+      await page.textPresent("Fake Executor UI"),
+      "the original renderer should stay loaded",
+    ).toBe(true);
+    await page.screenshot(join(runDir, "02-still-rendering-after-slow-health.png"));
   } finally {
-    await stopProcess(appProcess);
+    await closePackaged(app);
     await restoreLaunchdService(launchdSnapshot);
     await new Promise<void>((resolve) => server.close(() => resolve()));
     rmSync(home, { recursive: true, force: true });