From dedad14ae449f2b49b20d22860b4d4aa46cdc21d Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Tue, 26 May 2026 00:04:39 +0900 Subject: [PATCH] fix(#706): skills show returns error+exit1 when skill not found; classify_error_kind covers skill_not_found from prose message --- ROADMAP.md | 2 ++ rust/crates/commands/src/lib.rs | 11 +++++++++++ rust/crates/rusty-claude-cli/src/main.rs | 21 ++++++++++++++++----- 3 files changed, 29 insertions(+), 5 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index 2f345e2f..45c3a4ce 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -7577,3 +7577,5 @@ Original filing (2026-04-18): the session emitted `SessionStart hook (completed) 704. **`claw doctor --output-format json` all `checks[].label` fields are `null` — downstream claws cannot identify which check produced a `warn`/`error` without scraping the prose `name` or `details[]` array** — dogfooded 2026-05-25 on `1a6f54b9`. Reproduction: `claw doctor --output-format json | jq '[.checks[] | {label, status}]'` returns 7 entries all with `"label": null`. The `status` field correctly encodes `"ok"/"warn"/"error"` but there is no stable machine-readable identifier for each check. A claw automating `claw doctor` must either enumerate checks by positional index (fragile) or scrape the `name` prose string (brittle). **Required fix shape:** (a) add a stable `id` or `label` field to each `DiagnosticCheck` (e.g. `"credentials"`, `"git"`, `"sandbox"`, `"config"`, `"mcp"`, `"trust"`, `"workspace"`) that downstream parsers can key on; (b) the field should be `snake_case` and never change across releases; (c) the `name` field can remain as the human-readable title; (d) add regression asserting at least one check has a non-null `label` in `output_format_contract.rs`. **Why it matters:** doctor automation (e.g. preflight gates, CI health checks) requires routing on which check failed, not just that *a* check failed; positional-index routing breaks whenever a new check is added. Source: Jobdori dogfood on `1a6f54b9`, 2026-05-25. 705. **`status` and `export` usage JSON `estimated_cost_usd` is a string `"$0.0000"` not a number — downstream claws must strip `$` and parse float to compute costs** — dogfooded 2026-05-25 on `8f809d9a`. `claw status --output-format json | jq '.usage.estimated_cost_usd'` returns `"$0.0000"` (string). Cost aggregation or threshold checks require `parseFloat(x.replace("$",""))`. **Fix shape:** emit `estimated_cost_usd` as a JSON number and add `estimated_cost_usd_formatted` as the display string. Partial fix landed: `estimated_cost_usd_num` (float) added as a companion field at `cb...` alongside the legacy string field for backwards compatibility; `estimated_cost_usd` string preserved. Source: Jobdori dogfood on `8f809d9a`, 2026-05-25. + +706. **`claw skills show --output-format json` silently returns `status:"ok"` with empty `skills:[]` when the named skill does not exist — downstream claws cannot distinguish "no skill installed" from "skill name typo"** — dogfooded 2026-05-26 on `f84799c8`. Reproduction: `claw skills show nonexistent --output-format json` → `{kind:"skills", action:"list", status:"ok", skills:[], summary:{total:0,...}}` exit 0. A claw checking whether a skill is available treats empty success as "no skills installed anywhere" rather than "skill not found". **Fix shape:** return `{kind:"skills", action:"show", status:"error", error_kind:"skill_not_found", requested:""}` + exit 1 when `show ` matches nothing; landed at `...`. Source: Jobdori dogfood on `f84799c8`, 2026-05-26. diff --git a/rust/crates/commands/src/lib.rs b/rust/crates/commands/src/lib.rs index 5e987b79..9b18ea05 100644 --- a/rust/crates/commands/src/lib.rs +++ b/rust/crates/commands/src/lib.rs @@ -2485,6 +2485,17 @@ pub fn handle_skills_slash_command_json(args: Option<&str>, cwd: &Path) -> std:: .into_iter() .filter(|s| s.name.to_lowercase() == name) .collect(); + // #706: return typed error when named skill is not found instead of silent empty list + if matched.is_empty() { + return Ok(json!({ + "kind": "skills", + "action": "show", + "status": "error", + "error_kind": "skill_not_found", + "message": format!("skill '{}' not found", name), + "requested": name, + })); + } Ok(render_skills_report_json(&matched)) } Some("install") => Ok(render_skills_usage_json(Some("install"))), diff --git a/rust/crates/rusty-claude-cli/src/main.rs b/rust/crates/rusty-claude-cli/src/main.rs index d6aa04fa..97cece06 100644 --- a/rust/crates/rusty-claude-cli/src/main.rs +++ b/rust/crates/rusty-claude-cli/src/main.rs @@ -304,7 +304,9 @@ fn classify_error_kind(message: &str) -> &'static str { "unknown_agents_subcommand" } else if message.contains("is not installed") { "plugin_not_found" - } else if message.contains("skill source") && message.contains("not found") { + } else if (message.contains("skill source") && message.contains("not found")) + || message.starts_with("skill '") + { "skill_not_found" } else if message.contains("Unsupported config section") { "unsupported_config_section" @@ -5940,10 +5942,19 @@ impl LiveCli { let cwd = env::current_dir()?; match output_format { CliOutputFormat::Text => println!("{}", handle_skills_slash_command(args, &cwd)?), - CliOutputFormat::Json => println!( - "{}", - serde_json::to_string_pretty(&handle_skills_slash_command_json(args, &cwd)?)? - ), + CliOutputFormat::Json => { + let result = handle_skills_slash_command_json(args, &cwd)?; + let is_error = result.get("status").and_then(|v| v.as_str()) == Some("error"); + println!("{}", serde_json::to_string_pretty(&result)?); + if is_error { + return Err(result + .get("message") + .and_then(|v| v.as_str()) + .unwrap_or("skills command failed") + .to_string() + .into()); + } + } } Ok(()) }