Skip to content
Closed
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
49 changes: 49 additions & 0 deletions packages/loopover-mcp/bin/loopover-mcp.js
Original file line number Diff line number Diff line change
Expand Up @@ -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() }))
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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
Expand Down
15 changes: 15 additions & 0 deletions src/api/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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
Expand Down
4 changes: 3 additions & 1 deletion src/mcp/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
88 changes: 88 additions & 0 deletions test/unit/mcp-cli-improvement-potential-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 { 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);
});
});
10 changes: 5 additions & 5 deletions test/unit/mcp-tool-rename-aliases.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand All @@ -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());
});
});
Expand Down
76 changes: 76 additions & 0 deletions test/unit/routes-improvement-potential.test.ts
Original file line number Diff line number Diff line change
@@ -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<typeof buildStructuralImprovementAssessment>[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);
});
});
5 changes: 5 additions & 0 deletions test/unit/support/mcp-cli-harness.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down