From aaa4392163139be3eec2e13e60c430d8b7d2fca1 Mon Sep 17 00:00:00 2001 From: luciferlive112116 <291889058+luciferlive112116@users.noreply.github.com> Date: Thu, 16 Jul 2026 13:31:57 +0800 Subject: [PATCH] feat(mcp): register the maintain REST surface as local stdio tools The maintain CLI subcommands call real REST endpoints that the remote server has exposed as tools since #6087, but the local stdio server registered none of them. An agent on the local server had to shell out to the CLI to list the approval queue, decide a staged action, toggle the kill-switch, set an autonomy level, or read gate precision. Register the five missing counterparts: loopover_list_pending_actions, loopover_decide_pending_action, loopover_set_agent_paused, loopover_set_action_autonomy, and loopover_get_gate_precision. Each calls the endpoint its CLI subcommand already calls, through the same apiGet/apiPost/apiFetch client, so auth, timeouts, and error shaping come from there rather than a second HTTP path. Shapes mirror the remote's, and categories mirror the remote's MCP_TOOL_CATEGORIES entries for the same names, so one caller sees one surface. The remote server is untouched. set_action_autonomy read-merge-writes like `maintain set-level` does: PUT /settings replaces the whole autonomy map, so sending one class alone would silently clear the others. A test pins that the other classes survive, and that the write is a GET-then-PUT. One deliberate divergence from the remote: its list_pending_actions takes an optional `status`, which it can honour because it queries the queue store directly. This server has only GET /agent/pending-actions, which takes no query parameters and hardcodes status "pending" (src/api/routes.ts). Offering the filter here would let a caller ask for "rejected", receive the pending list, and be told it succeeded -- so the schema omits it and the description names the queue it really returns. Tests cover success and an API-failure path for each tool, plus the read-merge-write invariant and pre-flight rejection of unknown action classes and autonomy levels. The alias-retirement suite pins the exact registered-tool count; these five take it from 42 to 47. Closes #6152 --- packages/loopover-mcp/bin/loopover-mcp.js | 154 ++++++++++++++++++++++ test/unit/mcp-cli-maintain-tools.test.ts | 139 +++++++++++++++++++ test/unit/mcp-tool-rename-aliases.test.ts | 13 +- 3 files changed, 300 insertions(+), 6 deletions(-) create mode 100644 test/unit/mcp-cli-maintain-tools.test.ts diff --git a/packages/loopover-mcp/bin/loopover-mcp.js b/packages/loopover-mcp/bin/loopover-mcp.js index 0986eea09c..d86d5cc220 100755 --- a/packages/loopover-mcp/bin/loopover-mcp.js +++ b/packages/loopover-mcp/bin/loopover-mcp.js @@ -457,6 +457,52 @@ const agentRunIdShape = { runId: z.string().min(1), }; +// #6152 maintain-surface tools. Each shape mirrors its already-shipped remote counterpart in src/mcp/server.ts +// (listPendingActionsShape, decidePendingActionShape, setAgentPausedShape, setActionAutonomyShape, +// ownerRepoWindowShape) so the same call works against either server. The `decision` verb is accept|reject -- +// the approval-queue route's own vocabulary (#779) -- rather than the maintain CLI's approve|reject, because a +// tool caller is talking to the route, not to the CLI's surface. +// +// One deliberate divergence: the remote's listPendingActionsShape takes an optional `status`, which it can honour +// because it queries the approval-queue store directly. This server reaches the queue only through +// GET /v1/repos/:owner/:repo/agent/pending-actions, which takes no query parameters and hardcodes status +// "pending" (src/api/routes.ts). Offering a `status` here would let a caller ask for "rejected", get the pending +// list, and be told it succeeded -- so it is left out of the schema and the description names the queue as the +// pending one. An agent picks its arguments from the published schema, so a filter that isn't there is one it +// won't ask for; a key sent anyway is dropped by the MCP layer before this handler and never reaches the URL. +const listPendingActionsShape = { + owner: z.string().min(1), + repo: z.string().min(1), +}; + +const decidePendingActionShape = { + owner: z.string().min(1), + repo: z.string().min(1), + id: z.string().min(1), + decision: z.enum(["accept", "reject"]), +}; + +const setAgentPausedShape = { + owner: z.string().min(1), + repo: z.string().min(1), + paused: z.boolean(), +}; + +// Reuses the CLI's own constants, so `maintain set-level`'s validation and this tool's schema can never disagree +// about what the server accepts. +const setActionAutonomyShape = { + owner: z.string().min(1), + repo: z.string().min(1), + action: z.enum(MAINTAIN_ACTION_CLASSES), + level: z.enum(MAINTAIN_AUTONOMY_LEVELS), +}; + +const gatePrecisionShape = { + owner: z.string().min(1), + repo: z.string().min(1), + windowDays: z.number().int().positive().optional(), +}; + // Single source of truth for stdio tool name + one-line description (#2233). // Registration and `loopover-mcp tools` both read this list. const STDIO_TOOL_DESCRIPTORS = [ @@ -672,6 +718,34 @@ const STDIO_TOOL_DESCRIPTORS = [ category: "maintainer", description: "Return the skipped-PR audit trail: pull requests LoopOver's automated reviewer intentionally stayed quiet on, each with a reason code and a remediation hint. Optionally filter by repoFullName, reason, or since. Maintainer-authenticated; read-only measurement, not a moderation or override action.", }, + // #6152 — the maintain CLI's REST surface, exposed as tools so an agent can drive it without shelling out. + // Categories mirror the remote server's MCP_TOOL_CATEGORIES entries for the same names, so a caller sees one + // consistent grouping across both surfaces. + { + name: "loopover_list_pending_actions", + category: "agent", + description: "List the agent actions currently staged and awaiting a decision in a repo's approval queue, so a maintainer can review what is pending. Returns the pending queue only — the same list as `loopover-mcp maintain queue`. Maintainer access required.", + }, + { + name: "loopover_decide_pending_action", + category: "agent", + description: "Accept (execute) or reject a staged approval-queue action by id. Accept runs it through the live executor gates; reject cancels it. Scoped to this repo, same as `loopover-mcp maintain approve|reject `. Maintainer access required.", + }, + { + name: "loopover_set_agent_paused", + category: "agent", + description: "Pause or resume ALL agent actions on a repo (the kill-switch toggle), same as `loopover-mcp maintain pause|resume`. Maintainer access required.", + }, + { + name: "loopover_set_action_autonomy", + category: "agent", + description: "Set the autonomy level for one action class via a read-merge-write, so the other classes are left untouched. Same as `loopover-mcp maintain set-level `. Maintainer access required.", + }, + { + name: "loopover_get_gate_precision", + 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.", + }, ]; // #6301 — coarse tool categories for grouping `loopover-mcp tools` output. Ordered @@ -1444,6 +1518,86 @@ registerStdioTool( }, ); +// ── #6152 maintain surface: the REST calls maintainCli already makes, exposed as tools ─────────────────────── +// +// These five mirror remote tools that have existed since #6087 but were never registered locally, so an agent on +// the stdio server had to shell out to the `maintain` CLI to reach them. Each one calls the same endpoint its +// CLI subcommand calls, through the same apiGet/apiPost/apiFetch client (auth, timeouts, and error shaping come +// from there) -- no new HTTP paths, and no behaviour the CLI doesn't already have. + +/** `/v1/repos/:owner/:repo` for a tool's owner+repo input, matching maintainCli's own repoBase. */ +function toolRepoBase(owner, repo) { + return `/v1/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}`; +} + +registerStdioTool( + "loopover_list_pending_actions", + { + description: stdioToolDescription("loopover_list_pending_actions"), + inputSchema: listPendingActionsShape, + }, + async ({ owner, repo }) => { + const payload = await apiGet(`${toolRepoBase(owner, repo)}/agent/pending-actions`); + return toolResult(`Agent approval queue for ${owner}/${repo}: ${(payload.pendingActions ?? []).length} pending.`, payload); + }, +); + +registerStdioTool( + "loopover_decide_pending_action", + { + description: stdioToolDescription("loopover_decide_pending_action"), + inputSchema: decidePendingActionShape, + }, + async ({ owner, repo, id, decision }) => { + const payload = await apiPost(`${toolRepoBase(owner, repo)}/agent/pending-actions/${encodeURIComponent(id)}/${decision}`, {}); + return toolResult(`${decision === "accept" ? "Accepted" : "Rejected"} ${id}: ${payload.status ?? "ok"}.`, payload); + }, +); + +registerStdioTool( + "loopover_set_agent_paused", + { + description: stdioToolDescription("loopover_set_agent_paused"), + inputSchema: setAgentPausedShape, + }, + async ({ owner, repo, paused }) => { + const payload = await apiFetch(`${toolRepoBase(owner, repo)}/settings`, { method: "PUT", body: JSON.stringify({ agentPaused: paused }) }); + return toolResult(`Agent actions ${paused ? "paused" : "resumed"} for ${owner}/${repo}.`, payload); + }, +); + +registerStdioTool( + "loopover_set_action_autonomy", + { + description: stdioToolDescription("loopover_set_action_autonomy"), + inputSchema: setActionAutonomyShape, + }, + async ({ owner, repo, action, level }) => { + // Read-merge-write, exactly as `maintain set-level` does it: PUT /settings replaces the whole autonomy map, + // so sending only this class would silently clear every other one. + const base = toolRepoBase(owner, repo); + const current = await apiGet(`${base}/settings`); + const autonomy = { ...(current.autonomy ?? {}), [action]: level }; + const payload = await apiFetch(`${base}/settings`, { method: "PUT", body: JSON.stringify({ autonomy }) }); + return toolResult(`Set ${action} autonomy to ${level} for ${owner}/${repo}.`, payload); + }, +); + +registerStdioTool( + "loopover_get_gate_precision", + { + description: stdioToolDescription("loopover_get_gate_precision"), + inputSchema: gatePrecisionShape, + }, + async ({ owner, repo, windowDays }) => { + // The schema already rejects a non-positive windowDays, so an omitted window is the only way to full history + // -- matching the route's own behaviour when ?windowDays is absent. + const query = windowDays ? `?windowDays=${encodeURIComponent(windowDays)}` : ""; + const payload = await apiGet(`${toolRepoBase(owner, repo)}/gate-precision${query}`); + return toolResult(`Gate precision for ${owner}/${repo}.`, payload); + }, +); + // ── Resources: decision-pack, doctor, compatibility, changelog (#292) ───────── server.registerResource( diff --git a/test/unit/mcp-cli-maintain-tools.test.ts b/test/unit/mcp-cli-maintain-tools.test.ts new file mode 100644 index 0000000000..7bb9450726 --- /dev/null +++ b/test/unit/mcp-cli-maintain-tools.test.ts @@ -0,0 +1,139 @@ +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, describe, expect, it } from "vitest"; +import { closeFixtureServer, run, startFixtureServer } from "./support/mcp-cli-harness"; + +const bin = join(process.cwd(), "packages/loopover-mcp/bin/loopover-mcp.js"); + +// #6152: the maintain CLI's REST surface, exposed as stdio tools. These assert the proxy contract -- that each +// tool reaches the endpoint its CLI subcommand already calls, with the same method and body -- rather than +// re-testing the endpoints themselves, which test/unit/mcp-cli-maintain.test.ts already covers via the CLI. +let client: Client | null = null; +let transport: StdioClientTransport | null = null; +let configDir: string | null = null; +let capturedRequests: Array<{ url: string; method: string }>; + +async function connect() { + configDir = mkdtempSync(join(tmpdir(), "loopover-maintain-tools-")); + capturedRequests = []; + const apiUrl = await startFixtureServer({ + onApiRequest: (request) => { + const url = request.url ?? ""; + if (/pending-actions|settings|gate-precision/.test(url)) capturedRequests.push({ url, method: request.method ?? "GET" }); + }, + }); + 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: "maintain-tools-test", version: "0.0.1" }); + await client.connect(transport); +} + +afterEach(async () => { + await client?.close().catch(() => undefined); + client = null; + transport = null; + await closeFixtureServer(); + if (configDir) rmSync(configDir, { recursive: true, force: true }); + configDir = null; +}); + +const REPO = { owner: "owner", repo: "repo" }; + +/** Every #6152 tool, with an argument set the fixture serves and a field its real payload carries. */ +const MAINTAIN_TOOLS = [ + { name: "loopover_list_pending_actions", args: REPO, contains: "pa-1" }, + { name: "loopover_decide_pending_action", args: { ...REPO, id: "pa-1", decision: "accept" }, contains: "accepted" }, + { name: "loopover_set_agent_paused", args: { ...REPO, paused: true }, contains: "agentPaused" }, + { name: "loopover_set_action_autonomy", args: { ...REPO, action: "merge", level: "auto" }, contains: "autonomy" }, + { name: "loopover_get_gate_precision", args: REPO, contains: "falsePositiveRate" }, +] as const; + +describe("loopover-mcp maintain stdio proxies (#6152)", () => { + it("registers all 5 maintain tools in the stdio server tool list", async () => { + await connect(); + const names = (await client!.listTools()).tools.map((tool) => tool.name); + for (const tool of MAINTAIN_TOOLS) expect(names).toContain(tool.name); + }); + + it("lists all 5 maintain tools via `loopover-mcp tools --json` with non-empty descriptions", async () => { + await connect(); + const payload = JSON.parse(run(["tools", "--json"])) as { tools: Array<{ name: string; description: string; category?: string }> }; + for (const tool of MAINTAIN_TOOLS) { + const entry = payload.tools.find((t) => t.name === tool.name); + expect(entry, `missing descriptor for ${tool.name}`).toBeTruthy(); + expect(entry!.description.trim().length).toBeGreaterThan(0); + } + }); + + for (const tool of MAINTAIN_TOOLS) { + it(`${tool.name} proxies to its REST endpoint and returns the payload`, async () => { + await connect(); + const result = await client!.callTool({ name: tool.name, arguments: { ...tool.args } }); + expect(result.isError).toBeFalsy(); + expect(JSON.stringify(result)).toContain(tool.contains); + expect(capturedRequests.length).toBeGreaterThan(0); + for (const request of capturedRequests) expect(request.url).toContain("/v1/repos/owner/repo/"); + }); + + // The fixture serves owner/repo only and 404s anything else, so an unregistered repo exercises the same + // failure path a real caller hits without maintainer access to the target: an API error, surfaced as a tool + // error rather than a silent empty success. + it(`${tool.name} surfaces an API failure as a tool error`, async () => { + await connect(); + const result = await client!.callTool({ name: tool.name, arguments: { ...tool.args, owner: "nobody", repo: "missing" } }); + expect(result.isError).toBe(true); + expect(JSON.stringify(result.content)).toMatch(/404|not_found/); + }); + } + + // GET /agent/pending-actions takes no query parameters and hardcodes status "pending" (src/api/routes.ts), so + // this server cannot honour the `status` filter its remote counterpart offers. The tool therefore doesn't + // advertise one: an agent reads the published schema to decide what to send, so a filter absent from the schema + // is a filter it won't ask for -- and can't be told "ok" about. (An unknown key sent anyway is dropped by the + // MCP layer before the handler, so it can never reach the URL either.) + it("list_pending_actions advertises no status filter, which this server's route could not honour", async () => { + await connect(); + const tool = (await client!.listTools()).tools.find((entry) => entry.name === "loopover_list_pending_actions"); + expect(tool, "loopover_list_pending_actions is not registered").toBeTruthy(); + expect(Object.keys(tool!.inputSchema.properties ?? {}).sort()).toEqual(["owner", "repo"]); + + const result = await client!.callTool({ name: "loopover_list_pending_actions", arguments: { ...REPO, status: "rejected" } }); + expect(result.isError).toBeFalsy(); + for (const request of capturedRequests) expect(request.url).not.toContain("status="); + }); + + it("set_action_autonomy read-merge-writes so the other action classes survive", async () => { + await connect(); + const result = await client!.callTool({ name: "loopover_set_action_autonomy", arguments: { ...REPO, action: "merge", level: "auto" } }); + expect(result.isError).toBeFalsy(); + // The fixture's stored autonomy is { label: "auto" }; a blind PUT of just `merge` would drop it. + const payload = JSON.stringify(result); + expect(payload).toContain("label"); + expect(payload).toContain("merge"); + expect(capturedRequests.map((request) => request.method)).toEqual(["GET", "PUT"]); + }); + + it("rejects an unknown action class and an unknown autonomy level before any API call", async () => { + await connect(); + for (const args of [ + { ...REPO, action: "bogus", level: "auto" }, + { ...REPO, action: "merge", level: "bogus" }, + ]) { + const result = await client!.callTool({ name: "loopover_set_action_autonomy", arguments: args }); + expect(result.isError).toBe(true); + } + expect(capturedRequests).toEqual([]); + }); +}); diff --git a/test/unit/mcp-tool-rename-aliases.test.ts b/test/unit/mcp-tool-rename-aliases.test.ts index 472c5275fc..3f6f4c3286 100644 --- a/test/unit/mcp-tool-rename-aliases.test.ts +++ b/test/unit/mcp-tool-rename-aliases.test.ts @@ -1,8 +1,9 @@ // #4777: retire every gittensory_-prefixed deprecated alias that #4775 left in place for one -// minor-version deprecation cycle. This suite pins the post-retirement shape: exactly the 42 +// minor-version deprecation cycle. This suite pins the post-retirement shape: exactly the 47 // canonical loopover_-prefixed stdio tools are registered, none of their old gittensory_-prefixed // alias names resolve anymore, no description carries a stale deprecation notice, and the CLI's // `tools --json` listing stays in lockstep with what the live server actually registers. +// (#6152 registered the 5 maintain-surface tools, taking the count from 42 to 47.) import { Client } from "@modelcontextprotocol/sdk/client/index.js"; import { StdioClientTransport } from "@modelcontextprotocol/sdk/client/stdio.js"; import { mkdtempSync, rmSync } from "node:fs"; @@ -46,14 +47,14 @@ describe("MCP legacy alias retirement (#4777) — discovery invariants", () => { }); afterEach(disconnect); - it("lists exactly 42 loopover_ tools and zero gittensory_-prefixed aliases", async () => { + it("lists exactly 47 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(42); + expect(primary.length).toBe(47); expect(legacy.length).toBe(0); - expect(names.length).toBe(42); + expect(names.length).toBe(47); }); it("no loopover_ tool's description carries a stale deprecation notice", async () => { @@ -63,11 +64,11 @@ describe("MCP legacy alias retirement (#4777) — discovery invariants", () => { } }); - it("`loopover-mcp tools --json` reports the same 42-tool count the live server registers", async () => { + it("`loopover-mcp tools --json` reports the same 47-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(42); + expect(payload.count).toBe(47); expect([...payload.tools.map((t) => t.name)].sort()).toEqual([...tools.map((t) => t.name)].sort()); }); });