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]); +});