fix(issue-fix): preserve exact changed Git paths and history

loopx/capabilities/issue_fix/reviewer_recommendation.py

@@ -79,6 +79,26 @@ def _run_git(repo_path: Path, args: Sequence[str]) -> str:
     return result.stdout
 
 
+def _run_git_bytes(repo_path: Path, args: Sequence[str]) -> bytes:
+    """Run a path-listing git command without text decoding.
+
+    ``text=True`` applies universal-newline translation before any framing
+    happens, so a pathname carrying CR or CRLF can arrive as an extra LF and
+    split one record into two. Callers that parse machine output frame the raw
+    bytes on their own separator.
+    """
+    result = subprocess.run(
+        ["git", *args],
+        cwd=repo_path,
+        stdout=subprocess.PIPE,
+        stderr=subprocess.PIPE,
+        timeout=20,
+    )
+    if result.returncode != 0:
+        raise RuntimeError(f"git {' '.join(args)} failed with exit code {result.returncode}")
+    return result.stdout
+
+
 def _normalise_changed_file(value: Any) -> str:
     text = str(value or "").strip().replace("\\", "/")
     while text.startswith("./"):
@@ -89,6 +109,23 @@ def _normalise_changed_file(value: Any) -> str:
     return path.as_posix()
 
 
+def _validate_derived_changed_file(value: Any) -> str:
+    """Validate a Git path after NUL framing without rewriting its bytes.
+
+    Git already emits a repository-relative name. Treating a POSIX literal
+    backslash as a Windows separator made a valid root file such as
+    ``\\notes.txt`` look absolute during packet validation. Keep the explicit
+    ``--changed-file`` normalization above as the compatibility boundary for
+    caller-supplied path syntax, and validate derived names as emitted.
+    """
+    if not isinstance(value, str) or not value:
+        raise ValueError("changed files must be non-empty repo-relative paths")
+    path = PurePosixPath(value)
+    if path.as_posix() == "." or path.is_absolute() or ".." in path.parts:
+        raise ValueError("changed files must be non-empty repo-relative paths")
+    return value
+
+
 def _normalise_changed_files(values: Sequence[Any]) -> list[str]:
     files: list[str] = []
     for raw in values:
@@ -624,14 +661,34 @@ def _apply_reviewer_sources_for_path(
 
 
 def _derive_changed_files(repo_path: Path, base_ref: str) -> list[str]:
-    output = _run_git(repo_path, ["diff", "--name-only", f"{base_ref}...HEAD"])
-    # `git diff --name-only` frames one pathname per LF. Under the default
-    # `core.quotePath=true` a non-ASCII pathname arrives octal-escaped, but a
-    # repository may set `core.quotePath=false`, and a pathname carrying
-    # U+0085/U+2028/U+2029 is then emitted raw. `str.splitlines()` treats those
-    # as record boundaries and would tear one path into two, so frame on LF and
-    # drop the empty trailing record instead.
-    return _normalise_changed_files([line for line in output.split("\n") if line])
+    output = _run_git_bytes(
+        repo_path,
+        ["diff", "--name-only", "-z", "--no-ext-diff", f"{base_ref}...HEAD"],
+    )
+    # `-z` frames one raw pathname per NUL byte, so the name reaches this
+    # function exactly as Git recorded it. With the default
+    # `core.quotePath=true`, Git C-quotes non-ASCII and control-character
+    # names in a line-oriented listing; normalising those quotes as literal
+    # path text produced names such as "/344/270/255/..." that match neither
+    # CODEOWNERS rules nor commit history. NUL framing avoids the quoting
+    # entirely, and decoding each record strictly keeps CR, CRLF, leading and
+    # trailing spaces and POSIX literal backslashes intact.
+    files: list[str] = []
+    for record in output.split(b"\0"):
+        if not record:
+            continue
+        try:
+            name = record.decode("utf-8")
+        except UnicodeDecodeError as error:
+            raise ValueError(
+                "changed files derived from git must be valid UTF-8"
+            ) from error
+        _validate_derived_changed_file(name)
+        if name not in files:
+            files.append(name)
+    if len(files) > 100:
+        raise ValueError("reviewer recommendation accepts at most 100 changed files")
+    return files
 
 
 def _collect_history(
@@ -641,9 +698,17 @@ def _collect_history(
     revision: str,
     history_limit: int,
 ) -> list[tuple[str, str]]:
+    # NUL framing gives an exact pathname, but ``git log -- <path>`` still
+    # interprets the argument as a pathspec. A changed file whose name
+    # contains a wildcard or starts with pathspec magic such as
+    # ``:(exclude)`` would then collect commits for other files and credit
+    # their authors as historical contributors of this path. Literal
+    # pathspecs apply to the exact changed file and to the module fallback
+    # below, which share this helper.
     output = _run_git(
         repo_path,
         [
+            "--literal-pathspecs",
             "log",
             f"--max-count={history_limit}",
             "--format=%aN%x1f%aE",
@@ -967,7 +1032,7 @@ def validate_issue_fix_reviewer_recommendation_packet(
         changed_files = []
     for path in changed_files:
         try:
-            _normalise_changed_file(path)
+            _validate_derived_changed_file(path)
         except ValueError:
             errors.append("changed_files must contain only repo-relative paths")
             break

tests/capabilities/test_issue_fix_reviewer_history_framing.py

@@ -1,5 +1,8 @@
 from __future__ import annotations
 
+import contextlib
+import io
+import json
 import os
 import subprocess
 from pathlib import Path
@@ -9,7 +12,9 @@
 from loopx.capabilities.issue_fix.reviewer_recommendation import (
     _collect_history,
     _derive_changed_files,
+    build_issue_fix_reviewer_recommendation_packet,
 )
+from loopx.cli import main as cli_main
 
 AUTHOR_NAME_WITH_SEPARATOR = "Zo{separator}e"
 
@@ -111,6 +116,185 @@ def test_derive_changed_files_frames_records_on_lf_when_quote_path_is_disabled(
     assert _derive_changed_files(repo, "HEAD~1") == [path]
 
 
+@pytest.mark.parametrize(
+    "path",
+    [
+        "中文名字.txt",
+        "cr\rname.txt",
+        "crlf\r\nname.txt",
+        " leading-space.txt",
+        "trailing-space.txt ",
+        "back\\slash.txt",
+    ],
+)
+def test_derive_changed_files_keeps_paths_git_would_c_quote(
+    tmp_path: Path,
+    path: str,
+) -> None:
+    """One `git diff --name-only -z` record must keep the exact pathname.
+
+    With the default `core.quotePath=true`, a line-oriented listing C-quotes
+    non-ASCII and control-character names. Replacing the quote escapes with
+    slashes turned `中文名字.txt` into `/344/270/255/...`, so CODEOWNERS rules
+    and commit history were queried for a path that does not exist. NUL framing
+    and strict record decoding must preserve the exact name, including CR,
+    CRLF, surrounding spaces and a POSIX literal backslash.
+    """
+    repo = tmp_path / "repo"
+    repo.mkdir()
+    _git(repo, "init", "-b", "main")
+    _git(repo, "config", "user.name", "LoopX Test")
+    _git(repo, "config", "user.email", "loopx@example.invalid")
+
+    (repo / "base.txt").write_text("base\n", encoding="utf-8")
+    _git(repo, "add", "base.txt")
+    _git(repo, "commit", "-m", "base")
+
+    (repo / path).write_text("x\n", encoding="utf-8")
+    _git(repo, "add", "--", path)
+    _git(repo, "commit", "-m", "add a path Git would quote")
+
+    assert _derive_changed_files(repo, "HEAD~1") == [path]
+
+
+@pytest.mark.parametrize(
+    ("path", "other"),
+    [
+        ("note\n*.txt", "note\nX.txt"),
+        (":(exclude)skip\n.txt", "other.txt"),
+        ("\\notes.txt", "other.txt"),
+        (" ", "other.txt"),
+        ("back\\..\\slash.txt", "other.txt"),
+    ],
+)
+def test_reviewer_plan_history_binds_the_literal_changed_path(
+    tmp_path: Path,
+    path: str,
+    other: str,
+) -> None:
+    """History must query the changed path literally, not as a pathspec.
+
+    A pathname containing a wildcard, or starting with pathspec magic such as
+    ``:(exclude)``, was trusted to Git as a pathspec by ``git log -- <path>``.
+    The query then collected commits for different files and ranked their
+    author first as an exact changed-path contributor.
+    """
+    work_repo = tmp_path / "repo"
+    work_repo.mkdir()
+    _git(work_repo, "init", "-b", "main")
+    _git(work_repo, "config", "user.name", "LoopX Test")
+    _git(work_repo, "config", "user.email", "loopx@example.invalid")
+
+    (work_repo / path).write_text("base\n", encoding="utf-8")
+    (work_repo / other).write_text("other\n", encoding="utf-8")
+    _git(work_repo, "--literal-pathspecs", "add", "--", path, other)
+    _git(work_repo, "commit", "-m", "base")
+
+    (work_repo / other).write_text("other change\n", encoding="utf-8")
+    env = dict(os.environ)
+    env.update(
+        GIT_AUTHOR_NAME="Other Author",
+        GIT_AUTHOR_EMAIL="other@example.invalid",
+        GIT_COMMITTER_NAME="LoopX Test",
+        GIT_COMMITTER_EMAIL="loopx@example.invalid",
+    )
+    _git(work_repo, "commit", "-am", "other author edits its own file", env=env)
+
+    (work_repo / path).write_text("target change\n", encoding="utf-8")
+    _git(work_repo, "--literal-pathspecs", "add", "--", path)
+    _git(work_repo, "commit", "-m", "adds the changed path")
+
+    packet = build_issue_fix_reviewer_recommendation_packet(
+        repo_path=work_repo,
+        repo="approved_local_repo",
+        base_ref="HEAD~1",
+        history_limit=10,
+        execute=True,
+        generated_at="2026-08-01T00:00:00Z",
+    )
+
+    assert packet["changed_files"] == [path]
+    assert [row["display_name"] for row in packet["candidates"]] == ["LoopX Test"]
+    assert packet["candidates"][0]["history_commit_count"] == 1
+    assert "changed_path_commit_history" in packet["candidates"][0]["reason_codes"]
+
+
+def test_reviewer_plan_cli_accepts_derived_literal_backslash_root_path(
+    tmp_path: Path,
+) -> None:
+    r"""The public CLI must accept a derived root file whose name starts ``\``."""
+    work_repo = tmp_path / "repo"
+    work_repo.mkdir()
+    _git(work_repo, "init", "-b", "main")
+    _git(work_repo, "config", "user.name", "LoopX Test")
+    _git(work_repo, "config", "user.email", "loopx@example.invalid")
+
+    path = "\\notes.txt"
+    (work_repo / path).write_text("base\n", encoding="utf-8")
+    _git(work_repo, "--literal-pathspecs", "add", "--", path)
+    _git(work_repo, "commit", "-m", "base")
+    (work_repo / path).write_text("target change\n", encoding="utf-8")
+    _git(work_repo, "--literal-pathspecs", "add", "--", path)
+    _git(work_repo, "commit", "-m", "target change")
+
+    output = io.StringIO()
+    with contextlib.redirect_stdout(output):
+        result = cli_main(
+            [
+                "--format",
+                "json",
+                "issue-fix",
+                "reviewer-plan",
+                "--repo-path",
+                str(work_repo),
+                "--repo",
+                "approved_local_repo",
+                "--base-ref",
+                "HEAD~1",
+                "--execute",
+                "--generated-at",
+                "2026-08-01T00:00:00Z",
+            ]
+        )
+    assert result == 0, output.getvalue()
+    packet = json.loads(output.getvalue())
+    assert packet["ok"] is True
+    assert packet["changed_files"] == [path]
+    assert [row["display_name"] for row in packet["candidates"]] == ["LoopX Test"]
+    assert packet["candidates"][0]["history_commit_count"] == 1
+
+
+def test_reviewer_plan_preview_keeps_explicit_path_normalisation(
+    tmp_path: Path,
+) -> None:
+    """Explicit --changed-file input keeps its established separator normalisation."""
+    missing_repo = tmp_path / "not-read"
+    output = io.StringIO()
+    with contextlib.redirect_stdout(output):
+        result = cli_main(
+            [
+                "--format",
+                "json",
+                "issue-fix",
+                "reviewer-plan",
+                "--repo-path",
+                str(missing_repo),
+                "--repo",
+                "approved_local_repo",
+                "--changed-file",
+                "docs\\one.py",
+                "--generated-at",
+                "2026-08-01T00:00:00Z",
+            ]
+        )
+    assert result == 0, output.getvalue()
+    packet = json.loads(output.getvalue())
+    assert packet["changed_files"] == ["docs/one.py"]
+    assert packet["recommendation_status"] == "preview_only"
+    assert packet["private_repo_state_read"] is False
+    assert not missing_repo.exists()
+
+
 def test_derive_changed_files_keeps_ordinary_ascii_paths(tmp_path: Path) -> None:
     repo = tmp_path / "repo"
     repo.mkdir()