Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 49 additions & 0 deletions packages/loopover-mcp/bin/loopover-mcp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand Down Expand Up @@ -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 <action-class> <pull-number>`. Maintainer access required; the action is NOT executed until approved.",
},
{
name: "loopover_open_pr",
category: "agent",
Expand Down Expand Up @@ -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.
Expand Down
129 changes: 129 additions & 0 deletions test/unit/mcp-cli-propose-action-tool.test.ts
Original file line number Diff line number Diff line change
@@ -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<typeof startFixtureServer>[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([]);
});
});
11 changes: 6 additions & 5 deletions test/unit/mcp-tool-rename-aliases.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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 () => {
Expand All @@ -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(),
);
Expand Down
10 changes: 9 additions & 1 deletion test/unit/support/mcp-cli-harness.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down Expand Up @@ -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") {
Expand Down