From b1e12d7bb41db6c8de03951d8323cfff4a3e1329 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sun, 4 Oct 2026 14:37:40 -0700 Subject: [PATCH] fix(native-chat): a new structured chat's model list comes from the machine that runs it (#25143) * fix(native-chat): a new structured chat's model list comes from the host that runs it agentSession.modelCatalog builds the structured-session host like agentSession.options, so a host with no saved chats since it started answers instead of refusing. When the host answers unknown because its first listing runs in the background, the picker re-reads on a bounded schedule until that listing lands. Local terminal-backed chat skips the structured catalog when a custom launch command is configured. * fix(native-chat): wait on the host's first model listing instead of re-reading on a timer A new structured chat's picker read the host catalog once; on an account the host had never listed, the answer was "unknown" while a background listing ran, and the client re-read on a 1-30 s schedule. Replace the schedule with the host's own completion signal: - The host answers a cold read with `listingInProgress: true` (new optional field) once it has started or joined that listing. A read that passes the new optional `waitForListing` param awaits the same joined listing and answers with it, or a plain "unknown" if it failed. The at-rest options read never waits. A host that predates the field never sends it, so the client never sends the param to a host that would refuse it. - The picker reads once per open and attach; after the host's report it sends one waiting read, with a 90 s client timeout above the slowest listing. While that read is out, the model pill keeps its label but cannot open or be set (typed /model included). An answer, failure, timeout, hide, attach or the provider's own list releases it. - `agentSession.modelCatalog` builds the structured-session host only for a read that names a session (a structured chat). Terminal-backed chat's session-less read keeps the non-building gate, so a desktop that never runs structured chat never opens the session journal. * fix(native-chat): one waiting model-list read per chat, and no late menu open The waiting catalog read was owned by one run of the picker's effect. Attach (a new fence), hide/show or a send re-ran the effect: the cleanup released the model picker onto the built-in list for a round trip, and the new run sent a second waiting read while the first, which cannot be withdrawn, kept a remote call slot until the listing ended. The waiting read now belongs to the chat (runtime target + agent + session): a small registry keeps one in flight per chat, every re-run or remount joins it, and the entry is deleted when the read settles. The picker hold is derived from that entry being in flight, so it lasts across attach and hide/show and ends when the read settles, the provider reports its own list, or the pane switches to another session. Answers still pass the stale and record checks. A bare /model typed while the list loads no longer opens the model menu by itself when the list lands: the menu stays keyed on the request, and only its initial open is suppressed while pending, so the request is spent shut and the end of the pending period never remounts it. * fix(native-chat): release the model picker in the same commit as the host list When the waiting catalog read settled in the chat that started it, the registry dropped its entry and told subscribers first, and the host list was applied a few microtasks later. React committed once with the picker enabled on the built-in list, then again with the host's list. Joiners now hand the registry their apply callback, and the registry runs every joiner (with the answer, or nothing when the read failed or timed out) before it deletes the entry and notifies. The release and the list land in one commit. An effect cleanup leaves the wait instead of flagging itself stale. * fix(runtime): queue model catalog reads in the long-wait lane A model catalog read that waits on a host's first listing replies only when that listing ends, yet it took one of the 8 foreground call slots for its server. Enough chats opened during one cold listing would stall that server's sends and interrupts until a wait settled. agentSession.modelCatalog now joins worktree.rm in the long-wait lane: same concurrency, counted apart from the foreground calls. The queue classifies by method only, and a warm catalog read answers at once, so the whole method moves. --- .../agent-model-catalog-service.test.ts | 105 +++++- .../agent-model-catalog-service.ts | 32 +- ...uctured-agent-session-options-read.test.ts | 48 +++ ...uctured-agent-session-options-read.test.ts | 62 +++ .../structured-agent-session-options-read.ts | 10 +- ...SessionOptionPickers.menu-request.test.tsx | 109 ++++++ .../NativeChatSessionOptionPickers.test.tsx | 38 ++ .../NativeChatSessionOptionPickers.tsx | 7 +- .../native-chat/host-model-listing-waits.ts | 62 +++ .../native-chat-session-option-discovery.ts | 8 +- ...ive-chat-session-option-enrichment.test.ts | 18 + .../use-host-model-catalog-upgrade.test.tsx | 357 ++++++++++++++++++ .../use-host-model-catalog-upgrade.ts | 74 +++- .../use-structured-agent-session-options.ts | 13 +- .../structured-agent-session-client.test.ts | 24 ++ .../structured-agent-session-client.ts | 14 +- src/shared/agent-session-wire.ts | 7 +- src/shared/native-chat-session-options.ts | 2 + .../structured-agent-session-params.ts | 7 +- src/shared/runtime-rpc-call-queue.test.ts | 20 + src/shared/runtime-rpc-call-queue.ts | 7 +- .../structured-agent-session-options.ts | 11 + 22 files changed, 989 insertions(+), 46 deletions(-) create mode 100644 src/main/native-chat/agent-session-wire/structured-agent-session-options-read.test.ts create mode 100644 src/renderer/src/components/native-chat/NativeChatSessionOptionPickers.menu-request.test.tsx create mode 100644 src/renderer/src/components/native-chat/host-model-listing-waits.ts create mode 100644 src/renderer/src/components/native-chat/use-host-model-catalog-upgrade.test.tsx diff --git a/src/main/native-chat/agent-model-catalog/agent-model-catalog-service.test.ts b/src/main/native-chat/agent-model-catalog/agent-model-catalog-service.test.ts index d51a98b3d8b..02025485151 100644 --- a/src/main/native-chat/agent-model-catalog/agent-model-catalog-service.test.ts +++ b/src/main/native-chat/agent-model-catalog/agent-model-catalog-service.test.ts @@ -5,7 +5,11 @@ import { agentModelCatalogFingerprintForRecord } from './agent-model-catalog-fingerprint' import { createAgentModelCatalogService } from './agent-model-catalog-service' -import { AgentModelCatalogStore, type AgentModelCatalogSuccess } from './agent-model-catalog-store' +import { + AGENT_MODEL_CATALOG_FRESH_MS, + AgentModelCatalogStore, + type AgentModelCatalogSuccess +} from './agent-model-catalog-store' function record(accountHomePath: string): AgentSessionRecord { // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the service reads only provider, accountHome and location; the rest of the record is irrelevant here. @@ -55,7 +59,8 @@ describe('agent model catalog service', () => { probes: { codex: probe } }) expect(await service.read({ agent: 'codex', sessionId: 'session-1' })).toEqual({ - origin: 'unknown' + origin: 'unknown', + listingInProgress: true }) // A second read while the probe is in flight must not start another, and a // record-scoped read probes the RECORD's pinned home, not the selection. @@ -81,7 +86,10 @@ describe('agent model catalog service', () => { probes: { codex: probe } }) // The record-less read follows the CURRENT selection: unknown, never gpt-old. - expect(await service.read({ agent: 'codex' })).toEqual({ origin: 'unknown' }) + expect(await service.read({ agent: 'codex' })).toEqual({ + origin: 'unknown', + listingInProgress: true + }) expect(probe).toHaveBeenCalledWith('/homes/new') await vi.waitFor(async () => { const result = await service.read({ agent: 'codex' }) @@ -139,7 +147,8 @@ describe('agent model catalog service', () => { probes: { codex: probe } }) expect(await service.read({ agent: 'codex', sessionId: 'session-1' })).toEqual({ - origin: 'unknown' + origin: 'unknown', + listingInProgress: true }) await vi.waitFor(() => expect(probe).toHaveBeenCalledTimes(1)) // Still a clean unknown — and the failure TTL suppresses a probe storm. @@ -164,6 +173,94 @@ describe('agent model catalog service', () => { expect(probe).not.toHaveBeenCalled() }) + describe('a read that waits for the first listing', () => { + function deferredListing() { + let resolve!: (success: AgentModelCatalogSuccess) => void + let reject!: (error: Error) => void + const promise = new Promise((res, rej) => { + resolve = res + reject = rej + }) + return { promise, resolve, reject } + } + + function coldService(probe: (home: string) => Promise) { + const store = new AgentModelCatalogStore() + const service = createAgentModelCatalogService({ + store, + getRecord: () => undefined, + resolveAccountHome: async () => CODEX_HOME('/homes/selected'), + probes: { codex: probe } + }) + return { store, service } + } + + it('joins the listing the first read started and answers with it', async () => { + const pending = deferredListing() + const probe = vi.fn(() => pending.promise) + const { service } = coldService(probe) + expect(await service.read({ agent: 'codex' })).toEqual({ + origin: 'unknown', + listingInProgress: true + }) + const waited = service.read({ agent: 'codex', waitForListing: true }) + pending.resolve(listing('gpt-listed')) + const result = await waited + expect(result.origin === 'unknown' ? null : result.models[0]!.id).toBe('gpt-listed') + expect(probe).toHaveBeenCalledTimes(1) + }) + + it('answers a plain unknown when the listing fails', async () => { + const pending = deferredListing() + const { service } = coldService(() => pending.promise) + const waited = service.read({ agent: 'codex', waitForListing: true }) + pending.reject(new Error('spawn failed')) + expect(await waited).toEqual({ origin: 'unknown' }) + }) + + it('does not wait or report a listing while a failure is inside its TTL', async () => { + const probe = vi.fn(async (): Promise => { + throw new Error('spawn failed') + }) + const { store, service } = coldService(probe) + store.recordFailure(selectedHomeFingerprint('/homes/selected'), 'spawn failed') + expect(await service.read({ agent: 'codex', waitForListing: true })).toEqual({ + origin: 'unknown' + }) + expect(await service.read({ agent: 'codex' })).toEqual({ origin: 'unknown' }) + expect(probe).not.toHaveBeenCalled() + }) + + it('reports no listing where the host has no lister for the account', async () => { + const store = new AgentModelCatalogStore() + const service = createAgentModelCatalogService({ + store, + getRecord: () => undefined, + resolveAccountHome: async () => CODEX_HOME('/homes/selected') + }) + expect(await service.read({ agent: 'codex', waitForListing: true })).toEqual({ + origin: 'unknown' + }) + }) + + it('serves an aged entry at once and refreshes it behind the answer', async () => { + let now = 0 + const store = new AgentModelCatalogStore({ now: () => now }) + store.recordSuccess(selectedHomeFingerprint('/homes/selected'), 'codex', listing('gpt-old')) + now = AGENT_MODEL_CATALOG_FRESH_MS + const probe = vi.fn(() => new Promise(() => {})) + const service = createAgentModelCatalogService({ + store, + getRecord: () => undefined, + resolveAccountHome: async () => CODEX_HOME('/homes/selected'), + probes: { codex: probe } + }) + const result = await service.read({ agent: 'codex', waitForListing: true }) + expect(result.origin === 'unknown' ? null : result.models[0]!.id).toBe('gpt-old') + expect(probe).toHaveBeenCalledTimes(1) + }) + }) + describe('a read for the workspace a new chat runs in', () => { function serviceWith(mayOverride: boolean) { const store = new AgentModelCatalogStore() diff --git a/src/main/native-chat/agent-model-catalog/agent-model-catalog-service.ts b/src/main/native-chat/agent-model-catalog/agent-model-catalog-service.ts index 8c794331a2c..638b0f54272 100644 --- a/src/main/native-chat/agent-model-catalog/agent-model-catalog-service.ts +++ b/src/main/native-chat/agent-model-catalog/agent-model-catalog-service.ts @@ -35,6 +35,8 @@ export type AgentModelCatalogService = { sessionId?: string /** Where a new chat would run; null when one was named but is not a local directory. */ workspacePath?: string | null + /** With no entry yet, answer from the listing this read starts or joins instead of `unknown`. */ + waitForListing?: boolean }) => Promise } @@ -79,8 +81,9 @@ async function workspaceKeepsListedDefault( * launch); without one, the key is the account a launch would pin right now — * never "whichever account listed last". `unknown` tells the client to keep * its static seed, and a missing or aged entry kicks one joined background - * probe so the next read is warm. Failures are the store's 30s TTL, never an - * answer — a picker is a user surface and must not block. + * probe so the next read is warm. With no entry, the answer says that listing + * is running, and only a read that asks waits for it. Failures are the store's + * 30s TTL, never an answer: inside it a read answers `unknown` at once. */ export function createAgentModelCatalogService( deps: AgentModelCatalogServiceDeps @@ -110,14 +113,27 @@ export function createAgentModelCatalogService( }) accountHomePath = resolved.path } - const entry = deps.store.get(fingerprint) + let entry = deps.store.get(fingerprint) const probe = deps.probes?.[params.agent] - if (probe && accountHomePath && deps.store.shouldRefresh(fingerprint)) { - const home = accountHomePath - void deps.store.refresh(fingerprint, params.agent, () => probe(home)) - } + const home = accountHomePath + // Without an entry, join a running listing too: that is the one a waiting read answers from. + const listing = + probe && + home && + (entry ? deps.store.shouldRefresh(fingerprint) : !deps.store.hasActiveFailure(fingerprint)) + ? deps.store.refresh(fingerprint, params.agent, () => probe(home)) + : null if (!entry) { - return { origin: 'unknown' } + if (!listing) { + return { origin: 'unknown' } + } + if (!params.waitForListing) { + return { origin: 'unknown', listingInProgress: true } + } + entry = await listing + if (!entry) { + return { origin: 'unknown' } + } } return resultFromEntry( entry, 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 new file mode 100644 index 00000000000..5657fcfb4d8 --- /dev/null +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-options-read.test.ts @@ -0,0 +1,48 @@ +// A chat's options at rest come from the host catalog without waiting on a listing. + +import { describe, expect, it, vi } from 'vitest' +import type { AgentSessionRecord } from '../../../shared/agent-session-record' +import { createAgentModelCatalogService } from '../agent-model-catalog/agent-model-catalog-service' +import { + AgentModelCatalogStore, + type AgentModelCatalogSuccess +} from '../agent-model-catalog/agent-model-catalog-store' +import type { StructuredAgentSessionMutationContext } from './structured-agent-session-host-mutations' +import { readStructuredAgentSessionOptions } from './structured-agent-session-options-read' + +const SESSION = 'session-1' + +function restingRecord(): AgentSessionRecord { + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the resting read and the catalog key touch only these fields. + return { + provider: 'codex', + accountHome: { variable: 'CODEX_HOME', path: '/homes/a' }, + location: { wslDistro: null }, + options: {} + } as unknown as 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, + resolveAccountHome: async () => ({ variable: 'CODEX_HOME', path: '/homes/a' }), + probes: { codex: probe } + }) + const resting = { child: null, params: { provider: 'codex' } } + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the resting read touches only these members. + const context = { + deps: { adapter: {}, 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([]) + }) +}) diff --git a/src/main/runtime/rpc/methods/structured-agent-session-options-read.test.ts b/src/main/runtime/rpc/methods/structured-agent-session-options-read.test.ts index 77d44ece745..459ae5c75a4 100644 --- a/src/main/runtime/rpc/methods/structured-agent-session-options-read.test.ts +++ b/src/main/runtime/rpc/methods/structured-agent-session-options-read.test.ts @@ -56,4 +56,66 @@ describe('agentSession.modelCatalog', () => { await call('agentSession.modelCatalog', { agent: 'claude' }, STRUCTURED_CLIENT) expect(read).toHaveBeenCalledWith({ agent: 'claude' }) }) + + it('passes a wait for the listing through to the catalog', async () => { + await call( + 'agentSession.modelCatalog', + { agent: 'codex', sessionId: SESSION, waitForListing: true }, + STRUCTURED_CLIENT + ) + expect(read).toHaveBeenCalledWith({ agent: 'codex', sessionId: SESSION, waitForListing: true }) + }) +}) + +describe('agentSession.modelCatalog before anything built the host', () => { + const read = vi.fn(async () => ({ + origin: 'probe' as const, + models: [{ id: 'gpt-host', label: 'GPT Host', isDefault: true, efforts: [] }], + fetchedAt: 1 + })) + const installHost = vi.fn(async () => { + setStructuredAgentSessionHost(Object.assign(hostStub(), { deps: { modelCatalog: { read } } })) + }) + + beforeEach(() => { + read.mockClear() + installHost.mockClear() + clearStructuredHostStub() + }) + + // A new chat's picker reads before its create lands; on a host with no saved chats nothing else + // has built the host yet, and a refusal here left the picker on the client's built-in list. + it('builds the host for a structured chat and answers from its catalog', async () => { + const reply = await call( + 'agentSession.modelCatalog', + { agent: 'codex', sessionId: SESSION }, + STRUCTURED_CLIENT, + { ensureStructuredAgentSessionHost: installHost } + ) + expect(installHost).toHaveBeenCalledTimes(1) + expect(reply).toMatchObject({ ok: true, result: { origin: 'probe' } }) + expect(read).toHaveBeenCalledWith({ agent: 'codex', sessionId: SESSION }) + }) + + // Terminal-backed chat reads with no session: a host that runs no structured chat keeps its + // journal closed, and the read falls back to the CLI listing. + it('does not build the host for a read that names no session', async () => { + const reply = await call('agentSession.modelCatalog', { agent: 'codex' }, STRUCTURED_CLIENT, { + ensureStructuredAgentSessionHost: installHost + }) + expect(installHost).not.toHaveBeenCalled() + expect(reply).toMatchObject({ ok: false }) + expect(read).not.toHaveBeenCalled() + }) + + it('does not build the host for a client that cannot read structured sessions', async () => { + const reply = await call( + 'agentSession.modelCatalog', + { agent: 'codex', sessionId: SESSION }, + { clientKind: 'runtime', clientCapabilities: [] }, + { ensureStructuredAgentSessionHost: installHost } + ) + expect(installHost).not.toHaveBeenCalled() + expect(reply).toMatchObject({ ok: false }) + }) }) 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 19e93689577..3cd86c876ad 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 @@ -11,7 +11,7 @@ import { defineMethod } from '../core' import { requireInstalledStructuredHost, - requireStructuredHost as requireHost + requireStructuredHost } from './structured-agent-session-gate' import { ModelCatalogParams, OptionsParams } from './structured-agent-session-schemas' @@ -25,8 +25,14 @@ export const STRUCTURED_AGENT_SESSION_OPTIONS_READ_METHODS = [ defineMethod({ name: 'agentSession.modelCatalog', params: ModelCatalogParams, + // A structured chat's read names its session and builds the host, since it may come first; + // terminal-backed chat's session-less read must not open the journal where none runs. handler: async ({ worktree, ...params }, ctx) => { - const catalog = requireHost(ctx).deps.modelCatalog + const host = + params.sessionId === undefined + ? requireStructuredHost(ctx) + : await requireInstalledStructuredHost(ctx) + const catalog = host.deps.modelCatalog if (!catalog) { return { origin: 'unknown' as const } } diff --git a/src/renderer/src/components/native-chat/NativeChatSessionOptionPickers.menu-request.test.tsx b/src/renderer/src/components/native-chat/NativeChatSessionOptionPickers.menu-request.test.tsx new file mode 100644 index 00000000000..90abdc3ef25 --- /dev/null +++ b/src/renderer/src/components/native-chat/NativeChatSessionOptionPickers.menu-request.test.tsx @@ -0,0 +1,109 @@ +// @vitest-environment happy-dom + +// Against the real menu: a `/model` request opens the menu once, and a period while the host still +// lists models neither opens it later nor reopens one the user closed. + +import { act, cleanup, fireEvent, render, screen } from '@testing-library/react' +import { afterEach, describe, expect, it, vi } from 'vitest' +import type { + SessionOptionDescriptor, + SessionOptionsSurface +} from '../../../../shared/native-chat-session-options' + +vi.mock('sonner', () => ({ toast: { error: vi.fn() } })) + +vi.mock('@/i18n/i18n', () => ({ + translate: (_key: string, fallback: string, values?: Record) => + values + ? Object.entries(values).reduce( + (text, [name, value]) => text.replaceAll(`{{${name}}}`, String(value)), + fallback + ) + : fallback +})) + +import { TooltipProvider } from '@/components/ui/tooltip' +import { NativeChatSessionOptionPickers } from './NativeChatSessionOptionPickers' +import type { NativeChatOptionPickerRequest } from './native-chat-composer-types' + +function model(pending: boolean): SessionOptionDescriptor { + return { + id: 'model', + label: 'Model', + category: 'model', + kind: { + type: 'select', + currentValue: 'opus', + choices: [ + { value: 'opus', label: 'Opus 4.8' }, + { value: 'sonnet', label: 'Sonnet 5' } + ] + }, + valueSource: 'applied', + transport: 'agent-session', + settable: !pending, + ...(pending ? { choicesPending: true as const } : {}) + } +} + +const surface: SessionOptionsSurface = { + getSnapshot: () => [], + setOption: vi.fn(async () => ({ snapshot: [] })), + invokeAction: vi.fn(async () => ({ snapshot: [] })), + subscribe: () => () => {} +} + +function view(pending: boolean, request: NativeChatOptionPickerRequest | null): React.JSX.Element { + return ( + +