fix(config): save_config lost-update — three-way merge-preserve + loud CAS conflict refusal at the atomic_config_write seam

[benegessarit]

The defect

Any code path that loads config into memory, holds it, and later calls
hermes_cli.config.save_config(cfg) silently reverts every config.yaml edit made
in between by anyone else — another process, a human hand-edit, a second gateway.
save_config reserializes the caller's whole dict; its preservation logic
(explicit_raw_paths → _strip_default_values(..., preserve_keys=...)) can only KEEP
paths already present in that dict, never resurrect a path that exists on current disk
but not in the caller's stale copy. _CONFIG_LOCK is thread-level only, so
cross-process writers race freely.

123c6f3a2 added atomic_config_write as the fail-closed single chokepoint for
config writes (unreadable-file guard), and fe25806a6 (#60591) added last-known-good
retention when the file fails to parse — both harden this exact seam against
corruption-shaped loss. This PR extends the same seam against concurrency-shaped
loss, and routes save_config through atomic_config_write instead of its direct
atomic_yaml_write call while at it.

Real-world impact (how we hit it, repeatedly)

Multi-writer setup: several agent sessions + four long-running gateways sharing
profile configs.

• 2026-07-05: a session held a config dict ~1.5h; its final save reverted a
  privacy-relevant agentmail.suppress_outbound_email key added on disk (and
  committed to git) in the interim. The platform fails closed on the missing key —
  outbound email silently disabled until the next restart. Only keys added AFTER the
  stale reader's load are lost, which makes the failure look selective and mysterious.
• 2026-07-09/10 (live audit): all four profile config.yaml files carried
  uncommitted gateway-reserialization churn: comments stripped (including a
  4-line reviewed rationale block), a deliberately pinned
  auxiliary_models.compression slot wiped back to auto, and a session_reset
  mode flip riding a save of unrelated keys. Cleaning it up cost a multi-slice
  remediation batch.
• Operational side effects of the blind rewrite: the gateway's whole-file
  reserialization emitted 78 trailing-whitespace lines in one profile config (0 at
  git HEAD) — our commit gate (git diff --check) then hard-blocked the snapshot
  commit until the file was whitespace-normalized with parse-equality proof. And
  because gateways run under launchd KeepAlive (one respawned mid-procedure during
  this very remediation), ANY manual config edit currently requires a full
  bootout/edit/bootstrap cycle to be safe from a concurrent rewrite. The write path
  itself has to stop reverting what it didn't change; procedures can't.

This class of blind whole-state overwrite is also what keeps un-fixing our other
fixes (e.g. a dependency-pin repair orphaned by an unrelated reset on 2026-07-09 took
every profile's memory daemon down for hours) — hence the insistence on fixing the
writer, not adding another healer.

The fix

Inside save_config's existing lock, after the normal pipeline (normalize →
${ENV}-template restore → default-strip):

1. Re-read the disk (fresh, dup-key-rejecting parse) — the merge's "theirs".
2. Three-way merge-preserve against the process's observed disk-state history
   (_DISK_STATE_OBSERVATIONS: the last 8 DISTINCT on-disk states this process
   parsed via read_raw_config/load_config, or wrote itself). Per key path:
   • equal both sides → keep;
   • the disk value is one this process authored in its own last write →
     in-process sequential saves stay last-write-wins (a process may overwrite its
     own writes, even with older values — e.g. fallback_add's restore-the-primary
     flow). "Authored" = changed relative to the disk state that write replaced;
     values a save merely carried through or merge-preserved don't count, so a
     foreign edit can't be laundered into "ours" by an intermediate save;
   • the caller's value (or absence) matches a previously observed disk state
     while disk now differs → stale snapshot: the disk-side edit wins — foreign
     additions, value changes, and deletions the caller never saw all survive.
     Logged at WARNING with the preserved key paths;
   • caller introduces a new value on a quiet disk → caller wins (incl. deletions);
   • same key changed on both sides → ConfigWriteConflictError: loud, logged,
     fail-closed, nothing written. Message names key paths only — config values can
     be secrets.
3. Duplicate mapping keys on disk → ConfigDuplicateKeyError (write refused).
   YAML is last-key-wins, so a file with duplicates is semantically ambiguous —
   usually a human mid-edit; rewriting it would silently collapse a stanza.
4. Garbled-key WARN (never blocks): an unknown second-level key inside a
   DEFAULT_CONFIG-defined subtree warns only when it near-matches a known or
   sibling key (difflib, cutoff 0.8) — the shape of a real incident key
   display.tool_progress_gr―ouping. Everything else stays silent: DEFAULT_CONFIG
   is NOT a complete schema (live configs legitimately carry session_reset,
   plugins, mcp_servers at top level and 15–25 un-enumerated second-level keys
   like agent.verbose). Calibration receipt: against our five live estate configs
   this warns on exactly 0 keys, while the historical garble would have matched.
5. The write goes through atomic_config_write (the blessed chokepoint), and the
   post-write state is recorded as a self-written observation.

The history-based base matters: a naive "merge against the last observation" is
defeated by any unrelated in-process load_config() that runs after the foreign edit
(long-running gateways reload constantly) — it absorbs the edit into the base and
launders the stale clobber. Test
test_foreign_edit_survives_even_after_intervening_load pins this.

Relation to the two options floated in the original issue draft: this is (a)+(b)
hybridized without signature churn — merge-preserve generalized to ALL keys (not just
ex-schema ones) plus CAS-style refusal for true conflicts. The draft's removals:
escape-hatch turned out unnecessary: delete-by-omission callers (migration pops,
_remove_mcp_server) act on a fresh snapshot over a quiet disk, which the merge
resolves as caller-wins; the MCP and migration suites pass unchanged.

Preserved behaviors (tested)

• ${ENV} template preservation (_preserve_env_ref_templates) — templates still
  land on disk, expanded secrets still don't, including through a two-writer merge.
• Schema-default stripping — save(load()) round-trip writes only user-authored keys.
• require_readable_config_before_write fail-closed guard (now double-covered via
  atomic_config_write).
• fe25806a6 adjacency: when the on-disk YAML is unparseable the disk state is
  UNKNOWN — no merge, no spurious conflict; save keeps its legacy overwrite (the
  corrupt content was already .bak-snapshotted by _warn_config_parse_failure,
  and load-side last-known-good retention is untouched). The corrupt-file test pins
  this. _LAST_EXPANDED_CONFIG_BY_PATH (the LKG source) now also folds in
  merge-preserved disk values so a later parse-failure fallback doesn't serve them
  stale.

Known limitations (deliberate)

• A fresh process that writes without ever reading has no observation history —
  behavior is unchanged from today (caller wins). Every real caller loads first.
• The window between save's re-read and the atomic rename is not cross-process
  locked (no flock); it shrinks from "caller's entire hold time" (minutes–hours in a
  gateway) to milliseconds.
• Bias under ambiguity is preservation: a long-running process deleting a key whose
  foreign addition it once observed gets the deletion suppressed WITH a WARNING
  naming the path. We chose visible suppression over silent loss — silent loss is
  the defect.

Tests

tests/hermes_cli/test_config_save_lost_update.py — 18 tests: two-writer hand-edit
survival (the repro — fails on unpatched main), foreign value-change survival,
intervening-load survival, foreign-deletion non-resurrection, in-process
last-write-wins, anti-laundering, caller deletion on quiet disk, CAS refusal (+
paths-not-values message), dup-key rejection, garble WARN + benign-key silence +
top-level silence, env-template round-trip, default-strip round-trip, corrupt-disk
legacy path, absent-file create.

Full sweep of every save_config-touching test file (56 files, 2190 tests): 0
failures. Verified on main (a9f3f0870) and cherry-pick-clean onto our production
checkout.

Environment

hermes-agent main @ a9f3f0870 (2026-07-10), macOS, Python 3.11 venv.

[teknium1]

Thanks for the detailed reproduction and for preserving the existing env-template and default-stripping cases. The stale-snapshot premise is real on this checkout: hermes_cli/config.py:7158 derives output from the caller snapshot and :7202 replaces the file.

Problems

• The proposed check is not a CAS across processes. PR hermes_cli/config.py:7604 reads the state used for the merge, but :7652 later performs an unconditional write. Two processes can both read the same version, both find no conflict, and the later replacement loses the earlier write. utils.py:227-282 supplies crash-safe replacement, not mutual exclusion.
• The new tests stage a foreign disk edit, but do not force two writers past the pre-write read concurrently. This leaves the race above untested.
• The same full-file read/write pattern remains in hermes_cli/config.py:8126-8164 (set_config_value), outside the proposed merge seam.

Suggested changes

• Add cross-process serialization or a conditional-write protocol spanning read/merge/replace, with a barrier-based two-process regression test.
• Audit or route direct whole-config writers through the resulting seam, or narrow the claimed guarantee to save_config.

Automated hermes-sweeper review.

[teknium1]

This write is not conditional on the state read at line 7604. Two processes can both merge against the same old disk state, then each call this replacement; the second silently loses the first process's update. Please serialize the fresh read through replacement across processes (or use a conditional-write equivalent) and add a barrier-based two-writer regression test.