diff --git a/bin/fm-brief.sh b/bin/fm-brief.sh index b5fba5b5a72..243a3083074 100755 --- a/bin/fm-brief.sh +++ b/bin/fm-brief.sh @@ -395,6 +395,14 @@ case "$MODE" in esac DOD=$(fm_dod_block "$MODE" "$ID") || exit 1 +# A mode=no-mistakes ship task's status report routes through the guarded +# fm-status-append.sh helper instead of a bare echo: it refuses a `done:` line +# that names no PR URL, printing the exact next step instead, so "committed, +# gates green" mechanically cannot pass as done. Other modes have no pipeline +# step to skip, so they keep the bare echo. +REPORT_CMD=$(fm_status_report_line "$MODE" "$FM_ROOT" "$STATUS_FILE") +REPORT_GUARD_NOTE=$(fm_status_report_guard_note "$MODE" "$STATUS_FILE") + cat > "$BRIEF" <> $STATUS_FILE\` + \`$REPORT_CMD\` States: working, needs-decision, blocked, $PAUSED_VERB, done, failed. Each append wakes firstmate, so report sparingly: only phase changes a supervisor would act on (setup done, bug reproduced, fix implemented, validation passed) and the @@ -428,7 +436,7 @@ $RULE1 Use \`$PAUSED_VERB: {why}\` - distinct from \`blocked:\` - ONLY when you are deliberately idling on a known external wait you expect to clear on its own (an upstream release, a rate-limit reset, a scheduled window): firstmate then leaves your idle pane alone and rechecks it on a long - cadence instead of treating it as a possible wedge. Use \`blocked:\` when you are stuck and need help. + cadence instead of treating it as a possible wedge. Use \`blocked:\` when you are stuck and need help.$REPORT_GUARD_NOTE 5. If you hit the same obstacle twice, append \`blocked: {why}\` and stop; firstmate will help. 6. If a decision belongs above the implementation worker (product choices, destructive actions, ask-user findings), append \`needs-decision: {summary of options}\` and stop. Firstmate will reply with the decision. diff --git a/bin/fm-crew-state.sh b/bin/fm-crew-state.sh index e7ec1bf9e43..96098a4a9d3 100755 --- a/bin/fm-crew-state.sh +++ b/bin/fm-crew-state.sh @@ -48,7 +48,10 @@ # 4. No run for this crew (pre-validation, or kind=scout): fall back to the # recorded backend's pane busy state, then the status log's last line only # when its verb maps to a recognized run-state. Decision-only events such as -# `resolved` never become current state or detail. +# `resolved` never become current state or detail. A mode=no-mistakes ship +# task's own `done:` with no run ever attributed and no validated pr= +# recorded in state/.meta is reported as parked, not done: it declared +# done without ever starting the pipeline it still owes. # 5. Missing meta or torn-down worktree: report unknown · none. If no run is # attributed to this crew, a dead endpoint also reports unknown · none rather # than trusting a stale status log. @@ -108,8 +111,18 @@ WT=$(meta_value worktree) KIND=$(meta_value kind) HARNESS=$(meta_value harness) REMOTE_HOST=$(meta_value remote_host) +MODE=$(meta_value mode) [ -n "$KIND" ] || KIND=ship +# 0 if this is a mode=no-mistakes ship task that has declared done without the +# pipeline ever recording a validated PR (bin/fm-pr-check.sh's pr= line). Such a +# task is NOT done - a worker that stops at "committed, gates green" has not run +# /no-mistakes yet - so callers use this to keep that status-log verb from being +# read as the real terminal state. +crew_declared_done_without_pr() { + [ "$KIND" = ship ] && [ "$MODE" = no-mistakes ] && ! grep -q '^pr=' "$META" 2>/dev/null +} + # A torn-down (or never-created) worktree has no current state to read. A # remote secondmate's recorded worktree is a path on ITS host, so the local # probe proves nothing for it - the remote arm below reads the true source. @@ -691,6 +704,14 @@ fi # `unknown` verdict as the "not a state" test needs no second verb list here. if [ -n "$LOG_VERB" ]; then LOG_STATE=$(map_log_state "$LOG_LINE") + # A no-mistakes ship task's own `done:` line means "implementation complete", + # never "shipped" - no-mistakes still owns review, fixes, push, PR, and CI. With + # no run attributed at all (this fallback) and no validated pr= recorded, that + # line is reported as parked (needing firstmate's attention), not done, so a + # crew that stopped short of the pipeline never reads as finished. + if [ "$LOG_STATE" = "done" ] && crew_declared_done_without_pr; then + emit parked status-log "implementation complete, pipeline not started - run /no-mistakes" + fi if [ "$LOG_STATE" != unknown ]; then emit "$LOG_STATE" status-log "$(status_line_note "$LOG_LINE")" fi diff --git a/bin/fm-dod-lib.sh b/bin/fm-dod-lib.sh index 34d1f8ab38d..a7df7e4ba80 100755 --- a/bin/fm-dod-lib.sh +++ b/bin/fm-dod-lib.sh @@ -1,15 +1,25 @@ #!/usr/bin/env bash -# Single owner of a ship task's mode-specific "Definition of done" block. -# Sourced by bin/fm-brief.sh, which renders it into a generated ship brief, and by -# bin/fm-promote.sh, which renders it into the ship instructions a promoted scout +# Single owner of a ship task's mode-specific "Definition of done" block, and of +# the mode-specific status-report command that goes with it. +# Sourced by bin/fm-brief.sh, which renders both into a generated ship brief, and by +# bin/fm-promote.sh, which renders both into the ship instructions a promoted scout # receives. Both paths must hand the worker the same contract: a promoted -# no-mistakes worker that never received the ask-user escalation rule or the -# `--yes` ban is the exact delivery hole this single owner exists to close. +# no-mistakes worker that never received the ask-user escalation rule, the +# `--yes` ban, or the guarded status-report command is the exact delivery hole +# this single owner exists to close. # fm_dod_block prints the block on # stdout with no trailing blank line. The caller validates the mode; an unknown # mode is refused rather than silently rendered as the pipeline contract. # The block opens with the fixed machine-readable "Delivery contract: mode=" # line that bin/fm-spawn.sh checks a ship brief against. +# fm_status_report_line prints the exact +# status-report command a worker on that mode must run: the guarded +# bin/fm-status-append.sh helper for mode=no-mistakes (the only mode with a +# pipeline step to skip), otherwise the bare `echo ... >> status-file` every +# other mode keeps. fm_status_report_guard_note +# prints the accompanying warning against bypassing the helper (empty for +# every mode but no-mistakes), formatted to append inline after a sentence +# (leading newline, no trailing one). # Every heredoc here stays outside a command substitution: `VAR=$(cat < + local mode=$1 fm_root=$2 status_file_q=$3 + case "$mode" in + no-mistakes) + printf '%s %s "{state}: {one short line}"' "$(printf '%q' "$fm_root/bin/fm-status-append.sh")" "$status_file_q" + ;; + *) + printf 'echo "{state}: {one short line}" >> %s' "$status_file_q" + ;; + esac +} + +fm_status_report_guard_note() { # + local mode=$1 status_file_q=$2 + [ "$mode" = no-mistakes ] || return 0 + cat <> $status_file_q\`. +EOF +} diff --git a/bin/fm-promote.sh b/bin/fm-promote.sh index bdc2e1fd327..ae79f531528 100755 --- a/bin/fm-promote.sh +++ b/bin/fm-promote.sh @@ -7,9 +7,12 @@ # delivers them. Those instructions carry the scratch-state inventory, the clean # default-branch base, the fm/ branch, and - rendered from # bin/fm-dod-lib.sh, the single owner an ordinary ship brief also uses - the -# mode-specific Definition of done, so a promoted worker receives exactly the same -# delivery contract as a briefed one, including the no-mistakes mode's ask-user -# escalation rule and --yes ban. +# mode-specific Definition of done and status-report command, so a promoted +# worker receives exactly the same delivery contract as a briefed one, +# including the no-mistakes mode's ask-user escalation rule, --yes ban, and +# guarded bin/fm-status-append.sh status-report command (a scout is never +# mode=no-mistakes, so its own status-report rule is always the bare echo that +# command replaces for a promoted no-mistakes worker). # A scout records no delivery posture, so promotion is where this task's delivery # contract is decided: --mode and --yolo are REQUIRED and written into the meta # alongside the kind= flip. Firstmate resolves both at promotion time, having just @@ -136,6 +139,20 @@ grep -qx 'kind=scout' "$META" || { echo "error: task $ID is not a scout task (ki INSTRUCTIONS="$DATA/$ID/ship-instructions.md" mkdir -p "$DATA/$ID" [ ! -d "$INSTRUCTIONS" ] || { echo "error: ship instructions path is a directory: $INSTRUCTIONS" >&2; exit 1; } + +# A promoted worker's status-report command is rendered from the same single +# owner (bin/fm-dod-lib.sh) an ordinary ship brief uses, not hand-copied from +# the scout brief's bare echo: a scout is never mode=no-mistakes, so its rule 4 +# is always the bare echo, and that command carrying over unchanged into a +# promoted no-mistakes worker's instructions is the exact structural hole the +# guarded helper (bin/fm-status-append.sh) exists to close. +STATUS_FILE_Q=$(printf '%q' "$STATE/$ID.status") +REPORT_CMD=$(fm_status_report_line "$MODE" "$FM_ROOT" "$STATUS_FILE_Q") +REPORT_GUARD_NOTE=$(fm_status_report_guard_note "$MODE" "$STATUS_FILE_Q") +CARRYOVER_RULE="6. These ship instructions supersede the scout delivery rules and report-based Definition of done. Everything else in your original instructions carries over unchanged: the instruction inbox and its acknowledgement; the escalation rules, including ask-user; and every safety rule. +7. Report status by appending one line: + \`$REPORT_CMD\`$REPORT_GUARD_NOTE" + TMP="$DATA/$ID/.ship-instructions.md.${BASHPID:-$$}" { cat <.status +# (AGENTS.md section 3's status protocol). For a mode=no-mistakes ship task, +# done only means the pipeline reported CI green with a PR - no-mistakes still +# owns review, fixes, tests, documentation, push, PR, and CI, so a worker that +# stops at "committed, gates green" without ever starting it is not done. +# Brief wording alone has repeatedly failed to stop that early stop (see git +# history for this file's introducing PR), so this is the mechanical +# backstop: bin/fm-brief.sh's generated no-mistakes ship brief routes its +# status-report command through this helper instead of a bare +# `echo ... >> status-file`. Other delivery modes and kinds keep the bare +# echo, since they have no pipeline step to skip. +# +# The gate cannot key off state/.meta's pr= line: firstmate writes that +# (via bin/fm-pr-check.sh) only AFTER seeing the worker's own done report +# (AGENTS.md section 7), so pr= is never present yet at the moment this exact +# call fires - checking it here would refuse every legitimate final done too. +# Instead this checks the line's own shape: the DOD's only prescribed final +# done text is `done: PR {url} checks green`, so a done: line that names a +# real http(s) URL is accepted, and one that does not (e.g. "committed, gates +# green", or the DOD's earlier "done: {summary}" wording) is refused. +# +# Usage: fm-status-append.sh +# the crew's state/.status path; its sibling +# state/.meta (same basename, .meta instead of .status) +# is read for kind= and mode=. +# the exact line to append, e.g. "done: implemented". +# +# Refuses (exit 1, printing the required next step to stderr instead of +# appending) only a `done:` line on a mode=no-mistakes ship task that names no +# URL. Every other line, mode, and kind appends exactly as a bare echo would - +# this is a drop-in replacement, not a new contract. +set -eu + +[ $# -eq 2 ] || { echo "usage: fm-status-append.sh " >&2; exit 2; } +STATUS_FILE=$1 +LINE=$2 +META="${STATUS_FILE%.status}.meta" + +meta_value() { # + [ -f "$META" ] || return 0 + grep "^$1=" "$META" 2>/dev/null | tail -1 | cut -d= -f2- || true +} + +case "$LINE" in + done:*) + KIND=$(meta_value kind) + [ -n "$KIND" ] || KIND=ship + MODE=$(meta_value mode) + if [ "$KIND" = ship ] && [ "$MODE" = no-mistakes ]; then + case "$LINE" in + *https://*|*http://*) ;; + *) + cat >&2 <<'EOF' +refused: this is a mode=no-mistakes task, so "committed, gates green" is not done. +Run /no-mistakes now and respond to its gates until it reports CI green, then +report done as: done: PR {url} checks green +EOF + exit 1 + ;; + esac + fi + ;; +esac + +mkdir -p "$(dirname "$STATUS_FILE")" 2>/dev/null || true +printf '%s\n' "$LINE" >> "$STATUS_FILE" diff --git a/docs/architecture.md b/docs/architecture.md index 73df89b0992..9d63daa9467 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -284,7 +284,7 @@ The `data/secondmates.md` line contract is owned by the [`secondmate-provisionin Each task's mode and `yolo` merge posture are firstmate's decision at intake. The mode is passed explicitly to `bin/fm-brief.sh`, and both values are passed explicitly to `bin/fm-spawn.sh` and `bin/fm-promote.sh`; each command refuses to guess the values it consumes. A ship brief records its mode as a fixed machine-readable line and the spawn refuses to launch on a different one, so the worker's instructions and the recorded task delivery cannot diverge. -`bin/fm-dod-lib.sh` is the one owner of that mode's definition of done, rendered both into a generated ship brief and into the ship instructions a promoted scout receives, so a promoted worker cannot be handed a weaker contract than a briefed one. +`bin/fm-dod-lib.sh` is the one owner of that mode's definition of done and status-report command, rendered both into a generated ship brief and into the ship instructions a promoted scout receives, so a promoted worker cannot be handed a weaker contract than a briefed one. `data/projects.md` records each project's standing posture and optional `+yolo` merge flag as the captain's default and as context for that decision, including the conditional `no-mistakes-prod-only` policy; a ship spawn that drops below the registered rigor prints a deviation notice and continues. `bin/fm-project-mode.sh` remains the one registry parser for the mechanical consumers that have no task in hand: fleet sync's `local-only` skip and home seeding's refusal and no-mistakes initialization. When a selected delivery path calls for a diff, `bin/fm-review-diff.sh` refreshes the authoritative base and, when task meta records `pr=`, always fetches and compares against `refs/pull//head` by default (recorded `pr_head=` is only an offline fallback) before falling back to the local branch with a warning. diff --git a/docs/scripts.md b/docs/scripts.md index 5bbcebfdf1b..304b2b34b11 100644 --- a/docs/scripts.md +++ b/docs/scripts.md @@ -32,6 +32,7 @@ The shared no-mistakes gate refusal for fleet lifecycle entrypoints is summarize | `fm-decision-hold.sh` | One-release compatibility shim mapping the retired decision commands onto fm-captain-hold.sh | | `fm-brief.sh` | Scaffold ship (explicit `--mode`), scout, secondmate-charter, and Herdr-lab briefs | | `fm-dod-lib.sh` | One owner of the ship task's mode-specific definition of done, rendered by both the brief scaffold and a scout promotion | +| `fm-status-append.sh` | Guarded status-line append a mode=no-mistakes ship brief routes its status report through; refuses a `done:` line that names no PR URL | | `fm-herdr-lab.sh` | Provision and guardedly operate an isolated, never-default Herdr lab session | | `fm-install-herdr.sh` | Install CI's exact-version Herdr pin with official asset URL, SHA-256, and protocol checks | | `fm-install-treehouse.sh`| Install CI's exact-version Treehouse pin for real-Herdr E2E that needs spawn worktrees | diff --git a/tests/fm-bearings-snapshot.test.sh b/tests/fm-bearings-snapshot.test.sh index b27764548ed..26996a8a836 100755 --- a/tests/fm-bearings-snapshot.test.sh +++ b/tests/fm-bearings-snapshot.test.sh @@ -756,7 +756,7 @@ EOF EOF fm_write_meta "$mate/state/done.meta" \ "window=firstmate:fm-done" "worktree=$mate/projects/done" "project=sample" \ - "harness=claude" "kind=ship" "mode=no-mistakes" + "harness=claude" "kind=ship" "mode=no-mistakes" "pr=https://github.com/sample/sample/pull/1" fm_write_meta "$mate/state/failed.meta" \ "window=firstmate:fm-failed" "worktree=$mate/projects/failed" "project=sample" \ "harness=claude" "kind=ship" "mode=no-mistakes" diff --git a/tests/fm-brief.test.sh b/tests/fm-brief.test.sh index 3e5d3064fd5..845fc2d7eb1 100755 --- a/tests/fm-brief.test.sh +++ b/tests/fm-brief.test.sh @@ -256,7 +256,7 @@ test_ship_mode_is_explicit_not_registry() { brief="$home/data/brief-explicit-a5/brief.md" grep -qx "Delivery contract: mode=no-mistakes" "$brief" \ || fail "registered direct-PR posture overrode the explicit --mode" - assert_grep "Firstmate will then instruct you to run /no-mistakes" "$brief" \ + assert_grep "Immediately invoke /no-mistakes yourself" "$brief" \ "explicit no-mistakes brief did not render the pipeline definition of done" # An unregistered project is not a blocker either, because nothing is looked up. @@ -365,6 +365,57 @@ test_no_mistakes_dod_wording() { pass "fm-brief.sh: no-mistakes DOD keeps its apostrophe prose and bans --yes outright" } +# Direct regression for the 08-30/08-31 done-drift pattern: nine ship workers +# in a row reported `done: ... committed, gates green` without ever starting +# /no-mistakes, including three that carried an explicit capitalized +# DEFINITION OF DONE paragraph and still stopped short - so prose alone is not +# enough. A mode=no-mistakes brief must (a) make invoking the pipeline the +# literal next step after the commit rather than something the worker waits +# for firstmate to trigger, and (b) route its status-report command through +# the guarded bin/fm-status-append.sh helper, which mechanically refuses a +# `done:` line until the pipeline has recorded a validated PR. +test_no_mistakes_done_drift_is_structurally_blocked() { + local home id brief + home="$TMP_ROOT/done-drift-home" + mkdir -p "$home/data" + id="brief-done-drift-b1" + FM_HOME="$home" "$ROOT/bin/fm-brief.sh" "$id" some-proj --mode no-mistakes >/dev/null 2>&1 + brief="$home/data/$id/brief.md" + assert_present "$brief" "brief was not scaffolded" + + assert_grep "fm-status-append.sh" "$brief" \ + "no-mistakes brief must route its status report through the guarded helper" + assert_grep "is refused, with the exact next step printed instead" "$brief" \ + "no-mistakes brief must document the mechanical done refusal" + assert_no_grep "Firstmate will then instruct you to run /no-mistakes" "$brief" \ + "no-mistakes brief must not defer starting the pipeline to a later firstmate instruction" + assert_grep "Immediately invoke /no-mistakes yourself" "$brief" \ + "no-mistakes brief must make running the pipeline the literal next step after the commit" + assert_grep '"Committed, gates green" is NOT done.' "$brief" \ + "no-mistakes brief must state outright that committed-with-gates-green is not done" + pass "fm-brief.sh: no-mistakes done is structurally blocked before a validated PR, not left to prose" +} + +# Every other delivery mode has no pipeline step to skip, so its status report +# must stay a bare echo - the guard would be a no-op there but should not add +# an unfamiliar command to a brief that does not need it. +test_non_no_mistakes_modes_keep_bare_status_echo() { + local home mode id brief + home="$TMP_ROOT/bare-echo-home" + mkdir -p "$home/data" + for mode in direct-PR local-only; do + id="brief-bare-echo-$mode" + FM_HOME="$home" "$ROOT/bin/fm-brief.sh" "$id" some-proj --mode "$mode" >/dev/null 2>&1 + brief="$home/data/$id/brief.md" + assert_present "$brief" "$mode brief was not scaffolded" + assert_grep 'echo "{state}: {one short line}" >>' "$brief" \ + "$mode brief must keep the bare status echo" + assert_no_grep "fm-status-append.sh" "$brief" \ + "$mode brief must not reference the no-mistakes-only guard helper" + done + pass "fm-brief.sh: direct-PR and local-only briefs keep the bare status echo" +} + test_ship_project_memory_wording() { local home id brief home="$TMP_ROOT/project-memory-home" @@ -771,6 +822,8 @@ test_ship_mode_is_explicit_not_registry test_delivery_flags_are_refused_where_they_do_not_apply test_faster_paths_use_configured_authority_without_stacked_review test_no_mistakes_dod_wording +test_no_mistakes_done_drift_is_structurally_blocked +test_non_no_mistakes_modes_keep_bare_status_echo test_ship_project_memory_wording test_herdr_lab_contract_is_explicit_and_complete test_herdr_lab_contract_quotes_foreign_firstmate_path diff --git a/tests/fm-crew-state.test.sh b/tests/fm-crew-state.test.sh index 87f61a60324..b70f60c80a5 100755 --- a/tests/fm-crew-state.test.sh +++ b/tests/fm-crew-state.test.sh @@ -1306,6 +1306,65 @@ test_no_run_idle_pane_uses_log() { pass "no run + idle pane uses the status-log verb" } +# A mode=no-mistakes ship task that declared done at "committed, gates green" +# without ever starting a run and without a validated pr= in its meta must not +# read as done: the direct regression case for the 08-30/08-31 done-drift +# pattern, where nine workers reported done at commit and the pipeline was +# never run. +test_no_run_no_mistakes_done_without_pr_reports_parked() { + reset_fakes + local d; d=$(new_case no-mistakes-done-no-pr) + make_repo_on_branch "$d/wt" fm/feat-nm + make_fakebin "$d" >/dev/null + fm_write_meta "$d/state/feat-nm.meta" "window=fm:fm-feat-nm" "worktree=$d/wt" "kind=ship" \ + "harness=claude" "mode=no-mistakes" + printf 'done: committed, gates green\n' > "$d/state/feat-nm.status" + FM_FAKE_AXI_STATUS="" + FM_FAKE_RUNS_LIST="" + arm_idle_record "$d/state" feat-nm + local out; out=$(run_crew_state "$d" feat-nm) + assert_contains "$out" "state: parked" "premature no-mistakes done reports parked, not done" + assert_contains "$out" "pipeline not started" "detail names the missing pipeline step" + pass "a no-mistakes done with no run and no pr reports parked" +} + +# The same crew, once its meta records the pipeline's validated pr=, is +# genuinely done: fm-pr-check.sh's pr= line is what un-blocks the terminal +# state, not a second run attribution path. +test_no_run_no_mistakes_done_with_pr_reports_done() { + reset_fakes + local d; d=$(new_case no-mistakes-done-with-pr) + make_repo_on_branch "$d/wt" fm/feat-nm2 + make_fakebin "$d" >/dev/null + fm_write_meta "$d/state/feat-nm2.meta" "window=fm:fm-feat-nm2" "worktree=$d/wt" "kind=ship" \ + "harness=claude" "mode=no-mistakes" "pr=https://github.com/x/y/pull/1" + printf 'done: PR https://github.com/x/y/pull/1 checks green\n' > "$d/state/feat-nm2.status" + FM_FAKE_AXI_STATUS="" + FM_FAKE_RUNS_LIST="" + arm_idle_record "$d/state" feat-nm2 + local out; out=$(run_crew_state "$d" feat-nm2) + assert_contains "$out" "state: done" "a validated pr= lets the done line stand" + pass "a no-mistakes done with a recorded pr= reports done" +} + +# A non-no-mistakes ship mode has no pipeline step to skip, so its done line is +# never held back for a missing pr=. +test_no_run_direct_pr_done_without_pr_still_reports_done() { + reset_fakes + local d; d=$(new_case direct-pr-done-no-pr) + make_repo_on_branch "$d/wt" fm/feat-dp + make_fakebin "$d" >/dev/null + fm_write_meta "$d/state/feat-dp.meta" "window=fm:fm-feat-dp" "worktree=$d/wt" "kind=ship" \ + "harness=claude" "mode=direct-PR" + printf 'done: PR https://github.com/x/y/pull/2\n' > "$d/state/feat-dp.status" + FM_FAKE_AXI_STATUS="" + FM_FAKE_RUNS_LIST="" + arm_idle_record "$d/state" feat-dp + local out; out=$(run_crew_state "$d" feat-dp) + assert_contains "$out" "state: done" "direct-PR mode has no pipeline gate to withhold done for" + pass "a direct-PR done with no pr= metadata still reports done" +} + test_no_run_idle_pane_uses_keyed_log() { reset_fakes local d; d=$(new_case keyed-idle) @@ -2039,6 +2098,9 @@ test_no_run_herdr_unknown_uses_backend_capture test_no_run_herdr_idle_agent_status_outranked_by_record test_no_run_herdr_idle_agent_status_and_idle_record_stays_idle test_no_run_idle_pane_uses_log +test_no_run_no_mistakes_done_without_pr_reports_parked +test_no_run_no_mistakes_done_with_pr_reports_done +test_no_run_direct_pr_done_without_pr_still_reports_done test_no_run_idle_pane_uses_keyed_log test_no_run_idle_pane_paused test_no_run_idle_pane_custom_paused_verb diff --git a/tests/fm-status-append.test.sh b/tests/fm-status-append.test.sh new file mode 100755 index 00000000000..6047738b9fd --- /dev/null +++ b/tests/fm-status-append.test.sh @@ -0,0 +1,155 @@ +#!/usr/bin/env bash +# Behavior tests for bin/fm-status-append.sh - the guarded status-line append +# helper that bin/fm-brief.sh routes a mode=no-mistakes ship task's status +# report through. +# +# This is the mechanical backstop for the 08-30/08-31 done-drift pattern: nine +# ship workers in a row reported `done: ... committed, gates green` without +# ever starting /no-mistakes, and prose alone (even an explicit capitalized +# DEFINITION OF DONE paragraph) did not stop three later occurrences. This +# helper refuses that exact line instead of silently appending it. +set -u + +# shellcheck source=tests/lib.sh +. "$(dirname "${BASH_SOURCE[0]}")/lib.sh" + +HELPER="$ROOT/bin/fm-status-append.sh" +TMP_ROOT=$(fm_test_tmproot fm-status-append) + +new_case() { # -> echoes case dir with a state/ dir, meta path, status path + local d="$TMP_ROOT/$1" + mkdir -p "$d/state" + printf '%s\n' "$d" +} + +test_script_parses() { + local out rc + out=$(bash -n "$HELPER" 2>&1); rc=$? + expect_code 0 "$rc" "bash -n bin/fm-status-append.sh must parse cleanly (got: $out)" + pass "fm-status-append.sh: bash -n succeeds" +} + +test_refuses_premature_done_on_no_mistakes_without_url() { + local d status meta out rc + d=$(new_case refuse-no-url) + status="$d/state/t1.status" + meta="$d/state/t1.meta" + printf 'kind=ship\nmode=no-mistakes\n' > "$meta" + out=$("$HELPER" "$status" "done: committed, gates green" 2>&1); rc=$? + expect_code 1 "$rc" "a premature done on a no-mistakes task must be refused" + assert_contains "$out" "is not done" "refusal must explain why committed-with-gates-green is not done" + assert_contains "$out" "Run /no-mistakes now" "refusal must print the exact next step" + [ ! -e "$status" ] || fail "refused done must not be written to the status file" + pass "fm-status-append.sh: refuses a premature no-mistakes done that names no PR URL" +} + +# The DOD's earlier "done: {summary}" pseudo-terminal wording (fm-dod-lib.sh, +# before this fix) is exactly as unacceptable as free-text prose: no URL, no +# pass. +test_refuses_the_old_dod_summary_wording() { + local d status meta out rc + d=$(new_case refuse-summary-wording) + status="$d/state/t1b.status" + meta="$d/state/t1b.meta" + printf 'kind=ship\nmode=no-mistakes\n' > "$meta" + out=$("$HELPER" "$status" "done: implemented the fix" 2>&1); rc=$? + expect_code 1 "$rc" "the retired 'done: {summary}' shape must still be refused" + [ ! -e "$status" ] || fail "refused done must not be written to the status file" + pass "fm-status-append.sh: refuses the retired done: {summary} wording" +} + +test_allows_nonterminal_lines_on_no_mistakes() { + local d status meta out rc + d=$(new_case allow-working) + status="$d/state/t2.status" + meta="$d/state/t2.meta" + printf 'kind=ship\nmode=no-mistakes\n' > "$meta" + out=$("$HELPER" "$status" "working: implementation committed, starting /no-mistakes" 2>&1); rc=$? + expect_code 0 "$rc" "a working: line must always be allowed (got: $out)" + assert_contains "$(cat "$status")" "working: implementation committed" \ + "the working: line must be appended verbatim" + pass "fm-status-append.sh: allows a nonterminal working: line on a no-mistakes task" +} + +# The critical case: state/.meta's pr= line is written by firstmate's own +# bin/fm-pr-check.sh only AFTER it sees this exact done report (AGENTS.md +# section 7), so pr= is NEVER present yet at the moment a worker legitimately +# reports it - gating on pr= would refuse every real completion. The line's +# own URL is what must carry the proof instead. +test_allows_done_with_no_pr_recorded_when_the_line_names_a_url() { + local d status meta rc + d=$(new_case allow-done-no-pr-yet) + status="$d/state/t3.status" + meta="$d/state/t3.meta" + printf 'kind=ship\nmode=no-mistakes\n' > "$meta" + "$HELPER" "$status" "done: PR https://github.com/x/y/pull/9 checks green"; rc=$? + expect_code 0 "$rc" "done must be allowed on its own URL even with no pr= recorded yet (got rc=$rc)" + assert_contains "$(cat "$status")" "checks green" "the done line must be appended verbatim" + pass "fm-status-append.sh: allows a done: PR ... line with no pr= recorded in meta yet" +} + +# A recorded pr= (e.g. from a later re-report after firstmate already ran +# fm-pr-check.sh once) does not exempt a line that itself names no URL - the +# line's own shape is what is checked, not stale meta state. +test_recorded_pr_does_not_exempt_a_urlless_done_line() { + local d status meta out rc + d=$(new_case pr-recorded-but-no-url-in-line) + status="$d/state/t3b.status" + meta="$d/state/t3b.meta" + printf 'kind=ship\nmode=no-mistakes\npr=https://github.com/x/y/pull/9\n' > "$meta" + out=$("$HELPER" "$status" "done: committed, gates green" 2>&1); rc=$? + expect_code 1 "$rc" "a urlless done line must be refused even with an unrelated pr= present in meta" + [ ! -e "$status" ] || fail "refused done must not be written to the status file" + pass "fm-status-append.sh: a recorded pr= does not exempt a done line that names no URL" +} + +test_other_modes_never_gated_on_done() { + local d mode status meta rc + d=$(new_case other-modes) + for mode in direct-PR local-only; do + status="$d/state/$mode.status" + meta="$d/state/$mode.meta" + printf 'kind=ship\nmode=%s\n' "$mode" > "$meta" + "$HELPER" "$status" "done: PR https://example.invalid/1"; rc=$? + expect_code 0 "$rc" "$mode has no pipeline step to gate done behind" + assert_contains "$(cat "$status")" "done: PR" "$mode done line must be appended verbatim" + done + pass "fm-status-append.sh: direct-PR and local-only tasks are never gated on a recorded pr=" +} + +test_missing_meta_never_gates_done() { + local d status rc + d=$(new_case no-meta) + status="$d/state/t4.status" + "$HELPER" "$status" "done: whatever"; rc=$? + expect_code 0 "$rc" "a task with no meta file at all must not be gated (nothing to classify)" + assert_contains "$(cat "$status")" "done: whatever" "the done line must still be appended" + pass "fm-status-append.sh: a missing meta file never gates a done line" +} + +test_non_done_lines_always_pass_through() { + local d status meta + d=$(new_case pass-through) + status="$d/state/t5.status" + meta="$d/state/t5.meta" + printf 'kind=ship\nmode=no-mistakes\n' > "$meta" + "$HELPER" "$status" "blocked: waiting on a decision" + "$HELPER" "$status" "needs-decision: which library?" + "$HELPER" "$status" "paused: upstream release pending" + assert_contains "$(cat "$status")" "blocked: waiting on a decision" "blocked: must pass through" + assert_contains "$(cat "$status")" "needs-decision: which library?" "needs-decision: must pass through" + assert_contains "$(cat "$status")" "paused: upstream release pending" "paused: must pass through" + pass "fm-status-append.sh: every non-done verb always passes through unguarded" +} + +test_script_parses +test_refuses_premature_done_on_no_mistakes_without_url +test_refuses_the_old_dod_summary_wording +test_allows_nonterminal_lines_on_no_mistakes +test_allows_done_with_no_pr_recorded_when_the_line_names_a_url +test_recorded_pr_does_not_exempt_a_urlless_done_line +test_other_modes_never_gated_on_done +test_missing_meta_never_gates_done +test_non_done_lines_always_pass_through + +echo "all fm-status-append tests passed" diff --git a/tests/fm-task-delivery.test.sh b/tests/fm-task-delivery.test.sh index af9bf2105e0..acc01c9afbf 100755 --- a/tests/fm-task-delivery.test.sh +++ b/tests/fm-task-delivery.test.sh @@ -361,8 +361,12 @@ STUB payload="$TMP_ROOT/promote-dod/payload-promote-dod-direct-pr" assert_grep "supersede the scout delivery rules and report-based Definition of done" "$payload" \ "promoted worker retained the scout delivery contract" - assert_grep "status protocol; the instruction inbox and its acknowledgement; the escalation rules, including ask-user; and every safety rule" "$payload" \ + assert_grep "the instruction inbox and its acknowledgement; the escalation rules, including ask-user; and every safety rule" "$payload" \ "promoted worker lost the scout protocols and safety rules that still apply" + assert_grep 'echo "{state}: {one short line}" >>' "$payload" \ + "promoted direct-PR worker's status-report command was not regenerated as the bare echo" + assert_no_grep "fm-status-append.sh" "$payload" \ + "promoted direct-PR worker was routed through the no-mistakes-only guard helper" # The faster paths keep their own contracts rather than inheriting the pipeline's. assert_grep "Do NOT run /no-mistakes" "$payload" \ @@ -374,6 +378,116 @@ STUB pass "fm-promote: a promoted worker receives the same mode-specific delivery contract a briefed one does" } +# Direct regression for the promoted-scout half of the 08-30/08-31 done-drift +# pattern: a scout is never mode=no-mistakes, so its own status-report rule is +# always the bare echo bin/fm-brief.sh never had a chance to guard. Promotion +# used to hand-copy that bare echo forward via "the status protocol ... carries +# over unchanged", so a promoted no-mistakes worker could still mechanically +# append `done: committed, gates green` with zero refusal. This drives the real +# promotion path, extracts the exact status-report command the delivered +# instructions tell the worker to run, and then RUNS it against the promoted +# task's own status/meta files - proving the guarded helper is what actually +# executes, not just what the text happens to mention. +test_promoted_no_mistakes_status_report_is_guarded() { + local home meta out sendroot payload id status_file report_cmd rc done_cmd + home="$TMP_ROOT/promote-guard/home" + sendroot="$TMP_ROOT/promote-guard/sendroot" + mkdir -p "$home/state" "$sendroot/bin" + cat > "$sendroot/bin/fm-send.sh" <<'STUB' +#!/usr/bin/env bash +printf '%s' "$2" > "$FM_TEST_CAPTURE" +STUB + chmod +x "$sendroot/bin/fm-send.sh" + + id=promote-guard-nm + meta="$home/state/$id.meta" + status_file="$home/state/$id.status" + printf 'window=fm-%s\nkind=scout\nworktree=/tmp/wt\n' "$id" > "$meta" + out=$(FM_HOME="$home" FM_STATE_OVERRIDE="$home/state" "$PROMOTE" "$id" --mode no-mistakes --yolo off 2>&1) \ + || fail "promotion to mode=no-mistakes should succeed" + + payload="$TMP_ROOT/promote-guard/payload-$id" + ( cd "$sendroot" \ + && FM_TEST_CAPTURE="$payload" \ + eval "$(printf '%s\n' "$out" | sed -n 's/^next: //p' | grep 'fm-send\.sh')" ) \ + || fail "promotion's delivery command did not run" + assert_present "$payload" "promotion delivered no message to the worker" + assert_no_grep "the status protocol; the instruction inbox" "$payload" \ + "a promoted no-mistakes worker was still told its status protocol carries over unchanged" + + # Pull the exact status-report command out of the delivered instructions - + # the line is unique because only the status-report rule carries this + # literal placeholder - then execute it for real instead of grepping for the + # helper's name. + # shellcheck disable=SC2016 # the backticks are a literal grep pattern, not command substitution. + report_cmd=$(grep -F '{state}: {one short line}' "$payload" | grep -o '`[^`]*`' | head -1) + report_cmd=${report_cmd#\`} + report_cmd=${report_cmd%\`} + [ -n "$report_cmd" ] || fail "could not find a rendered status-report command in the delivered instructions" + case "$report_cmd" in + echo\ *) fail "a promoted no-mistakes worker was told to use the bare echo, not the guarded helper" ;; + esac + printf '%s' "$report_cmd" | grep -q 'fm-status-append\.sh' \ + || fail "a promoted no-mistakes worker was not told to use the guarded status-append helper" + + done_cmd=${report_cmd/'{state}: {one short line}'/'done: committed, gates green'} + out=$(eval "$done_cmd" 2>&1); rc=$? + [ "$rc" -ne 0 ] || fail "the promoted worker's own status-report command let a premature done through" + assert_contains "$out" "is not done" "refusal did not explain why committed-with-gates-green is not done" + assert_absent "$status_file" "a refused done was still appended to the promoted worker's status file" + + done_cmd=${report_cmd/'{state}: {one short line}'/'done: PR https://github.com/x/y/pull/1 checks green'} + eval "$done_cmd" || fail "the promoted worker's own status-report command refused a done line naming a real PR URL" + assert_contains "$(cat "$status_file")" "checks green" \ + "the allowed done line was not appended through the promoted worker's status-report command" + pass "fm-promote: a promoted no-mistakes worker's own status-report command is guarded exactly like a fresh brief's" +} + +# The guard is specific to mode=no-mistakes: a promoted direct-PR or local-only +# worker has no pipeline step to skip, so it must keep the bare echo firstmate +# never needs to gate - the same command a fresh brief on the same mode uses. +test_promoted_other_modes_keep_bare_status_echo() { + local home meta out sendroot payload id mode status_file report_cmd + sendroot="$TMP_ROOT/promote-bare-echo/sendroot" + mkdir -p "$sendroot/bin" + cat > "$sendroot/bin/fm-send.sh" <<'STUB' +#!/usr/bin/env bash +printf '%s' "$2" > "$FM_TEST_CAPTURE" +STUB + chmod +x "$sendroot/bin/fm-send.sh" + + for mode in direct-PR local-only; do + home="$TMP_ROOT/promote-bare-echo/home-$mode" + mkdir -p "$home/state" + id="promote-bare-echo-$mode" + meta="$home/state/$id.meta" + status_file="$home/state/$id.status" + printf 'window=fm-%s\nkind=scout\nworktree=/tmp/wt\n' "$id" > "$meta" + out=$(FM_HOME="$home" FM_STATE_OVERRIDE="$home/state" "$PROMOTE" "$id" --mode "$mode" --yolo off 2>&1) \ + || fail "$mode: promotion should succeed" + + payload="$TMP_ROOT/promote-bare-echo/payload-$id" + ( cd "$sendroot" \ + && FM_TEST_CAPTURE="$payload" \ + eval "$(printf '%s\n' "$out" | sed -n 's/^next: //p' | grep 'fm-send\.sh')" ) \ + || fail "$mode: promotion's delivery command did not run" + + # shellcheck disable=SC2016 # the backticks are a literal grep pattern, not command substitution. + report_cmd=$(grep -F '{state}: {one short line}' "$payload" | grep -o '`[^`]*`' | head -1) + report_cmd=${report_cmd#\`} + report_cmd=${report_cmd%\`} + [ -n "$report_cmd" ] || fail "$mode: could not find a rendered status-report command in the delivered instructions" + printf '%s' "$report_cmd" | grep -q 'fm-status-append\.sh' \ + && fail "$mode: a promoted worker with no pipeline step to skip was routed through the no-mistakes-only guard" + + eval "${report_cmd/'{state}: {one short line}'/'done: whatever the worker landed'}" \ + || fail "$mode: the promoted worker's own bare-echo status-report command failed" + assert_contains "$(cat "$status_file")" "done: whatever the worker landed" \ + "$mode: the promoted worker's bare-echo status-report command did not append the done line" + done + pass "fm-promote: direct-PR and local-only promotions keep the bare status echo" +} + # The registry parser survives for the mechanical consumers only. It accepts the # conditional policy, maps it to its most rigorous leg for them, and exposes the # raw annotation for the one caller that must tell a policy from a flat mode. @@ -416,5 +530,7 @@ test_scout_records_no_delivery_posture test_promote_requires_and_records_the_delivery_contract test_promote_refuses_a_symlinked_task_record test_promotion_delivers_the_real_definition_of_done +test_promoted_no_mistakes_status_report_is_guarded +test_promoted_other_modes_keep_bare_status_echo test_project_mode_maps_the_conditional_policy echo "# all fm-task-delivery tests passed"