Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions packages/loopover-mcp/bin/loopover-mcp.js
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
{
Expand Down
26 changes: 26 additions & 0 deletions src/api/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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({
Expand Down Expand Up @@ -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);
Expand Down
88 changes: 88 additions & 0 deletions test/unit/mcp-cli-results-payload-tool.test.ts
Original file line number Diff line number Diff line change
@@ -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);
}
});
});
11 changes: 6 additions & 5 deletions test/unit/mcp-tool-rename-aliases.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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 () => {
Expand All @@ -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());
});
});
Expand Down
97 changes: 97 additions & 0 deletions test/unit/routes-results-payload.test.ts
Original file line number Diff line number Diff line change
@@ -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);
});
});