Skip to content

Disable auto-merge on maintainer push to bot PR - #79

Merged
ptr727 merged 7 commits into
developfrom
auto-merge-disable-safeguard
May 12, 2026
Merged

Disable auto-merge on maintainer push to bot PR#79
ptr727 merged 7 commits into
developfrom
auto-merge-disable-safeguard

Conversation

@ptr727

@ptr727 ptr727 commented May 12, 2026

Copy link
Copy Markdown
Owner

Implements the follow-up that PR #78 deferred: a workflow job that automatically disables auto-merge on a bot PR when a maintainer pushes commits to the bot's branch.

Problem

PR #78 acknowledged that the github.actor check on merge-codegen / merge-dependabot only stops those jobs from re-invoking gh pr merge --auto on a maintainer-triggered synchronize. It does not disable auto-merge that's already enabled from the initial bot-driven opened event. So once auto-merge is on a bot PR, any maintainer commit pushed to the bot's branch would be auto-merged when CI passed. The documented workaround ("gh pr merge --disable-auto <PR> before pushing") is fragile.

Solution

Three-job model in .github/workflows/merge-bot-pull-request.yml:

  1. merge-dependabot / merge-codegen — now restricted to opened and reopened events only. Each enables auto-merge exactly once per PR. Skipping synchronize is what keeps step 3's disable sticky (otherwise a bot-triggered rebase synchronize would re-enable auto-merge and undo the safeguard).
  2. Method dispatch unchanged — same case statement on pull_request.base.ref (develop → --squash, main → --merge).
  3. NEW: disable-auto-merge-on-maintainer-push — fires on synchronize events against bot-authored PRs when the event actor is NOT the same bot. Calls gh pr merge --disable-auto. The command is idempotent.

Token strategy

The new job uses an App token (same pattern as the other jobs) because Dependabot PRs run the workflow with restricted secrets regardless of event actor — GITHUB_TOKEN would be read-only.

Side change

Dropped github.actor == 'ptr727-codegen[bot]' from merge-codegen's if:. It was a partial safeguard against the same case that the new disable job now handles properly. The remaining checks (PR author, strict head/base pairing, opened/reopened filter) are sufficient.

Documentation

Test plan

CI on this PR can only verify YAML/workflow syntax. The real test is post-merge, manual:

  • CI passes on this PR.
  • After merge, wait for the next Dependabot scheduled bump on either branch.
  • Verify merge-dependabot enables auto-merge (visible as the "Auto-merge enabled" banner on the PR).
  • As a maintainer, push a trivial commit to the Dependabot branch.
  • Verify disable-auto-merge-on-maintainer-push runs (visible in Actions log) and the "Auto-merge enabled" banner disappears.
  • Verify the PR does NOT auto-merge when its CI passes.
  • Manually re-enable auto-merge (gh pr merge --auto <PR> or UI) and confirm it merges normally.

Same test applies to codegen PRs (next Monday's run, or workflow_dispatch on run-periodic-codegen-pull-request.yml).

PR #78 documented a limitation in the merge-bot's actor-check
guardrail: it stopped the merge jobs from re-invoking
`gh pr merge --auto` on a maintainer-triggered `synchronize`, but
did NOT disable auto-merge that was already enabled by the initial
bot-driven `opened` event. Once auto-merge was on a bot PR, any
maintainer commit pushed to the bot's branch would be auto-merged
when CI passed. The honest workaround was "remember to run
`gh pr merge --disable-auto <PR>` first" — fragile.

This PR adds the real safeguard.

Workflow changes (`.github/workflows/merge-bot-pull-request.yml`)
- New job `disable-auto-merge-on-maintainer-push`. Fires on
  `pull_request.synchronize` events against bot-authored PRs
  (`dependabot[bot]` or `ptr727-codegen[bot]`) when the event
  actor is NOT the same bot. Calls `gh pr merge --disable-auto`
  (idempotent — safe to call repeatedly). Uses an App token
  because Dependabot PRs run the workflow with restricted
  secrets regardless of who triggered the event.
- `merge-dependabot` and `merge-codegen` `if:` blocks now require
  `github.event.action == 'opened' || github.event.action ==
  'reopened'`. Skipping `synchronize` is what keeps the
  maintainer-triggered disable sticky — without this, the next
  bot-triggered `synchronize` (e.g. a Dependabot rebase) would
  re-enable auto-merge and undo the safeguard.
- `merge-codegen` `github.actor == 'ptr727-codegen[bot]'` line
  dropped: it was a partial safeguard against the same case that
  the new disable job now handles properly. The remaining checks
  (PR author, head/base pairing, opened/reopened filter) are
  sufficient.
- Header comment rewritten to describe the new three-job model
  (enable on open, disable on maintainer sync, dispatch method by
  base).

Documentation
- `AGENTS.md` Branching Model: new bullet describing the
  maintainer-push-disables-auto-merge invariant.
- `README.md` Template - GitHub Setup: codegen auto-merge
  condition list now leads with the `opened`/`reopened` event
  filter, drops the obsolete `github.actor` warning that PR #78
  had to soften, and adds a dedicated bullet for the
  `disable-auto-merge-on-maintainer-push` job.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 12, 2026 01:01

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the bot auto-merge workflow to prevent maintainer-authored commits pushed onto bot PR branches (Dependabot/codegen) from being silently auto-merged once CI passes, and documents the new invariant.

Changes:

  • Restrict merge-dependabot / merge-codegen to opened/reopened so auto-merge is enabled only once per PR.
  • Add a synchronize-driven job that disables auto-merge when a maintainer (non-bot actor) pushes to a bot-authored PR.
  • Update AGENTS.md and README.md to describe the new behavior and rationale.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
.github/workflows/merge-bot-pull-request.yml Implements the three-job model and adds the new “disable auto-merge on maintainer push” safeguard.
AGENTS.md Documents the “maintainer push disables auto-merge on bot PRs” rule in the Branching Model section.
README.md Updates the GitHub setup / workflow behavior documentation to match the new safeguard and event filtering.

Comment thread .github/workflows/merge-bot-pull-request.yml Outdated
Copilot review on PR #79 caught a race: with `cancel-in-progress:
true` and our new `opened`/`reopened` filter on the merge-enable
jobs, a fast follow-up `synchronize` (e.g. Dependabot rebase right
after PR open) would cancel the in-flight `opened` run before it
reached `gh pr merge --auto`. The new `synchronize` run skips the
enable jobs (per the new filter), so auto-merge would never be
enabled on the PR.

Fix: `cancel-in-progress: false`. Runs in the same concurrency group
queue instead of cancel, so opened completes (enables auto-merge),
then synchronize runs in arrival order (disables if maintainer,
no-ops if bot). End state is deterministic regardless of event
arrival timing.

Considered Copilot's other suggestion (include `github.event.action`
in the concurrency group): that introduces its own race — if opened
runs longer than a maintainer synchronize, opened would re-enable
auto-merge AFTER the disable job already disabled it. Single group
with cancel disabled avoids both races.

Header comment block updated to explain why the cancel setting is
load-bearing here.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Comment thread .github/workflows/merge-bot-pull-request.yml Outdated
@ptr727
ptr727 enabled auto-merge (squash) May 12, 2026 01:15
@ptr727
ptr727 requested a review from Copilot May 12, 2026 01:16
@ptr727
ptr727 disabled auto-merge May 12, 2026 01:16

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

ptr727 and others added 2 commits May 11, 2026 18:21
Copilot review on PR #79 flagged the unpinned `actions/create-github-
app-token@v1` and I initially declined, citing AGENTS.md's clause
that "first-party `actions/*` are encouraged but not required" to be
SHA-pinned. The maintainer corrected: that softening was meant
narrowly for `dotnet/nbgv@master` (where tag-tracking would propose
a downgrade), not as a blanket first-party exemption. Every other
action must be SHA-pinned.

Rule change in AGENTS.md "Workflow YAML Conventions":

- Old: third-party actions must be SHA-pinned; first-party `actions/*`
  are encouraged but not required.
- New: every action (first- or third-party) must be SHA-pinned. The
  only documented exception is `dotnet/nbgv@master`, whose rationale
  is recorded inline in get-version-task.yml.
- Also updated the meta-note from "Don't open a PR purely to apply
  these rules across the repo" to "Sweep PRs that apply a rule
  everywhere are welcome when a rule changes" — which is exactly
  what this commit does.

Sweep applied to every workflow file:

- `actions/checkout@v6` → `de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2`
- `actions/setup-dotnet@v5` → `c2fa09f4bde5ebb9d1777cf28262a3eb3db3ced7 # v5.2.0`
- `actions/create-github-app-token@v1` → `d72941d797fd3113feb6b93fd0dec494b13a2547 # v1.12.0` (4 occurrences across merge-bot + run-codegen)
- `actions/upload-artifact@v6` → `b7c566a772e6b6bfb58ed0dc250532a479d7789f # v6.0.0`
- `actions/download-artifact@v7` → `37930b1c2abaa49bbe596cd826c3c89aef350131 # v7.0.0`

Every SHA is the current target of the floating tag it replaces, so
behaviour is unchanged at the moment of the pin; only the
defence-in-depth against tag retargeting is added. Future bumps come
through Dependabot's GitHub Actions ecosystem (already configured for
both `main` and `develop` per PR #78).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous SHA-pin sweep used `sed -i` which rewrote every workflow file with LF endings. The repo's `.gitattributes` has `* -text` (no line-ending normalization), so the original files were stored as CRLF and the LF rewrite showed up as an additional ~600 line-ending churn alongside the actual SHA pins.

Convert the files back to CRLF. Once this PR squash-merges to develop, the cumulative diff is only the SHA pins (the LF detour cancels itself out).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 12, 2026 01:23

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 12 out of 13 changed files in this pull request and generated 3 comments.

Comment thread .github/workflows/merge-bot-pull-request.yml
Comment thread .github/workflows/build-docker-task.yml Outdated
Comment thread AGENTS.md
…urrency exception in AGENTS.md

Third Copilot pass on PR #79 caught two valid follow-on issues from
the SHA-pin sweep in 5daf12f:

1. The first sweep only touched `actions/*` references but missed
   the other floating tags. Pinning those now:
   - RubbaBoy/BYOB@v1 -> a4919104bc0ec7cfd7f113e42c405cc45246f2a4 # v1
     (the floating @v1 tag at this upstream doesn't match a specific
     v1.x release SHA; pinning to current floating-tag SHA so behaviour
     is unchanged and Dependabot can later suggest a versioned bump)
   - docker/setup-qemu-action@v3 -> c7c53464625b32c7a7e944ae62b3e17d2b600130 # v3.7.0
   - docker/setup-buildx-action@v3 -> 8d2750c68a42422c14e847fe6c8ac0403b4cbd6f # v3.12.0
   - docker/login-action@v3 -> c94ce9fb468520275223c153574b00df6fe4bcc9 # v3.7.0
   - docker/build-push-action@v6 -> 10e90e3645eae34f1e60eeb005ba3a3d33f178e8 # v6.19.2
   `dotnet/nbgv@master` remains the documented exception.

2. The AGENTS.md "Concurrency" convention says top-level workflows
   should use `cancel-in-progress: true`, but the safeguard PR set it
   to `false` in merge-bot-pull-request.yml — load-bearing for the
   three-job model. Add a documented exception to the convention so
   it's not a silent contradiction.

Used the Edit tool per-line this time instead of `sed -i` to avoid
the CRLF -> LF rewrite that happened in 5daf12f (and was reversed in
94e6b8a).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 13 out of 14 changed files in this pull request and generated 2 comments.

Comment thread .github/workflows/run-periodic-codegen-pull-request.yml Outdated
Comment thread .github/workflows/build-datebadge-task.yml
… tag matches

Two follow-on Copilot findings on PR #79:

1. run-periodic-codegen-pull-request.yml header comment claimed the
   workflow "checks out and targets main/codegen" — stale since PR #78
   converted the reusable workflow to a matrix over both `main` and
   `develop` (branch names codegen-main / codegen-develop). Rewrote the
   concurrency comment and renamed the group from `codegen-main` to
   plain `codegen` to match the actual behavior (one group for the whole
   workflow; a new scheduled or manual run supersedes the matrix-legs
   of an in-flight one).

2. The `RubbaBoy/BYOB@a491910... # v1` pin uses a major-only version
   comment, but the AGENTS.md action-pinning rule prescribes
   `# vX.Y.Z`. The reason for the major-only comment: the upstream's
   `@v1` floating tag at RubbaBoy/BYOB points at a SHA that doesn't
   correspond to any specific v1.x tag (v1.3.0 is the latest specific
   release and has a different SHA). Pinning to v1.3.0 would be a
   behavior change away from what the floating tag currently delivers.
   Updating AGENTS.md to allow `# vX` (major-only) in this narrow case
   — the SHA pin still gives full protection, only the version comment
   loses specificity.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.

Comment thread .github/workflows/run-periodic-codegen-pull-request.yml Outdated
Copilot review on PR #79 caught that my prior rename ('codegen-main'
to 'codegen') made the concurrency group a constant — which deviates
from AGENTS.md's documented convention of
`${{ github.workflow }}-${{ github.ref }}`. The constant was a
pre-existing pattern that my change inherited rather than fixed.

Switching to the AGENTS.md convention. In practice the behavior is
identical for this workflow: scheduled cron runs always have
`github.ref == refs/heads/<default-branch>`, so the group is
effectively constant across all scheduled runs anyway. The reusable
workflow it calls then matrixes over both `main` and `develop`
internally, so one group per workflow+ref still serializes all matrix
legs.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated no new comments.

@ptr727
ptr727 enabled auto-merge (squash) May 12, 2026 01:48
@ptr727
ptr727 merged commit bb66475 into develop May 12, 2026
26 checks passed
@ptr727
ptr727 deleted the auto-merge-disable-safeguard branch May 12, 2026 01:55
ptr727 added a commit that referenced this pull request May 12, 2026
…forward-only) (#81)

Resolves the root cause behind why [PR
#80](#80) (the develop →
main release) is blocked by "the head branch is not up to date with the
base branch". The forward-only develop model PR #78 codified is
fundamentally incompatible with the ruleset's `Require branches to be up
to date before merging` rule, and the README's documented "shared
settings" block hid the contradiction.

## What's actually happening

The "up to date" check is **graph-based**: it asks "is main's tip commit
reachable from develop?", not "does develop have main's content?". After
any develop → main release, main has a new merge commit (e.g. `fb10a16`
from PR #77) whose first-parent walk isn't in develop's history. Develop
is strictly *ahead* in content but "behind" in graph terms.

Historical back-merge commits (`ffb9e64`, `5ce95cf`) had been **quietly
compensating** for this — each one created a develop commit whose second
parent was main's release-merge commit, making main's tip reachable from
develop. PR #78 codified forward-only and forbade back-merges, but the
README rulesets section still listed `Require branches to be up to date`
as a shared setting. The contradiction was invisible until the first
release without a preceding back-merge tried to land — which is PR #80.

## What this PR changes

- **`README.md`** "Rules / Rulesets": move `Require branches to be up to
date before merging` out of "Shared settings" and into the Develop-only
ruleset entry (where it's standard hygiene for feature → develop
merges). Add explicit "intentionally OFF" callout in the Main ruleset
entry with the full rationale.
- **`AGENTS.md`** "Branching Model": new bullet codifying *why* the main
ruleset omits this rule, with a pointer to README for the configured
state.

## What you'll need to do in the GitHub UI

Untick `Require branches to be up to date before merging` in **Settings
→ Rulesets → Main**. That's a config change, not a code change, and
rulesets are security-sensitive so it stays your direct action. After:

- PR #80 will merge cleanly via `gh pr merge --merge` (no admin bypass
needed).
- Future develop → main releases land without admin bypass.

## Test plan

- [ ] CI passes on this PR.
- [ ] After merge to develop, PR #80's head auto-advances to include
this docs update.
- [ ] After you untick the rule on Settings → Rulesets → Main, PR #80
merges cleanly without `--admin`.
- [ ] Future develop → main releases also land without admin bypass (the
safeguard PR #79's content joins this release on main once PR #80
lands).

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ptr727 added a commit that referenced this pull request May 12, 2026
…rules-docs alignment (#80)

Release merge: brings two squashed PRs from `develop` into `main`.

## Squashed PRs included

- **#79 — Disable auto-merge on maintainer push to bot PR.** Started
narrow (a `synchronize`-triggered job that calls `gh pr merge
--disable-auto` when a maintainer pushes to a bot PR, closing the gap PR
#78 documented but didn't fix) and grew to cover repo-wide SHA pinning
of every action after the maintainer corrected my reading of AGENTS.md's
first-party-actions clause.

  ### What landed
- **New `disable-auto-merge-on-maintainer-push` job** in
`.github/workflows/merge-bot-pull-request.yml`. Fires on
`pull_request.synchronize` events against bot-authored PRs (Dependabot
or codegen) when the event actor isn't the same bot — calls `gh pr merge
--disable-auto`. App-token-driven (Dependabot PRs run with restricted
secrets regardless of event actor).
- **`merge-dependabot` and `merge-codegen` restricted to
`opened`/`reopened`** so auto-merge is enabled exactly once per PR;
skipping `synchronize` is what keeps the disable safeguard sticky
against bot rebases.
- **`concurrency.cancel-in-progress: false`** in
`merge-bot-pull-request.yml` so the three-job model runs events to
completion in arrival order.
- **Every action SHA-pinned** across all workflows: `actions/*`
(checkout, setup-dotnet, create-github-app-token, upload-artifact,
download-artifact), `docker/*` (setup-qemu-action, setup-buildx-action,
login-action, build-push-action), and `RubbaBoy/BYOB`.
`dotnet/nbgv@master` is the only documented exception.
- **AGENTS.md "Workflow YAML Conventions"** tightened: every action must
be SHA-pinned (the prior "first-party `actions/*` encouraged but not
required" softening is gone). `# vX` major-only comment allowed when
upstream's floating major tag doesn't correspond to a specific
patch/minor release SHA. Concurrency convention gains a documented
exception for `merge-bot-pull-request.yml`.
- **AGENTS.md "Branching Model"** + **README "Template - GitHub Setup"**
updated for the new disable job and the auto-merge condition list.

- **#81 — Drop "branches up to date" rule from main ruleset
(incompatible with forward-only).** Resolved the root cause behind PR
#80 being initially blocked. GitHub's "Require branches to be up to date
before merging" is a graph-based check (it asks whether main's tip merge
commit is reachable from develop) that's fundamentally incompatible with
the forward-only develop model PR #78 codified. Historical back-merges
had been quietly compensating for this; PR #78 forbade them but left the
README's documented "shared settings" ruleset block contradictorily
listing the rule.
- **README "Rules / Rulesets"**: moved `Require branches to be up to
date before merging` out of "Shared settings" into the Develop-only
ruleset entry (where it's standard hygiene). Added explicit
"intentionally OFF" callout in the Main ruleset entry with the full
rationale.
- **AGENTS.md "Branching Model"**: new bullet codifying *why* the main
ruleset omits this rule, framed purely in graph-reachability terms.

## Operator action already completed

- `Require branches to be up to date before merging` unticked on
Settings → Rulesets → Main. ✓ Verified via API.

## Notes

- Merge method: **merge-commit** (per [AGENTS.md branching
model](https://github.com/ptr727/ProjectTemplate/blob/develop/AGENTS.md#branching-model)).
- **No rebase required.** With the ruleset rule now disabled, GitHub no
longer enforces graph-reachability of main's tip from develop, so the
merge proceeds cleanly without admin bypass or back-merge.

## Test plan

- [ ] CI passes on the merge commit.
- [ ] `publish-release.yml` on main produces the next stable release.
- [ ] Next bot PR (Dependabot or codegen) opens with auto-merge enabled
exactly once. A maintainer push to the bot's branch disables auto-merge;
re-enable manually to land the maintainer's edits.
- [ ] No floating-tag actions remain anywhere in `.github/workflows/`
except `dotnet/nbgv@master`.
- [ ] Future develop → main releases land without admin bypass.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants