fix(autoreview): preserve commit scope at history boundaries - #205
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. |
|
@clawsweeper review The current head is a8466c1. The body now records the Windows CI failure, deterministic fail-before/pass-after clock reproduction, and isolated fixture repair. Production behavior and existing assertions are unchanged. Both the original provenance integration and the follow-up test repair have clean independent reviews; fresh Linux/Windows validation is attached to this exact head. |
|
🦞👀 Re-review progress:
|
|
Land-ready technical verification for a8466c1. The raw-parent comparison now owns both review text and captured finding scope; blame and graph-truncated ancestry no longer stand in for introduction proof. The current Exact-head validation run 33278532267 passed on both Linux and Windows; CodeQL passed too. The earlier Windows failure was reproduced locally as a floating-point fixture error and repaired without changing the ticker, threshold, threading, or assertions. Its identical fail-before case now passes, along with the three focused progress tests. The initial full local run completed 225 tests with one platform skip. A redundant later local full-suite repeat was stopped after the exact-head cross-platform run passed; that repeat is not counted as a pass. The full provenance integration received a clean independent P0–P2 review (0.90), and the isolated fixture repair received a separate clean review (0.97). The PR body contains the exact full/focused commands and dependency-source evidence. No new configuration, dependency, storage format, or provider-isolation exception was introduced. The historical Browser attribution remains unassigned rather than guessing a replacement introducer. Hosted ClawSweeper review did not produce a verdict: run 33278765964 refused an unchanged reserved-domain synthetic proxy fixture before review inference. The PR body records the reproduced detector/source evidence and the separate follow-up. No scanner bypass or fixture change was made. The repository has no mandatory hosted-bot or independent GitHub-approval gate; the reviewed patch and exact-head CI are the landing evidence, not the failed hosted review. |
What Problem This Solves
Autoreview commit inputs currently rely on Git's traversal view of ancestry. At a shallow boundary,
rev-list --parentsandgit showcan make carried-forward code look newly added. Local ancestry overlays can also make the reported comparison differ from the underlying commit objects. A review can therefore receive the wrong patch and finding scope.The provenance instructions also need an explicit evidence gate: blame, commit subjects, PR metadata, and merger identity locate candidates, but do not establish who introduced a regression.
Why This Change Was Made
The repair belongs at the Git-input owner, before the reviewer receives a bundle:
CapturedBundle/build_bundlecontract.review_pathsstays deleted; consumers use the captured text and filtered paths rather than performing another membership lookup. Upstream evidence capture/revalidation, preparation progress, ref pinning, and full-tree integrity checks remain intact.This also repairs the smoke harness's stale prompt-builder call and isolates the prompt-limit fixture from the host's Kimi configuration. Provider isolation and credential-scanning protections are preserved.
Both companion repairs have landed: agent-scripts#36 — require patch-backed regression provenance and ClawSweeper#1298 — verify provenance against raw Git history.
User Impact
Commit reviews receive the actual commit delta rather than an inferred root snapshot or a misleading empty comparison. Genuine roots remain reviewable, and shallow checkouts remain usable when the recorded parent is available. When it is unavailable, the helper stops before sending the review and explains what history is needed.
There are no new CLI/configuration options or runtime dependencies. Historical attribution remains explicitly uncertain when the parent-relative behavioral change cannot be verified. This PR does not validate or reassign any original historical introduction claim: an author, merger, or automation trigger alone is not proof of causation.
Evidence
Current candidate:
a8466c1d860588a083610fe41fd277c1d88b14e0, based onae75f60e8d454f1cf44ec4613e10ec9ea7f2ade7. The provenance implementation was validated ateb026f9893a60ba8ff41250b7234f3fd0e9fa715; the current head adds only the two-net-line test-clock repair described below.OK (skipped=1). The single skip is platform-specific.90153836997184205b879723434fc0c3579f98e6363ba298b330b4034e82197c.git diff --checkpassed.scoped-clean, no actionable P0–P2 findings, confidence 0.90. The subsequent isolated clock-fixture repair separately returnedscoped-clean, no findings, confidence 0.97. These were static reviews; executed behavioral proof is recorded separately.+50/-64(net -14); tests+157/-15(net +142); test-support+1/-1; skill documentation+8/-3; changelog+1.Regression-first reproductions covered shallow-boundary scope errors, identity separators including CR/VT/FF and Unicode separators, one- and two-parent legacy grafts, and misplaced parent headers. Positive and refusal coverage includes true roots, retained shallow parents, replacement refs, full object-ID formats, malformed parent records, and merge exclusion. The integrated tests inspect the text and membership from one captured result. Upstream capture/evidence/progress owner implementations and their corresponding tests were preserved.
Dependency contracts were checked directly against Git's identity serialization, its consecutive LF-delimited parent parser, its independent graft/replacement setup, and Python's binary versus text-stream behavior, with real-Git regression proof.
Executed commands
Local proof used Python 3.14.7 and Git 2.55.0 on macOS. These are the repository-root forms of the executed commands; only machine-specific executable and checkout prefixes have been omitted. Git selection was process-local, not a change to machine defaults.
Focused integration suite — 20 tests
The skill validator ran in a disposable environment containing the existing pinned
requirements-dev.txtdependencies. Changed Python files were syntax-checked withast.parsebefore the test runs.These are local results, not a claim of live-provider E2E coverage or GitHub CI completion. Required PR CI remains a separate landing gate.
Windows CI follow-up
The first validation run passed Linux but exposed one existing progress-ticker fixture failure on Windows:
test_preparation_ticker_reports_while_caller_is_blockedobserved one flush instead of two. The fixture captured the real host clock before mocking the later tick. A reproducible start of1010.1makesstarted + 15 - startedequal14.999999999999886, correctly below the production reporting threshold.The pre-fix test reproduced the same
1 != 2failure under that clock. The repaired fixture owns initialization too, using an exactly representable clock origin. The same reproduction now passes, as do all three focused ticker/counting/cleanup tests. The real ticker, its 15-second threshold, thread ordering, and every existing assertion are unchanged; there is no retry, larger timeout, or production workaround. The independent review of this isolated test repair is clean. Current-head validation run 33278532267 passed on both Linux and Windows; CodeQL also passed.Hosted-review limitation
Hosted ClawSweeper review run 33278765964 stopped at input scanning before review inference. A local reproduction with the production scanner flags identified two
URIunknown results for the same unchanged negative proxy-URL fixture in the complete base/head test-file blobs. The fixture uses a reserved.invalidhostname and explicit synthetic username/password words; it tests rejection of credentialed proxies. It exists unchanged in both this PR's base and head, and already exists in OpenClaw's vendor copy outside the downstream diff.No real credential was identified or exposed, and no scanner override, exemption, fixture obfuscation, or retry-to-green was used. The hosted review remains unavailable; the clean independent reviews used the approved, scanned exact-diff path. A separate follow-up tracks narrowly auditable handling of synthetic URI fixtures in ClawSweeper. Landing proof is the independent review plus the executed regression and exact-head cross-platform CI, not a claimed hosted-bot verdict.