Skip to content

Commit c151b75

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 b3e1bc3 commit c151b75

4 files changed

Lines changed: 194 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
@@ -957,6 +957,23 @@ const planRepoIssuesShape = {
957957
limit: z.number().int().min(1).max(10).optional().default(5),
958958
};
959959

960+
// #7753: loopover_propose_action — the stdio counterpart to the remote tool of the same name
961+
// (src/mcp/server.ts's proposeActionShape) and to `maintain propose` (loopover-mcp.ts's maintainCli, "propose"
962+
// subcommand). #6744 added the route + CLI mirror but never this stdio registration, so it fell outside #6152's
963+
// batch even though it is the same maintain-adjacent family. actionClass reuses PROPOSE_ACTION_CLASSES so this
964+
// schema and `maintain propose`'s own validation can never disagree about what the route accepts.
965+
const proposeActionShape = {
966+
owner: z.string().min(1),
967+
repo: z.string().min(1),
968+
pullNumber: z.number().int().positive(),
969+
actionClass: z.enum(PROPOSE_ACTION_CLASSES),
970+
reason: z.string().max(500).optional(),
971+
label: z.string().min(1).max(100).optional(),
972+
reviewBody: z.string().max(60000).optional(),
973+
mergeMethod: z.enum(["merge", "squash", "rebase"]).optional(),
974+
closeComment: z.string().max(60000).optional(),
975+
};
976+
960977
// Single source of truth for stdio tool name + one-line description (#2233).
961978
// Registration and `loopover-mcp tools` both read this list.
962979
const STDIO_TOOL_DESCRIPTORS = [
@@ -1343,6 +1360,13 @@ const STDIO_TOOL_DESCRIPTORS = [
13431360
category: "maintainer",
13441361
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.",
13451362
},
1363+
// #7753 — the sixth maintain-surface tool (#6744's route + CLI mirror never got a stdio registration in
1364+
// #6152's batch). Category mirrors the remote server's MCP_TOOL_CATEGORIES entry for the same name.
1365+
{
1366+
name: "loopover_propose_action",
1367+
category: "agent",
1368+
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.",
1369+
},
13461370
{
13471371
name: "loopover_plan_repo_issues",
13481372
category: "maintainer",
@@ -2706,6 +2730,31 @@ registerStdioTool(
27062730
);
27072731
},
27082732
);
2733+
2734+
// #7753: the sixth maintain-surface tool -- calls the exact endpoint `maintain propose` already calls
2735+
// (POST .../agent/pending-actions, see maintainCli's "propose" subcommand above), through the same apiPost
2736+
// client, so this adds no new HTTP path. The route always returns a fully-populated `action` (id/actionClass/
2737+
// status set unconditionally, see src/api/routes.ts's POST handler) -- only `created` genuinely varies (false
2738+
// when an equivalent action is already staged), so that's the only branch this formats defensively.
2739+
registerStdioTool(
2740+
"loopover_propose_action",
2741+
{
2742+
description: stdioToolDescription("loopover_propose_action"),
2743+
inputSchema: proposeActionShape,
2744+
},
2745+
async ({ owner, repo, pullNumber, actionClass, reason, label, reviewBody, mergeMethod, closeComment }: any) => {
2746+
const payload = await apiPost(
2747+
`${toolRepoBase(owner, repo)}/agent/pending-actions`,
2748+
stripUndefined({ pullNumber, actionClass, reason, label, reviewBody, mergeMethod, closeComment }),
2749+
);
2750+
const action = payload.action;
2751+
return toolResult(
2752+
`${payload.created ? "Staged" : "Already staged"} ${action.actionClass} on ${owner}/${repo}#${pullNumber} (${action.status}), id ${action.id}.`,
2753+
payload,
2754+
);
2755+
},
2756+
);
2757+
27092758
// ── Write-tools (#6149): pure LOCAL-execution spec builders. loopover NEVER performs the write -- each tool
27102759
// returns a spec the caller runs with its OWN gh creds. Brings the local stdio server to parity with the
27112760
// 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: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@
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.)
2525
// (#7764 registered the loopover_plan_repo_issues stdio + CLI + REST tool, taking the count from 80 to 81.)
26+
// (another concurrent registration landed without bumping this pin, live count became 82.)
27+
// (#7753 registered the loopover_propose_action stdio mirror -- same maintain-adjacent family #6152 batched, taking the count from 82 to 83.)
2628
import { Client } from "@modelcontextprotocol/sdk/client/index.js";
2729
import { StdioClientTransport } from "@modelcontextprotocol/sdk/client/stdio.js";
2830
import { mkdtempSync, rmSync } from "node:fs";
@@ -70,14 +72,14 @@ describe("MCP legacy alias retirement (#4777) — discovery invariants", () => {
7072
});
7173
afterEach(disconnect);
7274

73-
it("lists exactly 81 loopover_ tools and zero gittensory_-prefixed aliases", async () => {
75+
it("lists exactly 83 loopover_ tools and zero gittensory_-prefixed aliases", async () => {
7476
const { tools } = await client.listTools();
7577
const names = tools.map((t) => t.name);
7678
const primary = names.filter((n) => n.startsWith("loopover_"));
7779
const legacy = names.filter((n) => n.startsWith("gittensory_"));
78-
expect(primary.length).toBe(81);
80+
expect(primary.length).toBe(83);
7981
expect(legacy.length).toBe(0);
80-
expect(names.length).toBe(81);
82+
expect(names.length).toBe(83);
8183
});
8284

8385
it("no loopover_ tool's description carries a stale deprecation notice", async () => {
@@ -89,14 +91,14 @@ describe("MCP legacy alias retirement (#4777) — discovery invariants", () => {
8991
}
9092
});
9193

92-
it("`loopover-mcp tools --json` reports the same 81-tool count the live server registers", async () => {
94+
it("`loopover-mcp tools --json` reports the same 83-tool count the live server registers", async () => {
9395
const { tools } = await client.listTools();
9496
const payload = JSON.parse(run(["tools", "--json"])) as {
9597
count: number;
9698
tools: Array<{ name: string }>;
9799
};
98100
expect(payload.count).toBe(tools.length);
99-
expect(payload.count).toBe(81);
101+
expect(payload.count).toBe(83);
100102
expect([...payload.tools.map((t) => t.name)].sort()).toEqual(
101103
[...tools.map((t) => t.name)].sort(),
102104
);

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

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

0 commit comments

Comments
 (0)