Fix Recordly 2.0 part 6 follow-up review findings

Preserve existing PR history and integrate validated current-cycle fixes.

services/recordly-share/worker/src/library.js

@@ -64,6 +64,11 @@ export async function cleanupExpired(env) {
 		"DELETE FROM comment_sessions WHERE datetime(expires_at) < datetime('now')",
 	).run();
 
+	// Match handleReport's one-hour admission window; keep reports for moderation.
+	await env.DB.prepare(
+		"UPDATE abuse_reports SET client_ip = '' WHERE client_ip <> '' AND created_at <= datetime('now', '-1 hour')",
+	).run();
+
 	const expired = await env.DB.prepare(
 		"SELECT id, share_code FROM videos WHERE never_expires = 0 AND datetime(expires_at) <= datetime('now')",
 	).all();

services/recordly-share/worker/test/api.test.js

@@ -2,6 +2,7 @@ import { describe, it, expect, beforeAll, vi } from "vitest";
 import { env, SELF } from "cloudflare:test";
 import worker, { sha256Hex } from "../src/index.js";
 import { withRecordingLock } from "../src/recordingLock.js";
+import { cleanupExpired } from "../src/library.js";
 
 import { localCommentHeaders } from "./commentFixtures.js";
 let commentAuth;
@@ -939,3 +940,128 @@ it("keeps an old live lock exclusive and recovers an orphan only by its exact ob
 			.first(),
 	).toBeNull();
 });
+
+describe("report IP cleanup", () => {
+	it("clears expired reporter IPs while preserving reports and moderation history", async () => {
+		const { shareCode } = await createShare();
+		await completeUpload(shareCode);
+		const target = crypto.randomUUID();
+		await env.DB.batch([
+			env.DB.prepare(
+				"INSERT INTO abuse_reports (share_code,reason,details,client_ip,created_at) VALUES (?,'spam','old report','198.51.100.240',datetime('now','-2 hours'))",
+			).bind(shareCode),
+			env.DB.prepare(
+				"INSERT INTO abuse_reports (share_code,reason,details,client_ip,created_at) VALUES (?,'privacy','boundary report','198.51.100.241',datetime('now','-1 hour'))",
+			).bind(shareCode),
+			env.DB.prepare(
+				"INSERT INTO abuse_reports (share_code,reason,details,client_ip,created_at) VALUES (?,'other','active report','198.51.100.242',datetime('now','-59 minutes'))",
+			).bind(shareCode),
+			env.DB.prepare(
+				"INSERT INTO abuse_reports (share_code,reason,details,client_ip,created_at) VALUES (?,'spam','already cleared','',datetime('now','-3 hours'))",
+			).bind(shareCode),
+			env.DB.prepare(
+				"INSERT INTO moderation_actions (action,target,reason) VALUES ('suspend-account',?,'Keep the audit record')",
+			).bind(target),
+		]);
+		const reports = await env.DB.prepare(
+			"SELECT id,share_code,reason,details,created_at FROM abuse_reports WHERE share_code = ? ORDER BY id",
+		)
+			.bind(shareCode)
+			.all();
+		const audit = await env.DB.prepare("SELECT * FROM moderation_actions WHERE target = ?")
+			.bind(target)
+			.all();
+		await env.DB.prepare(
+			"UPDATE videos SET expires_at = datetime('now','-1 day') WHERE share_code = ?",
+		)
+			.bind(shareCode)
+			.run();
+		await cleanupExpired(env);
+		expect(
+			await env.DB.prepare("SELECT id FROM videos WHERE share_code = ?")
+				.bind(shareCode)
+				.first(),
+		).toBeNull();
+		expect(
+			(
+				await env.DB.prepare(
+					"SELECT id,share_code,reason,details,created_at FROM abuse_reports WHERE share_code = ? ORDER BY id",
+				)
+					.bind(shareCode)
+					.all()
+			).results,
+		).toEqual(reports.results);
+		const addresses = await env.DB.prepare(
+			"SELECT details,client_ip FROM abuse_reports WHERE share_code = ? ORDER BY id",
+		)
+			.bind(shareCode)
+			.all();
+		expect(addresses.results).toEqual([
+			{ details: "old report", client_ip: "" },
+			{ details: "boundary report", client_ip: "" },
+			{ details: "active report", client_ip: "198.51.100.242" },
+			{ details: "already cleared", client_ip: "" },
+		]);
+		expect(
+			(
+				await env.DB.prepare("SELECT * FROM moderation_actions WHERE target = ?")
+					.bind(target)
+					.all()
+			).results,
+		).toEqual(audit.results);
+		expect(
+			(
+				await env.DB.prepare(
+					"SELECT suspended FROM account_restrictions WHERE owner_id = ?",
+				)
+					.bind(target)
+					.first()
+			).suspended,
+		).toBe(1);
+		await cleanupExpired(env);
+		expect(
+			(
+				await env.DB.prepare(
+					"SELECT details,client_ip FROM abuse_reports WHERE share_code = ? ORDER BY id",
+				)
+					.bind(shareCode)
+					.all()
+			).results,
+		).toEqual(addresses.results);
+	});
+
+	it("retains active per-IP and per-recording report admission caps after cleanup", async () => {
+		const { shareCode: first } = await createShare();
+		const { shareCode: second } = await createShare();
+		await completeUpload(first);
+		await completeUpload(second);
+		const ip = "198.51.100.244";
+		await env.DB.batch([
+			...Array.from({ length: 3 }, () =>
+				env.DB.prepare(
+					"INSERT INTO abuse_reports (share_code,reason,details,client_ip,created_at) VALUES (?,'spam','old',?,datetime('now','-2 hours'))",
+				).bind(first, ip),
+			),
+			...Array.from({ length: 3 }, () =>
+				env.DB.prepare(
+					"INSERT INTO abuse_reports (share_code,reason,details,client_ip) VALUES (?,'spam','active',?)",
+				).bind(first, ip),
+			),
+			...Array.from({ length: 20 }, (_, i) =>
+				env.DB.prepare(
+					"INSERT INTO abuse_reports (share_code,reason,details,client_ip) VALUES (?,'spam','active recording report',?)",
+				).bind(second, `203.0.113.${i + 1}`),
+			),
+		]);
+		await cleanupExpired(env);
+		const report = (code, clientIp) =>
+			SELF.fetch(`${BASE}/s/${code}/report`, {
+				method: "POST",
+				headers: { "Content-Type": "application/json", "CF-Connecting-IP": clientIp },
+				body: JSON.stringify({ reason: "spam", details: "New report" }),
+			});
+		expect((await report(first, ip)).status).toBe(429);
+		expect((await report(second, "198.51.100.245")).status).toBe(429);
+		expect((await report(first, "198.51.100.245")).status).toBe(200);
+	});
+});

src/dev/announcementStudioModel.test.ts

@@ -100,4 +100,43 @@ describe("studio runtime compatibility", () => {
 			}),
 		).toHaveLength(2);
 	});
+	it.each([
+		["id", "Use an ID between 1 and 100 characters."],
+		["title", "Use a title between 1 and 160 characters."],
+		["body", "Use a message between 1 and 2,000 characters."],
+	] as const)("does not blame a valid action when the %s is missing", (field, message) => {
+		for (const action of [
+			{ actionType: "section", actionDestination: "settings" },
+			{ actionType: "url", actionDestination: "https://recordly.dev" },
+		] as const) {
+			expect(
+				validateDraft({ ...defaults, [field]: "  ", actionEnabled: true, ...action }),
+			).toEqual([message]);
+		}
+	});
+	it.each([
+		{ actionLabel: "" },
+		{ actionType: "url", actionDestination: "http://recordly.dev" },
+		{ actionType: "section", actionDestination: "not-an-editor-section" },
+	] as const)("still rejects an invalid action on an otherwise valid draft: %o", (action) => {
+		expect(validateDraft({ ...defaults, actionEnabled: true, ...action })).toEqual([
+			"Use an action label (up to 80 characters) and a valid HTTPS URL or editor section.",
+		]);
+	});
+	it("keeps independent dashboard presentation errors when a required field is missing", () => {
+		expect(
+			validateDraft({
+				...defaults,
+				id: "",
+				presentation: "dashboard-banner",
+				mediaType: "video",
+				mediaUrl: "/demo.mp4",
+				actionEnabled: true,
+			}),
+		).toEqual([
+			"Use an ID between 1 and 100 characters.",
+			"Dashboard banners support images only.",
+			"Dashboard banners link to an HTTPS URL.",
+		]);
+	});
 });

src/dev/announcementStudioModel.ts

@@ -182,9 +182,9 @@ export function validateDraft(draft: StudioDraft): string[] {
 	if (draft.posterUrl && !safeMedia(draft.posterUrl))
 		errors.push("Poster must use HTTPS or a /public-path.");
 	if (draft.actionEnabled) {
-		const action = parseAnnouncementFeed({ announcements: [buildAnnouncement(draft)] })
-			.announcements[0]?.action;
-		if (!action)
+		const announcement = parseAnnouncementFeed({ announcements: [buildAnnouncement(draft)] })
+			.announcements[0];
+		if (announcement && !announcement.action)
 			errors.push(
 				"Use an action label (up to 80 characters) and a valid HTTPS URL or editor section.",
 			);

src/hooks/recordingFileWriter.test.ts

@@ -1,5 +1,6 @@
 import { describe, expect, it, vi } from "vitest";
 import { RecordingFileWriter } from "./recordingFileWriter";
+
 function setup() {
 	const api = {
 		openRecordingFileStream: vi.fn(async () => ({
@@ -54,12 +55,74 @@ describe("recording chunk persistence", () => {
 		expect(await writer.finish()).toMatchObject({ success: false, message: "No space" });
 		expect(api.appendRecordingFileStream).not.toHaveBeenCalled();
 	});
-	it("bounds backlog and stops accepting chunks when storage stalls", async () => {
+	it("accepts a single large chunk on an empty queue and drains it in ordered bounded IPC writes", async () => {
 		const { api, writer, fail } = setup();
-		writer.write(new Blob([new Uint8Array(33 * 1024 * 1024)]));
+		const slices = Array.from({ length: 9 }, (_, index) =>
+			new Uint8Array((index === 8 ? 1 : 4) * 1024 * 1024).fill(index),
+		);
+		writer.write(new Blob(slices));
+		await expect(writer.finish(100)).resolves.toEqual({ success: true, path: "/final.mp4" });
+		expect(fail).not.toHaveBeenCalled();
+		expect(api.openRecordingFileStream).toHaveBeenCalledOnce();
+		expect(api.appendRecordingFileStream.mock.calls).toHaveLength(9);
+		for (const [index, call] of api.appendRecordingFileStream.mock.calls.entries()) {
+			const bytes = new Uint8Array(call[1]);
+			expect(bytes.byteLength).toBe(slices[index].byteLength);
+			expect(bytes[0]).toBe(index);
+			expect(bytes.at(-1)).toBe(index);
+		}
+		expect(api.closeRecordingFileStream.mock.invocationCallOrder[0]).toBeGreaterThan(
+			api.appendRecordingFileStream.mock.invocationCallOrder.at(-1)!,
+		);
+	});
+	it.each([
+		16, 33,
+	])("bounds queued backlog while a %s MiB chunk is already stalled", async (sizeMiB) => {
+		const { api, writer, fail } = setup();
+		let release!: () => void;
+		api.appendRecordingFileStream.mockImplementationOnce(
+			() =>
+				new Promise((resolve) => {
+					release = () => resolve({ success: true });
+				}),
+		);
+		writer.write(new Blob([new Uint8Array(sizeMiB * 1024 * 1024)]));
+		await vi.waitFor(() => expect(api.appendRecordingFileStream).toHaveBeenCalledOnce());
+		if (sizeMiB === 16) writer.write(new Blob([new Uint8Array(16 * 1024 * 1024)]));
+		expect(fail).not.toHaveBeenCalled();
+		writer.write(new Blob(["overflow"]));
+		writer.write(new Blob(["ignored after failure"]));
 		expect(fail).toHaveBeenCalledOnce();
-		expect((await writer.finish()).success).toBe(false);
-		expect(api.openRecordingFileStream).not.toHaveBeenCalled();
+		const finished = writer.finish();
+		expect(api.closeRecordingFileStream).not.toHaveBeenCalled();
+		release();
+		await expect(finished).resolves.toMatchObject({ success: false, path: "/final.mp4" });
+		expect((await finished).message).toContain("Partial recording: /final.mp4");
+		expect(api.appendRecordingFileStream).toHaveBeenCalledOnce();
+		expect(api.closeRecordingFileStream).toHaveBeenCalledOnce();
+	});
+	it("stops draining a large chunk and discards its stream when cancelled", async () => {
+		const { api, writer, fail } = setup();
+		let release!: () => void;
+		api.appendRecordingFileStream.mockImplementationOnce(
+			() =>
+				new Promise((resolve) => {
+					release = () => resolve({ success: true });
+				}),
+		);
+		writer.write(new Blob([new Uint8Array(33 * 1024 * 1024)]));
+		await vi.waitFor(() => expect(api.appendRecordingFileStream).toHaveBeenCalledOnce());
+		writer.cancel();
+		const finished = writer.finish();
+		release();
+		await expect(finished).resolves.toEqual({
+			success: false,
+			message: "Recording cancelled.",
+		});
+		expect(fail).not.toHaveBeenCalled();
+		expect(api.appendRecordingFileStream).toHaveBeenCalledOnce();
+		expect(api.discardRecordingFileStream).toHaveBeenCalledExactlyOnceWith("1");
+		expect(api.closeRecordingFileStream).not.toHaveBeenCalled();
 	});
 	it("splits large recorder chunks into bounded IPC messages", async () => {
 		const { api, writer } = setup();

src/hooks/recordingFileWriter.ts

@@ -28,7 +28,8 @@ export class RecordingFileWriter {
 	}
 	write(chunk: Blob) {
 		if (this.cancelled || this.failure || !chunk.size) return;
-		if (this.pendingBytes + chunk.size > 32 * 1024 * 1024) {
+		// A single delayed recorder Blob drains in slices; bound additional queued data.
+		if (this.pendingBytes > 0 && this.pendingBytes + chunk.size > 32 * 1024 * 1024) {
 			this.fail(
 				new Error(
 					"Recording storage cannot keep up. The partial recording will be preserved.",

src/hooks/recordingMicrophoneFallback.test.ts

@@ -422,3 +422,235 @@ describe("browser microphone fallback capture", () => {
 		expect(storeMicrophoneSidecar).not.toHaveBeenCalled();
 	});
 });
+
+const microphoneHooks = vi.hoisted(() => {
+	const states: unknown[] = [];
+	const cleanups: (() => void)[] = [];
+	let writes = 0;
+	return {
+		states,
+		reset() {
+			states.length = 0;
+			cleanups.length = 0;
+			writes = 0;
+		},
+		cleanup() {
+			for (const cleanup of cleanups.splice(0)) cleanup();
+		},
+		get writes() {
+			return writes;
+		},
+		useState<T>(initial: T) {
+			const index = states.length;
+			states.push(initial);
+			return [
+				initial,
+				(value: T | ((previous: T) => T)) => {
+					writes++;
+					states[index] =
+						typeof value === "function"
+							? (value as (previous: T) => T)(states[index] as T)
+							: value;
+				},
+			] as const;
+		},
+		useEffect(effect: () => (() => void) | undefined) {
+			const cleanup = effect();
+			if (cleanup) cleanups.push(cleanup);
+		},
+	};
+});
+vi.mock("react", () => ({
+	useState: microphoneHooks.useState,
+	useEffect: microphoneHooks.useEffect,
+}));
+vi.mock("@/contexts/I18nContext", () => ({
+	useI18n: () => ({
+		t: (key: string, fallback: string, values: Record<string, string> = {}) =>
+			key === "notifications.microphone"
+				? `Micrófono ${values.id}`
+				: fallback.replace(/\{\{(\w+)\}\}/g, (_match, name: string) => values[name] ?? ""),
+	}),
+}));
+
+describe("microphone device label permission", () => {
+	let useMicrophoneDevices: typeof import("./useMicrophoneDevices").useMicrophoneDevices;
+	let enumerateDevices: ReturnType<typeof vi.fn>;
+	let getUserMedia: ReturnType<typeof vi.fn>;
+	let stop: ReturnType<typeof vi.fn>;
+	let stream: MediaStream;
+	let deviceChange: (() => void) | undefined;
+	let removeEventListener: ReturnType<typeof vi.fn>;
+	const microphone = (label = "", deviceId = "microphone-1") => ({
+		kind: "audioinput",
+		deviceId,
+		groupId: "group-1",
+		label,
+	});
+	const flush = async () => {
+		for (let step = 0; step < 10; step++) await Promise.resolve();
+	};
+	const mount = (enabled = true, preferredDeviceId?: string) => {
+		useMicrophoneDevices(enabled, preferredDeviceId);
+	};
+	beforeEach(async () => {
+		vi.resetModules();
+		microphoneHooks.reset();
+		deviceChange = undefined;
+		stop = vi.fn();
+		stream = { getTracks: () => [{ stop }] } as unknown as MediaStream;
+		enumerateDevices = vi.fn().mockResolvedValue([microphone()]);
+		getUserMedia = vi.fn().mockResolvedValue(stream);
+		removeEventListener = vi.fn();
+		vi.stubGlobal("navigator", {
+			mediaDevices: {
+				enumerateDevices,
+				getUserMedia,
+				addEventListener: (_event: string, listener: () => void) => {
+					deviceChange = listener;
+				},
+				removeEventListener,
+			},
+		});
+		vi.spyOn(console, "error").mockImplementation(() => {});
+		({ useMicrophoneDevices } = await import("./useMicrophoneDevices"));
+	});
+	afterEach(() => {
+		microphoneHooks.cleanup();
+		vi.restoreAllMocks();
+		vi.unstubAllGlobals();
+	});
+
+	it("requests hidden microphone labels from raw devices and releases the permission stream", async () => {
+		enumerateDevices
+			.mockResolvedValueOnce([
+				microphone(),
+				{ kind: "videoinput", deviceId: "camera", label: "Camera" },
+			])
+			.mockResolvedValueOnce([microphone("Desk microphone")]);
+		mount(true, "microphone-1");
+		await flush();
+		expect(getUserMedia).toHaveBeenCalledExactlyOnceWith({ audio: true });
+		expect(enumerateDevices).toHaveBeenCalledTimes(2);
+		expect(microphoneHooks.states[0]).toEqual(
+			[microphone("Desk microphone")].map(({ kind: _kind, ...device }) => device),
+		);
+		expect(microphoneHooks.states[1]).toBe("microphone-1");
+		expect(microphoneHooks.states[2]).toBe(false);
+		expect(stop).toHaveBeenCalledOnce();
+	});
+
+	it("retains localized display fallbacks if permission still leaves labels hidden", async () => {
+		mount();
+		await flush();
+		expect(getUserMedia).toHaveBeenCalledOnce();
+		expect(microphoneHooks.states[0]).toEqual([
+			{ deviceId: "microphone-1", groupId: "group-1", label: "Micrófono micropho" },
+		]);
+		expect(stop).toHaveBeenCalledOnce();
+	});
+
+	it.each([
+		["labeled", [microphone("Desk")]],
+		["mixed", [microphone("Desk"), microphone("", "second")]],
+		["no audio", [{ kind: "videoinput", deviceId: "camera", label: "" }]],
+	])("does not request label permission for %s inputs", async (_label, devices) => {
+		enumerateDevices.mockResolvedValue(devices);
+		mount();
+		await flush();
+		expect(getUserMedia).not.toHaveBeenCalled();
+		expect(stop).not.toHaveBeenCalled();
+		if (_label === "mixed") {
+			expect(microphoneHooks.states[0]).toContainEqual({
+				deviceId: "second",
+				groupId: "group-1",
+				label: "Micrófono second",
+			});
+		}
+	});
+
+	it("treats whitespace-only raw labels as hidden", async () => {
+		enumerateDevices.mockResolvedValue([microphone("   ")]);
+		mount();
+		await flush();
+		expect(getUserMedia).toHaveBeenCalledOnce();
+		expect(stop).toHaveBeenCalledOnce();
+	});
+
+	it("does not enumerate or request permission while disabled", async () => {
+		mount(false);
+		await flush();
+		expect(enumerateDevices).not.toHaveBeenCalled();
+		expect(getUserMedia).not.toHaveBeenCalled();
+		expect(deviceChange).toBeUndefined();
+	});
+
+	it("reports permission denial and does not prompt again on device changes", async () => {
+		getUserMedia.mockRejectedValue(new Error("Microphone permission denied"));
+		mount();
+		await flush();
+		expect(microphoneHooks.states[3]).toBe("Microphone permission denied");
+		expect(microphoneHooks.states[2]).toBe(false);
+		expect(stop).not.toHaveBeenCalled();
+		deviceChange?.();
+		await flush();
+		expect(getUserMedia).toHaveBeenCalledOnce();
+		expect(microphoneHooks.states[3]).toBeNull();
+	});
+
+	it("releases the permission stream when follow-up enumeration rejects", async () => {
+		enumerateDevices
+			.mockResolvedValueOnce([microphone()])
+			.mockRejectedValueOnce(new Error("Enumeration failed"));
+		mount();
+		await flush();
+		expect(getUserMedia).toHaveBeenCalledOnce();
+		expect(microphoneHooks.states[3]).toBe("Enumeration failed");
+		expect(microphoneHooks.states[2]).toBe(false);
+		expect(stop).toHaveBeenCalledOnce();
+	});
+
+	it("does not request permission after pending enumeration is unmounted", async () => {
+		let resolve!: (devices: ReturnType<typeof microphone>[]) => void;
+		enumerateDevices.mockReturnValueOnce(
+			new Promise((done) => {
+				resolve = done;
+			}),
+		);
+		mount();
+		microphoneHooks.cleanup();
+		const writes = microphoneHooks.writes;
+		resolve([microphone()]);
+		await flush();
+		expect(getUserMedia).not.toHaveBeenCalled();
+		expect(microphoneHooks.writes).toBe(writes);
+		expect(removeEventListener).toHaveBeenCalledWith("devicechange", deviceChange);
+	});
+
+	it("releases a late permission stream without state writes after unmount", async () => {
+		let resolve!: (stream: MediaStream) => void;
+		getUserMedia.mockReturnValueOnce(
+			new Promise((done) => {
+				resolve = done;
+			}),
+		);
+		mount();
+		await flush();
+		expect(getUserMedia).toHaveBeenCalledOnce();
+		microphoneHooks.cleanup();
+		const writes = microphoneHooks.writes;
+		resolve(stream);
+		await flush();
+		expect(stop).toHaveBeenCalledOnce();
+		expect(microphoneHooks.writes).toBe(writes);
+	});
+
+	it("requests label permission only once per session after success", async () => {
+		mount();
+		await flush();
+		deviceChange?.();
+		await flush();
+		expect(getUserMedia).toHaveBeenCalledOnce();
+		expect(stop).toHaveBeenCalledOnce();
+	});
+});

src/hooks/useMicrophoneDevices.ts

@@ -31,22 +31,22 @@ export function useMicrophoneDevices(enabled: boolean = true, preferredDeviceId?
 				setError(null);
 
 				let allDevices = await navigator.mediaDevices.enumerateDevices();
-				let audioInputs = allDevices
-					.filter((device) => device.kind === "audioinput")
-					.map((device) => ({
-						deviceId: device.deviceId,
-						label:
-							device.label ||
-							t("notifications.microphone", "Microphone {{id}}", {
-								id: device.deviceId.slice(0, 8),
-							}),
-						groupId: device.groupId,
-					}));
+				const rawAudioInputs = allDevices.filter((device) => device.kind === "audioinput");
+				let audioInputs = rawAudioInputs.map((device) => ({
+					deviceId: device.deviceId,
+					label:
+						device.label ||
+						t("notifications.microphone", "Microphone {{id}}", {
+							id: device.deviceId.slice(0, 8),
+						}),
+					groupId: device.groupId,
+				}));
 
 				const needsLabelPermission =
-					audioInputs.length > 0 && audioInputs.every((device) => !device.label.trim());
+					rawAudioInputs.length > 0 &&
+					rawAudioInputs.every((device) => !device.label.trim());
 
-				if (needsLabelPermission && !hasRequestedMicrophoneLabels) {
+				if (mounted && needsLabelPermission && !hasRequestedMicrophoneLabels) {
 					hasRequestedMicrophoneLabels = true;
 					permissionStream = await navigator.mediaDevices.getUserMedia({ audio: true });
 					allDevices = await navigator.mediaDevices.enumerateDevices();