From ff72faba28d10c86611863d0aaa5d3122f2d8cb0 Mon Sep 17 00:00:00 2001 From: David Brailovsky Date: Fri, 25 Sep 2026 16:27:23 +0200 Subject: [PATCH] fix(coding-agent): save new session file at the first user message A new session was only written to disk once the first assistant message existed. If pi exited during the first turn (Ctrl+C twice while streaming, or the process was killed), the user's prompt was lost and the session never appeared in /resume. The file is now created as soon as the session has a user or assistant message. A session with only setup entries (model, thinking level) still stays in memory, so starting pi and quitting without chatting leaves no file. Forking uses the same rule through a shared helper, so the two paths cannot drift apart and write the header twice (#1672). closes #10000 --- packages/coding-agent/CHANGELOG.md | 1 + .../src/core/agent-session-runtime.ts | 4 +- .../coding-agent/src/core/session-manager.ts | 34 +++++------ .../session-manager/file-operations.test.ts | 54 +++++++++++++++- .../session-manager/tree-traversal.test.ts | 61 +++++++------------ .../test/suite/agent-session-runtime.test.ts | 2 +- packages/coding-agent/test/utilities.ts | 14 +++++ 7 files changed, 107 insertions(+), 63 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 04810c1c8..e4f848e93 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -22,6 +22,7 @@ - Fixed pinned git extensions loaded with `-e` continuing to use the first downloaded commit after the ref changes ([#9982](https://github.com/earendil-works/pi/issues/9982)). - Fixed `RpcClient` skipping the next event listener when a listener unsubscribes while handling an event, which could make `waitForIdle()` time out after `collectEvents()` ([#9990](https://github.com/earendil-works/pi/issues/9990)). - Fixed full-file `read` calls rendering as `:1` when models send `null` for omitted `offset` and `limit` ([#9996](https://github.com/earendil-works/pi/issues/9996)). +- Fixed new sessions being lost when pi exits before the first assistant response. The session file is now created when the first user message is sent ([#10000](https://github.com/earendil-works/pi/issues/10000)). ## [0.87.1] - 2026-09-22 diff --git a/packages/coding-agent/src/core/agent-session-runtime.ts b/packages/coding-agent/src/core/agent-session-runtime.ts index 851258b45..c1d7655dc 100644 --- a/packages/coding-agent/src/core/agent-session-runtime.ts +++ b/packages/coding-agent/src/core/agent-session-runtime.ts @@ -310,9 +310,7 @@ export class AgentSessionRuntime { } if (!existsSync(currentSessionFile)) { - throw new Error( - "This session has not been saved yet. Wait for the first assistant response before cloning or forking it.", - ); + throw new Error("This session has not been saved yet. Send a message before cloning or forking it."); } const sessionManager = SessionManager.open(currentSessionFile, sessionDir); const forkedSessionPath = sessionManager.createBranchedSession(targetLeafId); diff --git a/packages/coding-agent/src/core/session-manager.ts b/packages/coding-agent/src/core/session-manager.ts index fd87a2d88..6ef060fa9 100644 --- a/packages/coding-agent/src/core/session-manager.ts +++ b/packages/coding-agent/src/core/session-manager.ts @@ -1157,21 +1157,23 @@ export class SessionManager { return this.sessionFile; } + /** + * A new session file is created only once the session contains a user or assistant message. + * Setup entries alone (model, thinking level, system prompt) stay in memory so opening and + * closing pi without chatting leaves no file behind. Starting at the user message (not the + * first assistant reply) keeps the prompt on disk if the first turn never completes (#10000). + */ + private _hasConversation(): boolean { + return this.fileEntries.some( + (e) => e.type === "message" && (e.message.role === "user" || e.message.role === "assistant"), + ); + } + _persist(entry: SessionEntry): void { if (!this.persist || !this.sessionFile) return; - const hasAssistant = this.fileEntries.some((e) => e.type === "message" && e.message.role === "assistant"); - if (!hasAssistant) { - if (this.flushed) { - appendFileSync(this.sessionFile, `${JSON.stringify(entry)}\n`); - } else { - // Mark as not flushed so when assistant arrives, all entries get written - this.flushed = false; - } - return; - } - if (!this.flushed) { + if (!this._hasConversation()) return; const fd = openSync(this.sessionFile, "wx"); try { for (const e of this.fileEntries) { @@ -1707,13 +1709,9 @@ export class SessionManager { this.sessionFile = newSessionFile; this._buildIndex(); - // Only write the file now if it contains an assistant message. - // Otherwise defer to _persist(), which creates the file on the - // first assistant response, matching the newSession() contract - // and avoiding the duplicate-header bug when _persist()'s - // no-assistant guard later resets flushed to false. - const hasAssistant = this.fileEntries.some((e) => e.type === "message" && e.message.role === "assistant"); - if (hasAssistant) { + // Use the same rule as _persist(): write now if the branched path already + // has a conversation, otherwise let _persist() create the file later. + if (this._hasConversation()) { this._rewriteFile(); this.flushed = true; } else { diff --git a/packages/coding-agent/test/session-manager/file-operations.test.ts b/packages/coding-agent/test/session-manager/file-operations.test.ts index 5b201488b..8353ccdb7 100644 --- a/packages/coding-agent/test/session-manager/file-operations.test.ts +++ b/packages/coding-agent/test/session-manager/file-operations.test.ts @@ -1,9 +1,21 @@ import { constants as bufferConstants } from "buffer"; -import { appendFileSync, closeSync, mkdirSync, openSync, readFileSync, rmSync, writeFileSync, writeSync } from "fs"; +import { + appendFileSync, + closeSync, + existsSync, + mkdirSync, + mkdtempSync, + openSync, + readFileSync, + rmSync, + writeFileSync, + writeSync, +} from "fs"; import { tmpdir } from "os"; import { join } from "path"; import { afterEach, beforeEach, describe, expect, it } from "vitest"; import { findMostRecentSession, loadEntriesFromFile, SessionManager } from "../../src/core/session-manager.ts"; +import { assistantMsg, readSessionFileRoles, userMsg } from "../utilities.ts"; const HEADER_SCAN_LIMIT_BYTES = 1024 * 1024; @@ -420,3 +432,43 @@ describe("SessionManager.setSessionFile with corrupted files", () => { expect(sm2.getHeader()?.type).toBe("session"); }); }); + +describe("SessionManager session file creation", () => { + let tempDir: string; + + beforeEach(() => { + tempDir = mkdtempSync(join(tmpdir(), "pi-session-persist-")); + }); + + afterEach(() => { + rmSync(tempDir, { recursive: true, force: true }); + }); + + it("does not create a file for a session with only setup entries", () => { + const session = SessionManager.create(tempDir, tempDir); + session.appendModelChange("anthropic", "claude-sonnet-4-5"); + session.appendThinkingLevelChange("off"); + + expect(existsSync(session.getSessionFile()!)).toBe(false); + }); + + // #10000: the first prompt must survive a first turn that never produces an assistant message + it("creates the file when the first user message is appended", () => { + const session = SessionManager.create(tempDir, tempDir); + session.appendModelChange("anthropic", "claude-sonnet-4-5"); + session.appendMessage(userMsg("first question")); + + const file = session.getSessionFile()!; + expect(readSessionFileRoles(file)).toEqual(["session", "model_change", "user"]); + expect(SessionManager.open(file, tempDir).buildSessionContext().messages).toHaveLength(1); + }); + + it("appends later entries to the file without rewriting earlier ones", () => { + const session = SessionManager.create(tempDir, tempDir); + session.appendMessage(userMsg("first question")); + session.appendCustomEntry("preset-state", { name: "plan" }); + session.appendMessage(assistantMsg("first answer")); + + expect(readSessionFileRoles(session.getSessionFile()!)).toEqual(["session", "user", "custom", "assistant"]); + }); +}); diff --git a/packages/coding-agent/test/session-manager/tree-traversal.test.ts b/packages/coding-agent/test/session-manager/tree-traversal.test.ts index 123dc65cd..50980f700 100644 --- a/packages/coding-agent/test/session-manager/tree-traversal.test.ts +++ b/packages/coding-agent/test/session-manager/tree-traversal.test.ts @@ -1,9 +1,9 @@ -import { existsSync, mkdirSync, readFileSync, rmSync } from "fs"; +import { existsSync, mkdirSync, rmSync } from "fs"; import { tmpdir } from "os"; import { join } from "path"; import { describe, expect, it } from "vitest"; import { type CustomEntry, SessionManager } from "../../src/core/session-manager.ts"; -import { assistantMsg, userMsg } from "../utilities.ts"; +import { assistantMsg, readSessionFileRoles, userMsg } from "../utilities.ts"; describe("SessionManager append and tree traversal", () => { describe("append operations", () => { @@ -480,45 +480,33 @@ describe("createBranchedSession", () => { expect(entries.map((e) => e.id)).toEqual([id1, id2, id4, id5]); }); - it("does not duplicate entries when forking from first user message", () => { + it("does not duplicate entries when forking from before the first user message", () => { const tempDir = join(tmpdir(), `session-fork-dedup-${Date.now()}`); mkdirSync(tempDir, { recursive: true }); try { // Create a persisted session with a couple of turns const session = SessionManager.create(tempDir, tempDir); - const id1 = session.appendMessage(userMsg("first question")); + const modelChangeId = session.appendModelChange("anthropic", "claude-sonnet-4-5"); + session.appendMessage(userMsg("first question")); session.appendMessage(assistantMsg("first answer")); - session.appendMessage(userMsg("second question")); - session.appendMessage(assistantMsg("second answer")); - // Fork from the very first user message (no assistant in the branched path) - const newFile = session.createBranchedSession(id1); + // Fork from a setup entry (no user or assistant message in the branched path) + const newFile = session.createBranchedSession(modelChangeId); expect(newFile).toBeDefined(); - // The branched path has no assistant, so the file should not exist yet - // (deferred to _persist on first assistant, matching newSession() contract) + // Nothing to save yet, so the file is created later by the first user message expect(existsSync(newFile!)).toBe(false); + session.appendMessage(userMsg("new question")); + expect(existsSync(newFile!)).toBe(true); + // Simulate extension adding entry before assistant (like preset on turn_start) session.appendCustomEntry("preset-state", { name: "plan" }); - - // Now the assistant responds session.appendMessage(assistantMsg("new answer")); - // File should now exist with exactly one header and no duplicate IDs - expect(existsSync(newFile!)).toBe(true); - const content = readFileSync(newFile!, "utf-8"); - const lines = content.trim().split("\n").filter(Boolean); - const records = lines.map((line) => JSON.parse(line)); - - expect(records.filter((r) => r.type === "session")).toHaveLength(1); - - const entryIds = records - .filter((r) => r.type !== "session") - .map((r) => r.id) - .filter((id): id is string => typeof id === "string"); - expect(new Set(entryIds).size).toBe(entryIds.length); + // Exactly one header and each entry written once + expect(readSessionFileRoles(newFile!)).toEqual(["session", "model_change", "user", "custom", "assistant"]); } finally { rmSync(tempDir, { recursive: true, force: true }); } @@ -570,27 +558,20 @@ describe("createBranchedSession", () => { } }); - it("writes file immediately when forking from a point with assistant messages", () => { - const tempDir = join(tmpdir(), `session-fork-with-assistant-${Date.now()}`); + it("writes file immediately when forking at a user message", () => { + const tempDir = join(tmpdir(), `session-fork-with-user-${Date.now()}`); mkdirSync(tempDir, { recursive: true }); try { const session = SessionManager.create(tempDir, tempDir); - session.appendMessage(userMsg("first question")); - const id2 = session.appendMessage(assistantMsg("first answer")); - session.appendMessage(userMsg("second question")); - session.appendMessage(assistantMsg("second answer")); + const id1 = session.appendMessage(userMsg("first question")); + session.appendMessage(assistantMsg("first answer")); - // Fork including the assistant message - const newFile = session.createBranchedSession(id2); - expect(newFile).toBeDefined(); - - // Path includes an assistant, so file should be written immediately + const newFile = session.createBranchedSession(id1); expect(existsSync(newFile!)).toBe(true); - const content = readFileSync(newFile!, "utf-8"); - const lines = content.trim().split("\n").filter(Boolean); - const records = lines.map((line) => JSON.parse(line)); - expect(records.filter((r) => r.type === "session")).toHaveLength(1); + + session.appendMessage(assistantMsg("new answer")); + expect(readSessionFileRoles(newFile!)).toEqual(["session", "user", "assistant"]); } finally { rmSync(tempDir, { recursive: true, force: true }); } diff --git a/packages/coding-agent/test/suite/agent-session-runtime.test.ts b/packages/coding-agent/test/suite/agent-session-runtime.test.ts index 921e5a9d2..93d40fc56 100644 --- a/packages/coding-agent/test/suite/agent-session-runtime.test.ts +++ b/packages/coding-agent/test/suite/agent-session-runtime.test.ts @@ -384,7 +384,7 @@ describe("AgentSessionRuntime characterization", () => { expect(leafId).toBeTruthy(); await expect(runtime.fork(leafId!, { position: "at" })).rejects.toThrow( - "This session has not been saved yet. Wait for the first assistant response before cloning or forking it.", + "This session has not been saved yet. Send a message before cloning or forking it.", ); }); diff --git a/packages/coding-agent/test/utilities.ts b/packages/coding-agent/test/utilities.ts index 41641db7c..3839ab847 100644 --- a/packages/coding-agent/test/utilities.ts +++ b/packages/coding-agent/test/utilities.ts @@ -152,6 +152,20 @@ export function assistantMsg(text: string) { }; } +/** + * Read a session JSONL file and return one label per record: the message role for + * message entries, otherwise the entry type (e.g. "session", "model_change"). + */ +export function readSessionFileRoles(file: string): string[] { + return readFileSync(file, "utf-8") + .trim() + .split("\n") + .map((line) => { + const record = JSON.parse(line); + return record.message?.role ?? record.type; + }); +} + /** * Options for creating a test session. */