mirror of
https://github.com/github/awesome-copilot.git
synced 2026-10-13 23:34:25 +00:00
fix(canvas): address review board feedback
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: dc011747-d114-43af-93e9-7f0036f9b4b7
This commit is contained in:
co-authored by
Copilot App
parent
8431cac934
commit
c0fb55cd28
@@ -70,7 +70,7 @@ async function getIssue(number, { force = false } = {}) {
|
||||
|
||||
async function startReview({ numbers, instanceId, send }) {
|
||||
const items = store.pendingReview(numbers);
|
||||
if (!items.length) return { queued: [], message: "Nothing to review — all items already have an AI review." };
|
||||
if (!items.length) return { queued: [], message: "Nothing to review — no eligible unreviewed items remain." };
|
||||
const queued = items.map((item) => item.number);
|
||||
const snapshots = await store.markQueued(queued);
|
||||
const prompt = buildReviewPrompt({
|
||||
|
||||
@@ -136,7 +136,8 @@ 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) =>
|
||||
!item.decision && !item.pendingDecision && item.reviewStatus !== "queued");
|
||||
if (numbers?.length) {
|
||||
const wanted = new Set(numbers.map(Number));
|
||||
return all.filter((item) => wanted.has(item.number));
|
||||
|
||||
@@ -561,6 +561,11 @@ function resetConfirm() {
|
||||
ui.approve.disabled = false;
|
||||
ui.reject.disabled = false;
|
||||
updatePreview();
|
||||
const item = findItem(selected);
|
||||
if (bulkBusy || item?.pendingDecision) {
|
||||
ui.approve.disabled = true;
|
||||
ui.reject.disabled = true;
|
||||
}
|
||||
}
|
||||
|
||||
async function decide(kind) {
|
||||
|
||||
@@ -0,0 +1,50 @@
|
||||
import assert from "node:assert/strict";
|
||||
import { readFile } from "node:fs/promises";
|
||||
import vm from "node:vm";
|
||||
import { test } from "node:test";
|
||||
import { BoardStore } from "../.github/extensions/external-plugin-review-board/lib/state.mjs";
|
||||
|
||||
test("external-plugin-review-board excludes items with pending decisions from AI review", () => {
|
||||
const store = new BoardStore("unused", "github/awesome-copilot");
|
||||
store.state.items = {
|
||||
"1": { number: 1, reviewStatus: "unreviewed" },
|
||||
"2": { number: 2, reviewStatus: "unreviewed", pendingDecision: { kind: "approve" } },
|
||||
"3": { number: 3, reviewStatus: "queued" },
|
||||
"4": { number: 4, reviewStatus: "unreviewed", decision: { kind: "reject" } },
|
||||
};
|
||||
|
||||
assert.deepEqual(store.pendingReview().map(({ number }) => number), [1]);
|
||||
assert.deepEqual(store.pendingReview([1, 2, 3, 4]).map(({ number }) => number), [1]);
|
||||
});
|
||||
|
||||
test("resetConfirm keeps decision buttons disabled while a decision is pending", async () => {
|
||||
const source = await readFile(
|
||||
new URL("../.github/extensions/external-plugin-review-board/public/app.js", import.meta.url),
|
||||
"utf8",
|
||||
);
|
||||
const start = source.indexOf("function resetConfirm()");
|
||||
const end = source.indexOf("async function decide(", start);
|
||||
assert.notEqual(start, -1);
|
||||
assert.notEqual(end, -1);
|
||||
|
||||
const button = () => ({
|
||||
disabled: false,
|
||||
textContent: "",
|
||||
classList: { remove() {} },
|
||||
});
|
||||
const context = vm.createContext({
|
||||
clearTimeout() {},
|
||||
pendingConfirm: { timer: null, kind: "approve" },
|
||||
ui: { approve: button(), reject: button() },
|
||||
bulkBusy: false,
|
||||
selected: 42,
|
||||
findItem: () => ({ pendingDecision: { kind: "approve" } }),
|
||||
updatePreview() {},
|
||||
});
|
||||
vm.runInContext(source.slice(start, end), context);
|
||||
|
||||
context.resetConfirm();
|
||||
|
||||
assert.equal(context.ui.approve.disabled, true);
|
||||
assert.equal(context.ui.reject.disabled, true);
|
||||
});
|
||||
@@ -6,7 +6,7 @@ import vm from "node:vm";
|
||||
import { test } from "node:test";
|
||||
import { BoardStore } from "../.github/extensions/open-pr-board/lib/state.mjs";
|
||||
import { applyDecisionOnGitHub } from "../.github/extensions/open-pr-board/lib/decisions.mjs";
|
||||
import { approveAndEnableAutoMerge } from "../.github/extensions/open-pr-board/lib/github.mjs";
|
||||
import { approveAndEnableAutoMerge, listOpenPullRequests } from "../.github/extensions/open-pr-board/lib/github.mjs";
|
||||
import { REVIEW_FIELDS_SCHEMA } from "../.github/extensions/open-pr-board/lib/review-prompt.mjs";
|
||||
|
||||
const pr = (headRefOid = "first") => ({
|
||||
@@ -59,6 +59,50 @@ test("approval and auto-merge are pinned to the reviewed commit", async () => {
|
||||
assert.deepEqual(calls[2].args, ["pr", "merge", "1", "-R", "example/repo", "--auto", "--merge", "--match-head-commit", headRefOid]);
|
||||
});
|
||||
|
||||
test("open pull requests fetch checks with bounded concurrency, preserve order, and tolerate a failed check request", async () => {
|
||||
const numbers = [41, 7, 23, 4, 99, 12, 68, 51];
|
||||
let active = 0;
|
||||
let peak = 0;
|
||||
const requested = [];
|
||||
const runner = async (args) => {
|
||||
if (args[1] === "list") {
|
||||
return JSON.stringify(numbers.map((number) => ({ number, title: `PR ${number}`, labels: [] })));
|
||||
}
|
||||
if (args[1] === "view") {
|
||||
const number = Number(args[2]);
|
||||
requested.push(number);
|
||||
active++;
|
||||
peak = Math.max(peak, active);
|
||||
try {
|
||||
await new Promise((resolve) => setTimeout(resolve, 10));
|
||||
if (number === 23) throw new Error("check request failed");
|
||||
return JSON.stringify({
|
||||
statusCheckRollup: [{ name: `check-${number}`, status: "COMPLETED", conclusion: "SUCCESS" }],
|
||||
});
|
||||
} finally {
|
||||
active--;
|
||||
}
|
||||
}
|
||||
throw new Error(`Unexpected gh command: ${args.join(" ")}`);
|
||||
};
|
||||
|
||||
const pullRequests = await listOpenPullRequests("example/repo", runner);
|
||||
|
||||
assert.ok(peak <= 6, "check requests should not exceed the six-request limit");
|
||||
assert.ok(peak > 1, "check requests should run concurrently");
|
||||
assert.deepEqual(pullRequests.map(({ number }) => number), numbers, "results should preserve list order");
|
||||
assert.deepEqual(requested.sort((a, b) => a - b), [...numbers].sort((a, b) => a - b), "every PR should request checks");
|
||||
for (const pullRequest of pullRequests) {
|
||||
if (pullRequest.number === 23) {
|
||||
assert.deepEqual(pullRequest.checks, []);
|
||||
assert.equal(pullRequest.checksState, "none");
|
||||
} else {
|
||||
assert.deepEqual(pullRequest.checks.map(({ name }) => name), [`check-${pullRequest.number}`]);
|
||||
assert.equal(pullRequest.checksState, "success");
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
test("approval refuses a missing SHA, changed head, or closed pull request without writing", async () => {
|
||||
let calls = 0;
|
||||
const runner = async () => { calls++; throw new Error("must not run"); };
|
||||
|
||||
Reference in New Issue
Block a user