From 46154be2eac10f4a5aa923efdcaa6884b152a882 Mon Sep 17 00:00:00 2001 From: Jake Wallin Date: Thu, 15 Oct 2026 04:00:40 +0000 Subject: [PATCH] GH-34389: [Dev] Verify issue assignment before reporting success MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ### Rationale for this change Closes #34389. GitHub can silently ignore an issue assignment when the contributor is not yet eligible to be assigned. The PR bot currently announces success even when the returned issue has no assignees. ### What changes are included in this PR? Send the assignees as an array and verify that the assignment response includes the PR creator before posting the success message. If assignment was ignored, link the issue and ask the contributor to leave a comment requesting assignment by a committer or collaborator, following the contributor documentation. Add dependency-free Node regression tests and run them in a read-only Dev workflow job. ### Are these changes tested? - `node --test .github/workflows/dev_pr/*.test.js` — 6 passed. Three cases fail with the original implementation. - Coverage includes successful assignment, silently ignored assignment, a different returned assignee, an already assigned issue, and unrelated PR titles. - JavaScript syntax checks, workflow YAML parsing, and `git diff --check` pass. ### Are there any user-facing changes? The bot reports assignment success only when GitHub confirms it. Contributors whose automatic assignment is rejected receive a link and instructions to request assignment instead of an incorrect success message. --- .github/workflows/dev.yml | 14 +++ .github/workflows/dev_pr/issue_check.js | 13 ++- .github/workflows/dev_pr/issue_check.test.js | 102 +++++++++++++++++++ 3 files changed, 126 insertions(+), 3 deletions(-) create mode 100644 .github/workflows/dev_pr/issue_check.test.js diff --git a/.github/workflows/dev.yml b/.github/workflows/dev.yml index 7c3607bd1df3..9419f2f2029e 100644 --- a/.github/workflows/dev.yml +++ b/.github/workflows/dev.yml @@ -39,6 +39,20 @@ env: jobs: + pr-scripts: + name: PR Workflow Script Tests + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - uses: actions/checkout@v7 + with: + persist-credentials: false + - uses: actions/setup-node@v6 + with: + node-version: '24' + - name: Test PR workflow scripts + run: node --test .github/workflows/dev_pr/*.test.js + lint: name: Lint C++, Python, R, Docker, RAT # Use Ubuntu 24.04 to ensure working pre-commit on Ubuntu 24.04. diff --git a/.github/workflows/dev_pr/issue_check.js b/.github/workflows/dev_pr/issue_check.js index 176349cdd051..ef2187af504d 100644 --- a/.github/workflows/dev_pr/issue_check.js +++ b/.github/workflows/dev_pr/issue_check.js @@ -26,17 +26,24 @@ const helpers = require("./helpers.js"); * @param {Object} issueInfo */ async function assignGitHubIssue(github, context, pullRequestNumber, issueInfo) { - await github.rest.issues.addAssignees({ + const creator = context.payload.pull_request.user.login; + const response = await github.rest.issues.addAssignees({ owner: context.repo.owner, repo: context.repo.repo, issue_number: issueInfo.number, - assignees: context.payload.pull_request.user.login + assignees: [creator] }); + const assigned = response.data.assignees.some((assignee) => assignee.login === creator); + const body = assigned + ? ":warning: GitHub issue #" + issueInfo.number + " **has been automatically assigned in GitHub** to PR creator." + : ":warning: GitHub issue #" + issueInfo.number + " **could not be automatically assigned** to the PR creator. " + + "Please leave a comment on " + issueInfo.html_url + " requesting assignment. " + + "A committer or collaborator can then assign the issue to you."; await github.rest.issues.createComment({ owner: context.repo.owner, repo: context.repo.repo, issue_number: pullRequestNumber, - body: ":warning: GitHub issue #" + issueInfo.number + " **has been automatically assigned in GitHub** to PR creator." + body }); } diff --git a/.github/workflows/dev_pr/issue_check.test.js b/.github/workflows/dev_pr/issue_check.test.js new file mode 100644 index 000000000000..37a50e7c9b9e --- /dev/null +++ b/.github/workflows/dev_pr/issue_check.test.js @@ -0,0 +1,102 @@ +// 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. + +const assert = require("node:assert/strict"); +const {test, mock} = require("node:test"); +const checkIssue = require("./issue_check.js"); + +function request({assignees = [], returnedAssignees = [{login: "contributor"}], + title = "GH-34389: Check assignment results"} = {}) { + const issue = { + number: 34389, + html_url: "https://github.com/apache/arrow/issues/34389", + assignees, + labels: [{name: "Component: Developer Tools"}] + }; + const issues = { + get: mock.fn(async () => ({data: issue})), + addAssignees: mock.fn(async () => ({ + data: {...issue, assignees: returnedAssignees} + })), + createComment: mock.fn(async () => ({})) + }; + const github = {rest: {issues}}; + const context = { + repo: {owner: "apache", repo: "arrow"}, + payload: { + number: 50000, + pull_request: {title, user: {login: "contributor"}} + } + }; + return {github, context, issues}; +} + +test("reports successful assignment only after GitHub assigns the PR creator", async () => { + const input = request(); + await checkIssue(input); + + assert.equal(input.issues.addAssignees.mock.callCount(), 1); + assert.deepEqual(input.issues.addAssignees.mock.calls[0].arguments[0], { + owner: "apache", repo: "arrow", issue_number: 34389, + assignees: ["contributor"] + }); + assert.equal(input.issues.createComment.mock.callCount(), 1); + assert.deepEqual(input.issues.createComment.mock.calls[0].arguments[0], { + owner: "apache", repo: "arrow", issue_number: 50000, + body: ":warning: GitHub issue #34389 **has been automatically assigned in GitHub** to PR creator." + }); +}); + +for (const [description, returnedAssignees] of [ + ["silently ignores the assignment", []], + ["returns a different assignee", [{login: "another-contributor"}]] +]) { + test(`provides assignment instructions when GitHub ${description}`, async () => { + const input = request({returnedAssignees}); + await checkIssue(input); + + assert.equal(input.issues.addAssignees.mock.callCount(), 1); + assert.equal(input.issues.createComment.mock.callCount(), 1); + const comment = input.issues.createComment.mock.calls[0].arguments[0]; + assert.equal(comment.issue_number, 50000); + assert.doesNotMatch(comment.body, /has been automatically assigned/); + assert.match(comment.body, /https:\/\/github.com\/apache\/arrow\/issues\/34389/); + assert.match(comment.body, /comment/i); + assert.match(comment.body, /assign/i); + assert.match(comment.body, /maintainer|committer|collaborator/i); + }); +} + +test("leaves an already assigned issue unchanged", async () => { + const input = request({assignees: [{login: "another-contributor"}]}); + await checkIssue(input); + + assert.equal(input.issues.get.mock.callCount(), 1); + assert.equal(input.issues.addAssignees.mock.callCount(), 0); + assert.equal(input.issues.createComment.mock.callCount(), 0); +}); + +for (const title of ["MINOR: Fix a typo", "Fix assignment results"]) { + test(`skips API calls for PR title '${title}'`, async () => { + const input = request({title}); + await checkIssue(input); + + assert.equal(input.issues.get.mock.callCount(), 0); + assert.equal(input.issues.addAssignees.mock.callCount(), 0); + assert.equal(input.issues.createComment.mock.callCount(), 0); + }); +}