From cae0a01f3a7ae1b91ae8bc9deaa6f92cda791460 Mon Sep 17 00:00:00 2001 From: kokokoXUY <13682395396@163.com> Date: Sat, 26 Sep 2026 23:11:09 +0800 Subject: [PATCH] fix(reviewer): frame changed-file records on LF when quotePath is disabled `git diff --name-only` frames one pathname per LF, but `_derive_changed_files` split its output with `str.splitlines()`. Under the default `core.quotePath=true` a non-ASCII pathname arrives octal-escaped, so the previous comment held; a repository that sets `core.quotePath=false` receives the raw pathname instead, and one carrying U+0085 is then reported as two paths. Reproduced on a real repository with `core.quotePath=false` and a commit adding `oddname.txt`: the derived list was `["odd", "name.txt"]` instead of the single pathname. Frame the records on LF and drop the empty trailing record, and state in the comment that the escaped-output guarantee is a property of the default configuration rather than an unconditional one. Validation: the new regression in `tests/capabilities/test_issue_fix_reviewer_history_framing.py` fails against the unmodified function and passes with this change; the ordinary ASCII path is pinned as well. Signed-off-by: kokokoXUY <13682395396@163.com> --- .../issue_fix/reviewer_recommendation.py | 13 ++--- ...test_issue_fix_reviewer_history_framing.py | 52 ++++++++++++++++++- 2 files changed, 58 insertions(+), 7 deletions(-) diff --git a/loopx/capabilities/issue_fix/reviewer_recommendation.py b/loopx/capabilities/issue_fix/reviewer_recommendation.py index 7c2f550253..e99addf966 100644 --- a/loopx/capabilities/issue_fix/reviewer_recommendation.py +++ b/loopx/capabilities/issue_fix/reviewer_recommendation.py @@ -625,12 +625,13 @@ 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"]) - # Deliberately left on `splitlines()`: `git diff --name-only` quotes - # non-ASCII pathnames as octal escapes (`core.quotePath` defaults to true), - # so this stream cannot carry a raw U+0085/U+2028 that would tear a record. - # Streams that emit substituted values verbatim are framed on LF instead; - # see `_collect_history`. - return _normalise_changed_files(output.splitlines()) + # `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]) def _collect_history( diff --git a/tests/capabilities/test_issue_fix_reviewer_history_framing.py b/tests/capabilities/test_issue_fix_reviewer_history_framing.py index de2537ff6d..aeaddf10b1 100644 --- a/tests/capabilities/test_issue_fix_reviewer_history_framing.py +++ b/tests/capabilities/test_issue_fix_reviewer_history_framing.py @@ -6,7 +6,10 @@ import pytest -from loopx.capabilities.issue_fix.reviewer_recommendation import _collect_history +from loopx.capabilities.issue_fix.reviewer_recommendation import ( + _collect_history, + _derive_changed_files, +) AUTHOR_NAME_WITH_SEPARATOR = "Zo{separator}e" @@ -77,3 +80,50 @@ def test_collect_history_keeps_ordinary_ascii_names_unchanged(tmp_path: Path) -> rows = _collect_history(repo, path, revision="HEAD", history_limit=10) assert rows == [("Zoe", "z@example.invalid")] + + +def test_derive_changed_files_frames_records_on_lf_when_quote_path_is_disabled( + tmp_path: Path, +) -> None: + """One changed pathname must stay one record whatever `core.quotePath` is. + + `core.quotePath=false` is a legal repository setting. `git diff --name-only` + then emits a non-ASCII pathname verbatim, and `str.splitlines()` treated the + U+0085 inside it as a record boundary, so one changed file was reported as + the two paths `odd` and `name.txt`. + """ + 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") + _git(repo, "config", "core.quotePath", "false") + + (repo / "base.txt").write_text("base\n", encoding="utf-8") + _git(repo, "add", "base.txt") + _git(repo, "commit", "-m", "base") + + path = "odd\x85name.txt" + (repo / path).write_text("x\n", encoding="utf-8") + _git(repo, "add", path) + _git(repo, "commit", "-m", "add a pathname with a raw separator") + + assert _derive_changed_files(repo, "HEAD~1") == [path] + + +def test_derive_changed_files_keeps_ordinary_ascii_paths(tmp_path: Path) -> None: + 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 / "added.txt").write_text("x\n", encoding="utf-8") + _git(repo, "add", "added.txt") + _git(repo, "commit", "-m", "add a plain path") + + assert _derive_changed_files(repo, "HEAD~1") == ["added.txt"]