fix(provider): distinguish legacy providers from db-only when tolerating live config errors

Change `ProviderMeta.live_config_managed` from `bool` to `Option<bool>`
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.
This commit is contained in:
YoVinchen
2026-03-30 22:38:32 +08:00
parent 98c8255dbb
commit fda88b2e42
4 changed files with 147 additions and 51 deletions
+3 -7
View File
@@ -284,13 +284,9 @@ pub struct ProviderMeta {
#[serde(rename = "promptCacheKey", skip_serializing_if = "Option::is_none")]
pub prompt_cache_key: Option<String>,
/// 累加模式应用中,该 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<bool>,
/// 供应商类型标识(用于特殊供应商检测)
/// - "github_copilot": GitHub Copilot 供应商
#[serde(rename = "providerType", skip_serializing_if = "Option::is_none")]
+70 -34
View File
@@ -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<bool>,
) -> Result<bool, AppError> {
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<bool> {
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)?;
}
+25 -10
View File
@@ -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]),
);
+49
View File
@@ -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();
});
});