From 9148ec01ecd13c91982d44761d65b0c8ff192887 Mon Sep 17 00:00:00 2001 From: can1357 Date: Mon, 16 Mar 2026 16:06:04 +0100 Subject: [PATCH] feat(session-manager): added optional sessionDir with multi-root migration support - Made sessionDir parameter optional in SessionManager.create(), forkFrom(), continueRecent(), and list() methods with automatic default computation. - Updated SessionManager.getDefaultSessionDir() to accept optional agentDir parameter for custom sessions root configuration. - Changed SessionManager.list() signature to require cwd parameter as first argument with sessionDir now optional. - Implemented multi-root session migration support by replacing global migration state with per-root tracking and extracting session directory encoding logic. - Added resolveManagedSessionRoot() function to determine if session directory is managed and extract its root. --- packages/coding-agent/CHANGELOG.md | 4 + .../coding-agent/examples/sdk/11-sessions.ts | 6 +- packages/coding-agent/src/main.ts | 7 +- packages/coding-agent/src/sdk.ts | 6 +- .../src/session/session-manager.ts | 119 +++++++++++------- .../test/sdk-session-isolation.test.ts | 50 ++++++++ .../session-manager/file-operations.test.ts | 6 +- .../test/session-manager/move-to.test.ts | 8 +- 8 files changed, 145 insertions(+), 61 deletions(-) create mode 100644 packages/coding-agent/test/sdk-session-isolation.test.ts diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index aeeb8a157..1577284a2 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -1,6 +1,7 @@ # Changelog ## [Unreleased] + ### Breaking Changes - Changed `SessionManager.create()` to require explicit `sessionDir` parameter instead of optional—callers must now pass `SessionManager.getDefaultSessionDir(cwd)` to use default behavior @@ -16,6 +17,9 @@ ### Changed +- Made `sessionDir` parameter optional in `SessionManager.create()`, `SessionManager.continueRecent()`, and `SessionManager.forkFrom()`—callers can now omit it to use the default session directory +- Changed `SessionManager.list()` signature to accept `cwd` as the first parameter instead of requiring an explicit `sessionDir`—callers can now omit `sessionDir` to use the default for the given working directory +- Updated `SessionManager.getDefaultSessionDir()` to accept optional `agentDir` parameter for computing session directories within a custom agent root - Improved status line path display to strip display roots using canonical path resolution, correctly handling symlink aliases to home and Projects directories - Improved error messaging in ast_grep when no matches are found with parse errors, now suggests narrowing `path`/`glob` or setting `lang` to resolve mis-scoped queries diff --git a/packages/coding-agent/examples/sdk/11-sessions.ts b/packages/coding-agent/examples/sdk/11-sessions.ts index 3249bd9e8..17c9c0c19 100644 --- a/packages/coding-agent/examples/sdk/11-sessions.ts +++ b/packages/coding-agent/examples/sdk/11-sessions.ts @@ -13,19 +13,19 @@ console.log("In-memory session:", inMemory.sessionFile ?? "(none)"); // New persistent session const { session: newSession } = await createAgentSession({ - sessionManager: SessionManager.create(process.cwd(), SessionManager.getDefaultSessionDir(process.cwd())), + sessionManager: SessionManager.create(process.cwd()), }); console.log("New session file:", newSession.sessionFile); // Continue most recent session (or create new if none) const { session: continued, modelFallbackMessage } = await createAgentSession({ - sessionManager: await SessionManager.continueRecent(process.cwd(), SessionManager.getDefaultSessionDir(process.cwd())), + sessionManager: await SessionManager.continueRecent(process.cwd()), }); if (modelFallbackMessage) console.log("Note:", modelFallbackMessage); console.log("Continued session:", continued.sessionFile); // List and open specific session -const sessions = await SessionManager.list(SessionManager.getDefaultSessionDir(process.cwd())); +const sessions = await SessionManager.list(process.cwd()); console.log(`\nFound ${sessions.length} sessions:`); for (const info of sessions.slice(0, 3)) { console.log(` ${info.id.slice(0, 8)}… - "${info.firstMessage.slice(0, 30)}…"`); diff --git a/packages/coding-agent/src/main.ts b/packages/coding-agent/src/main.ts index 8e2c13893..cfe6577ad 100644 --- a/packages/coding-agent/src/main.ts +++ b/packages/coding-agent/src/main.ts @@ -233,7 +233,6 @@ async function createSessionManager(parsed: Args, cwd: string): Promise - SessionManager.list(parsedArgs.sessionDir ?? SessionManager.getDefaultSessionDir(cwd)), + SessionManager.list(cwd, parsedArgs.sessionDir), ); if (sessions.length === 0) { process.stdout.write(`${chalk.dim("No sessions found")}\n`); diff --git a/packages/coding-agent/src/sdk.ts b/packages/coding-agent/src/sdk.ts index 75b7fb7a2..9221e5393 100644 --- a/packages/coding-agent/src/sdk.ts +++ b/packages/coding-agent/src/sdk.ts @@ -191,7 +191,7 @@ export interface CreateAgentSessionOptions { /** Parent task ID prefix for nested artifact naming (e.g., "6-Extensions") */ parentTaskPrefix?: string; - /** Session manager. Default: SessionManager.create(cwd, SessionManager.getDefaultSessionDir(cwd)) */ + /** Session manager. Default: session stored under the configured agentDir sessions root */ sessionManager?: SessionManager; /** Settings instance. Default: Settings.init({ cwd, agentDir }) */ @@ -658,7 +658,9 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {} const sessionManager = options.sessionManager ?? - logger.time("sessionManager", () => SessionManager.create(cwd, SessionManager.getDefaultSessionDir(cwd))); + logger.time("sessionManager", () => + SessionManager.create(cwd, SessionManager.getDefaultSessionDir(cwd, agentDir)), + ); const sessionId = sessionManager.getSessionId(); const modelApiKeyAvailability = new Map(); const getModelAvailabilityKey = (candidate: Model): string => diff --git a/packages/coding-agent/src/session/session-manager.ts b/packages/coding-agent/src/session/session-manager.ts index 9e408739a..6f1a3c89d 100644 --- a/packages/coding-agent/src/session/session-manager.ts +++ b/packages/coding-agent/src/session/session-manager.ts @@ -347,7 +347,7 @@ export function migrateSessionEntries(entries: FileEntry[]): void { migrateToCurrentVersion(entries); } -let sessionDirsMigrated = false; +const migratedSessionRoots = new Set(); /** * Merge or rename a legacy session directory into its canonical target. @@ -377,19 +377,36 @@ function encodeLegacyAbsoluteSessionDirName(cwd: string): string { return `--${resolvedCwd.replace(/^[/\\]/, "").replace(/[/\\:]/g, "-")}--`; } +function encodeRelativeSessionDirName(prefix: string, root: string, cwd: string): string { + const relative = path.relative(root, cwd).replace(/[/\\:]/g, "-"); + return relative ? (prefix.endsWith("-") ? `${prefix}${relative}` : `${prefix}-${relative}`) : prefix; +} + +function getDefaultSessionDirName(cwd: string): { encodedDirName: string; resolvedCwd: string } { + const resolvedCwd = path.resolve(cwd); + const canonicalCwd = resolveEquivalentPath(resolvedCwd); + const home = resolveEquivalentPath(os.homedir()); + const tempRoot = resolveEquivalentPath(os.tmpdir()); + const encodedDirName = pathIsWithin(home, canonicalCwd) + ? encodeRelativeSessionDirName("-", home, canonicalCwd) + : pathIsWithin(tempRoot, canonicalCwd) + ? encodeRelativeSessionDirName("-tmp", tempRoot, canonicalCwd) + : encodeLegacyAbsoluteSessionDirName(canonicalCwd); + return { encodedDirName, resolvedCwd }; +} + /** * Migrate old `---*--` session dirs to the new `-*` format. - * Runs once on first access, best-effort. + * Runs once per sessions root on first access, best-effort. */ -function migrateHomeSessionDirs(): void { - if (sessionDirsMigrated) return; - sessionDirsMigrated = true; +function migrateHomeSessionDirs(sessionsRoot: string): void { + if (migratedSessionRoots.has(sessionsRoot)) return; + migratedSessionRoots.add(sessionsRoot); const home = os.homedir(); const homeEncoded = home.replace(/^[/\\]/, "").replace(/[/\\:]/g, "-"); const oldPrefix = `--${homeEncoded}-`; const oldExact = `--${homeEncoded}--`; - const sessionsRoot = getSessionsDir(); let entries: string[]; try { @@ -420,8 +437,8 @@ function migrateHomeSessionDirs(): void { } } -function migrateLegacyAbsoluteSessionDir(cwd: string, sessionDir: string): void { - const legacyDir = path.join(getSessionsDir(), encodeLegacyAbsoluteSessionDirName(cwd)); +function migrateLegacyAbsoluteSessionDir(cwd: string, sessionDir: string, sessionsRoot: string): void { + const legacyDir = path.join(sessionsRoot, encodeLegacyAbsoluteSessionDirName(cwd)); if (legacyDir === sessionDir || !fs.existsSync(legacyDir)) return; try { @@ -431,6 +448,15 @@ function migrateLegacyAbsoluteSessionDir(cwd: string, sessionDir: string): void } } +function resolveManagedSessionRoot(sessionDir: string, cwd: string): string | undefined { + const currentDirName = path.basename(sessionDir); + const { encodedDirName } = getDefaultSessionDirName(cwd); + if (currentDirName !== encodedDirName && currentDirName !== encodeLegacyAbsoluteSessionDirName(cwd)) { + return undefined; + } + return path.dirname(sessionDir); +} + /** Exported for compaction.test.ts */ export function parseSessionEntries(content: string): FileEntry[] { return parseJsonlLenient(content); @@ -630,25 +656,15 @@ export function buildSessionContext( * Classifies cwd by canonical location so symlink/alias paths resolve to the * same home-relative or temp-root directory names as their real targets. */ -function computeDefaultSessionDir(cwd: string, storage: SessionStorage): string { - const resolvedCwd = path.resolve(cwd); - const canonicalCwd = resolveEquivalentPath(resolvedCwd); - const home = resolveEquivalentPath(os.homedir()); - const tempRoot = resolveEquivalentPath(os.tmpdir()); - let encodedDirName: string; - if (pathIsWithin(home, canonicalCwd)) { - const relative = path.relative(home, canonicalCwd).replace(/[/\\:]/g, "-"); - encodedDirName = relative ? `-${relative}` : "-"; - } else if (pathIsWithin(tempRoot, canonicalCwd)) { - const relative = path.relative(tempRoot, canonicalCwd).replace(/[/\\:]/g, "-"); - encodedDirName = relative ? `-tmp-${relative}` : "-tmp"; - } else { - encodedDirName = encodeLegacyAbsoluteSessionDirName(canonicalCwd); - } - - migrateHomeSessionDirs(); - const sessionDir = path.join(getSessionsDir(), encodedDirName); - migrateLegacyAbsoluteSessionDir(resolvedCwd, sessionDir); +function computeDefaultSessionDir( + cwd: string, + storage: SessionStorage, + sessionsRoot: string = getSessionsDir(), +): string { + const { encodedDirName, resolvedCwd } = getDefaultSessionDirName(cwd); + migrateHomeSessionDirs(sessionsRoot); + const sessionDir = path.join(sessionsRoot, encodedDirName); + migrateLegacyAbsoluteSessionDir(resolvedCwd, sessionDir, sessionsRoot); storage.ensureDirSync(sessionDir); return sessionDir; } @@ -1310,8 +1326,8 @@ export async function resolveResumableSession( sessionDir?: string, storage: SessionStorage = new FileSessionStorage(), ): Promise { - const localSessionDir = sessionDir ?? SessionManager.getDefaultSessionDir(cwd, storage); - const localSessions = await SessionManager.list(localSessionDir, storage); + const localSessionDir = sessionDir ?? SessionManager.getDefaultSessionDir(cwd, undefined, storage); + const localSessions = await SessionManager.list(cwd, localSessionDir, storage); const localMatch = localSessions.find(session => sessionMatchesResumeArg(session, sessionArg)); if (localMatch) { return { session: localMatch, scope: "local" }; @@ -1478,7 +1494,10 @@ export class SessionManager { const resolvedCwd = path.resolve(newCwd); if (resolvedCwd === this.cwd) return; - const newSessionDir = computeDefaultSessionDir(resolvedCwd, this.storage); + const managedSessionsRoot = resolveManagedSessionRoot(this.sessionDir, this.cwd); + const newSessionDir = managedSessionsRoot + ? computeDefaultSessionDir(resolvedCwd, this.storage, managedSessionsRoot) + : computeDefaultSessionDir(resolvedCwd, this.storage); let hadSessionFile = false; if (this.persist && this.#sessionFile) { @@ -2437,19 +2456,23 @@ export class SessionManager { /** * Resolve the canonical default session directory for a cwd. - * Callers must opt in explicitly instead of silently falling back to the global agent root. */ - static getDefaultSessionDir(cwd: string, storage: SessionStorage = new FileSessionStorage()): string { - return computeDefaultSessionDir(cwd, storage); + static getDefaultSessionDir( + cwd: string, + agentDir?: string, + storage: SessionStorage = new FileSessionStorage(), + ): string { + return computeDefaultSessionDir(cwd, storage, getSessionsDir(agentDir)); } /** * Create a new session. * @param cwd Working directory (stored in session header) - * @param sessionDir Explicit session directory for persistence. + * @param sessionDir Optional session directory. If omitted, uses default (~/.omp/agent/sessions//). */ - static create(cwd: string, sessionDir: string, storage: SessionStorage = new FileSessionStorage()): SessionManager { - const manager = new SessionManager(cwd, sessionDir, true, storage); + static create(cwd: string, sessionDir?: string, storage: SessionStorage = new FileSessionStorage()): SessionManager { + const dir = sessionDir ?? SessionManager.getDefaultSessionDir(cwd, undefined, storage); + const manager = new SessionManager(cwd, dir, true, storage); manager.#initNewSession(); return manager; } @@ -2461,10 +2484,11 @@ export class SessionManager { static async forkFrom( sourcePath: string, cwd: string, - sessionDir: string, + sessionDir?: string, storage: SessionStorage = new FileSessionStorage(), ): Promise { - const manager = new SessionManager(cwd, sessionDir, true, storage); + const dir = sessionDir ?? SessionManager.getDefaultSessionDir(cwd, undefined, storage); + const manager = new SessionManager(cwd, dir, true, storage); const forkEntries = structuredClone(await loadEntriesFromFile(sourcePath, storage)) as FileEntry[]; migrateToCurrentVersion(forkEntries); await resolveBlobRefsInEntries(forkEntries, manager.#blobStore); @@ -2504,17 +2528,18 @@ export class SessionManager { /** * Continue the most recent session, or create new if none. * @param cwd Working directory - * @param sessionDir Explicit session directory for persistence. + * @param sessionDir Optional session directory. If omitted, uses default (~/.omp/agent/sessions//). */ static async continueRecent( cwd: string, - sessionDir: string, + sessionDir?: string, storage: SessionStorage = new FileSessionStorage(), ): Promise { + const dir = sessionDir ?? SessionManager.getDefaultSessionDir(cwd, undefined, storage); // Prefer terminal-scoped breadcrumb (handles concurrent sessions correctly) const terminalSession = await readTerminalBreadcrumb(cwd); - const mostRecent = terminalSession ?? (await findMostRecentSession(sessionDir, storage)); - const manager = new SessionManager(cwd, sessionDir, true, storage); + const mostRecent = terminalSession ?? (await findMostRecentSession(dir, storage)); + const manager = new SessionManager(cwd, dir, true, storage); if (mostRecent) { await manager.#initSessionFile(mostRecent); } else { @@ -2534,14 +2559,18 @@ export class SessionManager { } /** - * List all sessions in an explicit session directory. + * List all sessions. + * @param cwd Working directory (used to compute default session directory) + * @param sessionDir Optional session directory. If omitted, uses default (~/.omp/agent/sessions//). */ static async list( - sessionDir: string, + cwd: string, + sessionDir?: string, storage: SessionStorage = new FileSessionStorage(), ): Promise { + const dir = sessionDir ?? SessionManager.getDefaultSessionDir(cwd, undefined, storage); try { - const files = storage.listFilesSync(sessionDir, "*.jsonl"); + const files = storage.listFilesSync(dir, "*.jsonl"); return await collectSessionsFromFiles(files, storage); } catch { return []; diff --git a/packages/coding-agent/test/sdk-session-isolation.test.ts b/packages/coding-agent/test/sdk-session-isolation.test.ts new file mode 100644 index 000000000..762d69598 --- /dev/null +++ b/packages/coding-agent/test/sdk-session-isolation.test.ts @@ -0,0 +1,50 @@ +import { afterEach, describe, expect, it } from "bun:test"; +import * as fs from "node:fs"; +import * as os from "node:os"; +import * as path from "node:path"; +import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; +import { createAgentSession } from "@oh-my-pi/pi-coding-agent/sdk"; +import { getSessionsDir, Snowflake } from "@oh-my-pi/pi-utils"; + +describe("createAgentSession session storage isolation", () => { + const tempDirs: string[] = []; + + afterEach(async () => { + for (const tempDir of tempDirs.splice(0)) { + fs.rmSync(tempDir, { recursive: true, force: true }); + } + }); + + it("uses the provided agentDir for the default persistent session root", async () => { + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), `pi-sdk-session-isolation-${Snowflake.next()}-`)); + tempDirs.push(tempDir); + const cwd = path.join(tempDir, `project-${Snowflake.next()}`); + const agentDir = path.join(tempDir, "agent"); + fs.mkdirSync(cwd, { recursive: true }); + + const { session } = await createAgentSession({ + cwd, + agentDir, + settings: Settings.isolated(), + disableExtensionDiscovery: true, + skills: [], + contextFiles: [], + promptTemplates: [], + slashCommands: [], + enableMCP: false, + enableLsp: false, + }); + + try { + const sessionFile = session.sessionFile; + if (!sessionFile) { + throw new Error("Expected session file path"); + } + + expect(sessionFile.startsWith(path.join(agentDir, "sessions"))).toBe(true); + expect(sessionFile.startsWith(getSessionsDir())).toBe(false); + } finally { + await session.dispose(); + } + }); +}); diff --git a/packages/coding-agent/test/session-manager/file-operations.test.ts b/packages/coding-agent/test/session-manager/file-operations.test.ts index d55e0dd13..9273a03a1 100644 --- a/packages/coding-agent/test/session-manager/file-operations.test.ts +++ b/packages/coding-agent/test/session-manager/file-operations.test.ts @@ -247,7 +247,7 @@ describe("SessionManager temp cwd session dirs", () => { fs.symlinkSync(os.homedir(), homeAlias, "dir"); const aliasedCwd = path.join(homeAlias, "Projects", path.basename(realProjectDir), "nested"); - const session = SessionManager.create(aliasedCwd, SessionManager.getDefaultSessionDir(aliasedCwd)); + const session = SessionManager.create(aliasedCwd); const sessionFile = session.getSessionFile(); if (!sessionFile) throw new Error("Expected session file path"); @@ -266,7 +266,7 @@ describe("SessionManager temp cwd session dirs", () => { const tempCwd = path.join(testAgentDir, `temp-cwd-${Snowflake.next()}`); fs.mkdirSync(tempCwd, { recursive: true }); - const session = SessionManager.create(tempCwd, SessionManager.getDefaultSessionDir(tempCwd)); + const session = SessionManager.create(tempCwd); const sessionFile = session.getSessionFile(); if (!sessionFile) throw new Error("Expected session file path"); @@ -282,7 +282,7 @@ describe("SessionManager temp cwd session dirs", () => { fs.mkdirSync(legacyDir, { recursive: true }); fs.writeFileSync(markerFile, "marker\n"); - const session = SessionManager.create(tempCwd, SessionManager.getDefaultSessionDir(tempCwd)); + const session = SessionManager.create(tempCwd); const sessionFile = session.getSessionFile(); if (!sessionFile) throw new Error("Expected session file path"); diff --git a/packages/coding-agent/test/session-manager/move-to.test.ts b/packages/coding-agent/test/session-manager/move-to.test.ts index 1f0fbf9a8..7c776e9cd 100644 --- a/packages/coding-agent/test/session-manager/move-to.test.ts +++ b/packages/coding-agent/test/session-manager/move-to.test.ts @@ -106,7 +106,7 @@ describe("SessionManager.moveTo", () => { }); it("moves session file and updates header cwd (baseline)", async () => { - const session = SessionManager.create(cwdA, SessionManager.getDefaultSessionDir(cwdA)); + const session = SessionManager.create(cwdA); session.appendMessage({ role: "user", content: "hello", timestamp: 1 }); session.appendMessage(makeAssistantMessage()); await session.flush(); @@ -130,7 +130,7 @@ describe("SessionManager.moveTo", () => { }); it("succeeds on fresh session without ENOENT, then deferred persistence works", async () => { - const session = SessionManager.create(cwdA, SessionManager.getDefaultSessionDir(cwdA)); + const session = SessionManager.create(cwdA); // No messages — file never written to disk const oldFile = session.getSessionFile()!; expect(fs.existsSync(oldFile)).toBe(false); @@ -154,7 +154,7 @@ describe("SessionManager.moveTo", () => { }); it("recreates file from memory when old file is deleted (assistant exists)", async () => { - const session = SessionManager.create(cwdA, SessionManager.getDefaultSessionDir(cwdA)); + const session = SessionManager.create(cwdA); session.appendMessage({ role: "user", content: "hello", timestamp: 1 }); session.appendMessage(makeAssistantMessage()); await session.flush(); @@ -223,7 +223,7 @@ describe("SessionManager.moveTo", () => { }); it("moves artifact dir independently when session file does not exist", async () => { - const session = SessionManager.create(cwdA, SessionManager.getDefaultSessionDir(cwdA)); + const session = SessionManager.create(cwdA); // Allocate an artifact — creates dir via ArtifactManager const { path: artifactPath } = await session.allocateArtifactPath("bash"); if (!artifactPath) throw new Error("Expected artifact path");