fix(external-plugins): block approval during intake reruns

Remove stale ready-for-review state before evaluating edited submissions or running quality gates. Require a current ready state before fast-tracking or approving, and cover the pending transition with focused tests.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 821f4796-505e-4fb8-96cf-0b5bbf80a636

.github/extensions/external-plugin-review-board/extension.mjs

@@ -134,8 +134,11 @@ async function postDecision({ number, kind, comment }) {
 
     const status = await gh.getIssueStatus(store.state.repo, number);
     if (status.state !== "OPEN") throw new Error(`#${number} is ${status.state.toLowerCase()}. Refresh the board.`);
-    if (kind === "approve" && !status.labels.includes("ready-for-review")) {
-        throw new Error(`#${number} is no longer labelled ready-for-review, so /approve would be ignored.`);
+    if (
+        kind === "approve" &&
+        (!status.labels.includes("ready-for-review") || status.labels.includes("awaiting-review"))
+    ) {
+        throw new Error(`#${number} is pending intake or is no longer ready-for-review, so /approve would be ignored.`);
     }
 
     const text = stripCommand(comment, kind);

.github/extensions/external-plugin-review-board/lib/state.mjs

@@ -17,12 +17,14 @@ export const COLUMNS = [
 const HISTORY_LIMIT = 200;
 export const FIRST_PARTY_LABEL = "first party";
 
-// Submissions from first-party organizations skip AI triage and land in Accept. Only cards without
-// an AI review, queued review, or manual bucket are fast-tracked so maintainer choices are kept;
-// dragging a card back to Unreviewed opts it out so it gets a normal AI review.
+// Validated submissions from first-party organizations skip AI triage and land in Accept. Only
+// cards without an AI review, queued review, or manual bucket are fast-tracked so maintainer
+// choices are kept; dragging a card back to Unreviewed opts it out so it gets a normal AI review.
 function applyFirstPartyFastTrack(item) {
     if (
         !item.labels?.includes(FIRST_PARTY_LABEL) ||
+        !item.labels?.includes("ready-for-review") ||
+        item.labels?.includes("awaiting-review") ||
         item.fastTrackDismissed ||
         item.decision ||
         item.review ||
@@ -44,7 +46,13 @@ function applyFirstPartyFastTrack(item) {
 }
 
 function clearFirstPartyFastTrack(item) {
-    if (item.labels?.includes(FIRST_PARTY_LABEL) || !item.review?.fastTracked || item.reviewStatus === "queued") {
+    if (
+        (item.labels?.includes(FIRST_PARTY_LABEL) &&
+            item.labels?.includes("ready-for-review") &&
+            !item.labels?.includes("awaiting-review")) ||
+        !item.review?.fastTracked ||
+        item.reviewStatus === "queued"
+    ) {
         return;
     }
     item.review = null;

.github/workflows/external-plugin-command-router.yml

@@ -85,7 +85,8 @@ jobs:
             });
 
             const fallbackName = evaluation.plugin?.name ?? `issue-${context.issue.number}`;
-            const canApprove = labelNames.has('ready-for-review') || labelNames.has('approved');
+            const canApprove = labelNames.has('approved') ||
+              (labelNames.has('ready-for-review') && !labelNames.has('awaiting-review'));
             const canReject = !labelNames.has('approved');
 
             if (parsedCommand.command === 'approve' && !canApprove) {

.github/workflows/external-plugin-intake.yml

@@ -48,6 +48,22 @@ jobs:
             core.setOutput('issue-labels', JSON.stringify(labels));
             core.setOutput('should-sync', (!isApproved && !isClosedWithoutReopen) ? 'true' : 'false');
 
+      - name: Mark submission as awaiting review
+        if: steps.guard.outputs.should-sync == 'true'
+        uses: actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b # v7.1.0
+        with:
+          script: |
+            const path = require('path');
+            const { pathToFileURL } = require('url');
+            const intakeState = await import(pathToFileURL(path.join(process.env.GITHUB_WORKSPACE, 'eng', 'external-plugin-intake-state.mjs')).href);
+
+            await intakeState.markExternalPluginIntakeAwaitingReview({
+              github,
+              owner: context.repo.owner,
+              repo: context.repo.repo,
+              issueNumber: context.issue.number
+            });
+
       - name: Evaluate submission
         id: evaluation
         env:

eng/external-plugin-intake-state.mjs

@@ -100,6 +100,19 @@ export async function syncExternalPluginIntakeLabels({
   }
 }
 
+export async function markExternalPluginIntakeAwaitingReview({ github, owner, repo, issueNumber }) {
+  for (const name of ["ready-for-review", "requires-submitter-fixes", "rejected"]) {
+    await removeLabel({ github, owner, repo, issueNumber, name });
+  }
+
+  await github.rest.issues.addLabels({
+    owner,
+    repo,
+    issue_number: issueNumber,
+    labels: ["external-plugin", "awaiting-review"],
+  });
+}
+
 export async function upsertExternalPluginIntakeComment({
   github,
   owner,

eng/external-plugin-intake-state.test.mjs

@@ -2,13 +2,16 @@ import assert from "node:assert/strict";
 import { test } from "node:test";
 import {
   applyExternalPluginIntakeEvaluation,
+  markExternalPluginIntakeAwaitingReview,
   syncExternalPluginIntakeLabels,
 } from "./external-plugin-intake-state.mjs";
 
 function mockGitHub(currentLabels) {
   const labels = new Set(currentLabels);
+  const operations = [];
   return {
     labels,
+    operations,
     async paginate() {
       return [...labels].map((name) => ({ name }));
     },
@@ -16,9 +19,11 @@ function mockGitHub(currentLabels) {
       issues: {
         listLabelsOnIssue() {},
         async addLabels({ labels: addedLabels }) {
+          operations.push(["add", addedLabels]);
           for (const name of addedLabels) labels.add(name);
         },
         async removeLabel({ name }) {
+          operations.push(["remove", name]);
           labels.delete(name);
         },
         async listComments() {
@@ -68,3 +73,32 @@ test("state-only label sync preserves ownership labels for maintainer overrides
 
   assert.deepEqual(github.labels, new Set(["external-plugin", "ready-for-review", "first party"]));
 });
+
+test("marking intake pending removes stale state labels but preserves ownership, canvas, and unrelated labels", async () => {
+  const github = mockGitHub([
+    "external-plugin",
+    "external-plugin-canvas",
+    "first party",
+    "ready-for-review",
+    "requires-submitter-fixes",
+    "rejected",
+    "unrelated",
+  ]);
+
+  await markExternalPluginIntakeAwaitingReview({
+    github,
+    owner: "github",
+    repo: "awesome-copilot",
+    issueNumber: 1,
+  });
+
+  assert.deepEqual(
+    github.labels,
+    new Set(["external-plugin", "external-plugin-canvas", "first party", "awaiting-review", "unrelated"]),
+  );
+  const readyRemoval = github.operations.findIndex(([action, name]) => action === "remove" && name === "ready-for-review");
+  const pendingAddition = github.operations.findIndex(
+    ([action, labels]) => action === "add" && labels.includes("awaiting-review"),
+  );
+  assert.ok(readyRemoval >= 0 && pendingAddition > readyRemoval);
+});

eng/external-plugin-review-board-state.test.mjs

@@ -37,6 +37,43 @@ test("first-party submissions are fast-tracked to accept on refresh", async (t)
   assert.deepEqual(store.pendingReview().map((item) => item.number).sort(), [2, 3]);
 });
 
+test("first-party submissions are not fast-tracked while intake is pending", async (t) => {
+  const store = await createStore(t);
+  const pendingIssue = issue(1, ["first party", "awaiting-review"]);
+  await store.syncIssues([pendingIssue]);
+
+  const item = store.getItem(1);
+  assert.equal(item.review, null);
+  assert.equal(item.reviewStatus, "unreviewed");
+  assert.equal(effectiveColumn(item), "unreviewed");
+});
+
+test("first-party submissions are not fast-tracked without a ready-for-review label", async (t) => {
+  const store = await createStore(t);
+  const pendingIssue = issue(1, ["first party"]);
+  pendingIssue.labels = pendingIssue.labels.filter((label) => label !== "ready-for-review");
+
+  await store.syncIssues([pendingIssue]);
+
+  const item = store.getItem(1);
+  assert.equal(item.review, null);
+  assert.equal(item.reviewStatus, "unreviewed");
+});
+
+test("an existing first-party fast-track is cleared while intake is pending", async (t) => {
+  const store = await createStore(t);
+  await store.syncIssues([issue(1, ["first party"])]);
+  const pendingIssue = issue(1, ["first party", "awaiting-review"]);
+  pendingIssue.labels = pendingIssue.labels.filter((label) => label !== "ready-for-review");
+
+  await store.syncIssues([pendingIssue]);
+
+  const item = store.getItem(1);
+  assert.equal(item.review, null);
+  assert.equal(item.reviewStatus, "unreviewed");
+  assert.equal(effectiveColumn(item), "unreviewed");
+});
+
 test("fast-track applies when an existing card gains the first party label", async (t) => {
   const store = await createStore(t);
   await store.syncIssues([issue(1)]);