From 3bc0ec20efd1101723f94bf8cda846773e2f99da Mon Sep 17 00:00:00 2001 From: ChengLong GU <88359731+gcl-coder@users.noreply.github.com> Date: Thu, 12 Nov 2026 17:31:26 +0800 Subject: [PATCH 1/2] fix(change-quality): preserve CR and CRLF in Git paths Read Git path output as bytes and split NUL-framed records before decoding. This avoids universal-newline translation rewriting legal CR and CRLF filenames, and stops replacement decoding from collapsing distinct undecodable paths. The repository root read uses the same lossless path boundary and removes only Git's terminal LF. Tests cover untracked CR and CRLF names and a checkout directory ending in LF. Signed-off-by: ChengLong GU <88359731+gcl-coder@users.noreply.github.com> --- loopx/capabilities/change_quality/scope.py | 41 ++++++++++++++++++---- tests/test_change_quality_scope_paths.py | 30 ++++++++++++++-- 2 files changed, 62 insertions(+), 9 deletions(-) diff --git a/loopx/capabilities/change_quality/scope.py b/loopx/capabilities/change_quality/scope.py index d526b52621..be963cdb0a 100644 --- a/loopx/capabilities/change_quality/scope.py +++ b/loopx/capabilities/change_quality/scope.py @@ -79,13 +79,29 @@ def _git( ) +def _git_path_text(output: bytes, *, operation: str) -> str: + """Decode one Git path output without normalizing its contents.""" + + raw = bytes(output) + # Git terminates path output with one LF. Strip only that separator so a + # repository directory ending in CR or LF keeps its literal name. + if raw.endswith(b"\n"): + raw = raw[:-1] + try: + return raw.decode("utf-8") + except UnicodeDecodeError: + raise ValueError(f"{operation} returned a path that is not valid UTF-8") from None + + def resolve_git_root(repo_path: Path) -> Path: candidate = repo_path.expanduser().resolve() - result = _git(candidate, "rev-parse", "--show-toplevel", text=True) - root = str(result.stdout or "").strip() - if result.returncode != 0 or not root: - detail = str(result.stderr or "").strip() + result = _git(candidate, "rev-parse", "--show-toplevel") + if result.returncode != 0 or not result.stdout: + detail = bytes(result.stderr or b"").decode("utf-8", errors="replace").strip() raise ValueError(f"repo_path is not a git worktree: {candidate}; {detail}") + root = _git_path_text(result.stdout, operation="git rev-parse --show-toplevel") + if not root: + raise ValueError(f"repo_path is not a git worktree: {candidate}") return Path(root).resolve() @@ -107,11 +123,22 @@ def _name_only(repo_root: Path, *args: str) -> list[str]: literal backslash would arrive as a fragment the caller could not open. """ - result = _git(repo_root, *args, text=True) + result = _git(repo_root, *args) if result.returncode != 0: - detail = str(result.stderr or "").strip() + detail = bytes(result.stderr or b"").decode("utf-8", errors="replace").strip() raise ValueError(f"git {' '.join(args)} failed: {detail}") - return [name for name in str(result.stdout or "").split("\0") if name] + records = bytes(result.stdout or b"").split(b"\0") + names: list[str] = [] + for record in records: + if not record: + continue + try: + names.append(record.decode("utf-8")) + except UnicodeDecodeError: + raise ValueError( + "git reported a changed pathname that is not valid UTF-8" + ) from None + return names def _dedupe(values: list[str]) -> list[str]: diff --git a/tests/test_change_quality_scope_paths.py b/tests/test_change_quality_scope_paths.py index 60e09ba028..4318c64d6e 100644 --- a/tests/test_change_quality_scope_paths.py +++ b/tests/test_change_quality_scope_paths.py @@ -15,8 +15,8 @@ from loopx.capabilities.change_quality.scope import build_change_quality_scope -def _repository(tmp_path: Path) -> Path: - repo = tmp_path / "repo" +def _repository(tmp_path: Path, dirname: str = "repo") -> Path: + repo = tmp_path / dirname repo.mkdir() subprocess.run(["git", "init", "-q", "-b", "main", str(repo)], check=True) subprocess.run( @@ -36,6 +36,32 @@ def _scope(repo: Path) -> dict: return build_change_quality_scope(repo_path=repo, base_ref="HEAD") +@pytest.mark.skipif(os.name != "posix", reason="a carriage return in a path is POSIX-only") +@pytest.mark.parametrize("name", ["line\rbreak.txt", "line\r\nbreak.txt"]) +def test_untracked_names_with_carriage_returns_stay_exact( + tmp_path: Path, name: str +) -> None: + """Text-mode Git reads normalize CR and CRLF; the scope must not.""" + + repo = _repository(tmp_path) + (repo / name).write_text("content\n", encoding="utf-8") + + scope = _scope(repo) + assert scope["changed_files"] == [name] + assert scope["sources"]["untracked"] == [name] + + +@pytest.mark.skipif(os.name != "posix", reason="a newline in a directory is POSIX-only") +def test_repository_root_with_control_characters_is_preserved(tmp_path: Path) -> None: + """A checkout directory ending in LF must not be trimmed to another path.""" + + repo = _repository(tmp_path, "repo\nline") + (repo / "base.txt").write_text("changed\n", encoding="utf-8") + + scope = _scope(repo) + assert scope["changed_files"] == ["base.txt"] + + @pytest.mark.parametrize("name", ["中文文件.txt", "café.txt", "ファイル.txt"]) def test_untracked_names_outside_ascii_keep_their_exact_spelling( tmp_path: Path, name: str From 260936bb2261dab6d1b74d0d6c34cbb19cb5049a Mon Sep 17 00:00:00 2001 From: gcl-coder Date: Thu, 12 Nov 2026 12:00:00 +0000 Subject: [PATCH 2/2] test(change-quality): cover CR, CRLF, and non-UTF-8 Git scope paths Signed-off-by: gcl-coder --- tests/test_change_quality_scope_paths.py | 132 +++++++++++++++++++++-- 1 file changed, 126 insertions(+), 6 deletions(-) diff --git a/tests/test_change_quality_scope_paths.py b/tests/test_change_quality_scope_paths.py index 4318c64d6e..1b7ae16506 100644 --- a/tests/test_change_quality_scope_paths.py +++ b/tests/test_change_quality_scope_paths.py @@ -36,6 +36,50 @@ def _scope(repo: Path) -> dict: return build_change_quality_scope(repo_path=repo, base_ref="HEAD") +def _commit_tree_with_paths(repo: Path, base: str, names: list[bytes]) -> None: + """Commit a synthetic Git tree without requiring those names on disk.""" + + blob = subprocess.check_output( + ["git", "-C", str(repo), "hash-object", "-w", "--stdin"], + input=b"content\n", + ).strip() + tree_input = b"".join( + b"100644 blob " + blob + b"\t" + name + b"\0" for name in names + ) + tree = subprocess.check_output( + ["git", "-C", str(repo), "mktree", "-z"], input=tree_input + ).strip().decode("ascii") + commit = subprocess.check_output( + ["git", "-C", str(repo), "commit-tree", tree, "-p", base, "-m", "fixture"], + text=True, + ).strip() + subprocess.run( + ["git", "-C", str(repo), "update-ref", "refs/heads/main", commit], + check=True, + ) + + + + +def test_committed_cr_names_remain_distinct_from_git_tree(tmp_path: Path) -> None: + """Real Git bytes prove identity even where the host cannot create the names.""" + + repo = _repository(tmp_path) + base = subprocess.check_output( + ["git", "-C", str(repo), "rev-parse", "HEAD"], text=True + ).strip() + names = [b"base.txt", b"line\rbreak.txt", b"line\r\nbreak.txt"] + _commit_tree_with_paths(repo, base, names) + + scope = build_change_quality_scope(repo_path=repo, base_ref=base) + assert set(scope["sources"]["committed"]) == { + "base.txt", + "line\rbreak.txt", + "line\r\nbreak.txt", + } + assert scope["changed_file_count"] == 3 + + @pytest.mark.skipif(os.name != "posix", reason="a carriage return in a path is POSIX-only") @pytest.mark.parametrize("name", ["line\rbreak.txt", "line\r\nbreak.txt"]) def test_untracked_names_with_carriage_returns_stay_exact( @@ -51,17 +95,68 @@ def test_untracked_names_with_carriage_returns_stay_exact( assert scope["sources"]["untracked"] == [name] -@pytest.mark.skipif(os.name != "posix", reason="a newline in a directory is POSIX-only") -def test_repository_root_with_control_characters_is_preserved(tmp_path: Path) -> None: - """A checkout directory ending in LF must not be trimmed to another path.""" +@pytest.mark.skipif(os.name != "posix", reason="a trailing control character in a directory is POSIX-only") +@pytest.mark.parametrize("dirname", ["repo\n", "repo\r", "repo "]) +def test_repository_root_with_trailing_whitespace_is_preserved( + tmp_path: Path, dirname: str +) -> None: + """A checkout directory ending in whitespace must not be trimmed.""" - repo = _repository(tmp_path, "repo\nline") + repo = _repository(tmp_path, dirname) (repo / "base.txt").write_text("changed\n", encoding="utf-8") scope = _scope(repo) assert scope["changed_files"] == ["base.txt"] +@pytest.mark.skipif(os.name != "posix", reason="special names are POSIX-only") +def test_cr_and_lf_names_remain_distinct(tmp_path: Path) -> None: + repo = _repository(tmp_path) + names = ["line\rbreak.txt", "line\nbreak.txt", "line\r\nbreak.txt"] + for name in names: + (repo / name).write_text("content\n", encoding="utf-8") + + scope = _scope(repo) + assert scope["changed_files"] == sorted(names) + assert scope["changed_file_count"] == 3 + + +@pytest.mark.skipif(os.name != "posix", reason="special names are POSIX-only") +def test_cr_names_are_preserved_across_all_scope_sources(tmp_path: Path) -> None: + repo = _repository(tmp_path) + base = subprocess.check_output( + ["git", "-C", str(repo), "rev-parse", "HEAD"], text=True + ).strip() + committed_name = "committed\r.txt" + unstaged_name = "unstaged\r.txt" + for name in (committed_name, unstaged_name): + (repo / name).write_text("first\n", encoding="utf-8") + subprocess.run( + ["git", "-C", str(repo), "add", "--", committed_name, unstaged_name], + check=True, + ) + subprocess.run( + ["git", "-C", str(repo), "commit", "-qm", "add control-character paths"], + check=True, + ) + (repo / unstaged_name).write_text("second\n", encoding="utf-8") + staged_name = "staged\r.txt" + untracked_name = "untracked\r.txt" + (repo / staged_name).write_text("staged\n", encoding="utf-8") + (repo / untracked_name).write_text("untracked\n", encoding="utf-8") + subprocess.run(["git", "-C", str(repo), "add", "--", staged_name], check=True) + + scope = build_change_quality_scope(repo_path=repo, base_ref=base) + assert scope["sources"]["committed"] == sorted([committed_name, unstaged_name]) + assert scope["sources"]["staged"] == [staged_name] + assert scope["sources"]["unstaged"] == [unstaged_name] + assert scope["sources"]["untracked"] == [untracked_name] + assert set(scope["changed_files"]) == { + committed_name, unstaged_name, staged_name, untracked_name + } + assert scope["changed_file_count"] == 4 + + @pytest.mark.parametrize("name", ["中文文件.txt", "café.txt", "ファイル.txt"]) def test_untracked_names_outside_ascii_keep_their_exact_spelling( tmp_path: Path, name: str @@ -115,7 +210,14 @@ def test_changing_one_byte_of_an_untracked_file_changes_the_fingerprint( @pytest.mark.parametrize( "name", - [" padded.py ", "line\nbreak.txt", "back\\slash.py", "café.txt"], + [ + " padded.py ", + "line\rbreak.txt", + "line\r\nbreak.txt", + "line\nbreak.txt", + "back\\slash.py", + "café.txt", + ], ) def test_record_evidence_keeps_the_exact_scope_path(name: str) -> None: """Evidence must name the file the scope reported, not a normalized copy.""" @@ -149,7 +251,14 @@ def test_scope_names_match_the_path_evidence_they_are_recorded_with( """The names the scope reports must be the names evidence resolves against.""" repo = _repository(tmp_path) - names = [" padded.py ", "line\nbreak.txt", "back\\slash.py", "café.txt"] + names = [ + " padded.py ", + "line\rbreak.txt", + "line\r\nbreak.txt", + "line\nbreak.txt", + "back\\slash.py", + "café.txt", + ] for name in names: (repo / name).write_text("content\n", encoding="utf-8") @@ -165,3 +274,14 @@ def test_scope_names_match_the_path_evidence_they_are_recorded_with( instruction_refs=set(), validator_ids=set(), ) + + +def test_non_utf8_changed_path_fails_explicitly(tmp_path: Path) -> None: + repo = _repository(tmp_path) + base = subprocess.check_output( + ["git", "-C", str(repo), "rev-parse", "HEAD"], text=True + ).strip() + _commit_tree_with_paths(repo, base, [b"bad-\xff.txt"]) + + with pytest.raises(ValueError, match="not valid UTF-8"): + build_change_quality_scope(repo_path=repo, base_ref=base)