diff --git a/src-tauri/src/commands/openai_voice_credentials.rs b/src-tauri/src/commands/openai_voice_credentials.rs index 926d46326..725cc69e6 100644 --- a/src-tauri/src/commands/openai_voice_credentials.rs +++ b/src-tauri/src/commands/openai_voice_credentials.rs @@ -2,7 +2,6 @@ const KEYCHAIN_SERVICE: &str = "berd-openai-voice"; const KEYCHAIN_ACCOUNT: &str = "api-key"; -const LEGACY_TTS_KEYCHAIN_ACCOUNT: &str = "tts-api-key"; #[derive(Clone, Copy)] pub(crate) enum OpenAiVoiceCredential { @@ -59,45 +58,19 @@ fn clear_account(account: &str) -> Result<(), String> { } } -fn canonical_mutation_with_legacy_cleanup( - canonical_mutation: impl FnOnce() -> Result, - legacy_cleanup: impl FnOnce() -> Result<(), String>, -) -> Result { - let value = canonical_mutation()?; - if let Err(error) = legacy_cleanup() { - log::warn!("Could not remove Berd's legacy OpenAI voice credential: {error}"); - } - Ok(value) -} - pub(crate) fn read(credential: OpenAiVoiceCredential) -> Result, String> { - if let Some(api_key) = read_account(credential.account())? { - return Ok(Some(api_key)); - } - let Some(api_key) = read_account(LEGACY_TTS_KEYCHAIN_ACCOUNT)? else { - return Ok(None); - }; - store(credential, &api_key)?; - Ok(Some(api_key)) + read_account(credential.account()) } pub(crate) fn store(credential: OpenAiVoiceCredential, api_key: &str) -> Result<(), String> { let entry = entry(credential.account())?; - canonical_mutation_with_legacy_cleanup( - || { - entry - .set_password(api_key) - .map_err(|error| format!("Could not save Berd's OpenAI voice credential: {error}")) - }, - || clear_account(LEGACY_TTS_KEYCHAIN_ACCOUNT), - ) + entry + .set_password(api_key) + .map_err(|error| format!("Could not save Berd's OpenAI voice credential: {error}")) } pub(crate) fn clear(credential: OpenAiVoiceCredential) -> Result<(), String> { - canonical_mutation_with_legacy_cleanup( - || clear_account(credential.account()), - || clear_account(LEGACY_TTS_KEYCHAIN_ACCOUNT), - ) + clear_account(credential.account()) } pub(crate) fn require(credential: OpenAiVoiceCredential) -> Result { @@ -107,36 +80,10 @@ pub(crate) fn require(credential: OpenAiVoiceCredential) -> Result { - return invoke("get_openai_voice_status"); -} +export const getOpenAiVoiceStatus = shareInFlight( + (): Promise => invoke("get_openai_voice_status"), +); export function setOpenAiTtsApiKey(apiKey: string): Promise { return invoke("set_openai_tts_api_key", { apiKey }); diff --git a/src/features/voice-conversation/hooks/useOpenAiVoiceSetup.test.tsx b/src/features/voice-conversation/hooks/useOpenAiVoiceSetup.test.tsx index d6a4c4ad8..cbf771b4b 100644 --- a/src/features/voice-conversation/hooks/useOpenAiVoiceSetup.test.tsx +++ b/src/features/voice-conversation/hooks/useOpenAiVoiceSetup.test.tsx @@ -4,15 +4,17 @@ import type { OpenAiVoiceStatus } from "../api/openAiVoice"; import { useOpenAiVoiceSetup } from "./useOpenAiVoiceSetup"; const mocks = vi.hoisted(() => ({ - getStatus: vi.fn<() => Promise>(), - settingsChanged: null as (() => void) | null, + getStatus: + vi.fn<(options?: { coalesce?: boolean }) => Promise>(), + settingsChanged: null as ((event?: unknown) => void) | null, finishListening: null as (() => void) | null, listenerError: null as Error | null, })); vi.mock("../api/openAiVoice", () => ({ - getOpenAiVoiceStatus: () => mocks.getStatus(), - listenToOpenAiVoiceSettings: (listener: () => void) => { + getOpenAiVoiceStatus: (options?: { coalesce?: boolean }) => + mocks.getStatus(options), + listenToOpenAiVoiceSettings: (listener: (event?: unknown) => void) => { mocks.settingsChanged = listener; if (mocks.listenerError) return Promise.reject(mocks.listenerError); return new Promise<() => void>((resolve) => { @@ -66,8 +68,16 @@ describe("useOpenAiVoiceSetup", () => { await waitFor(() => expect(mocks.settingsChanged).not.toBeNull()); act(() => mocks.finishListening?.()); await waitFor(() => expect(mocks.getStatus).toHaveBeenCalledTimes(1)); - - act(() => mocks.settingsChanged?.()); + expect(mocks.getStatus).toHaveBeenLastCalledWith({ coalesce: true }); + + act(() => + mocks.settingsChanged?.({ + event: "openai-voice:settings-changed", + id: 1, + payload: null, + }), + ); + expect(mocks.getStatus).toHaveBeenLastCalledWith(undefined); refreshed.resolve(status(true)); await waitFor(() => expect(result.current.status?.sttConfigured).toBe(true), @@ -123,7 +133,7 @@ describe("useOpenAiVoiceSetup", () => { expect(result.current.error).toBe("Keychain unavailable"); }); - it("does not expose cached readiness while disabled", async () => { + it("does not reuse cached readiness after being disabled", async () => { mocks.listenerError = new Error("listener unavailable"); mocks.getStatus.mockResolvedValue(status(true)); const { result, rerender } = renderHook( @@ -137,5 +147,16 @@ describe("useOpenAiVoiceSetup", () => { rerender({ enabled: false }); expect(result.current).toEqual({ status: null, error: null }); + + const reloaded = deferred(); + mocks.getStatus.mockReturnValueOnce(reloaded.promise); + rerender({ enabled: true }); + + expect(result.current).toEqual({ status: null, error: null }); + + reloaded.resolve(status(false)); + await waitFor(() => + expect(result.current.status?.ttsConfigured).toBe(false), + ); }); }); diff --git a/src/features/voice-conversation/hooks/useOpenAiVoiceSetup.ts b/src/features/voice-conversation/hooks/useOpenAiVoiceSetup.ts index f53d75090..a67742844 100644 --- a/src/features/voice-conversation/hooks/useOpenAiVoiceSetup.ts +++ b/src/features/voice-conversation/hooks/useOpenAiVoiceSetup.ts @@ -10,13 +10,17 @@ export function useOpenAiVoiceSetup(enabled = true) { const [error, setError] = useState(null); useEffect(() => { - if (!enabled) return; + if (!enabled) { + setStatus(null); + setError(null); + return; + } let active = true; let refreshGeneration = 0; let unsubscribe: (() => void) | null = null; - const refresh = () => { + const refresh = (coalesce = false) => { const generation = ++refreshGeneration; - void getOpenAiVoiceStatus().then( + void getOpenAiVoiceStatus(coalesce ? { coalesce: true } : undefined).then( (next) => { if (active && generation === refreshGeneration) { setStatus(next); @@ -31,11 +35,11 @@ export function useOpenAiVoiceSetup(enabled = true) { }, ); }; - void listenToOpenAiVoiceSettings(refresh).then( + void listenToOpenAiVoiceSettings(() => refresh()).then( (nextUnsubscribe) => { if (active) { unsubscribe = nextUnsubscribe; - refresh(); + refresh(true); } else nextUnsubscribe(); }, () => { diff --git a/src/features/voice-conversation/ui/VoiceSettings.test.tsx b/src/features/voice-conversation/ui/VoiceSettings.test.tsx index 01baa584d..3158d309c 100644 --- a/src/features/voice-conversation/ui/VoiceSettings.test.tsx +++ b/src/features/voice-conversation/ui/VoiceSettings.test.tsx @@ -11,6 +11,20 @@ import type { VoiceInputBackend } from "../lib/voiceInputPreference"; import type { VoiceOutputBackend } from "../lib/voiceOutputPreference"; import { VoiceSettings } from "./VoiceSettings"; +if (!HTMLElement.prototype.hasPointerCapture) { + HTMLElement.prototype.hasPointerCapture = () => false; +} + +if (!HTMLElement.prototype.scrollIntoView) { + HTMLElement.prototype.scrollIntoView = () => {}; +} + +const platformState = vi.hoisted(() => ({ current: "mac" })); + +vi.mock("@/shared/lib/platform", () => ({ + getPlatform: () => platformState.current, +})); + const setupState = vi.hoisted(() => ({ current: null as PocketVoiceSetup | null, })); @@ -51,6 +65,8 @@ const microphonePermissionState = vi.hoisted(() => ({ openSettings: vi.fn(), })); const openAiStatusState = vi.hoisted(() => ({ + enabled: null as boolean | null, + loaded: true, current: { sttConfigured: true, ttsConfigured: true, @@ -81,10 +97,14 @@ vi.mock("../api/openAiVoice", () => ({ clearOpenAiTtsApiKey: openAiApiMocks.clearTtsApiKey, })); vi.mock("../hooks/useOpenAiVoiceSetup", () => ({ - useOpenAiVoiceSetup: () => ({ - status: openAiStatusState.current, - error: null, - }), + useOpenAiVoiceSetup: (enabled: boolean) => { + openAiStatusState.enabled = enabled; + return { + status: + enabled && openAiStatusState.loaded ? openAiStatusState.current : null, + error: null, + }; + }, })); vi.mock("../hooks/usePocketVoiceSetup", () => ({ usePocketVoiceSetup: () => setupState.current, @@ -207,6 +227,8 @@ describe("VoiceSettings", () => { microphonePermissionState.openSettings.mockReset(); inputState.backend = "parakeet"; outputState.backend = "pocket"; + platformState.current = "mac"; + openAiStatusState.loaded = true; macSpeechSetupState.current = { status: { supported: false, @@ -246,6 +268,68 @@ describe("VoiceSettings", () => { openAiApiMocks.clearSttApiKey.mockClear(); }); + it("does not inspect OpenAI credentials for Apple speech input and output", () => { + inputState.backend = "macos"; + outputState.backend = "siri"; + + renderWithProviders(); + + expect(openAiStatusState.enabled).toBe(false); + }); + + it("keeps OpenAI voice playback selectable without inspecting credentials", async () => { + setupState.current = setup(pocketStatus()); + renderWithProviders(); + + expect(openAiStatusState.enabled).toBe(false); + + const user = userEvent.setup(); + await user.click(screen.getByRole("combobox", { name: "Speech output" })); + + expect( + screen.getByRole("option", { name: "OpenAI text-to-speech" }), + ).toBeInTheDocument(); + }); + + it("does not offer OpenAI voice playback on unsupported platforms", async () => { + platformState.current = "linux"; + setupState.current = setup(pocketStatus()); + renderWithProviders(); + + const user = userEvent.setup(); + await user.click(screen.getByRole("combobox", { name: "Speech output" })); + + expect( + screen.queryByRole("option", { name: "OpenAI text-to-speech" }), + ).not.toBeInTheDocument(); + }); + + it("waits for OpenAI credential status before showing readiness guidance", () => { + outputState.backend = "openai"; + openAiStatusState.loaded = false; + setupState.current = setup(pocketStatus({ parakeetInstalled: true })); + + renderWithProviders(); + + expect(screen.queryByRole("alert")).not.toBeInTheDocument(); + expect( + screen.getByText("Checking OpenAI voice settings…"), + ).toBeInTheDocument(); + }); + + it.each([ + ["openai", "siri"], + ["macos", "openai"], + ["openai", "openai"], + ] as const)("inspects OpenAI credentials for %s speech input and %s speech output", (inputBackend, outputBackend) => { + inputState.backend = inputBackend; + outputState.backend = outputBackend; + + renderWithProviders(); + + expect(openAiStatusState.enabled).toBe(true); + }); + it("renders independently selected OpenAI input and output settings", async () => { inputState.backend = "openai"; outputState.backend = "openai"; diff --git a/src/features/voice-conversation/ui/VoiceSettings.tsx b/src/features/voice-conversation/ui/VoiceSettings.tsx index 8f2aef7b0..55bb7e15b 100644 --- a/src/features/voice-conversation/ui/VoiceSettings.tsx +++ b/src/features/voice-conversation/ui/VoiceSettings.tsx @@ -95,16 +95,18 @@ export function VoiceSettings() { const { t } = useTranslation("settings"); const setup = usePocketVoiceSetup(); const macSpeechSetup = useMacSpeechSetup(); - const { status: openAiStatus, error: openAiError } = useOpenAiVoiceSetup(); const [openAiSpeed, setOpenAiSpeed] = useState(1); const [openAiSpeedError, setOpenAiSpeedError] = useState(null); - useEffect(() => { - if (openAiStatus) setOpenAiSpeed(openAiStatus.playbackSpeed); - }, [openAiStatus]); const input = useVoiceInputPreference( isMacSpeechAvailable(macSpeechSetup.status, macSpeechSetup.loading), ); const output = useVoiceOutputPreference(); + const { status: openAiStatus, error: openAiError } = useOpenAiVoiceSetup( + input.backend === "openai" || output.backend === "openai", + ); + useEffect(() => { + if (openAiStatus) setOpenAiSpeed(openAiStatus.playbackSpeed); + }, [openAiStatus]); const interruption = useVoiceInterruptionPreference(); const mode = useVoiceConversationModePreference(); const siriSetup = useSiriVoiceSetup(output.backend === "siri"); @@ -141,22 +143,27 @@ export function VoiceSettings() { const pocketStatusLoaded = (input.backend !== "parakeet" && output.backend !== "pocket") || setup.status !== null; - const readinessKey = !pocketStatusLoaded - ? null - : !inputReady && output.backend === "siri" && !siriOutputLoaded - ? input.backend === "macos" - ? "voice.notReadyMacInput" - : "voice.notReadyInput" - : output.backend === "siri" && !siriOutputLoaded - ? null - : input.backend === null + const openAiStatusLoaded = + (input.backend !== "openai" && output.backend !== "openai") || + openAiStatus !== null || + openAiError !== null; + const readinessKey = + !pocketStatusLoaded || !openAiStatusLoaded + ? null + : !inputReady && output.backend === "siri" && !siriOutputLoaded + ? input.backend === "macos" + ? "voice.notReadyMacInput" + : "voice.notReadyInput" + : output.backend === "siri" && !siriOutputLoaded ? null - : readinessDescriptionKey( - inputReady, - outputReady, - output.backend, - input.backend, - ); + : input.backend === null + ? null + : readinessDescriptionKey( + inputReady, + outputReady, + output.backend, + input.backend, + ); return ( {t("voice.backendPocket")} - {openAiStatus?.ttsAvailable ? ( + {getPlatform() === "mac" ? ( {t("voice.backendOpenAiTts")}