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