* feat(core): add git diff statistics utility
Port numstat + unified-diff parsing into `packages/core/src/utils/gitDiff.ts`
to surface structured working-tree change summaries (files changed, lines
added/removed, per-file hunks) against HEAD. Caps mirror issue #2997:
50 files, 1MB per file, 400 lines per file, with a 500-file short-circuit
via `git diff --shortstat` to avoid expensive work on massive diffs.
- `fetchGitDiff(cwd)` returns stats + per-file summaries (tracked + untracked).
- `fetchGitDiffHunks(cwd)` returns structured hunks on demand.
- `resolveGitDir(cwd)` follows `.git` file indirection so linked worktrees
and submodules report the correct gitdir.
- Transient-state short-circuit covers merge, cherry-pick, revert, and both
`rebase-merge` / `rebase-apply` layouts.
- `core.quotepath=false` is forced so non-ASCII filenames stay as UTF-8.
Refs #2997
* feat(cli): add /diff slash command
Surface the `fetchGitDiff` utility through an interactive `/diff` command.
Prints a header (`N files changed, +A / -R`) followed by per-file rows with
padded add/remove counts. Untracked files are marked `?`, binary files are
marked `~`. When the change set exceeds the per-file cap, a trailing
`…and N more` note tells the user how many entries are hidden.
Returns a `MessageActionReturn` so it renders the same way in interactive
and non-interactive modes.
* fix(cli): harden /diff command after adversarial audit
- Wrap `fetchGitDiff` in try/catch so permission errors on `.git` surface
as a friendly error message instead of crashing the action.
- Declare `supportedModes: ['interactive', 'non_interactive', 'acp']` so
the command is reachable outside the interactive Ink UI — the default
for `commandType: 'local'` is interactive-only.
- Align `?` (untracked) and `~` (binary) markers with the `+X -Y` stat
column via a padded prefix, so filenames line up regardless of row kind.
- Drop the "…and N more" hint when no rows are shown (shortstat fast-path
with >500 files) — the count alone is sufficient and "showing first 0"
is noise.
- Switch header to full-phrase i18n templates (separate singular/plural
variants) instead of word-by-word `t()` calls that don't survive
non-English locales.
- Extend tests to 12 scenarios: empty cwd, fetch rejection, singular
"file" form, mixed untracked/binary/tracked alignment, 4-digit padding,
shortstat fast-path, and supportedModes declaration. Mocks carry a
`satisfies GitDiffResult` annotation so shape drift in core breaks the
test at compile time.
* fix(cli): clean up /diff feature review issues
- Remove invalid `commandType` field from diffCommand (SlashCommand has
no such property; caused a TS build failure).
- Drop duplicate `NumstatResult` interface in gitDiff.ts — it is
structurally identical to `GitDiffResult`.
- Register the 9 missing `/diff` i18n strings in en.js / zh.js so the
command is translatable (previously only `Configuration not available.`
had entries).
* fix(core): harden git diff stats after multi-round review
- fetchUntrackedPaths now uses `ls-files -z` so filenames containing
newlines, tabs, or non-ASCII bytes round-trip cleanly instead of
being C-style quoted and split into phantom entries.
- fetchGitDiff runs the `--shortstat` probe and the untracked-paths
lookup in parallel, since both are needed regardless of which path
the function takes.
- parseGitDiff measures per-file diff size via Buffer.byteLength so
MAX_DIFF_SIZE_BYTES matches its documented meaning on non-ASCII diffs.
- Adds a regression test for an untracked file whose name contains a
literal newline.
* fix(core): address /diff PR review comments
Addresses the five open review threads on #3491:
- parseShortstat: anchored and bounded the regex (`^...$` with `\d{1,10}`)
so adversarial inputs can no longer drive polynomial backtracking. Closes
CodeQL alert #137.
- fetchGitDiff: only parse the untracked-path list when we actually need
it; the fast path now counts NUL bytes in the raw `ls-files -z` stdout
(wenshao P1).
- fetchGitDiff: base the `MAX_FILES_FOR_DETAILS` short-circuit on
`tracked + untracked`, so repos with few edits but many untracked files
still take the summary-only path (wenshao P2).
- fetchGitDiff: count newlines in each untracked text file (binary sniff +
1 MB read cap) and fold that into both the header `+N` and the per-file
row, so a brand-new file no longer renders as `+0 / -0` (BZ-D P2).
- parseGitNumstat: switch to `git diff --numstat -z`. The parser now uses
index-based slicing and a rename-pair state machine, so tracked
filenames containing tabs/newlines/non-ASCII keep their real bytes
(BZ-D P3). Renames collapse into a single `old => new` entry.
UI: untracked rows render as `+N filename (new)` (or
`~ filename (binary, new)`) instead of the placeholder `?` marker;
`/diff` now shows real additions for fresh files.
* fix(core): surface truncated untracked counts and decouple totals from display
Two issues surfaced during a directionless multi-round audit of the /diff
feature:
1. `countUntrackedLines` reads at most `UNTRACKED_READ_CAP_BYTES` (1 MB)
per file, so a 10 MB new log was silently reported as `+~20k` when the
real count is ~10×. The helper now `fstat`s the file and returns a
`truncated: true` flag when size exceeds the read window; `/diff`
surfaces it as `(new, partial)` so the `+N` isn't read as exact.
2. Line-count aggregation was coupled to the per-file display cap: when
tracked changes filled the `MAX_FILES` slot, untracked line counts
beyond the remaining slots were dropped from `stats.linesAdded`
entirely (header under-reported additions). Decoupled: we now read up
to `MAX_FILES` untracked files for their line counts regardless of
display slots, and only restrict the visible rows to `remainingSlots`.
Added regression tests for both: a 1.5 MB new file asserts `truncated:
true` and a lower-bound line count, and a `MAX_FILES`-saturated tracked
set + 5 untracked files asserts that untracked additions still appear in
the header totals even though none of them get displayed.
* fix(core): parse filenames from +++/--- lines to handle paths with ' b/'
`diff --git a/X b/Y` is ambiguous when X contains ` b/` — a file literally
named `a b/c.txt` produces `diff --git a/a b/c.txt b/a b/c.txt` with no
escape or quoting, and the previous regex `^a\/(.+?) b\/(.+)$` keyed the
hunks under the wrong path. Consumers of the exported `fetchGitDiffHunks`
API would then fail to correlate hunks with stats or editor paths.
Introduces `extractFilePath(lines)` which walks the block for the
unambiguous markers (`rename to` / `copy to` / `+++ b/<path>` with a
`/dev/null` fallback to `--- a/<path>`) and strips the trailing TAB git
appends to paths containing whitespace. Adds unit tests for the
`a b/c.txt`, rename, delete, and new-file cases plus an end-to-end test
that creates a real `a b/c.txt` file and asserts `fetchGitDiffHunks`
keys the hunks correctly.
Addresses wenshao review comment #3136657141 on #3491.
* feat(cli): colorize /diff output via a themed Ink component
The /diff stats used to come back as a plain-text MessageActionReturn.
Pipes and ACP still get that, but in interactive terminals we now dispatch
a structured history item so the numbers can carry theme colors.
- packages/cli/src/ui/types.ts — new DiffRenderRow / DiffRenderModel /
HistoryItemDiffStats, MessageType.DIFF_STATS.
- packages/cli/src/ui/components/messages/DiffStatsDisplay.tsx — renders
+N in theme.status.success (green), -M in theme.status.error (red), and
the (new) / (binary) / (new, partial) markers in theme.text.secondary
(dim). Column alignment matches the plain-text fallback.
- packages/cli/src/ui/components/HistoryItemDisplay.tsx — routes the new
item type.
- packages/cli/src/ui/commands/diffCommand.ts — builds a DiffRenderModel
once and fans out: interactive calls context.ui.addItem; other modes
fall through to renderDiffModelText() for the plain-text path. Error
and "clean tree" branches keep the existing info/error
MessageActionReturn in every mode.
- Tests: existing diffCommand suite moved to an explicit non_interactive
context (it was asserting text content); new interactive suite covers
addItem dispatch and model shape; DiffStatsDisplay component tests
cover the four row variants and the "…and N more" note.
* refactor(cli): factor /diff column widths into a shared helper
Audit of the colorize commit found one real DRY hazard: DiffStatsDisplay
and renderDiffModelText each independently re-derived addWidth /
remWidth / statColumnWidth from the same row list. If anyone later
changed one formula, the interactive Ink output and the non-interactive
plain text would silently fall out of column alignment.
Extract the computation into computeDiffColumnWidths() exported from
diffCommand.ts; both renderers now call it. Adds a focused unit test of
the contract (empty rows, widest non-binary row wins, binary rows are
ignored, untracked text rows count). Drop a redundant
`Omit<HistoryItemDiffStats, 'id'>` annotation since the type already has
no id field.
* 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.
* chore: revert stray .npmrc/README.md test edits swept into bb0164d99
The previous fix(core) commit accidentally bundled two unrelated
working-tree edits (a test comment in .npmrc and a TODO in README.md)
that I had used while sanity-testing /diff. They have nothing to do
with the fix; restore them to their pre-bb0164d99 state.
* perf(core): stream untracked-file line counts in 64 KB chunks
`countUntrackedLines` allocated a fresh `UNTRACKED_READ_CAP_BYTES`
(1 MB) buffer per file. With up to MAX_FILES (=50) line-counts
running concurrently via `Promise.all`, the worst-case heap
footprint of a single `/diff` invocation was ~50 MB of transient
buffers — avoidable spike on small containers / low-memory hosts
flagged by wenshao on PR #3491.
Switch to a fixed 64 KB chunk buffer and read in a loop, accumulating
line counts and tracking the last byte across iterations. Peak
footprint is now ~3.2 MB (50 × 64 KB). Behavior is identical: same
binary sniff over the first 8 KB, same truncation flag when the read
hits the cap with bytes still on disk, same trailing-partial-line
rule. All 44 gitDiff tests pass unchanged, including the 1.5 MB
truncation test which now crosses chunk boundaries.
* refactor(core): collapse redundant ancestor walks; harden untracked open
Multi-round audit of the recent /diff fixes turned up two real issues:
1. `fetchGitDiff` and `fetchGitDiffHunks` walked worktree ancestors
three times each — `isGitRepository(cwd)`, then
`isInTransientGitState → resolveGitDir → findGitRoot`, then
`findGitRoot(cwd)` to pin git ops to the root. Resolve once at the
top, then thread `gitRoot` everywhere. Removes the now-dead
`isGitRepository` import and adds a private
`resolveGitDirFromRoot(gitRoot)` so the public `resolveGitDir(cwd)`
contract stays untouched for the test suite and external callers.
2. `countUntrackedLines` had a TOCTOU window between `lstat` (which
gates on regular files) and `open(absPath, 'r')` — if the path was
replaced by a symlink in that gap, `open` would silently follow it
and read the target. Open with `O_RDONLY | O_NOFOLLOW` (falling
back to `O_RDONLY` on platforms that don't expose the flag, e.g.
Windows) so the open rejects with `ELOOP` instead. Also unified the
five "couldn't read this file" branches (lstat throws / non-regular
/ open throws / binary sniff / mid-read failure) to all return
`{isBinary:true, added:0}` — the row appears in the listing as an
opaque `~ (binary, new)` marker rather than masquerading as an
empty text file with `+0 (new)`.
44 gitDiff tests pass unchanged.
* docs(cli): clarify row-order contract; simplify DiffRow key
Two non-blocking suggestions from qqqys's CR on PR #3491:
- `buildDiffRenderModel`: expand the JSDoc to call out the implicit
row-ordering contract that both renderers depend on (tracked entries
first in numstat order, then untracked appended in ls-files order).
Future replacements of the underlying Map need to preserve this
sequence.
- `DiffStatsDisplay`: drop the `${i}-${filename}` React key in favor of
bare `filename`. Filenames are unique within a single
`DiffRenderModel` (perFileStats is a Map keyed by filename), so the
index prefix added no information.
* feat(cli): add (deleted) marker for files removed in the worktree
Symmetrical to the (new) marker for untracked files: tracked files that
were removed from the worktree relative to HEAD now render with a
(deleted) suffix (or (binary, deleted) for binary deletes), so users
can tell a delete apart from a heavy edit.
Implementation:
- core: `fetchGitDiff` now runs `git diff HEAD --name-status -z` in
parallel with the existing numstat call. `parseDeletedFromNameStatus`
extracts the set of D-status paths (skipping R/C rename and copy
pairs, both halves of which still exist on disk under one name or
the other). Each `perFileStats` entry whose key is in that set gets
`isDeleted: true`. Numstat alone could not distinguish a delete
(`0\t10\tpath`) from a heavy edit; the name-status pass disambiguates.
- cli: `DiffRenderRow` carries `isDeleted: boolean`; both the plain-text
renderer and the Ink component append the new suffix in
`theme.text.secondary` (dim).
- i18n: new `(deleted)` and `(binary, deleted)` keys in en/zh/zh-TW.
Tests:
- Unit: `parseDeletedFromNameStatus` covers D-only extraction, R/C pair
skipping, NUL-safe paths (tabs / non-ASCII), and empty input.
- Integration: real repo deletes a tracked text + a tracked binary plus
edits another file; asserts the deleted entries get `isDeleted: true`
but the heavy edit does not. Second test verifies neither half of a
`git mv` rename gets flagged as deleted.
- CLI / component: `(deleted)` and `(binary, deleted)` rendering
variants with column alignment intact.
* fix(core): pin --no-ext-diff on every git diff invocation
Plain `git diff HEAD` honors `GIT_EXTERNAL_DIFF` and configured
`diff.<name>.command` drivers, so the exported `fetchGitDiffHunks`
utility could execute arbitrary commands when invoked inside a
worktree whose user-global or repo-local config registers an
external driver.
Add `--no-ext-diff` to every `git diff` call:
- `fetchGitDiffHunks`'s plain `git diff HEAD` — the actual
vulnerability surface.
- `fetchGitDiff`'s `--shortstat`, `--numstat`, `--name-status`
variants — defense-in-depth. Empirically these stats modes
already bypass external drivers in current git, but git's
behavior here has shifted between versions before, and
pinning the flag everywhere is a zero-cost hardening that
keeps the policy uniform across every `git diff` we run.
Regression test plants `GIT_EXTERNAL_DIFF=evil.sh` (a driver that
writes a sentinel file as its side effect) before calling
`fetchGitDiffHunks`, then asserts the sentinel never appears —
confirming `--no-ext-diff` actually stops git from spawning the
driver.
Closes wenshao critical comment on PR #3491.
* fix(core): lazy O_NOFOLLOW lookup so vitest fs mocks don't blow up
PR #3491 CI was failing across all 9 platform/Node combos with:
Error: [vitest] No "constants" export is defined on the "node:fs" mock.
at gitDiff.ts:70 const UNTRACKED_OPEN_FLAGS =
fsConstants.O_RDONLY | (fsConstants.O_NOFOLLOW ?? 0)
at index.ts:279 export * from './utils/gitDiff.js'
Six unrelated test files (`client.test.ts`, `geminiChat.test.ts`,
`marketplace.test.ts`, `npm.test.ts`, `mcp-client.test.ts`,
`nextSpeakerChecker.test.ts`) `vi.mock('node:fs', ...)` without
returning `constants`, and their transitive import of
`@qwen-code/qwen-code-core` pulls in `gitDiff.ts`, whose
module-load-time `import { constants as fsConstants }` plus the
top-level `UNTRACKED_OPEN_FLAGS` constant tripped vitest's strict
mock proxy.
Two changes:
1. Switch `import { constants }` to `import * as nodeFs from 'node:fs'`.
Strict-mock no longer rejects the import statement itself.
2. Move the flag computation out of a module-load constant into a
memoized `getUntrackedOpenFlags()` called from inside
`countUntrackedLines`. Tests that don't actually invoke
`fetchGitDiff` / `fetchGitDiffHunks` (i.e. all six broken ones)
never reach the property access, so vitest's proxy never trips.
`?? 0` fallback on each constant lookup is preserved so Windows
(no `O_NOFOLLOW`) and the genuine "constants is undefined" mock
path both degrade to plain `O_RDONLY` without throwing.
Locally re-ran all six previously-failing files (199 tests) — all
green. Existing 51 gitDiff tests unchanged.
* fix(core): make resolveGitDir worktree assertion platform-agnostic
Windows CI was failing only on:
resolveGitDir > follows the gitdir pointer for linked worktrees
AssertionError: expected 'C:/Users/runneradmin/.../main/.git/worktrees/wt'
to contain '.git\worktrees'
Git writes the linked-worktree pointer in the `.git` *file* using
forward slashes — `gitdir: C:/Users/.../main/.git/worktrees/wt` —
even on Windows. `resolveGitDir` surfaces that string verbatim
(intentional, since fs APIs on Windows accept both separators). But
the assertion used `path.join('.git', 'worktrees')`, which is
`'.git\\worktrees'` on Windows, so the substring-contains check
failed despite the value being correct.
Switch to a regex that matches either separator: `/[/\\]\.git[/\\]worktrees[/\\]/`.
Now the assertion holds on POSIX (where path.join uses `/` anyway)
and Windows (where git's value uses `/` but the host uses `\`).
6285/6289 Windows tests already passed before this; only this one
assertion was platform-dependent.
* fix(core): C-unquote diff path headers and fix untracked-only fast path
Two Critical findings + one suggestion from wenshao on PR #3491:
1. (line 615) `extractFilePath` only accepted unquoted `+++ b/...` /
`--- a/...` headers. Git wraps a path in `"..."` and applies
C-style escaping (`\t`, `\n`, `\r`, `\"`, `\\`, plus octal `\NNN`
for non-ASCII bytes) whenever the raw path contains a character
that breaks space-delimited parsing. `core.quotepath=false` only
disables the octal form for non-ASCII bytes — control chars and
quotes are still escaped — so `fetchGitDiffHunks` silently dropped
hunks for any tracked file whose name contained a tab, newline,
or quote.
Add `unquoteCStylePath()`: detects the surrounding quotes, decodes
`\t`/`\n`/`\r`/`\"`/`\\` plus octal `\NNN` to raw bytes, then
UTF-8-decodes the byte sequence so multi-byte octal sequences like
`\346\226\207` (= `文`) round-trip correctly. `extractFilePath`
pipes every candidate through `stripTab` -> `unquoteCStylePath`
before checking the `a/` / `b/` prefix.
Two unit tests cover the tab and octal cases; one integration test
creates a real `tab\there.txt` tracked file, modifies it, and asserts
`fetchGitDiffHunks` keys hunks under the real name. The integration
test no-ops on filesystems that reject tab-in-name (NTFS).
2. (line 146) The >MAX_FILES_FOR_DETAILS fast path was guarded by
`quickStats &&`, which short-circuited to false when shortstat
returned an empty string. A workspace with 0 tracked changes plus
501 untracked files therefore slipped past the guardrail and ran
the slow path, line-counting only the first MAX_FILES untracked
files — header reported `filesCount: 501` but `linesAdded` missed
the other 451.
Treat empty/null/unparseable shortstat as `EMPTY_STATS` and apply
the threshold on `tracked + untracked` uniformly. Integration test
plants 501 untracked files + 0 tracked and asserts the result has
`filesCount: 501` with an empty perFileStats Map (summary-only).
3. (line 263) `fetchGitDiffHunks` reads the full `git diff HEAD`
stdout before parser caps apply. Documented in the JSDoc as a
known limitation: streaming the parser to terminate git early at
MAX_FILES is a reasonable follow-up but a non-trivial refactor
(spawn + incremental parse + UTF-8 boundary handling) and out of
scope for this PR. The existing `runGit` 64 MB maxBuffer keeps
pathological cases from runaway-allocating.
55 gitDiff tests pass (51 + 4 new).
* fix(core): also pass --no-textconv to block .gitattributes textconv drivers
Builds on the earlier `--no-ext-diff` hardening. wenshao pointed out
that `--no-ext-diff` covers `GIT_EXTERNAL_DIFF` and
`diff.<name>.command`, but it does NOT block textconv filters
registered via `.gitattributes` + `diff.<name>.textconv` — those run
on a separate code path inside `git diff`.
Verified locally:
git config diff.evil.textconv /tmp/evil.sh
echo '*.pdf diff=evil' > .gitattributes
# ... commit + modify doc.pdf ...
git diff HEAD --no-ext-diff -> /tmp/evil.sh fires
git diff HEAD --no-ext-diff --no-textconv -> driver does NOT fire
Add `--no-textconv` to all four `git diff` invocations
(shortstat / numstat / name-status / plain hunks). As with
`--no-ext-diff`, only the plain-diff call (`fetchGitDiffHunks`) is
known to invoke textconv in current git, but pinning both flags
uniformly is defense-in-depth and keeps the policy declarative.
Regression test plants a real textconv driver in a worktree's
`.git/config` + `.gitattributes` and asserts the driver's sentinel
file is NOT written when `fetchGitDiffHunks` runs. Without the new
flag the test fails with the sentinel present.
* fix(diff): close untracked-line undercount and ANSI injection in /diff output
Two Critical issues from PR #3491 review:
1. fetchGitDiff slow path only line-counted the first MAX_FILES (50)
untracked paths via `untrackedPaths.slice(0, MAX_FILES)`. With 51-500
untracked files in a clean tree the header reported the full file count
but only ~50 files' worth of additions, materially under-reporting the
total. Now read every untracked path that survived the
>MAX_FILES_FOR_DETAILS fast-path filter, with concurrency bounded to
MAX_FILES so peak heap stays around MAX_FILES *
UNTRACKED_READ_CHUNK_BYTES (~3.2 MB) regardless of input size.
2. renderDiffModelText interpolated raw filenames into the non-interactive
/ ACP text path. The interactive history is sanitized via
escapeAnsiCtrlCodes(item) inside HistoryItemDisplay, but the text path
streams to stdout / logs / transports with no equivalent hop, so a
tracked or untracked filename containing \x1b[2J etc. could inject
color resets, cursor moves, or full screen clears into CI logs and
downstream terminals. Pipe r.filename through escapeAnsiCtrlCodes at
the rendering boundary on every row variant (binary, untracked,
deleted, modified).
Tests:
- gitDiff.test.ts: regression that asserts every one of MAX_FILES + 10
untracked one-line files contributes to stats.linesAdded (would be 50
pre-fix vs 60 actual).
- diffCommand.test.ts: two new specs covering ANSI escapes in
modified-file rows and in binary / untracked / deleted suffix rows.
Verifies raw \x1b never reaches stdout while suffix markers ((binary),
(new), (deleted)) still render.
* fix(diff): harden quoted-path decoder and filename sanitizer
- unquoteCStylePath now walks Unicode code points so non-BMP characters
(e.g. emoji) inside a forced-quoted path no longer get split into lone
surrogates and decoded as replacement characters.
- Add explicit C-escape mappings for \a, \b, \f, \v so paths using those
control bytes decode to BEL/BS/FF/VT instead of dropping the backslash.
- Replace escapeAnsiCtrlCodes(filename) at the /diff text-rendering
boundary with a sanitizer that also escapes standalone C0/C1 control
bytes plus DEL, closing newline / CR / BS / BEL injection vectors that
ansi-regex does not match.
6556adcdba