diff --git a/packages/loopover-mcp/bin/loopover-mcp.js b/packages/loopover-mcp/bin/loopover-mcp.js index 93c03b6bb9..ec0ce3cee1 100755 --- a/packages/loopover-mcp/bin/loopover-mcp.js +++ b/packages/loopover-mcp/bin/loopover-mcp.js @@ -30,6 +30,8 @@ import { buildSlopAssessment, SLOP_RUBRIC_MARKDOWN } from "@loopover/engine/sign import { buildTestEvidenceReport } from "@loopover/engine/signals/test-evidence"; // #6754: the same pure evaluator the remote MCP tool + /v1/loop/evaluate-escalation both call. import { evaluateEscalation } from "@loopover/engine"; +// #6752: the same pure composer the remote MCP tool + /v1/loop/results-payload both call. +import { buildResultsPayload } from "@loopover/engine"; import { z } from "zod"; import { buildBranchAnalysisPayload, collectLocalDiff, collectLocalBranchMetadata, probeLocalScorer, referenceScorePreviewExample, resolveScorePreviewCommand, resolveWorkspaceCwd, sanitizeLocalScorerStatus, setupGuidanceForLocalScorer, isTestFile } from "../lib/local-branch.js"; import { formatTable } from "../lib/format-table.js"; @@ -545,6 +547,19 @@ const evaluateEscalationShape = { killRequested: z.boolean().optional(), }; +// #6752: mirrors buildResultsPayloadShape in src/mcp/server.ts exactly, so the local tool, the remote tool, and +// the REST route all accept an identical payload. +const resultsPayloadShape = { + repoFullName: z.string().min(1), + prNumber: z.number().int().nullable().optional(), + title: z.string(), + changedFiles: z + .array(z.object({ path: z.string(), additions: z.number().int().optional(), deletions: z.number().int().optional() })) + .max(5000) + .optional(), + status: z.enum(["open", "merged", "closed"]).optional(), +}; + // #6749: mirrors checkTestEvidenceShape in src/mcp/server.ts VERBATIM (same bounds, same optionality). const checkTestEvidenceShape = { changedPaths: z.array(z.string().min(1).max(400)).max(2000), @@ -890,6 +905,12 @@ const STDIO_TOOL_DESCRIPTORS = [ description: "Decide whether a rented loop needs a human, and what action to take, from an already-computed run outcome, health tier, and operator/customer signals — the deterministic support/escalation-path logic. Source-free; returns shouldEscalate + action (none/notify/human_review/stop) + severity + reasons. It decides; the caller wires the action. Computed in-process; no API round-trip.", }, + { + name: "loopover_build_results_payload", + category: "agent", + description: + "Package a completed loop iteration into the customer-facing result (#4801): a PR link, a plain-language summary, and a bounded diff preview, from already-computed iteration metadata. Deterministic and source-free — it formats the result, it does not fetch, open, or deliver anything. Computed in-process; no API round-trip.", + }, { name: "loopover_check_issue_slop", category: "review", @@ -1487,6 +1508,18 @@ registerStdioTool( (input) => toolResult("LoopOver escalation decision.", evaluateEscalation(input)), ); +registerStdioTool( + "loopover_build_results_payload", + { + description: stdioToolDescription("loopover_build_results_payload"), + inputSchema: resultsPayloadShape, + }, + // Computed in-process from @loopover/engine (#6752) — the same pure buildResultsPayload the remote server + // (src/mcp/server.ts) and the /v1/loop/results-payload route both call, so all three surfaces return an + // identical payload for identical input, and results composition works fully offline. + (input) => toolResult("LoopOver loop results payload.", buildResultsPayload(input)), +); + registerStdioTool( "loopover_check_issue_slop", { diff --git a/src/api/routes.ts b/src/api/routes.ts index c6bc539bab..9dd2530839 100644 --- a/src/api/routes.ts +++ b/src/api/routes.ts @@ -200,6 +200,7 @@ import { runIssueRagRetrieval, validateIssueRagInput, type IssueRagInput } from import { buildBoundaryTestGenerationFinding, buildBoundaryTestGenerationSpec } from "../signals/boundary-test-generation"; import { buildTestEvidenceReport } from "../signals/test-evidence"; import { evaluateEscalation } from "../loop-escalation"; +import { buildResultsPayload } from "../results-payload"; import { loadPrAiReviewFindings } from "../mcp/pr-ai-review-findings"; import { buildMcpCompatibilityMetadata, @@ -490,6 +491,19 @@ const evaluateEscalationSchema = z.object({ killRequested: z.boolean().optional(), }); +// #6752: mirrors buildResultsPayloadShape in src/mcp/server.ts VERBATIM (same bounds, same optionality) so the +// REST surface can never accept an input the MCP tool would reject, or vice versa. +const resultsPayloadSchema = z.object({ + repoFullName: z.string().min(1), + prNumber: z.number().int().nullable().optional(), + title: z.string(), + changedFiles: z + .array(z.object({ path: z.string(), additions: z.number().int().optional(), deletions: z.number().int().optional() })) + .max(5000) + .optional(), + status: z.enum(["open", "merged", "closed"]).optional(), +}); + // #6749: mirrors checkTestEvidenceShape in src/mcp/server.ts VERBATIM (same bounds, same optionality) so the // REST surface can never accept an input the MCP tool would reject, or vice versa. const testEvidenceSchema = z.object({ @@ -3293,6 +3307,18 @@ export function createApp() { return c.json(evaluateEscalation(parsed.data)); }); + // #6752: REST mirror of the loopover_build_results_payload MCP tool, bringing it to the same REST/CLI parity + // its same-tier sibling loopover_check_slop_risk (/v1/lint/slop-risk) already has. Both are pure, source-free + // composers over caller-supplied, already-computed iteration metadata, so this route delegates to the same + // buildResultsPayload the tool calls and adds no logic of its own -- it formats the result, it does not fetch, + // open, or deliver anything. + app.post("/v1/loop/results-payload", async (c) => { + const body = await c.req.json().catch(() => null); + const parsed = resultsPayloadSchema.safeParse(body); + if (!parsed.success) return c.json({ error: "invalid_results_payload_request", issues: parsed.error.issues }, 400); + return c.json(buildResultsPayload(parsed.data)); + }); + app.post("/v1/lint/issue-slop", async (c) => { const body = await c.req.json().catch(() => null); const parsed = issueSlopSchema.safeParse(body); diff --git a/test/unit/mcp-cli-results-payload-tool.test.ts b/test/unit/mcp-cli-results-payload-tool.test.ts new file mode 100644 index 0000000000..2c5a6e7661 --- /dev/null +++ b/test/unit/mcp-cli-results-payload-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 { buildResultsPayload, type IterationResult } from "../../src/results-payload"; + +// #6752: the local mirror of loopover_build_results_payload. Like its same-tier sibling loopover_check_slop_risk, +// it composes IN-PROCESS from @loopover/engine — no API round-trip — so results composition works fully offline. +// The point of these tests is cross-surface PARITY: the stdio tool must return exactly what the pure +// buildResultsPayload returns for identical input (the same function /v1/loop/results-payload delegates to). +const bin = join(process.cwd(), "packages/loopover-mcp/bin/loopover-mcp.js"); + +let client: Client; +let transport: StdioClientTransport; +let configDir: string; + +beforeEach(async () => { + configDir = mkdtempSync(join(tmpdir(), "loopover-results-payload-")); + transport = new StdioClientTransport({ + command: "node", + args: [bin, "--stdio"], + // Pure + in-process: a black-holed API URL proves no round-trip happens. + env: { ...process.env, LOOPOVER_CONFIG_DIR: configDir, LOOPOVER_TOKEN: "session-token", LOOPOVER_API_URL: "http://127.0.0.1:1", LOOPOVER_API_TIMEOUT_MS: "1000" }, + }); + client = new Client({ name: "results-payload-test", version: "0.0.1" }); + await client.connect(transport); +}); + +afterEach(async () => { + await client?.close().catch(() => undefined); + if (configDir) rmSync(configDir, { recursive: true, force: true }); +}); + +describe("loopover_build_results_payload stdio mirror (#6752)", () => { + it("registers the tool alongside its same-tier check_slop_risk sibling", async () => { + const names = new Set((await client.listTools()).tools.map((t) => t.name)); + expect(names).toContain("loopover_build_results_payload"); + expect(names).toContain("loopover_check_slop_risk"); + }); + + it("matches the pure composer for every input shape — offline, with no API reachable", async () => { + const cases: IterationResult[] = [ + { repoFullName: "acme/widgets", prNumber: 42, title: "Opened", status: "open" }, + { repoFullName: "acme/widgets", prNumber: 42, title: "Merged", status: "merged" }, + { repoFullName: "acme/widgets", prNumber: 42, title: "Closed", status: "closed" }, + { repoFullName: "acme/widgets", prNumber: 7, title: "Status omitted defaults to open" }, + { repoFullName: "acme/widgets", title: "prNumber absent entirely" }, + { repoFullName: "acme/widgets", prNumber: null, title: "prNumber null" }, + { repoFullName: "acme/widgets", prNumber: 9, title: "Empty changed set", changedFiles: [] }, + { repoFullName: "acme/widgets", prNumber: 9, title: "Counts omitted", changedFiles: [{ path: "README.md" }] }, + { + repoFullName: "acme/widgets", + prNumber: 9, + title: "Over the preview cap", + changedFiles: Array.from({ length: 12 }, (_, i) => ({ path: `src/f${i}.ts`, additions: i, deletions: 1 })), + }, + ]; + for (const args of cases) { + const result = await client.callTool({ name: "loopover_build_results_payload", arguments: args }); + expect(result.isError, JSON.stringify(args)).toBeFalsy(); + // PARITY: identical to what the REST route returns, because both call this same function. + expect((result as { structuredContent?: unknown }).structuredContent, JSON.stringify(args)).toEqual( + JSON.parse(JSON.stringify(buildResultsPayload(args))), + ); + } + }); + + it("rejects invalid input (zod input-schema validation)", async () => { + for (const args of [ + {}, + { title: "missing repoFullName" }, + { repoFullName: "", title: "empty repoFullName" }, + { repoFullName: "acme/widgets" }, + { repoFullName: "acme/widgets", title: 7 }, + { repoFullName: "acme/widgets", title: "bad status", status: "reopened" }, + { repoFullName: "acme/widgets", title: "bad prNumber", prNumber: 1.5 }, + { repoFullName: "acme/widgets", title: "bad changedFiles", changedFiles: [{ additions: 1 }] }, + ]) { + const rejected = await client.callTool({ name: "loopover_build_results_payload", arguments: args }).then( + (r) => Boolean(r.isError), + () => true, + ); + expect(rejected, `${JSON.stringify(args)} should be rejected`).toBe(true); + } + }); +}); diff --git a/test/unit/mcp-tool-rename-aliases.test.ts b/test/unit/mcp-tool-rename-aliases.test.ts index 79c718bbf0..dce95ced24 100644 --- a/test/unit/mcp-tool-rename-aliases.test.ts +++ b/test/unit/mcp-tool-rename-aliases.test.ts @@ -10,6 +10,7 @@ // (#6621 registered the loopover_get_eligibility_plan REST/CLI mirror, taking the count from 61 to 62.) // (#6615 registered the loopover_close_pr write-tool — 9th of the 9 buildXSpec builders — taking the count from 62 to 63.) // (#6732 registered the loopover_monitor_open_prs CLI mirror, taking the count from 63 to 64.) +// (#6752 registered the loopover_build_results_payload CLI mirror, taking the count from 67 to 68.) import { Client } from "@modelcontextprotocol/sdk/client/index.js"; import { StdioClientTransport } from "@modelcontextprotocol/sdk/client/stdio.js"; import { mkdtempSync, rmSync } from "node:fs"; @@ -53,14 +54,14 @@ describe("MCP legacy alias retirement (#4777) — discovery invariants", () => { }); afterEach(disconnect); - it("lists exactly 67 loopover_ tools and zero gittensory_-prefixed aliases", async () => { + it("lists exactly 68 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(67); + expect(primary.length).toBe(68); expect(legacy.length).toBe(0); - expect(names.length).toBe(67); + expect(names.length).toBe(68); }); it("no loopover_ tool's description carries a stale deprecation notice", async () => { @@ -70,11 +71,11 @@ describe("MCP legacy alias retirement (#4777) — discovery invariants", () => { } }); - it("`loopover-mcp tools --json` reports the same 67-tool count the live server registers", async () => { + it("`loopover-mcp tools --json` reports the same 68-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(67); + expect(payload.count).toBe(68); expect([...payload.tools.map((t) => t.name)].sort()).toEqual([...tools.map((t) => t.name)].sort()); }); }); diff --git a/test/unit/routes-results-payload.test.ts b/test/unit/routes-results-payload.test.ts new file mode 100644 index 0000000000..6723a49655 --- /dev/null +++ b/test/unit/routes-results-payload.test.ts @@ -0,0 +1,97 @@ +import { describe, expect, it } from "vitest"; +import { createApp } from "../../src/api/routes"; +import { buildResultsPayload, type IterationResult } from "../../src/results-payload"; +import { createTestEnv } from "../helpers/d1"; + +// #6752: POST /v1/loop/results-payload — the REST mirror bringing loopover_build_results_payload to the same +// parity its same-tier sibling loopover_check_slop_risk (/v1/lint/slop-risk) already has. The route delegates to +// the pure buildResultsPayload (covered by its own unit tests in results-payload.test.ts), so these pin the ROUTE +// contract: the composed payload is returned unmodified for every shape the MCP tool accepts, and a bad body is +// rejected rather than passed through to the composer. +const apiHeaders = (env: Env) => ({ authorization: `Bearer ${env.LOOPOVER_API_TOKEN}`, "content-type": "application/json" }); +const PATH = "/v1/loop/results-payload"; + +const post = (env: Env, body: unknown) => + createApp().request(PATH, { method: "POST", headers: apiHeaders(env), body: JSON.stringify(body) }, env); + +describe("POST /v1/loop/results-payload (#6752)", () => { + it("composes the customer-facing result for an iteration that opened a PR", async () => { + const env = createTestEnv(); + const response = await post(env, { + repoFullName: "acme/widgets", + prNumber: 42, + title: "Add retry to the upload client", + changedFiles: [ + { path: "src/upload.ts", additions: 12, deletions: 3 }, + { path: "test/upload.test.ts", additions: 30, deletions: 0 }, + ], + status: "merged", + }); + expect(response.status).toBe(200); + await expect(response.json()).resolves.toMatchObject({ + prLink: "https://github.com/acme/widgets/pull/42", + totals: { files: 2, additions: 42, deletions: 3 }, + }); + }); + + it("returns null prLink when the iteration opened no pull request", async () => { + const env = createTestEnv(); + const response = await post(env, { repoFullName: "acme/widgets", prNumber: null, title: "Nothing to open" }); + expect(response.status).toBe(200); + await expect(response.json()).resolves.toMatchObject({ prLink: null, totals: { files: 0, additions: 0, deletions: 0 } }); + }); + + it("returns exactly what the pure composer returns for every shape the tool accepts", async () => { + const env = createTestEnv(); + // One case per meaningful arm of the composer's input: PR/no-PR, absent vs null prNumber, each status, + // absent/empty/partial changedFiles, and a change over the diff-preview cap (totals must still count all). + const cases: IterationResult[] = [ + { repoFullName: "acme/widgets", prNumber: 42, title: "Opened", status: "open" }, + { repoFullName: "acme/widgets", prNumber: 42, title: "Merged", status: "merged" }, + { repoFullName: "acme/widgets", prNumber: 42, title: "Closed", status: "closed" }, + { repoFullName: "acme/widgets", prNumber: 7, title: "Status omitted defaults to open" }, + { repoFullName: "acme/widgets", title: "prNumber absent entirely" }, + { repoFullName: "acme/widgets", prNumber: null, title: "prNumber null" }, + { repoFullName: "acme/widgets", prNumber: 9, title: "Empty changed set", changedFiles: [] }, + { repoFullName: "acme/widgets", prNumber: 9, title: "Counts omitted", changedFiles: [{ path: "README.md" }] }, + { + repoFullName: "acme/widgets", + prNumber: 9, + title: "Over the preview cap", + changedFiles: Array.from({ length: 12 }, (_, i) => ({ path: `src/f${i}.ts`, additions: i, deletions: 1 })), + }, + ]; + for (const body of cases) { + const response = await post(env, body); + expect(response.status, JSON.stringify(body)).toBe(200); + // PARITY: the route must return exactly what the pure composer the MCP tool calls returns. + await expect(response.json(), JSON.stringify(body)).resolves.toEqual(JSON.parse(JSON.stringify(buildResultsPayload(body)))); + } + }); + + it("rejects an invalid or unparseable body with 400", async () => { + const env = createTestEnv(); + for (const body of [ + {}, + { title: "missing repoFullName" }, + { repoFullName: "", title: "empty repoFullName" }, + { repoFullName: "acme/widgets" }, + { repoFullName: "acme/widgets", title: 7 }, + { repoFullName: "acme/widgets", title: "bad status", status: "reopened" }, + { repoFullName: "acme/widgets", title: "bad prNumber", prNumber: 1.5 }, + { repoFullName: "acme/widgets", title: "bad changedFiles", changedFiles: [{ additions: 1 }] }, + ]) { + const response = await post(env, body); + expect(response.status, JSON.stringify(body)).toBe(400); + await expect(response.json()).resolves.toMatchObject({ error: "invalid_results_payload_request" }); + } + const malformed = await createApp().request(PATH, { method: "POST", headers: apiHeaders(createTestEnv()), body: "{not json" }, createTestEnv()); + expect(malformed.status).toBe(400); + }); + + it("leaks no wallet/hotkey/trust-score terms", async () => { + const env = createTestEnv(); + const text = JSON.stringify(await (await post(env, { repoFullName: "acme/widgets", prNumber: 42, title: "Add retry" })).json()); + expect(text).not.toMatch(/wallet|hotkey|coldkey|trust score|reward/i); + }); +});