Files
orca/src/main/codex/config-sync-stall.test.ts
T
Brennan Benson c3526cc19d feat(codex): surface a stalled config sync instead of failing silently (#10449)
* feat(codex): surface a stalled config sync instead of failing silently

Why: the mirror keeps serving the last synced settings when ~/.codex/config.toml
is missing, blank, or unreadable. That is the right call for data safety, but it
is invisible — a downed WSL distro or an unhydrated cloud-synced home leaves
"Orca ignores my config edits" with no log line and no UI to diagnose.

Status is derived on demand from the same predicates the mirror uses, so the two
cannot disagree. The stall is logged once per episode rather than on every launch
and quota poll, and the Codex account section names the file and what to do.

* fix(codex): latch an unreadable source and stop over-claiming recovery

An unreadable source throws out of the mirror, so reporting only on the success
path left that stall latch-less: it logged the raw failure on every launch and
quota poll while its reason never reached the surfaced status. Report from the
catch path too.

The clear message also claimed the source was "readable again", which is false
when the stall ended because the runtime config was removed rather than because
the source came back.

Restoring console.warn now happens in afterEach — an inline mockRestore is
skipped by a failing assertion, and the leaked spy made every later case in the
block fail spuriously.

* fix(codex): latch the stall promotion hits first, and scope it to the host

Review round 1 findings:

- The unreadable-source latch still never fired in the steady state. Once a
  baseline exists, promotion reads the source before the mirror does, so it
  throws first and `!promotionPlan` returned before any reporting — logging a
  reasonless failure every launch and quota poll, which is exactly what the
  previous commit claimed to fix. Report from that branch too. The test only
  passed because its fixture had no baseline; it now seeds one first and fails
  without the fix.
- The banner named the host's ~/.codex while a WSL or per-account runtime was
  selected, whose real source is a different file entirely. Gate it to the host
  scope, matching how the sign-in warning is already gated.
- Three new translate keys were missing from the locale catalogs, failing the
  localization gate in `pnpm lint`.
- The registrar mock was never asserted, so deleting the registration left the
  suite green.
- `codexConfigSyncStatus` hung off the `agentHooks` namespace despite having
  nothing to do with agent hooks; moved to its own `codexConfigSync.status`
  while it is still a four-file change.

* fix(codex): report sync health for the home the selection actually mirrors

Review round 2:

- The status resolved the shared runtime home, but the system default now runs
  Codex directly against ~/.codex and managed accounts get their own home. So a
  stalled per-account mirror showed no banner at all, while a stale shared home
  could warn about a config the active lane never reads. Resolve the mirrored
  home from the current selection, and report synced when the lane has no mirror
  to fall behind.
- The round-1 report on the promotion failure path could clear the latch on a
  pass where no mirror ran, claiming a recovery that never happened and
  silencing every later pass. Only ever latch a stall there; leave clearing to
  the path that actually mirrored.

* fix(codex): refetch sync status when the active Codex account changes

Review round 3:

- Resolving the status per selection made the fetch account-dependent, but the
  effect was not keyed on the active account. Switching accounts left the banner
  describing the previous one — and switching INTO a stalled account showed
  nothing at all, which is the silence this change exists to remove.
- Pin the home resolution itself: it had no direct test, and its shared-home
  path was a hand-copied literal that could drift from the real helper and
  silence the banner with every other test still green.
- Narrow the handler's dependency to the one method it calls, which also drops
  an `as unknown as` cast from its test.
- Skip the chmod-based test on Windows, where a read-only directory does not
  block writes so the scenario cannot be constructed; matches the convention
  already used in config-settings-promotion.test.ts.

* chore(codex): restore the handler docstring and isolate the resolver suite

Round 4 returned clean; these are its two non-blocking nits.

Narrowing the handler param left its JSDoc stranded above the new type, so the
function had no hover doc. The resolver suite also read the developer's real
CODEX_HOME and shell rc, so anyone exporting one would see it fail locally.
2026-07-24 18:30:54 -07:00

230 lines
9.2 KiB
TypeScript

import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import { chmodSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'
import { tmpdir } from 'node:os'
import { join } from 'node:path'
import {
getCodexConfigSyncStatus,
resetCodexConfigSyncStallLatchForTests
} from './config-sync-stall'
import { syncSystemConfigIntoManagedCodexHome } from './codex-config-mirror'
let root: string
let homes: { runtimeHomePath: string; systemHomePath: string }
function systemConfigPath(): string {
return join(homes.systemHomePath, 'config.toml')
}
function runtimeConfigPath(): string {
return join(homes.runtimeHomePath, 'config.toml')
}
beforeEach(() => {
root = mkdtempSync(join(tmpdir(), 'orca-codex-sync-stall-'))
homes = { runtimeHomePath: join(root, 'runtime'), systemHomePath: join(root, 'system') }
mkdirSync(homes.runtimeHomePath, { recursive: true })
mkdirSync(homes.systemHomePath, { recursive: true })
})
afterEach(() => {
rmSync(root, { recursive: true, force: true })
})
describe('getCodexConfigSyncStatus', () => {
it('reports synced while the source config is usable', () => {
writeFileSync(systemConfigPath(), 'model = "gpt-5"\n', 'utf-8')
writeFileSync(runtimeConfigPath(), 'model = "gpt-5"\n', 'utf-8')
expect(getCodexConfigSyncStatus(homes)).toEqual({
state: 'synced',
reason: null,
systemConfigPath: systemConfigPath()
})
})
it('reports a stall when the source config is missing', () => {
writeFileSync(runtimeConfigPath(), 'model = "runtime-model"\n', 'utf-8')
expect(getCodexConfigSyncStatus(homes)).toEqual({
state: 'stalled',
reason: 'missing-source',
systemConfigPath: systemConfigPath()
})
})
it('reports a stall when the source config is blank', () => {
writeFileSync(systemConfigPath(), '\n \n', 'utf-8')
writeFileSync(runtimeConfigPath(), 'model = "runtime-model"\n', 'utf-8')
expect(getCodexConfigSyncStatus(homes).reason).toBe('blank-source')
})
it('reports a stall when the source config cannot be read', () => {
// Why: a directory at the config path survives existsSync but throws on
// read, standing in for a permission-denied or otherwise unreadable home.
mkdirSync(systemConfigPath(), { recursive: true })
writeFileSync(runtimeConfigPath(), 'model = "runtime-model"\n', 'utf-8')
expect(getCodexConfigSyncStatus(homes).reason).toBe('unreadable-source')
})
it('stays synced before the runtime config exists, since nothing can fall behind yet', () => {
expect(getCodexConfigSyncStatus(homes)).toEqual({
state: 'synced',
reason: null,
systemConfigPath: systemConfigPath()
})
})
// Why: the status must never claim a sync the mirror would decline, so pin it
// to the mirror's real behavior rather than to a duplicated predicate.
it('reports a stall exactly when the mirror preserves the runtime config', () => {
writeFileSync(systemConfigPath(), 'model = "gpt-5"\n', 'utf-8')
syncSystemConfigIntoManagedCodexHome(homes)
expect(getCodexConfigSyncStatus(homes).state).toBe('synced')
rmSync(systemConfigPath())
syncSystemConfigIntoManagedCodexHome(homes)
expect(getCodexConfigSyncStatus(homes).state).toBe('stalled')
// The mirror keeps serving the last good settings — that is what makes the
// stall silent, and why it needs surfacing.
expect(readFileSync(runtimeConfigPath(), 'utf-8')).toContain('model = "gpt-5"')
})
it('clears the stall once the source config returns', () => {
writeFileSync(runtimeConfigPath(), 'model = "runtime-model"\n', 'utf-8')
expect(getCodexConfigSyncStatus(homes).state).toBe('stalled')
writeFileSync(systemConfigPath(), 'model = "gpt-5"\n', 'utf-8')
expect(getCodexConfigSyncStatus(homes).state).toBe('synced')
})
})
describe('reportCodexConfigSyncOutcome', () => {
beforeEach(() => {
resetCodexConfigSyncStallLatchForTests()
})
// Why: a failing assertion skips an inline mockRestore, and a leaked
// console.warn spy makes every later case in this block fail spuriously.
afterEach(() => {
vi.restoreAllMocks()
})
it('logs a stall once per episode rather than on every sync pass', () => {
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
writeFileSync(runtimeConfigPath(), 'model = "runtime-model"\n', 'utf-8')
for (let pass = 0; pass < 3; pass += 1) {
syncSystemConfigIntoManagedCodexHome(homes)
}
expect(warn.mock.calls.filter((call) => String(call[0]).includes('stalled'))).toHaveLength(1)
})
it('logs once when the stall clears after the source config returns', () => {
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
writeFileSync(runtimeConfigPath(), 'model = "runtime-model"\n', 'utf-8')
syncSystemConfigIntoManagedCodexHome(homes)
writeFileSync(systemConfigPath(), 'model = "gpt-5"\n', 'utf-8')
syncSystemConfigIntoManagedCodexHome(homes)
syncSystemConfigIntoManagedCodexHome(homes)
expect(
warn.mock.calls.filter((call) => String(call[0]).includes('stall cleared'))
).toHaveLength(1)
})
it('latches an unreadable source instead of logging the raw failure every pass', () => {
// Why: an unreadable source throws out of the mirror, so before the catch
// path reported it this stall logged the generic failure on every launch
// and quota poll and never surfaced its reason.
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
// Why: seed a baseline with one healthy pass first. Without it promotion
// no-ops before touching the source and the mirror is what throws — which
// is NOT the steady state every real install is in, where promotion reads
// the source first and throws there instead.
writeFileSync(systemConfigPath(), 'model = "gpt-5"\n', 'utf-8')
syncSystemConfigIntoManagedCodexHome(homes)
warn.mockClear()
rmSync(systemConfigPath())
mkdirSync(systemConfigPath(), { recursive: true })
for (let pass = 0; pass < 3; pass += 1) {
syncSystemConfigIntoManagedCodexHome(homes)
}
const stallLogs = warn.mock.calls.filter((call) => String(call[0]).includes('stalled'))
expect(stallLogs).toHaveLength(1)
expect(String(stallLogs[0]?.[0])).toContain('unreadable-source')
})
it('clears a stall without claiming the source became readable', () => {
// Why: a removed runtime config also reads as synced, so "readable again"
// would be a false claim about a source that is still gone.
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
writeFileSync(runtimeConfigPath(), 'model = "runtime-model"\n', 'utf-8')
syncSystemConfigIntoManagedCodexHome(homes)
rmSync(runtimeConfigPath())
syncSystemConfigIntoManagedCodexHome(homes)
const cleared = warn.mock.calls.filter((call) => String(call[0]).includes('stall cleared'))
expect(cleared).toHaveLength(1)
expect(String(cleared[0]?.[0])).not.toContain('readable again')
})
// chmod on a directory does not block writes on Windows, so promotion would
// succeed there and the scenario could not be constructed.
it.skipIf(process.platform === 'win32')(
'does not claim recovery on a pass where no mirror ran',
() => {
// Why: promotion throwing still means the mirror was skipped, so the runtime
// config is not tracking the source. Clearing the latch there would claim a
// recovery that never happened and silence every later pass.
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
writeFileSync(systemConfigPath(), 'model = "gpt-5"\n', 'utf-8')
syncSystemConfigIntoManagedCodexHome(homes)
// In-Codex change while the source is gone -> stall latches, and the change
// stays pending promotion.
rmSync(systemConfigPath())
writeFileSync(runtimeConfigPath(), 'model = "o4"\n', 'utf-8')
syncSystemConfigIntoManagedCodexHome(homes)
expect(warn.mock.calls.filter((c) => String(c[0]).includes('stalled'))).toHaveLength(1)
warn.mockClear()
// Source returns readable, but promotion cannot write the pending change
// back into a read-only ~/.codex, so it throws and the mirror is skipped.
writeFileSync(systemConfigPath(), 'model = "gpt-5"\n', 'utf-8')
chmodSync(homes.systemHomePath, 0o555)
try {
syncSystemConfigIntoManagedCodexHome(homes)
} finally {
chmodSync(homes.systemHomePath, 0o755)
}
expect(warn.mock.calls.filter((c) => String(c[0]).includes('stall cleared'))).toHaveLength(0)
// The runtime never picked up the source, so the change must still be there.
expect(readFileSync(runtimeConfigPath(), 'utf-8')).toContain('model = "o4"')
}
)
it('logs again when the stall reason changes', () => {
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
writeFileSync(runtimeConfigPath(), 'model = "runtime-model"\n', 'utf-8')
syncSystemConfigIntoManagedCodexHome(homes)
writeFileSync(systemConfigPath(), ' \n', 'utf-8')
syncSystemConfigIntoManagedCodexHome(homes)
const stallLogs = warn.mock.calls.filter((call) => String(call[0]).includes('stalled'))
expect(stallLogs).toHaveLength(2)
expect(String(stallLogs[1]?.[0])).toContain('blank-source')
})
})