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

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 unset — incremental 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 import — wholesale 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:

• 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

• Open [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 ("ocx sync overwrites hand-edited config.json from stale server memory") — same bug
  family (a config write clobbering a hand edit), but a distinct surface: [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's clobber was a
  service-time save while the long-lived server held stale memory; the 2.21.0 fix (re-read disk inside
  the reconcile, i.e. the very behavior of saveConfigPreservingClaudeCode at config.ts:3190) already
  covers that service-time path. Note: ocx system sync itself does not write opencodex config.json
  (see map). The CLI config set/import command path is not covered by the 2.21.0 fix. This report
  answers [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's question 1 ("any other management/GUI write path?") for the config set/import
  command surface.
• Open [Compatibility] Add an opt-in Responses stream/terminal repair escape hatch for custom providers #1809 — about Responses stream/terminal repair for custom providers; it does not touch
  config writes at all and is out of scope here (only confirms the two reports are unrelated families).
• Closed [Bug] Stale full-config writes can resurrect deleted providers and custom models #1273 ("Stale full-config writes can resurrect deleted providers and custom models") — same
  whole-config-write family (references the #1462 regression the reconcile's rebaseableKeys guard in
  config.ts:3205-3208 exists for); its fix reduced the blast radius but did not add CLI reconcile.

Verification needed before filing

• Re-confirm on current main that saveConfig vs saveConfigPreservingClaudeCode, the baseline gate at
  src/config.ts:3191-3192, and the CLI reads/writes at config-command.ts:133,145,178 are still as
  cited (line numbers may shift).
• Confirm no open issue already covers the CLI config set/import write path specifically (searches for
  "config clobber / set / import / overwrite / lost" surface only [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, [Feedback]: Small UX paper cuts — config overwrite on shutdown, silent invalid defaultEffort, model-toggle naming #488, ocx start auto-injects openai_base_url and overwrites provider config, breaking Codex Desktop on networks that cannot reach chatgpt.com #1090, [Bug]: manual opencode-go modelContextWindows entries are reset after update #1409, [Bug] Stale full-config writes can resurrect deleted providers and custom models #1273 — all
  non-CLI-config-command paths).

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.