mirror of
https://github.com/badlogic/pi-mono.git
synced 2026-11-05 23:37:25 +00:00
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
This commit is contained in:
parent
507d7649e9
commit
ff72faba28
7 changed files with 107 additions and 63 deletions
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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 {
|
||||
|
|
|
|||
|
|
@ -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"]);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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 });
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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.",
|
||||
);
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
*/
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue