util/yaml: make save_yaml atomic (temp + os.replace, Windows retry)

A plain truncate-and-write (open(path,"w")) is not atomic: a concurrent writer
(a second Serena process updating the auto-managed registered-projects list) or an
interrupted write can leave the file half-overwritten -- a shorter value over a longer
one leaves a stale tail, corrupting the YAML so every later load fails. This is what
wedged native benchmark runs (serena MCP crashed at startup with a ruamel ScannerError
-> 0 tools registered -> agent silently degraded to Bash).

Write to a temp file in the same dir, then os.replace onto the target (atomic;
last-writer-wins, never a corrupt interleave). Retry os.replace on Windows
PermissionError (WinError 5/32 sharing violation under contention). Adds
test/serena/util/test_yaml.py covering the shorter-over-longer corruption signature
plus a concurrent-writers stress.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

src/serena/util/yaml.py

@@ -1,5 +1,7 @@
 import logging
 import os
+import tempfile
+import time
 from collections.abc import Sequence
 from enum import Enum
 from typing import Any
@@ -199,9 +201,43 @@ def normalise_yaml_comments(commented_map: CommentedMap, comment_normalisation:
 
 def save_yaml(path: str, data: dict | CommentedMap, preserve_comments: bool = True) -> None:
     yaml = _create_yaml(preserve_comments)
-    os.makedirs(os.path.dirname(path), exist_ok=True)
-    with open(path, "w", encoding=SERENA_FILE_ENCODING) as f:
-        yaml.dump(data, f)
+    target_dir = os.path.dirname(path)
+    os.makedirs(target_dir, exist_ok=True)
+    # Atomic write: dump to a temp file in the SAME directory, then os.replace onto the target.
+    # A plain truncate-and-write (open(path, "w")) is NOT atomic: a concurrent writer (e.g. a second
+    # Serena process updating the auto-managed registered-projects list) or an interrupted write can
+    # leave the file half-overwritten — writing a shorter value over a longer one leaves a stale tail,
+    # which corrupts the YAML so it no longer parses and every later load fails. temp + os.replace makes
+    # each write all-or-nothing (last-writer-wins, never a corrupt interleave).
+    fd, tmp = tempfile.mkstemp(dir=target_dir, prefix=os.path.basename(path) + ".", suffix=".tmp")
+    try:
+        with os.fdopen(fd, "w", encoding=SERENA_FILE_ENCODING) as f:
+            yaml.dump(data, f)
+        _replace_with_retry(tmp, path)
+    except BaseException:
+        try:
+            os.unlink(tmp)
+        except OSError:
+            pass
+        raise
+
+
+def _replace_with_retry(src: str, dst: str, *, attempts: int = 10, delay_s: float = 0.05) -> None:
+    """``os.replace(src, dst)`` with a short retry on a Windows sharing violation.
+
+    On Windows the atomic rename fails with ``PermissionError`` (WinError 5/32) if another process
+    momentarily holds ``dst`` open — e.g. a second Serena process reading or replacing the same
+    config. A brief bounded retry rides out that contention; the temp file is still complete, so we
+    never fall back to a non-atomic write.
+    """
+    for attempt in range(attempts):
+        try:
+            os.replace(src, dst)
+            return
+        except PermissionError:
+            if attempt == attempts - 1:
+                raise
+            time.sleep(delay_s)
 
 
 def yaml_comment_entry_is_empty(comment_entry: Any) -> bool:

test/serena/util/test_yaml.py

@@ -0,0 +1,80 @@
+"""Tests for serena.util.yaml — focused on the atomic-write guarantee of ``save_yaml``.
+
+Regression: ``~/.serena/serena_config.yml`` was corrupted in the field when a non-atomic
+truncate-and-write saved a SHORTER value over a longer existing file (or two Serena processes
+saved concurrently), leaving a stale tail (e.g. a dangling ``ena`` line) that made the YAML
+unparseable, which wedged every later load. ``save_yaml`` now writes to a temp file and
+``os.replace``s it onto the target, so each write is all-or-nothing.
+"""
+
+import os
+import threading
+
+from ruamel.yaml import YAML
+
+from serena.util.yaml import load_yaml, save_yaml
+
+
+def _read_text(path: str) -> str:
+    with open(path, encoding="utf-8") as f:
+        return f.read()
+
+
+def test_save_yaml_roundtrips(tmp_path):
+    path = str(tmp_path / "config.yml")
+    save_yaml(path, {"projects": ["a", "b", "c"], "scalar": 1})
+    loaded = load_yaml(path)
+    assert list(loaded["projects"]) == ["a", "b", "c"]
+    assert loaded["scalar"] == 1
+
+
+def test_shorter_write_over_longer_leaves_no_stale_tail(tmp_path):
+    """The exact field corruption: a long file overwritten by a much shorter one.
+
+    A plain ``open(path, "w")`` truncates first, so this would pass even when buggy; the point
+    of the assertion is that the final file parses and contains EXACTLY the new content — no
+    leftover bytes from the longer previous version.
+    """
+    path = str(tmp_path / "config.yml")
+    long_projects = [f"C:/Users/misch/Projects/some/really/long/path/number/{i}/serena" for i in range(50)]
+    save_yaml(path, {"projects": long_projects})
+
+    save_yaml(path, {"projects": ["C:/Users/misch/Projects/oraios/serena"]})
+
+    loaded = load_yaml(path)
+    assert list(loaded["projects"]) == ["C:/Users/misch/Projects/oraios/serena"]
+    # no stale tail from the longer write survived
+    assert "number/49" not in _read_text(path)
+    # and it is still valid YAML on a fresh strict parse
+    with open(path, encoding="utf-8") as f:
+        YAML().load(f)
+
+
+def test_no_tmp_files_left_behind(tmp_path):
+    path = str(tmp_path / "config.yml")
+    save_yaml(path, {"projects": ["a"]})
+    leftovers = [p for p in os.listdir(tmp_path) if p != "config.yml"]
+    assert leftovers == [], f"temp files not cleaned up: {leftovers}"
+
+
+def test_concurrent_saves_never_corrupt(tmp_path):
+    """Many concurrent writers must yield a file that always parses (last-writer-wins is fine;
+    a corrupt interleave is not).
+    """
+    path = str(tmp_path / "config.yml")
+    save_yaml(path, {"projects": ["seed"]})
+
+    def writer(n: int) -> None:
+        for _ in range(20):
+            save_yaml(path, {"projects": [f"p{n}-{k}" for k in range(n + 1)]})
+
+    threads = [threading.Thread(target=writer, args=(n,)) for n in range(6)]
+    for t in threads:
+        t.start()
+    for t in threads:
+        t.join()
+
+    # whatever won, the file must be complete + parseable (no corruption)
+    loaded = load_yaml(path)
+    assert "projects" in loaded
+    assert all(isinstance(p, str) for p in loaded["projects"])