From bcd23ee7c1f7426fce080dd10b40e8bf9a02acd7 Mon Sep 17 00:00:00 2001 From: can1357 Date: Thu, 23 Jul 2026 17:22:37 +0200 Subject: [PATCH] fix: kept seeded workspace roots lazy until the session file is durable Seeding additionalDirectories at launch (via --add-dir or the workspace.additionalDirectories setting) called #rewriteAtomically on a brand-new session manager, which materialized a header-only JSONL and a fresh breadcrumb before any assistant output. Launching and exiting with configured roots therefore created an empty resumable session that --continue picked over the previous conversation. Gated all three workspace-directory mutators behind the existing #shouldHaveSessionFile() lazy-persistence gate (one shared helper), and made setAdditionalDirectories a no-op when the normalized list is unchanged so resuming large sessions no longer rewrites the whole JSONL on every startup. Roots set before the gate is crossed land in the header with the first durable write; added a regression test. --- .../src/session/session-manager.ts | 35 +++++++++++-------- .../workspace-directories.test.ts | 31 ++++++++++++---- 2 files changed, 46 insertions(+), 20 deletions(-) diff --git a/packages/coding-agent/src/session/session-manager.ts b/packages/coding-agent/src/session/session-manager.ts index 4b07c74c3..e3d8018e1 100644 --- a/packages/coding-agent/src/session/session-manager.ts +++ b/packages/coding-agent/src/session/session-manager.ts @@ -1334,6 +1334,18 @@ export class SessionManager { return [...this.#additionalDirectories]; } + /** + * Persist a workspace-directory change to the session header. Respects the + * lazy-persistence gate: a session with no durable output yet keeps the + * change in memory (the header lands with the first real write), so seeding + * roots at launch never materializes an empty resumable session file. + */ + async #persistWorkspaceDirectoriesChange(): Promise { + if (!this.#persist || !this.#sessionFile || !this.#shouldHaveSessionFile()) return; + this.#rewriteRequired = true; + await this.#rewriteAtomically(); + } + /** * Add a workspace directory. Normalizes (relative to cwd), dedupes, rejects * the cwd itself, persists to the session header, and triggers an atomic @@ -1348,10 +1360,7 @@ export class SessionManager { if (this.#additionalDirectories.includes(resolved)) return null; this.#additionalDirectories = [...this.#additionalDirectories, resolved]; this.#header.additionalDirectories = this.#additionalDirectories; - if (this.#persist && this.#sessionFile) { - this.#rewriteRequired = true; - await this.#rewriteAtomically(); - } + await this.#persistWorkspaceDirectoriesChange(); return resolved; } @@ -1370,26 +1379,24 @@ export class SessionManager { } else { this.#header.additionalDirectories = this.#additionalDirectories; } - if (this.#persist && this.#sessionFile) { - this.#rewriteRequired = true; - await this.#rewriteAtomically(); - } + await this.#persistWorkspaceDirectoriesChange(); return resolved; } - /** Seed additional directories from settings or a passed list. Also called on resumed sessions with --add-dir; persists the updated header when a session file already exists. */ + /** Seed additional directories from settings or a passed list. Also called on resumed sessions with --add-dir; persists the updated header when the session file is already durable. No-op when the normalized list is unchanged (avoids rewriting large session files on every startup). */ async setAdditionalDirectories(directories: string[]): Promise { const workspace = normalizeSessionWorkspace({ cwd: this.#cwd, directories }); - this.#additionalDirectories = additionalWorkspaceDirectories(workspace); + const next = additionalWorkspaceDirectories(workspace); + if (next.length === this.#additionalDirectories.length && next.every((d, i) => d === this.#additionalDirectories[i])) { + return; + } + this.#additionalDirectories = next; if (this.#additionalDirectories.length > 0) { this.#header.additionalDirectories = this.#additionalDirectories; } else { this.#header.additionalDirectories = undefined; } - if (this.#persist && this.#sessionFile) { - this.#rewriteRequired = true; - await this.#rewriteAtomically(); - } + await this.#persistWorkspaceDirectoriesChange(); } getUsageStatistics(): UsageStatistics { diff --git a/packages/coding-agent/test/session-manager/workspace-directories.test.ts b/packages/coding-agent/test/session-manager/workspace-directories.test.ts index efc06bb52..34c40eda6 100644 --- a/packages/coding-agent/test/session-manager/workspace-directories.test.ts +++ b/packages/coding-agent/test/session-manager/workspace-directories.test.ts @@ -8,6 +8,7 @@ import { normalizeSessionWorkspace, } from "@oh-my-pi/pi-coding-agent/session/session-workspace"; import { TempDir } from "@oh-my-pi/pi-utils"; +import { makeAssistantMessage } from "./helpers"; describe("normalizeSessionWorkspace", () => { it("places cwd first and dedupes additional directories", () => { @@ -105,8 +106,8 @@ describe("SessionManager workspace directories", () => { using tempDir = TempDir.createSync("@pi-session-workspace-persist-"); const session = SessionManager.create(tempDir.path(), tempDir.path()); await session.addWorkspaceDirectory(path.join(tempDir.path(), "sibling")); - // Materialize on disk so reopen reads the header. - session.appendMessage({ role: "user", content: "hello", timestamp: 1 }); + // Materialize on disk so reopen reads the header (lazy gate needs assistant output). + session.appendMessage(makeAssistantMessage()); await session.flush(); const file = session.getSessionFile(); @@ -123,7 +124,7 @@ describe("SessionManager workspace directories", () => { using tempDir = TempDir.createSync("@pi-session-workspace-clear-"); const session = SessionManager.create(tempDir.path(), tempDir.path()); await session.addWorkspaceDirectory(path.join(tempDir.path(), "extra")); - session.appendMessage({ role: "user", content: "hi", timestamp: 1 }); + session.appendMessage(makeAssistantMessage()); await session.flush(); await session.removeWorkspaceDirectory(path.join(tempDir.path(), "extra")); @@ -149,8 +150,8 @@ describe("SessionManager workspace directories", () => { it("setAdditionalDirectories persists the updated header on a resumed session", async () => { using tempDir = TempDir.createSync("@pi-session-workspace-resume-"); const session = SessionManager.create(tempDir.path(), tempDir.path()); - // Simulate a resumed session: append a message so the file exists, then setAdditionalDirectories. - session.appendMessage({ role: "user", content: "hello", timestamp: 1 }); + // Simulate a resumed session: append an assistant message so the file exists, then setAdditionalDirectories. + session.appendMessage(makeAssistantMessage()); await session.flush(); await session.setAdditionalDirectories([path.join(tempDir.path(), "added")]); @@ -164,11 +165,29 @@ describe("SessionManager workspace directories", () => { expect(header.additionalDirectories).toEqual([path.join(tempDir.path(), "added")]); }); + it("keeps seeded roots in memory until the session is durable (no empty session file)", async () => { + using tempDir = TempDir.createSync("@pi-session-workspace-lazy-"); + const session = SessionManager.create(tempDir.path(), tempDir.path()); + await session.setAdditionalDirectories([path.join(tempDir.path(), "extra")]); + await session.addWorkspaceDirectory(path.join(tempDir.path(), "extra2")); + // Seeding roots must not materialize an empty resumable session file. + expect(fs.readdirSync(tempDir.path()).filter(f => f.endsWith(".jsonl"))).toEqual([]); + + // Once the session produces durable output, the header carries the roots. + session.appendMessage(makeAssistantMessage()); + await session.flush(); + const reopened = await SessionManager.open(session.getSessionFile()!); + expect(reopened.getAdditionalDirectories()).toEqual([ + path.join(tempDir.path(), "extra"), + path.join(tempDir.path(), "extra2"), + ]); + }); + it("forkFrom preserves additionalDirectories from the source session", async () => { using tempDir = TempDir.createSync("@pi-session-workspace-fork-"); const source = SessionManager.create(tempDir.path(), tempDir.path()); await source.addWorkspaceDirectory(path.join(tempDir.path(), "extra")); - source.appendMessage({ role: "user", content: "hello", timestamp: 1 }); + source.appendMessage(makeAssistantMessage()); await source.flush(); const forked = await SessionManager.forkFrom(source.getSessionFile()!, tempDir.path());