Skip to content

feat(automerge): refuse auto-merge on non-default bases, with a selftest - #135

Merged
topcoder1 merged 2 commits into
mainfrom
fix/automerge-nonmain-base-gate
Jul 28, 2026
Merged

feat(automerge): refuse auto-merge on non-default bases, with a selftest#135
topcoder1 merged 2 commits into
mainfrom
fix/automerge-nonmain-base-gate

Conversation

@topcoder1

Copy link
Copy Markdown
Owner

TL;DR

Auto-merge may now only target the ref a branch ruleset actually protects. Non-default bases refuse by default and need an explicit per-PR opt-in label; a base that is another open PR's head refuses unconditionally. Ships with selftest/test_automerge_base_gate.sh (15 behavioural cases + 8 structural pins), verified non-vacuous against origin/main.

Fleet impact: none for the overwhelmingly common path. A PR targeting the default branch behaves exactly as before and the gate costs zero API calls.

Why this is a gate, not a routing preference

whois-api-llc/wxa-jake-ai#1027 (2026-07-28): a PR opened against another open PR's head branch squash-merged 68 seconds after creation, growing that PR's diff from 6 files to 8 after its 4-round codex review had completed. The human then clicks merge on a diff nobody reviewed.

Branch rulesets are conventionally scoped to the default branch (ref_name: ~DEFAULT_BRANCH), so a feature-branch base has no required status checks and gh pr merge --auto merges on the spot rather than waiting.

That detail is the whole finding. Every other protection in this lane is latency-shaped — the caller-side classification gate, this workflow's own hold read — and silently assumes there is a wait to lose. In #1027 the disarm that would have caught it began 7 seconds after the merge had already landed. Remove the wait and an unguarded arm always wins, which is why "the hold label protects main-targeted PRs" was true only by accident of timing.

Policy

Base Behaviour
== default branch unchanged — and costs no API calls
is another open PR's head refused, not liftable by the opt-in label
any other non-default base refused unless nonmain_base_optin_label (default auto-merge-nonmain) is present

Opting into a plain integration branch is a routing choice. Opting into rewriting an under-review PR's diff is not one a label should be able to make — hence the asymmetry.

Fail-closed throughout: an unreadable PR listing or label set is an unknown base state and must never read as "no refusal". A refusal revokes an existing arm, and the always() error-revoke now counts a failed base read (a fail-closed read must also disarm a PR armed by an earlier run).

Arm-time re-binding. --match-head-commit binds the arm to the head SHA, but the base, the opt-in label, and whether the base is an open PR's head all mutate without moving that SHA — and opening the parent PR fires no event for the child at all. So the arm re-reads all three immediately before firing, and every pre-arm rejection disarms rather than merely declining to re-arm.

Selftest

selftest/test_automerge_base_gate.sh follows test_automerge_hold_gate.sh conventions exactly: extract the shipped bash from the YAML, execute it against a stubbed gh, assert behaviour and fail-closed contracts. Registered in test_workflow_guards.py so tests-runner.yml enforces it, and documented in selftest/README.md.

15 behavioural cases, including the #1027 replay, the opt-in that must not lift the open-head refusal, exact-match/substring semantics, multi-page listings, and four fail-closed paths. 8 structural pins cover interpolation-freedom, pagination, non-aggregating jq, the same-repo head filter, arm-time re-binding, and "no pre-arm rejection exits without disarming".

Non-vacuity verified: every structural pin fails when run against origin/main's workflow. uv run pytest -q → 46 passed. actionlint clean.

Codex pre-review

Codex rounds: 6. Findings 1–5 fixed; round 6 declined with reasoning:

Round Finding Outcome
1 P1 verdict not bound to mutable base/label at the arm fixed
2 P1 base can become an open PR's head; opening the parent fires no event for the child fixed (re-list at arm time)
3 P1 pre-arm rejection exits without disarming an existing arm fixed (disarm_then_exit)
4/5 P2 a stale refusal can revoke a newer run's valid arm fixed (live-base ownership guard)
6 P1 arm still not atomic with its inputs declined — no GitHub primitive provides this; the window is now sub-second, down from minutes
6 P2 error-revoke needs the same ownership guard declined — a failed base gate means the base is unknown, so no ownership claim can be made; that path's documented contract is fail-toward-safety

One deliberate asymmetry is documented inline because it reads like an inconsistency: the pre-arm disarm stays unconditional on a base change (the run holds a live read proving its arm was validated against a base that is no longer current, and a retarget re-gates nothing), while the refusal-revoke skips on a base change (there the verdict is the stale artifact). Also documented inline: why the open-head check is scoped to non-default bases — in a main → release train the default branch is itself an open PR's head, and checking it there would disable auto-merge fleet-wide for the life of that release PR.

Follow-up (not in this PR)

Callers subscribe to [opened, synchronize, reopened, ready_for_review, labeled]. A base retarget emits edited, so a retarget currently fires no run anywhere. Adding edited to caller trigger types is the structural fix for that class; it changes run volume across 44+ callers and belongs in its own change rather than bundled into a security fix.

Auto-merge rationale

Manual merge. PRs to topcoder1/ci-workflows are always manual-merge per fleet policy, and this changes the auto-merge decision path for every @main caller. Opened as a draft with manual-merge. Default behaviour for default-branch PRs is unchanged; the only behaviour change is that non-default bases now refuse without an opt-in — deliberately failing closed.

🤖 Generated with Claude Code

whois-api-llc/wxa-jake-ai#1027 (2026-07-28): a PR opened against another
OPEN PR's head branch squash-merged 68 seconds after creation, growing that
PR's diff from 6 files to 8 AFTER its 4-round codex review completed. The
human is then asked to click merge on a diff nobody reviewed.

Branch rulesets are conventionally scoped to the default branch
(`ref_name: ~DEFAULT_BRANCH`), so a feature-branch base has NO required
status checks and `gh pr merge --auto` merges on the spot. That is what
makes this a gate rather than a routing preference: every other protection
in this lane is latency-shaped — the caller-side classification gate, this
workflow's own hold read — and silently assumes there is a wait to lose.
Remove the wait and an unguarded arm always wins.

Policy:

  * base == default branch          -> unchanged, and costs no API calls
  * base is another open PR's head  -> refused, NOT liftable by the opt-in
  * base != default branch          -> refused without the new
                                       `nonmain_base_optin_label`
                                       (default `auto-merge-nonmain`)

Fail-closed throughout: an unreadable PR listing or label set is an unknown
base state and must never read as "no refusal". A refusal also revokes an
existing arm, and the always() error-revoke now counts a failed base read.
The arm re-binds to the base (and, on a non-default base, re-lists open
heads and re-checks the opt-in) immediately before arming, because all
three mutate WITHOUT moving the head SHA that `--match-head-commit` binds
— and opening the parent PR fires no event for the child at all.

selftest/test_automerge_base_gate.sh extracts the shipped bash and runs it
against a stubbed `gh`, following test_automerge_hold_gate.sh conventions:
15 behavioural cases plus structural pins (pagination, non-aggregating jq,
same-repo head filter, and no pre-arm rejection that exits without
disarming). Verified non-vacuous — every structural pin fails against
origin/main.

Auto-Merge-Risk: high
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@topcoder1 topcoder1 added the manual-merge Durable hold: never auto-merge; a human must click. Outranks all automerge bypasses. label Jul 28, 2026
@github-actions

Copy link
Copy Markdown

Coverage Floor — mode: enforce

metric value
measured 100.0%
floor (current) 99.0%
target 100.0%
last bumped 2026-05-12

`printf '%s\n' "$x" | grep -qxF "$y"` under `set -o pipefail` can report
non-zero even on a MATCH: grep -q exits the moment it matches, and the
writer can take SIGPIPE, which pipefail then surfaces as pipeline failure.
In the base gate that direction fails OPEN — an opt-in label reading as
absent merely over-refuses, but the same shape guards the open-PR-head
check, where a missed match is an unreviewed merge.

Inert at today's input sizes (label and head lists are far smaller than
the 64KB pipe buffer, so the writer never blocks and never signals — the
selftest passes either way), but the safety margin is "the list stayed
small", which is not a property worth depending on. A here-string has no
pipeline and therefore no pipefail interaction at all.

Scoped to the code this branch introduces. The pre-existing hold step
keeps its `echo | grep` form: it is proven, test_automerge_hold_gate.sh
pins that exact shipped bash, and changing it is a drive-by.

Auto-Merge-Risk: high
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@topcoder1
topcoder1 marked this pull request as ready for review July 28, 2026 17:34
@topcoder1
topcoder1 merged commit a3ccafa into main Jul 28, 2026
15 checks passed
@cursor

cursor Bot commented Jul 28, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@topcoder1
topcoder1 deleted the fix/automerge-nonmain-base-gate branch July 28, 2026 17:34
@github-actions github-actions Bot added the risk:blocked Risk class: blocked label Jul 28, 2026
@github-actions

Copy link
Copy Markdown

Risk class: blocked — manual merge required.

This PR touches one of the blocked path categories from .github/risk-paths.yml (Dockerfiles, docker-compose, .github/workflows/**, **/.env*, **/secrets*, infra/, terraform/, k8s/, or the classifier config itself).

Auto-merge is refused by claude-author-automerge.yml. A maintainer should review the diff and click "Squash and merge" themselves.

(This is a policy notice, not a code-quality failure. The classify job itself does not fail — required CI checks remain authoritative for "is the code green.")

@claude

claude Bot commented Jul 28, 2026

Copy link
Copy Markdown

No issues found. Base-gate logic is fail-closed and consistent (null handling, non-aggregating pagination, exact-match labels, arm re-binding); selftest covers the new branches. Behavior change to refuse non-default bases is the deliberate, documented intent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

manual-merge Durable hold: never auto-merge; a human must click. Outranks all automerge bypasses. risk:blocked Risk class: blocked

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant