Address Copilot review on PR #71: spelling + secrets + actor gate - #72
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a US-English spelling inconsistency in the merge-bot workflow comment (“behaviour” → “behavior”) to align with the repo’s documentation style conventions and avoid recurring review noise in future release/merge diffs.
Changes:
- Update a single comment line in the merge-bot workflow to use US-English spelling.
ptr727
enabled auto-merge (squash)
May 11, 2026 20:25
Uh oh!
There was an error while loading. Please reload this page.
2 tasks
ptr727 added a commit
that referenced
this pull request
May 11, 2026
…#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 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.
Summary
Three small fixes for PR #71 (develop → main release) Copilot review:
behaviour→behavior(.github/workflows/merge-bot-pull-request.yml — comment) — AGENTS.md US-English style rule.secrets: inheriton theget-versionjob in build-pypilibrary-task.yml —get-version-task.ymldoesn't declare any required secrets, so inheriting was a no-op that widened the secret blast radius unnecessarily.merge-codegento App-actor synchronize events — theif:gate previously checked PR author/branch/base but not the event actor. A maintainer pushing extra commits to the App'scodegenbranch would fire asynchronizeevent the job would happily auto-merge, folding human changes into a release through the App PR. Restoredgithub.actor == 'ptr727-codegen[bot]'to the gate (it was carried by the pre-consolidationmerge-codegen-appjob; got dropped during the App-only consolidation in PR Standardize bots on GitHub App token, align merge methods, version PyPI via NBGV #70). Both checks are now in place with a comment explaining why each matters.Out of scope
secrets: inheriton its innerget-versioncall. This branch doesn't touch that file; per AGENTS.md "Workflow YAML Conventions" ("existing workflows are migrated opportunistically when they're being touched for other reasons"), leaving it alone here. Easy 1-line follow-up PR later.Test plan