feat(inference): provider connections — auth-first refactor#30162
Conversation
Creates the provider_connections SQLite table with an index on provider.
Seeds four canonical connections on every boot (INSERT OR IGNORE):
anthropic-managed auth={type:platform}
openai-managed auth={type:platform}
gemini-managed auth={type:platform}
ollama-local auth={type:none}
Wires migration 243 into db-init.ts.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds providerConnections Drizzle table definition mirroring the SQL schema from migration 243. Adds AuthSchema discriminated union (api_key | platform | none for v1; oauth_subscription | service_account accepted by schema but runtime-rejected for v2). Adds ProviderConnection + ResolvedAuth types. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Implements listConnections, getConnection, createConnection, updateConnection, deleteConnection on top of the providerConnections Drizzle schema. createConnection validates the provider against the code-defined enum and parses auth through AuthSchema. deleteConnection rejects when profiles reference the connection (overrideable with force). seedCanonicalConnections ensures the four canonical connections exist; safe to call on every boot (idempotent via presence check). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
ProfileEntry gains an optional provider_connection field. When set, the dispatcher resolves auth from the named connection instead of the global services.inference.mode toggle. Existing provider + source fields remain as deprecated-optional fallbacks so un-backfilled profiles still load. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
resolve-auth.ts: resolves an Auth config to a ResolvedAuth for a given provider. api_key reads from the credential vault; platform builds the managed proxy URL and fetches the platform API key; none passes through with no headers. oauth_subscription and service_account return a clear 'not_implemented (v2)' error. adapter-factory.ts: createAdapterFromConnection creates a Provider adapter from a ProviderConnection + ResolvedAuth. Adapters are pure functions of (resolved auth, request) — no vault access inside the adapter. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds resolveProviderFromConnection(connection, config) to registry.ts. Results are cached in connectionProviders (per-init-providers lifetime) to avoid redundant vault reads. Cache is cleared by initializeProviders. The existing global provider registry is preserved for backward compat: profiles without provider_connection still use services.inference.mode routing. Profiles with provider_connection use the new per-connection path, enabling mix-and-match (managed + your-own simultaneously per profile). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
runProviderConnectionsBackfill runs on every boot (idempotent):
1. Seeds canonical connections.
2. Walks llm.profiles.* in config.json; for each profile without
provider_connection, derives one from source + provider:
source=managed + managed provider → <provider>-managed
source=your-own (or default) → <provider>-personal
(creates connection if missing,
auth=api_key referencing
credential/<provider>/api_key)
provider=ollama → ollama-local
Vault credential name for your-own connections follows the existing
credentialKey(provider, 'api_key') convention (Open Question §10.1 answer).
Errors are caught and logged; backfill will retry on next boot.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds 'assistant inference connections' subcommand group with: list [--json] [--provider <p>] get <name> [--json] create <name> --provider <p> --auth <type> [--credential <vault-key>] [--json] update <name> --auth <type> [--credential <vault-key>] [--json] delete <name> [--force] [--json] Validation mirrors design §8.2: api_key requires --credential; platform and none reject --credential; delete rejects when profiles reference the connection (--force bypasses). All commands support --json output. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…atch
Tests cover:
- Migration creates table and seeds canonical connections (idempotency)
- Connection CRUD: create (happy path, dup name, invalid provider),
get (not found), list (provider filter), update, delete (with/without
--force, reference enforcement)
- AuthSchema: all variants parse correctly; api_key requires credential
- Mix-and-match: two connections for the same provider (anthropic-managed +
anthropic-personal) are distinct and carry different auth configs
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ections
- Rewrote inference.test.ts to use in-memory SQLite (new Database(":memory:") +
drizzle()) instead of the global initializeDb() singleton, matching the
bookmark-crud.test.ts isolation pattern
- Fixed duplicate getLogger import in registry.ts
- Added ConnectionProviderSchema validation on DB read in listConnections and
getConnection so provider field types as the literal union (not string)
- Refactored updateConnection to call getConnection() instead of raw select,
eliminating the provider string → ConnectionProvider type mismatch
- Added narrowing guard before result.error.count assertion in test
- Fixed import sort order violations (eslint --fix)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| export function attachConnectionsSubcommand(inference: Command): void { | ||
| const connections = inference | ||
| .command("connections") | ||
| .description("Manage provider connections (auth configs for inference)"); | ||
|
|
||
| connections.addHelpText( | ||
| "after", | ||
| ` | ||
| Provider connections map a name to a (provider, auth) pair. | ||
| Profiles reference connections via the 'provider_connection' field. | ||
|
|
||
| Canonical connections (seeded on every boot): | ||
| anthropic-managed → provider=anthropic, auth=platform | ||
| openai-managed → provider=openai, auth=platform | ||
| gemini-managed → provider=gemini, auth=platform | ||
| ollama-local → provider=ollama, auth=none | ||
|
|
||
| Examples: | ||
| $ assistant inference connections list | ||
| $ assistant inference connections get anthropic-managed | ||
| $ assistant inference connections create anthropic-personal \\ | ||
| --provider anthropic --auth api_key --credential credential/anthropic/api_key | ||
| $ assistant inference connections update anthropic-personal --auth platform | ||
| $ assistant inference connections delete anthropic-personal`, | ||
| ); | ||
|
|
||
| attachListSubcommand(connections); | ||
| attachGetSubcommand(connections); | ||
| attachCreateSubcommand(connections); | ||
| attachUpdateSubcommand(connections); | ||
| attachDeleteSubcommand(connections); | ||
| } |
There was a problem hiding this comment.
🔴 New inference connections CLI commands not registered in gateway bash risk registry
The assistant/src/cli/AGENTS.md rule states: "When adding/removing/renaming assistant CLI commands or subcommands, update the gateway bash risk registry coverage in gateway/src/risk/command-registry/commands/assistant.ts (supported command paths + risk overrides) so permission prompts stay correct." The PR adds 6 new command paths (inference connections, inference connections list, inference connections get, inference connections create, inference connections update, inference connections delete) but none are registered in gateway/src/risk/command-registry/commands/assistant.ts. The existing inference, inference send, and inference session * paths are registered there (lines 130–135), but the new connections subtree is missing. This means the guardian/risk system won't recognize these commands, leading to incorrect permission prompts when the assistant tries to run them.
Prompt for agents
The new inference connections CLI commands must be added to the gateway bash risk registry at gateway/src/risk/command-registry/commands/assistant.ts. Add the following command paths to the supported commands array (near lines 130-135 where other inference paths are listed): "inference connections", "inference connections list", "inference connections get", "inference connections create", "inference connections update", "inference connections delete". Also add risk overrides: list and get are read-only (low risk), create/update/delete modify state (medium risk). Follow the existing patterns for inference session commands at lines 407-421.
Was this helpful? React with 👍 or 👎 to provide feedback.
| /** Clear per-connection provider cache (called by initializeProviders on boot). */ | ||
| export function clearConnectionProviderCache(): void { | ||
| connectionProviders.clear(); | ||
| } |
There was a problem hiding this comment.
🚩 clearConnectionProviderCache is exported but never called
The function clearConnectionProviderCache at assistant/src/providers/registry.ts:336-338 is exported but never imported or called anywhere in the codebase. The cache it would clear (connectionProviders) is already cleared inline at assistant/src/providers/registry.ts:145 inside initializeProviders(). This appears to be dead code added preemptively for future callers. Per the project's "Dead Code Removal" rule in AGENTS.md, unused code should not be committed — but since this is a new public API being introduced alongside a new feature, it could be argued as intentional forward-looking API surface. Worth a reviewer decision.
Was this helpful? React with 👍 or 👎 to provide feedback.
| export async function resolveProviderFromConnection( | ||
| connection: ProviderConnection, | ||
| config: ProvidersConfig, | ||
| ): Promise<Provider | null> { | ||
| const cached = connectionProviders.get(connection.name); | ||
| if (cached) return cached; | ||
|
|
||
| const authResult = await resolveAuth(connection.auth, connection.provider); | ||
| if (!authResult.ok) { | ||
| const err = authResult.error; | ||
| if (err.code === "not_implemented") { | ||
| log.warn( | ||
| { connectionName: connection.name, authType: err.authType }, | ||
| `Auth type '${err.authType}' is not yet implemented (v2). ` + | ||
| "Update the connection to use 'api_key', 'platform', or 'none'.", | ||
| ); | ||
| } else if (err.code === "credential_not_found") { | ||
| log.warn( | ||
| { connectionName: connection.name, credential: err.credential }, | ||
| `Credential '${err.credential}' not found in vault for connection '${connection.name}'.`, | ||
| ); | ||
| } else { | ||
| log.warn( | ||
| { connectionName: connection.name }, | ||
| `Platform auth unavailable for connection '${connection.name}'.`, | ||
| ); | ||
| } | ||
| return null; | ||
| } | ||
|
|
||
| const streamTimeoutMs = | ||
| (config.timeouts?.providerStreamTimeoutSec ?? 1800) * 1000; | ||
| const useNativeWebSearch = | ||
| config.services["web-search"].provider === "inference-provider-native"; | ||
| const model = resolveModel(config, connection.provider); | ||
|
|
||
| const provider = createAdapterFromConnection(connection, authResult.resolved, { | ||
| model, | ||
| streamTimeoutMs, | ||
| useNativeWebSearch, | ||
| }); | ||
|
|
||
| if (provider) { | ||
| connectionProviders.set(connection.name, provider); | ||
| } | ||
|
|
||
| return provider; | ||
| } |
There was a problem hiding this comment.
🚩 Connection provider cache won't invalidate on CRUD updates
The connectionProviders cache at assistant/src/providers/registry.ts:32 is populated by resolveProviderFromConnection (line 290-291) and only cleared when initializeProviders runs (line 145). If a connection's auth is updated or the connection is deleted via the CLI CRUD commands, the cached provider instance will continue serving the old auth configuration until the next daemon boot. Currently resolveProviderFromConnection has no callers, so this is not an active bug, but once it's wired into the dispatch path, stale cache entries will cause updated connections to silently use old credentials.
Was this helpful? React with 👍 or 👎 to provide feedback.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0734f05a59
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const source = profile.source as string | undefined; | ||
| const inferenceMode = (raw.services as Record<string, unknown> | undefined) | ||
| ?.inference as Record<string, unknown> | undefined; | ||
| const globalMode = (inferenceMode?.mode as string | undefined) ?? "your-own"; | ||
|
|
||
| const effectiveSource = source ?? globalMode; |
There was a problem hiding this comment.
runProviderConnectionsBackfill() reads profile.source to decide between managed and personal connections, but this field is profile ownership metadata (managed/user), not legacy inference auth mode. In managed environments, user-owned profiles (source: "user") will be backfilled to ${provider}-personal and start requiring local API keys, which is a behavior regression from the prior global services.inference.mode routing.
Useful? React with 👍 / 👎.
| export async function resolveProviderFromConnection( | ||
| connection: ProviderConnection, | ||
| config: ProvidersConfig, | ||
| ): Promise<Provider | null> { |
There was a problem hiding this comment.
This resolver is added but never used by provider selection, while inference dispatch still resolves providers by name through the legacy registry path. Because there are no callers of resolveProviderFromConnection, provider_connection rows and the new backfill do not actually influence runtime routing, so mix-and-match auth per profile is not functional.
Useful? React with 👍 / 👎.
| attachSendSubcommand(inference); | ||
| attachSessionSubcommand(inference); | ||
| attachConnectionsSubcommand(inference); |
There was a problem hiding this comment.
Adding assistant inference connections here without updating gateway/src/risk/command-registry/commands/assistant.ts violates assistant/src/cli/AGENTS.md and leaves the new paths unmodeled. The bash risk classifier falls back to the parent command risk when a subcommand is unknown, so mutating operations like create, update, and delete can be under-classified in permission prompts.
Useful? React with 👍 / 👎.
CI: 3 test files green again. Mechanical fixes for Codex P1 + Devin findings. Dispatcher wiring (Codex P1 #2) deferred to PR-2; this PR is purely additive (schema + backfill + CRUD). - credential-security-invariants: allowlist resolve-auth.ts (mirrors registry.ts trust boundary; api_key path reads vault to inject bearer token, same as existing per-provider clients did). - inference-send.test + backfill-jobs.test: expand config/loader.ts mocks to cover getConfig/getConfigReadOnly/loadRawConfig/saveRawConfig. My new code paths now reach previously-unmocked exports. - inference-connections.ts: getConfig -> getConfigReadOnly for the delete-references lookup. Display-side path; read-only is correct. - backfill.ts: route on services.inference.mode (auth axis) only. Drop profile.source conflation. profile.source is ownership metadata (managed/user, system-vs-user-created), not legacy auth mode. The prior code regressed user-owned profiles in managed deployments to require local API keys. - gateway risk registry: add 6 paths (inference connections + 5 subcommands) with risk overrides. list/get low (read-only), create/update/delete medium. Resolves AGENTS.md violation. - connections.ts CRUD: clearConnectionProviderCache() after every insert/update/delete. Resolves Devin orphan-export + cache-staleness findings together. Tests: 82/82 across credential-security-invariants, inference-send, backfill-jobs, providers/inference. tsc + lint clean on assistant + gateway.
Cycle 1 fix push — commit
|
|
@codex review |
|
@devin review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ad56f81552
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const inferenceMode = (raw.services as Record<string, unknown> | undefined) | ||
| ?.inference as Record<string, unknown> | undefined; | ||
| const globalMode = (inferenceMode?.mode as string | undefined) ?? "your-own"; |
There was a problem hiding this comment.
This backfill reads services.inference.mode from loadRawConfig() and defaults missing values to "your-own", but platform deployments often rely on loadConfig()'s IS_PLATFORM context defaults to treat missing mode as managed. In that environment, profiles with no explicit mode on disk will be backfilled to *-personal connections and persisted that way, which bakes incorrect auth routing into config for later connection-based dispatch. The mode inference here should use the same effective config semantics as runtime (including deployment defaults) before deciding managed vs personal connections.
Useful? React with 👍 / 👎.
| } else if (opts.auth === "platform") { | ||
| authInput = { type: "platform" }; | ||
| } else if (opts.auth === "none") { | ||
| authInput = { type: "none" }; |
There was a problem hiding this comment.
The update command accepts --credential for all auth types but only enforces it for api_key; when --auth platform or --auth none is used, any provided credential is silently ignored. This makes typos and automation mistakes dangerous (for example, intending api_key but passing platform) because the command reports success while discarding the credential input and changing auth mode. Mirror the create command's validation by erroring when --credential is supplied with non-key auth types.
Useful? React with 👍 / 👎.
…g/applyNestedDefaults
Cycle 2: cycle-1 fix push surfaced two regressions in inference-send.test.ts.
Root cause: my cycle-1 fix-6 added clearConnectionProviderCache import
from registry.ts to connections.ts. registry.ts pulls in a chain that
calls loadConfig() at module-init time, which inference-send.test.ts's
narrower mock didn't expose. (Standalone bun test invocations don't
share mock state across files, so the test passed in batched runs but
failed in CI's per-file invocation.)
Fix:
- Add loadConfig + applyNestedDefaults to the mock so the import chain
resolves cleanly under all invocation patterns.
- Restore the lazy-factory pattern for the profile-config getters: the
cycle-1 attempt captured stubProfileConfig in a const, breaking the
test that reassigns mockProfileCatalog = {} mid-suite (it expected
the next config getter call to see the empty profiles).
Tests: all 4 affected files pass standalone (credential-security-invariants
32/32, inference-send 20/20, backfill-jobs 10/10, providers/inference 20/20).
tsc + lint clean.
…m default + reject stray credential on update - backfill.ts: lift mode resolution above the per-profile loop and apply the IS_PLATFORM=true → managed default when services.inference.mode is absent on disk. Mirrors getDeploymentContextDefaults() so platform deployments without an explicit mode-on-disk no longer backfill every profile to a *-personal connection. (Codex P1 — cycle-1) - inference-connections.ts: in 'inference connections update', reject --credential when --auth platform or --auth none is supplied. Mirrors create-command validation; previously the credential was silently ignored, masking typos like '--auth platform --credential ...'. (Codex P2 — cycle-1)
Cycle 2 fix push — commit
|
|
Codex Review: Didn't find any major issues. What shall we delve into next? ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
|
@devin review Cycle 2 commit |
|
Thanks for the assist @ichuwar-web ! |
Per review feedback: db-init is for schema/migrations/system rows; the
provider_connections backfill is application-level seeding alongside
seedOAuthProviders. Move it accordingly so future readers don't have
to wonder why one seed lives in db-init and the other in lifecycle.
- assistant/src/memory/db-init.ts: drop the call + import
- assistant/src/daemon/lifecycle.ts: invoke runProviderConnectionsBackfill
immediately after seedOAuthProviders, behind the same dbReady gate and
try/catch shape ('warn + continue startup')
- assistant/src/providers/inference/backfill.ts: tighten JSDoc to make
every-boot idempotent semantics explicit (both seed + walk are cheap
and self-healing — see review thread)
Behavior unchanged.
…nections Per Noa's request: nest the connection-management CLI under a 'providers' namespace so 'inference providers' becomes the home for future provider-scoped admin commands (capabilities, picklists, etc.) alongside 'connections'. - Rename file: inference-connections.ts → inference-providers.ts - Rename function: attachConnectionsSubcommand → attachProvidersSubcommand - Restructure: the function now creates an 'inference providers' parent command and attaches 'connections' to it. All five verbs (list, get, create, update, delete) keep their existing implementations. - Update import + call site in inference.ts. - Update gateway risk-registry: path list + per-command risk overrides now use 'inference providers connections <verb>'. - Update inline help-text examples to the new path. No behavior change beyond the path shift.
|
Pushed two follow-ups (head:
Local: typecheck ✅, lint ✅, provider tests ✅ (20/20), command-registry ✅ (50/50). @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 57880417e2
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| export async function resolveProviderFromConnection( | ||
| connection: ProviderConnection, | ||
| config: ProvidersConfig, | ||
| ): Promise<Provider | null> { |
There was a problem hiding this comment.
The new per-connection resolver is never invoked, so provider_connection settings currently have no runtime effect. Inference still goes through the legacy provider-name registry path (resolveCallSiteConfig(...).provider → getProvider(...)), and a repo-wide check shows no call site for resolveProviderFromConnection. This means assistant inference providers connections create/update/delete and the backfilled llm.profiles.*.provider_connection field do not actually control auth routing, so mixed auth setups (e.g., managed + personal for the same provider) cannot work in production yet.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and intentional. Scope split:
- This PR (feat(inference): provider connections — auth-first refactor #30162) — scaffold: schema, CRUD, CLI, backfill, resolver function.
resolveProviderFromConnectionships as zero-call-site dead code by design — it's the destination for the dispatch wiring. - Cycle-3 follow-up — wiring: thread
profile → provider_connection → resolved auth → bound providerthrough the canonical dispatch path (provider-send-message.ts) and the five satellite sites (rollup-producer,subagent/manager,conversation-store,approval-generators,guardian-action-generators). Includes a real E2E test that exercises two profiles with sameproviderbut differentprovider_connectionthrough dispatch and asserts the bound auth differs.
Until the wiring PR lands, profile.provider_connection is a dormant field — created/listed by the CLI, written by the backfill, read by no production code path. Acknowledged in updated PR description.
Cycle-3 review triage —
|
…le-3 `config-watcher.test.ts` mocks `providers/registry.js` with a stub factory that doesn't include the new exports added by the inference providers refactor: `clearConnectionProviderCache` (added in #30162) and `resolveProviderFromConnection` (used transitively by the new `connection-resolution.ts` module). When the test file is loaded in isolation — or in some test orderings — the import chain through `providers/inference/connections.ts` and `providers/connection-resolution.ts` hits a `SyntaxError: Export named '...' not found`. Adds both names to the mock factory. Verified locally: bun test src/__tests__/config-watcher.test.ts → 19 pass / 0 fail. Pre-existing on main (#30162 added the first import without updating the mock); landing here so CI can run Test green for this PR.
* feat(inference): wire provider_connection into canonical dispatch Cycle-3 wiring PR — addresses Codex P1 from #30162: the `resolveProviderFromConnection` resolver shipped as dead code with zero call sites. This commit wires the canonical inference dispatch path (`resolveConfiguredProvider` in provider-send-message.ts) to actually use the resolver. Behavior: 1. When the resolved profile has `provider_connection`, dispatch routes through `resolveProviderFromConnection(connection, config)`. 2. On any miss (connection not found, resolver returns null, lookup throws), warn and fall through to legacy `getProvider(name)` so misconfiguration doesn't break inference. 3. Profiles without `provider_connection` keep working unchanged via the legacy path. Also: add `provider_connection?: string` to LLMConfigBase. The field already flowed through `resolveCallSiteConfig` at runtime (`profileConfigFragment` strips only `source`/`label`/`description`), so this aligns the type with reality for type-safe access. Gate test: dispatch-connection-routing.test.ts is the test cycle-2 was missing. Four cases: - Two profiles, same provider, different `provider_connection` → resolver called twice, with the right connection each time, with auth bundles distinguishable per profile (mix-and-match goal #2). - Profile WITHOUT `provider_connection` → resolver NOT called, legacy path takes over. - `provider_connection` set but unknown → resolver NOT called, legacy fallback succeeds. - `provider_connection` set, found, but resolver returns null → resolver IS called, legacy fallback succeeds. If wiring regresses, `resolveProviderCalls.length` invariants break the first test. That's the gate cycle-2 lacked — it tested DB shape, not dispatch invocation. * feat(inference): wire provider_connection through satellite providers Cycle-3 follow-up to canonical-path wiring (c3416ac). Migrates the five satellite construction-time call sites — subagent manager, daemon conversation/approval/guardian generators, and rollup producer — to a single shared connection-aware default-provider resolution path. * New `providers/connection-resolution.ts`: - `tryResolveProviderForConnectionName(name, config)` — promoted from private helper in `provider-send-message.ts`. Looks up a `provider_connection` row and resolves a Provider with that connection's auth bound. Logs and returns null on any miss so callers can fall back to the legacy registry path. - `resolveDefaultProvider(config)` — for satellites' construction-time path. Reads `config.llm.default.{provider, provider_connection}`, routes through the connection if named, otherwise legacy lookup. - `resolveProviderForCallSite(callSite, config, opts)` — exported completeness analogue. * `call-site-routing.ts`: - `CallSiteRoutingProvider` gains an optional async `resolveByConnection` ctor param. `selectProvider` is now async; falls through to legacy on miss so existing call sites keep working unchanged. - New `wrapWithCallSiteRouting(base, config)` helper — replaces the per-file wrappers in `approval-generators` and `guardian-action-generators` so all five satellites share one wiring shape. * Satellites migrated (all five): - `subagent/manager.ts` — `resolveDefaultProvider` + `wrapWithCallSiteRouting`. Throws on null default (preserves existing failure mode). - `daemon/conversation-store.ts` — same pattern; throws on null default. - `daemon/approval-generators.ts` — both copy + conversation generators use the shared wrapping; copy returns null on miss, conversation throws. - `daemon/guardian-action-generators.ts` — both generators use the shared wrapper. `getConfiguredProvider` already routes through the canonical path, so only the wrapper changes here. - `home/rollup-producer.ts` — uses `resolveDefaultProvider`. The `RollupProducerDeps.resolveProvider?` injection point is widened to `() => Provider | null | Promise<Provider | null>` for async tests. * `provider-send-message.ts` switches to the shared `tryResolveProviderForConnectionName` and drops the now-redundant local `tryResolveFromConnection` (and its unused `log`/`getLogger` imports). * Hard-gate test: `__tests__/satellite-connection-routing.test.ts`. Four cases on `CallSiteRoutingProvider` directly: 1. Alternate-profile callSite with `provider_connection` → connection-resolution hook fires with the right name+auth and the transport that runs is the connection-bound stub (not default, not legacy registry). 2. Alternate-profile callSite WITHOUT `provider_connection` → connection hook never fires; legacy registry path produces the stub. 3. Alternate-profile callSite with unknown `provider_connection` → falls through; system stays operational on default. 4. No callSite → straight to default; no hook, no registry lookup. Combined with the cycle-3 first-commit dispatch tests, that's 8 gate tests across both code paths (canonical `provider-send-message` resolution + satellite `CallSiteRoutingProvider` wrapping). Cycle-2 + cycle-3-first-commit shipped `resolveProviderFromConnection` as dead code (zero call sites). This wiring removes that gap. Cycle-4 will strip the legacy fallback paths once we've shipped one release window with connection-awareness active. Verified: - typecheck clean (assistant + gateway). - lint clean on touched files. - `bun test src/providers/__tests__/` → 102 pass / 0 fail / 206 expects. * test(config-watcher): mock new registry exports added in #30162 / cycle-3 `config-watcher.test.ts` mocks `providers/registry.js` with a stub factory that doesn't include the new exports added by the inference providers refactor: `clearConnectionProviderCache` (added in #30162) and `resolveProviderFromConnection` (used transitively by the new `connection-resolution.ts` module). When the test file is loaded in isolation — or in some test orderings — the import chain through `providers/inference/connections.ts` and `providers/connection-resolution.ts` hits a `SyntaxError: Export named '...' not found`. Adds both names to the mock factory. Verified locally: bun test src/__tests__/config-watcher.test.ts → 19 pass / 0 fail. Pre-existing on main (#30162 added the first import without updating the mock); landing here so CI can run Test green for this PR. * fix(inference): address review feedback on provider_connection wiring Four review findings on PR #30198 commits c3416ac + 8b335eb: * **[Codex P1]** `provider-send-message.ts:127` — provider/connection mismatch silent misroute. A profile that names `provider: "openai"` together with an Anthropic-flavored `provider_connection` would silently dispatch traffic to the connection's backend (Anthropic) while metadata still reported the resolved profile (OpenAI). Now we verify `connection.provider === expectedProvider` and fall through to the legacy registry path (with a warn log) on mismatch instead of silently misrouting. `tryResolveProviderForConnectionName` gains an optional `expectedProvider` parameter; canonical dispatch passes `resolved.provider`, `resolveDefaultProvider` passes `profile.provider`, and `CallSiteRoutingProvider`'s async hook signature is widened to `(connectionName, expectedProvider)` so the shared wrapper threads the value through. * **[Codex P2 / Devin BUG #1]** `approval-generators.ts:141` — stale `listProviders().includes(...)` guard blocks connection-aware resolution. In `createApprovalConversationGenerator`, the legacy guard threw "No provider available" before the new `resolveDefaultProvider(config)` could resolve from connection auth. In `your-own` configurations where the default provider lives entirely behind a `provider_connection` and never appears in the legacy `listProviders()` list, this caused false failures. Removed the guard; `resolveDefaultProvider` already returns null on miss and we throw on null (right "no provider available" signal). `listProviders` import dropped along with it. * **[Devin BUG #2]** Dead code — `resolveProviderForCallSite` in `connection-resolution.ts` was exported for "completeness" but never imported or called anywhere. Deleted (and its now-unused imports of `resolveCallSiteConfig` + `LLMCallSite`). * **[Devin BUG #3]** JSDoc for `wrapWithCallSiteRouting` referenced helpers "that lived in approval-generators.ts and guardian-action-generators.ts" — those were removed in this PR, so the comment described history rather than current behavior. Rewrote to describe what the function does, not what it replaced. New gate test in `satellite-connection-routing.test.ts`: - profile says provider=openai, provider_connection points at an anthropic-flavored row → connection-resolution hook returns null on mismatch → legacy `getProvider("openai")` path produces the OpenAI stub. Without this fix the connection's anthropic stub would have run instead. Verified: - `bunx tsc --noEmit` → clean. - `bun test src/providers/__tests__/` → 103 pass / 0 fail / 209 expects (was 102/0/206; +1 test, +3 expects). * fix(inference): catch resolveProviderFromConnection rejections, fall through to legacy Codex P1 follow-up on PR #30198 (#discussion_r3215235367): `tryResolveProviderForConnectionName` returned the `resolveProviderFromConnection(connection, config)` promise directly, so any rejection from the inner async path — credential read failure in `resolveAuth`, managed-proxy context lookup throwing, transient provider-impl construction errors — bubbled out of the helper and hard-failed dispatch instead of falling through to the legacy `getProvider(resolved.provider)` path the helper's contract advertises. Wrap in try/catch; log a warn with `{ err, connectionName }` and return null so `getConfiguredProvider` and `CallSiteRoutingProvider` can serve the request through the legacy registry. The behavior now matches the lookup-failure and not-found branches above. New gate test in satellite-connection-routing.test.ts: a connection whose `resolveProviderFromConnection` throws does not fail dispatch. The wrapper catches the rejection, logs, and falls through; the profile's resolved provider matches default → reused default instance. Test infrastructure adds a `connectionsThatThrowOnResolve` Set so individual tests can opt into the throwing path without affecting the other cases. Verified: - `bunx tsc --noEmit` → clean. - `bun test src/providers/__tests__/satellite-connection-routing.test.ts` → 6 pass / 0 fail / 23 expects (was 5/0/19; +1 test, +4 expects). --------- Co-authored-by: credence-the-bot[bot] <test@test.com> Co-authored-by: Credence <credence@vellum.ai>
3 participants
Context
Replaces closed PR #30153 (v0). Sidd pushed back on the data-driven providers premise: providers need code anyway because of edge cases (OpenRouter-style "almost-OpenAI" providers force code changes regardless of how thin the schema looks). v1 pivots to an auth-first refactor: one SQLite table that names (provider, auth-config) tuples, with provider implementations staying in code.
Design doc was reviewed and approved by Sidd before this PR.
Scope (this PR)
This is PR-1 of a series. Purely additive — runtime routing is unchanged.
What ships now:
provider_connectionsSQLite table (migration243-provider-connections.ts). Columns:namePK,provider,authJSON, timestamps.provider_connection: string.llm.profiles.*, infers connections fromservices.inference.mode, creates rows, rewrites profiles. Seeds canonical connections (anthropic-managed,openai-managed,gemini-managed,ollama-local) on every boot.createConnection,updateConnection,deleteConnection) with cache invalidation hooks into the new per-connection adapter cache.assistant inference providers connections {list,get,create,update,delete}.resolveAuth) + adapter factory (createAdapterFromConnection) + connection→provider resolver (resolveProviderFromConnection). All wired internally; not yet called by the dispatch path — that's PR-2.Goals locked across the series:
api_key,platform,none. Schema accommodatesoauth_subscription,service_accountfor later (rejected at runtime).(ResolvedAuth, request) → response. (Adapter factory ships here; dispatcher refactor in PR-2.)Out of scope (deferred to follow-up PRs)
resolveProviderFromConnectionintogetConfiguredProviderso profiles withprovider_connectionroute through the new path. Until then,provider_connectionsrows + the new backfill are inert at runtime — they exist on disk but nothing reads them.model_profilestable — Sidd flagged "arbitrary cut" of normalized fields.rate_cardstable — Sidd flagged shape gaps (Anthropic 5m vs 1h cache, OpenAI/Google input-length tiers).Verification
bun test src/providers/__tests__/inference.test.ts src/__tests__/credential-security-invariants.test.ts src/cli/commands/__tests__/inference-send.test.ts src/memory/v2/__tests__/backfill-jobs.test.ts→ 82/82 pass after cycle-1 fixbunx tsc --noEmit→ clean (assistant + gateway)bun run lint→ clean (assistant + gateway)Cycle 1 fixes (commit
ad56f8155)Addresses Codex (3 P1) + Devin (3 surfaced) review:
resolve-auth.tsnot allowlisted in credential-security-invariantsregistry.ts(existing entry)getConfig/saveRawConfigmock missing in two testsinference-connections.tsswitched togetConfigReadOnly(correct for read-only delete-reference lookup)profile.source(ownership) withservices.inference.mode(auth)gateway/src/risk/command-registry/commands/assistant.tswith risk overrides (list/get low; create/update/delete medium)clearConnectionProviderCacheorphan + cache won't invalidate on CRUDconnections.ts. The orphan resolves itself.Diff
15 files this PR + 7 files cycle-1 fix = 16 files net (some overlap), +1586/-17.
Migration safety
SELECT 1 FROM sqlite_master).services.inference.modereads still honored for one release with a deprecation warning.Notes for review
provider_connectionsrows describe auth-config instances, not provider definitions. Provider impls stay in code (closed set).<provider>-<modifier>) is suggested in CLI help but not enforced.