diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 857f291b..5fe118e9 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -9,6 +9,7 @@ ### Fixed - `pi.registerProvider()` now takes effect immediately when called after the initial extension load phase (e.g. from a command handler). Previously the registration sat in a pending queue that was never flushed until the next `/reload` ([#1669](https://github.com/badlogic/pi-mono/pull/1669) by [@aliou](https://github.com/aliou)). +- Fixed duplicate session headers when forking from a point before any assistant message. `createBranchedSession` now defers file creation to `_persist()` when the branched path has no assistant message, matching the `newSession()` contract ([#1672](https://github.com/badlogic/pi-mono/pull/1672) by [@w-winter](https://github.com/w-winter)). ## [0.55.1] - 2026-02-26 diff --git a/packages/coding-agent/src/core/session-manager.ts b/packages/coding-agent/src/core/session-manager.ts index e28224ff..a7edb634 100644 --- a/packages/coding-agent/src/core/session-manager.ts +++ b/packages/coding-agent/src/core/session-manager.ts @@ -1187,11 +1187,7 @@ export class SessionManager { } if (this.persist) { - appendFileSync(newSessionFile, `${JSON.stringify(header)}\n`); - for (const entry of pathWithoutLabels) { - appendFileSync(newSessionFile, `${JSON.stringify(entry)}\n`); - } - // Write fresh label entries at the end + // Build label entries const lastEntryId = pathWithoutLabels[pathWithoutLabels.length - 1]?.id || null; let parentId = lastEntryId; const labelEntries: LabelEntry[] = []; @@ -1204,16 +1200,29 @@ export class SessionManager { targetId, label, }; - appendFileSync(newSessionFile, `${JSON.stringify(labelEntry)}\n`); pathEntryIds.add(labelEntry.id); labelEntries.push(labelEntry); parentId = labelEntry.id; } + this.fileEntries = [header, ...pathWithoutLabels, ...labelEntries]; this.sessionId = newSessionId; this.sessionFile = newSessionFile; - this.flushed = true; 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) { + this._rewriteFile(); + this.flushed = true; + } else { + this.flushed = false; + } + return newSessionFile; } 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 0a684d7d..2cfb10bc 100644 --- a/packages/coding-agent/test/session-manager/tree-traversal.test.ts +++ b/packages/coding-agent/test/session-manager/tree-traversal.test.ts @@ -1,3 +1,6 @@ +import { existsSync, mkdirSync, readFileSync, 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.js"; import { assistantMsg, userMsg } from "../utilities.js"; @@ -457,4 +460,74 @@ describe("createBranchedSession", () => { expect(entries).toHaveLength(4); expect(entries.map((e) => e.id)).toEqual([id1, id2, id4, id5]); }); + + it("does not duplicate entries when forking from 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")); + 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); + 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) + expect(existsSync(newFile!)).toBe(false); + + // 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); + } finally { + rmSync(tempDir, { recursive: true, force: true }); + } + }); + + it("writes file immediately when forking from a point with assistant messages", () => { + const tempDir = join(tmpdir(), `session-fork-with-assistant-${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")); + + // Fork including the assistant message + const newFile = session.createBranchedSession(id2); + expect(newFile).toBeDefined(); + + // Path includes an assistant, so file should be written immediately + 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); + } finally { + rmSync(tempDir, { recursive: true, force: true }); + } + }); });