feat(providers): resolve provider endpoints per agent - #424
Conversation
An account resolves to a different endpoint per agent, so it stores the
{placeholder} values a catalog endpoint needs rather than a resolved URL.
A variant is one protocol shape of one service, reached with the same secret; resolveBinding picks one per agent, preferring a provider the agent's own catalog already knows.
Sessions refuse to start when the bound account has no variant the agent speaks, instead of pointing it at an endpoint that answers 404.
Both derived the provider from the model string, so a bare model id left the credential uninjected; pi additionally inherited whatever wire the guessed provider name spoke.
The catalog is now shared with the daemon, so the client reads bindings from it instead of keeping its own copy of the adapters' injection facts.
Adding a workspace package re-resolved jose to 6.2.7, which the committed closure has to track.
All three shapes off one key: anthropic for claude-code, responses for codex, chat through the provider opencode and pi already know.
e5a816e to
9094066
Compare
|
@pullfrog review |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Accounts created from the OpenAI API catalog entry before this change stop working for codex after upgrade — start-options-resolver refuses the session rather than degrading. That contradicts "Existing accounts keep working untouched; no data rewrite." Details inline on resolve.ts.
Reviewed changes
Reworks account→endpoint resolution so the endpoint is picked per agent instead of being pinned at account creation, extracts the catalog and resolver into a shared @linkcode/providers read by both daemon and client, and lands DeepSeek on top.
- New
packages/foundation/providers:catalog.ts(services withvariantskeyed byAccountProtocol,credentialTypehoisted to the service),resolve.ts(resolveBinding),template.ts(isTemplateFilled). - Engine:
applyProviderDefaultsnow returns{ options, unavailable? }, andstart-options-resolver.tsturnsunavailableinto a fatalRequestError({ code: 'unsupported' }). - Adapters:
protocol+knownProvideronAgentCredential; opencode falls back tocred.knownProvider, pi passes an explicitapionly for guessed providers. - Client: protocol
RadioGroupand its i18n deleted; catalog accounts persistservice+endpointParams. - Wire 72 → 73 (additive, floor untouched);
cloudflare-gatewaysplit into two services.
The variant model is a real simplification — collapsing grok-build's and DeepSeek's service-name escapes back into one table is the right call, and the resolver tests are exact-value assertions rather than shape checks.
"Existing accounts keep working untouched" doesn't hold
The PR body diagnoses the root cause precisely — "the two mislabels cancelled, so codex + OpenAI worked by accident" — but the openai-chat mislabel doesn't only live in the catalog. It is persisted on disk in every account the old catalogAccount() wrote. This PR corrects the codex half of that cancelling pair and leaves the stored half in place, so the pair stops cancelling for accounts that already exist.
Worth deciding explicitly whether the read-side branch should also ignore a stored endpoint that exactly matches what the old catalog would have produced for that service. That single rule fixes the codex break and lets existing accounts pick up the knownProvider work this PR ships, instead of only new ones.
No test covers the new hard failure
resolve.test.ts covers resolveBinding thoroughly and provider-config.test.ts covers the unavailable passthrough, but nothing asserts that start-options-resolver actually converts unavailable into a RequestError. That conversion is the whole behavioral delta from master — it's what turns a previously-silent misconfiguration into a refused session — and right now it's only verifiable by reading the code.
Checked separately, no action needed
- All seven pi
knownProviderids are members ofKnownProviderin the installed@earendil-works/pi-ai; theopencode: 'vercel'/pi: 'vercel-ai-gateway'asymmetry is correct, not a typo. piApi()'s threeKnownApivalues andregisterProvider'sapifield both match the installed typings.https://gateway.ai.cloudflare.com/v1/{account_id}/{gateway_id}/anthropicmatches Cloudflare's current docs, and that page documents an own-key ("With API Key in Request") mode — so splittingcloudflare-anthropicoff withcredentialType: 'api-key'is sound.- opencode's ids can't be verified against the installed SDK (its catalog is remote models.dev data), so those rest on the verification recorded in
AGENTS.md.
Claude Opus ｜ 𝕏
The old add flow pinned an endpoint onto every catalog account, so honoring it refused codex on OpenAI accounts that work today. Pi's resume path also has to outrank the catalog default.
|
Both findings hold up and are fixed in b4a1f3d — replied inline on each thread with the detail. Summary here for the one that had no anchor. No test for the I checked each new test fails against the unfixed code rather than assuming: reverting The PR description's "Existing accounts keep working untouched" claim was wrong and has been corrected in place.
Still outstanding: this hasn't been driven end-to-end against a real daemon, which now matters more, since the migration path is exactly what unit tests can assert and reality can still contradict. |
There was a problem hiding this comment.
Caution
My previous review was wrong about three of these services, and this PR kept the behavior on the strength of it. xai, openrouter and vercel-gateway all serve POST /responses at the exact base URL the catalog declares, so omitting an openai-responses variant refuses codex on accounts that work on master — and on newly created accounts too. Review 4870508952 said those "were already broken, now loud"; that scope note was unverified and false. Evidence inline on catalog.ts.
Reviewed changes — one commit, b4a1f3db "fix(providers): keep pre-variant accounts working across the upgrade".
It addresses both prior threads, and addresses them well. isCatalogDerived is the better of the two proposals: comparing against the current catalog reaches the same answer as my "what the old catalog would have produced" suggestion without retaining a snapshot of the deleted table. I checked the cases the reply claims: service: 'custom' and custom accounts (no service) fall through to undefined and keep their endpoint; a filled Cloudflare URL never matches a templated variant, so those stay pinned exactly as on master. The pi reorder is right, and the resume test that pins it asserts the specific provider rather than a shape. All five new tests can genuinely fail.
Downgrading past this commit misroutes silently
Not introduced here and not obviously worth fixing — but it is the mirror of the bug b4a1f3db just fixed, so it deserves a decision rather than an accident.
New-style catalog accounts store no endpoint. On an older build, accountConfigBundle sets baseUrl/protocol only when account.endpoint exists, and the old serviceById returns undefined for a service it has never heard of (deepseek), so bindingAvailability reports native for every agent. The older build therefore injects the api key alone and lets the agent fall back to its own default vendor endpoint — a DeepSeek key sent to api.anthropic.com. The wire floor was deliberately left untouched (correct — the frame change is additive) and there is no on-disk data version, so nothing catches the mismatch. Forward compatibility is what this commit fixed; this is the other direction.
Checked, no action needed
resolveGrokBuild, theendpoint-incompletepath, andendpointParamspersistence are unchanged by this commit and behave as documented.endpointParamsis additive on the wire; the floor is correctly untouched.
Claude Opus ｜ 𝕏
xAI, OpenRouter and Vercel all serve POST /responses at the declared base URL, so omitting the variant refused codex outright. pi must also state the wire whenever the provider is not the resolved known one, and the client now shares the resolver's notion of a pinned endpoint.
Three optional fields expressed two mutually exclusive states, so the view models permitted combinations the producer never emits.
There was a problem hiding this comment.
Reviewed changes since b4a1f3d — one commit, a00dd614 fix(providers): serve codex on every endpoint that speaks Responses. It does three things: exports pinnedEndpoint and routes the three client read sites through it, adds openai-responses variants for xai / openrouter / vercel-gateway, and flips the pi api gate.
Two of the three open threads are cleanly addressed and I've resolved them:
view.tsdrift — both view models now derive frompinnedEndpoint, andadd-flow.tsxprefills from it too. I checked the emptied-form case:CustomDraftSchema.baseUrlis an unconstrainedz.string()andaccountFromCustomDraftdestructuresendpointout of the preserved...rest, so saving a form whose legacy pin was suppressed drops the pin without blocking the save or losingservice. Complete.- Catalog gap — the three responses variants carry no
knownProvider, andpreferredProtocolssorts known-provider variants first, so opencode/pi routing is undisturbed. The newxaitest pins exactly that.
The pi thread stays open — see line 178.
⚠️ The pi wire fix doesn't reach the wire, and that's my fault
I proposed the exact line in a00dd614 in my last review, and I was wrong about what it does. The gate is restored, but api never takes effect on this call shape, so the underlying bug is unchanged. Worse, packages/host/agent-adapter/AGENTS.md:118 now states both halves in the same paragraph without noticing they contradict each other — it correctly says the override-only branch "patches baseUrl and leaves each model's api at pi's built-in value for that provider name", then concludes that passing config.protocol states the wire. It doesn't. Details inline.
ℹ️ Nitpicks
packages/foundation/providers/AGENTS.md:58says "we pin 0.144.1" for codex. Nothing pins that:pnpm-lock.yamlresolves@openai/codex@0.144.6, and the only0.144.1in the repo is a staleminimumReleaseAgeExcludeentry inpnpm-workspace.yaml:118. The load-bearing claim (chat removed since 0.122) is unaffected — just drop the version or say "0.144.x".
Claude Opus ｜ 𝕏
registerProvider discards api on any call without models, so the hint never reached the wire. Injection is correct exactly where pi already ships metadata, which is every provider it knows.
There was a problem hiding this comment.
ℹ️ No critical issues — minor suggestions inline.
Re-reviewed the two commits since a00dd614: 61130e4 (UI routing view-model) and da2f30d (drop the pi wire hint).
da2f30d resolves the open [!IMPORTANT] correctly. Deleting piApi(), the api argument, and AgentCredential.protocol outright is the right call — a models-less registerProvider was discarding api anyway, so the honest move was to stop passing it rather than to keep a gate that reads as a fix. Both AGENTS.md files now describe the limit instead of claiming it away, and the two tests that asserted the discarded value are gone, which removes the test theatre I flagged last round. I checked for fallout: nothing under packages/host/agent-adapter/src still reads AgentCredential.protocol, and engine translation is unaffected because translator.ts:38 reads config.protocol off the config bundle emitted at provider-config.ts:110, not off the credential. And to be explicit about my own earlier framing — you were right that api had been inert since d675bb86; the gate change only moved when a dead value was passed.
61130e4 is a clean collapse of the contradictory optionals. Folding endpoint?/protocol?/protocols? into the ProviderAccountRouting discriminated union removes states the old shape allowed but nothing could render, and the single accountRouting() helper in view.ts keeps the list and detail view models from drifting apart the way the display/edit predicates did earlier in this PR. All consumers across presentation/ui and settings/providers are migrated.
One user-visible behavior change rides along inside that refactor — see the inline note.
Smaller point, no line to anchor to: settings/providers/__tests__/view.test.ts asserts the list view model with an exact toEqual, but only exercises the catalog routing kind. The pinned branch is the one that has to keep agreeing with pinnedEndpoint's exact-baseUrl match, so it is the branch most likely to regress silently. A second case over a pinned account would be cheap.
Also worth knowing: pnpm vitest fails three files here with Cannot find package '@linkcode/providers'. That is this ephemeral checkout never having linked the new workspace package — the manifests, pnpm-lock.yaml, and the root tsconfig.json references entry are all correct in the diff. Not a defect in the PR.
The downgrade direction remains the one open design question and is unchanged by this delta: a catalog account stores no endpoint, so an older build injects the key alone and lets the agent fall back to its own default vendor URL. The wire floor is deliberately not moved and there is no on-disk data version, so nothing detects it. Flagging it as carried-forward context, not as something to fix here.
Claude Opus ｜ 𝕏
The refactor widened a render gate that had been masking an unreachable branch, surfacing protocol identifiers on every catalog row. One function now decides the line, so no gate can disagree with it.
There was a problem hiding this comment.
✅ No new issues found.
e6e84b6 addresses both items from the previous review, and the fix for the first one is stronger than what was suggested.
Reviewed changes
account-master-list.tsx—accountDetailLinenow returnsstring | undefinedand the separate render gate is gone, so "does this row have a second line" is decided in exactly one place instead of two that could disagree.- The
catalogarm was deleted rather than re-gated:account.routing?.kind === 'pinned' ? … : undefined. That restores the pre-61130e4behavior (catalog rows show no third line) without recreating the unreachable branch that caused the regression in the first place. view.test.ts— newacc_cfixture covering thepinnedrouting kind, which no prior test exercised.
Verified the deleted arm does not leave dead code behind: the catalog variant of ProviderAccountRouting is still live — account-detail.tsx:194 renders routing.protocols under the labelled t('protocols') row, which is where bare protocol names actually have a label to give them meaning. view.ts's single accountRouting() still feeds both the list and the detail view models, so the two cannot drift. The search filter (routing.kind === 'pinned' ? baseUrl : '') is unchanged and stays consistent with the display rule.
The new test is genuine coverage, not theatre: acc_c stores https://proxy.internal/v1 under service: 'openrouter', a URL the catalog never emits, and asserts the exact { kind: 'pinned', baseUrl, protocol } shape via toEqual. If pinnedEndpoint's exact-baseUrl comparison ever loosens, the value flips to { kind: 'catalog', protocols: [...] } and the assertion fails.
Every Pullfrog thread opened across the four prior reviews on this PR is now resolved. The one thing still outstanding is the end-to-end run the PR description already calls out — binding a single OpenRouter account to claude-code and codex simultaneously against a real daemon, confirming claude-code reaches the endpoint natively with no translator spawn and that a pre-existing config still binds. No review pass can substitute for that.
Claude Opus ｜ 𝕏
|
E2E Tested using Claude Code + GPT-5.6 Luna on OpenRouter. Works properly. |
Live testing found a codex defect — filed as CODE-572, not fixed hereRan this branch against a real daemon. claude-code + OpenRouter works, which is the half this PR is about: the resolver picks OpenRouter's codex does not, and the reason is upstream of this PR. Binding the same OpenRouter account to codex fails with: That rejection is from OpenAI's ChatGPT backend, not OpenRouter — the request never reached the bound endpoint. It normally fails silently — a plain codex model id would have run fine on the wrong account. The OpenRouter-shaped model ref is the only reason it surfaced, and that ref reaching codex is the Pre-existing, and deliberately out of scope here. Full trace, root cause, proposed fix and open questions are in CODE-572. |
Rebuilds the provider/account model so an endpoint is resolved per agent instead of being pinned to the account at creation time, and closes #421 by landing DeepSeek on top of it.
Why
Three defects, all silent:
codex: ['openai-chat', 'openai-responses']was false. Chat Completions was removed from the codex CLI (wire_api = "chat"is a hard config-load error since 0.122, openai/codex#7782); we pin 0.144.1, and our injection is env-only (OPENAI_BASE_URL+CODEX_API_KEY), which overrides the base URL of codex's built-in Responses provider. A chat-only endpoint getsPOST /responses→ 404.openai-apiwas labelledopenai-chat, also false. The two mislabels cancelled, so codex + OpenAI worked by accident andopenai-responseswas never produced by any catalog entry.opencode: ['openai-chat'],pi: ['openai-chat']reported our own unimplemented injection as vendor limitations. Both accept any wire; both derived the provider id from the user-typed model string, so a bare model id left the credential uninjected entirely.Underneath:
capability.tswas a hand-maintained mirror ofpackages/host/agent-adapter's injection facts — its own doc comment claimed "the UI hardcodes nothing" — and it had drifted.grok-buildalready escaped the protocol table via a service-name check, and #421 added a second escape (account.service === 'deepseek'). Two of five agents were special-cased.An account was also pinned to one variant, so one key serving three protocols needed three account rows and the user had to know which agent wanted which — the
variantNote.*strings existed to teach them that.What changed
A variant is one protocol shape of one service, reached with the same secret. Endpoints needing a different secret, or meaning something different to the user, are separate services.
variantsis keyed byAccountProtocol, so duplicate shapes are impossible by construction;credentialTypemoved to the service, which is what the shared-secret rule means.Variants are invisible. The add form asks for a secret and any
{placeholder}values;resolveBinding(account, kind)picks the variant per agent, preferring one whoseknownProvidernames an entry in that agent's own catalog. The protocolRadioGroupand its i18n are gone.New
packages/foundation/providers, read by both ends — the daemon at session start, the client for availability badges.@linkcode/enginecan't import workbench, and one shared copy is the point.cloudflare-gatewaysplits in two. Its pass-through leg authenticates with the user's own Anthropic key, so auto-selection would have handed claude-code a Cloudflare token.DeepSeek lands as 51 added lines, nothing modified: anthropic for claude-code, responses for codex, chat via the
deepseekprovider opencode and pi already know. One key, four agents, no translator.Notes for review
Accountgains optionalendpointParams, andAccountsSchemaridesconfig.get.result/config.set. Nothing removed, renamed, or reinterpreted.Account.endpointoutranks the catalog only when the user named it: the pre-variant add flow wrote an endpoint onto every catalog account, and honoring those would have pinned existing accounts to one protocol — refusing codex on OpenAI accounts that work today.isCatalogDerivedcompares the storedbaseUrlagainst the service's own variants; a match is the catalog's own output and gets re-resolved per agent, anything else is a human-typed URL and is kept. No data rewrite.translation-needs-endpointbecame unreachable (a catalog service always supplies a base URL) and was replaced byendpoint-incomplete, which fires when a templated URL still holds a{placeholder}— otherwise we'd quietly request a literal{account_id}URL. Same union size, one i18n string reworded per locale.knownProvider. opencode sends{providerID, modelID}on every prompt and pi resolvesfind(ref.provider, ref.modelId), so if the model names a provider the credential must be injected under that one.knownProviderfills the gap where the bug actually was: a bare model id.registerProvider({ baseUrl, api })); the SDK discardsapion any call withoutmodels, so it never reached the wire — dropped in da2f30d rather than faked. Injection is correct exactly where the target provider's built-in wire already matches the endpoint, which covers every provider pi ships metadata for; differently-shaped endpoints belong with the unresolvedmodels[]metadata question under Known gaps.packages/host/assets/src/pi-closure.gen.tsis unrelated drift. Adding a workspace package re-resolvedjose6.2.4 → 6.2.7 under the release-age policy; the committed closure has to track the lockfile. Isolated in its own commit (950ad7ff), regenerated viagenerate:pi-closure.Known gaps
A deliberately hand-typed catalog URL is indistinguishable from a generated one.
pinnedEndpoint()infers intent from an exactbaseUrlmatch, which protects a hand-typed proxy but drops a catalog URL the user typed on purpose to pin the legacy shape. Distinguishing them needs an explicit "user pinned this" bit on the account — a schema change worth doing deliberately rather than smuggling in here.Downgrading past this branch misroutes silently. New-style catalog accounts store no
endpoint, so an older build injects the api key alone and lets the agent fall back to its own default vendor endpoint. The wire floor is correctly untouched (the frame change is additive) and there is no on-disk data version, so nothing catches it. Forward compatibility is fixed; this is the other direction, and it is pre-existing rather than introduced here.Registering a custom provider for an endpoint no agent knows (opencode
provider.<id>.{npm, models}; piregisterProviderwithmodels[]) is not implemented. Endpoints without a known provider keep the pre-existing behaviour. pi'smodels[]requiresreasoning/input/cost/contextWindow/maxTokens, none of which/v1/modelscarries, andcontextWindowfeeds pi's compaction math — so the metadata source is a real decision, recorded in the package'sAGENTS.md.Account.modelis one string across every variant. DeepSeek's anthropic leg remaps Claude names while the other two want DeepSeek ids, so filling the form's "Default model" hands one vocabulary to both bindings. Blank works (each agent falls back toproviders[kind].defaultModel). New consequence of one account spanning several endpoints; no per-variant model allowlist was added.DeepSeek's Responses API still serves
deepseek-v4-flashonly — v4-pro was slated for early August 2026 and hasn't landed.Verification
pnpm check:ciandpnpm testboth clean (2531 pass).registry-client.test.ts"falls through to the next registry on HTTP errors and network failures" fails on my box from loopback timeouts — confirmed failing onmastertoo, not a regression.Resolver unit tests cover: the three-variant service resolving natively for four agents; anthropic preferred over translation; codex refusing a service with no responses variant; the known-provider preference beating protocol order; templated endpoints filled from
endpointParamsand refused when unfilled; an explicit endpoint outranking the catalog; grok-build's vendor check; oauth agent locking.Not yet driven end-to-end. The check that matters needs a real daemon: bind one OpenRouter account to claude-code and codex simultaneously — impossible on
master, since the account is pinned to a single protocol — then confirm claude-code reacheshttps://openrouter.ai/apinatively with no translator spawn in the daemon log, and that a pre-existing config still binds.Closes #421.