mirror of
https://github.com/instructkr/claude-code.git
synced 2026-05-28 16:36:45 +00:00
Suppress config warnings on JSON local surfaces (#3192)
This commit is contained in:
@@ -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,
|
||||
|
||||
@@ -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 } => {
|
||||
@@ -6408,7 +6439,15 @@ impl LiveCli {
|
||||
}
|
||||
}
|
||||
}
|
||||
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)
|
||||
@@ -6735,7 +6774,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 +9173,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 +12846,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"),
|
||||
|
||||
@@ -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, &[])
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user