diff --git a/README.md b/README.md index 7a2103c329c..405f099fa31 100644 --- a/README.md +++ b/README.md @@ -214,6 +214,7 @@ Firstmate's skills live in two separate places with different audiences: - [docs/codex-app-backend.md](docs/codex-app-backend.md) - the current blocked Codex App backend boundary and rollout contract. - [docs/verification/runtime-backends.md](docs/verification/runtime-backends.md) - active maintainer verification for runtime backend guarantees. - [docs/gitlab-merge-watch.md](docs/gitlab-merge-watch.md) - maintainer verification for watching and merging GitLab merge requests on arbitrary instances. +- [docs/forgejo-merge-watch.md](docs/forgejo-merge-watch.md) - maintainer verification for watching and merging Forgejo pull requests on arbitrary instances. - [docs/turnend-guard.md](docs/turnend-guard.md) - the primary session's current "no turn ends blind" backstop, scope, loop safety, and compatibility limits. - [docs/verification/supervision.md](docs/verification/supervision.md) - active maintainer verification for session-start, guard, continuity, and wedge integrations. - [docs/supervision-protocols/](docs/supervision-protocols/) - rendered primary-harness watcher protocols for Claude, Codex, OpenCode, Pi and `pi-signed`, Grok, Cursor, and unknown harness fallback. diff --git a/bin/fm-crew-state.sh b/bin/fm-crew-state.sh index 267b0902a93..c49ffa18da3 100755 --- a/bin/fm-crew-state.sh +++ b/bin/fm-crew-state.sh @@ -30,11 +30,17 @@ # 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 -# the active step is ci, `axi status` alone cannot tell "still waiting on -# checks" from "checks green, waiting on merge" (see nm_ci_checks_state) - -# a ci-step log-tail check overrides working -> done once checks read -# green, so a green PR is never silently read as still-validating. +# passed/checks-passed -> done, failed/cancelled -> failed. A terminal +# passed run reports a merge ONLY when its own pr step and every step it +# reports after that one completed; a skipped pr step means the run opened +# and merged nothing, and a completed pr step whose later step never ran +# means it opened a PR it never carried to a merge - the detail names the +# step that stopped short instead of asserting a merge (nm_passed_pr_detail). +# EXCEPT: while the active step is ci, `axi status` alone cannot tell "still +# waiting on checks" from "checks green, waiting on merge" (see +# nm_ci_checks_state) - a ci-step log-tail check overrides working -> done +# once checks read green, so a green PR is never silently read as +# still-validating. # 3. Reconcile the status log: if its last line says needs-decision/blocked but # the run-step shows the run moved on, the log is deterministically stale and # is flagged superseded. A genuinely parked run plus a needs-decision log @@ -291,15 +297,149 @@ log_reports_ci_ready() { esac } -nm_ci_step_status() { +# Every row of the steps[N]{step,status,findings,duration_ms} table, in the +# order `axi status` renders them - the pipeline's own step order, so a row's +# position is what says which steps ran after which. +# +# A row belongs to that table when it sits INSIDE it: after the `steps[N]{...}:` +# header and indented deeper than it, up to the first non-blank line that +# dedents back to the header's own level or shallower. A blank line does not +# close the section, and a deeper line carrying no leading `,` is skipped +# rather than emitted, so neither blank spacing nor a wrapped non-row line can +# end the table early or be read as a step. Every other table `axi status` +# renders is therefore excluded structurally, whatever its rows happen to +# contain - the findings table, whose first column is a finding id that can +# read `pr` exactly like a step name, and the separate +# active_steps{step,status,active_for,last_activity,agent_pid,round} table an +# ACTIVE run also renders. Nothing is lost by rejecting the latter, because the +# same output's steps table still carries that step as `ci,running,0,0`. +# +# Scoping by position rather than by column content is deliberate. Column +# content is an encoder choice: this reader once separated a step row from a +# findings row by requiring an unquoted numeric third column, which held only +# because v1.60.2's encoder quotes a numeric-looking string field, so a finding +# on file `123` rendered as `"123"` and fell out. That is an observation about +# one version's quoting, not a guarantee the format offers - and the step a +# merge claim depends on is exactly the one a finding can be named after. The +# table a row sits in is structural, so it cannot be changed out from under this +# reader by a quoting decision. +# +# The header is matched with `steps[` anchored to the start of the line so +# `active_steps[...]` cannot open the section. Within the section the status +# word itself is matched loosely: an unrecognized or newly added status must +# still be read and reported, not silently dropped. +# +# Verified by re-running this positional reader over every run in the local +# store: it reads 657 of 657 step rows across all 73 runs `no-mistakes axi +# status --run` renders with v1.60.2. Rendered by, not produced by - the shape +# is set by the binary printing the status, and most of those runs were written +# by an older version, so the scan says this reader handles what v1.60.2 prints +# for them too. That 657 is not the reader counting its own matches - +# a regex counting the rows it accepts verifies nothing - it is the per-run row +# count compared against the daemon's own step_results table as ground truth, +# with zero mismatched runs. All but one of those runs was terminal at scan +# time, so the active-run shape was settled by direct observation instead - +# `axi status` captured mid-step on live runs (v1.60.2, eb4e379, built +# 2026-08-29), sampled on three different running steps - three distinct step +# names, not three runs - renders the running step as `,running,0,0` in +# the steps table beside `,running,,...` in active_steps. Both +# that count and that shape are facts about one rendering version and one local +# store, not laws; the table boundary this reads is the part of the shape the +# format itself defines. +nm_step_rows() { + printf '%s\n' "$RUN_OUT" | awk ' + { + if (in_table) { + if ($0 ~ /^[[:space:]]*$/) next + match($0, /^[[:space:]]*/) + if (RLENGTH > header_indent) { + if ($0 ~ /^[[:space:]]*[^,[:space:]][^,]*,/) print + next + } + in_table = 0 + } + if ($0 ~ /^[[:space:]]*steps\[[0-9]+\]/ && $0 ~ /:[[:space:]]*$/) { + match($0, /^[[:space:]]*/) + header_indent = RLENGTH + in_table = 1 + } + } + ' +} + +# Status word of one named step row, or empty when the run output carries no +# such row. +nm_step_status() { # local row rest - row=$(printf '%s\n' "$RUN_OUT" | grep -E '^[[:space:]]*ci,[[:space:]]*"?(running|fixing)"?[[:space:]]*,' | head -1) + row=$(nm_step_rows | grep -E "^[[:space:]]*$1," | head -1) [ -n "$row" ] || return 0 row=$(trim "$row") rest=${row#*,} strip_quotes "$(trim "${rest%%,*}")" } +# The ci step's status, deliberately narrowed to the two ACTIVE values its +# caller acts on. Any other ci status (completed, skipped, pending) must read +# as empty here so nm_effective_ci_step_status still falls through to the +# top-level RUN_STATUS check below. +nm_ci_step_status() { + local step_status + step_status=$(nm_step_status ci) + case "$step_status" in + running|fixing) printf '%s' "$step_status" ;; + esac +} + +# What outcome=passed actually proves about the pull request. A run reaches +# outcome=passed once its steps finish without failing, which INCLUDES a run +# whose pr and ci steps were SKIPPED - what happens whenever no-mistakes cannot +# resolve the push provider - so nothing was ever opened or merged. Verified +# 2026-09-01 on no-mistakes v1.60.2: run 01M1EA5NJVP18AE7SPY5MBYW42 reported +# `pr,skipped,0,17` and `ci,skipped,0,16` under outcome=passed while the forge +# still had that branch's pull request open and unmerged, and firstmate reported +# the work as landed. +# +# A COMPLETED pr step is not that evidence on its own either: the pr step only +# OPENS the pull request. On merged run 01M1669Y82JTHWEBSG7PR2TKNH its pr.log +# ends at `created pull request: ` while the LATER ci step's log ends at +# `PR has been merged!`. The merge is therefore proven only when the pr row AND +# every step row the run reports after it read `completed`; any of them skipped, +# pending or absent means the run stopped short of a merge, and the detail names +# the step it stopped at rather than asserting one. `no-mistakes axi run --skip=ci` +# reaches exactly that shape - pr completed, ci skipped, outcome passed, PR +# still open. No step name after `pr` is written here: the steps a merge claim +# depends on are read from the run's own step order, because `axi status` +# carries no merge signal to key on directly (v1.60.2 emits the PR url as `pr:` +# and never the pr_state its store records beside it). +# This never asks the forge itself: fm-crew-state reports the RUN's state, and +# the forge remains the authority on merge state (bin/fm-pr-merge.sh). +nm_passed_pr_detail() { + local step_status after row rest step status + step_status=$(nm_step_status pr) + case "$step_status" in + completed) ;; + skipped) printf 'run passed, PR step skipped: no PR was opened or merged by the run, merge state unknown to the run'; return ;; + '') printf 'run passed, no PR step reported: merge state unknown to the run'; return ;; + *) printf 'run passed, PR step %s: merge state unknown to the run' "$step_status"; return ;; + esac + after=$(nm_step_rows | sed -n '/^[[:space:]]*pr,/,$p' | tail -n +2) + if [ -z "$after" ]; then + printf 'run passed, PR opened, no step reported after it: merge state unknown to the run' + return + fi + while IFS= read -r row; do + row=$(trim "$row") + [ -n "$row" ] || continue + step=$(trim "${row%%,*}") + rest=${row#*,} + status=$(strip_quotes "$(trim "${rest%%,*}")") + [ "$status" = completed ] && continue + printf 'run passed, PR opened but %s step %s: merge state unknown to the run' "$step" "$status" + return + done <<< "$after" + printf 'run passed: PR merged/closed' +} + nm_effective_ci_step_status() { local step_status if [ "${RUN_STATUS:-}" = fixing ]; then @@ -319,18 +459,20 @@ nm_effective_ci_step_status() { # Root cause of the PR #252 incident (2026-07): for a repo where merge is left # to the captain, no-mistakes' ci step (and therefore top-level status/outcome) # stays "running" for the ENTIRE CI-monitor phase, including long after GitHub -# reports every check green - it only reaches outcome=passed once the PR is -# actually merged (or failed/cancelled if closed). `axi status`'s steps[] table -# never distinguishes "still waiting on checks" from "checks green, waiting on -# merge": both read as plain `ci,running,...`. The only place that transition is -# recorded is the ci step's own log text, e.g. "all CI checks passed - still -# monitoring until merged or closed" or "no CI checks reported - still -# monitoring until merged or closed" (verified against 360+ real run logs under -# ~/.no-mistakes/logs/*/ci.log on the installed v1.32.2 binary, including the -# actual PR #252 run). Reads the ci step's log tail via `axi logs` and scans it -# for the MOST RECENT recognized marker (the log is append-only/chronological, -# so the last match is current): green with nothing red after it means CI is -# green right now, still only waiting on merge/close. +# reports every check green: the ci step keeps monitoring until that PR is +# merged or closed. What a terminal outcome=passed does and does not prove about +# the PR is owned by nm_passed_pr_detail above, not restated here. `axi status`'s +# steps[] table never distinguishes "still waiting on checks" from "checks green, +# waiting on merge": both read as plain `ci,running,...`. The only place that +# transition is recorded is the ci step's own log text, e.g. "all CI checks +# passed - still monitoring until merged or closed" or "no CI checks reported - +# still monitoring until merged or closed" (verified against 360+ real run logs +# under ~/.no-mistakes/logs/*/ci.log on the installed v1.32.2 binary, including +# the actual PR #252 run). Reads the ci step's log tail via `axi logs` and scans +# it for the MOST RECENT recognized marker (the log is append-only and +# chronological, so the last match is current): +# green with nothing red after it means CI is green right now, still only +# waiting on merge/close. nm_ci_checks_state() { local run_id log_tail marker run_id=$(strip_quotes "$(nm_field id)") @@ -499,7 +641,7 @@ if [ "$HAVE_RUN" = 1 ]; then if [ -n "$outcome" ]; then case "$outcome" in - passed) RUN_STATE="done"; RUN_DETAIL="run passed: PR merged/closed" ;; + passed) RUN_STATE="done"; RUN_DETAIL="$(nm_passed_pr_detail)" ;; checks-passed) RUN_STATE="done"; RUN_DETAIL="checks green: PR ready for review" ;; failed) RUN_STATE=failed; RUN_DETAIL="run failed" ;; cancelled) RUN_STATE=failed; RUN_DETAIL="run cancelled" ;; diff --git a/bin/fm-pr-check.sh b/bin/fm-pr-check.sh index 99b3e025db2..fc8fad1d798 100755 --- a/bin/fm-pr-check.sh +++ b/bin/fm-pr-check.sh @@ -3,8 +3,9 @@ # exact pr_head= when available, then atomically arm a static merge poll. # The watcher check source is byte-for-byte bin/fm-pr-poll.sh; task and PR data # live only in a private sidecar and are never interpolated into shell source. -# A GitHub pull request URL and a GitLab merge request URL are both accepted, -# including a merge request on a self-hosted GitLab instance. +# A GitHub pull request URL, a GitLab merge request URL, and a Forgejo pull +# request URL are all accepted, including a request on a self-hosted GitLab or +# Forgejo instance. # Usage: fm-pr-check.sh set -eu @@ -51,26 +52,32 @@ fm_pr_poll_retirement_recover_one "$STATE" "$ID" "$SCRIPT_DIR/fm-pr-poll.sh" || exit 1 } -# Refuse to arm a GitLab watch with no glab on PATH. The poll is silent on -# every error by design, so a missing CLI would be indistinguishable from a -# merge request that is never merged. Arming is the one point where that can be +# Refuse to arm a self-hosted watch with no CLI to read it on PATH. The poll is +# silent on every error by design, so a missing CLI would be indistinguishable +# from a request that is never merged. Arming is the one point where that can be # reported, so the absent tool stops the watch here instead of watching nothing. if [ "$PROVIDER" = gitlab ] && ! command -v glab >/dev/null 2>&1; then echo "error: watching a GitLab merge request requires glab on PATH" >&2 exit 1 fi +if [ "$PROVIDER" = forgejo ] && ! command -v forgejo-axi >/dev/null 2>&1; then + echo "error: watching a Forgejo pull request requires forgejo-axi on PATH" >&2 + exit 1 +fi "$FM_ROOT/bin/fm-guard.sh" || true # pr_head is recorded only when the forge's CLI can supply it. gh exposes the -# head commit as a selectable field; plain glab exposes it only inside its JSON -# output, which would need a JSON processor firstmate does not require, so a -# GitLab task records no pr_head. Both consumers already treat it as optional: -# bin/fm-teardown.sh reads the head from the forge at teardown rather than from -# metadata and falls back to its provider-agnostic content check, and -# bin/fm-review-diff.sh resolves the head from the remote when none is recorded. -# bin/fm-pr-merge.sh reads a GitLab head live at merge time for the same reason, -# and treats a recorded value that disagrees as stale rather than authoritative. +# head commit as a selectable field and forgejo-axi exposes it as a selectable +# field too; plain glab exposes it only inside its JSON output, which would need +# a JSON processor firstmate does not require, so a GitLab task records no +# pr_head. Every consumer already treats it as optional: bin/fm-teardown.sh +# reads the head from the forge at teardown rather than from metadata and falls +# back to its provider-agnostic content check, and bin/fm-review-diff.sh +# resolves the head from the remote when none is recorded. +# bin/fm-pr-merge.sh reads a self-hosted head live at merge time for the same +# reason, and treats a recorded value that disagrees as stale rather than +# authoritative. WT=$(grep '^worktree=' "$META" | tail -1 | cut -d= -f2- || true) PR_HEAD= if [ "$PROVIDER" = github ] && [ -n "$WT" ] && [ -d "$WT" ] && command -v gh >/dev/null 2>&1; then @@ -79,6 +86,21 @@ if [ "$PROVIDER" = github ] && [ -n "$WT" ] && [ -d "$WT" ] && command -v gh >/d PR_HEAD=$REMOTE_HEAD fi fi +# forgejo-axi needs no repository on disk, because --base-url and the parsed +# owner/repository address the instance the URL names rather than an ambient +# default. A field the host does not supply is omitted rather than emitted as +# null, so requiring exactly one head_sha line refuses an absent value instead +# of recording an empty one. +if [ "$PROVIDER" = forgejo ]; then + if REMOTE_HEAD=$(forgejo-axi pr view --base-url "https://$HOST" --repo "$PROJECT_PATH" \ + "$NUMBER" --fields head_sha 2>/dev/null | awk ' + $1 == "head_sha:" { count++; value = $2 } + END { if (count == 1 && value != "") print value; else exit 1 } + ') \ + && fm_pr_head_valid "$REMOTE_HEAD"; then + PR_HEAD=$REMOTE_HEAD + fi +fi META_TMP= META_LOCK= diff --git a/bin/fm-pr-lib.sh b/bin/fm-pr-lib.sh index d9580dc9b4a..2dd7385ece6 100755 --- a/bin/fm-pr-lib.sh +++ b/bin/fm-pr-lib.sh @@ -4,13 +4,13 @@ # constructing task paths or performing any side effect. # # The stored identity is provider-tagged: provider, url, host, path, number. -# "path" is the full project path, which is owner/repository on GitHub and an -# arbitrarily nested group/subgroup/project namespace on GitLab. A GitLab -# project can sit at any depth, so no owner/repository pair can address one and -# the sidecar carries the whole path instead. GitLab also runs on self-hosted -# instances, so the host is part of that identity rather than a constant. Every -# consumer re-derives the identity from the stored URL and refuses any record -# whose parts do not reconstruct that exact URL. +# "path" is the full project path, which is owner/repository on GitHub and +# Forgejo and an arbitrarily nested group/subgroup/project namespace on GitLab. +# A GitLab project can sit at any depth, so no owner/repository pair can address +# one and the sidecar carries the whole path instead. GitLab and Forgejo both +# run on self-hosted instances, so the host is part of that identity rather than +# a constant. Every consumer re-derives the identity from the stored URL and +# refuses any record whose parts do not reconstruct that exact URL. # # A validated exact merged result is retired through a private receipt only # after its durable wake is appended. @@ -109,19 +109,16 @@ fm_task_id_creation_valid() { [ "${#id}" -le 64 ] } -# GitLab serves self-hosted instances, so the host is part of the identity -# rather than a constant. It is accepted only as a lowercase DNS name with no -# userinfo, port, or trailing dot, which keeps one canonical spelling per MR. -# github.com is refused here even though its shape is otherwise valid: it is -# GitHub's own host and never a GitLab instance, so a URL like -# https://github.com/o/r/-/merge_requests/1 (a typo'd or spoofed GitHub URL) -# would otherwise be armed as a GitLab watch that can never succeed. -fm_pr_gitlab_host_valid() { +# A self-hosted provider carries its host in the identity rather than as a +# constant, and both of them accept it only as a lowercase DNS name with no +# userinfo, port, or trailing dot, which keeps one canonical spelling per +# request. That shared shape is stated once here; each provider then refuses the +# well-known hosts that belong to another provider on top of it. +fm_pr_dns_host_valid() { local host=${1-} label local LC_ALL=C local -a labels [ "${#host}" -ge 1 ] && [ "${#host}" -le 253 ] || return 1 - [ "$host" != github.com ] || return 1 case "$host" in .*|*.|*..*|*[!a-z0-9.-]*) return 1 ;; esac @@ -134,6 +131,66 @@ fm_pr_gitlab_host_valid() { done } +# github.com is refused here even though its shape is otherwise valid: it is +# GitHub's own host and never a GitLab instance, so a URL like +# https://github.com/o/r/-/merge_requests/1 (a typo'd or spoofed GitHub URL) +# would otherwise be armed as a GitLab watch that can never succeed. +fm_pr_gitlab_host_valid() { + local host=${1-} + [ "$host" != github.com ] || return 1 + fm_pr_dns_host_valid "$host" +} + +# Forgejo is self-hosted with no host of its own, so the same refusal covers +# both of the hosts that are definitively somebody else's: neither +# https://github.com/o/r/pulls/1 nor https://gitlab.com/g/p/pulls/1 can name a +# Forgejo pull request, and arming either one would watch something that can +# never merge. Nothing else is excluded, because any other host may run Forgejo. +fm_pr_forgejo_host_valid() { + local host=${1-} + [ "$host" != github.com ] && [ "$host" != gitlab.com ] || return 1 + fm_pr_dns_host_valid "$host" +} + +# Forgejo owner names are at most 40 characters, start with an alphanumeric, +# and otherwise allow "-", "_", and "."; the forge additionally refuses a run of +# two or more of those three and refuses to end on one. A repository name is at +# most 100 characters over the same character set and is never one of the exact +# names the forge reserves, ".", ".." or "-", nor ends in one of its reserved +# route suffixes. The forge lowercases a name before comparing it against those +# reserved words, so the suffixes are matched here without regard to case. +# There is deliberately no rule against a run of "-", "_" or "." inside a +# repository name: that restriction is the forge's username rule, enforced above +# for the owner, and applying it to a repository would refuse one the forge can +# genuinely host. +# +# The forge's reserved USERNAME list is deliberately not encoded for the owner. +# It is forge policy that varies by version, so a copy here would rot into +# refusing owners the forge accepts. "-" is a different kind of thing: it is the +# route separator and can never name a hostable repository. +# +# bin/fm-pr-poll.sh re-validates these same rules rather than trusting its +# sidecar, so a change here needs the matching change there. +fm_pr_forgejo_owner_valid() { + local owner=${1-} + local LC_ALL=C + [ "${#owner}" -ge 1 ] && [ "${#owner}" -le 40 ] || return 1 + case "$owner" in + [!A-Za-z0-9]*|*[!A-Za-z0-9._-]*|*[-._]) return 1 ;; + *--*|*-.*|*-_*|*.-*|*..*|*._*|*_-*|*_.*|*__*) return 1 ;; + esac +} + +fm_pr_forgejo_repo_valid() { + local repo=${1-} + local LC_ALL=C + [ "${#repo}" -ge 1 ] && [ "${#repo}" -le 100 ] || return 1 + case "$repo" in + .|..|-|*[!A-Za-z0-9._-]*) return 1 ;; + *.[gG][iI][tT]|*.[wW][iI][kK][iI]|*.[rR][sS][sS]|*.[aA][tT][oO][mM]) return 1 ;; + esac +} + # A GitLab project path is group[/subgroup...]/project, so at least two # segments and no fixed depth. GitLab reserves "-" as its route separator and # forbids a leading hyphen, ".git", and ".atom", so none of those can name a @@ -158,15 +215,22 @@ fm_pr_gitlab_path_valid() { # Parse a canonical PR or MR URL into the provider-tagged identity. Validation # is strict and per provider: the GitHub username and repository rules are -# unchanged, and GitLab gets its own host and namespace rules rather than a -# loosened GitHub rule. +# unchanged, GitLab gets its own host and namespace rules rather than a loosened +# GitHub rule, and Forgejo gets its own again. No provider's accepted set is +# widened to make room for another one. # -# FM_PR_OWNER and FM_PR_REPO are additionally set for github because -# bin/fm-pr-merge.sh addresses GitHub by owner/repository. A gitlab URL leaves -# them empty, and that path addresses the project by FM_PR_HOST and FM_PR_PATH -# instead, so a merge request on any instance resolves without a hardcoded host. +# The three forms are told apart by the segment that cannot appear in the other +# two: GitHub anchors on its own host and "/pull/", GitLab on "/-/merge_requests/", +# and Forgejo on the plural "/pulls/". The plural is what a Forgejo pull request +# URL actually carries, so a GitHub URL misspelled that way matches no provider +# rather than being read as a Forgejo one on a host GitHub owns. +# +# FM_PR_OWNER and FM_PR_REPO are additionally set for github and forgejo, whose +# projects are an owner/repository pair that bin/fm-pr-merge.sh addresses +# directly. A gitlab URL leaves them empty, because a GitLab project sits at any +# depth under its groups and only FM_PR_HOST and FM_PR_PATH can address one. fm_pr_url_parse() { - local raw=${1-} pattern host path + local raw=${1-} pattern host path owner repo local LC_ALL=C FM_PR_PROVIDER= FM_PR_URL= @@ -195,16 +259,41 @@ fm_pr_url_parse() { # "/-/merge_requests/". Any earlier separator therefore lands inside the # captured path, where the reserved "-" segment is refused. pattern='^https://([a-z0-9.-]{1,253})/([A-Za-z0-9._/-]+)/-/merge_requests/([1-9][0-9]*)$' + if [[ "$raw" =~ $pattern ]]; then + host=${BASH_REMATCH[1]} + path=${BASH_REMATCH[2]} + fm_pr_gitlab_host_valid "$host" || return 1 + fm_pr_gitlab_path_valid "$path" || return 1 + FM_PR_PROVIDER=gitlab + FM_PR_URL=$raw + FM_PR_HOST=$host + FM_PR_PATH=$path + FM_PR_NUMBER=${BASH_REMATCH[3]} + return 0 + fi + # A Forgejo project is exactly one owner and one repository, so the two + # segments are captured separately and neither class contains "/". A URL with + # any other depth before "/pulls/" therefore matches nothing rather than + # folding extra segments into an owner or repository name. + pattern='^https://([a-z0-9.-]{1,253})/([A-Za-z0-9._-]{1,40})/([A-Za-z0-9._-]{1,100})/pulls/([1-9][0-9]*)$' [[ "$raw" =~ $pattern ]] || return 1 host=${BASH_REMATCH[1]} - path=${BASH_REMATCH[2]} - fm_pr_gitlab_host_valid "$host" || return 1 - fm_pr_gitlab_path_valid "$path" || return 1 - FM_PR_PROVIDER=gitlab + owner=${BASH_REMATCH[2]} + repo=${BASH_REMATCH[3]} + fm_pr_forgejo_host_valid "$host" || return 1 + fm_pr_forgejo_owner_valid "$owner" || return 1 + fm_pr_forgejo_repo_valid "$repo" || return 1 + FM_PR_PROVIDER=forgejo FM_PR_URL=$raw FM_PR_HOST=$host - FM_PR_PATH=$path - FM_PR_NUMBER=${BASH_REMATCH[3]} + FM_PR_PATH="$owner/$repo" + # Consumed by bin/fm-pr-merge.sh and bin/fm-pr-check.sh, which address a + # Forgejo instance by --base-url and its repository by owner/repository. + # shellcheck disable=SC2034 + FM_PR_OWNER=$owner + # shellcheck disable=SC2034 + FM_PR_REPO=$repo + FM_PR_NUMBER=${BASH_REMATCH[4]} } fm_pr_head_valid() { diff --git a/bin/fm-pr-merge.sh b/bin/fm-pr-merge.sh index 3e61b33f7bc..bfd58b4d648 100755 --- a/bin/fm-pr-merge.sh +++ b/bin/fm-pr-merge.sh @@ -4,7 +4,9 @@ # The full canonical URL is parsed by bin/fm-pr-lib.sh. A GitHub pull request is # addressed through gh-axi by the derived owner and repository; a GitLab merge # request is addressed through glab by the project URL rebuilt from the parsed -# host and path, so any instance works and no host is hardcoded. +# host and path, and a Forgejo pull request through forgejo-axi by that host as +# --base-url plus the derived owner and repository, so any instance of either +# self-hosted forge works and no host is hardcoded. # # Merge method on GitHub defaults to --squash when the caller passes none of # --squash, --merge, --rebase, or --method after the optional -- separator. @@ -40,6 +42,15 @@ # GitLab adds no method flag at all: its merge method is the project's own # setting, which the merge API applies, and imposing squash there would override # that convention rather than mirror the GitHub default. +# Forgejo has no such convention to preserve: its merge API requires the method +# in the request body, so some method is always chosen. This script therefore +# names the same --squash default GitHub gets, rather than inheriting +# forgejo-axi's own "merge" default, and a caller who wants another one passes +# -- --method merge|rebase. GitHub's --squash, --merge and --rebase spellings +# are not flags forgejo-axi takes, so they are refused by name before anything +# is recorded rather than suppressing that default and failing at the CLI. +# A repository that disallows the chosen method fails the merge loudly at the +# forge instead of landing something else. # # A GitLab merge is refused unless every pre-merge condition holds, each read # live at merge time rather than taken from recorded metadata: the merge request @@ -53,14 +64,29 @@ # recorded value stale. Reading that state needs glab and jq, and either one # absent stops the merge before any state is recorded. # +# A Forgejo merge is refused the same way, from one live mergeability read taken +# at merge time: the pull request must be mergeable by the forge's own judgement +# and its checks must pass at the current head, with every failing condition and +# the forge's own reasons reported rather than only the first. The verified head +# is then passed to forgejo-axi as --expected-head, which re-reads it before the +# merge, sends it as the request's head_commit_id, and refuses to report a merge +# it cannot prove landed at that exact commit. A recorded pr_head that disagrees +# with the live head is reported rather than trusted. Reading that state needs +# forgejo-axi and jq, and either one absent stops the merge before any state is +# recorded. +# # 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, nor --sha on GitLab because the head comes only from the live read. +# URL, nor --sha on GitLab or --expected-head on Forgejo because the head comes +# only from the live read, nor --base-url on Forgejo because the instance comes +# only from the URL, nor --squash, --merge or --rebase on Forgejo because that +# CLI names the method only as --method. # -# On GitLab, this script confirms the MR is actually merged before reporting it; -# an auto-merge-queued or unconfirmed request leaves the poll armed and records -# no landed outcome. bin/fm-merge-outcome-lib.sh owns a confirmed merge's -# destination, normal-case deduplication, and at-least-once recovery. +# On GitLab and Forgejo, this script confirms the request is actually merged +# before reporting it; an auto-merge-queued or unconfirmed request leaves the +# poll armed and records no landed outcome. +# bin/fm-merge-outcome-lib.sh owns a confirmed merge's destination, normal-case +# deduplication, and at-least-once recovery. # A landed merge whose outcome cannot be written is reported loudly rather than # misreported as a failed merge. # Usage: fm-pr-merge.sh [-- ] @@ -100,6 +126,11 @@ PR_NUMBER=$FM_PR_NUMBER # glab resolves the instance from the project URL passed to -R, so the host is # rebuilt from the parsed identity rather than read from any ambient default. PROJECT_URL="https://$FM_PR_HOST/$FM_PR_PATH" +# forgejo-axi resolves the instance from --base-url. It accepts a pull request +# URL too, but reads only owner/repository/number out of one and still sends the +# request to whatever host its own configuration resolves, so the host is passed +# explicitly rather than left to an ambient default. +BASE_URL="https://$FM_PR_HOST" shift 2 [ "${1:-}" = "--" ] && shift @@ -173,11 +204,14 @@ reject_repo_overrides() { done } +# The flag each provider's CLI takes for the head commit, rejected in extra +# arguments because the head comes only from this run's own live read. reject_head_overrides() { - local arg + local flag=$1 arg + shift for arg in "$@"; do case "$arg" in - --sha|--sha=*) + "$flag"|"$flag"=*) echo "error: extra merge arguments must not override the head commit" >&2 return 1 ;; @@ -185,8 +219,43 @@ reject_head_overrides() { done } +reject_base_url_overrides() { + local arg + for arg in "$@"; do + case "$arg" in + --base-url|--base-url=*) + echo "error: extra merge arguments must not override the forge instance" >&2 + return 1 + ;; + esac + done +} + +# forgejo-axi names the merge method only as --method . GitHub's own +# --squash, --merge and --rebase spellings are refused by name here rather than +# forwarded, because forgejo-axi rejects them as unknown flags after the live +# pre-merge read has already run, and they would first suppress this path's own +# --method squash default. +reject_github_merge_methods() { + local arg + for arg in "$@"; do + case "$arg" in + --squash|--merge|--rebase) + printf 'error: extra merge arguments must name a Forgejo merge method as --method squash|merge|rebase, not %s\n' \ + "$arg" >&2 + return 1 + ;; + esac + done +} + reject_repo_overrides "$@" || exit 1 -[ "$PROVIDER" != gitlab ] || reject_head_overrides "$@" || exit 1 +[ "$PROVIDER" != gitlab ] || reject_head_overrides --sha "$@" || exit 1 +if [ "$PROVIDER" = forgejo ]; then + reject_head_overrides --expected-head "$@" || exit 1 + reject_base_url_overrides "$@" || exit 1 + reject_github_merge_methods "$@" || exit 1 +fi # Task-derived paths are constructed only after the canonical ID validation. META="$STATE/$ID.meta" @@ -210,10 +279,22 @@ if [ "$PROVIDER" = gitlab ]; then fi fi +FORGEJO_MISSING= +if [ "$PROVIDER" = forgejo ]; then + command -v forgejo-axi >/dev/null 2>&1 || FORGEJO_MISSING="forgejo-axi" + if ! command -v jq >/dev/null 2>&1; then + FORGEJO_MISSING="${FORGEJO_MISSING:+$FORGEJO_MISSING and }jq" + fi + if [ -n "$FORGEJO_MISSING" ]; then + echo "error: merging a Forgejo pull request requires $FORGEJO_MISSING on PATH" >&2 + exit 1 + fi +fi + # The recorded head is read before bin/fm-pr-check.sh rewrites the metadata, # because that script re-records pr= and drops a pr_head= it cannot resolve. RECORDED_HEAD= -if [ "$PROVIDER" = gitlab ]; then +if [ "$PROVIDER" = gitlab ] || [ "$PROVIDER" = forgejo ]; then RECORDED_HEAD=$(grep '^pr_head=' "$META" | tail -1 | cut -d= -f2- || true) fi @@ -318,6 +399,132 @@ FIELDS FM_PR_MERGE_HEAD=$live_head } +# Pre-merge conditions for a Forgejo pull request, read from one live +# mergeability view. Sets FM_PR_MERGE_HEAD to the verified head on success and +# returns non-zero after reporting every condition that failed. +forgejo_verify_mergeable() { + local json fields line + local total=0 named=0 refusals='' + local number='' view_url='' live_head='' forge_mergeable='' checks_pass='' mergeable='' reasons='' + + if ! json=$(forgejo-axi pr mergeability --base-url "$BASE_URL" \ + --repo "$PR_OWNER/$PR_REPO" "$PR_NUMBER" --json 2>/dev/null) \ + || [ -z "$json" ]; then + echo "error: could not read the Forgejo pull request state before merging" >&2 + return 1 + fi + # One named field per line, the same shape the GitLab read uses. reasons is + # joined into one line because it is reported rather than compared, and a + # reason carrying a newline would otherwise split into a line no name matches + # and fail the exact-field-count check below. + if ! fields=$(printf '%s' "$json" | jq -r ' + if type == "object" and (.mergeability | type == "object") then + .mergeability | + "number=" + ((.number // "") | tostring), + "url=" + ((.url // "") | tostring), + "head=" + ((.head_sha // "") | tostring), + "forge_mergeable=" + (.forgejo_mergeable | tostring), + "checks_pass=" + (.checks_pass | tostring), + "mergeable=" + (.mergeable | tostring), + "reasons=" + ((.reasons // []) | map(tostring | gsub("[\r\n]"; " ")) | join(", ")) + else + error("mergeability payload is not an object") + end' 2>/dev/null); then + echo "error: could not read the Forgejo pull request state before merging" >&2 + return 1 + fi + while IFS= read -r line; do + total=$((total + 1)) + case "$line" in + number=*) number=${line#number=} ;; + url=*) view_url=${line#url=} ;; + head=*) live_head=${line#head=} ;; + forge_mergeable=*) forge_mergeable=${line#forge_mergeable=} ;; + checks_pass=*) checks_pass=${line#checks_pass=} ;; + mergeable=*) mergeable=${line#mergeable=} ;; + reasons=*) reasons=${line#reasons=} ;; + *) continue ;; + esac + named=$((named + 1)) + done <&2 + return 1 + fi + + # The forge's own answer has to be about the pull request this run named, so a + # view that came back for another one refuses instead of authorizing a merge. + if [ "$number" != "$PR_NUMBER" ] || [ "$view_url" != "$URL" ]; then + printf 'error: the Forgejo pull request state read back as %s #%s, not %s\n' \ + "${view_url:-}" "${number:-}" "$URL" >&2 + return 1 + fi + if ! fm_pr_head_valid "$live_head"; then + echo "error: could not read the Forgejo pull request head commit before merging" >&2 + return 1 + fi + # A rebase moves the head and leaves the recorded value behind, so the + # disagreement is reported and the live head is what gets verified and merged. + if [ -n "$RECORDED_HEAD" ] && [ "$RECORDED_HEAD" != "$live_head" ]; then + printf 'notice: recorded head %s disagrees with the live head %s; verifying the live head\n' \ + "$RECORDED_HEAD" "$live_head" >&2 + fi + + [ "$forge_mergeable" = true ] \ + || refusals="$refusals - the forge reports mergeable as \"${forge_mergeable:-unreadable}\", not true +" + [ "$checks_pass" = true ] \ + || refusals="$refusals - the checks at head $live_head report passing as \"${checks_pass:-unreadable}\", not true +" + [ "$mergeable" = true ] \ + || refusals="$refusals - the merged verdict is \"${mergeable:-unreadable}\", not true +" + if [ -n "$refusals" ]; then + printf 'error: refusing to merge %s\n' "$URL" >&2 + printf '%s' "$refusals" >&2 + [ -z "$reasons" ] || printf 'error: - the forge names: %s\n' "$reasons" >&2 + return 1 + fi + printf 'verified: %s is mergeable with passing checks at head %s\n' "$URL" "$live_head" >&2 + FM_PR_MERGE_HEAD=$live_head +} + +# Confirm a Forgejo pull request actually landed after the merge command +# returned. forgejo-axi already refuses to report a merge it cannot prove, so +# this read is the independent second opinion: an unreadable answer is reported +# as unconfirmed rather than as a failure, which leaves the merge poll armed. +forgejo_confirm_merged() { + local json merged url + if ! json=$(forgejo-axi pr merged --base-url "$BASE_URL" \ + --repo "$PR_OWNER/$PR_REPO" "$PR_NUMBER" --json 2>/dev/null) || [ -z "$json" ]; then + printf 'actionable: Forgejo accepted the merge for %s but its landed state could not be confirmed; the merge poll remains armed\n' \ + "$URL" >&2 + return 2 + fi + if ! merged=$(printf '%s' "$json" | jq -r \ + 'if type == "object" and (.proof | type == "object") and (.proof.merged | type == "boolean") then .proof.merged | tostring else error("invalid proof") end' \ + 2>/dev/null) \ + || ! url=$(printf '%s' "$json" | jq -r \ + 'if type == "object" and (.proof | type == "object") and (.proof.url | type == "string") then .proof.url else error("invalid proof") end' \ + 2>/dev/null); then + printf 'actionable: Forgejo accepted the merge for %s but its landed state could not be confirmed; the merge poll remains armed\n' \ + "$URL" >&2 + return 2 + fi + [ "$url" = "$URL" ] || { + printf 'actionable: Forgejo accepted the merge for %s but answered for %s instead; the merge poll remains armed\n' \ + "$URL" "$url" >&2 + return 2 + } + [ "$merged" = true ] || { + printf 'actionable: Forgejo accepted the merge for %s but reads back as not merged; the merge poll remains armed\n' \ + "$URL" >&2 + return 2 + } +} + # 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 @@ -684,6 +891,23 @@ case "$PROVIDER" in gitlab_confirm_merged || gitlab_confirm_rc=$? [ "$gitlab_confirm_rc" -eq 0 ] || exit 0 ;; + forgejo) + forgejo_verify_mergeable || exit 1 + forgejo_args=() + if ! caller_has_merge_method "$@"; then + forgejo_args=(--method squash) + fi + # --expected-head binds the merge to the head this run verified: forgejo-axi + # re-reads it before merging, sends it as the request's head_commit_id, and + # refuses to report a merge it cannot prove landed at that exact commit, so a + # push that lands in between is refused instead of merged unverified. + forgejo-axi pr merge --base-url "$BASE_URL" --repo "$PR_OWNER/$PR_REPO" \ + "$PR_NUMBER" --expected-head "$FM_PR_MERGE_HEAD" \ + "${forgejo_args[@]+"${forgejo_args[@]}"}" "$@" + forgejo_confirm_rc=0 + forgejo_confirm_merged || forgejo_confirm_rc=$? + [ "$forgejo_confirm_rc" -eq 0 ] || exit 0 + ;; *) echo "error: invalid PR merge request" >&2 exit 2 diff --git a/bin/fm-pr-poll.sh b/bin/fm-pr-poll.sh index ed705ce7073..e82ace7c814 100755 --- a/bin/fm-pr-poll.sh +++ b/bin/fm-pr-poll.sh @@ -4,8 +4,9 @@ # otherwise, including on every error, so a failed lookup can never be read as # a merge. The provider-tagged identity is data in the sidecar and is never # interpolated into this source: these bytes are identical for every task. -# Each provider is read through its own standard CLI, gh for GitHub and glab -# for GitLab, so an upstream checkout needs no extra tooling to follow either. +# Each provider is read through its own standard CLI, gh for GitHub, glab for +# GitLab, and forgejo-axi for Forgejo, so an upstream checkout needs no extra +# tooling to follow any of them. set -u LC_ALL=C export LC_ALL @@ -105,6 +106,47 @@ case "$provider" in state=$(printf '%s\n' "$raw" | sed -n 's/^state:[[:space:]]*//p' | head -1) || exit 0 [ "$state" = merged ] && printf '%s\n' merged ;; + forgejo) + [ "${#host}" -ge 1 ] && [ "${#host}" -le 253 ] || exit 0 + [ "$host" != github.com ] && [ "$host" != gitlab.com ] || exit 0 + case "$host" in + .*|*.|*..*|*[!a-z0-9.-]*) exit 0 ;; + esac + # A Forgejo project is exactly one owner and one repository, so the stored + # path splits once and neither half may contain a further separator. + owner=${path%%/*} + repo=${path#*/} + [ "$owner/$repo" = "$path" ] || exit 0 + [ "${#owner}" -ge 1 ] && [ "${#owner}" -le 40 ] || exit 0 + case "$owner" in + [!A-Za-z0-9]*|*[!A-Za-z0-9._-]*|*[-._]) exit 0 ;; + *--*|*-.*|*-_*|*.-*|*..*|*._*|*_-*|*_.*|*__*) exit 0 ;; + esac + # These rules are the ones bin/fm-pr-lib.sh applies; they are repeated here + # because this poll re-validates its sidecar rather than trusting it, so a + # change to either side needs the matching change to the other. "-" is one + # of the exact names the forge reserves, and it lowercases a name before + # comparing, so the reserved suffixes are matched without regard to case. + [ "${#repo}" -ge 1 ] && [ "${#repo}" -le 100 ] || exit 0 + case "$repo" in + .|..|-|*[!A-Za-z0-9._-]*) exit 0 ;; + *.[gG][iI][tT]|*.[wW][iI][kK][iI]|*.[rR][sS][sS]|*.[aA][tT][oO][mM]) exit 0 ;; + esac + [ "$url" = "https://$host/$owner/$repo/pulls/$number" ] || exit 0 + # --base-url is what binds this read to the instance the record names. + # forgejo-axi takes a pull request URL too, but it reads only the + # owner/repository/number out of one and still sends the request to whatever + # host its own configuration resolves, so passing the URL alone would let an + # ambient default answer for the host in the record. + # Its errors exit non-zero and carry no "merged:" line of their own, so an + # unreachable instance stays silent instead of reporting a merge. + raw=$(forgejo-axi pr merged --base-url "https://$host" --repo "$owner/$repo" "$number" 2>/dev/null) || exit 0 + state=$(printf '%s\n' "$raw" | awk ' + $1 == "merged:" { count++; value = $2 } + END { if (count == 1 && value != "") print value; else exit 1 } + ') || exit 0 + [ "$state" = true ] && printf '%s\n' merged + ;; *) exit 0 ;; esac exit 0 diff --git a/docs/architecture.md b/docs/architecture.md index aa7284dc02b..1c0fd93d526 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -294,9 +294,11 @@ PR-based task merges go through `bin/fm-pr-merge.sh`, which records `pr=` and an The helper requires a full canonical URL and rejects malformed URLs or repo override flags before recording merge state. A `https://github.com///pull/` URL invokes `gh-axi pr merge --repo /`, defaults to `--squash`, and preserves explicit merge-method flags. A `https:////-/merge_requests/` URL (see [docs/gitlab-merge-watch.md](gitlab-merge-watch.md)) invokes `glab mr merge -R https:///`, so the instance comes from the URL, and adds no merge-method flag because the project's own merge method applies. -That path merges only after one live read of the merge request confirms it is open, mergeable, conflict-free, with blocking discussions resolved and a successful pipeline at the current head, and it binds the merge to that verified head; recorded metadata is never the authority for those conditions because a rebase leaves it stale. +A `https://///pulls/` URL (see [docs/forgejo-merge-watch.md](forgejo-merge-watch.md)) invokes `forgejo-axi pr merge --base-url https:// --repo /`, so that instance also comes from the URL, and names `--method squash` by default because Forgejo's merge API takes the method rather than applying the repository's own, refusing GitHub's `--squash`, `--merge` and `--rebase` spellings because that CLI names the method only as `--method`. +Both self-hosted paths merge only after one live read confirms the request is mergeable at its current head, and bind the merge to that verified head; recorded metadata is never the authority for those conditions because a rebase leaves it stale. +The GitLab read additionally requires the merge request open, conflict-free, with blocking discussions resolved and a successful pipeline at that head, and the Forgejo read requires the forge's own mergeable verdict and passing checks at it, with every failing condition reported rather than only the first. After either forge command returns, the script confirms the PR or MR actually landed, and only a confirmed landing records a landed outcome; a queued or unconfirmed request records none and leaves its poll armed. -On GitLab an auto-merge-queued or unconfirmed request is reported without failing the run. +On GitLab and Forgejo an auto-merge-queued or unconfirmed request is reported without failing the run. On GitHub an outcome that is neither merged nor queued is refused loudly and non-zero, naming the observed state, and a base branch that requires the merge queue is refused with the concrete retry flags its configured method requires rather than having a merge method chosen on the caller's behalf. When the forge already accepted exactly those flags and the pull request still has not entered the queue, that refusal points at the queue state to re-check instead of echoing back the flags the caller just ran. An auto-merge request is held to the same standard: `--auto` that leaves the pull request neither merged nor queued is refused rather than reported as success. diff --git a/docs/documentation-audiences.json b/docs/documentation-audiences.json index 8f4a4265e43..70f391db95d 100644 --- a/docs/documentation-audiences.json +++ b/docs/documentation-audiences.json @@ -340,6 +340,10 @@ "path": "docs/fm-test-portable-shards.md", "audience": "maintainer-verification" }, + { + "path": "docs/forgejo-merge-watch.md", + "audience": "maintainer-verification" + }, { "path": "docs/gitlab-merge-watch.md", "audience": "maintainer-verification" diff --git a/docs/forgejo-merge-watch.md b/docs/forgejo-merge-watch.md new file mode 100644 index 00000000000..c04946a7c96 --- /dev/null +++ b/docs/forgejo-merge-watch.md @@ -0,0 +1,229 @@ +# Forgejo pull request watch and merge verification + +Empirical record for the merge watch and the merge path on Forgejo, alongside the existing GitHub and GitLab ones. +Every command below was run on 2026-08-28. +Every output is reproduced exactly. + +## Versions + +``` +$ forgejo-axi --version +1.3.0 + +$ jq --version +jq-1.8.1 + +$ bash --version | head -1 +GNU bash, version 5.3.9(1)-release (x86_64-redhat-linux-gnu) +``` + +## The evidence instance + +All live evidence here reads , the public Forgejo instance Forgejo itself is developed on. +Every command against it reads a public pull request and needs no credential, so a reader can rerun each one and see the same output. + +Two pull requests in `forgejo/forgejo` are used, both in a terminal state that cannot change: + +- `pulls/1000` is merged, so it is the one outcome that must produce a wake. +- `pulls/14141` is closed without being merged, so it is an outcome that must never produce one. + +A non-default host appears below only as the placeholder `forgejo.example`, which resolves nowhere. + +``` +$ forgejo-axi status --base-url https://codeberg.org +host: + url: "https://codeberg.org" + api_url: "https://codeberg.org/api/v1" +auth: + configured: false + authenticated: false + source: null +server: + version: 16.0.0-dev-714-11075108+gitea-1.22.0 +``` + +## Why the host is data rather than a constant + +Forgejo is self-hosted and has no host of its own, so a pull request can live under any host. +The stored record therefore carries `provider`, `url`, `host`, `path`, and `number`, exactly as the GitLab record does, and every consumer rebuilds the URL from those parts and refuses any record that does not reconstruct the stored URL exactly. +A Forgejo project is always one owner and one repository, so `path` is an `owner/repository` pair and `bin/fm-pr-merge.sh` can address it directly, which a GitLab namespace at arbitrary depth never allows. + +## Why `--base-url` is passed rather than the pull request URL + +`forgejo-axi` accepts a pull request URL, but it reads only the owner, repository, and number out of one and still sends the request to whatever host its own configuration resolves. +Passing the URL alone would therefore let an ambient default answer for the host the record names. +A `forgejo.example` URL answered from Codeberg proves it: + +``` +$ forgejo-axi pr merged --base-url https://codeberg.org https://forgejo.example/forgejo/forgejo/pulls/1000 +proof: + merged: true + number: 1000 + url: "https://codeberg.org/forgejo/forgejo/pulls/1000" + head_sha: 355add5cd7a842beb494cf9afad05f0f59331aa7 + merge_commit_sha: 355add5cd7a842beb494cf9afad05f0f59331aa7 + merged_at: "2023-07-10T09:30:05+02:00" + merged_by: caesar +``` + +Both the poll and the merge path therefore pass the host from the validated record as `--base-url` and the parsed pair as `--repo`, and never hand a URL to the CLI at all. + +## Why the plural `pulls` matters + +A Forgejo pull request URL carries `/pulls/`, while GitHub's carries the singular `/pull/`. +That single segment is what tells the two apart on a host that is not `github.com`, so each provider keeps its own spelling and neither accepts the other's. +`github.com` and `gitlab.com` are additionally refused as Forgejo hosts even though their shape is otherwise valid: they are those forges' own hosts and never a Forgejo instance, so `https://github.com/o/r/pulls/1` would otherwise arm a watch that can never succeed. +`tests/fm-pr-check-security.test.sh` covers the plural, the singular, both cross-provider spoofs, and the Forgejo owner and repository name rules. + +## End to end: arming and polling a real pull request + +Three tasks were armed, two against Codeberg and one against the placeholder host: + +``` +$ fm-pr-check.sh e1 https://codeberg.org/forgejo/forgejo/pulls/1000 +armed: state/e1.check.sh +$ fm-pr-check.sh e2 https://codeberg.org/forgejo/forgejo/pulls/14141 +armed: state/e2.check.sh +$ fm-pr-check.sh e3 https://forgejo.example/my-org/tools/pulls/7 +armed: state/e3.check.sh +``` + +The stored record for each, showing the host and the owner/repository pair as data: + +``` +$ cat state/e1.pr-poll +forgejo +https://codeberg.org/forgejo/forgejo/pulls/1000 +codeberg.org +forgejo/forgejo +1000 + +$ cat state/e3.pr-poll +forgejo +https://forgejo.example/my-org/tools/pulls/7 +forgejo.example +my-org/tools +7 +``` + +The provenance record for the non-default host, on the same version tag the other providers use: + +``` +$ cat state/e3.pr-poll-registration +fm-pr-poll-registration-v2 +e3 +forgejo +https://forgejo.example/my-org/tools/pulls/7 +forgejo.example +my-org/tools +7 +8141a5928e52af1e2b80ab27b594635583bbb13f935900f5105e1873e79142aa +4df03972907646e0d88f3ff1c98bc2cef30858cea409d372743b9aab41af1bfc +49:410182 +49:410183 +``` + +A Forgejo task records `pr_head=` too, because `forgejo-axi` exposes the head commit as a selectable field and needs no repository on disk to read it: + +``` +$ cat state/e1.meta +window=fm-e1 +kind=ship +pr=https://codeberg.org/forgejo/forgejo/pulls/1000 +pr_head=355add5cd7a842beb494cf9afad05f0f59331aa7 +``` + +Running each published poll the way the watcher does, where an empty result means the poll stayed silent and produced no wake: + +``` +$ fm-pr-poll.sh --validated $(tr '\n' ' ' < state/e1.pr-poll) +merged +$ fm-pr-poll.sh --validated $(tr '\n' ' ' < state/e2.pr-poll) +$ fm-pr-poll.sh --validated $(tr '\n' ' ' < state/e3.pr-poll) +``` + +The merged pull request produces exactly one `merged` line. +The closed one produces nothing, and the unreachable placeholder host produces nothing rather than a false merge. + +The same bytes work in the watcher's sidecar-driven mode, where the published check locates its own record: + +``` +$ state/e1.check.sh +merged +``` + +## A missing CLI produces no wake, never a false merge + +The poll is silent on every error by design, so a missing `forgejo-axi` would otherwise be indistinguishable from a pull request that is never merged. +With `forgejo-axi` removed from `PATH`, the poll stays silent even for the pull request that is genuinely merged: + +``` +$ PATH="$nofj" fm-pr-poll.sh --validated $(tr '\n' ' ' < state/e1.pr-poll) +``` + +Arming is the one point where that can be reported, so it refuses there instead of arming a watch that can never fire: + +``` +$ PATH="$nofj" fm-pr-check.sh e4 https://codeberg.org/forgejo/forgejo/pulls/1000 +error: watching a Forgejo pull request requires forgejo-axi on PATH +$ echo $? +1 +``` + +## Merging a pull request + +`bin/fm-pr-merge.sh` merges a Forgejo pull request through the shared recording helper and one live pre-merge read. +Every run below used a throwaway `FM_HOME`, so no live task record was touched, and a `forgejo-axi` wrapper that refused any `pr merge` subcommand outright, so no merge could reach the forge even if a check were wrong. + +A closed pull request is refused, and every failing condition is listed rather than just the first, followed by the forge's own reasons: + +``` +$ fm-pr-merge.sh e2 https://codeberg.org/forgejo/forgejo/pulls/14141 +armed: state/e2.check.sh +error: refusing to merge https://codeberg.org/forgejo/forgejo/pulls/14141 + - the forge reports mergeable as "false", not true + - the checks at head 910556df46e2bfb2f8547c57568ef45bc5f47683 report passing as "false", not true + - the merged verdict is "false", not true +error: - the forge names: forgejo_not_mergeable, checks_failure +``` + +A pull request whose state cannot be read at all is refused before any merge is attempted. +`pulls/1000` merged in 2023 and its base branch is long gone, so the branch-protection route the mergeability read needs answers 404: + +``` +$ fm-pr-merge.sh e1 https://codeberg.org/forgejo/forgejo/pulls/1000 +armed: state/e1.check.sh +error: could not read the Forgejo pull request state before merging +$ echo $? +1 +``` + +Both refusals came after `pr=` was recorded and the merge poll was armed, exactly as a failing `gh-axi pr merge` does on the GitHub side, so a refusal still leaves the audit trail and the watch in place. + +A repository whose pull requests report no passing checks therefore cannot merge through this path. +That is the intended reading of the requirement rather than an oversight, and it matches the GitLab path, where a project that runs no pipeline cannot merge either: passing checks at the head is a condition, and "there are no checks" does not satisfy it. + +The remaining refusal conditions, the merge itself, and the confirmation that follows it are covered by `tests/fm-pr-merge.test.sh` against fixtures. +The merge is not exercised against any live pull request, in either direction: `forgejo-axi pr merge` has no dry run, so a live success path would mean merging someone's work to produce evidence. + +## Why the head is read live and bound to the merge + +The verified head is passed to `forgejo-axi pr merge --expected-head`, which re-reads the pull request before merging, refuses when the head has moved, sends that commit as the merge request's `head_commit_id`, and refuses to report a merge it cannot prove landed at that exact commit. +Without it, a push landing between the read and the merge would merge commits nothing verified. +A recorded `pr_head=` that no longer matches the live head is reported and the live head is what gets verified, because a rebase moves the head and leaves the recorded value stale. + +## Why a merge method is named here and not on GitLab + +GitLab's merge API applies the project's own merge method, so imposing one there would override that convention. +Forgejo's merge API takes the method in the request body and has no such fallback, so some method is always chosen. +This path therefore passes `--method squash`, the same squash default GitHub gets, rather than inheriting `forgejo-axi`'s own `merge` default, and a caller who wants another one passes `-- --method merge` or `-- --method rebase`. +`forgejo-axi pr merge` takes no `--squash`, `--merge` or `--rebase` flag, so those GitHub spellings are refused by name before anything is recorded rather than suppressing the default and failing at the CLI after the live pre-merge read has already run. +A repository that disallows the chosen method fails the merge loudly at the forge instead of landing something else. + +## Known limits of this integration + +A Forgejo instance served under a path prefix, such as `https://example.test/forge/owner/repo/pulls/1`, is refused rather than misparsed. +The canonical form this path accepts is exactly `https://///pulls/`, and an owner or repository name outside Forgejo's own rules is refused with it. + +`forgejo-axi pr mergeability` reads commit statuses and branch protection, not Actions runs. +An instance that does not serve the Actions runs API is therefore fully supported here, because nothing in this path asks for it. diff --git a/docs/gitlab-merge-watch.md b/docs/gitlab-merge-watch.md index 0483b0e5557..25c850ec6d7 100644 --- a/docs/gitlab-merge-watch.md +++ b/docs/gitlab-merge-watch.md @@ -44,6 +44,7 @@ That is deliberate: the host-agnostic property is a property of the stored recor GitLab runs mostly on self-hosted instances, so a merge request can live under any host. A GitLab project also sits under at least one group at no fixed depth, so no owner-and-repository pair can address one the way it can on GitHub. The stored record therefore carries `provider`, `url`, `host`, `path`, and `number`, and every consumer rebuilds the URL from those parts and refuses any record that does not reconstruct the stored URL exactly. +Neither `bin/fm-pr-lib.sh` nor `bin/fm-pr-poll.sh` names `gitlab.com` as the GitLab host; the string appears in them only where the Forgejo provider refuses it as a host belonging to another forge (see [forgejo-merge-watch.md](forgejo-merge-watch.md)). `tests/fm-pr-check-security.test.sh` proves the host-agnostic path through a non-default-host sidecar and verifies that `glab` receives the reconstructed project URL. ## How plain glab is invoked, and why @@ -267,7 +268,7 @@ It skips only that prompt; the conditions above are what authorize the merge. ## Why a recorded head is not the authority -`bin/fm-pr-check.sh` records `pr_head=` only for GitHub, where `gh` exposes the head commit as a selectable field. +`bin/fm-pr-check.sh` records no `pr_head=` for GitLab, because plain `glab` exposes the head commit only inside its JSON output rather than as a selectable field; that script's header owns which providers do record one. It is optional by design, and the other consumers already treat it that way: `bin/fm-teardown.sh` reads the head from the forge at teardown and falls back to its provider-agnostic content check, and `bin/fm-review-diff.sh` resolves the head from the remote when none is recorded. The merge path does not record one either, and deliberately does not depend on one. diff --git a/docs/scripts.md b/docs/scripts.md index 9b33fced128..6a4af56dc3b 100644 --- a/docs/scripts.md +++ b/docs/scripts.md @@ -126,7 +126,7 @@ 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, merge-notification identity, and retirement | | `fm-pr-poll.sh` | Provide the byte-static watcher program for validated PR/MR-poll sidecars | | `fm-pr-check.sh` | Record validated `pr=` and `pr_head=` values, then atomically arm a static merge poll | -| `fm-pr-merge.sh` | Record PR metadata, merge a task's canonical full GitHub or GitLab URL, then refuse an outcome it cannot prove landed or queued | +| `fm-pr-merge.sh` | Record PR metadata, merge a task's canonical full GitHub, GitLab, or Forgejo URL, then refuse an outcome it cannot prove landed or queued | | `fm-merge-outcome-lib.sh` | Publish a confirmed merge's durable, role-routed supervision outcome | | `fm-parent-channel-lib.sh` | Resolve a secondmate home's parent channel and append a captain-facing outcome line to it at most once | | `fm-promote.sh` | Promote a scout task in place to a protected ship task with an explicit delivery mode, and write the ship instructions carrying that mode's definition of done | diff --git a/tests/fm-crew-state.test.sh b/tests/fm-crew-state.test.sh index a284cbe8eb6..d28ad9c9583 100755 --- a/tests/fm-crew-state.test.sh +++ b/tests/fm-crew-state.test.sh @@ -278,6 +278,61 @@ outcome: passed EOF } +# outcome=passed with the pr and ci steps SKIPPED - the exact shape a run takes +# when no-mistakes cannot resolve the push provider, so it never opened or +# merged a PR. Copied from the real `no-mistakes axi status --run +# 01M1EA5NJVP18AE7SPY5MBYW42` output on v1.60.2 (2026-09-01), which reported +# this while the forge still had that branch's pull request open and unmerged. +# Note the absent `pr:` field, exactly as the real run emits it. +run_passed_pr_skipped() { # + cat < + cat < cat < + cat </dev/null + fm_write_meta "$d/state/feat-ciactive.meta" "window=fm:fm-feat-ciactive" "worktree=$d/wt" "kind=ship" + FM_FAKE_AXI_STATUS="$(run_ci_monitoring_with_active_steps fm/feat-ciactive)" + FM_FAKE_CI_LOGS="all CI checks passed - still monitoring until merged or closed" + local out; out=$(run_crew_state "$d" feat-ciactive) + assert_contains "$out" "state: done" "an active_steps table must not hide the ci step's own steps row" + assert_contains "$out" "source: run-step" "the active run is still read from the run step" + assert_contains "$out" "checks green" "the green-checks override still fires for an active run" + assert_not_contains "$out" "state: working" "a green PR must not read as still validating" + pass "an active run's second active_steps table does not disturb the step-row reader" +} + test_top_level_ci_checks_green_surfaces_done() { reset_fakes local d; d=$(new_case top-level-ci-green) @@ -671,6 +788,235 @@ test_terminal_passed() { pass "terminal passed run is authoritative" } +# A run reaches outcome=passed whenever its steps finish without failing, which +# INCLUDES a run whose pr and ci steps were skipped for want of a resolvable +# push provider. Such a run pushed nothing to a pull request and merged +# nothing, so the reported detail must not claim a merge. +test_terminal_passed_pr_skipped_claims_no_merge() { + reset_fakes + local d; d=$(new_case passed-pr-skipped) + make_repo_on_branch "$d/wt" fm/feat-d-skipped + make_fakebin "$d" >/dev/null + fm_write_meta "$d/state/feat-d-skipped.meta" "window=fm:fm-feat-d-skipped" "worktree=$d/wt" "kind=ship" + FM_FAKE_AXI_STATUS="$(run_passed_pr_skipped fm/feat-d-skipped)" + local out; out=$(run_crew_state "$d" feat-d-skipped) + assert_contains "$out" "source: run-step" "passed with skipped pr -> run-step source" + assert_not_contains "$out" "PR merged" "skipped pr step must not claim a merged PR" + assert_not_contains "$out" "merged/closed" "skipped pr step must not claim a merged-or-closed PR" + assert_contains "$out" "run passed, PR step skipped: no PR was opened or merged by the run, merge state unknown to the run" \ + "skipped pr step must report the whole claim, scoped to what the run itself did" + pass "outcome=passed with a skipped pr step does not claim a merge" +} + +# The findings table is emitted BEFORE the steps table, so a findings row whose +# id is `pr` sits above the genuine pr step row and is matched first by any +# reader that scans the whole run output for a row shape. That collision makes +# the reported PR step status the finding's SEVERITY. This fixture is that +# collision - a `pr` finding above a genuine `pr,skipped` step row - and its +# file column is parameterized because a findings row can take more than one +# shape there. +# +# `a.go` is the ordinary shape. `123` is a bare numeric file column: on the +# encoder shape observed for v1.60.2 a numeric-looking string field renders +# quoted, so that row is not one this version emits today, but nothing in the +# reader's contract makes that quoting a guarantee. Both must be rejected +# because of where the row SITS in the output rather than what its columns +# happen to contain, so a change in how the encoder quotes cannot turn a +# finding into a step. +run_passed_findings_row_shadowing_pr_step() { # [] + cat </dev/null + fm_write_meta "$d/state/feat-d-landed.meta" "window=fm:fm-feat-d-landed" "worktree=$d/wt" "kind=ship" + FM_FAKE_AXI_STATUS="$(run_passed_pr_completed fm/feat-d-landed)" + local out; out=$(run_crew_state "$d" feat-d-landed) + assert_contains "$out" "state: done" "passed with completed pr -> done" + assert_contains "$out" "source: run-step" "passed with completed pr -> run-step source" + assert_contains "$out" "run passed: PR merged/closed" "completed pr step still reads as landed" + pass "outcome=passed with a completed pr step still reports the merge" +} + +# outcome=passed from `no-mistakes axi run --skip=ci`: the run opened the pull +# request and stopped there, leaving it open and unmerged. The pr URL is present +# exactly as a run that opened one emits it, and `ci,skipped,0,16` is a skipped +# step row copied from the real v1.60.2 status of run 01M1EA5NJVP18AE7SPY5MBYW42. +run_passed_ci_skipped() { # + cat </dev/null + fm_write_meta "$d/state/feat-d-ciskip.meta" "window=fm:fm-feat-d-ciskip" "worktree=$d/wt" "kind=ship" + FM_FAKE_AXI_STATUS="$(run_passed_ci_skipped fm/feat-d-ciskip)" + local out; out=$(run_crew_state "$d" feat-d-ciskip) + assert_contains "$out" "state: done" "passed with a skipped ci step is still a terminal done run" + assert_not_contains "$out" "merged/closed" "a PR the run never carried past opening must not read as merged" + assert_contains "$out" "run passed, PR opened but ci step skipped: merge state unknown to the run" \ + "the step that stopped short must be named, and the merge left unclaimed" + pass "outcome=passed with a completed pr step but a skipped later step does not claim a merge" +} + +# outcome=passed from a step table whose LAST row is the pr step, the shape any +# axi status revision that drops or renames the merge-carrying step after it +# would produce. Opening a PR is not merging it, so with nothing reported after +# the pr step the run has shown no merge either. +run_passed_pr_is_last_step() { # + cat </dev/null + fm_write_meta "$d/state/feat-d-prlast.meta" "window=fm:fm-feat-d-prlast" "worktree=$d/wt" "kind=ship" + FM_FAKE_AXI_STATUS="$(run_passed_pr_is_last_step fm/feat-d-prlast)" + local out; out=$(run_crew_state "$d" feat-d-prlast) + assert_contains "$out" "source: run-step" "passed with pr as the last step -> run-step source" + assert_not_contains "$out" "merged/closed" "a pr step with nothing after it must not claim a merged-or-closed PR" + assert_contains "$out" "run passed, PR opened, no step reported after it: merge state unknown to the run" \ + "the missing merge-carrying step must be reported plainly, and the merge left unclaimed" + pass "outcome=passed with the pr step last and nothing after it does not claim a merge" +} + +# outcome=passed from a run whose step table carries NO pr row at all - the +# shape any axi status revision that renames, reorders or drops that step would +# produce. The reader cannot then observe a merge either, so the safety property +# is the same one the skipped case pins: never assert a merge the run did not +# perform. +run_passed_no_pr_row() { # + cat </dev/null + fm_write_meta "$d/state/feat-d-nopr.meta" "window=fm:fm-feat-d-nopr" "worktree=$d/wt" "kind=ship" + FM_FAKE_AXI_STATUS="$(run_passed_no_pr_row fm/feat-d-nopr)" + local out; out=$(run_crew_state "$d" feat-d-nopr) + assert_contains "$out" "source: run-step" "passed with no pr row -> run-step source" + assert_not_contains "$out" "merged/closed" "an unreported pr step must not claim a merged-or-closed PR" + assert_contains "$out" "run passed, no PR step reported: merge state unknown to the run" \ + "an unreported pr step must say so plainly, and the merge left unclaimed" + pass "outcome=passed with no pr step row does not claim a merge" +} + +# Negative case for the table scoping in the step-row reader. The run below +# genuinely skipped its pr step, and a `pr` finding sits above that row in the +# output. Drop the scoping and the finding is matched first, so the pr step +# reads as the finding's severity ("warning") and the real skipped step is never +# seen. Asserting the SPECIFIC skipped detail, not merely the absence of a +# merge claim, is what makes this case fail when the guard is removed. +test_findings_row_is_not_read_as_a_step_row() { + local case_n=0 file_col d out + for file_col in a.go 123; do + case_n=$((case_n + 1)) + reset_fakes + d=$(new_case "findings-row-shadowing-pr-$case_n") + make_repo_on_branch "$d/wt" fm/feat-d-findings + make_fakebin "$d" >/dev/null + fm_write_meta "$d/state/feat-d-findings.meta" "window=fm:fm-feat-d-findings" "worktree=$d/wt" "kind=ship" + FM_FAKE_AXI_STATUS="$(run_passed_findings_row_shadowing_pr_step fm/feat-d-findings "$file_col")" + out=$(run_crew_state "$d" feat-d-findings) + assert_contains "$out" "PR step skipped" "the real skipped pr step must be read, not the pr-named finding (file column $file_col)" + assert_not_contains "$out" "PR step warning" "a finding severity must never be reported as a step status (file column $file_col)" + assert_not_contains "$out" "merged/closed" "the shadowed case must still not claim a merged-or-closed PR (file column $file_col)" + done + pass "a findings row is never mistaken for a step row, whatever its columns hold" +} + test_terminal_failed() { reset_fakes local d; d=$(new_case failed) @@ -1556,6 +1902,7 @@ test_scalar_gate_parked_not_superseded test_gate_block_parked_not_superseded test_ci_ready_done_log_beats_monitoring_run test_ci_monitoring_checks_green_surfaces_done +test_ci_monitoring_with_active_steps_table_still_reads_the_step_row test_top_level_ci_checks_green_surfaces_done test_ci_monitoring_no_checks_terminal_surfaces_done test_ci_monitoring_green_then_rearm_stays_working @@ -1567,6 +1914,12 @@ test_ci_fixing_after_green_stays_working test_top_level_fixing_ci_running_after_green_stays_working test_top_level_fixing_done_log_stays_working test_terminal_passed +test_terminal_passed_pr_skipped_claims_no_merge +test_terminal_passed_pr_completed_reads_as_landed +test_terminal_passed_ci_skipped_claims_no_merge +test_terminal_passed_with_pr_as_last_step_claims_no_merge +test_findings_row_is_not_read_as_a_step_row +test_terminal_passed_without_a_pr_row_claims_no_merge test_terminal_failed test_cross_branch_attribution_via_runs_list test_cross_branch_attribution_picks_most_recent_row diff --git a/tests/fm-pr-check-security.test.sh b/tests/fm-pr-check-security.test.sh index e5e3eb09ede..8e396b926dc 100755 --- a/tests/fm-pr-check-security.test.sh +++ b/tests/fm-pr-check-security.test.sh @@ -174,10 +174,33 @@ printf '%s\n' "$*" >> "$FM_TEST_GLAB_LOG" [ "${FM_TEST_GLAB_SLEEP:-0}" = 0 ] || sleep "$FM_TEST_GLAB_SLEEP" printf 'title:\tfixture merge request\nstate:\t%s\nauthor:\tsomeone\n' "${FM_TEST_GLAB_STATE:-opened}" SH - chmod +x "$fakebin/gh" "$fakebin/gh-axi" "$fakebin/glab" + # Plain forgejo-axi, reproducing the real CLI's contract: its TOON output on + # stdout, and a non-zero exit whose diagnostic also goes to stdout and carries + # no bare "merged:" line of its own. + cat > "$fakebin/forgejo-axi" <<'SH' +#!/usr/bin/env bash +printf '%s\n' "$*" >> "$FM_TEST_FORGEJO_LOG" +if [ "${FM_TEST_FORGEJO_FAIL:-0}" != 0 ]; then + printf 'error: "Unable to reach Forgejo"\ncode: NETWORK_ERROR\n' + exit 1 +fi +case "${1:-} ${2:-}" in + "pr merged") + printf 'proof:\n merged: %s\n number: 7\n merged_at: null\n' \ + "${FM_TEST_FORGEJO_MERGED:-false}" + ;; + "pr view") + printf 'pull_request:\n head_sha: %s\n' \ + "${FM_TEST_FORGEJO_HEAD:-0123456789abcdef0123456789abcdef01234567}" + ;; +esac +exit 0 +SH + chmod +x "$fakebin/gh" "$fakebin/gh-axi" "$fakebin/glab" "$fakebin/forgejo-axi" : > "$dir/gh.log" : > "$dir/gh-axi.log" : > "$dir/glab.log" + : > "$dir/forgejo.log" : > "$dir/guard.log" printf '%s\n' "$dir" } @@ -207,6 +230,7 @@ run_check_entry() { FM_ROOT_OVERRIDE="$dir/root" FM_HOME="$dir/home" \ FM_TEST_GUARD_LOG="$dir/guard.log" FM_TEST_GH_LOG="$dir/gh.log" \ FM_TEST_GH_AXI_LOG="$dir/gh-axi.log" FM_TEST_GLAB_LOG="$dir/glab.log" \ + FM_TEST_FORGEJO_LOG="$dir/forgejo.log" \ PATH="$dir/fakebin:$BASE_PATH" \ "$PR_CHECK" "$@" } @@ -217,6 +241,7 @@ run_merge_entry() { FM_ROOT_OVERRIDE="$dir/root" FM_HOME="$dir/home" \ FM_TEST_GUARD_LOG="$dir/guard.log" FM_TEST_GH_LOG="$dir/gh.log" \ FM_TEST_GH_AXI_LOG="$dir/gh-axi.log" FM_TEST_GLAB_LOG="$dir/glab.log" \ + FM_TEST_FORGEJO_LOG="$dir/forgejo.log" \ PATH="$dir/fakebin:$BASE_PATH" \ "$PR_MERGE" "$@" } @@ -310,6 +335,52 @@ INVALID_URLS=( 'https://github.com/o/'\''"r"'\''/pull/1' "https://github.com/o/r/pull/1'" 'https://github.com/o/r/pull/1"' + 'https://github.com/o/r/pulls/1' + 'https://gitlab.com/g/p/pulls/1' + 'https://forgejo.example/o/r/pull/1' + 'https://forgejo.example/o/r/-/pulls/1' + 'https://forgejo.example/g/s/p/pulls/1' + 'https://forgejo.example/o/pulls/1' + 'https://forgejo.example//r/pulls/1' + 'https://forgejo.example/o//pulls/1' + 'https://forgejo.example/o/r/pulls/' + 'https://forgejo.example/o/r/pulls/0' + 'https://forgejo.example/o/r/pulls/01' + 'https://forgejo.example/o/r/pulls/-1' + 'https://forgejo.example/o/r/pulls/1/files' + 'https://forgejo.example/o/r/pulls/1/' + 'https://forgejo.example/o/r/pulls/1?q=x' + 'https://forgejo.example/o/r/pulls/1#f' + 'http://forgejo.example/o/r/pulls/1' + 'https://Forgejo.Example/o/r/pulls/1' + 'https://forgejo.example:443/o/r/pulls/1' + 'https://user@forgejo.example/o/r/pulls/1' + 'https://.forgejo.example/o/r/pulls/1' + 'https://forgejo.example./o/r/pulls/1' + 'https://forgejo.example/-owner/r/pulls/1' + 'https://forgejo.example/owner-/r/pulls/1' + 'https://forgejo.example/.owner/r/pulls/1' + 'https://forgejo.example/owner./r/pulls/1' + 'https://forgejo.example/own--er/r/pulls/1' + 'https://forgejo.example/own._er/r/pulls/1' + 'https://forgejo.example/own__er/r/pulls/1' + 'https://forgejo.example/o+wner/r/pulls/1' + 'https://forgejo.example/o/./pulls/1' + 'https://forgejo.example/o/../pulls/1' + 'https://forgejo.example/o/r.git/pulls/1' + 'https://forgejo.example/o/r.wiki/pulls/1' + 'https://forgejo.example/o/r.rss/pulls/1' + 'https://forgejo.example/o/r.atom/pulls/1' + 'https://forgejo.example/o/-/pulls/1' + 'https://forgejo.example/o/r.GIT/pulls/1' + 'https://forgejo.example/o/r.Git/pulls/1' + 'https://forgejo.example/o/r.WIKI/pulls/1' + 'https://forgejo.example/o/r.Rss/pulls/1' + 'https://forgejo.example/o/r.AtOm/pulls/1' + 'https://forgejo.example/o/r+z/pulls/1' + 'https://forgejo.example/o/r`/pulls/1' + $'https://forgejo.example/o/r/pulls/1\n' + ' https://forgejo.example/o/r/pulls/1' ) # shellcheck disable=SC2016 # Literal shell syntax is task-ID test data. @@ -376,6 +447,55 @@ https://gitlab.com/group/sub/deep/project/-/merge_requests/42|gitlab.com|group/s https://gitlab.example.co.uk/g/p/-/merge_requests/7|gitlab.example.co.uk|g/p|7 https://code.internal/team/tools/ci-runner/-/merge_requests/123456|code.internal|team/tools/ci-runner|123456 EOF + while IFS='|' read -r url host owner repo number; do + [ -n "$url" ] || continue + fm_pr_url_parse "$url" || fail "parser rejected a canonical Forgejo pull request URL" + [ "$FM_PR_PROVIDER" = forgejo ] || fail "parser did not tag a Forgejo URL as forgejo" + [ "$FM_PR_URL" = "$url" ] || fail "parser changed a canonical Forgejo URL" + [ "$FM_PR_HOST" = "$host" ] || fail "parser returned wrong Forgejo host" + [ "$FM_PR_PATH" = "$owner/$repo" ] || fail "parser returned wrong Forgejo project path" + [ "$FM_PR_OWNER" = "$owner" ] || fail "parser returned wrong Forgejo owner" + [ "$FM_PR_REPO" = "$repo" ] || fail "parser returned wrong Forgejo repository" + [ "$FM_PR_NUMBER" = "$number" ] || fail "parser returned wrong Forgejo pull request number" + done <<'EOF' +https://forgejo.example/o/r/pulls/1|forgejo.example|o|r|1 +https://codeberg.org/forgejo/forgejo/pulls/1234|codeberg.org|forgejo|forgejo|1234 +https://git.example.co.uk/My-Org/repo-name_with.parts/pulls/123456|git.example.co.uk|My-Org|repo-name_with.parts|123456 +https://git.example/o/pulls/pulls/1|git.example|o|pulls|1 +https://forgejo.example/o/a..b/pulls/1|forgejo.example|o|a..b|1 +https://forgejo.example/o/a--b/pulls/1|forgejo.example|o|a--b|1 +https://forgejo.example/o/r.gitx/pulls/1|forgejo.example|o|r.gitx|1 +https://forgejo.example/o/-leading/pulls/1|forgejo.example|o|-leading|1 +EOF + # The forge reserves the exact repository names ".", ".." and "-", and matches + # its reserved route suffixes after lowercasing, so both are refused whatever + # case they arrive in. It applies no rule against a run of "-", "_" or "." + # inside a repository name - that restriction is its username rule, enforced + # separately for the owner - so a repository the forge can host is not refused + # here for carrying one. + for row in - r.GIT r.Wiki r.RSS r.AtOm; do + ! fm_pr_url_parse "https://forgejo.example/o/$row/pulls/1" \ + || fail "parser accepted a repository name the forge reserves" + done + for row in a..b a--b a__b r.gitx -leading trailing-; do + fm_pr_url_parse "https://forgejo.example/o/$row/pulls/1" \ + || fail "parser refused a repository name the forge can host" + done + # The plural is what tells a Forgejo pull request from a GitHub one, and each + # provider keeps its own spelling: the same host and path with the other + # provider's segment is not a second accepted spelling for either of them. + fm_pr_url_parse https://forgejo.example/o/r/pulls/1 \ + || fail "parser rejected the plural Forgejo pull request segment" + ! fm_pr_url_parse https://forgejo.example/o/r/pull/1 \ + || fail "parser accepted GitHub's singular segment on a Forgejo host" + fm_pr_url_parse https://github.com/o/r/pull/1 \ + || fail "parser rejected GitHub's singular pull request segment" + ! fm_pr_url_parse https://github.com/o/r/pulls/1 \ + || fail "parser read GitHub's own host as a Forgejo instance" + ! fm_pr_url_parse https://gitlab.com/g/p/pulls/1 \ + || fail "parser read GitLab's own host as a Forgejo instance" + ! fm_pr_url_parse https://github.com/o/r/-/merge_requests/1 \ + || fail "parser read GitHub's own host as a GitLab instance" fm_pr_url_parse https://github.com/a/b/pull/1 || fail "parser rejected canonical URL" [ "$FM_PR_PROVIDER" = github ] || fail "parser did not tag a pull request URL as github" [ "$FM_PR_HOST" = github.com ] || fail "parser returned wrong GitHub host" @@ -678,6 +798,7 @@ make_poll_fixture() { run_poll() { local dir=$1 FM_TEST_GH_LOG="$dir/gh.log" FM_TEST_GLAB_LOG="$dir/glab.log" \ + FM_TEST_FORGEJO_LOG="$dir/forgejo.log" \ PATH="$dir/fakebin:$BASE_PATH" \ bash "$dir/home/state/task-a.check.sh" } @@ -1390,6 +1511,153 @@ EOF pass "GitLab merge requests are followed on any instance and never wake falsely" } +test_forgejo_merge_watch() { + local dir state out rc url value nofj entry bindir name + dir=$(make_case forgejo-merge-watch) + state="$dir/home/state" + url=https://forgejo.example/my-org/tools/pulls/7 + + write_poll_meta "$state" task-a "$url" + fm_pr_poll_prepare "$state" task-a forgejo "$url" forgejo.example my-org/tools 7 "$POLL" \ + || fail "could not prepare a Forgejo poll" + fm_pr_poll_publish_prepared || fail "could not publish a Forgejo poll" + fm_pr_poll_artifacts_valid "$state" task-a "$POLL" \ + || fail "published Forgejo poll provenance or metadata binding was invalid" + [ "$(cat "$state/task-a.pr-poll")" = "forgejo +$url +forgejo.example +my-org/tools +7" ] || fail "published Forgejo sidecar bytes were not exact" + + # Only an exact merged proof wakes firstmate. Every other reading, including + # an unreadable pull request and a changed output format, stays silent. + for value in false '' null TRUE True merged not-a-boolean; do + out=$(FM_TEST_FORGEJO_MERGED="$value" run_poll "$dir") + [ -z "$out" ] || fail "Forgejo poll emitted for a non-merged proof" + done + out=$(FM_TEST_FORGEJO_MERGED=true run_poll "$dir") + [ "$out" = merged ] || fail "Forgejo poll did not emit exactly one merged line" + out=$(FM_TEST_FORGEJO_MERGED=true FM_TEST_FORGEJO_FAIL=1 run_poll "$dir") + [ -z "$out" ] || fail "Forgejo poll emitted after a forgejo-axi failure" + + # forgejo-axi is addressed by the instance the record names and the derived + # owner/repository, never by the pull request URL: it reads only + # owner/repository/number out of a URL and still sends the request wherever its + # own configuration points, which would let an ambient default answer instead. + grep -qF -- "pr merged --base-url https://forgejo.example --repo my-org/tools 7" "$dir/forgejo.log" \ + || fail "Forgejo poll did not address forgejo-axi by instance and repository" + ! grep -qF -- "$url" "$dir/forgejo.log" \ + || fail "Forgejo poll passed a pull request URL to forgejo-axi" + + # An absent CLI must produce no wake rather than a false merge. The whole + # search path is mirrored without forgejo-axi, because a real one anywhere on + # PATH would make this prove nothing. + nofj="$dir/nofj" + mkdir -p "$nofj" + while IFS= read -r bindir; do + [ -d "$bindir" ] || continue + for entry in "$bindir"/*; do + [ -e "$entry" ] || continue + name=$(basename "$entry") + [ "$name" = forgejo-axi ] && continue + [ -e "$nofj/$name" ] || ln -s "$entry" "$nofj/$name" 2>/dev/null + done + done </dev/null 2>&1 \ + || fail "the forgejo-axi-free search path still resolved forgejo-axi" + out=$(FM_TEST_FORGEJO_MERGED=true FM_TEST_FORGEJO_LOG="$dir/forgejo.log" \ + PATH="$nofj" \ + bash "$state/task-a.check.sh") + [ -z "$out" ] || fail "Forgejo poll emitted with forgejo-axi absent from PATH" + + # A doctored sidecar cannot redirect the poll: the stored parts must rebuild + # the stored URL exactly. + printf '%s\n%s\n%s\n%s\n%s\n' forgejo "$url" elsewhere.example my-org/tools 7 \ + > "$state/task-a.pr-poll" + out=$(FM_TEST_FORGEJO_MERGED=true run_poll "$dir") + [ -z "$out" ] || fail "Forgejo poll emitted for a sidecar whose host was swapped" + printf '%s\n%s\n%s\n%s\n%s\n' forgejo "$url" forgejo.example my-org/other 7 \ + > "$state/task-a.pr-poll" + out=$(FM_TEST_FORGEJO_MERGED=true run_poll "$dir") + [ -z "$out" ] || fail "Forgejo poll emitted for a sidecar whose repository was swapped" + printf '%s\n%s\n%s\n%s\n%s\n' forgejo "$url" forgejo.example my-org/sub/tools 7 \ + > "$state/task-a.pr-poll" + out=$(FM_TEST_FORGEJO_MERGED=true run_poll "$dir") + [ -z "$out" ] || fail "Forgejo poll emitted for a sidecar carrying a nested project path" + + # The poll re-validates rather than trusting the sidecar, so the repository + # names the forge reserves are refused here too, in whatever case they arrive. + for value in - tools.git tools.GIT tools.Wiki tools.RSS tools.AtOm; do + printf '%s\n%s\n%s\n%s\n%s\n' forgejo \ + "https://forgejo.example/my-org/$value/pulls/7" forgejo.example "my-org/$value" 7 \ + > "$state/task-a.pr-poll" + out=$(FM_TEST_FORGEJO_MERGED=true run_poll "$dir") + [ -z "$out" ] || fail "Forgejo poll emitted for a repository name the forge reserves" + done + # A repository the forge can host still wakes, so the refusals above are not + # simply a stricter poll that never fires. + printf '%s\n%s\n%s\n%s\n%s\n' forgejo \ + "https://forgejo.example/my-org/a..b/pulls/7" forgejo.example my-org/a..b 7 \ + > "$state/task-a.pr-poll" + out=$(FM_TEST_FORGEJO_MERGED=true run_poll "$dir") + [ "$out" = merged ] || fail "Forgejo poll refused a repository name the forge can host" + printf '%s\n%s\n%s\n%s\n%s\n' forgejo "$url" forgejo.example my-org/tools 7 \ + > "$state/task-a.pr-poll" + + # Arming is where a missing CLI can still be reported, so it refuses there. + write_task_meta "$dir" task-b + set +e + out=$(FM_ROOT_OVERRIDE="$dir/root" FM_HOME="$dir/home" \ + FM_TEST_GUARD_LOG="$dir/guard.log" PATH="$nofj" \ + "$PR_CHECK" task-b "$url" 2>&1) + rc=$? + set -e + [ "$rc" -ne 0 ] || fail "arming a Forgejo watch succeeded with forgejo-axi absent" + case "$out" in + *"requires forgejo-axi on PATH"*) ;; + *) fail "arming a Forgejo watch with forgejo-axi absent did not report the missing CLI" ;; + esac + [ ! -e "$state/task-b.check.sh" ] || fail "refused Forgejo arming left a poll armed" + + # Arming records the head forgejo-axi supplies, which needs no repository on + # disk the way the GitHub lookup does. + write_task_meta "$dir" task-d + : > "$dir/forgejo.log" + run_check_entry "$dir" task-d "$url" >/dev/null || fail "could not arm a Forgejo watch" + grep -qxF "pr_head=0123456789abcdef0123456789abcdef01234567" "$state/task-d.meta" \ + || fail "arming a Forgejo watch did not record the head forgejo-axi reported" + grep -qF -- "pr view --base-url https://forgejo.example --repo my-org/tools 7 --fields head_sha" \ + "$dir/forgejo.log" \ + || fail "the Forgejo head lookup did not address forgejo-axi by instance and repository" + + # The merge path addresses the forge the URL names, and never another one. + # This fixture's forgejo-axi answers nothing at all for mergeability, so the + # merge's read cannot be parsed, which must refuse rather than merge on a state + # it could not read. + write_task_meta "$dir" task-c + : > "$dir/forgejo.log" + ln -sf "$REAL_JQ" "$dir/fakebin/jq" + set +e + run_merge_entry "$dir" task-c "$url" >/dev/null 2> "$dir/merge-c.err" + rc=$? + set -e + [ "$rc" -ne 0 ] || fail "merge wrapper merged a Forgejo pull request it could not read" + grep -qF 'could not read the Forgejo pull request state before merging' "$dir/merge-c.err" \ + || fail "merge wrapper refused for some reason other than the state it could not read" + [ ! -s "$dir/gh-axi.log" ] || fail "merge wrapper reached the GitHub CLI for a Forgejo URL" + [ ! -s "$dir/glab.log" ] || fail "merge wrapper reached the GitLab CLI for a Forgejo URL" + grep -qF "pr mergeability --base-url https://forgejo.example --repo my-org/tools 7" \ + "$dir/forgejo.log" \ + || fail "merge wrapper did not read the pull request through forgejo-axi at its own instance" + ! grep -qE '^pr merge ' "$dir/forgejo.log" \ + || fail "merge wrapper merged despite an unreadable pull request state" + + pass "Forgejo pull requests are followed on any instance and never wake falsely" +} + seed_canonical_poll() { local dir=$1 id=$2 url=$3 template=${4:-$POLL} state provider host path number state="$dir/home/state" @@ -2123,6 +2391,7 @@ test_gitlab_merged_poll_retires() { test_parser_matrix test_gitlab_merge_watch +test_forgejo_merge_watch test_merged_poll_retires_once test_merged_poll_reregistration_after_notification_is_absorbed test_merged_poll_retries_a_failed_upward_report diff --git a/tests/fm-pr-merge.test.sh b/tests/fm-pr-merge.test.sh index cfb02b92197..15cc69dfbd7 100755 --- a/tests/fm-pr-merge.test.sh +++ b/tests/fm-pr-merge.test.sh @@ -87,6 +87,16 @@ MR_URL="$MR_PROJECT_URL/-/merge_requests/7" MR_HEAD=aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa MR_STALE_HEAD=bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb +# The Forgejo fixture. A placeholder host that resolves nowhere, and an +# owner/repository pair, because a Forgejo project is always exactly one of each. +FJ_HOST=forgejo.example +FJ_OWNER=my-org +FJ_REPO=tools +FJ_BASE_URL="https://$FJ_HOST" +FJ_URL="$FJ_BASE_URL/$FJ_OWNER/$FJ_REPO/pulls/7" +FJ_HEAD=cccccccccccccccccccccccccccccccccccccccc +FJ_STALE_HEAD=dddddddddddddddddddddddddddddddddddddddd + JQ_BIN=$(command -v jq) || fail "these tests read glab's JSON with the real jq, which was not found" REAL_MV=$(command -v mv) || fail "these tests need mv to simulate a failed poll publish" @@ -320,6 +330,99 @@ make_gitlab_case() { printf '%s\n' "$case_dir" } +add_forgejo_mock() { + local case_dir=$1 + cat > "$case_dir/fakebin/forgejo-axi" <<'SH' +#!/usr/bin/env bash +printf '%s\n' "$*" >> "$FM_TEST_FORGEJO_LOG" +case_dir=$(dirname "$FM_TEST_FORGEJO_JSON") +case "${1:-} ${2:-}" in + "pr mergeability") + [ ! -e "$case_dir/forgejo-view-fails" ] || exit 1 + cat "$FM_TEST_FORGEJO_JSON" + exit 0 + ;; + "pr merged") + [ ! -e "$case_dir/forgejo-merged-fails" ] || exit 1 + if [ -e "$case_dir/forgejo-stays-open" ]; then + cat "$case_dir/proof-open.json" + else + cat "$case_dir/proof-merged.json" + fi + exit 0 + ;; + "pr merge") + [ ! -e "$case_dir/forgejo-merge-fails" ] || { echo "error: pr merge failed" >&2 ; exit 1 ; } + : > "$case_dir/forgejo-merge-called" + exit 0 + ;; +esac +exit 0 +SH + chmod +x "$case_dir/fakebin/forgejo-axi" + ln -sf "$JQ_BIN" "$case_dir/fakebin/jq" +} + +# write_mergeability_json [= ...] +# A mergeability payload that satisfies every pre-merge condition, with the +# named fields overridden so one case drives exactly one condition. Values are +# written into the JSON as-is, so a value may carry a JSON escape. +write_mergeability_json() { + local file=$1 kv key value + local number=7 url=$FJ_URL head=$FJ_HEAD + local forge_mergeable=true checks_pass=true mergeable=true reasons='[]' + shift + for kv in "$@"; do + key=${kv%%=*} + value=${kv#*=} + case "$key" in + number) number=$value ;; + url) url=$value ;; + head) head=$value ;; + forge_mergeable) forge_mergeable=$value ;; + checks_pass) checks_pass=$value ;; + mergeable) mergeable=$value ;; + reasons) reasons=$value ;; + *) fail "write_mergeability_json: unknown field '$key'" ;; + esac + done + printf '{"mergeability":{"number":%s,"url":"%s","head_sha":"%s",' \ + "$number" "$url" "$head" > "$file" + printf '"forgejo_mergeable":%s,"checks_pass":%s,"mergeable":%s,"reasons":%s}}\n' \ + "$forge_mergeable" "$checks_pass" "$mergeable" "$reasons" >> "$file" +} + +write_merged_proof_json() { + local file=$1 merged=$2 url=${3:-$FJ_URL} + printf '{"proof":{"merged":%s,"number":7,"url":"%s","head_sha":"%s"}}\n' \ + "$merged" "$url" "$FJ_HEAD" > "$file" +} + +# make_forgejo_case [= ...]: a case dir with every forge +# mock and a mergeability payload. Echoes the case dir. +make_forgejo_case() { + local name=$1 case_dir + shift + case_dir=$(make_case "$name") + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" cccccccccccccccccccccccccccccccccccccccc + add_glab_mock "$case_dir" + add_forgejo_mock "$case_dir" + : > "$case_dir/gh-axi.log" + : > "$case_dir/glab.log" + : > "$case_dir/forgejo.log" + write_mergeability_json "$case_dir/mergeability.json" "$@" + write_merged_proof_json "$case_dir/proof-merged.json" true + write_merged_proof_json "$case_dir/proof-open.json" false + printf '%s\n' "$case_dir" +} + +# The merge line forgejo-axi was asked to run, so a test asserts one exact +# invocation rather than a substring of the whole log. +forgejo_merge_line() { + grep -E '^pr merge ' "$1" || true +} + # 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, @@ -364,6 +467,8 @@ run_pr_merge() { FM_TEST_REAL_MV="$REAL_MV" \ FM_TEST_GLAB_LOG="$case_dir/glab.log" \ FM_TEST_GLAB_JSON="$case_dir/mr.json" \ + FM_TEST_FORGEJO_LOG="$case_dir/forgejo.log" \ + FM_TEST_FORGEJO_JSON="$case_dir/mergeability.json" \ PATH="$case_dir/fakebin:$PATH" \ "$PR_MERGE" "$@" rc=$? @@ -1712,6 +1817,376 @@ test_gitlab_invalid_head_refuses() { pass "fm-pr-merge refuses a GitLab head commit it cannot validate" } +test_forgejo_url_resolves_and_merges() { + local case_dir rc merge_line + case_dir=$(make_forgejo_case forgejo-merges) + + set +e + run_pr_merge "$case_dir" task-x1 "$FJ_URL" \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 0 "$rc" "forgejo-merges: a well-formed pull request URL should merge, not error" + assert_grep "pr=$FJ_URL" "$case_dir/state/task-x1.meta" \ + "forgejo-merges: pr= was not recorded before merging" + assert_grep "pr mergeability --base-url $FJ_BASE_URL --repo $FJ_OWNER/$FJ_REPO 7 --json" \ + "$case_dir/forgejo.log" \ + "forgejo-merges: the pre-merge state was not read from the instance the URL names" + merge_line=$(forgejo_merge_line "$case_dir/forgejo.log") + [ "$merge_line" = "pr merge --base-url $FJ_BASE_URL --repo $FJ_OWNER/$FJ_REPO 7 --expected-head $FJ_HEAD --method squash" ] \ + || fail "forgejo-merges: unexpected merge invocation: '$merge_line'" + assert_grep "mergeable with passing checks at head $FJ_HEAD" "$case_dir/stderr" \ + "forgejo-merges: the verified head was not reported" + [ ! -s "$case_dir/gh-axi.log" ] || fail "forgejo-merges: a pull request reached the GitHub CLI" + [ ! -s "$case_dir/glab.log" ] || fail "forgejo-merges: a pull request reached the GitLab CLI" + pass "fm-pr-merge merges a Forgejo pull request through forgejo-axi instead of refusing it" +} + +test_forgejo_host_comes_from_the_url() { + local case_dir rc host base_url url + host=code.self-hosted.example + base_url="https://$host" + url="$base_url/team/ci-runner/pulls/31" + case_dir=$(make_forgejo_case forgejo-host-from-url "url=$url" number=31) + write_merged_proof_json "$case_dir/proof-merged.json" true "$url" + + set +e + run_pr_merge "$case_dir" task-x1 "$url" \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 0 "$rc" "forgejo-host-from-url: a self-hosted pull request should merge" + assert_grep "pr mergeability --base-url $base_url --repo team/ci-runner 31 --json" \ + "$case_dir/forgejo.log" \ + "forgejo-host-from-url: the read did not use the host from the URL" + assert_grep "pr merge --base-url $base_url --repo team/ci-runner 31" "$case_dir/forgejo.log" \ + "forgejo-host-from-url: the merge did not use the host from the URL" + assert_no_grep "$FJ_HOST" "$case_dir/forgejo.log" \ + "forgejo-host-from-url: a host was assumed instead of taken from the URL" + # forgejo-axi reads only owner/repository/number out of a pull request URL and + # still sends the request wherever its own configuration points, so passing one + # would leave the instance to an ambient default. + assert_no_grep "$url" "$case_dir/forgejo.log" \ + "forgejo-host-from-url: a pull request URL was passed instead of an explicit instance" + pass "fm-pr-merge takes the Forgejo instance from the URL rather than assuming one" +} + +test_forgejo_defaults_to_squash_and_keeps_an_explicit_method() { + local case_dir rc merge_line + case_dir=$(make_forgejo_case forgejo-default-method) + + set +e + run_pr_merge "$case_dir" task-x1 "$FJ_URL" > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + expect_code 0 "$rc" "forgejo-default-method: merge should succeed" + merge_line=$(forgejo_merge_line "$case_dir/forgejo.log") + case "$merge_line" in + *'--method squash'*) : ;; + *) fail "forgejo-default-method: the GitHub squash default was not mirrored: '$merge_line'" ;; + esac + + case_dir=$(make_forgejo_case forgejo-explicit-method) + set +e + run_pr_merge "$case_dir" task-x1 "$FJ_URL" -- --method rebase \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + expect_code 0 "$rc" "forgejo-explicit-method: merge should succeed" + merge_line=$(forgejo_merge_line "$case_dir/forgejo.log") + [ "$merge_line" = "pr merge --base-url $FJ_BASE_URL --repo $FJ_OWNER/$FJ_REPO 7 --expected-head $FJ_HEAD --method rebase" ] \ + || fail "forgejo-explicit-method: the caller's method was overridden: '$merge_line'" + pass "fm-pr-merge names the squash default on Forgejo and preserves an explicit method" +} + +test_forgejo_each_condition_refuses_independently() { + local case_dir rc name expected spec + set -- \ + "forge|forge_mergeable=false|the forge reports mergeable as \"false\", not true" \ + "checks|checks_pass=false|the checks at head $FJ_HEAD report passing as \"false\", not true" \ + "verdict|mergeable=false|the merged verdict is \"false\", not true" + for spec in "$@"; do + name=${spec%%|*} + expected=${spec##*|} + spec=${spec#*|} + case_dir=$(make_forgejo_case "forgejo-refuse-$name" "${spec%%|*}" mergeable=false) + + set +e + run_pr_merge "$case_dir" task-x1 "$FJ_URL" \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "forgejo-refuse-$name: fm-pr-merge should refuse" + assert_grep "error: refusing to merge $FJ_URL" "$case_dir/stderr" \ + "forgejo-refuse-$name: refusal did not name the pull request" + assert_grep "$expected" "$case_dir/stderr" \ + "forgejo-refuse-$name: refusal did not name the failing condition" + [ -z "$(forgejo_merge_line "$case_dir/forgejo.log")" ] \ + || fail "forgejo-refuse-$name: a merge was attempted despite the refusal" + assert_grep "pr=$FJ_URL" "$case_dir/state/task-x1.meta" \ + "forgejo-refuse-$name: a refusal should still leave the recorded PR reference" + assert_present "$case_dir/state/task-x1.check.sh" \ + "forgejo-refuse-$name: a refusal should still leave the merge poll armed" + done + pass "fm-pr-merge refuses on each Forgejo pre-merge condition independently" +} + +test_forgejo_reports_every_failing_condition_and_the_forge_reasons() { + local case_dir rc expected + case_dir=$(make_forgejo_case forgejo-refuse-all forge_mergeable=false checks_pass=false \ + mergeable=false 'reasons=["forgejo_not_mergeable","checks_failure"]') + + set +e + run_pr_merge "$case_dir" task-x1 "$FJ_URL" \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "forgejo-refuse-all: fm-pr-merge should refuse" + for expected in \ + 'the forge reports mergeable as "false", not true' \ + "the checks at head $FJ_HEAD report passing as \"false\", not true" \ + 'the merged verdict is "false", not true' \ + 'the forge names: forgejo_not_mergeable, checks_failure' + do + assert_grep "$expected" "$case_dir/stderr" \ + "forgejo-refuse-all: '$expected' was not reported" + done + pass "fm-pr-merge reports every failing Forgejo condition and the forge's own reasons" +} + +test_forgejo_answer_for_another_pull_request_refuses() { + local case_dir rc name + for name in number url; do + case_dir=$(make_forgejo_case "forgejo-mismatch-$name") + case "$name" in + number) write_mergeability_json "$case_dir/mergeability.json" number=8 ;; + url) write_mergeability_json "$case_dir/mergeability.json" \ + "url=$FJ_BASE_URL/$FJ_OWNER/other/pulls/7" ;; + esac + + set +e + run_pr_merge "$case_dir" task-x1 "$FJ_URL" \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "forgejo-mismatch-$name: fm-pr-merge should refuse" + assert_grep "not $FJ_URL" "$case_dir/stderr" \ + "forgejo-mismatch-$name: refusal did not name the pull request it asked about" + [ -z "$(forgejo_merge_line "$case_dir/forgejo.log")" ] \ + || fail "forgejo-mismatch-$name: a merge was authorized by a view of another pull request" + done + pass "fm-pr-merge refuses a Forgejo view that answered for another pull request" +} + +test_forgejo_stale_recorded_head_is_reported() { + local case_dir rc merge_line + case_dir=$(make_forgejo_case forgejo-stale-head) + # The recorded head is what a rebase leaves behind. It is read before + # fm-pr-check.sh rewrites the metadata, which re-records the head it resolves + # live, so reading it afterwards would find the fresh value instead. + printf 'pr_head=%s\n' "$FJ_STALE_HEAD" >> "$case_dir/state/task-x1.meta" + + set +e + run_pr_merge "$case_dir" task-x1 "$FJ_URL" \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 0 "$rc" "forgejo-stale-head: the live head satisfies every condition, so it should merge" + assert_grep "recorded head $FJ_STALE_HEAD disagrees with the live head $FJ_HEAD" \ + "$case_dir/stderr" "forgejo-stale-head: the stale recorded head was trusted silently" + merge_line=$(forgejo_merge_line "$case_dir/forgejo.log") + case "$merge_line" in + *"--expected-head $FJ_HEAD"*) : ;; + *) fail "forgejo-stale-head: the merge was not bound to the live head: '$merge_line'" ;; + esac + pass "fm-pr-merge reports a stale recorded Forgejo head and verifies the live one" +} + +test_forgejo_unreadable_state_refuses() { + local case_dir rc name + for name in view-fails not-an-object missing-mergeability split-value; do + case_dir=$(make_forgejo_case "forgejo-unreadable-$name") + case "$name" in + view-fails) : > "$case_dir/forgejo-view-fails" ;; + not-an-object) printf '[]\n' > "$case_dir/mergeability.json" ;; + missing-mergeability) printf '{"proof":{}}\n' > "$case_dir/mergeability.json" ;; + # A value carrying a newline splits into a line no field name matches, so + # it must refuse rather than be truncated into a value a check accepts. + split-value) write_mergeability_json "$case_dir/mergeability.json" \ + 'url=opened\nnot-a-field' ;; + esac + + set +e + run_pr_merge "$case_dir" task-x1 "$FJ_URL" \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "forgejo-unreadable-$name: fm-pr-merge should refuse" + assert_grep 'could not read the Forgejo pull request state before merging' \ + "$case_dir/stderr" "forgejo-unreadable-$name: refusal did not name the unreadable state" + [ -z "$(forgejo_merge_line "$case_dir/forgejo.log")" ] \ + || fail "forgejo-unreadable-$name: a merge was attempted on an unreadable state" + done + pass "fm-pr-merge refuses an unreadable Forgejo pull request state rather than merging blind" +} + +test_forgejo_invalid_head_refuses() { + local case_dir rc + case_dir=$(make_forgejo_case forgejo-invalid-head head=not-a-sha) + + set +e + run_pr_merge "$case_dir" task-x1 "$FJ_URL" \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "forgejo-invalid-head: fm-pr-merge should refuse" + assert_grep 'could not read the Forgejo pull request head commit before merging' \ + "$case_dir/stderr" "forgejo-invalid-head: refusal did not name the unreadable head" + [ -z "$(forgejo_merge_line "$case_dir/forgejo.log")" ] \ + || fail "forgejo-invalid-head: a merge was bound to a head that is not a commit" + pass "fm-pr-merge refuses a Forgejo head commit it cannot validate" +} + +test_forgejo_merge_failure_propagates() { + local case_dir rc + case_dir=$(make_forgejo_case forgejo-merge-fails) + : > "$case_dir/forgejo-merge-fails" + + set +e + run_pr_merge "$case_dir" task-x1 "$FJ_URL" > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + [ "$rc" -ne 0 ] || fail "forgejo-merge-fails: a failed forgejo-axi merge was reported as success" + assert_grep "pr=$FJ_URL" "$case_dir/state/task-x1.meta" \ + "forgejo-merge-fails: a failed merge should still leave the recorded PR reference" + pass "fm-pr-merge propagates a real forgejo-axi merge failure without silently succeeding" +} + +test_forgejo_unconfirmed_merge_leaves_the_poll_armed() { + local case_dir rc name + for name in unreadable still-open other-pull-request; do + case_dir=$(make_forgejo_case "forgejo-unconfirmed-$name") + case "$name" in + unreadable) : > "$case_dir/forgejo-merged-fails" ;; + still-open) : > "$case_dir/forgejo-stays-open" ;; + other-pull-request) + write_merged_proof_json "$case_dir/proof-merged.json" true \ + "$FJ_BASE_URL/$FJ_OWNER/other/pulls/7" + ;; + esac + + set +e + run_pr_merge "$case_dir" task-x1 "$FJ_URL" \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 0 "$rc" "forgejo-unconfirmed-$name: an unconfirmed landing should not fail the run" + [ -n "$(forgejo_merge_line "$case_dir/forgejo.log")" ] \ + || fail "forgejo-unconfirmed-$name: the merge itself never ran" + assert_present "$case_dir/state/task-x1.check.sh" \ + "forgejo-unconfirmed-$name: an unconfirmed landing should leave the merge poll armed" + assert_grep 'the merge poll remains armed' "$case_dir/stderr" \ + "forgejo-unconfirmed-$name: the unconfirmed landing was not reported" + done + pass "fm-pr-merge records no landed Forgejo outcome it could not confirm" +} + +test_forgejo_missing_tool_refuses_before_recording() { + local case_dir rc tool other + for tool in forgejo-axi jq; do + if [ "$tool" = forgejo-axi ]; then other=jq; else other=forgejo-axi; fi + case_dir=$(make_forgejo_case "forgejo-no-$tool") + mirror_path_without "$case_dir/no$tool" "$tool" "$case_dir/fakebin" + # One tool absent, the other still answered by this case's own mock, so the + # refusal names exactly one tool on a host that ships neither. + PATH="$case_dir/no$tool" command -v "$other" >/dev/null 2>&1 \ + || fail "forgejo-no-$tool: the $tool-free search path lost the $other mock as well" + + set +e + FM_ROOT_OVERRIDE="$ROOT" \ + FM_STATE_OVERRIDE="$case_dir/state" \ + FM_TEST_GH_AXI_LOG="$case_dir/gh-axi.log" \ + FM_TEST_FORGEJO_LOG="$case_dir/forgejo.log" \ + FM_TEST_FORGEJO_JSON="$case_dir/mergeability.json" \ + PATH="$case_dir/no$tool" \ + "$PR_MERGE" task-x1 "$FJ_URL" > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "forgejo-no-$tool: fm-pr-merge should refuse" + assert_grep "merging a Forgejo pull request requires $tool on PATH" "$case_dir/stderr" \ + "forgejo-no-$tool: the refusal did not name the missing tool" + assert_no_grep "$other" "$case_dir/stderr" \ + "forgejo-no-$tool: the refusal named a tool that is present" + assert_no_grep "pr=" "$case_dir/state/task-x1.meta" \ + "forgejo-no-$tool: a missing tool still recorded PR metadata" + assert_absent "$case_dir/state/task-x1.check.sh" \ + "forgejo-no-$tool: a missing tool still armed a merge poll" + done + pass "fm-pr-merge refuses a Forgejo merge with either required tool absent" +} + +test_forgejo_override_args_refuse_before_recording() { + local case_dir rc spec flag expected + set -- \ + "--expected-head|$FJ_HEAD|must not override the head commit" \ + "--base-url|https://elsewhere.example|must not override the forge instance" \ + "--repo|other/repo|must not override the repository" + for spec in "$@"; do + flag=${spec%%|*} + expected=${spec##*|} + spec=${spec#*|} + case_dir=$(make_forgejo_case "forgejo-override${flag}") + + set +e + run_pr_merge "$case_dir" task-x1 "$FJ_URL" -- "$flag" "${spec%%|*}" \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "forgejo-override$flag: fm-pr-merge should refuse" + assert_grep "$expected" "$case_dir/stderr" \ + "forgejo-override$flag: the refusal did not name the override" + assert_no_grep "pr=" "$case_dir/state/task-x1.meta" \ + "forgejo-override$flag: an override still recorded PR metadata" + [ ! -s "$case_dir/forgejo.log" ] \ + || fail "forgejo-override$flag: an override still reached forgejo-axi" + done + pass "fm-pr-merge refuses Forgejo arguments that would override the URL's own identity" +} + +test_forgejo_github_method_spellings_refuse_before_recording() { + local case_dir rc flag + for flag in --squash --merge --rebase; do + case_dir=$(make_forgejo_case "forgejo-method-${flag#--}") + + set +e + run_pr_merge "$case_dir" task-x1 "$FJ_URL" -- "$flag" \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "forgejo-method$flag: fm-pr-merge should refuse" + assert_grep "--method squash|merge|rebase, not $flag" "$case_dir/stderr" \ + "forgejo-method$flag: the refusal did not name the spelling forgejo-axi rejects" + assert_no_grep "pr=" "$case_dir/state/task-x1.meta" \ + "forgejo-method$flag: a rejected method spelling still recorded PR metadata" + [ ! -s "$case_dir/forgejo.log" ] \ + || fail "forgejo-method$flag: a rejected method spelling still reached forgejo-axi" + done + pass "fm-pr-merge refuses GitHub merge-method spellings forgejo-axi does not accept" +} + test_gitlab_missing_tool_refuses_before_recording() { local case_dir rc tool other for tool in glab jq; do @@ -2132,6 +2607,20 @@ test_gitlab_unreadable_state_refuses test_gitlab_invalid_head_refuses test_gitlab_missing_tool_refuses_before_recording test_gitlab_head_override_args_refuse_before_recording +test_forgejo_url_resolves_and_merges +test_forgejo_host_comes_from_the_url +test_forgejo_defaults_to_squash_and_keeps_an_explicit_method +test_forgejo_each_condition_refuses_independently +test_forgejo_reports_every_failing_condition_and_the_forge_reasons +test_forgejo_answer_for_another_pull_request_refuses +test_forgejo_stale_recorded_head_is_reported +test_forgejo_unreadable_state_refuses +test_forgejo_invalid_head_refuses +test_forgejo_merge_failure_propagates +test_forgejo_unconfirmed_merge_leaves_the_poll_armed +test_forgejo_missing_tool_refuses_before_recording +test_forgejo_override_args_refuse_before_recording +test_forgejo_github_method_spellings_refuse_before_recording test_secondmate_merge_reports_upward_once test_secondmate_merge_reports_on_the_local_route test_gitlab_merge_reports_upward