diff --git a/src/hooks/gmail-watcher.test.ts b/src/hooks/gmail-watcher.test.ts index ab50900b481..143f1611483 100644 --- a/src/hooks/gmail-watcher.test.ts +++ b/src/hooks/gmail-watcher.test.ts @@ -197,4 +197,164 @@ describe("startGmailWatcher", () => { }); expect(mocks.spawn).not.toHaveBeenCalled(); }); + + it("kills existing watcher process on re-entry before spawning new one", async () => { + mocks.runCommandWithTimeout.mockResolvedValue({ code: 0, stdout: "", stderr: "" }); + const spawnedChildren: Array< + EventEmitter & { kill: ReturnType; killed: boolean } + > = []; + mocks.spawn.mockImplementation(() => { + const child = new EventEmitter(); + const mockedChild = Object.assign(child, { + kill: vi.fn(() => { + queueMicrotask(() => child.emit("exit", null, "SIGTERM")); + return true; + }), + killed: false, + }); + spawnedChildren.push(mockedChild); + return mockedChild; + }); + + // First start + await startGmailWatcher(createGmailConfig()); + expect(spawnedChildren).toHaveLength(1); + expect(spawnedChildren[0].kill).not.toHaveBeenCalled(); + + // Second start (re-entry) should kill the first process + await startGmailWatcher(createGmailConfig()); + expect(spawnedChildren).toHaveLength(2); + expect(spawnedChildren[0].kill).toHaveBeenCalledWith("SIGTERM"); + }); + + it("clears existing renewInterval on re-entry to prevent interval leak", async () => { + vi.useFakeTimers(); + try { + mocks.runCommandWithTimeout.mockResolvedValue({ code: 0, stdout: "", stderr: "" }); + + // First start - creates a renewal interval + await startGmailWatcher(createGmailConfig()); + const timersAfterFirstStart = vi.getTimerCount(); + expect(timersAfterFirstStart).toBeGreaterThanOrEqual(1); + + // Second start (re-entry without stop) - the guard should clear the old + // interval before creating a new one, keeping the timer count stable. + await startGmailWatcher(createGmailConfig()); + expect(vi.getTimerCount()).toBe(timersAfterFirstStart); + } finally { + vi.useRealTimers(); + } + }); + + it("only one renewal fires per tick after multiple starts", async () => { + vi.useFakeTimers(); + try { + // Resolve watch-start immediately on every call + mocks.runCommandWithTimeout.mockResolvedValue({ code: 0, stdout: "", stderr: "" }); + + // Start twice without stopping + await startGmailWatcher(createGmailConfig()); + await startGmailWatcher(createGmailConfig()); + + // runCommandWithTimeout is called once per start (the gog watch start + // call). After two successful starts it has been called twice. + expect(mocks.runCommandWithTimeout).toHaveBeenCalledTimes(2); + + // Advance by one full renewal cycle. + // Default renewEveryMinutes = 720 (12 h) = 43_200_000 ms. + // If the old interval leaked, the callback would fire twice per cycle. + await vi.advanceTimersByTimeAsync(720 * 60_000); + + // Only ONE renewal should have fired (the latest interval). + expect(mocks.runCommandWithTimeout).toHaveBeenCalledTimes(3); + } finally { + vi.useRealTimers(); + } + }); + + it("escalates to SIGKILL and resolves on final timeout when process ignores signals", async () => { + vi.useFakeTimers(); + try { + mocks.runCommandWithTimeout.mockResolvedValue({ code: 0, stdout: "", stderr: "" }); + + // Spawn a process that never emits exit/close/error + const stubbornChild = new EventEmitter(); + const killCalls: string[] = []; + Object.assign(stubbornChild, { + kill: vi.fn((sig: string) => { + killCalls.push(sig); + return true; + }), + killed: false, + }); + mocks.spawn.mockReturnValueOnce(stubbornChild); + + await startGmailWatcher(createGmailConfig()); + expect(mocks.spawn).toHaveBeenCalledTimes(1); + + // Now spawn a normal child for the second start so re-entry triggers settle + mocks.spawn.mockImplementation(() => { + const child = new EventEmitter(); + return Object.assign(child, { + kill: vi.fn(() => { + queueMicrotask(() => child.emit("exit", null, "SIGTERM")); + return true; + }), + killed: false, + }); + }); + + // Re-entry starts settle on stubbornChild + const startPromise = startGmailWatcher(createGmailConfig()); + + // After 3s the escalation fires SIGKILL + await vi.advanceTimersByTimeAsync(3_000); + expect(killCalls).toContain("SIGTERM"); + expect(killCalls).toContain("SIGKILL"); + + // After 8s total the final timeout resolves even though exit never fired + await vi.advanceTimersByTimeAsync(5_000); + await expect(startPromise).resolves.toEqual({ started: true }); + } finally { + vi.useRealTimers(); + } + }); + + it("cancels stale respawn timeout when re-entry happens during 5s window", async () => { + vi.useFakeTimers(); + try { + mocks.runCommandWithTimeout.mockResolvedValue({ code: 0, stdout: "", stderr: "" }); + const spawnedChildren: Array }> = []; + mocks.spawn.mockImplementation(() => { + const child = new EventEmitter(); + const mockedChild = Object.assign(child, { + kill: vi.fn(() => { + queueMicrotask(() => child.emit("exit", null, "SIGTERM")); + return true; + }), + }); + spawnedChildren.push(mockedChild); + return mockedChild; + }); + + // First start + await startGmailWatcher(createGmailConfig()); + expect(spawnedChildren).toHaveLength(1); + + // Process crashes (exit code 1). This queues a 5s respawn timeout. + spawnedChildren[0].emit("exit", 1, null); + + // Before the 5s timer fires, a config reload triggers re-entry. + // The re-entry guard should cancel the stale respawn timeout. + await startGmailWatcher(createGmailConfig()); + expect(spawnedChildren).toHaveLength(2); + + // Advance past the 5s respawn window. If the stale timeout was NOT + // cancelled, it would spawn a 3rd process (duplicate). + await vi.advanceTimersByTimeAsync(6000); + expect(spawnedChildren).toHaveLength(2); // No duplicate spawned + } finally { + vi.useRealTimers(); + } + }); }); diff --git a/src/hooks/gmail-watcher.ts b/src/hooks/gmail-watcher.ts index c46d3a3d8fc..66d9d382a3c 100644 --- a/src/hooks/gmail-watcher.ts +++ b/src/hooks/gmail-watcher.ts @@ -28,6 +28,7 @@ let watcherProcess: ChildProcess | null = null; let renewInterval: ReturnType | null = null; let shuttingDown = false; let currentConfig: GmailHookRuntimeConfig | null = null; +let respawnTimeout: ReturnType | null = null; /** * Check if gog binary is available @@ -114,7 +115,8 @@ function spawnGogServe(cfg: GmailHookRuntimeConfig): ChildProcess { } log.warn(`gog exited (code=${code}, signal=${signal}); restarting in 5s`); watcherProcess = null; - setTimeout(() => { + respawnTimeout = setTimeout(() => { + respawnTimeout = null; if (shuttingDown || !currentConfig) { return; } @@ -125,6 +127,46 @@ function spawnGogServe(cfg: GmailHookRuntimeConfig): ChildProcess { return child; } +/** + * Send SIGTERM, escalate to SIGKILL after 3 s, and resolve on exit/close/error + * or a final 5 s timeout after SIGKILL so the caller never hangs. + */ +function settleProcess(proc: ChildProcess): Promise { + return new Promise((resolve) => { + let settled = false; + const settle = () => { + if (settled) { + return; + } + settled = true; + clearTimeout(escalation); + clearTimeout(finalTimeout); + resolve(); + }; + + proc.on("exit", settle); + proc.on("close", settle); + proc.on("error", settle); + + proc.kill("SIGTERM"); + + const escalation = setTimeout(() => { + try { + proc.kill("SIGKILL"); + } catch { + // already dead + } + }, 3_000); + + const finalTimeout = setTimeout(() => { + if (!settled) { + log.warn("gog process did not exit after SIGKILL; giving up"); + settle(); + } + }, 8_000); + }); +} + export type GmailWatcherStartResult = { started: boolean; reason?: string; @@ -235,6 +277,28 @@ export async function startGmailWatcher( } currentConfig = runtimeConfig; + // Stop any existing watcher before doing async setup so a re-entry + // does not orphan the old serve process or leave a dangling timer. + // This must run before Tailscale/watch-start to prevent the old + // process from exiting and queuing a respawn during async work. + if (watcherProcess || renewInterval || respawnTimeout) { + shuttingDown = true; + if (respawnTimeout) { + clearTimeout(respawnTimeout); + respawnTimeout = null; + } + if (renewInterval) { + clearInterval(renewInterval); + renewInterval = null; + } + if (watcherProcess) { + const oldProcess = watcherProcess; + watcherProcess = null; + await settleProcess(oldProcess); + } + shuttingDown = false; + } + // Set up Tailscale endpoint if needed if (runtimeConfig.tailscale.mode !== "off") { const cancellation = createGmailWatcherCancellation(options); @@ -283,8 +347,6 @@ export async function startGmailWatcher( } shuttingDown = false; watcherProcess = spawnGogServe(runtimeConfig); - - // Set up renewal interval const renewMs = runtimeConfig.renewEveryMinutes * 60_000; renewInterval = setInterval(() => { if (shuttingDown) { @@ -306,6 +368,10 @@ export async function startGmailWatcher( export async function stopGmailWatcher(): Promise { shuttingDown = true; + if (respawnTimeout) { + clearTimeout(respawnTimeout); + respawnTimeout = null; + } if (renewInterval) { clearInterval(renewInterval); renewInterval = null; @@ -313,24 +379,9 @@ export async function stopGmailWatcher(): Promise { if (watcherProcess) { log.info("stopping gmail watcher"); - watcherProcess.kill("SIGTERM"); - - // Wait a bit for graceful shutdown - await new Promise((resolve) => { - const timeout = setTimeout(() => { - if (watcherProcess) { - watcherProcess.kill("SIGKILL"); - } - resolve(); - }, 3000); - - watcherProcess?.on("exit", () => { - clearTimeout(timeout); - resolve(); - }); - }); - + const proc = watcherProcess; watcherProcess = null; + await settleProcess(proc); } currentConfig = null;