diff --git a/CHANGELOG.md b/CHANGELOG.md index 767c097..f85dcd6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,9 @@ Cutting that release is tracked in ### Added +- The single MCP tool now accurately declares its mutating, destructive, + non-idempotent, open-world behavior; integration tests lock the metadata and + deliberate `stop` / `session close` exposure. - An MCP stdio integration suite covering initialization, tool discovery, browser-command calls, malformed and oversized input, and rejection of local CLI commands. diff --git a/apps/headless/MCP/main.swift b/apps/headless/MCP/main.swift index 53f3634..8620479 100644 --- a/apps/headless/MCP/main.swift +++ b/apps/headless/MCP/main.swift @@ -23,7 +23,14 @@ private func toolResult(id: Any?, text: String, isError: Bool = false) { private let tool: [String: Any] = [ "name": "headless", - "description": "Run a safe Headless CLI command against the already-running local browser host. Supply argv without the headless binary name.", + "description": "Run a Headless CLI browser command against the already-running local browser host. Commands may navigate or mutate page/session state; stop and session close are destructive. Supply argv without the headless binary name.", + "annotations": [ + "title": "Headless browser command", + "readOnlyHint": false, + "destructiveHint": true, + "idempotentHint": false, + "openWorldHint": true, + ], "inputSchema": [ "type": "object", "additionalProperties": false, "properties": ["argv": ["type": "array", "items": ["type": "string"], "maxItems": 32]], diff --git a/apps/headless/Tests/HeadlessMCPTests/main.swift b/apps/headless/Tests/HeadlessMCPTests/main.swift index 6cd0793..a1fbd40 100644 --- a/apps/headless/Tests/HeadlessMCPTests/main.swift +++ b/apps/headless/Tests/HeadlessMCPTests/main.swift @@ -58,9 +58,11 @@ func run() throws { #"{"jsonrpc":"2.0","method":"notifications/initialized"}"#, #"{"jsonrpc":"2.0","id":2,"method":"tools/list"}"#, #"{"jsonrpc":"2.0","id":3,"method":"tools/call","params":{"name":"headless","arguments":{"argv":["status"]}}}"#, + #"{"jsonrpc":"2.0","id":4,"method":"tools/call","params":{"name":"headless","arguments":{"argv":["stop"]}}}"#, + #"{"jsonrpc":"2.0","id":5,"method":"tools/call","params":{"name":"headless","arguments":{"argv":["session","close","disposable"]}}}"#, "not-json", String(repeating: "x", count: headlessMaximumMessageBytes + 1), - #"{"jsonrpc":"2.0","id":4,"method":"tools/call","params":{"name":"headless","arguments":{"argv":["start"]}}}"#, + #"{"jsonrpc":"2.0","id":6,"method":"tools/call","params":{"name":"headless","arguments":{"argv":["start"]}}}"#, ] try process.run() @@ -76,7 +78,7 @@ func run() throws { let value = try JSONSerialization.jsonObject(with: Data(line.utf8)) return try object(value, "MCP response was not a JSON object") } - try expect(responses.count == 6, "expected six MCP responses, received \(responses.count)") + try expect(responses.count == 8, "expected eight MCP responses, received \(responses.count)") let initialize = try object(responses[0]["result"], "initialize result was absent") try expect(initialize["protocolVersion"] as? String == "2025-06-18", "initialize protocol version changed") @@ -88,6 +90,14 @@ func run() throws { throw TestFailure(description: "tools/list did not expose exactly one tool") } try expect(tools[0]["name"] as? String == "headless", "tools/list exposed the wrong tool") + let description = tools[0]["description"] as? String ?? "" + try expect(!description.contains("safe Headless"), "destructive MCP tool was still described as safe") + try expect(description.contains("stop and session close are destructive"), "destructive-command guidance was absent") + let annotations = try object(tools[0]["annotations"], "MCP tool annotations were absent") + try expect(annotations["readOnlyHint"] as? Bool == false, "MCP tool was marked read-only") + try expect(annotations["destructiveHint"] as? Bool == true, "MCP tool was not marked destructive") + try expect(annotations["idempotentHint"] as? Bool == false, "MCP tool was marked idempotent") + try expect(annotations["openWorldHint"] as? Bool == true, "MCP tool was not marked open-world") let call = try object(responses[2]["result"], "tools/call result was absent") try expect(call["isError"] as? Bool == false, "browser tools/call unexpectedly failed") @@ -103,13 +113,33 @@ func run() throws { let browserResult = try object(browserResponse["result"], "browser protocol result was absent") try expect(browserResult["ready"] as? Bool == true, "browser command did not reach the local host") - for index in 3...4 { + for (index, expectedCommand) in [(3, "shutdown"), (4, "session.close")] { + let destructiveCall = try object(responses[index]["result"], "destructive tools/call result was absent") + try expect(destructiveCall["isError"] as? Bool == false, "annotated destructive command was rejected") + guard let destructiveContent = destructiveCall["content"] as? [[String: Any]], + let destructiveText = destructiveContent.first?["text"] as? String else { + throw TestFailure(description: "destructive tools/call content was absent") + } + let destructiveResponse = try object( + JSONSerialization.jsonObject(with: Data(destructiveText.utf8)), + "destructive tools/call content was not a protocol response" + ) + let destructiveResult = try object( + destructiveResponse["result"], "destructive browser protocol result was absent" + ) + try expect( + destructiveResult["command"] as? String == expectedCommand, + "destructive command did not reach the local host" + ) + } + + for index in 5...6 { let parseError = try object(responses[index]["error"], "invalid input did not return JSON-RPC error") let code = try integer(parseError["code"], "parse error code was absent") try expect(code == -32700, "invalid input returned the wrong error code") } - let localCall = try object(responses[5]["result"], "local-command result was absent") + let localCall = try object(responses[7]["result"], "local-command result was absent") try expect(localCall["isError"] as? Bool == true, "local CLI command was accepted over MCP") guard let localContent = localCall["content"] as? [[String: Any]], let localText = localContent.first?["text"] as? String else { diff --git a/apps/headless/docs/P2.md b/apps/headless/docs/P2.md index b217183..88b23f5 100644 --- a/apps/headless/docs/P2.md +++ b/apps/headless/docs/P2.md @@ -72,6 +72,12 @@ only expose cookie/storage values when the host is started with socket used by the CLI. It creates no TCP listener. Start the local browser host first, then configure a remote agent to run it through SSH: +The single MCP tool mirrors the remote CLI command surface, including `stop` +and `session close`. Its discovery metadata is therefore explicitly +non-read-only, destructive, non-idempotent, and open-world; clients should +confirm mutating calls according to their policy. These annotations are hints, +not a substitute for the host's socket, validation, and navigation boundaries. + ```sh ssh hermes-vm headless start ssh hermes-vm headless-mcp diff --git a/docs/roadmap/architecture-decisions.md b/docs/roadmap/architecture-decisions.md index d52772b..8710290 100644 --- a/docs/roadmap/architecture-decisions.md +++ b/docs/roadmap/architecture-decisions.md @@ -277,6 +277,30 @@ the boundary explicit and testable. value containing a literal `--json` or `--session`. This changes only CLI parsing; the wire protocol and protocol version remain unchanged. +## 17. MCP exposes the full remote-command surface with pessimistic annotations + +**Decision:** keep `stop` and `session close` callable through the single +argv-based MCP tool. Describe the tool as state-mutating and explicitly set +the MCP annotations `readOnlyHint: false`, `destructiveHint: true`, +`idempotentHint: false`, and `openWorldHint: true`. + +**Status:** decided 2026-08-10 while resolving backlog §C2. + +**Rationale:** the MCP adapter deliberately mirrors the remote CLI surface. +Special-casing two valid remote commands in the adapter would create policy +drift and prevent an MCP operator from recovering a wedged host or cleaning up +a session. The same tool already navigates, clicks, fills, and changes browser +state, so describing it as universally "safe" was inaccurate. Because MCP +annotations apply to the whole tool rather than individual argv variants, the +tool must advertise the risk of its most destructive valid invocation. + +**Consequences:** trusted MCP clients can require confirmation for the tool, +and callers can still invoke the complete browser-command surface. Annotations +are risk metadata, not authorization; the private socket, peer-UID check, +protocol validation, and host-enforced safety rules remain the security +boundary. Local-only commands such as `start` remain rejected by the adapter. +This changes MCP discovery metadata only and does not bump the wire protocol. + --- ## Decision log @@ -291,5 +315,6 @@ parsing; the wire protocol and protocol version remain unchanged. | 12 | Version unification on git tag | Planned (Phase 3) | 2026-08-04 | | 15 | Package-manager distribution set | Decided (owner) | 2026-08-04 | | 16 | Preserve CLI value boundaries with `--` and shell quoting | Decided | 2026-08-10 | +| 17 | Keep full MCP surface; annotate its maximum risk | Decided | 2026-08-10 | New decisions append here with the same format. diff --git a/docs/roadmap/improvements-backlog.md b/docs/roadmap/improvements-backlog.md index 7dfb897..a81c156 100644 --- a/docs/roadmap/improvements-backlog.md +++ b/docs/roadmap/improvements-backlog.md @@ -215,9 +215,12 @@ via MCP. ~~Derive identically.~~ **Done:** both adapters now call the same `HeadlessProtocol.requestTimeout(for:)` helper. Coverage locks the ordinary, wait-derived, tour, screenshot-series, screenshot, and recording-stop cases. -**C2. Destructive verbs over MCP.** ([#30](https://github.com/LockInTime/headless/issues/30)) `stop` (shutdown) and `session close` are +**C2. Destructive verbs over MCP.** ([#30](https://github.com/LockInTime/headless/issues/30)) ~~`stop` (shutdown) and `session close` are callable though the tool description says "safe"; decide policy (deny, or -annotate) and test it. +annotate) and test it.~~ **Done:** architecture decision §17 keeps the complete +remote CLI surface and pessimistically annotates the single tool as mutating, +destructive, non-idempotent, and open-world. The stdio integration suite locks +the metadata and proves both destructive commands still reach the local host. **C3. Zero MCP tests** ([#31](https://github.com/LockInTime/headless/issues/31)) — ~~the only coverage is inside `qa-videos.sh`. Add a stdio harness test: initialize / tools/list / tools/call / malformed line /