fix(codex): scope refresh lock acquisition and release to file identity

src/codex/account-store.ts

@@ -1,5 +1,5 @@
 import { createHash } from "node:crypto";
-import { closeSync, existsSync, readFileSync, mkdirSync, openSync, unlinkSync, writeFileSync } from "node:fs";
+import { closeSync, existsSync, fstatSync, readFileSync, mkdirSync, openSync, statSync, unlinkSync, writeFileSync } from "node:fs";
 import { join } from "node:path";
 import {
   ConfigMutationLockError,
@@ -654,7 +654,14 @@ function isRefreshLockStale(path: string): boolean {
     const parsed = JSON.parse(readFileSync(path, "utf-8")) as { acquiredAt?: unknown };
     return typeof parsed.acquiredAt !== "number" || Date.now() - parsed.acquiredAt > REFRESH_LOCK_STALE_MS;
   } catch {
-    return true;
+    // The owner creates the file and writes its metadata in two steps, so a live lock is
+    // briefly unreadable. Age the file itself instead of calling that window stale, which
+    // let a waiter delete a lock whose owner was still inside its critical section.
+    try {
+      return Date.now() - statSync(path).mtimeMs > REFRESH_LOCK_STALE_MS;
+    } catch {
+      return false;
+    }
   }
 }
 
@@ -690,9 +697,25 @@ export async function withCodexRefreshFileLock<T>(lockKey: string, signal: Abort
   try {
     return await fn();
   } finally {
-    if (fd != null) closeSync(fd);
+    // Release only the lock this call created. If a waiter reclaimed the path as stale and a
+    // new owner recreated it, unlinking by name would delete the live lock of that owner.
+    let owned: { dev: bigint; ino: bigint } | null = null;
+    if (fd != null) {
+      try {
+        const info = fstatSync(fd, { bigint: true });
+        if (info.dev >= 0n && info.ino > 0n) {
+          owned = { dev: info.dev, ino: info.ino };
+        }
+      } catch {
+        owned = null;
+      }
+      closeSync(fd);
+    }
     try {
-      unlinkSync(path);
+      const current = statSync(path, { bigint: true });
+      // An unreadable or unusable identity never authorizes removing the current path.
+      // Leave it for stale-lock recovery instead of deleting a possible replacement owner.
+      if (owned && current.dev === owned.dev && current.ino === owned.ino) unlinkSync(path);
     } catch (err) {
       if (errCode(err) !== "ENOENT") throw err;
     }

structure/catalog.md

@@ -241,6 +241,9 @@ Pool mode routes across main plus added Codex credentials. Key rules:
   generation it started from still holds; a lost race raises a generation-conflict error rather
   than overwriting the newer credential (`src/codex/account-store.ts`). Callers handle that error;
   they do not assume a silent retry.
+  The lock itself is identity-scoped: a not-yet-readable lock counts as held until it ages out,
+  and release requires a usable matching descriptor identity. Unknown identity leaves the path
+  for stale recovery; stat followed by unlink does not provide atomic compare-and-delete.
 - **Authentication identity, quota domain, and cache domain are tracked separately**
   (`src/routing/identity-domains.ts`). `classifyCredential` returns all three with provenance:
   `pool.credentialGroups` supplies operator-declared quota domains, a small built-in table

structure/providers/openai-tiers.md

@@ -428,6 +428,12 @@ Pool mode needs stable public names and a store that survives concurrent refresh
 - The credential store is generation-guarded and refresh-locked (`src/codex/account-store.ts`): a
   refresh persists only if the generation it started from still holds, and a lost race raises a
   generation-conflict error instead of overwriting the newer credential.
+  The lock is held and released by file identity rather than by path. A lock that exists but is
+  not yet readable counts as held until it ages past the stale window, because its owner creates
+  the file and writes its metadata as two steps, and a holder deletes the lock only while the
+  path still resolves to the file it created. If descriptor identity is unavailable or unusable,
+  release leaves the path for stale-lock recovery. The stat/unlink pair is not an atomic
+  compare-and-delete, so this check alone does not eliminate concurrent replacement races.
 
 ## Sidecars, management, and UI
 
@@ -560,8 +566,8 @@ consulted, and they run in `resolveCodexAccountForThreadDetailed` ahead of it. A
 with no recorded refusal is deliberately not a release path on its own — stickiness until the
 account actually refuses is intended — but it does surrender the binding as soon as a sibling with
 headroom exists. Unbound assignment is untouched and still takes the coolest eligible account,
-because a fresh request has no warm prefix to lose. `pool.cacheAffinity` remains the stronger
-opt-in, raising the bar from the threshold to genuine exhaustion.
+because a fresh request has no warm prefix to lose. `pool.cacheAffinity` is enabled by default,
+raising the bar from the threshold to genuine exhaustion.
 
 Two call sites need the rule — the live path in `reevaluateAffinityQuota` and the side-effect-free
 `previewReusableAffinityAccount` that subagent fallback reads — and they share one helper rather

tests/codex-integration/codex-account-store.test.ts

@@ -1,6 +1,7 @@
 import { describe, expect, test, beforeEach, afterEach, spyOn } from "bun:test";
 import { createHash } from "node:crypto";
-import { existsSync, mkdtempSync, readdirSync, readFileSync, unlinkSync, writeFileSync } from "node:fs";
+import * as fs from "node:fs";
+import { existsSync, mkdtempSync, readdirSync, readFileSync, renameSync, unlinkSync, writeFileSync } from "node:fs";
 import { tmpdir } from "node:os";
 import { join } from "node:path";
 import { setAsyncIcaclsRunnerForTests, setIcaclsRunnerForTests } from "../../src/lib/windows-secret-acl";
@@ -624,6 +625,72 @@ describe("codex-account-store CRUD", () => {
     }
   });
 
+  test("a refresh lock that is still being initialized is not reclaimed as stale", async () => {
+    const { getValidCodexToken, saveCodexAccountCredential } = await import("../../src/codex/account-store");
+    saveCodexAccountCredential("refresh-empty-lock", { accessToken: "old", refreshToken: "empty-r", expiresAt: 0, chatgptAccountId: "acc" });
+    // The owner creates the lock file and writes its metadata as two steps, so a live lock is
+    // briefly unreadable. Treating that window as stale let a waiter delete a lock whose owner
+    // was still inside its critical section, and both then ran the refresh.
+    const lockPath = refreshLockPathForToken("empty-r");
+    writeFileSync(lockPath, "");
+    let fetchCalls = 0;
+    const originalFetch = globalThis.fetch;
+    globalThis.fetch = (async () => {
+      fetchCalls += 1;
+      return new Response(JSON.stringify({ access_token: "new", expires_in: 3600 }), { status: 200 });
+    }) as typeof fetch;
+
+    try {
+      const pending = getValidCodexToken("refresh-empty-lock");
+      await new Promise(resolve => setTimeout(resolve, 200));
+      expect(existsSync(lockPath)).toBe(true);
+      expect(fetchCalls).toBe(0);
+      unlinkSync(lockPath);
+      const result = await pending;
+      expect(result.accessToken).toBe("new");
+      expect(fetchCalls).toBe(1);
+    } finally {
+      globalThis.fetch = originalFetch;
+    }
+  });
+
+  test("releasing a refresh lock leaves a lock another owner recreated in place", async () => {
+    const { withCodexRefreshFileLock } = await import("../../src/codex/account-store");
+    const lockKey = "recreated-owner";
+    const lockPath = join(TEST_DIR, `codex-refresh-${createHash("sha256").update(lockKey).digest("hex").slice(0, 32)}.lock`);
+    await withCodexRefreshFileLock(lockKey, new AbortController().signal, async () => {
+      // A waiter reclaimed this path and a second owner took it over while we held it.
+      renameSync(lockPath, `${lockPath}.reclaimed`);
+      writeFileSync(lockPath, JSON.stringify({ acquiredAt: Date.now(), pid: 999_001 }) + "\n");
+    });
+    expect(existsSync(lockPath)).toBe(true);
+    expect((JSON.parse(readFileSync(lockPath, "utf-8")) as { pid: number }).pid).toBe(999_001);
+    unlinkSync(lockPath);
+    unlinkSync(`${lockPath}.reclaimed`);
+  });
+
+  test("refresh release preserves the path when descriptor identity cannot be read", async () => {
+    const { withCodexRefreshFileLock } = await import("../../src/codex/account-store");
+    const lockKey = "unknown-owner";
+    const lockPath = join(TEST_DIR, `codex-refresh-${createHash("sha256").update(lockKey).digest("hex").slice(0, 32)}.lock`);
+    const original = fs.fstatSync;
+    let released = false;
+    const probe = spyOn(fs, "fstatSync").mockImplementation((...args: Parameters<typeof fs.fstatSync>) => {
+      if (released) throw new Error("identity probe unavailable");
+      return original(...args);
+    });
+    try {
+      await withCodexRefreshFileLock(lockKey, new AbortController().signal, async () => {
+        renameSync(lockPath, `${lockPath}.reclaimed`);
+        writeFileSync(lockPath, "replacement-owner");
+        released = true;
+      });
+      expect(readFileSync(lockPath, "utf8")).toBe("replacement-owner");
+    } finally {
+      probe.mockRestore();
+    }
+  });
+
   test("same refresh grant joins a live flight", async () => {
     const {
       getCodexAccountCredential,