Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
115 changes: 115 additions & 0 deletions src/main/acp/acp-structured-missing-model.test.ts
Original file line number Diff line number Diff line change
@@ -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' })
})
})
4 changes: 2 additions & 2 deletions src/main/acp/acp-structured-options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -134,7 +134,7 @@ export class AcpStructuredOptions {
return {
models,
current: {
model: currentModel,
...(currentModel ? { model: currentModel } : {}),
...(effortOption ? { effort: effortOption.currentValue } : {}),
confirmed
}
Expand Down
57 changes: 57 additions & 0 deletions src/main/acp/fixtures/omp-v17-windows-new-no-model.json
Original file line number Diff line number Diff line change
@@ -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"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
2 changes: 1 addition & 1 deletion src/main/codex/codex-structured-model-catalog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ function modelOption(value: unknown): ParsedCodexModelOption | null {
}

export type CodexSessionOptionCatalog = {
result: AgentSessionOptionsResult
result: AgentSessionOptionsResult & { current: { model: string } }
fastModeTierByModel: Map<string, string>
}

Expand Down
2 changes: 2 additions & 0 deletions src/main/ipc/desktop-renderer-runtime-capabilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export function nativeSessionOptionsFromReport(input: {
: encodeStructuredAgentSessionOptionValue('fastMode', reported.fastMode)
const options: Record<string, string> = {
...restored,
model: reported.model,
...(reported.model ? { model: reported.model } : {}),
...(reported.effort ? { effort: reported.effort } : {}),
...(fastMode !== undefined && fastMode !== null ? { fastMode } : {})
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<AgentModelCatalogSuccess>(() => {}))
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<unknown>) => 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<AgentModelCatalogSuccess>(() => {}))
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<unknown>) => 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', () => {
Expand Down Expand Up @@ -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()
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -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 ??
Expand All @@ -69,7 +68,7 @@ async function readStructuredAgentSessionOptionsAtRest(
? { fastModeSupport: catalog.fastModeSupport }
: {}),
current: {
model,
...(model ? { model } : {}),
...(effort ? { effort } : {}),
...(typeof fastMode === 'boolean' ? { fastMode } : {})
}
Expand Down
Loading
Loading