mirror of
https://github.com/instructkr/claude-code.git
synced 2026-05-28 00:16:45 +00:00
fix(#706): skills show <name> returns error+exit1 when skill not found; classify_error_kind covers skill_not_found from prose message
This commit is contained in:
@@ -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.
|
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.
|
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 <name> --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:"<name>"}` + exit 1 when `show <name>` matches nothing; landed at `...`. Source: Jobdori dogfood on `f84799c8`, 2026-05-26.
|
||||||
|
|||||||
@@ -2485,6 +2485,17 @@ pub fn handle_skills_slash_command_json(args: Option<&str>, cwd: &Path) -> std::
|
|||||||
.into_iter()
|
.into_iter()
|
||||||
.filter(|s| s.name.to_lowercase() == name)
|
.filter(|s| s.name.to_lowercase() == name)
|
||||||
.collect();
|
.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))
|
Ok(render_skills_report_json(&matched))
|
||||||
}
|
}
|
||||||
Some("install") => Ok(render_skills_usage_json(Some("install"))),
|
Some("install") => Ok(render_skills_usage_json(Some("install"))),
|
||||||
|
|||||||
@@ -304,7 +304,9 @@ fn classify_error_kind(message: &str) -> &'static str {
|
|||||||
"unknown_agents_subcommand"
|
"unknown_agents_subcommand"
|
||||||
} else if message.contains("is not installed") {
|
} else if message.contains("is not installed") {
|
||||||
"plugin_not_found"
|
"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"
|
"skill_not_found"
|
||||||
} else if message.contains("Unsupported config section") {
|
} else if message.contains("Unsupported config section") {
|
||||||
"unsupported_config_section"
|
"unsupported_config_section"
|
||||||
@@ -5940,10 +5942,19 @@ impl LiveCli {
|
|||||||
let cwd = env::current_dir()?;
|
let cwd = env::current_dir()?;
|
||||||
match output_format {
|
match output_format {
|
||||||
CliOutputFormat::Text => println!("{}", handle_skills_slash_command(args, &cwd)?),
|
CliOutputFormat::Text => println!("{}", handle_skills_slash_command(args, &cwd)?),
|
||||||
CliOutputFormat::Json => println!(
|
CliOutputFormat::Json => {
|
||||||
"{}",
|
let result = handle_skills_slash_command_json(args, &cwd)?;
|
||||||
serde_json::to_string_pretty(&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(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user