chore: harden local Git and validation test fixtures - #1270
Merged
Conversation
Contributor
|
🦞👀 Pull request received. I will update this pull request when review starts. |
clawsweeper
Bot
added
P3
Contributor
|
Codex review: needs maintainer review before merge. Reviewed August 27, 2026, 7:57 PM ET / 23:57 UTC. ClawSweeper reviewWhat this changesThe branch adds local Git and validation-fixture coverage for pruning, shallow-history recovery, timeout handling, retry isolation, and checkout identity checks, plus a README safety note. Merge readinessKeep open. This narrow test/docs follow-up adds regression coverage not present on current main for the already-landed qualified-ref fetch behavior; no introduced correctness or security defect was found. Priority: P3 Review scores
Verification
Live VerificationCommand: Result: PASS (completed) Assertions:
How this fits togetherClawSweeper’s review and repair lanes fetch a target branch, run approved validation commands, and verify that validation did not alter the checkout. These fixtures protect the Git-ref and validation boundaries that produce safe repair and review evidence. flowchart LR
A[Repair or review target] --> B[Local Git checkout]
B --> C[Base branch fetch]
C --> D[Validation command runner]
D --> E[Checkout identity check]
E --> F[Review and repair evidence]
Before merge
Agent review detailsSecurityNone. Review metrics
Technical reviewBest possible solution: Keep the production refspec implementation and land these focused regression fixtures after normal exact-head review and required checks complete. Do we have a high-confidence way to reproduce the issue? Not applicable: this PR adds fixture coverage for an already-landed fetch contract rather than repairing a currently reported user failure. Is this the best way to solve the issue? Yes: the coverage is placed in the narrow existing command, Git-helper, and target-validation test files while leaving the established runtime contract unchanged. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against d103c0cf5c98. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
|
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
1 participant
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.
What Problem This Solves
Resolves a problem where local Git and validation tests depend on host pruning settings, package-manager availability, or incidental setup scheduling instead of reliably exercising their intended behavior.
This is a test/docs-only follow-up to #1265. That PR already landed the three fully qualified production refspecs, basic retry-fixture isolation and exactly-two-attempt assertion, command-test pruning setup, and shallow helper coverage. This PR does not claim those production fixes again.
Why This Change Was Made
The remaining fixtures prove stale tracking-ref updates and
branchHasBaseDiffshallow-history recovery under both pruning settings, exercise real target-validation commands, and explicitly setfetch.prune=falsefor remote-only cases. Deadline fixtures account for completed setup/command phases while retaining actual subprocess execution, a real timeout, tracked-file mutation detection, and clean-checkout assertions. The test-owned package-script runner is generalized without changing the six existing pinned-base scenarios.Only README and three test files change: +252/-102 lines. The
src/**diff is empty. Upstream production behavior, absent-lockfile coverage, timeout/retry/security/offline gates, dependencies, suite concurrency, and coverage thresholds remain unchanged.User Impact
No production behavior change. Developers get fixtures that test the intended pruning, retry, and identity boundaries with less dependence on local setup timing. No global Git or package-manager configuration changes are required.
OpenClaw Bay Impact
None: no queue, lifecycle, review-publication, status, dashboard, or UI contract changes.
Documentation Impact
Reviewed README, CONTRIBUTING.md, AGENTS.md, and the documentation index. README's active Safety Model now explains the already-landed qualified-refspec behavior. ClawSweeper maintainers own that explanation; its source of truth is the review runtime and shared repair fetch helper, verified at the head below. Revisit it when those fetch contracts change. No duplicate changelog entry is needed for this test hardening.
Evidence
Validated head:
a6089c4eff7d29b53fdbd2e9470c4710050728de.Base:
d103c0cf5c98b0f3d4242bb4202c032856c36a49(main).Native macOS, Node 24.20.0, Git 2.55.0, repository-pinned pnpm 11.10.0. No install or global configuration changes were made for these runs. Independent committed-branch inspection and isolated Codex review reported no P0 findings.
The first full check failed in unchanged
test/live-proof-review-environment.test.ts: the original-pane and consecutive-command cases exceeded the terminal cleanup polling allowance. The failing test and driver/helper bytes match the captured base. Both focused reruns and both cases in the final full retry passed without code changes. Host timing is plausible, but the terminal cause remains unknown and is not fixed by this PR. The original failure remains part of the evidence, not a discarded attempt.Both full checks used normal inherited
fetch.prune=true, unchanged concurrency16, coverage thresholds, and budgets. The successful retry also passed static checks, builds, lint, formatting, docs checks, and the separate 12-test coverage stage. Overall retry coverage: 82.22% lines, 74.64% branches, 87.67% functions.An additional optional setup-budget mutant was rejected with an identity-deadline error instead of the fixture's required setup-deadline error. A scratch driver's prediction of
Missing expected exceptionwas over-specific and failed; that driver was not changed or rerun. This is recorded only as an observed mutant rejection, not as a green harness or proof of a complete fresh-global-deadline mutation. It is excluded from the five core negative assertions above.Real Behavior Proof
Claim and surface: actual local Git repositories and validator/package-script subprocesses preserve requested refs and exercise retries, shared deadlines, real command timeout, and post-command identity checks under explicit pruning, missing host package managers, and setup contention. The changed surface is the fixture behavior, not a production contract.
Scenarios and observations: a stale
origin/mainupdates to the new remote commit; a shallow feature checkout initially lacks a merge base, then recovers it throughbranchHasBaseDiffwithout losing the base ref. Target validation reaches its real Node script and retains the base ref and a clean checkout. The retry script runs exactly twice. Setup phases consume the shared budget before install. The runtime-budget case runs once and preserves the actual transient error as its cause. The mutation script changes a tracked file, writesmutated, stays alive until the real supervisor timeout, then recordsterminatedon macOS; the validator reports the required checkout-identity mutation error.Reproduce the complete touched-file surface after building, using Node24 and the repository's pinned pnpm:
pnpm run build:node node --test --test-reporter=tap \ test/command.test.ts \ test/repair/git-repo-utils.test.ts \ test/repair/target-validation.test.ts GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_NOSYSTEM=1 GIT_CONFIG_COUNT=0 \ node --test --test-reporter=tap \ --test-name-pattern='base fetch updates its requested ref|target validation preserves its base ref|local exact review explains when GitHub item is not open|local exact review selects PATH Codex' \ test/command.test.ts \ test/repair/git-repo-utils.test.ts \ test/repair/target-validation.test.ts pnpm run checkFor the contention replay, the two selected cases are
changed validation shares one timeout with checkout identity proofandvalidation reserves deadline to prove checkout mutation after command timeout; each was repeated 12 times in four concurrent test processes with process-localfetch.prune=falseandremote.origin.prune=true. This diagnostic replay did not alter suite concurrency. The no-host replay selected the sixpinned-basecases and the transient retry case with failing sentinel executables prepended to PATH. Child-local loader controls disabled retry, renewed the retry deadline, removed the identity proof window, or skipped the post-command identity check; each fixture rejected the corresponding broken behavior without modifying source files.Compact normalized trace from the executed runs:
Limits: native macOS proof only; platform skips remain. No Windows or Linux containment claim, live automation/review/apply execution, or real package-manager download/install claim. The focused selections overlap and are not additive unique-test counts. No terminal cleanup fix or proven contention root cause is claimed.