Skip to content

Commit f36dc51

Browse files
authored
ams(mcp): the AMS management tool family — governor-gated mutations, doctor, tenant health (#9523) (#9607)
* feat(mcp): add the AMS management tool family behind the governor gate The miner MCP exposed 11 read-only tools while its entire mutating ops surface was CLI-only. This adds 10 tools: a dedicated doctor (split out of status so status stays cheap), a structured metrics snapshot, and eight mutations. Every mutation dispatches through the miner's existing governor-gated chat-action chokepoint -- the same boundary the dashboard's own actions use. The MCP layer never touches a store: it dispatches an action NAME, and the registry structurally refuses any handler not produced by governorGatedHandler(), whose brand is a private symbol a raw function cannot forge. That is what makes "an MCP caller cannot reach a write path the dashboard could not" a property of the code rather than of review discipline, and the structural test asserts it from both ends. Three claims I had to correct against the real code rather than ship as written. The migrate CLI has no dry-run, because applying a migration IS opening the store -- so the tool has no apply flag either, instead of advertising a safety mode that does not exist. The deny-hook store keys proposals by (repo, id), so the decide tool takes the repo rather than scanning every repo's proposals to resolve an id. And purgeRepoAcrossStores is extracted from runPurge so the tool runs the CLI's own purge over the CLI's own target list, rather than a second implementation free to miss a store. collectMinerPredictionMetrics is likewise extracted in the engine: the Prometheus text renderer now formats those families, so the scrape and the JSON snapshot share one aggregation and cannot disagree about what a counter means. validate:mcp earned its keep twice here. It caught the two AMS tenant tools declared but never registered, and it caught loopover_miner_run_migrations/_purge_repo returning fields their output schemas did not declare -- the .shape re-wrap that drops looseObject's catchall, the same -32602 class this epic already fixed once. Both outputs now declare every field their handlers return. AMS tenant create/list/destroy are deliberately absent: #9522's loopover_tenant_* tools are product-parameterized and already serve product "ams", because the control plane's routes are. Only health and wake -- the pair with no ORB counterpart -- are added. The catalog's recorded exclusions (calibration floors, raw run-state set, the one-way kill switch) stay CLI-only, pinned by a test so reversing that decision has to be deliberate. * test(mcp): cover the miner ops actions and the full dispatch path The store operations and the registered handlers were only reachable through the MCP tools, so nothing measured them directly. These drive each action end to end -- action name through the governor gate to the store call -- plus the validator guard clauses for null, primitive, and array params, and the fail-closed paths (flag disabled, unknown action, malformed params never reaching a store). registerMinerOpsChatActions gains the same evaluateGate seam the dashboard's own registerPortfolioQueueChatActions already exposes, so a test can drive the dispatch path without the real chokepoint; production passes none and gets the real one. paramsOf loses its `?? {}`: every action that calls it has a validator requiring an object, and dispatchChatAction runs that validator first, so the fallback had no reachable case. * chore(docs): let #9521's generator own the miner tool table The hand-edited list this branch added is replaced by the generated one now that #9590 has landed -- which is what the issue asked for: the tool tables pick new tools up with no hand-edits. 163 registry entries across the three servers. * 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. * test(engine): cover the prediction-metrics split in the engine's OWN suite codecov reported this file at 35.84% under the `engine` flag while the root vitest suite showed it fully covered. Both were right: @loopover/engine uploads coverage from its own node:test suite (packages/loopover-engine/test/**), and that suite had no test for this file at all — the vitest one lands under `backend` and does not lift the engine flag. So the aggregation and the renderer get tests where the engine actually measures itself, including the property the split exists for: every sample `collectMinerPredictionMetrics` reports appears verbatim in what `renderMinerPredictionMetrics` emits, so the JSON snapshot and the Prometheus scrape cannot report different numbers. Also pins the escaping, which is a correctness rule rather than cosmetics: a conclusion is DATA, so a quote, backslash, or newline inside one must not forge a second series.
1 parent edf0883 commit f36dc51

18 files changed

Lines changed: 1773 additions & 30 deletions

apps/loopover-ui/src/lib/mcp-tool-reference.ts

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,20 @@ export const MCP_TOOL_REFERENCE: readonly McpToolReferenceEntry[] = [
108108
"availability": "both",
109109
"description": "Create a queued copilot-only LoopOver agent run. The agent plans and explains; it does not edit code or open PRs."
110110
},
111+
{
112+
"name": "loopover_ams_tenant_health",
113+
"category": "tenant",
114+
"locality": "remote",
115+
"availability": "cloud",
116+
"description": "Operator only. One hosted AMS tenant's health: lifecycle state, its cron-wake cadence, when it last woke, that cycle's outcome, and container health. Read-only, and scoped server-side to the authenticated tenant — a name outside that scope is refused rather than answered."
117+
},
118+
{
119+
"name": "loopover_ams_tenant_wake",
120+
"category": "tenant",
121+
"locality": "remote",
122+
"availability": "cloud",
123+
"description": "Operator only. Trigger an immediate cycle for one hosted AMS tenant, instead of waiting for its next scheduled wake. Bounded by the SAME per-tenant schedule guards the cron path obeys — a wake too soon after the last one is reported as throttled rather than forced through."
124+
},
111125
{
112126
"name": "loopover_apply_labels",
113127
"category": "agent",
@@ -696,6 +710,27 @@ export const MCP_TOOL_REFERENCE: readonly McpToolReferenceEntry[] = [
696710
"availability": "both",
697711
"description": "Mark a contributor's own delivered notifications as read (clears the badge). Self-scoped; pass `ids` to clear specific notifications or omit to clear all."
698712
},
713+
{
714+
"name": "loopover_miner_claim_release",
715+
"category": "ops",
716+
"locality": "miner",
717+
"availability": "selfhost",
718+
"description": "Release this miner's claim on one issue, so the claim ledger no longer reserves it. Dispatches through the governor-gated chokepoint and is recorded in the event ledger with source=mcp."
719+
},
720+
{
721+
"name": "loopover_miner_deny_hooks_decide",
722+
"category": "ops",
723+
"locality": "miner",
724+
"availability": "selfhost",
725+
"description": "Approve or reject one synthesized deny-hook awaiting review. Approving puts it into force for future runs; rejecting discards it. Dispatches through the governor-gated chokepoint and is recorded in the event ledger with source=mcp."
726+
},
727+
{
728+
"name": "loopover_miner_doctor",
729+
"category": "ops",
730+
"locality": "miner",
731+
"availability": "selfhost",
732+
"description": "Read-only diagnostic checks for this AMS miner: state directory, engine version match, store reachability, credentials, and configuration. Split out of loopover_miner_status (#9523) so status stays cheap and doctor can grow checks. Every check runs and reports its own pass/warn/fail — nothing is mutated and nothing stops at the first failure."
733+
},
699734
{
700735
"name": "loopover_miner_get_audit_feed",
701736
"category": "agent",
@@ -724,6 +759,13 @@ export const MCP_TOOL_REFERENCE: readonly McpToolReferenceEntry[] = [
724759
"availability": "selfhost",
725760
"description": "Read-only manage-phase status: the per-managed-PR rows `loopover-miner manage status` reports (branch, CI state, gate verdict, outcome, last-polled-at, queue status/priority) plus the run-level portfolio view (one row per tracked repo: run state, updated-at, PR count). Joins the portfolio queue, the append-only event ledger, and run-state by reusing the existing collectManageStatus/collectRunPortfolio aggregators -- no new join logic. Read-only: never calls GitHub, never mutates local stores. Takes no arguments."
726761
},
762+
{
763+
"name": "loopover_miner_get_metrics_snapshot",
764+
"category": "ops",
765+
"locality": "miner",
766+
"availability": "selfhost",
767+
"description": "The same Prometheus metric families the `metrics` CLI exports, as structured JSON — so an agent can read them without parsing the text exposition format. Read-only."
768+
},
727769
{
728770
"name": "loopover_miner_get_plan",
729771
"category": "agent",
@@ -745,6 +787,20 @@ export const MCP_TOOL_REFERENCE: readonly McpToolReferenceEntry[] = [
745787
"availability": "selfhost",
746788
"description": "Read-only per-repo miner run-state (idle/discovering/planning/preparing). Pass repoFullName for a single repo (a null state means none has been recorded for it yet), or omit it to list every repo's state. The read-only analog of ORB's loopover_get_automation_state; adds no state-set or mutation capability."
747789
},
790+
{
791+
"name": "loopover_miner_governor_pause",
792+
"category": "ops",
793+
"locality": "miner",
794+
"availability": "selfhost",
795+
"description": "Pause this miner's governor: no new work is admitted until it resumes. Administrative control, not a content write — the same action the dashboard's pause button dispatches, through the same governor-gated chokepoint, firing the same notification side-channel. Recorded in the event ledger with source=mcp."
796+
},
797+
{
798+
"name": "loopover_miner_governor_resume",
799+
"category": "ops",
800+
"locality": "miner",
801+
"availability": "selfhost",
802+
"description": "Resume this miner's governor after a pause, re-admitting work. The same action the dashboard's resume button dispatches, through the same governor-gated chokepoint. Recorded in the event ledger with source=mcp."
803+
},
748804
{
749805
"name": "loopover_miner_list_claims",
750806
"category": "agent",
@@ -766,6 +822,34 @@ export const MCP_TOOL_REFERENCE: readonly McpToolReferenceEntry[] = [
766822
"availability": "selfhost",
767823
"description": "Health check for the loopover-miner MCP server. Returns a static status object confirming the server is reachable. Reads no AMS state and takes no arguments."
768824
},
825+
{
826+
"name": "loopover_miner_purge_repo",
827+
"category": "ops",
828+
"locality": "miner",
829+
"availability": "selfhost",
830+
"description": "Right-to-be-forgotten: delete every trace of one repo from this miner's local stores, returning the same per-store report as the CLI. IRREVERSIBLE — the rows are gone, not archived. Requires confirm=true, elicits confirmation where the client supports it, and dispatches through the governor-gated chokepoint."
831+
},
832+
{
833+
"name": "loopover_miner_queue_release",
834+
"category": "ops",
835+
"locality": "miner",
836+
"availability": "selfhost",
837+
"description": "Release one claimed portfolio-queue item back to unclaimed, so another cycle can pick it up. Mirrors the dashboard's release action and dispatches through the same governor-gated chokepoint. Recorded in the event ledger with source=mcp."
838+
},
839+
{
840+
"name": "loopover_miner_queue_requeue",
841+
"category": "ops",
842+
"locality": "miner",
843+
"availability": "selfhost",
844+
"description": "Return one portfolio-queue item to the pending pool for another attempt. Mirrors the dashboard's requeue action and dispatches through the same governor-gated chokepoint. Recorded in the event ledger with source=mcp."
845+
},
846+
{
847+
"name": "loopover_miner_run_migrations",
848+
"category": "ops",
849+
"locality": "miner",
850+
"availability": "selfhost",
851+
"description": "Apply pending schema migrations to this miner's EXISTING local stores — it never creates a store that is not already there. Reports each store as migrated, up-to-date, or failed. There is no dry-run mode: applying a migration is opening the store, and the CLI has none either. Dispatches through the governor-gated chokepoint."
852+
},
769853
{
770854
"name": "loopover_miner_status",
771855
"category": "utility",
Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
// The hosted AMS tenant surface (#9523, #9199).
2+
//
3+
// CATALOG AMENDMENT, recorded here rather than improvised: #9523 listed
4+
// `loopover_ams_tenant_create` / `_list` / `_destroy`, and they are NOT here. #9522 landed
5+
// `loopover_tenant_create` / `_list` / `_destroy` taking a `product` of "ams" or "orb", because the control
6+
// plane's own `/v1/tenants` routes are product-parameterized and key their registry by `${product}:${name}`.
7+
// A second, AMS-only spelling of those three would be two names for one capability -- the same rot that kept
8+
// `loopover_fleet_get_analytics` out of #9522 -- and would leave an agent guessing which to call. Use the
9+
// product-parameterized tools with `product: "ams"`.
10+
//
11+
// What IS genuinely AMS-specific, and therefore here, is the pair that has no ORB counterpart: a tenant's
12+
// wake schedule and cycle outcomes, and the ability to trigger a cycle now.
13+
import { z } from "zod";
14+
import { defineTool } from "../tool-definition.js";
15+
16+
const TenantName = z.string().min(1).max(200);
17+
18+
export const AmsTenantHealthInput = z.object({
19+
name: TenantName.describe("The tenant's name, as reported by loopover_tenant_list with product=ams."),
20+
});
21+
22+
export const AmsTenantHealthOutput = z.looseObject({
23+
configured: z.boolean().describe("False when this deployment administers no hosted tenants."),
24+
name: z.string().optional(),
25+
state: z.string().optional().describe("The control plane's own lifecycle vocabulary, passed through verbatim."),
26+
schedule: z.string().nullable().optional().describe("The cron-wake cadence, or null when the tenant wakes only on demand."),
27+
lastWakeAt: z.string().nullable().optional(),
28+
lastCycleOutcome: z.string().nullable().optional(),
29+
containerHealthy: z.boolean().nullable().optional(),
30+
error: z.string().optional(),
31+
});
32+
33+
export const amsTenantHealthTool = defineTool({
34+
name: "loopover_ams_tenant_health",
35+
title: "Read a hosted AMS tenant's health",
36+
description:
37+
"Operator only. One hosted AMS tenant's health: lifecycle state, its cron-wake cadence, when it last woke, that cycle's outcome, and container health. Read-only, and scoped server-side to the authenticated tenant — a name outside that scope is refused rather than answered.",
38+
category: "tenant",
39+
auth: "internal",
40+
locality: "remote",
41+
availability: "cloud",
42+
input: AmsTenantHealthInput,
43+
output: AmsTenantHealthOutput,
44+
});
45+
46+
export const AmsTenantWakeInput = z.object({
47+
name: TenantName,
48+
});
49+
50+
export const AmsTenantWakeOutput = z.looseObject({
51+
configured: z.boolean(),
52+
name: z.string().optional(),
53+
woken: z.boolean().optional(),
54+
throttled: z.boolean().optional().describe("True when the tenant's own schedule guard refused a wake this soon after the last one."),
55+
error: z.string().optional(),
56+
});
57+
58+
export const amsTenantWakeTool = defineTool({
59+
name: "loopover_ams_tenant_wake",
60+
title: "Wake a hosted AMS tenant now",
61+
description:
62+
"Operator only. Trigger an immediate cycle for one hosted AMS tenant, instead of waiting for its next scheduled wake. Bounded by the SAME per-tenant schedule guards the cron path obeys — a wake too soon after the last one is reported as throttled rather than forced through.",
63+
category: "tenant",
64+
auth: "internal",
65+
locality: "remote",
66+
availability: "cloud",
67+
annotations: { readOnlyHint: false, destructiveHint: false },
68+
input: AmsTenantWakeInput,
69+
output: AmsTenantWakeOutput,
70+
});
71+
72+
export const AMS_TENANT_TOOLS = [amsTenantHealthTool, amsTenantWakeTool] as const;

packages/loopover-contract/src/tools/index.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,8 @@ import { OPS_TOOLS } from "./ops.js";
147147
import { FLEET_TOOLS } from "./fleet.js";
148148
import { TENANT_TOOLS } from "./tenant.js";
149149
import { INSTANCE_OPS_TOOLS } from "./instance-ops.js";
150+
import { MINER_OPS_TOOLS } from "./miner-ops.js";
151+
import { AMS_TENANT_TOOLS } from "./ams-tenant.js";
150152
import { adminRotateSecretTool } from "./admin-config.js";
151153

152154
export const TOOL_CONTRACTS: readonly ToolContract[] = [
@@ -282,6 +284,8 @@ export const TOOL_CONTRACTS: readonly ToolContract[] = [
282284
...OPS_TOOLS,
283285
...FLEET_TOOLS,
284286
...TENANT_TOOLS,
287+
...MINER_OPS_TOOLS,
288+
...AMS_TENANT_TOOLS,
285289
];
286290

287291
const CONTRACTS_BY_NAME: ReadonlyMap<string, ToolContract> = new Map(
@@ -318,3 +322,5 @@ export * from "./ops.js";
318322
export * from "./fleet.js";
319323
export * from "./tenant.js";
320324
export * from "./instance-ops.js";
325+
export * from "./miner-ops.js";
326+
export * from "./ams-tenant.js";

0 commit comments

Comments
 (0)