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/renderer/src/components/settings/source-control-action-recipe-options.ts b/src/renderer/src/components/settings/source-control-action-recipe-options.ts index dfeaeda128b..4a6f491c1ca 100644 --- a/src/renderer/src/components/settings/source-control-action-recipe-options.ts +++ b/src/renderer/src/components/settings/source-control-action-recipe-options.ts @@ -62,7 +62,8 @@ const AGENT_ARGS_PLACEHOLDER_OVERRIDES: Partial> = { // Why: Source Control AI action prompts are short, reviewable tasks; the // mini Codex model is a better default hint than the frontier model. codex: '--model gpt-5.4-mini', - copilot: '--model gpt-5.4-mini' + copilot: '--model gpt-5.4-mini', + omp: '--model ' } const MODEL_FLAG_BY_AGENT: Partial> = { diff --git a/src/shared/agent-model-probe-spec.test.ts b/src/shared/agent-model-probe-spec.test.ts index 229e964d337..68fd44b7bb7 100644 --- a/src/shared/agent-model-probe-spec.test.ts +++ b/src/shared/agent-model-probe-spec.test.ts @@ -31,14 +31,23 @@ describe('getAgentModelProbeSpec', () => { ) }) - it('aliases commit-message agents by identity rather than copying them', () => { + it('aliases agents without a generation-only default by identity', () => { // A lossy adapter here would silently drop fields like // `modelDiscovery.stdinPayload` and break Claude discovery. - for (const id of listCommitMessageAgentIds()) { + for (const id of listCommitMessageAgentIds().filter((agentId) => agentId !== 'omp')) { expect(getAgentModelProbeSpec(id)).toBe(getCommitMessageAgentSpec(id)) } }) + it('removes only the OMP generation sentinel while preserving discovery', () => { + const generation = getCommitMessageAgentSpec('omp')! + const probe = getAgentModelProbeSpec('omp')! + expect(generation.defaultModelId).toBe('default') + expect(probe.defaultModelId).toBe('') + expect(probe.models).toEqual([]) + expect(probe.modelDiscovery).toBe(generation.modelDiscovery) + }) + it('keeps grok out of the commit-message registry', () => { expect(getCommitMessageAgentSpec('grok')).toBeUndefined() expect(listCommitMessageAgentIds()).not.toContain('grok') diff --git a/src/shared/agent-model-probe-spec.ts b/src/shared/agent-model-probe-spec.ts index ea696a3ab9c..583262217ab 100644 --- a/src/shared/agent-model-probe-spec.ts +++ b/src/shared/agent-model-probe-spec.ts @@ -26,5 +26,17 @@ const MODEL_DISCOVERY_ONLY_SPECS: Partial> } export function getAgentModelProbeSpec(agentId: TuiAgent): AgentModelProbeSpec | undefined { - return getCommitMessageAgentSpec(agentId) ?? MODEL_DISCOVERY_ONLY_SPECS[agentId] + const spec = getCommitMessageAgentSpec(agentId) ?? MODEL_DISCOVERY_ONLY_SPECS[agentId] + if (!spec) { + return undefined + } + // OMP's `default` means the provider configured in its own settings, not a selectable model. + if (agentId !== 'omp') { + return spec + } + return { + ...spec, + models: spec.models.filter((model) => model.id !== 'default'), + defaultModelId: spec.defaultModelId === 'default' ? '' : spec.defaultModelId + } } diff --git a/src/shared/commit-message-agent-spec.test.ts b/src/shared/commit-message-agent-spec.test.ts index de5cc14529b..ae068ad8759 100644 --- a/src/shared/commit-message-agent-spec.test.ts +++ b/src/shared/commit-message-agent-spec.test.ts @@ -37,6 +37,7 @@ describe('COMMIT_MESSAGE_AGENT_SPECS', () => { 'copilot', 'cursor', 'kimi', + 'omp', 'opencode', 'pi' ]) diff --git a/src/shared/commit-message-agent-spec.ts b/src/shared/commit-message-agent-spec.ts index 6223b968b67..7a4145a7630 100644 --- a/src/shared/commit-message-agent-spec.ts +++ b/src/shared/commit-message-agent-spec.ts @@ -1,3 +1,4 @@ +import { OMP_MODEL_LIST_ARGS, parseOmpModelList } from './omp-model-list-probe' import type { TuiAgent } from './tui-agent' import { isTuiAgentEnabled } from './tui-agent-selection' import { labelFromModelId } from './model-id-label' @@ -88,6 +89,29 @@ export type CommitMessageAgentCapability = { } export const COMMIT_MESSAGE_AGENT_SPECS: Partial> = { + omp: { + id: 'omp', + label: 'OMP', + binary: 'omp', + promptDelivery: 'stdin', + buildArgs: ({ model, thinkingLevel }) => [ + '--print', + '--no-session', + '--no-tools', + '--no-extensions', + '--no-skills', + '--no-rules', + '--mode', + 'text', + ...(model && model !== 'default' ? ['--model', model] : []), + ...(thinkingLevel ? ['--thinking', thinkingLevel] : []) + ], + singletonOptions: [['--model'], ['--thinking']], + modelSource: 'dynamic', + modelDiscovery: { binary: 'omp', args: OMP_MODEL_LIST_ARGS, parse: parseOmpModelList }, + models: [{ id: 'default', label: 'Config default' }], + defaultModelId: 'default' + }, ...buildPrimaryCommitMessageAgentSpecs({ CLAUDE_THINKING_LEVELS, OPENAI_THINKING_LEVELS, diff --git a/src/shared/commit-message-omp.test.ts b/src/shared/commit-message-omp.test.ts new file mode 100644 index 00000000000..2ffa43d9c44 --- /dev/null +++ b/src/shared/commit-message-omp.test.ts @@ -0,0 +1,83 @@ +import { planCommitMessageGeneration } from './commit-message-plan' +import { getAgentModelProbeSpec } from './agent-model-probe-spec' +import { describe, expect, it } from 'vitest' +import { getDefaultSettings } from './constants' +import { getCommitMessageAgentSpec } from './commit-message-agent-spec' +import { resolveSourceControlAiForOperation } from './source-control-ai' + +describe('OMP Source Control AI', () => { + it.each(['commitMessage', 'pullRequest', 'branchName'] as const)( + 'uses the configured OMP default for %s without requiring a model override', + (operation) => { + const settings = getDefaultSettings('/tmp') + settings.defaultTuiAgent = 'omp' + const result = resolveSourceControlAiForOperation({ settings, repo: null, operation }) + expect(result).toMatchObject({ + ok: true, + value: { params: { agentId: 'omp', model: 'default' } } + }) + } + ) + it('delivers large diffs on stdin and keeps generation isolated from tools and extensions', () => { + const spec = getCommitMessageAgentSpec('omp') + expect(spec?.promptDelivery).toBe('stdin') + if (!spec) { + throw new Error('Missing OMP spec') + } + const args = spec.buildArgs({ prompt: 'large diff', model: 'default' }) + expect(args).toEqual([ + '--print', + '--no-session', + '--no-tools', + '--no-extensions', + '--no-skills', + '--no-rules', + '--mode', + 'text' + ]) + expect( + spec?.buildArgs({ prompt: '', model: 'provider/exact-model', thinkingLevel: 'low' }) + ).toEqual([...args, '--model', 'provider/exact-model', '--thinking', 'low']) + }) + it('discovers provider-qualified models through the existing OMP JSON parser', () => { + const discovery = getCommitMessageAgentSpec('omp')?.modelDiscovery + expect(discovery?.args).toEqual(['models', '--json']) + expect( + discovery?.parse( + JSON.stringify({ + models: [{ provider: 'provider', id: 'm', selector: 'provider/m', name: 'Model' }] + }) + ) + ).toEqual([{ id: 'provider/m', label: 'Model', description: 'provider' }]) + }) + 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 result = planCommitMessageGeneration( + { + agentId: 'omp', + model: 'provider/model', + backslash, + agentArgs: '--model provider/override' + }, + prompt + ) + expect(result.ok).toBe(true) + if (!result.ok) { + throw new Error(result.error) + } + expect(result.plan.stdinPayload).toBe(prompt) + expect(result.plan.args).not.toContain(prompt) + expect(result.plan.args.filter((arg) => arg === '--model')).toHaveLength(1) + expect(result.plan.args).toContain('provider/override') + expect(result.plan.args).not.toContain('provider/model') + } + ) +}) + +it('does not expose OMP config default as a terminal discovery model', () => { + const spec = getAgentModelProbeSpec('omp') + expect(spec?.models.some((model) => model.id === 'default')).toBe(false) + expect(spec?.defaultModelId).toBe('') +}) diff --git a/src/shared/omp-model-list-probe.ts b/src/shared/omp-model-list-probe.ts new file mode 100644 index 00000000000..1654b51ad90 --- /dev/null +++ b/src/shared/omp-model-list-probe.ts @@ -0,0 +1,66 @@ +import type { CommitMessageModel } from './commit-message-agent-spec' +import { labelFromModelId } from './model-id-label' + +// Why: `omp models --json` is the one machine-readable listing OMP offers; the +// default table output groups rows per provider and would need a brittle parser. +export const OMP_MODEL_LIST_ARGS = ['models', '--json'] + +/** The outermost JSON value on stdout, or null when none parses. */ +function parseJsonObject(stdout: string): unknown { + const trimmed = stdout.trim() + try { + return JSON.parse(trimmed) + } catch { + // Why: an update notice or extension warning can precede the JSON on stdout; + // the listing itself is the outermost object. + const start = trimmed.indexOf('{') + const end = trimmed.lastIndexOf('}') + if (start === -1 || end <= start) { + return null + } + try { + return JSON.parse(trimmed.slice(start, end + 1)) + } catch { + return null + } + } +} + +/** Parses `omp models --json`. Ids are OMP's `provider/model` selector — the form + * `--model` and `/model` resolve exactly, unlike a bare model id that several + * providers can share. */ +export function parseOmpModelList(stdout: string): CommitMessageModel[] { + const parsed = parseJsonObject(stdout) + if (!parsed || typeof parsed !== 'object' || !('models' in parsed)) { + return [] + } + const rows: unknown = parsed.models + if (!Array.isArray(rows)) { + return [] + } + const byId = new Map() + for (const row of rows) { + const value: unknown = row + if (!value || typeof value !== 'object' || Array.isArray(value)) { + continue + } + const provider = + 'provider' in value && typeof value.provider === 'string' ? value.provider.trim() : '' + const bareId = 'id' in value && typeof value.id === 'string' ? value.id.trim() : '' + const selector = + 'selector' in value && typeof value.selector === 'string' ? value.selector.trim() : '' + const id = selector || (provider && bareId ? `${provider}/${bareId}` : '') + if (!id || byId.has(id)) { + continue + } + const name = 'name' in value && typeof value.name === 'string' ? value.name.trim() : '' + byId.set(id, { + id, + label: name || labelFromModelId(id), + // Why: the same model name ships under several providers; the provider is + // what tells two "DeepSeek V4 Pro" rows apart in the picker. + ...(provider ? { description: provider } : {}) + }) + } + return [...byId.values()] +} 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/src/shared/source-control-ai-action-recipes.test.ts b/src/shared/source-control-ai-action-recipes.test.ts index aba81519d09..5255f75b7e0 100644 --- a/src/shared/source-control-ai-action-recipes.test.ts +++ b/src/shared/source-control-ai-action-recipes.test.ts @@ -427,7 +427,7 @@ describe('source-control AI action recipes', () => { ).toEqual({ ok: false, error: - 'Agent "aider" does not support Source Control AI commit messages. Supported agents: Claude, Codex, OpenCode, Pi, Amp, Cursor, Kimi, GitHub Copilot, Antigravity, or Custom command.' + 'Agent "aider" does not support Source Control AI commit messages. Supported agents: OMP, Claude, Codex, OpenCode, Pi, Amp, Cursor, Kimi, GitHub Copilot, Antigravity, or Custom command.' }) }) }) diff --git a/tests/tools/omp-source-control-runtime-smoke.mjs b/tests/tools/omp-source-control-runtime-smoke.mjs new file mode 100644 index 00000000000..8dce4a54775 --- /dev/null +++ b/tests/tools/omp-source-control-runtime-smoke.mjs @@ -0,0 +1,23 @@ +import assert from 'node:assert/strict' +import { pathToFileURL } from 'node:url' +import { join, resolve } from 'node:path' +import { getCommitMessageAgentSpec } from '../../src/shared/commit-message-agent-spec.ts' +const reference = process.argv[2] +assert.ok(reference, 'Pass the read-only source checkout path') +const { parseArgs } = await import(pathToFileURL(join(resolve(reference), 'packages/coding-agent/src/cli/args.ts')).href) +const spec = getCommitMessageAgentSpec('omp') +assert.ok(spec) +for (const model of ['default', 'provider/model']) { + const args = spec.buildArgs({ prompt: 'Generated entirely from stdin', model }) + const parsed = parseArgs(args) + assert.equal(parsed.print, true) + assert.equal(parsed.noSession, true) + assert.equal(parsed.noTools, true) + assert.equal(parsed.noExtensions, true) + assert.equal(parsed.noSkills, true) + assert.equal(parsed.noRules, true) + assert.equal(parsed.mode, 'text') + assert.equal(parsed.model, model === 'default' ? undefined : model) + assert.deepEqual(parsed.messages, []) +} +console.log(JSON.stringify({ actualArgumentParser: true, promptDelivery: spec.promptDelivery, configDefault: true, explicitModel: true, modelCalls: 0 })) 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() +}