fix(external-plugins): prevent false fast-track approvals

Exclude manually overridden submissions from first-party fast-track until a fresh intake pass clears the override label. Disable individual approval controls while intake is pending.

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

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

@@ -16,6 +16,7 @@ export const COLUMNS = [
 
 const HISTORY_LIMIT = 200;
 export const FIRST_PARTY_LABEL = "first party";
+const MANUAL_INTAKE_OVERRIDE_LABEL = "intake-manually-overridden";
 const PENDING_INTAKE_LABELS = ["awaiting-review", "requires-submitter-fixes"];
 
 export function isReadyForApproval(labels) {
@@ -30,6 +31,7 @@ function applyFirstPartyFastTrack(item) {
         !item.labels?.includes(FIRST_PARTY_LABEL) ||
         !item.labels?.includes("ready-for-review") ||
         item.labels?.some((label) => PENDING_INTAKE_LABELS.includes(label)) ||
+        item.labels?.includes(MANUAL_INTAKE_OVERRIDE_LABEL) ||
         item.fastTrackDismissed ||
         item.decision ||
         item.review ||
@@ -54,6 +56,7 @@ function clearFirstPartyFastTrack(item) {
     if (
         (item.labels?.includes(FIRST_PARTY_LABEL) &&
             item.labels?.includes("ready-for-review") &&
+            !item.labels?.includes(MANUAL_INTAKE_OVERRIDE_LABEL) &&
             !item.labels?.some((label) => PENDING_INTAKE_LABELS.includes(label))) ||
         !item.review?.fastTracked ||
         item.reviewStatus === "queued"

.github/extensions/external-plugin-review-board/public/app.js

@@ -412,7 +412,7 @@ function renderDrawer() {
 
     ui.drawerFooter.hidden = Boolean(item.decision);
     updatePreview();
-    ui.approve.disabled = bulkBusy || Boolean(item.pendingDecision);
+    ui.approve.disabled = bulkBusy || !item.reviewEligible || Boolean(item.pendingDecision);
     ui.reject.disabled ||= bulkBusy || Boolean(item.pendingDecision);
     renderTab(item);
 }
@@ -584,6 +584,7 @@ function resetConfirm() {
         ui.approve.disabled = true;
         ui.reject.disabled = true;
     }
+    if (!item?.reviewEligible) ui.approve.disabled = true;
 }
 
 async function decide(kind) {

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

@@ -549,6 +549,23 @@ jobs:
               return;
             }
 
+            try {
+              await github.rest.issues.getLabel({
+                owner: context.repo.owner,
+                repo: context.repo.repo,
+                name: 'intake-manually-overridden'
+              });
+            } catch (error) {
+              if (error.status !== 404) throw error;
+              await github.rest.issues.createLabel({
+                owner: context.repo.owner,
+                repo: context.repo.repo,
+                name: 'intake-manually-overridden',
+                color: 'BFD4F2',
+                description: 'Submission was manually moved to review without passing automated intake gates'
+              });
+            }
+
             await github.rest.reactions.createForIssueComment({
               owner: context.repo.owner,
               repo: context.repo.repo,
@@ -564,6 +581,7 @@ jobs:
               desiredLabels: new Set([
                 'external-plugin',
                 'ready-for-review',
+                'intake-manually-overridden',
                 ...(labelNames.has('external-plugin-canvas') ? ['external-plugin-canvas'] : [])
               ])
             });

.github/workflows/setup-labels.yml

@@ -81,6 +81,10 @@ jobs:
                 color: 'D93F0B',
                 description: 'Submission has quality-gate findings that submitter must fix before maintainer review'
               },
+              'intake-manually-overridden': {
+                color: 'BFD4F2',
+                description: 'Submission was manually moved to review without passing automated intake gates'
+              },
               'approved': {
                 color: '1D76DB',
                 description: 'Submission was approved by a maintainer'

eng/external-plugin-intake-state.mjs

@@ -27,6 +27,10 @@ export const EXTERNAL_PLUGIN_INTAKE_LABELS = Object.freeze({
     color: "D93F0B",
     description: "Submission has quality-gate findings that submitter must fix before maintainer review",
   },
+  "intake-manually-overridden": {
+    color: "BFD4F2",
+    description: "Submission was manually moved to review without passing automated intake gates",
+  },
   approved: {
     color: "1D76DB",
     description: "Submission was approved by a maintainer",
@@ -43,6 +47,7 @@ const EXTERNAL_PLUGIN_INTAKE_SYNC_LABELS = Object.freeze([
   "awaiting-review",
   "ready-for-review",
   "requires-submitter-fixes",
+  "intake-manually-overridden",
   "rejected",
 ]);
 

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

@@ -35,6 +35,30 @@ function mockGitHub(currentLabels) {
   };
 }
 
+test("a fresh intake evaluation clears the manual override label", async () => {
+  const github = mockGitHub([
+    "external-plugin",
+    "ready-for-review",
+    "first party",
+    "intake-manually-overridden",
+  ]);
+
+  await applyExternalPluginIntakeEvaluation({
+    github,
+    owner: "github",
+    repo: "awesome-copilot",
+    issueNumber: 1,
+    evaluation: {
+      intakeState: "ready-for-review",
+      reviewSignals: { repository: { ownershipLabel: "first party" } },
+      commentMarker: "<!-- external-plugin-intake -->",
+      commentBody: "Intake result",
+    },
+  });
+
+  assert.deepEqual(github.labels, new Set(["external-plugin", "ready-for-review", "first party"]));
+});
+
 for (const state of ["ready-for-review", "requires-submitter-fixes", "awaiting-review", "rejected"]) {
   for (const ownershipLabel of ["first party", "third party", null, undefined]) {
     test(`intake syncs ${ownershipLabel ?? "no ownership label"} in ${state} state`, async () => {

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

@@ -63,6 +63,36 @@ test("first-party submissions are not fast-tracked in other pending intake state
   assert.equal(effectiveColumn(item), "unreviewed");
 });
 
+test("manually overridden first-party submissions are never fast-tracked", async (t) => {
+  const store = await createStore(t);
+  const overriddenIssue = issue(1, ["first party", "intake-manually-overridden"]);
+  await store.syncIssues([overriddenIssue]);
+
+  const item = store.getItem(1);
+  assert.equal(item.review, null);
+  assert.equal(item.reviewStatus, "unreviewed");
+  assert.equal(effectiveColumn(item), "unreviewed");
+
+  await store.syncIssues([issue(1, ["first party", "intake-manually-overridden"])]);
+  assert.equal(store.getItem(1).review, null);
+
+  const validationPassed = await store.syncIssues([issue(1, ["first party"])]);
+  assert.deepEqual(validationPassed.fastTracked, [1]);
+  assert.equal(store.getItem(1).review.fastTracked, true);
+});
+
+test("a manual override clears an existing synthetic first-party fast-track", async (t) => {
+  const store = await createStore(t);
+  await store.syncIssues([issue(1, ["first party"])]);
+
+  const result = await store.syncIssues([issue(1, ["first party", "intake-manually-overridden"])]);
+
+  assert.deepEqual(result.fastTracked, []);
+  assert.equal(store.getItem(1).review, null);
+  assert.equal(store.getItem(1).reviewStatus, "unreviewed");
+  assert.equal(effectiveColumn(store.getItem(1)), "unreviewed");
+});
+
 test("approval requires ready status and no pending intake label", () => {
   assert.equal(isReadyForApproval(["external-plugin", "ready-for-review"]), true);
   assert.equal(isReadyForApproval(["external-plugin", "ready-for-review", "awaiting-review"]), false);

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

@@ -0,0 +1,98 @@
+import assert from "node:assert/strict";
+import { readFile } from "node:fs/promises";
+import vm from "node:vm";
+import { test } from "node:test";
+
+const appSource = await readFile(
+    new URL("../.github/extensions/external-plugin-review-board/public/app.js", import.meta.url),
+    "utf8",
+);
+
+function button() {
+    return {
+        disabled: false,
+        textContent: "",
+        classList: { remove() {} },
+    };
+}
+
+function createContext(item, { bulkBusy = false } = {}) {
+    const ui = {
+        drawerKicker: {},
+        drawerTitle: {},
+        drawerMeta: {},
+        drawerLink: {},
+        drawerMove: { replaceChildren() {} },
+        drawerRereview: {},
+        tabs: [],
+        drawerFooter: {},
+        approve: button(),
+        reject: button(),
+    };
+    const context = vm.createContext({
+        board: { columns: [] },
+        bulkBusy,
+        clearTimeout() {},
+        details: new Map(),
+        findItem: () => item,
+        h: (...args) => args,
+        hideRereview() {},
+        itemName: () => "Submission",
+        pendingConfirm: { timer: null, kind: null },
+        relativeTime: () => "now",
+        renderTab() {},
+        selected: item.number,
+        ui,
+        updatePreview() {},
+    });
+    return { context, ui };
+}
+
+function evaluateFunction(context, source, signature, nextSignature) {
+    const start = source.indexOf(signature);
+    const end = source.indexOf(nextSignature, start);
+    assert.notEqual(start, -1, `Expected to find ${signature}`);
+    assert.notEqual(end, -1, `Expected to find ${nextSignature}`);
+    vm.runInContext(source.slice(start, end), context);
+}
+
+test("Approve is disabled for submissions that are not review-eligible", () => {
+    const item = { number: 42, author: "contributor", labels: ["awaiting-review"], reviewEligible: false };
+    const { context, ui } = createContext(item);
+
+    evaluateFunction(context, appSource, "function renderDrawer()", "function renderTab(");
+    context.renderDrawer();
+
+    assert.equal(ui.approve.disabled, true);
+    assert.equal(ui.reject.disabled, false);
+
+    evaluateFunction(context, appSource, "function resetConfirm()", "async function decide(");
+    context.resetConfirm();
+
+    assert.equal(ui.approve.disabled, true);
+    assert.equal(ui.reject.disabled, false);
+});
+
+test("Approve retains its existing eligible, pending-decision, and bulk-busy states", () => {
+    const eligible = { number: 42, author: "contributor", labels: [], reviewEligible: true };
+
+    const available = createContext(eligible);
+    evaluateFunction(available.context, appSource, "function renderDrawer()", "function renderTab(");
+    available.context.renderDrawer();
+    assert.equal(available.ui.approve.disabled, false);
+
+    const pending = createContext({ ...eligible, pendingDecision: { kind: "approve" } });
+    evaluateFunction(pending.context, appSource, "function renderDrawer()", "function renderTab(");
+    pending.context.renderDrawer();
+    assert.equal(pending.ui.approve.disabled, true);
+
+    const busy = createContext(eligible, { bulkBusy: true });
+    evaluateFunction(busy.context, appSource, "function renderDrawer()", "function renderTab(");
+    busy.context.renderDrawer();
+    assert.equal(busy.ui.approve.disabled, true);
+
+    const reset = createContext({ ...eligible, pendingDecision: { kind: "approve" } });
+    evaluateFunction(reset.context, appSource, "function resetConfirm()", "async function decide(");
+    reset.context.resetConfirm();
+    assert.equal(reset.ui.approve.disabled, true);
+});