From 7835cd47bc3ea63d0d58c53387c1023400a45e36 Mon Sep 17 00:00:00 2001 From: VL <123257737+vlisitskii@users.noreply.github.com> Date: Fri, 10 Jul 2026 19:12:53 +0100 Subject: [PATCH] fix(cli): actionable diagnostics for non-4K page-size buffer manager failures (#2424) --- gitnexus/scripts/cross-platform-tests.ts | 4 + gitnexus/src/cli/analyze.ts | 52 ++++ gitnexus/src/cli/cli-message.ts | 1 + gitnexus/src/cli/doctor.ts | 41 +++ gitnexus/src/core/lbug/lbug-config.ts | 101 +++++++ .../test/unit/analyze-pagesize-error.test.ts | 265 ++++++++++++++++++ gitnexus/test/unit/analyze-wal-error.test.ts | 16 +- gitnexus/test/unit/analyze-wipe-error.test.ts | 16 +- gitnexus/test/unit/doctor-format.test.ts | 37 +++ .../test/unit/lbug-config-pagesize.test.ts | 162 +++++++++++ 10 files changed, 693 insertions(+), 2 deletions(-) create mode 100644 gitnexus/test/unit/analyze-pagesize-error.test.ts create mode 100644 gitnexus/test/unit/lbug-config-pagesize.test.ts diff --git a/gitnexus/scripts/cross-platform-tests.ts b/gitnexus/scripts/cross-platform-tests.ts index b80266ef1..03f4673da 100644 --- a/gitnexus/scripts/cross-platform-tests.ts +++ b/gitnexus/scripts/cross-platform-tests.ts @@ -36,6 +36,10 @@ const PLATFORM_LOGIC = [ // must exercise the Windows backslash branch, so run it on the OS matrix (#2394). 'test/unit/cli-entry.test.ts', 'test/unit/platform-capabilities.test.ts', + // getconf page-size probe: explicit process.platform gate (win32 short-circuit) + // plus a live-probe test whose only real non-4K coverage is macos-arm64's + // 16 KiB pages — the exact hardware class #1231 targets (#2424 review). + 'test/unit/lbug-config-pagesize.test.ts', 'test/unit/worker-pool-windows-quarantine.test.ts', 'test/unit/lbug-pool-fts-load.test.ts', 'test/unit/repo-manager.test.ts', diff --git a/gitnexus/src/cli/analyze.ts b/gitnexus/src/cli/analyze.ts index 8dbbe5117..77b49504c 100644 --- a/gitnexus/src/cli/analyze.ts +++ b/gitnexus/src/cli/analyze.ts @@ -16,7 +16,10 @@ import cliProgress from 'cli-progress'; import { isLbugReady, LbugWipeError } from '../core/lbug/lbug-adapter.js'; import { boundedCheckpointBeforeExit } from '../core/lbug/shutdown-helpers.js'; import { + getOsPageSize, isLbugCheckpointIoError, + isLbugPageSizeFrameError, + isPageSizeAwareLadybug, isWalCorruptionError, parseWalCheckpointThreshold, WAL_RECOVERY_SUGGESTION, @@ -37,6 +40,7 @@ import { GitNexusRcError, } from './analyze-config.js'; import { runFullAnalysis } from '../core/run-analyze.js'; +import { getRuntimeFingerprint } from '../core/platform/capabilities.js'; import { getMaxFileSizeBannerMessage } from '../core/ingestion/utils/max-file-size.js'; import { warnMissingOptionalGrammars, getOptionalGrammarExtensions } from './optional-grammars.js'; import { glob } from 'glob'; @@ -1635,6 +1639,54 @@ const analyzeCommandImpl = async ( return; } + // Buffer-manager frame-release failure on non-4K-page kernels (#1231). + // LadybugDB <= 0.17.x assumed 4 KiB OS pages when releasing evicted + // frames; Raspberry Pi 5 (16 KiB kernel pages) and other arm64 systems + // crash mid-COPY with a raw native message. 0.18.0 detects the page size + // at runtime, so the actionable fix depends on which side of that + // boundary the installed @ladybugdb/core is. + if (isLbugPageSizeFrameError(err)) { + const pageSize = getOsPageSize(); + const ladybug = getRuntimeFingerprint().ladybugdb; + const pageLine = + pageSize !== undefined && pageSize !== 4096 + ? ` Detected OS page size: ${pageSize} bytes (non-4K — e.g. Raspberry Pi 5 16K kernel, Asahi Linux).\n` + : ''; + // The upgrade variant must not assert version facts about an unknown + // version — mirror the doctor-side wording rule (#2424 review R2). + const upgradeIntro = + ladybug === undefined + ? ` The installed @ladybugdb/core version is unknown — it may predate the\n` + + ` runtime OS-page-size detection added in 0.18.0.\n` + : ` The installed @ladybugdb/core (${ladybug}) assumes 4 KiB pages in its buffer\n` + + ` manager.\n`; + const guidance = isPageSizeAwareLadybug(ladybug) + ? ` The installed @ladybugdb/core (${ladybug}) already detects the OS page size at runtime,\n` + + ` so this configuration was expected to work. Please report it:\n` + + ` https://github.com/abhigyanpatwari/GitNexus/issues/1231\n` + + ` and include: gitnexus --version, node --version, getconf PAGE_SIZE, uname -a,\n` + + ` and the full error message above.\n` + : upgradeIntro + + ` Upgrade GitNexus to a release that bundles @ladybugdb/core >= 0.18.0\n` + + ` (gitnexus >= 1.6.9), which detects the OS page size at runtime:\n` + + ` npm install -g gitnexus@latest\n` + + ` Last-resort workaround on Raspberry Pi 5: boot the 4 KiB-page kernel\n` + + ` (config.txt: kernel=kernel8.img), at the cost of Pi 5 optimizations.\n`; + // Embed the raw native text (indented, no stack) so "the full error + // message above" is fulfillable — same idiom as the LbugWipeError + // branch. The errno suffix and the 0.18.0 guard's frame/granule numbers + // are the discriminating triage content (#2424 review P2). + cliError( + ` LadybugDB's buffer manager failed to release frame memory.\n` + + ` ${msg.replace(/\n/g, '\n ')}\n` + + pageLine + + guidance, + { recoveryHint: 'lbug-page-size', pageSize, ladybugVersion: ladybug }, + ); + process.exitCode = 1; + return; + } + // Local embedding runtime unsupported on this platform (macOS Intel ships no // darwin/x64 ONNX native binding, #1515). The guard threw before importing // transformers.js, so this is a clean, actionable GitNexus message. Checked diff --git a/gitnexus/src/cli/cli-message.ts b/gitnexus/src/cli/cli-message.ts index 8dcbd96b3..9d27b6686 100644 --- a/gitnexus/src/cli/cli-message.ts +++ b/gitnexus/src/cli/cli-message.ts @@ -47,6 +47,7 @@ export type RecoveryHint = | 'wal-corruption' | 'wal-checkpoint-threshold' | 'lbug-wipe-failed' + | 'lbug-page-size' | 'heap-oom-respawn' | 'native-worker-abort' | 'hf-endpoint-unreachable' diff --git a/gitnexus/src/cli/doctor.ts b/gitnexus/src/cli/doctor.ts index 4031ab023..ce9811de6 100644 --- a/gitnexus/src/cli/doctor.ts +++ b/gitnexus/src/cli/doctor.ts @@ -13,6 +13,7 @@ import { } from '../core/embeddings/runtime-install.js'; import { cudaRedirectDoctorStatus } from '../core/embeddings/onnxruntime-node-resolver.js'; import { checkLbugNative, probeFtsExtensionLoad } from '../core/lbug/native-check.js'; +import { getOsPageSize, isPageSizeAwareLadybug } from '../core/lbug/lbug-config.js'; import { diagnoseExtensionLoad } from '../core/lbug/extension-load-error.js'; import { getExtensionInstallPolicy } from '../core/lbug/extension-loader.js'; import { t } from './i18n/index.js'; @@ -112,6 +113,39 @@ export function localEmbeddingDoctorStatus(opts: { return { status: '✓ local embeddings supported', detail: null }; } +/** + * Page-size lines for the `doctor` Runtime section (#1231). Pure so the + * warning gate can be unit-tested without running the whole command (the + * `localEmbeddingDoctorStatus` precedent above) — but takes the probed + * values as plain params rather than injectable probes, because `undefined` + * is a *meaningful* pageSize state here (probe unavailable / win32) and + * would collide with a "not provided → use default" DI convention. + * + * Returns 0 lines (page size unknown), 1 line (page size), or 2 lines + * (page size + non-4K warning when the installed @ladybugdb/core does not + * detect the OS page size at runtime). + */ +export function pageSizeDoctorLines( + pageSize: number | undefined, + ladybugVersion: string | undefined, +): string[] { + if (pageSize === undefined) return []; + const lines = [` ${padDisplayEnd('page size', 10)}${pageSize}`]; + if (pageSize > 4096 && !isPageSizeAwareLadybug(ladybugVersion)) { + // Don't assert "< 0.18.0" as fact when the version is unresolvable + // (#2424 review R2) — name the unknown state instead. + const versionClause = + ladybugVersion === undefined + ? 'an unknown @ladybugdb/core version (may predate 0.18.0)' + : `@ladybugdb/core < 0.18.0`; + lines.push( + ` ${padDisplayEnd('', 10)}⚠ non-4K page size with ${versionClause} — ` + + `'gitnexus analyze' may fail during COPY (#1231). Upgrade gitnexus (npm install -g gitnexus@latest).`, + ); + } + return lines; +} + export const doctorCommand = async () => { const fingerprint = getRuntimeFingerprint(); const capabilities = getRuntimeCapabilities(); @@ -123,6 +157,13 @@ export const doctorCommand = async () => { console.log(` ${label('doctor.labels.node', 10)}${fingerprint.node}`); console.log(` ${label('doctor.labels.gitnexus', 10)}${fingerprint.gitnexus}`); console.log(` ${label('doctor.labels.ladybugdb', 10)}${fingerprint.ladybugdb ?? 'unknown'}`); + // OS page size next to the LadybugDB version because the two interact: + // @ladybugdb/core < 0.18.0 assumed 4 KiB pages in its buffer manager and + // crashes mid-COPY on 16 KiB/64 KiB-page kernels (#1231). Literal label + // (like the 'native' line below) to avoid adding i18n keys. + for (const line of pageSizeDoctorLines(getOsPageSize(), fingerprint.ladybugdb)) { + console.log(line); + } const nativeCheck = checkLbugNative(); if (nativeCheck.ok) { console.log(` ${padDisplayEnd('native', 10)}✓ lbugjs.node loaded`); diff --git a/gitnexus/src/core/lbug/lbug-config.ts b/gitnexus/src/core/lbug/lbug-config.ts index 8d9e28f6a..d3159a732 100644 --- a/gitnexus/src/core/lbug/lbug-config.ts +++ b/gitnexus/src/core/lbug/lbug-config.ts @@ -355,6 +355,107 @@ export const isLbugCheckpointIoError = (err: unknown): boolean => { return LBUG_CHECKPOINT_PERMISSIVE_RE.test(msg); }; +// ─── Ladybug non-4K page-size frame-release matcher (#1231) ───────────────── +// +// LadybugDB <= 0.17.x hardcoded a 4 KiB OS-page assumption in its buffer +// manager: evicting a frame released physical memory with +// `madvise(frame, frameSize, MADV_DONTNEED)` on 4 KiB-aligned frame +// addresses (verified by disassembling `VMRegion::releaseFrame` in +// @ladybugdb/core-linux-arm64 0.17.1 — `mov w2, #0x4` = MADV_DONTNEED, +// throw on non-zero return). On kernels with 16 KiB pages (Raspberry Pi 5 +// default 2712 kernel, Asahi Linux) or 64 KiB pages (some enterprise arm64 +// distros), madvise rejects addresses that are not multiples of the real +// page size with EINVAL, surfacing as: +// "Buffer manager exception: Releasing physical memory associated with a +// frame failed with error code -1: Invalid argument." +// which aborts `gitnexus analyze` mid-COPY. +// +// @ladybugdb/core 0.18.0 rewrote the release path with runtime OS-page-size +// detection and discard-granule-aligned madvise (new binary strings: +// "Failed to detect the operating system page size.", "Unsupported page +// size combination: frame size {}, discard granule size {}, frame group +// size {}."), so upgrading is the fix. The residual 0.18.0 guard +// ("Unsupported page size combination") is matched here too so exotic +// configurations receive the same actionable guidance instead of a raw +// native message. +const LBUG_FRAME_RELEASE_RE = /releasing physical memory associated with a frame failed/i; +const LBUG_PAGE_COMBO_RE = /unsupported page size combination/i; + +/** + * True when `err` looks like the LadybugDB buffer manager failing to release + * frame memory — the failure mode of a 4 KiB page-size assumption on a + * 16 KiB/64 KiB-page kernel (#1231). Deliberately does NOT match the + * generic "buffer pool is full" exhaustion error, which is a sizing + * problem, not a page-size one. + */ +export const isLbugPageSizeFrameError = (err: unknown): boolean => { + if (!err) return false; + const msg = err instanceof Error ? err.message : String(err); + return LBUG_FRAME_RELEASE_RE.test(msg) || LBUG_PAGE_COMBO_RE.test(msg); +}; + +/** + * True when the given `@ladybugdb/core` version contains the runtime + * OS-page-size detection introduced in 0.18.0 (see the matcher comment + * above). Unknown/unparseable versions return false so callers err on the + * side of showing the upgrade hint. + */ +export const isPageSizeAwareLadybug = (version: string | undefined): boolean => { + if (!version) return false; + const m = /^(\d+)\.(\d+)/.exec(version.trim()); + if (!m) return false; + const major = Number(m[1]); + const minor = Number(m[2]); + return major > 0 || minor >= 18; +}; + +// `undefined` = not probed yet; `null` = probed and unavailable. Cached +// because analyze error paths and doctor may both ask, and getconf forks. +let cachedOsPageSize: number | null | undefined; + +/** + * OS memory page size in bytes, or `undefined` when it cannot be determined + * (Windows, missing getconf, sandboxed exec). Node exposes no page-size API, + * so this shells out to POSIX `getconf PAGE_SIZE` — same execFileSync shape + * as the Windows 8.3 short-path probe above, but with a tighter timeout and + * an explicit killSignal (see the options comment below). + */ +export const getOsPageSize = (): number | undefined => { + if (cachedOsPageSize !== undefined) return cachedOsPageSize ?? undefined; + if (process.platform === 'win32') { + // Windows allocation granularity is not what madvise alignment is about; + // the #1231 failure mode is POSIX-only. + cachedOsPageSize = null; + return undefined; + } + try { + // killSignal SIGKILL (first use in this repo): the default SIGTERM is + // catchable, so a signal-trapping child held the "5s" timeout for 9s in + // review reproduction — SIGKILL makes the timeout real for everything + // except a child stuck in uninterruptible I/O (D state). 2000ms, not + // 5000: doctor runs this probe on its happy path and real getconf + // answers in ~2ms, but keep margin for loaded Pi-class hardware — a + // too-tight ceiling would silently drop the very #1231 diagnostics this + // probe exists to provide (the catch caches the failure). (#2424 review) + const out = execFileSync('getconf', ['PAGE_SIZE'], { + encoding: 'utf-8', + timeout: 2000, + killSignal: 'SIGKILL', + stdio: ['ignore', 'pipe', 'pipe'], + }); + const parsed = Number(out.trim()); + cachedOsPageSize = Number.isInteger(parsed) && parsed > 0 ? parsed : null; + } catch { + cachedOsPageSize = null; + } + return cachedOsPageSize ?? undefined; +}; + +/** Exported only for unit tests — clears the getconf probe cache. */ +export const _resetOsPageSizeCacheForTest = (): void => { + cachedOsPageSize = undefined; +}; + type LbugModule = typeof lbug; export interface LbugDatabaseOptions { diff --git a/gitnexus/test/unit/analyze-pagesize-error.test.ts b/gitnexus/test/unit/analyze-pagesize-error.test.ts new file mode 100644 index 000000000..687816f21 --- /dev/null +++ b/gitnexus/test/unit/analyze-pagesize-error.test.ts @@ -0,0 +1,265 @@ +/** + * Tests for non-4K page-size buffer-manager error handling in `analyzeCommand` + * (#1231). + * + * On kernels with 16 KiB pages (Raspberry Pi 5, Asahi Linux) a + * @ladybugdb/core < 0.18.0 buffer manager fails to release evicted frames + * (madvise EINVAL) and analyze aborts mid-COPY with a raw native message. + * The CLI must catch that shape before the generic error path and render an + * actionable message: what the OS page size is, and whether the fix is + * upgrading (@ladybugdb/core < 0.18.0) or reporting (>= 0.18.0). + * + * Mirrors the mock shape of analyze-wal-error.test.ts. + */ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import type { RuntimeFingerprint } from '../../src/core/platform/capabilities.js'; + +const runFullAnalysisMock = vi.fn(); + +vi.mock('../../src/core/run-analyze.js', () => ({ + runFullAnalysis: runFullAnalysisMock, +})); + +vi.mock('../../src/core/lbug/lbug-adapter.js', async (importOriginal) => ({ + ...(await importOriginal()), + closeLbug: vi.fn(async () => undefined), + closeLbugBeforeExit: vi.fn(async () => undefined), + isLbugReady: vi.fn(() => false), +})); + +vi.mock('../../src/storage/repo-manager.js', () => ({ + getStoragePaths: vi.fn(() => ({ storagePath: '.gitnexus', lbugPath: '.gitnexus/lbug' })), + getGlobalRegistryPath: vi.fn(() => 'registry.json'), + RegistryNameCollisionError: class RegistryNameCollisionError extends Error {}, + AnalysisNotFinalizedError: class AnalysisNotFinalizedError extends Error {}, + assertAnalysisFinalized: vi.fn(async () => undefined), +})); + +vi.mock('../../src/storage/git.js', () => ({ + getGitRoot: vi.fn(() => '/repo'), + hasGitDir: vi.fn(() => true), +})); + +vi.mock('../../src/core/ingestion/utils/max-file-size.js', () => ({ + getMaxFileSizeBannerMessage: vi.fn(() => null), +})); + +// analyze.ts imports isHfDownloadFailure from hf-env.js, which in turn imports +// from gitnexus-shared (not linked in dev). Mock the module to break the chain. +vi.mock('../../src/core/embeddings/hf-env.js', () => ({ + isHfDownloadFailure: vi.fn(() => false), +})); + +// Pin the fingerprint so assertions do not depend on the dev environment's +// installed @ladybugdb/core. +const fingerprintMock = vi.fn( + (): RuntimeFingerprint => ({ + platform: process.platform, + arch: process.arch, + node: process.version, + gitnexus: 'test', + ladybugdb: '0.17.1', + }), +); +vi.mock('../../src/core/platform/capabilities.js', async (importOriginal) => ({ + ...(await importOriginal()), + getRuntimeFingerprint: fingerprintMock, +})); + +// Pin the OS page size so the "Detected OS page size" line is deterministic +// (on 4 KiB dev/CI hosts the real probe would render nothing). The spread is +// load-bearing: analyze.ts also takes isWalCorruptionError / +// isLbugCheckpointIoError / isLbugPageSizeFrameError from this module, and +// the WAL/checkpoint branches run BEFORE the page-size branch in the same +// catch — a non-spread mock would stub them and reroute the test errors. +const getOsPageSizeMock = vi.fn((): number | undefined => 16384); +vi.mock('../../src/core/lbug/lbug-config.js', async (importOriginal) => ({ + ...(await importOriginal()), + getOsPageSize: getOsPageSizeMock, +})); + +const PI5_COPY_ERROR = + 'COPY failed for File: Buffer manager exception: Releasing physical memory ' + + 'associated with a frame failed with error code -1: Invalid argument.'; + +describe('analyzeCommand non-4K page-size error handling (#1231)', () => { + // Capture the host's NODE_OPTIONS once so afterEach can restore it cleanly. + // Without the restore, beforeEach's append accumulated duplicate + // --max-old-space-size tokens across tests (analyze-worker-pool-size.test.ts + // pattern; #2424 review). + const ORIGINAL_NODE_OPTIONS = process.env.NODE_OPTIONS; + + beforeEach(() => { + vi.resetModules(); + runFullAnalysisMock.mockReset(); + getOsPageSizeMock.mockReset(); + getOsPageSizeMock.mockReturnValue(16384); + process.exitCode = undefined; + process.env.NODE_OPTIONS = `${process.env.NODE_OPTIONS ?? ''} --max-old-space-size=8192`.trim(); + }); + + afterEach(() => { + if (ORIGINAL_NODE_OPTIONS === undefined) { + delete process.env.NODE_OPTIONS; + } else { + process.env.NODE_OPTIONS = ORIGINAL_NODE_OPTIONS; + } + }); + + it('recommends upgrading when @ladybugdb/core < 0.18.0 hits the frame-release error', async () => { + fingerprintMock.mockReturnValue({ + platform: process.platform, + arch: process.arch, + node: process.version, + gitnexus: 'test', + ladybugdb: '0.17.1', + }); + runFullAnalysisMock.mockRejectedValue(new Error(PI5_COPY_ERROR)); + + const { _captureLogger } = await import('../../src/core/logger.js'); + const cap = _captureLogger(); + const { analyzeCommand } = await import('../../src/cli/analyze.js'); + + await analyzeCommand(undefined, {}); + + expect(process.exitCode).toBe(1); + const records = cap.records(); + const hint = records.find( + (r) => typeof r.msg === 'string' && r.msg.includes('failed to release frame memory'), + ); + expect(hint).toBeDefined(); + expect(hint?.msg).toContain('0.18.0'); + expect(hint?.msg).toContain('npm install -g gitnexus@latest'); + // The raw native error text is embedded so users can attach it to reports + // (#2424 review P2) — and the page-size line renders the mocked probe. + expect(hint?.msg).toContain(PI5_COPY_ERROR); + expect(hint?.msg).toContain('Detected OS page size: 16384 bytes'); + // Structured fields flow to log aggregation (mirror analyze-wipe-error). + expect(hint).toMatchObject({ + recoveryHint: 'lbug-page-size', + pageSize: 16384, + ladybugVersion: '0.17.1', + }); + // Raw stack trace must NOT appear via cliError + const stackRecord = records.find( + (r) => typeof r.msg === 'string' && r.msg.includes('at analyzeCommand'), + ); + expect(stackRecord).toBeUndefined(); + + cap.restore(); + }); + + it('asks for a bug report when @ladybugdb/core >= 0.18.0 still hits the error', async () => { + fingerprintMock.mockReturnValue({ + platform: process.platform, + arch: process.arch, + node: process.version, + gitnexus: 'test', + ladybugdb: '0.18.0', + }); + runFullAnalysisMock.mockRejectedValue(new Error(PI5_COPY_ERROR)); + + const { _captureLogger } = await import('../../src/core/logger.js'); + const cap = _captureLogger(); + const { analyzeCommand } = await import('../../src/cli/analyze.js'); + + await analyzeCommand(undefined, {}); + + expect(process.exitCode).toBe(1); + const records = cap.records(); + const hint = records.find( + (r) => typeof r.msg === 'string' && r.msg.includes('failed to release frame memory'), + ); + expect(hint).toBeDefined(); + expect(hint?.msg).toContain('issues/1231'); + expect(hint?.msg).not.toContain('npm install -g gitnexus@latest'); + // The report-a-bug path asks for "the full error message above" — the + // embedded raw text is what makes that instruction fulfillable. + expect(hint?.msg).toContain(PI5_COPY_ERROR); + expect(hint).toMatchObject({ + recoveryHint: 'lbug-page-size', + pageSize: 16384, + ladybugVersion: '0.18.0', + }); + + cap.restore(); + }); + + it.each([ + ['a 4 KiB host', 4096], + ['an unavailable probe', undefined], + ])('omits the page-size line on %s', async (_label, probed) => { + getOsPageSizeMock.mockReturnValue(probed); + runFullAnalysisMock.mockRejectedValue(new Error(PI5_COPY_ERROR)); + + const { _captureLogger } = await import('../../src/core/logger.js'); + const cap = _captureLogger(); + const { analyzeCommand } = await import('../../src/cli/analyze.js'); + + await analyzeCommand(undefined, {}); + + const hint = cap + .records() + .find((r) => typeof r.msg === 'string' && r.msg.includes('failed to release frame memory')); + expect(hint).toBeDefined(); + expect(hint?.msg).not.toContain('Detected OS page size'); + expect(hint?.msg).toContain(PI5_COPY_ERROR); + + cap.restore(); + }); + + it('names the unknown version instead of asserting facts about it', async () => { + fingerprintMock.mockReturnValue({ + platform: process.platform, + arch: process.arch, + node: process.version, + gitnexus: 'test', + }); + runFullAnalysisMock.mockRejectedValue(new Error(PI5_COPY_ERROR)); + + const { _captureLogger } = await import('../../src/core/logger.js'); + const cap = _captureLogger(); + const { analyzeCommand } = await import('../../src/cli/analyze.js'); + + await analyzeCommand(undefined, {}); + + expect(process.exitCode).toBe(1); + const hint = cap + .records() + .find((r) => typeof r.msg === 'string' && r.msg.includes('failed to release frame memory')); + expect(hint).toBeDefined(); + // Unknown version must not read "(unknown) assumes 4 KiB pages" (#2424 + // review R2) — name the unknown state, keep the upgrade instruction. + expect(hint?.msg).toContain('version is unknown'); + expect(hint?.msg).not.toContain('(unknown) assumes'); + expect(hint?.msg).toContain('npm install -g gitnexus@latest'); + + cap.restore(); + }); + + it('does NOT route buffer-pool exhaustion through the page-size handler', async () => { + runFullAnalysisMock.mockRejectedValue( + new Error( + 'COPY failed for File: Buffer manager exception: Unable to allocate memory! ' + + 'The buffer pool is full and no memory could be freed!', + ), + ); + + const { _captureLogger } = await import('../../src/core/logger.js'); + const cap = _captureLogger(); + const { analyzeCommand } = await import('../../src/cli/analyze.js'); + + await analyzeCommand(undefined, {}); + + expect(process.exitCode).toBe(1); + const records = cap.records(); + expect( + records.some( + (r) => typeof r.msg === 'string' && r.msg.includes('failed to release frame memory'), + ), + ).toBe(false); + + cap.restore(); + }); +}); diff --git a/gitnexus/test/unit/analyze-wal-error.test.ts b/gitnexus/test/unit/analyze-wal-error.test.ts index 922438977..097175aa2 100644 --- a/gitnexus/test/unit/analyze-wal-error.test.ts +++ b/gitnexus/test/unit/analyze-wal-error.test.ts @@ -10,7 +10,7 @@ * - drive `analyzeCommand` with a mocked `runFullAnalysis` that throws * - assert on process.exitCode and the logged output */ -import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; const runFullAnalysisMock = vi.fn(); @@ -55,6 +55,12 @@ vi.mock('../../src/core/embeddings/hf-env.js', () => ({ // ─── Tests ─────────────────────────────────────────────────────────────────── describe('analyzeCommand WAL corruption error handling', () => { + // Capture the host's NODE_OPTIONS once so afterEach can restore it cleanly. + // Without the restore, beforeEach's append accumulated duplicate + // --max-old-space-size tokens across tests (analyze-worker-pool-size.test.ts + // pattern; #2424 review). + const ORIGINAL_NODE_OPTIONS = process.env.NODE_OPTIONS; + beforeEach(() => { vi.resetModules(); runFullAnalysisMock.mockReset(); @@ -63,6 +69,14 @@ describe('analyzeCommand WAL corruption error handling', () => { process.env.NODE_OPTIONS = `${process.env.NODE_OPTIONS ?? ''} --max-old-space-size=8192`.trim(); }); + afterEach(() => { + if (ORIGINAL_NODE_OPTIONS === undefined) { + delete process.env.NODE_OPTIONS; + } else { + process.env.NODE_OPTIONS = ORIGINAL_NODE_OPTIONS; + } + }); + it('surfaces a clean recovery message on a re-wrapped WAL corruption error', async () => { // This error shape is what lbug-adapter throws after detecting WAL corruption // in doInitLbug and re-wrapping it with the recovery suggestion. diff --git a/gitnexus/test/unit/analyze-wipe-error.test.ts b/gitnexus/test/unit/analyze-wipe-error.test.ts index 4e415c1d6..d6d881ffa 100644 --- a/gitnexus/test/unit/analyze-wipe-error.test.ts +++ b/gitnexus/test/unit/analyze-wipe-error.test.ts @@ -14,7 +14,7 @@ * - drive `analyzeCommand` with a mocked `runFullAnalysis` that rejects * - assert on process.exitCode and the captured logger records */ -import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; const runFullAnalysisMock = vi.fn(); @@ -71,6 +71,12 @@ vi.mock('../../src/core/embeddings/hf-env.js', () => ({ })); describe('analyzeCommand LadybugDB wipe-failure handling (#2409, tri-review 4669518496)', () => { + // Capture the host's NODE_OPTIONS once so afterEach can restore it cleanly. + // Without the restore, beforeEach's append accumulated duplicate + // --max-old-space-size tokens across tests (analyze-worker-pool-size.test.ts + // pattern; #2424 review). + const ORIGINAL_NODE_OPTIONS = process.env.NODE_OPTIONS; + beforeEach(() => { vi.resetModules(); runFullAnalysisMock.mockReset(); @@ -81,6 +87,14 @@ describe('analyzeCommand LadybugDB wipe-failure handling (#2409, tri-review 4669 process.env.NODE_OPTIONS = `${process.env.NODE_OPTIONS ?? ''} --max-old-space-size=8192`.trim(); }); + afterEach(() => { + if (ORIGINAL_NODE_OPTIONS === undefined) { + delete process.env.NODE_OPTIONS; + } else { + process.env.NODE_OPTIONS = ORIGINAL_NODE_OPTIONS; + } + }); + it('routes a wipe failure to the dedicated recovery hint, not the raw-stack fallback', async () => { // Install the stderr spy BEFORE importing analyze.js: the module binds // `realStderrWrite = process.stderr.write.bind(...)` at load time, so a diff --git a/gitnexus/test/unit/doctor-format.test.ts b/gitnexus/test/unit/doctor-format.test.ts index 11f3bac59..2d06f5124 100644 --- a/gitnexus/test/unit/doctor-format.test.ts +++ b/gitnexus/test/unit/doctor-format.test.ts @@ -4,6 +4,7 @@ import { doctorCommand, localEmbeddingDoctorStatus, padDisplayEnd, + pageSizeDoctorLines, } from '../../src/cli/doctor.js'; describe('doctor output formatting', () => { @@ -127,6 +128,42 @@ describe('doctor embedding-runtime support status', () => { }); }); +describe('doctor page-size lines (#1231, #2424 review)', () => { + it('warns on a non-4K page size with a pre-0.18.0 @ladybugdb/core', () => { + const lines = pageSizeDoctorLines(16384, '0.17.1'); + expect(lines).toHaveLength(2); + expect(lines[0]).toBe(` ${padDisplayEnd('page size', 10)}16384`); + // Byte-identical to the pre-extraction inline rendering — guards the + // helper extraction against output drift. + expect(lines[1]).toBe( + ` ${padDisplayEnd('', 10)}⚠ non-4K page size with @ladybugdb/core < 0.18.0 — ` + + `'gitnexus analyze' may fail during COPY (#1231). Upgrade gitnexus (npm install -g gitnexus@latest).`, + ); + }); + + it.each([ + ['page-size-aware LadybugDB', 16384, '0.18.0'], + ['a 4 KiB page size', 4096, '0.17.1'], + ])('prints the page size without a warning for %s', (_label, pageSize, version) => { + const lines = pageSizeDoctorLines(pageSize, version); + expect(lines).toHaveLength(1); + expect(lines[0]).toContain('page size'); + expect(lines[0]).toContain(String(pageSize)); + }); + + it('prints nothing when the page size is unknown', () => { + expect(pageSizeDoctorLines(undefined, '0.17.1')).toHaveLength(0); + }); + + it('names an unknown version instead of asserting "< 0.18.0" about it', () => { + const lines = pageSizeDoctorLines(16384, undefined); + expect(lines).toHaveLength(2); + expect(lines[1]).toContain('an unknown @ladybugdb/core version (may predate 0.18.0)'); + expect(lines[1]).not.toContain('with @ladybugdb/core < 0.18.0'); + expect(lines[1]).toContain('npm install -g gitnexus@latest'); + }); +}); + describe('doctor survives a malformed GITNEXUS_EMBEDDING_DIMS (#2385)', () => { const ENV_KEYS = [ 'GITNEXUS_EMBEDDING_URL', diff --git a/gitnexus/test/unit/lbug-config-pagesize.test.ts b/gitnexus/test/unit/lbug-config-pagesize.test.ts new file mode 100644 index 000000000..f2108d5f8 --- /dev/null +++ b/gitnexus/test/unit/lbug-config-pagesize.test.ts @@ -0,0 +1,162 @@ +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { execFileSync } from 'child_process'; +import { + _resetOsPageSizeCacheForTest, + getOsPageSize, + isLbugPageSizeFrameError, + isPageSizeAwareLadybug, +} from '../../src/core/lbug/lbug-config.js'; + +// Pass-through spy on the bare 'child_process' specifier (lbug-config.ts +// imports execFileSync from 'child_process', not 'node:child_process'). +// Real behavior is preserved by default, so the live-probe test below still +// exercises the host getconf — including the real 16 KiB pages on the +// macos-arm64 CI matrix — while failure-path tests override single calls via +// mockImplementationOnce. Recipe: sibling-clone-drift.test.ts. +vi.mock('child_process', async () => { + const actual = await vi.importActual('child_process'); + return { ...actual, execFileSync: vi.fn(actual.execFileSync) }; +}); + +const execFileSyncSpy = vi.mocked(execFileSync); + +// getOsPageSize short-circuits before any exec on win32, so every +// exec-asserting test below is POSIX-only (skipIf pairs, no if-branching). +const onWindows = process.platform === 'win32'; + +// ─── #1231: non-4K page-size frame-release matcher ────────────────────────── + +describe('isLbugPageSizeFrameError', () => { + it.each([ + [ + 'exact Raspberry Pi 5 failure (issue #1231)', + 'Buffer manager exception: Releasing physical memory associated with a frame failed with error code -1: Invalid argument.', + ], + [ + 'wrapped by the node-COPY error path', + 'COPY failed for File: Buffer manager exception: Releasing physical memory associated with a frame failed with error code -1: Invalid argument.', + ], + [ + '0.18.0 residual guard', + 'Buffer manager exception: Unsupported page size combination: frame size 4096, discard granule size 65536, frame group size 16384.', + ], + ])('matches %s', (_label, msg) => { + expect(isLbugPageSizeFrameError(msg)).toBe(true); + expect(isLbugPageSizeFrameError(new Error(msg))).toBe(true); + }); + + it.each([ + [ + 'buffer pool exhaustion (a sizing problem, not page size)', + 'Buffer manager exception: Unable to allocate memory! The buffer pool is full and no memory could be freed!', + ], + ['8TB mmap failure (#785)', 'Buffer manager exception: Mmap for size 8796093022208 failed.'], + ['WAL corruption', 'Runtime exception: Corrupted wal file. Read out invalid WAL record type.'], + ['lock contention', 'Could not set lock on file : /path/to/db'], + ['generic', 'Query failed'], + ])('does NOT match %s', (_label, msg) => { + expect(isLbugPageSizeFrameError(msg)).toBe(false); + }); + + it('handles non-string input', () => { + expect(isLbugPageSizeFrameError(undefined)).toBe(false); + expect(isLbugPageSizeFrameError(null)).toBe(false); + expect(isLbugPageSizeFrameError(42)).toBe(false); + }); +}); + +// ─── #1231: page-size-aware LadybugDB version gate ────────────────────────── + +describe('isPageSizeAwareLadybug', () => { + it.each([ + ['0.18.0', true], + ['0.18.0-dev.20260708', true], + ['0.19.2', true], + ['1.0.0', true], + ['0.17.1', false], + ['0.16.0', false], + ['0.15.4', false], + ])('%s -> %s', (version, expected) => { + expect(isPageSizeAwareLadybug(version)).toBe(expected); + }); + + it('returns false for unknown/unparseable versions (err on showing the upgrade hint)', () => { + expect(isPageSizeAwareLadybug(undefined)).toBe(false); + expect(isPageSizeAwareLadybug('')).toBe(false); + expect(isPageSizeAwareLadybug('unknown')).toBe(false); + expect(isPageSizeAwareLadybug('v0.18.0')).toBe(false); + }); +}); + +// ─── #1231: OS page-size probe ─────────────────────────────────────────────── + +describe('getOsPageSize', () => { + afterEach(() => { + _resetOsPageSizeCacheForTest(); + execFileSyncSpy.mockClear(); + }); + + it.skipIf(onWindows)('returns a positive power-of-two page size on POSIX platforms', () => { + const pageSize = getOsPageSize(); + expect(pageSize).toBeDefined(); + expect(Number.isInteger(pageSize)).toBe(true); + // Every real page size is a power of two (4K, 16K, 64K, ...). log2 of a + // power of two is an integer; `?? 0` narrows without a cast or branch and + // Math.log2(0) is -Infinity, which fails isInteger. + expect(Number.isInteger(Math.log2(pageSize ?? 0))).toBe(true); + }); + + it.skipIf(!onWindows)('returns undefined on Windows without forking', () => { + expect(getOsPageSize()).toBeUndefined(); + expect(execFileSyncSpy).not.toHaveBeenCalled(); + }); + + it.skipIf(onWindows)('returns undefined when the probe cannot exec', () => { + execFileSyncSpy.mockImplementationOnce(() => { + throw new Error('spawnSync getconf ENOENT'); + }); + expect(getOsPageSize()).toBeUndefined(); + }); + + it.skipIf(onWindows)('returns undefined on non-numeric probe output', () => { + execFileSyncSpy.mockImplementationOnce(() => 'unlimited'); + expect(getOsPageSize()).toBeUndefined(); + }); + + it.skipIf(onWindows)('returns undefined on empty probe output', () => { + execFileSyncSpy.mockImplementationOnce(() => ''); + expect(getOsPageSize()).toBeUndefined(); + }); + + it.skipIf(onWindows)('execs getconf with a SIGKILL-hardened 2s timeout', () => { + getOsPageSize(); + expect(execFileSyncSpy).toHaveBeenCalledWith( + 'getconf', + ['PAGE_SIZE'], + expect.objectContaining({ timeout: 2000, killSignal: 'SIGKILL' }), + ); + }); + + it.skipIf(onWindows)('returns undefined when the probe times out', () => { + // execFileSync's timeout kill surfaces as a throw with `signal` set and + // `status` null — the fail-safe must hold for the kill path too. + execFileSyncSpy.mockImplementationOnce(() => { + const err = new Error('spawnSync getconf ETIMEDOUT') as Error & { + signal: string; + status: null; + }; + err.signal = 'SIGKILL'; + err.status = null; + throw err; + }); + expect(getOsPageSize()).toBeUndefined(); + }); + + it.skipIf(onWindows)('probes at most once per process (cached)', () => { + expect(getOsPageSize()).toBe(getOsPageSize()); + expect(execFileSyncSpy).toHaveBeenCalledTimes(1); + _resetOsPageSizeCacheForTest(); + getOsPageSize(); + expect(execFileSyncSpy).toHaveBeenCalledTimes(2); + }); +});