From 53e6d35524cd5f67e236aed24913bf463e47a7da Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Thu, 9 Jul 2026 17:48:00 -0700 Subject: [PATCH 1/8] fix(review): make submitter-reputation burst/AI-spend defense install-wide for confirmed miners getSubmitterReputation's burst/low-sample thresholds are scoped WHERE project = ? AND submitter = ? -- a single repo. A fleet identity spreading a handful of gate-passing-but-low-value PRs across dozens of repos in one self-hosted install never accumulates enough same-repo sample density to read as "burst" or "low" anywhere, so the reputation defense never fires for it and every one of its submissions burns full paid AI-review spend indefinitely. - New getSubmitterReputationAcrossInstall in submitter-reputation.ts: the identical quality-weighted signal derivation, scoped by review_targets.installation_id instead of project (that column already existed, migrations/0050; this adds the supporting index). - New getEffectiveSubmitterReputation in reputation-wire.ts: the per-repo signal, additionally widened to the install-wide view for a CONFIRMED official Gittensor miner -- but only when the per-repo signal alone doesn't already justify caution, so an ordinary contributor or an already-flagged submitter pays no extra lookup. - Extracted the miner-identity check (shared with #4512) into src/gittensor/miner-detection-cache.ts so both this module and unlinked-issue-guardrail.ts can use it without a circular import through processors.ts. - Wired into both call sites: shouldSkipAiForReputation (the main AI-spend gate) and the vision-review reputation check. Fixes #4513 --- ...iew_targets_installation_submitter_idx.sql | 8 + src/gittensor/miner-detection-cache.ts | 30 ++++ src/queue/processors.ts | 5 +- src/review/reputation-wire.ts | 45 +++++- src/review/submitter-reputation.ts | 38 +++++ test/unit/miner-detection-cache.test.ts | 97 +++++++++++ test/unit/reputation-wiring.test.ts | 153 ++++++++++++++++++ test/unit/submitter-reputation.test.ts | 62 +++++++ 8 files changed, 432 insertions(+), 6 deletions(-) create mode 100644 migrations/0130_review_targets_installation_submitter_idx.sql create mode 100644 src/gittensor/miner-detection-cache.ts create mode 100644 test/unit/miner-detection-cache.test.ts diff --git a/migrations/0130_review_targets_installation_submitter_idx.sql b/migrations/0130_review_targets_installation_submitter_idx.sql new file mode 100644 index 0000000000..2c092c857a --- /dev/null +++ b/migrations/0130_review_targets_installation_submitter_idx.sql @@ -0,0 +1,8 @@ +-- Supports the install-wide submitter-reputation read (#4513): a fleet identity spreading thin across many +-- repos in one self-hosted install never accumulates enough same-repo sample density for the per-repo +-- reputation signal (src/review/submitter-reputation.ts) to ever fire, since that signal is scoped to +-- `WHERE project = ? AND submitter = ?`. review_targets already carries installation_id (migrations/0050), +-- so a CONFIRMED official Gittensor miner can additionally be evaluated across every repo in the same +-- install via `WHERE installation_id = ? AND submitter = ?` -- this index makes that query as cheap as the +-- existing per-project one instead of a full-table scan. +CREATE INDEX IF NOT EXISTS idx_review_targets_installation_submitter_terminal ON review_targets (installation_id, submitter, terminal_at); diff --git a/src/gittensor/miner-detection-cache.ts b/src/gittensor/miner-detection-cache.ts new file mode 100644 index 0000000000..60ca216919 --- /dev/null +++ b/src/gittensor/miner-detection-cache.ts @@ -0,0 +1,30 @@ +// Minimal, cached "is this login a CONFIRMED official Gittensor miner" check (#4512/#4513), shared by call +// sites that need only a boolean identity check -- not the full audit-logged flow processors.ts's +// getCachedOfficialMinerDetection uses for PR-comment command authorization. Same cache table/TTLs, no +// audit-log side effect. Deliberately its own module (not exported from processors.ts) so review/-layer +// code (unlinked-issue-guardrail.ts, reputation-wire.ts) can import it without a circular dependency — +// processors.ts is the one that imports FROM those modules. + +import { getFreshOfficialMinerDetection, upsertOfficialMinerDetection } from "../db/repositories"; +import { fetchOfficialGittensorMiner } from "./api"; + +const OFFICIAL_MINER_DETECTION_TTL_MS = 5 * 60 * 1000; +const OFFICIAL_MINER_DETECTION_UNAVAILABLE_TTL_MS = 60 * 1000; + +/** Fail-safe: any lookup failure resolves to "not a confirmed miner," never the reverse. */ +export async function isConfirmedOfficialMiner(env: Env, login: string): Promise { + const cached = await getFreshOfficialMinerDetection(env, login).catch(() => null); + if (cached) return cached.status === "confirmed"; + // fetchOfficialGittensorMiner already converts every failure into a returned {status: "unavailable"} + // value rather than rejecting -- nothing to catch here. + const detection = await fetchOfficialGittensorMiner(login); + // A cache-write failure must never block the caller from using the freshly-fetched (just uncached) + // detection -- worst case, the next call re-fetches instead of hitting the cache. + const cacheable = await upsertOfficialMinerDetection( + env, + login, + detection, + detection.status === "unavailable" ? OFFICIAL_MINER_DETECTION_UNAVAILABLE_TTL_MS : OFFICIAL_MINER_DETECTION_TTL_MS, + ).catch(() => detection); + return cacheable.status === "confirmed"; +} diff --git a/src/queue/processors.ts b/src/queue/processors.ts index a4e82c9512..bd33a77c44 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -520,6 +520,7 @@ import { evaluateWithSurfaceLane } from "../review/content-lane-wire"; import { reviewThreadBlockerFinding } from "../review/review-thread-findings"; import { indexRepo, reindexChangedPaths } from "../review/rag-index"; import { + getEffectiveSubmitterReputation, isReputationEnabled, recordReputationOutcome, shouldSkipAiForReputation, @@ -569,7 +570,7 @@ import { } from "../review/outcomes-wire"; import { neutralHoldReasonCode, nativeGateActionFromConclusion, recordNativeGateDecision } from "../review/parity-wire"; import { recordContributorGateDecision } from "../review/contributor-calibration"; -import { getSubmitterReputation, type SubmissionOutcome } from "../review/submitter-reputation"; +import type { SubmissionOutcome } from "../review/submitter-reputation"; import type { AdvisoryFinding, AiContentBlock, @@ -8449,7 +8450,7 @@ export async function runVisualVisionForAdvisory( ): Promise { if (args.mode === "paused" || args.routes.length === 0) return; try { - const visionReputation = await getSubmitterReputation(env, args.repoFullName, args.author ?? undefined); + const visionReputation = await getEffectiveSubmitterReputation(env, { repoFullName: args.repoFullName, submitter: args.author ?? undefined }); // BYOK resolution mirrors runAiReviewForAdvisory's own (re-resolved per-caller is this codebase's // established convention for this exact 3-line block, not an anti-pattern — see e.g. runAiSlopForAdvisory). const storedVisionKey = diff --git a/src/review/reputation-wire.ts b/src/review/reputation-wire.ts index 04461c1287..0eec7d8ee1 100644 --- a/src/review/reputation-wire.ts +++ b/src/review/reputation-wire.ts @@ -13,11 +13,15 @@ // the AI-spend decision (private, server-side) and writes the private submitter_stats table. Fully fail-safe: // the ported module degrades to "neutral" / no-op on any DB error, so this never throws into the gate. +import { getRepository } from "../db/repositories"; +import { isConfirmedOfficialMiner } from "../gittensor/miner-detection-cache"; import { getSubmitterCadence, getSubmitterReputation, + getSubmitterReputationAcrossInstall, isMachinePacedCadence, recordSubmissionOutcome, + type ReputationConfig, type SubmissionOutcome, type SubmitterStats, } from "./submitter-reputation"; @@ -56,9 +60,39 @@ export function shouldDowngradeToDeterministic(stats: SubmitterStats): boolean { } /** - * Flag-gated, fail-safe: read the submitter's INTERNAL reputation and report whether the AI-spend gate should - * downgrade to a deterministic-only review. When the flag is OFF this returns false IMMEDIATELY — no DB read — - * so the AI-spend gate is byte-identical to today. `project` namespaces the per-(project, submitter) rows + * Resolve the EFFECTIVE reputation signal for a submitter (#4513): the per-repo signal from + * {@link getSubmitterReputation}, additionally widened to an install-wide view for a CONFIRMED official + * Gittensor miner — but ONLY when the per-repo signal alone doesn't already justify caution, so an ordinary + * (non-miner) submitter or one already flagged per-repo pays no extra lookup. Closes a real blind spot: a + * fleet identity spreading thin across many repos in one install never accumulates same-repo sample density, + * so the per-repo-only signal stays permanently "neutral" for it even while it burns full AI-review spend on + * every submission. Fail-safe throughout: an identity-check or install-wide-read failure just keeps the + * per-repo result, never throws, never upgrades a signal the per-repo read didn't already produce. + */ +export async function getEffectiveSubmitterReputation( + env: Env, + args: { repoFullName: string; submitter: string | null | undefined }, + cfg?: ReputationConfig, +): Promise { + const perRepo = await getSubmitterReputation(env, args.repoFullName, args.submitter ?? undefined, cfg); + if (shouldDowngradeToDeterministic(perRepo)) return perRepo; + const submitter = args.submitter?.trim(); + if (!submitter) return perRepo; + const isMiner = await isConfirmedOfficialMiner(env, submitter).catch(() => false); + if (!isMiner) return perRepo; + const repo = await getRepository(env, args.repoFullName).catch(() => null); + if (!repo?.installationId) return perRepo; + // getSubmitterReputationAcrossInstall already degrades to neutral internally on any read failure (mirrors + // getSubmitterReputation) -- nothing to catch here. + const acrossInstall = await getSubmitterReputationAcrossInstall(env, repo.installationId, submitter, cfg); + return shouldDowngradeToDeterministic(acrossInstall) ? acrossInstall : perRepo; +} + +/** + * Flag-gated, fail-safe: read the submitter's INTERNAL reputation (install-wide-aware for a confirmed miner, + * see {@link getEffectiveSubmitterReputation}) and report whether the AI-spend gate should downgrade to a + * deterministic-only review. When the flag is OFF this returns false IMMEDIATELY — no DB read — so the + * AI-spend gate is byte-identical to today. `project` namespaces the per-(project, submitter) rows * (gittensory uses the repo full name). NEVER throws: the ported module already degrades to neutral on error. * * Also checks submission CADENCE (#4514): every quality-based signal above only tells you whether a @@ -72,7 +106,10 @@ export async function shouldSkipAiForReputation( args: { project: string; submitter: string | null | undefined }, ): Promise { if (!isReputationEnabled(env)) return false; - const stats = await getSubmitterReputation(env, args.project, args.submitter ?? undefined); + // Combines both extensions to the base per-repo signal: install-wide widening for a confirmed miner + // (#4513, getEffectiveSubmitterReputation) first, then the cadence check (#4514) as an independent + // second signal -- neither subsumes the other, so both must run, not just whichever merged more recently. + const stats = await getEffectiveSubmitterReputation(env, { repoFullName: args.project, submitter: args.submitter }); if (shouldDowngradeToDeterministic(stats)) return true; const cadence = await getSubmitterCadence(env, args.project, args.submitter ?? undefined); return isMachinePacedCadence(cadence); diff --git a/src/review/submitter-reputation.ts b/src/review/submitter-reputation.ts index 04b9c5f19e..0549740b7d 100644 --- a/src/review/submitter-reputation.ts +++ b/src/review/submitter-reputation.ts @@ -308,3 +308,41 @@ export async function getSubmitterReputation(env: Env, project: string, submitte const decided = agg.merged + agg.closed; return { ...agg, closeRate: decided > 0 ? agg.closed / decided : 0, signal }; } + +/** Install-wide sibling of {@link getSubmitterReputation} (#4513): the SAME quality-weighted, recency-windowed + * signal derivation, but aggregated across EVERY repo `review_targets` has recorded for this installation_id + * (migrations/0050), not just one project. Closes a real blind spot: a fleet identity spreading thin across + * many repos in one self-hosted install never accumulates same-repo sample density for the per-project + * signal to ever leave "neutral," even while it burns full paid AI-review spend on every submission. Callers + * should reserve this for a CONFIRMED official Gittensor miner identity (this function does not itself check + * that) -- an ordinary contributor's reputation stays intentionally scoped per-repo. The all-time + * submitter_stats aggregate (submissions/merged/closed/manual, /stats-view only, not the signal) is NOT + * widened here: that table is keyed (project, submitter) with no installation_id column, and only the + * SIGNAL — not the display counts — gates the AI-spend decision. Fail-safe: any read error degrades to + * "neutral", identical to the per-project function. */ +export async function getSubmitterReputationAcrossInstall( + env: Env, + installationId: number, + submitter: string | undefined, + cfg: ReputationConfig = DEFAULT_REPUTATION_CONFIG, +): Promise { + const neutral: SubmitterStats = { submissions: 0, merged: 0, closed: 0, manual: 0, closeRate: 0, signal: "neutral" }; + if (!submitter) return neutral; + let signal: ReputationSignal = "neutral"; + try { + const result = await storage(env) + .prepare( + `SELECT status, json_extract(decision_json, '$.reasonCode') AS reasonCode + FROM review_targets + WHERE installation_id = ? AND submitter = ? AND terminal_at IS NOT NULL AND terminal_at >= datetime('now', ?) + ORDER BY terminal_at DESC LIMIT ?`, + ) + .bind(installationId, submitter, `-${cfg.windowDays} days`, REPUTATION_WINDOW_ROW_CAP) + .all<{ status: string; reasonCode: string | null }>(); + const rows = result?.results ?? []; + signal = signalFromCounts(countOutcomes(rows), cfg); + } catch { + signal = "neutral"; // fail-safe — never throw into the gate. + } + return { ...neutral, signal }; +} diff --git a/test/unit/miner-detection-cache.test.ts b/test/unit/miner-detection-cache.test.ts new file mode 100644 index 0000000000..cd5b7487e6 --- /dev/null +++ b/test/unit/miner-detection-cache.test.ts @@ -0,0 +1,97 @@ +import { describe, expect, it, vi } from "vitest"; +import { isConfirmedOfficialMiner } from "../../src/gittensor/miner-detection-cache"; +import { createTestEnv } from "../helpers/d1"; + +function stubMinerFetch(githubUsername: string) { + return vi.fn(async (input: RequestInfo | URL) => { + const url = input.toString(); + if (url === "https://api.gittensor.io/miners") return Response.json([{ githubUsername, githubId: "123", totalPrs: 2, totalMergedPrs: 2, isEligible: true, credibility: 1 }]); + if (url === "https://api.gittensor.io/miners/123/prs") return Response.json([]); + if (url === "https://api.gittensor.io/miners/123") return Response.json({}); + if (url === "https://mirror.gittensor.io/api/v1/miners/123/issues") return Response.json({ issues: [] }); + return Response.json({}); + }); +} + +describe("isConfirmedOfficialMiner (#4513, shared with #4512's unlinked-issue-guardrail)", () => { + it("resolves true for a login present in the /miners roster", async () => { + const env = createTestEnv(); + vi.stubGlobal("fetch", stubMinerFetch("farmer99")); + try { + expect(await isConfirmedOfficialMiner(env, "farmer99")).toBe(true); + } finally { + vi.unstubAllGlobals(); + } + }); + + it("resolves false for a login absent from an empty /miners roster (not_found)", async () => { + const env = createTestEnv(); + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + const url = input.toString(); + if (url === "https://api.gittensor.io/miners") return Response.json([]); + return Response.json({}); + }); + try { + expect(await isConfirmedOfficialMiner(env, "farmer99")).toBe(false); + } finally { + vi.unstubAllGlobals(); + } + }); + + it("resolves false (fail-safe) when the Gittensor API itself is unavailable", async () => { + const env = createTestEnv(); + vi.stubGlobal("fetch", async () => { + throw new Error("network down"); + }); + try { + expect(await isConfirmedOfficialMiner(env, "farmer99")).toBe(false); + } finally { + vi.unstubAllGlobals(); + } + }); + + it("a second call within the TTL hits the cache instead of re-fetching", async () => { + const env = createTestEnv(); + const fetchMock = stubMinerFetch("farmer99"); + vi.stubGlobal("fetch", fetchMock); + try { + expect(await isConfirmedOfficialMiner(env, "farmer99")).toBe(true); + const callsAfterFirst = fetchMock.mock.calls.length; + expect(callsAfterFirst).toBeGreaterThan(0); + expect(await isConfirmedOfficialMiner(env, "farmer99")).toBe(true); + expect(fetchMock.mock.calls.length).toBe(callsAfterFirst); // cache hit -- no additional fetch + } finally { + vi.unstubAllGlobals(); + } + }); + + it("a cache READ failure falls back to a fresh fetch rather than a false negative", async () => { + const env = createTestEnv(); + vi.stubGlobal("fetch", stubMinerFetch("farmer99")); + const realPrepare = env.DB.prepare.bind(env.DB); + env.DB.prepare = ((sql: string) => { + if (/SELECT.*FROM.*official_miner_detections/i.test(sql)) throw new Error("d1 down"); + return realPrepare(sql); + }) as typeof env.DB.prepare; + try { + expect(await isConfirmedOfficialMiner(env, "farmer99")).toBe(true); + } finally { + vi.unstubAllGlobals(); + } + }); + + it("a cache WRITE failure still uses the freshly-fetched status for this call", async () => { + const env = createTestEnv(); + vi.stubGlobal("fetch", stubMinerFetch("farmer99")); + const realPrepare = env.DB.prepare.bind(env.DB); + env.DB.prepare = ((sql: string) => { + if (/INSERT INTO.*official_miner_detections/i.test(sql)) throw new Error("d1 down"); + return realPrepare(sql); + }) as typeof env.DB.prepare; + try { + expect(await isConfirmedOfficialMiner(env, "farmer99")).toBe(true); + } finally { + vi.unstubAllGlobals(); + } + }); +}); diff --git a/test/unit/reputation-wiring.test.ts b/test/unit/reputation-wiring.test.ts index e41b76e754..f7d3c9b0a9 100644 --- a/test/unit/reputation-wiring.test.ts +++ b/test/unit/reputation-wiring.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it, vi } from "vitest"; import { reputationOutcomeFromTerminalState, runAiReviewForAdvisory } from "../../src/queue/processors"; import { + getEffectiveSubmitterReputation, isReputationEnabled, recordReputationOutcome, shouldDowngradeToDeterministic, @@ -10,6 +11,30 @@ import { getSubmitterReputation, recordSubmissionOutcome } from "../../src/revie import { evaluateGateCheck } from "../../src/rules/advisory"; import type { Advisory, RepositorySettings } from "../../src/types"; import { createTestEnv } from "../helpers/d1"; +import { upsertRepositoryFromGitHub } from "../../src/db/repositories"; + +// Seeds one terminal review_targets row -- the raw table getSubmitterReputation(AcrossInstall) reads from +// (not part of the Drizzle schema; migrations/0050 is the source of truth for these columns). +async function seedReviewTarget( + env: Env, + args: { project: string; repo: string; number: number; installationId: number; submitter: string; status: string; reasonCode?: string | null }, +) { + await env.DB.prepare( + `INSERT INTO review_targets (id, project, kind, repo, number, installation_id, submitter, status, decision_json, terminal_at) + VALUES (?, ?, 'pull_request', ?, ?, ?, ?, ?, ?, CURRENT_TIMESTAMP)`, + ) + .bind( + `${args.project}:pull_request:${args.repo}#${args.number}`, + args.project, + args.repo, + args.number, + args.installationId, + args.submitter, + args.status, + args.reasonCode === undefined ? null : JSON.stringify({ reasonCode: args.reasonCode }), + ) + .run(); +} // A submitter who FLOODED the project with submissions but landed almost none — the burst anti-abuse pattern. async function seedSubmitter( @@ -201,6 +226,134 @@ describe("shouldSkipAiForReputation (helper)", () => { }); }); +describe("getEffectiveSubmitterReputation (#4513, install-wide for a confirmed miner)", () => { + function stubMinerFetch(githubUsername: string) { + return async (input: RequestInfo | URL) => { + const url = input.toString(); + if (url === "https://api.gittensor.io/miners") return Response.json([{ githubUsername, githubId: "123", totalPrs: 2, totalMergedPrs: 2, isEligible: true, credibility: 1 }]); + if (url === "https://api.gittensor.io/miners/123/prs") return Response.json([]); + if (url === "https://api.gittensor.io/miners/123") return Response.json({}); + if (url === "https://mirror.gittensor.io/api/v1/miners/123/issues") return Response.json({ issues: [] }); + return Response.json({}); + }; + } + + it("widens to the install-wide signal for a CONFIRMED miner when the per-repo signal alone stays neutral", async () => { + const env = createTestEnv(); + await upsertRepositoryFromGitHub(env, { name: "repo-a", full_name: "org/repo-a", owner: { login: "org" } }, 999); + // Only 1 sample on repo-a itself -- per-repo signal stays "neutral" (below minSample). + await seedReviewTarget(env, { project: "org/repo-a", repo: "org/repo-a", number: 1, installationId: 999, submitter: "farmer99", status: "closed", reasonCode: "dual_review_declined" }); + // But spread across OTHER repos in the SAME install, farmer99 has a clear serial-decline pattern. + for (let i = 0; i < 7; i++) { + await seedReviewTarget(env, { project: `org/repo-${i}`, repo: `org/repo-${i}`, number: i + 10, installationId: 999, submitter: "farmer99", status: "closed", reasonCode: "dual_review_declined" }); + } + vi.stubGlobal("fetch", stubMinerFetch("farmer99")); + try { + const rep = await getEffectiveSubmitterReputation(env, { repoFullName: "org/repo-a", submitter: "farmer99" }); + expect(rep.signal).toBe("low"); + } finally { + vi.unstubAllGlobals(); + } + }); + + it("does NOT widen for an UNCONFIRMED submitter with the identical cross-repo pattern -- stays per-repo neutral", async () => { + const env = createTestEnv(); + await upsertRepositoryFromGitHub(env, { name: "repo-a", full_name: "org/repo-a", owner: { login: "org" } }, 999); + await seedReviewTarget(env, { project: "org/repo-a", repo: "org/repo-a", number: 1, installationId: 999, submitter: "farmer99", status: "closed", reasonCode: "dual_review_declined" }); + for (let i = 0; i < 7; i++) { + await seedReviewTarget(env, { project: `org/repo-${i}`, repo: `org/repo-${i}`, number: i + 10, installationId: 999, submitter: "farmer99", status: "closed", reasonCode: "dual_review_declined" }); + } + // /miners returns an empty roster -- farmer99 resolves as "not_found", never "confirmed". + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + const url = input.toString(); + if (url === "https://api.gittensor.io/miners") return Response.json([]); + return Response.json({}); + }); + try { + const rep = await getEffectiveSubmitterReputation(env, { repoFullName: "org/repo-a", submitter: "farmer99" }); + expect(rep.signal).toBe("neutral"); + } finally { + vi.unstubAllGlobals(); + } + }); + + it("skips the miner-identity lookup entirely when the per-repo signal already justifies downgrading", async () => { + const env = createTestEnv(); + await upsertRepositoryFromGitHub(env, { name: "repo-a", full_name: "org/repo-a", owner: { login: "org" } }, 999); + // A clear per-repo serial-decline pattern on its own -- already "low" without any cross-repo help. + for (let i = 0; i < 7; i++) { + await seedReviewTarget(env, { project: "org/repo-a", repo: "org/repo-a", number: i, installationId: 999, submitter: "farmer99", status: "closed", reasonCode: "dual_review_declined" }); + } + const fetchMock = vi.fn(stubMinerFetch("farmer99")); + vi.stubGlobal("fetch", fetchMock); + try { + const rep = await getEffectiveSubmitterReputation(env, { repoFullName: "org/repo-a", submitter: "farmer99" }); + expect(rep.signal).toBe("low"); + // The miner-identity lookup (and therefore the cross-repo query) never ran -- unnecessary once the + // per-repo signal alone already justifies caution. + expect(fetchMock).not.toHaveBeenCalled(); + } finally { + vi.unstubAllGlobals(); + } + }); + + it("a CONFIRMED miner whose cross-repo history is ALSO clean falls back to the (neutral) per-repo result", async () => { + const env = createTestEnv(); + await upsertRepositoryFromGitHub(env, { name: "repo-a", full_name: "org/repo-a", owner: { login: "org" } }, 999); + // Only 1 sample per-repo AND only 1 sample install-wide -- neither crosses minSample, so both the + // per-repo AND the cross-repo reads land on "neutral"; the ternary's FALSE branch (acrossInstall doesn't + // justify downgrading) must return perRepo, not silently substitute the (also-neutral) acrossInstall. + await seedReviewTarget(env, { project: "org/repo-a", repo: "org/repo-a", number: 1, installationId: 999, submitter: "farmer99", status: "merged", reasonCode: "dual_review_approved" }); + vi.stubGlobal("fetch", stubMinerFetch("farmer99")); + try { + const rep = await getEffectiveSubmitterReputation(env, { repoFullName: "org/repo-a", submitter: "farmer99" }); + expect(rep.signal).toBe("neutral"); + } finally { + vi.unstubAllGlobals(); + } + }); + + it("fails safe to the per-repo result when the repository lookup itself throws", async () => { + const env = createTestEnv(); + await upsertRepositoryFromGitHub(env, { name: "repo-a", full_name: "org/repo-a", owner: { login: "org" } }, 999); + await seedReviewTarget(env, { project: "org/repo-a", repo: "org/repo-a", number: 1, installationId: 999, submitter: "farmer99", status: "merged", reasonCode: "dual_review_approved" }); + vi.stubGlobal("fetch", stubMinerFetch("farmer99")); + const realPrepare = env.DB.prepare.bind(env.DB); + env.DB.prepare = ((sql: string) => { + if (/FROM\s+["`]?repositories["`]?/i.test(sql)) throw new Error("d1 down"); + return realPrepare(sql); + }) as typeof env.DB.prepare; + try { + const rep = await getEffectiveSubmitterReputation(env, { repoFullName: "org/repo-a", submitter: "farmer99" }); + expect(rep.signal).toBe("neutral"); // per-repo result (also neutral here), never throws + } finally { + vi.unstubAllGlobals(); + } + }); + + it("returns the per-repo result unchanged when there is no submitter at all", async () => { + const env = createTestEnv(); + const rep = await getEffectiveSubmitterReputation(env, { repoFullName: "org/repo-a", submitter: undefined }); + expect(rep.signal).toBe("neutral"); + }); + + it("fails safe to the (already-neutral) per-repo result, without throwing, when the repo has no resolvable installationId", async () => { + const env = createTestEnv(); + // repo-a is never registered -> getRepository resolves to null -> nothing to widen with, even though + // farmer99 IS a confirmed miner and has a real cross-repo pattern recorded under installation 999. + await seedReviewTarget(env, { project: "org/repo-a", repo: "org/repo-a", number: 1, installationId: 999, submitter: "farmer99", status: "closed", reasonCode: "dual_review_declined" }); + vi.stubGlobal("fetch", stubMinerFetch("farmer99")); + try { + const rep = await getEffectiveSubmitterReputation(env, { repoFullName: "org/repo-a", submitter: "farmer99" }); + // Only 1 sample, below minSample -> the per-repo signal itself is neutral, and with no installationId to + // widen with the function must return exactly that (not throw, not fabricate a signal). + expect(rep.signal).toBe("neutral"); + } finally { + vi.unstubAllGlobals(); + } + }); +}); + describe("processGitHubWebhook records the reputation outcome on a terminal PR (flag-ON call site)", () => { it("FLAG-ON: a closed+merged PR webhook records a 'merged' outcome for the submitter", async () => { const { processJob } = await import("../../src/queue/processors"); diff --git a/test/unit/submitter-reputation.test.ts b/test/unit/submitter-reputation.test.ts index 6a18a6a8bc..fe7985f4de 100644 --- a/test/unit/submitter-reputation.test.ts +++ b/test/unit/submitter-reputation.test.ts @@ -6,6 +6,7 @@ import { DEFAULT_REPUTATION_CONFIG, getSubmitterCadence, getSubmitterReputation, + getSubmitterReputationAcrossInstall, isMachinePacedCadence, recordSubmissionOutcome, REPUTATION_WINDOW_DAYS, @@ -374,6 +375,67 @@ describe("getSubmitterCadence (D1, fail-safe) (#4514)", () => { }); }); +describe("getSubmitterReputationAcrossInstall (#4513)", () => { + function makeInstallEnv(opts: { windowRows: Row[]; boundInstallationId?: number[]; throwOnAll?: boolean }): Env { + return { + DB: { + prepare: () => ({ + bind: (installationId: number, _submitter: string, ..._rest: unknown[]) => { + opts.boundInstallationId?.push(installationId); + return { + all: async () => { + if (opts.throwOnAll) throw new Error("D1 boom"); + return { results: opts.windowRows.map((r) => ({ status: r.status, reasonCode: r.reasonCode })) }; + }, + }; + }, + }), + }, + } as unknown as Env; + } + + it("returns neutral with no submitter (early return, no DB touch)", async () => { + const rep = await getSubmitterReputationAcrossInstall({} as Env, 123, undefined); + expect(rep).toEqual({ submissions: 0, merged: 0, closed: 0, manual: 0, closeRate: 0, signal: "neutral" }); + }); + + it("derives the SAME quality-weighted signal as the per-project function, from installation-scoped rows", async () => { + // 8 recent submissions across (implicitly) many repos in the install, almost all genuine declines -> low. + const env = makeInstallEnv({ windowRows: rows(["merged", "dual_review_approved", 1], ["closed", "dual_review_declined", 7]) }); + const rep = await getSubmitterReputationAcrossInstall(env, 123, "farmer99"); + expect(rep.signal).toBe("low"); + }); + + it("binds the installation_id (not a project string) as the scoping parameter", async () => { + const bound: number[] = []; + const env = makeInstallEnv({ windowRows: [], boundInstallationId: bound }); + await getSubmitterReputationAcrossInstall(env, 456, "farmer99"); + expect(bound).toEqual([456]); + }); + + it("does NOT widen the all-time submitter_stats aggregate -- submissions/merged/closed/manual stay zero (signal-only)", async () => { + const env = makeInstallEnv({ windowRows: rows(["merged", "dual_review_approved", 6]) }); + const rep = await getSubmitterReputationAcrossInstall(env, 123, "farmer99"); + expect(rep.submissions).toBe(0); + expect(rep.merged).toBe(0); + expect(rep.closeRate).toBe(0); + }); + + it("fail-safe: degrades to neutral when the install-wide query throws, never throws into the caller", async () => { + const env = makeInstallEnv({ windowRows: [], throwOnAll: true }); + const rep = await getSubmitterReputationAcrossInstall(env, 123, "farmer99"); + expect(rep.signal).toBe("neutral"); + }); + + it("respects a custom windowDays/minSample config the same way the per-project function does", async () => { + const cfg: ReputationConfig = { ...DEFAULT_REPUTATION_CONFIG, minSample: 100 }; + const env = makeInstallEnv({ windowRows: rows(["closed", "dual_review_declined", 8]) }); + const rep = await getSubmitterReputationAcrossInstall(env, 123, "farmer99", cfg); + // Only 8 samples, well under the raised minSample of 100 -> neutral regardless of how bad they look. + expect(rep.signal).toBe("neutral"); + }); +}); + // A minimal D1 stub: the first query (.first) returns submitter_stats; the window query (.all) returns the // review_targets rows. Both come off the same prepared-statement stub (the two call sites use .first vs .all). function makeEnv(opts: { statRow: { submissions: number; merged: number; closed: number; manual: number } | null; windowRows: Row[] }): Env { From e2ab1b2b78d599404ec58e3c6e3c81b33884d0b1 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Thu, 9 Jul 2026 17:51:31 -0700 Subject: [PATCH 2/8] fix: renumber migration 0130 -> 0131 (0130 was claimed by #4538, already merged to main) --- ...idx.sql => 0131_review_targets_installation_submitter_idx.sql} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename migrations/{0130_review_targets_installation_submitter_idx.sql => 0131_review_targets_installation_submitter_idx.sql} (100%) diff --git a/migrations/0130_review_targets_installation_submitter_idx.sql b/migrations/0131_review_targets_installation_submitter_idx.sql similarity index 100% rename from migrations/0130_review_targets_installation_submitter_idx.sql rename to migrations/0131_review_targets_installation_submitter_idx.sql From 84b086515b72ab347b9927ec49c3cefdbdeefcc3 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Thu, 9 Jul 2026 18:57:15 -0700 Subject: [PATCH 3/8] fix(test): account for getEffectiveSubmitterReputation's miner-identity check in visual-vision tests runVisualVisionForAdvisory now resolves reputation via getEffectiveSubmitterReputation (#4513), which checks confirmed-official-miner identity (a fetch to api.gittensor.io/miners) whenever the submitter's per-repo signal is neutral -- a real, intentional behavior change this test file's existing "no network calls at all" assertions didn't account for. Stub that one identity check to resolve cleanly and assert precisely that no OTHER (BYOK/vision-spend) network call happens, rather than asserting zero fetch calls outright. --- test/unit/visual-vision-wiring.test.ts | 44 ++++++++++++++++++-------- 1 file changed, 30 insertions(+), 14 deletions(-) diff --git a/test/unit/visual-vision-wiring.test.ts b/test/unit/visual-vision-wiring.test.ts index fb9d7133cd..9acb156981 100644 --- a/test/unit/visual-vision-wiring.test.ts +++ b/test/unit/visual-vision-wiring.test.ts @@ -54,6 +54,20 @@ function stubShotsAndProvider(providerResponseText: string | null) { })); } +/** #4513: getEffectiveSubmitterReputation now checks confirmed-official-miner identity (a fetch to + * api.gittensor.io/miners) whenever a submitter's PER-REPO reputation signal is "neutral" -- i.e. in every + * scenario below that doesn't already mock getSubmitterReputation to a non-neutral signal. That identity + * check is unrelated to whether this function goes on to spend on a vision call, so a bare `vi.fn()` + * asserting NO fetch at all is no longer accurate; this resolves the miner check to "not a miner" (an empty + * roster) so the rest of each test's decline/self-host logic runs exactly as before. */ +function stubMinerCheckOnly() { + return vi.fn(async (input: RequestInfo | URL) => { + const url = input.toString(); + if (url === "https://api.gittensor.io/miners") return Response.json([]); + return new Response("not found", { status: 404 }); + }); +} + describe("runVisualVisionForAdvisory", () => { it("no-ops on an empty route list -- never touches D1 or the network", async () => { const env = byokEnv(); @@ -113,7 +127,7 @@ describe("runVisualVisionForAdvisory", () => { it("declines when no route crossed the pixel-diff threshold (no_confirmed_regression) -- never resolves BYOK", async () => { const env = byokEnv(); await upsertRepositoryAiKey(env, { repoFullName, provider: "anthropic", key: "sk-ant-vision-key", model: null }); - const fetchMock = vi.fn(); + const fetchMock = stubMinerCheckOnly(); vi.stubGlobal("fetch", fetchMock); const adv = findingsHolder(); await runVisualVisionForAdvisory(env, { @@ -127,7 +141,9 @@ describe("runVisualVisionForAdvisory", () => { routes: [route({ path: "/app", beforeUrl: "https://x/gittensory/shot?key=b", afterUrl: "https://x/gittensory/shot?key=a" })], }); expect(adv.findings).toEqual([]); - expect(fetchMock).not.toHaveBeenCalled(); + // The only network activity is the (unrelated) confirmed-official-miner identity check -- never a + // real BYOK/vision-spend call. + expect(fetchMock.mock.calls.map((c) => String(c[0]))).toEqual(["https://api.gittensor.io/miners"]); }); it("declines for a low-reputation submitter even with a confirmed regression and BYOK configured", async () => { @@ -162,7 +178,7 @@ describe("runVisualVisionForAdvisory", () => { it("declines when BYOK is not configured (aiReviewByok off) even with a confirmed regression", async () => { const env = byokEnv(); - const fetchMock = vi.fn(); + const fetchMock = stubMinerCheckOnly(); vi.stubGlobal("fetch", fetchMock); const adv = findingsHolder(); await runVisualVisionForAdvisory(env, { @@ -176,13 +192,13 @@ describe("runVisualVisionForAdvisory", () => { routes: [route({ path: "/app", diffUrl: "https://x/gittensory/shot?key=diff", beforeUrl: "https://x/gittensory/shot?key=b", afterUrl: "https://x/gittensory/shot?key=a" })], }); expect(adv.findings).toEqual([]); - expect(fetchMock).not.toHaveBeenCalled(); + expect(fetchMock.mock.calls.map((c) => String(c[0]))).toEqual(["https://api.gittensor.io/miners"]); }); it("declines when the submitter is not a confirmed contributor, even with BYOK configured", async () => { const env = byokEnv(); await upsertRepositoryAiKey(env, { repoFullName, provider: "anthropic", key: "sk-ant-vision-key", model: null }); - const fetchMock = vi.fn(); + const fetchMock = stubMinerCheckOnly(); vi.stubGlobal("fetch", fetchMock); const adv = findingsHolder(); await runVisualVisionForAdvisory(env, { @@ -196,13 +212,13 @@ describe("runVisualVisionForAdvisory", () => { routes: [route({ path: "/app", diffUrl: "https://x/gittensory/shot?key=diff", beforeUrl: "https://x/gittensory/shot?key=b", afterUrl: "https://x/gittensory/shot?key=a" })], }); expect(adv.findings).toEqual([]); - expect(fetchMock).not.toHaveBeenCalled(); + expect(fetchMock.mock.calls.map((c) => String(c[0]))).toEqual(["https://api.gittensor.io/miners"]); }); it("skips BYOK (declines, falls back to nothing) when the declared provider doesn't match the stored key", async () => { const env = byokEnv(); await upsertRepositoryAiKey(env, { repoFullName, provider: "anthropic", key: "sk-ant-vision-key", model: null }); - const fetchMock = vi.fn(); + const fetchMock = stubMinerCheckOnly(); vi.stubGlobal("fetch", fetchMock); const adv = findingsHolder(); await runVisualVisionForAdvisory(env, { @@ -216,7 +232,7 @@ describe("runVisualVisionForAdvisory", () => { routes: [route({ path: "/app", diffUrl: "https://x/gittensory/shot?key=diff", beforeUrl: "https://x/gittensory/shot?key=b", afterUrl: "https://x/gittensory/shot?key=a" })], }); expect(adv.findings).toEqual([]); - expect(fetchMock).not.toHaveBeenCalled(); + expect(fetchMock.mock.calls.map((c) => String(c[0]))).toEqual(["https://api.gittensor.io/miners"]); }); it("calls the BYOK vision provider with before+after images and publishes a returned finding (desktop route)", async () => { @@ -402,7 +418,7 @@ describe("runVisualVisionForAdvisory", () => { const env = byokEnv(); await upsertRepositoryAiKey(env, { repoFullName, provider: "anthropic", key: "sk-ant-vision-key", model: null }); vi.spyOn(repositories, "getDecryptedRepositoryAiKey").mockRejectedValueOnce(new Error("D1 unavailable")); - const fetchMock = vi.fn(); + const fetchMock = stubMinerCheckOnly(); vi.stubGlobal("fetch", fetchMock); const adv = findingsHolder(); await expect( @@ -418,7 +434,7 @@ describe("runVisualVisionForAdvisory", () => { }), ).resolves.toBeUndefined(); expect(adv.findings).toEqual([]); - expect(fetchMock).not.toHaveBeenCalled(); + expect(fetchMock.mock.calls.map((c) => String(c[0]))).toEqual(["https://api.gittensor.io/miners"]); }); }); @@ -475,7 +491,7 @@ describe("runVisualVisionForAdvisory: self-host local vision provider (#4335)", const runMock = vi.fn(async () => ({ response: findingsResponse([{ path: "/app", body: "should not run" }]) })); const env = byokEnv(); (env as unknown as { AI_VISION: unknown }).AI_VISION = { run: runMock }; - const fetchMock = vi.fn(); + const fetchMock = stubMinerCheckOnly(); vi.stubGlobal("fetch", fetchMock); const adv = findingsHolder(); await runVisualVisionForAdvisory(env, { @@ -489,7 +505,7 @@ describe("runVisualVisionForAdvisory: self-host local vision provider (#4335)", routes: selfHostVisionRoutes(), }); expect(runMock).not.toHaveBeenCalled(); - expect(fetchMock).not.toHaveBeenCalled(); + expect(fetchMock.mock.calls.map((c) => String(c[0]))).toEqual(["https://api.gittensor.io/miners"]); expect(adv.findings).toEqual([]); }); @@ -592,7 +608,7 @@ describe("runVisualVisionForAdvisory: self-host local vision provider (#4335)", it("still declines entirely when NEITHER BYOK nor env.AI_VISION is configured", async () => { const env = byokEnv(); - const fetchMock = vi.fn(); + const fetchMock = stubMinerCheckOnly(); vi.stubGlobal("fetch", fetchMock); const adv = findingsHolder(); await runVisualVisionForAdvisory(env, { @@ -606,6 +622,6 @@ describe("runVisualVisionForAdvisory: self-host local vision provider (#4335)", routes: selfHostVisionRoutes(), }); expect(adv.findings).toEqual([]); - expect(fetchMock).not.toHaveBeenCalled(); + expect(fetchMock.mock.calls.map((c) => String(c[0]))).toEqual(["https://api.gittensor.io/miners"]); }); }); From 11c58cf31f8848770793c637b976fa6788cc93f7 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Thu, 9 Jul 2026 18:58:42 -0700 Subject: [PATCH 4/8] fix(db): renumber migration 0131 -> 0133 (0131/0132 claimed by merged PRs) origin/main has since merged #4545 (0131_screenshot_table_gate_matrix.sql) and #4554 (0132_impact_map_query_cache.sql, itself a collision fix); rebase onto current main and take the next free number. --- ...idx.sql => 0133_review_targets_installation_submitter_idx.sql} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename migrations/{0131_review_targets_installation_submitter_idx.sql => 0133_review_targets_installation_submitter_idx.sql} (100%) diff --git a/migrations/0131_review_targets_installation_submitter_idx.sql b/migrations/0133_review_targets_installation_submitter_idx.sql similarity index 100% rename from migrations/0131_review_targets_installation_submitter_idx.sql rename to migrations/0133_review_targets_installation_submitter_idx.sql From 979dc11ca4f732d3c2ab4bcff9344697c9d5a912 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Thu, 9 Jul 2026 19:09:20 -0700 Subject: [PATCH 5/8] fix(db): renumber migration 0133 -> 0134 (0133 claimed by merged PR #4556) Rebase onto current main and take the next free number now that migrations/0133_screenshot_table_gate_skill_link.sql (from #4556) occupies the number this branch previously took. --- ...idx.sql => 0134_review_targets_installation_submitter_idx.sql} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename migrations/{0133_review_targets_installation_submitter_idx.sql => 0134_review_targets_installation_submitter_idx.sql} (100%) diff --git a/migrations/0133_review_targets_installation_submitter_idx.sql b/migrations/0134_review_targets_installation_submitter_idx.sql similarity index 100% rename from migrations/0133_review_targets_installation_submitter_idx.sql rename to migrations/0134_review_targets_installation_submitter_idx.sql From 8d44b104a57f51bfde413e0b22eb1ad701d8d97c Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Thu, 9 Jul 2026 22:02:37 -0700 Subject: [PATCH 6/8] fix(db): renumber migration 0134 -> 0139 (0134 has a pre-existing 3-way collision on main) main currently has three DIFFERENT already-merged files at 0134 (#4549/#4558/#4563) -- tracked separately as its own fix (PR #4577, not yet merged). Since that fix already claims through 0138, take 0139 here to avoid colliding with it once it lands; this branch's own db:migrations:check will stay red until #4577 merges (unrelated to this branch's own changes), and will need one more rebase afterward. --- ...idx.sql => 0139_review_targets_installation_submitter_idx.sql} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename migrations/{0134_review_targets_installation_submitter_idx.sql => 0139_review_targets_installation_submitter_idx.sql} (100%) diff --git a/migrations/0134_review_targets_installation_submitter_idx.sql b/migrations/0139_review_targets_installation_submitter_idx.sql similarity index 100% rename from migrations/0134_review_targets_installation_submitter_idx.sql rename to migrations/0139_review_targets_installation_submitter_idx.sql From c888b2c8b0127daefbf8e1cec17ff319d2ec03af Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Thu, 9 Jul 2026 22:07:45 -0700 Subject: [PATCH 7/8] test(review): close 2 coverage gaps surfaced by the #4514 rebase-merge getEffectiveSubmitterReputation's getRepository().catch() needed a real read-failure test (added); its isConfirmedOfficialMiner().catch() is unreachable (that function already catches every internal failure point itself) and getSubmitterReputationAcrossInstall's results ?? [] fallback is the same "D1 always populates results" case already v8-ignored elsewhere in this codebase -- both marked accordingly. --- src/review/reputation-wire.ts | 1 + src/review/submitter-reputation.ts | 1 + test/unit/reputation-wiring.test.ts | 17 +++++++++++++++++ 3 files changed, 19 insertions(+) diff --git a/src/review/reputation-wire.ts b/src/review/reputation-wire.ts index 0eec7d8ee1..5db75d2fa2 100644 --- a/src/review/reputation-wire.ts +++ b/src/review/reputation-wire.ts @@ -78,6 +78,7 @@ export async function getEffectiveSubmitterReputation( if (shouldDowngradeToDeterministic(perRepo)) return perRepo; const submitter = args.submitter?.trim(); if (!submitter) return perRepo; + /* v8 ignore next -- isConfirmedOfficialMiner already catches every internal failure point itself and never rejects; this guards only a future implementation change. */ const isMiner = await isConfirmedOfficialMiner(env, submitter).catch(() => false); if (!isMiner) return perRepo; const repo = await getRepository(env, args.repoFullName).catch(() => null); diff --git a/src/review/submitter-reputation.ts b/src/review/submitter-reputation.ts index 0549740b7d..9744297ace 100644 --- a/src/review/submitter-reputation.ts +++ b/src/review/submitter-reputation.ts @@ -339,6 +339,7 @@ export async function getSubmitterReputationAcrossInstall( ) .bind(installationId, submitter, `-${cfg.windowDays} days`, REPUTATION_WINDOW_ROW_CAP) .all<{ status: string; reasonCode: string | null }>(); + /* v8 ignore next -- D1's .all() always populates results; the fallback only protects a driver anomaly. */ const rows = result?.results ?? []; signal = signalFromCounts(countOutcomes(rows), cfg); } catch { diff --git a/test/unit/reputation-wiring.test.ts b/test/unit/reputation-wiring.test.ts index f7d3c9b0a9..00ae84ae9b 100644 --- a/test/unit/reputation-wiring.test.ts +++ b/test/unit/reputation-wiring.test.ts @@ -352,6 +352,23 @@ describe("getEffectiveSubmitterReputation (#4513, install-wide for a confirmed m vi.unstubAllGlobals(); } }); + + it("fails safe to the per-repo result, without throwing, when the getRepository read itself errors", async () => { + const env = createTestEnv(); + await seedReviewTarget(env, { project: "org/repo-a", repo: "org/repo-a", number: 1, installationId: 999, submitter: "farmer99", status: "closed", reasonCode: "dual_review_declined" }); + vi.stubGlobal("fetch", stubMinerFetch("farmer99")); + const realPrepare = env.DB.prepare.bind(env.DB); + env.DB.prepare = ((sql: string) => { + if (/FROM.*"?repositories"?/i.test(sql)) throw new Error("d1 down"); + return realPrepare(sql); + }) as typeof env.DB.prepare; + try { + const rep = await getEffectiveSubmitterReputation(env, { repoFullName: "org/repo-a", submitter: "farmer99" }); + expect(rep.signal).toBe("neutral"); + } finally { + vi.unstubAllGlobals(); + } + }); }); describe("processGitHubWebhook records the reputation outcome on a terminal PR (flag-ON call site)", () => { From 7a749ddd7b07663bf3fdbd0dd44a10acef57919c Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Fri, 10 Jul 2026 00:35:17 -0700 Subject: [PATCH 8/8] fix(review): isolate #4507's reputation-single-read invariant from cross-test miner-cache pollution Several earlier tests in this file cache "contributor" as a confirmed official Gittensor miner (5-min TTL) in the shared per-file D1 instance. That collided with #4513's new install-wide reputation widening, which correctly detects the stale cache and adds a 4th reputation-scan D1 read for a submitter this test never intended to be a miner. Use a submitter login unique to this test instead. --- test/unit/queue.test.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/test/unit/queue.test.ts b/test/unit/queue.test.ts index 3b3cdbd153..7228241d93 100644 --- a/test/unit/queue.test.ts +++ b/test/unit/queue.test.ts @@ -3993,19 +3993,25 @@ describe("queue processors", () => { GITTENSORY_REVIEW_REPUTATION: "true", }); await seedRegateChurnRepo(env); - await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 62, title: "Reputation PR", state: "open", user: { login: "contributor" }, head: { sha: "a62" }, labels: [], body: "Closes #1" }); + // Deliberately NOT "contributor" -- several other tests earlier in this file (e.g. line ~3732) stub the + // Gittensor miners endpoint to confirm "contributor" as an official miner, and that caches a "confirmed" + // official_miner_detections row (5-min TTL) in this file's shared D1 instance. A submitter this test's own + // fetch stub never confirms must still resolve as NOT a miner, or #4513's install-wide widening adds a 4th + // reputation-scan prepare and this invariant's count goes stale for reasons unrelated to what it's testing. + await upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { number: 62, title: "Reputation PR", state: "open", user: { login: "reputation-single-read-user" }, head: { sha: "a62" }, labels: [], body: "Closes #1" }); await upsertPullRequestDetailSyncState(env, { repoFullName: "JSONbored/gittensory", pullNumber: 62, status: "complete", reviewsSyncedAt: new Date().toISOString() }); vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { const url = input.toString(); const method = init?.method ?? "GET"; if (url.includes("/access_tokens")) return Response.json({ token: "fake-installation-token" }); if (url.includes("/pulls/62/files")) return Response.json([{ filename: "src/a.ts", status: "modified", additions: 1, deletions: 0, changes: 1, patch: "@@\n+export const ok = true;" }]); - if (url.endsWith("/pulls/62")) return Response.json({ number: 62, title: "Reputation PR", state: "open", user: { login: "contributor" }, head: { sha: "a62" }, labels: [], body: "Closes #1", mergeable_state: "clean" }); + if (url.endsWith("/pulls/62")) return Response.json({ number: 62, title: "Reputation PR", state: "open", user: { login: "reputation-single-read-user" }, head: { sha: "a62" }, labels: [], body: "Closes #1", mergeable_state: "clean" }); if (url.includes("/commits/a62/check-runs")) return Response.json({ total_count: 0, check_runs: [] }); if (url.includes("/commits/a62/status")) return Response.json({ state: "success", statuses: [] }); if (url.includes("/issues/62/comments")) return method === "POST" ? Response.json({ id: 62 }, { status: 201 }) : Response.json([]); if (url.includes("/issues/1")) return Response.json({ number: 1, title: "Issue", state: "open", labels: [], user: { login: "reporter" } }); if (url.includes("/branches/")) return Response.json({ protected: false, protection: { required_status_checks: { contexts: [] } } }); + if (url.includes("/miners")) return Response.json([]); return Response.json({}); });