Skip to content

Fix maintainer-approval not blocking PRs without approval #4935

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
simonfaltum merged 2 commits into from
Apr 13, 2026
+19 −39
Merged

Fix maintainer-approval not blocking PRs without approval #4935

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
340bb35
Fix maintainer-approval not blocking PRs without approval
simonfaltum Apr 13, 2026
23926d5
Merge branch 'main' into simonfaltum/fix-maintainer-approval-merge-queue
simonfaltum Apr 13, 2026
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
33 changes: 10 additions & 23 deletions .github/workflows/maintainer-approval.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -516,7 +516,10 @@ module.exports = async ({ github, context, core }) => {
core
);

// Set commit status. Approved PRs return early (commit status is sufficient).
// Approved PRs get a success check run and return early.
// Pending PRs intentionally create NO check run or status. The required
// status check "maintainer-approval" stays as "Expected" (yellow dot) in
// the GitHub UI, which blocks the merge until approval is granted.
if (result.allCovered && approverLogins.length > 0) {
core.info("All ownership groups have per-path approval.");
await github.rest.checks.create({
Expand All @@ -531,36 +534,20 @@ module.exports = async ({ github, context, core }) => {

if (result.hasWildcardFiles) {
const fileList = result.wildcardFiles.join(", ");
const msg =
core.info(
`Files need maintainer review: ${fileList}. ` +
`Maintainers: ${maintainers.join(", ")}`;
core.info(msg);
await github.rest.checks.create({
...checkParams,
status: "in_progress",
output: { title: STATUS_CONTEXT, summary: msg },
});
`Maintainers: ${maintainers.join(", ")}`
);
} else if (result.uncovered && result.uncovered.length > 0) {
const groupList = result.uncovered
.map(({ pattern, owners }) => `${pattern} (needs: ${owners.join(", ")})`)
.join("; ");
const msg = `Needs approval: ${groupList}`;
core.info(
`${msg}. Alternatively, any maintainer can approve: ${maintainers.join(", ")}.`
`Needs approval: ${groupList}. ` +
`Alternatively, any maintainer can approve: ${maintainers.join(", ")}.`
);
await github.rest.checks.create({
...checkParams,
status: "in_progress",
output: { title: STATUS_CONTEXT, summary: msg },
});
} else {
const msg = `Waiting for maintainer approval: ${maintainers.join(", ")}`;
core.info(msg);
await github.rest.checks.create({
...checkParams,
status: "in_progress",
output: { title: STATUS_CONTEXT, summary: msg },
});
core.info(`Waiting for maintainer approval: ${maintainers.join(", ")}`);
}

// Score contributors via git history
Expand Down
25 changes: 9 additions & 16 deletions .github/workflows/maintainer-approval.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,12 +251,11 @@ describe("maintainer-approval", () => {

await runModule({ github, context, core });

assert.equal(github._checkRuns.length, 1);
assert.equal(github._checkRuns[0].status, "in_progress");
assert.ok(github._checkRuns[0].output.summary.includes("/bundle/"));
// No check run created; the required check stays as "Expected" (yellow dot).
assert.equal(github._checkRuns.length, 0);
});

it("wildcard files present -> pending, mentions maintainer", async () => {
it("wildcard files present -> pending, no check run", async () => {
const github = makeGithub({
reviews: [
{ state: "APPROVED", user: { login: "randomreviewer" } },
Expand All @@ -268,12 +267,10 @@ describe("maintainer-approval", () => {

await runModule({ github, context, core });

assert.equal(github._checkRuns.length, 1);
assert.equal(github._checkRuns[0].status, "in_progress");
assert.ok(github._checkRuns[0].output.summary.includes("maintainer"));
assert.equal(github._checkRuns.length, 0);
});

it("no approvals at all -> pending", async () => {
it("no approvals at all -> pending, no check run", async () => {
const github = makeGithub({
reviews: [],
files: [{ filename: "cmd/pipelines/foo.go" }],
Expand All @@ -283,8 +280,7 @@ describe("maintainer-approval", () => {

await runModule({ github, context, core });

assert.equal(github._checkRuns.length, 1);
assert.equal(github._checkRuns[0].status, "in_progress");
assert.equal(github._checkRuns.length, 0);
});

it("team member approved -> success for team-owned path", async () => {
Expand Down Expand Up @@ -317,8 +313,7 @@ describe("maintainer-approval", () => {

await runModule({ github, context, core });

assert.equal(github._checkRuns.length, 1);
assert.equal(github._checkRuns[0].status, "in_progress");
assert.equal(github._checkRuns.length, 0);
});

it("CHANGES_REQUESTED does not count as approval", async () => {
Expand All @@ -333,8 +328,7 @@ describe("maintainer-approval", () => {

await runModule({ github, context, core });

assert.equal(github._checkRuns.length, 1);
assert.equal(github._checkRuns[0].status, "in_progress");
assert.equal(github._checkRuns.length, 0);
});

it("self-approval by PR author is excluded", async () => {
Expand All @@ -349,8 +343,7 @@ describe("maintainer-approval", () => {

await runModule({ github, context, core });

assert.equal(github._checkRuns.length, 1);
assert.equal(github._checkRuns[0].status, "in_progress");
assert.equal(github._checkRuns.length, 0);
});

it("no * rule in OWNERS -> setFailed", async () => {
Expand Down
Loading