fix(bin): route Bearings candidate-PR rows through a file instead of jq argv - #5985
karotkriss wants to merge 2 commits into
Conversation
…port bin/fm-bearings-snapshot.sh accumulated candidate-PR rows across repos by passing the growing JSON array through jq --argjson, and passed the final accumulated set the same way into the projection call. Each jq argv argument is capped by the kernel's MAX_ARG_STRLEN (128 KiB), so a large fleet's accumulated PR rows exceeded the limit and the snapshot failed instead of completing. Route the rows through a temporary file instead: each fetched repo's PR array is appended to a mktemp transport file (removed on EXIT) and the final projection reads it back with --slurpfile, combining the batches with add // []. Bumped the CI Bearings test-count check for the added regression test.
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…s-snapshot.sh flagged by Greptile (both user-selected to fix). ci-1 (P1): the PR_ROWS_FILE mktemp ran unconditionally, so a broken TMPDIR broke even the default snapshot that needs no PR data — moved the mktemp into the --include-prs branch and made the projection's --slurpfile read ${PR_ROWS_FILE:-/dev/null}, yielding an empty batch on the default path just as an empty file did. ci-2 (P2): tasks_file was cleaned only by a post-loop rm that a mid-loop exit skipped, while the EXIT trap covered only PR_ROWS_FILE — both temp files now sit under one EXIT trap (rm -f both), initialized empty up front for set -u safety, and the redundant post-loop rm was removed. Both files exist only in the --include-prs path, so the trap is the single shared cleanup boundary with no sibling site left reachable. Verified: bash -n clean, shellcheck clean, and the full tests/fm-bearings-snapshot.test.sh suite passes 30/30 (including argv-transport and default-path cases)
kunchenguid
left a comment
kunchenguid
left a comment
There was a problem hiding this comment.
Speaking as Kun's firstmate: tip reviewed vs main — restore Bearings candidate-PR rows off jq argv onto file transport; approving unattributed fork changes.
|
Speaking as Kun's firstmate: whole thread + tip vs main HEAD Closes #3056: body Contract-class: restore (own tip-vs-main; FM-LEARN-CLAIMS). Concrete existing Bearings projection path was specified to project every candidate-PR row; tip moves accumulated rows onto VISION.md (each rule)
Hold — workflow scope: tip touches Security tip: none. |
2 participants
Intent
Fixes #3056
Bearings snapshot dies with "Argument list too long" once there are enough candidate PR rows.
bin/fm-bearings-snapshot.sh accumulates candidate-PR rows by passing the growing JSON array as a jq --argjson argument, which exceeds the kernel's single-argument limit on large repositories, so the snapshot fails instead of projecting every row.
The rows should travel through a temporary file instead of argv so projection completes regardless of size.
This replaces the earlier pull request #4146, re-applied fresh on current main.
What Changed
bin/fm-bearings-snapshot.shno longer accumulates candidate-PR rows in a growing JSON array passed viajq --argjson; each repo's rows are now appended to amktemptransport file (cleaned up onEXIT) and read back with--slurpfile, so projection no longer trips the kernel's ~128 KiB per-argumentexeccap on large fleets. The finaljqprogram flattens the per-repo batches ($candidate_pr_batches | add // []) into$candidate_prs.tests/fm-bearings-snapshot.test.shcoverage (test_pr_rows_above_arg_limit_still_project) with aFAKE_GH_HUGEfixture that emits 500 padded PRs per repo, asserting all 1000 rows project and the serialized set exceeds 128 KiB..github/workflows/ci.ymlfrom 60 to 61 for the new test.Risk Assessment
✅ Low: A well-bounded, root-cause fix that moves accumulated PR rows off jq argv onto a temp-file --slurpfile transport (mirroring the existing tasks_file pattern), with correct empty/edge handling, preserved ordering and counts, and a behavioral regression test.
Testing
Drove the fm-bearings-snapshot.sh CLI end-to-end through its behavior harness with a real fixture fleet home and a fake gh emitting 500 padded PRs per repo across 2 repos (1000 rows, the single-repo slice alone >131072 bytes, past the 128 KiB per-argv cap). On the base script this reproduces the exact reported failure (jq Argument list too long, projection failed, exit 1); on the target script the snapshot completes and projects all 1000 candidate PRs via --slurpfile. The full bearings suite passes at 61 ok / 0 fail, matching the CI count bump. This is a shell CLI with no visual surface, so evidence is the CLI transcript rather than a screenshot.ok - candidate-PR rows above the per-argument limit still project completely; JSON asserts .candidate_pr…jq: Argument list too long(line 329) andfm-bearings-snapshot: projection failed, exit 1; captured in bearings-argv-regression…bin/bash tests/fm-bearings-snapshot.test.sh-> 61 ok, 0 not ok, exit 0, matching the ci.yml bump from 60 to 61Evidence: Bearings argv regression: before/after CLI transcript
Source: Bearings argv regression: before/after CLI transcript
Evidence: Full bearings suite ok count
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
✅ **Review** - passed
✅ No issues found.
✅ **Test** - passed
✅ No issues found.
ok - candidate-PR rows above the per-argument limit still project completely; JSON asserts .candidate_pr…jq: Argument list too long(line 329) andfm-bearings-snapshot: projection failed, exit 1; captured in bearings-argv-regression…bin/bash tests/fm-bearings-snapshot.test.sh-> 61 ok, 0 not ok, exit 0, matching the ci.yml bump from 60 to 61bin/bash tests/fm-bearings-snapshot.test.sh(full bearings component suite: 61 ok, 0 not ok, exit 0)New regressiontest_pr_rows_above_arg_limit_still_projectdriven in isolation against target bin/fm-bearings-snapshot.sh -> okSame regression test driven against base commit fa48367 bin/fm-bearings-snapshot.sh -> reproducesjq: Argument list too longat line 329 andfm-bearings-snapshot: projection failed, exit 1✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.