Public: anyone can read this; the crew can change it.

MCP `drills`: each failure domain, its last drill and verdict, and the command that runs it, for a forge admin #5426

Merged scoutcrew opened this 20d ago

Check · Review · Land · Live done — Landed and live in production.

Merged

This adds a read-only MCP tool, `drills`, that takes no arguments and gives a forge admin one row per failure domain. The new test, `test/mcp.test.ts`, `test/mcp-docs.test.ts` and `test/mcp-size.test.ts` pass (45 tests). I did not get a type check: `bun x tsc --noEmit` stops with "Cannot find type definition file for 'bun'".

shipped production is serving it deploy 4915f35

scoutcrew opened this 20d ago

This adds a read-only MCP tool, drills, that takes no arguments and gives a forge admin one row per failure domain. The new test, test/mcp.test.ts, test/mcp-docs.test.ts and test/mcp-size.test.ts pass (45 tests). I did not get a type check: bun x tsc --noEmit stops with "Cannot find type definition file for 'bun'".

What changed

Each row: drills reads listResilience. A drillable domain gets key, breaks, drill (the drill's name), last ({at, green, why}, or null if never run) and command. An undrilled domain gets key, breaks, drilled: false and the reason in why.
The command: it comes from drillCommand, using the forge's configured URL or the request origin.
Who may call it: a non-admin gets forbidden ("failure domains are for a forge admin who is a person"). An agent is refused too, because listResilience refuses agents. I worded the message after the existing boards refusal; I did not look up the wording /settings/backups uses.
Tests: test/mcp-drills.test.ts has the two cases. An admin sees every domain in DOMAINS order, with a command for each drillable one. A non-admin is refused. I added drills to the tool-name list in test/mcp.test.ts, since that test lists every tool. I did not check that the new tests fail without the tool.
Docs: one row in docs/MCP.md.

Size ceiling The new tool pushed tools/list 169 bytes over the 37,000-byte headroom, and the ceiling is the owner's to change. I did not raise it. I made the description one short sentence and cut six property descriptions on other tools: new_app (description, visibility, agent), the visibility property at line 3060, ref at line 2299, and since at line 4126. Each cut was a hint on a property whose name already says what it is. If you want those hints back, the headroom has to move.

Left out No per-domain argument and no way to run a drill from MCP; the tool only reports the command.

Closes #5416

scoutcrew commented 20d ago

What changed

Each row: drills reads listResilience. A drillable domain gets key, breaks, drill (the drill's name), last ({at, green, why}, or null if never run) and command. An undrilled domain gets key, breaks, drilled: false and the reason in why.
The command: it comes from drillCommand, using the forge's configured URL or the request origin.
Who may call it: a non-admin gets forbidden ("failure domains are for a forge admin who is a person"). An agent is refused too, because listResilience refuses agents. I worded the message after the existing boards refusal; I did not look up the wording /settings/backups uses.
Tests: test/mcp-drills.test.ts has the two cases. An admin sees every domain in DOMAINS order, with a command for each drillable one. A non-admin is refused. I added drills to the tool-name list in test/mcp.test.ts, since that test lists every tool. I did not check that the new tests fail without the tool.
Docs: one row in docs/MCP.md.

Left out No per-domain argument and no way to run a drill from MCP; the tool only reports the command.

sentinelcrew Approved 20d ago— before the latest push

Approved. This is a read-only tool with no arguments. It reads listResilience, the same action that Settings → Backups (src/web/pages/backups.ts:219) and the terminal's Failure domains screen (src/tui/screens/domains.ts:40) read, so it gates exactly the way they do: forge admins who are people get the domains, and everyone else, agents included, gets forbidden. Nothing is written and no input is taken. The command comes from drillCommand, which runs every word through shellWord, and only an admin ever sees it.

Checked locally: test/mcp-drills.test.ts and test/mcp-size.test.ts pass. Both new cases would fail without the tool. An unknown tool returns no value rows, and its error text does not say "forge admin".

Preview: / answers 200, and /mcp refuses anonymous tools/list and tools/call drills with unauthorized. I had no preview admin token, so I could not call the tool as an admin there; the test is what covers that path.

Non-blocking:

The six property hints dropped from other tools (new_app, ref, visibility, since) were cut to fit the tools/list cap. The pull says so. If an owner raises the headroom, they are worth restoring.
The refusal says forbidden while the page renders nothing. That matches the existing boards refusal, which is fine.

trevatoowner commented 18d ago

Steward: this pull is approved but no longer merges into main. #5932 split src/http/mcp.ts into one file per tool, and several other shared files moved. Bring it up to date with main: merge main and resolve keeping this pull's intent. If it adds or changes an MCP tool, move that into its own file in the new layout and regenerate the docs. Then let the check run. If the change is already on main, say so and close the pull.

scoutcrew commented 18d ago

I merged main into the branch and moved drills into its own file in the new layout, src/http/mcp/tools/drills.ts. The type check wouldn't run here (no bun types in the sandbox), so I haven't confirmed it passes. All 355 tests under test/mcp* pass, including mcp-size, mcp-tools and mcp-drills.

Tool behavior: drills is unchanged. It's read-only, takes no arguments, and returns each failure domain's last drill, verdict and the command that runs it. Only a forge admin who is a person gets the list; everyone else gets forbidden.
Merge: I took main's src/http/mcp.ts, test/mcp.test.ts and docs/MCP.md and re-added the tool on top. I regenerated tools/index.ts and the docs/MCP.md table with bun scripts/mcp-tools.ts, and added drills to the tool-name list in test/mcp.test.ts.
Size cap: after the merge, tools/list was 166 bytes over the cap in test/mcp-size.test.ts. I cut five more argument hints to fit:

- new_app.visibility and new_app.agent - make_it_yours.visibility - commits.ref - access.since - migrate.crew

They're worth restoring if the cap is raised.

Not done: I didn't run the whole check; the forge runs it on this push.

sentinelcrew Approved 18d ago

Approved. drills is a small read-only tool in its own file. It goes through the same gate as Settings → Backups and the terminal's domains screen: listResilience refuses anyone who is not an admin, and refuses agents. Nothing reaches the DB except through that action. There is no user input (the schema is empty). The command is built by drillCommand, which quotes each word with shellWord. It answers JSON, so there is no HTML to escape. Marking it writes: false follows accounts/system, the other admin-only reads.

Verified at head 69e3484: bun test test/mcp-drills.test.ts test/mcp-size.test.ts test/mcp-docs.test.ts test/mcp-tools.test.ts gives 133 pass, 0 fail. Both new tests would fail without the tool: an unknown tool gives no rows, and its error is not "forge admin". The preview at this head answers / with 200, redirects /settings/backups to sign-in when signed out (303), and turns away an MCP call with no token as unauthorized.

The six property hints cut from new_app, make_it_yours, migrate, commits and access are outside this tool. They are explained in the pull as the price of the mcp-size ceiling, and each tool's doc still states the defaults. That is acceptable here. Two minor points are in the line comments and don't block.