fix(sync): unquote git C-style-quoted paths in buildSyncManifest (#3897) (#3899)

Wave-assembled from PR #3899 by @SergeyShol.

Co-Authored-By: Sergey Sholom <sergey.sholom@gmail.com>

src/core/sync.ts

@@ -126,6 +126,54 @@ const CODE_EXTENSIONS = new Set<string>([
  * a real, accepted CLI flag on every command that imports this file. Pinned by
  * `test/cli-flag-validation.test.ts`.
  */
+/**
+ * Undo git's C-style path quoting.
+ *
+ * git quotes any path containing `"`, `\` or a control character, and it does
+ * so unconditionally: `core.quotepath=false` (buildGitInvocation in
+ * commands/sync.ts, #119) only suppresses octal-escaping of NON-ASCII bytes.
+ * A path like `people/Jason "Jay" Strand.md` therefore reaches
+ * buildSyncManifest as `"people/Jason \"Jay\" Strand.md"` — surrounding quotes
+ * included — so it ends `.md"`, fails isMarkdownFilePath(), and is dropped from
+ * the manifest with no error and no counter.
+ *
+ * Octal escapes are decoded as BYTES and utf-8 decoded once at the end: a
+ * single codepoint can span several \NNN escapes, so per-escape decoding
+ * produces mojibake.
+ *
+ * An unquoted path is returned unchanged, so this is a no-op for the common
+ * case.
+ */
+export function unquoteGitPath(raw: string): string {
+  if (raw.length < 2 || !raw.startsWith('"') || !raw.endsWith('"')) return raw;
+  const body = raw.slice(1, -1);
+  const simple: Record<string, number> = {
+    n: 10, t: 9, r: 13, '"': 34, '\\': 92, a: 7, b: 8, f: 12, v: 11,
+  };
+  const bytes: number[] = [];
+  let i = 0;
+  while (i < body.length) {
+    const c = body[i];
+    if (c === '\\' && i + 1 < body.length) {
+      const nxt = body[i + 1];
+      if (nxt in simple) {
+        bytes.push(simple[nxt]);
+        i += 2;
+        continue;
+      }
+      const trio = body.slice(i + 1, i + 4);
+      if (trio.length === 3 && /^[0-7]{3}$/.test(trio)) {
+        bytes.push(parseInt(trio, 8));
+        i += 4;
+        continue;
+      }
+    }
+    for (const b of new TextEncoder().encode(c)) bytes.push(b);
+    i += 1;
+  }
+  return new TextDecoder('utf-8').decode(new Uint8Array(bytes));
+}
+
 export function buildSyncManifest(gitDiffOutput: string): SyncManifest {
   const manifest: SyncManifest = {
     added: [],
@@ -145,19 +193,22 @@ export function buildSyncManifest(gitDiffOutput: string): SyncManifest {
 
     const action = parts[0];
 
+    // Unquote at the single point every path enters the manifest, so the
+    // extension filter and every downstream consumer (slug resolution, file
+    // reads) all see the real name. (#3897)
     if (action === 'A') {
-      manifest.added.push(parts[1]);
+      manifest.added.push(unquoteGitPath(parts[1]));
     } else if (action === 'M' || action === 'T' || action === 'U') {
       // T (typechange) and U (unmerged) both mean "this path exists and its
       // content is not what we imported" — the same remedy as M.
-      manifest.modified.push(parts[1]);
+      manifest.modified.push(unquoteGitPath(parts[1]));
     } else if (action === 'D') {
-      manifest.deleted.push(parts[1]);
+      manifest.deleted.push(unquoteGitPath(parts[1]));
     } else if (action.startsWith('R') || action.startsWith('C')) {
       // Rename/copy: R100\told-path\tnew-path. Copy is unreachable without -C,
       // but if the flags ever change, the destination must still be imported.
-      const oldPath = parts[1];
-      const newPath = parts[2];
+      const oldPath = unquoteGitPath(parts[1]);
+      const newPath = unquoteGitPath(parts[2]);
       if (oldPath && newPath) {
         if (action.startsWith('C')) {
           // A copy leaves the source in place — only the destination is new.

test/sync.test.ts

@@ -1,5 +1,5 @@
 import { describe, test, expect, beforeAll, afterAll, beforeEach, afterEach } from 'bun:test';
-import { buildSyncManifest, isSyncable, pathToSlug, pruneDir, isCodeFilePath } from '../src/core/sync.ts';
+import { buildSyncManifest, isSyncable, pathToSlug, pruneDir, isCodeFilePath, unquoteGitPath } from '../src/core/sync.ts';
 import { buildAutoEmbedArgs, buildGitInvocation } from '../src/commands/sync.ts';
 import { mkdtempSync, writeFileSync, rmSync, mkdirSync } from 'fs';
 import { join } from 'path';
@@ -307,6 +307,100 @@ describe('buildSyncManifest edge cases', () => {
   });
 });
 
+// ────────────────────────────────────────────────────────────────
+// C-style-quoted paths. git quotes any path containing `"`, `\` or a
+// control character, unconditionally — core.quotepath=false (#119) only
+// governs octal-escaping of NON-ASCII bytes. Before unquoteGitPath, such
+// an entry ended `.md"`, failed isSyncable(), and was dropped from the
+// manifest silently: no error, no warning, no counter.
+// ────────────────────────────────────────────────────────────────
+
+describe('unquoteGitPath', () => {
+  test('leaves an unquoted path untouched', () => {
+    expect(unquoteGitPath('people/plain-name.md')).toBe('people/plain-name.md');
+    expect(unquoteGitPath('people/Ольга Петрова.md')).toBe('people/Ольга Петрова.md');
+    expect(unquoteGitPath('')).toBe('');
+  });
+
+  test('strips the wrapping quotes and unescapes embedded ones', () => {
+    expect(unquoteGitPath('"people/Jason \\"Jay\\" Strand.md"'))
+      .toBe('people/Jason "Jay" Strand.md');
+  });
+
+  test('unescapes a literal backslash', () => {
+    expect(unquoteGitPath('"people/a\\\\b.md"')).toBe('people/a\\b.md');
+  });
+
+  test('decodes single-character escapes', () => {
+    expect(unquoteGitPath('"people/a\\tb\\nc.md"')).toBe('people/a\tb\nc.md');
+  });
+
+  test('decodes octal escapes as bytes, utf-8 decoding once at the end', () => {
+    // "ы" is U+044B = 0xD1 0x8B — one codepoint, two \NNN escapes. Decoding
+    // per-escape instead of per-byte yields mojibake here.
+    expect(unquoteGitPath('"people/\\321\\213.md"')).toBe('people/ы.md');
+  });
+
+  test('resulting path passes the extension filter', () => {
+    expect(isSyncable(unquoteGitPath('"people/Jason \\"Jay\\" Strand.md"'))).toBe(true);
+  });
+});
+
+describe('buildSyncManifest — C-style-quoted paths', () => {
+  test('unquotes add, modify and delete entries', () => {
+    const output = [
+      'A\t"people/Alice \\"Ace\\" Example.md"',
+      'M\t"companies/ПАО \\"Ростелеком\\".md"',
+      'D\t"people/Jason \\"Jay\\" Strand.md"',
+    ].join('\n');
+    const manifest = buildSyncManifest(output);
+    expect(manifest.added).toEqual(['people/Alice "Ace" Example.md']);
+    expect(manifest.modified).toEqual(['companies/ПАО "Ростелеком".md']);
+    expect(manifest.deleted).toEqual(['people/Jason "Jay" Strand.md']);
+  });
+
+  test('unquotes both sides of a rename', () => {
+    const output = 'R100\t"people/old \\"nick\\".md"\t"people/new \\"nick\\".md"';
+    const manifest = buildSyncManifest(output);
+    expect(manifest.renamed).toEqual([
+      { from: 'people/old "nick".md', to: 'people/new "nick".md' },
+    ]);
+  });
+
+  test('quoted entries survive the syncable filter', () => {
+    const output = 'M\t"people/Christian \\"Raz\\" Kippelt.md"';
+    const manifest = buildSyncManifest(output);
+    expect(manifest.modified.filter(p => isSyncable(p))).toHaveLength(1);
+  });
+
+  test('real git output for a quoted filename reaches the manifest', () => {
+    const repo = mkdtempSync(join(tmpdir(), 'gbrain-quoted-path-'));
+    try {
+      const name = 'people/Alice "Ace" Example.md';
+      execSync('git init -q .', { cwd: repo });
+      execSync('git config user.email t@t.t && git config user.name t', { cwd: repo, shell: '/bin/bash' });
+      mkdirSync(join(repo, 'people'));
+      writeFileSync(join(repo, name), 'x\n');
+      execSync('git add -A && git commit -q -m one', { cwd: repo, shell: '/bin/bash' });
+      writeFileSync(join(repo, name), 'x\ny\n');
+      execSync('git add -A && git commit -q -m two', { cwd: repo, shell: '/bin/bash' });
+
+      // gbrain's own invocation, quotepath and all.
+      const argv = buildGitInvocation(repo, ['diff', '--name-status', '-M', 'HEAD~1..HEAD']);
+      const out = execSync(`git ${argv.map(a => JSON.stringify(a)).join(' ')}`, { encoding: 'utf-8' });
+
+      // git really does quote it, whatever core.quotepath says.
+      expect(out.trim()).toBe('M\t"people/Alice \\"Ace\\" Example.md"');
+
+      const manifest = buildSyncManifest(out);
+      expect(manifest.modified).toEqual([name]);
+      expect(manifest.modified.filter(p => isSyncable(p))).toHaveLength(1);
+    } finally {
+      rmSync(repo, { recursive: true, force: true });
+    }
+  });
+});
+
 // ────────────────────────────────────────────────────────────────
 // performSync dry-run (v0.17 regression guard for full-sync silent writes)
 // ────────────────────────────────────────────────────────────────