diff --git a/.env.example b/.env.example index 4948613fb7..abeab92c44 100644 --- a/.env.example +++ b/.env.example @@ -173,6 +173,7 @@ REDIS_URL=redis://redis:6379 # REQUIRED for the self-host review # GITHUB_CACHE_TTL_SECONDS=20 # Short default Redis TTL for safe GitHub GET response caching. Set 0 to disable. # GITHUB_BRANCH_PROTECTION_CACHE_TTL_SECONDS=1200 # TTL for required-status branch protection reads. # GITHUB_METADATA_CACHE_TTL_SECONDS=600 # TTL for stable repo/user/installation metadata reads. +# GITHUB_COMMIT_CACHE_TTL_SECONDS=900 # TTL for bare /commits/{ref} resolves; dedups the two hourly upstream ref→SHA reads. # QDRANT_URL= # set to http://qdrant:6333 to use Qdrant as the RAG vector store # # (--profile qdrant). Overrides the built-in sqlite-vec / pgvector. # DISCORD_WEBHOOK_URL= # one Discord channel for per-action notifications (merged/closed/ diff --git a/src/github/client.ts b/src/github/client.ts index ee76d0acd7..7b278d4b55 100644 --- a/src/github/client.ts +++ b/src/github/client.ts @@ -23,6 +23,10 @@ const GITHUB_REST_RATE_LIMIT_OBSERVATION_METRIC = "gittensory_github_rest_rate_l const GITHUB_REST_RATE_LIMIT_RESPONSE_METRIC = "gittensory_github_rest_rate_limit_responses_total"; const DEFAULT_BRANCH_PROTECTION_TTL_SECONDS = 20 * 60; const DEFAULT_METADATA_TTL_SECONDS = 10 * 60; +// A bare `/commits/{ref}` read resolves a ref to its HEAD commit — mutable (a branch moves), so cache it only +// briefly. Long enough to dedup the two upstream ref→SHA resolves that fire in the SAME hourly window (scoring + +// drift), short enough that the pinned SHA is never meaningfully stale. +const DEFAULT_COMMIT_TTL_SECONDS = 15 * 60; export const GITHUB_RESPONSE_CACHE_REPLAY_HEADER = "x-gittensory-cache"; /** A shared cache for safe GitHub GET responses (e.g. Redis on the self-host). Stores only status/body/ @@ -45,13 +49,18 @@ export function setGitHubResponseCache(cache: GitHubResponseCache | null): void responseCache = cache; } -export type GitHubCacheClass = "branch_protection" | "metadata"; +export type GitHubCacheClass = "branch_protection" | "metadata" | "commit"; type EnvLookup = Record; export type GitHubTimeoutFetchInit = RequestInit & { /** Opt in to using this response's REST bucket headers for self-host queue admission control. */ githubRateLimitAdmission?: boolean; /** Stable actor key for admission control. Installation-token reads should use the installation id. */ githubRateLimitAdmissionKey?: string; + /** Consulted ONLY when this GET is about to make a NETWORK read (a cache hit is always served first, for free). + * Return true to skip the network read — timeoutFetch then resolves to a synthetic 503 so a best-effort caller + * can fall back without spending a REST request. Lets a budget-gate suppress fresh reads while still serving + * free cache hits under pressure. */ + githubSkipNetworkWhen?: () => boolean | Promise; }; export type GitHubRateLimitAdmissionKey = string; export type LocalGitHubRestRateLimitObservation = { @@ -99,6 +108,9 @@ function githubCacheClassForUrl(url: string): GitHubCacheClass | null { if (!url.startsWith(`${GITHUB_API_PREFIX}/`)) return null; const path = githubApiPath(url); if (/^\/repos\/[^/]+\/[^/]+\/branches\/[^/]+\/protection\/required_status_checks(?:$|[?#])/.test(path)) return "branch_protection"; + // A BARE `/repos/{o}/{r}/commits/{ref}` read (no `/check-runs`, `/status`, `/pulls`, … suffix) resolves a ref to + // its HEAD commit. Only the two upstream ref→SHA resolves use this shape; caching it briefly dedups them. + if (/^\/repos\/[^/]+\/[^/]+\/commits\/[^/?#]+(?:$|[?#])/.test(path)) return "commit"; if ( (/^\/users\/[^/?#]+(?:$|[?#])/.test(path) || /^\/repos\/[^/?#]+\/[^/?#]+(?:$|[?#])/.test(path) || @@ -122,6 +134,9 @@ export function githubResponseCacheTtlSeconds(cls: GitHubCacheClass, env: EnvLoo if (cls === "branch_protection") { return positiveEnvSeconds(env, "GITHUB_BRANCH_PROTECTION_CACHE_TTL_SECONDS", DEFAULT_BRANCH_PROTECTION_TTL_SECONDS); } + if (cls === "commit") { + return positiveEnvSeconds(env, "GITHUB_COMMIT_CACHE_TTL_SECONDS", DEFAULT_COMMIT_TTL_SECONDS); + } return positiveEnvSeconds(env, "GITHUB_METADATA_CACHE_TTL_SECONDS", DEFAULT_METADATA_TTL_SECONDS); } @@ -444,6 +459,17 @@ function waitForVolatileReplay(shared: Promise, sig // A 12s hard cap on every GitHub request. Centralised here so the app token/installation raw fetches plus comment / // label / check-run / pr-action Octokit helpers all inherit the cache boundary, retry, and timeout behavior. +/** A caller's githubSkipNetworkWhen opts a best-effort read out of the NETWORK (never out of the cache). */ +async function shouldSkipGitHubNetworkRead(init?: GitHubTimeoutFetchInit): Promise { + return init?.githubSkipNetworkWhen ? Boolean(await init.githubSkipNetworkWhen()) : false; +} + +// A synthetic non-OK returned when githubSkipNetworkWhen suppresses a network read (e.g. a budget-gated best-effort +// resolve). The caller detects !response.ok and falls back without spending a REST request. +function githubNetworkSkippedResponse(): Response { + return new Response(null, { status: 503, headers: { [GITHUB_RESPONSE_CACHE_REPLAY_HEADER]: "network-skip" } }); +} + export async function timeoutFetch(input: RequestInfo | URL, init?: GitHubTimeoutFetchInit): Promise { const method = requestMethod(input, init); const url = requestUrl(input); @@ -455,6 +481,8 @@ export async function timeoutFetch(input: RequestInfo | URL, init?: GitHubTimeou } const useCache = responseCache !== null && cls !== null; if (!useCache) { + // No cache to hit → this IS a network read, so honor a caller's budget-gate before spending the request. + if (await shouldSkipGitHubNetworkRead(init)) return githubNetworkSkippedResponse(); recordGitHubCacheMetric("bypassed", cacheBypassClass(method, url, headers)); return fetchWithGitHubRetry(input, init); } @@ -479,6 +507,10 @@ export async function timeoutFetch(input: RequestInfo | URL, init?: GitHubTimeou if (replay) return responseFromCached(replay, "coalesced"); } + // Cache MISS with no in-flight fetch to coalesce onto → a fresh network read. Honor a caller's budget-gate here + // (AFTER the cache-hit + coalesce checks, so a free cached/in-flight result is never suppressed by budget pressure). + if (await shouldSkipGitHubNetworkRead(init)) return githubNetworkSkippedResponse(); + const request = fetchAndMaybeCacheGitHubGet(input, init, url, cacheKey, cls).then( (result) => ({ ok: true as const, result }), (error: unknown) => ({ ok: false as const, error }), diff --git a/src/scoring/model.ts b/src/scoring/model.ts index c25005eba8..712a93a402 100644 --- a/src/scoring/model.ts +++ b/src/scoring/model.ts @@ -4,6 +4,7 @@ import { } from "../db/repositories"; import { timeoutFetch } from "../github/client"; import { getLatestRegistrySnapshot } from "../registry/sync"; +import { resolveUpstreamCommitSha } from "../upstream/commit"; import { syncUnmodeledScoringConstantDrift } from "../upstream/unmodeled-scoring-drift"; import type { JsonValue, ScoringModelSnapshotRecord } from "../types"; import { errorMessage, nowIso } from "../utils/json"; @@ -77,19 +78,6 @@ function upstreamRawUrl(config: { repo: string; ref: string }, path: string): st return `https://raw.githubusercontent.com/${config.repo}/${encodeURIComponent(config.ref)}/${path}`; } -// Fetch the HEAD commit SHA of the upstream ref for audit trail. Fail-open: a network hiccup or a -// missing administration token must never block the constants refresh itself. -async function fetchUpstreamRefSha(upstream: { repo: string; ref: string }, token: string | undefined): Promise { - try { - const response = await timeoutFetch(`https://api.github.com/repos/${upstream.repo}/commits/${encodeURIComponent(upstream.ref)}`, { headers: githubHeaders(token, "application/vnd.github+json") }); - if (!response.ok) return null; - const data = (await response.json()) as { sha?: string }; - return typeof data.sha === "string" && data.sha.length > 0 ? data.sha : null; - } catch { - return null; - } -} - // Single source of truth (#812): every recognized upstream constant name is a key of // DEFAULT_SCORING_CONSTANTS, so the known-only parser, the unmodeled-drift detector, and the preview-side // fallbacks all derive from one place. The density-era constants are included because the density model is @@ -109,7 +97,7 @@ export async function refreshScoringModelSnapshot(env: Env): Promise { + return { + accept: "application/vnd.github+json", + "user-agent": "gittensory/0.1", + "x-github-api-version": "2022-11-28", + ...(token ? { authorization: `Bearer ${token}` } : {}), + }; +} + +/** + * Resolve an upstream ref (branch/tag) to its immutable HEAD commit SHA — the pin recorded by BOTH the + * scoring-model refresh (`refreshScoringModelSnapshot`) and the upstream-drift refresh. It was fetched twice per + * hour (one `GET /repos/{repo}/commits/{ref}` in each of those hourly jobs). This is the single shared resolver so: + * + * - **Dedup:** both jobs issue the identical bare `/commits/{ref}` read through `timeoutFetch`, which the + * self-host GitHub response cache serves from its short-TTL `commit` class — so within a window the ref + * resolves ONCE (a cache hit for the second job) instead of twice. + * - **Budget-gate:** it yields (returns null) when the shared REST budget is at/below the low-water floor, so + * this best-effort audit resolve never spends a scarce request during a rate-limit crunch (the same hourly + * window where the heavy maintenance fan-out runs). + * + * Fail-open: a rate-limit yield, a network/parse error, a non-OK status, or a missing SHA all return null — every + * caller already treats null as "fall back to the mutable ref", so a resolve failure never blocks the refresh. + */ +export async function resolveUpstreamCommitSha( + env: Env, + config: { repo: string; ref: string }, +): Promise { + try { + const response = await timeoutFetch( + `https://api.github.com/repos/${config.repo}/commits/${encodeURIComponent(config.ref)}`, + { + headers: upstreamCommitHeaders(env.GITHUB_PUBLIC_TOKEN), + // Budget-gate the NETWORK read only: a cached resolve is still served for free even under pressure; a fresh + // read is skipped (→ synthetic non-OK → null → caller falls back to the mutable ref) when the REST budget is + // at/below the low-water floor. This callback runs ONLY on a cache miss, so it never suppresses a cache hit. + githubSkipNetworkWhen: () => shouldWaitForGitHubRateLimit(env).then(Boolean), + }, + ); + if (!response.ok) return null; + const data = (await response.json()) as { sha?: string }; + return typeof data.sha === "string" && data.sha.length > 0 ? data.sha : null; + } catch { + return null; + } +} diff --git a/src/upstream/ruleset.ts b/src/upstream/ruleset.ts index 18c668f05b..a7b36418f1 100644 --- a/src/upstream/ruleset.ts +++ b/src/upstream/ruleset.ts @@ -11,6 +11,7 @@ import { upsertUpstreamDriftReport, } from "../db/repositories"; import { timeoutFetch } from "../github/client"; +import { resolveUpstreamCommitSha } from "./commit"; import { isGlobalAgentPause } from "../settings/agent-execution"; import { normalizeRegistryPayload } from "../registry/normalize"; import { DEFAULT_GITTENSOR_UPSTREAM_REF, DEFAULT_GITTENSOR_UPSTREAM_REPO, detectActiveModel, findUnmodeledConstantKeys, parsePythonNumberConstants } from "../scoring/model"; @@ -104,7 +105,7 @@ export type UpstreamStatus = { export async function refreshUpstreamSourceSnapshots(env: Env): Promise { const config = upstreamConfig(env); const fetchedAt = nowIso(); - const [previousByKey, commitSha] = await Promise.all([latestSourcesByKey(env), fetchUpstreamCommitSha(env, config)]); + const [previousByKey, commitSha] = await Promise.all([latestSourcesByKey(env), resolveUpstreamCommitSha(env, config)]); const snapshots = await Promise.all( TRACKED_SOURCES.map((source) => fetchTrackedSource(env, config, source, fetchedAt, commitSha, previousByKey.get(source.key))), ); @@ -412,18 +413,6 @@ async function latestSourcesByKey(env: Env): Promise [source.sourceKey, source])); } -async function fetchUpstreamCommitSha(env: Env, config: { repo: string; ref: string }): Promise { - const url = `https://api.github.com/repos/${config.repo}/commits/${encodeURIComponent(config.ref)}`; - try { - const response = await timeoutFetch(url, { headers: githubHeaders(env.GITHUB_PUBLIC_TOKEN, "application/vnd.github+json") }); - if (!response.ok) return null; - const payload = (await response.json()) as { sha?: string }; - return payload.sha ?? null; - } catch { - return null; - } -} - async function fetchTrackedSource( env: Env, config: { repo: string; ref: string }, diff --git a/test/unit/github-client.test.ts b/test/unit/github-client.test.ts index 7299e377b4..b37c37338c 100644 --- a/test/unit/github-client.test.ts +++ b/test/unit/github-client.test.ts @@ -585,8 +585,11 @@ describe("timeoutFetch", () => { it("resolves per-class cache TTL env overrides with safe fallbacks", () => { expect(githubResponseCacheTtlSeconds("branch_protection", {})).toBe(20 * 60); expect(githubResponseCacheTtlSeconds("metadata", {})).toBe(10 * 60); + expect(githubResponseCacheTtlSeconds("commit", {})).toBe(15 * 60); expect(githubResponseCacheTtlSeconds("branch_protection", { GITHUB_BRANCH_PROTECTION_CACHE_TTL_SECONDS: "3600" })).toBe(3600); expect(githubResponseCacheTtlSeconds("metadata", { GITHUB_METADATA_CACHE_TTL_SECONDS: "90.8" })).toBe(90); + expect(githubResponseCacheTtlSeconds("commit", { GITHUB_COMMIT_CACHE_TTL_SECONDS: "300" })).toBe(300); + expect(githubResponseCacheTtlSeconds("commit", { GITHUB_COMMIT_CACHE_TTL_SECONDS: "0" })).toBe(15 * 60); expect(githubResponseCacheTtlSeconds("branch_protection", { GITHUB_BRANCH_PROTECTION_CACHE_TTL_SECONDS: "" })).toBe(20 * 60); expect(githubResponseCacheTtlSeconds("metadata", { GITHUB_METADATA_CACHE_TTL_SECONDS: "0" })).toBe(10 * 60); expect(githubResponseCacheTtlSeconds("metadata", { GITHUB_METADATA_CACHE_TTL_SECONDS: "0.5" })).toBe(10 * 60); @@ -594,6 +597,15 @@ describe("timeoutFetch", () => { expect(githubResponseCacheTtlSeconds("metadata", { GITHUB_METADATA_CACHE_TTL_SECONDS: "Infinity" })).toBe(10 * 60); }); + it("caches a BARE /commits/{ref} resolve (the upstream ref→SHA read) but NOT its suffixed CI subresources", () => { + expect(isCacheableGithubUrl("https://api.github.com/repos/entrius/gittensor/commits/main")).toBe(true); + expect(isCacheableGithubUrl("https://api.github.com/repos/o/r/commits/abc123?foo=1")).toBe(true); + // The mutable CI/pulls subresources under /commits/{sha} must still always hit the live API. + expect(isCacheableGithubUrl("https://api.github.com/repos/o/r/commits/abc/status")).toBe(false); + expect(isCacheableGithubUrl("https://api.github.com/repos/o/r/commits/abc/check-runs?per_page=100")).toBe(false); + expect(isCacheableGithubUrl("https://api.github.com/repos/o/r/commits/abc/pulls")).toBe(false); + }); + it("uses configured TTL overrides for stable GitHub metadata and branch-protection reads", async () => { vi.stubEnv("GITHUB_BRANCH_PROTECTION_CACHE_TTL_SECONDS", "3600"); vi.stubEnv("GITHUB_METADATA_CACHE_TTL_SECONDS", "900"); diff --git a/test/unit/upstream-commit.test.ts b/test/unit/upstream-commit.test.ts new file mode 100644 index 0000000000..45647e8595 --- /dev/null +++ b/test/unit/upstream-commit.test.ts @@ -0,0 +1,97 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { setGitHubResponseCache, type CachedGitHubResponse } from "../../src/github/client"; + +// Only shouldWaitForGitHubRateLimit is mocked (the budget signal); the real response-cache path in timeoutFetch +// runs so the dedup can be exercised end-to-end. +const rateLimitMock = vi.hoisted(() => ({ shouldWaitForGitHubRateLimit: vi.fn() })); +vi.mock("../../src/github/rate-limit", async (importActual) => ({ + ...(await importActual()), + shouldWaitForGitHubRateLimit: rateLimitMock.shouldWaitForGitHubRateLimit, +})); + +import { resolveUpstreamCommitSha } from "../../src/upstream/commit"; + +const config = { repo: "entrius/gittensor", ref: "main" }; +const env = { GITHUB_PUBLIC_TOKEN: "pub-tok" } as unknown as Env; + +afterEach(() => { + setGitHubResponseCache(null); + vi.unstubAllGlobals(); + vi.clearAllMocks(); +}); + +describe("resolveUpstreamCommitSha — one shared, cached, budget-gated upstream ref→SHA resolve (#1942)", () => { + it("resolves the ref to its HEAD commit SHA via a single bare /commits/{ref} read", async () => { + rateLimitMock.shouldWaitForGitHubRateLimit.mockResolvedValue(undefined); + const calls: string[] = []; + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + calls.push(String(input)); + return Response.json({ sha: "abc123def" }); + }); + expect(await resolveUpstreamCommitSha(env, config)).toBe("abc123def"); + expect(calls).toEqual(["https://api.github.com/repos/entrius/gittensor/commits/main"]); + }); + + it("budget-gates: yields null WITHOUT any request when the shared REST budget is depleted", async () => { + rateLimitMock.shouldWaitForGitHubRateLimit.mockResolvedValue("2099-01-01T00:00:00.000Z"); + const fetchSpy = vi.fn(); + vi.stubGlobal("fetch", fetchSpy); + expect(await resolveUpstreamCommitSha(env, config)).toBeNull(); + expect(fetchSpy).not.toHaveBeenCalled(); + }); + + it("dedups: a second resolve within the window is served from the response cache — ONE /commits read for both", async () => { + rateLimitMock.shouldWaitForGitHubRateLimit.mockResolvedValue(undefined); + const store = new Map(); + setGitHubResponseCache({ get: async (k) => store.get(k) ?? null, set: async (k, v) => void store.set(k, v) }); + let fetches = 0; + vi.stubGlobal("fetch", async () => { + fetches += 1; + return Response.json({ sha: "shared-sha" }); + }); + // Two jobs (scoring + drift) resolving the SAME upstream ref within the cache window. + expect(await resolveUpstreamCommitSha(env, config)).toBe("shared-sha"); + expect(await resolveUpstreamCommitSha(env, config)).toBe("shared-sha"); + expect(fetches).toBe(1); + }); + + it("still serves a CACHED resolve under budget pressure — only a FRESH network read is gated (#1998 review)", async () => { + const store = new Map(); + setGitHubResponseCache({ get: async (k) => store.get(k) ?? null, set: async (k, v) => void store.set(k, v) }); + let fetches = 0; + vi.stubGlobal("fetch", async () => { + fetches += 1; + return Response.json({ sha: "cached-sha" }); + }); + // 1) Budget OK → the first resolve populates the cache (one network read). + rateLimitMock.shouldWaitForGitHubRateLimit.mockResolvedValue(undefined); + expect(await resolveUpstreamCommitSha(env, config)).toBe("cached-sha"); + // 2) Budget now DEPLETED — a fresh read would be gated, but the cached resolve is served for free. + rateLimitMock.shouldWaitForGitHubRateLimit.mockResolvedValue("2099-01-01T00:00:00.000Z"); + expect(await resolveUpstreamCommitSha(env, config)).toBe("cached-sha"); + expect(fetches).toBe(1); // no new network read despite budget pressure — the cache hit was served + }); + + it("budget-gates a cache MISS: with the response cache ON but empty, still skips the network (null) and makes no request", async () => { + rateLimitMock.shouldWaitForGitHubRateLimit.mockResolvedValue("2099-01-01T00:00:00.000Z"); + setGitHubResponseCache({ get: async () => null, set: async () => undefined }); + const fetchSpy = vi.fn(); + vi.stubGlobal("fetch", fetchSpy); + expect(await resolveUpstreamCommitSha(env, config)).toBeNull(); + expect(fetchSpy).not.toHaveBeenCalled(); + }); + + it("fails open (null) on a non-OK status, a missing/empty sha, or a thrown fetch", async () => { + rateLimitMock.shouldWaitForGitHubRateLimit.mockResolvedValue(undefined); + vi.stubGlobal("fetch", async () => new Response("nope", { status: 404 })); + expect(await resolveUpstreamCommitSha(env, config)).toBeNull(); + vi.stubGlobal("fetch", async () => Response.json({ sha: "" })); + expect(await resolveUpstreamCommitSha(env, config)).toBeNull(); + vi.stubGlobal("fetch", async () => Response.json({})); + expect(await resolveUpstreamCommitSha(env, config)).toBeNull(); + vi.stubGlobal("fetch", async () => { + throw new Error("network down"); + }); + expect(await resolveUpstreamCommitSha(env, config)).toBeNull(); + }); +});