fix(cli): route the remaining destructive user-file rewrites through atomic writes

`utils.atomic_write_text`'s docstring states the invariant: it exists "so that
every destructive file rewrite in the codebase shares one implementation."
Four full-file rewrites of *existing user-authored files* still bypass it and
use a bare truncating `open(path, "w")` / `Path.write_text()`, which truncates
the target before the new content is produced. A crash, SIGINT, or ENOSPC
mid-write therefore leaves the file empty or half-written.

In all four cases the read half degrades silently to a default rather than
erroring, so the damage is invisible and the next write cements it:

* `xai_retirement.apply_migration()` rewrites the user's config.yaml. Merged
  commit beaa1a08e added a readability guard here and noted the writer "lives
  outside the atomic_yaml_write path, so the chokepoint didn't cover it"; this
  closes the durability half it left open. `--no-backup` is a documented flag,
  so on that path the truncated file is the only copy that exists, and the
  loader returns early on `doc is None` — the next run reports nothing to
  migrate rather than surfacing the damage.
* `uninstall.remove_path_from_shell_configs()` rewrites the user's shell rc
  (~/.bashrc, ~/.zshrc, ...). Hermes does not own these files and this function
  takes no backup; the enclosing `except Exception` downgrades a partial write
  to a warning, so the next login just starts a bare shell.
* `web_routers.profiles.update_profile_soul()` replaces SOUL.md from the
  dashboard editor. The paired GET reports an unreadable file as
  `{"content": "", "exists": False}`, so an interrupted save presents as "your
  persona was never set" and the editor's next Save persists the empty document.
* `profile_distribution.write_manifest()` rewrites distribution.yaml on every
  install/update. `read_manifest` treats an unparseable manifest as "not a
  distribution", silently dropping update tracking and env_requires.

The xAI migration keeps its ruamel round-trip dumper (comments, key order and
quoting must survive) and now serializes to a string before handing the bytes
to the shared writer. `write_manifest` moves to `atomic_yaml_write`, whose
SafeDumper output the manifest already round-trips through, retiring the local
`_dump_yaml` helper.

`atomic_write_text` recreates the target from a 0600 temp file, so each of its
call sites re-applies the file's previous permission bits: `_secure_file`
deliberately leaves config.yaml alone under managed (NixOS 0640) and container
installs, shell rc files are normally 0644, and profile SOUL.md is created 0644
and never secured. `atomic_yaml_write` already preserves mode and owner itself.
Routing through `atomic_replace` also keeps a symlinked config.yaml or ~/.zshrc
(dotfiles repo, managed deployment) pointing at the real file.

Tests: one regression test per site fails on clean main (the interrupted write
completes there and destroys the file) and passes here; the remaining cases are
behaviour guards covering symlink survival, permission preservation, comment
round-tripping, and the existing happy paths.

hermes_cli/profile_distribution.py

@@ -251,12 +251,6 @@ def _load_yaml(text: str) -> Any:
     return yaml.safe_load(text)
 
 
-def _dump_yaml(data: Any) -> str:
-    import yaml
-
-    return yaml.safe_dump(data, sort_keys=False, default_flow_style=False)
-
-
 def read_manifest(profile_dir: Path) -> Optional[DistributionManifest]:
     """Return the manifest for *profile_dir*, or None if it isn't a distribution."""
     mf_path = profile_dir / MANIFEST_FILENAME
@@ -271,7 +265,20 @@ def read_manifest(profile_dir: Path) -> Optional[DistributionManifest]:
 
 def write_manifest(profile_dir: Path, manifest: DistributionManifest) -> Path:
     mf_path = profile_dir / MANIFEST_FILENAME
-    mf_path.write_text(_dump_yaml(manifest.to_dict()), encoding="utf-8")
+    # Route through the shared atomic YAML writer (temp file + fsync + atomic
+    # replace, preserving mode/owner and symlinks). A bare write_text()
+    # truncates distribution.yaml before the dump lands, and read_manifest()
+    # treats a missing-or-unparseable manifest as "not a distribution" -- so an
+    # interrupted install/update silently demotes the profile, losing update
+    # tracking and env_requires with no error surfaced anywhere.
+    from utils import atomic_yaml_write
+
+    atomic_yaml_write(
+        mf_path,
+        manifest.to_dict(),
+        sort_keys=False,
+        default_flow_style=False,
+    )
     return mf_path
 
 

hermes_cli/uninstall.py

@@ -8,6 +8,7 @@ Provides options for:
 
 import os
 import shutil
+import stat
 import subprocess
 import sys
 from pathlib import Path
@@ -87,7 +88,26 @@ def remove_path_from_shell_configs():
                 new_content = new_content.replace('\n\n\n', '\n\n')
             
             if new_content != original_content:
-                config_path.write_text(new_content, encoding="utf-8")
+                from utils import atomic_write_text
+
+                # This is the user's own shell rc, not a Hermes-owned file, and
+                # nothing in this function backs it up. A bare write_text()
+                # truncates it before the new content lands, so a crash or
+                # SIGINT mid-write leaves the user with an empty or truncated
+                # ~/.zshrc -- and the enclosing `except Exception` downgrades
+                # that to a warning, so the next login just starts a bare
+                # shell. atomic_replace also resolves a symlinked rc file, so a
+                # dotfiles-repo setup keeps the symlink instead of having it
+                # replaced by a regular file.
+                prior_mode = stat.S_IMODE(config_path.stat().st_mode)
+                atomic_write_text(config_path, new_content)
+                # atomic_write_text swaps in a fresh 0600 temp file; shell rc
+                # files are normally 0644 and removing Hermes' PATH block must
+                # not quietly change their permissions.
+                try:
+                    os.chmod(config_path, prior_mode)
+                except OSError:
+                    pass
                 removed_from.append(config_path)
                 
         except Exception as e:

hermes_cli/web_routers/profiles.py

@@ -14,6 +14,8 @@ late-binding seam in :mod:`hermes_cli.web_deps` so tests that
 
 import asyncio  # noqa: F401 — used by handlers
 import logging
+import os
+import stat
 import subprocess  # noqa: F401
 import sys  # noqa: F401
 import time  # noqa: F401
@@ -613,7 +615,28 @@ async def get_profile_soul(name: str):
 async def update_profile_soul(name: str, body: ProfileSoulUpdate):
     soul_path = _resolve_profile_dir(name) / "SOUL.md"
     try:
-        soul_path.write_text(body.content, encoding="utf-8")
+        from utils import atomic_write_text
+
+        # PUT replaces the whole persona document from the dashboard editor.
+        # A bare write_text() truncates SOUL.md before the new body lands, and
+        # the paired GET above reports an unreadable file as
+        # ``{"content": "", "exists": False}`` -- so an interrupted save shows
+        # up as "your persona was never set" and the editor's next Save
+        # persists that empty document over it.
+        try:
+            prior_mode = stat.S_IMODE(soul_path.stat().st_mode)
+        except OSError:
+            # First save for this profile -- there is no prior file to match.
+            prior_mode = None
+
+        atomic_write_text(soul_path, body.content)
+        # atomic_write_text swaps in a fresh 0600 temp file; profile SOUL.md is
+        # created 0644 and is not run through _secure_file, so re-apply.
+        if prior_mode is not None:
+            try:
+                os.chmod(soul_path, prior_mode)
+            except OSError:
+                pass
     except OSError as e:
         _log.exception("PUT /api/profiles/%s/soul failed", name)
         raise HTTPException(status_code=500, detail=f"Could not write SOUL.md: {e}")

hermes_cli/xai_retirement.py

@@ -137,6 +137,9 @@ def format_issue(issue: RetirementIssue) -> str:
 # ---------------------------------------------------------------------------
 
 import datetime as _dt
+import io
+import os
+import stat
 from pathlib import Path
 import shutil
 
@@ -243,10 +246,38 @@ def apply_migration(
         shutil.copy2(config_path, backup_path)
 
     from hermes_cli.config import require_readable_config_before_write
+    from utils import atomic_write_text
 
     require_readable_config_before_write(config_path)
-    with config_path.open("w", encoding="utf-8") as fh:
-        yaml.dump(doc, fh)
+
+    # Serialize with the round-trip dumper first, then hand the finished text
+    # to the shared atomic writer (temp file + fsync + atomic replace).
+    # ``open(config_path, "w")`` truncates before the dump runs, so a crash or
+    # SIGINT mid-write leaves config.yaml empty or half-written -- and
+    # ``--no-backup`` is a documented flag, so on that path the truncated file
+    # is the only copy left. The load half above returns early when ``doc is
+    # None``, so the next `hermes migrate xai` reports nothing to migrate
+    # rather than surfacing the damage. atomic_replace also keeps a symlinked
+    # config.yaml (dotfiles repo / managed deployment) intact (GitHub #16743).
+    buf = io.StringIO()
+    yaml.dump(doc, buf)
+
+    # atomic_write_text swaps in a fresh 0600 temp file, so carry the existing
+    # permission bits across: _secure_file deliberately leaves config.yaml
+    # alone under managed (NixOS 0640) and container installs, and a migration
+    # must not silently tighten what those setups widened.
+    try:
+        prior_mode = stat.S_IMODE(config_path.stat().st_mode)
+    except OSError:
+        prior_mode = None
+
+    atomic_write_text(config_path, buf.getvalue())
+
+    if prior_mode is not None:
+        try:
+            os.chmod(config_path, prior_mode)
+        except OSError:
+            pass
 
     return ApplyResult(
         file_path=config_path,

tests/hermes_cli/test_migrate_xai.py

@@ -206,3 +206,89 @@ class TestUnreadableExistingConfig:
             os.chmod(trap_config, 0o644)
 
         assert trap_config.read_bytes() == original
+
+
+# ---------------------------------------------------------------------------
+# Crash durability — the rewrite must be atomic
+# ---------------------------------------------------------------------------
+
+class TestCrashDurability:
+    """apply_migration() rewrites the whole config.yaml in place.
+
+    A bare ``open(path, "w")`` truncates the file *before* the dump runs, so an
+    interruption (crash, SIGINT, ENOSPC) leaves config.yaml empty or
+    half-written.  Routing through ``utils.atomic_write_text`` means the target
+    is only ever swapped in via an atomic rename after the temp file is fully
+    written and fsynced.
+    """
+
+    def test_config_survives_an_interrupted_write(self, trap_config: Path):
+        """A failure mid-write must leave the original config.yaml untouched.
+
+        ``--no-backup`` is a documented flag, so on that path the file being
+        rewritten is the only copy in existence.
+        """
+        import os
+
+        issues = find_retired_xai_refs(_parse(trap_config))
+        assert issues  # sanity: trap_config has retired refs
+        original = trap_config.read_bytes()
+
+        def boom(fd):
+            raise OSError("simulated crash mid-write")
+
+        with pytest.MonkeyPatch.context() as mp:
+            mp.setattr(os, "fsync", boom)
+            with pytest.raises(OSError):
+                apply_migration(trap_config, issues, backup=False)
+
+        # The original bytes must survive verbatim...
+        assert trap_config.read_bytes() == original
+        # ...and the aborted write must not leave a temp file behind.
+        assert list(trap_config.parent.glob("*.tmp")) == []
+
+    def test_symlinked_config_is_replaced_in_place(self, tmp_path: Path):
+        """A config.yaml symlinked into a dotfiles repo must stay a symlink."""
+        real_dir = tmp_path / "dotfiles"
+        real_dir.mkdir()
+        real = real_dir / "hermes-config.yaml"
+        real.write_text(
+            "principal:\n"
+            "  provider: xai\n"
+            "  model: grok-3\n",
+            encoding="utf-8",
+        )
+        link = tmp_path / "config.yaml"
+        link.symlink_to(real)
+
+        issues = find_retired_xai_refs(_parse(link))
+        assert issues
+        apply_migration(link, issues, backup=False)
+
+        assert link.is_symlink(), "atomic replace detached the symlink"
+        assert real.read_text(encoding="utf-8") == link.read_text(encoding="utf-8")
+        assert "grok-4.3" in real.read_text(encoding="utf-8")
+
+    def test_existing_file_mode_is_preserved(self, trap_config: Path):
+        """Managed (NixOS 0640) and container installs widen config.yaml
+        deliberately; the migration must not silently tighten it to 0600."""
+        import os
+        import stat
+
+        os.chmod(trap_config, 0o640)
+        issues = find_retired_xai_refs(_parse(trap_config))
+        apply_migration(trap_config, issues, backup=False)
+
+        mode = stat.S_IMODE(trap_config.stat().st_mode)
+        assert mode == 0o640, f"mode changed to {oct(mode)}"
+
+    def test_comments_survive_the_atomic_write(self, trap_config: Path):
+        """The ruamel round-trip must still run — serializing via a string
+        buffer instead of the file handle must not drop comments."""
+        issues = find_retired_xai_refs(_parse(trap_config))
+        apply_migration(trap_config, issues, backup=False)
+
+        text = trap_config.read_text(encoding="utf-8")
+        assert "# Hermes config (sample)" in text
+        assert "# the main model" in text
+        assert "# not affected" in text

tests/hermes_cli/test_profile_distribution.py

@@ -672,3 +672,64 @@ class TestErrorSurfaces:
         with pytest.raises((ValueError, DistributionError)):
             plan_install(str(staged), tmp_path / "work")
 
+
+# ===========================================================================
+# Crash durability: write_manifest rewrites distribution.yaml in place
+# ===========================================================================
+
+
+class TestManifestCrashDurability:
+    """``write_manifest`` runs on every install and update of a shared profile.
+
+    ``read_manifest`` reports a missing-or-unparseable manifest as "this isn't
+    a distribution", so a truncated distribution.yaml silently demotes the
+    profile — update tracking and ``env_requires`` just stop existing, with no
+    error surfaced anywhere.
+    """
+
+    def test_previous_manifest_survives_an_interrupted_write(self, tmp_path):
+        import os
+
+        original = DistributionManifest(
+            name="keepme",
+            version="1.0.0",
+            description="the manifest already on disk",
+            env_requires=[EnvRequirement(name="FOO", description="foo")],
+        )
+        write_manifest(tmp_path, original)
+        on_disk = (tmp_path / "distribution.yaml").read_bytes()
+
+        def boom(fd):
+            raise OSError("simulated crash mid-write")
+
+        with pytest.MonkeyPatch.context() as mp:
+            mp.setattr(os, "fsync", boom)
+            with pytest.raises(OSError):
+                write_manifest(
+                    tmp_path,
+                    DistributionManifest(name="replacement", version="2.0.0"),
+                )
+
+        # The old manifest must still be byte-identical and still parse.
+        assert (tmp_path / "distribution.yaml").read_bytes() == on_disk
+        parsed = read_manifest(tmp_path)
+        assert parsed is not None, "profile silently stopped being a distribution"
+        assert parsed.name == "keepme"
+        assert parsed.env_requires[0].name == "FOO"
+
+        # No temp file left behind next to the manifest.
+        assert list(tmp_path.glob("*.tmp")) == []
+
+    def test_existing_file_mode_is_preserved(self, tmp_path):
+        import os
+        import stat
+
+        write_manifest(tmp_path, DistributionManifest(name="modes", version="1.0.0"))
+        mf = tmp_path / "distribution.yaml"
+        os.chmod(mf, 0o644)
+
+        write_manifest(tmp_path, DistributionManifest(name="modes", version="2.0.0"))
+
+        mode = stat.S_IMODE(mf.stat().st_mode)
+        assert mode == 0o644, f"mode changed to {oct(mode)}"
+

tests/hermes_cli/test_uninstall_shell_configs.py

@@ -0,0 +1,116 @@
+"""Tests for ``remove_path_from_shell_configs`` — the uninstaller's shell-rc rewrite.
+
+This rewrites files Hermes does not own (``~/.bashrc``, ``~/.zshrc``, ...) and
+takes no backup of them, so the rewrite has to be atomic: a bare
+``write_text()`` truncates the rc file before the new content lands, and the
+caller wraps everything in ``except Exception: log_warn(...)``, so a partial
+write is downgraded to a warning and the user's next login starts a bare shell.
+"""
+from __future__ import annotations
+
+import os
+import stat
+from pathlib import Path
+
+import pytest
+
+from hermes_cli import uninstall
+
+
+ZSHRC = (
+    "export EDITOR=vim\n"
+    "alias ll='ls -la'\n"
+    "\n"
+    "# Hermes Agent\n"
+    'export PATH="$HOME/.local/bin:$PATH"\n'
+    "\n"
+    "source ~/.work-profile\n"
+)
+
+
+@pytest.fixture
+def fake_home(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Path:
+    """Point both ``Path.home()`` and ``HERMES_HOME`` at a throwaway dir."""
+    home = tmp_path / "home"
+    home.mkdir()
+    monkeypatch.setattr(Path, "home", classmethod(lambda cls: home))
+    monkeypatch.setenv("HERMES_HOME", str(home / ".hermes"))
+    return home
+
+
+class TestHappyPath:
+    def test_hermes_path_block_is_removed(self, fake_home: Path):
+        rc = fake_home / ".zshrc"
+        rc.write_text(ZSHRC, encoding="utf-8")
+
+        removed = uninstall.remove_path_from_shell_configs()
+
+        assert removed == [rc]
+        text = rc.read_text(encoding="utf-8")
+        assert "# Hermes Agent" not in text
+        # The user's own lines are untouched.
+        assert "export EDITOR=vim" in text
+        assert "source ~/.work-profile" in text
+
+    def test_untouched_rc_is_not_reported(self, fake_home: Path):
+        rc = fake_home / ".zshrc"
+        rc.write_text("export EDITOR=vim\n", encoding="utf-8")
+
+        assert uninstall.remove_path_from_shell_configs() == []
+        assert rc.read_text(encoding="utf-8") == "export EDITOR=vim\n"
+
+
+class TestCrashDurability:
+    def test_shell_config_survives_an_interrupted_rewrite(self, fake_home: Path):
+        """An interrupted rewrite must leave the rc file byte-identical.
+
+        There is no backup of the user's shell rc anywhere in this code path,
+        so a truncated write is unrecoverable.
+        """
+        rc = fake_home / ".zshrc"
+        rc.write_text(ZSHRC, encoding="utf-8")
+        original = rc.read_bytes()
+
+        def boom(fd):
+            raise OSError("simulated crash mid-write")
+
+        # Scoped context so restoring os.fsync doesn't also undo the
+        # Path.home()/HERMES_HOME patches the fake_home fixture installed.
+        with pytest.MonkeyPatch.context() as mp:
+            mp.setattr(os, "fsync", boom)
+            removed = uninstall.remove_path_from_shell_configs()
+
+        # The write failed, so the rc must not be reported as modified...
+        assert removed == []
+        # ...and it must still be exactly what the user had.
+        assert rc.read_bytes() == original
+        # The aborted write must not leave a temp file behind in $HOME.
+        assert list(fake_home.glob("*.tmp")) == []
+
+    def test_symlinked_shell_config_stays_a_symlink(self, fake_home: Path):
+        """A dotfiles-repo ``~/.zshrc`` is a symlink; replacing it with a
+        regular file silently detaches the user's dotfiles."""
+        dotfiles = fake_home / "dotfiles"
+        dotfiles.mkdir()
+        real = dotfiles / "zshrc"
+        real.write_text(ZSHRC, encoding="utf-8")
+        rc = fake_home / ".zshrc"
+        rc.symlink_to(real)
+
+        removed = uninstall.remove_path_from_shell_configs()
+
+        assert removed == [rc]
+        assert rc.is_symlink(), "the symlink was replaced by a regular file"
+        assert "# Hermes Agent" not in real.read_text(encoding="utf-8")
+        assert "export EDITOR=vim" in real.read_text(encoding="utf-8")
+
+    def test_existing_file_mode_is_preserved(self, fake_home: Path):
+        """Shell rc files are normally 0644; uninstalling must not change that."""
+        rc = fake_home / ".zshrc"
+        rc.write_text(ZSHRC, encoding="utf-8")
+        os.chmod(rc, 0o644)
+
+        uninstall.remove_path_from_shell_configs()
+
+        mode = stat.S_IMODE(rc.stat().st_mode)
+        assert mode == 0o644, f"mode changed to {oct(mode)}"

tests/hermes_cli/test_web_profile_soul_writes.py

@@ -0,0 +1,110 @@
+"""``PUT /api/profiles/{name}/soul`` must not destroy an existing SOUL.md.
+
+The dashboard persona editor replaces the whole document on every Save. A bare
+``write_text()`` truncates SOUL.md before the new body lands, and the paired
+``GET`` reports an unreadable file as ``{"content": "", "exists": False}`` — so
+an interrupted save presents as "your persona was never set" and the editor's
+next Save persists that empty document over the original.
+
+Lives in its own module rather than ``test_web_server.py`` to keep the harness
+small and focused on this one endpoint pair.
+"""
+
+from __future__ import annotations
+
+import os
+import stat
+from pathlib import Path
+
+import pytest
+
+fastapi = pytest.importorskip("fastapi")
+from fastapi.testclient import TestClient  # noqa: E402
+
+
+SOUL = "# Persona\n\nYou are a careful, terse assistant.\n"
+
+
+@pytest.fixture()
+def client(tmp_path, monkeypatch):
+    monkeypatch.setenv("HERMES_HOME", str(tmp_path))
+    monkeypatch.setenv("HERMES_DASHBOARD_SESSION_TOKEN", "soul-test-token")
+    from hermes_cli import web_server
+
+    with TestClient(web_server.app, raise_server_exceptions=False) as c:
+        c.headers["Authorization"] = "Bearer soul-test-token"
+        yield c
+
+
+@pytest.fixture()
+def profile_dir(tmp_path, monkeypatch) -> Path:
+    """Create a real profile directory under the test HERMES_HOME."""
+    monkeypatch.setenv("HERMES_HOME", str(tmp_path))
+    from hermes_cli import profiles as profiles_mod
+
+    d = profiles_mod.get_profile_dir("demo")
+    d.mkdir(parents=True, exist_ok=True)
+    return d
+
+
+class TestSoulWriteDurability:
+    def test_put_replaces_soul(self, client, profile_dir: Path):
+        """Happy path: the editor's Save still works."""
+        (profile_dir / "SOUL.md").write_text(SOUL, encoding="utf-8")
+
+        r = client.put("/api/profiles/demo/soul", json={"content": "# New\n"})
+
+        assert r.status_code == 200, r.text
+        assert (profile_dir / "SOUL.md").read_text(encoding="utf-8") == "# New\n"
+
+    def test_put_creates_soul_when_absent(self, client, profile_dir: Path):
+        """A first save has no prior file to preserve permissions from."""
+        assert not (profile_dir / "SOUL.md").exists()
+
+        r = client.put("/api/profiles/demo/soul", json={"content": SOUL})
+
+        assert r.status_code == 200, r.text
+        assert (profile_dir / "SOUL.md").read_text(encoding="utf-8") == SOUL
+
+    def test_existing_soul_survives_an_interrupted_save(
+        self, client, profile_dir: Path
+    ):
+        soul = profile_dir / "SOUL.md"
+        soul.write_text(SOUL, encoding="utf-8")
+        original = soul.read_bytes()
+
+        def boom(fd):
+            raise OSError("simulated crash mid-write")
+
+        # Scoped context so restoring os.fsync doesn't also undo the
+        # HERMES_HOME patch the client/profile_dir fixtures installed.
+        with pytest.MonkeyPatch.context() as mp:
+            mp.setattr(os, "fsync", boom)
+            r = client.put(
+                "/api/profiles/demo/soul", json={"content": "# clobbered\n"}
+            )
+
+        assert r.status_code == 500
+        # The persona the user already had must survive verbatim...
+        assert soul.read_bytes() == original
+        # ...and the paired GET must not report it as never-set, which is what
+        # would make the next Save persist an empty document.
+        g = client.get("/api/profiles/demo/soul")
+        assert g.status_code == 200, g.text
+        assert g.json()["exists"] is True
+        assert g.json()["content"] == SOUL
+        # No temp file left behind in the profile directory.
+        assert list(profile_dir.glob("*.tmp")) == []
+
+    def test_existing_file_mode_is_preserved(self, client, profile_dir: Path):
+        """Profile SOUL.md is created 0644 and never run through
+        ``_secure_file``; saving from the dashboard must not change that."""
+        soul = profile_dir / "SOUL.md"
+        soul.write_text(SOUL, encoding="utf-8")
+        os.chmod(soul, 0o644)
+
+        r = client.put("/api/profiles/demo/soul", json={"content": "# New\n"})
+
+        assert r.status_code == 200, r.text
+        mode = stat.S_IMODE(soul.stat().st_mode)
+        assert mode == 0o644, f"mode changed to {oct(mode)}"