diff --git a/AGENTS.md b/AGENTS.md index 7ea57f1be5..0c9ad529ac 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -321,6 +321,7 @@ Before deciding any ask-user finding, load `ask-user-authority`; the implementat Never merge a red PR. Without a current explicit captain instruction that states the concrete merge, that default stands, and standing `yolo` cannot authorize a red merge; section 1 owns when such an instruction overrides a Firstmate-written standing rule within its exact scope. Use `bin/fm-pr-merge.sh` for every task PR merge so merge metadata is recorded, and use `bin/fm-merge-local.sh` for approved local-only landing; never call a lower-level merge command around their guards. +`bin/fm-pr-merge.sh` reads the forge's own check verdict, including the merge queue's separate verdict on the combined commit, and refuses the merge when it is failing or unreadable; its `--allow-failing-checks` override is a red merge, so it needs the same current explicit captain instruction that any red merge needs. After an autonomous merge, give the captain a one-line full-URL or local-main outcome. ### Validate diff --git a/bin/fm-pr-checks-lib.sh b/bin/fm-pr-checks-lib.sh new file mode 100644 index 0000000000..f5f600233f --- /dev/null +++ b/bin/fm-pr-checks-lib.sh @@ -0,0 +1,276 @@ +#!/usr/bin/env bash +# fm-pr-checks-lib.sh - read the forge's own verdict on a GitHub pull request's +# checks, so bin/fm-pr-merge.sh can refuse a red merge instead of trusting +# silence. Sourced, never executed. +# +# Two independent sources are read, because either can be red on its own: +# +# 1. the pull request head's status check rollup, and +# 2. the newest merge-queue attempt for that pull request. A merge queue runs +# the checks AGAIN on a combined commit published as a temporary +# "gh-readonly-queue//pr--" branch, so its verdict +# is a different fact from the branch's own and can be red while every +# branch check is green. +# +# Source 2 is not hypothetical. On 2026-08-10 the queue run for nguzen/aln pull +# request 182 failed at 17:54 UTC, the pull request was merged at 17:59 UTC, and +# the defect in that combined commit then blocked every production deploy for +# two days. A branch-only read would not have refused that merge. +# +# `gh pr checks` is deliberately unused: it exits non-zero both when a check has +# failed and when one is still running, so its exit status cannot classify a +# result (verified 2026-08-12) and only its human-facing lines carry the answer. +# The JSON reads below are classified by value instead. +# +# Every read failure - a missing tool, a failed call, a bound hit, output that +# does not parse, or a payload for a different pull request - is reported as +# "unreadable", never as a pass. Treating the absence of a red signal as green +# is the exact failure that produced the incident above, so the caller must +# refuse on "unreadable" as it refuses on "failing". +# +# The caller must have validated owner, repository and number through +# bin/fm-pr-lib.sh before calling in, because those values are passed to `gh`. +# +# GitHub only, deliberately. bin/fm-pr-lib.sh also parses GitLab merge requests +# for the watcher, but bin/fm-pr-merge.sh refuses a GitLab URL before it ever +# reaches this library, so a forge whose check state cannot be read here is +# refused by name at the merge entrypoint rather than mis-classified in here. +# +# fm_pr_checks_read +# Sets FM_PR_CHECKS_STATE to one of: +# failing at least one check the forge reports as failed +# pending nothing failed, but something has not finished +# green every reported check succeeded (or was neutral/skipped) +# none the forge reports no checks at all for this pull request +# unreadable the state could not be established (see _REASON) +# FM_PR_CHECKS_FAILING and FM_PR_CHECKS_PENDING carry one human-readable +# line per check, FM_PR_CHECKS_REASON explains an unreadable state, and +# FM_PR_CHECKS_QUEUE_REF names the merge-queue attempt that was read. +# Always returns 0: the classification is the result, not the exit status. + +set -u + +# Every FM_PR_CHECKS_* value below is this library's OUTPUT: it is read by the +# sourcing caller (bin/fm-pr-merge.sh), never inside this file. +# shellcheck disable=SC2034 +FM_PR_CHECKS_STATE= +FM_PR_CHECKS_FAILING= +FM_PR_CHECKS_PENDING= +FM_PR_CHECKS_REASON= +FM_PR_CHECKS_QUEUE_REF= + +FM_PR_CHECKS_LIB_DIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) +# shellcheck source=bin/fm-timeout-lib.sh +. "$FM_PR_CHECKS_LIB_DIR/fm-timeout-lib.sh" + +# Bound every forge read so a hung CLI cannot stall a merge indefinitely. A +# non-positive bound is not a bound (see bin/fm-timeout-lib.sh), so reject it. +fm_pr_checks_timeout() { + local seconds=${FM_PR_CHECKS_TIMEOUT:-45} + case "$seconds" in + ''|*[!0-9]*) seconds=45 ;; + esac + [ "$seconds" -gt 0 ] || seconds=45 + printf '%s\n' "$seconds" +} + +fm_pr_checks_gh() { + fm_run_timed "$(fm_pr_checks_timeout)" \ + env GH_PROMPT_DISABLED=1 GH_NO_UPDATE_NOTIFIER=1 gh "$@" +} + +# A CheckRun carries status+conclusion, a StatusContext carries only state, and +# both shapes appear side by side in one rollup. CANCELLED and STALE on the +# branch itself are treated as failures rather than noise: the branch is not +# green and nothing else re-runs them. +# The jq program is deliberately literal: $want is a jq argument, not a shell one. +# shellcheck disable=SC2016 +FM_PR_CHECKS_ROLLUP_JQ=' +def norm: (. // "") | tostring | ascii_upcase; +def verdict: + (.conclusion | norm) as $c + | ((.status // .state) | norm) as $s + | if ($c == "FAILURE" or $c == "ERROR" or $c == "TIMED_OUT" or $c == "CANCELLED" + or $c == "ACTION_REQUIRED" or $c == "STARTUP_FAILURE" or $c == "STALE" + or $s == "FAILURE" or $s == "ERROR") then "fail" + elif ($c == "SUCCESS" or $c == "NEUTRAL" or $c == "SKIPPED" or $s == "SUCCESS") then "ok" + else "pending" end; +def check_name: (.name // .context // "check") | tostring; +def check_detail: (.conclusion // .status // .state // "unknown") | tostring; +if (has("number") | not) or (has("baseRefName") | not) or (has("statusCheckRollup") | not) then "invalid" +elif ((.number | tostring) != ($want | tostring)) then "identity" +else + ("base\t" + ((.baseRefName // "") | tostring)), + ("total\t" + ((.statusCheckRollup // []) | length | tostring)), + ((.statusCheckRollup // [])[] | (verdict + "\t" + check_name + "\t" + check_detail)) +end +' + +# Merge-queue runs are Actions runs on the queue branch. Only the NEWEST attempt +# for this pull request is judged, so a superseded red attempt cannot keep +# refusing a pull request that has since been re-queued green; within that +# attempt the newest run per workflow wins, so a re-run replaces its own result. +# cancelled/stale queue attempts are inconclusive rather than failures: the +# queue discards and re-runs them as PRs ahead of this one land. +# $prefix is a jq argument, not a shell one. +# shellcheck disable=SC2016 +FM_PR_CHECKS_QUEUE_JQ=' +def norm: (. // "") | tostring | ascii_upcase; +def verdict: + (.conclusion | norm) as $c + | (.status | norm) as $s + | if ($c == "FAILURE" or $c == "TIMED_OUT" or $c == "ACTION_REQUIRED" + or $c == "STARTUP_FAILURE") then "fail" + elif ($c == "SUCCESS" or $c == "NEUTRAL" or $c == "SKIPPED") then "ok" + else "pending" end; +if (has("workflow_runs") | not) then "invalid" else +[ (.workflow_runs // [])[] + | select(((.head_branch // "") | tostring) | startswith($prefix)) ] +| (sort_by((.created_at // "") | tostring) | reverse) as $runs +| if ($runs | length) == 0 then "queue_none" + else (($runs[0].head_branch) | tostring) as $ref + | ("queue_ref\t" + $ref), + ( [ $runs[] | select((((.head_branch // "") | tostring)) == $ref) ] + | group_by((.name // "") | tostring) + | map(sort_by([((.created_at // "") | tostring), ((.run_attempt // 0) | tonumber? // 0)]) | last) + | .[] + | (verdict + "\t" + ((.name // "run") | tostring) + "\t" + + ((.conclusion // .status // "unknown") | tostring)) ) + end +end +' + +fm_pr_checks_unreadable() { + FM_PR_CHECKS_STATE=unreadable + FM_PR_CHECKS_REASON=$1 + return 0 +} + +fm_pr_checks_read() { + local owner=$1 repo=$2 number=$3 + local head_json runs_json parsed prefix + local kind field_a field_b base='' total=0 queue_seen=0 fail_count=0 pend_count=0 + + # Reset the caller-visible outputs, which this file only ever writes. + # shellcheck disable=SC2034 + FM_PR_CHECKS_STATE= + FM_PR_CHECKS_FAILING= + FM_PR_CHECKS_PENDING= + # shellcheck disable=SC2034 + FM_PR_CHECKS_REASON= + # shellcheck disable=SC2034 + FM_PR_CHECKS_QUEUE_REF= + + command -v gh >/dev/null 2>&1 \ + || { fm_pr_checks_unreadable 'gh not found, so the forge check state cannot be read'; return 0; } + command -v jq >/dev/null 2>&1 \ + || { fm_pr_checks_unreadable 'jq not found, so the forge check state cannot be classified'; return 0; } + + head_json=$(fm_pr_checks_gh pr view "$number" --repo "$owner/$repo" \ + --json number,baseRefName,statusCheckRollup 2>/dev/null) \ + || { fm_pr_checks_unreadable "gh pr view failed for $owner/$repo#$number"; return 0; } + [ -n "$head_json" ] \ + || { fm_pr_checks_unreadable "gh pr view returned no data for $owner/$repo#$number"; return 0; } + + parsed=$(printf '%s' "$head_json" \ + | jq -r --arg want "$number" "$FM_PR_CHECKS_ROLLUP_JQ" 2>/dev/null) \ + || { fm_pr_checks_unreadable "the pull request check payload for $owner/$repo#$number did not parse"; return 0; } + case $parsed in + identity|identity$'\n'*) + fm_pr_checks_unreadable "the check payload did not describe $owner/$repo#$number" + return 0 + ;; + invalid|invalid$'\n'*) + fm_pr_checks_unreadable "the check payload for $owner/$repo#$number was missing the fields that were asked for" + return 0 + ;; + esac + + while IFS=$'\t' read -r kind field_a field_b; do + case $kind in + base) base=$field_a ;; + total) + case $field_a in + ''|*[!0-9]*) + fm_pr_checks_unreadable "the check count of $owner/$repo#$number was not a number" + return 0 + ;; + esac + total=$field_a + ;; + fail) + fail_count=$((fail_count + 1)) + FM_PR_CHECKS_FAILING="${FM_PR_CHECKS_FAILING}branch check: $field_a ($field_b)"$'\n' + ;; + pending) + pend_count=$((pend_count + 1)) + FM_PR_CHECKS_PENDING="${FM_PR_CHECKS_PENDING}branch check: $field_a ($field_b)"$'\n' + ;; + ok|'') ;; + *) + fm_pr_checks_unreadable "the pull request check payload for $owner/$repo#$number was not understood" + return 0 + ;; + esac + done </dev/null) \ + || { fm_pr_checks_unreadable "the merge-queue check runs of $owner/$repo could not be listed"; return 0; } + [ -n "$runs_json" ] \ + || { fm_pr_checks_unreadable "the merge-queue check runs of $owner/$repo returned no data"; return 0; } + + parsed=$(printf '%s' "$runs_json" \ + | jq -r --arg prefix "$prefix" "$FM_PR_CHECKS_QUEUE_JQ" 2>/dev/null) \ + || { fm_pr_checks_unreadable "the merge-queue check payload of $owner/$repo did not parse"; return 0; } + + while IFS=$'\t' read -r kind field_a field_b; do + case $kind in + queue_none|'') ;; + invalid) + fm_pr_checks_unreadable "the merge-queue payload of $owner/$repo did not list any workflow runs field" + return 0 + ;; + queue_ref) + # shellcheck disable=SC2034 # Read by the sourcing caller. + FM_PR_CHECKS_QUEUE_REF=$field_a + queue_seen=1 + ;; + fail) + fail_count=$((fail_count + 1)) + FM_PR_CHECKS_FAILING="${FM_PR_CHECKS_FAILING}merge-queue check: $field_a ($field_b)"$'\n' + ;; + pending) + pend_count=$((pend_count + 1)) + FM_PR_CHECKS_PENDING="${FM_PR_CHECKS_PENDING}merge-queue check: $field_a ($field_b)"$'\n' + ;; + ok) ;; + *) + fm_pr_checks_unreadable "the merge-queue check payload of $owner/$repo was not understood" + return 0 + ;; + esac + done < [-- ] +# +# Before merging, the forge's own check verdict is read through +# bin/fm-pr-checks-lib.sh and a failing or unreadable verdict REFUSES the merge. +# That guard exists because "never merge a red PR" previously lived only in +# AGENTS.md: on 2026-08-10 pull request 182 of nguzen/aln was merged five minutes +# after its merge-queue run failed, and the resulting commit blocked production +# deploys for two days. Pass --allow-failing-checks to merge anyway when the +# failure is known to be infrastructural rather than in the change. +# Usage: fm-pr-merge.sh [--allow-failing-checks] +# [-- ] set -eu SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" @@ -17,6 +26,8 @@ STATE="${FM_STATE_OVERRIDE:-$FM_HOME/state}" # shellcheck source=bin/fm-pr-lib.sh . "$SCRIPT_DIR/fm-pr-lib.sh" +# shellcheck source=bin/fm-pr-checks-lib.sh +. "$SCRIPT_DIR/fm-pr-checks-lib.sh" if [ "$#" -lt 2 ]; then echo "error: invalid PR merge request" >&2 @@ -37,6 +48,20 @@ PR_OWNER=$FM_PR_OWNER PR_REPO=$FM_PR_REPO PR_NUMBER=$FM_PR_NUMBER shift 2 + +# Own flags are consumed before the optional -- separator, so they are never +# forwarded to gh-axi. An unknown flag here is a usage error rather than a +# silently ignored intent. +ALLOW_FAILING_CHECKS=0 +while [ "$#" -gt 0 ] && [ "${1:-}" != "--" ]; do + case $1 in + --allow-failing-checks) ALLOW_FAILING_CHECKS=1; shift ;; + *) + echo "error: unknown merge flag $1 (own flags: --allow-failing-checks; pass gh-axi flags after --)" >&2 + exit 2 + ;; + esac +done [ "${1:-}" = "--" ] && shift caller_has_merge_method() { @@ -70,6 +95,33 @@ if [ ! -f "$META" ] || [ -L "$META" ]; then exit 1 fi +# The check gate runs before any state is recorded or any poll is armed, so a +# refused merge leaves nothing behind, exactly like the earlier refusals above. +if [ "$ALLOW_FAILING_CHECKS" -eq 1 ]; then + echo "warning: merging $URL without reading the forge's check verdict (--allow-failing-checks)" >&2 +else + fm_pr_checks_read "$PR_OWNER" "$PR_REPO" "$PR_NUMBER" + case $FM_PR_CHECKS_STATE in + failing) + echo "error: refusing to merge $URL: the forge reports failing checks" >&2 + printf '%s' "$FM_PR_CHECKS_FAILING" | sed 's/^/ /' >&2 + [ -z "$FM_PR_CHECKS_QUEUE_REF" ] \ + || echo " (merge-queue attempt read: $FM_PR_CHECKS_QUEUE_REF)" >&2 + echo "hint: land a green head, or pass --allow-failing-checks when the failure is infrastructural" >&2 + exit 1 + ;; + unreadable) + echo "error: refusing to merge $URL: the forge's check state is unreadable ($FM_PR_CHECKS_REASON)" >&2 + echo "hint: an unreadable verdict is never a pass; fix the read, or pass --allow-failing-checks deliberately" >&2 + exit 1 + ;; + pending) + echo "note: merging $URL with checks still running:" >&2 + printf '%s' "$FM_PR_CHECKS_PENDING" | sed 's/^/ /' >&2 + ;; + esac +fi + "$SCRIPT_DIR/fm-pr-check.sh" "$ID" "$URL" grep -qxF "pr=$URL" "$META" || { echo "error: PR metadata recording failed" >&2 diff --git a/docs/architecture.md b/docs/architecture.md index 1e9114ad81..df3de8e321 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -243,6 +243,9 @@ For target project repos shipped through their own no-mistakes pipeline, commits The firstmate repo itself is the exception: its `.no-mistakes/` directory is local state, stays gitignored, and is rejected by CI if tracked. 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 `gh-axi pr merge`. The helper requires a full `https://github.com///pull/` URL, invokes `gh-axi pr merge --repo /`, defaults to `--squash`, preserves explicit merge-method flags, and rejects malformed URLs or repo override flags before recording merge state; a well-formed GitLab merge request URL (see [docs/gitlab-merge-watch.md](gitlab-merge-watch.md)) is refused too, explicitly, rather than sent to the wrong forge. +Before recording anything it reads the forge's check verdict through [`bin/fm-pr-checks-lib.sh`](../bin/fm-pr-checks-lib.sh) and refuses a failing or unreadable verdict, so "never merge a red PR" is enforced by the one merge path instead of by session attention alone. +That read has two sources because either can be red on its own: the pull request head's status check rollup, and the newest merge-queue attempt, whose checks run again on a combined commit and therefore judge something the branch never did. +An unreadable verdict refuses exactly like a failing one, and `--allow-failing-checks` is the single deliberate override. Teardown is fail-closed for ship worktrees: dirty worktrees refuse, and committed work must be landed before the worktree is returned. [`bin/fm-teardown.sh`](../bin/fm-teardown.sh)'s header owns the landed-work proofs, PR-discovery fallback, and stale-lock recovery procedure. diff --git a/docs/scripts.md b/docs/scripts.md index a1bc29d276..81e23d6f90 100644 --- a/docs/scripts.md +++ b/docs/scripts.md @@ -105,7 +105,8 @@ The shared no-mistakes gate refusal for fleet lifecycle entrypoints is summarize | `fm-pr-poll.sh` | Provide the byte-static watcher program for validated PR/MR-poll sidecars | | `fm-pr-check-migrate.sh` | Quarantine older task polls without execution and rebuild only canonical polls | | `fm-pr-check.sh` | Record validated `pr=` and `pr_head=` values, then atomically arm a static merge poll | -| `fm-pr-merge.sh` | Record PR metadata, then merge a task's canonical full GitHub URL | +| `fm-pr-merge.sh` | Refuse a failing or unreadable forge check verdict, record PR metadata, then merge a task's canonical full GitHub URL | +| `fm-pr-checks-lib.sh` | Classify a GitHub PR's check verdict from its head rollup and its newest merge-queue attempt | | `fm-promote.sh` | Promote a scout task in place to a protected ship task with an explicit delivery mode | | `fm-teardown.sh` | Fail-closed teardown: return landed ship worktrees, require completed scout deliverables, retire secondmate homes | | `fm-harness.sh` | Detect the running harness and resolve crew or secondmate harness, model, and effort | diff --git a/tests/fm-pr-check-security.test.sh b/tests/fm-pr-check-security.test.sh index 03c6ce688e..82e9fb725d 100755 --- a/tests/fm-pr-check-security.test.sh +++ b/tests/fm-pr-check-security.test.sh @@ -76,6 +76,25 @@ SH cat > "$fakebin/gh" <<'SH' #!/usr/bin/env bash printf '%s\n' "$*" >> "$FM_TEST_GH_LOG" +# bin/fm-pr-merge.sh reads the forge's check verdict before merging, through +# bin/fm-pr-checks-lib.sh: the PR head's rollup and the merge-queue Actions runs. +# The defaults here are a green PR with no queue attempt, so this suite keeps +# testing recording and derivation rather than the check gate, which +# tests/fm-pr-merge.test.sh owns. +case " $* " in + *statusCheckRollup*) + [ "${FM_TEST_GH_CHECKS_FAIL:-0}" = 0 ] || exit 1 + printf '{"number":%s,"baseRefName":"main","statusCheckRollup":%s}\n' \ + "$3" "${FM_TEST_GH_ROLLUP:-[]}" + exit 0 + ;; +esac +case "${1:-}" in + api) + printf '{"workflow_runs":%s}\n' "${FM_TEST_GH_QUEUE_RUNS:-[]}" + exit 0 + ;; +esac case " $* " in *" headRefOid "*) printf '%s\n' "${FM_TEST_GH_HEAD:-0123456789abcdef0123456789abcdef01234567}" ;; *" state "*) diff --git a/tests/fm-pr-merge.test.sh b/tests/fm-pr-merge.test.sh index a064b6919b..015d5ac10b 100755 --- a/tests/fm-pr-merge.test.sh +++ b/tests/fm-pr-merge.test.sh @@ -14,6 +14,17 @@ # (f) malformed PR URL fails fast without calling gh-axi # (g) explicit merge method is not overridden by the default --squash # (h) repo override args fail fast because the repo comes from the URL +# (i) a failing check on the PR head refuses before any state is recorded +# (j) a failing MERGE-QUEUE run refuses even when every branch check is green +# (k) a superseded red queue attempt does not block a re-queued green one +# (l) an unreadable check state refuses - forge silence is never a pass +# (m) --allow-failing-checks merges a red PR deliberately, and says so +# (n) an unknown own flag is a usage error, not a flag forwarded to gh-axi +# +# Case (j) is the 2026-08-10 incident in nguzen/aln: the merge-queue run for pull +# request 182 failed and the merge landed five minutes later, and the resulting +# commit blocked production deploys for two days. The queue runs the checks again +# on a combined commit, so its verdict is a source of its own. set -u # shellcheck source=tests/lib.sh @@ -42,27 +53,58 @@ make_case() { printf '%s\n' "$case_dir" } -# 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 -add_gh_mocks() { +# gh mock reproducing every read bin/fm-pr-merge.sh makes, so the real jq +# classification in bin/fm-pr-checks-lib.sh runs against real forge JSON shapes +# instead of being mocked away: +# * pr view --json headRefOid fm-pr-check.sh's pr_head lookup +# * pr view --json ...statusCheckRollup the PR head's own checks +# * api repos///actions/runs?event=merge_group... the queue attempts +# Fixtures come from the environment, and the defaults are the "repo with no PR +# CI" shape every pre-existing case here relies on: an empty rollup and no queue +# attempt, which is a definitive "no checks" rather than an unreadable state. +# FM_TEST_ROLLUP statusCheckRollup array JSON +# FM_TEST_QUEUE_RUNS workflow_runs array JSON +# FM_TEST_PR_NUMBER number the payload claims to describe (identity check) +# FM_TEST_CHECKS_FAIL 1 = the rollup read fails, 2 = the queue read fails +write_gh_mock() { 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" -exit 0 -SH cat > "$case_dir/fakebin/gh" <> "\${FM_TEST_GH_LOG:-/dev/null}" +case " \$* " in + *headRefOid*) printf '%s\n' '$head' ; exit 0 ;; +esac case "\${1:-} \${2:-}" in "pr view") - case " \$* " in - *headRefOid*) printf '%s\n' '$head' ; exit 0 ;; - esac + [ "\${FM_TEST_CHECKS_FAIL:-0}" = 1 ] && exit 1 + printf '{"number":%s,"baseRefName":"main","statusCheckRollup":%s}\n' \\ + "\${FM_TEST_PR_NUMBER:-\$3}" "\${FM_TEST_ROLLUP:-[]}" + exit 0 ;; esac +case "\${1:-}" in + api) + [ "\${FM_TEST_CHECKS_FAIL:-0}" = 2 ] && exit 1 + printf '{"workflow_runs":%s}\n' "\${FM_TEST_QUEUE_RUNS:-[]}" + exit 0 + ;; +esac +exit 0 +SH + chmod +x "$case_dir/fakebin/gh" +} + +# gh-axi mock recording every invocation to a log file, plus the gh mock above. +# Args: case_dir head_sha +add_gh_mocks() { + 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" exit 0 SH - chmod +x "$case_dir/fakebin/gh-axi" "$case_dir/fakebin/gh" + chmod +x "$case_dir/fakebin/gh-axi" + write_gh_mock "$case_dir" "$head" } # gh-axi mock that fails the merge call but succeeds everything else, so a @@ -77,11 +119,19 @@ case "${1:-} ${2:-}" in esac exit 0 SH - cat > "$case_dir/fakebin/gh" <<'SH' -#!/usr/bin/env bash -exit 0 -SH - chmod +x "$case_dir/fakebin/gh-axi" "$case_dir/fakebin/gh" + chmod +x "$case_dir/fakebin/gh-axi" + write_gh_mock "$case_dir" 1111111111111111111111111111111111111111 +} + +# One rollup entry in the CheckRun shape the forge really returns. +rollup_entry() { # + printf '[{"__typename":"CheckRun","name":"%s","status":"COMPLETED","conclusion":"%s"}]' "$1" "$2" +} + +# One merge-queue Actions run on the temporary queue branch of a PR. +queue_run() { # + printf '{"name":"%s","head_branch":"gh-readonly-queue/main/pr-%s-%s","event":"merge_group","status":"completed","conclusion":"%s","created_at":"%s","run_attempt":1}' \ + "$3" "$1" "$2" "$4" "$5" } run_pr_merge() { @@ -89,6 +139,7 @@ run_pr_merge() { FM_ROOT_OVERRIDE="$ROOT" \ FM_STATE_OVERRIDE="$case_dir/state" \ FM_TEST_GH_AXI_LOG="$case_dir/gh-axi.log" \ + FM_TEST_GH_LOG="$case_dir/gh.log" \ PATH="$case_dir/fakebin:$PATH" \ "$PR_MERGE" "$@" rc=$? @@ -301,6 +352,174 @@ test_parses_pr_url_for_gh_axi() { pass "fm-pr-merge parses a GitHub PR URL into gh-axi number and --repo arguments" } +test_failing_branch_check_refuses_before_recording() { + local case_dir rc + case_dir=$(make_case failing-branch-check) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" aaaa111111111111111111111111111111111111 + : > "$case_dir/gh-axi.log" + + set +e + FM_TEST_ROLLUP=$(rollup_entry 'e2e · visual QA' FAILURE) \ + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/40 \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "failing-branch-check: fm-pr-merge should refuse a red PR" + assert_grep 'the forge reports failing checks' "$case_dir/stderr" \ + "failing-branch-check: refusal did not name the failing check state" + assert_grep 'e2e · visual QA (FAILURE)' "$case_dir/stderr" \ + "failing-branch-check: refusal did not list which check is red" + assert_no_grep 'pr merge' "$case_dir/gh-axi.log" \ + "failing-branch-check: gh-axi pr merge was invoked for a red PR" + assert_no_grep 'pr=https://github.com/example/repo/pull/40' "$case_dir/state/task-x1.meta" \ + "failing-branch-check: a refused merge recorded PR metadata" + assert_absent "$case_dir/state/task-x1.check.sh" \ + "failing-branch-check: a refused merge armed a merge poll" + pass "fm-pr-merge refuses to merge when the PR head has a failing check" +} + +test_failing_merge_queue_run_refuses_green_branch() { + local case_dir rc runs + case_dir=$(make_case failing-queue-run) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" bbbb222222222222222222222222222222222222 + : > "$case_dir/gh-axi.log" + # The 2026-08-10 shape: every branch check green, the queue's re-run of the + # same checks on the combined commit red. + runs="[$(queue_run 182 94c950e5 CI failure 2026-08-10T17:54:29Z)]" + + set +e + FM_TEST_ROLLUP=$(rollup_entry 'typecheck · test · build' SUCCESS) \ + FM_TEST_QUEUE_RUNS="$runs" \ + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/182 \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "failing-queue-run: fm-pr-merge should refuse a red merge-queue verdict" + assert_grep 'merge-queue check: CI (failure)' "$case_dir/stderr" \ + "failing-queue-run: refusal did not name the red merge-queue run" + assert_grep 'gh-readonly-queue/main/pr-182-94c950e5' "$case_dir/stderr" \ + "failing-queue-run: refusal did not name the queue attempt it read" + assert_no_grep 'pr merge' "$case_dir/gh-axi.log" \ + "failing-queue-run: gh-axi pr merge was invoked despite the red queue run" + assert_absent "$case_dir/state/task-x1.check.sh" \ + "failing-queue-run: a refused merge armed a merge poll" + pass "fm-pr-merge refuses a failing merge-queue run even when the branch itself is green" +} + +test_superseded_red_queue_attempt_does_not_block() { + local case_dir runs + case_dir=$(make_case superseded-queue-attempt) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" cccc333333333333333333333333333333333333 + : > "$case_dir/gh-axi.log" + # An older red attempt plus a newer green one for the same PR: only the newest + # attempt is the forge's current verdict, so this must merge. + runs="[$(queue_run 182 94c950e5 CI failure 2026-08-10T17:54:29Z)," + runs="$runs$(queue_run 182 ffff0001 CI success 2026-08-11T09:00:00Z)]" + + FM_TEST_ROLLUP=$(rollup_entry CI SUCCESS) FM_TEST_QUEUE_RUNS="$runs" \ + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/182 \ + > "$case_dir/stdout" 2> "$case_dir/stderr" \ + || fail "superseded-queue-attempt: fm-pr-merge refused a re-queued green PR: $(cat "$case_dir/stderr")" + + grep -qxF 'pr merge 182 --repo example/repo --squash' "$case_dir/gh-axi.log" \ + || fail "superseded-queue-attempt: a green PR was not merged" + pass "fm-pr-merge judges only the newest merge-queue attempt, so a superseded red one does not block" +} + +test_unreadable_check_state_refuses() { + local case_dir rc which + for which in 1 2; do + case_dir=$(make_case "unreadable-check-state-$which") + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" dddd444444444444444444444444444444444444 + : > "$case_dir/gh-axi.log" + + set +e + FM_TEST_CHECKS_FAIL="$which" \ + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/44 \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "unreadable-check-state-$which: fm-pr-merge should refuse an unreadable verdict" + assert_grep 'check state is unreadable' "$case_dir/stderr" \ + "unreadable-check-state-$which: refusal did not say the state was unreadable" + assert_no_grep 'pr merge' "$case_dir/gh-axi.log" \ + "unreadable-check-state-$which: gh-axi pr merge was invoked on an unreadable verdict" + assert_absent "$case_dir/state/task-x1.check.sh" \ + "unreadable-check-state-$which: a refused merge armed a merge poll" + done + pass "fm-pr-merge refuses when the forge check state cannot be read (silence is not green)" +} + +test_wrong_pull_request_payload_refuses() { + local case_dir rc + case_dir=$(make_case wrong-pr-payload) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" eeee555555555555555555555555555555555555 + : > "$case_dir/gh-axi.log" + + set +e + FM_TEST_PR_NUMBER=999 FM_TEST_ROLLUP=$(rollup_entry CI SUCCESS) \ + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/45 \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "wrong-pr-payload: a green verdict for another PR must not authorize this merge" + assert_grep 'unreadable' "$case_dir/stderr" \ + "wrong-pr-payload: refusal did not treat a mismatched payload as unreadable" + assert_no_grep 'pr merge' "$case_dir/gh-axi.log" \ + "wrong-pr-payload: gh-axi pr merge was invoked on a payload for another PR" + pass "fm-pr-merge refuses a check payload that describes a different pull request" +} + +test_allow_failing_checks_overrides_red() { + local case_dir + case_dir=$(make_case allow-failing-checks) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" ffff666666666666666666666666666666666666 + : > "$case_dir/gh-axi.log" + + FM_TEST_ROLLUP=$(rollup_entry CI FAILURE) \ + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/46 --allow-failing-checks \ + > "$case_dir/stdout" 2> "$case_dir/stderr" \ + || fail "allow-failing-checks: the override did not merge: $(cat "$case_dir/stderr")" + + grep -qxF 'pr merge 46 --repo example/repo --squash' "$case_dir/gh-axi.log" \ + || fail "allow-failing-checks: the override did not reach gh-axi pr merge" + assert_grep 'without reading the forge' "$case_dir/stderr" \ + "allow-failing-checks: the override merged silently instead of saying so" + assert_no_grep 'allow-failing-checks' "$case_dir/gh-axi.log" \ + "allow-failing-checks: firstmate's own flag was forwarded to gh-axi" + pass "fm-pr-merge merges a red PR only with --allow-failing-checks, and reports that it did" +} + +test_unknown_own_flag_is_usage_error() { + local case_dir rc + case_dir=$(make_case unknown-own-flag) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" 1212121212121212121212121212121212121212 + : > "$case_dir/gh-axi.log" + + set +e + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/47 --force \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 2 "$rc" "unknown-own-flag: an unknown own flag should be a usage error" + assert_grep 'unknown merge flag --force' "$case_dir/stderr" \ + "unknown-own-flag: refusal did not name the unknown flag" + [ ! -s "$case_dir/gh-axi.log" ] || fail "unknown-own-flag: gh-axi was invoked for a usage error" + pass "fm-pr-merge rejects an unknown own flag instead of forwarding or ignoring it" +} + test_records_pr_and_head_before_merging test_merge_failure_propagates_after_recording test_extra_merge_args_forwarded @@ -311,3 +530,10 @@ test_repo_override_args_refuse_before_recording test_explicit_merge_method_not_overridden test_method_equals_merge_method_not_overridden test_parses_pr_url_for_gh_axi +test_failing_branch_check_refuses_before_recording +test_failing_merge_queue_run_refuses_green_branch +test_superseded_red_queue_attempt_does_not_block +test_unreadable_check_state_refuses +test_wrong_pull_request_payload_refuses +test_allow_failing_checks_overrides_red +test_unknown_own_flag_is_usage_error