diff --git a/.release-please-manifest.json b/.release-please-manifest.json index a112e0abc4..effd9a1389 100644 --- a/.release-please-manifest.json +++ b/.release-please-manifest.json @@ -1,6 +1,6 @@ { "packages/loopover-mcp": "3.14.1", - "packages/loopover-engine": "3.14.1", + "packages/loopover-engine": "3.15.0", "packages/loopover-miner": "3.14.1", "packages/loopover-ui-kit": "1.2.0" } diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 70cc8736be..b35a438827 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -7348,6 +7348,39 @@ export function maybeAddRequiredAutoReviewSkipHold( return true; } +/** + * #9015 — the REPUTATION skip's fail-closed hold, the exact sibling of the contributor-controlled skip + * above. A reputation downgrade (low signal, or the submissions>=8/merged<1 burst) suppresses AI review + * entirely; without this hold the PR then proceeds on deterministic checks alone — none of which read code + * semantics — and can auto-merge with ZERO defect detection. That inverts the feature's intent: a SUSPECTED + * abuser would receive LESS scrutiny than a trusted contributor. Where the repo requires blocking AI review, + * the skip must therefore hold for a human rather than silently pass. PURE (mutates the advisory it is + * given, like its sibling); the caller owns the reputation decision itself. + */ +export function maybeAddReputationSkipHold( + env: Env, + args: { + settings: RepositorySettings; + advisory: Pick>, "headSha" | "findings">; + repoFullName: string; + author: string | null; + confirmedContributor: boolean; + skipAiReview?: boolean | undefined; + reputationSkipped: boolean; + }, +): boolean { + if (!args.reputationSkipped || !shouldRequirePublicAiReviewForAdvisory(env, args)) return false; + args.advisory.findings.push({ + code: "ai_review_inconclusive", + severity: "warning", + title: "Required AI review was skipped by a submitter-reputation downgrade", + detail: + "This repository requires blocking AI review, and the submitter's recent-submission signal downgraded this PR to deterministic-only checks. Those checks do not read code semantics, so the gate is held for human review instead of passing automatically.", + action: "Review this PR manually, or run AI review with a trusted override, before merging.", + }); + return true; +} + /** Record a quiet auto-review skip (never a gate failure). Exported for unit tests. (#1954) */ export async function auditPullRequestAutoReviewSkip( env: Env, @@ -9926,6 +9959,19 @@ async function maybePublishPrPublicSurface( isReputationEnabled(env) && isConvergenceRepoAllowed(env, repoFullName) ? await shouldSkipAiForReputation(env, { project: repoFullName, submitter: author }) : undefined; + // #9015: the reputation skip must FAIL CLOSED wherever blocking AI review is required — otherwise a + // suspected abuser's PR proceeds on deterministic checks alone (no code-semantics review at all) and can + // auto-merge, i.e. suspicion would BUY less scrutiny. Exact sibling of the contributor-controlled skip + // hold above; a no-op when the repo does not require blocking AI review, or when no skip fired. + maybeAddReputationSkipHold(env, { + settings, + advisory, + repoFullName, + author, + confirmedContributor, + skipAiReview: webhook.skipAiReview, + reputationSkipped: preComputedReputationSkip === true, + }); // #one-shot-review-cadence: only even attempts the lookup when the review would otherwise be eligible to // run fresh this pass (mirrors how the frozen/paused branches below are similarly mutually exclusive) -- // a PR that's blacklisted/frozen/already-skipped for another reason never shows AI content at all today, diff --git a/src/review/submitter-reputation.ts b/src/review/submitter-reputation.ts index 5ff0941789..7997def8b6 100644 --- a/src/review/submitter-reputation.ts +++ b/src/review/submitter-reputation.ts @@ -111,8 +111,13 @@ export function isMachinePacedCadence(cadence: SubmissionCadence): boolean { export async function getSubmitterCadence(env: Env, project: string, submitter: string | undefined): Promise { if (!submitter) return { count: 0, medianGapMs: null }; try { + // #9015: reads the LIVE ledger. This query previously read `review_targets`, which stopped receiving + // writes at the 2026-06-22 self-host cutover — the cadence leg was silently inert (newest row frozen at + // the cutover date), so the machine-paced signal never fired for any submitter. `pull_requests` is the + // live per-PR table the same pipeline maintains; `created_at` is its ingest timestamp, which is what a + // SUBMISSION cadence is about. const result = await storage(env) - .prepare(`SELECT created_at AS createdAt FROM review_targets WHERE project = ? AND submitter = ? AND created_at >= datetime('now', ?) ORDER BY created_at DESC LIMIT ?`) + .prepare(`SELECT created_at AS createdAt FROM pull_requests WHERE repo_full_name = ? AND LOWER(author_login) = LOWER(?) AND created_at >= datetime('now', ?) ORDER BY created_at DESC LIMIT ?`) .bind(project, submitter, `-${CADENCE_WINDOW_HOURS} hours`, REPUTATION_WINDOW_ROW_CAP) .all<{ createdAt: string }>(); const createdAts = (result?.results ?? []).map((r) => r.createdAt); diff --git a/test/unit/auto-review-wiring.test.ts b/test/unit/auto-review-wiring.test.ts index 42c9f4aba5..09e761fd62 100644 --- a/test/unit/auto-review-wiring.test.ts +++ b/test/unit/auto-review-wiring.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it, vi } from "vitest"; import { auditPullRequestAutoReviewSkip, maybeAddRequiredAutoReviewSkipHold, + maybeAddReputationSkipHold, resolveAutoReviewSkipForPullRequest, resolveReviewManifestForAiReview, } from "../../src/queue/processors"; @@ -405,6 +406,52 @@ describe("review.auto_review wiring (#1954)", () => { loadSpy.mockRestore(); }); + it("#9015: a reputation skip HOLDS where blocking AI review is required — suspicion must never buy less scrutiny", () => { + const blockingEnv = { AI_SUMMARIES_ENABLED: "true", AI_PUBLIC_COMMENTS_ENABLED: "true", AI: {} } as Env; + const settings = { gatePack: "oss-anti-slop", aiReviewMode: "block", aiReviewAllAuthors: false } as never; + const advisory = { headSha: "sha", findings: [] as unknown[] }; + const added = maybeAddReputationSkipHold(blockingEnv, { + settings, + advisory: advisory as never, + repoFullName: "acme/widgets", + author: "burst-farmer", + confirmedContributor: false, + reputationSkipped: true, + }); + expect(added).toBe(true); + expect(advisory.findings).toEqual([ + expect.objectContaining({ code: "ai_review_inconclusive", severity: "warning", title: expect.stringContaining("submitter-reputation") }), + ]); + // No skip fired: nothing is added (the overwhelmingly common path). + const untouched = { headSha: "sha", findings: [] as unknown[] }; + expect( + maybeAddReputationSkipHold(blockingEnv, { + settings, + advisory: untouched as never, + repoFullName: "acme/widgets", + author: "alice", + confirmedContributor: false, + reputationSkipped: false, + }), + ).toBe(false); + expect(untouched.findings).toEqual([]); + // AI review is OFF for this repo: nothing was expected to run, so a skip is not a suppression — silent, + // exactly like the contributor-controlled sibling (both share shouldRequirePublicAiReviewForAdvisory). + // In advisory mode the finding IS recorded and is non-blocking by nature there. + const reviewOff = { headSha: "sha", findings: [] as unknown[] }; + expect( + maybeAddReputationSkipHold(blockingEnv, { + settings: { gatePack: "oss-anti-slop", aiReviewMode: "off", aiReviewAllAuthors: false } as never, + advisory: reviewOff as never, + repoFullName: "acme/widgets", + author: "burst-farmer", + confirmedContributor: false, + reputationSkipped: true, + }), + ).toBe(false); + expect(reviewOff.findings).toEqual([]); + }); + it("holds instead of quietly skipping when contributor-controlled metadata suppresses required AI review", () => { const advisory = { headSha: "sha", findings: [] }; const added = maybeAddRequiredAutoReviewSkipHold( diff --git a/test/unit/submitter-reputation.test.ts b/test/unit/submitter-reputation.test.ts index a1b8e24fca..c5f28b8d2d 100644 --- a/test/unit/submitter-reputation.test.ts +++ b/test/unit/submitter-reputation.test.ts @@ -416,6 +416,22 @@ describe("getSubmitterCadence (D1, fail-safe) (#4514)", () => { expect(await getSubmitterCadence({} as Env, "p", undefined)).toEqual({ count: 0, medianGapMs: null }); }); + it("#9015: queries the LIVE pull_requests ledger, not the frozen review_targets table", async () => { + let sql = ""; + const env = { + DB: { + prepare: (query: string) => { + sql = query; + return { bind: () => ({ all: async () => ({ results: [] }) }) }; + }, + }, + } as unknown as Env; + await getSubmitterCadence(env, "acme/widgets", "farmer99"); + expect(sql).toContain("FROM pull_requests"); + expect(sql).not.toContain("review_targets"); + expect(sql).toContain("author_login"); + }); + it("derives cadence from the queried created_at timestamps", async () => { const t0 = new Date("2026-01-01T00:00:00.000Z").getTime(); const env = makeCadenceEnv([t0, t0 + 5 * 60_000, t0 + 10 * 60_000, t0 + 15 * 60_000, t0 + 20 * 60_000].map((ms) => new Date(ms).toISOString()));