fix(cli): preserve settings file metadata on atomic merge write

Author: RbBtSn0w

src/specify_cli/__init__.py

@@ -34,6 +34,7 @@
 import shlex
 import json
 import json5
+import stat
 import yaml
 from pathlib import Path
 from typing import Any, Optional, Tuple
@@ -656,11 +657,47 @@ def init_git_repo(project_path: Path, quiet: bool = False) -> Tuple[bool, Option
         os.chdir(original_cwd)
 
 def handle_vscode_settings(sub_item, dest_file, rel_path, verbose=False, tracker=None) -> None:
-    """Handle merging or copying of .vscode/settings.json files."""
+    """Handle merging or copying of .vscode/settings.json files.
+
+    Note: when merge produces changes, rewritten output is normalized JSON and
+    existing JSONC comments/trailing commas are not preserved.
+    """
     def log(message, color="green"):
         if verbose and not tracker:
             console.print(f"[{color}]{message}[/] {rel_path}")
 
+    def atomic_write_json(target_file: Path, payload: dict[str, Any]) -> None:
+        """Atomically write JSON while preserving existing mode bits when possible."""
+        fd, temp_path = tempfile.mkstemp(
+            dir=target_file.parent,
+            prefix=f"{target_file.name}.",
+            suffix=".tmp",
+        )
+        try:
+            with os.fdopen(fd, 'w', encoding='utf-8') as f:
+                json.dump(payload, f, indent=4)
+                f.write('\n')
+
+            if target_file.exists():
+                try:
+                    existing_stat = target_file.stat()
+                    os.chmod(temp_path, stat.S_IMODE(existing_stat.st_mode))
+                    if hasattr(os, "chown"):
+                        try:
+                            os.chown(temp_path, existing_stat.st_uid, existing_stat.st_gid)
+                        except PermissionError:
+                            # Best-effort owner/group preservation without requiring elevated privileges.
+                            pass
+                except OSError:
+                    # Best-effort metadata preservation; data safety is prioritized.
+                    pass
+
+            os.replace(temp_path, target_file)
+        except Exception:
+            if os.path.exists(temp_path):
+                os.unlink(temp_path)
+            raise
+
     try:
         with open(sub_item, 'r', encoding='utf-8') as f:
             # json5 natively supports comments and trailing commas (JSONC)
@@ -669,18 +706,9 @@ def log(message, color="green"):
         if dest_file.exists():
             merged = merge_json_files(dest_file, new_settings, verbose=verbose and not tracker)
             if merged is not None:
-                # Use atomic write: write to temp file then replace
-                fd, temp_path = tempfile.mkstemp(dir=dest_file.parent, prefix="settings.", suffix=".json")
-                try:
-                    with os.fdopen(fd, 'w', encoding='utf-8') as f:
-                        json.dump(merged, f, indent=4)
-                        f.write('\n')
-                    os.replace(temp_path, dest_file)
-                except Exception:
-                    if os.path.exists(temp_path):
-                        os.unlink(temp_path)
-                    raise
+                atomic_write_json(dest_file, merged)
                 log("Merged:", "green")
+                log("Note: comments/trailing commas are normalized when rewritten", "yellow")
             else:
                 log("Skipped merge (preserved existing settings)", "yellow")
         else:
@@ -762,7 +790,7 @@ def deep_merge_polite(base: dict[str, Any], update: dict[str, Any]) -> dict[str,
 
     merged = deep_merge_polite(existing_content, new_content)
 
-    # Detect if anything actually changed. If not, return None so the caller 
+    # Detect if anything actually changed. If not, return None so the caller
     # can skip rewriting the file (preserving user's comments/formatting).
     if merged == existing_content:
         return None

tests/test_merge.py

@@ -1,4 +1,7 @@
+import stat
+
 from specify_cli import merge_json_files
+from specify_cli import handle_vscode_settings
 
 # --- Dimension 2: Polite Deep Merge Strategy ---
 
@@ -160,3 +163,28 @@ def test_merge_json_files_type_mismatch_no_op(tmp_path):
 
     # Should return None because we preserved the user's string and nothing else changed
     assert merge_json_files(existing_file, template_settings) is None
+
+
+def test_handle_vscode_settings_preserves_mode_on_atomic_write(tmp_path):
+    """Atomic rewrite should preserve existing file mode bits."""
+    vscode_dir = tmp_path / ".vscode"
+    vscode_dir.mkdir()
+    dest_file = vscode_dir / "settings.json"
+    template_file = tmp_path / "template_settings.json"
+
+    dest_file.write_text('{"a": 1}\n', encoding="utf-8")
+    dest_file.chmod(0o640)
+    before_mode = stat.S_IMODE(dest_file.stat().st_mode)
+
+    template_file.write_text('{"b": 2}\n', encoding="utf-8")
+
+    handle_vscode_settings(
+        template_file,
+        dest_file,
+        "settings.json",
+        verbose=False,
+        tracker=None,
+    )
+
+    after_mode = stat.S_IMODE(dest_file.stat().st_mode)
+    assert after_mode == before_mode