mirror of
https://github.com/getcompanion-ai/co-mono.git
synced 2026-04-15 09:01:14 +00:00
fix(coding-agent): prevent duplicate session headers when forking from pre-assistant entry
createBranchedSession() wrote the file and set flushed=true even when the branched path had no assistant message. The next _persist() call saw no assistant, reset flushed=false, and the subsequent flush appended all in-memory entries to the already-populated file, duplicating the header and entries. Fix: defer file creation when the branched path has no assistant message, matching the newSession() contract. _persist() creates the file on the first assistant response. closes #1672
This commit is contained in:
parent
9825c13f5f
commit
2f64df1e52
3 changed files with 90 additions and 7 deletions
|
|
@ -9,6 +9,7 @@
|
||||||
### Fixed
|
### 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)).
|
- `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
|
## [0.55.1] - 2026-02-26
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -1187,11 +1187,7 @@ export class SessionManager {
|
||||||
}
|
}
|
||||||
|
|
||||||
if (this.persist) {
|
if (this.persist) {
|
||||||
appendFileSync(newSessionFile, `${JSON.stringify(header)}\n`);
|
// Build label entries
|
||||||
for (const entry of pathWithoutLabels) {
|
|
||||||
appendFileSync(newSessionFile, `${JSON.stringify(entry)}\n`);
|
|
||||||
}
|
|
||||||
// Write fresh label entries at the end
|
|
||||||
const lastEntryId = pathWithoutLabels[pathWithoutLabels.length - 1]?.id || null;
|
const lastEntryId = pathWithoutLabels[pathWithoutLabels.length - 1]?.id || null;
|
||||||
let parentId = lastEntryId;
|
let parentId = lastEntryId;
|
||||||
const labelEntries: LabelEntry[] = [];
|
const labelEntries: LabelEntry[] = [];
|
||||||
|
|
@ -1204,16 +1200,29 @@ export class SessionManager {
|
||||||
targetId,
|
targetId,
|
||||||
label,
|
label,
|
||||||
};
|
};
|
||||||
appendFileSync(newSessionFile, `${JSON.stringify(labelEntry)}\n`);
|
|
||||||
pathEntryIds.add(labelEntry.id);
|
pathEntryIds.add(labelEntry.id);
|
||||||
labelEntries.push(labelEntry);
|
labelEntries.push(labelEntry);
|
||||||
parentId = labelEntry.id;
|
parentId = labelEntry.id;
|
||||||
}
|
}
|
||||||
|
|
||||||
this.fileEntries = [header, ...pathWithoutLabels, ...labelEntries];
|
this.fileEntries = [header, ...pathWithoutLabels, ...labelEntries];
|
||||||
this.sessionId = newSessionId;
|
this.sessionId = newSessionId;
|
||||||
this.sessionFile = newSessionFile;
|
this.sessionFile = newSessionFile;
|
||||||
this.flushed = true;
|
|
||||||
this._buildIndex();
|
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;
|
return newSessionFile;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -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 { describe, expect, it } from "vitest";
|
||||||
import { type CustomEntry, SessionManager } from "../../src/core/session-manager.js";
|
import { type CustomEntry, SessionManager } from "../../src/core/session-manager.js";
|
||||||
import { assistantMsg, userMsg } from "../utilities.js";
|
import { assistantMsg, userMsg } from "../utilities.js";
|
||||||
|
|
@ -457,4 +460,74 @@ describe("createBranchedSession", () => {
|
||||||
expect(entries).toHaveLength(4);
|
expect(entries).toHaveLength(4);
|
||||||
expect(entries.map((e) => e.id)).toEqual([id1, id2, id4, id5]);
|
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 });
|
||||||
|
}
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue