Replace CODEOWNERS with /merge command#32
Open
magicmark wants to merge 20 commits into
Open
Conversation
CODEOWNERS requires all listed users to have repo write access, which doesn't work for external GAP authors. Replace it with a GitHub Action that reads metadata.yml to determine who can merge PRs scoped to a single GAP directory. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Remove auto-merge on PR approval — merging should always be an explicit action via /merge comment. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- Rename author-merge.yml -> merge.yml - Rename job to "merge" - Use contains() for slightly more forgiving /merge detection - Remove redundant comments Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- Fix shell injection: route all ${{ }} through env vars
- Fix empty-grep bug: explicit check for zero GAP dirs
- Fix regex matching: use grep -qFx for username comparison
- Use portable grep -oE instead of Perl -oP
- Add concurrency guard to prevent merge races
- Remove redundant "Get PR details" step
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
magicmark
changed the title
/merge command
magicmark
requested review from
benjie and
martinbonnin
and removed request for
benjie
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Bash step resolves metadata (authors/sponsor as JSON), then a github-script step does the authorization and path-scoping checks using the GitHub API. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This reverts commit 0a58804.
magicmark
commented
May 15, 2026
Use actions/github-script for verification logic instead of shell. Checks changed files via the GitHub API and reads metadata.yml with js-yaml to authorize the commenter. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
benjie
reviewed
May 15, 2026
Addresses review feedback: move JS out of YAML into scripts/verify-merge.js. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <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
2 participants
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.
Oops!
CODEOWNERS is currently invalid:
This makes sense.... most authors will not be "members" of the repo. So CODEOWNERS as a gating mechanism doesn't really work.
Proposal: /merge
Instead, claude and I propose
/mergewhich is a command that can be run to merge on behalf of authors. It will only merge iff:validate-stucture.js)/mergefor file(s) changed in their GAP directory.Bonus: this means we can re-enable
Require a pull request before mergingcc @benjie wdyt?