diff --git a/loopx/capabilities/change_quality/result.py b/loopx/capabilities/change_quality/result.py index 8ede4c277b..9f3f8b3f01 100644 --- a/loopx/capabilities/change_quality/result.py +++ b/loopx/capabilities/change_quality/result.py @@ -105,14 +105,28 @@ def _bounded_text(value: Any, *, field: str, limit: int) -> str: return text +def _exact_text(value: Any, *, field: str, limit: int) -> str: + """Keep the caller's value byte for byte; only reject what cannot be a path. + + Evidence names a file the scope just enumerated, so trimming or collapsing + whitespace here would turn ``" padded.py"`` into a different, unrecordable + name. + """ + + text = str(value or "") + validate_public_safe_text(field, text) + if len(text) > limit: + raise ValueError(f"{field} exceeds {limit} characters") + return text + + def _relative_path(value: Any, *, field: str) -> str | None: - text = str(value or "").strip().replace("\\", "/") + text = _exact_text(value, field=field, limit=1000) if not text: return None path = PurePosixPath(text) if path.is_absolute() or ".." in path.parts: raise ValueError(f"{field} must be a repo-relative path") - validate_public_safe_text(field, text) return path.as_posix() @@ -138,7 +152,7 @@ def _bounded_text_list( def _normalize_evidence_ref(value: Any, *, field: str) -> str: - text = _bounded_text(value, field=field, limit=400) + text = _exact_text(value, field=field, limit=400) kind, separator, target = text.partition(":") if not separator or kind not in EVIDENCE_REF_KINDS: raise ValueError( @@ -155,15 +169,13 @@ def _normalize_evidence_ref(value: Any, *, field: str) -> str: def _normalize_evidence_refs(value: Any, *, field: str) -> list[str]: - refs = [ - _normalize_evidence_ref(item, field=f"{field}[]") - for item in _bounded_text_list( - value, - field=field, - item_limit=400, - count_limit=20, - ) - ] + if value is None: + return [] + if not isinstance(value, list): + raise ValueError(f"{field} must be an array") + if len(value) > 20: + raise ValueError(f"{field} supports at most 20 items") + refs = [_normalize_evidence_ref(item, field=f"{field}[]") for item in value] if len(set(refs)) != len(refs): raise ValueError(f"{field} must not contain duplicates") return refs @@ -270,12 +282,13 @@ def _normalize_validation(value: Any, *, index: int) -> dict[str, Any]: if status in {"failed", "skipped"} and not reason: raise ValueError(f"{field} with status={status} requires reason") if "covers_paths" in value: + covers = value["covers_paths"] + if not isinstance(covers, list): + raise ValueError(f"{field}.covers_paths must be an array") + if len(covers) > 200: + raise ValueError(f"{field}.covers_paths supports at most 200 items") evidence["covers_paths"] = [ - _relative_path(path, field=f"{field}.covers_paths[]") - for path in _bounded_text_list( - value["covers_paths"], field=f"{field}.covers_paths", - item_limit=240, count_limit=200, - ) + _relative_path(item, field=f"{field}.covers_paths[]") for item in covers ] if "failure_attribution" in value: _validate_attribution_public_safety(value["failure_attribution"], field=field) diff --git a/loopx/capabilities/change_quality/scope.py b/loopx/capabilities/change_quality/scope.py index 5a66a75a42..d526b52621 100644 --- a/loopx/capabilities/change_quality/scope.py +++ b/loopx/capabilities/change_quality/scope.py @@ -99,15 +99,19 @@ def _required_git_text(repo_root: Path, *args: str) -> str: def _name_only(repo_root: Path, *args: str) -> list[str]: + """Return the exact paths Git reported, one per NUL-separated record. + + ``-z`` is required at every call site: without it Git quotes non-ASCII + names (``"\\346\\226\\226..."``) and separates records with newlines, so a + legal file name that contains a newline, leading or trailing spaces, or a + literal backslash would arrive as a fragment the caller could not open. + """ + result = _git(repo_root, *args, text=True) if result.returncode != 0: detail = str(result.stderr or "").strip() raise ValueError(f"git {' '.join(args)} failed: {detail}") - return [ - line.strip().replace("\\", "/") - for line in str(result.stdout or "").splitlines() - if line.strip() - ] + return [name for name in str(result.stdout or "").split("\0") if name] def _dedupe(values: list[str]) -> list[str]: @@ -174,12 +178,15 @@ def build_change_quality_scope( repo_root, "diff", "--name-only", + "-z", committed_range, "--", ) - staged = _name_only(repo_root, "diff", "--name-only", "--cached") - unstaged = _name_only(repo_root, "diff", "--name-only") - untracked = _name_only(repo_root, "ls-files", "--others", "--exclude-standard") + staged = _name_only(repo_root, "diff", "--name-only", "-z", "--cached") + unstaged = _name_only(repo_root, "diff", "--name-only", "-z") + untracked = _name_only( + repo_root, "ls-files", "--others", "--exclude-standard", "-z" + ) changed_files = _dedupe([*committed, *staged, *unstaged, *untracked]) digest = hashlib.sha256() diff --git a/tests/test_change_quality_scope_paths.py b/tests/test_change_quality_scope_paths.py new file mode 100644 index 0000000000..60e09ba028 --- /dev/null +++ b/tests/test_change_quality_scope_paths.py @@ -0,0 +1,141 @@ +from __future__ import annotations + +import os +import subprocess +from pathlib import Path + +import pytest + +from loopx.capabilities.change_quality.result import ( + _validate_evidence_targets, + _normalize_evidence_ref, + _normalize_validation, + _relative_path, +) +from loopx.capabilities.change_quality.scope import build_change_quality_scope + + +def _repository(tmp_path: Path) -> Path: + repo = tmp_path / "repo" + repo.mkdir() + subprocess.run(["git", "init", "-q", "-b", "main", str(repo)], check=True) + subprocess.run( + ["git", "-C", str(repo), "config", "user.email", "fixture@example.invalid"], + check=True, + ) + subprocess.run( + ["git", "-C", str(repo), "config", "user.name", "Fixture"], check=True + ) + (repo / "base.txt").write_text("base\n", encoding="utf-8") + subprocess.run(["git", "-C", str(repo), "add", "base.txt"], check=True) + subprocess.run(["git", "-C", str(repo), "commit", "-qm", "base"], check=True) + return repo + + +def _scope(repo: Path) -> dict: + return build_change_quality_scope(repo_path=repo, base_ref="HEAD") + + +@pytest.mark.parametrize("name", ["中文文件.txt", "café.txt", "ファイル.txt"]) +def test_untracked_names_outside_ascii_keep_their_exact_spelling( + tmp_path: Path, name: str +) -> None: + """Git quotes non-ASCII names by default; the scope must still open them.""" + + repo = _repository(tmp_path) + (repo / name).write_text("content\n", encoding="utf-8") + + first = _scope(repo) + assert first["changed_files"] == [name] + assert first["sources"]["untracked"] == [name] + + second = _scope(repo) + assert second["scope_fingerprint"] == first["scope_fingerprint"] + + +def test_untracked_names_keep_surrounding_spaces(tmp_path: Path) -> None: + repo = _repository(tmp_path) + name = " padded file .txt" + (repo / name).write_text("content\n", encoding="utf-8") + + scope = _scope(repo) + assert scope["changed_files"] == [name] + + +@pytest.mark.skipif(os.name != "posix", reason="a newline in a path is POSIX-only") +def test_untracked_names_with_a_newline_stay_one_entry(tmp_path: Path) -> None: + repo = _repository(tmp_path) + name = "line\nbreak.txt" + (repo / name).write_text("content\n", encoding="utf-8") + + scope = _scope(repo) + assert scope["changed_files"] == [name] + assert scope["changed_file_count"] == 1 + + +def test_changing_one_byte_of_an_untracked_file_changes_the_fingerprint( + tmp_path: Path, +) -> None: + repo = _repository(tmp_path) + name = "中文文件.txt" + (repo / name).write_text("content\n", encoding="utf-8") + before = _scope(repo)["scope_fingerprint"] + + (repo / name).write_text("contenu\n", encoding="utf-8") + after = _scope(repo)["scope_fingerprint"] + + assert after != before + + +@pytest.mark.parametrize( + "name", + [" padded.py ", "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.""" + + ref = _normalize_evidence_ref(f"path:{name}", field="evidence_refs[]") + assert ref == f"path:{name}" + assert _relative_path(name, field="risk.path") == name + + evidence = _normalize_validation( + { + "validator": "pytest", + "status": "passed", + "scope": "unit", + "covers_paths": [name], + }, + index=0, + ) + assert evidence["covers_paths"] == [name] + + +@pytest.mark.parametrize("target", ["/abs/file.txt", "../outside.py", "a/../../b"]) +def test_record_evidence_still_rejects_escapes(target: str) -> None: + with pytest.raises(ValueError, match="repo-relative"): + _normalize_evidence_ref(f"path:{target}", field="evidence_refs[]") + + +@pytest.mark.skipif(os.name != "posix", reason="special names are POSIX-only") +def test_scope_names_match_the_path_evidence_they_are_recorded_with( + tmp_path: Path, +) -> None: + """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"] + for name in names: + (repo / name).write_text("content\n", encoding="utf-8") + + changed = set(_scope(repo)["changed_files"]) + assert changed == set(names) + + for name in names: + ref = _normalize_evidence_ref(f"path:{name}", field="evidence_refs[]") + _validate_evidence_targets( + [ref], + field="evidence_refs", + changed_files=changed, + instruction_refs=set(), + validator_ids=set(), + )