diff --git a/CHANGELOG.md b/CHANGELOG.md index d300700..12c8140 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,13 @@ # Changelog +## v0.14.17 + +- Recording controls now identify the active session. Delayed stop or cancellation requests cannot affect a replacement recording, and duplicate capture-stop requests no longer cancel transcription. +- Existing CLI and shortcut commands retain compatible stop behavior, including dictionary-training captures. +- Settings, microphone selection, tray language changes, and setup activation share one ordered configuration path. Delayed responses no longer replace newer selections. +- Cancelling setup while activation is queued prevents it from changing the configured engine or model. +- Shared frontend test helpers and streamlined CI reduce maintenance overhead without removing application features. + ## v0.14.16 - Home no longer suggests an unsafe terminal command to remove old Echo copies. Use the existing **Remove old copies** action. diff --git a/Cargo.lock b/Cargo.lock index 864d5c8..66f36c5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1166,7 +1166,7 @@ checksum = "d0881ea181b1df73ff77ffaaf9c7544ecc11e82fba9b5f27b262a3c73a332555" [[package]] name = "echo" -version = "0.14.16" +version = "0.14.17" dependencies = [ "bzip2", "cpal", @@ -1190,7 +1190,7 @@ dependencies = [ [[package]] name = "echo-core" -version = "0.14.16" +version = "0.14.17" dependencies = [ "caseless", "fs2", @@ -1204,7 +1204,7 @@ dependencies = [ [[package]] name = "echo-desktop" -version = "0.14.16" +version = "0.14.17" dependencies = [ "ashpd", "clap", @@ -1223,7 +1223,7 @@ dependencies = [ [[package]] name = "echo-ipc" -version = "0.14.16" +version = "0.14.17" dependencies = [ "echo", "echo-core", @@ -1234,7 +1234,7 @@ dependencies = [ [[package]] name = "echo-ipc-gen" -version = "0.14.16" +version = "0.14.17" dependencies = [ "echo-ipc", ] @@ -6087,7 +6087,7 @@ checksum = "b9cc00251562a284751c9973bace760d86c0276c471b4be569fe6b068ee97a56" [[package]] name = "xtask" -version = "0.14.16" +version = "0.14.17" dependencies = [ "png 0.18.1", "resvg", diff --git a/Cargo.toml b/Cargo.toml index dc19c81..42a8dd0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -7,7 +7,7 @@ edition = "2021" license = "MIT" repository = "https://github.com/ddv1982/echo" rust-version = "1.89" -version = "0.14.16" +version = "0.14.17" [workspace.lints.rust] unsafe_code = "forbid" diff --git a/crates/echo-ipc/src/lib.rs b/crates/echo-ipc/src/lib.rs index 3c5744a..9122c98 100644 --- a/crates/echo-ipc/src/lib.rs +++ b/crates/echo-ipc/src/lib.rs @@ -300,11 +300,23 @@ pub enum SettingsChange { #[derive(Debug, Clone, PartialEq, Serialize, TS)] #[serde(rename_all = "camelCase")] pub struct SettingsSnapshot { + pub revision: u64, pub preferences: Settings, pub transcription: TranscriptionSnapshot, pub readiness: Readiness, } +#[derive(Debug, Clone, PartialEq, Serialize, TS)] +#[serde( + tag = "kind", + rename_all = "camelCase", + rename_all_fields = "camelCase" +)] +pub enum ChannelReply { + Ok { value: T }, + Err { error: String }, +} + #[derive(Debug, Clone, PartialEq, Serialize, TS)] #[serde(rename_all = "camelCase")] pub struct TranscriptionSnapshot { @@ -581,6 +593,7 @@ pub enum MicrophoneSource { #[derive(Debug, Clone, PartialEq, Eq, Serialize, TS)] #[serde(rename_all = "camelCase")] pub struct MicrophoneSnapshot { + pub revision: u64, pub host: AudioHost, pub source: MicrophoneSource, pub system_default: Option, @@ -821,6 +834,7 @@ macro_rules! schema_types { schema::AppPhase => schema::AppPhase, schema::AppStatus => schema::AppStatus, schema::AudioHost => schema::AudioHost, + schema::ChannelReply => schema::ChannelReply, schema::ComponentId => schema::ComponentId, schema::ComponentOrigin => schema::ComponentOrigin, schema::ComponentStatus => schema::ComponentStatus, diff --git a/crates/echo-ipc/src/projections.rs b/crates/echo-ipc/src/projections.rs index b26f803..1972560 100644 --- a/crates/echo-ipc/src/projections.rs +++ b/crates/echo-ipc/src/projections.rs @@ -260,6 +260,7 @@ impl From for MicrophoneSource { impl From for MicrophoneSnapshot { fn from(value: echo::microphone::MicrophoneSnapshot) -> Self { Self { + revision: 0, host: value.host.into(), source: value.source.into(), system_default: value.system_default.map(Into::into), diff --git a/docs/architecture.md b/docs/architecture.md index 6511b43..11e7cd8 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -60,11 +60,18 @@ flat control-file protocol. ## Desktop boundary -Tauri command functions are thin adapters. Settings owns serialized config -writes and invalidates the status cache after a successful save. Status owns -health caching and the `AppStatus` projection. Focused command modules own -devices, library data, recording, settings, shortcuts, status, and system -operations. +Tauri command functions are thin adapters. `ConfigMutationService` accepts +settings and microphone requests synchronously, then performs their work in +FIFO order on a worker. Typed Tauri channels deliver the results without +blocking the UI thread. Tray language changes and setup-plan configuration +changes use the same owner. Capture and transcription run independently. + +Every settings and microphone response carries a monotonic revision for that +desktop process. Views retain the newest snapshot, so a delayed response cannot +replace a newer selection. Successful saves invalidate the status cache. +Status owns health caching and the `AppStatus` projection. Focused command +modules own devices, library data, recording, settings, shortcuts, status, and +system operations. The Settings boundary returns one `SettingsSnapshot`. The snapshot keeps saved preferences, the resolved next transcription, setup readiness, and previous-run @@ -95,8 +102,11 @@ browser development graph and cannot enter the production bundle. `App.tsx` composes navigation and feature surfaces. Shared status, theme, history, dictionary, and error state live in the app controller. Home and -Settings own their subscriptions and device/setup lifetimes. Settings changes -are serialized, and stale setup refreshes cannot replace a newer snapshot. +Settings own their subscriptions and device/setup lifetimes. Backend responses +define settings order, and stale setup refreshes cannot replace a newer +snapshot. Test suites share the typed desktop API harness in +`frontend/src/test/desktopApiHarness.ts`, with scenario-specific overrides kept +in each test. Serial polling never overlaps requests and stops with component disposal. Subscriptions await their unlisten handle and dispose it even when unmount diff --git a/frontend/src/api/previewDesktopApi.ts b/frontend/src/api/previewDesktopApi.ts index b172bc7..8622740 100644 --- a/frontend/src/api/previewDesktopApi.ts +++ b/frontend/src/api/previewDesktopApi.ts @@ -62,6 +62,7 @@ export function createPreviewDesktopApi(): PreviewDesktopApi { let recordingSequence = 0 let previewSettings: Settings = defaultPreviewSettings() + let previewRevision = 0 let previewRecordingDeadline: number | null = null const previewTimers = new Set() @@ -470,6 +471,7 @@ export function createPreviewDesktopApi(): PreviewDesktopApi { previewRecordingDeadline = null previewSettings = defaultPreviewSettings() previewStatus = richPreviewStatus() + previewRevision = 0 previewDictionary = defaultPreviewDictionary() previewTrainingIndex = 0 activeTrainingCapture = null @@ -491,7 +493,7 @@ export function createPreviewDesktopApi(): PreviewDesktopApi { let previewMicrophones: MicrophoneSnapshot = defaultPreviewMicrophones(previewDevices) function getMicrophones(): Promise { - return Promise.resolve(ipcSnapshot(previewMicrophones)) + return Promise.resolve(ipcSnapshot(revisionedMicrophones(previewMicrophones))) } function setMicrophone(id: string | null): Promise { @@ -512,7 +514,7 @@ export function createPreviewDesktopApi(): PreviewDesktopApi { firstRunComplete: microphoneReady && previewReadiness.speechReady && previewReadiness.hasSuccessfulDictation, } - return Promise.resolve(ipcSnapshot(previewMicrophones)) + return Promise.resolve(ipcSnapshot(revisionedMicrophones(previewMicrophones))) } function previewMicrophoneTest(device: InputDevice | null): MicrophoneTestResult { @@ -657,6 +659,7 @@ export function createPreviewDesktopApi(): PreviewDesktopApi { function previewSettingsSnapshot(): SettingsSnapshot { const nextRun = previewNextRun() return { + revision: nextPreviewRevision(), preferences: previewSettings, transcription: { nextRun, @@ -669,6 +672,15 @@ export function createPreviewDesktopApi(): PreviewDesktopApi { } } + function revisionedMicrophones(snapshot: MicrophoneSnapshot): MicrophoneSnapshot { + return { ...snapshot, revision: nextPreviewRevision() } + } + + function nextPreviewRevision(): number { + previewRevision += 1 + return previewRevision + } + function previewNextRun(): SettingsSnapshot['transcription']['nextRun'] { if (previewSettings.engine.effective === 'fake') { return { kind: 'ready', engine: { kind: 'fake' }, language: previewSettings.language.effective } diff --git a/frontend/src/api/previewDesktopFixtures.ts b/frontend/src/api/previewDesktopFixtures.ts index a8cb63b..5b0ac3f 100644 --- a/frontend/src/api/previewDesktopFixtures.ts +++ b/frontend/src/api/previewDesktopFixtures.ts @@ -169,7 +169,7 @@ export function defaultPreviewSystemDefault(): InputDevice { export function defaultPreviewMicrophones(devices: InputDevice[]): MicrophoneSnapshot { const systemDefault = defaultPreviewSystemDefault() - return { host: 'pipe-wire', source: 'default', systemDefault, systemDefaultIsProxy: true, devices, selection: { kind: 'system-default', active: systemDefault }, enumerationWarning: null } + return { revision: 0, host: 'pipe-wire', source: 'default', systemDefault, systemDefaultIsProxy: true, devices, selection: { kind: 'system-default', active: systemDefault }, enumerationWarning: null } } export function defaultPreviewReadiness(): Readiness { diff --git a/frontend/src/api/tauriDesktopApi.test.ts b/frontend/src/api/tauriDesktopApi.test.ts index 8c41037..22bff38 100644 --- a/frontend/src/api/tauriDesktopApi.test.ts +++ b/frontend/src/api/tauriDesktopApi.test.ts @@ -2,20 +2,32 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' import type { SettingsChange, SetupEvent } from '../generated/ipc' import { createTauriDesktopApi, tauriDesktopApi } from './tauriDesktopApi' -const { invokeMock, listenMock } = vi.hoisted(() => ({ - invokeMock: vi.fn((command: string, commandArguments?: unknown) => { - void command - void commandArguments - return Promise.resolve() - }), - listenMock: vi.fn((event: string, handler: (event: { payload: SetupEvent }) => void) => { - void event - void handler - return Promise.resolve(() => undefined) - }), -})) - -vi.mock('@tauri-apps/api/core', () => ({ invoke: invokeMock })) +const { TestChannel, invokeMock, listenMock } = vi.hoisted(() => { + class HoistedTestChannel { + onmessage: (message: T) => void + + constructor(onmessage: (message: T) => void) { + this.onmessage = onmessage + } + } + return { + TestChannel: HoistedTestChannel, + invokeMock: vi.fn<(command: string, commandArguments?: unknown) => Promise>( + (command, commandArguments) => { + void command + void commandArguments + return Promise.resolve() + }, + ), + listenMock: vi.fn((event: string, handler: (event: { payload: SetupEvent }) => void) => { + void event + void handler + return Promise.resolve(() => undefined) + }), + } +}) + +vi.mock('@tauri-apps/api/core', () => ({ Channel: TestChannel, invoke: invokeMock })) vi.mock('@tauri-apps/api/event', () => ({ listen: listenMock })) function requireFixture(value: T | undefined, description: string): T { @@ -23,6 +35,30 @@ function requireFixture(value: T | undefined, description: string): T { return value } +function resolveLastChannel(message: unknown): void { + const call = requireFixture(invokeMock.mock.calls.at(-1), 'queued command invocation') + const args = call[1] + if (!hasReply(args)) { + throw new Error('queued command invocation did not include a reply channel') + } + args.reply.onmessage(message) +} + +function hasReply(value: unknown): value is { reply: InstanceType } { + return typeof value === 'object' && value != null && 'reply' in value && value.reply instanceof TestChannel +} + +function queuedArgs(command: string): unknown { + const call = invokeMock.mock.calls.find(([called]) => called === command) + return requireFixture(call, `${command} invocation`)[1] +} + +function queuedReply(command: string): InstanceType { + const args = queuedArgs(command) + if (!hasReply(args)) throw new Error(`${command} invocation did not include a reply channel`) + return args.reply +} + describe('Tauri desktop adapter contract', () => { beforeEach(() => { invokeMock.mockClear() @@ -53,14 +89,22 @@ describe('Tauri desktop adapter contract', () => { await tauriDesktopApi.copyText('text') await tauriDesktopApi.quitApp() await tauriDesktopApi.removeStaleInstalls() - await tauriDesktopApi.getSettings() + const settings = tauriDesktopApi.getSettings() + resolveLastChannel({ kind: 'ok', value: undefined }) + await settings await tauriDesktopApi.listModels() await tauriDesktopApi.listLanguages() - await tauriDesktopApi.setSettings(change) + const settingsWrite = tauriDesktopApi.setSettings(change) + resolveLastChannel({ kind: 'ok', value: undefined }) + await settingsWrite await tauriDesktopApi.listGpuDevices() await tauriDesktopApi.listGpuDevices(true) - await tauriDesktopApi.getMicrophones() - await tauriDesktopApi.setMicrophone('device') + const microphones = tauriDesktopApi.getMicrophones() + resolveLastChannel({ kind: 'ok', value: undefined }) + await microphones + const microphoneWrite = tauriDesktopApi.setMicrophone('device') + resolveLastChannel({ kind: 'ok', value: undefined }) + await microphoneWrite await tauriDesktopApi.testInputDevice('device') await tauriDesktopApi.testMicrophoneFallback() await tauriDesktopApi.getReadiness() @@ -94,14 +138,14 @@ describe('Tauri desktop adapter contract', () => { ['copy_text', { text: 'text' }], ['quit_app'], ['remove_stale_installs'], - ['get_settings'], + ['get_settings', { reply: queuedReply('get_settings') }], ['list_models'], ['list_languages'], - ['set_settings', { change }], + ['set_settings', { change, reply: queuedReply('set_settings') }], ['list_gpu_devices', { refresh: false }], ['list_gpu_devices', { refresh: true }], - ['get_microphones'], - ['set_microphone', { id: 'device' }], + ['get_microphones', { reply: queuedReply('get_microphones') }], + ['set_microphone', { id: 'device', reply: queuedReply('set_microphone') }], ['test_input_device', { id: 'device' }], ['test_microphone_fallback'], ['get_readiness'], @@ -112,6 +156,34 @@ describe('Tauri desktop adapter contract', () => { ['remove_managed', { component: 'whisper-small' }], ['cancel_setup', { operation: 'operation' }], ]) + expect(invokeMock).toHaveBeenCalledWith('delete_history_item', { id: 'history-id' }) + expect(invokeMock).toHaveBeenCalledWith('set_settings', { + change, + reply: queuedReply('set_settings'), + }) + expect(queuedReply('get_settings')).toBeInstanceOf(TestChannel) + expect(queuedReply('get_microphones')).toBeInstanceOf(TestChannel) + expect(invokeMock).toHaveBeenCalledWith('set_microphone', { + id: 'device', + reply: queuedReply('set_microphone'), + }) + }) + + it('resolves queued command promises from the channel reply', async () => { + const pending = tauriDesktopApi.getSettings() + expect(hasReply(queuedArgs('get_settings'))).toBe(true) + + resolveLastChannel({ kind: 'ok', value: { revision: 42 } }) + + await expect(pending).resolves.toEqual({ revision: 42 }) + }) + + it('rejects queued command promises from the channel reply', async () => { + const pending = tauriDesktopApi.setSettings({ kind: 'hud', value: false }) + + resolveLastChannel({ kind: 'err', error: 'write failed' }) + + await expect(pending).rejects.toThrow('write failed') }) it('subscribes to setup-event and unwraps its payload', async () => { diff --git a/frontend/src/api/tauriDesktopApi.ts b/frontend/src/api/tauriDesktopApi.ts index 83c7552..6555d13 100644 --- a/frontend/src/api/tauriDesktopApi.ts +++ b/frontend/src/api/tauriDesktopApi.ts @@ -1,7 +1,8 @@ -import { invoke } from '@tauri-apps/api/core' +import { Channel, invoke } from '@tauri-apps/api/core' import { listen } from '@tauri-apps/api/event' import type { AppStatus, + ChannelReply, DictionaryBatchResult, DictionaryItem, DictionaryTrainingSample, @@ -21,6 +22,19 @@ import type { } from '../generated/ipc' import type { DesktopApi } from './DesktopApi' +function invokeQueued(command: string, args: Record = {}): Promise { + return new Promise((resolve, reject) => { + const reply = new Channel>((message) => { + if (message.kind === 'ok') { + resolve(message.value) + } else { + reject(new Error(message.error)) + } + }) + invoke(command, { ...args, reply }).catch(reject) + }) +} + export function createTauriDesktopApi(): DesktopApi { return { getAppStatus: () => invoke('get_app_status'), @@ -51,14 +65,14 @@ export function createTauriDesktopApi(): DesktopApi { copyText: (text) => invoke('copy_text', { text }), quitApp: () => invoke('quit_app'), removeStaleInstalls: () => invoke('remove_stale_installs'), - getSettings: () => invoke('get_settings'), + getSettings: () => invokeQueued('get_settings'), listModels: () => invoke('list_models'), listLanguages: () => invoke('list_languages'), setSettings: (change: SettingsChange) => - invoke('set_settings', { change }), + invokeQueued('set_settings', { change }), listGpuDevices: (refresh = false) => invoke('list_gpu_devices', { refresh }), - getMicrophones: () => invoke('get_microphones'), - setMicrophone: (id) => invoke('set_microphone', { id }), + getMicrophones: () => invokeQueued('get_microphones'), + setMicrophone: (id) => invokeQueued('set_microphone', { id }), testInputDevice: (id) => invoke('test_input_device', { id }), testMicrophoneFallback: () => invoke('test_microphone_fallback'), getReadiness: () => invoke('get_readiness'), diff --git a/frontend/src/app/AppSetup.test.tsx b/frontend/src/app/AppSetup.test.tsx index 3c01632..0155cf8 100644 --- a/frontend/src/app/AppSetup.test.tsx +++ b/frontend/src/app/AppSetup.test.tsx @@ -15,6 +15,7 @@ import { } from '../tauri' import { deferred, resetDesktopApiMocks } from '../test/desktopApiHarness' import type { + MicrophoneSnapshot, SetupEvent, } from '../generated/ipc' @@ -127,6 +128,42 @@ describe('Echo desktop shell', () => { }) + it('does not let a delayed SetupChecklist microphone read replace a newer selection', async () => { + const actual = await vi.importActual('../tauri') + seedPreviewReadiness({ + ...await actual.getReadiness(), + microphoneReady: false, + speechReady: true, + hasSuccessfulDictation: false, + firstRunComplete: false, + }) + const initial = await actual.getMicrophones() + const selectedDevice = requireFixture(initial.devices.find((device) => !device.isDefault), 'selectable microphone') + const staleRefresh = deferred() + const selection = deferred() + + render() + const selectedChoice = await screen.findByRole('radio', { name: new RegExp(selectedDevice.label) }) + vi.mocked(getMicrophones).mockImplementationOnce(() => staleRefresh.promise) + vi.mocked(setMicrophone).mockImplementationOnce(() => selection.promise) + + fireEvent.click(screen.getByRole('button', { name: 'Refresh' })) + fireEvent.click(selectedChoice) + await waitFor(() => expect(setMicrophone).toHaveBeenCalledWith(selectedDevice.id)) + const selected = await actual.setMicrophone(selectedDevice.id) + selection.resolve(selected) + await act(async () => selection.promise) + await waitFor(() => expect(selectedChoice).toBeChecked()) + + staleRefresh.resolve({ + ...initial, + revision: selected.revision - 1, + }) + await act(async () => staleRefresh.promise) + expect(selectedChoice).toBeChecked() + }) + + it('serializes terminal refreshes without delaying a failed setup event', async () => { const listener: { current: ((event: SetupEvent) => void) | null } = { current: null } vi.mocked(onSetupEvent).mockImplementation((handler) => { diff --git a/frontend/src/generated/ipc.ts b/frontend/src/generated/ipc.ts index 47252b9..dafb2f6 100644 --- a/frontend/src/generated/ipc.ts +++ b/frontend/src/generated/ipc.ts @@ -10,6 +10,8 @@ export type AppStatus = { phase: AppPhase, lastTranscript: string | null, lastHi export type AudioHost = "pipe-wire" | "pulse-audio" | "alsa" | "core-audio" | "wasapi" | "other"; +export type ChannelReply = { "kind": "ok", value: T, } | { "kind": "err", error: string, }; + export type ComponentId = "whisper-runtime" | "whisper-vulkan-runtime" | "whisper-base-q51" | "whisper-small" | "whisper-large-v3-turbo-q50" | "silero-vad" | "sherpa-runtime" | "parakeet-tdt06b-v3-int8"; export type ComponentOrigin = "system" | "external"; @@ -68,7 +70,7 @@ export type MicrophoneFailure = "disconnected" | "selection" | "permission" | "b export type MicrophoneSelection = { "kind": "system-default", active: InputDevice | null, } | { "kind": "selected", device: InputDevice, } | { "kind": "legacy-match", name: string, device: InputDevice, } | { "kind": "missing-with-fallback", requestedId: string, requestedLabel: string, fallback: InputDevice, } | { "kind": "missing-without-fallback", requestedId: string, requestedLabel: string, } | { "kind": "ambiguous-legacy-name", name: string, matches: Array, fallback: InputDevice | null, }; -export type MicrophoneSnapshot = { host: AudioHost, source: MicrophoneSource, systemDefault: InputDevice | null, systemDefaultIsProxy: boolean, devices: Array, selection: MicrophoneSelection, enumerationWarning: string | null, }; +export type MicrophoneSnapshot = { revision: number, host: AudioHost, source: MicrophoneSource, systemDefault: InputDevice | null, systemDefaultIsProxy: boolean, devices: Array, selection: MicrophoneSelection, enumerationWarning: string | null, }; export type MicrophoneSource = "environment" | "config" | "default"; @@ -106,7 +108,7 @@ export type Settings = { engine: SettingField, whisperModel: SettingFiel export type SettingsChange = { "kind": "engine", value: string | null, } | { "kind": "whisperModel", value: string | null, } | { "kind": "hud", value: boolean | null, } | { "kind": "recordSeconds", value: number | null, } | { "kind": "language", value: string | null, } | { "kind": "whisperAcceleration", value: string | null, } | { "kind": "whisperGpuDevice", value: string | null, } | { "kind": "enableWhisperGpu" }; -export type SettingsSnapshot = { preferences: Settings, transcription: TranscriptionSnapshot, readiness: Readiness, }; +export type SettingsSnapshot = { revision: number, preferences: Settings, transcription: TranscriptionSnapshot, readiness: Readiness, }; export type SetupEvent = { "kind": "progress", progress: InstallProgress, } | { "kind": "finished", operationId: string, } | { "kind": "cancelled", operationId: string, } | { "kind": "failed", operationId: string, error: string, }; diff --git a/frontend/src/home/SetupChecklist.tsx b/frontend/src/home/SetupChecklist.tsx index ba90a01..6df248e 100644 --- a/frontend/src/home/SetupChecklist.tsx +++ b/frontend/src/home/SetupChecklist.tsx @@ -5,6 +5,7 @@ import { SectionHeading } from '../app/chrome' import { messageFrom } from '../app/formatting' import { useAsyncSubscription } from '../hooks/useAsyncSubscription' import { MicrophoneChooser } from '../settings/MicrophoneChooser' +import { newestSnapshot } from '../settings/snapshotFreshness' import { SpeechSetupSection } from '../settings/SpeechSetupSection' import { applySetupProgress, classifySetupEvent } from '../setup' import { presentShortcut } from '../shortcut' @@ -42,6 +43,9 @@ export function SetupChecklist({ const reportSetupError = useCallback((reason: unknown) => { if (mountedRef.current) setSetupError(messageFrom(reason)) }, []) + const applyMicrophoneSnapshot = useCallback((next: MicrophoneSnapshot) => { + if (mountedRef.current) setMicrophones((current) => newestSnapshot(current, next)) + }, []) useEffect(() => { let current = true @@ -52,7 +56,7 @@ export function SetupChecklist({ if (current && mountedRef.current) reportSetupError(reason) }) void getMicrophones().then((next) => { - if (current && mountedRef.current) setMicrophones(next) + if (current && mountedRef.current) applyMicrophoneSnapshot(next) }).catch((reason: unknown) => { if (current && mountedRef.current) reportSetupError(reason) }) @@ -61,7 +65,7 @@ export function SetupChecklist({ mountedRef.current = false micTestVersion.current += 1 } - }, [reportSetupError]) + }, [applyMicrophoneSnapshot, reportSetupError]) const handleSetupEvent = useCallback((event: SetupEvent) => { if (!mountedRef.current) return @@ -119,7 +123,7 @@ export function SetupChecklist({ void Promise.all([getMicrophones(), getReadiness()]) .then(([nextMicrophones, nextReadiness]) => { if (!mountedRef.current) return - setMicrophones(nextMicrophones) + applyMicrophoneSnapshot(nextMicrophones) setReadiness(nextReadiness) }) .catch(reportSetupError) @@ -131,7 +135,7 @@ export function SetupChecklist({ void setMicrophone(id) .then((nextMicrophones) => { if (!mountedRef.current) return null - setMicrophones(nextMicrophones) + applyMicrophoneSnapshot(nextMicrophones) return getReadiness() }) .then((next) => { diff --git a/frontend/src/settings/snapshotFreshness.test.ts b/frontend/src/settings/snapshotFreshness.test.ts new file mode 100644 index 0000000..40cfb18 --- /dev/null +++ b/frontend/src/settings/snapshotFreshness.test.ts @@ -0,0 +1,20 @@ +import { describe, expect, it } from 'vitest' + +import { newestSnapshot } from './snapshotFreshness' + +type TestSnapshot = { revision: number; value: string } + +describe('newestSnapshot', () => { + it('ignores lower and missing revisions and accepts newer snapshots', () => { + const current: TestSnapshot = { revision: 4, value: 'current' } + const missingRevision: TestSnapshot = { revision: 3, value: 'missing' } + Object.defineProperty(missingRevision, 'revision', { value: undefined }) + + expect(newestSnapshot(current, { revision: 3, value: 'old' })).toBe(current) + expect(newestSnapshot(current, missingRevision)).toBe(current) + expect(newestSnapshot(current, { revision: 5, value: 'new' })).toEqual({ + revision: 5, + value: 'new', + }) + }) +}) diff --git a/frontend/src/settings/snapshotFreshness.ts b/frontend/src/settings/snapshotFreshness.ts new file mode 100644 index 0000000..d966740 --- /dev/null +++ b/frontend/src/settings/snapshotFreshness.ts @@ -0,0 +1,3 @@ +export function newestSnapshot(current: T | null, next: T): T | null { + return next.revision >= (current?.revision ?? 0) ? next : current +} diff --git a/frontend/src/settings/useSettingsController.test.tsx b/frontend/src/settings/useSettingsController.test.tsx index 6ff2942..0259cbe 100644 --- a/frontend/src/settings/useSettingsController.test.tsx +++ b/frontend/src/settings/useSettingsController.test.tsx @@ -4,16 +4,19 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' import { createPreviewDesktopApi } from '../api/previewDesktopApi' import type { ComponentId, + MicrophoneSnapshot, SettingsChange, SettingsSnapshot, SetupEvent, } from '../generated/ipc' import { configureDesktopApi, + getMicrophones, getSettings, onSettingsEvent, onSetupEvent, repairManaged, + setMicrophone, setSettings, } from '../tauri' import { useSettingsController } from './useSettingsController' @@ -24,14 +27,21 @@ vi.mock('../tauri', async (importOriginal) => { const actual = await importOriginal() return { ...actual, + getMicrophones: vi.fn(() => actual.getMicrophones()), getSettings: vi.fn(() => actual.getSettings()), onSettingsEvent: vi.fn((handler: () => void) => actual.onSettingsEvent(handler)), onSetupEvent: vi.fn((handler: (event: SetupEvent) => void) => actual.onSetupEvent(handler)), repairManaged: vi.fn((component: ComponentId) => actual.repairManaged(component)), + setMicrophone: vi.fn((id: string | null) => actual.setMicrophone(id)), setSettings: vi.fn((change: SettingsChange) => actual.setSettings(change)), } }) +function requireFixture(value: T | null | undefined, description: string): T { + if (value == null) throw new Error(`missing test fixture: ${description}`) + return value +} + function deferred() { let resolvePromise: ((value: T | PromiseLike) => void) | null = null const promise = new Promise((resolve) => { @@ -51,6 +61,8 @@ describe('useSettingsController', () => { configureDesktopApi(previewDesktopApi) previewDesktopApi.resetPreviewSettings() const actual = await vi.importActual('../tauri') + vi.mocked(getMicrophones).mockReset() + vi.mocked(getMicrophones).mockImplementation(() => actual.getMicrophones()) vi.mocked(getSettings).mockReset() vi.mocked(getSettings).mockImplementation(() => actual.getSettings()) vi.mocked(onSettingsEvent).mockReset() @@ -59,11 +71,13 @@ describe('useSettingsController', () => { vi.mocked(onSetupEvent).mockImplementation((handler) => actual.onSetupEvent(handler)) vi.mocked(repairManaged).mockReset() vi.mocked(repairManaged).mockImplementation((component) => actual.repairManaged(component)) + vi.mocked(setMicrophone).mockReset() + vi.mocked(setMicrophone).mockImplementation((id) => actual.setMicrophone(id)) vi.mocked(setSettings).mockReset() vi.mocked(setSettings).mockImplementation((change) => actual.setSettings(change)) }) - it('applies queued field changes in call order', async () => { + it('delegates rapid field changes in call order', async () => { const actual = await vi.importActual('../tauri') const firstWriteStarted = deferred() const releaseFirstWrite = deferred() @@ -88,21 +102,21 @@ describe('useSettingsController', () => { hudWrite = result.current.updateHud(false) }) await firstWriteStarted.promise - expect(setSettings).toHaveBeenCalledOnce() - - await act(async () => releaseFirstWrite.resolve()) - await act(async () => Promise.all([languageWrite, hudWrite])) - expect(setSettings).toHaveBeenCalledTimes(2) expect(vi.mocked(setSettings).mock.calls.map(([change]) => change)).toEqual([ { kind: 'language', value: 'en' }, { kind: 'hud', value: false }, ]) + + await act(async () => releaseFirstWrite.resolve()) + await act(async () => Promise.all([languageWrite, hudWrite])) + + expect(setSettings).toHaveBeenCalledTimes(2) expect(result.current.settings?.language.value).toBe('en') expect(result.current.settings?.hud.value).toBe(false) }) - it('continues the settings write chain after a rejected write', async () => { + it('continues settings writes after a rejected write', async () => { const actual = await vi.importActual('../tauri') vi.mocked(setSettings) .mockRejectedValueOnce(new Error('first write failed')) @@ -132,6 +146,47 @@ describe('useSettingsController', () => { expect(result.current.settingsWritePending).toBe(false) }) + it('does not let a delayed microphone read replace a newer selection', async () => { + const actual = await vi.importActual('../tauri') + const initial = await actual.getMicrophones() + const selectedDevice = requireFixture(initial.devices.find((device) => !device.isDefault), 'selectable microphone') + const staleRefresh = deferred() + const selection = deferred() + const onStatusChange = vi.fn().mockResolvedValue(undefined) + const onError = vi.fn() + const { result } = renderHook(() => useSettingsController({ + onStatusChange, + onError, + })) + await waitFor(() => expect(result.current.microphones).not.toBeNull()) + vi.mocked(getMicrophones).mockImplementationOnce(() => staleRefresh.promise) + vi.mocked(setMicrophone).mockImplementationOnce(() => selection.promise) + + let refresh: Promise + act(() => { + refresh = result.current.refreshMicrophones() + result.current.selectMicrophone(selectedDevice.id) + }) + await waitFor(() => expect(setMicrophone).toHaveBeenCalledWith(selectedDevice.id)) + const selected = await actual.setMicrophone(selectedDevice.id) + selection.resolve(selected) + await act(async () => selection.promise) + expect(result.current.microphones?.selection).toMatchObject({ + kind: 'selected', + device: { id: selectedDevice.id }, + }) + + staleRefresh.resolve({ + ...initial, + revision: selected.revision - 1, + }) + await act(async () => refresh) + expect(result.current.microphones?.selection).toMatchObject({ + kind: 'selected', + device: { id: selectedDevice.id }, + }) + }) + it('does not let the initial settings read replace a newer settings write', async () => { const staleSettings = await previewDesktopApi.getSettings() const initialRead = deferred() @@ -307,7 +362,7 @@ describe('useSettingsController', () => { expect(onError).not.toHaveBeenCalled() }) - it('waits for an already queued settings write before refreshing readiness', async () => { + it('delegates readiness refreshes while a backend settings write is queued', async () => { const actual = await vi.importActual('../tauri') const writeStarted = deferred() const releaseWrite = deferred() @@ -335,15 +390,14 @@ describe('useSettingsController', () => { await Promise.resolve() }) - expect(getSettings).toHaveBeenCalledOnce() + expect(getSettings).toHaveBeenCalledTimes(2) await act(async () => releaseWrite.resolve()) await act(async () => write) - await waitFor(() => expect(getSettings).toHaveBeenCalledTimes(2)) expect(result.current.settings?.hud.value).toBe(false) }) - it('waits for an already queued settings write before the GPU repair refresh', async () => { + it('delegates GPU repair refreshes while a backend settings write is queued', async () => { const actual = await vi.importActual('../tauri') const initialSettings = await previewDesktopApi.getSettings() const component = initialSettings.readiness.components.find( @@ -393,14 +447,44 @@ describe('useSettingsController', () => { }) expect(repairManaged).toHaveBeenCalledWith('whisper-vulkan-runtime') - expect(getSettings).toHaveBeenCalledOnce() + expect(getSettings).toHaveBeenCalledTimes(2) await act(async () => releaseWrite.resolve()) await act(async () => write) - await waitFor(() => expect(getSettings).toHaveBeenCalledTimes(2)) expect(result.current.settings?.hud.value).toBe(false) }) + it('does not let a snapshot without a revision replace a newer settings write', async () => { + let setupEvent: ((event: SetupEvent) => void) | null = null + vi.mocked(onSetupEvent).mockImplementation((handler) => { + setupEvent = handler + return Promise.resolve(vi.fn()) + }) + const onStatusChange = vi.fn().mockResolvedValue(undefined) + const onError = vi.fn() + const { result } = renderHook(() => useSettingsController({ + onStatusChange, + onError, + })) + await waitFor(() => { + expect(result.current.settings).not.toBeNull() + expect(setupEvent).not.toBeNull() + }) + const staleSettings = await previewDesktopApi.getSettings() + const missingRevision = { ...staleSettings } + Object.defineProperty(missingRevision, 'revision', { value: undefined }) + const staleRefresh = deferred>>() + vi.mocked(getSettings).mockImplementationOnce(() => staleRefresh.promise) + + act(() => setupEvent?.({ kind: 'finished', operationId: 'setup' })) + await waitFor(() => expect(getSettings).toHaveBeenCalledTimes(2)) + await act(async () => result.current.updateHud(false)) + + staleRefresh.resolve(missingRevision) + await act(async () => staleRefresh.promise) + await waitFor(() => expect(result.current.settings?.hud.value).toBe(false)) + }) + it('does not let a stale setup refresh replace a newer settings write', async () => { let setupEvent: ((event: SetupEvent) => void) | null = null vi.mocked(onSetupEvent).mockImplementation((handler) => { diff --git a/frontend/src/settings/useSettingsController.ts b/frontend/src/settings/useSettingsController.ts index f98c195..38b3706 100644 --- a/frontend/src/settings/useSettingsController.ts +++ b/frontend/src/settings/useSettingsController.ts @@ -3,6 +3,7 @@ import { useCallback, useEffect, useRef, useState } from 'react' import { messageFrom } from '../app/formatting' import { useAsyncSubscription } from '../hooks/useAsyncSubscription' import { useSerialPoll } from '../hooks/useSerialPoll' +import { newestSnapshot } from './snapshotFreshness' import { applySetupProgress, classifySetupEvent } from '../setup' import { getMicrophones, @@ -32,20 +33,12 @@ interface UseSettingsControllerArgs { onError: (message: string) => void } -interface SettingsReadResult { - snapshot: SettingsSnapshot - mutationVersion: number - readVersion: number -} - export function useSettingsController({ onStatusChange, onError, }: UseSettingsControllerArgs) { const [snapshot, setSnapshot] = useState(null) - const writeChainRef = useRef(Promise.resolve()) - const settingsMutationVersion = useRef(0) - const settingsReadVersion = useRef(0) + const pendingSettingsWrites = useRef(0) const active = useRef(true) const [microphones, setMicrophones] = useState(null) const [micTest, setMicTest] = useState(null) @@ -59,24 +52,13 @@ export function useSettingsController({ if (active.current) onError(messageFrom(reason)) }, [onError]) - const loadSettingsSnapshot = useCallback(async (): Promise => { - const readVersion = ++settingsReadVersion.current - const mutationVersion = settingsMutationVersion.current - const queuedWrites = writeChainRef.current - await queuedWrites - if (!active.current || settingsMutationVersion.current !== mutationVersion) return null - return { snapshot: await getSettings(), mutationVersion, readVersion } + const loadSettingsSnapshot = useCallback(async (): Promise => { + const next = await getSettings() + return active.current ? next : null }, []) - const applySettingsSnapshot = useCallback((result: SettingsReadResult | null) => { - if ( - result != null && - active.current && - settingsMutationVersion.current === result.mutationVersion && - settingsReadVersion.current === result.readVersion - ) { - setSnapshot(result.snapshot) - } + const applySettingsSnapshot = useCallback((next: SettingsSnapshot | null) => { + if (next != null && active.current) setSnapshot((current) => newestSnapshot(current, next)) }, []) useEffect(() => { @@ -118,9 +100,13 @@ export function useSettingsController({ } }, [wantsGpu, gpuRuntimeReady, reportSettingsError]) + const applyMicrophoneSnapshot = useCallback((next: MicrophoneSnapshot) => { + if (active.current) setMicrophones((current) => newestSnapshot(current, next)) + }, []) + const refreshMicrophones = useSerialPoll({ request: getMicrophones, - onResult: setMicrophones, + onResult: applyMicrophoneSnapshot, onError: reportSettingsError, intervalMs: 3_000, }) @@ -177,30 +163,28 @@ export function useSettingsController({ try { const written = await setSettings(change) if (!active.current) return - setSnapshot(written) + applySettingsSnapshot(written) await onStatusChange() } catch (reason) { reportSettingsError(reason) throw reason } - }, [onStatusChange, reportSettingsError]) + }, [applySettingsSnapshot, onStatusChange, reportSettingsError]) const updateSettings = useCallback(async (change: SettingsChange) => { setSettingsWritePending(true) - settingsMutationVersion.current += 1 - const queued = writeChainRef.current.then(() => commit(change)) - const continuing = queued.catch(() => undefined) - writeChainRef.current = continuing + pendingSettingsWrites.current += 1 try { - await queued + await commit(change) } catch { - const result = await loadSettingsSnapshot().catch((reason: unknown) => { + const next = await loadSettingsSnapshot().catch((reason: unknown) => { reportSettingsError(reason) return null }) - applySettingsSnapshot(result) + applySettingsSnapshot(next) } finally { - if (active.current && writeChainRef.current === continuing) setSettingsWritePending(false) + pendingSettingsWrites.current = Math.max(0, pendingSettingsWrites.current - 1) + if (active.current && pendingSettingsWrites.current === 0) setSettingsWritePending(false) } }, [applySettingsSnapshot, commit, loadSettingsSnapshot, reportSettingsError]) @@ -259,11 +243,11 @@ export function useSettingsController({ void setMicrophone(id) .then((next) => { if (!active.current) return null - setMicrophones(next) + applyMicrophoneSnapshot(next) return onStatusChange() }) .catch(reportSettingsError) - }, [onStatusChange, reportSettingsError]) + }, [applyMicrophoneSnapshot, onStatusChange, reportSettingsError]) const testMicrophone = useCallback((id: string | null, fallback: boolean) => { const version = ++micTestVersion.current diff --git a/frontend/src/tauri.test.ts b/frontend/src/tauri.test.ts index e6071ce..7128474 100644 --- a/frontend/src/tauri.test.ts +++ b/frontend/src/tauri.test.ts @@ -1,6 +1,5 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' import { createPreviewDesktopApi } from './api/previewDesktopApi' -import type { MicrophoneSnapshot } from './generated/ipc' import { configureDesktopApi, getMicrophones as getConfiguredMicrophones, @@ -19,28 +18,6 @@ const { cancelTranscription, } = createPreviewDesktopApi() -function deferred() { - const state: { - resolve: ((value: T | PromiseLike) => void) | null - reject: ((reason?: unknown) => void) | null - } = { resolve: null, reject: null } - const promise = new Promise((resolvePromise, rejectPromise) => { - state.resolve = resolvePromise - state.reject = rejectPromise - }) - return { - promise, - resolve(value: T | PromiseLike) { - if (!state.resolve) throw new Error('deferred promise is not initialized') - state.resolve(value) - }, - reject(reason?: unknown) { - if (!state.reject) throw new Error('deferred promise is not initialized') - state.reject(reason) - }, - } -} - describe('settings preview wrappers', () => { it('rejects stale stop and cancellation requests without returning another session', async () => { const started = await startCapture() @@ -75,7 +52,11 @@ describe('settings preview wrappers', () => { expect(written.preferences.engine).toEqual({ value: 'fake', effective: 'fake', source: 'file' }) expect(written.preferences.hud).toEqual({ value: false, effective: false, source: 'file' }) expect(written.preferences.recordSeconds).toEqual({ value: 12, effective: 12, source: 'file' }) - expect(await getSettings()).toEqual(written) + const reread = await getSettings() + expect(reread.revision).toBeGreaterThan(written.revision) + expect(reread.preferences).toEqual(written.preferences) + expect(reread.transcription).toEqual(written.transcription) + expect(reread.readiness).toEqual(written.readiness) }) it('snapshots the effective limit when preview recording starts', async () => { @@ -118,44 +99,41 @@ describe('settings preview wrappers', () => { } }) - it('serializes microphone selections so the final choice wins', async () => { + it('delegates microphone operations directly to the configured adapter', async () => { const preview = createPreviewDesktopApi() const initial = await preview.getMicrophones() - const first = deferred() const set = preview.setMicrophone.bind(preview) const select = vi.spyOn(preview, 'setMicrophone') - .mockImplementationOnce(() => first.promise) .mockImplementation((id) => set(id)) + const read = vi.spyOn(preview, 'getMicrophones') configureDesktopApi(preview) const firstChoice = initial.devices[0] const finalChoice = initial.devices[1] if (!firstChoice || !finalChoice) throw new Error('preview needs two microphones') - const firstRequest = setConfiguredMicrophone(firstChoice.id) - const finalRequest = setConfiguredMicrophone(finalChoice.id) - await Promise.resolve() - expect(select).toHaveBeenCalledOnce() + await Promise.all([ + setConfiguredMicrophone(firstChoice.id), + setConfiguredMicrophone(finalChoice.id), + ]) + await getConfiguredMicrophones() - first.resolve(initial) - await firstRequest - await finalRequest + expect(select).toHaveBeenCalledTimes(2) expect(select).toHaveBeenNthCalledWith(1, firstChoice.id) expect(select).toHaveBeenNthCalledWith(2, finalChoice.id) + expect(read).toHaveBeenCalledOnce() expect((await getConfiguredMicrophones()).selection).toMatchObject({ kind: 'selected', device: { id: finalChoice.id }, }) }) - it('continues microphone operations after a rejected selection and keeps reads ordered', async () => { + it('continues microphone operations after a rejected selection', async () => { const preview = createPreviewDesktopApi() const initial = await preview.getMicrophones() - const reject = deferred() const set = preview.setMicrophone.bind(preview) const select = vi.spyOn(preview, 'setMicrophone') - .mockImplementationOnce(() => reject.promise) + .mockRejectedValueOnce(new Error('device disconnected')) .mockImplementation((id) => set(id)) - const read = vi.spyOn(preview, 'getMicrophones') configureDesktopApi(preview) const firstChoice = initial.devices[0] @@ -163,37 +141,23 @@ describe('settings preview wrappers', () => { if (!firstChoice || !finalChoice) throw new Error('preview needs two microphones') const rejected = setConfiguredMicrophone(firstChoice.id) const finalRequest = setConfiguredMicrophone(finalChoice.id) - const readRequest = getConfiguredMicrophones() - await Promise.resolve() - expect(select).toHaveBeenCalledOnce() - expect(read).not.toHaveBeenCalled() - reject.reject(new Error('device disconnected')) await expect(rejected).rejects.toThrow('device disconnected') await finalRequest - await readRequest expect(select).toHaveBeenCalledTimes(2) - expect(read).toHaveBeenCalledOnce() }) - it('keeps queued microphone work with the adapter that accepted it', async () => { + it('uses the current adapter for each microphone call', async () => { const firstAdapter = createPreviewDesktopApi() const secondAdapter = createPreviewDesktopApi() - const initial = await firstAdapter.getMicrophones() - const pending = deferred() - const firstSet = vi.spyOn(firstAdapter, 'setMicrophone').mockImplementation(() => pending.promise) + const firstSet = vi.spyOn(firstAdapter, 'setMicrophone') const secondRead = vi.spyOn(secondAdapter, 'getMicrophones') - const choice = initial.devices[0] - if (!choice) throw new Error('preview needs a microphone') configureDesktopApi(firstAdapter) - const firstRequest = setConfiguredMicrophone(choice.id) + await setConfiguredMicrophone(null) configureDesktopApi(secondAdapter) await getConfiguredMicrophones() expect(firstSet).toHaveBeenCalledOnce() expect(secondRead).toHaveBeenCalledOnce() - - pending.resolve(initial) - await firstRequest }) }) diff --git a/frontend/src/tauri.ts b/frontend/src/tauri.ts index 0acb895..f9f2375 100644 --- a/frontend/src/tauri.ts +++ b/frontend/src/tauri.ts @@ -1,12 +1,9 @@ import type { DesktopApi } from './api/DesktopApi' -import type { MicrophoneSnapshot } from './generated/ipc' let desktopApi: DesktopApi | undefined -let microphoneQueue: Promise = Promise.resolve() export function configureDesktopApi(api: DesktopApi): void { desktopApi = api - microphoneQueue = Promise.resolve() } function api(): DesktopApi { @@ -14,15 +11,6 @@ function api(): DesktopApi { return desktopApi } -function queueMicrophoneOperation(operation: () => Promise): Promise { - const request = microphoneQueue.then(operation) - microphoneQueue = request.then( - () => undefined, - () => undefined, - ) - return request -} - export const getAppStatus: DesktopApi['getAppStatus'] = () => api().getAppStatus() export const getShortcutStatus: DesktopApi['getShortcutStatus'] = () => api().getShortcutStatus() export const retryShortcut: DesktopApi['retryShortcut'] = () => api().retryShortcut() @@ -56,14 +44,8 @@ export const listModels: DesktopApi['listModels'] = () => api().listModels() export const listLanguages: DesktopApi['listLanguages'] = () => api().listLanguages() export const setSettings: DesktopApi['setSettings'] = (settings) => api().setSettings(settings) export const listGpuDevices: DesktopApi['listGpuDevices'] = (refresh = false) => api().listGpuDevices(refresh) -export const getMicrophones: DesktopApi['getMicrophones'] = () => { - const configuredApi = api() - return queueMicrophoneOperation(() => configuredApi.getMicrophones()) -} -export const setMicrophone: DesktopApi['setMicrophone'] = (id) => { - const configuredApi = api() - return queueMicrophoneOperation(() => configuredApi.setMicrophone(id)) -} +export const getMicrophones: DesktopApi['getMicrophones'] = () => api().getMicrophones() +export const setMicrophone: DesktopApi['setMicrophone'] = (id) => api().setMicrophone(id) export const testInputDevice: DesktopApi['testInputDevice'] = (id) => api().testInputDevice(id) export const testMicrophoneFallback: DesktopApi['testMicrophoneFallback'] = () => diff --git a/src-tauri/src/commands/devices.rs b/src-tauri/src/commands/devices.rs index a252ed3..8ff39b5 100644 --- a/src-tauri/src/commands/devices.rs +++ b/src-tauri/src/commands/devices.rs @@ -1,8 +1,8 @@ -use std::env; use std::sync::{Mutex, OnceLock}; use echo::audio::AudioCapture; -use echo_desktop::ipc::{LanguageOptions, ModelInventory}; +use echo_desktop::ipc::{ChannelReply, LanguageOptions, ModelInventory}; +use tauri::ipc::Channel; #[tauri::command] pub(crate) fn list_languages() -> LanguageOptions { @@ -51,60 +51,21 @@ pub(crate) async fn list_models() -> Result { } #[tauri::command] -pub(crate) async fn get_microphones() -> Result { - crate::blocking::run_blocking("microphone enumeration", || { - echo::audio::microphone_snapshot().into() - }) - .await +pub(crate) fn get_microphones( + owner: tauri::State<'_, crate::settings::ConfigMutationService>, + reply: Channel>, +) -> Result<(), String> { + owner.request_microphone_snapshot(reply) } #[tauri::command] -pub(crate) async fn set_microphone( +pub(crate) fn set_microphone( id: Option, -) -> Result { - crate::blocking::run_blocking("microphone selection", move || { - if env::var("ECHO_MICROPHONE") - .ok() - .is_some_and(|value| !value.trim().is_empty()) - { - return Err("ECHO_MICROPHONE controls the microphone in this process".to_string()); - } - let snapshot = echo::audio::microphone_snapshot(); - let selection = match id { - None => None, - Some(raw) => { - let id = echo::microphone::MicrophoneId::parse(raw)?; - let device = snapshot - .devices - .iter() - .find(|device| device.id == id) - .ok_or_else(|| { - "that microphone is no longer connected; refresh and choose again" - .to_string() - })?; - Some((id, device.label.clone())) - } - }; - crate::settings::update_file_config(|config| { - update_microphone_config(config, selection); - Ok(()) - })?; - Ok(echo::audio::microphone_snapshot().into()) - }) - .await? -} - -fn update_microphone_config( - config: &mut echo_core::Config, - selection: Option<(echo::microphone::MicrophoneId, String)>, -) { - config.microphone = - selection.map( - |(id, last_seen_label)| echo_core::MicrophoneSelection::Device { - id: id.as_str().to_string(), - last_seen_label, - }, - ); + owner: tauri::State<'_, crate::settings::ConfigMutationService>, + app: tauri::AppHandle, + reply: Channel>, +) -> Result<(), String> { + owner.request_microphone_selection(id, app, reply) } fn microphone_test( @@ -188,26 +149,11 @@ mod tests { ) { } - fn assert_async_microphones( - _: impl Future>, - ) { - } - #[test] fn gpu_device_listing_yields_before_detection() { assert_async_gpu_devices(list_gpu_devices(false)); } - #[test] - fn microphone_listing_yields_before_detection() { - assert_async_microphones(get_microphones()); - } - - #[test] - fn microphone_selection_yields_before_detection_and_config_write() { - assert_async_microphones(set_microphone(None)); - } - #[test] fn language_command_projection_covers_every_support_mode() { let multilingual = @@ -260,30 +206,4 @@ mod tests { assert_eq!(option.group, echo_desktop::ipc::LanguageGroup::All); } } - - #[test] - fn dedicated_microphone_update_writes_id_and_clears_legacy_name() { - let mut config = echo_core::Config { - microphone: Some(echo_core::MicrophoneSelection::LegacyName { - name: "USB Mic".into(), - }), - ..echo_core::Config::default() - }; - update_microphone_config( - &mut config, - Some(( - echo::microphone::MicrophoneId::parse("alsa:usb-one").unwrap(), - "USB Mic".into(), - )), - ); - assert_eq!( - config.microphone, - Some(echo_core::MicrophoneSelection::Device { - id: "alsa:usb-one".into(), - last_seen_label: "USB Mic".into(), - }) - ); - update_microphone_config(&mut config, None); - assert_eq!(config.microphone, None); - } } diff --git a/src-tauri/src/commands/settings.rs b/src-tauri/src/commands/settings.rs index 543726e..75a155b 100644 --- a/src-tauri/src/commands/settings.rs +++ b/src-tauri/src/commands/settings.rs @@ -1,43 +1,31 @@ -use echo_desktop::ipc::{SettingsChange, SettingsSnapshot}; +use echo_desktop::ipc::{ChannelReply, SettingsChange, SettingsSnapshot}; +use tauri::ipc::Channel; use tauri::{AppHandle, State}; #[cfg(feature = "status-perf-probe")] use tauri::Manager; #[tauri::command] -pub(crate) async fn get_settings( +pub(crate) fn get_settings( state: State<'_, crate::setup::SetupService>, -) -> Result { + owner: State<'_, crate::settings::ConfigMutationService>, + reply: Channel>, +) -> Result<(), String> { let service = state.inner().clone(); - crate::blocking::run_blocking("settings snapshot", move || { - crate::settings::snapshot(service.snapshot()) - }) - .await? + owner.request_settings_snapshot(move || service.snapshot(), reply) } #[tauri::command] -pub(crate) async fn set_settings( +pub(crate) fn set_settings( change: SettingsChange, state: State<'_, crate::setup::SetupService>, + owner: State<'_, crate::settings::ConfigMutationService>, app: AppHandle, -) -> Result { + reply: Channel>, +) -> Result<(), String> { let service = state.inner().clone(); - apply_settings_change(change, service, app).await -} - -async fn apply_settings_change( - change: SettingsChange, - service: crate::setup::SetupService, - app: AppHandle, -) -> Result { let tray_request = crate::tray::request(); - let (revision, snapshot) = crate::blocking::run_blocking("settings change", move || { - crate::settings::change(change)?; - crate::settings::snapshot_with_revision(|| service.snapshot()) - }) - .await??; - crate::tray::sync(&app, tray_request, revision, &snapshot); - Ok(snapshot) + owner.request_settings_change(change, move || service.snapshot(), app, tray_request, reply) } #[cfg(feature = "status-perf-probe")] @@ -46,16 +34,24 @@ pub(crate) fn run_test_hook(app: &AppHandle) { return; }; let service = app.state::().inner().clone(); + let Some(owner) = app.try_state::() else { + return; + }; let app = app.clone(); - tauri::async_runtime::spawn(async move { - if let Err(error) = apply_settings_change( - SettingsChange::Language { value: Some(value) }, - service, - app, - ) - .await - { - eprintln!("tray settings test hook: {error}"); - } - }); + let tray_request = crate::tray::request(); + if let Err(error) = owner.request_settings_change( + SettingsChange::Language { value: Some(value) }, + move || service.snapshot(), + app, + tray_request, + Channel::new(|body| { + let message: serde_json::Value = body.deserialize()?; + if let Some(error) = message.get("error").and_then(serde_json::Value::as_str) { + eprintln!("tray settings test hook: {error}"); + } + Ok(()) + }), + ) { + eprintln!("tray settings test hook: {error}"); + } } diff --git a/src-tauri/src/ipc.rs b/src-tauri/src/ipc.rs index cb96bc1..0f23eb4 100644 --- a/src-tauri/src/ipc.rs +++ b/src-tauri/src/ipc.rs @@ -20,6 +20,7 @@ mod tests { const LIBRARY: &str = include_str!("commands/library.rs"); const RECORDING: &str = include_str!("commands/recording.rs"); const SETTINGS: &str = include_str!("commands/settings.rs"); + const SETTINGS_OWNER: &str = include_str!("settings.rs"); const SHORTCUTS: &str = include_str!("commands/shortcuts.rs"); const STATUS: &str = include_str!("commands/status.rs"); const SYSTEM: &str = include_str!("commands/system.rs"); @@ -139,12 +140,12 @@ mod tests { CommandContract { handler: "get_settings", source: SETTINGS, - payload_types: &["SettingsSnapshot"], + payload_types: &["ChannelReply", "SettingsSnapshot"], }, CommandContract { handler: "set_settings", source: SETTINGS, - payload_types: &["SettingsChange", "SettingsSnapshot"], + payload_types: &["ChannelReply", "SettingsChange", "SettingsSnapshot"], }, CommandContract { handler: "list_models", @@ -194,12 +195,12 @@ mod tests { CommandContract { handler: "get_microphones", source: DEVICES, - payload_types: &["MicrophoneSnapshot"], + payload_types: &["ChannelReply", "MicrophoneSnapshot"], }, CommandContract { handler: "set_microphone", source: DEVICES, - payload_types: &["MicrophoneSnapshot"], + payload_types: &["ChannelReply", "MicrophoneSnapshot"], }, CommandContract { handler: "test_input_device", @@ -221,7 +222,7 @@ mod tests { }, EventContract { name: "settings-event", - source: include_str!("tray.rs"), + source: SETTINGS_OWNER, payload_types: &[], }, ]; @@ -288,6 +289,6 @@ mod tests { manifest_types.insert((*payload_type).to_string()); } } - assert_eq!(manifest_types.len(), 20); + assert_eq!(manifest_types.len(), 21); } } diff --git a/src-tauri/src/main.rs b/src-tauri/src/main.rs index eae92b3..e34f6d4 100644 --- a/src-tauri/src/main.rs +++ b/src-tauri/src/main.rs @@ -100,6 +100,7 @@ fn run_desktop() -> Result<(), String> { context.config_mut().app.tray_icon = None; let builder = tauri::Builder::default() .manage(setup::SetupService::default()) + .manage(settings::ConfigMutationService::default()) .manage(DictionaryTrainingCaptures::default()); #[cfg(not(feature = "status-perf-probe"))] let builder = builder.plugin(tauri_plugin_single_instance::init(|app, _args, _cwd| { diff --git a/src-tauri/src/settings.rs b/src-tauri/src/settings.rs index b64dee3..a66f509 100644 --- a/src-tauri/src/settings.rs +++ b/src-tauri/src/settings.rs @@ -1,42 +1,320 @@ +use std::collections::VecDeque; use std::env; +use std::panic::{self, AssertUnwindSafe}; use std::path::Path; -use std::sync::atomic::{AtomicU64, Ordering}; -use std::sync::Mutex; +use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; +use std::sync::{Arc, Mutex}; use echo_desktop::ipc::{ - Readiness, SettingField, SettingSource, Settings, SettingsChange, SettingsSnapshot, + ChannelReply, Readiness, SettingField, SettingSource, Settings, SettingsChange, + SettingsSnapshot, }; +use tauri::ipc::Channel; +use tauri::{Emitter, Manager}; -static CONFIG_WRITE_LOCK: Mutex<()> = Mutex::new(()); -static SETTINGS_REVISION: AtomicU64 = AtomicU64::new(0); +static SNAPSHOT_RESPONSE_REVISION: AtomicU64 = AtomicU64::new(0); -struct SettingsWriteRevision; +struct ConfigJob { + run: Box, + fail: Box, +} -impl SettingsWriteRevision { - fn begin() -> Self { - SETTINGS_REVISION.fetch_add(1, Ordering::SeqCst); - Self +impl ConfigJob { + fn fire_and_forget(run: impl FnOnce() + Send + 'static) -> Self { + Self { + run: Box::new(run), + fail: Box::new(|error| eprintln!("configuration queue: {error}")), + } + } + + fn reply( + reply: Channel>, + work: impl FnOnce() -> Result + Send + 'static, + ) -> Self + where + T: serde::Serialize + Send + 'static, + { + let reply = Arc::new(Mutex::new(Some(reply))); + let run_reply = Arc::clone(&reply); + let fail_reply = Arc::clone(&reply); + Self { + run: Box::new(move || { + let result = match panic::catch_unwind(AssertUnwindSafe(work)) { + Ok(result) => result, + Err(_) => Err("configuration request failed unexpectedly".to_string()), + }; + send_reply_once(&run_reply, result); + }), + fail: Box::new(move |error| { + send_reply_once(&fail_reply, Err(error)); + }), + } } } -impl Drop for SettingsWriteRevision { - fn drop(&mut self) { - SETTINGS_REVISION.fetch_add(1, Ordering::SeqCst); +#[derive(Default)] +struct ConfigJobQueue { + active: bool, + pending: VecDeque, +} + +impl ConfigJobQueue { + fn enqueue(&mut self, job: ConfigJob) -> bool { + self.pending.push_back(job); + if self.active { + false + } else { + self.active = true; + true + } + } + + fn next(&mut self) -> Option { + let next = self.pending.pop_front(); + if next.is_none() { + self.active = false; + } + next } } -fn lock_config_writes(lock: &Mutex<()>) -> Result, String> { - lock.lock().map_err(|_| { - "Preferences changes are unavailable because the configuration write lock is poisoned." - .to_string() - }) +#[derive(Clone, Default)] +pub(crate) struct ConfigMutationService { + queue: Arc>, } -pub(super) fn update_file_config( +impl ConfigMutationService { + fn enqueue(&self, job: ConfigJob) -> Result<(), String> { + let start_worker = self + .queue + .lock() + .map(|mut queue| queue.enqueue(job)) + .map_err(|_| "configuration queue is unavailable".to_string())?; + if start_worker { + let service = self.clone(); + if let Err(error) = std::thread::Builder::new() + .name("echo-config-owner".to_string()) + .spawn(move || service.drain()) + { + let message = format!("configuration worker could not start: {error}"); + self.fail_pending_jobs(message.clone()); + return Err(message); + } + } + Ok(()) + } + + fn fail_pending_jobs(&self, error: String) { + let jobs = match self.queue.lock() { + Ok(mut queue) => { + queue.active = false; + queue.pending.drain(..).collect::>() + } + Err(_) => return, + }; + for job in jobs { + (job.fail)(error.clone()); + } + } + + fn drain(self) { + loop { + let job = match self.queue.lock() { + Ok(mut queue) => queue.next(), + Err(_) => { + eprintln!("configuration queue is unavailable"); + return; + } + }; + let Some(job) = job else { + return; + }; + let fail = job.fail; + if panic::catch_unwind(AssertUnwindSafe(job.run)).is_err() { + fail("configuration request failed unexpectedly".to_string()); + } + } + } + + pub(crate) fn request_settings_snapshot( + &self, + mut readiness: impl FnMut() -> Readiness + Send + 'static, + reply: Channel>, + ) -> Result<(), String> { + self.enqueue(ConfigJob::reply(reply, move || { + snapshot_with_revision(&mut readiness).map(|(_, snapshot)| snapshot) + })) + } + + pub(crate) fn request_settings_change( + &self, + settings_change: SettingsChange, + mut readiness: impl FnMut() -> Readiness + Send + 'static, + app: tauri::AppHandle, + tray_request: crate::tray::LanguageMenuRequest, + reply: Channel>, + ) -> Result<(), String> { + self.enqueue(ConfigJob::reply(reply, move || { + change(settings_change) + .and_then(|_| snapshot_with_revision(&mut readiness)) + .map(|(revision, snapshot)| { + crate::tray::sync(&app, tray_request, revision, &snapshot); + snapshot + }) + })) + } + + pub(crate) fn request_microphone_snapshot( + &self, + reply: Channel>, + ) -> Result<(), String> { + self.enqueue(ConfigJob::reply(reply, move || { + Ok(revisioned_microphone_snapshot()) + })) + } + + pub(crate) fn request_microphone_selection( + &self, + id: Option, + app: tauri::AppHandle, + reply: Channel>, + ) -> Result<(), String> { + self.enqueue(ConfigJob::reply(reply, move || { + set_microphone_selection(id).map(|()| { + let snapshot = revisioned_microphone_snapshot(); + let _ = app.emit("settings-event", ()); + snapshot + }) + })) + } + + pub(crate) fn request_tray_language( + &self, + value: String, + app: tauri::AppHandle, + tray_request: crate::tray::LanguageMenuRequest, + ) -> Result<(), String> { + let service = app.state::().inner().clone(); + self.enqueue(ConfigJob::fire_and_forget(move || { + let outcome = change(SettingsChange::Language { value: Some(value) }) + .and_then(|_| snapshot_with_revision(|| service.snapshot())); + match outcome { + Ok((revision, snapshot)) => { + crate::tray::sync(&app, tray_request, revision, &snapshot); + let _ = app.emit("settings-event", ()); + } + Err(error) => { + eprintln!("tray language: {error}"); + crate::tray::restore(&app); + } + } + })) + } + + pub(crate) fn request_tray_refresh( + &self, + app: tauri::AppHandle, + tray_request: crate::tray::LanguageMenuRequest, + ) -> Result<(), String> { + let service = app.state::().inner().clone(); + self.enqueue(ConfigJob::fire_and_forget( + move || match snapshot_with_revision(|| service.snapshot()) { + Ok((settings, snapshot)) => crate::tray::sync_requested( + &app, + crate::tray::LanguageMenuRevision { + settings, + request: tray_request.0, + }, + &snapshot, + ), + Err(error) => eprintln!("tray language: failed to read settings: {error}"), + }, + )) + } + + pub(crate) fn apply_setup_plan_blocking( + &self, + plan_id: echo::install::SetupPlanId, + cancel: Arc, + ) -> Result<(), echo::install::InstallError> { + self.apply_setup_plan_blocking_with(plan_id, cancel, move |plan_id| { + update_file_config(|config| apply_plan_config(config, plan_id)) + }) + } + + fn apply_setup_plan_blocking_with( + &self, + plan_id: echo::install::SetupPlanId, + cancel: Arc, + activate: impl FnOnce(echo::install::SetupPlanId) -> Result<(), String> + Send + 'static, + ) -> Result<(), echo::install::InstallError> { + let (sender, receiver) = std::sync::mpsc::channel(); + let fail_sender = sender.clone(); + self.enqueue(ConfigJob { + run: Box::new(move || { + let result = match panic::catch_unwind(AssertUnwindSafe(|| { + if cancel.load(Ordering::Relaxed) { + Err(echo::install::InstallError::Cancelled) + } else { + activate(plan_id).map_err(echo::install::InstallError::IoMessage) + } + })) { + Ok(result) => result, + Err(_) => Err(echo::install::InstallError::IoMessage( + "configuration request failed unexpectedly".to_string(), + )), + }; + let _ = sender.send(result); + }), + fail: Box::new(move |error| { + let _ = fail_sender.send(Err(echo::install::InstallError::IoMessage(error))); + }), + }) + .map_err(echo::install::InstallError::IoMessage)?; + receiver + .recv() + .map_err(|error| echo::install::InstallError::IoMessage(error.to_string()))? + } +} + +fn send_reply_once( + reply: &Arc>>>>, + result: Result, +) where + T: serde::Serialize, +{ + let Ok(mut reply) = reply.lock() else { + return; + }; + let Some(reply) = reply.take() else { + return; + }; + let message = match result { + Ok(value) => ChannelReply::Ok { value }, + Err(error) => ChannelReply::Err { error }, + }; + let _ = reply.send(message); +} + +fn next_snapshot_response_revision() -> u64 { + SNAPSHOT_RESPONSE_REVISION.fetch_add(1, Ordering::SeqCst) + 1 +} + +fn with_snapshot_revision(mut snapshot: SettingsSnapshot) -> SettingsSnapshot { + snapshot.revision = next_snapshot_response_revision(); + snapshot +} + +fn revisioned_microphone_snapshot() -> echo_desktop::ipc::MicrophoneSnapshot { + let mut snapshot: echo_desktop::ipc::MicrophoneSnapshot = + echo::audio::microphone_snapshot().into(); + snapshot.revision = next_snapshot_response_revision(); + snapshot +} + +fn update_file_config( update: impl FnOnce(&mut echo_core::Config) -> Result<(), String>, ) -> Result<(), String> { - let _write = lock_config_writes(&CONFIG_WRITE_LOCK)?; - let _revision = SettingsWriteRevision::begin(); update_file_config_at(&echo_core::config_path(), update)?; echo::settings::reload(); crate::status::health_invalidate(); @@ -92,33 +370,17 @@ struct SettingsEnv { whisper_acceleration: Option, } -pub(super) fn snapshot(readiness: Readiness) -> Result { +fn snapshot(readiness: Readiness) -> Result { let file = load_preferences_for_update(&echo_core::config_path())?; let preferences = read_from_file(&file)?; Ok(crate::speech::snapshot(preferences, &file, readiness)) } -pub(super) fn snapshot_with_revision( +fn snapshot_with_revision( mut readiness: impl FnMut() -> Readiness, ) -> Result<(u64, SettingsSnapshot), String> { - read_at_stable_revision(&SETTINGS_REVISION, || snapshot(readiness())) -} - -fn read_at_stable_revision( - revision_source: &AtomicU64, - mut read: impl FnMut() -> Result, -) -> Result<(u64, T), String> { - loop { - let revision = revision_source.load(Ordering::SeqCst); - if revision % 2 != 0 { - std::thread::yield_now(); - continue; - } - let value = read()?; - if revision == revision_source.load(Ordering::SeqCst) { - return Ok((revision, value)); - } - } + let snapshot = with_snapshot_revision(snapshot(readiness())?); + Ok((snapshot.revision, snapshot)) } fn read_from_file(file: &echo_core::Config) -> Result { @@ -132,7 +394,7 @@ fn read_from_file(file: &echo_core::Config) -> Result { settings_from(&process_settings_env(), file, language_default) } -pub(super) fn change(change: SettingsChange) -> Result<(), String> { +fn change(change: SettingsChange) -> Result<(), String> { if matches!(&change, SettingsChange::EnableWhisperGpu) { if let Some(variable) = whisper_gpu_environment_override(&process_settings_env()) { return Err(format!( @@ -143,6 +405,90 @@ pub(super) fn change(change: SettingsChange) -> Result<(), String> { update_file_config(|config| apply_change(config, change)) } +fn set_microphone_selection(id: Option) -> Result<(), String> { + if env::var("ECHO_MICROPHONE") + .ok() + .is_some_and(|value| !value.trim().is_empty()) + { + return Err("ECHO_MICROPHONE controls the microphone in this process".to_string()); + } + let snapshot = echo::audio::microphone_snapshot(); + let selection = match id { + None => None, + Some(raw) => { + let id = echo::microphone::MicrophoneId::parse(raw)?; + let device = snapshot + .devices + .iter() + .find(|device| device.id == id) + .ok_or_else(|| { + "that microphone is no longer connected; refresh and choose again".to_string() + })?; + Some((id, device.label.clone())) + } + }; + update_file_config(|config| { + update_microphone_config(config, selection); + Ok(()) + }) +} + +fn update_microphone_config( + config: &mut echo_core::Config, + selection: Option<(echo::microphone::MicrophoneId, String)>, +) { + config.microphone = + selection.map( + |(id, last_seen_label)| echo_core::MicrophoneSelection::Device { + id: id.as_str().to_string(), + last_seen_label, + }, + ); +} + +pub(crate) fn apply_plan_config( + config: &mut echo_core::Config, + plan_id: echo::install::SetupPlanId, +) -> Result<(), String> { + match plan_id { + echo::install::SetupPlanId::Parakeet => { + config.engine = Some(echo_core::EngineChoice::Parakeet); + config.whisper_model = None; + } + echo::install::SetupPlanId::Recommended + | echo::install::SetupPlanId::WhisperBase + | echo::install::SetupPlanId::WhisperSmall + | echo::install::SetupPlanId::WhisperLargeV3Turbo => { + config.engine = Some(echo_core::EngineChoice::Whisper); + let model = match plan_id { + echo::install::SetupPlanId::Recommended => { + echo::install::catalog::recommended_model() + } + echo::install::SetupPlanId::WhisperBase => { + echo::install::ComponentId::WhisperBaseQ51 + } + echo::install::SetupPlanId::WhisperSmall => { + echo::install::ComponentId::WhisperSmall + } + echo::install::SetupPlanId::WhisperLargeV3Turbo => { + echo::install::ComponentId::WhisperLargeV3TurboQ50 + } + echo::install::SetupPlanId::Parakeet => unreachable!(), + }; + config.whisper_model = Some( + match model { + echo::install::ComponentId::WhisperBaseQ51 => "base-q5_1", + echo::install::ComponentId::WhisperSmall => "small", + echo::install::ComponentId::WhisperLargeV3TurboQ50 => "large-v3-turbo-q5_0", + _ => return Err("invalid Whisper model plan".to_string()), + } + .to_string(), + ); + } + } + Ok(()) +} + fn whisper_gpu_environment_override(env: &SettingsEnv) -> Option<&'static str> { if env .engine @@ -410,7 +756,8 @@ fn nonempty(value: Option) -> Option { mod tests { use super::*; use echo_core::{Config, EngineChoice}; - use std::sync::atomic::{AtomicU64, Ordering}; + use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; + use std::sync::mpsc; fn scratch_path(label: &str) -> std::path::PathBuf { static SEQ: AtomicU64 = AtomicU64::new(0); @@ -425,24 +772,166 @@ mod tests { } #[test] - fn revisioned_read_retries_when_a_write_spans_the_snapshot() { - let revision = AtomicU64::new(0); - let mut reads = 0; - - let result = read_at_stable_revision(&revision, || { - reads += 1; - if reads == 1 { - revision.store(1, Ordering::SeqCst); - revision.store(2, Ordering::SeqCst); - Ok("stale") - } else { - Ok("fresh") - } - }) - .unwrap(); + fn config_owner_runs_delayed_jobs_in_enqueue_order() { + let owner = ConfigMutationService::default(); + let (started_sender, started_receiver) = mpsc::channel(); + let (release_sender, release_receiver) = mpsc::channel(); + let (order_sender, order_receiver) = mpsc::channel(); + let first_order = order_sender.clone(); + + owner + .enqueue(ConfigJob::fire_and_forget(move || { + started_sender.send(()).unwrap(); + release_receiver.recv().unwrap(); + first_order.send(1).unwrap(); + })) + .unwrap(); + started_receiver.recv().unwrap(); + owner + .enqueue(ConfigJob::fire_and_forget(move || { + order_sender.send(2).unwrap(); + })) + .unwrap(); + + release_sender.send(()).unwrap(); + + assert_eq!(order_receiver.recv().unwrap(), 1); + assert_eq!(order_receiver.recv().unwrap(), 2); + } + + #[test] + fn config_owner_continues_after_failed_job_result() { + let owner = ConfigMutationService::default(); + let (sender, receiver) = mpsc::channel(); + let first_sender = sender.clone(); + + owner + .enqueue(ConfigJob::fire_and_forget(move || { + first_sender + .send(Result::<(), &str>::Err("failed")) + .unwrap(); + })) + .unwrap(); + owner + .enqueue(ConfigJob::fire_and_forget(move || { + sender.send(Ok(())).unwrap(); + })) + .unwrap(); + + assert_eq!(receiver.recv().unwrap(), Err("failed")); + assert_eq!(receiver.recv().unwrap(), Ok(())); + } + + #[test] + fn config_owner_continues_after_panicked_job() { + let owner = ConfigMutationService::default(); + let (sender, receiver) = mpsc::channel(); + let healthy_sender = sender.clone(); + + owner + .enqueue(ConfigJob { + run: Box::new(move || panic!("boom")), + fail: Box::new(move |error| sender.send(error).unwrap()), + }) + .unwrap(); + owner + .enqueue(ConfigJob::fire_and_forget(move || { + healthy_sender.send("healthy".to_string()).unwrap(); + })) + .unwrap(); - assert_eq!(result, (2, "fresh")); - assert_eq!(reads, 2); + assert_eq!( + receiver.recv().unwrap(), + "configuration request failed unexpectedly" + ); + assert_eq!(receiver.recv().unwrap(), "healthy"); + } + + #[test] + fn dedicated_microphone_update_writes_id_and_clears_legacy_name() { + let mut config = echo_core::Config { + microphone: Some(echo_core::MicrophoneSelection::LegacyName { + name: "USB Mic".into(), + }), + ..echo_core::Config::default() + }; + update_microphone_config( + &mut config, + Some(( + echo::microphone::MicrophoneId::parse("alsa:usb-one").unwrap(), + "USB Mic".into(), + )), + ); + assert_eq!( + config.microphone, + Some(echo_core::MicrophoneSelection::Device { + id: "alsa:usb-one".into(), + last_seen_label: "USB Mic".into(), + }) + ); + update_microphone_config(&mut config, None); + assert_eq!(config.microphone, None); + } + + #[test] + fn setup_plan_activation_cancelled_while_queued_does_not_write_config() { + let owner = ConfigMutationService::default(); + let path = scratch_path("cancelled-queued-setup-activation"); + let cancel = Arc::new(AtomicBool::new(false)); + let (blocked_sender, blocked_receiver) = mpsc::channel(); + let (release_sender, release_receiver) = mpsc::channel(); + + owner + .enqueue(ConfigJob::fire_and_forget(move || { + blocked_sender.send(()).unwrap(); + release_receiver.recv().unwrap(); + })) + .unwrap(); + blocked_receiver.recv().unwrap(); + + let worker_owner = owner.clone(); + let worker_cancel = Arc::clone(&cancel); + let worker_path = path.clone(); + let activation = std::thread::spawn(move || { + worker_owner.apply_setup_plan_blocking_with( + echo::install::SetupPlanId::Parakeet, + worker_cancel, + move |plan_id| { + update_file_config_at(&worker_path, |config| apply_plan_config(config, plan_id)) + }, + ) + }); + + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(5); + while owner + .queue + .lock() + .map(|queue| queue.pending.is_empty()) + .unwrap_or(false) + { + assert!( + std::time::Instant::now() < deadline, + "activation was not queued" + ); + std::thread::yield_now(); + } + + cancel.store(true, Ordering::Relaxed); + release_sender.send(()).unwrap(); + + assert!(matches!( + activation.join().unwrap(), + Err(echo::install::InstallError::Cancelled) + )); + assert!(!path.exists()); + } + + #[test] + fn snapshot_response_revisions_are_unique_for_reads() { + let first = next_snapshot_response_revision(); + let second = next_snapshot_response_revision(); + + assert!(second > first); } #[test] @@ -548,24 +1037,6 @@ mod tests { ); } - #[test] - fn poisoned_config_write_protocol_returns_an_explicit_error() { - let lock = std::sync::Arc::new(Mutex::new(())); - let poison = std::sync::Arc::clone(&lock); - assert!(std::thread::spawn(move || { - let _guard = poison.lock().unwrap(); - panic!("poison config write protocol"); - }) - .join() - .is_err()); - - let error = lock_config_writes(&lock).unwrap_err(); - assert!( - error.contains("configuration write lock is poisoned"), - "{error}" - ); - } - #[test] fn corrupt_preferences_remain_byte_for_byte_unchanged_after_an_update() { let path = scratch_path("corrupt-update"); diff --git a/src-tauri/src/setup.rs b/src-tauri/src/setup.rs index 8f99728..66c4426 100644 --- a/src-tauri/src/setup.rs +++ b/src-tauri/src/setup.rs @@ -14,7 +14,7 @@ use echo_desktop::ipc::{ ActiveComponentOrigin, ComponentId, ComponentOrigin, ComponentStatus, ExternalComponent, InstallProgress, ManagedComponentState, Readiness, SetupEvent, SetupPlan, SetupPlanId, }; -use tauri::{Emitter, State}; +use tauri::{Emitter, Manager, State}; #[derive(Debug, Clone, PartialEq, Eq)] enum SetupAction { @@ -357,6 +357,10 @@ impl SetupService { disk: &disk, probe: &probe, }; + let config_service = app + .state::() + .inner() + .clone(); let mut last_progress_phase = None; let mut last_progress_emit = Instant::now() - Duration::from_secs(1); let mut emit_progress = |progress: CoreInstallProgress| { @@ -417,7 +421,8 @@ impl SetupService { if cancel.load(Ordering::Relaxed) { Err(echo::install::InstallError::Cancelled) } else { - activate_plan_config(plan_id) + config_service + .apply_setup_plan_blocking(plan_id, Arc::clone(&cancel)) } }) } @@ -469,46 +474,6 @@ impl SetupService { } } -fn activate_plan_config(plan_id: CoreSetupPlanId) -> Result<(), echo::install::InstallError> { - crate::settings::update_file_config(|config| apply_plan_config(config, plan_id)) - .map_err(echo::install::InstallError::IoMessage) -} - -fn apply_plan_config( - config: &mut echo_core::Config, - plan_id: CoreSetupPlanId, -) -> Result<(), String> { - match plan_id { - CoreSetupPlanId::Parakeet => { - config.engine = Some(echo_core::EngineChoice::Parakeet); - config.whisper_model = None; - } - CoreSetupPlanId::Recommended - | CoreSetupPlanId::WhisperBase - | CoreSetupPlanId::WhisperSmall - | CoreSetupPlanId::WhisperLargeV3Turbo => { - config.engine = Some(echo_core::EngineChoice::Whisper); - let model = match plan_id { - CoreSetupPlanId::Recommended => recommended_model(), - CoreSetupPlanId::WhisperBase => CoreComponentId::WhisperBaseQ51, - CoreSetupPlanId::WhisperSmall => CoreComponentId::WhisperSmall, - CoreSetupPlanId::WhisperLargeV3Turbo => CoreComponentId::WhisperLargeV3TurboQ50, - CoreSetupPlanId::Parakeet => unreachable!(), - }; - config.whisper_model = Some( - match model { - CoreComponentId::WhisperBaseQ51 => "base-q5_1", - CoreComponentId::WhisperSmall => "small", - CoreComponentId::WhisperLargeV3TurboQ50 => "large-v3-turbo-q5_0", - _ => return Err("invalid Whisper model plan".to_string()), - } - .to_string(), - ); - } - } - Ok(()) -} - #[tauri::command] pub async fn get_readiness(state: State<'_, SetupService>) -> Result { let service = state.inner().clone(); @@ -574,10 +539,8 @@ pub fn cancel_setup(operation: String, state: State<'_, SetupService>) -> bool { #[cfg(test)] mod tests { - use super::{ - apply_plan_config, get_readiness, lock_active_operation, plan_space, Readiness, - SetupService, - }; + use super::{get_readiness, lock_active_operation, plan_space, Readiness, SetupService}; + use crate::settings::apply_plan_config; use echo::install::SetupPlanId; use echo_core::{Config, EngineChoice}; use std::future::Future; diff --git a/src-tauri/src/speech.rs b/src-tauri/src/speech.rs index 555c992..da73ed8 100644 --- a/src-tauri/src/speech.rs +++ b/src-tauri/src/speech.rs @@ -59,6 +59,7 @@ pub(crate) fn snapshot( }; let whisper = whisper_applicability(&preferences, &next_run, &readiness); SettingsSnapshot { + revision: 0, preferences, transcription: TranscriptionSnapshot { next_run, diff --git a/src-tauri/src/tray.rs b/src-tauri/src/tray.rs index 2fbabc7..4bea27c 100644 --- a/src-tauri/src/tray.rs +++ b/src-tauri/src/tray.rs @@ -1,19 +1,17 @@ -use std::collections::{HashSet, VecDeque}; +use std::collections::HashSet; use std::sync::atomic::{AtomicU64, Ordering}; use std::sync::Mutex; -use echo_desktop::ipc::{ - LanguageMode, LanguageOption, SettingSource, SettingsChange, SettingsSnapshot, -}; +use echo_desktop::ipc::{LanguageMode, LanguageOption, SettingSource, SettingsSnapshot}; use tauri::image::Image; use tauri::menu::{CheckMenuItem, IsMenuItem, Menu, MenuItem, PredefinedMenuItem, Submenu}; use tauri::tray::TrayIconBuilder; -use tauri::{App, AppHandle, Emitter, Manager, Wry}; +use tauri::{App, AppHandle, Manager, Wry}; static NEXT_LANGUAGE_REQUEST: AtomicU64 = AtomicU64::new(0); #[derive(Clone, Copy, Debug, PartialEq, Eq)] -pub(crate) struct LanguageMenuRequest(u64); +pub(crate) struct LanguageMenuRequest(pub(crate) u64); pub(crate) struct TrayMenu { // Tauri's Linux AppIndicator backend requires the menu owner to outlive setup. @@ -21,7 +19,6 @@ pub(crate) struct TrayMenu { language_menu: Submenu, language_items: Vec<(String, CheckMenuItem)>, language_state: Mutex, - language_writes: Mutex, } pub(crate) fn build(app: &mut App) -> tauri::Result { @@ -75,7 +72,6 @@ pub(crate) fn build(app: &mut App) -> tauri::Result { revision: LanguageMenuRevision::default(), projection: None, }), - language_writes: Mutex::new(LanguageWriteQueue::default()), }; TrayIconBuilder::new() .menu(&tray_menu._menu) @@ -132,41 +128,10 @@ struct LanguageMenuState { projection: Option, } -struct LanguageWrite { - request: LanguageMenuRequest, - value: String, -} - -#[derive(Default)] -struct LanguageWriteQueue { - active: bool, - pending: VecDeque, -} - -impl LanguageWriteQueue { - fn enqueue(&mut self, write: LanguageWrite) -> bool { - self.pending.push_back(write); - if self.active { - false - } else { - self.active = true; - true - } - } - - fn next(&mut self) -> Option { - let next = self.pending.pop_front(); - if next.is_none() { - self.active = false; - } - next - } -} - #[derive(Clone, Copy, Default, PartialEq, Eq, PartialOrd, Ord)] -struct LanguageMenuRevision { - settings: u64, - request: u64, +pub(crate) struct LanguageMenuRevision { + pub(crate) settings: u64, + pub(crate) request: u64, } impl LanguageMenuState { @@ -275,20 +240,6 @@ impl TrayMenu { } } } - - fn enqueue_language_write(&self, write: LanguageWrite) -> Result { - self.language_writes - .lock() - .map(|mut queue| queue.enqueue(write)) - .map_err(|_| "tray language write queue is unavailable".to_string()) - } - - fn next_language_write(&self) -> Result, String> { - self.language_writes - .lock() - .map(|mut queue| queue.next()) - .map_err(|_| "tray language write queue is unavailable".to_string()) - } } pub(crate) fn request() -> LanguageMenuRequest { @@ -311,7 +262,11 @@ pub(crate) fn sync( ); } -fn sync_requested(app: &AppHandle, revision: LanguageMenuRevision, snapshot: &SettingsSnapshot) { +pub(crate) fn sync_requested( + app: &AppHandle, + revision: LanguageMenuRevision, + snapshot: &SettingsSnapshot, +) { let projection = language_menu_projection( snapshot.transcription.languages.mode, &snapshot.transcription.languages.options, @@ -337,26 +292,12 @@ pub(crate) fn refresh(app: &AppHandle) { } pub(crate) fn refresh_requested(app: &AppHandle, request: LanguageMenuRequest) { - let app = app.clone(); - let service = app.state::().inner().clone(); - tauri::async_runtime::spawn(async move { - let result = crate::blocking::run_blocking("tray language refresh", move || { - crate::settings::snapshot_with_revision(|| service.snapshot()) - }) - .await - .and_then(|result| result); - match result { - Ok((settings, snapshot)) => sync_requested( - &app, - LanguageMenuRevision { - settings, - request: request.0, - }, - &snapshot, - ), - Err(error) => eprintln!("tray language: failed to read settings: {error}"), - } - }); + let Some(owner) = app.try_state::() else { + return; + }; + if let Err(error) = owner.request_tray_refresh(app.clone(), request) { + eprintln!("tray language: failed to queue refresh: {error}"); + } } fn language_event_value(id: &str) -> Option { @@ -376,58 +317,17 @@ fn select_language(app: &AppHandle, value: String) { return; } let request = request(); - let start_worker = match menu.enqueue_language_write(LanguageWrite { request, value }) { - Ok(start_worker) => start_worker, - Err(error) => { - eprintln!("tray language: {error}"); - restore(app); - return; - } + let Some(owner) = app.try_state::() else { + restore(app); + return; }; - if start_worker { - tauri::async_runtime::spawn(process_language_writes(app.clone())); - } -} - -async fn process_language_writes(app: AppHandle) { - loop { - let write = { - let Some(menu) = app.try_state::() else { - return; - }; - match menu.next_language_write() { - Ok(write) => write, - Err(error) => { - eprintln!("tray language: {error}"); - return; - } - } - }; - let Some(write) = write else { - return; - }; - let service = app.state::().inner().clone(); - let value = write.value; - let outcome = crate::blocking::run_blocking("tray language change", move || { - crate::settings::change(SettingsChange::Language { value: Some(value) })?; - crate::settings::snapshot_with_revision(|| service.snapshot()) - }) - .await - .and_then(|result| result); - match outcome { - Ok((revision, snapshot)) => { - sync(&app, write.request, revision, &snapshot); - let _ = app.emit("settings-event", ()); - } - Err(error) => { - eprintln!("tray language: {error}"); - restore(&app); - } - } + if let Err(error) = owner.request_tray_language(value, app.clone(), request) { + eprintln!("tray language: {error}"); + restore(app); } } -fn restore(app: &AppHandle) { +pub(crate) fn restore(app: &AppHandle) { let app_for_update = app.clone(); if let Err(error) = app.run_on_main_thread(move || { if let Some(menu) = app_for_update.try_state::() { @@ -490,35 +390,6 @@ mod tests { assert!(state.projection.unwrap().item_checked("de")); } - #[test] - fn language_writes_are_dequeued_in_click_order() { - let mut queue = LanguageWriteQueue::default(); - assert!(queue.enqueue(LanguageWrite { - request: LanguageMenuRequest(1), - value: "fr".to_string(), - })); - assert!(!queue.enqueue(LanguageWrite { - request: LanguageMenuRequest(2), - value: "de".to_string(), - })); - - let first = queue.next().unwrap(); - let second = queue.next().unwrap(); - assert_eq!( - (first.request, first.value.as_str()), - (LanguageMenuRequest(1), "fr") - ); - assert_eq!( - (second.request, second.value.as_str()), - (LanguageMenuRequest(2), "de") - ); - assert!(queue.next().is_none()); - assert!(queue.enqueue(LanguageWrite { - request: LanguageMenuRequest(3), - value: "es".to_string(), - })); - } - #[test] fn multilingual_projection_selects_and_enables_available_languages() { let projection = language_menu_projection( diff --git a/src-tauri/tests/desktop_entry.rs b/src-tauri/tests/desktop_entry.rs index c5dc472..c788909 100644 --- a/src-tauri/tests/desktop_entry.rs +++ b/src-tauri/tests/desktop_entry.rs @@ -83,19 +83,16 @@ fn tray_menu_is_retained_in_managed_state() { fn settings_changes_publish_an_ordered_tray_snapshot() { let settings_command = include_str!("../src/commands/settings.rs"); let setup_runtime = include_str!("../src/setup.rs"); + let settings_owner = include_str!("../src/settings.rs"); let tray_runtime = include_str!("../src/tray.rs"); - assert!(settings_command.contains("crate::settings::snapshot_with_revision")); let request = settings_command .find("let tray_request = crate::tray::request();") - .expect("Settings reserves a tray update before detached work"); - let detached_work = settings_command - .find("crate::blocking::run_blocking(\"settings change\"") - .expect("Settings change uses detached work"); - assert!(request < detached_work); - assert!( - settings_command.contains("crate::tray::sync(&app, tray_request, revision, &snapshot);") - ); + .expect("Settings reserves a tray update before enqueueing work"); + let enqueue = settings_command + .find("owner.request_settings_change(") + .expect("Settings change enters the config owner"); + assert!(request < enqueue); let setup_completion = setup_runtime .find("let mut active = lock_active_operation(&worker_state);") .expect("setup completion clears its active operation"); @@ -108,17 +105,20 @@ fn settings_changes_publish_an_ordered_tray_snapshot() { .expect("setup releases its active-operation lock"); assert!(setup_request < setup_unlock); assert!(setup_completion.contains("crate::tray::refresh_requested(&app, tray_request);")); + assert!(settings_owner.contains("struct ConfigJobQueue")); + assert!(settings_owner.contains("snapshot_with_revision")); + assert!(settings_owner.contains("crate::tray::sync(&app, tray_request, revision, &snapshot);")); assert!(tray_runtime.contains("app.run_on_main_thread(move ||")); - assert!(tray_runtime.contains("LanguageWriteQueue")); + assert!(tray_runtime.contains("owner.request_tray_language(value, app.clone(), request)")); assert!(tray_runtime.contains("language_menu.set_enabled(true)")); } #[test] fn tray_language_changes_notify_the_settings_ui() { - let tray_runtime = include_str!("../src/tray.rs"); + let settings_owner = include_str!("../src/settings.rs"); let settings_controller = include_str!("../../frontend/src/settings/useSettingsController.ts"); - assert!(tray_runtime.contains("app.emit(\"settings-event\", ())")); + assert!(settings_owner.contains("app.emit(\"settings-event\", ())")); assert!(settings_controller.contains("onSettingsEvent")); }