diff --git a/ROADMAP.md b/ROADMAP.md index 12b50e42..0c34f8d6 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -6303,7 +6303,7 @@ Original filing (2026-04-18): the session emitted `SessionStart hook (completed) 422. **`export --output-format json` and `--resume latest` report the same "no managed sessions" scenario using two different `kind` codes — `no_managed_sessions` vs `session_load_failed` — making "no session found" undetectable by a single kind-code check** — dogfooded 2026-04-30 KST (UTC+9) by Jobdori on `e939777f`. Running `claw export --output-format json` with no session present returns (on stderr, exit 1): `{"error":"no managed sessions found in .claw/sessions//","hint":"Start \`claw\` to create a session, then rerun with \`--resume latest\`.\nNote: claw partitions sessions per workspace fingerprint; sessions from other CWDs are invisible.","kind":"no_managed_sessions","type":"error"}`. Running `claw --resume latest /status --output-format json` with no session present returns (on stderr, exit 1): `{"error":"failed to restore session: no managed sessions found in .claw/sessions//","hint":"Start \`claw\` to create a session, then rerun with \`--resume latest\`.\nNote: claw partitions sessions per workspace fingerprint; sessions from other CWDs are invisible.","kind":"session_load_failed","type":"error"}`. Both describe the same root condition — there are no sessions to operate on — but they expose it via different `kind` discriminants. Automation that checks `kind == "no_managed_sessions"` to detect a cold workspace will miss the `--resume` path's `session_load_failed`, and vice versa. A wrapper that guards "run with --resume only if a session exists" must special-case both codes. The hint text is identical between them, suggesting the messages are logically equivalent. Additionally neither code matches the proposed canonical names `session_not_found` / `session_load_failed` as stable `ErrorKind` discriminants described in ROADMAP #77's fix shape, which explicitly proposes typed error-kind codes for session lifecycle failures. **Required fix shape:** (a) unify "no sessions found for this workspace fingerprint" under a single canonical `kind` code — either `no_managed_sessions` or `session_not_found` — used consistently by every command path that encounters an empty session registry; (b) if `session_load_failed` is a more general category (covering e.g. corrupt session files, IO errors, schema version mismatches), it should nest a concrete `reason:"no_managed_sessions"` or `reason:"session_not_found"` sub-field so callers can distinguish "empty registry" from "found but unreadable"; (c) align with the canonical error-kind contract proposed in #77; (d) add regression coverage proving `export` and `--resume latest` in an empty workspace both return an error with the same top-level `kind` code. **Why this matters:** session guard-rails in orchestration need a single stable `kind` to detect cold workspaces without enumerating all possible no-session synonyms. Two divergent codes for the same condition make defensive automation brittle and contradict the promise of machine-readable error envelopes. Source: Jobdori live dogfood, `e939777f`, 2026-04-30 KST (UTC+9). -407. **`config --output-format json` returns `files[].loaded:false` with no `load_error`, `not_found`, or `skip_reason` field — automation cannot distinguish "file does not exist", "file exists but parse failed", and "file exists but was skipped by policy" from the same `loaded:false` value; also `loaded_files` and `merged_keys` are bare integers with no per-file attribution** — dogfooded 2026-04-30 by Jobdori on `e939777f`. Running `./claw --output-format json config` on a workspace with 5 discovered config files returns `{"kind":"config","cwd":"...","files":[{"loaded":false,"path":"/Users/yeongyu/.claw.json","source":"user"},{"loaded":true,"path":"/Users/yeongyu/.claw/settings.json","source":"user"},{"loaded":true,"path":"/Users/yeongyu/clawd/claw-code/.claw.json","source":"project"},{"loaded":false,"path":"/Users/yeongyu/clawd/claw-code/.claw/settings.json","source":"project"},{"loaded":false,"path":"/Users/yeongyu/clawd/claw-code/.claw/settings.local.json","source":"local"}],"loaded_files":2,"merged_keys":2}`. Three of five files have `loaded:false` with no accompanying `not_found:true`, `parse_error`, `io_error`, or `skip_reason`; automation must stat each path separately to guess why. Also `loaded_files:2` and `merged_keys:2` are bare counts — ambiguous whether `merged_keys:2` means 2 total top-level JSON keys across all files or 2 unique merged settings. **Required fix shape:** (a) add `not_found: bool` and optional `load_error: string` to each `files[]` entry so callers can distinguish missing, parse-broken, and policy-skipped files without filesystem probing; (b) document or rename `merged_keys` as `merged_setting_count` or `total_merged_keys` to remove the int-semantics ambiguity; (c) optionally add `merged_keys_by_file: [{path, keys}]` for attribution; (d) add regression coverage proving `files[]` entries with `loaded:false` carry at minimum `not_found` distinguishing non-existent paths from load failures. Source: Jobdori live dogfood, `e939777f`, 2026-04-30. +407. **DONE — `config --output-format json` returns structured file load states instead of bare `loaded:false` ambiguity** — fixed 2026-06-03 in `fix: report config file load statuses`. `claw config --output-format json` now emits `files[].status` (`loaded`, `not_found`, `skipped`, `load_error`), `reason`/`skip_reason`, and `detail` where applicable, plus top-level `load_error`, `merged_key_count`, and `merged_keys_meaning`. The config list surface is best-effort so one broken settings file no longer erases the rest of the discovery report, while section-specific config requests still preserve the typed nonzero parse-error envelope. Regression coverage: `inspect_classifies_missing_loaded_and_legacy_skipped_files`, `inspect_reports_parse_errors_but_keeps_valid_merged_config`, `config_json_reports_structured_unloaded_file_reasons_407`, `config_json_list_reports_parse_errors_without_dropping_file_statuses_407`, and existing `config_parse_error_has_typed_error_kind_and_hint_764`. 408. **`status --output-format json` `workspace.changed_files` is ambiguous — on a workspace with 5 untracked files, `changed_files:5`, `staged_files:0`, `unstaged_files:0`, `untracked_files:5`; it is unclear whether `changed_files` is the sum of all four git-status categories or only a subset; automation cannot tell if `changed_files:5` means "5 tracked modified" or "5 total non-clean files including untracked"** — dogfooded 2026-04-30 by Jobdori on `e939777f`. Running `./claw --output-format json status` returns `{"workspace":{"changed_files":5,"staged_files":0,"unstaged_files":0,"untracked_files":5,...}}` — `changed_files==untracked_files==5` with staged and unstaged both zero. The field name `changed_files` implies "modified tracked files" but the value equals the untracked count, not `staged+unstaged`. Without a comment or documented definition, automation must probe whether `changed_files = staged + unstaged` (excludes untracked) or `changed_files = staged + unstaged + untracked + conflicted` (total dirty). Also `git_state:"dirty · 5 files · 5 untracked"` repeats the same data as a prose string alongside the structured integer fields — redundant human-readable string alongside machine-readable integers. **Required fix shape:** (a) document and stabilize `changed_files` as either `tracked_dirty_count` (staged+unstaged only) or `total_non-clean_count` (staged+unstaged+untracked+conflicted) and rename to remove the ambiguity; (b) ensure a machine consumer can compute `is_clean` as a single boolean field without interpreting `git_state` prose; (c) deprecate or remove `git_state` prose string now that all its constituent counts are available as integers; (d) add regression coverage proving `changed_files` semantics against a workspace with staged, unstaged, untracked, and conflicted files. Source: Jobdori live dogfood, `e939777f`, 2026-04-30. diff --git a/rust/crates/runtime/src/config.rs b/rust/crates/runtime/src/config.rs index d165c17f..a532dca4 100644 --- a/rust/crates/runtime/src/config.rs +++ b/rust/crates/runtime/src/config.rs @@ -70,6 +70,46 @@ pub struct RuntimeConfig { feature_config: RuntimeFeatureConfig, } +/// Machine-readable load state for a discovered config file. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ConfigFileStatus { + Loaded, + NotFound, + Skipped, + LoadError, +} + +impl ConfigFileStatus { + #[must_use] + pub fn as_str(self) -> &'static str { + match self { + Self::Loaded => "loaded", + Self::NotFound => "not_found", + Self::Skipped => "skipped", + Self::LoadError => "load_error", + } + } +} + +/// Structured status for one discovered config file. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ConfigFileReport { + pub entry: ConfigEntry, + pub loaded: bool, + pub status: ConfigFileStatus, + pub reason: Option, + pub detail: Option, +} + +/// Best-effort inspection of the config discovery and load pipeline. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ConfigInspection { + pub files: Vec, + pub runtime_config: Option, + pub warnings: Vec, + pub load_error: Option, +} + /// Parsed plugin-related settings extracted from runtime config. #[derive(Debug, Clone, PartialEq, Eq, Default)] pub struct RuntimePluginConfig { @@ -347,7 +387,7 @@ impl ConfigLoader { for entry in self.discover() { crate::config_validate::check_unsupported_format(&entry.path)?; - let Some(parsed) = read_optional_json_object(&entry.path)? else { + let OptionalConfigFile::Loaded(parsed) = read_optional_json_object(&entry.path)? else { continue; }; let validation = crate::config_validate::validate_config_file( @@ -370,30 +410,7 @@ impl ConfigLoader { emit_config_warning_once(&warning.to_string()); } - let merged_value = JsonValue::Object(merged.clone()); - - let feature_config = RuntimeFeatureConfig { - hooks: parse_optional_hooks_config(&merged_value)?, - plugins: parse_optional_plugin_config(&merged_value)?, - mcp: McpConfigCollection { - servers: mcp_servers, - }, - oauth: parse_optional_oauth_config(&merged_value, "merged settings.oauth")?, - model: parse_optional_model(&merged_value), - aliases: parse_optional_aliases(&merged_value)?, - permission_mode: parse_optional_permission_mode(&merged_value)?, - permission_rules: parse_optional_permission_rules(&merged_value)?, - sandbox: parse_optional_sandbox_config(&merged_value)?, - provider_fallbacks: parse_optional_provider_fallbacks(&merged_value)?, - trusted_roots: parse_optional_trusted_roots(&merged_value)?, - rules_import: parse_optional_rules_import(&merged_value)?, - }; - - Ok(RuntimeConfig { - merged, - loaded_entries, - feature_config, - }) + build_runtime_config(merged, loaded_entries, mcp_servers) } /// Like [`load`] but also returns the list of validation warnings collected during @@ -409,7 +426,7 @@ impl ConfigLoader { for entry in self.discover() { crate::config_validate::check_unsupported_format(&entry.path)?; - let Some(parsed) = read_optional_json_object(&entry.path)? else { + let OptionalConfigFile::Loaded(parsed) = read_optional_json_object(&entry.path)? else { continue; }; let validation = crate::config_validate::validate_config_file( @@ -428,32 +445,200 @@ impl ConfigLoader { loaded_entries.push(entry); } - let merged_value = JsonValue::Object(merged.clone()); - - let feature_config = RuntimeFeatureConfig { - hooks: parse_optional_hooks_config(&merged_value)?, - plugins: parse_optional_plugin_config(&merged_value)?, - mcp: McpConfigCollection { - servers: mcp_servers, - }, - oauth: parse_optional_oauth_config(&merged_value, "merged settings.oauth")?, - model: parse_optional_model(&merged_value), - aliases: parse_optional_aliases(&merged_value)?, - permission_mode: parse_optional_permission_mode(&merged_value)?, - permission_rules: parse_optional_permission_rules(&merged_value)?, - sandbox: parse_optional_sandbox_config(&merged_value)?, - provider_fallbacks: parse_optional_provider_fallbacks(&merged_value)?, - trusted_roots: parse_optional_trusted_roots(&merged_value)?, - rules_import: parse_optional_rules_import(&merged_value)?, - }; - - let config = RuntimeConfig { - merged, - loaded_entries, - feature_config, - }; + let config = build_runtime_config(merged, loaded_entries, mcp_servers)?; Ok((config, all_warnings)) } + + /// Inspect every discovered config path and return per-file status details. + /// Unlike [`Self::load`], this is best-effort: invalid files are reported in + /// `files[]` and skipped from the merged runtime view so JSON config callers can + /// show the whole discovery picture without collapsing every unloaded path to + /// `loaded:false`. + #[must_use] + pub fn inspect_collecting_warnings(&self) -> ConfigInspection { + let mut merged = BTreeMap::new(); + let mut loaded_entries = Vec::new(); + let mut mcp_servers = BTreeMap::new(); + let mut warnings = Vec::new(); + let mut files = Vec::new(); + let mut load_error = None; + + for entry in self.discover() { + if let Err(error) = crate::config_validate::check_unsupported_format(&entry.path) { + let detail = error.to_string(); + load_error.get_or_insert_with(|| detail.clone()); + files.push(ConfigFileReport::load_error( + entry, + "unsupported_format", + detail, + )); + continue; + } + + let parsed = match read_optional_json_object(&entry.path) { + Ok(OptionalConfigFile::Loaded(parsed)) => parsed, + Ok(OptionalConfigFile::NotFound) => { + files.push(ConfigFileReport::not_found(entry)); + continue; + } + Ok(OptionalConfigFile::Skipped { reason, detail }) => { + files.push(ConfigFileReport::skipped(entry, reason, detail)); + continue; + } + Err(error) => { + let reason = config_error_reason(&error).to_string(); + let detail = error.to_string(); + load_error.get_or_insert_with(|| detail.clone()); + files.push(ConfigFileReport::load_error(entry, reason, detail)); + continue; + } + }; + + let validation = crate::config_validate::validate_config_file( + &parsed.object, + &parsed.source, + &entry.path, + ); + if !validation.is_ok() { + let detail = validation.errors[0].to_string(); + load_error.get_or_insert_with(|| detail.clone()); + files.push(ConfigFileReport::load_error( + entry, + "validation_error", + detail, + )); + continue; + } + warnings.extend( + validation + .warnings + .iter() + .map(|warning| warning.to_string()), + ); + + if let Err(error) = validate_optional_hooks_config(&parsed.object, &entry.path) { + let detail = error.to_string(); + load_error.get_or_insert_with(|| detail.clone()); + files.push(ConfigFileReport::load_error( + entry, + "validation_error", + detail, + )); + continue; + } + + if let Err(error) = + merge_mcp_servers(&mut mcp_servers, entry.source, &parsed.object, &entry.path) + { + let detail = error.to_string(); + load_error.get_or_insert_with(|| detail.clone()); + files.push(ConfigFileReport::load_error(entry, "parse_error", detail)); + continue; + } + + deep_merge_objects(&mut merged, &parsed.object); + loaded_entries.push(entry.clone()); + files.push(ConfigFileReport::loaded(entry)); + } + + let runtime_config = match build_runtime_config(merged, loaded_entries, mcp_servers) { + Ok(config) => Some(config), + Err(error) => { + load_error.get_or_insert_with(|| error.to_string()); + None + } + }; + + ConfigInspection { + files, + runtime_config, + warnings, + load_error, + } + } +} + +impl ConfigFileReport { + fn loaded(entry: ConfigEntry) -> Self { + Self { + entry, + loaded: true, + status: ConfigFileStatus::Loaded, + reason: None, + detail: None, + } + } + + fn not_found(entry: ConfigEntry) -> Self { + Self { + entry, + loaded: false, + status: ConfigFileStatus::NotFound, + reason: Some("not_found".to_string()), + detail: None, + } + } + + fn skipped(entry: ConfigEntry, reason: String, detail: Option) -> Self { + Self { + entry, + loaded: false, + status: ConfigFileStatus::Skipped, + reason: Some(reason), + detail, + } + } + + fn load_error(entry: ConfigEntry, reason: impl Into, detail: String) -> Self { + Self { + entry, + loaded: false, + status: ConfigFileStatus::LoadError, + reason: Some(reason.into()), + detail: Some(detail), + } + } +} + +fn build_runtime_config( + merged: BTreeMap, + loaded_entries: Vec, + mcp_servers: BTreeMap, +) -> Result { + let merged_value = JsonValue::Object(merged.clone()); + + let feature_config = RuntimeFeatureConfig { + hooks: parse_optional_hooks_config(&merged_value)?, + plugins: parse_optional_plugin_config(&merged_value)?, + mcp: McpConfigCollection { + servers: mcp_servers, + }, + oauth: parse_optional_oauth_config(&merged_value, "merged settings.oauth")?, + model: parse_optional_model(&merged_value), + aliases: parse_optional_aliases(&merged_value)?, + permission_mode: parse_optional_permission_mode(&merged_value)?, + permission_rules: parse_optional_permission_rules(&merged_value)?, + sandbox: parse_optional_sandbox_config(&merged_value)?, + provider_fallbacks: parse_optional_provider_fallbacks(&merged_value)?, + trusted_roots: parse_optional_trusted_roots(&merged_value)?, + rules_import: parse_optional_rules_import(&merged_value)?, + }; + + Ok(RuntimeConfig { + merged, + loaded_entries, + feature_config, + }) +} + +fn config_error_reason(error: &ConfigError) -> &'static str { + match error { + ConfigError::Io(io_error) if io_error.kind() == std::io::ErrorKind::PermissionDenied => { + "permission_denied" + } + ConfigError::Io(_) => "io_error", + ConfigError::Parse(_) => "parse_error", + } } impl RuntimeConfig { @@ -1078,16 +1263,27 @@ struct ParsedConfigFile { source: String, } -fn read_optional_json_object(path: &Path) -> Result, ConfigError> { +enum OptionalConfigFile { + Loaded(ParsedConfigFile), + NotFound, + Skipped { + reason: String, + detail: Option, + }, +} + +fn read_optional_json_object(path: &Path) -> Result { let is_legacy_config = path.file_name().and_then(|name| name.to_str()) == Some(".claw.json"); let contents = match fs::read_to_string(path) { Ok(contents) => contents, - Err(error) if error.kind() == std::io::ErrorKind::NotFound => return Ok(None), + Err(error) if error.kind() == std::io::ErrorKind::NotFound => { + return Ok(OptionalConfigFile::NotFound); + } Err(error) => return Err(ConfigError::Io(error)), }; if contents.trim().is_empty() { - return Ok(Some(ParsedConfigFile { + return Ok(OptionalConfigFile::Loaded(ParsedConfigFile { object: BTreeMap::new(), source: contents, })); @@ -1095,19 +1291,30 @@ fn read_optional_json_object(path: &Path) -> Result, Co let parsed = match JsonValue::parse(&contents) { Ok(parsed) => parsed, - Err(_error) if is_legacy_config => return Ok(None), + Err(error) if is_legacy_config => { + return Ok(OptionalConfigFile::Skipped { + reason: "legacy_invalid_json".to_string(), + detail: Some(format!("{}: {error}", path.display())), + }); + } Err(error) => return Err(ConfigError::Parse(format!("{}: {error}", path.display()))), }; let Some(object) = parsed.as_object() else { if is_legacy_config { - return Ok(None); + return Ok(OptionalConfigFile::Skipped { + reason: "legacy_non_object".to_string(), + detail: Some(format!( + "{}: top-level legacy settings value is not a JSON object", + path.display() + )), + }); } return Err(ConfigError::Parse(format!( "{}: top-level settings value must be a JSON object", path.display() ))); }; - Ok(Some(ParsedConfigFile { + Ok(OptionalConfigFile::Loaded(ParsedConfigFile { object: object.clone(), source: contents, })) @@ -1784,8 +1991,8 @@ fn deep_merge_objects( #[cfg(test)] mod tests { use super::{ - deep_merge_objects, parse_permission_mode_label, ConfigLoader, ConfigSource, - McpServerConfig, McpTransport, ResolvedPermissionMode, RuntimeFeatureConfig, + deep_merge_objects, parse_permission_mode_label, ConfigFileStatus, ConfigLoader, + ConfigSource, McpServerConfig, McpTransport, ResolvedPermissionMode, RuntimeFeatureConfig, RuntimeHookCommand, RuntimeHookConfig, RuntimePluginConfig, CLAW_SETTINGS_SCHEMA_NAME, }; use crate::json::JsonValue; @@ -1977,6 +2184,86 @@ mod tests { fs::remove_dir_all(root).expect("cleanup temp dir"); } + #[test] + fn inspect_classifies_missing_loaded_and_legacy_skipped_files() { + let root = temp_dir(); + let cwd = root.join("project"); + let home = root.join("home").join(".claw"); + fs::create_dir_all(cwd.join(".claw")).expect("project config dir"); + fs::create_dir_all(&home).expect("home config dir"); + fs::write(cwd.join(".claw.json"), "{not json").expect("write legacy config"); + fs::write( + cwd.join(".claw").join("settings.json"), + r#"{"model":"opus"}"#, + ) + .expect("write project settings"); + + let inspection = ConfigLoader::new(&cwd, &home).inspect_collecting_warnings(); + + assert!( + inspection.load_error.is_none(), + "{:?}", + inspection.load_error + ); + assert!(inspection.runtime_config.is_some()); + let loaded = inspection + .files + .iter() + .find(|file| file.status == ConfigFileStatus::Loaded) + .expect("loaded file"); + assert!(loaded.loaded); + assert!(loaded.reason.is_none()); + let missing = inspection + .files + .iter() + .find(|file| file.status == ConfigFileStatus::NotFound) + .expect("missing file"); + assert_eq!(missing.reason.as_deref(), Some("not_found")); + let skipped = inspection + .files + .iter() + .find(|file| file.status == ConfigFileStatus::Skipped) + .expect("skipped legacy file"); + assert_eq!(skipped.reason.as_deref(), Some("legacy_invalid_json")); + assert!(!skipped.loaded); + + fs::remove_dir_all(root).expect("cleanup temp dir"); + } + + #[test] + fn inspect_reports_parse_errors_but_keeps_valid_merged_config() { + let root = temp_dir(); + let cwd = root.join("project"); + let home = root.join("home").join(".claw"); + fs::create_dir_all(cwd.join(".claw")).expect("project config dir"); + fs::create_dir_all(&home).expect("home config dir"); + fs::write(home.join("settings.json"), r#"{"model":"sonnet"}"#) + .expect("write user settings"); + fs::write(cwd.join(".claw").join("settings.json"), "{not json") + .expect("write invalid project settings"); + + let inspection = ConfigLoader::new(&cwd, &home).inspect_collecting_warnings(); + + assert!(inspection + .load_error + .as_deref() + .is_some_and(|error| error.contains("settings.json"))); + let runtime_config = inspection.runtime_config.expect("valid files still merge"); + assert_eq!(runtime_config.model(), Some("sonnet")); + let error_file = inspection + .files + .iter() + .find(|file| file.status == ConfigFileStatus::LoadError) + .expect("load error file"); + assert_eq!(error_file.reason.as_deref(), Some("parse_error")); + assert!(error_file + .detail + .as_deref() + .is_some_and(|detail| detail.contains("settings.json"))); + + fs::remove_dir_all(root).expect("cleanup temp dir"); + } + #[test] fn parses_sandbox_config() { let root = temp_dir(); diff --git a/rust/crates/runtime/src/lib.rs b/rust/crates/runtime/src/lib.rs index 64330dc6..0f0eed8d 100644 --- a/rust/crates/runtime/src/lib.rs +++ b/rust/crates/runtime/src/lib.rs @@ -65,13 +65,13 @@ pub use compact::{ get_compact_continuation_message, should_compact, CompactionConfig, CompactionResult, }; pub use config::{ - suppress_config_warnings_for_json_mode, ConfigEntry, ConfigError, ConfigLoader, ConfigSource, - McpConfigCollection, McpManagedProxyServerConfig, McpOAuthConfig, McpRemoteServerConfig, - McpSdkServerConfig, McpServerConfig, McpStdioServerConfig, McpTransport, - McpWebSocketServerConfig, OAuthConfig, ProviderFallbackConfig, ResolvedPermissionMode, - RulesImportConfig, RuntimeConfig, RuntimeFeatureConfig, RuntimeHookCommand, RuntimeHookConfig, - RuntimePermissionRuleConfig, RuntimePluginConfig, ScopedMcpServerConfig, - CLAW_SETTINGS_SCHEMA_NAME, + suppress_config_warnings_for_json_mode, ConfigEntry, ConfigError, ConfigFileReport, + ConfigFileStatus, ConfigInspection, ConfigLoader, ConfigSource, McpConfigCollection, + McpManagedProxyServerConfig, McpOAuthConfig, McpRemoteServerConfig, McpSdkServerConfig, + McpServerConfig, McpStdioServerConfig, McpTransport, McpWebSocketServerConfig, OAuthConfig, + ProviderFallbackConfig, ResolvedPermissionMode, RulesImportConfig, RuntimeConfig, + RuntimeFeatureConfig, RuntimeHookCommand, RuntimeHookConfig, RuntimePermissionRuleConfig, + RuntimePluginConfig, ScopedMcpServerConfig, CLAW_SETTINGS_SCHEMA_NAME, }; pub use config_validate::{ check_unsupported_format, format_diagnostics, validate_config_file, ConfigDiagnostic, diff --git a/rust/crates/rusty-claude-cli/src/main.rs b/rust/crates/rusty-claude-cli/src/main.rs index e1d71395..88ccd24e 100644 --- a/rust/crates/rusty-claude-cli/src/main.rs +++ b/rust/crates/rusty-claude-cli/src/main.rs @@ -55,8 +55,8 @@ use render::{MarkdownStreamState, Spinner, TerminalRenderer}; use runtime::{ check_base_commit, format_stale_base_warning, format_usd, load_oauth_credentials, load_system_prompt, pricing_for_model, resolve_expected_base, resolve_sandbox_status, - ApiClient, ApiRequest, AssistantEvent, BaseCommitState, CompactionConfig, ConfigLoader, - ConfigSource, ContentBlock, ConversationMessage, ConversationRuntime, McpServer, + ApiClient, ApiRequest, AssistantEvent, BaseCommitState, CompactionConfig, ConfigFileReport, + ConfigLoader, ConfigSource, ContentBlock, ConversationMessage, ConversationRuntime, McpServer, McpServerManager, McpServerSpec, McpTool, MessageRole, ModelPricing, PermissionMode, PermissionPolicy, ProjectContext, PromptCacheEvent, ResolvedPermissionMode, RuntimeError, Session, TokenUsage, ToolError, ToolExecutor, UsageTracker, @@ -417,6 +417,9 @@ fn fallback_hint_for_error_kind(kind: &str) -> Option<&'static str> { "missing_credentials" => { Some("Set ANTHROPIC_API_KEY or ANTHROPIC_AUTH_TOKEN before running claw.") } + "config_parse_error" => Some( + "Fix the JSON syntax or schema in the referenced .claw/settings.json or .claw.json file, then rerun the command.", + ), // #787: session load failures have no \n-delimited hint from the OS error path "session_load_failed" => Some( "Pass a path to a .jsonl session file, not a directory. Managed sessions live in .claw/sessions/.", @@ -8483,38 +8486,28 @@ fn render_config_json( ) -> Result> { let cwd = env::current_dir()?; let loader = ConfigLoader::default_for(&cwd); - let discovered = loader.discover(); - // #773: use load_collecting_warnings so deprecation warnings are surfaced in the - // JSON envelope instead of only as unstructured stderr text. - let (runtime_config, config_warnings) = loader.load_collecting_warnings()?; - - let loaded_paths: Vec<_> = runtime_config - .loaded_entries() + // #773: keep deprecation warnings in the JSON envelope, and #407: include + // per-file status/reason/detail for every discovered config path. + let inspection = loader.inspect_collecting_warnings(); + if section.is_some() { + if let Some(error) = &inspection.load_error { + return Err(error.clone().into()); + } + } + let runtime_config = inspection + .runtime_config + .clone() + .unwrap_or_else(runtime::RuntimeConfig::empty); + let loaded_files = runtime_config.loaded_entries().len(); + let merged_keys = runtime_config.merged().len(); + let files: Vec<_> = inspection + .files .iter() - .map(|e| e.path.display().to_string()) + .map(config_file_report_json) .collect(); - let files: Vec<_> = discovered - .iter() - .map(|e| { - let source = match e.source { - ConfigSource::User => "user", - ConfigSource::Project => "project", - ConfigSource::Local => "local", - }; - let is_loaded = runtime_config - .loaded_entries() - .iter() - .any(|le| le.path == e.path); - serde_json::json!({ - "path": e.path.display().to_string(), - "source": source, - "loaded": is_loaded, - }) - }) - .collect(); - - let warnings_json: Vec = config_warnings + let warnings_json: Vec = inspection + .warnings .iter() .map(|w| serde_json::Value::String(w.clone())) .collect(); @@ -8522,14 +8515,15 @@ fn render_config_json( let base = serde_json::json!({ "kind": "config", "action": if section.is_some() { "show" } else { "list" }, - "status": "ok", + "status": if inspection.load_error.is_some() { "error" } else { "ok" }, "cwd": cwd.display().to_string(), - "loaded_files": loaded_paths.len(), - "merged_keys": runtime_config.merged().len(), + "loaded_files": loaded_files, + "merged_keys": merged_keys, + "merged_key_count": merged_keys, + "merged_keys_meaning": "count of top-level keys in the effective merged JSON object", "files": files, - // #773: deprecation warnings surfaced structurally so JSON-mode callers - // don't need to strip unstructured text from stderr "warnings": warnings_json, + "load_error": inspection.load_error.clone(), }); if let Some(section) = section { @@ -8576,8 +8570,8 @@ fn render_config_json( "hint": hint, "supported_sections": ["env", "hooks", "model", "plugins", "mcp", "sandbox", "permissions", "skills", "agents", "settings"], "cwd": cwd.display().to_string(), - "loaded_files": loaded_paths.len(), - "files": files, + "loaded_files": loaded_files, + "files": base["files"].clone(), })); } }; @@ -8600,6 +8594,45 @@ fn render_config_json( Ok(base) } +fn config_file_report_json(file: &ConfigFileReport) -> serde_json::Value { + let source = match file.entry.source { + ConfigSource::User => "user", + ConfigSource::Project => "project", + ConfigSource::Local => "local", + }; + let mut object = serde_json::Map::new(); + object.insert( + "path".to_string(), + serde_json::Value::String(file.entry.path.display().to_string()), + ); + object.insert( + "source".to_string(), + serde_json::Value::String(source.to_string()), + ); + object.insert("loaded".to_string(), serde_json::Value::Bool(file.loaded)); + object.insert( + "status".to_string(), + serde_json::Value::String(file.status.as_str().to_string()), + ); + if let Some(reason) = &file.reason { + object.insert( + "reason".to_string(), + serde_json::Value::String(reason.clone()), + ); + object.insert( + "skip_reason".to_string(), + serde_json::Value::String(reason.clone()), + ); + } + if let Some(detail) = &file.detail { + object.insert( + "detail".to_string(), + serde_json::Value::String(detail.clone()), + ); + } + serde_json::Value::Object(object) +} + fn render_memory_report() -> Result> { let cwd = env::current_dir()?; let project_context = ProjectContext::discover(&cwd, DEFAULT_DATE)?; diff --git a/rust/crates/rusty-claude-cli/tests/output_format_contract.rs b/rust/crates/rusty-claude-cli/tests/output_format_contract.rs index f96c54d5..7c66b219 100644 --- a/rust/crates/rusty-claude-cli/tests/output_format_contract.rs +++ b/rust/crates/rusty-claude-cli/tests/output_format_contract.rs @@ -1458,6 +1458,114 @@ fn config_json_reports_deprecations_structurally_without_stderr_duplicate_815() ); } +#[test] +fn config_json_reports_structured_unloaded_file_reasons_407() { + let root = unique_temp_dir("config-file-status-407"); + let config_home = root.join("config-home"); + let home = root.join("home"); + fs::create_dir_all(root.join(".claw")).expect("workspace config should exist"); + fs::create_dir_all(&config_home).expect("config home should exist"); + fs::create_dir_all(&home).expect("home should exist"); + fs::write(root.join(".claw.json"), "{not json").expect("legacy skip fixture should write"); + fs::write( + root.join(".claw").join("settings.json"), + r#"{"model":"opus"}"#, + ) + .expect("project 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, &["--output-format", "json", "config"], &envs); + assert!( + output.status.success(), + "stdout:\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 valid json"); + + assert_eq!(parsed["kind"], "config"); + assert_eq!(parsed["status"], "ok"); + assert_eq!(parsed["loaded_files"], 1); + assert_eq!(parsed["merged_keys"], parsed["merged_key_count"]); + assert_eq!( + parsed["merged_keys_meaning"].as_str(), + Some("count of top-level keys in the effective merged JSON object") + ); + assert!(parsed["load_error"].is_null()); + + let files = parsed["files"].as_array().expect("files array"); + let loaded = files + .iter() + .find(|file| file["loaded"] == true) + .expect("loaded config file"); + assert_eq!(loaded["status"], "loaded"); + assert!(loaded.get("reason").is_none()); + let missing = files + .iter() + .find(|file| file["status"] == "not_found") + .expect("missing config file"); + assert_eq!(missing["loaded"], false); + assert_eq!(missing["reason"], "not_found"); + assert_eq!(missing["skip_reason"], "not_found"); + let skipped = files + .iter() + .find(|file| file["status"] == "skipped") + .expect("skipped legacy config file"); + assert_eq!(skipped["loaded"], false); + assert_eq!(skipped["reason"], "legacy_invalid_json"); + assert_eq!(skipped["skip_reason"], "legacy_invalid_json"); + assert!(skipped["detail"].as_str().is_some()); +} + +#[test] +fn config_json_list_reports_parse_errors_without_dropping_file_statuses_407() { + let root = unique_temp_dir("config-file-load-error-407"); + let config_home = root.join("config-home"); + let home = root.join("home"); + fs::create_dir_all(root.join(".claw")).expect("workspace config should exist"); + 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#"{"model":"sonnet"}"#) + .expect("user config fixture should write"); + fs::write(root.join(".claw").join("settings.json"), "{not json") + .expect("invalid project 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, &["--output-format", "json", "config"], &envs); + assert!( + output.status.success(), + "config list should be best-effort even with one parse-broken file; stdout:\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 valid json"); + + assert_eq!(parsed["status"], "error"); + assert!(parsed["load_error"].as_str().is_some()); + assert_eq!(parsed["loaded_files"], 1); + let files = parsed["files"].as_array().expect("files array"); + let error_file = files + .iter() + .find(|file| file["status"] == "load_error") + .expect("load error config file"); + assert_eq!(error_file["loaded"], false); + assert_eq!(error_file["reason"], "parse_error"); + assert_eq!(error_file["skip_reason"], "parse_error"); + assert!(error_file["detail"].as_str().is_some()); +} + #[test] fn global_json_surfaces_suppress_config_deprecation_stderr_810_821_824() { let root = unique_temp_dir("global-json-warning-810-821-824");