From ae95968af14e72aaa9524b9284f30de9fe46ccf2 Mon Sep 17 00:00:00 2001 From: jsdevninja Date: Fri, 17 Jul 2026 07:16:56 -0500 Subject: [PATCH] fix(review): wire AMS reputation bridge into reputation gate Feed getEffectiveSubmitterReputation through the feature-gated, upgrade-only bridgeAmsReputation path so amsReputationBridge can actually upgrade local neutral/low signals when AMS qualifies them. Closes #6801 Co-authored-by: Cursor --- src/review/reputation-wire.ts | 33 ++++++--- test/unit/reputation-wiring.test.ts | 108 ++++++++++++++++++++++++++++ 2 files changed, 131 insertions(+), 10 deletions(-) diff --git a/src/review/reputation-wire.ts b/src/review/reputation-wire.ts index 49acf44f3c..d1b714b93a 100644 --- a/src/review/reputation-wire.ts +++ b/src/review/reputation-wire.ts @@ -15,6 +15,10 @@ import { getRepository } from "../db/repositories"; import { isConfirmedOfficialMiner } from "../gittensor/miner-detection-cache"; +import { loadRepoFocusManifest } from "../signals/focus-manifest-loader"; +import { bridgeAmsReputation } from "./ams-reputation-bridge"; +import { isAmsReputationBridgeEnabled, resolveAmsTrackRecordEndpoint } from "./ams-reputation-bridge-wire"; +import { resolveConvergedFeature } from "./feature-activation"; import { getSubmitterCadence, getSubmitterReputation, @@ -61,6 +65,15 @@ export function shouldDowngradeToDeterministic(stats: SubmitterStats): boolean { return false; } +async function applyAmsReputationBridge(env: Env, repoFullName: string, submitter: string | undefined, local: SubmitterStats): Promise { + if (!isReputationEnabled(env)) return local; + if (!isAmsReputationBridgeEnabled(env)) return local; + const manifest = await loadRepoFocusManifest(env, repoFullName).catch(() => null); + if (!resolveConvergedFeature(env, manifest, "amsReputationBridge", repoFullName)) return local; + const signal = await bridgeAmsReputation(local.signal, submitter, { endpoint: resolveAmsTrackRecordEndpoint(env) }); + return signal === local.signal ? local : { ...local, signal }; +} + /** * 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 @@ -68,8 +81,8 @@ export function shouldDowngradeToDeterministic(stats: SubmitterStats): boolean { * (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. + * every submission. The final local signal then passes through the feature-gated, upgrade-only AMS bridge + * (#6801). Fail-safe throughout: identity, install-wide, manifest, or AMS failures keep the local result. */ export async function getEffectiveSubmitterReputation( env: Env, @@ -77,18 +90,19 @@ export async function getEffectiveSubmitterReputation( cfg?: ReputationConfig, ): Promise { const perRepo = await getSubmitterReputation(env, args.repoFullName, args.submitter ?? undefined, cfg); - if (shouldDowngradeToDeterministic(perRepo)) return perRepo; + if (shouldDowngradeToDeterministic(perRepo)) return applyAmsReputationBridge(env, args.repoFullName, args.submitter?.trim(), perRepo); const submitter = args.submitter?.trim(); - if (!submitter) return perRepo; + if (!submitter) return applyAmsReputationBridge(env, args.repoFullName, submitter, 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; + if (!isMiner) return applyAmsReputationBridge(env, args.repoFullName, submitter, perRepo); const repo = await getRepository(env, args.repoFullName).catch(() => null); - if (!repo?.installationId) return perRepo; + if (!repo?.installationId) return applyAmsReputationBridge(env, args.repoFullName, submitter, 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; + const local = shouldDowngradeToDeterministic(acrossInstall) ? acrossInstall : perRepo; + return applyAmsReputationBridge(env, args.repoFullName, submitter, local); } /** @@ -109,9 +123,8 @@ export async function shouldSkipAiForReputation( args: { project: string; submitter: string | null | undefined }, ): Promise { if (!isReputationEnabled(env)) return false; - // 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. + // Combines the extensions to the base per-repo signal: install-wide widening for a confirmed miner and the + // upgrade-only AMS bridge inside getEffectiveSubmitterReputation, then cadence (#4514) independently. 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); diff --git a/test/unit/reputation-wiring.test.ts b/test/unit/reputation-wiring.test.ts index 00dfc9cefe..ba0c166194 100644 --- a/test/unit/reputation-wiring.test.ts +++ b/test/unit/reputation-wiring.test.ts @@ -323,6 +323,114 @@ describe("shouldSkipAiForReputation (helper)", () => { }); }); +describe("ORB/AMS reputation bridge wiring (#6801)", () => { + const repoFullName = "acme/widgets"; + const submitter = "bridge-dev"; + const strongAmsRecord = [ + { repoFullName, authorLogin: submitter, state: "merged" }, + { repoFullName, authorLogin: submitter, state: "merged" }, + { repoFullName, authorLogin: submitter, state: "merged" }, + ]; + + async function seedLowReputation(env: Env) { + for (let i = 0; i < 7; i++) { + await seedReviewTarget(env, { + project: repoFullName, + repo: repoFullName, + number: i + 1, + installationId: 999, + submitter, + status: "closed", + reasonCode: "dual_review_declined", + }); + } + // Keep the quality outcomes recent while making their submission cadence old enough not to trip the + // independent 24-hour machine-paced check after AMS upgrades the quality signal. + await env.DB.prepare("UPDATE review_targets SET created_at = '2026-01-01T00:00:00.000Z' WHERE submitter = ?").bind(submitter).run(); + } + + it("REGRESSION (#6801): feature on upgrades a low local signal and changes the end-to-end gate decision", async () => { + const env = createTestEnv({ + LOOPOVER_REVIEW_REPUTATION: "true", + LOOPOVER_REVIEW_AMS_REPUTATION_BRIDGE: "true", + LOOPOVER_AMS_TRACK_RECORD_URL: "https://ams.internal", + }); + await seedLowReputation(env); + await upsertRepoFocusManifest(env, repoFullName, { features: { amsReputationBridge: true } }); + const fetchMock = vi.fn(async (input: RequestInfo | URL) => { + expect(input.toString()).toBe(`https://ams.internal/track-record/${submitter}`); + return Response.json(strongAmsRecord); + }); + vi.stubGlobal("fetch", fetchMock); + try { + const effective = await getEffectiveSubmitterReputation(env, { repoFullName, submitter }); + expect(effective.signal).toBe("trusted"); + expect(await shouldSkipAiForReputation(env, { project: repoFullName, submitter })).toBe(false); + expect(fetchMock).toHaveBeenCalledTimes(2); + } finally { + vi.unstubAllGlobals(); + } + }); + + it("keeps the pre-bridge output unchanged when any activation gate is off", async () => { + const cases = [ + { reputation: "false", bridge: "true", manifest: true }, + { reputation: "true", bridge: "false", manifest: true }, + { reputation: "true", bridge: "true", manifest: false }, + ] as const; + + for (const testCase of cases) { + const env = createTestEnv({ + LOOPOVER_REVIEW_REPUTATION: testCase.reputation, + LOOPOVER_REVIEW_AMS_REPUTATION_BRIDGE: testCase.bridge, + LOOPOVER_AMS_TRACK_RECORD_URL: "https://ams.internal", + }); + await seedLowReputation(env); + await upsertRepoFocusManifest(env, repoFullName, { features: { amsReputationBridge: testCase.manifest } }); + const local = await getSubmitterReputation(env, repoFullName, submitter); + const fetchMock = vi.fn(async () => Response.json(strongAmsRecord)); + vi.stubGlobal("fetch", fetchMock); + try { + expect(await getEffectiveSubmitterReputation(env, { repoFullName, submitter })).toEqual(local); + expect(fetchMock).not.toHaveBeenCalled(); + } finally { + vi.unstubAllGlobals(); + } + } + }); + + it("never downgrades the locally-computed signal when the enabled bridge does not vouch", async () => { + const env = createTestEnv({ + LOOPOVER_REVIEW_REPUTATION: "true", + LOOPOVER_REVIEW_AMS_REPUTATION_BRIDGE: "true", + LOOPOVER_AMS_TRACK_RECORD_URL: "https://ams.internal", + }); + await seedLowReputation(env); + await upsertRepoFocusManifest(env, repoFullName, { features: { amsReputationBridge: true } }); + const local = await getSubmitterReputation(env, repoFullName, submitter); + const fetchMock = vi.fn(async () => + Response.json(Array.from({ length: 9 }, () => ({ repoFullName, authorLogin: submitter, state: "closed" }))), + ); + vi.stubGlobal("fetch", fetchMock); + try { + expect(await getEffectiveSubmitterReputation(env, { repoFullName, submitter })).toEqual(local); + expect(fetchMock).toHaveBeenCalledOnce(); + } finally { + vi.unstubAllGlobals(); + } + }); + + it("fails safe to the local signal when manifest resolution throws", async () => { + const env = createTestEnv({ + LOOPOVER_REVIEW_REPUTATION: "true", + LOOPOVER_REVIEW_AMS_REPUTATION_BRIDGE: "true", + DB: undefined as unknown as D1Database, + }); + const effective = await getEffectiveSubmitterReputation(env, { repoFullName, submitter: undefined }); + expect(effective.signal).toBe("neutral"); + }); +}); + describe("getEffectiveSubmitterReputation (#4513, install-wide for a confirmed miner)", () => { function stubMinerFetch(githubUsername: string) { return async (input: RequestInfo | URL) => {