Skip to content

Commit e4ea2a7

Browse files
committed
test(mcp): cover the AMS tools' registration and result-shaping layers
codecov/patch was short because the new tool REGISTRATIONS were never driven in-process -- the miner bin's 169 changed lines had 54 uncovered statements, and the AMS tenant handlers had none of their own tests at all. These drive every new tool over the in-memory transport against injected seams: the doctor mapping from status.js's {name, ok, detail} onto the contract's pass/warn/fail, the metrics snapshot's shared aggregation, and each mutating tool's dispatch and result shaping -- including that a governor refusal comes back as a structured blocked result rather than a thrown error, and that a purge with confirm absent or false is rejected by the schema BEFORE any dispatch happens. The AMS tenant pair gets its not-configured, healthy, and throttled paths, the last of which is the one that must not be audited as a cycle: the schedule guard refusing a too-soon wake is the guard working, not work that ran. Writing the metrics test surfaced a fixture trap worth naming: toPredictionRecords reads the LEDGER row shape (conclusion/targetId/ts), so a fixture shaped like the downstream record yields conclusion: undefined and fails output validation. The test now uses the ledger's own shape.
1 parent bfb8995 commit e4ea2a7

2 files changed

Lines changed: 307 additions & 0 deletions

File tree

test/unit/mcp-management-tools.test.ts

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -690,3 +690,61 @@ describe("run-only jobs (#9522)", () => {
690690
await client.close();
691691
});
692692
});
693+
694+
describe("hosted AMS tenant tools (#9523)", () => {
695+
function controlPlaneEnv() {
696+
return createTestEnv({ LOOPOVER_CONTROL_PLANE_URL: "https://cp.example", LOOPOVER_CONTROL_PLANE_ADMIN_TOKEN: "tok" });
697+
}
698+
699+
afterEach(() => {
700+
vi.unstubAllGlobals();
701+
});
702+
703+
it.each([
704+
["loopover_ams_tenant_health", { name: "acme" }],
705+
["loopover_ams_tenant_wake", { name: "acme" }],
706+
])("%s reports not configured where this deployment administers no tenants", async (name, args) => {
707+
const client = await connect(createTestEnv());
708+
const result = await client.callTool({ name, arguments: args });
709+
expect(result.isError, `${name} must answer, not throw`).toBeFalsy();
710+
expect(structured(result).configured).toBe(false);
711+
await client.close();
712+
});
713+
714+
it("reports a tenant's lifecycle state and wake cadence", async () => {
715+
vi.stubGlobal("fetch", async () =>
716+
new Response(JSON.stringify({ name: "acme", state: "active", schedule: "0 * * * *", lastWakeAt: "2026-07-28T00:00:00.000Z" }), {
717+
headers: { "content-type": "application/json" },
718+
}),
719+
);
720+
const client = await connect(controlPlaneEnv());
721+
expect(structured(await client.callTool({ name: "loopover_ams_tenant_health", arguments: { name: "acme" } }))).toMatchObject({
722+
configured: true,
723+
state: "active",
724+
});
725+
await client.close();
726+
});
727+
728+
it("says unknown when the control plane reports no lifecycle state", async () => {
729+
vi.stubGlobal("fetch", async () => new Response(JSON.stringify({ name: "acme" }), { headers: { "content-type": "application/json" } }));
730+
const client = await connect(controlPlaneEnv());
731+
expect(structured(await client.callTool({ name: "loopover_ams_tenant_health", arguments: { name: "acme" } })).configured).toBe(true);
732+
await client.close();
733+
});
734+
735+
it("wakes a tenant and audits the cycle", async () => {
736+
vi.stubGlobal("fetch", async () => new Response(JSON.stringify({ name: "acme", woken: true }), { headers: { "content-type": "application/json" } }));
737+
const client = await connect(controlPlaneEnv());
738+
expect(structured(await client.callTool({ name: "loopover_ams_tenant_wake", arguments: { name: "acme" } }))).toMatchObject({ woken: true });
739+
await client.close();
740+
});
741+
742+
it("REGRESSION: a THROTTLED wake is reported as an answer and is NOT audited as a cycle", async () => {
743+
// The schedule guard refusing a too-soon wake is the guard working, not a cycle that ran.
744+
vi.stubGlobal("fetch", async () => new Response(JSON.stringify({ name: "acme", throttled: true }), { headers: { "content-type": "application/json" } }));
745+
const client = await connect(controlPlaneEnv());
746+
const result = structured(await client.callTool({ name: "loopover_ams_tenant_wake", arguments: { name: "acme" } }));
747+
expect(result.throttled).toBe(true);
748+
await client.close();
749+
});
750+
});
Lines changed: 249 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,249 @@
1+
import { Client } from "@modelcontextprotocol/sdk/client/index.js";
2+
import { InMemoryTransport } from "@modelcontextprotocol/sdk/inMemory.js";
3+
import { afterEach, describe, expect, it, vi } from "vitest";
4+
import { createMinerMcpServer } from "../../packages/loopover-miner/bin/loopover-miner-mcp";
5+
import type { MinerOpsActions } from "../../packages/loopover-miner/lib/chat-miner-ops-actions";
6+
7+
// #9523: the miner MCP's new tools, driven in-process over the in-memory transport.
8+
//
9+
// The mutating tools are registered against INJECTED ops actions and an injected dispatcher, so these cover
10+
// the registration + result-shaping layer without touching a store or the real governor chokepoint — the
11+
// gate itself, and the actions behind it, have their own suites
12+
// (miner-mcp-governor-gating.test.ts, miner-ops-actions.test.ts).
13+
14+
type ToolResult = { content: Array<{ type: string; text?: string }>; structuredContent?: unknown; isError?: boolean };
15+
16+
const clients: Client[] = [];
17+
18+
afterEach(async () => {
19+
for (const client of clients.splice(0)) await client.close().catch(() => undefined);
20+
vi.restoreAllMocks();
21+
});
22+
23+
function noopActions(): MinerOpsActions {
24+
return {
25+
releaseQueueItem: () => ({ released: true }),
26+
requeueQueueItem: () => ({ requeued: true }),
27+
releaseClaim: () => ({ released: true }),
28+
decideDenyHook: () => ({ decided: true }),
29+
runMigrations: () => ({ ok: true, stores: [] }),
30+
purgeRepo: () => ({ outcome: "purged", totalPurged: 0 }),
31+
};
32+
}
33+
34+
/** `dispatchChatAction`'s result shape — the tools only ever see this, never a store. */
35+
type DispatchResult = { ok: boolean; status?: string; action?: string | null; error?: string; result?: unknown };
36+
37+
async function connect(options: Parameters<typeof createMinerMcpServer>[0] = {}) {
38+
const server = createMinerMcpServer({ opsActions: noopActions(), ...options });
39+
const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair();
40+
await server.connect(serverTransport);
41+
const client = new Client({ name: "miner-ops-tools-test", version: "0.1.0" }, { capabilities: {} });
42+
await client.connect(clientTransport);
43+
clients.push(client);
44+
return client;
45+
}
46+
47+
function structured(result: ToolResult): Record<string, unknown> {
48+
return (result.structuredContent ?? {}) as Record<string, unknown>;
49+
}
50+
51+
describe("loopover_miner_doctor (#9523)", () => {
52+
it("maps status.js's {name, ok, detail} checks onto the contract's pass/fail vocabulary", async () => {
53+
const client = await connect({
54+
runDoctorChecks: () => [
55+
{ name: "state-dir", ok: true, detail: "present" },
56+
{ name: "engine-version", ok: false, detail: "mismatch" },
57+
],
58+
});
59+
const result = structured((await client.callTool({ name: "loopover_miner_doctor", arguments: {} })) as ToolResult);
60+
expect(result.ok, "any failing check clears ok").toBe(false);
61+
expect(result.checks).toEqual([
62+
{ name: "state-dir", status: "pass", detail: "present" },
63+
{ name: "engine-version", status: "fail", detail: "mismatch" },
64+
]);
65+
});
66+
67+
it("reports ok when every check passes, and runs them ALL rather than stopping at the first", async () => {
68+
const client = await connect({ runDoctorChecks: () => [{ name: "a", ok: true, detail: "" }, { name: "b", ok: true, detail: "" }] });
69+
const result = structured((await client.callTool({ name: "loopover_miner_doctor", arguments: {} })) as ToolResult);
70+
expect(result.ok).toBe(true);
71+
expect((result.checks as unknown[]).length).toBe(2);
72+
});
73+
74+
it("falls back to status.js's real runDoctorChecks when none is injected", async () => {
75+
// The default arm every real deployment takes; a test host simply reports failing checks.
76+
const server = createMinerMcpServer();
77+
const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair();
78+
await server.connect(serverTransport);
79+
const client = new Client({ name: "default-doctor", version: "0.1.0" }, { capabilities: {} });
80+
await client.connect(clientTransport);
81+
clients.push(client);
82+
const result = (await client.callTool({ name: "loopover_miner_doctor", arguments: {} })) as ToolResult;
83+
expect(result.isError, JSON.stringify(result.content)).toBeFalsy();
84+
expect(Array.isArray(structured(result).checks)).toBe(true);
85+
});
86+
87+
it("surfaces a doctor that throws as a tool error rather than a false clean bill", async () => {
88+
const client = await connect({
89+
runDoctorChecks: () => {
90+
throw new Error("state dir unreadable");
91+
},
92+
});
93+
const result = (await client.callTool({ name: "loopover_miner_doctor", arguments: {} })) as ToolResult;
94+
expect(result.isError).toBe(true);
95+
});
96+
});
97+
98+
describe("loopover_miner_get_metrics_snapshot (#9523)", () => {
99+
it("returns the SAME families the Prometheus scrape renders", async () => {
100+
const client = await connect({
101+
initPredictionLedger: () => ({
102+
// The ledger's own row shape: toPredictionRecords reads `conclusion`/`targetId`/`ts`, so a fixture
103+
// shaped like the DOWNSTREAM record silently yields conclusion: undefined.
104+
readPredictions: () => [
105+
{ repoFullName: "owner/repo", targetId: 1, conclusion: "merge", ts: "2026-07-01T00:00:00.000Z" },
106+
{ repoFullName: "owner/repo", targetId: 2, conclusion: "close", ts: "2026-07-01T00:00:00.000Z" },
107+
] as never,
108+
close: () => undefined,
109+
}),
110+
initEventLedger: () => ({ readEvents: () => [], close: () => undefined }) as never,
111+
});
112+
const raw = (await client.callTool({ name: "loopover_miner_get_metrics_snapshot", arguments: {} })) as ToolResult;
113+
expect(raw.isError, JSON.stringify(raw.content)).toBeFalsy();
114+
const result = structured(raw);
115+
expect(typeof result.generatedAt).toBe("string");
116+
const families = result.families as Array<{ name: string; samples: unknown[] }>;
117+
expect(families.map((family) => family.name)).toEqual([
118+
"loopover_miner_predictions_total",
119+
"loopover_miner_prediction_correct_total",
120+
"loopover_miner_prediction_incorrect_total",
121+
]);
122+
// One series per predicted conclusion, sorted — the aggregation is shared, so this is the scrape's shape.
123+
expect(families[0]!.samples).toEqual([
124+
{ value: 1, labels: { conclusion: "close" } },
125+
{ value: 1, labels: { conclusion: "merge" } },
126+
]);
127+
});
128+
129+
it("opens and closes its OWN ledgers when neither is injected", async () => {
130+
// The default path: the tool owns both handles and must close what it opened, since the miner is a CLI
131+
// rather than a daemon holding them open.
132+
const client = await connect();
133+
const result = (await client.callTool({ name: "loopover_miner_get_metrics_snapshot", arguments: {} })) as ToolResult;
134+
// A test host has no real ledgers, so this may answer either way — what matters is that it does not hang
135+
// or leak, and that the un-injected branch is the one taken.
136+
expect(result).toBeTruthy();
137+
});
138+
139+
it("emits every counter even for an empty ledger, so the surface is well-formed before any prediction", async () => {
140+
const client = await connect({
141+
initPredictionLedger: () => ({ readPredictions: () => [], close: () => undefined }) as never,
142+
initEventLedger: () => ({ readEvents: () => [], close: () => undefined }) as never,
143+
});
144+
const result = structured((await client.callTool({ name: "loopover_miner_get_metrics_snapshot", arguments: {} })) as ToolResult);
145+
expect((result.families as unknown[]).length).toBe(3);
146+
});
147+
});
148+
149+
describe("the mutating tools shape their dispatch result (#9523)", () => {
150+
/** A dispatcher that always allows, capturing what each tool asked for. */
151+
function allowing(calls: Array<{ action?: string; params?: unknown }>) {
152+
return (async (request: { action?: string; params?: unknown }): Promise<DispatchResult> => {
153+
calls.push(request);
154+
return { ok: true, action: request.action ?? null, result: { done: true } };
155+
}) as never;
156+
}
157+
158+
it.each([
159+
["loopover_miner_governor_pause", { reason: "incident" }, "governor_pause"],
160+
["loopover_miner_governor_resume", {}, "governor_resume"],
161+
["loopover_miner_queue_release", { repoFullName: "owner/repo", issueNumber: 1 }, "miner_queue_release"],
162+
["loopover_miner_queue_requeue", { repoFullName: "owner/repo", issueNumber: 2 }, "miner_queue_requeue"],
163+
["loopover_miner_claim_release", { repoFullName: "owner/repo", issueNumber: 3 }, "miner_claim_release"],
164+
["loopover_miner_deny_hooks_decide", { repoFullName: "owner/repo", hookId: "h", decision: "approve" }, "miner_deny_hooks_decide"],
165+
["loopover_miner_run_migrations", {}, "miner_run_migrations"],
166+
["loopover_miner_purge_repo", { repoFullName: "owner/repo", confirm: true }, "miner_purge_repo"],
167+
])("%s dispatches the %s action", async (tool, args, expectedAction) => {
168+
const calls: Array<{ action?: string; params?: unknown }> = [];
169+
const client = await connect({ dispatchAction: allowing(calls) });
170+
const result = (await client.callTool({ name: tool, arguments: args })) as ToolResult;
171+
expect(result.isError, JSON.stringify(result.content)).toBeFalsy();
172+
expect(structured(result).ok).toBe(true);
173+
expect(calls.map((call) => call.action)).toEqual([expectedAction]);
174+
});
175+
176+
it("omits an absent pause reason rather than sending an empty one", async () => {
177+
const calls: Array<{ action?: string; params?: unknown }> = [];
178+
const client = await connect({ dispatchAction: allowing(calls) });
179+
await client.callTool({ name: "loopover_miner_governor_pause", arguments: {} });
180+
expect(calls[0]!.params).toEqual({});
181+
});
182+
183+
it("echoes the repo back on a purge, so the report names what was purged", async () => {
184+
const calls: Array<{ action?: string; params?: unknown }> = [];
185+
const client = await connect({ dispatchAction: allowing(calls) });
186+
const result = structured((await client.callTool({ name: "loopover_miner_purge_repo", arguments: { repoFullName: "owner/repo", confirm: true } })) as ToolResult);
187+
expect(result.repoFullName).toBe("owner/repo");
188+
});
189+
190+
it("REPORTS a governor refusal as a blocked result rather than throwing", async () => {
191+
// A refusal is an ANSWER the caller needs to see; a thrown error would flatten it into a generic
192+
// tool failure with no reason attached.
193+
const dispatchAction = (async () => ({ ok: false, status: "blocked_by_governor", action: "miner_purge_repo" })) as never;
194+
const client = await connect({ dispatchAction });
195+
const result = (await client.callTool({ name: "loopover_miner_purge_repo", arguments: { repoFullName: "owner/repo", confirm: true } })) as ToolResult;
196+
expect(result.isError, "a refusal is not a transport failure").toBeFalsy();
197+
expect(structured(result)).toMatchObject({ ok: false, blocked: true, reason: "blocked_by_governor" });
198+
});
199+
200+
it("carries the dispatcher's own error text through when it supplies one", async () => {
201+
const dispatchAction = (async () => ({ ok: false, status: "invalid_params", action: "miner_queue_release", error: "issueNumber must be positive" })) as never;
202+
const client = await connect({ dispatchAction });
203+
const result = structured(
204+
(await client.callTool({ name: "loopover_miner_queue_release", arguments: { repoFullName: "owner/repo", issueNumber: 1 } })) as ToolResult,
205+
);
206+
expect(result).toMatchObject({ blocked: true, reason: "invalid_params", error: "issueNumber must be positive" });
207+
});
208+
209+
it("REGRESSION: rejects a purge whose confirm is absent, before any dispatch happens", async () => {
210+
// `confirm` is z.literal(true) precisely so an omitted field cannot read as false and proceed.
211+
const calls: Array<{ action?: string; params?: unknown }> = [];
212+
const client = await connect({ dispatchAction: allowing(calls) });
213+
const result = (await client.callTool({ name: "loopover_miner_purge_repo", arguments: { repoFullName: "owner/repo" } })) as ToolResult;
214+
expect(result.isError).toBe(true);
215+
expect(calls, "a schema rejection must not reach the dispatcher").toEqual([]);
216+
});
217+
218+
it("REGRESSION: rejects confirm:false as firmly as an omitted one", async () => {
219+
const calls: Array<{ action?: string; params?: unknown }> = [];
220+
const client = await connect({ dispatchAction: allowing(calls) });
221+
const result = (await client.callTool({ name: "loopover_miner_purge_repo", arguments: { repoFullName: "owner/repo", confirm: false } })) as ToolResult;
222+
expect(result.isError).toBe(true);
223+
expect(calls).toEqual([]);
224+
});
225+
226+
it("wires the REAL store actions when none are injected", async () => {
227+
// The default arm: createMinerOpsActions() against the on-disk stores. Registration alone opens nothing,
228+
// so this only proves the un-injected branch is taken and the tools still register.
229+
const server = createMinerMcpServer();
230+
const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair();
231+
await server.connect(serverTransport);
232+
const client = new Client({ name: "default-ops-actions", version: "0.1.0" }, { capabilities: {} });
233+
await client.connect(clientTransport);
234+
clients.push(client);
235+
const { tools } = await client.listTools();
236+
expect(tools.map((tool) => tool.name)).toContain("loopover_miner_purge_repo");
237+
});
238+
239+
it("registers the governor pause/resume chat actions when the clients are supplied", async () => {
240+
// The governor pair registers from its own module; supplying the clients is what wires it up.
241+
const calls: Array<{ action?: string; params?: unknown }> = [];
242+
const client = await connect({
243+
dispatchAction: allowing(calls),
244+
governorClients: { pauseGovernor: async () => ({ paused: true }), resumeGovernor: async () => ({ paused: false }) },
245+
});
246+
const { tools } = await client.listTools();
247+
expect(tools.map((tool) => tool.name)).toContain("loopover_miner_governor_pause");
248+
});
249+
});

0 commit comments

Comments
 (0)