Conversation
…PI via NBGV (#70) ## Summary A consolidation PR. Three threads land together because they overlap (all touch the merge-bot + codegen + PyPI release flow): 1. **Standardize all bot workflows on the GitHub App token.** Drop the dual codegen path (PAT + App) — App was already the better one. Drop the merge-bot's GITHUB_TOKEN-based codegen merge path. Result: every bot-authored push/PR fires downstream `pull_request` / `push` events directly (no recursion-guard skips), so `publish-release.yml` fires on bot-driven merges to develop and main exactly the same way it does on human merges. 2. **Align merge method per base branch.** Mirrors the homeassistant-purpleair pattern: develop ruleset allows only squash, main ruleset allows only merge commits. The merge-bot dispatches via a `case` on `pull_request.base.ref` so the form matches either base. 3. **Route Dependabot to develop and version the PyPI library via NBGV.** Closes "develop falls behind main" because scheduled bumps land on develop first. Picks the long-deferred PyPI versioning question by reading NBGV's `AssemblyFileVersion` and overwriting `_version.py` in CI before `uv build`, so PyPI and NuGet ship at matching versions per release. ## Standardize on GitHub App token ### Codegen - **Delete** `.github/workflows/run-codegen-app-pull-request-task.yml` and `.github/workflows/run-periodic-codegen-app-pull-request.yml`. - **Rewrite** `.github/workflows/run-codegen-pull-request-task.yml` with the App-token-based logic (formerly in the deleted *app* variant). No PAT, no close/reopen dance — App tokens trigger `pull_request` workflow events directly when opening a PR. - **Update** `.github/workflows/run-periodic-codegen-pull-request.yml`: drop the Monday/Thursday alternation note; the weekly Monday cron + `workflow_dispatch` remain. ### Merge bot - **Delete** `merge-codegen-app` job (the App-token-based duplicate of `merge-codegen`). - **Rewrite** the remaining `merge-codegen` to use the App token, match `ptr727-codegen[bot]`, and skip the legacy `(reopened & owner) || (!reopened & github-actions[bot])` actor dance that existed only because `GITHUB_TOKEN`-created PRs needed a close/reopen nudge. - **`merge-dependabot`** — switched to App token (was already on the chopping block for the publish-release trigger problem). Replaces unconditional `--squash` with a `case` on `pull_request.base.ref` that picks `--squash` for develop and `--merge` for main; explicit failure for unknown bases. - Header comment rewritten to reflect the new single-strategy model (App token everywhere; matched merge method per base). ## Align merge method per base branch Bot PRs targeting `main` previously used `gh pr merge --auto --squash` — fine while the main ruleset allowed squash, but fragile once main locks to merge-only (the purpleair experience). The dispatcher in `merge-bot-pull-request.yml` now lifts the form from `base.ref`: ```yaml case "${{ github.event.pull_request.base.ref }}" in develop) method=--squash ;; main) method=--merge ;; *) echo "::error::Unsupported base branch: ${{ github.event.pull_request.base.ref }}"; exit 1 ;; esac gh pr merge --auto "$method" "$PR_URL" ``` ## Route Dependabot to develop - **`.github/dependabot.yml`**: `target-branch: "main"` → `"develop"` on all three ecosystem entries (`nuget`, `github-actions`, `uv`). Scheduled bumps now land on develop first, then bundle into the next develop → main merge-commit alongside feature work. **Security update PRs still open against `main` directly** — Dependabot doesn't honor `target-branch` for those — but the new `case` statement in the merge-bot handles either base. Long comment in the file explains the trade-off and links the merge-bot. ## NBGV-driven PyPI version - **`.github/workflows/build-pypilibrary-task.yml`** — add an inner `get-version` job (mirroring `build-nugetlibrary-task.yml`) and a new "Write version into _version.py step" that overwrites `_version.py` with NBGV's `AssemblyFileVersion` (`Major.Minor.Patch.BuildNumber` — always numeric, PEP 440 valid) just before `uv build`. The wheel + sdist therefore carry the rewritten version, so the PyPI upload matches the NuGet, Docker, and executable artifacts for the same release commit. - **`PyPiLibrary/src/ptr727_projecttemplate_library/_version.py`** — docstring expanded to explain that `0.0.0` is a local-development convenience and CI overwrites the file before `uv build`. - **`PyPiLibrary/README.md`** — stack table lists Version alongside the rest of the tooling; "Template Adoption" section now frames NBGV as the default with hatch-vcs / manual as the documented forks. ## Documentation - **`AGENTS.md` → Branching Model** — new bullet noting Dependabot's `target-branch: develop` routing and the merge-bot's base-aware method dispatch. - **`.github/copilot-instructions.md` → Commit Messages and Pull Request Titles** — short paragraph telling AI agents to pick `--squash` for develop and `--merge` for main when invoking `gh pr merge`. - **`PyPiLibrary/README.md` → Publishing** — expanded the first-time PyPI Trusted Publishing setup with everything we learned during today's release session (the 2FA prereq, pending-publisher vs add-new-publisher distinction, the mandatory `pypi` environment deployment branch rule on `main`, optional required reviewer, troubleshooting for `invalid-publisher` and `manifest unknown`, and an API-token fallback recipe). - **`README.md` → Template - GitHub Setup** — drop the WORKFLOW_PAT block, make App setup required (was an alternative), collapse the dual codegen schedule into one entry. ## Test plan - [ ] CI passes on the PR. - [ ] After merge, the next Dependabot scheduled run opens its PRs against `develop` (currently opens against `main`). - [ ] When that Dependabot PR auto-merges, the resulting develop push fires `publish-release.yml`. - [ ] Next codegen PR auto-merges to `main` as a merge-commit (not a squash). - [ ] Next release on `main` publishes the PyPI library at the same `M.N.P.B` version as the NuGet package (no more `0.0.0` placeholder). --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This release-merge consolidates automation around a single GitHub App identity, aligns auto-merge behavior with branch protection rules, and makes the PyPiLibrary version match Nerdbank.GitVersioning outputs used elsewhere in the repo.
Changes:
- Standardize codegen + merge-bot workflows on a GitHub App token (remove PAT/close-reopen and delete the redundant “app” workflow variants).
- Make merge-bot choose
--squashvs--mergebased on the PR base branch (develop vs main) and route Dependabot scheduled updates todevelop. - Version PyPiLibrary builds via NBGV by rewriting
_version.pyin CI beforeuv build, and gate PyPI publish tomainonly.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| README.md | Removes PAT-based setup and documents GitHub App as the single required path for bots. |
| PyPiLibrary/src/ptr727_projecttemplate_library/_version.py | Expands docstring to document CI rewriting __version__ for NBGV-driven publishing. |
| PyPiLibrary/README.md | Documents NBGV-based Python versioning and expands Trusted Publishing setup/troubleshooting guidance. |
| AGENTS.md | Updates branching model notes to include Dependabot→develop routing and base-aware merge-bot behavior. |
| .github/workflows/run-periodic-codegen-pull-request.yml | Keeps the weekly codegen schedule while reflecting the single App-token strategy. |
| .github/workflows/run-periodic-codegen-app-pull-request.yml | Deleted (consolidated into the single periodic codegen workflow). |
| .github/workflows/run-codegen-pull-request-task.yml | Switches codegen PR creation to GitHub App token and pins create-pull-request action SHA. |
| .github/workflows/run-codegen-app-pull-request-task.yml | Deleted (logic merged into the single codegen task workflow). |
| .github/workflows/publish-release.yml | Skips the PyPI publish job on non-main pushes to avoid environment-gated stalls. |
| .github/workflows/merge-bot-pull-request.yml | Uses GitHub App token for merges and selects merge method based on PR base branch; consolidates codegen merge jobs. |
| .github/workflows/build-pypilibrary-task.yml | Adds a version job and rewrites _version.py from NBGV AssemblyFileVersion prior to building artifacts. |
| .github/dependabot.yml | Routes scheduled updates to develop for all ecosystems and documents the security-update caveat. |
| .github/copilot-instructions.md | Documents base-branch-aware gh pr merge flag selection for automated merges. |
2 tasks
## Summary One-character US-English typo fix in [.github/workflows/merge-bot-pull-request.yml](.github/workflows/merge-bot-pull-request.yml) (the new dependabot-merge step comment landed in PR #70): ```diff - # runtime behaviour, so they should land via human review. Other + # runtime behavior, so they should land via human review. Other ``` Flagged by Copilot review on [PR #71](#71) (the develop → main release). AGENTS.md "Documentation Style Conventions" requires US English. ## Test plan - [ ] CI passes on this PR. - [ ] After merge to develop, PR #71's diff absorbs the fix and the Copilot thread on #71 can be resolved. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ptr727
added a commit
that referenced
this pull request
May 11, 2026
Two additional Copilot findings on PR #71 that fit naturally with the spelling fix already on this branch: 1. build-pypilibrary-task.yml: the `get-version` job called `get-version-task.yml` with `secrets: inherit`, but the task workflow does not declare any required secrets. Drop the inherit so the get-version job runs without secrets in scope — smaller blast radius. (build-nugetlibrary-task.yml has the same pattern; that file is unmodified in this branch so leaving its inherit alone per the AGENTS.md opportunistic-pin scope rule.) 2. merge-bot-pull-request.yml `merge-codegen`: the `if:` gate checked PR author/branch/base but not the event actor. A maintainer pushing extra commits to the App's `codegen` branch would fire a `synchronize` event that this job would happily auto-merge — folding human changes into a release via the App PR. Restore the `github.actor == 'ptr727-codegen[bot]'` check (which the pre-consolidation `merge-codegen-app` job carried) alongside the existing PR author check. Comment expanded to explain why both checks matter. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2 tasks
…#73) ## Summary Two additional Copilot findings on [PR #71](#71) (develop → main release) that PR #72 was supposed to carry alongside the `behaviour → behavior` fix. Auto-merge on PR #72 fired before the second commit on its branch landed, so PR #72's squash captured only the spelling fix — these two land here separately. ### 1. `secrets: inherit` removed from `get-version` job in `build-pypilibrary-task.yml` The job calls [`get-version-task.yml`](.github/workflows/get-version-task.yml) which declares no required secrets. `secrets: inherit` was widening the secret blast radius for no benefit. (Same pattern exists in `build-nugetlibrary-task.yml`, untouched here per AGENTS.md "Workflow YAML Conventions" — *"existing workflows are migrated opportunistically when they're being touched for other reasons"*. Easy follow-up PR later.) ### 2. `merge-codegen` `if:` gate now requires App-actor too The current gate checks PR author/branch/base but not the event actor. A maintainer pushing extra commits to the App's `codegen` branch fires a `synchronize` event the job would happily auto-merge — folding human changes into a release through the App PR. Restored `github.actor == 'ptr727-codegen[bot]'` (which the pre-consolidation `merge-codegen-app` job carried in PR #70-era code) alongside the existing PR author check. Comment expanded to explain why both checks matter. ```diff + # Both the PR author AND the event actor must be the App: the author + # check stops human-opened PRs that happen to target the `codegen` + # branch from auto-merging; the actor check stops a maintainer + # pushing extra commits to the App's `codegen` branch (a + # `synchronize` event the human triggered) from auto-merging + # unintended changes through the App PR. if: >- github.event.pull_request.user.login == 'ptr727-codegen[bot]' && + github.actor == 'ptr727-codegen[bot]' && github.event.pull_request.head.ref == 'codegen' && github.event.pull_request.base.ref == 'main' && github.event.pull_request.head.repo.full_name == github.repository ``` ## Test plan - [ ] CI passes on this PR. - [ ] After merge to develop, PR #71's two remaining Copilot threads (lines 25 and 93) can be resolved. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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 free
to 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 one squashed PR from develop into main.
Squashed PR included
run-codegen-app-pull-request-task.ymlandrun-periodic-codegen-app-pull-request.yml. Rewroterun-codegen-pull-request-task.ymlwith App-token logic — no PAT, no close/reopen dance. Consolidatedmerge-codegen-appintomerge-codegenin the merge-bot; both jobs inmerge-bot-pull-request.ymlnow use the App token so the resulting push fires downstream workflows directly.caseonpull_request.base.refpicks--squashfor develop and--mergefor main, with an explicit failure for unknown bases. Bot PRs now match each ruleset.target-branch: "develop"so develop is the leading edge for dep bumps. Security update PRs still open againstmain; the merge-bot'scasehandles either base.get-versionjob added tobuild-pypilibrary-task.yml; a new "Write version into _version.py step" usessed -ito replace the__version__line with NBGV'sAssemblyFileVersion(PEP 440 valid,Major.Minor.Patch.BuildNumber) beforeuv build. PyPI's version string therefore equals the .NET assemblies'FileVersionstamp (a separate NBGV output fromAssemblyVersionandSemVer2).publish-pypigated tomainonly. Addedif: github.ref == 'refs/heads/main'so develop pushes don't stall at the env gate (defense in depth alongside thepypienvironment's deployment branch rule).dependabot/fetch-metadata@v2→21025c70…# v2.5.0andpeter-evans/create-pull-request@v8→5f6978fa…# v8.1.1.dotnet/nbgv@masteringet-version-task.ymlwas declined (out of scope per AGENTS.md opportunistic-pin rule).--squashvs--mergebased on base; PyPiLibrary/README.md "Publishing" section expanded with the lessons from the prior release (2FA prereq, pending-publisher vs add-new-publisher distinction, the mandatorypypienv deployment branch rule onmain, troubleshooting forinvalid-publisherandmanifest unknown, API-token fallback recipe); README.md "Template - GitHub Setup" drops the WORKFLOW_PAT block and makes the App setup the single required path.Operator action items already completed (out-of-band)
CODEGEN_APP_IDandCODEGEN_APP_PRIVATE_KEYadded to the Dependabot secret store (was already in Actions). Required because Dependabot-triggered runs don't see Actions secrets.WORKFLOW_PATsecret revoked from the repo and the underlying PAT revoked on github.com.pypiGitHub environment has a Deployment branch rule restricting tomain(added during the prior release session).Notes
Test plan
mainas a merge-commit (not a squash) under the App identity.develop; the auto-merge succeeds and the resulting develop push firespublish-release.yml.mainships PyPI at the sameMajor.Minor.Patch.BuildNumberas the .NETFileVersion(not0.0.0), andpublish-pypiis cleanly skipped on develop pushes.