Compare commits

...

4 Commits

Author SHA1 Message Date
Bellman
0800d7ae88 Route plugins list JSON parse errors to stdout (#3194) 2026-05-28 22:35:58 +09:00
Bellman
69b8b367c1 docs: record plugins trailing dash json routing (#3193) 2026-05-28 21:35:25 +09:00
Bellman
9494e3c26f Suppress config warnings on JSON local surfaces (#3192) 2026-05-28 20:34:18 +09:00
Bellman
ed3a616e62 docs: record global json warning leak (#3191) 2026-05-28 18:36:30 +09:00
4 changed files with 264 additions and 41 deletions

View File

@@ -7814,3 +7814,15 @@ Original filing (2026-04-18): the session emitted `SessionStart hook (completed)
**Required fix shape.** In JSON mode for config/list surfaces that already include `warnings[]`, suppress eager prose emission of the same config warning on stderr or mark it as already collected. Text mode should keep the human stderr warning. Add regression coverage asserting `claw --output-format json config` returns exactly one structured warning and zero duplicate `enabledPlugins` prose lines on stderr.
**Acceptance.** With a deprecated `enabledPlugins` key present, `claw --output-format json config` exits 0, stdout parses from byte 0 and includes `warnings[]`, and stderr has no duplicate deprecation warning for the same file/key. [SCOPE: claw-code]
816. **JSON-mode local/list surfaces still leak deprecated config prose warnings on stderr outside `config`** — dogfooded 2026-05-28 09:30 on `main` `89e7f415a` after #3190. `./target/debug/claw --output-format json config` is now fixed (`rc=0`, parseable stdout, `warnings[]`, stderr empty), but sibling JSON surfaces still emit the same app-level config warning to stderr when `~/.claw/settings.json` contains deprecated `enabledPlugins`: `plugins list` (`kind:"plugin"`), `mcp list` (`kind:"mcp"`), and `doctor` (`kind:"doctor"`) all return parseable JSON with `rc=0` while stderr contains `enabledPlugins is deprecated`. `skills list` and `version` stay clean. This leaves machine consumers with a global JSON-mode cleanliness gap even after the config-specific duplicate was fixed.
**Required fix shape.** Treat JSON output mode as a global app-level diagnostic routing contract: local/list/status surfaces that successfully return structured JSON should not write config deprecation prose to stderr. Either collect those warnings into each relevant JSON envelope where a warnings field exists, or suppress config-warning emission during JSON-mode preloading/default resolution for surfaces that cannot represent warnings yet. Preserve human stderr warnings in text mode.
**Acceptance.** With deprecated `enabledPlugins` present, `claw --output-format json plugins list`, `claw --output-format json mcp list`, and `claw --output-format json doctor` exit 0, stdout parses from byte 0, and stderr contains zero `enabledPlugins is deprecated` app-level warning lines. Text mode still prints the warning. [SCOPE: claw-code]
817. **`claw --output-format json plugins list --` writes its JSON error envelope to stderr while sibling local inventory commands use stdout** — dogfooded 2026-05-28 12:30 on `main` `9494e3c26`. Trailing bare `--` is a useful parser edge because automation sometimes injects delimiter sentinels. `agents list --` and `skills list --` return rc 1 with parseable JSON on stdout and empty stderr. `mcp list --` also returns a parseable JSON error on stdout. `config --` returns rc 0 with a structured config error on stdout. But `plugins list --` returns rc 1, stdout empty, and writes the JSON error envelope to stderr: `{"action":"abort","error":"unknown option for `claw plugins list`: --", ...}`. This is machine-readable, but channel-inconsistent and surprising for JSON-mode consumers that read stdout for command payloads.
**Required fix shape.** Align `plugins list` parse-error routing with the other JSON inventory/local surfaces: in JSON mode, print the structured CLI error envelope to stdout and keep stderr empty for this handled parse error. Preserve text-mode stderr behavior. Add regression coverage for `claw --output-format json plugins list --` asserting rc 1, stdout parseable JSON with `error_kind:"cli_parse"`, and empty stderr.
**Acceptance.** `claw --output-format json plugins list --` exits 1, stdout parses from byte 0 as the existing JSON error envelope, stderr is empty, and text mode still reports the parse error to stderr. [SCOPE: claw-code]

View File

@@ -7,7 +7,7 @@ use std::path::{Path, PathBuf};
use plugins::{PluginError, PluginLoadFailure, PluginManager, PluginSummary};
use runtime::{
compact_session, CompactionConfig, ConfigLoader, ConfigSource, McpOAuthConfig, McpServerConfig,
ScopedMcpServerConfig, Session,
RuntimeConfig, ScopedMcpServerConfig, Session,
};
use serde_json::{json, Value};
@@ -2542,6 +2542,14 @@ pub fn handle_mcp_slash_command_json(
render_mcp_report_json_for(&loader, cwd, args)
}
fn load_runtime_config_without_stderr_warnings(
loader: &ConfigLoader,
) -> Result<RuntimeConfig, runtime::ConfigError> {
loader
.load_collecting_warnings()
.map(|(runtime_config, _warnings)| runtime_config)
}
pub fn handle_skills_slash_command(args: Option<&str>, cwd: &Path) -> std::io::Result<String> {
if let Some(args) = normalize_optional_args(args) {
if let Some(help_path) = help_path_from_args(args) {
@@ -2994,7 +3002,7 @@ fn render_mcp_report_json_for(
// failure, emit top-level `status: "degraded"` with
// `config_load_error`, empty servers[], and exit 0. On clean
// runs, the existing serializer adds `status: "ok"` below.
match loader.load() {
match load_runtime_config_without_stderr_warnings(loader) {
Ok(runtime_config) => {
let mut value =
render_mcp_summary_report_json(cwd, runtime_config.mcp().servers());
@@ -3030,7 +3038,7 @@ fn render_mcp_report_json_for(
return Ok(render_mcp_usage_json(Some(args)));
}
// #144: same degradation pattern for show action.
match loader.load() {
match load_runtime_config_without_stderr_warnings(loader) {
Ok(runtime_config) => {
let mut value = render_mcp_server_report_json(
cwd,

View File

@@ -2414,6 +2414,24 @@ impl DiagnosticCheck {
}
}
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
enum ConfigWarningMode {
EmitStderr,
SuppressStderr,
}
fn load_config_with_warning_mode(
loader: &ConfigLoader,
mode: ConfigWarningMode,
) -> Result<runtime::RuntimeConfig, runtime::ConfigError> {
match mode {
ConfigWarningMode::EmitStderr => loader.load(),
ConfigWarningMode::SuppressStderr => loader
.load_collecting_warnings()
.map(|(runtime_config, _warnings)| runtime_config),
}
}
#[derive(Debug, Clone, PartialEq, Eq)]
struct DoctorReport {
checks: Vec<DiagnosticCheck>,
@@ -2503,10 +2521,12 @@ fn render_diagnostic_check(check: &DiagnosticCheck) -> String {
lines.join("\n")
}
fn render_doctor_report() -> Result<DoctorReport, Box<dyn std::error::Error>> {
fn render_doctor_report(
config_warning_mode: ConfigWarningMode,
) -> Result<DoctorReport, Box<dyn std::error::Error>> {
let cwd = env::current_dir()?;
let config_loader = ConfigLoader::default_for(&cwd);
let config = config_loader.load();
let config = load_config_with_warning_mode(&config_loader, config_warning_mode);
let discovered_config = config_loader.discover();
let project_context = ProjectContext::discover_with_git(&cwd, DEFAULT_DATE)?;
let (project_root, git_branch) =
@@ -2559,7 +2579,10 @@ fn render_doctor_report() -> Result<DoctorReport, Box<dyn std::error::Error>> {
}
fn run_doctor(output_format: CliOutputFormat) -> Result<(), Box<dyn std::error::Error>> {
let report = render_doctor_report()?;
let report = render_doctor_report(match output_format {
CliOutputFormat::Json => ConfigWarningMode::SuppressStderr,
CliOutputFormat::Text => ConfigWarningMode::EmitStderr,
})?;
let message = report.render();
match output_format {
CliOutputFormat::Text => println!("{message}"),
@@ -4641,7 +4664,12 @@ fn run_resume_command(
_ => {}
}
let cwd = env::current_dir()?;
let payload = plugins_command_payload_for(&cwd, action.as_deref(), target.as_deref())?;
let payload = plugins_command_payload_for(
&cwd,
action.as_deref(),
target.as_deref(),
ConfigWarningMode::EmitStderr,
)?;
let action_str = action.as_deref().unwrap_or("list");
let enabled_count = payload
.plugins
@@ -4675,7 +4703,7 @@ fn run_resume_command(
})
}
SlashCommand::Doctor => {
let report = render_doctor_report()?;
let report = render_doctor_report(ConfigWarningMode::EmitStderr)?;
Ok(ResumeCommandOutcome {
session: session.clone(),
message: Some(report.render()),
@@ -5981,7 +6009,10 @@ impl LiveCli {
false
}
SlashCommand::Doctor => {
println!("{}", render_doctor_report()?.render());
println!(
"{}",
render_doctor_report(ConfigWarningMode::EmitStderr)?.render()
);
false
}
SlashCommand::History { count } => {
@@ -6402,13 +6433,41 @@ impl LiveCli {
if action.as_deref() == Some("list") {
if let Some(filter) = target.as_deref() {
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!(
"unknown option for `claw plugins list`: {filter}\nUsage: claw plugins list [<filter>]\nFilters are id substrings, not flags."
).into());
}
}
}
let payload = plugins_command_payload_for(&cwd, action, target)?;
let payload = plugins_command_payload_for(
&cwd,
action,
target,
match output_format {
CliOutputFormat::Json => ConfigWarningMode::SuppressStderr,
CliOutputFormat::Text => ConfigWarningMode::EmitStderr,
},
)?;
match output_format {
CliOutputFormat::Text => {
// #806: text-mode show must return error when plugin not found (parity with JSON)
@@ -6474,20 +6533,6 @@ impl LiveCli {
}
} else if is_list_action {
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();
payload
.plugins
@@ -6735,7 +6780,8 @@ impl LiveCli {
target: Option<&str>,
) -> Result<bool, Box<dyn std::error::Error>> {
let cwd = env::current_dir()?;
let payload = plugins_command_payload_for(&cwd, action, target)?;
let payload =
plugins_command_payload_for(&cwd, action, target, ConfigWarningMode::EmitStderr)?;
println!("{}", payload.message);
if payload.reload_runtime {
self.reload_runtime_features()?;
@@ -9133,9 +9179,11 @@ fn plugins_command_payload_for(
cwd: &Path,
action: Option<&str>,
target: Option<&str>,
config_warning_mode: ConfigWarningMode,
) -> Result<PluginsCommandPayload, Box<dyn std::error::Error>> {
let loader = ConfigLoader::default_for(cwd);
let (runtime_config, config_load_error) = match loader.load() {
let loaded_config = load_config_with_warning_mode(&loader, config_warning_mode);
let (runtime_config, config_load_error) = match loaded_config {
Ok(runtime_config) => (runtime_config, None),
Err(error) => (runtime::RuntimeConfig::empty(), Some(error.to_string())),
};
@@ -12804,8 +12852,13 @@ mod tests {
let previous_config_home = std::env::var("CLAW_CONFIG_HOME").ok();
std::env::set_var("CLAW_CONFIG_HOME", &config_home);
let payload = super::plugins_command_payload_for(&cwd, None, None)
.expect("plugins list should not hard-fail on malformed MCP config");
let payload = super::plugins_command_payload_for(
&cwd,
None,
None,
super::ConfigWarningMode::EmitStderr,
)
.expect("plugins list should not hard-fail on malformed MCP config");
match previous_config_home {
Some(value) => std::env::set_var("CLAW_CONFIG_HOME", value),
None => std::env::remove_var("CLAW_CONFIG_HOME"),

View File

@@ -1319,6 +1319,102 @@ fn config_json_reports_deprecations_structurally_without_stderr_duplicate_815()
);
}
#[test]
fn local_json_surfaces_suppress_config_deprecation_stderr_816() {
let root = unique_temp_dir("global-json-warning-816");
let config_home = root.join("config-home");
let home = root.join("home");
fs::create_dir_all(&config_home).expect("config home should exist");
fs::create_dir_all(&home).expect("home should exist");
fs::write(
config_home.join("settings.json"),
r#"{"enabledPlugins": {}}"#,
)
.expect("deprecated config fixture should write");
let envs = [
(
"CLAW_CONFIG_HOME",
config_home.to_str().expect("utf8 config home"),
),
("HOME", home.to_str().expect("utf8 home")),
];
for (args, expected_kind, expected_action) in [
(
&["--output-format", "json", "plugins", "list"][..],
"plugin",
"list",
),
(
&["--output-format", "json", "mcp", "list"][..],
"mcp",
"list",
),
(
&["--output-format", "json", "doctor"][..],
"doctor",
"doctor",
),
] {
let output = run_claw(&root, args, &envs);
assert!(
output.status.success(),
"args={args:?}\nstdout:\n{}\n\nstderr:\n{}",
String::from_utf8_lossy(&output.stdout),
String::from_utf8_lossy(&output.stderr)
);
let parsed: Value =
serde_json::from_slice(&output.stdout).expect("stdout should be valid JSON");
assert_eq!(parsed["kind"], expected_kind, "args={args:?}");
assert_eq!(parsed["action"], expected_action, "args={args:?}");
assert!(
matches!(parsed["status"].as_str(), Some("ok" | "warn")),
"args={args:?} should report successful local status: {parsed}"
);
let stderr = String::from_utf8(output.stderr).expect("stderr utf8");
assert!(
!stderr.contains("field \"enabledPlugins\" is deprecated"),
"successful JSON surface must not leak config deprecation prose to stderr for args={args:?}:\n{stderr}"
);
}
}
#[test]
fn local_text_surface_preserves_config_deprecation_stderr_816() {
let root = unique_temp_dir("global-text-warning-816");
let config_home = root.join("config-home");
let home = root.join("home");
fs::create_dir_all(&config_home).expect("config home should exist");
fs::create_dir_all(&home).expect("home should exist");
fs::write(
config_home.join("settings.json"),
r#"{"enabledPlugins": {}}"#,
)
.expect("deprecated config fixture should write");
let envs = [
(
"CLAW_CONFIG_HOME",
config_home.to_str().expect("utf8 config home"),
),
("HOME", home.to_str().expect("utf8 home")),
];
let output = run_claw(&root, &["doctor"], &envs);
assert!(
output.status.success(),
"stdout:\n{}\n\nstderr:\n{}",
String::from_utf8_lossy(&output.stdout),
String::from_utf8_lossy(&output.stderr)
);
let stderr = String::from_utf8(output.stderr).expect("stderr utf8");
assert!(
stderr.contains("field \"enabledPlugins\" is deprecated"),
"text-mode doctor should preserve human config deprecation warnings on stderr"
);
}
fn assert_json_command(current_dir: &Path, args: &[&str]) -> Value {
assert_json_command_with_env(current_dir, args, &[])
}
@@ -3242,11 +3338,12 @@ fn skills_list_flag_shaped_filter_returns_unknown_option_792() {
}
#[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
// 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.
// 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");
fs::create_dir_all(&root).expect("temp dir");
std::process::Command::new("git")
@@ -3270,19 +3367,16 @@ fn plugins_list_flag_shaped_filter_returns_unknown_option_793() {
!output.status.success(),
"plugins list --unknown-flag must exit non-zero (#793)"
);
// #803: the early flag guard now returns Err before the JSON branch,
// so the error envelope goes to stderr via the main error handler.
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_eq!(output.status.code(), Some(1), "exit code must be 1 (#817)");
// #817: handled JSON local parse errors stay on stdout, with stderr empty.
assert!(
j["error_kind"] == "unknown_option" || j["error_kind"] == "cli_parse",
"plugins list flag-shaped filter must return typed error, got {:?}",
j["error_kind"]
output.stderr.is_empty(),
"plugins list flag-filter JSON 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("plugins list flag-filter should emit valid JSON on stdout");
assert_eq!(j["error_kind"], "cli_parse");
assert_eq!(j["status"], "error");
let h = j["hint"]
.as_str()
@@ -3601,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]
fn empty_prompt_has_non_null_hint_798() {
// #798: `claw --output-format json ""` returned empty_prompt + hint:null.