diff --git a/.github/extensions/external-plugin-review-board/README.md b/.github/extensions/external-plugin-review-board/README.md index 3ecb51cde..1346e98f9 100644 --- a/.github/extensions/external-plugin-review-board/README.md +++ b/.github/extensions/external-plugin-review-board/README.md @@ -22,7 +22,7 @@ Open it by asking Copilot to "open the plugin review board" (canvas id `external first-party fast-track are unavailable until intake is ready. - **First-party fast-track**: cards labelled `first party` (repository owned by a Microsoft/GitHub organization) skip AI review and go straight to *Accept* with a suggested `/approve`. Only submissions - that passed automated intake reach the board, so hard validation failures are still held back by intake. + that passed automated intake can use fast-track or be approved; pending submissions remain visible on the board. Fast-track never overrides an existing review or a bucket you chose; drag a card to *Unreviewed* to opt it out and have it reviewed normally. Cards show *First party* / *Third party* pills from the intake labels. - **Details panel** shows the AI review, the rendered issue body, and all comments. diff --git a/.github/extensions/external-plugin-review-board/extension.mjs b/.github/extensions/external-plugin-review-board/extension.mjs index e9f292542..25866459b 100644 --- a/.github/extensions/external-plugin-review-board/extension.mjs +++ b/.github/extensions/external-plugin-review-board/extension.mjs @@ -97,6 +97,9 @@ async function startRereview({ number, guidance, instanceId, send }) { const item = store.getItem(Number(number)); if (item.decision) throw new Error(`#${item.number} was already actioned (${item.decision.kind}).`); if (item.reviewStatus === "queued") throw new Error(`#${item.number} is already queued for review.`); + if (!isReadyForApproval(item.labels ?? [])) { + throw new Error(`#${item.number} is pending intake or is no longer ready-for-review.`); + } const text = String(guidance ?? "").trim().slice(0, MAX_GUIDANCE_LENGTH); const snapshots = await store.markQueued([item.number], { rereviewGuidance: text || null }); const prompt = buildRereviewPrompt({ diff --git a/.github/extensions/external-plugin-review-board/lib/state.mjs b/.github/extensions/external-plugin-review-board/lib/state.mjs index ca5ce7253..9d5477f69 100644 --- a/.github/extensions/external-plugin-review-board/lib/state.mjs +++ b/.github/extensions/external-plugin-review-board/lib/state.mjs @@ -140,7 +140,10 @@ export class BoardStore { repo: this.state.repo, lastRefreshedAt: this.state.lastRefreshedAt, columns: COLUMNS, - items, + items: items.map((item) => ({ + ...item, + reviewEligible: isReadyForApproval(item.labels ?? []), + })), history: this.state.history.slice(0, 50), }; } @@ -188,7 +191,12 @@ export class BoardStore { } pendingReview(numbers) { - const all = Object.values(this.state.items).filter((item) => !item.decision && item.reviewStatus !== "queued"); + const all = Object.values(this.state.items).filter( + (item) => + isReadyForApproval(item.labels ?? []) && + !item.decision && + item.reviewStatus !== "queued", + ); if (numbers?.length) { const wanted = new Set(numbers.map(Number)); return all.filter((item) => wanted.has(item.number)); diff --git a/.github/extensions/external-plugin-review-board/public/app.js b/.github/extensions/external-plugin-review-board/public/app.js index 9150d4e71..01429d1c5 100644 --- a/.github/extensions/external-plugin-review-board/public/app.js +++ b/.github/extensions/external-plugin-review-board/public/app.js @@ -151,7 +151,13 @@ function findItem(number) { function pendingReviewCount() { return board - ? board.items.filter((item) => !item.decision && item.reviewStatus !== "reviewed" && item.reviewStatus !== "queued").length + ? board.items.filter( + (item) => + item.reviewEligible && + !item.decision && + item.reviewStatus !== "reviewed" && + item.reviewStatus !== "queued", + ).length : 0; } @@ -314,7 +320,7 @@ function renderDrawer() { .map((column) => h("option", { value: column.id, selected: column.id === item.column }, column.title)); ui.drawerMove.replaceChildren(...options); ui.drawerMove.disabled = Boolean(item.decision); - ui.drawerRereview.disabled = Boolean(item.decision) || item.reviewStatus === "queued"; + ui.drawerRereview.disabled = !item.reviewEligible || Boolean(item.decision) || item.reviewStatus === "queued"; if (ui.drawerRereview.disabled) hideRereview(); for (const tab of ui.tabs) { diff --git a/.github/workflows/external-plugin-command-router.yml b/.github/workflows/external-plugin-command-router.yml index dd7a4bc38..22b42b33a 100644 --- a/.github/workflows/external-plugin-command-router.yml +++ b/.github/workflows/external-plugin-command-router.yml @@ -47,6 +47,7 @@ jobs: const approval = await import(pathToFileURL(path.join(process.env.GITHUB_WORKSPACE, 'eng', 'external-plugin-approval.mjs')).href); const intake = await import(pathToFileURL(path.join(process.env.GITHUB_WORKSPACE, 'eng', 'external-plugin-intake.mjs')).href); + const reviewBoard = await import(pathToFileURL(path.join(process.env.GITHUB_WORKSPACE, '.github', 'extensions', 'external-plugin-review-board', 'lib', 'state.mjs')).href); const parsedCommand = approval.parseDecisionCommand(context.payload.comment.body); core.setOutput('should-run', 'false'); @@ -86,7 +87,7 @@ jobs: const fallbackName = evaluation.plugin?.name ?? `issue-${context.issue.number}`; const canApprove = labelNames.has('approved') || - (labelNames.has('ready-for-review') && !labelNames.has('awaiting-review')); + reviewBoard.isReadyForApproval([...labelNames]); const canReject = !labelNames.has('approved'); if (parsedCommand.command === 'approve' && !canApprove) { diff --git a/eng/external-plugin-review-board-state.test.mjs b/eng/external-plugin-review-board-state.test.mjs index 015f533dd..8fd68e829 100644 --- a/eng/external-plugin-review-board-state.test.mjs +++ b/eng/external-plugin-review-board-state.test.mjs @@ -73,6 +73,49 @@ test("approval requires ready status and no pending intake label", () => { assert.equal(isReadyForApproval(["external-plugin", "rejected"]), false); }); +test("pending intake cards stay visible but become review-eligible only when ready", () => { + const store = new BoardStore("unused", "github/awesome-copilot"); + const existingReview = { recommendation: "needs-review", rationale: "Keep this review." }; + const queuedReview = { recommendation: "accept", rationale: "Keep the queued card's prior review." }; + store.state.items = { + 1: { number: 1, labels: ["external-plugin", "awaiting-review"], reviewStatus: "unreviewed" }, + 2: { + number: 2, + labels: ["external-plugin", "requires-submitter-fixes"], + reviewStatus: "reviewed", + review: existingReview, + }, + 3: { number: 3, labels: ["external-plugin", "ready-for-review"], reviewStatus: "unreviewed" }, + 4: { + number: 4, + labels: ["external-plugin", "awaiting-review"], + reviewStatus: "queued", + review: queuedReview, + queueId: "queued-review", + }, + }; + + const pendingSnapshot = store.snapshot(); + assert.deepEqual(pendingSnapshot.items.map((item) => item.number), [4, 3, 2, 1]); + assert.deepEqual( + pendingSnapshot.items.filter((item) => item.reviewEligible).map((item) => item.number), + [3], + ); + assert.deepEqual(store.pendingReview().map((item) => item.number), [3]); + assert.deepEqual(store.pendingReview([1, 2, 3, 4]).map((item) => item.number), [3]); + + store.getItem(1).labels = ["external-plugin", "ready-for-review"]; + store.getItem(2).labels = ["external-plugin", "ready-for-review"]; + store.getItem(4).labels = ["external-plugin", "ready-for-review"]; + + assert.deepEqual(store.pendingReview().map((item) => item.number), [1, 3]); + assert.equal(store.getItem(2).review, existingReview); + assert.equal(store.getItem(2).reviewStatus, "reviewed"); + assert.equal(store.getItem(4).review, queuedReview); + assert.equal(store.getItem(4).reviewStatus, "queued"); + assert.equal(store.getItem(4).queueId, "queued-review"); +}); + 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"]);