diff --git a/packages/loopover-mcp/bin/loopover-mcp.ts b/packages/loopover-mcp/bin/loopover-mcp.ts index 3bad6ac30a..a6272ee9fb 100644 --- a/packages/loopover-mcp/bin/loopover-mcp.ts +++ b/packages/loopover-mcp/bin/loopover-mcp.ts @@ -921,6 +921,23 @@ const gatePrecisionShape = { windowDays: z.number().int().positive().optional(), }; +// #7753: loopover_propose_action — the stdio counterpart to the remote tool of the same name +// (src/mcp/server.ts's proposeActionShape) and to `maintain propose` (loopover-mcp.ts's maintainCli, "propose" +// subcommand). #6744 added the route + CLI mirror but never this stdio registration, so it fell outside #6152's +// batch even though it is the same maintain-adjacent family. actionClass reuses PROPOSE_ACTION_CLASSES so this +// schema and `maintain propose`'s own validation can never disagree about what the route accepts. +const proposeActionShape = { + owner: z.string().min(1), + repo: z.string().min(1), + pullNumber: z.number().int().positive(), + actionClass: z.enum(PROPOSE_ACTION_CLASSES), + reason: z.string().max(500).optional(), + label: z.string().min(1).max(100).optional(), + reviewBody: z.string().max(60000).optional(), + mergeMethod: z.enum(["merge", "squash", "rebase"]).optional(), + closeComment: z.string().max(60000).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 = [ @@ -1302,6 +1319,13 @@ 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.", }, + // #7753 — the sixth maintain-surface tool (#6744's route + CLI mirror never got a stdio registration in + // #6152's batch). Category mirrors the remote server's MCP_TOOL_CATEGORIES entry for the same name. + { + name: "loopover_propose_action", + category: "agent", + description: "Stage a PR action (label / request_changes / approve / merge / close) into the repo's approval queue for a maintainer to accept or reject, same as `loopover-mcp maintain propose `. Maintainer access required; the action is NOT executed until approved.", + }, { name: "loopover_open_pr", category: "agent", @@ -2623,6 +2647,31 @@ registerStdioTool( return toolResult(`Gate precision for ${owner}/${repo}.`, payload); }, ); + +// #7753: the sixth maintain-surface tool -- calls the exact endpoint `maintain propose` already calls +// (POST .../agent/pending-actions, see maintainCli's "propose" subcommand above), through the same apiPost +// client, so this adds no new HTTP path. The route always returns a fully-populated `action` (id/actionClass/ +// status set unconditionally, see src/api/routes.ts's POST handler) -- only `created` genuinely varies (false +// when an equivalent action is already staged), so that's the only branch this formats defensively. +registerStdioTool( + "loopover_propose_action", + { + description: stdioToolDescription("loopover_propose_action"), + inputSchema: proposeActionShape, + }, + async ({ owner, repo, pullNumber, actionClass, reason, label, reviewBody, mergeMethod, closeComment }: any) => { + const payload = await apiPost( + `${toolRepoBase(owner, repo)}/agent/pending-actions`, + stripUndefined({ pullNumber, actionClass, reason, label, reviewBody, mergeMethod, closeComment }), + ); + const action = payload.action; + return toolResult( + `${payload.created ? "Staged" : "Already staged"} ${action.actionClass} on ${owner}/${repo}#${pullNumber} (${action.status}), id ${action.id}.`, + 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. diff --git a/test/unit/mcp-cli-propose-action-tool.test.ts b/test/unit/mcp-cli-propose-action-tool.test.ts new file mode 100644 index 0000000000..881547b458 --- /dev/null +++ b/test/unit/mcp-cli-propose-action-tool.test.ts @@ -0,0 +1,129 @@ +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"); + +// #7753: the stdio counterpart to the remote's already-shipped loopover_propose_action (src/mcp/server.ts) and +// to `maintain propose` -- same maintain-adjacent family as #6152's five siblings (test/unit/mcp-cli-maintain- +// tools.test.ts), added later because #6744 shipped the route + CLI mirror without a stdio registration. These +// assert the proxy contract -- that the tool reaches the same bare POST .../agent/pending-actions endpoint +// `maintain propose` already calls, with the same body -- rather than re-testing the endpoint itself, which +// test/unit/mcp-cli-maintain.test.ts's "propose" describe block 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(options: Parameters[0] = {}) { + configDir = mkdtempSync(join(tmpdir(), "loopover-propose-action-tool-")); + capturedRequests = []; + const apiUrl = await startFixtureServer({ + ...options, + onApiRequest: (request) => { + const url = request.url ?? ""; + if (url.includes("pending-actions")) 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: "propose-action-tool-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" }; + +describe("loopover-mcp loopover_propose_action stdio proxy (#7753)", () => { + it("registers loopover_propose_action in the stdio server tool list, with a non-empty description", async () => { + await connect(); + const tools = (await client!.listTools()).tools; + const tool = tools.find((entry) => entry.name === "loopover_propose_action"); + expect(tool, "loopover_propose_action is not registered").toBeTruthy(); + expect(tool!.description?.trim().length ?? 0).toBeGreaterThan(0); + }); + + it("lists loopover_propose_action via `loopover-mcp tools --json` with the same description the server carries", async () => { + await connect(); + const wireDescription = (await client!.listTools()).tools.find((entry) => entry.name === "loopover_propose_action")!.description; + const payload = JSON.parse(run(["tools", "--json"])) as { tools: Array<{ name: string; description: string }> }; + const entry = payload.tools.find((t) => t.name === "loopover_propose_action"); + expect(entry, "missing descriptor for loopover_propose_action").toBeTruthy(); + expect(entry!.description).toBe(wireDescription); + }); + + it("proxies to the bare POST .../agent/pending-actions endpoint `maintain propose` already calls, forwarding every field", async () => { + await connect(); + const result = await client!.callTool({ + name: "loopover_propose_action", + arguments: { ...REPO, pullNumber: 7, actionClass: "merge", reason: "needs a look", label: "priority", reviewBody: "lgtm", mergeMethod: "squash", closeComment: "n/a" }, + }); + expect(result.isError).toBeFalsy(); + expect(JSON.stringify(result)).toContain("pa-1"); + expect(capturedRequests).toHaveLength(1); + expect(capturedRequests[0]!.url).toBe("/v1/repos/owner/repo/agent/pending-actions"); + expect(capturedRequests[0]!.method).toBe("POST"); + }); + + it("reports 'Staged' when the route creates a new action", async () => { + await connect({ proposeActionCreated: true }); + const result = await client!.callTool({ name: "loopover_propose_action", arguments: { ...REPO, pullNumber: 7, actionClass: "merge" } }); + expect(result.isError).toBeFalsy(); + const text = (result.content as Array<{ type: string; text?: string }>).find((block) => block.type === "text")?.text ?? ""; + expect(text).toMatch(/^Staged /); + }); + + it("reports 'Already staged' when an equivalent action is already queued (created: false)", async () => { + await connect({ proposeActionCreated: false }); + const result = await client!.callTool({ name: "loopover_propose_action", arguments: { ...REPO, pullNumber: 7, actionClass: "merge" } }); + expect(result.isError).toBeFalsy(); + const text = (result.content as Array<{ type: string; text?: string }>).find((block) => block.type === "text")?.text ?? ""; + expect(text).toMatch(/^Already staged /); + }); + + // 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 -- same contract #6152's siblings assert in mcp-cli-maintain- + // tools.test.ts. + it("surfaces an API failure as a tool error", async () => { + await connect(); + const result = await client!.callTool({ name: "loopover_propose_action", arguments: { owner: "nobody", repo: "missing", pullNumber: 7, actionClass: "merge" } }); + expect(result.isError).toBe(true); + expect(JSON.stringify(result.content)).toMatch(/404|not_found/); + }); + + it("rejects an unknown action class before any API call", async () => { + await connect(); + const result = await client!.callTool({ name: "loopover_propose_action", arguments: { ...REPO, pullNumber: 7, actionClass: "bogus" } }); + expect(result.isError).toBe(true); + expect(capturedRequests).toEqual([]); + }); + + it("rejects a non-positive pull number before any API call", async () => { + await connect(); + const result = await client!.callTool({ name: "loopover_propose_action", arguments: { ...REPO, pullNumber: 0, actionClass: "merge" } }); + 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 5088d983c7..3a4bb9c889 100644 --- a/test/unit/mcp-tool-rename-aliases.test.ts +++ b/test/unit/mcp-tool-rename-aliases.test.ts @@ -22,6 +22,7 @@ // (#6747 registered the loopover_pr_outcome CLI mirror, taking the count from 77 to 78.) // (#6980 registered the loopover_explain_review_risk CLI mirror, taking the count from 78 to 79.) // (#7758 registered the loopover_get_outcome_calibration stdio tool, taking the count from 79 to 80.) +// (#7753 registered the loopover_propose_action stdio mirror -- same maintain-adjacent family #6152 batched, taking the count from 80 to 81.) import { Client } from "@modelcontextprotocol/sdk/client/index.js"; import { StdioClientTransport } from "@modelcontextprotocol/sdk/client/stdio.js"; import { mkdtempSync, rmSync } from "node:fs"; @@ -69,14 +70,14 @@ describe("MCP legacy alias retirement (#4777) — discovery invariants", () => { }); afterEach(disconnect); - it("lists exactly 80 loopover_ tools and zero gittensory_-prefixed aliases", async () => { + it("lists exactly 81 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(80); + expect(primary.length).toBe(81); expect(legacy.length).toBe(0); - expect(names.length).toBe(80); + expect(names.length).toBe(81); }); it("no loopover_ tool's description carries a stale deprecation notice", async () => { @@ -88,14 +89,14 @@ describe("MCP legacy alias retirement (#4777) — discovery invariants", () => { } }); - it("`loopover-mcp tools --json` reports the same 80-tool count the live server registers", async () => { + it("`loopover-mcp tools --json` reports the same 81-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(80); + expect(payload.count).toBe(81); expect([...payload.tools.map((t) => t.name)].sort()).toEqual( [...tools.map((t) => t.name)].sort(), ); diff --git a/test/unit/support/mcp-cli-harness.ts b/test/unit/support/mcp-cli-harness.ts index 2f26c988c5..ed401bd596 100644 --- a/test/unit/support/mcp-cli-harness.ts +++ b/test/unit/support/mcp-cli-harness.ts @@ -204,6 +204,9 @@ export async function startFixtureServer( /** #6743: overrides the repo-doc refresh route's default "opened a new PR" response, e.g. to exercise * the reused-PR or not-opened branches. */ repoDocRefresh?: unknown; + /** #7753: overrides the propose (POST bare pending-actions) route's default `created: true` response, + * e.g. to exercise the "already staged" (created: false) branch. */ + proposeActionCreated?: boolean; /** #6792: queued /v1/auth/github/device/poll responses, consumed one per request -- the last entry * repeats once exhausted. Lets a test simulate a transient 429 (or GitHub's own slow_down/pending * statuses) before the device flow eventually resolves. Requires deviceFlowStart to be set too. */ @@ -535,7 +538,12 @@ export async function startFixtureServer( if (request.url === "/v1/repos/owner/repo/agent/pending-actions" && request.method === "POST") { const body = (await readJsonRequest(request)) as { pullNumber?: number; actionClass?: string; reason?: string | null }; const action = { id: "pa-1", actionClass: body.actionClass ?? "merge", pullNumber: body.pullNumber ?? 7, status: "pending", reason: body.reason ?? null }; - response.end(JSON.stringify({ created: true, action: options.terminalInjection ? { ...action, actionClass: options.terminalInjection } : action })); + response.end( + JSON.stringify({ + created: options.proposeActionCreated ?? true, + action: options.terminalInjection ? { ...action, actionClass: options.terminalInjection } : action, + }), + ); return; } if (request.url === "/v1/repos/owner/repo/maintainer-noise" && request.method === "GET") {