mirror of
https://github.com/github/awesome-copilot.git
synced 2026-10-12 23:04:14 +00:00
fix(canvas): pin PR approval to the reviewed commit
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8045e7e0-dfd7-425a-9131-21ea5ca1553e
This commit is contained in:
co-authored by
Copilot App
parent
e9262f6c98
commit
d63527fe89
@@ -28,7 +28,9 @@ authenticated `gh` CLI.
|
||||
Use Left/Right arrows or Home/End to switch detail tabs.
|
||||
- **GitHub actions** — submit feedback, request changes, close a PR, or approve it and
|
||||
enable auto-merge with a merge commit. Each action needs two clicks to confirm and
|
||||
is also recorded in local board history.
|
||||
is also recorded in local board history. Approval is pinned to the board's commit
|
||||
SHA, and auto-merge requires that same head commit. If the head changes, refresh
|
||||
and review the new commit before approving again.
|
||||
Only one GitHub action can run per PR at a time.
|
||||
- **Reset board** — after a second click to confirm, clear every local AI review,
|
||||
queued review, manual move, recorded action, and history entry, then reload open PRs.
|
||||
|
||||
@@ -21,7 +21,7 @@ export async function applyDecisionOnGitHub({ store, number, kind, comment, refr
|
||||
|
||||
let result;
|
||||
if (kind === "approve") {
|
||||
result = await github.approveAndEnableAutoMerge(store.repo, number, { body });
|
||||
result = await github.approveAndEnableAutoMerge(store.repo, number, { body, headRefOid: item.headRefOid });
|
||||
} else if (kind === "close") {
|
||||
result = await github.closePullRequest(store.repo, number, { body });
|
||||
} else {
|
||||
|
||||
@@ -66,11 +66,18 @@ export async function closePullRequest(repo, number, { body } = {}, runner = run
|
||||
}
|
||||
}
|
||||
|
||||
export async function approveAndEnableAutoMerge(repo, number, { body } = {}, runner = runGh) {
|
||||
await submitPullRequestReview(repo, number, { kind: "approve", body }, runner);
|
||||
export async function approveAndEnableAutoMerge(repo, number, { body, headRefOid } = {}, runner = runGh) {
|
||||
if (!headRefOid) throw new Error("The board has no commit SHA for this pull request. Refresh before approving.");
|
||||
const current = JSON.parse(await runner(["api", `repos/${repo}/pulls/${number}`]));
|
||||
if (current.state !== "open" || current.head?.sha !== headRefOid) {
|
||||
throw new Error("The pull request changed or is no longer open. Refresh and review it again before approving.");
|
||||
}
|
||||
await runner(["api", `repos/${repo}/pulls/${number}/reviews`, "--method", "POST", "--input", "-"], {
|
||||
input: JSON.stringify({ event: "APPROVE", commit_id: headRefOid, body: String(body ?? "").trim() }),
|
||||
});
|
||||
|
||||
try {
|
||||
await runner(["pr", "merge", String(number), "-R", repo, "--auto", "--merge"]);
|
||||
await runner(["pr", "merge", String(number), "-R", repo, "--auto", "--merge", "--match-head-commit", headRefOid]);
|
||||
} catch (error) {
|
||||
return {
|
||||
status: "approval-submitted",
|
||||
|
||||
@@ -6,6 +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 { REVIEW_FIELDS_SCHEMA } from "../.github/extensions/open-pr-board/lib/review-prompt.mjs";
|
||||
|
||||
const pr = (headRefOid = "first") => ({
|
||||
@@ -27,6 +28,65 @@ function deferred() {
|
||||
return { promise, resolve, reject };
|
||||
}
|
||||
|
||||
test("approval passes the board's commit SHA to the GitHub helper", async (t) => {
|
||||
const store = await createStore(t);
|
||||
let options;
|
||||
await applyDecisionOnGitHub({
|
||||
store, number: 1, kind: "approve", comment: "Reviewed.", refresh: async () => {},
|
||||
github: {
|
||||
approveAndEnableAutoMerge: async (repo, number, input) => {
|
||||
options = input;
|
||||
return { status: "auto-merge-enabled" };
|
||||
},
|
||||
},
|
||||
});
|
||||
assert.deepEqual(options, { body: "Reviewed.", headRefOid: "first" });
|
||||
});
|
||||
|
||||
test("approval and auto-merge are pinned to the reviewed commit", async () => {
|
||||
const headRefOid = "a".repeat(40);
|
||||
const calls = [];
|
||||
const result = await approveAndEnableAutoMerge("example/repo", 1, { body: "Reviewed.", headRefOid },
|
||||
async (args, options) => {
|
||||
calls.push({ args, options });
|
||||
if (calls.length === 1) return JSON.stringify({ state: "open", head: { sha: headRefOid } });
|
||||
if (args[1] === "view") return JSON.stringify({ state: "OPEN", autoMergeRequest: {} });
|
||||
return "";
|
||||
});
|
||||
assert.equal(result.status, "auto-merge-enabled");
|
||||
assert.deepEqual(calls[1].args, ["api", "repos/example/repo/pulls/1/reviews", "--method", "POST", "--input", "-"]);
|
||||
assert.deepEqual(JSON.parse(calls[1].options.input), { event: "APPROVE", commit_id: headRefOid, body: "Reviewed." });
|
||||
assert.deepEqual(calls[2].args, ["pr", "merge", "1", "-R", "example/repo", "--auto", "--merge", "--match-head-commit", headRefOid]);
|
||||
});
|
||||
|
||||
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"); };
|
||||
await assert.rejects(approveAndEnableAutoMerge("example/repo", 1, {}, runner), /no commit SHA/);
|
||||
assert.equal(calls, 0);
|
||||
for (const current of [{ state: "open", head: { sha: "second" } }, { state: "closed", head: { sha: "first" } }]) {
|
||||
calls = 0;
|
||||
await assert.rejects(approveAndEnableAutoMerge("example/repo", 1, { headRefOid: "first" }, async () => {
|
||||
calls++;
|
||||
return JSON.stringify(current);
|
||||
}), /Refresh and review/);
|
||||
assert.equal(calls, 1);
|
||||
}
|
||||
});
|
||||
|
||||
test("a head change after approval reports partial success instead of retrying the merge", async () => {
|
||||
let calls = 0;
|
||||
const result = await approveAndEnableAutoMerge("example/repo", 1, { headRefOid: "first" }, async () => {
|
||||
calls++;
|
||||
if (calls === 1) return JSON.stringify({ state: "open", head: { sha: "first" } });
|
||||
if (calls === 3) throw new Error("Head commit changed");
|
||||
return "";
|
||||
});
|
||||
assert.equal(calls, 3);
|
||||
assert.equal(result.status, "approval-submitted");
|
||||
assert.match(result.warning, /Head commit changed/);
|
||||
});
|
||||
|
||||
test("review schema requires a nonempty queue token and a one-sentence rationale", () => {
|
||||
assert.ok(REVIEW_FIELDS_SCHEMA.required.includes("queueId"));
|
||||
assert.equal(REVIEW_FIELDS_SCHEMA.properties.queueId.minLength, 1);
|
||||
|
||||
Reference in New Issue
Block a user