mirror of
https://github.com/github/awesome-copilot.git
synced 2026-10-14 07:44:43 +00:00
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
This commit is contained in:
co-authored by
Copilot App
parent
666a521cd5
commit
2b726b5f7c
@@ -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.
|
||||
|
||||
@@ -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({
|
||||
|
||||
@@ -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));
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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"]);
|
||||
|
||||
Reference in New Issue
Block a user