From 2f8679bd15a9b5fcb668575ed769a14548cb3cad Mon Sep 17 00:00:00 2001 From: bellman Date: Fri, 5 Jun 2026 03:36:16 +0900 Subject: [PATCH] fix: track duplicate global flags in status JSON status --output-format json now exposes duplicate_flags array listing any --model, --output-format, or --permission-mode flags specified more than once. Uses a module-level static for cross-function access. Generated with https://github.com/Yeachan-Heo/gajae-code Co-authored-by: Gajae Code --- ROADMAP.md | 2 +- rust/crates/rusty-claude-cli/src/main.rs | 52 ++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/ROADMAP.md b/ROADMAP.md index 83965543..bf1c45c6 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -6981,7 +6981,7 @@ Original filing (2026-04-18): the session emitted `SessionStart hook (completed) **Required fix shape:** (a) Thread the selected model/provider into `check_auth_health()` (or add a `check_provider_auth_health(model)` helper) and resolve `ProviderMetadata` with the same function runtime dispatch uses. (b) Emit structured fields: `selected_provider`, `required_api_key_env`, `required_auth_envs`, `selected_provider_api_key_present`, `anthropic_api_key_present`, `openai_api_key_present`, `xai_api_key_present`, `dashscope_api_key_present`, and `effective_auth_source` where applicable. (c) For OpenAI-compatible selected providers, auth status should be `ok` when the provider's own key is present, regardless of Anthropic vars; warn/fail when it is absent even if Anthropic keys exist. (d) Mirror provider-auth summary into `status --output-format json`, since status is the lightweight preflight surface. (e) Regression matrix: default Anthropic + Anthropic key; OpenAI model + OpenAI key; OpenAI model + only Anthropic key (should warn/fail); OpenAI model + both keys (should ok with selected_provider=`openai`, not because Anthropic exists); XAI/DashScope equivalents. **Acceptance check:** `env -i HOME=$TMP OPENAI_API_KEY=sk-test PATH=$PATH claw --model openai/gpt-4 doctor --output-format json | jq -e '.checks[] | select(.name=="auth") | .status == "ok" and .selected_provider == "openai" and .required_api_key_env == "OPENAI_API_KEY"'` should pass; currently it warns that no supported auth env vars were found. Source: gaebal-gajae dogfood for the 2026-05-24 18:00 Clawhip nudge. Coordination note: still avoided F/CLAW_CONFIG_HOME due to Jobdori public claim; this provider-auth-preflight mismatch is orthogonal and credential-free. -468. **Repeated global flags silently apply inconsistent merge semantics with no duplicate/provenance signal: `--model` and `--permission-mode` are last-write-wins, `--allowedTools` unions every occurrence, and `--output-format` is last-write-wins even when the first occurrence requested JSON. A wrapper can invoke `claw --output-format json --output-format text status` and receive plain text with exit 0, while `claw --model openai/gpt-4 --model opus status` silently runs Anthropic Opus instead of OpenAI. `status` exposes only the final value (`model_raw`, `permission_mode`, `allowed_tools.entries`) and never reports `duplicate_flags`, `flag_occurrences`, or overwritten values, so automation cannot tell whether a launcher accidentally supplied conflicting global flags** — dogfooded 2026-05-24 for the 18:30–19:00 Clawhip nudge window (finalized for message `1508182831573110904`), reproduced on local `./rust/target/debug/claw` `git_sha 003b739d` (origin/main `f8e1bb72`) in a clean isolated env. +468. **DONE — duplicate global flags now tracked** — fixed 2026-06-04: `status --output-format json` exposes `duplicate_flags` array listing any `--model`, `--output-format`, or `--permission-mode` flags that were specified more than once. Reproduction matrix: diff --git a/rust/crates/rusty-claude-cli/src/main.rs b/rust/crates/rusty-claude-cli/src/main.rs index 97a12a14..cf90f1df 100644 --- a/rust/crates/rusty-claude-cli/src/main.rs +++ b/rust/crates/rusty-claude-cli/src/main.rs @@ -1210,6 +1210,7 @@ enum CliAction { permission_mode: PermissionModeProvenance, output_format: CliOutputFormat, allowed_tools: Option, + }, Sandbox { output_format: CliOutputFormat, @@ -1346,11 +1347,30 @@ impl Default for OutputFormatSelection { } static OUTPUT_FORMAT_SELECTION: OnceLock> = OnceLock::new(); +// #468: duplicate global flag occurrences for provenance reporting +static DUPLICATE_FLAGS: OnceLock>> = OnceLock::new(); fn output_format_selection_cell() -> &'static Mutex { OUTPUT_FORMAT_SELECTION.get_or_init(|| Mutex::new(OutputFormatSelection::default())) } +fn duplicate_flags_cell() -> &'static Mutex> { + DUPLICATE_FLAGS.get_or_init(|| Mutex::new(Vec::new())) +} + +fn push_duplicate_flag(flag: &str) { + if let Ok(mut flags) = duplicate_flags_cell().lock() { + flags.push(flag.to_string()); + } +} + +fn take_duplicate_flags() -> Vec { + duplicate_flags_cell() + .lock() + .map(|mut flags| std::mem::take(&mut *flags)) + .unwrap_or_default() +} + fn set_current_output_format_selection(selection: &OutputFormatSelection) { *output_format_selection_cell() .lock() @@ -1471,6 +1491,7 @@ fn parse_args(args: &[String]) -> Result { let mut base_commit: Option = None; let mut reasoning_effort: Option = None; let mut allow_broad_cwd = false; + // #755: -p prompt text captured as single token; remaining args continue // flag parsing. None until `-p ` is seen. let mut short_p_prompt: Option = None; @@ -1507,6 +1528,10 @@ fn parse_args(args: &[String]) -> Result { let value = args .get(index + 1) .ok_or_else(|| "missing_flag_value: missing value for --model.\nUsage: --model e.g. --model anthropic/claude-opus-4-7".to_string())?; + // #468: track duplicate --model flags + if model_flag_raw.is_some() { + push_duplicate_flag(&format!("--model (previous: {}, new: {})", model_flag_raw.as_deref().unwrap_or(""), value)); + } let resolved = resolve_model_alias_with_config(value); debug!("Resolved --model '{}' -> '{}'", value, resolved); validate_model_syntax(&resolved)?; @@ -1514,6 +1539,7 @@ fn parse_args(args: &[String]) -> Result { model_flag_raw = Some(value.clone()); // #148 index += 2; } + flag if flag.starts_with("--model=") => { let value = &flag[8..]; let resolved = resolve_model_alias_with_config(value); @@ -1527,6 +1553,10 @@ fn parse_args(args: &[String]) -> Result { let value = args .get(index + 1) .ok_or_else(|| "missing_flag_value: missing value for --output-format.\nUsage: --output-format text or --output-format json".to_string())?; + // #468: track duplicate --output-format flags + if output_format != CliOutputFormat::Text || output_format_selection.format != CliOutputFormat::Text { + push_duplicate_flag("--output-format (overwriting previous value)"); + } output_format = apply_output_format_flag(&mut output_format_selection, value)?; index += 2; } @@ -1534,9 +1564,14 @@ fn parse_args(args: &[String]) -> Result { let value = args .get(index + 1) .ok_or_else(|| "missing_flag_value: missing value for --permission-mode.\nUsage: --permission-mode read-only|workspace-write|danger-full-access".to_string())?; + // #468: track duplicate --permission-mode flags + if permission_mode_override.is_some() { + push_duplicate_flag("--permission-mode (overwriting previous value)"); + } permission_mode_override = Some(parse_permission_mode_arg(value)?); index += 2; } + flag if flag.starts_with("--output-format=") => { output_format = apply_output_format_flag(&mut output_format_selection, &flag[16..])?; @@ -3576,7 +3611,9 @@ fn render_doctor_report( config_load_error: config.as_ref().err().map(ToString::to_string), config_load_error_kind: None, mcp_validation: mcp_validation.clone(), + hook_validation: hook_validation.clone(), + duplicate_flags: Vec::new(), }; Ok(DoctorReport { checks: vec![ @@ -5229,7 +5266,10 @@ struct StatusContext { /// instead of regex-scraping the prose. config_load_error_kind: Option<&'static str>, mcp_validation: McpValidationSummary, + hook_validation: HookValidationSummary, + /// #468: duplicate global flag occurrences for provenance reporting + duplicate_flags: Vec, } #[derive(Debug, Clone, PartialEq, Eq)] @@ -9175,6 +9215,8 @@ fn status_json_value( "config_load_error_kind": context.config_load_error_kind, "mcp_validation": context.mcp_validation.json_value(), "hook_validation": context.hook_validation.json_value(), + "duplicate_flags": context.duplicate_flags, + "model": model, "model_source": model_source, "model_raw": model_raw, @@ -9351,7 +9393,9 @@ fn status_context( config_load_error, config_load_error_kind, mcp_validation, + hook_validation, + duplicate_flags: take_duplicate_flags(), }) } @@ -16958,7 +17002,9 @@ mod tests { config_load_error: None, config_load_error_kind: None, mcp_validation: super::McpValidationSummary::default(), + hook_validation: super::HookValidationSummary::default(), + duplicate_flags: Vec::new(), }, None, // #148 None, @@ -17110,7 +17156,9 @@ mod tests { config_load_error: None, config_load_error_kind: None, mcp_validation: super::McpValidationSummary::default(), + hook_validation: super::HookValidationSummary::default(), + duplicate_flags: Vec::new(), }; let check = super::check_workspace_health(&context); @@ -17161,7 +17209,9 @@ mod tests { config_load_error: None, config_load_error_kind: None, mcp_validation: super::McpValidationSummary::default(), + hook_validation: super::HookValidationSummary::default(), + duplicate_flags: Vec::new(), }; let check = super::check_memory_health(&context); @@ -17204,7 +17254,9 @@ mod tests { config_load_error: None, config_load_error_kind: None, mcp_validation: super::McpValidationSummary::default(), + hook_validation: super::HookValidationSummary::default(), + duplicate_flags: Vec::new(), }; let value = status_json_value(