From fe52250d78dc644252360708a28530a67b1a49a5 Mon Sep 17 00:00:00 2001 From: indifferentketchup Date: Fri, 29 May 2026 12:37:01 +0000 Subject: [PATCH] coder(providers): fix empty picker (loading-state) + config model overrides + current Claude models MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix: getProviderSnapshot returned synchronous installed:false 'loading' entries on a cache miss (v2.5.5/Phase 2), which AgentComposerBar filters out — with the Phase 5 client poll not yet built, a single fetch stranded on 'loading' and the picker showed no providers. It now awaits the build and returns terminal entries; the sync loading-return is deferred until Phase 5. Builds stay fast via the tier-2 cold-probe skip. Feature: wire the v2.3 config schema's models/additionalModels — buildResolvedRegistry carries them onto ResolvedProviderDef (models replace, additionalModels merge) and provider-snapshot applies them to every ready model list, so /data/coder-providers.json can edit any provider's models with no code change. Claude staticModels bumped from the stale 2-entry list to opus/sonnet/haiku latest-aliases + pinned claude-opus-4-8 / claude-sonnet-4-6 / claude-haiku-4-5-20251001 (passed verbatim to claude --model). +2 tests (109 total). Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 4 ++ .../provider-config-registry.test.ts | 16 ++++++ .../__tests__/provider-snapshot.test.ts | 31 +++++++++++ .../src/services/provider-config-registry.ts | 8 +++ apps/coder/src/services/provider-registry.ts | 13 ++++- apps/coder/src/services/provider-snapshot.ts | 52 ++++++++----------- 6 files changed, 92 insertions(+), 32 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 906966c..ff36f33 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ All notable changes per release tag. Most recent on top, ordered by tag creation date (which matches the git history). Tag names follow `vMAJOR.MINOR.PATCH-slug` — the slug describes what shipped, so the tag name alone is enough to recall the batch. +## v2.5.7-claude-models-and-picker-fix — 2026-05-29 + +Two provider-layer changes. **(1) Fix the empty provider picker** — a regression from `v2.5.5` (Phase 2): on a cache miss `getProviderSnapshot` returned synchronous `installed:false` `loading` entries, which `AgentComposerBar` filters out (`e.installed && e.status !== 'error'`); with the client-side poll deferred to Phase 5, a single fetch landed on `loading` forever and no providers appeared. `getProviderSnapshot` now awaits the build and returns terminal entries (the sync `loading` return is deferred until Phase 5 ships the poll); builds stay fast via the tier-2 cold-probe skip. **(2) Claude models** — the list was a hardcoded 2-entry static list (Opus 4 / Sonnet 4, May 2025), and the v2.3 config schema's `models`/`additionalModels` were parsed but never wired. `buildResolvedRegistry` now carries config `models` (replace) + `additionalModels` (merge) onto `ResolvedProviderDef`, and `provider-snapshot` applies them to every ready model list — so `/data/coder-providers.json` can add or replace any provider's models with no code change. Claude `staticModels` bumped to `opus`/`sonnet`/`haiku` latest-aliases plus pinned `claude-opus-4-8` / `claude-sonnet-4-6` / `claude-haiku-4-5-20251001` (passed verbatim to `claude --model`; the CLI accepts both aliases and pinned full names). +2 unit tests (109 total). Builds on `v2.5.6-provider-lifecycle-phase3`. + ## v2.5.6-provider-lifecycle-phase3 — 2026-05-29 Phase 3 of the v2.3 provider-lifecycle batch (`openspec/changes/v2-3-provider-lifecycle/design.md` §5): generic ACP dispatch. `acp-spawn.ts` gains `resolveLaunchSpec(resolved, installPath)` — it consults the resolved registry's `launchCommand` (a config override or a custom-ACP entry's command) first, falling back to the kept `resolveAcpSpawnArgs` switch for built-ins. `acp-dispatch.ts` now spawns `spec.binary`/`spec.args` with `env: { ...process.env, ...spec.env }` instead of the hardcoded per-name argv, and `dispatcher.ts` loads the resolved def by `task.agent` and passes it through. This lets config-defined custom ACP providers dispatch with no new switch case. Built-in dispatch (claude/opencode/goose/qwen) is **byte-identical** to pre-v2.3 — proven by a regression test asserting opencode→`['acp']`, goose→`['acp']`, qwen→`['--acp']`, binary=`installPath ?? id`, and empty config env → plain `process.env`. One deliberate deviation from the spec's literal `!installPath → null`: the `installPath ?? id` fallback is preserved so a missing install path still spawns the bare agent name as before. `setSessionMode`/permission/streaming and the dispatcher poll/NOTIFY/running-guard are untouched. 7 new `acp-spawn.test.ts` cases. No routes/UI (Phase 4+). Builds on `v2.5.5-provider-lifecycle-phase2`. diff --git a/apps/coder/src/services/__tests__/provider-config-registry.test.ts b/apps/coder/src/services/__tests__/provider-config-registry.test.ts index 3b5f761..9375fc9 100644 --- a/apps/coder/src/services/__tests__/provider-config-registry.test.ts +++ b/apps/coder/src/services/__tests__/provider-config-registry.test.ts @@ -61,6 +61,22 @@ describe('buildResolvedRegistry', () => { warn.mockRestore(); }); + it('carries config models + additionalModels onto built-in and custom defs', () => { + const reg = buildResolvedRegistry(PROVIDERS, { + providers: { + claude: { models: [{ id: 'claude-opus-4-8', label: 'Opus 4.8' }] }, + 'amp-acp': { + extends: 'acp', + label: 'Amp', + command: ['amp-acp'], + additionalModels: [{ id: 'amp-1', label: 'Amp 1' }], + }, + }, + }); + expect(reg.get('claude')!.configModels).toEqual([{ id: 'claude-opus-4-8', label: 'Opus 4.8' }]); + expect(reg.get('amp-acp')!.configAdditionalModels).toEqual([{ id: 'amp-1', label: 'Amp 1' }]); + }); + it('REGRESSION: empty config returns exactly the built-ins, all enabled', () => { const reg = buildResolvedRegistry(PROVIDERS, { providers: {} }); expect(reg.size).toBe(PROVIDERS.length); diff --git a/apps/coder/src/services/__tests__/provider-snapshot.test.ts b/apps/coder/src/services/__tests__/provider-snapshot.test.ts index 940167a..d6e452a 100644 --- a/apps/coder/src/services/__tests__/provider-snapshot.test.ts +++ b/apps/coder/src/services/__tests__/provider-snapshot.test.ts @@ -293,6 +293,37 @@ describe('getProviderSnapshot', () => { expect(boocode?.installed).toBe(true); }); + it('config models REPLACE the claude static list; additionalModels merge (+ thinking)', async () => { + loadConfigFixture({ + claude: { + models: [{ id: 'claude-opus-4-8', label: 'Opus 4.8' }], + additionalModels: [{ id: 'sonnet', label: 'Sonnet (latest)' }], + }, + }); + + const sql = mockSql([ + { + name: 'claude', + install_path: '/usr/bin/claude', + supports_acp: false, + models: [{ id: 'old-static', label: 'Old' }], + label: 'Claude Code', + transport: 'pty', + last_probed_at: new Date().toISOString(), + }, + ]); + + const entries = await getProviderSnapshot(sql, config, '/tmp/project', true); + const claude = entries.find((e) => e.name === 'claude'); + const ids = claude!.models.map((m) => m.id); + + expect(ids).toContain('claude-opus-4-8'); // config models replaced the DB/static list + expect(ids).toContain('sonnet'); // additionalModels merged on top + expect(ids).not.toContain('old-static'); // replaced, not appended + // thinking options still attach to the config-provided models + expect(claude!.models.find((m) => m.id === 'claude-opus-4-8')?.thinkingOptions?.length).toBeGreaterThan(0); + }); + it('2.7 warm cache: a second snapshot within the warm window spawns ZERO probes', async () => { loadConfigFixture({}); mockProbe.mockResolvedValue({ diff --git a/apps/coder/src/services/provider-config-registry.ts b/apps/coder/src/services/provider-config-registry.ts index e8e3b52..3fbb3e2 100644 --- a/apps/coder/src/services/provider-config-registry.ts +++ b/apps/coder/src/services/provider-config-registry.ts @@ -22,6 +22,10 @@ export interface ResolvedProviderDef extends ProviderDef { env: Record | undefined; configLabel?: string; configDescription?: string; + /** Config `models` — REPLACES the discovered/static model list when present. */ + configModels?: Array<{ id: string; label: string }>; + /** Config `additionalModels` — MERGED on top of the resolved model list. */ + configAdditionalModels?: Array<{ id: string; label: string }>; } /** @@ -61,6 +65,8 @@ export function buildResolvedRegistry( env: ov?.env, configLabel: ov?.label, configDescription: ov?.description, + configModels: ov?.models, + configAdditionalModels: ov?.additionalModels, }); } @@ -87,6 +93,8 @@ export function buildResolvedRegistry( env: ov.env, configLabel: ov.label, configDescription: ov.description, + configModels: ov.models, + configAdditionalModels: ov.additionalModels, }); } diff --git a/apps/coder/src/services/provider-registry.ts b/apps/coder/src/services/provider-registry.ts index 0e33f83..262f7ce 100644 --- a/apps/coder/src/services/provider-registry.ts +++ b/apps/coder/src/services/provider-registry.ts @@ -41,9 +41,18 @@ export const PROVIDERS: ProviderDef[] = [ label: 'Claude Code', transport: 'pty', modelSource: 'static', + // Passed verbatim to `claude --model ` (PTY dispatch). The CLI accepts a + // latest-alias ('opus'/'sonnet'/'haiku') or a pinned full name + // ('claude-opus-4-8'). Aliases never go stale; pinned IDs let you select an + // exact version. Extend/replace per-install via data/coder-providers.json + // (models / additionalModels) without a code change. staticModels: [ - { id: 'claude-opus-4-20250514', label: 'Opus 4' }, - { id: 'claude-sonnet-4-20250514', label: 'Sonnet 4' }, + { id: 'opus', label: 'Opus (latest)' }, + { id: 'claude-opus-4-8', label: 'Opus 4.8' }, + { id: 'sonnet', label: 'Sonnet (latest)' }, + { id: 'claude-sonnet-4-6', label: 'Sonnet 4.6' }, + { id: 'haiku', label: 'Haiku (latest)' }, + { id: 'claude-haiku-4-5-20251001', label: 'Haiku 4.5' }, ], }, { diff --git a/apps/coder/src/services/provider-snapshot.ts b/apps/coder/src/services/provider-snapshot.ts index 9a9af61..53b52fb 100644 --- a/apps/coder/src/services/provider-snapshot.ts +++ b/apps/coder/src/services/provider-snapshot.ts @@ -85,6 +85,16 @@ async function buildProviderEntry( const label = agentRow?.label ?? resolved.configLabel ?? resolved.label; const descr = resolved.configDescription ? { description: resolved.configDescription } : {}; + // v2.3: config `models` REPLACES the discovered/static list; `additionalModels` + // MERGES on top. Applied to every ready/installed model list below. + const withConfigModels = (m: ProviderModel[]): ProviderModel[] => { + let out = resolved.configModels && resolved.configModels.length > 0 ? resolved.configModels : m; + if (resolved.configAdditionalModels && resolved.configAdditionalModels.length > 0) { + out = mergeModels(out, resolved.configAdditionalModels); + } + return out; + }; + // ACP built-ins fall back to PTY transport when the installed binary lacks ACP. let transport = resolved.transport; if (agentRow && resolved.transport === 'acp' && !agentRow.supports_acp) { @@ -104,7 +114,7 @@ async function buildProviderEntry( if (isNative) { return { name, label: resolved.label, transport, status: 'ready', - enabled: true, installed: true, models: llamaModels, modes: [], + enabled: true, installed: true, models: withConfigModels(llamaModels), modes: [], defaultModeId: null, commands: manifestCommands, }; } @@ -137,7 +147,7 @@ async function buildProviderEntry( if (name === 'claude') { return { name, label, transport, status: 'ready', enabled: true, installed: true, - models: attachClaudeThinking(models), modes: fallbackModes, defaultModeId, + models: attachClaudeThinking(withConfigModels(models)), modes: fallbackModes, defaultModeId, commands: manifestCommands, }; } @@ -165,7 +175,7 @@ async function buildProviderEntry( } return { name, label, transport, status: 'ready', enabled: true, installed: true, - models: skipModels, modes: fallbackModes, defaultModeId, commands: manifestCommands, + models: withConfigModels(skipModels), modes: fallbackModes, defaultModeId, commands: manifestCommands, }; } @@ -188,7 +198,7 @@ async function buildProviderEntry( name, label, transport, status: probe.ok ? 'ready' : 'error', enabled: true, installed: true, - models: probeModels, + models: withConfigModels(probeModels), modes: probe.modes.length > 0 ? probe.modes : fallbackModes, defaultModeId: probe.defaultModeId ?? defaultModeId, commands: mergeCommands(manifestCommands, probe.commands), @@ -203,27 +213,10 @@ async function buildProviderEntry( } return { name, label, transport, status: 'ready', enabled: true, installed: true, - models, modes: fallbackModes, defaultModeId, commands: manifestCommands, + models: withConfigModels(models), modes: fallbackModes, defaultModeId, commands: manifestCommands, }; } -/** Synchronous placeholder entries for a cache-miss while the build runs (§4.4). */ -function loadingEntries(): ProviderSnapshotEntry[] { - return [...getResolvedRegistry().values()].map((r) => ({ - name: r.id, - label: r.configLabel ?? r.label, - ...(r.configDescription ? { description: r.configDescription } : {}), - transport: r.transport, - status: r.enabled ? ('loading' as const) : ('unavailable' as const), - enabled: r.enabled, - installed: false, - models: [], - modes: getManifestModes(r.id), - defaultModeId: getManifestDefaultModeId(r.id), - commands: getManifestCommands(r.id), - })); -} - const snapshotCache = new Map(); const snapshotInflight = new Map>(); const CACHE_TTL_MS = 5 * 60_000; @@ -269,14 +262,13 @@ export async function getProviderSnapshot( }); snapshotInflight.set(cacheKey, promise); - // force → await the full build (cold probes included). Non-force cache miss → - // return loading entries synchronously; the build settles in the background - // and the next call returns it via cache / inflight (§4.4; client polls). - if (force) return promise; - promise.catch(() => { - /* settled errors surface on the next call that awaits inflight/rebuilds */ - }); - return loadingEntries(); + // Await the build (force or cache-miss) and return terminal entries. The sync + // `loading` return (design §4.4) is DEFERRED until Phase 5 ships the client + // poll that resolves it: without that poll, a single fetch lands on + // installed:false `loading` entries, which AgentComposerBar filters out + // (`e.installed && ...`) → empty picker. Builds stay fast via the tier-2 skip + // once available_agents.models is warm. + return promise; } export function clearProviderSnapshotCache(): void {