Repository navigation
Resolve owned rebase conflicts in the sandbox - #144
Merged
Merged
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Plan-item capture is not guarded against concurrent plan revisions, allowing stale instructions to be applied to the rebase workspace.
1 open finding
What changed in this PR
Extends sandboxed rebase conflict resolution to owned commits while preserving attribution and provenance.
Changes:
- Passes trusted ownership and exact plan-item data to the sandbox.
- Preserves owned commit authors, trailers, and ledger attribution.
- Adds owned-conflict and deadline regression coverage.
| File | Description |
|---|---|
runner/rebase.ts |
Routes owned and foreign conflicts through the sandbox resolver. |
runner/rebase-conflict.ts |
Supplies owned plan-item context to conflict-resolution agents. |
test/runner-rebase.test.ts |
Tests owned conflict routing and provenance. |
test/runner-rebase-conflict.test.ts |
Tests plan-item input and deadline behavior. |
docs/implementation/pre-merge-rebase.md |
Documents the expanded conflict boundary. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Summary
This is part of issue #22. It supplies the owned-conflict resolution slice; the separate F5 orchestration work remains responsible for guarded admission and recovery integration.
Done when
An owned conflicting commit is resolved only in the sandbox with its exact plan item and conflict paths, while its owner, author identity, and trailers survive the rewrite; stale plan context cannot launch or apply a child result; the affected suites and typecheck pass on the pushed head.
Failing-before evidence
The new regressions failed against the prior behavior:
An owned commit conflict needs its plan-item resolverValidation
Validated head:
bb33705503a7688110e2a47c059f7b4a480eb2c3cb69e50d6f7d92f6e1d2e897c45db3f8152bb36c...bb33705503a7688110e2a47c059f7b4a480eb2c3Current-head CI passed on both push and pull-request runs. Real-Docker validation passed 2/2 on the exact head, covering the writable fix profile and sealed read-only schema input. Copilot round 2 returned zero open findings.
Review record
Self-review
The initial full-diff pass found and fixed:
schema.jsoninput contract by using an envelope instead of a second mounted fileAfter Copilot's first round, another full-diff pass added durable-state assertions to both controlled races and found no further issues.
Independent review
Round 1, base/head
cb69e50d6f7d92f6e1d2e897c45db3f8152bb36c...1530e5f55f2569dec6455bb073f30f884a524f3c: no findings in this diff.Round 2, base/head
cb69e50d6f7d92f6e1d2e897c45db3f8152bb36c...bb33705503a7688110e2a47c059f7b4a480eb2c3: clean after the context-drift fix. The reviewer reran 92 changed rebase tests, 9 focused Store tests, typecheck, and diff check.The reviewer found an integration requirement in the separate active F5 worktree: its orchestration must not call
abort()whenRebaseResourcesUnsettledretains durable ownership. That task needs a regression proving the workspace remains available for recovery.Copilot
Round 1 found one valid high-severity race: the owned plan item could come from revision N while launch/result context came from revision N+1. Fixed in
bb33705by capturing one exact context, reading its immutable plan revision, checking before launch, and checking after final export before copy-back. The thread was replied to and resolved after targeted failing-before/passing-after regressions.Round 2, head
bb33705503a7688110e2a47c059f7b4a480eb2c3: zero open findings; the prior high-severity finding is recorded as resolved.Review lessons