From 77cee4c91dd8246734eb2287c481a3b30387aa57 Mon Sep 17 00:00:00 2001 From: dnth Date: Mon, 5 Oct 2026 17:28:41 +0800 Subject: [PATCH 1/4] feat(bin): pre-fill firstmate verification criterion in firstmate-repo ship briefs Firstmate-repo briefs asked for a local full suite, contradicting .no-mistakes.yaml where ci.yml owns broad regression. The scaffold now appends reserved AC99 with targeted local tests, lint, and PR CI evidence when the repo argument resolves to this repository. --- .../firstmate-coding-guidelines/SKILL.md | 4 +- CONTRIBUTING.md | 2 +- bin/fm-brief.sh | 60 +++++++++++++++-- tests/fm-brief.test.sh | 67 +++++++++++++++++++ 4 files changed, 126 insertions(+), 7 deletions(-) diff --git a/.agents/skills/firstmate-coding-guidelines/SKILL.md b/.agents/skills/firstmate-coding-guidelines/SKILL.md index cb60e12dab4..714fb7292fe 100644 --- a/.agents/skills/firstmate-coding-guidelines/SKILL.md +++ b/.agents/skills/firstmate-coding-guidelines/SKILL.md @@ -69,8 +69,8 @@ A new skill is dead weight if nothing loads it. Every new skill needs its load trigger declared in its description plus an inline `AGENTS.md` pointer in the operating section whose always-loaded rule depends on it, because not every harness surfaces skill descriptions; `agent-skill-trigger-index` holds the complete list. State the trigger as a condition ("load before X", "load on Y wake"), never as a vague pointer. Briefs for tasks that touch firstmate's own tracked material should tell the crewmate to load this skill. -`bin/fm-brief.sh`'s `REPO` argument is a caller-supplied string with no reliable signal that it names firstmate's own repo, unlike a project registered in `data/projects.md`, so there is no clean point inside the scaffold to detect this case automatically. -Firstmate adds this skill's load instruction to firstmate-repo briefs by hand instead. +When `bin/fm-brief.sh`'s `REPO` argument resolves to a checkout of this repository, the scaffold auto-adds only the reserved verification criterion AC99 (the fm-brief.sh header owns that contract). +Firstmate still adds this skill's load instruction to firstmate-repo briefs by hand. `CONTRIBUTING.md`'s "Development" section carries the same instruction as a durable reminder. ## Compatibility and enforcement diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index d7986d741f1..4aa4322f92f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -70,7 +70,7 @@ See the [no-mistakes quick start](https://kunchenguid.github.io/no-mistakes/star Tracked changes to firstmate itself use the risk-based documentation rule above on a feature branch and require an explicit merge approval. Before making any such change, load the agent-only `firstmate-coding-guidelines` skill (`.agents/skills/firstmate-coding-guidelines/SKILL.md`). It has the knowledge-placement rules that keep `AGENTS.md` from regrowing after each diet pass. -There is no reliable way for `bin/fm-brief.sh`'s scaffold to detect that a task's repo is firstmate itself, so firstmate adds this skill's load line to firstmate-repo briefs by hand. +When a task's repo argument resolves to a checkout of this repository, `bin/fm-brief.sh`'s scaffold already appends the reserved verification criterion AC99 (the fm-brief.sh header owns that contract); firstmate still adds this skill's load line to firstmate-repo briefs by hand. A crewmate picking up such a brief should load the skill even if the brief predates this instruction. When supervising live crewmates, keep firstmate's own long validation or build commands in the background so watcher wakes can still be handled. Crewmate validation follows the installed no-mistakes version's SKILL.md and live `axi` help instead of duplicating gate mechanics in firstmate docs. diff --git a/bin/fm-brief.sh b/bin/fm-brief.sh index cad69fe9687..1afc437d6a4 100755 --- a/bin/fm-brief.sh +++ b/bin/fm-brief.sh @@ -31,8 +31,9 @@ # --herdr-lab is mandatory when the task will issue Herdr lifecycle commands. # It adds the hard isolation contract backed by bin/fm-herdr-lab.sh. # The flag must be explicit because {TASK} is filled after scaffolding and the -# caller-supplied repo string cannot reliably identify this repo. Briefs made -# without it carry a loud declaration so an omitted contract cannot be silent. +# caller-supplied repo string cannot be relied on to identify this repo for a +# safety gate. Briefs made without it carry a loud declaration so an omitted +# contract cannot be silent. # For ship tasks, --mode is REQUIRED and shapes the definition of done. Firstmate # resolves it per task at intake (AGENTS.md section 7); data/projects.md holds the # captain's standing posture as context, and this script never reads it: @@ -50,6 +51,13 @@ # "# Acceptance criteria" section and creates the append-only evidence ledger at # data//evidence.jsonl. bin/fm-receipt-check.sh owns the section parser, # evidence gate, conservative binary risk plan, and validation timing. +# When the repo argument resolves to a checkout of the same git repository as +# this code root (any worktree of it counts), the ship scaffold also appends the +# reserved criterion AC99 as the section's last line, matching .no-mistakes.yaml's +# test policy: targeted local tests plus lint, the PR's GitHub CI suite owns +# broad regression (local-only wording drops the CI clause), and no local +# full-suite run is required. Other repos get no extra criterion; the reserved +# high id keeps task criteria AC1..AC98 collision-free. # Ship briefs begin with a worktree-isolation assertion before the branch step. # --mode is refused on scout and secondmate scaffolds: a scout's deliverable is a # report rather than a merge, and a charter is not a delivery contract. @@ -436,6 +444,45 @@ fi REPO=${POS[1]} +# True when the ship task's repo argument resolves to a checkout (main checkout +# or any worktree) of the same git repository as this code root. This is a +# best-effort convenience for pre-filling the AC99 verification criterion, not +# a safety gate: a repo string that does not resolve to a directory - the +# common bare project-name case - simply gets the plain scaffold. +repo_is_firstmate_code_root() { + local dir=$1 dir_common root_common dir_abs root_abs + case "$dir" in + projects/*) dir="$FM_HOME/projects/${dir#projects/}" ;; + esac + [ -d "$dir" ] || return 1 + dir_common=$(cd "$dir" 2>/dev/null && cd "$(git rev-parse --git-common-dir 2>/dev/null)" 2>/dev/null && pwd -P) || dir_common= + root_common=$(cd "$FM_ROOT" 2>/dev/null && cd "$(git rev-parse --git-common-dir 2>/dev/null)" 2>/dev/null && pwd -P) || root_common= + if [ -n "$dir_common" ] && [ -n "$root_common" ]; then + [ "$dir_common" = "$root_common" ] + return + fi + dir_abs=$(cd "$dir" 2>/dev/null && pwd -P) || return 1 + root_abs=$(cd "$FM_ROOT" 2>/dev/null && pwd -P) || return 1 + [ "$dir_abs" = "$root_abs" ] +} + +# Reserved acceptance criterion, appended last so task criteria AC1..AC98 never +# collide. The wording follows .no-mistakes.yaml: targeted local verification, +# CI owns broad regression, no local full-suite run. +FIRSTMATE_VERIFICATION_AC= +if repo_is_firstmate_code_root "$REPO"; then + case "$MODE" in + local-only) + # shellcheck disable=SC2016 # single quotes are deliberate: the backticks are literal brief text + FIRSTMATE_VERIFICATION_AC='- AC99: changed tests green via `bin/fm-test-run.sh --changed` and `FM_LINT_JOBS=1 bin/fm-lint.sh` clean, recorded as an evidence line with the branch head before reporting ready in branch; no local full-suite run is required.' + ;; + *) + # shellcheck disable=SC2016 # single quotes are deliberate: the backticks are literal brief text + FIRSTMATE_VERIFICATION_AC='- AC99: changed tests green via `bin/fm-test-run.sh --changed`, `FM_LINT_JOBS=1 bin/fm-lint.sh` clean, and the PR'"'"'s full GitHub CI suite green, recorded as an evidence line with the CI run URL and head before reporting PR-ready; no local full-suite run is required because `.github/workflows/ci.yml` owns broad regression.' + ;; + esac +fi + if [ "$HERDR_LAB" -eq 1 ]; then HERDR_LAB_HELPER=$(shell_quote "$FM_ROOT/bin/fm-herdr-lab.sh") # shellcheck disable=SC2016 # single quotes are deliberate: these lines are literal brief text whose backtick-wrapped $(...) and "$HERDR_LAB_SESSION" snippets must reach the reading agent verbatim, not expand at scaffold time; only the '"$VAR"' break-outs interpolate. @@ -577,7 +624,8 @@ ${ORCHESTRATION_FRONTMATTER:+$ORCHESTRATION_FRONTMATTER}You are a crewmate: an a {TASK} # Acceptance criteria -- AC1: {ACCEPTANCE CRITERION} +- AC1: {ACCEPTANCE CRITERION}${FIRSTMATE_VERIFICATION_AC:+ +$FIRSTMATE_VERIFICATION_AC} ${ORCHESTRATION_SECTION:+$ORCHESTRATION_SECTION}$HERDR_SECTION @@ -642,4 +690,8 @@ if ! FM_HOME="$FM_HOME" FM_DATA_OVERRIDE="$DATA" FM_STATE_OVERRIDE="$STATE" \ exit 1 fi BRIEF_COMMITTED=1 -echo "scaffolded: $BRIEF (ship, mode=$MODE; replace {TASK} and every {ACCEPTANCE CRITERION})" +if [ -n "$FIRSTMATE_VERIFICATION_AC" ]; then + echo "scaffolded: $BRIEF (ship, mode=$MODE; replace {TASK} and every {ACCEPTANCE CRITERION}; AC99 is pre-filled with the firstmate verification criterion and must be kept)" +else + echo "scaffolded: $BRIEF (ship, mode=$MODE; replace {TASK} and every {ACCEPTANCE CRITERION})" +fi diff --git a/tests/fm-brief.test.sh b/tests/fm-brief.test.sh index 02a0a957618..9128a6f2a63 100755 --- a/tests/fm-brief.test.sh +++ b/tests/fm-brief.test.sh @@ -1014,6 +1014,72 @@ test_completion_boundary_contract_in_briefs() { pass "fm-brief.sh: ship and scout briefs carry the bounded completion contract" } +# When the ship task's repo argument resolves to a checkout of the same git +# repository as this code root, the scaffold appends the reserved AC99 +# verification criterion (targeted local tests plus lint, CI owns broad +# regression - per .no-mistakes.yaml) as the last criterion; every other repo's +# scaffold stays free of it. fm-receipt-check.sh must parse the result. +test_firstmate_repo_ship_brief_prefills_verification_criterion() { + local home brief filled criteria out status other_repo + home="$TMP_ROOT/firstmate-ac99-home" + mkdir -p "$home/data" + + out=$(FM_HOME="$home" "$ROOT/bin/fm-brief.sh" brief-fm-ac99-nm "$ROOT" --mode no-mistakes 2>&1); status=$? + expect_code 0 "$status" "firstmate-repo no-mistakes brief should scaffold" + assert_contains "$out" "AC99 is pre-filled" \ + "firstmate-repo scaffold did not announce the pre-filled AC99" + assert_contains "$out" "replace {TASK} and every {ACCEPTANCE CRITERION}" \ + "firstmate-repo scaffold dropped the placeholder-replacement instruction" + brief="$home/data/brief-fm-ac99-nm/brief.md" + # shellcheck disable=SC2016 # Literal backticks must remain unexpanded. + assert_grep '- AC99: changed tests green via `bin/fm-test-run.sh --changed`' "$brief" \ + "firstmate-repo brief missing the AC99 verification criterion" + assert_grep 'full GitHub CI suite green' "$brief" \ + "firstmate-repo brief AC99 did not defer broad regression to CI" + [ "$(grep -oE '^- AC[0-9]+' "$brief" | tail -1)" = "- AC99" ] \ + || fail "AC99 is not the last acceptance criterion in the firstmate-repo brief" + [ "$(grep -n -- '- AC1:' "$brief" | head -1 | cut -d: -f1)" -lt \ + "$(grep -n -- '- AC99:' "$brief" | cut -d: -f1)" ] \ + || fail "AC99 did not appear after the AC1 scaffold line" + + filled="$TMP_ROOT/brief-fm-ac99-nm-filled.md" + sed 's/{ACCEPTANCE CRITERION}/the change works as specified/' "$brief" > "$filled" + "$ROOT/bin/fm-receipt-check.sh" --parse-criteria "$filled" --require AC99 >/dev/null 2>&1 \ + || fail "fm-receipt-check --require AC99 rejected the filled firstmate brief" + criteria=$("$ROOT/bin/fm-receipt-check.sh" --parse-criteria "$filled") \ + || fail "fm-receipt-check could not parse the filled firstmate brief" + printf '%s\n' "$criteria" | cut -f1 | grep -Fx AC1 >/dev/null \ + || fail "parsed criteria lost AC1" + printf '%s\n' "$criteria" | cut -f1 | grep -Fx AC99 >/dev/null \ + || fail "parsed criteria lost AC99" + + out=$(FM_HOME="$home" "$ROOT/bin/fm-brief.sh" brief-fm-ac99-lo "$ROOT" --mode local-only 2>&1); status=$? + expect_code 0 "$status" "firstmate-repo local-only brief should scaffold" + brief="$home/data/brief-fm-ac99-lo/brief.md" + # shellcheck disable=SC2016 # Literal backticks must remain unexpanded. + assert_grep '- AC99: changed tests green via `bin/fm-test-run.sh --changed`' "$brief" \ + "firstmate-repo local-only brief missing the AC99 verification criterion" + assert_grep 'ready in branch' "$brief" \ + "local-only AC99 did not bind evidence to the branch-ready report" + assert_no_grep 'GitHub CI' "$brief" \ + "local-only AC99 referenced CI that a local-only delivery never runs" + + FM_HOME="$home" "$ROOT/bin/fm-brief.sh" brief-fm-ac99-bare some-proj --mode no-mistakes >/dev/null 2>&1 \ + || fail "bare-name ship brief did not scaffold" + assert_no_grep 'AC99' "$home/data/brief-fm-ac99-bare/brief.md" \ + "bare project name gained the firstmate-only AC99 criterion" + + other_repo="$TMP_ROOT/unrelated-repo" + mkdir -p "$other_repo" + git -C "$other_repo" init -q + FM_HOME="$home" "$ROOT/bin/fm-brief.sh" brief-fm-ac99-other "$other_repo" --mode no-mistakes >/dev/null 2>&1 \ + || fail "foreign-repo ship brief did not scaffold" + assert_no_grep 'AC99' "$home/data/brief-fm-ac99-other/brief.md" \ + "a checkout of a different repository gained the firstmate-only AC99 criterion" + + pass "fm-brief.sh: firstmate-repo ship briefs pre-fill AC99, other repos stay unchanged" +} + test_script_parses test_no_heredoc_in_command_substitution test_help_includes_entire_header @@ -1038,5 +1104,6 @@ test_scouting_delegation_section_in_ship_and_scout test_pause_verb_override_renders_all_brief_scaffolds test_scout_and_secondmate_load_captain_hold_policy test_scout_and_secondmate_scaffold +test_firstmate_repo_ship_brief_prefills_verification_criterion test_concurrent_ship_scaffold_has_one_owner test_ship_scaffold_rejects_destination_swap_and_retries_cleanly From 6e5341eb05c2965c59d30c7c2e1a7d6e171631fd Mon Sep 17 00:00:00 2001 From: dnth Date: Mon, 5 Oct 2026 19:20:16 +0800 Subject: [PATCH 2/4] no-mistakes(review): Fix pre-validation criteria and require Git repository identity --- CONTRIBUTING.md | 2 +- bin/fm-brief.sh | 21 +++++------- tests/fm-brief.test.sh | 73 +++++++++++++++++++++++++----------------- 3 files changed, 53 insertions(+), 43 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 4aa4322f92f..c0b47816fb8 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -70,7 +70,7 @@ See the [no-mistakes quick start](https://kunchenguid.github.io/no-mistakes/star Tracked changes to firstmate itself use the risk-based documentation rule above on a feature branch and require an explicit merge approval. Before making any such change, load the agent-only `firstmate-coding-guidelines` skill (`.agents/skills/firstmate-coding-guidelines/SKILL.md`). It has the knowledge-placement rules that keep `AGENTS.md` from regrowing after each diet pass. -When a task's repo argument resolves to a checkout of this repository, `bin/fm-brief.sh`'s scaffold already appends the reserved verification criterion AC99 (the fm-brief.sh header owns that contract); firstmate still adds this skill's load line to firstmate-repo briefs by hand. +When a task's repo argument resolves to a checkout of this repository, `bin/fm-brief.sh`'s scaffold already appends the reserved pre-validation verification criterion AC99 (the fm-brief.sh header owns that contract); firstmate still adds this skill's load line to firstmate-repo briefs by hand. A crewmate picking up such a brief should load the skill even if the brief predates this instruction. When supervising live crewmates, keep firstmate's own long validation or build commands in the background so watcher wakes can still be handled. Crewmate validation follows the installed no-mistakes version's SKILL.md and live `axi` help instead of duplicating gate mechanics in firstmate docs. diff --git a/bin/fm-brief.sh b/bin/fm-brief.sh index 1afc437d6a4..a7d27fd01c5 100755 --- a/bin/fm-brief.sh +++ b/bin/fm-brief.sh @@ -54,8 +54,9 @@ # When the repo argument resolves to a checkout of the same git repository as # this code root (any worktree of it counts), the ship scaffold also appends the # reserved criterion AC99 as the section's last line, matching .no-mistakes.yaml's -# test policy: targeted local tests plus lint, the PR's GitHub CI suite owns -# broad regression (local-only wording drops the CI clause), and no local +# test policy: evidence of targeted local tests plus lint is sufficient before +# validation planning; the existing checks-green PR-ready gate enforces GitHub +# CI broad regression (local-only wording drops the CI clause). No local # full-suite run is required. Other repos get no extra criterion; the reserved # high id keeps task criteria AC1..AC98 collision-free. # Ship briefs begin with a worktree-isolation assertion before the branch step. @@ -450,20 +451,14 @@ REPO=${POS[1]} # a safety gate: a repo string that does not resolve to a directory - the # common bare project-name case - simply gets the plain scaffold. repo_is_firstmate_code_root() { - local dir=$1 dir_common root_common dir_abs root_abs + local dir=$1 dir_common root_common case "$dir" in projects/*) dir="$FM_HOME/projects/${dir#projects/}" ;; esac [ -d "$dir" ] || return 1 - dir_common=$(cd "$dir" 2>/dev/null && cd "$(git rev-parse --git-common-dir 2>/dev/null)" 2>/dev/null && pwd -P) || dir_common= - root_common=$(cd "$FM_ROOT" 2>/dev/null && cd "$(git rev-parse --git-common-dir 2>/dev/null)" 2>/dev/null && pwd -P) || root_common= - if [ -n "$dir_common" ] && [ -n "$root_common" ]; then - [ "$dir_common" = "$root_common" ] - return - fi - dir_abs=$(cd "$dir" 2>/dev/null && pwd -P) || return 1 - root_abs=$(cd "$FM_ROOT" 2>/dev/null && pwd -P) || return 1 - [ "$dir_abs" = "$root_abs" ] + dir_common=$(cd "$dir" 2>/dev/null && common_dir=$(git rev-parse --git-common-dir 2>/dev/null) && cd "$common_dir" 2>/dev/null && pwd -P) || return 1 + root_common=$(cd "$FM_ROOT" 2>/dev/null && common_dir=$(git rev-parse --git-common-dir 2>/dev/null) && cd "$common_dir" 2>/dev/null && pwd -P) || return 1 + [ -n "$dir_common" ] && [ -n "$root_common" ] && [ "$dir_common" = "$root_common" ] } # Reserved acceptance criterion, appended last so task criteria AC1..AC98 never @@ -478,7 +473,7 @@ if repo_is_firstmate_code_root "$REPO"; then ;; *) # shellcheck disable=SC2016 # single quotes are deliberate: the backticks are literal brief text - FIRSTMATE_VERIFICATION_AC='- AC99: changed tests green via `bin/fm-test-run.sh --changed`, `FM_LINT_JOBS=1 bin/fm-lint.sh` clean, and the PR'"'"'s full GitHub CI suite green, recorded as an evidence line with the CI run URL and head before reporting PR-ready; no local full-suite run is required because `.github/workflows/ci.yml` owns broad regression.' + FIRSTMATE_VERIFICATION_AC='- AC99: changed tests green via `bin/fm-test-run.sh --changed` and `FM_LINT_JOBS=1 bin/fm-lint.sh` clean, recorded as an evidence line with the branch head before validation planning; no local full-suite run is required because the PR'"'"'s GitHub CI (`.github/workflows/ci.yml`) owns broad regression, enforced by the existing checks-green PR-ready gate.' ;; esac fi diff --git a/tests/fm-brief.test.sh b/tests/fm-brief.test.sh index 9128a6f2a63..3443a6b0122 100755 --- a/tests/fm-brief.test.sh +++ b/tests/fm-brief.test.sh @@ -1020,38 +1020,44 @@ test_completion_boundary_contract_in_briefs() { # regression - per .no-mistakes.yaml) as the last criterion; every other repo's # scaffold stays free of it. fm-receipt-check.sh must parse the result. test_firstmate_repo_ship_brief_prefills_verification_criterion() { - local home brief filled criteria out status other_repo + local home brief filled criteria out status other_repo mode non_git_root home="$TMP_ROOT/firstmate-ac99-home" mkdir -p "$home/data" - out=$(FM_HOME="$home" "$ROOT/bin/fm-brief.sh" brief-fm-ac99-nm "$ROOT" --mode no-mistakes 2>&1); status=$? - expect_code 0 "$status" "firstmate-repo no-mistakes brief should scaffold" - assert_contains "$out" "AC99 is pre-filled" \ - "firstmate-repo scaffold did not announce the pre-filled AC99" - assert_contains "$out" "replace {TASK} and every {ACCEPTANCE CRITERION}" \ - "firstmate-repo scaffold dropped the placeholder-replacement instruction" - brief="$home/data/brief-fm-ac99-nm/brief.md" - # shellcheck disable=SC2016 # Literal backticks must remain unexpanded. - assert_grep '- AC99: changed tests green via `bin/fm-test-run.sh --changed`' "$brief" \ - "firstmate-repo brief missing the AC99 verification criterion" - assert_grep 'full GitHub CI suite green' "$brief" \ - "firstmate-repo brief AC99 did not defer broad regression to CI" - [ "$(grep -oE '^- AC[0-9]+' "$brief" | tail -1)" = "- AC99" ] \ - || fail "AC99 is not the last acceptance criterion in the firstmate-repo brief" - [ "$(grep -n -- '- AC1:' "$brief" | head -1 | cut -d: -f1)" -lt \ - "$(grep -n -- '- AC99:' "$brief" | cut -d: -f1)" ] \ - || fail "AC99 did not appear after the AC1 scaffold line" - - filled="$TMP_ROOT/brief-fm-ac99-nm-filled.md" - sed 's/{ACCEPTANCE CRITERION}/the change works as specified/' "$brief" > "$filled" - "$ROOT/bin/fm-receipt-check.sh" --parse-criteria "$filled" --require AC99 >/dev/null 2>&1 \ - || fail "fm-receipt-check --require AC99 rejected the filled firstmate brief" - criteria=$("$ROOT/bin/fm-receipt-check.sh" --parse-criteria "$filled") \ - || fail "fm-receipt-check could not parse the filled firstmate brief" - printf '%s\n' "$criteria" | cut -f1 | grep -Fx AC1 >/dev/null \ - || fail "parsed criteria lost AC1" - printf '%s\n' "$criteria" | cut -f1 | grep -Fx AC99 >/dev/null \ - || fail "parsed criteria lost AC99" + for mode in no-mistakes direct-PR; do + out=$(FM_HOME="$home" "$ROOT/bin/fm-brief.sh" "brief-fm-ac99-$mode" "$ROOT" --mode "$mode" 2>&1); status=$? + expect_code 0 "$status" "firstmate-repo $mode brief should scaffold" + assert_contains "$out" "AC99 is pre-filled" \ + "firstmate-repo scaffold did not announce the pre-filled AC99" + assert_contains "$out" "replace {TASK} and every {ACCEPTANCE CRITERION}" \ + "firstmate-repo scaffold dropped the placeholder-replacement instruction" + brief="$home/data/brief-fm-ac99-$mode/brief.md" + # shellcheck disable=SC2016 # Literal backticks must remain unexpanded. + assert_grep '- AC99: changed tests green via `bin/fm-test-run.sh --changed` and `FM_LINT_JOBS=1 bin/fm-lint.sh` clean, recorded as an evidence line with the branch head before validation planning;' "$brief" \ + "firstmate-repo AC99 cannot be evidenced before PR creation" + assert_grep 'owns broad regression, enforced by the existing checks-green PR-ready gate' "$brief" \ + "firstmate-repo AC99 did not defer broad regression to the PR-ready gate" + assert_grep 'no local full-suite run is required' "$brief" \ + "firstmate-repo AC99 did not exclude local full-suite runs" + assert_no_grep 'CI run URL' "$brief" \ + "firstmate-repo AC99 still requires a pre-validation CI receipt" + [ "$(grep -oE '^- AC[0-9]+' "$brief" | tail -1)" = "- AC99" ] \ + || fail "AC99 is not the last acceptance criterion in the firstmate-repo brief" + [ "$(grep -n -- '- AC1:' "$brief" | head -1 | cut -d: -f1)" -lt \ + "$(grep -n -- '- AC99:' "$brief" | cut -d: -f1)" ] \ + || fail "AC99 did not appear after the AC1 scaffold line" + + filled="$TMP_ROOT/brief-fm-ac99-$mode-filled.md" + sed 's/{ACCEPTANCE CRITERION}/the change works as specified/' "$brief" > "$filled" + "$ROOT/bin/fm-receipt-check.sh" --parse-criteria "$filled" --require AC99 >/dev/null 2>&1 \ + || fail "fm-receipt-check --require AC99 rejected the filled firstmate brief" + criteria=$("$ROOT/bin/fm-receipt-check.sh" --parse-criteria "$filled") \ + || fail "fm-receipt-check could not parse the filled firstmate brief" + printf '%s\n' "$criteria" | cut -f1 | grep -Fx AC1 >/dev/null \ + || fail "parsed criteria lost AC1" + printf '%s\n' "$criteria" | cut -f1 | grep -Fx AC99 >/dev/null \ + || fail "parsed criteria lost AC99" + done out=$(FM_HOME="$home" "$ROOT/bin/fm-brief.sh" brief-fm-ac99-lo "$ROOT" --mode local-only 2>&1); status=$? expect_code 0 "$status" "firstmate-repo local-only brief should scaffold" @@ -1077,6 +1083,15 @@ test_firstmate_repo_ship_brief_prefills_verification_criterion() { assert_no_grep 'AC99' "$home/data/brief-fm-ac99-other/brief.md" \ "a checkout of a different repository gained the firstmate-only AC99 criterion" + non_git_root="$TMP_ROOT/non-git-code-root" + mkdir -p "$non_git_root" + cp -R "$ROOT/bin" "$non_git_root/bin" + GIT_CEILING_DIRECTORIES="$TMP_ROOT" FM_HOME="$home" FM_ROOT_OVERRIDE="$non_git_root" \ + "$non_git_root/bin/fm-brief.sh" brief-fm-ac99-non-git "$non_git_root" --mode no-mistakes >/dev/null 2>&1 \ + || fail "non-git code-root ship brief did not scaffold" + assert_no_grep 'AC99' "$home/data/brief-fm-ac99-non-git/brief.md" \ + "physical equality without Git identity gained the firstmate-only AC99 criterion" + pass "fm-brief.sh: firstmate-repo ship briefs pre-fill AC99, other repos stay unchanged" } From e7152d09da813141b61cce01f96b68db0ecdf4ee Mon Sep 17 00:00:00 2001 From: dnth Date: Mon, 5 Oct 2026 19:25:09 +0800 Subject: [PATCH 3/4] no-mistakes(review): Remove inaccurate CI enforcement claim from Firstmate briefs --- bin/fm-brief.sh | 6 +++--- tests/fm-brief.test.sh | 6 ++++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/bin/fm-brief.sh b/bin/fm-brief.sh index a7d27fd01c5..83f36cbcbe5 100755 --- a/bin/fm-brief.sh +++ b/bin/fm-brief.sh @@ -55,8 +55,8 @@ # this code root (any worktree of it counts), the ship scaffold also appends the # reserved criterion AC99 as the section's last line, matching .no-mistakes.yaml's # test policy: evidence of targeted local tests plus lint is sufficient before -# validation planning; the existing checks-green PR-ready gate enforces GitHub -# CI broad regression (local-only wording drops the CI clause). No local +# validation planning; broad regression is owned by the PR GitHub CI per +# .no-mistakes.yaml (local-only wording drops the CI clause). No local # full-suite run is required. Other repos get no extra criterion; the reserved # high id keeps task criteria AC1..AC98 collision-free. # Ship briefs begin with a worktree-isolation assertion before the branch step. @@ -473,7 +473,7 @@ if repo_is_firstmate_code_root "$REPO"; then ;; *) # shellcheck disable=SC2016 # single quotes are deliberate: the backticks are literal brief text - FIRSTMATE_VERIFICATION_AC='- AC99: changed tests green via `bin/fm-test-run.sh --changed` and `FM_LINT_JOBS=1 bin/fm-lint.sh` clean, recorded as an evidence line with the branch head before validation planning; no local full-suite run is required because the PR'"'"'s GitHub CI (`.github/workflows/ci.yml`) owns broad regression, enforced by the existing checks-green PR-ready gate.' + FIRSTMATE_VERIFICATION_AC='- AC99: changed tests green via `bin/fm-test-run.sh --changed` and `FM_LINT_JOBS=1 bin/fm-lint.sh` clean, recorded as an evidence line with the branch head before validation planning; no local full-suite run is required; broad regression is owned by the PR GitHub CI per .no-mistakes.yaml.' ;; esac fi diff --git a/tests/fm-brief.test.sh b/tests/fm-brief.test.sh index 3443a6b0122..3332b0eda73 100755 --- a/tests/fm-brief.test.sh +++ b/tests/fm-brief.test.sh @@ -1035,8 +1035,10 @@ test_firstmate_repo_ship_brief_prefills_verification_criterion() { # shellcheck disable=SC2016 # Literal backticks must remain unexpanded. assert_grep '- AC99: changed tests green via `bin/fm-test-run.sh --changed` and `FM_LINT_JOBS=1 bin/fm-lint.sh` clean, recorded as an evidence line with the branch head before validation planning;' "$brief" \ "firstmate-repo AC99 cannot be evidenced before PR creation" - assert_grep 'owns broad regression, enforced by the existing checks-green PR-ready gate' "$brief" \ - "firstmate-repo AC99 did not defer broad regression to the PR-ready gate" + assert_grep 'broad regression is owned by the PR GitHub CI per .no-mistakes.yaml' "$brief" \ + "firstmate-repo AC99 did not identify CI as the broad regression owner" + assert_no_grep 'checks-green PR-ready gate' "$brief" \ + "firstmate-repo AC99 claimed CI enforcement across all PR paths" assert_grep 'no local full-suite run is required' "$brief" \ "firstmate-repo AC99 did not exclude local full-suite runs" assert_no_grep 'CI run URL' "$brief" \ From 06c5b2414ac16f5263c09ba86e4918be6acbeb63 Mon Sep 17 00:00:00 2001 From: dnth Date: Mon, 5 Oct 2026 19:32:16 +0800 Subject: [PATCH 4/4] no-mistakes(document): Clarify verification scaffold contract and authoritative documentation pointers --- .../firstmate-coding-guidelines/SKILL.md | 2 +- CONTRIBUTING.md | 2 +- bin/fm-brief.sh | 19 +++++++++++-------- 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/.agents/skills/firstmate-coding-guidelines/SKILL.md b/.agents/skills/firstmate-coding-guidelines/SKILL.md index 714fb7292fe..22f495f967f 100644 --- a/.agents/skills/firstmate-coding-guidelines/SKILL.md +++ b/.agents/skills/firstmate-coding-guidelines/SKILL.md @@ -69,7 +69,7 @@ A new skill is dead weight if nothing loads it. Every new skill needs its load trigger declared in its description plus an inline `AGENTS.md` pointer in the operating section whose always-loaded rule depends on it, because not every harness surfaces skill descriptions; `agent-skill-trigger-index` holds the complete list. State the trigger as a condition ("load before X", "load on Y wake"), never as a vague pointer. Briefs for tasks that touch firstmate's own tracked material should tell the crewmate to load this skill. -When `bin/fm-brief.sh`'s `REPO` argument resolves to a checkout of this repository, the scaffold auto-adds only the reserved verification criterion AC99 (the fm-brief.sh header owns that contract). +The [`bin/fm-brief.sh` header](../../../bin/fm-brief.sh) owns automatic verification criteria. Firstmate still adds this skill's load instruction to firstmate-repo briefs by hand. `CONTRIBUTING.md`'s "Development" section carries the same instruction as a durable reminder. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index c0b47816fb8..3574ce93657 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -70,7 +70,7 @@ See the [no-mistakes quick start](https://kunchenguid.github.io/no-mistakes/star Tracked changes to firstmate itself use the risk-based documentation rule above on a feature branch and require an explicit merge approval. Before making any such change, load the agent-only `firstmate-coding-guidelines` skill (`.agents/skills/firstmate-coding-guidelines/SKILL.md`). It has the knowledge-placement rules that keep `AGENTS.md` from regrowing after each diet pass. -When a task's repo argument resolves to a checkout of this repository, `bin/fm-brief.sh`'s scaffold already appends the reserved pre-validation verification criterion AC99 (the fm-brief.sh header owns that contract); firstmate still adds this skill's load line to firstmate-repo briefs by hand. +The [`bin/fm-brief.sh` header](bin/fm-brief.sh) owns automatic verification criteria; firstmate still adds this skill's load line to firstmate-repo briefs by hand. A crewmate picking up such a brief should load the skill even if the brief predates this instruction. When supervising live crewmates, keep firstmate's own long validation or build commands in the background so watcher wakes can still be handled. Crewmate validation follows the installed no-mistakes version's SKILL.md and live `axi` help instead of duplicating gate mechanics in firstmate docs. diff --git a/bin/fm-brief.sh b/bin/fm-brief.sh index 83f36cbcbe5..cedd3bf17dc 100755 --- a/bin/fm-brief.sh +++ b/bin/fm-brief.sh @@ -51,14 +51,17 @@ # "# Acceptance criteria" section and creates the append-only evidence ledger at # data//evidence.jsonl. bin/fm-receipt-check.sh owns the section parser, # evidence gate, conservative binary risk plan, and validation timing. -# When the repo argument resolves to a checkout of the same git repository as -# this code root (any worktree of it counts), the ship scaffold also appends the -# reserved criterion AC99 as the section's last line, matching .no-mistakes.yaml's -# test policy: evidence of targeted local tests plus lint is sufficient before -# validation planning; broad regression is owned by the PR GitHub CI per -# .no-mistakes.yaml (local-only wording drops the CI clause). No local -# full-suite run is required. Other repos get no extra criterion; the reserved -# high id keeps task criteria AC1..AC98 collision-free. +# When the repo argument resolves to a directory whose git common dir equals +# this code root's git common dir (any worktree of it counts), the ship scaffold +# appends reserved criterion AC99 as the section's last line; keep it and use +# AC1..AC98 for task criteria. projects/ resolves under FM_HOME; unresolved +# names, non-git directories, and other repos get no extra criterion. +# AC99 requires bin/fm-test-run.sh --changed green and +# FM_LINT_JOBS=1 bin/fm-lint.sh clean, recorded as an evidence line with the branch +# head before validation planning. No local full-suite run is required; broad +# regression is owned by the PR GitHub CI per .no-mistakes.yaml. +# For local-only, AC99 drops the CI clause and binds the branch-head evidence to +# reporting "ready in branch" instead of validation planning. # Ship briefs begin with a worktree-isolation assertion before the branch step. # --mode is refused on scout and secondmate scaffolds: a scout's deliverable is a # report rather than a merge, and a charter is not a delivery contract.