Add foreign rebase conflict engine boundary

[mchwang]

Summary

• route missing-ledger and explicit unowned rebase conflicts through an injected, deadline-bound resolver and enforce exact ordinary conflict-file writes
• persist agent-resolved foreign provenance across rewrite/recovery and keep those lines Unplanned with a visible conflict resolved by agent label
• make that provenance review-significant while preserving legacy choice keys and approval fingerprints for ordinary segments
• harden the resolver boundary with pinned replay state, batched raw-path auditing, gitlink refusal/auditing, and original cancellation reasons
• bound rebase-state directory enumeration before retaining names and reject already-expired conflict-resolution work before entering the resolver
• document the safety boundary: the production rebase-fix container adapter remains intentionally unwired until it can own storage durably under the active rebase marker

Validation

Validated head: 0b41b4ed4eeb45fc8b28a8858e3e4a7dc1958c44

• git diff --check
• npm run typecheck
• npx vitest run --project parallel — 69 files, 1,774 passed, 1 skipped
• focused F4/recovery/store suite — 265 passed, 1 skipped
• targeted latest regressions — 3 passed
• npm run test:browser — 66 passed
• exact-head CI — all three jobs passed (test 4m28s, test 3m57s, real-docker 19m01s)
• local Docker project: not verified; the local Docker daemon stopped responding during image build, so Docker evidence comes from CI

Review rounds

1. Independent review found provenance missing from choices/approvals, adjacent lines merging mismatched provenance, and explicit-null recovery normalization. Fixed with regressions.
2. Independent review found ordinary review identities had lost backward compatibility. Fixed by serializing the new field only when true, with legacy-shape regressions.
3. Independent full pass found no new issues.
4. Final ordering check after moving ledger validation before Git/process work found no new issues.
5. Copilot found four issues: missing submodule guard, rejection of clean siblings in multi-file commits, masked cancellation reasons, and inaccurate callback-path documentation. Fixed, replied, and resolved with failing-before regressions.
6. Independent review found an index-only gitlink pointer escape. Fixed with raw gitlink-state hashing and unchanged/mutated regressions.
7. Independent review found mutable HEAD, allowed gitlink-conflict ambiguity, undersized path transport, and literal U+FFFD rejection. Fixed by pinning HEAD, routing gitlink conflicts to manual review, sharing explicit bounds, and using fatal raw-byte decoding.
8. Full-function reread and independent review found mutable rebase-control state and an allowed-file-to-embedded-repository conversion. Fixed with a byte-exact rebase-state digest, post-stage gitlink audit, and failing-before regressions.
9. Independent review found an unbounded per-path outside-file audit. Fixed with one batched, bounded, killable helper. Its shared-Git-metadata scenario was declined because the callback is trusted host orchestration, not agent code; the production adapter remains unwired and is required to expose only bounded lane-D storage, never the host worktree or shared Git metadata.
10. Independent full pass on d4ec5de found no actionable issues under that documented trust boundary.
11. Fresh Copilot review on d4ec5de found unbounded rebase-state name enumeration and an already-expired deadline that could enter the resolver. Fixed with fail-closed enumeration budgets and synchronous deadline admission, with regressions.
12. Independent full pass over all 19 changed files, including the round-11 fixes, found no actionable issues. Focused tests, typecheck, and diff check passed.
13. Fresh Copilot review on 2005f96 found that the deadline regression expired at the resolver eligibility check rather than resolver admission. Fixed by delaying expiry until the second resolver lookup and asserting that lookup occurred.
14. Independent full pass and mutation analysis confirmed the corrected regression reaches admission, passes the fix, and fails the pre-fix 0 ms timer path. No new actionable issues.
15. Fresh Copilot review on aee9851 found a symlink-ancestor escape, missing runtime validation of the ledger origin enum, and synchronous resolver fulfillment/rejection that could outrun its timer. Fixed with failing-before regressions.
16. Independent full pass over all 19 changed files found no new issues. It confirmed raw-byte ancestor rejection occurs before outside-file reads, malformed origins fail before Git, and both resolver settlement paths preserve deadline failure.
17. Fresh Copilot review on 8123477 found regular index-only changes missing from the outside-state audit and a conflict-path diff without the submodule guard. Fixed with a cached diff union, --ignore-submodules=all, and failing-before regressions.
18. Independent review found rename detection could collapse two different identical-content source deletions to one destination-only digest. Fixed both scans with --no-renames; the final full pass found no further issues.
19. Fresh Copilot review on e542a1a had no inline findings but found summary-only unbounded retention in the gitlink/index record splitter. Fixed by checking the entry budget before each retained view.
20. Independent full pass over all 19 changed files found no new issues, including the splitter boundary and all recent raw-path/digest/lifecycle fixes.
21. Fresh Copilot review on 0b41b4e returned no findings and no inline comments. Its general final-human-review note introduced no concrete defect.

Review lesson audit

• Covered by existing AGENTS.md rules: missing submodule guards, gitlink pointer/embedded-repository escapes, mutable replay state, symlink ancestors, raw-byte path handling, and trust-root checks are covered by Agent-controlled content and Owned host and Docker resources.
• Covered by existing AGENTS.md rules: masked cancellation, already-expired and synchronously outrun deadlines, and bounded/awaited subprocess settlement are covered by Async jobs and polling, Required race regressions, and Guarded external actions.
• Covered by existing AGENTS.md rules: unbounded outside-path, rebase-state-name, and index-record retention are covered by the fail-closed bounded safety scan and aligned payload-limit rules under Guarded external actions.
• Covered by existing AGENTS.md rules: malformed ledger origins, missing/explicit-null identity distinctions, approval/provenance staleness, and index-only state omissions are covered by the authorization-metadata, coupled-lifecycle, replacement-generation, and separate-state-holder rules.
• Covered by existing AGENTS.md rules: the deadline fixture that originally missed resolver admission is covered by Review readiness, which requires disputed intermediate state and production-faithful setup.
• One-off implementation findings, now regression-covered: adjacent provenance merging, legacy review-identity serialization, clean siblings in multi-file commits, callback-path wording, gitlink-conflict ambiguity, rename detection collapsing identical-content paths, and the regular index-only digest omission. These are local representation/contract defects rather than reusable missing policy.
• Declined as one-off boundary hypothesis: mutation of shared Git metadata by the injected callback. The callback is trusted host orchestration, the production adapter remains unwired, and its eventual implementation is constrained to bounded lane-D storage rather than the host worktree or shared Git metadata.
• No new AGENTS.md rule is required: every reusable lesson is already stated by the cited rules; the remaining findings are implementation-specific and have targeted regressions.

Remaining #22 work

• wire the production sandboxed rebase-fix child-attempt/storage lifecycle
• owned-conflict resolver behavior
• F5 post-rebase refresh and head-bound commands
• F6 push/check/guarded-merge integration

Does not close #22.

[Copilot]

Copilot review overview

🟡 Changes recommended

The post-resolution audit incorrectly rejects valid multi-file commits with cleanly applied non-conflicting changes.

Review effort: Balanced
Findings: 3 · 1

Open (4)

• Add submodule guard to audit diff invocation · New
• Allow cleanly staged files from multi-file commits during conflict resolution · New
• Preserve original abort reason when resolver rejection follows cancellation · New
• Correct resolver contract description for conflict file paths · New

What changed in this PR

Adds a foreign rebase-conflict resolver boundary with durable provenance and review visibility.

Changes:

• Resolves foreign conflicts through a bounded callback with write auditing.
• Persists agent-resolution provenance across rebases and recovery.
• Marks affected review segments and invalidates prior choices/approvals.

File	Description
core/​approvals.ts	Includes conflict provenance in review identity.
core/​linking.ts	Propagates provenance into linked segments.
docs/​implementation/​pre-merge-rebase.md	Documents the new boundary.
runner/​production.ts	Persists resolver result metadata.
runner/​rebase.ts	Implements foreign-conflict resolution.
runner/​recovery.ts	Validates recovered provenance.
runner/​review.ts	Supplies provenance to linking.
runner/​store.ts	Stores and remaps provenance.
test/​browser/​review.spec.ts	Tests the visible label.
test/​history.test.ts	Tests provenance attribution.
test/​runner-lifecycle-store.test.ts	Tests durable lifecycle behavior.
test/​runner-rebase.test.ts	Tests resolver behavior and cleanup.
test/​runner-recovery.test.ts	Tests recovery validation.
test/​store.test.ts	Tests persistence and review identity.
web/​public/​app.js	Displays agent-resolution provenance.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Copilot review overview

🟡 Changes recommended

Deadline admission and rebase-state enumeration still contain two bounded-lifecycle defects.

Review effort: Balanced
Findings: 1

Open (1)

• Enforce directory entry and name limits during enumeration · New

Resolved since last review (4)

• Preserve original abort reason when resolver rejection follows cancellation
• Allow cleanly staged files from multi-file commits during conflict resolution
• Add submodule guard to audit diff invocation
• Correct resolver contract description for conflict file paths

Previously missed (1)

In code that hasn't changed since last review

Abort synchronously when the work deadline has elapsed

runner/​rebase.ts:427

If the work deadline has already elapsed here, a 0 ms timer is deferred to a later timers turn, so throwIfAborted() still passes and the resolver is invoked after its deadline. Abort synchronously when no budget remains, and add a controllable-clock regression proving the callback is not entered in this case.

[mchwang]

Copilot round 2 is addressed in 2005f96. The inline enumeration finding is fixed and resolved. The summary-only deadline finding is also fixed: if the conflict-resolution work deadline is already exhausted, the resolver is synchronously aborted before its callback is entered. The regression controls the clock at resolver lookup and asserts zero callback entries. Fresh independent full-diff review found no new issues.

[Copilot]

Copilot review overview

🟡 Changes recommended

The deadline-admission regression passes before reaching the resolver check it claims to cover.

Review effort: Balanced
Findings: 1

Open (1)

• Clock expires on first read, bypassing resolver admission check · New

Resolved since last review (1)

• Enforce directory entry and name limits during enumeration

[Copilot]

Copilot review overview

🟡 Changes recommended

Resolver deadlines remain bypassable, and the filesystem audit can follow symlink ancestors outside the workspace.

Review effort: Balanced
Findings: 1

Open (1)

• Reject symlink ancestors during workspace path resolution · New

Resolved since last review (1)

• Clock expires on first read, bypassing resolver admission check

Previously missed (2)

In code that hasn't changed since last review

Validate ledger origin enum explicitly

runner/​rebase.ts:112

The runtime validator treats every non-owned value as the foreign branch, so a deserialized entry such as { origin: 'typo', owner: null } passes as a trusted ledger entry. Validate the origin enum explicitly so malformed ledger data fails before any Git or process work begins.

Recheck deadline after resolver completion

runner/​rebase.ts:426

A synchronous resolver can block past workDeadline and then resolve successfully: while it blocks, the timer cannot run, and the await continuation clears that timer before the timers phase. Recheck the monotonic deadline immediately after both resolver fulfillment and rejection, aborting with deadlineError() before throwIfAborted(), so expired work cannot be accepted.

[mchwang]

Copilot round 4 is addressed in 8123477. In addition to the resolved symlink-ancestor thread, runtime ledger validation now accepts only the explicit owned/foreign enum values before any Git work, and conflict resolution rechecks its monotonic deadline after both fulfillment and rejection so a synchronous blocker cannot outrun the timer. Four regressions failed before and pass after. Independent round 15 found no new issues across the full diff.

[Copilot]

Copilot review overview

🟡 Changes recommended

The audit misses regular index-only changes outside the conflict set, and one conflict probe lacks the required submodule guard.

Review effort: Balanced
Findings: 1

Open (1)

• Audit index-only changes outside the conflict set · New

Resolved since last review (1)

• Reject symlink ancestors during workspace path resolution

Previously missed (1)

In code that hasn't changed since last review

Ignore submodules in conflict-path git diff

runner/​list-git-paths.ts:12

This conflict-path git diff lacks the explicit submodule guard used by the surrounding conflict probes and outside-file audit. A populated submodule can make Git inspect nested repository state while processing resolver-controlled content; pass --ignore-submodules=all on this invocation too.

[mchwang]

Copilot round 5 is addressed in e542a1a. The inline cached-index gap is fixed and resolved. The summary-only conflict-path finding is also fixed: that exact Git diff now passes --ignore-submodules=all, with an argv-level regression. Independent review additionally found and verified the --no-renames digest-collision fix; its final full pass found no further issues.

[Copilot]

Copilot review overview

🔵 Needs a closer look

The gitlink audit retains over-limit index records before enforcing its entry bound.

Review effort: Balanced
Findings: None

Resolved since last review (1)

• Audit index-only changes outside the conflict set

Previously missed (1)

In code that hasn't changed since last review

Enforce entry limit before retaining records

runner/​verify-checkout.ts:8

MAX_INDEX_ENTRIES is enforced only after split0() has retained the entire listing. A valid 32 MiB staged listing can contain well over 262,144 short records, so the new repeated gitlink audit can allocate hundreds of thousands of extra Buffer views before failing the advertised entry bound. Enforce the limit before appending each record.

[mchwang]

Copilot round 6 summary-only finding is addressed in 0b41b4e. Index record splitting now enforces the entry limit before retaining each Buffer view; the post-allocation length checks were removed. A small-limit regression exercises the admission boundary. Independent round 19 reread the full diff and found no further issues.

[Copilot]

Copilot review overview

🔵 Needs a closer look

The security-sensitive Git, subprocess, recovery, and durable-state boundary warrants final human review despite extensive regression coverage.

Review effort: Balanced
Findings: None