From 63931c74fbc40bd809ad5ffb15bf190e91a4b2e4 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Fri, 29 May 2026 17:07:15 +0900 Subject: [PATCH 1/2] fix: mcp show (missing server name) emits missing_argument error_kind (#830) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `claw --output-format json mcp show` (no server name) previously emitted error_kind:"unknown_mcp_action" — misleading because `show` IS a known action; the problem is a missing required argument. Fix: - `render_mcp_report_json_for`: `Some("show")` arm now emits a dedicated JSON response with error_kind:"missing_argument" + usage hint - `classify_error_kind`: add classifier arm for "missing_argument:" prefix One new test: mcp_show_missing_server_name_emits_missing_argument 572+ tests pass. --- rust/crates/commands/src/lib.rs | 14 ++++++++- rust/crates/rusty-claude-cli/src/main.rs | 4 ++- .../tests/output_format_contract.rs | 30 +++++++++++++++++++ 3 files changed, 46 insertions(+), 2 deletions(-) diff --git a/rust/crates/commands/src/lib.rs b/rust/crates/commands/src/lib.rs index a8fd88d5b6..d148f2413f 100644 --- a/rust/crates/commands/src/lib.rs +++ b/rust/crates/commands/src/lib.rs @@ -3027,7 +3027,19 @@ 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"))), + // #830: `claw mcp show` with no server name is a missing required + // argument, not an unknown action. Emit a dedicated error_kind so + // machine consumers can distinguish "I know show, but need a name" + // from "I don't know this action". + Some("show") => Ok(serde_json::json!({ + "kind": "mcp", + "action": "show", + "status": "error", + "ok": false, + "error_kind": "missing_argument", + "hint": "Usage: claw mcp show \nRun `claw mcp list` to see available servers.", + "message": "missing required argument: mcp show requires a server name.", + })), Some(args) if args.split_whitespace().next() == Some("show") => { let mut parts = args.split_whitespace(); let _ = parts.next(); diff --git a/rust/crates/rusty-claude-cli/src/main.rs b/rust/crates/rusty-claude-cli/src/main.rs index 5febf8417a..874408de1e 100644 --- a/rust/crates/rusty-claude-cli/src/main.rs +++ b/rust/crates/rusty-claude-cli/src/main.rs @@ -271,7 +271,9 @@ Run `claw --help` for usage." /// matching against the error messages produced throughout the CLI surface. fn classify_error_kind(message: &str) -> &'static str { // Check specific patterns first (more specific before generic) - if message.starts_with("unknown_slash_command:") { + if message.starts_with("missing_argument:") || message.starts_with("missing required argument:") { + "missing_argument" + } else if message.starts_with("unknown_slash_command:") { "unknown_slash_command" } else if message.starts_with("command_not_found:") { "command_not_found" 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 240884bf45..a1f2e41124 100644 --- a/rust/crates/rusty-claude-cli/tests/output_format_contract.rs +++ b/rust/crates/rusty-claude-cli/tests/output_format_contract.rs @@ -4054,3 +4054,33 @@ fn resume_safe_interactive_only_hint_includes_resume_suggestion() { "/diff hint must suggest --resume (it is resume-safe) (#829): hint={hint:?}" ); } + +// #830: claw mcp show (missing server name) must emit missing_argument, not unknown_mcp_action +#[test] +fn mcp_show_missing_server_name_emits_missing_argument() { + let root = unique_temp_dir("mcp-show-missing-830"); + std::fs::create_dir_all(&root).expect("create temp dir"); + let output = run_claw(&root, &["--output-format", "json", "mcp", "show"], &[]); + assert_eq!(output.status.code(), Some(1), "mcp show (no name) should exit 1"); + let stdout = String::from_utf8_lossy(&output.stdout); + let stderr = String::from_utf8_lossy(&output.stderr); + let j: serde_json::Value = + serde_json::from_str(stdout.trim()).expect("mcp show (no name) must emit JSON (#830)"); + assert_eq!( + j["error_kind"], "missing_argument", + "mcp show (no name) must emit missing_argument, not unknown_mcp_action (#830): {j}" + ); + assert_ne!( + j["error_kind"], "unknown_mcp_action", + "mcp show (no name) must not emit unknown_mcp_action (#830): {j}" + ); + let hint = j["hint"].as_str().unwrap_or(""); + assert!( + hint.contains("claw mcp show") || hint.contains("mcp list"), + "mcp show (no name) hint should mention usage (#830): {hint:?}" + ); + assert!( + stderr.is_empty(), + "mcp show (no name) JSON must have empty stderr (#830): {stderr:?}" + ); +} From 892d8f9ea64b6cf4f4f80f0691abcca5fb29b401 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Fri, 29 May 2026 17:11:00 +0900 Subject: [PATCH 2/2] fix: direct-slash resume-safe commands route to CliAction instead of interactive_only (#831) `claw /status`, `claw /diff`, `claw /version`, `claw /doctor`, `claw /sandbox` all returned interactive_only because the direct-slash handler only had explicit arms for /help, /agents, /mcp, /skills. Other resume-safe slash commands fell through to the generic `Ok(Some(command)) => Err(interactive_only...)` arm. Fix: add explicit routing arms for SlashCommand::Status, ::Diff, ::Version, ::Doctor, ::Sandbox in `parse_direct_slash_command`. One new integration test: direct_slash_resume_safe_commands_route_correctly (covers /version, /sandbox, /diff) 572+ tests pass. --- ROADMAP.md | 6 ++++ rust/crates/rusty-claude-cli/src/main.rs | 15 +++++++++ .../tests/output_format_contract.rs | 31 +++++++++++++++++++ 3 files changed, 52 insertions(+) diff --git a/ROADMAP.md b/ROADMAP.md index e21cdc7939..e46bbe6fff 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -7906,3 +7906,9 @@ Original filing (2026-04-18): the session emitted `SessionStart hook (completed) **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] + +831. **Direct-slash resume-safe commands (`claw /status`, `claw /diff`, `claw /version`, `claw /doctor`, `claw /sandbox`) return `interactive_only` instead of executing** — dogfooded 2026-05-29 17:05 on `main` `ac5b19de`. `/help`, `/agents`, `/skills list` work because they have explicit `CliAction` routing arms in `parse_direct_slash_command`. `/status`, `/diff`, `/version`, `/doctor`, `/sandbox` fall through to the generic `Ok(Some(command)) => Err(interactive_only...)` arm. The hint correctly suggests `--resume SESSION.jsonl /status`, but `claw /status` directly should also work — these commands don't need a session. + + **Required fix shape.** Add explicit `Ok(Some(SlashCommand::Status | Diff | Version | Doctor | Sandbox))` arms in `parse_direct_slash_command` routing to the same `CliAction` variants as `"status"` / `"diff"` / `"version"` / `"doctor"` / `"sandbox"` subcommands. + + **Acceptance.** `claw --output-format json /status` exits 0 and emits the same envelope as `claw --output-format json status`. [SCOPE: claw-code] diff --git a/rust/crates/rusty-claude-cli/src/main.rs b/rust/crates/rusty-claude-cli/src/main.rs index 874408de1e..ad48ac7a6b 100644 --- a/rust/crates/rusty-claude-cli/src/main.rs +++ b/rust/crates/rusty-claude-cli/src/main.rs @@ -1741,6 +1741,21 @@ fn parse_direct_slash_cli_action( }), } } + // #831: resume-safe slash commands invoked directly (claw /status, + // claw /diff, claw /version, claw /doctor, claw /sandbox) should + // route to the same CliAction as their subcommand equivalents. + // Previously they fell through to the generic interactive_only arm. + Ok(Some(SlashCommand::Status)) => Ok(CliAction::Status { + model: model.to_string(), + model_flag_raw: None, + permission_mode: permission_mode_override.unwrap_or_else(default_permission_mode), + output_format, + allowed_tools, + }), + Ok(Some(SlashCommand::Diff)) => Ok(CliAction::Diff { output_format }), + Ok(Some(SlashCommand::Version)) => Ok(CliAction::Version { output_format }), + Ok(Some(SlashCommand::Doctor)) => Ok(CliAction::Doctor { output_format }), + Ok(Some(SlashCommand::Sandbox)) => Ok(CliAction::Sandbox { output_format }), Ok(Some(SlashCommand::Unknown(name))) => { // #828: /approve and /deny are valid REPL-only slash commands that // are not SlashCommand enum variants (they require an active tool 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 a1f2e41124..b3242acb03 100644 --- a/rust/crates/rusty-claude-cli/tests/output_format_contract.rs +++ b/rust/crates/rusty-claude-cli/tests/output_format_contract.rs @@ -4084,3 +4084,34 @@ fn mcp_show_missing_server_name_emits_missing_argument() { "mcp show (no name) JSON must have empty stderr (#830): {stderr:?}" ); } + +// #831: direct-slash resume-safe commands must route to their CliAction equivalents +#[test] +fn direct_slash_resume_safe_commands_route_correctly() { + let root = unique_temp_dir("direct-slash-resume-831"); + std::fs::create_dir_all(&root).expect("create temp dir"); + // /version, /doctor, /sandbox, /diff all work without --resume + for (cmd, expected_kind) in &[ + ("/version", "version"), + ("/sandbox", "sandbox"), + ("/diff", "diff"), + ] { + let output = run_claw(&root, &["--output-format", "json", cmd], &[]); + let stdout = String::from_utf8_lossy(&output.stdout); + let j: serde_json::Value = serde_json::from_str(stdout.trim()) + .unwrap_or_else(|_| panic!("{cmd} must emit JSON (#831), got: {stdout:?}")); + assert_eq!( + j["status"], "ok", + "{cmd} direct slash must exit ok (#831): {j}" + ); + assert_ne!( + j["error_kind"], "interactive_only", + "{cmd} direct slash must not emit interactive_only (#831): {j}" + ); + let kind = j["kind"].as_str().unwrap_or(""); + assert_eq!( + kind, *expected_kind, + "{cmd} direct slash must emit kind={expected_kind} (#831): {j}" + ); + } +}