From aed50e6d997814b4fab43b6b9c09e48722269b02 Mon Sep 17 00:00:00 2001 From: Chris Scott <99081550+chriswritescode-dev@users.noreply.github.com> Date: Fri, 9 Oct 2026 22:16:04 -0400 Subject: [PATCH 1/6] fix(tts): dedupe concurrent synthesis and write the audio cache atomically - Write cached audio to a temp file and rename it into place so a concurrent read never sees a partial or interleaved file. - Share one upstream synthesis call per text and cache key, so concurrent requests no longer produce different recordings that race on the same file. - Return the same upstream error to every waiter. --- backend/src/routes/tts.ts | 157 ++++++++++++++++++++++---------- backend/test/routes/tts.test.ts | 93 ++++++++++++++++++- 2 files changed, 199 insertions(+), 51 deletions(-) diff --git a/backend/src/routes/tts.ts b/backend/src/routes/tts.ts index 5f463ca9e..febda773a 100644 --- a/backend/src/routes/tts.ts +++ b/backend/src/routes/tts.ts @@ -1,8 +1,8 @@ import { Hono } from 'hono' import { z } from 'zod' import { Database } from 'bun:sqlite' -import { createHash } from 'crypto' -import { readFile, writeFile, readdir, stat, unlink } from 'fs/promises' +import { createHash, randomUUID } from 'crypto' +import { readFile, writeFile, readdir, stat, unlink, rename } from 'fs/promises' import { join } from 'path' import { SettingsService } from '../services/settings' import { logger } from '../utils/logger' @@ -107,7 +107,96 @@ async function cacheAudio(cacheKey: string, audioData: Buffer): Promise { await cleanupOldestFiles(audioData.length) } - await writeFile(filePath, audioData) + const tempPath = join(TTS_CACHE_DIR, `${cacheKey}.${process.pid}.${randomUUID()}.tmp`) + + try { + await writeFile(tempPath, audioData) + await rename(tempPath, filePath) + } catch (error) { + await unlink(tempPath).catch(() => undefined) + throw error + } +} + +interface SynthesisParams { + text: string + endpoint: string + apiKey: string + voice: string + model: string + speed: number +} + +class TTSUpstreamError extends Error { + readonly status: 400 | 500 + readonly details: string + + constructor(status: 400 | 500, details: string) { + super('TTS API request failed') + this.name = 'TTSUpstreamError' + this.status = status + this.details = details + } +} + +const inFlightSynthesis = new Map>() + +function parseErrorDetails(errorText: string): string { + try { + const errorJson = JSON.parse(errorText) + if (errorJson.detail?.error?.message) return errorJson.detail.error.message + if (errorJson.detail?.message) return errorJson.detail.message + if (errorJson.message) return errorJson.message + } catch { + return errorText + } + return errorText +} + +async function fetchSynthesizedAudio(params: SynthesisParams): Promise { + const { text, endpoint, apiKey, voice, model, speed } = params + const baseUrl = normalizeToBaseUrl(endpoint) + const speechEndpoint = `${baseUrl}/v1/audio/speech` + + const response = await fetch(speechEndpoint, { + method: 'POST', + headers: { + 'Authorization': `Bearer ${apiKey}`, + 'Content-Type': 'application/json', + }, + body: JSON.stringify({ + model, + voice, + input: text, + speed, + response_format: 'mp3', + }), + }) + + if (!response.ok) { + const errorText = await response.text() + logger.error(`TTS API error: ${response.status} - ${errorText}`) + const status = response.status >= 400 && response.status < 600 ? response.status as 400 | 500 : 500 + throw new TTSUpstreamError(status, parseErrorDetails(errorText)) + } + + return Buffer.from(await response.arrayBuffer()) +} + +function getOrStartSynthesis(cacheKey: string, params: SynthesisParams): Promise { + const existing = inFlightSynthesis.get(cacheKey) + if (existing) return existing + + const promise = (async () => { + const audioBuffer = await fetchSynthesizedAudio(params) + await cacheAudio(cacheKey, audioBuffer) + return audioBuffer + })() + + inFlightSynthesis.set(cacheKey, promise) + const cleanup = () => inFlightSynthesis.delete(cacheKey) + promise.finally(cleanup).catch(() => undefined) + return promise } @@ -272,56 +361,28 @@ export function createTTSRoutes(db: Database) { logger.info(`TTS cache miss, calling API: ${cacheKey.substring(0, 8)}...`) - const baseUrl = normalizeToBaseUrl(endpoint) - const speechEndpoint = `${baseUrl}/v1/audio/speech` - - const response = await fetch(speechEndpoint, { - method: 'POST', - headers: { - 'Authorization': `Bearer ${apiKey}`, - 'Content-Type': 'application/json', - }, - body: JSON.stringify({ - model, + let audioBuffer: Buffer + try { + audioBuffer = await getOrStartSynthesis(cacheKey, { + text, + endpoint, + apiKey, voice, - input: text, + model, speed, - response_format: 'mp3', - }), - signal: abortController.signal, - }) - - if (!response.ok) { - const errorText = await response.text() - logger.error(`TTS API error: ${response.status} - ${errorText}`) - const status = response.status >= 400 && response.status < 600 ? response.status as 400 | 500 : 500 - - // Try to parse error details for better frontend display - let errorDetails = errorText - try { - const errorJson = JSON.parse(errorText) - if (errorJson.detail?.error?.message) { - errorDetails = errorJson.detail.error.message - } else if (errorJson.detail?.message) { - errorDetails = errorJson.detail.message - } else if (errorJson.message) { - errorDetails = errorJson.message - } - } catch { - // Use raw error text if parsing fails + }) + } catch (error) { + if (error instanceof TTSUpstreamError) { + return c.json({ + error: 'TTS API request failed', + details: error.details, + voice: voice, + availableVoices: ttsConfig?.availableVoices || [] + }, error.status) } - - return c.json({ - error: 'TTS API request failed', - details: errorDetails, - voice: voice, - availableVoices: ttsConfig?.availableVoices || [] - }, status) + throw error } - const audioBuffer = Buffer.from(await response.arrayBuffer()) - - await cacheAudio(cacheKey, audioBuffer) logger.info(`TTS audio cached: ${cacheKey.substring(0, 8)}...`) return new Response(audioBuffer, { diff --git a/backend/test/routes/tts.test.ts b/backend/test/routes/tts.test.ts index 4faf8afd3..da2df8130 100644 --- a/backend/test/routes/tts.test.ts +++ b/backend/test/routes/tts.test.ts @@ -7,6 +7,7 @@ vi.mock('fs/promises', () => ({ readdir: vi.fn(), stat: vi.fn(), unlink: vi.fn(), + rename: vi.fn(), })) vi.mock('../../src/utils/fs-safe', () => ({ @@ -57,6 +58,7 @@ const mockUnlink = fs.unlink as any import { createTTSRoutes, cleanupExpiredCache, getCacheStats, generateCacheKey, ensureCacheDir, getCachedAudio, getCacheSize, cleanupOldestFiles, cacheAudio } from '../../src/routes/tts' const mockWriteFile = fs.writeFile as any +const mockRename = (fs as any).rename as any function createTtsConfig(overrides: Record = {}) { return { @@ -316,6 +318,9 @@ describe('TTS route handlers', () => { it('synthesizes and caches audio on a cache miss', async () => { mockStat.mockRejectedValue(new Error('not found')) + mockReaddir.mockResolvedValue([] as any) + mockWriteFile.mockResolvedValue(undefined) + mockRename.mockResolvedValue(undefined) const fetchMock = vi.fn().mockResolvedValue({ ok: true, arrayBuffer: async () => Buffer.from('fresh-audio'), @@ -339,9 +344,53 @@ describe('TTS route handlers', () => { }), ) expect(mockWriteFile).toHaveBeenCalledWith( - expect.stringContaining('.mp3'), + expect.stringMatching(/\.tmp$/), expect.any(Buffer), ) + expect(mockRename).toHaveBeenCalledWith( + expect.stringMatching(/\.tmp$/), + expect.stringContaining('.mp3'), + ) + }) + + it('deduplicates concurrent synthesis requests for the same text', async () => { + mockStat.mockRejectedValue(new Error('not found')) + mockReaddir.mockResolvedValue([] as any) + mockWriteFile.mockResolvedValue(undefined) + mockRename.mockResolvedValue(undefined) + + let resolveFetch: ((value: { ok: boolean; arrayBuffer: () => Promise }) => void) | undefined + const fetchMock = vi.fn().mockImplementation( + () => new Promise((resolve) => { + resolveFetch = resolve + }), + ) + vi.stubGlobal('fetch', fetchMock) + + const request = () => app.request('/synthesize', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ text: 'Hello' }), + }) + + const first = request() + const second = request() + + await new Promise((resolve) => setTimeout(resolve, 0)) + expect(fetchMock).toHaveBeenCalledTimes(1) + + resolveFetch!({ + ok: true, + arrayBuffer: async () => Buffer.from('shared-audio'), + }) + + const [firstRes, secondRes] = await Promise.all([first, second]) + + expect(fetchMock).toHaveBeenCalledTimes(1) + expect(firstRes.status).toBe(200) + expect(secondRes.status).toBe(200) + expect(Buffer.from(await firstRes.arrayBuffer()).toString()).toBe('shared-audio') + expect(Buffer.from(await secondRes.arrayBuffer()).toString()).toBe('shared-audio') }) it('returns the upstream error details when synthesis fails', async () => { @@ -463,27 +512,65 @@ describe('cacheAudio', () => { mockStat.mockResolvedValue({ size: 200 * 1024 * 1024, mtimeMs: 1000 } as any) mockUnlink.mockResolvedValue(undefined) mockWriteFile.mockResolvedValue(undefined) + mockRename.mockResolvedValue(undefined) await cacheAudio('cache-key', Buffer.from('audio')) expect(mockUnlink).toHaveBeenCalledWith(expect.stringContaining('big.mp3')) expect(mockWriteFile).toHaveBeenCalledWith( - expect.stringContaining('cache-key.mp3'), + expect.stringMatching(/\.tmp$/), expect.any(Buffer), ) + expect(mockRename).toHaveBeenCalledWith( + expect.stringMatching(/\.tmp$/), + expect.stringContaining('cache-key.mp3'), + ) }) it('writes audio without cleanup when the cache has room', async () => { mockReaddir.mockResolvedValue(['small.mp3'] as any) mockStat.mockResolvedValue({ size: 1024, mtimeMs: 1000 } as any) mockWriteFile.mockResolvedValue(undefined) + mockRename.mockResolvedValue(undefined) await cacheAudio('cache-key', Buffer.from('audio')) expect(mockUnlink).not.toHaveBeenCalled() expect(mockWriteFile).toHaveBeenCalledWith( - expect.stringContaining('cache-key.mp3'), + expect.stringMatching(/\.tmp$/), expect.any(Buffer), ) + expect(mockRename).toHaveBeenCalledWith( + expect.stringMatching(/\.tmp$/), + expect.stringContaining('cache-key.mp3'), + ) + }) + + it('writes to a unique temp path and renames it to the final .mp3 path', async () => { + mockReaddir.mockResolvedValue([] as any) + mockWriteFile.mockResolvedValue(undefined) + mockRename.mockResolvedValue(undefined) + + await cacheAudio('cache-key', Buffer.from('audio')) + + const [writtenPath] = mockWriteFile.mock.calls[0] + const [renamedFrom, renamedTo] = mockRename.mock.calls[0] + + expect(writtenPath).toContain('cache-key.') + expect(writtenPath).toMatch(/\.tmp$/) + expect(writtenPath).not.toContain('.mp3') + expect(renamedFrom).toBe(writtenPath) + expect(renamedTo).toContain('cache-key.mp3') + }) + + it('removes the temp file and rethrows when the rename fails', async () => { + mockReaddir.mockResolvedValue([] as any) + mockWriteFile.mockResolvedValue(undefined) + mockRename.mockRejectedValue(new Error('rename failed')) + mockUnlink.mockResolvedValue(undefined) + + await expect(cacheAudio('cache-key', Buffer.from('audio'))).rejects.toThrow('rename failed') + + expect(mockUnlink).toHaveBeenCalledWith(expect.stringMatching(/\.tmp$/)) }) }) From 1bfcdd0496cc89bd094bbe19aa6c7efc0121be98 Mon Sep 17 00:00:00 2001 From: Chris Scott <99081550+chriswritescode-dev@users.noreply.github.com> Date: Fri, 9 Oct 2026 22:16:04 -0400 Subject: [PATCH 2/6] fix(tts): isolate playback and fetch each chunk once - Tag each playback and ignore stale async continuations and audio events, so a previous message's chunk chain cannot play over the current one. - Cancel browser speech regardless of the saved provider. - Fetch each chunk at most once per playback and await an in-flight fetch instead of starting a duplicate. --- frontend/src/contexts/TTSContext.test.tsx | 296 ++++++++++++++++++++++ frontend/src/contexts/TTSContext.tsx | 179 ++++++------- 2 files changed, 388 insertions(+), 87 deletions(-) create mode 100644 frontend/src/contexts/TTSContext.test.tsx diff --git a/frontend/src/contexts/TTSContext.test.tsx b/frontend/src/contexts/TTSContext.test.tsx new file mode 100644 index 000000000..8c3a78d5b --- /dev/null +++ b/frontend/src/contexts/TTSContext.test.tsx @@ -0,0 +1,296 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' +import { render, act, waitFor } from '@testing-library/react' +import { TTSProvider } from './TTSContext' +import { useTTS } from '@/hooks/useTTS' +import type { TTSConfig, TTSContextValue } from './tts-context' +import type { WebSpeechSynthesizer } from '@/lib/webSpeechSynthesizer' + +const mocks = vi.hoisted(() => ({ + useSettings: vi.fn(), +})) + +const webSpeech = vi.hoisted(() => ({ + stop: vi.fn(), + clearCallbacks: vi.fn(), + waitForVoices: vi.fn(() => Promise.resolve()), + onEnd: vi.fn(), + onError: vi.fn(), + speakChunked: vi.fn(() => Promise.resolve()), +})) + +vi.mock('@/hooks/useSettings', () => ({ + useSettings: mocks.useSettings, +})) + +vi.mock('@/lib/webSpeechSynthesizer', () => ({ + getWebSpeechSynthesizer: () => webSpeech as unknown as WebSpeechSynthesizer, + isWebSpeechSupported: () => true, +})) + +class FakeAudio { + static instances: FakeAudio[] = [] + + src: string + paused = true + onended: (() => void) | null = null + onerror: (() => void) | null = null + + constructor(src: string) { + this.src = src + FakeAudio.instances.push(this) + } + + play = vi.fn(() => { + this.paused = false + return Promise.resolve() + }) + + pause = vi.fn(() => { + this.paused = true + }) + + load = vi.fn() + + removeAttribute = vi.fn((name: string) => { + if (name === 'src') this.src = '' + }) +} + +const ttsRef: { current: TTSContextValue | null } = { current: null } + +function Consumer() { + ttsRef.current = useTTS() + return null +} + +let fetchBodies: string[] = [] +let objectUrlCounter = 0 + +const createObjectURL = vi.fn(() => `blob:fake-${++objectUrlCounter}`) +const revokeObjectURL = vi.fn() + +const textA = 'Alpha one. Alpha two. Alpha three. Alpha four. Alpha five.' +const textB = 'Bravo one. Bravo two. Bravo three. Bravo four. Bravo five.' + +function setPreferences(overrides: Partial = {}) { + mocks.useSettings.mockReturnValue({ + preferences: { + tts: { + enabled: true, + autoPlay: false, + provider: 'external', + endpoint: 'https://api.openai.com', + apiKey: 'test-key', + voice: 'alloy', + model: 'tts-1', + speed: 1.0, + ...overrides, + }, + }, + updateSettings: vi.fn(), + }) +} + +const externalConfig: TTSConfig = { + enabled: true, + provider: 'external', + endpoint: 'https://api.openai.com', + apiKey: 'test-key', + voice: 'alloy', + model: 'tts-1', + speed: 1.0, +} + +const builtinConfig: TTSConfig = { + enabled: true, + provider: 'builtin', + endpoint: '', + apiKey: '', + voice: '', + model: '', + speed: 1.0, +} + +function renderProvider() { + return render( + + + , + ) +} + +beforeEach(() => { + vi.clearAllMocks() + FakeAudio.instances = [] + fetchBodies = [] + objectUrlCounter = 0 + ttsRef.current = null + + const fetchMock = vi.fn(async (_input: RequestInfo | URL, init?: RequestInit) => { + const body = JSON.parse(String(init?.body ?? '{}')) as { text: string } + fetchBodies.push(body.text) + const blob = new Blob([`audio:${body.text}`], { type: 'audio/mpeg' }) + return { + ok: true, + status: 200, + headers: { + get: (name: string) => (name.toLowerCase() === 'content-type' ? 'audio/mpeg' : null), + }, + blob: async () => blob, + json: async () => ({}), + } as unknown as Response + }) + + vi.stubGlobal('Audio', FakeAudio) + vi.stubGlobal('fetch', fetchMock) + URL.createObjectURL = createObjectURL + URL.revokeObjectURL = revokeObjectURL +}) + +afterEach(() => { + vi.unstubAllGlobals() +}) + +describe('TTSProvider playback isolation', () => { + it('does not start a concurrent chain when a stale onended fires after a new playback begins', async () => { + setPreferences() + renderProvider() + + await act(async () => { + await ttsRef.current!.speakMessage('A', textA) + }) + + await waitFor(() => expect(FakeAudio.instances.length).toBeGreaterThanOrEqual(1)) + const firstAudio = FakeAudio.instances[0] + await waitFor(() => expect(firstAudio.paused).toBe(false)) + const staleOnended = firstAudio.onended + expect(staleOnended).toBeTypeOf('function') + + await act(async () => { + await ttsRef.current!.speakMessage('B', textB) + }) + + await waitFor(() => expect(FakeAudio.instances.length).toBeGreaterThanOrEqual(2)) + const secondAudio = FakeAudio.instances[1] + await waitFor(() => expect(secondAudio.paused).toBe(false)) + await waitFor(() => expect(fetchBodies).toContain('Bravo three. Bravo four.')) + + expect(firstAudio.paused).toBe(true) + + await act(async () => { + staleOnended!() + await new Promise((resolve) => setTimeout(resolve, 0)) + }) + + const playing = FakeAudio.instances.filter((audio) => !audio.paused) + expect(playing).toHaveLength(1) + expect(playing[0]).toBe(secondAudio) + expect(FakeAudio.instances).toHaveLength(2) + }) + + it('ignores a stale onerror after cleanup without orphaning the new audio or corrupting state', async () => { + setPreferences() + renderProvider() + + await act(async () => { + await ttsRef.current!.speakMessage('A', textA) + }) + + await waitFor(() => expect(FakeAudio.instances.length).toBeGreaterThanOrEqual(1)) + const firstAudio = FakeAudio.instances[0] + await waitFor(() => expect(firstAudio.paused).toBe(false)) + const staleOnerror = firstAudio.onerror + expect(staleOnerror).toBeTypeOf('function') + + await act(async () => { + await ttsRef.current!.speakMessage('B', textB) + }) + + await waitFor(() => expect(FakeAudio.instances.length).toBeGreaterThanOrEqual(2)) + const secondAudio = FakeAudio.instances[1] + await waitFor(() => expect(secondAudio.paused).toBe(false)) + + await act(async () => { + staleOnerror!() + }) + + expect(ttsRef.current!.state).toBe('playing') + expect(ttsRef.current!.error).toBeNull() + expect(secondAudio.paused).toBe(false) + }) + + it('cancels active Web Speech playback when an external playback starts', async () => { + setPreferences({ provider: 'external' }) + renderProvider() + + await act(async () => { + await ttsRef.current!.speakWithConfig('Hello from web speech.', builtinConfig) + }) + + expect(webSpeech.speakChunked).toHaveBeenCalled() + webSpeech.stop.mockClear() + + await act(async () => { + await ttsRef.current!.speakWithConfig('Now external audio.', externalConfig) + }) + + expect(webSpeech.stop).toHaveBeenCalled() + }) + + it('fetches each chunk at most once when playback outruns an in-flight prefetch', async () => { + setPreferences() + renderProvider() + + const slowChunkText = 'Alpha three. Alpha four.' + let releaseSlowChunk: () => void = () => {} + const slowChunkGate = new Promise((resolve) => { + releaseSlowChunk = resolve + }) + + const baseFetch = globalThis.fetch + vi.stubGlobal( + 'fetch', + vi.fn(async (input: RequestInfo | URL, init?: RequestInit) => { + const body = JSON.parse(String(init?.body ?? '{}')) as { text: string } + if (body.text === slowChunkText) { + await slowChunkGate + } + return baseFetch(input, init) + }), + ) + + await act(async () => { + await ttsRef.current!.speakMessage('A', textA) + }) + + await waitFor(() => expect(FakeAudio.instances.length).toBeGreaterThanOrEqual(1)) + const firstAudio = FakeAudio.instances[0] + await waitFor(() => expect(firstAudio.paused).toBe(false)) + await waitFor(() => expect(fetchBodies).toContain('Alpha one. Alpha two.')) + expect(fetchBodies).not.toContain(slowChunkText) + + await act(async () => { + firstAudio.onended!() + await new Promise((resolve) => setTimeout(resolve, 0)) + }) + + await act(async () => { + releaseSlowChunk() + await new Promise((resolve) => setTimeout(resolve, 0)) + }) + + await waitFor(() => expect(FakeAudio.instances.length).toBeGreaterThanOrEqual(2)) + const secondAudio = FakeAudio.instances[1] + await waitFor(() => expect(secondAudio.paused).toBe(false)) + + expect(fetchBodies.filter((text) => text === slowChunkText)).toHaveLength(1) + + const fetchCounts = new Map() + for (const text of fetchBodies) { + fetchCounts.set(text, (fetchCounts.get(text) ?? 0) + 1) + } + for (const count of fetchCounts.values()) { + expect(count).toBe(1) + } + }) +}) diff --git a/frontend/src/contexts/TTSContext.tsx b/frontend/src/contexts/TTSContext.tsx index d5d814fd4..31dd42a94 100644 --- a/frontend/src/contexts/TTSContext.tsx +++ b/frontend/src/contexts/TTSContext.tsx @@ -37,11 +37,11 @@ export function TTSProvider({ children }: TTSProviderProps) { const abortControllerRef = useRef(null) const audioRef = useRef(null) - const stoppedRef = useRef(false) + const playbackIdRef = useRef(0) + const currentObjectUrlRef = useRef(null) const chunksRef = useRef([]) const chunkIndexRef = useRef(0) - const prefetchedBlobsRef = useRef>(new Map()) - const fetchingIndexRef = useRef(-1) + const chunkAudioRef = useRef>>(new Map()) // Web Speech API reference const webSpeechSynthRef = useRef | null>(null) @@ -66,34 +66,36 @@ export function TTSProvider({ children }: TTSProviderProps) { }, []); const cleanup = useCallback(() => { - // Stop external audio if (audioRef.current) { + audioRef.current.onended = null + audioRef.current.onerror = null audioRef.current.pause() - audioRef.current.src = '' + audioRef.current.removeAttribute('src') + audioRef.current.load() audioRef.current = null } - - // Stop Web Speech API - if (webSpeechSynthRef.current && isBuiltin) { + + if (currentObjectUrlRef.current) { + URL.revokeObjectURL(currentObjectUrlRef.current) + currentObjectUrlRef.current = null + } + + if (webSpeechSynthRef.current) { webSpeechSynthRef.current.stop() webSpeechSynthRef.current.clearCallbacks() } - + if (abortControllerRef.current) { abortControllerRef.current.abort() abortControllerRef.current = null } - prefetchedBlobsRef.current.forEach((_, key) => { - prefetchedBlobsRef.current.delete(key) - }) - prefetchedBlobsRef.current.clear() + chunkAudioRef.current.clear() chunksRef.current = [] chunkIndexRef.current = 0 - fetchingIndexRef.current = -1 - }, [isBuiltin]) + }, []) const stop = useCallback(() => { - stoppedRef.current = true + playbackIdRef.current += 1 cleanup() setState('idle') setCurrentText(null) @@ -104,14 +106,14 @@ export function TTSProvider({ children }: TTSProviderProps) { useEffect(() => { return () => { - stoppedRef.current = true + playbackIdRef.current += 1 cleanup() } }, [cleanup]) // External API synthesis - const synthesizeExternal = useCallback(async (text: string, signal?: AbortSignal): Promise => { - if (stoppedRef.current) return null + const synthesizeExternal = useCallback(async (text: string, signal: AbortSignal | undefined, playbackId: number): Promise => { + if (playbackIdRef.current !== playbackId) return null try { const response = await fetch(`${API_BASE_URL}/api/tts/synthesize`, { @@ -121,7 +123,7 @@ export function TTSProvider({ children }: TTSProviderProps) { signal, }) - if (stoppedRef.current) return null + if (playbackIdRef.current !== playbackId) return null if (!response.ok) { let errorMessage = 'TTS request failed' @@ -155,32 +157,31 @@ export function TTSProvider({ children }: TTSProviderProps) { } }, []) - const fetchNextChunk = useCallback(async (index: number) => { - if (stoppedRef.current) return - if (index >= chunksRef.current.length) return - if (prefetchedBlobsRef.current.has(index)) { - fetchNextChunk(index + 1) - return - } - - fetchingIndexRef.current = index - - try { - const blob = await synthesizeExternal(chunksRef.current[index], abortControllerRef.current?.signal) - if (blob && !stoppedRef.current) { - prefetchedBlobsRef.current.set(index, blob) - fetchNextChunk(index + 1) - } - } catch { - if (stoppedRef.current) return - } - - fetchingIndexRef.current = -1 + const getChunkAudio = useCallback((index: number, playbackId: number): Promise => { + const existing = chunkAudioRef.current.get(index) + if (existing) return existing + + const promise = synthesizeExternal(chunksRef.current[index], abortControllerRef.current?.signal, playbackId) + chunkAudioRef.current.set(index, promise) + return promise }, [synthesizeExternal]) - const playChunk = useCallback(async (index: number) => { - if (stoppedRef.current || index >= chunksRef.current.length) { - if (!stoppedRef.current) { + const prefetchChunk = useCallback((index: number, playbackId: number) => { + if (playbackIdRef.current !== playbackId) return + if (index >= chunksRef.current.length) return + if (chunkAudioRef.current.has(index)) return + + getChunkAudio(index, playbackId) + .then((blob) => { + if (!blob || playbackIdRef.current !== playbackId) return + prefetchChunk(index + 1, playbackId) + }) + .catch(() => {}) + }, [getChunkAudio]) + + const playChunk = useCallback(async (index: number, playbackId: number) => { + if (playbackIdRef.current !== playbackId || index >= chunksRef.current.length) { + if (playbackIdRef.current === playbackId) { setState('idle') setCurrentText(null) setActiveMessageId(null) @@ -191,50 +192,54 @@ export function TTSProvider({ children }: TTSProviderProps) { chunkIndexRef.current = index try { - let blob: Blob | undefined = prefetchedBlobsRef.current.get(index) - - if (!blob) { + if (!chunkAudioRef.current.has(index)) { setState('loading') - const fetched = await synthesizeExternal(chunksRef.current[index], abortControllerRef.current?.signal) - if (fetched && !stoppedRef.current) { - blob = fetched - } - fetchNextChunk(index + 1) } - - if (!blob || stoppedRef.current) return - prefetchedBlobsRef.current.delete(index) - + const blob = await getChunkAudio(index, playbackId) + if (!blob || playbackIdRef.current !== playbackId) return + + chunkAudioRef.current.delete(index) + prefetchChunk(index + 1, playbackId) + const url = URL.createObjectURL(blob) + currentObjectUrlRef.current = url const audio = new Audio(url) audioRef.current = audio audio.onended = () => { - URL.revokeObjectURL(url) - audioRef.current = null - if (!stoppedRef.current) { - playChunk(index + 1) + if (playbackIdRef.current !== playbackId) return + if (currentObjectUrlRef.current === url) { + URL.revokeObjectURL(url) + currentObjectUrlRef.current = null + } + if (audioRef.current === audio) { + audioRef.current = null } + playChunk(index + 1, playbackId) } audio.onerror = () => { - URL.revokeObjectURL(url) - audioRef.current = null - if (!stoppedRef.current) { - setError('Audio playback failed') - setState('error') + if (playbackIdRef.current !== playbackId) return + if (currentObjectUrlRef.current === url) { + URL.revokeObjectURL(url) + currentObjectUrlRef.current = null } + if (audioRef.current === audio) { + audioRef.current = null + } + setError('Audio playback failed') + setState('error') } setState('playing') await audio.play() } catch (err) { - if (stoppedRef.current) return + if (playbackIdRef.current !== playbackId) return setError(err instanceof Error ? err.message : 'TTS failed') setState('error') } - }, [synthesizeExternal, fetchNextChunk]) + }, [getChunkAudio, prefetchChunk]) // Builtin Web Speech synthesis - takes explicit config and optional messageId const speakBuiltinWithConfig = useCallback(async (text: string, config: TTSConfig, messageId: string | null = null): Promise => { @@ -259,7 +264,7 @@ export function TTSProvider({ children }: TTSProviderProps) { } stop() - stoppedRef.current = false + const playbackId = playbackIdRef.current setError(null) setOriginalText(text) @@ -274,38 +279,38 @@ export function TTSProvider({ children }: TTSProviderProps) { const synth = getSynthesizer() await synth.waitForVoices() + if (playbackIdRef.current !== playbackId) return false + const voiceName = config.voice || '' - + synth.clearCallbacks() - + synth.onEnd(() => { - if (!stoppedRef.current) { - setState('idle') - setCurrentText(null) - setActiveMessageId(null) - } + if (playbackIdRef.current !== playbackId) return + setState('idle') + setCurrentText(null) + setActiveMessageId(null) }) synth.onError((err) => { - if (!stoppedRef.current) { - setError(err) - setState('error') - } + if (playbackIdRef.current !== playbackId) return + setError(err) + setState('error') }) try { setState('playing') - + const rate = config.speed || 1.0 - + await synth.speakChunked(sanitizedText, 200, { voice: voiceName || undefined, rate: rate, }) - + return true } catch (err) { - if (stoppedRef.current) return false + if (playbackIdRef.current !== playbackId) return false setError(err instanceof Error ? err.message : 'TTS failed') setState('error') return false @@ -315,7 +320,7 @@ export function TTSProvider({ children }: TTSProviderProps) { // Internal helper to start external TTS playback const startExternalPlayback = useCallback((sanitizedText: string, original: string, messageId: string | null): boolean => { stop() - stoppedRef.current = false + const playbackId = playbackIdRef.current setError(null) setOriginalText(original) @@ -328,9 +333,9 @@ export function TTSProvider({ children }: TTSProviderProps) { abortControllerRef.current = new AbortController() chunksRef.current = splitIntoChunks(sanitizedText) - - playChunk(0) - + + playChunk(0, playbackId) + return true }, [stop, playChunk]) From 1eba33c6f1db28f4e8a2df9452e948ac16cc3273 Mon Sep 17 00:00:00 2001 From: Chris Scott <99081550+chriswritescode-dev@users.noreply.github.com> Date: Fri, 9 Oct 2026 23:57:09 -0400 Subject: [PATCH 3/6] fix(tts): bound synthesis requests and tolerate cache write failures --- backend/src/routes/tts.ts | 59 ++++++++++++++++++++------------- backend/test/routes/tts.test.ts | 42 +++++++++++++++++++++++ 2 files changed, 78 insertions(+), 23 deletions(-) diff --git a/backend/src/routes/tts.ts b/backend/src/routes/tts.ts index febda773a..18e4ff199 100644 --- a/backend/src/routes/tts.ts +++ b/backend/src/routes/tts.ts @@ -18,6 +18,7 @@ const TTS_CACHE_DIR = join(getWorkspacePath(), 'cache', 'tts') const CACHE_TTL_MS = 24 * 60 * 60 * 1000 const MAX_CACHE_SIZE_MB = 200 const MAX_CACHE_SIZE_BYTES = MAX_CACHE_SIZE_MB * 1024 * 1024 +const TTS_SYNTHESIS_TIMEOUT_MS = 60_000 const TTSRequestSchema = z.object({ text: z.string().min(1).max(4096), @@ -158,29 +159,37 @@ async function fetchSynthesizedAudio(params: SynthesisParams): Promise { const baseUrl = normalizeToBaseUrl(endpoint) const speechEndpoint = `${baseUrl}/v1/audio/speech` - const response = await fetch(speechEndpoint, { - method: 'POST', - headers: { - 'Authorization': `Bearer ${apiKey}`, - 'Content-Type': 'application/json', - }, - body: JSON.stringify({ - model, - voice, - input: text, - speed, - response_format: 'mp3', - }), - }) - - if (!response.ok) { - const errorText = await response.text() - logger.error(`TTS API error: ${response.status} - ${errorText}`) - const status = response.status >= 400 && response.status < 600 ? response.status as 400 | 500 : 500 - throw new TTSUpstreamError(status, parseErrorDetails(errorText)) + try { + const response = await fetch(speechEndpoint, { + method: 'POST', + headers: { + 'Authorization': `Bearer ${apiKey}`, + 'Content-Type': 'application/json', + }, + body: JSON.stringify({ + model, + voice, + input: text, + speed, + response_format: 'mp3', + }), + signal: AbortSignal.timeout(TTS_SYNTHESIS_TIMEOUT_MS), + }) + + if (!response.ok) { + const errorText = await response.text() + logger.error(`TTS API error: ${response.status} - ${errorText}`) + const status = response.status >= 400 && response.status < 600 ? response.status as 400 | 500 : 500 + throw new TTSUpstreamError(status, parseErrorDetails(errorText)) + } + + return Buffer.from(await response.arrayBuffer()) + } catch (error) { + if (error instanceof Error && error.name === 'TimeoutError') { + throw new TTSUpstreamError(500, 'TTS upstream request timed out') + } + throw error } - - return Buffer.from(await response.arrayBuffer()) } function getOrStartSynthesis(cacheKey: string, params: SynthesisParams): Promise { @@ -189,7 +198,11 @@ function getOrStartSynthesis(cacheKey: string, params: SynthesisParams): Promise const promise = (async () => { const audioBuffer = await fetchSynthesizedAudio(params) - await cacheAudio(cacheKey, audioBuffer) + try { + await cacheAudio(cacheKey, audioBuffer) + } catch (error) { + logger.error('TTS cache write failed:', error) + } return audioBuffer })() diff --git a/backend/test/routes/tts.test.ts b/backend/test/routes/tts.test.ts index da2df8130..b7b5f8f5a 100644 --- a/backend/test/routes/tts.test.ts +++ b/backend/test/routes/tts.test.ts @@ -341,6 +341,7 @@ describe('TTS route handlers', () => { expect.objectContaining({ method: 'POST', headers: expect.objectContaining({ Authorization: 'Bearer test-api-key' }), + signal: expect.any(AbortSignal), }), ) expect(mockWriteFile).toHaveBeenCalledWith( @@ -353,6 +354,28 @@ describe('TTS route handlers', () => { ) }) + it('returns synthesized audio when the cache write fails', async () => { + mockStat.mockRejectedValue(new Error('not found')) + mockReaddir.mockResolvedValue([] as any) + mockWriteFile.mockRejectedValue(new Error('disk full')) + mockUnlink.mockResolvedValue(undefined) + const fetchMock = vi.fn().mockResolvedValue({ + ok: true, + arrayBuffer: async () => Buffer.from('fresh-audio'), + }) + vi.stubGlobal('fetch', fetchMock) + + const res = await app.request('/synthesize', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ text: 'Hello' }), + }) + + expect(res.status).toBe(200) + expect(res.headers.get('X-Cache')).toBe('MISS') + expect(Buffer.from(await res.arrayBuffer()).toString()).toBe('fresh-audio') + }) + it('deduplicates concurrent synthesis requests for the same text', async () => { mockStat.mockRejectedValue(new Error('not found')) mockReaddir.mockResolvedValue([] as any) @@ -416,6 +439,25 @@ describe('TTS route handlers', () => { }) }) + it('returns an upstream error when synthesis times out', async () => { + mockStat.mockRejectedValue(new Error('not found')) + vi.stubGlobal('fetch', vi.fn().mockRejectedValue(new DOMException('The operation timed out.', 'TimeoutError'))) + + const res = await app.request('/synthesize', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ text: 'Hello' }), + }) + + expect(res.status).toBe(500) + expect(await res.json()).toEqual({ + error: 'TTS API request failed', + details: 'TTS upstream request timed out', + voice: 'alloy', + availableVoices: [], + }) + }) + it('lists models and stores them when not cached', async () => { mockDiscoverModelsCached.mockResolvedValue({ models: ['tts-1', 'tts-1-hd'], cached: false }) From 169ee25a8a95d4e9df0023c7f0b2c230cf5ee375 Mon Sep 17 00:00:00 2001 From: Chris Scott <99081550+chriswritescode-dev@users.noreply.github.com> Date: Sat, 10 Oct 2026 14:54:53 -0400 Subject: [PATCH 4/6] fix(tts): key the audio cache by provider and add a settings clear action - Hash text, voice, model, speed, normalized endpoint and API key into one SHA-256 key shared by the disk cache and in-flight synthesis, so different providers cannot reuse each other's audio and old keys are not read as fallback. - Add DELETE /api/tts/cache to remove completed cached audio files, keep temporary files, tolerate missing files and report real failures. - Add a Clear audio cache action to TTS settings, available even when TTS is disabled or the builtin provider is selected, with pending, success and retryable error states. --- backend/src/routes/tts.ts | 78 +++++-- backend/test/routes/tts.test.ts | 220 ++++++++++++++++-- frontend/src/api/tts.ts | 10 + .../components/settings/TTSSettings.test.tsx | 181 ++++++++++++++ .../src/components/settings/TTSSettings.tsx | 57 ++++- 5 files changed, 511 insertions(+), 35 deletions(-) create mode 100644 frontend/src/components/settings/TTSSettings.test.tsx diff --git a/backend/src/routes/tts.ts b/backend/src/routes/tts.ts index 18e4ff199..27df38b3f 100644 --- a/backend/src/routes/tts.ts +++ b/backend/src/routes/tts.ts @@ -24,9 +24,25 @@ const TTSRequestSchema = z.object({ text: z.string().min(1).max(4096), }) -function generateCacheKey(text: string, voice: string, model: string, speed: number): string { +interface SynthesisParams { + text: string + endpoint: string + apiKey: string + voice: string + model: string + speed: number +} + +function generateCacheKey(params: SynthesisParams): string { const hash = createHash('sha256') - hash.update(`${text}|${voice}|${model}|${speed}`) + hash.update(JSON.stringify([ + params.text, + params.voice, + params.model, + params.speed, + normalizeToBaseUrl(params.endpoint), + params.apiKey, + ])) return hash.digest('hex') } @@ -119,15 +135,6 @@ async function cacheAudio(cacheKey: string, audioData: Buffer): Promise { } } -interface SynthesisParams { - text: string - endpoint: string - apiKey: string - voice: string - model: string - speed: number -} - class TTSUpstreamError extends Error { readonly status: 400 | 500 readonly details: string @@ -322,7 +329,32 @@ export async function getCacheStats(): Promise<{ count: number; sizeBytes: numbe } } -export { generateCacheKey, ensureCacheDir, getCachedAudio, cacheAudio, getCacheSize, cleanupOldestFiles } +async function clearCache(): Promise { + let files: string[] + try { + files = await readdir(TTS_CACHE_DIR) + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') return 0 + throw error + } + + let cleared = 0 + for (const file of files) { + if (!file.endsWith('.mp3')) continue + + try { + await unlink(join(TTS_CACHE_DIR, file)) + cleared++ + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') continue + throw error + } + } + + return cleared +} + +export { generateCacheKey, ensureCacheDir, getCachedAudio, cacheAudio, getCacheSize, cleanupOldestFiles, clearCache } export function createTTSRoutes(db: Database) { const app = new Hono() @@ -353,7 +385,8 @@ export function createTTSRoutes(db: Database) { } const { endpoint, apiKey, voice, model, speed } = ttsConfig - const cacheKey = generateCacheKey(text, voice, model, speed) + const synthesisParams: SynthesisParams = { text, endpoint, apiKey, voice, model, speed } + const cacheKey = generateCacheKey(synthesisParams) await ensureCacheDir() @@ -376,14 +409,7 @@ export function createTTSRoutes(db: Database) { let audioBuffer: Buffer try { - audioBuffer = await getOrStartSynthesis(cacheKey, { - text, - endpoint, - apiKey, - voice, - model, - speed, - }) + audioBuffer = await getOrStartSynthesis(cacheKey, synthesisParams) } catch (error) { if (error instanceof TTSUpstreamError) { return c.json({ @@ -511,5 +537,15 @@ export function createTTSRoutes(db: Database) { }) }) + app.delete('/cache', async (c) => { + try { + const cleared = await clearCache() + return c.json({ cleared }) + } catch (error) { + logger.error('TTS cache clear failed:', error) + return c.json({ error: 'Failed to clear TTS cache' }, 500) + } + }) + return app } diff --git a/backend/test/routes/tts.test.ts b/backend/test/routes/tts.test.ts index b7b5f8f5a..8a0bcedd1 100644 --- a/backend/test/routes/tts.test.ts +++ b/backend/test/routes/tts.test.ts @@ -55,7 +55,7 @@ const mockReaddir = fs.readdir as any const mockStat = fs.stat as any const mockUnlink = fs.unlink as any -import { createTTSRoutes, cleanupExpiredCache, getCacheStats, generateCacheKey, ensureCacheDir, getCachedAudio, getCacheSize, cleanupOldestFiles, cacheAudio } from '../../src/routes/tts' +import { createTTSRoutes, cleanupExpiredCache, getCacheStats, generateCacheKey, ensureCacheDir, getCachedAudio, getCacheSize, cleanupOldestFiles, cacheAudio, clearCache } from '../../src/routes/tts' const mockWriteFile = fs.writeFile as any const mockRename = (fs as any).rename as any @@ -91,27 +91,112 @@ describe('TTS Routes', () => { }) describe('generateCacheKey', () => { + const params = { + text: 'Hello world', + endpoint: 'https://tts.example.com', + apiKey: 'test-api-key', + voice: 'alloy', + model: 'tts-1', + speed: 1.0, + } + it('should generate consistent cache keys for identical inputs', () => { - const text = 'Hello world' - const voice = 'alloy' - const model = 'tts-1' - const speed = 1.0 - - const key1 = generateCacheKey(text, voice, model, speed) - const key2 = generateCacheKey(text, voice, model, speed) - + const key1 = generateCacheKey(params) + const key2 = generateCacheKey({ ...params }) + expect(key1).toBe(key2) expect(key1).toMatch(/^[a-f0-9]{64}$/) }) - it('should generate different cache keys for different inputs', () => { - const key1 = generateCacheKey('Hello', 'alloy', 'tts-1', 1.0) - const key2 = generateCacheKey('World', 'alloy', 'tts-1', 1.0) - + it('should generate different cache keys for different text', () => { + expect(generateCacheKey(params)).not.toBe(generateCacheKey({ ...params, text: 'Other' })) + }) + + it('should generate different cache keys for different voices', () => { + expect(generateCacheKey(params)).not.toBe(generateCacheKey({ ...params, voice: 'echo' })) + }) + + it('should generate different cache keys for different models', () => { + expect(generateCacheKey(params)).not.toBe(generateCacheKey({ ...params, model: 'tts-1-hd' })) + }) + + it('should generate different cache keys for different speeds', () => { + expect(generateCacheKey(params)).not.toBe(generateCacheKey({ ...params, speed: 1.5 })) + }) + + it('should generate different cache keys for different api keys', () => { + expect(generateCacheKey(params)).not.toBe(generateCacheKey({ ...params, apiKey: 'other-api-key' })) + }) + + it('should generate different cache keys for different endpoints', () => { + expect(generateCacheKey(params)).not.toBe(generateCacheKey({ ...params, endpoint: 'https://other.example.com' })) + }) + + it('should normalize endpoints so trailing slashes share a key', () => { + expect(generateCacheKey(params)).toBe(generateCacheKey({ ...params, endpoint: 'https://tts.example.com/' })) + }) + + it('should avoid delimiter ambiguity between concatenated fields', () => { + const key1 = generateCacheKey({ ...params, text: 'a', voice: 'b|c' }) + const key2 = generateCacheKey({ ...params, text: 'a|b', voice: 'c' }) + expect(key1).not.toBe(key2) }) }) + describe('clearCache', () => { + it('should delete every .mp3 file and count them', async () => { + mockReaddir.mockResolvedValue(['a.mp3', 'b.mp3'] as any) + mockUnlink.mockResolvedValue(undefined) + + const cleared = await clearCache() + + expect(cleared).toBe(2) + expect(mockUnlink).toHaveBeenCalledTimes(2) + expect(mockUnlink).toHaveBeenCalledWith(expect.stringContaining('a.mp3')) + expect(mockUnlink).toHaveBeenCalledWith(expect.stringContaining('b.mp3')) + }) + + it('should tolerate a missing cache directory', async () => { + mockReaddir.mockRejectedValue(Object.assign(new Error('missing'), { code: 'ENOENT' })) + + const cleared = await clearCache() + + expect(cleared).toBe(0) + expect(mockUnlink).not.toHaveBeenCalled() + }) + + it('should tolerate a file removed by a race and keep clearing the rest', async () => { + mockReaddir.mockResolvedValue(['gone.mp3', 'present.mp3'] as any) + mockUnlink + .mockRejectedValueOnce(Object.assign(new Error('gone'), { code: 'ENOENT' })) + .mockResolvedValueOnce(undefined) + + const cleared = await clearCache() + + expect(cleared).toBe(1) + expect(mockUnlink).toHaveBeenCalledTimes(2) + }) + + it('should throw unexpected filesystem errors', async () => { + mockReaddir.mockResolvedValue(['a.mp3'] as any) + mockUnlink.mockRejectedValue(Object.assign(new Error('denied'), { code: 'EACCES' })) + + await expect(clearCache()).rejects.toThrow('denied') + }) + + it('should leave .tmp and unrelated files untouched', async () => { + mockReaddir.mockResolvedValue(['a.mp3', 'pending.tmp', 'notes.txt'] as any) + mockUnlink.mockResolvedValue(undefined) + + const cleared = await clearCache() + + expect(cleared).toBe(1) + expect(mockUnlink).toHaveBeenCalledTimes(1) + expect(mockUnlink).toHaveBeenCalledWith(expect.stringContaining('a.mp3')) + }) + }) + describe('ensureCacheDir', () => { it('should create cache directory when it does not exist', async () => { await expect(ensureCacheDir()).resolves.toBeUndefined() @@ -416,6 +501,85 @@ describe('TTS route handlers', () => { expect(Buffer.from(await secondRes.arrayBuffer()).toString()).toBe('shared-audio') }) + it('does not share an in-flight synthesis across different endpoints or API keys', async () => { + mockStat.mockRejectedValue(new Error('not found')) + mockReaddir.mockResolvedValue([] as any) + mockWriteFile.mockResolvedValue(undefined) + mockRename.mockResolvedValue(undefined) + mockGetSettings.mockImplementation((userId: string) => ({ + preferences: { + tts: createTtsConfig( + userId === 'first' + ? { endpoint: 'https://first.example.com', apiKey: 'first-key' } + : { endpoint: 'https://second.example.com', apiKey: 'second-key' }, + ), + }, + })) + + const resolvers: Array<(value: { ok: boolean; arrayBuffer: () => Promise }) => void> = [] + const fetchMock = vi.fn().mockImplementation( + () => new Promise((resolve) => { + resolvers.push(resolve) + }), + ) + vi.stubGlobal('fetch', fetchMock) + + const request = (userId: string) => app.request('/synthesize?userId=' + userId, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ text: 'Isolated text' }), + }) + + const first = request('first') + const second = request('second') + + await new Promise((resolve) => setTimeout(resolve, 0)) + expect(fetchMock).toHaveBeenCalledTimes(2) + + resolvers[0]!({ ok: true, arrayBuffer: async () => Buffer.from('first-audio') }) + resolvers[1]!({ ok: true, arrayBuffer: async () => Buffer.from('second-audio') }) + + const [firstRes, secondRes] = await Promise.all([first, second]) + + expect(firstRes.status).toBe(200) + expect(secondRes.status).toBe(200) + expect(Buffer.from(await firstRes.arrayBuffer()).toString()).toBe('first-audio') + expect(Buffer.from(await secondRes.arrayBuffer()).toString()).toBe('second-audio') + }) + + it('isolates the disk cache across providers for identical text', async () => { + mockStat.mockRejectedValue(new Error('not found')) + mockReaddir.mockResolvedValue([] as any) + mockWriteFile.mockResolvedValue(undefined) + mockRename.mockResolvedValue(undefined) + mockGetSettings.mockImplementation((userId: string) => ({ + preferences: { + tts: createTtsConfig( + userId === 'first' + ? { endpoint: 'https://first.example.com', apiKey: 'first-key' } + : { endpoint: 'https://second.example.com', apiKey: 'second-key' }, + ), + }, + })) + vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ + ok: true, + arrayBuffer: async () => Buffer.from('audio'), + })) + + const request = (userId: string) => app.request('/synthesize?userId=' + userId, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ text: 'Shared text' }), + }) + + await request('first') + await request('second') + + const statPaths = mockStat.mock.calls.map((call: unknown[]) => call[0] as string) + expect(statPaths).toHaveLength(2) + expect(statPaths[0]).not.toBe(statPaths[1]) + }) + it('returns the upstream error details when synthesis fails', async () => { mockStat.mockRejectedValue(new Error('not found')) vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ @@ -542,6 +706,36 @@ describe('TTS route handlers', () => { cache: { count: 1, sizeBytes: 1024, sizeMB: 0, maxSizeMB: 200, ttlHours: 24 }, }) }) + + it('clears the cache through the DELETE /cache route', async () => { + mockReaddir.mockResolvedValue(['a.mp3', 'b.mp3', 'pending.tmp'] as any) + mockUnlink.mockResolvedValue(undefined) + + const res = await app.request('/cache', { method: 'DELETE' }) + + expect(res.status).toBe(200) + expect(await res.json()).toEqual({ cleared: 2 }) + expect(mockUnlink).toHaveBeenCalledTimes(2) + }) + + it('returns zero when the cache directory is missing', async () => { + mockReaddir.mockRejectedValue(Object.assign(new Error('missing'), { code: 'ENOENT' })) + + const res = await app.request('/cache', { method: 'DELETE' }) + + expect(res.status).toBe(200) + expect(await res.json()).toEqual({ cleared: 0 }) + }) + + it('returns 500 when clearing the cache fails unexpectedly', async () => { + mockReaddir.mockResolvedValue(['a.mp3'] as any) + mockUnlink.mockRejectedValue(Object.assign(new Error('denied'), { code: 'EACCES' })) + + const res = await app.request('/cache', { method: 'DELETE' }) + + expect(res.status).toBe(500) + expect(await res.json()).toEqual({ error: 'Failed to clear TTS cache' }) + }) }) describe('cacheAudio', () => { diff --git a/frontend/src/api/tts.ts b/frontend/src/api/tts.ts index 4535a2289..b40dafdbf 100644 --- a/frontend/src/api/tts.ts +++ b/frontend/src/api/tts.ts @@ -11,6 +11,10 @@ export interface TTSVoicesResponse { cached: boolean } +export interface TTSClearCacheResponse { + cleared: number +} + export interface TTSStatusResponse { enabled: boolean configured: boolean @@ -42,6 +46,12 @@ export const ttsApi = { }) }, + clearCache: async (): Promise => { + return fetchWrapper(`${API_BASE_URL}/api/tts/cache`, { + method: 'DELETE', + }) + }, + synthesize: async (text: string, userId = 'default', signal?: AbortSignal): Promise => { return fetchWrapperBlob(`${API_BASE_URL}/api/tts/synthesize`, { method: 'POST', diff --git a/frontend/src/components/settings/TTSSettings.test.tsx b/frontend/src/components/settings/TTSSettings.test.tsx new file mode 100644 index 000000000..c11dea124 --- /dev/null +++ b/frontend/src/components/settings/TTSSettings.test.tsx @@ -0,0 +1,181 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' +import { render, screen } from '@testing-library/react' +import userEvent from '@testing-library/user-event' +import { QueryClient, QueryClientProvider } from '@tanstack/react-query' +import type { ReactNode } from 'react' +import { TTSSettings } from './TTSSettings' +import { useSettings } from '@/hooks/useSettings' +import { useTTS } from '@/hooks/useTTS' +import { useTTSModels, useTTSVoices, useTTSDiscovery } from '@/hooks/useTTSDiscovery' +import { getAvailableVoiceNames, isWebSpeechSupported } from '@/lib/webSpeechSynthesizer' +import { createUseSettingsMock } from '@/test/test-utils' +import type { UserPreferences } from '@/api/types/settings' + +vi.mock('@/hooks/useSettings') +vi.mock('@/hooks/useTTS') +vi.mock('@/hooks/useTTSDiscovery') +vi.mock('@/lib/webSpeechSynthesizer') + +const fetchMock = vi.fn() + +const baseTts: NonNullable = { + enabled: true, + provider: 'external', + autoPlay: false, + endpoint: 'https://api.openai.com', + apiKey: 'sk-test', + voice: 'alloy', + model: 'tts-1', + speed: 1, + availableVoices: [], + availableModels: [], + lastVoicesFetch: 0, + lastModelsFetch: 0, +} + +function buildPreferences(overrides: Partial = {}): UserPreferences { + return { + theme: 'dark', + mode: 'build', + autoScroll: true, + expandDiffs: true, + expandToolCalls: false, + showReasoning: false, + simpleChatMode: false, + keyboardShortcuts: {}, + customCommands: [], + tts: baseTts, + ...overrides, + } +} + +function jsonResponse(body: unknown, status = 200): Response { + return new Response(JSON.stringify(body), { + status, + headers: { 'Content-Type': 'application/json' }, + }) +} + +function renderTTSSettings() { + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false }, mutations: { retry: false } }, + }) + function Wrapper({ children }: { children: ReactNode }) { + return {children} + } + return render(, { wrapper: Wrapper }) +} + +function mockHooks(preferences: UserPreferences = buildPreferences()) { + vi.mocked(useSettings).mockReturnValue(createUseSettingsMock({ preferences })) + vi.mocked(useTTS).mockReturnValue({ + speak: vi.fn(), + speakWithConfig: vi.fn(), + speakMessage: vi.fn(), + stop: vi.fn(), + state: 'idle', + error: null, + currentText: null, + originalText: null, + activeMessageId: null, + isEnabled: true, + isPlaying: false, + isLoading: false, + isIdle: true, + }) + vi.mocked(useTTSDiscovery).mockReturnValue({ + refreshModels: vi.fn(), + refreshVoices: vi.fn(), + refreshAll: vi.fn(), + invalidateModels: vi.fn(), + invalidateVoices: vi.fn(), + invalidateAll: vi.fn(), + }) + vi.mocked(useTTSModels).mockReturnValue({ + data: undefined, + isLoading: false, + refetch: vi.fn(), + } as unknown as ReturnType) + vi.mocked(useTTSVoices).mockReturnValue({ + data: undefined, + isLoading: false, + refetch: vi.fn(), + } as unknown as ReturnType) + vi.mocked(isWebSpeechSupported).mockReturnValue(true) + vi.mocked(getAvailableVoiceNames).mockResolvedValue(['Alex']) +} + +describe('TTSSettings audio cache', () => { + beforeEach(() => { + vi.clearAllMocks() + vi.stubGlobal('fetch', fetchMock) + mockHooks() + }) + + afterEach(() => { + vi.unstubAllGlobals() + }) + + it('clears the server cache via DELETE /api/tts/cache and reports the count', async () => { + const user = userEvent.setup() + fetchMock.mockResolvedValueOnce(jsonResponse({ cleared: 3 })) + renderTTSSettings() + + await user.click(screen.getByRole('button', { name: 'Clear audio cache' })) + + expect(await screen.findByRole('status')).toHaveTextContent('Cleared 3 cached audio file(s).') + const [url, options] = fetchMock.mock.calls[0] as [string, RequestInit] + expect(String(url)).toContain('/api/tts/cache') + expect(options).toMatchObject({ method: 'DELETE' }) + }) + + it('disables the button and shows a pending label while clearing', async () => { + const user = userEvent.setup() + let resolveResponse: (response: Response) => void = () => {} + fetchMock.mockReturnValueOnce( + new Promise((resolve) => { + resolveResponse = resolve + }), + ) + renderTTSSettings() + + await user.click(screen.getByRole('button', { name: 'Clear audio cache' })) + + const pendingButton = screen.getByRole('button', { name: /clearing/i }) + expect(pendingButton).toBeDisabled() + + resolveResponse(jsonResponse({ cleared: 1 })) + expect(await screen.findByRole('status')).toHaveTextContent('Cleared 1 cached audio file(s).') + }) + + it('shows an actionable error and allows retrying after a failure', async () => { + const user = userEvent.setup() + fetchMock + .mockResolvedValueOnce(jsonResponse({ error: 'cache unavailable' }, 500)) + .mockResolvedValueOnce(jsonResponse({ cleared: 2 })) + renderTTSSettings() + + await user.click(screen.getByRole('button', { name: 'Clear audio cache' })) + + expect(await screen.findByRole('alert')).toHaveTextContent('cache unavailable') + const retryButton = screen.getByRole('button', { name: 'Clear audio cache' }) + expect(retryButton).toBeEnabled() + + await user.click(retryButton) + + expect(await screen.findByRole('status')).toHaveTextContent('Cleared 2 cached audio file(s).') + expect(fetchMock).toHaveBeenCalledTimes(2) + }) + + it('stays available when TTS is disabled or the builtin provider is selected', async () => { + mockHooks(buildPreferences({ tts: { ...baseTts, enabled: false } })) + const disabledRender = renderTTSSettings() + expect(screen.getByRole('button', { name: 'Clear audio cache' })).toBeInTheDocument() + disabledRender.unmount() + + mockHooks(buildPreferences({ tts: { ...baseTts, provider: 'builtin', voice: 'Alex' } })) + renderTTSSettings() + expect(screen.getByRole('button', { name: 'Clear audio cache' })).toBeInTheDocument() + expect(await screen.findByText('1 voices available in your browser')).toBeInTheDocument() + }) +}) diff --git a/frontend/src/components/settings/TTSSettings.tsx b/frontend/src/components/settings/TTSSettings.tsx index 2dc6d09fa..8b7fc98c9 100644 --- a/frontend/src/components/settings/TTSSettings.tsx +++ b/frontend/src/components/settings/TTSSettings.tsx @@ -1,12 +1,13 @@ import { useEffect, useState, useRef } from 'react' import { useForm } from 'react-hook-form' +import { useMutation } from '@tanstack/react-query' import { zodResolver } from '@hookform/resolvers/zod' import { z } from 'zod' import { useSettings } from '@/hooks/useSettings' import { useTTS } from '@/hooks/useTTS' import { useTTSModels, useTTSVoices, useTTSDiscovery } from '@/hooks/useTTSDiscovery' import { getAvailableVoiceNames, isWebSpeechSupported } from '@/lib/webSpeechSynthesizer' -import { Loader2, Volume2, XCircle, RefreshCw, MonitorSpeaker, Globe, CheckCircle2 } from 'lucide-react' +import { Loader2, Volume2, XCircle, RefreshCw, MonitorSpeaker, Globe, CheckCircle2, Trash2 } from 'lucide-react' import { Button } from '@/components/ui/button' import { SquareFill } from '@/components/ui/square-fill' import { Input } from '@/components/ui/input' @@ -14,6 +15,7 @@ import { Switch } from '@/components/ui/switch' import { Slider } from '@/components/ui/slider' import { Form, FormControl, FormDescription, FormField, FormItem, FormLabel, FormMessage } from '@/components/ui/form' import { Combobox } from '@/components/ui/combobox' +import { ttsApi } from '@/api/tts' import { DEFAULT_TTS_CONFIG } from '@/api/types/settings' const TEST_PHRASE = 'Text to speech is working correctly.' @@ -240,6 +242,15 @@ export function TTSSettings() { stop() } + const clearCacheMutation = useMutation({ + mutationFn: () => ttsApi.clearCache(), + }) + + const clearCacheError = + clearCacheMutation.error instanceof Error + ? clearCacheMutation.error.message + : 'Unable to clear the audio cache' + return (
@@ -681,6 +692,50 @@ export function TTSSettings() {
)} + +
+
+
Clear audio cache
+

+ Removes all cached server TTS audio. Audio is regenerated on next use, and active playback or synthesis may repopulate the cache. +

+

+ Does not affect model or voice discovery. +

+ {clearCacheMutation.isSuccess && ( +

+ + Cleared {clearCacheMutation.data?.cleared ?? 0} cached audio file(s). +

+ )} + {clearCacheMutation.isError && ( +

+ + Could not clear cached audio: {clearCacheError}. Try again. +

+ )} +
+ +
From bda123b6e150cfa0e7ccd42e82e1f058e3d667a9 Mon Sep 17 00:00:00 2001 From: Chris Scott <99081550+chriswritescode-dev@users.noreply.github.com> Date: Sun, 11 Oct 2026 00:14:14 -0400 Subject: [PATCH 5/6] refactor(tts): reuse the shared atomic write helper for the audio cache --- backend/src/routes/tts.ts | 16 ++---- backend/src/utils/fs-safe.ts | 2 +- backend/test/routes/tts.test.ts | 85 ++++++++++-------------------- backend/test/utils/fs-safe.test.ts | 24 ++++++++- 4 files changed, 57 insertions(+), 70 deletions(-) diff --git a/backend/src/routes/tts.ts b/backend/src/routes/tts.ts index 27df38b3f..2b9e6dba2 100644 --- a/backend/src/routes/tts.ts +++ b/backend/src/routes/tts.ts @@ -1,12 +1,12 @@ import { Hono } from 'hono' import { z } from 'zod' import { Database } from 'bun:sqlite' -import { createHash, randomUUID } from 'crypto' -import { readFile, writeFile, readdir, stat, unlink, rename } from 'fs/promises' +import { createHash } from 'crypto' +import { readFile, readdir, stat, unlink } from 'fs/promises' import { join } from 'path' import { SettingsService } from '../services/settings' import { logger } from '../utils/logger' -import { mkdirSafe } from '../utils/fs-safe' +import { mkdirSafe, writeFileAtomic } from '../utils/fs-safe' import { getWorkspacePath } from '@opencode-manager/shared/config/env' import { normalizeToBaseUrl, @@ -124,15 +124,7 @@ async function cacheAudio(cacheKey: string, audioData: Buffer): Promise { await cleanupOldestFiles(audioData.length) } - const tempPath = join(TTS_CACHE_DIR, `${cacheKey}.${process.pid}.${randomUUID()}.tmp`) - - try { - await writeFile(tempPath, audioData) - await rename(tempPath, filePath) - } catch (error) { - await unlink(tempPath).catch(() => undefined) - throw error - } + await writeFileAtomic(filePath, audioData) } class TTSUpstreamError extends Error { diff --git a/backend/src/utils/fs-safe.ts b/backend/src/utils/fs-safe.ts index 6a591ad06..1e70c180f 100644 --- a/backend/src/utils/fs-safe.ts +++ b/backend/src/utils/fs-safe.ts @@ -26,7 +26,7 @@ export function canonicalPathSync(target: string): string { } } -export async function writeFileAtomic(filePath: string, content: string, options: { mode?: number } = {}): Promise { +export async function writeFileAtomic(filePath: string, content: string | Buffer, options: { mode?: number } = {}): Promise { const dir = path.dirname(filePath) await mkdirSafe(dir) const tempPath = path.join(dir, `.${path.basename(filePath)}.ocm-tmp-${process.pid}-${Date.now()}`) diff --git a/backend/test/routes/tts.test.ts b/backend/test/routes/tts.test.ts index 8a0bcedd1..54a2a501c 100644 --- a/backend/test/routes/tts.test.ts +++ b/backend/test/routes/tts.test.ts @@ -3,16 +3,19 @@ import * as fs from 'fs/promises' vi.mock('fs/promises', () => ({ readFile: vi.fn(), - writeFile: vi.fn(), readdir: vi.fn(), stat: vi.fn(), unlink: vi.fn(), - rename: vi.fn(), +})) + +const { mockWriteFileAtomic } = vi.hoisted(() => ({ + mockWriteFileAtomic: vi.fn(), })) vi.mock('../../src/utils/fs-safe', () => ({ mkdirSafe: vi.fn().mockResolvedValue(undefined), mkdirSyncSafe: vi.fn(), + writeFileAtomic: mockWriteFileAtomic, })) vi.mock('bun:sqlite', () => ({ @@ -57,9 +60,6 @@ const mockUnlink = fs.unlink as any import { createTTSRoutes, cleanupExpiredCache, getCacheStats, generateCacheKey, ensureCacheDir, getCachedAudio, getCacheSize, cleanupOldestFiles, cacheAudio, clearCache } from '../../src/routes/tts' -const mockWriteFile = fs.writeFile as any -const mockRename = (fs as any).rename as any - function createTtsConfig(overrides: Record = {}) { return { enabled: true, @@ -185,8 +185,8 @@ describe('TTS Routes', () => { await expect(clearCache()).rejects.toThrow('denied') }) - it('should leave .tmp and unrelated files untouched', async () => { - mockReaddir.mockResolvedValue(['a.mp3', 'pending.tmp', 'notes.txt'] as any) + it('should leave .tmp, atomic temp and unrelated files untouched', async () => { + mockReaddir.mockResolvedValue(['a.mp3', 'pending.tmp', '.a.mp3.ocm-tmp-1-2', 'notes.txt'] as any) mockUnlink.mockResolvedValue(undefined) const cleared = await clearCache() @@ -404,8 +404,7 @@ describe('TTS route handlers', () => { it('synthesizes and caches audio on a cache miss', async () => { mockStat.mockRejectedValue(new Error('not found')) mockReaddir.mockResolvedValue([] as any) - mockWriteFile.mockResolvedValue(undefined) - mockRename.mockResolvedValue(undefined) + mockWriteFileAtomic.mockResolvedValue(undefined) const fetchMock = vi.fn().mockResolvedValue({ ok: true, arrayBuffer: async () => Buffer.from('fresh-audio'), @@ -429,21 +428,16 @@ describe('TTS route handlers', () => { signal: expect.any(AbortSignal), }), ) - expect(mockWriteFile).toHaveBeenCalledWith( - expect.stringMatching(/\.tmp$/), + expect(mockWriteFileAtomic).toHaveBeenCalledWith( + expect.stringMatching(/\.mp3$/), expect.any(Buffer), ) - expect(mockRename).toHaveBeenCalledWith( - expect.stringMatching(/\.tmp$/), - expect.stringContaining('.mp3'), - ) }) it('returns synthesized audio when the cache write fails', async () => { mockStat.mockRejectedValue(new Error('not found')) mockReaddir.mockResolvedValue([] as any) - mockWriteFile.mockRejectedValue(new Error('disk full')) - mockUnlink.mockResolvedValue(undefined) + mockWriteFileAtomic.mockRejectedValue(new Error('disk full')) const fetchMock = vi.fn().mockResolvedValue({ ok: true, arrayBuffer: async () => Buffer.from('fresh-audio'), @@ -464,8 +458,7 @@ describe('TTS route handlers', () => { it('deduplicates concurrent synthesis requests for the same text', async () => { mockStat.mockRejectedValue(new Error('not found')) mockReaddir.mockResolvedValue([] as any) - mockWriteFile.mockResolvedValue(undefined) - mockRename.mockResolvedValue(undefined) + mockWriteFileAtomic.mockResolvedValue(undefined) let resolveFetch: ((value: { ok: boolean; arrayBuffer: () => Promise }) => void) | undefined const fetchMock = vi.fn().mockImplementation( @@ -504,8 +497,7 @@ describe('TTS route handlers', () => { it('does not share an in-flight synthesis across different endpoints or API keys', async () => { mockStat.mockRejectedValue(new Error('not found')) mockReaddir.mockResolvedValue([] as any) - mockWriteFile.mockResolvedValue(undefined) - mockRename.mockResolvedValue(undefined) + mockWriteFileAtomic.mockResolvedValue(undefined) mockGetSettings.mockImplementation((userId: string) => ({ preferences: { tts: createTtsConfig( @@ -550,8 +542,7 @@ describe('TTS route handlers', () => { it('isolates the disk cache across providers for identical text', async () => { mockStat.mockRejectedValue(new Error('not found')) mockReaddir.mockResolvedValue([] as any) - mockWriteFile.mockResolvedValue(undefined) - mockRename.mockResolvedValue(undefined) + mockWriteFileAtomic.mockResolvedValue(undefined) mockGetSettings.mockImplementation((userId: string) => ({ preferences: { tts: createTtsConfig( @@ -708,7 +699,7 @@ describe('TTS route handlers', () => { }) it('clears the cache through the DELETE /cache route', async () => { - mockReaddir.mockResolvedValue(['a.mp3', 'b.mp3', 'pending.tmp'] as any) + mockReaddir.mockResolvedValue(['a.mp3', 'b.mp3', 'pending.tmp', '.a.mp3.ocm-tmp-1-2'] as any) mockUnlink.mockResolvedValue(undefined) const res = await app.request('/cache', { method: 'DELETE' }) @@ -747,66 +738,48 @@ describe('cacheAudio', () => { mockReaddir.mockResolvedValue(['big.mp3'] as any) mockStat.mockResolvedValue({ size: 200 * 1024 * 1024, mtimeMs: 1000 } as any) mockUnlink.mockResolvedValue(undefined) - mockWriteFile.mockResolvedValue(undefined) - mockRename.mockResolvedValue(undefined) + mockWriteFileAtomic.mockResolvedValue(undefined) await cacheAudio('cache-key', Buffer.from('audio')) expect(mockUnlink).toHaveBeenCalledWith(expect.stringContaining('big.mp3')) - expect(mockWriteFile).toHaveBeenCalledWith( - expect.stringMatching(/\.tmp$/), - expect.any(Buffer), - ) - expect(mockRename).toHaveBeenCalledWith( - expect.stringMatching(/\.tmp$/), + expect(mockWriteFileAtomic).toHaveBeenCalledWith( expect.stringContaining('cache-key.mp3'), + expect.any(Buffer), ) }) it('writes audio without cleanup when the cache has room', async () => { mockReaddir.mockResolvedValue(['small.mp3'] as any) mockStat.mockResolvedValue({ size: 1024, mtimeMs: 1000 } as any) - mockWriteFile.mockResolvedValue(undefined) - mockRename.mockResolvedValue(undefined) + mockWriteFileAtomic.mockResolvedValue(undefined) await cacheAudio('cache-key', Buffer.from('audio')) expect(mockUnlink).not.toHaveBeenCalled() - expect(mockWriteFile).toHaveBeenCalledWith( - expect.stringMatching(/\.tmp$/), - expect.any(Buffer), - ) - expect(mockRename).toHaveBeenCalledWith( - expect.stringMatching(/\.tmp$/), + expect(mockWriteFileAtomic).toHaveBeenCalledWith( expect.stringContaining('cache-key.mp3'), + expect.any(Buffer), ) }) - it('writes to a unique temp path and renames it to the final .mp3 path', async () => { + it('writes the audio buffer atomically to the final .mp3 path', async () => { mockReaddir.mockResolvedValue([] as any) - mockWriteFile.mockResolvedValue(undefined) - mockRename.mockResolvedValue(undefined) + mockWriteFileAtomic.mockResolvedValue(undefined) await cacheAudio('cache-key', Buffer.from('audio')) - const [writtenPath] = mockWriteFile.mock.calls[0] - const [renamedFrom, renamedTo] = mockRename.mock.calls[0] + expect(mockWriteFileAtomic).toHaveBeenCalledTimes(1) + const [writtenPath, writtenData] = mockWriteFileAtomic.mock.calls[0]! - expect(writtenPath).toContain('cache-key.') - expect(writtenPath).toMatch(/\.tmp$/) - expect(writtenPath).not.toContain('.mp3') - expect(renamedFrom).toBe(writtenPath) - expect(renamedTo).toContain('cache-key.mp3') + expect(writtenPath).toContain('cache-key.mp3') + expect(writtenData).toEqual(Buffer.from('audio')) }) - it('removes the temp file and rethrows when the rename fails', async () => { + it('propagates an atomic write failure', async () => { mockReaddir.mockResolvedValue([] as any) - mockWriteFile.mockResolvedValue(undefined) - mockRename.mockRejectedValue(new Error('rename failed')) - mockUnlink.mockResolvedValue(undefined) + mockWriteFileAtomic.mockRejectedValue(new Error('rename failed')) await expect(cacheAudio('cache-key', Buffer.from('audio'))).rejects.toThrow('rename failed') - - expect(mockUnlink).toHaveBeenCalledWith(expect.stringMatching(/\.tmp$/)) }) }) diff --git a/backend/test/utils/fs-safe.test.ts b/backend/test/utils/fs-safe.test.ts index 61b754949..ed46689d7 100644 --- a/backend/test/utils/fs-safe.test.ts +++ b/backend/test/utils/fs-safe.test.ts @@ -2,7 +2,7 @@ import { mkdtemp, readdir, readFile, realpath, rm, stat, writeFile } from 'node: import { tmpdir } from 'node:os' import path from 'node:path' import { afterEach, beforeEach, describe, expect, it } from 'vitest' -import { canonicalPath, canonicalPathSync, writeFileAtomicSync } from '../../src/utils/fs-safe' +import { canonicalPath, canonicalPathSync, writeFileAtomic, writeFileAtomicSync } from '../../src/utils/fs-safe' describe('canonicalPath', () => { let workDir = '' @@ -67,3 +67,25 @@ describe('writeFileAtomicSync', () => { await expect(readdir(workDir)).resolves.toEqual(['file.txt']) }) }) + +describe('writeFileAtomic', () => { + let workDir = '' + + beforeEach(async () => { + workDir = await mkdtemp(path.join(tmpdir(), 'fs-safe-atomic-async-')) + }) + + afterEach(async () => { + await rm(workDir, { recursive: true, force: true }) + }) + + it('writes Buffer bytes unchanged and leaves no temp file behind', async () => { + const target = path.join(workDir, 'audio.mp3') + const bytes = Buffer.from([0xff, 0x00, 0xfe, 0x80]) + + await writeFileAtomic(target, bytes) + + expect(await readFile(target)).toEqual(bytes) + await expect(readdir(workDir)).resolves.toEqual(['audio.mp3']) + }) +}) From 0b76b6d3c767d772575a9ad9677bcad863173862 Mon Sep 17 00:00:00 2001 From: Chris Scott <99081550+chriswritescode-dev@users.noreply.github.com> Date: Sun, 11 Oct 2026 00:18:47 -0400 Subject: [PATCH 6/6] test(fs-safe): cover writeFileAtomic failure cleanup and default mode --- backend/test/utils/fs-safe.test.ts | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/backend/test/utils/fs-safe.test.ts b/backend/test/utils/fs-safe.test.ts index ed46689d7..00dd55ce7 100644 --- a/backend/test/utils/fs-safe.test.ts +++ b/backend/test/utils/fs-safe.test.ts @@ -1,4 +1,4 @@ -import { mkdtemp, readdir, readFile, realpath, rm, stat, writeFile } from 'node:fs/promises' +import { mkdir, mkdtemp, readdir, readFile, realpath, rm, stat, writeFile } from 'node:fs/promises' import { tmpdir } from 'node:os' import path from 'node:path' import { afterEach, beforeEach, describe, expect, it } from 'vitest' @@ -86,6 +86,17 @@ describe('writeFileAtomic', () => { await writeFileAtomic(target, bytes) expect(await readFile(target)).toEqual(bytes) + expect((await stat(target)).mode & 0o777).toBe(0o600) await expect(readdir(workDir)).resolves.toEqual(['audio.mp3']) }) + + it('removes the temp file and rethrows when the rename fails', async () => { + const target = path.join(workDir, 'target') + await mkdir(target) + await writeFile(path.join(target, 'blocker'), 'x') + + await expect(writeFileAtomic(target, 'content')).rejects.toThrow() + + await expect(readdir(workDir)).resolves.toEqual(['target']) + }) })