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..1b7ae16506 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,127 @@ 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( + 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 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, 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 @@ -89,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.""" @@ -123,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") @@ -139,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)