Skip to content

Commit faf5125

Browse files
authored
fix(github): fail closed on third-party action required checks (#4414)
1 parent 46006b1 commit faf5125

2 files changed

Lines changed: 12 additions & 19 deletions

File tree

src/github/backfill.ts

Lines changed: 9 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2460,8 +2460,7 @@ async function fetchPullRequestChecks(
24602460
// ever ran. Excluded here, a github-actions action_required check falls through to anyPending → ciState="pending"
24612461
// → the PR is DEFERRED/held (never closed) until its runs are approved (manually, or auto-approved by fork CI
24622462
// auto-approval). (#fork-action-required) — a THIRD-PARTY app's own COMPLETED action_required verdict (e.g. a
2463-
// security/trust-scan tool) is handled separately below (isSettledThirdPartyActionRequired) and does NOT fall
2464-
// through to pending: it is a settled result, not an in-progress workflow awaiting approval to run.
2463+
// security/check tool) is handled separately below and fails closed as a manual-hold signal, not green CI.
24652464
const CI_FAILING_CONCLUSIONS = new Set(["failure", "timed_out", "cancelled", "startup_failure"]);
24662465
const CI_PASSING_CONCLUSIONS = new Set(["success", "neutral", "skipped"]);
24672466
// The bot's OWN check-runs — it posts these (in_progress, then concluded) as PART OF reviewing. They are NOT
@@ -2784,22 +2783,16 @@ async function reduceLiveCiAggregate(
27842783
total += 1;
27852784
const conclusion = (run.conclusion ?? "").toLowerCase();
27862785
const status = (run.status ?? "").toLowerCase();
2787-
// A THIRD-PARTY app's OWN action_required verdict on an already-COMPLETED check-run (e.g. Superagent
2788-
// Security's "Contributor trust") is a settled, terminal result -- it ran, and its own business logic
2789-
// says "a human should look at this", which is informational, not "still executing". This is NOT the
2790-
// github-actions "awaiting maintainer Approve and run" case the action_required exclusion above exists
2791-
// for (that one is scoped to appSlug === "github-actions" specifically) -- a non-Actions app's completed
2792-
// action_required check can never transition to any other state via a CI event gittensory observes, so
2793-
// treating it as pending made prReadyForReview defer the review FOREVER (#superagent-action-required-stuck:
2794-
// confirmed live, JSONbored/awesome-claude#4728 stuck 30+ minutes across 8 outage-repair attempts before
2795-
// exhausting REGATE_REPAIR_MAX_ATTEMPTS_PER_SHA and falling back to slow passive sweep cadence, never
2796-
// actually reviewed). Conservative: an unknown/absent app slug is NOT treated as settled here, only a
2797-
// confirmed non-Actions app.
2798-
const isSettledThirdPartyActionRequired = conclusion === "action_required" && status === "completed" && appSlug !== "" && appSlug !== "github-actions";
2799-
if (conclusion ? CI_FAILING_CONCLUSIONS.has(conclusion) : false) {
2786+
// A THIRD-PARTY app's OWN action_required verdict on an already-COMPLETED check-run (for example, a
2787+
// security/check tool asking for human review) is a settled, terminal adverse result. This is NOT the
2788+
// github-actions "awaiting maintainer Approve and run" case the action_required exclusion above exists for:
2789+
// non-Actions apps use their own conclusion as a policy signal, so fail closed instead of treating it as
2790+
// green CI. Conservative: an unknown/absent app slug is NOT treated as third-party here.
2791+
const isThirdPartyActionRequiredFailure = conclusion === "action_required" && status === "completed" && appSlug !== "" && appSlug !== "github-actions";
2792+
if (isThirdPartyActionRequiredFailure || (conclusion ? CI_FAILING_CONCLUSIONS.has(conclusion) : false)) {
28002793
const summary = [run.output?.title, run.output?.summary].find((value): value is string => typeof value === "string" && value.trim().length > 0)?.trim().slice(0, 200);
28012794
failingDetails.push({ name: run.name, ...(summary ? { summary } : {}), ...(run.details_url ? { detailsUrl: run.details_url } : {}) });
2802-
} else if (isSettledThirdPartyActionRequired || (conclusion ? CI_PASSING_CONCLUSIONS.has(conclusion) : status === "completed")) {
2795+
} else if (conclusion ? CI_PASSING_CONCLUSIONS.has(conclusion) : status === "completed") {
28032796
// concluded and not failing → passing
28042797
} else {
28052798
anyVisiblePending = true;

test/unit/backfill.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4211,7 +4211,7 @@ describe("GitHub backfill", () => {
42114211
expect(aggregate.failingDetails).toEqual([]);
42124212
});
42134213

4214-
it("a third-party app's COMPLETED action_required check-run is settled, not pending (Superagent Contributor Trust regression, #4728)", async () => {
4214+
it("a third-party app's COMPLETED action_required check-run fails closed as a manual-hold verdict", async () => {
42154215
const env = createTestEnv({ GITHUB_PUBLIC_TOKEN: "public-token" });
42164216
vi.stubGlobal("fetch", async (input: RequestInfo | URL) => {
42174217
const url = input.toString();
@@ -4230,11 +4230,11 @@ describe("GitHub backfill", () => {
42304230

42314231
const aggregate = await fetchLiveCiAggregate(env, "JSONbored/awesome-claude", "sha4728", "public-token", new Set(["coverage", "Contributor trust"]));
42324232

4233-
expect(aggregate.ciState).toBe("passed");
4233+
expect(aggregate.ciState).toBe("failed");
42344234
expect(aggregate.hasPending).toBe(false);
42354235
expect(aggregate.hasVisiblePending).toBe(false);
42364236
expect(aggregate.hasMissingRequiredContext).toBe(false);
4237-
expect(aggregate.failingDetails).toEqual([]);
4237+
expect(aggregate.failingDetails).toEqual([{ name: "Contributor trust" }]);
42384238
});
42394239

42404240
it("a github-actions workflow awaiting 'Approve and run' (action_required) is still treated as pending, not settled (#fork-action-required)", async () => {

0 commit comments

Comments
 (0)