diff --git a/crates/mcp/src/tools.rs b/crates/mcp/src/tools.rs index 3265b36f9..956e8801c 100644 --- a/crates/mcp/src/tools.rs +++ b/crates/mcp/src/tools.rs @@ -384,8 +384,7 @@ fn annotations(name: &str, title: &str) -> Value { | "ui_inspect" | "ui_screenshot" ); - let files = matches!(name, "export" | "render_photo" | "screenshot"); - json!({"title": title, "readOnlyHint": read_only, "destructiveHint": !read_only && !files, + json!({"title": title, "readOnlyHint": read_only, "destructiveHint": !read_only, "idempotentHint": read_only || matches!(name, "render_photo" | "screenshot"), "openWorldHint": false}) } diff --git a/crates/mcp/tests/e2e.rs b/crates/mcp/tests/e2e.rs index 8b0396490..66cb93460 100644 --- a/crates/mcp/tests/e2e.rs +++ b/crates/mcp/tests/e2e.rs @@ -78,6 +78,33 @@ fn exposure_roundtrip(server: &mut Server) { assert!(darker < before - 15.0, "exposure -1.5 should darken: {before} → {darker}"); } +#[test] +fn file_writers_advertise_destructive_effects() { + for with_commands in [false, true] { + let mut server = Server::new(Box::new(Headless::demo())).with_command_tools(with_commands); + let replies = session(&mut server, &[json!({"jsonrpc": "2.0", "id": 1, "method": "tools/list"})]); + let tools = replies[0]["result"]["tools"].as_array().unwrap(); + for name in ["export", "render_photo"] { + let tool = tools.iter().find(|t| t["name"] == name).unwrap(); + assert_eq!(tool["annotations"]["readOnlyHint"], false, "{name}"); + assert_eq!(tool["annotations"]["destructiveHint"], true, "{name} can replace a file"); + } + let preview = tools.iter().find(|t| t["name"] == "render_preview").unwrap(); + assert_eq!(preview["annotations"]["readOnlyHint"], true); + assert_eq!(preview["annotations"]["destructiveHint"], false); + } + let addr = fake_app(); + let mut server = Server::new(Box::new(Remote::connect(&addr).unwrap())); + let replies = session(&mut server, &[json!({"jsonrpc": "2.0", "id": 1, "method": "tools/list"})]); + let tools = replies[0]["result"]["tools"].as_array().unwrap(); + let screenshot = tools.iter().find(|t| t["name"] == "screenshot").unwrap(); + assert_eq!(screenshot["annotations"]["readOnlyHint"], false); + assert_eq!(screenshot["annotations"]["destructiveHint"], true); + let image_only = tools.iter().find(|t| t["name"] == "ui_screenshot").unwrap(); + assert_eq!(image_only["annotations"]["readOnlyHint"], true); + assert_eq!(image_only["annotations"]["destructiveHint"], false); +} + #[test] fn headless_set_exposure_and_render() { exposure_roundtrip(&mut Server::new(Box::new(Headless::demo()))); diff --git a/docs/mcp.md b/docs/mcp.md index 00841bec8..d99bbff87 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -146,8 +146,11 @@ Each edit in a batch has its own undo step. Connect mode also lists `ui_inspect` Ports and connect mode are unchanged. Every listed tool has a title and all four MCP hints. Generated command tools are conservatively -marked as edits. Helpers with optional output paths are marked as writers; `export` is not -idempotent because the default conflict policy chooses a new filename on repeated calls. +marked as edits. `export`, `render_photo` and `screenshot` have `readOnlyHint: false` and +`destructiveHint: true`: their caller-supplied output paths can replace existing files, even +when a path is optional. The image-only `render_preview` and `ui_screenshot` remain read-only +and non-destructive. These annotations are client hints, not a file-access sandbox. +`export` is not idempotent because the default conflict policy chooses a new filename on repeated calls. Unknown helper argument keys return JSON-RPC `-32602` naming the key and accepted arguments. Generated `cmd_*` tools and nested command params remain free-form: the registry has prose parameter docs, not machine-readable schemas. Existing command validation (including strict