fix(bin): render fleet snapshots past the argv size ceiling - #37
Merged
Conversation
jq's --argjson hands its value to execve as one argv string, and Linux caps a single argv string at MAX_ARG_STRLEN (131072 bytes) regardless of how much total ARG_MAX is left. bin/fm-fleet-snapshot.sh bound unbounded aggregates - the parsed backlog, the task inventory, a secondmate home summary, the aggregated secondmate records - that way, so once a home outgrew the ceiling every affected composition aborted with "Argument list too long". The failure was total rather than partial: --json, --secondmate-home-summary, and the bearings projection over them all produced no output at all, and the message named jq and an argument list, which reads like corrupt data rather than a size limit. On this vessel a 259 KB rendered backlog already tripped it. Feed every value that grows with the size of a home to jq on stdin instead and bind it in the filter with `input`, which has no argv ceiling. Values go from shell variable straight into the pipeline, so nothing large reaches argv and there are no temp files to create, guard, or clean up on failure paths. bin/fm-backlog-lint.sh already avoided the same hazard with --slurpfile. Small, explicitly bounded values - counts, flags, paths, a single status line - stay on --arg/--argjson. Output is byte-for-byte unchanged for inputs that worked before, and a failed composition still exits non-zero and names its stage rather than degrading to an empty result. Two further sites turned out to be reachable today, not merely latent. A registered secondmate's home summary is accepted up to FM_SNAPSHOT_SECONDMATE_MAX_BYTES (262144), which is above the argv ceiling, so a 140 KB summary passed validation and then broke the reconciliation, record composition, accumulator, and landed roll-up that consume it. That path failed silently - exit 0 with the whole home reported unreadable and its landed work erased - which is worse than the loud abort. bin/fm-bearings-snapshot.sh needed no change: it already pipes the snapshot on stdin and inherited the bug only through this script. Its --argjson PR row set is bounded by the per-repo and per-run PR caps, not by home size.
Follow-up to the argv-size ceiling fix, closing findings the review raised. secondmate_current_json still bound the full task inventory on argv in its union composition, so the same total failure stayed reachable through --json. That site has its own trigger, independent of backlog size: TASKS_JSON grows with the number of live tasks. Measured on a 130-task home, TASKS_JSON renders 243177 bytes and, with only that one site left on argv, --json aborts with "fm-fleet-snapshot: registered secondmate aggregation failed" and no output. Audited every remaining --argjson afterwards. All that are left carry a boolean, an integer bound, a path-presence object, or a single parsed status line, and stay on argv deliberately. Two test corrections: The failure-semantics case claimed to prove that piping through stdin does not soften the contract, but its stub failed the first jq the script reached - backlog_json, an untouched call site - so it never got as far as a converted one. It now passes -Rn through to the real jq and fails only -n, which lands on main_inventory_json with the whole rendered backlog in flight, and it asserts the specific stage name so a regression to the earlier shape is visible. It also asserts no writer-side pipe noise leaks when the consumer exits without draining. This case passes before and after by design; it is a non-regression guard, not one of the three that pin the bug. The secondmate case drove the child home summary through the production 8-second timeout while rendering a ~140 KB summary. A slow runner would have degraded the record to the parent-event fallback and reported it in the exact words this suite uses for a real regression, so the timeout is pinned. Added a fourth case for the task-inventory axis. It inflates per-task rows rather than writing the ~90 rows real growth needs, because per-task state reconciliation dominates the runtime and the axis under test is the size of the rendered inventory either way. Output stays byte-for-byte identical to the pre-fix baseline on a fixture that exercises the structured-home path, reconciliation, the landed roll-up, and a captain hold. bin/fm-lint.sh is clean and the snapshot-bearings family passes.
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.
Intent
Land the already-built fleet snapshot argv-limit fix at the firstmate pin source as a transplant, preserving its eleven audited stdin conversions and strict four-case regression coverage. Revalidate that the three oversized cases fail before and pass after, while the hard-failure guard passes both ways and remains strict. Correct the newly discovered producer-side EPIPE regression so a jq failure reports only jq and the named stage; the supplied report was wrong because Bash 5.2.21 builtin printf emits Broken pipe when the new stdin consumer exits early. Preserve byte-identical successful small outputs, do not change the out-of-scope 262144-byte cap, and ship through the normal PR path.
What Changed
bin/fm-fleet-snapshot.shnow feeds every home-sized aggregate (backlog, task inventory, scout reports, secondmate current/landed, and the top-level composition) tojqon stdin via a newjson_stdinhelper bound withinput, instead of--argjson. Values that crossed Linux's per-argumentMAX_ARG_STRLEN(131072 bytes) previously failed the whole run with "Argument list too long", taking--json, the secondmate home summary, and the bearings projection over them down together. Small bounded values (counts, flags, paths) stay on--arg/--argjson, and the out-of-scope 262144-byteFM_SNAPSHOT_SECONDMATE_MAX_BYTEScap is untouched.jqfails before draining stdin the report names onlyjqand the failing stage. Under an ignored SIGPIPE disposition — how systemd services run these watchers — Bash's builtinprintfoutlived the closed pipe and leakedprintf: write error: Broken pipeinto an otherwise clean failure. Exit status is stilljq's and callers keep their existing hard-failure checks.tests/fm-fleet-snapshot-argv-limit.test.sh, a four-case suite over synthetic oversized homes covering backlog, task inventory, and secondmate home rendering plus a hard-failure guard that pins both SIGPIPE dispositions;bin/fm-test-run.shroutes it into thesnapshot-bearingsfamily. The three oversized cases fail on the base commit and pass on the target; the hard-failure guard passes both ways.Risk Assessment
✅ Low: The change is a mechanical, semantics-preserving transport swap whose 11 conversions I verified site-by-site for argument/input ordering, single-output filters, unchanged
., and intact exit-status propagation underset -uwithoutpipefail, backed by new targeted regression coverage and with the out-of-scope byte cap and all remaining bounded--argjsonsites untouched.Testing
I ran the change's own four-case suite (all pass, also through bin/fm-test-run.sh, which confirms the new snapshot-bearings routing and the runner's coverage guard), then rebuilt the base 8502757 and intermediate 8375673 trees from git and drove each case individually to show the three oversized cases fail before and pass after while the hard-failure guard passes both ways. Beyond unit results I verified the intent through the real CLI: on a synthetic 400-row home the base tree emits nothing and dies with jq's "Argument list too long" across --json, --secondmate-home-summary and bearings, while the target returns complete valid snapshots with all 400 records; small-home output, including the rendered fm-fleet-view table and the secondmate aggregation path, is byte-identical between base and target with the clock pinned and both trees staged at the same path, the only apparent diff being bearings' own wall-clock stamp, which I proved is noise by re-running the same tree. I reproduced the producer-side EPIPE regression on 8375673 under an ignored SIGPIPE, the disposition systemd services inherit, where "printf: write error: Broken pipe" wedges into the captain-facing report, and confirmed the target reports only jq's error plus the named stage while the large-value success path stays clean. A trace of an instrumented throwaway copy shows all eleven stdin conversion sites executing successfully, and the 262144-byte cap is untouched. The one gap I found is that the suite only exercised the default SIGPIPE disposition, so it could not guard the EPIPE fix; I extended the existing fourth case with a SIGPIPE-ignored re-run that passes on base, fails on the regressed tree, and passes on the target. Transient scratch trees were removed; only the test file change remains in the worktree.
Evidence: Oversized-home CLI transcript: base 8502757 aborts, target renders
--- tree: 8502757 fm-fleet-snapshot.sh --json rc=1 stdout=0 bytes stderr| bin/fm-fleet-snapshot.sh: line 583: /usr/bin/jq: Argument list too long stderr| fm-fleet-snapshot: main inventory summary failed fm-fleet-snapshot.sh --secondmate-home-summary rc=1 stdout=0 bytes stderr| bin/fm-fleet-snapshot.sh: line 611: /usr/bin/jq: Argument list too long stderr| fm-fleet-snapshot: secondmate home summary failed fm-bearings-snapshot.sh --json rc=1 stdout=0 bytes stderr| bin/fm-fleet-snapshot.sh: line 583: /usr/bin/jq: Argument list too long stderr| fm-fleet-snapshot: main inventory summary failed --- tree: after (target) fm-fleet-snapshot.sh --json rc=0 stdout=547601 bytes json | {"schema":"fm-fleet-snapshot.v1","backlog_records":400,"landed":0,"inventory_valid":true} fm-fleet-snapshot.sh --secondmate-home-summary rc=0 stdout=3999 bytes fm-bearings-snapshot.sh --json rc=0 stdout=2111 bytes json | {"schema":"fm-bearings.v1"}Evidence: Producer-side EPIPE: captain-facing stderr before vs after
=== pre-EPIPE-fix (8375673) --- SIGPIPE ignored (exit=1) | jq: simulated composition failure | 8375673/bin/fm-fleet-snapshot.sh: line 217: printf: write error: Broken pipe | fm-fleet-snapshot: main inventory summary failed === target (346498e) --- SIGPIPE ignored (exit=1) | jq: simulated composition failure | fm-fleet-snapshot: main inventory summary failed === target (346498e): success path with SIGPIPE ignored, oversized home exit=0 stdout=547572 bytes stderr=0 bytes json | {"schema":"fm-fleet-snapshot.v1","backlog_records":400,"inventory_valid":true}Evidence: Byte-identical small-home output plus the rendered fleet view
fm-fleet-snapshot.sh --json rc 0 -> 0 BYTE-IDENTICAL (13676 bytes stdout, 0 stderr) fm-fleet-snapshot.sh --backlog-json rc 0 -> 0 BYTE-IDENTICAL (2861 bytes stdout, 0 stderr) fm-fleet-snapshot.sh --secondmate-home-summary rc 0 -> 0 BYTE-IDENTICAL (1799 bytes stdout, 0 stderr) fm-bearings-snapshot.sh --json rc 0 -> 0 DIFFERS (only its own wall-clock generated field; see bearings-timestamp-noise.txt) fm-fleet-view.sh rc 0 -> 0 BYTE-IDENTICAL (1217 bytes stdout, 0 stderr) fm-fleet-view.sh as the captain reads it on the target tree: # Fleet View Schema: fm-fleet-snapshot.v1 ## Under Way | alpha-0001 | unknown / none | ship | alpha | tmux | ... | | mate | working / status-log | secondmate | ... | present / alive | ... | ## Queued | alpha-0002 | Repaint the hull | alpha | ship | - | - | ## Done | alpha-0000 | Splice the mainbrace | alpha | ship | - | - |Evidence: Per-case before/after verdicts (base 8502757 vs target)
BEFORE :: not ok - --json exited 1 on an oversized backlog BEFORE :: not ok - --json failed on a home with an oversized task inventory BEFORE :: not ok - child home summary failed BEFORE :: ok - a failed composition on a large piped value still exits non-zero and names its stage AFTER :: ok - oversized backlog renders a complete snapshot, home summary, and bearings projection AFTER :: ok - oversized task inventory renders through every consumer that binds it AFTER :: ok - oversized secondmate home summary stays structured and keeps its landed work AFTER :: ok - a failed composition on a large piped value still exits non-zero and names its stageEvidence: Hardened hard-failure guard across three trees
=== base 8502757 (pre-argv-fix: no json_stdin at all) ok - a failed composition ... names its stage === 8375673 (argv fix, EPIPE regression present) not ok - a failed composition leaked writer-side pipe noise with SIGPIPE ignored: jq: simulated composition failure 8375673/bin/fm-fleet-snapshot.sh: line 217: printf: write error: Broken pipe fm-fleet-snapshot: main inventory summary failed === target 346498e (argv fix + EPIPE suppression) ok - a failed composition ... names its stageEvidence: All eleven stdin conversion sites exercised (traced)
line 534 calls=5 task_json_lines() line 611 calls=2 main_inventory_json() line 638 calls=2 secondmate_home_summary_json() line 1056 calls=1 parent_evidence_reconciliation_json() line 1137 calls=2 secondmate_current_json() line 1263 calls=1 secondmate_current_json() line 1290 calls=1 secondmate_current_json() line 1292 calls=2 secondmate_current_json() line 1296 calls=2 secondmate_current_json() line 1306 calls=2 secondmate_landed_from_current_json() line 1363 calls=2 top-level snapshot composition total distinct conversion sites exercised: 11 of 11Evidence: Runner routing and suite run through bin/fm-test-run.sh
=== --list --family snapshot-bearings tests/fm-bearings-snapshot.test.sh tests/fm-fleet-snapshot-argv-limit.test.sh tests/fm-fleet-snapshot-view.test.sh === --check-coverage FM_TEST_COVERAGE ok total=114 parallel=30 serial=75 herdr=9 FM_TEST_BEGIN tests/fm-fleet-snapshot-argv-limit.test.sh family=snapshot-bearings expected_gate_skip=optional-binary ok - oversized backlog renders a complete snapshot, home summary, and bearings projection ok - oversized task inventory renders through every consumer that binds it ok - oversized secondmate home summary stays structured and keeps its landed work ok - a failed composition on a large piped value still exits non-zero and names its stage FM_TEST_END exit=0 duration_ms=11649Evidence: Manual verification script used for the CLI transcripts
Evidence: Bearings timestamp-noise control run
target vs target: DIFFERS on generated only (02:50:53Z vs 02:50:55Z) base vs target, generated normalized: BYTE-IDENTICAL (1060 bytes)Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
tests/fm-fleet-snapshot-argv-limit.test.sh:254- The hard-failure guard asserts the stage name is present and that no writer-side pipe noise leaks, but never asserts that jq's own diagnostic actually reaches stderr. The intent's criterion is that a jq failure reports "only jq and the named stage" — the "jq" half is unpinned, so a future change that swallowed the consumer's stderr along with the writer's would still pass this case. The stub already writes "jq: simulated composition failure" anderris captured with2>&1, so addingassert_contains "$err" "jq: simulated composition failure" "..."next to the existing stage-name assertion closes the gap with no fixture change.bin/fm-fleet-snapshot.sh:1291- The per-home loop rebuilds the whole records array with a fresh jq process each iteration, so N homes cost N jq spawns and O(N^2) bytes re-serialized — now also pushed through the new pipe. With the defaults (FM_SNAPSHOT_SECONDMATES=20) this is bounded and harmless, but under FM_SNAPSHOT_SECONDMATES=0 combined with FM_SNAPSHOT_SECONDMATE_LANDED_PER_HOME=0 — the unbounded configuration the new suite deliberately exercises — it is the next scaling wall right beside the one being fixed. Appending each$recordto a newline-delimited accumulator and folding once with a singlejq -safter the loop removes the inner jq entirely and is behavior-preserving. Pre-existing shape, touched but not worsened by this diff; noted rather than blocking.tests/fm-fleet-snapshot-argv-limit.test.sh:264- The suite's fourth case only exercised the default SIGPIPE disposition, where Bash silently kills the pipeline writer, so it passed identically on the pre-EPIPE-fix tree (8375673) and could not guard the producer-side EPIPE correction this change ships. I extended that same case (still four cases, no fifth added) with a second run of the identical failing-jq scenario under an ignored SIGPIPE, matching how systemd-run services inherit SIG_IGN. Verified: passes on base 8502757, fails on 8375673 with the leaked "printf: write error: Broken pipe", passes on target 346498e. The change is uncommitted in the worktree at tests/fm-fleet-snapshot-argv-limit.test.sh.bash tests/fm-fleet-snapshot-argv-limit.test.shon the target tree - all four cases ok in 11sbash bin/fm-test-run.sh tests/fm-fleet-snapshot-argv-limit.test.sh- confirms the new routing resolves family=snapshot-bearings, exit=0bash bin/fm-test-run.sh --list --family snapshot-bearingsandbash bin/fm-test-run.sh --check-coveragePer-case before/after:git archive 8502757 | tar -xinto a scratch tree, then single-function drivers fortest_oversized_backlog_still_renders,test_oversized_task_inventory_still_renders,test_oversized_secondmate_home_stays_readable,test_hard_failure_still_surfacesrun against base and targetManual CLI verification on a synthetic 400-row home:bin/fm-fleet-snapshot.sh --json,bin/fm-fleet-snapshot.sh --secondmate-home-summary,bin/fm-bearings-snapshot.sh --jsonon base 8502757 versus targetByte-identity check: both trees staged at one identical path withFM_SNAPSHOT_NOWandFM_SNAPSHOT_NOW_EPOCHpinned, comparing--json,--backlog-json,--secondmate-home-summary,bin/fm-fleet-view.shand bearings viacmp, on a home with a live task plus a registered secondmateBearings control run: two consecutive target-treebin/fm-bearings-snapshot.sh --jsonruns to prove the only diff is its own wall-clock generated fieldEPIPE reproduction: failing-jq PATH stub plus a wrapper that ignores SIGPIPE viatrapthenexec bin/fm-fleet-snapshot.sh --json, run against 8375673 and against the target, plus the large-value success path under the same dispositionConversion-site trace: instrumented throwaway copy loggingBASH_LINENOperjson_stdincall across structured-secondmate, fallback-provenance and home-summary runs - 11 of 11 sites executed, all rc=0grep -n 262144 bin/fm-fleet-snapshot.shandgit diff 8502757..346498e- the FM_SNAPSHOT_SECONDMATE_MAX_BYTES cap is unchangedHardened fourth case re-run against all three trees: ok on 8502757, not ok on 8375673, ok on 346498edocs/herdr-backend.md:280- Repo-wide jq usage invariants have no single owner: the reserved-keyword guard for jq variable names lives in docs/herdr-backend.md:280 (a backend-verification doc), while this change's argv-ceiling invariant now lives in a bin/fm-fleet-snapshot.sh code comment. A future session adding a jq call site has no one place to consult. Consolidating both into .agents/skills/firstmate-coding-guidelines/SKILL.md (with pointers left behind) is a follow-up; doing it here would be an unrelated documentation migration, so this change was left safe instead.✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.