Repository navigation
fix(ce-code-review): bind finish workers to the reviewed checkout - #1845
arimetabot wants to merge 1 commit into
Conversation
|
Thanks for this, and for filing #1844 first with a clear repro. I'm going to close it though. I'm not convinced the failure happens in normal use yet. I've asked for the real-world case on #1844. If it turns out to be real, the fix we'd merge is just The index and worktree SHA-256 fingerprints and the recheck steps are more than we'd take even then. Once workers run in the right root, what the fingerprints add is detection of concurrent drift, which the PR itself says it doesn't protect against. They'd also fail the whole finish after all the reviewers have already run, which is a rough trade when someone keeps editing during a long review. And hashing raw stdout is left for the model to do in prose, so the dispatcher and a leaf can easily compute it differently and fail on a false mismatch. If #1844 shows a real harness doing this, happy to take a PR for the |
Summary
Fresh review workers now receive the checkout that produced the diff instead of relying on their inherited working directory. This prevents a same-branch, same-HEAD checkout with different staged changes from being treated as the reviewed tree.
The binding is captured during scope collection and checked by finish leaves and validators. The index is fingerprinted separately because an unstaged edit can cancel a staged change in the working-tree diff. Remote reviews retain their existing reviewed-ref rules; local apply authority is unchanged.
Fixes #1844.
Validation
No installed plugin or host configuration was changed. No claim is made about time or token savings.
Security Disclosure
This changes checkout-path and state-validation instructions. Missing or mismatched local bindings stop finish or leave validation unresolved. The checks assume a trusted, cooperative checkout; they are not a sandbox and do not prevent concurrent changes between verification and use. No dependencies, credentials, permissions, or external-provider routes changed.
Agent Disclosure