diff --git a/src/main/codex/codex-pane-account-registry-mutations.test.ts b/src/main/codex/codex-pane-account-registry-mutations.test.ts new file mode 100644 index 00000000000..80630d06912 --- /dev/null +++ b/src/main/codex/codex-pane-account-registry-mutations.test.ts @@ -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) + }) +}) diff --git a/src/main/codex/codex-pane-account-registry-mutations.ts b/src/main/codex/codex-pane-account-registry-mutations.ts new file mode 100644 index 00000000000..620d4620424 --- /dev/null +++ b/src/main/codex/codex-pane-account-registry-mutations.ts @@ -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() + private retryTimer: ReturnType | 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] + } +} diff --git a/src/main/codex/codex-pane-account-registry-types.ts b/src/main/codex/codex-pane-account-registry-types.ts new file mode 100644 index 00000000000..1c93de135b2 --- /dev/null +++ b/src/main/codex/codex-pane-account-registry-types.ts @@ -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:' — 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 +} diff --git a/src/main/codex/codex-pane-account-registry.ts b/src/main/codex/codex-pane-account-registry.ts index faa8dd87c0d..449c11ba711 100644 --- a/src/main/codex/codex-pane-account-registry.ts +++ b/src/main/codex/codex-pane-account-registry.ts @@ -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:' — 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 -} - -// 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).panes + const panes = (parsed as Partial).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 { - const registry = readRegistry() + const registry = readRegistryOrThrow() const lanesByPtyId: Record = {} for (const ptyId of ptyIds) { const record = registry.panes[ptyId] @@ -286,6 +240,21 @@ export function listRecordedCodexPaneLanes(ptyIds: readonly string[]): Record { + const registry = readRegistryOrThrow() + const records = new Map() + 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() } } diff --git a/src/main/codex/codex-stale-pane-accounts.ts b/src/main/codex/codex-stale-pane-accounts.ts index e3c63697cb9..c7e789b3b9b 100644 --- a/src/main/codex/codex-stale-pane-accounts.ts +++ b/src/main/codex/codex-stale-pane-accounts.ts @@ -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 } diff --git a/src/main/codex/sta-4823-shared-state-survives-a-failed-read.test.ts b/src/main/codex/sta-4823-shared-state-survives-a-failed-read.test.ts index e9fce52bb41..adab285b592 100644 --- a/src/main/codex/sta-4823-shared-state-survives-a-failed-read.test.ts +++ b/src/main/codex/sta-4823-shared-state-survives-a-failed-read.test.ts @@ -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())