From 82bb91117e2469bffc7ccecc7f2ab1e325dec8ce Mon Sep 17 00:00:00 2001 From: Matthew Ball Date: Fri, 8 May 2026 02:13:24 -0700 Subject: [PATCH 1/2] add /request-review and /unrequest-review comment commands for PR author and committers --- .github/workflows/comment-commands.yml | 183 +++++++++++++++++++++++++ .github/workflows/take-commands.yml | 85 ------------ 2 files changed, 183 insertions(+), 85 deletions(-) create mode 100644 .github/workflows/comment-commands.yml delete mode 100644 .github/workflows/take-commands.yml diff --git a/.github/workflows/comment-commands.yml b/.github/workflows/comment-commands.yml new file mode 100644 index 00000000000..d78096010e4 --- /dev/null +++ b/.github/workflows/comment-commands.yml @@ -0,0 +1,183 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# /take, /untake, /request-review, and /unrequest-review comment commands. +# +# Triage state is no longer materialized as a label — it is the search +# filter `is:issue is:open no:assignee`. Anyone can self-claim an issue +# by commenting `/take` (and self-release with `/untake`); PR-driven +# assignee sync is handled by `pr-assignment.yml`. +# +# On pull requests, the author or any committer (write/maintain/admin) +# can request or cancel reviewer requests via `/request-review @user +# [@user ...]` and `/unrequest-review @user [@user ...]`. We avoid the +# `/review` namespace so it stays free for future use (e.g. self-review). +name: Comment commands +on: + issue_comment: + types: [created] + +permissions: + issues: write + pull-requests: write + +jobs: + take: + # The startsWith filter at the job level keeps unrelated comments + # from allocating a runner; the regex inside the script enforces an + # exact `/take` or `/untake` so suffixes like `/take this` do not + # silently match. + if: >- + github.event_name == 'issue_comment' + && github.event.action == 'created' + && github.event.issue.pull_request == null + && github.event.comment.user.type != 'Bot' + && (startsWith(github.event.comment.body, '/take') + || startsWith(github.event.comment.body, '/untake')) + runs-on: ubuntu-latest + steps: + - uses: actions/github-script@v8 + with: + github-token: ${{ secrets.GITHUB_TOKEN }} + script: | + const body = (context.payload.comment.body || '').trim(); + const issue_number = context.payload.issue.number; + const login = context.payload.comment.user.login; + const { owner, repo } = context.repo; + core.info( + `take/untake candidate: ${login} on issue #${issue_number}; ` + + `body=${JSON.stringify(body)}`, + ); + + if (/^\/take\s*$/.test(body)) { + try { + await github.rest.issues.addAssignees({ + owner, repo, issue_number, assignees: [login], + }); + core.info(`Assigned ${login} to issue #${issue_number}`); + } catch (e) { + core.warning( + `addAssignees on #${issue_number} failed: ${e.message}`, + ); + } + } else if (/^\/untake\s*$/.test(body)) { + try { + await github.rest.issues.removeAssignees({ + owner, repo, issue_number, assignees: [login], + }); + core.info(`Unassigned ${login} from issue #${issue_number}`); + } catch (e) { + core.warning( + `removeAssignees on #${issue_number} failed: ${e.message}`, + ); + } + } else { + core.info( + `Comment does not match exact '/take' or '/untake'; skipping.`, + ); + } + + request-review: + # Job-level startsWith gate avoids spinning up a runner for every + # PR comment; the regex inside the script enforces the exact shape. + if: >- + github.event_name == 'issue_comment' + && github.event.action == 'created' + && github.event.issue.pull_request != null + && github.event.comment.user.type != 'Bot' + && (startsWith(github.event.comment.body, '/request-review') + || startsWith(github.event.comment.body, '/unrequest-review')) + runs-on: ubuntu-latest + steps: + - uses: actions/github-script@v8 + with: + github-token: ${{ secrets.GITHUB_TOKEN }} + script: | + const body = (context.payload.comment.body || '').trim(); + const pull_number = context.payload.issue.number; + const commenter = context.payload.comment.user.login; + const author = context.payload.issue.user.login; + const { owner, repo } = context.repo; + + const match = body.match( + /^\/(request-review|unrequest-review)\b(.*)$/s, + ); + if (!match) { + core.info(`Comment does not match exact command; skipping.`); + return; + } + const action = match[1]; + + // Authorize: PR author always, otherwise commenter must hold + // write/maintain/admin on the repo (i.e. a committer). + let allowed = commenter === author; + if (!allowed) { + try { + const { data: perm } = + await github.rest.repos.getCollaboratorPermissionLevel({ + owner, repo, username: commenter, + }); + allowed = ['write', 'maintain', 'admin'].includes( + perm.permission, + ); + } catch (e) { + core.warning( + `permission lookup for ${commenter} failed: ${e.message}`, + ); + } + } + if (!allowed) { + core.info( + `${commenter} is neither author nor committer on ` + + `#${pull_number}; skipping.`, + ); + return; + } + + // Parse @user and @org/team mentions; route teams to the + // team_reviewers bucket. Strip self so the API doesn't + // reject the whole atomic call over one bad name. + const reviewers = []; + const team_reviewers = []; + for (const [, h] of match[2].matchAll( + /@([\w-]+(?:\/[\w.-]+)?)/g, + )) { + if (h.includes('/')) team_reviewers.push(h.split('/')[1]); + else if (h.toLowerCase() !== author.toLowerCase()) + reviewers.push(h); + } + if (!reviewers.length && !team_reviewers.length) { + core.warning(`No valid @mentions in '${action}'; skipping.`); + return; + } + + const params = { owner, repo, pull_number, reviewers, team_reviewers }; + try { + if (action === 'request-review') { + await github.rest.pulls.requestReviewers(params); + } else { + await github.rest.pulls.removeRequestedReviewers(params); + } + core.info( + `${action} on #${pull_number} by ${commenter}: ` + + `users=[${reviewers.join(', ')}] ` + + `teams=[${team_reviewers.join(', ')}]`, + ); + } catch (e) { + core.warning( + `${action} on #${pull_number} failed: ${e.message}`, + ); + } diff --git a/.github/workflows/take-commands.yml b/.github/workflows/take-commands.yml deleted file mode 100644 index bf567f62408..00000000000 --- a/.github/workflows/take-commands.yml +++ /dev/null @@ -1,85 +0,0 @@ -# Licensed to the Apache Software Foundation (ASF) under one -# or more contributor license agreements. See the NOTICE file -# distributed with this work for additional information -# regarding copyright ownership. The ASF licenses this file -# to you under the Apache License, Version 2.0 (the -# "License"); you may not use this file except in compliance -# with the License. You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -# /take and /untake comment commands. -# -# Triage state is no longer materialized as a label — it is the search -# filter `is:issue is:open no:assignee`. Anyone can self-claim an issue -# by commenting `/take` (and self-release with `/untake`); PR-driven -# assignee sync is handled by `pr-assignment.yml`. -name: Issue take commands -on: - issue_comment: - types: [created] - -permissions: - issues: write - -jobs: - take: - # The startsWith filter at the job level keeps unrelated comments - # from allocating a runner; the regex inside the script enforces an - # exact `/take` or `/untake` so suffixes like `/take this` do not - # silently match. - if: >- - github.event_name == 'issue_comment' - && github.event.action == 'created' - && github.event.issue.pull_request == null - && github.event.comment.user.type != 'Bot' - && (startsWith(github.event.comment.body, '/take') - || startsWith(github.event.comment.body, '/untake')) - runs-on: ubuntu-latest - steps: - - uses: actions/github-script@v8 - with: - github-token: ${{ secrets.GITHUB_TOKEN }} - script: | - const body = (context.payload.comment.body || '').trim(); - const issue_number = context.payload.issue.number; - const login = context.payload.comment.user.login; - const { owner, repo } = context.repo; - core.info( - `take/untake candidate: ${login} on issue #${issue_number}; ` + - `body=${JSON.stringify(body)}`, - ); - - if (/^\/take\s*$/.test(body)) { - try { - await github.rest.issues.addAssignees({ - owner, repo, issue_number, assignees: [login], - }); - core.info(`Assigned ${login} to issue #${issue_number}`); - } catch (e) { - core.warning( - `addAssignees on #${issue_number} failed: ${e.message}`, - ); - } - } else if (/^\/untake\s*$/.test(body)) { - try { - await github.rest.issues.removeAssignees({ - owner, repo, issue_number, assignees: [login], - }); - core.info(`Unassigned ${login} from issue #${issue_number}`); - } catch (e) { - core.warning( - `removeAssignees on #${issue_number} failed: ${e.message}`, - ); - } - } else { - core.info( - `Comment does not match exact '/take' or '/untake'; skipping.`, - ); - } From 83c30bca79618079e5f14ac839d7d5c91a18e0e9 Mon Sep 17 00:00:00 2001 From: Matthew Ball Date: Fri, 8 May 2026 17:09:48 -0700 Subject: [PATCH 2/2] restrict /request-review to PR author and allow @copilot as reviewer --- .github/workflows/comment-commands.yml | 36 +++++++------------------- 1 file changed, 10 insertions(+), 26 deletions(-) diff --git a/.github/workflows/comment-commands.yml b/.github/workflows/comment-commands.yml index d78096010e4..3300db1353d 100644 --- a/.github/workflows/comment-commands.yml +++ b/.github/workflows/comment-commands.yml @@ -21,10 +21,10 @@ # by commenting `/take` (and self-release with `/untake`); PR-driven # assignee sync is handled by `pr-assignment.yml`. # -# On pull requests, the author or any committer (write/maintain/admin) -# can request or cancel reviewer requests via `/request-review @user -# [@user ...]` and `/unrequest-review @user [@user ...]`. We avoid the -# `/review` namespace so it stays free for future use (e.g. self-review). +# On pull requests, the author can request or cancel reviewer requests +# via `/request-review @user [@user ...]` and `/unrequest-review @user +# [@user ...]`. We avoid the `/review` namespace so it stays free for +# future use (e.g. self-review). name: Comment commands on: issue_comment: @@ -121,41 +121,25 @@ jobs: } const action = match[1]; - // Authorize: PR author always, otherwise commenter must hold - // write/maintain/admin on the repo (i.e. a committer). - let allowed = commenter === author; - if (!allowed) { - try { - const { data: perm } = - await github.rest.repos.getCollaboratorPermissionLevel({ - owner, repo, username: commenter, - }); - allowed = ['write', 'maintain', 'admin'].includes( - perm.permission, - ); - } catch (e) { - core.warning( - `permission lookup for ${commenter} failed: ${e.message}`, - ); - } - } - if (!allowed) { + if (commenter !== author) { core.info( - `${commenter} is neither author nor committer on ` + - `#${pull_number}; skipping.`, + `${commenter} is not the author of #${pull_number}; skipping.`, ); return; } // Parse @user and @org/team mentions; route teams to the // team_reviewers bucket. Strip self so the API doesn't - // reject the whole atomic call over one bad name. + // reject the whole atomic call over one bad name. Copilot + // is a bot reviewer that the REST API expects as the exact + // slug "Copilot", so normalize any casing of @copilot. const reviewers = []; const team_reviewers = []; for (const [, h] of match[2].matchAll( /@([\w-]+(?:\/[\w.-]+)?)/g, )) { if (h.includes('/')) team_reviewers.push(h.split('/')[1]); + else if (h.toLowerCase() === 'copilot') reviewers.push('Copilot'); else if (h.toLowerCase() !== author.toLowerCase()) reviewers.push(h); }