fix(llm): load persisted settings lazily, not in the constructor

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<string, unknown>): 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 })
+    }
+  })
+})

src/main/llm.ts

@@ -149,8 +149,30 @@
     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 @@
   /** 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 @@
   /** 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 @@
    *  `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 @@
   /** 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<void> {
+    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 @@
 
   /** 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 @@
   // 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 @@
   }
 
   modelsExist(): boolean {
+    this.ensureLoaded()
     this.resolveModel()
     return fs.existsSync(this.modelPath)
   }
@@ -510,6 +540,7 @@
    *  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 @@
   }
 
   async init(): Promise<void> {
+    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