Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 30 additions & 17 deletions loopx/capabilities/change_quality/result.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()


Expand All @@ -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(
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down
23 changes: 15 additions & 8 deletions loopx/capabilities/change_quality/scope.py
Original file line number Diff line number Diff line change
Expand Up @@ -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}")

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] 这里现在保留了合法名字,但 downstream record 的 _normalize_evidence_ref/_relative_path 仍 strip/collapse whitespace 并把 backslash 改成 slash。真实 prepare→record:带前后空格、内含换行和 literal POSIX backslash 的文件均 prepare 成功,提交 scope 报告的原路径后却分别被改成 path:padded.py、path:new line.py、path:back/slash.py,报 unknown evidence;普通 Unicode 路径则全旅程成功。请让 evidence、risk.path、covers_paths 保留同一精确 repo-relative 路径,并同时保留绝对/.. 越界拒绝。validator-only 引用的可能绕过不等于文件证据已可用。

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]:
Expand Down Expand Up @@ -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()
Expand Down
141 changes: 141 additions & 0 deletions tests/test_change_quality_scope_paths.py
Original file line number Diff line number Diff line change
@@ -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(),
)
Loading