fix: materialize symlinks as links instead of regular files

pr_split/cli.py

@@ -2,6 +2,7 @@
 
 import json as json_mod
 import shutil
+import stat
 import tempfile
 import time
 import urllib.request
@@ -361,16 +362,30 @@ def _create_single_branch_and_commit(
         modes = target_file_modes(parsed_diff, group)
         for file_path, content in materialized.items():
             p = Path(worktree_path) / file_path
-            if content is not None:
-                p.parent.mkdir(parents=True, exist_ok=True)
-                # newline="" keeps CRLF from the reconstructed content intact.
-                p.write_text(content, encoding="utf-8", errors="surrogateescape", newline="")
-                if file_path in modes:
-                    # Git only tracks the executable bit; apply the diff's
-                    # target mode so chmod changes reach the sub-PR.
-                    p.chmod(modes[file_path] & 0o777)
-            elif p.exists():
+            mode = modes.get(file_path)
+            if content is None:
+                # A dangling symlink is not "exists()" but must still go.
+                if p.is_symlink() or p.exists():
+                    p.unlink()
+                continue
+            p.parent.mkdir(parents=True, exist_ok=True)
+            if p.is_symlink():
+                # Never write through an existing link (it would modify the
+                # target file); replace the link itself.
                 p.unlink()
+            if mode is not None and stat.S_ISLNK(mode):
+                # Git stores a symlink's target as the blob content. A regular
+                # file being converted to a link is still on disk here.
+                if p.is_symlink() or p.exists():
+                    p.unlink()
+                p.symlink_to(content.rstrip("\n"))
+                continue
+            # newline="" keeps CRLF from the reconstructed content intact.
+            p.write_text(content, encoding="utf-8", errors="surrogateescape", newline="")
+            if mode is not None:
+                # Git only tracks the executable bit; apply the diff's target
+                # mode so chmod changes reach the sub-PR.
+                p.chmod(mode & 0o777)
 
         logger.info(logs.COMMITTING_GROUP.format(group=group.id, title=group.title))
         commit_sha = commit_files_in_dir(

pr_split/diff_ops/reconstructor.py

@@ -1,6 +1,7 @@
 from __future__ import annotations
 
 import re
+import stat
 import subprocess
 
 from loguru import logger
@@ -167,15 +168,22 @@ def materialize_group_files(
     return result
 
 
-_TARGET_MODE_RE = re.compile(r"^(?:new file mode|new mode) (\d{6})$", re.MULTILINE)
+# "new file mode"/"new mode" appear when the mode changes; an unchanged mode
+# is only visible on the "index <old>..<new> <mode>" line.
+_TARGET_MODE_RE = re.compile(
+    r"^(?:new file mode|new mode) (\d{6})$|^index [0-9a-f]+\.\.[0-9a-f]+ (\d{6})$",
+    re.MULTILINE,
+)
 
 
 def target_file_modes(parsed_diff: ParsedDiff, group: Group) -> dict[str, int]:
     """Map each file the group materializes to the mode the diff gives it.
 
-    unidiff parses ``old mode``/``new mode``/``new file mode`` headers into
-    ``patch_info`` only; nothing else applies them, so without this a
-    ``chmod +x`` that comes with a content change silently loses the bit.
+    unidiff parses ``old mode``/``new mode``/``new file mode`` and ``index``
+    headers into ``patch_info`` only; nothing else applies them, so without
+    this a ``chmod +x`` that comes with a content change silently loses the
+    bit and a symlink is written as a regular file. Regular files (100644,
+    100755) and symlinks (120000) are reported.
     """
     wanted = {assignment.file_path for assignment in group.assignments}
     modes: dict[str, int] = {}
@@ -188,10 +196,7 @@ def target_file_modes(parsed_diff: ParsedDiff, group: Group) -> dict[str, int]:
         match = _TARGET_MODE_RE.search(header)
         if not match:
             continue
-        mode = int(match.group(1), 8)
-        # Only regular files (100644 / 100755): a symlink's 120000 would mask
-        # to 000 and make the written file unreadable. Symlinks are written as
-        # plain files by the reconstructor, unchanged from before.
-        if mode & 0o170000 == 0o100000:
+        mode = int(match.group(1) or match.group(2), 8)
+        if stat.S_ISREG(mode) or stat.S_ISLNK(mode):
             modes[patch_file.path] = mode
     return modes

tests/test_cli_helpers.py

@@ -923,3 +923,65 @@ def capture(cwd: str, file_paths: list[str], message: str, **kwargs: object) ->
         _create_branches_and_commits([_group("pr-1", "t")], MagicMock(), "main", "sha", "ns")
         assert modes["run.sh"] & stat.S_IXUSR
         assert not modes["a.py"] & stat.S_IXUSR
+
+
+class TestWorktreeMaterializesSymlinks:
+    def _run(
+        self, materialized: dict[str, str | None], modes: dict[str, int]
+    ) -> dict[str, object]:
+        import os
+        from pathlib import Path
+
+        seen: dict[str, object] = {}
+
+        def capture(cwd: str, file_paths: list[str], message: str, **kwargs: object) -> str:
+            for file_path in file_paths:
+                p = Path(cwd) / file_path
+                seen[file_path] = (
+                    os.readlink(p) if p.is_symlink() else ("missing" if not p.exists() else "file")
+                )
+            return "sha1"
+
+        with (
+            patch("pr_split.cli.add_worktree"),
+            patch("pr_split.cli.remove_worktree"),
+            patch("pr_split.cli.materialize_group_files", return_value=materialized),
+            patch("pr_split.cli.target_file_modes", return_value=modes),
+            patch("pr_split.cli.commit_files_in_dir", side_effect=capture),
+        ):
+            _create_branches_and_commits([_group("pr-1", "t")], MagicMock(), "main", "sha", "ns")
+        return seen
+
+    def test_new_symlink_is_created_as_a_link(self) -> None:
+        seen = self._run({"link": "other.py\n", "other.py": "x\n"}, {"link": 0o120000})
+        assert seen["link"] == "other.py"
+        assert seen["other.py"] == "file"
+
+    def test_symlink_without_mode_info_stays_a_plain_file(self) -> None:
+        seen = self._run({"link": "other.py\n"}, {})
+        assert seen["link"] == "file"
+
+    def test_regular_file_converted_to_symlink(self) -> None:
+        import os
+        from pathlib import Path
+
+        seen: dict[str, object] = {}
+
+        def pre_populate(path: str, branch: str, start: str) -> None:
+            Path(path).mkdir(parents=True, exist_ok=True)
+            (Path(path) / "toref").write_text("old regular content\n")
+
+        def capture(cwd: str, file_paths: list[str], message: str, **kwargs: object) -> str:
+            p = Path(cwd) / "toref"
+            seen["toref"] = os.readlink(p) if p.is_symlink() else "file"
+            return "sha1"
+
+        with (
+            patch("pr_split.cli.add_worktree", side_effect=pre_populate),
+            patch("pr_split.cli.remove_worktree"),
+            patch("pr_split.cli.materialize_group_files", return_value={"toref": "a.py\n"}),
+            patch("pr_split.cli.target_file_modes", return_value={"toref": 0o120000}),
+            patch("pr_split.cli.commit_files_in_dir", side_effect=capture),
+        ):
+            _create_branches_and_commits([_group("pr-1", "t")], MagicMock(), "main", "sha", "ns")
+        assert seen["toref"] == "a.py"

tests/test_reconstructor.py

@@ -705,12 +705,27 @@ def test_removed_file_is_ignored(self) -> None:
         )
         assert target_file_modes(parsed, self._group("d.sh")) == {}
 
-    def test_symlink_mode_is_not_reported(self) -> None:
+    def test_new_symlink_mode_is_reported(self) -> None:
         parsed = parse_diff(
             "diff --git a/link b/link\nnew file mode 120000\n--- /dev/null\n+++ b/link\n"
             "@@ -0,0 +1 @@\n+other.py\n\\ No newline at end of file\n"
         )
-        assert target_file_modes(parsed, self._group("link")) == {}
+        assert target_file_modes(parsed, self._group("link")) == {"link": 0o120000}
+
+    def test_retargeted_symlink_mode_comes_from_index_line(self) -> None:
+        parsed = parse_diff(
+            "diff --git a/link b/link\nindex d90d485..5f35558 120000\n--- a/link\n+++ b/link\n"
+            "@@ -1 +1 @@\n-other.py\n\\ No newline at end of file\n"
+            "+missing.py\n\\ No newline at end of file\n"
+        )
+        assert target_file_modes(parsed, self._group("link")) == {"link": 0o120000}
+
+    def test_unchanged_regular_mode_from_index_line(self) -> None:
+        parsed = parse_diff(
+            "diff --git a/a.py b/a.py\nindex 1111111..2222222 100644\n--- a/a.py\n+++ b/a.py\n"
+            "@@ -1 +1 @@\n-x\n+y\n"
+        )
+        assert target_file_modes(parsed, self._group("a.py")) == {"a.py": 0o100644}
 
     def test_mode_change_back_to_non_executable_is_reported(self) -> None:
         parsed = parse_diff(