fix(config): improve Gemini merge and remove duplicate logging

Address multiple audit findings:

1. config_merge.rs:701 - Gemini merge now initializes env from common
   config when custom config has no env field, ensuring common API keys
   and base URLs are applied correctly.

2. Remove duplicate logging - merge functions now only return warnings,
   callers (live.rs, proxy.rs) handle logging in one place.

3. live.rs:169 - Unify error message style to plain English descriptions
   instead of error codes embedded in messages.

Changes:
- Add fallback to initialize env from common_env when missing
- Remove log::warn! calls from merge functions
- Use descriptive error messages consistently
This commit is contained in:
YoVinchen
2026-01-26 17:13:38 +08:00
Unverified
parent d5d7c87fd6
commit 4e4a445922
2 changed files with 16 additions and 17 deletions
+10 -10
View File
@@ -617,12 +617,10 @@ fn merge_claude_config(custom_config: &JsonValue, common_snippet: &str) -> Merge
let common_value: JsonValue = match serde_json::from_str(common_snippet) {
Ok(v) => v,
Err(e) => {
log::warn!(
"Claude common config snippet parse error, skipping merge: {e}"
);
// Return warning without logging - caller will log if needed
return MergeResult {
config: custom_config.clone(),
warning: Some(format!("COMMON_CONFIG_PARSE_ERROR: {e}")),
warning: Some(format!("Claude common config parse error: {e}")),
};
}
};
@@ -643,8 +641,8 @@ fn merge_codex_config(custom_config: &JsonValue, common_snippet: &str) -> MergeR
let (merged_toml, error) =
compute_final_toml_config_str(config_str, common_snippet, true);
if let Some(e) = error {
log::warn!("Codex common config merge warning: {e}");
warning = Some(format!("CODEX_TOML_MERGE_WARNING: {e}"));
// Return warning without logging - caller will log if needed
warning = Some(format!("Codex TOML merge warning: {e}"));
}
obj.insert("config".to_string(), JsonValue::String(merged_toml));
}
@@ -667,12 +665,10 @@ fn merge_gemini_config(custom_config: &JsonValue, common_snippet: &str) -> Merge
let common_value: JsonValue = match serde_json::from_str(common_snippet) {
Ok(v) => v,
Err(e) => {
log::warn!(
"Gemini common config snippet parse error, skipping merge: {e}"
);
// Return warning without logging - caller will log if needed
return MergeResult {
config: custom_config.clone(),
warning: Some(format!("COMMON_CONFIG_PARSE_ERROR: {e}")),
warning: Some(format!("Gemini common config parse error: {e}")),
};
}
};
@@ -699,6 +695,7 @@ fn merge_gemini_config(custom_config: &JsonValue, common_snippet: &str) -> Merge
};
// Merge only the env field
// If custom config has env, merge common into it; otherwise initialize with common env
if let Some(merged_obj) = merged_config.as_object_mut() {
if let Some(merged_env) = merged_obj.get_mut("env") {
if let Some(merged_env_obj) = merged_env.as_object_mut() {
@@ -709,6 +706,9 @@ fn merge_gemini_config(custom_config: &JsonValue, common_snippet: &str) -> Merge
}
*merged_env = JsonValue::Object(final_env);
}
} else if !common_env.is_empty() {
// Custom config has no env field - initialize with common env
merged_obj.insert("env".to_string(), JsonValue::Object(common_env));
}
}
+6 -7
View File
@@ -167,17 +167,16 @@ fn write_live_snapshot_internal(
}
AppType::Codex => {
let obj = config_to_write.as_object().ok_or_else(|| {
AppError::Config(
"CODEX_CONFIG_NOT_OBJECT: settings_config must be a JSON object".to_string(),
)
AppError::Config("Codex provider settings_config must be a JSON object".to_string())
})?;
let auth = obj.get("auth").ok_or_else(|| {
AppError::Config(
"CODEX_CONFIG_MISSING_AUTH: settings_config missing 'auth' field".to_string(),
)
AppError::Config("Codex provider settings_config missing 'auth' field".to_string())
})?;
let config_str = obj.get("config").and_then(|v| v.as_str()).ok_or_else(|| {
AppError::Config("CODEX_CONFIG_MISSING_CONFIG: settings_config missing 'config' field or not a string".to_string())
AppError::Config(
"Codex provider settings_config missing 'config' field or not a string"
.to_string(),
)
})?;
let auth_path = get_codex_auth_path();