fix(review): hydrate the complete pinned source - #1333
Merged
Conversation
Hydrate the exact pinned Git delta even when the 80-file API context is truncated or the structural cache is unavailable. Remove duplicate API file/status mapping and use the scanner's existing byte budget with bounded blob metadata batches. Preserve source identities and scanner refusal. Reproduced the historical-blob failure on the actual public PR1070 partial clone, then verified its complete offline binary patch and real input scan.
Contributor
|
🦞👀 Pull request received. I will update this pull request when review starts. |
clawsweeper
Bot
added
P2
Contributor
|
Codex review: needs maintainer review before merge. Reviewed September 1, 2026, 4:55 AM ET / 08:55 UTC. ClawSweeper reviewWhat this changesThe PR hydrates all blobs named by the pinned raw Git delta independently of the 80-file API context and review-cache eligibility, then documents and tests that behavior. Merge readinessKeep open for normal maintainer review: the pinned branch contains a focused, source-backed repair for an existing review-hydration failure, with no definite introduced correctness defect found. Priority: P2 Review scores
Verification
How this fits togetherClawSweeper prepares a restricted Git checkout and source blobs before scanning a pull request. Those hydrated blobs let the input scanner inspect the exact pinned change before producing a review decision. flowchart LR
A[Pull request revisions] --> B[Raw pinned Git delta]
B --> C[Blob size and budget check]
C --> D[Fetch missing blobs]
D --> E[Restricted review checkout]
E --> F[Input scanner]
F --> G[Review result]
Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Land the exact-delta hydrator once the current-head validation completes, retaining fail-closed refusal for incomplete, unsafe, unsupported, or over-budget source. Do we have a high-confidence way to reproduce the issue? Yes. The source path is clear, and the PR records a concrete before/after production-helper replay against a blobless checkout of #1070; this review did not independently rerun that GitHub-backed scenario. Is this the best way to solve the issue? Yes. Reading the pinned raw Git delta removes the duplicate, incomplete API-file mapping while preserving pinned identities, existing scanner accounting, and fail-closed behavior. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against ce04d08fa319. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
|
steipete
pushed a commit
that referenced
this pull request
Sep 1, 2026
Bound each signed state-blob POST and response-body read to 15 seconds per attempt, preserving the existing four-attempt retry policy and idempotent uploads. The shared publisher covers direct and multipart operations with one added production line. The Git hydration portion of the original proposal is covered by #1333. Keep this PR focused on the remaining publication hang. Exceptionally slow valid requests intentionally fail through the existing retry path. Validated with actual signed native HTTP on macOS and AWS Crabbox cbx_cc3f61925287: four stalled attempts terminate, attempt-two recovery succeeds, stalled body consumption terminates, and repeated normal publication remains idempotent. Linux proof: https://crabbox.openclaw.ai/portal/runs/run_de0e80556b08. CI tested the exact current-main merge with 4,256 passing tests, 14 skips, and 13 changed-coverage tests; CodeQL and managed review passed. Hosted review accepted proof with no actionable code findings. Co-authored-by: Sebastien Tardif <sebtardif@ncf.ca>
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.
Full reviews could refuse valid pull requests with
incomplete_sourcewhen GitHub's bounded file context omitted part of the pinned diff. Source hydration depended on the cache digest and the 80-file API window. A partial clone's head checkout populated current files but left historical and deleted baseline blobs missing, so the raw delta was readable while the scanner's complete binary patch was not.Prepare source from the exact Git delta, independently of API context and cache eligibility. This removes the duplicate API file/status mapping and keeps the pinned REST base/head identities. Missing-object metadata stays in batches of at most 160 objects; fetching uses the scanner's existing 256 MiB upper bound. The scanner still owns aggregate input accounting and refuses incomplete, unsupported, or changed source.
This follows #1308 and fixes a different source-completeness failure exposed while reviewing #1070 and #1073. Their terminal failures correctly stopped retrying the unchanged revisions.
Live proof
Replayed the actual public PR1070 in a fresh blobless clone, using the production hydration helpers, real GitHub blob-size metadata, and the unchanged production scanner on Node24.20.0.
02c930c53d1c7e22ce89d23ba76e6b547a4e2a7f; head:8d583a05915f737926cd54e8943aee471e2d35ac.incomplete_source.Regression coverage extends the existing partial-clone fixture to a historical base, more than 80 files, more than 160 missing objects, binary content, deletion, mode changes, and literal pathspec names. It compares the complete offline patch with the source repository. Negative coverage retains invalid-source/path and over-budget refusal before fetching; bounded API context remains bounded.
Validation
Build and all 24 focused context tests pass. Managed Codex reviews before and after commit are scoped-clean at P0. The committed AWS full check passed 4,252 tests, 14 skipped, zero failures, plus 13/13 focused coverage tests on Node24.18.1 and pnpm11.10.0: https://crabbox.openclaw.ai/portal/runs/run_dc9562e86e1f. Current-head CI, CodeQL, sparse repair build, and Windows launcher checks passed: https://github.com/openclaw/clawsweeper/actions/runs/33489263316.
Maintainer review accepts the bounded preparation cost. The existing two-endpoint preparation can perform two hydration passes; this change removes incomplete API-derived inputs without adding another hydrator. Exact-head functional proof and full validation are complete. Large or incomplete inputs still refuse safely; this is not a maximum-size throughput benchmark.
Preparation remains in full-context collection before restricted checkout inspection. Structural reuse retains its existing pinned-source scan and refusal behavior. The change adds no dependencies or new source-preparation mechanism.