Release v2.61.1

.changelog/v2.61.1.md

@@ -0,0 +1,22 @@
+# Release v2.61.1
+
+Released: 2026-09-09
+
+## Highlights
+
+- Backup scheduling now retries rejected or incomplete registrations, so a transient or invalid configuration does not silently disable future backups.
+- Disabled backup settings remain inexpensive to reconcile while confirmed schedules continue to avoid unnecessary re-registration.
+- Code review backend loading is now selective and preserves the capability fallback when an optional manager cannot initialize.
+
+## Changed
+
+- Backup scheduling caches only confirmed disabled or runnable configurations and treats failed replacements as retryable.
+- Review backend managers load only when the selected backend requires them.
+
+## Fixed
+
+- Review capability discovery now remains available when loading a backend manager fails.
+
+## Full Changelog
+
+**Full Diff**: https://github.com/atomantic/PortOS/compare/v2.61.0...v2.61.1

package.json

@@ -1,6 +1,6 @@
 {
   "name": "portos",
-  "version": "2.61.0",
+  "version": "2.61.1",
   "private": true,
   "description": "Local dev machine App OS portal",
   "author": "Adam Eivy (@antic|@atomantic)",

server/lib/importScoping.test.js

@@ -147,6 +147,12 @@ describe('Tailcat shared owners stay independent of forwarding (#6570)', () => {
 // [entry, target, why, specifier] — same first three columns as NARROWED above,
 // plus the specifier the call site must still name in its `await import()`.
 const DEFERRED = [
+  ['services/codeReview.js', 'services/lmStudioManager.js',
+    'reads the live endpoint only for a selected LM Studio review', './lmStudioManager.js'],
+  ['services/codeReview.js', 'services/ollamaManager.js',
+    'reads endpoints and model capabilities only for an Ollama review', './ollamaManager.js'],
+  ['services/codeReview.js', 'services/mtplxServerManager.js',
+    'resolves the managed daemon only for an MTPLX review', './mtplxServerManager.js'],
   ['services/agentManagement.js', 'lib/privateSecuritySandbox.js',
     'loads sandbox cleanup only for private assessments', '../lib/privateSecuritySandbox.js'],
   ['services/cos.js', 'services/persistentMindAdapter.js',

server/services/backupScheduler.js

@@ -16,10 +16,9 @@ import { resolveBackupConfig } from '../lib/backupConfig.js';
 
 const EVENT_ID = 'backup-daily';
 
-// Registration state, so an unrelated settings save is a cheap no-op and a
-// cancel only fires when a cron is actually registered.
-let registered = false;
-let lastSignature = null;
+// Only confirmed disabled or runnable configurations are cached.
+// null means stopped or failed, so identical inputs can retry.
+let reconciliationState = null;
 
 /**
  * The registration-affecting slice of settings: `null` when backup scheduling
@@ -47,31 +46,32 @@ export async function syncBackupSchedule(settings) {
   // Only `cron` + `timezone` + active/inactive are baked into the registration;
   // destPath and the exclude lists are re-read by the handler on every run.
   const signature = JSON.stringify({ active: Boolean(inputs), cron: inputs?.cron ?? null, tz: timezone });
-  if (signature === lastSignature) return registered;
+  if (signature === reconciliationState?.signature) return reconciliationState.kind === 'scheduled';
 
   if (!inputs) {
-    lastSignature = signature;
-    if (registered) {
+    if (reconciliationState?.kind === 'scheduled') {
       cancel(EVENT_ID);
-      registered = false;
       console.log('💾 Backup scheduler: disabled or destPath cleared — cron cancelled');
     } else {
       console.log('💾 Backup scheduler: disabled or no destPath configured — nothing scheduled');
     }
+    reconciliationState = { kind: 'disabled', signature };
     return false;
   }
 
+  reconciliationState = attemptRegistration(inputs, timezone, signature);
+  return reconciliationState?.kind === 'scheduled';
+}
+
+function attemptRegistration(inputs, timezone, signature) {
   // `schedule()` replaces an event with the same id, so a changed cron
   // expression cleanly re-registers. destPath, excludePaths and
   // disabledDefaultExcludes are re-read inside the handler so toggles saved in
   // the Settings UI take effect on the next scheduled run.
-  //
-  // try/catch (allowed here — this runs on the settings event bus / at boot,
-  // outside the request lifecycle): schedule() CANCELS the existing event
-  // before it validates the new cron, so a malformed expression tears down a
-  // working timer and throws. Leave `lastSignature` unset in that case so the
-  // next save — even one that re-submits the same value — retries instead of
-  // short-circuiting on a registration that never happened.
+  // schedule() cancels the old event before validating its replacement.
+  // A throw or missing next run leaves no confirmed state to cache.
+  // This catch owns failures at boot / on the settings event bus, outside
+  // the request lifecycle where errors would otherwise propagate to a caller.
   let event;
   try {
     event = schedule({
@@ -98,10 +98,8 @@ export async function syncBackupSchedule(settings) {
       metadata: { source: 'backupScheduler' }
     });
   } catch (err) {
-    registered = false;
-    lastSignature = null;
     console.error(`❌ Backup scheduler: cron "${inputs.cron}" rejected — no backup scheduled: ${err.message}`);
-    return false;
+    return null;
   }
 
   // Not every bad expression throws: a five-field cron with an out-of-range
@@ -110,16 +108,11 @@ export async function syncBackupSchedule(settings) {
   // retry once the user corrects it.
   if (!event?.nextRunAt) {
     cancel(EVENT_ID);
-    registered = false;
-    lastSignature = null;
     console.error(`❌ Backup scheduler: cron "${inputs.cron}" has no next run time — no backup scheduled`);
-    return false;
+    return null;
   }
-
-  registered = true;
-  lastSignature = signature;
   console.log(`💾 Backup scheduler: registered daily backup at cron "${inputs.cron}"`);
-  return true;
+  return { kind: 'scheduled', signature };
 }
 
 // Re-sync on every settings save rather than from the settings route — keeps
@@ -144,7 +137,6 @@ export async function startBackupScheduler() {
  */
 export function stopBackupScheduler() {
   cancel(EVENT_ID);
-  registered = false;
-  lastSignature = null;
+  reconciliationState = null;
   console.log('💾 Backup scheduler: stopped');
 }

server/services/backupScheduler.test.js

@@ -50,7 +50,7 @@ vi.mock('./userTimezone.js', () => ({
 import { schedule, cancel } from './eventScheduler.js';
 import { getSettings } from './settings.js';
 import { runBackup } from './backup.js';
-import { startBackupScheduler, stopBackupScheduler } from './backupScheduler.js';
+import { startBackupScheduler, stopBackupScheduler, syncBackupSchedule } from './backupScheduler.js';
 
 describe('startBackupScheduler', () => {
   beforeEach(() => {
@@ -294,3 +294,37 @@ describe('backup schedule defaults (#6632)', () => {
     });
   });
 });
+
+describe('confirmed backup schedule lifecycle', () => {
+  beforeEach(() => {
+    stopBackupScheduler();
+    vi.clearAllMocks();
+  });
+
+  it('applies disable once and retries the same schedule after stopping', async () => {
+    const settings = { backup: { enabled: true, destPath: '/dest' } };
+    getSettings.mockResolvedValue(settings);
+    expect(await startBackupScheduler()).toBe(true);
+    expect(await syncBackupSchedule({ backup: { enabled: false } })).toBe(false);
+    expect(await syncBackupSchedule({ backup: { enabled: false } })).toBe(false);
+    expect(cancel).toHaveBeenCalledTimes(1);
+    expect(await startBackupScheduler()).toBe(true);
+    stopBackupScheduler();
+    expect(await startBackupScheduler()).toBe(true);
+    expect(schedule).toHaveBeenCalledTimes(3);
+  });
+
+  it.each(['throw', 'no next run'])('forgets a successful signature after replacement fails with %s', async (failure) => {
+    const original = { backup: { enabled: true, destPath: '/dest', cronExpression: '0 1 * * *' } };
+    expect(await syncBackupSchedule(original)).toBe(true);
+    schedule.mockImplementationOnce(() => {
+      if (failure === 'throw') throw new Error('Rejected replacement');
+      return { id: 'backup-daily', nextRunAt: null };
+    });
+    expect(await syncBackupSchedule({
+      backup: { ...original.backup, cronExpression: '0 2 * * *' }
+    })).toBe(false);
+    expect(await syncBackupSchedule(original)).toBe(true);
+    expect(schedule).toHaveBeenCalledTimes(3);
+  });
+});

server/services/codeReview.backendLoading.test.js

@@ -0,0 +1,28 @@
+import { afterEach, expect, it, vi } from 'vitest';
+import { mockJsonResponse } from '../lib/testHelper.js';
+
+vi.mock('./settings.js', () => ({
+  getSettings: async () => ({}),
+  settingsEvents: { on: vi.fn() },
+}));
+vi.mock('./lmStudioManager.js', () => { throw new Error('Manager unavailable'); });
+vi.mock('./ollamaManager.js', () => { throw new Error('Manager unavailable'); });
+
+import { getCodeReviewDefaults, runLocalCodeReview } from './codeReview.js';
+
+afterEach(() => vi.unstubAllGlobals());
+
+it('reads defaults without managers and reviews an explicit endpoint when capability loading fails', async () => {
+  expect(await getCodeReviewDefaults()).toMatchObject({ reviewers: [] });
+  const fetchMock = vi.fn().mockResolvedValue(mockJsonResponse({
+    choices: [{ message: { content: 'No findings.' } }],
+  }));
+  vi.stubGlobal('fetch', fetchMock);
+
+  expect(await runLocalCodeReview({
+    backend: 'ollama', model: 'example-coder', effort: 'high',
+    diff: 'example diff', baseUrl: 'http://localhost:11434',
+  })).toMatchObject({ ok: true, findings: 'No findings.' });
+  expect(fetchMock).toHaveBeenCalledTimes(1);
+  expect(JSON.parse(fetchMock.mock.calls[0][1].body)).toMatchObject({ reasoning_effort: 'high' });
+});

server/services/codeReview.js

@@ -51,11 +51,6 @@ import {
   resolveGoalFidelityConfig,
 } from '../lib/goalFidelity.js'
 import { getSettings, settingsEvents } from './settings.js'
-import { getBaseUrl as getLmStudioBaseUrl } from './lmStudioManager.js'
-import {
-  getBaseUrl as getOllamaBaseUrl,
-  getModelCapabilities as getOllamaModelCapabilities,
-} from './ollamaManager.js'
 
 // LM Studio (`:1234`), Ollama (`:11434`) and MTPLX (`:8000/v1`) all ship
 // OpenAI-compatible `/v1/chat/completions`. Resolve through each manager's live
@@ -64,16 +59,13 @@ import {
 // otherwise the catalog UI and the reviewer would silently desync when a user
 // relocates their install.
 //
-// Every entry is awaited at the call site, which lets MTPLX's stay a DYNAMIC
-// import. That is deliberate: `mtplxServerManager.js` pulls in the managed-daemon
-// watcher and its PM2/filesystem graph, and this module is imported by the agent
-// spawn path — a static import would put that whole graph behind every one of its
-// importers (and did break suites that partially mock `lib/fileUtils.js`). The
-// review request is a one-off HTTP call, so paying the resolve lazily costs
-// nothing.
+// Every entry is awaited at the call site. Keep all manager imports lazy:
+// defaults-only callers (agent prompting, task generation and cleanup) must not
+// load model download/install or daemon-management dependencies. Each manager
+// remains the owner of its live endpoint; only a selected backend loads it.
 const BACKEND_BASE_URLS = {
-  lmstudio: () => getLmStudioBaseUrl(),
-  ollama: () => getOllamaBaseUrl(),
+  lmstudio: async () => (await import('./lmStudioManager.js')).getBaseUrl(),
+  ollama: async () => (await import('./ollamaManager.js')).getBaseUrl(),
   mtplx: async () => (await import('./mtplxServerManager.js')).getMtplxServerEndpoint(),
 }
 
@@ -368,7 +360,9 @@ async function modelRejectsThinking(backend, model) {
   const cacheKey = thinkingCacheKey(backend, model)
   if (thinkingUnsupportedModels.get(cacheKey) === true) return true
   if (backend !== 'ollama') return false
-  const capabilities = await getOllamaModelCapabilities(model).catch(() => null)
+  const capabilities = await import('./ollamaManager.js')
+    .then(({ getModelCapabilities }) => getModelCapabilities(model))
+    .catch(() => null)
   if (!Array.isArray(capabilities) || capabilities.length === 0) return false
   if (capabilities.includes('thinking')) return false
   thinkingUnsupportedModels.set(cacheKey, true)

server/services/voice/tts-qwen3.test.js

@@ -1,6 +1,13 @@
 import { describe, it, expect } from 'vitest';
+import { resolveTestPython } from '../../lib/testHelper.js';
 import { synthesizeQwen3, listQwen3Voices } from './tts-qwen3.js';
 
+// The runner boundary is valuable when Python is installed, but Windows CI
+// does not guarantee a Python runtime. Keep the deterministic preset test
+// available everywhere and skip only the subprocess case when no runnable
+// interpreter exists.
+const testPython = resolveTestPython();
+
 describe('tts-qwen3', () => {
   it('enumerates default Qwen3 voices', async () => {
     const voices = await listQwen3Voices();
@@ -9,7 +16,7 @@ describe('tts-qwen3', () => {
     expect(voices[0]).toHaveProperty('id');
   });
 
-  it('synthesizes speech with voice design and rate controls', async () => {
+  it.skipIf(!testPython)('synthesizes speech with voice design and rate controls', async () => {
     const result = await synthesizeQwen3('This is a test of voice design synthesis.', {
       mode: 'design',
       instructions: 'warm low alto',