fix(doctor): stop reporting Windows-drive image paths as missing under WSL (#1835) (#3523)

Co-Authored-By: Time Attakc <89218912+time-attack@users.noreply.github.com>

src/commands/doctor-asset-paths.ts

@@ -0,0 +1,108 @@
+/**
+ * #1835 — storage_path resolution for the doctor `image_assets` check.
+ *
+ * `files.storage_path` rows written by a Windows gbrain install carry Windows
+ * drive paths (`D:/foo/img.jpg`, `D:\foo\img.jpg`). On POSIX,
+ * `path.isAbsolute()` is false for those, so the old code joined them onto the
+ * repo root and produced a path that can never exist — a false-positive
+ * "missing from disk, restore from git" WARN under WSL and macOS.
+ *
+ * Policy:
+ *   - win32: drive paths are absolute; stat them as-is.
+ *   - WSL (linux + "microsoft" in /proc/version): translate `D:/x` to
+ *     `<automount root>/d/x` (automount root read from /etc/wsl.conf
+ *     `[automount] root`, default `/mnt`) and stat that.
+ *   - any other POSIX host (macOS, plain Linux): the path is unresolvable on
+ *     this platform — report it as foreign so the caller SKIPS the stat
+ *     instead of inventing a path that will never exist.
+ *
+ * Kept in its own module (not doctor.ts) so the pure tests don't pull the
+ * 7k-line doctor dep graph, and so open PRs rewriting the image_assets block
+ * (e.g. a `resolveImageAssetPath` helper) can adopt it with a one-line call.
+ */
+import { readFileSync } from 'node:fs';
+import { join, posix, win32 } from 'node:path';
+
+const WINDOWS_DRIVE_RE = /^([A-Za-z]):[\\/](.*)$/;
+
+export interface AssetPathResolution {
+  /** Absolute path to stat, or null when the path is unresolvable here. */
+  abs: string | null;
+  /** True when storage_path is a Windows drive path this host cannot stat. */
+  foreign: boolean;
+}
+
+/**
+ * Resolve a files.storage_path to a stat-able absolute path.
+ * `opts.platform` / `opts.wslMountRoot` exist for tests; production callers
+ * pass neither (process.platform + detected WSL automount root).
+ * `wslMountRoot: null` means "not under WSL".
+ */
+export function resolveAssetPath(
+  storagePath: string,
+  repoRoot: string,
+  opts: { platform?: NodeJS.Platform; wslMountRoot?: string | null } = {},
+): AssetPathResolution {
+  const platform = opts.platform ?? process.platform;
+  if (platform !== 'win32') {
+    const m = WINDOWS_DRIVE_RE.exec(storagePath);
+    if (m) {
+      const root = opts.wslMountRoot !== undefined ? opts.wslMountRoot : detectWslMountRoot();
+      if (root === null) return { abs: null, foreign: true };
+      const abs = `${root.replace(/\/+$/, '')}/${m[1].toLowerCase()}/${m[2].replace(/\\/g, '/')}`;
+      return { abs, foreign: false };
+    }
+  }
+  // Platform-appropriate absoluteness (not the host's) so injected-platform
+  // tests behave identically everywhere; in production platform === host.
+  const isAbs = platform === 'win32' ? win32.isAbsolute(storagePath) : posix.isAbsolute(storagePath);
+  return {
+    abs: isAbs ? storagePath : join(repoRoot, storagePath),
+    foreign: false,
+  };
+}
+
+/**
+ * Extract the `[automount] root` value from /etc/wsl.conf content.
+ * Defaults to `/mnt` (WSL's own default) when absent/unparseable.
+ */
+export function parseWslAutomountRoot(conf: string): string {
+  let inAutomount = false;
+  for (const raw of conf.split(/\r?\n/)) {
+    const line = raw.replace(/[#;].*$/, '').trim();
+    if (line.startsWith('[')) {
+      inAutomount = /^\[automount\]$/i.test(line);
+      continue;
+    }
+    if (!inAutomount) continue;
+    const m = /^root\s*=\s*"?([^"]+?)"?\s*$/.exec(line);
+    if (m) return m[1];
+  }
+  return '/mnt';
+}
+
+let cachedWslMountRoot: string | null | undefined;
+
+/**
+ * Detect the WSL Windows-drive automount root. Returns null when not running
+ * under WSL (including macOS and plain Linux). Memoized per process.
+ */
+export function detectWslMountRoot(): string | null {
+  if (cachedWslMountRoot === undefined) cachedWslMountRoot = computeWslMountRoot();
+  return cachedWslMountRoot;
+}
+
+function computeWslMountRoot(): string | null {
+  if (process.platform !== 'linux') return null;
+  try {
+    // The standard WSL tell: kernel version string names Microsoft.
+    if (!/microsoft/i.test(readFileSync('/proc/version', 'utf8'))) return null;
+  } catch {
+    return null;
+  }
+  try {
+    return parseWslAutomountRoot(readFileSync('/etc/wsl.conf', 'utf8'));
+  } catch {
+    return '/mnt'; // WSL default when wsl.conf is absent.
+  }
+}

src/commands/doctor.ts

@@ -7582,33 +7582,44 @@ export async function buildChecks(
         `SELECT storage_path FROM files WHERE mime_type LIKE 'image/%' LIMIT 1000`
       );
       let vanished = 0;
+      let foreign = 0;
       const vanishedPaths: string[] = [];
       const fs = await import('node:fs');
-      const nodePath = await import('node:path');
+      const { resolveAssetPath } = await import('./doctor-asset-paths.ts');
       // storage_path is repo-relative for sync-ingested assets. Resolving
       // against cwd made this check a false-positive WARN whenever doctor
       // ran outside the brain repo.
       const repoRoot = (await engine.getConfig('sync.repo_path')) ?? process.cwd();
       for (const r of rows) {
-        const abs = nodePath.isAbsolute(r.storage_path)
-          ? r.storage_path
-          : nodePath.join(repoRoot, r.storage_path);
+        // #1835: Windows drive paths (D:/…) translate to the WSL automount
+        // (/mnt/d/…) under WSL, and are SKIPPED (not "missing") on hosts
+        // where they cannot exist (macOS / plain Linux) — never joined onto
+        // repoRoot, which produced a false "restore from git" WARN.
+        const resolved = resolveAssetPath(r.storage_path, repoRoot);
+        if (resolved.abs === null) {
+          foreign++;
+          continue;
+        }
         try {
-          fs.statSync(abs);
+          fs.statSync(resolved.abs);
         } catch {
           vanished++;
           if (vanishedPaths.length < 5) vanishedPaths.push(r.storage_path);
         }
       }
+      const checked = rows.length - foreign;
+      const foreignNote = foreign > 0
+        ? ` (${foreign} Windows-drive path(s) skipped — not resolvable on this platform)`
+        : '';
       if (rows.length === 0) {
         checks.push({ name: 'image_assets', status: 'ok', message: 'No image assets indexed yet' });
       } else if (vanished === 0) {
-        checks.push({ name: 'image_assets', status: 'ok', message: `${rows.length} image(s) all present on disk` });
+        checks.push({ name: 'image_assets', status: 'ok', message: `${checked} image(s) all present on disk${foreignNote}` });
       } else {
         checks.push({
           name: 'image_assets',
           status: 'warn',
-          message: `${vanished} of ${rows.length} image(s) missing from disk (e.g. ${vanishedPaths.join(', ')}). ` +
+          message: `${vanished} of ${checked} image(s) missing from disk (e.g. ${vanishedPaths.join(', ')})${foreignNote}. ` +
                    `Fix: restore from git, or \`gbrain sync --skip-failed\` to acknowledge.`,
         });
       }

test/doctor-asset-paths.test.ts

@@ -0,0 +1,105 @@
+/**
+ * #1835 — pure unit coverage for src/commands/doctor-asset-paths.ts.
+ *
+ * Everything here is path/string-based with injected platform + WSL mount
+ * root, so it runs identically on macOS / Linux / CI. The WSL translation
+ * itself is UNVERIFIED-ON-PLATFORM (no real WSL host in this environment);
+ * these tests pin the intended mapping.
+ */
+import { describe, expect, test } from 'bun:test';
+import { resolveAssetPath, parseWslAutomountRoot } from '../src/commands/doctor-asset-paths.ts';
+
+const REPO = '/mnt/d/brain-repo';
+
+describe('resolveAssetPath — Windows drive paths', () => {
+  test('WSL: D:/ forward-slash path maps to <root>/d/…', () => {
+    const r = resolveAssetPath('D:/cicada3301/lost9999/img.jpg', REPO, {
+      platform: 'linux',
+      wslMountRoot: '/mnt',
+    });
+    expect(r).toEqual({ abs: '/mnt/d/cicada3301/lost9999/img.jpg', foreign: false });
+  });
+
+  test('WSL: D:\\ backslash path maps with separators normalized', () => {
+    const r = resolveAssetPath('D:\\cicada3301\\lost9999\\img.jpg', REPO, {
+      platform: 'linux',
+      wslMountRoot: '/mnt',
+    });
+    expect(r).toEqual({ abs: '/mnt/d/cicada3301/lost9999/img.jpg', foreign: false });
+  });
+
+  test('WSL: drive letter is lowercased, custom automount root honored', () => {
+    const r = resolveAssetPath('C:/Users/a/img.png', REPO, {
+      platform: 'linux',
+      wslMountRoot: '/windir/',
+    });
+    expect(r.abs).toBe('/windir/c/Users/a/img.png');
+  });
+
+  test('macOS: drive path is foreign (skip, never joined onto repoRoot)', () => {
+    const r = resolveAssetPath('D:/cicada3301/img.jpg', REPO, {
+      platform: 'darwin',
+      wslMountRoot: null,
+    });
+    expect(r).toEqual({ abs: null, foreign: true });
+  });
+
+  test('plain Linux (non-WSL): drive path is foreign', () => {
+    const r = resolveAssetPath('D:/x/img.jpg', REPO, {
+      platform: 'linux',
+      wslMountRoot: null,
+    });
+    expect(r).toEqual({ abs: null, foreign: true });
+  });
+
+  test('win32: drive path stats natively, untouched', () => {
+    const r = resolveAssetPath('D:/x/img.jpg', REPO, { platform: 'win32' });
+    expect(r).toEqual({ abs: 'D:/x/img.jpg', foreign: false });
+  });
+});
+
+describe('resolveAssetPath — non-drive paths keep pre-#1835 behavior', () => {
+  test('POSIX absolute path passes through', () => {
+    const r = resolveAssetPath('/var/data/img.jpg', REPO, {
+      platform: 'linux',
+      wslMountRoot: null,
+    });
+    expect(r).toEqual({ abs: '/var/data/img.jpg', foreign: false });
+  });
+
+  test('relative path joins onto repoRoot', () => {
+    const r = resolveAssetPath('assets/img.jpg', REPO, {
+      platform: 'darwin',
+      wslMountRoot: null,
+    });
+    expect(r).toEqual({ abs: `${REPO}/assets/img.jpg`, foreign: false });
+  });
+
+  test('lookalike without separator after colon is NOT treated as a drive', () => {
+    const r = resolveAssetPath('notes:draft.md', REPO, {
+      platform: 'linux',
+      wslMountRoot: '/mnt',
+    });
+    expect(r).toEqual({ abs: `${REPO}/notes:draft.md`, foreign: false });
+  });
+});
+
+describe('parseWslAutomountRoot', () => {
+  test('defaults to /mnt on empty or unrelated config', () => {
+    expect(parseWslAutomountRoot('')).toBe('/mnt');
+    expect(parseWslAutomountRoot('[boot]\nsystemd=true\n')).toBe('/mnt');
+  });
+
+  test('reads [automount] root', () => {
+    expect(parseWslAutomountRoot('[automount]\nroot = /custom\n')).toBe('/custom');
+  });
+
+  test('ignores root under a different section', () => {
+    expect(parseWslAutomountRoot('[network]\nroot = /nope\n')).toBe('/mnt');
+  });
+
+  test('handles quotes, comments, and CRLF', () => {
+    const conf = '[automount]\r\nroot = "/win" # drives here\r\noptions = "metadata"\r\n';
+    expect(parseWslAutomountRoot(conf)).toBe('/win');
+  });
+});

test/doctor-image-assets-wsl.test.ts

@@ -0,0 +1,104 @@
+/**
+ * #1835 — doctor `image_assets`: Windows drive paths (`D:/…`, `D:\…`) written
+ * by a Windows gbrain install must not be reported as "missing from disk"
+ * on POSIX hosts that cannot resolve them.
+ *
+ * Behavioral test through the master-existing `buildChecks` seam — it
+ * deliberately imports NOTHING introduced by this fix, so running this file
+ * against an unmodified master demonstrates the bug (image_assets WARNs
+ * "restore from git" for a drive path that was never lost).
+ *
+ * Pure translation-logic coverage (WSL /mnt mapping, wsl.conf parsing) lives
+ * in test/doctor-asset-paths.test.ts.
+ */
+import { describe, expect, test, beforeAll, afterAll, beforeEach } from 'bun:test';
+import { readFileSync, mkdtempSync, writeFileSync } from 'node:fs';
+import { tmpdir } from 'node:os';
+import { join } from 'node:path';
+import { PGLiteEngine } from '../src/core/pglite-engine.ts';
+import { resetPgliteState } from './helpers/reset-pglite.ts';
+import { buildChecks, type Check } from '../src/commands/doctor.ts';
+
+let engine: PGLiteEngine;
+let repoRoot: string;
+
+// These assertions describe non-WSL POSIX hosts (macOS, plain Linux — every
+// dev box + CI runner here). On real WSL the drive path is translated and
+// statted instead; on win32 it stats natively. Skip there.
+const onWsl = (() => {
+  try {
+    return process.platform === 'linux' && /microsoft/i.test(readFileSync('/proc/version', 'utf8'));
+  } catch {
+    return false;
+  }
+})();
+const skip = onWsl || process.platform === 'win32';
+
+beforeAll(async () => {
+  engine = new PGLiteEngine();
+  await engine.connect({});
+  await engine.initSchema();
+});
+
+afterAll(async () => {
+  await engine.disconnect();
+});
+
+beforeEach(async () => {
+  await resetPgliteState(engine);
+  repoRoot = mkdtempSync(join(tmpdir(), 'gbrain-1835-'));
+  await engine.setConfig('sync.repo_path', repoRoot);
+});
+
+async function insertImage(storagePath: string, hash: string): Promise<void> {
+  await engine.executeRaw(
+    `INSERT INTO files (source_id, filename, storage_path, mime_type, content_hash)
+     VALUES ('default', 'img.jpg', $1, 'image/jpeg', $2)`,
+    [storagePath, hash],
+  );
+}
+
+async function imageAssetsCheck(): Promise<Check> {
+  const checks = await buildChecks(engine, []);
+  const check = checks.find((c) => c.name === 'image_assets');
+  expect(check).toBeDefined();
+  return check!;
+}
+
+describe('doctor image_assets — Windows drive paths on POSIX (#1835)', () => {
+  test.skipIf(skip)('D:/ path is skipped with a note, not reported missing', async () => {
+    await insertImage('D:/cicada3301/lost9999/img.jpg', 'h1');
+    const check = await imageAssetsCheck();
+    // Master joins the drive path onto repoRoot and WARNs "missing from
+    // disk … restore from git" — a false data-loss report.
+    expect(check.status).toBe('ok');
+    expect(check.message).toContain('Windows-drive path(s) skipped');
+    expect(check.message).not.toContain('restore from git');
+  });
+
+  test.skipIf(skip)('backslash D:\\ path is also skipped', async () => {
+    await insertImage('D:\\cicada3301\\lost9999\\img.jpg', 'h2');
+    const check = await imageAssetsCheck();
+    expect(check.status).toBe('ok');
+    expect(check.message).toContain('Windows-drive path(s) skipped');
+  });
+
+  test.skipIf(skip)('drive path skip does not mask a genuinely vanished asset', async () => {
+    await insertImage('D:/cicada3301/lost9999/img.jpg', 'h3');
+    await insertImage('assets/really-gone.png', 'h4');
+    const check = await imageAssetsCheck();
+    expect(check.status).toBe('warn');
+    expect(check.message).toContain('assets/really-gone.png');
+    // The unresolvable drive path is excluded from the checked denominator.
+    expect(check.message).toContain('1 of 1 image(s) missing');
+    expect(check.message).toContain('Windows-drive path(s) skipped');
+  });
+
+  test('present relative asset still resolves against repoRoot (regression guard)', async () => {
+    writeFileSync(join(repoRoot, 'here.png'), 'x');
+    await insertImage('here.png', 'h5');
+    const check = await imageAssetsCheck();
+    expect(check.status).toBe('ok');
+    expect(check.message).toContain('all present on disk');
+  });
+});