From fda88b2e4295797e91f9bc780c474f63ba293d46 Mon Sep 17 00:00:00 2001 From: YoVinchen Date: Mon, 30 Mar 2026 22:38:32 +0800 Subject: [PATCH] fix(provider): distinguish legacy providers from db-only when tolerating live config errors Change `ProviderMeta.live_config_managed` from `bool` to `Option` to introduce a three-state semantic: - `Some(true)`: provider has been written to live config - `Some(false)`: explicitly db-only, never written to live config - `None`: legacy data or unknown state (pre-existing providers) Previously, legacy providers defaulted to `live_config_managed = false` via `#[serde(default)]`, which silently swallowed live config parse errors. This could mask genuine configuration issues for providers that had actually been synced to live config before the field was introduced. Now, only providers with an explicit `Some(false)` marker tolerate parse errors; legacy `None` providers surface errors as before, preserving safety for already-managed configurations. Also wrap the `ensureQueryData` call for live provider IDs during duplication in a try/catch so that a malformed config file shows a user-facing toast instead of silently failing. Add tests for both the legacy error propagation path and the frontend duplication failure scenario. --- src-tauri/src/provider.rs | 10 +-- src-tauri/src/services/provider/mod.rs | 104 +++++++++++++++++-------- src/App.tsx | 35 ++++++--- tests/integration/App.test.tsx | 49 ++++++++++++ 4 files changed, 147 insertions(+), 51 deletions(-) diff --git a/src-tauri/src/provider.rs b/src-tauri/src/provider.rs index 19d8d10e..9b624a21 100644 --- a/src-tauri/src/provider.rs +++ b/src-tauri/src/provider.rs @@ -284,13 +284,9 @@ pub struct ProviderMeta { #[serde(rename = "promptCacheKey", skip_serializing_if = "Option::is_none")] pub prompt_cache_key: Option, /// 累加模式应用中，该 provider 是否已写入 live config。 - /// 用于区分仅保存在数据库中的 provider，避免其更新被 live config 解析错误误伤。 - #[serde( - rename = "liveConfigManaged", - default, - skip_serializing_if = "std::ops::Not::not" - )] - pub live_config_managed: bool, + /// `None` 表示旧数据/未知状态，`Some(false)` 表示明确仅存在于数据库中。 + #[serde(rename = "liveConfigManaged", skip_serializing_if = "Option::is_none")] + pub live_config_managed: Option, /// 供应商类型标识（用于特殊供应商检测） /// - "github_copilot": GitHub Copilot 供应商 #[serde(rename = "providerType", skip_serializing_if = "Option::is_none")] diff --git a/src-tauri/src/services/provider/mod.rs b/src-tauri/src/services/provider/mod.rs index 4c8425e1..04f13752 100644 --- a/src-tauri/src/services/provider/mod.rs +++ b/src-tauri/src/services/provider/mod.rs @@ -235,7 +235,10 @@ base_url = "http://localhost:8080" .expect("query stored provider") .expect("provider should exist"); assert_eq!( - stored.meta.as_ref().map(|meta| meta.live_config_managed), + stored + .meta + .as_ref() + .and_then(|meta| meta.live_config_managed), Some(false), "db-only provider should be marked as not live-managed" ); @@ -260,6 +263,32 @@ base_url = "http://localhost:8080" assert_eq!(saved.name, "DeepSeek Edited"); }); } + + #[test] + fn legacy_additive_provider_still_errors_on_live_config_parse_failure() { + with_test_home(|state, home| { + let provider = openclaw_provider("legacy-provider"); + state + .db + .save_provider(AppType::OpenClaw.as_str(), &provider) + .expect("seed legacy provider without live_config_managed marker"); + + let openclaw_dir = home.join(".openclaw"); + fs::create_dir_all(&openclaw_dir).expect("create openclaw dir"); + fs::write(openclaw_dir.join("openclaw.json"), "{ invalid json5") + .expect("write malformed config"); + + let mut updated = provider.clone(); + updated.name = "Legacy Edited".to_string(); + + let err = ProviderService::update(state, AppType::OpenClaw, None, updated) + .expect_err("legacy providers should still surface live parse errors"); + assert!( + err.to_string().contains("Failed to parse OpenClaw config"), + "expected parse error, got {err:?}" + ); + }); + } } impl ProviderService { @@ -273,19 +302,33 @@ impl ProviderService { } /// Check whether a provider exists in live config, tolerating parse errors - /// for providers that have never been written to live (`live_config_managed == false`). + /// only for providers that are explicitly marked as DB-only. fn check_live_config_exists( app_type: &AppType, provider_id: &str, - is_db_only: bool, + live_config_managed: Option, ) -> Result { - if is_db_only { + if live_config_managed == Some(false) { Ok(provider_exists_in_live_config(app_type, provider_id).unwrap_or(false)) } else { provider_exists_in_live_config(app_type, provider_id) } } + fn provider_live_config_managed(provider: &Provider) -> Option { + provider + .meta + .as_ref() + .and_then(|meta| meta.live_config_managed) + } + + fn set_provider_live_config_managed(provider: &mut Provider, managed: bool) { + provider + .meta + .get_or_insert_with(Default::default) + .live_config_managed = Some(managed); + } + /// List all providers for an app type pub fn list( state: &AppState, @@ -323,10 +366,7 @@ impl ProviderService { Self::validate_provider_settings(&app_type, &provider)?; normalize_provider_common_config_for_storage(state.db.as_ref(), &app_type, &mut provider)?; if app_type.is_additive_mode() { - provider - .meta - .get_or_insert_with(Default::default) - .live_config_managed = add_to_live; + Self::set_provider_live_config_managed(&mut provider, add_to_live); } // Save to database @@ -394,12 +434,11 @@ impl ProviderService { app_type.as_str() ))); }; - let is_db_only = !existing_provider - .meta - .as_ref() - .is_some_and(|m| m.live_config_managed); - let original_in_live = - Self::check_live_config_exists(&app_type, &original_id, is_db_only)?; + let original_in_live = Self::check_live_config_exists( + &app_type, + &original_id, + Self::provider_live_config_managed(&existing_provider), + )?; if original_in_live { return Err(AppError::Message( "Provider key cannot be changed after the provider has been added to the app config" @@ -407,8 +446,11 @@ impl ProviderService { )); } - let next_id_in_live = - Self::check_live_config_exists(&app_type, &provider.id, is_db_only)?; + let next_id_in_live = Self::check_live_config_exists( + &app_type, + &provider.id, + Self::provider_live_config_managed(&existing_provider), + )?; if state .db .get_provider_by_id(&provider.id, app_type.as_str())? @@ -422,10 +464,7 @@ impl ProviderService { ))); } - provider - .meta - .get_or_insert_with(Default::default) - .live_config_managed = false; + Self::set_provider_live_config_managed(&mut provider, false); state.db.save_provider(app_type.as_str(), &provider)?; state.db.delete_provider(app_type.as_str(), &original_id)?; @@ -469,16 +508,16 @@ impl ProviderService { } return Ok(true); } - let is_db_only = !provider - .meta - .as_ref() - .is_some_and(|m| m.live_config_managed); - let live_config_managed = - Self::check_live_config_exists(&app_type, &provider.id, is_db_only)?; - provider - .meta - .get_or_insert_with(Default::default) - .live_config_managed = live_config_managed; + let live_config_managed = Self::check_live_config_exists( + &app_type, + &provider.id, + Self::provider_live_config_managed(&provider).or_else(|| { + existing_provider + .as_ref() + .and_then(Self::provider_live_config_managed) + }), + )?; + Self::set_provider_live_config_managed(&mut provider, live_config_managed); // Save to database after live-config presence is resolved so parse errors // do not report failure after already mutating DB state. @@ -664,10 +703,7 @@ impl ProviderService { } if let Some(mut provider) = state.db.get_provider_by_id(id, app_type.as_str())? { - provider - .meta - .get_or_insert_with(Default::default) - .live_config_managed = false; + Self::set_provider_live_config_managed(&mut provider, false); state.db.save_provider(app_type.as_str(), &provider)?; } diff --git a/src/App.tsx b/src/App.tsx index 20e88b33..16e2ef8a 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -615,16 +615,31 @@ function App() { }; if (activeApp === "opencode" || activeApp === "openclaw") { - const liveProviderIds = - activeApp === "opencode" - ? await queryClient.ensureQueryData({ - queryKey: ["opencodeLiveProviderIds"], - queryFn: () => providersApi.getOpenCodeLiveProviderIds(), - }) - : await queryClient.ensureQueryData({ - queryKey: openclawKeys.liveProviderIds, - queryFn: () => providersApi.getOpenClawLiveProviderIds(), - }); + let liveProviderIds: string[] = []; + try { + liveProviderIds = + activeApp === "opencode" + ? await queryClient.ensureQueryData({ + queryKey: ["opencodeLiveProviderIds"], + queryFn: () => providersApi.getOpenCodeLiveProviderIds(), + }) + : await queryClient.ensureQueryData({ + queryKey: openclawKeys.liveProviderIds, + queryFn: () => providersApi.getOpenClawLiveProviderIds(), + }); + } catch (error) { + console.error( + "[App] Failed to load live provider IDs for duplication", + error, + ); + const errorMessage = extractErrorMessage(error); + toast.error( + t("provider.duplicateLiveIdsLoadFailed", { + defaultValue: "读取配置中的供应商标识失败，请先修复配置后再试", + }) + (errorMessage ? `: ${errorMessage}` : ""), + ); + return; + } const existingKeys = Array.from( new Set([...Object.keys(providers), ...liveProviderIds]), ); diff --git a/tests/integration/App.test.tsx b/tests/integration/App.test.tsx index 2524be0c..e3a26d1c 100644 --- a/tests/integration/App.test.tsx +++ b/tests/integration/App.test.tsx @@ -2,6 +2,7 @@ import { Suspense, type ComponentType } from "react"; import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import { render, screen, waitFor, fireEvent } from "@testing-library/react"; import { describe, it, expect, beforeEach, vi } from "vitest"; +import { providersApi } from "@/lib/api/providers"; import { resetProviderState, setCurrentProviderId, @@ -282,4 +283,52 @@ describe("App integration with MSW", () => { expect.stringContaining("Provider key is required for openclaw"), ); }); + + it("shows toast when duplicate cannot load live provider ids", async () => { + setProviders("openclaw", { + deepseek: { + id: "deepseek", + name: "DeepSeek", + settingsConfig: { + baseUrl: "https://api.deepseek.com", + apiKey: "test-key", + api: "openai-completions", + models: [], + }, + category: "custom", + sortIndex: 0, + createdAt: Date.now(), + }, + }); + setCurrentProviderId("openclaw", "deepseek"); + + const liveIdsSpy = vi + .spyOn(providersApi, "getOpenClawLiveProviderIds") + .mockRejectedValueOnce(new Error("broken config")); + + const { default: App } = await import("@/App"); + renderApp(App); + + fireEvent.click(screen.getByText("switch-openclaw")); + + await waitFor(() => + expect(screen.getByTestId("provider-list").textContent).toContain( + "deepseek", + ), + ); + + fireEvent.click(screen.getByText("duplicate")); + + await waitFor(() => { + expect(toastErrorMock).toHaveBeenCalledWith( + expect.stringContaining("读取配置中的供应商标识失败"), + ); + }); + + expect(screen.getByTestId("provider-list").textContent).not.toContain( + "deepseek-copy", + ); + + liveIdsSpy.mockRestore(); + }); });