fix(web): keep voice-note processing owned by its recording

.github/workflows/ci.yml

@@ -141,6 +141,7 @@ jobs:
           __tests__/unit/invite-decline.test.ts
           __tests__/unit/linkified-text.test.ts
           __tests__/unit/loom-import-ui.test.ts
+          __tests__/unit/voice-recorder-lifecycle.test.ts
           __tests__/unit/media-comment-playback-cleanup.test.ts
           __tests__/unit/media-format-converter-selection.test.ts
           __tests__/unit/media-format-converter.test.ts

apps/web/__tests__/unit/voice-recorder-lifecycle.test.ts

@@ -0,0 +1,264 @@
+// @vitest-environment jsdom
+
+import { act, createElement } from "react";
+import { createRoot, type Root } from "react-dom/client";
+import { afterEach, beforeEach, expect, it, vi } from "vitest";
+import { useVoiceRecorder } from "@/app/s/[videoId]/_components/timeline/recording/useVoiceRecorder";
+
+const mocks = vi.hoisted(() => ({
+	mic: vi.fn(),
+	mixer: vi.fn(),
+	decode: vi.fn(),
+}));
+vi.mock("@cap/recorder-core/capture-streams", () => ({
+	acquireMicStream: mocks.mic,
+	createAudioMixer: mocks.mixer,
+}));
+vi.mock(
+	"@/app/s/[videoId]/_components/timeline/recording/compute-waveform-peaks",
+	async (importOriginal) => ({
+		...(await importOriginal<
+			typeof import("@/app/s/[videoId]/_components/timeline/recording/compute-waveform-peaks")
+		>()),
+		decodeToPeaks: mocks.decode,
+	}),
+);
+function deferred<T>() {
+	let resolve!: (value: T) => void;
+	const promise = new Promise<T>((done) => {
+		resolve = done;
+	});
+	return { promise, resolve };
+}
+class Track {
+	readyState = "live";
+	stop = vi.fn(() => {
+		this.readyState = "ended";
+	});
+	getSettings() {
+		return { deviceId: "microphone" };
+	}
+}
+class Stream {
+	constructor(readonly track = new Track()) {}
+	getTracks() {
+		return [this.track];
+	}
+	getAudioTracks() {
+		return [this.track];
+	}
+}
+class Recorder {
+	static isTypeSupported = () => true;
+	static startError: Error | null = null;
+	static pendingStops: Array<() => void> | null = null;
+	state: RecordingState = "inactive";
+	mimeType = "audio/webm";
+	ondataavailable: ((event: BlobEvent) => void) | null = null;
+	onstop: ((event: Event) => void) | null = null;
+	onerror: ((event: Event) => void) | null = null;
+	start() {
+		if (Recorder.startError) throw Recorder.startError;
+		this.state = "recording";
+	}
+	stop() {
+		this.state = "inactive";
+		const finish = () => {
+			const event = new Event("dataavailable");
+			Object.defineProperty(event, "target", { value: this });
+			Object.defineProperty(event, "data", {
+				value: new Blob(["audio"], { type: "audio/webm" }),
+			});
+			this.ondataavailable?.(event as BlobEvent);
+			const stopped = new Event("stop");
+			Object.defineProperty(stopped, "target", { value: this });
+			this.onstop?.(stopped);
+		};
+		if (Recorder.pendingStops) Recorder.pendingStops.push(finish);
+		else queueMicrotask(finish);
+	}
+}
+let root: Root;
+let container: HTMLDivElement;
+let mounted: boolean;
+let recorder: ReturnType<typeof useVoiceRecorder>;
+const onFinish = vi.fn();
+function mixer(stream = new Stream()) {
+	return {
+		context: {},
+		stream,
+		close: vi.fn(async () => {}),
+		setMicStream: vi.fn(),
+	};
+}
+beforeEach(async () => {
+	Recorder.startError = null;
+	Recorder.pendingStops = null;
+	vi.stubGlobal("IS_REACT_ACT_ENVIRONMENT", true);
+	vi.stubGlobal("MediaRecorder", Recorder);
+	onFinish.mockReset();
+	mocks.mic.mockReset().mockImplementation(async () => new Stream());
+	mocks.mixer.mockReset().mockImplementation(async () => mixer());
+	mocks.decode.mockReset().mockResolvedValue([0.25, 1]);
+	container = document.createElement("div");
+	document.body.append(container);
+	root = createRoot(container);
+	mounted = true;
+	function Harness() {
+		recorder = useVoiceRecorder({ onFinish, selectedMicId: null });
+		return null;
+	}
+	await act(async () => root.render(createElement(Harness)));
+});
+afterEach(async () => {
+	if (mounted) await unmount();
+	container.remove();
+	vi.restoreAllMocks();
+	vi.unstubAllGlobals();
+});
+async function unmount() {
+	await act(async () => root.unmount());
+	mounted = false;
+}
+async function beginDecoding() {
+	const decode = deferred<number[] | undefined>();
+	mocks.decode.mockReturnValueOnce(decode.promise);
+	await act(async () => recorder.start());
+	let stopping!: Promise<void>;
+	await act(async () => {
+		stopping = recorder.stop();
+	});
+	expect(mocks.decode).toHaveBeenCalledTimes(1);
+	return { decode, stopping };
+}
+
+it.each(["cancel", "unmount"])(
+	"does not finish a voice note after %s during waveform decoding",
+	async (action) => {
+		const { decode, stopping } = await beginDecoding();
+		if (action === "cancel") await act(async () => recorder.cancel());
+		else await unmount();
+		await act(async () => {
+			decode.resolve([1]);
+			await stopping;
+		});
+		expect(onFinish).not.toHaveBeenCalled();
+	},
+);
+
+it("does not finish an old take or reset the replacement while decoding completes", async () => {
+	const { decode, stopping } = await beginDecoding();
+	await act(async () => recorder.cancel());
+	await act(async () => recorder.start());
+	expect(recorder.phase).toBe("recording");
+	await act(async () => {
+		decode.resolve([1]);
+		await stopping;
+	});
+	expect(onFinish).not.toHaveBeenCalled();
+	expect(recorder.phase).toBe("recording");
+});
+
+it("does not close the replacement microphone when an abandoned mixer finishes", async () => {
+	const pendingMixer = deferred<ReturnType<typeof mixer>>();
+	const abandoned = mixer();
+	const replacement = mixer();
+	const firstMic = new Stream();
+	const nextMic = new Stream();
+	mocks.mic.mockResolvedValueOnce(firstMic).mockResolvedValueOnce(nextMic);
+	mocks.mixer
+		.mockReturnValueOnce(pendingMixer.promise)
+		.mockResolvedValueOnce(replacement);
+	let starting!: Promise<void>;
+	await act(async () => {
+		starting = recorder.start();
+	});
+	await act(async () => recorder.cancel());
+	await act(async () => recorder.start());
+	await act(async () => {
+		pendingMixer.resolve(abandoned);
+		await starting;
+	});
+	expect(abandoned.close).toHaveBeenCalled();
+	expect(firstMic.track.stop).toHaveBeenCalled();
+	expect(nextMic.track.stop).not.toHaveBeenCalled();
+	expect(replacement.close).not.toHaveBeenCalled();
+	expect(recorder.phase).toBe("recording");
+});
+
+it("still finishes a current recording with the decoded waveform", async () => {
+	await act(async () => recorder.start());
+	await act(async () => recorder.stop());
+	expect(onFinish).toHaveBeenCalledTimes(1);
+	expect(onFinish).toHaveBeenCalledWith(
+		expect.objectContaining({
+			mime: "audio/webm",
+			waveform: [0.25, 1],
+			blob: expect.objectContaining({ size: 5 }),
+		}),
+	);
+	expect(recorder.phase).toBe("idle");
+});
+
+it("releases a microphone granted after startup was cancelled", async () => {
+	const pendingMic = deferred<Stream>();
+	const stream = new Stream();
+	mocks.mic.mockReturnValueOnce(pendingMic.promise);
+	let starting!: Promise<void>;
+	await act(async () => {
+		starting = recorder.start();
+	});
+	await act(async () => recorder.cancel());
+	await act(async () => {
+		pendingMic.resolve(stream);
+		await starting;
+	});
+	expect(stream.track.stop).toHaveBeenCalled();
+	expect(mocks.mixer).not.toHaveBeenCalled();
+	expect(onFinish).not.toHaveBeenCalled();
+	expect(recorder.phase).toBe("idle");
+});
+
+it.each(["start", "restart"])(
+	"releases capture when recorder %s throws",
+	async (operation) => {
+		const stream = new Stream();
+		const activeMixer = mixer();
+		mocks.mic.mockResolvedValueOnce(stream);
+		mocks.mixer.mockResolvedValueOnce(activeMixer);
+		if (operation === "restart") await act(async () => recorder.start());
+		Recorder.startError = new Error("Encoder unavailable");
+		await act(async () => {
+			if (operation === "start") await recorder.start();
+			else recorder.restart();
+		});
+		expect(recorder.phase).toBe("denied");
+		expect(stream.track.stop).toHaveBeenCalled();
+		expect(activeMixer.close).toHaveBeenCalled();
+		expect(onFinish).not.toHaveBeenCalled();
+	},
+);
+
+it("keeps a replacement take when the previous stop event arrives late", async () => {
+	await act(async () => recorder.start());
+	Recorder.pendingStops = [];
+	let oldStop!: Promise<void>;
+	let newStart!: Promise<void>;
+	await act(async () => {
+		oldStop = recorder.stop();
+		recorder.cancel();
+		newStart = recorder.start();
+		await newStart;
+		await oldStop;
+	});
+	expect(recorder.phase).toBe("recording");
+	const pending = Recorder.pendingStops;
+	Recorder.pendingStops = null;
+	await act(async () => {
+		for (const finish of pending ?? []) finish();
+	});
+	expect(recorder.phase).toBe("recording");
+	await act(async () => recorder.stop());
+	expect(onFinish).toHaveBeenCalledTimes(1);
+	expect(recorder.phase).toBe("idle");
+});

apps/web/app/s/[videoId]/_components/timeline/recording/useVoiceRecorder.ts

@@ -169,7 +169,7 @@ export function useVoiceRecorder(options: {
 		}
 		if (runId !== runIdRef.current || discardRef.current) {
 			if (mixer) void mixer.close();
-			cleanupStreams();
+			for (const track of stream.getTracks()) track.stop();
 			return;
 		}
 		mixerRef.current = mixer;
@@ -213,16 +213,28 @@ export function useVoiceRecorder(options: {
 			}, LEVEL_SAMPLE_MS);
 		}
 
-		recorder.start(1000);
+		try {
+			recorder.start(1000);
+		} catch {
+			resetRecorder();
+			cleanupStreams();
+			clearTimer();
+			resetTimer();
+			updatePhase("denied");
+			return;
+		}
 		startTimer();
 		updatePhase("recording");
 	}, [
 		attachMeter,
 		cleanupStreams,
+		clearTimer,
 		mediaRecorderRef,
 		onRecorderDataAvailable,
 		onRecorderError,
 		onRecorderStop,
+		resetRecorder,
+		resetTimer,
 		setLocalRecordingStrategy,
 		startTimer,
 		updatePhase,
@@ -302,7 +314,16 @@ export function useVoiceRecorder(options: {
 		next.onerror = onRecorderError;
 		mediaRecorderRef.current = next;
 		peaksRef.current = new LivePeaksAccumulator();
-		next.start(1000);
+		try {
+			next.start(1000);
+		} catch {
+			resetRecorder();
+			cleanupStreams();
+			clearTimer();
+			resetTimer();
+			updatePhase("denied");
+			return;
+		}
 		startTimer();
 		updatePhase("recording");
 	}, [
@@ -367,6 +388,7 @@ export function useVoiceRecorder(options: {
 	const stop = useCallback(async () => {
 		if (phaseRef.current !== "recording" && phaseRef.current !== "paused")
 			return;
+		const runId = runIdRef.current;
 		// Pause accounting folds any open pause in, so this is active time only.
 		const recordedMs = syncDurationFromClock();
 		const mime =
@@ -381,6 +403,7 @@ export function useVoiceRecorder(options: {
 		} catch (cause) {
 			console.error("Failed to finalize voice note", cause);
 		}
+		if (runId !== runIdRef.current) return;
 		const livePeaks = peaksRef.current?.finish();
 		peaksRef.current = null;
 		resetRecorder();
@@ -393,6 +416,7 @@ export function useVoiceRecorder(options: {
 
 		const typedBlob = blob.type ? blob : new Blob([blob], { type: mime });
 		const waveform = livePeaks ?? (await decodeToPeaks(typedBlob));
+		if (runId !== runIdRef.current || discardRef.current) return;
 		optionsRef.current.onFinish({
 			blob: typedBlob,
 			mime,