fix(parse): retain metadata-only diff files (#3251)

* fix(parse): retain metadata-only diff files

* Address PR review feedback (#3251)

Keep detect_changes honest on C-quoted, TAB-terminated, and ambiguous
diff --git dests, and fail closed when a header cannot be parsed.

Co-authored-by: Cursor <cursoragent@cursor.com>

* Simplify parseDiffHunks dest-prefix helper (#3251)

Drop the unused a/ prefix arm and the redundant empty-parse unparsed-header check.

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(parse): ignore +++ inside hunks and accept mixed-quoted git headers

Stop treating hunk-body lines as file headers, and parse independently
C-quoted src/dest tokens on diff --git.

Co-authored-by: Cursor <cursoragent@cursor.com>

---------

Co-authored-by: ming <silverchris@foxmail.com>
Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>
Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
auyua9 2026-09-11 01:27:00 +08:00 • committed by GitHub
parent 0edf9ce0ff
commit a4769439dd
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 405 additions and 14 deletions

View file

@ -36,13 +36,12 @@ import { isWalCorruptionError, WAL_RECOVERY_SUGGESTION } from '../../core/lbug/l
// git utilities available if needed
// import { isGitRepo, getCurrentCommit, getGitRoot } from '../../storage/git.js';
import {
parseDiffHunks,
parseDiffHunksResult,
coalesceHunksByPath,
hunksOverlapRange,
findGitRootByDotGit,
getCanonicalRepoRoot,
getGitRoot,
type FileDiff,
} from '../../storage/git.js';
import { realpathSync } from 'fs';
import {
@ -1178,6 +1177,12 @@ export function buildDetectChangesDiffArgs(scope: string, baseRef?: string): str
// not `--default-prefix`, which needs git >= 2.42. `--no-ext-diff` stops a
// configured external diff driver from replacing the unified output we parse.
const args = [
// Before the subcommand: default core.quotePath C-quotes non-ASCII so
// `diff --git` / `+++` tokens no longer match the unquoted `a/` `b/` forms
// the parser also accepts. The parser still decodes quoted tokens; this
// pin keeps production git from emitting them.
'-c',
'core.quotePath=false',
'diff',
'--ignore-cr-at-eol',
'--no-ext-diff',
@ -5606,11 +5611,11 @@ export class LocalBackend {
return { error: `Git diff failed: ${err.message}` };
}
const fileDiffs: FileDiff[] = parseDiffHunks(diffOutput);
const { files: fileDiffs, unparsedGitHeaders } = parseDiffHunksResult(diffOutput);
if (fileDiffs.length === 0) {
// Git printed a diff but none of it parsed: the `+++ b/` headers were not
// where `parseDiffHunks` looks. That is a PARSE failure, not a clean tree,
// Git printed a diff but none of it parsed: no `diff --git` / `+++`
// file header was recognised. That is a PARSE failure, not a clean tree,
// and the clean branch below would report it to the pre-commit gate as
// `risk_level:'none'`, no `partial`, exit 0 — a false all-clear (#2915).
const parseFailed = diffOutput.trim().length > 0;
@ -5643,7 +5648,9 @@ export class LocalBackend {
const changedSymbols = new Map<string, any>();
// Set if a swallowed graph query fails below — surfaces `partial:true` so a
// degraded run cannot report a false-clean `risk_level:'low'` (#2283).
let queryDegraded = false;
// An unparsed `diff --git` is the same class: later hunks must not make
// the gate look complete.
let queryDegraded = unparsedGitHeaders > 0;
// Hunks arrive grouped per path and already in the graph's 0-based line
// space, so every comparison below is base-neutral (#2377).

View file

@ -750,10 +750,189 @@ export interface FileDiff {
hunks: DiffHunk[];
}
/** `parseDiffHunks` plus how many `diff --git` headers could not be decoded. */
export interface DiffHunkParseResult {
files: FileDiff[];
unparsedGitHeaders: number;
}
const DIFF_GIT_PREFIX = 'diff --git ';
/**
* Decode one Git C-quoted token (`"a/\\344\\270\\255.png"`). Returns
* `undefined` when the quotes are unbalanced or a trailing escape is bare.
*/
function unquoteCStyleGitToken(quoted: string): string | undefined {
if (quoted.length < 2 || quoted[0] !== '"' || quoted[quoted.length - 1] !== '"') {
return undefined;
}
// Git C-quotes are byte-oriented: non-ASCII is `\nnn` octal of the UTF-8
// code units, not JS UTF-16 characters.
const bytes: number[] = [];
const pushChar = (ch: string): void => {
const code = ch.charCodeAt(0);
if (code < 0x80) bytes.push(code);
else bytes.push(...new TextEncoder().encode(ch));
};
for (let i = 1; i < quoted.length - 1; i++) {
const ch = quoted[i];
if (ch !== '\\') {
pushChar(ch);
continue;
}
const next = quoted[++i];
if (next === undefined) return undefined;
switch (next) {
case '\\':
case '"':
pushChar(next);
break;
case 'n':
bytes.push(0x0a);
break;
case 't':
bytes.push(0x09);
break;
case 'r':
bytes.push(0x0d);
break;
case 'a':
bytes.push(0x07);
break;
case 'b':
bytes.push(0x08);
break;
case 'v':
bytes.push(0x0b);
break;
case 'f':
bytes.push(0x0c);
break;
default: {
if (next < '0' || next > '7') {
pushChar(next);
break;
}
let oct = next;
while (oct.length < 3 && i + 1 < quoted.length - 1) {
const digit = quoted[i + 1];
if (digit < '0' || digit > '7') break;
oct += digit;
i++;
}
bytes.push(parseInt(oct, 8));
}
}
}
return new TextDecoder('utf-8').decode(Uint8Array.from(bytes));
}
function takeCQuotedToken(
source: string,
start: number,
): { token: string; end: number } | undefined {
if (source[start] !== '"') return undefined;
for (let i = start + 1; i < source.length; i++) {
if (source[i] === '\\') {
i++;
continue;
}
if (source[i] === '"') return { token: source.slice(start, i + 1), end: i + 1 };
}
return undefined;
}
function stripGitDstPrefix(raw: string): string | undefined {
return raw.startsWith('b/') ? raw.slice(2) : undefined;
}
/** Unified-diff paths end at the first TAB (timestamp / empty terminator). */
function stripUnifiedDiffTab(pathWithOptionalTab: string): string {
const tab = pathWithOptionalTab.indexOf('\t');
return tab === -1 ? pathWithOptionalTab : pathWithOptionalTab.slice(0, tab);
}
function decodeGitPathToken(raw: string): string | undefined {
const trimmed = stripUnifiedDiffTab(raw);
if (trimmed.startsWith('"')) return unquoteCStyleGitToken(trimmed);
return trimmed;
}
/**
* Destination path from `diff --git a/<src> b/<dst>`.
*
* Same-path headers recover `name` from `a/${name} b/${name}` so a dest that
* itself contains ` b/` is not split at the last occurrence. C-quoted tokens
* (default `core.quotePath`) are decoded. Renames that the greedy split would
* mis-parse stay a best-effort dest; `rename to` / `+++` correct them.
*/
function takeGitHeaderPathToken(
source: string,
start: number,
): { path: string; end: number } | undefined {
if (start >= source.length) return undefined;
if (source[start] === '"') {
const tok = takeCQuotedToken(source, start);
if (!tok) return undefined;
const path = unquoteCStyleGitToken(tok.token);
if (path === undefined) return undefined;
return { path, end: tok.end };
}
if (source.startsWith('b/', start)) {
return { path: stripUnifiedDiffTab(source.slice(start)), end: source.length };
}
if (source.startsWith('a/', start)) {
for (let i = start + 2; i < source.length; i++) {
if (source.startsWith(' b/', i) || source.startsWith(' "', i)) {
return { path: source.slice(start, i), end: i };
}
}
}
return undefined;
}
function filePathFromGitHeader(line: string): string | undefined {
if (!line.startsWith(DIFF_GIT_PREFIX)) return undefined;
const rest = line.slice(DIFF_GIT_PREFIX.length);
if (rest.startsWith('a/')) {
for (let i = 2; i < rest.length; i++) {
if (!rest.startsWith(' b/', i)) continue;
const nameA = rest.slice(2, i);
const nameB = rest.slice(i + 3);
if (nameA.length > 0 && nameA === nameB) return nameA;
}
}
const src = takeGitHeaderPathToken(rest, 0);
if (!src) return undefined;
let i = src.end;
while (rest[i] === ' ') i++;
const dest = takeGitHeaderPathToken(rest, i);
return dest ? stripGitDstPrefix(dest.path) : undefined;
}
function pathFromPlusPlusPlus(line: string): string | undefined {
if (!line.startsWith('+++ ')) return undefined;
const raw = decodeGitPathToken(line.slice(4));
if (!raw || raw === '/dev/null') return undefined;
return stripGitDstPrefix(raw);
}
function pathFromRenameTo(line: string): string | undefined {
if (!line.startsWith('rename to ')) return undefined;
return decodeGitPathToken(line.slice('rename to '.length));
}
/**
* Parse unified diff output (with -U0) into per-file hunk ranges.
* Extracts the new-file line ranges from @@ hunk headers.
*
* The `diff --git` header is also retained as a file entry. This matters for
* binary, rename-only, and mode-only changes, which have no `+++ b/` header.
* Such entries intentionally have no hunks: callers can count the changed
* path without pretending that a symbol line range was touched.
*
* A pure deletion adds no new lines, and unified diff spells that empty range
* as the line BEFORE it: `@@ -4,2 +3,0 @@` removed old lines 4–5 from between
* new lines 3 and 4 (git emits `+0,0` when the deletion is at the head of the
@ -769,13 +948,52 @@ export interface FileDiff {
* gap — the widening {@link coalesceHunks} is careful never to do.
*/
export function parseDiffHunks(diffOutput: string): FileDiff[] {
return parseDiffHunksResult(diffOutput).files;
}
/**
* Same as {@link parseDiffHunks}, plus a count of `diff --git` lines that
* could not be decoded. `detect_changes` uses the count to fail closed
* (`partial` + `risk_level:'unknown'`) instead of attaching later hunks to a
* previous file.
*/
export function parseDiffHunksResult(diffOutput: string): DiffHunkParseResult {
const files: FileDiff[] = [];
let current: FileDiff | null = null;
let unparsedGitHeaders = 0;
// `+++` after the first `@@` of a file is hunk body (`+` plus source text
// that itself starts `++ …`), not another file header.
let inHunk = false;
for (const line of diffOutput.split('\n')) {
if (line.startsWith('+++ b/')) {
current = { filePath: line.slice(6), hunks: [] };
files.push(current);
if (line.startsWith(DIFF_GIT_PREFIX)) {
// Drop the previous file first: an unparsed header must not leave
// `current` live for a later `@@` / quoted `+++` to steal.
current = null;
inHunk = false;
const filePath = filePathFromGitHeader(line);
if (filePath) {
current = { filePath, hunks: [] };
files.push(current);
} else {
unparsedGitHeaders++;
}
} else if (line.startsWith('rename to ')) {
const filePath = pathFromRenameTo(line);
if (!filePath) continue;
if (current) current.filePath = filePath;
else {
current = { filePath, hunks: [] };
files.push(current);
}
} else if (!inHunk && line.startsWith('+++ ')) {
const filePath = pathFromPlusPlusPlus(line);
if (!filePath) continue;
if (!current || current.filePath !== filePath) {
current = { filePath, hunks: [] };
files.push(current);
}
} else if (line.startsWith('@@') && current) {
inHunk = true;
const match = line.match(/@@ -\d+(?:,\d+)? \+(\d+)(?:,(\d+))? @@/);
if (match) {
const start = parseInt(match[1], 10);
@ -791,7 +1009,7 @@ export function parseDiffHunks(diffOutput: string): FileDiff[] {
}
}
}
return files;
return { files, unparsedGitHeaders };
}
/**

View file

@ -8,8 +8,10 @@ import { parseDiffHunks } from '../../src/storage/git.js';
import { diffArgsFor } from '../helpers/detect-changes-diff-args.js';
import { commitAll, initGitRepo } from '../helpers/temp-git-repo.js';
/** The six flags every scope carries, ahead of its own ref/staging arguments. */
/** Flags every scope carries, ahead of its own ref/staging arguments. */
const GUARD_FLAGS = [
'-c',
'core.quotePath=false',
'diff',
'--ignore-cr-at-eol',
'--no-ext-diff',

View file

@ -361,11 +361,12 @@ describe('#2915 detect_changes hunk scaling', () => {
);
registerRepo(repoDir);
// Non-vacuous: the diff really does parse to two entries for one path.
// A content line that is itself `+++ b/code.py` must not open a second
// FileDiff — the same-path skip is the intended shape (#3251).
const parsed = parseDiffHunks(
execFileSync('git', diffArgsFor('unstaged'), { cwd: repoDir, encoding: 'utf-8' }),
);
expect(parsed.map((fileDiff) => fileDiff.filePath)).toEqual(['code.py', 'code.py']);
expect(parsed.map((fileDiff) => fileDiff.filePath)).toEqual(['code.py']);
const result = await runDetectChanges();

View file

@ -1,5 +1,5 @@
import { describe, it, expect } from 'vitest';
import { parseDiffHunks } from '../../src/storage/git.js';
import { parseDiffHunks, parseDiffHunksResult } from '../../src/storage/git.js';
describe('parseDiffHunks', () => {
it('parses a single file with one hunk', () => {
@ -133,4 +133,167 @@ describe('parseDiffHunks', () => {
expect(result[1].hunks[0]).toEqual({ startLine: 51, endLine: 51 });
expect(result[1].hunks[1]).toEqual({ startLine: 82, endLine: 84 });
});
it('retains binary-only files from the git header', () => {
const diff = [
'diff --git a/assets/logo.png b/assets/logo.png',
'index 1111111..2222222 100644',
'Binary files a/assets/logo.png and b/assets/logo.png differ',
].join('\n');
expect(parseDiffHunks(diff)).toEqual([{ filePath: 'assets/logo.png', hunks: [] }]);
});
it('retains the destination of a rename-only diff', () => {
const diff = [
'diff --git a/src/old.ts b/src/new.ts',
'similarity index 100%',
'rename from src/old.ts',
'rename to src/new.ts',
].join('\n');
expect(parseDiffHunks(diff)).toEqual([{ filePath: 'src/new.ts', hunks: [] }]);
});
it('keeps line ranges for whitespace-only hunks', () => {
const diff = [
'diff --git a/src/format.ts b/src/format.ts',
'--- a/src/format.ts',
'+++ b/src/format.ts',
'@@ -4,2 +4,2 @@ function format() {',
'- return value;',
'+return value;',
].join('\n');
expect(parseDiffHunks(diff)).toEqual([
{ filePath: 'src/format.ts', hunks: [{ startLine: 4, endLine: 5 }] },
]);
});
it('retains a mode-only change from the git header', () => {
const diff = ['diff --git a/script.sh b/script.sh', 'old mode 100644', 'new mode 100755'].join(
'\n',
);
expect(parseDiffHunks(diff)).toEqual([{ filePath: 'script.sh', hunks: [] }]);
});
it('decodes a C-quoted metadata-only header', () => {
const diff = [
'diff --git "a/assets/\\344\\270\\255.png" "b/assets/\\344\\270\\255.png"',
'Binary files differ',
].join('\n');
expect(parseDiffHunks(diff)).toEqual([{ filePath: 'assets/中.png', hunks: [] }]);
});
it('does not attach a later C-quoted hunk to a previous metadata-only file', () => {
const diff = [
'diff --git a/script.sh b/script.sh',
'old mode 100644',
'new mode 100755',
'diff --git "a/src/\\344\\275\\240\\345\\245\\275.ts" "b/src/\\344\\275\\240\\345\\245\\275.ts"',
'--- "a/src/\\344\\275\\240\\345\\245\\275.ts"',
'+++ "b/src/\\344\\275\\240\\345\\245\\275.ts"',
'@@ -1,0 +1,1 @@',
'+export const ok = 1;',
].join('\n');
expect(parseDiffHunks(diff)).toEqual([
{ filePath: 'script.sh', hunks: [] },
{ filePath: 'src/你好.ts', hunks: [{ startLine: 1, endLine: 1 }] },
]);
});
it('strips the unified-diff TAB on +++ so a spaced path is one FileDiff', () => {
const diff = [
'diff --git a/My Documents/file.ts b/My Documents/file.ts',
'--- a/My Documents/file.ts',
'+++ b/My Documents/file.ts\t',
'@@ -1,0 +1,1 @@',
'+x',
].join('\n');
expect(parseDiffHunks(diff)).toEqual([
{ filePath: 'My Documents/file.ts', hunks: [{ startLine: 1, endLine: 1 }] },
]);
});
it('recovers a same-path dest that itself contains " b/"', () => {
const diff = [
'diff --git a/foo b/bar.png b/foo b/bar.png',
'Binary files a/foo b/bar.png and b/foo b/bar.png differ',
].join('\n');
expect(parseDiffHunks(diff)).toEqual([{ filePath: 'foo b/bar.png', hunks: [] }]);
});
it('prefers rename to over a greedy b/ split', () => {
const diff = [
'diff --git a/plain.ts b/foo b/plain.ts',
'similarity index 100%',
'rename from plain.ts',
'rename to foo b/plain.ts',
].join('\n');
expect(parseDiffHunks(diff)).toEqual([{ filePath: 'foo b/plain.ts', hunks: [] }]);
});
it('keeps one FileDiff when a content line repeats +++ b/<same-path>', () => {
const diff = [
'diff --git a/code.py b/code.py',
'--- a/code.py',
'+++ b/code.py',
'@@ -5,0 +5,1 @@',
'+++ b/code.py',
].join('\n');
expect(parseDiffHunks(diff)).toEqual([
{ filePath: 'code.py', hunks: [{ startLine: 5, endLine: 5 }] },
]);
});
it('does not treat a quoted +++ content line as a second file', () => {
const diff = [
'diff --git a/src/foo.ts b/src/foo.ts',
'--- a/src/foo.ts',
'+++ b/src/foo.ts',
'@@ -1,0 +1,1 @@',
'+++ "b/generated.ts"',
].join('\n');
expect(parseDiffHunks(diff)).toEqual([
{ filePath: 'src/foo.ts', hunks: [{ startLine: 1, endLine: 1 }] },
]);
});
it('parses a quoted source and unquoted dest on the same git header', () => {
const diff = [
'diff --git "a/old name.ts" b/new.ts',
'similarity index 100%',
'rename from old name.ts',
'rename to new.ts',
].join('\n');
expect(parseDiffHunksResult(diff)).toEqual({
files: [{ filePath: 'new.ts', hunks: [] }],
unparsedGitHeaders: 0,
});
});
it('parses an unquoted source and quoted dest on the same git header', () => {
const diff = [
'diff --git a/old.ts "b/new name.ts"',
'similarity index 100%',
'rename from old.ts',
'rename to new name.ts',
].join('\n');
expect(parseDiffHunksResult(diff)).toEqual({
files: [{ filePath: 'new name.ts', hunks: [] }],
unparsedGitHeaders: 0,
});
});
it('counts an unparsed git header and does not keep current live', () => {
const diff = [
'diff --git a/script.sh b/script.sh',
'old mode 100644',
'new mode 100755',
'diff --git not-a-valid-header',
'@@ -1,0 +1,1 @@',
'+stolen',
].join('\n');
expect(parseDiffHunksResult(diff)).toEqual({
files: [{ filePath: 'script.sh', hunks: [] }],
unparsedGitHeaders: 1,
});
});
});