fix(setup): atomic write for interactive config file

CHANGELOG.md

@@ -4,6 +4,7 @@
 
 ### Fixed
 
+- **`mcp-synology setup` now writes the config file atomically** (#71) — closes #70. Cascades the atomic-write helper introduced in PR #69 (`core/fs.py::atomic_write_text`) into `cli/setup.py:178-183`, the last user-visible non-atomic write site in the project. Previously the interactive setup flow persisted the user-edited config via `config_path.write_text(header + raw_yaml, encoding="utf-8")` — same torn-write window as the runtime state files PR #69 fixed: a Ctrl+C, OOM, or power loss between the file truncate and the final write would leave a zero-byte or half-written `<instance_id>.yaml` that the next `mcp-synology check` / `serve` invocation would fail to parse. The window is small (single write of a small YAML payload) and the workflow is interactive, so the practical risk is much lower than the runtime-state case — but the helper was already sitting there ready to use, and closing this site means there are no remaining `path.write_text` calls in the project that persist user/runtime data. Drops the now-redundant `_CONFIG_DIR.mkdir(parents=True, exist_ok=True)` since `atomic_write_text` already creates parent directories. New `TestSetupAtomicConfigWrite::test_setup_writes_config_atomically_with_no_tmp_sibling` regression test in `tests/core/test_cli_setup.py` runs the full interactive setup flow against a not-pre-created `_CONFIG_DIR`, then asserts (a) the dir was auto-created (proves the helper ran), (b) no `.tmp` sibling lingers in the dir after a successful write, and (c) the resulting YAML starts with the generated header and contains the entered host.
 - **State file writes are now atomic** (#69) — closes #36. `core/state.py:save_state()` and `cli/version.py:_save_global_state()` previously called `path.write_text(...)`, which on POSIX is *not* atomic — a process kill (Claude Desktop quit, OOM, power loss) between the file truncate and the final write produces a zero-byte or half-written `state.yaml`/`global.yaml`. The next startup either fails to load it or silently falls back to default state, losing persistent state (device tokens, API version cache, update-check timestamps) and looping on re-authentication or spamming PyPI. New `core/fs.py` houses the shared `atomic_write_text(path, content, *, encoding="utf-8")` helper: it writes to a sibling `.tmp` file then `Path.replace()`s it onto the target. `os.replace` (which `Path.replace` maps to) is atomic on both POSIX and Windows, so a reader either sees the previous contents or the new contents — never a torn write. The helper also creates parent directories and best-effort cleans up the `.tmp` on rename failure (`FileNotFoundError` swallowed; other `OSError`s during cleanup logged at WARNING and the original exception is re-raised). Both `save_state` and `_save_global_state` now route through it; the per-call-site `path.parent.mkdir(parents=True, exist_ok=True)` is gone since the helper does that. Seven `TestAtomicWriteText` cases in `tests/core/test_fs.py` cover the happy path (writes content, creates missing parent dirs, overwrites existing file, no `.tmp` left behind on success, custom encoding) plus two `Path.replace`-patched cases that verify (a) the original `OSError` propagates, the `.tmp` sibling is removed, and no partial file lands at the target, and (b) an existing target file is preserved untouched when the rename fails.
 - **vdsm: refresh DSM search index for both setup-time and runtime-created test data** (#67) — fixes the `tests/test_integration.py::TestSearch::test_search_keyword_finds_directory` flake on vdsm CI. DSM Universal Search doesn't crawl non-indexed shares promptly on a freshly-booted vdsm, so search calls returned `0 results found` for several minutes after the data was created — well past the test's 65-second retry budget. Two-part fix: (1) `tests/vdsm/setup_dsm.py` calls `synoindex -A -d` for `/testshare/Documents` and `/testshare/Media` at golden-image build time, registering the static test data with DSM's search index. (2) New `refresh_search_index` async fixture in `tests/test_integration.py` (no-op default for real-NAS runs) and a vdsm override in `tests/vdsm/conftest.py` that invokes `synoindex -A -d` via SSH for the runtime-created `Bambu Studio` subdirectory. Without (2), the round-1 partial fix shipped via this PR's earlier head improved the test from 6-attempt fail to 4-attempt success — still flaky. With (2), search registers the new path synchronously and the test passes on attempt 1. SSH plumbing was lifted from `setup_dsm.py` into a new shared `tests/vdsm/ssh.py` module so both image-build-time and test-fixture-time SSH go through the same helper. Best-effort: a non-zero `synoindex` return logs a warning and falls back to the existing retry loop. Modifying `setup_dsm.py` invalidates the vdsm-workflow golden-image cache key, so the next CI run rebuilds the image with the fix baked in.
 - **Auth ignores empty / whitespace-only credentials at every read site** (#66) — closes #35. `_resolve_credentials()` in `core/auth.py` previously trusted that env / config / keyring values, if present, contained real credentials. Empty strings already fell through (Python truthiness handled them), but **whitespace-only** values like `auth: {username: "   "}` mid-edit slipped through and reached `login()` as `('   ', '\t', None)`, surfacing as a generic DSM 400 that pointed neither at the config nor at the empty values. New `_present_or_none()` helper returns the value unchanged when it has any non-whitespace content, otherwise `None`. Applied at all nine read sites (3 env vars × 3 storage tiers — env / plaintext config / keyring). Meaningful padding is preserved (e.g. a real password `"  pwd  "` keeps its spaces); only purely empty/whitespace-only inputs are filtered. Six regression tests cover whitespace at each strategy level (env, config, keyring), the empty-string regression, the whitespace-doesn't-shadow-valid-keyring case, and the preserve-internal-padding guarantee.

src/mcp_synology/cli/setup.py

@@ -175,11 +175,12 @@ def _setup_interactive(verbose: bool) -> None:
         click.echo("Aborted.")
         return
 
-    # Write config file
-    _CONFIG_DIR.mkdir(parents=True, exist_ok=True)
+    # Write config file atomically (closes #70)
+    from mcp_synology.core.fs import atomic_write_text
+
     raw_yaml = yaml.dump(config_dict, default_flow_style=False, sort_keys=False)
     header = "# Generated by mcp-synology setup\n"
-    config_path.write_text(header + raw_yaml, encoding="utf-8")
+    atomic_write_text(config_path, header + raw_yaml)
     click.echo(f"\nConfig written to {config_path}")
 
     # Emit Claude Desktop JSON snippet

src/mcp_synology/core/fs.py

@@ -1,7 +1,7 @@
 """Filesystem utilities used across `core/`.
 
-Currently houses `atomic_write_text`, used by `core/state.py` and
-`cli/version.py` to persist runtime state without a torn-write window.
+Currently houses `atomic_write_text`, used by every site that persists
+runtime state or user config without a torn-write window.
 """
 
 from __future__ import annotations

tests/core/test_cli_setup.py

@@ -490,3 +490,58 @@ def test_linux_uses_dbus_env_when_set(self, tmp_path: Path) -> None:
 
         assert result.exit_code == 0
         assert "/custom/bus" in result.output
+
+
+# ---------- atomic config-file write (closes #70) ----------
+
+
+class TestSetupAtomicConfigWrite:
+    """Regression for #70 — interactive setup persists the config file via
+    `atomic_write_text` (sibling .tmp + Path.replace), not the bare
+    `path.write_text` that the original implementation used. Verifies the
+    parent dir is auto-created (atomic_write_text handles it, replacing the
+    explicit `_CONFIG_DIR.mkdir(...)` that the call site previously did) and
+    that no `.tmp` sibling lingers after a successful write.
+    """
+
+    def test_setup_writes_config_atomically_with_no_tmp_sibling(self, tmp_path: Path) -> None:
+        config_dir = tmp_path / "config"  # intentionally not pre-created
+
+        clean_env: dict[str, str] = {
+            k: val for k, val in os.environ.items() if not k.startswith("SYNOLOGY_")
+        }
+
+        connect_result: dict[str, Any] = {"success": True}
+
+        runner = CliRunner()
+        with (
+            patch("mcp_synology.cli.setup._CONFIG_DIR", config_dir),
+            patch("mcp_synology.core.config.discover_config_path", side_effect=FileNotFoundError),
+            patch("mcp_synology.cli.setup._store_keyring", return_value=True),
+            patch("mcp_synology.cli.setup.asyncio.run", return_value=connect_result),
+            patch.dict(os.environ, clean_env, clear=True),
+        ):
+            result = runner.invoke(
+                main,
+                ["setup"],
+                input="192.168.1.50\nn\nread\n\nadmin\npassword\n",
+            )
+
+        assert result.exit_code == 0, f"setup failed:\n{result.output}"
+
+        # atomic_write_text creates parents; the test deliberately did not
+        # pre-create config_dir, so its existence proves the helper ran.
+        assert config_dir.exists() and config_dir.is_dir()
+
+        # No .tmp sibling left behind on the success path.
+        names = sorted(p.name for p in config_dir.iterdir())
+        assert not any(n.endswith(".tmp") for n in names), (
+            f"unexpected .tmp file lingered after setup: {names}"
+        )
+
+        # Exactly one YAML config file with the expected header + host.
+        yamls = [p for p in config_dir.iterdir() if p.suffix == ".yaml"]
+        assert len(yamls) == 1, f"expected exactly one yaml in config dir, got: {names}"
+        content = yamls[0].read_text(encoding="utf-8")
+        assert content.startswith("# Generated by mcp-synology setup\n")
+        assert "192.168.1.50" in content