fix(#796): agents/skills show <name> <extra> returned wrong not-found instead of unexpected_extra_args

This commit is contained in:
YeonGyu-Kim 2026-05-27 14:07:04 +09:00
parent 18b4cee5fd
commit 9976585f87
3 changed files with 129 additions and 2 deletions

View File

@ -7757,3 +7757,5 @@ Original filing (2026-04-18): the session emitted `SessionStart hook (completed)
794. **`claw plugins install /nonexistent/path` returned `error_kind:"unknown"` + `hint:null`** — dogfooded 2026-05-27 on `57a57ef7`. The error message `"plugin source '/path' was not found"` had no classifier arm, falling to `"unknown"`. Fix: added `plugin_source_not_found` classifier arm (`message.contains("plugin source") && message.contains("was not found")`); added `"plugin_source_not_found"``"Check that the path or URL is correct..."` to `fallback_hint_for_error_kind`. Unit test assertion added to `test_classify_error_kind`; integration test `plugins_install_not_found_path_returns_typed_kind_794` added. 56 CLI contract tests pass. [SCOPE: claw-code] Source: Jobdori plugins install probe on `57a57ef7`, 2026-05-27.
795. **`claw skills install /nonexistent` returned `skill_not_found + hint:null` and `claw skills uninstall x` returned `unsupported_skills_action + hint:null`** — dogfooded 2026-05-27 on `491f179a`. Both error kinds were missing from `fallback_hint_for_error_kind` table, so even though classify returned a typed kind, the hint field was always null. Fix: added `"skill_not_found"` → hint suggesting `claw skills list` / `claw skills install`; added `"unsupported_skills_action"` → hint listing supported actions. Integration test `skills_install_not_found_and_unsupported_action_have_hints_795` covers both paths. 57 CLI contract tests pass. [SCOPE: claw-code] Source: Jobdori skills lifecycle probe on `491f179a`, 2026-05-27.
796. **`claw agents show <name> <extra>` and `claw skills show <name> <extra>` returned confusing `agent_not_found`/`skill_not_found` for the concatenated "name extra" string** — dogfooded 2026-05-27 on `18b4cee5`. `join_optional_args` passes all tokens as a space-joined string; both `show` handlers called `split_once(' ')` to extract the name but did not check if the remainder (after the first split) contained additional tokens. Extra positional args (including `--flags`) became part of the "name", silently mangling the lookup. Fix: added second `split_once(' ')` on the extracted name; if the result has two parts, return `unexpected_extra_args` with a usage hint. Valid single-name lookups are unaffected. Two new integration tests `agents_show_extra_positional_arg_returns_unexpected_extra_796`, `skills_show_extra_positional_arg_returns_unexpected_extra_796`. 59 CLI contract tests pass. [SCOPE: claw-code] Source: Jobdori agents/skills show extra-arg probe on `18b4cee5`, 2026-05-27.

View File

@ -2459,12 +2459,29 @@ pub fn handle_agents_slash_command_json(args: Option<&str>, cwd: &Path) -> std::
|| args.starts_with("info ")
|| args.starts_with("describe ") =>
{
let name = args
let name_raw = args
.split_once(' ')
.map(|(_, name)| name)
.unwrap_or_default()
.trim()
.to_lowercase();
// #796: extra positional args after the name (e.g. `agents show foo extra`)
// produced a confusing agent_not_found for "foo extra" instead of flagging
// the unexpected extra argument.
let (name, extra) = name_raw
.split_once(' ')
.map(|(n, e)| (n.to_string(), Some(e.to_string())))
.unwrap_or_else(|| (name_raw.clone(), None));
if let Some(extra_token) = extra {
return Ok(serde_json::json!({
"kind": "agents",
"action": "show",
"status": "error",
"error_kind": "unexpected_extra_args",
"unexpected": extra_token,
"hint": format!("Usage: claw agents show <name>\nUnexpected extra: '{extra_token}'"),
}));
}
let roots = discover_definition_roots(cwd, "agents");
let agents = load_agents_from_roots(&roots)?;
let matched: Vec<_> = agents
@ -2624,12 +2641,29 @@ pub fn handle_skills_slash_command_json(args: Option<&str>, cwd: &Path) -> std::
|| args.starts_with("info ")
|| args.starts_with("describe ") =>
{
let name = args
let name_raw = args
.split_once(' ')
.map(|(_, name)| name)
.unwrap_or_default()
.trim()
.to_lowercase();
// #796: extra positional args after the name (e.g. `skills show foo extra`)
// produced a confusing skill_not_found for "foo extra" instead of flagging
// the unexpected extra argument.
let (name, extra) = name_raw
.split_once(' ')
.map(|(n, e)| (n.to_string(), Some(e.to_string())))
.unwrap_or_else(|| (name_raw.clone(), None));
if let Some(extra_token) = extra {
return Ok(json!({
"kind": "skills",
"action": "show",
"status": "error",
"error_kind": "unexpected_extra_args",
"unexpected": extra_token,
"hint": format!("Usage: claw skills show <name>\nUnexpected extra: '{extra_token}'"),
}));
}
let roots = discover_skill_roots(cwd);
let skills = load_skills_from_roots(&roots)?;
let matched: Vec<_> = skills

View File

@ -3282,3 +3282,94 @@ fn skills_install_not_found_and_unsupported_action_have_hints_795() {
.expect("unsupported_skills_action must have non-null hint (#795)");
assert!(!h2.is_empty(), "hint must be non-empty");
}
#[test]
fn agents_show_extra_positional_arg_returns_unexpected_extra_796() {
// #796: `claw agents show <name> <extra>` treated the full "name extra" as a single
// agent name, producing agent_not_found for "name extra" instead of flagging the
// unexpected extra argument. Fix: detect space-containing "name" and return
// unexpected_extra_args with usage hint.
let root = unique_temp_dir("agents-show-extra-796");
fs::create_dir_all(&root).expect("temp dir");
std::process::Command::new("git")
.args(["init", "-q"])
.current_dir(&root)
.output()
.ok();
let output = run_claw(
&root,
&[
"--output-format",
"json",
"agents",
"show",
"some-agent",
"--extra-flag",
],
&[],
);
assert!(
!output.status.success(),
"agents show with extra arg must exit non-zero (#796)"
);
let stdout = String::from_utf8_lossy(&output.stdout);
let j: serde_json::Value =
serde_json::from_str(stdout.trim()).expect("agents show extra arg should emit valid JSON");
assert_eq!(
j["error_kind"], "unexpected_extra_args",
"agents show extra arg should return unexpected_extra_args, got {:?}",
j["error_kind"]
);
let h = j["hint"]
.as_str()
.expect("unexpected_extra_args must have hint (#796)");
assert!(
h.contains("claw agents show") || h.contains("Usage"),
"hint should reference usage, got: {h:?}"
);
}
#[test]
fn skills_show_extra_positional_arg_returns_unexpected_extra_796() {
// #796: same gap as agents — `claw skills show <name> <extra>` treated "name extra"
// as a single skill name → skill_not_found. Fix: detect space-containing name.
let root = unique_temp_dir("skills-show-extra-796");
fs::create_dir_all(&root).expect("temp dir");
std::process::Command::new("git")
.args(["init", "-q"])
.current_dir(&root)
.output()
.ok();
let output = run_claw(
&root,
&[
"--output-format",
"json",
"skills",
"show",
"some-skill",
"--extra-flag",
],
&[],
);
assert!(
!output.status.success(),
"skills show with extra arg must exit non-zero (#796)"
);
let stdout = String::from_utf8_lossy(&output.stdout);
let j: serde_json::Value =
serde_json::from_str(stdout.trim()).expect("skills show extra arg should emit valid JSON");
assert_eq!(
j["error_kind"], "unexpected_extra_args",
"skills show extra arg should return unexpected_extra_args, got {:?}",
j["error_kind"]
);
assert!(
j["hint"]
.as_str()
.is_some_and(|h| h.contains("claw skills show") || h.contains("Usage")),
"hint should reference usage (#796)"
);
}