fix(init): serialize registration retries

Author: seanperez29

packages/cli-core/src/commands/init/ios/native-registration-retry.test.ts

@@ -1,5 +1,5 @@
 import { afterEach, describe, expect, test } from "bun:test";
-import { mkdtemp, readdir, readFile, rm, writeFile } from "node:fs/promises";
+import { mkdir, mkdtemp, readdir, readFile, rm, utimes, writeFile } from "node:fs/promises";
 import { tmpdir } from "node:os";
 import { join } from "node:path";
 import {
@@ -75,11 +75,23 @@ describe("iOS native registration retry state", () => {
     const target = identity();
     const first = await store.getOrCreate(target);
 
-    await store.clear(target);
+    expect(await store.clear(target, first)).toBe(true);
 
     expect(await store.getOrCreate(target)).not.toBe(first);
   });
 
+  test("does not let a delayed clear remove a newer registration generation", async () => {
+    const stateDirectory = await temporaryStateDirectory();
+    const store = createIOSNativeRegistrationRetryStore(() => stateDirectory);
+    const target = identity();
+    const first = await store.getOrCreate(target);
+    expect(await store.clear(target, first)).toBe(true);
+    const newer = await store.getOrCreate(target);
+
+    expect(await store.clear(target, first)).toBe(false);
+    expect(await store.peek(target)).toBe(newer);
+  });
+
   test("retains an old pending operation until remote verification clears it", async () => {
     const stateDirectory = await temporaryStateDirectory();
     const store = createIOSNativeRegistrationRetryStore(() => stateDirectory);
@@ -106,4 +118,25 @@ describe("iOS native registration retry state", () => {
 
     await expect(store.getOrCreate(target)).rejects.toThrow("retry record is malformed");
   });
+
+  test("fails closed without stealing an abandoned stale filesystem lock", async () => {
+    const stateDirectory = await temporaryStateDirectory();
+    const store = createIOSNativeRegistrationRetryStore(() => stateDirectory, {
+      lockRetryMs: 1,
+      lockTimeoutMs: 10,
+      lockStaleMs: 5,
+    });
+    const target = identity();
+    const first = await store.getOrCreate(target);
+    const directory = join(stateDirectory, "idempotency");
+    const [filename] = await readdir(directory);
+    const lock = join(directory, `${filename!}.lock`);
+    await mkdir(lock);
+    const stale = new Date(Date.now() - 60_000);
+    await utimes(lock, stale, stale);
+
+    expect(first).toStartWith("clerk-init-ios-registration-");
+    await expect(store.getOrCreate(target)).rejects.toThrow("lock is stale");
+    expect(await readdir(lock)).toEqual([]);
+  });
 });

packages/cli-core/src/commands/init/ios/native-registration-retry.ts

@@ -1,5 +1,5 @@
 import { createHash, randomUUID } from "node:crypto";
-import { mkdir, readFile, rmdir, unlink, writeFile } from "node:fs/promises";
+import { lstat, mkdir, readFile, rmdir, unlink, writeFile } from "node:fs/promises";
 import { setTimeout as sleep } from "node:timers/promises";
 import { dirname, join } from "node:path";
 import { getConfigFile } from "../../../lib/config.ts";
@@ -12,6 +12,15 @@ const IDEMPOTENCY_KEY_PATTERN =
   /^clerk-init-ios-registration-[0-9a-f]{8}-[0-9a-f]{4}-[1-8][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i;
 const CONCURRENT_WRITE_ATTEMPTS = 20;
 const CONCURRENT_WRITE_RETRY_MS = 5;
+const LOCK_RETRY_MS = 10;
+const LOCK_TIMEOUT_MS = 5_000;
+const LOCK_STALE_MS = 30_000;
+
+interface IOSNativeRegistrationRetryStoreOptions {
+  lockRetryMs?: number;
+  lockTimeoutMs?: number;
+  lockStaleMs?: number;
+}
 
 export interface IOSNativeRegistrationRetryIdentity {
   applicationId: string;
@@ -22,7 +31,8 @@ export interface IOSNativeRegistrationRetryIdentity {
 
 export interface IOSNativeRegistrationRetryStore {
   getOrCreate(identity: IOSNativeRegistrationRetryIdentity): Promise<string>;
-  clear(identity: IOSNativeRegistrationRetryIdentity): Promise<void>;
+  peek(identity: IOSNativeRegistrationRetryIdentity): Promise<string | undefined>;
+  clear(identity: IOSNativeRegistrationRetryIdentity, expectedKey: string): Promise<boolean>;
 }
 
 interface IOSNativeRegistrationRetryRecord {
@@ -69,6 +79,74 @@ function isExistingFile(error: unknown): boolean {
   return (error as NodeJS.ErrnoException).code === "EEXIST";
 }
 
+function lockPath(baseDirectory: string, identity: IOSNativeRegistrationRetryIdentity): string {
+  return `${retryPath(baseDirectory, identity)}.lock`;
+}
+
+async function acquireLock(
+  baseDirectory: string,
+  identity: IOSNativeRegistrationRetryIdentity,
+  options: Required<IOSNativeRegistrationRetryStoreOptions>,
+): Promise<string> {
+  await mkdir(retryDirectory(baseDirectory), { recursive: true, mode: 0o700 });
+  const path = lockPath(baseDirectory, identity);
+  const deadline = Date.now() + options.lockTimeoutMs;
+  while (true) {
+    try {
+      await mkdir(path, { mode: 0o700 });
+      return path;
+    } catch (error) {
+      if (!isExistingFile(error)) throw error;
+      if (Date.now() >= deadline) {
+        let stale = false;
+        try {
+          stale = Date.now() - (await lstat(path)).mtimeMs >= options.lockStaleMs;
+        } catch (statError) {
+          if (isMissingFile(statError)) continue;
+          throw statError;
+        }
+        throw new Error(
+          stale
+            ? `The Clerk iOS registration retry-state lock is stale and was left in place for safety: ${path}`
+            : "Timed out waiting for the Clerk iOS registration retry-state lock.",
+        );
+      }
+      await sleep(options.lockRetryMs);
+    }
+  }
+}
+
+async function withIdentityLock<T>(
+  baseDirectory: string,
+  identity: IOSNativeRegistrationRetryIdentity,
+  options: Required<IOSNativeRegistrationRetryStoreOptions>,
+  operation: () => Promise<T>,
+): Promise<T> {
+  const path = await acquireLock(baseDirectory, identity, options);
+  const release = async () => {
+    try {
+      await rmdir(path);
+    } catch (error) {
+      if (!isMissingFile(error)) throw error;
+    }
+  };
+  try {
+    const result = await operation();
+    await release();
+    return result;
+  } catch (operationError) {
+    try {
+      await release();
+    } catch (releaseError) {
+      throw new AggregateError(
+        [operationError, releaseError],
+        "The Clerk iOS registration retry operation and lock release both failed.",
+      );
+    }
+    throw operationError;
+  }
+}
+
 function isRetryRecord(
   value: unknown,
   identity: IOSNativeRegistrationRetryIdentity,
@@ -173,40 +251,64 @@ async function getOrCreateRetryKey(
 async function clearRetryKey(
   baseDirectory: string,
   identity: IOSNativeRegistrationRetryIdentity,
-): Promise<void> {
+  expectedKey: string,
+): Promise<boolean> {
+  const existing = await readRetryRecord(baseDirectory, identity);
+  if (!existing) return true;
+  if (existing.idempotencyKey !== expectedKey) return false;
   try {
     await unlink(retryPath(baseDirectory, identity));
   } catch (error) {
     if (!isMissingFile(error)) throw error;
-    return;
-  }
-
-  // Remove only the empty operational directory. Never remove other CLI state.
-  try {
-    await rmdir(retryDirectory(baseDirectory));
-  } catch {
-    // Another retry record is present, or another process started a retry.
+    return true;
   }
+  return true;
 }
 
 export function createIOSNativeRegistrationRetryStore(
   resolveBaseDirectory: () => string = () => dirname(getConfigFile()),
+  options: IOSNativeRegistrationRetryStoreOptions = {},
 ): IOSNativeRegistrationRetryStore {
+  const lockOptions: Required<IOSNativeRegistrationRetryStoreOptions> = {
+    lockRetryMs: options.lockRetryMs ?? LOCK_RETRY_MS,
+    lockTimeoutMs: options.lockTimeoutMs ?? LOCK_TIMEOUT_MS,
+    lockStaleMs: options.lockStaleMs ?? LOCK_STALE_MS,
+  };
   return {
     async getOrCreate(identity) {
       const baseDirectory = resolveBaseDirectory();
       const path = retryPath(baseDirectory, identity);
       return withHomeFsAccess(
         { operation: "write", target: path, label: "CLI idempotency state directory" },
-        async () => getOrCreateRetryKey(baseDirectory, identity),
+        async () =>
+          withIdentityLock(baseDirectory, identity, lockOptions, async () =>
+            getOrCreateRetryKey(baseDirectory, identity),
+          ),
       );
     },
-    async clear(identity) {
+    async peek(identity) {
       const baseDirectory = resolveBaseDirectory();
       const path = retryPath(baseDirectory, identity);
-      await withHomeFsAccess(
+      return withHomeFsAccess(
+        { operation: "read", target: path, label: "CLI idempotency state directory" },
+        async () =>
+          withIdentityLock(
+            baseDirectory,
+            identity,
+            lockOptions,
+            async () => (await readRetryRecord(baseDirectory, identity))?.idempotencyKey,
+          ),
+      );
+    },
+    async clear(identity, expectedKey) {
+      const baseDirectory = resolveBaseDirectory();
+      const path = retryPath(baseDirectory, identity);
+      return withHomeFsAccess(
         { operation: "delete", target: path, label: "CLI idempotency state directory" },
-        async () => clearRetryKey(baseDirectory, identity),
+        async () =>
+          withIdentityLock(baseDirectory, identity, lockOptions, async () =>
+            clearRetryKey(baseDirectory, identity, expectedKey),
+          ),
       );
     },
   };

packages/cli-core/src/commands/init/ios/native-remote.test.ts

@@ -117,8 +117,15 @@ function memoryRegistrationRetryStore(): {
         entries.set(key, created);
         return created;
       },
-      async clear(identity) {
-        entries.delete(scope(identity));
+      async peek(identity) {
+        return entries.get(scope(identity));
+      },
+      async clear(identity, expectedKey) {
+        const key = scope(identity);
+        const existing = entries.get(key);
+        if (existing && existing !== expectedKey) return false;
+        entries.delete(key);
+        return true;
       },
     },
     pending(identity) {
@@ -761,7 +768,7 @@ describe("Clerk Native Application remote setup", () => {
       message: expect.stringContaining("Xcode target identity changed"),
     });
 
-    expect(calls).toEqual(["GET native settings", "GET iOS registrations"]);
+    expect(calls).toEqual([]);
     expect(calls).not.toContain("POST iOS registration");
     expect(calls).not.toContain("PATCH native settings");
   });
@@ -785,7 +792,7 @@ describe("Clerk Native Application remote setup", () => {
       message: expect.stringContaining("Xcode target identity could not be rechecked"),
     });
 
-    expect(calls).toEqual(["GET native settings", "GET iOS registrations"]);
+    expect(calls).toEqual([]);
     expect(calls).not.toContain("POST iOS registration");
     expect(calls).not.toContain("PATCH native settings");
   });
@@ -802,7 +809,7 @@ describe("Clerk Native Application remote setup", () => {
       ),
     ).rejects.toMatchObject({ code: ERROR_CODE.IOS_SETUP_STALE });
 
-    expect(calls).toEqual(["GET native settings", "GET iOS registrations"]);
+    expect(calls).toEqual([]);
     expect(calls).not.toContain("POST iOS registration");
     expect(calls).not.toContain("PATCH native settings");
   });
@@ -943,6 +950,65 @@ describe("Clerk Native Application remote setup", () => {
     expect(retry.pending(registrationRetryIdentity())).toBeUndefined();
   });
 
+  test("rechecks remote state after a paused invocation acquires a newer retry generation", async () => {
+    const retry = memoryRegistrationRetryStore();
+    let releaseGet!: () => void;
+    const getGate = new Promise<void>((resolve) => {
+      releaseGet = resolve;
+    });
+    let reportPaused!: () => void;
+    const paused = new Promise<void>((resolve) => {
+      reportPaused = resolve;
+    });
+    const pausedStore: IOSNativeRegistrationRetryStore = {
+      async getOrCreate(identity) {
+        reportPaused();
+        await getGate;
+        return retry.store.getOrCreate(identity);
+      },
+      async peek(identity) {
+        return retry.store.peek(identity);
+      },
+      async clear(identity, expectedKey) {
+        return retry.store.clear(identity, expectedKey);
+      },
+    };
+    const exactRegistration = registration();
+    const resumed = scriptedAPI({
+      nativeReads: [nativeSettings(true), nativeSettings(true)],
+      registrationReads: [[exactRegistration], [exactRegistration]],
+    });
+
+    const resumedApply = applyRemoteSetup(
+      plan({ nativeApi: "satisfied", registration: "required" }),
+      resumed.api,
+      approvedTargetReader,
+      pausedStore,
+    );
+    await paused;
+
+    const first = scriptedAPI({
+      nativeReads: [nativeSettings(true), nativeSettings(true)],
+      registrationReads: [[], [exactRegistration]],
+    });
+    await applyRemoteSetup(
+      plan({ nativeApi: "satisfied", registration: "required" }),
+      first.api,
+      approvedTargetReader,
+      retry.store,
+    );
+    const completedKey = first.registrationIdempotencyKeys[0]!;
+    expect(retry.pending(registrationRetryIdentity())).toBeUndefined();
+
+    releaseGet();
+    await resumedApply;
+
+    expect(resumed.registrationIdempotencyKeys).toEqual([]);
+    expect(resumed.calls).not.toContain("POST iOS registration");
+    expect(retry.pending(registrationRetryIdentity())).toBeUndefined();
+    expect(completedKey).toStartWith("clerk-init-ios-registration-");
+  });
+
   test("reconciles an ambiguous Native API error when a re-read shows it enabled", async () => {
     const exactRegistration = registration();
     const ambiguousError = new Error("connection reset after enable");