fix(agent): validate fork entries against named branch ancestry

Reject off-branch and unknown fork points, handle null tips and before-root placement, and make SQLite validate from the named branch tip.
This commit is contained in:
David Brailovsky 2026-09-01 14:58:49 +02:00
parent 435b0c69f5
commit f2eae920c6
5 changed files with 100 additions and 66 deletions

View file

@ -103,21 +103,26 @@ function selectForkContents(
} else {
const sourceTip = sourceTips.find((stored) => stored.address.key === options.branch);
if (sourceTip === undefined) throw new Error(`Unknown source branch: ${options.branch}`);
const requested = options.entryId ?? sourceTip.value;
let tipId = requested;
if (requested !== null) {
const target = sourceEntries.get(requested);
if (target === undefined) throw new Error(`Unknown fork entry: ${requested}`);
if (options.position === "before") tipId = target.parentId;
}
let entryId = tipId;
const requested = options.entryId ?? sourceTip.value;
let found = requested === null;
let tipId: string | null = null;
let entryId = sourceTip.value;
while (entryId !== null) {
const entry = sourceEntries.get(entryId);
if (entry === undefined) throw new Error(`Corrupt source branch: missing parent ${entryId}`);
entryIds.add(entryId);
if (entry.id === requested) {
found = true;
tipId = options.position === "before" ? entry.parentId : entry.id;
if (options.position !== "before") entryIds.add(entry.id);
} else if (found) {
entryIds.add(entry.id);
}
entryId = entry.parentId;
}
if (!found) {
throw new Error(`Fork entry ${requested} is not on source branch ${JSON.stringify(options.branch)}`);
}
destinationTips.set(options.branch, tipId);
}
return { entryIds, destinationTips };

View file

@ -36,6 +36,7 @@ const SIBLING_ID = "00000000-0000-7000-8000-000000000003";
const USAGE_ID = "00000000-0000-7000-8000-000000000004";
const OPERATION_ID = "00000000-0000-7000-8000-000000000005";
const PENDING_ID = "00000000-0000-7000-8000-000000000006";
const UNKNOWN_ID = "00000000-0000-7000-8000-000000000007";
const idleLaneState = {
currentOperationId: null,
lastOperationId: null,
@ -476,59 +477,87 @@ export function createSessionRepoForkBehaviorConformance<TMetadata extends Sessi
await Promise.all([source.close(BACKGROUND_CONTEXT), fork.close(BACKGROUND_CONTEXT)]);
},
),
createCase(
factory,
"forks",
"supports before placement and rejects unknown fork points atomically",
async ({ repo }) => {
const source = await repo.create({ id: "source" }, BACKGROUND_CONTEXT);
await source.mutate(
(mutator) =>
mutator.commit(
[
insertEntry({
id: ROOT_ID,
parentId: null,
type: "custom",
customType: "root",
}),
insertEntry({
id: CHILD_ID,
parentId: ROOT_ID,
type: "custom",
customType: "child",
}),
setValue(branchTip("main"), CHILD_ID),
],
BACKGROUND_CONTEXT,
),
BACKGROUND_CONTEXT,
);
const before = await repo.fork(
source.metadata,
{ id: "before", scope: "branch", branch: "main", entryId: CHILD_ID, position: "before" },
BACKGROUND_CONTEXT,
);
strictEqual(await getBranchTip(before), ROOT_ID);
deepStrictEqual(
(await before.findEntries({ order: "asc" }, BACKGROUND_CONTEXT)).map(({ id }) => id),
[ROOT_ID],
);
await rejects(
repo.fork(
source.metadata,
{ id: "failed", scope: "branch", branch: "main", entryId: SIBLING_ID },
createCase(factory, "forks", "enforces branch ancestry for at and before placement", async ({ repo }) => {
const source = await repo.create({ id: "source" }, BACKGROUND_CONTEXT);
await source.mutate(
(mutator) =>
mutator.commit(
[
insertEntry({ id: ROOT_ID, parentId: null, type: "custom", customType: "root" }),
insertEntry({ id: CHILD_ID, parentId: ROOT_ID, type: "custom", customType: "child" }),
insertEntry({ id: SIBLING_ID, parentId: ROOT_ID, type: "custom", customType: "sibling" }),
setValue(branchTip("main"), CHILD_ID),
setValue(laneConfig("main"), configuration),
setValue(laneState("main"), idleLaneState),
setValue(branchTip("empty"), null),
setValue(laneConfig("empty"), configuration),
setValue(laneState("empty"), idleLaneState),
],
BACKGROUND_CONTEXT,
),
);
deepStrictEqual((await repo.list(undefined, BACKGROUND_CONTEXT)).map(({ id }) => id).sort(), [
"before",
"source",
]);
await Promise.all([source.close(BACKGROUND_CONTEXT), before.close(BACKGROUND_CONTEXT)]);
},
),
BACKGROUND_CONTEXT,
);
const before = await repo.fork(
source.metadata,
{ id: "before", scope: "branch", branch: "main", entryId: CHILD_ID, position: "before" },
BACKGROUND_CONTEXT,
);
strictEqual(await getBranchTip(before), ROOT_ID);
deepStrictEqual(
(await before.findEntries({ order: "asc" }, BACKGROUND_CONTEXT)).map(({ id }) => id),
[ROOT_ID],
);
const mid = await repo.fork(
source.metadata,
{ id: "mid", scope: "branch", branch: "main", entryId: ROOT_ID, position: "at" },
BACKGROUND_CONTEXT,
);
strictEqual(await getBranchTip(mid), ROOT_ID);
deepStrictEqual(
(await mid.findEntries({ order: "asc" }, BACKGROUND_CONTEXT)).map(({ id }) => id),
[ROOT_ID],
);
const beforeRoot = await repo.fork(
source.metadata,
{ id: "before-root", scope: "branch", branch: "main", entryId: ROOT_ID, position: "before" },
BACKGROUND_CONTEXT,
);
strictEqual(await getBranchTip(beforeRoot), null);
deepStrictEqual(await beforeRoot.findEntries({ order: "asc" }, BACKGROUND_CONTEXT), []);
const empty = await repo.fork(
source.metadata,
{ id: "empty", scope: "branch", branch: "empty" },
BACKGROUND_CONTEXT,
);
strictEqual(await getBranchTip(empty, "empty"), null);
deepStrictEqual(await empty.findEntries({ order: "asc" }, BACKGROUND_CONTEXT), []);
for (const [id, branch, entryId] of [
["off-branch", "main", SIBLING_ID],
["unknown", "main", UNKNOWN_ID],
["null-tip", "empty", ROOT_ID],
] as const) {
await rejects(repo.fork(source.metadata, { id, scope: "branch", branch, entryId }, BACKGROUND_CONTEXT));
}
deepStrictEqual((await repo.list(undefined, BACKGROUND_CONTEXT)).map(({ id }) => id).sort(), [
"before",
"before-root",
"empty",
"mid",
"source",
]);
await Promise.all([
source.close(BACKGROUND_CONTEXT),
before.close(BACKGROUND_CONTEXT),
beforeRoot.close(BACKGROUND_CONTEXT),
empty.close(BACKGROUND_CONTEXT),
mid.close(BACKGROUND_CONTEXT),
]);
}),
createCase(factory, "forks", "forks a closed source session", async ({ repo }) => {
const source = await repo.create({ id: "source" }, BACKGROUND_CONTEXT);
await source.mutate(

View file

@ -563,7 +563,7 @@ export type ForkOptions =
scope: "branch";
/** Source Branch to copy under the same name in the destination. */
branch: string;
/** Entry to fork from. Defaults to the source Branch's current tip. */
/** Entry on the source Branch's current tip ancestry. Defaults to the current tip. */
entryId?: string;
/**
* Whether the fork includes the selected entry or stops at its parent.

View file

@ -114,8 +114,9 @@ function readForkSourceEntries(
(stored) => stored.address.namespace === sourceAddress.namespace && stored.address.key === sourceAddress.key,
) as StoredValue<string | null> | undefined;
if (sourceTip === undefined) throw new Error(`Unknown source branch: ${options.branch}`);
const requested = options.entryId ?? sourceTip.value;
return requested === null ? [] : scanBranchEntries(db, sessionId, { start: requested, order: "oldestFirst" });
return sourceTip.value === null
? []
: scanBranchEntries(db, sessionId, { start: sourceTip.value, order: "oldestFirst" });
}
function createSqliteForkSnapshot(

View file

@ -155,10 +155,9 @@ export class SqliteStorage implements Storage {
(stored) => stored.address.namespace === sourceAddress.namespace && stored.address.key === sourceAddress.key,
) as StoredValue<string | null> | undefined;
if (sourceTip === undefined) throw new Error(`Unknown source branch: ${options.branch}`);
const requested = options.entryId ?? sourceTip.value;
return requested === null
return sourceTip.value === null
? []
: scanBranchEntries(this.db, this.sessionId, { start: requested, order: "oldestFirst" });
: scanBranchEntries(this.db, this.sessionId, { start: sourceTip.value, order: "oldestFirst" });
}
private applyCommit(writes: Write[]): CommitResult {