diff --git a/src/main/acp/acp-structured-missing-model.test.ts b/src/main/acp/acp-structured-missing-model.test.ts new file mode 100644 index 00000000000..738ba92bf33 --- /dev/null +++ b/src/main/acp/acp-structured-missing-model.test.ts @@ -0,0 +1,115 @@ +import { readFileSync } from 'node:fs' +import { describe, expect, it } from 'vitest' +import { isPersistedAgentSessionRecord } from '../../shared/agent-session-record' +import { agentSessionRecordFixture } from '../../shared/agent-session-record.test-fixture' +import { readNativeSessionOptions } from '../native-chat/agent-session-wire/structured-agent-session-option-restoration' +import { replaceAgentSessionRecordOptions } from '../runtime/agent-session-record-options' +import { agentSessionStoreDraftRowWrites } from '../runtime/agent-session-store-draft' +import type { AgentSessionStoreState } from '../runtime/agent-session-store-state' +import { AcpStructuredOptions } from './acp-structured-options' +import { + NewSessionResponseSchema, + type SessionConfigOption +} from './generated/acp-protocol.generated' + +// OMP 17.3.8 on Windows; only the captured conversation id is replaced. +const response = NewSessionResponseSchema.parse( + JSON.parse( + readFileSync(new URL('./fixtures/omp-v17-windows-new-no-model.json', import.meta.url), 'utf8') + ) +) +const modelOption: SessionConfigOption = { + id: 'model', + name: 'Model', + category: 'model', + type: 'select', + currentValue: 'reported-model', + options: [{ value: 'reported-model', name: 'Reported Model' }] +} + +async function writtenOptions(provider: string, reader: AcpStructuredOptions) { + const fixture = agentSessionRecordFixture() + const record = { + ...fixture, + provider, + launchDirectory: 'C:/work/chat-folder', + providerHandleChain: fixture.providerHandleChain.map((link) => ({ + ...link, + handle: { transport: 'acp', agent: provider, nativeId: response.sessionId } + })) + } + const published: AgentSessionStoreState = { + records: new Map([[record.sessionId, record]]), + operations: new Map(), + retiredClaimKeys: [], + unreadableRecords: new Map(), + sessionTabs: null + } + const options = await readNativeSessionOptions({ + adapter: { readOptions: async () => reader.read() }, + sessionId: record.sessionId, + fence: record.lease.runtimeFence + }) + if (!options) { + throw new Error('The live reader must report its options') + } + const changed = replaceAgentSessionRecordOptions(record, { + sessionId: record.sessionId, + fence: record.lease.runtimeFence, + options, + now: 3_000 + }) + const writes = agentSessionStoreDraftRowWrites(published, { + ...published, + records: new Map([[record.sessionId, changed]]) + }) + expect(writes?.records.upsert).toHaveLength(1) + const stored = JSON.parse(writes?.records.upsert[0]?.[1] ?? 'null') + expect(isPersistedAgentSessionRecord(stored)).toBe(true) + expect(stored.options).toEqual(options) + return options +} + +describe('ACP sessions without a reported model', () => { + it('writes the captured Windows OMP startup through the live reader and shared normalizer', async () => { + const reader = new AcpStructuredOptions() + reader.adoptSession(response) + expect(reader.read()).toEqual({ + models: [], + current: { effort: 'off', confirmed: ['effort'] } + }) + expect(reader.reported()).toEqual({ effort: 'off' }) + expect(await writtenOptions('omp', reader)).toEqual({ effort: 'off' }) + }) + + it.each(['grok', 'opencode'])('writes missing-model options for %s', async (provider) => { + const reader = new AcpStructuredOptions() + reader.adoptSession({}) + expect(reader.read().current).not.toHaveProperty('model') + expect(await writtenOptions(provider, reader)).toEqual({}) + }) + + it.each(['config', 'legacy'] as const)('does not expose an empty %s model value', (source) => { + const reader = new AcpStructuredOptions() + reader.adoptSession( + source === 'config' + ? { configOptions: [{ ...modelOption, currentValue: '', options: [] }] } + : { models: { currentModelId: '', availableModels: [] } } + ) + expect(reader.read().current).not.toHaveProperty('model') + expect(reader.reported()).toEqual({}) + }) + + it('records a model reported after a startup with no model', async () => { + const reader = new AcpStructuredOptions() + reader.adoptSession(response) + expect(await writtenOptions('omp', reader)).toEqual({ effort: 'off' }) + reader.adoptConfigOptions([...(response.configOptions ?? []), modelOption]) + expect(reader.read().current).toEqual({ + model: 'reported-model', + effort: 'off', + confirmed: ['model', 'effort'] + }) + expect(await writtenOptions('omp', reader)).toEqual({ model: 'reported-model', effort: 'off' }) + }) +}) diff --git a/src/main/acp/acp-structured-options.ts b/src/main/acp/acp-structured-options.ts index 662c3c98c1a..250541a7e21 100644 --- a/src/main/acp/acp-structured-options.ts +++ b/src/main/acp/acp-structured-options.ts @@ -112,7 +112,7 @@ export class AcpStructuredOptions { ...(choice.description ? { description: choice.description } : {}) })) : [] - const currentModel = modelOption?.currentValue ?? this.models?.currentModelId ?? '' + const currentModel = (modelOption?.currentValue ?? this.models?.currentModelId) || undefined const listed: { id: string; label: string; description?: string }[] = modelOption ? selectChoices(modelOption).map((choice) => ({ id: choice.value, @@ -134,7 +134,7 @@ export class AcpStructuredOptions { return { models, current: { - model: currentModel, + ...(currentModel ? { model: currentModel } : {}), ...(effortOption ? { effort: effortOption.currentValue } : {}), confirmed } diff --git a/src/main/acp/fixtures/omp-v17-windows-new-no-model.json b/src/main/acp/fixtures/omp-v17-windows-new-no-model.json new file mode 100644 index 00000000000..49dbaba3faf --- /dev/null +++ b/src/main/acp/fixtures/omp-v17-windows-new-no-model.json @@ -0,0 +1,57 @@ +{ + "sessionId": "session-omp-windows", + "configOptions": [ + { + "id": "mode", + "name": "Mode", + "category": "mode", + "type": "select", + "currentValue": "default", + "options": [ + { + "value": "default", + "name": "Default", + "description": "Standard ACP headless mode" + }, + { + "value": "plan", + "name": "Plan", + "description": "Read-only planning mode that drafts a plan to a markdown file before any code changes" + } + ] + }, + { + "id": "thinking", + "name": "Thinking", + "category": "thought_level", + "type": "select", + "currentValue": "off", + "options": [ + { + "value": "off", + "name": "Off" + }, + { + "value": "auto", + "name": "Auto", + "description": "Auto-detect per prompt" + } + ] + } + ], + "modes": { + "availableModes": [ + { + "id": "default", + "name": "Default", + "description": "Standard ACP headless mode" + }, + { + "id": "plan", + "name": "Plan", + "description": "Read-only planning mode that drafts a plan to a markdown file before any code changes" + } + ], + "currentModeId": "default" + } +} diff --git a/src/main/claude/claude-structured-effort-default-at-rest.test.ts b/src/main/claude/claude-structured-effort-default-at-rest.test.ts index 2fe352fa343..e0342094db6 100644 --- a/src/main/claude/claude-structured-effort-default-at-rest.test.ts +++ b/src/main/claude/claude-structured-effort-default-at-rest.test.ts @@ -305,7 +305,7 @@ describe('Claude effort default at rest', () => { const resting = await readAtRest(new AgentModelCatalogStore(), restingRecord({})) // The built-in list's default is a guess, not this account's: nothing to name. - expect(resting.current.model).toBe('') + expect(resting.current).not.toHaveProperty('model') const seed = getAgentSessionOptionCatalog('claude')! const state = [live, resting].reduce( (current, answer) => applyStructuredAgentSessionOptions(current, seed, answer), diff --git a/src/main/codex/codex-structured-model-catalog.ts b/src/main/codex/codex-structured-model-catalog.ts index ea47d4d6a1c..0cd56e0558c 100644 --- a/src/main/codex/codex-structured-model-catalog.ts +++ b/src/main/codex/codex-structured-model-catalog.ts @@ -82,7 +82,7 @@ function modelOption(value: unknown): ParsedCodexModelOption | null { } export type CodexSessionOptionCatalog = { - result: AgentSessionOptionsResult + result: AgentSessionOptionsResult & { current: { model: string } } fastModeTierByModel: Map } diff --git a/src/main/ipc/desktop-renderer-runtime-capabilities.ts b/src/main/ipc/desktop-renderer-runtime-capabilities.ts index 3b171d8bd51..4a5e64296a0 100644 --- a/src/main/ipc/desktop-renderer-runtime-capabilities.ts +++ b/src/main/ipc/desktop-renderer-runtime-capabilities.ts @@ -12,6 +12,7 @@ import { STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY, type RuntimeCapability } from '../../shared/protocol-version' +import { AGENT_SESSION_OPTIONAL_MODEL_CLIENT_CAPABILITY } from '../../shared/agent-session-optional-model-capability' import { AGENT_LAUNCH_PROMPT_UNCONFIRMED_RUNTIME_CAPABILITY, AGENT_LAUNCH_RUNTIME_CAPABILITY, @@ -41,6 +42,7 @@ export const DESKTOP_RENDERER_RUNTIME_CLIENT_CAPABILITIES: readonly RuntimeCapab AGENT_SESSION_BACKGROUND_TASK_CHILD_VIEWS_CAPABILITY, STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY, CLAUDE_STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY, + AGENT_SESSION_OPTIONAL_MODEL_CLIENT_CAPABILITY, // The renderer reads `agentSession.agents` and renders a chat tab of any agent its host lists. STRUCTURED_AGENT_SESSION_REGISTERED_AGENTS_RUNTIME_CAPABILITY, PI_STRUCTURED_DIALOGS_RUNTIME_CAPABILITY, diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-option-restoration.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-option-restoration.ts index a9cdc2d1418..63bbbe64d93 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-option-restoration.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-option-restoration.ts @@ -43,7 +43,7 @@ export function nativeSessionOptionsFromReport(input: { : encodeStructuredAgentSessionOptionValue('fastMode', reported.fastMode) const options: Record = { ...restored, - model: reported.model, + ...(reported.model ? { model: reported.model } : {}), ...(reported.effort ? { effort: reported.effort } : {}), ...(fastMode !== undefined && fastMode !== null ? { fastMode } : {}) } diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-options-read.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-options-read.test.ts index 64c203183db..49f321b379f 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-options-read.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-options-read.test.ts @@ -25,38 +25,47 @@ function restingRecord(): AgentSessionRecord { } describe('options at rest', () => { - it('answers while the first catalog listing is still running', async () => { - const record = restingRecord() - const probe = vi.fn(() => new Promise(() => {})) - const modelCatalog = createAgentModelCatalogService({ - store: new AgentModelCatalogStore(), - getRecord: () => record, - drivesRecord: () => true, - resolveAccountHome: async () => ({ variable: 'CODEX_HOME', path: '/homes/a' }), - probes: { codex: probe } - }) - const resting = { - child: null, - params: { provider: 'codex' }, - journal: { context: { floor: () => null } } - } - // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the resting read touches only these members. - const context = { - deps: { - adapter: {}, - agents: NO_STRUCTURED_AGENTS, - store: { getRecord: () => record }, - modelCatalog - }, - serialize: (_sessionId: string, task: () => Promise) => task(), - openConversation: async () => resting, - conversation: async () => resting - } as unknown as StructuredAgentSessionMutationContext + it.each(['codex', 'omp', 'grok', 'opencode'] as const)( + 'omits an unknown model for %s while the catalog is unavailable', + async (provider) => { + const record = { ...restingRecord(), provider } + const probe = vi.fn(() => new Promise(() => {})) + const modelCatalog = createAgentModelCatalogService({ + store: new AgentModelCatalogStore(), + getRecord: () => record, + drivesRecord: () => true, + resolveAccountHome: async () => ({ variable: 'CODEX_HOME', path: '/homes/a' }), + probes: { codex: probe } + }) + const resting = { + child: null, + params: { provider: 'codex' }, + journal: { context: { floor: () => null } } + } + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the resting read touches only these members. + const context = { + deps: { + adapter: {}, + agents: NO_STRUCTURED_AGENTS, + store: { getRecord: () => record }, + modelCatalog + }, + serialize: (_sessionId: string, task: () => Promise) => task(), + openConversation: async () => resting, + conversation: async () => resting + } as unknown as StructuredAgentSessionMutationContext - const result = await readStructuredAgentSessionOptions(context, SESSION) - expect(probe).toHaveBeenCalledTimes(1) - expect(result.models).toEqual([]) - }) + const result = await readStructuredAgentSessionOptions(context, SESSION) + expect(probe).toHaveBeenCalledTimes(provider === 'codex' ? 1 : 0) + expect(result.models).toEqual([]) + expect(result.current).not.toHaveProperty('model') + record.options = { model: 'later-model', effort: 'off' } + expect((await readStructuredAgentSessionOptions(context, SESSION)).current).toEqual({ + model: 'later-model', + effort: 'off' + }) + } + ) }) describe('live Codex option reads', () => { @@ -165,7 +174,7 @@ describe('live Codex option reads', () => { live.child = { fence: 8 } }) listing.resolve() - await expect(reading).resolves.toMatchObject({ current: { model: '' } }) + expect((await reading).current).not.toHaveProperty('model') expect(apply).not.toHaveBeenCalled() }) }) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-options-read.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-options-read.ts index 9bbc74d05cd..6b2bdaba959 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-options-read.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-options-read.ts @@ -55,8 +55,7 @@ async function readStructuredAgentSessionOptionsAtRest( // account's default; a built-in list's default is a guess, so with none the client keeps its own. const model = saved.model ?? - (catalog.origin === 'unknown' ? undefined : models.find((entry) => entry.isDefault)?.id) ?? - '' + (catalog.origin === 'unknown' ? undefined : models.find((entry) => entry.isDefault)?.id) // As a live child answers: the pick, else the model's default where the agent reports that. const effort = saved.effort ?? @@ -69,7 +68,7 @@ async function readStructuredAgentSessionOptionsAtRest( ? { fastModeSupport: catalog.fastModeSupport } : {}), current: { - model, + ...(model ? { model } : {}), ...(effort ? { effort } : {}), ...(typeof fastMode === 'boolean' ? { fastMode } : {}) } diff --git a/src/main/runtime/agent-session-record-options.test.ts b/src/main/runtime/agent-session-record-options.test.ts index c092018ea27..bef2bc424ef 100644 --- a/src/main/runtime/agent-session-record-options.test.ts +++ b/src/main/runtime/agent-session-record-options.test.ts @@ -1,10 +1,17 @@ import { mkdtemp, rm } from 'node:fs/promises' +import { readFileSync } from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' import { afterEach, beforeEach, expect, it } from 'vitest' import { readNativeSessionOptions } from '../native-chat/agent-session-wire/structured-agent-session-option-restoration' -import { openTestAgentSessionRecordStore } from './agent-session-record-store-test-harness' +import { + openTestAgentSessionRecordStore, + seedTestAgentSessionRecordStore +} from './agent-session-record-store-test-harness' import { codexProviderHandle } from '../../shared/agent-session-provider-handle-encoding' +import { agentSessionRecordFixture } from '../../shared/agent-session-record.test-fixture' +import { AcpStructuredOptions } from '../acp/acp-structured-options' +import { NewSessionResponseSchema } from '../acp/generated/acp-protocol.generated' const NOW = 1_800_000_000_000 const SESSION = 'session-options' @@ -46,6 +53,75 @@ it('drops provider-rejected persisted options before the next owner proof', asyn ).resolves.toEqual({ model: 'provider-model', other: 'keep' }) }) +it('omits empty model values from other option producers', async () => { + await expect( + readNativeSessionOptions({ + adapter: { readOptions: async () => ({ models: [], current: { model: '', effort: 'off' } }) }, + sessionId: SESSION, + fence: 2, + priorOptions: { model: 'old-model', other: 'keep' } + }) + ).resolves.toEqual({ effort: 'off', other: 'keep' }) +}) + +it('durably replaces model-less ACP options when a later report reaches the option writer', async () => { + const response = NewSessionResponseSchema.parse( + JSON.parse( + readFileSync( + new URL('../acp/fixtures/omp-v17-windows-new-no-model.json', import.meta.url), + 'utf8' + ) + ) + ) + const reader = new AcpStructuredOptions() + reader.adoptSession(response) + const fixture = agentSessionRecordFixture() + const record = { + ...fixture, + provider: 'omp', + options: reader.reported(), + providerHandleChain: fixture.providerHandleChain.map((link) => ({ + ...link, + handle: { transport: 'acp', agent: 'omp', nativeId: response.sessionId } + })) + } + await seedTestAgentSessionRecordStore(directory, { records: [record] }) + const store = await openTestAgentSessionRecordStore(directory) + expect(store.getRecord(record.sessionId)?.options).toEqual({ effort: 'off' }) + + reader.adoptConfigOptions([ + ...(response.configOptions ?? []), + { + id: 'model', + name: 'Model', + category: 'model', + type: 'select', + currentValue: 'reported-model', + options: [{ value: 'reported-model', name: 'Reported Model' }] + } + ]) + const options = await readNativeSessionOptions({ + adapter: { readOptions: async () => reader.read() }, + sessionId: record.sessionId, + fence: record.lease.runtimeFence, + priorOptions: store.getRecord(record.sessionId)?.options + }) + if (!options) { + throw new Error('The later ACP report must provide options') + } + await store.replaceSessionOptions({ + sessionId: record.sessionId, + fence: record.lease.runtimeFence, + options, + now: NOW + }) + const reopened = await openTestAgentSessionRecordStore(directory) + expect(reopened.getRecord(record.sessionId)?.options).toEqual({ + model: 'reported-model', + effort: 'off' + }) +}) + it('persists resumed provider options atomically with owner proof', async () => { const store = await openTestAgentSessionRecordStore(directory) const reserved = await store.reserveOwner({ diff --git a/src/main/runtime/rpc/methods/structured-agent-session-optional-model-capability.test.ts b/src/main/runtime/rpc/methods/structured-agent-session-optional-model-capability.test.ts new file mode 100644 index 00000000000..b5257f409a0 --- /dev/null +++ b/src/main/runtime/rpc/methods/structured-agent-session-optional-model-capability.test.ts @@ -0,0 +1,142 @@ +import { readFileSync } from 'node:fs' +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { AcpStructuredOptions } from '../../../acp/acp-structured-options' +import { NewSessionResponseSchema } from '../../../acp/generated/acp-protocol.generated' +import { DESKTOP_RENDERER_RUNTIME_CLIENT_CAPABILITIES } from '../../../ipc/desktop-renderer-runtime-capabilities' +import { ELECTRON_REMOTE_RUNTIME_CLIENT_CAPABILITIES } from '../../../../shared/electron-remote-runtime-client-capabilities' +import { AGENT_SESSION_OPTIONAL_MODEL_CLIENT_CAPABILITY } from '../../../../shared/agent-session-optional-model-capability' +import { remoteRuntimeClientCapabilities } from '../../../../shared/remote-runtime-client-capabilities' +import { parseRuntimeClientCapabilities } from '../runtime-client-capabilities' +import { + call, + clearStructuredHostStub, + hostCalls, + installStructuredHostStub, + SESSION, + STRUCTURED_CLIENT +} from './structured-agent-session-rpc.test-fixture' + +const CURRENT_CLIENT = { + ...STRUCTURED_CLIENT, + clientCapabilities: [ + ...STRUCTURED_CLIENT.clientCapabilities, + AGENT_SESSION_OPTIONAL_MODEL_CLIENT_CAPABILITY + ] +} +const DESKTOP_CLIENTS = [ + ['local desktop', [...DESKTOP_RENDERER_RUNTIME_CLIENT_CAPABILITIES]], + ['paired desktop', remoteRuntimeClientCapabilities(ELECTRON_REMOTE_RUNTIME_CLIENT_CAPABILITIES)] +] as const + +function missingModelReport() { + const reader = new AcpStructuredOptions() + reader.adoptSession( + NewSessionResponseSchema.parse( + JSON.parse( + readFileSync( + new URL('../../../acp/fixtures/omp-v17-windows-new-no-model.json', import.meta.url), + 'utf8' + ) + ) + ) + ) + return reader.read() +} + +beforeEach(installStructuredHostStub) +afterEach(clearStructuredHostStub) + +describe('options without a selected model across client versions', () => { + it.each(['runtime', 'mobile'] as const)( + 'refuses the captured model-less report for an older %s client', + async (clientKind) => { + hostCalls.readOptions.mockResolvedValue(missingModelReport()) + const reply = await call( + 'agentSession.options', + { sessionId: SESSION }, + { + ...STRUCTURED_CLIENT, + clientKind + } + ) + expect(reply).toMatchObject({ + ok: false, + error: { message: 'structured_agent_session_unsupported' } + }) + expect(reply).not.toHaveProperty('result') + expect(hostCalls.readOptions).toHaveBeenCalledTimes(1) + } + ) + + it.each(DESKTOP_CLIENTS)( + 'returns the captured absence to the %s', + async (_name, capabilities) => { + const report = missingModelReport() + hostCalls.readOptions.mockResolvedValue(report) + const reply = await call( + 'agentSession.options', + { sessionId: SESSION }, + { + clientKind: 'runtime', + clientCapabilities: [...capabilities] + } + ) + expect(reply).toMatchObject({ id: 'request-1', ok: true, result: report }) + if (!reply.ok) { + throw new Error('Updated desktops must receive the report') + } + expect(reply.result).toEqual(report) + expect(reply).not.toHaveProperty('result.current.model') + } + ) + + it('keeps same-build in-process reads available', async () => { + const report = missingModelReport() + hostCalls.readOptions.mockResolvedValue(report) + expect(await call('agentSession.options', { sessionId: SESSION })).toMatchObject({ + ok: true, + result: report + }) + }) + + it.each(['reported-model', ''])( + 'preserves an existing string-valued report (%j)', + async (model) => { + const report = { models: [], current: { model, effort: 'off' } } + hostCalls.readOptions.mockResolvedValue(report) + for (const client of [STRUCTURED_CLIENT, CURRENT_CLIENT]) { + expect(await call('agentSession.options', { sessionId: SESSION }, client)).toMatchObject({ + ok: true, + result: report + }) + } + } + ) + + it('resumes legacy discovery after the provider reports a model', async () => { + hostCalls.readOptions.mockResolvedValueOnce(missingModelReport()) + expect( + await call('agentSession.options', { sessionId: SESSION }, STRUCTURED_CLIENT) + ).toMatchObject({ + ok: false + }) + const report = { models: [], current: { model: 'later-model', effort: 'off' } } + hostCalls.readOptions.mockResolvedValueOnce(report) + expect( + await call('agentSession.options', { sessionId: SESSION }, STRUCTURED_CLIENT) + ).toMatchObject({ + ok: true, + result: report + }) + }) + + it('keeps authenticated capability input compatible with older parsers', () => { + for (const [, capabilities] of DESKTOP_CLIENTS) { + expect(parseRuntimeClientCapabilities(capabilities)).toEqual(capabilities) + expect(capabilities).toContain(AGENT_SESSION_OPTIONAL_MODEL_CLIENT_CAPABILITY) + } + expect(remoteRuntimeClientCapabilities()).not.toContain( + AGENT_SESSION_OPTIONAL_MODEL_CLIENT_CAPABILITY + ) + }) +}) diff --git a/src/main/runtime/rpc/methods/structured-agent-session-options-read.ts b/src/main/runtime/rpc/methods/structured-agent-session-options-read.ts index 4f650e9ed9c..2b0683d08a3 100644 --- a/src/main/runtime/rpc/methods/structured-agent-session-options-read.ts +++ b/src/main/runtime/rpc/methods/structured-agent-session-options-read.ts @@ -20,6 +20,8 @@ import { OptionsParams } from './structured-agent-session-schemas' import { agentSessionPinnedLaunchDirectory } from '../../agent-session-record-launch-directory' +import { agentSessionRefusalError } from '../../../../shared/agent-session-wire-refusals' +import { clientReadsOptionsWithoutModel } from './structured-agent-session-policy' import type { AgentSessionModelCatalogResult } from '../../../../shared/agent-session-wire' export const STRUCTURED_AGENT_SESSION_OPTIONS_READ_METHODS = [ @@ -41,8 +43,17 @@ export const STRUCTURED_AGENT_SESSION_OPTIONS_READ_METHODS = [ name: 'agentSession.options', permission: 'workspace', params: OptionsParams, - handler: async (params, ctx) => - (await requireInstalledStructuredHost(ctx, params.sessionId)).readOptions(params.sessionId) + handler: async (params, ctx) => { + const result = await ( + await requireInstalledStructuredHost(ctx, params.sessionId) + ).readOptions(params.sessionId) + if (result.current.model === undefined && !clientReadsOptionsWithoutModel(ctx)) { + throw agentSessionRefusalError('structured_agent_session_unsupported', { + reason: 'clientCapabilityMissing' + }) + } + return result + } }), defineMethod({ name: 'agentSession.modelCatalog', diff --git a/src/main/runtime/rpc/methods/structured-agent-session-policy.ts b/src/main/runtime/rpc/methods/structured-agent-session-policy.ts index a920a2c4192..bdac8dc6d67 100644 --- a/src/main/runtime/rpc/methods/structured-agent-session-policy.ts +++ b/src/main/runtime/rpc/methods/structured-agent-session-policy.ts @@ -6,10 +6,20 @@ import { STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY, type RuntimeCapability } from '../../../../shared/protocol-version' +import { AGENT_SESSION_OPTIONAL_MODEL_CLIENT_CAPABILITY } from '../../../../shared/agent-session-optional-model-capability' import { isNativeChatEnabled } from '../../../../shared/structured-native-chat-launch-route' import type { OrcaRuntimeService } from '../../orca-runtime' import type { RpcContext } from '../core' +export function clientReadsOptionsWithoutModel( + context: Pick +): boolean { + return ( + context.clientKind === undefined || + context.clientCapabilities?.includes(AGENT_SESSION_OPTIONAL_MODEL_CLIENT_CAPABILITY) === true + ) +} + /** * One rule for every caller: can this client read structured sessions? The host's own * The host's Chat UI setting is not consulted. It is the host user's launch preference, diff --git a/src/renderer/src/components/native-chat/NativeChatSessionOptionPickers.test.tsx b/src/renderer/src/components/native-chat/NativeChatSessionOptionPickers.test.tsx index 770547021d3..8b7186d42ff 100644 --- a/src/renderer/src/components/native-chat/NativeChatSessionOptionPickers.test.tsx +++ b/src/renderer/src/components/native-chat/NativeChatSessionOptionPickers.test.tsx @@ -3,6 +3,12 @@ import { cleanup, render, screen, waitFor } from '@testing-library/react' import type * as ReactModule from 'react' import { afterEach, describe, expect, it, vi } from 'vitest' +import { OMP_SESSION_OPTION_CATALOG } from '../../../../shared/agent-session-option-catalog-omp' +import { + applyStructuredAgentSessionOptions, + createStructuredAgentSessionOptionState, + structuredAgentSessionOptionSnapshot +} from '../../../../shared/structured-agent-session-options' import type { SessionOptionDescriptor } from '../../../../shared/native-chat-session-options' import { RuntimeRpcCallError } from '@/runtime/runtime-rpc-result' @@ -421,6 +427,23 @@ describe('NativeChatSessionOptionPickers', () => { ) }) + it('renders a session with no reported model without an empty model control', () => { + const state = applyStructuredAgentSessionOptions( + createStructuredAgentSessionOptionState('omp'), + OMP_SESSION_OPTION_CATALOG, + { models: [], current: { effort: 'off', confirmed: ['effort'] } } + ) + render( + + ) + expect(screen.queryByRole('button', { name: /^Model/ })).toBeNull() + expect(screen.queryAllByRole('button', { name: '' })).toEqual([]) + }) + it('does not duplicate titles for unknown values or misname generic controls', () => { const { rerender } = render( ) } describe('structured agent session options', () => { + it('keeps a missing model out of the picker and adopts a later report', () => { + const state = applyStructuredAgentSessionOptions( + createStructuredAgentSessionOptionState('omp'), + OMP_SESSION_OPTION_CATALOG, + { models: [], current: { effort: 'off', confirmed: ['effort'] } } + ) + expect(state.catalog?.models).toEqual([]) + expect(structuredAgentSessionOptionSnapshot(state)).toEqual([]) + expect(state.record.model).toBeUndefined() + + const reported = applyStructuredAgentSessionOptions(state, OMP_SESSION_OPTION_CATALOG, { + models: [{ id: 'reported-model', label: 'Reported Model', isDefault: true, efforts: [] }], + current: { model: 'reported-model', confirmed: ['model'] } + }) + expect(structuredAgentSessionOptionSnapshot(reported)[0]).toMatchObject({ + valueSource: 'reported', + kind: { + currentValue: 'reported-model', + choices: [{ value: 'reported-model', label: 'Reported Model' }] + } + }) + }) + it('projects native Codex selects while bridge Codex keeps its agent picker', () => { const state = applyStructuredAgentSessionOptions( createStructuredAgentSessionOptionState('codex'), diff --git a/src/shared/structured-agent-session-options.ts b/src/shared/structured-agent-session-options.ts index 1d1d559eaae..4b9f1575175 100644 --- a/src/shared/structured-agent-session-options.ts +++ b/src/shared/structured-agent-session-options.ts @@ -79,7 +79,7 @@ export function structuredAgentSessionOptionCatalog( const models: CatalogModel[] = result.models.map((model) => discoveredModel(model, result.fastModeSupport?.supported === true) ) - if (!models.some((model) => model.id === result.current.model)) { + if (result.current.model && !models.some((model) => model.id === result.current.model)) { models.push({ id: result.current.model, label: result.current.model, @@ -176,12 +176,12 @@ export function applyStructuredAgentSessionOptions( result: AgentSessionOptionsResult ): StructuredAgentSessionOptionState { if (result.current.fastMode === undefined) { - clearTrackedSessionOption(state.record, result.current.model, 'fastMode') + clearTrackedSessionOption(state.record, result.current.model ?? null, 'fastMode') } applyNativeChatReportedSessionOptions( state.record, { - model: result.current.model, + ...(result.current.model ? { model: result.current.model } : {}), ...(result.current.effort ? { effort: result.current.effort } : {}), ...(result.current.fastMode !== undefined ? { fastMode: result.current.fastMode } : {}) }, diff --git a/tests/e2e/cross-version-wire/cross-version-agent-session-model-options.unit.test.ts b/tests/e2e/cross-version-wire/cross-version-agent-session-model-options.unit.test.ts new file mode 100644 index 00000000000..2f952fb959b --- /dev/null +++ b/tests/e2e/cross-version-wire/cross-version-agent-session-model-options.unit.test.ts @@ -0,0 +1,102 @@ +import { afterEach, beforeAll, describe, expect, it } from 'vitest' +import { AGENT_SESSION_OPTIONAL_MODEL_CLIENT_CAPABILITY } from '../../../src/shared/agent-session-optional-model-capability' +import { STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY } from '../../../src/shared/protocol-version' +import { ELECTRON_REMOTE_RUNTIME_CLIENT_CAPABILITIES } from '../../../src/shared/electron-remote-runtime-client-capabilities' +import { remoteRuntimeClientCapabilities } from '../../../src/shared/remote-runtime-client-capabilities' +import { resolveBaselineReleaseRef } from './release-checkout' +import { callBuild } from './structured-agent-session-surface-execution' +import { installableHost, structuredHostStub } from './structured-agent-session-host-fixture' +import { + loadAgentSessionWireBuild, + WORKING_TREE, + type AgentSessionWireBuild +} from './versioned-agent-session-wire' + +let current: AgentSessionWireBuild +let baseline: AgentSessionWireBuild +const SESSION = 'model-options-session' + +beforeAll(async () => { + current = await loadAgentSessionWireBuild(WORKING_TREE) + baseline = await loadAgentSessionWireBuild(resolveBaselineReleaseRef()) +}, 180_000) + +afterEach(async () => { + await current.installStructuredHost(null) + if (baseline.methodNames.includes('agentSession.options')) { + await baseline.installStructuredHost(null) + } +}) + +describe('selected model reports across released client and host versions', () => { + it('refuses an absent model only for an older client against the new host', async () => { + const host = structuredHostStub(SESSION, 'folder-options') + const report = { models: [], current: { effort: 'off' } } + host.readOptions.mockResolvedValue(report) + await current.installStructuredHost(installableHost(host)) + const legacy = [ + ...baseline.capabilities.filter( + (value) => value !== AGENT_SESSION_OPTIONAL_MODEL_CLIENT_CAPABILITY + ), + STRUCTURED_AGENT_SESSION_RUNTIME_CAPABILITY + ] + expect( + await callBuild( + current, + 'agentSession.options', + { sessionId: SESSION }, + { + clientKind: 'runtime', + clientCapabilities: legacy + } + ) + ).toMatchObject([{ ok: false, error: { message: 'structured_agent_session_unsupported' } }]) + expect( + await callBuild( + current, + 'agentSession.options', + { sessionId: SESSION }, + { + clientKind: 'runtime', + clientCapabilities: remoteRuntimeClientCapabilities( + ELECTRON_REMOTE_RUNTIME_CLIENT_CAPABILITIES + ) + } + ) + ).toMatchObject([{ ok: true, result: report }]) + host.readOptions.mockResolvedValue({ models: [], current: { model: 'reported-model' } }) + expect( + await callBuild( + current, + 'agentSession.options', + { sessionId: SESSION }, + { + clientKind: 'runtime', + clientCapabilities: legacy + } + ) + ).toMatchObject([{ ok: true, result: { current: { model: 'reported-model' } } }]) + }) + + it('lets a new desktop client read existing model reports from the released host', async () => { + const client = { + clientKind: 'runtime' as const, + clientCapabilities: remoteRuntimeClientCapabilities( + ELECTRON_REMOTE_RUNTIME_CLIENT_CAPABILITIES + ) + } + if (!baseline.methodNames.includes('agentSession.options')) { + expect( + await callBuild(baseline, 'agentSession.options', { sessionId: SESSION }, client) + ).toMatchObject([{ ok: false, error: { code: 'method_not_found' } }]) + return + } + const host = structuredHostStub(SESSION, 'folder-options') + const report = { models: [], current: { model: 'released-model', effort: 'off' } } + host.readOptions.mockResolvedValue(report) + await baseline.installStructuredHost(installableHost(host)) + expect( + await callBuild(baseline, 'agentSession.options', { sessionId: SESSION }, client) + ).toMatchObject([{ ok: true, result: report }]) + }) +})