fix(review): verify checkout access before publication - #1170
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Co-authored-by: masatohoshino <g515hoshino@gmail.com>
7b9ffb2 to
add1e6e
Compare
clawsweeper
Bot
added
merge-risk: 🚨 automation
|
Codex review: needs maintainer review before merge. Reviewed August 17, 2026, 1:57 AM ET / 05:57 UTC. ClawSweeper reviewWhat this changesThis PR materializes the exact pull-request checkout and requires runner-owned read access verification before a review or cached result can be published. Merge readinessKeep open for maintainer review. The patch appears correct and has current-head before/after proof, but it deliberately trades a possible false-negative runner preflight for deferred review publication; the PR author is a repository member, so this policy choice should not be auto-closed. Priority: P1 Review scores
Verification
How this fits togetherClawSweeper's review lane gathers pull-request context, runs an agent against a restricted checkout, and publishes a durable review record. This change inserts exact-head checkout materialization and a read-only runner preflight before model execution or cache reuse. flowchart TD
A[Pull request head and context] --> B[Restricted review checkout]
B --> C[Read-only runner preflight]
C -->|verified| D[Model review or cache reuse]
C -->|failed| E[Failed retry record]
D --> F[Published review record]
Decision needed
Why: The patch's integrity benefit is clear, but the acceptable availability cost is an automation-policy choice rather than a mechanical correctness question. Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Make runner-owned exact-head checkout attestation the publication prerequisite while preserving the failed-record retry path for unavailable checkouts. Do we have a high-confidence way to reproduce the issue? Yes. The PR documents a controlled Node 24 current-head CLI regression in which a wrong tracked-file hash previously allowed a completed review, while the branch blocks model start and records a retryable failure. Is this the best way to solve the issue? Unclear. Exact-head materialization and runner-owned attestation are a narrow, tested solution, but maintainers must explicitly accept the deliberate safety-over-availability policy. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 682ef3ab4abc. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (42 earlier review cycles; latest 8 shown)
|
clawsweeper
Bot
added
rating: 🦪 silver shellfish
|
/clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
clawsweeper
Bot
added
proof: sufficient
clawsweeper
Bot
added
rating: 🦐 gold shrimp
|
/clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
clawsweeper
Bot
added
merge-risk: 🚨 availability
|
/clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
/clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
/clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
/clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
/clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
/clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
/clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
/clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
/clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
/clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
/clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
🦞👀 Pull request received. I will update this pull request when review starts. |
ClawSweeper could mark a review complete without proving the review process could read the code it was supposed to inspect. That meant a clean-looking result could be published even if it inspected the wrong checkout or no readable checkout at all. This PR makes the review process read a known file from the exact pull-request commit before it starts a new review or reuses an older one. If that check fails, ClawSweeper stops, records why, and retries instead of publishing.
Change breakdown
These are direct-base net changes. Every file appears once and the rows reconcile to the total.
Proof
The same clean committed range and the same test runner were used for both runs. The runner claimed success but returned the wrong hash for a tracked file.
Before — direct base
The old path trusted the runner, started the review, and recorded a complete result.
After — PR
The new path detects the mismatch before model review and leaves nothing publishable.
Follow-up — unavailable PR head
The first fresh review found that an unavailable PR head stopped the whole command before recording a retry. The current head now writes the failed record without starting model review.
Reproduce
The comparisons ran on Node 24 with
bash .artifacts/pr1170-ab-proof.shandbash .artifacts/pr1170-missing-head-proof.sh.Current-head receipt
Head
153a52f231c9b22421336662d3e8e98244fe49f0passed the real CLI regression and the full repository check. Crabbox receipt: provideraws, leasecbx_f962a3ac5005, machinec7a.8xlarge, exit 0, automatically stopped. The remote scenario used controlled GitHub and runner substitutes; it did not test another provider or a live OpenClaw review.OpenClaw Bay is unaffected: this changes review admission and retry behavior, not Bay routes, data, or UI.