From 7bdb16c28d794a5ff8e7485479c8e37eccd9a8d8 Mon Sep 17 00:00:00 2001 From: David Brailovsky Date: Sat, 8 Aug 2026 19:19:17 +0200 Subject: [PATCH] feat(agent): support clearing session names --- .../agent/src/harness/session/jsonl/codec.ts | 5 +++- .../src/harness/session/jsonl/storage.ts | 2 +- packages/agent/src/harness/session/memory.ts | 2 +- packages/agent/src/harness/session/session.ts | 2 +- packages/agent/src/harness/session/state.ts | 2 +- .../harness/session/testing/conformance.ts | 23 +++++++++++++++++++ packages/agent/src/harness/session/types.ts | 6 ++--- .../test/harness/session/jsonl-codec.test.ts | 5 ++-- .../sqlite-node/src/sqlite/repo.ts | 6 ++--- .../src/sqlite/storage/sessions.ts | 6 ++--- .../sqlite-node/test/repository.test.ts | 21 ++++++++++++++++- .../sqlite-node/test/search.test.ts | 21 ++++++----------- 12 files changed, 69 insertions(+), 32 deletions(-) diff --git a/packages/agent/src/harness/session/jsonl/codec.ts b/packages/agent/src/harness/session/jsonl/codec.ts index c2f970fef..84dbeeda0 100644 --- a/packages/agent/src/harness/session/jsonl/codec.ts +++ b/packages/agent/src/harness/session/jsonl/codec.ts @@ -180,7 +180,10 @@ function parseLaneMutation(value: Record, seq: number): Extract function parseFactMutation(value: Record, seq: number): Extract { if (value.fact === "name") { - return { kind: "fact", seq, fact: "name", name: requireString(value.name, "name") }; + if (value.name !== undefined && typeof value.name !== "string") { + throw new JsonlDecodeError("schema", "has invalid name"); + } + return { kind: "fact", seq, fact: "name", name: value.name }; } if (value.fact === "label") { if (value.label !== undefined && typeof value.label !== "string") { diff --git a/packages/agent/src/harness/session/jsonl/storage.ts b/packages/agent/src/harness/session/jsonl/storage.ts index 92f758dd2..3d76cf92e 100644 --- a/packages/agent/src/harness/session/jsonl/storage.ts +++ b/packages/agent/src/harness/session/jsonl/storage.ts @@ -224,7 +224,7 @@ export class JsonlSessionStorage implements SessionStorage return this.state.getName(); } - setName(name: string): Promise { + setName(name: string | undefined): Promise { return this.enqueue(async () => { const mutation: SessionMutation = { kind: "fact", seq: this.state.nextSequence, fact: "name", name }; await this.appendMutation(mutation); diff --git a/packages/agent/src/harness/session/memory.ts b/packages/agent/src/harness/session/memory.ts index 598d8dcf4..cccbd8209 100644 --- a/packages/agent/src/harness/session/memory.ts +++ b/packages/agent/src/harness/session/memory.ts @@ -121,7 +121,7 @@ export class InMemorySessionStorage implements SessionStorage { return this.state.getName(); } - async setName(name: string): Promise { + async setName(name: string | undefined): Promise { this.state.applyMutation({ kind: "fact", seq: this.state.nextSequence, fact: "name", name }); } diff --git a/packages/agent/src/harness/session/session.ts b/packages/agent/src/harness/session/session.ts index f4be8adfa..8027475dd 100644 --- a/packages/agent/src/harness/session/session.ts +++ b/packages/agent/src/harness/session/session.ts @@ -147,7 +147,7 @@ export class Session implem return this.storage.getName(); } - async setName(name: string): Promise { + async setName(name: string | undefined): Promise { await this.storage.setName(name); } diff --git a/packages/agent/src/harness/session/state.ts b/packages/agent/src/harness/session/state.ts index edc16f85b..c63bd5b93 100644 --- a/packages/agent/src/harness/session/state.ts +++ b/packages/agent/src/harness/session/state.ts @@ -18,7 +18,7 @@ export type SessionMutation = | { kind: "entry"; lane?: string; entry: Entry } | { kind: "record"; record: LaneRecord } | { kind: "lane"; seq: number; lane: string; leafId: string | null } - | { kind: "fact"; seq: number; fact: "name"; name: string } + | { kind: "fact"; seq: number; fact: "name"; name: string | undefined } | { kind: "fact"; seq: number; fact: "label"; targetId: string; label: string | undefined }; type InvalidMutation = (message: string) => never; diff --git a/packages/agent/src/harness/session/testing/conformance.ts b/packages/agent/src/harness/session/testing/conformance.ts index be33b2213..9adc10bb9 100644 --- a/packages/agent/src/harness/session/testing/conformance.ts +++ b/packages/agent/src/harness/session/testing/conformance.ts @@ -640,6 +640,29 @@ export function createSessionBackendConformance( }, ), + createCase(factory, "queries and facts", "clears session names durably", async (repository) => { + const session = await repository.create({ id: "session" }); + await session.setName("Temporary"); + await session.setName(undefined); + + strictEqual(await session.getName(), undefined); + deepStrictEqual(await session.getLog(), [ + { kind: "fact", seq: 1, fact: "name", name: "Temporary" }, + { kind: "fact", seq: 2, fact: "name", name: undefined }, + ]); + + const metadata = await session.getMetadata(); + const reopened = await repository.open(metadata); + strictEqual(await reopened.getName(), undefined); + deepStrictEqual(await reopened.getLog(), [ + { kind: "fact", seq: 1, fact: "name", name: "Temporary" }, + { kind: "fact", seq: 2, fact: "name", name: undefined }, + ]); + + const fork = await repository.fork(metadata, { id: "fork" }); + strictEqual(await fork.getName(), undefined); + }), + createCase(factory, "validation and immutability", "returns immutable copies from reads", async (repository) => { const session = await repository.create({ id: "immutable" }); const metadata = await session.getMetadata(); diff --git a/packages/agent/src/harness/session/types.ts b/packages/agent/src/harness/session/types.ts index 5df425f15..1164698ff 100644 --- a/packages/agent/src/harness/session/types.ts +++ b/packages/agent/src/harness/session/types.ts @@ -279,7 +279,7 @@ export type LogItem = | { kind: "entry"; seq: number; entry: Entry } | { kind: "record"; seq: number; record: LaneRecord } | { kind: "lane"; seq: number; lane: string; leafId: string | null } - | { kind: "fact"; seq: number; fact: "name"; name: string } + | { kind: "fact"; seq: number; fact: "name"; name: string | undefined } | { kind: "fact"; seq: number; fact: "label"; targetId: string; label: string | undefined }; export interface LogOptions { @@ -319,7 +319,7 @@ export interface SessionStorage; - setName(name: string): Promise; + setName(name: string | undefined): Promise; getLabel(id: string): Promise; setLabel(id: string, label: string | undefined): Promise; getStats(): Promise; @@ -333,7 +333,7 @@ export interface SessionTree { // Global facts. Latest wins; not branch-scoped. "set", not "append": // append vocabulary is reserved for tree writes. getName(): Promise; - setName(name: string): Promise; + setName(name: string | undefined): Promise; getLabel(targetId: string): Promise; setLabel(targetId: string, label: string | undefined): Promise; diff --git a/packages/agent/test/harness/session/jsonl-codec.test.ts b/packages/agent/test/harness/session/jsonl-codec.test.ts index 4f7621c21..b81767550 100644 --- a/packages/agent/test/harness/session/jsonl-codec.test.ts +++ b/packages/agent/test/harness/session/jsonl-codec.test.ts @@ -134,11 +134,12 @@ describe("JSONL v4 codec", () => { expectMutationRoundTrip({ kind: "lane", seq: 1, lane: "thread", leafId: "entry-1" }); }); - it("round trips both fact line discriminants", () => { + it("round trips fact lines, including cleared values", () => { expectMutationRoundTrip({ kind: "fact", seq: 1, fact: "name", name: "Example" }); + expectMutationRoundTrip({ kind: "fact", seq: 2, fact: "name", name: undefined }); expectMutationRoundTrip({ kind: "fact", - seq: 2, + seq: 3, fact: "label", targetId: "entry-1", label: "checkpoint", diff --git a/packages/session-backends/sqlite-node/src/sqlite/repo.ts b/packages/session-backends/sqlite-node/src/sqlite/repo.ts index da4b13c01..8135d7e32 100644 --- a/packages/session-backends/sqlite-node/src/sqlite/repo.ts +++ b/packages/session-backends/sqlite-node/src/sqlite/repo.ts @@ -606,7 +606,7 @@ class SqliteSessionStorage implements SessionStorage { kind: "fact" as const, seq: row.seq, fact: "name" as const, - name: JSON.parse(row.value ?? "null") as string, + name: row.value === null ? undefined : (JSON.parse(row.value) as string), }; return { kind: "fact" as const, @@ -627,10 +627,10 @@ class SqliteSessionStorage implements SessionStorage { return row?.value === undefined || row.value === null ? undefined : (JSON.parse(row.value) as string); } - async setName(name: string): Promise { + async setName(name: string | undefined): Promise { return this.enqueueWrite(() => { const seq = getNextSequence(this.db, this.metadata.id); - appendFact(this.db, this.metadata.id, seq, "name", null, JSON.stringify(name)); + appendFact(this.db, this.metadata.id, seq, "name", null, name === undefined ? null : JSON.stringify(name)); advanceSequence(this.db, this.metadata.id, seq); }); } diff --git a/packages/session-backends/sqlite-node/src/sqlite/storage/sessions.ts b/packages/session-backends/sqlite-node/src/sqlite/storage/sessions.ts index e25e8a809..f4c54ecf5 100644 --- a/packages/session-backends/sqlite-node/src/sqlite/storage/sessions.ts +++ b/packages/session-backends/sqlite-node/src/sqlite/storage/sessions.ts @@ -98,10 +98,8 @@ export function deleteSessionRow(db: SqliteDatabase, sessionId: string) { sql`DELETE FROM sessions WHERE id = ${sessionId}`.run(db); } -function parseSessionName(value: string | null, sessionId: string): string { - if (value === null) { - throw new SessionError("storage", `Invalid SQLite session ${sessionId}: name must be a string`); - } +function parseSessionName(value: string | null, sessionId: string): string | undefined { + if (value === null) return undefined; let parsed: unknown; try { parsed = JSON.parse(value); diff --git a/packages/session-backends/sqlite-node/test/repository.test.ts b/packages/session-backends/sqlite-node/test/repository.test.ts index 633c5d3c2..1d179c684 100644 --- a/packages/session-backends/sqlite-node/test/repository.test.ts +++ b/packages/session-backends/sqlite-node/test/repository.test.ts @@ -275,7 +275,6 @@ END; it.each([ ["invalid JSON", "not json", "name is not valid JSON"], ["a non-string value", "{}", "name must be a string"], - ["a NULL value", null, "name must be a string"], ])("rejects stored session names containing %s", async (_case, stored, message) => { const root = createTempDir(); const databasePath = join(root, "sessions.sqlite"); @@ -302,6 +301,26 @@ END; }); }); + it("omits a cleared session name from metadata", async () => { + const root = createTempDir(); + const databasePath = join(root, "sessions.sqlite"); + const env = new NodeExecutionEnv({ cwd: root }); + await using repo = new SqliteSessionRepository({ + env, + sqlite: createNodeSqliteFactory(), + databasePath, + }); + const session = await repo.create({ cwd: root, id: "session-1" }); + await session.setName("Temporary"); + expect(await session.getMetadata()).toMatchObject({ name: "Temporary" }); + + await session.setName(undefined); + + expect(await session.getName()).toBeUndefined(); + expect(await session.getMetadata()).not.toHaveProperty("name"); + expect((await repo.list())[0]).not.toHaveProperty("name"); + }); + it("fails loudly when a stored entry is read and cannot be decoded", async () => { const root = createTempDir(); const databasePath = join(root, "sessions.sqlite"); diff --git a/packages/session-backends/sqlite-node/test/search.test.ts b/packages/session-backends/sqlite-node/test/search.test.ts index 7917dd529..f9f5cec4c 100644 --- a/packages/session-backends/sqlite-node/test/search.test.ts +++ b/packages/session-backends/sqlite-node/test/search.test.ts @@ -42,7 +42,7 @@ describe("SQLite FTS5 session search", () => { ]); }); - it("rejects a stored NULL session name", async () => { + it("omits a cleared session name from search metadata", async () => { const root = createTempDir(); const env = new NodeExecutionEnv({ cwd: root }); const sqlite = createNodeSqliteFactory(); @@ -50,20 +50,13 @@ describe("SQLite FTS5 session search", () => { await using fixture = createSqliteFixture({ env, sqlite, databasePath }); const { repository, search } = fixture; const session = await repository.create({ cwd: root, id: "session-1" }); - await session.appendMessage(createUserMessage("Find the auth defect")); - await session.setName("valid name"); + const entryId = await session.appendMessage(createUserMessage("Find the auth defect")); + await session.setName("Temporary"); + await session.setName(undefined); - const db = await sqlite.open(databasePath); - try { - await db.prepare("UPDATE facts SET value = NULL WHERE session_id = ? AND kind = 'name'").run("session-1"); - } finally { - await db.close(); - } - - await expect(search.search({ text: "auth" })).rejects.toMatchObject({ - code: "storage", - message: expect.stringContaining("name must be a string"), - }); + const [result] = await search.search({ text: "auth" }); + expect(result).toMatchObject({ entryId, metadata: { id: "session-1" } }); + expect(result?.metadata).not.toHaveProperty("name"); }); it("handles quoted search text without exposing FTS syntax", async () => {