Repository navigation
[Bug] ocx config set / ocx config import bypass the protected 3-way reconcile and can clobber concurrent disk / hand edits #1838
Description
[Bug] ocx config import silently deletes already-saved hand edits (deterministic); ocx config set/unset can drop a concurrent edit (race)
Issue #1838, rewritten from a real user's live experience.
Client or integration
Codex CLI (ocx) — config import, config set, config unset.
Summary
As a user, I hand-edit ~/.opencodex/config.json (I set streamMode, or tweak emptyCompletionRetry, or add a field of my own). Then I run a normal config command. My hand-written values get silently deleted, with no warning, and I only notice when I ocx config show later.
Two distinct ways this happens, in increasing order of seriousness:
ocx config import file.json --yesdestroys an edit that is already committed to disk — even with no concurrency. Import is a wholesale overwrite: whatever the import file does not contain, the CLI forgets. I proved this live: hand-edit → import a valid file lacking that field → edit gone. No race involved.ocx config set/unsetuses a read-then-write against a stale snapshot. If something edits the file in the tiny window between the command's read and its write, that edit is dropped. This one is a real race and is narrower.
I expected config management commands to preserve the fields on disk they aren't touching. They don't.
Reproduction — deterministic import clobber (no concurrency needed)
Runs from a fresh temp HOME; CLI invoked via the bundled Bun runtime:
# 1. create a real config file
ocx config set emptyCompletionRetry true
# -> writes ~/.opencodex/config.json with emptyCompletionRetry=true
# 2. hand-edit it like a normal user (this is committed to disk now)
# add a field AND toggle an existing one
> edit config.json: add "streamMode": "eager-relay", "customFlag": "user-hand-edit"
> set "emptyCompletionRetry": false
ocx config show --json | grep -E 'streamMode|emptyCompletionRetry|customFlag'
# streamMode: eager-relay ; emptyCompletionRetry: false ; customFlag: user-hand-edit <- present
# 3. import a schema-valid file that happens to omit those edited fields
ocx config import ./fresh.json --yes # fresh.json has emptyCompletionRetry=true, no streamMode/customFlag
# -> "Imported config from ./fresh.json. Restart or run ocx sync if needed." (exit 0)
# 4. the on-disk hand edits are gone
ocx config get streamMode # error: config path not found: streamMode
ocx config get customFlag # error: config path not found: customFlag
ocx config get emptyCompletionRetry
# -> true (my hand-toggled false has been reverted)Observed output (live, Bun 1.3.14, v2.22.0):
config import ./fresh.json --yes→Imported config from ./fresh.json. Restart or run ocx sync if needed.(exit 0).- After (
ocx config showdisk state):streamMode,customFlagboth deleted;emptyCompletionRetryback totrue. All three hand edits destroyed.
There was no concurrency in this reproduction: the hand edits were fully written to disk before the command was invoked.
Caveat discovered live:
importis not a blind overwrite — it validates the import file against the config schema first and refuses to write on an invalid input (an import file missingproviderserrored withschema_invalidand wrote nothing). The data loss happens when the import file is schema-valid but simply doesn't carry the user's extra field, which is exactly the common case.
Reproduction — set/unset race (confirmed with a concurrent writer)
ocx config set some.other.path value reads the current config snapshot, applies only its one path, and writes the whole object back. An edit that lands between the snapshot read and the write is overwritten by the stale snapshot. I reproduced this live by running ocx config set port 10100 in a loop while a separate process continuously rewrote an unrelated field (a monotonic counter) in the same file: 34 of 51 rounds the counter was reverted to a stale lower value — i.e. set wrote a snapshot that predated the concurrent edit and destroyed it. Without an aggressive concurrent writer the window is narrow and often passes, which is why a plain hand-edit-then-set usually looks safe and this hides.
Expected vs Actual
Expected: ocx config set/unset rebase onto whatever is on disk right now, so they never destroy a concurrent hand edit. ocx config import (a deliberate overwrite) diffs its result against the current on-disk config and warns about keys it is about to drop before saving.
Actual:
importsilently overwrites the whole file; any key the import file lacks is dropped with no diff/warning. Reproduced deterministically.set/unsetwrite a whole-object snapshot taken a moment earlier; a concurrent disk edit in between is lost. Race reproduced live (34/51 counter reverts) under a concurrent writer.
Evidence — what I actually ran
Live CLI (throwaway copy /tmp/opencodex-cli, Bun 1.3.14, source v2.22.0 d9de895):
- Baseline:
ocx config set emptyCompletionRetry true→ file written. - Hand-edit of
streamMode,customFlag,emptyCompletionRetrycommitted to disk. - Schema-valid
ocx config import fresh.json --yes(file omitting those fields) → exit 0, thenocx config showconfirmed all three edits gone. ─ deterministic, no concurrency. ocx config set port 10100under a high-frequency concurrent writer with a monotonic disk counter → 51 valid rounds, 34 lost-update reverts of the counter (monotonic counter regressed to a lower snapshot value = stale snapshot written over a newer concurrent edit).
Source facts (read, not inferred):
src/cli/config-command.ts:133—config set/unsetsnapshotreadConfigDiagnostics().config.src/cli/config-command.ts:145— set/unset persist via rawsaveConfig(config)(whole-object write, no disk rebase).src/cli/config-command.ts:178—saveConfig(validate(loadInput(path)));loadInput(:67) reads only the import file, never the current config → import is structural whole-file overwrite.src/config.ts:2766saveConfig— only custom-model-catalog reconciliation, thenpersistConfigUnlocked; no general 3-way on-disk rebase.src/config.ts:3185saveConfigPreservingClaudeCode— the rebasing writer (re-reads disk, reconciles); used by server-side writers, not by these CLI config commands.src/server/index.ts:562— baseline is armed at server start.- Related 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 — same clobber family on the service path; its fix covered service saves, not these CLI commands.
Expected behavior (what I'd ask of a fix)
ocx config set/unsetshould merge onto the current on-disk config before writing (like the server-side rebasing writer), so a concurrent hand edit is never dropped.ocx config importshould diff its candidate against the current on-disk config and warn about (or require acking) keys it is about to remove, since it is a destructive whole-file overwrite.
Version
2.22.0 (main d9de895)
Operating system
Linux
Checks
- Reproduction is a deterministic sequence a user can run (import clobber);
set/unsetrace reproduced too with a high-frequency concurrent writer. - I searched existing issues and documentation (incl. [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 devlog reference).
- I removed secrets, tokens, account details, request credentials, and personal data.
Activity
coderabbitai commentedon Aug 16, 2026
ocx config set/ocx config importbypass the protected 3-way reconcile and can clobber concurrent disk / hand edits #1835🔗 Related PRs
#697 - security: close audit findings 1/2/4/7/9/11 and partially 3/16 [merged]
#770 - fix: research-batch reliability and security-boundary issues [closed]
#852 - feat(export): client config export for Pi and OpenCode [merged]
#886 - fix(server,config): extension CORS origin identity + symlink-safe atomic writes [open]
#892 - fix: bound the six remaining retained-state leaks (
#820wave 2) [merged]#1324 - fix(codex): recover incomplete account deletion cleanup [open]
#1331 - fix(catalog): keep failover combos when live discovery omits members [merged]
#1744 - feat: land six repaired cherry-picks (MiniMax, picker order, terminal guard, streamAborted, imageInput, droid docs) [merged]
📝 Issue Planner
Check the box below or use the
@coderabbitai plancommand to generate an implementation plan and prompts that you can use with your favorite coding assistant.🧪 Issue enrichment is currently in open beta.
To disable automatic issue enrichment, add the following to your
.coderabbit.yaml:💬 Have feedback or questions? Drop into our discord!
github-actions commentedon Aug 16, 2026
Maintainer decision respected
A maintainer has reopened this issue. The automated closure has been deactivated.
trungtaottn commentedon Aug 16, 2026
Folded into #1837 — the two defects (routing optimize.latency + CLI config clobber) are now tracked together in #1837 (OPEN). Keeping this record closed to avoid duplicates.
lidge-jun commentedon Aug 16, 2026
Reopening: the technical report here is still accurate on
devat7c348a032, and it is being fixed.config set/unsetread the disk snapshot atsrc/cli/config-command.ts:133, outside the mutation lock, and then sent that whole older snapshot throughsaveConfigat:145. A concurrent edit landing in between is silently reverted while the CLI reports success.The fix moves the operation inside
mutatePersistedConfig, which already exists (src/config.ts:2884) and reruns the callback against the latest validated disk state. No new mutation primitive is needed.importis deliberately left as a whole-document replacement — forcing it through patch semantics would silently merge where the user asked to replace.fix(cli): apply config set/unset inside the mutation transaction
lidge-jun commentedon Aug 16, 2026
Fixed on
devby #1848 (merged as73440d4bb).config set/unsetnow run insidemutatePersistedConfig, which reruns the operation against the latest validated disk state, so it is applied to what is actually there at commit time rather than to a snapshot read before the lock. No new primitive was needed — that one already existed.Two details worth recording, both of which had to be right for this to be a real fix:
changedcompares a snapshot taken before the mutation. Comparing after the write compares a value with itself, reports every no-op as a change, and bumps the config generation for nothing.Object.assignalone cannot remove a key, sounsetwould have reported success while changing nothing. That case was driven red before landing.Validation, the saved-value readback and the
codexAccountPrioritiespin release all moved inside the transaction — leaving the pin clear outside would have reintroduced the same race for the pin.importis deliberately unchanged. It is an intentional whole-document replacement, and forcing it through patch semantics would silently merge where you asked to replace. The dropped-key warning idea from this report is recorded as separate follow-up rather than bundled in.Closing as completed. The change is on
devand ships with the next release.fix(cli): apply config set/unset inside the mutation transaction