From 36d3181475e16fa3f881c6c9d9f8b5dc4c608bc3 Mon Sep 17 00:00:00 2001 From: lzt404 <2596933790@qq.com> Date: Thu, 10 Sep 2026 18:03:11 +0800 Subject: [PATCH] fix(aliases): prevent stale edits and preserve payload rules - Validate configuration revisions and reject ambiguous model mappings - Preserve raw overrides, conditions, and unchanged settings - Serialize saves and restore configuration after write failures - Add regression coverage and remove comments from affected files --- src-tauri/src/agents/commands.rs | 10 +- src-tauri/src/core_config.rs | 3 +- src-tauri/src/core_config/alias_edit.rs | 273 ++++++++++++---- src-tauri/src/core_config/alias_save.rs | 74 +++++ src-tauri/src/core_config/aliases.rs | 192 +++++------- src-tauri/src/tests.rs | 2 + src-tauri/src/tests/alias_edit.rs | 14 +- src-tauri/src/tests/alias_edit_regressions.rs | 196 ++++++++++++ src-tauri/src/tests/alias_save.rs | 296 ++++++++++++++++++ src/pages/ThinkingAliasesPage.tsx | 18 +- tests/fixtures/model-alias-editor.tsx | 8 +- tests/model-alias-editor-ui.cjs | 12 +- 12 files changed, 903 insertions(+), 195 deletions(-) create mode 100644 src-tauri/src/core_config/alias_save.rs create mode 100644 src-tauri/src/tests/alias_edit_regressions.rs create mode 100644 src-tauri/src/tests/alias_save.rs diff --git a/src-tauri/src/agents/commands.rs b/src-tauri/src/agents/commands.rs index 6690aa9..85bc00e 100644 --- a/src-tauri/src/agents/commands.rs +++ b/src-tauri/src/agents/commands.rs @@ -496,12 +496,12 @@ pub(crate) async fn get_thinking_alias_sources( pub(crate) async fn get_model_alias_edit_source( gui_config_state: tauri::State<'_, GuiConfigState>, alias: String, -) -> Result { +) -> Result { let config = gui_config_state.snapshot()?; let alias = validate_thinking_alias_model_id(&alias, "别名模型")?; let content = fetch_management_config_yaml(&config).await?; let definitions = fetch_oauth_model_definitions(&config).await; - Ok(resolve_model_alias_edit_source(&content, &alias, &definitions)?.source) + model_alias_edit_context(&content, &alias, &definitions) } #[tauri::command] @@ -512,6 +512,7 @@ pub(crate) async fn create_thinking_alias( effort: String, fast: Option, original_alias: Option, + expected_revision: Option, ) -> Result, String> { let config = gui_config_state.snapshot()?; let source_id = source_id.trim().to_string(); @@ -526,6 +527,9 @@ pub(crate) async fn create_thinking_alias( }; let fast = fast.unwrap_or(false); let content = fetch_management_config_yaml(&config).await?; + if original_alias.is_some() { + validate_model_alias_revision(&content, expected_revision.as_deref())?; + } let available_models = fetch_agent_models(config.port, effective_agent_api_key(&config)).await?; let definitions = fetch_oauth_model_definitions(&config).await; @@ -801,8 +805,6 @@ pub(crate) async fn fetch_codex_runtime_models( Err("本地内核不支持 Codex 模型列表接口".to_string()) } -// Codex client metadata distinguishes the default context from the maximum. -// Do not overwrite either with the generic model-definitions context_length. pub(crate) async fn fetch_codex_catalog_runtime_models( config: &GuiConfigFile, ) -> Result, String> { diff --git a/src-tauri/src/core_config.rs b/src-tauri/src/core_config.rs index e16a8df..e8ca330 100644 --- a/src-tauri/src/core_config.rs +++ b/src-tauri/src/core_config.rs @@ -1,13 +1,14 @@ -//! Core configuration commands and loss-minimizing YAML editing. use super::*; mod alias_edit; +mod alias_save; mod aliases; mod commands; mod settings; mod yaml; pub(crate) use alias_edit::*; +pub(crate) use alias_save::*; pub(crate) use aliases::*; pub(crate) use commands::*; pub(crate) use settings::*; diff --git a/src-tauri/src/core_config/alias_edit.rs b/src-tauri/src/core_config/alias_edit.rs index e59f442..6d4b8e2 100644 --- a/src-tauri/src/core_config/alias_edit.rs +++ b/src-tauri/src/core_config/alias_edit.rs @@ -1,9 +1,70 @@ -//! Edit aliases without rebuilding their configuration from a base model. use super::*; struct EditableModelAlias { source: ResolvedThinkingAliasSource, value: serde_norway::Value, + model_index: usize, +} + +#[derive(Serialize)] +#[serde(rename_all = "camelCase")] +pub(crate) struct ModelAliasEditContext { + pub(crate) source: ThinkingAliasSource, + pub(crate) revision: String, + pub(crate) effort: Option, + pub(crate) fast: bool, +} + +pub(crate) fn validate_model_alias_revision( + content: &str, + revision: Option<&str>, +) -> Result<(), String> { + if revision != Some(model_alias_config_revision(content)?.as_str()) { + return Err("配置已变化，请关闭编辑器并刷新后重试".to_string()); + } + Ok(()) +} + +fn model_alias_config_revision(content: &str) -> Result { + let document = serde_norway::from_str::(content) + .map_err(|error| format!("解析内核 YAML 配置失败: {error}"))?; + let mut value = serde_json::to_value(document).map_err(|error| error.to_string())?; + let root = value + .as_object_mut() + .ok_or("内核配置顶层必须是 YAML 映射")?; + if root + .get("oauth-model-alias") + .and_then(serde_json::Value::as_object) + .is_some_and(|aliases| aliases.is_empty()) + { + root.remove("oauth-model-alias"); + } + value.sort_all_objects(); + Ok(sha256_bytes( + serde_json::to_string(&value) + .map_err(|error| error.to_string())? + .as_bytes(), + )) +} + +pub(crate) fn model_alias_edit_context( + content: &str, + alias: &str, + definitions: &[OAuthModelDefinitions], +) -> Result { + let source = resolve_model_alias_edit_source(content, alias, definitions)?.source; + let document = serde_norway::from_str::(content) + .map_err(|error| error.to_string())?; + let root = document + .as_mapping() + .ok_or("内核配置顶层必须是 YAML 映射")?; + Ok(ModelAliasEditContext { + effort: find_thinking_alias_effort(root, alias, &source.protocol), + fast: find_speed_alias_service_tier(root, alias, &source.protocol).as_deref() + == Some("priority"), + source, + revision: model_alias_config_revision(content)?, + }) } pub(crate) fn model_alias_edit_source_id(alias: &str) -> String { @@ -15,6 +76,7 @@ fn editable_model_alias( alias: &str, ) -> Result { let mut matches = Vec::new(); + let mut matching_names = 0; for (section, provider, kind, protocol) in [ ("codex-api-key", "Codex API", "codex-api", "codex"), ( @@ -47,7 +109,11 @@ fn editable_model_alias( else { continue; }; - if upstream == client || !client.eq_ignore_ascii_case(alias) { + if !client.eq_ignore_ascii_case(alias) { + continue; + } + matching_names += 1; + if upstream == client { continue; } matches.push(EditableModelAlias { @@ -68,6 +134,7 @@ fn editable_model_alias( }, }, value: model.clone(), + model_index, }); } } @@ -80,12 +147,16 @@ fn editable_model_alias( let models = models .as_sequence() .ok_or("oauth-model-alias 的通道配置必须是数组")?; - for model in models { + for (model_index, model) in models.iter().enumerate() { let Some((upstream, client, display_name)) = configured_model_identity(model) else { continue; }; - if upstream == client || !client.eq_ignore_ascii_case(alias) { + if !client.eq_ignore_ascii_case(alias) { + continue; + } + matching_names += 1; + if upstream == client { continue; } let channel = channel_name @@ -109,11 +180,12 @@ fn editable_model_alias( }, }, value: model.clone(), + model_index, }); } } } - if matches.len() != 1 { + if matching_names != 1 || matches.len() != 1 { return Err("别名不存在或存在多个同名映射，请刷新并检查配置后重试".to_string()); } Ok(matches.remove(0)) @@ -140,7 +212,6 @@ pub(crate) fn resolve_model_alias_edit_source( source.source.reasoning_levels = model.reasoning_levels.clone(); } } - // An unavailable catalog must not prevent keeping an existing effort setting. if let Some(effort) = find_thinking_alias_effort(root, alias, &source.source.protocol) { if !source.source.reasoning_levels.contains(&effort) { source.source.reasoning_levels.push(effort); @@ -170,7 +241,6 @@ pub(crate) fn edit_model_alias_in_yaml( if !alias.eq_ignore_ascii_case(original_alias) && configured_model_alias_exists(root, alias) { return Err(format!("别名模型 {alias} 已存在")); } - // Capture the destination before removing anything so array indices cannot shift. let mut replacement = match &source.location { ThinkingAliasSourceLocation::ConfigModel { section, @@ -227,7 +297,6 @@ pub(crate) fn edit_model_alias_in_yaml( yaml_key("alias"), serde_norway::Value::String(alias.to_string()), ); - // Preserve ordering and metadata for edits within the same provider/channel. let same_group = match (&original.source.location, &source.location) { ( ThinkingAliasSourceLocation::ConfigModel { @@ -247,6 +316,35 @@ pub(crate) fn edit_model_alias_in_yaml( ) => a == b, _ => false, }; + let payload_edit = AliasPayloadEdit { + source: &source.source, + effort, + fast, + change_effort: find_thinking_alias_effort( + root, + original_alias, + &original.source.source.protocol, + ) + .as_deref() + .unwrap_or("") + != effort, + change_fast: (find_speed_alias_service_tier( + root, + original_alias, + &original.source.source.protocol, + ) + .as_deref() + == Some("priority")) + != fast, + }; + if same_group + && original.value.as_mapping() == Some(&replacement) + && original.source.source.protocol == source.source.protocol + && !payload_edit.change_effort + && !payload_edit.change_fast + { + return Ok(content.to_string()); + } if !same_group { remove_existing_claude_model_alias(root, original_alias)?; } @@ -274,11 +372,7 @@ pub(crate) fn edit_model_alias_in_yaml( }; if same_group { let model = models - .iter_mut() - .find(|model| { - configured_model_identity(model) - .is_some_and(|(_, name, _)| name.eq_ignore_ascii_case(original_alias)) - }) + .get_mut(original.model_index) .ok_or("原别名已经变化，请刷新后重试")?; *model = serde_norway::Value::Mapping(replacement); } else { @@ -289,32 +383,59 @@ pub(crate) fn edit_model_alias_in_yaml( original_alias, &original.source.source.protocol, alias, - &source.source.protocol, + &payload_edit, )?; - let mut params = serde_norway::Mapping::new(); - if !effort.is_empty() { - insert_thinking_effort_params(&mut params, &source.source, effort)?; - } - if fast { - params.insert( - yaml_key("service_tier"), - serde_norway::Value::String("priority".to_string()), - ); - } - append_alias_payload_override(root, alias, &source.source.protocol, params)?; render_updated_core_yaml(&mut document, updated) } +const ALIAS_EFFORT_KEYS: &[&str] = &[ + "reasoning.effort", + "reasoning_effort", + "output_config.effort", + "generationConfig.thinkingConfig.thinkingLevel", + "thinking.effort", + "thinking.type", +]; + +struct AliasPayloadEdit<'a> { + source: &'a ThinkingAliasSource, + effort: &'a str, + fast: bool, + change_effort: bool, + change_fast: bool, +} + +fn insert_alias_edit_params( + params: &mut serde_norway::Mapping, + replacement: serde_norway::Mapping, + raw: bool, +) -> Result<(), String> { + for (key, value) in replacement { + let value = if raw { + serde_norway::Value::String( + serde_json::to_string(&value).map_err(|error| error.to_string())?, + ) + } else { + value + }; + params.insert(key, value); + } + Ok(()) +} + fn edit_alias_payload( root: &mut serde_norway::Mapping, original_alias: &str, original_protocol: &str, alias: &str, - protocol: &str, + edit: &AliasPayloadEdit<'_>, ) -> Result<(), String> { - let Some(payload) = yaml_mapping_value_mut(root, "payload") else { - return Ok(()); - }; + let protocol = &edit.source.protocol; + let mut found_effort = false; + let mut found_fast = false; + let payload = root + .entry(yaml_key("payload")) + .or_insert_with(|| serde_norway::Value::Mapping(Default::default())); let payload = payload.as_mapping_mut().ok_or("payload 必须是 YAML 映射")?; for section in [ "default", @@ -342,25 +463,12 @@ fn edit_alias_payload( let models = models.as_sequence().ok_or("payload.models 必须是数组")?; let (mut target, others): (Vec<_>, Vec<_>) = models.iter().cloned().partition(|model| { - thinking_payload_model_name_matches(model, original_alias) - && model - .as_mapping() - .and_then(|model| yaml_mapping_value(model, "protocol")) - .is_none_or(|value| { - value.as_str().is_some_and(|value| { - value.eq_ignore_ascii_case(original_protocol) - }) - }) + thinking_payload_model_matches(model, original_alias, original_protocol) }); if target.is_empty() { result.push(rule.clone()); continue; } - if !others.is_empty() { - let mut shared = mapping.clone(); - shared.insert(yaml_key("models"), serde_norway::Value::Sequence(others)); - result.push(serde_norway::Value::Mapping(shared)); - } for model in &mut target { let model = model .as_mapping_mut() @@ -369,7 +477,10 @@ fn edit_alias_payload( yaml_key("name"), serde_norway::Value::String(alias.to_string()), ); - if model.contains_key(yaml_key("protocol")) { + if yaml_mapping_value(model, "protocol") + .and_then(serde_norway::Value::as_str) + .is_some_and(|value| !value.trim().is_empty()) + { model.insert( yaml_key("protocol"), serde_norway::Value::String(protocol.to_string()), @@ -378,28 +489,67 @@ fn edit_alias_payload( } let mut edited = mapping.clone(); edited.insert(yaml_key("models"), serde_norway::Value::Sequence(target)); + let mut remove_target = false; if matches!(section, "override" | "override-raw") { if let Some(params) = yaml_mapping_value_mut(&mut edited, "params") { let params = params .as_mapping_mut() .ok_or("payload.override.params 必须是映射")?; - for key in [ - "reasoning.effort", - "reasoning_effort", - "output_config.effort", - "generationConfig.thinkingConfig.thinkingLevel", - "thinking.effort", - "thinking.type", - "service_tier", - ] { - params.remove(yaml_key(key)); + let has_effort = ALIAS_EFFORT_KEYS + .iter() + .any(|key| params.contains_key(yaml_key(key))); + let has_fast = params.contains_key(yaml_key("service_tier")); + found_effort |= has_effort; + found_fast |= has_fast; + let raw = section == "override-raw"; + if has_effort && (edit.change_effort || original_protocol != protocol) { + let effort = if edit.change_effort { + edit.effort.to_string() + } else { + thinking_effort_from_params(params, original_protocol, raw).ok_or( + "此规则的思考配置无法转换，请先在配置中调整后再切换模型协议", + )? + }; + for key in ALIAS_EFFORT_KEYS { + params.remove(yaml_key(key)); + } + if !effort.is_empty() { + let mut replacement = serde_norway::Mapping::new(); + insert_thinking_effort_params(&mut replacement, edit.source, &effort)?; + insert_alias_edit_params(params, replacement, raw)?; + } } - if params.is_empty() { - continue; + if has_fast && edit.change_fast { + params.remove(yaml_key("service_tier")); + if edit.fast { + let replacement = serde_norway::Mapping::from_iter([( + yaml_key("service_tier"), + yaml_key("priority"), + )]); + insert_alias_edit_params(params, replacement, raw)?; + } } + remove_target = params.is_empty(); } } - result.push(serde_norway::Value::Mapping(edited)); + let mut unchanged = mapping.clone(); + unchanged.insert(yaml_key("models"), edited[yaml_key("models")].clone()); + if !remove_target + && original_alias == alias + && original_protocol == protocol + && edited == unchanged + { + result.push(rule.clone()); + continue; + } + if !others.is_empty() { + let mut shared = mapping.clone(); + shared.insert(yaml_key("models"), serde_norway::Value::Sequence(others)); + result.push(serde_norway::Value::Mapping(shared)); + } + if !remove_target { + result.push(serde_norway::Value::Mapping(edited)); + } } *rules = result; if rules.is_empty() { @@ -409,5 +559,12 @@ fn edit_alias_payload( if payload.is_empty() { root.remove(yaml_key("payload")); } - Ok(()) + let mut params = serde_norway::Mapping::new(); + if edit.change_effort && !found_effort && !edit.effort.is_empty() { + insert_thinking_effort_params(&mut params, edit.source, edit.effort)?; + } + if edit.change_fast && !found_fast && edit.fast { + params.insert(yaml_key("service_tier"), yaml_key("priority")); + } + append_alias_payload_override(root, alias, protocol, params) } diff --git a/src-tauri/src/core_config/alias_save.rs b/src-tauri/src/core_config/alias_save.rs new file mode 100644 index 0000000..6f6939f --- /dev/null +++ b/src-tauri/src/core_config/alias_save.rs @@ -0,0 +1,74 @@ +use super::*; + +fn alias_config_is_unchanged(current: &str, latest: &str) -> Result { + let changes = management_alias_config_changes(current, latest)?; + Ok(changes.oauth_model_aliases.is_none() && !changes.update_config_yaml) +} + +pub(crate) async fn put_management_alias_config_changes( + config: &GuiConfigFile, + current: &str, + updated: &str, +) -> Result<(), String> { + static SAVE_LOCK: tokio::sync::Mutex<()> = tokio::sync::Mutex::const_new(()); + let _guard = SAVE_LOCK.lock().await; + let changes = management_alias_config_changes(current, updated)?; + if changes.oauth_model_aliases.is_none() && !changes.update_config_yaml { + return Ok(()); + } + let latest = fetch_management_config_yaml(config).await?; + if !alias_config_is_unchanged(current, &latest)? { + return Err("配置已变化，请关闭编辑器并刷新后重试".to_string()); + } + let result = async { + if let Some(aliases) = changes.oauth_model_aliases.as_ref() { + put_management_oauth_model_aliases(config, aliases).await?; + } + if changes.update_config_yaml { + put_management_config_yaml(config, updated).await?; + } + Ok::<_, String>(()) + } + .await; + if let Err(error) = result { + return Err(match restore_management_alias_config(config, current, updated).await { + Ok(()) => format!("保存失败，已恢复原配置，可重试：{error}"), + Err(restore_error) => format!("保存失败：{error}；自动恢复失败，配置可能已部分写入，请关闭编辑器并刷新检查：{restore_error}"), + }); + } + Ok(()) +} + +async fn restore_management_alias_config( + config: &GuiConfigFile, + current: &str, + updated: &str, +) -> Result<(), String> { + let latest = fetch_management_config_yaml(config).await?; + let from_current = management_alias_config_changes(current, &latest)?; + let from_updated = management_alias_config_changes(updated, &latest)?; + if (from_current.update_config_yaml && from_updated.update_config_yaml) + || (from_current.oauth_model_aliases.is_some() + && from_updated.oauth_model_aliases.is_some()) + { + return Err("检测到其他配置修改，未覆盖这些修改".to_string()); + } + let mut errors = Vec::new(); + if from_current.update_config_yaml { + if let Err(error) = put_management_config_yaml(config, current).await { + errors.push(error); + } + } + if let Some(aliases) = management_alias_config_changes(updated, current)?.oauth_model_aliases { + if let Err(error) = put_management_oauth_model_aliases(config, &aliases).await { + errors.push(error); + } + } + if !errors.is_empty() { + return Err(errors.join("；")); + } + if !alias_config_is_unchanged(current, &fetch_management_config_yaml(config).await?)? { + return Err("恢复后配置与原配置不一致".to_string()); + } + Ok(()) +} diff --git a/src-tauri/src/core_config/aliases.rs b/src-tauri/src/core_config/aliases.rs index 1f262b0..1367035 100644 --- a/src-tauri/src/core_config/aliases.rs +++ b/src-tauri/src/core_config/aliases.rs @@ -290,27 +290,6 @@ pub(crate) fn management_alias_config_changes( }) } -pub(crate) async fn put_management_alias_config_changes( - config: &GuiConfigFile, - current: &str, - updated: &str, -) -> Result<(), String> { - if updated == current { - return Ok(()); - } - let changes = management_alias_config_changes(current, updated)?; - if let Some(oauth_model_aliases) = changes.oauth_model_aliases.as_ref() { - // The dedicated endpoint refreshes CPA's OAuth model registry immediately. - // Writing the same section only through config.yaml updates the file and - // management snapshot, but can leave /v1/models and routing stale. - put_management_oauth_model_aliases(config, oauth_model_aliases).await?; - } - if changes.update_config_yaml { - put_management_config_yaml(config, updated).await?; - } - Ok(()) -} - pub(crate) async fn ensure_claude_desktop_model_aliases( config: &GuiConfigFile, mappings: &ClaudeDesktopModelMappings, @@ -876,11 +855,6 @@ pub(crate) fn resolved_oauth_alias_sources( let root = document .as_mapping() .ok_or_else(|| "内核配置顶层必须是 YAML 映射".to_string())?; - // A configured API-key model must win over a catalog entry with the same - // name. `model-definitions/codex` describes the OAuth channel's capabilities; - // it is not evidence that a model returned by /v1/models is using OAuth. - // Otherwise a Codex API alias would be written to oauth-model-alias, which - // CPA deliberately does not apply to codex-api-key credentials. let mut sources = Vec::new(); collect_config_thinking_alias_sources( root, @@ -1017,6 +991,11 @@ pub(crate) fn collect_config_thinking_alias_sources( } let provider_name = thinking_alias_provider_name(provider, fallback_provider, provider_index); + let provider_revision = sha256_bytes( + serde_norway::to_string(provider) + .map_err(|error| format!("读取模型源配置失败: {error}"))? + .as_bytes(), + ); let Some(models) = yaml_mapping_value(provider, "models") else { continue; }; @@ -1040,7 +1019,7 @@ pub(crate) fn collect_config_thinking_alias_sources( let reasoning_levels = configured_model_reasoning_levels(model, protocol); sources.push(ResolvedThinkingAliasSource { source: ThinkingAliasSource { - id: format!("{section}:{provider_index}:{model_index}"), + id: format!("{section}:{provider_index}:{model_index}:{provider_revision}"), model: client_model, display_name, provider: provider_name.clone(), @@ -1417,64 +1396,79 @@ pub(crate) fn collect_config_speed_alias_entries( Ok(()) } +fn alias_override_params<'a>( + root: &'a serde_norway::Mapping, + alias: &'a str, + protocol: &'a str, +) -> impl Iterator { + ["override-raw", "override"] + .into_iter() + .flat_map(move |section| { + nested_yaml_value(root, &["payload", section]) + .and_then(serde_norway::Value::as_sequence) + .into_iter() + .flatten() + .rev() + .filter_map(move |rule| { + let rule = rule.as_mapping()?; + let models = yaml_mapping_value(rule, "models")?.as_sequence()?; + if !models + .iter() + .any(|model| thinking_payload_model_matches(model, alias, protocol)) + { + return None; + } + Some(( + yaml_mapping_value(rule, "params")?.as_mapping()?, + section == "override-raw", + )) + }) + }) +} + +fn alias_payload_string(params: &serde_norway::Mapping, key: &str, raw: bool) -> Option { + let value = yaml_mapping_value(params, key)?.as_str()?; + let decoded; + let value = if raw { + decoded = serde_json::from_str::(value).ok()?; + decoded.as_str()? + } else { + value + }; + let value = value.trim().to_ascii_lowercase(); + (!value.is_empty()).then_some(value) +} + +pub(crate) fn thinking_effort_from_params( + params: &serde_norway::Mapping, + protocol: &str, + raw: bool, +) -> Option { + [ + "reasoning.effort", + "reasoning_effort", + "output_config.effort", + "generationConfig.thinkingConfig.thinkingLevel", + "thinking.effort", + ] + .into_iter() + .find_map(|key| alias_payload_string(params, key, raw)) + .or_else( + || match alias_payload_string(params, "thinking.type", raw)?.as_str() { + "disabled" => Some("none".to_string()), + "adaptive" if protocol.eq_ignore_ascii_case("claude") => Some("auto".to_string()), + _ => None, + }, + ) +} + pub(crate) fn find_thinking_alias_effort( root: &serde_norway::Mapping, alias: &str, protocol: &str, ) -> Option { - let rules = nested_yaml_value(root, &["payload", "override"])?.as_sequence()?; - for rule in rules { - let Some(rule) = rule.as_mapping() else { - continue; - }; - let effort = yaml_mapping_value(rule, "params") - .and_then(serde_norway::Value::as_mapping) - .and_then(|params| { - let explicit = [ - "reasoning.effort", - "reasoning_effort", - "output_config.effort", - "generationConfig.thinkingConfig.thinkingLevel", - "thinking.effort", - ] - .into_iter() - .find_map(|key| yaml_mapping_value(params, key)) - .and_then(serde_norway::Value::as_str) - .map(str::trim) - .filter(|value| !value.is_empty()) - .map(str::to_ascii_lowercase); - explicit.or_else(|| { - let thinking_type = yaml_mapping_value(params, "thinking.type") - .and_then(serde_norway::Value::as_str) - .map(str::trim)?; - if thinking_type.eq_ignore_ascii_case("disabled") { - Some("none".to_string()) - } else if protocol.eq_ignore_ascii_case("claude") - && thinking_type.eq_ignore_ascii_case("adaptive") - { - Some("auto".to_string()) - } else { - None - } - }) - }) - .filter(|value| !value.is_empty()); - let Some(effort) = effort else { - continue; - }; - let Some(models) = - yaml_mapping_value(rule, "models").and_then(serde_norway::Value::as_sequence) - else { - continue; - }; - if models - .iter() - .any(|model| thinking_payload_model_matches(model, alias, protocol)) - { - return Some(effort); - } - } - None + alias_override_params(root, alias, protocol) + .find_map(|(params, raw)| thinking_effort_from_params(params, protocol, raw)) } pub(crate) fn find_speed_alias_service_tier( @@ -1482,33 +1476,8 @@ pub(crate) fn find_speed_alias_service_tier( alias: &str, protocol: &str, ) -> Option { - let rules = nested_yaml_value(root, &["payload", "override"])?.as_sequence()?; - for rule in rules { - let Some(rule) = rule.as_mapping() else { - continue; - }; - let service_tier = yaml_mapping_value(rule, "params") - .and_then(serde_norway::Value::as_mapping) - .and_then(|params| yaml_mapping_value(params, "service_tier")) - .and_then(serde_norway::Value::as_str) - .map(str::trim) - .filter(|value| !value.is_empty()); - let Some(service_tier) = service_tier else { - continue; - }; - let Some(models) = - yaml_mapping_value(rule, "models").and_then(serde_norway::Value::as_sequence) - else { - continue; - }; - if models - .iter() - .any(|model| thinking_payload_model_matches(model, alias, protocol)) - { - return Some(service_tier.to_ascii_lowercase()); - } - } - None + alias_override_params(root, alias, protocol) + .find_map(|(params, raw)| alias_payload_string(params, "service_tier", raw)) } pub(crate) fn thinking_payload_model_matches( @@ -1522,9 +1491,11 @@ pub(crate) fn thinking_payload_model_matches( let name_matches = yaml_mapping_value(model, "name") .and_then(serde_norway::Value::as_str) .is_some_and(|name| name.trim().eq_ignore_ascii_case(alias)); - let protocol_matches = yaml_mapping_value(model, "protocol") - .and_then(serde_norway::Value::as_str) - .is_some_and(|value| value.trim().eq_ignore_ascii_case(protocol)); + let protocol_matches = yaml_mapping_value(model, "protocol").is_none_or(|value| { + value.as_str().is_some_and(|value| { + value.trim().is_empty() || value.trim().eq_ignore_ascii_case(protocol) + }) + }); name_matches && protocol_matches } @@ -1569,7 +1540,6 @@ pub(crate) fn insert_thinking_effort_params( ); } "antigravity-oauth" => { - // Antigravity applies payload rules relative to its `request` object. insert( params, "generationConfig.thinkingConfig.thinkingLevel", diff --git a/src-tauri/src/tests.rs b/src-tauri/src/tests.rs index 741aaf8..d062da5 100644 --- a/src-tauri/src/tests.rs +++ b/src-tauri/src/tests.rs @@ -4,6 +4,8 @@ mod agent_configuration; mod agent_state; mod agent_transactions; mod alias_edit; +mod alias_edit_regressions; +mod alias_save; mod app_settings; mod app_update; mod core_config; diff --git a/src-tauri/src/tests/alias_edit.rs b/src-tauri/src/tests/alias_edit.rs index be96f1c..90dab49 100644 --- a/src-tauri/src/tests/alias_edit.rs +++ b/src-tauri/src/tests/alias_edit.rs @@ -37,7 +37,7 @@ fn alias_edit_preserves_other_parameters_and_shared_models() { let before = value(MIXED_PAYLOAD); let after = value(&updated); let rules = after["payload"]["override"].as_array().unwrap(); - assert_eq!(rules.len(), 3); + assert_eq!(rules.len(), 2); assert_eq!(rules[0]["models"].as_array().unwrap().len(), 1); assert_eq!(rules[0]["models"][0]["name"], "other-alias"); assert_eq!( @@ -47,11 +47,7 @@ fn alias_edit_preserves_other_parameters_and_shared_models() { assert_eq!(rules[1]["models"][0]["name"], "my-alias"); assert_eq!( rules[1]["params"], - serde_json::json!({"temperature": 0.2, "max_output_tokens": 1024}) - ); - assert_eq!( - rules[2]["params"], - serde_json::json!({"reasoning.effort": "low"}) + serde_json::json!({"temperature": 0.2, "max_output_tokens": 1024, "reasoning.effort": "low"}) ); assert_eq!(after["oauth-model-alias"], before["oauth-model-alias"]); assert!(updated.contains("# Keep this comment")); @@ -109,7 +105,6 @@ fn alias_edit_keeps_api_metadata_and_order_on_an_unchanged_save() { let content = "openai-compatibility:\n - name: my-provider\n models:\n - name: gpt-test\n display-name: Base Model\n - name: gpt-test\n alias: my-alias\n display-name: Custom Alias Label\n thinking:\n levels: [low, high]\n custom: {keep: true}\n - name: other\n"; let updated = edit(content, "my-alias", "my-alias", "", false); assert_eq!(value(&updated), value(content)); - // Selecting the base entry for the same upstream must preserve alias metadata too. let source = resolved_oauth_alias_sources( content, &[], @@ -198,7 +193,7 @@ fn alias_edit_selects_exact_provider_even_with_identical_display_names() { .unwrap(); let source = sources .iter() - .find(|source| source.source.id == "openai-compatibility:1:0") + .find(|source| source.source.id.starts_with("openai-compatibility:1:0:")) .unwrap(); let updated = edit_model_alias_in_yaml(content, "my-alias", source, "my-alias", "", false).unwrap(); @@ -240,7 +235,7 @@ fn alias_edit_moves_between_api_and_oauth_and_updates_payload_protocol() { ); assert_eq!( after["payload"]["override"][0]["params"], - serde_json::json!({"temperature":0.2}) + serde_json::json!({"temperature":0.2, "reasoning.effort":"low"}) ); let content = format!("{updated}codex-api-key:\n - models: [{{name: back-to-api}}]\n"); let source = resolved_oauth_alias_sources( @@ -282,7 +277,6 @@ fn alias_edit_rejects_duplicates_missing_aliases_and_name_collisions() { " fork: false\n - name: another\n alias: taken", ); assert!(edit_model_alias_in_yaml(&occupied, "my-alias", &source, "TAKEN", "", false).is_err()); - // A real model entry without an alias is not an editable alias. assert!(resolve_model_alias_edit_source( "codex-api-key:\n - models: [{name: real-model}]\n", "real-model", diff --git a/src-tauri/src/tests/alias_edit_regressions.rs b/src-tauri/src/tests/alias_edit_regressions.rs new file mode 100644 index 0000000..e67794c --- /dev/null +++ b/src-tauri/src/tests/alias_edit_regressions.rs @@ -0,0 +1,196 @@ +use super::support::*; +use super::*; + +const BASE: &str = + "oauth-model-alias:\n codex:\n - name: gpt-test\n alias: my-alias\n fork: true\n"; + +fn json(content: &str) -> serde_json::Value { + serde_json::to_value(serde_norway::from_str::(content).unwrap()).unwrap() +} + +fn unchanged_save(content: &str) -> String { + let context = model_alias_edit_context(content, "my-alias", &[]).unwrap(); + let source = resolve_model_alias_edit_source(content, "my-alias", &[]).unwrap(); + edit_model_alias_in_yaml( + content, + "my-alias", + &source, + "my-alias", + context.effort.as_deref().unwrap_or(""), + context.fast, + ) + .unwrap() +} + +#[test] +fn alias_regression_noop_preserves_raw_effort_and_fast() { + let content = format!("{BASE}payload:\n override-raw:\n - models: [{{name: my-alias, protocol: codex}}]\n params: {{reasoning.effort: '\"high\"', service_tier: '\"priority\"'}}\n"); + let updated = unchanged_save(&content); + assert_eq!( + json(&updated), + json(&content), + "An unchanged save must preserve raw effort/Fast overrides" + ); +} + +#[test] +fn alias_regression_noop_preserves_rules_without_protocol() { + let content = format!("{BASE}payload:\n override:\n - models: [{{name: my-alias}}]\n params: {{reasoning.effort: high, service_tier: priority}}\n"); + let updated = unchanged_save(&content); + assert_eq!( + json(&updated), + json(&content), + "Omitting protocol is a valid match-all selector, not an unset option" + ); +} + +#[test] +fn alias_regression_noop_preserves_conditional_effort_and_fast() { + let content = format!("{BASE}payload:\n override:\n - models:\n - name: my-alias\n protocol: codex\n headers: {{X-Client: premium}}\n from-protocol: responses\n params: {{reasoning.effort: high, service_tier: priority}}\n"); + let updated = unchanged_save(&content); + assert_eq!( + json(&updated), + json(&content), + "An unchanged save must not broaden a conditional override to every request" + ); +} + +#[test] +fn alias_regression_rejects_collision_with_real_model() { + let content = "codex-api-key:\n - models:\n - name: my-alias\n - name: gpt-test\n alias: my-alias\n"; + assert!(resolve_model_alias_edit_source(content, "my-alias", &[]).is_err()); + let unambiguous = content.replace(" - name: my-alias\n", ""); + let source = resolve_model_alias_edit_source(&unambiguous, "my-alias", &[]).unwrap(); + assert!(edit_model_alias_in_yaml(content, "my-alias", &source, "renamed", "", false).is_err()); +} + +#[test] +fn alias_regression_source_id_does_not_retarget_after_reordering() { + let content = "openai-compatibility:\n - name: original-provider\n models: [{name: original-model, alias: my-alias}]\n - name: intended-provider\n models: [{name: intended-model}]\n - name: other-provider\n models: [{name: other-model}]\n"; + let available = test_agent_models(&["my-alias", "intended-model", "other-model"]); + let chosen = + resolved_oauth_alias_sources(content, &[], &available, AliasSourceCapability::Base) + .unwrap() + .into_iter() + .find(|source| source.source.model == "intended-model") + .unwrap(); + let mut changed = json(content); + changed["openai-compatibility"] + .as_array_mut() + .unwrap() + .swap(1, 2); + let changed = serde_norway::to_string(&changed).unwrap(); + let resolved = + resolved_oauth_alias_sources(&changed, &[], &available, AliasSourceCapability::Base) + .unwrap() + .into_iter() + .find(|source| source.source.id == chosen.source.id); + assert!( + resolved.is_none(), + "A stale positional selection must be rejected" + ); +} + +#[test] +fn alias_regression_edit_snapshot_rejects_recreated_alias_and_missing_revision() { + let context = model_alias_edit_context(BASE, "my-alias", &[]).unwrap(); + assert!(validate_model_alias_revision(BASE, Some(&context.revision)).is_ok()); + let reformatted = serde_norway::to_string(&json(BASE)).unwrap(); + assert!(validate_model_alias_revision(&reformatted, Some(&context.revision)).is_ok()); + assert!(validate_model_alias_revision(BASE, None).is_err()); + let recreated = BASE.replace("gpt-test", "another-upstream"); + assert!(validate_model_alias_revision(&recreated, Some(&context.revision)).is_err()); +} + +#[test] +fn alias_regression_readers_use_raw_and_last_override_values() { + let content = format!("{BASE}payload:\n override:\n - models: [{{name: my-alias, protocol: codex}}]\n params: {{reasoning.effort: high, service_tier: priority}}\n - models: [{{name: my-alias, protocol: codex}}]\n params: {{reasoning.effort: low}}\n override-raw:\n - models: [{{name: my-alias}}]\n params: {{service_tier: '\"flex\"'}}\n"); + let context = model_alias_edit_context(&content, "my-alias", &[]).unwrap(); + assert_eq!(context.effort.as_deref(), Some("low")); + assert!(!context.fast); + assert_eq!(json(&unchanged_save(&content)), json(&content)); + assert_eq!( + thinking_aliases_from_yaml(&content).unwrap()[0] + .effort + .as_deref(), + Some("low") + ); + assert_eq!( + speed_aliases_from_yaml(&content).unwrap()[0].service_tier, + "flex" + ); +} + +#[test] +fn alias_regression_explicit_changes_keep_raw_conditions_and_other_fields() { + let content = format!("{BASE}payload:\n override-raw:\n - models:\n - name: my-alias\n protocol: ''\n headers: {{X-Client: premium}}\n from-protocol: responses\n match: [{{metadata.client: codex}}]\n - name: other-alias\n params: {{reasoning.effort: '\"high\"', service_tier: '\"priority\"', temperature: '0.2'}}\n"); + let source = resolve_model_alias_edit_source(&content, "my-alias", &[]).unwrap(); + let updated = + edit_model_alias_in_yaml(&content, "my-alias", &source, "renamed", "low", true).unwrap(); + let before = json(&content); + let after = json(&updated); + assert!( + after["payload"]["override"].is_null(), + "Existing conditional settings must not produce unconditional rules" + ); + let rules = after["payload"]["override-raw"].as_array().unwrap(); + assert_eq!(rules.len(), 2); + assert_eq!( + rules[0]["params"], + before["payload"]["override-raw"][0]["params"] + ); + assert_eq!(rules[0]["models"][0]["name"], "other-alias"); + let mut expected_selector = before["payload"]["override-raw"][0]["models"][0].clone(); + expected_selector["name"] = serde_json::json!("renamed"); + assert_eq!(rules[1]["models"][0], expected_selector); + assert_eq!( + rules[1]["params"], + serde_json::json!({"reasoning.effort":"\"low\"", "service_tier":"\"priority\"", "temperature":"0.2"}) + ); + let context = model_alias_edit_context(&updated, "renamed", &[]).unwrap(); + assert_eq!(context.effort.as_deref(), Some("low")); + assert!(context.fast); +} + +#[test] +fn alias_regression_toggle_fast_preserves_distinct_conditional_efforts() { + let content = format!("{BASE}payload:\n override:\n - models: [{{name: my-alias, protocol: codex, headers: {{X-Client: a}}}}]\n params: {{reasoning.effort: low, service_tier: flex}}\n - models: [{{name: my-alias, protocol: codex, headers: {{X-Client: b}}}}]\n params: {{reasoning.effort: high, service_tier: flex}}\n"); + let source = resolve_model_alias_edit_source(&content, "my-alias", &[]).unwrap(); + let updated = + edit_model_alias_in_yaml(&content, "my-alias", &source, "my-alias", "high", true).unwrap(); + let after = json(&updated); + let rules = after["payload"]["override"].as_array().unwrap(); + assert_eq!(rules.len(), 2); + assert_eq!(rules[0]["params"]["reasoning.effort"], "low"); + assert_eq!(rules[1]["params"]["reasoning.effort"], "high"); + for (index, rule) in rules.iter().enumerate() { + assert_eq!( + rule["models"], + json(&content)["payload"]["override"][index]["models"] + ); + assert_eq!(rule["params"]["service_tier"], "priority"); + } +} + +#[test] +fn alias_regression_provider_change_with_same_model_invalidates_selection() { + let content = "openai-compatibility:\n - name: provider\n base-url: https://one.example/v1\n models: [{name: shared-model}]\n"; + let models = test_agent_models(&["shared-model"]); + let original = + resolved_oauth_alias_sources(content, &[], &models, AliasSourceCapability::Base).unwrap(); + let changed = resolved_oauth_alias_sources( + &content.replace("one.example", "two.example"), + &[], + &models, + AliasSourceCapability::Base, + ) + .unwrap(); + assert_eq!(original[0].source.model, changed[0].source.model); + assert_ne!(original[0].source.id, changed[0].source.id); +} + +#[test] +fn alias_regression_unchanged_save_keeps_comments_and_empty_rules_byte_for_byte() { + let content = format!("# custom config\n{BASE}payload: {{override: []}} # keep placeholder\n"); + assert_eq!(unchanged_save(&content), content); +} diff --git a/src-tauri/src/tests/alias_save.rs b/src-tauri/src/tests/alias_save.rs new file mode 100644 index 0000000..faf8cce --- /dev/null +++ b/src-tauri/src/tests/alias_save.rs @@ -0,0 +1,296 @@ +use super::*; +use std::io::{Read, Write}; +use std::net::TcpListener; +use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::Arc; + +const CURRENT: &str = "oauth-model-alias:\n codex:\n - name: gpt-test\n alias: my-alias\n fork: true\npayload:\n override:\n - models: [{name: my-alias, protocol: codex}]\n params: {reasoning.effort: high}\n"; + +#[derive(Clone, Copy, PartialEq, Eq)] +enum Failure { + None, + YamlBeforeWrite, + YamlAfterWrite, + OauthAfterWrite, + Rollback, + ForeignChange, +} + +struct MockCore { + config: GuiConfigFile, + finished: Arc, + server: Option)>>, +} + +impl MockCore { + fn new(initial: &str, failure: Failure) -> Self { + assert!(!current_core_tls_settings().unwrap().enabled); + let listener = TcpListener::bind("127.0.0.1:0").unwrap(); + let port = listener.local_addr().unwrap().port(); + listener.set_nonblocking(true).unwrap(); + let finished = Arc::new(AtomicBool::new(false)); + let server_finished = finished.clone(); + let mut persisted = yaml_json(initial); + let server = std::thread::spawn(move || { + let deadline = std::time::Instant::now() + Duration::from_secs(20); + let mut requests = Vec::new(); + let mut oauth_writes = 0; + let mut yaml_writes = 0; + while !server_finished.load(Ordering::SeqCst) && std::time::Instant::now() < deadline { + let (mut stream, _) = match listener.accept() { + Ok(connection) => connection, + Err(error) if error.kind() == std::io::ErrorKind::WouldBlock => { + std::thread::sleep(Duration::from_millis(2)); + continue; + } + Err(error) => panic!("mock accept: {error}"), + }; + stream.set_nonblocking(false).unwrap(); + stream + .set_read_timeout(Some(Duration::from_secs(2))) + .unwrap(); + let mut bytes = Vec::new(); + let (header_end, content_length) = loop { + let mut buffer = [0; 2048]; + let read = stream.read(&mut buffer).unwrap(); + assert!(read > 0); + bytes.extend_from_slice(&buffer[..read]); + if let Some(end) = bytes.windows(4).position(|window| window == b"\r\n\r\n") { + let header = String::from_utf8_lossy(&bytes[..end]); + let length: usize = header + .lines() + .find_map(|line| { + let (key, value) = line.split_once(':')?; + key.eq_ignore_ascii_case("content-length") + .then(|| value.trim().parse().unwrap()) + }) + .unwrap_or(0); + break (end + 4, length); + } + }; + while bytes.len() < header_end + content_length { + let mut buffer = [0; 2048]; + let read = stream.read(&mut buffer).unwrap(); + assert!(read > 0); + bytes.extend_from_slice(&buffer[..read]); + } + let header = String::from_utf8_lossy(&bytes[..header_end]); + let first = header.lines().next().unwrap(); + let mut parts = first.split_whitespace(); + let method = parts.next().unwrap(); + let path = parts.next().unwrap(); + requests.push(format!("{method} {path}")); + let request_body = &bytes[header_end..header_end + content_length]; + let mut status = "200 OK"; + let mut response = "{\"status\":\"ok\"}".to_string(); + if method == "GET" { + assert_eq!(path, "/v0/management/config.yaml"); + response = serde_norway::to_string(&persisted).unwrap(); + } else if path == "/v0/management/oauth-model-alias" { + oauth_writes += 1; + if failure == Failure::Rollback && oauth_writes == 2 { + status = "500 Internal Server Error"; + } else { + persisted["oauth-model-alias"] = + serde_json::from_slice(request_body).unwrap(); + if failure == Failure::OauthAfterWrite && oauth_writes == 1 { + status = "500 Internal Server Error"; + } + } + } else { + assert_eq!(path, "/v0/management/config.yaml"); + yaml_writes += 1; + if yaml_writes == 1 + && matches!( + failure, + Failure::YamlBeforeWrite | Failure::Rollback | Failure::ForeignChange + ) + { + status = "500 Internal Server Error"; + if failure == Failure::ForeignChange { + persisted["debug"] = serde_json::json!(true); + } + } else { + persisted = yaml_json(std::str::from_utf8(request_body).unwrap()); + if yaml_writes == 1 && failure == Failure::YamlAfterWrite { + status = "500 Internal Server Error"; + } + } + } + if status.starts_with("500") { + response = "{\"error\":\"write_failed\"}".to_string(); + } + write!(stream, "HTTP/1.1 {status}\r\nContent-Type: text/plain\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{response}", response.len()).unwrap(); + } + (persisted, requests) + }); + Self { + config: GuiConfigFile { + port, + management_secret_key: "test-only".to_string(), + ..Default::default() + }, + finished, + server: Some(server), + } + } + + fn finish(mut self) -> (serde_json::Value, Vec) { + self.finished.store(true, Ordering::SeqCst); + self.server.take().unwrap().join().unwrap() + } +} + +impl Drop for MockCore { + fn drop(&mut self) { + self.finished.store(true, Ordering::SeqCst); + if let Some(server) = self.server.take() { + let _ = server.join(); + } + } +} + +fn yaml_json(content: &str) -> serde_json::Value { + serde_json::to_value(serde_norway::from_str::(content).unwrap()).unwrap() +} + +#[tokio::test(flavor = "current_thread")] +async fn alias_save_recovers_failures_before_and_after_writes() { + for failure in [ + Failure::YamlBeforeWrite, + Failure::YamlAfterWrite, + Failure::OauthAfterWrite, + ] { + let core = MockCore::new(CURRENT, failure); + let updated = CURRENT.replace("my-alias", "renamed"); + let result = put_management_alias_config_changes(&core.config, CURRENT, &updated).await; + assert!(result.unwrap_err().contains("已恢复原配置")); + let (persisted, requests) = core.finish(); + assert_eq!(persisted, yaml_json(CURRENT), "{requests:?}"); + assert_eq!( + requests + .iter() + .filter(|request| request.ends_with("oauth-model-alias")) + .count(), + 2 + ); + } +} + +#[tokio::test(flavor = "current_thread")] +async fn alias_save_reports_failed_rollback_without_claiming_restoration() { + let core = MockCore::new(CURRENT, Failure::Rollback); + let result = put_management_alias_config_changes( + &core.config, + CURRENT, + &CURRENT.replace("my-alias", "renamed"), + ) + .await; + let error = result.unwrap_err(); + assert!(error.contains("自动恢复失败")); + assert!(error.contains("配置可能已部分写入")); + let (persisted, _) = core.finish(); + assert_eq!( + persisted["oauth-model-alias"]["codex"][0]["alias"], + "renamed" + ); + assert_eq!(persisted["payload"], yaml_json(CURRENT)["payload"]); +} + +#[tokio::test(flavor = "current_thread")] +async fn alias_save_does_not_rollback_over_external_changes() { + let core = MockCore::new(CURRENT, Failure::ForeignChange); + let result = put_management_alias_config_changes( + &core.config, + CURRENT, + &CURRENT.replace("my-alias", "renamed"), + ) + .await; + assert!(result.unwrap_err().contains("检测到其他配置修改")); + let (persisted, requests) = core.finish(); + assert_eq!(persisted["debug"], true); + assert_eq!( + requests + .iter() + .filter(|request| request.starts_with("PUT")) + .count(), + 2 + ); +} + +#[tokio::test(flavor = "current_thread")] +async fn alias_save_rejects_stale_snapshot_before_any_write() { + let core = MockCore::new(&format!("{CURRENT}debug: true\n"), Failure::None); + let result = put_management_alias_config_changes( + &core.config, + CURRENT, + &CURRENT.replace("my-alias", "renamed"), + ) + .await; + assert!(result.unwrap_err().contains("配置已变化")); + let (_, requests) = core.finish(); + assert_eq!(requests, ["GET /v0/management/config.yaml"]); +} + +#[tokio::test(flavor = "current_thread")] +async fn alias_save_succeeds_for_mixed_oauth_only_and_yaml_only_changes() { + for updated in [ + CURRENT.replace("my-alias", "renamed"), + CURRENT.replacen("my-alias", "renamed", 1), + CURRENT.replace("high", "low"), + ] { + let core = MockCore::new(CURRENT, Failure::None); + put_management_alias_config_changes(&core.config, CURRENT, &updated) + .await + .unwrap(); + let (persisted, _) = core.finish(); + assert_eq!(persisted, yaml_json(&updated)); + } +} + +#[tokio::test(flavor = "current_thread")] +async fn alias_save_serializes_concurrent_writers_and_rejects_the_stale_one() { + let core = MockCore::new(CURRENT, Failure::None); + let first = CURRENT.replace("my-alias", "first"); + let second = CURRENT.replace("my-alias", "second"); + let (a, b) = tokio::join!( + put_management_alias_config_changes(&core.config, CURRENT, &first), + put_management_alias_config_changes(&core.config, CURRENT, &second), + ); + assert_ne!(a.is_ok(), b.is_ok()); + let (persisted, requests) = core.finish(); + assert_eq!( + persisted, + yaml_json(if a.is_ok() { &first } else { &second }) + ); + assert_eq!( + requests + .iter() + .filter(|request| request.starts_with("PUT")) + .count(), + 2 + ); +} + +#[tokio::test(flavor = "current_thread")] +async fn alias_save_can_retry_after_restoration_reformats_yaml() { + let context = model_alias_edit_context(CURRENT, "my-alias", &[]).unwrap(); + let core = MockCore::new(CURRENT, Failure::YamlBeforeWrite); + let updated = CURRENT.replace("my-alias", "renamed"); + assert!( + put_management_alias_config_changes(&core.config, CURRENT, &updated) + .await + .unwrap_err() + .contains("已恢复原配置") + ); + let restored = fetch_management_config_yaml(&core.config).await.unwrap(); + validate_model_alias_revision(&restored, Some(&context.revision)).unwrap(); + let source = resolve_model_alias_edit_source(&restored, "my-alias", &[]).unwrap(); + let updated = + edit_model_alias_in_yaml(&restored, "my-alias", &source, "renamed", "high", false).unwrap(); + put_management_alias_config_changes(&core.config, &restored, &updated) + .await + .unwrap(); + let (persisted, _) = core.finish(); + assert_eq!(persisted, yaml_json(&updated)); +} diff --git a/src/pages/ThinkingAliasesPage.tsx b/src/pages/ThinkingAliasesPage.tsx index 9e1967c..cedbeae 100644 --- a/src/pages/ThinkingAliasesPage.tsx +++ b/src/pages/ThinkingAliasesPage.tsx @@ -62,6 +62,13 @@ type ModelAliasSource = ThinkingAliasSource & { supportsFast: boolean; }; +type ModelAliasEditContext = { + source: ThinkingAliasSource; + revision: string; + effort: string | null; + fast: boolean; +}; + const effortOptions = [ { value: 'low', label: 'Low', hintKey: 'aliases.effort.low' }, { value: 'medium', label: 'Medium', hintKey: 'aliases.effort.medium' }, @@ -191,6 +198,7 @@ export function ThinkingAliasesPage() { const [alias, setAlias] = useState(''); const [editingEntry, setEditingEntry] = useState(null); const [editingSource, setEditingSource] = useState(null); + const [editingRevision, setEditingRevision] = useState(null); const [editorOpen, setEditorOpen] = useState(false); const [search, setSearch] = useState(''); const [modelPickerOpen, setModelPickerOpen] = useState(false); @@ -405,6 +413,7 @@ export function ThinkingAliasesPage() { await invoke('create_thinking_alias', { sourceId: selectedSource.id, alias: normalizedAlias, effort: normalizedEffort, fast: fastEnabled, originalAlias: editingEntry.alias, + expectedRevision: editingRevision, }); setNotice(t('aliases.updated', { alias: normalizedAlias })); } else if (normalizedEffort) { @@ -448,6 +457,7 @@ export function ThinkingAliasesPage() { const resetEditor = () => { setEditingEntry(null); setEditingSource(null); + setEditingRevision(null); setSelectedSourceId(''); setEffort(''); setFastEnabled(false); @@ -476,12 +486,14 @@ export function ThinkingAliasesPage() { setError(''); setNotice(''); try { - const source = await invoke('get_model_alias_edit_source', { alias: entry.alias }); + const context = await invoke('get_model_alias_edit_source', { alias: entry.alias }); + const source = context.source; setEditingEntry(entry); setEditingSource(source); + setEditingRevision(context.revision); setSelectedSourceId(source.id); - setEffort(entry.effort ?? ''); - setFastEnabled(entry.serviceTier === 'priority'); + setEffort(context.effort ?? ''); + setFastEnabled(context.fast); setAlias(entry.alias); generatedAliasRef.current = ''; setModelPickerOpen(false); diff --git a/tests/fixtures/model-alias-editor.tsx b/tests/fixtures/model-alias-editor.tsx index 30065c0..62479ec 100644 --- a/tests/fixtures/model-alias-editor.tsx +++ b/tests/fixtures/model-alias-editor.tsx @@ -9,8 +9,9 @@ localStorage.setItem('easy-cli-proxy-api.locale', 'en'); const calls: { cmd: string; args: Record }[] = []; (window as any).fixtureCalls = calls; (window as any).fixtureFailSave = false; +(window as any).fixtureCurrentEffort = null; const source = { - id: 'openai-compatibility:0:0', model: 'public-gpt', displayName: null, + id: 'openai-compatibility:0:0:provider-revision', model: 'public-gpt', displayName: null, provider: 'Provider', kind: 'openai-compatible', protocol: 'openai', reasoningLevels: ['low', 'high'], }; let entry = { @@ -24,7 +25,10 @@ mockIPC(async (cmd, rawArgs) => { if (cmd === 'get_thinking_aliases') return [entry]; if (cmd === 'get_speed_aliases') return []; if (['get_model_alias_sources', 'get_thinking_alias_sources', 'get_speed_alias_sources'].includes(cmd)) return [source]; - if (cmd === 'get_model_alias_edit_source') return { ...source, id: `alias-edit:${args.alias}`, model: 'upstream-gpt' }; + if (cmd === 'get_model_alias_edit_source') return { + source: { ...source, id: `alias-edit:${args.alias}`, model: 'upstream-gpt' }, + revision: `revision:${args.alias}`, effort: (window as any).fixtureCurrentEffort ?? entry.effort, fast: false, + }; if (cmd === 'create_thinking_alias') { if ((window as any).fixtureFailSave) throw new Error('Save failed: configuration changed'); entry = { ...entry, alias: args.alias, effort: args.effort }; diff --git a/tests/model-alias-editor-ui.cjs b/tests/model-alias-editor-ui.cjs index 1f9af4f..5d831bb 100644 --- a/tests/model-alias-editor-ui.cjs +++ b/tests/model-alias-editor-ui.cjs @@ -1,5 +1,3 @@ -// Run Vite on port 1423, then: node tests/model-alias-editor-ui.cjs -// Set PLAYWRIGHT_MODULE to an installed Playwright module path when needed. const { chromium } = require(process.env.PLAYWRIGHT_MODULE || 'playwright'); const assert = require('node:assert/strict'); const { mkdirSync } = require('node:fs'); @@ -9,7 +7,6 @@ const { mkdirSync } = require('node:fs'); try { const page = await browser.newPage({ viewport: { width: 1280, height: 800 } }); page.setDefaultTimeout(10000); - // External font loading is unrelated to the editor's behavior. await page.route('**/*', route => route.request().url().startsWith('http://127.0.0.1:1423/') ? route.continue() : route.abort()); const errors = []; @@ -21,7 +18,6 @@ const { mkdirSync } = require('node:fs'); await dialog.waitFor(); assert.equal(await dialog.locator('#thinking-model-search').inputValue(), 'upstream-gpt'); assert.equal(await dialog.locator('#thinking-alias-name').inputValue(), 'my-alias'); - // Escape dismisses the model picker before dismissing the editor. await page.keyboard.press('Escape'); assert.equal(await dialog.count(), 1); assert.equal(await page.getByRole('listbox').count(), 0); @@ -48,18 +44,21 @@ const { mkdirSync } = require('node:fs'); const saved = (await writes()).at(-1).args; assert.equal(saved.sourceId, 'alias-edit:my-alias'); assert.equal(saved.originalAlias, 'my-alias'); + assert.equal(saved.expectedRevision, 'revision:my-alias'); assert.equal(saved.alias, 'renamed-alias'); assert.equal(saved.effort, 'low'); await page.getByText('Updated model alias renamed-alias', { exact: true }).waitFor(); - // Switch from the original configuration source to a live model source. + await page.evaluate(() => { window.fixtureCurrentEffort = 'high'; }); await page.getByRole('button', { name: 'Edit', exact: true }).click(); + assert.match(await dialog.getByRole('button', { name: 'High', exact: true }).getAttribute('class'), /active/); await dialog.locator('#thinking-model-search').fill('public-gpt'); await page.getByRole('option').click(); assert.equal(await dialog.locator('#thinking-alias-name').inputValue(), 'renamed-alias'); await dialog.getByRole('button', { name: 'Save', exact: true }).click(); await dialog.waitFor({ state: 'detached' }); - assert.equal((await writes()).at(-1).args.sourceId, 'openai-compatibility:0:0'); + assert.equal((await writes()).at(-1).args.sourceId, 'openai-compatibility:0:0:provider-revision'); + assert.equal((await writes()).at(-1).args.expectedRevision, 'revision:renamed-alias'); await page.setViewportSize({ width: 640, height: 600 }); await page.getByRole('button', { name: 'Create Alias', exact: true }).click(); @@ -82,6 +81,7 @@ const { mkdirSync } = require('node:fs'); await dialog.getByRole('button', { name: 'Create Alias', exact: true }).click(); await dialog.waitFor({ state: 'detached' }); assert.equal((await writes()).at(-1).args.originalAlias, undefined); + assert.equal((await writes()).at(-1).args.expectedRevision, undefined); assert.equal((await writes()).at(-1).args.alias, 'new-alias'); assert.deepEqual(errors, []); console.log('PASS: remapped source, cancel, focus trap, Escape, inline errors, retry, rename, source switch, create, narrow dialog');