diff --git a/apps/desktop-tauri/src-tauri/src/commands/credentials.rs b/apps/desktop-tauri/src-tauri/src/commands/credentials.rs index cd4b9e916e..44b3dda116 100644 --- a/apps/desktop-tauri/src-tauri/src/commands/credentials.rs +++ b/apps/desktop-tauri/src-tauri/src/commands/credentials.rs @@ -79,11 +79,25 @@ pub fn get_api_key_providers() -> Vec { #[tauri::command] pub fn set_api_key( + app: tauri::AppHandle, provider_id: String, api_key: String, label: Option, ) -> Result, String> { - let canonical_provider = canonical_provider_arg(&provider_id)?; + let canonical_provider = write_api_key(&provider_id, &api_key, label)?; + events::emit_api_keys_updated(&app, &canonical_provider); + Ok(get_api_keys()) +} + +/// Validates and persists one provider's API key, returning the canonical +/// provider id. Emission is left to the command so this stays testable without +/// an `AppHandle`. +pub(super) fn write_api_key( + provider_id: &str, + api_key: &str, + label: Option, +) -> Result { + let canonical_provider = canonical_provider_arg(provider_id)?; if !codexbar::settings::get_api_key_providers() .iter() .any(|p| p.id.cli_name() == canonical_provider) @@ -92,22 +106,32 @@ pub fn set_api_key( "Provider '{canonical_provider}' does not support API-key storage" )); } - validate_single_line_secret(&api_key, "API key", MAX_API_KEY_LEN)?; + validate_single_line_secret(api_key, "API key", MAX_API_KEY_LEN)?; let label = sanitize_optional_label(label)?; let mut keys = ApiKeys::load(); keys.set(&canonical_provider, api_key.trim(), label.as_deref()); keys.save().map_err(|e| e.to_string())?; - Ok(get_api_keys()) + Ok(canonical_provider) } #[tauri::command] -pub fn remove_api_key(provider_id: String) -> Result, String> { - let canonical_provider = canonical_provider_arg(&provider_id)?; +pub fn remove_api_key( + app: tauri::AppHandle, + provider_id: String, +) -> Result, String> { + let canonical_provider = remove_stored_api_key(&provider_id)?; + events::emit_api_keys_updated(&app, &canonical_provider); + Ok(get_api_keys()) +} + +/// Removes one provider's stored API key, returning the canonical provider id. +pub(super) fn remove_stored_api_key(provider_id: &str) -> Result { + let canonical_provider = canonical_provider_arg(provider_id)?; let mut keys = ApiKeys::load(); keys.remove(&canonical_provider); keys.save().map_err(|e| e.to_string())?; - Ok(get_api_keys()) + Ok(canonical_provider) } #[tauri::command] diff --git a/apps/desktop-tauri/src-tauri/src/commands/tests.rs b/apps/desktop-tauri/src-tauri/src/commands/tests.rs index 3468714a0e..38940e2d86 100644 --- a/apps/desktop-tauri/src-tauri/src/commands/tests.rs +++ b/apps/desktop-tauri/src-tauri/src/commands/tests.rs @@ -152,15 +152,15 @@ fn credential_storage_status_serializes_access_tokens() { #[test] fn command_inputs_reject_invalid_provider_ids_before_storage_writes() { - assert!(super::set_api_key("not-a-provider".into(), "sk-test".into(), None).is_err()); + assert!(super::credentials::write_api_key("not-a-provider", "sk-test", None).is_err()); assert!(super::set_manual_cookie("not-a-provider".into(), "a=b".into()).is_err()); - assert!(super::remove_api_key("bad\nprovider".into()).is_err()); + assert!(super::credentials::remove_stored_api_key("bad\nprovider").is_err()); assert!(super::remove_manual_cookie("".into()).is_err()); } #[test] fn command_inputs_reject_multiline_secrets() { - assert!(super::set_api_key("openrouter".into(), "sk-test\nnext".into(), None).is_err()); + assert!(super::credentials::write_api_key("openrouter", "sk-test\nnext", None).is_err()); assert!(super::set_manual_cookie("codex".into(), "a=b\nc=d".into()).is_err()); } diff --git a/apps/desktop-tauri/src-tauri/src/events.rs b/apps/desktop-tauri/src-tauri/src/events.rs index b9d36cc4d1..1d2a5291cc 100644 --- a/apps/desktop-tauri/src-tauri/src/events.rs +++ b/apps/desktop-tauri/src-tauri/src/events.rs @@ -22,6 +22,7 @@ pub const LOCALE_CHANGED: &str = "locale-changed"; pub const SETTINGS_CHANGED: &str = "settings-changed"; pub const STAY_AWAKE_CHANGED: &str = "stay-awake-changed"; pub const CODEX_ACCOUNTS_UPDATED: &str = "codex-accounts-updated"; +pub const API_KEYS_UPDATED: &str = "api-keys-updated"; pub const LOGIN_PHASE: &str = "login-phase"; pub const PROVIDER_STATUSES_UPDATED: &str = "provider-statuses-updated"; @@ -48,6 +49,13 @@ pub struct RefreshStartedPayload { pub provider_ids: Vec, } +/// Names only the provider whose stored API key changed. Never carries the key. +#[derive(Debug, Clone, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct ApiKeysUpdatedPayload { + pub provider_id: String, +} + #[derive(Debug, Clone, Serialize)] #[serde(rename_all = "camelCase")] pub struct LoginPhasePayload { @@ -102,6 +110,17 @@ pub fn emit_codex_accounts_updated(app: &AppHandle) { let _emit_codex_accounts = app.emit(CODEX_ACCOUNTS_UPDATED, ()); } +/// Broadcast that a provider's stored API key was written or removed, so the +/// Settings API key section re-reads the key list in every window. +pub fn emit_api_keys_updated(app: &AppHandle, provider_id: &str) { + let _emit_api_keys = app.emit( + API_KEYS_UPDATED, + ApiKeysUpdatedPayload { + provider_id: provider_id.to_string(), + }, + ); +} + pub fn emit_update_state_changed(app: &AppHandle, payload: &UpdateStatePayload) { let _emit_update_state = app.emit(UPDATE_STATE_CHANGED, payload); } @@ -128,3 +147,18 @@ pub fn emit_login_phase(app: &AppHandle, provider_id: &str, phase: &str, auth_li }, ); } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn api_keys_updated_payload_names_the_provider_and_never_a_key() { + let json = serde_json::to_value(ApiKeysUpdatedPayload { + provider_id: "openrouter".into(), + }) + .expect("serialize"); + assert_eq!(json, serde_json::json!({ "providerId": "openrouter" })); + assert_eq!(API_KEYS_UPDATED, "api-keys-updated"); + } +} diff --git a/apps/desktop-tauri/src/surfaces/settings/providers/ApiKeySection.test.tsx b/apps/desktop-tauri/src/surfaces/settings/providers/ApiKeySection.test.tsx new file mode 100644 index 0000000000..22b848cb07 --- /dev/null +++ b/apps/desktop-tauri/src/surfaces/settings/providers/ApiKeySection.test.tsx @@ -0,0 +1,135 @@ +import { act, fireEvent, render, screen, waitFor } from "@testing-library/react"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import type { ApiKeyInfoBridge } from "../../../types/bridge"; + +const mocks = vi.hoisted(() => ({ + getApiKeyProviders: vi.fn(), + getApiKeys: vi.fn(), + openProviderDashboard: vi.fn(), + removeApiKey: vi.fn(), + setApiKey: vi.fn(), +})); +const events = vi.hoisted(() => ({ + listen: vi.fn<(event: string, listener: (event: { payload: unknown }) => void) => Promise<() => void>>(), +})); + +vi.mock("../../../lib/tauri", () => mocks); +vi.mock("@tauri-apps/api/event", () => events); +vi.mock("../../../hooks/useLocale", () => ({ + useLocale: () => ({ t: (key: string) => key }), +})); + +import { ApiKeySection } from "./ApiKeySection"; + +const openrouter = { + id: "openrouter", + displayName: "OpenRouter", + envVar: null, + help: null, + dashboardUrl: null, +}; +const savedKey: ApiKeyInfoBridge = { + providerId: "openrouter", + provider: "OpenRouter", + maskedKey: "sk-or-...t9Qz", + savedAt: "2026-05-01 10:00", + label: null, +}; + +function listenerFor(event: string): (event: { payload: unknown }) => void { + const call = events.listen.mock.calls.find(([name]) => name === event); + if (!call) throw new Error(`no listener registered for ${event}`); + return call[1]; +} + +describe("ApiKeySection external changes", () => { + beforeEach(() => { + vi.resetAllMocks(); + mocks.getApiKeyProviders.mockResolvedValue([openrouter]); + mocks.getApiKeys.mockResolvedValue([]); + events.listen.mockResolvedValue(() => {}); + }); + + it("re-reads the key list when the backend announces a change", async () => { + render(); + await screen.findByText("ApiKeyNotSet"); + + // A save from another window (or the CLI followed by a refresh) lands here. + mocks.getApiKeys.mockResolvedValue([savedKey]); + await act(async () => { + listenerFor("api-keys-updated")({ payload: { providerId: "openrouter" } }); + }); + + expect(await screen.findByText("KeySet")).toBeInTheDocument(); + expect(screen.getByText(savedKey.maskedKey)).toBeInTheDocument(); + }); + + it("re-reads the key list when the window regains focus", async () => { + render(); + await screen.findByText("ApiKeyNotSet"); + expect(mocks.getApiKeys).toHaveBeenCalledTimes(1); + + // A CLI write can't emit, so focus is the fallback signal. + mocks.getApiKeys.mockResolvedValue([savedKey]); + await act(async () => { + window.dispatchEvent(new Event("focus")); + }); + + expect(await screen.findByText("KeySet")).toBeInTheDocument(); + }); + + it("keeps an open edit form and its draft when a reload arrives", async () => { + render(); + await screen.findByText("ApiKeyNotSet"); + fireEvent.click(screen.getByRole("button", { name: "AddKey" })); + fireEvent.change(screen.getByPlaceholderText("PasteApiKeyHere"), { + target: { value: "sk-or-draft" }, + }); + + await act(async () => { + listenerFor("api-keys-updated")({ payload: { providerId: "openrouter" } }); + }); + + await waitFor(() => expect(mocks.getApiKeys).toHaveBeenCalledTimes(2)); + expect(screen.getByPlaceholderText("PasteApiKeyHere")).toHaveValue("sk-or-draft"); + }); + + it("keeps the newest reload when overlapping reloads finish out of order", async () => { + render(); + await screen.findByText("ApiKeyNotSet"); + + let resolveOlder: (keys: ApiKeyInfoBridge[]) => void = () => {}; + let resolveNewer: (keys: ApiKeyInfoBridge[]) => void = () => {}; + mocks.getApiKeys + .mockImplementationOnce(() => new Promise((resolve) => { resolveOlder = resolve; })) + .mockImplementationOnce(() => new Promise((resolve) => { resolveNewer = resolve; })); + + await act(async () => { + window.dispatchEvent(new Event("focus")); + listenerFor("api-keys-updated")({ payload: { providerId: "openrouter" } }); + }); + await act(async () => { + resolveNewer([savedKey]); + }); + expect(await screen.findByText("KeySet")).toBeInTheDocument(); + + await act(async () => { + resolveOlder([]); + }); + expect(screen.getByText("KeySet")).toBeInTheDocument(); + expect(screen.queryByText("ApiKeyNotSet")).not.toBeInTheDocument(); + }); + + it("stops listening and ignores focus after unmount", async () => { + const unlisten = vi.fn(); + events.listen.mockResolvedValue(unlisten); + const { unmount } = render(); + await screen.findByText("ApiKeyNotSet"); + unmount(); + + await waitFor(() => expect(unlisten).toHaveBeenCalledTimes(1)); + const callsBefore = mocks.getApiKeys.mock.calls.length; + window.dispatchEvent(new Event("focus")); + expect(mocks.getApiKeys.mock.calls.length).toBe(callsBefore); + }); +}); diff --git a/apps/desktop-tauri/src/surfaces/settings/providers/ApiKeySection.tsx b/apps/desktop-tauri/src/surfaces/settings/providers/ApiKeySection.tsx index 63923b03e8..8cb7fbdb56 100644 --- a/apps/desktop-tauri/src/surfaces/settings/providers/ApiKeySection.tsx +++ b/apps/desktop-tauri/src/surfaces/settings/providers/ApiKeySection.tsx @@ -1,4 +1,5 @@ -import { useCallback, useEffect, useState } from "react"; +import { useCallback, useEffect, useRef, useState } from "react"; +import { listen, type UnlistenFn } from "@tauri-apps/api/event"; import { getApiKeyProviders, getApiKeys, @@ -33,20 +34,27 @@ export function ApiKeySection({ providerId }: Props) { const [editValue, setEditValue] = useState(""); const [editLabel, setEditLabel] = useState(""); + // Shared by the mount and refresh effects: reloads can overlap and finish + // out of order, and only the newest one may write state. + const reloadGeneration = useRef(0); + const reload = useCallback(async (signal: { stale: boolean }) => { + const generation = ++reloadGeneration.current; + const isCurrent = () => + !signal.stale && generation === reloadGeneration.current; try { const [providers, keys] = await Promise.all([ getApiKeyProviders(), getApiKeys(), ]); - if (signal.stale) return; + if (!isCurrent()) return; setInfo(providers.find((p) => p.id === providerId) ?? null); setSaved(keys.find((k) => k.providerId === providerId) ?? null); } catch (err: unknown) { - if (signal.stale) return; + if (!isCurrent()) return; setError(err instanceof Error ? err.message : String(err)); } finally { - if (!signal.stale) setLoaded(true); + if (isCurrent()) setLoaded(true); } }, [providerId]); @@ -63,6 +71,36 @@ export function ApiKeySection({ providerId }: Props) { return () => { signal.stale = true; }; }, [reload]); + // Keys can change outside this section: another window saves through the + // bridge (`api-keys-updated`), or a CLI write lands in the keys file, which + // can't emit. Re-read on both events and on window focus. Unlike the mount + // effect above, this leaves an open edit form and its draft untouched. + useEffect(() => { + const signal = { stale: false }; + const refresh = () => void reload(signal); + const onVisibilityChange = () => { + if (document.visibilityState === "visible") refresh(); + }; + window.addEventListener("focus", refresh); + document.addEventListener("visibilitychange", onVisibilityChange); + let active = true; + let unlisten: UnlistenFn | undefined; + // `Promise.resolve` tolerates test mocks that return a bare unlisten fn. + Promise.resolve(listen("api-keys-updated", refresh)) + .then((fn) => { + if (active) unlisten = fn; + else fn?.(); + }) + .catch(() => {}); + return () => { + signal.stale = true; + active = false; + window.removeEventListener("focus", refresh); + document.removeEventListener("visibilitychange", onVisibilityChange); + unlisten?.(); + }; + }, [reload]); + if (!loaded) return null; // Provider doesn't support API keys — render nothing. // Distinguish from "failed to load" by checking error state.