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 <dev@gajae-code.com>
This commit is contained in:
parent
9ef21e23f3
commit
2f8679bd15
|
|
@ -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:
|
||||
|
||||
|
|
|
|||
|
|
@ -1210,6 +1210,7 @@ enum CliAction {
|
|||
permission_mode: PermissionModeProvenance,
|
||||
output_format: CliOutputFormat,
|
||||
allowed_tools: Option<AllowedToolSet>,
|
||||
|
||||
},
|
||||
Sandbox {
|
||||
output_format: CliOutputFormat,
|
||||
|
|
@ -1346,11 +1347,30 @@ impl Default for OutputFormatSelection {
|
|||
}
|
||||
|
||||
static OUTPUT_FORMAT_SELECTION: OnceLock<Mutex<OutputFormatSelection>> = OnceLock::new();
|
||||
// #468: duplicate global flag occurrences for provenance reporting
|
||||
static DUPLICATE_FLAGS: OnceLock<Mutex<Vec<String>>> = OnceLock::new();
|
||||
|
||||
fn output_format_selection_cell() -> &'static Mutex<OutputFormatSelection> {
|
||||
OUTPUT_FORMAT_SELECTION.get_or_init(|| Mutex::new(OutputFormatSelection::default()))
|
||||
}
|
||||
|
||||
fn duplicate_flags_cell() -> &'static Mutex<Vec<String>> {
|
||||
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<String> {
|
||||
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<CliAction, String> {
|
|||
let mut base_commit: Option<String> = None;
|
||||
let mut reasoning_effort: Option<String> = None;
|
||||
let mut allow_broad_cwd = false;
|
||||
|
||||
// #755: -p prompt text captured as single token; remaining args continue
|
||||
// flag parsing. None until `-p <text>` is seen.
|
||||
let mut short_p_prompt: Option<String> = None;
|
||||
|
|
@ -1507,6 +1528,10 @@ fn parse_args(args: &[String]) -> Result<CliAction, String> {
|
|||
let value = args
|
||||
.get(index + 1)
|
||||
.ok_or_else(|| "missing_flag_value: missing value for --model.\nUsage: --model <provider/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<CliAction, String> {
|
|||
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<CliAction, String> {
|
|||
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<CliAction, String> {
|
|||
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<String>,
|
||||
}
|
||||
|
||||
#[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(
|
||||
|
|
|
|||
Loading…
Reference in New Issue