Fix Recordly 2.0 part 7 follow-up review findings

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

electron/authCallback.ts

@@ -56,7 +56,7 @@ export function createAuthCallbackController({
 	const startupUrls: string[] = [];
 	const startupSeen = new Set<string>();
 	const pendingByOwner = new Map<WebContents, string[]>();
-	const flowOwners = new Map<string, WebContents>();
+	const flowOwners = new Map<string, { owner: WebContents; verifierKey: string }>();
 	const observedOwners = new WeakSet<WebContents>();
 	const relay = flowRoutingEnabled
 		? createAuthFlowRelay<WebContents>({
@@ -91,19 +91,29 @@ export function createAuthCallbackController({
 		/^sb-[a-z0-9-]{1,100}-auth-token-flow-([A-Za-z0-9_-]{8,64})-code-verifier$/.exec(key)?.[1];
 	const flowStorageHooks: AuthFlowStorageHooks | undefined = relay
 		? {
-				prepareSet: async (event, key) => {
+				prepareSet: async (event, key, persistedKeys) => {
 					const flowId = flowKey(key);
 					if (!flowId) return;
+					if (persistedKeys) {
+						const present = new Set(persistedKeys);
+						for (const [id, flow] of flowOwners) {
+							if (present.has(flow.verifierKey)) continue;
+							// A peer SDK may have explicitly evicted this verifier. Index
+							// absence alone never proves inactivity; exact slot absence does.
+							await relay.release(id, flow.owner);
+							if (flowOwners.get(id) === flow) flowOwners.delete(id);
+						}
+					}
 					const owner = event.sender;
-					const alreadyOwned = flowOwners.get(flowId) === owner;
+					const alreadyOwned = flowOwners.get(flowId)?.owner === owner;
 					const rollback = await relay.register(flowId, key, owner);
-					flowOwners.set(flowId, owner);
+					flowOwners.set(flowId, { owner, verifierKey: key });
 					if (!observedOwners.has(owner)) {
 						observedOwners.add(owner);
 						owner.once("destroyed", () => {
 							pendingByOwner.delete(owner);
 							for (const [id, candidate] of flowOwners)
-								if (candidate === owner) {
+								if (candidate.owner === owner) {
 									flowOwners.delete(id);
 									void relay.release(id, owner).catch(() => undefined);
 								}
@@ -111,16 +121,16 @@ export function createAuthCallbackController({
 					}
 					return async () => {
 						await rollback();
-						if (!alreadyOwned && flowOwners.get(flowId) === owner)
+						if (!alreadyOwned && flowOwners.get(flowId)?.owner === owner)
 							flowOwners.delete(flowId);
 					};
 				},
 				didRemove: async (_event, key) => {
 					const flowId = flowKey(key);
-					const owner = flowId && flowOwners.get(flowId);
-					if (flowId && owner) {
+					const flow = flowId && flowOwners.get(flowId);
+					if (flowId && flow && flow.verifierKey === key) {
 						flowOwners.delete(flowId);
-						await relay.release(flowId, owner);
+						await relay.release(flowId, flow.owner);
 					}
 				},
 			}

electron/authCallbackFlowRouting.test.ts

@@ -405,3 +405,48 @@ it("cleans a destroyed registered owner's routing without transferring its pendi
 	expect(state.flows.has("flow_alice")).toBe(false);
 	controller.close();
 });
+
+it("preserves active unindexed registrations and prunes only exact verifiers missing after a peer deletion", async () => {
+	const { createAuthCallbackController } = await import("./authCallback");
+	const controller = createAuthCallbackController({
+		isDev: false,
+		focusApp: vi.fn(),
+		flowRoutingEnabled: true,
+	});
+	const alice = owner(1),
+		bob = owner(2);
+	state.windows = [alice, bob];
+	const aliceKey = "sb-recordlytest-auth-token-flow-flow_alice-code-verifier";
+	const bobKey = "sb-recordlytest-auth-token-flow-flow_bob00-code-verifier";
+	await controller.flowStorageHooks!.prepareSet(event(alice), aliceKey, []);
+	// No SDK index is present in this snapshot. The exact active slot suffices.
+	await controller.flowStorageHooks!.prepareSet(event(bob), bobKey, [aliceKey]);
+	expect(state.flows.has("flow_alice")).toBe(true);
+	expect(await controller.route(url)).toBe("local");
+	expect(alice.send).toHaveBeenCalledWith("auth:callback", url);
+	const nextKey = "sb-recordlytest-auth-token-flow-flow_next0-code-verifier";
+	await controller.flowStorageHooks!.prepareSet(event(bob), nextKey, [bobKey]);
+	expect(state.flows.has("flow_alice")).toBe(false);
+	expect(state.flows.has("flow_bob00")).toBe(true);
+	expect(state.flows.has("flow_next0")).toBe(true);
+	controller.close();
+});
+
+it("does not release an owned flow when another project removes the same textual flow ID", async () => {
+	const { createAuthCallbackController } = await import("./authCallback");
+	const controller = createAuthCallbackController({
+		isDev: false,
+		focusApp: vi.fn(),
+		flowRoutingEnabled: true,
+	});
+	const alice = owner(1);
+	state.windows = [alice];
+	await controller.flowStorageHooks!.prepareSet(event(alice), key, []);
+	await controller.flowStorageHooks!.didRemove(
+		event(alice),
+		"sb-other-auth-token-flow-flow_alice-code-verifier",
+	);
+	expect(state.flows.has("flow_alice")).toBe(true);
+	expect(await controller.route(url)).toBe("local");
+	controller.close();
+});

electron/authStorage.test.ts

@@ -206,7 +206,11 @@ it("binds the installed SDK's actual redirect flow ID to its trusted initiating
 	const redirect = new URL(new URL(result.data.url!).searchParams.get("redirect_to")!);
 	const flowId = redirect.searchParams.get("sb_flow_id");
 	expect(flowId).toMatch(/^[A-Za-z0-9_-]{8,64}$/);
-	expect(prepared).toHaveBeenCalledWith(event, `${storageKey}-flow-${flowId}-code-verifier`);
+	expect(prepared).toHaveBeenCalledWith(
+		event,
+		`${storageKey}-flow-${flowId}-code-verifier`,
+		expect.any(Array),
+	);
 	expect(await invoke("get", `${storageKey}-flow-${flowId}-code-verifier`)).toBeTruthy();
 	await invoke("remove", `${storageKey}-flow-${flowId}-code-verifier`);
 	expect(removed).toHaveBeenCalledWith(event, `${storageKey}-flow-${flowId}-code-verifier`);
@@ -367,7 +371,23 @@ it("bounds per-account consent retries without evicting sessions, verifiers, or
 		expect(await invoke("get", `${prefix}-signup-consent-retry:account-${n}`)).toBe(
 			`retry-${n}`,
 		);
-	for (const [key, value] of entries) expect(await invoke("get", key)).toBe(value);
+	for (const [key, value] of entries) {
+		if (key === `${prefix}-flows-code-verifier`)
+			expect(JSON.parse((await invoke("get", key)) as string)).toEqual([
+				"pending00",
+				"pending01",
+				"pending02",
+				"pending03",
+				"pending04",
+			]);
+		else expect(await invoke("get", key)).toBe(value);
+	}
+	// Repair is read-only: the original bookkeeping value was not replaced.
+	expect(
+		JSON.parse(
+			await fs.readFile(path.join(native.directory, "private-auth/session.enc"), "utf8"),
+		).entries[`${prefix}-flows-code-verifier`],
+	).toBe("flow-index");
 });
 
 it.each([
@@ -399,3 +419,186 @@ it("fails closed on an externally corrupted retry count without replacing the en
 	await expect(invoke("set", prefix, "must-not-replace")).rejects.toThrow("Could not unlock");
 	expect(await fs.readFile(file, "utf8")).toBe(corrupt);
 });
+
+it("repairs a lost flow-index update from exact persisted slots without deleting active verifiers", async () => {
+	const prefix = "sb-recordlytest-auth-token";
+	const first = `${prefix}-flow-flow_alice-code-verifier`;
+	const second = `${prefix}-flow-flow_bob00-code-verifier`;
+	const other = "sb-other-auth-token-flow-flow_other-code-verifier";
+	await invoke("set", first, '"alice-verifier"');
+	await invoke("set", second, '"bob-verifier"');
+	await invoke("set", other, '"other-verifier"');
+	await invoke(
+		"set",
+		`${prefix}-flows-code-verifier`,
+		JSON.stringify(["flow_bob00", "flow_absent", "flow_bob00"]),
+	);
+	expect(JSON.parse((await invoke("get", `${prefix}-flows-code-verifier`)) as string)).toEqual([
+		"flow_bob00",
+		"flow_alice",
+	]);
+	expect(await invoke("get", first)).toBe('"alice-verifier"');
+	expect(await invoke("get", second)).toBe('"bob-verifier"');
+	expect(await invoke("get", other)).toBe('"other-verifier"');
+	await invoke("set", `${prefix}-flows-code-verifier`, "invalid JSON");
+	expect(JSON.parse((await invoke("get", `${prefix}-flows-code-verifier`)) as string)).toEqual([
+		"flow_alice",
+		"flow_bob00",
+	]);
+});
+
+it("keeps nested project namespaces out of another project's repaired flow index", async () => {
+	const prefix = "sb-a-auth-token";
+	const nestedPrefix = `${prefix}-flow-nested-auth-token`;
+	const ownKey = `${prefix}-flow-current0-code-verifier`;
+	const nestedKey = `${nestedPrefix}-flow-abcdefgh-code-verifier`;
+	await invoke("set", ownKey, '"own-verifier"');
+	await invoke("set", nestedKey, '"nested-project-verifier"');
+	// The broad flow-ID grammar could treat this whole suffix as one ID. Use
+	// the same full greedy project-prefix parser as native route ownership.
+	await invoke(
+		"set",
+		`${prefix}-flows-code-verifier`,
+		JSON.stringify(["current0", "nested-auth-token-flow-abcdefgh"]),
+	);
+	expect(JSON.parse((await invoke("get", `${prefix}-flows-code-verifier`)) as string)).toEqual([
+		"current0",
+	]);
+	expect(
+		JSON.parse((await invoke("get", `${nestedPrefix}-flows-code-verifier`)) as string),
+	).toEqual(["abcdefgh"]);
+	expect(await invoke("get", nestedKey)).toBe('"nested-project-verifier"');
+	expect(await invoke("get", ownKey)).toBe('"own-verifier"');
+});
+
+it("does not register against an empty snapshot when saved auth cannot be read", async () => {
+	const prepare = vi.fn(async () => undefined);
+	const { registerAuthStorageHandlers } = await import("./authStorage");
+	registerAuthStorageHandlers({ prepareSet: prepare, didRemove: async () => undefined });
+	const prefix = "sb-recordlytest-auth-token";
+	await invoke("set", prefix, "session");
+	prepare.mockClear();
+	const file = path.join(native.directory, "private-auth/session.enc");
+	await fs.writeFile(file, "corrupt saved credentials");
+	await expect(
+		invoke("set", `${prefix}-flow-flow_alice-code-verifier`, '"verifier"'),
+	).rejects.toThrow("Could not unlock");
+	expect(prepare).not.toHaveBeenCalled();
+	expect(await fs.readFile(file, "utf8")).toBe("corrupt saved credentials");
+});
+
+it("holds the local registration queue through verifier commit before providing the next snapshot", async () => {
+	const prepare = vi.fn(
+		async (_event: unknown, _key: string, _keys?: readonly string[]) => undefined,
+	);
+	const { registerAuthStorageHandlers } = await import("./authStorage");
+	registerAuthStorageHandlers({ prepareSet: prepare, didRemove: async () => undefined });
+	const firstKey = "sb-recordlytest-auth-token-flow-flow_alice-code-verifier";
+	const secondKey = "sb-recordlytest-auth-token-flow-flow_bob00-code-verifier";
+	let release!: () => void;
+	let entered!: () => void;
+	const committing = new Promise<void>((resolve) => {
+		entered = resolve;
+	});
+	const held = new Promise<void>((resolve) => {
+		release = resolve;
+	});
+	const rename = fs.rename.bind(fs);
+	let paused = false;
+	vi.spyOn(fs, "rename").mockImplementation(async (...args) => {
+		if (
+			!paused &&
+			String(args[1]) === path.join(native.directory, "private-auth/session.enc")
+		) {
+			paused = true;
+			entered();
+			await held;
+		}
+		return rename(...args);
+	});
+	const first = invoke("set", firstKey, '"alice-verifier"');
+	await committing;
+	const second = invoke("set", secondKey, '"bob-verifier"');
+	try {
+		await new Promise<void>((resolve) => setImmediate(resolve));
+		expect(prepare).toHaveBeenCalledOnce();
+		expect(prepare.mock.calls[0][2]).toEqual([]);
+		release();
+		await Promise.all([first, second]);
+		expect(prepare.mock.calls[1][2]).toContain(firstKey);
+		expect(await invoke("get", firstKey)).toBe('"alice-verifier"');
+	} finally {
+		release();
+		await Promise.allSettled([first, second]);
+	}
+});
+
+it.each([
+	"same-client",
+	"two-renderers",
+])("keeps actual SDK concurrent starts in the native effective index (%s)", async (mode) => {
+	const prefix = "sb-recordlytest-auth-token",
+		indexKey = `${prefix}-flows-code-verifier`;
+	let held = false,
+		reads = 0,
+		release!: () => void,
+		gate: Promise<void>;
+	const makeClient = () =>
+		createClient("https://recordlytest.supabase.co", "public-fixture", {
+			auth: {
+				storageKey: prefix,
+				flowType: "pkce",
+				persistSession: true,
+				autoRefreshToken: false,
+				detectSessionInUrl: false,
+				experimental: { appendPkceFlowIdToRedirects: true },
+				storage: {
+					getItem: async (key) => {
+						const snapshot = (await invoke("get", key)) as string | null;
+						if (held && key === indexKey) {
+							if (++reads === 2) {
+								held = false;
+								release();
+							}
+							await gate;
+						}
+						return snapshot;
+					},
+					setItem: async (key, value) => {
+						await invoke("set", key, value);
+					},
+					removeItem: async (key) => {
+						await invoke("remove", key);
+					},
+				},
+			},
+		});
+	const a = makeClient(),
+		b = mode === "same-client" ? a : makeClient();
+	await Promise.all([a.auth.initialize(), b.auth.initialize()]);
+	for (let pair = 0; pair < 5; pair++) {
+		reads = 0;
+		gate = new Promise<void>((resolve) => {
+			release = resolve;
+		});
+		held = true;
+		const results = await Promise.all(
+			[a, b].map((client) =>
+				client.auth.signInWithOAuth({
+					provider: "apple",
+					options: { redirectTo: "recordly://auth/callback", skipBrowserRedirect: true },
+				}),
+			),
+		);
+		expect(results.every((result) => result.error === null)).toBe(true);
+		const entries = JSON.parse(
+			await fs.readFile(path.join(native.directory, "private-auth/session.enc"), "utf8"),
+		).entries;
+		const slots = Object.keys(entries).filter((key) => key.startsWith(`${prefix}-flow-`));
+		const index = JSON.parse((await invoke("get", indexKey)) as string);
+		expect(index).toHaveLength(slots.length);
+		expect(index.length).toBeLessThanOrEqual(5);
+		for (const key of slots)
+			expect(index).toContain(key.slice(`${prefix}-flow-`.length, -"-code-verifier".length));
+	}
+});

electron/authStorage.ts

@@ -24,6 +24,7 @@ export type AuthFlowStorageHooks = {
 	prepareSet: (
 		event: IpcMainInvokeEvent,
 		key: string,
+		persistedKeys?: readonly string[],
 	) => Promise<(() => Promise<void>) | undefined>;
 	didRemove: (event: IpcMainInvokeEvent, key: string) => Promise<void>;
 };
@@ -103,6 +104,34 @@ function validateEntries(value: unknown): AuthEntries {
 	return result;
 }
 
+function readStoredValue(entries: AuthEntries, key: string): string | null {
+	const prefix = /^(sb-[a-z0-9-]{1,100}-auth-token)-flows-code-verifier$/.exec(key)?.[1];
+	if (!prefix) return entries[key] ?? null;
+	const present = new Set<string>();
+	for (const candidate of Object.keys(entries)) {
+		const slot =
+			/^(sb-[a-z0-9-]{1,100}-auth-token)-flow-([A-Za-z0-9_-]{8,64})-code-verifier$/.exec(
+				candidate,
+			);
+		if (slot?.[1] === prefix) present.add(slot[2]);
+	}
+	let stored: unknown;
+	try {
+		stored = JSON.parse(entries[key] ?? "null");
+	} catch {
+		// The index is bookkeeping; exact persisted verifier slots are authoritative.
+	}
+	const ids = new Set<string>(
+		Array.isArray(stored)
+			? stored.filter((id) => typeof id === "string" && present.has(id))
+			: [],
+	);
+	for (const id of present) ids.add(id);
+	// SDK starts race their index read/modify/write. Repair reads without deleting
+	// any active slot; only the SDK's normal explicit ring removals evict verifiers.
+	return ids.size || key in entries ? JSON.stringify([...ids]) : null;
+}
+
 function storagePath(): string {
 	return path.join(app.getPath("userData"), "private-auth", "session.enc");
 }
@@ -170,7 +199,7 @@ function transaction<T>(operation: () => Promise<T>): Promise<T> {
 
 export function readNativeAuthStorageValue(key: string): Promise<string | null> {
 	assertKey(key);
-	return serialize(() => transaction(async () => (await load()).entries[key] ?? null));
+	return serialize(() => transaction(async () => readStoredValue((await load()).entries, key)));
 }
 
 export function registerAuthStorageHandlers(flowHooks?: AuthFlowStorageHooks): void {
@@ -194,7 +223,7 @@ export function registerAuthStorageHandlers(flowHooks?: AuthFlowStorageHooks): v
 		return serialize(async () =>
 			completedCallback?.key === key
 				? completedCallback.fingerprint
-				: transaction(async () => (await load()).entries[key] ?? null),
+				: transaction(async () => readStoredValue((await load()).entries, key)),
 		);
 	});
 	ipcMain.handle("auth-storage:set", (event, key: unknown, value: unknown) => {
@@ -202,7 +231,15 @@ export function registerAuthStorageHandlers(flowHooks?: AuthFlowStorageHooks): v
 		assertKey(key);
 		const validated = validateEntries({ [key]: value });
 		return serialize(async () => {
-			const rollback = await flowHooks?.prepareSet(event, key);
+			// This process's queue spans snapshot, registration and commit/rollback.
+			// Only queued prepareSet calls add owned routes, so no earlier own route
+			// can still be preparing its verifier when this snapshot is captured.
+			const persistedKeys =
+				flowHooks &&
+				/^sb-[a-z0-9-]{1,100}-auth-token-flow-[A-Za-z0-9_-]{8,64}-code-verifier$/.test(key)
+					? await transaction(async () => Object.keys((await load()).entries))
+					: undefined;
+			const rollback = await flowHooks?.prepareSet(event, key, persistedKeys);
 			if (COMPLETED_CALLBACK_KEY.test(key)) {
 				// Keep one non-secret completion marker available to all our windows even
 				// if optional bookkeeping cannot be persisted after a successful exchange.

tests/ui/capture-picker.spec.ts

@@ -590,7 +590,7 @@ for (const theme of ["light", "dark"] as const) {
 			page.getByText(/Click a window|drag an area|Move or resize the area|Esc to cancel/i),
 		).toHaveCount(0);
 		await page.mouse.click(600, 300);
-		expect(await pick(page)).toEqual({
+		await expect.poll(() => pick(page)).toEqual({
 			kind: "window",
 			windowId: "window:101:0",
 			displayId: 1,