Skip to content

fix(core): preserve non-ASCII git paths in file crawler - #4766

Closed
kkhomej33-netizen wants to merge 1 commit into
QwenLM:mainfrom
kkhomej33-netizen:codex/fix-git-quote-path
Closed

fix(core): preserve non-ASCII git paths in file crawler#4766
kkhomej33-netizen wants to merge 1 commit into
QwenLM:mainfrom
kkhomej33-netizen:codex/fix-git-quote-path

Conversation

@kkhomej33-netizen

Copy link
Copy Markdown
Contributor

What this PR does

This PR makes the file crawler run its internal Git commands with path quoting disabled so non-ASCII tracked filenames are returned as normal UTF-8 paths instead of Git's octal-escaped form. It also adds regression coverage for a repository that explicitly enables Git path quoting.

Why it's needed

The @ file completion path can use git ls-files for tracked files. With Git's default core.quotePath behavior, tracked filenames such as Chinese names can be emitted as escaped byte sequences like \346\212..., and the CLI surfaces that escaped text directly in suggestions. Disabling quotePath for these internal list commands keeps the completion UI readable without changing the user's repository configuration.

Reviewer Test Plan

How to verify

Create or use a Git repository with a tracked non-ASCII filename and core.quotePath=true, then trigger @ file completion or run the crawler tests. The filename should appear in readable text, not as octal escape sequences. Locally verified with: cd packages/core && npx vitest run src/utils/filesearch/crawler.test.ts; cd packages/core && npm run typecheck; cd packages/core && npm run build.

Evidence (Before & After)

Before: Git can emit tracked non-ASCII paths as octal escapes when core.quotePath is enabled. After: the new regression test forces core.quotePath=true and confirms the crawler returns 设计文档.txt without \346-style escapes.

Tested on

OS Status
🍏 macOS ✅ tested
🪟 Windows ⚠️ not tested
🐧 Linux ⚠️ not tested

Environment (optional)

Node.js v22.22.2, package-level Vitest/typecheck/build in packages/core.

Risk & Scope

  • Main risk or tradeoff: This changes only internal Git command configuration for path listing; it should not mutate user Git config or repository state.
  • Not validated / out of scope: Manual TUI screenshot verification and Windows/Linux local runs were not performed.
  • Breaking changes / migration notes: None.

Linked Issues

N/A

中文说明

这个 PR 做了什么

这个 PR 让文件 crawler 在执行内部 Git 命令时关闭路径转义,使非 ASCII 的 tracked 文件名以正常 UTF-8 路径返回,而不是 Git 的八进制转义形式。同时新增了一个显式开启 Git 路径转义仓库的回归测试。

为什么需要

@ 文件补全路径可能会对 tracked 文件使用 git ls-files。在 Git 默认 core.quotePath 行为下,中文等非 ASCII 文件名可能被输出成类似 \346\212... 的字节转义,并被 CLI 原样展示到补全建议里。对这些内部列表命令禁用 quotePath 可以让补全 UI 保持可读,同时不修改用户仓库配置。

Reviewer Test Plan

如何验证

创建或使用一个包含 tracked 非 ASCII 文件名且 core.quotePath=true 的 Git 仓库,然后触发 @ 文件补全或运行 crawler 测试。文件名应该以可读文本显示,而不是八进制转义。本地已验证:cd packages/core && npx vitest run src/utils/filesearch/crawler.test.ts;cd packages/core && npm run typecheck;cd packages/core && npm run build。

证据(Before & After)

Before:当 core.quotePath 启用时,Git 可能把 tracked 非 ASCII 路径输出为八进制转义。After:新的回归测试强制 core.quotePath=true,并确认 crawler 返回 设计文档.txt,而不是 \346 形式的转义。

已测试平台

OS 状态
🍏 macOS ✅ 已测试
🪟 Windows ⚠️ 未测试
🐧 Linux ⚠️ 未测试

环境(可选)

Node.js v22.22.2,在 packages/core 中运行了包级 Vitest、typecheck 和 build。

风险与范围

  • 主要风险或取舍:只改变内部 Git 列表命令配置,不会修改用户 Git 配置或仓库状态。
  • 未验证 / 范围外:未进行手动 TUI 截图验证,也未在 Windows/Linux 本地运行。
  • 破坏性变更 / 迁移说明:无。

关联 Issue

N/A


if (args.includes('rev-parse') && args.includes('--show-toplevel')) {
return { success: true, lines: [tmpDir] };
}

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.

[Suggestion] expect.arrayContaining(['-c', 'core.quotePath=false']) checks that both strings exist somewhere in the args array but does not verify they are adjacent. Since withSafeGitConfig already produces multiple -c entries, the '-c' match is trivially satisfied by an unrelated entry. A bug that placed core.quotePath=false without a preceding -c would go undetected.

Consider verifying adjacency:

const idx = args.indexOf('core.quotePath=false');
expect(idx).toBeGreaterThan(0);
expect(args[idx - 1]).toBe('-c');

Note: the integration test above (real git, real non-ASCII file) would catch a positional regression, so this is low-risk in practice.

— qwen3.7-max via Qwen Code /review

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks, good point. Updated the mock assertion to verify that core.quotePath=false is immediately preceded by -c, while keeping the real Git regression test for the end-to-end behavior. Verified locally with cd packages/core && npx vitest run src/utils/filesearch/crawler.test.ts (46 tests passed).

@kkhomej33-netizen
kkhomej33-netizen force-pushed the codex/fix-git-quote-path branch from 88528f1 to f153c57 Compare June 5, 2026 02:27
@wenshao

wenshao commented Jun 5, 2026

Copy link
Copy Markdown
Collaborator

Local Verification Report

PR: #4766 — fix(core): preserve non-ASCII git paths in file crawler
Branch: codex/fix-git-quote-pathmain
Commit: latest on branch
Environment: macOS Darwin 25.4.0 (arm64), Node.js v22.17.0


Summary

Minimal 2-line production fix: adds -c core.quotePath=false to the withSafeGitConfig() helper in crawler.ts, which is used by all 4 internal git commands (rev-parse, ls-files --cached, --deleted, --others). This ensures non-ASCII filenames (e.g., Chinese characters) are returned as raw UTF-8 instead of Git's octal-escaped form (\346\212...), fixing garbled @ file completion suggestions.

Changes: 2 files (+76/−2 lines)

  • packages/core/src/utils/filesearch/crawler.ts — 2 lines: add -c core.quotePath=false to withSafeGitConfig()
  • packages/core/src/utils/filesearch/crawler.test.ts — 74 lines: 2 new tests

Build & Type Check

Step Result Notes
npm run build ✅ PASS (exit 0)
tsc --noEmit ✅ PASS (exit 0) No errors in PR-touched files.

Tests

Suite Tests Result Notes
New: should disable Git path quoting for crawler commands (mock-based) 1 passed ✅ PASS Verifies -c core.quotePath=false is passed to every git invocation via __setCommandRunnerForTests.
New: should preserve non-ASCII tracked filenames (integration) 1 timed out ⚠️ Pre-existing env issue Requires real git execution; times out at 5s like all other git-dependent crawler tests on this machine (10 of 44 existing tests also timeout).
Existing crawler tests 34 passed, 10 timed out ⚠️ Pre-existing Same 5s timeout pattern for all git-calling tests. All non-git tests pass.
Full core suite ~718 passed, ~17 failed ⚠️ Pre-existing Failures: crawler timeouts, fetchGitDiff timeouts, AnthropicContentGenerator identity — all unrelated.

Code Review

Correctness:

  • -c core.quotePath=false is a per-process config override — it does NOT modify the user's .gitconfig or repo config
  • Placed alongside existing -c core.fsmonitor=false and -c core.untrackedCache=false in withSafeGitConfig(), following the established pattern
  • Applied to all 4 call sites (rev-parse, ls-files --cached, --deleted, --others) — consistent coverage

Test quality:

  • Mock-based test (should disable Git path quoting) verifies the contract (every git call receives the -c flag) without needing real git — robust across environments
  • Integration test (should preserve non-ASCII tracked filenames) sets core.quotePath=true in a temp repo and asserts 设计文档.txt is returned without \346 escapes — excellent regression coverage when git is available

No issues found.


Verdict

PASS — Safe to merge. Minimal, well-targeted fix that follows existing patterns. The mock-based test confirms correct behavior; the integration test provides additional confidence in environments where git doesn't timeout.


Verified by wenshao (local tmux parallel execution)

@wenshao wenshao left a comment

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.

Clean, focused fix. The -c core.quotePath=false addition to withSafeGitConfig() correctly solves the non-ASCII path issue, and both tests (behavioral + mock-based contract) provide solid coverage. tsc clean, 46/46 tests pass, CI 10/10 green. LGTM. ✅ — qwen3.7-max via Qwen Code /review

xaelistic pushed a commit to xaelistic/qwen-code that referenced this pull request Jun 7, 2026
@tanzhenxin tanzhenxin added the type/bug Something isn't working as expected label Jun 8, 2026
@tanzhenxin

Copy link
Copy Markdown
Collaborator

Thanks for this fix, @kkhomej33-netizen — your diagnosis was spot on: git's default core.quotePath was octal-escaping non-ASCII tracked filenames, and -c core.quotePath=false in withSafeGitConfig is exactly the right fix at the right layer.

Closing this as already resolved on main. The same fix landed independently via #4596 ("fix(core): recurse into submodule files when crawling git repos", commit 6d64b34), which added core.quotePath=false to the same withSafeGitConfig function, plus an equivalent regression test (should preserve non-ASCII tracked paths from git output in crawler.test.ts). That's also why this branch now shows a conflict — both changes append the same lines to the same arg list.

So the non-ASCII path bug is fixed on main as of today; no further action needed here. The only piece unique to this PR is the mock-based guard test asserting the flag stays present in every git arg list — if you'd like to keep that as a regression guard, feel free to open a small standalone PR for just that test and we'll review it. Thanks again for catching and fixing the underlying issue!

@tanzhenxin tanzhenxin closed this Jun 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type/bug Something isn't working as expected

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants