diff --git a/.claude/skills/pullrequest/SKILL.md b/.claude/skills/pullrequest/SKILL.md index 71d36de6dd..810bcb727d 100644 --- a/.claude/skills/pullrequest/SKILL.md +++ b/.claude/skills/pullrequest/SKILL.md @@ -335,8 +335,9 @@ buy, is in [/ci](../ci/SKILL.md).) Many sessions and their agents work the fleet at once. Four practices keep them from undoing each other: -- **A hold on a PR is a DRAFT, and a draft needs a stated end.** `auto-arm.yml` re-arms auto-merge - on every push, so `--disable-auto` is not a hold; draft is the only state it respects. But a draft +- **A hold on a PR is a DRAFT, and a draft needs a stated end.** The control plane's PR steward + re-arms auto-merge once each new head is reviewed and answered, so `--disable-auto` is not a + hold; draft is the only state it respects. But a draft has no expiry: write in the PR body WHAT it waits for (a named PR merged, a sealed set carrying a commit) and re-check that condition before you leave it parked, because the blocker may already be gone. Copilot does not review pushes to a draft, so mark it ready before the fix you want @@ -412,11 +413,15 @@ that was tested — the fix for two independently-green PRs being red together ( changes about this procedure: - **`gh pr merge --auto` means "enqueue when this PR's own required checks are green".** - `auto-arm.yml` runs it on every non-draft PR, so a green PR lands without a hand on it. Convert to - **draft** to opt out. `gh pr merge --merge` on a green PR is the same thing done by hand: it - enters the queue, it does not merge on the spot. + The control plane's PR steward arms it on a non-draft PR once its CURRENT head has its review (a + Copilot review against that head, policy `copilot-code-review`), every reviewer thread is answered + and the required checks are green (`auto-arm.yml` only DISARMS, on every push — + Doc/Architecture/ReviewFindingsAnswered → "The arm gate"). Convert to **draft** to opt out. + `gh pr merge --merge` on a green PR is the same thing done by hand: it enters the queue, it + does not merge on the spot. - **A push to a queued branch ejects it.** The queue does not pick up the new commit in place; the - arm lane re-arms on `synchronize` and the new head re-enters once its own run is green. + push disarms it (`auto-arm.yml`), and the control plane re-arms the new head once it is reviewed, + answered and green. - **Dequeue via GraphQL** (`dequeuePullRequest(input:{id:})`), never by re-ordering or by `jump`. The queue's order is its correctness argument. - **Never re-queue an ejected PR by hand, and never re-run the failed queue build.** The diff --git a/.github/lane-caller-grants.yml b/.github/lane-caller-grants.yml index 0ffc09ee3d..3fb7f2ee9f 100644 --- a/.github/lane-caller-grants.yml +++ b/.github/lane-caller-grants.yml @@ -80,12 +80,6 @@ repos: job: arm lane: auto-arm.yml grants: {contents: write, pull-requests: write} - pending: RETIRING — replaced by the checks:read row below once this repository's caller grants it; auto-arm arms only after the internal review (MeshWeaver PR fix/arm-after-review, 2026-10-04) - - workflow: auto-arm.yml - job: arm - lane: auto-arm.yml - grants: {checks: read, contents: write, pull-requests: write} - pending: ARRIVING — the caller adds checks:read (the lane reads the internal-review check run) and the check_run / pull_request_review_comment triggers; auto-arm arms only after the internal review (MeshWeaver PR fix/arm-after-review, 2026-10-04) - workflow: ci.yml job: modules-floor lane: node-repo-module-pack.yml @@ -127,12 +121,6 @@ repos: job: arm lane: auto-arm.yml grants: {contents: write, pull-requests: write} - pending: RETIRING — replaced by the checks:read row below once this repository's caller grants it; auto-arm arms only after the internal review (MeshWeaver PR fix/arm-after-review, 2026-10-04) - - workflow: auto-arm.yml - job: arm - lane: auto-arm.yml - grants: {checks: read, contents: write, pull-requests: write} - pending: ARRIVING — the caller adds checks:read (the lane reads the internal-review check run) and the check_run / pull_request_review_comment triggers; auto-arm arms only after the internal review (MeshWeaver PR fix/arm-after-review, 2026-10-04) - workflow: ci.yml job: publish-bake lane: node-repo-publish-bake.yml @@ -182,12 +170,6 @@ repos: job: arm lane: auto-arm.yml grants: {contents: write, pull-requests: write} - pending: RETIRING — replaced by the checks:read row below once this repository's caller grants it; auto-arm arms only after the internal review (MeshWeaver PR fix/arm-after-review, 2026-10-04) - - workflow: auto-arm.yml - job: arm - lane: auto-arm.yml - grants: {checks: read, contents: write, pull-requests: write} - pending: ARRIVING — the caller adds checks:read (the lane reads the internal-review check run) and the check_run / pull_request_review_comment triggers; auto-arm arms only after the internal review (MeshWeaver PR fix/arm-after-review, 2026-10-04) - workflow: ci.yml job: compile-check lane: node-repo-compile-check.yml @@ -241,12 +223,6 @@ repos: job: arm lane: auto-arm.yml grants: {contents: write, pull-requests: write} - pending: RETIRING — replaced by the checks:read row below once this repository's caller grants it; auto-arm arms only after the internal review (MeshWeaver PR fix/arm-after-review, 2026-10-04) - - workflow: auto-arm.yml - job: arm - lane: auto-arm.yml - grants: {checks: read, contents: write, pull-requests: write} - pending: ARRIVING — the caller adds checks:read (the lane reads the internal-review check run) and the check_run / pull_request_review_comment triggers; auto-arm arms only after the internal review (MeshWeaver PR fix/arm-after-review, 2026-10-04) - workflow: ci.yml job: compile-check lane: node-repo-compile-check.yml @@ -301,12 +277,6 @@ repos: job: arm lane: auto-arm.yml grants: {contents: write, pull-requests: write} - pending: RETIRING — replaced by the checks:read row below once this repository's caller grants it; auto-arm arms only after the internal review (MeshWeaver PR fix/arm-after-review, 2026-10-04) - - workflow: auto-arm.yml - job: arm - lane: auto-arm.yml - grants: {checks: read, contents: write, pull-requests: write} - pending: ARRIVING — the caller adds checks:read (the lane reads the internal-review check run) and the check_run / pull_request_review_comment triggers; auto-arm arms only after the internal review (MeshWeaver PR fix/arm-after-review, 2026-10-04) - workflow: ci.yml job: compile-check lane: node-repo-compile-check.yml @@ -353,12 +323,6 @@ repos: job: arm lane: auto-arm.yml grants: {contents: write, pull-requests: write} - pending: RETIRING — replaced by the checks:read row below once this repository's caller grants it; auto-arm arms only after the internal review (MeshWeaver PR fix/arm-after-review, 2026-10-04) - - workflow: auto-arm.yml - job: arm - lane: auto-arm.yml - grants: {checks: read, contents: write, pull-requests: write} - pending: ARRIVING — the caller adds checks:read (the lane reads the internal-review check run) and the check_run / pull_request_review_comment triggers; auto-arm arms only after the internal review (MeshWeaver PR fix/arm-after-review, 2026-10-04) - workflow: ci.yml job: compile-check lane: node-repo-compile-check.yml @@ -409,12 +373,6 @@ repos: job: arm lane: auto-arm.yml grants: {contents: write, pull-requests: write} - pending: RETIRING — replaced by the checks:read row below once this repository's caller grants it; auto-arm arms only after the internal review (MeshWeaver PR fix/arm-after-review, 2026-10-04) - - workflow: auto-arm.yml - job: arm - lane: auto-arm.yml - grants: {checks: read, contents: write, pull-requests: write} - pending: ARRIVING — the caller adds checks:read (the lane reads the internal-review check run) and the check_run / pull_request_review_comment triggers; auto-arm arms only after the internal review (MeshWeaver PR fix/arm-after-review, 2026-10-04) - workflow: ci.yml job: compile-check lane: node-repo-compile-check.yml @@ -467,9 +425,3 @@ repos: job: arm lane: auto-arm.yml grants: {contents: write, pull-requests: write} - pending: RETIRING — replaced by the checks:read row below once this repository's caller grants it; auto-arm arms only after the internal review (MeshWeaver PR fix/arm-after-review, 2026-10-04) - - workflow: auto-arm.yml - job: arm - lane: auto-arm.yml - grants: {checks: read, contents: write, pull-requests: write} - pending: ARRIVING — the caller adds checks:read (the lane reads the internal-review check run) and the check_run / pull_request_review_comment triggers; auto-arm arms only after the internal review (MeshWeaver PR fix/arm-after-review, 2026-10-04) diff --git a/.github/scripts/check-review-answered.py b/.github/scripts/check-review-answered.py index e80312bf89..530620bf62 100644 --- a/.github/scripts/check-review-answered.py +++ b/.github/scripts/check-review-answered.py @@ -116,7 +116,7 @@ ----- check-review-answered.py --self-test check-review-answered.py --repo O/R --pr N [--as-of 2026-09-14T14:04:21Z] - check-review-answered.py --repo O/R --pr N --arm-gate (auto-arm.yml: may auto-merge be armed now?) + check-review-answered.py --repo O/R --pr N --arm-gate (the control plane's arm decision: may auto-merge be armed now?) check-review-answered.py --repo O/R --merge-group-ref refs/heads/gh-readonly-queue/main/pr-N- check-review-answered.py --repo O/R --pr N --stage-gate --since (node-repo-stage-gate.yml: may the expensive suites start for this head?) @@ -548,15 +548,18 @@ def reply_age_seconds(stamp: str, now: str | None = None) -> float | None: # ─────────────────────────────── the ARM gate (pure) ─────────────────────────────── # -# `auto-arm.yml` asks a DIFFERENT, stricter question than the merge gate above: not "may this merge" -# but "may auto-merge be armed NOW". Measured 2026-10-03/04 on MeshWeaver.Plugins, where the review +# The ARM decision (made by the control plane's PR steward, MeshWeaver.Plugins `PrArming`; this is +# its reference implementation and these self-test cases its test vectors — `auto-arm.yml` only +# disarms on a push) asks a DIFFERENT, stricter question than the merge gate above: not "may this +# merge" but "may auto-merge be armed NOW". Measured 2026-10-03/04 on MeshWeaver.Plugins, where the review # is comment-only and nothing required waits for it: the lane re-armed on every push and on undraft, # BEFORE the internal review had run on the new head, so #2549, #2643, #2647 and Memex#641 merged # with findings nobody had answered or with no review at all, and agents disarmed by hand after -# every push (#2791 twice in an hour). So the lane arms only when ALL of these hold: +# every push (#2791 twice in an hour). So auto-merge is armed only when ALL of these hold: # # 1. not a draft; -# 2. the internal reviewer's `internal-review` check run on the CURRENT head has COMPLETED, and it +# 2. the CURRENT head has its review — a Copilot review against it (see below), or the internal +# reviewer's `internal-review` check run on it has COMPLETED, and that run # is not the neutral "Reviewer unavailable" degradation. That degradation releases the MERGE # gate's review condition (so a down reviewer cannot hold every pull request), but it is not a # review, and arming on it would land an unreviewed change with nobody having decided to — @@ -1943,7 +1946,7 @@ def case(name: str, expect: tuple[str, ...], pr, reviews, comments, waiver=NO_WA failures += 0 if ok else 1 print(f"self-test {'ok' if ok else 'FAIL':4} {name:60} expected={expect} got={got}") - # ── the ARM gate (auto-arm.yml): arms only on a completed internal review of the CURRENT head, + # ── the ARM gate (control-plane reference): arms only on a review of the CURRENT head (Copilot's, or a completed internal-review run), # with every reviewer thread answered by a person, and never a draft. Each NO case names the # condition it must fail ON, so a fixture cannot pass by failing for another reason. HEAD = "a" * 40 @@ -2381,7 +2384,7 @@ def main(argv=None) -> int: ap.add_argument("--pr", help="pull request number") ap.add_argument("--merge-group-ref", help="a merge-queue head ref; the pull request number is read from it") ap.add_argument("--arm-gate", action="store_true", - help="answer auto-arm.yml's question instead — may auto-merge be armed NOW (see arm_readiness); " + help="answer the arm decision's question instead — may auto-merge be armed NOW (see arm_readiness); " "writes ready=true|false to $GITHUB_OUTPUT and one line to the job summary, exit 0 either way") ap.add_argument("--as-of", help="evaluate as of this ISO-8601 UTC instant (e.g. a merged_at)") ap.add_argument("--settle-replies", type=int, default=0, metavar="SECONDS", diff --git a/.github/workflows/auto-arm.yml b/.github/workflows/auto-arm.yml index d76ce5e468..221db7314f 100644 --- a/.github/workflows/auto-arm.yml +++ b/.github/workflows/auto-arm.yml @@ -1,46 +1,36 @@ -# Arm auto-merge on every non-draft PR, so a PR that goes green LANDS. +# Auto-merge lane — it no longer ARMS anything; it DISARMS on a push (policy review-then-suites). # -# 🚨 This does not weaken a single gate. Auto-merge waits for exactly the required contexts -# branch protection already demands; it removes only the human round-trip between "green" and -# "merged". The failure it fixes is the one measured on 2026-09-01: core had SIX PRs sitting -# CLEAN and unarmed with an empty merge queue — nothing was failing, nothing was blocked, and -# nothing was landing either. A green PR nobody arms is indistinguishable from a red one. +# 🚨🚨 ARMING MOVED TO THE CONTROL PLANE. Arming auto-merge is a DECISION — "this head was reviewed +# and every finding answered, and the required checks are green" — and the decision belongs to the +# control instance's PR steward (MeshWeaver.Plugins `PrBabysitter` / `PrArming`, App `systemorph-com`), +# which already reads the review state and the answered-findings verdict for every pull request in the +# fleet. Before the move this lane armed every non-draft pull request on open, push and undraft — +# BEFORE the automatic review had landed on the new head. Where that review is comment-only and nothing required waits for it +# (MeshWeaver.Plugins), #2549, #2643/#2647 and Memex#641 merged with unanswered findings or no +# review at all, and agents disarmed by hand after every push (Plugins #2791, twice in an hour). +# The predicate the control plane applies is `check-review-answered.py --arm-gate` (the reference +# implementation, with its self-test); see Doc/Architecture/ReviewFindingsAnswered → "The arm gate". # -# 🚨 THIS FILE IS THE FLEET'S ONLY COPY OF THE ARM LANE. It runs directly here on -# `pull_request_target`, and every OTHER repo in the fleet reaches it through `workflow_call` with -# a caller that forwards the App credential and nothing else. (Deliberately no list of repo names -# here: an enumeration in a comment is a copy of the fleet roster and would drift exactly the way -# the copies below did. `gh search code --owner Systemorph "auto-arm.yml@"` answers who calls it.) -# The repos are NOT identical — core runs a merge queue and the others do not — so the arm step -# handles both shapes rather than the file forking per repo. +# 🚨 WHAT IS LEFT HERE, AND WHY IT STAYS A WORKFLOW: a push invalidates the review, but GitHub keeps +# auto-merge armed across a push by anyone with write access. A pull request the control plane armed +# on a reviewed head would otherwise merge the NEXT, unreviewed head the moment the required +# contexts pass. The disarm must be immediate — a reaction to the `synchronize` event itself, not a +# control-plane cadence that a fast CI can outrun — so it stays on the event, here. The control +# plane re-arms once the new head is reviewed and answered (and arms with `expectedHeadOid`, so an +# arm racing a push is refused by GitHub rather than landing on the wrong head). # -# It used to be hand-copied into every one of them, and the copies DRIFTED exactly as AGENTS.md -# warns a copy-pasted lane does. Measured 2026-09-02 — the numbers below are a record of that -# measurement, not a live count: core had already been moved to a minted App token (below) while -# every satellite copy was still arming with `secrets.GITHUB_TOKEN`, so every satellite was living -# the #2916 outage this file's header documents, silently. MeshWeaver.Reinsurance's `main` had not -# seen a `push`-event run since 09:11Z despite PRs merging all afternoon. On top of that, only -# .Crm carried the "landed:" read-back fix below, and several copies had no `timeout-minutes` at -# all — four distinct variants of a file whose header claimed all copies were identical. One lane, -# called from everywhere, cannot drift. +# 🚨 THIS FILE IS THE FLEET'S ONLY COPY OF THE LANE. It runs directly here on +# `pull_request_target`, and every OTHER repo in the fleet reaches it through `workflow_call` with +# a caller that forwards the App credential and nothing else. The callers did not change when the +# arming moved: they still forward `[opened, reopened, ready_for_review, synchronize]`, and the job +# below simply runs on `synchronize` only. (Deliberately no list of repo names here: +# `gh search code --owner Systemorph "auto-arm.yml@"` answers who calls it.) # # 🚨 The `workflow_call` secrets are named `MESHWEAVER_APP_ID` / `MESHWEAVER_APP_PRIVATE_KEY` — the -# SAME names as the repository secrets the direct `pull_request_target` trigger reads. That is -# deliberate: `secrets.MESHWEAVER_APP_ID` then resolves correctly in BOTH modes (the repo secret -# when this workflow is triggered here, the forwarded value when it is called), so the body needs -# no `||` fallback and there is no second name to get wrong. A caller passes them explicitly — +# SAME names as the repository secrets the direct `pull_request_target` trigger reads, so +# `secrets.MESHWEAVER_APP_ID` resolves correctly in BOTH modes. A caller passes them explicitly — # never `secrets: inherit`, which would hand this lane every secret the calling repo owns. # -# Deliberately NOT here: -# * no `if: ${{ vars.X != '' }}` — invariant #3; a skipped job renders like a passed one. -# * drafts are skipped, which is the ONE opt-out. Mark a PR draft and it will not be armed; -# that is the intended way to say "not ready", rather than withholding the arm. -# * forks are skipped: arming a PR from a fork would let an outside branch merge itself the -# moment the required set passes. -# * a PR whose BASE is not the default branch is NOT armed, and is told so. Auto-merge waits for -# the base's required contexts, and an unprotected base has none — so an arm there is an -# immediate merge, not a deferred one. See the base step for the measured instance. -# # 🚨🚨 THE ARM CREDENTIAL IS LEAD, NOT TRIM — it decides whether main's push lanes run at all. # This lane armed with `secrets.GITHUB_TOKEN` for six hours on 2026-09-01, and the consequence was # measured on #2916: GitHub performs an auto-merge as the identity that ARMED it, so the merge @@ -60,26 +50,22 @@ # NEVER run" — both read `absent/none`. That is the AGENTS.md skip-trapdoor one level down: a # SUPPRESSED TRIGGER renders exactly like a slow one. # -# So the arm runs on a minted GitHub App installation token, whose merges are ordinary pushes that -# trigger everything. Never put `secrets.GITHUB_TOKEN` back here — see +# That is why every token this lane uses is a minted GitHub App installation token — and why the +# control plane arms with its App (`systemorph-com`), never a default token. Never put `secrets.GITHUB_TOKEN` back here — see # `ArmedMergeMustTriggerMainsPushLanesGuard`, which fails the build if anyone does. -name: Arm auto-merge +name: Disarm auto-merge on push on: pull_request_target: types: [opened, reopened, ready_for_review, synchronize] # The satellites' entry point. A caller keeps the SAME `pull_request_target` trigger and one job # that `uses:` this file — so `github.event` inside is the caller's pull-request event, and - # `github.repository` / `github.event.repository.name` are the CALLING repository, which is what - # the mint scopes to and what `gh pr merge` is pointed at. Nothing below needs to know whether it - # is running here or there. + # `github.repository` / `github.event.repository.name` are the CALLING repository. workflow_call: secrets: MESHWEAVER_APP_ID: description: >- App id of the org's meshweaver-cloud GitHub App, installed on every repo in the fleet. - Required: an arm performed with secrets.GITHUB_TOKEN merges as github-actions[bot], whose - push triggers no workflows at all (#2916). required: true MESHWEAVER_APP_PRIVATE_KEY: description: >- @@ -92,39 +78,21 @@ permissions: jobs: arm: - name: Arm auto-merge on this PR + name: Disarm auto-merge on an unreviewed push runs-on: ${{ vars.MW_RUNNER || 'ubuntu-latest' }} - # 🚨 Job-level, not workflow-level, and that is load-bearing now that this file is also called. - # A concurrency group is per-repository, and a CALLER run and the job it dispatches into this - # workflow are two separate holders of it: if both named `auto-arm-` the inner job would - # queue behind the caller run that is waiting for it, and the lane would deadlock until the - # cap fired. Declared on the job, the group is held once per run in either mode. Callers must - # therefore NOT add a `concurrency:` block of their own — this one already covers them. + # Job-level, not workflow-level: a CALLER run and the job it dispatches into this workflow are + # two holders of a per-repository group, and a workflow-level group would deadlock the inner + # job behind its own caller. Callers must NOT add a `concurrency:` block of their own. concurrency: group: auto-arm-${{ github.repository }}-${{ github.event.pull_request.number }} cancel-in-progress: true - # Four API calls and a mint — the same shape as arm-credential.yml, capped the same way. The - # fleet ceiling is 45 (check-workflow-timeouts.py); a lane whose whole job is to call gh three - # times has no business sitting on a billed runner for three quarters of an hour because a - # socket hung. This cap now applies to every repo that calls this lane. timeout-minutes: 10 - # A draft is the opt-out, and a fork must never arm itself. Both are properties of the - # event, not configuration — there is no variable here to forget to set. - if: >- - github.event.pull_request.draft == false && - github.event.pull_request.head.repo.full_name == github.repository + # Only a PUSH invalidates a review, so only `synchronize` has anything to do; the other forwarded + # actions skip. Forks are NOT skipped: a maintainer can arm a reviewed fork PR by hand, and + # GitHub keeps that arm across the contributor's next push. This `pull_request_target` job runs + # only the trusted base workflow and checks out no fork code, so disarming a fork PR is safe. + if: github.event.action == 'synchronize' steps: - # No `if:` on any step that ASSERTS anything. The credential assert, the base read and the - # mint run unconditionally: they assert and fail, they do not choose whether to run. Every - # exemption in this file is a property of the EVENT (draft, fork, base branch), never a - # question about whether a credential happens to be set — invariant #3. - # - # The `if:`s further down govern ACTIONS and DIAGNOSTICS, not gates: one prints because a - # failure already happened, one comments because the base already disqualified the arm, and - # the arm itself runs only when there is a token to arm with and a protected base to arm - # onto. None of them can make a verification not run, which is the distinction that matters — - # and the job itself is never skipped by any of them, so "not armed" and "the lane never ran" - # stay two different things a reader can tell apart. - name: The App credential, or a red run naming what to provision env: MESHWEAVER_APP_ID: ${{ secrets.MESHWEAVER_APP_ID }} @@ -134,107 +102,18 @@ jobs: [ -n "$MESHWEAVER_APP_ID" ] || missing+=("secrets.MESHWEAVER_APP_ID app id of the org's meshweaver-cloud GitHub App, installed on every repo in the fleet") [ -n "$MESHWEAVER_APP_PRIVATE_KEY" ] || missing+=("secrets.MESHWEAVER_APP_PRIVATE_KEY that App's private key (PEM); source of truth is Key Vault Systemorph/github-app-privatekey") if [ ${#missing[@]} -gt 0 ]; then - echo "::error::auto-arm cannot run — provision the following, then re-run:" + echo "::error::the disarm lane cannot run — provision the following, then re-run:" for m in "${missing[@]}"; do echo " • $m"; done exit 1 fi echo "App credential present." - # 🚨🚨 ARM ONLY ONTO THE DEFAULT BRANCH. On any other base, `--auto` does not mean - # "merge when green" — it means "merge NOW". - # - # Auto-merge waits for exactly the contexts BRANCH PROTECTION requires of the pull request's - # BASE. In this fleet protection is configured on the default branch and nowhere else (core: - # ruleset `main pr protection`; the satellites: classic protection on `main`). A STACKED pull - # request — one opened against another pull request's feature branch, which is how a change - # set gets split for review — therefore has a base that is protected by nothing, an EMPTY - # required set, and a condition that is satisfied the instant GitHub first reads it. - # - # Measured 2026-09-11: MeshWeaver.Plugins#1685 was opened against the feature branch of - # #1681, this lane armed it, and GitHub merged it at 21:03:23Z — 61 seconds after it was - # opened, BEFORE its own CI had started a single job. Nothing failed and nothing was - # bypassed: the stack simply collapsed unreviewed, exactly as configured. This lane is the - # fleet's only copy, so the same hole was open in all eight repositories at once. - # - # 🚨 The default branch is READ, never assumed. Every repo in the fleet uses `main` today — - # which is precisely why a literal `main` here would be indistinguishable from a correct - # check while quietly being a copy of a fact, the drift this file's header exists to stop. - # `github.event.repository.default_branch` is on the pull-request payload in BOTH modes (a - # caller forwards its own event), so there is one reading and no fallback to get wrong. - # - # 🚨 This is NOT a skip-trapdoor. It asks a property of the EVENT, exactly as the job's - # draft/fork exemption does, never whether a credential happens to be set — and the job still - # RUNS and still reports on a non-default base rather than vanishing, so "not armed" and - # "lane never ran" stay distinguishable. This lane publishes no required context. - - name: The base branch decides whether arming means "when green" or "now" - id: base - env: - BASE: ${{ github.event.pull_request.base.ref }} - DEFAULT_BRANCH: ${{ github.event.repository.default_branch }} - run: | - set -euo pipefail - # Assert the readings rather than defaulting them. An empty default branch would make - # every base look non-default and silently stop arming the entire fleet — the mirror - # image of the failure above — so it gets a red that names itself instead. - if [ -z "$DEFAULT_BRANCH" ]; then - echo "::error::the pull-request payload carried no repository.default_branch, so this lane cannot tell a protected base from an unprotected one. Refusing to arm." - exit 1 - fi - if [ -z "$BASE" ]; then - echo "::error::the pull-request payload carried no base.ref, so this lane cannot tell what this pull request would merge into. Refusing to arm." - exit 1 - fi - if [ "$BASE" = "$DEFAULT_BRANCH" ]; then - echo "onto-default=true" >> "$GITHUB_OUTPUT" - echo "base '$BASE' is the default branch: arming means 'merge when the required contexts pass'." - exit 0 - fi - echo "onto-default=false" >> "$GITHUB_OUTPUT" - echo "::warning::auto-merge was NOT armed on this pull request — it targets '$BASE', not the default branch '$DEFAULT_BRANCH'. A non-default base carries no branch protection and therefore no required contexts, so arming would merge this pull request immediately, before its own CI has run." - { - echo "### Auto-merge not armed — the base is not the default branch" - echo - echo "| | |" - echo "|---|---|" - echo "| base | \`$BASE\` |" - echo "| default branch | \`$DEFAULT_BRANCH\` |" - echo - echo "Auto-merge waits for the required contexts of the **base**. \`$BASE\` is not the" - echo "default branch, so branch protection does not cover it, the required set is empty," - echo "and the condition is satisfied the moment GitHub reads it — the pull request would" - echo "merge before any check started (measured: MeshWeaver.Plugins#1685, merged 61" - echo "seconds after it was opened)." - echo - echo "Merge this pull request by hand once you are satisfied with it, or retarget it at" - echo "\`$DEFAULT_BRANCH\` and the next event on it will arm it normally." - } >> "$GITHUB_STEP_SUMMARY" - - # 🚨 `gh pr merge --auto` either enables ordinary auto-merge or enqueues the PR, depending on - # the repository. Both mutations need `Pull requests: write`; the merge they later perform - # needs `Contents: write`. Both are requested explicitly, so an installation that is missing - # either fails HERE — loudly, on a step whose name says what it wanted — rather than degrading - # to a token that arms fine and then merges in a way that starts no CI. There is deliberately - # no fallback: falling back to `secrets.GITHUB_TOKEN` is precisely the outage this file's - # header documents. - # 🚨 `continue-on-error` — the ONE in this file, and the reason is that arming is an - # ACTION, not a GATE. AGENTS.md forbids this on a step that fetches a gate's input, because - # a verification that silently does not run is indistinguishable from one that passed. That - # hazard does not exist here: this job asserts NOTHING about the pull request. Every required - # context still runs, `Consolidate test results` still decides, and a PR that cannot be armed - # is merged by a human — the outcome is a lost convenience, not a lost check. - # - # What it buys is signal. A missing grant is a property of the REPOSITORY, identical on every - # PR at once, so failing here painted a red that was the same on all of them — a check whose - # value never varies carries no information about any PR, and a wall of permanent red is how - # a real red stops being read. The assertion still exists and is still RED; it moved to - # `arm-credential.yml`, which asks the repo-scoped question in the one place it is actionable. - # - # 🚨 That split is load-bearing: the tolerance below is only safe while that lane exists. - # `ArmedMergeMustTriggerMainsPushLanesGuard.ToleratingAFailedMintRequiresARepoScopedAssertion` - # fails the build if this `continue-on-error` outlives it. - - name: Mint an installation token whose merges actually trigger workflows + # 🚨 NO `continue-on-error`. Disarming is a SAFETY action: a disarm that silently does not + # happen lets an unreviewed head merge on a reviewed head's arm. So a mint that cannot get + # `Pull requests: write` is a RED run on the push that needed it, never a warning. + # (`arm-credential.yml` still asserts the same credential on a schedule, against the repository.) + - name: Mint an installation token that may take the arm off id: arm-token - continue-on-error: true uses: actions/create-github-app-token@v3 with: app-id: ${{ secrets.MESHWEAVER_APP_ID }} @@ -244,139 +123,26 @@ jobs: permission-contents: write permission-pull-requests: write - # PRESENT is not GRANTED. The mint fails with a bare "not permitted" when the App is - # installed but lacks one of the two permissions, and that message names neither the App nor - # the grant — so say it here, where a reader lands. - # - # 🚨 This is a WARNING, not an error, and the distinction is the point. The honest per-PR - # outcome of a missing grant is "this PR was not armed" — degraded automation, not a fault in - # the branch. Reporting it as a failure of the pull request is a false statement about the - # pull request, and it was being made on every one of them simultaneously. - - name: Say why nothing was armed, if the mint could not get the grant - if: steps.arm-token.outcome == 'failure' - run: | - echo "::warning::auto-merge was NOT armed on this PR — the meshweaver-cloud App could not" - echo "::warning::mint a token with Contents:write + PullRequests:write. Merge this PR by hand." - { - echo "### Auto-merge not armed" - echo - echo "The \`meshweaver-cloud\` App must carry **both** \`Contents: write\` and" - echo "\`Pull requests: write\`. It currently cannot mint one with both, so this PR was" - echo "left unarmed and needs a human merge once it is green." - echo - echo '```' - echo "gh api orgs/${{ github.repository_owner }}/installations \\" - echo " --jq '.installations[] | \"\(.app_slug) \(.permissions|tojson)\"'" - echo '```' - echo - echo "Grant *Pull requests: Read and write* in the App's settings, then **accept** the" - echo "new permission on the org installation — a requested-but-unaccepted permission is" - echo "not granted. \`arm-credential.yml\` is the lane that stays red until that is done." - } >> "$GITHUB_STEP_SUMMARY" - - # Silence is the thing to avoid. The warning above lands in a run log nobody opens for a - # lane whose entire normal output is the word "armed" — so an author whose stacked pull - # request is sitting there unarmed cannot tell this from a lane that is simply broken, and - # waits for an arm that is never coming. Say it once on the pull request itself, where the - # author already is. - # - # ONCE, not once per event: `synchronize` fires on every push, and a lane that repeats itself - # is a lane people mute. The marker is read back before anything is written. - - name: Say on the pull request that a non-default base was not armed - if: steps.base.outputs.onto-default == 'false' && steps.arm-token.outcome == 'success' + # 🚨 A PUSH INVALIDATES THE REVIEW. Read with REST; `--disable-auto` is the GraphQL mutation, + # which has no REST form. Read back afterwards: an exit code is the command's opinion, the + # pull request's `auto_merge` field is the evidence. + - name: Disarm on a push the review has not seen yet env: GH_TOKEN: ${{ steps.arm-token.outputs.token }} PR: ${{ github.event.pull_request.number }} REPO: ${{ github.repository }} - BASE: ${{ github.event.pull_request.base.ref }} - DEFAULT_BRANCH: ${{ github.event.repository.default_branch }} - MARKER: '' run: | set -euo pipefail - # Not swallowed: a read that fails says so and leaves the run-log warning standing. It - # must not fail the job — the honest per-PR outcome of a comment that could not be - # written is still "this pull request was not armed", which is already reported above. - if ! existing=$(gh api "repos/$REPO/issues/$PR/comments" --paginate --jq '.[].body'); then - echo "::warning::could not read the existing comments on #$PR, so nothing was posted; the run-log warning above stands." - exit 0 - fi - if printf '%s' "$existing" | grep -qF -- "$MARKER"; then - echo "already said so on #$PR — nothing to do" + armed=$(gh api "repos/$REPO/pulls/$PR" --jq '.auto_merge != null') + if [ "$armed" != "true" ]; then + echo "not armed — nothing to take off" exit 0 fi - body=$(printf '%s\n%s\n' \ - "$MARKER" \ - "**Auto-merge was not armed.** This pull request targets \`$BASE\`, not the default branch \`$DEFAULT_BRANCH\` — a non-default base has no branch protection and therefore no required contexts, so arming it would merge it immediately, before its own CI runs. Merge it by hand when you are ready, or retarget it at \`$DEFAULT_BRANCH\` to have it armed normally.") - gh pr comment "$PR" --repo "$REPO" --body "$body" - echo "commented on #$PR" - - # Conditioned on the MINT's outcome and on the BASE, never on whether a secret "looks set" — - # the shape AGENTS.md forbids. Without a token there is nothing to arm with; running gh - # anyway would fall back to the runner's default credential, which is the #2916 outage - # exactly. And without a default-branch base there is nothing to WAIT for, so an arm here is - # an immediate merge rather than a deferred one (see the base step above). - - name: Arm it - if: steps.arm-token.outcome == 'success' && steps.base.outputs.onto-default == 'true' - env: - GH_TOKEN: ${{ steps.arm-token.outputs.token }} - PR: ${{ github.event.pull_request.number }} - REPO: ${{ github.repository }} - run: | - set -euo pipefail - owner="${REPO%%/*}" - repository="${REPO#*/}" - # `autoMergeRequest` is populated on ordinary repositories, but it is NULL after a - # merge-queue repository accepts the PR. The queue has its own `mergeQueueEntry` field. - # Read both in one bounded GraphQL lookup so the read-back cannot report a successful - # queue admission as an unarmed PR (#3950, run 34555063094). - arm_state() { - gh api graphql \ - -f query='query($owner:String!,$repository:String!,$pr:Int!){repository(owner:$owner,name:$repository){pullRequest(number:$pr){state merged autoMergeRequest{enabledAt} mergeQueueEntry{id}}}}' \ - -f owner="$owner" -f repository="$repository" -F pr="$PR" \ - --jq '.data.repository.pullRequest | if . == null then "missing" elif .merged then "landed" elif .state == "CLOSED" then "closed" elif .mergeQueueEntry != null then "queued" elif .autoMergeRequest != null then "armed" else "open" end' - } - # Already armed/queued is the common case on `synchronize` and is NOT a failure. A - # merged/closed PR can also race this run; treat it as done, not as an error. - state=$(arm_state) - case "$state" in - armed) echo "already armed — nothing to do"; exit 0 ;; - queued) echo "already queued — nothing to do"; exit 0 ;; - landed) echo "PR already landed — nothing to do"; exit 0 ;; - closed) echo "::warning::pull request #$PR was closed without merging before it could be armed"; exit 0 ;; - missing) echo "::warning::pull request #$PR disappeared before it could be armed"; exit 0 ;; - esac - # Two repo shapes, one file. A merge QUEUE owns the merge strategy (core's is - # SQUASH), so `--merge` is redundant there and some gh versions REJECT the - # combination outright; on a repo with no queue, gh refuses to arm at all unless a - # method is named. So name the method, then fall back to letting the queue decide. - arm_error="" - if out=$(gh pr merge "$PR" --repo "$REPO" --merge --auto 2>&1); then - echo "arm requested (--merge): ${out:-ok}" - elif out=$(gh pr merge "$PR" --repo "$REPO" --auto 2>&1); then - echo "arm requested (strategy left to the merge queue): ${out:-ok}" - else - # Do not warn yet: another run or a human may have queued, armed or merged the PR - # between our first read and this command. The authoritative read-back below decides. - arm_error="$out" + gh pr merge "$PR" --repo "$REPO" --disable-auto + still=$(gh api "repos/$REPO/pulls/$PR" --jq '.auto_merge != null') + if [ "$still" = "true" ]; then + echo "::error::auto-merge is still armed on #$PR after --disable-auto — an unreviewed head may merge; take it off by hand" + exit 1 fi - # Read it back. An exit code of 0 is the command's opinion; this is the evidence. There - # are THREE successful states: ordinary auto-merge armed, merge-queue entry present, or - # the PR already landed because nothing remained to wait for. - state=$(arm_state) - case "$state" in - armed) echo "armed: #$PR will merge when the required contexts pass" ;; - queued) echo "queued: #$PR is in the merge queue" ;; - landed) echo "landed: #$PR had nothing left to wait for and merged immediately" ;; - closed) echo "::warning::pull request #$PR was closed without merging during the arm attempt" ;; - open) - # 🚨 Failing to arm must be VISIBLE. It is not fatal to the PR — a human can still - # merge it — but a silent failure here recreates exactly the state this exists to - # fix: a green PR that never lands, with nothing saying why. - if [ -n "$arm_error" ]; then - echo "::warning::could not arm auto-merge on #$PR — $arm_error" - else - echo "::warning::auto-merge still not set on #$PR after a clean arm — is 'Allow auto-merge' enabled on $REPO?" - fi - ;; - missing) echo "::warning::pull request #$PR disappeared after the arm command" ;; - esac + echo "Disarmed #$PR: the new head has not been reviewed yet; the control plane re-arms it once it is reviewed and every finding is answered." >> "$GITHUB_STEP_SUMMARY" + echo "disarmed #$PR" diff --git a/.github/workflows/review-answered.yml b/.github/workflows/review-answered.yml index 6108cc360f..452a488aed 100644 --- a/.github/workflows/review-answered.yml +++ b/.github/workflows/review-answered.yml @@ -1,7 +1,7 @@ # Hold a pull request while the automatic review is missing or any of its findings is unanswered. # # 🚨 WHY (MeshWeaver#4299, option B — the maintainer's decision of 2026-09-17). The ruleset reviews -# every pull request into main and auto-arm.yml arms every pull request the moment it opens, and +# every pull request into main and the control plane (PrArming) arms pull requests, while auto-arm.yml only disarms on a push, and # nothing joined the two: a review was ADVISORY. #4310 merged with 14 findings and 0 replies and # stopped the fleet's publication for six hours; 9 of the 31 merges on 2026-09-14/15 carried # unanswered findings; #4383 lost the fix push for its findings to the merge by a minute. In every diff --git a/AGENTS.md b/AGENTS.md index 716667c62d..2429af98a0 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -190,7 +190,7 @@ Full reference: [Issue Taxonomy and the Release Readiness Gate](src/MeshWeaver.D 🚨 **Merging is a shared action.** A merge supersedes the run QUEUED behind the one in flight, including one another session is waiting on. Before merging, check whether main has a run someone is gating a deploy on; if so, **hold and say so** — and push that hold to your subagents explicitly, because their default is merge-on-green. -🚦 **The MERGE QUEUE on core `main` is OFF** (measured 2026-09-27: `enqueuePullRequest` → "No merge queue found for branch 'main'"; the ruleset `main pr protection` carries no `merge_queue` rule). `main` merges on its required checks plus auto-merge: `gh pr merge --auto` ARMS auto-merge (`auto-arm.yml` does it for every non-draft PR; draft is the opt-out), and GitHub merges the moment the required set is green. `merge-queue-steward.yml` acts only on `dequeued` events and is dormant. Re-enabling the queue is a ruleset change, and a maintainer's — and even then nothing in it may wait on another repository (policy `core-merge-never-blocked`). Full reference: [MergeQueue.md](src/MeshWeaver.Documentation/Data/Architecture/MergeQueue.md) · [OnePromotionGate.md](src/MeshWeaver.Documentation/Data/Architecture/OnePromotionGate.md). +🚦 **The MERGE QUEUE on core `main` is OFF** (measured 2026-09-27: `enqueuePullRequest` → "No merge queue found for branch 'main'"; the ruleset `main pr protection` carries no `merge_queue` rule). `main` merges on its required checks plus auto-merge: `gh pr merge --auto` ARMS auto-merge (the control plane's PR steward, MeshWeaver.Plugins `PrArming`, arms a non-draft PR once its CURRENT head is reviewed, every finding answered and the required checks green; `auto-arm.yml` only DISARMS on a push; draft is the opt-out), and GitHub merges the moment the required set is green. `merge-queue-steward.yml` acts only on `dequeued` events and is dormant. Re-enabling the queue is a ruleset change, and a maintainer's — and even then nothing in it may wait on another repository (policy `core-merge-never-blocked`). Full reference: [MergeQueue.md](src/MeshWeaver.Documentation/Data/Architecture/MergeQueue.md) · [OnePromotionGate.md](src/MeshWeaver.Documentation/Data/Architecture/OnePromotionGate.md). 🚨 **A CLOSING KEYWORD IN A PR BODY READS NOTHING BUT THE NUMBER AFTER IT — not the negation in front of it, not the possessive behind it, not the sentence it sits in.** Measured on 2026-09-22, three times in one day: **`**Does not close #5057**` closed #5057** two seconds after the merge (#5201); `Closes #2299's classification half` closed the whole of #2299 against its own body (#5174); and a **documentation** PR narrating that misfire (`… closed #2299 …`) closed it AGAIN, 57 minutes after a deliberate reopen (#5190). **To mention an issue without closing it, write `Refs #N` or `see #N` — a disclaimer is not a way of being careful, it is a way of closing the issue while believing you have not.** 🚨 And a `sev:B`/`sev:H` issue may not be closed by a merge AT ALL: a merge puts the fix on `main`, while what the label gates is whether the defect is gone from the RUNNING portal, so those close on **post-roll production verification** (policy `severity-closes-on-verification`). `Closing keywords (no accidental close)` refuses all three shapes and hangs off the required check; when the verification has already happened, declare it per issue in the PR BODY as `Verified-closing: #N — `, which releases the severity check and nothing else. It needs no credential, so it runs on forks too. It meets about one merge in fourteen (measured over the last 99 merges: 16 carry a closing keyword, 7 would be red, and 6 of those 7 closed a `sev:H`). Full reference: [ClosingKeywordsAndIssueState.md](src/MeshWeaver.Documentation/Data/Architecture/ClosingKeywordsAndIssueState.md). diff --git a/src/MeshWeaver.Documentation/Data/Architecture/FleetCiProcess.md b/src/MeshWeaver.Documentation/Data/Architecture/FleetCiProcess.md index e2b9f2248c..c221cd8a97 100644 --- a/src/MeshWeaver.Documentation/Data/Architecture/FleetCiProcess.md +++ b/src/MeshWeaver.Documentation/Data/Architecture/FleetCiProcess.md @@ -17,7 +17,7 @@ satellite that re-implements a piece drifts, and the next incident is measured t | **Generated-lock conflicts** — a PR that conflicts only on `*/manifest.lock` lands anyway | `node-repo-resolve-locks.yml` + `.github/scripts/resolve-generated-conflicts.py` | a 12-line workflow on `push: main`, hourly, dispatch; the App secrets | | **Verdict adoption** — a merge from main (the resolver, `update-branch`, a hand merge) or a lock-only commit never costs the run | `.github/scripts/adopt-verdict.py`, decided by `node-repo-gate.yml` (`adopted-verdict: auto`) | nothing (Plugins decides itself in its change-set classifier and passes the URL) | | **Cancellation** — nothing on main cancels; a person's PR push supersedes; the bot's push never | `.github/scripts/check-main-runs-not-cancelled.py`, run by the validate lane | `cancel-in-progress: ${{ github.event_name == 'pull_request' && github.event.sender.type != 'Bot' }}` | -| Auto-merge armed on every non-draft PR to main; draft is the only hold | `auto-arm.yml` | the call | +| Auto-merge DISARMED on every push; arming is the control plane's (Plugins `PrArming`, on a reviewed, answered, green head) | `auto-arm.yml` (disarm-only, `synchronize`) | the call | | Build queue front door (admission through the build instance) | Plugins `admission` job today; a `node-repo-admission.yml` lane next | — | ## Why adoption is sound @@ -154,7 +154,7 @@ follow whatever `main` last passed. ## Draft is the hold — until the PR is IN the merge queue -`auto-arm.yml` arms auto-merge on every non-draft PR whose base is `main`, so **draft is the opt-out** +The control plane's PR steward arms auto-merge on a non-draft PR whose base is `main` once its current head is reviewed and answered ([Review Findings Answered](../ReviewFindingsAnswered) → *The arm gate*; `auto-arm.yml` only disarms on a push), so **draft is the opt-out** and the way any session holds a PR after a late finding. It stops being a hold the moment the entry is admitted to the merge queue, and that is not obvious from either UI. diff --git a/src/MeshWeaver.Documentation/Data/Architecture/HybridGitHubAzureDevOps.md b/src/MeshWeaver.Documentation/Data/Architecture/HybridGitHubAzureDevOps.md index 7522bd4472..9bde560824 100644 --- a/src/MeshWeaver.Documentation/Data/Architecture/HybridGitHubAzureDevOps.md +++ b/src/MeshWeaver.Documentation/Data/Architecture/HybridGitHubAzureDevOps.md @@ -84,7 +84,7 @@ that the fleet's GitHub-only shape does not. | Piece | Exists | The hybrid scenario adds | |---|---|---| -| Merge to `main` through the GitHub App | `auto-arm.yml` arms every non-draft PR into the merge queue with a token minted from `meshweaver-cloud`; the steward re-queues on evidence — [The Merge Queue](../MergeQueue), [GitHub App Credentials](../GitHubAppCredentials) | nothing | +| Merge to `main` through the GitHub App | the control plane's PR steward arms a reviewed, answered, green PR (`auto-arm.yml` disarms on a push, with a token minted from `meshweaver-cloud`); the steward re-queues on evidence — [The Merge Queue](../MergeQueue), [GitHub App Credentials](../GitHubAppCredentials) | nothing | | CD publishes an all-or-nothing set and registers it | `main-cd.yml` promotes and ends with one signed `platform-build` POST; the control instance registers `Hosting/PlatformBuilds/` — [Continuous Delivery Contract](../ContinuousDeliveryContract) | nothing | | The App's webhook reaches the control instance | `POST /webhooks/github`, HMAC-verified, dispatched by `GitHubWebhookProcessor` on `push`, `workflow_run`, … — [Webhook Inbox](../WebhookInbox) | a `push`-to-default-branch branch that files a `HelmRelease` action on every record whose `repository` is the pushed repository | | The deploy is an action on the record | `Hosting/InstanceAction` kind `HelmRelease`, `helmAction: capture \| adopt \| deploy`, dispatches the config repo's `helm-release.yml` and follows the run onto the node's log — [Operating from the portal](../OperatingFromThePortal) | a second **executor**: `azure-devops`, selected by the record (below) | diff --git a/src/MeshWeaver.Documentation/Data/Architecture/MergeQueue.md b/src/MeshWeaver.Documentation/Data/Architecture/MergeQueue.md index 6db5a0146e..3c02597fab 100644 --- a/src/MeshWeaver.Documentation/Data/Architecture/MergeQueue.md +++ b/src/MeshWeaver.Documentation/Data/Architecture/MergeQueue.md @@ -305,28 +305,29 @@ and "the lane is broken" become the same page. Instead the job runs, a `::warnin summary name the base, and one idempotent comment says so on the pull request itself. Draft and fork stay on the job condition, because for those there is genuinely nothing to say. -**The other lanes were checked and are clean structurally, not by luck** — which is why none is -exempted by name in the guard: `arm-credential.yml` mints and inspects a token and arms nothing; -this steward acts only on a `dequeued` event, which cannot occur for a pull request never admitted -to a queue, and a queue exists only on a branch configured to have one; `release.yml` and -`node-repo-platform-ref-bump.yml` OPEN pull requests (`--base main`, explicitly) without arming them. - -`ArmedMergeMustTriggerMainsPushLanesGuard` holds all three properties: the arming step is governed -by an `if:` consuming a step output whose producer reads `default_branch` (a condition moved into -`env:`, or one that consults only the token mint, breaks the chain and fails); no workflow in the -directory arms without that chain; and the non-default path still warns and still comments. +**This section is the HISTORY of the retired arm lane, kept for the lesson it taught.** `auto-arm.yml` +no longer arms anything, so the base-chain guard it once carried is gone with the arming step: the +control plane (Plugins `PrArming`) is the only armer and applies the arm gate, which includes the +base-is-the-default-branch condition. What still holds the property in core is +`ArmedMergeMustTriggerMainsPushLanesGuard.NoWorkflowArmsAutoMerge`: no workflow in the directory arms +auto-merge at all, so a stacked pull request can no longer be armed by a workflow here. The other +lanes were checked structurally, not by luck: `arm-credential.yml` mints and inspects a token and arms +nothing; the steward acts only on a `dequeued` event; `release.yml` and +`node-repo-platform-ref-bump.yml` OPEN pull requests (`--base main`) without arming them. ## Working with the queue - **`gh pr merge --auto` enqueues — but only onto the default branch.** With a queue enabled, - "auto-merge" means *enqueue when the PR's own required checks are green*. `auto-arm.yml` does this - for every non-draft PR **whose base is the default branch**; see the section above for why a - stacked PR is deliberately left unarmed. Marking a PR **draft** is the opt-out. + "auto-merge" means *enqueue when the PR's own required checks are green*. Arming is the control + plane's (Plugins `PrArming`), on a head that has been reviewed, whose findings are answered and + whose required checks are green, and only for a pull request whose base is the default branch; see + the section above for why a stacked PR is deliberately left unarmed. `auto-arm.yml` only + DISARMS, on every push. Marking a PR **draft** still holds it. - 🚨 **DISARMING auto-merge does NOT hold a pull request. Converting it to DRAFT does.** This is the single most likely way to believe you have stopped a merge and be wrong — see the section below. - **A push to a queued branch ejects it.** GitHub removes the entry (reason `MANUAL`-shaped from the - steward's point of view: it comments once and takes no action); auto-arm re-arms on the - `synchronize` event, so the new head re-enters the queue once its own run is green. Do not push to + steward's point of view: it comments once and takes no action); the push disarms it + (`auto-arm.yml`), and the control plane re-arms the new head once it is reviewed, answered and green. Do not push to a queued branch expecting the queue to pick up the new commit in place. - **Dequeue via GraphQL, never by re-ordering.** `gh api graphql -f query='mutation($id:ID!){dequeuePullRequest(input:{id:$id}){clientMutationId}}' -f id=`. @@ -341,35 +342,34 @@ directory arms without that chain; and the non-default path still warns and stil ## 🚨 Disarming auto-merge is not a hold — DRAFT is the only durable one **Measured: `MeshWeaver.Plugins#1683` was deliberately disarmed during a merge window and merged -anyway.** Nothing malfunctioned. `auto-arm.yml` fires on `synchronize` among other events, so **the -next push re-armed it** — and a pull request being held is exactly a pull request someone is still -pushing to. A push is what happened to re-arm *that* one; it is not the only thing that would have. +anyway.** Nothing malfunctioned: at that time `auto-arm.yml` armed on `synchronize`, so the next push +re-armed it. That lane has since been retired, and the shape of the lesson changed with it. -The asymmetry is the whole point, and it is structural rather than a bug to fix: +Today `auto-arm.yml` runs on `synchronize` only and **disarms**; it never arms. The control plane arms +a head once it is reviewed, every finding is answered and the required checks are green, so a +disarmed pull request stays disarmed until the control plane judges a new head ready. That is still +not a hold: any such head is armed again without anyone asking, and a pull request someone is still +pushing to is exactly one that will be. Draft remains the durable hold, because the control plane +never arms a draft. | act | what it is | how long it lasts | |---|---|---| -| `gh pr merge --disable-auto` | a **state** GitHub owns | until the next event this lane fires on: `synchronize` (a push), `reopened`, or `ready_for_review`. **A push is the common one, not the only one** — reopening a closed PR, or marking a draft ready, re-arms with no push at all. (`opened` cannot apply: a pull request has to exist before it can be disarmed.) | -| convert to **draft** | a **property of the pull request** the lane reads | until *you* mark it ready; `ready_for_review` is what re-arms | +| `gh pr merge --disable-auto` | a **state** GitHub owns | until the control plane next judges the head ready and arms it | +| convert to **draft** | a **property of the pull request** the control plane reads | until *you* mark it ready | -Read off the merged lane: `types: [opened, reopened, ready_for_review, synchronize]` and a job -condition of `github.event.pull_request.draft == false`. A draft is never armed no matter how many -times it is pushed to, and marking it ready arms it immediately — so draft is a **two-sided** hold -the lane honours on both edges, while a disarm is a one-shot the next event overwrites. (GitHub also -disables auto-merge when a pull request is converted to draft, so the conversion does both halves in -one act. That last clause is GitHub's documented behaviour rather than something measured here.) +(GitHub also disables auto-merge when a pull request is converted to draft, so the conversion does +both halves in one act. That clause is GitHub's documented behaviour rather than something measured here.) -**So: to hold a pull request, convert it to draft. Never rely on `--disable-auto`** — and if you -find a PR merged that you thought you had stopped, look for one of this lane's trigger events after -the disarm before looking for anything else. A push is the first thing to check because it is the -most frequent, but a reopen or a ready-for-review re-arms just as completely and leaves no new -commit to notice, which makes those the harder two to spot afterwards. +**So: to hold a pull request, convert it to draft. Never rely on `--disable-auto`** — and if you find +a PR merged that you thought you had stopped, look for the control plane's arm after the disarm. -**#4057 does not change this.** That fix stops the lane arming a pull request whose base is not the -default branch; for an ordinary pull request onto `main`, re-arming after a disarm is unchanged. +**#4057 does not change this.** That fix stopped the retired lane arming a pull request whose base is +not the default branch; the control plane's arm gate carries the same condition now. ### The option considered and NOT taken +*(Written when the lane still armed; kept as the design record. The re-arming agent is now the control plane, and the same two options would apply to it.)* + Making the lane refuse to re-arm a pull request a human explicitly disarmed is attractive and is **not small**, so it was left alone rather than half-built: diff --git a/src/MeshWeaver.Documentation/Data/Architecture/ReviewFindingsAnswered.md b/src/MeshWeaver.Documentation/Data/Architecture/ReviewFindingsAnswered.md index eed34675f9..8d916fec5e 100644 --- a/src/MeshWeaver.Documentation/Data/Architecture/ReviewFindingsAnswered.md +++ b/src/MeshWeaver.Documentation/Data/Architecture/ReviewFindingsAnswered.md @@ -61,8 +61,8 @@ is still BLOCKED"**. ## Why: a review was advisory The ruleset `main pr protection` (2128472) carries a `copilot_code_review` rule, so every pull -request into `main` is reviewed. `auto-arm.yml` arms every pull request for the merge queue the -moment it opens. Nothing joined the two, so a merge waited for the required checks and never for +request into `main` is reviewed. The control plane (`PrArming`) arms a pull request once its review +lane is satisfied; `auto-arm.yml` only DISARMS on a push. Nothing joined the two, so a merge waited for the required checks and never for the review — and on the pull requests whose checks are fast, the checks usually won. | Instance | What merged past the review | Cost | @@ -1060,6 +1060,68 @@ check counts replies whose `in_reply_to_id` is the reviewer's root comment, and thread it still considers unanswered. If the log disagrees with what you think you answered, read that list before reaching for the re-run. +## The arm gate — auto-merge is armed only on a reviewed, answered head + +**Measured 2026-10-03/04 on MeshWeaver.Plugins:** `auto-arm.yml` armed auto-merge on every open, push +and undraft, BEFORE the internal review had run on the new head. On Plugins the review is +comment-only and nothing required waits for it, so #2549 (6 of 6 findings unanswered), #2643/#2647 +(reviewer unavailable) and Memex#641 merged unreviewed or unanswered, and agents disarmed by hand +after every push (#2791 twice in an hour). + +**Arming is a decision, so it moved to the control plane** (policy +[`review-then-suites`](../PolicyNotProse)). The control instance's PR steward (MeshWeaver.Plugins +`PrArming`, App `systemorph-com`) already reads every fleet pull request's review state and +answered-findings verdict; it is the one place that arms. **No workflow arms auto-merge any more** — +`ArmedMergeMustTriggerMainsPushLanesGuard.NoWorkflowArmsAutoMerge` fails the build if one does. + +### What the control plane arms on + +It arms a pull request only when **all** of these hold, read on its **current head**: + +1. it is open, not a draft, and its head branch is in the same repository (a fork never arms itself); +2. its base is the repository's **default branch**, read from `repos/{o}/{r}` and never assumed. An + unprotected base has an empty required set, so an arm there is an immediate merge + (MeshWeaver.Plugins#1685 merged 61 seconds after it opened, before its own CI started); +3. the head has **its own review**: a Copilot review submitted against the current head (policy + [`copilot-code-review`](../PolicyNotProse); `commit_id` equal to the head, not `PENDING`, not a + refusal — `copilot_review_on`, see *Copilot is the reviewer again* above), or, while one is still + posted, a completed `internal-review` check run of the `systemorph-com` App (id 4918443) on the + head whose newest run is **not** the neutral `Reviewer unavailable …` degradation. The degradation + releases the *merge* gate's review condition (a down reviewer must not hold every pull request), + but it is not a review — arming on it would land an unreviewed change nobody decided to merge, so + a person merges it by hand; +4. every thread the automatic reviewer opened has a reply from a person, following `in_reply_to_id` + to the root, read from a provably complete listing — the **same** `reviewer_threads` / + `listing_incomplete` predicate this gate uses, so the two can never disagree; +5. every required status check of the base is `success` on the head (`required_checks_green`, + policy `review-then-suites` — see [Staged Pull Request Pipeline](../StagedPullRequestPipeline)). + +The reference implementation of 1 and 3–5 is `check-review-answered.py --arm-gate` +(`arm_readiness`), and its `--self-test` cases are the test vectors the control plane's port must +pass. The arm is the GraphQL `enablePullRequestAutoMerge` mutation (or `enqueuePullRequest` on a +base with a merge queue) carrying `expectedHeadOid` = the head it judged, so an arm that races a push +is refused by GitHub instead of landing on the new, unreviewed head. It is attempted **once per +head** and read back (`autoMergeRequest` / `mergeQueueEntry`); a head a person disarmed is not +re-armed. + +🚨 **The port has to keep up with the reference.** `PrArming` is a one-for-one port of +`arm_readiness`, so a condition widened here is not live until the port carries it. Copilot's +acceptance in condition 3 is exactly such a widening (the register lists the port as owed under +`copilot-code-review`): until `PrArming` accepts a Copilot review of the head, a head reviewed only +by Copilot is never armed by the control plane and needs a person's arm. + +### What stays in the workflow: the disarm + +**A push invalidates the review**, and GitHub keeps auto-merge armed across a push by anyone with +write access — so a pull request armed on a reviewed head would merge the next, unreviewed head the +moment the required contexts pass. `auto-arm.yml` (still the fleet's one lane, still called by every +repository through `workflow_call`, its name kept so no caller changes) therefore now does exactly +one thing: on `synchronize`, if the pull request is armed, it takes the arm off (`gh pr merge +--disable-auto`) and reads the result back. It reacts to the event itself, so a fast CI cannot +outrun it, and it tolerates no failed step: a disarm that silently did not happen is the hole +reopened. It uses the meshweaver-cloud token it always used (`contents: write`, +`pull-requests: write`) — no `checks` permission, and no change to any caller's grants. + ## Related - [The Merge Queue](../MergeQueue) — the queue this check runs in, and the steward named in the rollout diff --git a/test/MeshWeaver.Documentation.Test/ArmedMergeMustTriggerMainsPushLanesGuard.cs b/test/MeshWeaver.Documentation.Test/ArmedMergeMustTriggerMainsPushLanesGuard.cs index 4625a6db23..121f2b9bee 100644 --- a/test/MeshWeaver.Documentation.Test/ArmedMergeMustTriggerMainsPushLanesGuard.cs +++ b/test/MeshWeaver.Documentation.Test/ArmedMergeMustTriggerMainsPushLanesGuard.cs @@ -168,7 +168,7 @@ public void TheArmLaneNamesAMintedInstallationTokenExplicitly() lines.Any(l => l.Contains("GH_TOKEN", StringComparison.Ordinal) && l.Contains("steps.arm-token.outputs.token", StringComparison.Ordinal)), - "auto-arm.yml's arm step does not pass the minted token as GH_TOKEN. Without it gh " + "auto-arm.yml's disarm step does not pass the minted token as GH_TOKEN. Without it gh " + "falls back to the runner's default credential — the #2916 failure, by omission " + "rather than by choice."); @@ -176,44 +176,13 @@ public void TheArmLaneNamesAMintedInstallationTokenExplicitly() { Assert.True( lines.Any(l => l.Contains(permission, StringComparison.Ordinal)), - $"auto-arm.yml's token mint no longer requests '{permission}'. Arming is the " - + "enablePullRequestAutoMerge mutation (Pull requests: write) and the merge it " - + "performs writes to the branch (Contents: write); requesting both explicitly is " - + "what makes a missing grant fail at the mint instead of somewhere downstream."); + $"auto-arm.yml's token mint no longer requests '{permission}'. Taking an arm off is the " + + "disablePullRequestAutoMerge mutation (Pull requests: write) on a pull request whose " + + "merge writes to the branch (Contents: write); requesting both explicitly is what " + + "makes a missing grant fail at the mint instead of somewhere downstream."); } } - /// - /// A queue admission is not represented by autoMergeRequest. Core PR #3950 entered the - /// merge queue successfully on 2026-09-11, while the auto-arm read-back saw a null - /// autoMergeRequest and warned that nothing had been armed (run 34555063094). The queue - /// entry is the positive evidence for that repository shape; ordinary satellites still use - /// the auto-merge request. - /// - [Fact] - public void TheArmReadBackRecognisesOrdinaryAutoMergeAndMergeQueueAdmission() - { - var path = Path.Combine(WorkflowsDir(), "auto-arm.yml"); - var armStep = Assert.Single( - StepBlocks(File.ReadAllText(path)), - block => block.Contains("- name: Arm it", StringComparison.Ordinal)); - - Assert.Contains("state merged autoMergeRequest{enabledAt}", armStep, StringComparison.Ordinal); - Assert.Contains("autoMergeRequest{enabledAt}", armStep, StringComparison.Ordinal); - Assert.Contains("mergeQueueEntry{id}", armStep, StringComparison.Ordinal); - Assert.Contains("elif .merged then \"landed\"", armStep, StringComparison.Ordinal); - Assert.Contains("elif .state == \"CLOSED\" then \"closed\"", armStep, StringComparison.Ordinal); - Assert.Contains("elif .mergeQueueEntry != null then \"queued\"", armStep, StringComparison.Ordinal); - Assert.Contains("elif .autoMergeRequest != null then \"armed\"", armStep, StringComparison.Ordinal); - Assert.Contains("queued) echo \"queued: #$PR is in the merge queue\"", armStep, StringComparison.Ordinal); - - var failedCommand = armStep.IndexOf("arm_error=\"$out\"", StringComparison.Ordinal); - Assert.True(failedCommand >= 0, "A failed arm command is no longer captured for the read-back."); - Assert.True( - armStep.IndexOf("state=$(arm_state)", failedCommand, StringComparison.Ordinal) > failedCommand, - "The failed-command path no longer re-reads state, so a racing successful arm is reported as a failure."); - } - /// /// 🚨 auto-arm.yml tolerates a failed mint, and that tolerance is only defensible /// because the assertion it drops was MOVED rather than deleted. @@ -513,47 +482,6 @@ public void ThePlatformRefBumpLaneReportsItsOwnFailureSomewhereAPersonReads() + "lane spent its whole existence in. Assert the input and fail RED naming it instead."); } - /// - /// Every steps.<id>.outputs.* reference that appears inside a step's if: — - /// and nowhere else in the step. A reference sitting in env: says the step USES a value; - /// only one inside if: says the step is GOVERNED by it, and that difference is the whole - /// assertion below. Handles both the one-line form and a block scalar (if: >-), whose - /// continuation lines are the ones indented past the if: itself. Pure. - /// - private static string[] StepOutputsGoverningTheStep(string stepBlock) - { - var lines = stepBlock.Split('\n'); - var governing = new List(); - for (var i = 0; i < lines.Length; i++) - { - var indent = lines[i].Length - lines[i].TrimStart().Length; - if (!lines[i].TrimStart().StartsWith("if:", StringComparison.Ordinal)) - continue; - governing.Add(lines[i]); - for (var j = i + 1; j < lines.Length; j++) - { - if (lines[j].Trim().Length == 0) - continue; - if (lines[j].Length - lines[j].TrimStart().Length <= indent) - break; - governing.Add(lines[j]); - } - } - - return [.. Regex - .Matches(string.Join('\n', governing), @"steps\.([A-Za-z0-9_.\-]+)\.outputs") - .Select(m => m.Groups[1].Value) - .Distinct(StringComparer.Ordinal)]; - } - - /// The id: a step declares, or null. Pure. - private static string? StepId(string stepBlock) => - ExecutableLines(stepBlock) - .Select(l => Regex.Match(l, @"^id:\s*([A-Za-z0-9_.\-]+)\s*$")) - .Where(m => m.Success) - .Select(m => m.Groups[1].Value) - .FirstOrDefault(); - /// /// A step block ARMS auto-merge when it runs the command that either enables auto-merge or /// admits the pull request to a merge queue. Both are gh pr merge --auto; the GraphQL @@ -566,173 +494,94 @@ private static bool ArmsAutoMerge(string stepBlock) => || l.Contains("enablePullRequestAutoMerge", StringComparison.Ordinal)); /// - /// 🚨🚨 Arming a pull request whose BASE is not the default branch is not "merge when green" — - /// it is "merge NOW", and this guard is the thing that keeps it from being reintroduced. - /// - /// GitHub's auto-merge waits for exactly the contexts BRANCH PROTECTION requires of the - /// pull request's BASE. Protection in this fleet is configured on the default branch and - /// nowhere else, so a STACKED pull request — one opened against another pull request's feature - /// branch — has an unprotected base, an EMPTY required set, and a condition that is satisfied - /// the instant it is first read. - /// - /// Measured, 2026-09-11. MeshWeaver.Plugins#1685 was opened against the feature - /// branch of #1681. auto-arm.yml armed it and GitHub merged it at 21:03:23Z — 61 seconds - /// after it was opened, before its own CI had started a single job. Nothing failed and - /// nothing was bypassed: the stack collapsed unreviewed, exactly as configured. This file is - /// the fleet's only copy of the arm lane, reached by every satellite through - /// workflow_call, so the same hole was open in all eight repositories simultaneously. - /// + /// 🚨🚨 NO WORKFLOW ARMS AUTO-MERGE — arming is the control plane's decision (policy review-then-suites). /// - /// What is asserted is the CHAIN, not a spelling. The arming step must be governed - /// by an if:, that if: must consume a step output, and the step producing it must - /// read the repository's default branch from the event payload. A condition moved into - /// env:, or an if: that only consults the token mint, both break the chain and - /// fail here — which is the shape the regression would actually take. - /// - [Fact] - public void TheArmLaneArmsOnlyOntoTheRepositoryDefaultBranch() - { - var path = Path.Combine(WorkflowsDir(), "auto-arm.yml"); - Assert.True(File.Exists(path), $"{path} is missing — the arm lane is the subject of this guard."); - - var blocks = StepBlocks(File.ReadAllText(path)); - var armStep = Assert.Single(blocks.Where(ArmsAutoMerge)); - - var governing = StepOutputsGoverningTheStep(armStep); - Assert.True( - governing.Length > 0, - "auto-arm.yml's arming step is not governed by any step output — so it arms every pull " - + "request this lane sees, whatever its base.\n" - + "Auto-merge waits for the required contexts of the BASE. A base that is not the " - + "default branch has no branch protection and therefore no required contexts, so the " - + "condition is met the moment GitHub reads it and the pull request merges immediately " - + "(measured: MeshWeaver.Plugins#1685, merged 61 seconds after it was opened, before a " - + "single check had started).\n" - + "Read github.event.repository.default_branch in a step, compare it with " - + "github.event.pull_request.base.ref, and condition the arm on the result."); - - var producers = blocks - .Where(b => StepId(b) is { } id && governing.Contains(id, StringComparer.Ordinal)) - .ToArray(); - - Assert.True( - producers.Any(b => ExecutableLines(b).Any(l => l.Contains("default_branch", StringComparison.Ordinal))), - "auto-arm.yml's arming step is conditioned on step outputs " - + $"({string.Join(", ", governing)}), but none of the steps producing them reads the " - + "repository's default branch. The arm is therefore still unguarded against a " - + "non-default base — the #1685 failure, by omission rather than by choice.\n" - + "The step that decides must read github.event.repository.default_branch."); - - // 🚨 And it must be READ, never assumed. Every repository in the fleet uses `main` today, - // which is exactly what makes a literal indistinguishable from a correct check while being - // a copy of a fact — the drift the header of this lane exists to end. One lane serves eight - // repositories; the day one of them renames its default branch, a literal arms nothing and - // says nothing. - var literalMain = ExecutableLines(File.ReadAllText(path)) - .Where(l => Regex.IsMatch(l, @"(^|[^A-Za-z0-9_\-/.])main([^A-Za-z0-9_\-/.]|$)")) - .ToArray(); - - Assert.True( - literalMain.Length == 0, - "auto-arm.yml names the branch 'main' literally: " - + $"{string.Join(" | ", literalMain)}.\n" - + "This lane is called by every repository in the fleet through workflow_call, and the " - + "default branch is on the event payload in both modes. A literal is a copy of a fact " - + "that every repo happens to share today — precisely the drift that made the hand-copied " - + "versions of this file diverge into four variants. Read " - + "github.event.repository.default_branch instead."); - } - - /// - /// The same rule, ratcheted across the whole directory: no workflow anywhere in this repository - /// may arm auto-merge without the base check. auto-arm.yml is the only lane that arms - /// today, and the point of a directory-wide sweep is that the SECOND one — a release lane, a - /// bump lane, a convenience added in a hurry — cannot ship without meeting the same bar. + /// Arming says "this head was reviewed and every finding answered, so let it land when + /// green". auto-arm.yml used to make that call on every open, push and undraft — BEFORE + /// the automatic review had landed on the new head — and on MeshWeaver.Plugins, where the review is + /// comment-only, #2549, #2643/#2647 and Memex#641 merged with unanswered findings or no review + /// at all. The decision now lives with the control instance's PR steward (MeshWeaver.Plugins + /// PrArming, App systemorph-com), which already reads the review state and the answered-findings + /// verdict, and applies check-review-answered.py --arm-gate's predicate, the default-branch + /// base check (an unprotected base has an empty required set, so an arm there is an immediate + /// merge — MeshWeaver.Plugins#1685, merged 61 seconds after it opened) and an + /// expectedHeadOid on the mutation. /// - /// The neighbours were checked when this was written and are clean for structural - /// reasons rather than by luck, which is why they are not exempted by name: - /// arm-credential.yml mints and inspects a token and arms nothing; - /// merge-queue-steward.yml acts only on a dequeued event, which cannot occur for - /// a pull request that was never admitted to a merge queue — and a queue exists only on a - /// branch that has one configured, i.e. a protected one; and release.yml / - /// node-repo-platform-ref-bump.yml OPEN pull requests (already covered above) without - /// arming them. + /// So a workflow that arms is a SECOND decider, deciding without the review — exactly the + /// hole that was closed. This replaces the earlier per-workflow default-branch checks with the + /// stronger rule: there is nothing in this directory left to check them on. /// [Fact] - public void NoWorkflowArmsAutoMergeWithoutReadingTheDefaultBranch() + public void NoWorkflowArmsAutoMerge() { var offenders = Directory .EnumerateFiles(WorkflowsDir(), "*.yml") - .Select(path => (path, blocks: StepBlocks(File.ReadAllText(path)))) - .Where(w => w.blocks.Any(ArmsAutoMerge)) - .Where(w => - { - var governing = w.blocks.Where(ArmsAutoMerge).SelectMany(StepOutputsGoverningTheStep).ToArray(); - return !w.blocks.Any(b => - StepId(b) is { } id - && governing.Contains(id, StringComparer.Ordinal) - && ExecutableLines(b).Any(l => l.Contains("default_branch", StringComparison.Ordinal))); - }) - .Select(w => Path.GetFileName(w.path)) + .Where(path => StepBlocks(File.ReadAllText(path)).Any(ArmsAutoMerge)) + .Select(Path.GetFileName) .OrderBy(n => n, StringComparer.Ordinal) .ToArray(); Assert.True( offenders.Length == 0, - $"These workflows arm auto-merge without checking the base branch: {string.Join(", ", offenders)}.\n" - + "Auto-merge waits for the required contexts of the pull request's BASE, and only the " - + "default branch carries branch protection in this fleet. On any other base the required " - + "set is EMPTY, so arming merges the pull request immediately — measured on " - + "MeshWeaver.Plugins#1685, which was opened against another pull request's feature branch " - + "and merged 61 seconds later, before its own CI had started.\n" - + "Read github.event.repository.default_branch, compare it with " - + "github.event.pull_request.base.ref, and condition the arm on the result."); + $"These workflows arm auto-merge: {string.Join(", ", offenders)}.\n" + + "Arming is the control plane's decision (the control instance's PR steward, App " + + "systemorph-com): it arms only a non-draft, same-repository pull request onto the " + + "default branch whose CURRENT head has its own review (a Copilot review against that " + + "head, or a completed internal-review run that is not the 'Reviewer unavailable' " + + "degradation), every automatic-review thread answered by a person, and every required " + + "check green. A workflow that arms decides without the review — measured: #2549, " + + "#2643/#2647 and Memex#641 merged unreviewed that way. See " + + "Doc/Architecture/ReviewFindingsAnswered, 'The arm gate'."); } /// - /// 🚨 Not arming must be SAID, not merely done. + /// 🚨 What stays in auto-arm.yml is the DISARM, and it must stay able to happen. /// - /// An author whose stacked pull request is sitting there unarmed cannot tell "the lane - /// decided not to arm this one" from "the lane is broken" — both look like nothing happening, - /// which is the same defect as a gate that skips silently, one level down. So the base decision - /// is reported twice: a ::warning:: in the run log, and one comment on the pull request - /// itself, where the author already is. - /// - /// And the JOB must not be what skips. Moving the base test onto the job's if: - /// would be the tidy-looking version of this fix and would delete the message with it — a - /// skipped job renders like a passed one and carries no warning, no summary and no comment. - /// This test pins the base out of the job condition for exactly that reason. + /// GitHub keeps auto-merge armed across a push by anyone with write access, so a pull + /// request armed on a reviewed head would merge the NEXT, unreviewed head the moment the + /// required contexts pass. The disarm is a reaction to the synchronize event itself, so + /// a fast CI cannot outrun it. Pinned here: it runs on synchronize, it takes the arm off + /// (--disable-auto) and reads the result back, it tolerates no failed step (a disarm + /// that silently did not happen is the hole reopened), and it names no branch literally — the + /// lane serves every repository in the fleet through workflow_call. /// [Fact] - public void APullRequestOnANonDefaultBaseIsToldWhyItWasNotArmed() + public void TheAutoMergeLaneDisarmsOnEveryPushAndToleratesNoFailure() { - var text = File.ReadAllText(Path.Combine(WorkflowsDir(), "auto-arm.yml")); - var blocks = StepBlocks(text); - - var deciders = blocks - .Where(b => StepId(b) is not null) - .Where(b => ExecutableLines(b).Any(l => l.Contains("default_branch", StringComparison.Ordinal))) - .ToArray(); + var path = Path.Combine(WorkflowsDir(), "auto-arm.yml"); + Assert.True(File.Exists(path), $"{path} is missing — every fleet repository calls it through workflow_call."); + var text = File.ReadAllText(path); + var lines = ExecutableLines(text); + + var jobCondition = Regex.Match(text, @"\n if: (github\.event\.action == 'synchronize')[ \t]*\n"); + Assert.True(jobCondition.Success, + "auto-arm.yml's job-level `if:` is no longer exactly `github.event.action == 'synchronize'`. " + + "Only a push invalidates a review, so the disarm must run on that event and nothing else."); + + var disarm = Assert.Single(StepBlocks(text).Where(b => + ExecutableLines(b).Any(l => l.Contains("gh pr merge", StringComparison.Ordinal) + && l.Contains("--disable-auto", StringComparison.Ordinal)))); + Assert.True( + ExecutableLines(disarm).Count(l => l.Contains(".auto_merge != null", StringComparison.Ordinal)) >= 2, + "auto-arm.yml's disarm step no longer reads `auto_merge` both before and after " + + "--disable-auto. The read-back is the evidence the arm is off; without it a disarm " + + "that GitHub ignored reads exactly like one that worked."); Assert.True( - deciders.Any(b => ExecutableLines(b).Any(l => l.Contains("::warning::", StringComparison.Ordinal))), - "auto-arm.yml decides not to arm a non-default base without emitting a ::warning::. " - + "Deciding silently is indistinguishable from being broken: the author waits for an arm " - + "that is never coming, with nothing anywhere saying why."); + lines.All(l => !l.Contains("continue-on-error", StringComparison.Ordinal)), + "auto-arm.yml carries continue-on-error. Disarming is a SAFETY action: a mint or a " + + "disarm that fails quietly leaves a reviewed head's arm on an unreviewed head, which " + + "then merges the moment the required contexts pass."); + var literalMain = lines + .Where(l => Regex.IsMatch(l, @"(^|[^A-Za-z0-9_\-/.])main([^A-Za-z0-9_\-/.]|$)")) + .ToArray(); Assert.True( - blocks.Any(b => ExecutableLines(b).Any(l => l.Contains("gh pr comment", StringComparison.Ordinal))), - "auto-arm.yml no longer comments on the pull request when it declines to arm. The run " - + "log of a lane whose entire normal output is the word 'armed' is not somewhere anyone " - + "looks; the one place the author already is, is the pull request."); - - // The job-level `if:` may test draft and fork — both properties of the event that mean - // "there is nothing to do here at all". The base is different: there IS something to do, - // namely say why nothing was armed. - var jobCondition = Regex.Match(text, @"\n if: >-\n((?: .*\n)+)"); - Assert.True(jobCondition.Success, "auto-arm.yml's job-level `if:` is no longer in the expected block form."); - Assert.DoesNotContain("base.ref", jobCondition.Groups[1].Value, StringComparison.Ordinal); - Assert.DoesNotContain("default_branch", jobCondition.Groups[1].Value, StringComparison.Ordinal); + literalMain.Length == 0, + $"auto-arm.yml names the branch 'main' literally: {string.Join(" | ", literalMain)}.\n" + + "This lane is called by every repository in the fleet through workflow_call; a literal " + + "is a copy of a fact every repo happens to share today. Read " + + "github.event.repository.default_branch instead."); } private static string FindRepoRoot()