From 1081eb2126748588c6579df7117be42f40a9892e Mon Sep 17 00:00:00 2001 From: David Brailovsky Date: Tue, 1 Sep 2026 15:18:05 +0200 Subject: [PATCH] fix(agent): require configured lanes for branch forks Reject data-only source branches, preserve tree-scope support for them, and update fork fixtures to provide complete lane configuration and state. --- packages/agent/src/harness/session/fork.ts | 3 ++ .../session/testing/benchmark/session-repo.ts | 16 ++++++++-- .../testing/conformance/session-repo.ts | 23 ++++++++++++-- packages/agent/src/harness/session/types.ts | 6 +++- .../test/harness/jsonl-v3-migration.test.ts | 12 ++----- .../test/harness/memory-session-repo.test.ts | 10 ++++++ .../sqlite-node/test/repo.test.ts | 31 ++++++++++++++----- 7 files changed, 79 insertions(+), 22 deletions(-) diff --git a/packages/agent/src/harness/session/fork.ts b/packages/agent/src/harness/session/fork.ts index 3581a2bfd..810c5d53b 100644 --- a/packages/agent/src/harness/session/fork.ts +++ b/packages/agent/src/harness/session/fork.ts @@ -151,6 +151,9 @@ function validateForkSourceSnapshot( if ((configuration === undefined) !== (state === undefined)) { throw new Error(`Source session branch ${JSON.stringify(tip.address.key)} has incomplete lane state`); } + if (options.scope === "branch" && tip.address.key === options.branch && configuration === undefined) { + throw new Error(`Source branch ${JSON.stringify(options.branch)} is not a configured AgentLane`); + } if ( (source.entriesComplete !== false || options.scope === "tree") && tip.value !== null && diff --git a/packages/agent/src/harness/session/testing/benchmark/session-repo.ts b/packages/agent/src/harness/session/testing/benchmark/session-repo.ts index 7a848f23e..c8f07f920 100644 --- a/packages/agent/src/harness/session/testing/benchmark/session-repo.ts +++ b/packages/agent/src/harness/session/testing/benchmark/session-repo.ts @@ -1,6 +1,6 @@ import { BACKGROUND_CONTEXT } from "../../../context.ts"; import type { SessionMetadata, SessionRepo } from "../../types.ts"; -import { branchTip, setValue } from "../../values.ts"; +import { branchTip, laneConfig, laneState, setValue } from "../../values.ts"; import { STORAGE_BENCHMARK_DATASETS, type StorageBenchmarkDataset } from "./datasets.ts"; import { generateStorageBenchmarkSeedTransactions } from "./storage.ts"; @@ -142,7 +142,19 @@ export async function seedSessionRepoForkBenchmark( await session.mutate((mutator) => mutator.commit(transaction, BACKGROUND_CONTEXT), BACKGROUND_CONTEXT); } await session.mutate( - (mutator) => mutator.commit([setValue(branchTip("main"), dataset.tipId)], BACKGROUND_CONTEXT), + (mutator) => + mutator.commit( + [ + setValue(branchTip("main"), dataset.tipId), + setValue(laneConfig("main"), { + model: { provider: "benchmark", modelId: "benchmark" }, + thinkingLevel: "off", + activeToolNames: [], + }), + setValue(laneState("main"), { currentOperationId: null, lastOperationId: null, inbox: [] }), + ], + BACKGROUND_CONTEXT, + ), BACKGROUND_CONTEXT, ); return session.metadata; diff --git a/packages/agent/src/harness/session/testing/conformance/session-repo.ts b/packages/agent/src/harness/session/testing/conformance/session-repo.ts index 00b637a3f..e7a61e86c 100644 --- a/packages/agent/src/harness/session/testing/conformance/session-repo.ts +++ b/packages/agent/src/harness/session/testing/conformance/session-repo.ts @@ -334,6 +334,21 @@ export function createSessionRepoForkBehaviorConformance { + const source = await repo.create({ id: "source" }, BACKGROUND_CONTEXT); + await source.createBranch("data", null, BACKGROUND_CONTEXT); + + await rejects( + repo.fork(source.metadata, { id: "destination", scope: "branch", branch: "data" }, BACKGROUND_CONTEXT), + ); + deepStrictEqual( + (await repo.list(undefined, BACKGROUND_CONTEXT)).map(({ id }) => id), + ["source"], + ); + + const destination = await repo.create({ id: "destination" }, BACKGROUND_CONTEXT); + await Promise.all([source.close(BACKGROUND_CONTEXT), destination.close(BACKGROUND_CONTEXT)]); + }), createCase( factory, "forks", @@ -571,6 +586,8 @@ export function createSessionRepoForkBehaviorConformance { it("forks a closed source into a complete v4 destination without rewriting it", async () => { const { path, content, metadata } = await writeForkFixture(); - const fork = await repo.fork( - metadata, - { id: "closed-fork", scope: "branch", branch: "main" }, - BACKGROUND_CONTEXT, - ); + const fork = await repo.fork(metadata, { id: "closed-fork", scope: "tree" }, BACKGROUND_CONTEXT); expect(getOrThrow(await fileSystem.readTextFile(path, BACKGROUND_CONTEXT))).toBe(content); expect(fork.metadata.id).toBe("closed-fork"); @@ -313,11 +309,7 @@ describe("JSONL v3 migration", () => { const sourceEntries = await source.findEntries({ order: "asc" }, BACKGROUND_CONTEXT); const sourceStats = await source.getStats(BACKGROUND_CONTEXT); - const fork = await repo.fork( - metadata, - { id: "open-fork", scope: "branch", branch: "main" }, - BACKGROUND_CONTEXT, - ); + const fork = await repo.fork(metadata, { id: "open-fork", scope: "tree" }, BACKGROUND_CONTEXT); const forkEntries = await expectForkedState(fork); expect(fork.metadata.id).toBe("open-fork"); diff --git a/packages/agent/test/harness/memory-session-repo.test.ts b/packages/agent/test/harness/memory-session-repo.test.ts index fcf8fca77..1cd85a09c 100644 --- a/packages/agent/test/harness/memory-session-repo.test.ts +++ b/packages/agent/test/harness/memory-session-repo.test.ts @@ -98,6 +98,16 @@ describe("MemorySessionRepo metadata", () => { customType: "child", }), storedValues.setValue(storedValues.branchTip("main"), childId), + storedValues.setValue(storedValues.laneConfig("main"), { + model: { provider: "test", modelId: "test" }, + thinkingLevel: "off", + activeToolNames: [], + }), + storedValues.setValue(storedValues.laneState("main"), { + currentOperationId: null, + lastOperationId: null, + inbox: [], + }), ], BACKGROUND_CONTEXT, ); diff --git a/packages/session-backends/sqlite-node/test/repo.test.ts b/packages/session-backends/sqlite-node/test/repo.test.ts index b92d21ca0..f97ea0c11 100644 --- a/packages/session-backends/sqlite-node/test/repo.test.ts +++ b/packages/session-backends/sqlite-node/test/repo.test.ts @@ -8,6 +8,17 @@ import { describe, expect, it } from "vitest"; import type { SqliteDatabase, SqliteDatabaseFactory, SqliteStatement } from "../src/index.ts"; import { createNodeSqliteFactory, SqliteSessionRepo, sql } from "../src/index.ts"; +const TEST_LANE_CONFIGURATION = { + model: { provider: "test", modelId: "test" }, + thinkingLevel: "off", + activeToolNames: [], +} satisfies storedValues.LaneConfiguration; +const IDLE_LANE_STATE = { + currentOperationId: null, + lastOperationId: null, + inbox: [], +} satisfies storedValues.LaneState; + async function withTempDir(run: (directory: string) => Promise): Promise { const directory = await mkdtemp(join(tmpdir(), "pi-sqlite-session-")); try { @@ -202,23 +213,23 @@ function commitLaterSourceState(db: SqliteDatabase): void { const label = storedValues.entryLabel("root"); db.transaction(() => { sql`INSERT INTO entries (session_id, id, parent_id, seq, type, custom_type, timestamp, payload) - VALUES (${"source"}, ${"child"}, ${"root"}, ${5}, ${"message"}, ${null}, ${2}, ${JSON.stringify({ message: { role: "user", content: "after", timestamp: 2 } })})`.run( + VALUES (${"source"}, ${"child"}, ${"root"}, ${7}, ${"message"}, ${null}, ${2}, ${JSON.stringify({ message: { role: "user", content: "after", timestamp: 2 } })})`.run( db, ); sql`INSERT INTO branch_entries (session_id, branch_id, entry_id, entry_seq, entry_type) - VALUES (${"source"}, ${"root"}, ${"child"}, ${5}, ${"message"})`.run(db); - sql`UPDATE branch_meta SET tip_entry_id = ${"child"}, tip_seq = ${5} + VALUES (${"source"}, ${"root"}, ${"child"}, ${7}, ${"message"})`.run(db); + sql`UPDATE branch_meta SET tip_entry_id = ${"child"}, tip_seq = ${7} WHERE session_id = ${"source"} AND branch_id = ${"root"}`.run(db); for (const [namespace, key, seq, value] of [ - [tip.namespace, tip.key, 6, "child"], - [name.namespace, name.key, 7, "after"], - [label.namespace, label.key, 8, "after-label"], + [tip.namespace, tip.key, 8, "child"], + [name.namespace, name.key, 9, "after"], + [label.namespace, label.key, 10, "after-label"], ] as const) { sql`INSERT INTO scalar_values (session_id, namespace, key, seq, value) VALUES (${"source"}, ${namespace}, ${key}, ${seq}, ${JSON.stringify(value)}) ON CONFLICT(session_id, namespace, key) DO UPDATE SET seq = excluded.seq, value = excluded.value`.run(db); } - sql`UPDATE sessions SET message_count = ${1}, next_seq = ${9} WHERE id = ${"source"}`.run(db); + sql`UPDATE sessions SET message_count = ${1}, next_seq = ${11} WHERE id = ${"source"}`.run(db); }); } @@ -558,6 +569,8 @@ describe("SqliteSessionRepo", () => { message: { role: "user", content: "child", timestamp: 1 }, }), storedValues.setValue(storedValues.branchTip("main"), "child"), + storedValues.setValue(storedValues.laneConfig("main"), TEST_LANE_CONFIGURATION), + storedValues.setValue(storedValues.laneState("main"), IDLE_LANE_STATE), sessionWrites.insertUsage({ id: "usage", adjustment: false, @@ -756,6 +769,8 @@ describe("SqliteSessionRepo", () => { [ sessionWrites.insertEntry({ id: "left-root", parentId: null, type: "custom", customType: "left" }), storedValues.setValue(storedValues.branchTip("main"), "left-root"), + storedValues.setValue(storedValues.laneConfig("main"), TEST_LANE_CONFIGURATION), + storedValues.setValue(storedValues.laneState("main"), IDLE_LANE_STATE), storedValues.setValue(storedValues.sessionName, "left"), ], BACKGROUND_CONTEXT, @@ -813,6 +828,8 @@ describe("SqliteSessionRepo", () => { [ sessionWrites.insertEntry({ id: "root", parentId: null, type: "custom", customType: "root" }), storedValues.setValue(storedValues.branchTip("main"), "root"), + storedValues.setValue(storedValues.laneConfig("main"), TEST_LANE_CONFIGURATION), + storedValues.setValue(storedValues.laneState("main"), IDLE_LANE_STATE), storedValues.setValue(storedValues.sessionName, "before"), storedValues.setValue(storedValues.entryLabel("root"), "before-label"), ],