diff --git a/ROADMAP.md b/ROADMAP.md index e21cdc79..b57f5782 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -7901,8 +7901,12 @@ Original filing (2026-04-18): the session emitted `SessionStart hook (completed) **Acceptance.** `claw --output-format json /commit` hint does NOT mention `--resume`. `claw --output-format json /status` (resume-safe) hint still mentions `--resume`. [SCOPE: claw-code] -830. **`claw mcp show` (missing server name arg) emits `error_kind:"unknown_mcp_action"` instead of `missing_argument`** — dogfooded 2026-05-29 17:00 on `main` `ac5b19de`. `claw --output-format json mcp show` (no server name supplied) exits with `error_kind:"unknown_mcp_action"`. However `show` IS a known MCP action — the error is a missing required argument (server name), not an unknown action. Machine consumers inspecting `error_kind` cannot distinguish "I don't know this action" from "I know this action but a required arg is missing". +830. **DONE — `claw mcp show` (missing server name arg) emits `error_kind:"unknown_mcp_action"` instead of `missing_argument`** — dogfooded 2026-05-29 17:00 on `main` `ac5b19de`. `claw --output-format json mcp show` (no server name supplied) exits with `error_kind:"unknown_mcp_action"`. However `show` IS a known MCP action — the error is a missing required argument (server name), not an unknown action. Machine consumers inspecting `error_kind` cannot distinguish "I don't know this action" from "I know this action but a required arg is missing". **Required fix shape.** The MCP subcommand parser should detect `show` with no following token and emit `missing_argument: mcp show requires a server name.\nUsage: claw mcp show ` with a distinct `error_kind`. Update the classifier arm to return `missing_argument` for this prefix. **Acceptance.** `claw --output-format json mcp show` exits rc=1, stdout `error_kind:"missing_argument"`, stderr empty. Hint contains usage example. [SCOPE: claw-code] + + **Fix applied.** `mcp show` without a server name now emits a typed `missing_argument` response instead of reusing `unknown_mcp_action`. The direct JSON path returns `{kind:"mcp", action:"show", status:"error", error_kind:"missing_argument"}` with a usage hint on stdout and an empty stderr stream; the slash-command parser also classifies `/mcp show` as `missing_argument` via the shared error-kind classifier. + + **Verification.** `cargo fmt --manifest-path rust/Cargo.toml --all -- --check`; `cargo test --manifest-path rust/Cargo.toml -p commands mcp -- --nocapture`; `cargo test --manifest-path rust/Cargo.toml -p rusty-claude-cli mcp_show_missing_server_name_returns_missing_argument_830 -- --nocapture`; `cargo test --manifest-path rust/Cargo.toml -p rusty-claude-cli classify_error_kind_returns_correct_discriminants -- --nocapture`; direct probe `cargo run --manifest-path rust/Cargo.toml -q -p rusty-claude-cli -- --output-format json mcp show`. diff --git a/rust/crates/commands/src/lib.rs b/rust/crates/commands/src/lib.rs index a8fd88d5..79d401be 100644 --- a/rust/crates/commands/src/lib.rs +++ b/rust/crates/commands/src/lib.rs @@ -1670,7 +1670,11 @@ fn parse_mcp_command(args: &[&str]) -> Result Err(usage_error("mcp list", "")), - ["show"] => Err(usage_error("mcp show", "")), + ["show"] => Err(command_error( + "missing_argument: mcp show requires a server name.", + "mcp", + "/mcp show ", + )), ["show", target] => Ok(SlashCommand::Mcp { action: Some("show".to_string()), target: Some((*target).to_string()), @@ -2918,12 +2922,12 @@ fn render_mcp_report_for( } } Some(args) if is_help_arg(args) => Ok(render_mcp_usage(None)), - Some("show") => Ok(render_mcp_usage(Some("show"))), + Some("show") => Ok(render_mcp_missing_argument_text("show")), Some(args) if args.split_whitespace().next() == Some("show") => { let mut parts = args.split_whitespace(); let _ = parts.next(); let Some(server_name) = parts.next() else { - return Ok(render_mcp_usage(Some("show"))); + return Ok(render_mcp_missing_argument_text("show")); }; if parts.next().is_some() { return Ok(render_mcp_usage(Some(args))); @@ -3027,12 +3031,12 @@ fn render_mcp_report_json_for( } } Some(args) if is_help_arg(args) => Ok(render_mcp_usage_json(None)), - Some("show") => Ok(render_mcp_usage_json(Some("show"))), + Some("show") => Ok(render_mcp_missing_argument_json("show")), Some(args) if args.split_whitespace().next() == Some("show") => { let mut parts = args.split_whitespace(); let _ = parts.next(); let Some(server_name) = parts.next() else { - return Ok(render_mcp_usage_json(Some("show"))); + return Ok(render_mcp_missing_argument_json("show")); }; if parts.next().is_some() { return Ok(render_mcp_usage_json(Some(args))); @@ -4269,6 +4273,44 @@ fn render_mcp_usage(unexpected: Option<&str>) -> String { lines.join("\n") } +fn render_mcp_missing_argument_text(action: &str) -> String { + let hint = match action { + "show" => "use `claw mcp show ` to inspect a server", + _ => "provide the required argument for this MCP action", + }; + format!( + "MCP\n Error missing argument for '{action}'\n Hint {hint}\n Usage /mcp [list|show |help]" + ) +} + +fn render_mcp_missing_argument_json(action: &str) -> Value { + let (message, hint) = match action { + "show" => ( + "mcp show requires a server name", + "Usage: claw mcp show ", + ), + _ => ( + "mcp action requires an argument", + "Usage: claw mcp [list|show |help]", + ), + }; + json!({ + "kind": "mcp", + "action": action, + "ok": false, + "status": "error", + "error_kind": "missing_argument", + "message": message, + "hint": hint, + "usage": { + "slash_command": "/mcp [list|show |help]", + "direct_cli": "claw mcp [list|show |help]", + "sources": [".claw/settings.json", ".claw/settings.local.json"], + }, + "unexpected": Value::Null, + }) +} + fn render_mcp_usage_json(unexpected: Option<&str>) -> Value { // #748: add error_kind when unexpected is set, matching agents/plugins unknown-subcommand shape. let error_kind: Value = if unexpected.is_some() { diff --git a/rust/crates/rusty-claude-cli/src/main.rs b/rust/crates/rusty-claude-cli/src/main.rs index 5febf841..554b47bb 100644 --- a/rust/crates/rusty-claude-cli/src/main.rs +++ b/rust/crates/rusty-claude-cli/src/main.rs @@ -297,6 +297,8 @@ fn classify_error_kind(message: &str) -> &'static str { "session_load_failed" } else if message.contains("unsupported ACP invocation") { "unsupported_acp_invocation" + } else if message.starts_with("missing_argument:") { + "missing_argument" } else if message.contains("unsupported skills action") { "unsupported_skills_action" } else if message.contains("unrecognized argument") || message.contains("unknown option") { @@ -13476,6 +13478,11 @@ mod tests { classify_error_kind("unknown_option: unknown system-prompt option: --foo."), "unknown_option" ); + // #830: known command with missing required argument must not collapse to unknown. + assert_eq!( + classify_error_kind("missing_argument: mcp show requires a server name."), + "missing_argument" + ); } #[test] 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 240884bf..7a2189cc 100644 --- a/rust/crates/rusty-claude-cli/tests/output_format_contract.rs +++ b/rust/crates/rusty-claude-cli/tests/output_format_contract.rs @@ -2448,6 +2448,38 @@ fn agents_plugins_mcp_unknown_subcommand_have_hint_774() { } } +#[test] +fn mcp_show_missing_server_name_returns_missing_argument_830() { + let root = unique_temp_dir("mcp-show-missing-830"); + fs::create_dir_all(&root).expect("temp dir"); + + let output = run_claw(&root, &["--output-format", "json", "mcp", "show"], &[]); + assert!( + !output.status.success(), + "mcp show without server must fail" + ); + assert_eq!(output.status.code(), Some(1), "exit code must be 1 (#830)"); + assert!( + output.stderr.is_empty(), + "JSON mcp show missing-argument error must keep stderr empty (#830), got: {}", + String::from_utf8_lossy(&output.stderr) + ); + let parsed: serde_json::Value = serde_json::from_slice(&output.stdout) + .expect("mcp show missing server should emit valid JSON on stdout"); + assert_eq!(parsed["kind"], "mcp"); + assert_eq!(parsed["action"], "show"); + assert_eq!(parsed["status"], "error"); + assert_eq!(parsed["error_kind"], "missing_argument"); + assert!( + parsed["hint"] + .as_str() + .unwrap_or_default() + .contains("mcp show "), + "hint should contain usage example, got: {}", + parsed["hint"] + ); +} + #[test] fn interactive_only_guard_batch_769_to_771() { // #769-#771: a sweep of slash-only verbs with args that previously fell to