diff --git a/packages/coding-agent/src/modes/interactive/interactive-mode.ts b/packages/coding-agent/src/modes/interactive/interactive-mode.ts index 012e2f2af..901e12451 100644 --- a/packages/coding-agent/src/modes/interactive/interactive-mode.ts +++ b/packages/coding-agent/src/modes/interactive/interactive-mode.ts @@ -47,6 +47,7 @@ import { TUI, visibleWidth, } from "@earendil-works/pi-tui"; +import chalk from "chalk"; import { spawn, spawnSync } from "child_process"; import { APP_NAME, @@ -194,6 +195,28 @@ function isUnknownModel(model: Model | undefined): boolean { return !!model && model.provider === "unknown" && model.id === "unknown" && model.api === "unknown"; } +function quoteIfNeeded(value: string): string { + if (value.length > 0 && !/[^a-zA-Z0-9_\-./~:@]/.test(value)) { + return value; + } + return `'${value.replace(/'/g, `'\\''`)}'`; +} + +export function formatResumeCommand(sessionManager: SessionManager): string | undefined { + if (!process.stdout.isTTY) return undefined; + if (!sessionManager.isPersisted()) return undefined; + + const sessionFile = sessionManager.getSessionFile(); + if (!sessionFile || !fs.existsSync(sessionFile)) return undefined; + + const args = [APP_NAME]; + if (!sessionManager.usesDefaultSessionDir()) { + args.push("--session-dir", quoteIfNeeded(sessionManager.getSessionDir())); + } + args.push("--session", sessionManager.getSessionId()); + return args.join(" "); +} + function hasDefaultModelProvider(providerId: string): providerId is keyof typeof defaultModelPerProvider { return providerId in defaultModelPerProvider; } @@ -3276,6 +3299,12 @@ export class InteractiveMode { this.stop(); await this.runtimeHost.dispose(); + + const resumeCommand = formatResumeCommand(this.sessionManager); + if (resumeCommand) { + process.stdout.write(` ${chalk.dim("To resume this session:")} ${resumeCommand}\n`); + } + process.exit(0); } diff --git a/packages/coding-agent/test/format-resume-command.test.ts b/packages/coding-agent/test/format-resume-command.test.ts new file mode 100644 index 000000000..86c4475b5 --- /dev/null +++ b/packages/coding-agent/test/format-resume-command.test.ts @@ -0,0 +1,135 @@ +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, describe, expect, it } from "vitest"; +import { APP_NAME } from "../src/config.ts"; +import type { SessionManager } from "../src/core/session-manager.ts"; +import { formatResumeCommand } from "../src/modes/interactive/interactive-mode.ts"; + +const tempDirs: string[] = []; +const originalStdoutIsTTY = Object.getOwnPropertyDescriptor(process.stdout, "isTTY"); + +afterEach(() => { + if (originalStdoutIsTTY) { + Object.defineProperty(process.stdout, "isTTY", originalStdoutIsTTY); + } else { + Reflect.deleteProperty(process.stdout, "isTTY"); + } + + for (const dir of tempDirs.splice(0)) { + rmSync(dir, { recursive: true, force: true }); + } +}); + +function setStdoutIsTTY(value: boolean): void { + Object.defineProperty(process.stdout, "isTTY", { configurable: true, value }); +} + +function createTempFile(): string { + const dir = mkdtempSync(join(tmpdir(), "pi-format-resume-command-")); + tempDirs.push(dir); + const file = join(dir, "session.jsonl"); + writeFileSync(file, "\n"); + return file; +} + +function createSessionManager(options: { + persisted?: boolean; + sessionFile?: string; + sessionId?: string; + sessionDir?: string; + usesDefaultSessionDir?: boolean; +}): SessionManager { + return { + isPersisted: () => options.persisted ?? true, + getSessionFile: () => options.sessionFile, + getSessionId: () => options.sessionId ?? "0197f6e4-4cf9-7f44-a2d8-f8f7f49ee9d3", + getSessionDir: () => options.sessionDir ?? "/tmp/pi-sessions", + usesDefaultSessionDir: () => options.usesDefaultSessionDir ?? true, + } as unknown as SessionManager; +} + +describe("formatResumeCommand", () => { + it("returns a session resume command for default session dirs", () => { + setStdoutIsTTY(true); + const sessionFile = createTempFile(); + const sessionManager = createSessionManager({ sessionFile, sessionId: "test-session" }); + + expect(formatResumeCommand(sessionManager)).toBe(`${APP_NAME} --session test-session`); + }); + + it("includes unquoted safe session dirs for non-default session dirs", () => { + setStdoutIsTTY(true); + const sessionFile = createTempFile(); + const sessionManager = createSessionManager({ + sessionFile, + sessionId: "test-session", + sessionDir: "/tmp/custom-pi-sessions", + usesDefaultSessionDir: false, + }); + + expect(formatResumeCommand(sessionManager)).toBe( + `${APP_NAME} --session-dir /tmp/custom-pi-sessions --session test-session`, + ); + }); + + it("quotes session dirs containing spaces", () => { + setStdoutIsTTY(true); + const sessionFile = createTempFile(); + const sessionManager = createSessionManager({ + sessionFile, + sessionId: "test-session", + sessionDir: "/tmp/custom pi sessions", + usesDefaultSessionDir: false, + }); + + expect(formatResumeCommand(sessionManager)).toBe( + `${APP_NAME} --session-dir '/tmp/custom pi sessions' --session test-session`, + ); + }); + + it("quotes session dirs containing single quotes", () => { + setStdoutIsTTY(true); + const sessionFile = createTempFile(); + const sessionManager = createSessionManager({ + sessionFile, + sessionId: "test-session", + sessionDir: "/tmp/custom pi's sessions", + usesDefaultSessionDir: false, + }); + + expect(formatResumeCommand(sessionManager)).toBe( + `${APP_NAME} --session-dir '/tmp/custom pi'\\''s sessions' --session test-session`, + ); + }); + + it("returns undefined when stdout is not a TTY", () => { + setStdoutIsTTY(false); + const sessionFile = createTempFile(); + const sessionManager = createSessionManager({ sessionFile }); + + expect(formatResumeCommand(sessionManager)).toBeUndefined(); + }); + + it("returns undefined for in-memory sessions", () => { + setStdoutIsTTY(true); + const sessionFile = createTempFile(); + const sessionManager = createSessionManager({ persisted: false, sessionFile }); + + expect(formatResumeCommand(sessionManager)).toBeUndefined(); + }); + + it("returns undefined when the session file is missing", () => { + setStdoutIsTTY(true); + const sessionManager = createSessionManager({ sessionFile: "/tmp/pi-missing-session.jsonl" }); + + expect(formatResumeCommand(sessionManager)).toBeUndefined(); + }); + + it("returns undefined when the session file is not set", () => { + setStdoutIsTTY(true); + const sessionManager = createSessionManager({ sessionFile: undefined }); + + expect(formatResumeCommand(sessionManager)).toBeUndefined(); + }); +}); diff --git a/packages/coding-agent/test/suite/regressions/5080-signal-shutdown-extension-cleanup.test.ts b/packages/coding-agent/test/suite/regressions/5080-signal-shutdown-extension-cleanup.test.ts index d8bcd8675..3f221f4b5 100644 --- a/packages/coding-agent/test/suite/regressions/5080-signal-shutdown-extension-cleanup.test.ts +++ b/packages/coding-agent/test/suite/regressions/5080-signal-shutdown-extension-cleanup.test.ts @@ -1,4 +1,10 @@ +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import chalk from "chalk"; import { afterEach, describe, expect, test, vi } from "vitest"; +import { APP_NAME } from "../../../src/config.ts"; +import type { SessionManager } from "../../../src/core/session-manager.ts"; import { InteractiveMode } from "../../../src/modes/interactive/interactive-mode.ts"; // Regression for https://github.com/earendil-works/pi/issues/5080 @@ -16,6 +22,7 @@ type ShutdownThis = { runtimeHost: { dispose: () => Promise }; ui: { terminal: { drainInput: (ms: number) => Promise } }; stop: () => void; + sessionManager: SessionManager; }; type InteractiveModePrototypeWithShutdown = { @@ -23,10 +30,42 @@ type InteractiveModePrototypeWithShutdown = { }; const interactiveModePrototype = InteractiveMode.prototype as unknown; +const tempDirs: string[] = []; +const originalStdoutIsTTY = Object.getOwnPropertyDescriptor(process.stdout, "isTTY"); class ProcessExitError extends Error {} -function createContext(order: string[]): ShutdownThis { +function createSessionManager(options: { sessionFile?: string } = {}): SessionManager { + return { + isPersisted: () => options.sessionFile !== undefined, + getSessionFile: () => options.sessionFile, + getSessionId: () => "test-session", + getSessionDir: () => "/tmp/pi-sessions", + usesDefaultSessionDir: () => true, + } as unknown as SessionManager; +} + +function createTempFile(): string { + const dir = mkdtempSync(join(tmpdir(), "pi-shutdown-resume-hint-")); + tempDirs.push(dir); + const file = join(dir, "session.jsonl"); + writeFileSync(file, "\n"); + return file; +} + +function setStdoutIsTTY(value: boolean): void { + Object.defineProperty(process.stdout, "isTTY", { configurable: true, value }); +} + +function restoreStdoutIsTTY(): void { + if (originalStdoutIsTTY) { + Object.defineProperty(process.stdout, "isTTY", originalStdoutIsTTY); + } else { + Reflect.deleteProperty(process.stdout, "isTTY"); + } +} + +function createContext(order: string[], sessionManager = createSessionManager()): ShutdownThis { return { isShuttingDown: false, unregisterSignalHandlers: vi.fn(), @@ -45,6 +84,7 @@ function createContext(order: string[]): ShutdownThis { stop: vi.fn(() => { order.push("stop"); }), + sessionManager, }; } @@ -59,6 +99,10 @@ async function callShutdown(context: ShutdownThis, options?: { fromSignal?: bool describe("InteractiveMode.shutdown ordering (#5080)", () => { afterEach(() => { vi.restoreAllMocks(); + restoreStdoutIsTTY(); + for (const dir of tempDirs.splice(0)) { + rmSync(dir, { recursive: true, force: true }); + } }); test("signal-triggered shutdown emits session_shutdown before terminal writes", async () => { @@ -87,6 +131,43 @@ describe("InteractiveMode.shutdown ordering (#5080)", () => { expect(order).toEqual(["drainInput", "stop", "dispose"]); }); + test("interactive quit prints a resume hint for persisted sessions", async () => { + vi.spyOn(process, "exit").mockImplementation((() => { + throw new ProcessExitError(); + }) as typeof process.exit); + const stdoutWrite = vi + .spyOn(process.stdout, "write") + .mockImplementation((() => true) as typeof process.stdout.write); + setStdoutIsTTY(true); + const order: string[] = []; + const context = createContext(order, createSessionManager({ sessionFile: createTempFile() })); + + await callShutdown(context); + + expect(order).toEqual(["drainInput", "stop", "dispose"]); + expect(stdoutWrite).toHaveBeenCalledWith( + ` ${chalk.dim("To resume this session:")} ${APP_NAME} --session test-session\n`, + ); + }); + + test("signal-triggered shutdown does not print a resume hint", async () => { + vi.spyOn(process, "exit").mockImplementation((() => { + throw new ProcessExitError(); + }) as typeof process.exit); + const stdoutWrite = vi + .spyOn(process.stdout, "write") + .mockImplementation((() => true) as typeof process.stdout.write); + setStdoutIsTTY(true); + const order: string[] = []; + const context = createContext(order, createSessionManager({ sessionFile: createTempFile() })); + + await callShutdown(context, { fromSignal: true }); + + for (const call of stdoutWrite.mock.calls) { + expect(call[0]).not.toContain("To resume this session:"); + } + }); + test("re-entrant shutdown is a no-op", async () => { vi.spyOn(process, "exit").mockImplementation((() => { throw new ProcessExitError();