Route plugins list JSON parse errors to stdout (#3194)

This commit is contained in:
Bellman 2026-05-28 22:35:58 +09:00 committed by GitHub
parent 69b8b367c1
commit 0800d7ae88
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 87 additions and 27 deletions

View File

@ -6433,6 +6433,26 @@ impl LiveCli {
if action.as_deref() == Some("list") { if action.as_deref() == Some("list") {
if let Some(filter) = target.as_deref() { if let Some(filter) = target.as_deref() {
if filter.starts_with('-') { if filter.starts_with('-') {
if matches!(output_format, CliOutputFormat::Json) {
// ROADMAP #817: this is a handled local inventory parse error.
// Keep it on stdout in JSON mode so `plugins list --` matches the
// sibling JSON inventory/local surfaces instead of falling through
// to the top-level stderr error path.
let obj = json!({
"type": "error",
"kind": "plugin",
"action": "list",
"status": "error",
"error_kind": "cli_parse",
"error": format!("unknown option for `claw plugins list`: {filter}"),
"message": format!("unknown option for `claw plugins list`: {filter}"),
"unexpected": filter,
"hint": "Usage: claw plugins list [<filter>]\nFilters are id substrings, not flags.",
"exit_code": 1,
});
println!("{}", serde_json::to_string_pretty(&obj)?);
std::process::exit(1);
}
return Err(format!( return Err(format!(
"unknown option for `claw plugins list`: {filter}\nUsage: claw plugins list [<filter>]\nFilters are id substrings, not flags." "unknown option for `claw plugins list`: {filter}\nUsage: claw plugins list [<filter>]\nFilters are id substrings, not flags."
).into()); ).into());
@ -6513,20 +6533,6 @@ impl LiveCli {
} }
} else if is_list_action { } else if is_list_action {
if let Some(filter) = target { if let Some(filter) = target {
// #793: flag-shaped tokens silently became substring filters on
// plugins list, returning empty success instead of an error.
if filter.starts_with('-') {
let obj = json!({
"kind": "plugin",
"action": "list",
"status": "error",
"error_kind": "unknown_option",
"unexpected": filter,
"hint": "Usage: claw plugins list [<filter>]\nFilters are id substrings, not flags.",
});
println!("{}", serde_json::to_string_pretty(&obj)?);
std::process::exit(1);
}
let needle = filter.to_lowercase(); let needle = filter.to_lowercase();
payload payload
.plugins .plugins

View File

@ -3338,11 +3338,12 @@ fn skills_list_flag_shaped_filter_returns_unknown_option_792() {
} }
#[test] #[test]
fn plugins_list_flag_shaped_filter_returns_unknown_option_793() { fn plugins_list_flag_shaped_filter_returns_cli_parse_on_stdout_793_817() {
// #793: `claw plugins list --bogus-flag` silently returned status:"ok" with empty // #793: `claw plugins list --bogus-flag` silently returned status:"ok" with empty
// plugins list instead of an error. The list filter branch in print_plugins treated // plugins list instead of an error. The list filter branch in print_plugins treated
// "--bogus-flag" as an id substring filter and found no matches, producing a false-positive. // "--bogus-flag" as an id substring filter and found no matches, producing a false-positive.
// Fix: added flag-prefix guard; filter tokens starting with "-" now return unknown_option. // #817: in JSON mode, handled local parse errors now return error_kind:"cli_parse"
// on stdout with stderr empty.
let root = unique_temp_dir("plugins-list-flag-793"); let root = unique_temp_dir("plugins-list-flag-793");
fs::create_dir_all(&root).expect("temp dir"); fs::create_dir_all(&root).expect("temp dir");
std::process::Command::new("git") std::process::Command::new("git")
@ -3366,19 +3367,16 @@ fn plugins_list_flag_shaped_filter_returns_unknown_option_793() {
!output.status.success(), !output.status.success(),
"plugins list --unknown-flag must exit non-zero (#793)" "plugins list --unknown-flag must exit non-zero (#793)"
); );
// #803: the early flag guard now returns Err before the JSON branch, assert_eq!(output.status.code(), Some(1), "exit code must be 1 (#817)");
// so the error envelope goes to stderr via the main error handler. // #817: handled JSON local parse errors stay on stdout, with stderr empty.
let stderr = String::from_utf8_lossy(&output.stderr);
let j: serde_json::Value = stderr
.lines()
.find(|l| l.trim_start().starts_with('{'))
.and_then(|l| serde_json::from_str(l).ok())
.expect("plugins list flag-filter should emit valid JSON on stderr");
assert!( assert!(
j["error_kind"] == "unknown_option" || j["error_kind"] == "cli_parse", output.stderr.is_empty(),
"plugins list flag-shaped filter must return typed error, got {:?}", "plugins list flag-filter JSON error must keep stderr empty (#817), got: {}",
j["error_kind"] String::from_utf8_lossy(&output.stderr)
); );
let j: serde_json::Value = serde_json::from_slice(&output.stdout)
.expect("plugins list flag-filter should emit valid JSON on stdout");
assert_eq!(j["error_kind"], "cli_parse");
assert_eq!(j["status"], "error"); assert_eq!(j["status"], "error");
let h = j["hint"] let h = j["hint"]
.as_str() .as_str()
@ -3697,6 +3695,62 @@ fn plugins_extra_args_have_non_null_hint_797() {
); );
} }
#[test]
fn plugins_list_trailing_dash_json_error_uses_stdout_817() {
// ROADMAP #817: JSON inventory/local parse errors are machine-readable on
// stdout. `plugins list --` used to route through the top-level error path,
// leaving stdout empty and writing the JSON envelope to stderr.
let root = unique_temp_dir("plugins-list-dash-817");
fs::create_dir_all(&root).expect("temp dir");
let output = run_claw(
&root,
&["--output-format", "json", "plugins", "list", "--"],
&[],
);
assert!(
!output.status.success(),
"plugins list -- must exit non-zero (#817)"
);
assert_eq!(output.status.code(), Some(1), "exit code must be 1 (#817)");
assert!(
output.stderr.is_empty(),
"JSON parse error must keep stderr empty (#817), got: {}",
String::from_utf8_lossy(&output.stderr)
);
let j: serde_json::Value =
serde_json::from_slice(&output.stdout).expect("stdout should be JSON error (#817)");
assert_eq!(j["kind"], "plugin");
assert_eq!(j["action"], "list");
assert_eq!(j["status"], "error");
assert_eq!(j["error_kind"], "cli_parse");
assert_eq!(j["unexpected"], "--");
}
#[test]
fn plugins_list_trailing_dash_text_error_stays_on_stderr_817() {
let root = unique_temp_dir("plugins-list-dash-text-817");
fs::create_dir_all(&root).expect("temp dir");
let output = run_claw(&root, &["plugins", "list", "--"], &[]);
assert!(
!output.status.success(),
"plugins list -- text mode must exit non-zero (#817)"
);
assert_eq!(output.status.code(), Some(1), "exit code must be 1 (#817)");
assert!(
output.stdout.is_empty(),
"text parse error should not emit stdout (#817), got: {}",
String::from_utf8_lossy(&output.stdout)
);
let stderr = String::from_utf8_lossy(&output.stderr);
assert!(stderr.contains("[error-kind: cli_parse]"), "{stderr}");
assert!(
stderr.contains("unknown option for `claw plugins list`: --"),
"{stderr}"
);
}
#[test] #[test]
fn empty_prompt_has_non_null_hint_798() { fn empty_prompt_has_non_null_hint_798() {
// #798: `claw --output-format json ""` returned empty_prompt + hint:null. // #798: `claw --output-format json ""` returned empty_prompt + hint:null.