diff --git a/src/main/__tests__/llm-lazy-settings-load.test.ts b/src/main/__tests__/llm-lazy-settings-load.test.ts new file mode 100644 index 00000000..8bd9b7f1 --- /dev/null +++ b/src/main/__tests__/llm-lazy-settings-load.test.ts @@ -0,0 +1,110 @@ +// Regression: LLMService must read its persisted state LAZILY, not in the constructor. +// +// `llm` is a module-level singleton (`export const llm = new LLMService()`), so it is +// constructed while index.ts's IMPORTS are still evaluating — which under ESM finishes +// BEFORE index.ts's own body runs `unifyUserDataPath()` → `app.setPath('userData', …)`. +// Any path resolved during construction therefore points at the PRE-override profile. +// +// Two real consequences, both of which these tests pin: +// 1. Production: the canonical-dir migration ("My Memories" / "my-memories" → +// "Off Grid AI Desktop") has not run yet at construction, so the user's saved +// settings and active model were silently missed and replaced by defaults. +// 2. E2E/harness: an OFFGRID_USER_DATA temp profile was ignored entirely — which is +// what made `settings-sections.spec.ts` "resource mode survives a relaunch" fail. +// A probe confirmed the constructor resolving the REAL profile while +// OFFGRID_USER_DATA pointed at the temp dir. +// +// Writes never had the bug: `persist()` goes through the `settingsFile` getter, which +// resolves late. These tests assert the READ side now behaves the same way, by doing +// what production does — construct FIRST, point the data dir somewhere SECOND. +import { describe, it, expect, beforeEach, afterEach } from 'vitest' +import fs from 'fs' +import os from 'os' +import path from 'path' +import { LLMService } from '../llm' +import { configureRuntime } from '../runtime-env' + +let tmp: string + +/** Write an llm-settings.json into the models dir of a data dir, as `persist()` would. */ +const seedSettings = (dataDir: string, settings: Record): void => { + const modelsDir = path.join(dataDir, 'models') + fs.mkdirSync(modelsDir, { recursive: true }) + fs.writeFileSync(path.join(modelsDir, 'llm-settings.json'), JSON.stringify(settings)) +} + +beforeEach(() => { + tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'offgrid-llm-lazy-')) +}) + +afterEach(() => { + // Release the override so a later test isn't pinned to a deleted temp dir. + configureRuntime({ dataDir: undefined }) + fs.rmSync(tmp, { recursive: true, force: true }) +}) + +describe('LLMService reads persisted settings lazily (not at construction)', () => { + it('picks up a data dir configured AFTER the instance was constructed', () => { + // Construct FIRST — mirrors the module-level singleton being built during imports. + const svc = new LLMService() + // ...then point the runtime at the profile, as index.ts's body does later. + seedSettings(tmp, { performanceMode: 'extreme', temperature: 0.42 }) + configureRuntime({ dataDir: tmp }) + + const s = svc.getSettings() + expect(s.performanceMode).toBe('extreme') + expect(s.temperature).toBe(0.42) + }) + + it('survives the relaunch shape: persisted mode is read back by a fresh instance', () => { + // What settings-sections.spec.ts "resource mode survives a relaunch" exercises: + // one process writes the mode, the next process constructs and must read it back. + seedSettings(tmp, { performanceMode: 'conservative' }) + const relaunched = new LLMService() + configureRuntime({ dataDir: tmp }) + + expect(relaunched.getSettings().performanceMode).toBe('conservative') + }) + + it('loads once and does not re-read after the first access', () => { + seedSettings(tmp, { performanceMode: 'conservative' }) + const svc = new LLMService() + configureRuntime({ dataDir: tmp }) + expect(svc.getSettings().performanceMode).toBe('conservative') + + // A later on-disk edit must NOT leak in: the load is once-only, so in-memory state + // stays authoritative until something explicitly persists. This guards against + // turning the lazy guard into a read-on-every-call, which would re-read the file + // on every getSettings and clobber unsaved in-memory changes. + seedSettings(tmp, { performanceMode: 'extreme' }) + expect(svc.getSettings().performanceMode).toBe('conservative') + }) + + it('falls back to defaults when the profile has no settings file', () => { + const svc = new LLMService() + configureRuntime({ dataDir: tmp }) // seeded with nothing + expect(svc.getSettings().performanceMode).toBe('balanced') + }) + + // The direct guard on the defect, stated behaviourally rather than by spying on fs: + // if construction reads eagerly, it reads the profile configured AT THAT MOMENT. + // Point the runtime at profile A, construct, then switch to profile B before first + // use — a lazy reader returns B, an eager one returns A. This is the exact shape of + // the production bug (construct during imports, real profile chosen afterwards). + it('reads the profile configured at FIRST USE, not the one present at construction', () => { + const other = fs.mkdtempSync(path.join(os.tmpdir(), 'offgrid-llm-lazy-other-')) + try { + seedSettings(other, { performanceMode: 'extreme' }) // profile A + seedSettings(tmp, { performanceMode: 'conservative' }) // profile B + + configureRuntime({ dataDir: other }) // A is current... + const svc = new LLMService() // ...at construction + configureRuntime({ dataDir: tmp }) // the override lands afterwards + + // Eager construction would have pinned 'extreme' from profile A. + expect(svc.getSettings().performanceMode).toBe('conservative') + } finally { + fs.rmSync(other, { recursive: true, force: true }) + } + }) +}) diff --git a/src/main/llm.ts b/src/main/llm.ts index bfec6f99..3057ee1b 100644 --- a/src/main/llm.ts +++ b/src/main/llm.ts @@ -149,8 +149,30 @@ export class LLMService { return path.join(getModelsDir(), 'llm-settings.json') } - constructor() { + /** Whether the persisted state (active model + user settings) has been read yet. */ + private loaded = false + + /** Read persisted state ONCE, on first use — never from the constructor. + * + * `llm` is a module-level singleton, so it is constructed while index.ts's IMPORTS + * are still evaluating, which under ESM completes before index.ts's own body runs + * `unifyUserDataPath()` → `app.setPath('userData', …)`. Resolving paths at + * construction therefore reads the PRE-override profile: an OFFGRID_USER_DATA + * harness dir is ignored, and in production the canonical-dir migration ("My + * Memories" / "my-memories" → "Off Grid AI Desktop") has not happened yet, so the + * user's active model and saved settings are silently missed and replaced by + * defaults. Writes never had this bug — `persist()` goes through the settingsFile + * getter, which resolves late. This is exactly the hazard the activeModelFile / + * settingsFile getters were introduced to avoid; calling resolveModel() and reading + * the settings file from the constructor defeated them. */ + private ensureLoaded(): void { + if (this.loaded) return + this.loaded = true this.resolveModel() + this.loadPersistedSettings() + } + + private loadPersistedSettings(): void { try { const s = JSON.parse(fs.readFileSync(this.settingsFile, 'utf-8')) if (typeof s.temperature === 'number') this.temperature = s.temperature @@ -205,6 +227,7 @@ export class LLMService { /** The model's trained context window, or null if unknown — exposed so the UI can offer the * slider up to the model's own maximum instead of a hardcoded cap. */ modelMaxContext(): number | null { + this.ensureLoaded() return this.trainedContext() } @@ -258,10 +281,12 @@ export class LLMService { /** The EFFECTIVE (RAM-clamped) context window the server is actually running * with — the real ceiling for prompt + tools + answer. */ effectiveContextSize(): number { + this.ensureLoaded() return this.safeCtxSize(this.ctxSize) } getSettings(): LlmSettings { + this.ensureLoaded() return { temperature: this.temperature, ctxSize: this.ctxSize, @@ -289,6 +314,7 @@ export class LLMService { * `buildLaunchArgs` (single source of truth) after applying the impure RAM clamp, * so `_doInit` and tests build args the same way. */ launchArgs(): string[] { + this.ensureLoaded() return this.launchArgsFor(this.safeCtxSize(this.ctxSize), this.gpuLayers) } @@ -352,6 +378,7 @@ export class LLMService { /** Update inference settings; respawns the server if any launch-time arg changed * (context, KV-cache type, flash-attn, GPU layers, threads, batch). */ async setSettings(s: LlmSettings): Promise { + this.ensureLoaded() // Granular launch-time fields the user sets in THIS patch become pinned: a mode // preset (now or on a future restart / mode re-pick) must NOT clobber them. Pin // BEFORE applying the preset so an explicit q8_0 in the same patch survives. @@ -453,6 +480,7 @@ export class LLMService { /** Switch the active model without terminating a generation already using it. */ reloadModel(): void { + this.ensureLoaded() if (this.activeGenerations > 0) { this.modelReloadPending = true return @@ -481,6 +509,7 @@ export class LLMService { // on mmproj wrongly kept "Setup Required" up for an activated vision model.) /** Whether the active chat model can read images (has a vision projector / mmproj). */ hasVision(): boolean { + this.ensureLoaded() this.resolveModel() return !!this.mmProjPath && fs.existsSync(this.mmProjPath) } @@ -496,6 +525,7 @@ export class LLMService { } modelsExist(): boolean { + this.ensureLoaded() this.resolveModel() return fs.existsSync(this.modelPath) } @@ -510,6 +540,7 @@ export class LLMService { * loaded it yet (otherwise an idle/headless gateway reports no chat model). * Returns null when no model is downloaded. */ activeModelInfo(): { id: string; vision: boolean } | null { + this.ensureLoaded() this.resolveModel() if (!fs.existsSync(this.modelPath)) return null let id = path.basename(this.modelPath) @@ -531,6 +562,7 @@ export class LLMService { } async init(): Promise { + this.ensureLoaded() if (this.paused) { // A chat/tool turn needs the LLM NOW, but it's paused for a resident image // server (unified memory can't hold both). Ask the image server to evict