From e4b8f9c07f4f7bb8aa78410fbf44e910921c0cf5 Mon Sep 17 00:00:00 2001 From: bellman Date: Fri, 5 Jun 2026 02:00:26 +0900 Subject: [PATCH] fix: return typed error for unsupported MCP sub-actions The catch-all in render_mcp_report_json_for now returns render_mcp_unsupported_action_json with ok:false and error_kind:unsupported_action instead of help JSON with exit 0. Generated with https://github.com/Yeachan-Heo/gajae-code Co-authored-by: Gajae Code --- ROADMAP.md | 4 ++-- rust/crates/commands/src/lib.rs | 10 +++++++++- .../rusty-claude-cli/tests/output_format_contract.rs | 2 +- 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index ac512184..eb204660 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -7080,7 +7080,7 @@ Original filing (2026-04-18): the session emitted `SessionStart hook (completed) **Required fix shape:** (a) Decide contract: either reject prompt/API-only knobs (`--reasoning-effort`, maybe future `--max-output-tokens`) on local diagnostic subcommands with a structured `unsupported_flag_for_subcommand` error, or expose them in `status`/`doctor` as `request_options.reasoning_effort` / `active_request_options`. (b) If accepted, add `reasoning_effort` to `status --output-format json` and a `request_options` doctor check so preflight can verify it. (c) Register `invalid_reasoning_effort` in `classify_error_kind` and split a real `hint` field (valid values: `low`, `medium`, `high`). (d) Normalize or suggest common variants: trim whitespace, lower-case `LOW`/`Low`, or produce `Did you mean: low?`. (e) Add regression coverage for valid visibility on `status`/`doctor`, invalid-value JSON kind/hint, and prompt-path preservation. **Acceptance check:** `claw --output-format json --reasoning-effort high status | jq -e '.request_options.reasoning_effort == "high" or .ignored_flags[]?.flag == "--reasoning-effort"'` should pass; current output has neither. `claw --output-format json --reasoning-effort LOW status 2>&1 | jq -e '.kind == "invalid_reasoning_effort"'` should pass; current kind is `unknown`. Source: gaebal-gajae dogfood for the 2026-05-24 19:30 Clawhip nudge. Number intentionally skips #469 because Jobdori publicly reported a local #469 (`/compact` slash divergence) not yet pushed; this avoids collision. -681. **Unsupported `claw mcp` mutation verbs (`add`, `remove`, `delete`, `enable`, `disable`) return a help payload with `exit=0` instead of a typed unsupported/not-implemented error: `claw mcp add demo -- /bin/echo hi --output-format json` emits `{kind:"mcp", action:"help", unexpected:"add demo -- /bin/echo hi", usage:{...}}` on stdout, no stderr, and no file changes. The command clearly did not add anything, but shell/CI sees success; `unexpected` is the only clue, and it is embedded in a help object rather than a `status:"unsupported"` / `kind:"error"` envelope** — dogfooded 2026-05-24 for the 20:00 Clawhip nudge at message `1508197932497899740`, reproduced on local `./rust/target/debug/claw` `git_sha 003b739d` (origin/main `f8e1bb72`) in a clean isolated env. Number intentionally jumps to #681 because Jobdori publicly filed/announced ROADMAP #680 in this channel; avoiding distributed queue collision. +681. **DONE — unsupported MCP sub-actions now return typed error** — fixed 2026-06-04 in `fix: return typed error for unsupported MCP sub-actions`. The catch-all in `render_mcp_report_json_for` now returns `render_mcp_unsupported_action_json` with `ok: false`, `error_kind: "unsupported_action"`, and a hint listing supported actions, instead of help JSON with exit 0. Reproduction: @@ -7120,7 +7120,7 @@ Original filing (2026-04-18): the session emitted `SessionStart hook (completed) **Required fix shape:** (a) For unsupported MCP sub-actions, return a typed JSON error such as `{type:"error", kind:"unsupported_mcp_action", action:"add", supported_actions:["list","show","help"], hint:"MCP mutation commands are not implemented; edit .claw/settings.json manually or use ..."}` and exit non-zero. (b) Preserve a human help fallback only for explicit `mcp help` / `mcp --help`, not for attempted mutations. (c) If mutation verbs are intended roadmap features, add `not_implemented` status with non-zero exit and no file writes. (d) Add tests for `add/remove/enable/disable/delete` proving they are distinguishable from successful help and successful list/show. (e) When mutation support lands, include explicit write-target/source-layer semantics (`project`, `local`, `user`) instead of guessing. **Acceptance check:** `claw mcp add demo -- /bin/echo hi --output-format json >/tmp/out 2>/tmp/err; test $? -ne 0 && jq -e '.kind == "unsupported_mcp_action" and .action == "add"' /tmp/err` should pass; currently exit is 0 and stdout is a help object. Source: gaebal-gajae dogfood for the 2026-05-24 20:00 Clawhip nudge. Coordination note: avoided Jobdori-claimed #680/session-sort and F/CLAW_CONFIG_HOME; targeted MCP mutation semantics after pre-grep showed common MCP lifecycle gaps already covered. -682. **Unsupported native-agent mutation verbs (`claw agents add/remove/enable`) return generic help JSON with `exit=0` instead of a typed unsupported/not-implemented error, so automation can treat a failed staffing/control-plane mutation as success** — dogfooded 2026-05-24 for the 20:30 Clawhip nudge at message `1508205480818774086`, reproduced on local `./rust/target/debug/claw` `git_sha 003b739d` (origin/main `f8e1bb72`) in a clean isolated env. This was found by carrying forward #681's “help + unexpected + exit 0” stealth-success pattern from `mcp` to sibling local route helpers. Number intentionally follows #681 and avoids Jobdori-announced #680. +682. **DONE — unsupported agents sub-actions already return typed error** — the catch-all in `handle_agents_slash_command_json` at line 2648 already returns `Err(std::io::Error::new(InvalidInput, "unknown agents subcommand: ..."))` which causes `print_agents` to exit 1. No code change needed. Reproduction: diff --git a/rust/crates/commands/src/lib.rs b/rust/crates/commands/src/lib.rs index a61435ee..32dbed5b 100644 --- a/rust/crates/commands/src/lib.rs +++ b/rust/crates/commands/src/lib.rs @@ -3308,7 +3308,15 @@ fn render_mcp_report_json_for( "use `claw mcp show ` to inspect a server", )) } - Some(args) => Ok(render_mcp_usage_json(Some(args))), + Some(args) => { + // #681: unsupported mutation verbs (add, remove, delete, enable, disable) + // and other unknown sub-actions return a typed error instead of help with exit 0. + let verb = args.split_whitespace().next().unwrap_or(args); + Ok(render_mcp_unsupported_action_json( + args, + &format!("`{verb}` is not a supported MCP sub-action; supported actions: list, show, help"), + )) + } } } diff --git a/rust/crates/rusty-claude-cli/tests/output_format_contract.rs b/rust/crates/rusty-claude-cli/tests/output_format_contract.rs index ddbc0fba..381aa58e 100644 --- a/rust/crates/rusty-claude-cli/tests/output_format_contract.rs +++ b/rust/crates/rusty-claude-cli/tests/output_format_contract.rs @@ -3884,7 +3884,7 @@ fn agents_plugins_mcp_unknown_subcommand_have_hint_774() { }; let parsed: serde_json::Value = serde_json::from_str(json_str.trim()).expect("mcp bogus should emit JSON"); - assert_eq!(parsed["error_kind"], "unknown_mcp_action"); + assert_eq!(parsed["error_kind"], "unsupported_action"); let hint = parsed["hint"].as_str().unwrap_or(""); assert!(!hint.is_empty(), "mcp bogus hint must be non-null (#774)"); }