fix(codex): retry pane attribution writes

This commit is contained in:
Brennan Benson
2026-08-23 12:12:42 -07:00
parent 572b2fa7b1
commit 2bb78b878c
6 changed files with 350 additions and 98 deletions
@@ -0,0 +1,83 @@
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import { CodexPaneAccountRegistryMutations } from './codex-pane-account-registry-mutations'
import type { CodexPaneAccountRegistryFile } from './codex-pane-account-registry-types'
const EXISTING = {
selectionKey: 'host',
accountId: 'account-1',
homeRoute: 'account-home'
} as const
const SPAWNED = {
selectionKey: 'host',
accountId: 'account-2',
homeRoute: 'account-home'
} as const
function parseRegistry(serialized: string): CodexPaneAccountRegistryFile {
return JSON.parse(serialized) as CodexPaneAccountRegistryFile
}
describe('CodexPaneAccountRegistryMutations', () => {
beforeEach(() => vi.useFakeTimers())
afterEach(() => {
vi.clearAllTimers()
vi.useRealTimers()
})
it('retries a one-shot spawn record and merges it with the recovered file', async () => {
let readable = false
let persisted = JSON.stringify({ version: 2, panes: { existing: EXISTING } })
const write = vi.fn((registry: CodexPaneAccountRegistryFile) => {
persisted = JSON.stringify(registry)
return true
})
const mutations = new CodexPaneAccountRegistryMutations({
read: () => (readable ? parseRegistry(persisted) : null),
write
})
mutations.record('spawned', SPAWNED)
expect(write).not.toHaveBeenCalled()
readable = true
await vi.advanceTimersByTimeAsync(100)
expect(parseRegistry(persisted).panes).toEqual({ existing: EXISTING, spawned: SPAWNED })
expect(vi.getTimerCount()).toBe(0)
})
it('bounds automatic retries but keeps the pending mutation for a later read', async () => {
let readable = false
let persisted = JSON.stringify({ version: 2, panes: { existing: EXISTING } })
const read = vi.fn(() => (readable ? parseRegistry(persisted) : null))
const write = vi.fn((registry: CodexPaneAccountRegistryFile) => {
persisted = JSON.stringify(registry)
return true
})
const mutations = new CodexPaneAccountRegistryMutations({ read, write })
mutations.record('spawned', SPAWNED)
await vi.advanceTimersByTimeAsync(120_000)
expect(read).toHaveBeenCalledTimes(5)
expect(vi.getTimerCount()).toBe(0)
readable = true
mutations.flush()
expect(parseRegistry(persisted).panes.spawned).toEqual(SPAWNED)
})
it('retries durability after a write failure even when the cached value now matches', async () => {
const registry = parseRegistry(JSON.stringify({ version: 2, panes: { existing: EXISTING } }))
const write = vi.fn().mockReturnValueOnce(false).mockReturnValueOnce(true)
const mutations = new CodexPaneAccountRegistryMutations({ read: () => registry, write })
mutations.record('spawned', SPAWNED)
await vi.advanceTimersByTimeAsync(100)
expect(write).toHaveBeenCalledTimes(2)
expect(vi.getTimerCount()).toBe(0)
})
})
@@ -0,0 +1,157 @@
import type {
CodexPaneAccountRecord,
CodexPaneAccountRegistryFile
} from './codex-pane-account-registry-types'
// Why: bounds both crash-leaked disk records and mutations retained while the file is unavailable.
const MAX_TRACKED_PANES = 2000
const RETRY_DELAYS_MS = [100, 1_000, 5_000, 30_000] as const
type MutationStoreDependencies = {
read: () => CodexPaneAccountRegistryFile | null
write: (registry: CodexPaneAccountRegistryFile) => boolean
}
export class CodexPaneAccountRegistryMutations {
private readonly pending = new Map<string, CodexPaneAccountRecord | null>()
private retryTimer: ReturnType<typeof setTimeout> | null = null
private retryAttempt = 0
private needsPersistence = false
constructor(private readonly dependencies: MutationStoreDependencies) {}
record(ptyId: string, record: CodexPaneAccountRecord | null): void {
this.pending.delete(ptyId)
this.pending.set(ptyId, record)
if (this.pending.size > MAX_TRACKED_PANES) {
const oldestPtyId = this.pending.keys().next().value
if (typeof oldestPtyId === 'string') {
this.pending.delete(oldestPtyId)
console.warn('[codex-pane-accounts] Dropped oldest pending registry mutation at capacity')
}
}
this.flush()
}
flush(): void {
if (this.pending.size === 0) {
this.clearRetryState()
return
}
const registry = this.dependencies.read()
if (!registry) {
this.scheduleRetry()
return
}
let changed = false
for (const [ptyId, record] of this.pending) {
changed = applyMutation(registry, ptyId, record) || changed
}
trimRegistry(registry)
if ((changed || this.needsPersistence) && !this.dependencies.write(registry)) {
this.needsPersistence = true
this.scheduleRetry()
return
}
this.needsPersistence = false
this.pending.clear()
this.clearRetryState()
}
getPendingRegistry(): CodexPaneAccountRegistryFile | null {
if (this.pending.size === 0) {
return null
}
const registry: CodexPaneAccountRegistryFile = { version: 2, panes: {} }
for (const [ptyId, record] of this.pending) {
applyMutation(registry, ptyId, record)
}
return registry
}
persistReconciliation(registry: CodexPaneAccountRegistryFile, changed: boolean): void {
if (!changed) {
return
}
if (!this.dependencies.write(registry)) {
this.needsPersistence = true
this.scheduleRetry()
return
}
this.needsPersistence = false
this.pending.clear()
this.clearRetryState()
}
reset(): void {
this.needsPersistence = false
this.pending.clear()
this.clearRetryState()
}
private scheduleRetry(): void {
if (this.retryTimer || this.retryAttempt >= RETRY_DELAYS_MS.length) {
return
}
this.retryTimer = setTimeout(() => {
this.retryTimer = null
this.flush()
}, RETRY_DELAYS_MS[this.retryAttempt])
this.retryAttempt += 1
this.retryTimer.unref?.()
}
private clearRetryState(): void {
if (this.retryTimer) {
clearTimeout(this.retryTimer)
this.retryTimer = null
}
this.retryAttempt = 0
}
}
function applyMutation(
registry: CodexPaneAccountRegistryFile,
ptyId: string,
record: CodexPaneAccountRecord | null
): boolean {
if (!record) {
if (!(ptyId in registry.panes)) {
return false
}
delete registry.panes[ptyId]
return true
}
const existing = registry.panes[ptyId]
if (recordsEqual(existing, record)) {
return false
}
registry.panes[ptyId] = record
return true
}
function recordsEqual(
left: CodexPaneAccountRecord | undefined,
right: CodexPaneAccountRecord
): boolean {
return (
left?.selectionKey === right.selectionKey &&
left.accountId === right.accountId &&
left.homeRoute === right.homeRoute &&
left.shellStartupHomeOverride?.home === right.shellStartupHomeOverride?.home &&
left.shellStartupHomeOverride?.shell === right.shellStartupHomeOverride?.shell &&
left.shellStartupHomeOverride?.configHome === right.shellStartupHomeOverride?.configHome &&
left.shellStartupHomeOverride?.codexHome === right.shellStartupHomeOverride?.codexHome &&
left.environmentHomeOverride?.codexHome === right.environmentHomeOverride?.codexHome
)
}
function trimRegistry(registry: CodexPaneAccountRegistryFile): void {
const trackedPtyIds = Object.keys(registry.panes)
if (trackedPtyIds.length <= MAX_TRACKED_PANES) {
return
}
for (const staleId of trackedPtyIds.slice(0, trackedPtyIds.length - MAX_TRACKED_PANES)) {
delete registry.panes[staleId]
}
}
@@ -0,0 +1,29 @@
import type {
CodexEnvironmentHomeOverride,
CodexShellStartupHomeOverride
} from './codex-real-home-path'
export type CodexPaneHomeRoute =
| 'real-home'
| 'shared-home'
| 'account-home'
| 'custom-home'
| 'wsl-home'
export type CodexPaneAccountRecord = {
/** 'host' or 'wsl:<distro>' — the selection lane this pane launched from. */
selectionKey: string
/** Managed account id, or null for the system-default account. */
accountId: string | null
/** Absent only on records written before route provenance was introduced. */
homeRoute?: CodexPaneHomeRoute
/** Rechecked when CODEX_HOME came from process-global shell startup. */
shellStartupHomeOverride?: CodexShellStartupHomeOverride
/** Rechecked after restart when CODEX_HOME came from the process environment. */
environmentHomeOverride?: CodexEnvironmentHomeOverride
}
export type CodexPaneAccountRegistryFile = {
version: 2
panes: Record<string, CodexPaneAccountRecord>
}
+64 -95
View File
@@ -2,11 +2,22 @@ import { mkdirSync, readFileSync, renameSync, rmSync, writeFileSync } from 'node
import { isDefinitiveAbsence } from '../../shared/definitive-filesystem-absence'
import { dirname, join } from 'node:path'
import { getOrcaUserDataPath } from './codex-home-paths'
import { CodexPaneAccountRegistryMutations } from './codex-pane-account-registry-mutations'
import type {
CodexPaneAccountRecord,
CodexPaneAccountRegistryFile,
CodexPaneHomeRoute
} from './codex-pane-account-registry-types'
import type {
CodexEnvironmentHomeOverride,
CodexShellStartupHomeOverride
} from './codex-real-home-path'
export type {
CodexPaneAccountRecord,
CodexPaneHomeRoute
} from './codex-pane-account-registry-types'
/**
* Remembers which Codex account each live PTY was launched under.
*
@@ -17,36 +28,7 @@ import type {
* the old account and the user is stuck there with no prompt to escape it.
*/
export type CodexPaneHomeRoute =
| 'real-home'
| 'shared-home'
| 'account-home'
| 'custom-home'
| 'wsl-home'
export type CodexPaneAccountRecord = {
/** 'host' or 'wsl:<distro>' — the selection lane this pane launched from. */
selectionKey: string
/** Managed account id, or null for the system-default account. */
accountId: string | null
/** Absent only on records written before route provenance was introduced. */
homeRoute?: CodexPaneHomeRoute
/** Rechecked when CODEX_HOME came from process-global shell startup. */
shellStartupHomeOverride?: CodexShellStartupHomeOverride
/** Rechecked after restart when CODEX_HOME came from the process environment. */
environmentHomeOverride?: CodexEnvironmentHomeOverride
}
type RegistryFile = {
version: 2
panes: Record<string, CodexPaneAccountRecord>
}
// Why: bounds a file that only shrinks when Orca observes a PTY exit; a crash
// mid-session would otherwise leak an entry per terminal, forever.
const MAX_TRACKED_PANES = 2000
let cachedRegistry: RegistryFile | null = null
let cachedRegistry: CodexPaneAccountRegistryFile | null = null
function getRegistryPath(): string {
return join(getOrcaUserDataPath(), 'codex-pane-accounts.json')
@@ -59,7 +41,7 @@ function getRegistryPath(): string {
* every pane's account attribution AND pinned that erasure in `cachedRegistry`
* for the rest of the process, surviving the file becoming readable again.
*/
function readRegistryOrNull(): RegistryFile | null {
function readRegistryOrNull(): CodexPaneAccountRegistryFile | null {
if (cachedRegistry) {
return cachedRegistry
}
@@ -89,16 +71,32 @@ function parseRegistryJson(rawRegistry: string): unknown {
}
/** Read-only callers: an unreadable registry reports no attribution, uncached. */
function readRegistry(): RegistryFile {
function readRegistry(): CodexPaneAccountRegistryFile {
mutations.flush()
if (!cachedRegistry) {
const pendingRegistry = mutations.getPendingRegistry()
if (pendingRegistry) {
return pendingRegistry
}
}
return readRegistryOrNull() ?? { version: 2, panes: {} }
}
function parseRegistry(parsed: unknown): RegistryFile {
const empty: RegistryFile = { version: 2, panes: {} }
function readRegistryOrThrow(): CodexPaneAccountRegistryFile {
mutations.flush()
const registry = readRegistryOrNull()
if (!registry) {
throw new Error('Codex pane account registry could not be read')
}
return registry
}
function parseRegistry(parsed: unknown): CodexPaneAccountRegistryFile {
const empty: CodexPaneAccountRegistryFile = { version: 2, panes: {} }
if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed)) {
return empty
}
const panes = (parsed as Partial<RegistryFile>).panes
const panes = (parsed as Partial<CodexPaneAccountRegistryFile>).panes
if (!panes || typeof panes !== 'object' || Array.isArray(panes)) {
return empty
}
@@ -164,7 +162,7 @@ function isPaneHomeRoute(value: unknown): value is CodexPaneHomeRoute {
)
}
function writeRegistry(registry: RegistryFile): void {
function writeRegistry(registry: CodexPaneAccountRegistryFile): boolean {
const registryPath = getRegistryPath()
const temporaryPath = `${registryPath}.${process.pid}.tmp`
try {
@@ -174,6 +172,7 @@ function writeRegistry(registry: RegistryFile): void {
mode: 0o600
})
renameSync(temporaryPath, registryPath)
return true
} catch (error) {
// Why: this record only powers a restart hint; losing it must never break a
// terminal spawn or a PTY teardown — including when the cleanup itself fails.
@@ -181,68 +180,23 @@ function writeRegistry(registry: RegistryFile): void {
try {
rmSync(temporaryPath, { force: true })
} catch {}
return false
}
}
const mutations = new CodexPaneAccountRegistryMutations({
read: readRegistryOrNull,
write: writeRegistry
})
/**
* Records the account a PTY launched under. Pass null to forget a pinned pane.
*/
export function recordCodexPaneAccount(ptyId: string, record: CodexPaneAccountRecord | null): void {
// Why: every write below persists the whole registry. Deriving it from an
// empty stand-in because the real one could not be read would drop every
// other pane's attribution on disk. Skip; the next record retries.
const registry = readRegistryOrNull()
if (!registry) {
return
}
if (!record) {
if (!(ptyId in registry.panes)) {
return
}
delete registry.panes[ptyId]
writeRegistry(registry)
return
}
const existing = registry.panes[ptyId]
if (
existing?.selectionKey === record.selectionKey &&
existing.accountId === record.accountId &&
existing.homeRoute === record.homeRoute &&
shellStartupHomeOverridesEqual(
existing.shellStartupHomeOverride,
record.shellStartupHomeOverride
) &&
environmentHomeOverridesEqual(existing.environmentHomeOverride, record.environmentHomeOverride)
) {
return
}
registry.panes[ptyId] = record
const trackedPtyIds = Object.keys(registry.panes)
if (trackedPtyIds.length > MAX_TRACKED_PANES) {
for (const staleId of trackedPtyIds.slice(0, trackedPtyIds.length - MAX_TRACKED_PANES)) {
delete registry.panes[staleId]
}
}
writeRegistry(registry)
}
function environmentHomeOverridesEqual(
left: CodexEnvironmentHomeOverride | undefined,
right: CodexEnvironmentHomeOverride | undefined
): boolean {
return left?.codexHome === right?.codexHome
}
function shellStartupHomeOverridesEqual(
left: CodexShellStartupHomeOverride | undefined,
right: CodexShellStartupHomeOverride | undefined
): boolean {
return (
left?.home === right?.home &&
left?.shell === right?.shell &&
left?.configHome === right?.configHome &&
left?.codexHome === right?.codexHome
)
// Why: the spawn attribution is one-shot. Retain it while the registry is
// unreadable, then merge it with the recovered file instead of dropping it
// or rebuilding from an empty stand-in.
mutations.record(ptyId, record)
}
/**
@@ -275,7 +229,7 @@ export function isCodexPaneHomeRouteProvenAwayFromSharedHome(
* answers for a launch that never happened once the user edits those settings.
*/
export function listRecordedCodexPaneLanes(ptyIds: readonly string[]): Record<string, string> {
const registry = readRegistry()
const registry = readRegistryOrThrow()
const lanesByPtyId: Record<string, string> = {}
for (const ptyId of ptyIds) {
const record = registry.panes[ptyId]
@@ -286,6 +240,21 @@ export function listRecordedCodexPaneLanes(ptyIds: readonly string[]): Record<st
return lanesByPtyId
}
/** Reads restart-authoritative records without mapping an unavailable file to no attribution. */
export function listRecordedCodexPaneAccounts(
ptyIds: readonly string[]
): ReadonlyMap<string, CodexPaneAccountRecord> {
const registry = readRegistryOrThrow()
const records = new Map<string, CodexPaneAccountRecord>()
for (const ptyId of ptyIds) {
const record = registry.panes[ptyId]
if (record) {
records.set(ptyId, record)
}
}
return records
}
/** True when a retained host pane may still read the retired shared CODEX_HOME. */
export function hasRecordedLegacySharedCodexPane(): boolean {
return Object.values(readRegistry().panes).some(
@@ -314,6 +283,7 @@ export function reconcileCodexPaneAccountsWithLivePtys(livePtyIds: readonly stri
// Why: this deletes every pane not in the live list. Against an empty stand-in
// it is a no-op, but the write it guards would still persist that stand-in
// over the real file, so refuse rather than reconcile a registry nobody read.
mutations.flush()
const registry = readRegistryOrNull()
if (!registry) {
return
@@ -326,13 +296,12 @@ export function reconcileCodexPaneAccountsWithLivePtys(livePtyIds: readonly stri
changed = true
}
}
if (changed) {
writeRegistry(registry)
}
mutations.persistReconciliation(registry, changed)
}
export const _internals = {
resetCache: (): void => {
cachedRegistry = null
mutations.reset()
}
}
+3 -2
View File
@@ -2,7 +2,7 @@ import type { GlobalSettings } from '../../shared/global-settings-types'
import { getSelectedCodexAccountIdForTarget } from '../codex-accounts/runtime-selection'
import {
forgetCodexPaneAccount,
getCodexPaneAccount,
listRecordedCodexPaneAccounts,
type CodexPaneHomeRoute
} from './codex-pane-account-registry'
@@ -23,8 +23,9 @@ export function listStaleCodexPanes(args: {
activeHostHomeRoute?: CodexPaneHomeRoute
}): StaleCodexPane[] {
const stalePanes: StaleCodexPane[] = []
const records = listRecordedCodexPaneAccounts(args.ptyIds)
for (const ptyId of args.ptyIds) {
const record = getCodexPaneAccount(ptyId)
const record = records.get(ptyId)
if (!record) {
continue
}
@@ -4,6 +4,7 @@ import type * as NodeOs from 'node:os'
import { tmpdir } from 'node:os'
import { join } from 'node:path'
import { syncSystemConfigIntoManagedCodexHome } from './codex-config-mirror'
import { listStaleCodexPanes } from './codex-stale-pane-accounts'
import type { CodexTrustEntry } from './config-toml-trust'
// STA-4823: six shared Codex state files were rebuilt, erased or reported as
@@ -455,7 +456,7 @@ describe('STA-4823 D31 — an unreadable pane registry must not erase every attr
paneRegistry._internals.resetCache()
}
it('does not persist an empty registry over the real one', () => {
it('preserves existing records and retries the one-shot spawn mutation', () => {
seedTwoAttributedPanes()
const before = realFs.readFileSync(registryPath(), 'utf-8')
denials.denyReads(registryPath())
@@ -469,6 +470,12 @@ describe('STA-4823 D31 — an unreadable pane registry must not erase every attr
// Before the fix the read degraded to an empty registry and this write
// persisted it, dropping every other pane's account attribution on disk.
expect(realFs.readFileSync(registryPath(), 'utf-8')).toBe(before)
denials.release(registryPath())
expect(paneRegistry.getCodexPaneAccount('pty-3')).toMatchObject({ accountId: 'account-3' })
paneRegistry._internals.resetCache()
expect(paneRegistry.getCodexPaneAccount(PANE)).toMatchObject({ accountId: 'account-1' })
expect(paneRegistry.getCodexPaneAccount('pty-3')).toMatchObject({ accountId: 'account-3' })
})
it('does not cache the erasure, so attribution returns when the file does', () => {
@@ -476,6 +483,12 @@ describe('STA-4823 D31 — an unreadable pane registry must not erase every attr
denials.denyReads(registryPath())
expect(paneRegistry.getCodexPaneAccount(PANE)).toBeNull()
expect(() =>
listStaleCodexPanes({
ptyIds: [PANE],
settings: { activeCodexManagedAccountId: 'account-2' } as never
})
).toThrow('registry could not be read')
denials.release(registryPath())