From f6b7f85fbf02bdac60649d488344653e6a6c9774 Mon Sep 17 00:00:00 2001 From: Tony Narlock Date: Sun, 29 Mar 2026 04:59:30 -0500 Subject: [PATCH 01/12] fix(config[_atomic_write]) resolve symlinks before atomic rename MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit why: `Path.replace()` operates on directory entries, not file contents. When the target is a symlink, the symlink itself gets replaced by the temp file — the real target file is left stale. This breaks dotfile setups where `~/.vcspull.yaml` symlinks to a managed location. what: - Resolve target path before creating temp file and renaming - Temp file now created next to the real destination (also fixes latent cross-device rename when symlink spans filesystems) --- src/vcspull/config.py | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/src/vcspull/config.py b/src/vcspull/config.py index 85c46ff6..48502b6d 100644 --- a/src/vcspull/config.py +++ b/src/vcspull/config.py @@ -668,20 +668,28 @@ def is_config_file( def _atomic_write(target: pathlib.Path, content: str) -> None: """Write content to a file atomically via temp-file-then-rename. + If *target* is a symbolic link the write goes through the symlink: + the temporary file is created next to the resolved destination and + the rename replaces the resolved path, leaving the symlink intact. + Parameters ---------- target : pathlib.Path - Destination file path + Destination file path (may be a symlink) content : str Content to write """ + # Resolve symlinks so the temp file lives next to the real + # destination and the rename replaces the real file, not the symlink. + resolved = target.resolve() + original_mode: int | None = None - if target.exists(): - original_mode = target.stat().st_mode + if resolved.exists(): + original_mode = resolved.stat().st_mode fd, tmp_path = tempfile.mkstemp( - dir=target.parent, - prefix=f".{target.name}.", + dir=resolved.parent, + prefix=f".{resolved.name}.", suffix=".tmp", ) try: @@ -689,7 +697,7 @@ def _atomic_write(target: pathlib.Path, content: str) -> None: f.write(content) if original_mode is not None: pathlib.Path(tmp_path).chmod(original_mode) - pathlib.Path(tmp_path).replace(target) + pathlib.Path(tmp_path).replace(resolved) except BaseException: # Clean up the temp file on any failure with contextlib.suppress(OSError): From b425eba1c5eb1cbc4c9d1b09512a481d0a37fd5f Mon Sep 17 00:00:00 2001 From: Tony Narlock Date: Sun, 29 Mar 2026 04:59:42 -0500 Subject: [PATCH 02/12] fix(config[find_home_config_files]) resolve symlinks in path discovery MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit why: When no `-f` flag is given, config paths returned by `find_home_config_files()` used `.expanduser()` only — the symlink path was passed downstream. The `-f` codepaths already use `.expanduser().resolve()`. This makes discovery consistent. what: - Add `.resolve()` after `.expanduser()` for both YAML and JSON paths --- src/vcspull/config.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/vcspull/config.py b/src/vcspull/config.py index 48502b6d..54fa5958 100644 --- a/src/vcspull/config.py +++ b/src/vcspull/config.py @@ -357,9 +357,9 @@ def find_home_config_files( check_yaml = "yaml" in filetype check_json = "json" in filetype - yaml_config = pathlib.Path("~/.vcspull.yaml").expanduser() + yaml_config = pathlib.Path("~/.vcspull.yaml").expanduser().resolve() has_yaml_config = check_yaml and yaml_config.exists() - json_config = pathlib.Path("~/.vcspull.json").expanduser() + json_config = pathlib.Path("~/.vcspull.json").expanduser().resolve() has_json_config = check_json and json_config.exists() if not has_yaml_config and not has_json_config: From d37d5133b11c0de55c34729439d3d0d6a9e21b9e Mon Sep 17 00:00:00 2001 From: Tony Narlock Date: Sun, 29 Mar 2026 04:59:51 -0500 Subject: [PATCH 03/12] test(config[symlinks]) add regression tests for symlink preservation why: No existing tests verified that config writes through symlinks preserve the link. These tests prevent future regressions. what: - Test _atomic_write preserves symlink and updates real target - Test _atomic_write preserves file permissions through symlink - Test save_config end-to-end through symlink - Test find_home_config_files returns resolved path for symlinks --- tests/test_config_file.py | 20 +++++++++++ tests/test_config_writer.py | 68 +++++++++++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+) diff --git a/tests/test_config_file.py b/tests/test_config_file.py index 6a30b1fa..508636ba 100644 --- a/tests/test_config_file.py +++ b/tests/test_config_file.py @@ -244,6 +244,26 @@ def test_find_home_config_files_both_types_still_raises( config.find_home_config_files() +def test_find_home_config_files_resolves_symlink(tmp_path: pathlib.Path) -> None: + """Symlinked ~/.vcspull.yaml should be resolved to the real target path.""" + dotfiles_dir = tmp_path / ".dot-config" + dotfiles_dir.mkdir() + real_file = dotfiles_dir / ".vcspull.yaml" + real_file.write_text("~/code/: {}\n", encoding="utf-8") + + symlink = tmp_path / ".vcspull.yaml" + symlink.symlink_to(real_file) + + with EnvironmentVarGuard() as env: + env.set("HOME", str(tmp_path)) + results = config.find_home_config_files() + + assert len(results) == 1 + # Must be the resolved (real) path, not the symlink + assert results[0] == real_file.resolve() + assert not results[0].is_symlink() + + def test_in_dir( config_path: pathlib.Path, yaml_config: pathlib.Path, diff --git a/tests/test_config_writer.py b/tests/test_config_writer.py index fbc7c7fb..b596b1d4 100644 --- a/tests/test_config_writer.py +++ b/tests/test_config_writer.py @@ -8,6 +8,8 @@ import pytest from vcspull.config import ( + _atomic_write, + save_config, save_config_json, save_config_yaml, save_config_yaml_with_items, @@ -171,3 +173,69 @@ def test_save_config_json_atomic_preserves_permissions( save_config_json(config_path, data) assert config_path.stat().st_mode & 0o777 == 0o644 + + +def test_atomic_write_through_symlink_preserves_symlink( + tmp_path: pathlib.Path, +) -> None: + """Writing through a symlink should update the target and keep the link.""" + target_dir = tmp_path / "dotfiles" + target_dir.mkdir() + real_file = target_dir / ".vcspull.yaml" + real_file.write_text("~/old/: {}\n", encoding="utf-8") + + link_path = tmp_path / ".vcspull.yaml" + link_path.symlink_to(real_file) + + _atomic_write(link_path, "~/new/:\n repo: {}\n") + + # Symlink must still be a symlink pointing to the original target + assert link_path.is_symlink() + assert link_path.resolve() == real_file.resolve() + + # Real file has the new content + assert real_file.read_text(encoding="utf-8") == "~/new/:\n repo: {}\n" + + # No temp files left in either directory + for d in (tmp_path, target_dir): + leftovers = [f for f in d.iterdir() if ".tmp" in f.name] + assert leftovers == [] + + +def test_atomic_write_through_symlink_preserves_permissions( + tmp_path: pathlib.Path, +) -> None: + """File permissions of the real target should survive a symlink write.""" + target_dir = tmp_path / "dotfiles" + target_dir.mkdir() + real_file = target_dir / ".vcspull.yaml" + real_file.write_text("~/old/: {}\n", encoding="utf-8") + real_file.chmod(0o600) + + link_path = tmp_path / ".vcspull.yaml" + link_path.symlink_to(real_file) + + _atomic_write(link_path, "~/new/:\n repo: {}\n") + + assert real_file.stat().st_mode & 0o777 == 0o600 + assert link_path.is_symlink() + + +def test_save_config_via_symlink_preserves_link( + tmp_path: pathlib.Path, +) -> None: + """Full save_config path should preserve symlinks end-to-end.""" + target_dir = tmp_path / "dotfiles" + target_dir.mkdir() + real_file = target_dir / ".vcspull.yaml" + real_file.write_text("~/old/: {}\n", encoding="utf-8") + + link_path = tmp_path / ".vcspull.yaml" + link_path.symlink_to(real_file) + + data = {"~/code/": {"myrepo": {"repo": "git+https://example.com/repo.git"}}} + save_config(link_path, data) + + assert link_path.is_symlink() + assert link_path.resolve() == real_file.resolve() + assert "myrepo" in real_file.read_text(encoding="utf-8") From 9891374d9b2ebd7dee4b31c8e8d442f6af9b03ce Mon Sep 17 00:00:00 2001 From: Tony Narlock Date: Sun, 29 Mar 2026 05:22:09 -0500 Subject: [PATCH 04/12] docs(config[_atomic_write]) add doctests for atomic write and symlink behavior why: _atomic_write had no doctest coverage. Inline examples show basic usage and the symlink-preservation guarantee introduced in this branch. what: - Add doctest for basic atomic write - Add doctest demonstrating symlink preservation --- src/vcspull/config.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/vcspull/config.py b/src/vcspull/config.py index 54fa5958..27390745 100644 --- a/src/vcspull/config.py +++ b/src/vcspull/config.py @@ -678,6 +678,26 @@ def _atomic_write(target: pathlib.Path, content: str) -> None: Destination file path (may be a symlink) content : str Content to write + + Examples + -------- + >>> import pathlib, tempfile + >>> with tempfile.TemporaryDirectory() as tmp: + ... p = pathlib.Path(tmp) / "test.txt" + ... _atomic_write(p, "hello") + ... p.read_text(encoding="utf-8") + 'hello' + + Symlinks are preserved — the real target is updated: + + >>> with tempfile.TemporaryDirectory() as tmp: + ... real = pathlib.Path(tmp) / "real.txt" + ... _ = real.write_text("old", encoding="utf-8") + ... link = pathlib.Path(tmp) / "link.txt" + ... link.symlink_to(real) + ... _atomic_write(link, "new") + ... link.is_symlink(), link.read_text(encoding="utf-8") + (True, 'new') """ # Resolve symlinks so the temp file lives next to the real # destination and the rename replaces the real file, not the symlink. From 002fcb76377cfbed371411d2b8c628d9a961eb25 Mon Sep 17 00:00:00 2001 From: Tony Narlock Date: Sun, 29 Mar 2026 05:22:48 -0500 Subject: [PATCH 05/12] docs(config[find_home_config_files]) document symlink resolution and add param/return sections why: Docstring did not reflect the behavioral change from adding `.resolve()`, and was missing NumPy-style parameter/return sections. what: - Note that returned paths are resolved through symlinks - Add Parameters and Returns sections per NumPy docstring standard --- src/vcspull/config.py | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/vcspull/config.py b/src/vcspull/config.py index 27390745..4516ea4b 100644 --- a/src/vcspull/config.py +++ b/src/vcspull/config.py @@ -349,7 +349,21 @@ def is_valid_config_dict(val: t.Any) -> t.TypeGuard[ConfigDict]: def find_home_config_files( filetype: list[str] | None = None, ) -> list[pathlib.Path]: - """Return configs of ``.vcspull.{yaml,json}`` in user's home directory.""" + """Return configs of ``.vcspull.{yaml,json}`` in user's home directory. + + Paths are resolved through symlinks so callers receive the real + filesystem location, consistent with the ``-f`` flag codepath. + + Parameters + ---------- + filetype : list of str, optional + File types to search for (default ``["json", "yaml"]``) + + Returns + ------- + list of pathlib.Path + Resolved paths to discovered config files + """ if filetype is None: filetype = ["json", "yaml"] configs: list[pathlib.Path] = [] From 46fdc2858cb138d698bb6cda1b1f0d35a37e1fac Mon Sep 17 00:00:00 2001 From: Tony Narlock Date: Sun, 29 Mar 2026 05:23:52 -0500 Subject: [PATCH 06/12] docs(config[find_home_config_files]) add doctest why: find_home_config_files had no doctest coverage. Inline example shows the empty-return case when no home config exists. what: - Add doctest demonstrating empty return when no home config exists --- src/vcspull/config.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/vcspull/config.py b/src/vcspull/config.py index 4516ea4b..81d66429 100644 --- a/src/vcspull/config.py +++ b/src/vcspull/config.py @@ -363,6 +363,11 @@ def find_home_config_files( ------- list of pathlib.Path Resolved paths to discovered config files + + Examples + -------- + >>> find_home_config_files() + [] """ if filetype is None: filetype = ["json", "yaml"] From 9d9114da9e03447c79464444c63d983c36a7b37c Mon Sep 17 00:00:00 2001 From: Tony Narlock Date: Sun, 29 Mar 2026 06:30:54 -0500 Subject: [PATCH 07/12] fix(config[symlinked-config]) preserve logical config file type why: Symlinked config entries in `$HOME` or via `-f/--file` can point to extensionless or differently named targets. Resolving those paths before format detection changes the suffix that downstream code sees, which can make vcspull parse or save with the wrong format or reject supported symlinked dotfile setups outright. what: - Add a shared config-path normalizer that expands paths without resolving symlinks - Use the logical config path for home discovery and explicit config path handling in add, discover, fmt, and import - Keep `_atomic_write()` following the real target so writes still update the destination file while preserving the symlink entry - Add regressions for home config discovery, explicit config path resolution, and end-to-end `vcspull add` behavior through a symlinked home config --- src/vcspull/cli/add.py | 5 ++- src/vcspull/cli/discover.py | 5 ++- src/vcspull/cli/fmt.py | 5 ++- src/vcspull/cli/import_cmd/_common.py | 5 ++- src/vcspull/config.py | 53 ++++++++++++++++++++++++--- tests/cli/test_add.py | 37 +++++++++++++++++++ tests/cli/test_import_repos.py | 23 ++++++++++++ tests/test_config_file.py | 15 +++++--- 8 files changed, 132 insertions(+), 16 deletions(-) diff --git a/src/vcspull/cli/add.py b/src/vcspull/cli/add.py index b12b2164..979e913b 100644 --- a/src/vcspull/cli/add.py +++ b/src/vcspull/cli/add.py @@ -22,6 +22,7 @@ get_pin_reason, is_pinned_for_op, merge_duplicate_workspace_roots, + normalize_config_file_path, save_config, save_config_json, save_config_yaml_with_items, @@ -508,7 +509,9 @@ def add_repo( # Determine config file config_file_path: pathlib.Path if config_file_path_str: - config_file_path = pathlib.Path(config_file_path_str).expanduser().resolve() + config_file_path = normalize_config_file_path( + pathlib.Path(config_file_path_str) + ) else: home_configs = find_home_config_files(filetype=["yaml"]) if not home_configs: diff --git a/src/vcspull/cli/discover.py b/src/vcspull/cli/discover.py index 9e64a183..af9041ec 100644 --- a/src/vcspull/cli/discover.py +++ b/src/vcspull/cli/discover.py @@ -22,6 +22,7 @@ get_pin_reason, is_pinned_for_op, merge_duplicate_workspace_roots, + normalize_config_file_path, normalize_workspace_roots, save_config, workspace_root_label, @@ -327,7 +328,9 @@ def discover_repos( config_file_path: pathlib.Path if config_file_path_str: - config_file_path = pathlib.Path(config_file_path_str).expanduser().resolve() + config_file_path = normalize_config_file_path( + pathlib.Path(config_file_path_str) + ) else: home_configs = find_home_config_files(filetype=["yaml"]) if not home_configs: diff --git a/src/vcspull/cli/fmt.py b/src/vcspull/cli/fmt.py index a4c0a18c..858c32a7 100644 --- a/src/vcspull/cli/fmt.py +++ b/src/vcspull/cli/fmt.py @@ -19,6 +19,7 @@ find_home_config_files, is_pinned_for_op, merge_duplicate_workspace_roots, + normalize_config_file_path, normalize_workspace_roots, save_config, ) @@ -570,7 +571,9 @@ def format_config_file( else: # Format single config file if config_file_path_str: - config_file_path = pathlib.Path(config_file_path_str).expanduser().resolve() + config_file_path = normalize_config_file_path( + pathlib.Path(config_file_path_str) + ) else: home_configs = find_home_config_files(filetype=["yaml"]) if not home_configs: diff --git a/src/vcspull/cli/import_cmd/_common.py b/src/vcspull/cli/import_cmd/_common.py index 3580c29c..d8b73f9f 100644 --- a/src/vcspull/cli/import_cmd/_common.py +++ b/src/vcspull/cli/import_cmd/_common.py @@ -33,6 +33,7 @@ get_pin_reason, is_pinned_for_op, merge_duplicate_workspace_roots, + normalize_config_file_path, save_config, workspace_root_label, ) @@ -560,10 +561,10 @@ def _resolve_config_file(config_path_str: str | None) -> pathlib.Path: Returns ------- pathlib.Path - Resolved config file path + Absolute config file path """ if config_path_str: - path = pathlib.Path(config_path_str).expanduser().resolve() + path = normalize_config_file_path(pathlib.Path(config_path_str)) if path.suffix.lower() not in {".yaml", ".yml", ".json"}: msg = f"Unsupported config file type: {path.suffix}" raise ValueError(msg) diff --git a/src/vcspull/config.py b/src/vcspull/config.py index 81d66429..eb2cd083 100644 --- a/src/vcspull/config.py +++ b/src/vcspull/config.py @@ -54,6 +54,48 @@ def expand_dir( return dir_ +def normalize_config_file_path( + path: pathlib.Path, + cwd: pathlib.Path | Callable[[], pathlib.Path] = pathlib.Path.cwd, +) -> pathlib.Path: + """Return absolute config file path without resolving symlinks. + + Symlink entry names are preserved intact so that downstream operations + (e.g. atomic writes) can resolve them as needed, while the logical path + is used for display and identity. + + Parameters + ---------- + path : pathlib.Path + Config file path to normalize. + cwd : pathlib.Path, optional + Current working dir (used to resolve relative paths). Defaults to + :py:meth:`pathlib.Path.cwd`. + + Returns + ------- + pathlib.Path + Absolute config file path with symlink names preserved. + + Examples + -------- + >>> normalize_config_file_path(pathlib.Path("~/cfg.yaml")).name + 'cfg.yaml' + >>> normalize_config_file_path( + ... pathlib.Path("configs/vcspull.yaml"), + ... cwd=pathlib.Path("/tmp/project"), + ... ) # doctest: +ELLIPSIS + PosixPath('.../configs/vcspull.yaml') + """ + path = pathlib.Path(os.path.expandvars(str(path))).expanduser() + if callable(cwd): + cwd = cwd() + + if not path.is_absolute(): + path = pathlib.Path(os.path.normpath(cwd / path)) + return path + + def _validate_worktrees_config( worktrees_raw: t.Any, repo_name: str, @@ -351,8 +393,9 @@ def find_home_config_files( ) -> list[pathlib.Path]: """Return configs of ``.vcspull.{yaml,json}`` in user's home directory. - Paths are resolved through symlinks so callers receive the real - filesystem location, consistent with the ``-f`` flag codepath. + The returned path preserves the logical home entry name so callers + keep the config type implied by ``.yaml`` or ``.json`` even when the + file is a symlink. Parameters ---------- @@ -362,7 +405,7 @@ def find_home_config_files( Returns ------- list of pathlib.Path - Resolved paths to discovered config files + Absolute paths to discovered config files Examples -------- @@ -376,9 +419,9 @@ def find_home_config_files( check_yaml = "yaml" in filetype check_json = "json" in filetype - yaml_config = pathlib.Path("~/.vcspull.yaml").expanduser().resolve() + yaml_config = normalize_config_file_path(pathlib.Path("~/.vcspull.yaml")) has_yaml_config = check_yaml and yaml_config.exists() - json_config = pathlib.Path("~/.vcspull.json").expanduser().resolve() + json_config = normalize_config_file_path(pathlib.Path("~/.vcspull.json")) has_json_config = check_json and json_config.exists() if not has_yaml_config and not has_json_config: diff --git a/tests/cli/test_add.py b/tests/cli/test_add.py index 1cfee8ac..3b3c4166 100644 --- a/tests/cli/test_add.py +++ b/tests/cli/test_add.py @@ -330,6 +330,43 @@ def test_add_repo_creates_new_file( assert "newrepo" in config["~/"] +def test_add_repo_uses_home_symlink_config_without_losing_yaml_suffix( + tmp_path: pathlib.Path, + monkeypatch: MonkeyPatch, +) -> None: + """Default home config symlinks should still load and save as YAML.""" + monkeypatch.setenv("HOME", str(tmp_path)) + monkeypatch.chdir(tmp_path) + + dotfiles_dir = tmp_path / "dotfiles" + dotfiles_dir.mkdir() + + real_config = dotfiles_dir / "vcspull-config" + real_config.write_text("~/code/: {}\n", encoding="utf-8") + + symlink = tmp_path / ".vcspull.yaml" + symlink.symlink_to(real_config) + + repo_path = tmp_path / "code" / "newrepo" + repo_path.mkdir(parents=True) + + add_repo( + name="newrepo", + url="git+https://github.com/user/newrepo.git", + config_file_path_str=None, + path=str(repo_path), + workspace_root_path="~/code/", + dry_run=False, + ) + + assert symlink.is_symlink() + assert symlink.resolve() == real_config.resolve() + + config_text = real_config.read_text(encoding="utf-8") + assert "newrepo" in config_text + assert "git+https://github.com/user/newrepo.git" in config_text + + def test_add_repo_invalid_config_logs_private_path( user_path: pathlib.Path, caplog: pytest.LogCaptureFixture, diff --git a/tests/cli/test_import_repos.py b/tests/cli/test_import_repos.py index aeb432ca..5a1ce980 100644 --- a/tests/cli/test_import_repos.py +++ b/tests/cli/test_import_repos.py @@ -188,6 +188,29 @@ def test_resolve_config_file( assert result.name == expected_suffix +def test_resolve_config_file_preserves_symlink_suffix( + tmp_path: pathlib.Path, + monkeypatch: MonkeyPatch, +) -> None: + """Explicit config paths should keep symlink suffixes for format detection.""" + monkeypatch.setenv("HOME", str(tmp_path)) + + dotfiles_dir = tmp_path / "dotfiles" + dotfiles_dir.mkdir() + + real_config = dotfiles_dir / "vcspull-config" + real_config.write_text("~/repos/: {}\n", encoding="utf-8") + + symlink = tmp_path / ".vcspull.yaml" + symlink.symlink_to(real_config) + + result = _resolve_config_file(str(symlink)) + + assert result == symlink + assert result.suffix == ".yaml" + assert result.resolve() == real_config.resolve() + + class ImportReposFixture(t.NamedTuple): """Fixture for _run_import test cases.""" diff --git a/tests/test_config_file.py b/tests/test_config_file.py index 508636ba..8590c24e 100644 --- a/tests/test_config_file.py +++ b/tests/test_config_file.py @@ -244,11 +244,13 @@ def test_find_home_config_files_both_types_still_raises( config.find_home_config_files() -def test_find_home_config_files_resolves_symlink(tmp_path: pathlib.Path) -> None: - """Symlinked ~/.vcspull.yaml should be resolved to the real target path.""" +def test_find_home_config_files_preserves_symlink_suffix( + tmp_path: pathlib.Path, +) -> None: + """Symlinked home configs should keep the logical suffix for format detection.""" dotfiles_dir = tmp_path / ".dot-config" dotfiles_dir.mkdir() - real_file = dotfiles_dir / ".vcspull.yaml" + real_file = dotfiles_dir / "vcspull-config" real_file.write_text("~/code/: {}\n", encoding="utf-8") symlink = tmp_path / ".vcspull.yaml" @@ -259,9 +261,10 @@ def test_find_home_config_files_resolves_symlink(tmp_path: pathlib.Path) -> None results = config.find_home_config_files() assert len(results) == 1 - # Must be the resolved (real) path, not the symlink - assert results[0] == real_file.resolve() - assert not results[0].is_symlink() + assert results[0] == symlink + assert results[0].suffix == ".yaml" + assert results[0].is_symlink() + assert results[0].resolve() == real_file.resolve() def test_in_dir( From 60ccf9374c1309d806623ce5144614d8c1805a32 Mon Sep 17 00:00:00 2001 From: Tony Narlock Date: Sun, 29 Mar 2026 08:00:59 -0500 Subject: [PATCH 08/12] fix(config[symlink-format]) prefer target suffix for symlinked configs why: The first symlink-path fix preserved visible config names, which fixed extensionless dotfile targets but broke alias paths and mismatched symlinks. An explicit alias like `vcspull-config -> .vcspull.yaml` was rejected for having no suffix, and a home link like `.vcspull.yaml -> vcspull.json` could be rewritten with YAML because later format checks only saw the visible path. what: - Add shared config-format detection that prefers a symlink target's supported suffix and otherwise falls back to the visible path suffix - Use that helper for config reads, duplicate-aware YAML loading, save dispatch, ordered-item saves, and import config-path validation - Add regressions for target-format precedence, extensionless alias paths, and end-to-end JSON writes through a symlinked home config --- src/vcspull/_internal/config_reader.py | 59 +++++++++++++++++++++++--- src/vcspull/cli/add.py | 7 ++- src/vcspull/cli/import_cmd/_common.py | 7 ++- src/vcspull/config.py | 12 ++++-- tests/cli/test_add.py | 39 +++++++++++++++++ tests/cli/test_import_repos.py | 19 ++++----- tests/test_config_file.py | 15 ++++++- 7 files changed, 132 insertions(+), 26 deletions(-) diff --git a/src/vcspull/_internal/config_reader.py b/src/vcspull/_internal/config_reader.py index 22fc602e..0b13a75f 100644 --- a/src/vcspull/_internal/config_reader.py +++ b/src/vcspull/_internal/config_reader.py @@ -8,6 +8,56 @@ import yaml FormatLiteral = t.Literal["json", "yaml"] +_SUPPORTED_CONFIG_SUFFIXES: dict[str, FormatLiteral] = { + ".json": "json", + ".yaml": "yaml", + ".yml": "yaml", +} + + +def config_format_from_path(path: pathlib.Path) -> FormatLiteral | None: + """Return config format inferred from a path or symlink target. + + The visible path remains the config identity, but symlinks may point + at a differently named target. When the target advertises a supported + config suffix, prefer that format; otherwise fall back to the visible + path suffix. + + Parameters + ---------- + path : pathlib.Path + Path to inspect. + + Returns + ------- + FormatLiteral | None + ``"json"`` or ``"yaml"`` when a supported suffix is found, + otherwise ``None``. + + Examples + -------- + >>> config_format_from_path(pathlib.Path("config.yaml")) + 'yaml' + >>> import tempfile + >>> with tempfile.TemporaryDirectory() as tmp: + ... root = pathlib.Path(tmp) + ... target = root / "config.json" + ... _ = target.write_text("{}", encoding="utf-8") + ... link = root / ".vcspull.yaml" + ... link.symlink_to(target) + ... config_format_from_path(link) + 'json' + """ + path_format = _SUPPORTED_CONFIG_SUFFIXES.get(path.suffix.lower()) + + target_format: FormatLiteral | None = None + if path.is_symlink(): + resolved = path.resolve(strict=False) + target_format = _SUPPORTED_CONFIG_SUFFIXES.get(resolved.suffix.lower()) + + return target_format or path_format + + RawConfigData: t.TypeAlias = dict[t.Any, t.Any] @@ -117,11 +167,8 @@ def _from_file(cls, path: pathlib.Path) -> dict[str, t.Any]: # Revisit once the new ``vcspull add`` flow lands so both commands share # the same duplication safeguards. - if path.suffix in {".yaml", ".yml"}: - fmt: FormatLiteral = "yaml" - elif path.suffix == ".json": - fmt = "json" - else: + fmt = config_format_from_path(path) + if fmt is None: msg = f"{path.suffix} not supported in {path}" raise NotImplementedError(msg) @@ -335,7 +382,7 @@ def _load_from_path( cls, path: pathlib.Path, ) -> tuple[dict[str, t.Any], dict[str, list[t.Any]], list[tuple[str, t.Any]]]: - if path.suffix.lower() in {".yaml", ".yml"}: + if config_format_from_path(path) == "yaml": content = path.read_text(encoding="utf-8") return cls._load_yaml_with_duplicates(content) diff --git a/src/vcspull/cli/add.py b/src/vcspull/cli/add.py index 979e913b..a7b24aab 100644 --- a/src/vcspull/cli/add.py +++ b/src/vcspull/cli/add.py @@ -13,7 +13,10 @@ from colorama import Fore, Style -from vcspull._internal.config_reader import DuplicateAwareConfigReader +from vcspull._internal.config_reader import ( + DuplicateAwareConfigReader, + config_format_from_path, +) from vcspull._internal.private_path import PrivatePath from vcspull.config import ( canonicalize_workspace_path, @@ -341,7 +344,7 @@ def _save_ordered_items( >>> "~/code/" in data True """ - if config_file_path.suffix.lower() == ".json": + if config_format_from_path(config_file_path) == "json": save_config_json( config_file_path, _collapse_ordered_items_to_dict(ordered_items), diff --git a/src/vcspull/cli/import_cmd/_common.py b/src/vcspull/cli/import_cmd/_common.py index d8b73f9f..b1d050ab 100644 --- a/src/vcspull/cli/import_cmd/_common.py +++ b/src/vcspull/cli/import_cmd/_common.py @@ -14,7 +14,10 @@ import sys import typing as t -from vcspull._internal.config_reader import DuplicateAwareConfigReader +from vcspull._internal.config_reader import ( + DuplicateAwareConfigReader, + config_format_from_path, +) from vcspull._internal.private_path import PrivatePath from vcspull._internal.remotes import ( AuthenticationError, @@ -565,7 +568,7 @@ def _resolve_config_file(config_path_str: str | None) -> pathlib.Path: """ if config_path_str: path = normalize_config_file_path(pathlib.Path(config_path_str)) - if path.suffix.lower() not in {".yaml", ".yml", ".json"}: + if config_format_from_path(path) is None: msg = f"Unsupported config file type: {path.suffix}" raise ValueError(msg) return path diff --git a/src/vcspull/config.py b/src/vcspull/config.py index eb2cd083..8212c29c 100644 --- a/src/vcspull/config.py +++ b/src/vcspull/config.py @@ -18,7 +18,11 @@ from vcspull.validator import is_valid_config from . import exc -from ._internal.config_reader import ConfigReader, DuplicateAwareConfigReader +from ._internal.config_reader import ( + ConfigReader, + DuplicateAwareConfigReader, + config_format_from_path, +) from .types import ConfigDict, RawConfigDict, WorktreeConfigDict from .util import get_config_dir, update_dict @@ -839,17 +843,17 @@ def save_config(config_file_path: pathlib.Path, data: dict[t.Any, t.Any]) -> Non >>> with tempfile.TemporaryDirectory() as tmp: ... p = pathlib.Path(tmp) / "test.json" ... save_config(p, {"~/code/": {"repo": {"repo": "git+https://x"}}}) - ... loaded = json.loads(p.read_text()) + ... loaded = json.loads(p.read_text(encoding="utf-8")) ... loaded["~/code/"]["repo"]["repo"] 'git+https://x' >>> with tempfile.TemporaryDirectory() as tmp: ... p = pathlib.Path(tmp) / "test.yaml" ... save_config(p, {"~/code/": {"repo": {"repo": "git+https://x"}}}) - ... "repo" in p.read_text() + ... "repo" in p.read_text(encoding="utf-8") True """ - if config_file_path.suffix.lower() == ".json": + if config_format_from_path(config_file_path) == "json": save_config_json(config_file_path, data) else: save_config_yaml(config_file_path, data) diff --git a/tests/cli/test_add.py b/tests/cli/test_add.py index 3b3c4166..78d261c6 100644 --- a/tests/cli/test_add.py +++ b/tests/cli/test_add.py @@ -3,6 +3,7 @@ from __future__ import annotations import argparse +import json import logging import os import pathlib @@ -367,6 +368,44 @@ def test_add_repo_uses_home_symlink_config_without_losing_yaml_suffix( assert "git+https://github.com/user/newrepo.git" in config_text +def test_add_repo_uses_target_json_format_for_home_symlink( + tmp_path: pathlib.Path, + monkeypatch: MonkeyPatch, +) -> None: + """Default home symlinks should save using the supported target format.""" + monkeypatch.setenv("HOME", str(tmp_path)) + monkeypatch.chdir(tmp_path) + + dotfiles_dir = tmp_path / "dotfiles" + dotfiles_dir.mkdir() + + real_config = dotfiles_dir / "vcspull.json" + real_config.write_text("{}\n", encoding="utf-8") + + symlink = tmp_path / ".vcspull.yaml" + symlink.symlink_to(real_config) + + repo_path = tmp_path / "code" / "jsonrepo" + repo_path.mkdir(parents=True) + + add_repo( + name="jsonrepo", + url="git+https://github.com/user/jsonrepo.git", + config_file_path_str=None, + path=str(repo_path), + workspace_root_path="~/code/", + dry_run=False, + ) + + assert symlink.is_symlink() + assert symlink.resolve() == real_config.resolve() + + data = json.loads(real_config.read_text(encoding="utf-8")) + assert data["~/code/"]["jsonrepo"]["repo"] == ( + "git+https://github.com/user/jsonrepo.git" + ) + + def test_add_repo_invalid_config_logs_private_path( user_path: pathlib.Path, caplog: pytest.LogCaptureFixture, diff --git a/tests/cli/test_import_repos.py b/tests/cli/test_import_repos.py index 5a1ce980..3aa6e6d4 100644 --- a/tests/cli/test_import_repos.py +++ b/tests/cli/test_import_repos.py @@ -188,26 +188,23 @@ def test_resolve_config_file( assert result.name == expected_suffix -def test_resolve_config_file_preserves_symlink_suffix( +def test_resolve_config_file_accepts_extensionless_symlink_alias( tmp_path: pathlib.Path, monkeypatch: MonkeyPatch, ) -> None: - """Explicit config paths should keep symlink suffixes for format detection.""" + """Explicit alias paths should inherit a supported target config format.""" monkeypatch.setenv("HOME", str(tmp_path)) - dotfiles_dir = tmp_path / "dotfiles" - dotfiles_dir.mkdir() - - real_config = dotfiles_dir / "vcspull-config" + real_config = tmp_path / ".vcspull.yaml" real_config.write_text("~/repos/: {}\n", encoding="utf-8") - symlink = tmp_path / ".vcspull.yaml" - symlink.symlink_to(real_config) + alias_path = tmp_path / "vcspull-config" + alias_path.symlink_to(real_config) - result = _resolve_config_file(str(symlink)) + result = _resolve_config_file(str(alias_path)) - assert result == symlink - assert result.suffix == ".yaml" + assert result == alias_path + assert result.suffix == "" assert result.resolve() == real_config.resolve() diff --git a/tests/test_config_file.py b/tests/test_config_file.py index 8590c24e..b51fbc80 100644 --- a/tests/test_config_file.py +++ b/tests/test_config_file.py @@ -9,7 +9,7 @@ import pytest from vcspull import config, exc -from vcspull._internal.config_reader import ConfigReader +from vcspull._internal.config_reader import ConfigReader, config_format_from_path from vcspull.config import expand_dir, extract_repos from vcspull.validator import is_valid_config @@ -267,6 +267,19 @@ def test_find_home_config_files_preserves_symlink_suffix( assert results[0].resolve() == real_file.resolve() +def test_config_format_from_path_prefers_supported_symlink_target( + tmp_path: pathlib.Path, +) -> None: + """Symlink targets with supported suffixes should determine the config format.""" + real_json = tmp_path / "vcspull.json" + real_json.write_text("{}\n", encoding="utf-8") + + mismatch = tmp_path / ".vcspull.yaml" + mismatch.symlink_to(real_json) + + assert config_format_from_path(mismatch) == "json" + + def test_in_dir( config_path: pathlib.Path, yaml_config: pathlib.Path, From 306dde1e247bc8131d544d15e06964b8ef0d405d Mon Sep 17 00:00:00 2001 From: Tony Narlock Date: Sun, 29 Mar 2026 09:27:27 -0500 Subject: [PATCH 09/12] docs(config[save_config_yaml,save_config_json]) add doctests why: Both functions lacked Examples sections despite the project requiring working doctests on all public functions. what: - Add Examples section to save_config_yaml with round-trip read check - Add Examples section to save_config_json with json.loads round-trip check --- src/vcspull/config.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/vcspull/config.py b/src/vcspull/config.py index 8212c29c..92ef7d99 100644 --- a/src/vcspull/config.py +++ b/src/vcspull/config.py @@ -800,6 +800,15 @@ def save_config_yaml(config_file_path: pathlib.Path, data: dict[t.Any, t.Any]) - Path to the configuration file to write data : dict Configuration data to save + + Examples + -------- + >>> import pathlib, tempfile + >>> with tempfile.TemporaryDirectory() as tmp: + ... p = pathlib.Path(tmp) / "cfg.yaml" + ... save_config_yaml(p, {"~/code/": {"myrepo": "git+https://example.com/repo.git"}}) + ... "myrepo" in p.read_text(encoding="utf-8") + True """ yaml_content = ConfigReader._dump( fmt="yaml", @@ -818,6 +827,16 @@ def save_config_json(config_file_path: pathlib.Path, data: dict[t.Any, t.Any]) - Path to the configuration file to write data : dict Configuration data to save + + Examples + -------- + >>> import json, pathlib, tempfile + >>> with tempfile.TemporaryDirectory() as tmp: + ... p = pathlib.Path(tmp) / "cfg.json" + ... save_config_json(p, {"~/code/": {"myrepo": "git+https://example.com/repo.git"}}) + ... loaded = json.loads(p.read_text(encoding="utf-8")) + ... "~/code/" in loaded + True """ json_content = ConfigReader._dump( fmt="json", From bf9ee94ea0a430bf0f98dd700a38aca03c9635e7 Mon Sep 17 00:00:00 2001 From: Tony Narlock Date: Sun, 29 Mar 2026 09:28:18 -0500 Subject: [PATCH 10/12] docs(config[save_config_yaml_with_items]) add NumPy docstring with doctest why: Function had only a one-line summary with no Parameters, Returns, or Examples section despite the project requiring full NumPy docstrings and working doctests on all public functions. what: - Add description explaining why this differs from save_config_yaml (preserves duplicate top-level keys via ordered pairs) - Add Parameters and Returns sections per NumPy docstring standard - Add Examples section with duplicate-section round-trip doctest --- src/vcspull/config.py | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/src/vcspull/config.py b/src/vcspull/config.py index 92ef7d99..2d3ffc9a 100644 --- a/src/vcspull/config.py +++ b/src/vcspull/config.py @@ -882,7 +882,34 @@ def save_config_yaml_with_items( config_file_path: pathlib.Path, items: list[tuple[str, t.Any]], ) -> None: - """Persist configuration data while preserving duplicate top-level sections.""" + """Persist configuration data while preserving duplicate top-level sections. + + Unlike :func:`save_config_yaml`, which loses duplicate keys when given a + plain ``dict``, this function accepts ordered ``(label, data)`` pairs so + that two workspace-root entries with the same key are each serialised as a + separate YAML block. + + Parameters + ---------- + config_file_path : pathlib.Path + Destination config file (may be a symlink; the real target is updated). + items : list of tuple[str, Any] + Ordered ``(section_label, section_data)`` pairs. Each pair becomes one + YAML document block in the output. + + Examples + -------- + >>> import pathlib, tempfile + >>> with tempfile.TemporaryDirectory() as tmp: + ... p = pathlib.Path(tmp) / "cfg.yaml" + ... save_config_yaml_with_items(p, [ + ... ("~/code/", {"flask": "git+https://github.com/pallets/flask.git"}), + ... ("~/code/", {"django": "git+https://github.com/django/django.git"}), + ... ]) + ... content = p.read_text(encoding="utf-8") + ... "flask" in content and "django" in content + True + """ documents: list[str] = [] for label, section in items: From 26af9e8ff2ad441ab4eb8a1c0a2021a7ac1a79d3 Mon Sep 17 00:00:00 2001 From: Tony Narlock Date: Sun, 29 Mar 2026 09:29:05 -0500 Subject: [PATCH 11/12] docs(config_reader[ConfigReader._from_file]) update TODO: vcspull add landed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit why: The trigger condition in the TODO ("once the new vcspull add flow lands") was already satisfied — vcspull add now uses DuplicateAwareConfigReader for reading. The stale "Revisit once" sentence misled readers into thinking the integration was still pending. what: - Replace the stale "Revisit once the new vcspull add flow lands" sentence with a note reflecting the current state - Note that this basic loader remains for simpler read contexts - Identify Option 1 (shared utility) as the cleanest long-term path --- src/vcspull/_internal/config_reader.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/vcspull/_internal/config_reader.py b/src/vcspull/_internal/config_reader.py index 0b13a75f..9ff6f412 100644 --- a/src/vcspull/_internal/config_reader.py +++ b/src/vcspull/_internal/config_reader.py @@ -164,8 +164,9 @@ def _from_file(cls, path: pathlib.Path) -> dict[str, t.Any]: # the formatter helpers directly; # 3) Keep this basic loader but add an opt-in path for duplicate-aware # parsing so commands like ``vcspull add`` can avoid data loss. - # Revisit once the new ``vcspull add`` flow lands so both commands share - # the same duplication safeguards. + # ``vcspull add`` now uses ``DuplicateAwareConfigReader`` for reading + # (see ``cli/add.py``). This basic loader remains for simpler read + # contexts. Option 1 (shared utility) is the cleanest long-term path. fmt = config_format_from_path(path) if fmt is None: From fbb8fd109d2965bfd1a70cc3a48ee000ebcb56a6 Mon Sep 17 00:00:00 2001 From: Tony Narlock Date: Sun, 29 Mar 2026 10:12:20 -0500 Subject: [PATCH 12/12] docs(CHANGES) preserve symlinks when writing config files why: Document the symlink-preservation fix and format-detection improvement shipped in this branch. what: - Add Bug fixes entry under v1.59.x unreleased section --- CHANGES | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/CHANGES b/CHANGES index df38b5bf..562a2814 100644 --- a/CHANGES +++ b/CHANGES @@ -37,6 +37,20 @@ $ uvx --from 'vcspull' --prerelease allow vcspull _Notes on upcoming releases will be added here_ +### Bug fixes + +#### `config`: Preserve symlinks when writing config files (#538) + +Config files that are symbolic links (e.g. `~/.vcspull.yaml` pointing to a +dotfiles directory) were being silently replaced by regular files on every +write, destroying the symlink. The write now goes through the link — a temp +file is created next to the real target, renamed into place, and the symlink +directory entry is preserved. + +Format detection now also inspects the symlink target's extension, so a +`.yaml` symlink pointing to a `.json` file serialises correctly as JSON rather +than overwriting the target with YAML. + ## vcspull v1.58.0 (2026-03-01) ### New features