feat(providers): runtime provider management with CRUD, presets, and multi-auth#540
feat(providers): runtime provider management with CRUD, presets, and multi-auth#540
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📜 Recent review details🧰 Additional context used📓 Path-based instructions (2)web/src/**/*.{vue,ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
web/src/__tests__/**/*.{test,spec}.ts📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (3)📚 Learning: 2026-03-18T07:33:37.988ZApplied to files:
📚 Learning: 2026-03-18T07:33:37.988ZApplied to files:
📚 Learning: 2026-03-15T11:48:14.867ZApplied to files:
🪛 GitHub Check: Dashboard Lintweb/src/__tests__/stores/providers.test.ts[warning] 248-248: 🔇 Additional comments (6)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds runtime provider management: API endpoints and DTOs for provider CRUD, presets and connection tests; ProviderManagementService with serialized mutations, persistence and hot-reload wiring; multi-auth support (api_key|oauth|custom_header|none); provider presets; observability events; Litellm auth branching; frontend store, components and tests. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client (Web UI / API consumer)
participant API as API Controller (/api/v1/providers)
participant Service as ProviderManagementService
participant Settings as SettingsService (persistence)
participant Resolver as ConfigResolver / ProviderRegistry
participant AppState as AppState (swap provider_registry/model_router)
Client->>API: POST /api/v1/providers (CreateProviderRequest)
API->>Service: create_provider(request)
Service->>Settings: persist serialized providers
Settings-->>Service: persist OK
Service->>Resolver: build ProviderRegistry & ModelRouter (validate)
Resolver-->>Service: build OK
Service->>AppState: swap provider_registry/model_router (hot-reload)
AppState-->>Service: swap complete
Service-->>API: ProviderConfig (sanitized)
API-->>Client: 201 Created (ProviderResponse)
sequenceDiagram
participant Client as Client (Test button)
participant API as API Controller (/api/v1/providers/{name}/test)
participant Service as ProviderManagementService
participant Driver as LiteLLMDriver
participant External as External Provider Endpoint
Client->>API: POST /api/v1/providers/{name}/test
API->>Service: test_connection(name, request)
Service->>Driver: instantiate driver with provider config
Driver->>External: minimal request (model ping/completion)
External-->>Driver: response / error
Driver-->>Service: test result (latency / success/error)
Service-->>API: TestConnectionResponse
API-->>Client: 200 OK (test result)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
📝 Coding Plan
Comment |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the system's ability to manage LLM providers dynamically at runtime. It provides a robust set of API endpoints and a user-friendly web interface for creating, updating, deleting, and testing provider configurations. By supporting diverse authentication methods and offering convenient presets, it streamlines the integration of various LLM services, all while maintaining high security standards for credentials and ensuring continuous service availability through hot-reloads. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #540 +/- ##
==========================================
- Coverage 93.16% 93.01% -0.16%
==========================================
Files 523 526 +3
Lines 25314 25687 +373
Branches 2402 2437 +35
==========================================
+ Hits 23585 23893 +308
- Misses 1369 1429 +60
- Partials 360 365 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Code Review
This is an impressive pull request that introduces comprehensive runtime provider management, including CRUD operations, presets, and multi-auth support. The backend architecture is robust, featuring a dedicated service with locking for safe concurrent operations and a well-designed hot-reload mechanism for zero-downtime updates. The frontend has been significantly enhanced with a user-friendly card-based UI and a flexible form dialog for provider configuration. The code is well-structured and includes a substantial number of new tests, demonstrating a commitment to quality. I have a few suggestions to further improve logging clarity, code style, and the user experience when editing providers.
| from synthorg.api.dto import ( | ||
| ApiResponse, | ||
| CreateFromPresetRequest, | ||
| CreateProviderRequest, | ||
| ProviderResponse, | ||
| TestConnectionResponse, | ||
| UpdateProviderRequest, | ||
| to_provider_response, | ||
| ) | ||
| from synthorg.api.dto import ( | ||
| TestConnectionRequest as ConnTestRequest, | ||
| ) |
There was a problem hiding this comment.
For improved readability and to adhere to standard Python import practices, the separate import blocks from synthorg.api.dto can be consolidated into a single block.
from synthorg.api.dto import (
ApiResponse,
CreateFromPresetRequest,
CreateProviderRequest,
ProviderResponse,
TestConnectionRequest as ConnTestRequest,
TestConnectionResponse,
UpdateProviderRequest,
to_provider_response,
)| class ProviderResponse(BaseModel): | ||
| """Safe provider config for API responses -- secrets stripped. | ||
|
|
||
| Attributes: | ||
| driver: Driver backend name. | ||
| auth_type: Authentication type. | ||
| base_url: Provider API base URL. | ||
| models: Model configurations. | ||
| has_api_key: Whether an API key is configured. | ||
| has_oauth_credentials: Whether OAuth credentials are configured. | ||
| has_custom_header: Whether a custom header is configured. | ||
| """ | ||
|
|
||
| model_config = ConfigDict(frozen=True) | ||
|
|
||
| driver: NotBlankStr | ||
| auth_type: AuthType | ||
| base_url: NotBlankStr | None | ||
| models: tuple[ProviderModelConfig, ...] | ||
| has_api_key: bool | ||
| has_oauth_credentials: bool | ||
| has_custom_header: bool | ||
|
|
||
|
|
||
| class CreateFromPresetRequest(BaseModel): | ||
| """Payload for creating a provider from a preset. | ||
|
|
||
| Attributes: | ||
| preset_name: Preset identifier to use. | ||
| name: Provider name to create. | ||
| api_key: Override API key for presets that need one. | ||
| base_url: Override default base URL. | ||
| models: Override default models. | ||
| """ | ||
|
|
||
| model_config = ConfigDict(frozen=True) | ||
|
|
||
| preset_name: NotBlankStr | ||
| name: NotBlankStr = Field(max_length=64) | ||
| api_key: NotBlankStr | None = None | ||
| base_url: NotBlankStr | None = None | ||
| models: tuple[ProviderModelConfig, ...] | None = None | ||
|
|
||
| @field_validator("name") | ||
| @classmethod | ||
| def _validate_name(cls, v: str) -> str: | ||
| return _validate_provider_name(v) | ||
|
|
||
|
|
||
| def to_provider_response(config: ProviderConfig) -> ProviderResponse: | ||
| """Convert a ProviderConfig to a safe ProviderResponse. | ||
|
|
||
| Strips all secrets and provides boolean credential indicators. | ||
|
|
||
| Args: | ||
| config: Provider configuration (may contain secrets). | ||
|
|
||
| Returns: | ||
| Safe response DTO with secrets stripped. | ||
| """ | ||
| return ProviderResponse( | ||
| driver=config.driver, | ||
| auth_type=config.auth_type, | ||
| base_url=config.base_url, | ||
| models=config.models, | ||
| has_api_key=config.api_key is not None, | ||
| has_oauth_credentials=( | ||
| config.oauth_client_id is not None | ||
| and config.oauth_client_secret is not None | ||
| and config.oauth_token_url is not None | ||
| ), | ||
| has_custom_header=config.custom_header_value is not None, | ||
| ) |
There was a problem hiding this comment.
The ProviderResponse DTO currently strips all authentication-related fields for security, which is a good practice. However, this negatively impacts the user experience on the frontend when a user edits a provider, as they must re-enter all authentication fields (even non-secret ones like OAuth Token URL or Client ID) to make a simple change.
To improve usability, I suggest including non-secret auth fields in ProviderResponse. This would allow the frontend to pre-populate the edit form, requiring the user to only re-enter the actual secrets if they need to be changed.
class ProviderResponse(BaseModel):
"""Safe provider config for API responses -- secrets stripped.
Attributes:
driver: Driver backend name.
auth_type: Authentication type.
base_url: Provider API base URL.
models: Model configurations.
has_api_key: Whether an API key is configured.
has_oauth_credentials: Whether OAuth credentials are configured.
has_custom_header: Whether a custom header is configured.
oauth_token_url: OAuth token endpoint URL (non-secret).
oauth_client_id: OAuth client identifier (non-secret).
oauth_scope: OAuth scope string (non-secret).
custom_header_name: Name of custom auth header (non-secret).
"""
model_config = ConfigDict(frozen=True)
driver: NotBlankStr
auth_type: AuthType
base_url: NotBlankStr | None
models: tuple[ProviderModelConfig, ...]
has_api_key: bool
has_oauth_credentials: bool
has_custom_header: bool
# Non-secret auth fields for better UX in edit forms
oauth_token_url: NotBlankStr | None = None
oauth_client_id: NotBlankStr | None = None
oauth_scope: NotBlankStr | None = None
custom_header_name: NotBlankStr | None = None
def to_provider_response(config: ProviderConfig) -> ProviderResponse:
"""Convert a ProviderConfig to a safe ProviderResponse.
Strips all secrets and provides boolean credential indicators.
Args:
config: Provider configuration (may contain secrets).
Returns:
Safe response DTO with secrets stripped.
"""
return ProviderResponse(
driver=config.driver,
auth_type=config.auth_type,
base_url=config.base_url,
models=config.models,
has_api_key=config.api_key is not None,
has_oauth_credentials=(
config.oauth_client_id is not None
and config.oauth_client_secret is not None
and config.oauth_token_url is not None
),
has_custom_header=config.custom_header_value is not None,
# Expose non-secret auth fields for better frontend UX
oauth_token_url=config.oauth_token_url,
oauth_client_id=config.oauth_client_id,
oauth_scope=config.oauth_scope,
custom_header_name=config.custom_header_name,
)| config = providers.get(name) | ||
| if config is None: | ||
| msg = f"Provider {name!r} not found" | ||
| logger.warning(PROVIDER_UPDATED, provider=name, error=msg) |
There was a problem hiding this comment.
The log event PROVIDER_UPDATED is used here when a provider is not found during a get_provider operation. This is misleading, as this is a read operation, not an update. Using an event name associated with a write operation for a failed read can cause confusion when analyzing logs. Consider using a more appropriate log message or a different event constant.
| logger.warning(PROVIDER_UPDATED, provider=name, error=msg) | |
| logger.warning("Provider not found during get operation", provider=name, error=msg) |
There was a problem hiding this comment.
Actionable comments posted: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/src/stores/providers.ts (1)
17-24:⚠️ Potential issue | 🟠 MajorActually strip secret-bearing fields before writing providers into Pinia state.
This helper only filters prototype-pollution keys. If the backend ever accidentally serializes
api_key,oauth_client_secret, orcustom_header_value, those values are copied into reactive state unchanged, which defeats the defense-in-depth this helper is supposed to provide.🛡️ Proposed fix
function sanitizeProviders(raw: Record<string, ProviderConfig>): Record<string, ProviderConfig> { const result = Object.create(null) as Record<string, ProviderConfig> for (const [key, provider] of Object.entries(raw)) { if (UNSAFE_KEYS.has(key)) continue - result[key] = provider + const sanitized = { ...(provider as Record<string, unknown>) } + delete sanitized.api_key + delete sanitized.oauth_client_secret + delete sanitized.custom_header_value + result[key] = sanitized as ProviderConfig } return result }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/stores/providers.ts` around lines 17 - 24, sanitizeProviders currently only filters prototype-pollution keys (UNSAFE_KEYS) but copies provider objects with sensitive fields intact; update sanitizeProviders to create a shallow copy of each provider (referencing ProviderConfig) and explicitly delete or omit secret-bearing fields such as "api_key", "oauth_client_secret", "custom_header_value" (and any other secret keys defined in your model) before assigning to result[key]; ensure you do not mutate the original raw input and keep UNSAFE_KEYS filtering in place so result contains only sanitized provider objects.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/synthorg/api/dto.py`:
- Around line 633-639: The validator _validate_success_error_consistency on
TestConnectionResponse currently only forbids an error when success is true;
update it to also require an error when success is false so failed tests cannot
have error=None. In the method (on the TestConnectionResponse model) add a
branch that raises a ValueError with a clear message like "failed test must
include an error" if self.success is False and self.error is None, while keeping
the existing check that a successful test must not have an error.
- Around line 708-714: The has_custom_header flag currently checks only
config.custom_header_value; update its expression so it returns true only when
both config.custom_header_name and config.custom_header_value are non-None
(i.e., require both name and value). Locate the construction using
has_custom_header in the DTO (the tuple/object where has_oauth_credentials is
set) and change the has_custom_header assignment to check both
config.custom_header_name and config.custom_header_value so it accurately
represents presence of a complete custom header.
In `@src/synthorg/config/schema.py`:
- Around line 191-192: In the branch where self.auth_type ==
AuthType.CUSTOM_HEADER add an explicit type annotation to the local variable
`missing` (i.e., declare `missing: list[str]`) to match the earlier `missing:
list[str]` annotation and keep consistent typing in the same method (where
`AuthType.CUSTOM_HEADER` is handled).
In `@src/synthorg/providers/drivers/litellm_driver.py`:
- Line 311: The _build_kwargs method is too complex (noqa: C901); extract the
authentication-specific logic into a private helper named _build_auth_kwargs
that returns a dict of auth-related kwargs (e.g., api_key, username/password,
token, ssl options) and any conditional branches currently inside _build_kwargs;
then update _build_kwargs to call _build_auth_kwargs and merge its return into
the overall kwargs so behavior is identical, keep the helper private and
well-named, move unit-testable branches (like credential selection and
environment-variable fallbacks) into _build_auth_kwargs, and ensure existing
callers of _build_kwargs continue to receive the same final kwargs shape.
In `@src/synthorg/providers/management/service.py`:
- Around line 355-368: The router construction currently lives outside the
validation try/except so any exception from self._build_router(new_providers)
escapes as a raw error; move the call to self._build_router(new_providers) into
the same try block that calls ProviderRegistry.from_config(new_providers) (or
wrap it in the same except) and on any exception capture the exception, log the
same warning (using PROVIDER_UPDATED, error=str(exc),
provider_count=len(new_providers)) via logger.warning, and re-raise a
ProviderValidationError with the original exception as cause so router build
failures are surfaced as ProviderValidationError.
- Around line 464-480: When auth_type is changed away from API_KEY we must also
clear any stale api_key in the stored config; update the auth_type handling in
the function that builds the updates dict (references: request.auth_type,
AuthType, updates) so that if request.auth_type is not None and
request.auth_type != AuthType.API_KEY you set updates["api_key"] = None unless
the caller explicitly provided request.api_key (the existing api_key handling
using request.api_key or request.clear_api_key should still apply, but the
auth_type branch must proactively clear api_key when switching away from API_KEY
to prevent retaining an obsolete secret).
- Around line 458-482: The current code builds an updates dict and returns
existing.model_copy(update=updates), which skips Pydantic validators; replace
this with a full re-validation by merging existing.model_dump() with updates and
calling the model class's model_validate on the merged data (e.g., merged =
existing.model_dump(); merged.update(updates); return
existing.__class__.model_validate(merged)). This preserves the same merged
fields (including handling from _UPDATE_FIELDS, auth_type logic,
api_key/clear_api_key) but ensures validators like _validate_auth_fields and
_validate_unique_model_identifiers run.
In `@src/synthorg/providers/presets.py`:
- Around line 42-75: Replace concrete vendor names in the ProviderPreset
instances (currently using name/display_name values "ollama", "lm-studio",
"openrouter", "vllm") with generic identifiers and descriptions to comply with
the no-vendor-names rule; update the display_name and description fields for
each ProviderPreset (and any related default_base_url text if it embeds a vendor
name) to neutral names like "local-llm", "local-dev-llm", "api-gateway",
"high-throughput-llm" or similar, and ensure auth_type, driver, and
default_models remain unchanged so behavior is preserved.
In `@tests/unit/config/test_provider_auth.py`:
- Around line 99-105: Add a new unit test that complements
test_api_key_auth_no_extra_requirements by asserting that ProviderConfig stores
a provided API key: create a test function (e.g., test_api_key_auth_stores_key)
that constructs ProviderConfig(driver="litellm", auth_type=AuthType.API_KEY,
api_key="sk-test-key") and then asserts config.auth_type == AuthType.API_KEY and
config.api_key == "sk-test-key"; reference ProviderConfig and AuthType to locate
the class and enum to exercise.
In `@tests/unit/providers/drivers/test_litellm_auth.py`:
- Around line 88-97: Add a unit test to cover the OAuth case when no pre-fetched
token is provided: create a config via _make_config with
auth_type=AuthType.OAUTH and api_key=None (keeping oauth_token_url,
oauth_client_id, oauth_client_secret set), call _build_kwargs(config), and
assert that "api_key" is not present in the returned kwargs (i.e., verify
_build_kwargs omits the api_key when None).
In `@tests/unit/providers/management/test_service.py`:
- Around line 323-341: Update the test fixtures to remove vendor-branded names
by replacing any occurrences of "ollama", "my-ollama", and "ollama-local" used
in the CreateFromPresetRequest within test_create_from_preset and
test_create_from_preset_with_overrides with generic names such as
"test-local-preset" for preset_name and "test-provider" (or "test-instance") for
name and base_url overrides; ensure the assertions still validate the same
behavior (e.g., result.auth_type and result.base_url) and apply the same
renaming to the other affected tests that reference the branded presets (the
later test block noted in the comment).
In `@web/src/__tests__/components/providers/ProviderFormDialog.test.ts`:
- Around line 15-23: Replace the brittle import/name tests with behavior-driven
unit tests for ProviderFormDialog: mount ProviderFormDialog (use
mount/shallowMount from `@vue/test-utils`), assert initial rendered form state and
visible fields, simulate user interactions by setting inputs via
wrapper.find/input.setValue and triggering submit, verify validation messages
appear when inputs are invalid, assert the emitted events (e.g., 'close' and
'success') and that the 'success' event payload matches the expected submit
data, and add a test for cancel/close interaction to ensure it emits the close
event; target the ProviderFormDialog component and use wrapper.emitted() to
assert payloads.
In `@web/src/__tests__/components/providers/ProviderTestButton.test.ts`:
- Around line 46-47: The test currently only checks that
providersApi.testConnection was called; tighten it by asserting the exact call
arguments to catch wiring regressions—modify the test that renders
ProviderTestButton (or the surrounding test function) to use
expect(providersApi.testConnection).toHaveBeenCalledWith(<expectedProviderName>,
<expectedOptions>) (or toHaveBeenCalledTimes(1) combined with
toHaveBeenCalledWith) so the first argument matches the intended provider
id/name and any other expected params are validated.
In `@web/src/components/providers/ProviderCard.vue`:
- Around line 80-103: Wrap the delete confirmation UI update in an element with
aria-live="polite" so screen readers are notified when confirmingDelete changes;
update the template that renders the conditional block (the
confirmingDelete-controlled fragment in ProviderCard.vue) to include an
announcing container (e.g., a <div> around the template content) with
aria-live="polite" and ensure the status text/button labels updated by
handleDelete and cancelDelete are inside that container so the change is
announced to assistive tech.
In `@web/src/components/providers/ProviderFormDialog.vue`:
- Around line 185-188: The API Key input shown when authType === 'oauth' is
confusing; update the ProviderFormDialog.vue near the InputText with id
"pf-api-key" (v-model="apiKey") to conditionally render a short helper/hint
explaining that for OAuth this field accepts a pre-fetched token as an MVP
workaround and that full client-credentials flow is planned, e.g., add a small
descriptive <span> or <p> element beneath the label that only appears when
authType === 'oauth' to clarify usage and expected format. Ensure the helper is
only shown for the OAuth case and keeps existing behavior for authType ===
'api_key'.
- Around line 113-127: The current update branch skips setting data.base_url
when baseUrl.value is an empty string; change the conditional to detect changes
against the original value instead of truthiness so clearing is allowed: replace
the truthy check "if (baseUrl.value) data.base_url = baseUrl.value" with a
comparison "if (baseUrl.value !== props.providerConfig?.base_url) data.base_url
= baseUrl.value" (same pattern may be applied to other fields if needed) so
UpdateProviderRequest includes an explicit empty value when the user clears the
field before emit('save', data).
- Around line 91-95: The computed isValid in ProviderFormDialog.vue only checks
name and driver but must also enforce auth-type-specific required fields; update
the isValid computed to inspect the selected authType (authType.value or
props.authType) and, using a switch/if, validate the corresponding refs (e.g.,
for 'oauth' require clientId.value.trim() and clientSecret.value.trim(); for
'basic' require username.value.trim() and password.value.trim(); for 'token'
require token.value.trim(); for 'none' allow) in addition to the existing
name/driver checks, returning false if any required field is missing so the form
cannot submit incomplete auth data.
---
Outside diff comments:
In `@web/src/stores/providers.ts`:
- Around line 17-24: sanitizeProviders currently only filters
prototype-pollution keys (UNSAFE_KEYS) but copies provider objects with
sensitive fields intact; update sanitizeProviders to create a shallow copy of
each provider (referencing ProviderConfig) and explicitly delete or omit
secret-bearing fields such as "api_key", "oauth_client_secret",
"custom_header_value" (and any other secret keys defined in your model) before
assigning to result[key]; ensure you do not mutate the original raw input and
keep UNSAFE_KEYS filtering in place so result contains only sanitized provider
objects.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: a72041b5-915e-468e-9c30-2bcf95fbff07
📒 Files selected for processing (32)
CLAUDE.mddocs/design/operations.mdsrc/synthorg/api/controllers/providers.pysrc/synthorg/api/dto.pysrc/synthorg/api/state.pysrc/synthorg/config/schema.pysrc/synthorg/observability/events/api.pysrc/synthorg/observability/events/provider.pysrc/synthorg/providers/drivers/litellm_driver.pysrc/synthorg/providers/enums.pysrc/synthorg/providers/errors.pysrc/synthorg/providers/management/__init__.pysrc/synthorg/providers/management/service.pysrc/synthorg/providers/presets.pytests/unit/api/controllers/test_providers.pytests/unit/config/conftest.pytests/unit/config/test_provider_auth.pytests/unit/providers/drivers/test_litellm_auth.pytests/unit/providers/management/__init__.pytests/unit/providers/management/test_service.pytests/unit/providers/test_presets.pyweb/src/__tests__/components/providers/ProviderCard.test.tsweb/src/__tests__/components/providers/ProviderFormDialog.test.tsweb/src/__tests__/components/providers/ProviderTestButton.test.tsweb/src/__tests__/stores/providers.test.tsweb/src/api/endpoints/providers.tsweb/src/api/types.tsweb/src/components/providers/ProviderCard.vueweb/src/components/providers/ProviderFormDialog.vueweb/src/components/providers/ProviderTestButton.vueweb/src/stores/providers.tsweb/src/views/SettingsPage.vue
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Test (Python 3.14)
- GitHub Check: Build Backend
- GitHub Check: Build Sandbox
- GitHub Check: Build Web
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (6)
web/**/*.{js,ts,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
web/**/*.{js,ts,vue}: Use fast-check for property-based testing (fc.assert+fc.property)
NEVER use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, docstrings, comments, tests, or config examples. Use generic names:example-provider,example-large-001, etc.
Files:
web/src/__tests__/stores/providers.test.tsweb/src/components/providers/ProviderFormDialog.vueweb/src/components/providers/ProviderTestButton.vueweb/src/__tests__/components/providers/ProviderCard.test.tsweb/src/components/providers/ProviderCard.vueweb/src/api/types.tsweb/src/stores/providers.tsweb/src/api/endpoints/providers.tsweb/src/__tests__/components/providers/ProviderTestButton.test.tsweb/src/__tests__/components/providers/ProviderFormDialog.test.tsweb/src/views/SettingsPage.vue
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/**/*.py: Always read the relevantdocs/design/page before implementing any feature or planning any issue
If implementation deviates from the design spec, alert the user and explain why — user decides whether to proceed or update the spec. Do NOT silently diverge — every deviation needs explicit user approval
Nofrom __future__ import annotations— Python 3.14 has PEP 649 native lazy annotations
Useexcept A, B:syntax without parentheses (PEP 758) — ruff enforces this on Python 3.14
All public functions must have type hints; use mypy strict mode for type checking
Use Google-style docstrings, required on all public classes and functions (enforced by ruff D rules)
Create new objects for immutability, never mutate existing ones. For non-Pydantic internal collections (registries,BaseTool), usecopy.deepcopy()at construction +MappingProxyTypewrapping for read-only enforcement
Fordict/listfields in frozen Pydantic models, rely onfrozen=Truefor field reassignment prevention andcopy.deepcopy()at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence)
Use frozen Pydantic models (frozen=True,ConfigDict) for config and identity; use separate mutable-via-copy models (usingmodel_copy(update=...)) for runtime state that evolves
Use Pydantic v2 conventions:@computed_fieldfor derived values instead of storing + validating redundant fields (e.g.,TokenUsage.total_tokens)
UseNotBlankStr(fromcore.types) for all identifier/name fields — including optional (NotBlankStr | None) and tuple (tuple[NotBlankStr, ...]) variants — instead of manual whitespace validators
Preferasyncio.TaskGroupfor fan-out/fan-in parallel operations in new code (e.g., multiple tool invocations, parallel agent calls). Prefer structured concurrency over barecreate_task
Keep functions under 50 lines and files under 800 lines
Handle errors explicitly, never silently swallow excepti...
Files:
src/synthorg/providers/errors.pysrc/synthorg/api/state.pysrc/synthorg/observability/events/provider.pysrc/synthorg/api/dto.pysrc/synthorg/observability/events/api.pysrc/synthorg/config/schema.pysrc/synthorg/api/controllers/providers.pysrc/synthorg/providers/management/__init__.pysrc/synthorg/providers/enums.pysrc/synthorg/providers/presets.pysrc/synthorg/providers/drivers/litellm_driver.pysrc/synthorg/providers/management/service.py
src/synthorg/providers/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/providers/**/*.py: All provider calls go throughBaseCompletionProviderwhich applies retry + rate limiting automatically. Never implement retry logic in driver subclasses or calling code — it's handled by the base class
SetRetryConfigandRateLimiterConfigper-provider inProviderConfig; never implement retry logic at the call site
Files:
src/synthorg/providers/errors.pysrc/synthorg/providers/management/__init__.pysrc/synthorg/providers/enums.pysrc/synthorg/providers/presets.pysrc/synthorg/providers/drivers/litellm_driver.pysrc/synthorg/providers/management/service.py
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
NEVER use
cdin Bash commands within code or documentation — the working directory is already set to the project root. Use absolute paths or run commands directly
Files:
src/synthorg/providers/errors.pytests/unit/api/controllers/test_providers.pysrc/synthorg/api/state.pysrc/synthorg/observability/events/provider.pysrc/synthorg/api/dto.pytests/unit/config/conftest.pysrc/synthorg/observability/events/api.pytests/unit/providers/drivers/test_litellm_auth.pysrc/synthorg/config/schema.pytests/unit/providers/test_presets.pysrc/synthorg/api/controllers/providers.pysrc/synthorg/providers/management/__init__.pysrc/synthorg/providers/enums.pytests/unit/providers/management/test_service.pytests/unit/config/test_provider_auth.pysrc/synthorg/providers/presets.pysrc/synthorg/providers/drivers/litellm_driver.pysrc/synthorg/providers/management/service.py
**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
For Markdown documentation, always read the relevant
docs/design/page when implementing features to ensure alignment with design specifications
Files:
docs/design/operations.mdCLAUDE.md
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Use pytest markers:@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e,@pytest.mark.slow
Maintain 80% minimum code coverage (enforced in CI)
Useasyncio_mode = "auto"— no manual@pytest.mark.asyncioneeded
Set 30 second timeout per test
ALWAYS include-n autowhen running pytest with pytest-xdist, never run tests sequentially
Prefer@pytest.mark.parametrizefor testing similar cases
NEVER use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in tests. Use generic names:test-provider,test-small-001, etc. Vendor names may only appear in: (1) Operations design page provider list (docs/design/operations.md), (2).claude/skill/agent files, (3) third-party import paths/module names
Use Hypothesis for property-based testing with@given+@settings. Control viaHYPOTHESIS_PROFILEenv var:ci(200 examples, default) ordev(1000 examples). Run dev profile:HYPOTHESIS_PROFILE=dev uv run python -m pytest tests/ -m unit -n auto -k properties
NEVER skip, dismiss, or ignore flaky tests — always fix them fully and fundamentally. For timing-sensitive tests, mocktime.monotonic()andasyncio.sleep()to make them deterministic instead of widening timing margins
Files:
tests/unit/api/controllers/test_providers.pytests/unit/config/conftest.pytests/unit/providers/drivers/test_litellm_auth.pytests/unit/providers/test_presets.pytests/unit/providers/management/test_service.pytests/unit/config/test_provider_auth.py
🧠 Learnings (22)
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to tests/**/*.py : Tests must use test-provider, test-small-001, etc. for vendor-agnostic test data.
Applied to files:
web/src/__tests__/stores/providers.test.tstests/unit/api/controllers/test_providers.pytests/unit/providers/test_presets.pytests/unit/providers/management/test_service.py
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to src/synthorg/providers/**/*.py : Retryable errors (is_retryable=True): RateLimitError, ProviderTimeoutError, ProviderConnectionError, ProviderInternalError. Non-retryable errors raise immediately without retry. RetryExhaustedError signals that all retries failed — the engine layer catches this to trigger fallback chains.
Applied to files:
src/synthorg/providers/errors.pysrc/synthorg/api/dto.pysrc/synthorg/providers/drivers/litellm_driver.py
📚 Learning: 2026-03-17T22:48:14.482Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:48:14.482Z
Learning: Applies to src/synthorg/**/*.py : Retryable errors (`is_retryable=True`): `RateLimitError`, `ProviderTimeoutError`, `ProviderConnectionError`, `ProviderInternalError`. Non-retryable errors raise immediately without retry. `RetryExhaustedError` signals all retries failed
Applied to files:
src/synthorg/providers/errors.pysrc/synthorg/api/dto.pysrc/synthorg/providers/drivers/litellm_driver.py
📚 Learning: 2026-03-17T22:48:14.482Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:48:14.482Z
Learning: Applies to src/synthorg/providers/**/*.py : Set `RetryConfig` and `RateLimiterConfig` per-provider in `ProviderConfig`; never implement retry logic at the call site
Applied to files:
src/synthorg/providers/errors.pysrc/synthorg/api/state.pysrc/synthorg/observability/events/provider.pysrc/synthorg/api/dto.pytests/unit/config/conftest.pysrc/synthorg/config/schema.pysrc/synthorg/providers/management/service.py
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to src/synthorg/providers/**/*.py : RetryConfig and RateLimiterConfig are set per-provider in ProviderConfig.
Applied to files:
src/synthorg/providers/errors.pysrc/synthorg/api/state.pysrc/synthorg/observability/events/provider.pysrc/synthorg/api/dto.pytests/unit/config/conftest.pysrc/synthorg/config/schema.pysrc/synthorg/providers/management/service.py
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to src/synthorg/providers/**/*.py : Rate limiter respects RateLimitError.retry_after from providers — automatically pauses future requests.
Applied to files:
src/synthorg/providers/errors.py
📚 Learning: 2026-03-17T22:48:14.482Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:48:14.482Z
Learning: Applies to src/synthorg/**/*.py : Handle errors explicitly, never silently swallow exceptions
Applied to files:
src/synthorg/providers/errors.pysrc/synthorg/api/dto.py
📚 Learning: 2026-03-17T22:48:14.482Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:48:14.482Z
Learning: Applies to src/synthorg/**/*.py : Rate limiter respects `RateLimitError.retry_after` from providers — automatically pauses future requests
Applied to files:
src/synthorg/providers/errors.py
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to src/synthorg/providers/**/*.py : All provider calls go through BaseCompletionProvider which applies retry + rate limiting automatically. Never implement retry logic in driver subclasses or calling code — it's handled by the base class.
Applied to files:
src/synthorg/providers/errors.pysrc/synthorg/providers/drivers/litellm_driver.py
📚 Learning: 2026-03-17T22:48:14.482Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:48:14.482Z
Learning: Applies to src/synthorg/providers/**/*.py : All provider calls go through `BaseCompletionProvider` which applies retry + rate limiting automatically. Never implement retry logic in driver subclasses or calling code — it's handled by the base class
Applied to files:
src/synthorg/providers/errors.pysrc/synthorg/providers/drivers/litellm_driver.py
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to src/synthorg/**/*.py : Event names: always use constants from domain-specific modules under synthorg.observability.events (e.g., PROVIDER_CALL_START from events.provider, BUDGET_RECORD_ADDED from events.budget, etc.). Import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`.
Applied to files:
src/synthorg/api/state.pysrc/synthorg/observability/events/provider.pysrc/synthorg/observability/events/api.pysrc/synthorg/config/schema.py
📚 Learning: 2026-03-17T22:48:14.482Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:48:14.482Z
Learning: Applies to src/synthorg/**/*.py : Always use event name constants from domain-specific modules under `synthorg.observability.events` (e.g., `API_REQUEST_STARTED` from `events.api`, `TOOL_INVOKE_START` from `events.tool`). Import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`
Applied to files:
src/synthorg/api/state.pysrc/synthorg/observability/events/provider.pysrc/synthorg/observability/events/api.py
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to web/src/components/** : Vue components organized by feature (agents/, approvals/, budget/, common/, dashboard/, layout/, messages/, org-chart/, tasks/).
Applied to files:
web/src/components/providers/ProviderCard.vueCLAUDE.md
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to **/*.py : Models: Pydantic v2 (BaseModel, model_validator, computed_field, ConfigDict). Use computed_field for derived values instead of storing + validating redundant fields. Use NotBlankStr (from core.types) for all identifier/name fields — including optional (NotBlankStr | None) and tuple (tuple[NotBlankStr, ...]) variants — instead of manual whitespace validators.
Applied to files:
src/synthorg/api/dto.py
📚 Learning: 2026-03-17T22:48:14.482Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:48:14.482Z
Learning: Applies to src/synthorg/**/*.py : Validate at system boundaries (user input, external APIs, config files)
Applied to files:
src/synthorg/api/dto.py
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising.
Applied to files:
src/synthorg/api/dto.py
📚 Learning: 2026-03-17T22:48:14.482Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:48:14.482Z
Learning: Applies to src/synthorg/**/*.py : Always read the relevant `docs/design/` page before implementing any feature or planning any issue
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to web/** : Web dashboard: Node.js 20+, dependencies in web/package.json (Vue 3, PrimeVue, Tailwind CSS, Pinia, VueFlow, ECharts, Axios, vue-draggable-plus, Vitest, fast-check, ESLint, vue-tsc).
Applied to files:
CLAUDE.mdweb/src/views/SettingsPage.vue
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to docker/{Dockerfile*,compose.yml} : Docker: Backend uses 3-stage build (builder → setup → distroless runtime), Chainguard Python, non-root (UID 65532), CIS-hardened. Web uses nginxinc/nginx-unprivileged, Vue 3 SPA with PrimeVue + Tailwind CSS, SPA routing, API/WebSocket proxy to backend.
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-17T22:48:14.482Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:48:14.482Z
Learning: Applies to tests/**/*.py : NEVER use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in tests. Use generic names: `test-provider`, `test-small-001`, etc. Vendor names may only appear in: (1) Operations design page provider list (`docs/design/operations.md`), (2) `.claude/` skill/agent files, (3) third-party import paths/module names
Applied to files:
tests/unit/providers/management/test_service.py
📚 Learning: 2026-03-17T22:48:14.482Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:48:14.482Z
Learning: Applies to src/synthorg/**/*.py : Use frozen Pydantic models (`frozen=True`, `ConfigDict`) for config and identity; use separate mutable-via-copy models (using `model_copy(update=...)`) for runtime state that evolves
Applied to files:
src/synthorg/providers/presets.py
📚 Learning: 2026-03-17T22:48:14.482Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:48:14.482Z
Learning: Applies to src/synthorg/**/*.py : For `dict`/`list` fields in frozen Pydantic models, rely on `frozen=True` for field reassignment prevention and `copy.deepcopy()` at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence)
Applied to files:
src/synthorg/providers/presets.py
🧬 Code graph analysis (13)
web/src/__tests__/stores/providers.test.ts (1)
web/src/api/types.ts (2)
ProviderConfig(464-472)ProviderPreset(515-523)
web/src/__tests__/components/providers/ProviderCard.test.ts (2)
src/synthorg/config/schema.py (1)
ProviderConfig(85-230)web/src/api/types.ts (1)
ProviderConfig(464-472)
tests/unit/api/controllers/test_providers.py (3)
src/synthorg/api/dto.py (1)
to_provider_response(691-714)src/synthorg/config/schema.py (1)
ProviderConfig(85-230)src/synthorg/providers/enums.py (1)
AuthType(6-12)
src/synthorg/api/state.py (2)
tests/unit/providers/management/test_service.py (3)
service(89-99)settings_service(57-67)app_state(71-85)src/synthorg/providers/management/service.py (1)
ProviderManagementService(48-400)
web/src/api/types.ts (3)
src/synthorg/providers/enums.py (1)
AuthType(6-12)src/synthorg/api/dto.py (5)
CreateProviderRequest(521-557)UpdateProviderRequest(560-601)TestConnectionRequest(604-613)TestConnectionResponse(616-639)CreateFromPresetRequest(666-688)src/synthorg/providers/presets.py (1)
ProviderPreset(16-37)
src/synthorg/api/dto.py (3)
src/synthorg/config/schema.py (2)
ProviderConfig(85-230)ProviderModelConfig(43-82)web/src/api/types.ts (8)
ProviderConfig(464-472)ProviderModelConfig(452-459)AuthType(450-450)CreateProviderRequest(474-487)UpdateProviderRequest(489-502)TestConnectionRequest(504-506)TestConnectionResponse(508-513)CreateFromPresetRequest(525-531)src/synthorg/providers/enums.py (1)
AuthType(6-12)
web/src/stores/providers.ts (5)
src/synthorg/config/schema.py (1)
ProviderConfig(85-230)web/src/api/types.ts (7)
ProviderConfig(464-472)ProviderPreset(515-523)CreateProviderRequest(474-487)UpdateProviderRequest(489-502)TestConnectionRequest(504-506)TestConnectionResponse(508-513)CreateFromPresetRequest(525-531)src/synthorg/providers/presets.py (1)
ProviderPreset(16-37)web/src/utils/errors.ts (1)
getErrorMessage(17-69)web/src/api/endpoints/providers.ts (4)
createProvider(35-38)updateProvider(40-43)deleteProvider(45-47)createFromPreset(59-62)
src/synthorg/config/schema.py (2)
src/synthorg/providers/enums.py (1)
AuthType(6-12)web/src/api/types.ts (1)
AuthType(450-450)
tests/unit/providers/test_presets.py (1)
src/synthorg/providers/presets.py (2)
get_preset(84-93)list_presets(96-102)
src/synthorg/providers/enums.py (1)
web/src/api/types.ts (1)
AuthType(450-450)
tests/unit/config/test_provider_auth.py (3)
src/synthorg/config/schema.py (1)
ProviderConfig(85-230)web/src/api/types.ts (2)
ProviderConfig(464-472)AuthType(450-450)src/synthorg/providers/enums.py (1)
AuthType(6-12)
src/synthorg/providers/presets.py (3)
src/synthorg/config/schema.py (1)
ProviderModelConfig(43-82)web/src/api/types.ts (2)
ProviderModelConfig(452-459)AuthType(450-450)src/synthorg/providers/enums.py (1)
AuthType(6-12)
src/synthorg/providers/drivers/litellm_driver.py (3)
src/synthorg/providers/enums.py (2)
AuthType(6-12)StreamEventType(34-41)web/src/api/types.ts (1)
AuthType(450-450)tests/unit/providers/drivers/test_litellm_auth.py (1)
_build_kwargs(43-50)
🪛 GitHub Check: Dashboard Lint
web/src/api/endpoints/providers.ts
[warning] 20-20:
Generic Object Injection Sink
🔇 Additional comments (22)
src/synthorg/providers/enums.py (1)
6-12: LGTM!The
AuthTypeenum is well-defined with a clear docstring and values that align with the frontend TypeScript type ('api_key' | 'oauth' | 'custom_header' | 'none'inweb/src/api/types.ts).src/synthorg/observability/events/api.py (1)
59-60: LGTM!New event constants follow the established naming convention (
api.<domain>.<action>) and complement the provider management CRUD operations for conflict and validation failure scenarios.src/synthorg/providers/errors.py (1)
165-180: LGTM!The new error classes follow the established pattern and correctly set
is_retryable = False. These are configuration/presence errors that should not be retried, aligning with the coding guidelines for non-retryable errors.src/synthorg/config/schema.py (1)
172-205: LGTM!The auth validation logic correctly enforces required fields based on
auth_typeand properly logs warnings before raising. Secrets are appropriately marked withrepr=Falseto prevent leakage in logs/tracebacks.tests/unit/config/test_provider_auth.py (1)
10-116: LGTM!Comprehensive test coverage for auth type validation including backward compatibility, required field validation for OAuth and custom header auth types, and optional field handling.
src/synthorg/providers/drivers/litellm_driver.py (1)
331-349: LGTM!The match statement handles all
AuthTypevalues correctly. The MVP OAuth implementation (passing throughapi_key) is clearly documented. The defensive check for bothcustom_header_nameandcustom_header_valuebefore settingextra_headersprovides runtime safety even though the config validator enforces this.tests/unit/providers/drivers/test_litellm_auth.py (1)
53-110: LGTM!Comprehensive test coverage for the multi-auth
_build_kwargsbehavior across allAuthTypevariants, including edge cases for missing keys and base URL handling.src/synthorg/api/dto.py (1)
496-518: Strong provider-name boundary validation.Good guardrails on format + reserved names; this keeps path parameters predictable and avoids route collisions.
tests/unit/config/conftest.py (1)
54-54: Fixture default is aligned with new provider auth schema.Nice update to keep generated provider configs consistent with the new auth-aware model.
tests/unit/api/controllers/test_providers.py (1)
72-91: Good security-focused contract coverage for provider responses.These assertions meaningfully protect against credential leakage regressions while validating the new
ProviderResponseshape.Also applies to: 171-179
src/synthorg/providers/presets.py (1)
79-102: Good immutability posture for preset lookup/read APIs.Using
MappingProxyType+ tuple return keeps this registry safely read-only.web/src/api/types.ts (1)
450-531: Frontend contracts are well aligned with the new backend provider DTOs.The secret-free
ProviderConfigshape and auth-specific request types are clear and practical for UI flows.tests/unit/providers/test_presets.py (1)
12-45: Nice focused coverage for preset invariants and immutability.These tests protect key guarantees (lookup behavior, uniqueness, frozen model behavior) with low maintenance cost.
web/src/__tests__/stores/providers.test.ts (1)
1-133: LGTM! Solid test coverage for provider store CRUD operations.The test file properly exercises the core store functionality with compliant test data naming (
test-provider,test-model-001). Consider adding tests forupdateProviderandtestConnectionactions to complete the CRUD coverage.docs/design/operations.md (1)
103-113: LGTM! Clear and comprehensive documentation of the new provider management surface.The section accurately documents runtime CRUD, connection testing, presets, hot-reload semantics, auth types, and credential safety. This aligns well with the PR objectives and implementation.
src/synthorg/api/state.py (2)
121-130: LGTM! Proper deferred initialization following established patterns.The initialization follows the same conditional pattern as
ConfigResolverand correctly wires up the service with required dependencies. The circular reference betweenAppStateandProviderManagementServiceis intentional and necessary for the hot-reload swap mechanism.
277-288: LGTM! Property accessors follow the established_require_servicepattern.Consistent with other service accessors in the class.
src/synthorg/providers/management/__init__.py (1)
1-5: LGTM!Clean module re-export with appropriate
__all__declaration.web/src/__tests__/components/providers/ProviderCard.test.ts (1)
1-95: LGTM! Comprehensive component tests with proper mock data.Good coverage of rendering and interaction behaviors, including the two-step delete confirmation flow. Test data uses compliant generic names (
test-provider,test-model-001).src/synthorg/observability/events/provider.py (1)
43-49: LGTM! Well-structured event constants following established naming patterns.The new management events (
provider.management.*) are consistent with the existing event hierarchy and cover all provider lifecycle operations.web/src/components/providers/ProviderTestButton.vue (1)
1-71: LGTM! Well-structured component with proper lifecycle cleanup.Good implementation details:
- Timer cleanup in
onUnmountedprevents memory leaks- 10-second auto-clear of results improves UX
- Error handling with truncation and tooltip for full message
- Loading state properly managed
web/src/components/providers/ProviderCard.vue (1)
1-106: LGTM! Clean component implementation with good UX patterns.The component properly:
- Displays provider metadata with appropriate badges
- Implements two-step delete confirmation
- Integrates ProviderTestButton for connection testing
- Uses proper TypeScript types and Vue 3 composition API
| elif self.auth_type == AuthType.CUSTOM_HEADER: | ||
| missing = [] |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Minor: Add explicit type annotation for consistency.
Line 176 annotates missing: list[str], but line 192 omits the annotation. Consider adding it for consistency.
💡 Suggested fix
elif self.auth_type == AuthType.CUSTOM_HEADER:
- missing = []
+ missing: list[str] = []
if self.custom_header_name is None:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| elif self.auth_type == AuthType.CUSTOM_HEADER: | |
| missing = [] | |
| elif self.auth_type == AuthType.CUSTOM_HEADER: | |
| missing: list[str] = [] | |
| if self.custom_header_name is None: |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/synthorg/config/schema.py` around lines 191 - 192, In the branch where
self.auth_type == AuthType.CUSTOM_HEADER add an explicit type annotation to the
local variable `missing` (i.e., declare `missing: list[str]`) to match the
earlier `missing: list[str]` annotation and keep consistent typing in the same
method (where `AuthType.CUSTOM_HEADER` is handled).
| # ── Request building ───────────────────────────────────────── | ||
|
|
||
| def _build_kwargs( | ||
| def _build_kwargs( # noqa: C901 |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider extracting auth kwargs building to a separate method.
The noqa: C901 suggests elevated complexity. Extracting auth configuration into a private helper (e.g., _build_auth_kwargs) would improve readability and testability.
♻️ Proposed refactor
+ def _build_auth_kwargs(self) -> dict[str, Any]:
+ """Build authentication kwargs based on auth_type."""
+ match self._config.auth_type:
+ case AuthType.API_KEY:
+ if self._config.api_key is not None:
+ return {"api_key": self._config.api_key}
+ case AuthType.OAUTH:
+ if self._config.api_key is not None:
+ return {"api_key": self._config.api_key}
+ case AuthType.CUSTOM_HEADER:
+ if self._config.custom_header_name and self._config.custom_header_value:
+ return {
+ "extra_headers": {
+ self._config.custom_header_name: self._config.custom_header_value,
+ }
+ }
+ case AuthType.NONE:
+ pass
+ return {}
+
- def _build_kwargs( # noqa: C901
+ def _build_kwargs(
self,
...
) -> dict[str, Any]:
"""Build keyword arguments for ``litellm.acompletion``."""
kwargs: dict[str, Any] = {
"model": litellm_model,
"messages": messages_to_dicts(messages),
}
...
- match self._config.auth_type:
- ...
+ kwargs.update(self._build_auth_kwargs())
if self._config.base_url is not None:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/synthorg/providers/drivers/litellm_driver.py` at line 311, The
_build_kwargs method is too complex (noqa: C901); extract the
authentication-specific logic into a private helper named _build_auth_kwargs
that returns a dict of auth-related kwargs (e.g., api_key, username/password,
token, ssl options) and any conditional branches currently inside _build_kwargs;
then update _build_kwargs to call _build_auth_kwargs and merge its return into
the overall kwargs so behavior is identical, keep the helper private and
well-named, move unit-testable branches (like credential selection and
environment-variable fallbacks) into _build_auth_kwargs, and ensure existing
callers of _build_kwargs continue to receive the same final kwargs shape.
| expect(providersApi.testConnection).toHaveBeenCalled() | ||
| }) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Tighten the click test by asserting call arguments, not just call count.
Right now, a wrong provider name could still pass. Verifying the first argument catches wiring regressions.
💡 Example improvement
- expect(providersApi.testConnection).toHaveBeenCalled()
+ expect(providersApi.testConnection).toHaveBeenCalledTimes(1)
+ expect(vi.mocked(providersApi.testConnection).mock.calls[0]?.[0]).toBe(
+ 'test-provider',
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| expect(providersApi.testConnection).toHaveBeenCalled() | |
| }) | |
| expect(providersApi.testConnection).toHaveBeenCalledTimes(1) | |
| expect(vi.mocked(providersApi.testConnection).mock.calls[0]?.[0]).toBe( | |
| 'test-provider', | |
| ) | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/__tests__/components/providers/ProviderTestButton.test.ts` around
lines 46 - 47, The test currently only checks that providersApi.testConnection
was called; tighten it by asserting the exact call arguments to catch wiring
regressions—modify the test that renders ProviderTestButton (or the surrounding
test function) to use
expect(providersApi.testConnection).toHaveBeenCalledWith(<expectedProviderName>,
<expectedOptions>) (or toHaveBeenCalledTimes(1) combined with
toHaveBeenCalledWith) so the first argument matches the intended provider
id/name and any other expected params are validated.
| <div v-if="authType === 'api_key' || authType === 'oauth'"> | ||
| <label for="pf-api-key" class="mb-1 block text-xs text-slate-400">API Key</label> | ||
| <InputText id="pf-api-key" v-model="apiKey" type="password" class="w-full" placeholder="sk-..." /> | ||
| </div> |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding helper text for OAuth API Key field.
The API Key field is shown for OAuth auth type as an MVP workaround (pre-fetched token), but this may confuse users expecting client credentials flow. Consider adding a hint label.
💡 Suggested improvement
<!-- API Key (api_key or oauth) -->
<div v-if="authType === 'api_key' || authType === 'oauth'">
- <label for="pf-api-key" class="mb-1 block text-xs text-slate-400">API Key</label>
+ <label for="pf-api-key" class="mb-1 block text-xs text-slate-400">
+ {{ authType === 'oauth' ? 'Access Token (pre-fetched)' : 'API Key' }}
+ </label>
<InputText id="pf-api-key" v-model="apiKey" type="password" class="w-full" placeholder="sk-..." />
</div>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div v-if="authType === 'api_key' || authType === 'oauth'"> | |
| <label for="pf-api-key" class="mb-1 block text-xs text-slate-400">API Key</label> | |
| <InputText id="pf-api-key" v-model="apiKey" type="password" class="w-full" placeholder="sk-..." /> | |
| </div> | |
| <div v-if="authType === 'api_key' || authType === 'oauth'"> | |
| <label for="pf-api-key" class="mb-1 block text-xs text-slate-400"> | |
| {{ authType === 'oauth' ? 'Access Token (pre-fetched)' : 'API Key' }} | |
| </label> | |
| <InputText id="pf-api-key" v-model="apiKey" type="password" class="w-full" placeholder="sk-..." /> | |
| </div> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/components/providers/ProviderFormDialog.vue` around lines 185 - 188,
The API Key input shown when authType === 'oauth' is confusing; update the
ProviderFormDialog.vue near the InputText with id "pf-api-key"
(v-model="apiKey") to conditionally render a short helper/hint explaining that
for OAuth this field accepts a pre-fetched token as an MVP workaround and that
full client-credentials flow is planned, e.g., add a small descriptive <span> or
<p> element beneath the label that only appears when authType === 'oauth' to
clarify usage and expected format. Ensure the helper is only shown for the OAuth
case and keeps existing behavior for authType === 'api_key'.
…and multi-auth Add full provider lifecycle management -- create, update, delete, and test LLM providers at runtime through the API and web UI. Supports 4 auth types (api_key, oauth, custom_header, none) with encrypted credential storage. Backend: - AuthType enum with 4 variants and ProviderConfig model validator - Provider presets (Ollama, LM Studio, OpenRouter, vLLM) - LiteLLM driver multi-auth via match/case in _build_kwargs - Request/response DTOs with ProviderResponse (secrets stripped) - ProviderManagementService with asyncio.Lock for CRUD serialization - ProviderController CRUD endpoints with write access guards - AppState integration with hot-reload swap Frontend: - TypeScript types for auth, presets, and CRUD operations - API endpoint functions for all provider operations - Pinia store with CRUD actions and generation counter - ProviderCard, ProviderFormDialog, ProviderTestButton components - SettingsPage updated with provider card grid and form dialog Closes #451
Pre-reviewed by 14 agents, 23 findings addressed: - Add ProviderNotFoundError to distinguish 404 from 422 in controller - Build router before persisting to prevent DB/AppState divergence - Sanitize exception messages in test_connection responses - Add WARNING logs before raising in all service + controller error paths - Extract shared _validate_provider_name to eliminate duplication - Add UpdateProviderRequest mutual exclusion validator (api_key vs clear_api_key) - Add TestConnectionResponse success/error consistency validator - Fix has_oauth_credentials to check all 3 OAuth fields - Clear orphaned credentials on auth_type change in _apply_update - Wrap _PRESET_LOOKUP in MappingProxyType (immutability convention) - Remove forward annotation of PROVIDER_PRESETS - Fix dialog close-on-success (not before API resolves) - Add onUnmounted cleanup for ProviderTestButton timer - Add label/input associations for accessibility - Fix preset name auto-fill to not overwrite user input - Use getErrorMessage in ProviderTestButton catch - Add clear_api_key and name validation tests - Fix test assertions (use == not is, import beforeEach) - Update CLAUDE.md Package Structure and api/ description - Update docs/design/operations.md with runtime CRUD, auth types, presets
…d CodeRabbit Critical fixes: - Replace model_copy(update=...) with model_validate() in _apply_update to re-run Pydantic validators on auth_type/model changes - Wrap _build_router() in try/except to prevent raw 500 on router failure - Fix orphaned credential clearing: use direct assignment instead of setdefault, add api_key clear when switching away from API_KEY auth - Fix 6 wrong event constants on error paths (PROVIDER_UPDATED/CREATED/ DELETED used for failures, replaced with PROVIDER_NOT_FOUND/ ALREADY_EXISTS/VALIDATION_FAILED) - test_connection now raises ProviderNotFoundError instead of returning HTTP 200 with success=false for nonexistent providers - Wrap persistence call in try/except with structured error logging Major fixes: - Add auth-type-specific validation to frontend isValid computed - Fix frontend base_url clearing in edit mode (was unclearable) - has_custom_header now checks both name AND value - TestConnectionResponse reverse validator (failed test requires error) - Expose non-secret auth fields in ProviderResponse for edit form UX - Store mutation actions now set error.value on failure - MappingProxyType wrapping for LiteLLMDriver._model_lookup - Controller test_connection endpoint now has error handling - Consolidate split import blocks in providers controller - Update controller docstrings with missing Raises sections Frontend improvements: - OAuth API Key field labeled "Access Token (pre-fetched)" for clarity - Delete confirmation wrapped in aria-live="polite" for screen readers - Test result indicators have role="status" + aria-label - Frontend types updated with non-secret auth fields - sanitizeProviders strips secret-bearing fields (defense-in-depth) - Edit form pre-populates non-secret OAuth/custom_header fields Test improvements: - Auth-type transition tests (4 tests for credential clearing) - _validate_and_persist failure test (registry build error wrapping) - test_connection with explicit model override - Generic exception path test for _do_test_connection - Serialize round-trip test for provider persistence - OAuth without pre-fetched token test - api_key storage verification test - Move integration test from tests/unit/ to tests/integration/ - ProviderCard renders tests consolidated with it.each - ProviderTestButton test now asserts call arguments Documentation: - operations.md YAML example: add auth_type to first provider entry - operations.md Web UI Features: add provider management description - Add missing type annotation for consistency in schema.py - CreateFromPresetRequest docstring: add naming constraints
The `missing: list[str]` annotation in the elif branch of _validate_auth_fields conflicted with the same annotation in the if branch, causing mypy's no-redef rule to fail. Reverted to the original unannotated form since the type is already inferred.
ad80bb0 to
8c229be
Compare
There was a problem hiding this comment.
Actionable comments posted: 9
♻️ Duplicate comments (3)
web/src/__tests__/components/providers/ProviderFormDialog.test.ts (1)
15-23: 🛠️ Refactor suggestion | 🟠 MajorReplace these smoke tests with dialog-behavior coverage.
These assertions only prove the SFC compiled, and
default.__nameis a compiler detail. They will still pass if preset selection, auth-type field switching, validation, submit payloads, or close/success emits break. Mount the dialog and assert rendered state and emitted events instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/__tests__/components/providers/ProviderFormDialog.test.ts` around lines 15 - 23, The current tests only assert the SFC compiles (import and __name) and should be replaced with behavior tests that mount ProviderFormDialog.vue and verify dialog interaction: use Vue Test Utils (mount or shallowMount) to open the dialog component (ProviderFormDialog), simulate user interactions for preset selection and auth-type field switching, exercise validation failure and success paths, trigger submit and close actions, and assert the emitted events (e.g., submit payload and close/success emits) and rendered DOM state after each action; locate ProviderFormDialog.vue in the tests and replace the two smoke tests with mounted-interaction tests that check rendered fields change with auth type, validation messages appear when inputs are invalid, and the exact payload emitted on successful submit.web/src/components/providers/ProviderFormDialog.vue (1)
127-129:⚠️ Potential issue | 🟡 Minor
undefinedstill won't clearbase_url.If the user deletes the base URL,
data.base_url = undefinedis typically dropped during JSON serialization, so the backend sees “no change” instead of an explicit clear. Sendnull(or whatever clear sentinel the API expects) when the field is emptied.🔧 Suggested fix
if (baseUrl.value !== (props.providerConfig?.base_url ?? '')) { - data.base_url = baseUrl.value || undefined + data.base_url = baseUrl.value || null }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/providers/ProviderFormDialog.vue` around lines 127 - 129, The current change uses data.base_url = baseUrl.value || undefined which results in undefined being omitted in JSON and not clearing the backend value; update the assignment in ProviderFormDialog (where baseUrl, props.providerConfig?.base_url and data.base_url are used) to set data.base_url = baseUrl.value === '' ? null : baseUrl.value (or the API's clear sentinel) so an explicit null is sent when the user clears the field.src/synthorg/providers/presets.py (1)
42-75:⚠️ Potential issue | 🟠 MajorResolve the vendor-name policy conflict in the preset catalog.
These preset names, labels, descriptions, and default URLs publish concrete vendor names in a project-owned runtime module and public API surface. That still conflicts with the repo-wide vendor-agnostic rule, so either these presets need genericized naming or
CLAUDE.mdneeds an explicit exemption for provider preset catalogs before merge.As per coding guidelines: “Vendor-agnostic everywhere: NEVER use real vendor names in project-owned code, docstrings, comments, tests, or config examples. Use generic names instead; vendor names may only appear in the operations design page,
.claude/files, or third-party import paths.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synthorg/providers/presets.py` around lines 42 - 75, The preset catalog currently embeds real vendor identifiers in ProviderPreset entries (fields name, display_name, description, default_base_url for ProviderPreset instances named "ollama", "lm-studio", "openrouter", "vllm"), which violates the vendor-agnostic policy; replace these concrete vendor strings with generic equivalents (e.g., name="local-llm", display_name="Local LLM", description="Local inference server", default_base_url="http://localhost:<port>/v1" or similarly neutral labels) across the ProviderPreset definitions, ensuring all occurrences of name, display_name, description, and default_base_url in those ProviderPreset instances are updated consistently, or alternatively add a documented CLAUDE.md exemption if keeping vendor names is intentional.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/design/operations.md`:
- Around line 108-110: Documentation currently implies all CRUD verbs use
POST/PUT/DELETE on /api/v1/providers; update the text to show the correct
resource-specific routes: creation is POST /api/v1/providers, updates use PUT
/api/v1/providers/{name}, and deletes use DELETE /api/v1/providers/{name}; keep
the connection test POST /api/v1/providers/{name}/test and presets endpoints
unchanged but ensure the CRUD lines reference the {name} path for update/delete
so clients are directed to the correct controller surface.
In `@web/src/__tests__/components/providers/ProviderCard.test.ts`:
- Around line 13-30: The mock ProviderConfig object mockConfig is missing the
required nullable auth metadata keys defined on the ProviderConfig type, causing
vue-tsc errors; update mockConfig to include every auth-related fields that
ProviderConfig declares (the nullable auth metadata keys) and set them to null
(or appropriate empty values) so the object satisfies the type—locate the
ProviderConfig type definition and add the missing keys into mockConfig in
ProviderCard.test.ts.
In `@web/src/__tests__/stores/providers.test.ts`:
- Around line 18-35: mockProvider literal does not include the nullable auth
metadata properties required by the ProviderConfig type, causing vue-tsc
failures; update the mockProvider object used in the test to include
oauth_token_url, oauth_client_id, oauth_scope, and custom_header_name properties
(set to null or appropriate test strings) so the object conforms to the
ProviderConfig shape referenced by the tests and type-checks successfully.
In `@web/src/api/types.ts`:
- Around line 493-505: UpdateProviderRequest currently types clearing-able
fields only as optional strings so the UI cannot express "delete this value";
change the nullable fields (base_url, oauth_token_url, oauth_client_id,
oauth_client_secret, oauth_scope, custom_header_name, custom_header_value) to
allow null as well (i.e., string | null | undefined) so the frontend can send
explicit null to clear values to match the backend DTO; keep other fields
(driver, auth_type, api_key, clear_api_key, models) as-is.
In `@web/src/components/providers/ProviderFormDialog.vue`:
- Around line 50-63: The form validation currently requires oauthClientSecret
and customHeaderValue even in edit mode where the API doesn't return secrets;
update the validation used by isValid so that when props.mode === 'edit' and
props.providerConfig exists, missing oauthClientSecret.value or
customHeaderValue.value do not block saving (treat empty secret as "keep
existing") — only require those fields when creating (mode !== 'edit') or when
the user has entered a non-empty value to replace the secret; adjust any checks
referencing oauthClientSecret and customHeaderValue in the watch/submit logic
and the isValid computation accordingly (also apply the same change for the
similar block around the 91-103 logic).
- Around line 107-137: handleSave currently includes all non-empty auth-related
fields regardless of authType; change it so the payload only includes fields for
the active auth mode and, when updating, explicitly clear fields for auth modes
that were previously set but are no longer active. In the create branch
(CreateProviderRequest) only add api_key when authType.value === 'api_key', only
add oauth_* fields when authType.value === 'oauth', and only add custom_header_*
when authType.value === 'custom_header'; in the update branch
(UpdateProviderRequest) compare props.providerConfig?.auth_type to
authType.value and if switching away from a mode add explicit undefined (or set
clearing keys) for that mode’s fields (e.g., set api_key = undefined when
switching off api_key, or oauth_client_id = undefined/oauth_client_secret =
undefined when switching off oauth) and only include non-clearing updates for
the currently active mode; update references in handleSave,
CreateProviderRequest, UpdateProviderRequest, authType, and props.providerConfig
accordingly.
- Around line 23-26: The component currently only emits 'save' with
CreateProviderRequest | UpdateProviderRequest so selectedPreset never triggers
the preset-backed flow; update the defineEmits type to allow a preset-specific
payload (or add a separate event) and change the code path that calls
emit('save', ...) to emit the preset payload when selectedPreset is set.
Concretely, adjust defineEmits (the 'save' overload or add e.g.
'createFromPreset') to include a shape containing preset name and
default_models, and when selectedPreset is chosen emit that shape instead of a
partial CreateProviderRequest/UpdateProviderRequest so the parent can call
store.createFromPreset(selectedPreset, default_models).
In `@web/src/components/providers/ProviderTestButton.vue`:
- Around line 17-22: The component must guard against async completion after
unmount: add a local boolean (e.g., isUnmounted = false) and set it true inside
onUnmounted (alongside clearing clearTimer), then after awaiting
store.testConnection() check the flag (if isUnmounted) and return early to avoid
updating result/testing or scheduling setTimeout; update all places that set
result, testing or assign clearTimer (including the block around
store.testConnection() and the other sections noted at lines 24-49) to perform
that isUnmounted check before mutating state or creating new timeouts.
In `@web/src/stores/providers.ts`:
- Around line 103-106: createFromPreset currently calls
providersApi.createFromPreset and fetchProviders without updating the store
error state or re-throwing on failure; wrap the API call in a try/catch like the
other mutators: set error.value = null before the call, in try await
providersApi.createFromPreset(data) and then await fetchProviders(), and in
catch set error.value = the caught error (or its message) and re-throw the error
so callers can handle it; reference createFromPreset,
providersApi.createFromPreset, fetchProviders, and error.value when applying the
change.
---
Duplicate comments:
In `@src/synthorg/providers/presets.py`:
- Around line 42-75: The preset catalog currently embeds real vendor identifiers
in ProviderPreset entries (fields name, display_name, description,
default_base_url for ProviderPreset instances named "ollama", "lm-studio",
"openrouter", "vllm"), which violates the vendor-agnostic policy; replace these
concrete vendor strings with generic equivalents (e.g., name="local-llm",
display_name="Local LLM", description="Local inference server",
default_base_url="http://localhost:<port>/v1" or similarly neutral labels)
across the ProviderPreset definitions, ensuring all occurrences of name,
display_name, description, and default_base_url in those ProviderPreset
instances are updated consistently, or alternatively add a documented CLAUDE.md
exemption if keeping vendor names is intentional.
In `@web/src/__tests__/components/providers/ProviderFormDialog.test.ts`:
- Around line 15-23: The current tests only assert the SFC compiles (import and
__name) and should be replaced with behavior tests that mount
ProviderFormDialog.vue and verify dialog interaction: use Vue Test Utils (mount
or shallowMount) to open the dialog component (ProviderFormDialog), simulate
user interactions for preset selection and auth-type field switching, exercise
validation failure and success paths, trigger submit and close actions, and
assert the emitted events (e.g., submit payload and close/success emits) and
rendered DOM state after each action; locate ProviderFormDialog.vue in the tests
and replace the two smoke tests with mounted-interaction tests that check
rendered fields change with auth type, validation messages appear when inputs
are invalid, and the exact payload emitted on successful submit.
In `@web/src/components/providers/ProviderFormDialog.vue`:
- Around line 127-129: The current change uses data.base_url = baseUrl.value ||
undefined which results in undefined being omitted in JSON and not clearing the
backend value; update the assignment in ProviderFormDialog (where baseUrl,
props.providerConfig?.base_url and data.base_url are used) to set data.base_url
= baseUrl.value === '' ? null : baseUrl.value (or the API's clear sentinel) so
an explicit null is sent when the user clears the field.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 30665019-89ae-48f0-98ca-e025581b0478
📒 Files selected for processing (34)
CLAUDE.mddocs/design/operations.mdsrc/synthorg/api/controllers/providers.pysrc/synthorg/api/dto.pysrc/synthorg/api/state.pysrc/synthorg/config/schema.pysrc/synthorg/observability/events/api.pysrc/synthorg/observability/events/provider.pysrc/synthorg/providers/drivers/litellm_driver.pysrc/synthorg/providers/enums.pysrc/synthorg/providers/errors.pysrc/synthorg/providers/management/__init__.pysrc/synthorg/providers/management/service.pysrc/synthorg/providers/presets.pytests/integration/api/controllers/__init__.pytests/integration/api/controllers/test_providers.pytests/unit/api/controllers/test_providers.pytests/unit/config/conftest.pytests/unit/config/test_provider_auth.pytests/unit/providers/drivers/test_litellm_auth.pytests/unit/providers/management/__init__.pytests/unit/providers/management/test_service.pytests/unit/providers/test_presets.pyweb/src/__tests__/components/providers/ProviderCard.test.tsweb/src/__tests__/components/providers/ProviderFormDialog.test.tsweb/src/__tests__/components/providers/ProviderTestButton.test.tsweb/src/__tests__/stores/providers.test.tsweb/src/api/endpoints/providers.tsweb/src/api/types.tsweb/src/components/providers/ProviderCard.vueweb/src/components/providers/ProviderFormDialog.vueweb/src/components/providers/ProviderTestButton.vueweb/src/stores/providers.tsweb/src/views/SettingsPage.vue
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Test (Python 3.14)
- GitHub Check: Build Backend
🧰 Additional context used
📓 Path-based instructions (7)
src/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.py: Nofrom __future__ import annotations— Python 3.14 has PEP 649 native lazy annotations.
Useexcept A, B:syntax (no parentheses) for exception handling — PEP 758 except syntax enforced by ruff on Python 3.14.
Type hints: all public functions required, mypy strict mode enforced.
Docstrings: Google style, required on public classes/functions — enforced by ruff D rules.
Immutability: create new objects, never mutate existing ones. For non-Pydantic internal collections (registries,BaseTool), usecopy.deepcopy()at construction +MappingProxyTypewrapping for read-only enforcement.
Fordict/listfields in frozen Pydantic models, rely onfrozen=Truefor field reassignment prevention andcopy.deepcopy()at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence).
Config vs runtime state: frozen Pydantic models for config/identity; separate mutable-via-copy models (usingmodel_copy(update=...)) for runtime state that evolves. Never mix static config fields with mutable runtime fields in one model.
Models: Pydantic v2 (BaseModel,model_validator,computed_field,ConfigDict). Use@computed_fieldfor derived values instead of storing redundant fields. UseNotBlankStrfor all identifier/name fields — including optional (NotBlankStr | None) and tuple variants — instead of manual whitespace validators.
Async concurrency: preferasyncio.TaskGroupfor fan-out/fan-in parallel operations in new code (e.g., multiple tool invocations, parallel agent calls). Prefer structured concurrency over barecreate_task.
Line length: 88 characters — enforced by ruff.
Functions: < 50 lines, files < 800 lines.
Errors: handle explicitly, never silently swallow.
Validate: at system boundaries (user input, external APIs, config files).
Vendor-agnostic everywhere: NEVER use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, docstrings, comments, tests, or config ...
Files:
src/synthorg/providers/errors.pysrc/synthorg/observability/events/provider.pysrc/synthorg/api/state.pysrc/synthorg/observability/events/api.pysrc/synthorg/config/schema.pysrc/synthorg/providers/management/__init__.pysrc/synthorg/providers/enums.pysrc/synthorg/providers/presets.pysrc/synthorg/providers/drivers/litellm_driver.pysrc/synthorg/providers/management/service.pysrc/synthorg/api/controllers/providers.pysrc/synthorg/api/dto.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/**/*.py: Every module with business logic MUST have:from synthorg.observability import get_loggerthenlogger = get_logger(__name__). Never useimport logging/logging.getLogger()/print()in application code.
Logging variable name: alwayslogger(not_logger, notlog).
Event names: always use constants from the domain-specific module undersynthorg.observability.events(e.g., API_REQUEST_STARTED from events.api, TOOL_INVOKE_START from events.tool). Each domain has its own module.
Structured logging kwargs: alwayslogger.info(EVENT, key=value)— neverlogger.info("msg %s", val).
All error paths must log at WARNING or ERROR with context before raising.
All state transitions must log at INFO.
DEBUG logging for object creation, internal flow, entry/exit of key functions.
Pure data models, enums, and re-exports do NOT need logging.
All provider calls go throughBaseCompletionProviderwhich applies retry + rate limiting automatically. Never implement retry logic in driver subclasses or calling code.
RetryConfig and RateLimiterConfig are set per-provider inProviderConfig.
Retryable errors (is_retryable=True):RateLimitError,ProviderTimeoutError,ProviderConnectionError,ProviderInternalError. Non-retryable errors raise immediately.
RetryExhaustedErrorsignals that all retries failed — the engine layer catches this to trigger fallback chains.
Rate limiter respectsRateLimitError.retry_afterfrom providers — automatically pauses future requests.
Files:
src/synthorg/providers/errors.pysrc/synthorg/observability/events/provider.pysrc/synthorg/api/state.pysrc/synthorg/observability/events/api.pysrc/synthorg/config/schema.pysrc/synthorg/providers/management/__init__.pysrc/synthorg/providers/enums.pysrc/synthorg/providers/presets.pysrc/synthorg/providers/drivers/litellm_driver.pysrc/synthorg/providers/management/service.pysrc/synthorg/api/controllers/providers.pysrc/synthorg/api/dto.py
web/src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
web/src/**/*.{vue,ts,tsx}: Web Dashboard: Vue 3 + PrimeVue + Tailwind CSS, organized components by feature (agents/, approvals/, budget/, common/, dashboard/, layout/, messages/, org-chart/, tasks/). Use Pinia for state management.
Web Dashboard: ESLint linting and vue-tsc type checking required.
Files:
web/src/__tests__/components/providers/ProviderFormDialog.test.tsweb/src/components/providers/ProviderFormDialog.vueweb/src/components/providers/ProviderTestButton.vueweb/src/__tests__/components/providers/ProviderTestButton.test.tsweb/src/__tests__/stores/providers.test.tsweb/src/api/types.tsweb/src/components/providers/ProviderCard.vueweb/src/__tests__/components/providers/ProviderCard.test.tsweb/src/stores/providers.tsweb/src/api/endpoints/providers.tsweb/src/views/SettingsPage.vue
web/src/__tests__/**/*.{test,spec}.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Web Dashboard: use Vitest for unit tests with fast-check for property-based testing.
Files:
web/src/__tests__/components/providers/ProviderFormDialog.test.tsweb/src/__tests__/components/providers/ProviderTestButton.test.tsweb/src/__tests__/stores/providers.test.tsweb/src/__tests__/components/providers/ProviderCard.test.ts
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Test markers:@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e,@pytest.mark.slow.
Coverage: 80% minimum — enforced in CI.
Async:asyncio_mode = "auto"— no manual@pytest.mark.asyncioneeded.
Timeout: 30 seconds per test.
Parallelism:pytest-xdistvia-n auto— ALWAYS include-n autowhen running pytest, never run tests sequentially.
Parametrize: Prefer@pytest.mark.parametrizefor testing similar cases.
Tests must usetest-provider,test-small-001, etc. instead of vendor names.
Property-based testing: Python uses Hypothesis (@given+@settings). Profiles:ci(200 examples, default) anddev(1000 examples), controlled viaHYPOTHESIS_PROFILEenv var.
NEVER skip, dismiss, or ignore flaky tests — always fix them fully and fundamentally. For timing-sensitive tests, mocktime.monotonic()andasyncio.sleep()to make them deterministic.
Files:
tests/unit/config/conftest.pytests/unit/api/controllers/test_providers.pytests/unit/config/test_provider_auth.pytests/integration/api/controllers/test_providers.pytests/unit/providers/management/test_service.pytests/unit/providers/drivers/test_litellm_auth.pytests/unit/providers/test_presets.py
docs/design/**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
When approved deviations occur, update the relevant
docs/design/page to reflect the new reality.
Files:
docs/design/operations.md
docs/design/operations.md
📄 CodeRabbit inference engine (CLAUDE.md)
Vendor-agnostic everywhere: Vendor names may only appear in: (1) Operations design page provider list (docs/design/operations.md), (2)
.claude/skill/agent files, (3) third-party import paths/module names (e.g., litellm.types.llms.openai).
Files:
docs/design/operations.md
🧠 Learnings (31)
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to src/synthorg/providers/**/*.py : Retryable errors (is_retryable=True): RateLimitError, ProviderTimeoutError, ProviderConnectionError, ProviderInternalError. Non-retryable errors raise immediately without retry. RetryExhaustedError signals that all retries failed — the engine layer catches this to trigger fallback chains.
Applied to files:
src/synthorg/providers/errors.pysrc/synthorg/providers/drivers/litellm_driver.pysrc/synthorg/api/dto.py
📚 Learning: 2026-03-18T07:33:37.988Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T07:33:37.988Z
Learning: Applies to src/synthorg/**/*.py : Retryable errors (`is_retryable=True`): `RateLimitError`, `ProviderTimeoutError`, `ProviderConnectionError`, `ProviderInternalError`. Non-retryable errors raise immediately.
Applied to files:
src/synthorg/providers/errors.pysrc/synthorg/providers/drivers/litellm_driver.pysrc/synthorg/api/dto.py
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to src/synthorg/providers/**/*.py : RetryConfig and RateLimiterConfig are set per-provider in ProviderConfig.
Applied to files:
src/synthorg/providers/errors.pytests/unit/config/conftest.pysrc/synthorg/observability/events/provider.pysrc/synthorg/api/state.pysrc/synthorg/config/schema.pysrc/synthorg/providers/management/service.pysrc/synthorg/api/dto.py
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to src/synthorg/providers/**/*.py : Rate limiter respects RateLimitError.retry_after from providers — automatically pauses future requests.
Applied to files:
src/synthorg/providers/errors.py
📚 Learning: 2026-03-18T07:33:37.988Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T07:33:37.988Z
Learning: Applies to src/synthorg/**/*.py : Rate limiter respects `RateLimitError.retry_after` from providers — automatically pauses future requests.
Applied to files:
src/synthorg/providers/errors.py
📚 Learning: 2026-03-18T07:33:37.988Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T07:33:37.988Z
Learning: Applies to src/synthorg/**/*.py : RetryConfig and RateLimiterConfig are set per-provider in `ProviderConfig`.
Applied to files:
src/synthorg/providers/errors.pytests/unit/config/conftest.pysrc/synthorg/api/state.pysrc/synthorg/config/schema.pysrc/synthorg/providers/management/service.pysrc/synthorg/api/dto.py
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to src/synthorg/providers/**/*.py : All provider calls go through BaseCompletionProvider which applies retry + rate limiting automatically. Never implement retry logic in driver subclasses or calling code — it's handled by the base class.
Applied to files:
src/synthorg/providers/errors.pysrc/synthorg/providers/drivers/litellm_driver.py
📚 Learning: 2026-03-18T07:33:37.988Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T07:33:37.988Z
Learning: Applies to src/synthorg/**/*.py : All provider calls go through `BaseCompletionProvider` which applies retry + rate limiting automatically. Never implement retry logic in driver subclasses or calling code.
Applied to files:
src/synthorg/providers/errors.pysrc/synthorg/providers/drivers/litellm_driver.py
📚 Learning: 2026-03-18T07:33:37.988Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T07:33:37.988Z
Learning: Applies to web/src/__tests__/**/*.{test,spec}.ts : Web Dashboard: use Vitest for unit tests with fast-check for property-based testing.
Applied to files:
web/src/__tests__/components/providers/ProviderFormDialog.test.tsweb/src/__tests__/components/providers/ProviderTestButton.test.tsweb/src/__tests__/stores/providers.test.tsweb/src/__tests__/components/providers/ProviderCard.test.ts
📚 Learning: 2026-03-18T07:33:37.988Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T07:33:37.988Z
Learning: Applies to tests/**/*.py : Tests must use `test-provider`, `test-small-001`, etc. instead of vendor names.
Applied to files:
web/src/__tests__/components/providers/ProviderFormDialog.test.tsweb/src/__tests__/components/providers/ProviderTestButton.test.tsweb/src/__tests__/stores/providers.test.tstests/unit/api/controllers/test_providers.pytests/integration/api/controllers/test_providers.pyweb/src/__tests__/components/providers/ProviderCard.test.tstests/unit/providers/management/test_service.pytests/unit/providers/test_presets.pysrc/synthorg/providers/presets.py
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to tests/**/*.py : Tests must use test-provider, test-small-001, etc. for vendor-agnostic test data.
Applied to files:
web/src/__tests__/components/providers/ProviderTestButton.test.tsweb/src/__tests__/stores/providers.test.tstests/unit/api/controllers/test_providers.pytests/integration/api/controllers/test_providers.pytests/unit/providers/management/test_service.pytests/unit/providers/test_presets.pysrc/synthorg/providers/presets.py
📚 Learning: 2026-03-18T07:33:37.988Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T07:33:37.988Z
Learning: Applies to web/src/**/*.{vue,ts,tsx} : Web Dashboard: Vue 3 + PrimeVue + Tailwind CSS, organized components by feature (agents/, approvals/, budget/, common/, dashboard/, layout/, messages/, org-chart/, tasks/). Use Pinia for state management.
Applied to files:
CLAUDE.mdweb/src/components/providers/ProviderCard.vueweb/src/__tests__/components/providers/ProviderCard.test.tsweb/src/views/SettingsPage.vue
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to web/src/components/** : Vue components organized by feature (agents/, approvals/, budget/, common/, dashboard/, layout/, messages/, org-chart/, tasks/).
Applied to files:
CLAUDE.mdweb/src/components/providers/ProviderCard.vue
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to web/** : Web dashboard: Node.js 20+, dependencies in web/package.json (Vue 3, PrimeVue, Tailwind CSS, Pinia, VueFlow, ECharts, Axios, vue-draggable-plus, Vitest, fast-check, ESLint, vue-tsc).
Applied to files:
CLAUDE.mdweb/src/views/SettingsPage.vue
📚 Learning: 2026-03-18T07:33:37.988Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T07:33:37.988Z
Learning: Applies to web/src/**/*.{vue,ts,tsx} : Web Dashboard: ESLint linting and vue-tsc type checking required.
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to docker/{Dockerfile*,compose.yml} : Docker: Backend uses 3-stage build (builder → setup → distroless runtime), Chainguard Python, non-root (UID 65532), CIS-hardened. Web uses nginxinc/nginx-unprivileged, Vue 3 SPA with PrimeVue + Tailwind CSS, SPA routing, API/WebSocket proxy to backend.
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to src/synthorg/**/*.py : Event names: always use constants from domain-specific modules under synthorg.observability.events (e.g., PROVIDER_CALL_START from events.provider, BUDGET_RECORD_ADDED from events.budget, etc.). Import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`.
Applied to files:
src/synthorg/observability/events/provider.pysrc/synthorg/api/state.pysrc/synthorg/observability/events/api.pysrc/synthorg/config/schema.pysrc/synthorg/api/controllers/providers.py
📚 Learning: 2026-03-18T07:33:37.988Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T07:33:37.988Z
Learning: Applies to src/synthorg/**/*.py : Event names: always use constants from the domain-specific module under `synthorg.observability.events` (e.g., API_REQUEST_STARTED from events.api, TOOL_INVOKE_START from events.tool). Each domain has its own module.
Applied to files:
src/synthorg/observability/events/provider.pysrc/synthorg/api/state.pysrc/synthorg/observability/events/api.py
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to {src,tests,web,cli,site}/**/*.{py,ts,tsx,go,astro} : Vendor-agnostic everywhere: NEVER use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, docstrings, comments, tests, or config examples. Use generic names: example-provider, example-large-001, example-medium-001, example-small-001. Vendor names may only appear in: (1) Operations design page provider list (docs/design/operations.md), (2) .claude/ skill/agent files, (3) third-party import paths/module names.
Applied to files:
tests/unit/providers/management/test_service.pysrc/synthorg/providers/presets.py
📚 Learning: 2026-03-18T07:33:37.988Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T07:33:37.988Z
Learning: Applies to src/**/*.yaml : Never use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, docstrings, comments, tests, or config examples — use generic names like `example-provider`, `example-large-001`, `example-medium-001`, `example-small-001`.
Applied to files:
tests/unit/providers/management/test_service.pysrc/synthorg/providers/presets.py
📚 Learning: 2026-03-18T07:33:37.988Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T07:33:37.988Z
Learning: Applies to src/**/*.py : Vendor-agnostic everywhere: NEVER use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, docstrings, comments, tests, or config examples. Use generic names: `example-provider`, `example-large-001`, `example-medium-001`, `example-small-001`, `large`/`medium`/`small` aliases.
Applied to files:
tests/unit/providers/management/test_service.pysrc/synthorg/providers/presets.py
📚 Learning: 2026-03-18T07:33:37.988Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T07:33:37.988Z
Learning: Applies to docs/design/operations.md : Vendor-agnostic everywhere: Vendor names may only appear in: (1) Operations design page provider list (docs/design/operations.md), (2) `.claude/` skill/agent files, (3) third-party import paths/module names (e.g., litellm.types.llms.openai).
Applied to files:
tests/unit/providers/management/test_service.pysrc/synthorg/providers/presets.py
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to **/*.py : Config vs runtime state: frozen Pydantic models for config/identity; separate mutable-via-copy models (using model_copy(update=...)) for runtime state that evolves. Never mix static config fields with mutable runtime fields in one model.
Applied to files:
src/synthorg/providers/management/service.py
📚 Learning: 2026-03-18T07:33:37.988Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T07:33:37.988Z
Learning: Applies to src/**/*.py : Config vs runtime state: frozen Pydantic models for config/identity; separate mutable-via-copy models (using `model_copy(update=...)`) for runtime state that evolves. Never mix static config fields with mutable runtime fields in one model.
Applied to files:
src/synthorg/providers/management/service.py
📚 Learning: 2026-03-18T07:33:37.988Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T07:33:37.988Z
Learning: Applies to src/**/*.py : Models: Pydantic v2 (`BaseModel`, `model_validator`, `computed_field`, `ConfigDict`). Use `computed_field` for derived values instead of storing redundant fields. Use `NotBlankStr` for all identifier/name fields — including optional (`NotBlankStr | None`) and tuple variants — instead of manual whitespace validators.
Applied to files:
src/synthorg/providers/management/service.pysrc/synthorg/api/dto.py
📚 Learning: 2026-03-18T07:33:37.988Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T07:33:37.988Z
Learning: Applies to src/**/*.py : For `dict`/`list` fields in frozen Pydantic models, rely on `frozen=True` for field reassignment prevention and `copy.deepcopy()` at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence).
Applied to files:
src/synthorg/providers/management/service.py
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to **/*.py : For dict/list fields in frozen Pydantic models, rely on frozen=True for field reassignment prevention and copy.deepcopy() at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence).
Applied to files:
src/synthorg/providers/management/service.py
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to **/*.py : Models: Pydantic v2 (BaseModel, model_validator, computed_field, ConfigDict). Use computed_field for derived values instead of storing + validating redundant fields. Use NotBlankStr (from core.types) for all identifier/name fields — including optional (NotBlankStr | None) and tuple (tuple[NotBlankStr, ...]) variants — instead of manual whitespace validators.
Applied to files:
src/synthorg/providers/management/service.pysrc/synthorg/api/dto.py
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to **/*.py : Validate: at system boundaries (user input, external APIs, config files).
Applied to files:
src/synthorg/providers/management/service.py
📚 Learning: 2026-03-18T07:33:37.988Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T07:33:37.988Z
Learning: Applies to src/**/*.py : Validate: at system boundaries (user input, external APIs, config files).
Applied to files:
src/synthorg/providers/management/service.py
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising.
Applied to files:
src/synthorg/api/dto.py
🧬 Code graph analysis (15)
web/src/__tests__/stores/providers.test.ts (2)
web/src/api/types.ts (2)
ProviderConfig(464-476)ProviderPreset(519-527)web/src/stores/providers.ts (1)
useProviderStore(34-121)
tests/unit/api/controllers/test_providers.py (3)
src/synthorg/api/dto.py (1)
to_provider_response(704-734)src/synthorg/config/schema.py (1)
ProviderConfig(85-230)src/synthorg/providers/enums.py (1)
AuthType(6-12)
tests/unit/config/test_provider_auth.py (2)
src/synthorg/config/schema.py (1)
ProviderConfig(85-230)src/synthorg/providers/enums.py (1)
AuthType(6-12)
tests/integration/api/controllers/test_providers.py (4)
src/synthorg/settings/registry.py (1)
get_registry(111-113)src/synthorg/settings/service.py (1)
SettingsService(152-637)src/synthorg/api/app.py (1)
create_app(533-700)src/synthorg/settings/encryption.py (1)
SettingsEncryptor(21-125)
web/src/api/types.ts (4)
src/synthorg/providers/enums.py (1)
AuthType(6-12)src/synthorg/config/schema.py (2)
ProviderConfig(85-230)ProviderModelConfig(43-82)src/synthorg/api/dto.py (5)
CreateProviderRequest(521-557)UpdateProviderRequest(560-601)TestConnectionRequest(604-613)TestConnectionResponse(616-642)CreateFromPresetRequest(679-701)src/synthorg/providers/presets.py (1)
ProviderPreset(16-37)
web/src/__tests__/components/providers/ProviderCard.test.ts (2)
src/synthorg/config/schema.py (1)
ProviderConfig(85-230)web/src/api/types.ts (1)
ProviderConfig(464-476)
src/synthorg/api/state.py (2)
tests/unit/providers/management/test_service.py (3)
service(92-102)settings_service(60-70)app_state(74-88)src/synthorg/providers/management/service.py (1)
ProviderManagementService(51-414)
src/synthorg/config/schema.py (2)
web/src/api/types.ts (1)
AuthType(450-450)src/synthorg/providers/enums.py (1)
AuthType(6-12)
src/synthorg/providers/management/__init__.py (2)
tests/unit/providers/management/test_service.py (1)
service(92-102)src/synthorg/providers/management/service.py (1)
ProviderManagementService(51-414)
src/synthorg/providers/enums.py (1)
web/src/api/types.ts (1)
AuthType(450-450)
tests/unit/providers/test_presets.py (1)
src/synthorg/providers/presets.py (2)
get_preset(84-93)list_presets(96-102)
src/synthorg/providers/presets.py (3)
src/synthorg/config/schema.py (1)
ProviderModelConfig(43-82)web/src/api/types.ts (3)
ProviderModelConfig(452-459)AuthType(450-450)ProviderPreset(519-527)src/synthorg/providers/enums.py (1)
AuthType(6-12)
src/synthorg/providers/drivers/litellm_driver.py (3)
src/synthorg/providers/enums.py (1)
AuthType(6-12)src/synthorg/config/schema.py (1)
ProviderModelConfig(43-82)tests/unit/providers/drivers/test_litellm_auth.py (1)
_build_kwargs(43-50)
src/synthorg/providers/management/service.py (4)
src/synthorg/api/dto.py (1)
CreateProviderRequest(521-557)src/synthorg/config/schema.py (1)
ProviderConfig(85-230)src/synthorg/providers/enums.py (2)
AuthType(6-12)MessageRole(15-21)src/synthorg/settings/resolver.py (2)
ConfigResolver(37-673)get_provider_configs(453-472)
src/synthorg/api/dto.py (3)
src/synthorg/config/schema.py (2)
ProviderConfig(85-230)ProviderModelConfig(43-82)web/src/api/types.ts (7)
ProviderConfig(464-476)ProviderModelConfig(452-459)AuthType(450-450)UpdateProviderRequest(493-506)TestConnectionRequest(508-510)TestConnectionResponse(512-517)CreateFromPresetRequest(529-535)src/synthorg/providers/enums.py (1)
AuthType(6-12)
🪛 GitHub Check: Dashboard Lint
web/src/api/endpoints/providers.ts
[warning] 20-20:
Generic Object Injection Sink
🔇 Additional comments (23)
src/synthorg/observability/events/api.py (1)
59-60: LGTM: the new API events stay in the right namespace.They follow the existing
api.*naming scheme and keep provider-management logging on API-domain event constants.Based on learnings, event names should live in the matching
synthorg.observability.eventsdomain module.src/synthorg/providers/errors.py (1)
165-180: LGTM: these lifecycle errors are correctly marked non-retryable.
already exists,not found, and validation failures should fail fast, so keepingis_retryable = Falsematches the provider retry contract.Based on learnings, only rate-limit, timeout, connection, and internal provider failures are retryable.
web/src/__tests__/components/providers/ProviderTestButton.test.ts (1)
46-47: LGTM: this now checks the provider-name wiring, not just the click.The explicit argument assertion will catch regressions where the button targets the wrong provider.
src/synthorg/api/state.py (1)
121-130: Don't bind provider-management rebuilds to the startupRootConfig.
ProviderManagementServiceis initialized with aconfigparameter and rebuildsModelRouterduring provider CRUD operations. If routing or other runtime-editable settings change after startup, subsequent provider mutations could rebuild from the initial startup config values and silently overwrite the live router state. Verify that_build_router()resolves settings at rebuild time rather than from the stored startup config. If it does read the stored config, either fetch current settings at rebuild time or recreate the service when the effective config changes.web/src/components/providers/ProviderCard.vue (1)
80-105: Good accessibility follow-through.The live region now wraps the delete-confirmation toggle, so assistive tech can announce the state change instead of silently swapping buttons.
tests/unit/providers/management/test_service.py (2)
325-343: Replace branded preset names with generic test fixtures.The test uses
ollama,my-ollama, andollama-localwhich violate the vendor-agnostic test naming rule. These should be replaced with generic names liketest-local-presetandtest-provider.As per coding guidelines, "NEVER use real vendor names ... in tests. Tests must use
test-provider,test-small-001, etc."Also applies to: 436-436
1-122: LGTM!The test fixtures are well-structured with proper in-memory fakes for persistence and message bus. The helper function
_make_create_requestprovides sensible defaults using vendor-agnostic names (test-provider,test-model-001).web/src/views/SettingsPage.vue (2)
62-66: LGTM!Good use of
Promise.allfor parallel data fetching. The addition offetchPresets()alongside existing fetches ensures all provider-related data is loaded efficiently.
109-122: LGTM!The
handleFormSavefunction correctly discriminates between create and edit modes, using type assertions that are safe given the preceding mode check. Error handling with toast notifications provides good user feedback.src/synthorg/providers/drivers/litellm_driver.py (3)
143-145: LGTM!Correct use of
MappingProxyTypewrapping for_model_lookupto enforce read-only access, aligning with the immutability guidelines for internal collections.
334-352: LGTM!The multi-auth dispatch using Python's
matchstatement is clean and correctly handles allAuthTypevariants. OAuth MVP correctly reuses theapi_keyfield for pre-fetched tokens, with a clear comment explaining future work.
314-314: Consider extracting auth kwargs building to a separate method.The
noqa: C901suggests elevated complexity. Extracting auth configuration into a private helper (e.g.,_build_auth_kwargs) would improve readability and testability.web/src/api/endpoints/providers.ts (2)
17-22: LGTM!The prototype pollution protection at line 19 correctly filters out dangerous keys (
__proto__,constructor,prototype). The static analysis warning at line 20 is a false positive since the filtering is already in place.
35-62: LGTM!The new CRUD endpoints follow the established pattern with proper URL encoding via
encodeURIComponentand consistent use of theunwraphelper for typed responses.src/synthorg/api/dto.py (3)
492-518: LGTM!The provider name validation with regex and reserved names check provides robust input validation at the system boundary. The pattern enforces lowercase alphanumeric with hyphens (2-64 chars) and blocks reserved API path segments.
633-642: LGTM!The
_validate_success_error_consistencyvalidator now correctly enforces that failed tests must have an error message (line 639-641), addressing the previous review feedback.
704-734: LGTM!The
to_provider_responsefunction correctly strips secrets while providing boolean indicators. Thehas_custom_headercheck (lines 726-728) now requires bothcustom_header_nameandcustom_header_valueto be non-None, addressing the previous review feedback.web/src/stores/providers.ts (1)
41-58: LGTM!Excellent race-guard implementation using a generation counter. This ensures stale responses from superseded requests don't overwrite state, preventing UI flicker and data inconsistency.
src/synthorg/api/controllers/providers.py (2)
1-36: LGTM!Well-organized imports with proper separation of DTOs, errors, guards, and observability events. The controller follows the established pattern for Litestar controllers.
148-188: LGTM!The
create_providerendpoint correctly maps domain exceptions to HTTP semantics:ProviderAlreadyExistsError→ 409 Conflict,ProviderValidationError→ 422 Validation Error. Structured logging provides good observability.src/synthorg/providers/management/service.py (3)
64-76: LGTM!The constructor properly initializes the asyncio lock for serializing mutating operations. Dependency injection of services allows for testability.
346-393: LGTM!The
_validate_and_persistmethod correctly builds both the registry and router within the same try block before any I/O (addressing previous feedback). This prevents DB/AppState divergence on build failure. The hot-reload swap is performed atomically without an await gap.
456-500: LGTM!The
_apply_updatefunction correctly:
- Clears incompatible credentials on auth_type changes (lines 478-490)
- Handles explicit api_key and clear_api_key semantics (lines 492-496)
- Uses
model_validateinstead ofmodel_copy(update=...)for proper re-validation (lines 498-500)This addresses the previous review feedback about skipped validators.
| async function createFromPreset(data: CreateFromPresetRequest) { | ||
| await providersApi.createFromPreset(data) | ||
| await fetchProviders() | ||
| } |
There was a problem hiding this comment.
Add error handling to createFromPreset for consistency.
Unlike createProvider, updateProvider, and deleteProvider, this function doesn't set error.value or re-throw on failure. This inconsistency could leave the store error state stale and prevent callers from handling failures.
💡 Proposed fix
async function createFromPreset(data: CreateFromPresetRequest) {
- await providersApi.createFromPreset(data)
- await fetchProviders()
+ try {
+ await providersApi.createFromPreset(data)
+ await fetchProviders()
+ } catch (err) {
+ error.value = getErrorMessage(err)
+ throw err
+ }
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/stores/providers.ts` around lines 103 - 106, createFromPreset
currently calls providersApi.createFromPreset and fetchProviders without
updating the store error state or re-throwing on failure; wrap the API call in a
try/catch like the other mutators: set error.value = null before the call, in
try await providersApi.createFromPreset(data) and then await fetchProviders(),
and in catch set error.value = the caught error (or its message) and re-throw
the error so callers can handle it; reference createFromPreset,
providersApi.createFromPreset, fetchProviders, and error.value when applying the
change.
- operations.md: Fix CRUD routes to show {name} path params for PUT/DELETE
- ProviderCard.test.ts, providers.test.ts: Add missing nullable auth
metadata fields to mock objects (vue-tsc fix)
- UpdateProviderRequest: Allow null on clearable fields (string | null)
so frontend can send explicit null to clear backend values
- ProviderFormDialog isValid: Don't require secrets in edit mode --
empty secret fields mean "keep existing" (secrets aren't round-tripped)
- ProviderFormDialog handleSave: Scope fields to active auth type only;
when switching auth types in update mode, explicitly clear old fields
- ProviderFormDialog: Add savePreset emit for preset-backed creation
flow so parent can call store.createFromPreset instead of createProvider
- ProviderFormDialog base_url clearing: Send null (not undefined) when
user clears the field so backend receives explicit clear sentinel
- ProviderTestButton: Add isUnmounted guard to prevent state mutation
after component unmount during async test connection
- Store createFromPreset: Add try/catch error handling consistent with
other mutation actions
- SettingsPage: Wire savePreset event to store.createFromPreset handler
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
docs/design/operations.md (1)
998-998:⚠️ Potential issue | 🟡 MinorClarify path specificity for update/delete in API surface row.
/api/v1/providers+ “CRUD (create, update, delete)” is a bit ambiguous in this table. Consider explicitly noting update/delete use/{name}(e.g.,PUT /api/v1/providers/{name},DELETE /api/v1/providers/{name}) to avoid client misrouting.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/design/operations.md` at line 998, Update the table row for the API surface entry that currently lists `/api/v1/providers` and "CRUD (create, update, delete)" to explicitly list the specific endpoints and HTTP methods for each operation (e.g., "GET /api/v1/providers" for list, "POST /api/v1/providers" for create, "PUT /api/v1/providers/{name}" for update, "DELETE /api/v1/providers/{name}" for delete, and optionally "GET /api/v1/providers/{name}" for read), so clients understand that update/delete use `/{name}` and won’t be misrouted.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/design/operations.md`:
- Line 1066: Split the dense "Settings" paragraph into multiple short
sub-bullets for scanability: one bullet for capabilities (provider management UI
actions, DB-backed persistence, REST schema endpoints, Fernet for sensitive
values), one for setting types and 4-layer resolution order
(STRING/INTEGER/FLOAT/BOOLEAN/ENUM/JSON and DB > env > YAML > code defaults),
one for ConfigResolver behavior (typed scalar accessors that assemble Pydantic
models via asyncio.TaskGroup and structural readers like
get_agents/get_departments/get_provider_configs that validate JSON and fall back
to RootConfig defaults), and one for Hot-reload/subscription behavior
(SettingsChangeDispatcher polling `#settings`, SettingsSubscriber contract,
filtering restart_required changes, and concrete subscribers
ProviderSettingsSubscriber using AppState.swap_model_router and
MemorySettingsSubscriber logging). Ensure each bullet references the listed
symbols for easy location.
In `@web/src/__tests__/stores/providers.test.ts`:
- Around line 51-137: Add unit tests for updateProvider and createFromPreset in
web/src/__tests__/stores/providers.test.ts: for updateProvider, mock
providersApi.updateProvider and providersApi.listProviders, call
store.updateProvider with an updated payload, assert updateProvider and
listProviders were called and that store.providers reflects the updated entry;
for createFromPreset, mock providersApi.createFromPreset and
providersApi.listProviders, call store.createFromPreset with preset_name and
name, assert createFromPreset and listProviders were called and that
store.providers contains the new preset-provider; follow existing test patterns
(useProviderStore, fetch/mocks, vi.mocked(...).mockResolvedValue, and
expect(...).toHaveBeenCalledOnce()).
In `@web/src/stores/providers.ts`:
- Around line 61-67: fetchPresets leaves a previous error set even when the new
fetch succeeds; before calling providersApi.listPresets in fetchPresets(), clear
the error state by setting error.value = null (same pattern as fetchProviders)
so stale errors don't persist, then proceed to await providersApi.listPresets()
and only set error.value on catch; locate the fetchPresets function and the
error.value usage to apply this change.
---
Duplicate comments:
In `@docs/design/operations.md`:
- Line 998: Update the table row for the API surface entry that currently lists
`/api/v1/providers` and "CRUD (create, update, delete)" to explicitly list the
specific endpoints and HTTP methods for each operation (e.g., "GET
/api/v1/providers" for list, "POST /api/v1/providers" for create, "PUT
/api/v1/providers/{name}" for update, "DELETE /api/v1/providers/{name}" for
delete, and optionally "GET /api/v1/providers/{name}" for read), so clients
understand that update/delete use `/{name}` and won’t be misrouted.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0319d1dc-9b9e-4711-a5c0-3fec62a3ab95
📒 Files selected for processing (8)
docs/design/operations.mdweb/src/__tests__/components/providers/ProviderCard.test.tsweb/src/__tests__/stores/providers.test.tsweb/src/api/types.tsweb/src/components/providers/ProviderFormDialog.vueweb/src/components/providers/ProviderTestButton.vueweb/src/stores/providers.tsweb/src/views/SettingsPage.vue
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Test (Python 3.14)
- GitHub Check: Build Sandbox
- GitHub Check: Build Backend
🧰 Additional context used
📓 Path-based instructions (4)
web/src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
web/src/**/*.{vue,ts,tsx}: Web Dashboard: Vue 3 + PrimeVue + Tailwind CSS, organized components by feature (agents/, approvals/, budget/, common/, dashboard/, layout/, messages/, org-chart/, tasks/). Use Pinia for state management.
Web Dashboard: ESLint linting and vue-tsc type checking required.
Files:
web/src/components/providers/ProviderTestButton.vueweb/src/__tests__/stores/providers.test.tsweb/src/__tests__/components/providers/ProviderCard.test.tsweb/src/stores/providers.tsweb/src/components/providers/ProviderFormDialog.vueweb/src/views/SettingsPage.vueweb/src/api/types.ts
web/src/__tests__/**/*.{test,spec}.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Web Dashboard: use Vitest for unit tests with fast-check for property-based testing.
Files:
web/src/__tests__/stores/providers.test.tsweb/src/__tests__/components/providers/ProviderCard.test.ts
docs/design/**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
When approved deviations occur, update the relevant
docs/design/page to reflect the new reality.
Files:
docs/design/operations.md
docs/design/operations.md
📄 CodeRabbit inference engine (CLAUDE.md)
Vendor-agnostic everywhere: Vendor names may only appear in: (1) Operations design page provider list (docs/design/operations.md), (2)
.claude/skill/agent files, (3) third-party import paths/module names (e.g., litellm.types.llms.openai).
Files:
docs/design/operations.md
🧠 Learnings (7)
📚 Learning: 2026-03-18T07:33:37.988Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T07:33:37.988Z
Learning: Applies to web/src/__tests__/**/*.{test,spec}.ts : Web Dashboard: use Vitest for unit tests with fast-check for property-based testing.
Applied to files:
web/src/__tests__/stores/providers.test.tsweb/src/__tests__/components/providers/ProviderCard.test.ts
📚 Learning: 2026-03-18T07:33:37.988Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T07:33:37.988Z
Learning: Applies to tests/**/*.py : Tests must use `test-provider`, `test-small-001`, etc. instead of vendor names.
Applied to files:
web/src/__tests__/stores/providers.test.tsweb/src/__tests__/components/providers/ProviderCard.test.ts
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to tests/**/*.py : Tests must use test-provider, test-small-001, etc. for vendor-agnostic test data.
Applied to files:
web/src/__tests__/stores/providers.test.tsweb/src/__tests__/components/providers/ProviderCard.test.ts
📚 Learning: 2026-03-18T07:33:37.988Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T07:33:37.988Z
Learning: Applies to web/src/**/*.{vue,ts,tsx} : Web Dashboard: Vue 3 + PrimeVue + Tailwind CSS, organized components by feature (agents/, approvals/, budget/, common/, dashboard/, layout/, messages/, org-chart/, tasks/). Use Pinia for state management.
Applied to files:
web/src/__tests__/components/providers/ProviderCard.test.tsweb/src/views/SettingsPage.vue
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to web/** : Web dashboard: Node.js 20+, dependencies in web/package.json (Vue 3, PrimeVue, Tailwind CSS, Pinia, VueFlow, ECharts, Axios, vue-draggable-plus, Vitest, fast-check, ESLint, vue-tsc).
Applied to files:
web/src/views/SettingsPage.vue
📚 Learning: 2026-03-18T07:33:37.988Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T07:33:37.988Z
Learning: Applies to docs/design/**/*.md : When approved deviations occur, update the relevant `docs/design/` page to reflect the new reality.
Applied to files:
docs/design/operations.md
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: When approved deviations occur, update the relevant `docs/design/` page to reflect the new reality.
Applied to files:
docs/design/operations.md
🧬 Code graph analysis (3)
web/src/__tests__/stores/providers.test.ts (3)
web/src/api/types.ts (2)
ProviderConfig(464-476)ProviderPreset(519-527)src/synthorg/providers/presets.py (1)
ProviderPreset(16-37)web/src/stores/providers.ts (1)
useProviderStore(34-126)
web/src/__tests__/components/providers/ProviderCard.test.ts (1)
web/src/api/types.ts (1)
ProviderConfig(464-476)
web/src/api/types.ts (4)
src/synthorg/providers/enums.py (1)
AuthType(6-12)src/synthorg/config/schema.py (2)
ProviderConfig(85-230)ProviderModelConfig(43-82)src/synthorg/api/dto.py (1)
CreateProviderRequest(521-557)src/synthorg/providers/presets.py (1)
ProviderPreset(16-37)
🔇 Additional comments (20)
docs/design/operations.md (2)
46-81: Auth type documentation is clear and aligned with runtime behavior.Good update: the examples now reflect multi-auth support and show a no-auth local-provider case explicitly.
104-114: Provider management section is well-scoped and operationally useful.This section correctly captures runtime CRUD/test/preset flows, hot-reload semantics, and secret-handling guarantees.
web/src/api/types.ts (2)
450-476: LGTM!The
AuthTypeunion and extendedProviderConfiginterface correctly mirror the backend DTO structure. The credential indicator booleans (has_api_key,has_oauth_credentials,has_custom_header) properly replace actual secrets in API responses, and the nullable metadata fields (oauth_token_url,oauth_client_id, etc.) are correctly typed.
478-535: LGTM!The request/response interfaces align well with the backend DTOs:
CreateProviderRequestandUpdateProviderRequestcorrectly scope optional fields.UpdateProviderRequestnow supportsstring | nullfor clearable fields, enabling explicit value removal.TestConnectionResponseandProviderPresetstructures match the backend models.web/src/stores/providers.ts (3)
15-32: LGTM!Good defensive coding with
UNSAFE_KEYSfor prototype pollution protection andSECRET_FIELDSto strip accidentally serialized secrets. The sanitization ensures the store never holds sensitive credential values.
41-59: LGTM!The generation counter pattern correctly handles race conditions from concurrent
fetchProviderscalls. Only the most recent request updates state, preventing stale data from overwriting newer results.
103-111: LGTM!Error handling is now consistent with other mutators—sets
error.valueand re-throws for caller handling.web/src/components/providers/ProviderTestButton.vue (2)
1-57: LGTM!Well-implemented component with proper lifecycle handling:
isUnmountedflag guards all async completions against state updates after unmount.- Timer cleanup in
onUnmountedprevents memory leaks.- Auto-clear after 10 seconds provides good UX without stale indicators.
60-78: LGTM!Good accessibility with
role="status"andaria-labelattributes on result indicators. The truncation withtitletooltip for error messages handles long error strings gracefully.web/src/components/providers/ProviderFormDialog.vue (4)
95-109: LGTM!Validation correctly distinguishes create vs edit mode: secrets are required for new providers but optional when editing (empty means "keep existing"). The auth-type-specific field requirements are properly enforced.
111-147: LGTM!Create mode payload construction properly handles:
- Preset flow via
savePresetevent when a preset is selected.- Auth-type-scoped field inclusion (only active auth type's fields are sent).
- Base URL override detection for preset-based creation.
148-199: LGTM!Update mode correctly:
- Detects field changes against previous config before including in payload.
- Uses explicit
nullfor clearing optional fields (e.g.,base_url,oauth_token_url).- Clears orphaned credentials when switching auth types (e.g.,
clear_api_key: truewhen leavingapi_keymode).
256-262: LGTM!The dynamic label "Access Token (pre-fetched)" for OAuth mode clarifies the field's purpose when used with OAuth, addressing the previous review feedback about user confusion.
web/src/__tests__/components/providers/ProviderCard.test.ts (2)
13-34: LGTM!The
mockConfignow includes all requiredProviderConfigfields including the nullable auth metadata (oauth_token_url,oauth_client_id,oauth_scope,custom_header_name), ensuringvue-tsctype-checking passes.
36-77: LGTM!Good test coverage:
- Rendering verification via parameterized tests.
- Edit event emission.
- Two-step delete confirmation workflow.
The use of
test-providerandtest-model-001follows the vendor-agnostic naming convention. Based on learnings: "Tests must usetest-provider,test-small-001, etc. instead of vendor names."web/src/__tests__/stores/providers.test.ts (1)
18-49: LGTM!Mock objects now include all required
ProviderConfigfields, and theProviderPresetmock correctly matches the interface. The vendor-agnostic naming (test-provider,test-model-001,ollama) follows project conventions.web/src/views/SettingsPage.vue (4)
51-57: LGTM!Clean dialog state management with
formDialogMode,editingProviderName, and computededitingProviderConfig. The computed property correctly derives the config from the store when editing.
59-72: LGTM!The
retryFetchfunction now fetches presets alongside providers and config in parallel, ensuring the form dialog has preset data available for the "From Preset" selector.
109-141: LGTM!Handlers properly:
- Branch on create vs update mode in
handleFormSave.- Close dialog on success.
- Show toast notifications for success/failure.
- Handle preset-based creation separately via
handleFormSavePreset.
174-206: LGTM!The Providers tab implementation:
- Includes an "Add Provider" action button.
- Shows a friendly empty state when no providers exist.
- Renders a responsive card grid (1/2/3 columns).
- Wires
ProviderFormDialogwith all necessary props and event handlers.
- operations.md: Split dense Settings paragraph into 4 scannable sub-bullets
(provider management, DB-backed persistence, ConfigResolver, hot-reload)
- operations.md: Expand API surface table row to list all provider endpoints
with HTTP methods and {name} path params
- stores/providers.ts: Clear stale error in fetchPresets before fetching
(same pattern as fetchProviders)
- providers.test.ts: Add missing tests for updateProvider and createFromPreset
store actions
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/src/__tests__/stores/providers.test.ts`:
- Around line 51-170: Add property-based tests in
web/src/__tests__/stores/providers.test.ts that (1) generate random provider
objects with secret fields and exercise createProvider/createFromPreset (or
directly call the store mutation) to assert the store's sanitizer removes secret
fields from stored providers (use fast-check to assert no secret keys remain
across many random inputs), and (2) verify stale-fetch protection by mocking the
providers API listProviders to resolve promises out-of-order and calling
useProviderStore().fetchProviders twice (resolve the later call earlier) to
assert the store ends up with the results from the most recent fetch; reference
useProviderStore, fetchProviders, createProvider, and createFromPreset when
locating code to test.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: afcffe40-c63a-48aa-b6e7-b193ed1bc503
📒 Files selected for processing (3)
docs/design/operations.mdweb/src/__tests__/stores/providers.test.tsweb/src/stores/providers.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Test (Python 3.14)
- GitHub Check: Build Web
- GitHub Check: Build Backend
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (4)
web/src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
web/src/**/*.{vue,ts,tsx}: Web Dashboard: Vue 3 + PrimeVue + Tailwind CSS, organized components by feature (agents/, approvals/, budget/, common/, dashboard/, layout/, messages/, org-chart/, tasks/). Use Pinia for state management.
Web Dashboard: ESLint linting and vue-tsc type checking required.
Files:
web/src/__tests__/stores/providers.test.tsweb/src/stores/providers.ts
web/src/__tests__/**/*.{test,spec}.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Web Dashboard: use Vitest for unit tests with fast-check for property-based testing.
Files:
web/src/__tests__/stores/providers.test.ts
docs/design/**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
When approved deviations occur, update the relevant
docs/design/page to reflect the new reality.
Files:
docs/design/operations.md
docs/design/operations.md
📄 CodeRabbit inference engine (CLAUDE.md)
Vendor-agnostic everywhere: Vendor names may only appear in: (1) Operations design page provider list (docs/design/operations.md), (2)
.claude/skill/agent files, (3) third-party import paths/module names (e.g., litellm.types.llms.openai).
Files:
docs/design/operations.md
🧠 Learnings (5)
📚 Learning: 2026-03-18T07:33:37.988Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T07:33:37.988Z
Learning: Applies to web/src/__tests__/**/*.{test,spec}.ts : Web Dashboard: use Vitest for unit tests with fast-check for property-based testing.
Applied to files:
web/src/__tests__/stores/providers.test.ts
📚 Learning: 2026-03-18T07:33:37.988Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T07:33:37.988Z
Learning: Applies to tests/**/*.py : Tests must use `test-provider`, `test-small-001`, etc. instead of vendor names.
Applied to files:
web/src/__tests__/stores/providers.test.ts
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to tests/**/*.py : Tests must use test-provider, test-small-001, etc. for vendor-agnostic test data.
Applied to files:
web/src/__tests__/stores/providers.test.ts
📚 Learning: 2026-03-18T07:33:37.988Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T07:33:37.988Z
Learning: Applies to docs/design/**/*.md : When approved deviations occur, update the relevant `docs/design/` page to reflect the new reality.
Applied to files:
docs/design/operations.md
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: When approved deviations occur, update the relevant `docs/design/` page to reflect the new reality.
Applied to files:
docs/design/operations.md
🧬 Code graph analysis (1)
web/src/stores/providers.ts (3)
web/src/api/types.ts (7)
ProviderConfig(464-476)ProviderPreset(519-527)CreateProviderRequest(478-491)UpdateProviderRequest(493-506)TestConnectionRequest(508-510)TestConnectionResponse(512-517)CreateFromPresetRequest(529-535)web/src/utils/errors.ts (1)
getErrorMessage(17-69)web/src/api/endpoints/providers.ts (4)
createProvider(35-38)updateProvider(40-43)deleteProvider(45-47)createFromPreset(59-62)
🔇 Additional comments (2)
docs/design/operations.md (1)
104-114: Provider-management documentation update is solid.The runtime CRUD/test/preset/hot-reload/auth coverage is clear and significantly improves operational accuracy and discoverability.
Also applies to: 998-998, 1066-1070
web/src/stores/providers.ts (1)
16-30: Good hardening in provider-store state updates.Secret-field stripping plus the generation-based stale-response guard are well implemented, and mutators consistently refresh through a single fetch path.
Also applies to: 41-58, 70-112
- Deterministic test: fetchProviders strips leaked secret fields (api_key, oauth_client_secret, custom_header_value) from stored state - Property test (fast-check): sanitizer removes secret fields and prototype-pollution keys across random provider shapes - Stale-fetch test: when two fetchProviders resolve out of order, store keeps fresh data from the later call, discards stale
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/src/__tests__/stores/providers.test.ts`:
- Around line 198-250: The test currently reimplements sanitization instead of
invoking the real code, and it omits the 'custom_header' auth type; update the
test to exercise the actual sanitizer by calling
useProviderStore().fetchProviders() (or otherwise invoking sanitizeProviders
from providers.ts) with the generated rawProviders so the real sanitization path
is validated, and add 'custom_header' to the fc.constantFrom(...) list for
auth_type to cover that auth path; reference useProviderStore().fetchProviders()
and sanitizeProviders to locate the real sanitizer and SECRET_KEYS (api_key,
oauth_client_secret, custom_header_value) to ensure the assertions remain the
same against the store output.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: b2babd2e-8e0a-4eff-aa8e-0d65cea23aae
📒 Files selected for processing (1)
web/src/__tests__/stores/providers.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Test (Python 3.14)
- GitHub Check: Build Backend
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (2)
web/src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
web/src/**/*.{vue,ts,tsx}: Web Dashboard: Vue 3 + PrimeVue + Tailwind CSS, organized components by feature (agents/, approvals/, budget/, common/, dashboard/, layout/, messages/, org-chart/, tasks/). Use Pinia for state management.
Web Dashboard: ESLint linting and vue-tsc type checking required.
Files:
web/src/__tests__/stores/providers.test.ts
web/src/__tests__/**/*.{test,spec}.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Web Dashboard: use Vitest for unit tests with fast-check for property-based testing.
Files:
web/src/__tests__/stores/providers.test.ts
🧠 Learnings (3)
📚 Learning: 2026-03-18T07:33:37.988Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T07:33:37.988Z
Learning: Applies to web/src/__tests__/**/*.{test,spec}.ts : Web Dashboard: use Vitest for unit tests with fast-check for property-based testing.
Applied to files:
web/src/__tests__/stores/providers.test.ts
📚 Learning: 2026-03-18T07:33:37.988Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T07:33:37.988Z
Learning: Applies to tests/**/*.py : Tests must use `test-provider`, `test-small-001`, etc. instead of vendor names.
Applied to files:
web/src/__tests__/stores/providers.test.ts
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to tests/**/*.py : Tests must use test-provider, test-small-001, etc. for vendor-agnostic test data.
Applied to files:
web/src/__tests__/stores/providers.test.ts
🧬 Code graph analysis (1)
web/src/__tests__/stores/providers.test.ts (1)
web/src/api/types.ts (2)
ProviderConfig(464-476)ProviderPreset(519-527)
🪛 GitHub Check: Dashboard Lint
web/src/__tests__/stores/providers.test.ts
[warning] 240-240:
Function Call Object Injection Sink
[warning] 234-234:
Generic Object Injection Sink
[warning] 232-232:
Generic Object Injection Sink
🔇 Additional comments (2)
web/src/__tests__/stores/providers.test.ts (2)
58-170: Solid CRUD + error-path store coverage.These tests clearly validate baseline state transitions and refresh behavior after mutations.
252-283: Great stale-fetch regression test.Good coverage of out-of-order
fetchProviders()resolution and freshest-response retention.
- Property test now calls fetchProviders with mocked data so the real sanitizeProviders code path is exercised (not a reimplementation) - Added 'custom_header' to auth_type generator for full coverage - Fixed TS2352 cast errors: use 'as unknown as Record<string, unknown>' for ProviderConfig -> Record conversion (vue-tsc CI fix)
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/src/__tests__/stores/providers.test.ts`:
- Around line 53-55: In the beforeEach block that calls
setActivePinia(createPinia()), replace vi.clearAllMocks() with
vi.resetAllMocks() so mocks are fully reset (including
mockResolvedValue/mockRejectedValue and implementations) between tests; locate
the beforeEach that references setActivePinia, createPinia and vi.clearAllMocks
and change to vi.resetAllMocks() to make tests order-independent.
- Around line 205-243: The property filter in the arbitrary prevents exercising
prototype-pollution removal; update the test around the fc.dictionary usage so
that after generating rawProviders you inject a dedicated entry whose key is
each of the unsafe names (e.g., '__proto__', 'prototype', 'constructor') with
some benign value, then mock providersApi.listProviders with that augmented
object and call useProviderStore().fetchProviders(); finally assert that
store.providers does not have those unsafe keys and that the legitimate
providers still exist—this targets the sanitizer branch that drops UNSAFE_KEYS
while keeping the rest of fetchProviders/useProviderStore logic unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: df3e9a3d-74f5-471b-b856-b6298f9106a3
📒 Files selected for processing (1)
web/src/__tests__/stores/providers.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Test (Python 3.14)
- GitHub Check: Build Web
- GitHub Check: Build Backend
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (2)
web/src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
web/src/**/*.{vue,ts,tsx}: Web Dashboard: Vue 3 + PrimeVue + Tailwind CSS, organized components by feature (agents/, approvals/, budget/, common/, dashboard/, layout/, messages/, org-chart/, tasks/). Use Pinia for state management.
Web Dashboard: ESLint linting and vue-tsc type checking required.
Files:
web/src/__tests__/stores/providers.test.ts
web/src/__tests__/**/*.{test,spec}.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Web Dashboard: use Vitest for unit tests with fast-check for property-based testing.
Files:
web/src/__tests__/stores/providers.test.ts
🧠 Learnings (3)
📚 Learning: 2026-03-18T07:33:37.988Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T07:33:37.988Z
Learning: Applies to web/src/__tests__/**/*.{test,spec}.ts : Web Dashboard: use Vitest for unit tests with fast-check for property-based testing.
Applied to files:
web/src/__tests__/stores/providers.test.ts
📚 Learning: 2026-03-18T07:33:37.988Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T07:33:37.988Z
Learning: Applies to tests/**/*.py : Tests must use `test-provider`, `test-small-001`, etc. instead of vendor names.
Applied to files:
web/src/__tests__/stores/providers.test.ts
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to tests/**/*.py : Tests must use test-provider, test-small-001, etc. for vendor-agnostic test data.
Applied to files:
web/src/__tests__/stores/providers.test.ts
🧬 Code graph analysis (1)
web/src/__tests__/stores/providers.test.ts (1)
web/src/api/types.ts (2)
ProviderConfig(464-476)ProviderPreset(519-527)
🪛 GitHub Check: Dashboard Lint
web/src/__tests__/stores/providers.test.ts
[warning] 235-235:
Generic Object Injection Sink
- Replace vi.clearAllMocks() with vi.resetAllMocks() so mock implementations (mockResolvedValue) are fully reset between tests - Inject __proto__/prototype/constructor keys into generated provider data via Object.defineProperty so the sanitizer's UNSAFE_KEYS branch is actually exercised (previously filtered out by the generator) - Assert legitimate provider keys survive alongside unsafe key removal
🤖 I have created a release *beep* *boop* --- ## [0.3.3](v0.3.2...v0.3.3) (2026-03-18) ### Features * **backup:** implement automated backup and restore system ([#541](#541)) ([867b7c1](867b7c1)) * **providers:** runtime provider management with CRUD, presets, and multi-auth ([#540](#540)) ([936c345](936c345)), closes [#451](#451) * **tools:** wire per-category sandbox backend selection ([#534](#534)) ([311a1ab](311a1ab)) ### Bug Fixes * **cli:** switch cosign verification from .sig tags to OCI referrers ([#533](#533)) ([8ee5471](8ee5471)), closes [#532](#532) ### CI/CD * bump wrangler from 4.74.0 to 4.75.0 in /.github in the minor-and-patch group ([#535](#535)) ([de15867](de15867)) ### Maintenance * bump github.com/google/go-containerregistry from 0.21.2 to 0.21.3 in /cli in the minor-and-patch group ([#536](#536)) ([4a09aed](4a09aed)) * bump litellm from 1.82.3 to 1.82.4 in the minor-and-patch group ([#538](#538)) ([9f7f83d](9f7f83d)) * bump vue-tsc from 3.2.5 to 3.2.6 in /web in the minor-and-patch group across 1 directory ([#537](#537)) ([eb3dc4e](eb3dc4e)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
🤖 I have created a release *beep* *boop* --- ## [0.3.3](v0.3.2...v0.3.3) (2026-03-18) ### Features * **backup:** implement automated backup and restore system ([#541](#541)) ([867b7c1](867b7c1)) * **providers:** runtime provider management with CRUD, presets, and multi-auth ([#540](#540)) ([936c345](936c345)), closes [#451](#451) * **tools:** wire per-category sandbox backend selection ([#534](#534)) ([311a1ab](311a1ab)) ### Bug Fixes * **ci:** add COSIGN_EXPERIMENTAL=1 for OCI referrer mode in cosign sign ([#543](#543)) ([226ed2f](226ed2f)) * **cli:** switch cosign verification from .sig tags to OCI referrers ([#533](#533)) ([8ee5471](8ee5471)), closes [#532](#532) ### CI/CD * bump wrangler from 4.74.0 to 4.75.0 in /.github in the minor-and-patch group ([#535](#535)) ([de15867](de15867)) ### Maintenance * bump github.com/google/go-containerregistry from 0.21.2 to 0.21.3 in /cli in the minor-and-patch group ([#536](#536)) ([4a09aed](4a09aed)) * bump litellm from 1.82.3 to 1.82.4 in the minor-and-patch group ([#538](#538)) ([9f7f83d](9f7f83d)) * bump vue-tsc from 3.2.5 to 3.2.6 in /web in the minor-and-patch group across 1 directory ([#537](#537)) ([eb3dc4e](eb3dc4e)) * **main:** release 0.3.3 ([#539](#539)) ([c3de2a2](c3de2a2)) * revert v0.3.3 release artifacts (Docker signing failed) ([#544](#544)) ([7f48f52](7f48f52)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Summary
api_key,oauth,custom_header,nonewith encrypted credential storage via FernetAppStateslot swap under asyncio lock serializationProviderResponseDTO strips all secrets; provideshas_api_key/has_oauth_credentials/has_custom_headerboolean indicatorsChanges
Backend (Python)
AuthTypeenum (api_key/oauth/custom_header/none) inproviders/enums.pyProviderConfiginconfig/schema.pyProviderPresetmodel + 4 built-in presets inproviders/presets.pymatch/caseinLiteLLMDriver._build_kwargsCreateProviderRequest,UpdateProviderRequest,TestConnectionRequest/Response,ProviderResponse,CreateFromPresetRequestProviderManagementServiceinproviders/management/service.pywith lock-serialized CRUDProviderNotFoundErrorfor proper 404 vs 422 discriminationProviderControllerprovider_managementslot inAppStateFrontend (Vue 3 + TypeScript)
AuthType, provider request/response types,ProviderPresetinapi/types.tsapi/endpoints/providers.tsstores/providers.tsProviderCard.vue-- card with driver/auth badges, model count, edit/delete/testProviderFormDialog.vue-- create/edit with preset selector, auth-type-conditional fields, accessibility labelsProviderTestButton.vue-- inline test with latency display, auto-clear, proper cleanupSettingsPage.vueupdated with provider card grid replacing DataTableDocumentation
management/,providers/componentsdocs/design/operations.mdupdated with runtime CRUD, auth types, presets sectionPre-PR Review
Reviewed by 14 specialized agents, 23 findings addressed:
ProviderNotFoundErrorfor proper 404/422 error discrimination_validate_provider_nameextracted (DRY)UpdateProviderRequestmutual exclusion validatorTestConnectionResponseconsistency validatorhas_oauth_credentialschecks all 3 OAuth fieldsMappingProxyTypeon preset lookuponUnmountedtimer cleanupclear_api_key, name validationTest plan
Closes #451