diff --git a/src-tauri/src/mcode_config.rs b/src-tauri/src/mcode_config.rs index b3dcb3672..e9d4d3a03 100644 --- a/src-tauri/src/mcode_config.rs +++ b/src-tauri/src/mcode_config.rs @@ -103,6 +103,15 @@ pub(crate) fn get_providers() -> Result, AppError> { } } +/// Any `custom_provider` node under this key, including MCode's own account providers. +pub(crate) fn provider_key_exists(id: &str) -> Result { + let document = read(&config_path())?; + Ok(document + .get("custom_provider") + .and_then(serde_yaml::Value::as_mapping) + .is_some_and(|providers| providers.contains_key(serde_yaml::Value::from(id)))) +} + pub(crate) fn validate_provider(id: &str, config: &Value) -> Result<(), AppError> { if id.is_empty() || id @@ -162,6 +171,16 @@ impl Drop for ConfigLock { } fn update(path: &Path, id: &str, provider: Option) -> Result<(), AppError> { + write_entry(path, id, provider, false) +} + +/// `create_only` rejects a key that already exists, checked while holding MCode's lock. +fn write_entry( + path: &Path, + id: &str, + provider: Option, + create_only: bool, +) -> Result<(), AppError> { fs::create_dir_all(path.parent().expect("MCode configuration directory")) .map_err(|e| AppError::Message(format!("Cannot create MCode directory: {e}")))?; let path = if path.exists() { @@ -221,6 +240,11 @@ fn update(path: &Path, id: &str, provider: Option) -> Result<(), AppError let providers = entry .as_mapping_mut() .ok_or_else(|| AppError::Config("Invalid MCode custom_provider configuration".into()))?; + if create_only && providers.contains_key(serde_yaml::Value::from(id)) { + return Err(AppError::InvalidInput(format!( + "MCode provider key '{id}' already exists" + ))); + } if providers .get(serde_yaml::Value::from(id)) .and_then(|value| value.get("kind")) @@ -249,6 +273,11 @@ pub(crate) fn set_provider(id: &str, config: Value) -> Result<(), AppError> { validate_provider(id, &config)?; update(&config_path(), id, Some(config)) } +/// Like `set_provider`, but never replaces an existing node. +pub(crate) fn add_provider(id: &str, config: Value) -> Result<(), AppError> { + validate_provider(id, &config)?; + write_entry(&config_path(), id, Some(config), true) +} pub(crate) fn remove_provider(id: &str) -> Result<(), AppError> { if !config_path().exists() { return Ok(()); @@ -295,6 +324,22 @@ mod tests { ); } + #[test] + fn create_only_writes_never_replace_a_node_that_appeared_meanwhile() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("config.yaml"); + let text = "custom_provider:\n raced:\n name: Theirs\n"; + fs::write(&path, text).unwrap(); + assert!(write_entry(&path, "raced", Some(json!({"name":"Ours"})), true).is_err()); + assert_eq!(fs::read_to_string(&path).unwrap(), text); + assert!(!path.with_extension("yaml.lock").exists()); + write_entry(&path, "fresh", Some(json!({"name":"Ours"})), true).unwrap(); + assert_eq!( + read(&path).unwrap()["custom_provider"]["fresh"]["name"], + "Ours" + ); + } + #[test] fn native_provider_can_omit_api_format() { let mut provider = json!({"options":{"baseURL":"https://example.com","apiKey":"test-key"},"models":{"model":{}}}); diff --git a/src-tauri/src/services/provider/mod.rs b/src-tauri/src/services/provider/mod.rs index 8c4c93b4a..81ac418db 100644 --- a/src-tauri/src/services/provider/mod.rs +++ b/src-tauri/src/services/provider/mod.rs @@ -4814,13 +4814,18 @@ impl ProviderService { state: &AppState, provider: &Provider, write_live: bool, + create: bool, ) -> Result { let previous = state.db.get_provider_by_id(&provider.id, "mcode")?; state.db.save_provider("mcode", provider)?; if write_live { - if let Err(error) = - crate::mcode_config::set_provider(&provider.id, provider.settings_config.clone()) - { + let config = provider.settings_config.clone(); + let written = if create { + crate::mcode_config::add_provider(&provider.id, config) + } else { + crate::mcode_config::set_provider(&provider.id, config) + }; + if let Err(error) = written { match previous { Some(previous) => state.db.save_provider("mcode", &previous)?, None => state.db.delete_provider("mcode", &provider.id)?, @@ -4859,7 +4864,23 @@ impl ProviderService { } if app_type == AppType::Mcode { - return Self::save_mcode_provider(state, &provider, add_to_live); + // The key is user-chosen, and saving would overwrite an existing row or live node. + // Adds are serialized here; the live write repeats the check under MCode's file lock. + let _guard = futures::executor::block_on( + state.proxy_service.lock_switch_for_app(app_type.as_str()), + ); + if state + .db + .get_provider_by_id(&provider.id, "mcode")? + .is_some() + || crate::mcode_config::provider_key_exists(&provider.id)? + { + return Err(AppError::InvalidInput(format!( + "MCode provider key '{}' already exists", + provider.id + ))); + } + return Self::save_mcode_provider(state, &provider, add_to_live, true); } // 还没有当前供应商时这一家会被写进 live：和进入代理、切换互斥（同 `update`）。 @@ -5494,7 +5515,7 @@ impl ProviderService { Self::set_provider_live_config_managed(&mut provider, live_config_managed); if app_type == AppType::Mcode { - return Self::save_mcode_provider(state, &provider, live_config_managed); + return Self::save_mcode_provider(state, &provider, live_config_managed, false); } // Save to database after live-config presence is resolved so parse errors diff --git a/src-tauri/tests/mcode_commands.rs b/src-tauri/tests/mcode_commands.rs index 06ef5fe32..957ff52b1 100644 --- a/src-tauri/tests/mcode_commands.rs +++ b/src-tauri/tests/mcode_commands.rs @@ -1,4 +1,4 @@ -use cc_switch_lib::{AppType, Prompt, PromptService}; +use cc_switch_lib::{AppType, Prompt, PromptService, Provider, ProviderService}; use serde_json::json; use std::fs; @@ -41,3 +41,89 @@ fn failed_mcode_prompt_writes_preserve_state_and_can_be_retried() { assert_eq!(fs::read_to_string(&path).unwrap(), "updated"); assert!(state.db.get_prompts("mcode").unwrap()[&active.id].enabled); } + +#[test] +fn adding_an_mcode_provider_rejects_a_key_already_in_use() { + let _guard = test_mutex().lock().unwrap(); + reset_test_fs(); + let state = create_test_state().unwrap(); + let path = ensure_test_home().join(".minimax/config.yaml"); + fs::create_dir_all(path.parent().unwrap()).unwrap(); + let native = "custom_provider:\n handwritten:\n name: Mine\n models: {}\n minimax:\n kind: minimax\n name: Account\n"; + fs::write(&path, native).unwrap(); + let provider = |id: &str| { + Provider::with_id( + id.into(), + "Test".into(), + json!({"name":"Test", "kind":"custom", "enabled":true, "api":"openai-completions", + "options":{"baseURL":"https://api.example.com/v1","apiKey":"test-key"}, + "models":{"model":{"name":"Model"}}}), + None, + ) + }; + + // Live nodes, custom or owned by MCode, are never overwritten. + for id in ["handwritten", "minimax"] { + assert!(ProviderService::add(&state, AppType::Mcode, provider(id), true).is_err()); + } + assert_eq!(fs::read_to_string(&path).unwrap(), native); + assert!(state.db.get_all_providers("mcode").unwrap().is_empty()); + + // Neither is a row that only lives in the database. + ProviderService::add(&state, AppType::Mcode, provider("fresh"), false).unwrap(); + let mut renamed = provider("fresh"); + renamed.name = "Other".into(); + assert!(ProviderService::add(&state, AppType::Mcode, renamed, true).is_err()); + assert_eq!( + state + .db + .get_provider_by_id("fresh", "mcode") + .unwrap() + .unwrap() + .name, + "Test" + ); + assert_eq!(fs::read_to_string(&path).unwrap(), native); +} + +#[test] +fn concurrent_mcode_adds_with_one_key_leave_exactly_one_winner() { + let _guard = test_mutex().lock().unwrap(); + reset_test_fs(); + let state = create_test_state().unwrap(); + let successes = std::thread::scope(|scope| { + let handles: Vec<_> = (0..16) + .map(|index| { + let state = &state; + scope.spawn(move || { + let provider = Provider::with_id( + "same".into(), + format!("Racer {index}"), + json!({"name":"Racer", "kind":"custom", "enabled":true, + "api":"openai-completions", + "options":{"baseURL":"https://api.example.com/v1","apiKey":"test-key"}, + "models":{"model":{"name":"Model"}}}), + None, + ); + ProviderService::add(state, AppType::Mcode, provider, false) + .is_ok() + .then_some(index) + }) + }) + .collect(); + handles + .into_iter() + .filter_map(|handle| handle.join().unwrap()) + .collect::>() + }); + assert_eq!(successes.len(), 1); + assert_eq!( + state + .db + .get_provider_by_id("same", "mcode") + .unwrap() + .unwrap() + .name, + format!("Racer {}", successes[0]) + ); +} diff --git a/src/App.tsx b/src/App.tsx index 23b818a85..6e5d7e6c6 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -887,6 +887,13 @@ function App() { existingKeys, ); duplicatedProvider.addToLive = false; + } else if (activeApp === "mcode") { + // The MCode list already includes its live custom nodes; the backend + // rejects a key that MCode itself owns. + duplicatedProvider.providerKey = generateUniqueProviderCopyKey( + provider.id, + Object.keys(providers), + ); } if (provider.sortIndex !== undefined) { diff --git a/src/components/providers/forms/McodeProviderForm.tsx b/src/components/providers/forms/McodeProviderForm.tsx index 765afca3b..57aded38b 100644 --- a/src/components/providers/forms/McodeProviderForm.tsx +++ b/src/components/providers/forms/McodeProviderForm.tsx @@ -4,9 +4,11 @@ import { useForm } from "react-hook-form"; import { useTranslation } from "react-i18next"; import { Button } from "@/components/ui/button"; import { Form } from "@/components/ui/form"; +import { Input } from "@/components/ui/input"; import { Label } from "@/components/ui/label"; import JsonEditor from "@/components/JsonEditor"; import { useDarkMode } from "@/hooks/useDarkMode"; +import { useProvidersQuery } from "@/lib/query/queries"; import type { ProviderFormData } from "@/lib/schemas/provider"; import type { OpenCodeModel, OpenCodeProviderOptions } from "@/types"; import { mcodeProviderPresets } from "@/config/mcodeProviderPresets"; @@ -62,6 +64,9 @@ const configSchema = z }) .passthrough(); type McodeConfig = z.infer; +const KEY_PATTERN = /^[a-z0-9]+(-[a-z0-9]+)*$/; +const normalizeKey = (value: string) => + value.toLowerCase().replace(/[^a-z0-9-]/g, ""); export function McodeProviderForm({ providerId, @@ -95,6 +100,19 @@ export function McodeProviderForm({ ); const [busy, setBusy] = useState(false); const [error, setError] = useState(""); + // The key becomes `custom_provider.` and MCode's default-model + // references embed it, so it is fixed once the provider exists. + const isEdit = Boolean(initialData); + const [providerKey, setProviderKey] = useState(providerId ?? ""); + const { data: existing } = useProvidersQuery("mcode"); + const keyTaken = + !isEdit && + Object.prototype.hasOwnProperty.call( + existing?.providers ?? {}, + providerKey, + ); + const keyInvalid = + !isEdit && providerKey !== "" && !KEY_PATTERN.test(providerKey); const form = useForm({ defaultValues: { name: initialData?.name ?? "", @@ -109,6 +127,7 @@ export function McodeProviderForm({ const ready = Boolean( jsonValid && name.trim() && + (isEdit || (providerKey && !keyInvalid && !keyTaken)) && config.options?.baseURL?.trim() && config.options?.apiKey?.trim() && Object.keys(config.models ?? {}).length, @@ -136,6 +155,7 @@ export function McodeProviderForm({ }; update(next); setExtraOptions(toOpencodeExtraOptions(next.options)); + setProviderKey(selected?.providerKey ?? ""); form.reset({ name: selected?.name ?? "", notes: "", @@ -166,7 +186,7 @@ export function McodeProviderForm({ ...identity, name: identity.name.trim(), meta: initialData?.meta, - providerKey: providerId, + providerKey: isEdit ? providerId : providerKey, presetCategory: category, settingsConfig: JSON.stringify({ ...config, @@ -206,7 +226,45 @@ export function McodeProviderForm({ disabled={busy || !jsonValid} className="min-w-0 space-y-6 border-0 p-0 disabled:opacity-50" > - + + + + setProviderKey(normalizeKey(event.target.value)) + } + disabled={isEdit} + placeholder={t("opencode.providerKeyPlaceholder")} + autoComplete="off" + className={keyTaken || keyInvalid ? "border-destructive" : ""} + /> +

+ {keyTaken + ? t("opencode.providerKeyDuplicate") + : keyInvalid + ? t("opencode.providerKeyInvalid") + : isEdit + ? t("opencode.providerKeyLockedHint") + : t("opencode.providerKeyHint")} +

+ + } + /> { appId === "opencode" || appId === "openclaw" || appId === "hermes" || - appId === "pi" + appId === "pi" || + appId === "mcode" ) { if ( providerInput.category === "omo" || diff --git a/tests/components/McodeProviderForm.test.tsx b/tests/components/McodeProviderForm.test.tsx index 8c4cb7f22..ca2b1d7bf 100644 --- a/tests/components/McodeProviderForm.test.tsx +++ b/tests/components/McodeProviderForm.test.tsx @@ -1,8 +1,17 @@ -import { fireEvent, render, screen, waitFor } from "@testing-library/react"; +import type { ReactElement } from "react"; +import { QueryClientProvider } from "@tanstack/react-query"; +import { + fireEvent, + render as rtlRender, + screen, + waitFor, +} from "@testing-library/react"; import { describe, expect, it, vi } from "vitest"; import { McodeProviderForm } from "@/components/providers/forms/McodeProviderForm"; import { mcodeProviderPresets } from "@/config/mcodeProviderPresets"; import { piProviderPresets } from "@/config/piProviderPresets"; +import { setProviders } from "../msw/state"; +import { createTestQueryClient } from "../utils/testQueryClient"; vi.mock("@/components/JsonEditor", () => ({ default: ({ @@ -22,6 +31,13 @@ vi.mock("@/components/JsonEditor", () => ({ ), })); +const render = (ui: ReactElement) => + rtlRender( + + {ui} + , + ); + const original = { kind: "custom", api: "anthropic-messages", @@ -113,6 +129,66 @@ describe("McodeProviderForm", () => { expect(saved.options.apiKey).toBe("test-key"); expect(saved).not.toHaveProperty("npm"); expect(saved.models).toHaveProperty("MiniMax-M3"); + expect(submit.mock.calls[0][0].providerKey).toBe("cc-switch-mini-max"); + }); + + it("requires a well-formed, unused provider key for new providers", async () => { + setProviders("mcode", { + taken: { id: "taken", name: "Taken", settingsConfig: original }, + }); + const submit = vi.fn(); + render( + {}} + />, + ); + fireEvent.click(screen.getByRole("button", { name: /Minimax MiniMax$/ })); + fireEvent.change(screen.getByLabelText("API Key"), { + target: { value: "test-key" }, + }); + const key = screen.getByLabelText(/opencode\.providerKey/); + const save = screen.getByRole("button", { name: "Save" }); + expect(key).toHaveValue("cc-switch-mini-max"); + expect(save).toBeEnabled(); + + fireEvent.change(key, { target: { value: "" } }); + expect(save).toBeDisabled(); + fireEvent.change(key, { target: { value: "-bad-" } }); + expect(screen.getByText("opencode.providerKeyInvalid")).toBeInTheDocument(); + expect(save).toBeDisabled(); + fireEvent.change(key, { target: { value: "taken" } }); + expect( + await screen.findByText("opencode.providerKeyDuplicate"), + ).toBeInTheDocument(); + expect(save).toBeDisabled(); + + fireEvent.change(key, { target: { value: "My Key!" } }); + expect(key).toHaveValue("mykey"); + fireEvent.click(save); + await waitFor(() => expect(submit).toHaveBeenCalledOnce()); + expect(submit.mock.calls[0][0].providerKey).toBe("mykey"); + }); + + it("locks the provider key when editing", () => { + render( + {}} + />, + ); + const key = screen.getByLabelText(/opencode\.providerKey/); + expect(key).toHaveValue("existing"); + expect(key).toBeDisabled(); + expect( + screen.getByText("opencode.providerKeyLockedHint"), + ).toBeInTheDocument(); }); it("keeps invalid JSON drafts from breaking the structured fields", () => { diff --git a/tests/integration/App.test.tsx b/tests/integration/App.test.tsx index b111b41d1..7f24ad967 100644 --- a/tests/integration/App.test.tsx +++ b/tests/integration/App.test.tsx @@ -390,6 +390,42 @@ describe("App integration with MSW", () => { ); }); + it("duplicates MiniMax Code providers under a generated unused key", async () => { + localStorage.setItem("cc-switch-last-app", "mcode"); + const provider = (id: string, name: string) => ({ + id, + name, + settingsConfig: {}, + category: "custom" as const, + sortIndex: 0, + createdAt: Date.now(), + }); + setProviders("mcode", { + kimi: provider("kimi", "Kimi"), + "kimi-copy": provider("kimi-copy", "Kimi copy"), + }); + setCurrentProviderId("mcode", "kimi"); + + const { default: App } = await import("@/App"); + renderApp(App); + + await waitFor(() => + expect(screen.getByTestId("provider-list").textContent).toContain( + "kimi-copy", + ), + ); + fireEvent.click(screen.getByText("duplicate")); + + await waitFor(() => + expect(screen.getByTestId("provider-list").textContent).toContain( + "kimi-copy-2", + ), + ); + expect(toastErrorMock).not.toHaveBeenCalledWith( + expect.stringContaining("Provider key is required for mcode"), + ); + }); + it("refreshes MiniMax Code provider membership after removing it from live config", async () => { localStorage.setItem("cc-switch-last-app", "mcode"); let liveConfigManaged = true;