Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .agents/skills/firstmate-coding-guidelines/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,9 @@ Firstmate adds this skill's load instruction to firstmate-repo briefs by hand in
- Never add an agent name as a commit co-author.
- `bin/*.sh` and `bin/backends/*.sh` must pass `shellcheck`.
- Run `shellcheck bin/*.sh bin/backends/*.sh tests/*.sh` before treating a script change as done.
- Scripts must parse under macOS `/bin/bash` (bash 3.2), not just modern bash on Linux CI.
- Never assign a heredoc through command substitution (`VAR=$(cat <<EOF ... EOF)`): bash 3.2 tracks quote state through the heredoc body while scanning for the closing `)`, so a lone apostrophe in the body breaks parsing of the whole script (issue #166), and CI's modern bash cannot catch it via `bash -n`.
- Assign multi-line bodies with `read -r -d '' VAR <<EOF ... EOF || true` instead; `tests/fm-brief.test.sh` structurally refuses the fragile construct across `bin/`.
- Colocate tests with the existing pattern in `tests/`, name them `<subject>.test.sh`, and extend an existing script rather than inventing a new runner.
- A backend-verification doc (`docs/*-backend.md`) records empirical facts, not assumptions.
- Include the date, version, exact commands run, and exact output.
Expand Down
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ tests/fm-grok-harness.test.sh # grok adapter spawn hook, token guard
tests/fm-fleet-sync.test.sh # project clone refresh: safe detached recovery, STUCK drift reports, benign skips, and bootstrap relay
tests/fm-x-mode.test.sh # X-mode poll, inbox context round-trip, reply threading, dismiss, completion follow-up counters/caps, dry-run preview, and .env-presence activation tests
tests/fm-tangle-guard.test.sh # primary-checkout tangle detection, read-only remediation suppression, and spawn/brief isolation tests
tests/fm-brief.test.sh # fm-brief.sh bash -n parse regression guard (issue #166) and clean no-mistakes/direct-PR/local-only brief generation tests
tests/fm-brief.test.sh # fm-brief.sh bash -n parse regression guard (issue #166, also run under /bin/bash), a structural guard refusing $(cat <<...) across bin/*.sh and bin/backends/*.sh, and clean no-mistakes/direct-PR/local-only brief generation tests
tests/fm-spawn-batch.test.sh # batch dispatch and FM_HOME project-path scoping tests
tests/fm-spawn-dispatch-profile.test.sh # concrete dispatch profile flags: active-profile backstop, harness/model/effort meta, launch templates, batch forwarding, and secondmate exemption
tests/fm-update.test.sh # fast-forward-only self-update, reread, nudge, dedup, and skip-safety tests
Expand Down
6 changes: 2 additions & 4 deletions bin/fm-bootstrap.sh
Original file line number Diff line number Diff line change
Expand Up @@ -295,25 +295,23 @@ x_mode_setup() {

mkdir -p "$STATE" "$CONFIG" 2>/dev/null || { fmx_arm_failed; return 0; }

shim_body=$(cat <<EOF
read -r -d '' shim_body <<EOF || true
#!/usr/bin/env bash
# Auto-generated by fm-bootstrap.sh - X mode connector poll shim.
# The watcher runs this each check cycle; output becomes a check: wake.
export FM_HOME=$(printf '%q' "$FM_HOME")
exec $(printf '%q' "$FM_ROOT/bin/fm-x-poll.sh")
EOF
)
write_if_changed "$shim" "$shim_body" || { fmx_arm_failed; return 0; }
chmod +x "$shim" 2>/dev/null || { fmx_arm_failed; return 0; }

cadence_body=$(cat <<'EOF'
read -r -d '' cadence_body <<'EOF' || true
# Auto-generated by fm-bootstrap.sh - X mode watcher cadence.
# Source this before arming the watcher (see AGENTS.md "X mode") so fm-watch.sh
# polls the X check every 30s. Non-X instances have no such file and keep the
# default 300s cadence.
export FM_CHECK_INTERVAL=30
EOF
)
write_if_changed "$cadence" "$cadence_body" || { fmx_arm_failed; return 0; }

echo "FMX: X mode on - relay poll armed via state/x-watch.check.sh; 30s watcher cadence in config/x-mode.env"
Expand Down
13 changes: 7 additions & 6 deletions bin/fm-brief.sh
Original file line number Diff line number Diff line change
Expand Up @@ -171,37 +171,39 @@ read -r MODE _ <<EOF
$("$FM_ROOT/bin/fm-project-mode.sh" "$REPO")
EOF

# The DOD heredocs are assigned with `read -r -d ''` instead of `$(cat <<EOF ...)`:
# bash 3.2 (macOS /bin/bash) tracks quote state through a heredoc body nested in
# $( ), so a lone apostrophe in the brief text breaks parsing of the whole script
# (issue #166). read's exit status at EOF is nonzero, hence the `|| true`.
case "$MODE" in
direct-PR)
SETUP2=""
RULE1='1. Never push to the default branch (push only your `fm/'"$ID"'` branch). Never merge a PR.'
DOD=$(cat <<EOF
read -r -d '' DOD <<EOF || true
# Definition of done
This project ships **direct-PR**: you raise the PR yourself, without the no-mistakes pipeline.
The task is complete only when committed on your branch.
When it is implemented and committed, push your branch and open a PR with \`gh-axi\`, then append \`done: PR {url}\` to the status file and stop.
Do NOT run /no-mistakes. The captain reviews and merges the PR; firstmate relays it.
EOF
)
;;
local-only)
SETUP2=""
RULE1="1. Never push to any remote and never open a PR. Work only on your \`fm/$ID\` branch; firstmate handles the merge into local \`main\`."
DOD=$(cat <<EOF
read -r -d '' DOD <<EOF || true
# Definition of done
This project ships **local-only**: no remote, no PR, no pipeline.
The task is complete only when committed on your branch \`fm/$ID\`. Do NOT push, do NOT open a PR, do NOT merge.
Keep your branch a clean fast-forward onto the current default branch - if \`main\` has advanced, rebase onto it so the eventual merge stays a fast-forward.
When it is implemented and committed, append \`done: ready in branch fm/$ID\` to the status file and stop.
Firstmate then reviews your branch diff, the captain approves, and firstmate merges it into local \`main\`.
EOF
)
;;
*) # no-mistakes (default)
SETUP2="
2. Run \`no-mistakes doctor\`; if it reports the repo is not initialized here, run \`no-mistakes init\`."
RULE1='1. Never push to the default branch. Never merge a PR.'
DOD=$(cat <<EOF
read -r -d '' DOD <<EOF || true
# Definition of done
The task is complete only when committed on your branch.
When you believe it is complete, append \`done: {summary}\` to the status file and stop.
Expand All @@ -218,7 +220,6 @@ Two firstmate-specific rules layer on top of that guidance:

After /no-mistakes reports CI green, append \`done: PR {url} checks green\` and stop. You are finished.
EOF
)
;;
esac

Expand Down
39 changes: 30 additions & 9 deletions tests/fm-brief.test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,17 @@
# Behavior tests for bin/fm-brief.sh.
#
# Regression coverage for the heredoc-in-command-substitution parse bug (issue
# #166): each ship-mode branch builds its Definition-of-done text with
# `VAR=$(cat <<EOF ... EOF)`. Bash's lexer tracks quote state through the
# heredoc body while it scans for the matching `)` of the command
# substitution, so a single unescaped apostrophe anywhere in that body breaks
# parsing of the *entire rest of the script* - `bash -n` fails, not just the
# generated brief. A plain `cat > file <<EOF ... EOF` (not wrapped in `$(...)`)
# is unaffected, so the secondmate charter block does not need this guard.
# #166): the ship-mode branches used to build their Definition-of-done text
# with `VAR=$(cat <<EOF ... EOF)`. Bash 3.2 (macOS /bin/bash) tracks quote
# state through the heredoc body while it scans for the matching `)` of the
# command substitution, so a single unescaped apostrophe anywhere in that body
# breaks parsing of the *entire rest of the script* - `bash -n` fails, not
# just the generated brief. Modern bash (4+) parses it fine, so Linux CI never
# sees the failure. The branches now assign via `read -r -d '' VAR <<EOF`,
# which is immune; the structural test below keeps the fragile construct from
# coming back, since `bash -n` alone only catches it on a bash-3.2 machine.
# A plain `cat > file <<EOF ... EOF` (not wrapped in `$(...)`) is unaffected,
# so the secondmate charter block does not need this guard.
set -u

# shellcheck source=tests/lib.sh
Expand All @@ -17,13 +21,29 @@ set -u
TMP_ROOT=$(fm_test_tmproot fm-brief)

# The script itself must always parse. This is the direct regression test for
# issue #166: a stray apostrophe in any of the three DOD heredoc bodies
# (no-mistakes/direct-PR/local-only) breaks `bash -n` on the whole file.
# issue #166; on macOS it runs under the affected /bin/bash 3.2 itself.
test_script_parses() {
bash -n "$ROOT/bin/fm-brief.sh" 2>&1 || fail "bin/fm-brief.sh fails bash -n (heredoc/quote regression)"
if [ -x /bin/bash ]; then
/bin/bash -n "$ROOT/bin/fm-brief.sh" 2>&1 || fail "bin/fm-brief.sh fails /bin/bash -n (bash 3.2 heredoc/quote regression)"
fi
pass "fm-brief.sh: bash -n succeeds"
}

# Structural guard for issue #166: heredocs nested inside \$(cat <<...) are a
# bash-3.2 parse trap that Linux CI's modern bash cannot detect via `bash -n`,
# so refuse the construct itself anywhere in bin/ (including bin/backends/).
test_no_heredoc_in_command_substitution() {
local script
for script in "$ROOT"/bin/*.sh "$ROOT"/bin/backends/*.sh; do
# shellcheck disable=SC2016 # literal grep pattern; expansion is not wanted
if grep -n '^[^#]*\$(cat <<' "$script"; then
fail "${script#"$ROOT"/} uses \$(cat <<...) - breaks bash 3.2 parsing if the body ever gains an apostrophe; assign with read -r -d '' instead"
fi
done
pass "bin/*.sh, bin/backends/*.sh: no heredoc-in-command-substitution construct"
}

# Registry with one project per delivery mode, so each ship-mode DOD branch is
# exercised. A project absent from the registry defaults to no-mistakes.
write_registry() {
Expand Down Expand Up @@ -77,5 +97,6 @@ test_no_mistakes_dod_wording() {
}

test_script_parses
test_no_heredoc_in_command_substitution
test_ship_modes_generate_clean_briefs
test_no_mistakes_dod_wording
Loading