Skip to content

upstream(drift): stop filing a duplicate drift issue when the fingerprint search fails, and bound its unbounded pagination loop #10027

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

fileUpstreamDriftIssues decides create-vs-update from two lookups, and treats a null from either as
proof that no issue exists (src/upstream/ruleset.ts:350, :373):

    const existing = (await validateRecordedGitHubIssue(repo, token, report)) ?? (await findGitHubIssueForFingerprint(repo, token, report.fingerprint));
    if (existing) { /* … PATCH the existing issue … */ }
    const issue = await createGitHubDriftIssue(repo, token, report, assignees);

But findGitHubIssueForFingerprint returns null for a read failure exactly as it does for "no match"
(src/upstream/ruleset.ts:1095-1126):

async function findGitHubIssueForFingerprint(repo: string, token: string, fingerprint: string): Promise<ExistingDriftIssue | null> {
  const [owner, name] = repo.split("/");
  if (!owner || !name) return null;
  try {
    for (let page = 1; ; page += 1) {
      const url = `https://github.com/ghapi/repos/${owner}/${name}/issues?state=open&labels=signals&per_page=100&page=${page}`;
      const response = await timeoutFetch(url, { headers: githubHeaders({ token, accept: "application/vnd.github+json" }) });
      if (!response.ok) return null;
      
      if (!response.headers.get("link")?.includes('rel="next"')) return null;
    }
  } catch {
    return null;
  }
}

validateRecordedGitHubIssue (src/upstream/ruleset.ts:1154-1189) has the identical if (!response.ok) return null /
catch { return null } shape. So during any GitHub read failure that hits both lookups — a 403 rate-limit
(the cron runs on a shared REST budget), a 5xx, or a timeoutFetch abort — the loop falls straight through to
createGitHubDriftIssue and files a second issue for a fingerprint that already has one. The dedup is not
protected anywhere else: githubDriftIssueTitle is chore(upstream): reconcile Gittensor drift <fp8> for every
run, and createGitHubDriftIssue performs an unconditional POST /issues with no idempotency marker check.
Each cron tick that fails the same way files another copy, and the updateUpstreamDriftReportIssue write then
re-points the report at the newest duplicate, orphaning the original.

Separately, that same loop is the only unbounded pagination walk left in this file's GitHub reads:
for (let page = 1; ; page += 1) has no page cap, unlike every sibling in the codebase —
COMMENT_SEARCH_PAGE_LIMIT = 10 (src/github/comments.ts:28), REVIEW_PAGE_LIMIT = 10
(src/github/pr-actions.ts:12), MAX_WORKFLOW_RUN_LIST_PAGES = 10 (src/github/app.ts:612),
PR_DETAIL_MAX_PAGES = 10 / GITHUB_LIST_MAX_PAGES = 10 (src/github/backfill.ts:2433-2437), whose comment
states the convention outright: "every other pagination walk in this file already caps at 10 pages so a
pathological repo can't turn one sync into an unbounded fetch loop".

Requirements

  • findGitHubIssueForFingerprint MUST distinguish "searched successfully, found nothing" from "could not
    search". It MUST return a three-state result (e.g. ExistingDriftIssue | null | "unavailable", or an object
    with an explicit errored flag — mirroring LastCloserResult.errored in src/github/pr-actions.ts:300).
    A non-OK response, a thrown fetch, and a JSON-parse failure MUST all map to the read-failure state.
  • validateRecordedGitHubIssue MUST likewise distinguish a read failure (non-OK / thrown) from a
    successfully-read issue that fails its validity checks (wrong number, wrong url, not open, missing
    fingerprint marker, missing signals label). The validity failures MUST keep returning "not found" so the
    existing fall-through to the fingerprint search is unchanged.
  • fileUpstreamDriftIssues (src/upstream/ruleset.ts:349-381) MUST NOT call createGitHubDriftIssue when
    either lookup reported a read failure. It MUST instead count that report under the existing skipped counter
    and continue, leaving the report's stored issueNumber/issueUrl untouched.
  • The for (let page = 1; ; page += 1) loop MUST be bounded by a named module constant set to 10, matching
    the value used by COMMENT_SEARCH_PAGE_LIMIT / REVIEW_PAGE_LIMIT / MAX_WORKFLOW_RUN_LIST_PAGES /
    PR_DETAIL_MAX_PAGES. Exhausting the cap without a match MUST be reported as a read failure, not as
    "no match" — a truncated search is not evidence that no issue exists.
  • What must NOT change: the happy paths. A successful search that finds a fingerprint match MUST still PATCH
    (or skip via driftIssueUnchanged) exactly as today; a successful search that genuinely finds nothing MUST
    still create the issue exactly as today; the created/updated/unchanged counters and the
    upstream.drift_issues_filed audit event's metadata keys MUST keep their current names and meanings.

⚠️ Required pattern: mirror getLastActorForEvent's result shape in src/github/pr-actions.ts:292-300
a typed result that separates errored: true ("we learned NOTHING") from a completed scan that found no
match, with the caller failing conservative on errored. What does NOT satisfy this issue: (a) adding a
retry loop around the GitHub call instead of making the failure visible to the caller — a sustained
rate-limit still ends in a duplicate; (b) adding an idempotency marker/"does an issue with this title already
exist" second mechanism on the create path instead of fixing the existing lookup contract; (c) capping the
pagination loop but leaving the read-failure/not-found conflation in place; (d) a test-only PR.

Deliverables

  • findGitHubIssueForFingerprint in src/upstream/ruleset.ts returns a read-failure state for each of:
    a non-OK response on page 1, a non-OK response on page 2, a thrown timeoutFetch, and exhausting the new
    10-page cap with no match. Exact expectation: given a stub whose first GET …/issues?…&page=1 returns
    HTTP 403, fileUpstreamDriftIssues performs zero POST …/issues requests and reports
    { created: 0, skipped: 1 } for that report.
  • validateRecordedGitHubIssue in src/upstream/ruleset.ts returns a read-failure state on a non-OK
    response or a thrown fetch, and keeps returning "not found" when the fetched issue is closed, carries the
    wrong number/url, lacks the gittensory-upstream-drift:<fingerprint> body marker, or lacks the signals
    label.
  • A named page-cap constant in src/upstream/ruleset.ts with value 10, applied to the fingerprint-search
    loop, with a test asserting that a stub emitting link: <…>; rel="next" on every page issues exactly 10
    GETs and then reports a read failure (not a create).
  • Tests in test/unit/upstream-ruleset.test.ts covering: read-failure → no create; genuine empty search →
    create (unchanged); fingerprint match on page 2 → PATCH (unchanged); driftIssueUnchanged no-op path
    (unchanged).
  • A regression test at test/unit/upstream-ruleset.test.ts named for this bug (e.g.
    "REGRESSION: a failed fingerprint search must not file a duplicate drift issue") that asserts the
    report's stored issueNumber is left untouched after the failed pass.

All Deliverables above are required in a single PR. A PR that satisfies only some of them — for example one
that bounds the pagination loop and adds a cap test but leaves if (!response.ok) return null conflated with
"no match", so the duplicate-issue bug survives — 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 and packages/loopover-engine/src/**/*.ts; src/upstream/ruleset.ts is measured and
gated.

Every branch this change introduces or touches needs both arms tested: the page-cap comparison
(page <= CAP true and false), the !response.ok arm vs the OK arm on page 1 and on page 2+, the
catch arm, the link / rel="next" present-vs-absent arm, the new read-failure vs found-nothing arm at the
fileUpstreamDriftIssues call site, and both arms of validateRecordedGitHubIssue's read-failure split.

This change is NOT in packages/loopover-engine/src/**, so the dual-upload engine-coverage rule does not apply.

Expected Outcome

A GitHub read failure during the drift-issue reconcile leaves the existing issue alone and reports the report
as skipped, instead of silently filing a second issue for the same fingerprint on every failing cron tick and
re-pointing the stored report at the duplicate. The fingerprint search is also bounded to 10 pages, matching
every other pagination walk in the codebase, and a truncated search is treated as inconclusive rather than as
evidence of absence.

Links & Resources

  • src/upstream/ruleset.ts:1095-1126findGitHubIssueForFingerprint: unbounded loop, read failure → null
  • src/upstream/ruleset.ts:1154-1189validateRecordedGitHubIssue: same read-failure → null conflation
  • src/upstream/ruleset.ts:349-381 — the caller that falls straight through to createGitHubDriftIssue
  • src/github/pr-actions.ts:292-300LastCloserResult.errored, the result shape to mirror
  • src/github/backfill.ts:2433-2437 — the 10-page pagination-cap convention this loop is missing

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