From a79b22622a4def33465a7a16b6c7040c394b76f0 Mon Sep 17 00:00:00 2001 From: David Brailovsky Date: Wed, 5 Aug 2026 16:06:12 +0200 Subject: [PATCH] fix(agent): match coding-agent JSONL session layout Store sessions under cwd-encoded directories with timestamped filenames, scan project directories for listing and explicit-id collisions, and skip global scans for generated UUIDv7 ids. Document and test the shared layout. --- packages/agent/docs/harness-v2.md | 2 + .../agent/src/harness/session/jsonl/repo.ts | 179 ++++++++++-------- .../agent/src/harness/session/jsonl/types.ts | 1 + .../agent/test/harness/session/jsonl.test.ts | 56 +++++- 4 files changed, 151 insertions(+), 87 deletions(-) diff --git a/packages/agent/docs/harness-v2.md b/packages/agent/docs/harness-v2.md index ee16364ba..9c0cd4fdd 100644 --- a/packages/agent/docs/harness-v2.md +++ b/packages/agent/docs/harness-v2.md @@ -1652,6 +1652,8 @@ interface JsonlSessionListOptions { cwd?: string; } A v3 `parentSession` path resolves to the parent header's id when that file is available. If it is unavailable, metadata retains `legacyParentSessionPath`; first-write conversion preserves that optional header field rather than silently dropping the relationship. Format-4 code uses `parentSessionId` for repository relationships. `modifiedAt` is read from the filesystem and is not a sequenced session mutation. +The repository layout matches coding-agent v3. Under `sessionsRoot`, each resolved cwd uses a directory named `--${resolvedCwd.replace(/^[/\\]/, "").replace(/[/\\:]/g, "-")}--`. New files are named `${createdAtIso.replace(/[:.]/g, "-")}_${sessionId}.jsonl`. `list({ cwd })` scans that cwd's directory; `list()` scans every direct child directory. First-write v3 conversion replaces the original file in place and never changes its directory or filename. + One file per session: a header line, then one JSON object per line, in `seq` order. Every logical mutation is exactly one line; a line is the atomic unit. ```text diff --git a/packages/agent/src/harness/session/jsonl/repo.ts b/packages/agent/src/harness/session/jsonl/repo.ts index 1f122193d..f5076fdeb 100644 --- a/packages/agent/src/harness/session/jsonl/repo.ts +++ b/packages/agent/src/harness/session/jsonl/repo.ts @@ -13,6 +13,26 @@ import type { JsonlV4Header, } from "./types.ts"; +const SESSION_ID_PATTERN = /^[A-Za-z0-9](?:[A-Za-z0-9._-]*[A-Za-z0-9])?$/; + +function validateSessionId(id: string): void { + if (!SESSION_ID_PATTERN.test(id)) { + throw new SessionError( + "invalid_payload", + "Session id must be non-empty, contain only alphanumeric characters, '-', '_', and '.', and start and end with an alphanumeric character", + ); + } +} + +function sessionDirectoryName(cwd: string): string { + return `--${cwd.replace(/^[/\\]/, "").replace(/[/\\:]/g, "-")}--`; +} + +function sessionFileName(createdAt: number, id: string): string { + const timestamp = new Date(createdAt).toISOString().replace(/[:.]/g, "-"); + return `${timestamp}_${id}.jsonl`; +} + export class JsonlSessionRepo implements SessionRepo, AsyncDisposable { @@ -31,33 +51,7 @@ export class JsonlSessionRepo } create(options: JsonlSessionCreateOptions): Promise> { - return this.enqueue(async () => { - const id = options.id ?? uuidv7(); - const path = await this.pathForId(id); - if (fileResult(await this.fs.exists(path), `Failed to check session ${path}`)) { - throw new SessionError("already_exists", `Session already exists: ${id}`); - } - const cwd = options.cwd ?? this.cwd; - if (options.metadata !== undefined) assertJsonSerializable(options.metadata); - const header: JsonlV4Header = { - kind: "header", - version: 4, - id, - createdAt: Date.now(), - cwd, - parentSessionId: options.parentSessionId, - metadata: options.metadata, - }; - fileResult( - await this.fs.createDir(await this.root(), { recursive: true }), - `Failed to create sessions directory`, - ); - fileResult(await this.fs.writeFile(path, encodeHeader(header)), `Failed to create session ${path}`); - const fileInfo = fileResult(await this.fs.fileInfo(path), `Failed to read session metadata ${path}`); - const storage = new JsonlSessionStorage(this.fs, metadataFromHeader(header, path, fileInfo.mtimeMs)); - this.storages.set(path, storage); - return new Session(storage); - }); + return this.enqueue(() => this.createDirect(options)); } open(metadata: JsonlSessionMetadata): Promise> { @@ -79,34 +73,7 @@ export class JsonlSessionRepo list(): Promise; list(options: JsonlSessionListOptions): Promise; list(options: JsonlSessionListOptions = {}): Promise { - return this.enqueue(async () => { - const root = await this.root(); - if (!fileResult(await this.fs.exists(root), `Failed to check sessions directory ${root}`)) return []; - const files = fileResult(await this.fs.listDir(root), `Failed to list sessions directory ${root}`).filter( - (entry) => entry.kind !== "directory" && entry.name.endsWith(".jsonl"), - ); - const metadata: JsonlSessionMetadata[] = []; - for (const file of files) { - const existing = this.storages.get(file.path); - if (existing) { - const existingMetadata = await existing.getMetadata(); - if (options.cwd === undefined || existingMetadata.cwd === options.cwd) { - metadata.push({ ...existingMetadata, modifiedAt: file.mtimeMs }); - } - continue; - } - const content = fileResult( - await this.fs.readTextFile(file.path), - `Failed to read session header ${file.path}`, - ); - const firstLine = content.split("\n", 1)[0]; - if (!firstLine) throw invalidFile(file.path, 1, "is missing a header"); - const header = parseHeader(firstLine, file.path); - if (options.cwd !== undefined && header.cwd !== options.cwd) continue; - metadata.push(metadataFromHeader(header, file.path, file.mtimeMs)); - } - return metadata.sort((left, right) => right.modifiedAt - left.modifiedAt); - }); + return this.enqueue(() => this.listDirect(options)); } delete(metadata: JsonlSessionMetadata): Promise { @@ -197,25 +164,33 @@ export class JsonlSessionRepo private async createDirect(options: JsonlSessionCreateOptions): Promise> { const id = options.id ?? uuidv7(); - const path = await this.pathForId(id); - if (fileResult(await this.fs.exists(path), `Failed to check session ${path}`)) { + validateSessionId(id); + // Skip the repository-wide scan for generated UUIDv7 ids because their collision risk is negligible. + if (options.id !== undefined && (await this.sessionIdExists(id))) { throw new SessionError("already_exists", `Session already exists: ${id}`); } - const cwd = options.cwd ?? this.cwd; + + const cwd = fileResult( + await this.fs.absolutePath(options.cwd ?? this.cwd), + `Failed to resolve session cwd ${options.cwd ?? this.cwd}`, + ); + const createdAt = Date.now(); + const sessionDirectory = await this.sessionDirectory(cwd); + const path = fileResult( + await this.fs.joinPath([sessionDirectory, sessionFileName(createdAt, id)]), + `Failed to resolve path for session ${id}`, + ); if (options.metadata !== undefined) assertJsonSerializable(options.metadata); const header: JsonlV4Header = { kind: "header", version: 4, id, - createdAt: Date.now(), + createdAt, cwd, parentSessionId: options.parentSessionId, metadata: options.metadata, }; - fileResult( - await this.fs.createDir(await this.root(), { recursive: true }), - `Failed to create sessions directory`, - ); + fileResult(await this.fs.createDir(sessionDirectory, { recursive: true }), `Failed to create sessions directory`); fileResult(await this.fs.writeFile(path, encodeHeader(header)), `Failed to create session ${path}`); const fileInfo = fileResult(await this.fs.fileInfo(path), `Failed to read session metadata ${path}`); const storage = new JsonlSessionStorage(this.fs, metadataFromHeader(header, path, fileInfo.mtimeMs)); @@ -223,27 +198,71 @@ export class JsonlSessionRepo return new Session(storage); } + private async listDirect(options: JsonlSessionListOptions): Promise { + const directories = await this.sessionDirectories(options.cwd); + const metadata: JsonlSessionMetadata[] = []; + for (const directory of directories) { + const files = fileResult( + await this.fs.listDir(directory), + `Failed to list sessions directory ${directory}`, + ).filter((entry) => entry.kind !== "directory" && entry.name.endsWith(".jsonl")); + for (const file of files) { + const existing = this.storages.get(file.path); + if (existing) { + const existingMetadata = await existing.getMetadata(); + metadata.push({ ...existingMetadata, modifiedAt: file.mtimeMs }); + continue; + } + const content = fileResult( + await this.fs.readTextFile(file.path), + `Failed to read session header ${file.path}`, + ); + const firstLine = content.split("\n", 1)[0]; + if (!firstLine) throw invalidFile(file.path, 1, "is missing a header"); + metadata.push(metadataFromHeader(parseHeader(firstLine, file.path), file.path, file.mtimeMs)); + } + } + return metadata.sort((left, right) => right.modifiedAt - left.modifiedAt); + } + + private async sessionIdExists(id: string): Promise { + for (const storage of this.storages.values()) { + if ((await storage.getMetadata()).id === id) return true; + } + const suffix = `_${id}.jsonl`; + for (const directory of await this.sessionDirectories()) { + const files = fileResult(await this.fs.listDir(directory), `Failed to list sessions directory ${directory}`); + if (files.some((entry) => entry.kind !== "directory" && entry.name.endsWith(suffix))) return true; + } + return false; + } + + private async sessionDirectories(cwd?: string): Promise { + const root = await this.root(); + if (cwd !== undefined) { + const resolvedCwd = fileResult(await this.fs.absolutePath(cwd), `Failed to resolve session cwd ${cwd}`); + const directory = await this.sessionDirectory(resolvedCwd); + return fileResult(await this.fs.exists(directory), `Failed to check sessions directory ${directory}`) + ? [directory] + : []; + } + if (!fileResult(await this.fs.exists(root), `Failed to check sessions directory ${root}`)) return []; + return fileResult(await this.fs.listDir(root), `Failed to list sessions directory ${root}`) + .filter((entry) => entry.kind === "directory" || entry.kind === "symlink") + .map((entry) => entry.path); + } + + private async sessionDirectory(cwd: string): Promise { + return fileResult( + await this.fs.joinPath([await this.root(), sessionDirectoryName(cwd)]), + `Failed to resolve sessions directory for ${cwd}`, + ); + } + private root(): Promise { this.rootPromise ??= this.fs .absolutePath(this.sessionsRootInput) .then((result) => fileResult(result, `Failed to resolve sessions root ${this.sessionsRootInput}`)); return this.rootPromise; } - - private async pathForId(id: string): Promise { - let encoded: string; - try { - encoded = encodeURIComponent(id); - } catch (error) { - throw new SessionError( - "invalid_payload", - `Invalid session id ${JSON.stringify(id)}`, - error instanceof Error ? error : undefined, - ); - } - return fileResult( - await this.fs.joinPath([await this.root(), `session-${encoded}.jsonl`]), - `Failed to resolve path for session ${id}`, - ); - } } diff --git a/packages/agent/src/harness/session/jsonl/types.ts b/packages/agent/src/harness/session/jsonl/types.ts index adf0d6a20..36255b204 100644 --- a/packages/agent/src/harness/session/jsonl/types.ts +++ b/packages/agent/src/harness/session/jsonl/types.ts @@ -18,6 +18,7 @@ export type JsonlSessionRepoFileSystem = Pick< export interface JsonlSessionRepoOptions { fs: JsonlSessionRepoFileSystem; + /** Root containing coding-agent-compatible cwd-encoded session directories. */ sessionsRoot: string; cwd?: string; } diff --git a/packages/agent/test/harness/session/jsonl.test.ts b/packages/agent/test/harness/session/jsonl.test.ts index 7d96aed09..c98a1874e 100644 --- a/packages/agent/test/harness/session/jsonl.test.ts +++ b/packages/agent/test/harness/session/jsonl.test.ts @@ -26,6 +26,12 @@ function createRepository(root: string): JsonlSessionRepo { }); } +function expectedSessionPath(root: string, cwd: string, createdAt: number, id: string): string { + const directory = `--${cwd.replace(/^[/\\]/, "").replace(/[/\\:]/g, "-")}--`; + const timestamp = new Date(createdAt).toISOString().replace(/[:.]/g, "-"); + return join(root, directory, `${timestamp}_${id}.jsonl`); +} + afterEach(() => { while (tempDirs.length > 0) rmSync(tempDirs.pop()!, { recursive: true, force: true }); }); @@ -52,9 +58,10 @@ describe("JSONL v4 persistence", () => { it("exposes the complete metadata contract", async () => { const root = createTempDir(); await using repository = createRepository(root); + const cwd = join(root, "workspace", "project"); const session = await repository.create({ id: "metadata", - cwd: "/workspace/project", + cwd, parentSessionId: "parent", metadata: { owner: "agent", nested: { enabled: true } }, }); @@ -64,22 +71,56 @@ describe("JSONL v4 persistence", () => { id: "metadata", createdAt: expect.any(Number), parentSessionId: "parent", - path: join(root, "session-metadata.jsonl"), - cwd: "/workspace/project", + path: expectedSessionPath(root, metadata.cwd, metadata.createdAt, metadata.id), + cwd, modifiedAt: statSync(metadata.path).mtimeMs, sourceFormat: 4, metadata: { owner: "agent", nested: { enabled: true } }, }); - expect(await repository.list({ cwd: "/workspace/project" })).toEqual([metadata]); - expect(await repository.list({ cwd: "/other/project" })).toEqual([]); + expect(await repository.list({ cwd })).toEqual([metadata]); + expect(await repository.list({ cwd: join(root, "other", "project") })).toEqual([]); + }); + + it("rejects session ids that cannot be used in coding-agent filenames", async () => { + const root = createTempDir(); + await using repository = createRepository(root); + + await expect(repository.create({ id: "../escape", cwd: root })).rejects.toMatchObject({ + code: "invalid_payload", + }); + }); + + it("does not scan existing sessions for generated ids", async () => { + const root = createTempDir(); + const env = new NodeExecutionEnv({ cwd: root }); + let listCalls = 0; + const countingFs = new Proxy(env, { + get(target, property) { + if (property === "listDir") { + return (path: string, abortSignal?: AbortSignal) => { + listCalls++; + return target.listDir(path, abortSignal); + }; + } + const value: unknown = Reflect.get(target, property, target); + return typeof value === "function" ? value.bind(target) : value; + }, + }); + await using repository = new JsonlSessionRepo({ fs: countingFs, sessionsRoot: root, cwd: root }); + + await repository.create({ cwd: root }); + + expect(listCalls).toBe(0); }); it("sorts listed sessions by current filesystem modification time", async () => { const root = createTempDir(); await using repository = createRepository(root); - const newest = await repository.create({ id: "newest", cwd: root }); + const newestCwd = join(root, "workspaces", "newest"); + const oldestCwd = join(root, "workspaces", "oldest"); + const newest = await repository.create({ id: "newest", cwd: newestCwd }); const newestMetadata = await newest.getMetadata(); - const oldest = await repository.create({ id: "oldest", cwd: root }); + const oldest = await repository.create({ id: "oldest", cwd: oldestCwd }); const oldestMetadata = await oldest.getMetadata(); const newestTime = new Date(1_700_000_002_000); const oldestTime = new Date(1_700_000_001_000); @@ -89,6 +130,7 @@ describe("JSONL v4 persistence", () => { const listed = await repository.list(); expect(listed.map((metadata) => metadata.id)).toEqual(["newest", "oldest"]); + expect((await repository.list({ cwd: newestCwd })).map((metadata) => metadata.id)).toEqual(["newest"]); expect(listed.map((metadata) => metadata.modifiedAt)).toEqual([ statSync(newestMetadata.path).mtimeMs, statSync(oldestMetadata.path).mtimeMs,