Conversation
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`](.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 - [`AGENTS.md`](AGENTS.md) Branching Model — new bullet documenting the maintainer-push-disables-auto-merge invariant. - [`README.md`](README.md) Template - GitHub Setup — codegen auto-merge condition list now leads with the `opened`/`reopened` filter, drops the obsolete `github.actor` warning that PR #78 had to soften, and adds a dedicated bullet for the new disable job. ## 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`). --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Updates the repo’s GitHub Actions automation and documentation to (1) prevent accidental auto-merges when maintainers push commits onto bot-authored PR branches, and (2) enforce SHA-pinning for GitHub Actions usage in line with the tightened AGENTS.md rules.
Changes:
- Add a
pull_request.synchronizesafeguard in the merge-bot workflow to disable auto-merge when a non-bot actor pushes to a bot PR branch, and restrict auto-merge enabling toopened/reopenedonly. - SHA-pin GitHub Actions usages across workflows (with the existing documented
dotnet/nbgv@masterexception). - Update AGENTS.md and README.md to document the new invariant, exceptions, and concurrency rationale.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Documents the updated codegen auto-merge conditions and the new “disable auto-merge on maintainer push” safeguard. |
| AGENTS.md | Tightens workflow conventions (pin every action; concurrency exception) and documents the new maintainer-push auto-merge disable invariant. |
| .github/workflows/merge-bot-pull-request.yml | Implements the three-job model (enable once on open/reopen; disable on maintainer synchronize) and sets cancel-in-progress: false for deterministic ordering. |
| .github/workflows/test-release-task.yml | SHA-pins first-party actions used during PR validation. |
| .github/workflows/run-periodic-codegen-pull-request.yml | Aligns concurrency grouping with the documented convention for scheduled/dispatch runs. |
| .github/workflows/run-codegen-pull-request-task.yml | SHA-pins actions used for App-token creation, checkout, setup-dotnet, and PR creation. |
| .github/workflows/publish-release.yml | SHA-pins actions/download-artifact in the PyPI publish job. |
| .github/workflows/get-version-task.yml | SHA-pins setup-dotnet/checkout while retaining the documented dotnet/nbgv@master exception. |
| .github/workflows/build-release-task.yml | SHA-pins checkout and artifact download actions in the GitHub release publishing job. |
| .github/workflows/build-pypilibrary-task.yml | SHA-pins checkout and upload-artifact while preserving existing Python build/test steps. |
| .github/workflows/build-nugetlibrary-task.yml | SHA-pins setup-dotnet/checkout/upload-artifact in the NuGet build workflow. |
| .github/workflows/build-executable-task.yml | SHA-pins setup-dotnet/checkout/artifact upload/download in the executable build pipeline. |
| .github/workflows/build-docker-task.yml | SHA-pins docker build toolchain actions (qemu/buildx/login/build-push) and checkout. |
| .github/workflows/build-datebadge-task.yml | SHA-pins the BYOB action with an allowed major-only version comment. |
4 tasks
…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>
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Release merge: brings two squashed PRs from
developintomain.Squashed PRs included
Disable auto-merge on maintainer push to bot PR #79 — Disable auto-merge on maintainer push to bot PR. Started narrow (a
synchronize-triggered job that callsgh pr merge --disable-autowhen a maintainer pushes to a bot PR, closing the gap PR Forward-only develop with dual-target Dependabot + codegen; push-vs-pull docs #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
disable-auto-merge-on-maintainer-pushjob in.github/workflows/merge-bot-pull-request.yml. Fires onpull_request.synchronizeevents against bot-authored PRs (Dependabot or codegen) when the event actor isn't the same bot — callsgh pr merge --disable-auto. App-token-driven (Dependabot PRs run with restricted secrets regardless of event actor).merge-dependabotandmerge-codegenrestricted toopened/reopenedso auto-merge is enabled exactly once per PR; skippingsynchronizeis what keeps the disable safeguard sticky against bot rebases.concurrency.cancel-in-progress: falseinmerge-bot-pull-request.ymlso the three-job model runs events to completion in arrival order.actions/*(checkout, setup-dotnet, create-github-app-token, upload-artifact, download-artifact),docker/*(setup-qemu-action, setup-buildx-action, login-action, build-push-action), andRubbaBoy/BYOB.dotnet/nbgv@masteris the only documented exception.actions/*encouraged but not required" softening is gone).# vXmajor-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 formerge-bot-pull-request.yml.Drop "branches up to date" rule from main ruleset (incompatible with forward-only) #81 — Drop "branches up to date" rule from main ruleset (incompatible with forward-only). Resolved the root cause behind PR Auto-merge-disable safeguard, repo-wide SHA pins, AGENTS.md tighten, rules-docs alignment #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 Forward-only develop with dual-target Dependabot + codegen; push-vs-pull docs #78 codified. Historical back-merges had been quietly compensating for this; PR Forward-only develop with dual-target Dependabot + codegen; push-vs-pull docs #78 forbade them but left the README's documented "shared settings" ruleset block contradictorily listing the rule.
Require branches to be up to date before mergingout 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.Operator action already completed
Require branches to be up to date before mergingunticked on Settings → Rulesets → Main. ✓ Verified via API.Notes
Test plan
publish-release.ymlon main produces the next stable release..github/workflows/exceptdotnet/nbgv@master.