fix(bin): pass large fleet snapshot JSON to jq via files instead of argv - #2737
fix(bin): pass large fleet snapshot JSON to jq via files instead of argv#2737Omar-Nawaf wants to merge 2 commits into
Conversation
A grown backlog's parsed JSON exceeded the Linux per-argument cap (MAX_ARG_STRLEN, 131072 bytes), so every jq exec receiving it as a single --argjson value failed with "Argument list too long" and the whole fleet snapshot died at the main-inventory step. Stage the backlog and task inventories in a trap-cleaned temp directory and bind them with --slurpfile at each jq call site that consumes them. The jq program bodies are unchanged, so output stays byte-identical for a normal-sized backlog. Signal exits route through the same cleanup so a timed-out cross-home read leaves no directory behind. Add a colocated regression covering both output modes with a backlog JSON larger than the cap, and give the Perl-timeout fixture's minimal toolbin the mktemp/rm it now needs.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1954b6a8fe
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| registry=$(registry_secondmates_json) || return 1 | ||
| union=$(jq -n --argjson registry "$registry" --argjson tasks "$tasks" ' | ||
| ($registry.records // []) as $registered | ||
| union=$(jq -n --argjson registry "$registry" --slurpfile tasks_in "$tasks_file" ' |
There was a problem hiding this comment.
Captain, when FM_SNAPSHOT_REGISTRY_BYTES and FM_SNAPSHOT_REGISTRY_RECORDS are raised for a large secondmate registry, $registry can exceed Linux's 131072-byte per-argument limit and this remaining --argjson invocation fails with Argument list too long, aborting the entire snapshot. I reproduced this with an approximately 208 KB registry and otherwise empty fleet; stage the registry in the temporary directory and load it via --slurpfile like the task data.
Useful? React with 👍 / 👎.
1 participant
Intent
Publish the confirmed large-fleet snapshot correction in one upstream PR referencing #2629 (not duplicate #2613). Ensure every bin/fm-fleet-snapshot.sh call site that passes unbounded backlog or task aggregates to jq avoids Linux argv-size limits, while leaving bounded scalar --argjson uses unchanged unless evidence requires otherwise. Use the script's existing trap-cleaned temporary-file discipline or an equally direct single-owner input path, preserving jq semantics and normal-sized output. Preserve or improve an executable oversized regression where an ordinary source backlog comfortably beyond Linux MAX_ARG_STRLEN succeeds, and prove normal output compatibility without source-text assertions. Validate focused fleet snapshot, fleet view, and bearings behavior, the oversized regression, documentation audience checks if prose changes, exact bin/fm-lint.sh, and the full no-mistakes pipeline. Do not include private data, state, config, .env, project content, credentials, local paths, unrelated changes, or agent co-authorship. Push only this task branch through no-mistakes to the authorized public fork Omar-Nawaf/firstmate, open the PR against kunchenguid/firstmate:main, and never merge.
What Changed
bin/fm-fleet-snapshot.shnow writes unbounded JSON values (backlog, tasks, scout reports, main inventory, accepted child summaries, and the secondmate current/landed aggregates) to a trap-cleaned temporary directory and binds them in jq with--slurpfile, replacing the--argjsonargv handoffs that failed with "Argument list too long" once a value exceeded the Linux per-argument cap (MAX_ARG_STRLEN, 131072 bytes); per-record secondmate roll-up also accumulates into a JSONL file instead of re-feeding a growing shell variable through jq argv. Bounded scalar--argjsonuses are unchanged.tests/fm-fleet-snapshot-view.test.shadds three executable oversized regressions: a 400-item backlog beyond the argv cap in both--jsonand--secondmate-home-summarymodes, a single child-home summary landing between the argv cap and the byte-acceptance guard, and a multi-home fleet aggregate that only crosses the cap when combined — each asserting normal, complete snapshot output without source-text assertions.tests/fm-bearings-snapshot.test.shaddsmktempandrmto the restricted toolbin in the perl-fallback bounds test so the snapshot script's new temp-file staging works under that fixture.Risk Assessment
✅ Low: The fix-round commit is a semantics-preserving mechanical migration of every unbounded jq handoff to the trap-cleaned file/--slurpfile path, verified against each call site and covered by behavioral regressions that pin both the individual-summary (131073–262144 byte) and fleet-aggregate argv windows and would fail at the pre-fix head, with all intent constraints (bounded scalar argv preserved, jq bodies unchanged, no co-authorship or private data) confirmed and only one contrived informational failure-mode note remaining.
Testing
Ran the two focused suites touched by the change (fleet snapshot/view with its three new oversized regressions, and the full bearings snapshot suite) — all pass; manually proved the regression is real by running the base-commit script against a backlog whose parsed JSON exceeds Linux MAX_ARG_STRLEN (it fails with "Argument list too long") and the fixed script on the same fixture (complete 400-record snapshot); confirmed byte-identical output on normal-sized fixtures in both output modes, verified trap-based temp-directory cleanup leaves nothing behind, and exercised the real end-user surfaces (fm-fleet-view.sh and fm-bearings-snapshot.sh) over the oversized fleet. No visual artifact applies — this is a CLI/JSON tool, so CLI transcripts are the reviewer-visible surface. Lint, docs checks (no prose changed), and the full pipeline belong to other phases of this run.
Evidence: Before/after: base script fails with 'Argument list too long', fixed script snapshots 400 records
Source: Before/after: base script fails with 'Argument list too long', fixed script snapshots 400 records
$ # BEFORE (base commit a0cec26, --argjson passes backlog JSON on jq argv) $ FM_HOME=<fixture: 400-item queued backlog, parsed JSON > 131072 bytes> fm-fleet-snapshot.sh --json exit code: 1 --- stderr --- .../fm-fleet-snapshot.sh: line 611: jq: Argument list too long fm-fleet-snapshot: main inventory summary failed $ # AFTER (target commit 1954b6a, --slurpfile stages the same JSON through trap-cleaned temp files) exit code: 0 --- output checks --- backlog JSON bytes inside snapshot: 369977 (argv cap is 131072) backlog records: 400 main_inventory.valid: trueEvidence: Normal-sized output byte-identical across base and fixed, temp staging trap-cleaned
Source: Normal-sized output byte-identical across base and fixed, temp staging trap-cleaned
mode --json: outputs byte-identical (5180 bytes) mode --secondmate-home-summary: outputs byte-identical (1453 bytes) # Temp-file cleanup: TMPDIR contents after fixed-script runs (should be empty) (empty — trap cleanup removed all fm-fleet-snapshot.* staging dirs)Evidence: Fleet view and bearings render the oversized 400-item fleet (end-user CLI surfaces)
Source: Fleet view and bearings render the oversized 400-item fleet (end-user CLI surfaces)
Evidence: Full fm-fleet-snapshot-view.test.sh transcript (18/18 ok, includes the three oversized regressions)
Source: Full fm-fleet-snapshot-view.test.sh transcript (18/18 ok, includes the three oversized regressions)
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
bin/fm-fleet-snapshot.sh:1292- Residual argv-size window in the cross-home aggregation path: the intent requires that "every bin/fm-fleet-snapshot.sh call site that passes unbounded backlog or task aggregates to jq avoids Linux argv-size limits", but a child-home summary is still passed via--argjson summary(line 1292) after a byte guard whose default acceptance bound (FM_SNAPSHOT_SECONDMATE_MAX_BYTES=262144, line 82) is twice MAX_ARG_STRLEN (131072). A summary in the 131073–262144 byte window passes the check at line 1247 and then reproduces the exact fixed failure ("Argument list too long") in the parent. Concrete reachable paths: (a) the documented FM_SNAPSHOT_SECONDMATE_LANDED_PER_HOME=0 cap-lift (line 794 emits the child's full Done-section projection, i.e. an unbounded backlog aggregate); (b) the uncapped invalidity.ids/reason fields that scale with child task counts. Additionally, SECONDMATE_CURRENT_JSON (up to FM_SNAPSHOT_SECONDMATES=20 records, each embedding a summary that individually fit argv) can sum past the cap at the--argjson current(line 1348) and--argjson secondmate_current(line 1413) handoffs in a large busy fleet — the same population issue fm-fleet-snapshot dies at ~53 backlog items: unbounded aggregates passed to jq via argv hit the 128 KiB MAX_ARG_STRLEN cap #2629 concerns. These call sites are pre-existing and were deliberately left unchanged per the intent's "leaving bounded scalar --argjson uses unchanged" scoping, so this is flagged for the author's decision rather than as a blocker. Recommended shared boundary if pursued: route these aggregate handoffs through the jq_value_file/--slurpfile discipline this change introduces, or clamp FM_SNAPSHOT_SECONDMATE_MAX_BYTES below MAX_ARG_STRLEN.🔧 Fix: stage child summaries and fleet aggregates to jq via slurpfile
1 info still open:
bin/fm-fleet-snapshot.sh:1337- Failure-mode softening in the rewritten record accumulation: previously a failed per-record jq invocation inside secondmate_current_json cascaded into a loud snapshot failure (the next--argjson records "$records"accumulation errored and the script exited 1); nowprintf '%s\n' "$record" >>"$records_file"appends an empty line when $record is empty, --slurpfile silently skips the whitespace, and the mate's record is silently omitted fromsecondmate_current.recordswhileshown/total(computed from the union before the loop) still count it, producing an internally inconsistent snapshot instead of an error. Reachability is contrived — it requires one of the loop's internal jq execs to fail (OOM, or JSON-escape inflation pushing a byte-bounded --argjson capture such as activities past MAX_ARG_STRLEN on a control-character-dense status log) — and that same trigger also failed pre-change, just loudly. If desired, a one-line guard ([ -n "$record" ] || return 1before the append) restores fail-loud behavior; not required for merge.✅ **Test** - passed
✅ No issues found.
bash tests/fm-fleet-snapshot-view.test.sh— all 18 tests pass, including the three new oversized regressions (backlog over the argv cap in both output modes, child summary between the argv cap and the byte guard, fleet aggregate over the cap)bash tests/fm-bearings-snapshot.test.sh— fail-fast suite exited 0 (all 82 tests), including the modifiedtest_perl_fallback_bounds_github_callwhose minimal toolbin now carries mktemp/rmFail-before/pass-after manual repro: ran the base-commitbin/fm-fleet-snapshot.sh --jsonagainst a 400-item backlog home (parsed backlog JSON 369,977 bytes > MAX_ARG_STRLEN 131,072) — it fails withjq: Argument list too long; the target-commit script succeeds with all 400 records andmain_inventory.valid == trueOutput-compatibility check: base vs fixed script on a normal-sized fixture withFM_SNAPSHOT_NOWpinned —--json(5,180 bytes) and--secondmate-home-summary(1,453 bytes) outputs are byte-identicalTemp-file discipline: after fixed-script runs with a scratchTMPDIR, the directory is empty — the EXIT trap removed everyfm-fleet-snapshot.*staging dirEnd-user surfaces over the oversized home:bin/fm-fleet-view.shrenders the full 400-row fleet view (exit 0) andbin/fm-bearings-snapshot.shproduces its bearings projection (exit 0)Checked both commit messages for forbidden content — no agent co-authorship, private data, or local paths✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.