fix(write-through): honor the page's recorded source_path instead of re-deriving <slug>.md (#3782)

Wave-assembled from PR #3782 by @JonMcCutchen. Rider-check: verified the
non-default pageRoot join (repoPath/.sources/<sourceId>) matches how
pages.source_path is recorded (source-root-relative via importFile's
relative(dir, filePath)); no mismatch, no change needed.

Co-Authored-By: Jon McCutchen <jmmccutchen1@gmail.com>

src/core/write-through.ts

@@ -22,7 +22,7 @@
  */
 
 import { existsSync, statSync, mkdirSync, writeFileSync, renameSync, unlinkSync, readdirSync } from 'fs';
-import { basename, dirname, join } from 'path';
+import { basename, dirname, isAbsolute, join, relative, resolve } from 'path';
 import { randomBytes } from 'crypto';
 import type { BrainEngine } from './engine.ts';
 import { serializePageToMarkdown, resolvePageFilePath } from './markdown.ts';
@@ -73,6 +73,58 @@ export interface WritePageThroughOpts {
   logger?: WriteThroughLogger;
 }
 
+/**
+ * Vet a `pages.source_path` before it is trusted as a write target.
+ *
+ * The column is populated from the scanner's relative path at import time, so
+ * the normal value is a clean repo-relative `.md` path. This rejects the shapes
+ * that would make `join(root, value)` unsafe or nonsensical — absolute paths,
+ * `..` traversal, NUL bytes, non-markdown artifacts, and blanks. Containment is
+ * still re-checked by `isWriteTargetContained` after the join; this is the
+ * cheap structural filter in front of it.
+ */
+function sanitizeRecordedSourcePath(raw: string | null | undefined): string | null {
+  if (!raw) return null;
+  const value = raw.trim();
+  if (!value || value.includes('\0')) return null;
+  if (!value.toLowerCase().endsWith('.md')) return null;
+  if (isAbsolute(value)) return null;
+  if (value.split(/[\\/]/).some((segment) => segment === '..')) return null;
+  return value;
+}
+
+/**
+ * Recover a page-root-relative write target from a `file://` `source_uri`.
+ *
+ * `gbrain capture --file` records the absolute input path as `source_uri` but
+ * does NOT set `source_path` (that column is the file-scanner's). So a file the
+ * user authored INSIDE the brain repo and then captured has no file of record,
+ * and the slug-derived fallback would mint a twin beside the very file that was
+ * just read. When the recorded URI points at a path under `pageRoot`, that path
+ * IS the file of record — use it.
+ *
+ * Returns null for anything not a contained `.md` file so the caller falls back
+ * to the slug path.
+ */
+function recordedPathFromFileUri(sourceUri: string | null | undefined, pageRoot: string): string | null {
+  if (!sourceUri || !sourceUri.startsWith('file://')) return null;
+  let abs = sourceUri.slice('file://'.length);
+  if (!abs) return null;
+  // Percent-decode only when it looks encoded — the CLI stores raw paths, so a
+  // literal '%' in a filename must not be mangled.
+  if (/%[0-9A-Fa-f]{2}/.test(abs)) {
+    try {
+      abs = decodeURIComponent(abs);
+    } catch {
+      return null;
+    }
+  }
+  if (abs.includes('\0') || !abs.toLowerCase().endsWith('.md')) return null;
+  const rel = relative(resolve(pageRoot), resolve(abs));
+  if (!rel || isAbsolute(rel) || rel.split(/[\\/]/).some((segment) => segment === '..')) return null;
+  return rel;
+}
+
 /**
  * Render the DB row for `slug` to markdown and atomically write it under
  * `sync.repo_path`. Never throws — failures are reported via the result's
@@ -104,11 +156,37 @@ export async function writePageThrough(
       [sourceId],
     );
     const sourceLocalPath = srcRows[0]?.local_path ?? null;
+
+    // Prefer the page's recorded `source_path` — the ACTUAL file this row was
+    // imported from — over a slug-derived name. Deriving `<slug>.md` mints a
+    // SECOND file beside the original whenever the on-disk name isn't the slug,
+    // which is the common case for a human-authored vault: `Library/People/
+    // Steve Jobs.md` has slug `library/people/steve-jobs`, so a later put_page
+    // dropped a lowercase `steve-jobs.md` twin next to it. Two artifacts, one
+    // row, and the newer content in whichever the caller didn't expect.
+    //
+    // It also desyncs `gbrain sync`, which keys delete-reconcile on
+    // `source_path` (see collectMissingSourcePaths): the twin is invisible to
+    // reconcile, so deleting the ORIGINAL file deletes the page even though a
+    // file for it is still on disk.
+    //
+    // A NULL `source_path` means the page was born via put/capture and has no
+    // file of record yet — the slug-derived path stays correct for those.
+    const pathRows = await engine.executeRaw<{ source_path: string | null; source_uri: string | null }>(
+      `SELECT source_path, source_uri FROM pages WHERE source_id = $1 AND slug = $2 AND deleted_at IS NULL LIMIT 1`,
+      [sourceId, slug],
+    );
+    const recordedPath = sanitizeRecordedSourcePath(pathRows[0]?.source_path);
+    const recordedUri = pathRows[0]?.source_uri ?? null;
+
     if (sourceLocalPath) {
       if (!existsSync(sourceLocalPath) || !statSync(sourceLocalPath).isDirectory()) {
         return { written: false, skipped: 'repo_not_found' };
       }
-      filePath = join(sourceLocalPath, `${slug}.md`);
+      filePath = join(
+        sourceLocalPath,
+        recordedPath ?? recordedPathFromFileUri(recordedUri, sourceLocalPath) ?? `${slug}.md`,
+      );
       writeRoot = sourceLocalPath;
     } else {
       const repoPath = await engine.getConfig('sync.repo_path');
@@ -127,7 +205,9 @@ export async function writePageThrough(
       if (collide.length > 0) {
         return { written: false, skipped: 'source_repo_belongs_to_other_source' };
       }
-      filePath = resolvePageFilePath(repoPath, slug, sourceId);
+      const pageRoot = sourceId === 'default' ? repoPath : join(repoPath, '.sources', sourceId);
+      const knownPath = recordedPath ?? recordedPathFromFileUri(recordedUri, pageRoot);
+      filePath = knownPath ? join(pageRoot, knownPath) : resolvePageFilePath(repoPath, slug, sourceId);
       writeRoot = repoPath;
     }
 

test/write-through.test.ts

@@ -120,6 +120,117 @@ describe('writePageThrough', () => {
     expect(res).toEqual({ written: false, skipped: 'page_not_found_after_write' });
   });
 
+  test('[REGRESSION twin] honors the recorded source_path instead of minting a slug-named twin', async () => {
+    await engine.setConfig('sync.repo_path', brainDir);
+    // A human-authored vault file whose on-disk name is NOT its slug — the
+    // normal case for Obsidian (Title Case, spaces) once slugified.
+    const slug = 'library/people/steve-jobs';
+    const authored = 'Library/People/Steve Jobs.md';
+    await importFromContent(engine, slug, `---\ntitle: Steve Jobs\ntype: person\n---\n\n# Body\n`, {
+      noEmbed: true,
+      sourceId: 'default',
+      sourcePath: authored,
+    });
+    fs.mkdirSync(path.join(brainDir, 'Library', 'People'), { recursive: true });
+    fs.writeFileSync(path.join(brainDir, authored), 'stale\n');
+
+    const res = await writePageThrough(engine, slug, { sourceId: 'default' });
+
+    expect(res.written).toBe(true);
+    expect(res.path).toBe(path.join(brainDir, authored));
+    // The authored file was UPDATED in place...
+    expect(fs.readFileSync(path.join(brainDir, authored), 'utf8')).not.toBe('stale\n');
+    // ...and no slug-derived twin appeared anywhere in the tree.
+    const twin = resolvePageFilePath(brainDir, slug, 'default');
+    expect(fs.existsSync(twin)).toBe(false);
+    expect(walkFiles(brainDir).sort()).toEqual([path.join(brainDir, authored)]);
+  });
+
+  test('[REGRESSION twin] null source_path still falls back to the slug-derived path', async () => {
+    await engine.setConfig('sync.repo_path', brainDir);
+    const slug = 'inbox/2026-01-01-abc123';
+    // Born via put/capture: no file of record, so source_path stays NULL.
+    await importFromContent(engine, slug, `---\ntitle: T\ntype: note\n---\n\n# Body\n`, {
+      noEmbed: true,
+      sourceId: 'default',
+    });
+
+    const res = await writePageThrough(engine, slug, { sourceId: 'default' });
+
+    expect(res.written).toBe(true);
+    expect(res.path).toBe(resolvePageFilePath(brainDir, slug, 'default'));
+  });
+
+  test('[REGRESSION twin] falls back to a contained file:// source_uri when source_path is null (capture --file of a vault file)', async () => {
+    await engine.setConfig('sync.repo_path', brainDir);
+    const slug = 'library/companies/postiz';
+    const authored = 'Library/Companies/Postiz.md';
+    // `capture --file` records the absolute path as source_uri and leaves
+    // source_path NULL — the exact shape that used to mint a twin.
+    await importFromContent(engine, slug, `---\ntitle: Postiz\ntype: company\n---\n\n# Body\n`, {
+      noEmbed: true,
+      sourceId: 'default',
+    });
+    await engine.executeRaw(`UPDATE pages SET source_uri = $1 WHERE slug = $2`, [
+      `file://${path.join(brainDir, authored)}`,
+      slug,
+    ]);
+    fs.mkdirSync(path.join(brainDir, 'Library', 'Companies'), { recursive: true });
+    fs.writeFileSync(path.join(brainDir, authored), 'stale\n');
+
+    const res = await writePageThrough(engine, slug, { sourceId: 'default' });
+
+    expect(res.written).toBe(true);
+    expect(res.path).toBe(path.join(brainDir, authored));
+    // NB: no `existsSync(slug path)` assertion here — this slug differs from the
+    // authored name only by CASE, so a case-insensitive FS (macOS/Windows) folds
+    // the two and existsSync would report a twin that isn't there. walkFiles
+    // enumerates real directory entries, so it is case-truthful on every FS.
+    expect(walkFiles(brainDir).sort()).toEqual([path.join(brainDir, authored)]);
+  });
+
+  test('[REGRESSION twin] a file:// source_uri OUTSIDE the repo is ignored', async () => {
+    await engine.setConfig('sync.repo_path', brainDir);
+    const slug = 'inbox/from-elsewhere';
+    await importFromContent(engine, slug, `---\ntitle: T\ntype: note\n---\n\n# Body\n`, {
+      noEmbed: true,
+      sourceId: 'default',
+    });
+    // A file captured from outside the brain repo has no file of record inside it.
+    await engine.executeRaw(`UPDATE pages SET source_uri = $1 WHERE slug = $2`, [
+      `file://${path.join(tmpRoot, 'outside', 'Notes.md')}`,
+      slug,
+    ]);
+
+    const res = await writePageThrough(engine, slug, { sourceId: 'default' });
+
+    expect(res.written).toBe(true);
+    expect(res.path).toBe(resolvePageFilePath(brainDir, slug, 'default'));
+    expect(fs.existsSync(path.join(tmpRoot, 'outside', 'Notes.md'))).toBe(false);
+  });
+
+  test('[REGRESSION twin] a traversing source_path is ignored, not joined', async () => {
+    await engine.setConfig('sync.repo_path', brainDir);
+    const slug = 'wiki/ideas/hostile-1';
+    await importFromContent(engine, slug, `---\ntitle: T\ntype: note\n---\n\n# Body\n`, {
+      noEmbed: true,
+      sourceId: 'default',
+      sourcePath: `${slug}.md`,
+    });
+    // Simulate a hostile / corrupted row after the fact.
+    await engine.executeRaw(`UPDATE pages SET source_path = $1 WHERE slug = $2`, [
+      '../../escaped.md',
+      slug,
+    ]);
+
+    const res = await writePageThrough(engine, slug, { sourceId: 'default' });
+
+    // Falls back to the slug path rather than escaping the write root.
+    expect(res.written).toBe(true);
+    expect(res.path).toBe(resolvePageFilePath(brainDir, slug, 'default'));
+    expect(fs.existsSync(path.join(tmpRoot, '..', 'escaped.md'))).toBe(false);
+  });
+
   test('[REGRESSION #2018] default page (null local_path) in a multi-source brain → skipped, no leak into a sibling source repo', async () => {
     // A sibling federated source with its OWN working tree.
     const siblingDir = path.join(tmpRoot, 'housefax');