mirror of
https://github.com/instructkr/claw-code.git
synced 2026-06-04 05:34:11 +08:00
fix: classify mcp show missing server argument
This commit is contained in:
@@ -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 <server>` 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`.
|
||||
|
||||
@@ -1670,7 +1670,11 @@ fn parse_mcp_command(args: &[&str]) -> Result<SlashCommand, SlashCommandParseErr
|
||||
target: None,
|
||||
}),
|
||||
["list", ..] => Err(usage_error("mcp list", "")),
|
||||
["show"] => Err(usage_error("mcp show", "<server>")),
|
||||
["show"] => Err(command_error(
|
||||
"missing_argument: mcp show requires a server name.",
|
||||
"mcp",
|
||||
"/mcp show <server>",
|
||||
)),
|
||||
["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 <server>` 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 <server>|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 <server>",
|
||||
),
|
||||
_ => (
|
||||
"mcp action requires an argument",
|
||||
"Usage: claw mcp [list|show <server>|help]",
|
||||
),
|
||||
};
|
||||
json!({
|
||||
"kind": "mcp",
|
||||
"action": action,
|
||||
"ok": false,
|
||||
"status": "error",
|
||||
"error_kind": "missing_argument",
|
||||
"message": message,
|
||||
"hint": hint,
|
||||
"usage": {
|
||||
"slash_command": "/mcp [list|show <server>|help]",
|
||||
"direct_cli": "claw mcp [list|show <server>|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() {
|
||||
|
||||
@@ -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]
|
||||
|
||||
@@ -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 <server>"),
|
||||
"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
|
||||
|
||||
Reference in New Issue
Block a user