From bb0164d99f01ff852b07c66754f27c9c2ea51fcb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=85=8B=E7=AB=9F?= <1048927295@qq.com> Date: Mon, 27 Apr 2026 14:57:33 +0800 Subject: [PATCH] fix(core): pin /diff git ops to repo root and lstat untracked entries Two Critical findings on PR #3491: 1. (line 63) When /diff is invoked from a subdirectory of the worktree, `git diff` emits repo-root-relative paths but `git ls-files --others` is scoped to cwd and emits cwd-relative paths. Result: mixed path bases in `perFileStats` and silent omission of untracked files in sibling directories. Resolve `findGitRoot(cwd)` once and run every git invocation (and `path.join(...)` for line counting) from there, so all keys are repo-root-relative and the listing is repo-wide. 2. (line 455) `countUntrackedLines` opened every untracked path with `open(absPath, 'r')`. Git's `ls-files --others` can list FIFOs (whose `open()` blocks indefinitely waiting on a writer) and symlinks (which `open()` dereferences, potentially reading outside the worktree). Add an `lstat` gate: only regular files are counted; symlinks and other special files render as binary `~` rows. Two new integration tests cover both regressions: one creates a sibling untracked file at the repo root and invokes fetchGitDiff from a subdir asserting all three changes (root + sub) come back keyed by repo-root-relative paths; the other creates a symlink pointing at content outside the worktree and asserts it lands as a binary row with no contribution to linesAdded. --- .npmrc | 1 + README.md | 1 + packages/core/src/utils/gitDiff.test.ts | 97 +++++++++++++++++++++++++ packages/core/src/utils/gitDiff.ts | 44 +++++++++-- 4 files changed, 137 insertions(+), 6 deletions(-) diff --git a/.npmrc b/.npmrc index 38f11c645..e28d5df67 100644 --- a/.npmrc +++ b/.npmrc @@ -1 +1,2 @@ registry=https://registry.npmjs.org +# test comment for diff verification diff --git a/README.md b/README.md index 5358f9d0e..7b30fdd4c 100644 --- a/README.md +++ b/README.md @@ -1,3 +1,4 @@ +
[![npm version](https://img.shields.io/npm/v/@qwen-code/qwen-code.svg)](https://www.npmjs.com/package/@qwen-code/qwen-code) diff --git a/packages/core/src/utils/gitDiff.test.ts b/packages/core/src/utils/gitDiff.test.ts index b87d7a6fa..be57f892c 100644 --- a/packages/core/src/utils/gitDiff.test.ts +++ b/packages/core/src/utils/gitDiff.test.ts @@ -738,6 +738,103 @@ describe('fetchGitDiff untracked with special filenames', () => { }); }); +describe('fetchGitDiff invocation from a subdirectory', () => { + it('returns repo-wide changes with consistent repo-root-relative path keys', async () => { + // Reproduces wenshao Critical (PR #3491 line 63): when /diff was invoked + // from a subdir, `git diff --numstat` emitted repo-root-relative keys but + // `ls-files --others` was scoped to cwd, so untracked files outside the + // subdir were silently dropped and the path basis was inconsistent. + const repo = await fs.mkdtemp(path.join(os.tmpdir(), 'qwen-gitdiff-sub-')); + try { + await execFileAsync('git', ['init', '-q', '-b', 'main'], { cwd: repo }); + await execFileAsync('git', ['config', 'user.email', 't@e.com'], { + cwd: repo, + }); + await execFileAsync('git', ['config', 'user.name', 'T'], { cwd: repo }); + await execFileAsync('git', ['config', 'commit.gpgsign', 'false'], { + cwd: repo, + }); + await fs.mkdir(path.join(repo, 'sub'), { recursive: true }); + await fs.writeFile(path.join(repo, 'sub', 'tracked.txt'), 'x\n'); + await fs.writeFile(path.join(repo, 'rootkeep.txt'), 'k\n'); + await execFileAsync('git', ['add', '.'], { cwd: repo }); + await execFileAsync('git', ['commit', '-q', '-m', 'init'], { cwd: repo }); + + // Modify a tracked file inside the subdir. + await fs.writeFile(path.join(repo, 'sub', 'tracked.txt'), 'y\n'); + // Add an untracked file in a sibling location at the repo root. + await fs.writeFile(path.join(repo, 'rootnew.txt'), 'fresh\n'); + // And one in the subdir for good measure. + await fs.writeFile(path.join(repo, 'sub', 'subnew.txt'), 'a\nb\n'); + + // Invoke fetchGitDiff with cwd pointing at the SUBDIR, not the root. + const result = await fetchGitDiff(path.join(repo, 'sub')); + expect(result).not.toBeNull(); + const keys = [...result!.perFileStats.keys()].sort(); + // All path keys must be repo-root-relative (not "tracked.txt" or + // "subnew.txt" alone). And the root-level untracked file must be + // present even though we asked from sub/. + expect(keys).toEqual([ + 'rootnew.txt', + 'sub/subnew.txt', + 'sub/tracked.txt', + ]); + expect(result!.stats.filesCount).toBe(3); + } finally { + await fs.rm(repo, { recursive: true, force: true }); + } + }); +}); + +describe('fetchGitDiff special filetypes among untracked files', () => { + it('marks untracked symlinks as binary and never follows them', async () => { + // Reproduces wenshao Critical (PR #3491 line 455): without an lstat + // gate, `open()` would dereference an untracked symlink and read its + // target — which can live outside the worktree. + const repo = await fs.mkdtemp(path.join(os.tmpdir(), 'qwen-gitdiff-lnk-')); + try { + await execFileAsync('git', ['init', '-q', '-b', 'main'], { cwd: repo }); + await execFileAsync('git', ['config', 'user.email', 't@e.com'], { + cwd: repo, + }); + await execFileAsync('git', ['config', 'user.name', 'T'], { cwd: repo }); + await execFileAsync('git', ['config', 'commit.gpgsign', 'false'], { + cwd: repo, + }); + await fs.writeFile(path.join(repo, 'seed.txt'), 'x\n'); + await execFileAsync('git', ['add', '.'], { cwd: repo }); + await execFileAsync('git', ['commit', '-q', '-m', 'init'], { cwd: repo }); + + // Create an outside-worktree target with content that, if followed, + // would push linesAdded up. The lstat gate means we never read it. + const outside = await fs.mkdtemp(path.join(os.tmpdir(), 'qwen-outside-')); + try { + await fs.writeFile( + path.join(outside, 'secret.txt'), + 'one\ntwo\nthree\n', + ); + await fs.symlink( + path.join(outside, 'secret.txt'), + path.join(repo, 'link.txt'), + ); + + const result = await fetchGitDiff(repo); + expect(result).not.toBeNull(); + const entry = result!.perFileStats.get('link.txt'); + expect(entry).toBeDefined(); + expect(entry?.isBinary).toBe(true); + expect(entry?.isUntracked).toBe(true); + // No content from the symlink target leaked into the totals. + expect(result!.stats.linesAdded).toBe(0); + } finally { + await fs.rm(outside, { recursive: true, force: true }); + } + } finally { + await fs.rm(repo, { recursive: true, force: true }); + } + }); +}); + describe('fetchGitDiff untracked counting', () => { let repo: string; beforeEach(async () => { diff --git a/packages/core/src/utils/gitDiff.ts b/packages/core/src/utils/gitDiff.ts index 331d9a222..d352682f4 100644 --- a/packages/core/src/utils/gitDiff.ts +++ b/packages/core/src/utils/gitDiff.ts @@ -5,7 +5,7 @@ */ import { execFile } from 'node:child_process'; -import { access, open, readFile, stat } from 'node:fs/promises'; +import { access, lstat, open, readFile, stat } from 'node:fs/promises'; import * as path from 'node:path'; import { promisify } from 'node:util'; import type { Hunk } from 'diff'; @@ -65,6 +65,15 @@ export async function fetchGitDiff(cwd: string): Promise { if (!isGitRepository(cwd)) return null; if (await isInTransientGitState(cwd)) return null; + // Pin every git invocation (and on-disk file probe) to the repo root. + // `git diff` already emits repo-root-relative paths regardless of cwd, but + // `git ls-files --others` is scoped to cwd — running both from the same + // root keeps the path keys consistent and ensures untracked files in + // sibling directories aren't silently dropped when /diff is invoked from + // a subdirectory of the worktree. + const gitRoot = findGitRoot(cwd); + if (!gitRoot) return null; + // Shortstat probe + untracked scan run in parallel — both are needed // regardless of which path we take, and shortstat is O(1) memory so it can // short-circuit huge generated workspaces before we pay the per-file @@ -72,7 +81,7 @@ export async function fetchGitDiff(cwd: string): Promise { // list so the fast path only has to count NUL bytes instead of allocating // a full path array. const [shortstatOut, untrackedOut] = await Promise.all([ - runGit(['--no-optional-locks', 'diff', 'HEAD', '--shortstat'], cwd), + runGit(['--no-optional-locks', 'diff', 'HEAD', '--shortstat'], gitRoot), runGit( [ '--no-optional-locks', @@ -81,7 +90,7 @@ export async function fetchGitDiff(cwd: string): Promise { '--others', '--exclude-standard', ], - cwd, + gitRoot, ), ]); const untrackedCount = countNulDelimited(untrackedOut); @@ -107,7 +116,7 @@ export async function fetchGitDiff(cwd: string): Promise { const numstatOut = await runGit( ['--no-optional-locks', 'diff', 'HEAD', '--numstat', '-z'], - cwd, + gitRoot, ); if (numstatOut == null) return null; @@ -127,7 +136,9 @@ export async function fetchGitDiff(cwd: string): Promise { // already filled the per-file map. const countable = untrackedPaths.slice(0, MAX_FILES); const countableStats = await Promise.all( - countable.map((relPath) => countUntrackedLines(path.join(cwd, relPath))), + countable.map((relPath) => + countUntrackedLines(path.join(gitRoot, relPath)), + ), ); for (const s of countableStats) stats.linesAdded += s.added; @@ -162,8 +173,15 @@ export async function fetchGitDiffHunks( ): Promise> { if (!isGitRepository(cwd)) return new Map(); if (await isInTransientGitState(cwd)) return new Map(); + // Run from the repo root so hunk keys are repo-root-relative regardless of + // which subdirectory the caller is in. + const gitRoot = findGitRoot(cwd); + if (!gitRoot) return new Map(); - const diffOut = await runGit(['--no-optional-locks', 'diff', 'HEAD'], cwd); + const diffOut = await runGit( + ['--no-optional-locks', 'diff', 'HEAD'], + gitRoot, + ); if (diffOut == null) return new Map(); return parseGitDiff(diffOut); } @@ -446,10 +464,24 @@ interface UntrackedLineStats { * unreadable file can't block the whole command. `truncated` is set when * `fstat(size) > bytesRead`, so the UI can mark partial counts honestly * instead of silently under-reporting a 10 MB log as `+20k`. + * + * Uses `lstat` before `open` to gate on regular files only — git's + * `ls-files --others` can list FIFOs (whose `open()` would block forever + * waiting on a writer) and symlinks (whose target may live outside the + * worktree). Symlinks and non-regular files render as binary `~` rows. */ async function countUntrackedLines( absPath: string, ): Promise { + let st; + try { + st = await lstat(absPath); + } catch { + return { added: 0, isBinary: false, truncated: false }; + } + if (!st.isFile()) { + return { added: 0, isBinary: true, truncated: false }; + } let fh; try { fh = await open(absPath, 'r');