From 2cc8cf04ae995e246528868df4152a8ed6afcb5e Mon Sep 17 00:00:00 2001 From: Alex William Date: Tue, 8 Sep 2026 05:52:28 +0200 Subject: [PATCH 01/18] fix(bin): refuse a no-mistakes run that kept no change as validated When a branch's commits are already contained in the rebase base, the pipeline loses the whole diff, logs "empty diff after rebase, skipping remaining steps", and still records the run as completed. Review, test, document, lint, push, pr and ci never execute and no PR is opened, yet fm-crew-state.sh mapped that terminal success straight to done, so a run that validated and delivered nothing read as shippable. Read the steps table instead of the result word: when every mandatory delivery phase is present and skipped, report the run failed and say why. Positive evidence is required in both directions, matching the existing held-green reclassification - an absent table, or any mandatory row that is missing or not skipped, leaves the run's own reported result untouched, so a real delivery and a partial skip both stay done. Record at project-management the measured supported way to change a delivery target: the PR target is the clone's own origin remote, so it changes by repointing origin and re-running no-mistakes init, which also refreshes the gate mirror. Editing the gate mirror's remote URL alone changes neither the registration nor its tracking refs. --- .agents/skills/project-management/SKILL.md | 4 + bin/fm-crew-state.sh | 61 +++++++- tests/fm-crew-state.test.sh | 168 +++++++++++++++++++++ 3 files changed, 229 insertions(+), 4 deletions(-) diff --git a/.agents/skills/project-management/SKILL.md b/.agents/skills/project-management/SKILL.md index 86e37422d17..5be9aed6e59 100644 --- a/.agents/skills/project-management/SKILL.md +++ b/.agents/skills/project-management/SKILL.md @@ -82,6 +82,10 @@ Initialization configures the local gate and does not vendor a no-mistakes skill Do not create a commit merely because initialization ran. If doctor reports an environment, authentication, or daemon problem, resolve that blocker before dispatching work and never restart the shared daemon from a project operation. +The repository that a pipeline opens its PR against is the clone's own `origin` remote, and `--fork-url` only chooses where branches are pushed while the PR still targets `origin`. +So the delivery target is changed by repointing that clone's `origin` and running `no-mistakes init` again, which also refreshes the gate mirror to the newly registered target; `no-mistakes status` then reports the new target and keeps it across a daemon restart. +Editing the gate mirror's own remote URL is not a supported way to change the target: it leaves the registration and the mirror's tracking refs pointing at the old repository, so work keeps landing in the previous target and a rebase can silently resolve against a base the new target never had. + ## Remove Project removal is destructive. diff --git a/bin/fm-crew-state.sh b/bin/fm-crew-state.sh index f3a99c3e3e5..2745b7c2209 100755 --- a/bin/fm-crew-state.sh +++ b/bin/fm-crew-state.sh @@ -60,7 +60,14 @@ # coarse runs-ledger fallback (no steps table, no ci log), a terminal # FAILED record whose daemon an explicit probe proves down reads unknown, # never failed: an instrument failure must not read as work failure -# (nm_daemon_probe_down). +# (nm_daemon_probe_down). Conversely a terminal SUCCESS run whose steps +# table shows every mandatory delivery phase skipped reads failed, never +# done: when a branch's whole diff vanishes into the rebase base, +# no-mistakes still records the run completed although nothing was +# reviewed, tested, pushed, or opened as a PR, and validity must not be +# inferred from that word alone (nm_run_skipped_every_mandatory_step; +# 2026-09-08 fm-nm-depot-livraison-non-modifiable incident). The coarse +# fallback carries no steps table, so it cannot recognize that shape. # 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 @@ -453,6 +460,43 @@ nm_reclassify_failed_run_as_held_green() { return 0 } +# The delivery phases a no-mistakes ship run must actually execute before its +# result may be read as validated. `intent` and `rebase` are excluded: they +# prepare a run rather than validate or deliver it. +NM_MANDATORY_STEPS="review test document lint push pr ci" + +# 0 when the steps table proves a terminal-success run validated and delivered +# nothing: every phase in NM_MANDATORY_STEPS is present and `skipped`. This is +# the vacuous-pass shape (2026-09-08 fm-nm-depot-livraison-non-modifiable): a +# branch whose commits are already contained in the rebase base loses its whole +# diff, and no-mistakes then logs `empty diff after rebase, skipping remaining +# steps` and still records the run `completed`. Nothing was reviewed, tested, +# documented, linted, pushed, or opened as a PR, so the word alone is not +# validation. Positive evidence is required in both directions: an absent table, +# or any mandatory row that is missing or not `skipped`, is not this shape and +# leaves the run's own reported result untouched. +nm_run_skipped_every_mandatory_step() { + local rows want + rows=$(nm_steps_rows) + [ -n "$rows" ] || return 1 + for want in $NM_MANDATORY_STEPS; do + printf '%s\n' "$rows" \ + | grep -qE "^[[:space:]]*$want,[[:space:]]*\"?skipped\"?[[:space:]]*," || return 1 + done + return 0 +} + +# Reclassify a terminal SUCCESS run as failed when +# nm_run_skipped_every_mandatory_step matches, so a run that lost its change +# never reads as shippable. The branch still holds whatever the worker +# committed; what is refused is calling that outcome validated. +nm_reclassify_vacuous_success_as_failed() { + nm_run_skipped_every_mandatory_step || return 1 + RUN_STATE=failed + RUN_DETAIL="not validated: run kept no change - review, test, document, lint, push, pr and ci were all skipped" + return 0 +} + # 0 when an explicit probe proves the shared daemon down: `no-mistakes daemon # status` is the canonical down-probe (the same one fm-brief.sh hands crews # before a blocked append) and exits non-zero when the daemon is not running. @@ -638,8 +682,14 @@ if [ "$HAVE_RUN" = 1 ]; then if [ -n "$outcome" ]; then case "$outcome" in - passed) RUN_STATE="done"; RUN_DETAIL="run passed: PR merged/closed" ;; - checks-passed) RUN_STATE="done"; RUN_DETAIL="checks green: PR ready for review" ;; + passed) + if nm_reclassify_vacuous_success_as_failed; then :; else + RUN_STATE="done"; RUN_DETAIL="run passed: PR merged/closed" + fi ;; + checks-passed) + if nm_reclassify_vacuous_success_as_failed; then :; else + RUN_STATE="done"; RUN_DETAIL="checks green: PR ready for review" + fi ;; failed) if nm_reclassify_failed_run_as_held_green; then :; else RUN_STATE=failed; RUN_DETAIL="run failed" @@ -666,7 +716,10 @@ if [ "$HAVE_RUN" = 1 ]; then case "$status" in ci) RUN_STATE=working; RUN_DETAIL="ci running" ;; running|fixing) RUN_STATE=working; RUN_DETAIL="validating ($status)" ;; - completed) RUN_STATE="done"; RUN_DETAIL="run completed" ;; + completed) + if nm_reclassify_vacuous_success_as_failed; then :; else + RUN_STATE="done"; RUN_DETAIL="run completed" + fi ;; failed) if nm_reclassify_failed_run_as_held_green; then :; else RUN_STATE=failed; RUN_DETAIL="run failed" diff --git a/tests/fm-crew-state.test.sh b/tests/fm-crew-state.test.sh index 309a7008f8a..d7f9a2b39b2 100755 --- a/tests/fm-crew-state.test.sh +++ b/tests/fm-crew-state.test.sh @@ -422,6 +422,103 @@ steps[9]{step,status,findings,duration_ms}: EOF } +# The 2026-09-08 vacuous-pass shape, captured verbatim from a real run in an +# isolated NM_HOME fixture (no-mistakes v1.64.0): the branch's commits were +# already contained in the rebase base, so the diff vanished, no-mistakes logged +# "empty diff after rebase, skipping remaining steps", and it still recorded the +# run completed with every mandatory delivery phase skipped and nothing pushed. +run_completed_empty_diff() { # + cat < + cat < + cat < + cat < cat </dev/null + fm_write_meta "$d/state/feat-empty-diff.meta" "window=fm:fm-feat-empty-diff" "worktree=$d/wt" "kind=ship" + FM_FAKE_AXI_STATUS="$(run_completed_empty_diff fm/feat-empty-diff)" + local out; out=$(run_crew_state "$d" feat-empty-diff) + assert_contains "$out" "state: failed" "a run that kept no change must not read as validated" + assert_not_contains "$out" "state: done" "empty-diff run must never read done" + assert_contains "$out" "not validated" "the verdict must say why it is not validated" + pass "completed run with every mandatory phase skipped reads failed" +} + +test_passed_outcome_with_every_phase_skipped_reads_failed() { + reset_fakes + local d; d=$(new_case passed-empty-diff) + make_repo_on_branch "$d/wt" fm/feat-passed-empty + make_fakebin "$d" >/dev/null + fm_write_meta "$d/state/feat-passed-empty.meta" "window=fm:fm-feat-passed-empty" "worktree=$d/wt" "kind=ship" + FM_FAKE_AXI_STATUS="$(run_outcome_empty_diff fm/feat-passed-empty passed)" + local out; out=$(run_crew_state "$d" feat-passed-empty) + assert_contains "$out" "state: failed" "outcome passed must not be trusted on its own" + assert_not_contains "$out" "state: done" "vacuous passed outcome must never read done" + pass "outcome passed with every mandatory phase skipped reads failed" +} + +test_checks_passed_outcome_with_every_phase_skipped_reads_failed() { + reset_fakes + local d; d=$(new_case checks-passed-empty-diff) + make_repo_on_branch "$d/wt" fm/feat-checks-empty + make_fakebin "$d" >/dev/null + fm_write_meta "$d/state/feat-checks-empty.meta" "window=fm:fm-feat-checks-empty" "worktree=$d/wt" "kind=ship" + FM_FAKE_AXI_STATUS="$(run_outcome_empty_diff fm/feat-checks-empty checks-passed)" + local out; out=$(run_crew_state "$d" feat-checks-empty) + assert_contains "$out" "state: failed" "outcome checks-passed must not be trusted on its own" + assert_not_contains "$out" "state: done" "vacuous checks-passed outcome must never read done" + pass "outcome checks-passed with every mandatory phase skipped reads failed" +} + +test_completed_run_with_full_delivery_still_reads_done() { + reset_fakes + local d; d=$(new_case completed-full-delivery) + make_repo_on_branch "$d/wt" fm/feat-full-delivery + make_fakebin "$d" >/dev/null + fm_write_meta "$d/state/feat-full-delivery.meta" "window=fm:fm-feat-full-delivery" "worktree=$d/wt" "kind=ship" + FM_FAKE_AXI_STATUS="$(run_completed_full_delivery fm/feat-full-delivery)" + local out; out=$(run_crew_state "$d" feat-full-delivery) + assert_contains "$out" "state: done" "a run that executed every phase must stay done" + assert_not_contains "$out" "not validated" "a real delivery must not be flagged vacuous" + pass "completed run with every phase executed still reads done" +} + +test_completed_run_with_partial_skip_still_reads_done() { + reset_fakes + local d; d=$(new_case completed-partial-skip) + make_repo_on_branch "$d/wt" fm/feat-partial-skip + make_fakebin "$d" >/dev/null + fm_write_meta "$d/state/feat-partial-skip.meta" "window=fm:fm-feat-partial-skip" "worktree=$d/wt" "kind=ship" + FM_FAKE_AXI_STATUS="$(run_completed_partial_skip fm/feat-partial-skip)" + local out; out=$(run_crew_state "$d" feat-partial-skip) + assert_contains "$out" "state: done" "a delivered run with some skipped phases stays done" + assert_not_contains "$out" "not validated" "partial skips are not the vacuous shape" + pass "completed run with only some phases skipped still reads done" +} + test_terminal_failed_ci_genuine_red_stays_failed() { reset_fakes local d; d=$(new_case failed-ci-genuine-red) @@ -2259,6 +2422,11 @@ test_terminal_failed test_terminal_failed_ci_orphan_after_green_reads_done test_terminal_failed_ci_orphan_status_only_reads_done test_terminal_failed_ci_genuine_red_stays_failed +test_completed_run_with_every_phase_skipped_reads_failed +test_passed_outcome_with_every_phase_skipped_reads_failed +test_checks_passed_outcome_with_every_phase_skipped_reads_failed +test_completed_run_with_full_delivery_still_reads_done +test_completed_run_with_partial_skip_still_reads_done test_terminal_failed_ci_orphan_second_failed_step_stays_failed test_cross_branch_attribution_via_runs_list test_coarse_socket_refusal_reports_blocked From e45f0449f488057fa2495ac665d092acc999b235 Mon Sep 17 00:00:00 2001 From: Alex William Date: Tue, 8 Sep 2026 06:43:20 +0200 Subject: [PATCH 02/18] no-mistakes(review): refuse unverifiable coarse completed run and drop unreachable hook --- .agents/skills/project-management/SKILL.md | 4 +- bin/fm-crew-state.sh | 23 ++++++++--- tests/fm-crew-state.test.sh | 45 +++++++++++++++------- 3 files changed, 51 insertions(+), 21 deletions(-) diff --git a/.agents/skills/project-management/SKILL.md b/.agents/skills/project-management/SKILL.md index 5be9aed6e59..85a25904617 100644 --- a/.agents/skills/project-management/SKILL.md +++ b/.agents/skills/project-management/SKILL.md @@ -83,7 +83,9 @@ Do not create a commit merely because initialization ran. If doctor reports an environment, authentication, or daemon problem, resolve that blocker before dispatching work and never restart the shared daemon from a project operation. The repository that a pipeline opens its PR against is the clone's own `origin` remote, and `--fork-url` only chooses where branches are pushed while the PR still targets `origin`. -So the delivery target is changed by repointing that clone's `origin` and running `no-mistakes init` again, which also refreshes the gate mirror to the newly registered target; `no-mistakes status` then reports the new target and keeps it across a daemon restart. +A clone that contributes upstream therefore keeps `origin` on the parent repository on purpose and pairs it with `--fork-url`, which is that posture working as intended and not a target to correct; the paragraph below applies only when a project's registered delivery target is wrong for the repository the fleet is asked to land work in. +For that case the delivery target is changed by repointing that clone's `origin` and running `no-mistakes init` again, which also refreshes the gate mirror to the newly registered target; `no-mistakes status` then reports the new target and keeps it across a daemon restart. +Which repository a given project should deliver into is the captain's decision, so confirm the intended target before repointing anything and follow any contribution workflow the project documents for itself. Editing the gate mirror's own remote URL is not a supported way to change the target: it leaves the registration and the mirror's tracking refs pointing at the old repository, so work keeps landing in the previous target and a rebase can silently resolve against a base the new target never had. ## Remove diff --git a/bin/fm-crew-state.sh b/bin/fm-crew-state.sh index 2745b7c2209..1a6773080d3 100755 --- a/bin/fm-crew-state.sh +++ b/bin/fm-crew-state.sh @@ -67,7 +67,9 @@ # reviewed, tested, pushed, or opened as a PR, and validity must not be # inferred from that word alone (nm_run_skipped_every_mandatory_step; # 2026-09-08 fm-nm-depot-livraison-non-modifiable incident). The coarse -# fallback carries no steps table, so it cannot recognize that shape. +# fallback carries no steps table and the runs ledger names no run id to +# fetch one with, so it can neither prove nor exclude that shape: there a +# terminal COMPLETED record reads unknown-unverified, never done. # 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 @@ -656,7 +658,19 @@ if [ "$HAVE_RUN" = 1 ]; then # distinction, so a real gate is never silently missed. case "$COARSE_STATUS" in running) RUN_STATE=working; RUN_DETAIL="validating (background run)" ;; - completed) RUN_STATE="done"; RUN_DETAIL="run completed" ;; + completed) + # Symmetric to the failed row below: a terminal ledger word is not a + # verdict here. The vacuous-pass shape the full path refuses + # (nm_run_skipped_every_mandatory_step) is recorded `completed` too, + # and this path has no steps table to tell the two apart - the runs + # ledger carries no run id, so no per-run lookup can supply one + # either. Reporting done would accept as validated exactly the run + # that validated nothing, so the record is reported unverified and + # the crew's own done/failed status-log line stays the only thing + # that can conclude it. + RUN_STATE=unknown + RUN_DETAIL="last ledger record completed; no steps table to prove any delivery phase ran - unverified" + ;; failed) # The ledger row is terminal but the coarse path has no steps table # and no ci log, so the orphaned-monitor shape cannot be recognized @@ -686,10 +700,7 @@ if [ "$HAVE_RUN" = 1 ]; then if nm_reclassify_vacuous_success_as_failed; then :; else RUN_STATE="done"; RUN_DETAIL="run passed: PR merged/closed" fi ;; - checks-passed) - if nm_reclassify_vacuous_success_as_failed; then :; else - RUN_STATE="done"; RUN_DETAIL="checks green: PR ready for review" - fi ;; + checks-passed) RUN_STATE="done"; RUN_DETAIL="checks green: PR ready for review" ;; failed) if nm_reclassify_failed_run_as_held_green; then :; else RUN_STATE=failed; RUN_DETAIL="run failed" diff --git a/tests/fm-crew-state.test.sh b/tests/fm-crew-state.test.sh index d7f9a2b39b2..a3fabeaee4e 100755 --- a/tests/fm-crew-state.test.sh +++ b/tests/fm-crew-state.test.sh @@ -1106,19 +1106,6 @@ test_passed_outcome_with_every_phase_skipped_reads_failed() { pass "outcome passed with every mandatory phase skipped reads failed" } -test_checks_passed_outcome_with_every_phase_skipped_reads_failed() { - reset_fakes - local d; d=$(new_case checks-passed-empty-diff) - make_repo_on_branch "$d/wt" fm/feat-checks-empty - make_fakebin "$d" >/dev/null - fm_write_meta "$d/state/feat-checks-empty.meta" "window=fm:fm-feat-checks-empty" "worktree=$d/wt" "kind=ship" - FM_FAKE_AXI_STATUS="$(run_outcome_empty_diff fm/feat-checks-empty checks-passed)" - local out; out=$(run_crew_state "$d" feat-checks-empty) - assert_contains "$out" "state: failed" "outcome checks-passed must not be trusted on its own" - assert_not_contains "$out" "state: done" "vacuous checks-passed outcome must never read done" - pass "outcome checks-passed with every mandatory phase skipped reads failed" -} - test_completed_run_with_full_delivery_still_reads_done() { reset_fakes local d; d=$(new_case completed-full-delivery) @@ -1145,6 +1132,36 @@ test_completed_run_with_partial_skip_still_reads_done() { pass "completed run with only some phases skipped still reads done" } +# The two-run coarse fallback, the shape the full-path guard alone cannot +# reach: crew A's run ended in the vacuous empty-diff shape, and crew B then +# started a run on the same repo, so the shared daemon's bare `axi status` +# answers with B's branch. A's state read therefore falls to the coarse runs +# ledger, whose newest row for A's branch is `completed` at A's own head - and +# that ledger carries no steps table and no run id, so nothing here can prove +# any delivery phase ran. It must not be reported as a validated done. +test_coarse_completed_ledger_row_is_not_reported_done() { + reset_fakes + local d short; d=$(new_case coarse-completed-unverified) + make_repo_on_branch "$d/wt" fm/feat-coarse-completed + short=$(git -C "$d/wt" rev-parse --short=7 HEAD) + make_fakebin "$d" >/dev/null + fm_write_meta "$d/state/feat-coarse-completed.meta" \ + "window=fm:fm-feat-coarse-completed" "worktree=$d/wt" "kind=ship" + FM_FAKE_AXI_STATUS="$(run_running fm/other-crew)" + FM_FAKE_RUNS_LIST="$(cat < Date: Tue, 8 Sep 2026 06:55:14 +0200 Subject: [PATCH 03/18] no-mistakes(review): keep coarse completed done only on ledger PR evidence --- bin/fm-crew-state.sh | 40 ++++++++++++++++++++------------- bin/fm-nm-run-lib.sh | 25 ++++++++++++++++----- bin/fm-teardown.sh | 5 +++-- tests/fm-crew-state.test.sh | 44 +++++++++++++++++++++++++++++++------ 4 files changed, 85 insertions(+), 29 deletions(-) diff --git a/bin/fm-crew-state.sh b/bin/fm-crew-state.sh index 1a6773080d3..8ca8d77d349 100755 --- a/bin/fm-crew-state.sh +++ b/bin/fm-crew-state.sh @@ -67,9 +67,11 @@ # reviewed, tested, pushed, or opened as a PR, and validity must not be # inferred from that word alone (nm_run_skipped_every_mandatory_step; # 2026-09-08 fm-nm-depot-livraison-non-modifiable incident). The coarse -# fallback carries no steps table and the runs ledger names no run id to -# fetch one with, so it can neither prove nor exclude that shape: there a -# terminal COMPLETED record reads unknown-unverified, never done. +# fallback carries no steps table, and the runs ledger names no run id to +# fetch one with, so its only delivery evidence is the row's own PR URL: +# a COMPLETED row carrying one proves push and pr ran and reads done, +# while a bare COMPLETED row proves nothing either way and reads +# unknown-unverified rather than validated. # 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 @@ -605,6 +607,7 @@ HAVE_RUN=0 # the TOON field parsing entirely for this crew. RUN_SOURCE=full COARSE_STATUS="" +COARSE_PR="" # Scouts and secondmates never drive a no-mistakes validation of their own # worktree, so skip the lookup for them and read state from pane/log directly. if [ "$KIND" = ship ] && [ -n "$CREW_BRANCH" ] && command -v no-mistakes >/dev/null 2>&1; then @@ -625,7 +628,11 @@ if [ "$KIND" = ship ] && [ -n "$CREW_BRANCH" ] && command -v no-mistakes >/dev/n # `[ -n "$RUN_OUT" ]`: an empty/timed-out primary call means the CLI # itself did not respond, so retrying it immediately with a second # bounded call would just double the wait for no better answer. - COARSE_STATUS=$(fm_nm_runs_status_for_worktree "$WT" "$CREW_BRANCH" "$(nm_runs_list)") + coarse_row=$(fm_nm_runs_status_for_worktree "$WT" "$CREW_BRANCH" "$(nm_runs_list)") + COARSE_STATUS=${coarse_row%% *} + case "$coarse_row" in + *' '*) COARSE_PR=${coarse_row#* } ;; + esac if [ -n "$COARSE_STATUS" ]; then HAVE_RUN=1 # A branch-matching answer the strict rule rejected is this branch's @@ -659,18 +666,21 @@ if [ "$HAVE_RUN" = 1 ]; then case "$COARSE_STATUS" in running) RUN_STATE=working; RUN_DETAIL="validating (background run)" ;; completed) - # Symmetric to the failed row below: a terminal ledger word is not a - # verdict here. The vacuous-pass shape the full path refuses + # A terminal ledger word is not a verdict on its own here: the + # vacuous-pass shape the full path refuses # (nm_run_skipped_every_mandatory_step) is recorded `completed` too, - # and this path has no steps table to tell the two apart - the runs - # ledger carries no run id, so no per-run lookup can supply one - # either. Reporting done would accept as validated exactly the run - # that validated nothing, so the record is reported unverified and - # the crew's own done/failed status-log line stays the only thing - # that can conclude it. - RUN_STATE=unknown - RUN_DETAIL="last ledger record completed; no steps table to prove any delivery phase ran - unverified" - ;; + # and this path has neither a steps table nor a run id to fetch one + # with. The row's PR URL is the ledger's own positive delivery + # evidence - a run that skipped push and pr has none - so a completed + # row that carries one is a real delivery and keeps its done verdict, + # while a bare completed row cannot be told from the vacuous shape + # and is reported unverified rather than validated. + if [ -n "$COARSE_PR" ]; then + RUN_STATE="done"; RUN_DETAIL="run completed: $COARSE_PR" + else + RUN_STATE=unknown + RUN_DETAIL="last ledger record completed with no PR; nothing proves a delivery phase ran - unverified" + fi ;; failed) # The ledger row is terminal but the coarse path has no steps table # and no ci log, so the orphaned-monitor shape cannot be recognized diff --git a/bin/fm-nm-run-lib.sh b/bin/fm-nm-run-lib.sh index 9a04004e391..19000c85b38 100644 --- a/bin/fm-nm-run-lib.sh +++ b/bin/fm-nm-run-lib.sh @@ -126,13 +126,27 @@ fm_nm_run_is_pipeline_owned_active() { # fm_nm_run_is_active "$1" } +# The printed form of an attributed ledger row: the status word, plus the row's +# PR URL when it has one. +fm_nm_print_runs_row() { # + if [ -n "${2:-}" ]; then + printf '%s %s' "$1" "$2" + else + printf '%s' "$1" + fi +} + # ONE owner for attribution from the pipeline's own runs ledger, replacing a # per-row scan-and-skip. The ledger is the real top-level `no-mistakes runs # --limit N` listing (plain text, no run id, no quoting, newest-first, columns # " []"; the `axi` surface has no # runs-listing subcommand - verified against the installed CLI). Prints the -# status word of the branch's CURRENT run row, or nothing when the ledger -# cannot prove attribution. When optional expected head $4 is supplied, its +# branch's CURRENT run row as "[ ]" - the status word alone +# when the row carries no PR column - or nothing when the ledger cannot prove +# attribution. Callers that only want the word read the first field. The PR +# column is carried because it is the ledger's ONLY positive evidence that a +# row's run actually pushed a branch and opened a PR, which no status word can +# establish on its own. When optional expected head $4 is supplied, its # abbreviated commit identity must match the newest row. The branch's NEWEST # row alone decides; older rows are history and never answer for the present: # - newest row's head resolves and matches the worktree (fm_nm_head_matches_worktree): @@ -153,7 +167,7 @@ fm_nm_run_is_pipeline_owned_active() { # # Read-only: git reads resolve objects in place; custody never changes. fm_nm_runs_status_for_worktree() { # [expected-head] local wt=$1 branch=$2 list=$3 expected_head=${4:-} - local local_full row st br sha day clock pr extra year_num month_num day_num max_day pending_st='' + local local_full row st br sha day clock pr extra year_num month_num day_num max_day pending_st='' pending_pr='' local_full=$(git -C "$wt" rev-parse HEAD 2>/dev/null) || return 0 [ -n "$list" ] || return 0 while IFS= read -r row; do @@ -191,7 +205,7 @@ fm_nm_runs_status_for_worktree() { # [ex # the only admissible anchor, and only exact head equality proves the # worktree still sits at the submitted head. if [ "$(fm_nm_resolve_commit "$wt" "$sha")" = "$local_full" ]; then - printf '%s' "$pending_st" + fm_nm_print_runs_row "$pending_st" "$pending_pr" fi return 0 fi @@ -205,12 +219,13 @@ fm_nm_runs_status_for_worktree() { # [ex fi if [ -n "$(fm_nm_resolve_commit "$wt" "$sha")" ]; then if fm_nm_head_matches_worktree "$wt" "$sha"; then - printf '%s' "$st" + fm_nm_print_runs_row "$st" "$pr" fi return 0 fi [ "$st" = running ] || return 0 pending_st=$st + pending_pr=$pr done <<< "$list" return 0 } diff --git a/bin/fm-teardown.sh b/bin/fm-teardown.sh index fd2db419806..905a5a1266d 100755 --- a/bin/fm-teardown.sh +++ b/bin/fm-teardown.sh @@ -1718,7 +1718,7 @@ NM_TEARDOWN_RUNS_LIMIT=${FM_TEARDOWN_NM_RUNS_LIMIT:-200} case "$NM_TEARDOWN_RUNS_LIMIT" in ''|*[!0-9]*) NM_TEARDOWN_RUNS_LIMIT=200 ;; esac TASK_RUN_ID= task_status_is_own_parked_run() { # - local wt=$1 out=$2 branch run_id run_branch run_head status outcome awaiting has_gate ledger + local wt=$1 out=$2 branch run_id run_branch run_head status outcome awaiting has_gate ledger ledger_row TASK_RUN_ID= branch=$(git -C "$wt" symbolic-ref --quiet --short HEAD 2>/dev/null) || return 1 [ -n "$branch" ] || return 1 @@ -1751,7 +1751,8 @@ task_status_is_own_parked_run() { # [ -n "$run_head" ] || return 1 [ -z "$(fm_nm_resolve_commit "$wt" "$run_head")" ] || return 1 ledger=$(fm_nm_run "$wt" "$NM_TEARDOWN_TIMEOUT" runs --limit "$NM_TEARDOWN_RUNS_LIMIT") - [ "$(fm_nm_runs_status_for_worktree "$wt" "$branch" "$ledger" "$run_head")" = running ] || return 1 + ledger_row=$(fm_nm_runs_status_for_worktree "$wt" "$branch" "$ledger" "$run_head") + [ "${ledger_row%% *}" = running ] || return 1 fi awaiting=$(printf '%s\n' "$out" | grep -E '^[[:space:]]*awaiting_agent:' | head -1 || true) has_gate=$(printf '%s\n' "$out" | grep -Eq '^[[:space:]]*gate:[[:space:]]*' && echo 1 || echo 0) diff --git a/tests/fm-crew-state.test.sh b/tests/fm-crew-state.test.sh index a3fabeaee4e..200e9d5de51 100755 --- a/tests/fm-crew-state.test.sh +++ b/tests/fm-crew-state.test.sh @@ -1136,10 +1136,12 @@ test_completed_run_with_partial_skip_still_reads_done() { # reach: crew A's run ended in the vacuous empty-diff shape, and crew B then # started a run on the same repo, so the shared daemon's bare `axi status` # answers with B's branch. A's state read therefore falls to the coarse runs -# ledger, whose newest row for A's branch is `completed` at A's own head - and -# that ledger carries no steps table and no run id, so nothing here can prove -# any delivery phase ran. It must not be reported as a validated done. -test_coarse_completed_ledger_row_is_not_reported_done() { +# ledger, whose newest row for A's branch is `completed` at A's own head. That +# row carries no PR, because the vacuous run skipped push and pr, and the +# ledger offers no steps table and no run id to check any further - so nothing +# here proves a delivery phase ran and the row must not read as a validated +# done. +test_coarse_completed_ledger_row_without_pr_is_not_reported_done() { reset_fakes local d short; d=$(new_case coarse-completed-unverified) make_repo_on_branch "$d/wt" fm/feat-coarse-completed @@ -1155,11 +1157,38 @@ EOF )" local out; out=$(run_crew_state "$d" feat-coarse-completed) assert_not_contains "$out" "state: done" \ - "a coarse completed row cannot prove the run validated anything" + "a coarse completed row with no PR cannot prove the run validated anything" assert_contains "$out" "state: unknown" "an unprovable terminal record reads unknown" assert_contains "$out" "unverified" "the detail must say the record is unverified" assert_contains "$out" "source: run-step" "the ledger row is still this branch's attributed run" - pass "coarse completed ledger row reads unverified, never done" + pass "coarse completed ledger row without a PR reads unverified, never done" +} + +# The same two-crew coarse fallback for a run that really delivered: crew A ran +# every phase and opened its PR, so its ledger row carries the PR URL - the +# ledger's own positive proof that push and pr executed, which the vacuous +# shape can never have. That terminal outcome must still reach the captain as +# done, whether or not crew A managed to append its own `done:` status line. +test_coarse_completed_ledger_row_with_pr_still_reads_done() { + reset_fakes + local d short; d=$(new_case coarse-completed-delivered) + make_repo_on_branch "$d/wt" fm/feat-coarse-delivered + short=$(git -C "$d/wt" rev-parse --short=7 HEAD) + make_fakebin "$d" >/dev/null + fm_write_meta "$d/state/feat-coarse-delivered.meta" \ + "window=fm:fm-feat-coarse-delivered" "worktree=$d/wt" "kind=ship" + FM_FAKE_AXI_STATUS="$(run_running fm/other-crew)" + FM_FAKE_RUNS_LIST="$(cat < Date: Tue, 8 Sep 2026 07:05:01 +0200 Subject: [PATCH 04/18] no-mistakes(review): drop unreachable pending_pr plumbing from ledger continuation path --- bin/fm-nm-run-lib.sh | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/bin/fm-nm-run-lib.sh b/bin/fm-nm-run-lib.sh index 19000c85b38..c6bf2ca7d54 100644 --- a/bin/fm-nm-run-lib.sh +++ b/bin/fm-nm-run-lib.sh @@ -126,16 +126,6 @@ fm_nm_run_is_pipeline_owned_active() { # fm_nm_run_is_active "$1" } -# The printed form of an attributed ledger row: the status word, plus the row's -# PR URL when it has one. -fm_nm_print_runs_row() { # - if [ -n "${2:-}" ]; then - printf '%s %s' "$1" "$2" - else - printf '%s' "$1" - fi -} - # ONE owner for attribution from the pipeline's own runs ledger, replacing a # per-row scan-and-skip. The ledger is the real top-level `no-mistakes runs # --limit N` listing (plain text, no run id, no quoting, newest-first, columns @@ -167,7 +157,7 @@ fm_nm_print_runs_row() { # # Read-only: git reads resolve objects in place; custody never changes. fm_nm_runs_status_for_worktree() { # [expected-head] local wt=$1 branch=$2 list=$3 expected_head=${4:-} - local local_full row st br sha day clock pr extra year_num month_num day_num max_day pending_st='' pending_pr='' + local local_full row st br sha day clock pr extra year_num month_num day_num max_day pending_st='' local_full=$(git -C "$wt" rev-parse HEAD 2>/dev/null) || return 0 [ -n "$list" ] || return 0 while IFS= read -r row; do @@ -205,7 +195,7 @@ fm_nm_runs_status_for_worktree() { # [ex # the only admissible anchor, and only exact head equality proves the # worktree still sits at the submitted head. if [ "$(fm_nm_resolve_commit "$wt" "$sha")" = "$local_full" ]; then - fm_nm_print_runs_row "$pending_st" "$pending_pr" + printf '%s' "$pending_st" fi return 0 fi @@ -219,13 +209,16 @@ fm_nm_runs_status_for_worktree() { # [ex fi if [ -n "$(fm_nm_resolve_commit "$wt" "$sha")" ]; then if fm_nm_head_matches_worktree "$wt" "$sha"; then - fm_nm_print_runs_row "$st" "$pr" + if [ -n "$pr" ]; then + printf '%s %s' "$st" "$pr" + else + printf '%s' "$st" + fi fi return 0 fi [ "$st" = running ] || return 0 pending_st=$st - pending_pr=$pr done <<< "$list" return 0 } From 5904a74ad14294e2f73822e131f141713c0785c6 Mon Sep 17 00:00:00 2001 From: Alex William Date: Tue, 8 Sep 2026 07:28:09 +0200 Subject: [PATCH 05/18] no-mistakes(document): document vacuous-run and coarse PR-evidence crew-state rules --- AGENTS.md | 2 +- docs/architecture.md | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index ca09ab7a4cf..3372672d58e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -370,7 +370,7 @@ Require the matching `resolved` event, forbid `--yes`, and require the worker to Resume fleet supervision immediately after the decision lands. Judge validation by the currently attributed run step through `bin/fm-crew-state.sh`, not by shell liveness or the last status event. -Running, fixing, or CI states remain working; parked approval or fix-review states require the worker to follow the active gate help; passed or checks-passed is done; failed or cancelled is failed exactly as `bin/fm-crew-state.sh` prints it - only that state line reclassifies an orphaned ci monitor after green checks as held-for-merge done, or a terminal failed record with the daemon unreachable as unknown, never the raw run record. +Running, fixing, or CI states remain working; parked approval or fix-review states require the worker to follow the active gate help; passed or checks-passed is done; failed or cancelled is failed exactly as `bin/fm-crew-state.sh` prints it - only that state line reclassifies an orphaned ci monitor after green checks as held-for-merge done, a terminal failed record with the daemon unreachable as unknown, or a successful run that skipped every mandatory delivery phase as failed, never the raw run record. A worker hand-editing, committing, aborting, or restarting during an active validation run duplicates pipeline ownership outside the supersession sequence above; steer it back to the gate response flow. The worker reports the PR when CI first becomes green rather than waiting for merge monitoring to finish. diff --git a/docs/architecture.md b/docs/architecture.md index 58b900786d4..620018285e9 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -82,7 +82,9 @@ A run head the task copy cannot resolve locally is attributed only when the pipe During no-mistakes' `ci` monitor phase, it also reads the ci step log tail because `axi status` reports both "still waiting on checks" and "checks green, waiting on merge" as `ci,running`. The most recent recognized ci log marker wins, so checks-green monitoring reports done while a later re-arm, failed-check, or issue marker returns the crew to working. A terminal failed run whose only failure is the ci monitor step, after every substantive step completed and the same marker reads checks green, also reports done with the run's PR URL, because a monitor whose only remaining job is to observe a human merge decision must not convert the absence of that decision into a failure verdict. +Conversely a terminal successful run whose steps table shows every mandatory delivery phase - review, test, document, lint, push, pr, and ci - skipped reports failed rather than done, because a branch whose whole diff vanished into the rebase base is still recorded completed although nothing was reviewed, tested, or delivered, and that word alone is not validation. In the coarse runs-ledger fallback, which has no steps table and no ci log, a terminal failed record whose daemon an explicit `daemon status` probe proves down reports unknown as unverified instead: an instrument failure must never read as work failure. +That fallback equally cannot inspect a completed record's phases, so the ledger row's own PR URL is its only positive delivery evidence: a completed row carrying one reports done, while a bare completed row reports unknown as unverified rather than validated. Only when no matching run exists does it consult semantic busy state; exact busy reports working, exact idle permits fallback to a status-log event whose verb maps to a recognized run-state, and unknown or a dead pane stays unknown instead of trusting a stale log. Decision-only events such as `resolved` never become current state or leak their prose into the current-state detail. In that status-log fallback, a declared external wait reports the distinct `paused` state with its reason. From ea80b74b85f96e3ec6c22e113d7508f4af8e25f9 Mon Sep 17 00:00:00 2001 From: Alex William Date: Mon, 24 Aug 2026 17:35:57 +0200 Subject: [PATCH 06/18] fix(bin): omit a GitHub merge strategy for merge-queue branches GitHub merge-queue rulesets refuse any explicit merge method, and the unguarded --squash default made the guarded merge path unusable on those branches. --method=queue and --no-method now skip that default without forwarding a strategy, matching the GitLab "let the project decide" rule. A queued pull request stays OPEN until it lands, so the merge poll and teardown still wait for MERGED rather than treating enqueue as landing. --- bin/fm-pr-merge.sh | 39 +++++++++++- bin/fm-pr-poll.sh | 4 +- bin/fm-teardown.sh | 1 + docs/architecture.md | 3 +- docs/documentation-audiences.json | 4 ++ docs/verification/github-merge-queue.md | 83 +++++++++++++++++++++++++ tests/fm-pr-check-security.test.sh | 2 +- tests/fm-pr-merge.test.sh | 56 +++++++++++++++++ tests/fm-teardown.test.sh | 51 +++++++++++++++ 9 files changed, 237 insertions(+), 6 deletions(-) create mode 100644 docs/verification/github-merge-queue.md diff --git a/bin/fm-pr-merge.sh b/bin/fm-pr-merge.sh index 3e61b33f7bc..6cc697eaeb1 100755 --- a/bin/fm-pr-merge.sh +++ b/bin/fm-pr-merge.sh @@ -7,7 +7,11 @@ # host and path, so any instance works and no host is hardcoded. # # Merge method on GitHub defaults to --squash when the caller passes none of -# --squash, --merge, --rebase, or --method after the optional -- separator. +# --squash, --merge, --rebase, --method, or --no-method after the optional -- +# separator. --method=queue, --method queue, and --no-method count as a method +# so that default is skipped, then they are dropped rather than forwarded: a +# GitHub merge-queue branch refuses any explicit strategy, and gh-axi rejects +# --method=queue. # The gh-axi merge abstraction always performs the merge; the outcome read that # follows it never becomes a prerequisite for reaching that abstraction. After # gh-axi returns success, GitHub's live state is read back and accepted only @@ -107,7 +111,7 @@ caller_has_merge_method() { local arg for arg in "$@"; do case "$arg" in - --squash|--merge|--rebase|--method|--method=*) return 0 ;; + --squash|--merge|--rebase|--method|--method=*|--no-method) return 0 ;; esac done return 1 @@ -154,6 +158,33 @@ caller_requested_auto_merge() { return "$requested" } +# GitHub-only: drop tokens that mean "let the forge choose the method". +# They already satisfy caller_has_merge_method so the default --squash is not +# added. They are not GitHub merge strategies and must not reach gh-axi. +GITHUB_MERGE_FORWARD=() +github_drop_forge_decides_method() { + GITHUB_MERGE_FORWARD=() + while [ "$#" -gt 0 ]; do + case "$1" in + --no-method|--method=queue) + shift + ;; + --method) + if [ "${2-}" = queue ]; then + shift 2 + else + GITHUB_MERGE_FORWARD+=("$1") + shift + fi + ;; + *) + GITHUB_MERGE_FORWARD+=("$1") + shift + ;; + esac + done +} + reject_repo_overrides() { local arg for arg in "$@"; do @@ -639,8 +670,10 @@ case "$PROVIDER" in FM_PR_GITHUB_AUTO_REQUESTED=true fi FM_PR_GITHUB_CALLER_METHOD=$(caller_merge_method "$@") + github_drop_forge_decides_method "$@" if merge_output=$(gh-axi pr merge "$PR_NUMBER" --repo "$PR_OWNER/$PR_REPO" \ - "${merge_args[@]+"${merge_args[@]}"}" "$@" 2>&1); then + "${merge_args[@]+"${merge_args[@]}"}" \ + "${GITHUB_MERGE_FORWARD[@]+"${GITHUB_MERGE_FORWARD[@]}"}" 2>&1); then FM_PR_GITHUB_MERGE_ACCEPTED=true else merge_status=$? diff --git a/bin/fm-pr-poll.sh b/bin/fm-pr-poll.sh index ed705ce7073..7db2de996b0 100755 --- a/bin/fm-pr-poll.sh +++ b/bin/fm-pr-poll.sh @@ -2,7 +2,9 @@ # Static watcher program for a validated PR/MR poll sidecar. # It emits exactly one merged line for a merged PR or MR and stays silent # otherwise, including on every error, so a failed lookup can never be read as -# a merge. The provider-tagged identity is data in the sidecar and is never +# a merge. A GitHub pull request in a merge queue stays OPEN until it lands, +# so this poll waits; queue membership is not a merge. The provider-tagged +# identity is data in the sidecar and is never # interpolated into this source: these bytes are identical for every task. # Each provider is read through its own standard CLI, gh for GitHub and glab # for GitLab, so an upstream checkout needs no extra tooling to follow either. diff --git a/bin/fm-teardown.sh b/bin/fm-teardown.sh index 905a5a1266d..d4de04bc405 100755 --- a/bin/fm-teardown.sh +++ b/bin/fm-teardown.sh @@ -1321,6 +1321,7 @@ pr_is_merged() { resolved_url=${remainder#*$'\t'} [ "$head" != "$remainder" ] || return 1 case "$state" in + # OPEN includes a merge-queue enqueue: landing is MERGED only. MERGED|merged) ;; *) return 1 ;; esac diff --git a/docs/architecture.md b/docs/architecture.md index 620018285e9..2ea66f74228 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -307,7 +307,8 @@ Where a no-mistakes pipeline stores evidence in the repo, it publishes that PR-v This repo uses that setting, and its own `.no-mistakes/` directory remains local state that stays gitignored and is rejected by CI if tracked; [`configuration.md`](configuration.md) owns the setting. PR-based task merges go through `bin/fm-pr-merge.sh`, which records `pr=` and any available `pr_head=` through `bin/fm-pr-check.sh` before calling the forge CLI. The helper requires a full canonical URL and rejects malformed URLs or repo override flags before recording merge state. -A `https://github.com///pull/` URL invokes `gh-axi pr merge --repo /`, defaults to `--squash`, and preserves explicit merge-method flags. +A `https://github.com///pull/` URL invokes `gh-axi pr merge --repo /` with the merge-method rules owned by that helper's header, including an explicit forge-decides method for merge-queue branches. +A GitHub merge-queue enqueue leaves the pull request open until it lands; the merge poll and teardown wait for a merged state, with empirical queue-state evidence in [`docs/verification/github-merge-queue.md`](verification/github-merge-queue.md). A `https:////-/merge_requests/` URL (see [docs/gitlab-merge-watch.md](gitlab-merge-watch.md)) invokes `glab mr merge -R https:///`, so the instance comes from the URL, and adds no merge-method flag because the project's own merge method applies. That path merges only after one live read of the merge request confirms it is open, mergeable, conflict-free, with blocking discussions resolved and a successful pipeline at the current head, and it binds the merge to that verified head; recorded metadata is never the authority for those conditions because a rebase leaves it stale. After either forge command returns, the script confirms the PR or MR actually landed, and only a confirmed landing records a landed outcome; a queued or unconfirmed request records none and leaves its poll armed. diff --git a/docs/documentation-audiences.json b/docs/documentation-audiences.json index 4bd14979314..0768750b2c7 100644 --- a/docs/documentation-audiences.json +++ b/docs/documentation-audiences.json @@ -436,6 +436,10 @@ "path": "docs/verification/dispatch-auth.md", "audience": "maintainer-verification" }, + { + "path": "docs/verification/github-merge-queue.md", + "audience": "maintainer-verification" + }, { "path": "docs/verification/grok-interrupt.md", "audience": "maintainer-verification" diff --git a/docs/verification/github-merge-queue.md b/docs/verification/github-merge-queue.md new file mode 100644 index 00000000000..a24b120a21a --- /dev/null +++ b/docs/verification/github-merge-queue.md @@ -0,0 +1,83 @@ +# GitHub merge-queue merge verification + +Audience: maintainer verification. + +This record supports the current guarantee that a GitHub merge-queue enqueue is not a landing. +`bin/fm-pr-merge.sh` can omit a merge strategy so the forge chooses the method, `bin/fm-pr-poll.sh` stays silent until the pull request is merged, and `bin/fm-teardown.sh` refuses while the pull request is still open. +The merge-method flags themselves are owned by the `bin/fm-pr-merge.sh` header. + +No public pull request was in a merge queue at collection time, so queue membership was read from live GraphQL schema enums plus GitHub's documented CLI enqueue behavior, and the poll-relevant `state` field was read from a live open pull request. + +## Environment + +Recorded 2026-08-24 on GNU bash 5.2.37(1)-release (x86_64-pc-linux-gnu). + +``` +$ gh --version +gh version 2.46.0 (2025-01-13 Debian 2.46.0-3) +https://github.com/cli/cli/releases/tag/v2.46.0 + +$ gh-axi --version +0.1.33 +``` + +## Pull request state is not the queue entry state + +Live GraphQL introspection of `PullRequestState` has only three values. +`QUEUED` and `AWAITING_CHECKS` are not pull request states. + +``` +$ gh api graphql -f query='query { __type(name: "PullRequestState") { enumValues { name description } } }' +{"data":{"__type":{"enumValues":[{"name":"OPEN","description":"A pull request that is still open."},{"name":"CLOSED","description":"A pull request that has been closed without being merged."},{"name":"MERGED","description":"A pull request that has been closed by being merged."}]}}} +``` + +Live introspection of `MergeQueueEntryState` is the queue-entry machine the brief named. + +``` +$ gh api graphql -f query='query { __type(name: "MergeQueueEntryState") { enumValues { name description } } }' +{"data":{"__type":{"enumValues":[{"name":"QUEUED","description":"The entry is currently queued."},{"name":"AWAITING_CHECKS","description":"The entry is currently waiting for checks to pass."},{"name":"MERGEABLE","description":"The entry is currently mergeable."},{"name":"UNMERGEABLE","description":"The entry is currently unmergeable."},{"name":"LOCKED","description":"The entry is currently locked."}]}}} +``` + +The merge poll and the teardown landed-work check both read `gh pr view --json state`. +That CLI, at this `gh` version, does not expose `isInMergeQueue` or `mergeQueueEntry`. + +``` +$ gh pr view https://github.com/microsoft/TypeScript/pull/63931 --json isInMergeQueue +Unknown JSON field: "isInMergeQueue" +``` + +On a live open pull request, the poll-relevant fields stay `OPEN` with a null merge time, which is the same `state` a queued pull request keeps until it lands. + +``` +$ gh pr view https://github.com/microsoft/TypeScript/pull/63931 --json state,mergedAt,mergeStateStatus,url +{"mergeStateStatus":"BLOCKED","mergedAt":null,"state":"OPEN","url":"https://github.com/microsoft/TypeScript/pull/63931"} + +$ gh api graphql -f query='query { repository(owner:"microsoft", name:"TypeScript") { pullRequest(number:63931) { url state merged mergedAt mergeStateStatus isInMergeQueue mergeQueueEntry { state position } } } }' +{"data":{"repository":{"pullRequest":{"url":"https://github.com/microsoft/TypeScript/pull/63931","state":"OPEN","merged":false,"mergedAt":null,"mergeStateStatus":"BLOCKED","isInMergeQueue":false,"mergeQueueEntry":null}}}} +``` + +## Enqueue is not a merge + +`gh pr merge --help` on this CLI version states that a merge-queue target needs no strategy, enables auto-merge when checks have not passed, and adds the pull request to the queue when they have. + +``` +When targeting a branch that requires a merge queue, no merge strategy is required. +If required checks have not yet passed, auto-merge will be enabled. +If required checks have passed, the pull request will be added to the merge queue. +``` + +Passing an explicit strategy is what GitHub rejects on those branches, with the refusal `The merge strategy for is set by the merge queue`. + +`gh-axi` 0.1.33 accepts `--method` only as `merge`, `squash`, or `rebase`, so `--method=queue` must be consumed by `bin/fm-pr-merge.sh` and not forwarded. + +## Portable regressions + +```sh +bin/fm-test-run.sh tests/fm-pr-merge.test.sh +bin/fm-test-run.sh tests/fm-pr-check-security.test.sh +bin/fm-test-run.sh tests/fm-teardown.test.sh +``` + +The merge tests prove `--method=queue`, `--method queue`, and `--no-method` invoke `gh-axi pr merge` with no strategy flag. +The poll contract stays silent for `OPEN`, `QUEUED`, and `AWAITING_CHECKS`, and emits `merged` only for `MERGED`. +Teardown refuses an open pull request whose commits are not otherwise landed. diff --git a/tests/fm-pr-check-security.test.sh b/tests/fm-pr-check-security.test.sh index e5e3eb09ede..5b0479b28f1 100755 --- a/tests/fm-pr-check-security.test.sh +++ b/tests/fm-pr-check-security.test.sh @@ -687,7 +687,7 @@ test_static_poll_contract() { dir=$(make_case poll-contract) make_poll_fixture "$dir" - for state in OPEN CLOSED EMPTY MALFORMED; do + for state in OPEN CLOSED QUEUED AWAITING_CHECKS EMPTY MALFORMED; do case "$state" in EMPTY) value= ;; MALFORMED) value='not-a-state' ;; diff --git a/tests/fm-pr-merge.test.sh b/tests/fm-pr-merge.test.sh index cfb02b92197..6550ae26e92 100755 --- a/tests/fm-pr-merge.test.sh +++ b/tests/fm-pr-merge.test.sh @@ -13,6 +13,8 @@ # (e) PR URL is parsed to number + --repo for gh-axi (defaults to --squash) # (f) malformed PR URL fails fast without calling gh-axi # (g) explicit merge method is not overridden by the default --squash +# (g2) --method=queue, --method queue, and --no-method skip default --squash +# and forward no strategy flag, so a merge-queue branch can choose # (h) repo override args fail fast because the repo comes from the URL, # including a bundled short-option cluster that carries -R # (i) a GitLab MR URL resolves and merges through glab instead of erroring @@ -1453,6 +1455,58 @@ test_method_equals_merge_method_not_overridden() { pass "fm-pr-merge respects --method= as an explicit merge method" } +# Assert the GitHub CLI was asked to merge without imposing a strategy, so a +# merge-queue branch can apply the project's own method. +assert_github_merge_has_no_strategy() { + local log=$1 label=$2 line flag + line=$(grep -F 'pr merge ' "$log" || true) + [ -n "$line" ] || fail "$label: gh-axi pr merge was not invoked" + for flag in --squash --rebase --merge --method --no-method; do + case "$line" in + *"$flag"*) fail "$label: '$flag' was imposed on GitHub: '$line'" ;; + esac + done +} + +test_forge_decides_method_omits_strategy() { + local case_dir spelling + for spelling in \ + 'method-equals-queue|--method=queue' \ + 'method-queue|--method queue' \ + 'no-method|--no-method' + do + case_dir=$(make_case "forge-decides-${spelling%%|*}") + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee + : > "$case_dir/gh-axi.log" + + # shellcheck disable=SC2086 # The spelling is one or two extra merge flags. + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/24 -- ${spelling#*|} \ + > "$case_dir/stdout" 2> "$case_dir/stderr" \ + || fail "forge-decides-${spelling%%|*}: fm-pr-merge failed" + + grep -qxF 'pr merge 24 --repo example/repo' "$case_dir/gh-axi.log" \ + || fail "forge-decides-${spelling%%|*}: expected merge with no strategy, got '$(cat "$case_dir/gh-axi.log")'" + assert_github_merge_has_no_strategy "$case_dir/gh-axi.log" "forge-decides-${spelling%%|*}" + done + pass "fm-pr-merge omits a GitHub strategy when the caller asks the forge to decide" +} + +test_forge_decides_method_forwards_other_flags() { + local case_dir + case_dir=$(make_case forge-decides-extra-flags) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" ffffffffffffffffffffffffffffffffffffffff + : > "$case_dir/gh-axi.log" + + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/25 -- --method=queue --delete-branch \ + > "$case_dir/stdout" 2> "$case_dir/stderr" || fail "forge-decides-extra-flags: fm-pr-merge failed" + + grep -qxF 'pr merge 25 --repo example/repo --delete-branch' "$case_dir/gh-axi.log" \ + || fail "forge-decides-extra-flags: extra flags were not forwarded after dropping the forge-decides method" + pass "fm-pr-merge still forwards non-strategy GitHub flags after a forge-decides method" +} + test_parses_pr_url_for_gh_axi() { local case_dir case_dir=$(make_case url-parsing) @@ -2118,6 +2172,8 @@ test_repo_override_args_refuse_before_recording test_bundled_repo_override_args_refuse_before_recording test_explicit_merge_method_not_overridden test_method_equals_merge_method_not_overridden +test_forge_decides_method_omits_strategy +test_forge_decides_method_forwards_other_flags test_parses_pr_url_for_gh_axi test_github_still_forwards_sha_arg test_gitlab_url_resolves_and_merges diff --git a/tests/fm-teardown.test.sh b/tests/fm-teardown.test.sh index c5c274b7d8e..d56865243b4 100755 --- a/tests/fm-teardown.test.sh +++ b/tests/fm-teardown.test.sh @@ -32,6 +32,7 @@ # (i) no-mistakes + dirty worktree, even when work landed -> REFUSE (dirty wins) # (j) no-mistakes + gh lookup errors + content not in default -> REFUSE (fail-safe) # (k) no-mistakes + merged PR but HEAD moved afterward -> REFUSE (stale PR) +# (k2) no-mistakes + OPEN PR (merge-queue enqueue) -> REFUSE (not landed) # (l) no-mistakes + stale origin/main but fetched content -> ALLOW (fresh fetch) # (m) no-mistakes + local HEAD ancestor of merged PR head -> ALLOW (lagging local) # (n) no-mistakes + replayed unpushed patch in merged PR head -> ALLOW (replayed local) @@ -361,6 +362,34 @@ assert_refusal_retained_task_state() { || fail "$label: refusal moved the task branch off the unlanded commit" [ -e "$case_dir/state/task-x1.meta" ] \ || fail "$label: refusal erased the durable task record" +# Override GitHub lookups to report PR 7 as still open with the supplied head. +# A merge-queue enqueue leaves the pull request OPEN until it actually lands. +add_gh_pr_open_for_head() { + local case_dir=$1 head=$2 + cat > "$case_dir/fakebin/gh-axi" <<'SH' +#!/usr/bin/env bash +case "${1:-} ${2:-}" in + "pr list") + printf '%s\n' "count: 1 (showing first 1)" "pull_requests[1]{number,state}:" " 7,open" ; exit 0 ;; + "pr view") + printf '%s\n' "pull_request:" " number: 7" " state: open" ; exit 0 ;; +esac +exit 0 +SH + cat > "$case_dir/fakebin/gh" <&2 +exit 1 +SH + chmod +x "$case_dir/fakebin/gh-axi" "$case_dir/fakebin/gh" } append_pr_meta_for_current_head() { @@ -923,6 +952,27 @@ test_squash_merged_pr_allows_replayed_unpushed_patch() { pass "squash-merged PR accepts replayed unpushed local patches contained in the PR head" } +test_open_pr_does_not_count_as_landed() { + local case_dir rc pr_head + case_dir=$(make_case open-pr-unlanded) + write_meta "$case_dir" no-mistakes ship + # Same unpushed branch content as the squash-merged allow case, but the PR is + # still OPEN: that is the merge-queue enqueue state, not a landing. + wt_commit_file "$case_dir" feature.txt hello "add feature" + append_pr_meta_for_current_head "$case_dir" + pr_head=$(git -C "$case_dir/wt" rev-parse HEAD) + add_gh_pr_open_for_head "$case_dir" "$pr_head" + + set +e + run_teardown "$case_dir" > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "open-pr-unlanded: teardown should refuse while the PR is still open" + grep -q REFUSED "$case_dir/stderr" || fail "open-pr-unlanded: no REFUSED line in stderr" + pass "open PR, including a merge-queue enqueue, does not count as landed" +} + test_merged_pr_with_later_local_commit_refuses() { local case_dir rc pr_head case_dir=$(make_case stale-pr-head) @@ -3674,6 +3724,7 @@ test_squash_merged_branch_deleted_allows test_squash_merged_pr_allows_when_head_ancestor_of_pr_head test_no_pr_recorded_discovers_merged_pr_by_branch_allows test_squash_merged_pr_allows_replayed_unpushed_patch +test_open_pr_does_not_count_as_landed test_merged_pr_with_later_local_commit_refuses test_squash_merged_rebased_branch_allows test_squash_merged_same_file_different_content_refuses From 62a84540df5e527cbc4d88a7f96a832902f6a9cb Mon Sep 17 00:00:00 2001 From: Alex William Date: Mon, 24 Aug 2026 17:59:44 +0200 Subject: [PATCH 07/18] no-mistakes(review): report queued GitHub merges and refuse forge-decides flags on GitLab --- bin/fm-pr-merge.sh | 19 ++++ docs/verification/github-merge-queue.md | 24 +++- tests/fm-pr-merge.test.sh | 141 +++++++++++++++++++++++- tests/fm-teardown.test.sh | 37 ++++--- 4 files changed, 202 insertions(+), 19 deletions(-) diff --git a/bin/fm-pr-merge.sh b/bin/fm-pr-merge.sh index 6cc697eaeb1..a4eaa0e5679 100755 --- a/bin/fm-pr-merge.sh +++ b/bin/fm-pr-merge.sh @@ -44,6 +44,8 @@ # GitLab adds no method flag at all: its merge method is the project's own # setting, which the merge API applies, and imposing squash there would override # that convention rather than mirror the GitHub default. +# Those same forge-decides tokens are refused up front on GitLab, where no glab +# flag spells them. # # A GitLab merge is refused unless every pre-merge condition holds, each read # live at merge time rather than taken from recorded metadata: the merge request @@ -185,6 +187,22 @@ github_drop_forge_decides_method() { done } +# Firstmate-level tokens that ask the forge to choose the merge method. No glab +# flag spells them, and GitLab already applies the project's own merge method, +# so the request is a no-op there: refuse it by name before anything is recorded +# rather than forward an unknown flag to glab after the merge is armed. +reject_forge_decides_method() { + local arg prev='' + for arg in "$@"; do + if [ "$arg" = --no-method ] || [ "$arg" = --method=queue ] \ + || { [ "$prev" = --method ] && [ "$arg" = queue ]; }; then + echo "error: extra merge arguments must not ask GitLab to choose the merge method, which it already does" >&2 + return 1 + fi + prev=$arg + done +} + reject_repo_overrides() { local arg for arg in "$@"; do @@ -218,6 +236,7 @@ reject_head_overrides() { reject_repo_overrides "$@" || exit 1 [ "$PROVIDER" != gitlab ] || reject_head_overrides "$@" || exit 1 +[ "$PROVIDER" != gitlab ] || reject_forge_decides_method "$@" || exit 1 # Task-derived paths are constructed only after the canonical ID validation. META="$STATE/$ID.meta" diff --git a/docs/verification/github-merge-queue.md b/docs/verification/github-merge-queue.md index a24b120a21a..68371232e84 100644 --- a/docs/verification/github-merge-queue.md +++ b/docs/verification/github-merge-queue.md @@ -3,7 +3,7 @@ Audience: maintainer verification. This record supports the current guarantee that a GitHub merge-queue enqueue is not a landing. -`bin/fm-pr-merge.sh` can omit a merge strategy so the forge chooses the method, `bin/fm-pr-poll.sh` stays silent until the pull request is merged, and `bin/fm-teardown.sh` refuses while the pull request is still open. +`bin/fm-pr-merge.sh` can omit a merge strategy so the forge chooses the method and then names the live outcome of that merge call, `bin/fm-pr-poll.sh` stays silent until the pull request is merged, and `bin/fm-teardown.sh` refuses while the pull request is still open. The merge-method flags themselves are owned by the `bin/fm-pr-merge.sh` header. No public pull request was in a merge queue at collection time, so queue membership was read from live GraphQL schema enums plus GitHub's documented CLI enqueue behavior, and the poll-relevant `state` field was read from a live open pull request. @@ -70,6 +70,26 @@ Passing an explicit strategy is what GitHub rejects on those branches, with the `gh-axi` 0.1.33 accepts `--method` only as `merge`, `squash`, or `rebase`, so `--method=queue` must be consumed by `bin/fm-pr-merge.sh` and not forwarded. +Because the merge call succeeds either way and the forge CLI labels the enqueue a merge, the same live outcome read that follows every GitHub merge names whether the pull request is merged or in the merge queue. +That read is queue-aware when `gh` is present (`isInMergeQueue`) and degrades to the gh-axi view otherwise. + +``` +$ gh pr view 63931 --repo microsoft/TypeScript --json state -q .state +OPEN + +$ gh pr view 63927 --repo microsoft/TypeScript --json state -q .state +MERGED +``` + +An outcome that is neither merged nor queued is refused. +An unreadable outcome is refused and keeps the merge poll armed. +Landing is still confirmed by the merge poll and teardown, which accept `MERGED` alone. + +## GitLab is unchanged + +`--no-method`, `--method=queue`, and `--method queue` are firstmate-level tokens that no `glab` flag defines, and GitLab already applies the project's own merge method. +They are refused by name on GitLab before any state is recorded, rather than forwarded to `glab` after the merge is armed, and a merge method the caller spells for `glab` itself still forwards untouched. + ## Portable regressions ```sh @@ -78,6 +98,6 @@ bin/fm-test-run.sh tests/fm-pr-check-security.test.sh bin/fm-test-run.sh tests/fm-teardown.test.sh ``` -The merge tests prove `--method=queue`, `--method queue`, and `--no-method` invoke `gh-axi pr merge` with no strategy flag. +The merge tests prove `--method=queue`, `--method queue`, and `--no-method` invoke `gh-axi pr merge` with no strategy flag, that the same live outcome read names a queued pull request as queued and a merged one as merged, that an unreadable state refuses rather than claiming a merge, and that those tokens are refused on GitLab before any state is recorded while a real GitLab merge method still forwards. The poll contract stays silent for `OPEN`, `QUEUED`, and `AWAITING_CHECKS`, and emits `merged` only for `MERGED`. Teardown refuses an open pull request whose commits are not otherwise landed. diff --git a/tests/fm-pr-merge.test.sh b/tests/fm-pr-merge.test.sh index 6550ae26e92..1f61a8c5ba3 100755 --- a/tests/fm-pr-merge.test.sh +++ b/tests/fm-pr-merge.test.sh @@ -15,6 +15,10 @@ # (g) explicit merge method is not overridden by the default --squash # (g2) --method=queue, --method queue, and --no-method skip default --squash # and forward no strategy flag, so a merge-queue branch can choose +# (g3) that path uses the same live outcome read as every other GitHub merge, +# so an enqueued still-open PR is named queued rather than merged +# (g4) those forge-decides tokens are refused on GitLab before any state is +# recorded, while a real GitLab merge method still forwards # (h) repo override args fail fast because the repo comes from the URL, # including a bundled short-option cluster that carries -R # (i) a GitLab MR URL resolves and merges through glab instead of erroring @@ -119,9 +123,10 @@ make_case() { } # gh-axi mock recording every invocation to a log file, and gh mock answering -# headRefOid for fm-pr-check.sh's pr_head lookup. Args: case_dir head_sha +# headRefOid for fm-pr-check.sh's pr_head lookup and, when a state is given, the +# post-merge `--json state` read. Args: case_dir head_sha [pr_state] add_gh_mocks() { - local case_dir=$1 head=$2 + local case_dir=$1 head=$2 state=${3-} cat > "$case_dir/fakebin/gh-axi" <<'SH' #!/usr/bin/env bash printf '%s\n' "$*" >> "$FM_TEST_GH_AXI_LOG" @@ -141,6 +146,9 @@ case "\${1:-} \${2:-}" in "pr view") case " \$* " in *headRefOid*) printf '%s\n' '$head' ; exit 0 ;; + *"--json state"*) + [ -n '$state' ] || exit 1 + printf '%s\n' '$state' ; exit 0 ;; esac ;; "api graphql") @@ -1507,6 +1515,131 @@ test_forge_decides_method_forwards_other_flags() { pass "fm-pr-merge still forwards non-strategy GitHub flags after a forge-decides method" } +# On a merge-queue branch the merge call enqueues the pull request and leaves it +# open, while the forge CLI still reports that as a merge. The same live outcome +# read used for every GitHub merge must name which of the two actually happened, +# and the forge-decides tokens must still omit a strategy. +test_forge_decides_reports_queued_and_merged_outcomes() { + local case_dir rc + case_dir=$(make_case forge-decides-queued) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" 1111111111111111111111111111111111111111 + write_github_outcome "$case_dir" OPEN false true master + : > "$case_dir/gh-axi.log" + : > "$case_dir/gh.log" + + set +e + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/31 -- --method=queue \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 0 "$rc" "forge-decides-queued: a queued PR should succeed" + grep -qxF 'pr merge 31 --repo example/repo' "$case_dir/gh-axi.log" \ + || fail "forge-decides-queued: expected merge with no strategy, got '$(cat "$case_dir/gh-axi.log")'" + assert_github_merge_has_no_strategy "$case_dir/gh-axi.log" "forge-decides-queued" + assert_grep 'verified: https://github.com/example/repo/pull/31 is queued' \ + "$case_dir/stdout" "forge-decides-queued: success was not reported as queued" + assert_no_grep 'merged:' "$case_dir/stdout" \ + "forge-decides-queued: an enqueued, still-open PR was confirmed as merged" + + case_dir=$(make_case forge-decides-landed) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" 1111111111111111111111111111111111111111 + write_github_outcome "$case_dir" MERGED true false main + : > "$case_dir/gh-axi.log" + : > "$case_dir/gh.log" + + set +e + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/31 -- --method=queue \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 0 "$rc" "forge-decides-landed: a merged PR should succeed" + grep -qxF 'pr merge 31 --repo example/repo' "$case_dir/gh-axi.log" \ + || fail "forge-decides-landed: expected merge with no strategy, got '$(cat "$case_dir/gh-axi.log")'" + assert_github_merge_has_no_strategy "$case_dir/gh-axi.log" "forge-decides-landed" + assert_grep 'verified: https://github.com/example/repo/pull/31 is merged' \ + "$case_dir/stdout" "forge-decides-landed: success was not reported as verified" + pass "fm-pr-merge reports an enqueued PR as queued and a landed one as merged" +} + +# An unreadable state on the forge-decides path must not claim the PR merged. +# Main's outcome gate refuses rather than treating that read as report-only. +test_forge_decides_unreadable_state_reports_without_failing() { + local case_dir rc + case_dir=$(make_case forge-decides-unreadable-state) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" 2222222222222222222222222222222222222222 + add_gh_mock_outcome_read_fails "$case_dir" 2222222222222222222222222222222222222222 + add_gh_axi_mock_view_fails "$case_dir" + : > "$case_dir/gh-axi.log" + : > "$case_dir/gh.log" + + set +e + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/32 -- --no-method \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "forge-decides-unreadable-state: an unreadable outcome must fail" + assert_grep 'could not read the GitHub pull request outcome after the merge attempt' \ + "$case_dir/stderr" "forge-decides-unreadable-state: the unreadable state was not reported" + assert_no_grep 'verified: ' "$case_dir/stdout" \ + "forge-decides-unreadable-state: an unread state was confirmed as merged" + grep -qxF 'pr merge 32 --repo example/repo' "$case_dir/gh-axi.log" \ + || fail "forge-decides-unreadable-state: expected merge with no strategy, got '$(cat "$case_dir/gh-axi.log")'" + assert_grep 'pr=https://github.com/example/repo/pull/32' "$case_dir/state/task-x1.meta" \ + "forge-decides-unreadable-state: a successful merge call lost its PR reference" + assert_present "$case_dir/state/task-x1.check.sh" \ + "forge-decides-unreadable-state: no merge poll was armed for a merge that may have landed" + pass "fm-pr-merge refuses an unreadable forge-decides outcome without claiming a merge" +} + +# The forge-decides tokens are firstmate-level and no glab flag spells them. +# GitLab already applies the project's own merge method, so they are refused by +# name before anything is recorded rather than forwarded to glab afterwards. +test_gitlab_forge_decides_method_refuses_before_recording() { + local case_dir rc spelling + for spelling in 'no-method|--no-method' 'method-equals-queue|--method=queue' 'method-queue|--method queue'; do + case_dir=$(make_gitlab_case "gitlab-forge-decides-${spelling%%|*}") + + set +e + # shellcheck disable=SC2086 # The spelling is one or two extra merge flags. + run_pr_merge "$case_dir" task-x1 "$MR_URL" -- ${spelling#*|} \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "gitlab-forge-decides-${spelling%%|*}: fm-pr-merge should refuse the forge-decides token" + assert_grep 'extra merge arguments must not ask GitLab to choose the merge method' \ + "$case_dir/stderr" "gitlab-forge-decides-${spelling%%|*}: refusal did not name the forge-decides token" + assert_no_grep "pr=$MR_URL" "$case_dir/state/task-x1.meta" \ + "gitlab-forge-decides-${spelling%%|*}: the URL was recorded before rejecting the token" + assert_absent "$case_dir/state/task-x1.check.sh" \ + "gitlab-forge-decides-${spelling%%|*}: the refused token armed a merge poll" + [ ! -s "$case_dir/glab.log" ] \ + || fail "gitlab-forge-decides-${spelling%%|*}: glab was invoked with a flag it does not define" + done + pass "fm-pr-merge refuses forge-decides merge methods on GitLab before recording state" +} + +# A GitLab merge method the caller spells for glab itself is untouched by the +# new guard, so GitLab merge behavior is unchanged. +test_gitlab_squash_arg_still_forwarded() { + local case_dir + case_dir=$(make_gitlab_case gitlab-squash-arg) + + run_pr_merge "$case_dir" task-x1 "$MR_URL" -- --squash \ + > "$case_dir/stdout" 2> "$case_dir/stderr" || fail "gitlab-squash-arg: fm-pr-merge failed" + + [ "$(glab_merge_line "$case_dir/glab.log")" \ + = "GITLAB_HOST=$MR_HOST mr merge 7 -R $MR_PROJECT_URL --sha $MR_HEAD --yes --squash" ] \ + || fail "gitlab-squash-arg: expected the caller's --squash forwarded, got '$(glab_merge_line "$case_dir/glab.log")'" + pass "fm-pr-merge still forwards a caller's real GitLab merge method" +} + test_parses_pr_url_for_gh_axi() { local case_dir case_dir=$(make_case url-parsing) @@ -2174,6 +2307,8 @@ test_explicit_merge_method_not_overridden test_method_equals_merge_method_not_overridden test_forge_decides_method_omits_strategy test_forge_decides_method_forwards_other_flags +test_forge_decides_reports_queued_and_merged_outcomes +test_forge_decides_unreadable_state_reports_without_failing test_parses_pr_url_for_gh_axi test_github_still_forwards_sha_arg test_gitlab_url_resolves_and_merges @@ -2188,6 +2323,8 @@ test_gitlab_unreadable_state_refuses test_gitlab_invalid_head_refuses test_gitlab_missing_tool_refuses_before_recording test_gitlab_head_override_args_refuse_before_recording +test_gitlab_forge_decides_method_refuses_before_recording +test_gitlab_squash_arg_still_forwarded test_secondmate_merge_reports_upward_once test_secondmate_merge_reports_on_the_local_route test_gitlab_merge_reports_upward diff --git a/tests/fm-teardown.test.sh b/tests/fm-teardown.test.sh index d56865243b4..502e40bd2cd 100755 --- a/tests/fm-teardown.test.sh +++ b/tests/fm-teardown.test.sh @@ -247,16 +247,22 @@ land_on_origin_main() { rm -rf "$tmp" } -# Override GitHub lookups to report PR 7 as merged with the supplied head. -add_gh_pr_merged_for_head() { - local case_dir=$1 head=$2 - cat > "$case_dir/fakebin/gh-axi" <<'SH' +# Override GitHub lookups to report PR 7 in the given state with the supplied +# head. Args: case_dir state head, where state is the lowercase forge state +# ("merged" or "open"); an open PR is the merge-queue enqueue state, which +# leaves the pull request open until it actually lands. +add_gh_pr_state_for_head() { + local case_dir=$1 state=$2 head=$3 upper + upper=$(printf '%s' "$state" | tr '[:lower:]' '[:upper:]') + cat > "$case_dir/fakebin/gh-axi" < "$case_dir/stdout" 2> "$case_dir/stderr" @@ -882,7 +888,7 @@ test_squash_merged_pr_allows_when_head_ancestor_of_pr_head() { append_pr_meta_url "$case_dir" local_head=$(git -C "$case_dir/wt" rev-parse HEAD) pr_head=$(commit_tree_from_wt_head "$case_dir" "$local_head" "no-mistakes follow-up") - add_gh_pr_merged_for_head "$case_dir" "$pr_head" + add_gh_pr_state_for_head "$case_dir" merged "$pr_head" set +e run_teardown "$case_dir" > "$case_dir/stdout" 2> "$case_dir/stderr" @@ -910,6 +916,7 @@ test_no_pr_recorded_discovers_merged_pr_by_branch_allows() { local_head=$(git -C "$case_dir/wt" rev-parse HEAD) pr_head=$(commit_tree_from_wt_head "$case_dir" "$local_head" "no-mistakes auto-fix") land_on_origin_main "$case_dir" feature.txt hello +<<<<<<< HEAD add_gh_pr_merged_for_head "$case_dir" "$pr_head" seed_backlog_in_flight "$case_dir" # No append_pr_meta_* call: state/task-x1.meta has no pr= or pr_head= line. @@ -940,7 +947,7 @@ test_squash_merged_pr_allows_replayed_unpushed_patch() { wt_commit_file "$case_dir" feature.txt hello "add feature" append_pr_meta_url "$case_dir" pr_head=$(land_equivalent_patch_on_origin_branch "$case_dir" pr-head feature.txt hello "add feature") - add_gh_pr_merged_for_head "$case_dir" "$pr_head" + add_gh_pr_state_for_head "$case_dir" merged "$pr_head" set +e run_teardown "$case_dir" > "$case_dir/stdout" 2> "$case_dir/stderr" @@ -961,7 +968,7 @@ test_open_pr_does_not_count_as_landed() { wt_commit_file "$case_dir" feature.txt hello "add feature" append_pr_meta_for_current_head "$case_dir" pr_head=$(git -C "$case_dir/wt" rev-parse HEAD) - add_gh_pr_open_for_head "$case_dir" "$pr_head" + add_gh_pr_state_for_head "$case_dir" open "$pr_head" set +e run_teardown "$case_dir" > "$case_dir/stdout" 2> "$case_dir/stderr" @@ -981,7 +988,7 @@ test_merged_pr_with_later_local_commit_refuses() { append_pr_meta_for_current_head "$case_dir" pr_head=$(git -C "$case_dir/wt" rev-parse HEAD) wt_commit_file "$case_dir" later.txt local-only "local follow-up" - add_gh_pr_merged_for_head "$case_dir" "$pr_head" + add_gh_pr_state_for_head "$case_dir" merged "$pr_head" set +e run_teardown "$case_dir" > "$case_dir/stdout" 2> "$case_dir/stderr" @@ -1091,7 +1098,7 @@ test_pr_check_does_not_refresh_stale_pr_head() { write_meta "$case_dir" no-mistakes ship wt_commit_file "$case_dir" feature.txt hello "add feature" pr_head=$(git -C "$case_dir/wt" rev-parse HEAD) - add_gh_pr_merged_for_head "$case_dir" "$pr_head" + add_gh_pr_state_for_head "$case_dir" merged "$pr_head" FM_ROOT_OVERRIDE="$ROOT" \ FM_STATE_OVERRIDE="$case_dir/state" \ @@ -1128,7 +1135,7 @@ test_pr_check_records_remote_head_when_local_lags() { wt_commit_file "$case_dir" feature.txt hello "add feature" local_head=$(git -C "$case_dir/wt" rev-parse HEAD) pr_head=$(commit_tree_from_wt_head "$case_dir" "$local_head" "no-mistakes follow-up") - add_gh_pr_merged_for_head "$case_dir" "$pr_head" + add_gh_pr_state_for_head "$case_dir" merged "$pr_head" FM_ROOT_OVERRIDE="$ROOT" \ FM_STATE_OVERRIDE="$case_dir/state" \ @@ -1192,7 +1199,7 @@ test_dirty_worktree_refuses() { wt_commit_file "$case_dir" feature.txt hello "add feature" land_on_origin_main "$case_dir" feature.txt hello pr_head=$(git -C "$case_dir/wt" rev-parse HEAD) - add_gh_pr_merged_for_head "$case_dir" "$pr_head" + add_gh_pr_state_for_head "$case_dir" merged "$pr_head" printf '%s\n' "uncommitted edit" > "$case_dir/wt/feature.txt" set +e From 0964b698df367e0535bb2d7597441a58e6dc3422 Mon Sep 17 00:00:00 2001 From: Alex William Date: Mon, 24 Aug 2026 18:18:54 +0200 Subject: [PATCH 08/18] no-mistakes(review): state-proven merge verdicts and drop unreachable poll states --- tests/fm-pr-check-security.test.sh | 2 +- tests/fm-pr-merge.test.sh | 9 +++------ 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/tests/fm-pr-check-security.test.sh b/tests/fm-pr-check-security.test.sh index 5b0479b28f1..e5e3eb09ede 100755 --- a/tests/fm-pr-check-security.test.sh +++ b/tests/fm-pr-check-security.test.sh @@ -687,7 +687,7 @@ test_static_poll_contract() { dir=$(make_case poll-contract) make_poll_fixture "$dir" - for state in OPEN CLOSED QUEUED AWAITING_CHECKS EMPTY MALFORMED; do + for state in OPEN CLOSED EMPTY MALFORMED; do case "$state" in EMPTY) value= ;; MALFORMED) value='not-a-state' ;; diff --git a/tests/fm-pr-merge.test.sh b/tests/fm-pr-merge.test.sh index 1f61a8c5ba3..80094983097 100755 --- a/tests/fm-pr-merge.test.sh +++ b/tests/fm-pr-merge.test.sh @@ -123,10 +123,10 @@ make_case() { } # gh-axi mock recording every invocation to a log file, and gh mock answering -# headRefOid for fm-pr-check.sh's pr_head lookup and, when a state is given, the -# post-merge `--json state` read. Args: case_dir head_sha [pr_state] +# headRefOid for fm-pr-check.sh's pr_head lookup and the post-merge outcome +# reads. Args: case_dir head_sha add_gh_mocks() { - local case_dir=$1 head=$2 state=${3-} + local case_dir=$1 head=$2 cat > "$case_dir/fakebin/gh-axi" <<'SH' #!/usr/bin/env bash printf '%s\n' "$*" >> "$FM_TEST_GH_AXI_LOG" @@ -146,9 +146,6 @@ case "\${1:-} \${2:-}" in "pr view") case " \$* " in *headRefOid*) printf '%s\n' '$head' ; exit 0 ;; - *"--json state"*) - [ -n '$state' ] || exit 1 - printf '%s\n' '$state' ; exit 0 ;; esac ;; "api graphql") From 4dbd8051551a06707b7f7b6abc1e26eab452ec12 Mon Sep 17 00:00:00 2001 From: Alex William Date: Mon, 24 Aug 2026 18:27:06 +0200 Subject: [PATCH 09/18] no-mistakes(review): tighten forge-report assertion and correct verification claims --- docs/verification/github-merge-queue.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/verification/github-merge-queue.md b/docs/verification/github-merge-queue.md index 68371232e84..e3488a4bee4 100644 --- a/docs/verification/github-merge-queue.md +++ b/docs/verification/github-merge-queue.md @@ -99,5 +99,5 @@ bin/fm-test-run.sh tests/fm-teardown.test.sh ``` The merge tests prove `--method=queue`, `--method queue`, and `--no-method` invoke `gh-axi pr merge` with no strategy flag, that the same live outcome read names a queued pull request as queued and a merged one as merged, that an unreadable state refuses rather than claiming a merge, and that those tokens are refused on GitLab before any state is recorded while a real GitLab merge method still forwards. -The poll contract stays silent for `OPEN`, `QUEUED`, and `AWAITING_CHECKS`, and emits `merged` only for `MERGED`. +The poll contract stays silent for `OPEN`, `CLOSED`, an empty state, and a malformed one, and emits `merged` only for `MERGED`. Teardown refuses an open pull request whose commits are not otherwise landed. From 2334e9518c6975da9abd180333b4a85b9e59ca58 Mon Sep 17 00:00:00 2001 From: Alex William Date: Mon, 24 Aug 2026 18:50:35 +0200 Subject: [PATCH 10/18] no-mistakes(document): note forge-decides merge outcome report in architecture --- docs/architecture.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/architecture.md b/docs/architecture.md index 2ea66f74228..802d1f90e73 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -308,7 +308,7 @@ This repo uses that setting, and its own `.no-mistakes/` directory remains local PR-based task merges go through `bin/fm-pr-merge.sh`, which records `pr=` and any available `pr_head=` through `bin/fm-pr-check.sh` before calling the forge CLI. The helper requires a full canonical URL and rejects malformed URLs or repo override flags before recording merge state. A `https://github.com///pull/` URL invokes `gh-axi pr merge --repo /` with the merge-method rules owned by that helper's header, including an explicit forge-decides method for merge-queue branches. -A GitHub merge-queue enqueue leaves the pull request open until it lands; the merge poll and teardown wait for a merged state, with empirical queue-state evidence in [`docs/verification/github-merge-queue.md`](verification/github-merge-queue.md). +A GitHub merge-queue enqueue leaves the pull request open until it lands, and the forge CLI still reports that call as a merge, so the same live outcome read that follows every GitHub merge names a queued pull request as queued rather than merged; landing stays confirmed by the merge poll and teardown, which wait for a merged state, with empirical queue-state evidence in [`docs/verification/github-merge-queue.md`](verification/github-merge-queue.md). A `https:////-/merge_requests/` URL (see [docs/gitlab-merge-watch.md](gitlab-merge-watch.md)) invokes `glab mr merge -R https:///`, so the instance comes from the URL, and adds no merge-method flag because the project's own merge method applies. That path merges only after one live read of the merge request confirms it is open, mergeable, conflict-free, with blocking discussions resolved and a successful pipeline at the current head, and it binds the merge to that verified head; recorded metadata is never the authority for those conditions because a rebase leaves it stale. After either forge command returns, the script confirms the PR or MR actually landed, and only a confirmed landing records a landed outcome; a queued or unconfirmed request records none and leaves its poll armed. From dcd2ca6bf55aaad0da4d587ae04838e4d27ca0dc Mon Sep 17 00:00:00 2001 From: Alex William Date: Thu, 27 Aug 2026 22:15:33 +0200 Subject: [PATCH 11/18] fix(bin): refuse a forge-decides method combined with an explicit strategy Dropping only the queue token forwarded --squash, --merge, or --rebase, and a merge-queue branch would reject that merge. Name both incompatible requests and refuse before the forge is called. --- bin/fm-pr-merge.sh | 49 +++++++++++++++++++++++++ docs/verification/github-merge-queue.md | 2 +- tests/fm-pr-merge.test.sh | 47 ++++++++++++++++++++++++ 3 files changed, 97 insertions(+), 1 deletion(-) diff --git a/bin/fm-pr-merge.sh b/bin/fm-pr-merge.sh index a4eaa0e5679..458c0bc9022 100755 --- a/bin/fm-pr-merge.sh +++ b/bin/fm-pr-merge.sh @@ -46,6 +46,10 @@ # that convention rather than mirror the GitHub default. # Those same forge-decides tokens are refused up front on GitLab, where no glab # flag spells them. +# Combining a forge-decides token with an explicit GitHub strategy (--squash, +# --merge, --rebase, or --method other than queue) is refused before the forge +# is called: dropping only the queue token would silently forward the strategy, +# and a merge-queue branch would reject that merge. # # A GitLab merge is refused unless every pre-merge condition holds, each read # live at merge time rather than taken from recorded metadata: the merge request @@ -203,6 +207,50 @@ reject_forge_decides_method() { done } +# GitHub-only: a forge-decides token plus an explicit strategy is two method +# requests. Dropping only the queue token would forward the strategy and a +# merge-queue branch would reject the merge, so name both and refuse before +# anything is recorded. +reject_conflicting_forge_decides_method() { + local arg prev='' forge_decides='' explicit='' + for arg in "$@"; do + if [ "$prev" = --method ]; then + if [ "$arg" = queue ]; then + forge_decides="${forge_decides:+$forge_decides }--method queue" + else + explicit="${explicit:+$explicit }--method $arg" + fi + prev= + continue + fi + case "$arg" in + --no-method) + forge_decides="${forge_decides:+$forge_decides }--no-method" + ;; + --method=queue) + forge_decides="${forge_decides:+$forge_decides }--method=queue" + ;; + --squash|--merge|--rebase) + explicit="${explicit:+$explicit }$arg" + ;; + --method=*) + explicit="${explicit:+$explicit }$arg" + ;; + --method) + prev=--method + ;; + esac + done + if [ "$prev" = --method ]; then + explicit="${explicit:+$explicit }--method" + fi + if [ -n "$forge_decides" ] && [ -n "$explicit" ]; then + printf 'error: extra merge arguments must not combine a forge-decides method (%s) with an explicit merge strategy (%s)\n' \ + "$forge_decides" "$explicit" >&2 + return 1 + fi +} + reject_repo_overrides() { local arg for arg in "$@"; do @@ -237,6 +285,7 @@ reject_head_overrides() { reject_repo_overrides "$@" || exit 1 [ "$PROVIDER" != gitlab ] || reject_head_overrides "$@" || exit 1 [ "$PROVIDER" != gitlab ] || reject_forge_decides_method "$@" || exit 1 +[ "$PROVIDER" != github ] || reject_conflicting_forge_decides_method "$@" || exit 1 # Task-derived paths are constructed only after the canonical ID validation. META="$STATE/$ID.meta" diff --git a/docs/verification/github-merge-queue.md b/docs/verification/github-merge-queue.md index e3488a4bee4..5e147b33cd0 100644 --- a/docs/verification/github-merge-queue.md +++ b/docs/verification/github-merge-queue.md @@ -98,6 +98,6 @@ bin/fm-test-run.sh tests/fm-pr-check-security.test.sh bin/fm-test-run.sh tests/fm-teardown.test.sh ``` -The merge tests prove `--method=queue`, `--method queue`, and `--no-method` invoke `gh-axi pr merge` with no strategy flag, that the same live outcome read names a queued pull request as queued and a merged one as merged, that an unreadable state refuses rather than claiming a merge, and that those tokens are refused on GitLab before any state is recorded while a real GitLab merge method still forwards. +The merge tests prove `--method=queue`, `--method queue`, and `--no-method` invoke `gh-axi pr merge` with no strategy flag, that combining one of those tokens with an explicit GitHub strategy is refused before the forge is called, that the same live outcome read names a queued pull request as queued and a merged one as merged, that an unreadable state refuses rather than claiming a merge, and that those tokens are refused on GitLab before any state is recorded while a real GitLab merge method still forwards. The poll contract stays silent for `OPEN`, `CLOSED`, an empty state, and a malformed one, and emits `merged` only for `MERGED`. Teardown refuses an open pull request whose commits are not otherwise landed. diff --git a/tests/fm-pr-merge.test.sh b/tests/fm-pr-merge.test.sh index 80094983097..a7272f82b92 100755 --- a/tests/fm-pr-merge.test.sh +++ b/tests/fm-pr-merge.test.sh @@ -19,6 +19,8 @@ # so an enqueued still-open PR is named queued rather than merged # (g4) those forge-decides tokens are refused on GitLab before any state is # recorded, while a real GitLab merge method still forwards +# (g5) combining a forge-decides token with an explicit GitHub strategy is +# refused before the forge is called, naming both incompatible requests # (h) repo override args fail fast because the repo comes from the URL, # including a bundled short-option cluster that carries -R # (i) a GitLab MR URL resolves and merges through glab instead of erroring @@ -1512,6 +1514,50 @@ test_forge_decides_method_forwards_other_flags() { pass "fm-pr-merge still forwards non-strategy GitHub flags after a forge-decides method" } +# Combining a forge-decides token with an explicit strategy must not drop only +# the queue token and forward the strategy: that silently bypasses the queue +# path. Refuse before the forge is called and name both requests. +test_forge_decides_conflicting_strategy_refuses_before_forge() { + local case_dir rc name rest args forge explicit + for spec in \ + 'queue-squash|--method=queue --squash|--method=queue|--squash' \ + 'queue-merge|--method=queue --merge|--method=queue|--merge' \ + 'queue-rebase|--method=queue --rebase|--method=queue|--rebase' \ + 'no-method-squash|--no-method --squash|--no-method|--squash' \ + 'method-queue-merge|--method queue --merge|--method queue|--merge' \ + 'queue-method-squash|--method=queue --method=squash|--method=queue|--method=squash' + do + name=${spec%%|*} + rest=${spec#*|} + args=${rest%%|*} + rest=${rest#*|} + forge=${rest%%|*} + explicit=${rest#*|} + case_dir=$(make_case "forge-decides-conflict-$name") + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" 3333333333333333333333333333333333333333 + : > "$case_dir/gh-axi.log" + + set +e + # shellcheck disable=SC2086 # The spelling is one or two extra merge flags. + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/33 -- $args \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "forge-decides-conflict-$name: fm-pr-merge should refuse the contradictory methods" + assert_grep "must not combine a forge-decides method ($forge) with an explicit merge strategy ($explicit)" \ + "$case_dir/stderr" "forge-decides-conflict-$name: refusal did not name both incompatible requests" + [ ! -s "$case_dir/gh-axi.log" ] \ + || fail "forge-decides-conflict-$name: gh-axi pr merge was invoked despite the contradictory methods" + assert_no_grep 'pr=https://github.com/example/repo/pull/33' "$case_dir/state/task-x1.meta" \ + "forge-decides-conflict-$name: the URL was recorded before rejecting the contradictory methods" + assert_absent "$case_dir/state/task-x1.check.sh" \ + "forge-decides-conflict-$name: the contradictory methods armed a merge poll" + done + pass "fm-pr-merge refuses a forge-decides method combined with an explicit GitHub strategy" +} + # On a merge-queue branch the merge call enqueues the pull request and leaves it # open, while the forge CLI still reports that as a merge. The same live outcome # read used for every GitHub merge must name which of the two actually happened, @@ -2304,6 +2350,7 @@ test_explicit_merge_method_not_overridden test_method_equals_merge_method_not_overridden test_forge_decides_method_omits_strategy test_forge_decides_method_forwards_other_flags +test_forge_decides_conflicting_strategy_refuses_before_forge test_forge_decides_reports_queued_and_merged_outcomes test_forge_decides_unreadable_state_reports_without_failing test_parses_pr_url_for_gh_axi From a99edf6d16ff128f8bb9c69b45b975bfd4348361 Mon Sep 17 00:00:00 2001 From: Alex William Date: Sun, 30 Aug 2026 16:04:39 +0200 Subject: [PATCH 12/18] no-mistakes(review): name forge-decides retry in queue-governed merge refusals --- bin/fm-pr-merge.sh | 55 ++++++++----- docs/architecture.md | 3 +- docs/verification/github-merge-queue.md | 2 +- tests/fm-pr-merge.test.sh | 103 +++++++++++++++++++----- 4 files changed, 120 insertions(+), 43 deletions(-) diff --git a/bin/fm-pr-merge.sh b/bin/fm-pr-merge.sh index 458c0bc9022..76a110086e6 100755 --- a/bin/fm-pr-merge.sh +++ b/bin/fm-pr-merge.sh @@ -22,10 +22,14 @@ # and naming both failed reads when gh is present and its own read failed. # If the pull request remains open and the base branch has an effective # merge_queue rule, the refusal names the queue's configured merge method and -# the exact -- --auto -- retry flags, unless the caller already passed -# that method with --auto to a merge command that returned success, in which -# case it reports instead that the accepted request has not entered the queue -# and the queue state has to be re-checked. +# the exact -- --auto --no-method retry flags. It names no strategy because the +# queue sets the method itself and refuses an explicit one, so the same retry +# holds whether that method is known, ambiguous or unrecognised, and naming a +# strategy would send the operator back into the refusal this path exists for. +# The exception is a caller who already passed a forge-decides method with +# --auto to a merge command that returned success, in which case it reports +# instead that the accepted request has not entered the queue and the queue +# state has to be re-checked. # No method is selected for the caller in any case. A rules response that names # no queue rule, one that could not be read, rules that disagree, and a method # this script does not recognise are four distinct outcomes and are reported @@ -125,6 +129,8 @@ caller_has_merge_method() { # The merge method the caller's own extra arguments named, in the --flag, # --method and --method= forms caller_has_merge_method accepts. +# Every spelling of "let the forge choose" normalises to queue, so one request +# is not mistaken for another spelling of it or for naming no method at all. caller_merge_method() { local arg method='' pending=false for arg in "$@"; do @@ -137,6 +143,7 @@ caller_merge_method() { --squash) method=squash ;; --merge) method=merge ;; --rebase) method=rebase ;; + --no-method) method=queue ;; --method) pending=true ;; --method=*) method=${arg#--method=} ;; esac @@ -632,15 +639,20 @@ github_state_is_open() { esac } -# Whether the caller's own named method is the one the queue is configured for, -# compared without regard to the spelling either side happens to use. -github_caller_method_is() { - case "$FM_PR_GITHUB_CALLER_METHOD" in - [mM][eE][rR][gG][eE]) [ "$1" = merge ] ;; - [sS][qQ][uU][aA][sS][hH]) [ "$1" = squash ] ;; - [rR][eE][bB][aA][sS][eE]) [ "$1" = rebase ] ;; - *) return 1 ;; - esac +# Whether the caller already asked the forge to choose the merge method, in any +# of the spellings caller_merge_method normalises to queue. That is the one +# method request a queue-governed base accepts, so it is what this refusal +# treats as flags the caller had already got right. +github_caller_method_is_forge_decides() { + [ "$FM_PR_GITHUB_CALLER_METHOD" = queue ] +} + +# The one retry a queue-governed base accepts. It names no strategy, because the +# queue sets the merge method itself and refuses an explicit one, so it does not +# vary with the queue's configured method and stays nameable even when that +# method is ambiguous or unrecognised. +github_queue_retry_command() { + printf '%s %s %s -- --auto --no-method' "$0" "$ID" "$URL" } github_report_queue_rules() { @@ -655,23 +667,24 @@ github_report_queue_rules() { esac if github_merge_command_succeeded \ && [ "$FM_PR_GITHUB_AUTO_REQUESTED" = true ] \ - && github_caller_method_is "$queue_method"; then - printf 'error: this run refuses even though the request for %s was accepted with the exact flags base branch %s requires (--auto --%s): the pull request has still not entered the merge queue, so no landed or queued outcome is proven; re-check the pull request'"'"'s merge queue state before retrying\n' \ + && github_caller_method_is_forge_decides; then + printf 'error: this run refuses even though the request for %s was accepted with the exact flags base branch %s requires (--auto --no-method, which leaves the queue'"'"'s own %s method to apply): the pull request has still not entered the merge queue, so no landed or queued outcome is proven; re-check the pull request'"'"'s merge queue state before retrying\n' \ "$URL" "$FM_PR_GITHUB_BASE" "$queue_method" >&2 else - printf 'error: base branch %s requires the merge queue; retry with: %s %s %s -- --auto --%s\n' \ - "$FM_PR_GITHUB_BASE" "$0" "$ID" "$URL" "$queue_method" >&2 + printf 'error: base branch %s requires the merge queue, which sets the merge method (%s) itself and refuses an explicit strategy; retry with: %s\n' \ + "$FM_PR_GITHUB_BASE" "$queue_method" "$(github_queue_retry_command)" >&2 fi ;; conflicting) - printf 'error: base branch %s has conflicting merge queue methods (%s); exact retry flags are ambiguous\n' \ - "$FM_PR_GITHUB_BASE" "${FM_PR_GITHUB_QUEUE_METHODS//,/, }" >&2 + printf 'error: base branch %s has conflicting merge queue methods (%s), so which one it would apply is ambiguous; the merge queue applies its own without being told, so retry with: %s\n' \ + "$FM_PR_GITHUB_BASE" "${FM_PR_GITHUB_QUEUE_METHODS//,/, }" \ + "$(github_queue_retry_command)" >&2 ;; unrecognised) methods_display=${FM_PR_GITHUB_QUEUE_METHODS//,/, } [ -n "$methods_display" ] || methods_display='' - printf 'error: base branch %s requires the merge queue, but its configured merge method (%s) is not one this script recognises, so exact retry flags cannot be named\n' \ - "$FM_PR_GITHUB_BASE" "$methods_display" >&2 + printf 'error: base branch %s requires the merge queue, but its configured merge method (%s) is not one this script recognises; the merge queue applies its own without being told, so retry with: %s\n' \ + "$FM_PR_GITHUB_BASE" "$methods_display" "$(github_queue_retry_command)" >&2 ;; unreadable) printf 'error: the branch rules for base branch %s could not be read, so a merge queue requirement can be neither confirmed nor ruled out here\n' \ diff --git a/docs/architecture.md b/docs/architecture.md index 802d1f90e73..505b9611adf 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -313,7 +313,8 @@ A `https:////-/merge_requests/` URL (see [docs/gitlab-merge-watch That path merges only after one live read of the merge request confirms it is open, mergeable, conflict-free, with blocking discussions resolved and a successful pipeline at the current head, and it binds the merge to that verified head; recorded metadata is never the authority for those conditions because a rebase leaves it stale. After either forge command returns, the script confirms the PR or MR actually landed, and only a confirmed landing records a landed outcome; a queued or unconfirmed request records none and leaves its poll armed. On GitLab an auto-merge-queued or unconfirmed request is reported without failing the run. -On GitHub an outcome that is neither merged nor queued is refused loudly and non-zero, naming the observed state, and a base branch that requires the merge queue is refused with the concrete retry flags its configured method requires rather than having a merge method chosen on the caller's behalf. +On GitHub an outcome that is neither merged nor queued is refused loudly and non-zero, naming the observed state, and a base branch that requires the merge queue is refused with the concrete forge-decides retry that base accepts rather than having a merge method chosen on the caller's behalf. +That retry names no strategy, because a queue-governed base sets the merge method itself and refuses an explicit one, so it is the same command whether the queue's configured method is known, ambiguous or unrecognised. When the forge already accepted exactly those flags and the pull request still has not entered the queue, that refusal points at the queue state to re-check instead of echoing back the flags the caller just ran. An auto-merge request is held to the same standard: `--auto` that leaves the pull request neither merged nor queued is refused rather than reported as success. Every GitHub refusal states what it could not observe as plainly as what it did, so an unreadable branch-rule response, an unrecognised queue method, and a merge queue no available read can see are each named rather than left to look like a base branch with no queue at all. diff --git a/docs/verification/github-merge-queue.md b/docs/verification/github-merge-queue.md index 5e147b33cd0..8508cceadbd 100644 --- a/docs/verification/github-merge-queue.md +++ b/docs/verification/github-merge-queue.md @@ -98,6 +98,6 @@ bin/fm-test-run.sh tests/fm-pr-check-security.test.sh bin/fm-test-run.sh tests/fm-teardown.test.sh ``` -The merge tests prove `--method=queue`, `--method queue`, and `--no-method` invoke `gh-axi pr merge` with no strategy flag, that combining one of those tokens with an explicit GitHub strategy is refused before the forge is called, that the same live outcome read names a queued pull request as queued and a merged one as merged, that an unreadable state refuses rather than claiming a merge, and that those tokens are refused on GitLab before any state is recorded while a real GitLab merge method still forwards. +The merge tests prove `--method=queue`, `--method queue`, and `--no-method` invoke `gh-axi pr merge` with no strategy flag, that a queue-governed base is refused with a forge-decides retry rather than an explicit strategy the queue would reject, that the printed retry is itself accepted when run, that combining one of those tokens with an explicit GitHub strategy is refused before the forge is called, that the same live outcome read names a queued pull request as queued and a merged one as merged, that an unreadable state refuses rather than claiming a merge, and that those tokens are refused on GitLab before any state is recorded while a real GitLab merge method still forwards. The poll contract stays silent for `OPEN`, `CLOSED`, an empty state, and a malformed one, and emits `merged` only for `MERGED`. Teardown refuses an open pull request whose commits are not otherwise landed. diff --git a/tests/fm-pr-merge.test.sh b/tests/fm-pr-merge.test.sh index a7272f82b92..a3881ce14f0 100755 --- a/tests/fm-pr-merge.test.sh +++ b/tests/fm-pr-merge.test.sh @@ -35,13 +35,13 @@ # (r) GitHub success is accepted only after the PR is read back as merged # (s) an open GitHub PR that is neither merged nor queued fails verification # (t) a GitHub PR in the merge queue is reported as queued, not merged -# (u) a queue-required refusal names the exact compatible retry flags +# (u) a queue-required refusal names the forge-decides retry the queue accepts # (v) a failed poll setup cannot be reported as a verified GitHub merge # (w) a zero-exit queue-required refusal keeps merge semantics unchanged # (x) an unreadable outcome after a successful merge call keeps the PR # recorded and the merge poll armed # (y) agreeing queue rules still produce exact retry flags -# (z) conflicting queue rules report ambiguous retry guidance +# (z) conflicting queue rules name the ambiguity and still name a retry # (aa) gh-axi remains usable when gh is absent # (ab) a landed merge whose fallback outcome read fails keeps its poll armed # (ac) a successful merge in a secondmate home reports the landed PR upward @@ -67,11 +67,12 @@ # without ever claiming auto-merge was armed # (aq) an outcome read that fails after a zero-exit merge still quotes the # forge's own output, the only evidence left -# (ar) auto-merge with the queue's own method that is still unqueued refuses +# (ar) auto-merge with a forge-decides method that is still unqueued refuses # without echoing back the flags just used, and names the next step -# (as) a caller method the queue does not use still gets exact retry flags +# (as) an explicit caller method the queue refuses still gets exact retry flags # (at) an unrecognised queue method still names the queue requirement and # guesses no method +# (ax) the retry a queue-required refusal prints is itself accepted when run # (au) unreadable branch rules are reported apart from a queue-less base # (av) a base branch with no queue rule says nothing about a merge queue # (aw) a refusal built on the gh-axi view says the merge queue could not be @@ -715,10 +716,11 @@ test_github_failed_merge_with_queue_flags_never_claims_acceptance() { "github-failed-merge-queue-flags: a failed merge command was reported as an accepted request" assert_no_grep 'armed' "$case_dir/stderr" \ "github-failed-merge-queue-flags: a failed merge command was reported as an armed auto-merge" - assert_grep 'base branch main requires the merge queue; retry with:' "$case_dir/stderr" \ + assert_grep 'base branch main requires the merge queue, which sets the merge method (merge) itself and refuses an explicit strategy; retry with:' \ + "$case_dir/stderr" \ "github-failed-merge-queue-flags: the failed merge command lost its concrete retry guidance" - assert_grep 'task-x1 https://github.com/example/repo/pull/74 -- --auto --merge' "$case_dir/stderr" \ - "github-failed-merge-queue-flags: the retry guidance named no queue flags" + assert_grep 'task-x1 https://github.com/example/repo/pull/74 -- --auto --no-method' "$case_dir/stderr" \ + "github-failed-merge-queue-flags: the retry guidance named no queue-accepted flags" assert_no_grep 'verified: ' "$case_dir/stdout" \ "github-failed-merge-queue-flags: a failed merge command was reported as verified" pass "fm-pr-merge claims no acceptance for a failed merge command carrying queue flags" @@ -735,7 +737,7 @@ test_github_accepted_queue_flags_do_not_echo_back_the_same_command() { : > "$case_dir/gh.log" set +e - run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/68 -- --auto --merge \ + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/68 -- --auto --no-method \ > "$case_dir/stdout" 2> "$case_dir/stderr" rc=$? set -e @@ -743,7 +745,7 @@ test_github_accepted_queue_flags_do_not_echo_back_the_same_command() { expect_code 1 "$rc" "github-accepted-queue-flags: an unproved merge must still fail" assert_grep 'state=OPEN, merged=false, isInMergeQueue=false' "$case_dir/stderr" \ "github-accepted-queue-flags: refusal did not name the concrete observed state" - assert_grep 'this run refuses even though the request for https://github.com/example/repo/pull/68 was accepted with the exact flags base branch main requires (--auto --merge)' \ + assert_grep 'this run refuses even though the request for https://github.com/example/repo/pull/68 was accepted with the exact flags base branch main requires (--auto --no-method, which leaves the queue'"'"'s own merge method to apply)' \ "$case_dir/stderr" \ "github-accepted-queue-flags: the refusal did not explain that the right flags were already used" assert_grep "re-check the pull request's merge queue state" "$case_dir/stderr" \ @@ -772,15 +774,18 @@ test_github_mismatched_queue_flags_still_name_the_retry() { set -e expect_code 1 "$rc" "github-mismatched-queue-flags: an unproved merge must still fail" - assert_grep 'base branch main requires the merge queue; retry with:' "$case_dir/stderr" \ + assert_grep 'base branch main requires the merge queue, which sets the merge method (rebase) itself and refuses an explicit strategy; retry with:' \ + "$case_dir/stderr" \ "github-mismatched-queue-flags: a caller method the queue does not use lost its retry guidance" - assert_grep '-- --auto --rebase' "$case_dir/stderr" \ + assert_grep '-- --auto --no-method' "$case_dir/stderr" \ "github-mismatched-queue-flags: the exact compatible flags were not named" + assert_no_grep '-- --auto --rebase' "$case_dir/stderr" \ + "github-mismatched-queue-flags: the retry named a strategy the merge queue refuses" pass "fm-pr-merge still names retry flags when the caller used a different method" } test_github_unrecognised_queue_method_still_names_the_queue() { - local case_dir rc + local case_dir rc guessed case_dir=$(make_case github-unrecognised-queue-method) mkdir -p "$case_dir/wt" add_gh_mocks "$case_dir" 8383838383838383838383838383838383838383 @@ -799,13 +804,62 @@ test_github_unrecognised_queue_method_still_names_the_queue() { assert_grep 'base branch main requires the merge queue, but its configured merge method (FASTFORWARD) is not one this script recognises' \ "$case_dir/stderr" \ "github-unrecognised-queue-method: a readable queue rule produced no queue mention" - assert_no_grep 'retry with:' "$case_dir/stderr" \ - "github-unrecognised-queue-method: retry flags were named for a method nothing recognises" - assert_no_grep '--auto --' "$case_dir/stderr" \ - "github-unrecognised-queue-method: a merge method was guessed for the caller" + assert_grep '-- --auto --no-method' "$case_dir/stderr" \ + "github-unrecognised-queue-method: an unrecognised method lost the retry that needs no method" + for guessed in '--auto --merge' '--auto --squash' '--auto --rebase' '--auto --fastforward'; do + assert_no_grep "$guessed" "$case_dir/stderr" \ + "github-unrecognised-queue-method: a merge method was guessed for the caller" + done pass "fm-pr-merge names the queue requirement even when its method is unrecognised" } +# The refusal's value is the command it hands the operator, so prove that +# command by running it rather than by matching its text: the printed retry has +# to reach the forge and be accepted on a queue-governed base, not bounce off +# the guard that refuses an explicit strategy there. +test_github_queue_retry_guidance_is_runnable() { + local case_dir rc retry_line retry_cmd retry_args + case_dir=$(make_case github-queue-retry-runnable) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" 8484848484848484848484848484848484848484 + write_github_outcome "$case_dir" OPEN false false main + printf 'merge_method=REBASE\n' > "$case_dir/github-rules" + : > "$case_dir/gh-axi.log" + : > "$case_dir/gh.log" + + set +e + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/71 \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "github-queue-retry-runnable: an unproved merge must fail" + retry_line=$(grep -F -- 'retry with: ' "$case_dir/stderr") \ + || fail "github-queue-retry-runnable: the queue refusal named no retry command" + retry_cmd=${retry_line#*retry with: } + case "$retry_cmd" in + "$PR_MERGE "*) ;; + *) fail "github-queue-retry-runnable: the retry did not name this script: '$retry_cmd'" ;; + esac + retry_args=${retry_cmd#"$PR_MERGE" } + + write_github_outcome "$case_dir" OPEN false true main + : > "$case_dir/gh-axi.log" + set +e + # shellcheck disable=SC2086 # The retry command's own arguments, as printed. + run_pr_merge "$case_dir" $retry_args > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 0 "$rc" "github-queue-retry-runnable: the advised retry was itself refused" + grep -qxF 'pr merge 71 --repo example/repo --auto' "$case_dir/gh-axi.log" \ + || fail "github-queue-retry-runnable: the advised retry did not reach the forge without a strategy, got '$(cat "$case_dir/gh-axi.log")'" + assert_github_merge_has_no_strategy "$case_dir/gh-axi.log" github-queue-retry-runnable + assert_grep 'verified: https://github.com/example/repo/pull/71 is queued' \ + "$case_dir/stdout" "github-queue-retry-runnable: the advised retry did not land in the queue" + pass "fm-pr-merge's merge-queue refusal names a retry that base branch accepts" +} + test_github_unreadable_queue_rules_are_not_reported_as_no_queue() { local case_dir rc case_dir=$(make_case github-unreadable-queue-rules) @@ -1095,8 +1149,10 @@ test_github_zero_exit_queue_required_refuses_with_exact_retry() { "github-zero-exit-queue-required: refusal did not name the concrete observed state" assert_grep 'base branch release/2026 requires the merge queue' "$case_dir/stderr" \ "github-zero-exit-queue-required: refusal did not name the queue requirement" - assert_grep '-- --auto --rebase' "$case_dir/stderr" \ + assert_grep '-- --auto --no-method' "$case_dir/stderr" \ "github-zero-exit-queue-required: refusal did not name the exact compatible flags" + assert_no_grep '-- --auto --rebase' "$case_dir/stderr" \ + "github-zero-exit-queue-required: refusal named a strategy the merge queue refuses" assert_grep 'api --paginate repos/example/repo/rules/branches/release%2F2026' "$case_dir/gh.log" \ "github-zero-exit-queue-required: queue rules were not read with pagination and encoded branch path" grep -qxF 'pr merge 56 --repo example/repo --squash' "$case_dir/gh-axi.log" \ @@ -1188,8 +1244,10 @@ test_github_queue_required_refusal_names_retry_flags() { "github-queue-required: the original forge failure was not preserved" assert_grep 'base branch master requires the merge queue' "$case_dir/stderr" \ "github-queue-required: refusal did not name the queue requirement" - grep -F -- '-- --auto --merge' "$case_dir/stderr" >/dev/null \ + grep -F -- '-- --auto --no-method' "$case_dir/stderr" >/dev/null \ || fail "github-queue-required: refusal did not name the exact compatible flags" + assert_no_grep '-- --auto --merge' "$case_dir/stderr" \ + "github-queue-required: refusal named a strategy the merge queue refuses" grep -qxF 'pr merge 54 --repo example/repo --squash' "$case_dir/gh-axi.log" \ || fail "github-queue-required: the wrapper silently changed the attempted merge semantics" assert_present "$case_dir/state/task-x1.check.sh" \ @@ -1216,9 +1274,11 @@ test_github_agreeing_queue_rules_keep_retry_guidance() { expect_code 1 "$rc" "github-agreeing-queue-rules: an unproved merge must fail" assert_grep 'base branch main requires the merge queue' "$case_dir/stderr" \ "github-agreeing-queue-rules: refusal did not name the queue requirement" - assert_grep '-- --auto --rebase' "$case_dir/stderr" \ + assert_grep '-- --auto --no-method' "$case_dir/stderr" \ "github-agreeing-queue-rules: agreeing rules omitted exact retry flags" - assert_no_grep 'exact retry flags are ambiguous' "$case_dir/stderr" \ + assert_grep 'sets the merge method (rebase) itself' "$case_dir/stderr" \ + "github-agreeing-queue-rules: agreeing rules did not resolve to one merge method" + assert_no_grep 'conflicting merge queue methods' "$case_dir/stderr" \ "github-agreeing-queue-rules: agreeing rules were reported as ambiguous" pass "fm-pr-merge aggregates agreeing merge-queue rules" } @@ -1250,6 +1310,8 @@ test_github_conflicting_queue_rules_report_ambiguity() { "github-conflicting-queue-rules: an exact retry method was guessed" assert_no_grep 'SQUASH, SQUASH' "$case_dir/stderr" \ "github-conflicting-queue-rules: a repeated queue method was named twice" + assert_grep '-- --auto --no-method' "$case_dir/stderr" \ + "github-conflicting-queue-rules: ambiguous rules lost the retry that needs no method" pass "fm-pr-merge reports ambiguity for conflicting merge-queue rules" } @@ -2326,6 +2388,7 @@ test_github_unreadable_outcome_refusal_quotes_the_forge_output test_github_accepted_queue_flags_do_not_echo_back_the_same_command test_github_mismatched_queue_flags_still_name_the_retry test_github_unrecognised_queue_method_still_names_the_queue +test_github_queue_retry_guidance_is_runnable test_github_unreadable_queue_rules_are_not_reported_as_no_queue test_github_no_queue_rule_says_nothing_about_a_queue test_github_fallback_view_refusal_says_the_queue_was_unobservable From d2a649d035f6e38ed2f07d43b592f17862e257e5 Mon Sep 17 00:00:00 2001 From: Alex William Date: Sun, 30 Aug 2026 16:15:05 +0200 Subject: [PATCH 13/18] no-mistakes(review): apply queue retry echo guard to every rules outcome --- bin/fm-pr-merge.sh | 50 ++++++++++++++++++++++++------------ tests/fm-pr-merge.test.sh | 54 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 87 insertions(+), 17 deletions(-) diff --git a/bin/fm-pr-merge.sh b/bin/fm-pr-merge.sh index 76a110086e6..a10390d5e55 100755 --- a/bin/fm-pr-merge.sh +++ b/bin/fm-pr-merge.sh @@ -29,7 +29,9 @@ # The exception is a caller who already passed a forge-decides method with # --auto to a merge command that returned success, in which case it reports # instead that the accepted request has not entered the queue and the queue -# state has to be re-checked. +# state has to be re-checked. Because the retry does not vary with the queue's +# method, that exception cannot either: it is decided once, and holds for every +# rules outcome that would otherwise name the retry. # No method is selected for the caller in any case. A rules response that names # no queue rule, one that could not be read, rules that disagree, and a method # this script does not recognise are four distinct outcomes and are reported @@ -655,8 +657,19 @@ github_queue_retry_command() { printf '%s %s %s -- --auto --no-method' "$0" "$ID" "$URL" } +# Whether the merge command already accepted the exact flags a queue-governed +# base takes, which makes naming that retry an echo of the command the caller +# just ran. The retry never varies with the queue's configured method, so this +# cannot vary with it either: it is decided once for every status that names a +# retry rather than inside one of them. +github_queue_retry_already_used() { + github_merge_command_succeeded \ + && [ "$FM_PR_GITHUB_AUTO_REQUESTED" = true ] \ + && github_caller_method_is_forge_decides +} + github_report_queue_rules() { - local queue_method methods_display + local queue_method methods_display situation github_read_queue_method case "$FM_PR_GITHUB_QUEUE_STATUS" in single) @@ -665,32 +678,37 @@ github_report_queue_rules() { SQUASH) queue_method=squash ;; REBASE) queue_method=rebase ;; esac - if github_merge_command_succeeded \ - && [ "$FM_PR_GITHUB_AUTO_REQUESTED" = true ] \ - && github_caller_method_is_forge_decides; then - printf 'error: this run refuses even though the request for %s was accepted with the exact flags base branch %s requires (--auto --no-method, which leaves the queue'"'"'s own %s method to apply): the pull request has still not entered the merge queue, so no landed or queued outcome is proven; re-check the pull request'"'"'s merge queue state before retrying\n' \ - "$URL" "$FM_PR_GITHUB_BASE" "$queue_method" >&2 - else - printf 'error: base branch %s requires the merge queue, which sets the merge method (%s) itself and refuses an explicit strategy; retry with: %s\n' \ - "$FM_PR_GITHUB_BASE" "$queue_method" "$(github_queue_retry_command)" >&2 - fi + printf -v situation \ + 'base branch %s requires the merge queue, which sets the merge method (%s) itself and refuses an explicit strategy' \ + "$FM_PR_GITHUB_BASE" "$queue_method" ;; conflicting) - printf 'error: base branch %s has conflicting merge queue methods (%s), so which one it would apply is ambiguous; the merge queue applies its own without being told, so retry with: %s\n' \ - "$FM_PR_GITHUB_BASE" "${FM_PR_GITHUB_QUEUE_METHODS//,/, }" \ - "$(github_queue_retry_command)" >&2 + printf -v situation \ + 'base branch %s has conflicting merge queue methods (%s), so which one it would apply is ambiguous; the merge queue applies its own without being told' \ + "$FM_PR_GITHUB_BASE" "${FM_PR_GITHUB_QUEUE_METHODS//,/, }" ;; unrecognised) methods_display=${FM_PR_GITHUB_QUEUE_METHODS//,/, } [ -n "$methods_display" ] || methods_display='' - printf 'error: base branch %s requires the merge queue, but its configured merge method (%s) is not one this script recognises; the merge queue applies its own without being told, so retry with: %s\n' \ - "$FM_PR_GITHUB_BASE" "$methods_display" "$(github_queue_retry_command)" >&2 + printf -v situation \ + 'base branch %s requires the merge queue, but its configured merge method (%s) is not one this script recognises; the merge queue applies its own without being told' \ + "$FM_PR_GITHUB_BASE" "$methods_display" ;; unreadable) printf 'error: the branch rules for base branch %s could not be read, so a merge queue requirement can be neither confirmed nor ruled out here\n' \ "${FM_PR_GITHUB_BASE:-}" >&2 + return 0 + ;; + *) + return 0 ;; esac + if github_queue_retry_already_used; then + printf 'error: %s; this run refuses even though the request for %s was accepted with the exact flags that base requires (--auto --no-method): the pull request has still not entered the merge queue, so no landed or queued outcome is proven; re-check the pull request'"'"'s merge queue state before retrying\n' \ + "$situation" "$URL" >&2 + else + printf 'error: %s; retry with: %s\n' "$situation" "$(github_queue_retry_command)" >&2 + fi } github_report_unmerged_outcome() { diff --git a/tests/fm-pr-merge.test.sh b/tests/fm-pr-merge.test.sh index a3881ce14f0..953f095b30d 100755 --- a/tests/fm-pr-merge.test.sh +++ b/tests/fm-pr-merge.test.sh @@ -73,6 +73,8 @@ # (at) an unrecognised queue method still names the queue requirement and # guesses no method # (ax) the retry a queue-required refusal prints is itself accepted when run +# (ay) flags the forge already accepted are echoed back by no rules outcome, +# whether the queue's method is agreed, conflicting or unrecognised # (au) unreadable branch rules are reported apart from a queue-less base # (av) a base branch with no queue rule says nothing about a merge queue # (aw) a refusal built on the gh-axi view says the merge queue could not be @@ -745,7 +747,10 @@ test_github_accepted_queue_flags_do_not_echo_back_the_same_command() { expect_code 1 "$rc" "github-accepted-queue-flags: an unproved merge must still fail" assert_grep 'state=OPEN, merged=false, isInMergeQueue=false' "$case_dir/stderr" \ "github-accepted-queue-flags: refusal did not name the concrete observed state" - assert_grep 'this run refuses even though the request for https://github.com/example/repo/pull/68 was accepted with the exact flags base branch main requires (--auto --no-method, which leaves the queue'"'"'s own merge method to apply)' \ + assert_grep 'base branch main requires the merge queue, which sets the merge method (merge) itself and refuses an explicit strategy' \ + "$case_dir/stderr" \ + "github-accepted-queue-flags: the refusal stopped naming what the queue itself applies" + assert_grep 'this run refuses even though the request for https://github.com/example/repo/pull/68 was accepted with the exact flags that base requires (--auto --no-method)' \ "$case_dir/stderr" \ "github-accepted-queue-flags: the refusal did not explain that the right flags were already used" assert_grep "re-check the pull request's merge queue state" "$case_dir/stderr" \ @@ -1315,6 +1320,52 @@ test_github_conflicting_queue_rules_report_ambiguity() { pass "fm-pr-merge reports ambiguity for conflicting merge-queue rules" } +# The retry a queue-governed base accepts is the same command whatever the +# rules response says, so the guard against handing that command back to a +# caller who already ran it has to hold for every rules response that would +# otherwise name it, not only for a single agreed method. +test_github_accepted_queue_flags_never_echoed_back_on_any_rules_outcome() { + local case_dir rc spec name rules situation + for spec in \ + 'conflicting|merge_method=MERGE\nmerge_method=SQUASH\n|has conflicting merge queue methods (MERGE, SQUASH)' \ + 'unrecognised|merge_method=FASTFORWARD\n|its configured merge method (FASTFORWARD) is not one this script recognises' + do + name=${spec%%|*} + rules=${spec#*|} + situation=${rules#*|} + rules=${rules%%|*} + case_dir=$(make_case "github-accepted-queue-flags-$name") + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" 8585858585858585858585858585858585858585 + write_github_outcome "$case_dir" OPEN false false main + printf '%b' "$rules" > "$case_dir/github-rules" + : > "$case_dir/gh-axi.log" + : > "$case_dir/gh.log" + + set +e + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/72 -- --auto --no-method \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "github-accepted-queue-flags-$name: an unproved merge must still fail" + assert_grep "$situation" "$case_dir/stderr" \ + "github-accepted-queue-flags-$name: the refusal stopped naming what the rules response said" + assert_grep 'was accepted with the exact flags that base requires (--auto --no-method)' \ + "$case_dir/stderr" \ + "github-accepted-queue-flags-$name: the refusal did not explain that the right flags were already used" + assert_grep "re-check the pull request's merge queue state" "$case_dir/stderr" \ + "github-accepted-queue-flags-$name: the refusal named no concrete next step" + assert_no_grep 'retry with:' "$case_dir/stderr" \ + "github-accepted-queue-flags-$name: the refusal echoed back the command that just refused" + assert_no_grep '-- --auto --no-method' "$case_dir/stderr" \ + "github-accepted-queue-flags-$name: the refusal repeated the caller's own flags as guidance" + assert_no_grep 'verified: ' "$case_dir/stdout" \ + "github-accepted-queue-flags-$name: an unproved merge was reported as verified" + done + pass "fm-pr-merge never echoes back accepted queue flags on any rules outcome" +} + test_extra_merge_args_forwarded() { local case_dir rc case_dir=$(make_case extra-args) @@ -2378,6 +2429,7 @@ test_github_zero_exit_queue_required_refuses_with_exact_retry test_github_closed_unqueued_outcome_omits_retry_flags test_github_agreeing_queue_rules_keep_retry_guidance test_github_conflicting_queue_rules_report_ambiguity +test_github_accepted_queue_flags_never_echoed_back_on_any_rules_outcome test_verified_merge_records_pr_and_head test_pr_metadata_is_recorded_before_the_forge_call test_merge_failure_propagates_after_recording From aa914bbb5dbaf2005e91db55cff00cf5f67de1cd Mon Sep 17 00:00:00 2001 From: Alex William Date: Sun, 30 Aug 2026 16:25:54 +0200 Subject: [PATCH 14/18] no-mistakes(review): derive queue retry echo guard from caller arguments --- bin/fm-pr-merge.sh | 31 ++++++++------- docs/architecture.md | 2 +- tests/fm-pr-merge.test.sh | 81 +++++++++++++++++++++++---------------- 3 files changed, 64 insertions(+), 50 deletions(-) diff --git a/bin/fm-pr-merge.sh b/bin/fm-pr-merge.sh index a10390d5e55..938349eff6f 100755 --- a/bin/fm-pr-merge.sh +++ b/bin/fm-pr-merge.sh @@ -27,11 +27,11 @@ # holds whether that method is known, ambiguous or unrecognised, and naming a # strategy would send the operator back into the refusal this path exists for. # The exception is a caller who already passed a forge-decides method with -# --auto to a merge command that returned success, in which case it reports -# instead that the accepted request has not entered the queue and the queue -# state has to be re-checked. Because the retry does not vary with the queue's -# method, that exception cannot either: it is decided once, and holds for every -# rules outcome that would otherwise name the retry. +# --auto, in which case that retry would only repeat what they ran, so it says +# no different retry exists and points at the blocking cause reported above it. +# Because the retry itself is one fixed command, that exception is read from the +# caller's arguments alone: it holds for every rules outcome that would +# otherwise name the retry, and whether the merge command succeeded or failed. # No method is selected for the caller in any case. A rules response that names # no queue rule, one that could not be read, rules that disagree, and a method # this script does not recognise are four distinct outcomes and are reported @@ -657,15 +657,14 @@ github_queue_retry_command() { printf '%s %s %s -- --auto --no-method' "$0" "$ID" "$URL" } -# Whether the merge command already accepted the exact flags a queue-governed -# base takes, which makes naming that retry an echo of the command the caller -# just ran. The retry never varies with the queue's configured method, so this -# cannot vary with it either: it is decided once for every status that names a -# retry rather than inside one of them. -github_queue_retry_already_used() { - github_merge_command_succeeded \ - && [ "$FM_PR_GITHUB_AUTO_REQUESTED" = true ] \ - && github_caller_method_is_forge_decides +# Whether the caller's own arguments already are the retry a queue-governed base +# takes, which makes naming that retry an echo of the command just run. The +# retry is one fixed command, so whether it would repeat the caller is settled +# by what they typed: it is read from their arguments alone, never from what the +# merge command then returned or from which rules outcome came back, so no path +# can reach the point of handing the caller their own command back. +github_caller_already_ran_queue_retry() { + [ "$FM_PR_GITHUB_AUTO_REQUESTED" = true ] && github_caller_method_is_forge_decides } github_report_queue_rules() { @@ -703,8 +702,8 @@ github_report_queue_rules() { return 0 ;; esac - if github_queue_retry_already_used; then - printf 'error: %s; this run refuses even though the request for %s was accepted with the exact flags that base requires (--auto --no-method): the pull request has still not entered the merge queue, so no landed or queued outcome is proven; re-check the pull request'"'"'s merge queue state before retrying\n' \ + if github_caller_already_ran_queue_retry; then + printf 'error: %s; --auto --no-method is the only thing that base takes and this run already used it, so no different retry exists to name: the outcome reported above for %s is the blocking cause, and re-check the pull request'"'"'s merge queue state before running the same command again\n' \ "$situation" "$URL" >&2 else printf 'error: %s; retry with: %s\n' "$situation" "$(github_queue_retry_command)" >&2 diff --git a/docs/architecture.md b/docs/architecture.md index 505b9611adf..f116a22bb13 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -315,7 +315,7 @@ After either forge command returns, the script confirms the PR or MR actually la On GitLab an auto-merge-queued or unconfirmed request is reported without failing the run. On GitHub an outcome that is neither merged nor queued is refused loudly and non-zero, naming the observed state, and a base branch that requires the merge queue is refused with the concrete forge-decides retry that base accepts rather than having a merge method chosen on the caller's behalf. That retry names no strategy, because a queue-governed base sets the merge method itself and refuses an explicit one, so it is the same command whether the queue's configured method is known, ambiguous or unrecognised. -When the forge already accepted exactly those flags and the pull request still has not entered the queue, that refusal points at the queue state to re-check instead of echoing back the flags the caller just ran. +When the caller's own arguments already were exactly those flags, that refusal says no different retry exists, names the blocking cause reported above it, and points at the queue state to re-check, instead of echoing back the command just run; because the retry is one fixed command, that holds for every rules outcome and whether the merge command succeeded or failed. An auto-merge request is held to the same standard: `--auto` that leaves the pull request neither merged nor queued is refused rather than reported as success. Every GitHub refusal states what it could not observe as plainly as what it did, so an unreadable branch-rule response, an unrecognised queue method, and a merge queue no available read can see are each named rather than left to look like a base branch with no queue at all. A confirmed merge leaves a durable role-routed outcome instead of living only in the merging agent's memory, and [`bin/fm-merge-outcome-lib.sh`](../bin/fm-merge-outcome-lib.sh)'s header owns its destination, shape, identity, normal-case deduplication, and at-least-once recovery. diff --git a/tests/fm-pr-merge.test.sh b/tests/fm-pr-merge.test.sh index 953f095b30d..1045880db1e 100755 --- a/tests/fm-pr-merge.test.sh +++ b/tests/fm-pr-merge.test.sh @@ -73,8 +73,9 @@ # (at) an unrecognised queue method still names the queue requirement and # guesses no method # (ax) the retry a queue-required refusal prints is itself accepted when run -# (ay) flags the forge already accepted are echoed back by no rules outcome, -# whether the queue's method is agreed, conflicting or unrecognised +# (ay) flags the caller already used are echoed back by no rules outcome and +# by no merge result, whether the queue's method is agreed, conflicting +# or unrecognised and whether the merge command succeeded or failed # (au) unreadable branch rules are reported apart from a queue-less base # (av) a base branch with no queue rule says nothing about a merge queue # (aw) a refusal built on the gh-axi view says the merge queue could not be @@ -750,9 +751,12 @@ test_github_accepted_queue_flags_do_not_echo_back_the_same_command() { assert_grep 'base branch main requires the merge queue, which sets the merge method (merge) itself and refuses an explicit strategy' \ "$case_dir/stderr" \ "github-accepted-queue-flags: the refusal stopped naming what the queue itself applies" - assert_grep 'this run refuses even though the request for https://github.com/example/repo/pull/68 was accepted with the exact flags that base requires (--auto --no-method)' \ + assert_grep '--auto --no-method is the only thing that base takes and this run already used it, so no different retry exists to name' \ "$case_dir/stderr" \ "github-accepted-queue-flags: the refusal did not explain that the right flags were already used" + assert_grep 'the outcome reported above for https://github.com/example/repo/pull/68 is the blocking cause' \ + "$case_dir/stderr" \ + "github-accepted-queue-flags: the refusal named no blocking cause in place of a retry" assert_grep "re-check the pull request's merge queue state" "$case_dir/stderr" \ "github-accepted-queue-flags: the refusal named no concrete next step" assert_no_grep 'retry with:' "$case_dir/stderr" \ @@ -1325,8 +1329,9 @@ test_github_conflicting_queue_rules_report_ambiguity() { # caller who already ran it has to hold for every rules response that would # otherwise name it, not only for a single agreed method. test_github_accepted_queue_flags_never_echoed_back_on_any_rules_outcome() { - local case_dir rc spec name rules situation + local case_dir rc spec name rules situation result label for spec in \ + 'single|merge_method=MERGE\n|which sets the merge method (merge) itself and refuses an explicit strategy' \ 'conflicting|merge_method=MERGE\nmerge_method=SQUASH\n|has conflicting merge queue methods (MERGE, SQUASH)' \ 'unrecognised|merge_method=FASTFORWARD\n|its configured merge method (FASTFORWARD) is not one this script recognises' do @@ -1334,36 +1339,46 @@ test_github_accepted_queue_flags_never_echoed_back_on_any_rules_outcome() { rules=${spec#*|} situation=${rules#*|} rules=${rules%%|*} - case_dir=$(make_case "github-accepted-queue-flags-$name") - mkdir -p "$case_dir/wt" - add_gh_mocks "$case_dir" 8585858585858585858585858585858585858585 - write_github_outcome "$case_dir" OPEN false false main - printf '%b' "$rules" > "$case_dir/github-rules" - : > "$case_dir/gh-axi.log" - : > "$case_dir/gh.log" - - set +e - run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/72 -- --auto --no-method \ - > "$case_dir/stdout" 2> "$case_dir/stderr" - rc=$? - set -e - - expect_code 1 "$rc" "github-accepted-queue-flags-$name: an unproved merge must still fail" - assert_grep "$situation" "$case_dir/stderr" \ - "github-accepted-queue-flags-$name: the refusal stopped naming what the rules response said" - assert_grep 'was accepted with the exact flags that base requires (--auto --no-method)' \ - "$case_dir/stderr" \ - "github-accepted-queue-flags-$name: the refusal did not explain that the right flags were already used" - assert_grep "re-check the pull request's merge queue state" "$case_dir/stderr" \ - "github-accepted-queue-flags-$name: the refusal named no concrete next step" - assert_no_grep 'retry with:' "$case_dir/stderr" \ - "github-accepted-queue-flags-$name: the refusal echoed back the command that just refused" - assert_no_grep '-- --auto --no-method' "$case_dir/stderr" \ - "github-accepted-queue-flags-$name: the refusal repeated the caller's own flags as guidance" - assert_no_grep 'verified: ' "$case_dir/stdout" \ - "github-accepted-queue-flags-$name: an unproved merge was reported as verified" + # Whether the merge command returned success or failed cannot change the + # guidance: the caller typed the only retry there is either way. + for result in accepted failed; do + label="github-accepted-queue-flags-$name-$result" + case_dir=$(make_case "$label") + mkdir -p "$case_dir/wt" + if [ "$result" = failed ]; then + add_gh_mocks_merge_fails "$case_dir" + else + add_gh_mocks "$case_dir" 8585858585858585858585858585858585858585 + fi + write_github_outcome "$case_dir" OPEN false false main + printf '%b' "$rules" > "$case_dir/github-rules" + : > "$case_dir/gh-axi.log" + : > "$case_dir/gh.log" + + set +e + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/72 -- --auto --no-method \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "$label: an unproved merge must still fail" + assert_grep "$situation" "$case_dir/stderr" \ + "$label: the refusal stopped naming what the rules response said" + assert_grep 'so no different retry exists to name' "$case_dir/stderr" \ + "$label: the refusal did not explain that the right flags were already used" + assert_grep "re-check the pull request's merge queue state" "$case_dir/stderr" \ + "$label: the refusal named no concrete next step" + assert_no_grep 'retry with:' "$case_dir/stderr" \ + "$label: the refusal echoed back the command that just refused" + assert_no_grep '-- --auto --no-method' "$case_dir/stderr" \ + "$label: the refusal repeated the caller's own flags as guidance" + assert_no_grep 'verified: ' "$case_dir/stdout" \ + "$label: an unproved merge was reported as verified" + done + assert_no_grep 'was accepted' "$TMP_ROOT/github-accepted-queue-flags-$name-failed/stderr" \ + "github-accepted-queue-flags-$name-failed: a failed merge command was reported as an accepted request" done - pass "fm-pr-merge never echoes back accepted queue flags on any rules outcome" + pass "fm-pr-merge never echoes back queue flags the caller already used" } test_extra_merge_args_forwarded() { From 7dd5908720601b938792ce81d302169d5486399e Mon Sep 17 00:00:00 2001 From: Alex William Date: Sun, 30 Aug 2026 16:47:57 +0200 Subject: [PATCH 15/18] no-mistakes(document): record queue retry echo guard in verification evidence --- docs/verification/github-merge-queue.md | 2 +- tests/fm-teardown.test.sh | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/docs/verification/github-merge-queue.md b/docs/verification/github-merge-queue.md index 8508cceadbd..0a02f8d8560 100644 --- a/docs/verification/github-merge-queue.md +++ b/docs/verification/github-merge-queue.md @@ -98,6 +98,6 @@ bin/fm-test-run.sh tests/fm-pr-check-security.test.sh bin/fm-test-run.sh tests/fm-teardown.test.sh ``` -The merge tests prove `--method=queue`, `--method queue`, and `--no-method` invoke `gh-axi pr merge` with no strategy flag, that a queue-governed base is refused with a forge-decides retry rather than an explicit strategy the queue would reject, that the printed retry is itself accepted when run, that combining one of those tokens with an explicit GitHub strategy is refused before the forge is called, that the same live outcome read names a queued pull request as queued and a merged one as merged, that an unreadable state refuses rather than claiming a merge, and that those tokens are refused on GitLab before any state is recorded while a real GitLab merge method still forwards. +The merge tests prove `--method=queue`, `--method queue`, and `--no-method` invoke `gh-axi pr merge` with no strategy flag, that a queue-governed base is refused with a forge-decides retry rather than an explicit strategy the queue would reject, that the printed retry is itself accepted when run, that a caller whose own arguments already are that retry is told no different retry exists instead of being handed their own command back, on every rules outcome that would otherwise name it and whether the merge command succeeded or failed, that combining one of those tokens with an explicit GitHub strategy is refused before the forge is called, that the same live outcome read names a queued pull request as queued and a merged one as merged, that an unreadable state refuses rather than claiming a merge, and that those tokens are refused on GitLab before any state is recorded while a real GitLab merge method still forwards. The poll contract stays silent for `OPEN`, `CLOSED`, an empty state, and a malformed one, and emits `merged` only for `MERGED`. Teardown refuses an open pull request whose commits are not otherwise landed. diff --git a/tests/fm-teardown.test.sh b/tests/fm-teardown.test.sh index 502e40bd2cd..7917bc88d36 100755 --- a/tests/fm-teardown.test.sh +++ b/tests/fm-teardown.test.sh @@ -916,7 +916,6 @@ test_no_pr_recorded_discovers_merged_pr_by_branch_allows() { local_head=$(git -C "$case_dir/wt" rev-parse HEAD) pr_head=$(commit_tree_from_wt_head "$case_dir" "$local_head" "no-mistakes auto-fix") land_on_origin_main "$case_dir" feature.txt hello -<<<<<<< HEAD add_gh_pr_merged_for_head "$case_dir" "$pr_head" seed_backlog_in_flight "$case_dir" # No append_pr_meta_* call: state/task-x1.meta has no pr= or pr_head= line. From 82694675afa029f980ff139381bbff618a505f74 Mon Sep 17 00:00:00 2001 From: Alex William Date: Tue, 8 Sep 2026 18:31:46 +0200 Subject: [PATCH 16/18] fix(bin): consolidate reliable PR delivery --- .agents/skills/project-management/SKILL.md | 1 + bin/fm-pr-check.sh | 37 ++ bin/fm-pr-merge.sh | 516 +++++++++++++-------- docs/architecture.md | 15 +- docs/verification/github-merge-queue.md | 137 +++--- tests/fm-pr-check-security.test.sh | 53 +++ tests/fm-pr-merge.test.sh | 272 ++++++++--- tests/fm-teardown.test.sh | 8 +- 8 files changed, 700 insertions(+), 339 deletions(-) diff --git a/.agents/skills/project-management/SKILL.md b/.agents/skills/project-management/SKILL.md index 85a25904617..a8848cb7250 100644 --- a/.agents/skills/project-management/SKILL.md +++ b/.agents/skills/project-management/SKILL.md @@ -87,6 +87,7 @@ A clone that contributes upstream therefore keeps `origin` on the parent reposit For that case the delivery target is changed by repointing that clone's `origin` and running `no-mistakes init` again, which also refreshes the gate mirror to the newly registered target; `no-mistakes status` then reports the new target and keeps it across a daemon restart. Which repository a given project should deliver into is the captain's decision, so confirm the intended target before repointing anything and follow any contribution workflow the project documents for itself. Editing the gate mirror's own remote URL is not a supported way to change the target: it leaves the registration and the mirror's tracking refs pointing at the old repository, so work keeps landing in the previous target and a rebase can silently resolve against a base the new target never had. +For an autonomous GitHub task, `bin/fm-pr-check.sh` independently reads live push permission for the URL-derived repository before accepting the resulting PR as ready, so an accidental read-only target is reported instead of looking landable. ## Remove diff --git a/bin/fm-pr-check.sh b/bin/fm-pr-check.sh index 99b3e025db2..8a240dd4453 100755 --- a/bin/fm-pr-check.sh +++ b/bin/fm-pr-check.sh @@ -5,6 +5,11 @@ # live only in a private sidecar and are never interpolated into shell source. # A GitHub pull request URL and a GitLab merge request URL are both accepted, # including a merge request on a self-hosted GitLab instance. +# A GitHub task carrying yolo=on intends Firstmate to land the PR itself, so the +# URL-derived repository must report live push permission before the PR can be +# recorded as ready. A read-only upstream is refused instead of presenting a +# green-looking PR that this home has no authority to land. Tasks with yolo off +# retain the contribution workflow in which an upstream maintainer lands work. # Usage: fm-pr-check.sh set -eu @@ -43,6 +48,38 @@ if [ ! -f "$META" ] || [ -L "$META" ] || [ "$(fm_pr_file_link_count "$META")" != exit 1 fi +github_verify_autonomous_target_writable() { + local yolo permission + [ "$PROVIDER" = github ] || return 0 + yolo=$(grep '^yolo=' "$META" | tail -1 | cut -d= -f2- || true) + [ "$yolo" = on ] || return 0 + command -v gh-axi >/dev/null 2>&1 || { + echo "error: verifying an autonomous GitHub delivery target requires gh-axi on PATH" >&2 + return 1 + } + if ! permission=$(gh-axi api "/repos/$FM_PR_OWNER/$FM_PR_REPO" \ + --jq '.permissions.push' 2>/dev/null); then + printf 'error: could not verify live write permission for autonomous delivery target %s/%s\n' \ + "$FM_PR_OWNER" "$FM_PR_REPO" >&2 + return 1 + fi + case "$permission" in + true) return 0 ;; + false) + printf 'error: refusing autonomous delivery to read-only GitHub repository %s/%s; choose a writable intended target before opening or accepting the PR\n' \ + "$FM_PR_OWNER" "$FM_PR_REPO" >&2 + return 1 + ;; + *) + printf 'error: could not verify live write permission for autonomous delivery target %s/%s\n' \ + "$FM_PR_OWNER" "$FM_PR_REPO" >&2 + return 1 + ;; + esac +} + +github_verify_autonomous_target_writable || exit 1 + # A prior exact merged result may have queued its durable wake immediately # before interruption. # Finish only its identity-bound receipt before publishing a replacement poll. diff --git a/bin/fm-pr-merge.sh b/bin/fm-pr-merge.sh index 938349eff6f..43d7f3ac898 100755 --- a/bin/fm-pr-merge.sh +++ b/bin/fm-pr-merge.sh @@ -6,36 +6,30 @@ # request is addressed through glab by the project URL rebuilt from the parsed # host and path, so any instance works and no host is hardcoded. # -# Merge method on GitHub defaults to --squash when the caller passes none of -# --squash, --merge, --rebase, --method, or --no-method after the optional -- -# separator. --method=queue, --method queue, and --no-method count as a method -# so that default is skipped, then they are dropped rather than forwarded: a -# GitHub merge-queue branch refuses any explicit strategy, and gh-axi rejects -# --method=queue. -# The gh-axi merge abstraction always performs the merge; the outcome read that -# follows it never becomes a prerequisite for reaching that abstraction. After -# gh-axi returns success, GitHub's live state is read back and accepted only -# when the pull request is merged or in the merge queue. gh's GraphQL API -# supplies that queue-aware read when gh is on PATH; when gh is absent or its -# read fails, gh-axi's own view still proves a landed merge, and every outcome -# it cannot prove refuses, reporting the single failed read when gh is absent -# and naming both failed reads when gh is present and its own read failed. -# If the pull request remains open and the base branch has an effective -# merge_queue rule, the refusal names the queue's configured merge method and -# the exact -- --auto --no-method retry flags. It names no strategy because the -# queue sets the method itself and refuses an explicit one, so the same retry -# holds whether that method is known, ambiguous or unrecognised, and naming a -# strategy would send the operator back into the refusal this path exists for. -# The exception is a caller who already passed a forge-decides method with -# --auto, in which case that retry would only repeat what they ran, so it says -# no different retry exists and points at the blocking cause reported above it. -# Because the retry itself is one fixed command, that exception is read from the -# caller's arguments alone: it holds for every rules outcome that would -# otherwise name the retry, and whether the merge command succeeded or failed. -# No method is selected for the caller in any case. A rules response that names -# no queue rule, one that could not be read, rules that disagree, and a method -# this script does not recognise are four distinct outcomes and are reported -# apart, because each one leaves the operator somewhere different. +# GitHub direct merges default to --squash when the caller names no method. +# `--queue` is the canonical merge-queue request and invokes GitHub's supported +# GraphQL enqueuePullRequest mutation rather than the gh-axi merge parser. +# --no-method, --method=queue, and --method queue remain accepted aliases for +# retry commands emitted by the earlier implementation. +# One parser owns every queue spelling and all caller-argument interpretation. +# It refuses repeated queue tokens, a queue token mixed with an explicit merge +# strategy, and any extra argument the enqueue mutation cannot honour. +# Before enqueue, one live GraphQL read proves the URL-derived repository is +# writable, the pull request is open, its status-check rollup is not red, and +# its head still matches the head fm-pr-check recorded. +# enqueuePullRequest receives that same head as expectedHeadOid, so a later push +# makes the mutation fail instead of queueing a changed identity. +# The base must have an effective merge_queue rule, and a successful mutation +# must return a queue entry before an independent live read confirms membership. +# A direct merge that discovers a queue-governed base prints one runnable +# `--queue` retry, unless the caller already supplied any accepted queue token. +# Rules that are absent, unreadable, conflicting, or unrecognised remain +# distinct outcomes, but none can produce a retry the parser rejects. +# The gh-axi merge abstraction still owns non-queue GitHub merges. +# After it returns, GitHub's live state is read back and accepted only when the +# pull request is merged or in the merge queue. +# gh's GraphQL API supplies the queue-aware read when gh is on PATH; when gh is +# absent or its read fails, gh-axi's own view can still prove a landed merge. # A caller-requested --auto that leaves the pull request neither merged nor # queued is refused the same way and says auto-merge was armed with nothing # landed or queued yet, or, when the merge command itself failed, that auto-merge @@ -50,12 +44,7 @@ # GitLab adds no method flag at all: its merge method is the project's own # setting, which the merge API applies, and imposing squash there would override # that convention rather than mirror the GitHub default. -# Those same forge-decides tokens are refused up front on GitLab, where no glab -# flag spells them. -# Combining a forge-decides token with an explicit GitHub strategy (--squash, -# --merge, --rebase, or --method other than queue) is refused before the forge -# is called: dropping only the queue token would silently forward the strategy, -# and a merge-queue branch would reject that merge. +# Queue tokens are refused up front on GitLab, where no glab flag spells them. # # A GitLab merge is refused unless every pre-merge condition holds, each read # live at merge time rather than taken from recorded metadata: the merge request @@ -119,145 +108,114 @@ PROJECT_URL="https://$FM_PR_HOST/$FM_PR_PATH" shift 2 [ "${1:-}" = "--" ] && shift -caller_has_merge_method() { - local arg - for arg in "$@"; do - case "$arg" in - --squash|--merge|--rebase|--method|--method=*|--no-method) return 0 ;; - esac - done - return 1 -} - -# The merge method the caller's own extra arguments named, in the --flag, -# --method and --method= forms caller_has_merge_method accepts. -# Every spelling of "let the forge choose" normalises to queue, so one request -# is not mistaken for another spelling of it or for naming no method at all. -caller_merge_method() { - local arg method='' pending=false - for arg in "$@"; do - if [ "$pending" = true ]; then - method=$arg - pending=false - continue - fi - case "$arg" in - --squash) method=squash ;; - --merge) method=merge ;; - --rebase) method=rebase ;; - --no-method) method=queue ;; - --method) pending=true ;; - --method=*) method=${arg#--method=} ;; - esac - done - printf '%s' "$method" -} - -# Whether the caller's own extra arguments asked for auto-merge, including the -# --flag=value spelling the forge's flag parser accepts. --disable-auto cancels -# the request, and gh exposes no short option that could bundle either flag. -caller_requested_auto_merge() { - local arg requested=1 - for arg in "$@"; do - case "$arg" in - --auto) requested=0 ;; - --auto=*) - case "${arg#--auto=}" in - [tT]|[tT][rR][uU][eE]|1) requested=0 ;; - *) requested=1 ;; - esac - ;; - --disable-auto) requested=1 ;; - esac - done - return "$requested" -} +# Parse the caller's merge arguments once for every provider path. +# This is the single owner of queue-token grammar and of how caller arguments +# are interpreted. `--queue` is the canonical enqueue request; --no-method, +# --method=queue, and --method queue remain accepted aliases for commands the +# earlier merge-queue implementation printed. Queue tokens are Firstmate flags, +# never forge CLI flags. A repeated queue token, a queue token mixed with an +# explicit strategy, or an argument the GraphQL enqueue path cannot honour is +# refused rather than dropped or forwarded with changed meaning. +FM_PR_CALLER_HAS_METHOD=false +FM_PR_CALLER_METHOD= +FM_PR_CALLER_AUTO=false +FM_PR_CALLER_QUEUE=false +FM_PR_CALLER_QUEUE_COUNT=0 +FM_PR_CALLER_QUEUE_TOKENS= +FM_PR_CALLER_EXPLICIT_METHODS= +FM_PR_CALLER_FORWARD=() +fm_pr_parse_merge_args() { + local arg pending_method=false unsupported='' + FM_PR_CALLER_HAS_METHOD=false + FM_PR_CALLER_METHOD= + FM_PR_CALLER_AUTO=false + FM_PR_CALLER_QUEUE=false + FM_PR_CALLER_QUEUE_COUNT=0 + FM_PR_CALLER_QUEUE_TOKENS= + FM_PR_CALLER_EXPLICIT_METHODS= + FM_PR_CALLER_FORWARD=() -# GitHub-only: drop tokens that mean "let the forge choose the method". -# They already satisfy caller_has_merge_method so the default --squash is not -# added. They are not GitHub merge strategies and must not reach gh-axi. -GITHUB_MERGE_FORWARD=() -github_drop_forge_decides_method() { - GITHUB_MERGE_FORWARD=() while [ "$#" -gt 0 ]; do - case "$1" in - --no-method|--method=queue) - shift - ;; - --method) - if [ "${2-}" = queue ]; then - shift 2 - else - GITHUB_MERGE_FORWARD+=("$1") - shift - fi - ;; - *) - GITHUB_MERGE_FORWARD+=("$1") - shift - ;; - esac - done -} - -# Firstmate-level tokens that ask the forge to choose the merge method. No glab -# flag spells them, and GitLab already applies the project's own merge method, -# so the request is a no-op there: refuse it by name before anything is recorded -# rather than forward an unknown flag to glab after the merge is armed. -reject_forge_decides_method() { - local arg prev='' - for arg in "$@"; do - if [ "$arg" = --no-method ] || [ "$arg" = --method=queue ] \ - || { [ "$prev" = --method ] && [ "$arg" = queue ]; }; then - echo "error: extra merge arguments must not ask GitLab to choose the merge method, which it already does" >&2 - return 1 - fi - prev=$arg - done -} - -# GitHub-only: a forge-decides token plus an explicit strategy is two method -# requests. Dropping only the queue token would forward the strategy and a -# merge-queue branch would reject the merge, so name both and refuse before -# anything is recorded. -reject_conflicting_forge_decides_method() { - local arg prev='' forge_decides='' explicit='' - for arg in "$@"; do - if [ "$prev" = --method ]; then + arg=$1 + shift + if [ "$pending_method" = true ]; then if [ "$arg" = queue ]; then - forge_decides="${forge_decides:+$forge_decides }--method queue" + FM_PR_CALLER_QUEUE=true + FM_PR_CALLER_QUEUE_COUNT=$((FM_PR_CALLER_QUEUE_COUNT + 1)) + FM_PR_CALLER_QUEUE_TOKENS="${FM_PR_CALLER_QUEUE_TOKENS:+$FM_PR_CALLER_QUEUE_TOKENS }--method queue" + FM_PR_CALLER_METHOD=queue else - explicit="${explicit:+$explicit }--method $arg" + FM_PR_CALLER_METHOD=$arg + FM_PR_CALLER_EXPLICIT_METHODS="${FM_PR_CALLER_EXPLICIT_METHODS:+$FM_PR_CALLER_EXPLICIT_METHODS }--method $arg" + FM_PR_CALLER_FORWARD+=(--method "$arg") fi - prev= + pending_method=false continue fi case "$arg" in - --no-method) - forge_decides="${forge_decides:+$forge_decides }--no-method" + --queue|--no-method|--method=queue) + FM_PR_CALLER_HAS_METHOD=true + FM_PR_CALLER_METHOD=queue + FM_PR_CALLER_QUEUE=true + FM_PR_CALLER_QUEUE_COUNT=$((FM_PR_CALLER_QUEUE_COUNT + 1)) + FM_PR_CALLER_QUEUE_TOKENS="${FM_PR_CALLER_QUEUE_TOKENS:+$FM_PR_CALLER_QUEUE_TOKENS }$arg" ;; - --method=queue) - forge_decides="${forge_decides:+$forge_decides }--method=queue" + --method) + FM_PR_CALLER_HAS_METHOD=true + pending_method=true ;; - --squash|--merge|--rebase) - explicit="${explicit:+$explicit }$arg" + --squash|--merge|--rebase|--method=*) + FM_PR_CALLER_HAS_METHOD=true + FM_PR_CALLER_METHOD=${arg#--} + FM_PR_CALLER_METHOD=${FM_PR_CALLER_METHOD#method=} + FM_PR_CALLER_EXPLICIT_METHODS="${FM_PR_CALLER_EXPLICIT_METHODS:+$FM_PR_CALLER_EXPLICIT_METHODS }$arg" + FM_PR_CALLER_FORWARD+=("$arg") ;; - --method=*) - explicit="${explicit:+$explicit }$arg" + --auto) + FM_PR_CALLER_AUTO=true + FM_PR_CALLER_FORWARD+=("$arg") ;; - --method) - prev=--method + --auto=*) + case "${arg#--auto=}" in + [tT]|[tT][rR][uU][eE]|1) FM_PR_CALLER_AUTO=true ;; + *) FM_PR_CALLER_AUTO=false ;; + esac + FM_PR_CALLER_FORWARD+=("$arg") ;; + --disable-auto) + FM_PR_CALLER_AUTO=false + FM_PR_CALLER_FORWARD+=("$arg") + ;; + *) FM_PR_CALLER_FORWARD+=("$arg") ;; esac done - if [ "$prev" = --method ]; then - explicit="${explicit:+$explicit }--method" + if [ "$pending_method" = true ]; then + FM_PR_CALLER_EXPLICIT_METHODS="${FM_PR_CALLER_EXPLICIT_METHODS:+$FM_PR_CALLER_EXPLICIT_METHODS }--method" + FM_PR_CALLER_FORWARD+=(--method) + fi + + if [ "$FM_PR_CALLER_QUEUE_COUNT" -gt 1 ]; then + printf 'error: extra merge arguments repeat the queue request (%s); pass exactly one queue token\n' \ + "$FM_PR_CALLER_QUEUE_TOKENS" >&2 + return 1 fi - if [ -n "$forge_decides" ] && [ -n "$explicit" ]; then - printf 'error: extra merge arguments must not combine a forge-decides method (%s) with an explicit merge strategy (%s)\n' \ - "$forge_decides" "$explicit" >&2 + if [ "$FM_PR_CALLER_QUEUE" = true ] && [ -n "$FM_PR_CALLER_EXPLICIT_METHODS" ]; then + printf 'error: extra merge arguments must not combine a queue request (%s) with an explicit merge strategy (%s)\n' \ + "$FM_PR_CALLER_QUEUE_TOKENS" "$FM_PR_CALLER_EXPLICIT_METHODS" >&2 return 1 fi + if [ "$FM_PR_CALLER_QUEUE" = true ]; then + for arg in "${FM_PR_CALLER_FORWARD[@]+"${FM_PR_CALLER_FORWARD[@]}"}"; do + case "$arg" in + --auto|--auto=[tT]|--auto=[tT][rR][uU][eE]|--auto=1) ;; + *) unsupported="${unsupported:+$unsupported }$arg" ;; + esac + done + if [ -n "$unsupported" ]; then + printf 'error: queue enqueue does not support extra merge arguments (%s)\n' "$unsupported" >&2 + return 1 + fi + fi } reject_repo_overrides() { @@ -292,9 +250,12 @@ reject_head_overrides() { } reject_repo_overrides "$@" || exit 1 +fm_pr_parse_merge_args "$@" || exit 1 [ "$PROVIDER" != gitlab ] || reject_head_overrides "$@" || exit 1 -[ "$PROVIDER" != gitlab ] || reject_forge_decides_method "$@" || exit 1 -[ "$PROVIDER" != github ] || reject_conflicting_forge_decides_method "$@" || exit 1 +if [ "$PROVIDER" = gitlab ] && [ "$FM_PR_CALLER_QUEUE" = true ]; then + echo "error: extra merge arguments must not request GitHub's merge queue on GitLab" >&2 + exit 1 +fi # Task-derived paths are constructed only after the canonical ID validation. META="$STATE/$ID.meta" @@ -600,6 +561,134 @@ METHODS fi } +FM_PR_GITHUB_NODE_ID= +FM_PR_GITHUB_HEAD= + +# Read the exact PR identity and repository authority immediately before a +# queue enqueue. The head is also passed to enqueuePullRequest as +# expectedHeadOid, so a push after this read makes the mutation refuse rather +# than queueing code this run did not inspect. A red rollup is refused before +# enqueue, and repository READ permission cannot masquerade as merge authority. +github_read_enqueue_preflight() { + local fields line recorded_head + local total=0 named=0 state='' merged='' queued='' base='' + local node='' head='' checks='' permission='' + command -v gh >/dev/null 2>&1 || { + echo "error: enqueueing a GitHub pull request requires gh on PATH" >&2 + return 1 + } + # shellcheck disable=SC2016 # GraphQL variables are literal query syntax. + if ! fields=$(gh api graphql \ + -f query='query($owner:String!,$repo:String!,$number:Int!){repository(owner:$owner,name:$repo){viewerPermission pullRequest(number:$number){id state merged isInMergeQueue baseRefName headRefOid commits(last:1){nodes{commit{statusCheckRollup{state}}}}}}}' \ + -F "owner=$PR_OWNER" -F "repo=$PR_REPO" -F "number=$PR_NUMBER" \ + --jq '.data.repository as $r | $r.pullRequest | "state=" + (.state // ""), "merged=" + (.merged | tostring), "queued=" + (.isInMergeQueue | tostring), "base=" + (.baseRefName // ""), "node=" + (.id // ""), "head=" + (.headRefOid // ""), "checks=" + (.commits.nodes[0].commit.statusCheckRollup.state // "NONE"), "permission=" + ($r.viewerPermission // "")' \ + 2>/dev/null) || [ -z "$fields" ]; then + echo "error: could not read the GitHub pull request identity and merge authority before enqueueing" >&2 + return 1 + fi + while IFS= read -r line; do + total=$((total + 1)) + case "$line" in + state=*) state=${line#state=} ;; + merged=*) merged=${line#merged=} ;; + queued=*) queued=${line#queued=} ;; + base=*) base=${line#base=} ;; + node=*) node=${line#node=} ;; + head=*) head=${line#head=} ;; + checks=*) checks=${line#checks=} ;; + permission=*) permission=${line#permission=} ;; + *) continue ;; + esac + named=$((named + 1)) + done <&2 + return 1 + fi + case "$permission" in + WRITE|MAINTAIN|ADMIN) ;; + *) + printf 'error: refusing to enqueue %s: repository %s/%s grants only %s permission, not merge authority\n' \ + "$URL" "$PR_OWNER" "$PR_REPO" "${permission:-unreadable}" >&2 + return 1 + ;; + esac + case "$checks" in + FAILURE|ERROR) + printf 'error: refusing to enqueue %s: the current head %s has a red status-check rollup (%s)\n' \ + "$URL" "$head" "$checks" >&2 + return 1 + ;; + SUCCESS|PENDING|EXPECTED|NONE) ;; + *) + printf 'error: refusing to enqueue %s: the status-check rollup is unreadable (%s)\n' \ + "$URL" "${checks:-empty}" >&2 + return 1 + ;; + esac + recorded_head=$(grep '^pr_head=' "$META" | tail -1 | cut -d= -f2- || true) + if [ -n "$recorded_head" ] && [ "$recorded_head" != "$head" ]; then + printf 'error: refusing to enqueue %s: recorded head %s changed to %s before the queue request\n' \ + "$URL" "$recorded_head" "$head" >&2 + return 1 + fi + FM_PR_GITHUB_STATE=$state + FM_PR_GITHUB_MERGED=$merged + FM_PR_GITHUB_QUEUED=$queued + FM_PR_GITHUB_BASE=$base + FM_PR_GITHUB_QUEUE_OBSERVED=true + FM_PR_GITHUB_NODE_ID=$node + FM_PR_GITHUB_HEAD=$head +} + +# Invoke GitHub's supported queue operation. The mutation result must name a +# queue entry, then the ordinary live outcome read independently confirms queue +# membership. The expected head preserves the same changed-identity boundary +# that direct merges obtain from the forge CLI. +github_enqueue_pull_request() { + local fields line entry_id='' entry_state='' total=0 named=0 + # shellcheck disable=SC2016 # GraphQL variables are literal query syntax. + if ! fields=$(gh api graphql \ + -f query='mutation($pullRequestId:ID!,$expectedHeadOid:GitObjectID!){enqueuePullRequest(input:{pullRequestId:$pullRequestId,expectedHeadOid:$expectedHeadOid}){mergeQueueEntry{id state}}}' \ + -f "pullRequestId=$FM_PR_GITHUB_NODE_ID" \ + -f "expectedHeadOid=$FM_PR_GITHUB_HEAD" \ + --jq '.data.enqueuePullRequest.mergeQueueEntry | "entry_id=" + (.id // ""), "entry_state=" + (.state // "")' \ + 2>&1); then + [ -z "$fields" ] || printf '%s\n' "$fields" >&2 + echo "error: GitHub rejected enqueuePullRequest; nothing was reported as queued" >&2 + return 1 + fi + while IFS= read -r line; do + total=$((total + 1)) + case "$line" in + entry_id=*) entry_id=${line#entry_id=} ;; + entry_state=*) entry_state=${line#entry_state=} ;; + *) continue ;; + esac + named=$((named + 1)) + done <&2 + return 1 + fi + case "$entry_state" in + QUEUED|AWAITING_CHECKS|MERGEABLE|UNMERGEABLE|LOCKED) ;; + *) + printf 'error: enqueuePullRequest returned an unknown merge-queue state (%s)\n' \ + "${entry_state:-empty}" >&2 + return 1 + ;; + esac +} + record_pr_metadata() { if ! "$SCRIPT_DIR/fm-pr-check.sh" "$ID" "$URL"; then return 1 @@ -610,9 +699,8 @@ record_pr_metadata() { } } -FM_PR_GITHUB_AUTO_REQUESTED=false +FM_PR_GITHUB_AUTO_REQUESTED=$FM_PR_CALLER_AUTO FM_PR_GITHUB_MERGE_ACCEPTED=false -FM_PR_GITHUB_CALLER_METHOD= # The single gate every statement about what the forge accepted, armed, or # reported has to pass. A merge command that failed accepted nothing, so no @@ -641,30 +729,18 @@ github_state_is_open() { esac } -# Whether the caller already asked the forge to choose the merge method, in any -# of the spellings caller_merge_method normalises to queue. That is the one -# method request a queue-governed base accepts, so it is what this refusal -# treats as flags the caller had already got right. -github_caller_method_is_forge_decides() { - [ "$FM_PR_GITHUB_CALLER_METHOD" = queue ] -} - -# The one retry a queue-governed base accepts. It names no strategy, because the -# queue sets the merge method itself and refuses an explicit one, so it does not -# vary with the queue's configured method and stays nameable even when that -# method is ambiguous or unrecognised. +# The one retry a queue-governed base accepts. `--queue` is parsed by this +# script and invokes enqueuePullRequest directly, so the command names neither +# a merge strategy nor gh-axi's unrelated auto-merge flag. github_queue_retry_command() { - printf '%s %s %s -- --auto --no-method' "$0" "$ID" "$URL" + printf '%s %s %s -- --queue' "$0" "$ID" "$URL" } -# Whether the caller's own arguments already are the retry a queue-governed base -# takes, which makes naming that retry an echo of the command just run. The -# retry is one fixed command, so whether it would repeat the caller is settled -# by what they typed: it is read from their arguments alone, never from what the -# merge command then returned or from which rules outcome came back, so no path -# can reach the point of handing the caller their own command back. +# A caller who supplied any accepted queue spelling already requested the one +# operation the retry would perform. Never hand that same operation back under +# either the canonical spelling or a legacy alias. github_caller_already_ran_queue_retry() { - [ "$FM_PR_GITHUB_AUTO_REQUESTED" = true ] && github_caller_method_is_forge_decides + [ "$FM_PR_CALLER_QUEUE" = true ] } github_report_queue_rules() { @@ -703,7 +779,7 @@ github_report_queue_rules() { ;; esac if github_caller_already_ran_queue_retry; then - printf 'error: %s; --auto --no-method is the only thing that base takes and this run already used it, so no different retry exists to name: the outcome reported above for %s is the blocking cause, and re-check the pull request'"'"'s merge queue state before running the same command again\n' \ + printf 'error: %s; enqueuePullRequest is the one supported queue operation and this run already requested it, so no different retry exists to name: the outcome reported above for %s is the blocking cause, and re-check the pull request'"'"'s merge queue state before running the same command again\n' \ "$situation" "$URL" >&2 else printf 'error: %s; retry with: %s\n' "$situation" "$(github_queue_retry_command)" >&2 @@ -761,30 +837,64 @@ case "$PROVIDER" in github) merge_output= merge_args=() - if ! caller_has_merge_method "$@"; then - merge_args=(--squash) - fi - if caller_requested_auto_merge "$@"; then - FM_PR_GITHUB_AUTO_REQUESTED=true - fi - FM_PR_GITHUB_CALLER_METHOD=$(caller_merge_method "$@") - github_drop_forge_decides_method "$@" - if merge_output=$(gh-axi pr merge "$PR_NUMBER" --repo "$PR_OWNER/$PR_REPO" \ - "${merge_args[@]+"${merge_args[@]}"}" \ - "${GITHUB_MERGE_FORWARD[@]+"${GITHUB_MERGE_FORWARD[@]}"}" 2>&1); then - FM_PR_GITHUB_MERGE_ACCEPTED=true + if [ "$FM_PR_CALLER_QUEUE" = true ]; then + github_read_enqueue_preflight || exit 1 + if [ "$FM_PR_GITHUB_MERGED" = true ]; then + : # The ordinary outcome path below records the already-landed result. + elif [ "$FM_PR_GITHUB_QUEUED" = true ]; then + printf 'verified: %s is already queued (state=%s, merged=%s, isInMergeQueue=%s)\n' \ + "$URL" "$FM_PR_GITHUB_STATE" "$FM_PR_GITHUB_MERGED" "$FM_PR_GITHUB_QUEUED" + exit 0 + else + github_read_queue_method + case "$FM_PR_GITHUB_QUEUE_STATUS" in + single|conflicting|unrecognised) ;; + none) + printf 'error: refusing to enqueue %s: base branch %s has no effective merge-queue rule\n' \ + "$URL" "$FM_PR_GITHUB_BASE" >&2 + exit 1 + ;; + *) + printf 'error: refusing to enqueue %s: the merge-queue rule for base branch %s could not be read\n' \ + "$URL" "$FM_PR_GITHUB_BASE" >&2 + exit 1 + ;; + esac + enqueue_status=0 + github_enqueue_pull_request || enqueue_status=$? + if [ "$enqueue_status" -ne 0 ]; then + if github_read_outcome; then + if [ "$FM_PR_GITHUB_QUEUED" = true ]; then + printf 'actionable: enqueuePullRequest for %s failed, but the pull request now reads as queued\n' \ + "$URL" >&2 + else + github_report_unmerged_outcome + fi + fi + exit "$enqueue_status" + fi + fi else - merge_status=$? - [ -z "$merge_output" ] || printf '%s\n' "$merge_output" >&2 - if github_read_outcome; then - if [ "$FM_PR_GITHUB_MERGED" != true ] && [ "$FM_PR_GITHUB_QUEUED" != true ]; then - github_report_unmerged_outcome - else - printf 'actionable: the merge command for %s failed, but the pull request reads back as state=%s, merged=%s, isInMergeQueue=%s\n' \ - "$URL" "$FM_PR_GITHUB_STATE" "$FM_PR_GITHUB_MERGED" "$FM_PR_GITHUB_QUEUED" >&2 + if [ "$FM_PR_CALLER_HAS_METHOD" != true ]; then + merge_args=(--squash) + fi + if merge_output=$(gh-axi pr merge "$PR_NUMBER" --repo "$PR_OWNER/$PR_REPO" \ + "${merge_args[@]+"${merge_args[@]}"}" \ + "${FM_PR_CALLER_FORWARD[@]+"${FM_PR_CALLER_FORWARD[@]}"}" 2>&1); then + FM_PR_GITHUB_MERGE_ACCEPTED=true + else + merge_status=$? + [ -z "$merge_output" ] || printf '%s\n' "$merge_output" >&2 + if github_read_outcome; then + if [ "$FM_PR_GITHUB_MERGED" != true ] && [ "$FM_PR_GITHUB_QUEUED" != true ]; then + github_report_unmerged_outcome + else + printf 'actionable: the merge command for %s failed, but the pull request reads back as state=%s, merged=%s, isInMergeQueue=%s\n' \ + "$URL" "$FM_PR_GITHUB_STATE" "$FM_PR_GITHUB_MERGED" "$FM_PR_GITHUB_QUEUED" >&2 + fi fi + exit "$merge_status" fi - exit "$merge_status" fi if ! github_read_outcome; then github_report_forge_output "$merge_output" diff --git a/docs/architecture.md b/docs/architecture.md index f116a22bb13..cbb15f4f1f6 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -305,17 +305,20 @@ It is also the one owner of the no-mistakes `--intent` contract those workers fo 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. Where a no-mistakes pipeline stores evidence in the repo, it publishes that PR-viewable validation evidence to an orphan evidence branch that shares no history with code branches, so it never enters the crew branch or the default branch. This repo uses that setting, and its own `.no-mistakes/` directory remains local state that stays gitignored and is rejected by CI if tracked; [`configuration.md`](configuration.md) owns the setting. -PR-based task merges go through `bin/fm-pr-merge.sh`, which records `pr=` and any available `pr_head=` through `bin/fm-pr-check.sh` before calling the forge CLI. +PR-based task merges go through `bin/fm-pr-merge.sh`, which records `pr=` and any available `pr_head=` through `bin/fm-pr-check.sh` before calling the forge. The helper requires a full canonical URL and rejects malformed URLs or repo override flags before recording merge state. -A `https://github.com///pull/` URL invokes `gh-axi pr merge --repo /` with the merge-method rules owned by that helper's header, including an explicit forge-decides method for merge-queue branches. -A GitHub merge-queue enqueue leaves the pull request open until it lands, and the forge CLI still reports that call as a merge, so the same live outcome read that follows every GitHub merge names a queued pull request as queued rather than merged; landing stays confirmed by the merge poll and teardown, which wait for a merged state, with empirical queue-state evidence in [`docs/verification/github-merge-queue.md`](verification/github-merge-queue.md). +When task metadata carries `yolo=on`, `bin/fm-pr-check.sh` derives the GitHub repository from that canonical URL and requires live push permission before accepting the PR as ready, so a read-only upstream cannot look like an autonomously landable delivery; `yolo=off` contribution workflows remain unchanged. +A normal `https://github.com///pull/` URL invokes `gh-axi pr merge --repo /` with the merge-method rules owned by that helper's header. +The canonical `--queue` argument and its retained legacy aliases instead invoke GitHub's GraphQL `enqueuePullRequest` operation, after live repository permission, PR identity, head, red-check, and branch-rule checks. +The mutation is bound to the verified head through `expectedHeadOid`, and its returned queue entry is followed by an independent live queue-membership read. +A GitHub merge-queue enqueue leaves the pull request open until it lands, so landing stays confirmed by the merge poll and teardown, which wait for a merged state, with empirical queue-state evidence in [`docs/verification/github-merge-queue.md`](verification/github-merge-queue.md). A `https:////-/merge_requests/` URL (see [docs/gitlab-merge-watch.md](gitlab-merge-watch.md)) invokes `glab mr merge -R https:///`, so the instance comes from the URL, and adds no merge-method flag because the project's own merge method applies. That path merges only after one live read of the merge request confirms it is open, mergeable, conflict-free, with blocking discussions resolved and a successful pipeline at the current head, and it binds the merge to that verified head; recorded metadata is never the authority for those conditions because a rebase leaves it stale. After either forge command returns, the script confirms the PR or MR actually landed, and only a confirmed landing records a landed outcome; a queued or unconfirmed request records none and leaves its poll armed. On GitLab an auto-merge-queued or unconfirmed request is reported without failing the run. -On GitHub an outcome that is neither merged nor queued is refused loudly and non-zero, naming the observed state, and a base branch that requires the merge queue is refused with the concrete forge-decides retry that base accepts rather than having a merge method chosen on the caller's behalf. -That retry names no strategy, because a queue-governed base sets the merge method itself and refuses an explicit one, so it is the same command whether the queue's configured method is known, ambiguous or unrecognised. -When the caller's own arguments already were exactly those flags, that refusal says no different retry exists, names the blocking cause reported above it, and points at the queue state to re-check, instead of echoing back the command just run; because the retry is one fixed command, that holds for every rules outcome and whether the merge command succeeded or failed. +On GitHub an outcome that is neither merged nor queued is refused loudly and non-zero, naming the observed state, and a base branch that requires the merge queue is refused with the concrete `--queue` retry rather than a merge strategy the queue rejects. +One parser owns the canonical queue token, retained legacy aliases, caller method and auto-merge interpretation, repeated-token refusal, and unsupported enqueue arguments. +When the caller already supplied any accepted queue token, the refusal says no different retry exists, names the blocking cause reported above it, and points at the queue state to re-check instead of echoing the operation under another spelling. An auto-merge request is held to the same standard: `--auto` that leaves the pull request neither merged nor queued is refused rather than reported as success. Every GitHub refusal states what it could not observe as plainly as what it did, so an unreadable branch-rule response, an unrecognised queue method, and a merge queue no available read can see are each named rather than left to look like a base branch with no queue at all. A confirmed merge leaves a durable role-routed outcome instead of living only in the merging agent's memory, and [`bin/fm-merge-outcome-lib.sh`](../bin/fm-merge-outcome-lib.sh)'s header owns its destination, shape, identity, normal-case deduplication, and at-least-once recovery. diff --git a/docs/verification/github-merge-queue.md b/docs/verification/github-merge-queue.md index 0a02f8d8560..efc8d2585cd 100644 --- a/docs/verification/github-merge-queue.md +++ b/docs/verification/github-merge-queue.md @@ -1,18 +1,16 @@ -# GitHub merge-queue merge verification +# GitHub merge-queue verification Audience: maintainer verification. -This record supports the current guarantee that a GitHub merge-queue enqueue is not a landing. -`bin/fm-pr-merge.sh` can omit a merge strategy so the forge chooses the method and then names the live outcome of that merge call, `bin/fm-pr-poll.sh` stays silent until the pull request is merged, and `bin/fm-teardown.sh` refuses while the pull request is still open. -The merge-method flags themselves are owned by the `bin/fm-pr-merge.sh` header. - -No public pull request was in a merge queue at collection time, so queue membership was read from live GraphQL schema enums plus GitHub's documented CLI enqueue behavior, and the poll-relevant `state` field was read from a live open pull request. +This record supports the current guarantee that `bin/fm-pr-merge.sh` uses GitHub's GraphQL `enqueuePullRequest` operation for a queue request and never treats queue membership as a landing. +The helper's header owns queue-token grammar, caller-argument interpretation, preconditions, and retry behavior. +`bin/fm-pr-poll.sh` and `bin/fm-teardown.sh` continue to accept only a merged pull request as landed. ## Environment -Recorded 2026-08-24 on GNU bash 5.2.37(1)-release (x86_64-pc-linux-gnu). +Recorded 2026-09-08 on GNU bash 5.2.37(1)-release (x86_64-pc-linux-gnu). -``` +```text $ gh --version gh version 2.46.0 (2025-01-13 Debian 2.46.0-3) https://github.com/cli/cli/releases/tag/v2.46.0 @@ -21,74 +19,74 @@ $ gh-axi --version 0.1.33 ``` -## Pull request state is not the queue entry state - -Live GraphQL introspection of `PullRequestState` has only three values. -`QUEUED` and `AWAITING_CHECKS` are not pull request states. - -``` -$ gh api graphql -f query='query { __type(name: "PullRequestState") { enumValues { name description } } }' -{"data":{"__type":{"enumValues":[{"name":"OPEN","description":"A pull request that is still open."},{"name":"CLOSED","description":"A pull request that has been closed without being merged."},{"name":"MERGED","description":"A pull request that has been closed by being merged."}]}}} -``` - -Live introspection of `MergeQueueEntryState` is the queue-entry machine the brief named. - -``` -$ gh api graphql -f query='query { __type(name: "MergeQueueEntryState") { enumValues { name description } } }' -{"data":{"__type":{"enumValues":[{"name":"QUEUED","description":"The entry is currently queued."},{"name":"AWAITING_CHECKS","description":"The entry is currently waiting for checks to pass."},{"name":"MERGEABLE","description":"The entry is currently mergeable."},{"name":"UNMERGEABLE","description":"The entry is currently unmergeable."},{"name":"LOCKED","description":"The entry is currently locked."}]}}} -``` - -The merge poll and the teardown landed-work check both read `gh pr view --json state`. -That CLI, at this `gh` version, does not expose `isInMergeQueue` or `mergeQueueEntry`. - -``` -$ gh pr view https://github.com/microsoft/TypeScript/pull/63931 --json isInMergeQueue -Unknown JSON field: "isInMergeQueue" -``` - -On a live open pull request, the poll-relevant fields stay `OPEN` with a null merge time, which is the same `state` a queued pull request keeps until it lands. - -``` -$ gh pr view https://github.com/microsoft/TypeScript/pull/63931 --json state,mergedAt,mergeStateStatus,url -{"mergeStateStatus":"BLOCKED","mergedAt":null,"state":"OPEN","url":"https://github.com/microsoft/TypeScript/pull/63931"} - -$ gh api graphql -f query='query { repository(owner:"microsoft", name:"TypeScript") { pullRequest(number:63931) { url state merged mergedAt mergeStateStatus isInMergeQueue mergeQueueEntry { state position } } } }' -{"data":{"repository":{"pullRequest":{"url":"https://github.com/microsoft/TypeScript/pull/63931","state":"OPEN","merged":false,"mergedAt":null,"mergeStateStatus":"BLOCKED","isInMergeQueue":false,"mergeQueueEntry":null}}}} +## Supported enqueue operation + +Live GitHub GraphQL introspection reports `pullRequestId` and `expectedHeadOid` on `EnqueuePullRequestInput`. +The helper supplies both, with the head read during its live preflight. + +```text +$ jq -nc --arg q 'query { __type(name: "EnqueuePullRequestInput") { inputFields { name type { kind name ofType { kind name } } } } }' '{query:$q}' | gh-axi api POST /graphql --input - --jq '.data.__type.inputFields' +[4]: + - name: clientMutationId + type: + kind: SCALAR + name: String + ofType: null + - name: pullRequestId + type: + kind: NON_NULL + name: null + ofType: + kind: SCALAR + name: ID + - name: jump + type: + kind: SCALAR + name: Boolean + ofType: null + - name: expectedHeadOid + type: + kind: SCALAR + name: GitObjectID + ofType: null ``` -## Enqueue is not a merge - -`gh pr merge --help` on this CLI version states that a merge-queue target needs no strategy, enables auto-merge when checks have not passed, and adds the pull request to the queue when they have. - -``` -When targeting a branch that requires a merge queue, no merge strategy is required. -If required checks have not yet passed, auto-merge will be enabled. -If required checks have passed, the pull request will be added to the merge queue. +The payload exposes the queue entry that the helper requires before it performs an independent membership read. + +```text +$ jq -nc --arg q 'query { __type(name: "EnqueuePullRequestPayload") { fields { name type { kind name ofType { kind name } } } } }' '{query:$q}' | gh-axi api POST /graphql --input - --jq '.data.__type.fields' +[2]: + - name: clientMutationId + type: + kind: SCALAR + name: String + ofType: null + - name: mergeQueueEntry + type: + kind: OBJECT + name: MergeQueueEntry + ofType: null ``` -Passing an explicit strategy is what GitHub rejects on those branches, with the refusal `The merge strategy for is set by the merge queue`. +A queue entry and a pull request have different state machines. +A queued pull request remains open until GitHub lands it, while `MergeQueueEntryState` carries `QUEUED`, `AWAITING_CHECKS`, `MERGEABLE`, `UNMERGEABLE`, or `LOCKED`. +The merge poll therefore remains silent for an open queued pull request and reports only the later merged state. -`gh-axi` 0.1.33 accepts `--method` only as `merge`, `squash`, or `rebase`, so `--method=queue` must be consumed by `bin/fm-pr-merge.sh` and not forwarded. +## Delivery-target authority -Because the merge call succeeds either way and the forge CLI labels the enqueue a merge, the same live outcome read that follows every GitHub merge names whether the pull request is merged or in the merge queue. -That read is queue-aware when `gh` is present (`isInMergeQueue`) and degrades to the gh-axi view otherwise. +The consolidated delivery target is `cisrd/firstmate`, where the authenticated captain account has push authority. +The former upstream target `kunchenguid/firstmate` is read-only for the same account. -``` -$ gh pr view 63931 --repo microsoft/TypeScript --json state -q .state -OPEN +```text +$ gh-axi api /repos/cisrd/firstmate --jq '.permissions.push' +true -$ gh pr view 63927 --repo microsoft/TypeScript --json state -q .state -MERGED +$ gh-axi api /repos/kunchenguid/firstmate --jq '.permissions.push' +false ``` -An outcome that is neither merged nor queued is refused. -An unreadable outcome is refused and keeps the merge poll armed. -Landing is still confirmed by the merge poll and teardown, which accept `MERGED` alone. - -## GitLab is unchanged - -`--no-method`, `--method=queue`, and `--method queue` are firstmate-level tokens that no `glab` flag defines, and GitLab already applies the project's own merge method. -They are refused by name on GitLab before any state is recorded, rather than forwarded to `glab` after the merge is armed, and a merge method the caller spells for `glab` itself still forwards untouched. +`bin/fm-pr-check.sh` performs the same URL-derived live permission read for a GitHub task carrying `yolo=on` before it records the PR as ready. +A task without autonomous merge authority keeps the existing upstream-contribution path. ## Portable regressions @@ -98,6 +96,7 @@ bin/fm-test-run.sh tests/fm-pr-check-security.test.sh bin/fm-test-run.sh tests/fm-teardown.test.sh ``` -The merge tests prove `--method=queue`, `--method queue`, and `--no-method` invoke `gh-axi pr merge` with no strategy flag, that a queue-governed base is refused with a forge-decides retry rather than an explicit strategy the queue would reject, that the printed retry is itself accepted when run, that a caller whose own arguments already are that retry is told no different retry exists instead of being handed their own command back, on every rules outcome that would otherwise name it and whether the merge command succeeded or failed, that combining one of those tokens with an explicit GitHub strategy is refused before the forge is called, that the same live outcome read names a queued pull request as queued and a merged one as merged, that an unreadable state refuses rather than claiming a merge, and that those tokens are refused on GitLab before any state is recorded while a real GitLab merge method still forwards. -The poll contract stays silent for `OPEN`, `CLOSED`, an empty state, and a malformed one, and emits `merged` only for `MERGED`. -Teardown refuses an open pull request whose commits are not otherwise landed. +The merge tests execute the public wrapper and prove that the canonical `--queue` token and all retained aliases call `enqueuePullRequest`, bind `expectedHeadOid`, require an effective queue rule, reject a red status rollup and read-only repository, and independently confirm queue membership. +They also prove that direct-merge refusals emit a runnable canonical retry, never repeat a queue operation the caller already requested, reject repeated queue tokens, and reject arguments the mutation cannot honor. +The PR-check tests prove that autonomous delivery accepts a writable URL-derived target and refuses read-only or unverifiable targets before recording readiness. +The teardown tests prove that an open pull request, including one waiting in a merge queue, is not landed work. diff --git a/tests/fm-pr-check-security.test.sh b/tests/fm-pr-check-security.test.sh index e5e3eb09ede..2f116e23ff3 100755 --- a/tests/fm-pr-check-security.test.sh +++ b/tests/fm-pr-check-security.test.sh @@ -162,6 +162,10 @@ case "${1:-} ${2:-}" in [ "$#" -eq 5 ] && [ "${4:-}" = --repo ] || exit 2 printf 'pull_request:\n number: %s\n state: %s\n' "$3" "${FM_TEST_GH_MERGE_STATE:-merged}" ;; + api\ *) + [ "${FM_TEST_GH_PERMISSION_READ_FAIL:-0}" = 0 ] || exit 1 + printf '%s\n' "${FM_TEST_GH_PUSH_PERMISSION:-true}" + ;; esac exit "${FM_TEST_GH_AXI_RC:-0}" SH @@ -482,6 +486,54 @@ test_invalid_entrypoints_have_zero_side_effects() { pass "PR and teardown entrypoints reject invalid arguments before every side effect" } +test_autonomous_github_target_requires_live_write_permission() { + local dir rc + + dir=$(make_case autonomous-target-writable) + write_task_meta "$dir" + printf '%s\n' 'yolo=on' >> "$dir/home/state/task-a.meta" + FM_TEST_GH_PUSH_PERMISSION=true run_check_entry "$dir" task-a \ + https://github.com/cisrd/firstmate/pull/8 > "$dir/stdout" 2> "$dir/stderr" \ + || fail "autonomous-target-writable: writable target was refused" + assert_grep 'api /repos/cisrd/firstmate --jq .permissions.push' "$dir/gh-axi.log" \ + "autonomous-target-writable: permission was not read for the URL-derived repository" + assert_grep 'pr=https://github.com/cisrd/firstmate/pull/8' "$dir/home/state/task-a.meta" \ + "autonomous-target-writable: writable target was not recorded" + + dir=$(make_case autonomous-target-read-only) + write_task_meta "$dir" + printf '%s\n' 'yolo=on' >> "$dir/home/state/task-a.meta" + set +e + FM_TEST_GH_PUSH_PERMISSION=false run_check_entry "$dir" task-a \ + https://github.com/kunchenguid/firstmate/pull/3335 > "$dir/stdout" 2> "$dir/stderr" + rc=$? + set -e + [ "$rc" -ne 0 ] || fail "autonomous-target-read-only: read-only target was accepted" + assert_grep 'refusing autonomous delivery to read-only GitHub repository kunchenguid/firstmate' \ + "$dir/stderr" "autonomous-target-read-only: refusal did not name the URL-derived target" + assert_no_grep '^pr=' "$dir/home/state/task-a.meta" \ + "autonomous-target-read-only: read-only PR was recorded as ready" + assert_absent "$dir/home/state/task-a.check.sh" \ + "autonomous-target-read-only: read-only PR armed a merge poll" + [ ! -s "$dir/guard.log" ] \ + || fail "autonomous-target-read-only: refusal happened after the ordinary ready path began" + + dir=$(make_case autonomous-target-permission-unreadable) + write_task_meta "$dir" + printf '%s\n' 'yolo=on' >> "$dir/home/state/task-a.meta" + set +e + FM_TEST_GH_PERMISSION_READ_FAIL=1 run_check_entry "$dir" task-a \ + https://github.com/cisrd/firstmate/pull/9 > "$dir/stdout" 2> "$dir/stderr" + rc=$? + set -e + [ "$rc" -ne 0 ] || fail "autonomous-target-permission-unreadable: unknown permission was accepted" + assert_grep 'could not verify live write permission for autonomous delivery target cisrd/firstmate' \ + "$dir/stderr" "autonomous-target-permission-unreadable: unknown permission was not reported" + assert_no_grep '^pr=' "$dir/home/state/task-a.meta" \ + "autonomous-target-permission-unreadable: unverifiable PR was recorded as ready" + pass "autonomous GitHub delivery accepts writable targets and refuses read-only or unverifiable ones" +} + test_valid_recording_and_merge_derivation() { local dir expected sidecar count rc dir=$(make_case valid-recording) @@ -2136,6 +2188,7 @@ test_retirement_refuses_replacement_and_nonterminal_results test_retirement_queue_failure_and_receipt_tampering test_gitlab_merged_poll_retires test_invalid_entrypoints_have_zero_side_effects +test_autonomous_github_target_requires_live_write_permission test_valid_recording_and_merge_derivation test_rejected_metacharacter_bytes_are_inert test_static_poll_contract diff --git a/tests/fm-pr-merge.test.sh b/tests/fm-pr-merge.test.sh index 1045880db1e..b73eef7d7ea 100755 --- a/tests/fm-pr-merge.test.sh +++ b/tests/fm-pr-merge.test.sh @@ -155,7 +155,26 @@ case "\${1:-} \${2:-}" in esac ;; "api graphql") - cat "\$FM_TEST_GH_OUTCOME" + case " \$* " in + *enqueuePullRequest*) + [ ! -e "\$FM_TEST_GH_CASE/enqueue-fails" ] || { + echo "GraphQL: enqueue refused" >&2 + exit 1 + } + if [ ! -e "\$FM_TEST_GH_CASE/enqueue-does-not-stick" ]; then + sed 's/^queued=.*/queued=true/' "\$FM_TEST_GH_OUTCOME" > "\$FM_TEST_GH_OUTCOME.tmp" + mv "\$FM_TEST_GH_OUTCOME.tmp" "\$FM_TEST_GH_OUTCOME" + fi + printf '%s\n' 'entry_id=QUEUE_ENTRY' 'entry_state=QUEUED' + ;; + *viewerPermission*) + cat "\$FM_TEST_GH_OUTCOME" + printf '%s\n' 'node=PR_NODE' "head=\$(cat "\$FM_TEST_GH_CASE/github-head")" \ + "checks=\$(cat "\$FM_TEST_GH_CASE/github-checks")" \ + "permission=\$(cat "\$FM_TEST_GH_CASE/github-permission")" + ;; + *) cat "\$FM_TEST_GH_OUTCOME" ;; + esac exit 0 ;; api\ *) @@ -166,12 +185,15 @@ esac exit 0 SH chmod +x "$case_dir/fakebin/gh-axi" "$case_dir/fakebin/gh" + printf '%s\n' "$head" > "$case_dir/github-head" + printf '%s\n' SUCCESS > "$case_dir/github-checks" + printf '%s\n' WRITE > "$case_dir/github-permission" } # gh-axi mock that fails the merge call but succeeds everything else, so a # real merge failure is distinguishable from the recording step. add_gh_mocks_merge_fails() { - local case_dir=$1 + local case_dir=$1 head=8585858585858585858585858585858585858585 cat > "$case_dir/fakebin/gh-axi" <<'SH' #!/usr/bin/env bash printf '%s\n' "$*" >> "$FM_TEST_GH_AXI_LOG" @@ -185,7 +207,19 @@ SH printf '%s\n' "$*" >> "$FM_TEST_GH_LOG" case "${1:-} ${2:-}" in "api graphql") - cat "$FM_TEST_GH_OUTCOME" + case " $* " in + *enqueuePullRequest*) + echo "GraphQL: enqueue refused" >&2 + exit 1 + ;; + *viewerPermission*) + cat "$FM_TEST_GH_OUTCOME" + printf '%s\n' 'node=PR_NODE' "head=$(cat "$FM_TEST_GH_CASE/github-head")" \ + "checks=$(cat "$FM_TEST_GH_CASE/github-checks")" \ + "permission=$(cat "$FM_TEST_GH_CASE/github-permission")" + ;; + *) cat "$FM_TEST_GH_OUTCOME" ;; + esac exit 0 ;; api\ *) @@ -196,6 +230,9 @@ esac exit 0 SH chmod +x "$case_dir/fakebin/gh-axi" "$case_dir/fakebin/gh" + printf '%s\n' "$head" > "$case_dir/github-head" + printf '%s\n' SUCCESS > "$case_dir/github-checks" + printf '%s\n' WRITE > "$case_dir/github-permission" } # gh mock that still answers fm-pr-check.sh's head lookup but cannot answer the @@ -373,6 +410,7 @@ run_pr_merge() { FM_TEST_GH_LOG="$case_dir/gh.log" \ FM_TEST_GH_OUTCOME="$case_dir/github-outcome" \ FM_TEST_GH_RULES="$case_dir/github-rules" \ + FM_TEST_GH_CASE="$case_dir" \ FM_TEST_META_AT_MERGE="$case_dir/meta-at-merge" \ FM_TEST_REAL_MV="$REAL_MV" \ FM_TEST_GLAB_LOG="$case_dir/glab.log" \ @@ -722,7 +760,7 @@ test_github_failed_merge_with_queue_flags_never_claims_acceptance() { assert_grep 'base branch main requires the merge queue, which sets the merge method (merge) itself and refuses an explicit strategy; retry with:' \ "$case_dir/stderr" \ "github-failed-merge-queue-flags: the failed merge command lost its concrete retry guidance" - assert_grep 'task-x1 https://github.com/example/repo/pull/74 -- --auto --no-method' "$case_dir/stderr" \ + assert_grep 'task-x1 https://github.com/example/repo/pull/74 -- --queue' "$case_dir/stderr" \ "github-failed-merge-queue-flags: the retry guidance named no queue-accepted flags" assert_no_grep 'verified: ' "$case_dir/stdout" \ "github-failed-merge-queue-flags: a failed merge command was reported as verified" @@ -736,11 +774,12 @@ test_github_accepted_queue_flags_do_not_echo_back_the_same_command() { add_gh_mocks "$case_dir" 8181818181818181818181818181818181818181 write_github_outcome "$case_dir" OPEN false false main printf 'merge_method=MERGE\n' > "$case_dir/github-rules" + : > "$case_dir/enqueue-does-not-stick" : > "$case_dir/gh-axi.log" : > "$case_dir/gh.log" set +e - run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/68 -- --auto --no-method \ + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/68 -- --queue \ > "$case_dir/stdout" 2> "$case_dir/stderr" rc=$? set -e @@ -751,7 +790,7 @@ test_github_accepted_queue_flags_do_not_echo_back_the_same_command() { assert_grep 'base branch main requires the merge queue, which sets the merge method (merge) itself and refuses an explicit strategy' \ "$case_dir/stderr" \ "github-accepted-queue-flags: the refusal stopped naming what the queue itself applies" - assert_grep '--auto --no-method is the only thing that base takes and this run already used it, so no different retry exists to name' \ + assert_grep 'enqueuePullRequest is the one supported queue operation and this run already requested it, so no different retry exists to name' \ "$case_dir/stderr" \ "github-accepted-queue-flags: the refusal did not explain that the right flags were already used" assert_grep 'the outcome reported above for https://github.com/example/repo/pull/68 is the blocking cause' \ @@ -786,7 +825,7 @@ test_github_mismatched_queue_flags_still_name_the_retry() { assert_grep 'base branch main requires the merge queue, which sets the merge method (rebase) itself and refuses an explicit strategy; retry with:' \ "$case_dir/stderr" \ "github-mismatched-queue-flags: a caller method the queue does not use lost its retry guidance" - assert_grep '-- --auto --no-method' "$case_dir/stderr" \ + assert_grep '-- --queue' "$case_dir/stderr" \ "github-mismatched-queue-flags: the exact compatible flags were not named" assert_no_grep '-- --auto --rebase' "$case_dir/stderr" \ "github-mismatched-queue-flags: the retry named a strategy the merge queue refuses" @@ -813,7 +852,7 @@ test_github_unrecognised_queue_method_still_names_the_queue() { assert_grep 'base branch main requires the merge queue, but its configured merge method (FASTFORWARD) is not one this script recognises' \ "$case_dir/stderr" \ "github-unrecognised-queue-method: a readable queue rule produced no queue mention" - assert_grep '-- --auto --no-method' "$case_dir/stderr" \ + assert_grep '-- --queue' "$case_dir/stderr" \ "github-unrecognised-queue-method: an unrecognised method lost the retry that needs no method" for guessed in '--auto --merge' '--auto --squash' '--auto --rebase' '--auto --fastforward'; do assert_no_grep "$guessed" "$case_dir/stderr" \ @@ -852,7 +891,7 @@ test_github_queue_retry_guidance_is_runnable() { esac retry_args=${retry_cmd#"$PR_MERGE" } - write_github_outcome "$case_dir" OPEN false true main + write_github_outcome "$case_dir" OPEN false false main : > "$case_dir/gh-axi.log" set +e # shellcheck disable=SC2086 # The retry command's own arguments, as printed. @@ -861,9 +900,10 @@ test_github_queue_retry_guidance_is_runnable() { set -e expect_code 0 "$rc" "github-queue-retry-runnable: the advised retry was itself refused" - grep -qxF 'pr merge 71 --repo example/repo --auto' "$case_dir/gh-axi.log" \ - || fail "github-queue-retry-runnable: the advised retry did not reach the forge without a strategy, got '$(cat "$case_dir/gh-axi.log")'" - assert_github_merge_has_no_strategy "$case_dir/gh-axi.log" github-queue-retry-runnable + [ ! -s "$case_dir/gh-axi.log" ] \ + || fail "github-queue-retry-runnable: the advised retry called the merge parser instead of GraphQL" + assert_grep 'enqueuePullRequest' "$case_dir/gh.log" \ + "github-queue-retry-runnable: the advised retry did not invoke enqueuePullRequest" assert_grep 'verified: https://github.com/example/repo/pull/71 is queued' \ "$case_dir/stdout" "github-queue-retry-runnable: the advised retry did not land in the queue" pass "fm-pr-merge's merge-queue refusal names a retry that base branch accepts" @@ -1158,7 +1198,7 @@ test_github_zero_exit_queue_required_refuses_with_exact_retry() { "github-zero-exit-queue-required: refusal did not name the concrete observed state" assert_grep 'base branch release/2026 requires the merge queue' "$case_dir/stderr" \ "github-zero-exit-queue-required: refusal did not name the queue requirement" - assert_grep '-- --auto --no-method' "$case_dir/stderr" \ + assert_grep '-- --queue' "$case_dir/stderr" \ "github-zero-exit-queue-required: refusal did not name the exact compatible flags" assert_no_grep '-- --auto --rebase' "$case_dir/stderr" \ "github-zero-exit-queue-required: refusal named a strategy the merge queue refuses" @@ -1253,7 +1293,7 @@ test_github_queue_required_refusal_names_retry_flags() { "github-queue-required: the original forge failure was not preserved" assert_grep 'base branch master requires the merge queue' "$case_dir/stderr" \ "github-queue-required: refusal did not name the queue requirement" - grep -F -- '-- --auto --no-method' "$case_dir/stderr" >/dev/null \ + grep -F -- '-- --queue' "$case_dir/stderr" >/dev/null \ || fail "github-queue-required: refusal did not name the exact compatible flags" assert_no_grep '-- --auto --merge' "$case_dir/stderr" \ "github-queue-required: refusal named a strategy the merge queue refuses" @@ -1283,7 +1323,7 @@ test_github_agreeing_queue_rules_keep_retry_guidance() { expect_code 1 "$rc" "github-agreeing-queue-rules: an unproved merge must fail" assert_grep 'base branch main requires the merge queue' "$case_dir/stderr" \ "github-agreeing-queue-rules: refusal did not name the queue requirement" - assert_grep '-- --auto --no-method' "$case_dir/stderr" \ + assert_grep '-- --queue' "$case_dir/stderr" \ "github-agreeing-queue-rules: agreeing rules omitted exact retry flags" assert_grep 'sets the merge method (rebase) itself' "$case_dir/stderr" \ "github-agreeing-queue-rules: agreeing rules did not resolve to one merge method" @@ -1319,7 +1359,7 @@ test_github_conflicting_queue_rules_report_ambiguity() { "github-conflicting-queue-rules: an exact retry method was guessed" assert_no_grep 'SQUASH, SQUASH' "$case_dir/stderr" \ "github-conflicting-queue-rules: a repeated queue method was named twice" - assert_grep '-- --auto --no-method' "$case_dir/stderr" \ + assert_grep '-- --queue' "$case_dir/stderr" \ "github-conflicting-queue-rules: ambiguous rules lost the retry that needs no method" pass "fm-pr-merge reports ambiguity for conflicting merge-queue rules" } @@ -1349,6 +1389,7 @@ test_github_accepted_queue_flags_never_echoed_back_on_any_rules_outcome() { add_gh_mocks_merge_fails "$case_dir" else add_gh_mocks "$case_dir" 8585858585858585858585858585858585858585 + : > "$case_dir/enqueue-does-not-stick" fi write_github_outcome "$case_dir" OPEN false false main printf '%b' "$rules" > "$case_dir/github-rules" @@ -1356,7 +1397,7 @@ test_github_accepted_queue_flags_never_echoed_back_on_any_rules_outcome() { : > "$case_dir/gh.log" set +e - run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/72 -- --auto --no-method \ + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/72 -- --queue \ > "$case_dir/stdout" 2> "$case_dir/stderr" rc=$? set -e @@ -1370,7 +1411,7 @@ test_github_accepted_queue_flags_never_echoed_back_on_any_rules_outcome() { "$label: the refusal named no concrete next step" assert_no_grep 'retry with:' "$case_dir/stderr" \ "$label: the refusal echoed back the command that just refused" - assert_no_grep '-- --auto --no-method' "$case_dir/stderr" \ + assert_no_grep '-- --queue' "$case_dir/stderr" \ "$label: the refusal repeated the caller's own flags as guidance" assert_no_grep 'verified: ' "$case_dir/stdout" \ "$label: an unproved merge was reported as verified" @@ -1590,56 +1631,55 @@ test_method_equals_merge_method_not_overridden() { pass "fm-pr-merge respects --method= as an explicit merge method" } -# Assert the GitHub CLI was asked to merge without imposing a strategy, so a -# merge-queue branch can apply the project's own method. -assert_github_merge_has_no_strategy() { - local log=$1 label=$2 line flag - line=$(grep -F 'pr merge ' "$log" || true) - [ -n "$line" ] || fail "$label: gh-axi pr merge was not invoked" - for flag in --squash --rebase --merge --method --no-method; do - case "$line" in - *"$flag"*) fail "$label: '$flag' was imposed on GitHub: '$line'" ;; - esac - done -} - test_forge_decides_method_omits_strategy() { local case_dir spelling for spelling in \ + 'canonical|--queue' \ 'method-equals-queue|--method=queue' \ 'method-queue|--method queue' \ 'no-method|--no-method' do - case_dir=$(make_case "forge-decides-${spelling%%|*}") + case_dir=$(make_case "queue-token-${spelling%%|*}") mkdir -p "$case_dir/wt" add_gh_mocks "$case_dir" eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee + write_github_outcome "$case_dir" OPEN false false main + printf 'merge_method=MERGE\n' > "$case_dir/github-rules" : > "$case_dir/gh-axi.log" + : > "$case_dir/gh.log" # shellcheck disable=SC2086 # The spelling is one or two extra merge flags. run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/24 -- ${spelling#*|} \ > "$case_dir/stdout" 2> "$case_dir/stderr" \ - || fail "forge-decides-${spelling%%|*}: fm-pr-merge failed" + || fail "queue-token-${spelling%%|*}: fm-pr-merge failed" - grep -qxF 'pr merge 24 --repo example/repo' "$case_dir/gh-axi.log" \ - || fail "forge-decides-${spelling%%|*}: expected merge with no strategy, got '$(cat "$case_dir/gh-axi.log")'" - assert_github_merge_has_no_strategy "$case_dir/gh-axi.log" "forge-decides-${spelling%%|*}" + [ ! -s "$case_dir/gh-axi.log" ] \ + || fail "queue-token-${spelling%%|*}: queue request reached gh-axi's merge parser" + assert_grep 'enqueuePullRequest' "$case_dir/gh.log" \ + "queue-token-${spelling%%|*}: queue request did not use enqueuePullRequest" done - pass "fm-pr-merge omits a GitHub strategy when the caller asks the forge to decide" + pass "fm-pr-merge sends every supported queue token through enqueuePullRequest" } test_forge_decides_method_forwards_other_flags() { - local case_dir - case_dir=$(make_case forge-decides-extra-flags) + local case_dir rc + case_dir=$(make_case queue-unsupported-extra-flags) mkdir -p "$case_dir/wt" add_gh_mocks "$case_dir" ffffffffffffffffffffffffffffffffffffffff : > "$case_dir/gh-axi.log" - run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/25 -- --method=queue --delete-branch \ - > "$case_dir/stdout" 2> "$case_dir/stderr" || fail "forge-decides-extra-flags: fm-pr-merge failed" + set +e + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/25 -- --queue --delete-branch \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e - grep -qxF 'pr merge 25 --repo example/repo --delete-branch' "$case_dir/gh-axi.log" \ - || fail "forge-decides-extra-flags: extra flags were not forwarded after dropping the forge-decides method" - pass "fm-pr-merge still forwards non-strategy GitHub flags after a forge-decides method" + expect_code 1 "$rc" "queue-unsupported-extra-flags: unsupported queue flags must refuse" + assert_grep 'queue enqueue does not support extra merge arguments (--delete-branch)' \ + "$case_dir/stderr" "queue-unsupported-extra-flags: refusal did not name the unsupported flag" + [ ! -s "$case_dir/gh-axi.log" ] || fail "queue-unsupported-extra-flags: forge was called" + assert_no_grep 'pr=https://github.com/example/repo/pull/25' "$case_dir/state/task-x1.meta" \ + "queue-unsupported-extra-flags: unsupported request was recorded before refusal" + pass "fm-pr-merge refuses queue flags the GraphQL operation cannot honour" } # Combining a forge-decides token with an explicit strategy must not drop only @@ -1674,7 +1714,7 @@ test_forge_decides_conflicting_strategy_refuses_before_forge() { set -e expect_code 1 "$rc" "forge-decides-conflict-$name: fm-pr-merge should refuse the contradictory methods" - assert_grep "must not combine a forge-decides method ($forge) with an explicit merge strategy ($explicit)" \ + assert_grep "must not combine a queue request ($forge) with an explicit merge strategy ($explicit)" \ "$case_dir/stderr" "forge-decides-conflict-$name: refusal did not name both incompatible requests" [ ! -s "$case_dir/gh-axi.log" ] \ || fail "forge-decides-conflict-$name: gh-axi pr merge was invoked despite the contradictory methods" @@ -1695,7 +1735,8 @@ test_forge_decides_reports_queued_and_merged_outcomes() { case_dir=$(make_case forge-decides-queued) mkdir -p "$case_dir/wt" add_gh_mocks "$case_dir" 1111111111111111111111111111111111111111 - write_github_outcome "$case_dir" OPEN false true master + write_github_outcome "$case_dir" OPEN false false master + printf 'merge_method=MERGE\n' > "$case_dir/github-rules" : > "$case_dir/gh-axi.log" : > "$case_dir/gh.log" @@ -1706,9 +1747,10 @@ test_forge_decides_reports_queued_and_merged_outcomes() { set -e expect_code 0 "$rc" "forge-decides-queued: a queued PR should succeed" - grep -qxF 'pr merge 31 --repo example/repo' "$case_dir/gh-axi.log" \ - || fail "forge-decides-queued: expected merge with no strategy, got '$(cat "$case_dir/gh-axi.log")'" - assert_github_merge_has_no_strategy "$case_dir/gh-axi.log" "forge-decides-queued" + [ ! -s "$case_dir/gh-axi.log" ] \ + || fail "forge-decides-queued: queue request reached gh-axi's merge parser" + assert_grep 'enqueuePullRequest' "$case_dir/gh.log" \ + "forge-decides-queued: queue request did not invoke enqueuePullRequest" assert_grep 'verified: https://github.com/example/repo/pull/31 is queued' \ "$case_dir/stdout" "forge-decides-queued: success was not reported as queued" assert_no_grep 'merged:' "$case_dir/stdout" \ @@ -1728,16 +1770,120 @@ test_forge_decides_reports_queued_and_merged_outcomes() { set -e expect_code 0 "$rc" "forge-decides-landed: a merged PR should succeed" - grep -qxF 'pr merge 31 --repo example/repo' "$case_dir/gh-axi.log" \ - || fail "forge-decides-landed: expected merge with no strategy, got '$(cat "$case_dir/gh-axi.log")'" - assert_github_merge_has_no_strategy "$case_dir/gh-axi.log" "forge-decides-landed" + [ ! -s "$case_dir/gh-axi.log" ] \ + || fail "forge-decides-landed: an already-landed PR reached gh-axi's merge parser" + assert_no_grep 'enqueuePullRequest' "$case_dir/gh.log" \ + "forge-decides-landed: an already-landed PR was enqueued again" assert_grep 'verified: https://github.com/example/repo/pull/31 is merged' \ "$case_dir/stdout" "forge-decides-landed: success was not reported as verified" pass "fm-pr-merge reports an enqueued PR as queued and a landed one as merged" } -# An unreadable state on the forge-decides path must not claim the PR merged. -# Main's outcome gate refuses rather than treating that read as report-only. +test_queue_request_rejects_repeated_tokens_before_recording() { + local case_dir rc + case_dir=$(make_case queue-repeated-token) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" 4141414141414141414141414141414141414141 + + set +e + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/34 -- --queue --no-method \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "queue-repeated-token: repeated queue request must refuse" + assert_grep 'repeat the queue request (--queue --no-method); pass exactly one queue token' \ + "$case_dir/stderr" "queue-repeated-token: refusal did not name both repeated tokens" + assert_no_grep 'pr=https://github.com/example/repo/pull/34' "$case_dir/state/task-x1.meta" \ + "queue-repeated-token: repeated request was recorded before refusal" + [ ! -s "$case_dir/gh-axi.log" ] || fail "queue-repeated-token: forge was called" + pass "fm-pr-merge refuses repeated queue tokens before recording or calling the forge" +} + +test_queue_preflight_refuses_read_only_repository() { + local case_dir rc + case_dir=$(make_case queue-read-only-repository) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" 4242424242424242424242424242424242424242 + write_github_outcome "$case_dir" OPEN false false main + printf 'merge_method=MERGE\n' > "$case_dir/github-rules" + printf '%s\n' READ > "$case_dir/github-permission" + + set +e + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/35 -- --queue \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "queue-read-only-repository: read-only target must refuse" + assert_grep 'repository example/repo grants only READ permission, not merge authority' \ + "$case_dir/stderr" "queue-read-only-repository: refusal did not name live permission" + assert_no_grep 'enqueuePullRequest' "$case_dir/gh.log" \ + "queue-read-only-repository: read-only target reached enqueuePullRequest" + pass "fm-pr-merge refuses a read-only queue destination before enqueueing" +} + +test_queue_preflight_refuses_red_checks() { + local case_dir rc + case_dir=$(make_case queue-red-checks) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" 4343434343434343434343434343434343434343 + write_github_outcome "$case_dir" OPEN false false main + printf 'merge_method=MERGE\n' > "$case_dir/github-rules" + printf '%s\n' FAILURE > "$case_dir/github-checks" + + set +e + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/36 -- --queue \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "queue-red-checks: red checks must refuse" + assert_grep 'has a red status-check rollup (FAILURE)' "$case_dir/stderr" \ + "queue-red-checks: refusal did not name the red rollup" + assert_no_grep 'enqueuePullRequest' "$case_dir/gh.log" \ + "queue-red-checks: red head reached enqueuePullRequest" + pass "fm-pr-merge keeps red pull requests out of the merge queue" +} + +test_queue_preflight_and_mutation_bind_the_head() { + local case_dir rc old_head new_head + old_head=4444444444444444444444444444444444444444 + new_head=4545454545454545454545454545454545454545 + case_dir=$(make_case queue-changed-head) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" "$old_head" + write_github_outcome "$case_dir" OPEN false false main + printf 'merge_method=MERGE\n' > "$case_dir/github-rules" + printf '%s\n' "$new_head" > "$case_dir/github-head" + + set +e + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/37 -- --queue \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "queue-changed-head: changed PR head must refuse" + assert_grep "recorded head $old_head changed to $new_head before the queue request" \ + "$case_dir/stderr" "queue-changed-head: refusal did not name both head identities" + assert_no_grep 'enqueuePullRequest' "$case_dir/gh.log" \ + "queue-changed-head: changed head reached enqueuePullRequest" + + case_dir=$(make_case queue-expected-head) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" "$old_head" + write_github_outcome "$case_dir" OPEN false false main + printf 'merge_method=MERGE\n' > "$case_dir/github-rules" + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/38 -- --queue \ + > "$case_dir/stdout" 2> "$case_dir/stderr" \ + || fail "queue-expected-head: valid enqueue failed" + assert_grep "expectedHeadOid=$old_head" "$case_dir/gh.log" \ + "queue-expected-head: mutation was not bound to the preflight head" + pass "fm-pr-merge refuses changed identity and binds enqueuePullRequest to the verified head" +} + +# An unreadable preflight on the queue path must refuse before the mutation. +# Identity, permission, and head continuity are prerequisites, not report-only. test_forge_decides_unreadable_state_reports_without_failing() { local case_dir rc case_dir=$(make_case forge-decides-unreadable-state) @@ -1755,12 +1901,14 @@ test_forge_decides_unreadable_state_reports_without_failing() { set -e expect_code 1 "$rc" "forge-decides-unreadable-state: an unreadable outcome must fail" - assert_grep 'could not read the GitHub pull request outcome after the merge attempt' \ - "$case_dir/stderr" "forge-decides-unreadable-state: the unreadable state was not reported" + assert_grep 'could not read the GitHub pull request identity and merge authority before enqueueing' \ + "$case_dir/stderr" "forge-decides-unreadable-state: the unreadable preflight was not reported" assert_no_grep 'verified: ' "$case_dir/stdout" \ "forge-decides-unreadable-state: an unread state was confirmed as merged" - grep -qxF 'pr merge 32 --repo example/repo' "$case_dir/gh-axi.log" \ - || fail "forge-decides-unreadable-state: expected merge with no strategy, got '$(cat "$case_dir/gh-axi.log")'" + [ ! -s "$case_dir/gh-axi.log" ] \ + || fail "forge-decides-unreadable-state: unreadable preflight reached the merge parser" + assert_no_grep 'enqueuePullRequest' "$case_dir/gh.log" \ + "forge-decides-unreadable-state: unreadable preflight reached the enqueue mutation" assert_grep 'pr=https://github.com/example/repo/pull/32' "$case_dir/state/task-x1.meta" \ "forge-decides-unreadable-state: a successful merge call lost its PR reference" assert_present "$case_dir/state/task-x1.check.sh" \ @@ -1784,8 +1932,8 @@ test_gitlab_forge_decides_method_refuses_before_recording() { set -e expect_code 1 "$rc" "gitlab-forge-decides-${spelling%%|*}: fm-pr-merge should refuse the forge-decides token" - assert_grep 'extra merge arguments must not ask GitLab to choose the merge method' \ - "$case_dir/stderr" "gitlab-forge-decides-${spelling%%|*}: refusal did not name the forge-decides token" + assert_grep "extra merge arguments must not request GitHub's merge queue on GitLab" \ + "$case_dir/stderr" "gitlab-forge-decides-${spelling%%|*}: refusal did not name the queue token" assert_no_grep "pr=$MR_URL" "$case_dir/state/task-x1.meta" \ "gitlab-forge-decides-${spelling%%|*}: the URL was recorded before rejecting the token" assert_absent "$case_dir/state/task-x1.check.sh" \ @@ -2482,6 +2630,10 @@ test_forge_decides_method_omits_strategy test_forge_decides_method_forwards_other_flags test_forge_decides_conflicting_strategy_refuses_before_forge test_forge_decides_reports_queued_and_merged_outcomes +test_queue_request_rejects_repeated_tokens_before_recording +test_queue_preflight_refuses_read_only_repository +test_queue_preflight_refuses_red_checks +test_queue_preflight_and_mutation_bind_the_head test_forge_decides_unreadable_state_reports_without_failing test_parses_pr_url_for_gh_axi test_github_still_forwards_sha_arg diff --git a/tests/fm-teardown.test.sh b/tests/fm-teardown.test.sh index 7917bc88d36..00a0a902f4a 100755 --- a/tests/fm-teardown.test.sh +++ b/tests/fm-teardown.test.sh @@ -271,7 +271,7 @@ SH case "\${1:-} \${2:-}" in "pr view") case " \$* " in - *"state,headRefOid,url"*) printf '%s\t%s\t%s\n' 'MERGED' '$head' 'https://github.com/example/repo/pull/7' ; exit 0 ;; + *"state,headRefOid,url"*) printf '%s\t%s\t%s\n' '$upper' '$head' 'https://github.com/example/repo/pull/7' ; exit 0 ;; *"headRefOid"*) printf '%s\n' '$head' ; exit 0 ;; esac ;; @@ -282,6 +282,10 @@ SH chmod +x "$case_dir/fakebin/gh-axi" "$case_dir/fakebin/gh" } +add_gh_pr_merged_for_head() { + add_gh_pr_state_for_head "$1" merged "$2" +} + # Squash-merged history whose pipeline rebased the branch onto a newer main that # edited the same shared file. A local copy left behind by that rebase holds # different content for the shared file, so its per-commit patch ids against the @@ -368,6 +372,8 @@ assert_refusal_retained_task_state() { || fail "$label: refusal moved the task branch off the unlanded commit" [ -e "$case_dir/state/task-x1.meta" ] \ || fail "$label: refusal erased the durable task record" +} + # Override GitHub lookups to report PR 7 as still open with the supplied head. # A merge-queue enqueue leaves the pull request OPEN until it actually lands. add_gh_pr_open_for_head() { From 69b774ceac7d8a504856982afa9325681baffdb8 Mon Sep 17 00:00:00 2001 From: Alex William Date: Tue, 8 Sep 2026 18:51:10 +0200 Subject: [PATCH 17/18] no-mistakes(review): Remove intermediate queue aliases and unused teardown fixture --- bin/fm-pr-merge.sh | 21 +++------ tests/fm-pr-merge.test.sh | 95 +++++++++++++++++---------------------- tests/fm-teardown.test.sh | 29 ------------ 3 files changed, 46 insertions(+), 99 deletions(-) diff --git a/bin/fm-pr-merge.sh b/bin/fm-pr-merge.sh index 43d7f3ac898..365cfe807cc 100755 --- a/bin/fm-pr-merge.sh +++ b/bin/fm-pr-merge.sh @@ -9,8 +9,6 @@ # GitHub direct merges default to --squash when the caller names no method. # `--queue` is the canonical merge-queue request and invokes GitHub's supported # GraphQL enqueuePullRequest mutation rather than the gh-axi merge parser. -# --no-method, --method=queue, and --method queue remain accepted aliases for -# retry commands emitted by the earlier implementation. # One parser owns every queue spelling and all caller-argument interpretation. # It refuses repeated queue tokens, a queue token mixed with an explicit merge # strategy, and any extra argument the enqueue mutation cannot honour. @@ -110,9 +108,7 @@ shift 2 # Parse the caller's merge arguments once for every provider path. # This is the single owner of queue-token grammar and of how caller arguments -# are interpreted. `--queue` is the canonical enqueue request; --no-method, -# --method=queue, and --method queue remain accepted aliases for commands the -# earlier merge-queue implementation printed. Queue tokens are Firstmate flags, +# are interpreted. Queue tokens are Firstmate flags, # never forge CLI flags. A repeated queue token, a queue token mixed with an # explicit strategy, or an argument the GraphQL enqueue path cannot honour is # refused rather than dropped or forwarded with changed meaning. @@ -139,21 +135,14 @@ fm_pr_parse_merge_args() { arg=$1 shift if [ "$pending_method" = true ]; then - if [ "$arg" = queue ]; then - FM_PR_CALLER_QUEUE=true - FM_PR_CALLER_QUEUE_COUNT=$((FM_PR_CALLER_QUEUE_COUNT + 1)) - FM_PR_CALLER_QUEUE_TOKENS="${FM_PR_CALLER_QUEUE_TOKENS:+$FM_PR_CALLER_QUEUE_TOKENS }--method queue" - FM_PR_CALLER_METHOD=queue - else - FM_PR_CALLER_METHOD=$arg - FM_PR_CALLER_EXPLICIT_METHODS="${FM_PR_CALLER_EXPLICIT_METHODS:+$FM_PR_CALLER_EXPLICIT_METHODS }--method $arg" - FM_PR_CALLER_FORWARD+=(--method "$arg") - fi + FM_PR_CALLER_METHOD=$arg + FM_PR_CALLER_EXPLICIT_METHODS="${FM_PR_CALLER_EXPLICIT_METHODS:+$FM_PR_CALLER_EXPLICIT_METHODS }--method $arg" + FM_PR_CALLER_FORWARD+=(--method "$arg") pending_method=false continue fi case "$arg" in - --queue|--no-method|--method=queue) + --queue) FM_PR_CALLER_HAS_METHOD=true FM_PR_CALLER_METHOD=queue FM_PR_CALLER_QUEUE=true diff --git a/tests/fm-pr-merge.test.sh b/tests/fm-pr-merge.test.sh index b73eef7d7ea..b8f1bc97834 100755 --- a/tests/fm-pr-merge.test.sh +++ b/tests/fm-pr-merge.test.sh @@ -13,7 +13,7 @@ # (e) PR URL is parsed to number + --repo for gh-axi (defaults to --squash) # (f) malformed PR URL fails fast without calling gh-axi # (g) explicit merge method is not overridden by the default --squash -# (g2) --method=queue, --method queue, and --no-method skip default --squash +# (g2) --queue skips default --squash # and forward no strategy flag, so a merge-queue branch can choose # (g3) that path uses the same live outcome read as every other GitHub merge, # so an enqueued still-open PR is named queued rather than merged @@ -1632,31 +1632,23 @@ test_method_equals_merge_method_not_overridden() { } test_forge_decides_method_omits_strategy() { - local case_dir spelling - for spelling in \ - 'canonical|--queue' \ - 'method-equals-queue|--method=queue' \ - 'method-queue|--method queue' \ - 'no-method|--no-method' - do - case_dir=$(make_case "queue-token-${spelling%%|*}") - mkdir -p "$case_dir/wt" - add_gh_mocks "$case_dir" eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee - write_github_outcome "$case_dir" OPEN false false main - printf 'merge_method=MERGE\n' > "$case_dir/github-rules" - : > "$case_dir/gh-axi.log" - : > "$case_dir/gh.log" + local case_dir + case_dir=$(make_case "queue-token-canonical") + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee + write_github_outcome "$case_dir" OPEN false false main + printf 'merge_method=MERGE\n' > "$case_dir/github-rules" + : > "$case_dir/gh-axi.log" + : > "$case_dir/gh.log" - # shellcheck disable=SC2086 # The spelling is one or two extra merge flags. - run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/24 -- ${spelling#*|} \ - > "$case_dir/stdout" 2> "$case_dir/stderr" \ - || fail "queue-token-${spelling%%|*}: fm-pr-merge failed" + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/24 -- --queue \ + > "$case_dir/stdout" 2> "$case_dir/stderr" \ + || fail "queue-token-canonical: fm-pr-merge failed" - [ ! -s "$case_dir/gh-axi.log" ] \ - || fail "queue-token-${spelling%%|*}: queue request reached gh-axi's merge parser" - assert_grep 'enqueuePullRequest' "$case_dir/gh.log" \ - "queue-token-${spelling%%|*}: queue request did not use enqueuePullRequest" - done + [ ! -s "$case_dir/gh-axi.log" ] \ + || fail "queue-token-canonical: queue request reached gh-axi's merge parser" + assert_grep 'enqueuePullRequest' "$case_dir/gh.log" \ + "queue-token-canonical: queue request did not use enqueuePullRequest" pass "fm-pr-merge sends every supported queue token through enqueuePullRequest" } @@ -1688,12 +1680,10 @@ test_forge_decides_method_forwards_other_flags() { test_forge_decides_conflicting_strategy_refuses_before_forge() { local case_dir rc name rest args forge explicit for spec in \ - 'queue-squash|--method=queue --squash|--method=queue|--squash' \ - 'queue-merge|--method=queue --merge|--method=queue|--merge' \ - 'queue-rebase|--method=queue --rebase|--method=queue|--rebase' \ - 'no-method-squash|--no-method --squash|--no-method|--squash' \ - 'method-queue-merge|--method queue --merge|--method queue|--merge' \ - 'queue-method-squash|--method=queue --method=squash|--method=queue|--method=squash' + 'queue-squash|--queue --squash|--queue|--squash' \ + 'queue-merge|--queue --merge|--queue|--merge' \ + 'queue-rebase|--queue --rebase|--queue|--rebase' \ + 'queue-method-squash|--queue --method=squash|--queue|--method=squash' do name=${spec%%|*} rest=${spec#*|} @@ -1741,7 +1731,7 @@ test_forge_decides_reports_queued_and_merged_outcomes() { : > "$case_dir/gh.log" set +e - run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/31 -- --method=queue \ + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/31 -- --queue \ > "$case_dir/stdout" 2> "$case_dir/stderr" rc=$? set -e @@ -1764,7 +1754,7 @@ test_forge_decides_reports_queued_and_merged_outcomes() { : > "$case_dir/gh.log" set +e - run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/31 -- --method=queue \ + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/31 -- --queue \ > "$case_dir/stdout" 2> "$case_dir/stderr" rc=$? set -e @@ -1786,13 +1776,13 @@ test_queue_request_rejects_repeated_tokens_before_recording() { add_gh_mocks "$case_dir" 4141414141414141414141414141414141414141 set +e - run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/34 -- --queue --no-method \ + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/34 -- --queue --queue \ > "$case_dir/stdout" 2> "$case_dir/stderr" rc=$? set -e expect_code 1 "$rc" "queue-repeated-token: repeated queue request must refuse" - assert_grep 'repeat the queue request (--queue --no-method); pass exactly one queue token' \ + assert_grep 'repeat the queue request (--queue --queue); pass exactly one queue token' \ "$case_dir/stderr" "queue-repeated-token: refusal did not name both repeated tokens" assert_no_grep 'pr=https://github.com/example/repo/pull/34' "$case_dir/state/task-x1.meta" \ "queue-repeated-token: repeated request was recorded before refusal" @@ -1895,7 +1885,7 @@ test_forge_decides_unreadable_state_reports_without_failing() { : > "$case_dir/gh.log" set +e - run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/32 -- --no-method \ + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/32 -- --queue \ > "$case_dir/stdout" 2> "$case_dir/stderr" rc=$? set -e @@ -1920,27 +1910,24 @@ test_forge_decides_unreadable_state_reports_without_failing() { # GitLab already applies the project's own merge method, so they are refused by # name before anything is recorded rather than forwarded to glab afterwards. test_gitlab_forge_decides_method_refuses_before_recording() { - local case_dir rc spelling - for spelling in 'no-method|--no-method' 'method-equals-queue|--method=queue' 'method-queue|--method queue'; do - case_dir=$(make_gitlab_case "gitlab-forge-decides-${spelling%%|*}") + local case_dir rc + case_dir=$(make_gitlab_case "gitlab-forge-decides-canonical") - set +e - # shellcheck disable=SC2086 # The spelling is one or two extra merge flags. - run_pr_merge "$case_dir" task-x1 "$MR_URL" -- ${spelling#*|} \ - > "$case_dir/stdout" 2> "$case_dir/stderr" - rc=$? - set -e + set +e + run_pr_merge "$case_dir" task-x1 "$MR_URL" -- --queue \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e - expect_code 1 "$rc" "gitlab-forge-decides-${spelling%%|*}: fm-pr-merge should refuse the forge-decides token" - assert_grep "extra merge arguments must not request GitHub's merge queue on GitLab" \ - "$case_dir/stderr" "gitlab-forge-decides-${spelling%%|*}: refusal did not name the queue token" - assert_no_grep "pr=$MR_URL" "$case_dir/state/task-x1.meta" \ - "gitlab-forge-decides-${spelling%%|*}: the URL was recorded before rejecting the token" - assert_absent "$case_dir/state/task-x1.check.sh" \ - "gitlab-forge-decides-${spelling%%|*}: the refused token armed a merge poll" - [ ! -s "$case_dir/glab.log" ] \ - || fail "gitlab-forge-decides-${spelling%%|*}: glab was invoked with a flag it does not define" - done + expect_code 1 "$rc" "gitlab-forge-decides-canonical: fm-pr-merge should refuse the forge-decides token" + assert_grep "extra merge arguments must not request GitHub's merge queue on GitLab" \ + "$case_dir/stderr" "gitlab-forge-decides-canonical: refusal did not name the queue token" + assert_no_grep "pr=$MR_URL" "$case_dir/state/task-x1.meta" \ + "gitlab-forge-decides-canonical: the URL was recorded before rejecting the token" + assert_absent "$case_dir/state/task-x1.check.sh" \ + "gitlab-forge-decides-canonical: the refused token armed a merge poll" + [ ! -s "$case_dir/glab.log" ] \ + || fail "gitlab-forge-decides-canonical: glab was invoked with a flag it does not define" pass "fm-pr-merge refuses forge-decides merge methods on GitLab before recording state" } diff --git a/tests/fm-teardown.test.sh b/tests/fm-teardown.test.sh index 00a0a902f4a..45a5a6e24e0 100755 --- a/tests/fm-teardown.test.sh +++ b/tests/fm-teardown.test.sh @@ -375,35 +375,6 @@ assert_refusal_retained_task_state() { } # Override GitHub lookups to report PR 7 as still open with the supplied head. -# A merge-queue enqueue leaves the pull request OPEN until it actually lands. -add_gh_pr_open_for_head() { - local case_dir=$1 head=$2 - cat > "$case_dir/fakebin/gh-axi" <<'SH' -#!/usr/bin/env bash -case "${1:-} ${2:-}" in - "pr list") - printf '%s\n' "count: 1 (showing first 1)" "pull_requests[1]{number,state}:" " 7,open" ; exit 0 ;; - "pr view") - printf '%s\n' "pull_request:" " number: 7" " state: open" ; exit 0 ;; -esac -exit 0 -SH - cat > "$case_dir/fakebin/gh" <&2 -exit 1 -SH - chmod +x "$case_dir/fakebin/gh-axi" "$case_dir/fakebin/gh" -} - append_pr_meta_for_current_head() { local case_dir=$1 head head=$(git -C "$case_dir/wt" rev-parse HEAD) From 84d9c5baa5ae746f0b49ac4dfacda0fb2f12da50 Mon Sep 17 00:00:00 2001 From: Alex William Date: Tue, 8 Sep 2026 19:22:46 +0200 Subject: [PATCH 18/18] no-mistakes(document): Correct queue syntax documentation and stale comments --- bin/fm-pr-merge.sh | 9 ++++----- docs/architecture.md | 5 ++--- docs/verification/github-merge-queue.md | 4 +--- tests/fm-pr-merge.test.sh | 9 +++------ tests/fm-teardown.test.sh | 1 - 5 files changed, 10 insertions(+), 18 deletions(-) diff --git a/bin/fm-pr-merge.sh b/bin/fm-pr-merge.sh index 365cfe807cc..62fba2b44f8 100755 --- a/bin/fm-pr-merge.sh +++ b/bin/fm-pr-merge.sh @@ -7,9 +7,9 @@ # host and path, so any instance works and no host is hardcoded. # # GitHub direct merges default to --squash when the caller names no method. -# `--queue` is the canonical merge-queue request and invokes GitHub's supported +# `--queue` is the only merge-queue request syntax and invokes GitHub's supported # GraphQL enqueuePullRequest mutation rather than the gh-axi merge parser. -# One parser owns every queue spelling and all caller-argument interpretation. +# One parser owns queue syntax and all caller-argument interpretation. # It refuses repeated queue tokens, a queue token mixed with an explicit merge # strategy, and any extra argument the enqueue mutation cannot honour. # Before enqueue, one live GraphQL read proves the URL-derived repository is @@ -725,9 +725,8 @@ github_queue_retry_command() { printf '%s %s %s -- --queue' "$0" "$ID" "$URL" } -# A caller who supplied any accepted queue spelling already requested the one -# operation the retry would perform. Never hand that same operation back under -# either the canonical spelling or a legacy alias. +# A caller who supplied --queue already requested the operation the retry +# would perform. Report the blocking cause instead of repeating that request. github_caller_already_ran_queue_retry() { [ "$FM_PR_CALLER_QUEUE" = true ] } diff --git a/docs/architecture.md b/docs/architecture.md index cbb15f4f1f6..32d943205c5 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -309,7 +309,7 @@ PR-based task merges go through `bin/fm-pr-merge.sh`, which records `pr=` and an The helper requires a full canonical URL and rejects malformed URLs or repo override flags before recording merge state. When task metadata carries `yolo=on`, `bin/fm-pr-check.sh` derives the GitHub repository from that canonical URL and requires live push permission before accepting the PR as ready, so a read-only upstream cannot look like an autonomously landable delivery; `yolo=off` contribution workflows remain unchanged. A normal `https://github.com///pull/` URL invokes `gh-axi pr merge --repo /` with the merge-method rules owned by that helper's header. -The canonical `--queue` argument and its retained legacy aliases instead invoke GitHub's GraphQL `enqueuePullRequest` operation, after live repository permission, PR identity, head, red-check, and branch-rule checks. +Queue requests instead use GitHub's GraphQL `enqueuePullRequest` operation under the argument and preflight contract in `bin/fm-pr-merge.sh`'s header. The mutation is bound to the verified head through `expectedHeadOid`, and its returned queue entry is followed by an independent live queue-membership read. A GitHub merge-queue enqueue leaves the pull request open until it lands, so landing stays confirmed by the merge poll and teardown, which wait for a merged state, with empirical queue-state evidence in [`docs/verification/github-merge-queue.md`](verification/github-merge-queue.md). A `https:////-/merge_requests/` URL (see [docs/gitlab-merge-watch.md](gitlab-merge-watch.md)) invokes `glab mr merge -R https:///`, so the instance comes from the URL, and adds no merge-method flag because the project's own merge method applies. @@ -317,8 +317,7 @@ That path merges only after one live read of the merge request confirms it is op After either forge command returns, the script confirms the PR or MR actually landed, and only a confirmed landing records a landed outcome; a queued or unconfirmed request records none and leaves its poll armed. On GitLab an auto-merge-queued or unconfirmed request is reported without failing the run. On GitHub an outcome that is neither merged nor queued is refused loudly and non-zero, naming the observed state, and a base branch that requires the merge queue is refused with the concrete `--queue` retry rather than a merge strategy the queue rejects. -One parser owns the canonical queue token, retained legacy aliases, caller method and auto-merge interpretation, repeated-token refusal, and unsupported enqueue arguments. -When the caller already supplied any accepted queue token, the refusal says no different retry exists, names the blocking cause reported above it, and points at the queue state to re-check instead of echoing the operation under another spelling. +The helper's parser owns caller-argument interpretation and retry behavior; its header documents the supported queue syntax. An auto-merge request is held to the same standard: `--auto` that leaves the pull request neither merged nor queued is refused rather than reported as success. Every GitHub refusal states what it could not observe as plainly as what it did, so an unreadable branch-rule response, an unrecognised queue method, and a merge queue no available read can see are each named rather than left to look like a base branch with no queue at all. A confirmed merge leaves a durable role-routed outcome instead of living only in the merging agent's memory, and [`bin/fm-merge-outcome-lib.sh`](../bin/fm-merge-outcome-lib.sh)'s header owns its destination, shape, identity, normal-case deduplication, and at-least-once recovery. diff --git a/docs/verification/github-merge-queue.md b/docs/verification/github-merge-queue.md index efc8d2585cd..da15b6aa2ff 100644 --- a/docs/verification/github-merge-queue.md +++ b/docs/verification/github-merge-queue.md @@ -1,7 +1,5 @@ # GitHub merge-queue verification -Audience: maintainer verification. - This record supports the current guarantee that `bin/fm-pr-merge.sh` uses GitHub's GraphQL `enqueuePullRequest` operation for a queue request and never treats queue membership as a landing. The helper's header owns queue-token grammar, caller-argument interpretation, preconditions, and retry behavior. `bin/fm-pr-poll.sh` and `bin/fm-teardown.sh` continue to accept only a merged pull request as landed. @@ -96,7 +94,7 @@ bin/fm-test-run.sh tests/fm-pr-check-security.test.sh bin/fm-test-run.sh tests/fm-teardown.test.sh ``` -The merge tests execute the public wrapper and prove that the canonical `--queue` token and all retained aliases call `enqueuePullRequest`, bind `expectedHeadOid`, require an effective queue rule, reject a red status rollup and read-only repository, and independently confirm queue membership. +The merge tests execute the public wrapper and prove that `--queue` calls `enqueuePullRequest`, binds `expectedHeadOid`, requires an effective queue rule, rejects a red status rollup and read-only repository, and independently confirms queue membership. They also prove that direct-merge refusals emit a runnable canonical retry, never repeat a queue operation the caller already requested, reject repeated queue tokens, and reject arguments the mutation cannot honor. The PR-check tests prove that autonomous delivery accepts a writable URL-derived target and refuses read-only or unverifiable targets before recording readiness. The teardown tests prove that an open pull request, including one waiting in a merge queue, is not landed work. diff --git a/tests/fm-pr-merge.test.sh b/tests/fm-pr-merge.test.sh index b8f1bc97834..2b4ea6175e7 100755 --- a/tests/fm-pr-merge.test.sh +++ b/tests/fm-pr-merge.test.sh @@ -13,8 +13,7 @@ # (e) PR URL is parsed to number + --repo for gh-axi (defaults to --squash) # (f) malformed PR URL fails fast without calling gh-axi # (g) explicit merge method is not overridden by the default --squash -# (g2) --queue skips default --squash -# and forward no strategy flag, so a merge-queue branch can choose +# (g2) --queue invokes enqueuePullRequest without an explicit strategy # (g3) that path uses the same live outcome read as every other GitHub merge, # so an enqueued still-open PR is named queued rather than merged # (g4) those forge-decides tokens are refused on GitLab before any state is @@ -1716,10 +1715,8 @@ test_forge_decides_conflicting_strategy_refuses_before_forge() { pass "fm-pr-merge refuses a forge-decides method combined with an explicit GitHub strategy" } -# On a merge-queue branch the merge call enqueues the pull request and leaves it -# open, while the forge CLI still reports that as a merge. The same live outcome -# read used for every GitHub merge must name which of the two actually happened, -# and the forge-decides tokens must still omit a strategy. +# Enqueueing leaves the pull request open rather than landed. The shared live +# outcome read must distinguish queue membership from a confirmed merge. test_forge_decides_reports_queued_and_merged_outcomes() { local case_dir rc case_dir=$(make_case forge-decides-queued) diff --git a/tests/fm-teardown.test.sh b/tests/fm-teardown.test.sh index 45a5a6e24e0..d502e3205b8 100755 --- a/tests/fm-teardown.test.sh +++ b/tests/fm-teardown.test.sh @@ -374,7 +374,6 @@ assert_refusal_retained_task_state() { || fail "$label: refusal erased the durable task record" } -# Override GitHub lookups to report PR 7 as still open with the supplied head. append_pr_meta_for_current_head() { local case_dir=$1 head head=$(git -C "$case_dir/wt" rev-parse HEAD)