Skip to content

fix(bin): restore fm-brief.sh parse under macOS bash 3.2 and guard against regressions - #990

Closed
localxcrm wants to merge 4 commits into
kunchenguid:mainfrom
localxcrm:fm/fix-brief-bash32
Closed

localxcrm wants to merge 4 commits into
kunchenguid:mainfrom
localxcrm:fm/fix-brief-bash32

Conversation

@localxcrm

Copy link
Copy Markdown

Intent

Fix bin/fm-brief.sh, which since PR #945 (commit ec09871) fails to parse under macOS /bin/bash 3.2.57 - an apostrophe ('firstmate's authority check') inside an unquoted heredoc body that sits inside a $( ) command substitution in the no-mistakes case branch breaks bash 3.2's parser, so ALL brief generation was broken on every macOS firstmate home (bash 4/5 parse it, which is why Linux CI stayed green). The fix rewords that one line to 'the firstmate authority check' - the #945 ask-user authority wording must survive semantically, only the apostrophe goes. Audited the rest of bin/ (no other occurrences). Added the version-independent regression guard test_dod_heredocs_carry_no_apostrophes in tests/fm-brief.test.sh (bash -n on Linux bash 5 cannot reproduce the 3.2 quirk), pinned the #945 wording apostrophe-free, and documented the style rule in firstmate-coding-guidelines. Captain-approved follow-ups from prior runs' gates are already on the branch: the review gate extended the guard to quoted-delimiter heredocs (HERDR_SECTION site), and the document gate updated the stale assertion in tests/fm-ask-user-authority.test.sh. Per the captain's FORK+PR decision, the gate is configured with fork-url: branches push to the captain-authorized fork localxcrm/firstmate while the PR opens against upstream kunchenguid/firstmate main. Out of scope: no refactoring of fm-brief.sh beyond this fix, no other repos.

What Changed

  • Reworded the apostrophe in bin/fm-brief.sh's no-mistakes case-branch heredoc (firstmate's authority check → the firstmate authority check), fixing the bash 3.2 command-substitution/heredoc parser failure introduced in PR fix: enforce contract boundaries for ask-user findings #945 that broke brief generation on every macOS firstmate home while leaving Linux CI green.
  • Added a version-independent regression guard (test_dod_heredocs_carry_no_apostrophes / test_bin_heredocs_carry_no_apostrophes) in tests/fm-brief.test.sh, extended repo-wide across bin/*.sh to also cover quoted-delimiter heredocs, using an odd/even single-quote parity check since bash -n on Linux bash 5 cannot reproduce the 3.2 quirk.
  • Documented the new style rule in .agents/skills/firstmate-coding-guidelines/SKILL.md: heredoc bodies inside $(...) command substitutions must not carry an unbalanced count of single quotes.
  • Updated the stale wording assertion in tests/fm-ask-user-authority.test.sh to match the reworded, apostrophe-free string.

Risk Assessment

✅ Low: The follow-up commit correctly widens the regression guard to scan all bin/*.sh files with a parity-based single-quote check that I empirically verified against real /bin/bash 3.2.57 on this machine (constructed odd/even-quote heredoc test cases and confirmed the parser fails exactly on odd counts and passes on even/balanced counts), confirmed bin/fm-brief.sh, bin/fm-bootstrap.sh, and bin/fm-fleet-snapshot.sh all parse cleanly under that real bash 3.2 binary, and confirmed no stale references to the renamed test function remain — fully resolving the round-1 audit-accuracy gap with no new issues introduced.

Testing

Directly reproduced and resolved the reported bash 3.2 parse failure using real macOS /bin/bash 3.2.57 (conveniently the default bash on this host): the base commit fails bash -n with the exact error class described (unexpected EOF / syntax error from the unbalanced apostrophe inside the command-substitution heredoc), and the target commit parses cleanly. Full fm-brief and fm-ask-user-authority test suites pass under bash 3.2, and a manual fault-injection check confirms the new repo-wide regression guard and DOD-wording assertions are real (not tautological) — reintroducing the apostrophe reproduces the failure and restoring the fix clears it. Worktree is clean; no findings.

Evidence: bash 3.2.57 parse repro: base commit fails, target commit passes
=== bash --version (default on PATH in this environment) ===
GNU bash, version 3.2.57(1)-release (arm64-apple-darwin25)

=== BASE commit 10ee779 bin/fm-brief.sh under real bash 3.2.57: bash -n ===
/tmp/fm-brief-repro/base-fm-brief.sh: line 314: unexpected EOF while looking for matching `)'
/tmp/fm-brief-repro/base-fm-brief.sh: line 388: syntax error: unexpected end of file
exit: 2

=== TARGET commit c913543 bin/fm-brief.sh under real bash 3.2.57: bash -n ===
exit: 0
Evidence: tests/fm-brief.test.sh full run output under bash 3.2.57 (target commit)
ok - fm-brief.sh: bash -n succeeds
ok - bin/*.sh: command-substitution heredoc bodies carry no unbalanced single quotes
ok - fm-brief.sh: --help renders the complete header
ok - fm-brief.sh: no-mistakes/direct-PR/local-only briefs generate cleanly
ok - fm-brief.sh: faster paths use configured authority without stacked review
ok - fm-brief.sh: no-mistakes DOD wording avoids the apostrophe regression
ok - fm-brief.sh: ship project-memory wording carries the AGENTS.md authoring bar
ok - fm-brief.sh: --herdr-lab emits the complete hard safety contract
ok - fm-brief.sh: --herdr-lab uses its quoted Firstmate-owned helper path
ok - fm-brief.sh: ship and scout scaffolds make omitted Herdr intent fail-visible
ok - fm-brief.sh: Herdr lab contract covers scouts and rejects secondmate misuse
ok - fm-brief.sh: --no-projects scaffolds a project-less charter and guards misuse
ok - fm-brief.sh: marked requests avoid generic acknowledgements and preserve material reporting
ok - fm-brief.sh: custom pause verb renders in every scaffold
ok - fm-brief.sh: investigation and visual-review completions load the shared decision policy
ok - fm-brief: scout and secondmate code paths still scaffold well-formed briefs

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

🔧 **Review** - 1 issue found → auto-fixed ✅
  • ℹ️ bin/fm-bootstrap.sh:692 - The commit message for eae0a3d and the intent ('Audited the rest of bin/ (no other occurrences)') claim fm-brief.sh's three DOD branches are the only VAR=$(cat <<EOF ... EOF) heredocs in bin/. That's inaccurate: bin/fm-bootstrap.sh:692 (cadence_body=$(cat <<'EOF' ... EOF)) and bin/fm-fleet-snapshot.sh:787,837,849,885 (script=$(cat <<'BASH' ..., parse_filter=$(cat <<'JQ' ..., etc.) use the identical pattern, which is equally vulnerable to the bash 3.2 command-substitution/heredoc parser bug. None of those bodies currently contain an apostrophe, so nothing is broken today, but the new repo-wide style rule added to firstmate-coding-guidelines/SKILL.md ('a heredoc body that sits inside a $(...) command substitution') is worded generally, while the only enforcement (test_dod_heredocs_carry_no_apostrophes) scans exclusively bin/fm-brief.sh.

🔧 Fix: Widen bin heredoc apostrophe guard repo-wide with correct odd-quote parity check
✅ Re-checked - no issues remain.

✅ **Test** - passed

✅ No issues found.

  • /bin/bash -n on bin/fm-brief.sh at base commit 10ee779 (real macOS bash 3.2.57) — reproduces the PR #945 parse failure
  • /bin/bash -n on bin/fm-brief.sh at target commit c913543 (real macOS bash 3.2.57) — parses cleanly
  • bash tests/fm-brief.test.sh (16 tests, run under bash 3.2.57 since it's the default bash here) — all pass, including new test_bin_heredocs_carry_no_apostrophes guard and the #945 wording assertions in test_no_mistakes_dod_wording
  • bash tests/fm-ask-user-authority.test.sh (9 tests) — all pass, including the updated stale-wording assertion
  • Manual regression check: reintroduced "firstmate's authority check" apostrophe into bin/fm-brief.sh, reran tests/fm-brief.test.sh under bash 3.2 — test_script_parses failed immediately with the same bash -n error as the base commit, confirming the guard/fix is load-bearing; restored file via git checkout
  • grep audit of all bin/*.sh files with heredocs for other stray-apostrophe occurrences — none found outside comments (which are unaffected, being outside command-substitution heredoc bodies)
✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

PR kunchenguid#945 (ec09871) introduced an apostrophe ("firstmate's authority
check") inside a $(cat <<EOF ...) heredoc in the no-mistakes DOD
branch. Bash 3.2's command-substitution scanner mis-handles single
quotes in heredoc bodies, so /bin/bash -n bin/fm-brief.sh failed and
every macOS home could not generate briefs; bash 4/5 parses it, which
is why Linux CI stayed green.

- Reword to "the firstmate authority check" (semantics unchanged).
- Audit: the only $(cat <<EOF) heredocs in bin/ are fm-brief.sh's
  three DOD branches; no other bin script fails /bin/bash -n.
- Add a version-independent regression guard in tests/fm-brief.test.sh
  that rejects apostrophes in command-substitution heredoc bodies
  (bash -n alone cannot catch this on a bash 5 host), and pin the
  kunchenguid#945 authority wording in its apostrophe-free form.
- Document the no-apostrophe-in-$(...)-heredoc style rule in
  firstmate-coding-guidelines.

Verified: /bin/bash -n bin/fm-brief.sh passes under bash 3.2.57,
tests/fm-brief.test.sh green under bash 3.2 and 5, fm-lint clean,
and a real no-mistakes brief scaffolds with the kunchenguid#945 wording.
@localxcrm

Copy link
Copy Markdown
Author

Friendly ping — this fix is validated end to end (full pipeline green, guard test added per review) and is only waiting on workflow approval + maintainer merge. #1140 (DerivedData GC on teardown) is in the same situation; reviewing both together might save you a round-trip. Thanks!

@kunchenguid

kunchenguid commented Jul 28, 2026 •

Copy link
Copy Markdown
Owner

Automated reminder: thanks for the PR! This branch currently has a merge conflict with the base branch.

When you get a chance, please rebase onto (or merge) the latest base branch, resolve the conflict, and push. After that, checks will re-run and the PR will get looked at again.

Noted for firstmate#990 at c913543b.

@kunchenguid

Copy link
Copy Markdown
Owner

Automated reminder: this PR still looks blocked on a rebase or merge conflict fix.

If you are still interested, please rebase onto the current base branch, resolve the conflict, and push.

If I do not hear back, I may close this as inactive.

@kunchenguid kunchenguid removed the wheelhouse:pending-contributor-action Managed by Wheelhouse label Aug 11, 2026
@kunchenguid

Copy link
Copy Markdown
Owner

Speaking as Kun's firstmate: closing this as stale. It has been waiting on a contributor update for 14+ days with no author push or comment. Reopen if you want to pick it back up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants