diff --git a/.agents/skills/project-management/SKILL.md b/.agents/skills/project-management/SKILL.md index 86e37422d17..a8848cb7250 100644 --- a/.agents/skills/project-management/SKILL.md +++ b/.agents/skills/project-management/SKILL.md @@ -82,6 +82,13 @@ Initialization configures the local gate and does not vendor a no-mistakes skill Do not create a commit merely because initialization ran. If doctor reports an environment, authentication, or daemon problem, resolve that blocker before dispatching work and never restart the shared daemon from a project operation. +The repository that a pipeline opens its PR against is the clone's own `origin` remote, and `--fork-url` only chooses where branches are pushed while the PR still targets `origin`. +A clone that contributes upstream therefore keeps `origin` on the parent repository on purpose and pairs it with `--fork-url`, which is that posture working as intended and not a target to correct; the paragraph below applies only when a project's registered delivery target is wrong for the repository the fleet is asked to land work in. +For that case the delivery target is changed by repointing that clone's `origin` and running `no-mistakes init` again, which also refreshes the gate mirror to the newly registered target; `no-mistakes status` then reports the new target and keeps it across a daemon restart. +Which repository a given project should deliver into is the captain's decision, so confirm the intended target before repointing anything and follow any contribution workflow the project documents for itself. +Editing the gate mirror's own remote URL is not a supported way to change the target: it leaves the registration and the mirror's tracking refs pointing at the old repository, so work keeps landing in the previous target and a rebase can silently resolve against a base the new target never had. +For an autonomous GitHub task, `bin/fm-pr-check.sh` independently reads live push permission for the URL-derived repository before accepting the resulting PR as ready, so an accidental read-only target is reported instead of looking landable. + ## Remove Project removal is destructive. diff --git a/AGENTS.md b/AGENTS.md index ca09ab7a4cf..3372672d58e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -370,7 +370,7 @@ Require the matching `resolved` event, forbid `--yes`, and require the worker to Resume fleet supervision immediately after the decision lands. Judge validation by the currently attributed run step through `bin/fm-crew-state.sh`, not by shell liveness or the last status event. -Running, fixing, or CI states remain working; parked approval or fix-review states require the worker to follow the active gate help; passed or checks-passed is done; failed or cancelled is failed exactly as `bin/fm-crew-state.sh` prints it - only that state line reclassifies an orphaned ci monitor after green checks as held-for-merge done, or a terminal failed record with the daemon unreachable as unknown, never the raw run record. +Running, fixing, or CI states remain working; parked approval or fix-review states require the worker to follow the active gate help; passed or checks-passed is done; failed or cancelled is failed exactly as `bin/fm-crew-state.sh` prints it - only that state line reclassifies an orphaned ci monitor after green checks as held-for-merge done, a terminal failed record with the daemon unreachable as unknown, or a successful run that skipped every mandatory delivery phase as failed, never the raw run record. A worker hand-editing, committing, aborting, or restarting during an active validation run duplicates pipeline ownership outside the supersession sequence above; steer it back to the gate response flow. The worker reports the PR when CI first becomes green rather than waiting for merge monitoring to finish. diff --git a/bin/fm-crew-state.sh b/bin/fm-crew-state.sh index f3a99c3e3e5..8ca8d77d349 100755 --- a/bin/fm-crew-state.sh +++ b/bin/fm-crew-state.sh @@ -60,7 +60,18 @@ # coarse runs-ledger fallback (no steps table, no ci log), a terminal # FAILED record whose daemon an explicit probe proves down reads unknown, # never failed: an instrument failure must not read as work failure -# (nm_daemon_probe_down). +# (nm_daemon_probe_down). Conversely a terminal SUCCESS run whose steps +# table shows every mandatory delivery phase skipped reads failed, never +# done: when a branch's whole diff vanishes into the rebase base, +# no-mistakes still records the run completed although nothing was +# reviewed, tested, pushed, or opened as a PR, and validity must not be +# inferred from that word alone (nm_run_skipped_every_mandatory_step; +# 2026-09-08 fm-nm-depot-livraison-non-modifiable incident). The coarse +# fallback carries no steps table, and the runs ledger names no run id to +# fetch one with, so its only delivery evidence is the row's own PR URL: +# a COMPLETED row carrying one proves push and pr ran and reads done, +# while a bare COMPLETED row proves nothing either way and reads +# unknown-unverified rather than validated. # 3. Reconcile the status log: if its last line says needs-decision/blocked but # the run-step shows the run moved on, the log is deterministically stale and # is flagged superseded. A genuinely parked run plus a needs-decision log @@ -453,6 +464,43 @@ nm_reclassify_failed_run_as_held_green() { return 0 } +# The delivery phases a no-mistakes ship run must actually execute before its +# result may be read as validated. `intent` and `rebase` are excluded: they +# prepare a run rather than validate or deliver it. +NM_MANDATORY_STEPS="review test document lint push pr ci" + +# 0 when the steps table proves a terminal-success run validated and delivered +# nothing: every phase in NM_MANDATORY_STEPS is present and `skipped`. This is +# the vacuous-pass shape (2026-09-08 fm-nm-depot-livraison-non-modifiable): a +# branch whose commits are already contained in the rebase base loses its whole +# diff, and no-mistakes then logs `empty diff after rebase, skipping remaining +# steps` and still records the run `completed`. Nothing was reviewed, tested, +# documented, linted, pushed, or opened as a PR, so the word alone is not +# validation. Positive evidence is required in both directions: an absent table, +# or any mandatory row that is missing or not `skipped`, is not this shape and +# leaves the run's own reported result untouched. +nm_run_skipped_every_mandatory_step() { + local rows want + rows=$(nm_steps_rows) + [ -n "$rows" ] || return 1 + for want in $NM_MANDATORY_STEPS; do + printf '%s\n' "$rows" \ + | grep -qE "^[[:space:]]*$want,[[:space:]]*\"?skipped\"?[[:space:]]*," || return 1 + done + return 0 +} + +# Reclassify a terminal SUCCESS run as failed when +# nm_run_skipped_every_mandatory_step matches, so a run that lost its change +# never reads as shippable. The branch still holds whatever the worker +# committed; what is refused is calling that outcome validated. +nm_reclassify_vacuous_success_as_failed() { + nm_run_skipped_every_mandatory_step || return 1 + RUN_STATE=failed + RUN_DETAIL="not validated: run kept no change - review, test, document, lint, push, pr and ci were all skipped" + return 0 +} + # 0 when an explicit probe proves the shared daemon down: `no-mistakes daemon # status` is the canonical down-probe (the same one fm-brief.sh hands crews # before a blocked append) and exits non-zero when the daemon is not running. @@ -559,6 +607,7 @@ HAVE_RUN=0 # the TOON field parsing entirely for this crew. RUN_SOURCE=full COARSE_STATUS="" +COARSE_PR="" # Scouts and secondmates never drive a no-mistakes validation of their own # worktree, so skip the lookup for them and read state from pane/log directly. if [ "$KIND" = ship ] && [ -n "$CREW_BRANCH" ] && command -v no-mistakes >/dev/null 2>&1; then @@ -579,7 +628,11 @@ if [ "$KIND" = ship ] && [ -n "$CREW_BRANCH" ] && command -v no-mistakes >/dev/n # `[ -n "$RUN_OUT" ]`: an empty/timed-out primary call means the CLI # itself did not respond, so retrying it immediately with a second # bounded call would just double the wait for no better answer. - COARSE_STATUS=$(fm_nm_runs_status_for_worktree "$WT" "$CREW_BRANCH" "$(nm_runs_list)") + coarse_row=$(fm_nm_runs_status_for_worktree "$WT" "$CREW_BRANCH" "$(nm_runs_list)") + COARSE_STATUS=${coarse_row%% *} + case "$coarse_row" in + *' '*) COARSE_PR=${coarse_row#* } ;; + esac if [ -n "$COARSE_STATUS" ]; then HAVE_RUN=1 # A branch-matching answer the strict rule rejected is this branch's @@ -612,7 +665,22 @@ if [ "$HAVE_RUN" = 1 ]; then # distinction, so a real gate is never silently missed. case "$COARSE_STATUS" in running) RUN_STATE=working; RUN_DETAIL="validating (background run)" ;; - completed) RUN_STATE="done"; RUN_DETAIL="run completed" ;; + completed) + # A terminal ledger word is not a verdict on its own here: the + # vacuous-pass shape the full path refuses + # (nm_run_skipped_every_mandatory_step) is recorded `completed` too, + # and this path has neither a steps table nor a run id to fetch one + # with. The row's PR URL is the ledger's own positive delivery + # evidence - a run that skipped push and pr has none - so a completed + # row that carries one is a real delivery and keeps its done verdict, + # while a bare completed row cannot be told from the vacuous shape + # and is reported unverified rather than validated. + if [ -n "$COARSE_PR" ]; then + RUN_STATE="done"; RUN_DETAIL="run completed: $COARSE_PR" + else + RUN_STATE=unknown + RUN_DETAIL="last ledger record completed with no PR; nothing proves a delivery phase ran - unverified" + fi ;; failed) # The ledger row is terminal but the coarse path has no steps table # and no ci log, so the orphaned-monitor shape cannot be recognized @@ -638,7 +706,10 @@ if [ "$HAVE_RUN" = 1 ]; then if [ -n "$outcome" ]; then case "$outcome" in - passed) RUN_STATE="done"; RUN_DETAIL="run passed: PR merged/closed" ;; + passed) + if nm_reclassify_vacuous_success_as_failed; then :; else + RUN_STATE="done"; RUN_DETAIL="run passed: PR merged/closed" + fi ;; checks-passed) RUN_STATE="done"; RUN_DETAIL="checks green: PR ready for review" ;; failed) if nm_reclassify_failed_run_as_held_green; then :; else @@ -666,7 +737,10 @@ if [ "$HAVE_RUN" = 1 ]; then case "$status" in ci) RUN_STATE=working; RUN_DETAIL="ci running" ;; running|fixing) RUN_STATE=working; RUN_DETAIL="validating ($status)" ;; - completed) RUN_STATE="done"; RUN_DETAIL="run completed" ;; + completed) + if nm_reclassify_vacuous_success_as_failed; then :; else + RUN_STATE="done"; RUN_DETAIL="run completed" + fi ;; failed) if nm_reclassify_failed_run_as_held_green; then :; else RUN_STATE=failed; RUN_DETAIL="run failed" diff --git a/bin/fm-nm-run-lib.sh b/bin/fm-nm-run-lib.sh index 9a04004e391..c6bf2ca7d54 100644 --- a/bin/fm-nm-run-lib.sh +++ b/bin/fm-nm-run-lib.sh @@ -131,8 +131,12 @@ fm_nm_run_is_pipeline_owned_active() { # # --limit N` listing (plain text, no run id, no quoting, newest-first, columns # " []"; the `axi` surface has no # runs-listing subcommand - verified against the installed CLI). Prints the -# status word of the branch's CURRENT run row, or nothing when the ledger -# cannot prove attribution. When optional expected head $4 is supplied, its +# branch's CURRENT run row as "[ ]" - the status word alone +# when the row carries no PR column - or nothing when the ledger cannot prove +# attribution. Callers that only want the word read the first field. The PR +# column is carried because it is the ledger's ONLY positive evidence that a +# row's run actually pushed a branch and opened a PR, which no status word can +# establish on its own. When optional expected head $4 is supplied, its # abbreviated commit identity must match the newest row. The branch's NEWEST # row alone decides; older rows are history and never answer for the present: # - newest row's head resolves and matches the worktree (fm_nm_head_matches_worktree): @@ -205,7 +209,11 @@ fm_nm_runs_status_for_worktree() { # [ex fi if [ -n "$(fm_nm_resolve_commit "$wt" "$sha")" ]; then if fm_nm_head_matches_worktree "$wt" "$sha"; then - printf '%s' "$st" + if [ -n "$pr" ]; then + printf '%s %s' "$st" "$pr" + else + printf '%s' "$st" + fi fi return 0 fi diff --git a/bin/fm-pr-check.sh b/bin/fm-pr-check.sh index 99b3e025db2..8a240dd4453 100755 --- a/bin/fm-pr-check.sh +++ b/bin/fm-pr-check.sh @@ -5,6 +5,11 @@ # live only in a private sidecar and are never interpolated into shell source. # A GitHub pull request URL and a GitLab merge request URL are both accepted, # including a merge request on a self-hosted GitLab instance. +# A GitHub task carrying yolo=on intends Firstmate to land the PR itself, so the +# URL-derived repository must report live push permission before the PR can be +# recorded as ready. A read-only upstream is refused instead of presenting a +# green-looking PR that this home has no authority to land. Tasks with yolo off +# retain the contribution workflow in which an upstream maintainer lands work. # Usage: fm-pr-check.sh set -eu @@ -43,6 +48,38 @@ if [ ! -f "$META" ] || [ -L "$META" ] || [ "$(fm_pr_file_link_count "$META")" != exit 1 fi +github_verify_autonomous_target_writable() { + local yolo permission + [ "$PROVIDER" = github ] || return 0 + yolo=$(grep '^yolo=' "$META" | tail -1 | cut -d= -f2- || true) + [ "$yolo" = on ] || return 0 + command -v gh-axi >/dev/null 2>&1 || { + echo "error: verifying an autonomous GitHub delivery target requires gh-axi on PATH" >&2 + return 1 + } + if ! permission=$(gh-axi api "/repos/$FM_PR_OWNER/$FM_PR_REPO" \ + --jq '.permissions.push' 2>/dev/null); then + printf 'error: could not verify live write permission for autonomous delivery target %s/%s\n' \ + "$FM_PR_OWNER" "$FM_PR_REPO" >&2 + return 1 + fi + case "$permission" in + true) return 0 ;; + false) + printf 'error: refusing autonomous delivery to read-only GitHub repository %s/%s; choose a writable intended target before opening or accepting the PR\n' \ + "$FM_PR_OWNER" "$FM_PR_REPO" >&2 + return 1 + ;; + *) + printf 'error: could not verify live write permission for autonomous delivery target %s/%s\n' \ + "$FM_PR_OWNER" "$FM_PR_REPO" >&2 + return 1 + ;; + esac +} + +github_verify_autonomous_target_writable || exit 1 + # A prior exact merged result may have queued its durable wake immediately # before interruption. # Finish only its identity-bound receipt before publishing a replacement poll. diff --git a/bin/fm-pr-merge.sh b/bin/fm-pr-merge.sh index 3e61b33f7bc..62fba2b44f8 100755 --- a/bin/fm-pr-merge.sh +++ b/bin/fm-pr-merge.sh @@ -6,26 +6,28 @@ # request is addressed through glab by the project URL rebuilt from the parsed # host and path, so any instance works and no host is hardcoded. # -# Merge method on GitHub defaults to --squash when the caller passes none of -# --squash, --merge, --rebase, or --method after the optional -- separator. -# The gh-axi merge abstraction always performs the merge; the outcome read that -# follows it never becomes a prerequisite for reaching that abstraction. After -# gh-axi returns success, GitHub's live state is read back and accepted only -# when the pull request is merged or in the merge queue. gh's GraphQL API -# supplies that queue-aware read when gh is on PATH; when gh is absent or its -# read fails, gh-axi's own view still proves a landed merge, and every outcome -# it cannot prove refuses, reporting the single failed read when gh is absent -# and naming both failed reads when gh is present and its own read failed. -# If the pull request remains open and the base branch has an effective -# merge_queue rule, the refusal names the queue's configured merge method and -# the exact -- --auto -- retry flags, unless the caller already passed -# that method with --auto to a merge command that returned success, in which -# case it reports instead that the accepted request has not entered the queue -# and the queue state has to be re-checked. -# No method is selected for the caller in any case. A rules response that names -# no queue rule, one that could not be read, rules that disagree, and a method -# this script does not recognise are four distinct outcomes and are reported -# apart, because each one leaves the operator somewhere different. +# GitHub direct merges default to --squash when the caller names no method. +# `--queue` is the only merge-queue request syntax and invokes GitHub's supported +# GraphQL enqueuePullRequest mutation rather than the gh-axi merge parser. +# One parser owns queue syntax and all caller-argument interpretation. +# It refuses repeated queue tokens, a queue token mixed with an explicit merge +# strategy, and any extra argument the enqueue mutation cannot honour. +# Before enqueue, one live GraphQL read proves the URL-derived repository is +# writable, the pull request is open, its status-check rollup is not red, and +# its head still matches the head fm-pr-check recorded. +# enqueuePullRequest receives that same head as expectedHeadOid, so a later push +# makes the mutation fail instead of queueing a changed identity. +# The base must have an effective merge_queue rule, and a successful mutation +# must return a queue entry before an independent live read confirms membership. +# A direct merge that discovers a queue-governed base prints one runnable +# `--queue` retry, unless the caller already supplied any accepted queue token. +# Rules that are absent, unreadable, conflicting, or unrecognised remain +# distinct outcomes, but none can produce a retry the parser rejects. +# The gh-axi merge abstraction still owns non-queue GitHub merges. +# After it returns, GitHub's live state is read back and accepted only when the +# pull request is merged or in the merge queue. +# gh's GraphQL API supplies the queue-aware read when gh is on PATH; when gh is +# absent or its read fails, gh-axi's own view can still prove a landed merge. # A caller-requested --auto that leaves the pull request neither merged nor # queued is refused the same way and says auto-merge was armed with nothing # landed or queued yet, or, when the merge command itself failed, that auto-merge @@ -40,6 +42,7 @@ # GitLab adds no method flag at all: its merge method is the project's own # setting, which the merge API applies, and imposing squash there would override # that convention rather than mirror the GitHub default. +# Queue tokens are refused up front on GitLab, where no glab flag spells them. # # A GitLab merge is refused unless every pre-merge condition holds, each read # live at merge time rather than taken from recorded metadata: the merge request @@ -103,55 +106,105 @@ PROJECT_URL="https://$FM_PR_HOST/$FM_PR_PATH" shift 2 [ "${1:-}" = "--" ] && shift -caller_has_merge_method() { - local arg - for arg in "$@"; do - case "$arg" in - --squash|--merge|--rebase|--method|--method=*) return 0 ;; - esac - done - return 1 -} - -# The merge method the caller's own extra arguments named, in the --flag, -# --method and --method= forms caller_has_merge_method accepts. -caller_merge_method() { - local arg method='' pending=false - for arg in "$@"; do - if [ "$pending" = true ]; then - method=$arg - pending=false +# Parse the caller's merge arguments once for every provider path. +# This is the single owner of queue-token grammar and of how caller arguments +# are interpreted. Queue tokens are Firstmate flags, +# never forge CLI flags. A repeated queue token, a queue token mixed with an +# explicit strategy, or an argument the GraphQL enqueue path cannot honour is +# refused rather than dropped or forwarded with changed meaning. +FM_PR_CALLER_HAS_METHOD=false +FM_PR_CALLER_METHOD= +FM_PR_CALLER_AUTO=false +FM_PR_CALLER_QUEUE=false +FM_PR_CALLER_QUEUE_COUNT=0 +FM_PR_CALLER_QUEUE_TOKENS= +FM_PR_CALLER_EXPLICIT_METHODS= +FM_PR_CALLER_FORWARD=() +fm_pr_parse_merge_args() { + local arg pending_method=false unsupported='' + FM_PR_CALLER_HAS_METHOD=false + FM_PR_CALLER_METHOD= + FM_PR_CALLER_AUTO=false + FM_PR_CALLER_QUEUE=false + FM_PR_CALLER_QUEUE_COUNT=0 + FM_PR_CALLER_QUEUE_TOKENS= + FM_PR_CALLER_EXPLICIT_METHODS= + FM_PR_CALLER_FORWARD=() + + while [ "$#" -gt 0 ]; do + arg=$1 + shift + if [ "$pending_method" = true ]; then + FM_PR_CALLER_METHOD=$arg + FM_PR_CALLER_EXPLICIT_METHODS="${FM_PR_CALLER_EXPLICIT_METHODS:+$FM_PR_CALLER_EXPLICIT_METHODS }--method $arg" + FM_PR_CALLER_FORWARD+=(--method "$arg") + pending_method=false continue fi case "$arg" in - --squash) method=squash ;; - --merge) method=merge ;; - --rebase) method=rebase ;; - --method) pending=true ;; - --method=*) method=${arg#--method=} ;; - esac - done - printf '%s' "$method" -} - -# Whether the caller's own extra arguments asked for auto-merge, including the -# --flag=value spelling the forge's flag parser accepts. --disable-auto cancels -# the request, and gh exposes no short option that could bundle either flag. -caller_requested_auto_merge() { - local arg requested=1 - for arg in "$@"; do - case "$arg" in - --auto) requested=0 ;; + --queue) + FM_PR_CALLER_HAS_METHOD=true + FM_PR_CALLER_METHOD=queue + FM_PR_CALLER_QUEUE=true + FM_PR_CALLER_QUEUE_COUNT=$((FM_PR_CALLER_QUEUE_COUNT + 1)) + FM_PR_CALLER_QUEUE_TOKENS="${FM_PR_CALLER_QUEUE_TOKENS:+$FM_PR_CALLER_QUEUE_TOKENS }$arg" + ;; + --method) + FM_PR_CALLER_HAS_METHOD=true + pending_method=true + ;; + --squash|--merge|--rebase|--method=*) + FM_PR_CALLER_HAS_METHOD=true + FM_PR_CALLER_METHOD=${arg#--} + FM_PR_CALLER_METHOD=${FM_PR_CALLER_METHOD#method=} + FM_PR_CALLER_EXPLICIT_METHODS="${FM_PR_CALLER_EXPLICIT_METHODS:+$FM_PR_CALLER_EXPLICIT_METHODS }$arg" + FM_PR_CALLER_FORWARD+=("$arg") + ;; + --auto) + FM_PR_CALLER_AUTO=true + FM_PR_CALLER_FORWARD+=("$arg") + ;; --auto=*) case "${arg#--auto=}" in - [tT]|[tT][rR][uU][eE]|1) requested=0 ;; - *) requested=1 ;; + [tT]|[tT][rR][uU][eE]|1) FM_PR_CALLER_AUTO=true ;; + *) FM_PR_CALLER_AUTO=false ;; esac + FM_PR_CALLER_FORWARD+=("$arg") + ;; + --disable-auto) + FM_PR_CALLER_AUTO=false + FM_PR_CALLER_FORWARD+=("$arg") ;; - --disable-auto) requested=1 ;; + *) FM_PR_CALLER_FORWARD+=("$arg") ;; esac done - return "$requested" + if [ "$pending_method" = true ]; then + FM_PR_CALLER_EXPLICIT_METHODS="${FM_PR_CALLER_EXPLICIT_METHODS:+$FM_PR_CALLER_EXPLICIT_METHODS }--method" + FM_PR_CALLER_FORWARD+=(--method) + fi + + if [ "$FM_PR_CALLER_QUEUE_COUNT" -gt 1 ]; then + printf 'error: extra merge arguments repeat the queue request (%s); pass exactly one queue token\n' \ + "$FM_PR_CALLER_QUEUE_TOKENS" >&2 + return 1 + fi + if [ "$FM_PR_CALLER_QUEUE" = true ] && [ -n "$FM_PR_CALLER_EXPLICIT_METHODS" ]; then + printf 'error: extra merge arguments must not combine a queue request (%s) with an explicit merge strategy (%s)\n' \ + "$FM_PR_CALLER_QUEUE_TOKENS" "$FM_PR_CALLER_EXPLICIT_METHODS" >&2 + return 1 + fi + if [ "$FM_PR_CALLER_QUEUE" = true ]; then + for arg in "${FM_PR_CALLER_FORWARD[@]+"${FM_PR_CALLER_FORWARD[@]}"}"; do + case "$arg" in + --auto|--auto=[tT]|--auto=[tT][rR][uU][eE]|--auto=1) ;; + *) unsupported="${unsupported:+$unsupported }$arg" ;; + esac + done + if [ -n "$unsupported" ]; then + printf 'error: queue enqueue does not support extra merge arguments (%s)\n' "$unsupported" >&2 + return 1 + fi + fi } reject_repo_overrides() { @@ -186,7 +239,12 @@ reject_head_overrides() { } reject_repo_overrides "$@" || exit 1 +fm_pr_parse_merge_args "$@" || exit 1 [ "$PROVIDER" != gitlab ] || reject_head_overrides "$@" || exit 1 +if [ "$PROVIDER" = gitlab ] && [ "$FM_PR_CALLER_QUEUE" = true ]; then + echo "error: extra merge arguments must not request GitHub's merge queue on GitLab" >&2 + exit 1 +fi # Task-derived paths are constructed only after the canonical ID validation. META="$STATE/$ID.meta" @@ -492,6 +550,134 @@ METHODS fi } +FM_PR_GITHUB_NODE_ID= +FM_PR_GITHUB_HEAD= + +# Read the exact PR identity and repository authority immediately before a +# queue enqueue. The head is also passed to enqueuePullRequest as +# expectedHeadOid, so a push after this read makes the mutation refuse rather +# than queueing code this run did not inspect. A red rollup is refused before +# enqueue, and repository READ permission cannot masquerade as merge authority. +github_read_enqueue_preflight() { + local fields line recorded_head + local total=0 named=0 state='' merged='' queued='' base='' + local node='' head='' checks='' permission='' + command -v gh >/dev/null 2>&1 || { + echo "error: enqueueing a GitHub pull request requires gh on PATH" >&2 + return 1 + } + # shellcheck disable=SC2016 # GraphQL variables are literal query syntax. + if ! fields=$(gh api graphql \ + -f query='query($owner:String!,$repo:String!,$number:Int!){repository(owner:$owner,name:$repo){viewerPermission pullRequest(number:$number){id state merged isInMergeQueue baseRefName headRefOid commits(last:1){nodes{commit{statusCheckRollup{state}}}}}}}' \ + -F "owner=$PR_OWNER" -F "repo=$PR_REPO" -F "number=$PR_NUMBER" \ + --jq '.data.repository as $r | $r.pullRequest | "state=" + (.state // ""), "merged=" + (.merged | tostring), "queued=" + (.isInMergeQueue | tostring), "base=" + (.baseRefName // ""), "node=" + (.id // ""), "head=" + (.headRefOid // ""), "checks=" + (.commits.nodes[0].commit.statusCheckRollup.state // "NONE"), "permission=" + ($r.viewerPermission // "")' \ + 2>/dev/null) || [ -z "$fields" ]; then + echo "error: could not read the GitHub pull request identity and merge authority before enqueueing" >&2 + return 1 + fi + while IFS= read -r line; do + total=$((total + 1)) + case "$line" in + state=*) state=${line#state=} ;; + merged=*) merged=${line#merged=} ;; + queued=*) queued=${line#queued=} ;; + base=*) base=${line#base=} ;; + node=*) node=${line#node=} ;; + head=*) head=${line#head=} ;; + checks=*) checks=${line#checks=} ;; + permission=*) permission=${line#permission=} ;; + *) continue ;; + esac + named=$((named + 1)) + done <&2 + return 1 + fi + case "$permission" in + WRITE|MAINTAIN|ADMIN) ;; + *) + printf 'error: refusing to enqueue %s: repository %s/%s grants only %s permission, not merge authority\n' \ + "$URL" "$PR_OWNER" "$PR_REPO" "${permission:-unreadable}" >&2 + return 1 + ;; + esac + case "$checks" in + FAILURE|ERROR) + printf 'error: refusing to enqueue %s: the current head %s has a red status-check rollup (%s)\n' \ + "$URL" "$head" "$checks" >&2 + return 1 + ;; + SUCCESS|PENDING|EXPECTED|NONE) ;; + *) + printf 'error: refusing to enqueue %s: the status-check rollup is unreadable (%s)\n' \ + "$URL" "${checks:-empty}" >&2 + return 1 + ;; + esac + recorded_head=$(grep '^pr_head=' "$META" | tail -1 | cut -d= -f2- || true) + if [ -n "$recorded_head" ] && [ "$recorded_head" != "$head" ]; then + printf 'error: refusing to enqueue %s: recorded head %s changed to %s before the queue request\n' \ + "$URL" "$recorded_head" "$head" >&2 + return 1 + fi + FM_PR_GITHUB_STATE=$state + FM_PR_GITHUB_MERGED=$merged + FM_PR_GITHUB_QUEUED=$queued + FM_PR_GITHUB_BASE=$base + FM_PR_GITHUB_QUEUE_OBSERVED=true + FM_PR_GITHUB_NODE_ID=$node + FM_PR_GITHUB_HEAD=$head +} + +# Invoke GitHub's supported queue operation. The mutation result must name a +# queue entry, then the ordinary live outcome read independently confirms queue +# membership. The expected head preserves the same changed-identity boundary +# that direct merges obtain from the forge CLI. +github_enqueue_pull_request() { + local fields line entry_id='' entry_state='' total=0 named=0 + # shellcheck disable=SC2016 # GraphQL variables are literal query syntax. + if ! fields=$(gh api graphql \ + -f query='mutation($pullRequestId:ID!,$expectedHeadOid:GitObjectID!){enqueuePullRequest(input:{pullRequestId:$pullRequestId,expectedHeadOid:$expectedHeadOid}){mergeQueueEntry{id state}}}' \ + -f "pullRequestId=$FM_PR_GITHUB_NODE_ID" \ + -f "expectedHeadOid=$FM_PR_GITHUB_HEAD" \ + --jq '.data.enqueuePullRequest.mergeQueueEntry | "entry_id=" + (.id // ""), "entry_state=" + (.state // "")' \ + 2>&1); then + [ -z "$fields" ] || printf '%s\n' "$fields" >&2 + echo "error: GitHub rejected enqueuePullRequest; nothing was reported as queued" >&2 + return 1 + fi + while IFS= read -r line; do + total=$((total + 1)) + case "$line" in + entry_id=*) entry_id=${line#entry_id=} ;; + entry_state=*) entry_state=${line#entry_state=} ;; + *) continue ;; + esac + named=$((named + 1)) + done <&2 + return 1 + fi + case "$entry_state" in + QUEUED|AWAITING_CHECKS|MERGEABLE|UNMERGEABLE|LOCKED) ;; + *) + printf 'error: enqueuePullRequest returned an unknown merge-queue state (%s)\n' \ + "${entry_state:-empty}" >&2 + return 1 + ;; + esac +} + record_pr_metadata() { if ! "$SCRIPT_DIR/fm-pr-check.sh" "$ID" "$URL"; then return 1 @@ -502,9 +688,8 @@ record_pr_metadata() { } } -FM_PR_GITHUB_AUTO_REQUESTED=false +FM_PR_GITHUB_AUTO_REQUESTED=$FM_PR_CALLER_AUTO FM_PR_GITHUB_MERGE_ACCEPTED=false -FM_PR_GITHUB_CALLER_METHOD= # The single gate every statement about what the forge accepted, armed, or # reported has to pass. A merge command that failed accepted nothing, so no @@ -533,19 +718,21 @@ github_state_is_open() { esac } -# Whether the caller's own named method is the one the queue is configured for, -# compared without regard to the spelling either side happens to use. -github_caller_method_is() { - case "$FM_PR_GITHUB_CALLER_METHOD" in - [mM][eE][rR][gG][eE]) [ "$1" = merge ] ;; - [sS][qQ][uU][aA][sS][hH]) [ "$1" = squash ] ;; - [rR][eE][bB][aA][sS][eE]) [ "$1" = rebase ] ;; - *) return 1 ;; - esac +# The one retry a queue-governed base accepts. `--queue` is parsed by this +# script and invokes enqueuePullRequest directly, so the command names neither +# a merge strategy nor gh-axi's unrelated auto-merge flag. +github_queue_retry_command() { + printf '%s %s %s -- --queue' "$0" "$ID" "$URL" +} + +# A caller who supplied --queue already requested the operation the retry +# would perform. Report the blocking cause instead of repeating that request. +github_caller_already_ran_queue_retry() { + [ "$FM_PR_CALLER_QUEUE" = true ] } github_report_queue_rules() { - local queue_method methods_display + local queue_method methods_display situation github_read_queue_method case "$FM_PR_GITHUB_QUEUE_STATUS" in single) @@ -554,31 +741,37 @@ github_report_queue_rules() { SQUASH) queue_method=squash ;; REBASE) queue_method=rebase ;; esac - if github_merge_command_succeeded \ - && [ "$FM_PR_GITHUB_AUTO_REQUESTED" = true ] \ - && github_caller_method_is "$queue_method"; then - printf 'error: this run refuses even though the request for %s was accepted with the exact flags base branch %s requires (--auto --%s): the pull request has still not entered the merge queue, so no landed or queued outcome is proven; re-check the pull request'"'"'s merge queue state before retrying\n' \ - "$URL" "$FM_PR_GITHUB_BASE" "$queue_method" >&2 - else - printf 'error: base branch %s requires the merge queue; retry with: %s %s %s -- --auto --%s\n' \ - "$FM_PR_GITHUB_BASE" "$0" "$ID" "$URL" "$queue_method" >&2 - fi + printf -v situation \ + 'base branch %s requires the merge queue, which sets the merge method (%s) itself and refuses an explicit strategy' \ + "$FM_PR_GITHUB_BASE" "$queue_method" ;; conflicting) - printf 'error: base branch %s has conflicting merge queue methods (%s); exact retry flags are ambiguous\n' \ - "$FM_PR_GITHUB_BASE" "${FM_PR_GITHUB_QUEUE_METHODS//,/, }" >&2 + printf -v situation \ + 'base branch %s has conflicting merge queue methods (%s), so which one it would apply is ambiguous; the merge queue applies its own without being told' \ + "$FM_PR_GITHUB_BASE" "${FM_PR_GITHUB_QUEUE_METHODS//,/, }" ;; unrecognised) methods_display=${FM_PR_GITHUB_QUEUE_METHODS//,/, } [ -n "$methods_display" ] || methods_display='' - printf 'error: base branch %s requires the merge queue, but its configured merge method (%s) is not one this script recognises, so exact retry flags cannot be named\n' \ - "$FM_PR_GITHUB_BASE" "$methods_display" >&2 + printf -v situation \ + 'base branch %s requires the merge queue, but its configured merge method (%s) is not one this script recognises; the merge queue applies its own without being told' \ + "$FM_PR_GITHUB_BASE" "$methods_display" ;; unreadable) printf 'error: the branch rules for base branch %s could not be read, so a merge queue requirement can be neither confirmed nor ruled out here\n' \ "${FM_PR_GITHUB_BASE:-}" >&2 + return 0 + ;; + *) + return 0 ;; esac + if github_caller_already_ran_queue_retry; then + printf 'error: %s; enqueuePullRequest is the one supported queue operation and this run already requested it, so no different retry exists to name: the outcome reported above for %s is the blocking cause, and re-check the pull request'"'"'s merge queue state before running the same command again\n' \ + "$situation" "$URL" >&2 + else + printf 'error: %s; retry with: %s\n' "$situation" "$(github_queue_retry_command)" >&2 + fi } github_report_unmerged_outcome() { @@ -632,28 +825,64 @@ case "$PROVIDER" in github) merge_output= merge_args=() - if ! caller_has_merge_method "$@"; then - merge_args=(--squash) - fi - if caller_requested_auto_merge "$@"; then - FM_PR_GITHUB_AUTO_REQUESTED=true - fi - FM_PR_GITHUB_CALLER_METHOD=$(caller_merge_method "$@") - if merge_output=$(gh-axi pr merge "$PR_NUMBER" --repo "$PR_OWNER/$PR_REPO" \ - "${merge_args[@]+"${merge_args[@]}"}" "$@" 2>&1); then - FM_PR_GITHUB_MERGE_ACCEPTED=true + if [ "$FM_PR_CALLER_QUEUE" = true ]; then + github_read_enqueue_preflight || exit 1 + if [ "$FM_PR_GITHUB_MERGED" = true ]; then + : # The ordinary outcome path below records the already-landed result. + elif [ "$FM_PR_GITHUB_QUEUED" = true ]; then + printf 'verified: %s is already queued (state=%s, merged=%s, isInMergeQueue=%s)\n' \ + "$URL" "$FM_PR_GITHUB_STATE" "$FM_PR_GITHUB_MERGED" "$FM_PR_GITHUB_QUEUED" + exit 0 + else + github_read_queue_method + case "$FM_PR_GITHUB_QUEUE_STATUS" in + single|conflicting|unrecognised) ;; + none) + printf 'error: refusing to enqueue %s: base branch %s has no effective merge-queue rule\n' \ + "$URL" "$FM_PR_GITHUB_BASE" >&2 + exit 1 + ;; + *) + printf 'error: refusing to enqueue %s: the merge-queue rule for base branch %s could not be read\n' \ + "$URL" "$FM_PR_GITHUB_BASE" >&2 + exit 1 + ;; + esac + enqueue_status=0 + github_enqueue_pull_request || enqueue_status=$? + if [ "$enqueue_status" -ne 0 ]; then + if github_read_outcome; then + if [ "$FM_PR_GITHUB_QUEUED" = true ]; then + printf 'actionable: enqueuePullRequest for %s failed, but the pull request now reads as queued\n' \ + "$URL" >&2 + else + github_report_unmerged_outcome + fi + fi + exit "$enqueue_status" + fi + fi else - merge_status=$? - [ -z "$merge_output" ] || printf '%s\n' "$merge_output" >&2 - if github_read_outcome; then - if [ "$FM_PR_GITHUB_MERGED" != true ] && [ "$FM_PR_GITHUB_QUEUED" != true ]; then - github_report_unmerged_outcome - else - printf 'actionable: the merge command for %s failed, but the pull request reads back as state=%s, merged=%s, isInMergeQueue=%s\n' \ - "$URL" "$FM_PR_GITHUB_STATE" "$FM_PR_GITHUB_MERGED" "$FM_PR_GITHUB_QUEUED" >&2 + if [ "$FM_PR_CALLER_HAS_METHOD" != true ]; then + merge_args=(--squash) + fi + if merge_output=$(gh-axi pr merge "$PR_NUMBER" --repo "$PR_OWNER/$PR_REPO" \ + "${merge_args[@]+"${merge_args[@]}"}" \ + "${FM_PR_CALLER_FORWARD[@]+"${FM_PR_CALLER_FORWARD[@]}"}" 2>&1); then + FM_PR_GITHUB_MERGE_ACCEPTED=true + else + merge_status=$? + [ -z "$merge_output" ] || printf '%s\n' "$merge_output" >&2 + if github_read_outcome; then + if [ "$FM_PR_GITHUB_MERGED" != true ] && [ "$FM_PR_GITHUB_QUEUED" != true ]; then + github_report_unmerged_outcome + else + printf 'actionable: the merge command for %s failed, but the pull request reads back as state=%s, merged=%s, isInMergeQueue=%s\n' \ + "$URL" "$FM_PR_GITHUB_STATE" "$FM_PR_GITHUB_MERGED" "$FM_PR_GITHUB_QUEUED" >&2 + fi fi + exit "$merge_status" fi - exit "$merge_status" fi if ! github_read_outcome; then github_report_forge_output "$merge_output" diff --git a/bin/fm-pr-poll.sh b/bin/fm-pr-poll.sh index ed705ce7073..7db2de996b0 100755 --- a/bin/fm-pr-poll.sh +++ b/bin/fm-pr-poll.sh @@ -2,7 +2,9 @@ # Static watcher program for a validated PR/MR poll sidecar. # It emits exactly one merged line for a merged PR or MR and stays silent # otherwise, including on every error, so a failed lookup can never be read as -# a merge. The provider-tagged identity is data in the sidecar and is never +# a merge. A GitHub pull request in a merge queue stays OPEN until it lands, +# so this poll waits; queue membership is not a merge. The provider-tagged +# identity is data in the sidecar and is never # interpolated into this source: these bytes are identical for every task. # Each provider is read through its own standard CLI, gh for GitHub and glab # for GitLab, so an upstream checkout needs no extra tooling to follow either. diff --git a/bin/fm-teardown.sh b/bin/fm-teardown.sh index fd2db419806..d4de04bc405 100755 --- a/bin/fm-teardown.sh +++ b/bin/fm-teardown.sh @@ -1321,6 +1321,7 @@ pr_is_merged() { resolved_url=${remainder#*$'\t'} [ "$head" != "$remainder" ] || return 1 case "$state" in + # OPEN includes a merge-queue enqueue: landing is MERGED only. MERGED|merged) ;; *) return 1 ;; esac @@ -1718,7 +1719,7 @@ NM_TEARDOWN_RUNS_LIMIT=${FM_TEARDOWN_NM_RUNS_LIMIT:-200} case "$NM_TEARDOWN_RUNS_LIMIT" in ''|*[!0-9]*) NM_TEARDOWN_RUNS_LIMIT=200 ;; esac TASK_RUN_ID= task_status_is_own_parked_run() { # - local wt=$1 out=$2 branch run_id run_branch run_head status outcome awaiting has_gate ledger + local wt=$1 out=$2 branch run_id run_branch run_head status outcome awaiting has_gate ledger ledger_row TASK_RUN_ID= branch=$(git -C "$wt" symbolic-ref --quiet --short HEAD 2>/dev/null) || return 1 [ -n "$branch" ] || return 1 @@ -1751,7 +1752,8 @@ task_status_is_own_parked_run() { # [ -n "$run_head" ] || return 1 [ -z "$(fm_nm_resolve_commit "$wt" "$run_head")" ] || return 1 ledger=$(fm_nm_run "$wt" "$NM_TEARDOWN_TIMEOUT" runs --limit "$NM_TEARDOWN_RUNS_LIMIT") - [ "$(fm_nm_runs_status_for_worktree "$wt" "$branch" "$ledger" "$run_head")" = running ] || return 1 + ledger_row=$(fm_nm_runs_status_for_worktree "$wt" "$branch" "$ledger" "$run_head") + [ "${ledger_row%% *}" = running ] || return 1 fi awaiting=$(printf '%s\n' "$out" | grep -E '^[[:space:]]*awaiting_agent:' | head -1 || true) has_gate=$(printf '%s\n' "$out" | grep -Eq '^[[:space:]]*gate:[[:space:]]*' && echo 1 || echo 0) diff --git a/docs/architecture.md b/docs/architecture.md index 58b900786d4..32d943205c5 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -82,7 +82,9 @@ A run head the task copy cannot resolve locally is attributed only when the pipe During no-mistakes' `ci` monitor phase, it also reads the ci step log tail because `axi status` reports both "still waiting on checks" and "checks green, waiting on merge" as `ci,running`. The most recent recognized ci log marker wins, so checks-green monitoring reports done while a later re-arm, failed-check, or issue marker returns the crew to working. A terminal failed run whose only failure is the ci monitor step, after every substantive step completed and the same marker reads checks green, also reports done with the run's PR URL, because a monitor whose only remaining job is to observe a human merge decision must not convert the absence of that decision into a failure verdict. +Conversely a terminal successful run whose steps table shows every mandatory delivery phase - review, test, document, lint, push, pr, and ci - skipped reports failed rather than done, because a branch whose whole diff vanished into the rebase base is still recorded completed although nothing was reviewed, tested, or delivered, and that word alone is not validation. In the coarse runs-ledger fallback, which has no steps table and no ci log, a terminal failed record whose daemon an explicit `daemon status` probe proves down reports unknown as unverified instead: an instrument failure must never read as work failure. +That fallback equally cannot inspect a completed record's phases, so the ledger row's own PR URL is its only positive delivery evidence: a completed row carrying one reports done, while a bare completed row reports unknown as unverified rather than validated. Only when no matching run exists does it consult semantic busy state; exact busy reports working, exact idle permits fallback to a status-log event whose verb maps to a recognized run-state, and unknown or a dead pane stays unknown instead of trusting a stale log. Decision-only events such as `resolved` never become current state or leak their prose into the current-state detail. In that status-log fallback, a declared external wait reports the distinct `paused` state with its reason. @@ -303,15 +305,19 @@ It is also the one owner of the no-mistakes `--intent` contract those workers fo When a selected delivery path calls for a diff, `bin/fm-review-diff.sh` refreshes the authoritative base and, when task meta records `pr=`, always fetches and compares against `refs/pull//head` by default (recorded `pr_head=` is only an offline fallback) before falling back to the local branch with a warning. Where a no-mistakes pipeline stores evidence in the repo, it publishes that PR-viewable validation evidence to an orphan evidence branch that shares no history with code branches, so it never enters the crew branch or the default branch. This repo uses that setting, and its own `.no-mistakes/` directory remains local state that stays gitignored and is rejected by CI if tracked; [`configuration.md`](configuration.md) owns the setting. -PR-based task merges go through `bin/fm-pr-merge.sh`, which records `pr=` and any available `pr_head=` through `bin/fm-pr-check.sh` before calling the forge CLI. +PR-based task merges go through `bin/fm-pr-merge.sh`, which records `pr=` and any available `pr_head=` through `bin/fm-pr-check.sh` before calling the forge. The helper requires a full canonical URL and rejects malformed URLs or repo override flags before recording merge state. -A `https://github.com///pull/` URL invokes `gh-axi pr merge --repo /`, defaults to `--squash`, and preserves explicit merge-method flags. +When task metadata carries `yolo=on`, `bin/fm-pr-check.sh` derives the GitHub repository from that canonical URL and requires live push permission before accepting the PR as ready, so a read-only upstream cannot look like an autonomously landable delivery; `yolo=off` contribution workflows remain unchanged. +A normal `https://github.com///pull/` URL invokes `gh-axi pr merge --repo /` with the merge-method rules owned by that helper's header. +Queue requests instead use GitHub's GraphQL `enqueuePullRequest` operation under the argument and preflight contract in `bin/fm-pr-merge.sh`'s header. +The mutation is bound to the verified head through `expectedHeadOid`, and its returned queue entry is followed by an independent live queue-membership read. +A GitHub merge-queue enqueue leaves the pull request open until it lands, so landing stays confirmed by the merge poll and teardown, which wait for a merged state, with empirical queue-state evidence in [`docs/verification/github-merge-queue.md`](verification/github-merge-queue.md). A `https:////-/merge_requests/` URL (see [docs/gitlab-merge-watch.md](gitlab-merge-watch.md)) invokes `glab mr merge -R https:///`, so the instance comes from the URL, and adds no merge-method flag because the project's own merge method applies. That path merges only after one live read of the merge request confirms it is open, mergeable, conflict-free, with blocking discussions resolved and a successful pipeline at the current head, and it binds the merge to that verified head; recorded metadata is never the authority for those conditions because a rebase leaves it stale. After either forge command returns, the script confirms the PR or MR actually landed, and only a confirmed landing records a landed outcome; a queued or unconfirmed request records none and leaves its poll armed. On GitLab an auto-merge-queued or unconfirmed request is reported without failing the run. -On GitHub an outcome that is neither merged nor queued is refused loudly and non-zero, naming the observed state, and a base branch that requires the merge queue is refused with the concrete retry flags its configured method requires rather than having a merge method chosen on the caller's behalf. -When the forge already accepted exactly those flags and the pull request still has not entered the queue, that refusal points at the queue state to re-check instead of echoing back the flags the caller just ran. +On GitHub an outcome that is neither merged nor queued is refused loudly and non-zero, naming the observed state, and a base branch that requires the merge queue is refused with the concrete `--queue` retry rather than a merge strategy the queue rejects. +The helper's parser owns caller-argument interpretation and retry behavior; its header documents the supported queue syntax. An auto-merge request is held to the same standard: `--auto` that leaves the pull request neither merged nor queued is refused rather than reported as success. Every GitHub refusal states what it could not observe as plainly as what it did, so an unreadable branch-rule response, an unrecognised queue method, and a merge queue no available read can see are each named rather than left to look like a base branch with no queue at all. A confirmed merge leaves a durable role-routed outcome instead of living only in the merging agent's memory, and [`bin/fm-merge-outcome-lib.sh`](../bin/fm-merge-outcome-lib.sh)'s header owns its destination, shape, identity, normal-case deduplication, and at-least-once recovery. diff --git a/docs/documentation-audiences.json b/docs/documentation-audiences.json index 4bd14979314..0768750b2c7 100644 --- a/docs/documentation-audiences.json +++ b/docs/documentation-audiences.json @@ -436,6 +436,10 @@ "path": "docs/verification/dispatch-auth.md", "audience": "maintainer-verification" }, + { + "path": "docs/verification/github-merge-queue.md", + "audience": "maintainer-verification" + }, { "path": "docs/verification/grok-interrupt.md", "audience": "maintainer-verification" diff --git a/docs/verification/github-merge-queue.md b/docs/verification/github-merge-queue.md new file mode 100644 index 00000000000..da15b6aa2ff --- /dev/null +++ b/docs/verification/github-merge-queue.md @@ -0,0 +1,100 @@ +# GitHub merge-queue verification + +This record supports the current guarantee that `bin/fm-pr-merge.sh` uses GitHub's GraphQL `enqueuePullRequest` operation for a queue request and never treats queue membership as a landing. +The helper's header owns queue-token grammar, caller-argument interpretation, preconditions, and retry behavior. +`bin/fm-pr-poll.sh` and `bin/fm-teardown.sh` continue to accept only a merged pull request as landed. + +## Environment + +Recorded 2026-09-08 on GNU bash 5.2.37(1)-release (x86_64-pc-linux-gnu). + +```text +$ gh --version +gh version 2.46.0 (2025-01-13 Debian 2.46.0-3) +https://github.com/cli/cli/releases/tag/v2.46.0 + +$ gh-axi --version +0.1.33 +``` + +## Supported enqueue operation + +Live GitHub GraphQL introspection reports `pullRequestId` and `expectedHeadOid` on `EnqueuePullRequestInput`. +The helper supplies both, with the head read during its live preflight. + +```text +$ jq -nc --arg q 'query { __type(name: "EnqueuePullRequestInput") { inputFields { name type { kind name ofType { kind name } } } } }' '{query:$q}' | gh-axi api POST /graphql --input - --jq '.data.__type.inputFields' +[4]: + - name: clientMutationId + type: + kind: SCALAR + name: String + ofType: null + - name: pullRequestId + type: + kind: NON_NULL + name: null + ofType: + kind: SCALAR + name: ID + - name: jump + type: + kind: SCALAR + name: Boolean + ofType: null + - name: expectedHeadOid + type: + kind: SCALAR + name: GitObjectID + ofType: null +``` + +The payload exposes the queue entry that the helper requires before it performs an independent membership read. + +```text +$ jq -nc --arg q 'query { __type(name: "EnqueuePullRequestPayload") { fields { name type { kind name ofType { kind name } } } } }' '{query:$q}' | gh-axi api POST /graphql --input - --jq '.data.__type.fields' +[2]: + - name: clientMutationId + type: + kind: SCALAR + name: String + ofType: null + - name: mergeQueueEntry + type: + kind: OBJECT + name: MergeQueueEntry + ofType: null +``` + +A queue entry and a pull request have different state machines. +A queued pull request remains open until GitHub lands it, while `MergeQueueEntryState` carries `QUEUED`, `AWAITING_CHECKS`, `MERGEABLE`, `UNMERGEABLE`, or `LOCKED`. +The merge poll therefore remains silent for an open queued pull request and reports only the later merged state. + +## Delivery-target authority + +The consolidated delivery target is `cisrd/firstmate`, where the authenticated captain account has push authority. +The former upstream target `kunchenguid/firstmate` is read-only for the same account. + +```text +$ gh-axi api /repos/cisrd/firstmate --jq '.permissions.push' +true + +$ gh-axi api /repos/kunchenguid/firstmate --jq '.permissions.push' +false +``` + +`bin/fm-pr-check.sh` performs the same URL-derived live permission read for a GitHub task carrying `yolo=on` before it records the PR as ready. +A task without autonomous merge authority keeps the existing upstream-contribution path. + +## Portable regressions + +```sh +bin/fm-test-run.sh tests/fm-pr-merge.test.sh +bin/fm-test-run.sh tests/fm-pr-check-security.test.sh +bin/fm-test-run.sh tests/fm-teardown.test.sh +``` + +The merge tests execute the public wrapper and prove that `--queue` calls `enqueuePullRequest`, binds `expectedHeadOid`, requires an effective queue rule, rejects a red status rollup and read-only repository, and independently confirms queue membership. +They also prove that direct-merge refusals emit a runnable canonical retry, never repeat a queue operation the caller already requested, reject repeated queue tokens, and reject arguments the mutation cannot honor. +The PR-check tests prove that autonomous delivery accepts a writable URL-derived target and refuses read-only or unverifiable targets before recording readiness. +The teardown tests prove that an open pull request, including one waiting in a merge queue, is not landed work. diff --git a/tests/fm-crew-state.test.sh b/tests/fm-crew-state.test.sh index 309a7008f8a..200e9d5de51 100755 --- a/tests/fm-crew-state.test.sh +++ b/tests/fm-crew-state.test.sh @@ -422,6 +422,103 @@ steps[9]{step,status,findings,duration_ms}: EOF } +# The 2026-09-08 vacuous-pass shape, captured verbatim from a real run in an +# isolated NM_HOME fixture (no-mistakes v1.64.0): the branch's commits were +# already contained in the rebase base, so the diff vanished, no-mistakes logged +# "empty diff after rebase, skipping remaining steps", and it still recorded the +# run completed with every mandatory delivery phase skipped and nothing pushed. +run_completed_empty_diff() { # + cat < + cat < + cat < + cat < cat </dev/null + fm_write_meta "$d/state/feat-empty-diff.meta" "window=fm:fm-feat-empty-diff" "worktree=$d/wt" "kind=ship" + FM_FAKE_AXI_STATUS="$(run_completed_empty_diff fm/feat-empty-diff)" + local out; out=$(run_crew_state "$d" feat-empty-diff) + assert_contains "$out" "state: failed" "a run that kept no change must not read as validated" + assert_not_contains "$out" "state: done" "empty-diff run must never read done" + assert_contains "$out" "not validated" "the verdict must say why it is not validated" + pass "completed run with every mandatory phase skipped reads failed" +} + +test_passed_outcome_with_every_phase_skipped_reads_failed() { + reset_fakes + local d; d=$(new_case passed-empty-diff) + make_repo_on_branch "$d/wt" fm/feat-passed-empty + make_fakebin "$d" >/dev/null + fm_write_meta "$d/state/feat-passed-empty.meta" "window=fm:fm-feat-passed-empty" "worktree=$d/wt" "kind=ship" + FM_FAKE_AXI_STATUS="$(run_outcome_empty_diff fm/feat-passed-empty passed)" + local out; out=$(run_crew_state "$d" feat-passed-empty) + assert_contains "$out" "state: failed" "outcome passed must not be trusted on its own" + assert_not_contains "$out" "state: done" "vacuous passed outcome must never read done" + pass "outcome passed with every mandatory phase skipped reads failed" +} + +test_completed_run_with_full_delivery_still_reads_done() { + reset_fakes + local d; d=$(new_case completed-full-delivery) + make_repo_on_branch "$d/wt" fm/feat-full-delivery + make_fakebin "$d" >/dev/null + fm_write_meta "$d/state/feat-full-delivery.meta" "window=fm:fm-feat-full-delivery" "worktree=$d/wt" "kind=ship" + FM_FAKE_AXI_STATUS="$(run_completed_full_delivery fm/feat-full-delivery)" + local out; out=$(run_crew_state "$d" feat-full-delivery) + assert_contains "$out" "state: done" "a run that executed every phase must stay done" + assert_not_contains "$out" "not validated" "a real delivery must not be flagged vacuous" + pass "completed run with every phase executed still reads done" +} + +test_completed_run_with_partial_skip_still_reads_done() { + reset_fakes + local d; d=$(new_case completed-partial-skip) + make_repo_on_branch "$d/wt" fm/feat-partial-skip + make_fakebin "$d" >/dev/null + fm_write_meta "$d/state/feat-partial-skip.meta" "window=fm:fm-feat-partial-skip" "worktree=$d/wt" "kind=ship" + FM_FAKE_AXI_STATUS="$(run_completed_partial_skip fm/feat-partial-skip)" + local out; out=$(run_crew_state "$d" feat-partial-skip) + assert_contains "$out" "state: done" "a delivered run with some skipped phases stays done" + assert_not_contains "$out" "not validated" "partial skips are not the vacuous shape" + pass "completed run with only some phases skipped still reads done" +} + +# The two-run coarse fallback, the shape the full-path guard alone cannot +# reach: crew A's run ended in the vacuous empty-diff shape, and crew B then +# started a run on the same repo, so the shared daemon's bare `axi status` +# answers with B's branch. A's state read therefore falls to the coarse runs +# ledger, whose newest row for A's branch is `completed` at A's own head. That +# row carries no PR, because the vacuous run skipped push and pr, and the +# ledger offers no steps table and no run id to check any further - so nothing +# here proves a delivery phase ran and the row must not read as a validated +# done. +test_coarse_completed_ledger_row_without_pr_is_not_reported_done() { + reset_fakes + local d short; d=$(new_case coarse-completed-unverified) + make_repo_on_branch "$d/wt" fm/feat-coarse-completed + short=$(git -C "$d/wt" rev-parse --short=7 HEAD) + make_fakebin "$d" >/dev/null + fm_write_meta "$d/state/feat-coarse-completed.meta" \ + "window=fm:fm-feat-coarse-completed" "worktree=$d/wt" "kind=ship" + FM_FAKE_AXI_STATUS="$(run_running fm/other-crew)" + FM_FAKE_RUNS_LIST="$(cat </dev/null + fm_write_meta "$d/state/feat-coarse-delivered.meta" \ + "window=fm:fm-feat-coarse-delivered" "worktree=$d/wt" "kind=ship" + FM_FAKE_AXI_STATUS="$(run_running fm/other-crew)" + FM_FAKE_RUNS_LIST="$(cat <> "$dir/home/state/task-a.meta" + FM_TEST_GH_PUSH_PERMISSION=true run_check_entry "$dir" task-a \ + https://github.com/cisrd/firstmate/pull/8 > "$dir/stdout" 2> "$dir/stderr" \ + || fail "autonomous-target-writable: writable target was refused" + assert_grep 'api /repos/cisrd/firstmate --jq .permissions.push' "$dir/gh-axi.log" \ + "autonomous-target-writable: permission was not read for the URL-derived repository" + assert_grep 'pr=https://github.com/cisrd/firstmate/pull/8' "$dir/home/state/task-a.meta" \ + "autonomous-target-writable: writable target was not recorded" + + dir=$(make_case autonomous-target-read-only) + write_task_meta "$dir" + printf '%s\n' 'yolo=on' >> "$dir/home/state/task-a.meta" + set +e + FM_TEST_GH_PUSH_PERMISSION=false run_check_entry "$dir" task-a \ + https://github.com/kunchenguid/firstmate/pull/3335 > "$dir/stdout" 2> "$dir/stderr" + rc=$? + set -e + [ "$rc" -ne 0 ] || fail "autonomous-target-read-only: read-only target was accepted" + assert_grep 'refusing autonomous delivery to read-only GitHub repository kunchenguid/firstmate' \ + "$dir/stderr" "autonomous-target-read-only: refusal did not name the URL-derived target" + assert_no_grep '^pr=' "$dir/home/state/task-a.meta" \ + "autonomous-target-read-only: read-only PR was recorded as ready" + assert_absent "$dir/home/state/task-a.check.sh" \ + "autonomous-target-read-only: read-only PR armed a merge poll" + [ ! -s "$dir/guard.log" ] \ + || fail "autonomous-target-read-only: refusal happened after the ordinary ready path began" + + dir=$(make_case autonomous-target-permission-unreadable) + write_task_meta "$dir" + printf '%s\n' 'yolo=on' >> "$dir/home/state/task-a.meta" + set +e + FM_TEST_GH_PERMISSION_READ_FAIL=1 run_check_entry "$dir" task-a \ + https://github.com/cisrd/firstmate/pull/9 > "$dir/stdout" 2> "$dir/stderr" + rc=$? + set -e + [ "$rc" -ne 0 ] || fail "autonomous-target-permission-unreadable: unknown permission was accepted" + assert_grep 'could not verify live write permission for autonomous delivery target cisrd/firstmate' \ + "$dir/stderr" "autonomous-target-permission-unreadable: unknown permission was not reported" + assert_no_grep '^pr=' "$dir/home/state/task-a.meta" \ + "autonomous-target-permission-unreadable: unverifiable PR was recorded as ready" + pass "autonomous GitHub delivery accepts writable targets and refuses read-only or unverifiable ones" +} + test_valid_recording_and_merge_derivation() { local dir expected sidecar count rc dir=$(make_case valid-recording) @@ -2136,6 +2188,7 @@ test_retirement_refuses_replacement_and_nonterminal_results test_retirement_queue_failure_and_receipt_tampering test_gitlab_merged_poll_retires test_invalid_entrypoints_have_zero_side_effects +test_autonomous_github_target_requires_live_write_permission test_valid_recording_and_merge_derivation test_rejected_metacharacter_bytes_are_inert test_static_poll_contract diff --git a/tests/fm-pr-merge.test.sh b/tests/fm-pr-merge.test.sh index cfb02b92197..2b4ea6175e7 100755 --- a/tests/fm-pr-merge.test.sh +++ b/tests/fm-pr-merge.test.sh @@ -13,6 +13,13 @@ # (e) PR URL is parsed to number + --repo for gh-axi (defaults to --squash) # (f) malformed PR URL fails fast without calling gh-axi # (g) explicit merge method is not overridden by the default --squash +# (g2) --queue invokes enqueuePullRequest without an explicit strategy +# (g3) that path uses the same live outcome read as every other GitHub merge, +# so an enqueued still-open PR is named queued rather than merged +# (g4) those forge-decides tokens are refused on GitLab before any state is +# recorded, while a real GitLab merge method still forwards +# (g5) combining a forge-decides token with an explicit GitHub strategy is +# refused before the forge is called, naming both incompatible requests # (h) repo override args fail fast because the repo comes from the URL, # including a bundled short-option cluster that carries -R # (i) a GitLab MR URL resolves and merges through glab instead of erroring @@ -27,13 +34,13 @@ # (r) GitHub success is accepted only after the PR is read back as merged # (s) an open GitHub PR that is neither merged nor queued fails verification # (t) a GitHub PR in the merge queue is reported as queued, not merged -# (u) a queue-required refusal names the exact compatible retry flags +# (u) a queue-required refusal names the forge-decides retry the queue accepts # (v) a failed poll setup cannot be reported as a verified GitHub merge # (w) a zero-exit queue-required refusal keeps merge semantics unchanged # (x) an unreadable outcome after a successful merge call keeps the PR # recorded and the merge poll armed # (y) agreeing queue rules still produce exact retry flags -# (z) conflicting queue rules report ambiguous retry guidance +# (z) conflicting queue rules name the ambiguity and still name a retry # (aa) gh-axi remains usable when gh is absent # (ab) a landed merge whose fallback outcome read fails keeps its poll armed # (ac) a successful merge in a secondmate home reports the landed PR upward @@ -59,11 +66,15 @@ # without ever claiming auto-merge was armed # (aq) an outcome read that fails after a zero-exit merge still quotes the # forge's own output, the only evidence left -# (ar) auto-merge with the queue's own method that is still unqueued refuses +# (ar) auto-merge with a forge-decides method that is still unqueued refuses # without echoing back the flags just used, and names the next step -# (as) a caller method the queue does not use still gets exact retry flags +# (as) an explicit caller method the queue refuses still gets exact retry flags # (at) an unrecognised queue method still names the queue requirement and # guesses no method +# (ax) the retry a queue-required refusal prints is itself accepted when run +# (ay) flags the caller already used are echoed back by no rules outcome and +# by no merge result, whether the queue's method is agreed, conflicting +# or unrecognised and whether the merge command succeeded or failed # (au) unreadable branch rules are reported apart from a queue-less base # (av) a base branch with no queue rule says nothing about a merge queue # (aw) a refusal built on the gh-axi view says the merge queue could not be @@ -117,7 +128,8 @@ make_case() { } # gh-axi mock recording every invocation to a log file, and gh mock answering -# headRefOid for fm-pr-check.sh's pr_head lookup. Args: case_dir head_sha +# headRefOid for fm-pr-check.sh's pr_head lookup and the post-merge outcome +# reads. Args: case_dir head_sha add_gh_mocks() { local case_dir=$1 head=$2 cat > "$case_dir/fakebin/gh-axi" <<'SH' @@ -142,7 +154,26 @@ case "\${1:-} \${2:-}" in esac ;; "api graphql") - cat "\$FM_TEST_GH_OUTCOME" + case " \$* " in + *enqueuePullRequest*) + [ ! -e "\$FM_TEST_GH_CASE/enqueue-fails" ] || { + echo "GraphQL: enqueue refused" >&2 + exit 1 + } + if [ ! -e "\$FM_TEST_GH_CASE/enqueue-does-not-stick" ]; then + sed 's/^queued=.*/queued=true/' "\$FM_TEST_GH_OUTCOME" > "\$FM_TEST_GH_OUTCOME.tmp" + mv "\$FM_TEST_GH_OUTCOME.tmp" "\$FM_TEST_GH_OUTCOME" + fi + printf '%s\n' 'entry_id=QUEUE_ENTRY' 'entry_state=QUEUED' + ;; + *viewerPermission*) + cat "\$FM_TEST_GH_OUTCOME" + printf '%s\n' 'node=PR_NODE' "head=\$(cat "\$FM_TEST_GH_CASE/github-head")" \ + "checks=\$(cat "\$FM_TEST_GH_CASE/github-checks")" \ + "permission=\$(cat "\$FM_TEST_GH_CASE/github-permission")" + ;; + *) cat "\$FM_TEST_GH_OUTCOME" ;; + esac exit 0 ;; api\ *) @@ -153,12 +184,15 @@ esac exit 0 SH chmod +x "$case_dir/fakebin/gh-axi" "$case_dir/fakebin/gh" + printf '%s\n' "$head" > "$case_dir/github-head" + printf '%s\n' SUCCESS > "$case_dir/github-checks" + printf '%s\n' WRITE > "$case_dir/github-permission" } # gh-axi mock that fails the merge call but succeeds everything else, so a # real merge failure is distinguishable from the recording step. add_gh_mocks_merge_fails() { - local case_dir=$1 + local case_dir=$1 head=8585858585858585858585858585858585858585 cat > "$case_dir/fakebin/gh-axi" <<'SH' #!/usr/bin/env bash printf '%s\n' "$*" >> "$FM_TEST_GH_AXI_LOG" @@ -172,7 +206,19 @@ SH printf '%s\n' "$*" >> "$FM_TEST_GH_LOG" case "${1:-} ${2:-}" in "api graphql") - cat "$FM_TEST_GH_OUTCOME" + case " $* " in + *enqueuePullRequest*) + echo "GraphQL: enqueue refused" >&2 + exit 1 + ;; + *viewerPermission*) + cat "$FM_TEST_GH_OUTCOME" + printf '%s\n' 'node=PR_NODE' "head=$(cat "$FM_TEST_GH_CASE/github-head")" \ + "checks=$(cat "$FM_TEST_GH_CASE/github-checks")" \ + "permission=$(cat "$FM_TEST_GH_CASE/github-permission")" + ;; + *) cat "$FM_TEST_GH_OUTCOME" ;; + esac exit 0 ;; api\ *) @@ -183,6 +229,9 @@ esac exit 0 SH chmod +x "$case_dir/fakebin/gh-axi" "$case_dir/fakebin/gh" + printf '%s\n' "$head" > "$case_dir/github-head" + printf '%s\n' SUCCESS > "$case_dir/github-checks" + printf '%s\n' WRITE > "$case_dir/github-permission" } # gh mock that still answers fm-pr-check.sh's head lookup but cannot answer the @@ -360,6 +409,7 @@ run_pr_merge() { FM_TEST_GH_LOG="$case_dir/gh.log" \ FM_TEST_GH_OUTCOME="$case_dir/github-outcome" \ FM_TEST_GH_RULES="$case_dir/github-rules" \ + FM_TEST_GH_CASE="$case_dir" \ FM_TEST_META_AT_MERGE="$case_dir/meta-at-merge" \ FM_TEST_REAL_MV="$REAL_MV" \ FM_TEST_GLAB_LOG="$case_dir/glab.log" \ @@ -706,10 +756,11 @@ test_github_failed_merge_with_queue_flags_never_claims_acceptance() { "github-failed-merge-queue-flags: a failed merge command was reported as an accepted request" assert_no_grep 'armed' "$case_dir/stderr" \ "github-failed-merge-queue-flags: a failed merge command was reported as an armed auto-merge" - assert_grep 'base branch main requires the merge queue; retry with:' "$case_dir/stderr" \ + assert_grep 'base branch main requires the merge queue, which sets the merge method (merge) itself and refuses an explicit strategy; retry with:' \ + "$case_dir/stderr" \ "github-failed-merge-queue-flags: the failed merge command lost its concrete retry guidance" - assert_grep 'task-x1 https://github.com/example/repo/pull/74 -- --auto --merge' "$case_dir/stderr" \ - "github-failed-merge-queue-flags: the retry guidance named no queue flags" + assert_grep 'task-x1 https://github.com/example/repo/pull/74 -- --queue' "$case_dir/stderr" \ + "github-failed-merge-queue-flags: the retry guidance named no queue-accepted flags" assert_no_grep 'verified: ' "$case_dir/stdout" \ "github-failed-merge-queue-flags: a failed merge command was reported as verified" pass "fm-pr-merge claims no acceptance for a failed merge command carrying queue flags" @@ -722,11 +773,12 @@ test_github_accepted_queue_flags_do_not_echo_back_the_same_command() { add_gh_mocks "$case_dir" 8181818181818181818181818181818181818181 write_github_outcome "$case_dir" OPEN false false main printf 'merge_method=MERGE\n' > "$case_dir/github-rules" + : > "$case_dir/enqueue-does-not-stick" : > "$case_dir/gh-axi.log" : > "$case_dir/gh.log" set +e - run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/68 -- --auto --merge \ + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/68 -- --queue \ > "$case_dir/stdout" 2> "$case_dir/stderr" rc=$? set -e @@ -734,9 +786,15 @@ test_github_accepted_queue_flags_do_not_echo_back_the_same_command() { expect_code 1 "$rc" "github-accepted-queue-flags: an unproved merge must still fail" assert_grep 'state=OPEN, merged=false, isInMergeQueue=false' "$case_dir/stderr" \ "github-accepted-queue-flags: refusal did not name the concrete observed state" - assert_grep 'this run refuses even though the request for https://github.com/example/repo/pull/68 was accepted with the exact flags base branch main requires (--auto --merge)' \ + assert_grep 'base branch main requires the merge queue, which sets the merge method (merge) itself and refuses an explicit strategy' \ + "$case_dir/stderr" \ + "github-accepted-queue-flags: the refusal stopped naming what the queue itself applies" + assert_grep 'enqueuePullRequest is the one supported queue operation and this run already requested it, so no different retry exists to name' \ "$case_dir/stderr" \ "github-accepted-queue-flags: the refusal did not explain that the right flags were already used" + assert_grep 'the outcome reported above for https://github.com/example/repo/pull/68 is the blocking cause' \ + "$case_dir/stderr" \ + "github-accepted-queue-flags: the refusal named no blocking cause in place of a retry" assert_grep "re-check the pull request's merge queue state" "$case_dir/stderr" \ "github-accepted-queue-flags: the refusal named no concrete next step" assert_no_grep 'retry with:' "$case_dir/stderr" \ @@ -763,15 +821,18 @@ test_github_mismatched_queue_flags_still_name_the_retry() { set -e expect_code 1 "$rc" "github-mismatched-queue-flags: an unproved merge must still fail" - assert_grep 'base branch main requires the merge queue; retry with:' "$case_dir/stderr" \ + assert_grep 'base branch main requires the merge queue, which sets the merge method (rebase) itself and refuses an explicit strategy; retry with:' \ + "$case_dir/stderr" \ "github-mismatched-queue-flags: a caller method the queue does not use lost its retry guidance" - assert_grep '-- --auto --rebase' "$case_dir/stderr" \ + assert_grep '-- --queue' "$case_dir/stderr" \ "github-mismatched-queue-flags: the exact compatible flags were not named" + assert_no_grep '-- --auto --rebase' "$case_dir/stderr" \ + "github-mismatched-queue-flags: the retry named a strategy the merge queue refuses" pass "fm-pr-merge still names retry flags when the caller used a different method" } test_github_unrecognised_queue_method_still_names_the_queue() { - local case_dir rc + local case_dir rc guessed case_dir=$(make_case github-unrecognised-queue-method) mkdir -p "$case_dir/wt" add_gh_mocks "$case_dir" 8383838383838383838383838383838383838383 @@ -790,13 +851,63 @@ test_github_unrecognised_queue_method_still_names_the_queue() { assert_grep 'base branch main requires the merge queue, but its configured merge method (FASTFORWARD) is not one this script recognises' \ "$case_dir/stderr" \ "github-unrecognised-queue-method: a readable queue rule produced no queue mention" - assert_no_grep 'retry with:' "$case_dir/stderr" \ - "github-unrecognised-queue-method: retry flags were named for a method nothing recognises" - assert_no_grep '--auto --' "$case_dir/stderr" \ - "github-unrecognised-queue-method: a merge method was guessed for the caller" + assert_grep '-- --queue' "$case_dir/stderr" \ + "github-unrecognised-queue-method: an unrecognised method lost the retry that needs no method" + for guessed in '--auto --merge' '--auto --squash' '--auto --rebase' '--auto --fastforward'; do + assert_no_grep "$guessed" "$case_dir/stderr" \ + "github-unrecognised-queue-method: a merge method was guessed for the caller" + done pass "fm-pr-merge names the queue requirement even when its method is unrecognised" } +# The refusal's value is the command it hands the operator, so prove that +# command by running it rather than by matching its text: the printed retry has +# to reach the forge and be accepted on a queue-governed base, not bounce off +# the guard that refuses an explicit strategy there. +test_github_queue_retry_guidance_is_runnable() { + local case_dir rc retry_line retry_cmd retry_args + case_dir=$(make_case github-queue-retry-runnable) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" 8484848484848484848484848484848484848484 + write_github_outcome "$case_dir" OPEN false false main + printf 'merge_method=REBASE\n' > "$case_dir/github-rules" + : > "$case_dir/gh-axi.log" + : > "$case_dir/gh.log" + + set +e + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/71 \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "github-queue-retry-runnable: an unproved merge must fail" + retry_line=$(grep -F -- 'retry with: ' "$case_dir/stderr") \ + || fail "github-queue-retry-runnable: the queue refusal named no retry command" + retry_cmd=${retry_line#*retry with: } + case "$retry_cmd" in + "$PR_MERGE "*) ;; + *) fail "github-queue-retry-runnable: the retry did not name this script: '$retry_cmd'" ;; + esac + retry_args=${retry_cmd#"$PR_MERGE" } + + write_github_outcome "$case_dir" OPEN false false main + : > "$case_dir/gh-axi.log" + set +e + # shellcheck disable=SC2086 # The retry command's own arguments, as printed. + run_pr_merge "$case_dir" $retry_args > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 0 "$rc" "github-queue-retry-runnable: the advised retry was itself refused" + [ ! -s "$case_dir/gh-axi.log" ] \ + || fail "github-queue-retry-runnable: the advised retry called the merge parser instead of GraphQL" + assert_grep 'enqueuePullRequest' "$case_dir/gh.log" \ + "github-queue-retry-runnable: the advised retry did not invoke enqueuePullRequest" + assert_grep 'verified: https://github.com/example/repo/pull/71 is queued' \ + "$case_dir/stdout" "github-queue-retry-runnable: the advised retry did not land in the queue" + pass "fm-pr-merge's merge-queue refusal names a retry that base branch accepts" +} + test_github_unreadable_queue_rules_are_not_reported_as_no_queue() { local case_dir rc case_dir=$(make_case github-unreadable-queue-rules) @@ -1086,8 +1197,10 @@ test_github_zero_exit_queue_required_refuses_with_exact_retry() { "github-zero-exit-queue-required: refusal did not name the concrete observed state" assert_grep 'base branch release/2026 requires the merge queue' "$case_dir/stderr" \ "github-zero-exit-queue-required: refusal did not name the queue requirement" - assert_grep '-- --auto --rebase' "$case_dir/stderr" \ + assert_grep '-- --queue' "$case_dir/stderr" \ "github-zero-exit-queue-required: refusal did not name the exact compatible flags" + assert_no_grep '-- --auto --rebase' "$case_dir/stderr" \ + "github-zero-exit-queue-required: refusal named a strategy the merge queue refuses" assert_grep 'api --paginate repos/example/repo/rules/branches/release%2F2026' "$case_dir/gh.log" \ "github-zero-exit-queue-required: queue rules were not read with pagination and encoded branch path" grep -qxF 'pr merge 56 --repo example/repo --squash' "$case_dir/gh-axi.log" \ @@ -1179,8 +1292,10 @@ test_github_queue_required_refusal_names_retry_flags() { "github-queue-required: the original forge failure was not preserved" assert_grep 'base branch master requires the merge queue' "$case_dir/stderr" \ "github-queue-required: refusal did not name the queue requirement" - grep -F -- '-- --auto --merge' "$case_dir/stderr" >/dev/null \ + grep -F -- '-- --queue' "$case_dir/stderr" >/dev/null \ || fail "github-queue-required: refusal did not name the exact compatible flags" + assert_no_grep '-- --auto --merge' "$case_dir/stderr" \ + "github-queue-required: refusal named a strategy the merge queue refuses" grep -qxF 'pr merge 54 --repo example/repo --squash' "$case_dir/gh-axi.log" \ || fail "github-queue-required: the wrapper silently changed the attempted merge semantics" assert_present "$case_dir/state/task-x1.check.sh" \ @@ -1207,9 +1322,11 @@ test_github_agreeing_queue_rules_keep_retry_guidance() { expect_code 1 "$rc" "github-agreeing-queue-rules: an unproved merge must fail" assert_grep 'base branch main requires the merge queue' "$case_dir/stderr" \ "github-agreeing-queue-rules: refusal did not name the queue requirement" - assert_grep '-- --auto --rebase' "$case_dir/stderr" \ + assert_grep '-- --queue' "$case_dir/stderr" \ "github-agreeing-queue-rules: agreeing rules omitted exact retry flags" - assert_no_grep 'exact retry flags are ambiguous' "$case_dir/stderr" \ + assert_grep 'sets the merge method (rebase) itself' "$case_dir/stderr" \ + "github-agreeing-queue-rules: agreeing rules did not resolve to one merge method" + assert_no_grep 'conflicting merge queue methods' "$case_dir/stderr" \ "github-agreeing-queue-rules: agreeing rules were reported as ambiguous" pass "fm-pr-merge aggregates agreeing merge-queue rules" } @@ -1241,9 +1358,69 @@ test_github_conflicting_queue_rules_report_ambiguity() { "github-conflicting-queue-rules: an exact retry method was guessed" assert_no_grep 'SQUASH, SQUASH' "$case_dir/stderr" \ "github-conflicting-queue-rules: a repeated queue method was named twice" + assert_grep '-- --queue' "$case_dir/stderr" \ + "github-conflicting-queue-rules: ambiguous rules lost the retry that needs no method" pass "fm-pr-merge reports ambiguity for conflicting merge-queue rules" } +# The retry a queue-governed base accepts is the same command whatever the +# rules response says, so the guard against handing that command back to a +# caller who already ran it has to hold for every rules response that would +# otherwise name it, not only for a single agreed method. +test_github_accepted_queue_flags_never_echoed_back_on_any_rules_outcome() { + local case_dir rc spec name rules situation result label + for spec in \ + 'single|merge_method=MERGE\n|which sets the merge method (merge) itself and refuses an explicit strategy' \ + 'conflicting|merge_method=MERGE\nmerge_method=SQUASH\n|has conflicting merge queue methods (MERGE, SQUASH)' \ + 'unrecognised|merge_method=FASTFORWARD\n|its configured merge method (FASTFORWARD) is not one this script recognises' + do + name=${spec%%|*} + rules=${spec#*|} + situation=${rules#*|} + rules=${rules%%|*} + # Whether the merge command returned success or failed cannot change the + # guidance: the caller typed the only retry there is either way. + for result in accepted failed; do + label="github-accepted-queue-flags-$name-$result" + case_dir=$(make_case "$label") + mkdir -p "$case_dir/wt" + if [ "$result" = failed ]; then + add_gh_mocks_merge_fails "$case_dir" + else + add_gh_mocks "$case_dir" 8585858585858585858585858585858585858585 + : > "$case_dir/enqueue-does-not-stick" + fi + write_github_outcome "$case_dir" OPEN false false main + printf '%b' "$rules" > "$case_dir/github-rules" + : > "$case_dir/gh-axi.log" + : > "$case_dir/gh.log" + + set +e + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/72 -- --queue \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "$label: an unproved merge must still fail" + assert_grep "$situation" "$case_dir/stderr" \ + "$label: the refusal stopped naming what the rules response said" + assert_grep 'so no different retry exists to name' "$case_dir/stderr" \ + "$label: the refusal did not explain that the right flags were already used" + assert_grep "re-check the pull request's merge queue state" "$case_dir/stderr" \ + "$label: the refusal named no concrete next step" + assert_no_grep 'retry with:' "$case_dir/stderr" \ + "$label: the refusal echoed back the command that just refused" + assert_no_grep '-- --queue' "$case_dir/stderr" \ + "$label: the refusal repeated the caller's own flags as guidance" + assert_no_grep 'verified: ' "$case_dir/stdout" \ + "$label: an unproved merge was reported as verified" + done + assert_no_grep 'was accepted' "$TMP_ROOT/github-accepted-queue-flags-$name-failed/stderr" \ + "github-accepted-queue-flags-$name-failed: a failed merge command was reported as an accepted request" + done + pass "fm-pr-merge never echoes back queue flags the caller already used" +} + test_extra_merge_args_forwarded() { local case_dir rc case_dir=$(make_case extra-args) @@ -1453,6 +1630,319 @@ test_method_equals_merge_method_not_overridden() { pass "fm-pr-merge respects --method= as an explicit merge method" } +test_forge_decides_method_omits_strategy() { + local case_dir + case_dir=$(make_case "queue-token-canonical") + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee + write_github_outcome "$case_dir" OPEN false false main + printf 'merge_method=MERGE\n' > "$case_dir/github-rules" + : > "$case_dir/gh-axi.log" + : > "$case_dir/gh.log" + + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/24 -- --queue \ + > "$case_dir/stdout" 2> "$case_dir/stderr" \ + || fail "queue-token-canonical: fm-pr-merge failed" + + [ ! -s "$case_dir/gh-axi.log" ] \ + || fail "queue-token-canonical: queue request reached gh-axi's merge parser" + assert_grep 'enqueuePullRequest' "$case_dir/gh.log" \ + "queue-token-canonical: queue request did not use enqueuePullRequest" + pass "fm-pr-merge sends every supported queue token through enqueuePullRequest" +} + +test_forge_decides_method_forwards_other_flags() { + local case_dir rc + case_dir=$(make_case queue-unsupported-extra-flags) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" ffffffffffffffffffffffffffffffffffffffff + : > "$case_dir/gh-axi.log" + + set +e + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/25 -- --queue --delete-branch \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "queue-unsupported-extra-flags: unsupported queue flags must refuse" + assert_grep 'queue enqueue does not support extra merge arguments (--delete-branch)' \ + "$case_dir/stderr" "queue-unsupported-extra-flags: refusal did not name the unsupported flag" + [ ! -s "$case_dir/gh-axi.log" ] || fail "queue-unsupported-extra-flags: forge was called" + assert_no_grep 'pr=https://github.com/example/repo/pull/25' "$case_dir/state/task-x1.meta" \ + "queue-unsupported-extra-flags: unsupported request was recorded before refusal" + pass "fm-pr-merge refuses queue flags the GraphQL operation cannot honour" +} + +# Combining a forge-decides token with an explicit strategy must not drop only +# the queue token and forward the strategy: that silently bypasses the queue +# path. Refuse before the forge is called and name both requests. +test_forge_decides_conflicting_strategy_refuses_before_forge() { + local case_dir rc name rest args forge explicit + for spec in \ + 'queue-squash|--queue --squash|--queue|--squash' \ + 'queue-merge|--queue --merge|--queue|--merge' \ + 'queue-rebase|--queue --rebase|--queue|--rebase' \ + 'queue-method-squash|--queue --method=squash|--queue|--method=squash' + do + name=${spec%%|*} + rest=${spec#*|} + args=${rest%%|*} + rest=${rest#*|} + forge=${rest%%|*} + explicit=${rest#*|} + case_dir=$(make_case "forge-decides-conflict-$name") + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" 3333333333333333333333333333333333333333 + : > "$case_dir/gh-axi.log" + + set +e + # shellcheck disable=SC2086 # The spelling is one or two extra merge flags. + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/33 -- $args \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "forge-decides-conflict-$name: fm-pr-merge should refuse the contradictory methods" + assert_grep "must not combine a queue request ($forge) with an explicit merge strategy ($explicit)" \ + "$case_dir/stderr" "forge-decides-conflict-$name: refusal did not name both incompatible requests" + [ ! -s "$case_dir/gh-axi.log" ] \ + || fail "forge-decides-conflict-$name: gh-axi pr merge was invoked despite the contradictory methods" + assert_no_grep 'pr=https://github.com/example/repo/pull/33' "$case_dir/state/task-x1.meta" \ + "forge-decides-conflict-$name: the URL was recorded before rejecting the contradictory methods" + assert_absent "$case_dir/state/task-x1.check.sh" \ + "forge-decides-conflict-$name: the contradictory methods armed a merge poll" + done + pass "fm-pr-merge refuses a forge-decides method combined with an explicit GitHub strategy" +} + +# Enqueueing leaves the pull request open rather than landed. The shared live +# outcome read must distinguish queue membership from a confirmed merge. +test_forge_decides_reports_queued_and_merged_outcomes() { + local case_dir rc + case_dir=$(make_case forge-decides-queued) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" 1111111111111111111111111111111111111111 + write_github_outcome "$case_dir" OPEN false false master + printf 'merge_method=MERGE\n' > "$case_dir/github-rules" + : > "$case_dir/gh-axi.log" + : > "$case_dir/gh.log" + + set +e + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/31 -- --queue \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 0 "$rc" "forge-decides-queued: a queued PR should succeed" + [ ! -s "$case_dir/gh-axi.log" ] \ + || fail "forge-decides-queued: queue request reached gh-axi's merge parser" + assert_grep 'enqueuePullRequest' "$case_dir/gh.log" \ + "forge-decides-queued: queue request did not invoke enqueuePullRequest" + assert_grep 'verified: https://github.com/example/repo/pull/31 is queued' \ + "$case_dir/stdout" "forge-decides-queued: success was not reported as queued" + assert_no_grep 'merged:' "$case_dir/stdout" \ + "forge-decides-queued: an enqueued, still-open PR was confirmed as merged" + + case_dir=$(make_case forge-decides-landed) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" 1111111111111111111111111111111111111111 + write_github_outcome "$case_dir" MERGED true false main + : > "$case_dir/gh-axi.log" + : > "$case_dir/gh.log" + + set +e + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/31 -- --queue \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 0 "$rc" "forge-decides-landed: a merged PR should succeed" + [ ! -s "$case_dir/gh-axi.log" ] \ + || fail "forge-decides-landed: an already-landed PR reached gh-axi's merge parser" + assert_no_grep 'enqueuePullRequest' "$case_dir/gh.log" \ + "forge-decides-landed: an already-landed PR was enqueued again" + assert_grep 'verified: https://github.com/example/repo/pull/31 is merged' \ + "$case_dir/stdout" "forge-decides-landed: success was not reported as verified" + pass "fm-pr-merge reports an enqueued PR as queued and a landed one as merged" +} + +test_queue_request_rejects_repeated_tokens_before_recording() { + local case_dir rc + case_dir=$(make_case queue-repeated-token) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" 4141414141414141414141414141414141414141 + + set +e + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/34 -- --queue --queue \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "queue-repeated-token: repeated queue request must refuse" + assert_grep 'repeat the queue request (--queue --queue); pass exactly one queue token' \ + "$case_dir/stderr" "queue-repeated-token: refusal did not name both repeated tokens" + assert_no_grep 'pr=https://github.com/example/repo/pull/34' "$case_dir/state/task-x1.meta" \ + "queue-repeated-token: repeated request was recorded before refusal" + [ ! -s "$case_dir/gh-axi.log" ] || fail "queue-repeated-token: forge was called" + pass "fm-pr-merge refuses repeated queue tokens before recording or calling the forge" +} + +test_queue_preflight_refuses_read_only_repository() { + local case_dir rc + case_dir=$(make_case queue-read-only-repository) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" 4242424242424242424242424242424242424242 + write_github_outcome "$case_dir" OPEN false false main + printf 'merge_method=MERGE\n' > "$case_dir/github-rules" + printf '%s\n' READ > "$case_dir/github-permission" + + set +e + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/35 -- --queue \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "queue-read-only-repository: read-only target must refuse" + assert_grep 'repository example/repo grants only READ permission, not merge authority' \ + "$case_dir/stderr" "queue-read-only-repository: refusal did not name live permission" + assert_no_grep 'enqueuePullRequest' "$case_dir/gh.log" \ + "queue-read-only-repository: read-only target reached enqueuePullRequest" + pass "fm-pr-merge refuses a read-only queue destination before enqueueing" +} + +test_queue_preflight_refuses_red_checks() { + local case_dir rc + case_dir=$(make_case queue-red-checks) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" 4343434343434343434343434343434343434343 + write_github_outcome "$case_dir" OPEN false false main + printf 'merge_method=MERGE\n' > "$case_dir/github-rules" + printf '%s\n' FAILURE > "$case_dir/github-checks" + + set +e + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/36 -- --queue \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "queue-red-checks: red checks must refuse" + assert_grep 'has a red status-check rollup (FAILURE)' "$case_dir/stderr" \ + "queue-red-checks: refusal did not name the red rollup" + assert_no_grep 'enqueuePullRequest' "$case_dir/gh.log" \ + "queue-red-checks: red head reached enqueuePullRequest" + pass "fm-pr-merge keeps red pull requests out of the merge queue" +} + +test_queue_preflight_and_mutation_bind_the_head() { + local case_dir rc old_head new_head + old_head=4444444444444444444444444444444444444444 + new_head=4545454545454545454545454545454545454545 + case_dir=$(make_case queue-changed-head) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" "$old_head" + write_github_outcome "$case_dir" OPEN false false main + printf 'merge_method=MERGE\n' > "$case_dir/github-rules" + printf '%s\n' "$new_head" > "$case_dir/github-head" + + set +e + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/37 -- --queue \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "queue-changed-head: changed PR head must refuse" + assert_grep "recorded head $old_head changed to $new_head before the queue request" \ + "$case_dir/stderr" "queue-changed-head: refusal did not name both head identities" + assert_no_grep 'enqueuePullRequest' "$case_dir/gh.log" \ + "queue-changed-head: changed head reached enqueuePullRequest" + + case_dir=$(make_case queue-expected-head) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" "$old_head" + write_github_outcome "$case_dir" OPEN false false main + printf 'merge_method=MERGE\n' > "$case_dir/github-rules" + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/38 -- --queue \ + > "$case_dir/stdout" 2> "$case_dir/stderr" \ + || fail "queue-expected-head: valid enqueue failed" + assert_grep "expectedHeadOid=$old_head" "$case_dir/gh.log" \ + "queue-expected-head: mutation was not bound to the preflight head" + pass "fm-pr-merge refuses changed identity and binds enqueuePullRequest to the verified head" +} + +# An unreadable preflight on the queue path must refuse before the mutation. +# Identity, permission, and head continuity are prerequisites, not report-only. +test_forge_decides_unreadable_state_reports_without_failing() { + local case_dir rc + case_dir=$(make_case forge-decides-unreadable-state) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" 2222222222222222222222222222222222222222 + add_gh_mock_outcome_read_fails "$case_dir" 2222222222222222222222222222222222222222 + add_gh_axi_mock_view_fails "$case_dir" + : > "$case_dir/gh-axi.log" + : > "$case_dir/gh.log" + + set +e + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/32 -- --queue \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "forge-decides-unreadable-state: an unreadable outcome must fail" + assert_grep 'could not read the GitHub pull request identity and merge authority before enqueueing' \ + "$case_dir/stderr" "forge-decides-unreadable-state: the unreadable preflight was not reported" + assert_no_grep 'verified: ' "$case_dir/stdout" \ + "forge-decides-unreadable-state: an unread state was confirmed as merged" + [ ! -s "$case_dir/gh-axi.log" ] \ + || fail "forge-decides-unreadable-state: unreadable preflight reached the merge parser" + assert_no_grep 'enqueuePullRequest' "$case_dir/gh.log" \ + "forge-decides-unreadable-state: unreadable preflight reached the enqueue mutation" + assert_grep 'pr=https://github.com/example/repo/pull/32' "$case_dir/state/task-x1.meta" \ + "forge-decides-unreadable-state: a successful merge call lost its PR reference" + assert_present "$case_dir/state/task-x1.check.sh" \ + "forge-decides-unreadable-state: no merge poll was armed for a merge that may have landed" + pass "fm-pr-merge refuses an unreadable forge-decides outcome without claiming a merge" +} + +# The forge-decides tokens are firstmate-level and no glab flag spells them. +# GitLab already applies the project's own merge method, so they are refused by +# name before anything is recorded rather than forwarded to glab afterwards. +test_gitlab_forge_decides_method_refuses_before_recording() { + local case_dir rc + case_dir=$(make_gitlab_case "gitlab-forge-decides-canonical") + + set +e + run_pr_merge "$case_dir" task-x1 "$MR_URL" -- --queue \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "gitlab-forge-decides-canonical: fm-pr-merge should refuse the forge-decides token" + assert_grep "extra merge arguments must not request GitHub's merge queue on GitLab" \ + "$case_dir/stderr" "gitlab-forge-decides-canonical: refusal did not name the queue token" + assert_no_grep "pr=$MR_URL" "$case_dir/state/task-x1.meta" \ + "gitlab-forge-decides-canonical: the URL was recorded before rejecting the token" + assert_absent "$case_dir/state/task-x1.check.sh" \ + "gitlab-forge-decides-canonical: the refused token armed a merge poll" + [ ! -s "$case_dir/glab.log" ] \ + || fail "gitlab-forge-decides-canonical: glab was invoked with a flag it does not define" + pass "fm-pr-merge refuses forge-decides merge methods on GitLab before recording state" +} + +# A GitLab merge method the caller spells for glab itself is untouched by the +# new guard, so GitLab merge behavior is unchanged. +test_gitlab_squash_arg_still_forwarded() { + local case_dir + case_dir=$(make_gitlab_case gitlab-squash-arg) + + run_pr_merge "$case_dir" task-x1 "$MR_URL" -- --squash \ + > "$case_dir/stdout" 2> "$case_dir/stderr" || fail "gitlab-squash-arg: fm-pr-merge failed" + + [ "$(glab_merge_line "$case_dir/glab.log")" \ + = "GITLAB_HOST=$MR_HOST mr merge 7 -R $MR_PROJECT_URL --sha $MR_HEAD --yes --squash" ] \ + || fail "gitlab-squash-arg: expected the caller's --squash forwarded, got '$(glab_merge_line "$case_dir/glab.log")'" + pass "fm-pr-merge still forwards a caller's real GitLab merge method" +} + test_parses_pr_url_for_gh_axi() { local case_dir case_dir=$(make_case url-parsing) @@ -2086,6 +2576,7 @@ test_github_zero_exit_queue_required_refuses_with_exact_retry test_github_closed_unqueued_outcome_omits_retry_flags test_github_agreeing_queue_rules_keep_retry_guidance test_github_conflicting_queue_rules_report_ambiguity +test_github_accepted_queue_flags_never_echoed_back_on_any_rules_outcome test_verified_merge_records_pr_and_head test_pr_metadata_is_recorded_before_the_forge_call test_merge_failure_propagates_after_recording @@ -2096,6 +2587,7 @@ test_github_unreadable_outcome_refusal_quotes_the_forge_output test_github_accepted_queue_flags_do_not_echo_back_the_same_command test_github_mismatched_queue_flags_still_name_the_retry test_github_unrecognised_queue_method_still_names_the_queue +test_github_queue_retry_guidance_is_runnable test_github_unreadable_queue_rules_are_not_reported_as_no_queue test_github_no_queue_rule_says_nothing_about_a_queue test_github_fallback_view_refusal_says_the_queue_was_unobservable @@ -2118,6 +2610,15 @@ test_repo_override_args_refuse_before_recording test_bundled_repo_override_args_refuse_before_recording test_explicit_merge_method_not_overridden test_method_equals_merge_method_not_overridden +test_forge_decides_method_omits_strategy +test_forge_decides_method_forwards_other_flags +test_forge_decides_conflicting_strategy_refuses_before_forge +test_forge_decides_reports_queued_and_merged_outcomes +test_queue_request_rejects_repeated_tokens_before_recording +test_queue_preflight_refuses_read_only_repository +test_queue_preflight_refuses_red_checks +test_queue_preflight_and_mutation_bind_the_head +test_forge_decides_unreadable_state_reports_without_failing test_parses_pr_url_for_gh_axi test_github_still_forwards_sha_arg test_gitlab_url_resolves_and_merges @@ -2132,6 +2633,8 @@ test_gitlab_unreadable_state_refuses test_gitlab_invalid_head_refuses test_gitlab_missing_tool_refuses_before_recording test_gitlab_head_override_args_refuse_before_recording +test_gitlab_forge_decides_method_refuses_before_recording +test_gitlab_squash_arg_still_forwarded test_secondmate_merge_reports_upward_once test_secondmate_merge_reports_on_the_local_route test_gitlab_merge_reports_upward diff --git a/tests/fm-teardown.test.sh b/tests/fm-teardown.test.sh index c5c274b7d8e..d502e3205b8 100755 --- a/tests/fm-teardown.test.sh +++ b/tests/fm-teardown.test.sh @@ -32,6 +32,7 @@ # (i) no-mistakes + dirty worktree, even when work landed -> REFUSE (dirty wins) # (j) no-mistakes + gh lookup errors + content not in default -> REFUSE (fail-safe) # (k) no-mistakes + merged PR but HEAD moved afterward -> REFUSE (stale PR) +# (k2) no-mistakes + OPEN PR (merge-queue enqueue) -> REFUSE (not landed) # (l) no-mistakes + stale origin/main but fetched content -> ALLOW (fresh fetch) # (m) no-mistakes + local HEAD ancestor of merged PR head -> ALLOW (lagging local) # (n) no-mistakes + replayed unpushed patch in merged PR head -> ALLOW (replayed local) @@ -246,16 +247,22 @@ land_on_origin_main() { rm -rf "$tmp" } -# Override GitHub lookups to report PR 7 as merged with the supplied head. -add_gh_pr_merged_for_head() { - local case_dir=$1 head=$2 - cat > "$case_dir/fakebin/gh-axi" <<'SH' +# Override GitHub lookups to report PR 7 in the given state with the supplied +# head. Args: case_dir state head, where state is the lowercase forge state +# ("merged" or "open"); an open PR is the merge-queue enqueue state, which +# leaves the pull request open until it actually lands. +add_gh_pr_state_for_head() { + local case_dir=$1 state=$2 head=$3 upper + upper=$(printf '%s' "$state" | tr '[:lower:]' '[:upper:]') + cat > "$case_dir/fakebin/gh-axi" < "$case_dir/stdout" 2> "$case_dir/stderr" @@ -853,7 +864,7 @@ test_squash_merged_pr_allows_when_head_ancestor_of_pr_head() { append_pr_meta_url "$case_dir" local_head=$(git -C "$case_dir/wt" rev-parse HEAD) pr_head=$(commit_tree_from_wt_head "$case_dir" "$local_head" "no-mistakes follow-up") - add_gh_pr_merged_for_head "$case_dir" "$pr_head" + add_gh_pr_state_for_head "$case_dir" merged "$pr_head" set +e run_teardown "$case_dir" > "$case_dir/stdout" 2> "$case_dir/stderr" @@ -911,7 +922,7 @@ test_squash_merged_pr_allows_replayed_unpushed_patch() { wt_commit_file "$case_dir" feature.txt hello "add feature" append_pr_meta_url "$case_dir" pr_head=$(land_equivalent_patch_on_origin_branch "$case_dir" pr-head feature.txt hello "add feature") - add_gh_pr_merged_for_head "$case_dir" "$pr_head" + add_gh_pr_state_for_head "$case_dir" merged "$pr_head" set +e run_teardown "$case_dir" > "$case_dir/stdout" 2> "$case_dir/stderr" @@ -923,6 +934,27 @@ test_squash_merged_pr_allows_replayed_unpushed_patch() { pass "squash-merged PR accepts replayed unpushed local patches contained in the PR head" } +test_open_pr_does_not_count_as_landed() { + local case_dir rc pr_head + case_dir=$(make_case open-pr-unlanded) + write_meta "$case_dir" no-mistakes ship + # Same unpushed branch content as the squash-merged allow case, but the PR is + # still OPEN: that is the merge-queue enqueue state, not a landing. + wt_commit_file "$case_dir" feature.txt hello "add feature" + append_pr_meta_for_current_head "$case_dir" + pr_head=$(git -C "$case_dir/wt" rev-parse HEAD) + add_gh_pr_state_for_head "$case_dir" open "$pr_head" + + set +e + run_teardown "$case_dir" > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "open-pr-unlanded: teardown should refuse while the PR is still open" + grep -q REFUSED "$case_dir/stderr" || fail "open-pr-unlanded: no REFUSED line in stderr" + pass "open PR, including a merge-queue enqueue, does not count as landed" +} + test_merged_pr_with_later_local_commit_refuses() { local case_dir rc pr_head case_dir=$(make_case stale-pr-head) @@ -931,7 +963,7 @@ test_merged_pr_with_later_local_commit_refuses() { append_pr_meta_for_current_head "$case_dir" pr_head=$(git -C "$case_dir/wt" rev-parse HEAD) wt_commit_file "$case_dir" later.txt local-only "local follow-up" - add_gh_pr_merged_for_head "$case_dir" "$pr_head" + add_gh_pr_state_for_head "$case_dir" merged "$pr_head" set +e run_teardown "$case_dir" > "$case_dir/stdout" 2> "$case_dir/stderr" @@ -1041,7 +1073,7 @@ test_pr_check_does_not_refresh_stale_pr_head() { write_meta "$case_dir" no-mistakes ship wt_commit_file "$case_dir" feature.txt hello "add feature" pr_head=$(git -C "$case_dir/wt" rev-parse HEAD) - add_gh_pr_merged_for_head "$case_dir" "$pr_head" + add_gh_pr_state_for_head "$case_dir" merged "$pr_head" FM_ROOT_OVERRIDE="$ROOT" \ FM_STATE_OVERRIDE="$case_dir/state" \ @@ -1078,7 +1110,7 @@ test_pr_check_records_remote_head_when_local_lags() { wt_commit_file "$case_dir" feature.txt hello "add feature" local_head=$(git -C "$case_dir/wt" rev-parse HEAD) pr_head=$(commit_tree_from_wt_head "$case_dir" "$local_head" "no-mistakes follow-up") - add_gh_pr_merged_for_head "$case_dir" "$pr_head" + add_gh_pr_state_for_head "$case_dir" merged "$pr_head" FM_ROOT_OVERRIDE="$ROOT" \ FM_STATE_OVERRIDE="$case_dir/state" \ @@ -1142,7 +1174,7 @@ test_dirty_worktree_refuses() { wt_commit_file "$case_dir" feature.txt hello "add feature" land_on_origin_main "$case_dir" feature.txt hello pr_head=$(git -C "$case_dir/wt" rev-parse HEAD) - add_gh_pr_merged_for_head "$case_dir" "$pr_head" + add_gh_pr_state_for_head "$case_dir" merged "$pr_head" printf '%s\n' "uncommitted edit" > "$case_dir/wt/feature.txt" set +e @@ -3674,6 +3706,7 @@ test_squash_merged_branch_deleted_allows test_squash_merged_pr_allows_when_head_ancestor_of_pr_head test_no_pr_recorded_discovers_merged_pr_by_branch_allows test_squash_merged_pr_allows_replayed_unpushed_patch +test_open_pr_does_not_count_as_landed test_merged_pr_with_later_local_commit_refuses test_squash_merged_rebased_branch_allows test_squash_merged_same_file_different_content_refuses