mirror of
https://github.com/stablyai/orca.git
synced 2026-09-26 08:02:38 +00:00
fix(omp): separate terminal discovery from generation defaults
This commit is contained in:
@@ -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')
|
||||
}
|
||||
|
||||
|
||||
@@ -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' }]
|
||||
})
|
||||
})
|
||||
@@ -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<CommitMessageAgentSpec, 'promptDelivery' | 'buildArgs'>
|
||||
|
||||
/** Agents that support model discovery but are not commit-message agents. */
|
||||
const MODEL_DISCOVERY_ONLY_SPECS: Partial<Record<TuiAgent, AgentModelProbeSpec>> = {
|
||||
|
||||
@@ -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<Record<TuiAgent, CommitMessageA
|
||||
modelSource: 'dynamic',
|
||||
modelDiscovery: { binary: 'omp', args: OMP_MODEL_LIST_ARGS, parse: parseOmpModelList },
|
||||
models: [{ id: 'default', label: 'Config default' }],
|
||||
defaultModelId: 'default',
|
||||
configuredDefaultModelId: 'default'
|
||||
defaultModelId: 'default'
|
||||
},
|
||||
...buildPrimaryCommitMessageAgentSpecs({
|
||||
CLAUDE_THINKING_LEVELS,
|
||||
|
||||
@@ -52,7 +52,7 @@ describe('OMP Source Control AI', () => {
|
||||
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',
|
||||
|
||||
@@ -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')
|
||||
}
|
||||
}
|
||||
}
|
||||
)
|
||||
})
|
||||
}
|
||||
@@ -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.
|
||||
@@ -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';
|
||||
@@ -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<GlobalSettings>): Promise<void> => {
|
||||
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 (
|
||||
<TooltipProvider>
|
||||
<main className="mx-auto max-w-4xl p-6">
|
||||
<CommitMessageAiPane settings={current} updateSettings={updateSettings} />
|
||||
</main>
|
||||
</TooltipProvider>
|
||||
)
|
||||
}
|
||||
const root = document.getElementById('root')
|
||||
if (!root) {
|
||||
throw new Error('Missing fixture root')
|
||||
}
|
||||
createRoot(root).render(<Fixture />)
|
||||
@@ -0,0 +1,11 @@
|
||||
<!doctype html>
|
||||
<html class="dark">
|
||||
<head>
|
||||
<meta charset="UTF-8" />
|
||||
<title>OMP Source Control AI settings proof</title>
|
||||
</head>
|
||||
<body>
|
||||
<div id="root"></div>
|
||||
<script type="module" src="./fixture.tsx"></script>
|
||||
</body>
|
||||
</html>
|
||||
@@ -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()
|
||||
}
|
||||
Reference in New Issue
Block a user