From 9ac06bbc7d7e9c8c9c1f94bff97afeee04fa729c Mon Sep 17 00:00:00 2001 From: Shane Bracewell Date: Wed, 5 Aug 2026 23:18:32 -0400 Subject: [PATCH] fix(bin): refuse merges without verified green checks (land of upstream #1614) Ports upstream PR #1614 onto this fork's trunk. Nothing here is redesigned: the guard, its refusal wording, the --allow-unverified override, the merge_verification=/merge_verified_head= metadata keys, and the crew-state mapping changes are upstream's as written. Provenance: upstream PR kunchenguid/firstmate#1614 head commit e3e4b470fd8eff73ce581160dc94b223032eaad3 its base c8edff36b8466ea0fe547d3abf4b8aa330489986 landed onto 3611e49 (sbracewell64/firstmate main) The two trunks diverged at upstream #1495, so the diff did not apply cleanly. Resolutions, all of them fork-versus-upstream divergence rather than changes to what #1614 does: bin/fm-pr-merge.sh - this fork resolves a task's identity through either a live meta or the durable landing record a released task keeps, which upstream has no equivalent of. The record resolution stays, and verification is placed between it and the recording step, so upstream's property still holds exactly: a head the guard refuses leaves no pr= recorded and no merge poll armed. META is bound to whichever record the task actually has, so the verification metadata write addresses it unchanged. tests/fm-pr-merge.test.sh - upstream's file is the base, with this fork's six released-task cases and their fixtures re-added. Their gh mocks now answer the verification read as well as the forge-view read. test_missing_meta_refuses_before_merge exists on both trunks asserting opposite behavior: upstream refuses a task with no record before any forge lookup, while this fork deliberately rebuilds that record from the pull request itself. The fork's version of the case is kept, because that reconstruction is this trunk's behavior. AGENTS.md, docs/architecture.md, docs/scripts.md - upstream's sentences folded into the fork's own text for the landing record, the task base references, and the merge poll's conflict reporting. Verified on this fork: fm-lint.sh clean, fm-doc-audience-check.sh ok, and tests/fm-pr-merge.test.sh (34), tests/fm-crew-state.test.sh (54) and tests/fm-pr-check-security.test.sh (41) all pass. Checked once against the live forge with the merge command mocked: the pre-change path issued a squash merge for a pull request with three failing check runs, and this one refused it, naming the head and the failing count, before arming anything. --- AGENTS.md | 7 +- bin/fm-crew-state.sh | 75 +- bin/fm-pr-merge.sh | 272 ++++++- docs/architecture.md | 6 +- docs/scripts.md | 2 +- tests/fm-crew-state.test.sh | 190 ++++- tests/fm-pr-check-security.test.sh | 6 + tests/fm-pr-merge.test.sh | 1182 ++++++++++++++++++++++------ 8 files changed, 1459 insertions(+), 281 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 6ad76ec24fc..cb21b203b1c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -96,7 +96,7 @@ state/ volatile runtime signals; gitignored .turn-ended touched by turn-end hooks .grok-turnend-token firstmate-owned grok hook registry token for the task; removed by teardown .kimi-turnend-token firstmate-owned Kimi hook registry token for the task; removed by teardown - .meta written by fm-spawn: window=, endpoint_task_id=, worktree=, project=, harness=, model=, effort=, kind=, mode=, yolo=, tasktmp=; a ship or scout also records the task's two base references as slot_base=, contribution_target=, and base_state= (bin/fm-task-base-lib.sh); an optional traceparent= only when trace context is enabled (docs/configuration.md "Trace context propagation"); kind=secondmate also records home= and projects=, plus remote_host=/remote_root=/remote_backend=/remote_herdr_session=/remote_target= for a remote route; a non-default runtime backend records further backend-specific fields (docs/configuration.md "Runtime backend"; bin/fm-backend.sh, section 8); fm-pr-check, including through fm-pr-merge, records one canonical pr= and the forge's pr_head= when available (GitHub pull requests and GitLab merge requests; docs/gitlab-merge-watch.md); fm-x-link appends x_request=, x_request_ts=, x_followups=, and optional x_platform=/x_reply_max_chars= for an X-mode-originated task (section 14) + .meta written by fm-spawn: window=, endpoint_task_id=, worktree=, project=, harness=, model=, effort=, kind=, mode=, yolo=, tasktmp=; a ship or scout also records the task's two base references as slot_base=, contribution_target=, and base_state= (bin/fm-task-base-lib.sh); an optional traceparent= only when trace context is enabled (docs/configuration.md "Trace context propagation"); kind=secondmate also records home= and projects=, plus remote_host=/remote_root=/remote_backend=/remote_herdr_session=/remote_target= for a remote route; a non-default runtime backend records further backend-specific fields (docs/configuration.md "Runtime backend"; bin/fm-backend.sh, section 8); fm-pr-check, including through fm-pr-merge, records one canonical pr= and the forge's pr_head= when available (GitHub pull requests and GitLab merge requests; docs/gitlab-merge-watch.md); fm-pr-merge records merge_verification= plus merge_verified_head= for the head it re-verified, or merge_verification=override for an explicitly unverified merge; fm-x-link appends x_request=, x_request_ts=, x_followups=, and optional x_platform=/x_reply_max_chars= for an X-mode-originated task (section 14) .herdr-presentation quarantinable attempt and restart-binding journal for Herdr's optional visual projection; never task or endpoint authority; see docs/herdr-backend.md "Presentation spaces" .landing private minimal landing record (pr=, forge pr_head=, project=) written by fm-teardown when a ship task is released before its PR lands; stands in for the removed meta so fm-pr-merge can still land that PR and fm-pr-check can rearm its merge watch .check.sh authenticated slow poll; the watcher dispatches validated PR data and the byte-identified X shim through trusted repository scripts, runs registered custom checks from hash-validated private snapshots, and rejects every other state check without execution @@ -315,6 +315,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` re-checks the pull request's current head and refuses a merge it cannot confirm is green, mergeable, and unblocked by review, naming the concrete failing condition and the head whenever GitHub supplies one; treat that refusal as the state to fix, and use its recorded `--allow-unverified` override only on a current explicit captain instruction for that concrete merge. After an autonomous merge, give the captain a one-line full-URL or local-main outcome. ### Validate @@ -337,7 +338,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 current-code-matched 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. +Running, fixing, or CI states remain working; parked approval or fix-review states require the worker to follow the active gate help; passed is done, and checks-passed is done only when the run's own evidence records it and blocked otherwise, because a head no check examined is unverified rather than green; failed or cancelled is failed. 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 other exception is a rebase the pipeline hands back, which the ship brief requires the worker to resolve and commit itself before returning to the gates. The worker reports the PR when CI first becomes green rather than waiting for merge monitoring to finish. @@ -435,7 +436,7 @@ When evidence uses an internal label, rewrite it before sending: - teardown -> cleanup. - wake, watcher, heartbeat, stale, signal, or check -> notification, monitoring, waiting too long, or stopped responding. - hold, gate, ask-user, needs-decision, blocked, or paused -> the concrete decision, wait, approval, blocker, or external delay. -- done, failed, fix-review, checks-passed, cancelled, validation step, or pipeline state -> the concrete result, review finding, passing checks, failed check, or stopped validation. +- done, failed, fix-review, checks-passed, cancelled, validation step, or pipeline state -> the corroborated concrete result, review finding, verified checks, unverified claim, failed check, or stopped validation. - brief -> instructions. - crewmate -> worker, only when naming the helper matters. - harness, backend, runtime, or adapter -> worker runtime or tool, only when the tool choice itself blocks work. diff --git a/bin/fm-crew-state.sh b/bin/fm-crew-state.sh index 2cb290373cb..4617eebb32e 100755 --- a/bin/fm-crew-state.sh +++ b/bin/fm-crew-state.sh @@ -35,6 +35,10 @@ # checks" from "checks green, waiting on merge" (see nm_ci_checks_state) - # a ci-step log-tail check overrides working -> done once checks read # green, so a green PR is never silently read as still-validating. +# A terminal checks-passed is a REPORTED claim and is corroborated against +# the run's own ci log before it is repeated: a claim the evidence does not +# record reports blocked, because a head no check run examined is not work +# ready for review. See nm_ci_checks_state for the measured defect. # 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 @@ -289,6 +293,14 @@ nm_effective_ci_step_status() { # for the MOST RECENT recognized marker (the log is append-only/chronological, # so the last match is current): green with nothing red after it means CI is # green right now, still only waiting on merge/close. +# +# "no CI checks reported" is NOT green, and reading it as green is the defect +# measured on 2026-08-02: a cross-repo fork pull request holds its workflows at +# action_required until a maintainer approves them, so zero checks ever run and +# the pipeline reports that absence as a terminal success. Nothing was red +# because nothing executed. An absent verifier is a distinct state from a +# passing one and never maps to green here, so a head no verifier examined +# reaches firstmate as not-ready rather than as work ready for review. nm_ci_checks_state() { local run_id log_tail marker run_id=$(strip_quotes "$(nm_field id)") @@ -299,11 +311,15 @@ nm_ci_checks_state() { | grep -E 'CI checks passed|no CI checks reported - still monitoring|no CI checks reported yet|checks failed|issues detected|CI checks running|base branch advanced.*re-arming CI monitor timeout' \ | tail -1) case "$marker" in - *"checks passed"*|*"no CI checks reported - still monitoring"*) printf 'green' ;; - *"no CI checks reported yet"*|*"checks failed"*|*"issues detected"*|*"CI checks running"*|*"base branch advanced"*"re-arming CI monitor timeout"*) printf 'not-ready' ;; + *"checks passed"*) printf 'green' ;; + *"no CI checks reported - still monitoring"*|*"no CI checks reported yet"*|*"checks failed"*|*"issues detected"*|*"CI checks running"*|*"base branch advanced"*"re-arming CI monitor timeout"*) printf 'not-ready' ;; *) printf 'unknown' ;; esac } + +nm_ci_state_is_green() { + [ "${1:-}" = green ] +} # Coarse fallback for cross-branch attribution. `no-mistakes axi status` (bare) # reports the active-or-most-recent run for the CURRENT branch when one # exists, else falls back to some other branch's run purely as informational @@ -448,7 +464,29 @@ 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" ;; + # checks-passed is the pipeline's REPORTED terminal claim, and on + # 2026-08-02 it was reported for a head whose check-run set was empty. + # Corroborate it against the run's own ci log before repeating it: a + # claim of green that the run's own evidence does not record is not a + # green head, and a task whose checks never ran needs firstmate rather + # than a place in the ready-for-review queue. + checks-passed) + CI_LOG_STATE=$(nm_ci_checks_state) + if nm_ci_state_is_green "$CI_LOG_STATE"; then + RUN_STATE="done"; RUN_DETAIL="checks green: PR ready for review" + else + case "$CI_LOG_STATE" in + not-ready) + RUN_STATE=blocked + RUN_DETAIL="run reported a passing result its own CI log does not record: nothing verified this head" + ;; + unknown|"") + RUN_STATE=unknown + RUN_DETAIL="run reported checks-passed, but its CI log is unavailable: claim could not be corroborated" + ;; + esac + fi + ;; failed) RUN_STATE=failed; RUN_DETAIL="run failed" ;; cancelled) RUN_STATE=failed; RUN_DETAIL="run cancelled" ;; *) RUN_STATE=unknown; RUN_DETAIL="outcome: $outcome" ;; @@ -483,7 +521,7 @@ if [ "$HAVE_RUN" = 1 ]; then case "$CI_STEP_STATUS" in running) CI_LOG_STATE=$(nm_ci_checks_state) - if [ "$CI_LOG_STATE" = green ]; then + if nm_ci_state_is_green "$CI_LOG_STATE"; then RUN_STATE="done" RUN_DETAIL="checks green: PR ready for review (still monitoring for merge/close)" fi @@ -498,18 +536,23 @@ if [ "$HAVE_RUN" = 1 ]; then if [ "$RUN_STATE" = working ] && log_reports_ci_ready; then if [ "$RUN_SOURCE" = coarse ]; then - emit "done" status-log "$(status_line_note "$LOG_LINE")${SEP}run still monitoring PR" - fi - [ -n "$CI_STEP_STATUS" ] || CI_STEP_STATUS=$(nm_effective_ci_step_status) - if [ "$RUN_STATUS" = fixing ]; then - CI_LOG_STATE=not-ready - elif [ "$CI_STEP_STATUS" = running ] && [ -z "$CI_LOG_STATE" ]; then - CI_LOG_STATE=$(nm_ci_checks_state) - elif [ "$CI_STEP_STATUS" = fixing ]; then - CI_LOG_STATE=not-ready - fi - if [ "$CI_LOG_STATE" != not-ready ]; then - emit "done" status-log "$(status_line_note "$LOG_LINE")${SEP}run still monitoring PR" + RUN_STATE=unknown + RUN_DETAIL="status log reported readiness, but coarse run data cannot corroborate the claim" + else + [ -n "$CI_STEP_STATUS" ] || CI_STEP_STATUS=$(nm_effective_ci_step_status) + if [ "$RUN_STATUS" = fixing ]; then + CI_LOG_STATE=not-ready + elif [ "$CI_STEP_STATUS" = running ] && [ -z "$CI_LOG_STATE" ]; then + CI_LOG_STATE=$(nm_ci_checks_state) + elif [ "$CI_STEP_STATUS" = fixing ]; then + CI_LOG_STATE=not-ready + fi + if nm_ci_state_is_green "$CI_LOG_STATE"; then + emit "done" status-log "$(status_line_note "$LOG_LINE")${SEP}run still monitoring PR" + elif [ -z "$CI_LOG_STATE" ] || [ "$CI_LOG_STATE" = unknown ]; then + RUN_STATE=unknown + RUN_DETAIL="status log reported readiness, but CI evidence is unavailable: claim could not be corroborated" + fi fi fi diff --git a/bin/fm-pr-merge.sh b/bin/fm-pr-merge.sh index dab371fc142..efc3e9c0945 100755 --- a/bin/fm-pr-merge.sh +++ b/bin/fm-pr-merge.sh @@ -1,8 +1,51 @@ #!/usr/bin/env bash -# Merge a task's PR after recording pr= and any available pr_head= through -# bin/fm-pr-check.sh, so teardown can verify landed work after squash merges. -# The full canonical GitHub PR URL is parsed by bin/fm-pr-lib.sh and the derived -# owner/repository and PR number are passed to gh-axi as separate arguments. +# Merge a task's PR after re-verifying the pull request's current head, then +# record pr= and any available pr_head= through bin/fm-pr-check.sh, so teardown +# can verify landed work after squash merges. The full canonical GitHub PR URL +# is parsed by bin/fm-pr-lib.sh and the derived owner/repository and PR number +# are passed to gh-axi as separate arguments. +# +# Verification re-reads the pull request rather than trusting any recorded +# value, because a PR can go red between an earlier check and the merge and +# state/.meta may carry a stale pr_head=. An early read refuses without +# recording the PR or arming its poll, then a final authoritative read runs after +# fm-pr-check.sh and immediately before the verification metadata write and +# merge. Each `gh pr view` call reads the head, mergeability, review decision, +# and check rollup together, so every state-based refusal names the exact head it +# evaluated once GitHub has supplied a readable head. +# The merge is refused when: +# * no check runs exist on that head - an empty rollup is never read as green, +# which is the whole point of this guard: a cross-repo fork PR held at +# action_required dispatches zero workflows and reports zero failures. The +# refusal names why the set is empty, separating a head with no CI +# configured from one whose workflows are held awaiting approval; +# * any check run is not SUCCESS - a queued, in-progress, skipped, neutral, +# cancelled, or failed run all refuse, so the guard fails closed on anything +# that is not an observed pass. Runs that returned an adverse verdict and +# runs that returned no verdict are counted and reported separately, so a +# head nothing examined is never described as a head something rejected; +# * the pull request is not MERGEABLE - CONFLICTING and a not-yet-computed +# UNKNOWN both refuse; +# * a review requests changes. +# +# --allow-unverified is the captain's explicit override. It is never a default +# and never inferred from the environment: it skips verification entirely and +# records merge_verification=override in the task's meta so an unverified merge +# stays visible afterwards. A verified merge records merge_verification=verified +# and merge_verified_head=; absence of both keys means unknown, never +# verified. Both keys are written before pr= so the metadata identity contract in +# bin/fm-pr-lib.sh still parses. The flag is recognised only before the optional +# -- separator; after it, it is forwarded to gh-axi, which rejects it. +# +# The final verification is not atomically bound to the merge. It narrows the +# remaining race window to the verification metadata write, but a head can still +# change before the merge. Closing that race requires a server-side head +# precondition under decision +# pipeline-reports-green-on-absent-ci-decision-merge-atomic-binding. The real +# `gh pr merge` supports `--match-head-commit SHA`, but gh-axi constructs its gh +# arguments from a fixed allowlist of the method, --auto, --delete-branch, --body, +# and --subject and silently drops other flags. Adopting the precondition later +# therefore requires changing the single gh-axi invocation at the end. # # A task released before its pull request lands keeps a durable landing record # instead of a meta, and this path lands it through that record. A task released @@ -14,7 +57,7 @@ # Merge method defaults to --squash when the caller passes none of --squash, # --merge, --rebase, or --method after the optional -- separator. Extra args # must not include --repo or -R because the repository comes only from the URL. -# Usage: fm-pr-merge.sh [-- ] +# Usage: fm-pr-merge.sh [--allow-unverified] [-- ] set -eu SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" @@ -44,7 +87,15 @@ PR_OWNER=$FM_PR_OWNER PR_REPO=$FM_PR_REPO PR_NUMBER=$FM_PR_NUMBER shift 2 -[ "${1:-}" = "--" ] && shift + +ALLOW_UNVERIFIED=0 +while [ "$#" -gt 0 ]; do + case "$1" in + --allow-unverified) ALLOW_UNVERIFIED=1; shift ;; + --) shift; break ;; + *) break ;; + esac +done caller_has_merge_method() { local arg @@ -88,12 +139,194 @@ if ! RECORD=$(fm_pr_identity_record_path "$STATE" "$ID"); then REBUILD=1 fi +# One read of the live pull request, so the head reported in a refusal is the +# same head the checks, mergeability, and review decision were read from. +PR_VERIFY_FIELDS=headRefOid,mergeable,reviewDecision,statusCheckRollup +# A CheckRun carries .conclusion (empty while queued or running); a legacy +# StatusContext carries .state instead. Neither is treated as a pass unless it +# says SUCCESS. +# +# The members are counted in three disjoint buckets, not two, because "ran and +# reported a failure" and "never produced a result" are different facts about a +# head and collapsing them loses the one this guard exists to report. A run that +# failed, errored, timed out, or failed to start returned an adverse verdict; a +# run that is queued, in progress, skipped, neutral, cancelled, stale, or held +# at action_required returned no verdict at all. Both refuse, and each says so +# in its own words. PR_VERIFY_FAILING is the whole adverse set, so anything +# absent from it that is not SUCCESS counts as unrun rather than as a failure. +PR_VERIFY_FAILING='["FAILURE","ERROR","TIMED_OUT","STARTUP_FAILURE"]' +# $s below is jq's own binding, not a shell variable; only the interpolated +# PR_VERIFY_FAILING array is expanded by the shell. +# shellcheck disable=SC2016 +PR_VERIFY_QUERY='"head=\(.headRefOid // "")", +"mergeable=\(.mergeable // "")", +"review=\(.reviewDecision // "")", +"checks=\((.statusCheckRollup // []) | length)", +"unsuccessful=\((.statusCheckRollup // []) | map(select(((.conclusion // .state // "") | ascii_upcase) != "SUCCESS")) | length)", +"failing=\((.statusCheckRollup // []) | map(select(((.conclusion // .state // "") | ascii_upcase) as $s | ('"$PR_VERIFY_FAILING"' | index($s)) != null)) | length)", +"unrun=\((.statusCheckRollup // []) | map(select(((.conclusion // .state // "") | ascii_upcase) as $s | $s != "SUCCESS" and ('"$PR_VERIFY_FAILING"' | index($s)) == null)) | length)"' + +VERIFIED_HEAD= + +# An empty rollup has more than one cause, and the two common ones need +# different work from the captain: a repository with no CI configured for this +# head, and a cross-repo fork pull request whose workflows exist but are held at +# action_required until a maintainer approves them. GitHub reports neither as a +# check run, so both arrive here as the same empty list, but the check-suite +# read below separates them. This only enriches an already-decided refusal: it +# runs on the refusal path alone, reports nothing when it cannot read the +# suites, and can never turn a refusal into a merge. +empty_rollup_evidence() { + local head=$1 counts total held extra + command -v gh >/dev/null 2>&1 || return 0 + counts=$(gh api "repos/$PR_OWNER/$PR_REPO/commits/$head/check-suites" \ + -q '"\(.total_count // 0) \([.check_suites[]? | select(((.conclusion // "") | ascii_downcase) == "action_required")] | length)"' \ + 2>/dev/null) || return 0 + # Exactly two whole numbers, or this response was not the one asked for and + # the refusal stands with no added detail rather than an invented one. + read -r total held extra <<< "$counts" || return 0 + [ -z "$extra" ] || return 0 + [ -n "$total" ] && [ -z "${total//[0-9]/}" ] || return 0 + [ -n "$held" ] && [ -z "${held//[0-9]/}" ] || return 0 + if [ "$held" -gt 0 ]; then + printf ' (%s check suite(s) on it are held at action_required, so its workflows are waiting on a maintainer to approve them and will not run on their own)' \ + "$held" + elif [ "$total" -eq 0 ]; then + printf ' (no check suite exists for it either, so no CI is configured to run on this head)' + fi + return 0 +} + +verify_current_head() { + local output line joined + local head='' mergeable='' review='' checks='' unsuccessful='' failing='' unrun='' + local -a reasons=() + + command -v gh >/dev/null 2>&1 || { + echo "error: refusing to merge: the pull request could not be verified because gh is not on PATH" >&2 + return 1 + } + output=$(gh pr view "$PR_NUMBER" --repo "$PR_OWNER/$PR_REPO" \ + --json "$PR_VERIFY_FIELDS" -q "$PR_VERIFY_QUERY" 2>/dev/null) || { + echo "error: refusing to merge: the pull request state could not be read from GitHub" >&2 + return 1 + } + + while IFS= read -r line || [ -n "$line" ]; do + case "$line" in + head=*) head=${line#head=} ;; + mergeable=*) mergeable=${line#mergeable=} ;; + review=*) review=${line#review=} ;; + checks=*) checks=${line#checks=} ;; + unsuccessful=*) unsuccessful=${line#unsuccessful=} ;; + failing=*) failing=${line#failing=} ;; + unrun=*) unrun=${line#unrun=} ;; + esac + done <<< "$output" + + fm_pr_head_valid "$head" || { + echo "error: refusing to merge: the pull request head commit could not be read from GitHub" >&2 + return 1 + } + # Each count is validated on its own. Concatenating them would let one empty + # field hide behind the other's digits and reach the comparisons below as an + # empty string, which compares as neither zero nor positive and would merge. + if [ -z "$checks" ] || [ -n "${checks//[0-9]/}" ] \ + || [ -z "$unsuccessful" ] || [ -n "${unsuccessful//[0-9]/}" ] \ + || [ -z "$failing" ] || [ -n "${failing//[0-9]/}" ] \ + || [ -z "$unrun" ] || [ -n "${unrun//[0-9]/}" ]; then + printf 'error: refusing to merge head %s: the check rollup could not be read from GitHub\n' \ + "$head" >&2 + return 1 + fi + # The two disjoint buckets must account for exactly the members that are not + # successes. A response that breaks that identity was not understood, and an + # unreadable rollup is reported as unreadable rather than resolved either way. + if [ "$((failing + unrun))" -ne "$unsuccessful" ]; then + printf 'error: refusing to merge head %s: the check rollup could not be read from GitHub\n' \ + "$head" >&2 + return 1 + fi + + [ "$mergeable" = MERGEABLE ] \ + || reasons+=("the pull request is not mergeable (mergeable=${mergeable:-unreported})") + [ "$review" != CHANGES_REQUESTED ] || reasons+=("a review requests changes") + # Zero check runs and all-successful check runs both report zero failures, so + # the empty rollup is refused on its own count and never folded into the + # counts below. A non-empty rollup reports its failed and its unrun members + # separately, so "this was examined and found broken" never reaches the + # captain wearing the words of "this was never examined", or the reverse. + if [ "$checks" -eq 0 ]; then + reasons+=("no check runs exist on this head$(empty_rollup_evidence "$head")") + else + [ "$failing" -eq 0 ] \ + || reasons+=("$failing of $checks check runs failed") + [ "$unrun" -eq 0 ] \ + || reasons+=("$unrun of $checks check runs reported no result (queued, in progress, skipped, neutral, cancelled, or held for approval)") + fi + + if [ "${#reasons[@]}" -gt 0 ]; then + joined=$(printf '%s; ' "${reasons[@]}") + printf 'error: refusing to merge head %s: %s\n' "$head" "${joined%; }" >&2 + return 1 + fi + VERIFIED_HEAD=$head +} + +MERGE_META_TMP= +merge_meta_cleanup() { + [ -z "$MERGE_META_TMP" ] || rm -f -- "$MERGE_META_TMP" + MERGE_META_TMP= +} +trap merge_meta_cleanup EXIT +trap 'exit 1' HUP INT TERM + +# Record how this merge was authorised. The two keys are emitted before any +# pr=/pr_head= lines so fm_pr_metadata_identity_parse, which refuses any unknown +# key after pr=, still accepts the file at every instant. +record_merge_verification() { + local status=$1 head=$2 line state_device meta_device + state_device=$(fm_pr_file_device "$STATE") || return 1 + meta_device=$(fm_pr_file_device "$META") || return 1 + [ "$meta_device" = "$state_device" ] || return 1 + MERGE_META_TMP=$(mktemp "$STATE/.fm-pr-merge-meta.XXXXXX") || return 1 + { + while IFS= read -r line || [ -n "$line" ]; do + case "$line" in + merge_verification=*|merge_verified_head=*|pr=*|pr_head=*) ;; + *) printf '%s\n' "$line" ;; + esac + done < "$META" + printf 'merge_verification=%s\n' "$status" + [ -z "$head" ] || printf 'merge_verified_head=%s\n' "$head" + while IFS= read -r line || [ -n "$line" ]; do + case "$line" in + pr=*|pr_head=*) printf '%s\n' "$line" ;; + esac + done < "$META" + } > "$MERGE_META_TMP" || return 1 + chmod 0600 "$MERGE_META_TMP" || return 1 + fm_pr_private_file_valid "$MERGE_META_TMP" 600 "$state_device" || return 1 + fm_pr_regular_destination_on_device_or_absent "$META" "$state_device" || return 1 + mv -f -- "$MERGE_META_TMP" "$META" || return 1 + MERGE_META_TMP= +} + +# This fork carries a released task's landing record as well as a live task's +# meta, so the record is resolved in three ordered stages rather than one. Its +# own refusals - a torn-down meta, an invalid landing record, a record naming +# another request - come first and cost no forge read, exactly as before. +# Verification runs next, so nothing is recorded and no poll is armed for a head +# it refuses. Only then does the task's identity get written. +LIVE_TASK=0 if [ "$RECORD" != "$LANDING" ]; then if [ ! -f "$RECORD" ] || [ -L "$RECORD" ]; then echo "error: task metadata is unavailable" >&2 exit 1 fi - "$SCRIPT_DIR/fm-pr-check.sh" "$ID" "$URL" + # Recording and arming are deferred until after verification below, so a head + # the guard refuses leaves the task with no pr= and no armed poll. + LIVE_TASK=1 else if [ "$REBUILD" = 0 ]; then fm_pr_metadata_identity_parse "$RECORD" || { @@ -138,11 +371,36 @@ else } [ "$REBUILD" = 0 ] || printf 'rebuilt: state/%s.landing from %s\n' "$ID" "$URL" fi + +if [ "$ALLOW_UNVERIFIED" -ne 1 ]; then + verify_current_head || exit 1 +fi + +[ "$LIVE_TASK" = 0 ] || "$SCRIPT_DIR/fm-pr-check.sh" "$ID" "$URL" +META=$RECORD grep -qxF "pr=$URL" "$RECORD" || { echo "error: PR metadata recording failed" >&2 exit 1 } +if [ "$ALLOW_UNVERIFIED" -eq 1 ]; then + MERGE_VERIFICATION=override + VERIFIED_HEAD= +else + VERIFIED_HEAD= + verify_current_head || exit 1 + MERGE_VERIFICATION=verified +fi + +record_merge_verification "$MERGE_VERIFICATION" "$VERIFIED_HEAD" || { + echo "error: merge verification metadata could not be recorded" >&2 + exit 1 +} +grep -qxF "merge_verification=$MERGE_VERIFICATION" "$META" || { + echo "error: merge verification metadata could not be recorded" >&2 + exit 1 +} + merge_args=() if ! caller_has_merge_method "$@"; then merge_args=(--squash) diff --git a/docs/architecture.md b/docs/architecture.md index d5894f12e0f..f4c9c462b5b 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -275,12 +275,16 @@ The armed merge poll also reports conflicts: one request carries the pull reques Conflicts are deduped by the head commit reported with them, because poll silence cannot distinguish clean from unknown from a failed lookup while a changed head does mean the branch moved; an untouched conflict re-surfaces no more often than `FM_PR_DIRTY_RESURFACE_SECS`. GitHub can briefly report mergeability as unknown while it recomputes after a base push, which is silence here and resolves on the following sweep rather than costing the static poll a retry. GitLab merge requests keep merge-only detection, because plain `glab mr view` field output carries no conflict field and reading one would require the JSON processor firstmate deliberately does not depend on. -PR-based task merges go through `bin/fm-pr-merge.sh`, which resolves the task's forge-verified landing identity before calling `gh-axi pr merge`. +PR-based task merges go through `bin/fm-pr-merge.sh`, which resolves the task's forge-verified landing identity, pre-checks the pull request's current head, and then authoritatively re-verifies immediately before recording the verified head and calling `gh-axi pr merge`. For a live task it records `pr=` and any available `pr_head=` and arms the merge poll through `bin/fm-pr-check.sh`; for a released task it merges synchronously through the landing record without arming a poll. 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. Landing identity comes from the task's durable record, and from the pull request itself when the task has none. A ship task released before its pull request lands keeps a minimal `state/.landing` record instead of its meta, so the same merge helper still lands that request and `bin/fm-pr-check.sh` can still rearm its merge watch; a task released before landing records existed has its record rebuilt from a forge read of the request. Either way the merge re-reads the request at its forge, so no stale local value decides anything, and a task with no record and no resolvable request is still refused. +Verification reads the head, mergeability, review decision, and check rollup in one `gh pr view` call at merge time rather than trusting a recorded value, so a head that went red after an earlier check is still caught and every state-based refusal names the exact head it evaluated once GitHub supplies a readable head. +The final verification is not atomically bound to the merge: it narrows the remaining race window to the verification metadata write, but a race remains however small, and closing it requires a server-side head precondition tracked by decision `pipeline-reports-green-on-absent-ci-decision-merge-atomic-binding`. +An empty check rollup refuses on its own count instead of being read as green, because a cross-repo fork pull request held for maintainer approval dispatches no workflows and therefore reports no failures. +[`bin/fm-pr-merge.sh`](../bin/fm-pr-merge.sh)'s header owns the full refusal list and the `--allow-unverified` override, which is never inferred and is recorded in the task's metadata so an unverified merge stays visible. 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, landing-record rule, PR-discovery fallback, and stale-lock recovery procedure. diff --git a/docs/scripts.md b/docs/scripts.md index 87ddd570ce2..b8a946b9e01 100644 --- a/docs/scripts.md +++ b/docs/scripts.md @@ -107,7 +107,7 @@ The shared no-mistakes gate refusal for fleet lifecycle entrypoints is summarize | `fm-pr-poll.sh` | Provide the byte-static watcher program reporting merged and conflicted 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 identity in live meta or a landing record, then atomically arm a static PR poll | -| `fm-pr-merge.sh` | Forge-verify landing identity, then merge a task's canonical full GitHub URL | +| `fm-pr-merge.sh` | Forge-verify landing identity, re-verify a PR's current head, then merge a task's canonical full GitHub URL | | `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-crew-state.test.sh b/tests/fm-crew-state.test.sh index 8f986b6139e..be3b8b2ce0a 100755 --- a/tests/fm-crew-state.test.sh +++ b/tests/fm-crew-state.test.sh @@ -291,6 +291,19 @@ outcome: failed EOF } +run_checks_passed() { # + cat < cat < + local step_row="" + if [ "$2" != absent ]; then + step_row=" ci,$2,0,0" + fi + cat < "$d/state/feat-ci.status" FM_FAKE_AXI_STATUS="$(run_ci_monitoring fm/feat-ci)" + FM_FAKE_CI_LOGS="all CI checks passed - still monitoring until merged or closed" local out; out=$(run_crew_state "$d" feat-ci) assert_contains "$out" "state: done" "ci-ready status log -> done" - assert_contains "$out" "source: status-log" "ci-ready state comes from the status log" - assert_contains "$out" "checks green" "ci-ready detail preserves the report" + assert_contains "$out" "source: run-step" "corroborated ci-ready state comes from the run" + assert_contains "$out" "checks green" "ci-ready detail reports corroborated green checks" assert_not_contains "$out" "state: working" "ci-ready is not hidden by monitoring run" - pass "ci-ready status log beats monitoring run" + pass "ci-ready status log agrees with the corroborated run" } # Regression for the PR #252 incident: the crew's own status log never got a @@ -499,7 +534,13 @@ test_top_level_ci_checks_green_surfaces_done() { pass "top-level ci status uses ci log green marker" } -test_ci_monitoring_no_checks_terminal_surfaces_done() { +# The 2026-08-02 defect, pinned from the other side. This marker means the +# pipeline reached the end of its CI wait having seen zero check runs, which is +# what a cross-repo fork pull request produces when its workflows sit at +# action_required and never dispatch. Nothing was red because nothing ran, and +# reading that absence as a terminal green is what put an unverified head in +# front of the captain as work ready for review. +test_ci_monitoring_no_checks_terminal_is_not_green() { reset_fakes local d; d=$(new_case ci-nochecks) make_repo_on_branch "$d/wt" fm/feat-cinochecks @@ -508,9 +549,120 @@ test_ci_monitoring_no_checks_terminal_surfaces_done() { FM_FAKE_AXI_STATUS="$(run_ci_monitoring fm/feat-cinochecks)" FM_FAKE_CI_LOGS="no CI checks reported - still monitoring until merged or closed" local out; out=$(run_crew_state "$d" feat-cinochecks) - assert_contains "$out" "state: done" "terminal no-checks ci-monitor run -> done" - assert_contains "$out" "checks green" "terminal no-checks ci-monitor detail mentions checks green" - pass "terminal no-checks ci-monitor marker surfaces done" + assert_not_contains "$out" "checks green" "a run that saw zero checks must not read as checks green" + assert_not_contains "$out" "state: done" "a run that saw zero checks must not read as done" + assert_contains "$out" "state: working" "a run that saw zero checks stays working" + pass "terminal no-checks ci-monitor marker is never reported as green" +} + +# The positive control for the case above: the marker that means every check ran +# and passed still reaches the captain as ready work, so the fix above refuses +# absence rather than refusing everything. +test_ci_monitoring_all_checks_passed_still_green() { + reset_fakes + local d; d=$(new_case ci-allpassed) + make_repo_on_branch "$d/wt" fm/feat-ciallpassed + make_fakebin "$d" >/dev/null + fm_write_meta "$d/state/feat-ciallpassed.meta" "window=fm:fm-feat-ciallpassed" "worktree=$d/wt" "kind=ship" + FM_FAKE_AXI_STATUS="$(run_ci_monitoring fm/feat-ciallpassed)" + FM_FAKE_CI_LOGS="all CI checks passed - still monitoring until merged or closed" + local out; out=$(run_crew_state "$d" feat-ciallpassed) + assert_contains "$out" "state: done" "an all-passed ci-monitor run -> done" + assert_contains "$out" "checks green" "an all-passed ci-monitor run mentions checks green" + pass "all-checks-passed ci-monitor marker still surfaces done" +} + +# The same defect arriving through the other channel. Here the pipeline has +# finished and reported the terminal claim `checks-passed`, which is what it +# reported on 2026-08-02 for a head whose check-run set was empty. The claim is +# repeated only when the run's own evidence records checks actually passing. +test_terminal_checks_passed_claim_without_evidence_is_not_green() { + reset_fakes + local d; d=$(new_case ci-claimed-nochecks) + make_repo_on_branch "$d/wt" fm/feat-ciclaimed + make_fakebin "$d" >/dev/null + fm_write_meta "$d/state/feat-ciclaimed.meta" "window=fm:fm-feat-ciclaimed" "worktree=$d/wt" "kind=ship" + FM_FAKE_AXI_STATUS="$(run_checks_passed fm/feat-ciclaimed)" + FM_FAKE_CI_LOGS="no CI checks reported - still monitoring until merged or closed" + local out; out=$(run_crew_state "$d" feat-ciclaimed) + assert_not_contains "$out" "checks green" "an unevidenced checks-passed claim must not read as checks green" + assert_not_contains "$out" "state: done" "an unevidenced checks-passed claim must not read as done" + assert_contains "$out" "state: blocked" "an unevidenced checks-passed claim needs firstmate" + pass "a terminal checks-passed claim its own CI log does not support is never reported as green" +} + +# The matched positive control: the same terminal claim, corroborated by a run +# whose log records every check passing, still reaches the captain as ready. +test_terminal_checks_passed_claim_with_evidence_stays_green() { + reset_fakes + local d; d=$(new_case ci-claimed-passed) + make_repo_on_branch "$d/wt" fm/feat-ciclaimedok + make_fakebin "$d" >/dev/null + fm_write_meta "$d/state/feat-ciclaimedok.meta" "window=fm:fm-feat-ciclaimedok" "worktree=$d/wt" "kind=ship" + FM_FAKE_AXI_STATUS="$(run_checks_passed fm/feat-ciclaimedok)" + FM_FAKE_CI_LOGS="all CI checks passed - still monitoring until merged or closed" + local out; out=$(run_crew_state "$d" feat-ciclaimedok) + assert_contains "$out" "state: done" "an evidenced checks-passed claim -> done" + assert_contains "$out" "checks green" "an evidenced checks-passed claim mentions checks green" + pass "a terminal checks-passed claim its CI log corroborates still surfaces done" +} + +# An unreadable log is not evidence that nothing ran. UNAVAILABLE must not be +# resolved into either a pass or a refusal. +test_terminal_checks_passed_claim_survives_unreadable_log() { + reset_fakes + local d; d=$(new_case ci-claimed-nolog) + make_repo_on_branch "$d/wt" fm/feat-ciclaimednolog + make_fakebin "$d" >/dev/null + fm_write_meta "$d/state/feat-ciclaimednolog.meta" "window=fm:fm-feat-ciclaimednolog" "worktree=$d/wt" "kind=ship" + FM_FAKE_AXI_STATUS="$(run_checks_passed fm/feat-ciclaimednolog)" + FM_FAKE_CI_LOGS="" + local out; out=$(run_crew_state "$d" feat-ciclaimednolog) + assert_contains "$out" "state: unknown" "an unreadable CI log leaves corroboration unknown" + assert_contains "$out" "claim could not be corroborated" "an unreadable CI log reports the unavailable evidence" + assert_not_contains "$out" "checks green" "an unreadable CI log must not corroborate checks-passed" + assert_not_contains "$out" "state: blocked" "an unreadable CI log must not manufacture a refusal" + assert_not_contains "$out" "state: failed" "an unreadable CI log must not manufacture a failure" + pass "an unreadable CI log leaves the terminal claim uncorroborated" +} + +test_unavailable_ci_never_corroborates_green_claims() { + local source step claim case_name d branch id out short + for source in full coarse terminal; do + for step in running fixing completed absent; do + for claim in claim no-claim; do + reset_fakes + case_name="ci-unavailable-${source}-${step}-${claim}" + d=$(new_case "$case_name") + id=${case_name#ci-unavailable-} + branch="fm/$id" + make_repo_on_branch "$d/wt" "$branch" + make_fakebin "$d" >/dev/null + fm_write_meta "$d/state/$id.meta" "window=fm:fm-$id" "worktree=$d/wt" "kind=ship" + if [ "$claim" = claim ]; then + printf 'done: PR https://github.com/o/r/pull/2 checks green\n' > "$d/state/$id.status" + fi + case "$source" in + full) + FM_FAKE_AXI_STATUS=$(run_ci_step_state "$branch" "$step") + ;; + coarse) + short=$(git -C "$d/wt" rev-parse --short=7 HEAD) + FM_FAKE_AXI_STATUS=$(run_ci_step_state fm/other-crew "$step") + FM_FAKE_RUNS_LIST="running $branch $short 2026-08-03 12:00" + ;; + terminal) + FM_FAKE_AXI_STATUS=$(run_checks_passed "$branch") + ;; + esac + FM_FAKE_CI_LOGS="" + out=$(run_crew_state "$d" "$id") + assert_not_contains "$out" "state: done" "$source/$step/$claim unavailable CI must not read done" + assert_not_contains "$out" "checks green" "$source/$step/$claim unavailable CI must not read green" + done + done + done + pass "unavailable CI never corroborates green claims across run mappings" } test_ci_monitoring_green_then_rearm_stays_working() { @@ -738,7 +890,7 @@ EOF pass "cross-branch attribution picks the branch's most recent row" } -test_coarse_run_does_not_probe_other_branch_ci_log_for_ready_status() { +test_coarse_run_does_not_corroborate_ready_status() { reset_fakes local d short; d=$(new_case coarse-ready-other-log) make_repo_on_branch "$d/wt" fm/feat-coarseready @@ -754,10 +906,11 @@ EOF )" FM_FAKE_CI_LOGS="CI checks running, waiting for results..." local out; out=$(run_crew_state "$d" feat-coarseready) - assert_contains "$out" "state: done" "coarse ready status -> done" - assert_contains "$out" "source: status-log" "coarse ready status remains status-log sourced" - assert_not_contains "$out" "state: working" "coarse ready status must not be suppressed by another branch log" - pass "coarse run does not probe another branch's ci log" + assert_contains "$out" "state: unknown" "coarse ready status remains uncorroborated" + assert_contains "$out" "source: run-step" "coarse ready status remains run-step sourced" + assert_not_contains "$out" "checks green" "coarse run data cannot corroborate checks green" + assert_not_contains "$out" "state: done" "coarse run data must not produce done" + pass "coarse run does not corroborate a ready status" } # A different-branch run with NO matching runs-list row must NOT be @@ -1315,10 +1468,15 @@ test_stale_blocked_superseded test_genuine_parked_not_superseded test_scalar_gate_parked_not_superseded test_gate_block_parked_not_superseded -test_ci_ready_done_log_beats_monitoring_run +test_ci_ready_done_log_agrees_with_corroborated_run test_ci_monitoring_checks_green_surfaces_done test_top_level_ci_checks_green_surfaces_done -test_ci_monitoring_no_checks_terminal_surfaces_done +test_ci_monitoring_no_checks_terminal_is_not_green +test_ci_monitoring_all_checks_passed_still_green +test_terminal_checks_passed_claim_without_evidence_is_not_green +test_terminal_checks_passed_claim_with_evidence_stays_green +test_terminal_checks_passed_claim_survives_unreadable_log +test_unavailable_ci_never_corroborates_green_claims test_ci_monitoring_green_then_rearm_stays_working test_ci_monitoring_no_checks_yet_stays_working test_ci_monitoring_still_waiting_stays_working @@ -1331,7 +1489,7 @@ test_terminal_passed test_terminal_failed test_cross_branch_attribution_via_runs_list test_cross_branch_attribution_picks_most_recent_row -test_coarse_run_does_not_probe_other_branch_ci_log_for_ready_status +test_coarse_run_does_not_corroborate_ready_status test_other_branch_run_ignored test_no_run_busy_pane test_no_run_footer_text_alone_is_not_working diff --git a/tests/fm-pr-check-security.test.sh b/tests/fm-pr-check-security.test.sh index b9785f68c9d..61544a017fb 100755 --- a/tests/fm-pr-check-security.test.sh +++ b/tests/fm-pr-check-security.test.sh @@ -70,6 +70,12 @@ SH #!/usr/bin/env bash printf '%s\n' "$*" >> "$FM_TEST_GH_LOG" case " $* " in + *statusCheckRollup*) + # bin/fm-pr-merge.sh's merge-time head verification. Green by default here; + # tests/fm-pr-merge.test.sh owns the refusal matrix for that guard. + printf 'head=%s\nmergeable=MERGEABLE\nreview=\nchecks=1\nunsuccessful=0\nfailing=0\nunrun=0\n' \ + "${FM_TEST_GH_HEAD:-0123456789abcdef0123456789abcdef01234567}" + ;; *" state,mergeable,headRefOid "*) [ "${FM_TEST_GH_FAIL:-0}" = 0 ] || exit 1 [ "${FM_TEST_GH_SLEEP:-0}" = 0 ] || sleep "$FM_TEST_GH_SLEEP" diff --git a/tests/fm-pr-merge.test.sh b/tests/fm-pr-merge.test.sh index 6ca8512416b..327c5400292 100755 --- a/tests/fm-pr-merge.test.sh +++ b/tests/fm-pr-merge.test.sh @@ -5,32 +5,56 @@ # verify against, even on repos with no PR CI where the usual "checks green" # fm-pr-check.sh trigger never fires. # +# It must also re-verify the pull request's current head before merging. A +# cross-repo fork PR held at action_required dispatches zero workflows, so its +# check rollup is empty and reports zero failures; reading that absence as +# success is what let an unverified head reach a merge. Every refusal below has +# a negative control that constructs the failing condition and watches the guard +# fire, because a guard proven only by "no bad merge happened" is not proven. +# # Matrix: # (a) merge records pr= and pr_head= before merging, and merges # (b) merge is refused when gh-axi pr merge itself fails (no silent success) # (c) extra gh-axi pr merge args are forwarded after number and --repo -# (d) merge is refused before gh-axi when nothing resolves the task or its PR +# (d) merge is refused before gh-axi when task meta is missing # (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 # (h) repo override args fail fast because the repo comes from the URL +# (i) an empty check rollup refuses, distinguishably from a failing rollup +# (j) an all-successful rollup with the same zero failure count still merges +# (k) a non-success check run refuses +# (k1) check runs that returned no verdict refuse, distinguishably from failures +# (k2) failures and non-reporters present together are each named separately +# (k3) counts that do not reconcile refuse as unreadable, not as a verdict +# (l) a non-mergeable PR refuses, including a not-yet-computed UNKNOWN +# (m) a review requesting changes refuses +# (n) an unreadable or absent gh refuses rather than merging unverified +# (o) --allow-unverified merges without verifying and records the override +# (p) the override is never inferred from the environment or from after -- +# (q) the torn-down-metadata refusal still fires first, unchanged +# (r) the real GitHub query is exercised end to end against API-shaped JSON +# (s) a head that changes after the early check is refused by the final check # # Released tasks. The captain's parked-completion ruling releases a worker once # its PR is green and mergeable, which is before the PR lands, so the meta above # is gone by the time firstmate merges. These cases cover landing such a PR # through the same sanctioned command: -# (i) a released task's durable landing record lands its PR, arms no poll, +# (t) a released task's durable landing record lands its PR, arms no poll, # and the spent record is removed -# (j) a landing record whose PR is no longer open is refused, not merged -# (k) a task released before landing records existed has its record rebuilt +# (u) a landing record whose PR is no longer open is refused, not merged +# (v) a task released before landing records existed has its record rebuilt # from a forge read of the PR itself -# (l) a PR that resolves to nothing at its forge is still refused -# (m) a landing record naming another PR refuses before any forge or merge read -# (n) a malformed landing record refuses without being reconstructed +# (w) a PR that resolves to nothing at its forge is still refused +# (x) a landing record naming another PR refuses before any forge or merge read +# (y) a malformed landing record refuses without being reconstructed set -u # shellcheck source=tests/lib.sh . "$(dirname "${BASH_SOURCE[0]}")/lib.sh" +# The released-task cases build their landing-record fixtures through the same +# library entry point bin/fm-teardown.sh uses, so a fixture cannot drift from +# the written format. # shellcheck source=/dev/null . "$ROOT/bin/fm-pr-lib.sh" fm_git_identity fmtest fmtest@example.invalid @@ -38,13 +62,15 @@ fm_git_identity fmtest fmtest@example.invalid PR_MERGE="$ROOT/bin/fm-pr-merge.sh" TMP_ROOT=$(fm_test_tmproot fm-pr-merge-tests) +GREEN_HEAD=deadbeefcafefeed0000000000000000deadbeef + # Build a fresh sandbox for one test case: a state dir with a task meta and a # fakebin with a gh-axi mock that records how it was invoked. Echoes the case dir. make_case() { local name=$1 case_dir fakebin case_dir="$TMP_ROOT/$name" fakebin="$case_dir/fakebin" - mkdir -p "$case_dir/state" "$fakebin" + mkdir -p "$case_dir/state" "$fakebin" "$case_dir/emptybin" fm_write_meta "$case_dir/state/task-x1.meta" \ "window=fm-task-x1" \ "worktree=$case_dir/wt" \ @@ -57,34 +83,83 @@ 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 +# write_verify_payload +# [failing] [unrun] +# The seven lines fm-pr-merge.sh reads back from its single `gh pr view` call. +# When a case does not care how the unsuccessful members break down, they +# default to failures, which is the stricter reading and keeps the pre-existing +# cases meaning exactly what they meant before the split. +write_verify_payload() { + local failing=${7:-$6} unrun=${8:-0} + printf 'head=%s\nmergeable=%s\nreview=%s\nchecks=%s\nunsuccessful=%s\nfailing=%s\nunrun=%s\n' \ + "$2" "$3" "$4" "$5" "$6" "$failing" "$unrun" > "$1" +} + +# A green head: mergeable, no review blocking, ten check runs, none unsuccessful. +write_green_payload() { + write_verify_payload "$1" "${2:-$GREEN_HEAD}" MERGEABLE '' 10 0 +} + +# gh-axi mock recording every invocation to a log file, and a `gh` mock standing +# in for the forge. The `gh` mock answers both callers off its argv: the single +# headRefOid field is fm-pr-check.sh's pr_head lookup, and any request naming +# statusCheckRollup is fm-pr-merge.sh's merge-time verification. When a JSON +# fixture is supplied it evaluates the script's real -q query against that +# fixture with jq, so the query itself is under test and not just the branch +# logic reading a canned answer. add_gh_mocks() { - local case_dir=$1 head=$2 + local case_dir=$1 head=${2:-$GREEN_HEAD} 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" < "$case_dir/fakebin/gh" <<'SH' #!/usr/bin/env bash -printf '%s\n' "\$*" >> "\$FM_TEST_GH_LOG" -case "\${1:-} \${2:-}" in - "pr view") - case " \$* " in - *headRefOid*) printf '%s\n' '$head' ; exit 0 ;; - esac +printf '%s\n' "$*" >> "$FM_TEST_GH_LOG" +fields= +query= +while [ "$#" -gt 0 ]; do + case "$1" in + --json) fields=${2:-}; shift; [ "$#" -gt 0 ] && shift ;; + -q|--jq) query=${2:-}; shift; [ "$#" -gt 0 ] && shift ;; + *) shift ;; + esac +done +case "$fields" in + *statusCheckRollup*) + [ "${FM_TEST_GH_VERIFY_RC:-0}" = 0 ] || exit "${FM_TEST_GH_VERIFY_RC}" + if [ -n "${FM_TEST_GH_FIXTURE:-}" ]; then + jq -r "$query" "$FM_TEST_GH_FIXTURE" + exit $? + fi + if [ -n "${FM_TEST_GH_VERIFY_SEQUENCE_PREFIX:-}" ]; then + verify_call=$(cat "$FM_TEST_GH_VERIFY_SEQUENCE_PREFIX.count" 2>/dev/null || printf '0') + verify_call=$((verify_call + 1)) + printf '%s\n' "$verify_call" > "$FM_TEST_GH_VERIFY_SEQUENCE_PREFIX.count" + cat "$FM_TEST_GH_VERIFY_SEQUENCE_PREFIX.$verify_call" + exit 0 + fi + cat "$FM_TEST_GH_VERIFY_PAYLOAD" + exit 0 + ;; + *headRefOid*) + printf '%s\n' "${FM_TEST_GH_HEAD:-}" + exit 0 ;; esac exit 0 SH chmod +x "$case_dir/fakebin/gh-axi" "$case_dir/fakebin/gh" + printf '%s\n' "$head" > "$case_dir/head" + write_green_payload "$case_dir/verify.txt" "$head" } # 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 + add_gh_mocks "$case_dir" "${2:-$GREEN_HEAD}" cat > "$case_dir/fakebin/gh-axi" <<'SH' #!/usr/bin/env bash printf '%s\n' "$*" >> "$FM_TEST_GH_AXI_LOG" @@ -93,81 +168,22 @@ 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" -} - -# A released task's sandbox: a state dir with NO meta, which is what teardown -# leaves behind once the captain's parked-completion ruling releases a worker. -# Echoes the case dir. -make_released_case() { - local name=$1 case_dir - case_dir="$TMP_ROOT/$name" - mkdir -p "$case_dir/state" "$case_dir/fakebin" - printf '%s\n' "$case_dir" -} - -# Build the durable landing record through the same library entry point -# bin/fm-teardown.sh uses, so the fixture cannot drift from the written format. -# Args: case_dir task_id pr_url head_sha -write_landing_record() { - local case_dir=$1 id=$2 url=$3 head=$4 - fm_pr_landing_record_write "$case_dir/state" "$id" "$url" "$head" "$case_dir/project" \ - || fail "could not build the landing record fixture for $id" -} - -# gh-axi mock as above, plus a gh mock that answers the forge view landing -# identity is read from. Args: case_dir head_sha forge_state -add_gh_mocks_forge_state() { - 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" -exit 0 -SH - cat > "$case_dir/fakebin/gh" <> "\$FM_TEST_GH_LOG" -case "\${1:-} \${2:-}" in - "pr view") - case " \$* " in - *"state,headRefOid"*) printf '%s\t%s\n' '$state' '$head' ; exit 0 ;; - *"headRefOid"*) printf '%s\n' '$head' ; exit 0 ;; - esac - ;; -esac -echo "error: pull request not found" >&2 -exit 1 -SH - chmod +x "$case_dir/fakebin/gh-axi" "$case_dir/fakebin/gh" -} - -# gh mock whose pull request lookups all fail, so the forge answers nothing. -add_gh_mocks_forge_unreachable() { - local case_dir=$1 - 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" <<'SH' -#!/usr/bin/env bash -echo "error: pull request not found" >&2 -exit 1 -SH - chmod +x "$case_dir/fakebin/gh-axi" "$case_dir/fakebin/gh" + chmod +x "$case_dir/fakebin/gh-axi" } run_pr_merge() { - local case_dir=$1 rc; shift + local case_dir=$1 rc head; shift + head=$(cat "$case_dir/head" 2>/dev/null || printf '%s' "$GREEN_HEAD") 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" \ + FM_TEST_GH_HEAD="$head" \ + FM_TEST_GH_VERIFY_PAYLOAD="$case_dir/verify.txt" \ + FM_TEST_GH_VERIFY_RC="${FM_TEST_GH_VERIFY_RC:-0}" \ + FM_TEST_GH_FIXTURE="${FM_TEST_GH_FIXTURE:-}" \ + FM_TEST_GH_VERIFY_SEQUENCE_PREFIX="${FM_TEST_GH_VERIFY_SEQUENCE_PREFIX:-}" \ + PATH="${FM_TEST_PATH_OVERRIDE:-$case_dir/fakebin:$PATH}" \ "$PR_MERGE" "$@" rc=$? if [ "${case_dir##*/}" = unsafe-url-segment ] && [ "$rc" -eq 2 ]; then @@ -177,6 +193,18 @@ run_pr_merge() { return "$rc" } +# Every refusal must leave the task untouched: no PR recorded, no merge poll +# armed, and no merge attempted. +assert_no_merge_side_effects() { + local case_dir=$1 label=$2 + assert_no_grep 'pr=https://' "$case_dir/state/task-x1.meta" \ + "$label: a refused merge recorded a PR in the task record" + assert_absent "$case_dir/state/task-x1.check.sh" \ + "$label: a refused merge armed a merge poll" + assert_no_grep 'pr merge' "$case_dir/gh-axi.log" \ + "$label: a refused merge still invoked gh-axi pr merge" +} + test_records_pr_and_head_before_merging() { local case_dir rc case_dir=$(make_case records-before-merge) @@ -220,7 +248,7 @@ test_merge_failure_propagates_after_recording() { } test_extra_merge_args_forwarded() { - local case_dir rc + local case_dir case_dir=$(make_case extra-args) mkdir -p "$case_dir/wt" add_gh_mocks "$case_dir" 2222222222222222222222222222222222222222 @@ -261,156 +289,6 @@ test_missing_meta_refuses_before_merge() { pass "fm-pr-merge refuses when neither a task record nor a resolvable PR exists" } -test_landing_record_lands_released_task() { - local case_dir rc - case_dir=$(make_released_case landing-record) - write_landing_record "$case_dir" task-x1 https://github.com/example/repo/pull/20 \ - aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa - add_gh_mocks_forge_state "$case_dir" bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb OPEN - : > "$case_dir/gh-axi.log" - : > "$case_dir/gh.log" - - set +e - run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/20 \ - > "$case_dir/stdout" 2> "$case_dir/stderr" - rc=$? - set -e - - expect_code 0 "$rc" "landing-record: fm-pr-merge should land a released task's PR" - grep -qxF 'pr merge 20 --repo example/repo --squash' "$case_dir/gh-axi.log" \ - || fail "landing-record: gh-axi pr merge was not invoked for the released task" - assert_absent "$case_dir/state/task-x1.check.sh" \ - "landing-record: a released task has nothing to watch, so no poll should be armed" - assert_absent "$case_dir/state/task-x1.landing" \ - "landing-record: the spent landing record should be removed once the PR landed" - pass "fm-pr-merge lands a released task's PR through its durable landing record" -} - -test_landing_record_refuses_when_pr_is_not_open() { - local case_dir rc - case_dir=$(make_released_case landing-record-merged) - write_landing_record "$case_dir" task-x1 https://github.com/example/repo/pull/20 \ - aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa - # Same record as the case above; only the forge's answer differs, so the local - # record alone can never authorize the merge. - add_gh_mocks_forge_state "$case_dir" bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb MERGED - : > "$case_dir/gh-axi.log" - - set +e - run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/20 \ - > "$case_dir/stdout" 2> "$case_dir/stderr" - rc=$? - set -e - - expect_code 1 "$rc" "landing-record-merged: fm-pr-merge should refuse a PR that is not open" - assert_grep 'is merged at its forge' "$case_dir/stderr" \ - "landing-record-merged: refusal did not report the forge's own state" - assert_no_grep 'pr merge' "$case_dir/gh-axi.log" \ - "landing-record-merged: gh-axi pr merge was invoked for a PR that is not open" - assert_present "$case_dir/state/task-x1.landing" \ - "landing-record-merged: a refused merge must not discard the landing record" - pass "fm-pr-merge re-reads the forge and refuses a landing record whose PR is not open" -} - -test_landing_record_refuses_different_requested_pr() { - local case_dir rc recorded_url requested_url - recorded_url=https://github.com/example/repo/pull/20 - requested_url=https://github.com/example/repo/pull/21 - case_dir=$(make_released_case landing-record-mismatch) - write_landing_record "$case_dir" task-x1 "$recorded_url" \ - aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa - add_gh_mocks_forge_state "$case_dir" bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb OPEN - : > "$case_dir/gh-axi.log" - : > "$case_dir/gh.log" - - set +e - run_pr_merge "$case_dir" task-x1 "$requested_url" \ - > "$case_dir/stdout" 2> "$case_dir/stderr" - rc=$? - set -e - - expect_code 1 "$rc" "landing-record-mismatch: fm-pr-merge should refuse a different PR" - assert_grep "$recorded_url" "$case_dir/stderr" \ - "landing-record-mismatch: refusal did not name the recorded URL" - assert_grep "$requested_url" "$case_dir/stderr" \ - "landing-record-mismatch: refusal did not name the requested URL" - assert_no_grep 'pr merge' "$case_dir/gh-axi.log" \ - "landing-record-mismatch: gh-axi pr merge was invoked" - [ ! -s "$case_dir/gh.log" ] || fail "landing-record-mismatch: the forge was read" - assert_grep "pr=$recorded_url" "$case_dir/state/task-x1.landing" \ - "landing-record-mismatch: the landing record was rebound" - pass "fm-pr-merge refuses a requested PR that differs from the landing record" -} - -test_malformed_landing_record_refuses_without_rebuild() { - local case_dir rc url=https://github.com/example/repo/pull/20 - case_dir=$(make_released_case malformed-landing-record) - printf '%s\n' fm-landing-v1 'pr=not-a-url' > "$case_dir/state/task-x1.landing" - chmod 0600 "$case_dir/state/task-x1.landing" - add_gh_mocks_forge_state "$case_dir" bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb OPEN - : > "$case_dir/gh-axi.log" - : > "$case_dir/gh.log" - - set +e - run_pr_merge "$case_dir" task-x1 "$url" > "$case_dir/stdout" 2> "$case_dir/stderr" - rc=$? - set -e - - expect_code 1 "$rc" "malformed-landing-record: fm-pr-merge should refuse" - assert_grep 'error: task landing record is invalid' "$case_dir/stderr" \ - "malformed-landing-record: refusal did not identify invalid state" - [ ! -s "$case_dir/gh.log" ] || fail "malformed-landing-record: the forge was read" - assert_grep 'pr=not-a-url' "$case_dir/state/task-x1.landing" \ - "malformed-landing-record: malformed state was reconstructed" - pass "fm-pr-merge refuses a malformed landing record without rebuilding it" -} - -test_reconstructs_landing_record_from_pr_url() { - local case_dir rc - case_dir=$(make_released_case reconstruct-from-url) - # Released before landing records existed: no meta and no landing record, so - # the PR itself is the only authority for its own identity. - add_gh_mocks_forge_state "$case_dir" cccccccccccccccccccccccccccccccccccccccc OPEN - : > "$case_dir/gh-axi.log" - - set +e - run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/13 \ - > "$case_dir/stdout" 2> "$case_dir/stderr" - rc=$? - set -e - - expect_code 0 "$rc" "reconstruct-from-url: fm-pr-merge should rebuild the record and land the PR" - assert_grep 'rebuilt: state/task-x1.landing' "$case_dir/stdout" \ - "reconstruct-from-url: the rebuild was not reported" - grep -qxF 'pr merge 13 --repo example/repo --squash' "$case_dir/gh-axi.log" \ - || fail "reconstruct-from-url: gh-axi pr merge was not invoked" - assert_absent "$case_dir/state/task-x1.check.sh" \ - "reconstruct-from-url: no poll should be armed for a released task" - pass "fm-pr-merge rebuilds a released task's landing record from the PR itself" -} - -test_reconstruction_refuses_when_pr_is_not_open() { - local case_dir rc - case_dir=$(make_released_case reconstruct-closed) - add_gh_mocks_forge_state "$case_dir" dddddddddddddddddddddddddddddddddddddddd CLOSED - : > "$case_dir/gh-axi.log" - - set +e - run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/13 \ - > "$case_dir/stdout" 2> "$case_dir/stderr" - rc=$? - set -e - - expect_code 1 "$rc" "reconstruct-closed: fm-pr-merge should refuse to rebuild from a closed PR" - assert_grep 'error: task metadata is unavailable' "$case_dir/stderr" \ - "reconstruct-closed: refusal did not keep the missing-record diagnostic" - assert_no_grep 'pr merge' "$case_dir/gh-axi.log" \ - "reconstruct-closed: gh-axi pr merge was invoked for a closed PR" - assert_absent "$case_dir/state/task-x1.landing" \ - "reconstruct-closed: a closed PR must not leave a landing record behind" - pass "fm-pr-merge refuses to rebuild a landing record from a PR that is not open" -} - test_malformed_url_refuses_before_merge() { local case_dir rc case_dir=$(make_case malformed-url) @@ -533,6 +411,818 @@ test_parses_pr_url_for_gh_axi() { pass "fm-pr-merge parses a GitHub PR URL into gh-axi number and --repo arguments" } +# --- head verification: negative controls ----------------------------------- + +# The defect this guard exists for. An empty rollup reports zero failures, which +# is byte-identical to an all-successful rollup's zero failures, so the refusal +# must key on the run count and not on the failure count. +test_zero_check_runs_refuses() { + local case_dir rc head=1111111111111111111111111111111111111111 + case_dir=$(make_case zero-checks) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" "$head" + write_verify_payload "$case_dir/verify.txt" "$head" MERGEABLE '' 0 0 + : > "$case_dir/gh-axi.log" + + set +e + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/31 \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "zero-checks: an empty check rollup must refuse the merge" + assert_grep 'no check runs exist on this head' "$case_dir/stderr" \ + "zero-checks: refusal did not name the empty check rollup" + assert_grep "$head" "$case_dir/stderr" \ + "zero-checks: refusal did not name the head commit it evaluated" + assert_no_grep 'check runs failed' "$case_dir/stderr" \ + "zero-checks: empty rollup was reported as a failing rollup instead of an empty one" + assert_no_grep 'reported no result' "$case_dir/stderr" \ + "zero-checks: empty rollup was reported as members that ran without a verdict" + assert_no_merge_side_effects "$case_dir" zero-checks + pass "fm-pr-merge refuses a head with zero check runs and names it as empty, not failing" +} + +# The positive control paired with the case above: same zero failure count, but +# check runs actually exist and all passed. +test_all_successful_checks_still_merges() { + local case_dir rc + case_dir=$(make_case all-success) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" "$GREEN_HEAD" + : > "$case_dir/gh-axi.log" + + set +e + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/32 \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 0 "$rc" "all-success: a green, mergeable PR must still merge" + grep -qxF 'pr merge 32 --repo example/repo --squash' "$case_dir/gh-axi.log" \ + || fail "all-success: a verified green PR was not merged" + assert_grep "merge_verified_head=$GREEN_HEAD" "$case_dir/state/task-x1.meta" \ + "all-success: the verified head was not recorded" + pass "fm-pr-merge still merges a green, mergeable, unreviewed PR" +} + +test_failing_check_run_refuses() { + local case_dir rc head=2121212121212121212121212121212121212121 + case_dir=$(make_case failing-check) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" "$head" + write_verify_payload "$case_dir/verify.txt" "$head" MERGEABLE '' 10 2 + : > "$case_dir/gh-axi.log" + + set +e + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/33 \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "failing-check: a non-success check run must refuse the merge" + assert_grep '2 of 10 check runs failed' "$case_dir/stderr" \ + "failing-check: refusal did not name the failing check runs" + assert_grep "$head" "$case_dir/stderr" \ + "failing-check: refusal did not name the head commit it evaluated" + assert_no_grep 'no check runs exist' "$case_dir/stderr" \ + "failing-check: a failing rollup was reported as an empty one" + assert_no_grep 'reported no result' "$case_dir/stderr" \ + "failing-check: failed check runs were reported as runs that never produced a verdict" + assert_no_merge_side_effects "$case_dir" failing-check + pass "fm-pr-merge refuses a head with non-successful check runs, distinguishably from an empty one" +} + +# The third state, and the one the two-bucket reading collapsed. These members +# ran to completion and returned no verdict at all: nothing failed, and nothing +# passed either. Reporting them as failures would send the captain hunting for a +# broken test that does not exist. +test_unrun_check_runs_refuse_distinguishably() { + local case_dir rc head=2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b2b + case_dir=$(make_case unrun-checks) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" "$head" + write_verify_payload "$case_dir/verify.txt" "$head" MERGEABLE '' 4 4 0 4 + : > "$case_dir/gh-axi.log" + + set +e + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/34 \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "unrun-checks: check runs with no verdict must refuse the merge" + assert_grep '4 of 4 check runs reported no result' "$case_dir/stderr" \ + "unrun-checks: refusal did not name the members that produced no verdict" + assert_no_grep 'check runs failed' "$case_dir/stderr" \ + "unrun-checks: members that never reported were described as failures" + assert_no_grep 'no check runs exist' "$case_dir/stderr" \ + "unrun-checks: a populated rollup was reported as an empty one" + assert_no_merge_side_effects "$case_dir" unrun-checks + pass "fm-pr-merge separates check runs that reported no result from check runs that failed" +} + +# Both kinds present at once: each is counted and named on its own, so neither +# fact is lost behind the other. +test_failing_and_unrun_are_both_reported() { + local case_dir rc head=2c2c2c2c2c2c2c2c2c2c2c2c2c2c2c2c2c2c2c2c + case_dir=$(make_case mixed-checks) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" "$head" + write_verify_payload "$case_dir/verify.txt" "$head" MERGEABLE '' 9 5 2 3 + : > "$case_dir/gh-axi.log" + + set +e + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/35 \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "mixed-checks: a rollup with failures and non-reporters must refuse" + assert_grep '2 of 9 check runs failed' "$case_dir/stderr" \ + "mixed-checks: the failing members were not named" + assert_grep '3 of 9 check runs reported no result' "$case_dir/stderr" \ + "mixed-checks: the non-reporting members were not named" + assert_no_merge_side_effects "$case_dir" mixed-checks + pass "fm-pr-merge reports failed and unrun check runs as separate facts about one head" +} + +# The buckets must account for every unsuccessful member. A response that breaks +# that identity was not understood, and an unreadable rollup is reported as +# unreadable rather than resolved as either a pass or a failure. +test_inconsistent_check_buckets_refuse_as_unreadable() { + local case_dir rc head=2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d2d + case_dir=$(make_case inconsistent-buckets) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" "$head" + write_verify_payload "$case_dir/verify.txt" "$head" MERGEABLE '' 10 4 1 1 + : > "$case_dir/gh-axi.log" + + set +e + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/36 \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "inconsistent-buckets: an unaccounted-for rollup must refuse the merge" + assert_grep 'the check rollup could not be read from GitHub' "$case_dir/stderr" \ + "inconsistent-buckets: refusal did not report the rollup as unreadable" + assert_no_merge_side_effects "$case_dir" inconsistent-buckets + pass "fm-pr-merge reports a rollup whose counts do not reconcile as unreadable, not as a verdict" +} + +test_not_mergeable_refuses() { + local case_dir rc head=3131313131313131313131313131313131313131 + case_dir=$(make_case not-mergeable) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" "$head" + write_verify_payload "$case_dir/verify.txt" "$head" CONFLICTING '' 10 0 + : > "$case_dir/gh-axi.log" + + set +e + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/34 \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "not-mergeable: a conflicting PR must refuse the merge" + assert_grep 'the pull request is not mergeable (mergeable=CONFLICTING)' "$case_dir/stderr" \ + "not-mergeable: refusal did not name the mergeable state" + assert_grep "$head" "$case_dir/stderr" \ + "not-mergeable: refusal did not name the head commit it evaluated" + assert_no_merge_side_effects "$case_dir" not-mergeable + pass "fm-pr-merge refuses a pull request that is not mergeable" +} + +# GitHub computes mergeability asynchronously, so UNKNOWN means "not yet known", +# never "fine". It must refuse rather than merge on an unread state. +test_unknown_mergeable_refuses() { + local case_dir rc head=4141414141414141414141414141414141414141 + case_dir=$(make_case unknown-mergeable) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" "$head" + write_verify_payload "$case_dir/verify.txt" "$head" UNKNOWN '' 10 0 + : > "$case_dir/gh-axi.log" + + set +e + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/35 \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "unknown-mergeable: an uncomputed mergeable state must refuse the merge" + assert_grep 'the pull request is not mergeable (mergeable=UNKNOWN)' "$case_dir/stderr" \ + "unknown-mergeable: refusal did not name the uncomputed mergeable state" + assert_no_merge_side_effects "$case_dir" unknown-mergeable + pass "fm-pr-merge refuses a pull request whose mergeability is not yet computed" +} + +test_changes_requested_refuses() { + local case_dir rc head=5151515151515151515151515151515151515151 + case_dir=$(make_case changes-requested) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" "$head" + write_verify_payload "$case_dir/verify.txt" "$head" MERGEABLE CHANGES_REQUESTED 10 0 + : > "$case_dir/gh-axi.log" + + set +e + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/36 \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "changes-requested: a review requesting changes must refuse the merge" + assert_grep 'a review requests changes' "$case_dir/stderr" \ + "changes-requested: refusal did not name the blocking review" + assert_grep "$head" "$case_dir/stderr" \ + "changes-requested: refusal did not name the head commit it evaluated" + assert_no_merge_side_effects "$case_dir" changes-requested + pass "fm-pr-merge refuses a pull request whose review requests changes" +} + +# An approved review is not a blocker, so it must not be swept up with the +# changes-requested refusal. +test_approved_review_still_merges() { + local case_dir rc + case_dir=$(make_case approved-review) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" "$GREEN_HEAD" + write_verify_payload "$case_dir/verify.txt" "$GREEN_HEAD" MERGEABLE APPROVED 10 0 + : > "$case_dir/gh-axi.log" + + set +e + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/37 \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 0 "$rc" "approved-review: an approved green PR must still merge" + grep -qxF 'pr merge 37 --repo example/repo --squash' "$case_dir/gh-axi.log" \ + || fail "approved-review: an approved green PR was not merged" + pass "fm-pr-merge merges a green pull request that a review approved" +} + +# A truncated or garbled response must refuse rather than fall through the +# numeric comparisons. An absent check count paired with a zero failure count is +# the shape that most easily reads as "nothing wrong here". +test_unreadable_check_counts_refuse() { + local case_dir rc head=8181818181818181818181818181818181818181 + case_dir=$(make_case unreadable-counts) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" "$head" + printf 'head=%s\nmergeable=MERGEABLE\nreview=\nchecks=\nunsuccessful=0\n' "$head" \ + > "$case_dir/verify.txt" + : > "$case_dir/gh-axi.log" + + set +e + 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" "unreadable-counts: an unreadable check count must refuse the merge" + assert_grep 'the check rollup could not be read from GitHub' "$case_dir/stderr" \ + "unreadable-counts: refusal did not name the unreadable check rollup" + assert_no_merge_side_effects "$case_dir" unreadable-counts + + # The mirrored shape: a readable count with an unreadable failure count. + printf 'head=%s\nmergeable=MERGEABLE\nreview=\nchecks=3\nunsuccessful=\n' "$head" \ + > "$case_dir/verify.txt" + : > "$case_dir/gh-axi.log" + set +e + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/40 \ + > "$case_dir/stdout2" 2> "$case_dir/stderr2" + rc=$? + set -e + + expect_code 1 "$rc" "unreadable-counts: an unreadable failure count must refuse the merge" + assert_grep 'the check rollup could not be read from GitHub' "$case_dir/stderr2" \ + "unreadable-counts: refusal did not name the unreadable failure count" + assert_no_merge_side_effects "$case_dir" unreadable-counts-mirrored + pass "fm-pr-merge refuses a check rollup it could not read as two whole counts" +} + +test_unreadable_forge_state_refuses() { + local case_dir rc + case_dir=$(make_case unreadable-state) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" "$GREEN_HEAD" + : > "$case_dir/gh-axi.log" + + set +e + FM_TEST_GH_VERIFY_RC=1 \ + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/38 \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "unreadable-state: an unreadable pull request must refuse the merge" + assert_grep 'could not be read from GitHub' "$case_dir/stderr" \ + "unreadable-state: refusal did not explain the unreadable pull request" + assert_no_merge_side_effects "$case_dir" unreadable-state + pass "fm-pr-merge refuses when the pull request state cannot be read" +} + +# gh ships in the same system directory as the utilities the script needs, so a +# PATH filtered by directory would strip both. Build a curated directory holding +# the tools this path uses and no gh at all. A tool this misses shows up as a +# loud 127 rather than a quietly wrong pass. +minimal_bin_without_gh() { + local dir=$1 tool src + mkdir -p "$dir" + # bash and env are needed for the #!/usr/bin/env bash shebang to resolve. + for tool in bash env dirname uname stat mktemp grep chmod mv rm cat sed; do + src=$(command -v "$tool" 2>/dev/null) && ln -sf "$src" "$dir/$tool" + done + printf '%s\n' "$dir" +} + +# An absent forge CLI must refuse rather than skip verification. +test_absent_gh_refuses() { + local case_dir rc + case_dir=$(make_case absent-gh) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" "$GREEN_HEAD" + rm -f "$case_dir/fakebin/gh" + : > "$case_dir/gh-axi.log" + + set +e + FM_TEST_PATH_OVERRIDE="$case_dir/fakebin:$(minimal_bin_without_gh "$case_dir/minbin")" \ + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/39 \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "absent-gh: a missing forge CLI must refuse the merge" + assert_grep 'gh is not on PATH' "$case_dir/stderr" \ + "absent-gh: refusal did not name the missing forge CLI" + assert_no_merge_side_effects "$case_dir" absent-gh + pass "fm-pr-merge refuses rather than merging unverified when gh is unavailable" +} + +# --- explicit override ------------------------------------------------------ + +test_allow_unverified_merges_and_records_override() { + local case_dir rc head=6161616161616161616161616161616161616161 + case_dir=$(make_case override-allowed) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" "$head" + # Red on every axis, so only the explicit override can let this through. + write_verify_payload "$case_dir/verify.txt" "$head" CONFLICTING CHANGES_REQUESTED 0 0 + : > "$case_dir/gh-axi.log" + : > "$case_dir/gh.log" + + set +e + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/41 --allow-unverified \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 0 "$rc" "override-allowed: the explicit override should merge" + grep -qxF 'pr merge 41 --repo example/repo --squash' "$case_dir/gh-axi.log" \ + || fail "override-allowed: the overridden merge did not run" + assert_grep 'merge_verification=override' "$case_dir/state/task-x1.meta" \ + "override-allowed: the override was not recorded in the task record" + assert_no_grep 'merge_verification=verified' "$case_dir/state/task-x1.meta" \ + "override-allowed: an unverified merge was recorded as verified" + assert_no_grep 'merge_verified_head=' "$case_dir/state/task-x1.meta" \ + "override-allowed: an unverified merge recorded a verified head" + assert_no_grep 'statusCheckRollup' "$case_dir/gh.log" \ + "override-allowed: the override still queried the forge for a verification it ignores" + pass "fm-pr-merge merges on the explicit override and records it as unverified" +} + +test_verified_merge_records_verification() { + local case_dir rc + case_dir=$(make_case override-recorded-verified) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" "$GREEN_HEAD" + : > "$case_dir/gh-axi.log" + + set +e + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/42 \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 0 "$rc" "override-recorded-verified: a green PR should merge" + assert_grep 'merge_verification=verified' "$case_dir/state/task-x1.meta" \ + "override-recorded-verified: a verified merge was not recorded as verified" + assert_grep "merge_verified_head=$GREEN_HEAD" "$case_dir/state/task-x1.meta" \ + "override-recorded-verified: the verified head was not recorded" + assert_no_grep 'merge_verification=override' "$case_dir/state/task-x1.meta" \ + "override-recorded-verified: a verified merge was recorded as an override" + pass "fm-pr-merge records the verified head so a verified merge is distinguishable" +} + +test_final_verification_refuses_changed_head() { + local case_dir rc + local early_head=9191919191919191919191919191919191919191 + local changed_head=9292929292929292929292929292929292929292 + case_dir=$(make_case final-verification-race) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" "$early_head" + write_green_payload "$case_dir/verify-sequence.1" "$early_head" + write_verify_payload "$case_dir/verify-sequence.2" "$changed_head" MERGEABLE '' 10 1 + : > "$case_dir/gh-axi.log" + : > "$case_dir/gh.log" + + set +e + FM_TEST_GH_VERIFY_SEQUENCE_PREFIX="$case_dir/verify-sequence" \ + 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" "final-verification-race: a changed red head must refuse the merge" + assert_grep '1 of 10 check runs failed' "$case_dir/stderr" \ + "final-verification-race: the final check did not report the changed head's failure" + assert_grep "$changed_head" "$case_dir/stderr" \ + "final-verification-race: the refusal did not name the changed head" + assert_grep 'pr=https://github.com/example/repo/pull/44' "$case_dir/state/task-x1.meta" \ + "final-verification-race: the final check did not run after fm-pr-check" + assert_no_grep 'pr merge' "$case_dir/gh-axi.log" \ + "final-verification-race: the changed red head still reached gh-axi pr merge" + pass "fm-pr-merge re-verifies after fm-pr-check and refuses a changed red head" +} + +# The override must be an explicit flag on this invocation and nothing else: no +# environment fallback, and no smuggling it through to gh-axi after --. +test_override_is_never_inferred() { + local case_dir rc head=7171717171717171717171717171717171717171 + case_dir=$(make_case override-not-inferred) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" "$head" + write_verify_payload "$case_dir/verify.txt" "$head" MERGEABLE '' 0 0 + : > "$case_dir/gh-axi.log" + + set +e + FM_ALLOW_UNVERIFIED=1 ALLOW_UNVERIFIED=1 FM_PR_MERGE_ALLOW_UNVERIFIED=1 \ + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/43 \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "override-not-inferred: an environment variable must not grant the override" + assert_grep 'no check runs exist on this head' "$case_dir/stderr" \ + "override-not-inferred: the environment variable suppressed the refusal" + assert_no_grep 'merge_verification=override' "$case_dir/state/task-x1.meta" \ + "override-not-inferred: an environment variable recorded an override" + assert_no_merge_side_effects "$case_dir" override-not-inferred + + : > "$case_dir/gh-axi.log" + set +e + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/43 -- --allow-unverified \ + > "$case_dir/stdout2" 2> "$case_dir/stderr2" + rc=$? + set -e + + expect_code 1 "$rc" "override-not-inferred: --allow-unverified after -- must not grant the override" + assert_grep 'no check runs exist on this head' "$case_dir/stderr2" \ + "override-not-inferred: a forwarded flag suppressed the refusal" + assert_no_merge_side_effects "$case_dir" override-not-inferred-after-separator + pass "fm-pr-merge grants the override only for the explicit flag, never from the environment or after --" +} + +# --- the real GitHub query, end to end -------------------------------------- + +# The cases above feed fm-pr-merge.sh a canned answer, which proves the branch +# logic but not the query that produces it. Here the mock evaluates the script's +# own -q expression against JSON shaped exactly like the GitHub responses this +# guard was built from, so a query that mis-reads the API is caught too. +# jq is the same filter language gh embeds; the case is skipped without it. +write_rollup_fixture() { + printf '{"headRefOid":"%s","mergeable":"%s","reviewDecision":"%s","statusCheckRollup":%s}\n' \ + "$2" "$3" "$4" "$5" > "$1" +} + +check_runs_json() { + local total=$1 conclusion=$2 i out= + for ((i = 0; i < total; i++)); do + out="$out{\"__typename\":\"CheckRun\",\"status\":\"COMPLETED\",\"conclusion\":\"$conclusion\"}," + done + printf '%s' "$out" +} + +run_fixture_case() { + local name=$1 rollup=$2 mergeable=$3 review=$4 number=$5 expect_rc=$6 expect_msg=$7 + local case_dir rc head=9a9a9a9a9a9a9a9a9a9a9a9a9a9a9a9a9a9a9a9a + case_dir=$(make_case "fixture-$name") + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" "$head" + write_rollup_fixture "$case_dir/pr.json" "$head" "$mergeable" "$review" "$rollup" + : > "$case_dir/gh-axi.log" + + set +e + FM_TEST_GH_FIXTURE="$case_dir/pr.json" \ + run_pr_merge "$case_dir" task-x1 "https://github.com/example/repo/pull/$number" \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code "$expect_rc" "$rc" "fixture-$name: unexpected outcome from the real query" + if [ "$expect_rc" -eq 0 ]; then + grep -qxF "pr merge $number --repo example/repo --squash" "$case_dir/gh-axi.log" \ + || fail "fixture-$name: a green fixture was not merged" + else + assert_grep "$expect_msg" "$case_dir/stderr" \ + "fixture-$name: refusal did not name the expected condition" + assert_no_merge_side_effects "$case_dir" "fixture-$name" + fi +} + +test_real_query_against_api_shaped_json() { + if ! command -v jq >/dev/null 2>&1; then + pass "fm-pr-merge real-query control skipped: jq is not installed" + return 0 + fi + local success_runs + success_runs=$(check_runs_json 10 SUCCESS) + + # An empty rollup: the exact shape the incident PR returned. + run_fixture_case empty-rollup '[]' MERGEABLE '' 51 1 'no check runs exist on this head' + # A null rollup: a head GitHub reports no rollup for at all. + run_fixture_case null-rollup 'null' MERGEABLE '' 52 1 'no check runs exist on this head' + # Ten successful check runs: the same zero failures, but genuinely green. + run_fixture_case all-success "[${success_runs%,}]" MERGEABLE '' 53 0 '' + # One still-running check run among nine passes: not yet an observed pass, and + # not a failure either - it has returned no verdict at all. + run_fixture_case in-progress \ + "[${success_runs%,},{\"__typename\":\"CheckRun\",\"status\":\"IN_PROGRESS\",\"conclusion\":\"\"}]" \ + MERGEABLE '' 54 1 '1 of 11 check runs reported no result' + # A held cross-repo workflow reports ACTION_REQUIRED rather than a pass. This + # is the live shape of the 2026-08-02 defect once GitHub does surface the run. + run_fixture_case action-required \ + '[{"__typename":"CheckRun","status":"COMPLETED","conclusion":"ACTION_REQUIRED"}]' \ + MERGEABLE '' 55 1 '1 of 1 check runs reported no result' + # The brief's named non-verifying conclusions: each ran to completion and + # decided nothing, so a rollup made only of them is not a green rollup. + run_fixture_case only-skipped \ + '[{"__typename":"CheckRun","status":"COMPLETED","conclusion":"SKIPPED"}]' \ + MERGEABLE '' 60 1 '1 of 1 check runs reported no result' + run_fixture_case only-neutral \ + '[{"__typename":"CheckRun","status":"COMPLETED","conclusion":"NEUTRAL"}]' \ + MERGEABLE '' 61 1 '1 of 1 check runs reported no result' + run_fixture_case only-cancelled \ + '[{"__typename":"CheckRun","status":"COMPLETED","conclusion":"CANCELLED"}]' \ + MERGEABLE '' 62 1 '1 of 1 check runs reported no result' + # A whole rollup of non-verifying members alongside real passes still refuses: + # the weakest member decides, and three of these decided nothing. + run_fixture_case skipped-among-passes \ + "[${success_runs%,},{\"__typename\":\"CheckRun\",\"status\":\"COMPLETED\",\"conclusion\":\"SKIPPED\"},{\"__typename\":\"CheckRun\",\"status\":\"COMPLETED\",\"conclusion\":\"NEUTRAL\"},{\"__typename\":\"CheckRun\",\"status\":\"COMPLETED\",\"conclusion\":\"CANCELLED\"}]" \ + MERGEABLE '' 63 1 '3 of 13 check runs reported no result' + # The adverse conclusions, which must read as failures rather than as absence. + run_fixture_case timed-out \ + '[{"__typename":"CheckRun","status":"COMPLETED","conclusion":"TIMED_OUT"}]' \ + MERGEABLE '' 64 1 '1 of 1 check runs failed' + run_fixture_case startup-failure \ + '[{"__typename":"CheckRun","status":"COMPLETED","conclusion":"STARTUP_FAILURE"}]' \ + MERGEABLE '' 65 1 '1 of 1 check runs failed' + # A legacy commit status carries .state instead of .conclusion. + run_fixture_case legacy-status-success \ + '[{"__typename":"StatusContext","context":"ci/legacy","state":"SUCCESS"}]' \ + MERGEABLE '' 56 0 '' + run_fixture_case legacy-status-failure \ + '[{"__typename":"StatusContext","context":"ci/legacy","state":"FAILURE"}]' \ + MERGEABLE '' 57 1 '1 of 1 check runs failed' + run_fixture_case legacy-status-error \ + '[{"__typename":"StatusContext","context":"ci/legacy","state":"ERROR"}]' \ + MERGEABLE '' 66 1 '1 of 1 check runs failed' + run_fixture_case legacy-status-pending \ + '[{"__typename":"StatusContext","context":"ci/legacy","state":"PENDING"}]' \ + MERGEABLE '' 67 1 '1 of 1 check runs reported no result' + # Mergeability and review decision read off the same single response. + run_fixture_case conflicting "[${success_runs%,}]" CONFLICTING '' 58 1 \ + 'the pull request is not mergeable (mergeable=CONFLICTING)' + run_fixture_case changes-requested "[${success_runs%,}]" MERGEABLE CHANGES_REQUESTED 59 1 \ + 'a review requests changes' + pass "fm-pr-merge's own GitHub query reads API-shaped responses correctly, empty rollups included" +} +# --- released tasks (fork landing records) ---------------------------------- +# The captain's parked-completion ruling releases a worker once its PR is green +# and mergeable, which is before the PR lands, so the meta above is gone by the +# time firstmate merges. These cases cover landing such a PR through the same +# sanctioned command, now with merge verification in front of it. + +# A released task's sandbox: a state dir with NO meta, which is what teardown +# leaves behind once that ruling releases a worker. Echoes the case dir. +make_released_case() { + local name=$1 case_dir + case_dir="$TMP_ROOT/$name" + mkdir -p "$case_dir/state" "$case_dir/fakebin" + # Verification runs before the record is resolved, so every released case + # needs a readable head; the cases that must refuse do so on their record. + write_green_payload "$case_dir/verify.txt" + printf '%s\n' "$case_dir" +} + +# Build the durable landing record through the same library entry point +# bin/fm-teardown.sh uses, so the fixture cannot drift from the written format. +# Args: case_dir task_id pr_url head_sha +write_landing_record() { + local case_dir=$1 id=$2 url=$3 head=$4 + fm_pr_landing_record_write "$case_dir/state" "$id" "$url" "$head" "$case_dir/project" \ + || fail "could not build the landing record fixture for $id" +} + +# gh-axi mock as above, plus a gh mock answering the forge view landing identity +# is read from AND the merge-time verification read. +# Args: case_dir head_sha forge_state +add_gh_mocks_forge_state() { + 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" +exit 0 +SH + cat > "$case_dir/fakebin/gh" <> "\$FM_TEST_GH_LOG" +fields= +while [ "\$#" -gt 0 ]; do + case "\$1" in + --json) fields=\${2:-}; shift; [ "\$#" -gt 0 ] && shift ;; + *) shift ;; + esac +done +case "\$fields" in + *statusCheckRollup*) cat "\$FM_TEST_GH_VERIFY_PAYLOAD" ; exit 0 ;; + *"state,headRefOid"*) printf '%s\t%s\n' '$state' '$head' ; exit 0 ;; + *headRefOid*) printf '%s\n' '$head' ; exit 0 ;; +esac +echo "error: pull request not found" >&2 +exit 1 +SH + chmod +x "$case_dir/fakebin/gh-axi" "$case_dir/fakebin/gh" +} + +# gh mock whose pull request lookups all fail, so the forge answers nothing. +add_gh_mocks_forge_unreachable() { + local case_dir=$1 + 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" <<'SH' +#!/usr/bin/env bash +echo "error: pull request not found" >&2 +exit 1 +SH + chmod +x "$case_dir/fakebin/gh-axi" "$case_dir/fakebin/gh" +} +test_landing_record_lands_released_task() { + local case_dir rc + case_dir=$(make_released_case landing-record) + write_landing_record "$case_dir" task-x1 https://github.com/example/repo/pull/20 \ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + add_gh_mocks_forge_state "$case_dir" bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb OPEN + : > "$case_dir/gh-axi.log" + : > "$case_dir/gh.log" + + set +e + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/20 \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 0 "$rc" "landing-record: fm-pr-merge should land a released task's PR" + grep -qxF 'pr merge 20 --repo example/repo --squash' "$case_dir/gh-axi.log" \ + || fail "landing-record: gh-axi pr merge was not invoked for the released task" + assert_absent "$case_dir/state/task-x1.check.sh" \ + "landing-record: a released task has nothing to watch, so no poll should be armed" + assert_absent "$case_dir/state/task-x1.landing" \ + "landing-record: the spent landing record should be removed once the PR landed" + pass "fm-pr-merge lands a released task's PR through its durable landing record" +} + +test_landing_record_refuses_when_pr_is_not_open() { + local case_dir rc + case_dir=$(make_released_case landing-record-merged) + write_landing_record "$case_dir" task-x1 https://github.com/example/repo/pull/20 \ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + # Same record as the case above; only the forge's answer differs, so the local + # record alone can never authorize the merge. + add_gh_mocks_forge_state "$case_dir" bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb MERGED + : > "$case_dir/gh-axi.log" + + set +e + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/20 \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "landing-record-merged: fm-pr-merge should refuse a PR that is not open" + assert_grep 'is merged at its forge' "$case_dir/stderr" \ + "landing-record-merged: refusal did not report the forge's own state" + assert_no_grep 'pr merge' "$case_dir/gh-axi.log" \ + "landing-record-merged: gh-axi pr merge was invoked for a PR that is not open" + assert_present "$case_dir/state/task-x1.landing" \ + "landing-record-merged: a refused merge must not discard the landing record" + pass "fm-pr-merge re-reads the forge and refuses a landing record whose PR is not open" +} + +test_landing_record_refuses_different_requested_pr() { + local case_dir rc recorded_url requested_url + recorded_url=https://github.com/example/repo/pull/20 + requested_url=https://github.com/example/repo/pull/21 + case_dir=$(make_released_case landing-record-mismatch) + write_landing_record "$case_dir" task-x1 "$recorded_url" \ + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + add_gh_mocks_forge_state "$case_dir" bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb OPEN + : > "$case_dir/gh-axi.log" + : > "$case_dir/gh.log" + + set +e + run_pr_merge "$case_dir" task-x1 "$requested_url" \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "landing-record-mismatch: fm-pr-merge should refuse a different PR" + assert_grep "$recorded_url" "$case_dir/stderr" \ + "landing-record-mismatch: refusal did not name the recorded URL" + assert_grep "$requested_url" "$case_dir/stderr" \ + "landing-record-mismatch: refusal did not name the requested URL" + assert_no_grep 'pr merge' "$case_dir/gh-axi.log" \ + "landing-record-mismatch: gh-axi pr merge was invoked" + [ ! -s "$case_dir/gh.log" ] || fail "landing-record-mismatch: the forge was read" + assert_grep "pr=$recorded_url" "$case_dir/state/task-x1.landing" \ + "landing-record-mismatch: the landing record was rebound" + pass "fm-pr-merge refuses a requested PR that differs from the landing record" +} + +test_malformed_landing_record_refuses_without_rebuild() { + local case_dir rc url=https://github.com/example/repo/pull/20 + case_dir=$(make_released_case malformed-landing-record) + printf '%s\n' fm-landing-v1 'pr=not-a-url' > "$case_dir/state/task-x1.landing" + chmod 0600 "$case_dir/state/task-x1.landing" + add_gh_mocks_forge_state "$case_dir" bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb OPEN + : > "$case_dir/gh-axi.log" + : > "$case_dir/gh.log" + + set +e + run_pr_merge "$case_dir" task-x1 "$url" > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "malformed-landing-record: fm-pr-merge should refuse" + assert_grep 'error: task landing record is invalid' "$case_dir/stderr" \ + "malformed-landing-record: refusal did not identify invalid state" + [ ! -s "$case_dir/gh.log" ] || fail "malformed-landing-record: the forge was read" + assert_grep 'pr=not-a-url' "$case_dir/state/task-x1.landing" \ + "malformed-landing-record: malformed state was reconstructed" + pass "fm-pr-merge refuses a malformed landing record without rebuilding it" +} + +test_reconstructs_landing_record_from_pr_url() { + local case_dir rc + case_dir=$(make_released_case reconstruct-from-url) + # Released before landing records existed: no meta and no landing record, so + # the PR itself is the only authority for its own identity. + add_gh_mocks_forge_state "$case_dir" cccccccccccccccccccccccccccccccccccccccc OPEN + : > "$case_dir/gh-axi.log" + + set +e + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/13 \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 0 "$rc" "reconstruct-from-url: fm-pr-merge should rebuild the record and land the PR" + assert_grep 'rebuilt: state/task-x1.landing' "$case_dir/stdout" \ + "reconstruct-from-url: the rebuild was not reported" + grep -qxF 'pr merge 13 --repo example/repo --squash' "$case_dir/gh-axi.log" \ + || fail "reconstruct-from-url: gh-axi pr merge was not invoked" + assert_absent "$case_dir/state/task-x1.check.sh" \ + "reconstruct-from-url: no poll should be armed for a released task" + pass "fm-pr-merge rebuilds a released task's landing record from the PR itself" +} + +test_reconstruction_refuses_when_pr_is_not_open() { + local case_dir rc + case_dir=$(make_released_case reconstruct-closed) + add_gh_mocks_forge_state "$case_dir" dddddddddddddddddddddddddddddddddddddddd CLOSED + : > "$case_dir/gh-axi.log" + + set +e + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/13 \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "reconstruct-closed: fm-pr-merge should refuse to rebuild from a closed PR" + assert_grep 'error: task metadata is unavailable' "$case_dir/stderr" \ + "reconstruct-closed: refusal did not keep the missing-record diagnostic" + assert_no_grep 'pr merge' "$case_dir/gh-axi.log" \ + "reconstruct-closed: gh-axi pr merge was invoked for a closed PR" + assert_absent "$case_dir/state/task-x1.landing" \ + "reconstruct-closed: a closed PR must not leave a landing record behind" + pass "fm-pr-merge refuses to rebuild a landing record from a PR that is not open" +} + + + test_records_pr_and_head_before_merging test_merge_failure_propagates_after_recording test_extra_merge_args_forwarded @@ -543,6 +1233,24 @@ 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_zero_check_runs_refuses +test_all_successful_checks_still_merges +test_failing_check_run_refuses +test_unrun_check_runs_refuse_distinguishably +test_failing_and_unrun_are_both_reported +test_inconsistent_check_buckets_refuse_as_unreadable +test_not_mergeable_refuses +test_unknown_mergeable_refuses +test_changes_requested_refuses +test_approved_review_still_merges +test_unreadable_check_counts_refuse +test_unreadable_forge_state_refuses +test_absent_gh_refuses +test_allow_unverified_merges_and_records_override +test_verified_merge_records_verification +test_final_verification_refuses_changed_head +test_override_is_never_inferred +test_real_query_against_api_shaped_json test_landing_record_lands_released_task test_landing_record_refuses_when_pr_is_not_open test_landing_record_refuses_different_requested_pr