fix(config): serialize .env read-modify-write to close a lost-update race (RAH-02)

[JoaoMarcos44]

Summary

Closes #77187.

save_env_value() / remove_env_value() (hermes_cli/config.py) each read the whole .env file, mutate an in-memory line list, and atomic-replace the file. atomic_replace() rules out a torn/partial write, but there is no lock around the read-modify-write cycle, so two concurrent callers can both read the same on-disk snapshot and each atomic-replace their own version. The second write wins in full; the first caller's change disappears silently — no error, no partial file, just a perfectly valid .env missing one side's key.

Reproduced with a threading.Barrier-synchronized test: two threads save distinct keys off the same base snapshot — 3/3 runs against the unlocked code lose one key.

Fix

Reused the existing cross-process advisory lock (_file_lock, already used for auth.json in hermes_cli/auth.py) around the read-transform-write cycle in both functions.

%%{init: {'theme': 'dark', 'themeVariables': { 'primaryColor': '#8b0000', 'mainBkg': '#0a0204', 'primaryTextColor': '#ffccd5', 'primaryBorderColor': '#ff0038', 'lineColor': '#ff0038'}}}%%
graph TD
    A[🩸 Writer A: save RACE_A] -->|read snapshot| C[🔥 _file_lock cross-process]
    B[🩸 Writer B: save RACE_B] -->|read snapshot| C
    C -->|serialized read-modify-write| D[⚔️ Atomic Replace]
    D --> E[🩸 Both RACE_A and RACE_B survive]

Loading

Infographic:

Test plan

• New test TestSaveEnvValueConcurrency::test_concurrent_saves_of_distinct_keys_both_survive
• Confirmed to reproduce the lost update 3/3 runs against the unlocked code; passes 100% with the lock
• Full suite: tests/hermes_cli/test_config.py (env save/remove tests) — passes, no regressions
• python -m py_compile hermes_cli/config.py

[tneemo]

Independent verification — #77208 (config .env lost-update race)

Verified the PR head (7399ca38, 2 files) on a real checkout:

What the PR does: serializes the .env read-modify-write cycle in save_env_value() / remove_env_value() with the existing cross-process _file_lock (same lock already used for auth.json). The author reproduced the lost update 3/3 runs with a threading.Barrier-synchronized test and added a regression test for it.

Verification results:

• ✅ Head files fetched and ast.parse clean (config.py 5464 lines, _env_file_lock + lazy _file_lock import present as described)
• ✅ test_concurrent_saves_of_distinct_keys_both_survive passes (1 passed) — the actual race regression test works
• ✅ Full test_config.py suite: 74 passed, 1 failed — the failure (test_default_path) is environmental (this box has a custom HERMES_HOME set, so Path.home()/.hermes ≠ $HERMES_HOME) and fails identically on main without the PR — not a regression
• ✅ Lock scoping is correct: wraps the whole read-transform-write, no reentrancy needed, lazy import avoids the auth↔config cycle
• ✅ Only the two files claimed are changed; behavior unchanged when uncontended (the lock is advisory, cross-process, same pattern as auth.json)

Design notes:

• Reusing _file_lock instead of inventing a new lock is the right call — one locking primitive for both auth.json and .env
• The export KEY= handling (#6659/#40041) is preserved inside the lock — no behavior drift
• 15s timeout with a clear error message is sane for an advisory lock on a tiny file

Verdict: Ready to land. Real bug (silent key loss under concurrency), surgical fix, regression test included, no regressions.

This was generated by AI, Review is declarative