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/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)