mirror of
https://github.com/badlogic/pi-mono.git
synced 2026-11-05 23:37:25 +00:00
fix(coding-agent): write forked session through one fd
forkFrom reopened the target file with appendFileSync for every entry, which is slow where open/close is expensive. fixes #11253
This commit is contained in:
parent
f3fa8bd924
commit
8ddc44ec8b
2 changed files with 77 additions and 6 deletions
|
|
@ -1853,13 +1853,18 @@ export class SessionManager {
|
|||
cwd: resolvedTargetCwd,
|
||||
parentSession: resolvedSourcePath,
|
||||
};
|
||||
writeFileSync(newSessionFile, `${JSON.stringify(newHeader)}\n`, { flag: "wx" });
|
||||
|
||||
// Copy all non-header entries from source
|
||||
for (const entry of sourceEntries) {
|
||||
if (entry.type !== "session") {
|
||||
appendFileSync(newSessionFile, `${JSON.stringify(entry)}\n`);
|
||||
// Write the header and all non-header entries through one fd. Reopening the file per entry
|
||||
// is slow where open/close is expensive (e.g. filesystem security monitors).
|
||||
const fd = openSync(newSessionFile, "wx");
|
||||
try {
|
||||
writeFileSync(fd, `${JSON.stringify(newHeader)}\n`);
|
||||
for (const entry of sourceEntries) {
|
||||
if (entry.type !== "session") {
|
||||
writeFileSync(fd, `${JSON.stringify(entry)}\n`);
|
||||
}
|
||||
}
|
||||
} finally {
|
||||
closeSync(fd);
|
||||
}
|
||||
|
||||
return new SessionManager(resolvedTargetCwd, dir, newSessionFile, true);
|
||||
|
|
|
|||
66
packages/coding-agent/test/session-manager/fork-from.test.ts
Normal file
66
packages/coding-agent/test/session-manager/fork-from.test.ts
Normal file
|
|
@ -0,0 +1,66 @@
|
|||
import type * as Fs from "node:fs";
|
||||
import { mkdtempSync, readFileSync, writeFileSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import { SessionManager } from "../../src/core/session-manager.ts";
|
||||
|
||||
const { openSyncSpy, appendFileSyncSpy } = vi.hoisted(() => ({
|
||||
openSyncSpy: vi.fn(),
|
||||
appendFileSyncSpy: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock("fs", async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof Fs>();
|
||||
openSyncSpy.mockImplementation(actual.openSync);
|
||||
appendFileSyncSpy.mockImplementation(actual.appendFileSync);
|
||||
return { ...actual, openSync: openSyncSpy, appendFileSync: appendFileSyncSpy };
|
||||
});
|
||||
|
||||
describe("SessionManager.forkFrom", () => {
|
||||
// Regression test for #11253: forking reopened the target file once per entry.
|
||||
it("copies all entries while opening the new session file only once for writing", () => {
|
||||
const tempDir = mkdtempSync(join(tmpdir(), "pi-fork-from-"));
|
||||
const sourcePath = join(tempDir, "source.jsonl");
|
||||
const lines = [
|
||||
JSON.stringify({
|
||||
type: "session",
|
||||
version: 3,
|
||||
id: "source-id",
|
||||
timestamp: new Date().toISOString(),
|
||||
cwd: tempDir,
|
||||
}),
|
||||
];
|
||||
let parentId: string | null = null;
|
||||
for (let i = 0; i < 50; i++) {
|
||||
const id = `e${i}`;
|
||||
lines.push(
|
||||
JSON.stringify({
|
||||
type: "message",
|
||||
id,
|
||||
parentId,
|
||||
timestamp: new Date().toISOString(),
|
||||
message: { role: "user", content: `message ${i}`, timestamp: 0 },
|
||||
}),
|
||||
);
|
||||
parentId = id;
|
||||
}
|
||||
writeFileSync(sourcePath, `${lines.join("\n")}\n`);
|
||||
|
||||
const targetDir = join(tempDir, "sessions");
|
||||
openSyncSpy.mockClear();
|
||||
appendFileSyncSpy.mockClear();
|
||||
const forked = SessionManager.forkFrom(sourcePath, tempDir, targetDir);
|
||||
const newFile = forked.getSessionFile()!;
|
||||
|
||||
const writeOpens = openSyncSpy.mock.calls.filter(([path, flags]) => path === newFile && flags !== "r");
|
||||
expect(writeOpens).toEqual([[newFile, "wx"]]);
|
||||
expect(appendFileSyncSpy.mock.calls.filter(([path]) => path === newFile)).toHaveLength(0);
|
||||
|
||||
const written = readFileSync(newFile, "utf8").trimEnd().split("\n");
|
||||
expect(written).toHaveLength(51);
|
||||
expect(JSON.parse(written[0]).parentSession).toBe(sourcePath);
|
||||
expect(written.slice(1)).toEqual(lines.slice(1));
|
||||
expect(forked.getEntries()).toHaveLength(50);
|
||||
});
|
||||
});
|
||||
Loading…
Add table
Add a link
Reference in a new issue