From 36667634becd0687f203ee7d7b2e22a600339554 Mon Sep 17 00:00:00 2001 From: Aaron Powell Date: Tue, 13 Oct 2026 10:45:29 +1100 Subject: [PATCH 1/7] feat(external-plugins): label submissions by repository ownership Classify org-owned repositories as first party (Microsoft, GitHub, Azure, dotnet, MicrosoftDocs, microsoftgraph; case-insensitive) or third party. User-owned repositories receive no ownership label. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 821f4796-505e-4fb8-96cf-0b5bbf80a636 --- .github/workflows/setup-labels.yml | 8 +++ CONTRIBUTING.md | 3 + eng/external-plugin-intake-state.mjs | 27 ++++++++- eng/external-plugin-intake-state.test.mjs | 70 +++++++++++++++++++++++ eng/external-plugin-intake.mjs | 15 ++++- eng/external-plugin-intake.test.mjs | 62 +++++++++++++++++++- 6 files changed, 180 insertions(+), 5 deletions(-) create mode 100644 eng/external-plugin-intake-state.test.mjs diff --git a/.github/workflows/setup-labels.yml b/.github/workflows/setup-labels.yml index 803165f48..a415b1691 100644 --- a/.github/workflows/setup-labels.yml +++ b/.github/workflows/setup-labels.yml @@ -52,6 +52,14 @@ jobs: color: '1D76DB', description: 'External plugin submission includes a canvas extension' }, + 'first party': { + color: '1D76DB', + description: 'External plugin repository is owned by a Microsoft or GitHub first-party organization' + }, + 'third party': { + color: '5319E7', + description: 'External plugin repository is owned by another organization' + }, 'hooks': { color: 'C2E0C6', description: 'PR touches hooks' diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 95e2ac5fd..505e6993d 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -253,6 +253,7 @@ The public-submission policy builds on those rules and also requires `license` p 1. **Open an issue** using the external plugin issue form. Automation applies the `external-plugin` and `awaiting-review` labels. 2. **Automated intake validation** checks that the required fields are present and correctly formatted for a GitHub-hosted plugin. Invalid submissions are labeled `requires-submitter-fixes` with a comment explaining what must be fixed before maintainer review. + Intake also checks the repository owner's type using the GitHub API. Organization-owned repositories receive `first party` for the `Microsoft`, `GitHub`, `Azure`, `dotnet`, `MicrosoftDocs`, or `microsoftgraph` organizations (case-insensitive), or `third party` for any other organization. User-owned repositories receive neither label. Ownership labels are informational only and are updated on each intake pass. 3. **Automated quality gates** run after metadata validation: - `vally lint` against the submitted plugin path/ref/sha - install smoke test via Copilot CLI against an ephemeral marketplace entry generated from the submission @@ -289,6 +290,8 @@ Maintainers are responsible for confirming that the submission: ##### Review cadence and label semantics - `external-plugin`: applied to every public external plugin submission and retained on approved issues so scheduled review automation can find them later +- `first party`: the submitted repository is owned by a first-party organization (`Microsoft`, `GitHub`, `Azure`, `dotnet`, `MicrosoftDocs`, or `microsoftgraph`) +- `third party`: the submitted repository is owned by another organization; user-owned repositories have neither ownership label - `awaiting-review`: initial intake state before automation finishes validating the issue - `ready-for-review`: the issue passed automated intake checks and is waiting on a maintainer decision - `requires-submitter-fixes`: automated intake found metadata or quality-gate issues; submitter updates are required before human review diff --git a/eng/external-plugin-intake-state.mjs b/eng/external-plugin-intake-state.mjs index b73c72169..926399f5c 100644 --- a/eng/external-plugin-intake-state.mjs +++ b/eng/external-plugin-intake-state.mjs @@ -7,6 +7,14 @@ export const EXTERNAL_PLUGIN_INTAKE_LABELS = Object.freeze({ color: "1D76DB", description: "External plugin submission includes a canvas extension", }, + "first party": { + color: "1D76DB", + description: "External plugin repository is owned by a Microsoft or GitHub first-party organization", + }, + "third party": { + color: "5319E7", + description: "External plugin repository is owned by another organization", + }, "awaiting-review": { color: "FBCA04", description: "Submission is waiting for automated intake validation", @@ -53,7 +61,14 @@ async function removeLabel({ github, owner, repo, issueNumber, name }) { } } -export async function syncExternalPluginIntakeLabels({ github, owner, repo, issueNumber, desiredLabels }) { +export async function syncExternalPluginIntakeLabels({ + github, + owner, + repo, + issueNumber, + desiredLabels, + syncOwnershipLabels = false, +}) { const currentLabels = await github.paginate(github.rest.issues.listLabelsOnIssue, { owner, repo, @@ -63,7 +78,10 @@ export async function syncExternalPluginIntakeLabels({ github, owner, repo, issu const currentManagedLabels = currentLabels .map((label) => label.name) - .filter((name) => EXTERNAL_PLUGIN_INTAKE_SYNC_LABELS.includes(name)); + .filter((name) => + EXTERNAL_PLUGIN_INTAKE_SYNC_LABELS.includes(name) || + (syncOwnershipLabels && ["first party", "third party"].includes(name)) + ); const labelsToAdd = [...desiredLabels].filter((name) => !currentManagedLabels.includes(name)); const labelsToRemove = currentManagedLabels.filter((name) => !desiredLabels.has(name)); @@ -137,6 +155,10 @@ export async function applyExternalPluginIntakeEvaluation({ if (evaluation.isCanvasPlugin) { desiredLabels.add("external-plugin-canvas"); } + const ownershipLabel = evaluation.reviewSignals?.repository?.ownershipLabel; + if (ownershipLabel === "first party" || ownershipLabel === "third party") { + desiredLabels.add(ownershipLabel); + } await syncExternalPluginIntakeLabels({ github, @@ -144,6 +166,7 @@ export async function applyExternalPluginIntakeEvaluation({ repo, issueNumber, desiredLabels, + syncOwnershipLabels: true, }); await upsertExternalPluginIntakeComment({ diff --git a/eng/external-plugin-intake-state.test.mjs b/eng/external-plugin-intake-state.test.mjs new file mode 100644 index 000000000..c17a0eebf --- /dev/null +++ b/eng/external-plugin-intake-state.test.mjs @@ -0,0 +1,70 @@ +import assert from "node:assert/strict"; +import { test } from "node:test"; +import { + applyExternalPluginIntakeEvaluation, + syncExternalPluginIntakeLabels, +} from "./external-plugin-intake-state.mjs"; + +function mockGitHub(currentLabels) { + const labels = new Set(currentLabels); + return { + labels, + async paginate() { + return [...labels].map((name) => ({ name })); + }, + rest: { + issues: { + listLabelsOnIssue() {}, + async addLabels({ labels: addedLabels }) { + for (const name of addedLabels) labels.add(name); + }, + async removeLabel({ name }) { + labels.delete(name); + }, + async listComments() { + return { data: [] }; + }, + async createComment() {}, + }, + }, + }; +} + +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 () => { + const github = mockGitHub(["external-plugin", "first party", "third party", "unrelated"]); + const { desiredLabels } = await applyExternalPluginIntakeEvaluation({ + github, + owner: "github", + repo: "awesome-copilot", + issueNumber: 1, + evaluation: { + intakeState: state, + isCanvasPlugin: true, + reviewSignals: { repository: { ownershipLabel } }, + commentMarker: "", + commentBody: "Intake result", + }, + }); + + const expected = ["external-plugin", state, "external-plugin-canvas"]; + if (ownershipLabel) expected.push(ownershipLabel); + assert.deepEqual(desiredLabels, new Set(expected)); + assert.deepEqual(github.labels, new Set([...expected, "unrelated"])); + }); + } +} + +test("state-only label sync preserves ownership labels for maintainer overrides and PR checks", async () => { + const github = mockGitHub(["external-plugin", "requires-submitter-fixes", "first party"]); + await syncExternalPluginIntakeLabels({ + github, + owner: "github", + repo: "awesome-copilot", + issueNumber: 1, + desiredLabels: new Set(["external-plugin", "ready-for-review"]), + }); + + assert.deepEqual(github.labels, new Set(["external-plugin", "ready-for-review", "first party"])); +}); diff --git a/eng/external-plugin-intake.mjs b/eng/external-plugin-intake.mjs index f6bf57430..abede5938 100644 --- a/eng/external-plugin-intake.mjs +++ b/eng/external-plugin-intake.mjs @@ -75,6 +75,14 @@ const MARKETING_SIGNAL_PATTERNS = Object.freeze([ ["trial", /\bfree\s+trial\b|\bstart\s+your\s+trial\b|\bget\s+started\s+free\b/i], ["checkout", /\bstripe\b|\bcheckout\b|\bsubscribe\s+now\b|\bbuy\s+now\b/i], ]); +const FIRST_PARTY_ORGANIZATIONS = new Set([ + "microsoft", + "github", + "azure", + "dotnet", + "microsoftdocs", + "microsoftgraph", +]); const EXTERNAL_PLUGIN_ROOT_MANIFEST_PATHS = Object.freeze([ ".github/plugin/plugin.json", ".plugin/plugin.json", @@ -532,6 +540,11 @@ function buildRepositorySignals(repository) { return { status: "pass", + ownershipLabel: repository.owner?.type === "Organization" + ? FIRST_PARTY_ORGANIZATIONS.has(repository.owner.login?.toLowerCase()) + ? "first party" + : "third party" + : null, age_days: ageDays, stars: repository.stargazers_count, watchers: repository.subscribers_count, @@ -1346,7 +1359,7 @@ export async function evaluateExternalPluginIssue({ issue, token, runId, owner, } } - if (parsed.plugin?.source?.repo && (parsed.plugin?.source?.ref || parsed.plugin?.source?.sha)) { + if (parsed.plugin?.source?.repo) { repositorySignals = await validateRemoteRepository(parsed.plugin.source.repo, parsed.plugin.source, errors, warnings, token); } const homepageSignals = await inspectHomepage(parsed.plugin?.homepage); diff --git a/eng/external-plugin-intake.test.mjs b/eng/external-plugin-intake.test.mjs index 66f3ee4af..754de360d 100644 --- a/eng/external-plugin-intake.test.mjs +++ b/eng/external-plugin-intake.test.mjs @@ -567,11 +567,12 @@ test("pinned homepage dispatcher connects to the validated address and preserves } }); -function installMockFetch() { +function installMockFetch({ repositoryOwner = { type: "User", login: "octo" }, repositoryStatus = 200 } = {}) { global.fetch = async (url) => { const requestUrl = String(url); if (requestUrl === `https://api.github.com/repos/${INTAKE_REPO}`) { return jsonResponse({ + owner: repositoryOwner, private: false, archived: false, created_at: new Date().toISOString(), @@ -579,7 +580,7 @@ function installMockFetch() { subscribers_count: 3, forks_count: 0, open_issues_count: 4, - }); + }, { status: repositoryStatus }); } if ( @@ -602,6 +603,63 @@ function installMockFetch() { }; } +for (const [type, login, expectedLabel] of [ + ["Organization", "github", "first party"], + ["Organization", "Microsoft", "first party"], + ["Organization", "MICROSOFT", "first party"], + ["Organization", "GitHub", "first party"], + ["Organization", "Azure", "first party"], + ["Organization", "azure", "first party"], + ["Organization", "dotnet", "first party"], + ["Organization", "DotNet", "first party"], + ["Organization", "MicrosoftDocs", "first party"], + ["Organization", "microsoftdocs", "first party"], + ["Organization", "microsoftgraph", "first party"], + ["Organization", "MicrosoftGraph", "first party"], + ["Organization", "vendor", "third party"], + ["Organization", "github-vendor", "third party"], + ["User", "microsoft", null], + ["User", "octo", null], +]) { + test(`evaluateExternalPluginIssue classifies ${type} owner ${login}`, async () => { + installMockFetch({ repositoryOwner: { type, login } }); + const result = await evaluateExternalPluginIssue({ + issue: { body: buildIssueBody({ sha: PROVIDED_SHA }) }, + }); + + assert.equal(result.valid, true); + assert.equal(result.reviewSignals.repository.ownershipLabel, expectedLabel); + const mergedResult = applyQualityGateResult(result, { overall_status: "pass" }); + assert.equal(mergedResult.reviewSignals.repository.ownershipLabel, expectedLabel); + }); +} + +for (const status of [404, 503]) { + test(`evaluateExternalPluginIssue does not classify unavailable repository metadata (${status})`, async () => { + installMockFetch({ repositoryStatus: status }); + const result = await evaluateExternalPluginIssue({ + issue: { body: buildIssueBody({ sha: PROVIDED_SHA }) }, + }); + + assert.equal(result.reviewSignals.repository.ownershipLabel, undefined); + if (status === 404) { + assert.match(result.errors.join("\n"), /was not found/); + } else { + assert.match(result.warnings.join("\n"), /could not verify GitHub repository/); + } + }); +} + +test("evaluateExternalPluginIssue classifies ownership even when the submission has no release locator", async () => { + installMockFetch({ repositoryOwner: { type: "Organization", login: "github" } }); + const result = await evaluateExternalPluginIssue({ + issue: { body: buildIssueBody({}) }, + }); + + assert.equal(result.valid, false); + assert.equal(result.reviewSignals.repository.ownershipLabel, "first party"); +}); + test("evaluateExternalPluginIssue fails when ref and sha resolve to different commits", async () => { installMockFetch(); const issue = { body: buildIssueBody({ ref: "v1.2.3", sha: PROVIDED_SHA }) }; From b653c23244e2fe081d180a6c0488cd31cda0a024 Mon Sep 17 00:00:00 2001 From: Aaron Powell Date: Tue, 13 Oct 2026 10:51:42 +1100 Subject: [PATCH 2/7] feat(external-plugins): fast-track first-party reviews and note org ownership Auto-accept first-party cards on the review board with /approve suggested, skipping AI review unless the maintainer opts the card back into Unreviewed. Show ownership and fast-track pills on cards. Add a "Repository owner" reviewer-signal row to intake comments for organization-owned repositories; user-owned repositories get no row. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 821f4796-505e-4fb8-96cf-0b5bbf80a636 --- .../external-plugin-review-board/README.md | 5 ++ .../lib/state.mjs | 32 +++++++- .../public/app.js | 10 ++- .../public/styles.css | 10 +++ .../review-guidance.md | 11 +++ eng/external-plugin-intake.mjs | 7 ++ eng/external-plugin-intake.test.mjs | 12 +++ ...xternal-plugin-review-board-state.test.mjs | 73 +++++++++++++++++++ 8 files changed, 158 insertions(+), 2 deletions(-) create mode 100644 eng/external-plugin-review-board-state.test.mjs diff --git a/.github/extensions/external-plugin-review-board/README.md b/.github/extensions/external-plugin-review-board/README.md index 478324ec8..08917fb56 100644 --- a/.github/extensions/external-plugin-review-board/README.md +++ b/.github/extensions/external-plugin-review-board/README.md @@ -18,6 +18,11 @@ Open it by asking Copilot to "open the plugin review board" (canvas id `external back and it's recorded on the card along with your guidance. - **Refresh** fetches open `external-plugin` + `ready-for-review` issues, adds new ones, and removes any that are closed or no longer ready for review. +- **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. + 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. - **Quick actions** post `/approve` (with an optional note) or `/reject ` straight to the issue after a confirm click. The reject reason is prefilled from the AI's suggested comment. diff --git a/.github/extensions/external-plugin-review-board/lib/state.mjs b/.github/extensions/external-plugin-review-board/lib/state.mjs index f42a99521..86d47c17e 100644 --- a/.github/extensions/external-plugin-review-board/lib/state.mjs +++ b/.github/extensions/external-plugin-review-board/lib/state.mjs @@ -15,6 +15,33 @@ 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. +function applyFirstPartyFastTrack(item) { + if ( + !item.labels?.includes(FIRST_PARTY_LABEL) || + item.fastTrackDismissed || + item.decision || + item.review || + item.manualColumn || + item.reviewStatus === "queued" + ) { + return false; + } + item.review = { + recommendation: "accept", + fastTracked: true, + rationale: + "First-party submission (repository owned by a Microsoft/GitHub organization) that passed automated intake validation; fast-tracked to approval.", + suggestedComment: "/approve", + reviewedAt: new Date().toISOString(), + }; + item.reviewStatus = "reviewed"; + return true; +} function newQueueId() { return randomBytes(6).toString("base64url"); @@ -101,6 +128,7 @@ export class BoardStore { const incoming = new Map(issues.map((issue) => [String(issue.number), issue])); const added = []; const removed = []; + const fastTracked = []; for (const [key, issue] of incoming) { const existing = this.state.items[key]; if (existing) { @@ -123,6 +151,7 @@ export class BoardStore { }; added.push(issue.number); } + if (applyFirstPartyFastTrack(this.state.items[key])) fastTracked.push(issue.number); } for (const key of Object.keys(this.state.items)) { if (!incoming.has(key)) { @@ -132,7 +161,7 @@ export class BoardStore { } this.state.lastRefreshedAt = new Date().toISOString(); await this.commit(); - return { added, removed, total: incoming.size }; + return { added, removed, fastTracked, total: incoming.size }; } pendingReview(numbers) { @@ -218,6 +247,7 @@ export class BoardStore { const item = this.getItem(number); if (item.decision) throw new Error(`#${number} has already been actioned.`); if (column === "unreviewed") { + if (item.review?.fastTracked || item.labels?.includes(FIRST_PARTY_LABEL)) item.fastTrackDismissed = true; item.review = null; item.reviewStatus = "unreviewed"; item.manualColumn = null; diff --git a/.github/extensions/external-plugin-review-board/public/app.js b/.github/extensions/external-plugin-review-board/public/app.js index 22bcb320f..9150d4e71 100644 --- a/.github/extensions/external-plugin-review-board/public/app.js +++ b/.github/extensions/external-plugin-review-board/public/app.js @@ -115,8 +115,15 @@ function signalPills(item) { pills.push(h("span", { class: `pill pill-${level.toLowerCase()}`, title: "AGT contributor risk" }, `AGT ${level}`)); } else if (label === "external-plugin-canvas") { pills.push(h("span", { class: "pill" }, "canvas")); + } else if (label === "first party") { + pills.push(h("span", { class: "pill pill-first-party", title: "Repository owned by a Microsoft/GitHub organization" }, "First party")); + } else if (label === "third party") { + pills.push(h("span", { class: "pill pill-third-party", title: "Repository owned by a third-party organization" }, "Third party")); } } + if (item.review?.fastTracked && !item.decision) { + pills.push(h("span", { class: "pill pill-approve", title: "First-party submission fast-tracked to approval" }, "Fast-tracked")); + } if (item.decision) { pills.push(h("span", { class: `pill pill-${item.decision.kind}` }, item.decision.kind === "approve" ? "Approved" : "Rejected")); } else if (item.pendingDecision) { @@ -391,7 +398,7 @@ function renderReview(item) { return wrapper; } const rows = [ - ["Recommendation", RECOMMENDATION_LABELS[review.recommendation] + (item.manualColumn ? ` (you moved it to ${RECOMMENDATION_LABELS[item.manualColumn]})` : "")], + ["Recommendation", RECOMMENDATION_LABELS[review.recommendation] + (review.fastTracked ? " (first-party fast-track, no AI review)" : "") + (item.manualColumn ? ` (you moved it to ${RECOMMENDATION_LABELS[item.manualColumn]})` : "")], ["Plugin", review.pluginName], ["Repository", review.repository ? h("a", { href: `https://github.com/${review.repository}`, target: "_blank", rel: "noopener noreferrer" }, review.repository) : null], ["Repo facts", review.repoFacts], @@ -546,6 +553,7 @@ ui.refresh.addEventListener("click", () => const parts = [`${result.total} open`]; if (result.added.length) parts.push(`${result.added.length} new`); if (result.removed.length) parts.push(`${result.removed.length} removed`); + if (result.fastTracked?.length) parts.push(`${result.fastTracked.length} first-party fast-tracked`); toast(`Refreshed: ${parts.join(", ")}`); if (selected != null && activeTab !== "review") loadDetail(selected, { force: true }); } catch (error) { diff --git a/.github/extensions/external-plugin-review-board/public/styles.css b/.github/extensions/external-plugin-review-board/public/styles.css index a29dd4890..f36afadb2 100644 --- a/.github/extensions/external-plugin-review-board/public/styles.css +++ b/.github/extensions/external-plugin-review-board/public/styles.css @@ -376,6 +376,16 @@ h2 { background: var(--red-muted); color: var(--red); } +.pill-first-party { + border-color: transparent; + background: var(--green-muted); + color: var(--green); +} +.pill-third-party { + border-color: transparent; + background: var(--purple-muted); + color: var(--purple); +} .pill-queued { border-color: transparent; background: var(--purple-muted); diff --git a/.github/extensions/external-plugin-review-board/review-guidance.md b/.github/extensions/external-plugin-review-board/review-guidance.md index 36697fa8b..e90c60820 100644 --- a/.github/extensions/external-plugin-review-board/review-guidance.md +++ b/.github/extensions/external-plugin-review-board/review-guidance.md @@ -69,3 +69,14 @@ and read the `/approve` / `/reject` comments on the most recent ones. - `probably-reject` — likely reject, but something specific deserves a quick maintainer look. - `needs-review` — genuine judgement call. - `accept` — fits the approval pattern. + +## Ownership labels + +Intake labels submissions by who owns the plugin repository: + +- `first party` — owned by a Microsoft/GitHub organization (Microsoft, GitHub, Azure, dotnet, MicrosoftDocs, + microsoftgraph). The board fast-tracks these straight to `accept` without an AI review. If one is sent for + review anyway, recommend `accept` unless there is a hard failure (broken/missing plugin contents, + security concern, or something that would have failed intake validation). +- `third party` — owned by another organization. This is a signal only; apply the normal decision pattern. +- No ownership label — owned by a user account (or ownership couldn't be determined). diff --git a/eng/external-plugin-intake.mjs b/eng/external-plugin-intake.mjs index abede5938..b1418c4eb 100644 --- a/eng/external-plugin-intake.mjs +++ b/eng/external-plugin-intake.mjs @@ -1221,6 +1221,13 @@ function buildReviewSignalsCommentSection(reviewSignals) { "|---|---|", ]; if (repository?.status === "pass") { + const ownershipNote = { + "first party": "Organization-owned (first party: Microsoft/GitHub organization)", + "third party": "Organization-owned (third party)", + }[repository.ownershipLabel]; + if (ownershipNote) { + rows.push(`| Repository owner | ${ownershipNote} |`); + } rows.push( `| Repository age | ${repository.age_days === undefined ? "unknown" : `${repository.age_days} day(s)`} |`, `| Repository activity | ${repository.signals?.length ? repository.signals.join("; ") : "no configured signal"} |`, diff --git a/eng/external-plugin-intake.test.mjs b/eng/external-plugin-intake.test.mjs index 754de360d..0f8e06809 100644 --- a/eng/external-plugin-intake.test.mjs +++ b/eng/external-plugin-intake.test.mjs @@ -631,6 +631,17 @@ for (const [type, login, expectedLabel] of [ assert.equal(result.reviewSignals.repository.ownershipLabel, expectedLabel); const mergedResult = applyQualityGateResult(result, { overall_status: "pass" }); assert.equal(mergedResult.reviewSignals.repository.ownershipLabel, expectedLabel); + const expectedNote = { + "first party": "| Repository owner | Organization-owned (first party: Microsoft/GitHub organization) |", + "third party": "| Repository owner | Organization-owned (third party) |", + }[expectedLabel]; + for (const body of [result.commentBody, mergedResult.commentBody]) { + if (expectedNote) { + assert.ok(body.includes(expectedNote), `expected ownership note in:\n${body}`); + } else { + assert.doesNotMatch(body, /Repository owner/); + } + } }); } @@ -658,6 +669,7 @@ test("evaluateExternalPluginIssue classifies ownership even when the submission assert.equal(result.valid, false); assert.equal(result.reviewSignals.repository.ownershipLabel, "first party"); + assert.match(result.commentBody, /\| Repository owner \| Organization-owned \(first party/); }); test("evaluateExternalPluginIssue fails when ref and sha resolve to different commits", async () => { diff --git a/eng/external-plugin-review-board-state.test.mjs b/eng/external-plugin-review-board-state.test.mjs new file mode 100644 index 000000000..c3f126e22 --- /dev/null +++ b/eng/external-plugin-review-board-state.test.mjs @@ -0,0 +1,73 @@ +import assert from "node:assert/strict"; +import { mkdtemp, rm } from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { test } from "node:test"; + +import { BoardStore, effectiveColumn } from "../.github/extensions/external-plugin-review-board/lib/state.mjs"; + +function issue(number, labels = []) { + return { + number, + title: `[External Plugin]: Example ${number}`, + author: "contributor", + url: `https://github.com/github/awesome-copilot/issues/${number}`, + labels: ["external-plugin", "ready-for-review", ...labels], + }; +} + +async function createStore(t) { + const dir = await mkdtemp(path.join(os.tmpdir(), "epr-board-")); + t.after(() => rm(dir, { recursive: true, force: true })); + return new BoardStore(path.join(dir, "board.json"), "github/awesome-copilot"); +} + +test("first-party submissions are fast-tracked to accept on refresh", async (t) => { + const store = await createStore(t); + const result = await store.syncIssues([issue(1, ["first party"]), issue(2, ["third party"]), issue(3)]); + + assert.deepEqual(result.fastTracked, [1]); + const first = store.getItem(1); + assert.equal(effectiveColumn(first), "accept"); + assert.equal(first.reviewStatus, "reviewed"); + assert.equal(first.review.fastTracked, true); + assert.equal(first.review.suggestedComment, "/approve"); + assert.equal(effectiveColumn(store.getItem(2)), "unreviewed"); + assert.equal(effectiveColumn(store.getItem(3)), "unreviewed"); + assert.deepEqual(store.pendingReview().map((item) => item.number).sort(), [2, 3]); +}); + +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)]); + const result = await store.syncIssues([issue(1, ["first party"])]); + + assert.deepEqual(result.fastTracked, [1]); + assert.equal(effectiveColumn(store.getItem(1)), "accept"); +}); + +test("fast-track does not override an existing review or manual bucket", async (t) => { + const store = await createStore(t); + await store.syncIssues([issue(1), issue(2)]); + store.getItem(1).review = { recommendation: "needs-review", reviewedAt: new Date().toISOString() }; + store.getItem(1).reviewStatus = "reviewed"; + await store.moveItem(2, "probably-reject"); + + const result = await store.syncIssues([issue(1, ["first party"]), issue(2, ["first party"])]); + + assert.deepEqual(result.fastTracked, []); + assert.equal(effectiveColumn(store.getItem(1)), "needs-review"); + assert.equal(effectiveColumn(store.getItem(2)), "probably-reject"); +}); + +test("moving a fast-tracked card to unreviewed opts it out of fast-track", async (t) => { + const store = await createStore(t); + await store.syncIssues([issue(1, ["first party"])]); + await store.moveItem(1, "unreviewed"); + + const result = await store.syncIssues([issue(1, ["first party"])]); + + assert.deepEqual(result.fastTracked, []); + assert.equal(effectiveColumn(store.getItem(1)), "unreviewed"); + assert.deepEqual(store.pendingReview().map((item) => item.number), [1]); +}); From f4088ee18594a9c639ad5ce7e1eec8c58e6cf2d7 Mon Sep 17 00:00:00 2001 From: Aaron Powell Date: Tue, 13 Oct 2026 11:37:54 +1100 Subject: [PATCH 3/7] fix(external-plugins): address review feedback Revoke synthetic first-party fast-tracks when ownership changes while preserving human decisions, manual buckets, and queued re-reviews. Apply normal review guidance when explicitly opted out of fast-track, and document that first-party routing does not bypass validation or maintainer approval. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 821f4796-505e-4fb8-96cf-0b5bbf80a636 --- .../lib/state.mjs | 12 +++++- .../review-guidance.md | 8 ++-- CONTRIBUTING.md | 2 +- ...xternal-plugin-review-board-state.test.mjs | 41 +++++++++++++++++++ 4 files changed, 58 insertions(+), 5 deletions(-) diff --git a/.github/extensions/external-plugin-review-board/lib/state.mjs b/.github/extensions/external-plugin-review-board/lib/state.mjs index 86d47c17e..39fe6f5dd 100644 --- a/.github/extensions/external-plugin-review-board/lib/state.mjs +++ b/.github/extensions/external-plugin-review-board/lib/state.mjs @@ -43,6 +43,14 @@ function applyFirstPartyFastTrack(item) { return true; } +function clearFirstPartyFastTrack(item) { + if (item.labels?.includes(FIRST_PARTY_LABEL) || !item.review?.fastTracked || item.reviewStatus === "queued") { + return; + } + item.review = null; + if (!item.decision && !item.manualColumn) item.reviewStatus = "unreviewed"; +} + function newQueueId() { return randomBytes(6).toString("base64url"); } @@ -151,7 +159,9 @@ export class BoardStore { }; added.push(issue.number); } - if (applyFirstPartyFastTrack(this.state.items[key])) fastTracked.push(issue.number); + const item = this.state.items[key]; + if (applyFirstPartyFastTrack(item)) fastTracked.push(issue.number); + else clearFirstPartyFastTrack(item); } for (const key of Object.keys(this.state.items)) { if (!incoming.has(key)) { diff --git a/.github/extensions/external-plugin-review-board/review-guidance.md b/.github/extensions/external-plugin-review-board/review-guidance.md index e90c60820..a961f3b16 100644 --- a/.github/extensions/external-plugin-review-board/review-guidance.md +++ b/.github/extensions/external-plugin-review-board/review-guidance.md @@ -75,8 +75,10 @@ and read the `/approve` / `/reject` comments on the most recent ones. Intake labels submissions by who owns the plugin repository: - `first party` — owned by a Microsoft/GitHub organization (Microsoft, GitHub, Azure, dotnet, MicrosoftDocs, - microsoftgraph). The board fast-tracks these straight to `accept` without an AI review. If one is sent for - review anyway, recommend `accept` unless there is a hard failure (broken/missing plugin contents, - security concern, or something that would have failed intake validation). + microsoftgraph). The board fast-tracks these straight to `accept` without an AI review. If a maintainer + explicitly moves one to `Unreviewed`, that opts it out of the fast-track: review it using the same procedure, + maintainer decision pattern, and recommendation buckets as any other submission. Assess its repo fit, + usefulness, duplication, and content quality on their merits; first-party ownership is not a reason to + recommend `accept`. - `third party` — owned by another organization. This is a signal only; apply the normal decision pattern. - No ownership label — owned by a user account (or ownership couldn't be determined). diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 505e6993d..2056c09c1 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -253,7 +253,7 @@ The public-submission policy builds on those rules and also requires `license` p 1. **Open an issue** using the external plugin issue form. Automation applies the `external-plugin` and `awaiting-review` labels. 2. **Automated intake validation** checks that the required fields are present and correctly formatted for a GitHub-hosted plugin. Invalid submissions are labeled `requires-submitter-fixes` with a comment explaining what must be fixed before maintainer review. - Intake also checks the repository owner's type using the GitHub API. Organization-owned repositories receive `first party` for the `Microsoft`, `GitHub`, `Azure`, `dotnet`, `MicrosoftDocs`, or `microsoftgraph` organizations (case-insensitive), or `third party` for any other organization. User-owned repositories receive neither label. Ownership labels are informational only and are updated on each intake pass. + Intake also checks the repository owner's type using the GitHub API. Organization-owned repositories receive `first party` for the `Microsoft`, `GitHub`, `Azure`, `dotnet`, `MicrosoftDocs`, or `microsoftgraph` organizations (case-insensitive), or `third party` for any other organization. User-owned repositories receive neither label. On the triage board, `first party` skips AI review and creates an `Accept` recommendation with `/approve`; this is only a triage recommendation. Submissions must still pass automated validation and quality gates, and a human maintainer must review and approve them before listing. Ownership labels are updated on each intake pass. 3. **Automated quality gates** run after metadata validation: - `vally lint` against the submitted plugin path/ref/sha - install smoke test via Copilot CLI against an ephemeral marketplace entry generated from the submission diff --git a/eng/external-plugin-review-board-state.test.mjs b/eng/external-plugin-review-board-state.test.mjs index c3f126e22..b52ae9537 100644 --- a/eng/external-plugin-review-board-state.test.mjs +++ b/eng/external-plugin-review-board-state.test.mjs @@ -46,6 +46,47 @@ test("fast-track applies when an existing card gains the first party label", asy assert.equal(effectiveColumn(store.getItem(1)), "accept"); }); +test("removing the first-party label clears the synthetic fast-track", async (t) => { + const store = await createStore(t); + await store.syncIssues([issue(1, ["first party"])]); + + await store.syncIssues([issue(1, ["third party"])]); + + const item = store.getItem(1); + assert.equal(item.review, null); + assert.equal(item.reviewStatus, "unreviewed"); + assert.equal(effectiveColumn(item), "unreviewed"); + assert.deepEqual(store.pendingReview().map((pending) => pending.number), [1]); +}); + +test("removing the first-party label preserves decisions, manual buckets, and queued re-reviews", async (t) => { + const store = await createStore(t); + await store.syncIssues([ + issue(1, ["first party"]), + issue(2, ["first party"]), + issue(3, ["first party"]), + ]); + await store.recordDecision(1, { kind: "approve", at: new Date().toISOString() }); + await store.moveItem(2, "probably-reject"); + await store.markQueued([3], { rereviewGuidance: "Re-check the submission." }); + const queued = store.getItem(3); + const queueId = queued.queueId; + const queuedReview = queued.review; + + await store.syncIssues([issue(1, ["third party"]), issue(2, ["third party"]), issue(3, ["third party"])]); + + assert.equal(store.getItem(1).review, null); + assert.equal(store.getItem(1).decision.kind, "approve"); + assert.equal(effectiveColumn(store.getItem(1)), "actioned"); + assert.equal(store.getItem(2).review, null); + assert.equal(store.getItem(2).manualColumn, "probably-reject"); + assert.equal(effectiveColumn(store.getItem(2)), "probably-reject"); + assert.equal(store.getItem(3).reviewStatus, "queued"); + assert.equal(store.getItem(3).queueId, queueId); + assert.equal(store.getItem(3).rereviewGuidance, "Re-check the submission."); + assert.deepEqual(store.getItem(3).review, queuedReview); +}); + test("fast-track does not override an existing review or manual bucket", async (t) => { const store = await createStore(t); await store.syncIssues([issue(1), issue(2)]); From a48dab596130e77635a6a603713cd285869059eb Mon Sep 17 00:00:00 2001 From: Aaron Powell Date: Tue, 13 Oct 2026 11:58:49 +1100 Subject: [PATCH 4/7] 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 --- .../extension.mjs | 7 +++- .../lib/state.mjs | 16 ++++++-- .../external-plugin-command-router.yml | 3 +- .github/workflows/external-plugin-intake.yml | 16 ++++++++ eng/external-plugin-intake-state.mjs | 13 +++++++ eng/external-plugin-intake-state.test.mjs | 34 +++++++++++++++++ ...xternal-plugin-review-board-state.test.mjs | 37 +++++++++++++++++++ 7 files changed, 119 insertions(+), 7 deletions(-) diff --git a/.github/extensions/external-plugin-review-board/extension.mjs b/.github/extensions/external-plugin-review-board/extension.mjs index c65797e17..7e1332c96 100644 --- a/.github/extensions/external-plugin-review-board/extension.mjs +++ b/.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); diff --git a/.github/extensions/external-plugin-review-board/lib/state.mjs b/.github/extensions/external-plugin-review-board/lib/state.mjs index 39fe6f5dd..8000e9d51 100644 --- a/.github/extensions/external-plugin-review-board/lib/state.mjs +++ b/.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; diff --git a/.github/workflows/external-plugin-command-router.yml b/.github/workflows/external-plugin-command-router.yml index 8bce714fe..dd7a4bc38 100644 --- a/.github/workflows/external-plugin-command-router.yml +++ b/.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) { diff --git a/.github/workflows/external-plugin-intake.yml b/.github/workflows/external-plugin-intake.yml index a80661537..b5b4da006 100644 --- a/.github/workflows/external-plugin-intake.yml +++ b/.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: diff --git a/eng/external-plugin-intake-state.mjs b/eng/external-plugin-intake-state.mjs index 926399f5c..ef665f93b 100644 --- a/eng/external-plugin-intake-state.mjs +++ b/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, diff --git a/eng/external-plugin-intake-state.test.mjs b/eng/external-plugin-intake-state.test.mjs index c17a0eebf..7db6e38ae 100644 --- a/eng/external-plugin-intake-state.test.mjs +++ b/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); +}); diff --git a/eng/external-plugin-review-board-state.test.mjs b/eng/external-plugin-review-board-state.test.mjs index b52ae9537..ece1e2fc8 100644 --- a/eng/external-plugin-review-board-state.test.mjs +++ b/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)]); From 666a521cd50a19725786f1fa367b4f5027ec5fee Mon Sep 17 00:00:00 2001 From: Aaron Powell Date: Tue, 13 Oct 2026 13:01:19 +1100 Subject: [PATCH 5/7] fix(external-plugins): preserve cards during intake Include submissions awaiting review or submitter fixes in the review board feed so refreshes preserve their existing state. Keep fast-track and approval gated on a ready intake state, and add regression coverage for card persistence across pending transitions. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 821f4796-505e-4fb8-96cf-0b5bbf80a636 --- .../external-plugin-review-board/README.md | 10 +-- .../extension.mjs | 11 ++- .../lib/github.mjs | 29 ++++---- .../lib/github.test.mjs | 21 ++++++ .../lib/state.mjs | 9 ++- eng/board-state-storage.test.mjs | 2 +- eng/external-plugin-intake-state.mjs | 8 +-- eng/external-plugin-intake-state.test.mjs | 2 +- ...xternal-plugin-review-board-state.test.mjs | 71 ++++++++++++++++++- 9 files changed, 131 insertions(+), 32 deletions(-) create mode 100644 .github/extensions/external-plugin-review-board/lib/github.test.mjs diff --git a/.github/extensions/external-plugin-review-board/README.md b/.github/extensions/external-plugin-review-board/README.md index 08917fb56..3ecb51cde 100644 --- a/.github/extensions/external-plugin-review-board/README.md +++ b/.github/extensions/external-plugin-review-board/README.md @@ -1,7 +1,7 @@ # External Plugin Review Board -A maintainer-only Copilot canvas extension for triaging `external-plugin` submissions that are -`ready-for-review` in github/awesome-copilot. +A maintainer-only Copilot canvas extension for triaging active `external-plugin` submissions in +github/awesome-copilot. Open it by asking Copilot to "open the plugin review board" (canvas id `external-plugin-review-board`). @@ -16,8 +16,10 @@ Open it by asking Copilot to "open the plugin review board" (canvas id `external - **Re-review** (in the details panel) takes optional guidance on what to focus on, then has the agent spin up a separate sub-session to re-assess that one submission. The sub-session reports its result back and it's recorded on the card along with your guidance. -- **Refresh** fetches open `external-plugin` + `ready-for-review` issues, adds new ones, and removes any - that are closed or no longer ready for review. +- **Refresh** fetches open `external-plugin` issues in `ready-for-review`, `awaiting-review`, or + `requires-submitter-fixes` states. Pending intake cards stay on the board so their AI reviews, + manual buckets, queued re-reviews, and fast-track dismissal state are preserved. Approval and + 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. diff --git a/.github/extensions/external-plugin-review-board/extension.mjs b/.github/extensions/external-plugin-review-board/extension.mjs index 7e1332c96..e9f292542 100644 --- a/.github/extensions/external-plugin-review-board/extension.mjs +++ b/.github/extensions/external-plugin-review-board/extension.mjs @@ -6,7 +6,7 @@ import path from "node:path"; import { fileURLToPath } from "node:url"; import { joinSession, createCanvas, CanvasError } from "@github/copilot-sdk/extension"; -import { BoardStore, RECOMMENDATIONS } from "./lib/state.mjs"; +import { BoardStore, isReadyForApproval, RECOMMENDATIONS } from "./lib/state.mjs"; import * as gh from "./lib/github.mjs"; import { buildReviewPrompt, buildRereviewPrompt, REVIEW_FIELDS_SCHEMA } from "./lib/review-prompt.mjs"; import { startBoardServer, expectedError } from "./lib/server.mjs"; @@ -49,7 +49,7 @@ async function refresh() { refreshInFlight ??= (async () => { try { await resolvePendingDecisions(); - const issues = await gh.listReadyIssues(store.state.repo); + const issues = await gh.listBoardIssues(store.state.repo); detailCache.clear(); return await store.syncIssues(issues); } finally { @@ -134,10 +134,7 @@ 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") || status.labels.includes("awaiting-review")) - ) { + if (kind === "approve" && !isReadyForApproval(status.labels)) { throw new Error(`#${number} is pending intake or is no longer ready-for-review, so /approve would be ignored.`); } @@ -241,7 +238,7 @@ session = await joinSession({ }, { name: "refresh", - description: "Fetch open ready-for-review external plugin issues; adds new ones and removes closed ones.", + description: "Fetch open external plugin submissions in active intake states; adds new ones and removes inactive ones.", handler: wrap(() => refresh()), }, { diff --git a/.github/extensions/external-plugin-review-board/lib/github.mjs b/.github/extensions/external-plugin-review-board/lib/github.mjs index 128a2c847..bcca92e95 100644 --- a/.github/extensions/external-plugin-review-board/lib/github.mjs +++ b/.github/extensions/external-plugin-review-board/lib/github.mjs @@ -29,7 +29,22 @@ function runGh(args, { input } = {}) { }); } -export async function listReadyIssues(repo) { +export function mapBoardIssues(issues) { + const boardStates = new Set(["ready-for-review", "awaiting-review", "requires-submitter-fixes"]); + return issues + .filter((issue) => (issue.labels ?? []).some((label) => boardStates.has(label.name))) + .map((issue) => ({ + number: issue.number, + title: issue.title, + author: issue.author?.login ?? "unknown", + createdAt: issue.createdAt, + updatedAt: issue.updatedAt, + url: issue.url, + labels: (issue.labels ?? []).map((label) => label.name), + })); +} + +export async function listBoardIssues(repo) { const out = await runGh([ "issue", "list", @@ -37,8 +52,6 @@ export async function listReadyIssues(repo) { repo, "--label", "external-plugin", - "--label", - "ready-for-review", "--state", "open", "--limit", @@ -46,15 +59,7 @@ export async function listReadyIssues(repo) { "--json", "number,title,author,createdAt,updatedAt,labels,url", ]); - return JSON.parse(out).map((issue) => ({ - number: issue.number, - title: issue.title, - author: issue.author?.login ?? "unknown", - createdAt: issue.createdAt, - updatedAt: issue.updatedAt, - url: issue.url, - labels: (issue.labels ?? []).map((label) => label.name), - })); + return mapBoardIssues(JSON.parse(out)); } const FULL_JSON = ["-H", "Accept: application/vnd.github.full+json"]; diff --git a/.github/extensions/external-plugin-review-board/lib/github.test.mjs b/.github/extensions/external-plugin-review-board/lib/github.test.mjs new file mode 100644 index 000000000..5ac6dd2c2 --- /dev/null +++ b/.github/extensions/external-plugin-review-board/lib/github.test.mjs @@ -0,0 +1,21 @@ +import assert from "node:assert/strict"; +import { test } from "node:test"; + +import { mapBoardIssues } from "./github.mjs"; + +test("board feed includes submissions in each active intake state", () => { + const issues = ["ready-for-review", "awaiting-review", "requires-submitter-fixes", "rejected"].map( + (state, index) => ({ + number: index + 1, + title: `Submission ${index + 1}`, + author: { login: "contributor" }, + labels: [{ name: "external-plugin" }, { name: state }], + url: `https://github.com/github/awesome-copilot/issues/${index + 1}`, + }), + ); + + assert.deepEqual( + mapBoardIssues(issues).map((issue) => issue.number), + [1, 2, 3], + ); +}); diff --git a/.github/extensions/external-plugin-review-board/lib/state.mjs b/.github/extensions/external-plugin-review-board/lib/state.mjs index 8000e9d51..ca5ce7253 100644 --- a/.github/extensions/external-plugin-review-board/lib/state.mjs +++ b/.github/extensions/external-plugin-review-board/lib/state.mjs @@ -16,6 +16,11 @@ export const COLUMNS = [ const HISTORY_LIMIT = 200; export const FIRST_PARTY_LABEL = "first party"; +const PENDING_INTAKE_LABELS = ["awaiting-review", "requires-submitter-fixes"]; + +export function isReadyForApproval(labels) { + return labels.includes("ready-for-review") && !labels.some((label) => PENDING_INTAKE_LABELS.includes(label)); +} // 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 @@ -24,7 +29,7 @@ function applyFirstPartyFastTrack(item) { if ( !item.labels?.includes(FIRST_PARTY_LABEL) || !item.labels?.includes("ready-for-review") || - item.labels?.includes("awaiting-review") || + item.labels?.some((label) => PENDING_INTAKE_LABELS.includes(label)) || item.fastTrackDismissed || item.decision || item.review || @@ -49,7 +54,7 @@ function clearFirstPartyFastTrack(item) { if ( (item.labels?.includes(FIRST_PARTY_LABEL) && item.labels?.includes("ready-for-review") && - !item.labels?.includes("awaiting-review")) || + !item.labels?.some((label) => PENDING_INTAKE_LABELS.includes(label))) || !item.review?.fastTracked || item.reviewStatus === "queued" ) { diff --git a/eng/board-state-storage.test.mjs b/eng/board-state-storage.test.mjs index 3db0f568b..8edef8c43 100644 --- a/eng/board-state-storage.test.mjs +++ b/eng/board-state-storage.test.mjs @@ -80,7 +80,7 @@ async function loadCanvas({ id, extensionDir, workspacePath, triggerDuringJoin = } else if (specifier === "./lib/github.mjs") { exports = { listOpenPullRequests: async () => [item], - listReadyIssues: async () => [item], + listBoardIssues: async () => [item], }; } else { exports = await import(specifier.startsWith(".") diff --git a/eng/external-plugin-intake-state.mjs b/eng/external-plugin-intake-state.mjs index ef665f93b..7feac27bb 100644 --- a/eng/external-plugin-intake-state.mjs +++ b/eng/external-plugin-intake-state.mjs @@ -101,16 +101,16 @@ 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"], }); + + for (const name of ["ready-for-review", "requires-submitter-fixes", "rejected"]) { + await removeLabel({ github, owner, repo, issueNumber, name }); + } } export async function upsertExternalPluginIntakeComment({ diff --git a/eng/external-plugin-intake-state.test.mjs b/eng/external-plugin-intake-state.test.mjs index 7db6e38ae..6abbe3eaa 100644 --- a/eng/external-plugin-intake-state.test.mjs +++ b/eng/external-plugin-intake-state.test.mjs @@ -100,5 +100,5 @@ test("marking intake pending removes stale state labels but preserves ownership, const pendingAddition = github.operations.findIndex( ([action, labels]) => action === "add" && labels.includes("awaiting-review"), ); - assert.ok(readyRemoval >= 0 && pendingAddition > readyRemoval); + assert.ok(pendingAddition >= 0 && readyRemoval > pendingAddition); }); diff --git a/eng/external-plugin-review-board-state.test.mjs b/eng/external-plugin-review-board-state.test.mjs index ece1e2fc8..015f533dd 100644 --- a/eng/external-plugin-review-board-state.test.mjs +++ b/eng/external-plugin-review-board-state.test.mjs @@ -4,7 +4,11 @@ import os from "node:os"; import path from "node:path"; import { test } from "node:test"; -import { BoardStore, effectiveColumn } from "../.github/extensions/external-plugin-review-board/lib/state.mjs"; +import { + BoardStore, + effectiveColumn, + isReadyForApproval, +} from "../.github/extensions/external-plugin-review-board/lib/state.mjs"; function issue(number, labels = []) { return { @@ -48,6 +52,27 @@ test("first-party submissions are not fast-tracked while intake is pending", asy assert.equal(effectiveColumn(item), "unreviewed"); }); +test("first-party submissions are not fast-tracked in other pending intake states", async (t) => { + const store = await createStore(t); + const pendingIssue = issue(1, ["first party", "requires-submitter-fixes"]); + 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("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); + assert.equal( + isReadyForApproval(["external-plugin", "ready-for-review", "requires-submitter-fixes"]), + false, + ); + assert.equal(isReadyForApproval(["external-plugin", "rejected"]), false); +}); + 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"]); @@ -124,6 +149,50 @@ test("removing the first-party label preserves decisions, manual buckets, and qu assert.deepEqual(store.getItem(3).review, queuedReview); }); +test("pending intake refreshes preserve review, manual, queued, and fast-track dismissal state", async (t) => { + const store = await createStore(t); + await store.syncIssues([ + issue(1), + issue(2), + issue(3), + issue(4, ["first party"]), + ]); + + const aiReview = { recommendation: "needs-review", rationale: "Keep this review." }; + store.getItem(1).review = aiReview; + store.getItem(1).reviewStatus = "reviewed"; + await store.moveItem(2, "probably-reject"); + await store.markQueued([3], { rereviewGuidance: "Re-check this submission." }); + await store.moveItem(4, "unreviewed"); + + await store.syncIssues([ + issue(1, ["awaiting-review"]), + issue(2, ["requires-submitter-fixes"]), + issue(3, ["awaiting-review"]), + issue(4, ["first party", "requires-submitter-fixes"]), + ]); + + assert.deepEqual(store.getItem(1).review, aiReview); + assert.equal(store.getItem(2).manualColumn, "probably-reject"); + assert.equal(store.getItem(3).reviewStatus, "queued"); + assert.equal(store.getItem(3).rereviewGuidance, "Re-check this submission."); + assert.equal(store.getItem(4).fastTrackDismissed, true); + + const result = await store.syncIssues([ + issue(1), + issue(2), + issue(3), + issue(4, ["first party"]), + ]); + + assert.deepEqual(result.fastTracked, []); + assert.deepEqual(store.getItem(1).review, aiReview); + assert.equal(store.getItem(2).manualColumn, "probably-reject"); + assert.equal(store.getItem(3).reviewStatus, "queued"); + assert.equal(store.getItem(4).fastTrackDismissed, true); + assert.equal(store.getItem(4).review, null); +}); + test("fast-track does not override an existing review or manual bucket", async (t) => { const store = await createStore(t); await store.syncIssues([issue(1), issue(2)]); From 2b726b5f7c3184ab75d3554da974171404bd108e Mon Sep 17 00:00:00 2001 From: Aaron Powell Date: Tue, 13 Oct 2026 13:55:45 +1100 Subject: [PATCH 6/7] fix(external-plugins): gate reviews on ready intake Use the same intake readiness predicate across approval, AI review, and re-review paths. Keep pending submissions visible but do not count or queue them for review until validation completes; clarify board visibility in the README. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 821f4796-505e-4fb8-96cf-0b5bbf80a636 --- .../external-plugin-review-board/README.md | 2 +- .../extension.mjs | 3 ++ .../lib/state.mjs | 12 +++++- .../public/app.js | 10 ++++- .../external-plugin-command-router.yml | 3 +- ...xternal-plugin-review-board-state.test.mjs | 43 +++++++++++++++++++ 6 files changed, 67 insertions(+), 6 deletions(-) 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"]); From 4796642f484abc8bb3353b0028aadd583306038b Mon Sep 17 00:00:00 2001 From: Aaron Powell Date: Tue, 13 Oct 2026 14:24:47 +1100 Subject: [PATCH 7/7] 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 --- .../lib/state.mjs | 3 + .../public/app.js | 3 +- .../external-plugin-command-router.yml | 18 ++++ .github/workflows/setup-labels.yml | 4 + eng/external-plugin-intake-state.mjs | 5 + eng/external-plugin-intake-state.test.mjs | 24 +++++ ...xternal-plugin-review-board-state.test.mjs | 30 ++++++ eng/external-plugin-review-board-ui.test.mjs | 98 +++++++++++++++++++ 8 files changed, 184 insertions(+), 1 deletion(-) create mode 100644 eng/external-plugin-review-board-ui.test.mjs diff --git a/.github/extensions/external-plugin-review-board/lib/state.mjs b/.github/extensions/external-plugin-review-board/lib/state.mjs index fdd419e22..3d5db0392 100644 --- a/.github/extensions/external-plugin-review-board/lib/state.mjs +++ b/.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" diff --git a/.github/extensions/external-plugin-review-board/public/app.js b/.github/extensions/external-plugin-review-board/public/app.js index c40cafa5a..b28f9d8c0 100644 --- a/.github/extensions/external-plugin-review-board/public/app.js +++ b/.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) { diff --git a/.github/workflows/external-plugin-command-router.yml b/.github/workflows/external-plugin-command-router.yml index 22b42b33a..b40ded157 100644 --- a/.github/workflows/external-plugin-command-router.yml +++ b/.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'] : []) ]) }); diff --git a/.github/workflows/setup-labels.yml b/.github/workflows/setup-labels.yml index a415b1691..d529107b4 100644 --- a/.github/workflows/setup-labels.yml +++ b/.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' diff --git a/eng/external-plugin-intake-state.mjs b/eng/external-plugin-intake-state.mjs index 7feac27bb..5c60e5d5a 100644 --- a/eng/external-plugin-intake-state.mjs +++ b/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", ]); diff --git a/eng/external-plugin-intake-state.test.mjs b/eng/external-plugin-intake-state.test.mjs index 6abbe3eaa..b4bfc7cda 100644 --- a/eng/external-plugin-intake-state.test.mjs +++ b/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: "", + 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 () => { diff --git a/eng/external-plugin-review-board-state.test.mjs b/eng/external-plugin-review-board-state.test.mjs index 8fd68e829..b93a6469e 100644 --- a/eng/external-plugin-review-board-state.test.mjs +++ b/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); diff --git a/eng/external-plugin-review-board-ui.test.mjs b/eng/external-plugin-review-board-ui.test.mjs new file mode 100644 index 000000000..16dfe87d4 --- /dev/null +++ b/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); +});