Merge pull request #48 from closedloop-ai/FEAT-177

FEAT-177: fix CMD+Q not killing the process

Author: shafty023

.github/workflows/claude-code-review.yml

@@ -1,10 +1,6 @@
 name: Claude Code Review
 
 on:
-  pull_request:
-    types: [opened]
-  pull_request_target:
-    types: [opened]
   workflow_dispatch:
 
 # Cancel in-progress runs for the same PR to prevent duplicate reviews

.github/workflows/release.yml

@@ -63,6 +63,10 @@ jobs:
         if: steps.check_release.outputs.skip != 'true'
         run: pnpm -r prebuild
 
+      - name: Lint
+        if: steps.check_release.outputs.skip != 'true'
+        run: pnpm -C apps/desktop lint
+
       - name: Typecheck
         if: steps.check_release.outputs.skip != 'true'
         run: pnpm -C apps/desktop typecheck

apps/desktop/package.json

@@ -1,6 +1,6 @@
 {
   "name": "desktop",
-  "version": "0.9.0",
+  "version": "0.9.1",
   "description": "ClosedLoop Desktop",
   "author": "ClosedLoop AI <support@closedloop.ai>",
   "private": true,

apps/desktop/src/main/app.ts

@@ -58,6 +58,8 @@ import {
 import { LocalSessionStore } from "./local-session-store.js";
 import { enrichJobSnapshot } from "../server/operations/symphony-job-snapshot.js";
 import { GatewayRecoveryManager } from "./gateway-recovery.js";
+import { runShutdownSequence } from "./shutdown.js";
+import type { ShutdownResult } from "./shutdown.js";
 import pkg from "electron-updater";
 const { autoUpdater } = pkg;
 import { BUILD_COMMIT_HASH } from "../shared/build-info.js";
@@ -360,21 +362,26 @@ export class DesktopApplication {
     this.desktopWindow.show();
   }
 
-  async shutdown(): Promise<void> {
+  async shutdown(): Promise<ShutdownResult> {
     if (this.shuttingDown) {
-      return;
+      return "clean";
     }
 
     this.shuttingDown = true;
-    if (this.updateCheckTimer) {
-      clearInterval(this.updateCheckTimer);
-      this.updateCheckTimer = null;
-    }
-    this.cloudSocket.stop();
-    this.commandExecutor.dispose();
-    await this.server.stop();
-    this.desktopWindow.dispose();
-    this.tray.dispose();
+    return runShutdownSequence({
+      updateCheckTimer: this.updateCheckTimer,
+      clearUpdateCheckTimer: () => {
+        if (this.updateCheckTimer) {
+          clearInterval(this.updateCheckTimer);
+          this.updateCheckTimer = null;
+        }
+      },
+      cloudSocket: this.cloudSocket,
+      commandExecutor: this.commandExecutor,
+      server: this.server,
+      desktopWindow: this.desktopWindow,
+      tray: this.tray,
+    });
   }
 
   private async probeGatewayAlive(): Promise<boolean> {

apps/desktop/src/main/index.ts

@@ -18,10 +18,21 @@ app.on("activate", () => {
   desktopApplication.showWindow();
 });
 
-app.on("before-quit", () => {
-  void desktopApplication.shutdown().catch((error) => {
-    const message = error instanceof Error ? error.message : "unknown shutdown error";
-    console.error(`desktop shutdown failed: ${message}`);
+let quitPromise: Promise<void> | null = null;
+
+app.on("before-quit", (event) => {
+  // Prevent Electron from proceeding until async shutdown completes.
+  event.preventDefault();
+
+  // If shutdown is already in progress (e.g. window-all-closed fired app.quit()
+  // on non-macOS after DesktopWindow.dispose() closed the last window), do nothing.
+  // The first invocation's .then() continuation will call app.exit() exactly once.
+  if (quitPromise) {
+    return;
+  }
+
+  quitPromise = desktopApplication.shutdown().then((result) => {
+    app.exit(result === "clean" ? 0 : 1);
   });
 });
 

apps/desktop/src/main/shutdown.ts

@@ -0,0 +1,46 @@
+export interface ShutdownDeps {
+  updateCheckTimer: NodeJS.Timeout | null;
+  clearUpdateCheckTimer: () => void;
+  cloudSocket: { stop: () => void };
+  commandExecutor: { dispose: () => void };
+  server: { stop: () => Promise<void> };
+  desktopWindow: { dispose: () => void };
+  tray: { dispose: () => void };
+}
+
+export type ShutdownResult = "clean" | "timed_out" | "failed";
+
+export async function runShutdownSequence(
+  deps: ShutdownDeps,
+  options?: { timeoutMs?: number; setTimeoutFn?: typeof setTimeout }
+): Promise<ShutdownResult> {
+  const timeoutMs = options?.timeoutMs ?? 5000;
+  const setTimeoutFn = options?.setTimeoutFn ?? setTimeout;
+
+  let timer: ReturnType<typeof setTimeout> | null = null;
+
+  const cleanup = async (): Promise<"clean"> => {
+    deps.clearUpdateCheckTimer();
+    deps.cloudSocket.stop();
+    deps.commandExecutor.dispose();
+    await deps.server.stop();
+    deps.desktopWindow.dispose();
+    deps.tray.dispose();
+    return "clean";
+  };
+
+  const timeout = new Promise<"timed_out">((resolve) => {
+    timer = setTimeoutFn(() => resolve("timed_out"), timeoutMs);
+  });
+
+  try {
+    const result = await Promise.race([cleanup(), timeout]);
+    return result;
+  } catch {
+    return "failed";
+  } finally {
+    if (timer != null) {
+      clearTimeout(timer);
+    }
+  }
+}

apps/desktop/src/server/operations/symphony-loop.ts

@@ -311,7 +311,7 @@ function buildClaudePipeline(
       `tee -a ${shellEscape(jsonlFile)}`,
       `python3 ${shellEscape(formatter)}`,
     ].join(" | ");
-    return { cmd: "bash", args: ["-c", pipeline] };
+    return { cmd: "bash", args: ["-c", `${pipeline}; exit \${PIPESTATUS[0]}`] };
   }
 
   // No formatter — wrap in bash pipeline so grep|tee still writes claude-output.jsonl
@@ -320,7 +320,7 @@ function buildClaudePipeline(
     "grep --line-buffered '^{'",
     `tee -a ${shellEscape(jsonlFile)}`,
   ].join(" | ");
-  return { cmd: "bash", args: ["-c", pipeline] };
+  return { cmd: "bash", args: ["-c", `${pipeline}; exit \${PIPESTATUS[0]}`] };
 }
 
 /** Find the local repo path for a given fullName (e.g. "org/repo"). */

apps/desktop/test/shutdown.test.ts

@@ -0,0 +1,138 @@
+import assert from "node:assert/strict";
+import { describe, test } from "node:test";
+import {
+  runShutdownSequence,
+  type ShutdownDeps,
+} from "../src/main/shutdown.js";
+
+/** Build stub deps that record call order. */
+function makeStubDeps(overrides?: Partial<ShutdownDeps>) {
+  const calls: string[] = [];
+  const deps: ShutdownDeps = {
+    updateCheckTimer: null,
+    clearUpdateCheckTimer: () => {
+      calls.push("clearUpdateCheckTimer");
+    },
+    cloudSocket: {
+      stop: () => {
+        calls.push("cloudSocket.stop");
+      },
+    },
+    commandExecutor: {
+      dispose: () => {
+        calls.push("commandExecutor.dispose");
+      },
+    },
+    server: {
+      stop: async () => {
+        calls.push("server.stop");
+      },
+    },
+    desktopWindow: {
+      dispose: () => {
+        calls.push("desktopWindow.dispose");
+      },
+    },
+    tray: {
+      dispose: () => {
+        calls.push("tray.dispose");
+      },
+    },
+    ...overrides,
+  };
+  return { deps, calls };
+}
+
+describe("runShutdownSequence", () => {
+  test("clean path: all deps succeed, cleanup steps called in order", async () => {
+    const { deps, calls } = makeStubDeps();
+
+    const result = await runShutdownSequence(deps);
+
+    assert.equal(result, "clean");
+    assert.deepEqual(calls, [
+      "clearUpdateCheckTimer",
+      "cloudSocket.stop",
+      "commandExecutor.dispose",
+      "server.stop",
+      "desktopWindow.dispose",
+      "tray.dispose",
+    ]);
+  });
+
+  test("timeout path: result is 'timed_out' when server.stop never resolves", async () => {
+    const { deps } = makeStubDeps({
+      server: {
+        stop: () => new Promise<void>(() => {}), // never resolves
+      },
+    });
+
+    // Stub setTimeoutFn that fires the callback immediately
+    const stubSetTimeout = ((cb: () => void) => {
+      cb();
+      return 999 as unknown as ReturnType<typeof setTimeout>;
+    }) as unknown as typeof setTimeout;
+
+    const result = await runShutdownSequence(deps, {
+      setTimeoutFn: stubSetTimeout,
+    });
+
+    assert.equal(result, "timed_out");
+  });
+
+  test("failed path: server.stop rejects with an error", async () => {
+    const { deps } = makeStubDeps({
+      server: {
+        stop: () => Promise.reject(new Error("stop failed")),
+      },
+    });
+
+    // Use a setTimeoutFn that never fires so timeout doesn't win
+    const neverTimeout = (() =>
+      42 as unknown as ReturnType<typeof setTimeout>) as unknown as typeof setTimeout;
+
+    const result = await runShutdownSequence(deps, {
+      setTimeoutFn: neverTimeout,
+    });
+
+    assert.equal(result, "failed");
+  });
+
+  test("timer is cleared after cleanup resolves (no leaked handles)", async () => {
+    const { deps } = makeStubDeps();
+
+    let capturedTimerId: ReturnType<typeof setTimeout> | null = null;
+    let clearTimeoutCalledWith: unknown = null;
+
+    // Monkey-patch clearTimeout to observe the call
+    const origClearTimeout = globalThis.clearTimeout;
+    globalThis.clearTimeout = ((id: unknown) => {
+      clearTimeoutCalledWith = id;
+      origClearTimeout(id as ReturnType<typeof setTimeout>);
+    }) as typeof clearTimeout;
+
+    try {
+      // Use a real-ish setTimeoutFn that returns a recognizable timer id
+      const stubSetTimeout = ((_cb: () => void, _ms: number) => {
+        const id = origClearTimeout.bind(
+          null
+        ) as unknown as ReturnType<typeof setTimeout>;
+        capturedTimerId = 12345 as unknown as ReturnType<typeof setTimeout>;
+        return capturedTimerId;
+      }) as unknown as typeof setTimeout;
+
+      const result = await runShutdownSequence(deps, {
+        setTimeoutFn: stubSetTimeout,
+      });
+
+      assert.equal(result, "clean");
+      assert.equal(
+        clearTimeoutCalledWith,
+        capturedTimerId,
+        "clearTimeout should be called with the timer id returned by setTimeoutFn"
+      );
+    } finally {
+      globalThis.clearTimeout = origClearTimeout;
+    }
+  });
+});