From 4d5212ed794a3a2aba6a22f4c6fcfdb6b10fee0f Mon Sep 17 00:00:00 2001 From: davion-knight <298846663+davion-knight@users.noreply.github.com> Date: Fri, 17 Jul 2026 04:40:02 -0500 Subject: [PATCH] feat(api): add a REST route + CLI mirror for loopover_suggest_boundary_tests loopover_suggest_boundary_tests had neither a REST route nor a CLI mirror, unlike its same-tier advisory-lint sibling loopover_check_slop_risk, which already has full parity (POST /v1/lint/slop-risk + a stdio tool). Both are deterministic, advisory-only, no-repo-access evaluators over caller-supplied metadata, so the asymmetry was arbitrary. Adds POST /v1/lint/boundary-tests beside /v1/lint/slop-risk, reproducing the MCP tool's handler exactly: keep only boundary touches whose path is in the changed set, build the finding, and build the spec only when the finding fired. It reuses buildBoundaryTestGenerationFinding/Spec as-is. Advisory only -- never blocks, never writes, and returns criteria/hints only, never generated test code, so the review/execution boundary stays intact. Adds the loopover_suggest_boundary_tests stdio tool. Unlike the check_slop_risk sibling this one PROXIES to the route rather than computing in-process: the builders live app-side (src/signals/boundary-test-generation.ts depends on the app's AdvisoryFinding type), not in @loopover/engine, so extracting them to compute locally would drag an app type into the engine. The route therefore stays the single source of truth for the filtering + finding/spec logic. Both new input schemas mirror the MCP tool's suggestBoundaryTestsShape VERBATIM -- same bounds, same .strict() objects, same optionality -- so no surface can accept an input another would reject (including an empty changedFiles, which the tool accepts). Route tests pin the handler's filtering (an out-of-diff touch is dropped), the finding/spec pairing, the no-gap cases (test evidence present, no touches), schema parity, and 400s. The CLI test asserts the proxied POST and that zod rejects bad input before any API call. 100% line and branch coverage on the new route lines. Closes #6750 --- packages/loopover-mcp/bin/loopover-mcp.js | 30 ++++++++ src/api/routes.ts | 35 +++++++++ test/unit/mcp-cli-boundary-tests-tool.test.ts | 71 +++++++++++++++++++ test/unit/mcp-tool-rename-aliases.test.ts | 10 +-- test/unit/routes-boundary-tests.test.ts | 71 +++++++++++++++++++ test/unit/support/mcp-cli-harness.ts | 10 +++ 6 files changed, 222 insertions(+), 5 deletions(-) create mode 100644 test/unit/mcp-cli-boundary-tests-tool.test.ts create mode 100644 test/unit/routes-boundary-tests.test.ts diff --git a/packages/loopover-mcp/bin/loopover-mcp.js b/packages/loopover-mcp/bin/loopover-mcp.js index 8f06315d7..862b74444 100755 --- a/packages/loopover-mcp/bin/loopover-mcp.js +++ b/packages/loopover-mcp/bin/loopover-mcp.js @@ -552,6 +552,17 @@ const checkTestEvidenceShape = { tests: z.array(z.string().max(400)).max(2000).optional(), }; +// #6750: mirrors suggestBoundaryTestsShape in src/mcp/server.ts VERBATIM. +const suggestBoundaryTestsShape = { + changedFiles: z.array(z.object({ path: z.string().min(1).max(400) }).strict()).max(500), + boundaryTouches: z + .array(z.object({ path: z.string().min(1).max(400), kind: z.enum(["array_index_bounds", "null_or_undefined_branch", "empty_collection_check"]) }).strict()) + .max(20) + .optional(), + tests: z.array(z.string().max(400)).max(2000).optional(), + testFiles: z.array(z.string().max(400)).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() })) @@ -861,6 +872,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_suggest_boundary_tests", + category: "review", + description: + "Boundary-safe test-generation suggestion: evaluate locally precomputed boundary-touch metadata (path + pattern kind only; no patch/source text) with no test evidence in the diff, and return a LOCAL-execution action spec (criteria/hints only — never generated test code) for your OWN agent to scaffold tests with. Advisory-only; never blocks, never writes.", + }, { name: "loopover_check_test_evidence", category: "review", @@ -1433,6 +1450,19 @@ registerStdioTool( (input) => toolResult("LoopOver slop-risk self-check.", { ...buildSlopAssessment(input), rubric: SLOP_RUBRIC_MARKDOWN }), ); +// #6750: CLI mirror of the remote server's loopover_suggest_boundary_tests. Unlike its check_slop_risk sibling +// this one PROXIES rather than computing in-process: the builders live app-side (src/signals/ +// boundary-test-generation.ts, which depends on the app's AdvisoryFinding type), not in @loopover/engine, so +// POST /v1/lint/boundary-tests stays the single source of truth for the filtering + finding/spec logic. +registerStdioTool( + "loopover_suggest_boundary_tests", + { + description: stdioToolDescription("loopover_suggest_boundary_tests"), + inputSchema: suggestBoundaryTestsShape, + }, + async (input) => toolResult("LoopOver boundary-test suggestion.", await apiPost("/v1/lint/boundary-tests", input)), +); + registerStdioTool( "loopover_check_test_evidence", { diff --git a/src/api/routes.ts b/src/api/routes.ts index 6386296d6..c6bc539ba 100644 --- a/src/api/routes.ts +++ b/src/api/routes.ts @@ -197,6 +197,7 @@ import { } from "../services/control-panel-roles"; import { runFindOpportunities, validateFindOpportunitiesInput, type FindOpportunitiesInput } from "../mcp/find-opportunities"; import { runIssueRagRetrieval, validateIssueRagInput, type IssueRagInput } from "../mcp/issue-rag"; +import { buildBoundaryTestGenerationFinding, buildBoundaryTestGenerationSpec } from "../signals/boundary-test-generation"; import { buildTestEvidenceReport } from "../signals/test-evidence"; import { evaluateEscalation } from "../loop-escalation"; import { loadPrAiReviewFindings } from "../mcp/pr-ai-review-findings"; @@ -497,6 +498,25 @@ const testEvidenceSchema = z.object({ tests: z.array(z.string().max(400)).max(2000).optional(), }); +// #6750: mirrors suggestBoundaryTestsShape in src/mcp/server.ts VERBATIM (same bounds, same .strict() +// objects, same optionality) so the REST surface can never accept an input the MCP tool would reject. +const boundaryTestsSchema = z.object({ + changedFiles: z.array(z.object({ path: z.string().min(1).max(400) }).strict()).max(500), + boundaryTouches: z + .array( + z + .object({ + path: z.string().min(1).max(400), + kind: z.enum(["array_index_bounds", "null_or_undefined_branch", "empty_collection_check"]), + }) + .strict(), + ) + .max(20) + .optional(), + tests: z.array(z.string().max(400)).max(2000).optional(), + testFiles: z.array(z.string().max(400)).max(2000).optional(), +}); + const slopRiskSchema = z.object({ 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() })) @@ -3237,6 +3257,21 @@ export function createApp() { return c.json({ ...buildSlopAssessment(parsed.data), rubric: SLOP_RUBRIC_MARKDOWN }); }); + // #6750: REST mirror of the loopover_suggest_boundary_tests MCP tool, bringing it to the same parity its + // same-tier advisory-lint sibling /v1/lint/slop-risk (directly above) already has. Reproduces the tool's + // handler exactly: keep only touches whose path is actually in the changed set, build the finding, and build + // the spec only when the finding fired. Advisory-only -- never blocks, never writes, and returns criteria/ + // hints only (never generated test code), so the review/execution boundary stays intact. + app.post("/v1/lint/boundary-tests", async (c) => { + const body = await c.req.json().catch(() => null); + const parsed = boundaryTestsSchema.safeParse(body); + if (!parsed.success) return c.json({ error: "invalid_boundary_tests_request", issues: parsed.error.issues }, 400); + const changedPaths = new Set(parsed.data.changedFiles.map((file) => file.path)); + const touches = (parsed.data.boundaryTouches ?? []).filter((touch) => changedPaths.has(touch.path)); + const finding = buildBoundaryTestGenerationFinding({ touches, tests: parsed.data.tests, testFiles: parsed.data.testFiles }); + return c.json({ finding, spec: finding ? buildBoundaryTestGenerationSpec(touches) : null }); + }); + // #6749: REST mirror of the loopover_check_test_evidence MCP tool, bringing it to the same parity its // same-tier deterministic-lint sibling /v1/lint/slop-risk (directly above) already has. Delegates to the // engine's buildTestEvidenceReport -- the same function the MCP tool and the CLI mirror call. diff --git a/test/unit/mcp-cli-boundary-tests-tool.test.ts b/test/unit/mcp-cli-boundary-tests-tool.test.ts new file mode 100644 index 000000000..01642d80c --- /dev/null +++ b/test/unit/mcp-cli-boundary-tests-tool.test.ts @@ -0,0 +1,71 @@ +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"; + +// #6750: the CLI mirror of loopover_suggest_boundary_tests. Unlike the check_slop_risk sibling it PROXIES to +// POST /v1/lint/boundary-tests rather than computing in-process, because the builders live app-side +// (src/signals/boundary-test-generation.ts depends on the app's AdvisoryFinding type), not in @loopover/engine. +// The route therefore stays the single source of truth; these tests pin the request the tool composes and its +// zod shape, and routes-boundary-tests.test.ts pins the verdict itself. +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-boundary-tests-")); + captured = []; + const apiUrl = await startFixtureServer({ + onApiRequest: (request) => { + if (request.url?.includes("/lint/boundary-tests")) 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: "boundary-tests-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_suggest_boundary_tests stdio mirror (#6750)", () => { + it("registers 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_suggest_boundary_tests"); + expect(names).toContain("loopover_check_slop_risk"); + }); + + it("proxies to POST /v1/lint/boundary-tests and returns the route's finding + spec", async () => { + const result = await client.callTool({ + name: "loopover_suggest_boundary_tests", + arguments: { changedFiles: [{ path: "src/a.ts" }], boundaryTouches: [{ path: "src/a.ts", kind: "array_index_bounds" }] }, + }); + expect(result.isError).toBeFalsy(); + expect(captured).toHaveLength(1); + expect(captured[0]!.method).toBe("POST"); + const text = JSON.stringify(result); + expect(text).toContain("scaffold_boundary_tests"); + // Criteria/hints only — never generated test code, and nothing private. + expect(text).not.toMatch(/wallet|hotkey|coldkey|trust score|reward estimate/i); + }); + + it("rejects invalid input (zod) before any API call", async () => { + for (const args of [{}, { changedFiles: [{ path: "" }] }, { changedFiles: [{ path: "src/a.ts", extra: 1 }] }, { changedFiles: [{ path: "src/a.ts" }], boundaryTouches: [{ path: "src/a.ts", kind: "bogus" }] }]) { + const rejected = await client.callTool({ name: "loopover_suggest_boundary_tests", arguments: args }).then((r) => Boolean(r.isError), () => true); + expect(rejected, `${JSON.stringify(args)} should be rejected`).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 e77b0fb66..79c718bbf 100644 --- a/test/unit/mcp-tool-rename-aliases.test.ts +++ b/test/unit/mcp-tool-rename-aliases.test.ts @@ -53,14 +53,14 @@ describe("MCP legacy alias retirement (#4777) — discovery invariants", () => { }); afterEach(disconnect); - it("lists exactly 66 loopover_ tools and zero gittensory_-prefixed aliases", async () => { + it("lists exactly 67 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(66); + expect(primary.length).toBe(67); expect(legacy.length).toBe(0); - expect(names.length).toBe(66); + expect(names.length).toBe(67); }); it("no loopover_ tool's description carries a stale deprecation notice", async () => { @@ -70,11 +70,11 @@ describe("MCP legacy alias retirement (#4777) — discovery invariants", () => { } }); - it("`loopover-mcp tools --json` reports the same 66-tool count the live server registers", async () => { + it("`loopover-mcp tools --json` reports the same 67-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(66); + expect(payload.count).toBe(67); expect([...payload.tools.map((t) => t.name)].sort()).toEqual([...tools.map((t) => t.name)].sort()); }); }); diff --git a/test/unit/routes-boundary-tests.test.ts b/test/unit/routes-boundary-tests.test.ts new file mode 100644 index 000000000..2c2c4841b --- /dev/null +++ b/test/unit/routes-boundary-tests.test.ts @@ -0,0 +1,71 @@ +import { describe, expect, it } from "vitest"; +import { createApp } from "../../src/api/routes"; +import { buildBoundaryTestGenerationFinding, buildBoundaryTestGenerationSpec } from "../../src/signals/boundary-test-generation"; +import { createTestEnv } from "../helpers/d1"; + +// #6750: POST /v1/lint/boundary-tests — the REST mirror bringing loopover_suggest_boundary_tests to the parity +// its same-tier advisory-lint sibling /v1/lint/slop-risk already has. The builders' own logic is covered by +// boundary-test-generation's own tests; these pin the ROUTE contract: the tool handler's filtering reproduced +// exactly, the finding/spec pairing, schema parity, and 400s. +const apiHeaders = (env: Env) => ({ authorization: `Bearer ${env.LOOPOVER_API_TOKEN}`, "content-type": "application/json" }); +const PATH = "/v1/lint/boundary-tests"; +const post = (env: Env, body: unknown) => createApp().request(PATH, { method: "POST", headers: apiHeaders(env), body: JSON.stringify(body) }, env); +const TOUCH = { path: "src/a.ts", kind: "array_index_bounds" as const }; + +describe("POST /v1/lint/boundary-tests (#6750)", () => { + it("returns the finding + spec the tool's own handler would build, for a boundary touch with no test evidence", async () => { + const env = createTestEnv(); + const body = { changedFiles: [{ path: "src/a.ts" }], boundaryTouches: [TOUCH] }; + const response = await post(env, body); + expect(response.status).toBe(200); + // PARITY: identical to the MCP tool's own composition over the same builders. + const finding = buildBoundaryTestGenerationFinding({ touches: [TOUCH], tests: undefined, testFiles: undefined }); + await expect(response.json()).resolves.toEqual(JSON.parse(JSON.stringify({ finding, spec: finding ? buildBoundaryTestGenerationSpec([TOUCH]) : null }))); + const payload = (await (await post(env, body)).json()) as { finding: unknown; spec: { action?: string } | null }; + expect(payload.finding).toBeTruthy(); + expect(payload.spec?.action).toBe("scaffold_boundary_tests"); + }); + + it("drops touches whose path is not in the changed set (the handler's own filter)", async () => { + const env = createTestEnv(); + const payload = (await (await post(env, { + changedFiles: [{ path: "src/a.ts" }], + boundaryTouches: [{ path: "src/somewhere-else.ts", kind: "null_or_undefined_branch" }], + })).json()) as { finding: unknown; spec: unknown }; + // The only touch is filtered out, so there is no gap to report. + expect(payload.finding).toBeNull(); + expect(payload.spec).toBeNull(); + }); + + it("reports no gap when test evidence exists, and returns a null spec with a null finding", async () => { + const env = createTestEnv(); + for (const body of [ + { changedFiles: [{ path: "src/a.ts" }], boundaryTouches: [TOUCH], testFiles: ["test/a.test.ts"] }, + { changedFiles: [{ path: "src/a.ts" }], boundaryTouches: [TOUCH], tests: ["ran the suite locally"] }, + { changedFiles: [{ path: "src/a.ts" }] }, // no touches at all + ]) { + const payload = (await (await post(env, body)).json()) as { finding: unknown; spec: unknown }; + expect(payload.finding, JSON.stringify(body)).toBeNull(); + expect(payload.spec, JSON.stringify(body)).toBeNull(); + } + }); + + it("accepts exactly what the MCP tool's shape accepts, and 400s what it rejects", async () => { + const env = createTestEnv(); + expect((await post(env, { changedFiles: [] })).status).toBe(200); // empty changedFiles is valid for the tool + for (const body of [{}, { changedFiles: [{ path: "" }] }, { changedFiles: [{ path: "src/a.ts", extra: 1 }] }, { changedFiles: [{ path: "src/a.ts" }], boundaryTouches: [{ path: "src/a.ts", kind: "bogus" }] }]) { + const response = await post(env, body); + expect(response.status, JSON.stringify(body)).toBe(400); + await expect(response.json()).resolves.toMatchObject({ error: "invalid_boundary_tests_request" }); + } + const malformed = await createApp().request(PATH, { method: "POST", headers: apiHeaders(createTestEnv()), body: "{not json" }, createTestEnv()); + expect(malformed.status).toBe(400); + }); + + it("returns criteria/hints only — never generated test code — and no private terms", async () => { + const env = createTestEnv(); + const text = JSON.stringify(await (await post(env, { changedFiles: [{ path: "src/a.ts" }], boundaryTouches: [TOUCH] })).json()); + expect(text).not.toMatch(/wallet|hotkey|coldkey|trust score|reward/i); + expect(text).not.toMatch(/\bit\(|\bdescribe\(|\bexpect\(/); // no scaffolded test code crosses the boundary + }); +}); diff --git a/test/unit/support/mcp-cli-harness.ts b/test/unit/support/mcp-cli-harness.ts index b8533f976..fa2eec366 100644 --- a/test/unit/support/mcp-cli-harness.ts +++ b/test/unit/support/mcp-cli-harness.ts @@ -539,6 +539,16 @@ export async function startFixtureServer( response.end(JSON.stringify({ generatedAt: "2026-05-30T00:00:00.000Z", limit: 20, hasMore: false, items: [{ prNumber: 7, reason: "duplicate", remediation: "link the canonical PR" }] })); return; } + // #6750: the boundary-tests lint mirror proxies here; echo a fired finding + spec. + if (request.url === "/v1/lint/boundary-tests" && request.method === "POST") { + response.end( + JSON.stringify({ + finding: { code: "boundary_test_generation", severity: "advisory", summary: "Boundary-condition code changed without test evidence." }, + spec: { action: "scaffold_boundary_tests", hints: ["Cover the empty and one-element cases."], boundary: "Your agent scaffolds the tests; loopover never writes code." }, + }), + ); + return; + } if (request.url === "/v1/repos/owner/repo/pulls/7/reviewability" && request.method === "GET") { response.end( JSON.stringify({