diff --git a/AGENTS.md b/AGENTS.md index ab8a66bd438..9d9d5c52364 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -107,6 +107,7 @@ state/ volatile runtime signals; gitignored .pr-poll private validated data sidecar for the byte-static PR merge poll .pr-poll-registration private transactional provenance record binding the task, canonical metadata identity, sidecar, and static poll publication .pr-poll-retirement private identity-bound crash-recovery receipt for one exact validated merged result; removed after its poll artifacts retire + .pr-poll-merge-notified canonical PR identity of the last merge outcome delivered for this task; bin/fm-pr-lib.sh owns the marker format and identity mechanics, while bin/fm-merge-outcome-lib.sh owns locked publication, duplicate suppression, and replacement; removed by teardown x-watch.check.sh generated X-mode relay poll shim; present only when opted in (section 14) pending-replies/ parent-owned secondmate pending-reply records (correlation id, delivery vs reply, recovery, escalation); fm-pending-reply-lib.sh procevent/ registered process-to-event sources, one private record per canonical source id; written by bin/fm-procevent.sh or an adapter through the shared registration publisher, and their presence alone keeps supervision required (section 13) @@ -326,7 +327,7 @@ Delivery mode and `yolo` are orthogonal. Never merge a red PR under either setting; destructive, irreversible, and security-sensitive merges still escalate. Without a current explicit captain instruction that states the concrete merge, that default stands, and standing `yolo` cannot authorize a red merge; section 1 owns when such an instruction overrides a Firstmate-written standing rule within its exact scope. Load `ask-user-authority` before deciding any ask-user finding; the implementation worker never answers its own finding. -Use `bin/fm-pr-merge.sh` for every task PR merge so merge metadata is recorded, and use `bin/fm-merge-local.sh` for approved local-only landing; never call a lower-level merge command around their guards. +Use `bin/fm-pr-merge.sh` for every task PR merge so merge metadata is recorded and an unproved merge is refused instead of reported as landed, and use `bin/fm-merge-local.sh` for approved local-only landing; never call a lower-level merge command around their guards. After an autonomous merge, give the captain a one-line full-URL or local-main outcome. ### Validate @@ -354,7 +355,7 @@ For ordinary findings from any No-Mistakes tier, steer the original worker to re When a finding invalidates a receipt or acceptance claim, use the receipt checker owner to record it before returning branch custody. After the original worker's fix, return high-risk work to full validation with the updated receipts and delta context. -Judge validation by the current-code-matched run step through `bin/fm-crew-state.sh`, not by shell liveness or the last status event. +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. A worker hand-editing, committing, aborting, or restarting during an active validation run duplicates pipeline ownership outside the supersession or ordinary-finding custody-return sequences 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-brief.sh b/bin/fm-brief.sh index 37ad3646400..cff2117691d 100755 --- a/bin/fm-brief.sh +++ b/bin/fm-brief.sh @@ -357,7 +357,8 @@ Report only true captain-relevant outcomes or a declared external wait by append \`echo "{state}: {one short line}" >> $STATUS_FILE\` States: working, needs-decision, blocked, $PAUSED_VERB, done, failed. Use \`$PAUSED_VERB: {why}\` (distinct from \`blocked:\`) only when your domain is deliberately idling on a known external wait you expect to clear on its own; use \`blocked:\` when you are stuck and need firstmate to act. -Use this only for material phase changes, a captain decision, a real blocker, a failure, or work ready for review. +Use this only for material phase changes, a captain decision, a real blocker, a failure, work ready for review, or work you landed. +Work you landed includes a merge you performed yourself under standing merge authority and one the captain merged on the forge: under that authority nothing is ever \"ready for review\", so a landed merge that goes unreported reaches the captain as silence. This is also how you return the answer to a marked from-firstmate request above. A marked request requires one correlated answer after the work; it does not require a separate receipt or start acknowledgement. Never append \`working:\` merely to acknowledge receipt or announce that a marked request has started. diff --git a/bin/fm-crew-state.sh b/bin/fm-crew-state.sh index 02c8242be00..1c9e80d0343 100755 --- a/bin/fm-crew-state.sh +++ b/bin/fm-crew-state.sh @@ -9,10 +9,9 @@ # still does not describe the crew's current state as it resumes, fixes, or # re-validates. This helper never infers the current state from a tail of the log: # it reads the authoritative source (a -# no-mistakes run-step attributed to this crew's branch and current code -# identity, with a completed receipt allowing a proven pipeline descendant, -# else the pane busy-signature) and reconciles the possibly-stale log against -# it. +# no-mistakes run-step attributed under bin/fm-nm-run-lib.sh's contract, with a +# completed receipt allowing a proven pipeline descendant, else the pane +# busy-signature) and reconciles the possibly-stale log against it. # # The determinism lives entirely here - only run-step / pane / log reads plus # fixed mapping logic, no heuristics and no LLM. Output is one stable, parseable, @@ -22,14 +21,8 @@ # # Logic, in order: # 1. Resolve worktree + backend target + kind from state/.meta. -# 2. Matching no-mistakes run for this crew's branch AND current code identity, -# active or terminal (from `axi status`, or the coarse `no-mistakes runs` -# fallback)? Branch name alone is not enough: a historical run on a reused -# branch whose head was rewritten or diverged must not be attributed. -# A run matches when its head equals the worktree HEAD, or the worktree HEAD -# is an ancestor of the run head (pipeline fix commits advanced the run on -# the same line of history). Local work that advanced past the run head, or -# diverged from it, invalidates attribution. +# 2. Attribute an active or terminal no-mistakes run under the branch, head, +# pipeline-custody, and newest-first rules owned by bin/fm-nm-run-lib.sh. # The run-step is AUTHORITATIVE: running/fixing -> working, ci -> working, # awaiting_approval/fix_review -> parked (with gate findings), terminal # passed/checks-passed -> done, failed/cancelled -> failed. EXCEPT: while @@ -254,7 +247,7 @@ crew_busy_verdict() { # # --- no-mistakes run lookup (authoritative when a run matches this branch) -- # trim, strip_quotes, the bounded nm_run call, nm_field's TOON parse, and the -# branch+head attribution rule below are thin wrappers over the ONE owner in +# attribution helpers below are thin wrappers over the ONE owner in # bin/fm-nm-run-lib.sh, shared with fm-teardown.sh's pre-teardown run abort. trim() { fm_nm_trim "$@"; } @@ -411,6 +404,10 @@ nm_runs_status_for_branch() { # # Same code-identity rule as axi status: skip a same-branch row whose # short-sha does not match this worktree (rewritten or advanced tip). if ! nm_coarse_head_matches_worktree "$sha"; then + # An UNRESOLVABLE head is unknown attribution, not a proven + # mismatch. Stop instead of surfacing an older, superseded row; + # the caller's pane/log fallback can answer without misattribution. + fm_nm_head_resolvable "$WT" "$sha" || return 0 continue fi printf '%s' "$st" @@ -453,12 +450,17 @@ if [ "$KIND" = ship ] && [ "$MODE" = no-mistakes ] && [ -n "$CREW_BRANCH" ] && c RUN_OUT=$(nm_run axi status) if [ -n "$RUN_OUT" ]; then run_branch=$(strip_quotes "$(nm_field branch)") - if [ -n "$run_branch" ] && [ "$run_branch" = "$CREW_BRANCH" ] && nm_run_head_matches_worktree; then + # Head equality, or the pipeline-owned-active exemption: while the + # pipeline owns this branch, the daemon's own branch attribution is + # authoritative and the lane head need not be a git object here + # (fm_nm_run_is_pipeline_owned_active in bin/fm-nm-run-lib.sh). + if [ -n "$run_branch" ] && [ "$run_branch" = "$CREW_BRANCH" ] \ + && { nm_run_head_matches_worktree || fm_nm_run_is_pipeline_owned_active "$RUN_OUT"; }; then HAVE_RUN=1 else - # The active-or-most-recent run is for another branch, or same branch with - # a rewritten/diverged head (the CLI is alive and answered; only the - # attribution missed) - try the coarse fallback. + # The active-or-most-recent run is for another branch, or its same-branch + # attribution failed (the CLI is alive and answered) - try the coarse + # fallback. # Deliberately nested inside `[ -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 diff --git a/bin/fm-merge-outcome-lib.sh b/bin/fm-merge-outcome-lib.sh new file mode 100755 index 00000000000..8cb6b345d9b --- /dev/null +++ b/bin/fm-merge-outcome-lib.sh @@ -0,0 +1,148 @@ +#!/usr/bin/env bash +# Shared durable, supervisor-facing outcome publication for a confirmed merge. +# +# Both a merge performed by this home and a merge detected by its existing poll +# use this operation, so neither outcome depends on an agent remembering it. +# This operation publishes the poll's local actionable row; the watcher +# immediately delivers that row as observation handling, not a second outcome +# path. +# +# The destination is the home's role, never the caller's choice: +# - a secondmate home reports upward to its parent on the same reply channel +# a remote mate's parent-replies ingest and a local mate's parent status +# log already carry, in the same " [key=]: " shape the +# charter contract defines; +# - a main home reports to the captain through the durable wake queue. +# A poll observed in a secondmate home also receives a local durable wake after +# the upward write, so the mate can handle its own poll observation. +# No new state file and no new transport are involved. +# +# Normal operation deduplicates the task's latest canonical PR identity through +# the merge-notification marker owned by bin/fm-pr-lib.sh. Main-home wake keys +# also include that PR identity so distinct PRs for a reused task remain +# distinct in queue presentation. The outcome is published before the marker +# is committed, so a failed commit stays eligible for at-least-once retry and +# may rarely duplicate rather than leave a merge silent. +# +# The main-home row is an ordinary check-kind wake, which every supervision +# actor (including the OMP supervision branch's dispatch classifier) already +# treats as main-owned, so a merge outcome never routes to the branch. +# +# Sourced by bin/fm-pr-merge.sh, bin/fm-watch.sh, and tests. No side effects on +# source beyond its sourced libraries. + +_FM_MERGE_OUTCOME_LIB_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +# shellcheck source=bin/fm-pr-lib.sh +. "$_FM_MERGE_OUTCOME_LIB_DIR/fm-pr-lib.sh" +# shellcheck source=bin/fm-secondmate-parent-lib.sh +. "$_FM_MERGE_OUTCOME_LIB_DIR/fm-secondmate-parent-lib.sh" + +# The secondmate identity of the home reporting, or non-zero when this home is +# a main home (1) or carries an unusable identity marker (2). The marker's +# contract is the seeded .fm-secondmate-home identity (bin/fm-home-seed.sh). +fm_merge_outcome_home_id() { # + local home=$1 marker id + marker="$home/.fm-secondmate-home" + if [ ! -e "$marker" ] && [ ! -L "$marker" ]; then + return 1 + fi + [ -f "$marker" ] && [ ! -L "$marker" ] || return 2 + [ "$(wc -c < "$marker")" -eq "$(LC_ALL=C tr -d '\0' < "$marker" | wc -c)" ] || return 2 + id=$(cat "$marker" 2>/dev/null) || return 2 + fm_pr_task_id_valid "$id" || return 2 + printf '%s\n' "$id" +} + +# Append to unless that exact line is already there, so a repeat +# report of the same merge cannot duplicate it. +fm_merge_outcome_append_once() { # + local path=$1 line=$2 + [ ! -L "$path" ] || return 1 + mkdir -p "$(dirname "$path")" || return 1 + if grep -Fqx -- "$line" "$path" 2>/dev/null; then + return 0 + fi + printf '%s\n' "$line" >> "$path" +} + +# shellcheck disable=SC2034 # Public result consumed by sourcing callers. +FM_MERGE_OUTCOME_ALREADY_RECORDED=false + +# fm_merge_outcome_report +# +# says who observed the merge, because that decides whether the +# existing poll path also needs a local wake: +# self - this home performed the merge. +# poll - this home's merge poll detected the merge, so the canonical outcome +# also wakes this home after any upward hop needed by a secondmate. +# +# Returns 0 when the outcome is recorded (or already was), 2 on an invalid +# request, 3 when this home's own role or parent binding cannot be read well +# enough to say where the outcome belongs, and 1 on any other failure to +# record. A caller that has already merged must report a non-zero return rather +# than treat it as success: the merge landed and the record did not. +fm_merge_outcome_report() { # + local home=$1 state=$2 id=$3 url=$4 origin=$5 + local self='' self_rc=0 destination='' line lock status=0 + local provider host path number + # The wake helpers read these as globals; scoping them here points one call at + # this state directory without touching the caller's own queue settings. + # shellcheck disable=SC2034 # Sourced wake helpers consume these scoped globals. + local STATE=$state FM_WAKE_QUEUE="$state/.wake-queue" FM_WAKE_QUEUE_LOCK="$state/.wake-queue.lock" + FM_MERGE_OUTCOME_ALREADY_RECORDED=false + case "$origin" in self|poll) ;; *) return 2 ;; esac + fm_pr_task_id_valid "$id" || return 2 + fm_pr_url_parse "$url" || return 2 + provider=$FM_PR_PROVIDER + host=$FM_PR_HOST + path=$FM_PR_PATH + number=$FM_PR_NUMBER + [ -d "$state" ] && [ ! -L "$state" ] || return 1 + + if self=$(fm_merge_outcome_home_id "$home"); then + fm_secondmate_parent_record_parse "$home/.fm-secondmate-parent" || return 3 + case "$FM_SECONDMATE_PARENT_ROUTE" in + local) + [ -n "$FM_SECONDMATE_PARENT_HOME" ] || return 3 + destination="$FM_SECONDMATE_PARENT_HOME/state/$self.status" + ;; + remote) destination="$state/parent-replies.status" ;; + *) return 3 ;; + esac + line="done [key=merged-$id]: merged $id $FM_PR_URL" + else + self_rc=$? + [ "$self_rc" -eq 1 ] || return 3 + fi + + # Load the wake queue owner only when this process has not already: sourcing + # it again would reset the process-level recovery-marker and lock state a + # running watcher (bin/fm-watch.sh) carries across its own marker transitions. + if ! command -v fm_wake_append >/dev/null 2>&1; then + # shellcheck source=bin/fm-wake-lib.sh + . "$_FM_MERGE_OUTCOME_LIB_DIR/fm-wake-lib.sh" + fi + lock="$state/$id.pr-poll-merge-notified.lock" + fm_lock_acquire_wait "$lock" || return 1 + if fm_pr_poll_merge_already_notified "$state" "$id" \ + "$provider" "$host" "$path" "$number"; then + # shellcheck disable=SC2034 # Public result consumed by sourcing callers. + FM_MERGE_OUTCOME_ALREADY_RECORDED=true + fm_lock_release "$lock" + return 0 + fi + + if [ -n "$destination" ]; then + fm_merge_outcome_append_once "$destination" "$line" || status=1 + fi + if [ "$status" -eq 0 ] && { [ "$origin" = poll ] || [ -z "$destination" ]; }; then + fm_wake_append check "merged-$id-$FM_PR_URL" \ + "check: merge landed: $id $FM_PR_URL" || status=1 + fi + if [ "$status" -eq 0 ]; then + fm_pr_poll_merge_mark_notified "$state" "$id" \ + "$provider" "$host" "$path" "$number" || status=1 + fi + fm_lock_release "$lock" + return "$status" +} diff --git a/bin/fm-nm-run-lib.sh b/bin/fm-nm-run-lib.sh index 2833b4d6a50..632b30022bf 100644 --- a/bin/fm-nm-run-lib.sh +++ b/bin/fm-nm-run-lib.sh @@ -1,10 +1,12 @@ #!/usr/bin/env bash # Shared no-mistakes axi run attribution primitives. # -# ONE owner for the branch+code-identity matching rule that decides whether a -# no-mistakes run belongs to a given worktree, used by fm-crew-state.sh -# (read-only current-state reporting) and fm-teardown.sh (pre-teardown run -# abort, see its "Fix 1" header comment). Getting this wrong in either +# ONE owner for the no-mistakes run-attribution primitives used by +# fm-crew-state.sh (read-only current-state reporting), fm-teardown.sh +# (pre-teardown run abort, see its "Fix 1" header comment), and +# fm-receipt-check.sh (bound-run completion). Teardown uses only strict +# branch-and-head identity; crew-state additionally permits the active +# pipeline-owned exemption defined below. Getting this wrong in either # direction is unsafe: a false negative hides a genuinely parked run, and a # false positive lets teardown act on a run it does not own. # @@ -65,6 +67,8 @@ fm_nm_field() { # # the same history advanced the run tip past local HEAD) # - run head is a strict ancestor of worktree HEAD, or diverged: no match # (local work advanced outside the run, or the branch tip was rewritten) +# fm_nm_run_is_pipeline_owned_active below carries the one exemption: a live +# run whose pipeline currently owns the branch binds without head equality. fm_nm_head_matches_worktree() { # local wt=$1 run_head=$2 local_full run_full [ -n "$run_head" ] || return 1 @@ -128,6 +132,34 @@ fm_nm_run_is_active() { # case "$status" in completed|failed|cancelled) return 1 ;; esac } +# 0 if head $2 resolves to a commit object in worktree $1 at all. This +# distinguishes a PROVEN mismatch (resolvable but not current: a historical or +# diverged head fm_nm_head_matches_worktree correctly rejects) from UNKNOWN +# attribution (unresolvable: e.g. a pipeline-owned lane head that never +# reached this worktree). A caller scanning run rows newest-first must stop on +# unknown attribution rather than surface an older, superseded run. +fm_nm_head_resolvable() { # + [ -n "$2" ] || return 1 + git -C "$1" rev-parse --verify --quiet "$2^{commit}" >/dev/null 2>&1 +} + +# The one exemption to the head rule above: while the pipeline OWNS the branch +# (branch_sync.state=pipeline_owned), the daemon's own branch attribution IS +# the attribution for an ACTIVE run, and head equality must not be required - +# the pipeline's lane head is routinely not a git object in the task worktree +# (rebase and fix commits that were never pushed back), so the head rule +# rejects exactly the run that is most current. The exemption never applies to +# a terminal run: a terminal run has released the branch, and binding one by +# branch name alone is the historical reused-branch misattribution the head +# rule exists to prevent. fm_nm_branch_sync_state above reads the scalar +# directly under the top-level `branch_sync:` block; it is empty when the block +# is absent (no run on the current branch, another branch's run, or a CLI +# without branch sync). +fm_nm_run_is_pipeline_owned_active() { # + [ "$(fm_nm_branch_sync_state "$1")" = pipeline_owned ] || return 1 + fm_nm_run_is_active "$1" +} + # During no-mistakes' ci monitor, top-level status and outcome stay running after # checks turn green until the PR merges, while the append-only ci log records the # transition. The most recent recognized log marker is therefore authoritative: diff --git a/bin/fm-pr-lib.sh b/bin/fm-pr-lib.sh index 4c93906f8db..3517875c3b1 100755 --- a/bin/fm-pr-lib.sh +++ b/bin/fm-pr-lib.sh @@ -941,3 +941,81 @@ fm_pr_poll_retirement_recover_all() { done [ -z "$FM_PR_POLL_RETIREMENT_REJECTED" ] } + +# --- merge-notification canonical-identity marker ---------------------------- +# A merged-PR poll retires (fm_pr_poll_retirement_recover_one) in the same +# watcher cycle that detects it, which is normally enough on its own to stop a +# duplicate detection: the check.sh is gone, so nothing re-polls it. The +# exception is the same poll re-registered after its merge was already +# surfaced. Its retirement state is scoped to one registration, so this marker +# carries the canonical PR identity across registrations for the task. Only a +# matching identity is a no-op; a different PR for the same task reaches its +# role-routed supervision destination and replaces the marker when its first +# outcome is published. bin/fm-merge-outcome-lib.sh owns that locked +# publication and the ordering around this marker; this library owns only the +# marker's format and identity mechanics. +fm_pr_poll_merge_marker_matches() { # + local marker=$1 device=$2 expected_provider=$3 expected_host=$4 expected_path=$5 expected_number=$6 + local version provider host path number + fm_pr_private_file_valid "$marker" 600 "$device" || return 1 + exec 8< "$marker" || return 1 + IFS= read -r version <&8 || { exec 8<&-; return 1; } + IFS= read -r provider <&8 || { exec 8<&-; return 1; } + IFS= read -r host <&8 || { exec 8<&-; return 1; } + IFS= read -r path <&8 || { exec 8<&-; return 1; } + IFS= read -r number <&8 || { exec 8<&-; return 1; } + if IFS= read -r _extra <&8; then + exec 8<&- + return 1 + fi + exec 8<&- + [ "$version" = fm-pr-poll-merge-notified-v1 ] \ + && [ "$provider" = "$expected_provider" ] \ + && [ "$host" = "$expected_host" ] \ + && [ "$path" = "$expected_path" ] \ + && [ "$number" = "$expected_number" ] +} + +fm_pr_poll_merge_already_notified() { # + local state=$1 id=$2 provider=$3 host=$4 path=$5 number=$6 marker state_device + fm_pr_task_id_valid "$id" || return 1 + [ -d "$state" ] && [ ! -L "$state" ] || return 1 + state_device=$(fm_pr_file_device "$state") || return 1 + marker="$state/$id.pr-poll-merge-notified" + fm_pr_poll_merge_marker_matches "$marker" "$state_device" \ + "$provider" "$host" "$path" "$number" +} + +fm_pr_poll_merge_mark_notified() { # + local state=$1 id=$2 provider=$3 host=$4 path=$5 number=$6 marker tmp state_device + fm_pr_task_id_valid "$id" || return 1 + [ -d "$state" ] && [ ! -L "$state" ] || return 1 + state_device=$(fm_pr_file_device "$state") || return 1 + marker="$state/$id.pr-poll-merge-notified" + fm_pr_regular_destination_on_device_or_absent "$marker" "$state_device" || return 1 + umask 077 + tmp=$(mktemp "$state/.fm-pr-poll-merge-notified.XXXXXX") || return 1 + if ! printf '%s\n%s\n%s\n%s\n%s\n' \ + fm-pr-poll-merge-notified-v1 "$provider" "$host" "$path" "$number" > "$tmp" \ + || ! chmod 0600 "$tmp" \ + || ! fm_pr_poll_merge_marker_matches "$tmp" "$state_device" \ + "$provider" "$host" "$path" "$number" \ + || ! fm_pr_regular_destination_on_device_or_absent "$marker" "$state_device" \ + || ! mv -f -- "$tmp" "$marker" \ + || ! fm_pr_poll_merge_marker_matches "$marker" "$state_device" \ + "$provider" "$host" "$path" "$number"; then + rm -f -- "$tmp" + return 1 + fi +} + +# Removed at teardown alongside the other per-task PR-poll artifacts +# (bin/fm-teardown.sh) so a retired task id leaves no residue behind. +fm_pr_poll_merge_notified_remove() { # + local state=$1 id=$2 marker + fm_pr_task_id_valid "$id" || return 1 + marker="$state/$id.pr-poll-merge-notified" + [ -e "$marker" ] || [ -L "$marker" ] || return 0 + [ -f "$marker" ] && [ ! -L "$marker" ] || return 1 + rm -f -- "$marker" +} diff --git a/bin/fm-pr-merge.sh b/bin/fm-pr-merge.sh index 8647e63ef11..08a4c6ecfb8 100755 --- a/bin/fm-pr-merge.sh +++ b/bin/fm-pr-merge.sh @@ -3,10 +3,55 @@ # bin/fm-pr-check.sh, so teardown can verify landed work after squash merges. # The full canonical GitHub PR URL is parsed by bin/fm-pr-lib.sh and the derived # owner/repository and PR number are passed to gh-axi as separate arguments. +# bin/fm-pr-lib.sh also parses GitLab merge request URLs so the watcher can +# follow them, but this path still addresses only GitHub by owner and +# repository and refuses a GitLab URL until merge parity lands. # # Merge method defaults to --squash when the caller passes none of --squash, -# --merge, --rebase, or --method after the optional -- separator. Extra args -# must not include --repo or -R because the repository comes only from the URL. +# --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. +# 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 +# was only requested; both are read from the caller's own arguments rather than +# from the forge's prose. The observed state is judged the same way whichever +# read produced it, and a refusal built on the gh-axi view says the merge queue +# could not be observed at all rather than implying an unqueued pull request. +# Every refusal that follows a merge command which returned success quotes that +# command's own output, marked as the forge's text and kept apart from this +# script's verdict, including the refusal for an outcome that cannot be read; +# a merge command that failed keeps its original error surfaced raw and first. +# +# Extra args must not include --repo or -R in any form, including a bundled +# short-option cluster such as -yR, because the repository comes only from the +# URL. +# +# PR metadata recording and merge-poll arming happen once, unconditionally, +# before the forge call: they are not a success claim, so they survive any +# later refusal and a merge that actually lands never loses its poll. +# A verified merge leaves a durable role-routed outcome instead of living only +# in the merging agent's memory; bin/fm-merge-outcome-lib.sh owns its +# destination, normal-case deduplication, and at-least-once recovery. A queued +# or unconfirmed request records no landed outcome and leaves the poll armed. +# A landed merge whose outcome cannot be written is reported loudly rather than +# misreported as a failed merge. # Usage: fm-pr-merge.sh [-- ] set -eu @@ -17,6 +62,8 @@ STATE="${FM_STATE_OVERRIDE:-$FM_HOME/state}" # shellcheck source=bin/fm-pr-lib.sh . "$SCRIPT_DIR/fm-pr-lib.sh" +# shellcheck source=bin/fm-merge-outcome-lib.sh +. "$SCRIPT_DIR/fm-merge-outcome-lib.sh" # The supervision branch never merges a PR: it reports a green PR and leaves the # merge to main (role partition: docs/omp-supervision-branch.md; no-op in homes # without a branch actor - contract: bin/fm-lease-lib.sh). @@ -30,9 +77,6 @@ if [ "$#" -lt 2 ]; then fi ID=$1 RAW_URL=$2 -# bin/fm-pr-lib.sh parses GitLab merge request URLs so the watcher can follow -# them, but this path still addresses only GitHub by owner/repository. The -# provider check holds that refusal exactly as it was until merge parity lands. if ! fm_pr_task_id_valid "$ID" || ! fm_pr_url_parse "$RAW_URL" \ || [ "$FM_PR_PROVIDER" != github ]; then echo "error: invalid PR merge request" >&2 @@ -55,11 +99,59 @@ caller_has_merge_method() { 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 + 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 ;; + --auto=*) + case "${arg#--auto=}" in + [tT]|[tT][rR][uU][eE]|1) requested=0 ;; + *) requested=1 ;; + esac + ;; + --disable-auto) requested=1 ;; + esac + done + return "$requested" +} + reject_repo_overrides() { local arg for arg in "$@"; do case "$arg" in - --repo|--repo=*|-R|-R?*) + --repo|--repo=*) + echo "error: extra merge arguments must not override the repository" >&2 + return 1 + ;; + --*) ;; + # A single-dash argument is a short-option cluster, which the CLI expands + # one character at a time, so -yR carries --repo exactly as a bare -R does. + -*R*) echo "error: extra merge arguments must not override the repository" >&2 return 1 ;; @@ -76,15 +168,352 @@ if [ ! -f "$META" ] || [ -L "$META" ]; then exit 1 fi -"$SCRIPT_DIR/fm-pr-check.sh" "$ID" "$URL" -grep -qxF "pr=$URL" "$META" || { - echo "error: PR metadata recording failed" >&2 - exit 1 +# Read one live GitHub pull request view after gh-axi returns. The selected +# fields distinguish a landed pull request from a merge-queue entry and retain +# the concrete state needed for a refusal. gh supplies the complete queue-aware +# view when available; gh-axi remains the degradation path that can prove a +# landed merge without making gh a prerequisite for the merge abstraction. +FM_PR_GITHUB_STATE= +FM_PR_GITHUB_MERGED= +FM_PR_GITHUB_QUEUED= +FM_PR_GITHUB_BASE= +FM_PR_GITHUB_QUEUE_OBSERVED=false +github_read_outcome_with_gh() { + local fields line + local total=0 named=0 + local state='' merged='' queued='' base='' + + # 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){pullRequest(number:$number){state merged isInMergeQueue baseRefName}}}' \ + -F "owner=$PR_OWNER" -F "repo=$PR_REPO" -F "number=$PR_NUMBER" \ + --jq '.data.repository.pullRequest | "state=" + (.state // ""), "merged=" + (.merged | tostring), "queued=" + (.isInMergeQueue | tostring), "base=" + (.baseRefName // "")' \ + 2>/dev/null) || [ -z "$fields" ]; then + 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=} ;; + *) continue ;; + esac + named=$((named + 1)) + done </dev/null); then + return 1 + fi + if ! state=$(printf '%s\n' "$output" | awk ' + $1 == "state:" { count++; value=$2 } + END { if (count == 1 && value != "") print value; else exit 1 } + '); then + return 1 + fi + case "$state" in + merged) + FM_PR_GITHUB_STATE=MERGED + FM_PR_GITHUB_MERGED=true + FM_PR_GITHUB_QUEUED=false + ;; + *) + FM_PR_GITHUB_STATE=$state + FM_PR_GITHUB_MERGED=false + FM_PR_GITHUB_QUEUED=unknown + ;; + esac + FM_PR_GITHUB_BASE= + FM_PR_GITHUB_QUEUE_OBSERVED=false +} + +github_read_outcome() { + if ! command -v gh >/dev/null 2>&1; then + github_read_outcome_with_gh_axi && return 0 + echo "error: could not read the GitHub pull request outcome after the merge attempt; PR metadata and merge poll remain recorded" >&2 + return 1 + fi + # Only a failed gh read falls back. A gh read that completes and reports the + # pull request as neither merged nor queued is a concrete outcome, not a + # missing one, so it keeps its own refusal. The gh-axi view cannot observe the + # merge queue, so it can only turn this into a proved merge or into a refusal. + github_read_outcome_with_gh && return 0 + if github_read_outcome_with_gh_axi && [ "$FM_PR_GITHUB_MERGED" = true ]; then + return 0 + fi + echo "error: could not read the GitHub pull request outcome after the merge attempt: the gh read failed and the gh-axi view could not prove the outcome either; PR metadata and merge poll remain recorded" >&2 + return 1 +} + +github_urlencode_path_segment() { + local LC_ALL=C input=$1 encoded='' char octet hex + while [ -n "$input" ]; do + char=${input%"${input#?}"} + input=${input#?} + case "$char" in + [-._~a-zA-Z0-9]) encoded=$encoded$char ;; + *) + printf -v octet '%d' "'$char" + [ "$octet" -ge 0 ] || octet=$((octet + 256)) + printf -v hex '%02X' "$octet" + encoded=$encoded%$hex + ;; + esac + done + printf '%s' "$encoded" +} + +# Read the effective merge-queue method for the observed base branch. The four +# situations the refusal has to keep apart - no queue rule, a rules response +# that could not be read, several rules that disagree, and a rule whose method +# this script does not recognise - are reported as a status rather than folded +# into one failure, because each one means something different to the operator. +FM_PR_GITHUB_QUEUE_METHOD= +FM_PR_GITHUB_QUEUE_METHODS= +FM_PR_GITHUB_QUEUE_STATUS=unreadable +github_read_queue_method() { + local methods line candidate method='' count=0 branch_path + local unrecognised=false conflicting=false + FM_PR_GITHUB_QUEUE_METHOD= + FM_PR_GITHUB_QUEUE_METHODS= + FM_PR_GITHUB_QUEUE_STATUS=unreadable + command -v gh >/dev/null 2>&1 || return 0 + [ -n "$FM_PR_GITHUB_BASE" ] || return 0 + branch_path=$(github_urlencode_path_segment "$FM_PR_GITHUB_BASE") + if ! methods=$(gh api \ + --paginate "repos/$PR_OWNER/$PR_REPO/rules/branches/$branch_path" \ + --jq '.[] | select(.type == "merge_queue") | "merge_method=" + (.parameters.merge_method // "")' \ + 2>/dev/null); then + return 0 + fi + while IFS= read -r line; do + [ -n "$line" ] || continue + case "$line" in + merge_method=*) candidate=${line#merge_method=} ;; + *) return 0 ;; + esac + count=$((count + 1)) + case "$candidate" in + MERGE|SQUASH|REBASE) ;; + *) unrecognised=true ;; + esac + if [ -z "$FM_PR_GITHUB_QUEUE_METHODS" ] && [ "$count" -eq 1 ]; then + FM_PR_GITHUB_QUEUE_METHODS=$candidate + else + case ",$FM_PR_GITHUB_QUEUE_METHODS," in + *",$candidate,"*) ;; + *) + FM_PR_GITHUB_QUEUE_METHODS="$FM_PR_GITHUB_QUEUE_METHODS,$candidate" + conflicting=true + ;; + esac + fi + method=$candidate + done <&2 + return 1 + } +} + +FM_PR_GITHUB_AUTO_REQUESTED=false +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 +# such statement may be made on its path, and routing them all through one +# predicate keeps a later one from being written without the gate. +github_merge_command_succeeded() { + [ "$FM_PR_GITHUB_MERGE_ACCEPTED" = true ] +} + +github_report_forge_output() { + local output=$1 line + github_merge_command_succeeded || return 0 + [ -n "$output" ] || return 0 + echo "error: the merge command's own output follows, quoted; it is the forge CLI's report, not this script's verdict:" >&2 + while IFS= read -r line; do + printf 'error: > %s\n' "$line" >&2 + done <&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 + ;; + 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 + ;; + 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 + ;; + 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 + ;; + esac +} + +github_report_unmerged_outcome() { + printf 'error: GitHub merge outcome was not successful: state=%s, merged=%s, isInMergeQueue=%s\n' \ + "$FM_PR_GITHUB_STATE" "$FM_PR_GITHUB_MERGED" "$FM_PR_GITHUB_QUEUED" >&2 + if ! github_state_is_open || [ "$FM_PR_GITHUB_MERGED" != false ] \ + || [ "$FM_PR_GITHUB_QUEUED" = true ]; then + return 0 + fi + if [ "$FM_PR_GITHUB_AUTO_REQUESTED" = true ]; then + if github_merge_command_succeeded; then + printf 'error: auto-merge was requested and armed for %s, but nothing is merged or in the merge queue yet, so this run refuses instead of reporting an unproved merge\n' \ + "$URL" >&2 + else + printf 'error: auto-merge was requested for %s, but the merge command itself failed, so nothing was enabled, merged or queued\n' \ + "$URL" >&2 + fi + fi + if [ "$FM_PR_GITHUB_QUEUE_OBSERVED" != true ]; then + printf 'error: the merge queue could not be observed for %s because the queue-aware read was unavailable, so a pull request already in the merge queue cannot be told apart from one that never entered it; re-check the pull request'"'"'s merge queue state before retrying\n' \ + "$URL" >&2 + return 0 + fi + github_report_queue_rules +} + +# Record before the forge call. This arms the merge poll without claiming a +# landed outcome, so even a provider read failure after a real merge cannot +# leave teardown without the PR identity it needs to verify the result. +record_pr_metadata || exit 1 + +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 +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 +if ! github_read_outcome; then + github_report_forge_output "$merge_output" + exit 1 +fi +if [ "$FM_PR_GITHUB_MERGED" = true ]; then + printf 'verified: %s is merged (state=%s, merged=%s, isInMergeQueue=%s)\n' \ + "$URL" "$FM_PR_GITHUB_STATE" "$FM_PR_GITHUB_MERGED" "$FM_PR_GITHUB_QUEUED" +elif [ "$FM_PR_GITHUB_QUEUED" = true ]; then + printf 'verified: %s is 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_report_forge_output "$merge_output" + github_report_unmerged_outcome + exit 1 +fi -gh-axi pr merge "$PR_NUMBER" --repo "$PR_OWNER/$PR_REPO" "${merge_args[@]+"${merge_args[@]}"}" "$@" +# Reached only after the forge confirmed the merge landed: every refusal above +# exits non-zero, and a queued merge exits without an outcome while its +# existing poll remains armed. +outcome_rc=0 +fm_merge_outcome_report "$FM_HOME" "$STATE" "$ID" "$URL" self || outcome_rc=$? +case "$outcome_rc" in + 0) ;; + 3) + printf 'actionable: merged %s but could not report it upward: this home has no readable secondmate identity or parent binding (.fm-secondmate-home, .fm-secondmate-parent)\n' \ + "$URL" >&2 + ;; + *) + printf 'actionable: merged %s but could not record the outcome for supervision\n' "$URL" >&2 + ;; +esac diff --git a/bin/fm-teardown.sh b/bin/fm-teardown.sh index 8a141e96138..8df228814d4 100755 --- a/bin/fm-teardown.sh +++ b/bin/fm-teardown.sh @@ -102,8 +102,8 @@ # crew's worktree, so they are not orphaned by removing the worktree. # conclude_task_no_mistakes_run attributes the active-or-most-recent run to # THIS task only when its branch AND code identity (bin/fm-nm-run-lib.sh's -# fm_nm_head_matches_worktree, the same rule bin/fm-crew-state.sh uses) both -# match this worktree, then runs `no-mistakes axi abort --run ` for +# strict fm_nm_head_matches_worktree rule) both match this worktree, then +# runs `no-mistakes axi abort --run ` for # that verified run instance. A run already terminal # (an outcome is set) or not parked at a gate is left untouched. Idempotent: # an already-aborted run reads back terminal and is skipped on retry. @@ -725,6 +725,7 @@ remove_pr_poll_artifacts() { local state_dir=$1 id=$2 validate_pr_poll_cleanup "$state_dir" "$id" || return 1 fm_pr_poll_retirement_recover_one "$state_dir" "$id" "$SCRIPT_DIR/fm-pr-poll.sh" || return 1 + fm_pr_poll_merge_notified_remove "$state_dir" "$id" || return 1 rm -f "$state_dir/$id.check.sh" "$state_dir/$id.pr-poll" \ "$state_dir/$id.pr-poll-registration" "$state_dir/$id.pr-poll-retirement" \ "$state_dir/$id.check-trust" || return 1 diff --git a/bin/fm-watch.sh b/bin/fm-watch.sh index f3f5023c9fe..f1dd6bb1423 100755 --- a/bin/fm-watch.sh +++ b/bin/fm-watch.sh @@ -99,6 +99,17 @@ mkdir -p "$STATE" . "$SCRIPT_DIR/fm-push-transition-lib.sh" # shellcheck source=bin/fm-pr-lib.sh . "$SCRIPT_DIR/fm-pr-lib.sh" +# Single owner of durable merge-outcome publication, shared with +# bin/fm-pr-merge.sh so self and poll origins use the same role-routed outcome. +# The watcher still owns immediate delivery of its actionable poll result and +# poll retirement. +# This library is a canonical lint root in its own right, and it reaches the +# wake queue, PR identity, and secondmate parent libraries. Keep it an analysis +# boundary here for the same reason as the transition and inbox owners above and +# below: following its graph from this large runtime exceeds the bounded CI lint +# worker while adding no uncovered file. +# shellcheck source=/dev/null +. "$SCRIPT_DIR/fm-merge-outcome-lib.sh" # shellcheck source=bin/fm-x-lib.sh . "$SCRIPT_DIR/fm-x-lib.sh" # shellcheck source=bin/fm-check-lib.sh @@ -1199,15 +1210,35 @@ while :; do fi if [ -n "$out" ]; then reason="check: $c: $out" - fm_wake_append check "$c" "$reason" || exit 1 if [ "$is_pr_poll" -eq 1 ] && [ "$out" = merged ]; then + # One role-routed durable outcome for a merge this home detected: + # bin/fm-merge-outcome-lib.sh publishes the captain-facing row (main + # home) or the upward parent reply plus a local row (secondmate + # home) before the poll retires, and absorbs a repeat detection of + # an identity already recorded. A record that cannot be written + # keeps the poll armed and exits loudly rather than retiring the + # only evidence of the merge. + merge_outcome_rc=0 + fm_merge_outcome_report "$FM_HOME" "$STATE" "$id" "$url" poll \ + || merge_outcome_rc=$? + if [ "$merge_outcome_rc" -ne 0 ]; then + triage_log "merge outcome for $id could not be recorded (rc=$merge_outcome_rc)" + exit 1 + fi if fm_pr_poll_retirement_publish "$STATE" "$id" "$SCRIPT_DIR/fm-pr-poll.sh" "$out"; then fm_pr_poll_retirement_recover_one "$STATE" "$id" "$SCRIPT_DIR/fm-pr-poll.sh" \ || triage_log "merged PR poll retirement remains recoverable for $id" else triage_log "merged PR poll retirement deferred because its canonical snapshot changed for $id" fi + touch "$STATE/.last-check" + if [ "$FM_MERGE_OUTCOME_ALREADY_RECORDED" = true ]; then + triage_log "absorbed duplicate merged PR poll result for $id" + continue + fi + wake "$reason" fi + fm_wake_append check "$c" "$reason" || exit 1 touch "$STATE/.last-check" wake "$reason" fi diff --git a/docs/architecture.md b/docs/architecture.md index 02e0591243e..cf0bde29d22 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -19,11 +19,14 @@ A busy pane is otherwise exempt from staleness, but only until its live generati A crew that declared an external wait (`paused:`) or a verified captain-held transfer is the exception to that bound: its busy verdict supplies liveness while identifying the long-running foreground call as the declared wait, so it takes the bounded `FM_PAUSE_RESURFACE_SECS` recheck instead of wedge escalation. Lifting the declaration restores the unchanged busy-pane wedge path, while a pane that is no longer busy returns to the existing idle declared-wait classification. Those actionable wakes are written to a durable local queue (`state/.wake-queue`) only after generation-bound recovery evidence is published, so an interrupted watcher or handling turn can be recovered without losing the queue record. -When a canonical validated PR poll returns exactly `merged`, the watcher appends that durable notification before publishing a private receipt bound to the poll's registration, bytes, file identities, metadata, provider, URL, and task ID. +When a canonical validated PR poll returns exactly `merged`, the watcher routes it through the shared merge-outcome emitter before retiring the poll, and absorbs a repeat detection only when the task's notification marker already binds that same canonical PR identity. +[`bin/fm-merge-outcome-lib.sh`](../bin/fm-merge-outcome-lib.sh)'s header owns role routing, PR-specific wake identity, marker-locked normal deduplication, and the at-least-once ordering that prefers a rare duplicate over silence. +After successful outcome publication, the watcher immediately delivers the emitter's local actionable poll row and publishes a private retirement receipt bound to the poll's registration, bytes, file identities, metadata, provider, URL, and task ID. The receipt makes retirement safely retryable across restarts: fixed-path recovery revalidates the same evidence, removes the runnable check first, removes its registration and data sidecars, removes the receipt last, and preserves task metadata including `pr=` and `pr_head=`. A concurrent replacement remains armed, every non-merged or invalid observation remains unchanged, and retirement never performs task or persistent-secondmate cleanup. -`bin/fm-pr-lib.sh` owns the receipt format and strict identity mechanics, while `bin/fm-watch.sh` owns queue-before-retirement ordering. -No-verb wakes, such as `working:` notes and bare turn-ended signals, are benign only when `bin/fm-crew-state.sh` reports positive evidence that the crew is still working: an actively running no-mistakes step attributed to that crew's current code, or an exact busy verdict from the semantic busy-state contract. +`bin/fm-pr-lib.sh` owns the notification-marker and retirement-receipt formats plus their strict identity mechanics, [`bin/fm-merge-outcome-lib.sh`](../bin/fm-merge-outcome-lib.sh) owns role-routed publication, the local durable row, and marker ordering, and `bin/fm-watch.sh` owns immediate poll-result delivery and retirement. +A merge-outcome row is an ordinary check-kind wake, so the OMP supervision branch's dispatch classifier keeps it main-owned exactly like every other merge-confirmation poll result. +No-verb wakes, such as `working:` notes and bare turn-ended signals, are benign only when `bin/fm-crew-state.sh` reports positive evidence that the crew is still working: a currently attributed active no-mistakes step, or an exact busy verdict from the semantic busy-state contract. A crew that declares `paused:` for a known external wait is separately absorbed while idle and re-surfaced only on the longer pause cadence, rather than being treated as a possible wedge. For an ordinary crew that has stopped, the normal-mode watcher first surfaces one stale wake, then applies that same cadence to an unchanged `paused:` or durable `captain-held` endpoint only when the backend confidently reports its agent dead. Live or inconclusive liveness remains fail-open at that initial surface. @@ -42,8 +45,8 @@ A queued signal annotation prints every status line still unread at that cursor, A failed read, output, or concurrent-replacement check prevents the snapshot cursor from advancing across uncertain bytes, and teardown retires a task's manifest row before that task ID can be reused. The explicit resolution is written by the actor that answers, not the busy worker: `fm-send`'s `--resolve-key` appends the closing `resolved` line to this home's own copy of the ledger at answer time, which covers crewmates, local secondmates, and remote secondmates identically because a remote mate's escalations reach that local copy through the parent-replies ingest and only the answer message itself crosses the transport. Any direct or historical annotation prints every status line unread at the presentation cursor instead of replaying only the latest line. -`bin/fm-crew-state.sh ` is the cheap current-state read for an actionable heartbeat review: it attributes a no-mistakes run, active or terminal, only when it matches the crew's branch and current code identity, then keeps that run-step authoritative even if the pane has closed. -The script header owns the exact run-head ancestry rules. +`bin/fm-crew-state.sh ` is the cheap current-state read for an actionable heartbeat review: it attributes an active or terminal no-mistakes run under the shared run-attribution contract, then keeps that run-step authoritative even if the pane has closed. +[`bin/fm-nm-run-lib.sh`](../bin/fm-nm-run-lib.sh)'s header owns the exact branch, head, pipeline-custody, and newest-first attribution rules, including the one exemption that binds an active pipeline-owned run whose lane head is not yet a git object in the task worktree. 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. 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. @@ -266,8 +269,11 @@ A ship brief records its mode as a fixed machine-readable line and the spawn ref 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. For target project repos shipped through their own no-mistakes pipeline, commits under `.no-mistakes/evidence/` are the pipeline's PR-viewable validation evidence and are expected to stay in the crew branch until the evidence-hosting design changes. The firstmate repo itself is the exception: its `.no-mistakes/` directory is local state, stays gitignored, and is rejected by CI if tracked. -PR-based task merges go through `bin/fm-pr-merge.sh`, which records `pr=` and any available `pr_head=` through `bin/fm-pr-check.sh` before calling `gh-axi pr merge`. +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` once, unconditionally, before calling `gh-axi pr merge`, so the merge poll is armed without claiming a landed outcome. The helper requires a full `https://github.com///pull/` URL, invokes `gh-axi pr merge --repo /`, defaults to `--squash`, preserves explicit merge-method flags, and rejects malformed URLs or repo override flags before recording merge state; a well-formed GitLab merge request URL (see [docs/gitlab-merge-watch.md](gitlab-merge-watch.md)) is refused too, explicitly, rather than sent to the wrong forge. +After the forge command returns, the script reads the pull request's live state back and accepts only two outcomes as success: merged, or in the merge queue; anything else refuses non-zero naming the concrete observed state, quotes the forge's own output apart from the verdict, and names the exact merge-queue retry flags when the base branch requires the queue, without ever choosing a merge method on the caller's behalf. +A verified 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. +The same emitter handles a merge firstmate performed and one its poll detected, while the watcher immediately delivers the emitter's local actionable poll row. Teardown is fail-closed for ship worktrees: dirty worktrees refuse, and committed work must be landed before the worktree is returned. [`bin/fm-teardown.sh`](../bin/fm-teardown.sh)'s header owns the landed-work proofs, PR-discovery fallback, and stale-lock recovery procedure. diff --git a/docs/configuration.md b/docs/configuration.md index ba5a1e1bd87..bad970473a2 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -660,7 +660,7 @@ FM_CREW_STATE_NM_TIMEOUT=10 # seconds allowed per no-mistakes query inside fm- FM_TODO_ITEM_MAX=100 # characters per projected session-todo item in bin/fm-todo-project.sh --emit FM_TODO_PR_TIMEOUT=20 # seconds allowed per direct forge poll in fm-todo-project; invalid or non-positive values reset to 20 FM_TEARDOWN_NM_TIMEOUT=10 # seconds allowed per no-mistakes query or abort inside fm-teardown.sh -FM_CREW_STATE_RUNS_LIMIT=200 # recent no-mistakes run rows scanned when axi status cannot be attributed to the current code +FM_CREW_STATE_RUNS_LIMIT=200 # recent no-mistakes run rows scanned when axi status cannot be attributed directly FM_CREW_STATE_BIN=bin/fm-crew-state.sh # test override for the current-state reader used by working/paused watcher triage FMX_PAIRING_TOKEN= # X mode pairing token; .env opt-in authorizes replies and eligible lifecycle actions FMX_RELAY_URL=https://myfirstmate.io # optional X relay override, mainly for local relay development diff --git a/docs/scripts.md b/docs/scripts.md index 6a1c73d99d3..20355252a7a 100644 --- a/docs/scripts.md +++ b/docs/scripts.md @@ -93,7 +93,7 @@ The shared no-mistakes gate refusal for fleet lifecycle entrypoints is summarize | `fm-supervisor-target-lib.sh` | Resolve the shared supervisor target and backend for the daemon and launcher | | `fm-supervise-daemon.sh` | Presence-gated away-mode sub-supervisor: self-handle routine wakes, guard injection by the detected primary harness, escalate batched digests, alert on failed delivery | | `fm-crew-state.sh` | Print one deterministic current-state line for a crew | -| `fm-nm-run-lib.sh` | Shared branch-and-code-identity attribution for no-mistakes runs | +| `fm-nm-run-lib.sh` | Single owner of shared no-mistakes run-attribution primitives and rules | | `fm-tangle-lib.sh` | Shared default-branch resolution and primary-checkout tangle classification | | `fm-supervision-lib.sh` | Shared in-flight-work-without-fresh-watcher-beacon predicate | | `fm-ff-lib.sh` | Shared guarded fast-forward helper for origin pulls and local secondmate syncs | @@ -117,7 +117,8 @@ The shared no-mistakes gate refusal for fleet lifecycle entrypoints is summarize | `fm-pr-lib.sh` | Own canonical task and PR validation plus private atomic PR-poll publication and identity-bound retirement | | `fm-pr-poll.sh` | Provide the byte-static watcher program for validated PR/MR-poll sidecars | | `fm-pr-check.sh` | Publish a static merge poll and atomically record validated PR-path metadata | -| `fm-pr-merge.sh` | Record PR metadata, then merge a task's canonical full GitHub URL | +| `fm-pr-merge.sh` | Record PR metadata, merge a task's canonical full GitHub URL, and verify the real outcome | +| `fm-merge-outcome-lib.sh` | Publish a confirmed merge's durable, role-routed supervision outcome | | `fm-promote.sh` | Promote a scout task in place to a protected ship task with an explicit delivery mode | | `fm-promote-transaction.sh` | Execute the pinned store's recoverable scout-to-ship promotion phases | | `fm-worktree-clean-lib.sh` | Own the shared fail-closed worktree cleanliness predicate | diff --git a/tests/fm-brief.test.sh b/tests/fm-brief.test.sh index 7b23f0f009a..504cc6249a7 100755 --- a/tests/fm-brief.test.sh +++ b/tests/fm-brief.test.sh @@ -580,8 +580,12 @@ test_secondmate_marked_request_reporting_contract() { "secondmate charter lost detailed document pointers" assert_grep 'Report only true captain-relevant outcomes or a declared external wait' "$brief" \ "secondmate charter lost declared external waits" - assert_grep 'a captain decision, a real blocker, a failure, or work ready for review' "$brief" \ - "secondmate charter lost decisions, blockers, failures, or ready outcomes" + assert_grep 'a captain decision, a real blocker, a failure, work ready for review, or work you landed' "$brief" \ + "secondmate charter lost decisions, blockers, failures, ready outcomes, or landed work" + # Under standing merge authority nothing is ever "ready for review", so the + # landed merge is the trigger a charter without this line silently omits. + assert_grep 'a merge you performed yourself under standing merge authority and one the captain merged on the forge' "$brief" \ + "secondmate charter did not name a landed merge as a reporting trigger" assert_grep 'States: working, needs-decision, blocked, paused, done, failed.' "$brief" \ "secondmate charter changed the preserved status vocabulary" pass "fm-brief.sh: marked requests avoid generic acknowledgements and preserve material reporting" diff --git a/tests/fm-crew-state.test.sh b/tests/fm-crew-state.test.sh index b516e1b0d6a..019b4e8c050 100755 --- a/tests/fm-crew-state.test.sh +++ b/tests/fm-crew-state.test.sh @@ -1324,6 +1324,169 @@ test_local_advanced_past_run_head_invalidates() { pass "local work advanced past run head invalidates attribution" } +# --- Run-attribution precedence for pipeline-owned lane heads ---------------- +# A live run whose pipeline OWNS the branch (branch_sync.state=pipeline_owned) +# can report a lane head that is not a git object in the task worktree. +# Every fixture head is deliberately unresolvable so only the top-level +# branch_sync exemption - never an accidental nested-field match - attributes +# the run. +run_running_pipeline_owned() { # [] + cat </dev/null + fm_write_meta "$d/state/feat-f10.meta" "window=fm:fm-feat-f10" "worktree=$d/wt" "kind=ship" + FM_FAKE_AXI_STATUS="$(run_running_pipeline_owned fm/feat-f10 f0f0f0f0)" + FM_FAKE_RUNS_LIST="$(cat < working" + assert_contains "$out" "source: run-step" "pipeline-owned live run -> run-step source" + assert_not_contains "$out" "state: failed" "superseded failed row must not surface over the live run" + pass "pipeline-owned active run binds without head equality and beats the failed row" +} + +# T1 direction 2: a genuinely-failed run with NO later run on the branch still +# surfaces as failed - hiding real failures is equally wrong. +test_failed_run_with_no_later_run_still_surfaces() { + reset_fakes + local d short; d=$(new_case f10-genuine-failure) + make_repo_on_branch "$d/wt" fm/feat-f10b + short=$(git -C "$d/wt" rev-parse --short=8 HEAD) + make_fakebin "$d" >/dev/null + fm_write_meta "$d/state/feat-f10b.meta" "window=fm:fm-feat-f10b" "worktree=$d/wt" "kind=ship" + FM_FAKE_AXI_STATUS="$(run_failed fm/feat-f10b)" + FM_FAKE_RUNS_LIST=" failed fm/feat-f10b ${short} 2026-08-27 12:09" + local out; out=$(run_crew_state "$d" feat-f10b) + assert_contains "$out" "state: failed" "a genuinely failed run with no later run still reports failed" + assert_contains "$out" "source: run-step" "the genuine failure is run-step sourced" + pass "a genuinely failed run with no later run is not hidden" +} + +# The coarse runs-list scan: an ACTIVE row for this branch at an unresolvable +# head is unknown attribution and must STOP the scan, never fall through onto +# the older failed row (axi status answers another branch here, so attribution +# can only go through the coarse list). +test_coarse_unresolvable_active_row_never_falls_to_older_row() { + reset_fakes + local d short; d=$(new_case f10-coarse-guard) + make_repo_on_branch "$d/wt" fm/feat-f10c + short=$(git -C "$d/wt" rev-parse --short=8 HEAD) + make_fakebin "$d" >/dev/null + fm_write_meta "$d/state/feat-f10c.meta" "window=fm:fm-feat-f10c" "worktree=$d/wt" "kind=ship" "harness=claude" + FM_FAKE_AXI_STATUS="$(run_running fm/other-crew)" + FM_FAKE_RUNS_LIST="$(cat </dev/null + fm_write_meta "$d/state/feat-f10omp.meta" "window=fm:fm-feat-f10omp" "worktree=$d/wt" "kind=ship" "harness=omp" + FM_FAKE_AXI_STATUS="$(run_running fm/other-crew)" + FM_FAKE_RUNS_LIST="$(cat </dev/null + fm_write_meta "$d/state/feat-f10d.meta" "window=fm:fm-feat-f10d" "worktree=$d/wt" "kind=ship" "harness=claude" + printf 'working: implementing\n' > "$d/state/feat-f10d.status" + FM_FAKE_AXI_STATUS="$(run_running_pipeline_owned fm/feat-f10d f0f0f0f0 synced)" + FM_FAKE_RUNS_LIST="" + FM_FAKE_BUSY=0 + arm_idle_record "$d/state" feat-f10d + local out; out=$(run_crew_state "$d" feat-f10d) + assert_not_contains "$out" "source: run-step" "a non-pipeline-owned unresolvable head must not bind" + assert_contains "$out" "source: status-log" "falls back to the status log without the exemption" + pass "the exemption requires branch_sync.state=pipeline_owned" +} + +# Negative control: the exemption also requires an ACTIVE run - a terminal run +# released the branch, so an inconsistent pipeline_owned label must not bind a +# terminal run by branch name alone. +test_pipeline_owned_terminal_run_not_exempt() { + reset_fakes + local d; d=$(new_case f10-terminal-not-exempt) + make_repo_on_branch "$d/wt" fm/feat-f10e + make_fakebin "$d" >/dev/null + fm_write_meta "$d/state/feat-f10e.meta" "window=fm:fm-feat-f10e" "worktree=$d/wt" "kind=ship" "harness=claude" + printf 'working: stage 2 in progress\n' > "$d/state/feat-f10e.status" + FM_FAKE_AXI_STATUS="$(run_running_pipeline_owned fm/feat-f10e f0f0f0f0) +outcome: failed" + FM_FAKE_RUNS_LIST="" + FM_FAKE_BUSY=0 + arm_idle_record "$d/state" feat-f10e + local out; out=$(run_crew_state "$d" feat-f10e) + assert_not_contains "$out" "source: run-step" "a terminal run must not bind through the exemption" + assert_contains "$out" "source: status-log" "falls back to the status log for a terminal unresolvable head" + pass "the exemption never applies to a terminal run" +} + test_missing_run_head_falls_back_to_current_state() { reset_fakes local d out @@ -1612,6 +1775,12 @@ test_usage_error test_historical_same_branch_rewritten_head_not_current test_active_run_descendant_fix_head_remains_current test_local_advanced_past_run_head_invalidates +test_pipeline_owned_active_run_beats_superseded_failed_row +test_failed_run_with_no_later_run_still_surfaces +test_coarse_unresolvable_active_row_never_falls_to_older_row +test_coarse_unresolvable_active_row_omp_crew_uses_pane_fallback +test_non_pipeline_owned_unresolvable_head_not_attributed +test_pipeline_owned_terminal_run_not_exempt test_missing_run_head_falls_back_to_current_state test_ship_done_is_held_until_evidence_is_complete test_ship_done_with_malformed_brief_fails_closed diff --git a/tests/fm-pr-check-security.test.sh b/tests/fm-pr-check-security.test.sh index 3214c0c2dd2..b502650137e 100755 --- a/tests/fm-pr-check-security.test.sh +++ b/tests/fm-pr-check-security.test.sh @@ -133,6 +133,12 @@ SH cat > "$fakebin/gh-axi" <<'SH' #!/usr/bin/env bash printf '%s\n' "$*" >> "$FM_TEST_GH_AXI_LOG" +case "${1:-} ${2:-}" in + "pr view") + [ "$#" -eq 5 ] && [ "${4:-}" = --repo ] || exit 2 + printf 'pull_request:\n number: %s\n state: %s\n' "$3" "${FM_TEST_GH_MERGE_STATE:-merged}" + ;; +esac exit "${FM_TEST_GH_AXI_RC:-0}" SH # Plain glab, reproducing the real CLI's contract: its field output on stdout @@ -550,12 +556,23 @@ test_valid_recording_and_merge_derivation() { >/dev/null 2>/dev/null || fail "valid merge wrapper failed" grep -qxF 'pr merge 37 --repo my-org/repo_name.with-dots --merge' "$dir/gh-axi.log" \ || fail "merge wrapper did not preserve repository derivation and method" + # A merge this home performed leaves its own durable outcome, so the poll's + # confirmation is no longer the first the captain hears of it. Acknowledge that + # record before the watcher cycle below, which is what still retires the poll. + assert_grep 'https://github.com/my-org/repo_name.with-dots/pull/37' "$dir/home/state/.wake-queue" \ + "a merge this home performed left no durable outcome" + ack_watcher_cycle "$dir/home/state" || fail "merge outcome acknowledgement failed" + # With the merge already reported, the poll's own detection is a duplicate the + # watcher absorbs, so this cycle needs its own reason to end. + add_stop_custom_check "$dir" set +e FM_TEST_GH_STATE=MERGED run_watcher_bounded "$dir/home" "$dir/fakebin" > "$dir/merged-watch.out" 2> "$dir/merged-watch.err" rc=$? set -e [ "$rc" -eq 0 ] || fail "guarded merge poll retirement failed: $(cat "$dir/merged-watch.err")" assert_poll_absent "$dir/home/state" task-a + assert_no_grep "merged-task-a" "$dir/home/state/.wake-queue" \ + "the drained self-merge outcome was republished by its poll" grep -qxF 'pr=https://github.com/my-org/repo_name.with-dots/pull/37' "$dir/home/state/task-a.meta" \ || fail "guarded merge retirement removed pr metadata" grep -qxF "pr_head=$expected" "$dir/home/state/task-a.meta" \ @@ -1082,6 +1099,8 @@ test_teardown_removes_poll_artifacts() { printf 'data\n' > "$dir/home/state/task-a.pr-poll" printf 'registration\n' > "$dir/home/state/task-a.pr-poll-registration" printf 'trust\n' > "$dir/home/state/task-a.check-trust" + fm_pr_poll_merge_mark_notified "$dir/home/state" task-a github github.com o/r 1 \ + || fail "could not seed the merge-notified marker" cat > "$fakebin/tmux" <<'SH' #!/usr/bin/env bash exit 0 @@ -1091,6 +1110,8 @@ SH FM_HOME="$dir/home" FM_ROOT_OVERRIDE="$ROOT" PATH="$fakebin:$BASE_PATH" \ "$TEARDOWN" task-a --force > "$dir/teardown.out" 2> "$dir/teardown.err" \ || fail "teardown cleanup fixture failed" + [ ! -e "$dir/home/state/task-a.pr-poll-merge-notified" ] \ + || fail "teardown left the merge-notified marker" [ ! -e "$dir/home/state/task-a.check.sh" ] || fail "teardown left the runnable check" [ ! -e "$dir/home/state/task-a.pr-poll" ] || fail "teardown left the sidecar" [ ! -e "$dir/home/state/task-a.pr-poll-registration" ] \ @@ -1281,6 +1302,281 @@ test_merged_poll_retires_once() { pass "validated merged polls notify once and retire before the next watcher cycle" } +# A poll's own retirement state is scoped to ONE registration, so it cannot by +# itself catch a poll re-registered for a task whose merge was already +# surfaced (e.g. bin/fm-pr-check.sh re-armed after the fact). The per-task +# merge-notified marker (bin/fm-pr-lib.sh) is what stops that re-registration +# from producing a second main-blocking wake for the identical merge, while a +# genuinely first notification (test_merged_poll_retires_once above) still +# reaches main. +test_merged_poll_reregistration_after_notification_is_absorbed() { + local dir state rc first + dir=$(make_case merged-reregistration-absorbed) + state="$dir/home/state" + write_poll_meta "$state" task-a https://github.com/o/r/pull/1 + seed_canonical_poll "$dir" task-a https://github.com/o/r/pull/1 + add_stop_custom_check "$dir" + + set +e + FM_TEST_GH_STATE=MERGED run_watcher_bounded "$dir/home" "$dir/fakebin" > "$dir/watch-1.out" 2> "$dir/watch-1.err" + rc=$? + set -e + [ "$rc" -eq 0 ] || fail "first merged watcher cycle failed: $(cat "$dir/watch-1.err")" + first=$(cat "$dir/watch-1.out") + case "$first" in check:*task-a.check.sh:*merged) ;; *) fail "first merge confirmation was not delivered: $first" ;; esac + ack_watcher_cycle "$state" || fail "first merge confirmation acknowledgement failed" + assert_poll_absent "$state" task-a + [ -f "$state/task-a.pr-poll-merge-notified" ] || fail "the merge-notified marker was not recorded" + + # Re-registration: fm-pr-check.sh re-armed for a task whose PR is already + # merged (a fresh check.sh/pr-poll/pr-poll-registration, a distinct + # retirement identity from the one just retired). + seed_canonical_poll "$dir" task-a https://github.com/o/r/pull/1 + rm -f "$state/.last-check" + + set +e + FM_TEST_GH_STATE=MERGED run_watcher_bounded "$dir/home" "$dir/fakebin" > "$dir/watch-2.out" 2> "$dir/watch-2.err" + rc=$? + set -e + [ "$rc" -eq 0 ] || fail "second watcher cycle failed: $(cat "$dir/watch-2.err")" + case "$(cat "$dir/watch-2.out")" in + check:*z-stop.check.sh:*stop-cycle) ;; + *) fail "the re-registered duplicate did not fall through to the next check: $(cat "$dir/watch-2.out")" ;; + esac + ! grep -F 'task-a.check.sh: merged' "$dir/watch-2.out" >/dev/null \ + || fail "a repeat identical merged poll opened a main-blocking row: $(cat "$dir/watch-2.out")" + ! grep -F 'merged-task-a' "$state/.wake-queue" >/dev/null 2>&1 \ + || fail "the absorbed duplicate merge notice was queued as a main-blocking row" + assert_poll_absent "$state" task-a + pass "a repeat identical merged poll for an already-notified task is absorbed, never queued as a main-blocking row" +} + +# The captain merging a PR on the forge is the same outcome as a merge this +# home performed: bin/fm-merge-outcome-lib.sh carries both to the parent on +# the one reply channel, so no second watch path exists for the captain's case. +# The poll's own durable row still lands here, because the mate that owns the +# task still has to act on it. +seed_secondmate_home() { # [] + local dir=$1 route=${2:-remote} + printf '%s\n' mate-x > "$dir/home/.fm-secondmate-home" + printf 'schema=fm-secondmate-parent.v1\nroute=%s\n' "$route" \ + > "$dir/home/.fm-secondmate-parent" +} + +test_merged_poll_retries_a_failed_upward_report() { + local dir state rc replies url + url=https://github.com/o/r/pull/1 + dir=$(make_case merged-poll-upward-retry) + state="$dir/home/state" + replies="$state/parent-replies.status" + printf '%s\n' mate-x > "$dir/home/.fm-secondmate-home" + write_poll_meta "$state" task-a "$url" + seed_canonical_poll "$dir" task-a "$url" + add_stop_custom_check "$dir" + + set +e + FM_TEST_GH_STATE=MERGED run_watcher_bounded "$dir/home" "$dir/fakebin" \ + > "$dir/watch-1.out" 2> "$dir/watch-1.err" + rc=$? + set -e + [ "$rc" -ne 0 ] || fail "merged-poll-upward-retry: failed report did not keep the watcher loud" + [ -f "$state/task-a.check.sh" ] \ + || fail "merged-poll-upward-retry: failed report retired its retry poll" + [ ! -e "$state/task-a.pr-poll-merge-notified" ] \ + || fail "merged-poll-upward-retry: failed report was marked notified" + [ ! -e "$replies" ] \ + || fail "merged-poll-upward-retry: failed report wrote a parent reply" + + printf 'schema=fm-secondmate-parent.v1\nroute=remote\n' \ + > "$dir/home/.fm-secondmate-parent" + rm -f "$state/.last-check" + set +e + FM_TEST_GH_STATE=MERGED run_watcher_bounded "$dir/home" "$dir/fakebin" \ + > "$dir/watch-2.out" 2> "$dir/watch-2.err" + rc=$? + set -e + [ "$rc" -eq 0 ] || fail "merged-poll-upward-retry: retry failed: $(cat "$dir/watch-2.err")" + if [ ! -e "$replies" ]; then + ack_watcher_cycle "$state" \ + || fail "merged-poll-upward-retry: recovery acknowledgement failed" + rm -f "$state/.last-check" + set +e + FM_TEST_GH_STATE=MERGED run_watcher_bounded "$dir/home" "$dir/fakebin" \ + > "$dir/watch-3.out" 2> "$dir/watch-3.err" + rc=$? + set -e + [ "$rc" -eq 0 ] || fail "merged-poll-upward-retry: post-recovery retry failed: $(cat "$dir/watch-3.err")" + fi + assert_grep "done [key=merged-task-a]: merged task-a $url" "$replies" \ + "merged-poll-upward-retry: repaired binding did not receive the retry" + assert_poll_absent "$state" task-a + pass "a failed upward merge report keeps its poll armed for repair and retry" +} + +test_self_merge_and_poll_publish_one_outcome() { + local dir state replies url rc + url=https://github.com/o/r/pull/1 + + # Interleaving one: self publication commits before the poll observes the + # merge, so the poll absorbs the committed identity without reporting again. + dir=$(make_case merge-outcome-committed) + state="$dir/home/state" + replies="$state/parent-replies.status" + seed_secondmate_home "$dir" + write_task_meta "$dir" task-a + run_check_entry "$dir" task-a "$url" >/dev/null 2>"$dir/seed.err" \ + || fail "merge-outcome-committed: could not arm merge poll" + run_merge_entry "$dir" task-a "$url" >"$dir/merge.out" 2>"$dir/merge.err" \ + || fail "merge-outcome-committed: merge entrypoint failed: $(cat "$dir/merge.err")" + add_stop_custom_check "$dir" + set +e + FM_TEST_GH_STATE=MERGED run_watcher_bounded "$dir/home" "$dir/fakebin" \ + >"$dir/watch.out" 2>"$dir/watch.err" + rc=$? + set -e + [ "$rc" -eq 0 ] \ + || fail "merge-outcome-committed: watcher failed: $(cat "$dir/watch.err")" + [ "$(grep -c -F "$url" "$replies")" -eq 1 ] \ + || fail "merge-outcome-committed: self and poll reports produced duplicate outcomes" + assert_no_grep "check: $state/task-a.check.sh: merged" "$state/.wake-queue" \ + "merge-outcome-committed: absorbed poll published a second outcome" + assert_poll_absent "$state" task-a + + # Interleaving two: self publication lands but its marker commit fails. After + # that outcome is drained, the still-armed poll must publish it again rather + # than treating the interrupted attempt as complete and going silent. + dir=$(make_case merge-outcome-uncommitted) + state="$dir/home/state" + write_task_meta "$dir" task-a + run_check_entry "$dir" task-a "$url" >/dev/null 2>"$dir/seed.err" \ + || fail "merge-outcome-uncommitted: could not arm merge poll" + cat >"$dir/fakebin/mv" <<'SH' +#!/usr/bin/env bash +case " $* " in + *pr-poll-merge-notified*) exit 1 ;; +esac +exec "$FM_TEST_REAL_MV" "$@" +SH + chmod +x "$dir/fakebin/mv" + set +e + FM_TEST_REAL_MV="$REAL_MV" run_merge_entry "$dir" task-a "$url" \ + >"$dir/merge.out" 2>"$dir/merge.err" + rc=$? + set -e + [ "$rc" -eq 0 ] \ + || fail "merge-outcome-uncommitted: landed merge was reported as failed" + assert_grep "$url" "$state/.wake-queue" \ + "merge-outcome-uncommitted: interrupted publication emitted no outcome" + [ ! -e "$state/task-a.pr-poll-merge-notified" ] \ + || fail "merge-outcome-uncommitted: failed marker commit was treated as complete" + ack_watcher_cycle "$state" \ + || fail "merge-outcome-uncommitted: could not drain the first outcome" + assert_no_grep "$url" "$state/.wake-queue" \ + "merge-outcome-uncommitted: first outcome remained queued after its drain" + rm -f "$dir/fakebin/mv" "$state/.last-check" + + set +e + FM_TEST_GH_STATE=MERGED run_watcher_bounded "$dir/home" "$dir/fakebin" \ + >"$dir/watch.out" 2>"$dir/watch.err" + rc=$? + set -e + [ "$rc" -eq 0 ] \ + || fail "merge-outcome-uncommitted: poll retry failed: $(cat "$dir/watch.err")" + case "$(cat "$dir/watch.out")" in + check:*task-a.check.sh:*merged) ;; + *) fail "merge-outcome-uncommitted: poll retry did not re-emit the outcome" ;; + esac + assert_grep "$url" "$state/.wake-queue" \ + "merge-outcome-uncommitted: drained outcome was not durably re-emitted" + fm_pr_poll_merge_already_notified "$state" task-a github github.com o/r 1 \ + || fail "merge-outcome-uncommitted: successful retry did not commit the marker" + assert_poll_absent "$state" task-a + pass "staged self-merge and poll interleavings are never silent" +} + +test_merged_poll_reports_upward_from_a_secondmate_home_once() { + local dir state rc replies url + url=https://github.com/o/r/pull/1 + dir=$(make_case merged-poll-upward) + state="$dir/home/state" + replies="$state/parent-replies.status" + seed_secondmate_home "$dir" + write_poll_meta "$state" task-a "$url" + seed_canonical_poll "$dir" task-a "$url" + add_stop_custom_check "$dir" + + set +e + FM_TEST_GH_STATE=MERGED run_watcher_bounded "$dir/home" "$dir/fakebin" > "$dir/watch-1.out" 2> "$dir/watch-1.err" + rc=$? + set -e + [ "$rc" -eq 0 ] || fail "merged-poll-upward: watcher failed: $(cat "$dir/watch-1.err")" + case "$(cat "$dir/watch-1.out")" in + check:*task-a.check.sh:*merged) ;; + *) fail "merged-poll-upward: the poll's own row was lost: $(cat "$dir/watch-1.out")" ;; + esac + assert_grep "done [key=merged-task-a]: merged task-a $url" "$replies" \ + "merged-poll-upward: a merge this home did not perform was never reported upward" + [ "$(grep -c -F "$url" "$replies")" -eq 1 ] \ + || fail "merged-poll-upward: one detected merge produced more than one upward line" + ack_watcher_cycle "$state" || fail "merged-poll-upward: acknowledgement failed" + + # Re-registered for the same, already-reported merge: the absorbed duplicate + # must not tell the parent a second time either. + seed_canonical_poll "$dir" task-a "$url" + rm -f "$state/.last-check" + set +e + FM_TEST_GH_STATE=MERGED run_watcher_bounded "$dir/home" "$dir/fakebin" > "$dir/watch-2.out" 2> "$dir/watch-2.err" + rc=$? + set -e + [ "$rc" -eq 0 ] || fail "merged-poll-upward: second watcher cycle failed: $(cat "$dir/watch-2.err")" + [ "$(grep -c -F "$url" "$replies")" -eq 1 ] \ + || fail "merged-poll-upward: an absorbed duplicate detection reported the merge again" + pass "a merge detected by the poll is reported upward from a secondmate home exactly once" +} + +test_different_merged_pr_for_same_task_is_not_absorbed() { + local dir state rc + dir=$(make_case different-merged-pr-not-absorbed) + state="$dir/home/state" + write_poll_meta "$state" task-a https://github.com/o/r/pull/1 + seed_canonical_poll "$dir" task-a https://github.com/o/r/pull/1 + + set +e + FM_TEST_GH_STATE=MERGED run_watcher_bounded "$dir/home" "$dir/fakebin" > "$dir/watch-1.out" 2> "$dir/watch-1.err" + rc=$? + set -e + [ "$rc" -eq 0 ] || fail "first merged watcher cycle failed: $(cat "$dir/watch-1.err")" + case "$(cat "$dir/watch-1.out")" in + check:*task-a.check.sh:*merged) ;; + *) fail "first PR merge confirmation was not delivered: $(cat "$dir/watch-1.out")" ;; + esac + ack_watcher_cycle "$state" || fail "first PR merge confirmation acknowledgement failed" + assert_poll_absent "$state" task-a + + write_poll_meta "$state" task-a https://github.com/o/r/pull/2 + seed_canonical_poll "$dir" task-a https://github.com/o/r/pull/2 + rm -f "$state/.last-check" + + set +e + FM_TEST_GH_STATE=MERGED run_watcher_bounded "$dir/home" "$dir/fakebin" > "$dir/watch-2.out" 2> "$dir/watch-2.err" + rc=$? + set -e + [ "$rc" -eq 0 ] || fail "different-PR watcher cycle failed: $(cat "$dir/watch-2.err")" + case "$(cat "$dir/watch-2.out")" in + check:*task-a.check.sh:*merged) ;; + *) fail "a different PR merge was absorbed: $(cat "$dir/watch-2.out")" ;; + esac + grep -F "$(printf '\tcheck\tmerged-task-a-https://github.com/o/r/pull/2\t')" \ + "$state/.wake-queue" >/dev/null 2>&1 \ + || fail "the different PR merge did not create a main-blocking wake row" + fm_pr_poll_merge_already_notified "$state" task-a github github.com o/r 2 \ + || fail "the marker was not advanced to the different PR identity" + ! fm_pr_poll_merge_already_notified "$state" task-a github github.com o/r 1 \ + || fail "the marker still matched the superseded PR identity" + assert_poll_absent "$state" task-a + pass "a different merged PR for the same task gets its own first notification" +} + test_persistent_secondmate_retirement_is_poll_only() { local dir state meta_before status_before registry_before endpoint_before rc dir=$(make_case merged-retirement-secondmate) @@ -1340,10 +1636,12 @@ test_retirement_crash_recovery() { set -e [ "$rc" -eq 0 ] || fail "post-queue retry watcher failed: $(cat "$dir/watch.err")" assert_poll_absent "$state" task-a - raw_count=$(grep -c $'\tcheck\t.*task-a.check.sh\t' "$state/.wake-queue") + raw_count=$(grep -cF "$(printf '\tcheck\tmerged-task-a-https://github.com/o/r/pull/3\t')" \ + "$state/.wake-queue" || true) [ "$raw_count" -eq 1 ] || fail "post-queue retry did not publish exactly one new terminal row" FM_HOME="$dir/home" FM_ROOT_OVERRIDE="$ROOT" "$ROOT/bin/fm-wake-drain.sh" > "$dir/drain.out" 2>/dev/null - drain_count=$(grep -c $'\tcheck\t.*task-a.check.sh\t' "$dir/drain.out") + drain_count=$(grep -cF "$(printf '\tcheck\tmerged-task-a-https://github.com/o/r/pull/3\t')" \ + "$dir/drain.out" || true) [ "$drain_count" -eq 1 ] || fail "same-key crash retry rows did not deduplicate at drain" dir=$(make_case retirement-after-receipt) @@ -1815,6 +2113,11 @@ test_watcher_surfaces_pre_metadata_poll_after_validation_lock_stales() { test_parser_matrix test_gitlab_merge_watch test_merged_poll_retires_once +test_merged_poll_reregistration_after_notification_is_absorbed +test_merged_poll_retries_a_failed_upward_report +test_self_merge_and_poll_publish_one_outcome +test_merged_poll_reports_upward_from_a_secondmate_home_once +test_different_merged_pr_for_same_task_is_not_absorbed test_persistent_secondmate_retirement_is_poll_only test_retirement_crash_recovery test_external_merge_transition_retires_only_terminal_poll diff --git a/tests/fm-pr-merge.test.sh b/tests/fm-pr-merge.test.sh index a064b6919bc..5151b2f107f 100755 --- a/tests/fm-pr-merge.test.sh +++ b/tests/fm-pr-merge.test.sh @@ -3,7 +3,8 @@ # PR, which must always record pr= and any available pr_head= into the task's # meta before merging so fm-teardown.sh's landed-check has a PR reference to # verify against, even on repos with no PR CI where the usual "checks green" -# fm-pr-check.sh trigger never fires. +# fm-pr-check.sh trigger never fires, and which must then verify the real +# outcome and refuse rather than report a merge it cannot prove. # # Matrix: # (a) merge records pr= and pr_head= before merging, and merges @@ -11,9 +12,31 @@ # (c) extra gh-axi pr merge args are forwarded after number and --repo # (d) merge is refused before gh-axi when task meta is missing # (e) PR URL is parsed to number + --repo for gh-axi (defaults to --squash) -# (f) malformed PR URL fails fast without calling gh-axi +# (f) malformed or GitLab PR URL fails fast without calling gh-axi # (g) explicit merge method is not overridden by the default --squash # (h) repo override args fail fast because the repo comes from the URL +# (i) a merge call that succeeds with the PR really merged exits 0 verified +# (j) a merge call that succeeds while the PR stays open and unqueued exits +# non-zero naming the observed state (the original false-success bug) +# (k) a merge call that succeeds with the PR in the merge queue exits 0 +# reported as queued, never as merged +# (l) a queue-required base names the exact -- --auto -- retry flags +# and never selects a method for the caller; no-rule, unreadable, +# conflicting, and unrecognised rule readings are reported apart +# (m) an unreadable outcome after a successful merge call exits non-zero but +# keeps pr= and the armed merge poll; gh-axi's own view is reached with +# gh absent, and a failed gh read falls back to it +# (n) pr= is already recorded at the moment the forge call runs +# (o) a verified merge in a secondmate home reports the landed PR upward +# once, on the route its parent binding names, and a repeat merge of the +# same PR does not duplicate that line +# (p) a refused or failed merge reports nothing +# (q) a verified merge in a main home leaves a durable wake naming the PR +# (r) a secondmate home with no usable parent binding says so loudly instead +# of merging in silence +# (s) an accepted queued GitHub merge emits nothing and leaves its poll armed +# (t) an uncommitted marker retry never loses the durable outcome +# (u) distinct merged PRs for a reused task each survive queue deduplication set -u # shellcheck source=tests/lib.sh @@ -22,6 +45,8 @@ fm_git_identity fmtest fmtest@example.invalid PR_MERGE="$ROOT/bin/fm-pr-merge.sh" TMP_ROOT=$(fm_test_tmproot fm-pr-merge-tests) +BASE_PATH=$PATH +REAL_MV=$(command -v mv) || fail "these tests need mv to simulate a failed publish" # Build a fresh sandbox for one test case: a state dir with a task meta and a # fakebin with a gh-axi mock that records how it was invoked. Echoes the case dir. @@ -42,27 +67,48 @@ make_case() { printf '%s\n' "$case_dir" } -# gh-axi mock recording every invocation to a log file, and gh mock answering -# headRefOid for fm-pr-check.sh's pr_head lookup. Args: case_dir head_sha +# gh-axi mock recording every invocation to a log file and answering its own +# `pr view` with a merged state unless FM_TEST_GH_MERGE_STATE says otherwise, +# and a gh mock answering headRefOid for fm-pr-check.sh's pr_head lookup, the +# GraphQL outcome read from FM_TEST_GH_OUTCOME (a merged PR by default), and +# the branch-rules read from FM_TEST_GH_RULES. Args: case_dir head_sha add_gh_mocks() { local case_dir=$1 head=$2 cat > "$case_dir/fakebin/gh-axi" <<'SH' #!/usr/bin/env bash printf '%s\n' "$*" >> "$FM_TEST_GH_AXI_LOG" +case "${1:-} ${2:-}" in + "pr merge") printf 'merged:\n number: %s\n status: ok\n' "${3:-}" ;; + "pr view") + [ "$#" -eq 5 ] && [ "${4:-}" = --repo ] || exit 2 + printf 'pull_request:\n number: %s\n state: %s\n' "$3" "${FM_TEST_GH_MERGE_STATE:-merged}" + ;; +esac exit 0 SH cat > "$case_dir/fakebin/gh" <> "\$FM_TEST_GH_LOG" case "\${1:-} \${2:-}" in "pr view") case " \$* " in *headRefOid*) printf '%s\n' '$head' ; exit 0 ;; esac ;; + "api graphql") + cat "\$FM_TEST_GH_OUTCOME" + exit 0 + ;; + api\ *) + cat "\$FM_TEST_GH_RULES" + exit 0 + ;; esac exit 0 SH chmod +x "$case_dir/fakebin/gh-axi" "$case_dir/fakebin/gh" + [ -e "$case_dir/github-outcome" ] || write_github_outcome "$case_dir" MERGED true false main + [ -e "$case_dir/github-rules" ] || : > "$case_dir/github-rules" } # gh-axi mock that fails the merge call but succeeds everything else, so a @@ -79,16 +125,114 @@ exit 0 SH cat > "$case_dir/fakebin/gh" <<'SH' #!/usr/bin/env bash +printf '%s\n' "$*" >> "$FM_TEST_GH_LOG" +case "${1:-} ${2:-}" in + "api graphql") + cat "$FM_TEST_GH_OUTCOME" + exit 0 + ;; + api\ *) + cat "$FM_TEST_GH_RULES" + exit 0 + ;; +esac exit 0 SH chmod +x "$case_dir/fakebin/gh-axi" "$case_dir/fakebin/gh" + [ -e "$case_dir/github-outcome" ] || write_github_outcome "$case_dir" OPEN false false main + [ -e "$case_dir/github-rules" ] || : > "$case_dir/github-rules" +} + +# gh mock that still answers fm-pr-check.sh's head lookup but cannot answer the +# outcome read, so a merge call that returned success is followed by a live +# state nothing can prove. Args: case_dir head_sha +add_gh_mock_outcome_read_fails() { + local case_dir=$1 head=$2 + cat > "$case_dir/fakebin/gh" <> "\$FM_TEST_GH_LOG" +case "\${1:-} \${2:-}" in + "pr view") + case " \$* " in + *headRefOid*) printf '%s\n' '$head' ; exit 0 ;; + esac + ;; + "api graphql") + echo 'error: could not reach the GitHub API' >&2 + exit 1 + ;; +esac +exit 0 +SH + chmod +x "$case_dir/fakebin/gh" +} + +# gh-axi mock that merges but cannot answer its own view, so a case can prove +# what happens when neither reader can establish the outcome. Args: case_dir +add_gh_axi_mock_view_fails() { + local case_dir=$1 + cat > "$case_dir/fakebin/gh-axi" <<'SH' +#!/usr/bin/env bash +printf '%s\n' "$*" >> "$FM_TEST_GH_AXI_LOG" +case "${1:-} ${2:-}" in + "pr merge") printf 'merged:\n number: %s\n status: ok\n' "${3:-}" ;; + "pr view") exit 1 ;; +esac +exit 0 +SH + chmod +x "$case_dir/fakebin/gh-axi" +} + +add_failing_poll_publish_mv() { + local case_dir=$1 + cat > "$case_dir/fakebin/mv" <<'SH' +#!/usr/bin/env bash +for arg in "$@"; do + case "$arg" in + */.fm-pr-poll-data.*) exit 1 ;; + esac +done +exec "$FM_TEST_REAL_MV" "$@" +SH + chmod +x "$case_dir/fakebin/mv" +} + +# mirror_path_without [ ...]: the whole search path +# re-exposed by symlink except one tool, because a real copy anywhere on PATH +# would prove nothing. The named bindirs are mirrored ahead of the search path, +# so the case's own mocks answer for every tool that is not the omitted one and +# the refusal names that tool alone whatever the host happens to have installed. +mirror_path_without() { + local dir=$1 omit=$2 search bindir entry name + shift 2 + mkdir -p "$dir" + search=$(printf '%s\n' "$@"; printf '%s\n' "$BASE_PATH" | tr ':' '\n') + while IFS= read -r bindir; do + [ -d "$bindir" ] || continue + for entry in "$bindir"/*; do + [ -e "$entry" ] || continue + name=${entry##*/} + [ "$name" = "$omit" ] && continue + [ -e "$dir/$name" ] || ln -s "$entry" "$dir/$name" 2>/dev/null + done + done </dev/null 2>&1 \ + || fail "the $omit-free search path still resolved $omit" } run_pr_merge() { local case_dir=$1 rc; shift FM_ROOT_OVERRIDE="$ROOT" \ + FM_HOME="${FM_TEST_HOME:-$ROOT}" \ FM_STATE_OVERRIDE="$case_dir/state" \ FM_TEST_GH_AXI_LOG="$case_dir/gh-axi.log" \ + FM_TEST_GH_LOG="$case_dir/gh.log" \ + FM_TEST_GH_OUTCOME="$case_dir/github-outcome" \ + FM_TEST_GH_RULES="$case_dir/github-rules" \ + FM_TEST_META_AT_MERGE="$case_dir/meta-at-merge" \ + FM_TEST_REAL_MV="$REAL_MV" \ PATH="$case_dir/fakebin:$PATH" \ "$PR_MERGE" "$@" rc=$? @@ -99,7 +243,16 @@ run_pr_merge() { return "$rc" } -test_records_pr_and_head_before_merging() { +write_github_outcome() { + local case_dir=$1 state=$2 merged=$3 queued=$4 base=$5 + printf '%s\n' \ + "state=$state" \ + "merged=$merged" \ + "queued=$queued" \ + "base=$base" > "$case_dir/github-outcome" +} + +test_verified_merge_records_pr_and_head() { local case_dir rc case_dir=$(make_case records-before-merge) mkdir -p "$case_dir/wt" @@ -119,7 +272,47 @@ test_records_pr_and_head_before_merging() { "records-before-merge: pr_head= was not recorded" grep -qxF 'pr merge 9 --repo example/repo --squash' "$case_dir/gh-axi.log" \ || fail "records-before-merge: gh-axi pr merge was not invoked with number, --repo, and default --squash" - pass "fm-pr-merge records pr= and pr_head= before invoking gh-axi pr merge" + pass "fm-pr-merge records pr= and pr_head= for a verified GitHub merge" +} + +# The forge call is the point of no return: once gh-axi has merged, nothing this +# script does afterwards can un-merge it. Proving pr= is already in the task's +# meta at that moment is what makes a later failure unable to lose the merge. +test_pr_metadata_is_recorded_before_the_forge_call() { + local case_dir rc + case_dir=$(make_case records-ahead-of-forge-call) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" 5151515151515151515151515151515151515151 + cat > "$case_dir/fakebin/gh-axi" <<'SH' +#!/usr/bin/env bash +printf '%s\n' "$*" >> "$FM_TEST_GH_AXI_LOG" +case "${1:-} ${2:-}" in + "pr merge") + cat "$FM_STATE_OVERRIDE/task-x1.meta" > "$FM_TEST_META_AT_MERGE" + printf 'merged:\n number: %s\n status: ok\n' "${3:-}" + ;; + "pr view") + printf 'pull_request:\n number: %s\n state: merged\n' "$3" + ;; +esac +exit 0 +SH + chmod +x "$case_dir/fakebin/gh-axi" + : > "$case_dir/gh-axi.log" + : > "$case_dir/meta-at-merge" + + set +e + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/62 \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 0 "$rc" "records-ahead-of-forge-call: fm-pr-merge should succeed" + assert_grep 'pr merge 62 --repo example/repo --squash' "$case_dir/gh-axi.log" \ + "records-ahead-of-forge-call: the merge abstraction was never invoked" + assert_grep 'pr=https://github.com/example/repo/pull/62' "$case_dir/meta-at-merge" \ + "records-ahead-of-forge-call: the merge ran before pr= was recorded" + pass "fm-pr-merge records pr= before the forge call can land the merge" } test_merge_failure_propagates_after_recording() { @@ -137,12 +330,800 @@ test_merge_failure_propagates_after_recording() { expect_code 1 "$rc" "merge-fails: fm-pr-merge should propagate the gh-axi merge failure" assert_grep 'pr=https://github.com/example/repo/pull/13' "$case_dir/state/task-x1.meta" \ - "merge-fails: pr= should already be recorded even though the merge itself failed" + "merge-fails: pr= should still be recorded even when the merge fails" + assert_grep 'error: pr merge failed' "$case_dir/stderr" \ + "merge-fails: the gh-axi failure was not surfaced" + assert_no_grep 'verified: ' "$case_dir/stdout" \ + "merge-fails: a failed merge command was reported as verified" pass "fm-pr-merge propagates a real merge failure without silently succeeding" } -test_extra_merge_args_forwarded() { +test_github_merged_outcome_is_verified() { + local case_dir rc + case_dir=$(make_case github-verified-merged) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" 1010101010101010101010101010101010101010 + : > "$case_dir/gh-axi.log" + + set +e + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/51 \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 0 "$rc" "github-verified-merged: a merged PR should succeed" + assert_grep 'verified: https://github.com/example/repo/pull/51 is merged' \ + "$case_dir/stdout" "github-verified-merged: success was not reported as verified" + assert_grep 'api graphql' "$case_dir/gh.log" \ + "github-verified-merged: the PR outcome was not read back after merging" + pass "fm-pr-merge verifies a genuinely merged GitHub pull request" +} + +test_github_verified_merge_requires_poll_recording() { + local case_dir rc + case_dir=$(make_case github-poll-recording-fails) + add_gh_mocks "$case_dir" 1111111111111111111111111111111111111111 + add_failing_poll_publish_mv "$case_dir" + : > "$case_dir/gh-axi.log" + + set +e + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/55 \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "github-poll-recording-fails: poll setup failure should fail the merge wrapper" + assert_grep 'error: could not publish PR poll' "$case_dir/stderr" \ + "github-poll-recording-fails: poll setup failure was not reported" + assert_no_grep 'verified: ' "$case_dir/stdout" \ + "github-poll-recording-fails: failed poll setup was reported as a verified merge" + # This fork's bin/fm-pr-check.sh publishes the poll before its atomic + # metadata replacement, so a failed publication records no pr= either; the + # merge must then never run, because nothing would confirm it afterwards. + assert_absent "$case_dir/state/task-x1.check.sh" \ + "github-poll-recording-fails: the failed poll setup left a runnable poll" + assert_no_grep 'pr merge' "$case_dir/gh-axi.log" \ + "github-poll-recording-fails: the merge ran without its armed poll" + pass "fm-pr-merge refuses to merge when poll recording fails" +} + +test_github_open_unqueued_outcome_refuses() { + local case_dir rc + case_dir=$(make_case github-open-unqueued) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" 2020202020202020202020202020202020202020 + write_github_outcome "$case_dir" OPEN false false master + : > "$case_dir/gh-axi.log" + : > "$case_dir/gh.log" + + set +e + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/52 \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "github-open-unqueued: an unproved merge must fail" + assert_grep 'state=OPEN, merged=false, isInMergeQueue=false' "$case_dir/stderr" \ + "github-open-unqueued: refusal did not name the concrete observed state" + assert_grep 'pr=https://github.com/example/repo/pull/52' "$case_dir/state/task-x1.meta" \ + "github-open-unqueued: the attempted merge lost its PR reference" + assert_present "$case_dir/state/task-x1.check.sh" \ + "github-open-unqueued: the attempted merge did not leave its poll armed" + assert_absent "$case_dir/state/.wake-queue" \ + "github-open-unqueued: an unproved merge left a landed outcome" + pass "fm-pr-merge refuses a GitHub merge call that leaves the PR open and unqueued" +} + +test_github_unreadable_outcome_keeps_pr_bookkeeping() { + local case_dir rc + case_dir=$(make_case github-outcome-read-fails) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" 3131313131313131313131313131313131313131 + add_gh_mock_outcome_read_fails "$case_dir" 3131313131313131313131313131313131313131 + 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/57 \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "github-outcome-read-fails: an unreadable outcome must fail" + assert_grep 'could not read the GitHub pull request outcome after the merge attempt' \ + "$case_dir/stderr" "github-outcome-read-fails: the unreadable outcome was not reported" + assert_grep 'the gh read failed and the gh-axi view could not prove the outcome either' \ + "$case_dir/stderr" "github-outcome-read-fails: the refusal did not name both failed reads" + assert_no_grep 'verified: ' "$case_dir/stdout" \ + "github-outcome-read-fails: an unproved merge was reported as verified" + # The merge call itself returned success, so the pull request may well have + # landed. Losing the reference here would leave teardown with nothing to + # verify against and no merge poll to catch up. + assert_grep 'pr=https://github.com/example/repo/pull/57' "$case_dir/state/task-x1.meta" \ + "github-outcome-read-fails: a successful merge call lost its PR reference" + assert_present "$case_dir/state/task-x1.check.sh" \ + "github-outcome-read-fails: no merge poll was armed for a merge that may have landed" + pass "fm-pr-merge keeps PR bookkeeping when it cannot read a successful merge call's outcome" +} + +test_github_refusal_quotes_the_forge_output() { + local case_dir rc + case_dir=$(make_case github-refusal-quotes-forge) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" 6161616161616161616161616161616161616161 + cat > "$case_dir/fakebin/gh-axi" <<'SH' +#!/usr/bin/env bash +printf '%s\n' "$*" >> "$FM_TEST_GH_AXI_LOG" +case "${1:-} ${2:-}" in + "pr merge") echo "will be added to the merge queue when all requirements are met" ;; +esac +exit 0 +SH + chmod +x "$case_dir/fakebin/gh-axi" + write_github_outcome "$case_dir" OPEN false 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/65 \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "github-refusal-quotes-forge: an unproved merge must fail" + assert_grep 'error: > will be added to the merge queue when all requirements are met' \ + "$case_dir/stderr" \ + "github-refusal-quotes-forge: the forge's own explanation was discarded on the refusal" + assert_grep "not this script's verdict" "$case_dir/stderr" \ + "github-refusal-quotes-forge: the forge's text was not marked as the forge's own" + assert_grep 'error: GitHub merge outcome was not successful: state=OPEN, merged=false, isInMergeQueue=false' \ + "$case_dir/stderr" "github-refusal-quotes-forge: the wrapper's own verdict was lost" + # A forge sentence about the merge queue must never stand on its own line, or + # it reads as this script's verdict rather than as quoted forge output. + ! grep -qxF 'will be added to the merge queue when all requirements are met' \ + "$case_dir/stderr" \ + || fail "github-refusal-quotes-forge: forge text was emitted as the wrapper's own line" + assert_no_grep 'will be added to the merge queue' "$case_dir/stdout" \ + "github-refusal-quotes-forge: the forge's unverified report leaked to stdout" + assert_no_grep 'verified: ' "$case_dir/stdout" \ + "github-refusal-quotes-forge: an unproved merge was reported as verified" + pass "fm-pr-merge refuses with the forge's own output quoted apart from its verdict" +} + +test_github_auto_merge_without_queue_refuses_legibly() { + local case_dir rc spelling + for spelling in --auto --auto=true; do + case_dir=$(make_case "github-auto-no-queue${spelling#--auto}") + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" 7171717171717171717171717171717171717171 + write_github_outcome "$case_dir" OPEN false false main + : > "$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/66 \ + -- "$spelling" --merge \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "github-auto-no-queue: an armed but unlanded auto-merge must still fail" + assert_grep 'state=OPEN, merged=false, isInMergeQueue=false' "$case_dir/stderr" \ + "github-auto-no-queue: refusal did not name the concrete observed state" + assert_grep 'auto-merge was requested and armed for https://github.com/example/repo/pull/66' \ + "$case_dir/stderr" "github-auto-no-queue: the refusal never explained the armed auto-merge" + assert_grep 'nothing is merged or in the merge queue yet' "$case_dir/stderr" \ + "github-auto-no-queue: the refusal left the operator to infer the pending state" + grep -qxF "pr merge 66 --repo example/repo $spelling --merge" "$case_dir/gh-axi.log" \ + || fail "github-auto-no-queue: the attempted merge was changed unexpectedly" + [ "$(wc -l < "$case_dir/gh-axi.log" | tr -d '[:space:]')" = 1 ] \ + || fail "github-auto-no-queue: the wrapper attempted more than one merge" + assert_grep 'pr=https://github.com/example/repo/pull/66' "$case_dir/state/task-x1.meta" \ + "github-auto-no-queue: the attempted merge lost its PR reference" + assert_present "$case_dir/state/task-x1.check.sh" \ + "github-auto-no-queue: the attempted merge did not leave its poll armed" + done + pass "fm-pr-merge explains an armed auto-merge that landed nothing on a queue-less base" +} + +test_github_failed_merge_never_claims_armed_auto_merge() { local case_dir rc + case_dir=$(make_case github-auto-merge-command-fails) + mkdir -p "$case_dir/wt" + add_gh_mocks_merge_fails "$case_dir" + write_github_outcome "$case_dir" OPEN false false main + : > "$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/67 -- --auto --merge \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "github-auto-merge-command-fails: the forge failure must still fail the wrapper" + assert_grep 'error: pr merge failed' "$case_dir/stderr" \ + "github-auto-merge-command-fails: the original forge error was masked" + assert_grep 'state=OPEN, merged=false, isInMergeQueue=false' "$case_dir/stderr" \ + "github-auto-merge-command-fails: refusal did not name the concrete observed state" + assert_no_grep 'armed' "$case_dir/stderr" \ + "github-auto-merge-command-fails: a failed merge command was reported as an armed auto-merge" + assert_grep 'auto-merge was requested for https://github.com/example/repo/pull/67' \ + "$case_dir/stderr" \ + "github-auto-merge-command-fails: the refusal never said auto-merge had only been requested" + assert_no_grep 'verified: ' "$case_dir/stdout" \ + "github-auto-merge-command-fails: a failed merge command was reported as verified" + pass "fm-pr-merge never reports auto-merge as armed when the merge command failed" +} + +test_github_failed_merge_with_queue_flags_never_claims_acceptance() { + local case_dir rc + case_dir=$(make_case github-failed-merge-queue-flags) + mkdir -p "$case_dir/wt" + add_gh_mocks_merge_fails "$case_dir" + 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" + + set +e + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/74 -- --auto --merge \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "github-failed-merge-queue-flags: the forge failure must still fail the wrapper" + assert_grep 'error: pr merge failed' "$case_dir/stderr" \ + "github-failed-merge-queue-flags: the original forge error was masked" + assert_grep 'state=OPEN, merged=false, isInMergeQueue=false' "$case_dir/stderr" \ + "github-failed-merge-queue-flags: refusal did not name the concrete observed state" + assert_no_grep 'was accepted with the exact flags' "$case_dir/stderr" \ + "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" \ + "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_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" +} + +test_github_accepted_queue_flags_do_not_echo_back_the_same_command() { + local case_dir rc + case_dir=$(make_case github-accepted-queue-flags) + mkdir -p "$case_dir/wt" + 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/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 \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + 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)' \ + "$case_dir/stderr" \ + "github-accepted-queue-flags: the refusal did not explain that the right flags were already used" + assert_grep "re-check the pull request's merge queue state" "$case_dir/stderr" \ + "github-accepted-queue-flags: the refusal named no concrete next step" + assert_no_grep 'retry with:' "$case_dir/stderr" \ + "github-accepted-queue-flags: the refusal echoed back the command that just refused" + assert_no_grep 'verified: ' "$case_dir/stdout" \ + "github-accepted-queue-flags: an unproved merge was reported as verified" + pass "fm-pr-merge does not echo back queue flags the caller already used" +} + +test_github_mismatched_queue_flags_still_name_the_retry() { + local case_dir rc + case_dir=$(make_case github-mismatched-queue-flags) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" 8282828282828282828282828282828282828282 + 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/69 -- --auto --merge \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + 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" \ + "github-mismatched-queue-flags: a caller method the queue does not use lost its retry guidance" + assert_grep '-- --auto --rebase' "$case_dir/stderr" \ + "github-mismatched-queue-flags: the exact compatible flags were not named" + 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 + case_dir=$(make_case github-unrecognised-queue-method) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" 8383838383838383838383838383838383838383 + write_github_outcome "$case_dir" OPEN false false main + printf 'merge_method=FASTFORWARD\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/70 \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "github-unrecognised-queue-method: an unproved merge must fail" + 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" + pass "fm-pr-merge names the queue requirement even when its method is unrecognised" +} + +test_github_unreadable_queue_rules_are_not_reported_as_no_queue() { + local case_dir rc + case_dir=$(make_case github-unreadable-queue-rules) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" 8484848484848484848484848484848484848484 + write_github_outcome "$case_dir" OPEN false false main + cat > "$case_dir/fakebin/gh" <<'SH' +#!/usr/bin/env bash +printf '%s\n' "$*" >> "$FM_TEST_GH_LOG" +case "${1:-} ${2:-}" in + "pr view") + case " $* " in + *headRefOid*) printf '%s\n' 8484848484848484848484848484848484848484 ; exit 0 ;; + esac + ;; + "api graphql") + cat "$FM_TEST_GH_OUTCOME" + exit 0 + ;; + api\ *) exit 1 ;; +esac +exit 0 +SH + chmod +x "$case_dir/fakebin/gh" + : > "$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-unreadable-queue-rules: an unproved merge must fail" + assert_grep 'the branch rules for base branch main could not be read' "$case_dir/stderr" \ + "github-unreadable-queue-rules: an unreadable rules response read like a queue-less base" + assert_no_grep 'retry with:' "$case_dir/stderr" \ + "github-unreadable-queue-rules: retry flags were named from rules nothing could read" + pass "fm-pr-merge distinguishes unreadable branch rules from a base with no merge queue" +} + +test_github_no_queue_rule_says_nothing_about_a_queue() { + local case_dir rc + case_dir=$(make_case github-no-queue-rule) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" 8585858585858585858585858585858585858585 + write_github_outcome "$case_dir" OPEN false false main + : > "$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 \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "github-no-queue-rule: an unproved merge must fail" + assert_grep 'state=OPEN, merged=false, isInMergeQueue=false' "$case_dir/stderr" \ + "github-no-queue-rule: refusal did not name the concrete observed state" + assert_no_grep 'merge queue' "$case_dir/stderr" \ + "github-no-queue-rule: a base with no queue rule was told it requires the merge queue" + pass "fm-pr-merge says nothing about a merge queue when the base branch has no queue rule" +} + +test_github_fallback_view_refusal_says_the_queue_was_unobservable() { + local case_dir ghless_path rc + case_dir=$(make_case github-fallback-unobservable-queue) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" 8686868686868686868686868686868686868686 + cat > "$case_dir/fakebin/gh-axi" <<'SH' +#!/usr/bin/env bash +printf '%s\n' "$*" >> "$FM_TEST_GH_AXI_LOG" +case "${1:-} ${2:-}" in + "pr merge") printf 'merged:\n number: %s\n status: ok\n' "${3:-}" ;; + "pr view") printf 'pull_request:\n number: %s\n state: open\n' "$3" ;; +esac +exit 0 +SH + chmod +x "$case_dir/fakebin/gh-axi" + rm "$case_dir/fakebin/gh" + ghless_path="$case_dir/path-without-gh" + mirror_path_without "$ghless_path" gh "$case_dir/fakebin" + : > "$case_dir/gh-axi.log" + + set +e + PATH="$ghless_path" run_pr_merge "$case_dir" task-x1 \ + https://github.com/example/repo/pull/73 -- --auto --merge \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "github-fallback-unobservable-queue: an unproved merge must fail" + assert_grep 'isInMergeQueue=unknown' "$case_dir/stderr" \ + "github-fallback-unobservable-queue: refusal did not name the concrete observed state" + assert_grep 'the merge queue could not be observed for https://github.com/example/repo/pull/73' \ + "$case_dir/stderr" \ + "github-fallback-unobservable-queue: the refusal implied an unqueued PR it could not see" + assert_grep "re-check the pull request's merge queue state" "$case_dir/stderr" \ + "github-fallback-unobservable-queue: the refusal named no concrete next step" + # The lowercase state the fallback view reports must be judged the same way + # the queue-aware read's uppercase enum is, or every explanation is skipped. + assert_grep 'auto-merge was requested and armed for https://github.com/example/repo/pull/73' \ + "$case_dir/stderr" \ + "github-fallback-unobservable-queue: the fallback view's state skipped the auto-merge explanation" + assert_no_grep 'verified: ' "$case_dir/stdout" \ + "github-fallback-unobservable-queue: an unproved merge was reported as verified" + pass "fm-pr-merge says the merge queue was unobservable when only the gh-axi view answered" +} + +test_github_unreadable_outcome_refusal_quotes_the_forge_output() { + local case_dir rc + case_dir=$(make_case github-unreadable-outcome-quotes-forge) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" 8787878787878787878787878787878787878787 + cat > "$case_dir/fakebin/gh-axi" <<'SH' +#!/usr/bin/env bash +printf '%s\n' "$*" >> "$FM_TEST_GH_AXI_LOG" +case "${1:-} ${2:-}" in + "pr merge") echo "will be added to the merge queue when all requirements are met" ;; + "pr view") exit 1 ;; +esac +exit 0 +SH + chmod +x "$case_dir/fakebin/gh-axi" + add_gh_mock_outcome_read_fails "$case_dir" 8787878787878787878787878787878787878787 + : > "$case_dir/gh-axi.log" + : > "$case_dir/gh.log" + + set +e + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/74 \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "github-unreadable-outcome-quotes-forge: an unreadable outcome must fail" + assert_grep 'could not read the GitHub pull request outcome after the merge attempt' \ + "$case_dir/stderr" \ + "github-unreadable-outcome-quotes-forge: the unreadable outcome was not reported" + assert_grep 'error: > will be added to the merge queue when all requirements are met' \ + "$case_dir/stderr" \ + "github-unreadable-outcome-quotes-forge: the forge's only evidence was discarded" + ! grep -qxF 'will be added to the merge queue when all requirements are met' \ + "$case_dir/stderr" \ + || fail "github-unreadable-outcome-quotes-forge: forge text was emitted as the wrapper's own line" + assert_no_grep 'verified: ' "$case_dir/stdout" \ + "github-unreadable-outcome-quotes-forge: an unproved merge was reported as verified" + assert_present "$case_dir/state/task-x1.check.sh" \ + "github-unreadable-outcome-quotes-forge: the attempted merge lost its merge poll" + pass "fm-pr-merge quotes the forge output when it cannot read the outcome either" +} + +test_github_failed_gh_read_falls_back_to_gh_axi() { + local case_dir rc + case_dir=$(make_case github-gh-read-falls-back) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" 5151515151515151515151515151515151515151 + add_gh_mock_outcome_read_fails "$case_dir" 5151515151515151515151515151515151515151 + : > "$case_dir/gh-axi.log" + : > "$case_dir/gh.log" + + set +e + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/63 \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 0 "$rc" "github-gh-read-falls-back: a merge the gh-axi view proves must succeed" + assert_grep 'pr view 63 --repo example/repo' "$case_dir/gh-axi.log" \ + "github-gh-read-falls-back: the gh-axi view was never consulted after gh's read failed" + assert_grep 'verified: https://github.com/example/repo/pull/63 is merged' \ + "$case_dir/stdout" "github-gh-read-falls-back: the proven merge was not reported" + assert_grep 'pr=https://github.com/example/repo/pull/63' "$case_dir/state/task-x1.meta" \ + "github-gh-read-falls-back: the merged PR was not recorded for teardown" + pass "fm-pr-merge falls back to the gh-axi view when gh's read fails" +} + +test_github_failed_merge_names_an_observed_landed_state() { + local case_dir rc + case_dir=$(make_case github-failed-merge-actually-landed) + mkdir -p "$case_dir/wt" + add_gh_mocks_merge_fails "$case_dir" + 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/64 \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "github-failed-merge-actually-landed: the forge failure must still fail the wrapper" + assert_grep 'error: pr merge failed' "$case_dir/stderr" \ + "github-failed-merge-actually-landed: the original forge error was masked" + assert_grep 'state=MERGED, merged=true, isInMergeQueue=false' "$case_dir/stderr" \ + "github-failed-merge-actually-landed: the observed landed state was never named" + assert_no_grep 'verified: ' "$case_dir/stdout" \ + "github-failed-merge-actually-landed: a failed merge command was reported as verified" + assert_grep 'pr=https://github.com/example/repo/pull/64' "$case_dir/state/task-x1.meta" \ + "github-failed-merge-actually-landed: the landed PR lost its reference" + pass "fm-pr-merge names a landed state hiding behind a failed GitHub merge command" +} + +test_github_without_gh_still_uses_gh_axi_merge() { + local case_dir ghless_path rc + case_dir=$(make_case github-without-gh) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" 4141414141414141414141414141414141414141 + rm "$case_dir/fakebin/gh" + ghless_path="$case_dir/path-without-gh" + mirror_path_without "$ghless_path" gh "$case_dir/fakebin" + : > "$case_dir/gh-axi.log" + + set +e + PATH="$ghless_path" run_pr_merge "$case_dir" task-x1 \ + https://github.com/example/repo/pull/60 \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 0 "$rc" "github-without-gh: gh-axi can prove a landed merge without gh" + assert_grep 'pr merge 60 --repo example/repo --squash' "$case_dir/gh-axi.log" \ + "github-without-gh: the configured merge abstraction was not invoked" + assert_grep 'pr view 60 --repo example/repo' "$case_dir/gh-axi.log" \ + "github-without-gh: the gh-axi fallback did not verify the landed state" + assert_grep 'verified: https://github.com/example/repo/pull/60 is merged' \ + "$case_dir/stdout" "github-without-gh: the fallback did not report the proven merge" + pass "fm-pr-merge reaches and verifies the gh-axi merge path without gh" +} + +test_github_without_gh_failed_read_keeps_bookkeeping() { + local case_dir ghless_path rc + case_dir=$(make_case github-without-gh-read-fails) + mkdir -p "$case_dir/wt" + cat > "$case_dir/fakebin/gh-axi" <<'SH' +#!/usr/bin/env bash +printf '%s\n' "$*" >> "$FM_TEST_GH_AXI_LOG" +case "${1:-} ${2:-}" in + "pr merge") exit 0 ;; + "pr view") exit 1 ;; +esac +exit 0 +SH + chmod +x "$case_dir/fakebin/gh-axi" + ghless_path="$case_dir/path-without-gh" + mirror_path_without "$ghless_path" gh "$case_dir/fakebin" + : > "$case_dir/gh-axi.log" + + set +e + PATH="$ghless_path" run_pr_merge "$case_dir" task-x1 \ + https://github.com/example/repo/pull/61 \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "github-without-gh-read-fails: an unreadable outcome must fail" + assert_grep 'pr merge 61 --repo example/repo --squash' "$case_dir/gh-axi.log" \ + "github-without-gh-read-fails: the merge call did not happen before the failed read" + assert_grep 'could not read the GitHub pull request outcome after the merge attempt' \ + "$case_dir/stderr" "github-without-gh-read-fails: the failed read was not reported" + assert_grep 'pr=https://github.com/example/repo/pull/61' "$case_dir/state/task-x1.meta" \ + "github-without-gh-read-fails: a landed merge lost its PR metadata" + assert_present "$case_dir/state/task-x1.check.sh" \ + "github-without-gh-read-fails: a landed merge lost its merge poll" + pass "fm-pr-merge preserves bookkeeping when gh is absent and the fallback read fails" +} + +test_github_zero_exit_queue_required_refuses_with_exact_retry() { + local case_dir rc + case_dir=$(make_case github-zero-exit-queue-required) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" 2121212121212121212121212121212121212121 + write_github_outcome "$case_dir" OPEN false false 'release/2026' + 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/56 \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "github-zero-exit-queue-required: an unproved merge must fail" + assert_grep 'state=OPEN, merged=false, isInMergeQueue=false' "$case_dir/stderr" \ + "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" \ + "github-zero-exit-queue-required: refusal did not name the exact compatible flags" + 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" \ + || fail "github-zero-exit-queue-required: the attempted merge was changed unexpectedly" + [ "$(wc -l < "$case_dir/gh-axi.log" | tr -d '[:space:]')" = 1 ] \ + || fail "github-zero-exit-queue-required: the wrapper attempted more than one merge" + assert_no_grep --auto "$case_dir/gh-axi.log" \ + "github-zero-exit-queue-required: queue flags were auto-applied to the attempted merge" + assert_grep 'pr=https://github.com/example/repo/pull/56' "$case_dir/state/task-x1.meta" \ + "github-zero-exit-queue-required: the attempted merge lost its PR reference" + assert_present "$case_dir/state/task-x1.check.sh" \ + "github-zero-exit-queue-required: the attempted merge did not leave its poll armed" + pass "fm-pr-merge reports exact queue retry flags after a zero-exit false success" +} + +test_github_closed_unqueued_outcome_omits_retry_flags() { + local case_dir rc + case_dir=$(make_case github-closed-unqueued) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" 2323232323232323232323232323232323232323 + write_github_outcome "$case_dir" CLOSED 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/57 \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "github-closed-unqueued: an unproved merge must fail" + assert_grep 'state=CLOSED, merged=false, isInMergeQueue=false' "$case_dir/stderr" \ + "github-closed-unqueued: refusal did not name the concrete observed state" + assert_no_grep 'requires the merge queue' "$case_dir/stderr" \ + "github-closed-unqueued: closed PR received unusable queue guidance" + assert_no_grep '-- --auto --merge' "$case_dir/stderr" \ + "github-closed-unqueued: closed PR received retry flags" + assert_grep 'pr=https://github.com/example/repo/pull/57' "$case_dir/state/task-x1.meta" \ + "github-closed-unqueued: the attempted merge lost its PR reference" + assert_present "$case_dir/state/task-x1.check.sh" \ + "github-closed-unqueued: the attempted merge did not leave its poll armed" + pass "fm-pr-merge omits merge-queue retry guidance for a closed GitHub PR" +} + +test_github_queued_outcome_is_verified() { + local case_dir rc + case_dir=$(make_case github-verified-queued) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" 3030303030303030303030303030303030303030 + write_github_outcome "$case_dir" OPEN false true master + : > "$case_dir/gh-axi.log" + : > "$case_dir/gh.log" + + set +e + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/53 -- --auto --merge \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 0 "$rc" "github-verified-queued: a queued PR should succeed" + assert_grep 'verified: https://github.com/example/repo/pull/53 is queued' \ + "$case_dir/stdout" "github-verified-queued: success was not reported as queued" + assert_no_grep 'merged:' "$case_dir/stdout" \ + "github-verified-queued: the forge CLI's unverified merged report leaked through" + assert_grep 'pr=https://github.com/example/repo/pull/53' "$case_dir/state/task-x1.meta" \ + "github-verified-queued: the queued PR was not recorded for teardown" + pass "fm-pr-merge accepts and accurately reports a GitHub merge-queue entry" +} + +test_github_queue_required_refusal_names_retry_flags() { + local case_dir rc + case_dir=$(make_case github-queue-required) + mkdir -p "$case_dir/wt" + add_gh_mocks_merge_fails "$case_dir" + 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/54 \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "github-queue-required: an incompatible direct merge must fail" + assert_grep 'error: pr merge failed' "$case_dir/stderr" \ + "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 \ + || fail "github-queue-required: refusal did not name the exact compatible flags" + 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" \ + "github-queue-required: the failed forge call did not leave the merge poll armed" + pass "fm-pr-merge explains how to retry with the required GitHub merge queue method" +} + +test_github_agreeing_queue_rules_keep_retry_guidance() { + local case_dir rc + case_dir=$(make_case github-agreeing-queue-rules) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" 2424242424242424242424242424242424242424 + write_github_outcome "$case_dir" OPEN false false main + printf 'merge_method=REBASE\nmerge_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/58 \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + 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" \ + "github-agreeing-queue-rules: agreeing rules omitted exact retry flags" + assert_no_grep 'exact retry flags are ambiguous' "$case_dir/stderr" \ + "github-agreeing-queue-rules: agreeing rules were reported as ambiguous" + pass "fm-pr-merge aggregates agreeing merge-queue rules" +} + +test_github_conflicting_queue_rules_report_ambiguity() { + local case_dir rc + case_dir=$(make_case github-conflicting-queue-rules) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" 2525252525252525252525252525252525252525 + write_github_outcome "$case_dir" OPEN false false main + printf 'merge_method=MERGE\nmerge_method=SQUASH\nmerge_method=SQUASH\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/59 \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "github-conflicting-queue-rules: an unproved merge must fail" + assert_grep 'base branch main has conflicting merge queue methods (MERGE, SQUASH)' \ + "$case_dir/stderr" \ + "github-conflicting-queue-rules: conflicting methods were not named" + assert_no_grep '-- --auto --merge' "$case_dir/stderr" \ + "github-conflicting-queue-rules: an exact retry method was guessed" + assert_no_grep '-- --auto --squash' "$case_dir/stderr" \ + "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" + pass "fm-pr-merge reports ambiguity for conflicting merge-queue rules" +} + +test_extra_merge_args_forwarded() { + local case_dir case_dir=$(make_case extra-args) mkdir -p "$case_dir/wt" add_gh_mocks "$case_dir" 2222222222222222222222222222222222222222 @@ -179,6 +1160,9 @@ test_missing_meta_refuses_before_merge() { pass "fm-pr-merge refuses before merging when task meta is missing" } +# This fork still addresses GitHub only: a well-formed GitLab merge request URL +# is refused explicitly, before any state is recorded, rather than sent to the +# wrong forge (docs/gitlab-merge-watch.md owns that boundary). test_malformed_url_refuses_before_merge() { local case_dir rc case_dir=$(make_case malformed-url) @@ -201,7 +1185,9 @@ test_malformed_url_refuses_before_merge() { "malformed-url: malformed PR URL armed a merge poll" assert_no_grep 'pr merge' "$case_dir/gh-axi.log" \ "malformed-url: gh-axi pr merge was invoked for a malformed URL" - pass "fm-pr-merge refuses malformed PR URLs before calling gh-axi" + assert_absent "$case_dir/state/.wake-queue" \ + "malformed-url: a refused GitLab URL left a landed outcome" + pass "fm-pr-merge refuses malformed and GitLab PR URLs before calling gh-axi" } test_rejects_unsafe_url_segments_before_recording() { @@ -232,28 +1218,31 @@ test_rejects_unsafe_url_segments_before_recording() { } test_repo_override_args_refuse_before_recording() { - local case_dir rc - case_dir=$(make_case repo-override) - mkdir -p "$case_dir/wt" - add_gh_mocks "$case_dir" 9999999999999999999999999999999999999999 - : > "$case_dir/gh-axi.log" + local case_dir rc override + for override in '--repo wrong/repo' '-R wrong/repo' '-yR'; do + case_dir=$(make_case "repo-override-${override//[^A-Za-z]/}") + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" 9999999999999999999999999999999999999999 + : > "$case_dir/gh-axi.log" - set +e - run_pr_merge "$case_dir" task-x1 https://github.com/right/repo/pull/5 -- --repo wrong/repo \ - > "$case_dir/stdout" 2> "$case_dir/stderr" - rc=$? - set -e + set +e + # shellcheck disable=SC2086 # Deliberate word-splitting of the override spelling. + run_pr_merge "$case_dir" task-x1 https://github.com/right/repo/pull/5 -- $override \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e - expect_code 1 "$rc" "repo-override: fm-pr-merge should refuse repo override flags" - assert_grep 'extra merge arguments must not override the repository' "$case_dir/stderr" \ - "repo-override: refusal did not explain the repo override" - assert_no_grep 'pr=https://github.com/right/repo/pull/5' "$case_dir/state/task-x1.meta" \ - "repo-override: PR URL was recorded before rejecting repo override" - assert_absent "$case_dir/state/task-x1.check.sh" \ - "repo-override: repo override armed a merge poll" - assert_no_grep 'pr merge' "$case_dir/gh-axi.log" \ - "repo-override: gh-axi pr merge was invoked despite repo override" - pass "fm-pr-merge refuses repo override args before recording state" + expect_code 1 "$rc" "repo-override ($override): fm-pr-merge should refuse repo override flags" + assert_grep 'extra merge arguments must not override the repository' "$case_dir/stderr" \ + "repo-override ($override): refusal did not explain the repo override" + assert_no_grep 'pr=https://github.com/right/repo/pull/5' "$case_dir/state/task-x1.meta" \ + "repo-override ($override): PR URL was recorded before rejecting repo override" + assert_absent "$case_dir/state/task-x1.check.sh" \ + "repo-override ($override): repo override armed a merge poll" + assert_no_grep 'pr merge' "$case_dir/gh-axi.log" \ + "repo-override ($override): gh-axi pr merge was invoked despite repo override" + done + pass "fm-pr-merge refuses repo override args, including bundled short clusters, before recording state" } test_explicit_merge_method_not_overridden() { @@ -301,8 +1290,297 @@ test_parses_pr_url_for_gh_axi() { pass "fm-pr-merge parses a GitHub PR URL into gh-axi number and --repo arguments" } -test_records_pr_and_head_before_merging +# --- durable merge outcome --------------------------------------------------- +# A merge that lands must leave a record outside the merging agent's memory. +# bin/fm-merge-outcome-lib.sh owns where that record goes; these cases pin the +# behavior through the real merge entrypoint. + +# make_home_case [ []]: a case dir whose home is a +# secondmate home bound to a parent, or a plain main home when no route is +# given. Echoes the case dir; the home is "$case_dir/home". +make_home_case() { + local name=$1 route=${2:-} parent=${3:-} case_dir home + case_dir=$(make_case "$name") + home="$case_dir/home" + mkdir -p "$home" "$case_dir/wt" + if [ -n "$route" ]; then + printf '%s\n' mate-x >"$home/.fm-secondmate-home" + { + printf 'schema=fm-secondmate-parent.v1\n' + printf 'route=%s\n' "$route" + [ "$route" != local ] || printf 'parent_home=%s\n' "$parent" + } >"$home/.fm-secondmate-parent" + fi + printf '%s\n' "$case_dir" +} + +parent_reply_lines() { # + grep -c -F "$2" "$1" 2>/dev/null || true +} + +test_secondmate_merge_reports_upward_once() { + local case_dir replies url + url=https://github.com/example/repo/pull/61 + case_dir=$(make_home_case secondmate-merge-reports remote) + add_gh_mocks "$case_dir" 5555555555555555555555555555555555555555 + : >"$case_dir/gh-axi.log" + replies="$case_dir/state/parent-replies.status" + + FM_TEST_HOME="$case_dir/home" run_pr_merge "$case_dir" task-x1 "$url" \ + >"$case_dir/stdout" 2>"$case_dir/stderr" || fail "secondmate-merge-reports: merge failed" + + assert_grep "done [key=merged-task-x1]: merged task-x1 $url" "$replies" \ + "secondmate-merge-reports: the landed PR was not reported upward" + [ "$(wc -l <"$replies")" -eq 1 ] \ + || fail "secondmate-merge-reports: one merge produced more than one upward line" + assert_absent "$case_dir/state/.wake-queue" \ + "secondmate-merge-reports: a secondmate home also wrote a main-home record" + + # The same merge again: the forge accepts it in this fixture, so only the + # at-most-once contract can keep the parent from being told twice. + FM_TEST_HOME="$case_dir/home" run_pr_merge "$case_dir" task-x1 "$url" \ + >"$case_dir/stdout2" 2>"$case_dir/stderr2" || fail "secondmate-merge-reports: repeat merge failed" + [ "$(parent_reply_lines "$replies" "$url")" -eq 1 ] \ + || fail "secondmate-merge-reports: a repeat merge of the same PR duplicated the upward line" + pass "a merge a secondmate home performs itself is reported upward exactly once" +} + +test_secondmate_merge_reports_on_the_local_route() { + local case_dir parent_status url + url=https://github.com/example/repo/pull/62 + case_dir=$(make_home_case secondmate-merge-local local "$TMP_ROOT/secondmate-merge-local/parent") + mkdir -p "$TMP_ROOT/secondmate-merge-local/parent/state" + add_gh_mocks "$case_dir" 6666666666666666666666666666666666666666 + : >"$case_dir/gh-axi.log" + parent_status="$TMP_ROOT/secondmate-merge-local/parent/state/mate-x.status" + + FM_TEST_HOME="$case_dir/home" run_pr_merge "$case_dir" task-x1 "$url" \ + >"$case_dir/stdout" 2>"$case_dir/stderr" || fail "secondmate-merge-local: merge failed" + + assert_grep "done [key=merged-task-x1]: merged task-x1 $url" "$parent_status" \ + "secondmate-merge-local: the landed PR did not reach the parent home's channel" + [ ! -e "$case_dir/state/parent-replies.status" ] \ + || fail "secondmate-merge-local: a local-route report also wrote the remote reply channel" + pass "a locally routed secondmate home reports the landed PR into its parent's own channel" +} + +test_failed_merge_reports_nothing() { + local case_dir rc + case_dir=$(make_home_case failed-merge-silent remote) + add_gh_mocks_merge_fails "$case_dir" + : >"$case_dir/gh-axi.log" + + set +e + FM_TEST_HOME="$case_dir/home" run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/63 \ + >"$case_dir/stdout" 2>"$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "failed-merge-silent: a failed merge should propagate" + assert_absent "$case_dir/state/parent-replies.status" \ + "failed-merge-silent: a merge that never landed was reported as landed" + assert_absent "$case_dir/state/task-x1.pr-poll-merge-notified" \ + "failed-merge-silent: a merge that never landed was marked as reported" + pass "a refused or failed merge reports no outcome" +} + +test_unproved_merge_reports_nothing() { + local case_dir rc + case_dir=$(make_home_case unproved-merge-silent remote) + add_gh_mocks "$case_dir" 5252525252525252525252525252525252525252 + write_github_outcome "$case_dir" OPEN false false main + : >"$case_dir/gh-axi.log" + + set +e + FM_TEST_HOME="$case_dir/home" run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/70 \ + >"$case_dir/stdout" 2>"$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "unproved-merge-silent: an unproved merge should refuse" + assert_absent "$case_dir/state/parent-replies.status" \ + "unproved-merge-silent: an unproved merge was reported upward as landed" + assert_absent "$case_dir/state/task-x1.pr-poll-merge-notified" \ + "unproved-merge-silent: an unproved merge was marked as reported" + assert_present "$case_dir/state/task-x1.check.sh" \ + "unproved-merge-silent: the refused merge did not leave its poll armed" + pass "a merge call the forge accepted but did not land reports no outcome" +} + +test_main_home_merge_leaves_a_durable_wake() { + local case_dir url + url=https://github.com/example/repo/pull/64 + case_dir=$(make_home_case main-merge-wake) + add_gh_mocks "$case_dir" 7777777777777777777777777777777777777777 + : >"$case_dir/gh-axi.log" + + FM_TEST_HOME="$case_dir/home" run_pr_merge "$case_dir" task-x1 "$url" \ + >"$case_dir/stdout" 2>"$case_dir/stderr" || fail "main-merge-wake: merge failed" + + assert_grep "$url" "$case_dir/state/.wake-queue" \ + "main-merge-wake: a merge this home performed left no durable record naming the PR" + [ "$(grep -c -F "$url" "$case_dir/state/.wake-queue")" -eq 1 ] \ + || fail "main-merge-wake: one merge produced more than one durable record" + grep -F "$(printf '\tcheck\tmerged-task-x1-%s\t' "$url")" "$case_dir/state/.wake-queue" >/dev/null \ + || fail "main-merge-wake: the durable record is not a check-kind row keyed by the PR" + assert_absent "$case_dir/state/parent-replies.status" \ + "main-merge-wake: a main home wrote a parent reply channel it does not have" + assert_present "$case_dir/state/task-x1.pr-poll-merge-notified" \ + "main-merge-wake: the canonical notification marker was not committed" + pass "a merge a main home performs itself leaves one durable check-kind wake naming the PR" +} + +test_queued_github_merge_leaves_the_poll_armed() { + local case_dir url + url=https://github.com/example/repo/pull/66 + case_dir=$(make_home_case queued-github-merge) + add_gh_mocks "$case_dir" 9999999999999999999999999999999999999999 + write_github_outcome "$case_dir" OPEN false true main + : >"$case_dir/gh-axi.log" + + FM_TEST_GH_MERGE_STATE=open FM_TEST_HOME="$case_dir/home" \ + run_pr_merge "$case_dir" task-x1 "$url" \ + >"$case_dir/stdout" 2>"$case_dir/stderr" \ + || fail "queued-github-merge: accepted merge command failed" + + assert_absent "$case_dir/state/.wake-queue" \ + "queued-github-merge: a queued merge was reported as landed" + [ -f "$case_dir/state/task-x1.check.sh" ] \ + || fail "queued-github-merge: the merge poll was not left armed" + [ ! -e "$case_dir/state/task-x1.pr-poll-merge-notified" ] \ + || fail "queued-github-merge: a queued merge was marked as reported" + pass "a queued GitHub merge stays silent and leaves confirmation to the armed poll" +} + +test_distinct_merged_prs_keep_distinct_wakes() { + local case_dir first_url second_url + first_url=https://github.com/example/repo/pull/68 + second_url=https://github.com/example/repo/pull/69 + case_dir=$(make_home_case distinct-merge-wakes) + add_gh_mocks "$case_dir" bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb + : >"$case_dir/gh-axi.log" + + FM_TEST_HOME="$case_dir/home" run_pr_merge "$case_dir" task-x1 "$first_url" \ + >"$case_dir/stdout-1" 2>"$case_dir/stderr-1" \ + || fail "distinct-merge-wakes: first merge failed" + rm -f "$case_dir/state/task-x1.check.sh" \ + "$case_dir/state/task-x1.pr-poll" \ + "$case_dir/state/task-x1.pr-poll-registration" + FM_TEST_HOME="$case_dir/home" run_pr_merge "$case_dir" task-x1 "$second_url" \ + >"$case_dir/stdout-2" 2>"$case_dir/stderr-2" \ + || fail "distinct-merge-wakes: second merge failed" + + [ "$(grep -c -F "$first_url" "$case_dir/state/.wake-queue")" -eq 1 ] \ + || fail "distinct-merge-wakes: first merge wake was missing or duplicated" + [ "$(grep -c -F "$second_url" "$case_dir/state/.wake-queue")" -eq 1 ] \ + || fail "distinct-merge-wakes: second merge wake was missing or duplicated" + FM_STATE_OVERRIDE="$case_dir/state" "$ROOT/bin/fm-wake-drain.sh" \ + >"$case_dir/drain.out" 2>"$case_dir/drain.err" \ + || fail "distinct-merge-wakes: wake drain failed" + assert_grep "$first_url" "$case_dir/drain.out" \ + "distinct-merge-wakes: queue deduplication collapsed the first PR" + assert_grep "$second_url" "$case_dir/drain.out" \ + "distinct-merge-wakes: queue deduplication collapsed the second PR" + pass "distinct merged PRs for one task retain distinct captain-facing wakes" +} + +test_uncommitted_marker_retry_is_never_silent() { + local case_dir url count + url=https://github.com/example/repo/pull/67 + case_dir=$(make_home_case uncommitted-wake-retry) + add_gh_mocks "$case_dir" aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + : >"$case_dir/gh-axi.log" + cat >"$case_dir/fakebin/mv" <<'SH' +#!/usr/bin/env bash +case "${!#}" in + *.pr-poll-merge-notified) + if mkdir "$FM_TEST_MARKER_FAILURE.claim" 2>/dev/null; then + exit 1 + fi + ;; +esac +exec "$FM_TEST_REAL_MV" "$@" +SH + chmod +x "$case_dir/fakebin/mv" + export FM_TEST_MARKER_FAILURE="$case_dir/marker-failure" + + FM_TEST_HOME="$case_dir/home" run_pr_merge "$case_dir" task-x1 "$url" \ + >"$case_dir/stdout-1" 2>"$case_dir/stderr-1" \ + || fail "uncommitted-wake-retry: landed merge was reported as failed" + assert_grep 'could not record the outcome' "$case_dir/stderr-1" \ + "uncommitted-wake-retry: failed marker commit was not loud" + [ -f "$case_dir/state/task-x1.check.sh" ] \ + || fail "uncommitted-wake-retry: failed commit disarmed the retry poll" + count=$(grep -c -F "$url" "$case_dir/state/.wake-queue") + [ "$count" -ge 1 ] \ + || fail "uncommitted-wake-retry: failed marker commit lost the durable outcome" + [ ! -e "$case_dir/state/task-x1.pr-poll-merge-notified" ] \ + || fail "uncommitted-wake-retry: failed marker commit was treated as complete" + + FM_TEST_HOME="$case_dir/home" run_pr_merge "$case_dir" task-x1 "$url" \ + >"$case_dir/stdout-2" 2>"$case_dir/stderr-2" \ + || fail "uncommitted-wake-retry: retry failed" + unset FM_TEST_MARKER_FAILURE + count=$(grep -c -F "$url" "$case_dir/state/.wake-queue") + [ "$count" -ge 1 ] \ + || fail "uncommitted-wake-retry: retry left the merge silent" + [ -f "$case_dir/state/task-x1.pr-poll-merge-notified" ] \ + || fail "uncommitted-wake-retry: retry did not commit the canonical marker" + pass "an uncommitted marker retry preserves at least one durable outcome" +} + +test_secondmate_without_parent_binding_is_loud() { + local case_dir rc url + url=https://github.com/example/repo/pull/65 + case_dir=$(make_home_case unbound-secondmate) + add_gh_mocks "$case_dir" 8888888888888888888888888888888888888888 + : >"$case_dir/gh-axi.log" + # A secondmate identity with no parent binding: exactly the seeding gap that + # lets a real merge land in silence. + printf '%s\n' mate-x >"$case_dir/home/.fm-secondmate-home" + + set +e + FM_TEST_HOME="$case_dir/home" run_pr_merge "$case_dir" task-x1 "$url" \ + >"$case_dir/stdout" 2>"$case_dir/stderr" + rc=$? + set -e + + expect_code 0 "$rc" "unbound-secondmate: the merge itself landed and must not be reported as failed" + assert_grep 'could not report it upward' "$case_dir/stderr" \ + "unbound-secondmate: a merge that could not be reported upward said nothing about it" + assert_absent "$case_dir/state/.wake-queue" \ + "unbound-secondmate: a secondmate home fell back to the main-home record" + pass "a secondmate home that cannot report upward says so instead of merging in silence" +} + +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_verified_merge_records_pr_and_head +test_pr_metadata_is_recorded_before_the_forge_call test_merge_failure_propagates_after_recording +test_github_open_unqueued_outcome_refuses +test_github_unreadable_outcome_keeps_pr_bookkeeping +test_github_refusal_quotes_the_forge_output +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_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 +test_github_auto_merge_without_queue_refuses_legibly +test_github_failed_merge_never_claims_armed_auto_merge +test_github_failed_merge_with_queue_flags_never_claims_acceptance +test_github_failed_gh_read_falls_back_to_gh_axi +test_github_failed_merge_names_an_observed_landed_state +test_github_without_gh_still_uses_gh_axi_merge +test_github_without_gh_failed_read_keeps_bookkeeping +test_github_merged_outcome_is_verified +test_github_verified_merge_requires_poll_recording +test_github_queued_outcome_is_verified +test_github_queue_required_refusal_names_retry_flags test_extra_merge_args_forwarded test_missing_meta_refuses_before_merge test_malformed_url_refuses_before_merge @@ -311,3 +1589,14 @@ test_repo_override_args_refuse_before_recording test_explicit_merge_method_not_overridden test_method_equals_merge_method_not_overridden test_parses_pr_url_for_gh_axi +test_secondmate_merge_reports_upward_once +test_secondmate_merge_reports_on_the_local_route +test_failed_merge_reports_nothing +test_unproved_merge_reports_nothing +test_main_home_merge_leaves_a_durable_wake +test_queued_github_merge_leaves_the_poll_armed +test_distinct_merged_prs_keep_distinct_wakes +test_uncommitted_marker_retry_is_never_silent +test_secondmate_without_parent_binding_is_loud + +echo "all fm-pr-merge tests passed"