Skip to content

chore: Re-apply the repo-ci-baseline Template #252

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mpaulosky merged 1 commit into from
Oct 10, 2026
Merged

chore: Re-apply the repo-ci-baseline Template #252

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
147 changes: 77 additions & 70 deletions .github/scripts/tests/pr-automerge.test.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@ import { test } from "node:test";
const WORKFLOW = new URL("../../workflows/pr-automerge.yml", import.meta.url);
const OWNER = "octo";
const HEAD = "abc123";
// reapply.sh's branch, the head of a re-Apply PR.
const REAPPLY_BRANCH = "chore/reapply-baseline";
const NEEDS_HUMAN = "sandcastle:needs-human";

// Returns the body of the `script: |` block scalar with its indentation removed.
Expand Down Expand Up @@ -40,7 +38,6 @@ function readyPr(overrides = {}) {
isDraft: false,
isCrossRepository: false,
baseRefName: "main",
headRefName: "feature/x",
headRefOid: HEAD,
mergeable: "MERGEABLE",
mergeStateStatus: "CLEAN",
Expand Down Expand Up @@ -87,17 +84,19 @@ function copilotReviewsOf(...commits) {
// Claude Review's reviews (github-actions[bot] with the marker) of each given
// commit, oldest first, with ids claude-1, claude-2, ... A { oid, merge: true }
// entry is a merge commit; { oid, marked: false } is a github-actions[bot]
// review without the marker; { oid, offDiff: true } has findings outside the diff.
// review without the marker; { oid, offDiff: true } has findings outside the diff;
// { oid, at } was submitted at that time (default: the Nth minute of 2026-01-02).
function claudeReviewsOf(...commits) {
return {
nodes: commits.map((commit, index) => {
const { oid, merge, marked, offDiff } = typeof commit === "string" ? { oid: commit } : commit;
const { oid, merge, marked, offDiff, at } = typeof commit === "string" ? { oid: commit } : commit;
const body = marked === false
? "Some other workflow's review."
: offDiff
? CLAUDE_MARKER + "\n" + OFF_DIFF_MARKER + "\n**Claude Review**"
: CLAUDE_MARKER + "\nNo findings.";
return { id: `claude-${index + 1}`, body, commit: { oid, parents: { totalCount: merge ? 2 : 1 } } };
const submittedAt = at ?? `2026-01-02T00:${String(index).padStart(2, "0")}:00Z`;
return { id: `claude-${index + 1}`, body, submittedAt, commit: { oid, parents: { totalCount: merge ? 2 : 1 } } };
})
};
}
Expand Down Expand Up @@ -195,7 +194,8 @@ async function evaluate(pr, events = [], options = {}) {
poster = POSTER,
workflowRun,
associated = [],
removeLabelError
removeLabelError,
eventsError
} = options;
const rulesRequests = [];
const updates = [];
Expand Down Expand Up @@ -333,6 +333,10 @@ async function evaluate(pr, events = [], options = {}) {
paginate: async (method, params) => {
if (method === rest.issues.listEvents) {
eventRequests.push(params);
// Only the first read fails, so a later one (the hand-back check) still works.
if (eventsError && eventRequests.length === 1) {
throw eventsError;
}
return eventSets[Math.min(eventRequests.length - 1, eventSets.length - 1)];
}
if (method === rest.issues.listComments) {
Expand Down Expand Up @@ -630,26 +634,83 @@ test("leaves review:claude on below the review cap", async () => {
assert.deepEqual(removedLabels, []);
});

test("leaves review:claude on a re-Apply PR, which has no cap", async () => {
test("leaves review:claude on at a cap reached on Copilot's reviews alone", async () => {
const pr = readyPr({ copilotReviews: copilotReviewsOf("one", "two", HEAD), labels: labelled(CLAUDE_LABEL) });
const { removedLabels } = await evaluate(pr);

assert.deepEqual(removedLabels, []);
});

// A label event: review:claude added at the given time.
function claudeLabelled(at) {
return { event: "labeled", label: { name: CLAUDE_LABEL }, created_at: at, actor: { login: OWNER } };
}

test("leaves review:claude on past the cap until Claude has reviewed since it was last added", async () => {
const pr = readyPr({
headRefName: REAPPLY_BRANCH,
copilotReviews: copilotReviewsOf(),
claudeReviews: claudeReviewsOf("one", "two", HEAD),
labels: labelled(CLAUDE_LABEL)
});
const { removedLabels } = await evaluate(pr);
const { removedLabels } = await evaluate(pr, [claudeLabelled("2026-01-03T00:00:00Z")]);
Comment thread
mpaulosky marked this conversation as resolved.

assert.deepEqual(removedLabels, []);
});

test("takes review:claude off past the cap once Claude has reviewed since it was last added", async () => {
const pr = readyPr({
copilotReviews: copilotReviewsOf("one", "two"),
claudeReviews: claudeReviewsOf({ oid: HEAD, at: "2026-01-03T00:05:00Z" }),
labels: labelled(CLAUDE_LABEL)
});
const { removedLabels } = await evaluate(pr, [claudeLabelled("2026-01-01T00:00:00Z"), claudeLabelled("2026-01-03T00:00:00Z")]);

assert.deepEqual(removedLabels.map((params) => params.name), [CLAUDE_LABEL]);
});

test("falls back to any Claude review at the cap when the label events can't be read", async () => {
const pr = readyPr({ copilotReviews: copilotReviewsOf("one", "two"), claudeReviews: claudeReviewsOf(HEAD), labels: labelled(CLAUDE_LABEL) });
const { removedLabels, logs } = await evaluate(pr, [], { eventsError: new Error("Server Error") });

assert.deepEqual(removedLabels.map((params) => params.name), [CLAUDE_LABEL]);
assert.ok(logs.some((line) => line.startsWith("warning: Couldn't read PR #7's label events")), logs.join("\n"));
});

test("keeps review:claude at the cap when the label events can't be read and Claude never reviewed", async () => {
const pr = readyPr({ copilotReviews: copilotReviewsOf("one", "two", HEAD), labels: labelled(CLAUDE_LABEL) });
const { removedLabels } = await evaluate(pr, [], { eventsError: new Error("Server Error") });

assert.deepEqual(removedLabels, []);
});

test("asks for when each Claude review was submitted", async () => {
const { queries } = await evaluate(readyPr());

assert.ok(queries.some((query) => /claudeReviews:[\s\S]*\bsubmittedAt\b/.test(query)), queries.join("\n"));
});

for (const [scenario, overrides] of [
["a fork", { isCrossRepository: true }],
["a PR into another branch", { baseRefName: "release" }],
["a merged PR", { state: "MERGED" }],
["a closed PR", { state: "CLOSED" }]
]) {
test(`leaves review:claude on ${scenario} at the review cap`, async () => {
const pr = readyPr({ copilotReviews: copilotReviewsOf(), claudeReviews: claudeReviewsOf("one", "two", HEAD), labels: labelled(CLAUDE_LABEL), ...overrides });
const { removedLabels } = await evaluate(pr);

assert.deepEqual(removedLabels, []);
});
}

test("removes nothing at the review cap when the PR doesn't carry review:claude", async () => {
const { removedLabels } = await evaluate(readyPr({ copilotReviews: copilotReviewsOf("one", "two", HEAD) }));

assert.deepEqual(removedLabels, []);
});

test("still merges at the cap when taking review:claude off fails, warning about it", async () => {
const pr = readyPr({ copilotReviews: copilotReviewsOf("one", "two", HEAD), labels: labelled(CLAUDE_LABEL) });
const pr = readyPr({ copilotReviews: copilotReviewsOf("one", "two"), claudeReviews: claudeReviewsOf(HEAD), labels: labelled(CLAUDE_LABEL) });
const { merges, logs } = await evaluate(pr, [], { removeLabelError: Object.assign(new Error("Resource not accessible"), { status: 403 }) });

assert.equal(merges.length, 1);
Expand All @@ -662,7 +723,7 @@ for (const [scenario, overrides] of [
["red on CI", { mergeStateStatus: "UNSTABLE" }]
]) {
test(`takes review:claude off a PR at the review cap that's ${scenario}`, async () => {
const pr = readyPr({ copilotReviews: copilotReviewsOf("one", "two", "three"), labels: labelled(CLAUDE_LABEL), ...overrides });
const pr = readyPr({ copilotReviews: copilotReviewsOf("one", "two"), claudeReviews: claudeReviewsOf("three"), labels: labelled(CLAUDE_LABEL), ...overrides });
const { merges, removedLabels } = await evaluate(pr);

assert.deepEqual(merges, []);
Expand All @@ -671,7 +732,7 @@ for (const [scenario, overrides] of [
}

test("says nothing at the cap when review:claude is already gone (404)", async () => {
const pr = readyPr({ copilotReviews: copilotReviewsOf("one", "two", HEAD), labels: labelled(CLAUDE_LABEL) });
const pr = readyPr({ copilotReviews: copilotReviewsOf("one", "two"), claudeReviews: claudeReviewsOf(HEAD), labels: labelled(CLAUDE_LABEL) });
const { merges, logs } = await evaluate(pr, [], { removeLabelError: Object.assign(new Error("Label does not exist"), { status: 404 }) });

assert.equal(merges.length, 1);
Expand Down Expand Up @@ -833,68 +894,14 @@ test("ignores off-diff findings on an older head at the cap", async () => {
assert.equal(merges.length, 1);
});

// A re-Apply PR has no review cap: its rounds come from re-Apply commits, not
// from chasing comments, and it changes the gate itself.
test("still waits on an unresolved Copilot thread past the cap on a re-Apply PR", async () => {
const pr = readyPr({
headRefName: REAPPLY_BRANCH,
copilotReviews: copilotReviewsOf("one", "two", HEAD),
reviewThreads: threadsBy(COPILOT)
});
const { merges, logs } = await evaluate(pr);

assert.deepEqual(merges, []);
assert.ok(!logs.some((line) => line.includes("Review cap")), logs.join("\n"));
});

test("still waits on an unresolved Claude thread past the cap on a re-Apply PR", async () => {
const pr = readyPr({
headRefName: REAPPLY_BRANCH,
copilotReviews: copilotReviewsOf("one", "two"),
claudeReviews: claudeReviewsOf(HEAD),
reviewThreads: threadsBy({ login: ACTIONS, review: "claude-1" })
});
const { merges, logs } = await evaluate(pr);

assert.deepEqual(merges, []);
assert.ok(!logs.some((line) => line.includes("Review cap")), logs.join("\n"));
});

test("still waits for a review of the head past the cap on a re-Apply PR", async () => {
const pr = readyPr({ headRefName: REAPPLY_BRANCH, copilotReviews: copilotReviewsOf("one", "two", "three") });
const { merges } = await evaluate(pr);

assert.deepEqual(merges, []);
});

test("still waits on off-diff findings past the cap on a re-Apply PR", async () => {
const pr = readyPr({
headRefName: REAPPLY_BRANCH,
copilotReviews: copilotReviewsOf("one", "two"),
claudeReviews: claudeReviewsOf({ oid: HEAD, offDiff: true })
});
const { merges } = await evaluate(pr);

assert.deepEqual(merges, []);
});

test("merges a re-Apply PR past the cap once its head is reviewed and its threads are resolved", async () => {
const pr = readyPr({
headRefName: REAPPLY_BRANCH,
copilotReviews: copilotReviewsOf("one", "two", HEAD),
reviewThreads: threadsBy({ login: COPILOT, resolved: true })
});
// A re-Apply PR is capped like any other (#174).
test("merges a re-Apply PR past the cap without a review of the head, like any other", async () => {
const pr = readyPr({ headRefName: "chore/reapply-baseline", copilotReviews: copilotReviewsOf("one", "two", "three") });
const { merges } = await evaluate(pr);

assert.equal(merges.length, 1);
});

test("asks for the head branch's name", async () => {
const { queries } = await evaluate(readyPr());

assert.ok(queries.some((query) => /\bheadRefName\b/.test(query)), queries.join("\n"));
});

// ── Merges from main onto a reviewed commit ─────────────────────────────────

// Asserts the run waited for a review of head, with no API error on the way.
Expand Down
80 changes: 54 additions & 26 deletions .github/workflows/pr-automerge.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,12 @@ name: PR Auto-Merge
# and Claude's included, and so do Claude's findings outside the diff on its
# review covering the head: the cap only stops the wait for a review that may never come
# (#146). So a review that arrives after a cap merge comes too late: its
# findings land on a merged PR. A re-Apply PR (chore/reapply-baseline) has no
# cap: its rounds come from re-Apply commits, and it changes this gate itself.
# At the cap the run takes review:claude off the PR (#170): Claude Review
# reviews every push while it's on, and each review can open threads that
# hold the merge, so the PR would go on chasing them past the cap.
# findings land on a merged PR. A re-Apply PR (chore/reapply-baseline) is
# capped like any other (#174).
# At the cap the run takes review:claude off the PR (#170), once Claude has
# reviewed it since the label was last added: Claude Review reviews every
# push while it's on, and each review can open threads that hold the merge,
# so the PR would go on chasing them past the cap.
#
# A merge of main into a reviewed commit needs no new review: the head counts
# as reviewed when it is a clean merge of main into a commit a reviewer
Expand Down Expand Up @@ -132,16 +133,11 @@ jobs:
const CLAUDE_LABEL = "review:claude";

// Review rounds, Copilot's and Claude's together, after which the
// merge stops waiting for a review of the head (except on a
// re-Apply PR). Their threads still hold it.
// merge stops waiting for a review of the head. Their threads
// still hold it.
// Named for Copilot, the first reviewer: github-settings.sh reads
// the cap by this name.
const COPILOT_REVIEW_CAP = 3;
// The head branch of a re-Apply PR, as the repo-ci-baseline
// skill's reapply.sh names it. Such a PR has no cap: its rounds
// come from the re-Apply commits added to it, not from chasing
// comments, and it changes this gate itself.
const REAPPLY_BRANCH = "chore/reapply-baseline";
// Claude Review posts as github-actions[bot]; the marker tells its
// reviews apart from anything else posted under that login.
// Accepted limit (repo-ci-baseline's ADR 0004): the marker proves the
Expand Down Expand Up @@ -184,7 +180,6 @@ jobs:
isDraft
isCrossRepository
baseRefName
headRefName
headRefOid
mergeable
mergeStateStatus
Expand Down Expand Up @@ -245,6 +240,7 @@ jobs:
nodes {
id
body
submittedAt
commit {
oid
parents { totalCount }
Expand Down Expand Up @@ -321,8 +317,14 @@ jobs:
return true;
}

// An open PR from this repository into main: the PRs this workflow
// manages, drafts included.
function isOpenSameRepoPrIntoMain(pr) {
return Boolean(pr) && pr.state === "OPEN" && !pr.isCrossRepository && pr.baseRefName === "main";
}

function isOpenSameRepoPr(pullNumber, pr) {
if (!pr || pr.state !== "OPEN" || pr.isDraft || pr.isCrossRepository || pr.baseRefName !== "main") {
if (!isOpenSameRepoPrIntoMain(pr) || pr.isDraft) {
core.info(`Skipping PR #${pullNumber}: not an open, non-draft, same-repo PR into main.`);
return false;
}
Expand Down Expand Up @@ -667,8 +669,7 @@ jobs:

// The PR's Claude reviews, every commit either reviewer reviewed,
// and whether the two have reviewed COPILOT_REVIEW_CAP different
// non-merge commits between them. A re-Apply PR's reviews and
// threads hold the merge however many rounds it takes.
// non-merge commits between them.
function reviewState(pr) {
const claudeReviews = (pr.claudeReviews?.nodes ?? []).filter((review) => {
return review.body?.startsWith(CLAUDE_MARKER);
Expand All @@ -680,10 +681,41 @@ jobs:
const reviewRounds = new Set(
reviewed.filter((commit) => commit.parents?.totalCount === 1).map((commit) => commit.oid)
);
const capReached = pr.headRefName !== REAPPLY_BRANCH && reviewRounds.size >= COPILOT_REVIEW_CAP;
const capReached = reviewRounds.size >= COPILOT_REVIEW_CAP;
return { claudeReviews, reviewedCommits, capReached };
}

// Whether Claude has reviewed the PR since review:claude was last
// added. A cap reached on Copilot's reviews alone, or a label added
// again past the cap, keeps the label until Claude's review of it
// lands: claude-review.yml decides from the labeled event's payload,
// so a review already started runs to the end even after the label
// comes off, but taking it off before Claude ever reviewed would
// leave the person who added it without the review they asked for.
// Label events are read with GITHUB_TOKEN. When they can't be read,
// any Claude review on the PR counts, so a read that keeps failing
// can't keep the label on once Claude has reviewed. A Claude Review run
// that posts nothing (a failure, a timeout, a usage limit) leaves
// the label on until one posts; a run that posts nothing holds no
// merge, so that costs only the runs.
async function claudeReviewedSinceLabel(pullNumber, claudeReviews) {
let events;
try {
events = await eventsClient.paginate(eventsClient.rest.issues.listEvents, {
owner,
repo,
issue_number: pullNumber,
per_page: 100
});
} catch (error) {
core.warning(`Couldn't read PR #${pullNumber}'s label events, so any Claude review counts: ${error.message}`);
return claudeReviews.length > 0;
}
const added = events.findLast((event) => event.event === "labeled" && event.label?.name === CLAUDE_LABEL);
const since = added ? Date.parse(added.created_at) : -Infinity;
return claudeReviews.some((review) => Date.parse(review.submittedAt) > since);
}

// Past the cap, Claude Review would still review every push while
// review:claude is on, and its threads hold the merge, so the PR
// would keep chasing them. Taking the label off stops that; threads
Expand All @@ -710,14 +742,11 @@ jobs:
// Before any check that can end this run: a draft, a handed-back
// PR or a red one is still reviewed on every push while the
// label is on.
if (
pr?.state === "OPEN" &&
!pr.isCrossRepository &&
pr.baseRefName === "main" &&
pr.labels.nodes.some((label) => label.name === CLAUDE_LABEL) &&
reviewState(pr).capReached
) {
await removeClaudeLabel(pullNumber);
if (isOpenSameRepoPrIntoMain(pr) && pr.labels.nodes.some((label) => label.name === CLAUDE_LABEL)) {
const { claudeReviews, capReached } = reviewState(pr);
if (capReached && (await claudeReviewedSinceLabel(pullNumber, claudeReviews))) {
await removeClaudeLabel(pullNumber);
}
}
if (!isOpenSameRepoPr(pullNumber, pr) || !(await isHandBackReadyToMerge(pullNumber, pr))) {
return;
Expand Down Expand Up @@ -755,7 +784,6 @@ jobs:
// the commit a chain of clean merges from main sits on (see
// reviewCovering), until the reviewers have reviewed
// COPILOT_REVIEW_CAP different non-merge commits between them.
// A re-Apply PR never reaches the cap.
const { claudeReviews, reviewedCommits, capReached } = reviewState(pr);
// Past the cap a head with no covering review merges anyway, but
// a covering review's off-diff findings still hold it below.
Expand Down
Loading
Loading