diff --git a/packages/loopover-mcp/bin/loopover-mcp.js b/packages/loopover-mcp/bin/loopover-mcp.js index 9aca87cabe..7d396cc12f 100755 --- a/packages/loopover-mcp/bin/loopover-mcp.js +++ b/packages/loopover-mcp/bin/loopover-mcp.js @@ -636,6 +636,36 @@ const simulateOpenPrPressureShape = { contributorOpenPrCount: simulateOpenPrPressureCount.optional(), }; +// #6748: mirrors checkImprovementPotentialShape in src/mcp/server.ts VERBATIM. The bin cannot import from +// src/ (package boundary), so this is the one copy parity rests on by convention — its own tests pin that the +// bounds the real schema enforces are enforced here too, rather than waved through to a route 400. +const checkImprovementPotentialShape = { + changedFiles: z + .array(z.object({ path: z.string().min(1).max(400), additions: z.number().int().min(0).optional(), deletions: z.number().int().min(0).optional() })) + .max(2000) + .optional(), + tests: z.array(z.string().max(400)).max(2000).optional(), + testFiles: z.array(z.string().max(400)).max(2000).optional(), + patchCoverageDeltaPercent: z.number().optional(), + complexityDeltas: z + .array( + z.object({ + file: z.string().min(1).max(400), + line: z.number().int().min(1), + name: z.string().min(1).max(400), + before: z.number().int().min(0), + after: z.number().int().min(0), + delta: z.number().int(), + }), + ) + .max(2000) + .optional(), + duplicationDeltas: z + .array(z.object({ file: z.string().min(1).max(400), line: z.number().int().min(1), duplicateOfLine: z.number().int().min(1), lines: z.number().int().min(1) })) + .max(2000) + .optional(), +}; + const checkSlopRiskShape = { changedFiles: z .array(z.object({ path: z.string().min(1).max(400), additions: z.number().int().min(0).optional(), deletions: z.number().int().min(0).optional() })) @@ -945,6 +975,12 @@ const STDIO_TOOL_DESCRIPTORS = [ category: "review", description: "Assess the deterministic slop risk of a planned change from local diff metadata (paths + line counts) + the PR description — an agent-native, source-free quality self-check. Returns slopRisk (0-100), band, findings, and the rubric. Computed in-process; no repo data and no API round-trip.", }, + { + name: "loopover_check_improvement_potential", + category: "review", + description: + "Assess a planned change's deterministic structural-improvement potential from local diff metadata alone (no source uploaded) — reduced complexity, resolved duplication, patch-coverage delta, added test evidence. Returns improvementScore (0-100), band, and findings. The positive-axis counterpart to check_slop_risk; advisory-only, never blocks.", + }, { name: "loopover_simulate_open_pr_pressure", category: "discovery", @@ -1541,6 +1577,19 @@ registerStdioTool( (input) => toolResult("LoopOver slop-risk self-check.", { ...buildSlopAssessment(input), rubric: SLOP_RUBRIC_MARKDOWN }), ); +// #6748: CLI mirror of the remote server's loopover_check_improvement_potential. Proxies rather than computing +// in-process (like the boundary-tests / open-PR-pressure mirrors): buildStructuralImprovementAssessment lives +// app-side in src/signals/improvement.ts, not in @loopover/engine, so POST /v1/lint/improvement-potential stays +// the single source of truth for the scoring. +registerStdioTool( + "loopover_check_improvement_potential", + { + description: stdioToolDescription("loopover_check_improvement_potential"), + inputSchema: checkImprovementPotentialShape, + }, + async (input) => toolResult("LoopOver improvement-potential self-check.", await apiPost("/v1/lint/improvement-potential", input)), +); + // #6751: CLI mirror of the remote server's loopover_simulate_open_pr_pressure. Proxies rather than computing // in-process (like the boundary-tests mirror, #6750): simulateOpenPrPressure lives app-side in // src/services/open-pr-pressure-scenarios.ts, not in @loopover/engine, so POST /v1/lint/open-pr-pressure stays diff --git a/src/api/routes.ts b/src/api/routes.ts index 22f558ff56..baf0fc7e4d 100644 --- a/src/api/routes.ts +++ b/src/api/routes.ts @@ -155,6 +155,8 @@ import { } from "../orb/relay"; import { computeFleetAnalytics } from "../orb/analytics"; import { handleMcpRequest } from "../mcp/server"; +import { checkImprovementPotentialShape } from "../mcp/server"; +import { buildStructuralImprovementAssessment } from "../signals/improvement"; import { simulateOpenPrPressureShape } from "../mcp/server"; import { simulateOpenPrPressure, type OpenPrPressureInput } from "../services/open-pr-pressure-scenarios"; import { buildOpenApiSpec } from "../openapi/spec"; @@ -3292,6 +3294,19 @@ export function createApp() { return c.json({ ...buildSlopAssessment(parsed.data), rubric: SLOP_RUBRIC_MARKDOWN }); }); + // #6748: REST mirror of the loopover_check_improvement_potential MCP tool — deterministic, rate-limit-only, + // pure local-metadata, the same tier as the /v1/lint/* routes it sits with. Parses with the tool's OWN + // exported checkImprovementPotentialShape so the two surfaces cannot diverge on accepted input, then returns + // the SAME field subset the tool's handler returns (improvementScore/band/findings) rather than the whole + // assessment, so the mirror is byte-identical to the tool rather than merely similar. + app.post("/v1/lint/improvement-potential", async (c) => { + const body = await c.req.json().catch(() => null); + const parsed = z.object(checkImprovementPotentialShape).safeParse(body); + if (!parsed.success) return c.json({ error: "invalid_improvement_potential_request", issues: parsed.error.issues }, 400); + const assessment = buildStructuralImprovementAssessment(parsed.data); + return c.json({ improvementScore: assessment.improvementScore, band: assessment.band, findings: assessment.findings }); + }); + // #6751: REST mirror of the loopover_simulate_open_pr_pressure MCP tool — deterministic, public-safe, and // read-only (no repo access, no GitHub writes), the same tier as the lint routes it sits with. Parses with the // tool's OWN exported simulateOpenPrPressureShape so the two surfaces cannot diverge on accepted input, then diff --git a/src/mcp/server.ts b/src/mcp/server.ts index 5310504255..4e478a078b 100644 --- a/src/mcp/server.ts +++ b/src/mcp/server.ts @@ -1139,7 +1139,9 @@ const evaluateEscalationOutputSchema = { // auth required — same choice as checkSlopRisk: a pure function over caller-supplied structured data with no // owner/repo/login to scope, and improvementScore carries no gate/blocker power (advisory-only; see // improvement.ts's header comment), so there is nothing to gate. -const checkImprovementPotentialShape = { +// #6748: exported so POST /v1/lint/improvement-potential parses with this EXACT shape rather than a second, +// drifting copy — the REST mirror and this tool can never diverge on what they accept. +export const checkImprovementPotentialShape = { changedFiles: z .array(z.object({ path: z.string().min(1).max(400), additions: z.number().int().min(0).optional(), deletions: z.number().int().min(0).optional() })) .max(2000) diff --git a/test/unit/mcp-cli-improvement-potential-tool.test.ts b/test/unit/mcp-cli-improvement-potential-tool.test.ts new file mode 100644 index 0000000000..19b12f874a --- /dev/null +++ b/test/unit/mcp-cli-improvement-potential-tool.test.ts @@ -0,0 +1,88 @@ +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"; + +// #6748: the CLI mirror of loopover_check_improvement_potential. It PROXIES to +// POST /v1/lint/improvement-potential (buildStructuralImprovementAssessment lives app-side in +// src/signals/improvement.ts, not @loopover/engine), so the route is the single source of truth for scoring. +// The bin cannot import from src/, so its zod shape is a hand-mirror of the tool's — these tests pin that +// mirror: valid payloads reach the route, and every bound the real schema enforces is enforced here too. +const bin = join(process.cwd(), "packages/loopover-mcp/bin/loopover-mcp.js"); +let client: Client; +let transport: StdioClientTransport; +let configDir: string; +let captured: Array<{ url: string; method: string }>; + +beforeEach(async () => { + configDir = mkdtempSync(join(tmpdir(), "loopover-improvement-")); + captured = []; + const apiUrl = await startFixtureServer({ + onApiRequest: (request) => { + if (request.url?.includes("/lint/improvement-potential")) captured.push({ url: request.url ?? "", method: request.method ?? "" }); + }, + }); + 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: "improvement-tool-test", version: "0.0.1" }); + await client.connect(transport); +}); + +afterEach(async () => { + await client?.close().catch(() => undefined); + await closeFixtureServer(); + if (configDir) rmSync(configDir, { recursive: true, force: true }); +}); + +describe("loopover_check_improvement_potential stdio mirror (#6748)", () => { + it("registers alongside its check_slop_risk counterpart", async () => { + const names = new Set((await client.listTools()).tools.map((t) => t.name)); + expect(names).toContain("loopover_check_improvement_potential"); + expect(names).toContain("loopover_check_slop_risk"); + }); + + it("proxies a full payload to POST /v1/lint/improvement-potential and returns the score", async () => { + const result = await client.callTool({ + name: "loopover_check_improvement_potential", + arguments: { + changedFiles: [{ path: "src/a.ts", additions: 20, deletions: 60 }], + testFiles: ["test/a.test.ts"], + patchCoverageDeltaPercent: 4.5, + complexityDeltas: [{ file: "src/a.ts", line: 10, name: "handler", before: 14, after: 6, delta: -8 }], + duplicationDeltas: [{ file: "src/a.ts", line: 30, duplicateOfLine: 12, lines: 9 }], + }, + }); + expect(result.isError).toBeFalsy(); + expect(captured).toHaveLength(1); + expect(captured[0]!.method).toBe("POST"); + const text = JSON.stringify(result); + expect(text).toContain("moderate"); + expect(text).not.toMatch(/wallet|hotkey|coldkey|trust score|reward estimate/i); + }); + + it("accepts an empty payload — every field is optional on the tool's shape", async () => { + const result = await client.callTool({ name: "loopover_check_improvement_potential", arguments: {} }); + expect(result.isError).toBeFalsy(); + expect(captured).toHaveLength(1); + }); + + it("enforces the real schema's bounds itself, before any API call", async () => { + for (const args of [ + { changedFiles: [{ path: "" }] }, + { changedFiles: [{ path: "src/a.ts", additions: -1 }] }, + { complexityDeltas: [{ file: "src/a.ts", line: 0, name: "f", before: 1, after: 1, delta: 0 }] }, + { duplicationDeltas: [{ file: "src/a.ts", line: 1, duplicateOfLine: 1, lines: 0 }] }, + { patchCoverageDeltaPercent: "lots" }, + ]) { + const rejected = await client.callTool({ name: "loopover_check_improvement_potential", arguments: args }).then((r) => Boolean(r.isError), () => true); + expect(rejected, JSON.stringify(args)).toBe(true); + } + expect(captured).toHaveLength(0); + }); +}); diff --git a/test/unit/mcp-tool-rename-aliases.test.ts b/test/unit/mcp-tool-rename-aliases.test.ts index 3f7a6fa988..23d0205ab6 100644 --- a/test/unit/mcp-tool-rename-aliases.test.ts +++ b/test/unit/mcp-tool-rename-aliases.test.ts @@ -56,14 +56,14 @@ describe("MCP legacy alias retirement (#4777) — discovery invariants", () => { }); afterEach(disconnect); - it("lists exactly 70 loopover_ tools and zero gittensory_-prefixed aliases", async () => { + it("lists exactly 71 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(70); + expect(primary.length).toBe(71); expect(legacy.length).toBe(0); - expect(names.length).toBe(70); + expect(names.length).toBe(71); }); it("no loopover_ tool's description carries a stale deprecation notice", async () => { @@ -73,11 +73,11 @@ describe("MCP legacy alias retirement (#4777) — discovery invariants", () => { } }); - it("`loopover-mcp tools --json` reports the same 70-tool count the live server registers", async () => { + it("`loopover-mcp tools --json` reports the same 71-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(70); + expect(payload.count).toBe(71); expect([...payload.tools.map((t) => t.name)].sort()).toEqual([...tools.map((t) => t.name)].sort()); }); }); diff --git a/test/unit/routes-improvement-potential.test.ts b/test/unit/routes-improvement-potential.test.ts new file mode 100644 index 0000000000..b7263f9dbf --- /dev/null +++ b/test/unit/routes-improvement-potential.test.ts @@ -0,0 +1,76 @@ +import { describe, expect, it } from "vitest"; +import { createApp } from "../../src/api/routes"; +import { buildStructuralImprovementAssessment } from "../../src/signals/improvement"; +import { createTestEnv } from "../helpers/d1"; + +// #6748: POST /v1/lint/improvement-potential — the REST mirror of loopover_check_improvement_potential, the one +// deterministic lint tool whose siblings (slop-risk, issue-slop, lint-pr-text, validate-config) all already had +// REST parity. The route parses with the tool's OWN exported shape and returns the tool handler's exact field +// subset, so these pin the ROUTE contract; the scorer's own logic is covered by improvement's tests. +const apiHeaders = (env: Env) => ({ authorization: `Bearer ${env.LOOPOVER_API_TOKEN}`, "content-type": "application/json" }); +const PATH = "/v1/lint/improvement-potential"; +const post = (env: Env, body: unknown) => createApp().request(PATH, { method: "POST", headers: apiHeaders(env), body: JSON.stringify(body) }, env); + +/** The tool's handler returns only these three fields — the mirror must not leak the rest of the assessment. */ +const subset = (input: Parameters[0]) => { + const a = buildStructuralImprovementAssessment(input); + return JSON.parse(JSON.stringify({ improvementScore: a.improvementScore, band: a.band, findings: a.findings })); +}; + +describe("POST /v1/lint/improvement-potential (#6748)", () => { + it("returns the tool handler's exact field subset for a structurally-improving change", async () => { + const env = createTestEnv(); + const body = { + changedFiles: [{ path: "src/a.ts", additions: 20, deletions: 60 }], + testFiles: ["test/a.test.ts"], + patchCoverageDeltaPercent: 4.5, + complexityDeltas: [{ file: "src/a.ts", line: 10, name: "handler", before: 14, after: 6, delta: -8 }], + duplicationDeltas: [{ file: "src/a.ts", line: 30, duplicateOfLine: 12, lines: 9 }], + }; + const response = await post(env, body); + expect(response.status).toBe(200); + const payload = await response.json(); + expect(payload).toEqual(subset(body)); + // The mirror exposes exactly the tool's three fields — no extra assessment internals. + expect(Object.keys(payload as object).sort()).toEqual(["band", "findings", "improvementScore"]); + }); + + it("matches the scorer across empty, test-only, and coverage-regression inputs", async () => { + const env = createTestEnv(); + for (const body of [ + {}, + { changedFiles: [] }, + { changedFiles: [{ path: "src/a.ts" }] }, + { changedFiles: [{ path: "src/a.ts" }], tests: ["ran the suite"] }, + { changedFiles: [{ path: "src/a.ts" }], patchCoverageDeltaPercent: -12 }, + { complexityDeltas: [{ file: "src/a.ts", line: 1, name: "f", before: 2, after: 9, delta: 7 }] }, + ]) { + const response = await post(env, body); + expect(response.status, JSON.stringify(body)).toBe(200); + await expect(response.json()).resolves.toEqual(subset(body)); + } + }); + + it("rejects input the tool's shape rejects, with 400", async () => { + const env = createTestEnv(); + for (const body of [ + { changedFiles: [{ path: "" }] }, + { changedFiles: [{ path: "src/a.ts", additions: -1 }] }, + { complexityDeltas: [{ file: "src/a.ts", line: 0, name: "f", before: 1, after: 1, delta: 0 }] }, + { duplicationDeltas: [{ file: "src/a.ts", line: 1, duplicateOfLine: 1, lines: 0 }] }, + { patchCoverageDeltaPercent: "lots" }, + ]) { + const response = await post(env, body); + expect(response.status, JSON.stringify(body)).toBe(400); + await expect(response.json()).resolves.toMatchObject({ error: "invalid_improvement_potential_request" }); + } + const malformed = await createApp().request(PATH, { method: "POST", headers: apiHeaders(createTestEnv()), body: "{not json" }, createTestEnv()); + expect(malformed.status).toBe(400); + }); + + it("is public-safe: no wallet/hotkey/trust-score terms", async () => { + const env = createTestEnv(); + const text = JSON.stringify(await (await post(env, { changedFiles: [{ path: "src/a.ts" }] })).json()); + expect(text).not.toMatch(/wallet|hotkey|coldkey|trust score|reward estimate/i); + }); +}); diff --git a/test/unit/support/mcp-cli-harness.ts b/test/unit/support/mcp-cli-harness.ts index 4a7e04b166..2a1b1cc085 100644 --- a/test/unit/support/mcp-cli-harness.ts +++ b/test/unit/support/mcp-cli-harness.ts @@ -584,6 +584,11 @@ export async function startFixtureServer( response.end(JSON.stringify({ repoFullName: "acme/widgets", summary: "Ranked 2 scenarios.", scenarios: [{ id: "close_stale", rank: 1 }] })); return; } + // #6748: the improvement-potential mirror proxies here. + if (request.url === "/v1/lint/improvement-potential" && request.method === "POST") { + response.end(JSON.stringify({ improvementScore: 62, band: "moderate", findings: [{ code: "complexity_reduced", detail: "2 functions simplified." }] })); + return; + } if (request.url === "/v1/repos/owner/repo/pulls/7/reviewability" && request.method === "GET") { response.end( JSON.stringify({