Skip to content

fix(queue): cachedRequiredStatusContexts computes a resolved flag that no caller ever consumes #8358

Description

@JSONbored

Context

src/queue/ci-resolution.ts, cachedRequiredStatusContexts (lines 81-106) returns a RequiredStatusContextsLookup ({ requiredContexts: Set<string> | null; resolved: boolean }, type defined in src/queue/processors.ts line 697). The resolved field is deliberately computed to distinguish "the required-contexts merge reflects a successful LIVE branch-protection read" (resolved: true) from "the branch-protection fetch failed and this only reflects the .loopover.yml expectedCiContexts config fallback" (resolved: false, set via the branchProtectionFetchFailed flag and the onFetchFailure callback threaded through fetchRequiredStatusContexts, lines 93-102).

This module's own header comment describes the RC2 "live branch-protection required contexts merged with config fallback" contract as something callers were meant to be able to reason about. But the sole call site of cachedRequiredStatusContextssrc/queue/processors.ts line ~2860, destructured at line 2880 — only reads .requiredContexts:

const requiredContexts = requiredContextsLookup.requiredContexts;

requiredContextsLookup.resolved is never read anywhere else in the codebase (confirmed by grep). So today, a degraded pass where the live branch-protection read failed and the gate is silently running only off the config fallback is computed but structurally unobservable to any caller, log, or metric — the exact distinction this field exists to let a caller act on has no way to actually be consumed. Relatedly, test/unit/ci-resolution.test.ts never exercises the branchProtectionFetchFailed = true path (all tests mock fetchRequiredStatusContexts wholesale rather than driving its onFetchFailure callback), so this branch also has zero direct test coverage today.

Requirements

  • The resolved field of RequiredStatusContextsLookup must be consumed by its call site in src/queue/processors.ts (~line 2880), at minimum to emit an observable signal (a console.warn/incr() metric, matching this codebase's existing convention for degraded-fetch paths elsewhere in the same file — e.g. CI_STATE_CACHE_METRIC) when resolved === false for a PR's live gate evaluation, so a branch-protection-read failure during a live gate check is no longer silent.
  • Do NOT change the actual gate/merge/close disposition logic based on resolved — this issue is scoped to observability of the existing computed flag, not to introducing new blocking behavior. A behavior change here (e.g. holding the gate when resolved: false) would be a materially bigger, separate decision requiring maintainer sign-off — do not add it in this PR.
  • Alternatively, if investigation shows the resolved field genuinely serves no purpose and was always meant to be removed rather than wired up, that is an acceptable resolution instead — but it must be an explicit removal (delete the field from the type, its computation in cachedRequiredStatusContexts, and the branchProtectionFetchFailed tracking variable) with the PR description stating that was the chosen direction. Pick exactly one of the two directions; do not leave it ambiguous.

Deliverables

  • Either (a) requiredContextsLookup.resolved is read at its call site in src/queue/processors.ts and surfaced via a log/metric when false, or (b) the dead resolved field and its computation are cleanly removed from RequiredStatusContextsLookup / cachedRequiredStatusContexts — exactly one, stated in the PR description.
  • New test in test/unit/ci-resolution.test.ts exercising the onFetchFailure callback path (a mocked fetchRequiredStatusContexts that invokes its failure callback) and asserting the chosen new behavior.

Test Coverage Requirements

99%+ Codecov patch coverage, branch-counted, on every changed line/branch in src/queue/ci-resolution.ts and (if applicable) src/queue/processors.ts, per codecov.yml — this closes a currently-untested branch (branchProtectionFetchFailed === true) that today has zero direct test coverage in test/unit/ci-resolution.test.ts.

Expected Outcome

A branch-protection-read failure during live gate evaluation is either observable (logged/metriced) instead of silently invisible, or the dead field tracking it is removed — either way, RequiredStatusContextsLookup.resolved no longer exists as computed-but-unused state.

Links & Resources

  • src/queue/ci-resolution.ts (cachedRequiredStatusContexts)
  • src/queue/processors.ts (line ~2849-2880, the sole call site; RequiredStatusContextsLookup type at line 697)
  • test/unit/ci-resolution.test.ts

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