feat(coding-agent): print resume hint on interactive exit
This commit is contained in:
@@ -47,6 +47,7 @@ import {
|
|||||||
TUI,
|
TUI,
|
||||||
visibleWidth,
|
visibleWidth,
|
||||||
} from "@earendil-works/pi-tui";
|
} from "@earendil-works/pi-tui";
|
||||||
|
import chalk from "chalk";
|
||||||
import { spawn, spawnSync } from "child_process";
|
import { spawn, spawnSync } from "child_process";
|
||||||
import {
|
import {
|
||||||
APP_NAME,
|
APP_NAME,
|
||||||
@@ -194,6 +195,28 @@ function isUnknownModel(model: Model<any> | undefined): boolean {
|
|||||||
return !!model && model.provider === "unknown" && model.id === "unknown" && model.api === "unknown";
|
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 {
|
function hasDefaultModelProvider(providerId: string): providerId is keyof typeof defaultModelPerProvider {
|
||||||
return providerId in defaultModelPerProvider;
|
return providerId in defaultModelPerProvider;
|
||||||
}
|
}
|
||||||
@@ -3276,6 +3299,12 @@ export class InteractiveMode {
|
|||||||
|
|
||||||
this.stop();
|
this.stop();
|
||||||
await this.runtimeHost.dispose();
|
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);
|
process.exit(0);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
135
packages/coding-agent/test/format-resume-command.test.ts
Normal file
135
packages/coding-agent/test/format-resume-command.test.ts
Normal file
@@ -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();
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -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 { 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";
|
import { InteractiveMode } from "../../../src/modes/interactive/interactive-mode.ts";
|
||||||
|
|
||||||
// Regression for https://github.com/earendil-works/pi/issues/5080
|
// Regression for https://github.com/earendil-works/pi/issues/5080
|
||||||
@@ -16,6 +22,7 @@ type ShutdownThis = {
|
|||||||
runtimeHost: { dispose: () => Promise<void> };
|
runtimeHost: { dispose: () => Promise<void> };
|
||||||
ui: { terminal: { drainInput: (ms: number) => Promise<void> } };
|
ui: { terminal: { drainInput: (ms: number) => Promise<void> } };
|
||||||
stop: () => void;
|
stop: () => void;
|
||||||
|
sessionManager: SessionManager;
|
||||||
};
|
};
|
||||||
|
|
||||||
type InteractiveModePrototypeWithShutdown = {
|
type InteractiveModePrototypeWithShutdown = {
|
||||||
@@ -23,10 +30,42 @@ type InteractiveModePrototypeWithShutdown = {
|
|||||||
};
|
};
|
||||||
|
|
||||||
const interactiveModePrototype = InteractiveMode.prototype as unknown;
|
const interactiveModePrototype = InteractiveMode.prototype as unknown;
|
||||||
|
const tempDirs: string[] = [];
|
||||||
|
const originalStdoutIsTTY = Object.getOwnPropertyDescriptor(process.stdout, "isTTY");
|
||||||
|
|
||||||
class ProcessExitError extends Error {}
|
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 {
|
return {
|
||||||
isShuttingDown: false,
|
isShuttingDown: false,
|
||||||
unregisterSignalHandlers: vi.fn(),
|
unregisterSignalHandlers: vi.fn(),
|
||||||
@@ -45,6 +84,7 @@ function createContext(order: string[]): ShutdownThis {
|
|||||||
stop: vi.fn(() => {
|
stop: vi.fn(() => {
|
||||||
order.push("stop");
|
order.push("stop");
|
||||||
}),
|
}),
|
||||||
|
sessionManager,
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -59,6 +99,10 @@ async function callShutdown(context: ShutdownThis, options?: { fromSignal?: bool
|
|||||||
describe("InteractiveMode.shutdown ordering (#5080)", () => {
|
describe("InteractiveMode.shutdown ordering (#5080)", () => {
|
||||||
afterEach(() => {
|
afterEach(() => {
|
||||||
vi.restoreAllMocks();
|
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 () => {
|
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"]);
|
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 () => {
|
test("re-entrant shutdown is a no-op", async () => {
|
||||||
vi.spyOn(process, "exit").mockImplementation((() => {
|
vi.spyOn(process, "exit").mockImplementation((() => {
|
||||||
throw new ProcessExitError();
|
throw new ProcessExitError();
|
||||||
|
|||||||
Reference in New Issue
Block a user