From bd46022c4cc328b0e709be5058559fd4d90fb770 Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Sat, 8 Aug 2026 22:56:15 +0200 Subject: [PATCH 1/6] fix(codex): persist account deletion before cleanup --- src/codex/account-lifecycle.ts | 76 +++++++++++++++++++++++++++------- 1 file changed, 60 insertions(+), 16 deletions(-) diff --git a/src/codex/account-lifecycle.ts b/src/codex/account-lifecycle.ts index 7404089db..14d2a0d6c 100644 --- a/src/codex/account-lifecycle.ts +++ b/src/codex/account-lifecycle.ts @@ -1,3 +1,4 @@ +import { saveConfigPreservingClaudeCode, withConfigMutationLockSync } from "../config"; import { removeCodexAccountCredential } from "./account-store"; import { clearAccountNeedsReauth } from "./account-runtime-state"; import { getMainChatgptAccountId } from "./auth-collision"; @@ -13,6 +14,13 @@ import type { OcxConfig } from "../types"; let observedMainChatgptAccountId: string | undefined; +export class CodexAccountDeleteCleanupError extends Error { + constructor() { + super("Account deletion was saved, but local credential cleanup did not complete. Retry removal."); + this.name = "CodexAccountDeleteCleanupError"; + } +} + export function purgeCodexAccountRuntimeState(accountId: string): void { clearAccountNeedsReauth(accountId); clearAccountQuota(accountId); @@ -71,25 +79,61 @@ export function resetMainCodexAccountIdentityTrackingForTests(): void { clearMainAccountCredentialPresence(); } +function restoreRuntimeConfig(target: OcxConfig, snapshot: OcxConfig): void { + for (const key of Object.keys(target) as Array) delete target[key]; + Object.assign(target, snapshot); +} + /** * Delete a stored account while retaining its selector binding. + * + * Config deletion is committed before credentials or runtime state are destroyed. This prevents a + * failed config write from leaving a still-configured account with a tombstoned credential. The + * whole sequence shares the config mutation coordinator so a cooperating writer cannot re-add the + * account between the durable config commit and credential cleanup. + * * Returns true when a picker-visible row disappeared and the catalog must converge. */ export function deleteCodexAccount(runtimeConfig: OcxConfig, accountId: string): boolean { - const hadStoredAccount = (runtimeConfig.codexAccounts ?? []) - .some(account => !account.isMain && account.id === accountId); - const hadVisiblePickerBinding = hadStoredAccount - && codexAccountPickerEnabled(runtimeConfig) - && codexAccountNamespaceEntries(runtimeConfig) - .some(([, boundAccountId]) => boundAccountId === accountId); - removeCodexAccountCredential(accountId); - runtimeConfig.codexAccounts = (runtimeConfig.codexAccounts ?? []) - .filter(account => account.isMain || account.id !== accountId); - forgetCodexAccountPause(runtimeConfig, accountId); - forgetCodexAccountPriority(runtimeConfig, accountId); - clearCodexAccountPin(runtimeConfig, accountId); - if (runtimeConfig.activeCodexAccountId === accountId) runtimeConfig.activeCodexAccountId = undefined; - purgeCodexAccountRuntimeState(accountId); - invalidateCodexWebSocketsForAccount(accountId); - return hadVisiblePickerBinding; + let cleanupFailed = false; + const pickerVisibilityChanged = withConfigMutationLockSync(() => { + const previousConfig = structuredClone(runtimeConfig); + const hadStoredAccount = (runtimeConfig.codexAccounts ?? []) + .some(account => !account.isMain && account.id === accountId); + const hadVisiblePickerBinding = hadStoredAccount + && codexAccountPickerEnabled(runtimeConfig) + && codexAccountNamespaceEntries(runtimeConfig) + .some(([, boundAccountId]) => boundAccountId === accountId); + + runtimeConfig.codexAccounts = (runtimeConfig.codexAccounts ?? []) + .filter(account => account.isMain || account.id !== accountId); + forgetCodexAccountPause(runtimeConfig, accountId); + forgetCodexAccountPriority(runtimeConfig, accountId); + clearCodexAccountPin(runtimeConfig, accountId); + if (runtimeConfig.activeCodexAccountId === accountId) runtimeConfig.activeCodexAccountId = undefined; + + try { + // Persist first. Destructive cleanup below must never run for a deletion that did not become + // durable. The auth API's existing follow-up save is intentionally idempotent. + saveConfigPreservingClaudeCode(runtimeConfig); + } catch (error) { + restoreRuntimeConfig(runtimeConfig, previousConfig); + throw error; + } + + try { + removeCodexAccountCredential(accountId); + purgeCodexAccountRuntimeState(accountId); + invalidateCodexWebSocketsForAccount(accountId); + } catch { + // Do not throw through the mutation coordinator after config.json committed: that would roll + // back only the SQLite generation transaction, not the already-atomic file replacement. + cleanupFailed = true; + } + + return hadVisiblePickerBinding; + }); + + if (cleanupFailed) throw new CodexAccountDeleteCleanupError(); + return pickerVisibilityChanged; } From dae97e423e8eb6542124bfb3eae95decf4d3065d Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Sat, 8 Aug 2026 22:56:54 +0200 Subject: [PATCH 2/6] test(codex): cover account delete persistence ordering --- tests/codex-account-delete-atomicity.test.ts | 150 +++++++++++++++++++ 1 file changed, 150 insertions(+) create mode 100644 tests/codex-account-delete-atomicity.test.ts diff --git a/tests/codex-account-delete-atomicity.test.ts b/tests/codex-account-delete-atomicity.test.ts new file mode 100644 index 000000000..2fbdb2cab --- /dev/null +++ b/tests/codex-account-delete-atomicity.test.ts @@ -0,0 +1,150 @@ +import { afterEach, beforeEach, describe, expect, spyOn, test } from "bun:test"; +import { existsSync, mkdirSync, rmSync } from "node:fs"; +import { join } from "node:path"; +import * as accountStoreModule from "../src/codex/account-store"; +import { + getCodexAccountCredential, + saveCodexAccountCredential, +} from "../src/codex/account-store"; +import { + CodexAccountDeleteCleanupError, + deleteCodexAccount, +} from "../src/codex/account-lifecycle"; +import { + isAccountNeedsReauth, + markAccountNeedsReauth, +} from "../src/codex/account-runtime-state"; +import { + getAccountQuota, + updateAccountQuota, +} from "../src/codex/quota"; +import { loadConfig, saveConfig } from "../src/config"; +import * as configModule from "../src/config"; +import type { OcxConfig } from "../src/types"; + +const TEST_DIR = join(import.meta.dir, ".tmp-codex-account-delete-atomicity"); +const ACCOUNT_ID = "delete-atomicity"; +let previousHome: string | undefined; + +function seededConfig(): OcxConfig { + const config = loadConfig(); + config.codexAccounts = [{ + id: ACCOUNT_ID, + email: "delete-atomicity@example.test", + isMain: false, + }]; + config.codexAccountNamespaces = { stable: ACCOUNT_ID }; + config.codexAccountPickerEnabled = true; + config.pausedCodexAccountIds = [ACCOUNT_ID]; + config.codexAccountPriorities = { [ACCOUNT_ID]: 7 }; + config.activeCodexAccountPinned = ACCOUNT_ID; + config.activeCodexAccountId = ACCOUNT_ID; + saveConfig(config); + saveCodexAccountCredential(ACCOUNT_ID, { + accessToken: "delete-access", + refreshToken: "delete-refresh", + expiresAt: Date.now() + 60_000, + chatgptAccountId: "delete-chatgpt-id", + }); + markAccountNeedsReauth(ACCOUNT_ID); + updateAccountQuota(ACCOUNT_ID, 42); + return config; +} + +beforeEach(() => { + previousHome = process.env.OPENCODEX_HOME; + if (existsSync(TEST_DIR)) rmSync(TEST_DIR, { recursive: true }); + mkdirSync(TEST_DIR, { recursive: true }); + process.env.OPENCODEX_HOME = TEST_DIR; +}); + +afterEach(() => { + if (previousHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previousHome; + if (existsSync(TEST_DIR)) rmSync(TEST_DIR, { recursive: true }); +}); + +describe("Codex account delete persistence ordering", () => { + test("a config persistence failure leaves the account and destructive state intact", () => { + const config = seededConfig(); + const before = structuredClone(config); + const saveSpy = spyOn(configModule, "saveConfigPreservingClaudeCode") + .mockImplementation(() => { throw new Error("forced config write failure"); }); + + try { + expect(() => deleteCodexAccount(config, ACCOUNT_ID)).toThrow("forced config write failure"); + + expect(config).toEqual(before); + expect(loadConfig().codexAccounts?.some(account => account.id === ACCOUNT_ID)).toBe(true); + expect(getCodexAccountCredential(ACCOUNT_ID)).not.toBeNull(); + expect(isAccountNeedsReauth(ACCOUNT_ID)).toBe(true); + expect(getAccountQuota(ACCOUNT_ID)).not.toBeNull(); + } finally { + saveSpy.mockRestore(); + } + }); + + test("the durable config deletion happens before credential and runtime cleanup", () => { + const config = seededConfig(); + const realSave = configModule.saveConfigPreservingClaudeCode; + const saveSpy = spyOn(configModule, "saveConfigPreservingClaudeCode") + .mockImplementation(candidate => { + expect(getCodexAccountCredential(ACCOUNT_ID)).not.toBeNull(); + expect(isAccountNeedsReauth(ACCOUNT_ID)).toBe(true); + expect(getAccountQuota(ACCOUNT_ID)).not.toBeNull(); + realSave(candidate); + }); + + try { + expect(deleteCodexAccount(config, ACCOUNT_ID)).toBe(true); + } finally { + saveSpy.mockRestore(); + } + + const persisted = loadConfig(); + expect(persisted.codexAccounts?.some(account => account.id === ACCOUNT_ID)).toBe(false); + expect(persisted.codexAccountNamespaces).toEqual({ stable: ACCOUNT_ID }); + expect(config.codexAccounts?.some(account => account.id === ACCOUNT_ID)).toBe(false); + expect(config.pausedCodexAccountIds).toBeUndefined(); + expect(config.codexAccountPriorities).toBeUndefined(); + expect(config.activeCodexAccountPinned).toBeUndefined(); + expect(config.activeCodexAccountId).toBeUndefined(); + expect(getCodexAccountCredential(ACCOUNT_ID)).toBeNull(); + expect(isAccountNeedsReauth(ACCOUNT_ID)).toBe(false); + expect(getAccountQuota(ACCOUNT_ID)).toBeNull(); + }); + + test("a cleanup failure keeps the deletion durable and exposes only a fixed recovery error", () => { + const config = seededConfig(); + const removeSpy = spyOn(accountStoreModule, "removeCodexAccountCredential") + .mockImplementation(() => { + throw new Error("private cleanup detail /private/codex-accounts.json Bearer secret-token"); + }); + + try { + let thrown: unknown; + try { + deleteCodexAccount(config, ACCOUNT_ID); + } catch (error) { + thrown = error; + } + expect(thrown).toBeInstanceOf(CodexAccountDeleteCleanupError); + expect(String((thrown as Error).message)).toBe( + "Account deletion was saved, but local credential cleanup did not complete. Retry removal.", + ); + expect(String((thrown as Error).message)).not.toContain("private"); + expect(String((thrown as Error).message)).not.toContain("secret-token"); + expect(loadConfig().codexAccounts?.some(account => account.id === ACCOUNT_ID)).toBe(false); + expect(config.codexAccounts?.some(account => account.id === ACCOUNT_ID)).toBe(false); + expect(getCodexAccountCredential(ACCOUNT_ID)).not.toBeNull(); + } finally { + removeSpy.mockRestore(); + } + + // The route is retry-safe even after the durable row is gone: a second delete can finish the + // tombstone/runtime cleanup without recreating the account or selector mapping. + expect(deleteCodexAccount(config, ACCOUNT_ID)).toBe(false); + expect(getCodexAccountCredential(ACCOUNT_ID)).toBeNull(); + expect(loadConfig().codexAccountNamespaces).toEqual({ stable: ACCOUNT_ID }); + }); +}); From a8a76f02b147df73655e7f72c9594d361add657d Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Sat, 8 Aug 2026 23:16:21 +0200 Subject: [PATCH 3/6] fix(codex): keep transient account deletion side-effect free --- src/codex/account-lifecycle.ts | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/src/codex/account-lifecycle.ts b/src/codex/account-lifecycle.ts index 14d2a0d6c..15ce9e328 100644 --- a/src/codex/account-lifecycle.ts +++ b/src/codex/account-lifecycle.ts @@ -1,4 +1,5 @@ -import { saveConfigPreservingClaudeCode, withConfigMutationLockSync } from "../config"; +import { existsSync } from "node:fs"; +import { getConfigPath, saveConfigPreservingClaudeCode, withConfigMutationLockSync } from "../config"; import { removeCodexAccountCredential } from "./account-store"; import { clearAccountNeedsReauth } from "./account-runtime-state"; import { getMainChatgptAccountId } from "./auth-collision"; @@ -87,10 +88,11 @@ function restoreRuntimeConfig(target: OcxConfig, snapshot: OcxConfig): void { /** * Delete a stored account while retaining its selector binding. * - * Config deletion is committed before credentials or runtime state are destroyed. This prevents a - * failed config write from leaving a still-configured account with a tombstoned credential. The - * whole sequence shares the config mutation coordinator so a cooperating writer cannot re-add the - * account between the durable config commit and credential cleanup. + * When the runtime config is backed by an existing config.json, commit the config deletion before + * credentials or runtime state are destroyed. Pure in-memory callers intentionally remain + * side-effect free because they have no durable account row to protect. The whole sequence shares + * the config mutation coordinator so a cooperating writer cannot re-add a persisted account + * between the durable config commit and credential cleanup. * * Returns true when a picker-visible row disappeared and the catalog must converge. */ @@ -98,6 +100,7 @@ export function deleteCodexAccount(runtimeConfig: OcxConfig, accountId: string): let cleanupFailed = false; const pickerVisibilityChanged = withConfigMutationLockSync(() => { const previousConfig = structuredClone(runtimeConfig); + const hasPersistedConfig = existsSync(getConfigPath()); const hadStoredAccount = (runtimeConfig.codexAccounts ?? []) .some(account => !account.isMain && account.id === accountId); const hadVisiblePickerBinding = hadStoredAccount @@ -112,13 +115,15 @@ export function deleteCodexAccount(runtimeConfig: OcxConfig, accountId: string): clearCodexAccountPin(runtimeConfig, accountId); if (runtimeConfig.activeCodexAccountId === accountId) runtimeConfig.activeCodexAccountId = undefined; - try { - // Persist first. Destructive cleanup below must never run for a deletion that did not become - // durable. The auth API's existing follow-up save is intentionally idempotent. - saveConfigPreservingClaudeCode(runtimeConfig); - } catch (error) { - restoreRuntimeConfig(runtimeConfig, previousConfig); - throw error; + if (hasPersistedConfig) { + try { + // Persist first for durable configs. Destructive cleanup below must never run for a + // deletion that failed to commit. Transient configs intentionally skip this write. + saveConfigPreservingClaudeCode(runtimeConfig); + } catch (error) { + restoreRuntimeConfig(runtimeConfig, previousConfig); + throw error; + } } try { From 06054819b85e9dec1de378cb399102312f0f6d26 Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Sat, 8 Aug 2026 23:32:43 +0200 Subject: [PATCH 4/6] test(codex): cover post-write delete rollback --- tests/codex-account-delete-atomicity.test.ts | 23 ++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/codex-account-delete-atomicity.test.ts b/tests/codex-account-delete-atomicity.test.ts index 2fbdb2cab..f98203352 100644 --- a/tests/codex-account-delete-atomicity.test.ts +++ b/tests/codex-account-delete-atomicity.test.ts @@ -84,6 +84,29 @@ describe("Codex account delete persistence ordering", () => { } }); + test("a failure after durable config replacement restores the prior config", () => { + const config = seededConfig(); + const before = structuredClone(config); + const realSave = configModule.saveConfigPreservingClaudeCode; + const saveSpy = spyOn(configModule, "saveConfigPreservingClaudeCode") + .mockImplementation(candidate => { + realSave(candidate); + throw new Error("forced post-write failure"); + }); + + try { + expect(() => deleteCodexAccount(config, ACCOUNT_ID)).toThrow("forced post-write failure"); + + expect(config).toEqual(before); + expect(loadConfig().codexAccounts?.some(account => account.id === ACCOUNT_ID)).toBe(true); + expect(getCodexAccountCredential(ACCOUNT_ID)).not.toBeNull(); + expect(isAccountNeedsReauth(ACCOUNT_ID)).toBe(true); + expect(getAccountQuota(ACCOUNT_ID)).not.toBeNull(); + } finally { + saveSpy.mockRestore(); + } + }); + test("the durable config deletion happens before credential and runtime cleanup", () => { const config = seededConfig(); const realSave = configModule.saveConfigPreservingClaudeCode; From 7f577976a8b7657c50ca4b472f22895c16ea5385 Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Sat, 8 Aug 2026 23:33:10 +0200 Subject: [PATCH 5/6] fix(codex): roll back post-write delete failures --- src/codex/account-lifecycle.ts | 44 +++++++++++++++++++++++++++------- 1 file changed, 36 insertions(+), 8 deletions(-) diff --git a/src/codex/account-lifecycle.ts b/src/codex/account-lifecycle.ts index 15ce9e328..b9757a988 100644 --- a/src/codex/account-lifecycle.ts +++ b/src/codex/account-lifecycle.ts @@ -1,5 +1,10 @@ -import { existsSync } from "node:fs"; -import { getConfigPath, saveConfigPreservingClaudeCode, withConfigMutationLockSync } from "../config"; +import { existsSync, readFileSync } from "node:fs"; +import { + atomicWriteFile, + getConfigPath, + saveConfigPreservingClaudeCode, + withConfigMutationLockSync, +} from "../config"; import { removeCodexAccountCredential } from "./account-store"; import { clearAccountNeedsReauth } from "./account-runtime-state"; import { getMainChatgptAccountId } from "./auth-collision"; @@ -22,6 +27,13 @@ export class CodexAccountDeleteCleanupError extends Error { } } +export class CodexAccountDeleteRollbackError extends Error { + constructor() { + super("Account deletion failed and the previous config could not be restored. Restart before retrying."); + this.name = "CodexAccountDeleteRollbackError"; + } +} + export function purgeCodexAccountRuntimeState(accountId: string): void { clearAccountNeedsReauth(accountId); clearAccountQuota(accountId); @@ -85,14 +97,23 @@ function restoreRuntimeConfig(target: OcxConfig, snapshot: OcxConfig): void { Object.assign(target, snapshot); } +function restorePersistedConfig(configPath: string, previousBytes: string): void { + try { + if (readFileSync(configPath, "utf8") === previousBytes) return; + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== "ENOENT") throw error; + } + atomicWriteFile(configPath, previousBytes); +} + /** * Delete a stored account while retaining its selector binding. * * When the runtime config is backed by an existing config.json, commit the config deletion before - * credentials or runtime state are destroyed. Pure in-memory callers intentionally remain - * side-effect free because they have no durable account row to protect. The whole sequence shares - * the config mutation coordinator so a cooperating writer cannot re-add a persisted account - * between the durable config commit and credential cleanup. + * credentials or runtime state are destroyed. Transient callers intentionally skip durable config + * persistence because they have no durable account row to protect. The whole sequence shares the + * config mutation coordinator so a cooperating writer cannot re-add a persisted account between + * the durable config commit and credential cleanup. * * Returns true when a picker-visible row disappeared and the catalog must converge. */ @@ -100,7 +121,9 @@ export function deleteCodexAccount(runtimeConfig: OcxConfig, accountId: string): let cleanupFailed = false; const pickerVisibilityChanged = withConfigMutationLockSync(() => { const previousConfig = structuredClone(runtimeConfig); - const hasPersistedConfig = existsSync(getConfigPath()); + const configPath = getConfigPath(); + const hasPersistedConfig = existsSync(configPath); + const previousPersistedConfig = hasPersistedConfig ? readFileSync(configPath, "utf8") : undefined; const hadStoredAccount = (runtimeConfig.codexAccounts ?? []) .some(account => !account.isMain && account.id === accountId); const hadVisiblePickerBinding = hadStoredAccount @@ -115,13 +138,18 @@ export function deleteCodexAccount(runtimeConfig: OcxConfig, accountId: string): clearCodexAccountPin(runtimeConfig, accountId); if (runtimeConfig.activeCodexAccountId === accountId) runtimeConfig.activeCodexAccountId = undefined; - if (hasPersistedConfig) { + if (previousPersistedConfig !== undefined) { try { // Persist first for durable configs. Destructive cleanup below must never run for a // deletion that failed to commit. Transient configs intentionally skip this write. saveConfigPreservingClaudeCode(runtimeConfig); } catch (error) { restoreRuntimeConfig(runtimeConfig, previousConfig); + try { + restorePersistedConfig(configPath, previousPersistedConfig); + } catch { + throw new CodexAccountDeleteRollbackError(); + } throw error; } } From 528dcb747a0107370c1a3b2641a5a6b3931943c5 Mon Sep 17 00:00:00 2001 From: Wibias <37517432+Wibias@users.noreply.github.com> Date: Sat, 8 Aug 2026 23:34:18 +0200 Subject: [PATCH 6/6] test(codex): assert exact delete rollback bytes --- tests/codex-account-delete-atomicity.test.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/codex-account-delete-atomicity.test.ts b/tests/codex-account-delete-atomicity.test.ts index f98203352..8f71e730f 100644 --- a/tests/codex-account-delete-atomicity.test.ts +++ b/tests/codex-account-delete-atomicity.test.ts @@ -1,5 +1,5 @@ import { afterEach, beforeEach, describe, expect, spyOn, test } from "bun:test"; -import { existsSync, mkdirSync, rmSync } from "node:fs"; +import { existsSync, mkdirSync, readFileSync, rmSync } from "node:fs"; import { join } from "node:path"; import * as accountStoreModule from "../src/codex/account-store"; import { @@ -18,7 +18,7 @@ import { getAccountQuota, updateAccountQuota, } from "../src/codex/quota"; -import { loadConfig, saveConfig } from "../src/config"; +import { getConfigPath, loadConfig, saveConfig } from "../src/config"; import * as configModule from "../src/config"; import type { OcxConfig } from "../src/types"; @@ -87,6 +87,7 @@ describe("Codex account delete persistence ordering", () => { test("a failure after durable config replacement restores the prior config", () => { const config = seededConfig(); const before = structuredClone(config); + const beforeBytes = readFileSync(getConfigPath(), "utf8"); const realSave = configModule.saveConfigPreservingClaudeCode; const saveSpy = spyOn(configModule, "saveConfigPreservingClaudeCode") .mockImplementation(candidate => { @@ -98,6 +99,7 @@ describe("Codex account delete persistence ordering", () => { expect(() => deleteCodexAccount(config, ACCOUNT_ID)).toThrow("forced post-write failure"); expect(config).toEqual(before); + expect(readFileSync(getConfigPath(), "utf8")).toBe(beforeBytes); expect(loadConfig().codexAccounts?.some(account => account.id === ACCOUNT_ID)).toBe(true); expect(getCodexAccountCredential(ACCOUNT_ID)).not.toBeNull(); expect(isAccountNeedsReauth(ACCOUNT_ID)).toBe(true);