Skip to content

fix(pi): preserve account model IDs with slashes - #471

Open
lucas77778 wants to merge 4 commits into
masterfrom
chenyu/code-610
Open

fix(pi): preserve account model IDs with slashes#471
lucas77778 wants to merge 4 commits into
masterfrom
chenyu/code-610

Conversation

@lucas77778

Copy link
Copy Markdown
Member

Summary

  • resolve account-bound Pi models as (knownProvider, complete AccountModel.id) so endpoint-owned IDs containing slashes are not misread as Pi provider-qualified references
  • preserve Pi-qualified references without account provider evidence and resumed-session provider fallback
  • cover OpenRouter, Vercel AI Gateway, nested model IDs, and installed Pi SDK registry startup

CODE-610

Verification

  • pnpm check:ci
  • pnpm test — 347 files passed, 1 skipped; 2,986 tests passed, 1 skipped
  • real installed-SDK startup tests for direct Anthropic, OpenRouter, and Vercel AI Gateway model resolution

Checklist

  • pnpm check:ci and pnpm test both pass (plus cargo fmt / clippy / test for Rust changes)
  • I ran the affected surface and observed the change working
  • If a wire message changed: WIRE_PROTOCOL_VERSION is bumped
  • New code and assets are my own work, or their origin and license compatibility are noted above
  • Docs and comments are updated where behavior changed

Copilot AI lite review requested due to automatic review settings August 23, 2026 13:31
@linear-code

linear-code Bot commented Aug 23, 2026

Copy link
Copy Markdown

CODE-610

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@greptile-apps

greptile-apps Bot commented Aug 23, 2026

Copy link
Copy Markdown

Greptile Summary

The PR updates Pi account-model resolution so slash-containing endpoint IDs remain opaque under their known provider, while supporting same-provider qualified replay when the opaque key is unavailable.

  • Shares model-reference resolution between startup and live model selection.
  • Emits account-owned model IDs without provider prefixes and adds installed-registry coverage for direct and gateway providers.
  • Adds provider-specific endpoint environment mapping for Cloudflare AI Gateway’s per-model routes.

Confidence Score: 4/5

The PR should not merge until resumed Pi sessions can select models advertised from a saved provider that differs from the account’s known provider.

A resumed session scopes its credential and catalog to the transcript’s saved provider, but live selection resolves the advertised qualified model under the account’s different known provider and rejects it at the credential-scope guard.

Files Needing Attention: packages/host/agent-adapter/src/native/pi/adapter.ts

Important Files Changed

Filename Overview
packages/host/agent-adapter/src/native/pi/adapter.ts Centralizes Pi model resolution, account-aware catalog IDs, resume routing, and provider environment credential setup; the previously reported differing-provider resume rejection remains.
packages/foundation/providers/src/resolve.ts Resolves declared endpoint parameters into provider-specific environment variables while preserving normal endpoint completeness checks.
packages/foundation/providers/src/catalog.ts Declares Cloudflare AI Gateway’s Pi-specific endpoint environment mapping so Pi can retain per-model route construction.
packages/host/engine/src/agent/provider-config.ts Propagates resolved provider environment values into agent startup configuration.
packages/host/agent-adapter/src/credential.ts Parses provider environment data from startup configuration into the adapter credential shape.
packages/host/agent-adapter/src/tests/pi-model.test.ts Adds coverage for opaque slash-containing account IDs, qualified replay fallback, and explicit-model resume behavior.
packages/host/agent-adapter/src/tests/pi-model-registry.test.ts Adds installed-SDK startup coverage for direct Anthropic, OpenRouter, and Vercel AI Gateway model resolution.

Reviews (5): Last reviewed commit: "fix(providers,engine,agent-adapter): per..." | Re-trigger Greptile

Comment thread packages/host/agent-adapter/src/native/pi/adapter.ts Outdated

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Important

The fix is correct and the new integration test is real coverage, but the change silently reorders the provider-precedence chain that packages/host/agent-adapter/AGENTS.md:118 documents as an ordered list, and one pre-existing test that should have caught it is now a false green.

Reviewed changes59f102f (3 files, +86/-6): the createConfiguredRegistry model-ref branch in native/pi/adapter.ts, the new installed-SDK startup test __tests__/pi-model-registry.test.ts, and the reworked mocked-SDK cases in __tests__/pi-model.test.ts.

The bug is real and I confirmed it against the installed SDK. @earendil-works/pi-ai's MODELS table gives anthropic 14 models with zero slash-containing ids, while openrouter (270) and vercel-ai-gateway (192) both carry anthropic/claude-sonnet-4.6. So pre-fix an OpenRouter account model anthropic/claude-sonnet-4.6 was parseModel'd into provider anthropic + modelId claude-sonnet-4.6, which pi's anthropic table lacks, and onStart threw. The new pi-model-registry.test.ts genuinely fails without this patch — it is not test theatre, and using the real installed SDK rather than the vi.mock harness is the right call for exactly this class of bug.

I also checked the hypothesis that a pi-qualified provider/modelId string could round-trip back into StartOptions.model through the UI, and it cannot: packages/presentation/ui/src/shell/conversation-surface.tsx:209-212 passes agentModels={accountModels ?? null} and explicitly documents that the adapter's own catalog is not offered, accountModelOptions emits raw AccountModel.id, and apps/mobile has no picker at all. That path is closed.

Nitpick: cred.knownProvider at adapter.ts:160 is now unreachable

const provider = fallbackProvider ?? cred.knownProvider ?? endpointProvider; sits inside if (!ref && opts.model). After this change ref is null with a model present only when cred.knownProvider is absent, so that middle term can never contribute. It sits between the two diff hunks so I can't anchor it inline. Worth deleting, or the next reader will assume the endpoint-sniffing fallback still consults it.

Technical details

Pre-fix, ref was null whenever parseModel failed on a slash-less id, regardless of knownProvider, so the term was live. Post-fix, opts.model && cred.knownProvider always produces a ref, so entry into the block implies !cred.knownProvider. The endpointProviders baseUrl sniff below it is likewise now reachable only for account-less starts that carry a baseUrl, which is a narrower set than before.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

Comment thread packages/host/agent-adapter/src/native/pi/adapter.ts Outdated
Comment thread packages/host/agent-adapter/src/native/pi/adapter.ts Outdated
Comment thread packages/host/agent-adapter/src/native/pi/adapter.ts Outdated
Comment thread packages/host/agent-adapter/src/__tests__/pi-model.test.ts
Copilot AI review requested due to automatic review settings August 24, 2026 03:25

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@lucas77778
lucas77778 requested review from PeronGH and xiaoland August 24, 2026 03:35

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Important

The new unwrap branch is correct and its tests genuinely fail without it. But the thread that raised the onSetModel / createConfiguredRegistry divergence was resolved as fixed, and it isn't — the fix covers only the <knownProvider>/… replay shape at start, not live model switching.

Reviewed changes (delta vs. the prior review at 59f102f6)

  • native/pi/adapter.ts:141-154 — the account-id override is now narrowed by registry evidence: ref = accountRef ?? parsedRef, reverted to parsedRef only when the provider halves match, find(accountRef) misses, and find(parsedRef) hits.
  • native/pi/adapter.ts:166 — the unreachable cred.knownProvider tier in the endpoint-sniff fallback was deleted. Correct: entry into if (!ref && opts.model) implies !cred.knownProvider, so the term was dead.
  • AGENTS.md:118 — precedence paragraph rewritten to the new order.
  • Four new tests (pi-model-registry.test.ts it.each row 4, three mocked cases in pi-model.test.ts).

The new tests are real coverage. I neutered the unwrap condition (accountRef && Date.now() < 0 && …) and re-ran the suite: exactly resolves 'openrouter' input 'openrouter/anthropic/claude-sonnet-4.6' … and unwraps a same-provider Pi-qualified replay … turn red while the other 21 stay green.

On the unwrap's false-positive window. It can only misfire on an account id shaped <knownProvider>/<X> that Pi's table lacks while it knows bare <X> under the same provider. Against the installed @earendil-works/pi-ai table that window is empty: openrouter has exactly one slash-less id (auto) and also carries openrouter/auto; vercel-ai-gateway and nvidia have zero slash-less ids; the self-prefixed ids that do exist (openrouter/{auto,free,fusion}, five nvidia/*) are all present under their own provider, so find(accountRef) hits and the unwrap never runs. Data-dependent rather than structurally guaranteed, but no action needed today — noting it so a future table change doesn't surprise anyone.

Also worth knowing: the old model-less test injects the credential under the provider the session last routed through is a legitimate green now, not a leftover — it pins the "no explicit model → resume evidence wins" tier, and the new sibling test covers the explicit-model tier.

ℹ️ Nitpicks

  • AGENTS.md:118 — the lead-in still reads "The two provider-routed agents…" while the rewritten body now describes only Pi. Opencode's half of that precedence rule was dropped from the bullet.
  • AGENTS.md:109 — the capability matrix lists pi set-model as ✗ even though onSetModel is implemented and reachable. Pre-existing and independent of this PR, but it's the doc a reader would check when weighing the inline comment below.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

Comment thread packages/host/agent-adapter/src/native/pi/adapter.ts Outdated
@xiaoland

Copy link
Copy Markdown
Member

手上没有 Acceptance Criteria 要求的 Anthropic API / OpenRouter / Vercel AI Gateway / Cloudflare AI Gateway 可以进行测试。

Copy link
Copy Markdown
Member Author

@AprilNEA

Copy link
Copy Markdown
Member

Cloudflare AI Gateway 可以了吗

Copy link
Copy Markdown
Member

嗯,可以了,我稍后继续完成 review

@xiaoland xiaoland left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I test with Cloudflare AI Gateway (which is the only I have), using gateway linkcode and configures model anthropic/claude-sonnet-4-6 and custom-subscription/claude-sonnet-4-6 which is two model id that lincodehq's ai-gateway source has.

And I got these failures:

Image Image

From logs, I got:

pi: model 'custom-subscription/claude-sonnet-4-6' is not available
for provider 'cloudflare-ai-gateway'

This is likely to be a bug that this PR should resolve but not.

With model claude-sonnet-4-6, this error gone, but new error comes:

Image

Which likely to be a protocol mismatch.

Copy link
Copy Markdown
Member Author

I see this problem and will both test it, can you use anthropic/claude-opus-4.8 to test again. Since the custom-subscription/claude-sonnet-4-6 and anthropic/claude-sonnet-4-6 are not found in cf gateway.

Screenshot 2026-08-24 at 19.49.42.png

Copilot AI review requested due to automatic review settings August 24, 2026 12:49

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Important

The delta does exactly what the previous review asked for, and I verified it works. But it ships with zero test coverage, and while re-tracing it I found a reflection desync I should have caught in my two earlier reviews.

Reviewed changes (since 78c2f58):

  • 856065ed is adapter-only: onSetModel (packages/host/agent-adapter/src/native/pi/adapter.ts:472) now routes through the extracted resolveModelRef instead of bare parseModel, and createConfiguredRegistry returns credential so onSetModel can reuse the account's knownProvider/baseUrl (:342).
  • Tests and AGENTS.md are untouched by this commit.
  • The prior thread on the onSetModel divergence is resolved — I re-ran its own repro against 856065ed and it passes.

Extraction verified behavior-preserving on the start path: the old accountRef guard was opts.model && cred.knownProvider, the new one is cred.knownProvider inside a call already gated on opts.model. And ModelRegistry.find is a strict two-key lookup, so :477's if (!model) throw still blocks any wrong-provider pairing.

⚠️ The delta has zero test coverage

I neutered the fix to measure this: reverting :472 back to const ref = parseModel(value); if (!ref) throw … leaves all 19 pi-model.test.ts tests green. Nothing in the suite notices.

The reason is that the only existing set-model test (pi-model.test.ts:277, "switches model and effort live and reflects SDK readback") calls start() with no config, so there is no knownProvider — it never reaches the account-bound branch this PR exists to fix.

A test that does catch it is small: start a session with config: { apiKey, baseUrl: 'https://openrouter.ai/api/v1', knownProvider: 'openrouter' } on anthropic/claude-sonnet-4.6, then send({ type: 'set-model', model: 'openai/gpt-5' }). Pre-fix it dies with pi: this session's credential is scoped to 'openrouter'; post-fix it succeeds. Please add something like that — otherwise the next refactor of this helper silently reintroduces CODE-610.

⚠️ model-update emits an id the client cannot resolve back to a menu entry

I missed this in reviews 5002513941 and 5004367852 — apologies. The root cause is the changed ref resolution from 59f102f6; 856065ed extends the bad emission to the mid-session path, which previously rejected the pick outright.

I traced every link:

  • Emissionadapter.ts:377 (start) and :479 (set-model) send `${model.provider}/${model.id}`. Account-bound, model.id is now the whole account id, so the wire value becomes openrouter/anthropic/claude-sonnet-4.6.
  • Menu options are the raw AccountModel.idsettings/providers/model-options.ts:31-36 (id: model.id), and conversation-surface.tsx passes agentModels={accountModels ?? null} (the adapter's own catalog is deliberately not offered).
  • No normalization in betweenclient/core/src/conversation.ts:529 is currentModel = event.model; verbatim.
  • Resolution failsagent-models.ts:75-88 resolveModel tries exact match, then id.startsWith(`${option.id}-`). A prepended path segment matches neither, so it returns undefined.

Downstream:

  • composer-controls.tsx:295modelLabel falls back to the raw openrouter/anthropic/claude-sonnet-4.6 string in the chip.
  • composer.tsx:803-805effortOptionsForModel is computed from undefined, so per-model effort options are lost.
  • startup-selection.ts:32 — compares reflectedModel === requested.model exactly, so the startup selection is cleared.

Bounded, at least: selectedAccountId comes from session state (currentAccountId={accountId}), not from resolveModel, so there's no spurious cross-account relaunch.

Two things make this worth fixing in this PR rather than deferring: pi-model-registry.test.ts:57 asserts the desynced string (`${provider}/${modelId}`), so the PR's own test encodes it as intended behavior; and packages/host/agent-adapter/AGENTS.md records that opencode already solved exactly this by emitting the id the user picked, not its own prefixed readback. Pi should do the same.

ℹ️ Scope: the blocking review is about catalogs pi does not carry

The open CHANGES_REQUESTED review (5007232296) reports Cloudflare AI Gateway with custom-subscription/claude-sonnet-4-6 failing as pi: model 'custom-subscription/claude-sonnet-4-6' is not available for provider 'cloudflare-ai-gateway'. I confirmed the mechanism against the installed @earendil-works/pi-ai table: cloudflare-ai-gateway carries 38 ids, all slash-less, and no custom-subscription/*.

So the id resolution this PR introduces is correct there (accountRef = the whole id under cloudflare-ai-gateway) — the remaining failure is the separate, already-documented limitation that registerProvider without models cannot add table entries. Not a regression from this PR. But it does mean CODE-610's user-reported scenario still fails, and that blocking review stays unaddressed. Worth stating explicitly in the PR description whether this lands as a partial fix with the gateway case tracked separately, or whether it waits.

For context on the false-positive window of the new unwrap: I enumerated the table and it is empty today — every self-prefixed id (openrouter/{auto,free,fusion}, 5 nvidia/*) is present under its own provider. That's data-dependent, not structural, but it means the unwrap is safe as shipped.

Pullfrog  | Fix it ➔View workflow run | Using Claude Opus𝕏

Copilot AI review requested due to automatic review settings August 24, 2026 13:18

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@lucas77778

Copy link
Copy Markdown
Member Author

Two follow-up commits, both found by driving a real Pi session against a cloudflare-gateway account on this branch. They widen the scope beyond CODE-610's model-id parsing; happy to split them out if that reads better.

856065ed — share model resolution between session start and set-model

CODE-610 fixed resolution in createConfiguredRegistry but left onSetModel on the old parseModel-only path, so the two disagreed about the same id. No writing satisfied both:

account model id onStart onSetModel (before)
claude-opus-4-8 starts pi: model must be 'provider/modelId'
anthropic/claude-opus-4-8 not available for provider 'cloudflare-ai-gateway' credential is scoped to 'cloudflare-ai-gateway'
workers-ai/@cf/moonshotai/kimi-k2.6 starts parses as provider workers-ai, fails the scope guard

An account-bound session could start but never change model. The resolution logic is extracted verbatim into resolveModelRef and both paths now call it; onSetModel passes the running model's provider where onStart passes the resumed session's last-routed one.

c28f531e — per-leg gateway routing, and account model ids on the wire

The base-URL override flattened routes that differ per model

registerProvider(provider, { baseUrl }) rewrites the URL and leaves each model's wire at Pi's built-in value. That holds only where the provider's wire already matches the endpoint — and Pi's cloudflare-ai-gateway entry spans three legs:

wire leg models
anthropic-messages /anthropic 17 (claude-opus-4-8, …)
openai-responses /openai 16 (gpt-5.4, …)
openai-completions /compat 4 (workers-ai/@cf/…)

Pinning all of them to the variant's /compat answered 400 Compatibility endpoint: v1/messages is not supported on 33 of 37; only the four that were already /compat worked (verified live — workers-ai/@cf/moonshotai/kimi-k2.6 runs, claude-opus-4-8 400s).

Pi builds the right per-model URL itself from CLOUDFLARE_ACCOUNT_ID / CLOUDFLARE_GATEWAY_ID, but it reads those only off a stored credential: setRuntimeApiKey carries no env, ModelRegistry has no env-injection API, and ProviderConfigInput has no env field. So ServiceVariant gains endpointEnvendpointParams key → the env name that agent's own provider entry reads it under. resolveBinding resolves it against the account, the engine passes it as providerEnv, and the Pi adapter seeds AuthStorage.inMemory with { key, env } and skips the base-URL override. In-memory because set() on the file-backed store would leave the account's secret in ~/.pi/agent/auth.json.

Verified against the installed SDK: all 37 models resolve, each keeping its own template and wire.

endpointEnv is declared for pi only. OpenCode reaches this service through /compat with models.dev's anthropic/claude-opus-4.8-style ids and works today, so it is deliberately left alone.

Behavioural consequence worth a second opinion: a seeded store holds one provider's credential, so getAvailable() narrows to that provider and a user's own ~/.pi/agent/auth.json logins are invisible inside an account-bound session. credentialProviderId already refuses cross-provider switches there, so this is consistent — but it is observable.

The composer lost track of the running model

emitModel sent ${provider}/${id} while the composer's options come from the account's model list (accountModelOptions → bare claude-opus-4-8). resolveModel matched neither exactly nor by its startsWith(${option.id}-) rule, so composer-controls.tsx:295 fell through to rendering the raw id and the picker showed nothing selected — one turn into every session.

advertisedModelId now emits the account's own id whenever the model's provider is the account's knownProvider, in emitModel, emitModels, and startCatalog. Stripping the prefix client-side is not an option: workers-ai/@cf/moonshotai/kimi-k2.6 contains slashes of its own, so the UI cannot tell which segment is the provider.

Inbound resolution accepts both forms — bare ids via the account branch, already-persisted qualified pins via CODE-610's unwrap branch — so old model pins still resume.

The four pi-model-registry.test.ts cases asserted the qualified form; their assertions now match the bare id their own test names already state (resolves 'openrouter' input 'openrouter/anthropic/claude-sonnet-4.6' to 'anthropic/claude-sonnet-4.6').

Verification

  • pnpm check:ci clean
  • pnpm test — 2,987 passed, 1 skipped. 3 pre-existing failures in packages/foundation/common/src/node/__tests__/release-artifact.test.ts, which fail identically on 78c2f584 without these commits: on macOS os.tmpdir() resolves through the /var/private/var symlink, so relative() reports the isolated root as escaped. Untouched here.
  • Drove a real Pi session on a cloudflare-gateway account for each failure above and re-checked it after the fix.

Not fixed here

Pi and OpenCode use different id vocabularies for the same service — Pi wants bare claude-opus-4-8, OpenCode wants anthropic/claude-opus-4.8 — but Account.models is service-level, so one list cannot satisfy both. providers/AGENTS.md states the assumption that ids are identical whichever protocol shape an agent resolves to; that holds across one agent's variants, not across agents. Worth its own issue.

Comment thread packages/host/agent-adapter/src/native/pi/adapter.ts

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Reviewed changes since 856065e — one new commit, c28f531, which does two things: fixes the model-update reflection desync I raised last round, and adds per-leg Cloudflare gateway routing by seeding an in-memory AuthStorage instead of injecting a flat baseUrl.

Important

The gateway approach is right and I verified it against the installed SDK end-to-end. But the seeded path is reachable by no test in the repo, and it changes credential injection for the non-seeded provider in a way that regresses a behavior this PR's own test suite pins.

The seeded path is verified-by-reading, not verified-by-running

I checked the pi contract myself rather than trusting the design note, and it holds:

  • AuthStorage.inMemory(data?: AuthStorageData) exists (dist/core/auth-storage.d.ts) and ApiKeyCredential carries env.
  • getProviderEnv reads this.data; setRuntimeApiKey writes this.runtimeOverrides; getApiKey prefers the override. They are genuinely separate maps.
  • cloudflare-auth.js resolveValue reads credential.env?.[name] when a stored credential exists.
  • The real model table (pi-ai/dist/providers/cloudflare-ai-gateway.models.js) templates per-leg URLs off {CLOUDFLARE_ACCOUNT_ID} / {CLOUDFLARE_GATEWAY_ID} — the exact two names catalog.ts now declares. Anthropic-family models get /anthropic, OpenAI-family /openai, the rest /compat.

So the mechanism is sound. What I could not do is run it, because nothing exercises it:

surface providerEnv coverage
providers/src/__tests__/resolve.test.ts none — no endpointEnv, no providerEnvFor, no cloudflare-gateway binding assertion
pi-model.test.ts, pi.test.ts, pi-approval.test.ts, pi-ui-bridge.test.ts none — and all four mock AuthStorage as { create: ... } only, so a seeded start would throw pi.AuthStorage.inMemory is not a function
pi-model-registry.test.ts (real SDK) none — all four cases pass baseUrl + knownProvider, never providerEnv

The blocking human review on this PR reported a live 400 Compatibility endpoint: v1/messages is not supported. A fix for a reported runtime failure that no test can reach is the one place I'd want a test most. pi-model-registry.test.ts already drives the installed SDK against a temp PI_CODING_AGENT_DIR, so the case is cheap to add there — see the inline note.

Scope: only the second of the reporter's two errors is fixed

The reporter hit two failures. This commit addresses the protocol mismatch. The first one — custom-subscription/claude-sonnet-4-6 not existing in pi's table — is still unfixed, and correctly so: it needs registerProvider with models, which both AGENTS.md files now document as out of scope and explain why (declaring a model pi already knows destroys its real metadata). Worth saying explicitly in the PR description so the issue isn't closed as fully resolved.

Resolved since last round

✅ The model-update desync is fixed. advertisedModelId emits the account's own id unprefixed when model.provider === accountProvider, which is exactly the vocabulary accountModelOptions builds the picker from, so resolveModel's exact match now hits instead of falling through to the ${id}- prefix path. Start-time emit, emitModels, and the onSetModel reflection all route through it consistently. The previously-open thread on adapter.ts:144 was already resolved.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

}
if (key && provider) authStorage.setRuntimeApiKey(provider, key);
if (cred.baseUrl) {
if (key && provider && !seeded) authStorage.setRuntimeApiKey(provider, key);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The !seeded guard isn't needed, and it regresses a behavior the suite pins.

Not needed: setRuntimeApiKey writes this.runtimeOverrides while getProviderEnv reads this.data (pi-coding-agent/dist/core/auth-storage.js). A runtime key cannot clobber the seeded credential's env, so there is nothing to protect against.

Regresses: provider is not always cred.knownProvider. It resolves as ref?.provider ?? fallbackProvider ?? cred.knownProvider ?? …, so a model ref that parses to a different provider, or a model-less resume where fallbackProvider (the session's last-routed provider) wins, both leave provider !== cred.knownProvider while seeded is truthy. In that case this line and the registerProvider below are both skipped, so the provider pi actually routes through gets neither a key nor a base URL — it silently falls back to whatever is in ~/.pi/agent/auth.json, or 401s.

That tier is deliberate and tested: pi-model.test.ts "injects the credential under the provider the session last routed through" asserts exactly it. The test passes only because it supplies no providerEnv, so it takes the unseeded branch.

Dropping the guard here is safe. The !seeded condition on the registerProvider block below is the one that carries real meaning (seeded providers template their own URL), and it can stay — though it's worth deciding what should happen when a seeded credential's provider isn't the one being routed to.

}

/** The variant's declared env names carrying this account's endpoint params, for agents that
* template their own per-model URL. Only the pinned-endpoint path skips it: a URL the user typed

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Only the pinned-endpoint path skips it: a URL the user typed outranks whatever the agent's own catalog would build.

The carve-out is reasonable in principle, but pinnedEndpoint() keeps more than URLs a user typed — and the accounts it over-keeps are the ones this PR is trying to fix.

A pre-variant Cloudflare account has a filled endpoint (…/8f3a/prod/compat) written by the old add flow. It never equals the templated variant baseUrl, so pinnedEndpoint returns it, resolveBinding takes the early bind(kind, protocol, baseUrl, knownProvider) branch at line 62 — still four arguments, no providerEnv — and the adapter falls through to the flat registerProvider(baseUrl). That pins every model to /compat, which is precisely the 400 Compatibility endpoint: v1/messages is not supported the reporter filed.

__tests__/resolve.test.ts:287 pins this shape with a cloudflare-gateway account and the comment "the pre-variant behavior, which was never broken for these accounts". That claim was true before this PR; it isn't now — pi bound to such an account still can't reach the anthropic leg.

Worth confirming how many real accounts are in that state. The current add flow doesn't write endpoint (per add-flow.test.tsx), so this is bounded to accounts created before the variant work — but there's no migration, and the only user-facing recovery is delete-and-re-add, which nothing tells them to do. Options: backfill providerEnv when the pinned URL's origin+params match a templated variant, or narrow pinnedEndpoint so a filled-template match against the catalog isn't treated as user-authored.

kind: 'pi',
cwd: root,
model: inputModel,
config: { authToken: 'dummy', baseUrl, knownProvider: provider },

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This file drives the installed SDK, so it's the one place the seeded path can actually be proven — the four unit-test mocks stub AuthStorage as { create } only and would throw on inMemory.

A fifth case here would cover it end-to-end: knownProvider: 'cloudflare-ai-gateway', providerEnv: { CLOUDFLARE_ACCOUNT_ID: '8f3a', CLOUDFLARE_GATEWAY_ID: 'prod' }, no baseUrl, and an anthropic-family model id from pi's own table. Asserting the resolved model's baseUrl ends in /anthropic rather than /compat is the assertion that would have caught the reported bug, and it also proves the two env names in catalog.ts still match pi's templates if the SDK ever renames them.

A companion case in providers/src/__tests__/resolve.test.ts asserting resolveBinding(cloudflareAccount, 'pi') returns providerEnv and no baseUrl would cover the other half.

Copilot AI review requested due to automatic review settings August 24, 2026 13:36

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@lucas77778
lucas77778 requested a review from xiaoland August 24, 2026 13:39

Copy link
Copy Markdown
Member Author

@xiaoland you can re-review now, the models supported by pi agent 0.80.6 is shown below

Screenshot 2026-08-24 at 21.43.07.png

Screenshot 2026-08-24 at 21.43.00.png

Screenshot 2026-08-24 at 21.42.47.png

Copy link
Copy Markdown
Member Author

the pi adapter will be re-contributed to fit pi 0.84.2, but not in this PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants