diff --git a/src/commands/doctor-asset-paths.ts b/src/commands/doctor-asset-paths.ts new file mode 100644 index 000000000..489b5b0eb --- /dev/null +++ b/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 + * `/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. + } +} diff --git a/src/commands/doctor.ts b/src/commands/doctor.ts index 163ce1d73..6d6e5c7a8 100644 --- a/src/commands/doctor.ts +++ b/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.`, }); } diff --git a/test/doctor-asset-paths.test.ts b/test/doctor-asset-paths.test.ts new file mode 100644 index 000000000..0c717b6d0 --- /dev/null +++ b/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 /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'); + }); +}); diff --git a/test/doctor-image-assets-wsl.test.ts b/test/doctor-image-assets-wsl.test.ts new file mode 100644 index 000000000..50de884c1 --- /dev/null +++ b/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 { + 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 { + 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'); + }); +});