Skip to content

queue(ci-resolution): thread ignoredCheckRuns into the live CI aggregate instead of only into its cache key #10018

Description

@JSONbored

⚠️ Definition of Done: this issue must be completed in full, in a single PR. Do not split this
work across multiple PRs, and do not defer any Deliverable below to a follow-up issue. A PR that
satisfies only some of the Deliverables, stubs a required test, or leaves a checkbox
partially-done does NOT resolve this issue and will be closed.

Context

gate.ignoredCheckRuns (#9813) is supposed to exclude a maintainer-declared third-party check-run from CI
resolution entirely — "never gates, never pends, never holds". The reducer honours it
(src/github/backfill.ts:3053-3057) and fetchLiveCiAggregatePreferGraphQl accepts it as its 8th positional
argument (src/github/backfill.ts:3426-3443).

The webhook/maintenance path never delivers it. src/queue/ci-resolution.ts declares the field on all three
entry points and folds it into the cache key, but drops it at every hand-off:

src/queue/ci-resolution.ts:237-251fetchLiveCiAggregateWithRequiredContexts builds the arg object for
cachedFetchLiveCiAggregate and omits ignoredCheckRuns, even though it is in scope as args.ignoredCheckRuns
(declared at line 224) and is used one function below at line 206:

        requiredContextsKey: `${resolvedRequiredContextsKeyPart(requiredContexts)}|adv:${advisoryCheckRunsKeyPart(args.advisoryCheckRuns)}|ign:${ignoredCheckRunsKeyPart(args.ignoredCheckRuns)}`,
        advisoryCheckRuns: args.advisoryCheckRuns,
        forceRefresh: args.forceRefresh,
        requiredContextsResolved: resolved,
        admissionKey: args.admissionKey,
      }),

src/queue/ci-resolution.ts:276-287 (cachedLiveCiAggregate) and src/queue/ci-resolution.ts:312-323
(refreshLiveCiAggregate) drop it the same way when calling fetchLiveCiAggregateWithRequiredContexts.

So args.ignoredCheckRuns inside cachedFetchLiveCiAggregate (declared src/queue/ci-resolution.ts:187,
consumed at src/queue/ci-resolution.ts:206) is always undefined. TypeScript cannot catch it: the
property is optional at every hop.

Consequences, all on the live maintenance path:

  • src/queue/processors.ts:3311-3323 passes settings.ignoredCheckRuns into reuseOrRefreshLiveCiAggregate
    correctly, but the resulting ciAggregate.ignoredCheckDetails is always [], so
    src/queue/processors.ts:2910's ignoredCheckNonPassing is always empty and
    src/settings/agent-actions.ts:1167-1168's unstableExplainedByIgnoredChecks is always false. The
    fix(ops): scope the PagerDuty cooldown to rows that actually paged #9810 follow-up that stops an ignored check from holding a PR at mergeable_state: "unstable"
    (src/settings/pr-disposition.ts:128) can never fire in production.
  • An ignored check still counts toward ciState/hasPending in the planner's aggregate, while the executor's
    own pre-mutation re-check (src/services/agent-action-executor.ts:603) and the approval-queue re-check
    (src/services/agent-approval-queue.ts:281) both pass the list through correctly — so planner and executor
    can disagree about the same PR's CI state.

Requirements

  • fetchLiveCiAggregateWithRequiredContexts must forward args.ignoredCheckRuns into its
    cachedFetchLiveCiAggregate call.
  • cachedLiveCiAggregate must forward args.ignoredCheckRuns into its
    fetchLiveCiAggregateWithRequiredContexts call.
  • refreshLiveCiAggregate must forward args.ignoredCheckRuns into its
    fetchLiveCiAggregateWithRequiredContexts call.
  • The requiredContextsKey composition at src/queue/ci-resolution.ts:246 must NOT change — it already
    includes the ignore-list fingerprint and is correct.
  • advisoryCheckRuns threading must NOT change; it is already correct at all three hops.
  • No signature may change: ignoredCheckRuns stays optional at every hop so existing positional callers
    (src/queue/processors.ts:3322) remain byte-identical.
  • Nothing outside src/queue/ci-resolution.ts may be modified except the tests named below.

⚠️ Required pattern: mirror how advisoryCheckRuns is already threaded through the exact same three
hand-offs (src/queue/ci-resolution.ts:247, :284, :320) — one added property per call site, nothing
else. What does NOT satisfy this issue: (a) changing fetchLiveCiAggregatePreferGraphQl or
src/github/backfill.ts in any way — the reducer is already correct; (b) adding a new parallel
"ignored-check aware" fetch path alongside the existing one instead of fixing the three drops;
(c) a test-only PR that asserts the current (broken) undefined argument; (d) making
ignoredCheckRuns a required property, which changes every call signature for no benefit.

Deliverables

  • src/queue/ci-resolution.ts:237-251cachedFetchLiveCiAggregate is called with
    ignoredCheckRuns: args.ignoredCheckRuns.
  • src/queue/ci-resolution.ts:276-287fetchLiveCiAggregateWithRequiredContexts is called with
    ignoredCheckRuns: args.ignoredCheckRuns.
  • src/queue/ci-resolution.ts:312-323fetchLiveCiAggregateWithRequiredContexts is called with
    ignoredCheckRuns: args.ignoredCheckRuns.
  • A test in test/unit/ci-resolution.test.ts that calls cachedLiveCiAggregate with
    ignoredCheckRuns: [{ name: "Contributor trust", appSlug: "example-security-app" }] and asserts, via a
    spy on fetchLiveCiAggregatePreferGraphQl, that the 8th positional argument equals that exact array
    (today it is undefined).
  • The same assertion for refreshLiveCiAggregate in test/unit/ci-resolution.test.ts.
  • The same assertion for reuseOrRefreshLiveCiAggregate in test/unit/ci-resolution.test.ts (the
    positional-arg entry point src/queue/processors.ts:3311 actually uses).
  • A regression test at test/unit/ci-resolution.test.ts named for this bug asserting that with
    ignoredCheckRuns: null (unconfigured) the 8th argument is still null/undefined, so an
    un-opted-in repo stays byte-identical.

All Deliverables above are required in a single PR. A PR that satisfies only some of them — for example
fixing only fetchLiveCiAggregateWithRequiredContexts (the innermost hop) while leaving
cachedLiveCiAggregate and refreshLiveCiAggregate still dropping the field, so the value never arrives —
does not resolve this issue.

Test Coverage Requirements

This repo enforces 99%+ Codecov patch coverage, branch-counted. vitest.config.ts's coverage.include
covers src/**/*.ts, so src/queue/ci-resolution.ts is measured and gated. The change adds no new branch —
it adds three object properties on existing call paths — but each of the three touched call sites must be
executed by at least one test, and the ignoredCheckRunsKeyPart helper's both arms (empty/absent list →
"", non-empty list → JSON) must each be exercised, since a test that only ever passes a populated list
leaves the absent arm uncovered on the touched lines.

Expected Outcome

After this ships, a repo that declares gate.ignoredCheckRuns in .loopover.yml actually has that check
excluded from the live CI aggregate the maintenance planner reads: ciState/hasPending ignore it,
ignoredCheckDetails is populated, and unstableExplainedByIgnoredChecks can become true — so a PR whose
only instability is the ignored check stops being held for manual review. The planner and the executor's
pre-mutation re-check now compute CI state from the same effective check set.

Links & Resources

  • src/queue/ci-resolution.ts:187cachedFetchLiveCiAggregate's ignoredCheckRuns arg (never populated)
  • src/queue/ci-resolution.ts:206 — the only consumer of that always-undefined value
  • src/queue/ci-resolution.ts:237-251, :276-287, :312-323 — the three drops
  • src/queue/processors.ts:2910ignoredCheckNonPassing, always empty today
  • src/settings/agent-actions.ts:1167-1168unstableExplainedByIgnoredChecks, always false today
  • src/github/backfill.ts:3426-3443fetchLiveCiAggregatePreferGraphQl's 8-arg signature

Metadata

Metadata

Assignees

No one assigned

    Labels

    gittensor:bugGittensor-scored bug fix — scores a 0.05x multiplier.help wantedExtra attention is needed

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions