fix(bin): stream large snapshot JSON inputs to jq - #1893
Open
TheKingJulian wants to merge 8 commits into
Open
Conversation
…napshot-json-v1' into fm/firstmate-stream-large-snapshot-json-v1
TheKingJulian
changed the title
3 tasks
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
Fix the live fm-bearings-snapshot.sh failure caused by fm-fleet-snapshot.sh passing payload-sized JSON documents through jq --argjson command-line arguments. Reproduce the end-user path with the current private backlog, separating the initiating trigger, Linux MAX_ARG_STRLEN masking threshold, and visible Bearings failure. Inspect every payload-sized --argjson crossing in bin/fm-fleet-snapshot.sh and replace large JSON process-argument transport with a scalable streaming or file-backed design while retaining small scalar --argjson uses. Preserve every existing projection, output mode, bound, failure contract, and local-only default. Do not reduce, prune, or reinterpret the captain's backlog. Add an executable end-to-end regression through the public snapshot interface using parsed backlog JSON over Linux MAX_ARG_STRLEN and assert successful output plus preserved decisions or inventory behavior. Run focused snapshot and Bearings tests, bin/fm-lint.sh, broader required suites, and the full no-mistakes pipeline. Preserve the existing commits and PR 1893, use the supported fork push path, and do not bypass the Stock macOS Bash snapshot compatibility check. Reproduce and fix that macOS check failure through the public interface, then report the PR only when all checks are green.
What Changed
jqthrough file-backed inputs while retaining--argjsonfor bounded scalar values.MAX_ARG_STRLENand include it in the stock macOS Bash CI count.Risk Assessment
✅ Low: The payload transport change is well-bounded, preserves existing snapshot behavior, and the prior portability and backlog-path contract regressions are resolved.
Testing
After session bootstrap and diff inspection, focused snapshot and Bearings suites passed. An end-to-end base-versus-target reproduction showed Linux MAX_ARG_STRLEN causing the original public snapshot failure, while the repaired snapshot preserved the full oversized backlog and Bearings rendered normally. No lint or broad suites were run because this assigned phase explicitly forbids lint and reserves broad validation for remote CI; Stock macOS Bash remains unverified locally.
Evidence: Base failure and repaired end-to-end CLI transcript
Evidence: Linux MAX_ARG_STRLEN counterfactual
Evidence: Repaired snapshot summary
Evidence: Complete repaired snapshot response
Evidence: Repaired end-user Bearings output
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 1 issue found → auto-fixed (2) ✅
tests/fm-bearings-snapshot.test.sh:456- The required criterion says, “do not bypass the Stock macOS Bash snapshot compatibility check” and “fix that macOS check failure through the public interface.” The changed fixtureln -s /proc/1/mem "$unreadable/data/backlog.md"is Linux-only. On macOS/proc/1/memis absent, so the dangling symlink is classified as a missing backlog and producesmissing structured backlog, which line 487's accepted-reason regex excludes. The macOS Bearings suite therefore remains reachable as a failure. Replace this with a platform-neutral unreadable fixture, or use explicit Darwin/Linux fixture branches with matching behavioral assertions.🔧 Fix: Make unreadable backlog fixture portable across platforms
1 error still open:
bin/fm-fleet-snapshot.sh:257- The requirement says, “Preserve every existing projection, output mode, bound, failure contract, and local-only default.” This condition changes the established handling of a directory, symlink-to-directory, or dangling symlink atdata/backlog.md: the base regular-file check returned a successful snapshot withbacklog.present=false(and a secondmate summary reason ofmissing structured backlog), while the new check sends it to jq and makes the snapshot fail. The test then accepts the newsnapshot failedreason. Restore the regular-file absence contract and create the platform-neutral access failure through the test boundary without broadening production path semantics, unless the captain explicitly approves this contract change.🔧 Fix: Preserve backlog contract with portable failure injection
✅ Re-checked - no issues remain.
.github/workflows/ci.yml:350- Required Stock macOS Bash 3.2 compatibility could not be demonstrated locally because this test phase ran on Linux with GNU Bash 5.3. Keep the macOS CI gate mandatory and do not report PR 1893 green until that job passes.bin/fm-session-start.shInspectedgit diff 70aeba855527f7693082f6dd1bc731e334d0269f..d6a72f2ce55d1a821ff4fec83d6425330b560e56to identify the affected public interfaces and regression coverage./bin/bash tests/fm-fleet-snapshot-view.test.sh/bin/bash tests/fm-bearings-snapshot.test.shExecuted base commit70aeba855527f7693082f6dd1bc731e334d0269fthroughfm-fleet-snapshot.sh --jsonwith an 851-record backlog; it exited 1 at jq withArgument list too long.Passed a 132,002-byte JSON value throughjq --argjson; Linux rejected the process argument with exit 126.Executed targetbin/fm-fleet-snapshot.sh --jsonwith the same backlog and verified an 871,948-byte parsed backlog, 851 preserved records, and the orphan inventory contradiction.Executed targetbin/fm-bearings-snapshot.shwith the same backlog and verified successful bounded TOON output with the main-inventory gate and omitted-count disclosure.uname -srmand/bin/bash --versionconfirmed the available environment was Linux with GNU Bash 5.3, not Stock macOS Bash 3.2.✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.