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.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
Reference in New Issue
Block a user