From c436f5405b893baae36e604b8debaf6bfa031854 Mon Sep 17 00:00:00 2001 From: Lourince Daging Date: Wed, 15 Jul 2026 23:07:15 +0200 Subject: [PATCH] feat(mcp): add loopover_get_pr_reviewability tool on both MCP servers The GET /v1/repos/:owner/:repo/pulls/:number/reviewability route is already MCP-auth-aware (isMcpReadRepoAllowed, same gate as /intelligence and /issue-quality) but had no tool on either server. Adds loopover_get_pr_reviewability to the remote server, mirroring loopover_get_issue_quality's cached-snapshot-then-compute pattern and reusing the existing contributor-fast-context loader, plus the equivalent apiGet proxy tool on the local stdio server. Closes #6154. --- packages/loopover-mcp/bin/loopover-mcp.js | 22 ++++ src/mcp/server.ts | 88 ++++++++++++++ test/unit/mcp-cli-pr-reviewability.test.ts | 70 +++++++++++ test/unit/mcp-pr-reviewability.test.ts | 133 +++++++++++++++++++++ test/unit/support/mcp-cli-harness.ts | 14 +++ 5 files changed, 327 insertions(+) create mode 100644 test/unit/mcp-cli-pr-reviewability.test.ts create mode 100644 test/unit/mcp-pr-reviewability.test.ts diff --git a/packages/loopover-mcp/bin/loopover-mcp.js b/packages/loopover-mcp/bin/loopover-mcp.js index 9bfb159faf..677eb522bb 100755 --- a/packages/loopover-mcp/bin/loopover-mcp.js +++ b/packages/loopover-mcp/bin/loopover-mcp.js @@ -166,6 +166,12 @@ const ownerRepoShape = { repo: z.string().min(1), }; +const ownerRepoPullShape = { + owner: z.string().min(1), + repo: z.string().min(1), + number: z.number().int().positive(), +}; + const loginShape = { login: z.string().min(1), }; @@ -433,6 +439,10 @@ const STDIO_TOOL_DESCRIPTORS = [ name: "loopover_get_repo_context", description: "Return the canonical repo intelligence bundle from the private LoopOver API.", }, + { + name: "loopover_get_pr_reviewability", + description: "Return the reviewability report for an open PR: how ready it is to review/merge, the blocking or advisory signals against it, and its lane/duplicate/linked-issue context. Metadata-only, no GitHub writes.", + }, { name: "loopover_get_maintainer_noise", description: "Return the maintainer queue-noise triage report for a repo: a noise score/level, the specific noise sources to clear first, and recommended maintainer actions. Maintainer-authenticated; advisory only.", @@ -619,6 +629,18 @@ registerStdioTool( }, ); +registerStdioTool( + "loopover_get_pr_reviewability", + { + description: stdioToolDescription("loopover_get_pr_reviewability"), + inputSchema: ownerRepoPullShape, + }, + async ({ owner, repo, number }) => { + const prefix = `/v1/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}`; + return toolResult("LoopOver PR reviewability.", await apiGet(`${prefix}/pulls/${number}/reviewability`)); + }, +); + registerStdioTool( "loopover_get_maintainer_noise", { diff --git a/src/mcp/server.ts b/src/mcp/server.ts index 086a0c2873..3d2ce03a8e 100644 --- a/src/mcp/server.ts +++ b/src/mcp/server.ts @@ -63,7 +63,10 @@ import { upsertRepositorySettings, listOpenPullRequests, listPullRequests, + listPullRequestFiles, + listPullRequestReviews, listRecentMergedPullRequests, + listSignalSnapshots, listRepoSyncSegments, listRepoSyncStates, listRepositories, @@ -139,6 +142,7 @@ import { PUBLIC_SURFACE_SKIP_REASONS, skippedPrAuditRemediation, type PublicSurf import { buildContributorOpenPrMonitor } from "../signals/contributor-open-pr-monitor"; import { buildLocalBranchAnalysis, findCurrentBranchPullRequest } from "../signals/local-branch"; import { computeLocalScorerTokens } from "../signals/local-scorer"; +import { buildPullRequestReviewability, type PullRequestReviewability } from "../signals/reward-risk"; import { buildApplyLabelsSpec, buildCreateBranchSpec, @@ -195,6 +199,12 @@ const ownerRepoShape = { repo: z.string().min(1), }; +const ownerRepoPullShape = { + owner: z.string().min(1), + repo: z.string().min(1), + number: z.number().int().positive(), +}; + const ownerRepoWindowShape = { owner: z.string().min(1), repo: z.string().min(1), @@ -2118,6 +2128,17 @@ export class LoopoverMcp { async (input) => this.toolResult(await this.getIssueQuality(input)), ); + server.registerTool( + "loopover_get_pr_reviewability", + { + description: + "Return the cached or freshly-computed reviewability report for an open PR: how ready it is to review/merge, the blocking or advisory signals against it, and its lane/duplicate/linked-issue context. Metadata-only, repo-scoped, no GitHub writes.", + inputSchema: ownerRepoPullShape, + outputSchema: freshnessResponseOutputSchema, + }, + async (input) => this.toolResult(await this.getPrReviewability(input)), + ); + server.registerTool( "loopover_validate_linked_issue", { @@ -2949,6 +2970,73 @@ export class LoopoverMcp { }; } + private async getPrReviewability(input: { owner: string; repo: string; number: number }): Promise { + const fullName = `${input.owner}/${input.repo}`; + if (!(await this.canAccessRepo(fullName))) { + return { + summary: `Forbidden: session cannot access PR reviewability for ${fullName}.`, + data: { status: "forbidden", repoFullName: fullName }, + }; + } + // Prefer the persisted snapshot the /reviewability route writes (signal type "pr-reviewability", keyed by + // `${fullName}#${number}`), mirroring how getIssueQuality serves the cached snapshot before recomputing. + const cached = (await listSignalSnapshots(this.env, "pr-reviewability", `${fullName}#${input.number}`))[0]; + if (cached) { + const payload = cached.payload as unknown as PullRequestReviewability; + return { + summary: `LoopOver PR reviewability for ${fullName}#${input.number} (cached).`, + data: { + status: "ready", + source: "snapshot", + repoFullName: fullName, + generatedAt: cached.generatedAt || payload.generatedAt || new Date().toISOString(), + report: payload, + } as unknown as Record, + }; + } + const [repo, pullRequest] = await Promise.all([getRepository(this.env, fullName), getPullRequest(this.env, fullName, input.number)]); + if (!repo || !pullRequest) { + return { + summary: `LoopOver has no cached PR reviewability for ${fullName}#${input.number}.`, + data: { status: "not_found", repoFullName: fullName }, + }; + } + const [issues, pullRequests, files, reviews, checks, recentMergedPullRequests] = await Promise.all([ + listIssues(this.env, fullName), + listPullRequests(this.env, fullName), + listPullRequestFiles(this.env, fullName, input.number), + listPullRequestReviews(this.env, fullName, input.number), + listCheckSummaries(this.env, fullName, input.number), + listRecentMergedPullRequests(this.env, fullName), + ]); + const contributor = pullRequest.authorLogin; + const contributorContext = contributor ? await this.loadContributorFastContext(contributor) : null; + const report = buildPullRequestReviewability({ + repo, + pullRequest, + issues, + pullRequests, + files, + reviews, + checks, + recentMergedPullRequests, + repoFullName: fullName, + pullNumber: input.number, + profile: contributorContext?.profile, + outcomeHistory: contributorContext?.outcomeHistory, + }); + return { + summary: `LoopOver PR reviewability for ${fullName}#${input.number} (computed from cached metadata).`, + data: { + status: "ready", + source: "computed", + repoFullName: fullName, + generatedAt: report.generatedAt, + report, + } as unknown as Record, + }; + } + private async validateLinkedIssue(input: { owner: string; repo: string; diff --git a/test/unit/mcp-cli-pr-reviewability.test.ts b/test/unit/mcp-cli-pr-reviewability.test.ts new file mode 100644 index 0000000000..6027ccd23c --- /dev/null +++ b/test/unit/mcp-cli-pr-reviewability.test.ts @@ -0,0 +1,70 @@ +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, startFixtureServer } from "./support/mcp-cli-harness"; + +const bin = join(process.cwd(), "packages/loopover-mcp/bin/loopover-mcp.js"); +const FORBIDDEN_PUBLIC_TERMS = /wallet\s*[:=]\s*\S+|hotkey\s*[:=]\s*\S+|coldkey\s*[:=]\s*\S+|raw trust score is|your trust score|reward estimate is|estimated reward/i; + +let client: Client; +let transport: StdioClientTransport; +let configDir: string; +let apiUrl: string; +let capturedRequests: Array<{ url: string; method: string }>; + +async function connect() { + configDir = mkdtempSync(join(tmpdir(), "gittensory-pr-reviewability-")); + capturedRequests = []; + apiUrl = await startFixtureServer({ + onApiRequest: (request) => { + if (request.url && request.url.includes("/reviewability")) { + capturedRequests.push({ url: request.url ?? "", method: request.method ?? "GET" }); + } + }, + }); + 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: "pr-reviewability-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_get_pr_reviewability stdio proxy (#6154)", () => { + 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_get_pr_reviewability"); + }); + + it("proxies owner/repo/number to /v1/repos/:owner/:repo/pulls/:number/reviewability via apiGet", async () => { + const result = await client.callTool({ name: "loopover_get_pr_reviewability", arguments: { owner: "owner", repo: "repo", number: 7 } }); + expect(capturedRequests.length).toBe(1); + const captured = capturedRequests[0]!; + expect(captured.url).toContain("/v1/repos/owner/repo/pulls/7/reviewability"); + expect(captured.method).toBe("GET"); + expect(result.isError).toBeFalsy(); + const text = JSON.stringify(result); + expect(text).not.toMatch(FORBIDDEN_PUBLIC_TERMS); + expect(text).toContain("owner/repo"); + expect(text).toContain("readiness"); + }); +}); diff --git a/test/unit/mcp-pr-reviewability.test.ts b/test/unit/mcp-pr-reviewability.test.ts new file mode 100644 index 0000000000..7a2280ef0d --- /dev/null +++ b/test/unit/mcp-pr-reviewability.test.ts @@ -0,0 +1,133 @@ +import { Client } from "@modelcontextprotocol/sdk/client/index.js"; +import { InMemoryTransport } from "@modelcontextprotocol/sdk/inMemory.js"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { LoopoverMcp } from "../../src/mcp/server"; +import { persistSignalSnapshot, upsertPullRequestFromGitHub, upsertRepositoryFromGitHub } from "../../src/db/repositories"; +import type { AuthIdentity } from "../../src/auth/security"; +import { createTestEnv } from "../helpers/d1"; + +afterEach(() => { + vi.unstubAllGlobals(); +}); + +async function connect(env: Env, identity?: AuthIdentity) { + const server = (identity ? new LoopoverMcp(env, identity) : new LoopoverMcp(env)).createServer(); + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); + await server.connect(serverTransport); + const client = new Client({ name: "gittensory-reviewability-test", version: "0.1.0" }, { capabilities: {} }); + await client.connect(clientTransport); + return client; +} + +function prPayload(overrides: Record = {}) { + return { + number: 7, + title: "Add retry to the upload client", + state: "open", + user: { login: "contributor" }, + author_association: "CONTRIBUTOR", + head: { sha: "abc123", ref: "contributor/attempt-1" }, + base: { ref: "main" }, + html_url: "https://github.com/owner/repo/pull/7", + merged_at: null, + draft: false, + mergeable: true, + body: "Closes #1", + created_at: "2026-07-03T00:00:00Z", + updated_at: "2026-07-03T00:00:00Z", + closed_at: null, + labels: [{ name: "enhancement" }], + ...overrides, + }; +} + +async function seedRepo(env: Env) { + await upsertRepositoryFromGitHub(env, { name: "repo", full_name: "owner/repo", private: false, owner: { login: "owner" }, default_branch: "main" }); +} + +type ReviewabilityResponse = { status: string; source?: string; repoFullName?: string; generatedAt?: string; report?: { generatedAt?: string; pullNumber?: number } }; + +describe("MCP loopover_get_pr_reviewability (#6154)", () => { + it("forbids the static mcp identity when the repo is outside MCP_READ_REPO_ALLOWLIST", async () => { + const env = createTestEnv({ MCP_READ_REPO_ALLOWLIST: "" }); + const client = await connect(env); + const result = await client.callTool({ name: "loopover_get_pr_reviewability", arguments: { owner: "owner", repo: "repo", number: 7 } }); + expect(result.isError).toBeFalsy(); + expect((result.structuredContent as ReviewabilityResponse).status).toBe("forbidden"); + }); + + it("returns not_found when the repository or pull request is missing", async () => { + const env = createTestEnv(); + const client = await connect(env); + // Repo present, PR absent → still not_found (the route requires both). + await seedRepo(env); + const result = await client.callTool({ name: "loopover_get_pr_reviewability", arguments: { owner: "owner", repo: "repo", number: 404 } }); + expect((result.structuredContent as ReviewabilityResponse).status).toBe("not_found"); + + const noRepo = await connect(createTestEnv()); + const missingRepo = await noRepo.callTool({ name: "loopover_get_pr_reviewability", arguments: { owner: "owner", repo: "ghost", number: 7 } }); + expect((missingRepo.structuredContent as ReviewabilityResponse).status).toBe("not_found"); + }); + + it("serves the persisted pr-reviewability snapshot before recomputing", async () => { + const env = createTestEnv(); + await persistSignalSnapshot(env, { + id: "reviewability-cached", + signalType: "pr-reviewability", + targetKey: "owner/repo#7", + repoFullName: "owner/repo", + generatedAt: "2026-05-30T00:00:00.000Z", + payload: { repoFullName: "owner/repo", pullNumber: 7, generatedAt: "2026-05-30T00:00:00.000Z", summary: "cached" }, + }); + const client = await connect(env); + const result = await client.callTool({ name: "loopover_get_pr_reviewability", arguments: { owner: "owner", repo: "repo", number: 7 } }); + const data = result.structuredContent as ReviewabilityResponse; + expect(data.source).toBe("snapshot"); + expect(data.generatedAt).toBe("2026-05-30T00:00:00.000Z"); + expect(data.report?.pullNumber).toBe(7); + }); + + it("falls back to the payload timestamp for a snapshot row with an empty generated_at", async () => { + const env = createTestEnv(); + await env.DB.prepare( + `insert into signal_snapshots (id, signal_type, target_key, repo_full_name, payload_json, generated_at) + values ('reviewability-payload-generated', 'pr-reviewability', 'owner/repo#7', 'owner/repo', ?, '')`, + ) + .bind(JSON.stringify({ repoFullName: "owner/repo", pullNumber: 7, generatedAt: "2026-05-29T00:00:00.000Z", summary: "payload" })) + .run(); + const client = await connect(env); + const result = await client.callTool({ name: "loopover_get_pr_reviewability", arguments: { owner: "owner", repo: "repo", number: 7 } }); + const data = result.structuredContent as ReviewabilityResponse; + expect(data.source).toBe("snapshot"); + expect(data.generatedAt).toBe("2026-05-29T00:00:00.000Z"); + }); + + it("computes reviewability from cached metadata for an open PR with a contributor", async () => { + // Fail every outbound fetch fast so loadContributorFastContext degrades to its offline fallback + // deterministically instead of reaching for the live GitHub/Gittensor APIs. + vi.stubGlobal("fetch", vi.fn(async () => { throw new Error("offline"); })); + const env = createTestEnv(); + await seedRepo(env); + await upsertPullRequestFromGitHub(env, "owner/repo", prPayload()); + const client = await connect(env); + const result = await client.callTool({ name: "loopover_get_pr_reviewability", arguments: { owner: "owner", repo: "repo", number: 7 } }); + const data = result.structuredContent as ReviewabilityResponse; + expect(result.isError).toBeFalsy(); + expect(data.source).toBe("computed"); + expect(data.repoFullName).toBe("owner/repo"); + expect(data.report?.pullNumber).toBe(7); + expect(typeof data.generatedAt).toBe("string"); + }); + + it("computes reviewability when the pull request has no author (skips contributor context)", async () => { + const env = createTestEnv(); + await seedRepo(env); + // No `user` on the payload → authorLogin null → the contributor-context branch is skipped, so no fetch runs. + await upsertPullRequestFromGitHub(env, "owner/repo", prPayload({ number: 8, user: undefined, html_url: "https://github.com/owner/repo/pull/8" })); + const client = await connect(env); + const result = await client.callTool({ name: "loopover_get_pr_reviewability", arguments: { owner: "owner", repo: "repo", number: 8 } }); + const data = result.structuredContent as ReviewabilityResponse; + expect(data.source).toBe("computed"); + expect(data.report?.pullNumber).toBe(8); + }); +}); diff --git a/test/unit/support/mcp-cli-harness.ts b/test/unit/support/mcp-cli-harness.ts index cec8135c9c..cf21a3c4b8 100644 --- a/test/unit/support/mcp-cli-harness.ts +++ b/test/unit/support/mcp-cli-harness.ts @@ -440,6 +440,20 @@ export async function startFixtureServer( ); return; } + if (request.url === "/v1/repos/owner/repo/pulls/7/reviewability" && request.method === "GET") { + response.end( + JSON.stringify({ + repoFullName: "owner/repo", + pullNumber: 7, + generatedAt: "2026-05-30T00:00:00.000Z", + readiness: "ready", + blockers: [], + advisories: [], + summary: "PR 7 is ready to review.", + }), + ); + return; + } response.statusCode = 404; response.end(JSON.stringify({ error: "not_found" })); });