From 26443eefacce86a635fd3fc2dc7e7512e090ab05 Mon Sep 17 00:00:00 2001 From: can1357 Date: Tue, 23 Jun 2026 05:15:08 +0200 Subject: [PATCH] fix(coding-agent): terminated process on cancelled startup resume picker - Force process exit when the startup session picker is cancelled instead of returning. - Prevent hanging the event loop caused by long-lived startup handles such as theme listeners and timers. - Add regression test case to verify clean process termination upon picker cancellation. --- packages/coding-agent/src/main.ts | 19 ++++- .../test/main-resume-cancel-exit.test.ts | 84 +++++++++++++++++++ 2 files changed, 99 insertions(+), 4 deletions(-) create mode 100644 packages/coding-agent/test/main-resume-cancel-exit.test.ts diff --git a/packages/coding-agent/src/main.ts b/packages/coding-agent/src/main.ts index e8a09f817..a9d9b893b 100644 --- a/packages/coding-agent/src/main.ts +++ b/packages/coding-agent/src/main.ts @@ -945,6 +945,7 @@ async function buildSessionOptions( interface RunRootCommandDependencies { createAgentSession?: typeof createAgentSession; discoverAuthStorage?: typeof discoverAuthStorage; + selectSession?: typeof selectSession; runAcpMode?: RunAcpMode; settings?: Settings; forceSetupWizard?: boolean; @@ -1131,7 +1132,8 @@ export async function runRootCommand( // (see issue #1668). if (typeof parsedArgs.resume === "string" && !sessionManager) { writeStartupNotice(parsedArgs, `${chalk.dim("Resume cancelled: session is in another project.")}\n`); - return; + stopStartupWatchdog(); + process.exit(0); } // Handle --resume (no value): show session picker @@ -1147,17 +1149,26 @@ export async function runRootCommand( preloadedAllSessions = await logger.time("SessionManager.listAll", SessionManager.listAll); if (preloadedAllSessions.length === 0) { writeStartupNotice(parsedArgs, `${chalk.dim("No sessions found")}\n`); - return; + stopStartupWatchdog(); + process.exit(0); } } pauseStartupWatchdog(); - const selected = await logger.time("selectSession", selectSession, folderSessions, { + const selected = await logger.time("selectSession", deps.selectSession ?? selectSession, folderSessions, { allSessions: preloadedAllSessions, }); resumeStartupWatchdog(); if (!selected) { writeStartupNotice(parsedArgs, `${chalk.dim("No session selected")}\n`); - return; + // Quit instead of returning: startup already armed long-lived handles + // (theme watcher + SIGWINCH/macOS appearance listeners via initTheme, + // settings save timer, model registry) that keep the event loop alive, + // so a bare return hangs the process after the picker leaves the alt + // screen. No session was built here, so there is nothing to flush. The + // in-session `/resume` picker (selector-controller.ts) takes a different + // onCancel that just closes the overlay — only this startup path exits. + stopStartupWatchdog(); + process.exit(0); } // Resuming a session from another project: switch the process into that // project's directory and refresh cwd-derived caches before the session is diff --git a/packages/coding-agent/test/main-resume-cancel-exit.test.ts b/packages/coding-agent/test/main-resume-cancel-exit.test.ts new file mode 100644 index 000000000..c1884f81f --- /dev/null +++ b/packages/coding-agent/test/main-resume-cancel-exit.test.ts @@ -0,0 +1,84 @@ +/** + * Regression: cancelling the startup `--resume` session picker (e.g. pressing + * Esc) must terminate the process cleanly. Startup arms long-lived handles + * (theme/appearance listeners via initTheme, settings save timer, model + * registry), so the previous bare `return` left the event loop with live + * handles and the process hung after the picker left the alternate screen. + * + * The fix exits via `process.exit(0)` — matching the `--version`/`--export` + * early-exit convention in the same function. Only this startup call site + * exits; the in-session `/resume` picker (selector-controller.ts) keeps its own + * onCancel that just closes the overlay. + */ +import { describe, expect, it, vi } from "bun:test"; +import * as path from "node:path"; +import { parseArgs } from "@oh-my-pi/pi-coding-agent/cli/args"; +import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; +import { runRootCommand } from "@oh-my-pi/pi-coding-agent/main"; +import { AuthStorage } from "@oh-my-pi/pi-coding-agent/session/auth-storage"; +import { TempDir } from "@oh-my-pi/pi-utils"; + +class ProcessExitSignal extends Error { + constructor(readonly code: number) { + super(`process.exit(${code})`); + this.name = "ProcessExitSignal"; + } +} + +describe("runRootCommand — startup --resume picker cancellation", () => { + it("exits cleanly (process.exit 0) when the picker is cancelled instead of returning and hanging", async () => { + using tempDir = TempDir.createSync("@omp-resume-cancel-"); + const sessionDir = tempDir.path(); + // One valid session so folderSessions is non-empty and the picker (not the + // "No sessions found" probe) is the path under test. + await Bun.write( + path.join(sessionDir, "existing.jsonl"), + `${JSON.stringify({ type: "session", id: "existing-session", cwd: sessionDir, timestamp: new Date().toISOString() })}\n`, + ); + + const authStorage = await AuthStorage.create(path.join(sessionDir, "auth.db")); + const settings = Settings.isolated({ "marketplace.autoUpdate": "off" }); + + // --print keeps initTheme non-interactive so no global appearance/SIGWINCH + // listeners leak into the rest of the suite; the picker branch is gated on + // `resume === true`, not on interactivity, so it still runs. + const parsed = parseArgs(["--resume", "--print"]); + parsed.noExtensions = true; + parsed.noSkills = true; + parsed.noRules = true; + parsed.noTools = true; + parsed.noLsp = true; + parsed.sessionDir = sessionDir; + + const exitCodes: number[] = []; + vi.spyOn(process, "exit").mockImplementation(((code?: number) => { + exitCodes.push(code ?? 0); + throw new ProcessExitSignal(code ?? 0); + }) as typeof process.exit); + vi.spyOn(process.stdout, "write").mockImplementation(() => true); + + let pickerCalled = false; + let thrown: unknown; + try { + await runRootCommand(parsed, ["--resume", "--print"], { + discoverAuthStorage: async () => authStorage, + settings, + selectSession: async () => { + pickerCalled = true; + return null; // user cancelled (Esc) + }, + }); + } catch (err) { + thrown = err; + } finally { + vi.restoreAllMocks(); + authStorage.close(); + } + + expect(pickerCalled).toBe(true); + expect(thrown).toBeInstanceOf(ProcessExitSignal); + // Exactly one clean exit — proves the cancel branch terminates instead of + // falling through to session creation or returning into a hang. + expect(exitCodes).toEqual([0]); + }, 15_000); +});