From 4cb0813a219e45c133074b3da4ff0c03ae97fa46 Mon Sep 17 00:00:00 2001 From: Christopher McKay Date: Wed, 30 Sep 2026 08:10:33 -0400 Subject: [PATCH] fix(bin): require a real PR reference for the CI-ready done note A "checks green" note counted as the CI-ready ship report whenever any uppercase PR appeared, so words such as PROD or PROPERTIES matched. Require a word-bounded PR #N, a /pull/N URL, or a /merge_requests/N URL. Fixes #6200 --- bin/fm-dod-lib.sh | 8 ++++++-- tests/fm-dod-lib.test.sh | 20 ++++++++++++++++++++ 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/bin/fm-dod-lib.sh b/bin/fm-dod-lib.sh index 52cc1cc1c14..26a11579eb5 100755 --- a/bin/fm-dod-lib.sh +++ b/bin/fm-dod-lib.sh @@ -460,11 +460,15 @@ fm_dod_ref_contains() { # # 0 when a done: note reports the no-mistakes CI-ready PR (`PR checks # green`, with any surrounding text). bin/fm-crew-state.sh takes its CI-ready # path on this same test, so every CI-ready line it acts on is gated. +# The note must carry a real PR reference - a word-bounded `PR #N`, a GitHub +# `/pull/N` URL, or a GitLab `/merge_requests/N` URL - so an uppercase "PR" +# inside a word such as PROD or PROPERTIES never counts. fm_dod_note_reports_ci_ready() { # + local re='(^|[^[:alnum:]_])PR #[0-9]+|/pull/[0-9]+|/merge_requests/[0-9]+' case "$1" in - *PR*"checks green"*|*"checks green"*PR*) return 0 ;; + *"checks green"*) [[ $1 =~ $re ]] ;; + *) return 1 ;; esac - return 1 } # 0 when a done: note reports a change published to a Gerrit review server diff --git a/tests/fm-dod-lib.test.sh b/tests/fm-dod-lib.test.sh index dea79f0e3ed..d63d3768681 100644 --- a/tests/fm-dod-lib.test.sh +++ b/tests/fm-dod-lib.test.sh @@ -271,6 +271,25 @@ test_ci_ready_variants_are_gated() { pass "no-mistakes CI-ready done: with extra text is gated" } +test_ci_ready_needs_a_real_pr_reference() { + local note + for note in \ + 'PROD deploy watch, checks green on TEST' \ + 'PROPERTIES table migration, checks green' \ + 'checks green, PR pending'; do + ! fm_dod_note_reports_ci_ready "$note" \ + || fail "note without a PR reference counted as CI-ready: $note" + done + for note in \ + 'PR #12 checks green' \ + 'PR https://github.com/o/r/pull/12 checks green' \ + 'PR https://gitlab.example.test/g/s/p/-/merge_requests/7 checks green'; do + fm_dod_note_reports_ci_ready "$note" \ + || fail "note with a PR reference was not CI-ready: $note" + done + pass "a CI-ready note needs checks green and a real PR reference" +} + test_keyed_and_spaced_done_lines_are_gated() { local repo wt line mode rc repo="$TMP_ROOT/keyed-repo" @@ -393,6 +412,7 @@ test_merge_marker_binds_to_the_named_pr test_forge_recorded_head_is_accepted_without_local_object test_direct_pr_recorded_head_does_not_cover_unpushed_commit test_ci_ready_variants_are_gated +test_ci_ready_needs_a_real_pr_reference test_keyed_and_spaced_done_lines_are_gated test_local_only_linked_branch_is_accepted test_local_only_detached_head_is_refused