fix(#789): agents show and plugins show not-found now exit 1; parity with skills (#788) and mcp (#68)

This commit is contained in:
YeonGyu-Kim
2026-05-27 10:07:51 +09:00
parent abdbf61acf
commit e4c3c1aa80
3 changed files with 93 additions and 6 deletions

View File

@@ -7743,3 +7743,5 @@ Original filing (2026-04-18): the session emitted `SessionStart hook (completed)
787. **`claw --resume /tmp` (directory path) returned `error_kind:"session_load_failed"` + `hint:null`; resume error emission sites didn't apply `fallback_hint_for_error_kind`** — dogfooded 2026-05-27 on `22b423b6`. Two gaps: (1) the OS error `"Is a directory (os error 21)"` had no classifier arm, falling to generic `session_load_failed`; (2) both resume error emission paths (session load at line 3338, command execution at line 3484) called `split_error_hint` but not `fallback_hint_for_error_kind`, so API-layer errors with no `\n` always got `hint:null`. Fix: added `session_path_is_directory` classifier arm for `"Is a directory"` / `"os error 21"` messages; added `fallback_hint_for_error_kind` fallback to both resume error sites; added `session_path_is_directory` and `session_load_failed` to `fallback_hint_for_error_kind`. Unit test + integration test `resume_directory_path_returns_typed_kind_and_hint_787`. 46 CLI contract tests pass. [SCOPE: claw-code] Source: Jobdori resume-path probe on `22b423b6`, 2026-05-27.
788. **`claw --output-format json skills show <not-found>` emitted two JSON objects — one from the skills handler, one duplicate from the top-level error path** — dogfooded 2026-05-27 on `113145a4`. `print_skills` in JSON mode called `println!` to emit the `skill_not_found` error envelope, then returned `Err(...)`. The `?` propagation triggered the top-level error handler which emitted a second `action:"abort"` JSON envelope on stderr. Callers reading both stdout and stderr got two JSON objects with the same `error_kind` but different `action` fields — the first was the authoritative response, the second was a duplicate. Fix: replaced `return Err(...)` with `std::process::exit(1)` after the skills error JSON is emitted, mirroring the existing `is_help_action` guard pattern. Integration test `skills_show_not_found_emits_single_json_object_788` asserts exactly 1 JSON object on stdout and no JSON on stderr. 47 CLI contract tests pass. [SCOPE: claw-code] Source: Jobdori skills double-emission probe on `113145a4`, 2026-05-27.
789. **`claw --output-format json agents show <not-found>` and `plugins show <not-found>` both returned exit 0 despite `status:"error"` in the JSON** — dogfooded 2026-05-27 on `abdbf61a`. Skills was fixed in #788 (exit 1 via process::exit). Agents and plugins had the identical gap: `print_agents` had no error check at all (just println + Ok(())); `print_plugins`'s not-found branch used `return Ok(())`. MCP was already fixed in an earlier cycle (#68). Fix: added `is_error` check in `print_agents` JSON path (exit 1 when status=="error"); changed plugins not-found branch from `return Ok(())` to `std::process::exit(1)`. Existing `inventory_commands_emit_structured_json_when_requested` test updated to use `run_claw` directly for the not-found case. Two new tests added: `agents_show_not_found_exits_nonzero_789`, `plugins_show_not_found_exits_nonzero_789`. 49 CLI contract tests pass. [SCOPE: claw-code] Source: Jobdori exit-code consistency probe on `abdbf61a`, 2026-05-27.

View File

@@ -6281,10 +6281,17 @@ impl LiveCli {
let cwd = env::current_dir()?;
match output_format {
CliOutputFormat::Text => println!("{}", handle_agents_slash_command(args, &cwd)?),
CliOutputFormat::Json => println!(
"{}",
serde_json::to_string_pretty(&handle_agents_slash_command_json(args, &cwd)?)?
),
CliOutputFormat::Json => {
let value = handle_agents_slash_command_json(args, &cwd)?;
// #789: parity with print_mcp/#788 print_skills — exit 1 when envelope
// reports an error so automation can rely on exit code instead of
// parsing the JSON status field.
let is_error = value.get("status").and_then(|v| v.as_str()) == Some("error");
println!("{}", serde_json::to_string_pretty(&value)?);
if is_error {
std::process::exit(1);
}
}
}
Ok(())
}
@@ -6428,7 +6435,8 @@ impl LiveCli {
"hint": "Run `claw plugins list` to see available plugins.",
});
println!("{}", serde_json::to_string_pretty(&obj)?);
return Ok(());
// #789: exit 1 on not-found so automation can rely on exit code
std::process::exit(1);
}
}
}

View File

@@ -203,7 +203,9 @@ fn inventory_commands_emit_structured_json_when_requested() {
isolated_codex.to_str().expect("utf8 codex home"),
),
];
let agents_show_missing = assert_json_command_with_env(
// #789: agents show not-found now exits 1 (parity with skills #788);
// use run_claw directly instead of assert_json_command_with_env which checks success.
let agents_show_out = run_claw(
&root,
&[
"--output-format",
@@ -214,6 +216,12 @@ fn inventory_commands_emit_structured_json_when_requested() {
],
&agents_show_env,
);
assert!(
!agents_show_out.status.success(),
"agents show not-found must exit non-zero"
);
let agents_show_missing: serde_json::Value =
serde_json::from_slice(&agents_show_out.stdout).expect("agents show stdout should be json");
assert_eq!(agents_show_missing["kind"], "agents", "agents show kind");
assert_eq!(agents_show_missing["action"], "show", "agents show action");
assert_eq!(
@@ -2765,3 +2773,72 @@ fn skills_show_not_found_emits_single_json_object_788() {
);
assert_eq!(json_objects[0]["status"], "error");
}
#[test]
fn agents_show_not_found_exits_nonzero_789() {
// #789: `claw --output-format json agents show <not-found>` returned exit 0 despite
// emitting status:"error". print_agents had no error check — just println + Ok(()).
// Skills was fixed in #788 (exit 1 via process::exit); agents/plugins had the same gap.
let root = unique_temp_dir("agents-show-exit-789");
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",
"no-such-agent-xyz-789",
],
&[],
);
assert!(
!output.status.success(),
"agents show not-found must exit non-zero (#789), got exit 0"
);
let stdout = String::from_utf8_lossy(&output.stdout);
let j: serde_json::Value =
serde_json::from_str(stdout.trim()).expect("agents show should emit valid JSON");
assert_eq!(j["error_kind"], "agent_not_found");
assert_eq!(j["status"], "error");
}
#[test]
fn plugins_show_not_found_exits_nonzero_789() {
// #789: same as agents — `claw --output-format json plugins show <not-found>` exited 0
// despite status:"error". The not-found branch used `return Ok(())` instead of exit(1).
let root = unique_temp_dir("plugins-show-exit-789");
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",
"plugins",
"show",
"no-such-plugin-xyz-789",
],
&[],
);
assert!(
!output.status.success(),
"plugins show not-found must exit non-zero (#789), got exit 0"
);
let stdout = String::from_utf8_lossy(&output.stdout);
let j: serde_json::Value =
serde_json::from_str(stdout.trim()).expect("plugins show should emit valid JSON");
assert_eq!(j["error_kind"], "plugin_not_found");
assert_eq!(j["status"], "error");
}