diff --git a/.loopover.yml.example b/.loopover.yml.example index 5341c9b7db..1296817623 100644 --- a/.loopover.yml.example +++ b/.loopover.yml.example @@ -1154,7 +1154,7 @@ settings: # Per-repo activation overrides for the converged review features that ship behind a deployment-wide # LOOPOVER_REVIEW_* env kill-switch (rag/reputation/safety/grounding/e2eTests/screenshots/ -# improvementSignal). Each key is `true` (force on for this repo, subject to the env flag still being +# improvementSignal/amsReputationBridge). Each key is `true` (force on for this repo, subject to the env flag still being # enabled), `false` (force off), or omitted (falls back to the LOOPOVER_REVIEW_REPOS allowlist default -- # an operator who sets nothing keeps today's behavior). Exception: `safety` is force-on-only -- an untrusted # repo-controlled `false` is treated as "no opinion" rather than an active force-off (#2269), since a @@ -1171,6 +1171,14 @@ settings: # security-hardening or full-file-fetch rationale that would justify a force-on/force-off asymmetry. # `improvementSignal` is activation wiring only for now -- no tier reads the resolved # value yet. +# `amsReputationBridge` (#6485, per #6208's decided design) is a plain symmetric override too. It lets a +# submitter's genuine track record on a LOCAL AMS instance UPGRADE their reputation standing here -- toward +# `trusted` only, never toward `low`, so an AMS record can never be used punitively against a contributor. +# ORB pulls from AMS (never AMS pushing in), so an arbitrary AMS instance cannot write trust signals into +# this one. Needs BOTH the LOOPOVER_REVIEW_AMS_REPUTATION_BRIDGE env kill-switch AND +# LOOPOVER_AMS_TRACK_RECORD_URL (the AMS base URL to pull from) -- with either unset the bridge applies no +# bonus signal and the reputation path is byte-identical to today. An unreachable/slow/absent AMS degrades +# the same way: no bonus, never a throw into the gate. # features: # rag: true # reputation: false @@ -1179,6 +1187,7 @@ settings: # e2eTests: false # screenshots: false # improvementSignal: false +# amsReputationBridge: false # Optional ecosystem/network PLUGINS -- distinct from `features:` above, which only toggles loopover's own # converged review capabilities. Each key here couples this instance to an external system and is OFF unless diff --git a/apps/loopover-ui/content/docs/privacy-security.mdx b/apps/loopover-ui/content/docs/privacy-security.mdx index feb83d2839..d1187e4603 100644 --- a/apps/loopover-ui/content/docs/privacy-security.mdx +++ b/apps/loopover-ui/content/docs/privacy-security.mdx @@ -58,6 +58,7 @@ LOOPOVER_REVIEW_PLANNER="true" # @loopover plan on-demand implem LOOPOVER_REVIEW_SCREENSHOTS="true" # before/after visual capture for UI changes LOOPOVER_REVIEW_E2E_TESTS="true" # AI-generated E2E test coverage (needs features.e2eTests too) LOOPOVER_REVIEW_IMPROVEMENT_SIGNAL="true" # read-only PR quality-delta signal (activation only, no-op for now) +LOOPOVER_REVIEW_AMS_REPUTATION_BRIDGE="true" # upgrade-only AMS track-record bridge (never shown; needs features.amsReputationBridge + LOOPOVER_AMS_TRACK_RECORD_URL) # Global (cron / endpoint) flags, not scoped by LOOPOVER_REVIEW_REPOS. LOOPOVER_REVIEW_CONTINUOUS="true" # fleet-wide default: re-review on every push (else one-shot) diff --git a/apps/loopover-ui/content/docs/tuning.mdx b/apps/loopover-ui/content/docs/tuning.mdx index 90105bbedf..f0bb290c98 100644 --- a/apps/loopover-ui/content/docs/tuning.mdx +++ b/apps/loopover-ui/content/docs/tuning.mdx @@ -92,6 +92,13 @@ any per-PR feature to run on a given repo. So a per-PR feature activates only wh Off by default; config-as-code activation only for now — no tier reads the resolved value yet, so turning this on has no visible effect until a later release wires real behavior behind it. Per-PR. +- `LOOPOVER_REVIEW_AMS_REPUTATION_BRIDGE` — master kill-switch for the upgrade-only + ORB/AMS reputation bridge: a submitter's genuine track record on a local AMS instance can + raise their reputation standing here, never lower it. Off by default; a repo also needs its + own `features.amsReputationBridge: true` override in `.loopover.yml`, and the operator must + point `LOOPOVER_AMS_TRACK_RECORD_URL` at the AMS instance to pull from — with either unset + the bridge applies no bonus signal. ORB pulls from AMS (never the reverse), and an + absent/unreachable/slow AMS simply yields no bonus. Per-PR. - `LOOPOVER_REVIEW_CONTINUOUS` — fleet-wide default AI review re-trigger cadence. Off by default (one-shot): AI-generated content (main review, slop advisory, linked-issue satisfaction) is produced once per PR and never regenerated automatically afterward — only diff --git a/config/examples/loopover.full.yml b/config/examples/loopover.full.yml index 280c0592db..2bb9968156 100644 --- a/config/examples/loopover.full.yml +++ b/config/examples/loopover.full.yml @@ -1168,7 +1168,7 @@ settings: # Per-repo activation overrides for the converged review features that ship behind a deployment-wide # LOOPOVER_REVIEW_* env kill-switch (rag/reputation/safety/grounding/e2eTests/screenshots/ -# improvementSignal). Each key is `true` (force on for this repo, subject to the env flag still being +# improvementSignal/amsReputationBridge). Each key is `true` (force on for this repo, subject to the env flag still being # enabled), `false` (force off), or omitted (falls back to the LOOPOVER_REVIEW_REPOS allowlist default -- # an operator who sets nothing keeps today's behavior). Exception: `safety` is force-on-only -- an untrusted # repo-controlled `false` is treated as "no opinion" rather than an active force-off (#2269), since a @@ -1185,6 +1185,14 @@ settings: # security-hardening or full-file-fetch rationale that would justify a force-on/force-off asymmetry. # `improvementSignal` is activation wiring only for now -- no tier reads the resolved # value yet. +# `amsReputationBridge` (#6485, per #6208's decided design) is a plain symmetric override too. It lets a +# submitter's genuine track record on a LOCAL AMS instance UPGRADE their reputation standing here -- toward +# `trusted` only, never toward `low`, so an AMS record can never be used punitively against a contributor. +# ORB pulls from AMS (never AMS pushing in), so an arbitrary AMS instance cannot write trust signals into +# this one. Needs BOTH the LOOPOVER_REVIEW_AMS_REPUTATION_BRIDGE env kill-switch AND +# LOOPOVER_AMS_TRACK_RECORD_URL (the AMS base URL to pull from) -- with either unset the bridge applies no +# bonus signal and the reputation path is byte-identical to today. An unreachable/slow/absent AMS degrades +# the same way: no bonus, never a throw into the gate. # features: # rag: true # reputation: false @@ -1193,6 +1201,7 @@ settings: # e2eTests: false # screenshots: false # improvementSignal: false +# amsReputationBridge: false # Optional ecosystem/network PLUGINS -- distinct from `features:` above, which only toggles loopover's own # converged review capabilities. Each key here couples this instance to an external system and is OFF unless diff --git a/packages/loopover-engine/src/focus-manifest.ts b/packages/loopover-engine/src/focus-manifest.ts index 02f2dd433a..d8cc35a70a 100644 --- a/packages/loopover-engine/src/focus-manifest.ts +++ b/packages/loopover-engine/src/focus-manifest.ts @@ -265,7 +265,16 @@ export type CopycatGateMode = "off" | "warn" | "label" | "block"; // level `false` behaves like any other plain override with no floor/ceiling. This is activation wiring only // -- no tier reads the resolved value yet (sibling sub-issues #4739-#4746 build the deterministic/LLM/panel // behavior that will gate on it). -export const CONVERGED_FEATURE_KEYS = ["rag", "reputation", "safety", "grounding", "e2eTests", "screenshots", "improvementSignal"] as const; +export const CONVERGED_FEATURE_KEYS = [ + "rag", + "reputation", + "safety", + "grounding", + "e2eTests", + "screenshots", + "improvementSignal", + "amsReputationBridge", +] as const; export type ConvergedFeatureKey = (typeof CONVERGED_FEATURE_KEYS)[number]; /** Per-repo activation overrides for the converged review features (`features:` block). `true`/`false` force the @@ -1141,6 +1150,7 @@ const EMPTY_FEATURES_CONFIG: FocusManifestFeaturesConfig = { e2eTests: null, screenshots: null, improvementSignal: null, + amsReputationBridge: null, }; const EMPTY_EXPERIMENTAL_CONFIG: FocusManifestExperimentalConfig = { diff --git a/src/env.d.ts b/src/env.d.ts index 51dff10728..6c3baf7b92 100644 --- a/src/env.d.ts +++ b/src/env.d.ts @@ -346,6 +346,17 @@ declare global { * build the deterministic/LLM/panel behavior that will gate on it. Default OFF — unset/false the feature * is never active for any repo regardless of a per-repo `features.improvementSignal` override. */ LOOPOVER_REVIEW_IMPROVEMENT_SIGNAL?: string; + /** #6485 (per #6208's decided design): master kill-switch for the `amsReputationBridge` converged feature — + * an UPGRADE-ONLY, pull-based bridge letting a submitter's genuine AMS track record improve (never worsen) + * their ORB reputation standing. Default OFF — unset/false the bridge is never active for any repo + * regardless of a per-repo `features.amsReputationBridge` override, so the reputation path is + * byte-identical to today. Needs LOOPOVER_AMS_TRACK_RECORD_URL to have anything to pull from. */ + LOOPOVER_REVIEW_AMS_REPUTATION_BRIDGE?: string; + /** #6485: the operator-configured base URL of the LOCAL AMS instance the reputation bridge pulls a + * submitter's track record from (`GET {url}/track-record/{login}`). Unset ⇒ the bridge applies no bonus + * signal even when enabled. ORB pulls (never AMS pushing in), so an arbitrary AMS instance can never write + * trust signals into ORB — see #6208. */ + LOOPOVER_AMS_TRACK_RECORD_URL?: string; /** #one-shot-review-cadence: the operator's FLEET-WIDE default for AI review re-trigger cadence, consulted * only when a repo's `.loopover.yml review.auto_review.cadence` is unset (a per-repo value always wins * regardless of this flag — see resolveAiReviewCadence). Default OFF (unset/false) ⇒ "one_shot": the diff --git a/src/review/ams-reputation-bridge-wire.ts b/src/review/ams-reputation-bridge-wire.ts new file mode 100644 index 0000000000..5cee664920 --- /dev/null +++ b/src/review/ams-reputation-bridge-wire.ts @@ -0,0 +1,29 @@ +// Convergence (ORB/AMS reputation bridge, #6485, per #6208's decided design): the master kill-switch for the +// `amsReputationBridge` converged feature -- an UPGRADE-ONLY, pull-based bridge that lets a submitter's genuine +// AMS track record improve (never worsen) their ORB reputation standing. Mirrors the shape of +// `improvement-signal-wire.ts`/`rag-wire.ts`: this file is deliberately just the env flag, so the +// per-repo `features.amsReputationBridge` override in `.loopover.yml` has a global switch to gate on. +// +// Single env switch: LOOPOVER_REVIEW_AMS_REPUTATION_BRIDGE. Default OFF (unset/"false") -- when OFF the bridge +// never runs for any repo, regardless of a per-repo `.loopover.yml` override (see `resolveConvergedFeature` in +// `./feature-activation`), so the reputation path is byte-identical to today. Truthy follows the codebase +// convention (`/^(1|true|yes|on)$/i`, same as isReputationEnabled / isImprovementSignalEnabled). +// +// STRICTLY INTERNAL: like the `reputation` signal it extends, the bridged standing NEVER appears in any public +// comment, label, or check-run -- it only routes the private, server-side AI-spend decision. + +/** True when the ORB/AMS reputation bridge is enabled at the deployment level. Flag-OFF (default) → the bridge + * is never active for any repo, regardless of a per-repo `features.amsReputationBridge` override. */ +export function isAmsReputationBridgeEnabled(env: { + LOOPOVER_REVIEW_AMS_REPUTATION_BRIDGE?: string | undefined; +}): boolean { + return /^(1|true|yes|on)$/i.test((env.LOOPOVER_REVIEW_AMS_REPUTATION_BRIDGE ?? "").trim()); +} + +/** The operator-configured LOCAL AMS base URL the bridge pulls from, or undefined when unset/blank (⇒ the + * bridge applies no bonus signal even when the feature is otherwise active). Kept next to the kill-switch so + * both halves of this feature's deployment config resolve in one place. */ +export function resolveAmsTrackRecordEndpoint(env: { LOOPOVER_AMS_TRACK_RECORD_URL?: string | undefined }): string | undefined { + const url = (env.LOOPOVER_AMS_TRACK_RECORD_URL ?? "").trim(); + return url === "" ? undefined : url; +} diff --git a/src/review/ams-reputation-bridge.ts b/src/review/ams-reputation-bridge.ts new file mode 100644 index 0000000000..390532b484 --- /dev/null +++ b/src/review/ams-reputation-bridge.ts @@ -0,0 +1,123 @@ +// ORB/AMS reputation bridge (#6485), implementing #6208's decided design. ORB PULLS a submitter's AMS track +// record on demand and may only ever UPGRADE the locally-computed ReputationSignal toward "trusted" -- never +// downgrade it. Rationale (from #6208): a push model would let any AMS instance -- including a self-hosted one +// running against an arbitrary repo -- write arbitrary trust signals into ORB's internal reputation store, a +// direct gaming vector. Pull keeps ORB in control; upgrade-only closes the second vector (an AMS track record +// must never be usable punitively against a contributor's standing on an unrelated repo). +// +// Identity is plain `authorLogin` -- the same axis `submitter-reputation.ts` already keys on, and the same axis +// `TrackRecordPullRequestOutcome` already carries. No new identity system, and deliberately NOT hotkey/wallet +// (forbidden/redacted terms in this codebase). +// +// Privacy: `TrackRecordPullRequestOutcome` has no score/ranking/wallet/hotkey fields by construction, so the +// consumed shape is already safe -- nothing new to redact. STRICTLY INTERNAL, inherited from +// `submitter-reputation.ts`: never surfaced in a label, comment, or check-run. +// +// Fail-safe contract, matching every other guard in the reputation path: any fetch error, timeout, non-OK +// status, or malformed payload degrades to "no bonus signal applied" -- this must never throw into the gate. + +import type { TrackRecordPullRequestOutcome } from "@loopover/engine"; +import type { ReputationSignal } from "./submitter-reputation"; + +/** A slow or unreachable AMS instance must never slow gate evaluation -- a few hundred ms, consistent with the + * other fail-safe external reads in this codebase. */ +export const AMS_TRACK_RECORD_TIMEOUT_MS = 400; + +/** `trusted` needs at least this many merged AMS PRs. Mirrors `submitter-reputation.ts`'s "default GENEROUS" + * philosophy: a sparse record is simply no bonus, never a penalty. */ +export const AMS_BRIDGE_TRUSTED_MIN_MERGED = 3; + +/** …AND a merge rate at/above this share of that submitter's terminal AMS outcomes (0–1). */ +export const AMS_BRIDGE_TRUSTED_MIN_MERGE_RATE = 0.6; + +export type AmsTrackRecordFetch = (url: string, init: RequestInit) => Promise; + +export type AmsBridgeOptions = { + /** The operator-configured AMS endpoint base (`LOOPOVER_AMS_TRACK_RECORD_URL`). */ + endpoint: string | undefined; + fetchImpl?: AmsTrackRecordFetch | undefined; + timeoutMs?: number | undefined; +}; + +/** PURE: is a raw value a usable TrackRecordPullRequestOutcome for this bridge? Only the two fields the bridge + * actually reads are required -- a payload carrying extra keys is fine (and any score-ish key is simply never + * read), but one missing `authorLogin`/`state` is malformed and dropped. */ +function isUsableOutcome(value: unknown): value is TrackRecordPullRequestOutcome { + if (value === null || typeof value !== "object") return false; + const row = value as Record; + return typeof row.authorLogin === "string" && row.authorLogin.trim() !== "" && typeof row.state === "string"; +} + +/** PURE: keep only this login's outcomes from a payload. Case-insensitive, matching GitHub login semantics. */ +export function outcomesForLogin(rows: readonly unknown[], login: string): TrackRecordPullRequestOutcome[] { + const wanted = login.trim().toLowerCase(); + if (!wanted) return []; + return rows.filter(isUsableOutcome).filter((row) => row.authorLogin.trim().toLowerCase() === wanted); +} + +/** + * PURE: does this AMS track record qualify the submitter as `trusted`? Deliberately a single boolean rather + * than a full ReputationSignal: the bridge is upgrade-only, so "does not qualify" and "looks bad" are the same + * outcome (no bonus) -- there is no representable way for AMS data to push a submitter down. + */ +export function amsRecordQualifiesAsTrusted(outcomes: readonly TrackRecordPullRequestOutcome[]): boolean { + const merged = outcomes.filter((o) => o.state === "merged").length; + // Only TERMINAL outcomes form the denominator: an open PR is not yet evidence either way. + const terminal = outcomes.filter((o) => o.state === "merged" || o.state === "closed").length; + if (merged < AMS_BRIDGE_TRUSTED_MIN_MERGED) return false; + /* v8 ignore next -- merged >= 3 above guarantees terminal >= 3, so terminal is never 0 here; the guard is kept + so a future caller passing pre-filtered rows can never divide by zero. */ + if (terminal === 0) return false; + return merged / terminal >= AMS_BRIDGE_TRUSTED_MIN_MERGE_RATE; +} + +/** + * PURE: the upgrade-only merge. `neutral`/`low` may move to `trusted` when AMS vouches; nothing else changes. + * An already-`trusted` submitter is unaffected, and no input can ever move a submitter DOWN -- the whole point + * of #6208's upgrade-only weighting. + */ +export function upgradeReputationSignal(local: ReputationSignal, amsTrusted: boolean): ReputationSignal { + return amsTrusted ? "trusted" : local; +} + +/** + * Fetch a login's AMS track record from the operator-configured endpoint. Returns null -- meaning "no bonus + * signal applied" -- when the bridge has no endpoint configured, the login is blank, or the call fails in ANY + * way (network error, timeout, non-OK status, non-array/malformed body). Never throws. + */ +export async function fetchAmsTrackRecord(login: string, options: AmsBridgeOptions): Promise { + const endpoint = typeof options.endpoint === "string" ? options.endpoint.trim().replace(/\/+$/, "") : ""; + const submitter = typeof login === "string" ? login.trim() : ""; + if (!endpoint || !submitter) return null; + const fetchImpl = options.fetchImpl ?? fetch; + const timeoutMs = options.timeoutMs ?? AMS_TRACK_RECORD_TIMEOUT_MS; + try { + const response = await fetchImpl(`${endpoint}/track-record/${encodeURIComponent(submitter)}`, { + method: "GET", + headers: { accept: "application/json" }, + signal: AbortSignal.timeout(timeoutMs), + }); + if (!response.ok) return null; + const payload: unknown = await response.json(); + // Accept either a bare array or a { pullRequests: [...] } envelope; anything else is malformed → no bonus. + const rows = Array.isArray(payload) ? payload : Array.isArray((payload as { pullRequests?: unknown })?.pullRequests) ? (payload as { pullRequests: unknown[] }).pullRequests : null; + if (!rows) return null; + return outcomesForLogin(rows, submitter); + } catch { + return null; // fail-safe: unreachable / timed out / malformed JSON ⇒ no bonus, never a throw into the gate. + } +} + +/** + * The bridge entry point: given the locally-computed signal, return the possibly-UPGRADED signal for `login`. + * Callers must only invoke this once `resolveConvergedFeature(env, manifest, "amsReputationBridge", repo)` is + * true; when the feature is off this is never reached and the local signal stands, byte-identical to today. + */ +export async function bridgeAmsReputation(local: ReputationSignal, login: string | undefined, options: AmsBridgeOptions): Promise { + // Already at the ceiling — skip the network entirely; an upgrade-only bridge has nothing to add. + if (local === "trusted") return local; + if (!login) return local; + const outcomes = await fetchAmsTrackRecord(login, options); + if (outcomes === null) return local; + return upgradeReputationSignal(local, amsRecordQualifiesAsTrusted(outcomes)); +} diff --git a/src/review/feature-activation.ts b/src/review/feature-activation.ts index a5b5a5a1a4..158f6b7f35 100644 --- a/src/review/feature-activation.ts +++ b/src/review/feature-activation.ts @@ -32,6 +32,7 @@ import { isConvergenceRepoAllowed } from "./cutover-gate"; import { isE2eTestGenerationEnabled } from "./e2e-test-gen-wire"; import { isGroundingEnabled } from "./grounding-wire"; import { isImprovementSignalEnabled } from "./improvement-signal-wire"; +import { isAmsReputationBridgeEnabled } from "./ams-reputation-bridge-wire"; import { isRagEnabled } from "./rag-wire"; import { isReputationEnabled } from "./reputation-wire"; import { isSafetyEnabled } from "./safety"; @@ -84,6 +85,7 @@ const FEATURE_GLOBAL_FLAG: Record boolean> = e2eTests: isE2eTestGenerationEnabled, screenshots: isScreenshotsEnabled, improvementSignal: isImprovementSignalEnabled, + amsReputationBridge: isAmsReputationBridgeEnabled, }; /** The named per-feature exceptions to `resolveConvergedFeature`'s default `"standard"` precedence — every diff --git a/test/unit/ams-reputation-bridge.test.ts b/test/unit/ams-reputation-bridge.test.ts new file mode 100644 index 0000000000..7ac8e1dea8 --- /dev/null +++ b/test/unit/ams-reputation-bridge.test.ts @@ -0,0 +1,258 @@ +import { describe, expect, it, vi } from "vitest"; +import { + AMS_BRIDGE_TRUSTED_MIN_MERGED, + AMS_TRACK_RECORD_TIMEOUT_MS, + amsRecordQualifiesAsTrusted, + bridgeAmsReputation, + fetchAmsTrackRecord, + outcomesForLogin, + upgradeReputationSignal, + type AmsTrackRecordFetch, +} from "../../src/review/ams-reputation-bridge"; +import { isAmsReputationBridgeEnabled, resolveAmsTrackRecordEndpoint } from "../../src/review/ams-reputation-bridge-wire"; + +const ENDPOINT = "https://ams.internal"; + +function outcome(overrides: Record = {}) { + return { repoFullName: "acme/widgets", authorLogin: "dev", state: "merged", ...overrides }; +} + +/** A fetchImpl returning `body` as JSON with `ok` derived from status. */ +function jsonFetch(body: unknown, status = 200): AmsTrackRecordFetch { + return async () => + ({ + ok: status >= 200 && status < 300, + status, + json: async () => body, + }) as unknown as Response; +} + +describe("isAmsReputationBridgeEnabled (#6485 kill-switch)", () => { + it("is OFF by default and for falsy/garbage values, ON only for the codebase's truthy convention", () => { + expect(isAmsReputationBridgeEnabled({})).toBe(false); + expect(isAmsReputationBridgeEnabled({ LOOPOVER_REVIEW_AMS_REPUTATION_BRIDGE: undefined })).toBe(false); + expect(isAmsReputationBridgeEnabled({ LOOPOVER_REVIEW_AMS_REPUTATION_BRIDGE: "" })).toBe(false); + expect(isAmsReputationBridgeEnabled({ LOOPOVER_REVIEW_AMS_REPUTATION_BRIDGE: "false" })).toBe(false); + expect(isAmsReputationBridgeEnabled({ LOOPOVER_REVIEW_AMS_REPUTATION_BRIDGE: "maybe" })).toBe(false); + for (const truthy of ["1", "true", "TRUE", "yes", "on", " on "]) { + expect(isAmsReputationBridgeEnabled({ LOOPOVER_REVIEW_AMS_REPUTATION_BRIDGE: truthy })).toBe(true); + } + }); +}); + +describe("resolveAmsTrackRecordEndpoint (#6485 operator config)", () => { + it("returns the configured URL, and undefined when unset or blank (⇒ no bonus signal)", () => { + expect(resolveAmsTrackRecordEndpoint({ LOOPOVER_AMS_TRACK_RECORD_URL: ENDPOINT })).toBe(ENDPOINT); + expect(resolveAmsTrackRecordEndpoint({ LOOPOVER_AMS_TRACK_RECORD_URL: ` ${ENDPOINT} ` })).toBe(ENDPOINT); + expect(resolveAmsTrackRecordEndpoint({})).toBeUndefined(); + expect(resolveAmsTrackRecordEndpoint({ LOOPOVER_AMS_TRACK_RECORD_URL: undefined })).toBeUndefined(); + expect(resolveAmsTrackRecordEndpoint({ LOOPOVER_AMS_TRACK_RECORD_URL: " " })).toBeUndefined(); + }); +}); + +describe("outcomesForLogin", () => { + it("keeps only the target login's rows, case-insensitively, and drops malformed ones", () => { + const rows = [ + outcome({ authorLogin: "dev" }), + outcome({ authorLogin: "DEV" }), // same login, different case + outcome({ authorLogin: "someone-else" }), + outcome({ authorLogin: " " }), // blank login → malformed + outcome({ authorLogin: 42 }), // non-string login → malformed + outcome({ state: 7 }), // non-string state → malformed + { authorLogin: "dev" }, // missing state → malformed + null, + "not-an-object", + ]; + expect(outcomesForLogin(rows, "dev")).toHaveLength(2); + }); + + it("returns nothing for a blank requested login", () => { + expect(outcomesForLogin([outcome()], " ")).toEqual([]); + }); +}); + +describe("amsRecordQualifiesAsTrusted", () => { + it("qualifies a genuinely strong record (enough merges, high merge rate)", () => { + const rows = [outcome(), outcome(), outcome(), outcome({ state: "closed" })]; // 3 merged / 4 terminal = 0.75 + expect(amsRecordQualifiesAsTrusted(rows)).toBe(true); + }); + + it("does not qualify a sparse record (below the merged floor) — sparse is no bonus, never a penalty", () => { + const rows = Array.from({ length: AMS_BRIDGE_TRUSTED_MIN_MERGED - 1 }, () => outcome()); + expect(amsRecordQualifiesAsTrusted(rows)).toBe(false); + }); + + it("does not qualify a high-volume but low-merge-rate record", () => { + // 3 merged / 10 terminal = 0.3, under the rate floor despite clearing the merged floor. + const rows = [...Array.from({ length: 3 }, () => outcome()), ...Array.from({ length: 7 }, () => outcome({ state: "closed" }))]; + expect(amsRecordQualifiesAsTrusted(rows)).toBe(false); + }); + + it("ignores non-terminal (open) PRs in the denominator — an open PR is not yet evidence either way", () => { + // 3 merged / 3 terminal = 1.0; the 5 open rows must not dilute the rate. + const rows = [...Array.from({ length: 3 }, () => outcome()), ...Array.from({ length: 5 }, () => outcome({ state: "open" }))]; + expect(amsRecordQualifiesAsTrusted(rows)).toBe(true); + }); + + it("does not qualify an empty record", () => { + expect(amsRecordQualifiesAsTrusted([])).toBe(false); + }); +}); + +describe("upgradeReputationSignal — upgrade-only (#6208)", () => { + it("upgrades neutral and low toward trusted when AMS vouches", () => { + expect(upgradeReputationSignal("neutral", true)).toBe("trusted"); + expect(upgradeReputationSignal("low", true)).toBe("trusted"); + }); + + it("NEVER downgrades: a non-vouching AMS record leaves every local signal exactly as-is", () => { + expect(upgradeReputationSignal("trusted", false)).toBe("trusted"); + expect(upgradeReputationSignal("neutral", false)).toBe("neutral"); + expect(upgradeReputationSignal("low", false)).toBe("low"); + }); +}); + +describe("fetchAmsTrackRecord — fail-safe pull", () => { + it("returns the login's outcomes from a bare-array payload", async () => { + const rows = await fetchAmsTrackRecord("dev", { endpoint: ENDPOINT, fetchImpl: jsonFetch([outcome(), outcome({ authorLogin: "other" })]) }); + expect(rows).toHaveLength(1); + }); + + it("accepts the { pullRequests: [...] } envelope too", async () => { + const rows = await fetchAmsTrackRecord("dev", { endpoint: ENDPOINT, fetchImpl: jsonFetch({ pullRequests: [outcome()] }) }); + expect(rows).toHaveLength(1); + }); + + it("builds the URL from the endpoint (trailing slashes trimmed) and encodes the login", async () => { + let seen = ""; + const fetchImpl: AmsTrackRecordFetch = async (url) => { + seen = url; + return { ok: true, status: 200, json: async () => [] } as unknown as Response; + }; + await fetchAmsTrackRecord("weird/login", { endpoint: `${ENDPOINT}///`, fetchImpl }); + expect(seen).toBe(`${ENDPOINT}/track-record/weird%2Flogin`); + }); + + it("returns null (no bonus) when no endpoint is configured or the login is blank", async () => { + expect(await fetchAmsTrackRecord("dev", { endpoint: undefined, fetchImpl: jsonFetch([outcome()]) })).toBeNull(); + expect(await fetchAmsTrackRecord("dev", { endpoint: " ", fetchImpl: jsonFetch([outcome()]) })).toBeNull(); + expect(await fetchAmsTrackRecord(" ", { endpoint: ENDPOINT, fetchImpl: jsonFetch([outcome()]) })).toBeNull(); + }); + + it("returns null for a non-string login (defensive: a JS caller can pass anything)", async () => { + expect(await fetchAmsTrackRecord(undefined as unknown as string, { endpoint: ENDPOINT, fetchImpl: jsonFetch([outcome()]) })).toBeNull(); + expect(await fetchAmsTrackRecord(42 as unknown as string, { endpoint: ENDPOINT, fetchImpl: jsonFetch([outcome()]) })).toBeNull(); + }); + + it("falls back to the global fetch when no fetchImpl is injected", async () => { + const calls: string[] = []; + vi.stubGlobal("fetch", async (url: string) => { + calls.push(url); + return { ok: true, status: 200, json: async () => [outcome()] } as unknown as Response; + }); + try { + const rows = await fetchAmsTrackRecord("dev", { endpoint: ENDPOINT }); + expect(rows).toHaveLength(1); + expect(calls).toEqual([`${ENDPOINT}/track-record/dev`]); + } finally { + vi.unstubAllGlobals(); + } + }); + + it("returns null on a non-OK status", async () => { + expect(await fetchAmsTrackRecord("dev", { endpoint: ENDPOINT, fetchImpl: jsonFetch([outcome()], 503) })).toBeNull(); + }); + + it("returns null on a malformed (non-array, non-envelope) body", async () => { + expect(await fetchAmsTrackRecord("dev", { endpoint: ENDPOINT, fetchImpl: jsonFetch({ nope: true }) })).toBeNull(); + expect(await fetchAmsTrackRecord("dev", { endpoint: ENDPOINT, fetchImpl: jsonFetch(null) })).toBeNull(); + }); + + it("returns null — never throws — when the endpoint is unreachable or times out", async () => { + const boom: AmsTrackRecordFetch = async () => { + throw new Error("ECONNREFUSED"); + }; + await expect(fetchAmsTrackRecord("dev", { endpoint: ENDPOINT, fetchImpl: boom })).resolves.toBeNull(); + + const timeout: AmsTrackRecordFetch = async () => { + throw Object.assign(new Error("The operation was aborted due to timeout"), { name: "TimeoutError" }); + }; + await expect(fetchAmsTrackRecord("dev", { endpoint: ENDPOINT, fetchImpl: timeout, timeoutMs: 1 })).resolves.toBeNull(); + }); + + it("returns null when the body itself fails to parse as JSON", async () => { + const badJson: AmsTrackRecordFetch = async () => + ({ + ok: true, + status: 200, + json: async () => { + throw new SyntaxError("Unexpected token"); + }, + }) as unknown as Response; + await expect(fetchAmsTrackRecord("dev", { endpoint: ENDPOINT, fetchImpl: badJson })).resolves.toBeNull(); + }); + + it("bounds the request with a short timeout signal so a slow AMS never stalls the gate", async () => { + let signal: AbortSignal | undefined; + const fetchImpl: AmsTrackRecordFetch = async (_url, init) => { + signal = init.signal as AbortSignal; + return { ok: true, status: 200, json: async () => [] } as unknown as Response; + }; + await fetchAmsTrackRecord("dev", { endpoint: ENDPOINT, fetchImpl }); + expect(signal).toBeInstanceOf(AbortSignal); + expect(AMS_TRACK_RECORD_TIMEOUT_MS).toBeLessThanOrEqual(1000); + }); +}); + +describe("bridgeAmsReputation — the entry point", () => { + const strong = [outcome(), outcome(), outcome()]; + + it("upgrades a neutral submitter whose AMS record vouches for them", async () => { + expect(await bridgeAmsReputation("neutral", "dev", { endpoint: ENDPOINT, fetchImpl: jsonFetch(strong) })).toBe("trusted"); + }); + + it("upgrades a low submitter too — AMS evidence can only help", async () => { + expect(await bridgeAmsReputation("low", "dev", { endpoint: ENDPOINT, fetchImpl: jsonFetch(strong) })).toBe("trusted"); + }); + + it("leaves the signal unchanged when the submitter has no AMS data", async () => { + expect(await bridgeAmsReputation("neutral", "dev", { endpoint: ENDPOINT, fetchImpl: jsonFetch([]) })).toBe("neutral"); + expect(await bridgeAmsReputation("low", "dev", { endpoint: ENDPOINT, fetchImpl: jsonFetch([]) })).toBe("low"); + }); + + it("leaves the signal unchanged — and never throws — when AMS is unreachable", async () => { + const boom: AmsTrackRecordFetch = async () => { + throw new Error("ECONNREFUSED"); + }; + await expect(bridgeAmsReputation("low", "dev", { endpoint: ENDPOINT, fetchImpl: boom })).resolves.toBe("low"); + }); + + it("skips the network entirely for an already-trusted submitter (nothing to upgrade)", async () => { + let called = false; + const spy: AmsTrackRecordFetch = async () => { + called = true; + return { ok: true, status: 200, json: async () => [] } as unknown as Response; + }; + expect(await bridgeAmsReputation("trusted", "dev", { endpoint: ENDPOINT, fetchImpl: spy })).toBe("trusted"); + expect(called).toBe(false); + }); + + it("leaves the signal unchanged when there is no submitter login", async () => { + expect(await bridgeAmsReputation("neutral", undefined, { endpoint: ENDPOINT, fetchImpl: jsonFetch(strong) })).toBe("neutral"); + }); + + it("never downgrades even when the AMS record looks bad — the upgrade-only guarantee", async () => { + // A deliberately terrible AMS record (0 merged / 9 closed) must still leave the local signal untouched. + const terrible = Array.from({ length: 9 }, () => outcome({ state: "closed" })); + expect(await bridgeAmsReputation("neutral", "dev", { endpoint: ENDPOINT, fetchImpl: jsonFetch(terrible) })).toBe("neutral"); + expect(await bridgeAmsReputation("trusted", "dev", { endpoint: ENDPOINT, fetchImpl: jsonFetch(terrible) })).toBe("trusted"); + }); + + it("carries no score/wallet/hotkey fields through the bridge (privacy-safe by construction)", async () => { + // Even if an AMS instance sends extra keys, the bridge's output is a bare signal — nothing can cross. + const withJunk = [outcome({ score: 99, wallet: "w", hotkey: "h" }), outcome(), outcome()]; + const result = await bridgeAmsReputation("neutral", "dev", { endpoint: ENDPOINT, fetchImpl: jsonFetch(withJunk) }); + expect(result).toBe("trusted"); + expect(JSON.stringify(result)).not.toMatch(/wallet|hotkey|score/i); + }); +}); diff --git a/test/unit/feature-activation.test.ts b/test/unit/feature-activation.test.ts index 18610a03a8..d527557261 100644 --- a/test/unit/feature-activation.test.ts +++ b/test/unit/feature-activation.test.ts @@ -15,6 +15,7 @@ const FLAG: Record = { e2eTests: "LOOPOVER_REVIEW_E2E_TESTS", screenshots: "LOOPOVER_REVIEW_SCREENSHOTS", improvementSignal: "LOOPOVER_REVIEW_IMPROVEMENT_SIGNAL", + amsReputationBridge: "LOOPOVER_REVIEW_AMS_REPUTATION_BRIDGE", }; function env(overrides: Record): Env { diff --git a/test/unit/focus-manifest.test.ts b/test/unit/focus-manifest.test.ts index 028e111dd5..2377bcd994 100644 --- a/test/unit/focus-manifest.test.ts +++ b/test/unit/focus-manifest.test.ts @@ -430,6 +430,7 @@ describe(".loopover.yml.example field-exhaustiveness (#1670)", () => { e2eTests: "e2eTests:", screenshots: "screenshots:", improvementSignal: "improvementSignal:", + amsReputationBridge: "amsReputationBridge:", } satisfies Record, string>; it.each(Object.entries(FEATURES_FIELD_TOKENS))("documents features.%s", (_field, token) => { @@ -914,7 +915,7 @@ describe("compileFocusManifestPolicy", () => { gate: { present: false, enabled: null, checkMode: null, pack: null, linkedIssue: null, duplicates: null, readinessMode: null, readinessMinScore: null, slopMode: null, slopMinScore: null, slopAiAdvisory: null, sizeMode: null, sizeMaxFiles: null, sizeMaxLines: null, lockfileIntegrityMode: null, aiReviewMode: null, aiReviewByok: null, aiReviewProvider: null, aiReviewModel: null, aiReviewAllAuthors: null, aiReviewCloseConfidence: null, aiReviewLowConfidenceDisposition: null, aiReviewCombine: null, aiReviewOnMerge: null, aiReviewReviewers: null, mergeReadiness: null, selfAuthoredLinkedIssue: null, linkedIssueSatisfaction: null, manifestPolicy: null, dryRun: null, firstTimeContributorGrace: null, premergeContentRecheck: null, requireFreshRebaseWindowMinutes: null, claMode: null, claConsentPhrase: null, claCheckRunName: null, claCheckRunAppSlug: null, expectedCiContexts: null, advisoryCheckRuns: null, aiJudgmentBlockersMode: null, copycatMode: null, copycatMinScore: null }, settings: {}, review: { present: false, footerText: null, note: null, fields: {}, enrichmentAnalyzers: {}, profile: null, tone: null, securityFocus: null, inlineComments: null, fixHandoff: null, autoMergeSummary: null, suggestions: null, changedFilesSummary: null, effortScore: null, impactMap: null, cultureProfile: null, selftune: null, sweepWatchdog: null, prReconciliation: null, reviewMemory: null, findingCategories: null, inlineCommentsPerCategory: null, minFindingSeverity: null, maxFindings: { blockers: null, nits: null }, commentVerbosity: null, e2eTestDelivery: null, e2eTestAutoTrigger: null, pathInstructions: [], instructions: null, excludePaths: [], pathFilters: [], preMergeChecks: [], autoReview: { ...EMPTY_AUTO_REVIEW_CONFIG }, aiModel: { ...EMPTY_SELF_HOST_AI_MODEL_CONFIG }, visual: { ...EMPTY_VISUAL_CONFIG }, linkedIssueSatisfaction: null, sharedConfigSource: null }, - features: { present: false, rag: null, reputation: null, safety: null, grounding: null, e2eTests: null, screenshots: null, improvementSignal: null }, + features: { present: false, rag: null, reputation: null, safety: null, grounding: null, e2eTests: null, screenshots: null, improvementSignal: null, amsReputationBridge: null }, experimental: { present: false, gittensor: null }, contentLane: { present: false, entryFileGlob: null, providerFileGlob: null, artifactGlob: null, collectionField: null, maxAppendedEntries: null, duplicateKeyFields: [], validatorId: null }, repoDocGeneration: { present: false, enabled: false, scope: ["agents"], allowOverwriteExisting: false, refreshIntervalDays: 7 }, diff --git a/test/unit/visual-wire.test.ts b/test/unit/visual-wire.test.ts index 82f44e214b..98cbdf7ee1 100644 --- a/test/unit/visual-wire.test.ts +++ b/test/unit/visual-wire.test.ts @@ -26,7 +26,7 @@ describe("isScreenshotsEnabled", () => { describe("screenshots converged-feature activation (env flag AND repo cutover allowlist, with manifest opt-out)", () => { const repo = "JSONbored/gittensory"; const noOverride: Pick = { - features: { present: false, rag: null, reputation: null, safety: null, grounding: null, e2eTests: null, screenshots: null, improvementSignal: null }, + features: { present: false, rag: null, reputation: null, safety: null, grounding: null, e2eTests: null, screenshots: null, improvementSignal: null, amsReputationBridge: null }, }; it("requires BOTH the global flag and the repo allowlist when no override is set", () => {