Skip to content

github(client): make githubBypassResponseCache actually force an own network read instead of coalescing or falling back into the cache #10032

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

githubBypassResponseCache is documented as an absolute guarantee (src/github/client.ts:105-106):

  /** Force this request to hit GitHub instead of the persistent response cache; use for freshness/security reads. */
  githubBypassResponseCache?: boolean;

It has exactly one production caller: fetchLiveBaseBranchAdvancedAt (src/github/backfill.ts:3472-3487),
the force-fresh-rebase gate's live base-tip read (GET /repos/{o}/{r}/commits/{baseRef}), whose own doc
comment explains why it must be live — a base that advanced with a non-conflicting sibling commit still reads
mergeable_state: "clean", so this timestamp is the only thing that forces a rebase + CI recheck before
merge. It fails open: .catch(() => undefined) and the caller does not force a rebase, so a wrong answer
here merges on a stale base.

The flag does not deliver that guarantee. Two independent legs:

1. It reroutes the read into the volatile single-flight coalescer. timeoutFetch
(src/github/client.ts:635-638):

  const cls = method === "GET" && !conditional && !init?.githubBypassResponseCache ? githubCacheClassForUrl(url) : null;
  if (method === "GET" && !conditional && cls === null && isVolatileSingleFlightEligibleGithubUrl(url, headers)) {
    return fetchWithVolatileSingleFlight(input, init, volatileSingleFlightScope(url, headers));
  }

Setting the flag forces cls = null, which is precisely the condition that admits a URL to the coalescer.
/repos/{o}/{r}/commits/{ref} is not in isVolatileSingleFlightEligibleGithubUrl's exclusion list
(src/github/client.ts:332-354, which excludes only /contents, /git/trees|blobs/, /issues/{n}, and
/collaborators/{login}/permission), so the bypass read joins any concurrent identical in-flight read and is
answered by responseFromCached(replay, "coalesced") (src/github/client.ts:567-571) — never issuing a
request of its own. Without the flag this same URL is a commit-class cacheable read and is routed away from
the coalescer entirely, so the flag increases the coalescing surface it was written to remove. That is
exactly the hazard the exclusion list exists for, per its own comment (src/github/client.ts:341-350):
sharing one in-flight promise's outcome — "success OR a transient failure" — across genuinely independent
callers means "one caller's momentary fetch/rate-limit hiccup silently becomes every concurrent caller's
answer too". A coalesced transient failure on this read means no forced rebase.

2. The 404 fallback drops the flag entirely. githubJsonWithHeaders (src/github/backfill.ts:4873-4890)
sets it on the first request and then re-issues without it:

  let response = await timeoutFetch(url, {
    headers: githubRestHeaders(token, options?.validators),
    ...(options?.rateLimitAdmissionKey ? { githubRateLimitAdmission: true, githubRateLimitAdmissionKey: options.rateLimitAdmissionKey } : {}),
    ...(options?.bypassResponseCache ? { githubBypassResponseCache: true } : {}),
  });
  
  if (response.status === 404 && token && token === env.GITHUB_PUBLIC_TOKEN) {
    response = await timeoutFetch(url, { headers: githubRestHeaders(undefined, options?.validators) });

The unauthenticated retry omits githubBypassResponseCache, so on that path the read becomes an ordinary
cacheable commit-class GET and can be answered from the persistent response cache — with up to
GITHUB_COMMIT_CACHE_TTL_SECONDS (default 15 minutes, src/github/client.ts:28) of staleness — for a call
whose entire purpose is liveness. The admission-key omission on that same retry IS deliberate and documented
two lines below (src/github/backfill.ts:4887-4889); the bypass omission is not mentioned anywhere.

Requirements

  • timeoutFetch MUST NOT route a request carrying githubBypassResponseCache: true into
    fetchWithVolatileSingleFlight. Such a request MUST go straight to fetchWithGitHubRetry and MUST NOT
    publish its response into inFlightVolatileGets for another caller to replay.
  • The volatile single-flight path MUST be unchanged for every request that does NOT set the flag: the same
    URLs coalesce, the same exclusion list applies, and recordGitHubCacheMetric("coalesced"|"bypassed", "sensitive")
    is emitted exactly as today.
  • githubJsonWithHeaders' 404 unauthenticated retry (src/github/backfill.ts:4886) MUST carry
    githubBypassResponseCache: true whenever options?.bypassResponseCache was set on the first request.
  • The 404 retry MUST continue to omit githubRateLimitAdmission / githubRateLimitAdmissionKey — that
    omission is deliberate and documented at src/github/backfill.ts:4887-4889 and MUST NOT be "fixed".
  • recordGitHubResponse's existing suppression for bypass/replay reads (src/github/backfill.ts:4881-4883)
    MUST be unchanged.
  • Do NOT add /commits/{ref} to isVolatileSingleFlightEligibleGithubUrl's exclusion list — that would
    de-coalesce the two ordinary hourly resolveUpstreamCommitSha reads (src/upstream/commit.ts:33-47) that
    deliberately share that path, which is a separate behaviour change with its own blast radius.
  • Do NOT change GITHUB_COMMIT_CACHE_TTL_SECONDS, DEFAULT_COMMIT_TTL_SECONDS, or any cache class/TTL.

⚠️ Required pattern: gate the volatile-single-flight branch on the flag the same way the cache branch already
is at src/github/client.ts:635 (!init?.githubBypassResponseCache), and propagate the option on the retry
exactly as the first request spreads it at src/github/backfill.ts:4876. What does NOT satisfy this issue:
(a) adding /commits/ to the volatile exclusion list, which changes behaviour for every non-bypass caller;
(b) introducing a second "really really bypass" flag alongside the existing one instead of making the
existing one correct; (c) fixing only the timeoutFetch leg and leaving the 404 retry dropping the option
(or vice versa) — both legs are required; (d) a test-only PR.

Deliverables

  • timeoutFetch in src/github/client.ts sends a githubBypassResponseCache: true GET straight to the
    network. Exact expectation: with a response cache installed and two concurrent
    timeoutFetch("https://github.com/ghapi/repos/o/r/commits/main", { githubBypassResponseCache: true })
    calls, the underlying fetch is invoked twice, and neither response carries the
    x-loopover-cache header (GITHUB_RESPONSE_CACHE_REPLAY_HEADER).
  • A test in test/unit/github-client.test.ts asserting a bypass GET issued concurrently with a
    non-bypass GET for the same URL does not receive the other's replayed body: the bypass caller's response
    body must be the one its own fetch returned.
  • A test in test/unit/github-client.test.ts pinning the unchanged case: two concurrent NON-bypass GETs
    for a volatile-eligible URL still coalesce to a single fetch and the joiner's response carries
    x-loopover-cache: coalesced.
  • githubJsonWithHeaders' 404 unauthenticated retry in src/github/backfill.ts carries
    githubBypassResponseCache: true when the caller set bypassResponseCache, with a test in
    test/unit/backfill.test.ts asserting that a bypassResponseCache: true call whose first (public-token)
    request 404s issues a second request that also bypasses the cache — i.e. the second response is not a
    cache replay even when a matching entry is present in the installed response cache.
  • A test in test/unit/backfill.test.ts asserting the 404 retry still sends no githubRateLimitAdmission
    / githubRateLimitAdmissionKey (the documented, deliberate omission).
  • A regression test at test/unit/github-client.test.ts named for this bug (e.g.
    "REGRESSION: githubBypassResponseCache must not be answered by the volatile single-flight coalescer").

All Deliverables above are required in a single PR. A PR that satisfies only some of them — for example one
that adds !init?.githubBypassResponseCache to the volatile branch but leaves the 404 retry in
src/github/backfill.ts:4886 dropping the option — 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; both src/github/client.ts and
src/github/backfill.ts are measured and gated.

Both arms of every touched branch need a test: the new githubBypassResponseCache term in the
volatile-single-flight condition (flag set → straight to network; flag unset → coalesce, for a
volatile-eligible URL); the cls === null and cls !== null paths that reach it; the
options?.bypassResponseCache ? … : {} spread on the 404 retry (present and absent); and the
response.status === 404 && token === env.GITHUB_PUBLIC_TOKEN guard (both arms). clearGitHubResponseCacheForTest()
(src/github/client.ts:684-689) must be used between cases so inFlightVolatileGets state does not leak.

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

Expected Outcome

A read that sets githubBypassResponseCache provably performs its own network request: it is never answered by
another caller's in-flight response, never publishes its own into the volatile coalescer, and never silently
falls back onto the persistent response cache via the 404 unauthenticated retry. The force-fresh-rebase gate's
base-tip timestamp is therefore live, rather than being able to inherit a concurrent caller's transient failure
or a cached value up to 15 minutes old.

Links & Resources

  • src/github/client.ts:105-106 — the flag's documented "force this request to hit GitHub" contract
  • src/github/client.ts:635-638 — the bypass flag forcing cls = null, which admits the request to the coalescer
  • src/github/client.ts:332-354isVolatileSingleFlightEligibleGithubUrl and its exclusion-list rationale
  • src/github/client.ts:562-601fetchWithVolatileSingleFlight, which replays another caller's body
  • src/github/backfill.ts:4873-4890 — the first request setting the flag and the 404 retry dropping it
  • src/github/backfill.ts:3464-3487fetchLiveBaseBranchAdvancedAt, the only production caller
  • src/upstream/commit.ts:33-47 — the ordinary /commits/{ref} reads that must keep coalescing

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