diff --git a/.changelog/v2.61.1.md b/.changelog/v2.61.1.md new file mode 100644 index 0000000000..1362c48ed3 --- /dev/null +++ b/.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 diff --git a/package-lock.json b/package-lock.json index cb95ec0e3a..f44b2a56b0 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "portos", - "version": "2.61.0", + "version": "2.61.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "portos", - "version": "2.61.0", + "version": "2.61.1", "license": "MIT", "dependencies": { "pm2": "7.0.4" diff --git a/package.json b/package.json index eae3819d93..101de968e1 100644 --- a/package.json +++ b/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)", diff --git a/server/lib/importScoping.test.js b/server/lib/importScoping.test.js index 148edbdd25..30d09ca8fe 100644 --- a/server/lib/importScoping.test.js +++ b/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', diff --git a/server/services/backupScheduler.js b/server/services/backupScheduler.js index cb7e3872cc..c54b9ae2e1 100644 --- a/server/services/backupScheduler.js +++ b/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'); } diff --git a/server/services/backupScheduler.test.js b/server/services/backupScheduler.test.js index 4250c6576d..58df30eac0 100644 --- a/server/services/backupScheduler.test.js +++ b/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); + }); +}); diff --git a/server/services/codeReview.backendLoading.test.js b/server/services/codeReview.backendLoading.test.js new file mode 100644 index 0000000000..11f4b54ba8 --- /dev/null +++ b/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' }); +}); diff --git a/server/services/codeReview.js b/server/services/codeReview.js index fa8fe9c788..3cd82c0402 100644 --- a/server/services/codeReview.js +++ b/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) diff --git a/server/services/voice/tts-qwen3.test.js b/server/services/voice/tts-qwen3.test.js index b885794209..ff1fdf2a7a 100644 --- a/server/services/voice/tts-qwen3.test.js +++ b/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',