Skip to content

feat(vault): provider connections carry models, harnesses, and stable slugs - #5995

Merged
mmabrouk merged 19 commits into
release/v0.112.1from
feat/provider-connections-api
Aug 13, 2026
Merged

feat(vault): provider connections carry models, harnesses, and stable slugs#5995
mmabrouk merged 19 commits into
release/v0.112.1from
feat/provider-connections-api

Conversation

@mmabrouk

Copy link
Copy Markdown
Member

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

  • DTOs (api/oss/src/core/secrets/dtos.py): StandardProviderDTO gains optional models
    (list of {slug}) and harnesses; CustomProviderDTO gains optional harnesses. Old records
    read with None defaults; nothing is migrated. Because both shapes now carry models, the
    secret kind now explicitly decides which DTO validates the data (pydantic smart-union picked the
    wrong shape for URL-less custom providers).
  • Identity and naming (services.py): new provider_key and custom_provider records get a
    stable 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.
  • Update semantics: an update payload that omits models/harnesses preserves the stored
    values; an explicit [] still clears. This closes the read-modify-write hole where rotating a
    key wiped saved policy. The key-rotation modal also stops resetting a custom display name.
  • Resolver (sdks/.../agents/platform/connections.py): standard and custom candidates resolve
    by stored slug, with the legacy fallbacks (provider family for standard, header name for
    custom). The ResolvedConnection wire format is unchanged.
  • Default models (sdks/.../agents/capabilities.py): per-provider default_models ship in
    the harness catalog capabilities, expressed in each harness's own spelling (Pi gets
    openai/gpt-5.6-luna, Codex gpt-5.6-luna, Claude sonnet/haiku/claude-fable-5). Every
    identifier is verified against the generated Pi catalog; claude-opus-5 is commented as
    pending the catalog refresh.
  • Web entity: the secret types, transforms, and atoms round-trip the new optional fields (no
    UI change). Interface docs updated in the same change.

Scope / risk

  • No UI behavior change: the settings page and both playgrounds render exactly as before, and
    default_models has no consumer yet.
  • Existing agents resolve as before: provider-only selection still works when unambiguous, and
    legacy records without slugs use the old identification paths (covered by tests).
  • The Fern client is not regenerated yet; the generated client passes bodies/responses through, so
    the fields round-trip at runtime, and the local zod schemas carry the types. Regeneration is
    deferred until the series stabilizes the contract.
  • Two acceptance checks ("two OpenAI keys selected independently") hold at the API and resolver
    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.

  1. 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 stable slug.
  2. PUT one of them changing only the key. Expected: name, slug, saved models, and harnesses survive.
  3. Create a record with "models": [{"slug": "gpt-5.6-luna"}], "harnesses": ["pi_core"]. Expected: both fields round-trip on GET and list.
  4. GET /workflows/catalog/harnesses/ and check each harness's capabilities.default_models.
  5. Open the agent playground and the prompt playground. Expected: model menus unchanged.

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 versus
omitted-preserves, slug selection between two same-provider records, legacy records without slugs,
and the Claude alias spellings.

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.
@dosubot dosubot Bot added the size:XL This PR changes 500-999 lines, ignoring generated files. label Aug 12, 2026
@vercel

vercel Bot commented Aug 12, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
agenta-documentation Error Error Aug 13, 2026 3:48pm

Request Review

@dosubot dosubot Bot added the enhancement New feature or request label Aug 12, 2026
@coderabbitai

coderabbitai Bot commented Aug 12, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: 1753f25e-04de-4343-8e55-645c14a8539c

📥 Commits

Reviewing files that changed from the base of the PR and between e8d4613 and 4da9581.

⛔ Files ignored due to path filters (1)
  • web/oss/src/styles/theme/antd-overrides.generated.ts is excluded by !**/*.generated.*
📒 Files selected for processing (157)
  • api/entrypoints/routers.py
  • api/oss/src/apis/fastapi/providers/__init__.py
  • api/oss/src/apis/fastapi/providers/models.py
  • api/oss/src/apis/fastapi/providers/router.py
  • api/oss/src/core/providers/__init__.py
  • api/oss/src/core/providers/adapters.py
  • api/oss/src/core/providers/dtos.py
  • api/oss/src/core/providers/endpoints.py
  • api/oss/src/core/providers/exceptions.py
  • api/oss/src/core/providers/service.py
  • api/oss/src/resources/evaluators/evaluators.py
  • api/oss/tests/pytest/unit/providers/__init__.py
  • api/oss/tests/pytest/unit/providers/test_provider_probe.py
  • docs/docs/self-host/agents/01-use-your-own-subscription.mdx
  • hosting/docker-compose/ee/docker-compose.dev.yml
  • hosting/docker-compose/oss/docker-compose.dev.yml
  • sdks/python/agenta/sdk/agents/connections/errors.py
  • sdks/python/agenta/sdk/agents/platform/connections.py
  • sdks/python/agenta/sdk/engines/running/errors.py
  • sdks/python/agenta/sdk/engines/running/handlers.py
  • sdks/python/agenta/sdk/engines/running/interfaces.py
  • sdks/python/agenta/sdk/managers/secrets.py
  • sdks/python/agenta/sdk/middlewares/running/vault.py
  • sdks/python/agenta/sdk/utils/assets.py
  • sdks/python/agenta/sdk/utils/providers.py
  • sdks/python/agenta/sdk/utils/types.py
  • sdks/python/oss/tests/pytest/unit/agents/platform/test_connections_http.py
  • sdks/python/oss/tests/pytest/unit/test_auto_ai_critique_v0_connection.py
  • sdks/python/oss/tests/pytest/unit/test_auto_ai_critique_v0_runtime.py
  • sdks/python/oss/tests/pytest/unit/test_chat_v0_inputs.py
  • sdks/python/oss/tests/pytest/unit/test_litellm_model_ids.py
  • sdks/python/oss/tests/pytest/unit/test_llm_v0_handler_flags_running.py
  • sdks/python/oss/tests/pytest/unit/test_llm_v0_provider_key_binding.py
  • sdks/python/oss/tests/pytest/unit/test_prompt_template_extensions.py
  • sdks/python/oss/tests/pytest/unit/test_secrets_manager_connection_slug.py
  • sdks/python/oss/tests/pytest/unit/test_secrets_manager_model_normalization.py
  • sdks/python/oss/tests/pytest/unit/test_supported_llm_models.py
  • sdks/python/oss/tests/pytest/unit/test_vault_secrets_multiple_connections.py
  • services/oss/src/agent/app.py
  • services/oss/src/agent/config.py
  • services/oss/src/agent/runtime_status.py
  • services/oss/tests/pytest/unit/agent/test_subscription_status.py
  • services/runner/src/engines/sandbox_agent/daytona.ts
  • services/runner/src/engines/sandbox_agent/environment-setup.ts
  • services/runner/src/engines/sandbox_agent/environment.ts
  • services/runner/src/engines/sandbox_agent/pi-assets.ts
  • services/runner/src/engines/sandbox_agent/pi-builtin-registry.ts
  • services/runner/src/engines/sandbox_agent/pi-model-config.ts
  • services/runner/src/server.ts
  • services/runner/src/subscription-status.ts
  • services/runner/tests/unit/sandbox-agent-pi-model-registration.test.ts
  • services/runner/tests/unit/server.test.ts
  • services/runner/tests/unit/subscription-status.test.ts
  • web/oss/src/components/AgentChatSlice/assets/onboardingModelSwitch.test.ts
  • web/oss/src/components/AgentChatSlice/assets/onboardingModelSwitch.ts
  • web/oss/src/components/AgentChatSlice/components/AgentComposerDock.tsx
  • web/oss/src/components/AgentChatSlice/components/ConnectModelBanner.tsx
  • web/oss/src/components/AgentChatSlice/hooks/useAgentModelKeyStatus.test.ts
  • web/oss/src/components/AgentChatSlice/hooks/useAgentModelKeyStatus.ts
  • web/oss/src/components/AgentChatSlice/hooks/useChatSlashCommands.tsx
  • web/oss/src/components/AgentChatSlice/hooks/useOnboardingProviderSetup.ts
  • web/oss/src/components/pages/evaluations/NewEvaluation/Components/NewEvaluationModalContent.tsx
  • web/oss/src/components/pages/settings/AIProviders/AIProviders.tsx
  • web/oss/src/components/pages/settings/Preferences/Preferences.tsx
  • web/oss/src/components/pages/settings/Secrets/SecretProviderTable/index.tsx
  • web/oss/src/components/pages/settings/Secrets/Secrets.tsx
  • web/oss/src/components/pages/settings/assets/navigation.ts
  • web/oss/src/hooks/useLLMProviderConfig.tsx
  • web/oss/src/pages/w/[workspace_id]/p/[project_id]/settings/index.tsx
  • web/oss/src/styles/theme-variables.css
  • web/oss/src/styles/theme/palette.ts
  • web/packages/agenta-chat/src/hooks/index.ts
  • web/packages/agenta-chat/src/hooks/useAgentModelKeyStatus.ts
  • web/packages/agenta-entities/src/runnable/evaluatorTransforms.ts
  • web/packages/agenta-entities/src/secret/api/index.ts
  • web/packages/agenta-entities/src/secret/api/probe.ts
  • web/packages/agenta-entities/src/secret/core/cardCopy.ts
  • web/packages/agenta-entities/src/secret/core/connectionSummary.ts
  • web/packages/agenta-entities/src/secret/core/connections.ts
  • web/packages/agenta-entities/src/secret/core/index.ts
  • web/packages/agenta-entities/src/secret/core/litellmModelId.ts
  • web/packages/agenta-entities/src/secret/core/promptModelGroups.ts
  • web/packages/agenta-entities/src/secret/core/providerCatalog.ts
  • web/packages/agenta-entities/src/secret/core/providerFields.ts
  • web/packages/agenta-entities/src/secret/core/subscriptionPairs.ts
  • web/packages/agenta-entities/src/secret/core/transforms.ts
  • web/packages/agenta-entities/src/secret/core/types.ts
  • web/packages/agenta-entities/src/secret/index.ts
  • web/packages/agenta-entities/src/secret/state/connections.ts
  • web/packages/agenta-entities/src/secret/state/index.ts
  • web/packages/agenta-entities/src/secret/state/persistence.ts
  • web/packages/agenta-entities/src/secret/state/subscriptionModels.ts
  • web/packages/agenta-entities/src/workflow/api/index.ts
  • web/packages/agenta-entities/src/workflow/api/subscriptionStatus.ts
  • web/packages/agenta-entities/src/workflow/index.ts
  • web/packages/agenta-entities/src/workflow/state/index.ts
  • web/packages/agenta-entities/src/workflow/state/subscriptionStatus.ts
  • web/packages/agenta-entities/tests/unit/connected-row-subtitle.test.ts
  • web/packages/agenta-entities/tests/unit/evaluator-connection.test.ts
  • web/packages/agenta-entities/tests/unit/litellm-model-id.test.ts
  • web/packages/agenta-entities/tests/unit/prompt-model-groups.test.ts
  • web/packages/agenta-entities/tests/unit/provider-card-copy.test.ts
  • web/packages/agenta-entities/tests/unit/provider-connections.test.ts
  • web/packages/agenta-entities/tests/unit/secret-transforms.test.ts
  • web/packages/agenta-entities/tests/unit/subscription-pairs.test.ts
  • web/packages/agenta-entities/tests/unit/subscriptionStatusApi.test.ts
  • web/packages/agenta-entities/tests/unit/subscriptionStatusDisplay.test.ts
  • web/packages/agenta-entity-ui/src/DrillInView/SchemaControls/AgentTemplateControl.tsx
  • web/packages/agenta-entity-ui/src/DrillInView/SchemaControls/GroupedChoiceControl.tsx
  • web/packages/agenta-entity-ui/src/DrillInView/SchemaControls/agentTemplate/ModelPickerControl.tsx
  • web/packages/agenta-entity-ui/src/DrillInView/SchemaControls/agentTemplate/ProviderCredentialsSection.tsx
  • web/packages/agenta-entity-ui/src/DrillInView/SchemaControls/agentTemplate/ProviderCredentialsSectionView.tsx
  • web/packages/agenta-entity-ui/src/DrillInView/SchemaControls/agentTemplate/SectionChangeBody.tsx
  • web/packages/agenta-entity-ui/src/DrillInView/SchemaControls/agentTemplate/useModelHarness.tsx
  • web/packages/agenta-entity-ui/src/DrillInView/SchemaControls/connectionPicker.ts
  • web/packages/agenta-entity-ui/src/DrillInView/SchemaControls/connectionUtils.ts
  • web/packages/agenta-entity-ui/src/DrillInView/SchemaControls/index.ts
  • web/packages/agenta-entity-ui/src/DrillInView/SchemaControls/pickerSections.ts
  • web/packages/agenta-entity-ui/src/DrillInView/components/PlaygroundConfigSection.tsx
  • web/packages/agenta-entity-ui/src/DrillInView/components/PlaygroundConfigSection/FallbackConfigTab.tsx
  • web/packages/agenta-entity-ui/src/DrillInView/components/PlaygroundConfigSection/ModelConfigEditor.tsx
  • web/packages/agenta-entity-ui/src/DrillInView/components/PlaygroundConfigSection/RetryConfigTab.tsx
  • web/packages/agenta-entity-ui/src/DrillInView/components/PlaygroundConfigSection/useFieldSlots.tsx
  • web/packages/agenta-entity-ui/src/DrillInView/components/PlaygroundConfigSection/useModelConfigurePopover.tsx
  • web/packages/agenta-entity-ui/src/DrillInView/index.ts
  • web/packages/agenta-entity-ui/src/drawers/shared/DrawerFooter.tsx
  • web/packages/agenta-entity-ui/src/secretProvider/ActiveModelsSection.tsx
  • web/packages/agenta-entity-ui/src/secretProvider/CustomProviderForm.tsx
  • web/packages/agenta-entity-ui/src/secretProvider/HarnessesSection.tsx
  • web/packages/agenta-entity-ui/src/secretProvider/PlaygroundProviderSections.tsx
  • web/packages/agenta-entity-ui/src/secretProvider/ProviderCatalogList.tsx
  • web/packages/agenta-entity-ui/src/secretProvider/ProviderConnectionCard.tsx
  • web/packages/agenta-entity-ui/src/secretProvider/ProviderDrawer.tsx
  • web/packages/agenta-entity-ui/src/secretProvider/ScrollScrim.tsx
  • web/packages/agenta-entity-ui/src/secretProvider/SubscriptionPairCard.tsx
  • web/packages/agenta-entity-ui/src/secretProvider/harnessMark.tsx
  • web/packages/agenta-entity-ui/src/secretProvider/index.ts
  • web/packages/agenta-entity-ui/src/secretProvider/providerIcon.ts
  • web/packages/agenta-entity-ui/tests/unit/connectionPicker.test.ts
  • web/packages/agenta-entity-ui/tests/unit/connectionUtils.test.ts
  • web/packages/agenta-entity-ui/tests/unit/pickerSections.test.ts
  • web/packages/agenta-entity-ui/tests/unit/subscriptionPickerRows.test.ts
  • web/packages/agenta-shared/src/state/draftConfigChangeSignal.ts
  • web/packages/agenta-shared/src/types/llmProvider.ts
  • web/packages/agenta-shared/src/utils/curatedLabel.ts
  • web/packages/agenta-shared/src/utils/index.ts
  • web/packages/agenta-shared/tests/unit/curated-label.test.ts
  • web/packages/agenta-ui/src/LLMIcons/assets/Pi.tsx
  • web/packages/agenta-ui/src/LLMIcons/index.ts
  • web/packages/agenta-ui/src/SelectLLMProvider/HarnessTooltip.tsx
  • web/packages/agenta-ui/src/SelectLLMProvider/ManageProvidersRow.tsx
  • web/packages/agenta-ui/src/SelectLLMProvider/SelectLLMProviderBase.tsx
  • web/packages/agenta-ui/src/SelectLLMProvider/index.ts
  • web/packages/agenta-ui/src/SelectLLMProvider/types.ts
  • web/packages/agenta-ui/src/SelectLLMProvider/utils.ts
  • web/packages/agenta-ui/src/drill-in/context/DrillInUIContext.tsx
  • web/scripts/generate-tailwind-tokens.ts

Disabled knowledge base sources:

  • Linear integration is disabled

You can enable these sources in your CodeRabbit configuration.


📝 Walkthrough

Summary by CodeRabbit

  • New Features

    • Manage AI provider connections from a dedicated settings page.
    • Test credentials and discover available models before saving.
    • Configure saved models and harnesses, with curated defaults and unique connection names.
    • Use connection-aware model selection in agents, prompts, and evaluations.
    • View self-managed subscription and harness status.
  • Bug Fixes

    • Preserved connection names, identifiers, and settings across edits and renames.
    • Improved legacy connection compatibility and model resolution.
    • Strengthened endpoint validation and credential protection.
  • Documentation

    • Clarified connection identity, fallback behavior, and default model metadata.

Walkthrough

Provider 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.

Changes

Provider platform and connection lifecycle

Layer / File(s) Summary
Secret contracts and connection lifecycle
api/oss/src/core/secrets/..., web/packages/agenta-entities/src/secret/...
Secret DTOs validate provider-specific payloads and headers. Connections preserve names, slugs, models, harnesses, and omitted-versus-empty update semantics.
Provider credential probing
api/oss/src/core/providers/..., api/oss/src/apis/fastapi/providers/...
Provider adapters probe credentials and model catalogs. Endpoint validation blocks disallowed destinations. The API returns structured, credential-safe probe results.
Connection-aware SDK resolution
sdks/python/agenta/sdk/managers/secrets.py, sdks/python/agenta/sdk/agents/platform/connections.py, sdks/python/agenta/sdk/engines/running/...
SDK resolution selects connections by slug or saved model declarations, preserves legacy fallbacks, normalizes provider kinds and LiteLLM model IDs, and passes connection settings through runtime handlers.
Subscription status detection
services/runner/src/subscription-status.ts, services/runner/src/server.ts, services/oss/src/agent/runtime_status.py, web/packages/agenta-entities/src/workflow/...
The runner detects harness login states. The agent service normalizes runner responses. Web state polls and maps the results to display status.
Provider drawer and connection-first model selection
web/packages/agenta-entity-ui/src/secretProvider/..., web/packages/agenta-entity-ui/src/DrillInView/SchemaControls/..., web/oss/src/components/pages/settings/AIProviders/...
The web application adds provider catalogs, connection cards, credential tests, model and harness controls, subscription model selection, onboarding setup, and connection-first model pickers.

Estimated code review effort: 5 (Critical) | ~120 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 13.46% which is insufficient. The required threshold is 60.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly summarizes the main changes: provider connections gain models, harnesses, and stable slugs.
Description check ✅ Passed The description directly explains the provider connection contract, resolver changes, compatibility behavior, tests, and known risks.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/provider-connections-api

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

return f"{title} {index}"


def _carry_over_saved_policy(*, stored_data: Any, update_data: Any) -> None:

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

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(

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

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]] = {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

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.

Comment thread sdks/python/agenta/sdk/agents/platform/connections.py

@coderabbitai coderabbitai 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.

Actionable comments posted: 5

🧹 Nitpick comments (1)
web/packages/agenta-entities/tests/unit/secret-transforms.test.ts (1)

92-101: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Add explicit-empty policy payload tests.

Add assertions that models: [] and harnesses: [] remain present in the standard-provider payload. Add an assertion that harnesses: [] 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

📥 Commits

Reviewing files that changed from the base of the PR and between 728f39f and ddc0f1a.

📒 Files selected for processing (21)
  • api/oss/src/core/secrets/dtos.py
  • api/oss/src/core/secrets/enums.py
  • api/oss/src/core/secrets/services.py
  • api/oss/tests/legacy/vault_router/test_vault_secrets_apis.py
  • api/oss/tests/pytest/unit/secrets/test_dtos.py
  • api/oss/tests/pytest/unit/secrets/test_services.py
  • docs/design/agent-workflows/interfaces/cross-service/service-to-vault-and-tool-providers.md
  • docs/design/agent-workflows/interfaces/in-service/model-connection-resolution.md
  • docs/design/agent-workflows/interfaces/public-edge/workflow-inspect.md
  • sdks/python/agenta/sdk/agents/capabilities.py
  • sdks/python/agenta/sdk/agents/platform/connections.py
  • sdks/python/oss/tests/pytest/unit/agents/connections/test_model_catalog.py
  • sdks/python/oss/tests/pytest/unit/agents/platform/test_connections_http.py
  • web/oss/src/components/ModelRegistry/Modals/ConfigureProviderModal/index.tsx
  • web/packages/agenta-entities/src/secret/core/index.ts
  • web/packages/agenta-entities/src/secret/core/transforms.ts
  • web/packages/agenta-entities/src/secret/core/types.ts
  • web/packages/agenta-entities/src/secret/state/atoms.ts
  • web/packages/agenta-entities/src/workflow/state/inspectMeta.ts
  • web/packages/agenta-entities/tests/unit/secret-transforms.test.ts
  • web/packages/agenta-shared/src/types/llmProvider.ts

Comment thread api/oss/src/core/secrets/services.py
Comment thread api/oss/tests/pytest/unit/secrets/test_services.py
Comment thread sdks/python/agenta/sdk/agents/capabilities.py
Comment thread web/oss/src/components/ModelRegistry/Modals/ConfigureProviderModal/index.tsx Outdated
@github-actions

github-actions Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Railway Preview Environment

Preview URL https://gateway-pr-5995.up.railway.app/w
Project agenta-oss-clone-spike
Image tag pr-5995-a0db08d
Status Deployed
Railway logs Open logs
Workflow logs View workflow run
Updated at 2026-08-13T15:59:52.826Z

@mmabrouk mmabrouk left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

lgtm

Copy link
Copy Markdown
Member Author

One actionable issue found:

[P2] Allow header to be omitted for unnamed provider keys (api/oss/src/core/secrets/dtos.py:214)

ensure_header_exists permits an unnamed provider_key, but header remains a required field. The QA payload shown in this PR, which omits header entirely, fails validation with 422: header Field required. The added tests only cover "header": {}, so they do not catch the documented payload.

Please make header optional or supply a default empty Header for provider_key creation, and add a test that omits the field entirely.

Copy link
Copy Markdown
Member Author

Review findings:

  1. [P1] Header-less provider creation still fails. In api/oss/src/core/secrets/dtos.py, CreateSecretDTO.header remains required. The documented payload containing only secret therefore returns Pydantic's Field required error before ensure_header_exists can allow a provider key without a header. I reproduced this against commit ddc0f1a46f412fdc2a9bdf31a21e6d151b1aac6a. Please make header optional or give it a default Header(), and add a test that omits the header property entirely rather than passing "header": {}.

  2. [P2] Concurrent creates can receive the same display name. In VaultService._name_and_slug_provider_key, naming uses an unprotected list-then-create sequence. Two simultaneous OpenAI requests can both observe the same existing names and choose OpenAI. The database only enforces slug uniqueness, so both records can succeed. Please add an atomic database constraint/retry mechanism, or weaken the sequential-name guarantee and document the behavior.

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.
@mmabrouk
mmabrouk changed the base branch from main to release/v0.112.1 August 13, 2026 14:48
feat(web): the AI providers experience - settings, picker, and run-path wiring
feat(agents): runner subscription status on the credential card
@dosubot dosubot Bot removed the size:XL This PR changes 500-999 lines, ignoring generated files. label Aug 13, 2026
@dosubot dosubot Bot added the size:XXL This PR changes 1000+ lines, ignoring generated files. label Aug 13, 2026
@mmabrouk
mmabrouk merged commit 3bc76b2 into release/v0.112.1 Aug 13, 2026
18 of 21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request lgtm This PR has been approved by a maintainer size:XXL This PR changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant