Skip to content

fix(shell): handle undecodable git output in @ file mention#2016

Open
pi-dal wants to merge 2 commits intoMoonshotAI:mainfrom
pi-dal:fix-issue-2015-file-mention-decode
Open

fix(shell): handle undecodable git output in @ file mention#2016
pi-dal wants to merge 2 commits intoMoonshotAI:mainfrom
pi-dal:fix-issue-2015-file-mention-decode

Conversation

@pi-dal
Copy link
Copy Markdown

@pi-dal pi-dal commented Apr 23, 2026

Related Issue

Resolve #2015

Description

This fixes a Windows-only crash in @ file mention completion.

Root cause:

  • entering the third character switches completion from top-level listing to the git-backed deep file scan
  • git ls-files -z output was read with locale-dependent subprocess text decoding
  • on Windows, undecodable path bytes could trigger a UnicodeDecodeError, then cascade into a NoneType.split crash

Changes:

  • read git ls-files output as raw bytes instead of locale-decoded text
  • decode in file_filter.py with utf-8 and surrogateescape
  • handle missing stdout defensively in the parser
  • add a regression test for the Windows decode-failure path

Validation:

  • uv run pytest tests/utils/test_file_filter.py tests/ui_and_conv/test_file_completer.py

Checklist

  • I have read the CONTRIBUTING document.
  • I have linked the related issue, if any.
  • I have added tests that prove my fix is effective or that my feature works.
  • I have run make gen-changelog to update the changelog.
  • I have run make gen-docs to update the user documentation.

@pi-dal pi-dal marked this pull request as ready for review April 23, 2026 02:46
Copilot AI review requested due to automatic review settings April 23, 2026 02:46
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 3 additional findings.

Open in Devin Review

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes a Windows-only crash in @ file mention completion by making git-backed file discovery resilient to undecodable git ls-files -z output.

Changes:

  • Switch git ls-files subprocess calls to read raw bytes output (no locale-dependent text decoding).
  • Add _decode_git_output(..., errors="surrogateescape") and make the ls-files parser tolerate stdout=None.
  • Add a regression test intended to cover the Windows decode-failure scenario.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/kimi_cli/utils/file_filter.py Reads git output as bytes and decodes in a controlled way to avoid Windows locale decode crashes.
tests/utils/test_file_filter.py Adds a regression test for the decode-failure/missing-stdout scenario in git-backed file listing.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +363 to +391
"""Undecodable git output must not crash file discovery on Windows."""

def fake_run(cmd, **kwargs):
text = kwargs.get("text", False)
if "--deleted" in cmd or "--others" in cmd:
return subprocess.CompletedProcess(
cmd,
0,
stdout="" if text else b"",
stderr="" if text else b"",
)
if text:
# Simulate Windows text-mode subprocess decoding failure, where
# stdout ends up as None after the reader thread dies.
return subprocess.CompletedProcess(cmd, 0, stdout=None, stderr="")
return subprocess.CompletedProcess(
cmd,
0,
stdout=b"src/\xadbad.py\0",
stderr=b"",
)

monkeypatch.setattr(subprocess, "run", fake_run)

result = list_files_git(tmp_path)

assert result is not None
assert "src/" in result
assert any(path.endswith("bad.py") for path in result)
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed. I split the coverage instead of keeping the dead text=True branch: one test still exercises undecodable bytes from the main git ls-files call, and another explicitly asserts _parse_ls_files_output(None) == [] so the defensive stdout=None path is covered directly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

通过@搜索文件直接报错 || Search files through @ and report errors directly

2 participants