fix: require merge evidence for passed crew runs

.github/pr-bodies/39.md

@@ -0,0 +1,38 @@
+## CEO overview
+
+- **What is changing:** Firstmate only reports a worker's pull request as merged when it has matching merge evidence.
+- **Why it matters:** A finished worker could previously appear to have landed its work while its pull request was still open.
+- **Customer or business impact:** The captain can distinguish work ready for review from work already merged, reducing the risk of overlooking unfinished delivery.
+- **Risk and rollout:** Low risk; this delivery stops when CI is ready for review, with merging reserved for the captain's approval.
+  Active workers keep their current status, and safeguards still refuse to remove unlanded work.
+  Older runs without matching evidence may now show that their merge is unverified.
+
+## What changed technically
+
+- Passed crew runs require matching current-run PR identity, task metadata, and owned PR-poll merge evidence before displaying `PR merged/closed`.
+- Missing or conflicting evidence produces `PR merge unverified`; runs without valid PR metadata retain plain `run passed`.
+- Regression coverage exercises the public crew-state interface and teardown refusal, including stale evidence for a different PR on the same branch and head.
+- The decision re-arm test uses changing pane output and waits for observed captures so an independent idle-pane wake cannot be mistaken for a duplicate decision.
+
+## Validation
+
+- **Checks passed:** The preceding pipeline phases passed targeted crew-state, PR-poll, teardown, shell portability, lint, and documentation checks.
+  CI also passed both portable parallel shards, serial shards 1, 3, and 4, Herdr, stock macOS Bash compatibility, coverage, repository invariants, and the no-mistakes requirement.
+  After the CI repair, the full inactive-reconciliation and PR communication suites, both PR-body checkers, targeted ShellCheck, Bash syntax, documentation audience checks, and diff checks passed locally.
+- **Checks not run:** Hosted communication checks have not passed against this replacement narrative; the live PR description still uses the rejected legacy headings.
+  The outer executor must compose this narrative with the latest live body using `bin/fm-pr-body-compose.sh`, publish the result while preserving the machine-owned Pipeline section, and verify the resulting hosted checks.
+  No live forge mutation or merge was performed.
+- **Evidence and limitations:** Isolated public-script fixtures reproduced the false merge claim and verified the corrected open, merged, and closed-unmerged paths, while teardown preserved unlanded work.
+  The live PR body reproduces all nine missing-field failures locally; replacing only its narrative with this file passes both executable body checkers while preserving the existing Pipeline suffix.
+  Committing this file alone does not update the live body that GitHub Actions assesses.
+  The [crew-state transcript](https://github.com/bingb0t5/firstmate/blob/806e500eef89f3d967310faf328a1c8a3945ecc1/.no-mistakes/evidence/fm/fm-crew-state-open-pr-truth-r1/crew-state-transcript.txt), [poll-to-watcher transcript](https://github.com/bingb0t5/firstmate/blob/806e500eef89f3d967310faf328a1c8a3945ecc1/.no-mistakes/evidence/fm/fm-crew-state-open-pr-truth-r1/poll-to-crew-transcript.txt), and [teardown transcript](https://github.com/bingb0t5/firstmate/blob/806e500eef89f3d967310faf328a1c8a3945ecc1/.no-mistakes/evidence/fm/fm-crew-state-open-pr-truth-r1/teardown-transcript.txt) use controlled forge responses rather than live GitHub state.
+
+## Module-boundary decision
+
+Current module retained: crew-state owns the displayed run verdict and reuses the existing PR identity and owned merge-evidence helpers.
+No new merge authority or cleanup path is introduced.
+
+## Decision needed
+
+No implementation decision required.
+Merge approval remains with the captain; this validation run does not authorize merging.

bin/fm-crew-state.sh

@@ -12,8 +12,9 @@
 # identity, else the pane busy-signature) and reconciles the possibly-stale log
 # against it.
 #
-# The determinism lives entirely here - only run-step / pane / log reads plus
-# fixed mapping logic, no heuristics and no LLM. Output is one stable, parseable,
+# The determinism lives entirely here - only run-step / pane / log reads and
+# owned PR-poll merge evidence plus fixed mapping logic, no heuristics and no LLM.
+# Output is one stable, parseable,
 # token-tight line firstmate can read every heartbeat:
 #
 #   state: <working|parked|done|blocked|paused|failed|unknown> · source: <run-step|pane|status-log|remote-endpoint|none> · <detail>
@@ -42,6 +43,14 @@
 #      checks" from "checks green, waiting on merge" (see nm_ci_checks_state) -
 #      a ci-step log-tail check reports checks green without terminalizing a
 #      live run, so a green PR is never silently read as still-validating.
+#      A terminal passed run is done, but does not itself prove a PR merged.
+#      Its detail claims PR merged/closed only when the run's canonical PR
+#      identity matches task metadata and the owned PR-poll merge-notification
+#      marker validated by fm-pr-lib.sh. With valid PR metadata but missing or
+#      mismatched evidence, detail is "run passed: PR merge unverified";
+#      without valid PR metadata, it is "run passed". This read makes no live
+#      forge query, and done is not permission to tear down unlanded work:
+#      fm-teardown.sh independently owns the landed-work test.
 #   3. Reconcile the status log: if its last line says needs-decision/blocked but
 #      the run-step shows the run moved on, the log is deterministically stale and
 #      is flagged superseded. A genuinely parked run plus a needs-decision log
@@ -73,6 +82,8 @@ STATE="${FM_STATE_OVERRIDE:-$FM_HOME/state}"
 . "$SCRIPT_DIR/fm-busy-lib.sh"
 # shellcheck source=bin/fm-nm-run-lib.sh
 . "$SCRIPT_DIR/fm-nm-run-lib.sh"
+# shellcheck source=bin/fm-pr-lib.sh
+. "$SCRIPT_DIR/fm-pr-lib.sh"
 
 ID=${1:-}
 [ -n "$ID" ] || { echo "usage: fm-crew-state.sh <id>" >&2; exit 2; }
@@ -323,21 +334,20 @@ nm_effective_ci_step_status() {
   fi
 }
 
-# Root cause of the PR #252 incident (2026-07): for a repo where merge is left
-# to the captain, no-mistakes' ci step (and therefore top-level status/outcome)
-# stays "running" for the ENTIRE CI-monitor phase, including long after GitHub
-# reports every check green - it only reaches outcome=passed once the PR is
-# actually merged (or failed/cancelled if closed). `axi status`'s steps[] table
-# never distinguishes "still waiting on checks" from "checks green, waiting on
-# merge": both read as plain `ci,running,...`. The only place that transition is
-# recorded is the ci step's own log text, e.g. "all CI checks passed - still
-# monitoring until merged or closed" or "no CI checks reported - still
-# monitoring until merged or closed" (verified against 360+ real run logs under
-# ~/.no-mistakes/logs/*/ci.log on the installed v1.32.2 binary, including the
-# actual PR #252 run). Reads the ci step's log tail via `axi logs` and scans it
-# for the MOST RECENT recognized marker (the log is append-only/chronological,
-# so the last match is current): green with nothing red after it means CI is
-# green right now, still only waiting on merge/close.
+# Corroborate merge detail under the header's evidence contract.
+pr_merge_observed() {
+  fm_pr_metadata_identity_parse "$META" || return 1
+  fm_pr_url_parse "$(strip_quotes "$(nm_field pr)")" || return 1
+  [ "$FM_PR_URL" = "$FM_PR_META_URL" ] || return 1
+  fm_pr_poll_merge_already_notified "$STATE" "$ID" \
+    "$FM_PR_META_PROVIDER" "$FM_PR_META_HOST" "$FM_PR_META_PATH" \
+    "$FM_PR_META_NUMBER"
+}
+
+# During CI monitoring, `axi status`'s steps[] table reports both pending
+# checks and green checks awaiting merge as `ci,running,...`.
+# Read the ci step's append-only log tail via `axi logs`; the most recent
+# recognized marker wins so a later failure or re-arm supersedes earlier green.
 nm_ci_checks_state() {
   local run_id log_tail marker
   run_id=$(strip_quotes "$(nm_field id)")
@@ -502,7 +512,16 @@ if [ "$HAVE_RUN" = 1 ]; then
     case "$status" in running|fixing|ci) active_status=1 ;; esac
     if [ -n "$outcome" ] && [ "$active_status" -eq 0 ]; then
       case "$outcome" in
-        passed)        RUN_STATE="done"; RUN_DETAIL="run passed: PR merged/closed" ;;
+        passed)
+          RUN_STATE="done"
+          if pr_merge_observed; then
+            RUN_DETAIL="run passed: PR merged/closed"
+          elif fm_pr_metadata_identity_parse "$META"; then
+            RUN_DETAIL="run passed: PR merge unverified"
+          else
+            RUN_DETAIL="run passed"
+          fi
+          ;;
         checks-passed) RUN_STATE="done"; RUN_DETAIL="checks green: PR ready for review" ;;
         failed)        RUN_STATE=failed; RUN_DETAIL="run failed" ;;
         cancelled)     RUN_STATE=failed; RUN_DETAIL="run cancelled" ;;

tests/fm-crew-state.test.sh

@@ -31,6 +31,8 @@ set -u
 . "$(dirname "${BASH_SOURCE[0]}")/lib.sh"
 # shellcheck source=/dev/null
 . "$ROOT/bin/fm-classify-lib.sh"
+# shellcheck source=bin/fm-pr-lib.sh
+. "$ROOT/bin/fm-pr-lib.sh"
 
 CREW_STATE="$ROOT/bin/fm-crew-state.sh"
 TMP_ROOT=$(fm_test_tmproot fm-crew-state)
@@ -272,7 +274,7 @@ run:
   branch: $1
   status: completed
   head: "${FM_FAKE_RUN_HEAD:-abc1234}"
-  pr: "https://github.com/o/r/pull/1"
+  pr: "${2-https://github.com/o/r/pull/1}"
   findings: none
 outcome: passed
 EOF
@@ -677,6 +679,97 @@ test_top_level_fixing_done_log_stays_working() {
 }
 
 # (d) terminal run-step is authoritative
+mark_merged_poll() {  # <state-dir> <id> <number>
+  fm_pr_poll_merge_mark_notified "$1" "$2" github github.com o/r "$3" \
+    || fail "could not create merged PR poll evidence fixture"
+}
+
+# A stopped worker can leave no-mistakes outcome=passed after CI concluded while
+# the recorded PR remains open. Without the owned PR-poll marker, the state
+# reader must report the run as complete but must not claim that the PR merged.
+test_passed_open_pr_does_not_claim_merge() {
+  reset_fakes
+  local d; d=$(new_case passed-open-pr)
+  make_repo_on_branch "$d/wt" fm/feat-open-pr
+  make_fakebin "$d" >/dev/null
+  fm_write_meta "$d/state/feat-open-pr.meta" "window=fm:fm-feat-open-pr" \
+    "worktree=$d/wt" "kind=ship" "pr=https://github.com/o/r/pull/1"
+  printf 'done: PR https://github.com/o/r/pull/1 checks green\n' > \
+    "$d/state/feat-open-pr.status"
+  FM_FAKE_AXI_STATUS="$(run_passed fm/feat-open-pr)"
+  local out; out=$(run_crew_state "$d" feat-open-pr)
+  assert_contains "$out" "state: done" "stopped passed run remains complete"
+  assert_contains "$out" "source: run-step" "passed run remains run-step sourced"
+  assert_contains "$out" "PR merge unverified" \
+    "an open or unknown PR is reported as merge-unverified"
+  assert_not_contains "$out" "PR merged/closed" \
+    "a passed run without forge evidence must not claim a merge"
+  pass "passed open PR is not reported as merged"
+}
+
+# A matching marker is the owned poll's durable proof that its forge read saw a
+# merge. It restores the terminal merged detail without requiring a live network
+# call or trusting a marker for another PR.
+test_passed_pr_with_owned_merge_evidence_claims_merge() {
+  reset_fakes
+  local d; d=$(new_case passed-merged-pr)
+  make_repo_on_branch "$d/wt" fm/feat-merged-pr
+  make_fakebin "$d" >/dev/null
+  fm_write_meta "$d/state/feat-merged-pr.meta" "window=fm:fm-feat-merged-pr" \
+    "worktree=$d/wt" "kind=ship" "pr=https://github.com/o/r/pull/1"
+  mark_merged_poll "$d/state" feat-merged-pr 1
+  FM_FAKE_AXI_STATUS="$(run_passed fm/feat-merged-pr)"
+  local out; out=$(run_crew_state "$d" feat-merged-pr)
+  assert_contains "$out" "state: done" "merged run remains complete"
+  assert_contains "$out" "run passed: PR merged/closed" \
+    "matching owned merge evidence restores merged detail"
+  pass "passed PR with matching owned merge evidence is reported as merged"
+}
+
+# A marker for a different PR is not evidence for the currently recorded PR.
+test_passed_pr_with_mismatched_merge_evidence_stays_unverified() {
+  reset_fakes
+  local d; d=$(new_case passed-mismatched-pr)
+  make_repo_on_branch "$d/wt" fm/feat-mismatched-pr
+  make_fakebin "$d" >/dev/null
+  fm_write_meta "$d/state/feat-mismatched-pr.meta" "window=fm:fm-feat-mismatched-pr" \
+    "worktree=$d/wt" "kind=ship" "pr=https://github.com/o/r/pull/1"
+  mark_merged_poll "$d/state" feat-mismatched-pr 2
+  FM_FAKE_AXI_STATUS="$(run_passed fm/feat-mismatched-pr)"
+  local out; out=$(run_crew_state "$d" feat-mismatched-pr)
+  assert_contains "$out" "PR merge unverified" \
+    "mismatched merge evidence stays unverified"
+  assert_not_contains "$out" "PR merged/closed" \
+    "mismatched merge evidence cannot claim a merge"
+  pass "mismatched owned merge evidence is rejected"
+}
+
+test_passed_run_with_stale_pr_metadata_stays_unverified() {
+  reset_fakes
+  local d run_pr out
+  d=$(new_case passed-stale-pr-metadata)
+  make_repo_on_branch "$d/wt" fm/feat-stale-pr
+  make_fakebin "$d" >/dev/null
+  fm_write_meta "$d/state/feat-stale-pr.meta" "window=fm:fm-feat-stale-pr" \
+    "worktree=$d/wt" "kind=ship" "pr=https://github.com/o/r/pull/1"
+  mark_merged_poll "$d/state" feat-stale-pr 1
+  FM_FAKE_TMUX_MISSING=1
+  for run_pr in \
+    https://github.com/o/r/pull/2 \
+    https://github.com/o/other/pull/1 \
+    https://gitlab.com/o/r/-/merge_requests/1 \
+    https://github.com/o/r/pull/1/invalid \
+    ''; do
+    FM_FAKE_AXI_STATUS="$(run_passed fm/feat-stale-pr "$run_pr")"
+    out=$(run_crew_state "$d" feat-stale-pr)
+    assert_contains "$out" "state: done" "stopped run remains complete: $run_pr"
+    assert_contains "$out" "source: run-step" "current run remains attributed: $run_pr"
+    assert_contains "$out" "PR merge unverified" "run PR must match merge evidence: $run_pr"
+    assert_not_contains "$out" "PR merged/closed" "stale metadata cannot prove this run merged: $run_pr"
+  done
+  pass "retained metadata and merge evidence cannot certify another run PR"
+}
+
 test_terminal_passed() {
   reset_fakes
   local d; d=$(new_case passed)
@@ -1488,6 +1581,10 @@ test_ci_ready_done_log_relapse_stays_working
 test_ci_fixing_after_green_stays_working
 test_top_level_fixing_ci_running_after_green_stays_working
 test_top_level_fixing_done_log_stays_working
+test_passed_open_pr_does_not_claim_merge
+test_passed_pr_with_owned_merge_evidence_claims_merge
+test_passed_pr_with_mismatched_merge_evidence_stays_unverified
+test_passed_run_with_stale_pr_metadata_stays_unverified
 test_terminal_passed
 test_terminal_failed
 test_live_ci_status_outranks_stale_failure

tests/fm-inactive-reconcile.test.sh

@@ -842,9 +842,21 @@ test_fresh_progress_is_not_aged_from_task_creation() {
 }
 
 test_decision_backstop_commits_the_watcher_generation() {
-  local actor out pid i
+  local actor out pid i captures
   for actor in main away; do
     make_world "decision-generation-$actor"
+    # Keep pane-staleness independent of decision-generation deduplication.
+    # A static idle pane legitimately wakes the away watcher after two polls.
+    cat > "$WORLD/fakebin/tmux" <<'SH'
+#!/usr/bin/env bash
+case "${1:-}" in
+  display-message) printf '%%1\n' ;;
+  capture-pane)
+    printf 'capture\n' >> "${FM_HOME:?}/captures"
+    printf 'independent output %s\n> \n' "$(wc -l < "$FM_HOME/captures")"
+    ;;
+esac
+SH
     write_child "$MAIN" child 'needs-decision [key=api-shape]: choose the API shape'
     # Count completed polling opportunities instead of depending on how many
     # watcher cycles this machine happens to squeeze into a three-second sleep.
@@ -885,18 +897,17 @@ SH
       FM_WATCH_HANDLING_SUCCESSOR=1 "$WATCH" > "$out" 2>&1 &
     pid=$!
     i=0
-    while [ "$i" -lt 200 ] && kill -0 "$pid" 2>/dev/null; do
-      [ "$(wc -l < "$MAIN/captures")" -ge 4 ] && break
+    captures=0
+    while [ "$i" -lt 100 ] && kill -0 "$pid" 2>/dev/null; do
+      captures=$(wc -l < "$MAIN/captures")
+      [ "$captures" -ge 4 ] && break
       sleep 0.1
       i=$((i + 1))
     done
-    if ! kill -0 "$pid" 2>/dev/null; then
-      wait "$pid" || true
-      fail "$actor re-arm duplicated the handled decision: $(cat "$out")"
-    fi
+    kill -0 "$pid" 2>/dev/null \
+      || fail "$actor re-arm duplicated the handled decision: $(cat "$out")"
     reap "$pid"
-    [ "$(wc -l < "$MAIN/captures")" -ge 4 ] \
-      || fail "$actor re-arm did not complete repeated polling cycles: $(cat "$out")"
+    [ "$captures" -ge 4 ] || fail "$actor re-arm did not reach its fourth pane capture"
     [ "$(wake_count "$MAIN" 'child.status')" = 0 ] \
       || fail "$actor re-arm queued a duplicate handled decision"
   done

tests/fm-teardown.test.sh

@@ -316,6 +316,20 @@ land_equivalent_patch_on_origin_branch() {
   git -C "$case_dir/project" rev-parse "refs/remotes/origin/$branch"
 }
 
+# Override GitHub lookups to report the recorded PR as OPEN and unmerged.
+add_gh_pr_open_for_head() {
+  local case_dir=$1 head=$2
+  cat > "$case_dir/fakebin/gh" <<SH
+#!/usr/bin/env bash
+case "\${1:-} \${2:-}" in
+  "pr view") printf '%s\\t%s\\n' 'OPEN' '$head' ; exit 0 ;;
+esac
+echo "error: pull request not found" >&2
+exit 1
+SH
+  chmod +x "$case_dir/fakebin/gh"
+}
+
 # Override gh-axi so every call fails, simulating an API/network error.
 add_gh_axi_error() {
   local case_dir=$1
@@ -695,6 +709,42 @@ test_no_mistakes_origin_remote_allows() {
   pass "no-mistakes worktree with HEAD on origin is torn down (no regression)"
 }
 
+# This is the exact false-merge fixture: no-mistakes reports a passed run and
+# metadata records a PR, but the forge still reports that PR as OPEN. Teardown
+# must independently refuse the unlanded work even if crew state once rendered
+# the old false `PR merged/closed` detail.
+test_no_mistakes_passed_open_pr_still_refuses() {
+  local case_dir rc head
+  case_dir=$(make_case nm-passed-open-pr)
+  write_meta "$case_dir" no-mistakes ship
+  wt_commit_file "$case_dir" feature.txt hello "open PR work"
+  append_pr_meta_url "$case_dir"
+  head=$(git -C "$case_dir/wt" rev-parse HEAD)
+  add_gh_pr_open_for_head "$case_dir" "$head"
+  FM_FAKE_AXI_STATUS=$(cat <<EOF
+run:
+  id: "01RUN3060"
+  branch: fm/task-x1
+  status: completed
+  head: "$head"
+  pr: "https://github.com/example/repo/pull/7"
+  findings: none
+outcome: passed
+EOF
+)
+  export FM_FAKE_AXI_STATUS
+
+  set +e
+  run_teardown "$case_dir" > "$case_dir/stdout" 2> "$case_dir/stderr"
+  rc=$?
+  set -e
+
+  expect_code 1 "$rc" "nm-passed-open-pr: teardown should refuse"
+  grep -q REFUSED "$case_dir/stderr" \
+    || fail "nm-passed-open-pr: no REFUSED line in stderr"
+  pass "passed run with open PR is refused as unlanded"
+}
+
 test_no_mistakes_truly_unpushed_refuses() {
   local case_dir rc
   case_dir=$(make_case nm-unpushed)
@@ -2619,6 +2669,7 @@ test_teardown_manual_backend_prompts_hand_edit_even_when_tasks_axi_present
 test_local_only_truly_unpushed_refuses
 test_local_only_merged_to_local_main_allows
 test_no_mistakes_origin_remote_allows
+test_no_mistakes_passed_open_pr_still_refuses
 test_no_mistakes_truly_unpushed_refuses
 test_local_only_force_overrides_unpushed
 test_teardown_missing_busy_sidecar_completes