fix: resolve second-round PR review findings

Harden storage fallback lock/error handling, preserve xhigh for GPT-5 legacy aliases mapped to GPT-5.4, and address review nits in tests/model map.

Author: ndycode

lib/request/helpers/model-map.ts

@@ -19,6 +19,7 @@ const DATED_ALIAS_EFFORT_SUFFIXES = [
 	"-high",
 	"-xhigh",
 ] as const;
+const GPT_54_SNAPSHOT_DATE = "2026-03-05" as const;
 
 function expandDatedAliases(prefix: string, target: string): Record<string, string> {
 	return Object.fromEntries(
@@ -82,7 +83,7 @@ export const MODEL_MAP: Record<string, string> = {
 	"gpt-5.4-medium": "gpt-5.4",
 	"gpt-5.4-high": "gpt-5.4",
 	"gpt-5.4-xhigh": "gpt-5.4",
-	...expandDatedAliases("gpt-5.4-2026-03-05", "gpt-5.4"),
+	...expandDatedAliases(`gpt-5.4-${GPT_54_SNAPSHOT_DATE}`, "gpt-5.4"),
 
 	// ============================================================================
 	// GPT-5.4 Pro Models (optional/manual config)
@@ -93,7 +94,7 @@ export const MODEL_MAP: Record<string, string> = {
 	"gpt-5.4-pro-medium": "gpt-5.4-pro",
 	"gpt-5.4-pro-high": "gpt-5.4-pro",
 	"gpt-5.4-pro-xhigh": "gpt-5.4-pro",
-	...expandDatedAliases("gpt-5.4-pro-2026-03-05", "gpt-5.4-pro"),
+	...expandDatedAliases(`gpt-5.4-pro-${GPT_54_SNAPSHOT_DATE}`, "gpt-5.4-pro"),
 
 	// ============================================================================
 	// GPT-5.2 Models (supports none/low/medium/high/xhigh per OpenAI API docs)

lib/request/request-transformer.ts

@@ -426,6 +426,7 @@ export function getReasoningConfig(
 	userConfig: ConfigOptions = {},
 ): ReasoningConfig {
 	const normalizedName = modelName?.toLowerCase() ?? "";
+	const canonicalModelName = normalizeModel(modelName);
 
 	// Canonical GPT-5 Codex (stable) defaults to high and does not support "none".
 	const isGpt5Codex =
@@ -456,6 +457,10 @@ export function getReasoningConfig(
 	const isGpt52General =
 		(normalizedName.includes("gpt-5.2") || normalizedName.includes("gpt 5.2")) &&
 		!isGpt52Codex;
+	const canonicalSupportsXhigh =
+		canonicalModelName === "gpt-5.4" ||
+		canonicalModelName === "gpt-5.4-pro" ||
+		canonicalModelName === "gpt-5.2";
 	const isCodexMax =
 		normalizedName.includes("codex-max") ||
 		normalizedName.includes("codex max");
@@ -487,7 +492,12 @@ export function getReasoningConfig(
 
 	// GPT-5.4/5.2 general, GPT-5.4 Pro, legacy GPT-5.2/5.3 Codex aliases, and Codex Max support xhigh reasoning
 	const supportsXhigh =
-		isGpt54General || isGpt54Pro || isGpt52General || isGpt53Codex || isGpt52Codex || isCodexMax;
+		isGpt54General ||
+		isGpt54Pro ||
+		isGpt52General ||
+		isGpt53Codex ||
+		isGpt52Codex ||
+		isCodexMax;
 
 	// GPT 5.1/5.2/5.4 general support "none" reasoning per:
 	// - OpenAI API docs: "gpt-5.1 defaults to none, supports: none, low, medium, high"
@@ -496,7 +506,10 @@ export function getReasoningConfig(
 	// - Codex CLI: docs/config.md lists "none" as valid for model_reasoning_effort
 	// - gpt-5.2 and gpt-5.4 general models support: none, low, medium, high, xhigh
 	// - Codex/Pro models (including GPT-5 Codex, GPT-5.4 Pro, and legacy GPT-5.3/5.2 Codex aliases) do NOT support "none"
-	const supportsNone = isGpt54General || isGpt52General || isGpt51General;
+	const supportsNone =
+		isGpt54General ||
+		isGpt52General ||
+		isGpt51General;
 
 	// Default based on model type (Codex CLI defaults + plugin opinionated tuning)
 	// Note: OpenAI docs say gpt-5.1 defaults to "none", but we default to "medium"
@@ -531,7 +544,9 @@ export function getReasoningConfig(
 	}
 
 	// For models that don't support xhigh, downgrade to high
-	if (!supportsXhigh && effort === "xhigh") {
+	// Legacy aliases like gpt-5-mini/gpt-5-nano normalize to gpt-5.4, which supports xhigh.
+	const supportsRequestedXhigh = supportsXhigh || canonicalSupportsXhigh;
+	if (!supportsRequestedXhigh && effort === "xhigh") {
 		effort = "high";
 	}
 

lib/storage.ts

@@ -676,7 +676,7 @@ export function normalizeAccountStorage(data: unknown): AccountStorageV3 | null
  * @returns AccountStorageV3 if file exists and is valid, null otherwise
  */
 export async function loadAccounts(): Promise<AccountStorageV3 | null> {
-  return loadAccountsInternal(saveAccounts);
+  return withStorageLock(async () => loadAccountsInternal(saveAccountsUnlocked));
 }
 
 function getGlobalAccountsStoragePath(): string {
@@ -775,8 +775,16 @@ async function loadAccountsInternal(
           try {
             await fs.access(seedPath);
             return;
-          } catch {
-            // Another concurrent caller has not seeded the project path yet.
+          } catch (accessError) {
+            const accessCode = (accessError as NodeJS.ErrnoException).code;
+            if (accessCode !== "ENOENT") {
+              log.warn("Failed to inspect project seed path before fallback seeding", {
+                path: seedPath,
+                error: String(accessError),
+              });
+              return;
+            }
+            // File is missing; proceed with seed write.
           }
 
           try {

test/codex-prompts.test.ts

@@ -44,7 +44,8 @@ describe("Codex Prompts Module", () => {
 
 			it("should be a readonly array", () => {
 				expect(Array.isArray(MODEL_FAMILIES)).toBe(true);
-				expect(MODEL_FAMILIES.length).toBe(7);
+				expect(MODEL_FAMILIES.length).toBeGreaterThanOrEqual(7);
+				expect(new Set(MODEL_FAMILIES).size).toBe(MODEL_FAMILIES.length);
 			});
 		});
 

test/request-transformer.test.ts

@@ -2102,6 +2102,32 @@ describe('Request Transformer Module', () => {
 					expect(result.model).toBe('gpt-5.4');  // legacy gpt-5 aliases now map to gpt-5.4
 					expect(result.reasoning?.effort).toBe('minimal');  // Lightweight gpt-5-mini defaults to minimal
 				});
+
+				it('should preserve xhigh effort for gpt-5-mini (normalized to gpt-5.4)', async () => {
+					const body: RequestBody = {
+						model: 'gpt-5-mini',
+						input: [],
+						reasoning: { effort: 'xhigh' },
+					};
+
+					const result = await transformRequestBody(body, codexInstructions);
+
+					expect(result.model).toBe('gpt-5.4');
+					expect(result.reasoning?.effort).toBe('xhigh');
+				});
+
+				it('should preserve xhigh effort for gpt-5-nano (normalized to gpt-5.4)', async () => {
+					const body: RequestBody = {
+						model: 'gpt-5-nano',
+						input: [],
+						reasoning: { effort: 'xhigh' },
+					};
+
+					const result = await transformRequestBody(body, codexInstructions);
+
+					expect(result.model).toBe('gpt-5.4');
+					expect(result.reasoning?.effort).toBe('xhigh');
+				});
 			});
 
 			describe('Scenario 2: Custom preset names (new style)', () => {

test/storage.test.ts

@@ -1921,6 +1921,66 @@ describe("storage", () => {
       }
     });
 
+    it("skips seed write when project path access fails with non-ENOENT error", async () => {
+      const fakeHome = join(testWorkDir, "home-fallback-access-error");
+      const projectDir = join(testWorkDir, "project-fallback-access-error");
+      const projectGitDir = join(projectDir, ".git");
+      const globalConfigDir = join(fakeHome, ".opencode");
+      const globalStoragePath = join(globalConfigDir, "openai-codex-accounts.json");
+
+      await fs.mkdir(fakeHome, { recursive: true });
+      await fs.mkdir(projectGitDir, { recursive: true });
+      await fs.mkdir(globalConfigDir, { recursive: true });
+      process.env.HOME = fakeHome;
+      process.env.USERPROFILE = fakeHome;
+      setStoragePath(projectDir);
+
+      const globalStorage = {
+        version: 3,
+        activeIndex: 0,
+        accounts: [
+          {
+            refreshToken: "global-refresh-access-error",
+            accountId: "global-account-access-error",
+            addedAt: 1,
+            lastUsed: 1,
+          },
+        ],
+      };
+      await fs.writeFile(globalStoragePath, JSON.stringify(globalStorage), "utf-8");
+
+      const projectScopedPath = getStoragePath();
+      const originalAccess = fs.access.bind(fs);
+      const originalRename = fs.rename.bind(fs);
+      let projectSeedWriteCount = 0;
+
+      const accessSpy = vi.spyOn(fs, "access").mockImplementation(async (path, mode) => {
+        if (String(path) === projectScopedPath) {
+          const err = new Error("EACCES access failure") as NodeJS.ErrnoException;
+          err.code = "EACCES";
+          throw err;
+        }
+        return originalAccess(path as string, mode);
+      });
+
+      const renameSpy = vi.spyOn(fs, "rename").mockImplementation(async (sourcePath, destinationPath) => {
+        if (String(destinationPath) === projectScopedPath) {
+          projectSeedWriteCount += 1;
+        }
+        return originalRename(sourcePath, destinationPath);
+      });
+
+      try {
+        const loaded = await loadAccounts();
+        expect(loaded?.accounts[0]?.accountId).toBe("global-account-access-error");
+        expect(projectSeedWriteCount).toBe(0);
+        expect(existsSync(projectScopedPath)).toBe(false);
+      } finally {
+        accessSpy.mockRestore();
+        renameSpy.mockRestore();
+      }
+    });
+
     it("returns null when global fallback storage is corrupted", async () => {
       const fakeHome = join(testWorkDir, "home-fallback-corrupted");
       const projectDir = join(testWorkDir, "project-fallback-corrupted");