fix(review): stop attributing main-only changes to PRs - #1265
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. |
clawsweeper
Bot
added
P2
|
Codex review: needs maintainer review before merge. Reviewed August 27, 2026, 10:48 AM ET / 14:48 UTC. ClawSweeper reviewWhat this changesAdds bounded Git evidence that distinguishes PR-introduced changes from main-branch drift, improves review-history hydration, and narrows false stored-data warnings for Markdown prose. Merge readiness✅ Ready for maintainer review Keep open: the central provenance and hydration behavior is not on current main, and the prior changelog finding is superseded by the target-specific policy correction in #1261. The current patch has no actionable correctness finding. Priority: P2 Review scores
Verification
Live VerificationCommand: Result: PASS (completed) Assertions:
How this fits togetherClawSweeper gathers GitHub context and local Git history before sending a bounded evidence package to Codex for PR review. This change improves the provenance inputs and review-tree hydration that determine which branch actually owns a finding. flowchart LR
A[GitHub PR context] --> B[Review tree hydration]
B --> C[Merge-base evidence]
C --> D[Review prompt]
D --> E[Codex review result]
F[Current main history] --> C
E --> G[Findings and labels]
Before mergeNone. Agent review detailsSecurityNone. Review metrics
Technical reviewBest possible solution: Land the bounded provenance, hydration, and classifier corrections after routine maintainer approval. Do we have a high-confidence way to reproduce the issue? Yes — source-reproducible: the focused real-Git fixture models the base/head/main/test-merge topology and asserts that main-only changes are excluded from introduced evidence. Is this the best way to solve the issue? Yes — separating pinned merge-base evidence from endpoint drift is the narrowest maintainable fix, while retaining explicit unavailable and ambiguous-history outcomes. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against cab813112d6f. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (1 earlier review cycle)
|
|
@clawsweeper re-review The main PR body now records the disposition of the sole P3 changelog finding and matching optional rank-up suggestion, with exact policy links. The release-owned restriction applies to Head |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
🦞👀 Re-review progress:
|
1 participant
What Problem This Solves
Fixes an issue where reviewers could blame an older PR for an upgrade made only on newer main, then turn that false attribution into findings, merge risks, ratings, and unnecessary restoration requests. Confirmed example: openclaw/openclaw#130734 (comment). Maintainer patch/test-merge proof: openclaw/openclaw#130734 (comment).
The same review classified ordinary bundled-hook Markdown prose as a stored-data change, although both edited HOOK.md frontmatter blocks were unchanged.
Why This Change Was Made
The host now supplies pinned checkout/base/head/merge-base identities and separates the introduced patch from endpoint drift and base-only changes. Test-merge evidence requires matching both parents and is compared against its own base parent. Missing ancestry, stale merges, multiple merge bases, and truncated evidence remain explicit limitations rather than guessed ownership.
Every finding and derived risk/label/fixup must have an introduced trigger. This is not an untouched-file suppression rule: a changed caller can still cause a valid finding in an unchanged callee. A clean merge does not establish semantic correctness. The Markdown detector separately distinguishes prose beside source from structured storage keys, SQL, frontmatter, and documented persisted-format contracts.
Validation exposed a nearby ref-hydration defect: with Git pruning enabled, shorthand
main:refs/remotes/origin/maincan removeorigin/maineven though fetch succeeds. Review and repair fetches now fully qualify the source asrefs/heads/<branch>. Pruning-enabled regressions cover both normal and shallow history. No pruning setting or validation gate is disabled.User Impact
Review input no longer presents main-owned upgrades as PR-introduced changes. Prose-only hook documentation does not acquire a bogus migration gate. Review and repair setup works with Git pruning enabled.
No decision, repair, publication, or observer schema changes; no new write authority or automatic closes. The referenced OpenClaw PR is untouched.
OpenClaw Bay Impact
Unaffected: no observer fields, routes, publication behavior, or mutation controls change.
Documentation Impact
Updated the active
docs/pr-review-comments.mdcontract with evidence ownership, bounds, merge-parent verification, Markdown classification, and Bay impact. Updated the review prompt and changelog. No new documentation lifecycle category.Evidence
test/review-provenance.test.tsexercises production prompt assembly, shallow/blobless hydration, unavailable/stale/final merge identities, criss-cross ancestry, truncation, a real introduced downgrade, and a cross-file trigger.fetch.prune.710819aa9edc15c54fde044a6639b05ac395d006: build, docs/whitespace checks, and 262 focused tests passed. Precommit and committed-branch isolated Codex autoreviews reported no actionable findings at their default P0 threshold.pnpm run checkpassed on the committed head with task-local Node 24.20.0 and Bash 5.3.15: 3,785 passed, 9 platform skips, zero failures; coverage 82.09% lines / 74.56% branches / 87.59% functions. This rerun replaced an invalid attempt during concurrent host maintenance that removed its Node executable; no tests or thresholds were weakened.Review Disposition
The initial ClawSweeper review accepts the implementation and real behavior proof. Its sole P3 finding, “Remove release-owned CHANGELOG entries,” is rejected as a repository-policy scope mismatch, not an implementation defect.
At this exact head, AGENTS.md:50-52 names OpenClaw's release-owned changelog, while CONTRIBUTING.md:16-19 explicitly limits that restriction to
openclaw/openclawand distinguishes ClawSweeper's own policy. This is a maintainer-authoredopenclaw/clawsweeperfix. Retain both concise changelog entries documenting its user-visible changes. The identical optional rank-up suggestion is declined for the same reason.The review-policy scoping defect has independently been fixed on main by PR #1261, commit
cab813112d6fbba18ce7876bbe016d7dbd618bc0. No source change or restoration is needed in this PR. Head710819aa9edc15c54fde044a6639b05ac395d006and its executed proof remain unchanged; the updated body records this disposition for a fresh review under the correctly scoped policy.Real Behavior Proof
Claim/surface: the actual local-range ClawSweeper CLI, with real Codex inference in its read-only sandbox, distinguishes an unrelated old docs branch from a genuinely introduced dependency mismatch. Neither GitHub transport nor model output was mocked.
Fixture/environment: a clean disposable Git repository under the OpenClaw policy profile, valid package JSON, executable TypeScript, and the B/H/M/T graph above. Positive head P starts at M and changes only the dependency back to 0.149.1 while the unchanged version guard requires 0.150.1. macOS; Node 24.19.0 at CLI launch (24.20.0 for the later host check); pnpm 11.10.0; Codex CLI 0.150.0-alpha.13 verified from both exact sessions; high reasoning; fast service tier; read-only sandbox.
Source binding: both reviews exercised committed head
710819aa9edc15c54fde044a6639b05ac395d006, based on0bd84d42bc0487c32af2285006884d4f9b2f7763. All 1,273 tracked source files and 497 compiled files retained their hashes throughout. Source snapshot SHA-256:9848f7307bc105a1970468e140cd5d2147b52467c27cf398a7bf97134c4a84d5.Command: each control used the real product command, with the operator-configured model supplied privately:
Observed results:
The negative reviewer independently inspected newer
origin/mainand excluded its separate dependency upgrade. The positive finding identified the changed manifest entry, unchanged checking caller, and version guard; its risk and proposed repair followed that introduced defect. Full structured decisions and rendered report sections were inspected, not only finding arrays. Severity varied between repeated live controls; the assertion is correct attribution, not identical stochastic severity.Host execution of
node extensions/codex/src/check.tssucceeded on H and failed on P withexpected 0.150.1, received 0.149.1. Both review targets retained their heads, refs, index entries, tracked bytes, and clean status. No model-side secret-file reads, target mutations, dependency installs, or GitHub operations were observed; existing Codex authentication was used normally.Artifact fingerprints: negative structured decision SHA-256
c2caed4edd009b09f9df86ee947a44e17a424d9d1db832c6b0f1b73a332f995a; positive structured decision SHA-2564b42395f5bd899fbcd84c24487a8cdc1c74a1b110c1677a67885773a0f8471e2. Raw prompts, model logs, and transcripts are intentionally omitted. Synthetic fixture commits are not presented as real GitHub objects.Limits: these are controlled stochastic reviews, not a statistical guarantee or a before/after live comparison. Local-range pins its base to the merge base and supplies no test-merge identity; the model independently inspected newer main. Exact parent/blob validation is separate real-Git proof, not a claim about hosted review or the original foreign PR. Qualified hosted-fetch behavior is covered by the pruning regressions. The third-party dependency was not installed or executed. No production queue, publication, or merge was exercised by this proof.