Skip to content

miner(ci-poller): fetchCheckRuns' page-follow loop has no page cap and can spin forever #10007

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

fetchCheckRuns walks the check-runs endpoint's pages in an unbounded while (true)
packages/loopover-miner/lib/ci-poller.ts (fetchCheckRuns):

  const checks: NormalizedCheckRun[] = [];
  let page = 1;
  let expectedTotalCount: number | null = null;
  while (true) {
    const { payload, response } = await githubGetJsonResponse(
      apiUrl(
        options.apiBaseUrl,
        repoPath(target, `/commits/${encodeURIComponent(headSha)}/check-runs`),
        `?per_page=100&page=${page}`,
      ),
      options,
    );
    ...
    if (!hasNextLink(response) && (expectedTotalCount === null || checks.length >= expectedTotalCount)) {
      return checks;
    }
    if (pageChecks.length === 0) {
      throw new Error("github_check_runs_pagination_incomplete");
    }
    page += 1;
  }

There is no iteration cap. The loop exits only when the response stops advertising rel="next"
and the accumulated count reaches total_count, or when a page comes back empty. A response stream that keeps
returning a Link: <…>; rel="next" header alongside non-empty check_runs arrays — a proxy that mishandles
pagination, a cached/looping edge, or a forge-compatible endpoint whose Link semantics differ — makes this loop
run without end, accumulating into checks on every pass. hasNextLink is a bare regex over the header
(packages/loopover-miner/lib/ci-poller.ts, hasNextLink); nothing validates that the advertised next page is
actually different from the one just fetched.

The sibling page-follow loop in the same package cured exactly this, and says so —
packages/loopover-miner/lib/opportunity-fanout.ts:99-101:

// Follow the GitHub Link header past the first page so a repo/search with >100 open issues isn't silently
// truncated (#4831); cap the follow loop so a pathological Link chain can't run away.
const defaultMaxPages = 10;

and enforces it in the loop head (packages/loopover-miner/lib/opportunity-fanout.ts:537,
:580): for (let page = 0; url !== null && page < options.maxPages; page += 1).

The blast radius is operational, not theoretical. pollCheckRuns is reached from manage poll
(packages/loopover-miner/lib/manage-poll.ts:212-225), which
packages/loopover-miner/docs/unattended-scheduling.md documents as a scheduled cron/systemd command with an
exit-code contract of 0 or 2. A hung poll never produces either — the cron job simply never terminates, and an
operator alerting on the documented exit codes sees nothing. loop-cli.ts calls the same poller inside its own
cycle (packages/loopover-miner/lib/loop-cli.ts:317), so an autonomous loop wedges on the same condition.

Every other bound in this file already exists: maxAttempts, minIntervalMs, maxIntervalMs and
requestTimeoutMs are all clamped in normalizeOptions. The per-attempt HTTP timeout bounds one request, not the
number of requests, so it does not help here.

Requirements

  • fetchCheckRuns must stop after a bounded number of pages. Add a maxPages knob to PollCheckRunsOptions,
    normalized through the file's existing normalizePositiveInt with a documented default and clamp range,
    exactly as maxAttempts / minIntervalMs / maxIntervalMs / requestTimeoutMs already are in
    normalizeOptions.
  • The default must be large enough that no realistic PR is truncated: 100 check runs per page × the default page
    cap must comfortably exceed any real head SHA's check-run count. State the chosen default and its reasoning in a
    comment, mirroring packages/loopover-miner/lib/opportunity-fanout.ts:99-101.
  • Reaching the cap must be a loud, distinguishable failure, not a silent truncation: throw an Error whose
    message is distinct from the existing github_check_runs_pagination_incomplete, so an operator can tell
    "the server kept advertising more pages" from "a page came back empty mid-stream".
  • A run that legitimately completes within the cap must behave byte-identically to today: same returned
    NormalizedCheckRun[], same ordering, same github_check_runs_malformed and
    github_check_runs_pagination_incomplete errors on their existing conditions, and the same
    total_count-vs-Link exit condition.
  • Do NOT change pollCheckRuns's attempt loop, backoffDelayMs, aggregateConclusion, normalizeConclusion, or
    the head-SHA re-check.
  • Do NOT change opportunity-fanout.ts.

⚠️ Required pattern: mirror packages/loopover-miner/lib/opportunity-fanout.ts:99-101 and :537 — a named
default constant with a comment, normalized/clamped alongside the other option bounds, and enforced in the loop
head. What does NOT satisfy this issue: (a) capping by accumulated checks.length instead of by page count,
which still spins forever if a looping endpoint returns the same page repeatedly and total_count is absent;
(b) silently return checks at the cap, which turns a broken pagination stream into a wrong CI conclusion fed
straight into mapPollConclusionToGateVerdict (packages/loopover-miner/lib/manage-poll.ts:64-73); (c) relying
on requestTimeoutMs, which bounds one request, not the loop.

Deliverables

  • PollCheckRunsOptions in packages/loopover-miner/lib/ci-poller.ts gains maxPages?: number, normalized in
    normalizeOptions via normalizePositiveInt with an explicit default and clamp range.
  • With an injected fetchFn that always returns a non-empty check_runs page and a
    link: <…>; rel="next" header, pollCheckRuns("acme/widgets", 4, { fetchFn, maxPages: 3, … }) rejects with
    the new cap error after exactly 3 check-runs requests (asserted by call count) — asserted in
    test/unit/miner-ci-poller.test.ts or test/unit/miner-ci-poller-failure-modes.test.ts, whichever already
    holds the pagination fixtures.
  • A two-page response (page 1 with a rel="next" link, page 2 without) still returns all check runs from both
    pages, in order, with the default maxPages — asserted in the same test file.
  • An empty second page still throws github_check_runs_pagination_incomplete, unchanged — asserted in the
    same test file.
  • A regression test named for this bug (e.g. REGRESSION: a never-ending rel="next" chain stops at the page cap instead of looping forever) that fails (by timing out or hanging) against the current code and passes
    after the fix.

All Deliverables above are required in a single PR. A PR that satisfies only some of them — for example one that
adds the cap but returns the partial checks array instead of throwing, so a wedged endpoint silently yields a
"success" CI conclusion — does not resolve this issue.

Test Coverage Requirements

This repo enforces 99%+ Codecov patch coverage, branch-counted. vitest.config.ts's coverage.include lists
packages/loopover-miner/lib/**/*.ts, so ci-poller.ts is measured and gated. Every branch the change introduces
needs both arms tested: the loop-head page <= maxPages guard (cap reached and not reached); the
options.maxPages supplied vs omitted default in normalizeOptions, plus its clamp floor and ceiling; and the
existing hasNextLink / expectedTotalCount === null || checks.length >= expectedTotalCount exit condition and
the pageChecks.length === 0 throw, both of which must still be reachable after the change.

Expected Outcome

A malfunctioning or non-conforming check-runs endpoint can no longer wedge loopover-miner manage poll or the
autonomous loop in an infinite page-follow. The poller either completes within a bounded number of requests or
fails loudly with a distinguishable error, so the scheduled-run exit-code contract in
packages/loopover-miner/docs/unattended-scheduling.md is honoured instead of the process simply never
terminating.

Links & Resources

  • packages/loopover-miner/lib/ci-poller.tsfetchCheckRuns (while (true)), hasNextLink,
    normalizeOptions, normalizePositiveInt
  • packages/loopover-miner/lib/opportunity-fanout.ts:99-101, :537, :580 — the capped sibling loop to mirror
  • packages/loopover-miner/lib/manage-poll.ts:212-225recordManagePollSnapshot's call into the poller
  • packages/loopover-miner/lib/loop-cli.ts:317 — the autonomous loop's call into the poller
  • packages/loopover-miner/docs/unattended-scheduling.md:16-23 — the scheduled-run exit-code contract

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