fix(bin): require a real PR reference for the CI-ready done note

[karotkriss]

Intent

Fixes #6200

fm_dod_note_reports_ci_ready in bin/fm-dod-lib.sh decides whether a status note is the CI-ready ship report with the substring glob *PR*"checks green"*|*"checks green"*PR*, so any uppercase PR inside a word matches: a note such as paused: PROD deploy watch, checks green on TEST or waiting: PROPERTIES table migration, checks green is classified as a CI-ready done report, turning a task that is still monitoring into a completed one.
A note should count as the CI-ready report only when it says "checks green" and carries a real PR reference (PR #N, a /pull/N URL, or a /merge_requests/N URL).

What Changed

• fm_dod_note_reports_ci_ready in bin/fm-dod-lib.sh still requires "checks green", but it now also requires a real PR reference: a word-bounded PR #N, a /pull/N URL, or a /merge_requests/N URL. The old check used a *PR* substring glob, so an uppercase "PR" inside a word such as PROD or PROPERTIES could make a still-monitoring note count as a CI-ready done report.
• Adds test_ci_ready_needs_a_real_pr_reference to tests/fm-dod-lib.test.sh. It checks that PROD ..., PROPERTIES ..., and checks green, PR pending are rejected, and that PR #12, GitHub pull URLs, and GitLab merge request URLs are accepted.

Fixes #6200

Risk Assessment

✅ Low: The change narrows a single shared classifier to exactly the three PR-reference forms the intent requires (word-bounded PR #N, /pull/N, /merge_requests/N). Both consumers (fm_dod_should_gate_ship_done and fm-crew-state's log_reports_ci_ready) go through that one function, so they stay consistent. The new behavioral test fails on the old glob and passes on the new regex.

Testing

I ran the real fm-crew-state.sh on base and on HEAD against disposable task fixtures. A fake no-mistakes reported a run still in its ci step. On base, all three notes without a real PR reference (PROD, PROPERTIES, and a note that says PR but gives no number) read as done. On HEAD they read as working. The three notes with real PR references read as done on both base and HEAD. The targeted fm-dod-lib and fm-crew-state test files both passed. Probing the matcher directly shows PR#12 (no space) is no longer counted as CI-ready. The ship brief and fm-pr-check always use the PR <url> form, so that does not affect real reports. There is no UI to screenshot: these are CLI helpers, so the evidence is a before/after transcript.

• Live validation: ✅ go - 6 of 6 scenarios driven live against the product

Scenario	Result	Live	Evidence
A worker note 'done: PROD deploy watch, checks green on TEST' while the run is still monitoring CI reads as working, not done	✅ pass	live	crew-state-before-after.txt: base gives state: done · status-log; HEAD gives state: working · run-step
A worker note 'done: PROPERTIES table migration, checks green' reads as working, not done	✅ pass	live	crew-state-before-after.txt: base gives done; HEAD gives working
Adversarial: a note that contains the word PR but no reference ('checks green on staging, SPRINT PR pending') is not treated as CI-ready	✅ pass	live	crew-state-before-after.txt: base gives done; HEAD gives working
The canonical 'done: PR https://github.com/o/r/pull/2 checks green' still reads as done (run still monitoring PR)	✅ pass	live	crew-state-before-after.txt: HEAD gives state: done · status-log
A GitLab '/merge_requests/N' URL note and a 'PR #12 checks green' note still read as done	✅ pass	live	crew-state-before-after.txt: HEAD gives state: done for both
Adversarial matcher probes: SPR #12, a /pulls path and /pull/abc are rejected, (PR #12) is accepted, and a note without 'checks green' is rejected	✅ pass	live	edge-cases.txt

Evidence: fm-crew-state before/after transcript

Source: fm-crew-state before/after transcript

##### BASE eb77f02 (before fix)
=== done: PROD deploy watch, checks green on TEST
    state: done · source: status-log · PROD deploy watch, checks green on TEST · run still monitoring PR
=== done: PROPERTIES table migration, checks green
    state: done · source: status-log · PROPERTIES table migration, checks green · run still monitoring PR
=== done: checks green on staging, SPRINT PR pending
    state: done · source: status-log · checks green on staging, SPRINT PR pending · run still monitoring PR
=== done: PR https://github.com/o/r/pull/2 checks green
    state: done · source: status-log · PR https://github.com/o/r/pull/2 checks green · run still monitoring PR
=== done: PR https://gitlab.example.test/g/s/p/-/merge_requests/7 checks green
    state: done · source: status-log · PR https://gitlab.example.test/g/s/p/-/merge_requests/7 checks green · run still monitoring PR
=== done: PR #12 checks green
    state: done · source: status-log · PR #12 checks green · run still monitoring PR

##### HEAD 4cb0813 (after fix)
=== done: PROD deploy watch, checks green on TEST
    state: working · source: run-step · validating (running)
=== done: PROPERTIES table migration, checks green
    state: working · source: run-step · validating (running)
=== done: checks green on staging, SPRINT PR pending
    state: working · source: run-step · validating (running)
=== done: PR https://github.com/o/r/pull/2 checks green
    state: done · source: status-log · PR https://github.com/o/r/pull/2 checks green · run still monitoring PR
=== done: PR https://gitlab.example.test/g/s/p/-/merge_requests/7 checks green
    state: done · source: status-log · PR https://gitlab.example.test/g/s/p/-/merge_requests/7 checks green · run still monitoring PR
=== done: PR #12 checks green
    state: done · source: status-log · PR #12 checks green · run still monitoring PR

Evidence: Driver script for the crew-state scenarios

Source: Driver script for the crew-state scenarios

#!/usr/bin/env bash
# Drives the real bin/fm-crew-state.sh (from the tree at $1) over a disposable
# ship task whose status log carries each note below, while its no-mistakes run
# is still monitoring CI. Prints the crew state the supervisor would read.
set -u
TREE=$1
WT=~/.no-mistakes/worktrees/80aee654c94f/01M3S91J04KSZD9VRQK78GHGMB
# Reuse the fixture helpers (fake no-mistakes/tmux, repo builder) from the test file only.
eval "$(sed -n '1,800p' "$WT/tests/fm-crew-state.test.sh" | sed 's#$(dirname "${BASH_SOURCE\[0\]}")/lib.sh#'"$WT"'/tests/lib.sh#')"
CREW_STATE="$TREE/bin/fm-crew-state.sh"
i=0
while IFS= read -r note; do
  i=$((i+1))
  reset_fakes
  d=$(new_case "c$i")
  make_repo_on_branch "$d/wt" "fm/feat-$i"
  make_fakebin "$d" >/dev/null
  fm_write_meta "$d/state/feat-$i.meta" "window=fm:fm-feat-$i" "worktree=$d/wt" "kind=ship" "mode=no-mistakes"
  printf 'done: %s\n' "$note" > "$d/state/feat-$i.status"
  FM_FAKE_CI_LOGS='waiting for checks'
  FM_FAKE_AXI_STATUS="$(run_ci_monitoring "fm/feat-$i")"
  out=$(run_crew_state "$d" "feat-$i")
  printf '=== done: %s\n' "$note"
  printf '%s\n' "$out" | grep -E '^(state|source|detail):' | sed 's/^/    /'
done <<'EOF'
PROD deploy watch, checks green on TEST
PROPERTIES table migration, checks green
checks green on staging, SPRINT PR pending
PR https://github.com/o/r/pull/2 checks green
PR https://gitlab.example.test/g/s/p/-/merge_requests/7 checks green
PR #12 checks green
EOF

Evidence: CI-ready matcher edge cases

Source: CI-ready matcher edge cases

PR#12 checks green                                      -> not-ready
(PR #12) checks green                                   -> ci-ready
SPR #12 checks green                                    -> not-ready
checks green PR https://github.com/o/r/pull/9           -> ci-ready
PROD https://github.com/o/r/pulls checks green          -> not-ready
PR https://github.com/o/r/pull/abc checks green         -> not-ready
PR #12 checks pending                                   -> not-ready

- Outcome: ⚠️ 1 info across 1 run (5m2s)

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

✅ **Review** - passed

✅ No issues found.

⚠️ **Test** - 1 info

• ℹ️ bin/fm-dod-lib.sh:467 - The new pattern needs a space in PR #N, so PR#12 checks green is no longer counted as CI-ready (the old substring glob accepted it). The generated brief and fm-pr-check.sh always write PR <url> checks green, so real reports are unaffected. The issue's suggested pattern (PR[[:space:]]*#) would also accept the no-space form if that matters.
• Live validation: ✅ go - 6 of 6 scenarios driven live against the product

Scenario	Result	Live	Evidence
A worker note 'done: PROD deploy watch, checks green on TEST' while the run is still monitoring CI reads as working, not done	✅ pass	live	crew-state-before-after.txt: base gives state: done · status-log; HEAD gives state: working · run-step
A worker note 'done: PROPERTIES table migration, checks green' reads as working, not done	✅ pass	live	crew-state-before-after.txt: base gives done; HEAD gives working
Adversarial: a note that contains the word PR but no reference ('checks green on staging, SPRINT PR pending') is not treated as CI-ready	✅ pass	live	crew-state-before-after.txt: base gives done; HEAD gives working
The canonical 'done: PR https://github.com/o/r/pull/2 checks green' still reads as done (run still monitoring PR)	✅ pass	live	crew-state-before-after.txt: HEAD gives state: done · status-log
A GitLab '/merge_requests/N' URL note and a 'PR #12 checks green' note still read as done	✅ pass	live	crew-state-before-after.txt: HEAD gives state: done for both
Adversarial matcher probes: SPR #12, a /pulls path and /pull/abc are rejected, (PR #12) is accepted, and a note without 'checks green' is rejected	✅ pass	live	edge-cases.txt

• bash drive-crew-state.sh <base-tree> and bash drive-crew-state.sh <worktree>: runs the real bin/fm-crew-state.sh from base eb77f02 (extracted with git archive) and from HEAD 4cb0813. Each run uses a disposable ship task with mode=no-mistakes whose no-mistakes run is still monitoring CI. The status notes cover PROD, PROPERTIES, a note that says PR but gives no number, /pull/N, /merge_requests/N and PR #N.
• bash tests/fm-dod-lib.test.sh (includes the new test_ci_ready_needs_a_real_pr_reference): all passed
• bash tests/fm-crew-state.test.sh: all passed
• Called fm_dod_note_reports_ci_ready directly with edge-case notes: PR#12 (no space), (PR #12), SPR #12, a trailing /pull/N URL, a /pulls path, /pull/abc, and a note without 'checks green'

✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

[greptile-apps]

Confidence Score: 4/5

[Medium risk] Tightens PR reference validation in CI-ready check.

The PR should not merge until bare paths can no longer cause a monitoring task to be reported as done.

_{Reviews (1) · Last reviewed commit: "fix(bin): require a real PR reference fo..."}

[greptile-apps]

Bare paths count as PRs A note such as done: checks green; see /pull/12 passes this matcher even though /pull/12 is not a PR URL. If the run is still monitoring CI and the worker’s head is reachable outside its worktree, crew-state can report the task as done instead of working. Require the URL alternatives to match an actual PR URL, and add a negative test for bare paths.

[kunchenguid]

Speaking as Kun's firstmate: whole thread read (body + Greptile P1). Diff reviewed vs main 23e5584714e6765cc223a1740d385d0e85f8ad8e (fm_dod_note_reports_ci_ready requires checks green plus a real PR reference via word-bounded PR #N / /pull/N / /merge_requests/N). Author karotkriss not blocked. Closes ready-for-pr #6200: yes.

HEAD 4cb0813a219e45c133074b3da4ff0c03ae97fa46. MERGEABLE/UNSTABLE. Attestation MATCH. Tip CI 36724931962 SUCCESS. Tip NM 36724931954 SUCCESS. Greptile Review FAILURE with open P1: bare path /pull/12 (no forge host) still matches and can flip a monitoring task to done. No .github/workflows/*.

contract-class: restore — tip substring glob *PR*"checks green"* treats PROD/PROPERTIES as a PR reference and mis-completes monitoring tasks (#6200). Tightening to a real PR reference restores the intended CI-ready gate. Not a new default-on surface (FM-LEARN-4627 / FM-LEARN-CLAIMS).

VISION.md (each rule):

• One captain, one interface: Aligns — false CI-ready completion buries still-working tasks.
• Authority is explicit: n/a (classifier restore).
• Scripts own the mechanics: Aligns — note gate stays scripted.
• A restart is a non-event: Aligns — durable status honesty.
• Delegation with a spine: Aligns — DoD / crew-state CI-ready path.
• The fleet outlives any vendor: Aligns — GitHub + GitLab URL shapes.
• Scope: Aligns — DoD command layer.

Outcome: waiting-author — require forge URL shape for the /pull/ and /merge_requests/ alternatives (and a negative test for bare paths), then re-push. No merge while Greptile red/UNSTABLE. No Firstmate flag.