From a9759ff636caa4cfc15b8d6ffe783644772124b4 Mon Sep 17 00:00:00 2001 From: davion-knight <298846663+davion-knight@users.noreply.github.com> Date: Wed, 15 Jul 2026 23:42:49 -0500 Subject: [PATCH] feat(mcp): register the 8 miner write-tools on the local stdio server packages/loopover-mcp's miner-auto-dev profile lists loopover_open_pr, file_issue, apply_labels, post_eligibility_comment, create_branch, delete_branch, generate_tests, and file_follow_up_issue in its recommendedTools, but none were registered as local stdio tools -- a contributor on the local server could not invoke any of them. Register all 8, reusing the same pure @loopover/engine spec builders the remote server (src/mcp/server.ts) uses: each returns a LOCAL-execution action spec the caller runs with its OWN gh/git creds (loopover never performs the write). Input shapes mirror the remote bounds; the test-framework enum mirrors @loopover/engine's TEST_FRAMEWORKS. Adds a CLI-harness test per tool (spec composition + a zod-rejection failure path). Closes #6149 --- packages/loopover-mcp/bin/loopover-mcp.js | 194 ++++++++++++++++++++++ test/unit/mcp-cli-write-tools.test.ts | 154 +++++++++++++++++ test/unit/mcp-tool-rename-aliases.test.ts | 10 +- 3 files changed, 353 insertions(+), 5 deletions(-) create mode 100644 test/unit/mcp-cli-write-tools.test.ts diff --git a/packages/loopover-mcp/bin/loopover-mcp.js b/packages/loopover-mcp/bin/loopover-mcp.js index 7c4ad8489b..7971f0a687 100755 --- a/packages/loopover-mcp/bin/loopover-mcp.js +++ b/packages/loopover-mcp/bin/loopover-mcp.js @@ -6,6 +6,18 @@ import { delimiter, dirname, join } from "node:path"; import { McpServer, ResourceTemplate } from "@modelcontextprotocol/sdk/server/mcp.js"; import { StdioServerTransport } from "@modelcontextprotocol/sdk/server/stdio.js"; import { buildFeasibilityVerdict, buildPrTextLint } from "@loopover/engine"; +// #6149: the miner write-tools are PURE local-execution spec builders (loopover never performs the write); +// registering them locally is just importing the same engine builders the remote server uses. +import { + buildApplyLabelsSpec, + buildCreateBranchSpec, + buildDeleteBranchSpec, + buildFileIssueSpec, + buildFollowUpIssueSpec, + buildOpenPrSpec, + buildPostEligibilityCommentSpec, + buildTestGenSpec, +} from "@loopover/engine"; import { buildSlopAssessment, SLOP_RUBRIC_MARKDOWN } from "@loopover/engine/signals/slop"; import { z } from "zod"; import { buildBranchAnalysisPayload, collectLocalDiff, collectLocalBranchMetadata, probeLocalScorer, referenceScorePreviewExample, resolveScorePreviewCommand, resolveWorkspaceCwd, sanitizeLocalScorerStatus, setupGuidanceForLocalScorer, isTestFile } from "../lib/local-branch.js"; @@ -197,6 +209,65 @@ const ownerRepoPullShape = { number: z.number().int().positive(), }; +// #6149 write-tool input shapes -- mirror src/mcp/server.ts's remote shapes (same bounds) so the local +// server validates identically. The builders (buildOpenPrSpec, ...) are the same @loopover/engine functions. +const WRITE_TOOL_REPO_FULL_NAME_MAX = 200; +const WRITE_TOOL_BRANCH_REF_MAX = 200; +const WRITE_TOOL_TITLE_MAX = 400; +const WRITE_TOOL_BODY_MAX = 60000; +const WRITE_TOOL_BRANCH_MAX = 255; +// Mirrors @loopover/engine/signals/test-evidence's TEST_FRAMEWORKS (the detectTestConvention framework set), +// so a caller cannot request a test-gen spec for a framework the detector could never produce -- same guard the +// remote server's testGenShape uses. +const TEST_FRAMEWORKS = ["vitest", "jest", "pytest", "go-test", "rspec", "cargo-test"]; +const writeToolRepoFullName = z.string().min(3).max(WRITE_TOOL_REPO_FULL_NAME_MAX); +const openPrShape = { + repoFullName: writeToolRepoFullName, + base: z.string().min(1).max(WRITE_TOOL_BRANCH_REF_MAX), + head: z.string().min(1).max(WRITE_TOOL_BRANCH_REF_MAX), + title: z.string().min(1).max(WRITE_TOOL_TITLE_MAX), + body: z.string().max(WRITE_TOOL_BODY_MAX), + draft: z.boolean().optional(), +}; +const fileIssueShape = { + repoFullName: writeToolRepoFullName, + title: z.string().min(1).max(WRITE_TOOL_TITLE_MAX), + body: z.string().max(WRITE_TOOL_BODY_MAX), + labels: z.array(z.string().min(1).max(100)).max(20).optional(), +}; +const applyLabelsShape = { + repoFullName: writeToolRepoFullName, + number: z.number().int().positive(), + labels: z.array(z.string().min(1).max(100)).min(1).max(20), +}; +const postEligibilityCommentShape = { + repoFullName: writeToolRepoFullName, + number: z.number().int().positive(), + body: z.string().min(1).max(WRITE_TOOL_BODY_MAX), +}; +const createBranchShape = { + branch: z.string().min(1).max(WRITE_TOOL_BRANCH_MAX), + base: z.string().min(1).max(WRITE_TOOL_BRANCH_MAX).optional(), +}; +const deleteBranchShape = { + branch: z.string().min(1).max(WRITE_TOOL_BRANCH_MAX), + remote: z.boolean().optional(), +}; +const testGenShape = { + repoFullName: writeToolRepoFullName, + targetFiles: z.array(z.string().min(1).max(500)).min(1).max(50), + framework: z.enum(TEST_FRAMEWORKS), + testDir: z.string().min(1).max(255).optional(), + criteria: z.array(z.string().min(1).max(300)).max(20).optional(), +}; +const followUpIssueShape = { + repoFullName: writeToolRepoFullName, + path: z.string().min(1).max(500), + line: z.number().int().positive().optional(), + finding: z.string().min(1).max(WRITE_TOOL_BODY_MAX), + label: z.string().min(1).max(100).optional(), +}; + const loginShape = { login: z.string().min(1), }; @@ -746,6 +817,51 @@ const STDIO_TOOL_DESCRIPTORS = [ category: "maintainer", description: "Return per-gate-type false-positive precision for a repo's recorded gate blocks — blocked / blocked-then-merged counts and false-positive rates with low-sample guards. Optionally bounded by windowDays. Maintainer-authenticated; measurement only.", }, + { + name: "loopover_open_pr", + category: "agent", + description: + "Build a LOCAL-execution spec to open a pull request from your branch (run it with your own gh creds; loopover never performs the write).", + }, + { + name: "loopover_file_issue", + category: "agent", + description: "Build a LOCAL-execution spec to file an issue (run it with your own gh creds; loopover never performs the write).", + }, + { + name: "loopover_apply_labels", + category: "agent", + description: + "Build a LOCAL-execution spec to add labels to an issue or PR (run it with your own gh creds; loopover never performs the write).", + }, + { + name: "loopover_post_eligibility_comment", + category: "agent", + description: + "Build a LOCAL-execution spec to post an eligibility/context comment on an issue or PR (run it with your own gh creds; loopover never performs the write).", + }, + { + name: "loopover_create_branch", + category: "agent", + description: "Build a LOCAL-execution spec to create a branch (run it locally; loopover never performs the write).", + }, + { + name: "loopover_delete_branch", + category: "agent", + description: "Build a LOCAL-execution spec to delete a branch (run it locally; loopover never performs the write).", + }, + { + name: "loopover_generate_tests", + category: "agent", + description: + "Build a LOCAL-execution spec describing WHAT boundary-safe test cases should exist for the given target files, using the repo's detected framework/convention. LoopOver supplies the criteria; your OWN agent scaffolds and runs the actual test files locally -- no source code is uploaded and loopover never performs the write.", + }, + { + name: "loopover_file_follow_up_issue", + category: "agent", + description: + "Build a LOCAL-execution spec to file a follow-up issue for a review finding a maintainer wants TRACKED rather than blocked on this PR. Composes a bounded, public-safe title/body from the finding (run it with your own gh creds; loopover never performs the write).", + }, ]; // #6301 — coarse tool categories for grouping `loopover-mcp tools` output. Ordered @@ -1596,6 +1712,84 @@ registerStdioTool( const payload = await apiGet(`${toolRepoBase(owner, repo)}/gate-precision${query}`); return toolResult(`Gate precision for ${owner}/${repo}.`, payload); }, + ); +// ── Write-tools (#6149): pure LOCAL-execution spec builders. loopover NEVER performs the write -- each tool +// returns a spec the caller runs with its OWN gh creds. Brings the local stdio server to parity with the +// miner-auto-dev profile's recommendedTools, using the same @loopover/engine builders as the remote server. +function localWriteSpecResult(spec) { + return toolResult(`${spec.action}: ${spec.description} ${spec.boundary}`, spec); +} + +registerStdioTool( + "loopover_open_pr", + { + description: stdioToolDescription("loopover_open_pr"), + inputSchema: openPrShape, + }, + (input) => localWriteSpecResult(buildOpenPrSpec(input)), +); + +registerStdioTool( + "loopover_file_issue", + { + description: stdioToolDescription("loopover_file_issue"), + inputSchema: fileIssueShape, + }, + (input) => localWriteSpecResult(buildFileIssueSpec(input)), +); + +registerStdioTool( + "loopover_apply_labels", + { + description: stdioToolDescription("loopover_apply_labels"), + inputSchema: applyLabelsShape, + }, + (input) => localWriteSpecResult(buildApplyLabelsSpec(input)), +); + +registerStdioTool( + "loopover_post_eligibility_comment", + { + description: stdioToolDescription("loopover_post_eligibility_comment"), + inputSchema: postEligibilityCommentShape, + }, + (input) => localWriteSpecResult(buildPostEligibilityCommentSpec(input)), +); + +registerStdioTool( + "loopover_create_branch", + { + description: stdioToolDescription("loopover_create_branch"), + inputSchema: createBranchShape, + }, + (input) => localWriteSpecResult(buildCreateBranchSpec(input)), +); + +registerStdioTool( + "loopover_delete_branch", + { + description: stdioToolDescription("loopover_delete_branch"), + inputSchema: deleteBranchShape, + }, + (input) => localWriteSpecResult(buildDeleteBranchSpec(input)), +); + +registerStdioTool( + "loopover_generate_tests", + { + description: stdioToolDescription("loopover_generate_tests"), + inputSchema: testGenShape, + }, + (input) => localWriteSpecResult(buildTestGenSpec(input)), +); + +registerStdioTool( + "loopover_file_follow_up_issue", + { + description: stdioToolDescription("loopover_file_follow_up_issue"), + inputSchema: followUpIssueShape, + }, + (input) => localWriteSpecResult(buildFollowUpIssueSpec(input)), ); // ── Resources: decision-pack, doctor, compatibility, changelog (#292) ───────── diff --git a/test/unit/mcp-cli-write-tools.test.ts b/test/unit/mcp-cli-write-tools.test.ts new file mode 100644 index 0000000000..0de741ec63 --- /dev/null +++ b/test/unit/mcp-cli-write-tools.test.ts @@ -0,0 +1,154 @@ +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"; + +// #6149: the 8 miner write-tools are PURE local-execution spec builders (loopover never performs the write); +// each returns a { action, command, boundary } spec the caller runs with its OWN gh/git creds. These tests +// drive the real local stdio server and assert the composed spec, plus a zod-rejection failure path per tool. +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-write-tools-")); + transport = new StdioClientTransport({ + command: "node", + args: [bin, "--stdio"], + // Write-tools are pure and never call the API, but the stdio server still needs a config dir + token to boot. + env: { ...process.env, LOOPOVER_CONFIG_DIR: configDir, LOOPOVER_TOKEN: "session-token", LOOPOVER_API_TIMEOUT_MS: "5000" }, + }); + client = new Client({ name: "write-tools-test", version: "0.0.1" }); + await client.connect(transport); +}); + +afterEach(async () => { + await client.close().catch(() => undefined); + if (configDir) rmSync(configDir, { recursive: true, force: true }); +}); + +const WRITE_TOOLS = [ + "loopover_open_pr", + "loopover_file_issue", + "loopover_apply_labels", + "loopover_post_eligibility_comment", + "loopover_create_branch", + "loopover_delete_branch", + "loopover_generate_tests", + "loopover_file_follow_up_issue", +]; + +function spec(result: unknown): { action: string; command: string; boundary: string } { + return (result as { structuredContent?: unknown }).structuredContent as { action: string; command: string; boundary: string }; +} + +describe("loopover-mcp write-tools (#6149)", () => { + it("registers all 8 write-tools on the local stdio server", async () => { + const names = new Set((await client.listTools()).tools.map((t) => t.name)); + for (const name of WRITE_TOOLS) expect(names, `missing ${name}`).toContain(name); + }); + + it("loopover_open_pr composes a gh pr create spec (loopover never performs the write)", async () => { + const result = await client.callTool({ + name: "loopover_open_pr", + arguments: { repoFullName: "acme/widgets", base: "main", head: "feat-x", title: "Add X", body: "Body" }, + }); + expect(result.isError).toBeFalsy(); + const s = spec(result); + expect(s.action).toBe("open_pr"); + expect(s.command).toContain("gh pr create --repo 'acme/widgets'"); + expect(s.command).toContain("--head 'feat-x'"); + expect(JSON.stringify(result)).not.toMatch(/wallet|hotkey|coldkey|reward estimate/i); + }); + + it("loopover_file_issue composes a gh issue create spec", async () => { + const result = await client.callTool({ + name: "loopover_file_issue", + arguments: { repoFullName: "acme/widgets", title: "Bug", body: "desc", labels: ["bug"] }, + }); + expect(result.isError).toBeFalsy(); + expect(spec(result).action).toBe("file_issue"); + expect(spec(result).command).toContain("gh issue create --repo 'acme/widgets'"); + }); + + it("loopover_apply_labels composes a gh issue edit --add-label spec", async () => { + const result = await client.callTool({ + name: "loopover_apply_labels", + arguments: { repoFullName: "acme/widgets", number: 7, labels: ["bug", "help wanted"] }, + }); + expect(result.isError).toBeFalsy(); + expect(spec(result).action).toBe("apply_labels"); + expect(spec(result).command).toContain("gh issue edit 7 --repo 'acme/widgets'"); + expect(spec(result).command).toContain("--add-label"); + }); + + it("loopover_post_eligibility_comment composes a gh issue comment spec", async () => { + const result = await client.callTool({ + name: "loopover_post_eligibility_comment", + arguments: { repoFullName: "acme/widgets", number: 7, body: "context" }, + }); + expect(result.isError).toBeFalsy(); + expect(spec(result).action).toBe("post_eligibility_comment"); + expect(spec(result).command).toContain("gh issue comment 7 --repo 'acme/widgets'"); + }); + + it("loopover_create_branch composes a git switch -c spec", async () => { + const result = await client.callTool({ name: "loopover_create_branch", arguments: { branch: "feat-x", base: "main" } }); + expect(result.isError).toBeFalsy(); + expect(spec(result).action).toBe("create_branch"); + expect(spec(result).command).toContain("git switch -c 'feat-x'"); + }); + + it("loopover_delete_branch composes a git branch -D spec", async () => { + const result = await client.callTool({ name: "loopover_delete_branch", arguments: { branch: "feat-x", remote: true } }); + expect(result.isError).toBeFalsy(); + expect(spec(result).action).toBe("delete_branch"); + expect(spec(result).command).toContain("git branch -D 'feat-x'"); + }); + + it("loopover_generate_tests composes a boundary-safe test-scaffold spec for the detected framework", async () => { + const result = await client.callTool({ + name: "loopover_generate_tests", + arguments: { repoFullName: "acme/widgets", targetFiles: ["src/x.ts"], framework: "vitest" }, + }); + expect(result.isError).toBeFalsy(); + expect(spec(result).action).toBe("generate_tests"); + expect(spec(result).command).toContain("vitest"); + }); + + it("loopover_file_follow_up_issue composes a follow-up gh issue create spec", async () => { + const result = await client.callTool({ + name: "loopover_file_follow_up_issue", + arguments: { repoFullName: "acme/widgets", path: "src/x.ts", finding: "possible leak" }, + }); + expect(result.isError).toBeFalsy(); + expect(spec(result).action).toBe("file_issue"); + expect(spec(result).command).toContain("gh issue create --repo 'acme/widgets'"); + expect(spec(result).command).toContain("Follow up"); + }); + + it("rejects invalid input for each write-tool (zod input-schema validation)", async () => { + // One representative invalid payload per tool: a missing/blank required field the shape forbids. + const invalid: Record> = { + loopover_open_pr: { repoFullName: "acme/widgets", base: "main", head: "feat-x", title: "", body: "b" }, // title min(1) + loopover_file_issue: { repoFullName: "ab", title: "T", body: "b" }, // repoFullName min(3) + loopover_apply_labels: { repoFullName: "acme/widgets", number: 7, labels: [] }, // labels min(1) + loopover_post_eligibility_comment: { repoFullName: "acme/widgets", number: 0, body: "b" }, // number positive + loopover_create_branch: { base: "main" }, // branch required + loopover_delete_branch: { branch: "" }, // branch min(1) + loopover_generate_tests: { repoFullName: "acme/widgets", targetFiles: ["src/x.ts"], framework: "mocha" }, // not in enum + loopover_file_follow_up_issue: { repoFullName: "acme/widgets", path: "src/x.ts" }, // finding required + }; + for (const [name, args] of Object.entries(invalid)) { + const outcome = await client.callTool({ name, arguments: args }).then( + (r) => ({ threw: false, isError: Boolean(r.isError) }), + () => ({ threw: true, isError: true }), + ); + expect(outcome.isError, `${name} should reject invalid input`).toBe(true); + } + }); +}); diff --git a/test/unit/mcp-tool-rename-aliases.test.ts b/test/unit/mcp-tool-rename-aliases.test.ts index 3f6f4c3286..bba49561b4 100644 --- a/test/unit/mcp-tool-rename-aliases.test.ts +++ b/test/unit/mcp-tool-rename-aliases.test.ts @@ -47,14 +47,14 @@ describe("MCP legacy alias retirement (#4777) — discovery invariants", () => { }); afterEach(disconnect); - it("lists exactly 47 loopover_ tools and zero gittensory_-prefixed aliases", async () => { + it("lists exactly 55 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(47); + expect(primary.length).toBe(55); expect(legacy.length).toBe(0); - expect(names.length).toBe(47); + expect(names.length).toBe(55); }); it("no loopover_ tool's description carries a stale deprecation notice", async () => { @@ -64,11 +64,11 @@ describe("MCP legacy alias retirement (#4777) — discovery invariants", () => { } }); - it("`loopover-mcp tools --json` reports the same 47-tool count the live server registers", async () => { + it("`loopover-mcp tools --json` reports the same 55-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(47); + expect(payload.count).toBe(55); expect([...payload.tools.map((t) => t.name)].sort()).toEqual([...tools.map((t) => t.name)].sort()); }); });