Skip to content

fix(review): raise checkout inspection ls-files buffer for large repos - #1196

Merged
obviyus merged 1 commit into
mainfrom
fix/checkout-inspection-ls-files-buffer
Aug 17, 2026
Merged

fix(review): raise checkout inspection ls-files buffer for large repos#1196
obviyus merged 1 commit into
mainfrom
fix/checkout-inspection-ls-files-buffer

Conversation

@obviyus

@obviyus obviyus commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Problem

Since #1170, runAgentCheckoutInspection lists tracked files with git ls-files --stage -z under the 1 MB spawnSync default maxBuffer. openclaw/openclaw's stage listing is >3 MB, so every local review of an openclaw PR fails before Codex runs:

codex-failure classification=content_or_output
Read-only checkout inspection failed for #125278: ... spawnSync git ENOBUFS

Observed on pnpm run review -- --local-only --target-repo openclaw/openclaw --item-number 125278 at clawsweeper 866506fb0f.

Fix

src/agent-runner.ts: maxBuffer: 64 MiB on the listing spawn, with a comment naming the measurement.

pr-behavior-proof

  • claim: checkout inspection no longer ENOBUFS on repositories whose ls-files --stage -z output exceeds 1 MB
  • surface: runAgentCheckoutInspection
  • fixture: test/agent-runner.test.ts — index-only repo with 6000 × 220-byte paths (>1 MB listing), OpenClaw runner
  • result: pre-fix build → spawnSync git ENOBUFS; post-fix build → proceeds to challenge selection (could not select a tracked text line, the expected outcome for an index-only fixture)
  • command: node --test test/agent-runner.test.ts → 8/8 pass post-fix, 7/8 pre-fix (new test fails with ENOBUFS)
  • limits: not re-run against a live openclaw review from main yet; local review of openclaw #125278 with this build follows and will be linked here.

Live proof with this build

Local review of openclaw/openclaw#125278 (head 14ddf72f) with this branch built at 0617d19e: checkout inspection passed and Codex completed (decision=keep_open confidence=high, overallCorrectness: patch is correct, 0 findings, security cleared). Same command on main (866506fb0f) failed with Read-only checkout inspection failed ... ENOBUFS in ~1s.

OpenClaw Bay impact

None. This changes only the local pre-review checkout inspection command's output buffer; no publication, queue, status/telemetry, or dashboard data contract changes, so Bay needs no update.

runAgentCheckoutInspection lists tracked files with git ls-files --stage -z
using the 1 MB spawnSync default. openclaw/openclaw's listing is over 3 MB,
so every read-only checkout inspection failed with ENOBUFS (output buffer
overflow) and no review could complete.
@clawsweeper

clawsweeper Bot commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

@obviyus
obviyus marked this pull request as ready for review August 17, 2026 12:52
@clawsweeper clawsweeper Bot added merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. P1 Urgent regression or broken agent/channel workflow affecting real users now. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Aug 17, 2026
@clawsweeper

clawsweeper Bot commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Codex review: found issues before merge. Reviewed August 17, 2026, 8:56 AM ET / 12:56 UTC.

ClawSweeper review

What this changes

This PR raises the Git tracked-file listing buffer used during checkout inspection and adds a regression test for an index listing exceeding Node’s default buffer.

Regression provenance

Possible regression — probable (reproduction; reviewed change). No predecessor PR is attributed.

Merge readiness

⚠️ Ready for maintainer review - 4 items remain

Keep open for normal maintainer review: current main still has the 1 MiB default on this pre-review path, while this focused patch raises the bound, adds regression coverage, and supplies credible after-fix real-review evidence. One non-blocking OpenClaw Bay-impact statement remains due under repository policy.

Priority: P1
Reviewed head: 0617d19e53197da3e73162894350d16b2df1e135

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) The patch is narrowly scoped, regression-backed, and supported by an after-fix real-review receipt; only a non-blocking Bay-impact statement remains.
Proof confidence 🐚 platinum hermit (4/6) Sufficient (live_output): The PR body records an after-fix real large-repository review completing past checkout inspection, with the same command on main reported to fail with ENOBUFS.
Patch quality 🐚 platinum hermit (4/6) 1 actionable review finding remain.

Verification

Check Result Evidence
Real behavior Verified Sufficient (live_output): The PR body records an after-fix real large-repository review completing past checkout inspection, with the same command on main reported to fail with ENOBUFS.
Evidence reviewed 7 items Current-main failure path: Current main invokes git ls-files --stage -z without an explicit buffer, leaving Node’s default limit in place before candidate selection.
Narrow repair: The PR applies a bounded 64 MiB buffer only to the tracked-file listing that feeds checkout inspection.
Regression coverage: The added test creates a >1 MiB index listing, verifies that threshold, and asserts inspection reaches the expected challenge-selection failure instead of ENOBUFS.
Findings 1 actionable finding [P3] State the OpenClaw Bay impact
Security None None.

How this fits together

Checkout inspection validates a target repository’s tracked files before ClawSweeper starts an agent review. A successful inspection permits Codex or OpenClaw review to proceed; a failure stops review publication and records a retryable infrastructure error.

flowchart LR
A[Target checkout] --> B[Git tracked-file listing]
B --> C[Checkout inspection]
C --> D{Challenge available?}
D -->|Yes| E[Agent review starts]
D -->|No or command error| F[Retryable review failure]
Loading

Before merge

  • State the OpenClaw Bay impact (P3) - This change affects review admission before publication, so repository policy requires the PR body or handoff to say whether OpenClaw Bay is affected. The prior review requested this and the updated body still lacks the statement; add a brief note that Bay is unaffected if that remains true.
  • Resolve merge risk (P1) - The pre-review command now permits up to 64 MiB of tracked-file output in memory; this is a deliberate bounded availability tradeoff, while repositories beyond that ceiling still fail closed.
  • Resolve merge risk (P1) - The PR body does not yet state whether OpenClaw Bay is affected, as required for review-lifecycle changes by repository policy.
  • Complete next step (P2) - No repair dispatch is appropriate for an active contributor PR; the remaining item is a small PR-body policy follow-up.

Findings

  • [P3] State the OpenClaw Bay impact — src/agent-runner.ts:93
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Patch scope production +3, tests +37; 2 files affected The implementation is tightly scoped to the blocking command and a regression scenario.

Merge-risk options

Maintainer options:

  1. Accept the bounded inspection capacity (recommended)
    Keep the 64 MiB ceiling because it covers the reported >3 MiB repository while preserving a deterministic fail-closed limit for exceptional output sizes.

Technical review

Best possible solution:

Retain the bounded 64 MiB limit and regression test, add a brief statement that OpenClaw Bay is unaffected by this local admission-path change, then land after the current review and checks remain valid.

Do we have a high-confidence way to reproduce the issue?

Yes. The PR body supplies a concrete current-main command and ENOBUFS result against a large OpenClaw checkout, plus a controlled oversized-index regression scenario.

Is this the best way to solve the issue?

Yes. Raising only this listing command’s bound to the repository-standard 64 MiB value fixes the documented failure without weakening the fail-closed inspection behavior.

Full review comments:

  • [P3] State the OpenClaw Bay impact — src/agent-runner.ts:93
    This change affects review admission before publication, so repository policy requires the PR body or handoff to say whether OpenClaw Bay is affected. The prior review requested this and the updated body still lacks the statement; add a brief note that Bay is unaffected if that remains true.
    Confidence: 0.99

Overall correctness: patch is correct
Overall confidence: 0.94

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 866506fb0f57.

Labels

Label changes:

  • add proof: sufficient: Contributor real behavior proof is sufficient. The PR body records an after-fix real large-repository review completing past checkout inspection, with the same command on main reported to fail with ENOBUFS.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body records an after-fix real large-repository review completing past checkout inspection, with the same command on main reported to fail with ENOBUFS.
  • remove status: 📣 needs proof: Current PR status label is status: 👀 ready for maintainer look.
  • remove rating: 🦪 silver shellfish: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.

Label justifications:

  • P1: The reported buffer overflow prevents large-repository reviews from starting before an agent can inspect the PR.
  • merge-risk: 🚨 availability: The changed limit controls a pre-review command whose failure blocks the review workflow.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body records an after-fix real large-repository review completing past checkout inspection, with the same command on main reported to fail with ENOBUFS.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body records an after-fix real large-repository review completing past checkout inspection, with the same command on main reported to fail with ENOBUFS.

Evidence

What I checked:

  • Current-main failure path: Current main invokes git ls-files --stage -z without an explicit buffer, leaving Node’s default limit in place before candidate selection. (src/agent-runner.ts:86, 866506fb0f57)
  • Narrow repair: The PR applies a bounded 64 MiB buffer only to the tracked-file listing that feeds checkout inspection. (src/agent-runner.ts:93, 0617d19e5319)
  • Regression coverage: The added test creates a >1 MiB index listing, verifies that threshold, and asserts inspection reaches the expected challenge-selection failure instead of ENOBUFS. (test/agent-runner.test.ts:265, 0617d19e5319)
  • Review-blocking call site: A checkout-inspection error throws before model review starts, matching the reported availability impact. (src/clawsweeper-review-runtime.ts:1005, 0617d19e5319)
  • Consistent existing capacity convention: The repository command wrapper already uses a 64 MiB default, supporting the selected bounded value. (src/command.ts:48, 0617d19e5319)
  • Feature provenance: Checkout inspection appears to date to the merged checkout-access work, which introduced the feature this follow-up repairs. (src/agent-runner.ts:86, ad5cf512a448)

Likely related people:

  • jesse-merhi: The merged checkout-access feature that established this inspection path is attributed to Jesse Merhi in repository history. (role: checkout-inspection feature introducer; confidence: high; commits: ad5cf512a448; files: src/agent-runner.ts, src/clawsweeper-review-runtime.ts)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Add a short PR-body statement that OpenClaw Bay is unaffected by this checkout-inspection change.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (1 earlier review cycle)
  • reviewed 2026-08-17T12:51:54.029Z sha 0617d19 :: needs real behavior proof before merge. :: [P3] State the OpenClaw Bay impact

@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Aug 17, 2026
@obviyus
obviyus merged commit 9bbc0b0 into main Aug 17, 2026
27 of 28 checks passed
@obviyus
obviyus deleted the fix/checkout-inspection-ls-files-buffer branch August 17, 2026 12:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. P1 Urgent regression or broken agent/channel workflow affecting real users now. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant