feat(vault): provider connections carry models, harnesses, and stable slugs - #5995
Conversation
Standard provider records gain optional models and harnesses fields and, on create, a stable slug plus a computed display name (OpenAI, OpenAI 2) when the header has no name. Custom provider records gain harnesses and the same slug-on-create treatment so a rename can no longer strand agent configs. The agent connection resolver selects standard and custom records by stored slug with the legacy fallbacks kept. Updates preserve saved models and harnesses when the payload omits them; an explicit empty list still clears. Per-provider default model lists ship in the harness catalog capabilities as default_models, expressed per harness in its own model spelling. The web secret entity round-trips the new fields without UI changes, and the interface docs are updated in the same change.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (157)
Disabled knowledge base sources:
📝 WalkthroughSummary by CodeRabbit
WalkthroughProvider connections now support stable names, slugs, model and harness policies, credential probing, subscription status, and connection-aware model selection. The API, SDK, runner, and web application add persistence, resolution, validation, and management flows. ChangesProvider platform and connection lifecycle
Estimated code review effort: 5 (Critical) | ~120 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| return f"{title} {index}" | ||
|
|
||
|
|
||
| def _carry_over_saved_policy(*, stored_data: Any, update_data: Any) -> None: |
There was a problem hiding this comment.
Decision: absent means unchanged, [] means clear. Without this, any key rotation (or the 5-minute-stale frontend cache) wiped saved models/harnesses because the backend replaces the data blob wholesale on PUT. Cost: one extra get_by_id per secret update; kept unconditional to keep kind knowledge out of the update flow.
| from oss.src.core.secrets.dtos import CreateSecretDTO, UpdateSecretDTO | ||
|
|
||
|
|
||
| def next_provider_key_name( |
There was a problem hiding this comment.
The naming rule from the plan: first unnamed connection gets the provider display name, later ones a number. Two concurrent creates can compute the same display name; accepted because the slug, not the name, is identity.
| # connection (design: provider-connections-models/provider-discovery.md, "Default models"). The | ||
| # ids are the shared catalog spelling (``provider/id``); each harness republishes them below in | ||
| # the spelling it accepts. A saved list on a connection — including an empty one — always wins. | ||
| PROVIDER_DEFAULT_MODELS: Dict[str, List[str]] = { |
There was a problem hiding this comment.
Default-model lists from the reviewed plan (founder-corrected on PR 5987): GPT-5.6 Luna/Terra/Sol, Fable 5, the usage-ranked OpenRouter set with GLM-5.2 and DeepSeek V4. claude-opus-5 is intentionally absent until the pinned Pi catalog refresh (sync-model-catalog); every identifier verified against pi_models.generated.json. Each harness republishes in its own spelling; Claude tier aliases are prefix-mapped.
| raise ValueError( | ||
| "The provided kind in data is not a valid StandardProviderKind enum" | ||
| ) | ||
| # Both provider shapes now accept {kind, provider, models}, so the union can no |
There was a problem hiding this comment.
Decision: the secret kind now explicitly picks the DTO shape. StandardProviderKind and CustomProviderKind overlap on fourteen values, and models used to be the accidental tiebreaker; adding models to the standard shape made pydantic's smart union misclassify URL-less custom providers and silently drop provider_slug and per-model extras. Reviewer verified malformed payloads still 422 and the nested error cannot leak the submitted key.
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
web/packages/agenta-entities/tests/unit/secret-transforms.test.ts (1)
92-101: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd explicit-empty policy payload tests.
Add assertions that
models: []andharnesses: []remain present in the standard-provider payload. Add an assertion thatharnesses: []remains present in the custom-provider payload. These cases enforce the API contract that explicit empty lists clear saved policies.Also applies to: 122-138
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 3d7cee8d-101e-476e-b825-9f2b08b29fd5
📒 Files selected for processing (21)
api/oss/src/core/secrets/dtos.pyapi/oss/src/core/secrets/enums.pyapi/oss/src/core/secrets/services.pyapi/oss/tests/legacy/vault_router/test_vault_secrets_apis.pyapi/oss/tests/pytest/unit/secrets/test_dtos.pyapi/oss/tests/pytest/unit/secrets/test_services.pydocs/design/agent-workflows/interfaces/cross-service/service-to-vault-and-tool-providers.mddocs/design/agent-workflows/interfaces/in-service/model-connection-resolution.mddocs/design/agent-workflows/interfaces/public-edge/workflow-inspect.mdsdks/python/agenta/sdk/agents/capabilities.pysdks/python/agenta/sdk/agents/platform/connections.pysdks/python/oss/tests/pytest/unit/agents/connections/test_model_catalog.pysdks/python/oss/tests/pytest/unit/agents/platform/test_connections_http.pyweb/oss/src/components/ModelRegistry/Modals/ConfigureProviderModal/index.tsxweb/packages/agenta-entities/src/secret/core/index.tsweb/packages/agenta-entities/src/secret/core/transforms.tsweb/packages/agenta-entities/src/secret/core/types.tsweb/packages/agenta-entities/src/secret/state/atoms.tsweb/packages/agenta-entities/src/workflow/state/inspectMeta.tsweb/packages/agenta-entities/tests/unit/secret-transforms.test.tsweb/packages/agenta-shared/src/types/llmProvider.ts
Railway Preview Environment
|
|
One actionable issue found: [P2] Allow
Please make |
|
Review findings:
|
POST /providers/probe validates a credential and discovers models in one action while keeping the two results separate: a public catalog can refresh models but can never produce a key-valid claim, and a missing list endpoint is unsupported, not invalid. Adapters cover the standard API-key providers, OpenRouter's authenticated key check, Azure and Bedrock bearer paths, and OpenAI-compatible endpoints behind the shared egress guard with DNS pinning. Credentials are never persisted or logged; the vault's SecretSafeRoute keeps them out of 422 bodies.
… card The Settings tab becomes AI providers: a table of connections (provider, masked credential, active models, connected date) whose rows open the connection card directly, plus an Add provider button that opens the catalog drawer. The catalog lists every standard provider except the defunct Aleph Alpha, adds AWS Bedrock, Azure OpenAI, and Google Vertex AI as first-class rows with their own credential field sets, and ends with the OpenAI-compatible endpoint row. The card runs one Test action against the probe API and keeps its two results honest, pre-checks the default models and tags them, always accepts manual model IDs, saves a policy only when the user actually chose one, and never shows a modal or toast. The old Secrets settings surface is deleted; its custom provider form lives on for other consumers.
…onnection SecretsManager resolves a stored connection slug first, of either vault kind, and raises a clean 400 for an unknown slug instead of silently running on another key. Without a slug the old family mapping still applies, extended so a record whose saved model list contains the requested model wins over the bare family match. The vault middleware stops collapsing same-family records, which had made a second OpenAI connection unreachable on every prompt path. The prompt and judge model pickers show one group per named connection, suppress the ambiguous static family group when two or more connections claim it, and persist the slug beside the model in one dispatch. The prompt-side model catalog gains the current identifiers it was missing.
The picker's first level lists the project's connections and the two consumer subscriptions instead of provider families; the flyout lists each connection's models crossed with the harnesses that may drive them, with the harness as a tag and a cost hint when a model is reachable twice. A pick writes model, provider, connection slug, and harness in one dispatch. The drawer's playground context adds the Connected and Subscriptions sections around the shared catalog, and the closed pill goes dashed with Set up AI providers when nothing is connected. Custom connections are matched by their stored slug with a name fallback for legacy records, and the chat composer's /model picker shares the same rows.
One family-to-prefix table, mirrored in TypeScript and Python with cross-referencing drift tests, turns provider-native model ids into the spelling litellm routes. Pick-time translation keeps stored configs in litellm format, so pre-existing prompts are untouched by construction; the resolver keeps an idempotent safety net on both context paths and custom-provider records are structurally exempt, pinned by mutation- checked tests. The saved-models tiebreak compares normalized spellings so an explicit claim beats first-record. Unknown connection slugs fail as a clean 400 naming the candidates, never by running on another key.
From accepted review findings: ProviderCredentials.key becomes SecretStr and extras drops out of repr, so a stray log line or traceback cannot print a key; adapters unwrap at the single point of use and canary tests pin repr, str, and both dump forms. The llm_v0 fallback path stops pre-binding one key per family and resolves each entry through the shared slug-first resolver, so an entry naming a specific connection runs on that connection's key across the whole retry chain.
Three entry contexts with their exact titles and section sets, the catalog as the only scrolling region, folded connected rows with one composed subtitle, subscription rows as one pair per plan and harness derived from the runner's live status, the setup row and empty state verbatim from the spec, the pair card, and the connection card per the handoff: one secret line, green-dot result with fetch count, recommended tags, manual IDs always, drawer-body scrolling with the close control hard right and a truncation guard on long names. Saved policy fields are sent only when the user actually chose them.
One cascade component: search on top, connections left with counts and olive Subscription tags on rows named by plan, flyouts right with curated names, quiet default and cheapest hints, harness sections with logos and a via-header for single-harness connections, the ink harness tooltip, and one Manage model providers footer. The completion context strips every trace of harness. Uncataloged saved ids appear in the harness's own spelling, labelled exactly as the user typed them, and the closed pill resolves the same beautified label as the menu. Subscription rows consume the runner's live pairs with a static fallback, sharing one status query with the drawer. The tooltip link color becomes a real palette token.
The Fallback and Retry titles inherited the 16px body over 12px labels; they join the panel's own scale, and the header aligns with its tabs. One shared function now computes a no-list connection's effective models, so the settings table, the drawer count, and both pickers cannot disagree; the playground model row renders the resolved label instead of the raw stored id, with manual ids shown exactly as typed.
A hand-added model id absent from Pi's registry is registered into the per-run models.json with its base model's real metadata, so it runs as a first-class model; the operator's mounted Pi folder is never written. The Pi probe also names which provider logins the mount holds (OAuth entries only, family names only), the service passes them through a closed allow-list, and subscription pairs derive from the live answer, which is how a ChatGPT login in Pi becomes a visible pair.
The connect gate opens the providers drawer directly, and a connection created from onboarding auto-switches the agent to its first model and commits, leaving the user typing; a second connection added mid-session never touches the current pick. The dead duplicate gate hook is deleted rather than synced. The settings table matches its siblings (Provider and Name columns, Created, an actions menu), Preferences reads Feature flags with a Developer mode toggle describing exactly what it gates, the eval stepper's all-rows hover highlight is fixed at its CSS root, and the dev api gains the graceful-shutdown bound that ends the reload deadlock.
The onboarding auto-switch pulses the config section through the shared draft signal; the new origin keeps it out of the approval dock's Undo.
The runner gains an authenticated GET /subscription-status that inspects the mounted login location per harness (codex, claude, pi_core, pi_agenta) and returns only state words. The Python agent service proxies it at POST /runtime/subscription-status behind the same auth as invoke, mapping network failure to unavailable and an old runner to incompatible. The self-managed credential card shows the status for the selected harness, polls while visible, and offers Check again. The status never claims provider access; a run remains the proof.
…check by provider The fake secrets DAO discarded project_id and organization_id, so every test shared one namespace: provider numbering and saved-policy carry-over could cross a project boundary and the naming test only passed because of it. Records now carry their scope and every read filters on it, with two cases pinning the boundary. The catalog subset check unioned every provider's catalog ids, so a default under one provider could pass on an id that only exists under another. Scoped to the entry's own provider. Also shortens a two-line comment in the configure-provider modal to one, per the repo rule.
… and provider The public status proxy allow-listed state words and the `providers` list, but two runner-controlled strings still passed through untouched: the harness map's KEY, which becomes an object key in the browser's JSON, and the singular `provider`. A future or faulty runner could put an account name, a path, or a credential in either. Both are now closed sets. Keys must name a known harness (`HarnessKind`) and unknown ones are dropped, which also caps the map — a runner cannot push more entries than there are harnesses. `provider` gets the same treatment `providers` already had: a family the card cannot render is dropped rather than failing the entry, since the state word is still good. Both sets already match what the runner sends (its own SUBSCRIPTION_HARNESSES and its constant provider values), so this narrows the contract without changing any real response.
…el spelling Five review findings on the provider-connections surface: - The vault shadow filter compared provider kinds raw, so MISTRALAI_API_KEY survived next to a stored `mistral` connection and — locals coming first in the combined list — then won the resolver's tiebreak, beating the key the user saved. Both sides now normalize through a shared `normalize_provider_kind`, moved to `sdk/utils/providers.py` because the manager already imports the vault middleware and could not be imported back. - The family tiebreak fell through to the first record, which could be one the user narrowed away from the requested model while a later list-less record still offered it. A record with no saved list follows Agenta's defaults, so it now wins over a narrowed one; an explicit `[]` still means "offer nothing" and wins nothing. - The probe applied its 10s budget per httpx request, and OpenRouter issues two, so the worst case behind the button was double. The budget now covers the whole probe; running out of it maps to `unknown` + `failed`, which is what it honestly proved. - The playground picker offered a credential-set connection its bare saved slugs when it carried both spellings, but the SDK matches those connections on `model_keys` only, so the pick resolved to no provider settings. - `together_ai/moonshotai/Kimi-K2-Instruct` is gone from Together's serverless API and from the agent catalog; replaced with `Kimi-K2.6`, which the agent catalog carries. Plus a doc comment that stated the opposite of the save rule, and a catalog call moved inside its own try so an exception degrades to an empty set instead of failing module collection.
feat(web): the AI providers experience - settings, picker, and run-path wiring
feat(agents): runner subscription status on the credential card
Implements pull request 1 of the plan in #5987 (docs: plan provider connection model lists). First
of a stacked series; the settings experience and playground picker build on this contract.
Context
Two OpenAI keys cannot be selected independently today. The resolver identifies a standard key by
its provider family, the frontend matches standard secrets by env-var name, and neither record
type can save model or harness choices. The plan's first slice fixes the contract and the
resolver without changing anything a user sees.
What changed
api/oss/src/core/secrets/dtos.py):StandardProviderDTOgains optionalmodels(list of
{slug}) andharnesses;CustomProviderDTOgains optionalharnesses. Old recordsread with
Nonedefaults; nothing is migrated. Because both shapes now carrymodels, thesecret kind now explicitly decides which DTO validates the data (pydantic smart-union picked the
wrong shape for URL-less custom providers).
services.py): newprovider_keyandcustom_providerrecords get astable slug on create. An unnamed standard connection receives the provider display name:
"OpenAI" first, then "OpenAI 2". The empty-header 422 is kept for every other secret kind.
models/harnessespreserves the storedvalues; an explicit
[]still clears. This closes the read-modify-write hole where rotating akey wiped saved policy. The key-rotation modal also stops resetting a custom display name.
sdks/.../agents/platform/connections.py): standard and custom candidates resolveby stored slug, with the legacy fallbacks (provider family for standard, header name for
custom). The
ResolvedConnectionwire format is unchanged.sdks/.../agents/capabilities.py): per-providerdefault_modelsship inthe harness catalog capabilities, expressed in each harness's own spelling (Pi gets
openai/gpt-5.6-luna, Codexgpt-5.6-luna, Claudesonnet/haiku/claude-fable-5). Everyidentifier is verified against the generated Pi catalog;
claude-opus-5is commented aspending the catalog refresh.
UI change). Interface docs updated in the same change.
Scope / risk
default_modelshas no consumer yet.legacy records without slugs use the old identification paths (covered by tests).
the fields round-trip at runtime, and the local zod schemas carry the types. Regeneration is
deferred until the series stabilizes the contract.
layer; the UI that exposes them ships in the settings slice.
How to QA
Prerequisites: a dev stack from this branch, a project API key.
POST /secrets/twice with{"secret": {"kind": "provider_key", "data": {"kind": "openai", "provider": {"key": "sk-1"}}}}and no header name. Expected: two records named "OpenAI" and "OpenAI 2", each with a distinct stableslug.PUTone of them changing only the key. Expected: name, slug, savedmodels, andharnessessurvive."models": [{"slug": "gpt-5.6-luna"}], "harnesses": ["pi_core"]. Expected: both fields round-trip on GET and list.GET /workflows/catalog/harnesses/and check each harness'scapabilities.default_models.Test commands:
cd api && uv run pytest oss/tests/pytest/unit/secrets/ -q(45 tests)cd sdks/python && uv run pytest ../../sdks/python/oss/tests/pytest/unit/agents/ -q(889 tests)cd web && pnpm --filter @agenta/entities run test:unit(1009 tests)Edge cases in tests: ambiguous union payloads, empty header per kind,
[]-clears versusomitted-preserves, slug selection between two same-provider records, legacy records without slugs,
and the Claude alias spellings.