Skip to content

fix(queue): thread ignoredCheckRuns into the live CI aggregate, not just its cache key - #10163

Merged
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
shin-core:fix/ci-resolution-thread-ignored-checks-10018
Jul 31, 2026
Merged

fix(queue): thread ignoredCheckRuns into the live CI aggregate, not just its cache key#10163
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
shin-core:fix/ci-resolution-thread-ignored-checks-10018

Conversation

@shin-core

Copy link
Copy Markdown
Contributor

What & why

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 and fetchLiveCiAggregatePreferGraphQl accepts it as its 8th positional argument. But src/queue/ci-resolution.ts folded it into the durable cache key at all three hand-offs while dropping it from the arg object passed downstream:

requiredContextsKey: `…|ign:${ignoredCheckRunsKeyPart(args.ignoredCheckRuns)}`,  // in the key…
advisoryCheckRuns: args.advisoryCheckRuns,
// …but no `ignoredCheckRuns:` here — so cachedFetchLiveCiAggregate's ignoredCheckRuns is always undefined

TypeScript can't catch it: the property is optional at every hop. So args.ignoredCheckRuns inside cachedFetchLiveCiAggregate (its only consumer) is always undefined. Consequences, all on the live maintenance path:

  • ciAggregate.ignoredCheckDetails is always [], so ignoredCheckNonPassing is always empty and 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" can never fire in production.
  • An ignored check still counts toward ciState/hasPending in the planner's aggregate, while the executor's and approval-queue's own re-checks pass the list through correctly — so planner and executor can disagree about the same PR's CI state.

The fix

Forward args.ignoredCheckRuns at all three call sites — fetchLiveCiAggregateWithRequiredContexts, cachedLiveCiAggregate, and refreshLiveCiAggregate — mirroring how advisoryCheckRuns is already threaded through the same three hops. One added property per call site, nothing else.

Unchanged: signatures stay optional so positional callers (reuseOrRefreshLiveCiAggregate at processors.ts:3322) are byte-identical; the requiredContextsKey composition (already includes the ignore-list fingerprint) and the advisoryCheckRuns threading are unchanged; nothing outside ci-resolution.ts is modified.

Tests (test/unit/ci-resolution.test.ts)

  • Via a spy on fetchLiveCiAggregatePreferGraphQl, its 8th positional argument equals the exact ignore array [{ name: "Contributor trust", appSlug: "example-security-app" }] for cachedLiveCiAggregate, refreshLiveCiAggregate, and the positional entry point reuseOrRefreshLiveCiAggregate — today it is undefined.
  • REGRESSION: an unconfigured repo (ignoredCheckRuns: null) still passes null/undefined through, so an un-opted-in repo stays byte-identical.
  • The three forwarding assertions fail on main.

Validation

  • Diff coverage on src/queue/ci-resolution.ts is 100% line and branch (each of the three touched call sites executed; the populated and absent ignoredCheckRunsKeyPart arms both exercised).
  • npm run typecheck clean; npm run engine-parity:drift-check passes (not a twin); npm run dead-exports:check clean; the suite (15 tests) green.
  • git diff --check clean; no schema/migration/generated-artifact change.

Closes #10018

…ust its cache key

gate.ignoredCheckRuns (JSONbored#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 JSONbored#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 JSONbored#10018
@shin-core
shin-core requested a review from JSONbored as a code owner July 31, 2026 10:19
@loopover-orb

loopover-orb Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Tip

✅ LoopOver review result - approve/merge recommended

Review updated: 2026-07-31 10:39:49 UTC

2 files · 1 AI reviewer · no blockers · readiness 95/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This PR fixes a real bug: `ignoredCheckRuns` was folded into the cache key at three call sites but never forwarded into the arg object passed to `cachedFetchLiveCiAggregate`/`fetchLiveCiAggregatePreferGraphQl`, so the ignore list never actually took effect on the live aggregate despite affecting cache invalidation. The fix mirrors the existing `advisoryCheckRuns` threading pattern exactly at all three call sites (`fetchLiveCiAggregateWithRequiredContexts`, `cachedLiveCiAggregate`, `refreshLiveCiAggregate`), is minimal (7 lines), and is backed by tests that assert the value reaches the 8th positional argument of `fetchLiveCiAggregatePreferGraphQl` via spy inspection, including a byte-identical-behavior regression test for the null/unconfigured case. The diff is self-contained, low-risk, and the full file content confirms `ignoredCheckRuns` was indeed accepted but dropped before this change.

Nits — 3 non-blocking

Decision drivers

  • ✅ Code review — No blockers (1 reviewer)
  • ✅ Gate result — Passing (No configured blocker found.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
Linked issue ✅ Linked #10018
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 78 registered-repo PR(s), 60 merged, 0 issue(s).
Contributor context ✅ Confirmed Gittensor contributor shin-core; Gittensor profile; 78 PR(s), 0 issue(s).
Improvement ✅ Minor risk: clean · value: minor · LLM: moderate
Linked issue satisfaction

Addressed
The diff adds ignoredCheckRuns: args.ignoredCheckRuns at all three specified call sites (fetchLiveCiAggregateWithRequiredContexts, cachedLiveCiAggregate, refreshLiveCiAggregate), mirroring advisoryCheckRuns exactly as required, and leaves requiredContextsKey/signatures unchanged. It also adds the required tests in test/unit/ci-resolution.test.ts covering cachedLiveCiAggregate, refreshLiveCiAggrega

Review context
  • Author: shin-core
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: not available
  • Official Gittensor activity: 78 PR(s), 0 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Start here: Triage stale or unlinked PRs.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.
🧪 Chat with LoopOver

Ask LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

  • @loopover ask <question> answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat <question> answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @loopover mention with a real question is routed to the closest matching read-only command automatically — no exact syntax required.

Full command reference: https://loopover.ai/docs/loopover-commands

🧪 Experimental — new and may change.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.

  • Re-run LoopOver review

@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 31, 2026
@codecov

codecov Bot commented Jul 31, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.45%. Comparing base (1a2b41a) to head (aae9c36).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #10163      +/-   ##
==========================================
+ Coverage   80.43%   80.45%   +0.02%     
==========================================
  Files         282      283       +1     
  Lines       58744    58829      +85     
  Branches     6963     6983      +20     
==========================================
+ Hits        47248    47331      +83     
  Misses      11205    11205              
- Partials      291      293       +2     
Flag Coverage Δ
backend 97.64% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/queue/ci-resolution.ts 97.64% <ø> (ø)

@loopover-orb loopover-orb Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LoopOver approves — the gate is satisfied and CI is green.

@loopover-orb
loopover-orb Bot merged commit 7510075 into JSONbored:main Jul 31, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

1 participant