Skip to content
Merged
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
9 changes: 8 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,14 @@ jobs:
esac
/bin/bash --version | head -1
command -v jq >/dev/null || { echo "::error::jq is required"; exit 1; }
/bin/bash -n bin/fm-fleet-snapshot.sh

shell_inventory="$RUNNER_TEMP/fm-shell-inventory"
bin/fm-lint.sh --list-files > "$shell_inventory"
parse_fail=0
while IFS= read -r f; do
/bin/bash -n "$f" || { echo "::error::stock macOS Bash 3.2 failed to parse $f"; parse_fail=1; }
done < "$shell_inventory"
[ "$parse_fail" -eq 0 ] || { echo "::error::stock macOS Bash 3.2 parse sweep failed"; exit 1; }

snapshot_output=$(/bin/bash tests/fm-fleet-snapshot-view.test.sh)
printf '%s\n' "$snapshot_output"
Expand Down
4 changes: 2 additions & 2 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ That is firstmate-specific; do not commit `.no-mistakes/evidence/` here even whe
Check and test the toolbelt before pushing:

```sh
for script in bin/*.sh bin/backends/*.sh; do bash -n "$script"; done # syntax-check the toolbelt
while IFS= read -r script; do /bin/bash -n "$script" || exit; done < <(bin/fm-lint.sh --list-files) # syntax-check the canonical shell surface
bin/fm-lint.sh # lint the toolbelt and behavior tests; the single owner CI and the no-mistakes gate both run
bin/fm-test-run.sh tests/<subject>.test.sh # one script (primary local focus path, timed)
bin/fm-test-run.sh --family pure-contract-unit # ordinary family-scoped local path (serial, timed)
Expand All @@ -93,7 +93,7 @@ Its header and `--help` own the flags, family labels, lanes, and changed-file ma
Portable shard balance evidence lives in `docs/fm-test-portable-shards.md`.
Local no-mistakes Test stays intent-targeted and must not wire `commands.test` to `--all` or a `tests/*.test.sh` walk.
Family selection is the ordinary local path; `--all` is deliberate full regression only.
CI owns broad regression across required portable parallel shards, the portable serial lane, the Herdr lane, lint, invariants, the coverage guard, and macOS snapshot compatibility in [`.github/workflows/ci.yml`](.github/workflows/ci.yml).
CI owns broad regression across required portable parallel shards, the portable serial lane, the Herdr lane, lint, invariants, the coverage guard, and stock macOS Bash compatibility in [`.github/workflows/ci.yml`](.github/workflows/ci.yml).
Use `bin/fm-test-run.sh --help` for lane names, `--jobs` rules, and required gate-skip flags when reproducing a lane locally.
Discover tests by listing `tests/*.test.sh`: each is a self-contained bash script named `<subject>.test.sh`, and its header comment describes what it covers, so pass one to `bin/fm-test-run.sh` to focus on a subject with canonical timing output.
Tests that need a real optional backend or an explicit opt-in (real herdr/zellij/cmux smoke tests, the live Pi regression) skip themselves and print the tool or environment gate needed to enable them, so the portable suite remains safe on machines without those tools.
Expand Down
18 changes: 10 additions & 8 deletions bin/fm-brief.sh
Original file line number Diff line number Diff line change
Expand Up @@ -217,13 +217,13 @@ HERDR_SECTION=$(printf '%s\n' \
'Never bypass the helper, even for a read-only lifecycle probe or cleanup after failure.' \
'The captain fleet uses the running `default` session.')
else
HERDR_SECTION=$(cat <<'EOF'
IFS= read -r -d '' HERDR_SECTION <<'EOF' || true
# Herdr lifecycle declaration - NOT ENABLED
**HARD SAFETY GATE:** this scaffold cannot inspect the task text that replaces `{TASK}` later.
If the task will start, stop, delete, restart, profile, or otherwise drive Herdr lifecycle behavior, stop and regenerate the brief with `--herdr-lab` before dispatch.
Do not add Herdr lifecycle commands to this unguarded brief by hand.
EOF
)
HERDR_SECTION=${HERDR_SECTION%$'\n'}
fi

if [ "$KIND" = scout ]; then
Expand Down Expand Up @@ -285,33 +285,31 @@ 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
IFS= 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 configured merge authority decides whether to merge the PR; firstmate relays the outcome.
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
IFS= 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.
The configured merge authority approves the ready branch, then firstmate merges it into local \`main\` through the guarded fast-forward path.
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
IFS= 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 @@ -329,10 +327,14 @@ Two firstmate-specific rules layer on top of that guidance:

After /no-mistakes reports CI green (the CI-ready return point - do not wait for it to keep monitoring in the background until merge), append \`done: PR {url} checks green\` and stop. You are finished.
EOF
)
;;
esac

# read -r -d '' preserves the heredoc's trailing newline that the removed
# $(...) command substitution used to strip. Drop that one newline so generated
# briefs stay byte-identical to the historical Bash 5 output.
DOD=${DOD%$'\n'}

cat > "$BRIEF" <<EOF
You are a crewmate: an autonomous worker agent managed by firstmate. Work on your own; do not wait for a human.

Expand Down
33 changes: 23 additions & 10 deletions bin/fm-lint.sh
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
# fm-lint.sh --jobs <1|2> [path]... override bounded worker count
# fm-lint.sh --telemetry <path> ... write a quiet metrics snapshot
# fm-lint.sh --required-version print the ShellCheck pin
# fm-lint.sh --list-files print the canonical file set
# fm-lint.sh --help print this usage
set -u

Expand Down Expand Up @@ -83,11 +84,12 @@ if [ "${1:-}" = "--required-version" ]; then
fi

fm_lint_usage() {
sed -n '2,25{s/^# \{0,1\}//;p;}' "$SELF"
sed -n '2,26{s/^# \{0,1\}//;p;}' "$SELF"
}

JOBS=${FM_LINT_JOBS:-2}
TELEMETRY=${FM_LINT_TELEMETRY:-}
LIST_FILES=0
while [ "$#" -gt 0 ]; do
case "$1" in
--jobs)
Expand All @@ -108,6 +110,10 @@ while [ "$#" -gt 0 ]; do
TELEMETRY=${1#*=}
shift
;;
--list-files)
LIST_FILES=1
shift
;;
--help|-h)
fm_lint_usage
exit 0
Expand All @@ -125,6 +131,22 @@ case "$JOBS" in
*) printf 'fm-lint.sh: jobs must be 1 or 2, got %s.\n' "$JOBS" >&2; exit 2 ;;
esac

if [ "$#" -gt 0 ]; then
ROOTS=("$@")
else
ROOTS=(bin/*.sh bin/backends/*.sh tests/*.sh)
fi
ROOT_COUNT=${#ROOTS[@]}

if [ "$LIST_FILES" -eq 1 ]; then
[ "$#" -eq 0 ] || {
printf 'fm-lint.sh: --list-files does not accept explicit paths.\n' >&2
exit 2
}
printf '%s\n' "${ROOTS[@]}"
exit 0
fi

if ! command -v shellcheck >/dev/null 2>&1; then
printf 'fm-lint.sh: ShellCheck not found; install ShellCheck %s for CI parity.\n' \
"$REQUIRED_SHELLCHECK" >&2
Expand All @@ -144,15 +166,6 @@ if [ "$resolved" != "$REQUIRED_SHELLCHECK" ]; then
exit 1
fi

if [ "$#" -gt 0 ]; then
ROOTS=("$@")
else
# Canonical file set: the one authoritative definition. Callers never repeat
# these globs, and every adapter and test shell remains an independent root.
ROOTS=(bin/*.sh bin/backends/*.sh tests/*.sh)
fi
ROOT_COUNT=${#ROOTS[@]}

if [ -n "$TELEMETRY" ]; then
telemetry_parent=$(dirname "$TELEMETRY")
[ -d "$telemetry_parent" ] || {
Expand Down
174 changes: 160 additions & 14 deletions tests/fm-brief.test.sh
Original file line number Diff line number Diff line change
@@ -1,14 +1,18 @@
#!/usr/bin/env bash
# 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.
# Regression coverage for the heredoc-in-command-substitution parse bug (issues
# #166, #958, #1069). Building a variable with `VAR=$(cat <<EOF ... EOF)` is
# unsafe on Bash 3.2 (macOS /bin/bash): the lexer scans for the matching `)` of
# the command substitution textually and tracks quote state through the heredoc
# body, so a single apostrophe, unbalanced quote, or unbalanced paren anywhere
# in that body breaks parsing of the *entire rest of the script* - `bash -n`
# fails, not just the generated brief. The DOD and Herdr-section builders now
# use `IFS= read -r -d '' VAR <<EOF || true` instead, which removes the `$(...)`
# wrapper and eliminates the whole defect class regardless of future prose.
# test_no_heredoc_in_command_substitution guards that structure directly.
# Ambient `bash -n` here is Bash 5 and cannot see the bug, so the real
# cross-version enforcement lives in the macos-stock-bash CI job.
set -u

# shellcheck source=tests/lib.sh
Expand All @@ -18,9 +22,10 @@ TMP_ROOT=$(fm_test_tmproot fm-brief)
BRIEF_HOME="$TMP_ROOT/home"
mkdir -p "$BRIEF_HOME/data"

# 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.
# The script itself must always parse under the ambient bash. That is Bash 5 in
# CI and locally, where the issue #958/#1069 parser bug does not fire, so this
# is a weak guard on its own; test_no_heredoc_in_command_substitution and the
# macos-stock-bash CI job carry the real cross-version enforcement.
test_script_parses() {
local out rc
out=$(bash -n "$ROOT/bin/fm-brief.sh" 2>&1); rc=$?
Expand All @@ -29,6 +34,142 @@ test_script_parses() {
pass "fm-brief.sh: bash -n succeeds"
}

# Structural class guard (issues #166, #958, #1069): never build a variable by
# wrapping a heredoc in a command substitution (`VAR=$(cat <<EOF ... EOF)`).
# That construct is what breaks Bash 3.2 parsing, and pinning one historical
# apostrophe phrase (as the old test did) missed the #945 reintroduction. This
# guards the *shape* directly against the whole file, so any future DOD or
# section builder that reintroduces the class fails here regardless of prose.
test_no_heredoc_in_command_substitution() {
local unsafe safe
unsafe="$TMP_ROOT/heredoc-in-substitution.sh"
safe="$TMP_ROOT/plain-heredoc.sh"
# shellcheck disable=SC2016 # Literal shell fixtures must remain unexpanded.
printf '%s\n' 'value=$(' ' cat <<EOF' 'body' 'EOF' ')' > "$unsafe"
# shellcheck disable=SC2016 # Literal shell fixtures must remain unexpanded.
printf '%s\n' 'cat <<EOF' '$(' ' cat <<INNER' 'INNER' ')' 'EOF' > "$safe"
if no_heredoc_in_command_substitution "$unsafe"; then
fail "structural guard accepted a multiline heredoc nested in a command substitution"
fi
no_heredoc_in_command_substitution "$safe" \
|| fail "structural guard treated heredoc body prose as shell structure"
no_heredoc_in_command_substitution "$ROOT/bin/fm-brief.sh" \
|| fail "fm-brief.sh wraps a heredoc in a command substitution (breaks Bash 3.2 parsing)"
pass "fm-brief.sh: no heredoc is nested inside a command substitution (Bash 3.2 parse-safe)"
}

no_heredoc_in_command_substitution() {
perl - "$1" <<'PERL'
use strict;
use warnings;

my $path = shift;
open my $source, '<', $path or die "$path: $!\n";
my @frames;
my @heredocs;
my $quote = '';
my $line_number = 0;

while (my $line = <$source>) {
$line_number++;
if (@heredocs) {
my $candidate = $line;
$candidate =~ s/\r?\n\z//;
$candidate =~ s/^\t+// if $heredocs[0]{strip_tabs};
shift @heredocs if $candidate eq $heredocs[0]{delimiter};
next;
}

my $length = length $line;
for (my $i = 0; $i < $length; $i++) {
my $char = substr($line, $i, 1);
if ($quote eq "'") {
$quote = '' if $char eq "'";
next;
}
if ($char eq '\\') {
$i++;
next;
}
if ($quote eq '"' && $char eq '"') {
$quote = '';
next;
}
if ($char eq "'" && $quote eq '') {
$quote = "'";
next;
}
if ($char eq '"' && $quote eq '') {
$quote = '"';
next;
}
if ($char eq '#' && $quote eq '' && ($i == 0 || substr($line, $i - 1, 1) =~ /[\s;|&()]/)) {
last;
}
if ($char eq '$' && substr($line, $i + 1, 1) eq '(') {
push @frames, { depth => 1, quote => $quote };
$quote = '';
$i++;
next;
}
if (@frames && $quote eq '' && $char eq '(') {
$frames[-1]{depth}++;
next;
}
if (@frames && $quote eq '' && $char eq ')') {
$frames[-1]{depth}--;
if ($frames[-1]{depth} == 0) {
my $frame = pop @frames;
$quote = $frame->{quote};
}
next;
}
next unless $quote eq '' && $char eq '<' && substr($line, $i + 1, 1) eq '<';
if (@frames) {
print STDERR "$path:$line_number\n";
exit 1;
}

my $j = $i + 2;
my $strip_tabs = substr($line, $j, 1) eq '-';
$j++ if $strip_tabs;
$j++ while substr($line, $j, 1) =~ /[ \t]/;
my $delimiter = '';
my $delimiter_quote = '';
for (; $j < $length; $j++) {
my $token = substr($line, $j, 1);
if ($delimiter_quote) {
if ($token eq $delimiter_quote) {
$delimiter_quote = '';
} elsif ($token eq '\\' && $delimiter_quote eq '"') {
$j++;
$delimiter .= substr($line, $j, 1);
} else {
$delimiter .= $token;
}
next;
}
if ($token eq "'" || $token eq '"') {
$delimiter_quote = $token;
next;
}
if ($token eq '\\') {
$j++;
$delimiter .= substr($line, $j, 1);
next;
}
last if $token =~ /[\s;|&()<>]/;
$delimiter .= $token;
}
push @heredocs, { delimiter => $delimiter, strip_tabs => $strip_tabs };
$i = $j - 1;
}
}

exit 0;
PERL
}

test_help_includes_entire_header() {
local help
help=$("$ROOT/bin/fm-brief.sh" --help)
Expand Down Expand Up @@ -114,9 +255,13 @@ test_no_mistakes_dod_wording() {
# shellcheck disable=SC2016 # single quotes are deliberate: the backticks must stay literal
assert_grep '`help`' "$brief" \
"no-mistakes DOD must render literal backticks around help"
assert_no_grep "no-mistakes' own guidance" "$brief" \
"no-mistakes DOD regressed to the apostrophe form that breaks bash -n"
pass "fm-brief.sh: no-mistakes DOD wording avoids the apostrophe regression"
# The apostrophe in "firstmate's authority check" is now structurally safe
# (no `$(...)` wrapper around the heredoc), so it renders verbatim instead of
# being reworded or escaped away. test_no_heredoc_in_command_substitution
# guards the structure that makes it safe.
assert_grep "firstmate's authority check" "$brief" \
"no-mistakes DOD lost the apostrophe prose that the structural fix makes parse-safe"
pass "fm-brief.sh: no-mistakes DOD keeps its apostrophe prose, now parse-safe"
}

test_ship_project_memory_wording() {
Expand Down Expand Up @@ -383,6 +528,7 @@ test_scout_and_secondmate_scaffold() {
}

test_script_parses
test_no_heredoc_in_command_substitution
test_help_includes_entire_header
test_ship_modes_generate_clean_briefs
test_faster_paths_use_configured_authority_without_stacked_review
Expand Down
Loading
Loading