fix(ci): arm auto-merge only onto the repository's default branch by rbuergi · Pull Request #4057 · Systemorph/MeshWeaver

rbuergi · 2026-09-11T21:30:22Z

The defect

auto-arm.yml armed auto-merge on every non-draft, non-fork pull request, whatever its base.

--auto does not mean "merge when this pull request is green". It means "merge when the contexts
branch protection requires of the BASE are satisfied" — and protection in this fleet is
configured on the default branch and nowhere else. A stacked pull request, opened against
another pull request's feature branch, therefore had a base protected by nothing, an empty
required set, and a condition satisfied the instant GitHub first read 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 collapsed
unreviewed, exactly as configured.

This reaches six of the seven callers without a satellite-side change. auto-arm.yml is the
fleet's only copy of the arm lane; every other repository reaches it through workflow_call with a
thin caller that forwards the App credential and nothing else. Measured today over the contents API,
all seven callers exist and six pin the lane at @main — MeshWeaver.Plugins, .Reinsurance,
.SocialMedia, .Education, .Crm and .Manufacturing — so the fix lands there the moment this merges.

🚨 The exception is Systemorph/Memex, which pins a SHA
(auto-arm.yml@c7fef7a2d9cebce36889028c0b542acb05e77947, 971 commits behind core's main), so
it does not pick this up and needs a one-line pin bump. That follows this PR, in dependency
order.

The fix

The lane reads github.event.repository.default_branch off the event — never a literal main,
which would be a copy of a fact every repo happens to share today and the exact drift the
single-copy design exists to end — compares it with github.event.pull_request.base.ref, and arms
only on a match.

🚨 The decision is announced, and the JOB is never what skips. Putting the base test on the
job's if: is the tidy-looking version of this fix and deletes the message with it: a skipped job
renders like a passed one and carries no warning, no summary and no comment, so "the lane declined"
and "the lane is broken" become the same page — an author waits for an arm that is never coming.
Instead the job runs, a ::warning:: plus a step summary name the base, and one idempotent
comment (marker read back before writing, so synchronize cannot repeat it) says so on the pull
request itself. Draft and fork stay on the job condition, because for those there is genuinely
nothing to say.

Everything else is unchanged: draft is still the opt-out, forks are still skipped, and the arm still
runs on the minted App installation token (#2916).

No gate can skip silently here. The new condition asks a property of the event, never
whether a secret is set; the credential assert, the base read and the mint all still run
unconditionally; and this lane publishes no required context — measured today, core requires
Consolidate test results only, and MeshWeaver.Plugins / .Reinsurance / .SocialMedia / .Education
require none of this lane's jobs. The job also never becomes skipped on any base, so the property
holds even if a repo were to start requiring it.

The other arming lanes, checked

Clean structurally rather than by luck — which is why none is exempted by name in the guard:

lane	why it is not affected
`arm-credential.yml`	mints and inspects a token; arms nothing
`merge-queue-steward.yml`	acts only on a `dequeued` event, which cannot occur for a PR never admitted to a queue — and a queue exists only on a branch configured to have one, i.e. a protected one
`release.yml`, `node-repo-platform-ref-bump.yml`	open pull requests (`--base main`, explicitly) without arming them

The guard

Three cases added to ArmedMergeMustTriggerMainsPushLanesGuard, all three of which fail on
current main (verified by restoring origin/main's auto-arm.yml in the worktree and re-running
— 3 failed, the 8 pre-existing cases still passed):

Failed NoWorkflowArmsAutoMergeWithoutReadingTheDefaultBranch
  These workflows arm auto-merge without checking the base branch: auto-arm.yml.

Failed TheArmLaneArmsOnlyOntoTheRepositoryDefaultBranch
  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.

Failed APullRequestOnANonDefaultBaseIsToldWhyItWasNotArmed
  auto-arm.yml decides not to arm a non-default base without emitting a ::warning::.

What is asserted is the CHAIN, not a spelling. The arming step must be governed by an if: that
consumes a step output, and the step producing that output must read default_branch from the event
payload. A condition moved into env:, or an if: that consults only the token mint, breaks the
chain and fails — which is the shape the regression would actually take. A companion case refuses a
literal main anywhere in the file's executable lines, and the directory-wide sweep makes this a
ratchet the second arming lane must also meet.

Verification

dotnet build -c Release -warnaserror on MeshWeaver.Documentation and
MeshWeaver.Documentation.Test — 0 Warning(s), 0 Error(s).
ArmedMergeMustTriggerMainsPushLanesGuard + MergeQueueStewardGuard +
WhatsNewEntryIntegrityTest + DocumentationLinkIntegrityTest + DocumentationEmbedIntegrityTest
- ArchitectureTopicMapTest: 31 passed, 0 failed.
check-workflow-timeouts.py (84 jobs, 0 violations; the arm job keeps its literal
timeout-minutes: 10), check-workflow-yaml-keys.py (34 files, 0 violations),
check-workflow-shell.py (0 live findings), check-workflow-permission-pairing.py (0 violations).

Work products conserved

Doc/Architecture/MergeQueue gains the section "Auto-merge waits on the BASE's protection — so a
stacked PR is armed to merge NOW", and there is a What's New entry (Category: Fix) — an author
who stacks a pull request can notice both the old behaviour and the new comment.

Pairs-with: none — this removes no public surface.

🤖 Generated with Claude Code

`auto-arm.yml` armed auto-merge on every non-draft, non-fork pull request, whatever its base. Auto-merge waits for the contexts branch protection requires of the BASE, and protection in this fleet is configured on the default branch and nowhere else — so a STACKED pull request, opened against another pull request's feature branch, had an EMPTY required set and a condition satisfied the instant GitHub first read 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 collapsed unreviewed, exactly as configured. This file is the fleet's only copy of the arm lane and every satellite reaches it through `workflow_call`, so the hole was open in all eight repositories at once. The lane now reads `github.event.repository.default_branch` off the event — never a literal `main`, which would be a copy of a fact every repo happens to share today — compares it with `github.event.pull_request.base.ref`, and arms only on a match. 🚨 The decision is ANNOUNCED and the JOB is never what skips. Putting the base test on 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, so "the lane declined" and "the lane is broken" become the same page. Instead the job runs, a `::warning::` plus a step summary name the base, and one idempotent comment says so on the pull request itself. Draft and fork stay on the job condition — for those there is genuinely nothing to say. Everything else is unchanged: draft is still the opt-out, forks are still skipped, the arm still runs on the minted App token (#2916), and the lane still publishes no required context (measured: core requires `Consolidate test results` only; Plugins, Reinsurance, SocialMedia and Education require none of this lane's jobs). Other arming lanes checked, clean structurally rather than by luck: `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 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. Guarded by three new cases in `ArmedMergeMustTriggerMainsPushLanesGuard`, all three of which fail on current main: NoWorkflowArmsAutoMergeWithoutReadingTheDefaultBranch These workflows arm auto-merge without checking the base branch: auto-arm.yml. TheArmLaneArmsOnlyOntoTheRepositoryDefaultBranch 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. APullRequestOnANonDefaultBaseIsToldWhyItWasNotArmed auto-arm.yml decides not to arm a non-default base without emitting a ::warning::. What is asserted is the CHAIN, not a spelling: the arming step must be governed by an `if:` consuming a step output whose producing step reads `default_branch` — a condition moved into `env:`, or one that consults only the token mint, breaks the chain and fails. The directory-wide arm becomes a ratchet the SECOND arming lane must also meet. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…s not reach it Measured 2026-09-11 over the contents API: all seven callers of auto-arm.yml exist, six pin `@main` (Plugins, Reinsurance, SocialMedia, Education, Crm, Manufacturing) and Systemorph/Memex pins @c7fef7a2 — 971 commits behind core's main. A fleet-wide fix and a six-of-seven fix are indistinguishable from core, and the odd one out is silent rather than red. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…l runs The event runs the workflow as it exists on the pull request's BASE branch, not on main. A stacked PR onto a feature branch cut before this fix still runs the old, unguarded copy and is still armed immediately — nothing in core can reach that, and it clears when the branch catches up with main. Named so the next reader does not mistake it for the guard failing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot

🟡 Changes recommended

Unresolved moderate findings remain in workflow retarget handling, comment idempotence, and regression-guard coverage.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This PR prevents stacked pull requests from being auto-merged on non-default branches, adds regression guards, and updates documentation.

Changes:

Gate auto-merge on the repository’s default branch.
Notify authors when a PR is not eligible for arming.
Add workflow regression coverage and documentation updates.

File summaries

File	Reviewed changes and findings
`test/MeshWeaver.Documentation.Test/ArmedMergeMustTriggerMainsPushLanesGuard.cs`	Adds structural guards. Moderate findings remain around exact event-field validation, base-false comment gating, and `.yaml` coverage; one nit concerns mutable collection usage.
`src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-11-a-stacked-pull-request-is-no-longer-merged-the-moment-it-is-opened.md`	Adds the user-facing fix entry. Nit: clarify that the former behavior applied to eligible default-branch PRs.
`src/MeshWeaver.Documentation/Data/Architecture/MergeQueue.md`	Documents stacked PR behavior. Nit: mention the same-repository restriction for fork PRs.
`.github/workflows/auto-arm.yml`	Adds default-branch validation and notifications. Moderate findings remain for retargeted PR handling and pipefail-sensitive comment idempotence.

Review details

Suppressed comments (7)

.github/workflows/auto-arm.yml:303

This check is not reliably idempotent under the step's set -o pipefail: when the marker is found before the end of a sufficiently large existing value, grep -q exits early and printf can receive SIGPIPE, making the pipeline false and causing a duplicate comment. Use a non-pipeline containment check (for example, [[ "$existing" == *"$MARKER"* ]]) so a found marker cannot fall through to gh pr comment.

          if printf '%s' "$existing" | grep -qF -- "$MARKER"; then

src/MeshWeaver.Documentation/Data/Architecture/MergeQueue.md:221

This sentence still overstates the lane: auto-arm.yml skips fork pull requests at the job level, so not every non-draft PR with the default base is auto-armed. Please include the same-repository restriction here so the queue documentation does not promise auto-merge to fork contributors.

and "the lane is broken" become the same page. Instead the job runs, a `::warning::` plus a step
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.

src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-11-a-stacked-pull-request-is-no-longer-merged-the-moment-it-is-opened.md:13

This paragraph uses present tense to say every non-draft pull request is armed, but the new behavior deliberately leaves non-default bases unarmed. As written, the What's New entry contradicts the fix before explaining it; make the paragraph explicitly describe the pre-fix behavior or qualify it to the default branch.

Every pull request that is not a draft is armed to merge itself as soon as its checks pass. That is
deliberate: it removes the wait between "everything is green" and "somebody pressed the button", and
it weakens nothing, because the merge still waits for exactly the checks the target branch requires.

test/MeshWeaver.Documentation.Test/ArmedMergeMustTriggerMainsPushLanesGuard.cs:724

This assertion accepts an unrelated comment step: it only checks that some block contains gh pr comment, without requiring that step to be gated by steps.base.outputs.onto-default == 'false'. A regression can remove the non-default notification and keep another PR comment, while this guard remains green. Tie the comment check to the same base-false condition within the block.

            blocks.Any(b => ExecutableLines(b).Any(l => l.Contains("gh pr comment", StringComparison.Ordinal))),

test/MeshWeaver.Documentation.Test/ArmedMergeMustTriggerMainsPushLanesGuard.cs:526

governing introduces a mutable List<string> in this new helper. The repository's collection policy requires immutable collections (the nearby guard code uses immutable builders/arrays); use an immutable-array builder or immutable accumulation here instead of adding another mutable collection.

        var governing = new List<string>();

test/MeshWeaver.Documentation.Test/ArmedMergeMustTriggerMainsPushLanesGuard.cs:675

This directory-wide check merges the governing-output IDs from every arming step, then passes when any one of those producers reads default_branch. A workflow containing one correctly guarded gh pr merge --auto step and a second unguarded arming step would therefore pass the ratchet. Evaluate each ArmsAutoMerge block independently and fail if any individual arm lacks its own governing output producer.

                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)));
            })

test/MeshWeaver.Documentation.Test/ArmedMergeMustTriggerMainsPushLanesGuard.cs:666

The ratchet says it covers the whole workflow directory, but it enumerates only *.yml; GitHub also loads *.yaml. A future arming lane added with the supported .yaml extension would be invisible to this guard and could reintroduce the exact defect. Include both workflow extensions in the sweep.

        var offenders = Directory
            .EnumerateFiles(WorkflowsDir(), "*.yml")
            .Select(path => (path, blocks: StepBlocks(File.ReadAllText(path))))

Files reviewed: 4/4 changed files
Comments generated: 2
Review effort level: Lite

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+            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."


+            .ToArray();
+
+        Assert.True(
+            producers.Any(b => ExecutableLines(b).Any(l => l.Contains("default_branch", StringComparison.Ordinal))),


github-actions · 2026-09-11T21:55:51Z