diff --git a/src/main/text-generation/commit-message-model-discovery-policy.ts b/src/main/text-generation/commit-message-model-discovery-policy.ts index 26debdd0619..94fb6c81e49 100644 --- a/src/main/text-generation/commit-message-model-discovery-policy.ts +++ b/src/main/text-generation/commit-message-model-discovery-policy.ts @@ -57,12 +57,9 @@ export function finalizeModelDiscoveryOutput( } return { success: false, error: `${spec.label} returned no available models.` } } - const defaultModelId = - 'configuredDefaultModelId' in spec && typeof spec.configuredDefaultModelId === 'string' - ? spec.configuredDefaultModelId - : models.some((model) => model.id === spec.defaultModelId) - ? spec.defaultModelId - : models[0].id + const defaultModelId = models.some((model) => model.id === spec.defaultModelId) + ? spec.defaultModelId + : models[0].id return staticModelDiscoveryResult(spec, models, defaultModelId, 'probe') } diff --git a/src/main/text-generation/omp-model-discovery-policy.test.ts b/src/main/text-generation/omp-model-discovery-policy.test.ts new file mode 100644 index 00000000000..3b70966f141 --- /dev/null +++ b/src/main/text-generation/omp-model-discovery-policy.test.ts @@ -0,0 +1,25 @@ +import { expect, it } from 'vitest' +import { getAgentModelProbeSpec } from '../../shared/agent-model-probe-spec' +import { finalizeModelDiscoveryOutput } from './commit-message-model-discovery-policy' + +const output = JSON.stringify({ + models: [ + { + provider: 'provider', + id: 'exact-model', + selector: 'provider/exact-model', + name: 'Exact model' + } + ] +}) +it('keeps the OMP configured default out of generic terminal discovery results', () => { + const spec = getAgentModelProbeSpec('omp') + if (!spec) { + throw new Error('Missing OMP probe spec') + } + expect(finalizeModelDiscoveryOutput(spec, output, '', 0)).toMatchObject({ + success: true, + defaultModelId: 'provider/exact-model', + models: [{ id: 'provider/exact-model' }] + }) +}) diff --git a/src/shared/agent-model-probe-spec.ts b/src/shared/agent-model-probe-spec.ts index 53f19af2847..583262217ab 100644 --- a/src/shared/agent-model-probe-spec.ts +++ b/src/shared/agent-model-probe-spec.ts @@ -4,10 +4,7 @@ import type { TuiAgent } from './tui-agent' /** Why: model discovery reads only these fields; excluding the prompt-delivery * half is what keeps a probe-only agent out of the commit-message registry. */ -export type AgentModelProbeSpec = Omit< - CommitMessageAgentSpec, - 'promptDelivery' | 'buildArgs' | 'configuredDefaultModelId' -> +export type AgentModelProbeSpec = Omit /** Agents that support model discovery but are not commit-message agents. */ const MODEL_DISCOVERY_ONLY_SPECS: Partial> = { diff --git a/src/shared/commit-message-agent-spec.ts b/src/shared/commit-message-agent-spec.ts index 76f61d3f93c..7a4145a7630 100644 --- a/src/shared/commit-message-agent-spec.ts +++ b/src/shared/commit-message-agent-spec.ts @@ -67,8 +67,6 @@ export type CommitMessageAgentSpec = { } models: CommitMessageModel[] defaultModelId: string - /** OMP config default is a runtime setting, not a selectable discovered model. */ - configuredDefaultModelId?: string } export type CommitMessageModelCapability = { @@ -112,8 +110,7 @@ export const COMMIT_MESSAGE_AGENT_SPECS: Partial { it.each(['escape', 'literal'] as const)( 'plans recipe model overrides with %s path parsing', (backslash) => { - const prompt = 'diff --git a/a b/a\n' + 'large patch\n'.repeat(10000) + const prompt = `diff --git a/a b/a\n${'large patch\n'.repeat(10000)}` const result = planCommitMessageGeneration( { agentId: 'omp', diff --git a/src/shared/omp-source-control-discovery.test.ts b/src/shared/omp-source-control-discovery.test.ts new file mode 100644 index 00000000000..432810914fc --- /dev/null +++ b/src/shared/omp-source-control-discovery.test.ts @@ -0,0 +1,75 @@ +import { describe, expect, it } from 'vitest' +import { getDefaultSettings } from './constants' +import { getCommitMessageAgentSpec } from './commit-message-agent-spec' +import { + resolveSourceControlAiForOperation, + normalizeSourceControlAiSettings +} from './source-control-ai' + +for (const hostKey of ['local', 'ssh:omp-host']) { + describe(`OMP discovery preserves generation choices on ${hostKey}`, () => { + it.each([undefined, 'default', 'provider/exact-model'])( + 'retains selected model %s when a discovered catalog is cached', + (selectedModel) => { + const spec = getCommitMessageAgentSpec('omp') + if (!spec) { + throw new Error('Missing OMP spec') + } + const models = + spec.modelDiscovery?.parse( + JSON.stringify({ + models: [ + { + provider: 'provider', + id: 'other-model', + selector: 'provider/other-model', + name: 'Other model' + }, + { + provider: 'provider', + id: 'exact-model', + selector: 'provider/exact-model', + name: 'Exact model' + } + ] + }) + ) ?? [] + expect(models.map((model) => model.id)).toEqual([ + 'provider/other-model', + 'provider/exact-model' + ]) + const settings = getDefaultSettings('/disposable-omp-home') + settings.defaultTuiAgent = 'omp' + const config = normalizeSourceControlAiSettings( + settings.sourceControlAi, + settings.commitMessageAi + ) + config.selectedModelByAgentByHost = selectedModel + ? { [hostKey]: { omp: selectedModel } } + : {} + settings.sourceControlAi = { + ...config, + discoveredModelsByAgentByHost: { [hostKey]: { omp: models } } + } + for (const operation of ['commitMessage', 'pullRequest', 'branchName'] as const) { + const resolved = resolveSourceControlAiForOperation({ + settings, + operation, + discoveryHostKey: hostKey + }) + expect(resolved.ok).toBe(true) + if (!resolved.ok) { + throw new Error(resolved.error) + } + expect(resolved.value.params.model).toBe(selectedModel ?? 'default') + const args = spec.buildArgs({ prompt: '', model: resolved.value.params.model }) + if (selectedModel === 'provider/exact-model') { + expect(args.slice(-2)).toEqual(['--model', 'provider/exact-model']) + } else { + expect(args).not.toContain('--model') + } + } + } + ) + }) +} diff --git a/tests/tools/omp-source-control-ui/README.md b/tests/tools/omp-source-control-ui/README.md new file mode 100644 index 00000000000..7b320f0a8ea --- /dev/null +++ b/tests/tools/omp-source-control-ui/README.md @@ -0,0 +1,28 @@ +# OMP Source Control AI settings proof + +Run `ORCA_BACKGROUND_LAUNCH=1 node tests/tools/omp-source-control-ui/run.mjs`. +The runner rebuilds the existing background-safe Electron fixture and the production +`CommitMessageAiPane` with its real styles. It uses disposable HOME/ZDOTDIR/userData, +never reveals a window, and closes its own app after recording evidence under +`.bench-fixtures/omp-source-control-*/`. + +DOM assertions and screenshots cover: + +- OMP appears in the commit-message agent menu and can be selected. +- Selecting OMP keeps CLI arguments empty so OMP uses its configured provider/model. +- An explicit `--model provider/exact-model` argument can be saved and remains in the + input after the save completes. + +The settings persistence adapter is in-memory. This is a rendered production +component check, not a full app IPC, restart-persistence, or generation test. No +generator is called or mocked. Current Source Control AI settings expose action +recipes with CLI arguments; they do not have a Discover Models button. The separate +`omp-model-discovery-policy.test.ts` and `omp-source-control-discovery.test.ts` +regressions cover real discovery-result policy and production generation selection +with cached models, including SSH host keys. The configured-default sentinel stays +out of generic terminal catalogs while generation without a model override still +uses OMP configuration. + +`tests/tools/omp-source-control-runtime-smoke.mjs` separately validates argv against +the actual OMP argument parser in a read-only reference checkout. It makes no model +call and must not be described as real generated output. diff --git a/tests/tools/omp-source-control-ui/fixture.css b/tests/tools/omp-source-control-ui/fixture.css new file mode 100644 index 00000000000..7087d4088e2 --- /dev/null +++ b/tests/tools/omp-source-control-ui/fixture.css @@ -0,0 +1,5 @@ +@import '../../../src/renderer/src/assets/main.css'; +@source './fixture.tsx'; +@source '../../../src/renderer/src/components/settings'; +@source '../../../src/renderer/src/components/source-control'; +@source '../../../src/renderer/src/components/ui'; diff --git a/tests/tools/omp-source-control-ui/fixture.tsx b/tests/tools/omp-source-control-ui/fixture.tsx new file mode 100644 index 00000000000..131da1fe316 --- /dev/null +++ b/tests/tools/omp-source-control-ui/fixture.tsx @@ -0,0 +1,46 @@ +import React from 'react' +import { createRoot } from 'react-dom/client' +import { getDefaultSettings } from '../../../src/shared/constants' +import type { GlobalSettings } from '../../../src/shared/global-settings-types' +import { CommitMessageAiPane } from '../../../src/renderer/src/components/settings/CommitMessageAiPane' +import { TooltipProvider } from '../../../src/renderer/src/components/ui/tooltip' +import { useAppStore } from '../../../src/renderer/src/store' +import './fixture.css' + +const settings = getDefaultSettings('/disposable-omp-ui-home') +settings.sourceControlAi = { + enabled: true, + agentId: null, + selectedModelByAgent: {}, + selectedThinkingByModel: {}, + instructionsByOperation: {}, + actions: {}, + customAgentCommand: '' +} +const updateSettings = async (patch: Partial): Promise => { + const current = useAppStore.getState().settings + if (!current) { + throw new Error('Settings fixture not initialized') + } + useAppStore.setState({ settings: { ...current, ...patch } }) +} +useAppStore.setState({ settings, repos: [], settingsSearchQuery: '', updateSettings }) + +function Fixture(): React.JSX.Element { + const current = useAppStore((state) => state.settings) + if (!current) { + throw new Error('Missing fixture settings') + } + return ( + +
+ +
+
+ ) +} +const root = document.getElementById('root') +if (!root) { + throw new Error('Missing fixture root') +} +createRoot(root).render() diff --git a/tests/tools/omp-source-control-ui/index.html b/tests/tools/omp-source-control-ui/index.html new file mode 100644 index 00000000000..f17a0128b22 --- /dev/null +++ b/tests/tools/omp-source-control-ui/index.html @@ -0,0 +1,11 @@ + + + + + OMP Source Control AI settings proof + + +
+ + + diff --git a/tests/tools/omp-source-control-ui/run.mjs b/tests/tools/omp-source-control-ui/run.mjs new file mode 100644 index 00000000000..a43efeca6be --- /dev/null +++ b/tests/tools/omp-source-control-ui/run.mjs @@ -0,0 +1,74 @@ +import { _electron as electron, expect } from '@stablyai/playwright-test' +import { build as buildMain } from 'esbuild' +import { build as buildRenderer } from 'vite' +import tailwindcss from '@tailwindcss/vite' +import { mkdirSync, mkdtempSync, writeFileSync } from 'node:fs' +import path from 'node:path' +import { fileURLToPath, pathToFileURL } from 'node:url' + +const root = fileURLToPath(new URL('../../../', import.meta.url)) +const parent = path.join(root, '.bench-fixtures') +mkdirSync(parent, { recursive: true }) +const output = mkdtempSync(path.join(parent, 'omp-source-control-')) +const home = path.join(output, 'home') +mkdirSync(home) +const main = path.join(output, 'main.cjs') +await buildMain({ + entryPoints: [path.join(root, 'tests/tools/benchmarks/spinner-rendering/main.ts')], + outfile: main, + bundle: true, + platform: 'node', + format: 'cjs', + external: ['electron'] +}) +await buildRenderer({ + configFile: false, + root: import.meta.dirname, + base: './', + logLevel: 'silent', + plugins: [tailwindcss()], + resolve: { alias: { '@': path.join(root, 'src/renderer/src') } }, + build: { outDir: path.join(output, 'renderer'), emptyOutDir: true } +}) +const { ELECTRON_RUN_AS_NODE: _node, ...env } = process.env +const app = await electron.launch({ + args: [main], + env: { ...env, HOME: home, ZDOTDIR: home, ORCA_BACKGROUND_LAUNCH: '1' } +}) +const report = { + scope: + 'Production CommitMessageAiPane in hidden Electron; in-memory settings persistence; no generator invoked', + errors: [] +} +try { + const page = await app.firstWindow() + page.on('pageerror', (error) => report.errors.push(error.message)) + await page.goto(pathToFileURL(path.join(output, 'renderer/index.html')).href) + const firstAgent = page.getByRole('combobox').first() + await expect(firstAgent).toBeVisible() + await firstAgent.click() + const omp = page.getByRole('option', { name: 'OMP', exact: true }) + await expect(omp).toBeVisible() + await page.screenshot({ path: path.join(output, 'omp-selectable.png') }) + await omp.click() + await expect(firstAgent).toContainText('OMP') + const firstArgs = page.locator('input').first() + await expect(firstArgs).toHaveValue('') + await page.screenshot({ path: path.join(output, 'omp-configured-default.png') }) + await firstArgs.fill('--model provider/exact-model') + const firstSave = page.getByRole('button', { name: 'Save', exact: true }).first() + await firstSave.click() + await expect(firstSave).toBeDisabled() + await expect(firstArgs).toHaveValue('--model provider/exact-model') + await page.screenshot({ path: path.join(output, 'omp-explicit-model-saved.png') }) + report.hidden = await app.evaluate(({ BrowserWindow }) => + BrowserWindow.getAllWindows().every((window) => !window.isVisible()) + ) + expect(report.hidden).toBe(true) + expect(report.errors).toEqual([]) + report.passed = true +} finally { + writeFileSync(path.join(output, 'report.json'), `${JSON.stringify(report, null, 2)}\n`) + console.log(output) + await app.close() +}