From 518078331a6c6e2d2600e15bafc92c31b4fca36e Mon Sep 17 00:00:00 2001 From: mchwang Date: Tue, 6 Oct 2026 14:32:15 -0700 Subject: [PATCH 01/11] Add foreign rebase conflict boundary --- core/approvals.ts | 4 +- core/linking.ts | 18 ++-- docs/implementation/pre-merge-rebase.md | 14 ++- runner/production.ts | 2 +- runner/rebase.ts | 118 +++++++++++++++++++++--- runner/recovery.ts | 16 +++- runner/review.ts | 4 +- runner/store.ts | 45 +++++++-- test/browser/review.spec.ts | 11 +++ test/history.test.ts | 16 ++++ test/runner-lifecycle-store.test.ts | 14 ++- test/runner-rebase.test.ts | 80 +++++++++++++++- test/runner-recovery.test.ts | 30 +++++- test/store.test.ts | 20 +++- web/public/app.js | 8 +- 15 files changed, 343 insertions(+), 57 deletions(-) diff --git a/core/approvals.ts b/core/approvals.ts index c1480e1e..5c9ca53b 100644 --- a/core/approvals.ts +++ b/core/approvals.ts @@ -9,7 +9,8 @@ export function stable(value: unknown): string { if (value !== null && typeof value === 'object') return `{${Object.entries(value).sort(([a], [b]) => a.localeCompare(b)).map(([key, val]) => `${JSON.stringify(key)}:${stable(val)}`).join(',')}}`; return JSON.stringify(value); } -const contentKey = (s: Segment) => stable({ path: s.path, oldPath: s.oldPath, kind: s.kind, operation: s.operation, content: s.content }); +const contentKey = (s: Segment) => stable({ path: s.path, oldPath: s.oldPath, kind: s.kind, operation: s.operation, + content: s.content, ...(s.conflictResolved ? { conflictResolved: true } : {}) }); export interface SegmentChoice { key: string; action: 'assign' | 'accept'; item: string | null } /** Position among identical segments and total copies prevent approval transfer. */ export function choiceKeys(segments: readonly Segment[], identity: PlanIdentity): string[] { @@ -37,6 +38,7 @@ export interface Approval { item: string; fingerprint: string } export const reviewedSegment = (s: Segment) => ({ path: s.path, oldPath: s.oldPath, kind: s.kind, operation: s.operation, content: s.content, context: s.context, owners: [...s.owners].sort(), + ...(s.conflictResolved ? { conflictResolved: true } : {}), }); export function fingerprint(item: PlanItem, segments: readonly Segment[], identity: PlanIdentity): string { return stable({ identity: identityKey(identity), item, segments: segments.filter(s => s.row === item.id).map(reviewedSegment) }); diff --git a/core/linking.ts b/core/linking.ts index 945eab8f..3d7f7e5e 100644 --- a/core/linking.ts +++ b/core/linking.ts @@ -17,8 +17,10 @@ export interface Segment { row: string; scope: 'in-scope' | 'out-of-scope' | 'unplanned' | 'ambiguous'; oldLine: number | null; newLine: number | null; operation: '+' | '-' | null; content: string; context: string; hunk: number; sharesHunkWith: string[]; + /** At least one contributing foreign commit was resolved by the sandboxed rebase agent. */ + conflictResolved?: boolean; } -interface Evidence { owners: (string | null)[]; outOfScope: string[] } +interface Evidence { owners: (string | null)[]; outOfScope: string[]; conflictResolved: boolean } interface TrackedLine { text: string; evidence: Evidence; origins: string[]; moved: Evidence } interface TrackedFile { lines: TrackedLine[]; metadata: Evidence; metadataPaths: string[] } const unique = (values: T[]): T[] => [...new Set(values)]; @@ -30,7 +32,7 @@ function metadataChange(delta: FileDelta): boolean { return delta.oldPath !== delta.newPath || delta.before?.mode !== delta.after?.mode || !textFile(delta.before) || !textFile(delta.after); } -const empty = (): Evidence => ({ owners: [], outOfScope: [] }); +const empty = (): Evidence => ({ owners: [], outOfScope: [], conflictResolved: false }); function classify(evidence: Evidence): Pick { const { owners, outOfScope } = evidence; if (!owners.length || owners.includes(null)) return { row: 'Unplanned', scope: 'unplanned' }; @@ -42,7 +44,8 @@ function classify(evidence: Evidence): Pick { export interface LinkingLimits { maxLines?: number; maxSegments?: number; maxReferences?: number; maxDurationMs?: number } /** Replays a linear history. Commit messages and Plan-Item trailers are never trusted. */ -export function linkHistory(plan: Plan, history: History, ledger: ReadonlyMap, pathKey: (path: string) => string, limits: LinkingLimits = {}): Segment[] { +export function linkHistory(plan: Plan, history: History, ledger: ReadonlyMap, pathKey: (path: string) => string, + limits: LinkingLimits = {}, resolvedConflicts: ReadonlySet = new Set()): Segment[] { if (typeof pathKey !== 'function') throw new Error('Known checkout path identity is required.'); const budget = (value: number | undefined, ceiling: number, name: string) => { const limit = value ?? ceiling; @@ -87,7 +90,7 @@ export function linkHistory(plan: Plan, history: History, ledger: ReadonlyMap entry.conflictResolved) }; }; const mergeOrigins = (tracked: readonly TrackedLine[]): string[] => { const origins = new Set(); @@ -111,7 +114,8 @@ export function linkHistory(plan: Plan, history: History, ledger: ReadonlyMap item.id === owner); const declared = new Set(item?.files.flatMap(file => [file.path, ...(file.renamed_from ? [file.renamed_from] : [])]).map(pathKey)); const touched = [delta.oldPath, delta.newPath].filter((path): path is string => path !== null); - const current: Evidence = { owners: [owner], outOfScope: owner !== null && touched.some(path => !declared.has(pathKey(path))) ? [owner] : [] }; + const current: Evidence = { owners: [owner], outOfScope: owner !== null && touched.some(path => !declared.has(pathKey(path))) ? [owner] : [], + conflictResolved: owner === null && resolvedConflicts.has(commit.sha) }; let previous = oldPath ? files.get(oldPath) : undefined; if (!previous) previous = { lines: lines(textFile(delta.before) ? delta.before!.text : '').map((text, i) => { @@ -170,7 +174,8 @@ export function linkHistory(plan: Plan, history: History, ledger: ReadonlyMap, evidence: Evidence) => { remaining(); if (++segmentCount > maxSegments) throw new Error('Linking exceeds its cumulative segment budget.'); - fileSegments.push({ ...part, owners: evidence.owners, ...classify(evidence), sharesHunkWith: [] }); + fileSegments.push({ ...part, owners: evidence.owners, ...classify(evidence), sharesHunkWith: [], + ...(evidence.conflictResolved ? { conflictResolved: true } : {}) }); }; if (metadataChange(delta)) { const evidence = combine(affectedPaths.map(path => metadata.get(path) ?? empty())); @@ -213,6 +218,7 @@ export function linkHistory(plan: Plan, history: History, ledger: ReadonlyMap service.store.setRebaseProcessGroup(rebasePlanKey, attemptId, 'spawning', group), onProcessGroupSettled: (attemptId, group) => service.store.setRebaseProcessGroup(rebasePlanKey, attemptId, group, null), onProcessUnsettled: (attemptId, group) => service.store.setRebaseProcessGroup(rebasePlanKey, attemptId, group, 'unsettled'), - onResultPrepared: (attemptId, head, history) => service.store.prepareRebaseResult(rebasePlanKey, attemptId, head, history), + onResultPrepared: (attemptId, head, history, conflicts) => service.store.prepareRebaseResult(rebasePlanKey, attemptId, head, history, conflicts), onResultState: (attemptId, state) => state === 'ready' ? service.store.completeRebaseResult(rebasePlanKey, attemptId) : service.store.setRebaseResultState(rebasePlanKey, attemptId, state) })); diff --git a/runner/rebase.ts b/runner/rebase.ts index 34bb4897..c5791b64 100644 --- a/runner/rebase.ts +++ b/runner/rebase.ts @@ -16,7 +16,19 @@ const CLEANUP_RESERVE_MS = 30_000; const MIN_REBASE_TIMEOUT_MS = CLEANUP_RESERVE_MS + PROCESS_SETTLEMENT_RESERVE_MS + 1; export interface RebaseMapping { oldSha: string; newSha: string } -export interface RebaseResult { oldHead: string; base: string; head: string; mappings: RebaseMapping[] } +export interface RebaseConflictInput { + readonly attemptId: string; + readonly commit: string; + readonly files: readonly string[]; + /** The isolated runner-owned worktree. A production resolver must expose it only through lane D's bounded storage. */ + readonly repository: string; + readonly signal?: AbortSignal; +} +export interface RebaseResult { + oldHead: string; base: string; head: string; mappings: RebaseMapping[]; + /** Source commits whose foreign conflicts were resolved by the configured agent boundary. */ + resolvedConflicts: string[]; +} export interface GitRebaserOptions { repository: RunnerRepository; runnerRoot: string; @@ -27,8 +39,11 @@ export interface GitRebaserOptions { onProcessGroup: (attemptId: string, group: ProcessGroup) => void; onProcessGroupSettled: (attemptId: string, group: ProcessGroup | 'spawning') => void; onProcessUnsettled: (attemptId: string, group: ProcessGroup) => void; - onResultPrepared: (attemptId: string, intendedHead: string | null, rewrittenHistory: readonly string[]) => void; + onResultPrepared: (attemptId: string, intendedHead: string | null, rewrittenHistory: readonly string[], + resolvedConflicts: readonly string[]) => void; onResultState: (attemptId: string, state: 'uncertain' | 'refused' | 'ready') => void; + /** F4's sandboxed resolver. It must settle all owned resources before resolving or rejecting. */ + resolveForeignConflict?: (input: RebaseConflictInput) => Promise; } interface CallScope { attemptId: string; signal?: AbortSignal; deadline: number; workDeadline: number } @@ -84,10 +99,18 @@ export class GitRebaser { this.#root = ownerOnlyDirectory(options.runnerRoot, options.runnerOwner, 'rebases'); } - async run(input: { attemptId: string; oldBase: string; oldHead: string; oldHistory: readonly string[]; onto: string; signal?: AbortSignal }): Promise { + async run(input: { attemptId: string; oldBase: string; oldHead: string; oldHistory: readonly string[]; onto: string; + ledger?: readonly { sha: string; owner: string | null; origin: 'owned' | 'foreign' }[]; signal?: AbortSignal }): Promise { const { attemptId, oldBase, oldHead, oldHistory, onto, signal } = input; rebaseRef(attemptId); for (const value of [oldBase, oldHead, onto]) if (!COMMIT_ID.test(value)) throw new Error('A full commit ID is required for rebasing.'); + const ledger = new Map(); + for (const entry of input.ledger ?? []) { + if (!COMMIT_ID.test(entry.sha) || ledger.has(entry.sha) || + (entry.origin === 'owned' ? typeof entry.owner !== 'string' || entry.owner.length === 0 : entry.owner !== null)) + throw new Error('The trusted commit ledger is invalid.'); + ledger.set(entry.sha, { owner: entry.owner, origin: entry.origin }); + } signal?.throwIfAborted(); const scope = this.#scope(attemptId, signal); const ref = rebaseRef(attemptId); @@ -101,17 +124,18 @@ export class GitRebaser { if (onto === oldBase) { const mappings = old.map(sha => ({ oldSha: sha, newSha: sha })); this.#assertResultCurrent(scope); - this.#options.onResultPrepared(attemptId, null, old); + this.#options.onResultPrepared(attemptId, null, old, []); this.#assertResultCurrent(scope); this.#options.onResultState(attemptId, 'ready'); this.#assertResultCurrent(scope); - return { oldHead, base: onto, head: oldHead, mappings }; + return { oldHead, base: onto, head: oldHead, mappings, resolvedConflicts: [] }; } const path = this.#path(attemptId); if (lstatSync(path, { throwIfNoEntry: false })) throw new Error('The rebase workspace already exists.'); let added = false, retained = false, uncertainRef = false, attemptedRef: string | undefined; let result: RebaseResult | undefined, primary: unknown; + const resolvedConflicts: string[] = []; try { await this.#git(this.#options.repository.path, ['worktree', 'add', '--detach', '--', path, oldHead], scope); added = true; @@ -119,16 +143,48 @@ export class GitRebaser { // case-colliding or normalization-colliding names on default macOS volumes). Hash the files into the index and // compare trees; status/stat-cache metadata is not proof that the contents are unchanged. await this.#assertCheckout(path, oldHead, 'reviewed head', scope); - const outcome = await this.#call(path, ['rebase', '--quiet', '--reapply-cherry-picks', '--keep-empty', '--empty=keep', '--onto', onto, oldBase, oldHead], scope, false, true); - this.#throwIfCancelled(outcome, scope); - if (outcome.status !== 0) { + let outcome = await this.#call(path, ['rebase', '--quiet', '--reapply-cherry-picks', '--keep-empty', '--empty=keep', '--onto', onto, oldBase, oldHead], scope, false, true); + for (;;) { + this.#throwIfCancelled(outcome, scope); + if (outcome.status === 0) break; if (outcome.status === null || outcome.error) throw this.#failure('rebase', outcome); // Rebase diagnostics can grow with valid commit messages and path counts. The index, not bounded text, proves a conflict. const conflicted = await this.#call(path, ['diff', '--quiet', '--ignore-submodules=all', '--diff-filter=U', '--'], scope); this.#throwIfCancelled(conflicted, scope); - if (conflicted.status === 1) throw new RebaseConflict('The rebase has conflicts and needs review.'); - if (conflicted.status !== 0) throw this.#failure('diff', conflicted); - throw this.#failure('rebase', outcome); + if (conflicted.status !== 1) { + if (conflicted.status !== 0) throw this.#failure('diff', conflicted); + throw this.#failure('rebase', outcome); + } + const commit = await this.#git(path, ['rev-parse', '--verify', 'REBASE_HEAD^{commit}'], scope); + if (!old.includes(commit)) throw new RebaseConflict('The conflicted commit is outside the captured rebase history.'); + const entry = ledger.get(commit); + // Missing ledger entries and explicit foreign entries share the foreign branch. A forged trailer is never read. + if (entry && (entry.origin !== 'foreign' || entry.owner !== null)) + throw new RebaseConflict('An owned commit conflict needs its plan-item resolver.'); + if (!this.#options.resolveForeignConflict) + throw new RebaseConflict('A foreign commit conflict needs review.'); + const files = Object.freeze(await this.#paths(path, ['diff', '--name-only', '--diff-filter=U', '-z', '--'], scope)); + if (!files.length) throw new RebaseConflict('Git reported a conflict without any conflicted files.'); + const allowed = new Set(files); + this.#assertResultCurrent(scope); + await this.#resolveConflict({ attemptId, commit, files, repository: path }, scope); + this.#assertResultCurrent(scope); + if (await this.#git(path, ['rev-parse', '--verify', 'REBASE_HEAD^{commit}'], scope) !== commit) + throw new RebaseConflict('The conflict resolver changed the rebase operation.'); + const changed = [...await this.#paths(path, ['diff', '--name-only', '-z', 'HEAD', '--'], scope), + ...await this.#paths(path, ['ls-files', '--others', '-z', '--'], scope)]; + if (changed.some(file => !allowed.has(file))) + throw new RebaseConflict('The conflict resolver changed a file outside the conflicted set.'); + const pathspec = Buffer.from(`${files.join('\0')}\0`); + await this.#git(path, ['--literal-pathspecs', 'add', '--all', '--pathspec-from-file=-', '--pathspec-file-nul'], scope, false, pathspec); + const unresolved = await this.#call(path, ['diff', '--quiet', '--ignore-submodules=all', '--diff-filter=U', '--'], scope); + this.#throwIfCancelled(unresolved, scope); + if (unresolved.status !== 0) { + if (unresolved.status === 1) throw new RebaseConflict('The conflict resolver left unresolved files.'); + throw this.#failure('diff', unresolved); + } + resolvedConflicts.push(commit); + outcome = await this.#call(path, ['rebase', '--continue'], scope, false, true); } const head = await this.#git(path, ['rev-parse', '--verify', 'HEAD^{commit}'], scope); if (!COMMIT_ID.test(head)) throw new Error('Git returned an invalid rebased head.'); @@ -138,7 +194,7 @@ export class GitRebaser { const mappings = old.map((oldSha, index) => ({ oldSha, newSha: next[index]! })); attemptedRef = head; this.#assertResultCurrent(scope); - this.#options.onResultPrepared(attemptId, head, next); + this.#options.onResultPrepared(attemptId, head, next, resolvedConflicts); this.#assertResultCurrent(scope); const created = await this.#call(this.#options.repository.path, ['update-ref', ref, head, '0'.repeat(head.length)], scope); if (created.status === 0) retained = true; @@ -155,7 +211,7 @@ export class GitRebaser { this.#assertResultCurrent(scope); this.#options.onResultState(attemptId, 'ready'); this.#assertResultCurrent(scope); - result = { oldHead, base: onto, head, mappings }; + result = { oldHead, base: onto, head, mappings, resolvedConflicts }; } catch (error) { primary = error; } const cleanupFailures: unknown[] = []; if (added || lstatSync(path, { throwIfNoEntry: false })) { @@ -269,8 +325,40 @@ export class GitRebaser { await this.#git(this.#options.repository.path, ['worktree', 'prune'], scope, true); } - async #git(repository: string, args: readonly string[], scope: CallScope, cleanup = false): Promise { - const outcome = await this.#call(repository, args, scope, cleanup); + async #paths(repository: string, args: readonly string[], scope: CallScope): Promise { + const outcome = await this.#call(repository, args, scope); + this.#throwIfCancelled(outcome, scope); + if (outcome.status !== 0) throw this.#failure(args[0] ?? 'git', outcome); + // Lane D's path APIs are strings. Refuse undecodable names rather than let UTF-8 replacement alias two paths. + if (outcome.stdout.includes('\ufffd') || (outcome.stdout && !outcome.stdout.endsWith('\0'))) + throw new RebaseConflict('A conflicted path cannot be represented safely; resolve it manually.'); + const paths = outcome.stdout ? outcome.stdout.slice(0, -1).split('\0') : []; + if (paths.some(path => !path || path.includes('\0'))) throw new RebaseConflict('Git returned an invalid conflicted path.'); + return paths; + } + + async #resolveConflict(input: Omit, scope: CallScope): Promise { + const resolveConflict = this.#options.resolveForeignConflict!; + const controller = new AbortController(); + const abort = () => controller.abort(scope.signal?.reason ?? new Error('The rebase was cancelled.')); + if (scope.signal?.aborted) abort(); + else scope.signal?.addEventListener('abort', abort, { once: true }); + const timeoutMs = Math.max(0, Math.ceil(scope.workDeadline - performance.now())); + const timer = setTimeout(() => controller.abort(Object.assign(new Error('The rebase deadline expired during conflict resolution.'), + { code: 'ETIMEDOUT' })), timeoutMs); + try { + controller.signal.throwIfAborted(); + // Await settlement after abort: the resolver owns its container/storage until its promise ends. + await resolveConflict(Object.freeze({ ...input, signal: controller.signal })); + controller.signal.throwIfAborted(); + } finally { + clearTimeout(timer); + scope.signal?.removeEventListener('abort', abort); + } + } + + async #git(repository: string, args: readonly string[], scope: CallScope, cleanup = false, input?: Buffer): Promise { + const outcome = await this.#call(repository, args, scope, cleanup, false, input); if (!cleanup) this.#throwIfCancelled(outcome, scope); if (outcome.status !== 0) throw this.#failure(args[0] ?? 'git', outcome); return outcome.stdout.trim(); diff --git a/runner/recovery.ts b/runner/recovery.ts index a85c7993..f8036b99 100644 --- a/runner/recovery.ts +++ b/runner/recovery.ts @@ -185,6 +185,14 @@ function validRebaseResult(marker: Partial, history: unknown, head return !!endpoint && endpoint.oldSha === marker.oldHead && (head === null ? endpoint.newSha === marker.oldHead && mappings.every(mapping => mapping.oldSha === mapping.newSha) : endpoint.newSha === head); } +function validResolvedConflicts(history: unknown, mappings: unknown, conflicts: unknown): boolean { + if (!Array.isArray(conflicts) || conflicts.length > 500) return false; + if (history === null) return conflicts.length === 0; + if (!Array.isArray(history) || !Array.isArray(mappings)) return false; + const sources = new Map((mappings as { oldSha?: unknown; newSha?: unknown }[]).map(mapping => [mapping.oldSha, mapping.newSha])); + return new Set(conflicts).size === conflicts.length && conflicts.every(value => typeof value === 'string' && + /^(?:[0-9a-f]{40}|[0-9a-f]{64})$/.test(value) && history.includes(value) && sources.get(value) !== value); +} const EXPORT_LIMIT = 1024 * 1024; /** @@ -241,7 +249,8 @@ export async function recoverStartup(o: RecoveryOptions): Promise entry.conflictResolved).map(entry => entry.sha))); const saved = this.store.getReview(identity), keys = choiceKeys(raw, identity); const deltas = new Map(history.final.map(file => [JSON.stringify([file.newPath ?? file.oldPath, file.oldPath]), file])); let previewBudget = 6 * 1024 * 1024; diff --git a/runner/store.ts b/runner/store.ts index 680352b1..24258439 100644 --- a/runner/store.ts +++ b/runner/store.ts @@ -93,6 +93,8 @@ export interface RebaseMarker { resultState: 'none' | 'prepared' | 'uncertain' | 'refused' | 'ready'; resultHead: string | null; resultMappings: { oldSha: string; newSha: string }[] | null; + /** Source commits whose foreign conflicts were resolved by the sandboxed F4 agent. */ + resolvedConflicts: string[]; processGroup: { pgid: number; startedAt: number; identity: string | null } | 'spawning' | 'unsettled' | null; } export interface AttemptRecord { @@ -118,7 +120,11 @@ export interface FeedbackEvent { id: string; planKey: string; actionId: string; planRevision: number; snapshotId: string | null; item: string | null; kind: FeedbackKind; text: string | null; sourceRef: string; supersedes: string | null; createdAt: string; } -export interface LedgerEntry { sha: string; owner: string | null; origin: 'owned' | 'foreign'; sourceSha: string | null } +export interface LedgerEntry { + sha: string; owner: string | null; origin: 'owned' | 'foreign'; sourceSha: string | null; + /** True only on a rewritten foreign commit whose conflict a sandboxed agent resolved. */ + conflictResolved?: boolean; +} export interface Checkpoint { id: string; revision: number; snapshotId: string; item: string; /** Runner-audited actual tree, retained separately from the declared plan. */ @@ -677,10 +683,13 @@ export class Store { #entry(key: string, entry: LedgerEntry): void { sha(entry.sha); if (entry.sourceSha !== null) sha(entry.sourceSha); if ((entry.origin === 'foreign' && entry.owner !== null) || (entry.origin === 'owned' && !entry.owner) || !['foreign', 'owned'].includes(entry.origin)) throw new Error('Invalid ledger ownership.'); + if (entry.conflictResolved !== undefined && entry.conflictResolved !== true) throw new Error('Invalid conflict-resolution provenance.'); + if (entry.conflictResolved && entry.origin !== 'foreign') throw new Error('Only a foreign ledger entry can carry conflict-resolution provenance.'); const existing = this.#get('SELECT data FROM ledger WHERE key=? AND sha=?', key, entry.sha); if (existing) { const prior = decode(existing.data); - if (prior.sha !== entry.sha || prior.owner !== entry.owner || prior.origin !== entry.origin || prior.sourceSha !== entry.sourceSha) + if (prior.sha !== entry.sha || prior.owner !== entry.owner || prior.origin !== entry.origin || prior.sourceSha !== entry.sourceSha || + !!prior.conflictResolved !== !!entry.conflictResolved) throw new Error('Cannot overwrite immutable ledger ownership.'); return; } @@ -736,7 +745,7 @@ export class Store { assertUuidV4(attemptId, 'Rebase attempt ID'); if (!Number.isSafeInteger(startedAt) || startedAt < 0) throw new Error('Invalid rebase start time.'); const marker: RebaseMarker = { attemptId, oldBase: input.oldBase, oldHead: input.oldHead, onto: input.onto, oldHistory, startedAt, - resultState: 'none', resultHead: null, resultMappings: null, processGroup: null }; + resultState: 'none', resultHead: null, resultMappings: null, resolvedConflicts: [], processGroup: null }; const key = identityKey(identity); return this.#transaction(() => { this.#expect(key, expected); @@ -774,17 +783,22 @@ export class Store { }); } /** Persist the complete intended result before the rebaser's first ref write. */ - prepareRebaseResult(planKey: string, attemptId: string, head: string | null, rewrittenHistory: readonly string[]): void { + prepareRebaseResult(planKey: string, attemptId: string, head: string | null, rewrittenHistory: readonly string[], + resolvedConflicts: readonly string[] = []): void { assertUuidV4(attemptId, 'Rebase attempt ID'); if (head !== null) sha(head); if (rewrittenHistory.length > 500) throw new Error('Rebase history exceeds 500 commits.'); for (const value of rewrittenHistory) sha(value); if (new Set(rewrittenHistory).size !== rewrittenHistory.length) throw new Error('Rebase history must not repeat a commit.'); + for (const value of resolvedConflicts) sha(value); + if (new Set(resolvedConflicts).size !== resolvedConflicts.length) throw new Error('Resolved conflicts must not repeat a commit.'); this.#transaction(() => { const task = this.#task(planKey), marker = task.rebase_in_progress === null ? null : decode(task.rebase_in_progress); if (marker?.attemptId !== attemptId) throw new GuardRefusal('This rebase attempt no longer owns the task.'); if (marker.processGroup !== null) throw new GuardRefusal('The rebase Git process has not settled.'); if (marker.oldHistory === null || rewrittenHistory.length !== marker.oldHistory.length) throw new GuardRefusal('The rebase result does not cover the complete captured history.'); + if (resolvedConflicts.some(value => !marker.oldHistory!.includes(value))) + throw new GuardRefusal('A resolved conflict is outside the captured history.'); if ((marker.onto === marker.oldBase) !== (head === null)) throw new GuardRefusal('The retained rebase result does not match its target.'); const rewrittenHead = rewrittenHistory.at(-1); if (marker.oldHead === marker.oldBase ? rewrittenHistory.length !== 0 : @@ -793,13 +807,23 @@ export class Store { if (marker.oldHead === marker.oldBase && head !== (marker.onto === marker.oldBase ? null : marker.onto)) throw new GuardRefusal('An empty rebase history must resolve exactly to its target base.'); const mappings = marker.oldHistory.map((oldSha, index) => ({ oldSha, newSha: rewrittenHistory[index]! })); + const ledger = new Map(this.#db.prepare('SELECT data FROM ledger WHERE key=?').all(planKey) + .map(row => decode(row.data)).map(entry => [entry.sha, entry])); + for (const source of resolvedConflicts) { + const mapping = mappings.find(value => value.oldSha === source)!; + if (mapping.oldSha === mapping.newSha) + throw new GuardRefusal('A resolved conflict must rewrite its source commit.'); + if (ledger.get(source)?.origin === 'owned') + throw new GuardRefusal('An owned commit cannot be prepared as a foreign conflict resolution.'); + } if (marker.resultState !== 'none') { - if (marker.resultHead !== head || stable(marker.resultMappings) !== stable(mappings)) + if (marker.resultHead !== head || stable(marker.resultMappings) !== stable(mappings) || + stable(marker.resolvedConflicts ?? []) !== stable(resolvedConflicts)) throw new GuardRefusal('Another result owns this rebase.'); return; } if (this.#run('UPDATE tasks SET rebase_in_progress=? WHERE plan_key=? AND rebase_in_progress=?', - encode({ ...marker, resultState: 'prepared', resultHead: head, resultMappings: [...mappings] }), + encode({ ...marker, resultState: 'prepared', resultHead: head, resultMappings: [...mappings], resolvedConflicts: [...resolvedConflicts] }), planKey, task.rebase_in_progress as string).changes !== 1) throw new GuardRefusal('This rebase attempt no longer owns the task.'); }); @@ -849,7 +873,7 @@ export class Store { : !endpoint || endpoint.oldSha !== marker.oldHead || endpoint.newSha !== head || stable(mappings) !== stable(marker.resultMappings)) throw new GuardRefusal('The rebase mapping does not end at the captured and rewritten heads.'); this.#assertContextWritable(key); - const snapshot = this.#applyRebase(identity, key, base, head, mappings); + const snapshot = this.#applyRebase(identity, key, base, head, mappings, marker.resolvedConflicts ?? []); this.#run('UPDATE tasks SET rebase_in_progress=NULL WHERE plan_key=?', key); return snapshot; }); @@ -869,7 +893,7 @@ export class Store { }); } #applyRebase(identity: PlanIdentity, key: string, base: string, head: string, - mappings: readonly { oldSha: string; newSha: string }[]): Snapshot { + mappings: readonly { oldSha: string; newSha: string }[], resolvedConflicts: readonly string[] = []): Snapshot { const ledger = new Map(this.getLedger(identity).map(entry => [entry.sha, entry])); const sources = new Set(), destinations = new Set(); // Validate the complete response before the snapshot or ledger changes. @@ -879,9 +903,12 @@ export class Store { sources.add(oldSha); destinations.add(newSha); } const snapshot = this.#snapshot(key, base, head); + const resolved = new Set(resolvedConflicts); for (const { oldSha, newSha } of mappings) { const source = ledger.get(oldSha); - if (oldSha !== newSha) this.#entry(key, { sha: newSha, owner: source?.owner ?? null, origin: source?.origin ?? 'foreign', sourceSha: oldSha }); + if (resolved.has(oldSha) && source?.origin === 'owned') throw new Error('An owned commit cannot be published as a foreign conflict resolution.'); + if (oldSha !== newSha) this.#entry(key, { sha: newSha, owner: source?.owner ?? null, origin: source?.origin ?? 'foreign', sourceSha: oldSha, + ...(resolved.has(oldSha) || source?.conflictResolved ? { conflictResolved: true } : {}) }); else if (!source) this.#entry(key, { sha: newSha, owner: null, origin: 'foreign', sourceSha: null }); this.#run('INSERT INTO rewrites VALUES (?,?,?,?)', key, snapshot.id, oldSha, newSha); } diff --git a/test/browser/review.spec.ts b/test/browser/review.spec.ts index e7fd7491..c028ced0 100644 --- a/test/browser/review.spec.ts +++ b/test/browser/review.spec.ts @@ -98,6 +98,17 @@ test('reviews real changes, persists approval and conversation, and assigns fore await page.getByRole('button',{name:/P1 Bound exponential retries/}).click();await expect(page.getByText('! Stale:',{exact:false})).toBeVisible();await expect(page.getByRole('heading',{name:'At approval'})).toBeVisible(); await page.screenshot({path:'test-results/review-desktop.png',fullPage:true});expect(errors).toEqual([]); }); +test('labels a resolved foreign conflict as Unplanned agent-resolved work',async({page})=>{ + const service=app.service,identity=service.config.identity,initial=service.load(),repository=service.config.repository; + writeFileSync(join(repository,'resolved-conflict.txt'),'resolved by agent\n');fixtureGit(repository,'add','resolved-conflict.txt'); + fixtureGit(repository,'commit','-m','Resolved foreign conflict');const head=fixtureGit(repository,'rev-parse','HEAD'); + service.store.recordHistory(identity,initial.expected,initial.snapshot.base,head,[ + {sha:head,owner:null,origin:'foreign',sourceSha:initial.snapshot.head,conflictResolved:true}, + ]); + await page.goto(app.url);await page.getByRole('button',{name:/Unplanned changes/}).click(); + await expect(page.getByText('Unplanned · conflict resolved by agent',{exact:true}).first()).toBeVisible(); + await expect(page.getByRole('article',{name:/resolved-conflict\.txt, Unplanned · conflict resolved by agent/}).first()).toBeVisible(); +}); test('opens an item selected while an assignment is in flight in the view its answered state calls for',async({page})=>{ await page.goto(app.url);await page.getByRole('button',{name:'Approve P1',exact:true}).click();await expect(page.getByText('1 of 3 approved')).toBeVisible(); let release!:()=>void,arrived!:()=>void;const held=new Promise(resolve=>{release=resolve;}),assignArrived=new Promise(resolve=>{arrived=resolve;}); diff --git a/test/history.test.ts b/test/history.test.ts index 7711d016..4ee83601 100644 --- a/test/history.test.ts +++ b/test/history.test.ts @@ -41,6 +41,22 @@ it('splits a shared hunk by ledger owner; detects out-of-scope and forged traile expect(parts.filter(s => s.path === 'outside.txt').every(s => s.scope === 'out-of-scope')).toBe(true); expect(parts.filter(s => s.path === 'foreign.txt').every(s => s.row === 'Unplanned')).toBe(true); }); +it('keeps a resolved foreign conflict Unplanned and carries its visible provenance', () => { + const f = fixture(); f.write('a.txt', 'resolved by agent\ntwo\nthree\n'); const sha = f.commit(); + const parts = linkHistory(f.plan, readHistory(f.dir, f.base), new Map([[sha, null]]), path => path, {}, new Set([sha])); + expect(parts.length).toBeGreaterThan(0); + expect(parts.every(segment => segment.row === 'Unplanned' && segment.conflictResolved === true)).toBe(true); +}); +it('does not merge adjacent foreign lines with different conflict-resolution provenance', () => { + const f = fixture(); f.write('a.txt', 'ONE\ntwo\nthree\n'); const first = f.commit(); + f.write('a.txt', 'ONE\nTWO\nthree\n'); const second = f.commit(); + const history = readHistory(f.dir, f.base), ledger = new Map([[first, null], [second, null]]); + for (const resolved of [first, second]) { + const added = linkHistory(f.plan, history, ledger, path => path, {}, new Set([resolved])).filter(segment => segment.operation === '+'); + expect(added.find(segment => segment.content === 'ONE\n')?.conflictResolved).toBe(resolved === first ? true : undefined); + expect(added.find(segment => segment.content === 'TWO\n')?.conflictResolved).toBe(resolved === second ? true : undefined); + } +}); it('attributes pure deletions and marks repeated edits as ambiguous', () => { const f = fixture(); f.write('a.txt', 'one\nthree\n'); f.commit('P1'); expect(f.segments().find(s => s.content === 'two\n')?.row).toBe('P1'); diff --git a/test/runner-lifecycle-store.test.ts b/test/runner-lifecycle-store.test.ts index b3a61a6a..173f1d03 100644 --- a/test/runner-lifecycle-store.test.ts +++ b/test/runner-lifecycle-store.test.ts @@ -545,7 +545,7 @@ describe('pre-merge rebase ownership', () => { { oldBase: oid(1), oldHead: oid(2), onto: oid(3), oldHistory: [oid(2)], startedAt: 123 }); expect(store.getTask(identity).rebaseInProgress).toEqual({ attemptId: marker.attemptId, oldBase: oid(1), oldHead: oid(2), onto: oid(3), oldHistory: [oid(2)], startedAt: 123, - resultState: 'none', resultHead: null, resultMappings: null, processGroup: null }); + resultState: 'none', resultHead: null, resultMappings: null, resolvedConflicts: [], processGroup: null }); const group = { pgid: 4242, startedAt: 456, identity: 'linux:00000000-0000-0000-0000-000000000000:1' }; expect(() => store.setRebaseProcessGroup(store.getTask(identity).planKey, marker.attemptId, null, { pgid: 1, startedAt: 456, identity: null })).toThrow(/Invalid rebase process group/); @@ -592,8 +592,14 @@ describe('pre-merge rebase ownership', () => { expect({ snapshot: store.getSnapshot(identity), ledger: store.getLedger(identity), marker: store.getTask(identity).rebaseInProgress }) .toEqual(beforeResult); const resultMappings = [{ oldSha: oid(3), newSha: oid(6) }, { oldSha: oid(4), newSha: oid(7) }]; - store.prepareRebaseResult(store.getTask(identity).planKey, marker.attemptId, oid(7), [oid(6), oid(7)]); - expect(store.getTask(identity).rebaseInProgress).toMatchObject({ resultState: 'prepared', resultMappings }); + expect(() => store.prepareRebaseResult(store.getTask(identity).planKey, marker.attemptId, oid(7), [oid(6), oid(7)], [oid(3)])) + .toThrow(/owned commit/); + expect(() => store.prepareRebaseResult(store.getTask(identity).planKey, marker.attemptId, oid(4), [oid(3), oid(4)], [oid(4)])) + .toThrow(/must rewrite/); + expect(() => store.prepareRebaseResult(store.getTask(identity).planKey, marker.attemptId, oid(7), [oid(6), oid(7)], [oid(4), oid(4)])) + .toThrow(/must not repeat/); + store.prepareRebaseResult(store.getTask(identity).planKey, marker.attemptId, oid(7), [oid(6), oid(7)], [oid(4)]); + expect(store.getTask(identity).rebaseInProgress).toMatchObject({ resultState: 'prepared', resultMappings, resolvedConflicts: [oid(4)] }); store.completeRebaseResult(store.getTask(identity).planKey, marker.attemptId); expect(() => store.finishRebase(identity, expected, taskVersion, marker.attemptId, oid(5), oid(7), [ { oldSha: oid(4), newSha: oid(7) }, @@ -615,7 +621,7 @@ describe('pre-merge rebase ownership', () => { expect(store.getTask(identity).rebaseInProgress).toBeNull(); expect(store.getLedger(identity)).toEqual(expect.arrayContaining([ { sha: oid(6), owner: 'P1', origin: 'owned', sourceSha: oid(3) }, - { sha: oid(7), owner: null, origin: 'foreign', sourceSha: oid(4) }, + { sha: oid(7), owner: null, origin: 'foreign', sourceSha: oid(4), conflictResolved: true }, ])); expect(store.getRewrites(identity, snapshot.id)).toEqual([ { oldSha: oid(3), newSha: oid(6) }, { oldSha: oid(4), newSha: oid(7) }, diff --git a/test/runner-rebase.test.ts b/test/runner-rebase.test.ts index 33bd2c98..41947958 100644 --- a/test/runner-rebase.test.ts +++ b/test/runner-rebase.test.ts @@ -5,7 +5,7 @@ import { join } from 'node:path'; import { randomUUID } from 'node:crypto'; import { afterEach, describe, expect, it, vi } from 'vitest'; import { fixtureGit as git } from './fixtures/git.ts'; -import { GitRebaser, RebaseConflict, rebaseRef } from '../runner/rebase.ts'; +import { GitRebaser, RebaseConflict, rebaseRef, type GitRebaserOptions } from '../runner/rebase.ts'; import { ensureCommit, openRunnerRepository } from '../runner/runner-repository.ts'; import { verifyCheckout } from '../runner/verify-checkout.ts'; @@ -24,15 +24,18 @@ function commit(repository: string, path: string, text: string, message: string) return git(repository, 'rev-parse', 'HEAD'); } -async function setup(conflict = false) { +async function setup(conflict: boolean | 'foreign' = false) { const root = mkdtempSync(join(tmpdir(), 'codeboost-rebase-')); roots.push(root); const source = join(root, 'source'); git(root, 'init', '-q', '-b', 'main', source); git(source, 'config', 'user.name', 'Source'); git(source, 'config', 'user.email', 'source@example.invalid'); const base = commit(source, 'a.txt', 'base\n', 'base'); git(source, 'switch', '-qc', 'feature'); - const owned = commit(source, 'a.txt', 'feature\n', 'owned'); - const foreign = commit(source, 'feature.txt', 'collaborator\n', 'foreign'); + const owned = commit(source, conflict === 'foreign' ? 'owned.txt' : 'a.txt', 'feature\n', 'owned'); + if (conflict === 'foreign') { git(source, 'config', 'user.name', 'Collaborator'); git(source, 'config', 'user.email', 'collaborator@example.invalid'); } + const foreign = commit(source, conflict === 'foreign' ? 'a.txt' : 'feature.txt', 'collaborator\n', + conflict === 'foreign' ? 'foreign\n\nPlan-Item: P1' : 'foreign'); + git(source, 'config', 'user.name', 'Source'); git(source, 'config', 'user.email', 'source@example.invalid'); git(source, 'switch', '-q', 'main'); const onto = commit(source, conflict ? 'a.txt' : 'base.txt', conflict ? 'main\n' : 'main moved\n', 'move base'); const runnerRoot = join(root, 'runner'); @@ -45,6 +48,13 @@ async function setup(conflict = false) { return { root, source, runnerRoot, repository, rebaser, base, owned, foreign, history: [owned, foreign], onto }; } +function createRebaser(s: Awaited>, options: Partial = {}) { + return new GitRebaser({ repository: s.repository, runnerRoot: s.runnerRoot, runnerOwner: OWNER, + committer: { name: 'Codeboost', email: 'codeboost@example.invalid' }, + onProcessStarting: () => {}, onProcessGroup: () => {}, onProcessGroupSettled: () => {}, onProcessUnsettled: () => {}, + onResultPrepared: () => {}, onResultState: () => {}, ...options }); +} + describe('trusted pre-merge rebase', () => { it('uses the configured Git PATH inside the checkout verifier subprocess', async () => { const s = await setup(), attemptId = randomUUID(), bin = join(s.root, 'git-bin'), marker = join(s.root, 'verified'); @@ -117,7 +127,7 @@ describe('trusted pre-merge rebase', () => { const s = await setup(), attemptId = randomUUID(); const result = await s.rebaser.run({ attemptId, oldBase: s.base, oldHead: s.foreign, oldHistory: s.history, onto: s.base }); expect(result).toEqual({ oldHead: s.foreign, base: s.base, head: s.foreign, - mappings: [{ oldSha: s.owned, newSha: s.owned }, { oldSha: s.foreign, newSha: s.foreign }] }); + mappings: [{ oldSha: s.owned, newSha: s.owned }, { oldSha: s.foreign, newSha: s.foreign }], resolvedConflicts: [] }); expect(existsSync(join(s.runnerRoot, OWNER, 'rebases', attemptId))).toBe(false); expect(() => git(s.repository.path, 'show-ref', '--verify', rebaseRef(attemptId))).toThrow(); }); @@ -312,6 +322,66 @@ describe('trusted pre-merge rebase', () => { expect(git(s.source, 'rev-parse', 'feature')).toBe(s.foreign); }); + it('routes a missing-ledger foreign conflict to the resolver and preserves its author and provenance', async () => { + const s = await setup('foreign'), attemptId = randomUUID(); + const seen: unknown[] = []; + const runner = createRebaser(s, { resolveForeignConflict: async input => { + seen.push({ commit: input.commit, files: input.files }); + writeFileSync(join(input.repository, 'a.txt'), 'resolved foreign change\n'); + } }); + const result = await runner.run({ attemptId, oldBase: s.base, oldHead: s.foreign, oldHistory: s.history, onto: s.onto, + ledger: [{ sha: s.owned, owner: 'P1', origin: 'owned' }] }); + expect(seen).toEqual([{ commit: s.foreign, files: ['a.txt'] }]); + expect(result.resolvedConflicts).toEqual([s.foreign]); + const rewritten = result.mappings.find(entry => entry.oldSha === s.foreign)!.newSha; + expect(git(s.repository.path, 'show', '-s', '--format=%an <%ae>', rewritten)).toBe('Collaborator '); + expect(git(s.repository.path, 'show', '-s', '--format=%B', rewritten)).toContain('Plan-Item: P1'); + expect(git(s.repository.path, 'show', `${result.head}:a.txt`)).toBe('resolved foreign change'); + }); + + it('refuses a resolver edit outside the exact conflicted files and drops the partial rewrite', async () => { + const s = await setup('foreign'), attemptId = randomUUID(); + const runner = createRebaser(s, { resolveForeignConflict: async input => { + writeFileSync(join(input.repository, 'a.txt'), 'resolved\n'); + writeFileSync(join(input.repository, 'extra.txt'), 'outside conflict\n'); + } }); + await expect(runner.run({ attemptId, oldBase: s.base, oldHead: s.foreign, oldHistory: s.history, onto: s.onto, + ledger: [{ sha: s.foreign, owner: null, origin: 'foreign' }] })).rejects.toThrow(/outside the conflicted set/); + expect(existsSync(join(s.runnerRoot, OWNER, 'rebases', attemptId))).toBe(false); + expect(() => git(s.repository.path, 'rev-parse', '--verify', rebaseRef(attemptId))).toThrow(); + }); + + it('aborts and awaits the resolver before removing its workspace', async () => { + const s = await setup('foreign'), attemptId = randomUUID(), controller = new AbortController(); + let entered!: () => void, settled = false; + const started = new Promise(resolve => { entered = resolve; }); + const runner = createRebaser(s, { resolveForeignConflict: input => new Promise((_, reject) => { + entered(); input.signal!.addEventListener('abort', () => { settled = true; reject(input.signal!.reason); }, { once: true }); + }) }); + const operation = runner.run({ attemptId, oldBase: s.base, oldHead: s.foreign, oldHistory: s.history, onto: s.onto, + ledger: [{ sha: s.foreign, owner: null, origin: 'foreign' }], signal: controller.signal }); + await started; controller.abort(new Error('stop conflict resolver')); + await expect(operation).rejects.toThrow(/stop conflict resolver/); + expect(settled).toBe(true); + expect(existsSync(join(s.runnerRoot, OWNER, 'rebases', attemptId))).toBe(false); + }); + + it('never sends an owned conflict through the foreign resolver', async () => { + const s = await setup(true), attemptId = randomUUID(), resolveForeignConflict = vi.fn(async () => {}); + const runner = createRebaser(s, { resolveForeignConflict }); + await expect(runner.run({ attemptId, oldBase: s.base, oldHead: s.foreign, oldHistory: s.history, onto: s.onto, + ledger: [{ sha: s.owned, owner: 'P1', origin: 'owned' }, { sha: s.foreign, owner: null, origin: 'foreign' }] })) + .rejects.toThrow(/owned commit conflict/); + expect(resolveForeignConflict).not.toHaveBeenCalled(); + }); + + it('refuses an inconsistent trusted ledger before creating a rebase workspace', async () => { + const s = await setup('foreign'), attemptId = randomUUID(); + await expect(s.rebaser.run({ attemptId, oldBase: s.base, oldHead: s.foreign, oldHistory: s.history, onto: s.onto, + ledger: [{ sha: s.foreign, owner: 'P1', origin: 'foreign' }] })).rejects.toThrow(/ledger is invalid/); + expect(existsSync(join(s.runnerRoot, OWNER, 'rebases', attemptId))).toBe(false); + }); + it('refuses a case-colliding reviewed tree when the host cannot check it out faithfully', async () => { const s = await setup(), attemptId = randomUUID(); const probe = join(s.root, 'case-probe'); mkdirSync(probe); diff --git a/test/runner-recovery.test.ts b/test/runner-recovery.test.ts index 6c654ab6..374a76e8 100644 --- a/test/runner-recovery.test.ts +++ b/test/runner-recovery.test.ts @@ -289,9 +289,37 @@ describe('startup recovery sequence', () => { await recoverStartup({ store, runnerOwner: token, runnerRoot: join(root, 'r'), diagnosticsDir: join(root, 'd'), deps: deps({ abortRebase }).d }); expect(abortRebase).toHaveBeenCalledWith(planKey, { ...marker, oldHistory: null, resultState: 'none', - resultHead: null, resultMappings: null }); + resultHead: null, resultMappings: null, resolvedConflicts: [] }); expect(store.getTask(id(1)).rebaseInProgress).toBeNull(); }); + it('retains a recovery marker whose resolved-conflict provenance is outside its captured history', async () => { + const { d: root, store, raw } = fixture(); + store.transitionTask(id(1), store.getTask(id(1)).stateVersion, 'approved but merge blocked'); + const snapshot = store.getSnapshot(id(1)), attemptId = randomUUID(), planKey = store.getTask(id(1)).planKey; + const marker = { attemptId, oldBase: snapshot.base, oldHead: snapshot.head, onto: oid(3), oldHistory: [snapshot.head], startedAt: 123, + resultState: 'ready', resultHead: oid(4), resultMappings: [{ oldSha: snapshot.head, newSha: oid(4) }], + resolvedConflicts: [oid(9)], processGroup: null }; + raw(`UPDATE tasks SET rebase_in_progress='${JSON.stringify(marker)}' WHERE plan_key='${planKey}'`); + const abortRebase = vi.fn(async () => undefined); + await expect(recoverStartup({ store, runnerOwner: token, runnerRoot: join(root, 'r'), diagnosticsDir: join(root, 'd'), + deps: deps({ abortRebase }).d })).rejects.toThrow(/invalid recovery marker/); + expect(abortRebase).not.toHaveBeenCalled(); + expect(store.getTask(id(1)).rebaseInProgress).toEqual(marker); + }); + it('rejects explicit null resolved-conflict provenance instead of treating it as a legacy field', async () => { + const { d: root, store, raw } = fixture(); + store.transitionTask(id(1), store.getTask(id(1)).stateVersion, 'approved but merge blocked'); + const snapshot = store.getSnapshot(id(1)), attemptId = randomUUID(), planKey = store.getTask(id(1)).planKey; + const marker = { attemptId, oldBase: snapshot.base, oldHead: snapshot.head, onto: oid(3), oldHistory: [snapshot.head], startedAt: 123, + resultState: 'ready', resultHead: oid(4), resultMappings: [{ oldSha: snapshot.head, newSha: oid(4) }], + resolvedConflicts: null, processGroup: null }; + raw(`UPDATE tasks SET rebase_in_progress='${JSON.stringify(marker)}' WHERE plan_key='${planKey}'`); + const abortRebase = vi.fn(async () => undefined); + await expect(recoverStartup({ store, runnerOwner: token, runnerRoot: join(root, 'r'), diagnosticsDir: join(root, 'd'), + deps: deps({ abortRebase }).d })).rejects.toThrow(/invalid recovery marker/); + expect(abortRebase).not.toHaveBeenCalled(); + expect(store.getTask(id(1)).rebaseInProgress).toEqual(marker); + }); it('rejects incomplete and cleanup-only markers that claim retained-result ownership', async () => { for (const shape of ['missing-state', 'missing-history'] as const) { const { d: root, store, raw } = fixture(); diff --git a/test/store.test.ts b/test/store.test.ts index 9cf175e9..5ca267e4 100644 --- a/test/store.test.ts +++ b/test/store.test.ts @@ -8,9 +8,10 @@ import { DatabaseSync } from 'node:sqlite'; import { afterEach, expect, it } from 'vitest'; import { Store, requireSupportedNode } from '../runner/store.ts'; import type { Plan, PlanContext, EditReply } from '../core/plan.ts'; -import { approveItem, approvalStates, choiceKeys, applyChoices } from '../core/approvals.ts'; +import { approveItem, approvalStates, choiceKeys, applyChoices, stable } from '../core/approvals.ts'; import { linkHistory, type Segment } from '../core/linking.ts'; import { readHistory } from '../git/history.ts'; +import { identityKey } from '../core/identity.ts'; const identity = { repositoryId: 'repo', taskId: 'task', planId: 'plan' }; const context: PlanContext = { identity, issue: 1, baseEntries: [{ path: 'a', kind: 'file' }], pathKey: p => p, allowedCommands: [] }; const plan = (): Plan => ({ schema_version: 1, revision: 99, issue: 1, summary: 'Example', questions: [], items: [{ id: 'P1', title: 'Change', intent: 'Improve', files: [{ path: 'a', kind: 'edit', renamed_from: null, change: 'Change' }], acceptance: [{ type: 'check', text: 'Works' }], depends_on: [] }] }); @@ -160,14 +161,16 @@ it('rolls back the entire ledger batch and snapshot after a late ownership colli expect(() => store.recordRebase(identity, state(store), oid(5), oid(6), [{ oldSha: oid(2), newSha: oid(6) }, { oldSha: oid(3), newSha: oid(6) }])).toThrow(/one-to-one/); expect(store.getSnapshot(identity)).toEqual(before); expect(store.getLedger(identity)).toHaveLength(1); }); -it('preserves owned and missing/null foreign provenance through repeated rebase mappings and restart', () => { +it('preserves owned, foreign, and conflict-resolution provenance through repeated rebase mappings and restart', () => { const { store, path } = fixture(); store.recordHistory(identity, state(store), oid(1), oid(3), [ - { sha: oid(2), owner: 'P1', origin: 'owned', sourceSha: null }, { sha: oid(3), owner: null, origin: 'foreign', sourceSha: null }, + { sha: oid(2), owner: 'P1', origin: 'owned', sourceSha: null }, + { sha: oid(3), owner: null, origin: 'foreign', sourceSha: null, conflictResolved: true }, ]); const snapshot = store.recordRebase(identity, state(store), oid(10), oid(14), [2,3,4].map(n => ({ oldSha: oid(n), newSha: oid(n + 10) }))); store.recordRebase(identity, state(store), oid(20), oid(24), [12,13,14].map(n => ({ oldSha: oid(n), newSha: oid(n + 10) }))); const recovered = open(path); expect(recovered.ownership(identity).get(oid(22))).toBe('P1'); expect(recovered.ownership(identity).get(oid(23))).toBeNull(); expect(recovered.ownership(identity).get(oid(24))).toBeNull(); + expect(recovered.getLedger(identity).find(e => e.sha === oid(23))).toEqual({ sha: oid(23), owner: null, origin: 'foreign', sourceSha: oid(13), conflictResolved: true }); expect(recovered.getLedger(identity).find(e => e.sha === oid(24))).toEqual({ sha: oid(24), owner: null, origin: 'foreign', sourceSha: oid(14) }); expect(recovered.getRewrites(identity, snapshot.id)).toHaveLength(3); }); @@ -190,10 +193,21 @@ it('retains typed file-card approvals/choices while fingerprints detect later me const segment: Segment = { path: 'a', oldPath: 'a', kind: 'file', operation: null, content: JSON.stringify({ kind: 'binary', oldObject: { type: 'blob', oid: oid(1) }, newObject: { type: 'blob', oid: oid(2) } }), context: '', owners: ['P1'], row: 'P1', scope: 'in-scope', oldLine: null, newLine: null, hunk: 0, sharesHunkWith: [] }; const approval = approveItem(current, [segment], 'P1', identity); const choice = { key: choiceKeys([segment], identity)[0]!, action: 'accept' as const, item: null }; + const legacyContent = stable({ path: segment.path, oldPath: segment.oldPath, kind: segment.kind, + operation: segment.operation, content: segment.content }); + expect(choice.key).toBe(stable([identityKey(identity), legacyContent, 1, 1])); + expect(approval.fingerprint).toBe(stable({ identity: identityKey(identity), item: current.items[0], segments: [{ + path: segment.path, oldPath: segment.oldPath, kind: segment.kind, operation: segment.operation, + content: segment.content, context: segment.context, owners: ['P1'], + }] })); store.saveReview(identity, state(store), [approval], [choice]); const recovered = open(path).getReview(identity); expect(recovered.approvals[0]!.fingerprint).toBe(approval.fingerprint); expect(recovered.choices[0]!.key).toBe(choice.key); expect(approvalStates(current, [segment], recovered.approvals, identity).P1).toBe('approved'); + const resolved = { ...segment, conflictResolved: true as const }; + expect(choiceKeys([resolved], identity)[0]).not.toBe(choice.key); + expect(applyChoices(current, [{ ...resolved, row: 'Unplanned', owners: [null], scope: 'unplanned' }], recovered.choices, identity)[0]!.row).toBe('Unplanned'); + expect(approvalStates(current, [resolved], recovered.approvals, identity).P1).toBe('stale'); expect(approvalStates(current, [{ ...segment, content: segment.content.replace(oid(2), oid(3)) }], recovered.approvals, identity).P1).toBe('stale'); const old = state(store); store.importRevision(JSON.stringify({ ...plan(), items: [{ ...plan().items[0]!, title: 'Changed' }] }), 'json', context, 1); expect(() => store.saveReview(identity, old, [approval], [])).toThrow(/Stale/); diff --git a/web/public/app.js b/web/public/app.js index 3aadafe6..aeed68e6 100644 --- a/web/public/app.js +++ b/web/public/app.js @@ -355,8 +355,8 @@ function renderCode() { comparison + (segments .map((segment, index) => { - const provenance = - segment.row === "Accepted" ? "Accepted" : segment.row; + const provenance = `${segment.row === "Accepted" ? "Accepted" : segment.row}${segment.conflictResolved ? " · conflict resolved by agent" : ""}`; + const unplanned = segment.row === "Unplanned"; let content; if (segment.kind === "file") { const meta = JSON.parse(segment.content); @@ -372,13 +372,13 @@ function renderCode() { meta.newMode ? "File mode changed" : "File change"; - content = `
${esc(provenance)}
▧ ${label}
Path
${esc(meta.oldPath || "Absent")} → ${esc(meta.newPath || "Absent")}
Mode
${esc(meta.oldMode || "Absent")} → ${esc(meta.newMode || "Absent")}

Size: ${segment.file?.oldSize ?? "N/A"} → ${segment.file?.newSize ?? "N/A"} bytes

${segment.file?.beforePreview || segment.file?.afterPreview ? `
${segment.file.beforePreview ? `
Before
Previous image in ${esc(segment.path)}
` : ""}${segment.file.afterPreview ? `
After
Current image in ${esc(segment.path)}
` : ""}
` : '

No preview available

'}
Details · content IDs
${esc(JSON.stringify(meta, null, 2))}
`; + content = `
${esc(provenance)}
▧ ${label}
Path
${esc(meta.oldPath || "Absent")} → ${esc(meta.newPath || "Absent")}
Mode
${esc(meta.oldMode || "Absent")} → ${esc(meta.newMode || "Absent")}

Size: ${segment.file?.oldSize ?? "N/A"} → ${segment.file?.newSize ?? "N/A"} bytes

${segment.file?.beforePreview || segment.file?.afterPreview ? `
${segment.file.beforePreview ? `
Before
Previous image in ${esc(segment.path)}
` : ""}${segment.file.afterPreview ? `
After
Current image in ${esc(segment.path)}
` : ""}
` : '

No preview available

'}
Details · content IDs
${esc(JSON.stringify(meta, null, 2))}
`; } else { const lines = segment.content.split("\n"); if (lines.at(-1) === "") lines.pop(); const start = segment.operation === "+" ? segment.newLine : segment.oldLine; - content = `
${esc(provenance)}
${lines.map((_, i) => ``).join("")}
${esc(segment.operation)}
${lines.map((line,i) => `${esc(line) || "​"}`).join("\n")}
`; + content = `
${esc(provenance)}
${lines.map((_, i) => ``).join("")}
${esc(segment.operation)}
${lines.map((line,i) => `${esc(line) || "​"}`).join("\n")}
`; } const choices = ["Unplanned", "Ambiguous"].includes(segment.row) ? `

Assigning this change makes the selected item’s approval stale.

` From 286a8eb09a5792047562292920e9ee3ac19e1965 Mon Sep 17 00:00:00 2001 From: mchwang Date: Tue, 6 Oct 2026 14:44:21 -0700 Subject: [PATCH 02/11] Fix foreign conflict resolver audit --- docs/implementation/pre-merge-rebase.md | 2 +- runner/rebase.ts | 50 ++++++++++++++++++++++--- test/runner-rebase.test.ts | 47 +++++++++++++++++++++++ 3 files changed, 92 insertions(+), 7 deletions(-) diff --git a/docs/implementation/pre-merge-rebase.md b/docs/implementation/pre-merge-rebase.md index efd8a411..46b724da 100644 --- a/docs/implementation/pre-merge-rebase.md +++ b/docs/implementation/pre-merge-rebase.md @@ -18,7 +18,7 @@ Admission checks the current plan revision, snapshot, review version, task state After replay settles and before its first result-ref write, the rebaser supplies the Store's preparation hook only the ordered rewritten history. The Store zips that history by position with the independently captured `oldHistory`, then durably binds the resulting complete mapping and exact intended ref value as `prepared`. The atomic create outcome advances it to `ready`, `uncertain`, or `refused`; an already-current no-op uses `prepared` then `ready` without a ref write. `Store.finishRebase` accepts only a `ready`, byte-for-byte mapping from the same attempt while the captured plan, snapshot, review version, task version, old base, old head, and target base are unchanged. A non-empty mapping must end at both the captured old head and the committed rewritten head; an empty mapping is valid only when the captured history is empty and the result equals the new base. The snapshot, rewrite map, and owned/foreign ledger provenance commit atomically, and the marker clears in that transaction. A stale result changes nothing and leaves its marker for exact-attempt cleanup. -For each conflict, classification uses only the supplied trusted ledger. A missing entry and an explicit `{ owner: null, origin: "foreign" }` entry take the foreign branch; trailers are never read. An owned entry is refused until the separate owned-commit resolver exists. The injected foreign resolver receives the source commit and exact NUL-delimited conflicting path set. After it settles, the rebaser proves `REBASE_HEAD` is unchanged, rejects every tracked or untracked edit outside that set, stages only literal path data on stdin, and refuses any remaining unmerged entry. A rejection or invocation failure aborts and removes the partial rewrite. The production adapter must expose the rebase worktree only through lane D's bounded task storage; this engine callback is not permission to mount the host worktree directly. +For each conflict, classification uses only the supplied trusted ledger. A missing entry and an explicit `{ owner: null, origin: "foreign" }` entry take the foreign branch; trailers are never read. An owned entry is refused until the separate owned-commit resolver exists. The injected foreign resolver receives the source commit and exact conflicting path set as a read-only string array. After it settles, the rebaser proves `REBASE_HEAD` is unchanged, rejects every tracked or untracked edit outside that set, stages only literal NUL-delimited path data on stdin, and refuses any remaining unmerged entry. A rejection or invocation failure aborts and removes the partial rewrite. The production adapter must expose the rebase worktree only through lane D's bounded task storage; this engine callback is not permission to mount the host worktree directly. The Store records each resolved source SHA before the result ref write. Preparation refuses duplicates, sources outside the captured history, owned sources, and sources that did not actually rewrite. On publication, the mapped commit retains `owner: null`, `origin: foreign`, and `sourceSha`, plus `conflictResolved: true`. Linking therefore keeps its lines in Unplanned while the review UI adds the text label “conflict resolved by agent.” The commit author is preserved by Git's replay and no Plan-Item trailer is added. diff --git a/runner/rebase.ts b/runner/rebase.ts index c5791b64..3c095b4f 100644 --- a/runner/rebase.ts +++ b/runner/rebase.ts @@ -1,5 +1,6 @@ -import { accessSync, constants, lstatSync, realpathSync, statSync } from 'node:fs'; -import { delimiter, join, resolve } from 'node:path'; +import { createHash } from 'node:crypto'; +import { accessSync, closeSync, constants, fstatSync, lstatSync, openSync, readlinkSync, readSync, realpathSync, statSync } from 'node:fs'; +import { delimiter, join, resolve, sep } from 'node:path'; import { fileURLToPath } from 'node:url'; import { runInProcessGroup, type ProcessGroup } from '../agents/process-group.ts'; import type { DockerOutcome } from '../agents/docker.ts'; @@ -166,14 +167,13 @@ export class GitRebaser { const files = Object.freeze(await this.#paths(path, ['diff', '--name-only', '--diff-filter=U', '-z', '--'], scope)); if (!files.length) throw new RebaseConflict('Git reported a conflict without any conflicted files.'); const allowed = new Set(files); + const outsideBefore = await this.#outsideConflictState(path, allowed, scope); this.#assertResultCurrent(scope); await this.#resolveConflict({ attemptId, commit, files, repository: path }, scope); this.#assertResultCurrent(scope); if (await this.#git(path, ['rev-parse', '--verify', 'REBASE_HEAD^{commit}'], scope) !== commit) throw new RebaseConflict('The conflict resolver changed the rebase operation.'); - const changed = [...await this.#paths(path, ['diff', '--name-only', '-z', 'HEAD', '--'], scope), - ...await this.#paths(path, ['ls-files', '--others', '-z', '--'], scope)]; - if (changed.some(file => !allowed.has(file))) + if (await this.#outsideConflictState(path, allowed, scope) !== outsideBefore) throw new RebaseConflict('The conflict resolver changed a file outside the conflicted set.'); const pathspec = Buffer.from(`${files.join('\0')}\0`); await this.#git(path, ['--literal-pathspecs', 'add', '--all', '--pathspec-from-file=-', '--pathspec-file-nul'], scope, false, pathspec); @@ -337,6 +337,40 @@ export class GitRebaser { return paths; } + async #outsideConflictState(repository: string, allowed: ReadonlySet, scope: CallScope): Promise { + const changed = new Set([ + ...await this.#paths(repository, ['diff', '--name-only', '--ignore-submodules=all', '-z', 'HEAD', '--'], scope), + ...await this.#paths(repository, ['ls-files', '--others', '-z', '--'], scope), + ]); + const entries: string[][] = []; + for (const path of [...changed].filter(value => !allowed.has(value)).sort()) { + const index = await this.#git(repository, ['--literal-pathspecs', 'ls-files', '--stage', '-z', '--', path], scope); + entries.push([path, index, this.#worktreeFingerprint(repository, path)]); + } + return JSON.stringify(entries); + } + + #worktreeFingerprint(repository: string, path: string): string { + const full = resolve(repository, path), root = `${resolve(repository)}${sep}`; + if (!full.startsWith(root)) throw new RebaseConflict('Git returned a path outside the rebase workspace.'); + const before = lstatSync(full, { throwIfNoEntry: false }); + if (!before) return 'missing'; + if (before.isSymbolicLink()) return `link:${before.mode}:${readlinkSync(full, { encoding: 'buffer' }).toString('hex')}`; + if (!before.isFile()) throw new RebaseConflict('A changed path is not a regular file or symbolic link.'); + const fd = openSync(full, constants.O_RDONLY | constants.O_NOFOLLOW); + try { + const stat = fstatSync(fd); + if (!stat.isFile()) throw new RebaseConflict('A changed path changed type while it was audited.'); + const hash = createHash('sha256'), buffer = Buffer.allocUnsafe(64 * 1024); + for (;;) { + const length = readSync(fd, buffer, 0, buffer.length, null); + if (!length) break; + hash.update(buffer.subarray(0, length)); + } + return `file:${stat.mode}:${hash.digest('hex')}`; + } finally { closeSync(fd); } + } + async #resolveConflict(input: Omit, scope: CallScope): Promise { const resolveConflict = this.#options.resolveForeignConflict!; const controller = new AbortController(); @@ -349,7 +383,11 @@ export class GitRebaser { try { controller.signal.throwIfAborted(); // Await settlement after abort: the resolver owns its container/storage until its promise ends. - await resolveConflict(Object.freeze({ ...input, signal: controller.signal })); + try { await resolveConflict(Object.freeze({ ...input, signal: controller.signal })); } + catch (error) { + controller.signal.throwIfAborted(); + throw error; + } controller.signal.throwIfAborted(); } finally { clearTimeout(timer); diff --git a/test/runner-rebase.test.ts b/test/runner-rebase.test.ts index 41947958..5da561eb 100644 --- a/test/runner-rebase.test.ts +++ b/test/runner-rebase.test.ts @@ -339,6 +339,39 @@ describe('trusted pre-merge rebase', () => { expect(git(s.repository.path, 'show', `${result.head}:a.txt`)).toBe('resolved foreign change'); }); + it('allows clean files from the conflicted commit without attributing them to the resolver', async () => { + const s = await setup('foreign'), attemptId = randomUUID(); + git(s.source, 'switch', '-q', 'feature'); + writeFileSync(join(s.source, 'clean.txt'), 'clean foreign change\n'); + git(s.source, 'add', '--', 'clean.txt'); + git(s.source, 'commit', '--amend', '--no-edit', '-q'); + const foreign = git(s.source, 'rev-parse', 'HEAD'); + await ensureCommit(s.repository, foreign); + const runner = createRebaser(s, { resolveForeignConflict: async input => { + writeFileSync(join(input.repository, 'a.txt'), 'resolved foreign change\n'); + } }); + const result = await runner.run({ attemptId, oldBase: s.base, oldHead: foreign, oldHistory: [s.owned, foreign], onto: s.onto, + ledger: [{ sha: foreign, owner: null, origin: 'foreign' }] }); + expect(git(s.repository.path, 'show', `${result.head}:a.txt`)).toBe('resolved foreign change'); + expect(git(s.repository.path, 'show', `${result.head}:clean.txt`)).toBe('clean foreign change'); + }); + + it('refuses a resolver edit to a cleanly applied file from the conflicted commit', async () => { + const s = await setup('foreign'), attemptId = randomUUID(); + git(s.source, 'switch', '-q', 'feature'); + writeFileSync(join(s.source, 'clean.txt'), 'clean foreign change\n'); + git(s.source, 'add', '--', 'clean.txt'); + git(s.source, 'commit', '--amend', '--no-edit', '-q'); + const foreign = git(s.source, 'rev-parse', 'HEAD'); + await ensureCommit(s.repository, foreign); + const runner = createRebaser(s, { resolveForeignConflict: async input => { + writeFileSync(join(input.repository, 'a.txt'), 'resolved foreign change\n'); + writeFileSync(join(input.repository, 'clean.txt'), 'resolver changed clean file\n'); + } }); + await expect(runner.run({ attemptId, oldBase: s.base, oldHead: foreign, oldHistory: [s.owned, foreign], onto: s.onto, + ledger: [{ sha: foreign, owner: null, origin: 'foreign' }] })).rejects.toThrow(/outside the conflicted set/); + }); + it('refuses a resolver edit outside the exact conflicted files and drops the partial rewrite', async () => { const s = await setup('foreign'), attemptId = randomUUID(); const runner = createRebaser(s, { resolveForeignConflict: async input => { @@ -366,6 +399,20 @@ describe('trusted pre-merge rebase', () => { expect(existsSync(join(s.runnerRoot, OWNER, 'rebases', attemptId))).toBe(false); }); + it('preserves the cancellation reason when the resolver rejects with another error after abort', async () => { + const s = await setup('foreign'), attemptId = randomUUID(), controller = new AbortController(); + let entered!: () => void; + const started = new Promise(resolve => { entered = resolve; }); + const runner = createRebaser(s, { resolveForeignConflict: input => new Promise((_, reject) => { + entered(); input.signal!.addEventListener('abort', () => reject(new Error('resolver cleanup failed')), { once: true }); + }) }); + const operation = runner.run({ attemptId, oldBase: s.base, oldHead: s.foreign, oldHistory: s.history, onto: s.onto, + ledger: [{ sha: s.foreign, owner: null, origin: 'foreign' }], signal: controller.signal }); + await started; + controller.abort(new Error('original cancellation')); + await expect(operation).rejects.toThrow(/original cancellation/); + }); + it('never sends an owned conflict through the foreign resolver', async () => { const s = await setup(true), attemptId = randomUUID(), resolveForeignConflict = vi.fn(async () => {}); const runner = createRebaser(s, { resolveForeignConflict }); From 1eb8f53dad3c4adf918d006311cdb0d1dad19be6 Mon Sep 17 00:00:00 2001 From: mchwang Date: Tue, 6 Oct 2026 14:54:09 -0700 Subject: [PATCH 03/11] Audit gitlinks during conflict resolution --- docs/implementation/pre-merge-rebase.md | 4 ++-- runner/rebase.ts | 8 ++++++- runner/verify-checkout.ts | 19 +++++++++++++++- test/runner-rebase.test.ts | 30 +++++++++++++++++++++++++ 4 files changed, 57 insertions(+), 4 deletions(-) diff --git a/docs/implementation/pre-merge-rebase.md b/docs/implementation/pre-merge-rebase.md index 46b724da..4bd5e4bc 100644 --- a/docs/implementation/pre-merge-rebase.md +++ b/docs/implementation/pre-merge-rebase.md @@ -18,7 +18,7 @@ Admission checks the current plan revision, snapshot, review version, task state After replay settles and before its first result-ref write, the rebaser supplies the Store's preparation hook only the ordered rewritten history. The Store zips that history by position with the independently captured `oldHistory`, then durably binds the resulting complete mapping and exact intended ref value as `prepared`. The atomic create outcome advances it to `ready`, `uncertain`, or `refused`; an already-current no-op uses `prepared` then `ready` without a ref write. `Store.finishRebase` accepts only a `ready`, byte-for-byte mapping from the same attempt while the captured plan, snapshot, review version, task version, old base, old head, and target base are unchanged. A non-empty mapping must end at both the captured old head and the committed rewritten head; an empty mapping is valid only when the captured history is empty and the result equals the new base. The snapshot, rewrite map, and owned/foreign ledger provenance commit atomically, and the marker clears in that transaction. A stale result changes nothing and leaves its marker for exact-attempt cleanup. -For each conflict, classification uses only the supplied trusted ledger. A missing entry and an explicit `{ owner: null, origin: "foreign" }` entry take the foreign branch; trailers are never read. An owned entry is refused until the separate owned-commit resolver exists. The injected foreign resolver receives the source commit and exact conflicting path set as a read-only string array. After it settles, the rebaser proves `REBASE_HEAD` is unchanged, rejects every tracked or untracked edit outside that set, stages only literal NUL-delimited path data on stdin, and refuses any remaining unmerged entry. A rejection or invocation failure aborts and removes the partial rewrite. The production adapter must expose the rebase worktree only through lane D's bounded task storage; this engine callback is not permission to mount the host worktree directly. +For each conflict, classification uses only the supplied trusted ledger. A missing entry and an explicit `{ owner: null, origin: "foreign" }` entry take the foreign branch; trailers are never read. An owned entry is refused until the separate owned-commit resolver exists. The injected foreign resolver receives the source commit and exact conflicting path set as a read-only string array. Before and after it runs, the rebaser snapshots every non-conflict changed path and hashes every index-only gitlink entry without entering a nested repository. After the resolver settles, the rebaser proves `REBASE_HEAD` is unchanged, rejects every tracked or untracked edit outside the conflict set, stages only literal NUL-delimited path data on stdin, and refuses any remaining unmerged entry. A rejection or invocation failure aborts and removes the partial rewrite. The production adapter must expose the rebase worktree only through lane D's bounded task storage; this engine callback is not permission to mount the host worktree directly. The Store records each resolved source SHA before the result ref write. Preparation refuses duplicates, sources outside the captured history, owned sources, and sources that did not actually rewrite. On publication, the mapped commit retains `owner: null`, `origin: foreign`, and `sourceSha`, plus `conflictResolved: true`. Linking therefore keeps its lines in Unplanned while the review UI adds the text label “conflict resolved by agent.” The commit author is preserved by Git's replay and no Plan-Item trailer is added. @@ -32,4 +32,4 @@ This ordering is intentionally local-only. The later push integration must exten ## Regression evidence -`test/runner-rebase.test.ts` covers clean one-to-one replay, the already-current no-op, process-group ownership, the whole-operation deadline, foreign conflict resolution, invalid ledger classification, outside-file refusal, owned-conflict refusal, cancellation before and during Git, exact cleanup, retained refs, and a read-only source checkout. `test/runner-lifecycle-store.test.ts` covers admission, durable process ownership, compare-and-swap races, resolved-source validation, and owned/foreign provenance. `test/runner-recovery.test.ts` covers marker validation, process termination and restart ordering plus fail-closed absence of a recovery implementation. `test/history.test.ts` and `test/browser/review.spec.ts` cover Unplanned attribution and the visible conflict-resolution label. `test/runner-production.test.ts` covers exact-plan recovery routing. +`test/runner-rebase.test.ts` covers clean one-to-one replay, the already-current no-op, process-group ownership, the whole-operation deadline, foreign conflict resolution, clean sibling files, gitlink-pointer auditing, invalid ledger classification, outside-file refusal, owned-conflict refusal, cancellation before and during Git, exact cleanup, retained refs, and a read-only source checkout. `test/runner-lifecycle-store.test.ts` covers admission, durable process ownership, compare-and-swap races, resolved-source validation, and owned/foreign provenance. `test/runner-recovery.test.ts` covers marker validation, process termination and restart ordering plus fail-closed absence of a recovery implementation. `test/history.test.ts` and `test/browser/review.spec.ts` cover Unplanned attribution and the visible conflict-resolution label. `test/runner-production.test.ts` covers exact-plan recovery routing. diff --git a/runner/rebase.ts b/runner/rebase.ts index 3c095b4f..0ba4a3ba 100644 --- a/runner/rebase.ts +++ b/runner/rebase.ts @@ -338,6 +338,12 @@ export class GitRebaser { } async #outsideConflictState(repository: string, allowed: ReadonlySet, scope: CallScope): Promise { + const verifier = fileURLToPath(new URL('./verify-checkout.ts', import.meta.url)); + const gitlinks = await this.#process(process.execPath, [verifier, this.#gitExecutable, 'gitlinks'], repository, + this.#gitEnvironment, scope, false, 64 * 1024); + this.#throwIfCancelled(gitlinks, scope); + if (gitlinks.status !== 0 || !/^[0-9a-f]{64}$/.test(gitlinks.stdout)) + throw this.#failure('gitlink index audit', gitlinks, 'node'); const changed = new Set([ ...await this.#paths(repository, ['diff', '--name-only', '--ignore-submodules=all', '-z', 'HEAD', '--'], scope), ...await this.#paths(repository, ['ls-files', '--others', '-z', '--'], scope), @@ -347,7 +353,7 @@ export class GitRebaser { const index = await this.#git(repository, ['--literal-pathspecs', 'ls-files', '--stage', '-z', '--', path], scope); entries.push([path, index, this.#worktreeFingerprint(repository, path)]); } - return JSON.stringify(entries); + return JSON.stringify([gitlinks.stdout, entries]); } #worktreeFingerprint(repository: string, path: string): string { diff --git a/runner/verify-checkout.ts b/runner/verify-checkout.ts index 02fbdf45..55852daa 100644 --- a/runner/verify-checkout.ts +++ b/runner/verify-checkout.ts @@ -35,6 +35,21 @@ const splitPath = (path: Buffer): Buffer[] => { const objectId = (algorithm: 'sha1' | 'sha256', bytes: Buffer): string => createHash(algorithm).update(Buffer.from(`blob ${bytes.length}\0`)).update(bytes).digest('hex'); +/** Hash index-only gitlink entries without entering or reading their nested repositories. */ +export function gitlinkIndexDigest(root = process.cwd(), executable = 'git'): string { + const rawOutput = git(executable, ['-C', root, 'ls-files', '--stage', '-z']); + if (rawOutput.length > MAX_INDEX_BYTES) throw new Error('Checkout index exceeds its byte bound.'); + const raw = split0(rawOutput); + if (raw.length > MAX_DIRECTORY_ENTRIES) throw new Error('Checkout index exceeds its entry bound.'); + const hash = createHash('sha256'); + for (const record of raw) { + const tab = record.indexOf(0x09), header = record.subarray(0, tab).toString('ascii').split(' '); + if (tab < 0 || header.length !== 3 || !/^[0-3]$/.test(header[2]!)) throw new Error('Git returned an invalid index record.'); + if (header[0] === '160000') hash.update(record).update('\0'); + } + return hash.digest('hex'); +} + /** Byte-exact, content-based proof that the disposable worktree represents its index. Gitlinks stay index-only. */ export function verifyCheckout(root = process.cwd(), executable = 'git'): void { const repositoryGit = (args: readonly string[], input?: Buffer) => git(executable, ['-C', root, ...args], input); @@ -105,5 +120,7 @@ export function verifyCheckout(root = process.cwd(), executable = 'git'): void { if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { if (!process.argv[2]) throw new Error('Checkout verification requires a pinned Git executable.'); - verifyCheckout(process.cwd(), process.argv[2]); + if (process.argv[3] === 'gitlinks') process.stdout.write(gitlinkIndexDigest(process.cwd(), process.argv[2])); + else if (process.argv[3] === undefined) verifyCheckout(process.cwd(), process.argv[2]); + else throw new Error('Unknown checkout verification mode.'); } diff --git a/test/runner-rebase.test.ts b/test/runner-rebase.test.ts index 5da561eb..1b6b1fc2 100644 --- a/test/runner-rebase.test.ts +++ b/test/runner-rebase.test.ts @@ -372,6 +372,36 @@ describe('trusted pre-merge rebase', () => { ledger: [{ sha: foreign, owner: null, origin: 'foreign' }] })).rejects.toThrow(/outside the conflicted set/); }); + it('preserves an unchanged gitlink from the conflicted commit', async () => { + const s = await setup('foreign'), attemptId = randomUUID(); + git(s.source, 'switch', '-q', 'feature'); + git(s.source, 'update-index', '--add', '--cacheinfo', `160000,${s.base},submodule`); + git(s.source, 'commit', '--amend', '--no-edit', '-q'); + const foreign = git(s.source, 'rev-parse', 'HEAD'); + await ensureCommit(s.repository, foreign); + const runner = createRebaser(s, { resolveForeignConflict: async input => { + writeFileSync(join(input.repository, 'a.txt'), 'resolved foreign change\n'); + } }); + const result = await runner.run({ attemptId, oldBase: s.base, oldHead: foreign, oldHistory: [s.owned, foreign], onto: s.onto, + ledger: [{ sha: foreign, owner: null, origin: 'foreign' }] }); + expect(git(s.repository.path, 'ls-tree', result.head, '--', 'submodule')).toContain(`commit ${s.base}\tsubmodule`); + }); + + it('refuses a resolver index-only change to a gitlink outside the conflict', async () => { + const s = await setup('foreign'), attemptId = randomUUID(); + git(s.source, 'switch', '-q', 'feature'); + git(s.source, 'update-index', '--add', '--cacheinfo', `160000,${s.base},submodule`); + git(s.source, 'commit', '--amend', '--no-edit', '-q'); + const foreign = git(s.source, 'rev-parse', 'HEAD'); + await ensureCommit(s.repository, foreign); + const runner = createRebaser(s, { resolveForeignConflict: async input => { + writeFileSync(join(input.repository, 'a.txt'), 'resolved foreign change\n'); + git(input.repository, 'update-index', '--cacheinfo', `160000,${s.onto},submodule`); + } }); + await expect(runner.run({ attemptId, oldBase: s.base, oldHead: foreign, oldHistory: [s.owned, foreign], onto: s.onto, + ledger: [{ sha: foreign, owner: null, origin: 'foreign' }] })).rejects.toThrow(/outside the conflicted set/); + }); + it('refuses a resolver edit outside the exact conflicted files and drops the partial rewrite', async () => { const s = await setup('foreign'), attemptId = randomUUID(); const runner = createRebaser(s, { resolveForeignConflict: async input => { From 34abc4d7f9db80a6decfa68d6e9e4fe42366e445 Mon Sep 17 00:00:00 2001 From: mchwang Date: Tue, 6 Oct 2026 15:06:15 -0700 Subject: [PATCH 04/11] Harden conflict path auditing --- docs/implementation/pre-merge-rebase.md | 4 +- runner/list-git-paths.ts | 34 +++++++++++++++ runner/rebase.ts | 57 +++++++++++++++++++------ runner/verify-checkout.ts | 10 ++--- test/runner-rebase.test.ts | 50 +++++++++++++++++++--- 5 files changed, 129 insertions(+), 26 deletions(-) create mode 100644 runner/list-git-paths.ts diff --git a/docs/implementation/pre-merge-rebase.md b/docs/implementation/pre-merge-rebase.md index 4bd5e4bc..0e5f6485 100644 --- a/docs/implementation/pre-merge-rebase.md +++ b/docs/implementation/pre-merge-rebase.md @@ -18,7 +18,7 @@ Admission checks the current plan revision, snapshot, review version, task state After replay settles and before its first result-ref write, the rebaser supplies the Store's preparation hook only the ordered rewritten history. The Store zips that history by position with the independently captured `oldHistory`, then durably binds the resulting complete mapping and exact intended ref value as `prepared`. The atomic create outcome advances it to `ready`, `uncertain`, or `refused`; an already-current no-op uses `prepared` then `ready` without a ref write. `Store.finishRebase` accepts only a `ready`, byte-for-byte mapping from the same attempt while the captured plan, snapshot, review version, task version, old base, old head, and target base are unchanged. A non-empty mapping must end at both the captured old head and the committed rewritten head; an empty mapping is valid only when the captured history is empty and the result equals the new base. The snapshot, rewrite map, and owned/foreign ledger provenance commit atomically, and the marker clears in that transaction. A stale result changes nothing and leaves its marker for exact-attempt cleanup. -For each conflict, classification uses only the supplied trusted ledger. A missing entry and an explicit `{ owner: null, origin: "foreign" }` entry take the foreign branch; trailers are never read. An owned entry is refused until the separate owned-commit resolver exists. The injected foreign resolver receives the source commit and exact conflicting path set as a read-only string array. Before and after it runs, the rebaser snapshots every non-conflict changed path and hashes every index-only gitlink entry without entering a nested repository. After the resolver settles, the rebaser proves `REBASE_HEAD` is unchanged, rejects every tracked or untracked edit outside the conflict set, stages only literal NUL-delimited path data on stdin, and refuses any remaining unmerged entry. A rejection or invocation failure aborts and removes the partial rewrite. The production adapter must expose the rebase worktree only through lane D's bounded task storage; this engine callback is not permission to mount the host worktree directly. +For each conflict, classification uses only the supplied trusted ledger. A missing entry and an explicit `{ owner: null, origin: "foreign" }` entry take the foreign branch; trailers are never read. An owned entry is refused until the separate owned-commit resolver exists. A gitlink conflict is also refused for manual resolution before the file resolver runs; the boundary never enters a nested repository. The injected foreign resolver receives the source commit and exact non-gitlink conflicting path set as a read-only string array. Raw Git path streams use the same explicit 32 MiB and 262,144-entry bounds as checkout verification, travel between processes as base64, and undergo fatal UTF-8 decoding so malformed names fail closed without rejecting a valid literal replacement character. Before and after the resolver runs, the rebaser snapshots every non-conflict changed path against the same pinned `HEAD` and hashes every index-only gitlink entry. After the resolver settles, the rebaser proves both `HEAD` and `REBASE_HEAD` are unchanged, rejects every tracked or untracked edit outside the conflict set, stages only literal NUL-delimited path data on stdin, and refuses any remaining unmerged entry. A rejection or invocation failure aborts and removes the partial rewrite. The production adapter must expose the rebase worktree only through lane D's bounded task storage; this engine callback is not permission to mount the host worktree directly. The Store records each resolved source SHA before the result ref write. Preparation refuses duplicates, sources outside the captured history, owned sources, and sources that did not actually rewrite. On publication, the mapped commit retains `owner: null`, `origin: foreign`, and `sourceSha`, plus `conflictResolved: true`. Linking therefore keeps its lines in Unplanned while the review UI adds the text label “conflict resolved by agent.” The commit author is preserved by Git's replay and no Plan-Item trailer is added. @@ -32,4 +32,4 @@ This ordering is intentionally local-only. The later push integration must exten ## Regression evidence -`test/runner-rebase.test.ts` covers clean one-to-one replay, the already-current no-op, process-group ownership, the whole-operation deadline, foreign conflict resolution, clean sibling files, gitlink-pointer auditing, invalid ledger classification, outside-file refusal, owned-conflict refusal, cancellation before and during Git, exact cleanup, retained refs, and a read-only source checkout. `test/runner-lifecycle-store.test.ts` covers admission, durable process ownership, compare-and-swap races, resolved-source validation, and owned/foreign provenance. `test/runner-recovery.test.ts` covers marker validation, process termination and restart ordering plus fail-closed absence of a recovery implementation. `test/history.test.ts` and `test/browser/review.spec.ts` cover Unplanned attribution and the visible conflict-resolution label. `test/runner-production.test.ts` covers exact-plan recovery routing. +`test/runner-rebase.test.ts` covers clean one-to-one replay, the already-current no-op, process-group ownership, the whole-operation deadline, foreign conflict resolution, clean sibling files, gitlink-pointer auditing and manual gitlink-conflict routing, literal replacement-character paths, invalid ledger classification, outside-file and resolver-commit refusal, owned-conflict refusal, cancellation before and during Git, exact cleanup, retained refs, and a read-only source checkout. `test/runner-lifecycle-store.test.ts` covers admission, durable process ownership, compare-and-swap races, resolved-source validation, and owned/foreign provenance. `test/runner-recovery.test.ts` covers marker validation, process termination and restart ordering plus fail-closed absence of a recovery implementation. `test/history.test.ts` and `test/browser/review.spec.ts` cover Unplanned attribution and the visible conflict-resolution label. `test/runner-production.test.ts` covers exact-plan recovery routing. diff --git a/runner/list-git-paths.ts b/runner/list-git-paths.ts new file mode 100644 index 00000000..61eb0c36 --- /dev/null +++ b/runner/list-git-paths.ts @@ -0,0 +1,34 @@ +import { spawnSync } from 'node:child_process'; +import { pathToFileURL } from 'node:url'; +import { GIT_OPTIONS, gitEnvironment } from '../git/clone.ts'; +import { MAX_INDEX_BYTES, MAX_INDEX_ENTRIES } from './verify-checkout.ts'; + +const MAX_GIT_OUTPUT_BYTES = MAX_INDEX_BYTES * 4 + 1024; + +type PathListMode = 'conflicts' | 'changed' | 'untracked' | 'unmerged'; + +/** Return one bounded raw NUL stream as base64 so the parent can perform fatal UTF-8 decoding without replacement. */ +export function listGitPaths(root: string, executable: string, mode: PathListMode, head?: string): string { + if (mode === 'changed' && !/^(?:[0-9a-f]{40}|[0-9a-f]{64})$/.test(head ?? '')) + throw new Error('A full pinned head is required for the changed-path scan.'); + if (mode !== 'changed' && head !== undefined) throw new Error('This path scan does not accept a head.'); + const command = mode === 'conflicts' ? ['diff', '--name-only', '--diff-filter=U', '-z', '--'] + : mode === 'changed' ? ['diff', '--name-only', '--ignore-submodules=all', '-z', head!, '--'] + : mode === 'untracked' ? ['ls-files', '--others', '-z', '--'] + : ['ls-files', '--unmerged', '-z', '--']; + const result = spawnSync(executable, [...GIT_OPTIONS, '-c', 'gc.auto=0', '-c', 'maintenance.auto=false', ...command], + { cwd: root, env: gitEnvironment(), maxBuffer: MAX_GIT_OUTPUT_BYTES, stdio: ['ignore', 'pipe', 'pipe'] }); + if (result.status !== 0 || result.error) throw result.error ?? new Error(`git ${command[0]} failed while listing paths.`); + const output = result.stdout; + if (output.length > MAX_INDEX_BYTES) throw new Error('Git path output exceeds its byte bound.'); + let entries = 0; + for (const byte of output) if (byte === 0 && ++entries > MAX_INDEX_ENTRIES) + throw new Error('Git path output exceeds its entry bound.'); + if (output.length && output[output.length - 1] !== 0) throw new Error('Git returned an unterminated path record.'); + return output.toString('base64'); +} + +if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { + if (!process.argv[2] || !process.argv[3]) throw new Error('Path listing requires a pinned Git executable and mode.'); + process.stdout.write(listGitPaths(process.cwd(), process.argv[2], process.argv[3] as PathListMode, process.argv[4])); +} diff --git a/runner/rebase.ts b/runner/rebase.ts index 0ba4a3ba..edfc8f28 100644 --- a/runner/rebase.ts +++ b/runner/rebase.ts @@ -7,6 +7,7 @@ import type { DockerOutcome } from '../agents/docker.ts'; import { GIT_OPTIONS, gitEnvironment } from '../git/clone.ts'; import { isUuidV4 } from './lifecycle.ts'; import { ownerOnlyDirectory, type RunnerRepository } from './runner-repository.ts'; +import { MAX_INDEX_BYTES } from './verify-checkout.ts'; const COMMIT_ID = /^(?:[0-9a-f]{40}|[0-9a-f]{64})$/; const MAX_REBASE_COMMITS = 500; @@ -164,16 +165,21 @@ export class GitRebaser { throw new RebaseConflict('An owned commit conflict needs its plan-item resolver.'); if (!this.#options.resolveForeignConflict) throw new RebaseConflict('A foreign commit conflict needs review.'); - const files = Object.freeze(await this.#paths(path, ['diff', '--name-only', '--diff-filter=U', '-z', '--'], scope)); + const files = Object.freeze(await this.#paths(path, 'conflicts', scope)); if (!files.length) throw new RebaseConflict('Git reported a conflict without any conflicted files.'); + if (await this.#hasUnmergedGitlink(path, scope)) + throw new RebaseConflict('A gitlink conflict needs manual resolution.'); const allowed = new Set(files); - const outsideBefore = await this.#outsideConflictState(path, allowed, scope); + const priorHead = await this.#git(path, ['rev-parse', '--verify', 'HEAD^{commit}'], scope); + const outsideBefore = await this.#outsideConflictState(path, allowed, priorHead, scope); this.#assertResultCurrent(scope); await this.#resolveConflict({ attemptId, commit, files, repository: path }, scope); this.#assertResultCurrent(scope); if (await this.#git(path, ['rev-parse', '--verify', 'REBASE_HEAD^{commit}'], scope) !== commit) throw new RebaseConflict('The conflict resolver changed the rebase operation.'); - if (await this.#outsideConflictState(path, allowed, scope) !== outsideBefore) + if (await this.#git(path, ['rev-parse', '--verify', 'HEAD^{commit}'], scope) !== priorHead) + throw new RebaseConflict('The conflict resolver changed the rebase operation.'); + if (await this.#outsideConflictState(path, allowed, priorHead, scope) !== outsideBefore) throw new RebaseConflict('The conflict resolver changed a file outside the conflicted set.'); const pathspec = Buffer.from(`${files.join('\0')}\0`); await this.#git(path, ['--literal-pathspecs', 'add', '--all', '--pathspec-from-file=-', '--pathspec-file-nul'], scope, false, pathspec); @@ -325,19 +331,42 @@ export class GitRebaser { await this.#git(this.#options.repository.path, ['worktree', 'prune'], scope, true); } - async #paths(repository: string, args: readonly string[], scope: CallScope): Promise { - const outcome = await this.#call(repository, args, scope); + async #pathRecords(repository: string, mode: 'conflicts' | 'changed' | 'untracked' | 'unmerged', scope: CallScope, + head?: string): Promise { + const helper = fileURLToPath(new URL('./list-git-paths.ts', import.meta.url)); + const args = [helper, this.#gitExecutable, mode, ...(head ? [head] : [])]; + // Base64 expands the explicitly bounded 32 MiB raw stream by four thirds. + const outcome = await this.#process(process.execPath, args, repository, this.#gitEnvironment, scope, false, + Math.ceil(MAX_INDEX_BYTES / 3) * 4 + 4); this.#throwIfCancelled(outcome, scope); - if (outcome.status !== 0) throw this.#failure(args[0] ?? 'git', outcome); - // Lane D's path APIs are strings. Refuse undecodable names rather than let UTF-8 replacement alias two paths. - if (outcome.stdout.includes('\ufffd') || (outcome.stdout && !outcome.stdout.endsWith('\0'))) - throw new RebaseConflict('A conflicted path cannot be represented safely; resolve it manually.'); - const paths = outcome.stdout ? outcome.stdout.slice(0, -1).split('\0') : []; - if (paths.some(path => !path || path.includes('\0'))) throw new RebaseConflict('Git returned an invalid conflicted path.'); + if (outcome.status !== 0) throw this.#failure('path listing', outcome, 'node'); + const encoded = outcome.stdout; + const raw = Buffer.from(encoded, 'base64'); + if (raw.toString('base64') !== encoded || (raw.length && raw[raw.length - 1] !== 0)) + throw new RebaseConflict('Git returned an invalid path listing.'); + const records = raw.length ? raw.subarray(0, -1).toString('binary').split('\0').map(value => Buffer.from(value, 'binary')) : []; + const decoder = new TextDecoder('utf-8', { fatal: true }); + try { return records.map(record => decoder.decode(record)); } + catch { throw new RebaseConflict('A Git path cannot be represented safely; resolve it manually.'); } + } + + async #paths(repository: string, mode: 'conflicts' | 'changed' | 'untracked', scope: CallScope, head?: string): Promise { + const paths = await this.#pathRecords(repository, mode, scope, head); + if (paths.some(path => !path || path.includes('\0'))) throw new RebaseConflict('Git returned an invalid path.'); return paths; } - async #outsideConflictState(repository: string, allowed: ReadonlySet, scope: CallScope): Promise { + async #hasUnmergedGitlink(repository: string, scope: CallScope): Promise { + const records = await this.#pathRecords(repository, 'unmerged', scope); + return records.some(record => { + const tab = record.indexOf('\t'), header = record.slice(0, tab).split(' '); + if (tab < 0 || header.length !== 3 || !/^[0-3]$/.test(header[2]!)) + throw new RebaseConflict('Git returned an invalid unmerged index record.'); + return header[0] === '160000'; + }); + } + + async #outsideConflictState(repository: string, allowed: ReadonlySet, head: string, scope: CallScope): Promise { const verifier = fileURLToPath(new URL('./verify-checkout.ts', import.meta.url)); const gitlinks = await this.#process(process.execPath, [verifier, this.#gitExecutable, 'gitlinks'], repository, this.#gitEnvironment, scope, false, 64 * 1024); @@ -345,8 +374,8 @@ export class GitRebaser { if (gitlinks.status !== 0 || !/^[0-9a-f]{64}$/.test(gitlinks.stdout)) throw this.#failure('gitlink index audit', gitlinks, 'node'); const changed = new Set([ - ...await this.#paths(repository, ['diff', '--name-only', '--ignore-submodules=all', '-z', 'HEAD', '--'], scope), - ...await this.#paths(repository, ['ls-files', '--others', '-z', '--'], scope), + ...await this.#paths(repository, 'changed', scope, head), + ...await this.#paths(repository, 'untracked', scope), ]); const entries: string[][] = []; for (const path of [...changed].filter(value => !allowed.has(value)).sort()) { diff --git a/runner/verify-checkout.ts b/runner/verify-checkout.ts index 55852daa..93d701f8 100644 --- a/runner/verify-checkout.ts +++ b/runner/verify-checkout.ts @@ -4,8 +4,8 @@ import { spawnSync } from 'node:child_process'; import { pathToFileURL } from 'node:url'; import { GIT_OPTIONS, gitEnvironment } from '../git/clone.ts'; -const MAX_INDEX_BYTES = 32 * 1024 * 1024; -const MAX_DIRECTORY_ENTRIES = 262_144; +export const MAX_INDEX_BYTES = 32 * 1024 * 1024; +export const MAX_INDEX_ENTRIES = 262_144; // A C-quoted path can expand every input byte to four bytes. Keep transport above every accepted index listing; // accepted input is then rejected by our explicit bounds, never by spawnSync's smaller implementation limit. const MAX_GIT_OUTPUT_BYTES = MAX_INDEX_BYTES * 4 + 1024; @@ -40,7 +40,7 @@ export function gitlinkIndexDigest(root = process.cwd(), executable = 'git'): st const rawOutput = git(executable, ['-C', root, 'ls-files', '--stage', '-z']); if (rawOutput.length > MAX_INDEX_BYTES) throw new Error('Checkout index exceeds its byte bound.'); const raw = split0(rawOutput); - if (raw.length > MAX_DIRECTORY_ENTRIES) throw new Error('Checkout index exceeds its entry bound.'); + if (raw.length > MAX_INDEX_ENTRIES) throw new Error('Checkout index exceeds its entry bound.'); const hash = createHash('sha256'); for (const record of raw) { const tab = record.indexOf(0x09), header = record.subarray(0, tab).toString('ascii').split(' '); @@ -56,7 +56,7 @@ export function verifyCheckout(root = process.cwd(), executable = 'git'): void { const rawOutput = repositoryGit(['ls-files', '--stage', '-z']); if (rawOutput.length > MAX_INDEX_BYTES) throw new Error('Checkout index exceeds its byte bound.'); const raw = split0(rawOutput); - if (raw.length > MAX_DIRECTORY_ENTRIES) throw new Error('Checkout index exceeds its entry bound.'); + if (raw.length > MAX_INDEX_ENTRIES) throw new Error('Checkout index exceeds its entry bound.'); const quotedOutput = repositoryGit(['ls-files', '--stage']).toString('utf8'); if (quotedOutput && !quotedOutput.endsWith('\n')) throw new Error('Git returned an unterminated quoted index listing.'); const quoted = quotedOutput ? quotedOutput.slice(0, -1).split('\n') : []; @@ -77,7 +77,7 @@ export function verifyCheckout(root = process.cwd(), executable = 'git'): void { const names = new Set(); try { for (let entry = opened.readSync(); entry; entry = opened.readSync()) { - if (++entries > MAX_DIRECTORY_ENTRIES) throw new Error('Checkout verification exceeded its directory-entry bound.'); + if (++entries > MAX_INDEX_ENTRIES) throw new Error('Checkout verification exceeded its directory-entry bound.'); names.add((entry.name as unknown as Buffer).toString('hex')); } } finally { opened.closeSync(); } diff --git a/test/runner-rebase.test.ts b/test/runner-rebase.test.ts index 1b6b1fc2..c7c8a010 100644 --- a/test/runner-rebase.test.ts +++ b/test/runner-rebase.test.ts @@ -24,20 +24,20 @@ function commit(repository: string, path: string, text: string, message: string) return git(repository, 'rev-parse', 'HEAD'); } -async function setup(conflict: boolean | 'foreign' = false) { +async function setup(conflict: boolean | 'foreign' = false, conflictPath = 'a.txt') { const root = mkdtempSync(join(tmpdir(), 'codeboost-rebase-')); roots.push(root); const source = join(root, 'source'); git(root, 'init', '-q', '-b', 'main', source); git(source, 'config', 'user.name', 'Source'); git(source, 'config', 'user.email', 'source@example.invalid'); - const base = commit(source, 'a.txt', 'base\n', 'base'); + const base = commit(source, conflictPath, 'base\n', 'base'); git(source, 'switch', '-qc', 'feature'); - const owned = commit(source, conflict === 'foreign' ? 'owned.txt' : 'a.txt', 'feature\n', 'owned'); + const owned = commit(source, conflict === 'foreign' ? 'owned.txt' : conflictPath, 'feature\n', 'owned'); if (conflict === 'foreign') { git(source, 'config', 'user.name', 'Collaborator'); git(source, 'config', 'user.email', 'collaborator@example.invalid'); } - const foreign = commit(source, conflict === 'foreign' ? 'a.txt' : 'feature.txt', 'collaborator\n', + const foreign = commit(source, conflict === 'foreign' ? conflictPath : 'feature.txt', 'collaborator\n', conflict === 'foreign' ? 'foreign\n\nPlan-Item: P1' : 'foreign'); git(source, 'config', 'user.name', 'Source'); git(source, 'config', 'user.email', 'source@example.invalid'); git(source, 'switch', '-q', 'main'); - const onto = commit(source, conflict ? 'a.txt' : 'base.txt', conflict ? 'main\n' : 'main moved\n', 'move base'); + const onto = commit(source, conflict ? conflictPath : 'base.txt', conflict ? 'main\n' : 'main moved\n', 'move base'); const runnerRoot = join(root, 'runner'); const repository = await openRunnerRepository({ runnerRoot, runnerOwner: OWNER, repositoryId: 'repo', source }); await ensureCommit(repository, foreign); await ensureCommit(repository, onto); @@ -402,6 +402,46 @@ describe('trusted pre-merge rebase', () => { ledger: [{ sha: foreign, owner: null, origin: 'foreign' }] })).rejects.toThrow(/outside the conflicted set/); }); + it('refuses a resolver commit that moves HEAD and smuggles an outside file', async () => { + const s = await setup('foreign'), attemptId = randomUUID(); + const runner = createRebaser(s, { resolveForeignConflict: async input => { + writeFileSync(join(input.repository, 'a.txt'), 'resolved foreign change\n'); + writeFileSync(join(input.repository, 'smuggled.txt'), 'outside conflict\n'); + git(input.repository, 'add', '--all'); + git(input.repository, '-c', 'user.name=Resolver', '-c', 'user.email=resolver@example.invalid', 'commit', '-qm', 'smuggle'); + } }); + await expect(runner.run({ attemptId, oldBase: s.base, oldHead: s.foreign, oldHistory: s.history, onto: s.onto, + ledger: [{ sha: s.foreign, owner: null, origin: 'foreign' }] })).rejects.toThrow(/changed the rebase operation/); + }); + + it('routes a gitlink conflict to manual review without invoking the file resolver', async () => { + const s = await setup('foreign'), attemptId = randomUUID(); + git(s.source, 'switch', '-q', 'feature'); + git(s.source, 'update-index', '--add', '--cacheinfo', `160000,${s.base},submodule`); + git(s.source, 'commit', '--amend', '--no-edit', '-q'); + const foreign = git(s.source, 'rev-parse', 'HEAD'); + git(s.source, 'switch', '-q', 'main'); + git(s.source, 'update-index', '--add', '--cacheinfo', `160000,${s.owned},submodule`); + git(s.source, 'commit', '--amend', '--no-edit', '-q'); + const onto = git(s.source, 'rev-parse', 'HEAD'); + await ensureCommit(s.repository, foreign); await ensureCommit(s.repository, onto); + const resolveForeignConflict = vi.fn(async () => {}); + const runner = createRebaser(s, { resolveForeignConflict }); + await expect(runner.run({ attemptId, oldBase: s.base, oldHead: foreign, oldHistory: [s.owned, foreign], onto, + ledger: [{ sha: foreign, owner: null, origin: 'foreign' }] })).rejects.toThrow(/gitlink conflict needs manual/); + expect(resolveForeignConflict).not.toHaveBeenCalled(); + }); + + it('passes a literal replacement character in a valid UTF-8 conflict path to the resolver', async () => { + const name = '\ufffd.txt', s = await setup('foreign', name), attemptId = randomUUID(); + const resolveForeignConflict = vi.fn(async input => { writeFileSync(join(input.repository, name), 'resolved\n'); }); + const runner = createRebaser(s, { resolveForeignConflict }); + await expect(runner.run({ attemptId, oldBase: s.base, oldHead: s.foreign, oldHistory: s.history, onto: s.onto, + ledger: [{ sha: s.foreign, owner: null, origin: 'foreign' }] })).resolves.toMatchObject({ resolvedConflicts: [s.foreign] }); + expect(resolveForeignConflict).toHaveBeenCalledOnce(); + expect(resolveForeignConflict.mock.calls[0]![0].files).toEqual([name]); + }); + it('refuses a resolver edit outside the exact conflicted files and drops the partial rewrite', async () => { const s = await setup('foreign'), attemptId = randomUUID(); const runner = createRebaser(s, { resolveForeignConflict: async input => { From 8f410305cd97e2e2eb16b84b6e35c869ff91abc5 Mon Sep 17 00:00:00 2001 From: mchwang Date: Tue, 6 Oct 2026 15:20:21 -0700 Subject: [PATCH 05/11] Freeze rebase control state during resolution --- docs/implementation/pre-merge-rebase.md | 4 +- runner/hash-rebase-state.ts | 61 +++++++++++++++++++++++++ runner/rebase.ts | 34 +++++++++++--- test/runner-rebase.test.ts | 32 ++++++++++++- 4 files changed, 122 insertions(+), 9 deletions(-) create mode 100644 runner/hash-rebase-state.ts diff --git a/docs/implementation/pre-merge-rebase.md b/docs/implementation/pre-merge-rebase.md index 0e5f6485..222abd42 100644 --- a/docs/implementation/pre-merge-rebase.md +++ b/docs/implementation/pre-merge-rebase.md @@ -18,7 +18,7 @@ Admission checks the current plan revision, snapshot, review version, task state After replay settles and before its first result-ref write, the rebaser supplies the Store's preparation hook only the ordered rewritten history. The Store zips that history by position with the independently captured `oldHistory`, then durably binds the resulting complete mapping and exact intended ref value as `prepared`. The atomic create outcome advances it to `ready`, `uncertain`, or `refused`; an already-current no-op uses `prepared` then `ready` without a ref write. `Store.finishRebase` accepts only a `ready`, byte-for-byte mapping from the same attempt while the captured plan, snapshot, review version, task version, old base, old head, and target base are unchanged. A non-empty mapping must end at both the captured old head and the committed rewritten head; an empty mapping is valid only when the captured history is empty and the result equals the new base. The snapshot, rewrite map, and owned/foreign ledger provenance commit atomically, and the marker clears in that transaction. A stale result changes nothing and leaves its marker for exact-attempt cleanup. -For each conflict, classification uses only the supplied trusted ledger. A missing entry and an explicit `{ owner: null, origin: "foreign" }` entry take the foreign branch; trailers are never read. An owned entry is refused until the separate owned-commit resolver exists. A gitlink conflict is also refused for manual resolution before the file resolver runs; the boundary never enters a nested repository. The injected foreign resolver receives the source commit and exact non-gitlink conflicting path set as a read-only string array. Raw Git path streams use the same explicit 32 MiB and 262,144-entry bounds as checkout verification, travel between processes as base64, and undergo fatal UTF-8 decoding so malformed names fail closed without rejecting a valid literal replacement character. Before and after the resolver runs, the rebaser snapshots every non-conflict changed path against the same pinned `HEAD` and hashes every index-only gitlink entry. After the resolver settles, the rebaser proves both `HEAD` and `REBASE_HEAD` are unchanged, rejects every tracked or untracked edit outside the conflict set, stages only literal NUL-delimited path data on stdin, and refuses any remaining unmerged entry. A rejection or invocation failure aborts and removes the partial rewrite. The production adapter must expose the rebase worktree only through lane D's bounded task storage; this engine callback is not permission to mount the host worktree directly. +For each conflict, classification uses only the supplied trusted ledger. A missing entry and an explicit `{ owner: null, origin: "foreign" }` entry take the foreign branch; trailers are never read. An owned entry is refused until the separate owned-commit resolver exists. A gitlink conflict is also refused for manual resolution before the file resolver runs; the boundary never enters a nested repository. The injected foreign resolver receives the source commit and exact non-gitlink conflicting path set as a read-only string array. Raw Git path streams use the same explicit 32 MiB and 262,144-entry bounds as checkout verification, travel between processes as base64, and undergo fatal UTF-8 decoding so malformed names fail closed without rejecting a valid literal replacement character. Before and after the resolver runs, the rebaser snapshots every non-conflict changed path against the same pinned `HEAD`, hashes every index-only gitlink entry, and hashes the byte-exact rebase control directory without following links. After the resolver settles, the rebaser proves both `HEAD` and `REBASE_HEAD` are unchanged, rejects every tracked or untracked edit outside the conflict set, refuses any change to the remaining rebase operation, stages only literal NUL-delimited path data on stdin, then rechecks the gitlink index so an allowed file cannot become an embedded repository. It also refuses any remaining unmerged entry. A rejection or invocation failure aborts and removes the partial rewrite. The production adapter must expose the rebase worktree only through lane D's bounded task storage; this engine callback is not permission to mount the host worktree directly. The Store records each resolved source SHA before the result ref write. Preparation refuses duplicates, sources outside the captured history, owned sources, and sources that did not actually rewrite. On publication, the mapped commit retains `owner: null`, `origin: foreign`, and `sourceSha`, plus `conflictResolved: true`. Linking therefore keeps its lines in Unplanned while the review UI adds the text label “conflict resolved by agent.” The commit author is preserved by Git's replay and no Plan-Item trailer is added. @@ -32,4 +32,4 @@ This ordering is intentionally local-only. The later push integration must exten ## Regression evidence -`test/runner-rebase.test.ts` covers clean one-to-one replay, the already-current no-op, process-group ownership, the whole-operation deadline, foreign conflict resolution, clean sibling files, gitlink-pointer auditing and manual gitlink-conflict routing, literal replacement-character paths, invalid ledger classification, outside-file and resolver-commit refusal, owned-conflict refusal, cancellation before and during Git, exact cleanup, retained refs, and a read-only source checkout. `test/runner-lifecycle-store.test.ts` covers admission, durable process ownership, compare-and-swap races, resolved-source validation, and owned/foreign provenance. `test/runner-recovery.test.ts` covers marker validation, process termination and restart ordering plus fail-closed absence of a recovery implementation. `test/history.test.ts` and `test/browser/review.spec.ts` cover Unplanned attribution and the visible conflict-resolution label. `test/runner-production.test.ts` covers exact-plan recovery routing. +`test/runner-rebase.test.ts` covers clean one-to-one replay, the already-current no-op, process-group ownership, the whole-operation deadline, foreign conflict resolution, clean sibling files, gitlink-pointer auditing and manual gitlink-conflict routing, embedded-repository refusal, literal replacement-character paths, invalid ledger classification, outside-file, resolver-commit and rebase-control refusal, owned-conflict refusal, cancellation before and during Git, exact cleanup, retained refs, and a read-only source checkout. `test/runner-lifecycle-store.test.ts` covers admission, durable process ownership, compare-and-swap races, resolved-source validation, and owned/foreign provenance. `test/runner-recovery.test.ts` covers marker validation, process termination and restart ordering plus fail-closed absence of a recovery implementation. `test/history.test.ts` and `test/browser/review.spec.ts` cover Unplanned attribution and the visible conflict-resolution label. `test/runner-production.test.ts` covers exact-plan recovery routing. diff --git a/runner/hash-rebase-state.ts b/runner/hash-rebase-state.ts new file mode 100644 index 00000000..66802922 --- /dev/null +++ b/runner/hash-rebase-state.ts @@ -0,0 +1,61 @@ +import { spawnSync } from 'node:child_process'; +import { createHash } from 'node:crypto'; +import { closeSync, constants, fstatSync, lstatSync, opendirSync, openSync, readSync } from 'node:fs'; +import { pathToFileURL } from 'node:url'; +import { GIT_OPTIONS, gitEnvironment } from '../git/clone.ts'; +import { MAX_INDEX_BYTES, MAX_INDEX_ENTRIES } from './verify-checkout.ts'; + +const gitPath = (root: string, executable: string): Buffer => { + const result = spawnSync(executable, [...GIT_OPTIONS, 'rev-parse', '--path-format=absolute', '--git-path', 'rebase-merge'], + { cwd: root, env: gitEnvironment(), maxBuffer: 64 * 1024, stdio: ['ignore', 'pipe', 'pipe'] }); + if (result.status !== 0 || result.error) throw result.error ?? new Error('Git could not locate its rebase state.'); + const output = result.stdout; + if (!output.length || output.includes(0) || output[output.length - 1] !== 0x0a) + throw new Error('Git returned an invalid rebase-state path.'); + return output.subarray(0, output.length - 1); +}; + +/** Hash the byte-exact rebase control directory without following any link. */ +export function rebaseStateDigest(root = process.cwd(), executable = 'git'): string { + const directory = gitPath(root, executable), initial = lstatSync(directory); + if (!initial.isDirectory() || initial.isSymbolicLink()) throw new Error('Git rebase state is not a plain directory.'); + const hash = createHash('sha256'), pending: { full: Buffer; relative: Buffer }[] = [{ full: directory, relative: Buffer.alloc(0) }]; + let entries = 0, bytes = 0; + while (pending.length) { + const current = pending.pop()!, opened = opendirSync(current.full, { encoding: 'buffer' as BufferEncoding }); + const children: Buffer[] = []; + try { for (let entry = opened.readSync(); entry; entry = opened.readSync()) children.push(entry.name as unknown as Buffer); } + finally { opened.closeSync(); } + children.sort(Buffer.compare); + for (const name of children) { + if (++entries > MAX_INDEX_ENTRIES) throw new Error('Git rebase state exceeds its entry bound.'); + const full = Buffer.concat([current.full, Buffer.from('/'), name]); + const relative = current.relative.length ? Buffer.concat([current.relative, Buffer.from('/'), name]) : name; + const stat = lstatSync(full); + hash.update(String(relative.length)).update(':').update(relative).update(':').update(String(stat.mode)).update(':'); + if (stat.isDirectory() && !stat.isSymbolicLink()) { + hash.update('directory\0'); pending.push({ full, relative }); continue; + } + if (!stat.isFile() || stat.isSymbolicLink()) throw new Error('Git rebase state contains an unsupported entry.'); + const fd = openSync(full, constants.O_RDONLY | constants.O_NOFOLLOW); + try { + const openedStat = fstatSync(fd); + if (!openedStat.isFile()) throw new Error('Git rebase state changed type while it was audited.'); + hash.update(`file:${openedStat.size}\0`); + const buffer = Buffer.allocUnsafe(64 * 1024); + for (;;) { + const length = readSync(fd, buffer, 0, buffer.length, null); + if (!length) break; + if ((bytes += length) > MAX_INDEX_BYTES) throw new Error('Git rebase state exceeds its byte bound.'); + hash.update(buffer.subarray(0, length)); + } + } finally { closeSync(fd); } + } + } + return hash.digest('hex'); +} + +if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { + if (!process.argv[2]) throw new Error('Rebase-state hashing requires a pinned Git executable.'); + process.stdout.write(rebaseStateDigest(process.cwd(), process.argv[2])); +} diff --git a/runner/rebase.ts b/runner/rebase.ts index edfc8f28..78ca873f 100644 --- a/runner/rebase.ts +++ b/runner/rebase.ts @@ -171,7 +171,9 @@ export class GitRebaser { throw new RebaseConflict('A gitlink conflict needs manual resolution.'); const allowed = new Set(files); const priorHead = await this.#git(path, ['rev-parse', '--verify', 'HEAD^{commit}'], scope); + const gitlinksBefore = await this.#gitlinkState(path, scope); const outsideBefore = await this.#outsideConflictState(path, allowed, priorHead, scope); + const operationBefore = await this.#rebaseState(path, scope); this.#assertResultCurrent(scope); await this.#resolveConflict({ attemptId, commit, files, repository: path }, scope); this.#assertResultCurrent(scope); @@ -179,10 +181,16 @@ export class GitRebaser { throw new RebaseConflict('The conflict resolver changed the rebase operation.'); if (await this.#git(path, ['rev-parse', '--verify', 'HEAD^{commit}'], scope) !== priorHead) throw new RebaseConflict('The conflict resolver changed the rebase operation.'); + if (await this.#rebaseState(path, scope) !== operationBefore) + throw new RebaseConflict('The conflict resolver changed the rebase operation.'); + if (await this.#gitlinkState(path, scope) !== gitlinksBefore) + throw new RebaseConflict('The conflict resolver changed a gitlink outside the conflicted file set.'); if (await this.#outsideConflictState(path, allowed, priorHead, scope) !== outsideBefore) throw new RebaseConflict('The conflict resolver changed a file outside the conflicted set.'); const pathspec = Buffer.from(`${files.join('\0')}\0`); await this.#git(path, ['--literal-pathspecs', 'add', '--all', '--pathspec-from-file=-', '--pathspec-file-nul'], scope, false, pathspec); + if (await this.#gitlinkState(path, scope) !== gitlinksBefore) + throw new RebaseConflict('Conflict resolution cannot add or change a gitlink.'); const unresolved = await this.#call(path, ['diff', '--quiet', '--ignore-submodules=all', '--diff-filter=U', '--'], scope); this.#throwIfCancelled(unresolved, scope); if (unresolved.status !== 0) { @@ -366,13 +374,27 @@ export class GitRebaser { }); } - async #outsideConflictState(repository: string, allowed: ReadonlySet, head: string, scope: CallScope): Promise { + async #rebaseState(repository: string, scope: CallScope): Promise { + const helper = fileURLToPath(new URL('./hash-rebase-state.ts', import.meta.url)); + const outcome = await this.#process(process.execPath, [helper, this.#gitExecutable], repository, + this.#gitEnvironment, scope, false, 64 * 1024); + this.#throwIfCancelled(outcome, scope); + if (outcome.status !== 0 || !/^[0-9a-f]{64}$/.test(outcome.stdout)) + throw this.#failure('rebase-state audit', outcome, 'node'); + return outcome.stdout; + } + + async #gitlinkState(repository: string, scope: CallScope): Promise { const verifier = fileURLToPath(new URL('./verify-checkout.ts', import.meta.url)); - const gitlinks = await this.#process(process.execPath, [verifier, this.#gitExecutable, 'gitlinks'], repository, + const outcome = await this.#process(process.execPath, [verifier, this.#gitExecutable, 'gitlinks'], repository, this.#gitEnvironment, scope, false, 64 * 1024); - this.#throwIfCancelled(gitlinks, scope); - if (gitlinks.status !== 0 || !/^[0-9a-f]{64}$/.test(gitlinks.stdout)) - throw this.#failure('gitlink index audit', gitlinks, 'node'); + this.#throwIfCancelled(outcome, scope); + if (outcome.status !== 0 || !/^[0-9a-f]{64}$/.test(outcome.stdout)) + throw this.#failure('gitlink index audit', outcome, 'node'); + return outcome.stdout; + } + + async #outsideConflictState(repository: string, allowed: ReadonlySet, head: string, scope: CallScope): Promise { const changed = new Set([ ...await this.#paths(repository, 'changed', scope, head), ...await this.#paths(repository, 'untracked', scope), @@ -382,7 +404,7 @@ export class GitRebaser { const index = await this.#git(repository, ['--literal-pathspecs', 'ls-files', '--stage', '-z', '--', path], scope); entries.push([path, index, this.#worktreeFingerprint(repository, path)]); } - return JSON.stringify([gitlinks.stdout, entries]); + return JSON.stringify(entries); } #worktreeFingerprint(repository: string, path: string): string { diff --git a/test/runner-rebase.test.ts b/test/runner-rebase.test.ts index c7c8a010..7a017558 100644 --- a/test/runner-rebase.test.ts +++ b/test/runner-rebase.test.ts @@ -399,7 +399,7 @@ describe('trusted pre-merge rebase', () => { git(input.repository, 'update-index', '--cacheinfo', `160000,${s.onto},submodule`); } }); await expect(runner.run({ attemptId, oldBase: s.base, oldHead: foreign, oldHistory: [s.owned, foreign], onto: s.onto, - ledger: [{ sha: foreign, owner: null, origin: 'foreign' }] })).rejects.toThrow(/outside the conflicted set/); + ledger: [{ sha: foreign, owner: null, origin: 'foreign' }] })).rejects.toThrow(/gitlink/); }); it('refuses a resolver commit that moves HEAD and smuggles an outside file', async () => { @@ -414,6 +414,23 @@ describe('trusted pre-merge rebase', () => { ledger: [{ sha: s.foreign, owner: null, origin: 'foreign' }] })).rejects.toThrow(/changed the rebase operation/); }); + it('refuses a resolver change to the remaining rebase operation', async () => { + const s = await setup('foreign'), attemptId = randomUUID(); + git(s.source, 'switch', '-q', 'feature'); + const later = commit(s.source, 'later.txt', 'expected later change\n', 'later'); + git(s.source, 'switch', '-qc', 'alternate', s.foreign); + const alternate = commit(s.source, 'smuggled.txt', 'substituted change\n', 'alternate'); + await ensureCommit(s.repository, later); await ensureCommit(s.repository, alternate); + const runner = createRebaser(s, { resolveForeignConflict: async input => { + writeFileSync(join(input.repository, 'a.txt'), 'resolved foreign change\n'); + const todo = git(input.repository, 'rev-parse', '--path-format=absolute', '--git-path', 'rebase-merge/git-rebase-todo'); + writeFileSync(todo, `pick ${alternate} alternate\n`); + } }); + await expect(runner.run({ attemptId, oldBase: s.base, oldHead: later, + oldHistory: [s.owned, s.foreign, later], onto: s.onto, + ledger: [{ sha: s.foreign, owner: null, origin: 'foreign' }] })).rejects.toThrow(/changed the rebase operation/); + }); + it('routes a gitlink conflict to manual review without invoking the file resolver', async () => { const s = await setup('foreign'), attemptId = randomUUID(); git(s.source, 'switch', '-q', 'feature'); @@ -432,6 +449,19 @@ describe('trusted pre-merge rebase', () => { expect(resolveForeignConflict).not.toHaveBeenCalled(); }); + it('refuses replacing an allowed conflicted file with an embedded repository', async () => { + const s = await setup('foreign'), attemptId = randomUUID(); + const runner = createRebaser(s, { resolveForeignConflict: async input => { + const nested = join(input.repository, 'a.txt'); + rmSync(nested); mkdirSync(nested); + git(nested, 'init', '-q', '-b', 'main'); + git(nested, 'config', 'user.name', 'Nested'); git(nested, 'config', 'user.email', 'nested@example.invalid'); + commit(nested, 'nested.txt', 'nested\n', 'nested'); + } }); + await expect(runner.run({ attemptId, oldBase: s.base, oldHead: s.foreign, oldHistory: s.history, onto: s.onto, + ledger: [{ sha: s.foreign, owner: null, origin: 'foreign' }] })).rejects.toThrow(/gitlink/); + }); + it('passes a literal replacement character in a valid UTF-8 conflict path to the resolver', async () => { const name = '\ufffd.txt', s = await setup('foreign', name), attemptId = randomUUID(); const resolveForeignConflict = vi.fn(async input => { writeFileSync(join(input.repository, name), 'resolved\n'); }); From d4ec5dee224b76f08c4578be7ead44c45e6bb42e Mon Sep 17 00:00:00 2001 From: mchwang Date: Tue, 6 Oct 2026 15:29:14 -0700 Subject: [PATCH 06/11] Bound outside conflict auditing --- docs/implementation/pre-merge-rebase.md | 2 +- runner/hash-outside-conflict.ts | 89 +++++++++++++++++++++++++ runner/list-git-paths.ts | 11 +-- runner/rebase.ts | 53 ++++----------- 4 files changed, 107 insertions(+), 48 deletions(-) create mode 100644 runner/hash-outside-conflict.ts diff --git a/docs/implementation/pre-merge-rebase.md b/docs/implementation/pre-merge-rebase.md index 222abd42..506235a3 100644 --- a/docs/implementation/pre-merge-rebase.md +++ b/docs/implementation/pre-merge-rebase.md @@ -18,7 +18,7 @@ Admission checks the current plan revision, snapshot, review version, task state After replay settles and before its first result-ref write, the rebaser supplies the Store's preparation hook only the ordered rewritten history. The Store zips that history by position with the independently captured `oldHistory`, then durably binds the resulting complete mapping and exact intended ref value as `prepared`. The atomic create outcome advances it to `ready`, `uncertain`, or `refused`; an already-current no-op uses `prepared` then `ready` without a ref write. `Store.finishRebase` accepts only a `ready`, byte-for-byte mapping from the same attempt while the captured plan, snapshot, review version, task version, old base, old head, and target base are unchanged. A non-empty mapping must end at both the captured old head and the committed rewritten head; an empty mapping is valid only when the captured history is empty and the result equals the new base. The snapshot, rewrite map, and owned/foreign ledger provenance commit atomically, and the marker clears in that transaction. A stale result changes nothing and leaves its marker for exact-attempt cleanup. -For each conflict, classification uses only the supplied trusted ledger. A missing entry and an explicit `{ owner: null, origin: "foreign" }` entry take the foreign branch; trailers are never read. An owned entry is refused until the separate owned-commit resolver exists. A gitlink conflict is also refused for manual resolution before the file resolver runs; the boundary never enters a nested repository. The injected foreign resolver receives the source commit and exact non-gitlink conflicting path set as a read-only string array. Raw Git path streams use the same explicit 32 MiB and 262,144-entry bounds as checkout verification, travel between processes as base64, and undergo fatal UTF-8 decoding so malformed names fail closed without rejecting a valid literal replacement character. Before and after the resolver runs, the rebaser snapshots every non-conflict changed path against the same pinned `HEAD`, hashes every index-only gitlink entry, and hashes the byte-exact rebase control directory without following links. After the resolver settles, the rebaser proves both `HEAD` and `REBASE_HEAD` are unchanged, rejects every tracked or untracked edit outside the conflict set, refuses any change to the remaining rebase operation, stages only literal NUL-delimited path data on stdin, then rechecks the gitlink index so an allowed file cannot become an embedded repository. It also refuses any remaining unmerged entry. A rejection or invocation failure aborts and removes the partial rewrite. The production adapter must expose the rebase worktree only through lane D's bounded task storage; this engine callback is not permission to mount the host worktree directly. +For each conflict, classification uses only the supplied trusted ledger. A missing entry and an explicit `{ owner: null, origin: "foreign" }` entry take the foreign branch; trailers are never read. An owned entry is refused until the separate owned-commit resolver exists. A gitlink conflict is also refused for manual resolution before the file resolver runs; the boundary never enters a nested repository. The injected foreign resolver receives the source commit and exact non-gitlink conflicting path set as a read-only string array. Raw Git path streams use the same explicit 32 MiB and 262,144-entry bounds as checkout verification, travel between processes as base64, and undergo fatal UTF-8 decoding so malformed names fail closed without rejecting a valid literal replacement character. Before and after the resolver runs, one killable helper batches the index scan and hashes every non-conflict changed path against the same pinned `HEAD`; separate bounded helpers hash every index-only gitlink entry and the byte-exact rebase control directory without following links. These helpers run inside the operation deadline instead of blocking the server event loop or spawning once per path. After the resolver settles, the rebaser proves both `HEAD` and `REBASE_HEAD` are unchanged, rejects every tracked or untracked edit outside the conflict set, refuses any change to the remaining rebase operation, stages only literal NUL-delimited path data on stdin, then rechecks the gitlink index so an allowed file cannot become an embedded repository. It also refuses any remaining unmerged entry. A rejection or invocation failure aborts and removes the partial rewrite. The production adapter must expose the rebase worktree only through lane D's bounded task storage; this engine callback is trusted orchestration, not agent code, and is not permission to mount the host worktree or its shared Git metadata directly. The Store records each resolved source SHA before the result ref write. Preparation refuses duplicates, sources outside the captured history, owned sources, and sources that did not actually rewrite. On publication, the mapped commit retains `owner: null`, `origin: foreign`, and `sourceSha`, plus `conflictResolved: true`. Linking therefore keeps its lines in Unplanned while the review UI adds the text label “conflict resolved by agent.” The commit author is preserved by Git's replay and no Plan-Item trailer is added. diff --git a/runner/hash-outside-conflict.ts b/runner/hash-outside-conflict.ts new file mode 100644 index 00000000..6e16c0fb --- /dev/null +++ b/runner/hash-outside-conflict.ts @@ -0,0 +1,89 @@ +import { spawnSync } from 'node:child_process'; +import { createHash } from 'node:crypto'; +import { closeSync, constants, fstatSync, lstatSync, openSync, readlinkSync, readSync } from 'node:fs'; +import { pathToFileURL } from 'node:url'; +import { GIT_OPTIONS, gitEnvironment } from '../git/clone.ts'; +import { MAX_INDEX_BYTES, MAX_INDEX_ENTRIES } from './verify-checkout.ts'; + +const MAX_GIT_OUTPUT_BYTES = MAX_INDEX_BYTES * 4 + 1024; +const split0 = (input: Buffer): Buffer[] => { + const values: Buffer[] = []; + let start = 0; + for (let index = 0; index < input.length; index++) if (input[index] === 0) { + if (++values.length > MAX_INDEX_ENTRIES) throw new Error('Git path output exceeds its entry bound.'); + values[values.length - 1] = input.subarray(start, index); start = index + 1; + } + if (start !== input.length) throw new Error('Git returned an unterminated path record.'); + return values; +}; +const git = (root: string, executable: string, args: readonly string[]): Buffer => { + const result = spawnSync(executable, [...GIT_OPTIONS, '-c', 'gc.auto=0', '-c', 'maintenance.auto=false', ...args], + { cwd: root, env: gitEnvironment(), maxBuffer: MAX_GIT_OUTPUT_BYTES, stdio: ['ignore', 'pipe', 'pipe'] }); + if (result.status !== 0 || result.error) throw result.error ?? new Error(`git ${args[0]} failed while auditing conflict scope.`); + if (result.stdout.length > MAX_INDEX_BYTES) throw new Error('Git path output exceeds its byte bound.'); + return result.stdout; +}; +const safePath = (root: Buffer, path: Buffer): Buffer => { + const parts = split0(Buffer.concat([Buffer.from(path).map(byte => byte === 0x2f ? 0 : byte), Buffer.from([0])])); + if (!parts.length || parts.some(part => !part.length || part.equals(Buffer.from('.')) || part.equals(Buffer.from('..')))) + throw new Error('Git returned an unsafe path.'); + return Buffer.concat([root, Buffer.from('/'), path]); +}; + +/** Hash every tracked or untracked state outside the allowed conflict paths in one killable helper process. */ +export function outsideConflictDigest(root: string, executable: string, head: string, allowedInput: Buffer): string { + if (!/^(?:[0-9a-f]{40}|[0-9a-f]{64})$/.test(head)) throw new Error('A full pinned head is required.'); + if (allowedInput.length > MAX_INDEX_BYTES) throw new Error('Allowed conflict paths exceed their byte bound.'); + const allowed = new Set(split0(allowedInput).map(path => path.toString('hex'))); + const changed = new Map(); + let changedBytes = 0; + for (const path of [...split0(git(root, executable, ['diff', '--name-only', '--ignore-submodules=all', '-z', head, '--'])), + ...split0(git(root, executable, ['ls-files', '--others', '-z', '--']))]) { + const key = path.toString('hex'); + if (!allowed.has(key) && !changed.has(key)) { + if (changed.size >= MAX_INDEX_ENTRIES || (changedBytes += path.length + 1) > MAX_INDEX_BYTES) + throw new Error('Changed paths exceed their combined bound.'); + changed.set(key, path); + } + } + const index = new Map(); + for (const record of split0(git(root, executable, ['ls-files', '--stage', '-z']))) { + const tab = record.indexOf(0x09); + if (tab < 0) throw new Error('Git returned an invalid index record.'); + const key = record.subarray(tab + 1).toString('hex'), records = index.get(key) ?? []; + records.push(record); index.set(key, records); + } + const rootBytes = Buffer.from(root), hash = createHash('sha256'), buffer = Buffer.allocUnsafe(64 * 1024); + for (const key of [...changed.keys()].sort()) { + const path = changed.get(key)!, records = index.get(key) ?? []; + hash.update(`path:${path.length}:`).update(path).update(`:index:${records.length}:`); + for (const record of records) hash.update(`${record.length}:`).update(record); + const full = safePath(rootBytes, path), before = lstatSync(full, { throwIfNoEntry: false }); + if (!before) { hash.update('missing\0'); continue; } + if (before.isSymbolicLink()) { + const target = readlinkSync(full, { encoding: 'buffer' }); + hash.update(`link:${before.mode}:${target.length}:`).update(target).update('\0'); continue; + } + if (!before.isFile()) throw new Error('A changed path is not a regular file or symbolic link.'); + const fd = openSync(full, constants.O_RDONLY | constants.O_NOFOLLOW); + try { + const stat = fstatSync(fd); + if (!stat.isFile()) throw new Error('A changed path changed type while it was audited.'); + hash.update(`file:${stat.mode}:${stat.size}:`); + for (;;) { + const length = readSync(fd, buffer, 0, buffer.length, null); + if (!length) break; + hash.update(buffer.subarray(0, length)); + } + hash.update('\0'); + } finally { closeSync(fd); } + } + return hash.digest('hex'); +} + +if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { + if (!process.argv[2] || !process.argv[3]) throw new Error('Outside-conflict hashing requires a pinned Git executable and head.'); + const chunks: Buffer[] = []; + for await (const chunk of process.stdin) chunks.push(chunk); + process.stdout.write(outsideConflictDigest(process.cwd(), process.argv[2], process.argv[3], Buffer.concat(chunks))); +} diff --git a/runner/list-git-paths.ts b/runner/list-git-paths.ts index 61eb0c36..b4259113 100644 --- a/runner/list-git-paths.ts +++ b/runner/list-git-paths.ts @@ -5,16 +5,11 @@ import { MAX_INDEX_BYTES, MAX_INDEX_ENTRIES } from './verify-checkout.ts'; const MAX_GIT_OUTPUT_BYTES = MAX_INDEX_BYTES * 4 + 1024; -type PathListMode = 'conflicts' | 'changed' | 'untracked' | 'unmerged'; +type PathListMode = 'conflicts' | 'unmerged'; /** Return one bounded raw NUL stream as base64 so the parent can perform fatal UTF-8 decoding without replacement. */ -export function listGitPaths(root: string, executable: string, mode: PathListMode, head?: string): string { - if (mode === 'changed' && !/^(?:[0-9a-f]{40}|[0-9a-f]{64})$/.test(head ?? '')) - throw new Error('A full pinned head is required for the changed-path scan.'); - if (mode !== 'changed' && head !== undefined) throw new Error('This path scan does not accept a head.'); +export function listGitPaths(root: string, executable: string, mode: PathListMode): string { const command = mode === 'conflicts' ? ['diff', '--name-only', '--diff-filter=U', '-z', '--'] - : mode === 'changed' ? ['diff', '--name-only', '--ignore-submodules=all', '-z', head!, '--'] - : mode === 'untracked' ? ['ls-files', '--others', '-z', '--'] : ['ls-files', '--unmerged', '-z', '--']; const result = spawnSync(executable, [...GIT_OPTIONS, '-c', 'gc.auto=0', '-c', 'maintenance.auto=false', ...command], { cwd: root, env: gitEnvironment(), maxBuffer: MAX_GIT_OUTPUT_BYTES, stdio: ['ignore', 'pipe', 'pipe'] }); @@ -30,5 +25,5 @@ export function listGitPaths(root: string, executable: string, mode: PathListMod if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { if (!process.argv[2] || !process.argv[3]) throw new Error('Path listing requires a pinned Git executable and mode.'); - process.stdout.write(listGitPaths(process.cwd(), process.argv[2], process.argv[3] as PathListMode, process.argv[4])); + process.stdout.write(listGitPaths(process.cwd(), process.argv[2], process.argv[3] as PathListMode)); } diff --git a/runner/rebase.ts b/runner/rebase.ts index 78ca873f..3f327ca6 100644 --- a/runner/rebase.ts +++ b/runner/rebase.ts @@ -1,6 +1,5 @@ -import { createHash } from 'node:crypto'; -import { accessSync, closeSync, constants, fstatSync, lstatSync, openSync, readlinkSync, readSync, realpathSync, statSync } from 'node:fs'; -import { delimiter, join, resolve, sep } from 'node:path'; +import { accessSync, constants, lstatSync, realpathSync, statSync } from 'node:fs'; +import { delimiter, join, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; import { runInProcessGroup, type ProcessGroup } from '../agents/process-group.ts'; import type { DockerOutcome } from '../agents/docker.ts'; @@ -339,10 +338,9 @@ export class GitRebaser { await this.#git(this.#options.repository.path, ['worktree', 'prune'], scope, true); } - async #pathRecords(repository: string, mode: 'conflicts' | 'changed' | 'untracked' | 'unmerged', scope: CallScope, - head?: string): Promise { + async #pathRecords(repository: string, mode: 'conflicts' | 'unmerged', scope: CallScope): Promise { const helper = fileURLToPath(new URL('./list-git-paths.ts', import.meta.url)); - const args = [helper, this.#gitExecutable, mode, ...(head ? [head] : [])]; + const args = [helper, this.#gitExecutable, mode]; // Base64 expands the explicitly bounded 32 MiB raw stream by four thirds. const outcome = await this.#process(process.execPath, args, repository, this.#gitEnvironment, scope, false, Math.ceil(MAX_INDEX_BYTES / 3) * 4 + 4); @@ -358,8 +356,8 @@ export class GitRebaser { catch { throw new RebaseConflict('A Git path cannot be represented safely; resolve it manually.'); } } - async #paths(repository: string, mode: 'conflicts' | 'changed' | 'untracked', scope: CallScope, head?: string): Promise { - const paths = await this.#pathRecords(repository, mode, scope, head); + async #paths(repository: string, mode: 'conflicts', scope: CallScope): Promise { + const paths = await this.#pathRecords(repository, mode, scope); if (paths.some(path => !path || path.includes('\0'))) throw new RebaseConflict('Git returned an invalid path.'); return paths; } @@ -395,37 +393,14 @@ export class GitRebaser { } async #outsideConflictState(repository: string, allowed: ReadonlySet, head: string, scope: CallScope): Promise { - const changed = new Set([ - ...await this.#paths(repository, 'changed', scope, head), - ...await this.#paths(repository, 'untracked', scope), - ]); - const entries: string[][] = []; - for (const path of [...changed].filter(value => !allowed.has(value)).sort()) { - const index = await this.#git(repository, ['--literal-pathspecs', 'ls-files', '--stage', '-z', '--', path], scope); - entries.push([path, index, this.#worktreeFingerprint(repository, path)]); - } - return JSON.stringify(entries); - } - - #worktreeFingerprint(repository: string, path: string): string { - const full = resolve(repository, path), root = `${resolve(repository)}${sep}`; - if (!full.startsWith(root)) throw new RebaseConflict('Git returned a path outside the rebase workspace.'); - const before = lstatSync(full, { throwIfNoEntry: false }); - if (!before) return 'missing'; - if (before.isSymbolicLink()) return `link:${before.mode}:${readlinkSync(full, { encoding: 'buffer' }).toString('hex')}`; - if (!before.isFile()) throw new RebaseConflict('A changed path is not a regular file or symbolic link.'); - const fd = openSync(full, constants.O_RDONLY | constants.O_NOFOLLOW); - try { - const stat = fstatSync(fd); - if (!stat.isFile()) throw new RebaseConflict('A changed path changed type while it was audited.'); - const hash = createHash('sha256'), buffer = Buffer.allocUnsafe(64 * 1024); - for (;;) { - const length = readSync(fd, buffer, 0, buffer.length, null); - if (!length) break; - hash.update(buffer.subarray(0, length)); - } - return `file:${stat.mode}:${hash.digest('hex')}`; - } finally { closeSync(fd); } + const helper = fileURLToPath(new URL('./hash-outside-conflict.ts', import.meta.url)); + const paths = Buffer.from(`${[...allowed].join('\0')}\0`); + const outcome = await this.#process(process.execPath, [helper, this.#gitExecutable, head], repository, + this.#gitEnvironment, scope, false, 64 * 1024, paths); + this.#throwIfCancelled(outcome, scope); + if (outcome.status !== 0 || !/^[0-9a-f]{64}$/.test(outcome.stdout)) + throw this.#failure('outside-conflict audit', outcome, 'node'); + return outcome.stdout; } async #resolveConflict(input: Omit, scope: CallScope): Promise { From 2005f965dea59140c85367f92eb512f33d6f7518 Mon Sep 17 00:00:00 2001 From: mchwang Date: Tue, 6 Oct 2026 16:09:43 -0700 Subject: [PATCH 07/11] Bound rebase conflict admission --- runner/hash-rebase-state.ts | 26 ++++++++++++++++++++++---- runner/rebase.ts | 11 +++++++---- test/runner-rebase.test.ts | 32 ++++++++++++++++++++++++++++++++ 3 files changed, 61 insertions(+), 8 deletions(-) diff --git a/runner/hash-rebase-state.ts b/runner/hash-rebase-state.ts index 66802922..604a2ded 100644 --- a/runner/hash-rebase-state.ts +++ b/runner/hash-rebase-state.ts @@ -15,20 +15,38 @@ const gitPath = (root: string, executable: string): Buffer => { return output.subarray(0, output.length - 1); }; +export function readBoundedRebaseStateNames(read: () => Buffer | null, + budget: { entries: number; nameBytes: number }, + limits = { entries: MAX_INDEX_ENTRIES, nameBytes: MAX_INDEX_BYTES }): Buffer[] { + const children: Buffer[] = []; + for (let name = read(); name; name = read()) { + if (++budget.entries > limits.entries) throw new Error('Git rebase state exceeds its entry bound.'); + if ((budget.nameBytes += name.length) > limits.nameBytes) + throw new Error('Git rebase state exceeds its name-byte bound.'); + children.push(name); + } + return children; +} + /** Hash the byte-exact rebase control directory without following any link. */ export function rebaseStateDigest(root = process.cwd(), executable = 'git'): string { const directory = gitPath(root, executable), initial = lstatSync(directory); if (!initial.isDirectory() || initial.isSymbolicLink()) throw new Error('Git rebase state is not a plain directory.'); const hash = createHash('sha256'), pending: { full: Buffer; relative: Buffer }[] = [{ full: directory, relative: Buffer.alloc(0) }]; - let entries = 0, bytes = 0; + const enumeration = { entries: 0, nameBytes: 0 }; + let bytes = 0; while (pending.length) { const current = pending.pop()!, opened = opendirSync(current.full, { encoding: 'buffer' as BufferEncoding }); - const children: Buffer[] = []; - try { for (let entry = opened.readSync(); entry; entry = opened.readSync()) children.push(entry.name as unknown as Buffer); } + let children: Buffer[]; + try { + children = readBoundedRebaseStateNames(() => { + const entry = opened.readSync(); + return entry ? entry.name as unknown as Buffer : null; + }, enumeration); + } finally { opened.closeSync(); } children.sort(Buffer.compare); for (const name of children) { - if (++entries > MAX_INDEX_ENTRIES) throw new Error('Git rebase state exceeds its entry bound.'); const full = Buffer.concat([current.full, Buffer.from('/'), name]); const relative = current.relative.length ? Buffer.concat([current.relative, Buffer.from('/'), name]) : name; const stat = lstatSync(full); diff --git a/runner/rebase.ts b/runner/rebase.ts index 3f327ca6..6f83001b 100644 --- a/runner/rebase.ts +++ b/runner/rebase.ts @@ -409,9 +409,12 @@ export class GitRebaser { const abort = () => controller.abort(scope.signal?.reason ?? new Error('The rebase was cancelled.')); if (scope.signal?.aborted) abort(); else scope.signal?.addEventListener('abort', abort, { once: true }); - const timeoutMs = Math.max(0, Math.ceil(scope.workDeadline - performance.now())); - const timer = setTimeout(() => controller.abort(Object.assign(new Error('The rebase deadline expired during conflict resolution.'), - { code: 'ETIMEDOUT' })), timeoutMs); + const deadlineError = () => Object.assign(new Error('The rebase deadline expired during conflict resolution.'), + { code: 'ETIMEDOUT' }); + const remaining = scope.workDeadline - performance.now(); + let timer: NodeJS.Timeout | undefined; + if (remaining <= 0) controller.abort(deadlineError()); + else timer = setTimeout(() => controller.abort(deadlineError()), Math.ceil(remaining)); try { controller.signal.throwIfAborted(); // Await settlement after abort: the resolver owns its container/storage until its promise ends. @@ -422,7 +425,7 @@ export class GitRebaser { } controller.signal.throwIfAborted(); } finally { - clearTimeout(timer); + if (timer) clearTimeout(timer); scope.signal?.removeEventListener('abort', abort); } } diff --git a/test/runner-rebase.test.ts b/test/runner-rebase.test.ts index 7a017558..6fd17ddd 100644 --- a/test/runner-rebase.test.ts +++ b/test/runner-rebase.test.ts @@ -8,6 +8,7 @@ import { fixtureGit as git } from './fixtures/git.ts'; import { GitRebaser, RebaseConflict, rebaseRef, type GitRebaserOptions } from '../runner/rebase.ts'; import { ensureCommit, openRunnerRepository } from '../runner/runner-repository.ts'; import { verifyCheckout } from '../runner/verify-checkout.ts'; +import { readBoundedRebaseStateNames } from '../runner/hash-rebase-state.ts'; const OWNER = '0123456789abcdef0123456789abcdef'; const roots: string[] = []; @@ -56,6 +57,19 @@ function createRebaser(s: Awaited>, options: Partial { + it('bounds rebase-state directory names while they are enumerated', () => { + const names = [Buffer.from('one'), Buffer.from('two'), Buffer.from('three')]; + let index = 0; + expect(() => readBoundedRebaseStateNames(() => names[index++] ?? null, + { entries: 0, nameBytes: 0 }, { entries: 1, nameBytes: 64 })).toThrow(/entry bound/); + expect(index).toBe(2); + + index = 0; + expect(() => readBoundedRebaseStateNames(() => names[index++] ?? null, + { entries: 0, nameBytes: 0 }, { entries: 64, nameBytes: 5 })).toThrow(/name-byte bound/); + expect(index).toBe(2); + }); + it('uses the configured Git PATH inside the checkout verifier subprocess', async () => { const s = await setup(), attemptId = randomUUID(), bin = join(s.root, 'git-bin'), marker = join(s.root, 'verified'); mkdirSync(bin); @@ -513,6 +527,24 @@ describe('trusted pre-merge rebase', () => { await expect(operation).rejects.toThrow(/original cancellation/); }); + it('does not enter the conflict resolver after its work deadline has already elapsed', async () => { + const s = await setup('foreign'), attemptId = randomUUID(); + const realNow = performance.now.bind(performance); + let expire = false, entered = 0; + vi.spyOn(performance, 'now').mockImplementation(() => expire ? Number.MAX_SAFE_INTEGER : realNow()); + const runner = new GitRebaser({ repository: s.repository, runnerRoot: s.runnerRoot, runnerOwner: OWNER, + committer: { name: 'Codeboost', email: 'codeboost@example.invalid' }, + onProcessStarting: () => {}, onProcessGroup: () => {}, onProcessGroupSettled: () => {}, onProcessUnsettled: () => {}, + onResultPrepared: () => {}, onResultState: () => {}, + get resolveForeignConflict() { + expire = true; + return async () => { entered++; }; + } }); + await expect(runner.run({ attemptId, oldBase: s.base, oldHead: s.foreign, oldHistory: s.history, onto: s.onto, + ledger: [{ sha: s.foreign, owner: null, origin: 'foreign' }] })).rejects.toThrow(/deadline expired/); + expect(entered).toBe(0); + }); + it('never sends an owned conflict through the foreign resolver', async () => { const s = await setup(true), attemptId = randomUUID(), resolveForeignConflict = vi.fn(async () => {}); const runner = createRebaser(s, { resolveForeignConflict }); From aee98514905c9b2cf6df7f31c43c6cd7abf43e25 Mon Sep 17 00:00:00 2001 From: mchwang Date: Tue, 6 Oct 2026 16:40:20 -0700 Subject: [PATCH 08/11] Exercise conflict deadline admission --- test/runner-rebase.test.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/runner-rebase.test.ts b/test/runner-rebase.test.ts index 6fd17ddd..6ab10e54 100644 --- a/test/runner-rebase.test.ts +++ b/test/runner-rebase.test.ts @@ -530,19 +530,20 @@ describe('trusted pre-merge rebase', () => { it('does not enter the conflict resolver after its work deadline has already elapsed', async () => { const s = await setup('foreign'), attemptId = randomUUID(); const realNow = performance.now.bind(performance); - let expire = false, entered = 0; + let expire = false, entered = 0, resolverReads = 0; vi.spyOn(performance, 'now').mockImplementation(() => expire ? Number.MAX_SAFE_INTEGER : realNow()); const runner = new GitRebaser({ repository: s.repository, runnerRoot: s.runnerRoot, runnerOwner: OWNER, committer: { name: 'Codeboost', email: 'codeboost@example.invalid' }, onProcessStarting: () => {}, onProcessGroup: () => {}, onProcessGroupSettled: () => {}, onProcessUnsettled: () => {}, onResultPrepared: () => {}, onResultState: () => {}, get resolveForeignConflict() { - expire = true; + if (++resolverReads === 2) expire = true; return async () => { entered++; }; } }); await expect(runner.run({ attemptId, oldBase: s.base, oldHead: s.foreign, oldHistory: s.history, onto: s.onto, ledger: [{ sha: s.foreign, owner: null, origin: 'foreign' }] })).rejects.toThrow(/deadline expired/); expect(entered).toBe(0); + expect(resolverReads).toBe(2); }); it('never sends an owned conflict through the foreign resolver', async () => { From 8123477ddd5974a77f8862d1841b7b7fe6b32a4a Mon Sep 17 00:00:00 2001 From: mchwang Date: Tue, 6 Oct 2026 17:10:24 -0700 Subject: [PATCH 09/11] Harden resolver audit boundaries --- runner/hash-outside-conflict.ts | 10 +++++ runner/rebase.ts | 7 ++- test/runner-rebase.test.ts | 75 +++++++++++++++++++++++++++++++-- 3 files changed, 88 insertions(+), 4 deletions(-) diff --git a/runner/hash-outside-conflict.ts b/runner/hash-outside-conflict.ts index 6e16c0fb..b085406b 100644 --- a/runner/hash-outside-conflict.ts +++ b/runner/hash-outside-conflict.ts @@ -27,6 +27,16 @@ const safePath = (root: Buffer, path: Buffer): Buffer => { const parts = split0(Buffer.concat([Buffer.from(path).map(byte => byte === 0x2f ? 0 : byte), Buffer.from([0])])); if (!parts.length || parts.some(part => !part.length || part.equals(Buffer.from('.')) || part.equals(Buffer.from('..')))) throw new Error('Git returned an unsafe path.'); + const rootStat = lstatSync(root, { throwIfNoEntry: false }); + if (!rootStat?.isDirectory() || rootStat.isSymbolicLink()) throw new Error('The conflict workspace is not a plain directory.'); + let ancestor = root; + for (const part of parts.slice(0, -1)) { + ancestor = Buffer.concat([ancestor, Buffer.from('/'), part]); + const stat = lstatSync(ancestor, { throwIfNoEntry: false }); + if (!stat) break; + if (stat.isSymbolicLink()) throw new Error('A changed path has a symlink ancestor.'); + if (!stat.isDirectory()) throw new Error('A changed path has a non-directory ancestor.'); + } return Buffer.concat([root, Buffer.from('/'), path]); }; diff --git a/runner/rebase.ts b/runner/rebase.ts index 6f83001b..8f21a6ff 100644 --- a/runner/rebase.ts +++ b/runner/rebase.ts @@ -107,7 +107,7 @@ export class GitRebaser { for (const value of [oldBase, oldHead, onto]) if (!COMMIT_ID.test(value)) throw new Error('A full commit ID is required for rebasing.'); const ledger = new Map(); for (const entry of input.ledger ?? []) { - if (!COMMIT_ID.test(entry.sha) || ledger.has(entry.sha) || + if (!COMMIT_ID.test(entry.sha) || ledger.has(entry.sha) || (entry.origin !== 'owned' && entry.origin !== 'foreign') || (entry.origin === 'owned' ? typeof entry.owner !== 'string' || entry.owner.length === 0 : entry.owner !== null)) throw new Error('The trusted commit ledger is invalid.'); ledger.set(entry.sha, { owner: entry.owner, origin: entry.origin }); @@ -411,6 +411,9 @@ export class GitRebaser { else scope.signal?.addEventListener('abort', abort, { once: true }); const deadlineError = () => Object.assign(new Error('The rebase deadline expired during conflict resolution.'), { code: 'ETIMEDOUT' }); + const abortExpired = () => { + if (!controller.signal.aborted && performance.now() >= scope.workDeadline) controller.abort(deadlineError()); + }; const remaining = scope.workDeadline - performance.now(); let timer: NodeJS.Timeout | undefined; if (remaining <= 0) controller.abort(deadlineError()); @@ -420,9 +423,11 @@ export class GitRebaser { // Await settlement after abort: the resolver owns its container/storage until its promise ends. try { await resolveConflict(Object.freeze({ ...input, signal: controller.signal })); } catch (error) { + abortExpired(); controller.signal.throwIfAborted(); throw error; } + abortExpired(); controller.signal.throwIfAborted(); } finally { if (timer) clearTimeout(timer); diff --git a/test/runner-rebase.test.ts b/test/runner-rebase.test.ts index 6ab10e54..e29627ac 100644 --- a/test/runner-rebase.test.ts +++ b/test/runner-rebase.test.ts @@ -1,7 +1,7 @@ -import { existsSync, mkdirSync, mkdtempSync, readdirSync, rmSync, writeFileSync } from 'node:fs'; +import { existsSync, mkdirSync, mkdtempSync, readdirSync, rmSync, symlinkSync, writeFileSync } from 'node:fs'; import { execFileSync } from 'node:child_process'; import { tmpdir } from 'node:os'; -import { join } from 'node:path'; +import { dirname, join } from 'node:path'; import { randomUUID } from 'node:crypto'; import { afterEach, describe, expect, it, vi } from 'vitest'; import { fixtureGit as git } from './fixtures/git.ts'; @@ -25,11 +25,16 @@ function commit(repository: string, path: string, text: string, message: string) return git(repository, 'rev-parse', 'HEAD'); } -async function setup(conflict: boolean | 'foreign' = false, conflictPath = 'a.txt') { +async function setup(conflict: boolean | 'foreign' = false, conflictPath = 'a.txt', baseSiblingPath?: string) { const root = mkdtempSync(join(tmpdir(), 'codeboost-rebase-')); roots.push(root); const source = join(root, 'source'); git(root, 'init', '-q', '-b', 'main', source); git(source, 'config', 'user.name', 'Source'); git(source, 'config', 'user.email', 'source@example.invalid'); + if (baseSiblingPath) { + mkdirSync(dirname(join(source, baseSiblingPath)), { recursive: true }); + writeFileSync(join(source, baseSiblingPath), 'base sibling\n'); + git(source, 'add', '--', baseSiblingPath); + } const base = commit(source, conflictPath, 'base\n', 'base'); git(source, 'switch', '-qc', 'feature'); const owned = commit(source, conflict === 'foreign' ? 'owned.txt' : conflictPath, 'feature\n', 'owned'); @@ -386,6 +391,24 @@ describe('trusted pre-merge rebase', () => { ledger: [{ sha: foreign, owner: null, origin: 'foreign' }] })).rejects.toThrow(/outside the conflicted set/); }); + it('refuses a symlink ancestor before auditing a clean sibling outside the workspace', async () => { + const s = await setup('foreign', 'a.txt', 'nested/clean.txt'), attemptId = randomUUID(), outside = join(s.root, 'outside'); + git(s.source, 'switch', '-q', 'feature'); + writeFileSync(join(s.source, 'nested', 'clean.txt'), 'clean foreign change\n'); + git(s.source, 'add', '--', 'nested/clean.txt'); + git(s.source, 'commit', '--amend', '--no-edit', '-q'); + const foreign = git(s.source, 'rev-parse', 'HEAD'); + await ensureCommit(s.repository, foreign); + mkdirSync(outside); writeFileSync(join(outside, 'clean.txt'), 'outside data must not be audited\n'); + const runner = createRebaser(s, { resolveForeignConflict: async input => { + writeFileSync(join(input.repository, 'a.txt'), 'resolved foreign change\n'); + rmSync(join(input.repository, 'nested'), { recursive: true }); + symlinkSync(outside, join(input.repository, 'nested'), 'dir'); + } }); + await expect(runner.run({ attemptId, oldBase: s.base, oldHead: foreign, oldHistory: [s.owned, foreign], onto: s.onto, + ledger: [{ sha: foreign, owner: null, origin: 'foreign' }] })).rejects.toThrow(/symlink ancestor/); + }); + it('preserves an unchanged gitlink from the conflicted commit', async () => { const s = await setup('foreign'), attemptId = randomUUID(); git(s.source, 'switch', '-q', 'feature'); @@ -546,6 +569,44 @@ describe('trusted pre-merge rebase', () => { expect(resolverReads).toBe(2); }); + it('rejects a resolver that synchronously exhausts its deadline before its timer can run', async () => { + const s = await setup('foreign'), attemptId = randomUUID(); + const realNow = performance.now.bind(performance); + let expireNextRead = false; + vi.spyOn(performance, 'now').mockImplementation(() => { + if (!expireNextRead) return realNow(); + expireNextRead = false; + return Number.MAX_SAFE_INTEGER; + }); + const runner = createRebaser(s, { resolveForeignConflict: async input => { + writeFileSync(join(input.repository, 'a.txt'), 'resolved foreign change\n'); + expireNextRead = true; + } }); + await expect(runner.run({ attemptId, oldBase: s.base, oldHead: s.foreign, oldHistory: s.history, onto: s.onto, + ledger: [{ sha: s.foreign, owner: null, origin: 'foreign' }] })) + .rejects.toThrow(/deadline expired during conflict resolution/); + expect(expireNextRead).toBe(false); + }); + + it('preserves an exhausted deadline when the synchronously blocking resolver also rejects', async () => { + const s = await setup('foreign'), attemptId = randomUUID(); + const realNow = performance.now.bind(performance); + let expireNextRead = false; + vi.spyOn(performance, 'now').mockImplementation(() => { + if (!expireNextRead) return realNow(); + expireNextRead = false; + return Number.MAX_SAFE_INTEGER; + }); + const runner = createRebaser(s, { resolveForeignConflict: async () => { + expireNextRead = true; + throw new Error('resolver failed after blocking'); + } }); + await expect(runner.run({ attemptId, oldBase: s.base, oldHead: s.foreign, oldHistory: s.history, onto: s.onto, + ledger: [{ sha: s.foreign, owner: null, origin: 'foreign' }] })) + .rejects.toThrow(/deadline expired during conflict resolution/); + expect(expireNextRead).toBe(false); + }); + it('never sends an owned conflict through the foreign resolver', async () => { const s = await setup(true), attemptId = randomUUID(), resolveForeignConflict = vi.fn(async () => {}); const runner = createRebaser(s, { resolveForeignConflict }); @@ -562,6 +623,14 @@ describe('trusted pre-merge rebase', () => { expect(existsSync(join(s.runnerRoot, OWNER, 'rebases', attemptId))).toBe(false); }); + it('refuses an unknown trusted-ledger origin before starting Git', async () => { + const s = await setup('foreign'), attemptId = randomUUID(); + const runner = createRebaser(s, { onProcessStarting: () => { throw new Error('Git must not start for a malformed ledger.'); } }); + await expect(runner.run({ attemptId, oldBase: s.base, oldHead: s.foreign, oldHistory: s.history, onto: s.onto, + ledger: [{ sha: s.foreign, owner: null, origin: 'typo' as 'foreign' }] })).rejects.toThrow(/ledger is invalid/); + expect(existsSync(join(s.runnerRoot, OWNER, 'rebases', attemptId))).toBe(false); + }); + it('refuses a case-colliding reviewed tree when the host cannot check it out faithfully', async () => { const s = await setup(), attemptId = randomUUID(); const probe = join(s.root, 'case-probe'); mkdirSync(probe); From e542a1ae250f13fde29e80c5a567faae4e5e3acb Mon Sep 17 00:00:00 2001 From: mchwang Date: Tue, 6 Oct 2026 17:40:12 -0700 Subject: [PATCH 10/11] Complete conflict scope auditing --- runner/hash-outside-conflict.ts | 3 ++- runner/list-git-paths.ts | 2 +- test/runner-rebase.test.ts | 41 ++++++++++++++++++++++++++++++++- 3 files changed, 43 insertions(+), 3 deletions(-) diff --git a/runner/hash-outside-conflict.ts b/runner/hash-outside-conflict.ts index b085406b..4956ce3c 100644 --- a/runner/hash-outside-conflict.ts +++ b/runner/hash-outside-conflict.ts @@ -47,7 +47,8 @@ export function outsideConflictDigest(root: string, executable: string, head: st const allowed = new Set(split0(allowedInput).map(path => path.toString('hex'))); const changed = new Map(); let changedBytes = 0; - for (const path of [...split0(git(root, executable, ['diff', '--name-only', '--ignore-submodules=all', '-z', head, '--'])), + for (const path of [...split0(git(root, executable, ['diff', '--name-only', '--no-renames', '--ignore-submodules=all', '-z', head, '--'])), + ...split0(git(root, executable, ['diff', '--cached', '--name-only', '--no-renames', '--ignore-submodules=all', '-z', head, '--'])), ...split0(git(root, executable, ['ls-files', '--others', '-z', '--']))]) { const key = path.toString('hex'); if (!allowed.has(key) && !changed.has(key)) { diff --git a/runner/list-git-paths.ts b/runner/list-git-paths.ts index b4259113..ebec0508 100644 --- a/runner/list-git-paths.ts +++ b/runner/list-git-paths.ts @@ -9,7 +9,7 @@ type PathListMode = 'conflicts' | 'unmerged'; /** Return one bounded raw NUL stream as base64 so the parent can perform fatal UTF-8 decoding without replacement. */ export function listGitPaths(root: string, executable: string, mode: PathListMode): string { - const command = mode === 'conflicts' ? ['diff', '--name-only', '--diff-filter=U', '-z', '--'] + const command = mode === 'conflicts' ? ['diff', '--name-only', '--ignore-submodules=all', '--diff-filter=U', '-z', '--'] : ['ls-files', '--unmerged', '-z', '--']; const result = spawnSync(executable, [...GIT_OPTIONS, '-c', 'gc.auto=0', '-c', 'maintenance.auto=false', ...command], { cwd: root, env: gitEnvironment(), maxBuffer: MAX_GIT_OUTPUT_BYTES, stdio: ['ignore', 'pipe', 'pipe'] }); diff --git a/test/runner-rebase.test.ts b/test/runner-rebase.test.ts index e29627ac..45214699 100644 --- a/test/runner-rebase.test.ts +++ b/test/runner-rebase.test.ts @@ -1,4 +1,4 @@ -import { existsSync, mkdirSync, mkdtempSync, readdirSync, rmSync, symlinkSync, writeFileSync } from 'node:fs'; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, readdirSync, rmSync, symlinkSync, writeFileSync } from 'node:fs'; import { execFileSync } from 'node:child_process'; import { tmpdir } from 'node:os'; import { dirname, join } from 'node:path'; @@ -9,6 +9,8 @@ import { GitRebaser, RebaseConflict, rebaseRef, type GitRebaserOptions } from '. import { ensureCommit, openRunnerRepository } from '../runner/runner-repository.ts'; import { verifyCheckout } from '../runner/verify-checkout.ts'; import { readBoundedRebaseStateNames } from '../runner/hash-rebase-state.ts'; +import { listGitPaths } from '../runner/list-git-paths.ts'; +import { outsideConflictDigest } from '../runner/hash-outside-conflict.ts'; const OWNER = '0123456789abcdef0123456789abcdef'; const roots: string[] = []; @@ -75,6 +77,21 @@ describe('trusted pre-merge rebase', () => { expect(index).toBe(2); }); + it('distinguishes identical-content outside deletions instead of collapsing them as renames', () => { + const root = mkdtempSync(join(tmpdir(), 'codeboost-outside-digest-')); roots.push(root); + git(root, 'init', '-q', '-b', 'main'); + git(root, 'config', 'user.name', 'Source'); git(root, 'config', 'user.email', 'source@example.invalid'); + writeFileSync(join(root, 'a'), 'same\n'); writeFileSync(join(root, 'b'), 'same\n'); + git(root, 'add', '--', 'a', 'b'); git(root, 'commit', '-qm', 'base'); + const head = git(root, 'rev-parse', 'HEAD'); + const realGit = execFileSync('/bin/sh', ['-c', 'command -v git'], { encoding: 'utf8' }).trim(); + rmSync(join(root, 'a')); writeFileSync(join(root, 'c'), 'same\n'); git(root, 'add', '--all'); + const first = outsideConflictDigest(root, realGit, head, Buffer.alloc(0)); + git(root, 'reset', '--hard', '-q', head); + rmSync(join(root, 'b')); writeFileSync(join(root, 'c'), 'same\n'); git(root, 'add', '--all'); + expect(outsideConflictDigest(root, realGit, head, Buffer.alloc(0))).not.toBe(first); + }); + it('uses the configured Git PATH inside the checkout verifier subprocess', async () => { const s = await setup(), attemptId = randomUUID(), bin = join(s.root, 'git-bin'), marker = join(s.root, 'verified'); mkdirSync(bin); @@ -99,6 +116,14 @@ describe('trusted pre-merge rebase', () => { } }); + it('disables submodule inspection in the conflict-path diff', () => { + const root = mkdtempSync(join(tmpdir(), 'codeboost-path-list-')); roots.push(root); + const executable = join(root, 'git'), args = join(root, 'args'); + writeFileSync(executable, `#!/bin/sh\nprintf '%s\\n' "$@" > '${args}'\n`, { mode: 0o755 }); + expect(listGitPaths(root, executable, 'conflicts')).toBe(''); + expect(readFileSync(args, 'utf8').split('\n')).toContain('--ignore-submodules=all'); + }); + it('never resolves Git from a relative PATH component inside the untrusted checkout', async () => { const s = await setup(), attemptId = randomUUID(), marker = join(s.root, 'untrusted-git-ran'); const realGit = execFileSync('/bin/sh', ['-c', 'command -v git'], { encoding: 'utf8' }).trim(); @@ -499,6 +524,20 @@ describe('trusted pre-merge rebase', () => { ledger: [{ sha: s.foreign, owner: null, origin: 'foreign' }] })).rejects.toThrow(/gitlink/); }); + it('refuses a resolver index-only regular file outside the conflict', async () => { + const s = await setup('foreign'), attemptId = randomUUID(); + const runner = createRebaser(s, { resolveForeignConflict: async input => { + writeFileSync(join(input.repository, 'a.txt'), 'resolved foreign change\n'); + const payload = join(input.repository, 'payload.tmp'); + writeFileSync(payload, 'index-only payload\n'); + const blob = git(input.repository, 'hash-object', '-w', 'payload.tmp'); + rmSync(payload); + git(input.repository, 'update-index', '--add', '--cacheinfo', `100644,${blob},smuggled.txt`); + } }); + await expect(runner.run({ attemptId, oldBase: s.base, oldHead: s.foreign, oldHistory: s.history, onto: s.onto, + ledger: [{ sha: s.foreign, owner: null, origin: 'foreign' }] })).rejects.toThrow(/outside the conflicted set/); + }); + it('passes a literal replacement character in a valid UTF-8 conflict path to the resolver', async () => { const name = '\ufffd.txt', s = await setup('foreign', name), attemptId = randomUUID(); const resolveForeignConflict = vi.fn(async input => { writeFileSync(join(input.repository, name), 'resolved\n'); }); From 0b41b4ed4eeb45fc8b28a8858e3e4a7dc1958c44 Mon Sep 17 00:00:00 2001 From: mchwang Date: Tue, 6 Oct 2026 18:07:51 -0700 Subject: [PATCH 11/11] Bound index record retention --- runner/verify-checkout.ts | 13 +++++++------ test/runner-rebase.test.ts | 6 +++++- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/runner/verify-checkout.ts b/runner/verify-checkout.ts index 93d701f8..77203873 100644 --- a/runner/verify-checkout.ts +++ b/runner/verify-checkout.ts @@ -15,10 +15,13 @@ const git = (executable: string, args: readonly string[], input?: Buffer): Buffe if (result.status !== 0 || result.error) throw result.error ?? new Error(`git ${args[0]} failed while verifying the checkout.`); return result.stdout; }; -const split0 = (input: Buffer): Buffer[] => { +export const splitBoundedIndexRecords = (input: Buffer, maxEntries = MAX_INDEX_ENTRIES): Buffer[] => { const values: Buffer[] = []; let start = 0; - for (let i = 0; i < input.length; i++) if (input[i] === 0) { values.push(input.subarray(start, i)); start = i + 1; } + for (let i = 0; i < input.length; i++) if (input[i] === 0) { + if (values.length >= maxEntries) throw new Error('Checkout index exceeds its entry bound.'); + values.push(input.subarray(start, i)); start = i + 1; + } if (start !== input.length) throw new Error('Git returned an unterminated index record.'); return values; }; @@ -39,8 +42,7 @@ const objectId = (algorithm: 'sha1' | 'sha256', bytes: Buffer): string => export function gitlinkIndexDigest(root = process.cwd(), executable = 'git'): string { const rawOutput = git(executable, ['-C', root, 'ls-files', '--stage', '-z']); if (rawOutput.length > MAX_INDEX_BYTES) throw new Error('Checkout index exceeds its byte bound.'); - const raw = split0(rawOutput); - if (raw.length > MAX_INDEX_ENTRIES) throw new Error('Checkout index exceeds its entry bound.'); + const raw = splitBoundedIndexRecords(rawOutput); const hash = createHash('sha256'); for (const record of raw) { const tab = record.indexOf(0x09), header = record.subarray(0, tab).toString('ascii').split(' '); @@ -55,8 +57,7 @@ export function verifyCheckout(root = process.cwd(), executable = 'git'): void { const repositoryGit = (args: readonly string[], input?: Buffer) => git(executable, ['-C', root, ...args], input); const rawOutput = repositoryGit(['ls-files', '--stage', '-z']); if (rawOutput.length > MAX_INDEX_BYTES) throw new Error('Checkout index exceeds its byte bound.'); - const raw = split0(rawOutput); - if (raw.length > MAX_INDEX_ENTRIES) throw new Error('Checkout index exceeds its entry bound.'); + const raw = splitBoundedIndexRecords(rawOutput); const quotedOutput = repositoryGit(['ls-files', '--stage']).toString('utf8'); if (quotedOutput && !quotedOutput.endsWith('\n')) throw new Error('Git returned an unterminated quoted index listing.'); const quoted = quotedOutput ? quotedOutput.slice(0, -1).split('\n') : []; diff --git a/test/runner-rebase.test.ts b/test/runner-rebase.test.ts index 45214699..551f9cb3 100644 --- a/test/runner-rebase.test.ts +++ b/test/runner-rebase.test.ts @@ -7,7 +7,7 @@ import { afterEach, describe, expect, it, vi } from 'vitest'; import { fixtureGit as git } from './fixtures/git.ts'; import { GitRebaser, RebaseConflict, rebaseRef, type GitRebaserOptions } from '../runner/rebase.ts'; import { ensureCommit, openRunnerRepository } from '../runner/runner-repository.ts'; -import { verifyCheckout } from '../runner/verify-checkout.ts'; +import { splitBoundedIndexRecords, verifyCheckout } from '../runner/verify-checkout.ts'; import { readBoundedRebaseStateNames } from '../runner/hash-rebase-state.ts'; import { listGitPaths } from '../runner/list-git-paths.ts'; import { outsideConflictDigest } from '../runner/hash-outside-conflict.ts'; @@ -77,6 +77,10 @@ describe('trusted pre-merge rebase', () => { expect(index).toBe(2); }); + it('rejects an over-limit index record before retaining it', () => { + expect(() => splitBoundedIndexRecords(Buffer.from([0, 0]), 1)).toThrow(/entry bound/); + }); + it('distinguishes identical-content outside deletions instead of collapsing them as renames', () => { const root = mkdtempSync(join(tmpdir(), 'codeboost-outside-digest-')); roots.push(root); git(root, 'init', '-q', '-b', 'main');