fix(bin): fail closed on fleet snapshot framing and jq argv overflow - #3
Merged
Conversation
Reject truncated or duplicate scout-report framing and refuse to publish JSON when staged cleanup fails. Stage large snapshot and bearings payloads on disk so jq never hits the kernel argv limit.
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
Correct remaining fleet read-framing duplicate-ID and cleanup false successes. Also fix the jq argv overflow path used by bearings (oversized --argjson) so fm-bearings-snapshot.sh works on this fleet size. Firstmate shared tracked fix. Load firstmate-coding-guidelines before edit. Tests must stay green. Do not force-push.
What Changed
--argjson, so oversized homes no longer hit the kernel argv limit.Risk Assessment
✅ Low: Both prior findings are verifiably fixed at the shared file-staging boundary, the full diff satisfies every source-verifiable intent constraint, behavior-based fail-closed tests cover the new framing and cleanup paths, and no residual argv-scaling or new defects were found.
Testing
Ran the two changed behavior suites (all green), then exercised the public snapshot and bearings CLIs on the live firstmate home and an oversized disposable home. The live 327KB snapshot and 1.4MB oversized snapshot both publish complete JSON and project through bearings; the old --argjson transport still fails with Argument list too long on that live payload. Duplicate IDs, truncated report-list framing, and cleanup failure all fail closed with empty stdout and no leftover workspace.
Evidence: Reviewer packet tying live, overflow, and fail-closed evidence
Evidence: Live fleet snapshot transcript
FM_HOME=/home/willc/firstmate backlog_bytes=65542 scout_reports=701 snapshot_rc=0 snapshot_bytes=327031 elapsed_sec=5.71Evidence: Live fleet snapshot summary
schema=fm-fleet-snapshot.v1 backlog_records=99 tasks=6 scout_reports=701 unique_ids=701 unique_paths=701 main_inventory.valid=trueEvidence: Live fleet snapshot JSON
Evidence: Live bearings --json transcript
bearings_rc=0 bearings_bytes=109302 schema=fm-bearings.v1 reports=701 landed=9 decisions_open=20 of 57/tmp/no-mistakes-evidence/01KZXPPT95NPM8EGP8E0NBEME3/live-bearings.json)Evidence: Default TOON bearings on the live home
/tmp/no-mistakes-evidence/01KZXPPT95NPM8EGP8E0NBEME3/live-bearings-default.toon)Evidence: Old --argjson E2BIG vs new file-fed jq
snapshot_bytes=327031 old_argjson_rc=126 stderr=/home/willc/.local/bin/jq: Argument list too long; new file path rc=0 schema=fm-fleet-snapshot.v1Evidence: Oversized-home snapshot and bearings CLI
backlog_bytes=211233 snapshot_rc=0 snapshot_bytes=1433679 done_count=1200 bearings_rc=0 landed=1200 no leftover workspaces/tmp/no-mistakes-evidence/01KZXPPT95NPM8EGP8E0NBEME3/oversized-fleet.json) - Evidence: Oversized-home bearings JSON (local file:/tmp/no-mistakes-evidence/01KZXPPT95NPM8EGP8E0NBEME3/oversized-bearings.json)Evidence: Fail-closed duplicate-ID, cleanup, and truncated-framing CLI
Source: Fail-closed duplicate-ID, cleanup, and truncated-framing CLI (local file:
/tmp/no-mistakes-evidence/01KZXPPT95NPM8EGP8E0NBEME3/fail-closed-transcript.txt)Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
bin/fm-bearings-snapshot.sh:283- Residual jq argv overflow path of the same class this change fixes: the candidate-PR accumulation loop round-trips the growing rows array throughjq -n --argjson a "$rows"on every repo iteration. With --all-pr-repos or a raised FM_BEARINGS_PR_LIMIT (the script's own omitted-disclosure at line 506 directs users to raise it), the aggregate can exceed Linux's 128 KiB per-argument limit (MAX_ARG_STRLEN) and fail with E2BIG. The CANDIDATE_PRS_FILE staging added at line 305 only protects payload sizes that this earlier --argjson would already have crashed on, making that staging moot until this site is also converted. Fix at the same shared boundary the rest of the change uses: append each repo_rows document to a staged file under $BEARINGS_TMP and combine once withjq -s 'add'. Default caps (10 repos x 20 PRs) stay well under the limit, so the stated "works on this fleet size" goal is still met.bin/fm-bearings-snapshot.sh:196- Cleanup-trap asymmetry introduced in the same commit: fm-fleet-snapshot.sh traps HUP/INT/TERM toexitso its EXIT cleanup runs on signal death, but fm-bearings-snapshot.sh registers onlytrap cleanup_bearings_tmp EXIT. Bash does not run EXIT traps when killed by an untrapped signal, so an interrupted bearings run leaks its $BEARINGS_TMP directory under /tmp. Adding the same three signal traps used by the fleet script closes the gap.🔧 Fix: stage candidate-PR rows in file and trap signals for cleanup
1 info still open:
bin/fm-bearings-snapshot.sh:295- The fix-round PR-rows staging (pr-rows.jsonl +jq -s 'add // []') has no dedicated regression test reproducing the oversized candidate-PR aggregate, unlike the SNAP_FILE overflow which got test_oversized_canonical_snapshot_survives_projection. The mechanism is identical to the already-tested file-staging boundary and the code is straightforwardly verifiable, so this is informational only.✅ **Test** - passed
✅ No issues found.
bash tests/fm-fleet-snapshot-view.test.shbash tests/fm-bearings-snapshot.test.shFM_HOME=/home/willc/firstmate bin/fm-fleet-snapshot.sh --jsonon the live firstmate home (65,542-byte backlog, 701 scout reports)FM_HOME=/home/willc/firstmate bin/fm-bearings-snapshot.sh --json --all-landed --all-reportsFM_HOME=/home/willc/firstmate bin/fm-bearings-snapshot.shdefault TOONjq -n --argjson snap "$(cat live-fleet-snapshot.json)" '$snap.schema'vsjq -er '.schema' < live-fleet-snapshot.jsonFM_HOME=<oversized-home> bin/fm-fleet-snapshot.sh --jsonthenbin/fm-bearings-snapshot.sh --json --all-landed(211,233-byte backlog / 1,200 done rows)manualbin/fm-fleet-snapshot.sh --jsonagainst duplicate-ID, truncated NUL framing, and first-rmcleanup mutants✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.