feat(providers,schema,engine,workbench): retire the default model — user-selected model sets from provider model lists - #426
feat(providers,schema,engine,workbench): retire the default model — user-selected model sets from provider model lists#426PeronGH wants to merge 9 commits into
Conversation
Each endpoint service names the URL that lists the ids it serves, spelled out rather than derived from a variant's baseUrl and protocol: DeepSeek's `/anthropic` variant would derive `/anthropic/v1/models` and Vercel's bare-origin one a root `/models`, and neither route exists. Both Cloudflare entries serve no list at all, so they stay absent and those accounts remain freeform-only.
…el source An account now carries the models the user selected (`Account.models`) instead of one free-text default, and the pick itself lives per agent as `ProviderConfig.model`. Nothing falls back to the agent's own choice any more, so a bound agent with no pick refuses to start rather than running on a model the user never chose; an agent with no account bound keeps resolving its own. `config.probe-models` now names a service and lets the daemon resolve the list URL from the catalog, so a saved account is probed by id and its stored secret never travels back out to the client. Both wire versions move: removing `Account.model`, renaming `defaultModel`, and dropping the `null` tier from `StartOptions.model` are breaking. `loadConfig` carries both old fields over on read, since zod would otherwise strip them and silently lose every existing user's configured model. The model inputs are gone from the account forms; the multi-select that replaces them lands with the picker work.
…known provider A picked model id comes from the service's own model list and carries no provider, while opencode routes only by `providerID/modelID`. `resolveModelRef` qualifies it with `config.knownProvider`; with neither half available the ref is still refused, since a stored unroutable id would report a successful switch while every prompt silently omitted the field. Both reflection paths compare resolved refs and emit the id the user picked rather than opencode's prefixed readback, so the client's selected set still matches what the session reports.
… list Every account form now carries a model set instead of a free-text default: fetch the ids the service serves, tick the ones to keep, and add any missing id by hand. Endpoints that serve no list — the Cloudflare gateways, custom accounts — are freeform only, which is the same control with the fetch button absent. Sources differ by account and are injected rather than read in the form, since the forms are presentation and only the settings page sits inside the data-plane provider tree: a catalog service is probed with the unsaved secret, a saved account by id, and a subscription reads codex's start catalog or, for claude-code, the curated table it has no enumeration API to replace. A picked id the list stops returning is kept and stays ticked. It is either a hand-typed entry or one the vendor retired, and dropping it would change the account's model set behind the user's back on the next fetch.
… to send without one The composer and the new-session surface now read the set picked on the agent's bound account, which outranks both the adapter-advertised catalog and the curated table: a claude-code account pointing at DeepSeek stops offering Anthropic ids it cannot reach. Present-but-empty and absent mean different things in that set, and the send gate turns on the difference. An account bound with nothing picked blocks sending, matching the daemon's own refusal instead of discovering it a round trip later. An agent with no account bound is absent, still resolves its own model, and is not blocked. AGENT_DEFAULT_MODELS is gone: guessing a provider's model is exactly what the picked set replaces, and an unresolved model now blocks the send rather than silently starting on a vendor default. Rebinding an agent drops a pick the new account does not list, since keeping it would run the next session on a model that account never offered.
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
The reshaped config.probe-models frame lets a caller pair any saved account's secret with any service's model-list URL. A crafted frame sends a stored Anthropic key to openrouter.ai. Details inline on request-handler.ts.
The direction here is right, and the schema work is careful — ProviderConfig.model's "Not a fallback default: unset means no session can start" is exactly the doc comment that makes the new contract legible, and AGENT_DEFAULT_MODELS is removed cleanly (I grepped: zero stragglers). The MIN_COMPATIBLE_WIRE_VERSION 68→74 bump is the correct call for a field rename plus a removed null tier, and the PR body already says partial upgrades are off the table.
Two anchored defects below, plus one scope question that has no line to point at.
The orphan-model drop exists in only one place, and it's client-side
withBinding (packages/client/workbench/src/settings/providers/view.ts) now takes accounts and drops providers[kind].model when the newly bound account doesn't offer it. Good — but that's the rebind path only.
Nothing re-runs it when the account's models set is edited. handleUpdate in providers-settings.tsx calls saveAccounts and never touches providers, so a user who unchecks the very model an agent is configured to run on leaves providers[kind].model pointing at an id the account no longer offers. On the engine side, applyProviderDefaults (packages/host/engine/src/agent/provider-config.ts:137) does next.model = config.model verbatim — the account's models set is used only to build the credential/endpoint bundle, never to validate the model string. The new accountBound && model === undefined guard in start-options-resolver.ts doesn't fire either, because the model is present, just wrong.
Net effect: the user gets a provider-level failure mid-start ("model X is not available…", or a 400 from the vendor) instead of the clean, local "no model selected" refusal this PR is built to produce. Since the engine is where the invariant is now enforced, that's probably where membership belongs too — the client-side drop is a nicety, not the guarantee. Worth deciding deliberately rather than leaving the two halves out of step.
Smaller notes
packages/host/agent-adapter/src/__tests__/pi-model.test.ts:177still assertsstart({ model: null })preserves "an explicit null model reset for the Pi provider default". That tier is no longer expressible on the wire now thatAgentStartInput.modeldropped.nullable(), so the test is pinning adapter behavior no client can reach. Not wrong, just worth knowing it's decorative now.- The six vendor model-list URLs in
catalog.tsare hardcoded and I could not verify them from here (no network). Worth one manual pass, particularlydeepseek'shttps://api.deepseek.com/modelsand the?limit=1000on anthropic-api.
Claude Opus ｜ 𝕏
| private probeSecret( | ||
| credential: Extract<AgentRequest, { kind: 'config.probe-models' }>['credential'], | ||
| ): AccountSecret { | ||
| if (credential.type === 'inline') return credential.secret; | ||
| const account = this.providers | ||
| .getAccounts() | ||
| .find((candidate) => candidate.id === credential.accountId); | ||
| if (!account) throw new Error('Account not found'); | ||
| if (account.credential.type === 'oauth') { | ||
| throw new Error('A subscription login holds no secret to read the model list with'); | ||
| } | ||
| return account.credential; | ||
| } |
There was a problem hiding this comment.
The destination URL and the credential come from two independent client-controlled fields, and nothing checks they agree.
At line 154 the URL is derived purely from payload.service:
const source = modelListSource(payload.service);
...
const models = await this.probeModels(source, this.probeSecret(payload.credential));and probeSecret below resolves the secret purely from credential.accountId. So a config.probe-models frame carrying service: 'openrouter' plus credential: {type:'account', accountId: '<the-anthropic-account>'} makes the daemon send the stored Anthropic key to https://openrouter.ai/api/v1/models.
That directly contradicts this method's own docstring. "A saved account is named by id rather than shipping its secret back out to the client" keeps the secret away from the client, but the caller still gets to choose which third party receives it — which is the part that actually matters. The blast radius is bounded (six catalog URLs, not arbitrary-URL SSRF, and the client is loopback-local), but "only a local process can exfiltrate your API keys to a competitor" is still a weaker guarantee than the one written here.
The information needed for the check already exists: Account.service (packages/foundation/schema/src/model/account.ts:58). Thread the service id in and compare. Note the undefined case — custom and pre-catalog accounts have no service, and those should be refused rather than allowed through, since modelListSource only ever resolves a catalog id anyway.
| private probeSecret( | |
| credential: Extract<AgentRequest, { kind: 'config.probe-models' }>['credential'], | |
| ): AccountSecret { | |
| if (credential.type === 'inline') return credential.secret; | |
| const account = this.providers | |
| .getAccounts() | |
| .find((candidate) => candidate.id === credential.accountId); | |
| if (!account) throw new Error('Account not found'); | |
| if (account.credential.type === 'oauth') { | |
| throw new Error('A subscription login holds no secret to read the model list with'); | |
| } | |
| return account.credential; | |
| } | |
| /** The secret to probe with. A saved account is named by id rather than shipping its secret back | |
| * out to the client and in again; an oauth login holds none, so it cannot be probed. The account | |
| * must belong to the service being probed, or the caller could aim one vendor's key at another. */ | |
| private probeSecret( | |
| service: string, | |
| credential: Extract<AgentRequest, { kind: 'config.probe-models' }>['credential'], | |
| ): AccountSecret { | |
| if (credential.type === 'inline') return credential.secret; | |
| const account = this.providers | |
| .getAccounts() | |
| .find((candidate) => candidate.id === credential.accountId); | |
| if (!account) throw new Error('Account not found'); | |
| if (account.service !== service) { | |
| throw new Error('Account does not belong to the service being probed'); | |
| } | |
| if (account.credential.type === 'oauth') { | |
| throw new Error('A subscription login holds no secret to read the model list with'); | |
| } | |
| return account.credential; | |
| } |
| // The catalog default is what the agent's own config would start on, so it yields to anything the | ||
| // user expressed through LinkCode. Nothing guesses past it: an unresolved model blocks the send | ||
| // rather than starting a session on a model nobody chose. | ||
| const displayedModel = | ||
| selectedModel ?? | ||
| (defaultModels === null | ||
| ? null | ||
| : (defaultModels?.[provider] ?? | ||
| catalog?.defaultModel ?? | ||
| AGENT_DEFAULT_MODELS[provider] ?? | ||
| null)); | ||
| (defaultModels === null ? null : (defaultModels?.[provider] ?? catalog?.defaultModel ?? null)); |
There was a problem hiding this comment.
The new comment says "an unresolved model blocks the send rather than starting a session on a model nobody chose" — but the catalog?.defaultModel arm on line 195 means it doesn't.
catalog.defaultModel is still populated by the adapters (native/claude-code.ts:623, native/pi/adapter.ts:223, native/codex/adapter.ts:418-426). Take an agent with an account bound and a non-empty picked set, where the user has never chosen a model — so selectedModels[provider] is undefined, preferredModels?.[provider] is absent, and configuredDefaultModels has no entry because providers[kind].model is unset. Then:
selectedModelisnull, so the??on line 194 does not short-circuit;displayedModelresolves tocatalog.defaultModel, a non-null string;boundSetis defined, sosendBlocked = (boundSet !== undefined && displayedModel === null)isfalse;resolveModel(pickable, displayedModel)finds nothing, so the composer displays a model the bound account may not even offer;- submit (line 240) evaluates
localModel === null ? undefined : (selectedModel ?? undefined)→undefined; - the daemon rejects with
No model selected for <kind>.
That's the exact round trip this PR set out to eliminate, and it shows up in the default state for any freshly bound account. Note the fix must keep the defaultModels?.[provider] arm — a configured model legitimately submits as undefined and gets refilled by applyProviderDefaults — and drop only the catalog fallback:
| // The catalog default is what the agent's own config would start on, so it yields to anything the | |
| // user expressed through LinkCode. Nothing guesses past it: an unresolved model blocks the send | |
| // rather than starting a session on a model nobody chose. | |
| const displayedModel = | |
| selectedModel ?? | |
| (defaultModels === null | |
| ? null | |
| : (defaultModels?.[provider] ?? | |
| catalog?.defaultModel ?? | |
| AGENT_DEFAULT_MODELS[provider] ?? | |
| null)); | |
| (defaultModels === null ? null : (defaultModels?.[provider] ?? catalog?.defaultModel ?? null)); | |
| // The catalog default is what the agent's own config would start on, so it yields to anything the | |
| // user expressed through LinkCode. Nothing guesses past it: an unresolved model blocks the send | |
| // rather than starting a session on a model nobody chose. A bound account narrows that further — | |
| // its picked set is the only source, so the adapter's own default is not a candidate at all. | |
| const displayedModel = | |
| selectedModel ?? | |
| (defaultModels === null | |
| ? null | |
| : (defaultModels?.[provider] ?? | |
| (accountModels?.[provider] === undefined ? catalog?.defaultModel : undefined) ?? | |
| null)); |
A live session's account is fixed at spawn — credentials and base URL are injected once — so the client needs to know it to scope that session's model menu. Nothing recorded it: the resolved account existed only inside `applyProviderDefaults`. `accountConfigBundle` now echoes `accountId` into the resolved config, which `resolveAccount` already reads on the way in, so the same key serves both directions and a client can pin a session to one account. Each run lifts just that id into `SessionRun`, and `SessionInfo` reports the latest run's, mirroring how `historyId` already works — a rebind between runs is legitimate, so only the newest describes what a session is actually talking to. Only the id is persisted; the rest of `config` carries secrets.
The new-session menu now spans every account `resolveBinding` accepts for the agent, grouped per account, so choosing a model also chooses which account serves it. One agent reaches several providers without a trip through Settings — previously the menu showed only the single bound account's set, which made it offer less than the curated table it replaced. A live session's menu stays scoped to its own account: credentials and base URL are injected at spawn, so offering another account's models would advertise a switch the adapter cannot make. Model identity becomes (account, model). Two accounts legitimately serve the same id — a direct DeepSeek key and an OpenRouter one both list `deepseek-v4-pro` — and the menu previously used the bare id as both its React key and its radio value, which would collapse the two into one unselectable row. `modelChoiceKey` keys them apart, the pick hands back the whole entry rather than a string to re-parse, and `resolveModel` takes an account tiebreak so the trigger label names the right one. The chosen account rides `config.accountId`, which `resolveAccount` already honours ahead of the bound one.
The model an agent runs on was remembered twice: `providers[kind].model` on the daemon and `modelsByProvider` in renderer localStorage. Two owners meant Settings and the composer could disagree, and a scheduled or script session ignored whatever the composer last used. The accepted pick now writes daemon config, carrying the account it came from — choosing a model is also choosing who serves it, so leaving the old binding would run the next session on an account that never listed that model. The client copy is gone and the persisted store moves to v6 so a stale blob cannot resurrect a memory with no owner. Written once a selection is known to have been accepted rather than on the menu click, keeping the existing confirm-then-remember discipline: an abandoned draft never rewrites config, and a provider that rejects a model leaves the previous one standing. The pick still takes effect on the session immediately — it rides the start options either way. Nothing re-sends a configured model at session start now; the daemon resolves it, so the client specifying it again could only let the two disagree.
|
Three commits pushed that close a gap the first five left open. The Provider → Harness rename follows separately, so the textual sweep does not hide logic. What was missingThe original five made the picker read the one account bound to an agent, which narrowed it — claude-code offered whatever was ticked on one account instead of the whole curated table it replaced.
How it works nowThe new-session menu aggregates every account A live session's menu stays scoped to its own account. Credentials and base URL are injected at spawn, so offering another account's models would advertise a switch the adapter cannot make — opencode rejects cross-provider outright, and claude-code would send the new id to the old endpoint. That required the session to start recording its account, which nothing did. The accepted pick writes Three findings worth attention in reviewThe menu-keying bug was real, not hypothetical. The send gate from Configured models are no longer re-sent at session start. Three tests asserted the remembered pick travels in the submission; that only held while the memory was client-side and invisible to the daemon. The daemon now resolves Deliberate deviationThe pick takes effect on the session immediately, but the persisted default is written only once the selection is confirmed — keeping the existing Checks
Still not driven in the real app, which remains the gap for this PR overall. The manual pass now needs two accounts bindable to one agent: confirm the new-session menu groups both, that picking a DeepSeek model starts a session actually running on DeepSeek, that Settings shows the rebind after the first turn, that the live thread's menu shows only that account's models, and that two accounts sharing a model id give two separately selectable entries. |
There was a problem hiding this comment.
Caution
The new commits widen the send-gate from bound to bindable. One unrelated API-key account in the pool is now enough to replace opencode's own model menu with that account's models and block the composer — while the daemon, which gates on activeAccountId, would have started the session fine. Details inline on default-models.ts.
The account-pinned model pick is a good design, and the pieces that make it work are careful: modelChoiceKey identifies an entry by (accountId, id) rather than id alone, resolveModel's account scoping falls back instead of returning nothing, and handleModelChange writing model and account together is the right shape. Account.models' doc comment states the new contract plainly.
Three new defects below, all in the two newest commits. Independent of these, both threads from the previous review are still open at this HEAD — probeSecret still takes no service argument, and the catalog?.defaultModel arm in displayedModel is unchanged. Not re-raised here.
Two comments in this PR contradict each other, and the tests encode the wrong one
start-options-resolver.ts:52-54, on the daemon's new refusal:
With an account bound, its selected set is the only model source and nothing falls back to the agent's own choice. Unbound agents keep running on whatever they resolve themselves.
accountModelOptions' doc, on the client's:
[]says "bindable, nothing picked yet" and blocks sends the way the daemon does.
Those describe different gates. The daemon keys on providers[kind].activeAccountId !== undefined; the client keys on whether any account could theoretically back the agent — which for opencode and pi is nearly every API-key account in the pool (resolve.ts:149-151 returns native for every protocol, and resolve.ts:66-69 keeps a service-less bare key bindable everywhere). opencode and pi are exactly the agents the PR body says must keep running on their own CLI login.
The new tests preserve the conflation rather than catching it. new-session-surface.test.tsx:898 is titled "refuses to send when an account is bound but no model is picked" and comments "Bound with an empty set: the daemon would refuse this start, so the composer does too" — but the prop it passes, accountModels={{ 'claude-code': [] }}, is what accountModelOptions produces for a bindable account, per default-models.test.ts:104-111 ("keeps a bindable-but-unpicked one empty"). The test passes while asserting a parity the code does not have, so the gap is invisible from the suite.
Whichever way you settle it, the two gates should read the same field — otherwise the composer and the daemon will keep disagreeing about which agents are startable.
Claude Opus ｜ 𝕏
| export function accountModelOptions( | ||
| accounts: Accounts | undefined, | ||
| ): Partial<Record<AgentKind, ModelOption[]>> { | ||
| const options: Partial<Record<AgentKind, ModelOption[]>> = {}; | ||
| for (const kind of AgentKindSchema.options) { | ||
| const bindable = (accounts ?? []).filter( | ||
| (account) => resolveBinding(account, kind).tier !== 'unavailable', | ||
| ); | ||
| if (bindable.length === 0) continue; | ||
| options[kind] = bindable.flatMap((account) => modelOptionsOf(account)); | ||
| } |
There was a problem hiding this comment.
bindable filters on resolveBinding(...).tier !== 'unavailable' — could this account back the agent, not is it bound. For opencode and pi, bind() returns native for every protocol (resolve.ts:149-151), and an account with no catalog service stays bindable everywhere (resolve.ts:66-69). So one API-key account added for codex makes options.opencode present for a user whose opencode authenticates through its own opencode auth login.
Presence is load-bearing twice over in new-session-surface.tsx:
pickable = bindableSet ?? dynamicModels ?? AGENT_MODEL_OPTIONS[provider](line 206) — a present entry discards opencode's own live catalog, so the menu shows an unrelated vendor's models.sendBlocked = bindableSet !== undefined && displayedModel === null(line 383).
opencode is the one adapter that deliberately publishes no defaultModel (native/opencode/adapter.ts:751), and providers.opencode.model is unset because the account was never bound — so displayedModel is null and the send is blocked. The user's only way forward is to pick a model belonging to a vendor account they never meant to use for opencode, which then pins accountId to it and rebinds the agent.
It gets worse when that account has no picked models. models is optional and modelOptionsOf does account.models ?? [], so options[kind] is [] — present, hence blocking, but with nothing in the menu. default-models.test.ts:104-111 pins exactly this ("keeps a bindable-but-unpicked one empty"). The agent is then unstartable from the composer and no interaction there can recover it.
Meanwhile start-options-resolver.ts:52-54 says the opposite for the same configuration: "Unbound agents keep running on whatever they resolve themselves." It gates on activeAccountId, so it would have started this session.
The PR's intent — one agent drawing models from several accounts — argues against narrowing this function to the bound account, since the wider menu is the feature. The narrower fix is downstream: gate the send on the agent actually being bound (the field the daemon uses), and have pickable union the account sets with the adapter catalog instead of replacing it, so an unbound agent keeps its own models on offer.
| if (newlyConfirmed.model === undefined && newlyConfirmed.effort === undefined) return; | ||
| rememberSelection(submission.kind, newlyConfirmed); | ||
| if (newlyConfirmed.effort !== undefined) { | ||
| rememberSelection(submission.kind, { effort: newlyConfirmed.effort }); | ||
| } | ||
| // The model lands in daemon config rather than a client store, together with the account it | ||
| // came from, so Settings shows the rebind and non-composer sessions inherit the pick. | ||
| if (newlyConfirmed.model) { | ||
| void persistPickedModel( | ||
| submission.kind, | ||
| newlyConfirmed.model, | ||
| submission.accountId, | ||
| ).catch(noop); | ||
| } |
There was a problem hiding this comment.
persistPickedModel runs only on the late-confirmation path, so the ordinary case never persists.
newlyConfirmedStartupSelection requires initial.model === null (startup-selection.ts:59) — that is, it fires only when the adapter failed to reflect the pick at session start. When the adapter reflects immediately, reflectedStartupSelection at line 392 already set startupSelection.model = requested.model, so the precondition is false, newlyConfirmed.model is undefined, and the guard on line 415 returns before line 421 is reached.
Nothing else records it now: selectionPatch returns only { effortsByProvider }, modelsByProvider is gone from the v6 store, and the preferredModels prop was removed from NewSessionSurface. The next draft's defaultModels reads providers[kind].model, which was never written.
So a composer model pick survives only when the provider is slow to confirm it. On claude-code and codex, which reflect at start, the user re-picks on every new session — and usePersistPickedModel's own doc says it is called "once a selection is known to have been accepted", which is precisely what immediate reflection is.
Persisting from startupSelection when it already carries a confirmed model, with the late promotion kept for adapters that only confirm after the first turn, covers both. The existing "don't erase a newer live selection" reasoning still argues for keeping the late path conditional.
| model: localModel === null ? undefined : (selectedModel ?? undefined), | ||
| // Pins the session to the account whose entry was picked; without it the daemon would fall | ||
| // back to whichever account happens to be bound. | ||
| ...(localModel !== null && | ||
| modelOption?.accountId !== undefined && { accountId: modelOption.accountId }), |
There was a problem hiding this comment.
This guard excludes an explicit reset, but not an untouched draft — and an untouched draft can still pin an account.
selectedAccounts is only ever written by handleModelChange/handleResetModel, so on a draft where the user never opened the model menu, localAccount and localModel are both undefined. localModel !== null is therefore true, and selectedAccountId is undefined, which makes resolveModel skip the account filter entirely (agent-models.ts:67-68) and return the first entry whose id matches.
accountModelOptions flat-maps accounts in pool order, so when two accounts serve the same model id — a direct vendor account and an OpenRouter/gateway relay, the exact case modelChoiceKey exists for, and which default-models.test.ts:113-119 covers — the winner is whichever account happens to come first in the array. That id ships as accountId, and resolveAccount prefers it over providers[kind].activeAccountId (provider-config.ts:90), so the session runs on a different account's key and base URL than the one the user bound. Nothing in the composer looks any different.
Gating on the account the user actually picked keeps the bound account authoritative for untouched drafts, and still pins correctly once a menu entry is chosen (handleModelChange writes next.accountId ?? null).
| model: localModel === null ? undefined : (selectedModel ?? undefined), | |
| // Pins the session to the account whose entry was picked; without it the daemon would fall | |
| // back to whichever account happens to be bound. | |
| ...(localModel !== null && | |
| modelOption?.accountId !== undefined && { accountId: modelOption.accountId }), | |
| model: localModel === null ? undefined : (selectedModel ?? undefined), | |
| // Pins the session to the account whose entry was picked; without it the daemon would fall | |
| // back to whichever account happens to be bound. | |
| ...(localModel !== null && localAccount != null && { accountId: localAccount }), |
"Provider" meant two different things in adjacent UI: the composer's provider picker chose the *agent*, while the Providers settings page means accounts. `onOpenProviderSettings: (kind: AgentKind) => void` had both in one signature. Agent-meaning UI text and client identifiers now say harness — `selectableHarnesses`, `onHarnessChange`, `lastHarness`, and the composer's menu label. Account-meaning strings keep "provider", `AgentKind` and every wire and daemon term are untouched, and `groupModelsByProvider` stays as the one genuine model-provider use. Same UI/i18n-only discipline already recorded for Thread/`session`. Two things this turned up. Translation keys are not typechecked, so the rename would have silently emptied strings — a test asserting the old menu label is what caught it. And the store test had hand-copied its storage key, which drifted at the previous version bump and had quietly turned the malformed-blob test into a vacuous pass; the key is now exported and imported, and the test verified to fail against a well-formed blob.
|
Why"Provider" meant two different things in adjacent UI. The composer's provider picker chose the agent; the Providers settings page means accounts. What movedAgent-meaning UI text and client identifiers now say harness:
Unchanged on purpose: account-meaning strings (the settings nav Two things the sweep exposedTranslation keys are not typechecked. A test had drifted from the store it tests. Deliberately out of scope
Checks
Pure rename: no behaviour moved, so the manual pass still outstanding for this PR is the one described in the previous comment. |
There was a problem hiding this comment.
ℹ️ The rename is clean — nothing new is broken by it. The findings from both earlier reviews still reproduce at this HEAD, untouched by this commit, so this isn't an approval.
Reviewed changes — one commit since the prior review (8634d2c → 12fde61): a UI/i18n-only rename of the agent meaning of "provider" to "harness".
- Renamed the agent-meaning client identifiers —
selectableProviders→selectableHarnesses,onProviderChange→onHarnessChange,NewSessionDraft.initialProvider→initialHarness,lastProvider→lastHarness, across the composer, the new-session surface, the workbench, and the desktop history-import tab.AgentKindand every wire/daemon term are untouched, andgroupModelsByProvideris left as the genuine model-provider use. I read the commit line by line: it is mechanical — no branch, guard, or ordering changed. - Renamed three interpolation placeholders and one label —
{provider}→{harness}inplugins.discoveryFailed,plugins.runtimeMissingandhistoryImport.panelTitle, pluscomposer.provider→composer.harness, in both locales. Checked each against its call site (plugins-tab.tsx:81,92,history-import-tab.tsx:74,composer-controls.tsx:296); no{provider}placeholder is left in either locale, and no call site passes a name the string no longer has. - Bumped the defaults storage key to
:v7and exported it — correct per the frontend rule (bump on any shape change), and it costs released users nothing beyond the:v6bump already in this PR. The test's hand-copied:v5had been writing to a key nothing read; with the key imported, the malformed-blob fixture really is rejected —'unsupported'is not inEffortLevelSchema(low|medium|high|xhigh|max|ultra|ultracode), so that case is no longer vacuous. - Recorded the terminology rule in
AGENTS.md— provider = account/service, harness = agent, withgroupModelsByProvidernamed as the exception.
I ran the suites this commit touches — new-session-defaults-store, new-session-surface, and the ui plugins tests: 55 tests, all passing.
Claude Opus ｜ 𝕏
| export interface NewSessionDefaultsState { | ||
| /** Provider of the last successful new-session submit; null before the first (→ claude-code). */ | ||
| lastProvider: AgentKind | null; | ||
| lastHarness: AgentKind | null; |
There was a problem hiding this comment.
The rename stops short of the rest of this file. The doc comment two lines up still says v7 renamed lastProvider to lastHarness (fine — that's history) but line 44's /** Provider of the last successful new-session submit; null before the first (→ claude-code). */ now describes a field named lastHarness, and effortsByProvider is still keyed by AgentKind, i.e. by harness, not by provider. Same for the provider: AgentKind parameters on remember and rememberSelection.
If effortsByProvider is going to be renamed at all, it has to happen inside this same :v7 window — renaming it in a later commit costs users a second persisted-state discard for one logical change. If leaving it is deliberate (e.g. you plan to key efforts by account later), the doc line should say so, because right now the name and its key type disagree.
Technical details
# Finish the harness rename inside the defaults store
## Affected sites
- line 40 — doc comment references the pre-rename name in prose describing current state
- line 44 — `/** Provider of the last successful new-session submit … */` documents `lastHarness`
- lines 25, 45, 102-107 — `effortsByProvider`, keyed by `AgentKind` (a harness)
- lines 49, 54, 59, 65-66, 86, 96 — `provider: AgentKind` parameters and locals
## Required outcome
Every agent-meaning identifier and doc line in this file reads "harness"; if `effortsByProvider` is renamed, the rename lands under the `:v7` bump already in this commit so users pay one discard rather than two.
## Open questions for the human
Is leaving `effortsByProvider` under the old name deliberate (future re-keying by account), or an oversight?
Closes CODE-574.
Model selection was a free-text field plus a fixed per-agent list that ignored which account was bound. This replaces both with a set of model ids picked per account: fetch what the service serves, tick what you want, and the composer offers exactly that. Ids can also be typed by hand, which is how endpoints that serve no list work.
Not a bug fix — nothing was broken. It removes the guessing.
Breaking
WIRE_PROTOCOL_VERSIONandMIN_COMPATIBLE_WIRE_VERSIONboth move to 74:Account.modelis removed,ProviderConfig.defaultModelrenamed tomodel, andStartOptions.modelloses itsnulltier. Rebuild and restart the daemon and all clients together — a peer below the floor has its frames refused and dies in the handshake timeout.loadConfigmigrates on read (Account.model→models: [{id}],defaultModel→model). Without it zod strips the unknown keys and existing users lose their configured model.Commits
3a0686402d5dc3a1Account.models,ProviderConfig.model, probe reshape, wire 74, migration0bae7397f7435f85c430eba8AGENT_DEFAULT_MODELSremovalNon-obvious bits
The list URL is hardcoded per service rather than derived from the resolved variant, because derivation is wrong wherever variants sit on different paths — DeepSeek's
anthropicvariant would give/anthropic/v1/models, Vercel's bare-origin one a root/models. All six URLs checked against vendor docs; both Cloudflare services carry none, since/compathas no models route.In the account's model set, present-but-empty and absent differ.
[]means an account is bound with nothing picked, so sending is blocked to match the daemon's refusal. Absent means no account is bound, where the agent resolves its own model and is not blocked — blocking there would break opencode/pi on their own auth.Ids only, no metadata. Both provider-routed agents accept a bare id and fill the rest themselves, and for pi declaring a model it already knows is harmful:
applyModelsJsonreplaces on id match, so redeclaringdeepseek-v4-prooverwrites its real 1M context window with pi's 128k default. Checked against opencode's config schema (allModelfields optional, v1 and v2) and pi'smodelFromJson.Fetch sources are injected rather than called in the forms, which are presentation and sit outside the data-plane provider tree. Catalog services probe with the unsaved secret, saved accounts probe by id so the stored secret stays daemon-side, and subscriptions read codex's start catalog or — for claude-code, which has no enumeration API — the curated table.
Selection lives in the edit form rather than the account detail pane as CODE-574 task 5 described, since
EditAccountFormalready routes non-OAuth accounts throughCustomAccountForm.Checks
pnpm check:ciexits 0.pnpm test: 2742 passing. Two failures are outside this change's module graph — the knownpackages/host/assetsregistry loopback test, andpackages/host/enginerun-command's timeout assertion, which passes in isolation and whose imports (node:child_process,effect,foxts/noop,../observability) this branch never touches.Not driven in the real app. Needs a live DeepSeek key: add the account, Refresh, multi-select, confirm the composer offers that set for every bound agent, confirm send is blocked with none picked, start a session on a picked model, and confirm an upgraded config keeps its previous model.
Follow-ups
cloudflare-gatewayentry points at/compat/chat/completions, which Cloudflare has deprecated in favour ofapi.cloudflare.com/client/v4/accounts/{ACCOUNT_ID}/ai/v1/chat/completions— a different URL shape, so it needs its own issue./v1/modelsreturns per-model effort capabilities anddisplay_namein a response we already parse; useful for the effort picker.