Bypass the approved-issue check for all repository collaborators

docs/pr_policy.md

@@ -26,7 +26,7 @@ To keep review bandwidth focused on high-leverage improvements and prevent dupli
 
 Our automated workflow checks every pull request from external contributors when a PR is `opened`, `edited`, `reopened`, or marked `ready_for_review`:
 
-- **Who is checked**: All external contributors (`FIRST_TIME_CONTRIBUTOR`, `CONTRIBUTOR`, `NONE`). Repository owners (`OWNER`), organization members (`MEMBER`), collaborators (`COLLABORATOR`), and bots (`Bot` / `[bot]`) are exempt.
+- **Who is checked**: All external contributors (`FIRST_TIME_CONTRIBUTOR`, `CONTRIBUTOR`, `NONE`). Repository owners (`OWNER`), organization members (`MEMBER`, whether public or private), collaborators (`COLLABORATOR`, whether granted directly or through a team), and bots (`Bot` / `[bot]`) are exempt.
 - **How issues are matched**:
   - The workflow strips HTML comments (`<!-- ... -->`) from the PR description and scans for issue references belonging to the same repository (`#xxx`, `owner/repo#xxx`, or `https://github.com/owner/repo/issues/xxx`).
   - Pull request numbers are ignored — the linked number must be an **issue** in the repository, and your GitHub username must be in the issue's **Assignees** list.

scripts/pr-approved-issue.js

@@ -26,7 +26,10 @@
  * - When the check fails: converts non-draft PRs to draft, posts a comment (on
  *   `opened` or when converting to draft), and marks the check as failed.
  *
- * Maintainers, collaborators and bots are skipped by the workflow `if:`.
+ * Maintainers, collaborators and bots are skipped by the workflow `if:`. All
+ * repository collaborators (including private org members and team members) are
+ * also skipped here, because the event payload's `author_association` only
+ * reports `MEMBER` when the org membership is public.
  */
 
 const POLICY_DOC_URL =
@@ -66,6 +69,14 @@ module.exports = async function prApprovedIssue({ github, context, core }) {
     return;
   }
 
+  // `author_association` only reports MEMBER when the org membership is
+  // public. Private org members whose access comes through the org or a team
+  // show up as CONTRIBUTOR, so check collaborator membership directly.
+  if (await isCollaborator(github, core, owner, repo, author)) {
+    core.info(`Skipping #${pr.number}: ${author} is a collaborator on ${owner}/${repo}.`);
+    return;
+  }
+
   const body = pr.body || "";
 
   // 1. Find issues referenced in the description that are assigned to the author.
@@ -148,6 +159,26 @@ function findIssueNumbers(body, owner, repo) {
   return [...numbers];
 }
 
+/**
+ * True when `username` is a collaborator on the repo (204), including via
+ * organization or team membership at any role (admin, maintain, write, triage,
+ * or org-default read). External contributors return 404.
+ */
+async function isCollaborator(github, core, owner, repo, username) {
+  try {
+    await github.rest.repos.checkCollaborator({ owner, repo, username });
+    return true;
+  } catch (err) {
+    const message = `Could not check collaborator status for ${username}: ${err.message}`;
+    if (err.status === 404) {
+      core.info(message);
+    } else {
+      core.warning(message);
+    }
+    return false;
+  }
+}
+
 async function convertToDraft(github, core, pr) {
   try {
     await github.graphql(

scripts/pr-approved-issue.test.js

@@ -28,12 +28,15 @@ describe('prApprovedIssue', () => {
     pr = null,
     payloadPr = undefined,
     issuesByNumber = {},
+    collaborators = new Set(),
+    collaboratorError = null,
     graphqlError = null,
   }) {
     const infoMessages = [];
     const warningMessages = [];
     let failedMessage = null;
     const fetchedIssues = [];
+    const collaboratorLookups = [];
     const graphqlCalls = [];
     const createdComments = [];
 
@@ -50,6 +53,17 @@ describe('prApprovedIssue', () => {
         pulls: {
           get: async () => ({ data: pr }),
         },
+        repos: {
+          checkCollaborator: async ({ username }) => {
+            collaboratorLookups.push(username);
+            if (collaboratorError) {
+              throw collaboratorError;
+            }
+            if (!collaborators.has(username)) {
+              throw Object.assign(new Error('Not Found'), { status: 404 });
+            }
+          },
+        },
         issues: {
           get: async ({ issue_number }) => {
             fetchedIssues.push(issue_number);
@@ -86,6 +100,7 @@ describe('prApprovedIssue', () => {
       getWarningMessages: () => warningMessages,
       getFailedMessage: () => failedMessage,
       getFetchedIssues: () => fetchedIssues,
+      getCollaboratorLookups: () => collaboratorLookups,
       getGraphqlCalls: () => graphqlCalls,
       getCreatedComments: () => createdComments,
     };
@@ -120,9 +135,93 @@ describe('prApprovedIssue', () => {
       assert.strictEqual(harness.getFailedMessage(), null);
       assert.strictEqual(harness.getGraphqlCalls().length, 0);
       assert.strictEqual(harness.getCreatedComments().length, 0);
+      // The association fast path returns before the collaborator lookup.
+      assert.deepStrictEqual(harness.getCollaboratorLookups(), []);
     }
   });
 
+  it('skips repository collaborators (including private org and team members) even when author_association is not bypassed', async () => {
+    // A private org member whose access comes through the org or a team is
+    // reported as CONTRIBUTOR (or NONE) in the event payload, not MEMBER,
+    // regardless of whether their role is admin, maintain, write, triage, or
+    // org-default read. `checkCollaborator` returns 204 for all of them.
+    for (const association of ['CONTRIBUTOR', 'FIRST_TIME_CONTRIBUTOR', 'NONE']) {
+      const harness = createHarness({
+        action: 'opened',
+        pr: {
+          number: 20,
+          node_id: 'PR_20',
+          draft: false,
+          author_association: association,
+          user: { login: 'private-member', type: 'User' },
+          body: 'No issue link needed',
+        },
+        collaborators: new Set(['private-member']),
+      });
+      await harness.run();
+      assert.strictEqual(harness.getFailedMessage(), null);
+      assert.strictEqual(harness.getGraphqlCalls().length, 0);
+      assert.strictEqual(harness.getCreatedComments().length, 0);
+      assert.deepStrictEqual(harness.getCollaboratorLookups(), ['private-member']);
+      assert.ok(
+        harness
+          .getInfoMessages()
+          .includes('Skipping #20: private-member is a collaborator on keras-team/keras-hub.')
+      );
+    }
+  });
+
+  it('still runs the check for non-collaborators (404) without emitting a warning', async () => {
+    const harness = createHarness({
+      action: 'opened',
+      pr: {
+        number: 30,
+        node_id: 'PR_30',
+        draft: false,
+        author_association: 'CONTRIBUTOR',
+        user: { login: 'external-dev', type: 'User' },
+        body: 'Here is my new feature.',
+      },
+    });
+    await harness.run();
+    assert.deepStrictEqual(harness.getCollaboratorLookups(), ['external-dev']);
+    // A 404 is the expected non-collaborator path and must not warn.
+    assert.strictEqual(harness.getWarningMessages().length, 0);
+    assert.strictEqual(harness.getGraphqlCalls().length, 1);
+    assert.strictEqual(harness.getCreatedComments().length, 1);
+    assert.strictEqual(
+      harness.getFailedMessage(),
+      'No approved issue assigned to external-dev is linked in #30.'
+    );
+  });
+
+  it('warns but still runs the check when the collaborator lookup fails for a non-404 reason', async () => {
+    const harness = createHarness({
+      action: 'opened',
+      pr: {
+        number: 31,
+        node_id: 'PR_31',
+        draft: false,
+        author_association: 'CONTRIBUTOR',
+        user: { login: 'external-dev', type: 'User' },
+        body: 'Here is my new feature.',
+      },
+      collaboratorError: Object.assign(new Error('Forbidden'), { status: 403 }),
+    });
+
+    await harness.run();
+
+    assert.strictEqual(harness.getWarningMessages().length, 1);
+    assert.ok(
+      harness
+        .getWarningMessages()[0]
+        .includes('Could not check collaborator status for external-dev: Forbidden')
+    );
+    assert.strictEqual(harness.getGraphqlCalls().length, 1);
+    assert.strictEqual(harness.getCreatedComments().length, 1);
+    assert.notStrictEqual(harness.getFailedMessage(), null);
+  });
+
   it('fetches live PR state via pulls.get instead of relying on frozen event payload when re-run', async () => {
     const harness = createHarness({
       action: 'opened',