Skip to content
This repository was archived by the owner on Jun 8, 2026. It is now read-only.

Commit 9f3579c

Browse files
committed
FEAT-177: fix CMD+Q not killing the process
- Rewrite before-quit handler to call event.preventDefault(), run async shutdown, then app.exit() to guarantee termination - Extract ShutdownDeps interface and runShutdownSequence() into new shutdown.ts with 5s timeout and injectable timer for tests - Change shutdown() in app.ts to delegate to runShutdownSequence and return "clean", "timed_out", or "failed" result - Add shutdown.test.ts covering clean, timeout, failed, and timer cleanup paths - Bump version 0.8.9 -> 0.8.10 Testing: All 432 tests pass, lint clean, typecheck clean Risks: app.exit() skips Electron window cleanup events, but we already clean up everything in shutdown(); strictly better than the current behavior where the process never exits
1 parent ed04193 commit 9f3579c

5 files changed

Lines changed: 218 additions & 16 deletions

File tree

apps/desktop/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "desktop",
3-
"version": "0.9.0",
3+
"version": "0.9.1",
44
"description": "ClosedLoop Desktop",
55
"author": "ClosedLoop AI <support@closedloop.ai>",
66
"private": true,

apps/desktop/src/main/app.ts

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,8 @@ import {
5858
import { LocalSessionStore } from "./local-session-store.js";
5959
import { enrichJobSnapshot } from "../server/operations/symphony-job-snapshot.js";
6060
import { GatewayRecoveryManager } from "./gateway-recovery.js";
61+
import { runShutdownSequence } from "./shutdown.js";
62+
import type { ShutdownResult } from "./shutdown.js";
6163
import pkg from "electron-updater";
6264
const { autoUpdater } = pkg;
6365
import { BUILD_COMMIT_HASH } from "../shared/build-info.js";
@@ -360,21 +362,26 @@ export class DesktopApplication {
360362
this.desktopWindow.show();
361363
}
362364

363-
async shutdown(): Promise<void> {
365+
async shutdown(): Promise<ShutdownResult> {
364366
if (this.shuttingDown) {
365-
return;
367+
return "clean";
366368
}
367369

368370
this.shuttingDown = true;
369-
if (this.updateCheckTimer) {
370-
clearInterval(this.updateCheckTimer);
371-
this.updateCheckTimer = null;
372-
}
373-
this.cloudSocket.stop();
374-
this.commandExecutor.dispose();
375-
await this.server.stop();
376-
this.desktopWindow.dispose();
377-
this.tray.dispose();
371+
return runShutdownSequence({
372+
updateCheckTimer: this.updateCheckTimer,
373+
clearUpdateCheckTimer: () => {
374+
if (this.updateCheckTimer) {
375+
clearInterval(this.updateCheckTimer);
376+
this.updateCheckTimer = null;
377+
}
378+
},
379+
cloudSocket: this.cloudSocket,
380+
commandExecutor: this.commandExecutor,
381+
server: this.server,
382+
desktopWindow: this.desktopWindow,
383+
tray: this.tray,
384+
});
378385
}
379386

380387
private async probeGatewayAlive(): Promise<boolean> {

apps/desktop/src/main/index.ts

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,21 @@ app.on("activate", () => {
1818
desktopApplication.showWindow();
1919
});
2020

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

apps/desktop/src/main/shutdown.ts

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
export interface ShutdownDeps {
2+
updateCheckTimer: NodeJS.Timeout | null;
3+
clearUpdateCheckTimer: () => void;
4+
cloudSocket: { stop: () => void };
5+
commandExecutor: { dispose: () => void };
6+
server: { stop: () => Promise<void> };
7+
desktopWindow: { dispose: () => void };
8+
tray: { dispose: () => void };
9+
}
10+
11+
export type ShutdownResult = "clean" | "timed_out" | "failed";
12+
13+
export async function runShutdownSequence(
14+
deps: ShutdownDeps,
15+
options?: { timeoutMs?: number; setTimeoutFn?: typeof setTimeout }
16+
): Promise<ShutdownResult> {
17+
const timeoutMs = options?.timeoutMs ?? 5000;
18+
const setTimeoutFn = options?.setTimeoutFn ?? setTimeout;
19+
20+
let timer: ReturnType<typeof setTimeout> | null = null;
21+
22+
const cleanup = async (): Promise<"clean"> => {
23+
deps.clearUpdateCheckTimer();
24+
deps.cloudSocket.stop();
25+
deps.commandExecutor.dispose();
26+
await deps.server.stop();
27+
deps.desktopWindow.dispose();
28+
deps.tray.dispose();
29+
return "clean";
30+
};
31+
32+
const timeout = new Promise<"timed_out">((resolve) => {
33+
timer = setTimeoutFn(() => resolve("timed_out"), timeoutMs);
34+
});
35+
36+
try {
37+
const result = await Promise.race([cleanup(), timeout]);
38+
return result;
39+
} catch {
40+
return "failed";
41+
} finally {
42+
if (timer != null) {
43+
clearTimeout(timer);
44+
}
45+
}
46+
}

apps/desktop/test/shutdown.test.ts

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,138 @@
1+
import assert from "node:assert/strict";
2+
import { describe, test } from "node:test";
3+
import {
4+
runShutdownSequence,
5+
type ShutdownDeps,
6+
} from "../src/main/shutdown.js";
7+
8+
/** Build stub deps that record call order. */
9+
function makeStubDeps(overrides?: Partial<ShutdownDeps>) {
10+
const calls: string[] = [];
11+
const deps: ShutdownDeps = {
12+
updateCheckTimer: null,
13+
clearUpdateCheckTimer: () => {
14+
calls.push("clearUpdateCheckTimer");
15+
},
16+
cloudSocket: {
17+
stop: () => {
18+
calls.push("cloudSocket.stop");
19+
},
20+
},
21+
commandExecutor: {
22+
dispose: () => {
23+
calls.push("commandExecutor.dispose");
24+
},
25+
},
26+
server: {
27+
stop: async () => {
28+
calls.push("server.stop");
29+
},
30+
},
31+
desktopWindow: {
32+
dispose: () => {
33+
calls.push("desktopWindow.dispose");
34+
},
35+
},
36+
tray: {
37+
dispose: () => {
38+
calls.push("tray.dispose");
39+
},
40+
},
41+
...overrides,
42+
};
43+
return { deps, calls };
44+
}
45+
46+
describe("runShutdownSequence", () => {
47+
test("clean path: all deps succeed, cleanup steps called in order", async () => {
48+
const { deps, calls } = makeStubDeps();
49+
50+
const result = await runShutdownSequence(deps);
51+
52+
assert.equal(result, "clean");
53+
assert.deepEqual(calls, [
54+
"clearUpdateCheckTimer",
55+
"cloudSocket.stop",
56+
"commandExecutor.dispose",
57+
"server.stop",
58+
"desktopWindow.dispose",
59+
"tray.dispose",
60+
]);
61+
});
62+
63+
test("timeout path: result is 'timed_out' when server.stop never resolves", async () => {
64+
const { deps } = makeStubDeps({
65+
server: {
66+
stop: () => new Promise<void>(() => {}), // never resolves
67+
},
68+
});
69+
70+
// Stub setTimeoutFn that fires the callback immediately
71+
const stubSetTimeout = ((cb: () => void) => {
72+
cb();
73+
return 999 as unknown as ReturnType<typeof setTimeout>;
74+
}) as unknown as typeof setTimeout;
75+
76+
const result = await runShutdownSequence(deps, {
77+
setTimeoutFn: stubSetTimeout,
78+
});
79+
80+
assert.equal(result, "timed_out");
81+
});
82+
83+
test("failed path: server.stop rejects with an error", async () => {
84+
const { deps } = makeStubDeps({
85+
server: {
86+
stop: () => Promise.reject(new Error("stop failed")),
87+
},
88+
});
89+
90+
// Use a setTimeoutFn that never fires so timeout doesn't win
91+
const neverTimeout = (() =>
92+
42 as unknown as ReturnType<typeof setTimeout>) as unknown as typeof setTimeout;
93+
94+
const result = await runShutdownSequence(deps, {
95+
setTimeoutFn: neverTimeout,
96+
});
97+
98+
assert.equal(result, "failed");
99+
});
100+
101+
test("timer is cleared after cleanup resolves (no leaked handles)", async () => {
102+
const { deps } = makeStubDeps();
103+
104+
let capturedTimerId: ReturnType<typeof setTimeout> | null = null;
105+
let clearTimeoutCalledWith: unknown = null;
106+
107+
// Monkey-patch clearTimeout to observe the call
108+
const origClearTimeout = globalThis.clearTimeout;
109+
globalThis.clearTimeout = ((id: unknown) => {
110+
clearTimeoutCalledWith = id;
111+
origClearTimeout(id as ReturnType<typeof setTimeout>);
112+
}) as typeof clearTimeout;
113+
114+
try {
115+
// Use a real-ish setTimeoutFn that returns a recognizable timer id
116+
const stubSetTimeout = ((_cb: () => void, _ms: number) => {
117+
const id = origClearTimeout.bind(
118+
null
119+
) as unknown as ReturnType<typeof setTimeout>;
120+
capturedTimerId = 12345 as unknown as ReturnType<typeof setTimeout>;
121+
return capturedTimerId;
122+
}) as unknown as typeof setTimeout;
123+
124+
const result = await runShutdownSequence(deps, {
125+
setTimeoutFn: stubSetTimeout,
126+
});
127+
128+
assert.equal(result, "clean");
129+
assert.equal(
130+
clearTimeoutCalledWith,
131+
capturedTimerId,
132+
"clearTimeout should be called with the timer id returned by setTimeoutFn"
133+
);
134+
} finally {
135+
globalThis.clearTimeout = origClearTimeout;
136+
}
137+
});
138+
});

0 commit comments

Comments
 (0)