Skip to content

Commit 86b273c

Browse files
Aurelioloclaude
Aureliolo
and
claude
committed
fix: address 49 review findings from 16 agents, CodeRabbit, and Gemini
Backend: - Replace vendor names in tests with generic test-provider/test-subscription-token - Wrap _AUTH_OWNED_FIELDS in MappingProxyType for immutability - Guard ToS consent stamping to subscription auth only - Resolve auth_type override before preset discovery decision - Fix litellm_driver class docstring (litellm_provider routing key) - Consistent extra_headers assignment in _build_kwargs - Rename _validate_api_key_clear_consistency to _validate_credential_clear_consistency - Restore DTO Attributes docstrings (CreateProviderRequest, ProviderResponse, CreateFromPresetRequest) - Remove vendor names from schema.py litellm_provider docstring - Fix USD -> base currency in cost docstrings - Fix subscription_token docstring (not OAuth) - Update Anthropic preset Sonnet model to claude-sonnet-4-6-20250514 Tests: - Add SUBSCRIPTION auth tests for _build_kwargs, build_discovery_headers - Add _apply_credential_updates SUBSCRIPTION transition tests - Add auth-type-switch cleanup tests (subscription <-> api_key) - Add ProviderPreset validator rejection test - Remove redundant @pytest.mark.unit class decorators Frontend: - Remove double-fetch on mount in useProvidersData/useProviderDetailData - Remove dead deleteTarget code from ProvidersPage - Use shared InputField/SelectField in ProviderFilters - Export ProviderHealthBadgeProps, add a11y.test to stories - ProviderFormDrawer: hide unsupported auth options, reset Custom state, send clear_* flags in edit mode, fix base URL visibility, show presetsError - Extract PresetOptionCard, ProviderGridItem, ProviderModelRow components - Use SectionCard in ProviderDetailSkeleton - Add aria-live to TestConnectionResult, fix error truncation - Fix replaceAll for auth_type display - Replace hardcoded hover shadow with design token - EmptyState for missing provider instead of null - Remove redundant decodeURIComponent - Fix last-check date format (add date, not just time) - Fix health sort: unknown sorts after down - Remove hardcoded $ currency symbol - Add discoveringModels loading flag to store Docs: - Add missing API endpoints to operations.md - Add supported_auth_types and fix vLLM auto-probe in operations.md - Add ProviderHealthStatus type and getProviderHealthColor to brand-and-ux.md - Expand Providers description in page-structure.md Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent e088d4b commit 86b273c

33 files changed

Lines changed: 582 additions & 211 deletions

‎docs/design/brand-and-ux.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -240,6 +240,7 @@ The following shared components live in `web/src/components/ui/` and form the bu
240240
| `getPriorityColor()` | `utils/tasks.ts` | Maps `Priority` to `SemanticColor`. |
241241
| `getPriorityLabel()` | `utils/tasks.ts` | Maps `Priority` to display label. |
242242
| `getTaskTypeLabel()` | `utils/tasks.ts` | Maps `TaskType` to display label. |
243+
| `getProviderHealthColor()` | `utils/providers.ts` | Maps `ProviderHealthStatus` to `SemanticColor`. |
243244

244245
### Animation Hooks
245246

@@ -257,6 +258,7 @@ The following shared components live in `web/src/components/ui/` and form the bu
257258
| `SemanticColor` | `lib/utils.ts` | `"success"`, `"accent"`, `"warning"`, `"danger"` |
258259
| `TaskStatus` | `api/types` | `"created"`, `"assigned"`, `"in_progress"`, `"in_review"`, `"completed"`, `"blocked"`, `"failed"`, `"interrupted"`, `"cancelled"` |
259260
| `Priority` | `api/types` | `"critical"`, `"high"`, `"medium"`, `"low"` |
261+
| `ProviderHealthStatus` | `api/types` | `"up"`, `"degraded"`, `"down"` |
260262

261263
### When to Create a New Shared Component
262264

‎docs/design/operations.md‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -117,8 +117,8 @@ Providers can be managed at runtime through the API without restarting:
117117
- Auto-triggered on preset creation for no-auth providers with empty model lists.
118118
- SSRF trust is determined by a dynamic `host:port` allowlist (`ProviderDiscoveryPolicy`), seeded from preset `candidate_urls` at startup and auto-updated on provider create/update/delete. Trusted URLs bypass SSRF validation; untrusted URLs go through full private-IP/DNS-rebinding checks. Bypasses are logged at WARNING level (`PROVIDER_DISCOVERY_SSRF_BYPASSED`).
119119
- **Discovery allowlist**: `GET /api/v1/providers/discovery-policy` (read), `POST /api/v1/providers/discovery-policy/entries` (add entry), `POST /api/v1/providers/discovery-policy/remove-entry` (remove entry) -- manage the dynamic SSRF allowlist of trusted `host:port` pairs for provider discovery. Persisted in the settings system (DB > env > YAML > code).
120-
- **Presets**: `GET /api/v1/providers/presets` lists built-in cloud and local provider templates (11 presets: Anthropic, OpenAI, Google AI, Mistral, Groq, DeepSeek, Azure OpenAI, Ollama, LM Studio, vLLM, OpenRouter); `POST /api/v1/providers/from-preset` creates from a template
121-
- **Preset auto-probe**: `POST /api/v1/providers/probe-preset` -- for presets with `candidate_urls` (local providers: Ollama, LM Studio, vLLM), probes each URL in priority order (`host.docker.internal`, Docker bridge IP, `localhost`) with a 5-second timeout. Returns the first reachable URL and discovered model count. Used by the setup wizard to auto-detect local providers running on the host machine. SSRF validation is intentionally skipped because only hardcoded preset URLs are probed, never user input.
120+
- **Presets**: `GET /api/v1/providers/presets` lists built-in cloud and local provider templates (11 presets: Anthropic, OpenAI, Google AI, Mistral, Groq, DeepSeek, Azure OpenAI, Ollama, LM Studio, vLLM, OpenRouter); `POST /api/v1/providers/from-preset` creates from a template. Each preset declares `supported_auth_types` (e.g. `["api_key"]`, `["none"]`, `["api_key", "subscription"]`) which the UI uses to present the available authentication options during provider creation.
121+
- **Preset auto-probe**: `POST /api/v1/providers/probe-preset` -- for presets with `candidate_urls` (local providers: Ollama and LM Studio), probes each URL in priority order (`host.docker.internal`, Docker bridge IP, `localhost`) with a 5-second timeout. Returns the first reachable URL and discovered model count. Used by the setup wizard to auto-detect local providers running on the host machine. SSRF validation is intentionally skipped because only hardcoded preset URLs are probed, never user input. Note: vLLM's `candidate_urls` is intentionally empty (users deploy vLLM at arbitrary endpoints), so it cannot be auto-probed and requires manual URL configuration.
122122
- **Hot-reload**: On mutation, `ProviderManagementService` rebuilds `ProviderRegistry` + `ModelRouter` and atomically swaps them in `AppState` -- no downtime
123123
- **Auth types**: `api_key` (default), `subscription` (OAuth bearer token for provider subscription plans, requires ToS acceptance), `oauth` (stores credentials, MVP uses pre-fetched token), `custom_header`, `none` (local providers)
124124
- **Routing key**: Optional `litellm_provider` field decouples the provider display name from LiteLLM routing (e.g. a provider named "my-claude" can route to `anthropic` via `litellm_provider: anthropic`). Falls back to provider name when unset.
@@ -1103,7 +1103,7 @@ future CLI tool are thin clients that call the API -- they contain no business l
11031103
| `/api/v1/approvals` | Pending human approvals queue |
11041104
| `/api/v1/analytics` | `GET /overview` (metrics summary with budget status, 7d spend sparkline, agent counts), `GET /trends?period=7d\|30d\|90d&metric=spend\|tasks_completed\|active_agents\|success_rate` (time-series bucketed metrics; hourly buckets for 7d, daily for 30d/90d; defaults: `period=7d`, `metric=spend`), `GET /forecast?horizon_days=1..90` (budget spend projection with daily projections and exhaustion estimate; default 14; 400 on out-of-range) |
11051105
| `/api/v1/settings` | Runtime-editable configuration (9 namespaces), schema discovery |
1106-
| `GET /api/v1/providers`, `POST /api/v1/providers`, `PUT /api/v1/providers/{name}`, `DELETE /api/v1/providers/{name}`, `POST /api/v1/providers/{name}/test`, `GET /api/v1/providers/presets`, `POST /api/v1/providers/from-preset`, `POST /api/v1/providers/{name}/discover-models`, `POST /api/v1/providers/probe-preset`, `GET /api/v1/providers/discovery-policy`, `POST /api/v1/providers/discovery-policy/entries`, `POST /api/v1/providers/discovery-policy/remove-entry` | Provider CRUD, connection testing, presets, preset auto-probe, model discovery, discovery SSRF allowlist management, 5 auth types (api_key, subscription, oauth, custom_header, none) |
1106+
| `GET /api/v1/providers`, `GET /api/v1/providers/{name}`, `GET /api/v1/providers/{name}/models`, `POST /api/v1/providers`, `PUT /api/v1/providers/{name}`, `DELETE /api/v1/providers/{name}`, `POST /api/v1/providers/{name}/test`, `GET /api/v1/providers/presets`, `POST /api/v1/providers/from-preset`, `POST /api/v1/providers/{name}/discover-models`, `POST /api/v1/providers/probe-preset`, `GET /api/v1/providers/discovery-policy`, `POST /api/v1/providers/discovery-policy/entries`, `POST /api/v1/providers/discovery-policy/remove-entry` | Provider CRUD, single provider detail, model listing, connection testing, presets, preset auto-probe, model discovery, discovery SSRF allowlist management, 5 auth types (api_key, subscription, oauth, custom_header, none) |
11071107
| `GET /api/v1/setup/status`, `GET /api/v1/setup/templates`, `POST /api/v1/setup/company`, `POST /api/v1/setup/agent`, `GET /api/v1/setup/agents`, `PUT /api/v1/setup/agents/{index}/model` (`{index}` = zero-based position in the list returned by `GET /api/v1/setup/agents`; not a stable ID -- re-fetch to resolve; out-of-range returns 404), `GET /api/v1/setup/name-locales/available`, `GET /api/v1/setup/name-locales`, `PUT /api/v1/setup/name-locales`, `POST /api/v1/setup/complete` | First-run setup wizard: status check (public, reports `has_company`/`has_agents`/`has_providers`/`has_name_locales` for step resume), template listing, company creation (auto-creates template agents with model matching), agent listing + model reassignment, manual agent creation (blank path), name locale management (list available Faker locales, get/set selected locales for agent name generation), completion gate (requires company + agents + providers) |
11081108
| `/api/v1/users` | CEO-only user CRUD: create, list, get, update role, delete human user accounts |
11091109
| `/api/v1/admin/backups` | Manual backup, list, detail, delete |

‎docs/design/page-structure.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ Meeting history list with status/type filters. Click opens meeting detail (`/mee
9999

100100
#### Providers (`/providers`)
101101

102-
LLM provider management. CRUD cards for configured providers. Connection test button. Preset-based creation flow. Model auto-discovery. Provider detail/edit at `/providers/{name}`.
102+
LLM provider management. CRUD cards for configured providers with health status display (up/degraded/down) and health metrics (average response time, error rate percentage, call count). Connection test button. Preset-based creation flow with subscription auth support requiring ToS acceptance for applicable providers. Model auto-discovery. Provider list supports filtering and sorting by health status, name, and model count. Provider detail/edit at `/providers/{name}`.
103103

104104
No WebSocket subscription -- provider changes are low-frequency admin operations. TanStack Query polling is sufficient.
105105

‎src/synthorg/api/dto.py‎

Lines changed: 39 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -529,7 +529,19 @@ def _validate_provider_name(v: str) -> str:
529529

530530

531531
class CreateProviderRequest(BaseModel):
532-
"""Payload for creating a new provider."""
532+
"""Payload for creating a new provider.
533+
534+
Attributes:
535+
name: Unique provider name (2-64 chars, lowercase alphanumeric + hyphens).
536+
driver: Driver backend name (default ``"litellm"``).
537+
litellm_provider: LiteLLM routing identifier override.
538+
auth_type: Authentication mechanism for this provider.
539+
api_key: API key credential (optional, depends on auth_type).
540+
subscription_token: Bearer token for subscription-based auth.
541+
tos_accepted: Whether the user accepted the subscription ToS warning.
542+
base_url: Provider API base URL.
543+
models: Pre-configured model definitions.
544+
"""
533545

534546
model_config = ConfigDict(frozen=True)
535547

@@ -583,8 +595,8 @@ class UpdateProviderRequest(BaseModel):
583595
models: tuple[ProviderModelConfig, ...] | None = None
584596

585597
@model_validator(mode="after")
586-
def _validate_api_key_clear_consistency(self) -> Self:
587-
"""Reject simultaneous api_key and clear_api_key."""
598+
def _validate_credential_clear_consistency(self) -> Self:
599+
"""Reject simultaneous set and clear for credential fields."""
588600
if self.api_key is not None and self.clear_api_key:
589601
msg = "api_key and clear_api_key are mutually exclusive"
590602
raise ValueError(msg)
@@ -641,6 +653,20 @@ class ProviderResponse(BaseModel):
641653
"""Safe provider config for API responses -- secrets stripped.
642654
643655
Non-secret auth fields are included for frontend edit form UX.
656+
Boolean ``has_*`` indicators signal credential presence without
657+
exposing values.
658+
659+
Attributes:
660+
driver: Driver backend name.
661+
litellm_provider: LiteLLM routing identifier override.
662+
auth_type: Authentication mechanism.
663+
base_url: Provider API base URL.
664+
models: Configured model definitions.
665+
has_api_key: Whether an API key is set.
666+
has_oauth_credentials: Whether OAuth credentials are configured.
667+
has_custom_header: Whether a custom auth header is configured.
668+
has_subscription_token: Whether a subscription token is set.
669+
tos_accepted_at: ISO timestamp of ToS acceptance (or ``None``).
644670
"""
645671

646672
model_config = ConfigDict(frozen=True)
@@ -662,7 +688,16 @@ class ProviderResponse(BaseModel):
662688

663689

664690
class CreateFromPresetRequest(BaseModel):
665-
"""Payload for creating a provider from a preset."""
691+
"""Payload for creating a provider from a preset.
692+
693+
Attributes:
694+
preset_name: Name of the preset to create from.
695+
name: Unique provider name (2-64 chars, lowercase alphanumeric + hyphens).
696+
auth_type: Override the preset's default auth type (optional).
697+
subscription_token: Bearer token for subscription-based auth.
698+
tos_accepted: Whether the user accepted the subscription ToS warning.
699+
base_url: Override the preset's default base URL (optional).
700+
"""
666701

667702
model_config = ConfigDict(frozen=True)
668703

‎src/synthorg/config/schema.py‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,8 @@ class ProviderModelConfig(BaseModel):
4747
Attributes:
4848
id: Model identifier (e.g. ``"example-medium-001"``).
4949
alias: Short alias for referencing this model in routing rules.
50-
cost_per_1k_input: Cost per 1,000 input tokens in USD (base currency).
51-
cost_per_1k_output: Cost per 1,000 output tokens in USD (base currency).
50+
cost_per_1k_input: Cost per 1,000 input tokens (base currency).
51+
cost_per_1k_output: Cost per 1,000 output tokens (base currency).
5252
max_context: Maximum context window size in tokens.
5353
estimated_latency_ms: Estimated median latency in milliseconds.
5454
"""
@@ -88,13 +88,13 @@ class ProviderConfig(BaseModel):
8888
8989
Attributes:
9090
driver: Driver backend name (e.g. ``"litellm"``).
91-
litellm_provider: LiteLLM routing identifier (e.g. ``"anthropic"``,
92-
``"openai"``). When set, the driver uses this instead of the
93-
provider name as the model prefix for LiteLLM routing. Falls
94-
back to the provider name when ``None``.
91+
litellm_provider: LiteLLM routing identifier (e.g.
92+
``"example-provider"``). When set, the driver uses this instead
93+
of the provider name as the model prefix for LiteLLM routing.
94+
Falls back to the provider name when ``None``.
9595
auth_type: Authentication type for this provider.
9696
api_key: API key (typically injected by secret management).
97-
subscription_token: OAuth bearer token for subscription-based auth
97+
subscription_token: Bearer token for subscription-based auth
9898
(e.g. provider subscription plans). Encrypted at rest.
9999
tos_accepted_at: Timestamp when the user accepted the subscription
100100
Terms of Service warning. Required when ``auth_type`` is
@@ -124,7 +124,7 @@ class ProviderConfig(BaseModel):
124124
default=None,
125125
description=(
126126
"LiteLLM provider identifier for routing "
127-
"(e.g. 'anthropic', 'openai'). Falls back to "
127+
"(e.g. 'example-provider'). Falls back to "
128128
"the provider name when None."
129129
),
130130
)

‎src/synthorg/providers/drivers/litellm_driver.py‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -108,9 +108,10 @@ class LiteLLMDriver(BaseCompletionProvider):
108108
"""Completion driver backed by LiteLLM.
109109
110110
Uses ``litellm.acompletion`` for both streaming and non-streaming
111-
calls. Model identifiers are prefixed with the provider name
112-
(e.g. ``example-provider/example-medium-001``) so LiteLLM routes to
113-
the correct backend.
111+
calls. Model identifiers are prefixed with the LiteLLM routing key
112+
(``litellm_provider`` if set, otherwise the provider name -- e.g.
113+
``example-provider/example-medium-001``) so LiteLLM routes to the
114+
correct backend.
114115
115116
Args:
116117
provider_name: Provider key from config (e.g. ``"example-provider"``).
@@ -351,9 +352,9 @@ def _build_kwargs( # noqa: C901
351352
}
352353
case AuthType.SUBSCRIPTION:
353354
if self._config.subscription_token is not None:
354-
kwargs.setdefault("extra_headers", {})["Authorization"] = (
355-
f"Bearer {self._config.subscription_token}"
356-
)
355+
kwargs["extra_headers"] = {
356+
"Authorization": f"Bearer {self._config.subscription_token}",
357+
}
357358
case AuthType.NONE:
358359
pass
359360

‎src/synthorg/providers/management/_helpers.py‎

Lines changed: 25 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,16 @@ def build_provider_config(
2525
Returns:
2626
Frozen ProviderConfig.
2727
"""
28-
tos_accepted_at = datetime.now(UTC) if request.tos_accepted else None
28+
is_subscription = request.auth_type == AuthType.SUBSCRIPTION
29+
tos_accepted_at = (
30+
datetime.now(UTC) if is_subscription and request.tos_accepted else None
31+
)
2932
return ProviderConfig(
3033
driver=request.driver,
3134
litellm_provider=request.litellm_provider,
3235
auth_type=request.auth_type,
3336
api_key=request.api_key,
34-
subscription_token=request.subscription_token,
37+
subscription_token=request.subscription_token if is_subscription else None,
3538
tos_accepted_at=tos_accepted_at,
3639
base_url=request.base_url,
3740
oauth_token_url=request.oauth_token_url,
@@ -58,20 +61,25 @@ def build_provider_config(
5861
)
5962

6063

61-
# Credential fields to clear when switching away from an auth type.
62-
_AUTH_OWNED_FIELDS: dict[AuthType, tuple[str, ...]] = {
63-
AuthType.API_KEY: ("api_key",),
64-
AuthType.OAUTH: (
65-
"api_key",
66-
"oauth_client_secret",
67-
"oauth_token_url",
68-
"oauth_client_id",
69-
"oauth_scope",
70-
),
71-
AuthType.CUSTOM_HEADER: ("custom_header_name", "custom_header_value"),
72-
AuthType.SUBSCRIPTION: ("subscription_token", "tos_accepted_at"),
73-
AuthType.NONE: (),
74-
}
64+
# Fields owned by each auth type. When switching auth types, fields
65+
# not owned by the new type are cleared.
66+
_AUTH_OWNED_FIELDS: Final[MappingProxyType[AuthType, tuple[str, ...]]] = (
67+
MappingProxyType(
68+
{
69+
AuthType.API_KEY: ("api_key",),
70+
AuthType.OAUTH: (
71+
"api_key",
72+
"oauth_client_secret",
73+
"oauth_token_url",
74+
"oauth_client_id",
75+
"oauth_scope",
76+
),
77+
AuthType.CUSTOM_HEADER: ("custom_header_name", "custom_header_value"),
78+
AuthType.SUBSCRIPTION: ("subscription_token", "tos_accepted_at"),
79+
AuthType.NONE: (),
80+
}
81+
)
82+
)
7583

7684

7785
def apply_update(
@@ -118,7 +126,7 @@ def _apply_credential_updates(
118126
request: UpdateProviderRequest,
119127
final_auth_type: AuthType,
120128
) -> None:
121-
"""Apply set/clear logic for api_key and subscription_token."""
129+
"""Apply set/clear logic for api_key, subscription_token, and tos_accepted_at."""
122130
# api_key: only set/clear when the resulting auth type supports it
123131
if final_auth_type in (AuthType.API_KEY, AuthType.OAUTH):
124132
if request.api_key is not None:

‎src/synthorg/providers/management/service.py‎

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -362,8 +362,10 @@ async def create_from_preset(
362362
) -> ProviderConfig:
363363
"""Create a provider from a preset template.
364364
365-
When the preset has ``auth_type=none`` and a base URL but no
366-
models, attempts auto-discovery before creating the provider.
365+
The request's ``auth_type`` overrides the preset default when
366+
provided. When the effective auth type is ``AuthType.NONE``
367+
and a base URL is available but no models are given, attempts
368+
auto-discovery before creating the provider.
367369
368370
Args:
369371
request: Preset-based creation request.
@@ -387,13 +389,13 @@ async def create_from_preset(
387389

388390
models = request.models if request.models is not None else preset.default_models
389391
base_url = request.base_url or preset.default_base_url
392+
auth_type = request.auth_type or preset.auth_type
390393
models = await self._maybe_discover_preset_models(
391394
preset,
392395
base_url,
393396
models,
397+
auth_type=auth_type,
394398
)
395-
396-
auth_type = request.auth_type or preset.auth_type
397399
create_request = CreateProviderRequest(
398400
name=request.name,
399401
driver=preset.driver,
@@ -412,23 +414,26 @@ async def _maybe_discover_preset_models(
412414
preset: ProviderPreset,
413415
base_url: str | None,
414416
models: tuple[ProviderModelConfig, ...],
417+
*,
418+
auth_type: AuthType,
415419
) -> tuple[ProviderModelConfig, ...]:
416420
"""Auto-discover models for no-auth presets when none are provided.
417421
418-
Only attempts discovery when the preset uses ``AuthType.NONE``,
419-
a base URL is available, and no models were explicitly provided.
420-
Trust is determined by the discovery allowlist -- preset default
421-
URLs are seeded on first access, so they are automatically trusted.
422+
Only attempts discovery when the effective auth type is
423+
``AuthType.NONE``, a base URL is available, and no models were
424+
explicitly provided. The request's ``auth_type`` override is
425+
applied before this check.
422426
423427
Args:
424428
preset: Resolved preset definition.
425429
base_url: Provider base URL (may be user-overridden).
426430
models: Explicitly provided models (may be empty).
431+
auth_type: Effective auth type (request override or preset default).
427432
428433
Returns:
429434
Discovered models if any, otherwise the original models.
430435
"""
431-
if models or preset.auth_type != AuthType.NONE or not base_url:
436+
if models or auth_type != AuthType.NONE or not base_url:
432437
return models
433438
if self._is_self_connection(base_url):
434439
return models

0 commit comments

Comments
 (0)