From 8f6d9a58cda06559185982360fd98fdeecd8ea53 Mon Sep 17 00:00:00 2001 From: RealDiligent Date: Sat, 18 Jul 2026 02:35:31 +0800 Subject: [PATCH] feat(api): REST + CLI mirror for loopover_explain_review_risk (#6980) Expose review-risk explanations via POST /v1/preflight/review-risk and stdio/shell CLI so the tool is not MCP-only. Co-authored-by: Cursor --- apps/loopover-ui/public/openapi.json | 68 ++++++++++++ packages/loopover-mcp/bin/loopover-mcp.js | 72 ++++++++++++ src/api/routes.ts | 20 ++++ src/mcp/server.ts | 23 +--- src/openapi/schemas.ts | 9 ++ src/openapi/spec.ts | 12 ++ src/signals/review-risk.ts | 56 ++++++++++ test/unit/mcp-cli-review-risk.test.ts | 124 +++++++++++++++++++++ test/unit/mcp-tool-rename-aliases.test.ts | 11 +- test/unit/openapi.test.ts | 1 + test/unit/routes-review-risk.test.ts | 127 ++++++++++++++++++++++ test/unit/support/mcp-cli-harness.ts | 45 ++++++++ 12 files changed, 546 insertions(+), 22 deletions(-) create mode 100644 src/signals/review-risk.ts create mode 100644 test/unit/mcp-cli-review-risk.test.ts create mode 100644 test/unit/routes-review-risk.test.ts diff --git a/apps/loopover-ui/public/openapi.json b/apps/loopover-ui/public/openapi.json index 51b9ee2a65..59b5136ab2 100644 --- a/apps/loopover-ui/public/openapi.json +++ b/apps/loopover-ui/public/openapi.json @@ -14490,6 +14490,43 @@ "login", "marked" ] + }, + "ReviewRiskExplanation": { + "type": "object", + "properties": { + "preflight": { + "$ref": "#/components/schemas/PreflightResult" + }, + "roleContext": { + "allOf": [ + { + "$ref": "#/components/schemas/RoleContext" + }, + { + "nullable": true + } + ] + }, + "recommendation": { + "type": "string", + "enum": [ + "likely_duplicate", + "maintainer_lane", + "needs_author", + "review", + "watch" + ] + }, + "summary": { + "type": "string" + } + }, + "required": [ + "preflight", + "roleContext", + "recommendation", + "summary" + ] } }, "parameters": {}, @@ -18869,6 +18906,37 @@ } ] } + }, + "/v1/preflight/review-risk": { + "post": { + "summary": "Explain review risk for a planned pull request", + "responses": { + "200": { + "description": "Review-risk explanation with preflight, role context, and recommendation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ReviewRiskExplanation" + } + } + } + }, + "400": { + "description": "Invalid preflight input" + }, + "403": { + "description": "Forbidden when contributorLogin does not match the authenticated session" + } + }, + "security": [ + { + "LoopOverBearer": [] + }, + { + "LoopOverSessionCookie": [] + } + ] + } } }, "servers": [ diff --git a/packages/loopover-mcp/bin/loopover-mcp.js b/packages/loopover-mcp/bin/loopover-mcp.js index bfc33b627f..02e368bf36 100644 --- a/packages/loopover-mcp/bin/loopover-mcp.js +++ b/packages/loopover-mcp/bin/loopover-mcp.js @@ -90,6 +90,7 @@ const CLI_COMMAND_SPEC = { "contributor-profile": [], "monitor-open-prs": [], "pr-outcomes": [], + "explain-review-risk": [], notifications: [], "notifications-read": [], "analyze-branch": [], @@ -936,6 +937,11 @@ const STDIO_TOOL_DESCRIPTORS = [ category: "discovery", description: "Preflight planned PR metadata against lane, duplicate, linked issue, test, and queue signals.", }, + { + name: "loopover_explain_review_risk", + category: "review", + description: "Explain review risk for a planned PR using preflight, lane, duplicate, and role context.", + }, { name: "loopover_validate_linked_issue", category: "discovery", @@ -1516,6 +1522,19 @@ registerStdioTool( async (input) => toolResult("LoopOver PR preflight.", await apiPost("/v1/preflight/pr", input)), ); +// #6980: CLI stdio mirror of loopover_explain_review_risk — proxies POST /v1/preflight/review-risk. +registerStdioTool( + "loopover_explain_review_risk", + { + description: stdioToolDescription("loopover_explain_review_risk"), + inputSchema: preflightShape, + }, + async (input) => { + const payload = await apiPost("/v1/preflight/review-risk", input); + return toolResult(payload.summary ?? `LoopOver review-risk explanation for ${input.repoFullName}.`, payload); + }, +); + registerStdioTool( "loopover_validate_linked_issue", { @@ -3401,6 +3420,7 @@ async function runCli(args) { if (command === "contributor-profile") return contributorProfileCli(options); if (command === "monitor-open-prs") return monitorOpenPrsCli(options); if (command === "pr-outcomes") return prOutcomesCli(options); + if (command === "explain-review-risk") return explainReviewRiskCli(options); if (command === "notifications") return notificationsCli(options); if (command === "notifications-read") return notificationsReadCli(options); if (command === "review-pr") return reviewPrCli(options); @@ -3904,6 +3924,57 @@ async function prOutcomesCli(options) { } } +function printExplainReviewRiskHelp() { + process.stdout.write( + [ + "Usage: loopover-mcp explain-review-risk --repo owner/repo --title [--login ] [--body ] [--json]", + "", + "Explain review risk for a planned PR (preflight + optional role context + recommendation).", + "Mirrors the loopover_explain_review_risk MCP tool and POST /v1/preflight/review-risk. No source upload.", + "", + "Pass --repo or --repoFullName, --title, and optionally --login as contributorLogin.", + "Pass --json for machine-readable output.", + ].join("\n") + "\n", + ); +} + +async function explainReviewRiskCli(options) { + if (options.help === true) return printExplainReviewRiskHelp(); + const repoFullName = options.repoFullName ?? options.repo; + if (!repoFullName || !String(repoFullName).includes("/")) throw new Error("Pass --repo owner/repo or --repoFullName owner/repo."); + if (!options.title) throw new Error("Pass --title ."); + const contributorLogin = options.login ?? options.contributorLogin; + const labels = Array.isArray(options.label) ? options.label : options.label ? [options.label] : undefined; + const changedFiles = Array.isArray(options.changedFile) ? options.changedFile : options.changedFile ? [options.changedFile] : undefined; + const linkedIssues = Array.isArray(options.issue) + ? options.issue.map((value) => Number(value)).filter((value) => Number.isInteger(value) && value > 0) + : options.issue + ? [Number(options.issue)].filter((value) => Number.isInteger(value) && value > 0) + : undefined; + const tests = Array.isArray(options.test) ? options.test : options.test ? [options.test] : undefined; + const payload = await apiPost( + "/v1/preflight/review-risk", + stripUndefined({ + repoFullName, + title: options.title, + contributorLogin, + body: options.body, + labels, + changedFiles, + linkedIssues: linkedIssues && linkedIssues.length > 0 ? linkedIssues : undefined, + tests, + authorAssociation: options.authorAssociation, + }), + ); + if (options.json) { + process.stdout.write(`${JSON.stringify(payload, null, 2)}\n`); + return; + } + process.stdout.write(`${sanitizePlainTextTerminalOutput(payload.summary ?? `LoopOver review-risk explanation for ${repoFullName}.`)}\n`); + if (payload.recommendation) process.stdout.write(`Recommendation: ${sanitizePlainTextTerminalOutput(payload.recommendation)}\n`); + if (payload.preflight?.status) process.stdout.write(`Preflight status: ${sanitizePlainTextTerminalOutput(payload.preflight.status)}\n`); +} + function printNotificationsHelp() { process.stdout.write( [ @@ -4444,6 +4515,7 @@ function printHelp() { loopover-mcp repo-decision --login --repo owner/repo [--json] loopover-mcp monitor-open-prs --login [--json] loopover-mcp pr-outcomes --login [--limit N] [--json] + loopover-mcp explain-review-risk --repo owner/repo --title [--login ] [--body ] [--json] loopover-mcp notifications --login [--json] loopover-mcp notifications-read --login [--id ]... [--json] loopover-mcp analyze-branch --login [--repo owner/repo] [--base origin/main] [--branch-eligibility eligible|ineligible|unknown] [--pending-merged-prs 3] [--expected-open-prs 0] [--projected-credibility 0.8] [--scenario-note "..."] [--validation "passed|npm test|summary"] [--format table] [--json] diff --git a/src/api/routes.ts b/src/api/routes.ts index 911b0c6eaf..cf6eb14928 100644 --- a/src/api/routes.ts +++ b/src/api/routes.ts @@ -275,6 +275,7 @@ import { import { attachDataQuality, buildCoreSignalFidelity, buildFreshnessSloReport, buildRepoDataQuality, buildSignalFidelity } from "../signals/data-quality"; import { buildContributorOpenPrMonitor } from "../signals/contributor-open-pr-monitor"; import { buildContributorPrOutcomes } from "../signals/contributor-pr-outcomes"; +import { buildReviewRiskExplanation } from "../signals/review-risk"; import { buildNotificationFeed } from "../notifications/service"; import { buildPullRequestReviewability, type PullRequestReviewability } from "../signals/reward-risk"; import { buildLocalBranchAnalysis, findCurrentBranchPullRequest } from "../signals/local-branch"; @@ -3598,6 +3599,25 @@ export function createApp() { return c.json(buildPreflightResult(parsed.data, repo, issues, pullRequests, bounties, issueQuality?.report)); }); + // #6980: REST mirror of loopover_explain_review_risk — same preflightSchema as /v1/preflight/pr, richer + // payload (preflight + optional roleContext + recommendation + summary). Does NOT pass issueQuality. + app.post("/v1/preflight/review-risk", async (c) => { + const body = await c.req.json().catch(() => null); + const parsed = preflightSchema.safeParse(body); + if (!parsed.success) return c.json({ error: "invalid_preflight_request", issues: parsed.error.issues }, 400); + if (parsed.data.contributorLogin) { + const unauthorized = await requireContributorAccess(c, parsed.data.contributorLogin); + if (unauthorized) return unauthorized; + } + const [repo, issues, pullRequests, bounties] = await Promise.all([ + getRepository(c.env, parsed.data.repoFullName), + listIssues(c.env, parsed.data.repoFullName), + listPullRequests(c.env, parsed.data.repoFullName), + listBountiesByRepo(c.env, parsed.data.repoFullName), + ]); + return c.json(buildReviewRiskExplanation({ input: parsed.data, repo, issues, pullRequests, bounties })); + }); + app.post("/v1/preflight/local-diff", async (c) => { const body = await c.req.json().catch(() => null); const parsed = localDiffPreflightSchema.safeParse(body); diff --git a/src/mcp/server.ts b/src/mcp/server.ts index d77180447b..689cbd5254 100644 --- a/src/mcp/server.ts +++ b/src/mcp/server.ts @@ -138,11 +138,11 @@ import { buildPrTextLint, buildQueueHealth, buildRegistryChangeReport, - buildRoleContext, } from "../signals/engine"; import { PUBLIC_SURFACE_SKIP_REASONS, skippedPrAuditRemediation, type PublicSurfaceSkipReason } from "../signals/settings-preview"; import { buildContributorOpenPrMonitor } from "../signals/contributor-open-pr-monitor"; import { buildContributorPrOutcomes } from "../signals/contributor-pr-outcomes"; +import { buildReviewRiskExplanation } from "../signals/review-risk"; import { buildLocalBranchAnalysis, findCurrentBranchPullRequest } from "../signals/local-branch"; import { computeLocalScorerTokens } from "../signals/local-scorer"; import { buildPullRequestReviewability, type PullRequestReviewability } from "../signals/reward-risk"; @@ -4209,24 +4209,13 @@ export class LoopoverMcp { listPullRequests(this.env, input.repoFullName), listBountiesByRepo(this.env, input.repoFullName), ]); - const preflight = buildPreflightResult(input, repo, issues, pullRequests, bounties); - const roleContext = input.contributorLogin - ? buildRoleContext({ login: input.contributorLogin, repo, repoFullName: input.repoFullName, pullRequests, issues }) - : null; + const explanation = buildReviewRiskExplanation({ input, repo, issues, pullRequests, bounties }); return { - summary: `LoopOver review-risk explanation for ${input.repoFullName}.`, + summary: explanation.summary, data: { - preflight, - roleContext, - recommendation: preflight.collisions.some((cluster) => cluster.risk === "high") - ? "likely_duplicate" - : roleContext?.maintainerLane - ? "maintainer_lane" - : preflight.status === "needs_work" - ? "needs_author" - : preflight.status === "ready" - ? "review" - : "watch", + preflight: explanation.preflight, + roleContext: explanation.roleContext, + recommendation: explanation.recommendation, }, }; } diff --git a/src/openapi/schemas.ts b/src/openapi/schemas.ts index 35ffffbc58..daf774cd7d 100644 --- a/src/openapi/schemas.ts +++ b/src/openapi/schemas.ts @@ -1908,6 +1908,15 @@ export const RoleContextSchema = z }) .openapi("RoleContext"); +export const ReviewRiskExplanationSchema = z + .object({ + preflight: PreflightResultSchema, + roleContext: RoleContextSchema.nullable(), + recommendation: z.enum(["likely_duplicate", "maintainer_lane", "needs_author", "review", "watch"]), + summary: z.string(), + }) + .openapi("ReviewRiskExplanation"); + const ContributorOutcomeCountsSchema = z.object({ pullRequests: z.number(), mergedPullRequests: z.number(), diff --git a/src/openapi/spec.ts b/src/openapi/spec.ts index 78812f79bf..574e765773 100644 --- a/src/openapi/spec.ts +++ b/src/openapi/spec.ts @@ -77,6 +77,7 @@ import { RepositorySettingsSchema, RepoDocRefreshResultSchema, RoleContextSchema, + ReviewRiskExplanationSchema, RewardRiskActionSchema, ScorePreviewSchema, ScoringModelSnapshotSchema, @@ -123,6 +124,7 @@ export function buildOpenApiSpec() { registry.register("RepoFitRecommendation", RepoFitRecommendationSchema); registry.register("PreflightResult", PreflightResultSchema); registry.register("LocalDiffPreflightResult", LocalDiffPreflightResultSchema); + registry.register("ReviewRiskExplanation", ReviewRiskExplanationSchema); registry.register("LocalBranchAnalysis", LocalBranchAnalysisSchema); registry.register("MaintainerPacket", MaintainerPacketSchema); registry.register("MaintainerLaneReport", MaintainerLaneReportSchema); @@ -847,6 +849,16 @@ export function buildOpenApiSpec() { 400: { description: "Invalid preflight input" }, }, }); + registry.registerPath({ + method: "post", + path: "/v1/preflight/review-risk", + summary: "Explain review risk for a planned pull request", + responses: { + 200: { description: "Review-risk explanation with preflight, role context, and recommendation", content: { "application/json": { schema: ReviewRiskExplanationSchema } } }, + 400: { description: "Invalid preflight input" }, + 403: { description: "Forbidden when contributorLogin does not match the authenticated session" }, + }, + }); registry.registerPath({ method: "post", path: "/v1/preflight/local-diff", diff --git a/src/signals/review-risk.ts b/src/signals/review-risk.ts new file mode 100644 index 0000000000..593a51d51a --- /dev/null +++ b/src/signals/review-risk.ts @@ -0,0 +1,56 @@ +import type { BountyRecord, IssueRecord, PullRequestRecord, RepositoryRecord } from "../types"; +import { + buildPreflightResult, + buildRoleContext, + type PreflightInput, + type PreflightResult, + type RoleContext, +} from "./engine"; + +export type ReviewRiskRecommendation = + | "likely_duplicate" + | "maintainer_lane" + | "needs_author" + | "review" + | "watch"; + +export type ReviewRiskExplanation = { + preflight: PreflightResult; + roleContext: RoleContext | null; + recommendation: ReviewRiskRecommendation; + summary: string; +}; + +/** + * Review-risk explanation for a planned PR — shared by `loopover_explain_review_risk` + * and `POST /v1/preflight/review-risk`. Uses the same `buildPreflightResult` core as + * PR preflight (without issueQuality) plus optional per-contributor role context. + */ +export function buildReviewRiskExplanation(args: { + input: PreflightInput; + repo: RepositoryRecord | null; + issues: IssueRecord[]; + pullRequests: PullRequestRecord[]; + bounties?: BountyRecord[]; +}): ReviewRiskExplanation { + const { input, repo, issues, pullRequests, bounties = [] } = args; + const preflight = buildPreflightResult(input, repo, issues, pullRequests, bounties); + const roleContext = input.contributorLogin + ? buildRoleContext({ login: input.contributorLogin, repo, repoFullName: input.repoFullName, pullRequests, issues }) + : null; + const recommendation: ReviewRiskRecommendation = preflight.collisions.some((cluster) => cluster.risk === "high") + ? "likely_duplicate" + : roleContext?.maintainerLane + ? "maintainer_lane" + : preflight.status === "needs_work" + ? "needs_author" + : preflight.status === "ready" + ? "review" + : "watch"; + return { + preflight, + roleContext, + recommendation, + summary: `LoopOver review-risk explanation for ${input.repoFullName}.`, + }; +} diff --git a/test/unit/mcp-cli-review-risk.test.ts b/test/unit/mcp-cli-review-risk.test.ts new file mode 100644 index 0000000000..ffea7df5b0 --- /dev/null +++ b/test/unit/mcp-cli-review-risk.test.ts @@ -0,0 +1,124 @@ +// #6980: CLI + stdio mirrors for loopover_explain_review_risk. The host MCP tool already existed; this pins +// the REST-backed stdio proxy and shell CLI against the same fixture payload. +import { Client } from "@modelcontextprotocol/sdk/client/index.js"; +import { StdioClientTransport } from "@modelcontextprotocol/sdk/client/stdio.js"; +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { closeFixtureServer, reviewRiskFixture, run, runAsync, runExpectingFailure, startFixtureServer } from "./support/mcp-cli-harness"; + +const bin = join(process.cwd(), "packages/loopover-mcp/bin/loopover-mcp.js"); + +let client: Client; +let transport: StdioClientTransport; +let configDir: string; +let apiUrl: string; +let capturedBodies: unknown[]; + +async function connect() { + configDir = mkdtempSync(join(tmpdir(), "loopover-review-risk-")); + capturedBodies = []; + apiUrl = await startFixtureServer({ + onReviewRiskRequest: (body) => { + capturedBodies.push(body); + }, + }); + transport = new StdioClientTransport({ + command: "node", + args: [bin, "--stdio"], + env: { + ...process.env, + LOOPOVER_CONFIG_DIR: configDir, + LOOPOVER_API_URL: apiUrl, + LOOPOVER_TOKEN: "session-token", + LOOPOVER_API_TIMEOUT_MS: "5000", + }, + }); + client = new Client({ name: "review-risk-test", version: "0.0.1" }); + await client.connect(transport); +} + +async function disconnect() { + await client.close().catch(() => undefined); + await closeFixtureServer(); + if (configDir) rmSync(configDir, { recursive: true, force: true }); +} + +describe("loopover_explain_review_risk stdio proxy (#6980)", () => { + beforeEach(connect); + afterEach(disconnect); + + it("registers the tool in the stdio server tool list", async () => { + const { tools } = await client.listTools(); + expect(tools.map((t) => t.name)).toContain("loopover_explain_review_risk"); + }); + + it("proxies preflight input to POST /v1/preflight/review-risk", async () => { + const args = { repoFullName: "JSONbored/loopover", title: "Fix cache", contributorLogin: "JSONbored" }; + const result = await client.callTool({ name: "loopover_explain_review_risk", arguments: args }); + expect(capturedBodies).toEqual([args]); + expect(result.isError).toBeFalsy(); + const toolData = (result as { structuredContent?: unknown }).structuredContent; + expect(toolData).toEqual(reviewRiskFixture(args)); + }); +}); + +describe("loopover-mcp explain-review-risk CLI (#6980)", () => { + beforeEach(connect); + afterEach(disconnect); + + it("--json emits exactly the payload the MCP tool surfaces for the same input (mirror parity)", async () => { + const args = { repoFullName: "JSONbored/loopover", title: "Fix cache" }; + const viaTool = await client.callTool({ name: "loopover_explain_review_risk", arguments: args }); + const toolData = (viaTool as { structuredContent?: unknown }).structuredContent; + const viaCli = JSON.parse( + await runAsync(["explain-review-risk", "--repo", "JSONbored/loopover", "--title", "Fix cache", "--json"], { + LOOPOVER_API_URL: apiUrl, + LOOPOVER_TOKEN: "session-token", + }), + ); + expect(viaCli).toEqual(reviewRiskFixture(args)); + if (toolData !== undefined) expect(viaCli).toEqual(toolData); + }); + + it("prints the summary and recommendation on the plain-text path", async () => { + const out = await runAsync(["explain-review-risk", "--repoFullName", "JSONbored/loopover", "--title", "Fix cache"], { + LOOPOVER_API_URL: apiUrl, + LOOPOVER_TOKEN: "session-token", + }); + expect(out).toContain("LoopOver review-risk explanation for JSONbored/loopover."); + expect(out).toContain("Recommendation: review"); + expect(out).toContain("Preflight status: ready"); + }); + + it("forwards --login as contributorLogin", async () => { + await runAsync(["explain-review-risk", "--repo", "JSONbored/loopover", "--title", "Fix cache", "--login", "JSONbored", "--json"], { + LOOPOVER_API_URL: apiUrl, + LOOPOVER_TOKEN: "session-token", + }); + expect(capturedBodies.at(-1)).toMatchObject({ contributorLogin: "JSONbored", repoFullName: "JSONbored/loopover", title: "Fix cache" }); + }); + + it("fails when --repo or --title is missing", () => { + const noRepo = runExpectingFailure(["explain-review-risk", "--title", "Fix cache"], { + LOOPOVER_API_URL: apiUrl, + LOOPOVER_TOKEN: "session-token", + }); + expect(noRepo.status).toBe(1); + expect(`${noRepo.stdout}${noRepo.stderr}`).toMatch(/Pass --repo owner\/repo or --repoFullName/); + + const noTitle = runExpectingFailure(["explain-review-risk", "--repo", "JSONbored/loopover"], { + LOOPOVER_API_URL: apiUrl, + LOOPOVER_TOKEN: "session-token", + }); + expect(noTitle.status).toBe(1); + expect(`${noTitle.stdout}${noTitle.stderr}`).toMatch(/Pass --title/); + }); + + it("documents itself in --help and in the shell-completion command list", () => { + expect(run(["--help"])).toContain("loopover-mcp explain-review-risk --repo owner/repo --title "); + expect(run(["explain-review-risk", "--help"])).toContain("Mirrors the loopover_explain_review_risk MCP tool"); + expect(run(["completion", "bash"])).toContain("explain-review-risk"); + }); +}); diff --git a/test/unit/mcp-tool-rename-aliases.test.ts b/test/unit/mcp-tool-rename-aliases.test.ts index 8e8be0c06f..6bc40a746b 100644 --- a/test/unit/mcp-tool-rename-aliases.test.ts +++ b/test/unit/mcp-tool-rename-aliases.test.ts @@ -20,6 +20,7 @@ // (#6740 registered the loopover_explain_gate_disposition CLI mirror, taking the count from 75 to 76.) // (#6741 registered the loopover_draft_pr_body CLI mirror, taking the count from 76 to 77.) // (#6747 registered the loopover_pr_outcome CLI mirror, taking the count from 77 to 78.) +// (#6980 registered the loopover_explain_review_risk CLI mirror, taking the count from 78 to 79.) import { Client } from "@modelcontextprotocol/sdk/client/index.js"; import { StdioClientTransport } from "@modelcontextprotocol/sdk/client/stdio.js"; import { mkdtempSync, rmSync } from "node:fs"; @@ -67,14 +68,14 @@ describe("MCP legacy alias retirement (#4777) — discovery invariants", () => { }); afterEach(disconnect); - it("lists exactly 78 loopover_ tools and zero gittensory_-prefixed aliases", async () => { + it("lists exactly 79 loopover_ tools and zero gittensory_-prefixed aliases", async () => { const { tools } = await client.listTools(); const names = tools.map((t) => t.name); const primary = names.filter((n) => n.startsWith("loopover_")); const legacy = names.filter((n) => n.startsWith("gittensory_")); - expect(primary.length).toBe(78); + expect(primary.length).toBe(79); expect(legacy.length).toBe(0); - expect(names.length).toBe(78); + expect(names.length).toBe(79); }); it("no loopover_ tool's description carries a stale deprecation notice", async () => { @@ -86,14 +87,14 @@ describe("MCP legacy alias retirement (#4777) — discovery invariants", () => { } }); - it("`loopover-mcp tools --json` reports the same 78-tool count the live server registers", async () => { + it("`loopover-mcp tools --json` reports the same 79-tool count the live server registers", async () => { const { tools } = await client.listTools(); const payload = JSON.parse(run(["tools", "--json"])) as { count: number; tools: Array<{ name: string }>; }; expect(payload.count).toBe(tools.length); - expect(payload.count).toBe(78); + expect(payload.count).toBe(79); expect([...payload.tools.map((t) => t.name)].sort()).toEqual( [...tools.map((t) => t.name)].sort(), ); diff --git a/test/unit/openapi.test.ts b/test/unit/openapi.test.ts index 2892ad63c0..87d7fa726c 100644 --- a/test/unit/openapi.test.ts +++ b/test/unit/openapi.test.ts @@ -27,6 +27,7 @@ describe("OpenAPI contract", () => { expect(spec.paths["/v1/contributors/{login}/pr-outcomes"]).toBeDefined(); expect(spec.paths["/v1/contributors/{login}/repos/{owner}/{repo}/decision"]).toBeDefined(); expect(spec.paths["/v1/preflight/pr"]).toBeDefined(); + expect(spec.paths["/v1/preflight/review-risk"]).toBeDefined(); expect(spec.paths["/v1/preflight/local-diff"]).toBeDefined(); expect(spec.paths["/v1/local/branch-analysis"]).toBeDefined(); expect(spec.paths["/v1/agent/runs"]).toBeDefined(); diff --git a/test/unit/routes-review-risk.test.ts b/test/unit/routes-review-risk.test.ts new file mode 100644 index 0000000000..321b8d8bbd --- /dev/null +++ b/test/unit/routes-review-risk.test.ts @@ -0,0 +1,127 @@ +import { describe, expect, it } from "vitest"; +import { Client } from "@modelcontextprotocol/sdk/client/index.js"; +import { InMemoryTransport } from "@modelcontextprotocol/sdk/inMemory.js"; +import { createApp } from "../../src/api/routes"; +import { LoopoverMcp } from "../../src/mcp/server"; +import { createSessionForGitHubUser } from "../../src/auth/security"; +import { buildReviewRiskExplanation } from "../../src/signals/review-risk"; +import { createTestEnv } from "../helpers/d1"; + +const apiHeaders = (env: Env) => ({ + authorization: `Bearer ${env.LOOPOVER_API_TOKEN}`, + "content-type": "application/json", +}); + +describe("POST /v1/preflight/review-risk (#6980)", () => { + it("rejects an invalid body with 400", async () => { + const app = createApp(); + const env = createTestEnv(); + const response = await app.request("/v1/preflight/review-risk", { method: "POST", headers: apiHeaders(env), body: "{}" }, env); + expect(response.status).toBe(400); + await expect(response.json()).resolves.toMatchObject({ error: "invalid_preflight_request" }); + }); + + it("returns a review-risk explanation without contributorLogin", async () => { + const app = createApp(); + const env = createTestEnv(); + const input = { repoFullName: "missing/repo", title: "Docs note" }; + const expected = buildReviewRiskExplanation({ input, repo: null, issues: [], pullRequests: [], bounties: [] }); + const response = await app.request( + "/v1/preflight/review-risk", + { method: "POST", headers: apiHeaders(env), body: JSON.stringify(input) }, + env, + ); + expect(response.status).toBe(200); + const body = (await response.json()) as { + preflight: { repoFullName: string; status: string }; + roleContext: null; + recommendation: string; + summary: string; + }; + expect(body).toMatchObject({ + roleContext: null, + summary: expected.summary, + recommendation: expected.recommendation, + preflight: { repoFullName: "missing/repo", status: expected.preflight.status }, + }); + }); + + it("honors contributorLogin when the session matches", async () => { + const app = createApp(); + const env = createTestEnv({ ADMIN_GITHUB_LOGINS: "miner" }); + const { token } = await createSessionForGitHubUser(env, { login: "miner", id: 1 }); + const input = { + repoFullName: "missing/repo", + title: "Docs note", + contributorLogin: "miner", + }; + const response = await app.request( + "/v1/preflight/review-risk", + { + method: "POST", + headers: { authorization: `Bearer ${token}`, "content-type": "application/json" }, + body: JSON.stringify(input), + }, + env, + ); + expect(response.status).toBe(200); + const body = (await response.json()) as { roleContext: { login: string } | null; recommendation: string }; + expect(body.roleContext).toMatchObject({ login: "miner" }); + expect(body.recommendation).toEqual(expect.any(String)); + }); + + it("forbids a session from using another login as contributorLogin", async () => { + const app = createApp(); + const env = createTestEnv({ ADMIN_GITHUB_LOGINS: "miner" }); + const { token } = await createSessionForGitHubUser(env, { login: "miner", id: 1 }); + const response = await app.request( + "/v1/preflight/review-risk", + { + method: "POST", + headers: { authorization: `Bearer ${token}`, "content-type": "application/json" }, + body: JSON.stringify({ repoFullName: "missing/repo", title: "Docs note", contributorLogin: "other" }), + }, + env, + ); + expect(response.status).toBe(403); + await expect(response.json()).resolves.toMatchObject({ error: "forbidden_contributor" }); + }); + + it("matches the host MCP tool payload for the same input (mirror parity)", async () => { + const env = createTestEnv(); + const input = { repoFullName: "missing/repo", title: "Unknown repo preflight", body: "Fixes #999", changedFiles: ["docs/setup.md"] }; + const expected = buildReviewRiskExplanation({ input, repo: null, issues: [], pullRequests: [], bounties: [] }); + + const app = createApp(); + const viaRest = (await ( + await app.request("/v1/preflight/review-risk", { method: "POST", headers: apiHeaders(env), body: JSON.stringify(input) }, env) + ).json()) as { + preflight: { status: string; repoFullName: string }; + roleContext: unknown; + recommendation: string; + summary: string; + }; + + const server = new LoopoverMcp(env).createServer(); + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); + await server.connect(serverTransport); + const client = new Client({ name: "review-risk-parity", version: "0.1.0" }, { capabilities: {} }); + await client.connect(clientTransport); + const viaMcp = await client.callTool({ name: "loopover_explain_review_risk", arguments: input }); + const mcpData = (viaMcp as { structuredContent?: { preflight: { status: string }; roleContext: unknown; recommendation: string } }).structuredContent; + + expect(viaRest).toMatchObject({ + summary: expected.summary, + recommendation: expected.recommendation, + roleContext: expected.roleContext, + preflight: { repoFullName: expected.preflight.repoFullName, status: expected.preflight.status }, + }); + expect(mcpData).toMatchObject({ + recommendation: expected.recommendation, + roleContext: expected.roleContext, + preflight: { repoFullName: expected.preflight.repoFullName, status: expected.preflight.status }, + }); + expect(viaRest.recommendation).toBe(mcpData?.recommendation); + expect(viaRest.preflight.status).toBe(mcpData?.preflight.status); + }); +}); diff --git a/test/unit/support/mcp-cli-harness.ts b/test/unit/support/mcp-cli-harness.ts index 45c8f47b41..30f76d99c8 100644 --- a/test/unit/support/mcp-cli-harness.ts +++ b/test/unit/support/mcp-cli-harness.ts @@ -177,6 +177,9 @@ export async function startFixtureServer( validateConfigWarnings?: string[]; openPrMonitor?: Record; prOutcomes?: Record; + /** #6980: overrides POST /v1/preflight/review-risk and captures the request body. */ + reviewRisk?: Record; + onReviewRiskRequest?: (body: unknown) => void; /** #6745: overrides the notification feed / mark-read responses, and captures the mark-read POST body. */ notifications?: Record; notificationsRead?: Record; @@ -414,6 +417,16 @@ export async function startFixtureServer( response.end(JSON.stringify(withTerminalInjection(slopRiskFixture(body), options.terminalInjection))); return; } + if (request.url === "/v1/preflight/review-risk" && request.method === "POST") { + const body = (await readJsonRequest(request)) as { + repoFullName?: string; + title?: string; + contributorLogin?: string; + }; + options.onReviewRiskRequest?.(body); + response.end(JSON.stringify({ ...reviewRiskFixture(body), ...(options.reviewRisk ?? {}) })); + return; + } if (request.url === "/v1/lint/improvement-potential" && request.method === "POST") { const body = (await readJsonRequest(request)) as { changedFiles?: Array<{ path: string; additions?: number; deletions?: number }>; @@ -914,6 +927,38 @@ export function prOutcomesFixture(login = "JSONbored") { }; } +/** #6980: mirrors POST /v1/preflight/review-risk / buildReviewRiskExplanation. */ +export function reviewRiskFixture(input: { repoFullName?: string; title?: string; contributorLogin?: string } = {}) { + const repoFullName = input.repoFullName ?? "JSONbored/loopover"; + return { + preflight: { + repoFullName, + generatedAt: "2026-06-01T00:00:00.000Z", + status: "ready" as const, + lane: { lane: "direct_pr", reasons: ["Fixture lane."] }, + reviewBurden: "low" as const, + linkedIssues: [] as number[], + findings: [] as unknown[], + collisions: [] as unknown[], + }, + roleContext: input.contributorLogin + ? { + login: input.contributorLogin.toLowerCase(), + repoFullName, + generatedAt: "2026-06-01T00:00:00.000Z", + role: "outside_contributor" as const, + maintainerLane: false, + normalContributorEvidenceAllowed: true, + source: "unknown" as const, + reasons: ["Fixture role context."], + guidance: "Outside-contributor work is scoreable when the lane fits.", + } + : null, + recommendation: "review" as const, + summary: `LoopOver review-risk explanation for ${repoFullName}.`, + }; +} + /** #6745: mirrors the NotificationFeed { login, unreadCount, notifications } shape the route/tool returns. */ export function notificationsFixture() { return {