From eb336e9f777352d0e5682eec244d7ccdbda7a7d5 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sat, 5 Sep 2026 00:50:35 -0400 Subject: [PATCH] fix(#4081): decode git C-quoted paths in codebase-drift --name-status parser (#4307) * test(#4081): failing-first regression for quotepath C-quoted paths in codebase-drift * fix(#4081): decode git C-quoted paths in codebase-drift --name-status parse * test(#4081): set drift_threshold 1 so decoded-path test triggers action_required * chore(#4081): add changeset fragment * chore(#4081): fix changeset fragment formatting * chore(#4081): backfill PR number in changeset --------- Co-authored-by: sim --- .changeset/serene-yaks-wake.md | 5 ++ src/verify.cts | 16 ++++- src/worktree-safety.cts | 8 +++ tests/drift-detection.test.cjs | 111 +++++++++++++++++++++++++++++++++ 4 files changed, 139 insertions(+), 1 deletion(-) create mode 100644 .changeset/serene-yaks-wake.md diff --git a/.changeset/serene-yaks-wake.md b/.changeset/serene-yaks-wake.md new file mode 100644 index 000000000..48f25d92b --- /dev/null +++ b/.changeset/serene-yaks-wake.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 4307 +--- +**`verify codebase-drift` no longer misclassifies non-ASCII paths as unmapped drift** — with git's default `core.quotepath`, C-quoted diff paths garbled `affected_paths`/`elements` and flagged documented directories as `new_dir`. Paths are now decoded before classification. (#4081) diff --git a/src/verify.cts b/src/verify.cts index 556cbdf61..05e9d338c 100644 --- a/src/verify.cts +++ b/src/verify.cts @@ -28,6 +28,11 @@ const { findOrphanSummaries, findUnsummarizedPlans } = coreUtilsMod; // eslint-disable-next-line @typescript-eslint/no-require-imports -- planning-scope.cjs is an export= CommonJS module import planningScopeMod = require('./planning-scope.cjs'); const { SCOPE } = planningScopeMod; +// eslint-disable-next-line @typescript-eslint/no-require-imports -- worktree-safety.cjs is an export= CommonJS module +import worktreeSafetyMod = require('./worktree-safety.cjs'); +// Single owner of git C-quoted-path decoding (see #4081 note at the +// codebase-drift --name-status parse loop). +const { decodeGitQuotedPath } = worktreeSafetyMod; import { execGit, platformReadSync as safeReadFile } from './shell-command-projection.cjs'; import { validatePath } from './security.cjs'; import { formatGsdSlash, resolveRuntime } from './runtime-slash.cjs'; @@ -2309,7 +2314,16 @@ function cmdVerifyCodebaseDrift(cwd: string, raw: boolean): void { const m = line.match(/^([A-Z])\d*\t(.+?)(?:\t(.+))?$/); if (!m) continue; const status = m[1]; - const file = m[3] || m[2]; + // execGit sets no core.quotepath config, so git's default `true` applies: + // any path containing non-ASCII bytes (or `"`, `\`, control bytes) is + // C-quoted — `"docs/\350\256\276…/overview.md"`. Capturing that verbatim + // garbles affected_paths/elements and makes isPathMapped compare the + // quoted prefix (`"docs`) against STRUCTURE.md, misclassifying DOCUMENTED + // directories as new_dir (#4081). Decode with the single owner of the + // git C-quote seam (worktree-safety.cjs); a non-quoted value — the plain + // ASCII common case — passes through untouched. Both capture groups are + // decoded: R/C lines carry old AND new paths, either may be quoted. + const file = decodeGitQuotedPath(m[3] || m[2]); if (status === 'A' || status === 'R' || status === 'C') added.push(file); else if (status === 'M') modified.push(file); else if (status === 'D') deleted.push(file); diff --git a/src/worktree-safety.cts b/src/worktree-safety.cts index 60b75eeb9..d0b23ecb6 100644 --- a/src/worktree-safety.cts +++ b/src/worktree-safety.cts @@ -674,6 +674,11 @@ const SUMMARY_ARTIFACT_SUFFIX = 'SUMMARY.md'; * (a trailing lone backslash, or an octal escape with fewer than three * digits) never throws — it degrades to treating the character literally, so * a single bad path can never take down the whole cleanup wave. + * + * Exported (#4081) so the codebase-drift gate's `--name-status` parser can + * decode the identical C-quoted form before classifying paths — the gate's + * `execGit` call sets no `core.quotepath` config, so it receives the same + * quoting and must decode it with the same single owner of this seam. */ function decodeGitQuotedPath(raw: string): string { if (raw.length < 2 || !raw.startsWith('"') || !raw.endsWith('"')) return raw; @@ -2303,6 +2308,9 @@ function pruneOrphanedWorktrees(repoRoot: string, deps: WorktreeDeps & { writeEr } export = { + // Re-exported for the codebase-drift gate's --name-status parser (#4081): + // single owner of git C-quoted-path decoding. + decodeGitQuotedPath, resolveWorktreeContext, resolveWorktreeLinkage, parseWorktreePorcelain, diff --git a/tests/drift-detection.test.cjs b/tests/drift-detection.test.cjs index d8b973990..2fe07477f 100644 --- a/tests/drift-detection.test.cjs +++ b/tests/drift-detection.test.cjs @@ -712,6 +712,117 @@ describe('verify codebase-drift CLI', () => { assert.ok(data.elements.length >= 3); }); + // ─── Regression #4081 — core.quotepath C-quoted non-ASCII paths ────────── + // + // With git's default core.quotepath=true, `diff --name-status` C-quotes any + // path containing non-ASCII bytes: `docs/设计说明/overview.md` arrives as the + // literal `"docs/\350\256\276…/overview.md"`. The gate's line parser used to + // capture that quoted string verbatim, so (1) `isPathMapped` compared the + // quoted prefix `"docs` against STRUCTURE.md and misclassified DOCUMENTED + // directories as new_dir, and (2) the garbled string flowed into + // elements[].path / affected_paths. Paths must be decoded before use. + + test('non-ASCII path under documented dir is not new_dir and paths decode (#4081)', () => { + const structure = path.join(tmp, '.planning', 'codebase', 'STRUCTURE.md'); + fs.writeFileSync(structure, '# Codebase Structure\n\n- `docs/`\n'); + writeMappedCommit(structure, git(tmp, 'rev-parse', 'HEAD'), '2026-09-04'); + git(tmp, 'add', '-A'); + git(tmp, 'commit', '-m', 'map codebase'); + + // Non-ASCII directory + file name under the documented `docs/` prefix. + const dir = path.join(tmp, 'docs', '设计说明'); + fs.mkdirSync(dir, { recursive: true }); + fs.writeFileSync(path.join(dir, 'overview.md'), '# overview\n'); + git(tmp, 'add', '-A'); + git(tmp, 'commit', '-m', 'add non-ascii doc'); + + const r = runGsdTools(['verify', 'codebase-drift'], tmp); + assert.strictEqual(r.success, true, r.error); + const data = JSON.parse(r.output); + // The file lives under a DOCUMENTED directory — it is mapped, not drift: + // no element may exist for it, under any path spelling. + const garbled = data.elements.filter((el) => String(el.path).includes('docs')); + assert.strictEqual( + garbled.length, 0, + `docs/ file must classify as mapped (no element); got ${JSON.stringify(data.elements)}`, + ); + // And nothing anywhere in the output may carry git's C-quoting artifacts + // (a literal leading quote or backslash-octal escapes). + const serialized = JSON.stringify([data.affected_paths, data.elements]); + assert.ok(!/\\\\3[0-9]{2}/.test(serialized) && !serialized.includes('\\"docs'), + `garbled C-quoted path leaked into output: ${serialized}`); + }); + + test('elements and affected_paths contain decoded repo-relative paths (#4081)', () => { + // Threshold 1 so a single drift element flips action_required — the + // assertion below depends on the gate triggering, not staying latent + // below the default threshold of 3. + fs.writeFileSync( + path.join(tmp, '.planning', 'config.json'), + JSON.stringify({ workflow: { drift_threshold: 1 } }, null, 2), + ); + const structure = path.join(tmp, '.planning', 'codebase', 'STRUCTURE.md'); + fs.writeFileSync(structure, '# Codebase Structure\n\n- `src/`\n'); + writeMappedCommit(structure, git(tmp, 'rev-parse', 'HEAD'), '2026-09-04'); + git(tmp, 'add', '-A'); + git(tmp, 'commit', '-m', 'map codebase'); + + // Undocumented non-ASCII top-level dir: legitimately drift, but the + // reported path must be the real repo-relative UTF-8 path — git's + // C-quoted form must be decoded, never passed through. + const dir = path.join(tmp, '设计资料'); + fs.mkdirSync(dir, { recursive: true }); + fs.writeFileSync(path.join(dir, '文件.md'), '# doc\n'); + git(tmp, 'add', '-A'); + git(tmp, 'commit', '-m', 'add non-ascii dir'); + + const r = runGsdTools(['verify', 'codebase-drift'], tmp); + assert.strictEqual(r.success, true, r.error); + const data = JSON.parse(r.output); + assert.strictEqual(data.action_required, true, 'undocumented dir must stay drift'); + const paths = [ + ...data.elements.map((el) => el.path), + ...(data.affected_paths || []), + ]; + assert.ok(paths.length > 0, 'expected at least one drift element'); + for (const p of paths) { + assert.ok(!p.startsWith('"'), + `path must be decoded, not C-quoted: ${JSON.stringify(p)}`); + assert.ok(!/\\[0-9]{3}/.test(p), + `path must not contain octal escapes: ${JSON.stringify(p)}`); + } + assert.ok(paths.includes('设计资料/文件.md'), + `expected the real UTF-8 path in output; got ${JSON.stringify(paths)}`); + }); + + test('rename (R100) with non-ASCII target parses and decodes both fields (#4081)', () => { + const structure = path.join(tmp, '.planning', 'codebase', 'STRUCTURE.md'); + fs.writeFileSync(structure, '# Codebase Structure\n\n- `docs/`\n'); + const seed = path.join(tmp, 'docs', 'old.md'); + fs.mkdirSync(path.dirname(seed), { recursive: true }); + fs.writeFileSync(seed, '# seed\n'); + git(tmp, 'add', '-A'); + git(tmp, 'commit', '-m', 'seed ascii file'); + writeMappedCommit(structure, git(tmp, 'rev-parse', 'HEAD'), '2026-09-04'); + git(tmp, 'add', '-A'); + git(tmp, 'commit', '-m', 'map codebase'); + + // Rename into a non-ASCII name — diff emits R100\t"docs/\350…" (quoted + // second field). The new path must decode and classify as mapped, and no + // quoted path may leak into the output. + const renamed = path.join(tmp, 'docs', '新名称.md'); + fs.renameSync(seed, renamed); + git(tmp, 'add', '-A'); + git(tmp, 'commit', '-m', 'rename to non-ascii'); + + const r = runGsdTools(['verify', 'codebase-drift'], tmp); + assert.strictEqual(r.success, true, r.error); + const data = JSON.parse(r.output); + const leaked = JSON.stringify(data.elements.map((el) => el.path)); + assert.ok(!/\\[0-9]{3}/.test(leaked), + `quoted path leaked from rename entry: ${leaked}`); + }); + test('never exits non-zero when git repo is missing (non-blocking)', () => { const nonGit = createTempProject('gsd-drift-nongit-'); try {