fix(git): print non-ASCII paths as written and replace undecodable diff bytes

src/git/src/mcp_server_git/server.py

@@ -73,18 +73,47 @@ class GitTools(str, Enum):
     BRANCH = "git_branch"
 
 
+def _git_text(repo: git.Repo, command: str, *args: str) -> str:
+    """Run `git <command> <args>` for a tool that hands git's output to the client.
+
+    Two things about that output are fixed here, for git_status and the three
+    diff tools alike:
+
+    - Paths (#5129). Under git's default `core.quotepath=true`, every byte of a
+      path at or above 0x80 prints as an octal escape (`"\\346\\227\\245.txt"`),
+      a name the client cannot pass back to git_add. `-c core.quotepath=false`
+      prints such names as written, for this one command only: nothing is
+      written to the repository's config. Git still C-quotes a name holding a
+      control character (a tab, say) or a double quote or backslash; the flag
+      governs only the high-bit bytes.
+    - Content (#5193). GitPython decodes output with surrogateescape, so a
+      non-UTF-8 file (Latin-1, say) leaves lone surrogates that cannot be
+      serialized as UTF-8. Take the bytes and replace what does not decode
+      with U+FFFD, as git_show does.
+
+    The `-c` has to come before the subcommand, so it goes through
+    `repo.git(c=...)`; `repo.git.status(c=...)` would put it after and fail.
+    That call sets the option on the shared `repo.git` until its next command
+    runs, which is the one made here.
+    """
+    output = getattr(repo.git(c="core.quotepath=false"), command)(
+        *args, stdout_as_string=False
+    )
+    return output.decode("utf-8", errors="replace")
+
+
 def git_status(repo: git.Repo) -> str:
-    return repo.git.status()
+    return _git_text(repo, "status")
 
 
 def git_diff_unstaged(
     repo: git.Repo, context_lines: int = DEFAULT_CONTEXT_LINES
 ) -> str:
-    return repo.git.diff(f"--unified={context_lines}")
+    return _git_text(repo, "diff", f"--unified={context_lines}")
 
 
 def git_diff_staged(repo: git.Repo, context_lines: int = DEFAULT_CONTEXT_LINES) -> str:
-    return repo.git.diff(f"--unified={context_lines}", "--cached")
+    return _git_text(repo, "diff", f"--unified={context_lines}", "--cached")
 
 
 def git_diff(
@@ -95,7 +124,7 @@ def git_diff(
     if target.startswith("-"):
         raise FlagInjectionError(f"Invalid target: '{target}' - cannot start with '-'")
     repo.rev_parse(target)  # Validates target is a real git ref, throws BadName if not
-    return repo.git.diff(f"--unified={context_lines}", target)
+    return _git_text(repo, "diff", f"--unified={context_lines}", target)
 
 
 def _has_staged_changes(repo: git.Repo) -> bool:

src/git/tests/test_protocol.py

@@ -315,6 +315,42 @@ async def test_git_diff_rejects_flag_injection(repo: git.Repo):
     )
 
 
+async def test_git_diff_of_non_utf8_content_is_a_normal_result(repo: git.Repo):
+    """A Latin-1 file's diff reaches the client with U+FFFD in place of the
+    undecodable bytes, as git_show's does, and serializes as UTF-8 (#5193)."""
+    root = root_of(repo)
+    (root / "latin1.txt").write_bytes(b"caf\xe9\n")
+    repo.index.add(["latin1.txt"])
+    repo.index.commit("add Latin-1 file")
+    (root / "latin1.txt").write_bytes(b"caf\xe9 na\xefve\n")
+    result = await call(None, "git_diff_unstaged", {"repo_path": str(root)})
+    assert result["isError"] is False
+    text = result["content"][0]["text"]
+    assert text.startswith("Unstaged changes:\ndiff --git a/latin1.txt b/latin1.txt")
+    assert "-caf�\n+caf� na�ve" in text
+    json.dumps(result, ensure_ascii=False).encode("utf-8")  # no lone surrogates
+
+
+async def test_git_status_non_ascii_path_round_trips_through_git_add(
+    repo: git.Repo,
+):
+    """Under git's default core.quotepath=true the name still comes back as
+    written, so a client can pass it straight to git_add (#5129)."""
+    root = root_of(repo)
+    with repo.config_writer() as config:
+        config.set_value("core", "quotepath", "true")
+    (root / "日本語.txt").write_text("new\n", newline="\n")
+    status = await call(None, "git_status", {"repo_path": str(root)})
+    assert "\t日本語.txt\n" in status["content"][0]["text"]
+    added = await call(
+        None, "git_add", {"repo_path": str(root), "files": ["日本語.txt"]}
+    )
+    assert added["isError"] is False
+    # Read the index directly: `staged_paths` lists through git, which would
+    # octal-escape the name under the pinned core.quotepath.
+    assert "日本語.txt" in {path for path, _stage in repo.index.entries}
+
+
 # --------------------------------------------------------------------------
 # git_commit
 # --------------------------------------------------------------------------

src/git/tests/test_server.py

@@ -268,6 +268,101 @@ def test_git_diff(test_repository):
     assert "feature changes" in result
 
 
+# A non-ASCII name, and the octal escape git prints for its first character
+# under its default core.quotepath=true (#5129).
+NON_ASCII_FILENAME = "日本語.txt"
+NON_ASCII_OCTAL = "\\346\\227\\245"
+
+
+def pin_quotepath_true(repo: git.Repo) -> None:
+    """Pin git's default repo-locally, so a global config cannot hide the bug."""
+    with repo.config_writer() as config:
+        config.set_value("core", "quotepath", "true")
+
+
+def commit_non_ascii_file(repo: git.Repo) -> None:
+    """Commit NON_ASCII_FILENAME, then leave an unstaged change to it."""
+    pin_quotepath_true(repo)
+    file_path = Path(repo.working_dir) / NON_ASCII_FILENAME
+    file_path.write_text("original\n")
+    repo.index.add([NON_ASCII_FILENAME])
+    repo.index.commit("add non-ASCII file")
+    file_path.write_text("changed\n")
+
+
+def test_git_status_prints_non_ascii_path_unquoted(test_repository):
+    pin_quotepath_true(test_repository)
+    (Path(test_repository.working_dir) / NON_ASCII_FILENAME).write_text("new\n")
+
+    result = git_status(test_repository)
+
+    assert f"\t{NON_ASCII_FILENAME}\n" in result
+    assert NON_ASCII_OCTAL not in result
+
+
+def test_git_status_non_ascii_path_round_trips_through_git_add(test_repository):
+    pin_quotepath_true(test_repository)
+    (Path(test_repository.working_dir) / NON_ASCII_FILENAME).write_text("new\n")
+
+    untracked = [
+        line.strip()
+        for line in git_status(test_repository).splitlines()
+        if line.startswith("\t")
+    ]
+    git_add(test_repository, untracked)
+
+    staged = [path for path, _stage in test_repository.index.entries]
+    assert NON_ASCII_FILENAME in staged
+
+
+@pytest.mark.parametrize("tool", ["unstaged", "staged", "target"])
+def test_git_diff_tools_print_non_ascii_path_unquoted(test_repository, tool):
+    commit_non_ascii_file(test_repository)
+    if tool == "unstaged":
+        result = git_diff_unstaged(test_repository)
+    elif tool == "staged":
+        test_repository.index.add([NON_ASCII_FILENAME])
+        result = git_diff_staged(test_repository)
+    else:
+        result = git_diff(test_repository, "HEAD")
+
+    assert f"diff --git a/{NON_ASCII_FILENAME} b/{NON_ASCII_FILENAME}" in result
+    assert NON_ASCII_OCTAL not in result
+
+
+def test_quotepath_override_is_not_written_to_repo_config(test_repository):
+    commit_non_ascii_file(test_repository)
+    config_path = Path(test_repository.git_dir) / "config"
+    before = config_path.read_bytes()
+
+    git_status(test_repository)
+    git_diff_unstaged(test_repository)
+
+    assert config_path.read_bytes() == before
+    # The override was for that one command: a plain call still escapes.
+    assert NON_ASCII_OCTAL in test_repository.git.status()
+
+
+@pytest.mark.parametrize("tool", ["unstaged", "staged", "target"])
+def test_git_diff_tools_replace_undecodable_bytes(test_repository, tool):
+    """A Latin-1 file's diff comes back with U+FFFD, not lone surrogates (#5193)."""
+    file_path = Path(test_repository.working_dir) / "latin1.txt"
+    file_path.write_bytes(b"caf\xe9\n")
+    test_repository.index.add(["latin1.txt"])
+    test_repository.index.commit("add Latin-1 file")
+    file_path.write_bytes(b"caf\xe9 na\xefve\n")
+    if tool == "unstaged":
+        result = git_diff_unstaged(test_repository)
+    elif tool == "staged":
+        test_repository.index.add(["latin1.txt"])
+        result = git_diff_staged(test_repository)
+    else:
+        result = git_diff(test_repository, "HEAD")
+
+    assert "+caf� na�ve" in result
+    result.encode("utf-8")  # raises UnicodeEncodeError on a lone surrogate
+
+
 def test_git_commit(test_repository):
     file_path = Path(test_repository.working_dir) / "commit_test.txt"
     file_path.write_text("content to commit")