mirror of
https://github.com/github/awesome-copilot.git
synced 2026-10-12 23:04:14 +00:00
fix(gate): require one approval for high risk and report awaiting review as action_required (#4639)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: f280c771-d797-4eb1-a96f-ada49c076e86
This commit is contained in:
co-authored by
Copilot App
parent
c06f74af0d
commit
f94871f1ce
+17
-1
@@ -1097,6 +1097,22 @@ export async function findImpostorGateChecks(github, { owner, repo, headSha }) {
|
||||
return runs.filter((run) => run.external_id !== GATE_CHECK_EXTERNAL_ID);
|
||||
}
|
||||
|
||||
/**
|
||||
* A PR that is only waiting for a review is not broken: report `action_required`, which
|
||||
* still blocks the required check but reads as "needs a reviewer" rather than a failure.
|
||||
*/
|
||||
export function gateConclusion(evaluation) {
|
||||
if (evaluation.passed) return "success";
|
||||
const { automation, approvals } = evaluation;
|
||||
const onlyAwaitingReview =
|
||||
automation.contributionFailures.length === 0 &&
|
||||
automation.infrastructureFailures.length === 0 &&
|
||||
automation.pending.length === 0 &&
|
||||
(approvals.changesRequestedBy ?? []).length === 0 &&
|
||||
!approvals.satisfied;
|
||||
return onlyAwaitingReview ? "action_required" : "failure";
|
||||
}
|
||||
|
||||
/**
|
||||
* Publish the required `submission-gate` check on the PR head commit. Only the trusted
|
||||
* writer calls this, so the check can't be satisfied by editing a PR-controlled workflow.
|
||||
@@ -1114,7 +1130,7 @@ export async function publishGateCheck(github, { owner, repo, evaluation, detail
|
||||
};
|
||||
const fields = pending
|
||||
? { status: "in_progress", output }
|
||||
: { status: "completed", conclusion: evaluation.passed ? "success" : "failure", completed_at: new Date().toISOString(), output };
|
||||
: { status: "completed", conclusion: gateConclusion(evaluation), completed_at: new Date().toISOString(), output };
|
||||
if (detailsUrl) fields.details_url = detailsUrl;
|
||||
|
||||
const runs = await listGateCheckRuns(github, { owner, repo, headSha: evaluation.headSha });
|
||||
|
||||
@@ -11,6 +11,7 @@ import {
|
||||
classifyRisk,
|
||||
computeState,
|
||||
evaluateApprovals,
|
||||
gateConclusion,
|
||||
evaluateCheck,
|
||||
evaluateSubmission,
|
||||
globToRegExp,
|
||||
@@ -286,17 +287,25 @@ test("medium tier requires a domain reviewer when the pool is staffed", () => {
|
||||
assert.equal(noRouting.satisfied, true);
|
||||
});
|
||||
|
||||
test("high tier requires two approvals including a core maintainer", () => {
|
||||
test("high tier requires one approval from a core maintainer", () => {
|
||||
const permissions = new Map([["alice", "write"], ["bob", "write"], ["corea", "write"], ["admin1", "admin"]]);
|
||||
const files = [file(".github/workflows/x.yml")];
|
||||
const base = { tier: "high", tiers, author: "author", permissions, routing, files };
|
||||
assert.equal(evaluateApprovals({ ...base, reviews: [review("corea", "APPROVED")] }).satisfied, false);
|
||||
assert.equal(evaluateApprovals({ ...base, reviews: [review("corea", "APPROVED")] }).satisfied, true);
|
||||
assert.equal(evaluateApprovals({ ...base, reviews: [review("alice", "APPROVED"), review("bob", "APPROVED")] }).satisfied, false);
|
||||
assert.equal(evaluateApprovals({ ...base, reviews: [review("alice", "APPROVED"), review("corea", "APPROVED")] }).satisfied, true);
|
||||
|
||||
const fallback = { ...base, routing: null };
|
||||
assert.equal(evaluateApprovals({ ...fallback, reviews: [review("alice", "APPROVED"), review("bob", "APPROVED")] }).satisfied, false);
|
||||
assert.equal(evaluateApprovals({ ...fallback, reviews: [review("alice", "APPROVED"), review("admin1", "APPROVED")] }).satisfied, true);
|
||||
assert.equal(evaluateApprovals({ ...fallback, reviews: [review("alice", "APPROVED")] }).satisfied, false);
|
||||
assert.equal(evaluateApprovals({ ...fallback, reviews: [review("admin1", "APPROVED")] }).satisfied, true);
|
||||
});
|
||||
|
||||
test("gateConclusion reports a PR that only awaits review as action_required", () => {
|
||||
const clean = { contributionFailures: [], infrastructureFailures: [], pending: [] };
|
||||
const waiting = { satisfied: false, changesRequestedBy: [] };
|
||||
assert.equal(gateConclusion({ passed: true, automation: clean, approvals: { satisfied: true, changesRequestedBy: [] } }), "success");
|
||||
assert.equal(gateConclusion({ passed: false, automation: clean, approvals: waiting }), "action_required");
|
||||
assert.equal(gateConclusion({ passed: false, automation: clean, approvals: { ...waiting, changesRequestedBy: ["bob"] } }), "failure");
|
||||
assert.equal(gateConclusion({ passed: false, automation: { ...clean, contributionFailures: [{}] }, approvals: waiting }), "failure");
|
||||
});
|
||||
|
||||
// --- checks -------------------------------------------------------------------
|
||||
@@ -916,7 +925,7 @@ test("publishGateCheck corrects copies of the writer's check that claim success
|
||||
await publishGateCheck(github, { owner: "github", repo: "awesome-copilot", evaluation, log: (line) => logs.push(line) });
|
||||
const updates = github.calls.filter((call) => call.name === "checks.update");
|
||||
assert.deepEqual(updates.map((call) => call.params.check_run_id), [11]);
|
||||
assert.equal(updates[0].params.conclusion, "failure");
|
||||
assert.equal(updates[0].params.conclusion, "action_required");
|
||||
assert.ok(logs.some((line) => /Correcting 1/.test(line)));
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user