Skip to content

Commit 610440b

Browse files
committed
fix: address PR review comments for GPT-5.4.3 support
Clarify fallback docs, harden fallback seeding under concurrent loads, and add regression coverage for storage fallback edge cases.
1 parent 4d5d478 commit 610440b

File tree

7 files changed

+185
-38
lines changed

7 files changed

+185
-38
lines changed

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ opencode run "Hello" --model=openai/gpt-5.4 --variant=medium
133133
| Model | Variants | Notes |
134134
|-------|----------|-------|
135135
| `gpt-5.4` | none, low, medium, high, xhigh | Latest GPT-5.4 with reasoning levels and 1,000,000 context window |
136-
| `gpt-5.4-pro` | low, medium, high, xhigh | Optional manual model for deeper reasoning; fallback default is `gpt-5.4-pro -> gpt-5.4` (also 1,000,000 context window) |
136+
| `gpt-5.4-pro` | low, medium, high, xhigh | Optional manual model for deeper reasoning; when `unsupportedCodexPolicy=fallback`, fallback includes `gpt-5.4-pro -> gpt-5.4` (also 1,000,000 context window) |
137137
| `gpt-5-codex` | low, medium, high | Canonical Codex model for code generation (default: high) |
138138
| `gpt-5.3-codex-spark` | low, medium, high, xhigh | Spark IDs are supported by the plugin, but access is entitlement-gated by account/workspace |
139139
| `gpt-5.1-codex-max` | low, medium, high, xhigh | Maximum context Codex |

lib/request/helpers/model-map.ts

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,21 @@
1111
* Key: The model ID as specified in opencode.json config
1212
* Value: The normalized model name to send to the API
1313
*/
14+
const DATED_ALIAS_EFFORT_SUFFIXES = [
15+
"",
16+
"-none",
17+
"-low",
18+
"-medium",
19+
"-high",
20+
"-xhigh",
21+
] as const;
22+
23+
function expandDatedAliases(prefix: string, target: string): Record<string, string> {
24+
return Object.fromEntries(
25+
DATED_ALIAS_EFFORT_SUFFIXES.map((suffix) => [`${prefix}${suffix}`, target]),
26+
);
27+
}
28+
1429
export const MODEL_MAP: Record<string, string> = {
1530
// ============================================================================
1631
// GPT-5 Codex Models (canonical stable family)
@@ -67,12 +82,7 @@ export const MODEL_MAP: Record<string, string> = {
6782
"gpt-5.4-medium": "gpt-5.4",
6883
"gpt-5.4-high": "gpt-5.4",
6984
"gpt-5.4-xhigh": "gpt-5.4",
70-
"gpt-5.4-2026-03-05": "gpt-5.4",
71-
"gpt-5.4-2026-03-05-none": "gpt-5.4",
72-
"gpt-5.4-2026-03-05-low": "gpt-5.4",
73-
"gpt-5.4-2026-03-05-medium": "gpt-5.4",
74-
"gpt-5.4-2026-03-05-high": "gpt-5.4",
75-
"gpt-5.4-2026-03-05-xhigh": "gpt-5.4",
85+
...expandDatedAliases("gpt-5.4-2026-03-05", "gpt-5.4"),
7686

7787
// ============================================================================
7888
// GPT-5.4 Pro Models (optional/manual config)
@@ -83,12 +93,7 @@ export const MODEL_MAP: Record<string, string> = {
8393
"gpt-5.4-pro-medium": "gpt-5.4-pro",
8494
"gpt-5.4-pro-high": "gpt-5.4-pro",
8595
"gpt-5.4-pro-xhigh": "gpt-5.4-pro",
86-
"gpt-5.4-pro-2026-03-05": "gpt-5.4-pro",
87-
"gpt-5.4-pro-2026-03-05-none": "gpt-5.4-pro",
88-
"gpt-5.4-pro-2026-03-05-low": "gpt-5.4-pro",
89-
"gpt-5.4-pro-2026-03-05-medium": "gpt-5.4-pro",
90-
"gpt-5.4-pro-2026-03-05-high": "gpt-5.4-pro",
91-
"gpt-5.4-pro-2026-03-05-xhigh": "gpt-5.4-pro",
96+
...expandDatedAliases("gpt-5.4-pro-2026-03-05", "gpt-5.4-pro"),
9297

9398
// ============================================================================
9499
// GPT-5.2 Models (supports none/low/medium/high/xhigh per OpenAI API docs)

lib/storage.ts

Lines changed: 32 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,7 @@ export function formatStorageErrorHint(error: unknown, path: string): string {
107107
}
108108

109109
let storageMutex: Promise<void> = Promise.resolve();
110+
let fallbackSeedMutex: Promise<void> = Promise.resolve();
110111

111112
function withStorageLock<T>(fn: () => Promise<T>): Promise<T> {
112113
const previousMutex = storageMutex;
@@ -117,6 +118,15 @@ function withStorageLock<T>(fn: () => Promise<T>): Promise<T> {
117118
return previousMutex.then(fn).finally(() => releaseLock());
118119
}
119120

121+
function withFallbackSeedLock<T>(fn: () => Promise<T>): Promise<T> {
122+
const previousMutex = fallbackSeedMutex;
123+
let releaseLock: () => void;
124+
fallbackSeedMutex = new Promise<void>((resolve) => {
125+
releaseLock = resolve;
126+
});
127+
return previousMutex.then(fn).finally(() => releaseLock());
128+
}
129+
120130
const WINDOWS_RENAME_RETRY_ATTEMPTS = 5;
121131
const WINDOWS_RENAME_RETRY_BASE_DELAY_MS = 10;
122132
const PRE_IMPORT_BACKUP_WRITE_TIMEOUT_MS = 3_000;
@@ -760,18 +770,28 @@ async function loadAccountsInternal(
760770
if (!globalFallback) return null;
761771

762772
if (persistMigration) {
763-
try {
764-
await persistMigration(globalFallback);
765-
log.info("Seeded project account storage from global fallback", {
766-
path: getStoragePath(),
767-
accounts: globalFallback.accounts.length,
768-
});
769-
} catch (persistError) {
770-
log.warn("Failed to seed project storage from global fallback", {
771-
path: getStoragePath(),
772-
error: String(persistError),
773-
});
774-
}
773+
await withFallbackSeedLock(async () => {
774+
const seedPath = getStoragePath();
775+
try {
776+
await fs.access(seedPath);
777+
return;
778+
} catch {
779+
// Another concurrent caller has not seeded the project path yet.
780+
}
781+
782+
try {
783+
await persistMigration(globalFallback);
784+
log.info("Seeded project account storage from global fallback", {
785+
path: seedPath,
786+
accounts: globalFallback.accounts.length,
787+
});
788+
} catch (persistError) {
789+
log.warn("Failed to seed project storage from global fallback", {
790+
path: seedPath,
791+
error: String(persistError),
792+
});
793+
}
794+
});
775795
}
776796

777797
return globalFallback;

test/codex-prompts.test.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -501,6 +501,9 @@ describe("Codex Prompts Module", () => {
501501
const writeTargets = mockedWriteFile.mock.calls.map(([target]) => String(target));
502502
expect(rawGitHubCall?.[0]).toContain("gpt_5_2_prompt.md");
503503
expect(writeTargets.some((target) => target.includes("gpt-5.4-pro-instructions.md"))).toBe(true);
504+
expect(
505+
writeTargets.some((target) => /gpt-5\.4-instructions\.md$/.test(target)),
506+
).toBe(false);
504507
});
505508

506509
it("should map gpt-5.3-codex-spark prompts to the current codex prompt file", async () => {

test/edge-cases.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ describe("Edge Cases and Boundary Conditions", () => {
4242
// Mixed separators may miss explicit 5.4/pro patterns and fall through to generic GPT-5 fallback.
4343
expect(normalizeModel("gpt_5.4-high")).toBe("gpt-5.4");
4444
expect(normalizeModel("gpt-5_4 pro")).toBe("gpt-5.4");
45+
expect(normalizeModel("gpt-5.4-pro-high")).toBe("gpt-5.4-pro");
4546
});
4647

4748
it("should handle models with numeric-only names", () => {

test/gpt54-models.test.ts

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ import { normalizeModel, getReasoningConfig } from "../lib/request/request-trans
88
* Tests cover model normalization, family detection, and reasoning configuration
99
*/
1010
describe("GPT-5.4 Model Support", () => {
11+
const SNAPSHOT = "2026-03-05";
12+
1113
describe("GPT-5.4 Model Normalization", () => {
1214
it("should normalize gpt-5.4 base model", () => {
1315
expect(normalizeModel("gpt-5.4")).toBe("gpt-5.4");
@@ -52,14 +54,14 @@ describe("GPT-5.4 Model Support", () => {
5254
expect(MODEL_MAP["gpt-5.4-medium"]).toBe("gpt-5.4");
5355
expect(MODEL_MAP["gpt-5.4-high"]).toBe("gpt-5.4");
5456
expect(MODEL_MAP["gpt-5.4-xhigh"]).toBe("gpt-5.4");
55-
expect(MODEL_MAP["gpt-5.4-2026-03-05"]).toBe("gpt-5.4");
56-
expect(MODEL_MAP["gpt-5.4-2026-03-05-low"]).toBe("gpt-5.4");
57+
expect(MODEL_MAP[`gpt-5.4-${SNAPSHOT}`]).toBe("gpt-5.4");
58+
expect(MODEL_MAP[`gpt-5.4-${SNAPSHOT}-low`]).toBe("gpt-5.4");
5759
});
5860

5961
it("should normalize GPT-5.4 snapshot aliases", () => {
60-
expect(normalizeModel("gpt-5.4-2026-03-05")).toBe("gpt-5.4");
61-
expect(normalizeModel("gpt-5.4-2026-03-05-high")).toBe("gpt-5.4");
62-
expect(getNormalizedModel("gpt-5.4-2026-03-05-medium")).toBe("gpt-5.4");
62+
expect(normalizeModel(`gpt-5.4-${SNAPSHOT}`)).toBe("gpt-5.4");
63+
expect(normalizeModel(`gpt-5.4-${SNAPSHOT}-high`)).toBe("gpt-5.4");
64+
expect(getNormalizedModel(`gpt-5.4-${SNAPSHOT}-medium`)).toBe("gpt-5.4");
6365
});
6466
});
6567

@@ -107,14 +109,14 @@ describe("GPT-5.4 Model Support", () => {
107109
expect(MODEL_MAP["gpt-5.4-pro-medium"]).toBe("gpt-5.4-pro");
108110
expect(MODEL_MAP["gpt-5.4-pro-high"]).toBe("gpt-5.4-pro");
109111
expect(MODEL_MAP["gpt-5.4-pro-xhigh"]).toBe("gpt-5.4-pro");
110-
expect(MODEL_MAP["gpt-5.4-pro-2026-03-05"]).toBe("gpt-5.4-pro");
111-
expect(MODEL_MAP["gpt-5.4-pro-2026-03-05-high"]).toBe("gpt-5.4-pro");
112+
expect(MODEL_MAP[`gpt-5.4-pro-${SNAPSHOT}`]).toBe("gpt-5.4-pro");
113+
expect(MODEL_MAP[`gpt-5.4-pro-${SNAPSHOT}-high`]).toBe("gpt-5.4-pro");
112114
});
113115

114116
it("should normalize GPT-5.4-pro snapshot aliases", () => {
115-
expect(normalizeModel("gpt-5.4-pro-2026-03-05")).toBe("gpt-5.4-pro");
116-
expect(normalizeModel("gpt-5.4-pro-2026-03-05-high")).toBe("gpt-5.4-pro");
117-
expect(getNormalizedModel("gpt-5.4-pro-2026-03-05-medium")).toBe("gpt-5.4-pro");
117+
expect(normalizeModel(`gpt-5.4-pro-${SNAPSHOT}`)).toBe("gpt-5.4-pro");
118+
expect(normalizeModel(`gpt-5.4-pro-${SNAPSHOT}-high`)).toBe("gpt-5.4-pro");
119+
expect(getNormalizedModel(`gpt-5.4-pro-${SNAPSHOT}-medium`)).toBe("gpt-5.4-pro");
118120
});
119121

120122
it("should prioritize -pro suffix over base model", () => {
@@ -144,8 +146,8 @@ describe("GPT-5.4 Model Support", () => {
144146
it("should isolate gpt-5.4-pro family from gpt-5.4 base family", () => {
145147
expect(getModelFamily("gpt-5.4")).toBe("gpt-5.4");
146148
expect(getModelFamily("gpt-5.4-pro")).toBe("gpt-5.4-pro");
147-
expect(getModelFamily("gpt-5.4-2026-03-05")).toBe("gpt-5.4");
148-
expect(getModelFamily("gpt-5.4-pro-2026-03-05")).toBe("gpt-5.4-pro");
149+
expect(getModelFamily(`gpt-5.4-${SNAPSHOT}`)).toBe("gpt-5.4");
150+
expect(getModelFamily(`gpt-5.4-pro-${SNAPSHOT}`)).toBe("gpt-5.4-pro");
149151
});
150152

151153
it("should not confuse gpt-5.4 with gpt-5.3 or gpt-5.2", () => {
@@ -228,7 +230,7 @@ describe("GPT-5.4 Model Support", () => {
228230
expect(normalizeModel("gpt_5_4")).toBe("gpt-5.4");
229231
});
230232

231-
it("should not match gpt-5.4x patterns as gpt-5.4", () => {
233+
it("should route gpt-5.4x patterns through generic GPT-5 fallback to gpt-5.4", () => {
232234
// Boundary-aware matching prevents accidental 5.4 family match, then generic GPT-5 fallback applies.
233235
expect(normalizeModel("gpt-5.40")).toBe("gpt-5.4");
234236
expect(normalizeModel("gpt-5.44")).toBe("gpt-5.4");

test/storage.test.ts

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1824,6 +1824,122 @@ describe("storage", () => {
18241824
};
18251825
expect(seeded.accounts?.[0]?.accountId).toBe("global-account");
18261826
});
1827+
1828+
it("seeds project storage only once for concurrent global-fallback loads", async () => {
1829+
const fakeHome = join(testWorkDir, "home-fallback-concurrent");
1830+
const projectDir = join(testWorkDir, "project-fallback-concurrent");
1831+
const projectGitDir = join(projectDir, ".git");
1832+
const globalConfigDir = join(fakeHome, ".opencode");
1833+
const globalStoragePath = join(globalConfigDir, "openai-codex-accounts.json");
1834+
1835+
await fs.mkdir(fakeHome, { recursive: true });
1836+
await fs.mkdir(projectGitDir, { recursive: true });
1837+
await fs.mkdir(globalConfigDir, { recursive: true });
1838+
process.env.HOME = fakeHome;
1839+
process.env.USERPROFILE = fakeHome;
1840+
setStoragePath(projectDir);
1841+
1842+
const globalStorage = {
1843+
version: 3,
1844+
activeIndex: 0,
1845+
accounts: [
1846+
{
1847+
refreshToken: "global-refresh-concurrent",
1848+
accountId: "global-account-concurrent",
1849+
addedAt: 1,
1850+
lastUsed: 1,
1851+
},
1852+
],
1853+
};
1854+
await fs.writeFile(globalStoragePath, JSON.stringify(globalStorage), "utf-8");
1855+
1856+
const projectScopedPath = getStoragePath();
1857+
const originalRename = fs.rename.bind(fs);
1858+
let projectSeedWriteCount = 0;
1859+
const renameSpy = vi.spyOn(fs, "rename").mockImplementation(async (sourcePath, destinationPath) => {
1860+
if (String(destinationPath) === projectScopedPath) {
1861+
projectSeedWriteCount += 1;
1862+
}
1863+
return originalRename(sourcePath, destinationPath);
1864+
});
1865+
1866+
try {
1867+
const [first, second] = await Promise.all([loadAccounts(), loadAccounts()]);
1868+
expect(first?.accounts[0]?.accountId).toBe("global-account-concurrent");
1869+
expect(second?.accounts[0]?.accountId).toBe("global-account-concurrent");
1870+
expect(projectSeedWriteCount).toBe(1);
1871+
} finally {
1872+
renameSpy.mockRestore();
1873+
}
1874+
});
1875+
1876+
it("returns global fallback when project seed write fails", async () => {
1877+
const fakeHome = join(testWorkDir, "home-fallback-seed-fail");
1878+
const projectDir = join(testWorkDir, "project-fallback-seed-fail");
1879+
const projectGitDir = join(projectDir, ".git");
1880+
const globalConfigDir = join(fakeHome, ".opencode");
1881+
const globalStoragePath = join(globalConfigDir, "openai-codex-accounts.json");
1882+
1883+
await fs.mkdir(fakeHome, { recursive: true });
1884+
await fs.mkdir(projectGitDir, { recursive: true });
1885+
await fs.mkdir(globalConfigDir, { recursive: true });
1886+
process.env.HOME = fakeHome;
1887+
process.env.USERPROFILE = fakeHome;
1888+
setStoragePath(projectDir);
1889+
1890+
const globalStorage = {
1891+
version: 3,
1892+
activeIndex: 0,
1893+
accounts: [
1894+
{
1895+
refreshToken: "global-refresh-fail",
1896+
accountId: "global-account-fail",
1897+
addedAt: 1,
1898+
lastUsed: 1,
1899+
},
1900+
],
1901+
};
1902+
await fs.writeFile(globalStoragePath, JSON.stringify(globalStorage), "utf-8");
1903+
1904+
const projectScopedPath = getStoragePath();
1905+
const originalRename = fs.rename.bind(fs);
1906+
const renameSpy = vi.spyOn(fs, "rename").mockImplementation(async (sourcePath, destinationPath) => {
1907+
if (String(destinationPath) === projectScopedPath) {
1908+
const err = new Error("EPERM seed failure") as NodeJS.ErrnoException;
1909+
err.code = "EPERM";
1910+
throw err;
1911+
}
1912+
return originalRename(sourcePath, destinationPath);
1913+
});
1914+
1915+
try {
1916+
const loaded = await loadAccounts();
1917+
expect(loaded?.accounts[0]?.accountId).toBe("global-account-fail");
1918+
expect(existsSync(projectScopedPath)).toBe(false);
1919+
} finally {
1920+
renameSpy.mockRestore();
1921+
}
1922+
});
1923+
1924+
it("returns null when global fallback storage is corrupted", async () => {
1925+
const fakeHome = join(testWorkDir, "home-fallback-corrupted");
1926+
const projectDir = join(testWorkDir, "project-fallback-corrupted");
1927+
const projectGitDir = join(projectDir, ".git");
1928+
const globalConfigDir = join(fakeHome, ".opencode");
1929+
const globalStoragePath = join(globalConfigDir, "openai-codex-accounts.json");
1930+
1931+
await fs.mkdir(fakeHome, { recursive: true });
1932+
await fs.mkdir(projectGitDir, { recursive: true });
1933+
await fs.mkdir(globalConfigDir, { recursive: true });
1934+
process.env.HOME = fakeHome;
1935+
process.env.USERPROFILE = fakeHome;
1936+
setStoragePath(projectDir);
1937+
1938+
await fs.writeFile(globalStoragePath, "{ invalid json", "utf-8");
1939+
1940+
await expect(loadAccounts()).resolves.toBeNull();
1941+
expect(existsSync(getStoragePath())).toBe(false);
1942+
});
18271943
});
18281944

18291945
describe("saveAccounts EPERM/EBUSY retry logic", () => {

0 commit comments

Comments
 (0)