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
133 changes: 123 additions & 10 deletions src/main/claude/claude-structured-effort-default-at-rest.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { agentModelCatalogFingerprintForRecord } from '../native-chat/agent-mode
import { AgentModelCatalogStore } from '../native-chat/agent-model-catalog/agent-model-catalog-store'
import type { StructuredAgentSessionMutationContext } from '../native-chat/agent-session-wire/structured-agent-session-host-mutations'
import { readStructuredAgentSessionOptions } from '../native-chat/agent-session-wire/structured-agent-session-options-read'
import { composeCodexSessionOptionCatalog } from '../codex/codex-structured-model-catalog'
import { nativeSessionOptionsFromReport } from '../native-chat/agent-session-wire/structured-agent-session-option-restoration'
import {
ClaudeStructuredSessionAdapter,
Expand Down Expand Up @@ -55,20 +56,22 @@ const CATALOG = [
}
]

/** get_settings with nothing overriding the CLI's own default; an effort write moves both views. */
function settingsWithNoOverride() {
/** get_settings with nothing overriding the CLI's own default; an effort write moves both views.
* `model` is what the CLI says it runs, and `listing` the models it lists. */
function settingsWithNoOverride(model = 'claude-opus-5-5[1m]', listing: unknown[] = CATALOG) {
const effective: { effortLevel?: string } = {}
const settings = {
effective,
sources: [],
applied: { model: 'claude-opus-5-5[1m]', effort: 'medium', advisor: null, ultracode: false }
applied: { model, effort: 'medium', advisor: null, ultracode: false }
}
const claude = fakeClaude({
initProof: 'session-start',
initModels: CATALOG,
initModel: model,
initModels: listing,
settings,
routes: {
list_models: () => CATALOG,
list_models: () => listing,
apply_flag_settings: (params) => {
const written = params?.settings
const effort =
Expand All @@ -88,7 +91,8 @@ function settingsWithNoOverride() {
async function startChild(
store: AgentModelCatalogStore,
options?: Record<string, string>,
events: ClaudeStructuredSessionEvent[] = []
events: ClaudeStructuredSessionEvent[] = [],
cli: { runs?: string; lists?: unknown[] } = {}
): Promise<ClaudeStructuredSessionAdapter> {
const adapter = new ClaudeStructuredSessionAdapter({
resolveLaunch: async () => ({
Expand All @@ -102,7 +106,7 @@ async function startChild(
continuesChain: false
}),
onEvent: (event) => events.push(event),
openConnection: settingsWithNoOverride().openConnection,
openConnection: settingsWithNoOverride(cli.runs, cli.lists).openConnection,
readProcessStartTime: async () => 1_700_000_000_000,
now: () => 1_700_000_000_500,
persistHandle: async () => {},
Expand Down Expand Up @@ -152,10 +156,10 @@ function readAtRest(store: AgentModelCatalogStore, record: AgentSessionRecord) {
return readStructuredAgentSessionOptions(context, SESSION)
}

function pickerEffort(result: Awaited<ReturnType<typeof readAtRest>>) {
const seed = getAgentSessionOptionCatalog('claude')!
function pickerEffort(result: Awaited<ReturnType<typeof readAtRest>>, agent = 'claude') {
const seed = getAgentSessionOptionCatalog(agent)!
const state = applyStructuredAgentSessionOptions(
createStructuredAgentSessionOptionState('claude', seed),
createStructuredAgentSessionOptionState(agent, seed),
seed,
result
)
Expand Down Expand Up @@ -206,6 +210,115 @@ describe('Claude effort default at rest', () => {
expect(catalogDefault(other, 'opus[1m]')).toBe('medium')
})

it('offers no effort for a model the catalog does not list, at rest as live', async () => {
const store = new AgentModelCatalogStore()
const unlisted = { model: 'claude-unlisted-9', effort: 'medium' }
// The CLI runs a model its own catalog does not list, as a newer or pinned model can be.
const child = await startChild(store, unlisted, [], { runs: unlisted.model })
const live = await child.readOptions({ sessionId: SESSION, fence: 7 })

const resting = await readAtRest(store, restingRecord(unlisted))

const row = (result: typeof resting) =>
result.models.find((entry) => entry.id === unlisted.model)
expect(row(live)).toEqual(expect.objectContaining({ id: unlisted.model, efforts: [] }))
expect(row(resting)).toEqual(row(live))
expect(pickerEffort(live)).toBeUndefined()
expect(pickerEffort(resting)).toBeUndefined()
})

it('lists an unlisted Codex model at rest exactly as its live child does', async () => {
const store = new AgentModelCatalogStore()
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the resting read and the catalog key touch only these fields.
const record = {
provider: 'codex',
accountHome: { variable: 'CODEX_HOME', path: '/accounts/codex' },
location: { wslDistro: null },
options: { model: 'gpt-unlisted', effort: 'high' }
} as unknown as AgentSessionRecord
const listing = {
models: [
{
id: 'gpt-5.5',
label: 'GPT-5.5',
isDefault: true,
efforts: [
{ value: 'medium', label: 'Medium' },
{ value: 'high', label: 'High' }
]
}
],
fastModeTierByModel: new Map<string, string>()
}
store.recordSuccess(agentModelCatalogFingerprintForRecord(record), 'codex', {
...listing,
origin: 'live-session'
})

const resting = await readAtRest(store, record)
const live = composeCodexSessionOptionCatalog(listing, {
current: { model: 'gpt-unlisted', effort: 'high' }
}).result

expect(resting.models).toEqual(live.models)
expect(live.models.find((entry) => entry.id === 'gpt-unlisted')).toMatchObject({ efforts: [] })
expect(pickerEffort(resting, 'codex')).toBeUndefined()
})

it("keeps a Claude model's effort at rest with no catalog yet, as a live child with none does", async () => {
const saved = { model: 'opus', effort: 'high' }
// A CLI that lists nothing: the live child falls back to Claude's built-in models.
const child = await startChild(new AgentModelCatalogStore(), saved, [], {
runs: 'opus',
lists: []
})
const live = await child.readOptions({ sessionId: SESSION, fence: 7 })

// An account the host has no catalog for yet.
const resting = await readAtRest(new AgentModelCatalogStore(), restingRecord(saved))

expect(resting.models).toEqual(live.models)
expect(pickerEffort(live)).toBe('high')
expect(pickerEffort(resting)).toBe('high')
})

it('names no model at rest with no pick and no catalog, so the client keeps what the child ran', async () => {
// The CLI's default model, reported live; the chat never picked one.
const child = await startChild(new AgentModelCatalogStore(), undefined, [], {
runs: 'opus',
lists: []
})
const live = await child.readOptions({ sessionId: SESSION, fence: 7 })

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('')
const seed = getAgentSessionOptionCatalog('claude')!
const state = [live, resting].reduce(
(current, answer) => applyStructuredAgentSessionOptions(current, seed, answer),
createStructuredAgentSessionOptionState('claude', seed)
)
const model = structuredAgentSessionOptionSnapshot(state).find((row) => row.id === 'model')
expect(model?.kind.type === 'select' ? model.kind.currentValue : undefined).toBe('opus')
})

it("keeps a Codex model's effort at rest with no catalog yet, from the client's own defaults", async () => {
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the resting read and the catalog key touch only these fields.
const record = {
provider: 'codex',
accountHome: { variable: 'CODEX_HOME', path: '/accounts/codex' },
location: { wslDistro: null },
options: { model: 'gpt-5.5', effort: 'high' }
} as unknown as AgentSessionRecord

const resting = await readAtRest(new AgentModelCatalogStore(), record)

// A Codex child lists nothing without its catalog, so the answer leaves the list to the client.
expect(resting.models).toEqual([])
expect(pickerEffort(resting, 'codex')).toBe('high')
})

it("leaves a Codex chat's unsaved effort blank at rest, as its live child does", async () => {
const store = new AgentModelCatalogStore()
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the resting read and the catalog key touch only these fields.
Expand Down
18 changes: 13 additions & 5 deletions src/main/claude/claude-structured-session-options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
type ListedModel
} from './claude-structured-model-catalog'
import type { ClaudeSession } from './claude-structured-session-state'
import { structuredAgentSessionOptionModels } from '../native-chat/agent-session-wire/structured-agent-session-option-models'
import { decodeStructuredAgentSessionOptionValue } from '../../shared/structured-agent-session-option-codec'

/**
Expand Down Expand Up @@ -240,6 +241,12 @@ function wireClaudeModels(models: readonly ListedModel[]): WireClaudeModel[] {
return models.map(wireClaudeModel)
}

/** The built-in models a running child lists when the CLI gives it none; a chat at rest with no
* catalog lists the same. */
export function claudeFallbackModelOptions(): WireClaudeModel[] {
return wireClaudeModels(seedModels())
}

/** The listing, with what the CLI runs when no effort is sent on each model the child applies —
* a default only a running child knows, and only while this session has no effort pick. */
function catalogClaudeModels(session: ClaudeSession, discovered: ListedModel[]): WireClaudeModel[] {
Expand Down Expand Up @@ -326,12 +333,13 @@ export function claudeStructuredSessionOptionsFrom(
): AgentSessionOptionsResult {
const discovered = listedModels(catalog ? { models: catalog } : null)
writeClaudeCatalogThrough(session, discovered)
const models = discovered.length > 0 ? discovered : seedModels()
const listed = discovered.length > 0 ? discovered : seedModels()
const current = readClaudeCurrentModel(session)
const model = currentModelId(models, current.id)
if (!models.some((entry) => entry.id === model)) {
models.push({ id: model, label: model, isDefault: false, efforts: [], resolvedModel: null })
}
const model = currentModelId(listed, current.id)
const models = structuredAgentSessionOptionModels(listed, model, (row) => ({
...row,
resolvedModel: null
}))
const effort =
session.options.get('effort') ??
session.reportedOptions.effort ??
Expand Down
15 changes: 6 additions & 9 deletions src/main/codex/codex-structured-model-catalog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import type {
AgentSessionOptionsResult
} from '../../shared/agent-session-wire'
import type { CodexAppServerConnection } from './codex-app-server-connection'
import { structuredAgentSessionOptionModels } from '../native-chat/agent-session-wire/structured-agent-session-option-models'
import { codexFastModeSupport, readCodexFastModeTier } from './codex-structured-fast-mode'
import {
applyCodexConfiguredLaunchDefaults,
Expand Down Expand Up @@ -141,15 +142,11 @@ export function composeCodexSessionOptionCatalog(
reportedServiceTierKnown?: boolean
}
): CodexSessionOptionCatalog {
const models = listing.models.map((entry) => ({ ...entry }))
if (input.current.model && !models.some((model) => model.id === input.current.model)) {
models.push({
id: input.current.model,
label: input.current.model,
isDefault: false,
efforts: []
})
}
const models = structuredAgentSessionOptionModels(
listing.models.map((entry) => ({ ...entry })),
input.current.model,
(row) => row
)
const model = input.current.model ?? models.find((entry) => entry.isDefault)?.id ?? models[0]?.id
if (!model) {
throw new Error('codex app-server returned no available models')
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// The models a session's options answer lists, the same whether a child answers live or the host
// answers for a chat at rest: the catalog's rows, plus the current model when the catalog does not
// list it. Nothing says which efforts an unlisted model takes, so its row offers none, and a client
// shows no effort control for it.

import type { AgentSessionModelOption } from '../../../shared/agent-session-wire'

/** `row` builds a provider's own row type from the shared one. */
export function structuredAgentSessionOptionModels<TModel extends AgentSessionModelOption>(
catalog: readonly TModel[],
current: string | undefined,
row: (unlisted: AgentSessionModelOption) => TModel
): TModel[] {
const models = [...catalog]
if (current && !models.some((entry) => entry.id === current)) {
models.push(row({ id: current, label: current, isDefault: false, efforts: [] }))
}
return models
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,33 @@

import {
refuse,
type AgentSessionModelOption,
type AgentSessionOptionResult,
type AgentSessionOptionsResult
} from '../../../shared/agent-session-wire'
import type { AgentSessionRecord } from '../../../shared/agent-session-record'
import { claudeFallbackModelOptions } from '../../claude/claude-structured-session-options'
import { decodeStructuredAgentSessionOptionValue } from '../../../shared/structured-agent-session-option-codec'
import type { AgentSessionRecordStore } from '../../runtime/agent-session-record-store'
import { journalOpenReadRefusal } from '../agent-session-journal/journal-open-failure'
import { isClaudeStructuredOptionKey } from '../../claude/claude-structured-options'
import { isCodexTurnOptionKey } from '../../codex/codex-structured-turn-start'
import type { StructuredAgentSessionHostDeps } from './structured-agent-session-host-types'
import { structuredAgentSessionOptionModels } from './structured-agent-session-option-models'
import type { AgentSessionTurnContext, TurnOutcome } from './structured-agent-session-turns'
import type { StructuredAgentSessionMutationContext } from './structured-agent-session-host-mutations'

type RestingOptions = Pick<AgentSessionOptionsResult, 'models' | 'fastModeSupport' | 'current'>

/** With no catalog for the account, the list a running child falls back to: Claude's built-in
* models. A Codex child has no such list, so none: the client fills the current model from its
* own unknown-model defaults, unchanged from before this list was shared. */
function restingFallbackModels(
provider: AgentSessionRecord['provider']
): AgentSessionModelOption[] | null {
return provider === 'claude' ? claudeFallbackModelOptions() : null

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

claudeFallbackModelOptions() keeps isDefault: true on the seed's default row. The at-rest answer sends this list with current.model: '', and the client's structuredAgentSessionOptionCatalog unconditionally sets defaultModelIsCliDefault: true, so the picker names Sonnet even though the host named no model — exactly the guess the comment above says to avoid. Before this PR the no-catalog list was empty, so nothing was named, and a running agent always reports its real model so it never falls back to the seed default either.

Technical details
# No-catalog Claude fallback names a guessed default

## Affected sites
- `src/main/native-chat/agent-session-wire/structured-agent-session-options-read.ts:34` — fallback rows retain `isDefault` from `seedModels()`.
- `src/main/claude/claude-structured-session-options.ts:246` — `claudeFallbackModelOptions` wires `seedModels()` including `isDefault`.
- `src/shared/structured-agent-session-options.ts:89` — client `structuredAgentSessionOptionCatalog` sets `defaultModelIsCliDefault: true` unconditionally.
- `src/shared/native-chat-session-option-snapshot.ts:190` — `cliDefaultModelId` then returns `models.find((m) => m.isDefault)?.id`.

## Required outcome
- A no-catalog, no-saved-pick resting answer must not render a selected model, matching both the pre-PR behavior and a running agent (which reports its actual model).

## Suggested approach (optional)
- Strip `isDefault` from the no-catalog fallback, or gate the client's `defaultModelIsCliDefault` on a non-empty `current.model`.
- `claude-structured-effort-default-at-rest.test.ts:280` asserts the resting and live fallback lists are equal, so stripping only the resting side needs that expectation reconciled.

## Open questions for the human (optional)
- Is a record with no saved model actually reachable at rest (e.g. a first start that never proved)? The added no-pick test treats it as reachable; if it is not, the origin guard here is dead code. The new test only checks `resting.current.model === ''` and applies a prior live read first, so it does not cover the rendered picker.

}

async function readStructuredAgentSessionOptionsAtRest(
deps: Pick<StructuredAgentSessionHostDeps, 'store' | 'modelCatalog'>,
sessionId: string
Expand All @@ -32,14 +45,20 @@ async function readStructuredAgentSessionOptionsAtRest(
const catalog = (await deps.modelCatalog
?.read({ agent: record.provider, sessionId })
.catch(() => null)) ?? { origin: 'unknown' as const }
const models = catalog.origin === 'unknown' ? [] : catalog.models
const listed =
catalog.origin === 'unknown' ? restingFallbackModels(record.provider) : catalog.models
Comment on lines +48 to +49

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Unverified Claude model appears selected When a Claude chat has no saved model and the host has no catalog, this fallback list marks sonnet as the default even though the response leaves the current model empty. On a first resting read, the client therefore shows sonnet as selected without knowing what the account runs, and a user can explicitly pick and persist that guess. The new no-pick test applies a live answer first, so it does not cover this case.

Knowledge Base Used: Agent workspaces and sessions

const models = listed ?? []
const saved = record.options ?? {}
const fastMode =
saved.fastMode === undefined
? null
: decodeStructuredAgentSessionOptionValue('fastMode', saved.fastMode)
// An unknown model is one the client already treats as unconfirmed.
const model = saved.model ?? models.find((entry) => entry.isDefault)?.id ?? ''
// An unknown model is one the client already treats as unconfirmed. Only a real listing names the
// 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) ??
''
// As a live child answers: the pick, else what Claude runs for this model when none is sent.
// A live Codex child answers only the effort its thread reported, never the model's default.
const effort =
Expand All @@ -48,7 +67,7 @@ async function readStructuredAgentSessionOptionsAtRest(
? models.find((entry) => entry.id === model)?.defaultEffort
: undefined)
return {
models,
models: listed ? structuredAgentSessionOptionModels(listed, model, (row) => row) : [],
...(catalog.origin !== 'unknown' && catalog.fastModeSupport
? { fastModeSupport: catalog.fastModeSupport }
: {}),
Expand Down
Loading