From aae9c36103619c82de049d28d88bbc56a21f65c3 Mon Sep 17 00:00:00 2001 From: shin-core <153108882+shin-core@users.noreply.github.com> Date: Fri, 31 Jul 2026 19:19:18 +0900 Subject: [PATCH] fix(queue): thread ignoredCheckRuns into the live CI aggregate, not just its cache key gate.ignoredCheckRuns (#9813) is supposed to exclude a maintainer-declared check-run from CI resolution entirely, and the reducer + fetchLiveCiAggregatePreferGraphQl honour it. But ci-resolution.ts folded it into the durable cache key at all three hand-offs while dropping it from the arg object passed downstream, so cachedFetchLiveCiAggregate's ignoredCheckRuns was always undefined -- TypeScript can't catch it because the property is optional at every hop. The consequence on the live maintenance path: ciAggregate's ignoredCheckDetails is always [], ignoredCheckNonPassing is always empty, and unstableExplainedByIgnoredChecks is always false, so the #9810 follow-up that stops an ignored check from holding a PR at mergeable_state 'unstable' can never fire in production, and the planner and executor can disagree about the same PR's CI state. Forward args.ignoredCheckRuns at all three call sites (fetchLiveCiAggregateWithRequiredContexts, cachedLiveCiAggregate, refreshLiveCiAggregate), mirroring how advisoryCheckRuns is already threaded through the same hops. Signatures stay optional so positional callers are byte-identical; the requiredContextsKey composition and advisoryCheckRuns threading are unchanged, and nothing outside ci-resolution.ts is touched. Closes #10018 --- src/queue/ci-resolution.ts | 7 ++++ test/unit/ci-resolution.test.ts | 62 +++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+) diff --git a/src/queue/ci-resolution.ts b/src/queue/ci-resolution.ts index 9c5e2476ae..d408a7e7e7 100644 --- a/src/queue/ci-resolution.ts +++ b/src/queue/ci-resolution.ts @@ -245,6 +245,11 @@ function fetchLiveCiAggregateWithRequiredContexts( // would keep serving a stale aggregate computed against the old advisory list. requiredContextsKey: `${resolvedRequiredContextsKeyPart(requiredContexts)}|adv:${advisoryCheckRunsKeyPart(args.advisoryCheckRuns)}|ign:${ignoredCheckRunsKeyPart(args.ignoredCheckRuns)}`, advisoryCheckRuns: args.advisoryCheckRuns, + // #10018: thread the ignore list into the aggregate itself, not just its cache key — it was folded + // into `requiredContextsKey` above but dropped from the arg object, so `cachedFetchLiveCiAggregate`'s + // `ignoredCheckRuns` was always undefined and a maintainer-declared ignored check never actually + // dropped out of the live CI aggregate the maintenance planner reads. Mirrors advisoryCheckRuns above. + ignoredCheckRuns: args.ignoredCheckRuns, forceRefresh: args.forceRefresh, requiredContextsResolved: resolved, admissionKey: args.admissionKey, @@ -282,6 +287,7 @@ export function cachedLiveCiAggregate( token: args.token, expectedCiContexts: args.expectedCiContexts, advisoryCheckRuns: args.advisoryCheckRuns, + ignoredCheckRuns: args.ignoredCheckRuns, // #10018: forward the ignore list, mirroring advisoryCheckRuns forceRefresh: false, admissionKey: args.admissionKey, }), @@ -318,6 +324,7 @@ export function refreshLiveCiAggregate( token: args.token, expectedCiContexts: args.expectedCiContexts, advisoryCheckRuns: args.advisoryCheckRuns, + ignoredCheckRuns: args.ignoredCheckRuns, // #10018: forward the ignore list, mirroring advisoryCheckRuns forceRefresh: true, admissionKey: args.admissionKey, }), diff --git a/test/unit/ci-resolution.test.ts b/test/unit/ci-resolution.test.ts index a48e90f17d..3a16124650 100644 --- a/test/unit/ci-resolution.test.ts +++ b/test/unit/ci-resolution.test.ts @@ -4,8 +4,10 @@ import { cachedLiveCiAggregate, cachedRequiredStatusContexts, observeRequiredContextsLookup, + refreshLiveCiAggregate, refreshLiveMergeState, REQUIRED_CONTEXTS_UNRESOLVED_METRIC, + reuseOrRefreshLiveCiAggregate, setMergeStateUnknownRetryDelayMsForTest, } from "../../src/queue/ci-resolution"; import type { LiveGithubFacts } from "../../src/queue/processors"; @@ -135,6 +137,66 @@ describe("cachedLiveCiAggregate request-scoped memoization (#4498)", () => { }); }); +describe("ignoredCheckRuns is threaded into the live CI aggregate, not just its cache key (#10018)", () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + const IGNORED = [{ name: "Contributor trust", appSlug: "example-security-app" }]; + const okAggregate = { + ciState: "passed" as const, + hasPending: false, + hasVisiblePending: false, + hasMissingRequiredContext: false, + failingDetails: [], + nonRequiredFailingDetails: [], + advisoryHoldDetails: [], + ignoredCheckDetails: [], + ciCompletenessWarning: null, + }; + // Distinct headSha per call so the request-scoped memo never serves one test's aggregate to another. + const base = (headSha: string) => ({ + repoFullName: "owner/repo", + facts: emptyFacts(), + prNumber: 7, + headSha, + baseRef: null, + token: "tok", + expectedCiContexts: null, + advisoryCheckRuns: null, + }); + // fetchLiveCiAggregatePreferGraphQl's 8th positional argument (index 7) is ignoredCheckRuns. + const eighthArg = (spy: ReturnType) => spy.mock.calls[0]?.[7]; + + it("cachedLiveCiAggregate forwards the ignore list as the 8th positional argument", async () => { + const env = createTestEnv(); + const spy = vi.spyOn(backfillModule, "fetchLiveCiAggregatePreferGraphQl").mockResolvedValue(okAggregate); + await cachedLiveCiAggregate(env, { ...base("sha-cached"), ignoredCheckRuns: IGNORED }); + expect(eighthArg(spy)).toEqual(IGNORED); + }); + + it("refreshLiveCiAggregate forwards the ignore list as the 8th positional argument", async () => { + const env = createTestEnv(); + const spy = vi.spyOn(backfillModule, "fetchLiveCiAggregatePreferGraphQl").mockResolvedValue(okAggregate); + await refreshLiveCiAggregate(env, { ...base("sha-refresh"), ignoredCheckRuns: IGNORED }); + expect(eighthArg(spy)).toEqual(IGNORED); + }); + + it("reuseOrRefreshLiveCiAggregate (the positional-arg entry point) forwards the ignore list", async () => { + const env = createTestEnv(); + const spy = vi.spyOn(backfillModule, "fetchLiveCiAggregatePreferGraphQl").mockResolvedValue(okAggregate); + // reuseOrRefreshLiveCiAggregate is positional: ignoredCheckRuns is its 11th arg (after advisoryCheckRuns). + await reuseOrRefreshLiveCiAggregate(env, "owner/repo", emptyFacts(), 7, "abc123", null, "tok", null, undefined, null, IGNORED); + expect(eighthArg(spy)).toEqual(IGNORED); + }); + + it("REGRESSION #10018: an unconfigured repo (ignoredCheckRuns null) still passes null/undefined through — byte-identical", async () => { + const env = createTestEnv(); + const spy = vi.spyOn(backfillModule, "fetchLiveCiAggregatePreferGraphQl").mockResolvedValue(okAggregate); + await cachedLiveCiAggregate(env, { ...base("sha-null"), ignoredCheckRuns: null }); + expect(eighthArg(spy) ?? null).toBeNull(); // no ignore list configured ⇒ the arg is null/undefined + }); +}); + describe("cachedRequiredStatusContexts resolved flag (#8358)", () => { afterEach(() => { vi.restoreAllMocks();