Skip to content

[Bug] ocx config set / ocx config import bypass the protected 3-way reconcile and can clobber concurrent disk / hand edits #1835

Description

@trungtaottn

Summary

The mutation-protected, disk-rebasing config reconcile that the GUI/management routes use is not
applied on the CLI config set / config import command paths. Those commands read the on-disk
config, mutate a clone, and persist it via the raw saveConfig(config) (src/cli/config-command.ts:145
for set/unset, :178 for import), which serializes the earlier snapshot verbatim and never rebases
on a concurrent disk edit. A hand-edited (or externally-written) field that changes between the CLI's
read and its write is silently reverted.

The window is tiny: the off-lock disk read at src/cli/config-command.ts:133 sits immediately before the
on-lock write inside saveConfig. This is a race / consistency issue (severity low–medium), not a hot
always-reproducible data-losing bug, and it shares the bug family with open #1802 but covers a distinct
command surface (config set/import vs #1802's service-time save-triggered-by ocx sync).

Routing config set/unset through the reconciling save alone is NOT sufficient (see below): the
reconcile is gated on a baseline the CLI config never arms. A correct fix must arm a baseline for the CLI
config (or use a baseline-independent disk-read reconcile); import (a wholesale overwrite by design)
should instead document-and-warn before dropping keys.

Why it is a real issue (file:line evidence)

  • src/cli/config-command.ts:133 reads with structuredClone(readConfigDiagnostics().config)
    outside the mutation lock — then:
    • config-command.ts:145 calls saveConfig(config) for set/unset,
    • config-command.ts:178 calls saveConfig(validate(loadInput(path))) for import.
  • saveConfig (src/config.ts:2766) is the raw saver: under the mutation lock it only (a) projects the
    one-time custom-model-catalog migration from disk (projectCustomModelCatalogMigration, src/codex/ custom-model-catalog-migration.ts:107) and (b) re-preserves disk-only providers /
    modelCosts overlays (withPreservedDiskOnlyProviders, src/usage/user-cost-overlays.ts:169). It
    performs no whole-record 3-way reconcile of other concurrent disk edits.
  • The reconciling alternative the management/GUI routes use is saveConfigPreservingClaudeCode
    (src/config.ts:3185): it re-reads the on-disk file inside the lock (readRawConfigJson,
    config.ts:3135), then rebases via reconcileConfigRecord (config.ts:3027) against an armed
    live-config baseline, with an explicit skip set {"hostname","port","claudeCode"}
    (config.ts:3209).
    • Management routes all use the reconciling save: src/server/management/config-routes.ts:338,568,609;
      also agent-settings-routes.ts, combo-routes.ts, model-routes.ts, oauth-account-routes.ts,
      provider-routes.ts, routing-profile-routes.ts, all on the server's long-lived, armed
      ctx.config.
  • So a field hand-edited to disk between the CLI read (:133) and the CLI write (:145/:178) — e.g.
    emptyCompletionRetry, streamMode, appOwnedMemoryBudgetMb, or a new provider-model row — is
    clobbered by the CLI's verbatim snapshot save. (Avoid port/claudeCode/hostname as examples: those
    are deliberately skipped even by the reconciling save — config.ts:3209 — so they are server-owned
    and would not be preserved by either path.)

CRITICAL caveat on the proposed fix (baseline gating)

Routing config set/unset through saveConfigPreservingClaudeCode is not sufficient by itself.
That function's reconcile is gated on:

// src/config.ts:3191-3192
const baseline = liveConfigBaseline.get(config);
if (baseline && onDisk !== undefined) { ... }

liveConfigBaseline (and claudeCodeBaseline) are populated only by armClaudeCodeBaseline
(src/config.ts:2907), which is documented as "MANDATORY at startServer, not lazy on first save"
(config.ts:2902-2909) and is called only in the server start path (src/server/index.ts:562,
src/server/index.ts:556-562). A CLI-constructed config object is a server-unarmed instance, so
liveConfigBaseline.get(cliConfig) is undefined and the whole reconcile block is skipped — the
function then degrades to essentially a plain persist (still doing only custom-model projection +
disk-only-provider preservation). The name saveConfigPreservingClaudeCode is misleading for non-server
callers: this is corroborated in-tree by src/cli/claude-desktop.ts:53 (and :166,177,196), which does
call saveConfigPreservingClaudeCode from a CLI process on a freshly loadConfig()-ed, unarmed config —
the reconcile silently never runs there either.

Proposed direction (correct, validated against source)

Split by semantics, because set/unset and import are different operations:

  1. config set / config unsetincremental mutation of existing state, so reconciliation is
    appropriate. Fix: validate that the freshly-read candidate is the config these writes should rebase on,
    then arm a fresh baseline on that candidate (armClaudeCodeBaseline(candidate) after the read at
    config-command.ts:133) before calling the reconciling save. Because the reconcile compares
    live (candidate) vs baseline (armed snapshot of that same read) vs persisted (on-disk read inside
    the lock, config.ts:3190), only the key the operator actually set reports liveChanged; any other key
    hand-edited to disk in between reports persistedChanged-and-not-liveChanged and is adopted from disk
    (reconcileConfigValue, config.ts:3053-3078). Keep the disk-read inside the mutation lock so the
    comparison sees one atomic pre-write state. Alternative of equal value: a dedicated baseline-independent
    CLI reconcile that reads disk inside withConfigMutationLockSync and merges non-set keys from disk
    before persisting.
  2. config importwholesale replacement of config.json by design (the file supplies its own full
    config and its own account-priority pin; see the comment at config-command.ts:137-143). Reconciliation
    is semantically wrong here: arming a baseline on the import file would make onDisk differ "from
    baseline" on nearly every key and the reconcile would adopt disk over the import, defeating the import.
    The correct, honest fix is document-and-warn: before overwriting, diff the would-be-written config
    against current disk and, when the write would drop a key that exists only on disk, print exactly which
    keys (import already requires --yes; make the list of dropped keys explicit). This matches the
    lower-risk direction the prior review rounds suggested, and avoids any silent data loss.
  3. Either way, the write must run with the disk read inside the config-mutation lock, mirroring the
    2.21.0 save-path fix that [Bug] ocx sync overwrites hand-edited config.json from stale server memory (GLM-5.3 tune lost); 2.21.0 save path fixed it but sync path question remains #1802 confirms.

Write-path map (V2.22.0, main d9de895)

All config.json write surfaces today, classified as reconciling vs raw-save:

Surface Entry Saver Armed baseline? Outcome
CLI config set/unset src/cli/config-command.ts:145 saveConfig (raw) no clobber window
CLI config import src/cli/config-command.ts:178 saveConfig (raw) no clobber window (wholesale)
CLI init src/cli/init.ts:170 saveConfig (raw) no raw (one-shot scaffold)
CLI ocx start port persist src/cli/index.ts:182 saveConfig (raw) no writes only port (a skipped key anyway)
CLI models add/remove src/cli/models.ts:236,282 saveConfig (raw) no raw
CLI provider add/remove src/cli/provider.ts:71 saveConfig (raw) no raw
CLI v2 src/cli/v2.ts:198,221 saveConfig (raw) no raw
CLI oauth login src/cli/login-cli.ts:160 / src/oauth/index.ts:944 saveConfig (raw) no raw
Startup migrations src/server/index.ts:509,515,531 saveConfig (raw) arm happens after (:562) documented exception (runs before baseline exists, config held by nobody else)
All management routes (settings, models, providers, oauth accounts, routing-profiles, log-guard policy, cost overlays, key-failover) config-routes.ts:338,568,609; agent-settings-routes.ts; combo-routes.ts; model-routes.ts; oauth-account-routes.ts; provider-routes.ts; routing-profile-routes.ts; codex/log-guard/policy.ts; providers/api-keys.ts; providers/key-failover.ts; codex/auth-api.ts:443; account-lifecycle.ts:145 saveConfigPreservingClaudeCode yes (server ctx.config) reconciled
CLI ocx claude desktop import apply src/cli/claude-desktop.ts:53,166,177,196 saveConfigPreservingClaudeCode no (CLI process) reconcile silently skipped → degrades to raw
Routing-profile apply, storage cleanup policy routing-profile-routes.ts:305,334; storage/policy.ts:267; codex/routing.ts:1212,1304 saveConfigPreservingClaudeCode yes (server) reconciled
ocx system sync (POST /api/sync) config-routes.ts:383-396codex/sync.ts syncModelsToCodex does not write ~/.opencodex/config.json; writes Codex's own catalog/~/.codex config n/a to opencodex config
  • MCP: not a config-write surface. opencodex's own config schema only carries external-MCP routing
    limits (mcpMaxTools, mcpMaxSchemaBytes, mcpMaxResultBytes, src/config.ts:726-728); opencodex acts
    as a client/proxy to external MCP servers, and no server-side MCP tool mutates config.json. The
    src/adapters/cursor/mcp-*.ts / src/lab/*mcp* files are transport/probing code, not config writers.
  • The CLI config set/import is the notable command-surface omission: every server-side writer is
    reconciled, while the two CLI config mutators are raw.

Reproduction (illustrative, race-window dependent)

  1. ocx start (long-lived proxy / service).
  2. Hand-edit ~/.opencodex/config.json to add a non-skipped field, e.g. set streamMode: "eager-relay",
    or emptyCompletionRetry: true.
  3. Immediately run ocx config set some.other.path value (or ocx config import file.json --yes).
  4. Inspect the saved config.json: the hand-edit from step 2 is gone, because the CLI wrote back the
    snapshot it read at step 2's moment (plus its schema-normalized defaults) rather than rebasing onto the
    current disk state via reconcileConfigRecord.

This mirrors #1802's clobber (a hand-added glm-5.3 model row under provider zai was lost) but through
the CLI command path rather than a service-time save. The window is small (read at config-command.ts:133
→ write at :145/:178), so it is a race, not a deterministic repro.

Relation to open issues

Verification needed before filing

Checks

  • src/cli/config-command.ts:133,145,178 use raw saveConfig, not the reconciling save.
  • src/config.ts:2766 (saveConfig) vs src/config.ts:3185 (saveConfigPreservingClaudeCode) are
    two distinct save paths; the latter's reconcile gates on liveConfigBaseline (config.ts:3191-3192).
  • The CLI disk read (config-command.ts:133) occurs outside the mutation lock; the write is on-lock.
  • armClaudeCodeBaseline is called only at src/server/index.ts:562 (server start), so a CLI config is
    unarmed → the reconciling save would skip reconcile.
  • port/claudeCode/hostname are excluded from reconcile (config.ts:3209) and are not used as
    clobber examples in this report.

Metadata

Metadata

Assignees

No one assigned

    Labels

    cliCLI, config inject, packaging flags

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions