fix(coding-agent): reject invalid session files

closes #6002
This commit is contained in:
Mario Zechner
2026-06-25 15:16:18 +02:00
parent f14b3594c1
commit 543710f643
5 changed files with 106 additions and 28 deletions
+1
View File
@@ -4,6 +4,7 @@
### Fixed
- Fixed `--session` and `SessionManager.open()` to reject non-empty invalid session files without overwriting them ([#6002](https://github.com/earendil-works/pi/issues/6002)).
- Fixed assistant messages stopped by output length to show a visible incomplete-response error ([#4290](https://github.com/earendil-works/pi/issues/4290)).
- Fixed `--no-session --session-id` so ephemeral CLI runs can use deterministic session IDs for provider cache affinity ([#6070](https://github.com/earendil-works/pi/issues/6070)).
- Fixed disk BMP image files to be detected, converted to PNG, and attached through `read` and CLI `@file` inputs ([#6047](https://github.com/earendil-works/pi/issues/6047)).
@@ -795,10 +795,13 @@ export class SessionManager {
if (existsSync(this.sessionFile)) {
this.fileEntries = loadEntriesFromFile(this.sessionFile);
// If file was empty or corrupted (no valid header), truncate and start fresh
// to avoid appending messages without a session header (which breaks the session)
// If file was empty, initialize it with a valid session header. If it was
// non-empty but did not parse as a pi session, fail without modifying it.
if (this.fileEntries.length === 0) {
const explicitPath = this.sessionFile;
if (statSync(explicitPath).size > 0) {
throw new Error(`Session file is not a valid pi session and was not modified: ${explicitPath}`);
}
this.newSession();
this.sessionFile = explicitPath;
this._rewriteFile();
+11 -1
View File
@@ -241,6 +241,16 @@ function validateSessionIdFlags(parsed: Args): void {
}
}
function openSessionOrExit(path: string, sessionDir?: string): SessionManager {
try {
return SessionManager.open(path, sessionDir);
} catch (error: unknown) {
const message = error instanceof Error ? error.message : String(error);
console.error(chalk.red(`Error: ${message}`));
process.exit(1);
}
}
function forkSessionOrExit(sourcePath: string, cwd: string, sessionDir?: string, sessionId?: string): SessionManager {
try {
return SessionManager.forkFrom(sourcePath, cwd, sessionDir, { id: sessionId });
@@ -290,7 +300,7 @@ async function createSessionManager(
switch (resolved.type) {
case "path":
case "local":
return SessionManager.open(resolved.path, sessionDir);
return openSessionOrExit(resolved.path, sessionDir);
case "global": {
console.log(chalk.yellow(`Session found in different project: ${resolved.cwd}`));
@@ -0,0 +1,67 @@
import { spawn } from "node:child_process";
import { mkdirSync, mkdtempSync, readFileSync, realpathSync, rmSync, writeFileSync } from "node:fs";
import { tmpdir } from "node:os";
import { join, resolve } from "node:path";
import { afterEach, describe, expect, it } from "vitest";
import { ENV_AGENT_DIR } from "../src/config.ts";
const cliPath = resolve(__dirname, "../src/cli.ts");
const tempDirs: string[] = [];
afterEach(() => {
for (const dir of tempDirs.splice(0)) {
rmSync(dir, { recursive: true, force: true });
}
});
function createTempDir(): string {
const dir = realpathSync(mkdtempSync(join(tmpdir(), "pi-session-file-invalid-")));
tempDirs.push(dir);
return dir;
}
async function runCli(args: string[], cwd: string, agentDir: string): Promise<{ code: number | null; stderr: string }> {
let stderr = "";
const code = await new Promise<number | null>((resolvePromise, reject) => {
const child = spawn(process.execPath, [cliPath, ...args], {
cwd,
env: {
...process.env,
[ENV_AGENT_DIR]: agentDir,
PI_OFFLINE: "1",
TSX_TSCONFIG_PATH: resolve(__dirname, "../../../tsconfig.json"),
},
stdio: ["ignore", "ignore", "pipe"],
});
child.stderr.on("data", (chunk) => {
stderr += chunk.toString();
});
child.on("error", reject);
child.on("close", resolvePromise);
});
return { code, stderr };
}
describe("--session invalid file handling", () => {
it("prints a friendly error and preserves non-session file content", async () => {
const tempRoot = createTempDir();
const agentDir = join(tempRoot, "agent");
const projectDir = join(tempRoot, "project");
const sessionFile = join(tempRoot, "not-a-session.log");
const originalContent = '{"type":"event","data":"not a session"}\n';
mkdirSync(agentDir, { recursive: true });
mkdirSync(projectDir, { recursive: true });
writeFileSync(sessionFile, originalContent);
const result = await runCli(["--session", sessionFile, "-p", "hi"], projectDir, agentDir);
expect(result.code).toBe(1);
expect(result.stderr).toContain(
`Error: Session file is not a valid pi session and was not modified: ${sessionFile}`,
);
expect(result.stderr).not.toContain("SessionManager.open");
expect(result.stderr).not.toContain("at ");
expect(readFileSync(sessionFile, "utf8")).toBe(originalContent);
});
});
@@ -268,28 +268,27 @@ describe("SessionManager.setSessionFile with corrupted files", () => {
expect(header.id).toBe(sm.getSessionId());
});
it("truncates and rewrites file without valid header", () => {
it("throws and preserves non-empty file without valid header", () => {
const noHeaderFile = join(tempDir, "no-header.jsonl");
// File with messages but no session header (corrupted state)
writeFileSync(
noHeaderFile,
'{"type":"message","id":"abc","parentId":"orphaned","timestamp":"2025-01-01T00:00:00Z","message":{"role":"assistant","content":"test"}}\n',
const originalContent =
'{"type":"message","id":"abc","parentId":"orphaned","timestamp":"2025-01-01T00:00:00Z","message":{"role":"assistant","content":"test"}}\n';
writeFileSync(noHeaderFile, originalContent);
expect(() => SessionManager.open(noHeaderFile, tempDir)).toThrow(
`Session file is not a valid pi session and was not modified: ${noHeaderFile}`,
);
expect(readFileSync(noHeaderFile, "utf-8")).toBe(originalContent);
});
const sm = SessionManager.open(noHeaderFile, tempDir);
it("throws and preserves non-session JSONL files", () => {
const nonSessionFile = join(tempDir, "not-a-session.log");
const originalContent = '{"type":"event","data":"not a session"}\n';
writeFileSync(nonSessionFile, originalContent);
// Should have created a new session with valid header
expect(sm.getSessionId()).toBeTruthy();
expect(sm.getHeader()).toBeTruthy();
expect(sm.getHeader()?.type).toBe("session");
// File should now contain only a valid header (old content truncated)
const content = readFileSync(noHeaderFile, "utf-8");
const lines = content.trim().split("\n").filter(Boolean);
expect(lines.length).toBe(1);
const header = JSON.parse(lines[0]);
expect(header.type).toBe("session");
expect(header.id).toBe(sm.getSessionId());
expect(() => SessionManager.open(nonSessionFile, tempDir)).toThrow(
`Session file is not a valid pi session and was not modified: ${nonSessionFile}`,
);
expect(readFileSync(nonSessionFile, "utf-8")).toBe(originalContent);
});
it("preserves explicit session file path when recovering from corrupted file", () => {
@@ -302,16 +301,14 @@ describe("SessionManager.setSessionFile with corrupted files", () => {
expect(sm.getSessionFile()).toBe(explicitPath);
});
it("subsequent loads of recovered file work correctly", () => {
const corruptedFile = join(tempDir, "corrupted.jsonl");
writeFileSync(corruptedFile, "garbage content\n");
it("subsequent loads of initialized empty file work correctly", () => {
const emptyFile = join(tempDir, "empty.jsonl");
writeFileSync(emptyFile, "");
// First open recovers the file
const sm1 = SessionManager.open(corruptedFile, tempDir);
const sm1 = SessionManager.open(emptyFile, tempDir);
const sessionId = sm1.getSessionId();
// Second open should load the recovered file successfully
const sm2 = SessionManager.open(corruptedFile, tempDir);
const sm2 = SessionManager.open(emptyFile, tempDir);
expect(sm2.getSessionId()).toBe(sessionId);
expect(sm2.getHeader()?.type).toBe("session");
});