From c94a5fa1b2cbf6074e6feb56622020647987abe5 Mon Sep 17 00:00:00 2001
From: binhnt92 <binhnt92@users.noreply.github.com>
Date: Tue, 31 Mar 2026 12:19:10 -0700
Subject: [PATCH] fix(cli): use atomic write in save_config_value to prevent
 config loss on interrupt

save_config_value() used bare open(path, 'w') + yaml.dump() which truncates
the file to zero bytes on open. If the process is interrupted mid-write,
config.yaml is left empty. Replace with atomic_yaml_write() (temp file +
fsync + os.replace), matching the gateway config write path.

Co-authored-by: Hermes Agent <hermes@nousresearch.com>
---
 cli.py                              |  7 +--
 tests/test_cli_save_config_value.py | 80 +++++++++++++++++++++++++++++
 2 files changed, 84 insertions(+), 3 deletions(-)
 create mode 100644 tests/test_cli_save_config_value.py

diff --git a/cli.py b/cli.py
index e5f88e752..1f72207aa 100644
--- a/cli.py
+++ b/cli.py
@@ -991,9 +991,10 @@ def save_config_value(key_path: str, value: any) -> bool:
             current = current[key]
         current[keys[-1]] = value
         
-        # Save back
-        with open(config_path, 'w') as f:
-            yaml.dump(config, f, default_flow_style=False, sort_keys=False)
+        # Save back atomically — write to temp file + fsync + os.replace
+        # so an interrupt never leaves config.yaml truncated or empty.
+        from utils import atomic_yaml_write
+        atomic_yaml_write(config_path, config)
         
         # Enforce owner-only permissions on config files (contain API keys)
         try:
diff --git a/tests/test_cli_save_config_value.py b/tests/test_cli_save_config_value.py
new file mode 100644
index 000000000..7d030c03c
--- /dev/null
+++ b/tests/test_cli_save_config_value.py
@@ -0,0 +1,80 @@
+"""Tests for save_config_value() in cli.py — atomic write behavior."""
+
+import os
+import yaml
+from pathlib import Path
+from unittest.mock import patch, MagicMock
+
+import pytest
+
+
+class TestSaveConfigValueAtomic:
+    """save_config_value() must use atomic_yaml_write to avoid data loss."""
+
+    @pytest.fixture
+    def config_env(self, tmp_path, monkeypatch):
+        """Isolated config environment with a writable config.yaml."""
+        hermes_home = tmp_path / ".hermes"
+        hermes_home.mkdir()
+        config_path = hermes_home / "config.yaml"
+        config_path.write_text(yaml.dump({
+            "model": {"default": "test-model", "provider": "openrouter"},
+            "display": {"skin": "default"},
+        }))
+        monkeypatch.setattr("cli._hermes_home", hermes_home)
+        return config_path
+
+    def test_calls_atomic_yaml_write(self, config_env, monkeypatch):
+        """save_config_value must route through atomic_yaml_write, not bare open()."""
+        mock_atomic = MagicMock()
+        monkeypatch.setattr("utils.atomic_yaml_write", mock_atomic)
+
+        from cli import save_config_value
+        save_config_value("display.skin", "mono")
+
+        mock_atomic.assert_called_once()
+        written_path, written_data = mock_atomic.call_args[0]
+        assert Path(written_path) == config_env
+        assert written_data["display"]["skin"] == "mono"
+
+    def test_preserves_existing_keys(self, config_env):
+        """Writing a new key must not clobber existing config entries."""
+        from cli import save_config_value
+        save_config_value("agent.max_turns", 50)
+
+        result = yaml.safe_load(config_env.read_text())
+        assert result["model"]["default"] == "test-model"
+        assert result["model"]["provider"] == "openrouter"
+        assert result["display"]["skin"] == "default"
+        assert result["agent"]["max_turns"] == 50
+
+    def test_creates_nested_keys(self, config_env):
+        """Dot-separated paths create intermediate dicts as needed."""
+        from cli import save_config_value
+        save_config_value("compression.summary_model", "google/gemini-3-flash-preview")
+
+        result = yaml.safe_load(config_env.read_text())
+        assert result["compression"]["summary_model"] == "google/gemini-3-flash-preview"
+
+    def test_overwrites_existing_value(self, config_env):
+        """Updating an existing key replaces the value."""
+        from cli import save_config_value
+        save_config_value("display.skin", "ares")
+
+        result = yaml.safe_load(config_env.read_text())
+        assert result["display"]["skin"] == "ares"
+
+    def test_file_not_truncated_on_error(self, config_env, monkeypatch):
+        """If atomic_yaml_write raises, the original file is untouched."""
+        original_content = config_env.read_text()
+
+        def exploding_write(*args, **kwargs):
+            raise OSError("disk full")
+
+        monkeypatch.setattr("utils.atomic_yaml_write", exploding_write)
+
+        from cli import save_config_value
+        result = save_config_value("display.skin", "broken")
+
+        assert result is False
+        assert config_env.read_text() == original_content