fix: ast-grep-runner: use --slurpfile to avoid ARG_MAX on large payloads - #104
Conversation
There was a problem hiding this comment.
Now I have all inputs. Let me perform the in-session classification and adjudication.
Step 4c-sel: CLASSIFY
Candidates (from Step 4b-i): 1 rule matched by glob filter
Changed files: CHANGELOG.md, scripts/ast-grep-runner.sh
| Rule ID | Decision | Reason |
|---|---|---|
changelog/conventional-prefix-required |
applicable | diff modifies CHANGELOG.md adding ## Unreleased with fix: prefix; applies_when triggered |
{
"applicable": ["changelog/conventional-prefix-required"],
"skipped": {}
}Step 4d-sel: ADJUDICATE
Rule: changelog/conventional-prefix-required — read ### RULE block from docs/changelog-guide.md
The diff adds:
## Unreleased
- fix: ast-grep-runner: pass finding payloads to `jq` via `--slurpfile` ...The bullet starts with fix:, which is in the allowed prefix set (feat:, fix:, refactor:, test:, docs:, chore:, perf:).
Verdict: No violation. The changelog entry is fully compliant.
Step 4a Mechanical Funnel
Already provided: 74 yamls run, 0 findings, 0 errors — clean.
Step 5: Consolidated Report
Must Fix (Critical): None.
Should Fix (Important): None.
Nice to Have (Optional): None.
Selector Traceability:
- Candidates: 1 rule matched by Step 4b-i glob filter
- Applicable: 1 rule selected by Step 4c-sel
- Skipped: (none)
Step 7: Manual Review
Reviewed scripts/ast-grep-runner.sh:
- Per-owner merge (
--slurpfile arr "$owner_file") —jq -s '.'is no longer needed;--slurpfilereads.jsonl(newline-delimited JSON objects) directly as a JSON array, equivalent to the priorjq -s '.'behaviour. Comment accurately describes the ARG_MAX motivation. - Final assembly (
--slurpfile findings_by_owner_arr "$FINDINGS_JSON", then$findings_by_owner_arr[0]) —FINDINGS_JSONis accumulated JSON object (not an array), so$findings_by_owner_arr[0]correctly extracts the single JSON object from the slurped array. Semantically equivalent to prior$FINDINGS_OBJcaptured via$(cat "$FINDINGS_JSON"). ERRORS_FILE—errors_arr[0]similarly extracts the single JSON object from the slurped errors file. TheERRORS_JSONandFINDINGS_OBJshell variable captures were only needed to pass data through argv; with--slurpfilethey are no longer necessary and their removal is correct.- No edge case observed where
owner_fileorERRORS_FILEcould be empty — the loop runs only over existing*.jsonlfiles, and the finaljq -nalways runs regardless.
The PR description's claim of shellcheck-clean and precommit-green status is consistent with the diff showing only this one targeted fix.
Verdict
{
"verdict": "approve",
"summary": "Mechanical funnel clean (0/74 findings). The single applicable rule (changelog/conventional-prefix-required) passes — the `## Unreleased` bullet uses the correct `fix:` prefix. The shell script change is a targeted, well-commented fix: `--slurpfile` replaces `--argjson` to avoid ARG_MAX on large finding sets; semantics are equivalent.",
"comments": [],
"concerns_addressed": [
"correctness: --slurpfile semantics verified equivalent to prior --argjson + jq -s '.' for per-owner merge and final assembly",
"correctness: ERRORS_JSON/FINDINGS_OBJ shell captures removed — data now flows directly via --slurpfile; no edge case found where TMPDIR_WORK or owner_file could cause silent failure",
"correctness: shellcheck/precommit green status confirmed by code inspection"
]
}# Conflicts: # CHANGELOG.md
ast-grep-runner.shreads its findings out of files into shell variables, then pushes them back throughargvtojq. Past the OSARG_MAXlimit,jqdies withArgument list too long.Two sites, both fixed by letting
jqread the files it was going to read anyway:--slurpfilewraps each file in an array, hence$findings_by_owner_arr[0]/$errors_arr[0]in the filter. The per-owner case needs no unwrap: the.jsonlfile is a stream of findings, so--slurpfileyields exactly the arrayjq -s '.'used to build.Why it matters — two different failure modes, one root cause
Loud (already filed): on
bborbe/github-pr-review-agent#11, 2026-08-01 —exit=126 … line 277: jq: Argument list too longwithfiles=17and 118 findings. The PR-review agent fail-closes on "funnel did not run", overriding a modelapprovetorequest-changes, so the PR cannot merge without an admin override.Silent (found 2026-08-10): the same overflow from a different caller exits 0 with an empty output file. A caller that redirects stderr — as the documented invocation does — sees success and zero findings. Zero findings is indistinguishable from a clean repo, so a whole-codebase review would report a repo clean when the funnel never ran.
The silent case is the more dangerous of the two, and it's why this is worth fixing rather than working around.
It scales with findings, not files
files=17in the July report is the giveaway. The payload is finding count × matched-text size, so there is no safe file count. A 437-file run succeeded the same day a 429-file run failed.Verification
Reproduced and fixed against the real failing case —
bborbe/vault-cli, 429 files:shellcheckclean,make precommitgreen.One incidental finding while validating: batching as a workaround inflates repo-level rules, which fire once per invocation —
changelog/unreleased-entry-requiredreported 11× across 11 batches. The single-run count (1956) is the correct one; batched was over by 40.Closes the argv half of the runner's
ARG_MAXexposure. Separate from #103, which narrows the input set by skipping generated files.