Skip to content

Commit dd7f7ea

Browse files
feat(mcp): register loopover_propose_action as a local stdio MCP tool
Closes #7753 Mirrors the exact registerStdioTool pattern PR #6382 used for the 5 maintain-surface siblings (loopover_list_pending_actions et al): the handler calls the same bare POST .../agent/pending-actions endpoint `maintain propose` already calls, through the same apiPost client, and its description comes from the same stdioToolDescription centralized lookup. #6744 added the route + CLI mirror without a stdio registration, so it fell outside #6152's batch despite being the same family. The route's response always carries a fully-populated `action` (id/actionClass/status set unconditionally, per src/api/routes.ts's POST handler) -- only `created` genuinely varies, so that's the only branch the handler formats defensively. New dedicated suite (mcp-cli-propose-action-tool.test.ts) covers registration, the proxy contract, both the "Staged"/"Already staged" branches, an API-failure path, and pre-flight schema rejection -- following mcp-cli-maintain- tools.test.ts's shape. Bumped the pinned stdio tool count 80 -> 81 in mcp-tool-rename-aliases.test.ts (rebased past #7877's own 79 -> 80 bump).
1 parent e7e10e7 commit dd7f7ea

4 files changed

Lines changed: 193 additions & 6 deletions

File tree

packages/loopover-mcp/bin/loopover-mcp.ts

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -921,6 +921,23 @@ const gatePrecisionShape = {
921921
windowDays: z.number().int().positive().optional(),
922922
};
923923

924+
// #7753: loopover_propose_action — the stdio counterpart to the remote tool of the same name
925+
// (src/mcp/server.ts's proposeActionShape) and to `maintain propose` (loopover-mcp.ts's maintainCli, "propose"
926+
// subcommand). #6744 added the route + CLI mirror but never this stdio registration, so it fell outside #6152's
927+
// batch even though it is the same maintain-adjacent family. actionClass reuses PROPOSE_ACTION_CLASSES so this
928+
// schema and `maintain propose`'s own validation can never disagree about what the route accepts.
929+
const proposeActionShape = {
930+
owner: z.string().min(1),
931+
repo: z.string().min(1),
932+
pullNumber: z.number().int().positive(),
933+
actionClass: z.enum(PROPOSE_ACTION_CLASSES),
934+
reason: z.string().max(500).optional(),
935+
label: z.string().min(1).max(100).optional(),
936+
reviewBody: z.string().max(60000).optional(),
937+
mergeMethod: z.enum(["merge", "squash", "rebase"]).optional(),
938+
closeComment: z.string().max(60000).optional(),
939+
};
940+
924941
// Single source of truth for stdio tool name + one-line description (#2233).
925942
// Registration and `loopover-mcp tools` both read this list.
926943
const STDIO_TOOL_DESCRIPTORS = [
@@ -1302,6 +1319,13 @@ const STDIO_TOOL_DESCRIPTORS = [
13021319
category: "maintainer",
13031320
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.",
13041321
},
1322+
// #7753 — the sixth maintain-surface tool (#6744's route + CLI mirror never got a stdio registration in
1323+
// #6152's batch). Category mirrors the remote server's MCP_TOOL_CATEGORIES entry for the same name.
1324+
{
1325+
name: "loopover_propose_action",
1326+
category: "agent",
1327+
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 <action-class> <pull-number>`. Maintainer access required; the action is NOT executed until approved.",
1328+
},
13051329
{
13061330
name: "loopover_open_pr",
13071331
category: "agent",
@@ -2623,6 +2647,31 @@ registerStdioTool(
26232647
return toolResult(`Gate precision for ${owner}/${repo}.`, payload);
26242648
},
26252649
);
2650+
2651+
// #7753: the sixth maintain-surface tool -- calls the exact endpoint `maintain propose` already calls
2652+
// (POST .../agent/pending-actions, see maintainCli's "propose" subcommand above), through the same apiPost
2653+
// client, so this adds no new HTTP path. The route always returns a fully-populated `action` (id/actionClass/
2654+
// status set unconditionally, see src/api/routes.ts's POST handler) -- only `created` genuinely varies (false
2655+
// when an equivalent action is already staged), so that's the only branch this formats defensively.
2656+
registerStdioTool(
2657+
"loopover_propose_action",
2658+
{
2659+
description: stdioToolDescription("loopover_propose_action"),
2660+
inputSchema: proposeActionShape,
2661+
},
2662+
async ({ owner, repo, pullNumber, actionClass, reason, label, reviewBody, mergeMethod, closeComment }: any) => {
2663+
const payload = await apiPost(
2664+
`${toolRepoBase(owner, repo)}/agent/pending-actions`,
2665+
stripUndefined({ pullNumber, actionClass, reason, label, reviewBody, mergeMethod, closeComment }),
2666+
);
2667+
const action = payload.action;
2668+
return toolResult(
2669+
`${payload.created ? "Staged" : "Already staged"} ${action.actionClass} on ${owner}/${repo}#${pullNumber} (${action.status}), id ${action.id}.`,
2670+
payload,
2671+
);
2672+
},
2673+
);
2674+
26262675
// ── Write-tools (#6149): pure LOCAL-execution spec builders. loopover NEVER performs the write -- each tool
26272676
// returns a spec the caller runs with its OWN gh creds. Brings the local stdio server to parity with the
26282677
// miner-auto-dev profile's recommendedTools, using the same @loopover/engine builders as the remote server.
Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
import { Client } from "@modelcontextprotocol/sdk/client/index.js";
2+
import { StdioClientTransport } from "@modelcontextprotocol/sdk/client/stdio.js";
3+
import { mkdtempSync, rmSync } from "node:fs";
4+
import { tmpdir } from "node:os";
5+
import { join } from "node:path";
6+
import { afterEach, describe, expect, it } from "vitest";
7+
import { closeFixtureServer, run, startFixtureServer } from "./support/mcp-cli-harness";
8+
9+
const bin = join(process.cwd(), "packages/loopover-mcp/bin/loopover-mcp.js");
10+
11+
// #7753: the stdio counterpart to the remote's already-shipped loopover_propose_action (src/mcp/server.ts) and
12+
// to `maintain propose` -- same maintain-adjacent family as #6152's five siblings (test/unit/mcp-cli-maintain-
13+
// tools.test.ts), added later because #6744 shipped the route + CLI mirror without a stdio registration. These
14+
// assert the proxy contract -- that the tool reaches the same bare POST .../agent/pending-actions endpoint
15+
// `maintain propose` already calls, with the same body -- rather than re-testing the endpoint itself, which
16+
// test/unit/mcp-cli-maintain.test.ts's "propose" describe block already covers via the CLI.
17+
let client: Client | null = null;
18+
let transport: StdioClientTransport | null = null;
19+
let configDir: string | null = null;
20+
let capturedRequests: Array<{ url: string; method: string }>;
21+
22+
async function connect(options: Parameters<typeof startFixtureServer>[0] = {}) {
23+
configDir = mkdtempSync(join(tmpdir(), "loopover-propose-action-tool-"));
24+
capturedRequests = [];
25+
const apiUrl = await startFixtureServer({
26+
...options,
27+
onApiRequest: (request) => {
28+
const url = request.url ?? "";
29+
if (url.includes("pending-actions")) capturedRequests.push({ url, method: request.method ?? "GET" });
30+
},
31+
});
32+
transport = new StdioClientTransport({
33+
command: "node",
34+
args: [bin, "--stdio"],
35+
env: {
36+
...process.env,
37+
LOOPOVER_CONFIG_DIR: configDir,
38+
LOOPOVER_API_URL: apiUrl,
39+
LOOPOVER_TOKEN: "session-token",
40+
LOOPOVER_API_TIMEOUT_MS: "5000",
41+
},
42+
});
43+
client = new Client({ name: "propose-action-tool-test", version: "0.0.1" });
44+
await client.connect(transport);
45+
}
46+
47+
afterEach(async () => {
48+
await client?.close().catch(() => undefined);
49+
client = null;
50+
transport = null;
51+
await closeFixtureServer();
52+
if (configDir) rmSync(configDir, { recursive: true, force: true });
53+
configDir = null;
54+
});
55+
56+
const REPO = { owner: "owner", repo: "repo" };
57+
58+
describe("loopover-mcp loopover_propose_action stdio proxy (#7753)", () => {
59+
it("registers loopover_propose_action in the stdio server tool list, with a non-empty description", async () => {
60+
await connect();
61+
const tools = (await client!.listTools()).tools;
62+
const tool = tools.find((entry) => entry.name === "loopover_propose_action");
63+
expect(tool, "loopover_propose_action is not registered").toBeTruthy();
64+
expect(tool!.description?.trim().length ?? 0).toBeGreaterThan(0);
65+
});
66+
67+
it("lists loopover_propose_action via `loopover-mcp tools --json` with the same description the server carries", async () => {
68+
await connect();
69+
const wireDescription = (await client!.listTools()).tools.find((entry) => entry.name === "loopover_propose_action")!.description;
70+
const payload = JSON.parse(run(["tools", "--json"])) as { tools: Array<{ name: string; description: string }> };
71+
const entry = payload.tools.find((t) => t.name === "loopover_propose_action");
72+
expect(entry, "missing descriptor for loopover_propose_action").toBeTruthy();
73+
expect(entry!.description).toBe(wireDescription);
74+
});
75+
76+
it("proxies to the bare POST .../agent/pending-actions endpoint `maintain propose` already calls, forwarding every field", async () => {
77+
await connect();
78+
const result = await client!.callTool({
79+
name: "loopover_propose_action",
80+
arguments: { ...REPO, pullNumber: 7, actionClass: "merge", reason: "needs a look", label: "priority", reviewBody: "lgtm", mergeMethod: "squash", closeComment: "n/a" },
81+
});
82+
expect(result.isError).toBeFalsy();
83+
expect(JSON.stringify(result)).toContain("pa-1");
84+
expect(capturedRequests).toHaveLength(1);
85+
expect(capturedRequests[0]!.url).toBe("/v1/repos/owner/repo/agent/pending-actions");
86+
expect(capturedRequests[0]!.method).toBe("POST");
87+
});
88+
89+
it("reports 'Staged' when the route creates a new action", async () => {
90+
await connect({ proposeActionCreated: true });
91+
const result = await client!.callTool({ name: "loopover_propose_action", arguments: { ...REPO, pullNumber: 7, actionClass: "merge" } });
92+
expect(result.isError).toBeFalsy();
93+
const text = (result.content as Array<{ type: string; text?: string }>).find((block) => block.type === "text")?.text ?? "";
94+
expect(text).toMatch(/^Staged /);
95+
});
96+
97+
it("reports 'Already staged' when an equivalent action is already queued (created: false)", async () => {
98+
await connect({ proposeActionCreated: false });
99+
const result = await client!.callTool({ name: "loopover_propose_action", arguments: { ...REPO, pullNumber: 7, actionClass: "merge" } });
100+
expect(result.isError).toBeFalsy();
101+
const text = (result.content as Array<{ type: string; text?: string }>).find((block) => block.type === "text")?.text ?? "";
102+
expect(text).toMatch(/^Already staged /);
103+
});
104+
105+
// The fixture serves owner/repo only and 404s anything else, so an unregistered repo exercises the same
106+
// failure path a real caller hits without maintainer access to the target: an API error, surfaced as a tool
107+
// error rather than a silent empty success -- same contract #6152's siblings assert in mcp-cli-maintain-
108+
// tools.test.ts.
109+
it("surfaces an API failure as a tool error", async () => {
110+
await connect();
111+
const result = await client!.callTool({ name: "loopover_propose_action", arguments: { owner: "nobody", repo: "missing", pullNumber: 7, actionClass: "merge" } });
112+
expect(result.isError).toBe(true);
113+
expect(JSON.stringify(result.content)).toMatch(/404|not_found/);
114+
});
115+
116+
it("rejects an unknown action class before any API call", async () => {
117+
await connect();
118+
const result = await client!.callTool({ name: "loopover_propose_action", arguments: { ...REPO, pullNumber: 7, actionClass: "bogus" } });
119+
expect(result.isError).toBe(true);
120+
expect(capturedRequests).toEqual([]);
121+
});
122+
123+
it("rejects a non-positive pull number before any API call", async () => {
124+
await connect();
125+
const result = await client!.callTool({ name: "loopover_propose_action", arguments: { ...REPO, pullNumber: 0, actionClass: "merge" } });
126+
expect(result.isError).toBe(true);
127+
expect(capturedRequests).toEqual([]);
128+
});
129+
});

test/unit/mcp-tool-rename-aliases.test.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
// (#6747 registered the loopover_pr_outcome CLI mirror, taking the count from 77 to 78.)
2323
// (#6980 registered the loopover_explain_review_risk CLI mirror, taking the count from 78 to 79.)
2424
// (#7758 registered the loopover_get_outcome_calibration stdio tool, taking the count from 79 to 80.)
25+
// (#7753 registered the loopover_propose_action stdio mirror -- same maintain-adjacent family #6152 batched, taking the count from 80 to 81.)
2526
import { Client } from "@modelcontextprotocol/sdk/client/index.js";
2627
import { StdioClientTransport } from "@modelcontextprotocol/sdk/client/stdio.js";
2728
import { mkdtempSync, rmSync } from "node:fs";
@@ -69,14 +70,14 @@ describe("MCP legacy alias retirement (#4777) — discovery invariants", () => {
6970
});
7071
afterEach(disconnect);
7172

72-
it("lists exactly 80 loopover_ tools and zero gittensory_-prefixed aliases", async () => {
73+
it("lists exactly 81 loopover_ tools and zero gittensory_-prefixed aliases", async () => {
7374
const { tools } = await client.listTools();
7475
const names = tools.map((t) => t.name);
7576
const primary = names.filter((n) => n.startsWith("loopover_"));
7677
const legacy = names.filter((n) => n.startsWith("gittensory_"));
77-
expect(primary.length).toBe(80);
78+
expect(primary.length).toBe(81);
7879
expect(legacy.length).toBe(0);
79-
expect(names.length).toBe(80);
80+
expect(names.length).toBe(81);
8081
});
8182

8283
it("no loopover_ tool's description carries a stale deprecation notice", async () => {
@@ -88,14 +89,14 @@ describe("MCP legacy alias retirement (#4777) — discovery invariants", () => {
8889
}
8990
});
9091

91-
it("`loopover-mcp tools --json` reports the same 80-tool count the live server registers", async () => {
92+
it("`loopover-mcp tools --json` reports the same 81-tool count the live server registers", async () => {
9293
const { tools } = await client.listTools();
9394
const payload = JSON.parse(run(["tools", "--json"])) as {
9495
count: number;
9596
tools: Array<{ name: string }>;
9697
};
9798
expect(payload.count).toBe(tools.length);
98-
expect(payload.count).toBe(80);
99+
expect(payload.count).toBe(81);
99100
expect([...payload.tools.map((t) => t.name)].sort()).toEqual(
100101
[...tools.map((t) => t.name)].sort(),
101102
);

test/unit/support/mcp-cli-harness.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,9 @@ export async function startFixtureServer(
204204
/** #6743: overrides the repo-doc refresh route's default "opened a new PR" response, e.g. to exercise
205205
* the reused-PR or not-opened branches. */
206206
repoDocRefresh?: unknown;
207+
/** #7753: overrides the propose (POST bare pending-actions) route's default `created: true` response,
208+
* e.g. to exercise the "already staged" (created: false) branch. */
209+
proposeActionCreated?: boolean;
207210
/** #6792: queued /v1/auth/github/device/poll responses, consumed one per request -- the last entry
208211
* repeats once exhausted. Lets a test simulate a transient 429 (or GitHub's own slow_down/pending
209212
* statuses) before the device flow eventually resolves. Requires deviceFlowStart to be set too. */
@@ -535,7 +538,12 @@ export async function startFixtureServer(
535538
if (request.url === "/v1/repos/owner/repo/agent/pending-actions" && request.method === "POST") {
536539
const body = (await readJsonRequest(request)) as { pullNumber?: number; actionClass?: string; reason?: string | null };
537540
const action = { id: "pa-1", actionClass: body.actionClass ?? "merge", pullNumber: body.pullNumber ?? 7, status: "pending", reason: body.reason ?? null };
538-
response.end(JSON.stringify({ created: true, action: options.terminalInjection ? { ...action, actionClass: options.terminalInjection } : action }));
541+
response.end(
542+
JSON.stringify({
543+
created: options.proposeActionCreated ?? true,
544+
action: options.terminalInjection ? { ...action, actionClass: options.terminalInjection } : action,
545+
}),
546+
);
539547
return;
540548
}
541549
if (request.url === "/v1/repos/owner/repo/maintainer-noise" && request.method === "GET") {

0 commit comments

Comments
 (0)