From 7e2393d2954b3cc965344615a79a685486fdc5fd Mon Sep 17 00:00:00 2001 From: Aurelio <19254254+Aureliolo@users.noreply.github.com> Date: Tue, 17 Mar 2026 19:14:04 +0100 Subject: [PATCH 1/7] feat(settings): route structural data reads through SettingsService Fix config_bridge serialization to produce valid JSON for Pydantic models, tuples/lists, and dicts instead of Python repr strings. Register JSON setting definitions for agents, departments, and provider configs. Add get_json, get_agents, get_departments, and get_provider_configs accessors to ConfigResolver. Migrate 5 API controllers (10 endpoints) from direct RootConfig reads to resolver calls, enabling runtime DB overrides via the settings resolution chain (DB > env > YAML > code defaults). --- src/synthorg/api/controllers/agents.py | 10 +- src/synthorg/api/controllers/analytics.py | 2 +- src/synthorg/api/controllers/company.py | 10 +- src/synthorg/api/controllers/departments.py | 10 +- src/synthorg/api/controllers/providers.py | 11 +- src/synthorg/settings/config_bridge.py | 55 +++- src/synthorg/settings/definitions/company.py | 24 ++ .../settings/definitions/providers.py | 12 + src/synthorg/settings/resolver.py | 129 +++++++- tests/unit/api/controllers/test_agents.py | 58 ++++ tests/unit/api/controllers/test_analytics.py | 55 +++- tests/unit/api/controllers/test_company.py | 53 ++- .../unit/api/controllers/test_departments.py | 54 ++++ tests/unit/api/controllers/test_providers.py | 55 ++++ tests/unit/settings/test_config_bridge.py | 113 ++++++- tests/unit/settings/test_resolver.py | 302 ++++++++++++++++++ 16 files changed, 923 insertions(+), 30 deletions(-) diff --git a/src/synthorg/api/controllers/agents.py b/src/synthorg/api/controllers/agents.py index 40aa19f6ad..c100d83c73 100644 --- a/src/synthorg/api/controllers/agents.py +++ b/src/synthorg/api/controllers/agents.py @@ -40,11 +40,8 @@ async def list_agents( Paginated agent configurations. """ app_state: AppState = state.app_state - page, meta = paginate( - app_state.config.agents, - offset=offset, - limit=limit, - ) + agents = await app_state.config_resolver.get_agents() + page, meta = paginate(agents, offset=offset, limit=limit) return PaginatedResponse(data=page, pagination=meta) @get("/{agent_name:str}") @@ -66,7 +63,8 @@ async def get_agent( NotFoundError: If the agent is not found. """ app_state: AppState = state.app_state - for agent in app_state.config.agents: + agents = await app_state.config_resolver.get_agents() + for agent in agents: if agent.name == agent_name: return ApiResponse(data=agent) msg = f"Agent {agent_name!r} not found" diff --git a/src/synthorg/api/controllers/analytics.py b/src/synthorg/api/controllers/analytics.py index b3a01fd18e..5bd5363968 100644 --- a/src/synthorg/api/controllers/analytics.py +++ b/src/synthorg/api/controllers/analytics.py @@ -66,7 +66,7 @@ async def get_overview( data=OverviewMetrics( total_tasks=len(all_tasks), tasks_by_status=by_status, - total_agents=len(app_state.config.agents), + total_agents=len(await app_state.config_resolver.get_agents()), total_cost_usd=total_cost, ), ) diff --git a/src/synthorg/api/controllers/company.py b/src/synthorg/api/controllers/company.py index f44add92f3..4c295bb353 100644 --- a/src/synthorg/api/controllers/company.py +++ b/src/synthorg/api/controllers/company.py @@ -41,11 +41,12 @@ async def get_company( company_name = await app_state.config_resolver.get_str( "company", "company_name" ) - config = app_state.config + agents = await app_state.config_resolver.get_agents() + departments = await app_state.config_resolver.get_departments() data: dict[str, Any] = { "company_name": company_name, - "agents": [a.model_dump(mode="json") for a in config.agents], - "departments": [d.model_dump(mode="json") for d in config.departments], + "agents": [a.model_dump(mode="json") for a in agents], + "departments": [d.model_dump(mode="json") for d in departments], } return ApiResponse(data=data) @@ -63,4 +64,5 @@ async def list_departments( Departments envelope. """ app_state: AppState = state.app_state - return ApiResponse(data=app_state.config.departments) + departments = await app_state.config_resolver.get_departments() + return ApiResponse(data=departments) diff --git a/src/synthorg/api/controllers/departments.py b/src/synthorg/api/controllers/departments.py index 0c2f794d94..2eb4b3f65e 100644 --- a/src/synthorg/api/controllers/departments.py +++ b/src/synthorg/api/controllers/departments.py @@ -40,11 +40,8 @@ async def list_departments( Paginated department list. """ app_state: AppState = state.app_state - page, meta = paginate( - app_state.config.departments, - offset=offset, - limit=limit, - ) + departments = await app_state.config_resolver.get_departments() + page, meta = paginate(departments, offset=offset, limit=limit) return PaginatedResponse(data=page, pagination=meta) @get("/{name:str}") @@ -66,7 +63,8 @@ async def get_department( NotFoundError: If the department is not found. """ app_state: AppState = state.app_state - for dept in app_state.config.departments: + departments = await app_state.config_resolver.get_departments() + for dept in departments: if dept.name == name: return ApiResponse(data=dept) msg = f"Department {name!r} not found" diff --git a/src/synthorg/api/controllers/providers.py b/src/synthorg/api/controllers/providers.py index 2a7d88e4e9..24d1806797 100644 --- a/src/synthorg/api/controllers/providers.py +++ b/src/synthorg/api/controllers/providers.py @@ -43,9 +43,8 @@ async def list_providers( Provider configurations envelope (api_key stripped). """ app_state: AppState = state.app_state - safe = { - name: _safe_provider(p) for name, p in app_state.config.providers.items() - } + providers = await app_state.config_resolver.get_provider_configs() + safe = {name: _safe_provider(p) for name, p in providers.items()} return ApiResponse(data=safe) @get("/{name:str}") @@ -67,7 +66,8 @@ async def get_provider( NotFoundError: If the provider is not found. """ app_state: AppState = state.app_state - provider = app_state.config.providers.get(name) + providers = await app_state.config_resolver.get_provider_configs() + provider = providers.get(name) if provider is None: msg = f"Provider {name!r} not found" logger.warning(API_RESOURCE_NOT_FOUND, resource="provider", name=name) @@ -93,7 +93,8 @@ async def list_models( NotFoundError: If the provider is not found. """ app_state: AppState = state.app_state - provider = app_state.config.providers.get(name) + providers = await app_state.config_resolver.get_provider_configs() + provider = providers.get(name) if provider is None: msg = f"Provider {name!r} not found" logger.warning(API_RESOURCE_NOT_FOUND, resource="provider", name=name) diff --git a/src/synthorg/settings/config_bridge.py b/src/synthorg/settings/config_bridge.py index 1a1bfa6a58..ea111f02c0 100644 --- a/src/synthorg/settings/config_bridge.py +++ b/src/synthorg/settings/config_bridge.py @@ -4,18 +4,67 @@ ``RootConfig`` for YAML-layer resolution in the settings service. """ +import json + +from pydantic import BaseModel + from synthorg.observability import get_logger from synthorg.observability.events.settings import SETTINGS_CONFIG_PATH_MISS logger = get_logger(__name__) +def _serialize_value(value: object) -> str: + """Serialize a resolved config value to a string. + + Handles Pydantic models, collections of models, dicts with + model values, and plain collections by producing valid JSON. + Scalars fall back to ``str()``. + + Args: + value: The resolved config attribute. + + Returns: + A string representation suitable for the settings layer. + """ + if isinstance(value, BaseModel): + return json.dumps(value.model_dump(mode="json")) + + if isinstance(value, (tuple, list)): + if value and isinstance(value[0], BaseModel): + return json.dumps( + [ + item.model_dump(mode="json") + if isinstance(item, BaseModel) + else item + for item in value + ] + ) + return json.dumps(list(value)) + + if isinstance(value, dict): + if any(isinstance(v, BaseModel) for v in value.values()): + return json.dumps( + { + k: v.model_dump(mode="json") if isinstance(v, BaseModel) else v + for k, v in value.items() + } + ) + return json.dumps(value) + + return str(value) + + def extract_from_config(config: object, yaml_path: str) -> str | None: """Resolve a dotted path against a config object. Traverses the object attribute chain for each segment in - *yaml_path*. Returns ``str(value)`` if the final attribute - exists and is not ``None``, otherwise ``None``. + *yaml_path*. Returns a serialized string if the final + attribute exists and is not ``None``, otherwise ``None``. + + For Pydantic models, tuples/lists containing models, and + dicts with model values, the result is valid JSON. For + scalars, the result is ``str(value)``. Args: config: Root config object (typically ``RootConfig``). @@ -39,4 +88,4 @@ def extract_from_config(config: object, yaml_path: str) -> str | None: return None if current is None: return None - return str(current) + return _serialize_value(current) diff --git a/src/synthorg/settings/definitions/company.py b/src/synthorg/settings/definitions/company.py index 5cf9468804..4a025ad658 100644 --- a/src/synthorg/settings/definitions/company.py +++ b/src/synthorg/settings/definitions/company.py @@ -49,3 +49,27 @@ yaml_path="graceful_shutdown.grace_seconds", ) ) + +_r.register( + SettingDefinition( + namespace=SettingNamespace.COMPANY, + key="agents", + type=SettingType.JSON, + default=None, + description="Agent configurations (JSON array of AgentConfig objects)", + group="Structure", + yaml_path="agents", + ) +) + +_r.register( + SettingDefinition( + namespace=SettingNamespace.COMPANY, + key="departments", + type=SettingType.JSON, + default=None, + description="Department hierarchy (JSON array of Department objects)", + group="Structure", + yaml_path="departments", + ) +) diff --git a/src/synthorg/settings/definitions/providers.py b/src/synthorg/settings/definitions/providers.py index f627f3d0a8..95d3c49823 100644 --- a/src/synthorg/settings/definitions/providers.py +++ b/src/synthorg/settings/definitions/providers.py @@ -44,3 +44,15 @@ max_value=10, ) ) + +_r.register( + SettingDefinition( + namespace=SettingNamespace.PROVIDERS, + key="configs", + type=SettingType.JSON, + default=None, + description="LLM provider configurations (JSON object keyed by name)", + group="General", + yaml_path="providers", + ) +) diff --git a/src/synthorg/settings/resolver.py b/src/synthorg/settings/resolver.py index 94efc31ac4..9f58d7e42d 100644 --- a/src/synthorg/settings/resolver.py +++ b/src/synthorg/settings/resolver.py @@ -8,8 +8,9 @@ """ import asyncio +import json from enum import StrEnum -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Any from synthorg.observability import get_logger from synthorg.observability.events.settings import ( @@ -22,7 +23,8 @@ if TYPE_CHECKING: from synthorg.api.config import ApiConfig from synthorg.budget.config import BudgetAlertConfig, BudgetConfig - from synthorg.config.schema import RootConfig + from synthorg.config.schema import AgentConfig, ProviderConfig, RootConfig + from synthorg.core.company import Department from synthorg.core.enums import AutonomyLevel from synthorg.engine.coordination.config import CoordinationConfig from synthorg.settings.service import SettingsService @@ -261,6 +263,129 @@ async def get_autonomy_level(self) -> AutonomyLevel: return await self.get_enum("company", "autonomy_level", AutonomyLevel) + async def get_json(self, namespace: str, key: str) -> Any: + """Resolve a setting as parsed JSON. + + Args: + namespace: Setting namespace. + key: Setting key. + + Returns: + The parsed JSON value (list, dict, scalar, etc.). + + Raises: + SettingNotFoundError: If the key is not in the registry. + ValueError: If the value is not valid JSON. + """ + try: + result = await self._settings.get(namespace, key) + except SettingNotFoundError: + logger.warning( + SETTINGS_NOT_FOUND, + namespace=namespace, + key=key, + ) + raise + try: + return json.loads(result.value) + except json.JSONDecodeError: + logger.warning( + SETTINGS_VALIDATION_FAILED, + namespace=namespace, + key=key, + reason="invalid_json", + exc_info=True, + ) + msg = f"Setting {namespace}/{key} has an invalid JSON value" + raise ValueError(msg) from None + + async def get_agents(self) -> tuple[AgentConfig, ...]: + """Resolve agent configurations from settings. + + Falls back to ``RootConfig.agents`` if the setting is + empty or contains invalid JSON. + + Returns: + A tuple of ``AgentConfig`` objects. + + Raises: + SettingNotFoundError: If the agents key is not + in the registry. + """ + from synthorg.config.schema import AgentConfig # noqa: PLC0415 + + try: + raw = await self.get_json("company", "agents") + except ValueError: + logger.warning( + SETTINGS_FETCH_FAILED, + namespace="company", + key="agents", + reason="invalid_json_fallback", + ) + return self._config.agents + if not raw: + return self._config.agents + return tuple(AgentConfig.model_validate(item) for item in raw) + + async def get_departments(self) -> tuple[Department, ...]: + """Resolve department configurations from settings. + + Falls back to ``RootConfig.departments`` if the setting + is empty or contains invalid JSON. + + Returns: + A tuple of ``Department`` objects. + + Raises: + SettingNotFoundError: If the departments key is not + in the registry. + """ + from synthorg.core.company import Department # noqa: PLC0415 + + try: + raw = await self.get_json("company", "departments") + except ValueError: + logger.warning( + SETTINGS_FETCH_FAILED, + namespace="company", + key="departments", + reason="invalid_json_fallback", + ) + return self._config.departments + if not raw: + return self._config.departments + return tuple(Department.model_validate(item) for item in raw) + + async def get_provider_configs(self) -> dict[str, ProviderConfig]: + """Resolve provider configurations from settings. + + Falls back to ``RootConfig.providers`` if the setting + is empty or contains invalid JSON. + + Returns: + A dict of provider name to ``ProviderConfig``. + + Raises: + SettingNotFoundError: If the configs key is not + in the registry. + """ + from synthorg.config.schema import ProviderConfig # noqa: PLC0415 + + try: + raw = await self.get_json("providers", "configs") + except ValueError: + logger.warning( + SETTINGS_FETCH_FAILED, + namespace="providers", + key="configs", + reason="invalid_json_fallback", + ) + return self._config.providers + if not raw: + return self._config.providers + return {name: ProviderConfig.model_validate(conf) for name, conf in raw.items()} + async def get_budget_config(self) -> BudgetConfig: """Assemble a ``BudgetConfig`` from individually resolved settings. diff --git a/tests/unit/api/controllers/test_agents.py b/tests/unit/api/controllers/test_agents.py index 30c9578824..4b4e407e8e 100644 --- a/tests/unit/api/controllers/test_agents.py +++ b/tests/unit/api/controllers/test_agents.py @@ -1,11 +1,14 @@ """Tests for agent controller.""" +import json from typing import Any import pytest from litestar.testing import TestClient from synthorg.config.schema import AgentConfig, RootConfig +from synthorg.settings.registry import get_registry +from synthorg.settings.service import SettingsService from tests.unit.api.conftest import ( FakeMessageBus, FakePersistenceBackend, @@ -44,12 +47,18 @@ def test_list_agents_with_data( ) auth_service: AuthService = _make_test_auth_service() _seed_test_users(fake_persistence, auth_service) + settings_service = SettingsService( + repository=fake_persistence.settings, + registry=get_registry(), + config=config, + ) app = create_app( config=config, persistence=fake_persistence, message_bus=fake_message_bus, cost_tracker=CostTracker(), auth_service=auth_service, + settings_service=settings_service, ) with TestClient(app) as client: client.headers.update(make_auth_headers("observer")) @@ -62,3 +71,52 @@ def test_get_agent_not_found(self, test_client: TestClient[Any]) -> None: resp = test_client.get("/api/v1/agents/nonexistent") assert resp.status_code == 404 assert resp.json()["success"] is False + + +@pytest.mark.unit +class TestAgentControllerDbOverride: + """Test that DB-stored settings override YAML agents.""" + + async def test_db_agents_override_config( + self, + fake_persistence: FakePersistenceBackend, + fake_message_bus: FakeMessageBus, + ) -> None: + from synthorg.api.app import create_app + from synthorg.api.auth.service import AuthService + from synthorg.budget.tracker import CostTracker + from tests.unit.api.conftest import _make_test_auth_service, _seed_test_users + + config = RootConfig( + company_name="test", + agents=(AgentConfig(name="yaml-agent", role="dev", department="eng"),), + ) + auth_service: AuthService = _make_test_auth_service() + _seed_test_users(fake_persistence, auth_service) + settings_service = SettingsService( + repository=fake_persistence.settings, + registry=get_registry(), + config=config, + ) + + db_agents = [ + {"name": "db-agent-1", "role": "qa", "department": "eng"}, + {"name": "db-agent-2", "role": "pm", "department": "ops"}, + ] + await settings_service.set("company", "agents", json.dumps(db_agents)) + + app = create_app( + config=config, + persistence=fake_persistence, + message_bus=fake_message_bus, + cost_tracker=CostTracker(), + auth_service=auth_service, + settings_service=settings_service, + ) + with TestClient(app) as client: + client.headers.update(make_auth_headers("observer")) + resp = client.get("/api/v1/agents") + body = resp.json() + assert body["pagination"]["total"] == 2 + names = {a["name"] for a in body["data"]} + assert names == {"db-agent-1", "db-agent-2"} diff --git a/tests/unit/api/controllers/test_analytics.py b/tests/unit/api/controllers/test_analytics.py index 8002acd5b1..3a28fa7fd2 100644 --- a/tests/unit/api/controllers/test_analytics.py +++ b/tests/unit/api/controllers/test_analytics.py @@ -1,12 +1,20 @@ """Tests for analytics controller.""" +import json from typing import Any import pytest from litestar.testing import TestClient +from synthorg.config.schema import RootConfig from synthorg.core.enums import TaskStatus -from tests.unit.api.conftest import make_auth_headers +from synthorg.settings.registry import get_registry +from synthorg.settings.service import SettingsService +from tests.unit.api.conftest import ( + FakeMessageBus, + FakePersistenceBackend, + make_auth_headers, +) _HEADERS = make_auth_headers("ceo") @@ -31,3 +39,48 @@ def test_overview_requires_read_access(self, test_client: TestClient[Any]) -> No headers={"Authorization": "Bearer invalid-token"}, ) assert resp.status_code == 401 + + +@pytest.mark.unit +class TestAnalyticsControllerDbOverride: + """Test that DB-stored agents affect analytics agent count.""" + + async def test_db_agents_count_in_overview( + self, + fake_persistence: FakePersistenceBackend, + fake_message_bus: FakeMessageBus, + ) -> None: + from synthorg.api.app import create_app + from synthorg.api.auth.service import AuthService + from synthorg.budget.tracker import CostTracker + from tests.unit.api.conftest import _make_test_auth_service, _seed_test_users + + config = RootConfig(company_name="test") + auth_service: AuthService = _make_test_auth_service() + _seed_test_users(fake_persistence, auth_service) + settings_service = SettingsService( + repository=fake_persistence.settings, + registry=get_registry(), + config=config, + ) + + db_agents = [ + {"name": "a1", "role": "dev", "department": "eng"}, + {"name": "a2", "role": "qa", "department": "eng"}, + {"name": "a3", "role": "pm", "department": "ops"}, + ] + await settings_service.set("company", "agents", json.dumps(db_agents)) + + app = create_app( + config=config, + persistence=fake_persistence, + message_bus=fake_message_bus, + cost_tracker=CostTracker(), + auth_service=auth_service, + settings_service=settings_service, + ) + with TestClient(app) as client: + client.headers.update(make_auth_headers("ceo")) + resp = client.get("/api/v1/analytics/overview") + body = resp.json() + assert body["data"]["total_agents"] == 3 diff --git a/tests/unit/api/controllers/test_company.py b/tests/unit/api/controllers/test_company.py index 66295dcc81..86fa1db099 100644 --- a/tests/unit/api/controllers/test_company.py +++ b/tests/unit/api/controllers/test_company.py @@ -1,11 +1,19 @@ """Tests for company controller.""" +import json from typing import Any import pytest from litestar.testing import TestClient -from tests.unit.api.conftest import make_auth_headers +from synthorg.config.schema import RootConfig +from synthorg.settings.registry import get_registry +from synthorg.settings.service import SettingsService +from tests.unit.api.conftest import ( + FakeMessageBus, + FakePersistenceBackend, + make_auth_headers, +) _HEADERS = make_auth_headers("ceo") @@ -32,3 +40,46 @@ def test_company_requires_read_access(self, test_client: TestClient[Any]) -> Non headers={"Authorization": "Bearer invalid-token"}, ) assert resp.status_code == 401 + + +@pytest.mark.unit +class TestCompanyControllerDbOverride: + """Test that DB-stored settings override YAML company data.""" + + async def test_db_company_departments_override( + self, + fake_persistence: FakePersistenceBackend, + fake_message_bus: FakeMessageBus, + ) -> None: + from synthorg.api.app import create_app + from synthorg.api.auth.service import AuthService + from synthorg.budget.tracker import CostTracker + from tests.unit.api.conftest import _make_test_auth_service, _seed_test_users + + config = RootConfig(company_name="test") + auth_service: AuthService = _make_test_auth_service() + _seed_test_users(fake_persistence, auth_service) + settings_service = SettingsService( + repository=fake_persistence.settings, + registry=get_registry(), + config=config, + ) + + db_depts = [{"name": "db-sales", "head": "bob"}] + await settings_service.set("company", "departments", json.dumps(db_depts)) + + app = create_app( + config=config, + persistence=fake_persistence, + message_bus=fake_message_bus, + cost_tracker=CostTracker(), + auth_service=auth_service, + settings_service=settings_service, + ) + with TestClient(app) as client: + client.headers.update(make_auth_headers("ceo")) + resp = client.get("/api/v1/company/departments") + body = resp.json() + assert body["success"] is True + assert len(body["data"]) == 1 + assert body["data"][0]["name"] == "db-sales" diff --git a/tests/unit/api/controllers/test_departments.py b/tests/unit/api/controllers/test_departments.py index 21fa024b01..9f5687b8a9 100644 --- a/tests/unit/api/controllers/test_departments.py +++ b/tests/unit/api/controllers/test_departments.py @@ -1,10 +1,20 @@ """Tests for department controller.""" +import json from typing import Any import pytest from litestar.testing import TestClient +from synthorg.config.schema import RootConfig +from synthorg.settings.registry import get_registry +from synthorg.settings.service import SettingsService +from tests.unit.api.conftest import ( + FakeMessageBus, + FakePersistenceBackend, + make_auth_headers, +) + @pytest.mark.unit class TestDepartmentController: @@ -19,3 +29,47 @@ def test_get_department_not_found(self, test_client: TestClient[Any]) -> None: resp = test_client.get("/api/v1/departments/nonexistent") assert resp.status_code == 404 assert resp.json()["success"] is False + + +@pytest.mark.unit +class TestDepartmentControllerDbOverride: + """Test that DB-stored settings override YAML departments.""" + + async def test_db_departments_override_config( + self, + fake_persistence: FakePersistenceBackend, + fake_message_bus: FakeMessageBus, + ) -> None: + from synthorg.api.app import create_app + from synthorg.api.auth.service import AuthService + from synthorg.budget.tracker import CostTracker + from tests.unit.api.conftest import _make_test_auth_service, _seed_test_users + + config = RootConfig(company_name="test") + auth_service: AuthService = _make_test_auth_service() + _seed_test_users(fake_persistence, auth_service) + settings_service = SettingsService( + repository=fake_persistence.settings, + registry=get_registry(), + config=config, + ) + + db_depts = [ + {"name": "db-dept", "head": "alice"}, + ] + await settings_service.set("company", "departments", json.dumps(db_depts)) + + app = create_app( + config=config, + persistence=fake_persistence, + message_bus=fake_message_bus, + cost_tracker=CostTracker(), + auth_service=auth_service, + settings_service=settings_service, + ) + with TestClient(app) as client: + client.headers.update(make_auth_headers("observer")) + resp = client.get("/api/v1/departments") + body = resp.json() + assert body["pagination"]["total"] == 1 + assert body["data"][0]["name"] == "db-dept" diff --git a/tests/unit/api/controllers/test_providers.py b/tests/unit/api/controllers/test_providers.py index 1b2b3db58f..b262084ed9 100644 --- a/tests/unit/api/controllers/test_providers.py +++ b/tests/unit/api/controllers/test_providers.py @@ -1,10 +1,20 @@ """Tests for provider controller.""" +import json from typing import Any import pytest from litestar.testing import TestClient +from synthorg.config.schema import RootConfig +from synthorg.settings.registry import get_registry +from synthorg.settings.service import SettingsService +from tests.unit.api.conftest import ( + FakeMessageBus, + FakePersistenceBackend, + make_auth_headers, +) + @pytest.mark.unit class TestProviderController: @@ -40,3 +50,48 @@ def test_provider_api_key_stripped( ) safe = _safe_provider(provider) assert safe.api_key is None + + +@pytest.mark.unit +class TestProviderControllerDbOverride: + """Test that DB-stored settings override YAML providers.""" + + async def test_db_providers_override_config( + self, + fake_persistence: FakePersistenceBackend, + fake_message_bus: FakeMessageBus, + ) -> None: + from synthorg.api.app import create_app + from synthorg.api.auth.service import AuthService + from synthorg.budget.tracker import CostTracker + from tests.unit.api.conftest import _make_test_auth_service, _seed_test_users + + config = RootConfig(company_name="test") + auth_service: AuthService = _make_test_auth_service() + _seed_test_users(fake_persistence, auth_service) + settings_service = SettingsService( + repository=fake_persistence.settings, + registry=get_registry(), + config=config, + ) + + db_providers = { + "db-provider": {"driver": "litellm"}, + } + await settings_service.set("providers", "configs", json.dumps(db_providers)) + + app = create_app( + config=config, + persistence=fake_persistence, + message_bus=fake_message_bus, + cost_tracker=CostTracker(), + auth_service=auth_service, + settings_service=settings_service, + ) + with TestClient(app) as client: + client.headers.update(make_auth_headers("observer")) + resp = client.get("/api/v1/providers") + body = resp.json() + assert "db-provider" in body["data"] + # api_key should be stripped + assert body["data"]["db-provider"].get("api_key") is None diff --git a/tests/unit/settings/test_config_bridge.py b/tests/unit/settings/test_config_bridge.py index ef418932a9..4aa67d7217 100644 --- a/tests/unit/settings/test_config_bridge.py +++ b/tests/unit/settings/test_config_bridge.py @@ -1,9 +1,11 @@ """Unit tests for config bridge.""" +import json + import pytest from pydantic import BaseModel, ConfigDict -from synthorg.settings.config_bridge import extract_from_config +from synthorg.settings.config_bridge import _serialize_value, extract_from_config class _InnerConfig(BaseModel): @@ -12,11 +14,20 @@ class _InnerConfig(BaseModel): enabled: bool = True +class _ItemModel(BaseModel): + model_config = ConfigDict(frozen=True) + name: str = "item" + value: int = 1 + + class _FakeConfig(BaseModel): model_config = ConfigDict(frozen=True) company_name: str = "TestCo" budget: _InnerConfig = _InnerConfig() optional_field: str | None = None + items: tuple[_ItemModel, ...] = () + providers: dict[str, _InnerConfig] = {} + tags: tuple[str, ...] = () @pytest.mark.unit @@ -52,3 +63,103 @@ def test_empty_path(self) -> None: config = _FakeConfig() # Empty string splits to [''] — getattr('') fails assert extract_from_config(config, "") is None + + +@pytest.mark.unit +@pytest.mark.timeout(30) +class TestSerializeValue: + """Tests for _serialize_value() helper.""" + + def test_single_model(self) -> None: + model = _InnerConfig(daily_limit=20.0, enabled=False) + result = _serialize_value(model) + parsed = json.loads(result) + assert parsed == {"daily_limit": 20.0, "enabled": False} + + def test_tuple_of_models(self) -> None: + items = ( + _ItemModel(name="a", value=1), + _ItemModel(name="b", value=2), + ) + result = _serialize_value(items) + parsed = json.loads(result) + assert parsed == [ + {"name": "a", "value": 1}, + {"name": "b", "value": 2}, + ] + + def test_dict_of_models(self) -> None: + providers = { + "p1": _InnerConfig(daily_limit=5.0), + "p2": _InnerConfig(daily_limit=15.0), + } + result = _serialize_value(providers) + parsed = json.loads(result) + assert parsed["p1"]["daily_limit"] == 5.0 + assert parsed["p2"]["daily_limit"] == 15.0 + + def test_tuple_of_strings(self) -> None: + tags = ("alpha", "beta", "gamma") + result = _serialize_value(tags) + assert json.loads(result) == ["alpha", "beta", "gamma"] + + def test_empty_tuple(self) -> None: + result = _serialize_value(()) + assert result == "[]" + + def test_empty_dict(self) -> None: + result = _serialize_value({}) + assert result == "{}" + + def test_scalar_string_unchanged(self) -> None: + assert _serialize_value("hello") == "hello" + + def test_scalar_int_unchanged(self) -> None: + assert _serialize_value(42) == "42" + + def test_scalar_float_unchanged(self) -> None: + assert _serialize_value(3.14) == "3.14" + + def test_scalar_bool_unchanged(self) -> None: + assert _serialize_value(True) == "True" + + +@pytest.mark.unit +@pytest.mark.timeout(30) +class TestExtractFromConfigStructural: + """Tests for extract_from_config with structural data types.""" + + def test_single_model_produces_json(self) -> None: + config = _FakeConfig() + result = extract_from_config(config, "budget") + assert result is not None + parsed = json.loads(result) + assert parsed == {"daily_limit": 10.0, "enabled": True} + + def test_tuple_of_models_produces_json(self) -> None: + config = _FakeConfig( + items=(_ItemModel(name="x", value=9),), + ) + result = extract_from_config(config, "items") + assert result is not None + parsed = json.loads(result) + assert parsed == [{"name": "x", "value": 9}] + + def test_dict_of_models_produces_json(self) -> None: + config = _FakeConfig( + providers={"test": _InnerConfig(daily_limit=7.0)}, + ) + result = extract_from_config(config, "providers") + assert result is not None + parsed = json.loads(result) + assert parsed["test"]["daily_limit"] == 7.0 + + def test_empty_tuple_produces_json(self) -> None: + config = _FakeConfig(items=()) + result = extract_from_config(config, "items") + assert result == "[]" + + def test_empty_dict_produces_json(self) -> None: + config = _FakeConfig(providers={}) + result = extract_from_config(config, "providers") + assert result == "{}" diff --git a/tests/unit/settings/test_resolver.py b/tests/unit/settings/test_resolver.py index f915d7b3d6..e63948f8fc 100644 --- a/tests/unit/settings/test_resolver.py +++ b/tests/unit/settings/test_resolver.py @@ -1,5 +1,6 @@ """Unit tests for ConfigResolver.""" +import json from enum import StrEnum from typing import Literal from unittest.mock import AsyncMock @@ -113,12 +114,33 @@ class _CompanyConfig(BaseModel): model_config = ConfigDict(frozen=True) +class _FakeAgentConfig(BaseModel): + model_config = ConfigDict(frozen=True) + name: str = "agent-1" + role: str = "developer" + department: str = "eng" + + +class _FakeDepartment(BaseModel): + model_config = ConfigDict(frozen=True) + name: str = "eng" + head: str = "lead" + + +class _FakeProviderConfig(BaseModel): + model_config = ConfigDict(frozen=True) + driver: str = "litellm" + + class _FakeRootConfig(BaseModel): model_config = ConfigDict(frozen=True) api: _FakeApiConfig = _FakeApiConfig() budget: _BudgetConfig = _BudgetConfig() coordination: _CoordinationSection = _CoordinationSection() config: _CompanyConfig = _CompanyConfig() + agents: tuple[_FakeAgentConfig, ...] = () + departments: tuple[_FakeDepartment, ...] = () + providers: dict[str, _FakeProviderConfig] = {} # ── Fixtures ────────────────────────────────────────────────────── @@ -803,3 +825,283 @@ async def test_invalid_enum_value_propagates( ) with pytest.raises(ValueError, match=r"invalid.*RateLimitTimeUnit"): await resolver.get_api_config() + + +# ── JSON Accessor Tests ────────────────────────────────────────── + + +@pytest.mark.unit +@pytest.mark.timeout(30) +class TestGetJson: + """Tests for get_json() generic accessor.""" + + async def test_valid_json_array( + self, resolver: ConfigResolver, mock_settings: AsyncMock + ) -> None: + mock_settings.get.return_value = _make_value('[{"name": "a"}]') + result = await resolver.get_json("company", "agents") + assert result == [{"name": "a"}] + mock_settings.get.assert_awaited_once_with("company", "agents") + + async def test_valid_json_object( + self, resolver: ConfigResolver, mock_settings: AsyncMock + ) -> None: + mock_settings.get.return_value = _make_value('{"k": "v"}') + result = await resolver.get_json("test", "key") + assert result == {"k": "v"} + + async def test_invalid_json_raises_value_error( + self, resolver: ConfigResolver, mock_settings: AsyncMock + ) -> None: + mock_settings.get.return_value = _make_value("not-json{}") + with pytest.raises(ValueError, match="invalid JSON"): + await resolver.get_json("test", "key") + + async def test_not_found_propagates( + self, resolver: ConfigResolver, mock_settings: AsyncMock + ) -> None: + mock_settings.get.side_effect = SettingNotFoundError("nope") + with pytest.raises(SettingNotFoundError): + await resolver.get_json("bad", "key") + + async def test_empty_array( + self, resolver: ConfigResolver, mock_settings: AsyncMock + ) -> None: + mock_settings.get.return_value = _make_value("[]") + assert await resolver.get_json("test", "key") == [] + + +# ── Composed Read: Agents ──────────────────────────────────────── + + +@pytest.mark.unit +@pytest.mark.timeout(30) +class TestGetAgents: + """Tests for get_agents() composed read.""" + + async def test_json_roundtrip(self, mock_settings: AsyncMock) -> None: + """Agent configs parsed from JSON setting.""" + from synthorg.config.schema import AgentConfig + + agent_data = [ + {"name": "alice", "role": "dev", "department": "eng"}, + {"name": "bob", "role": "qa", "department": "eng"}, + ] + mock_settings.get.return_value = _make_value( + json.dumps(agent_data), + namespace=SettingNamespace.COMPANY, + key="agents", + ) + config = _FakeRootConfig() + resolver = ConfigResolver( + settings_service=mock_settings, + config=config, # type: ignore[arg-type] + ) + result = await resolver.get_agents() + + assert len(result) == 2 + assert isinstance(result[0], AgentConfig) + assert result[0].name == "alice" + assert result[1].name == "bob" + + async def test_empty_list_falls_back_to_config( + self, mock_settings: AsyncMock + ) -> None: + """Empty JSON list → fall back to config.agents.""" + mock_settings.get.return_value = _make_value( + "[]", + namespace=SettingNamespace.COMPANY, + key="agents", + ) + agent = _FakeAgentConfig(name="fallback-agent") + config = _FakeRootConfig(agents=(agent,)) + resolver = ConfigResolver( + settings_service=mock_settings, + config=config, # type: ignore[arg-type] + ) + result = await resolver.get_agents() + + assert len(result) == 1 + assert result[0].name == "fallback-agent" + + async def test_invalid_json_falls_back_to_config( + self, mock_settings: AsyncMock + ) -> None: + """Invalid JSON → fall back to config.agents.""" + mock_settings.get.return_value = _make_value( + "not-json", + namespace=SettingNamespace.COMPANY, + key="agents", + ) + agent = _FakeAgentConfig(name="safe-agent") + config = _FakeRootConfig(agents=(agent,)) + resolver = ConfigResolver( + settings_service=mock_settings, + config=config, # type: ignore[arg-type] + ) + result = await resolver.get_agents() + + assert len(result) == 1 + assert result[0].name == "safe-agent" + + async def test_not_found_propagates( + self, resolver: ConfigResolver, mock_settings: AsyncMock + ) -> None: + mock_settings.get.side_effect = SettingNotFoundError("nope") + with pytest.raises(SettingNotFoundError): + await resolver.get_agents() + + +# ── Composed Read: Departments ─────────────────────────────────── + + +@pytest.mark.unit +@pytest.mark.timeout(30) +class TestGetDepartments: + """Tests for get_departments() composed read.""" + + async def test_json_roundtrip(self, mock_settings: AsyncMock) -> None: + """Departments parsed from JSON setting.""" + from synthorg.core.company import Department + + dept_data = [ + {"name": "engineering", "head": "alice"}, + ] + mock_settings.get.return_value = _make_value( + json.dumps(dept_data), + namespace=SettingNamespace.COMPANY, + key="departments", + ) + config = _FakeRootConfig() + resolver = ConfigResolver( + settings_service=mock_settings, + config=config, # type: ignore[arg-type] + ) + result = await resolver.get_departments() + + assert len(result) == 1 + assert isinstance(result[0], Department) + assert result[0].name == "engineering" + + async def test_empty_list_falls_back_to_config( + self, mock_settings: AsyncMock + ) -> None: + mock_settings.get.return_value = _make_value( + "[]", + namespace=SettingNamespace.COMPANY, + key="departments", + ) + dept = _FakeDepartment(name="fallback-dept") + config = _FakeRootConfig(departments=(dept,)) + resolver = ConfigResolver( + settings_service=mock_settings, + config=config, # type: ignore[arg-type] + ) + result = await resolver.get_departments() + + assert len(result) == 1 + assert result[0].name == "fallback-dept" + + async def test_invalid_json_falls_back_to_config( + self, mock_settings: AsyncMock + ) -> None: + mock_settings.get.return_value = _make_value( + "{bad-json", + namespace=SettingNamespace.COMPANY, + key="departments", + ) + dept = _FakeDepartment(name="safe-dept") + config = _FakeRootConfig(departments=(dept,)) + resolver = ConfigResolver( + settings_service=mock_settings, + config=config, # type: ignore[arg-type] + ) + result = await resolver.get_departments() + + assert len(result) == 1 + assert result[0].name == "safe-dept" + + async def test_not_found_propagates( + self, resolver: ConfigResolver, mock_settings: AsyncMock + ) -> None: + mock_settings.get.side_effect = SettingNotFoundError("nope") + with pytest.raises(SettingNotFoundError): + await resolver.get_departments() + + +# ── Composed Read: Provider Configs ────────────────────────────── + + +@pytest.mark.unit +@pytest.mark.timeout(30) +class TestGetProviderConfigs: + """Tests for get_provider_configs() composed read.""" + + async def test_json_roundtrip(self, mock_settings: AsyncMock) -> None: + """Provider configs parsed from JSON setting.""" + from synthorg.config.schema import ProviderConfig + + prov_data = { + "test-provider": {"driver": "litellm"}, + } + mock_settings.get.return_value = _make_value( + json.dumps(prov_data), + namespace=SettingNamespace.PROVIDERS, + key="configs", + ) + config = _FakeRootConfig() + resolver = ConfigResolver( + settings_service=mock_settings, + config=config, # type: ignore[arg-type] + ) + result = await resolver.get_provider_configs() + + assert "test-provider" in result + assert isinstance(result["test-provider"], ProviderConfig) + assert result["test-provider"].driver == "litellm" + + async def test_empty_dict_falls_back_to_config( + self, mock_settings: AsyncMock + ) -> None: + mock_settings.get.return_value = _make_value( + "{}", + namespace=SettingNamespace.PROVIDERS, + key="configs", + ) + config = _FakeRootConfig( + providers={"fallback": _FakeProviderConfig(driver="test-driver")}, + ) + resolver = ConfigResolver( + settings_service=mock_settings, + config=config, # type: ignore[arg-type] + ) + result = await resolver.get_provider_configs() + + assert "fallback" in result + assert result["fallback"].driver == "test-driver" + + async def test_invalid_json_falls_back_to_config( + self, mock_settings: AsyncMock + ) -> None: + mock_settings.get.return_value = _make_value( + "not-valid-json", + namespace=SettingNamespace.PROVIDERS, + key="configs", + ) + config = _FakeRootConfig( + providers={"safe": _FakeProviderConfig()}, + ) + resolver = ConfigResolver( + settings_service=mock_settings, + config=config, # type: ignore[arg-type] + ) + result = await resolver.get_provider_configs() + + assert "safe" in result + + async def test_not_found_propagates( + self, resolver: ConfigResolver, mock_settings: AsyncMock + ) -> None: + mock_settings.get.side_effect = SettingNotFoundError("nope") + with pytest.raises(SettingNotFoundError): + await resolver.get_provider_configs() From 730efa58d3f2a9aca48457cfede2c647ea0ec3bd Mon Sep 17 00:00:00 2001 From: Aurelio <19254254+Aureliolo@users.noreply.github.com> Date: Tue, 17 Mar 2026 19:40:06 +0100 Subject: [PATCH 2/7] fix(settings): harden structural data resolver with validation fallbacks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pre-reviewed by 12 agents, 11 findings addressed: - Catch ValidationError from model_validate() in get_agents, get_departments, get_provider_configs — fall back to YAML config instead of producing unhandled 500 errors - Add isinstance guards for JSON shape (list vs dict) before iterating or calling .items() - Use any() instead of value[0] check in _serialize_value for consistent heterogeneous collection handling - Return dict copy from provider fallback to prevent mutation of frozen config state - Parallelize 3 sequential awaits in CompanyController.get_company using asyncio.TaskGroup - Extract structural data tests to test_resolver_structural.py (keep test_resolver.py under 800 lines) - Add @pytest.mark.timeout(30) to all controller test classes - Add DB-override test for GET /company endpoint - Add ValidationError and wrong-shape fallback tests - Strengthen weak assertion in test_company.py --- src/synthorg/api/controllers/company.py | 17 +- src/synthorg/settings/config_bridge.py | 2 +- src/synthorg/settings/resolver.py | 78 ++- tests/unit/api/controllers/test_agents.py | 2 + tests/unit/api/controllers/test_analytics.py | 2 + tests/unit/api/controllers/test_company.py | 42 +- .../unit/api/controllers/test_departments.py | 2 + tests/unit/api/controllers/test_providers.py | 3 + tests/unit/settings/test_resolver.py | 281 ----------- .../unit/settings/test_resolver_structural.py | 460 ++++++++++++++++++ 10 files changed, 590 insertions(+), 299 deletions(-) create mode 100644 tests/unit/settings/test_resolver_structural.py diff --git a/src/synthorg/api/controllers/company.py b/src/synthorg/api/controllers/company.py index 4c295bb353..443e5bc67a 100644 --- a/src/synthorg/api/controllers/company.py +++ b/src/synthorg/api/controllers/company.py @@ -1,5 +1,6 @@ """Company configuration controller.""" +import asyncio from typing import Any from litestar import Controller, get @@ -38,15 +39,15 @@ async def get_company( Company configuration envelope. """ app_state: AppState = state.app_state - company_name = await app_state.config_resolver.get_str( - "company", "company_name" - ) - agents = await app_state.config_resolver.get_agents() - departments = await app_state.config_resolver.get_departments() + resolver = app_state.config_resolver + async with asyncio.TaskGroup() as tg: + t_name = tg.create_task(resolver.get_str("company", "company_name")) + t_agents = tg.create_task(resolver.get_agents()) + t_depts = tg.create_task(resolver.get_departments()) data: dict[str, Any] = { - "company_name": company_name, - "agents": [a.model_dump(mode="json") for a in agents], - "departments": [d.model_dump(mode="json") for d in departments], + "company_name": t_name.result(), + "agents": [a.model_dump(mode="json") for a in t_agents.result()], + "departments": [d.model_dump(mode="json") for d in t_depts.result()], } return ApiResponse(data=data) diff --git a/src/synthorg/settings/config_bridge.py b/src/synthorg/settings/config_bridge.py index ea111f02c0..f1806c7eef 100644 --- a/src/synthorg/settings/config_bridge.py +++ b/src/synthorg/settings/config_bridge.py @@ -31,7 +31,7 @@ def _serialize_value(value: object) -> str: return json.dumps(value.model_dump(mode="json")) if isinstance(value, (tuple, list)): - if value and isinstance(value[0], BaseModel): + if any(isinstance(item, BaseModel) for item in value): return json.dumps( [ item.model_dump(mode="json") diff --git a/src/synthorg/settings/resolver.py b/src/synthorg/settings/resolver.py index 9f58d7e42d..5cf2720e61 100644 --- a/src/synthorg/settings/resolver.py +++ b/src/synthorg/settings/resolver.py @@ -303,7 +303,7 @@ async def get_agents(self) -> tuple[AgentConfig, ...]: """Resolve agent configurations from settings. Falls back to ``RootConfig.agents`` if the setting is - empty or contains invalid JSON. + empty, contains invalid JSON, or fails schema validation. Returns: A tuple of ``AgentConfig`` objects. @@ -312,6 +312,8 @@ async def get_agents(self) -> tuple[AgentConfig, ...]: SettingNotFoundError: If the agents key is not in the registry. """ + from pydantic import ValidationError # noqa: PLC0415 + from synthorg.config.schema import AgentConfig # noqa: PLC0415 try: @@ -326,13 +328,31 @@ async def get_agents(self) -> tuple[AgentConfig, ...]: return self._config.agents if not raw: return self._config.agents - return tuple(AgentConfig.model_validate(item) for item in raw) + if not isinstance(raw, list): + logger.warning( + SETTINGS_FETCH_FAILED, + namespace="company", + key="agents", + reason="expected_list_fallback", + ) + return self._config.agents + try: + return tuple(AgentConfig.model_validate(item) for item in raw) + except ValidationError: + logger.warning( + SETTINGS_FETCH_FAILED, + namespace="company", + key="agents", + reason="invalid_schema_fallback", + exc_info=True, + ) + return self._config.agents async def get_departments(self) -> tuple[Department, ...]: """Resolve department configurations from settings. Falls back to ``RootConfig.departments`` if the setting - is empty or contains invalid JSON. + is empty, contains invalid JSON, or fails schema validation. Returns: A tuple of ``Department`` objects. @@ -341,6 +361,8 @@ async def get_departments(self) -> tuple[Department, ...]: SettingNotFoundError: If the departments key is not in the registry. """ + from pydantic import ValidationError # noqa: PLC0415 + from synthorg.core.company import Department # noqa: PLC0415 try: @@ -355,13 +377,31 @@ async def get_departments(self) -> tuple[Department, ...]: return self._config.departments if not raw: return self._config.departments - return tuple(Department.model_validate(item) for item in raw) + if not isinstance(raw, list): + logger.warning( + SETTINGS_FETCH_FAILED, + namespace="company", + key="departments", + reason="expected_list_fallback", + ) + return self._config.departments + try: + return tuple(Department.model_validate(item) for item in raw) + except ValidationError: + logger.warning( + SETTINGS_FETCH_FAILED, + namespace="company", + key="departments", + reason="invalid_schema_fallback", + exc_info=True, + ) + return self._config.departments async def get_provider_configs(self) -> dict[str, ProviderConfig]: """Resolve provider configurations from settings. Falls back to ``RootConfig.providers`` if the setting - is empty or contains invalid JSON. + is empty, contains invalid JSON, or fails schema validation. Returns: A dict of provider name to ``ProviderConfig``. @@ -370,6 +410,8 @@ async def get_provider_configs(self) -> dict[str, ProviderConfig]: SettingNotFoundError: If the configs key is not in the registry. """ + from pydantic import ValidationError # noqa: PLC0415 + from synthorg.config.schema import ProviderConfig # noqa: PLC0415 try: @@ -381,10 +423,30 @@ async def get_provider_configs(self) -> dict[str, ProviderConfig]: key="configs", reason="invalid_json_fallback", ) - return self._config.providers + return dict(self._config.providers) if not raw: - return self._config.providers - return {name: ProviderConfig.model_validate(conf) for name, conf in raw.items()} + return dict(self._config.providers) + if not isinstance(raw, dict): + logger.warning( + SETTINGS_FETCH_FAILED, + namespace="providers", + key="configs", + reason="expected_dict_fallback", + ) + return dict(self._config.providers) + try: + return { + name: ProviderConfig.model_validate(conf) for name, conf in raw.items() + } + except ValidationError: + logger.warning( + SETTINGS_FETCH_FAILED, + namespace="providers", + key="configs", + reason="invalid_schema_fallback", + exc_info=True, + ) + return dict(self._config.providers) async def get_budget_config(self) -> BudgetConfig: """Assemble a ``BudgetConfig`` from individually resolved settings. diff --git a/tests/unit/api/controllers/test_agents.py b/tests/unit/api/controllers/test_agents.py index 4b4e407e8e..2d8890d946 100644 --- a/tests/unit/api/controllers/test_agents.py +++ b/tests/unit/api/controllers/test_agents.py @@ -17,6 +17,7 @@ @pytest.mark.unit +@pytest.mark.timeout(30) class TestAgentController: def test_list_agents_empty(self, test_client: TestClient[Any]) -> None: resp = test_client.get("/api/v1/agents") @@ -74,6 +75,7 @@ def test_get_agent_not_found(self, test_client: TestClient[Any]) -> None: @pytest.mark.unit +@pytest.mark.timeout(30) class TestAgentControllerDbOverride: """Test that DB-stored settings override YAML agents.""" diff --git a/tests/unit/api/controllers/test_analytics.py b/tests/unit/api/controllers/test_analytics.py index 3a28fa7fd2..74666491b6 100644 --- a/tests/unit/api/controllers/test_analytics.py +++ b/tests/unit/api/controllers/test_analytics.py @@ -20,6 +20,7 @@ @pytest.mark.unit +@pytest.mark.timeout(30) class TestAnalyticsController: def test_overview_empty(self, test_client: TestClient[Any]) -> None: resp = test_client.get("/api/v1/analytics/overview", headers=_HEADERS) @@ -42,6 +43,7 @@ def test_overview_requires_read_access(self, test_client: TestClient[Any]) -> No @pytest.mark.unit +@pytest.mark.timeout(30) class TestAnalyticsControllerDbOverride: """Test that DB-stored agents affect analytics agent count.""" diff --git a/tests/unit/api/controllers/test_company.py b/tests/unit/api/controllers/test_company.py index 86fa1db099..127a8826c5 100644 --- a/tests/unit/api/controllers/test_company.py +++ b/tests/unit/api/controllers/test_company.py @@ -19,6 +19,7 @@ @pytest.mark.unit +@pytest.mark.timeout(30) class TestCompanyController: def test_get_company(self, test_client: TestClient[Any]) -> None: resp = test_client.get("/api/v1/company", headers=_HEADERS) @@ -32,7 +33,7 @@ def test_list_departments(self, test_client: TestClient[Any]) -> None: assert resp.status_code == 200 body = resp.json() assert body["success"] is True - assert isinstance(body["data"], list) + assert body["data"] == [] def test_company_requires_read_access(self, test_client: TestClient[Any]) -> None: resp = test_client.get( @@ -43,6 +44,7 @@ def test_company_requires_read_access(self, test_client: TestClient[Any]) -> Non @pytest.mark.unit +@pytest.mark.timeout(30) class TestCompanyControllerDbOverride: """Test that DB-stored settings override YAML company data.""" @@ -83,3 +85,41 @@ async def test_db_company_departments_override( assert body["success"] is True assert len(body["data"]) == 1 assert body["data"][0]["name"] == "db-sales" + + async def test_db_company_overview_includes_db_agents( + self, + fake_persistence: FakePersistenceBackend, + fake_message_bus: FakeMessageBus, + ) -> None: + from synthorg.api.app import create_app + from synthorg.api.auth.service import AuthService + from synthorg.budget.tracker import CostTracker + from tests.unit.api.conftest import _make_test_auth_service, _seed_test_users + + config = RootConfig(company_name="test") + auth_service: AuthService = _make_test_auth_service() + _seed_test_users(fake_persistence, auth_service) + settings_service = SettingsService( + repository=fake_persistence.settings, + registry=get_registry(), + config=config, + ) + + db_agents = [{"name": "db-agent", "role": "dev", "department": "eng"}] + await settings_service.set("company", "agents", json.dumps(db_agents)) + + app = create_app( + config=config, + persistence=fake_persistence, + message_bus=fake_message_bus, + cost_tracker=CostTracker(), + auth_service=auth_service, + settings_service=settings_service, + ) + with TestClient(app) as client: + client.headers.update(make_auth_headers("ceo")) + resp = client.get("/api/v1/company") + body = resp.json() + assert body["success"] is True + assert len(body["data"]["agents"]) == 1 + assert body["data"]["agents"][0]["name"] == "db-agent" diff --git a/tests/unit/api/controllers/test_departments.py b/tests/unit/api/controllers/test_departments.py index 9f5687b8a9..8c6b5cb0b2 100644 --- a/tests/unit/api/controllers/test_departments.py +++ b/tests/unit/api/controllers/test_departments.py @@ -17,6 +17,7 @@ @pytest.mark.unit +@pytest.mark.timeout(30) class TestDepartmentController: def test_list_departments_empty(self, test_client: TestClient[Any]) -> None: resp = test_client.get("/api/v1/departments") @@ -32,6 +33,7 @@ def test_get_department_not_found(self, test_client: TestClient[Any]) -> None: @pytest.mark.unit +@pytest.mark.timeout(30) class TestDepartmentControllerDbOverride: """Test that DB-stored settings override YAML departments.""" diff --git a/tests/unit/api/controllers/test_providers.py b/tests/unit/api/controllers/test_providers.py index b262084ed9..19e0a87511 100644 --- a/tests/unit/api/controllers/test_providers.py +++ b/tests/unit/api/controllers/test_providers.py @@ -17,6 +17,7 @@ @pytest.mark.unit +@pytest.mark.timeout(30) class TestProviderController: def test_list_providers_empty(self, test_client: TestClient[Any]) -> None: resp = test_client.get("/api/v1/providers") @@ -35,6 +36,7 @@ def test_list_models_not_found(self, test_client: TestClient[Any]) -> None: @pytest.mark.unit +@pytest.mark.timeout(30) class TestProviderApiKeySecurity: def test_provider_api_key_stripped( self, @@ -53,6 +55,7 @@ def test_provider_api_key_stripped( @pytest.mark.unit +@pytest.mark.timeout(30) class TestProviderControllerDbOverride: """Test that DB-stored settings override YAML providers.""" diff --git a/tests/unit/settings/test_resolver.py b/tests/unit/settings/test_resolver.py index e63948f8fc..3b4b8483c5 100644 --- a/tests/unit/settings/test_resolver.py +++ b/tests/unit/settings/test_resolver.py @@ -1,6 +1,5 @@ """Unit tests for ConfigResolver.""" -import json from enum import StrEnum from typing import Literal from unittest.mock import AsyncMock @@ -825,283 +824,3 @@ async def test_invalid_enum_value_propagates( ) with pytest.raises(ValueError, match=r"invalid.*RateLimitTimeUnit"): await resolver.get_api_config() - - -# ── JSON Accessor Tests ────────────────────────────────────────── - - -@pytest.mark.unit -@pytest.mark.timeout(30) -class TestGetJson: - """Tests for get_json() generic accessor.""" - - async def test_valid_json_array( - self, resolver: ConfigResolver, mock_settings: AsyncMock - ) -> None: - mock_settings.get.return_value = _make_value('[{"name": "a"}]') - result = await resolver.get_json("company", "agents") - assert result == [{"name": "a"}] - mock_settings.get.assert_awaited_once_with("company", "agents") - - async def test_valid_json_object( - self, resolver: ConfigResolver, mock_settings: AsyncMock - ) -> None: - mock_settings.get.return_value = _make_value('{"k": "v"}') - result = await resolver.get_json("test", "key") - assert result == {"k": "v"} - - async def test_invalid_json_raises_value_error( - self, resolver: ConfigResolver, mock_settings: AsyncMock - ) -> None: - mock_settings.get.return_value = _make_value("not-json{}") - with pytest.raises(ValueError, match="invalid JSON"): - await resolver.get_json("test", "key") - - async def test_not_found_propagates( - self, resolver: ConfigResolver, mock_settings: AsyncMock - ) -> None: - mock_settings.get.side_effect = SettingNotFoundError("nope") - with pytest.raises(SettingNotFoundError): - await resolver.get_json("bad", "key") - - async def test_empty_array( - self, resolver: ConfigResolver, mock_settings: AsyncMock - ) -> None: - mock_settings.get.return_value = _make_value("[]") - assert await resolver.get_json("test", "key") == [] - - -# ── Composed Read: Agents ──────────────────────────────────────── - - -@pytest.mark.unit -@pytest.mark.timeout(30) -class TestGetAgents: - """Tests for get_agents() composed read.""" - - async def test_json_roundtrip(self, mock_settings: AsyncMock) -> None: - """Agent configs parsed from JSON setting.""" - from synthorg.config.schema import AgentConfig - - agent_data = [ - {"name": "alice", "role": "dev", "department": "eng"}, - {"name": "bob", "role": "qa", "department": "eng"}, - ] - mock_settings.get.return_value = _make_value( - json.dumps(agent_data), - namespace=SettingNamespace.COMPANY, - key="agents", - ) - config = _FakeRootConfig() - resolver = ConfigResolver( - settings_service=mock_settings, - config=config, # type: ignore[arg-type] - ) - result = await resolver.get_agents() - - assert len(result) == 2 - assert isinstance(result[0], AgentConfig) - assert result[0].name == "alice" - assert result[1].name == "bob" - - async def test_empty_list_falls_back_to_config( - self, mock_settings: AsyncMock - ) -> None: - """Empty JSON list → fall back to config.agents.""" - mock_settings.get.return_value = _make_value( - "[]", - namespace=SettingNamespace.COMPANY, - key="agents", - ) - agent = _FakeAgentConfig(name="fallback-agent") - config = _FakeRootConfig(agents=(agent,)) - resolver = ConfigResolver( - settings_service=mock_settings, - config=config, # type: ignore[arg-type] - ) - result = await resolver.get_agents() - - assert len(result) == 1 - assert result[0].name == "fallback-agent" - - async def test_invalid_json_falls_back_to_config( - self, mock_settings: AsyncMock - ) -> None: - """Invalid JSON → fall back to config.agents.""" - mock_settings.get.return_value = _make_value( - "not-json", - namespace=SettingNamespace.COMPANY, - key="agents", - ) - agent = _FakeAgentConfig(name="safe-agent") - config = _FakeRootConfig(agents=(agent,)) - resolver = ConfigResolver( - settings_service=mock_settings, - config=config, # type: ignore[arg-type] - ) - result = await resolver.get_agents() - - assert len(result) == 1 - assert result[0].name == "safe-agent" - - async def test_not_found_propagates( - self, resolver: ConfigResolver, mock_settings: AsyncMock - ) -> None: - mock_settings.get.side_effect = SettingNotFoundError("nope") - with pytest.raises(SettingNotFoundError): - await resolver.get_agents() - - -# ── Composed Read: Departments ─────────────────────────────────── - - -@pytest.mark.unit -@pytest.mark.timeout(30) -class TestGetDepartments: - """Tests for get_departments() composed read.""" - - async def test_json_roundtrip(self, mock_settings: AsyncMock) -> None: - """Departments parsed from JSON setting.""" - from synthorg.core.company import Department - - dept_data = [ - {"name": "engineering", "head": "alice"}, - ] - mock_settings.get.return_value = _make_value( - json.dumps(dept_data), - namespace=SettingNamespace.COMPANY, - key="departments", - ) - config = _FakeRootConfig() - resolver = ConfigResolver( - settings_service=mock_settings, - config=config, # type: ignore[arg-type] - ) - result = await resolver.get_departments() - - assert len(result) == 1 - assert isinstance(result[0], Department) - assert result[0].name == "engineering" - - async def test_empty_list_falls_back_to_config( - self, mock_settings: AsyncMock - ) -> None: - mock_settings.get.return_value = _make_value( - "[]", - namespace=SettingNamespace.COMPANY, - key="departments", - ) - dept = _FakeDepartment(name="fallback-dept") - config = _FakeRootConfig(departments=(dept,)) - resolver = ConfigResolver( - settings_service=mock_settings, - config=config, # type: ignore[arg-type] - ) - result = await resolver.get_departments() - - assert len(result) == 1 - assert result[0].name == "fallback-dept" - - async def test_invalid_json_falls_back_to_config( - self, mock_settings: AsyncMock - ) -> None: - mock_settings.get.return_value = _make_value( - "{bad-json", - namespace=SettingNamespace.COMPANY, - key="departments", - ) - dept = _FakeDepartment(name="safe-dept") - config = _FakeRootConfig(departments=(dept,)) - resolver = ConfigResolver( - settings_service=mock_settings, - config=config, # type: ignore[arg-type] - ) - result = await resolver.get_departments() - - assert len(result) == 1 - assert result[0].name == "safe-dept" - - async def test_not_found_propagates( - self, resolver: ConfigResolver, mock_settings: AsyncMock - ) -> None: - mock_settings.get.side_effect = SettingNotFoundError("nope") - with pytest.raises(SettingNotFoundError): - await resolver.get_departments() - - -# ── Composed Read: Provider Configs ────────────────────────────── - - -@pytest.mark.unit -@pytest.mark.timeout(30) -class TestGetProviderConfigs: - """Tests for get_provider_configs() composed read.""" - - async def test_json_roundtrip(self, mock_settings: AsyncMock) -> None: - """Provider configs parsed from JSON setting.""" - from synthorg.config.schema import ProviderConfig - - prov_data = { - "test-provider": {"driver": "litellm"}, - } - mock_settings.get.return_value = _make_value( - json.dumps(prov_data), - namespace=SettingNamespace.PROVIDERS, - key="configs", - ) - config = _FakeRootConfig() - resolver = ConfigResolver( - settings_service=mock_settings, - config=config, # type: ignore[arg-type] - ) - result = await resolver.get_provider_configs() - - assert "test-provider" in result - assert isinstance(result["test-provider"], ProviderConfig) - assert result["test-provider"].driver == "litellm" - - async def test_empty_dict_falls_back_to_config( - self, mock_settings: AsyncMock - ) -> None: - mock_settings.get.return_value = _make_value( - "{}", - namespace=SettingNamespace.PROVIDERS, - key="configs", - ) - config = _FakeRootConfig( - providers={"fallback": _FakeProviderConfig(driver="test-driver")}, - ) - resolver = ConfigResolver( - settings_service=mock_settings, - config=config, # type: ignore[arg-type] - ) - result = await resolver.get_provider_configs() - - assert "fallback" in result - assert result["fallback"].driver == "test-driver" - - async def test_invalid_json_falls_back_to_config( - self, mock_settings: AsyncMock - ) -> None: - mock_settings.get.return_value = _make_value( - "not-valid-json", - namespace=SettingNamespace.PROVIDERS, - key="configs", - ) - config = _FakeRootConfig( - providers={"safe": _FakeProviderConfig()}, - ) - resolver = ConfigResolver( - settings_service=mock_settings, - config=config, # type: ignore[arg-type] - ) - result = await resolver.get_provider_configs() - - assert "safe" in result - - async def test_not_found_propagates( - self, resolver: ConfigResolver, mock_settings: AsyncMock - ) -> None: - mock_settings.get.side_effect = SettingNotFoundError("nope") - with pytest.raises(SettingNotFoundError): - await resolver.get_provider_configs() diff --git a/tests/unit/settings/test_resolver_structural.py b/tests/unit/settings/test_resolver_structural.py new file mode 100644 index 0000000000..a8f930d531 --- /dev/null +++ b/tests/unit/settings/test_resolver_structural.py @@ -0,0 +1,460 @@ +"""Unit tests for ConfigResolver structural data accessors. + +Tests for ``get_json``, ``get_agents``, ``get_departments``, and +``get_provider_configs`` — extracted from ``test_resolver.py`` to +keep files under the 800-line limit. +""" + +import json +from unittest.mock import AsyncMock + +import pytest +from pydantic import BaseModel, ConfigDict + +from synthorg.settings.enums import SettingNamespace, SettingSource +from synthorg.settings.errors import SettingNotFoundError +from synthorg.settings.models import SettingValue +from synthorg.settings.resolver import ConfigResolver + +# ── Helpers ─────────────────────────────────────────────────────── + + +def _make_value( + value: str, + namespace: SettingNamespace = SettingNamespace.BUDGET, + key: str = "total_monthly", +) -> SettingValue: + return SettingValue( + namespace=namespace, + key=key, + value=value, + source=SettingSource.DEFAULT, + ) + + +class _FakeAgentConfig(BaseModel): + model_config = ConfigDict(frozen=True) + name: str = "agent-1" + role: str = "developer" + department: str = "eng" + + +class _FakeDepartment(BaseModel): + model_config = ConfigDict(frozen=True) + name: str = "eng" + head: str = "lead" + + +class _FakeProviderConfig(BaseModel): + model_config = ConfigDict(frozen=True) + driver: str = "litellm" + + +class _FakeRootConfig(BaseModel): + model_config = ConfigDict(frozen=True) + agents: tuple[_FakeAgentConfig, ...] = () + departments: tuple[_FakeDepartment, ...] = () + providers: dict[str, _FakeProviderConfig] = {} + + +# ── Fixtures ────────────────────────────────────────────────────── + + +@pytest.fixture +def mock_settings() -> AsyncMock: + return AsyncMock() + + +@pytest.fixture +def root_config() -> _FakeRootConfig: + return _FakeRootConfig() + + +@pytest.fixture +def resolver(mock_settings: AsyncMock, root_config: _FakeRootConfig) -> ConfigResolver: + return ConfigResolver( + settings_service=mock_settings, + config=root_config, # type: ignore[arg-type] + ) + + +# ── JSON Accessor Tests ────────────────────────────────────────── + + +@pytest.mark.unit +@pytest.mark.timeout(30) +class TestGetJson: + """Tests for get_json() generic accessor.""" + + async def test_valid_json_array( + self, resolver: ConfigResolver, mock_settings: AsyncMock + ) -> None: + mock_settings.get.return_value = _make_value('[{"name": "a"}]') + result = await resolver.get_json("company", "agents") + assert result == [{"name": "a"}] + mock_settings.get.assert_awaited_once_with("company", "agents") + + async def test_valid_json_object( + self, resolver: ConfigResolver, mock_settings: AsyncMock + ) -> None: + mock_settings.get.return_value = _make_value('{"k": "v"}') + result = await resolver.get_json("test", "key") + assert result == {"k": "v"} + + async def test_invalid_json_raises_value_error( + self, resolver: ConfigResolver, mock_settings: AsyncMock + ) -> None: + mock_settings.get.return_value = _make_value("not-json{}") + with pytest.raises(ValueError, match="invalid JSON"): + await resolver.get_json("test", "key") + + async def test_not_found_propagates( + self, resolver: ConfigResolver, mock_settings: AsyncMock + ) -> None: + mock_settings.get.side_effect = SettingNotFoundError("nope") + with pytest.raises(SettingNotFoundError): + await resolver.get_json("bad", "key") + + async def test_empty_array( + self, resolver: ConfigResolver, mock_settings: AsyncMock + ) -> None: + mock_settings.get.return_value = _make_value("[]") + assert await resolver.get_json("test", "key") == [] + + +# ── Composed Read: Agents ──────────────────────────────────────── + + +@pytest.mark.unit +@pytest.mark.timeout(30) +class TestGetAgents: + """Tests for get_agents() composed read.""" + + async def test_json_roundtrip(self, mock_settings: AsyncMock) -> None: + """Agent configs parsed from JSON setting.""" + from synthorg.config.schema import AgentConfig + + agent_data = [ + {"name": "alice", "role": "dev", "department": "eng"}, + {"name": "bob", "role": "qa", "department": "eng"}, + ] + mock_settings.get.return_value = _make_value( + json.dumps(agent_data), + namespace=SettingNamespace.COMPANY, + key="agents", + ) + config = _FakeRootConfig() + resolver = ConfigResolver( + settings_service=mock_settings, + config=config, # type: ignore[arg-type] + ) + result = await resolver.get_agents() + + assert len(result) == 2 + assert isinstance(result[0], AgentConfig) + assert result[0].name == "alice" + assert result[1].name == "bob" + + async def test_empty_list_falls_back_to_config( + self, mock_settings: AsyncMock + ) -> None: + """Empty JSON list -> fall back to config.agents.""" + mock_settings.get.return_value = _make_value( + "[]", + namespace=SettingNamespace.COMPANY, + key="agents", + ) + agent = _FakeAgentConfig(name="fallback-agent") + config = _FakeRootConfig(agents=(agent,)) + resolver = ConfigResolver( + settings_service=mock_settings, + config=config, # type: ignore[arg-type] + ) + result = await resolver.get_agents() + + assert len(result) == 1 + assert result[0].name == "fallback-agent" + + async def test_invalid_json_falls_back_to_config( + self, mock_settings: AsyncMock + ) -> None: + """Invalid JSON -> fall back to config.agents.""" + mock_settings.get.return_value = _make_value( + "not-json", + namespace=SettingNamespace.COMPANY, + key="agents", + ) + agent = _FakeAgentConfig(name="safe-agent") + config = _FakeRootConfig(agents=(agent,)) + resolver = ConfigResolver( + settings_service=mock_settings, + config=config, # type: ignore[arg-type] + ) + result = await resolver.get_agents() + + assert len(result) == 1 + assert result[0].name == "safe-agent" + + async def test_not_found_propagates( + self, resolver: ConfigResolver, mock_settings: AsyncMock + ) -> None: + mock_settings.get.side_effect = SettingNotFoundError("nope") + with pytest.raises(SettingNotFoundError): + await resolver.get_agents() + + async def test_invalid_schema_falls_back_to_config( + self, mock_settings: AsyncMock + ) -> None: + """Valid JSON but invalid AgentConfig schema -> fall back.""" + mock_settings.get.return_value = _make_value( + '[{"not_a_valid_field": "value"}]', + namespace=SettingNamespace.COMPANY, + key="agents", + ) + agent = _FakeAgentConfig(name="schema-fallback") + config = _FakeRootConfig(agents=(agent,)) + resolver = ConfigResolver( + settings_service=mock_settings, + config=config, # type: ignore[arg-type] + ) + result = await resolver.get_agents() + + assert len(result) == 1 + assert result[0].name == "schema-fallback" + + async def test_wrong_json_shape_falls_back_to_config( + self, mock_settings: AsyncMock + ) -> None: + """JSON dict instead of list -> fall back.""" + mock_settings.get.return_value = _make_value( + '{"name": "alice"}', + namespace=SettingNamespace.COMPANY, + key="agents", + ) + agent = _FakeAgentConfig(name="shape-fallback") + config = _FakeRootConfig(agents=(agent,)) + resolver = ConfigResolver( + settings_service=mock_settings, + config=config, # type: ignore[arg-type] + ) + result = await resolver.get_agents() + + assert len(result) == 1 + assert result[0].name == "shape-fallback" + + +# ── Composed Read: Departments ─────────────────────────────────── + + +@pytest.mark.unit +@pytest.mark.timeout(30) +class TestGetDepartments: + """Tests for get_departments() composed read.""" + + async def test_json_roundtrip(self, mock_settings: AsyncMock) -> None: + """Departments parsed from JSON setting.""" + from synthorg.core.company import Department + + dept_data = [ + {"name": "engineering", "head": "alice"}, + ] + mock_settings.get.return_value = _make_value( + json.dumps(dept_data), + namespace=SettingNamespace.COMPANY, + key="departments", + ) + config = _FakeRootConfig() + resolver = ConfigResolver( + settings_service=mock_settings, + config=config, # type: ignore[arg-type] + ) + result = await resolver.get_departments() + + assert len(result) == 1 + assert isinstance(result[0], Department) + assert result[0].name == "engineering" + + async def test_empty_list_falls_back_to_config( + self, mock_settings: AsyncMock + ) -> None: + mock_settings.get.return_value = _make_value( + "[]", + namespace=SettingNamespace.COMPANY, + key="departments", + ) + dept = _FakeDepartment(name="fallback-dept") + config = _FakeRootConfig(departments=(dept,)) + resolver = ConfigResolver( + settings_service=mock_settings, + config=config, # type: ignore[arg-type] + ) + result = await resolver.get_departments() + + assert len(result) == 1 + assert result[0].name == "fallback-dept" + + async def test_invalid_json_falls_back_to_config( + self, mock_settings: AsyncMock + ) -> None: + mock_settings.get.return_value = _make_value( + "{bad-json", + namespace=SettingNamespace.COMPANY, + key="departments", + ) + dept = _FakeDepartment(name="safe-dept") + config = _FakeRootConfig(departments=(dept,)) + resolver = ConfigResolver( + settings_service=mock_settings, + config=config, # type: ignore[arg-type] + ) + result = await resolver.get_departments() + + assert len(result) == 1 + assert result[0].name == "safe-dept" + + async def test_not_found_propagates( + self, resolver: ConfigResolver, mock_settings: AsyncMock + ) -> None: + mock_settings.get.side_effect = SettingNotFoundError("nope") + with pytest.raises(SettingNotFoundError): + await resolver.get_departments() + + async def test_invalid_schema_falls_back_to_config( + self, mock_settings: AsyncMock + ) -> None: + """Valid JSON but invalid Department schema -> fall back.""" + mock_settings.get.return_value = _make_value( + '[{"bad_field": "value"}]', + namespace=SettingNamespace.COMPANY, + key="departments", + ) + dept = _FakeDepartment(name="schema-dept") + config = _FakeRootConfig(departments=(dept,)) + resolver = ConfigResolver( + settings_service=mock_settings, + config=config, # type: ignore[arg-type] + ) + result = await resolver.get_departments() + + assert len(result) == 1 + assert result[0].name == "schema-dept" + + +# ── Composed Read: Provider Configs ────────────────────────────── + + +@pytest.mark.unit +@pytest.mark.timeout(30) +class TestGetProviderConfigs: + """Tests for get_provider_configs() composed read.""" + + async def test_json_roundtrip(self, mock_settings: AsyncMock) -> None: + """Provider configs parsed from JSON setting.""" + from synthorg.config.schema import ProviderConfig + + prov_data = { + "test-provider": {"driver": "litellm"}, + } + mock_settings.get.return_value = _make_value( + json.dumps(prov_data), + namespace=SettingNamespace.PROVIDERS, + key="configs", + ) + config = _FakeRootConfig() + resolver = ConfigResolver( + settings_service=mock_settings, + config=config, # type: ignore[arg-type] + ) + result = await resolver.get_provider_configs() + + assert "test-provider" in result + assert isinstance(result["test-provider"], ProviderConfig) + assert result["test-provider"].driver == "litellm" + + async def test_empty_dict_falls_back_to_config( + self, mock_settings: AsyncMock + ) -> None: + mock_settings.get.return_value = _make_value( + "{}", + namespace=SettingNamespace.PROVIDERS, + key="configs", + ) + config = _FakeRootConfig( + providers={ + "fallback": _FakeProviderConfig(driver="test-driver"), + }, + ) + resolver = ConfigResolver( + settings_service=mock_settings, + config=config, # type: ignore[arg-type] + ) + result = await resolver.get_provider_configs() + + assert "fallback" in result + assert result["fallback"].driver == "test-driver" + + async def test_invalid_json_falls_back_to_config( + self, mock_settings: AsyncMock + ) -> None: + mock_settings.get.return_value = _make_value( + "not-valid-json", + namespace=SettingNamespace.PROVIDERS, + key="configs", + ) + config = _FakeRootConfig( + providers={"safe": _FakeProviderConfig()}, + ) + resolver = ConfigResolver( + settings_service=mock_settings, + config=config, # type: ignore[arg-type] + ) + result = await resolver.get_provider_configs() + + assert "safe" in result + + async def test_not_found_propagates( + self, resolver: ConfigResolver, mock_settings: AsyncMock + ) -> None: + mock_settings.get.side_effect = SettingNotFoundError("nope") + with pytest.raises(SettingNotFoundError): + await resolver.get_provider_configs() + + async def test_invalid_schema_falls_back_to_config( + self, mock_settings: AsyncMock + ) -> None: + """Valid JSON but invalid ProviderConfig schema -> fall back.""" + mock_settings.get.return_value = _make_value( + '{"bad": {"driver": ""}}', + namespace=SettingNamespace.PROVIDERS, + key="configs", + ) + config = _FakeRootConfig( + providers={"safe": _FakeProviderConfig()}, + ) + resolver = ConfigResolver( + settings_service=mock_settings, + config=config, # type: ignore[arg-type] + ) + result = await resolver.get_provider_configs() + + assert "safe" in result + + async def test_wrong_json_shape_falls_back_to_config( + self, mock_settings: AsyncMock + ) -> None: + """JSON list instead of dict -> fall back.""" + mock_settings.get.return_value = _make_value( + '[{"driver": "litellm"}]', + namespace=SettingNamespace.PROVIDERS, + key="configs", + ) + config = _FakeRootConfig( + providers={"shape-safe": _FakeProviderConfig()}, + ) + resolver = ConfigResolver( + settings_service=mock_settings, + config=config, # type: ignore[arg-type] + ) + result = await resolver.get_provider_configs() + + assert "shape-safe" in result From 97ec9bfcde4a3f4d6a0231d9e69cb3d1d4e5376d Mon Sep 17 00:00:00 2001 From: Aurelio <19254254+Aureliolo@users.noreply.github.com> Date: Tue, 17 Mar 2026 21:35:07 +0100 Subject: [PATCH 3/7] fix(settings): address 20 PR review findings from local agents and external reviewers Security: mark providers/configs as sensitive (Fernet encryption at rest), catch SettingsEncryptionError in get_json, preserve JSONDecodeError chain. Correctness: change `if not raw:` to `if raw is None:` so empty [] and {} are valid overrides (not silent fallbacks), add ExceptionGroup unwrapping in company controller, parallelize analytics controller with TaskGroup. Code quality: simplify _serialize_value (remove redundant any() checks), fix bool serialization to lowercase JSON "true"/"false", extract shared test helpers to conftest.py (test_resolver.py back under 800 lines). Tests: add missing departments wrong-shape fallback test, mixed list/dict tests for _serialize_value, company TaskGroup error propagation test. Docs: update CLAUDE.md and operations.md to document JSON setting type and structural data accessors. --- CLAUDE.md | 2 +- docs/design/operations.md | 2 +- src/synthorg/api/controllers/agents.py | 2 +- src/synthorg/api/controllers/analytics.py | 14 ++- src/synthorg/api/controllers/company.py | 11 +- src/synthorg/settings/config_bridge.py | 43 +++---- .../settings/definitions/providers.py | 1 + src/synthorg/settings/resolver.py | 54 +++++--- tests/unit/api/controllers/test_analytics.py | 3 + tests/unit/api/controllers/test_company.py | 41 ++++++ tests/unit/api/controllers/test_providers.py | 11 +- tests/unit/settings/conftest.py | 41 ++++++ tests/unit/settings/test_config_bridge.py | 25 +++- tests/unit/settings/test_resolver.py | 52 ++------ .../unit/settings/test_resolver_structural.py | 117 ++++++++---------- 15 files changed, 260 insertions(+), 159 deletions(-) create mode 100644 tests/unit/settings/conftest.py diff --git a/CLAUDE.md b/CLAUDE.md index 866134f33b..e12ca6cd94 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -128,7 +128,7 @@ src/synthorg/ persistence/ # Operational data persistence — pluggable PersistenceBackend protocol, SQLite initial, SettingsRepository (namespaced settings CRUD) (see Memory & Persistence design page) observability/ # Structured logging, correlation tracking, log sinks providers/ # LLM provider abstraction (LiteLLM adapter) - settings/ # Runtime-editable settings persistence (DB > env > YAML > code defaults), typed definitions (9 namespaces), Fernet encryption for sensitive values, config bridge, ConfigResolver (typed composed reads for controllers), validation, registry, change notifications via message bus, SettingsSubscriber protocol (subscriber.py), SettingsChangeDispatcher (dispatcher.py, polls #settings channel, routes to subscribers, restart_required filtering) + settings/ # Runtime-editable settings persistence (DB > env > YAML > code defaults), typed definitions (9 namespaces, including JSON type for structural data), Fernet encryption for sensitive values, config bridge (JSON serialization for Pydantic models/collections), ConfigResolver (typed scalar + structural data accessors for controllers — get_agents, get_departments, get_provider_configs with validation fallbacks to YAML), validation, registry, change notifications via message bus, SettingsSubscriber protocol (subscriber.py), SettingsChangeDispatcher (dispatcher.py, polls #settings channel, routes to subscribers, restart_required filtering) definitions/ # Per-namespace setting definitions (api, company, providers, memory, budget, security, coordination, observability, backup) subscribers/ # Concrete settings subscribers (ProviderSettingsSubscriber — rebuilds ModelRouter on strategy change, MemorySettingsSubscriber — advisory logging for memory config) security/ # SecOps agent, rule engine (soft-allow/hard-deny, fail-closed), audit log, output scanner, output scan response policies (redact/withhold/log-only/autonomy-tiered), risk classifier, risk tier classifier, action type registry, ToolInvoker security integration, progressive trust (4 strategies: disabled/weighted/per-category/milestone), autonomy levels (presets, resolver, change strategy), timeout policies (park/resume) diff --git a/docs/design/operations.md b/docs/design/operations.md index f5afe8435b..789ca5bf6b 100644 --- a/docs/design/operations.md +++ b/docs/design/operations.md @@ -1042,7 +1042,7 @@ and retry guidance. - **Budget Panel**: Spending charts, per-agent breakdown (projections/alerts planned) - **Meeting Logs**: Placeholder — coming soon - **Artifact Browser**: Placeholder — coming soon -- **Settings**: Runtime-editable configuration via DB-backed settings persistence (9 namespaces: api, company, providers, memory, budget, security, coordination, observability, backup). 4-layer resolution (DB > env > YAML > code defaults), Fernet encryption for sensitive values, REST API (GET/PUT/DELETE + schema endpoints for dynamic UI generation), change notifications via message bus. `ConfigResolver` provides typed composed reads for API controllers (assembles full Pydantic config models from individually resolved settings, using `asyncio.TaskGroup` for parallel resolution). **Hot-reload**: `SettingsChangeDispatcher` polls the `#settings` bus channel and routes change notifications to registered `SettingsSubscriber` implementations. Settings marked `restart_required=True` are filtered (logged as WARNING, not dispatched). Concrete subscribers: `ProviderSettingsSubscriber` (rebuilds `ModelRouter` on `routing_strategy` change via `AppState.swap_model_router`), `MemorySettingsSubscriber` (advisory logging for non-restart memory settings) +- **Settings**: Runtime-editable configuration via DB-backed settings persistence (9 namespaces: api, company, providers, memory, budget, security, coordination, observability, backup). Setting types: STRING, INTEGER, FLOAT, BOOLEAN, ENUM, JSON. 4-layer resolution (DB > env > YAML > code defaults), Fernet encryption for sensitive values, REST API (GET/PUT/DELETE + schema endpoints for dynamic UI generation), change notifications via message bus. `ConfigResolver` provides typed scalar accessors and structural data accessors for API controllers: scalar reads assemble full Pydantic config models from individually resolved settings (using `asyncio.TaskGroup` for parallel resolution); structural reads (`get_agents`, `get_departments`, `get_provider_configs`) resolve JSON-typed settings with Pydantic schema validation and graceful fallback to `RootConfig` defaults on invalid data. **Hot-reload**: `SettingsChangeDispatcher` polls the `#settings` bus channel and routes change notifications to registered `SettingsSubscriber` implementations. Settings marked `restart_required=True` are filtered (logged as WARNING, not dispatched). Concrete subscribers: `ProviderSettingsSubscriber` (rebuilds `ModelRouter` on `routing_strategy` change via `AppState.swap_model_router`), `MemorySettingsSubscriber` (advisory logging for non-restart memory settings). ### Human Roles diff --git a/src/synthorg/api/controllers/agents.py b/src/synthorg/api/controllers/agents.py index c100d83c73..160c9ca0c3 100644 --- a/src/synthorg/api/controllers/agents.py +++ b/src/synthorg/api/controllers/agents.py @@ -16,7 +16,7 @@ class AgentController(Controller): - """Read-only access to agent configurations from ``RootConfig``.""" + """Read-only access to agent configurations resolved through settings.""" path = "/agents" tags = ("agents",) diff --git a/src/synthorg/api/controllers/analytics.py b/src/synthorg/api/controllers/analytics.py index 5bd5363968..52ca8fbe5b 100644 --- a/src/synthorg/api/controllers/analytics.py +++ b/src/synthorg/api/controllers/analytics.py @@ -1,5 +1,6 @@ """Analytics controller — derived read-only metrics.""" +import asyncio from collections import Counter from litestar import Controller, get @@ -56,17 +57,20 @@ async def get_overview( """ app_state: AppState = state.app_state - all_tasks = await app_state.persistence.tasks.list_tasks() + async with asyncio.TaskGroup() as tg: + t_tasks = tg.create_task(app_state.persistence.tasks.list_tasks()) + t_cost = tg.create_task(app_state.cost_tracker.get_total_cost()) + t_agents = tg.create_task(app_state.config_resolver.get_agents()) + + all_tasks = t_tasks.result() counts = Counter(t.status.value for t in all_tasks) by_status = {s.value: counts.get(s.value, 0) for s in TaskStatus} - total_cost = await app_state.cost_tracker.get_total_cost() - return ApiResponse( data=OverviewMetrics( total_tasks=len(all_tasks), tasks_by_status=by_status, - total_agents=len(await app_state.config_resolver.get_agents()), - total_cost_usd=total_cost, + total_agents=len(t_agents.result()), + total_cost_usd=t_cost.result(), ), ) diff --git a/src/synthorg/api/controllers/company.py b/src/synthorg/api/controllers/company.py index 443e5bc67a..a29b87fca3 100644 --- a/src/synthorg/api/controllers/company.py +++ b/src/synthorg/api/controllers/company.py @@ -40,10 +40,13 @@ async def get_company( """ app_state: AppState = state.app_state resolver = app_state.config_resolver - async with asyncio.TaskGroup() as tg: - t_name = tg.create_task(resolver.get_str("company", "company_name")) - t_agents = tg.create_task(resolver.get_agents()) - t_depts = tg.create_task(resolver.get_departments()) + try: + async with asyncio.TaskGroup() as tg: + t_name = tg.create_task(resolver.get_str("company", "company_name")) + t_agents = tg.create_task(resolver.get_agents()) + t_depts = tg.create_task(resolver.get_departments()) + except ExceptionGroup as eg: + raise eg.exceptions[0] from eg data: dict[str, Any] = { "company_name": t_name.result(), "agents": [a.model_dump(mode="json") for a in t_agents.result()], diff --git a/src/synthorg/settings/config_bridge.py b/src/synthorg/settings/config_bridge.py index f1806c7eef..f6b631058a 100644 --- a/src/synthorg/settings/config_bridge.py +++ b/src/synthorg/settings/config_bridge.py @@ -17,40 +17,41 @@ def _serialize_value(value: object) -> str: """Serialize a resolved config value to a string. - Handles Pydantic models, collections of models, dicts with - model values, and plain collections by producing valid JSON. - Scalars fall back to ``str()``. + Handles Pydantic models, tuples/lists, and dicts by producing + valid JSON. Scalar booleans produce lowercase JSON-style + ``"true"``/``"false"``. Other scalars fall back to ``str()``. Args: value: The resolved config attribute. Returns: A string representation suitable for the settings layer. + + Raises: + TypeError: If *value* contains non-JSON-serializable types + (e.g. ``set``, ``bytes``, ``datetime``). """ if isinstance(value, BaseModel): return json.dumps(value.model_dump(mode="json")) if isinstance(value, (tuple, list)): - if any(isinstance(item, BaseModel) for item in value): - return json.dumps( - [ - item.model_dump(mode="json") - if isinstance(item, BaseModel) - else item - for item in value - ] - ) - return json.dumps(list(value)) + return json.dumps( + [ + item.model_dump(mode="json") if isinstance(item, BaseModel) else item + for item in value + ] + ) if isinstance(value, dict): - if any(isinstance(v, BaseModel) for v in value.values()): - return json.dumps( - { - k: v.model_dump(mode="json") if isinstance(v, BaseModel) else v - for k, v in value.items() - } - ) - return json.dumps(value) + return json.dumps( + { + k: v.model_dump(mode="json") if isinstance(v, BaseModel) else v + for k, v in value.items() + } + ) + + if isinstance(value, bool): + return "true" if value else "false" return str(value) diff --git a/src/synthorg/settings/definitions/providers.py b/src/synthorg/settings/definitions/providers.py index 95d3c49823..10eb66cec0 100644 --- a/src/synthorg/settings/definitions/providers.py +++ b/src/synthorg/settings/definitions/providers.py @@ -54,5 +54,6 @@ description="LLM provider configurations (JSON object keyed by name)", group="General", yaml_path="providers", + sensitive=True, ) ) diff --git a/src/synthorg/settings/resolver.py b/src/synthorg/settings/resolver.py index 5cf2720e61..1a8dba07c5 100644 --- a/src/synthorg/settings/resolver.py +++ b/src/synthorg/settings/resolver.py @@ -18,7 +18,7 @@ SETTINGS_NOT_FOUND, SETTINGS_VALIDATION_FAILED, ) -from synthorg.settings.errors import SettingNotFoundError +from synthorg.settings.errors import SettingNotFoundError, SettingsEncryptionError if TYPE_CHECKING: from synthorg.api.config import ApiConfig @@ -272,9 +272,11 @@ async def get_json(self, namespace: str, key: str) -> Any: Returns: The parsed JSON value (list, dict, scalar, etc.). + Note that JSON ``null`` parses to Python ``None``. Raises: SettingNotFoundError: If the key is not in the registry. + SettingsEncryptionError: If the value cannot be decrypted. ValueError: If the value is not valid JSON. """ try: @@ -286,9 +288,18 @@ async def get_json(self, namespace: str, key: str) -> Any: key=key, ) raise + except SettingsEncryptionError: + logger.warning( + SETTINGS_FETCH_FAILED, + namespace=namespace, + key=key, + reason="decryption_failed", + exc_info=True, + ) + raise try: return json.loads(result.value) - except json.JSONDecodeError: + except json.JSONDecodeError as exc: logger.warning( SETTINGS_VALIDATION_FAILED, namespace=namespace, @@ -297,13 +308,15 @@ async def get_json(self, namespace: str, key: str) -> Any: exc_info=True, ) msg = f"Setting {namespace}/{key} has an invalid JSON value" - raise ValueError(msg) from None + raise ValueError(msg) from exc async def get_agents(self) -> tuple[AgentConfig, ...]: """Resolve agent configurations from settings. - Falls back to ``RootConfig.agents`` if the setting is - empty, contains invalid JSON, or fails schema validation. + Falls back to ``RootConfig.agents`` if the setting value is + ``None``, contains invalid JSON, or fails schema validation. + An explicit empty list ``[]`` is a valid override that returns + an empty tuple. Returns: A tuple of ``AgentConfig`` objects. @@ -311,6 +324,7 @@ async def get_agents(self) -> tuple[AgentConfig, ...]: Raises: SettingNotFoundError: If the agents key is not in the registry. + SettingsEncryptionError: If decryption fails. """ from pydantic import ValidationError # noqa: PLC0415 @@ -324,9 +338,10 @@ async def get_agents(self) -> tuple[AgentConfig, ...]: namespace="company", key="agents", reason="invalid_json_fallback", + exc_info=True, ) return self._config.agents - if not raw: + if raw is None: return self._config.agents if not isinstance(raw, list): logger.warning( @@ -334,6 +349,7 @@ async def get_agents(self) -> tuple[AgentConfig, ...]: namespace="company", key="agents", reason="expected_list_fallback", + value_type=type(raw).__name__, ) return self._config.agents try: @@ -351,8 +367,10 @@ async def get_agents(self) -> tuple[AgentConfig, ...]: async def get_departments(self) -> tuple[Department, ...]: """Resolve department configurations from settings. - Falls back to ``RootConfig.departments`` if the setting - is empty, contains invalid JSON, or fails schema validation. + Falls back to ``RootConfig.departments`` if the setting value + is ``None``, contains invalid JSON, or fails schema validation. + An explicit empty list ``[]`` is a valid override that returns + an empty tuple. Returns: A tuple of ``Department`` objects. @@ -360,6 +378,7 @@ async def get_departments(self) -> tuple[Department, ...]: Raises: SettingNotFoundError: If the departments key is not in the registry. + SettingsEncryptionError: If decryption fails. """ from pydantic import ValidationError # noqa: PLC0415 @@ -373,9 +392,10 @@ async def get_departments(self) -> tuple[Department, ...]: namespace="company", key="departments", reason="invalid_json_fallback", + exc_info=True, ) return self._config.departments - if not raw: + if raw is None: return self._config.departments if not isinstance(raw, list): logger.warning( @@ -383,6 +403,7 @@ async def get_departments(self) -> tuple[Department, ...]: namespace="company", key="departments", reason="expected_list_fallback", + value_type=type(raw).__name__, ) return self._config.departments try: @@ -400,15 +421,18 @@ async def get_departments(self) -> tuple[Department, ...]: async def get_provider_configs(self) -> dict[str, ProviderConfig]: """Resolve provider configurations from settings. - Falls back to ``RootConfig.providers`` if the setting - is empty, contains invalid JSON, or fails schema validation. + Falls back to ``RootConfig.providers`` if the setting value + is ``None``, contains invalid JSON, or fails schema validation. + An explicit empty dict ``{}`` is a valid override that returns + an empty dict. Returns: A dict of provider name to ``ProviderConfig``. Raises: - SettingNotFoundError: If the configs key is not + SettingNotFoundError: If the ``configs`` key is not in the registry. + SettingsEncryptionError: If decryption fails. """ from pydantic import ValidationError # noqa: PLC0415 @@ -422,9 +446,10 @@ async def get_provider_configs(self) -> dict[str, ProviderConfig]: namespace="providers", key="configs", reason="invalid_json_fallback", + exc_info=True, ) return dict(self._config.providers) - if not raw: + if raw is None: return dict(self._config.providers) if not isinstance(raw, dict): logger.warning( @@ -432,6 +457,7 @@ async def get_provider_configs(self) -> dict[str, ProviderConfig]: namespace="providers", key="configs", reason="expected_dict_fallback", + value_type=type(raw).__name__, ) return dict(self._config.providers) try: @@ -679,7 +705,7 @@ def _build_budget_alerts(warn: int, crit: int, stop: int) -> BudgetAlertConfig: reason="threshold_ordering", exc_info=True, ) - msg = f"Budget alert thresholds violate ordering constraint: {exc}" + msg = "Budget alert thresholds must satisfy warn < critical < hard_stop" raise ValueError(msg) from exc diff --git a/tests/unit/api/controllers/test_analytics.py b/tests/unit/api/controllers/test_analytics.py index 74666491b6..16ab10cace 100644 --- a/tests/unit/api/controllers/test_analytics.py +++ b/tests/unit/api/controllers/test_analytics.py @@ -85,4 +85,7 @@ async def test_db_agents_count_in_overview( client.headers.update(make_auth_headers("ceo")) resp = client.get("/api/v1/analytics/overview") body = resp.json() + assert body["success"] is True assert body["data"]["total_agents"] == 3 + assert body["data"]["total_tasks"] == 0 + assert body["data"]["total_cost_usd"] == 0.0 diff --git a/tests/unit/api/controllers/test_company.py b/tests/unit/api/controllers/test_company.py index 127a8826c5..1fdbf10da0 100644 --- a/tests/unit/api/controllers/test_company.py +++ b/tests/unit/api/controllers/test_company.py @@ -2,11 +2,13 @@ import json from typing import Any +from unittest.mock import AsyncMock import pytest from litestar.testing import TestClient from synthorg.config.schema import RootConfig +from synthorg.settings.errors import SettingNotFoundError from synthorg.settings.registry import get_registry from synthorg.settings.service import SettingsService from tests.unit.api.conftest import ( @@ -86,6 +88,45 @@ async def test_db_company_departments_override( assert len(body["data"]) == 1 assert body["data"][0]["name"] == "db-sales" + async def test_taskgroup_error_returns_clean_error_response( + self, + fake_persistence: FakePersistenceBackend, + fake_message_bus: FakeMessageBus, + ) -> None: + """Verify TaskGroup exception unwraps to a clean API error.""" + from synthorg.api.app import create_app + from synthorg.api.auth.service import AuthService + from synthorg.budget.tracker import CostTracker + from tests.unit.api.conftest import _make_test_auth_service, _seed_test_users + + config = RootConfig(company_name="test") + auth_service: AuthService = _make_test_auth_service() + _seed_test_users(fake_persistence, auth_service) + settings_service = SettingsService( + repository=fake_persistence.settings, + registry=get_registry(), + config=config, + ) + + app = create_app( + config=config, + persistence=fake_persistence, + message_bus=fake_message_bus, + cost_tracker=CostTracker(), + auth_service=auth_service, + settings_service=settings_service, + ) + with TestClient(app) as client: + client.headers.update(make_auth_headers("ceo")) + resolver = app.state.app_state.config_resolver + resolver.get_str = AsyncMock( + side_effect=SettingNotFoundError("company/company_name"), + ) + resp = client.get("/api/v1/company") + assert resp.status_code == 500 + body = resp.json() + assert body.get("success") is False or "detail" in body + async def test_db_company_overview_includes_db_agents( self, fake_persistence: FakePersistenceBackend, diff --git a/tests/unit/api/controllers/test_providers.py b/tests/unit/api/controllers/test_providers.py index 19e0a87511..f962d0d339 100644 --- a/tests/unit/api/controllers/test_providers.py +++ b/tests/unit/api/controllers/test_providers.py @@ -38,10 +38,7 @@ def test_list_models_not_found(self, test_client: TestClient[Any]) -> None: @pytest.mark.unit @pytest.mark.timeout(30) class TestProviderApiKeySecurity: - def test_provider_api_key_stripped( - self, - root_config: Any, - ) -> None: + def test_provider_api_key_stripped(self) -> None: """Verify api_key is stripped from provider responses.""" from synthorg.api.controllers.providers import _safe_provider from synthorg.config.schema import ProviderConfig @@ -72,10 +69,16 @@ async def test_db_providers_override_config( config = RootConfig(company_name="test") auth_service: AuthService = _make_test_auth_service() _seed_test_users(fake_persistence, auth_service) + from cryptography.fernet import Fernet + + from synthorg.settings.encryption import SettingsEncryptor + + encryptor = SettingsEncryptor(Fernet.generate_key()) settings_service = SettingsService( repository=fake_persistence.settings, registry=get_registry(), config=config, + encryptor=encryptor, ) db_providers = { diff --git a/tests/unit/settings/conftest.py b/tests/unit/settings/conftest.py new file mode 100644 index 0000000000..13fe521892 --- /dev/null +++ b/tests/unit/settings/conftest.py @@ -0,0 +1,41 @@ +"""Shared fixtures and helpers for settings unit tests.""" + +from pydantic import BaseModel, ConfigDict + +from synthorg.settings.enums import SettingNamespace, SettingSource +from synthorg.settings.models import SettingValue + + +def make_setting_value( + value: str, + namespace: SettingNamespace = SettingNamespace.BUDGET, + key: str = "total_monthly", +) -> SettingValue: + """Build a ``SettingValue`` for testing.""" + return SettingValue( + namespace=namespace, + key=key, + value=value, + source=SettingSource.DEFAULT, + ) + + +# ── Fake structural models ────────────────────────────────────── + + +class FakeAgentConfig(BaseModel): + model_config = ConfigDict(frozen=True) + name: str = "agent-1" + role: str = "developer" + department: str = "eng" + + +class FakeDepartment(BaseModel): + model_config = ConfigDict(frozen=True) + name: str = "eng" + head: str = "lead" + + +class FakeProviderConfig(BaseModel): + model_config = ConfigDict(frozen=True) + driver: str = "litellm" diff --git a/tests/unit/settings/test_config_bridge.py b/tests/unit/settings/test_config_bridge.py index 4aa67d7217..bd34c0dbe2 100644 --- a/tests/unit/settings/test_config_bridge.py +++ b/tests/unit/settings/test_config_bridge.py @@ -45,7 +45,7 @@ def test_nested_field(self) -> None: def test_nested_bool(self) -> None: config = _FakeConfig() - assert extract_from_config(config, "budget.enabled") == "True" + assert extract_from_config(config, "budget.enabled") == "true" def test_missing_top_level(self) -> None: config = _FakeConfig() @@ -120,8 +120,27 @@ def test_scalar_int_unchanged(self) -> None: def test_scalar_float_unchanged(self) -> None: assert _serialize_value(3.14) == "3.14" - def test_scalar_bool_unchanged(self) -> None: - assert _serialize_value(True) == "True" + def test_scalar_bool_lowercase_json(self) -> None: + assert _serialize_value(True) == "true" + assert _serialize_value(False) == "false" + + def test_mixed_list_models_and_scalars(self) -> None: + items = [_ItemModel(name="x", value=1), "plain", 42] + result = _serialize_value(items) + parsed = json.loads(result) + assert parsed == [{"name": "x", "value": 1}, "plain", 42] + + def test_mixed_dict_models_and_scalars(self) -> None: + providers: dict[str, object] = { + "a": _InnerConfig(daily_limit=5.0), + "b": "just-a-string", + } + result = _serialize_value(providers) + parsed = json.loads(result) + assert parsed == { + "a": {"daily_limit": 5.0, "enabled": True}, + "b": "just-a-string", + } @pytest.mark.unit diff --git a/tests/unit/settings/test_resolver.py b/tests/unit/settings/test_resolver.py index 3b4b8483c5..018c9b654f 100644 --- a/tests/unit/settings/test_resolver.py +++ b/tests/unit/settings/test_resolver.py @@ -12,10 +12,18 @@ from synthorg.api.config import RateLimitTimeUnit from synthorg.core.enums import AutonomyLevel from synthorg.core.types import NotBlankStr -from synthorg.settings.enums import SettingNamespace, SettingSource +from synthorg.settings.enums import SettingNamespace from synthorg.settings.errors import SettingNotFoundError from synthorg.settings.models import SettingValue from synthorg.settings.resolver import ConfigResolver, _parse_bool +from tests.unit.settings.conftest import ( + FakeAgentConfig, + FakeDepartment, + FakeProviderConfig, +) +from tests.unit.settings.conftest import ( + make_setting_value as _make_value, +) # ── Helpers ─────────────────────────────────────────────────────── @@ -26,19 +34,6 @@ class _Color(StrEnum): BLUE = "blue" -def _make_value( - value: str, - namespace: SettingNamespace = SettingNamespace.BUDGET, - key: str = "total_monthly", -) -> SettingValue: - return SettingValue( - namespace=namespace, - key=key, - value=value, - source=SettingSource.DEFAULT, - ) - - class _BudgetAlerts(BaseModel): model_config = ConfigDict(frozen=True) warn_at: int = 75 @@ -109,37 +104,14 @@ class _FakeApiConfig(BaseModel): api_prefix: NotBlankStr = "/api/v1" -class _CompanyConfig(BaseModel): - model_config = ConfigDict(frozen=True) - - -class _FakeAgentConfig(BaseModel): - model_config = ConfigDict(frozen=True) - name: str = "agent-1" - role: str = "developer" - department: str = "eng" - - -class _FakeDepartment(BaseModel): - model_config = ConfigDict(frozen=True) - name: str = "eng" - head: str = "lead" - - -class _FakeProviderConfig(BaseModel): - model_config = ConfigDict(frozen=True) - driver: str = "litellm" - - class _FakeRootConfig(BaseModel): model_config = ConfigDict(frozen=True) api: _FakeApiConfig = _FakeApiConfig() budget: _BudgetConfig = _BudgetConfig() coordination: _CoordinationSection = _CoordinationSection() - config: _CompanyConfig = _CompanyConfig() - agents: tuple[_FakeAgentConfig, ...] = () - departments: tuple[_FakeDepartment, ...] = () - providers: dict[str, _FakeProviderConfig] = {} + agents: tuple[FakeAgentConfig, ...] = () + departments: tuple[FakeDepartment, ...] = () + providers: dict[str, FakeProviderConfig] = {} # ── Fixtures ────────────────────────────────────────────────────── diff --git a/tests/unit/settings/test_resolver_structural.py b/tests/unit/settings/test_resolver_structural.py index a8f930d531..089f712751 100644 --- a/tests/unit/settings/test_resolver_structural.py +++ b/tests/unit/settings/test_resolver_structural.py @@ -11,50 +11,24 @@ import pytest from pydantic import BaseModel, ConfigDict -from synthorg.settings.enums import SettingNamespace, SettingSource +from synthorg.settings.enums import SettingNamespace from synthorg.settings.errors import SettingNotFoundError -from synthorg.settings.models import SettingValue from synthorg.settings.resolver import ConfigResolver - -# ── Helpers ─────────────────────────────────────────────────────── - - -def _make_value( - value: str, - namespace: SettingNamespace = SettingNamespace.BUDGET, - key: str = "total_monthly", -) -> SettingValue: - return SettingValue( - namespace=namespace, - key=key, - value=value, - source=SettingSource.DEFAULT, - ) - - -class _FakeAgentConfig(BaseModel): - model_config = ConfigDict(frozen=True) - name: str = "agent-1" - role: str = "developer" - department: str = "eng" - - -class _FakeDepartment(BaseModel): - model_config = ConfigDict(frozen=True) - name: str = "eng" - head: str = "lead" - - -class _FakeProviderConfig(BaseModel): - model_config = ConfigDict(frozen=True) - driver: str = "litellm" +from tests.unit.settings.conftest import ( + FakeAgentConfig, + FakeDepartment, + FakeProviderConfig, +) +from tests.unit.settings.conftest import ( + make_setting_value as _make_value, +) class _FakeRootConfig(BaseModel): model_config = ConfigDict(frozen=True) - agents: tuple[_FakeAgentConfig, ...] = () - departments: tuple[_FakeDepartment, ...] = () - providers: dict[str, _FakeProviderConfig] = {} + agents: tuple[FakeAgentConfig, ...] = () + departments: tuple[FakeDepartment, ...] = () + providers: dict[str, FakeProviderConfig] = {} # ── Fixtures ────────────────────────────────────────────────────── @@ -155,16 +129,14 @@ async def test_json_roundtrip(self, mock_settings: AsyncMock) -> None: assert result[0].name == "alice" assert result[1].name == "bob" - async def test_empty_list_falls_back_to_config( - self, mock_settings: AsyncMock - ) -> None: - """Empty JSON list -> fall back to config.agents.""" + async def test_empty_list_is_valid_override(self, mock_settings: AsyncMock) -> None: + """Empty JSON list is a valid override returning empty tuple.""" mock_settings.get.return_value = _make_value( "[]", namespace=SettingNamespace.COMPANY, key="agents", ) - agent = _FakeAgentConfig(name="fallback-agent") + agent = FakeAgentConfig(name="fallback-agent") config = _FakeRootConfig(agents=(agent,)) resolver = ConfigResolver( settings_service=mock_settings, @@ -172,8 +144,7 @@ async def test_empty_list_falls_back_to_config( ) result = await resolver.get_agents() - assert len(result) == 1 - assert result[0].name == "fallback-agent" + assert result == () async def test_invalid_json_falls_back_to_config( self, mock_settings: AsyncMock @@ -184,7 +155,7 @@ async def test_invalid_json_falls_back_to_config( namespace=SettingNamespace.COMPANY, key="agents", ) - agent = _FakeAgentConfig(name="safe-agent") + agent = FakeAgentConfig(name="safe-agent") config = _FakeRootConfig(agents=(agent,)) resolver = ConfigResolver( settings_service=mock_settings, @@ -211,7 +182,7 @@ async def test_invalid_schema_falls_back_to_config( namespace=SettingNamespace.COMPANY, key="agents", ) - agent = _FakeAgentConfig(name="schema-fallback") + agent = FakeAgentConfig(name="schema-fallback") config = _FakeRootConfig(agents=(agent,)) resolver = ConfigResolver( settings_service=mock_settings, @@ -231,7 +202,7 @@ async def test_wrong_json_shape_falls_back_to_config( namespace=SettingNamespace.COMPANY, key="agents", ) - agent = _FakeAgentConfig(name="shape-fallback") + agent = FakeAgentConfig(name="shape-fallback") config = _FakeRootConfig(agents=(agent,)) resolver = ConfigResolver( settings_service=mock_settings, @@ -274,15 +245,14 @@ async def test_json_roundtrip(self, mock_settings: AsyncMock) -> None: assert isinstance(result[0], Department) assert result[0].name == "engineering" - async def test_empty_list_falls_back_to_config( - self, mock_settings: AsyncMock - ) -> None: + async def test_empty_list_is_valid_override(self, mock_settings: AsyncMock) -> None: + """Empty JSON list is a valid override returning empty tuple.""" mock_settings.get.return_value = _make_value( "[]", namespace=SettingNamespace.COMPANY, key="departments", ) - dept = _FakeDepartment(name="fallback-dept") + dept = FakeDepartment(name="fallback-dept") config = _FakeRootConfig(departments=(dept,)) resolver = ConfigResolver( settings_service=mock_settings, @@ -290,8 +260,7 @@ async def test_empty_list_falls_back_to_config( ) result = await resolver.get_departments() - assert len(result) == 1 - assert result[0].name == "fallback-dept" + assert result == () async def test_invalid_json_falls_back_to_config( self, mock_settings: AsyncMock @@ -301,7 +270,7 @@ async def test_invalid_json_falls_back_to_config( namespace=SettingNamespace.COMPANY, key="departments", ) - dept = _FakeDepartment(name="safe-dept") + dept = FakeDepartment(name="safe-dept") config = _FakeRootConfig(departments=(dept,)) resolver = ConfigResolver( settings_service=mock_settings, @@ -328,7 +297,7 @@ async def test_invalid_schema_falls_back_to_config( namespace=SettingNamespace.COMPANY, key="departments", ) - dept = _FakeDepartment(name="schema-dept") + dept = FakeDepartment(name="schema-dept") config = _FakeRootConfig(departments=(dept,)) resolver = ConfigResolver( settings_service=mock_settings, @@ -339,6 +308,26 @@ async def test_invalid_schema_falls_back_to_config( assert len(result) == 1 assert result[0].name == "schema-dept" + async def test_wrong_json_shape_falls_back_to_config( + self, mock_settings: AsyncMock + ) -> None: + """JSON dict instead of list -> fall back.""" + mock_settings.get.return_value = _make_value( + '{"name": "eng"}', + namespace=SettingNamespace.COMPANY, + key="departments", + ) + dept = FakeDepartment(name="shape-dept") + config = _FakeRootConfig(departments=(dept,)) + resolver = ConfigResolver( + settings_service=mock_settings, + config=config, # type: ignore[arg-type] + ) + result = await resolver.get_departments() + + assert len(result) == 1 + assert result[0].name == "shape-dept" + # ── Composed Read: Provider Configs ────────────────────────────── @@ -371,9 +360,8 @@ async def test_json_roundtrip(self, mock_settings: AsyncMock) -> None: assert isinstance(result["test-provider"], ProviderConfig) assert result["test-provider"].driver == "litellm" - async def test_empty_dict_falls_back_to_config( - self, mock_settings: AsyncMock - ) -> None: + async def test_empty_dict_is_valid_override(self, mock_settings: AsyncMock) -> None: + """Empty JSON dict is a valid override returning empty dict.""" mock_settings.get.return_value = _make_value( "{}", namespace=SettingNamespace.PROVIDERS, @@ -381,7 +369,7 @@ async def test_empty_dict_falls_back_to_config( ) config = _FakeRootConfig( providers={ - "fallback": _FakeProviderConfig(driver="test-driver"), + "fallback": FakeProviderConfig(driver="test-driver"), }, ) resolver = ConfigResolver( @@ -390,8 +378,7 @@ async def test_empty_dict_falls_back_to_config( ) result = await resolver.get_provider_configs() - assert "fallback" in result - assert result["fallback"].driver == "test-driver" + assert result == {} async def test_invalid_json_falls_back_to_config( self, mock_settings: AsyncMock @@ -402,7 +389,7 @@ async def test_invalid_json_falls_back_to_config( key="configs", ) config = _FakeRootConfig( - providers={"safe": _FakeProviderConfig()}, + providers={"safe": FakeProviderConfig()}, ) resolver = ConfigResolver( settings_service=mock_settings, @@ -429,7 +416,7 @@ async def test_invalid_schema_falls_back_to_config( key="configs", ) config = _FakeRootConfig( - providers={"safe": _FakeProviderConfig()}, + providers={"safe": FakeProviderConfig()}, ) resolver = ConfigResolver( settings_service=mock_settings, @@ -449,7 +436,7 @@ async def test_wrong_json_shape_falls_back_to_config( key="configs", ) config = _FakeRootConfig( - providers={"shape-safe": _FakeProviderConfig()}, + providers={"shape-safe": FakeProviderConfig()}, ) resolver = ConfigResolver( settings_service=mock_settings, From 534667bb7f28a4ede8859d7414a3efa827d095b5 Mon Sep 17 00:00:00 2001 From: Aurelio <19254254+Aureliolo@users.noreply.github.com> Date: Tue, 17 Mar 2026 22:52:53 +0100 Subject: [PATCH 4/7] fix(settings): address 7 CodeRabbit review findings on latest commit - Add terminal period to operations.md Settings paragraph - Wrap analytics TaskGroup in ExceptionGroup handler (matches company.py) - Add warning log in company ExceptionGroup handler with event constant - Add Google-style docstrings to test fixture classes in conftest.py - Parametrize scalar _serialize_value tests (DRY) - Consolidate split conftest imports via module-level alias --- src/synthorg/api/controllers/analytics.py | 11 +++++---- src/synthorg/api/controllers/company.py | 8 +++++++ tests/unit/settings/conftest.py | 21 ++++++++++++++++ tests/unit/settings/test_config_bridge.py | 24 +++++++++---------- tests/unit/settings/test_resolver.py | 6 ++--- .../unit/settings/test_resolver_structural.py | 6 ++--- 6 files changed, 54 insertions(+), 22 deletions(-) diff --git a/src/synthorg/api/controllers/analytics.py b/src/synthorg/api/controllers/analytics.py index 52ca8fbe5b..1cb632e497 100644 --- a/src/synthorg/api/controllers/analytics.py +++ b/src/synthorg/api/controllers/analytics.py @@ -57,10 +57,13 @@ async def get_overview( """ app_state: AppState = state.app_state - async with asyncio.TaskGroup() as tg: - t_tasks = tg.create_task(app_state.persistence.tasks.list_tasks()) - t_cost = tg.create_task(app_state.cost_tracker.get_total_cost()) - t_agents = tg.create_task(app_state.config_resolver.get_agents()) + try: + async with asyncio.TaskGroup() as tg: + t_tasks = tg.create_task(app_state.persistence.tasks.list_tasks()) + t_cost = tg.create_task(app_state.cost_tracker.get_total_cost()) + t_agents = tg.create_task(app_state.config_resolver.get_agents()) + except ExceptionGroup as eg: + raise eg.exceptions[0] from eg all_tasks = t_tasks.result() counts = Counter(t.status.value for t in all_tasks) diff --git a/src/synthorg/api/controllers/company.py b/src/synthorg/api/controllers/company.py index a29b87fca3..eb562e3e4a 100644 --- a/src/synthorg/api/controllers/company.py +++ b/src/synthorg/api/controllers/company.py @@ -11,6 +11,7 @@ from synthorg.api.state import AppState # noqa: TC001 from synthorg.core.company import Department # noqa: TC001 from synthorg.observability import get_logger +from synthorg.observability.events.settings import SETTINGS_FETCH_FAILED logger = get_logger(__name__) @@ -46,6 +47,13 @@ async def get_company( t_agents = tg.create_task(resolver.get_agents()) t_depts = tg.create_task(resolver.get_departments()) except ExceptionGroup as eg: + logger.warning( + SETTINGS_FETCH_FAILED, + namespace="company", + key="_composed", + error_count=len(eg.exceptions), + exc_info=True, + ) raise eg.exceptions[0] from eg data: dict[str, Any] = { "company_name": t_name.result(), diff --git a/tests/unit/settings/conftest.py b/tests/unit/settings/conftest.py index 13fe521892..21d5ff05bb 100644 --- a/tests/unit/settings/conftest.py +++ b/tests/unit/settings/conftest.py @@ -24,6 +24,14 @@ def make_setting_value( class FakeAgentConfig(BaseModel): + """Frozen test fixture for agent configuration. + + Attributes: + name: Agent identifier. + role: Agent role. + department: Owning department. + """ + model_config = ConfigDict(frozen=True) name: str = "agent-1" role: str = "developer" @@ -31,11 +39,24 @@ class FakeAgentConfig(BaseModel): class FakeDepartment(BaseModel): + """Frozen test fixture for department configuration. + + Attributes: + name: Department identifier. + head: Department head name. + """ + model_config = ConfigDict(frozen=True) name: str = "eng" head: str = "lead" class FakeProviderConfig(BaseModel): + """Frozen test fixture for LLM provider configuration. + + Attributes: + driver: Provider driver name. + """ + model_config = ConfigDict(frozen=True) driver: str = "litellm" diff --git a/tests/unit/settings/test_config_bridge.py b/tests/unit/settings/test_config_bridge.py index bd34c0dbe2..aec046af48 100644 --- a/tests/unit/settings/test_config_bridge.py +++ b/tests/unit/settings/test_config_bridge.py @@ -111,18 +111,18 @@ def test_empty_dict(self) -> None: result = _serialize_value({}) assert result == "{}" - def test_scalar_string_unchanged(self) -> None: - assert _serialize_value("hello") == "hello" - - def test_scalar_int_unchanged(self) -> None: - assert _serialize_value(42) == "42" - - def test_scalar_float_unchanged(self) -> None: - assert _serialize_value(3.14) == "3.14" - - def test_scalar_bool_lowercase_json(self) -> None: - assert _serialize_value(True) == "true" - assert _serialize_value(False) == "false" + @pytest.mark.parametrize( + ("value", "expected"), + [ + ("hello", "hello"), + (42, "42"), + (3.14, "3.14"), + (True, "true"), + (False, "false"), + ], + ) + def test_scalar_values(self, value: object, expected: str) -> None: + assert _serialize_value(value) == expected def test_mixed_list_models_and_scalars(self) -> None: items = [_ItemModel(name="x", value=1), "plain", 42] diff --git a/tests/unit/settings/test_resolver.py b/tests/unit/settings/test_resolver.py index 018c9b654f..14f85a5af9 100644 --- a/tests/unit/settings/test_resolver.py +++ b/tests/unit/settings/test_resolver.py @@ -20,10 +20,10 @@ FakeAgentConfig, FakeDepartment, FakeProviderConfig, + make_setting_value, ) -from tests.unit.settings.conftest import ( - make_setting_value as _make_value, -) + +_make_value = make_setting_value # ── Helpers ─────────────────────────────────────────────────────── diff --git a/tests/unit/settings/test_resolver_structural.py b/tests/unit/settings/test_resolver_structural.py index 089f712751..2d3f036ba7 100644 --- a/tests/unit/settings/test_resolver_structural.py +++ b/tests/unit/settings/test_resolver_structural.py @@ -18,10 +18,10 @@ FakeAgentConfig, FakeDepartment, FakeProviderConfig, + make_setting_value, ) -from tests.unit.settings.conftest import ( - make_setting_value as _make_value, -) + +_make_value = make_setting_value class _FakeRootConfig(BaseModel): From 52d626ae69cefbcb0cd9a948bf8253f3e6c1b8d5 Mon Sep 17 00:00:00 2001 From: Aurelio <19254254+Aureliolo@users.noreply.github.com> Date: Tue, 17 Mar 2026 23:14:19 +0100 Subject: [PATCH 5/7] fix(settings): address 7 CodeRabbit round-2 findings - Add ExceptionGroup warning log in analytics controller (matches company.py) - Recursive _to_json_compatible for nested BaseModel serialization in config bridge - Raise TypeError for unsupported types in _serialize_value (set, bytes, etc.) - Extract _resolve_list_setting / _resolve_dict_setting helpers in resolver (DRY, each public method now under 50 lines) - Add HTTP status assertions in company/provider DB-override tests - Add defensive-copy regression test for get_provider_configs --- src/synthorg/api/controllers/analytics.py | 8 + src/synthorg/settings/config_bridge.py | 48 +++-- src/synthorg/settings/resolver.py | 197 +++++++++--------- tests/unit/api/controllers/test_company.py | 5 +- tests/unit/api/controllers/test_providers.py | 1 + tests/unit/settings/test_config_bridge.py | 4 + .../unit/settings/test_resolver_structural.py | 22 ++ 7 files changed, 166 insertions(+), 119 deletions(-) diff --git a/src/synthorg/api/controllers/analytics.py b/src/synthorg/api/controllers/analytics.py index 1cb632e497..beb772a7fc 100644 --- a/src/synthorg/api/controllers/analytics.py +++ b/src/synthorg/api/controllers/analytics.py @@ -12,6 +12,7 @@ from synthorg.api.state import AppState # noqa: TC001 from synthorg.core.enums import TaskStatus from synthorg.observability import get_logger +from synthorg.observability.events.settings import SETTINGS_FETCH_FAILED logger = get_logger(__name__) @@ -63,6 +64,13 @@ async def get_overview( t_cost = tg.create_task(app_state.cost_tracker.get_total_cost()) t_agents = tg.create_task(app_state.config_resolver.get_agents()) except ExceptionGroup as eg: + logger.warning( + SETTINGS_FETCH_FAILED, + namespace="analytics", + key="_overview", + error_count=len(eg.exceptions), + exc_info=True, + ) raise eg.exceptions[0] from eg all_tasks = t_tasks.result() diff --git a/src/synthorg/settings/config_bridge.py b/src/synthorg/settings/config_bridge.py index f6b631058a..5a0172b739 100644 --- a/src/synthorg/settings/config_bridge.py +++ b/src/synthorg/settings/config_bridge.py @@ -14,12 +14,29 @@ logger = get_logger(__name__) +def _to_json_compatible(value: object) -> object: + """Recursively convert Pydantic models to JSON-compatible dicts. + + Walks nested structures so that ``BaseModel`` instances at any + depth are replaced by their ``model_dump(mode="json")`` output. + """ + if isinstance(value, BaseModel): + return value.model_dump(mode="json") + if isinstance(value, (tuple, list)): + return [_to_json_compatible(item) for item in value] + if isinstance(value, dict): + return {k: _to_json_compatible(v) for k, v in value.items()} + return value + + def _serialize_value(value: object) -> str: """Serialize a resolved config value to a string. - Handles Pydantic models, tuples/lists, and dicts by producing - valid JSON. Scalar booleans produce lowercase JSON-style - ``"true"``/``"false"``. Other scalars fall back to ``str()``. + Handles Pydantic models, tuples/lists, and dicts (including + nested models at any depth) by producing valid JSON. Scalar + booleans produce lowercase JSON-style ``"true"``/``"false"``. + Other accepted scalars (``str``, ``int``, ``float``) use + ``str()``. Args: value: The resolved config attribute. @@ -28,32 +45,27 @@ def _serialize_value(value: object) -> str: A string representation suitable for the settings layer. Raises: - TypeError: If *value* contains non-JSON-serializable types - (e.g. ``set``, ``bytes``, ``datetime``). + TypeError: If *value* is not an accepted type (accepted: + ``BaseModel``, ``tuple``, ``list``, ``dict``, ``str``, + ``int``, ``float``, ``bool``). """ if isinstance(value, BaseModel): return json.dumps(value.model_dump(mode="json")) if isinstance(value, (tuple, list)): - return json.dumps( - [ - item.model_dump(mode="json") if isinstance(item, BaseModel) else item - for item in value - ] - ) + return json.dumps(_to_json_compatible(value)) if isinstance(value, dict): - return json.dumps( - { - k: v.model_dump(mode="json") if isinstance(v, BaseModel) else v - for k, v in value.items() - } - ) + return json.dumps(_to_json_compatible(value)) if isinstance(value, bool): return "true" if value else "false" - return str(value) + if isinstance(value, (str, int, float)): + return str(value) + + msg = f"Cannot serialize {type(value).__name__} to settings string" + raise TypeError(msg) def extract_from_config(config: object, yaml_path: str) -> str | None: diff --git a/src/synthorg/settings/resolver.py b/src/synthorg/settings/resolver.py index 1a8dba07c5..34d3d9d72f 100644 --- a/src/synthorg/settings/resolver.py +++ b/src/synthorg/settings/resolver.py @@ -21,6 +21,8 @@ from synthorg.settings.errors import SettingNotFoundError, SettingsEncryptionError if TYPE_CHECKING: + from pydantic import BaseModel + from synthorg.api.config import ApiConfig from synthorg.budget.config import BudgetAlertConfig, BudgetConfig from synthorg.config.schema import AgentConfig, ProviderConfig, RootConfig @@ -310,169 +312,164 @@ async def get_json(self, namespace: str, key: str) -> Any: msg = f"Setting {namespace}/{key} has an invalid JSON value" raise ValueError(msg) from exc - async def get_agents(self) -> tuple[AgentConfig, ...]: - """Resolve agent configurations from settings. - - Falls back to ``RootConfig.agents`` if the setting value is - ``None``, contains invalid JSON, or fails schema validation. - An explicit empty list ``[]`` is a valid override that returns - an empty tuple. - - Returns: - A tuple of ``AgentConfig`` objects. + async def _resolve_list_setting( + self, + namespace: str, + key: str, + model_cls: type[BaseModel], + fallback: tuple[Any, ...], + ) -> tuple[Any, ...]: + """Resolve a JSON list setting to a tuple of validated models. - Raises: - SettingNotFoundError: If the agents key is not - in the registry. - SettingsEncryptionError: If decryption fails. + Falls back to *fallback* on ``None``, invalid JSON, wrong + shape, or schema validation failure. """ from pydantic import ValidationError # noqa: PLC0415 - from synthorg.config.schema import AgentConfig # noqa: PLC0415 - try: - raw = await self.get_json("company", "agents") + raw = await self.get_json(namespace, key) except ValueError: logger.warning( SETTINGS_FETCH_FAILED, - namespace="company", - key="agents", + namespace=namespace, + key=key, reason="invalid_json_fallback", exc_info=True, ) - return self._config.agents + return fallback if raw is None: - return self._config.agents + return fallback if not isinstance(raw, list): logger.warning( SETTINGS_FETCH_FAILED, - namespace="company", - key="agents", + namespace=namespace, + key=key, reason="expected_list_fallback", value_type=type(raw).__name__, ) - return self._config.agents + return fallback try: - return tuple(AgentConfig.model_validate(item) for item in raw) + return tuple(model_cls.model_validate(item) for item in raw) except ValidationError: logger.warning( SETTINGS_FETCH_FAILED, - namespace="company", - key="agents", + namespace=namespace, + key=key, reason="invalid_schema_fallback", exc_info=True, ) - return self._config.agents - - async def get_departments(self) -> tuple[Department, ...]: - """Resolve department configurations from settings. + return fallback - Falls back to ``RootConfig.departments`` if the setting value - is ``None``, contains invalid JSON, or fails schema validation. - An explicit empty list ``[]`` is a valid override that returns - an empty tuple. - - Returns: - A tuple of ``Department`` objects. + async def _resolve_dict_setting( + self, + namespace: str, + key: str, + model_cls: type[BaseModel], + fallback: dict[str, Any], + ) -> dict[str, Any]: + """Resolve a JSON dict setting to a dict of validated models. - Raises: - SettingNotFoundError: If the departments key is not - in the registry. - SettingsEncryptionError: If decryption fails. + Falls back to *fallback* on ``None``, invalid JSON, wrong + shape, or schema validation failure. """ from pydantic import ValidationError # noqa: PLC0415 - from synthorg.core.company import Department # noqa: PLC0415 - try: - raw = await self.get_json("company", "departments") + raw = await self.get_json(namespace, key) except ValueError: logger.warning( SETTINGS_FETCH_FAILED, - namespace="company", - key="departments", + namespace=namespace, + key=key, reason="invalid_json_fallback", exc_info=True, ) - return self._config.departments + return fallback if raw is None: - return self._config.departments - if not isinstance(raw, list): + return fallback + if not isinstance(raw, dict): logger.warning( SETTINGS_FETCH_FAILED, - namespace="company", - key="departments", - reason="expected_list_fallback", + namespace=namespace, + key=key, + reason="expected_dict_fallback", value_type=type(raw).__name__, ) - return self._config.departments + return fallback try: - return tuple(Department.model_validate(item) for item in raw) + return {name: model_cls.model_validate(conf) for name, conf in raw.items()} except ValidationError: logger.warning( SETTINGS_FETCH_FAILED, - namespace="company", - key="departments", + namespace=namespace, + key=key, reason="invalid_schema_fallback", exc_info=True, ) - return self._config.departments + return fallback + + async def get_agents(self) -> tuple[AgentConfig, ...]: + """Resolve agent configurations from settings. + + Falls back to ``RootConfig.agents`` if the setting value is + ``None``, contains invalid JSON, or fails schema validation. + An explicit empty list ``[]`` is a valid override. + + Raises: + SettingNotFoundError: If the agents key is not + in the registry. + SettingsEncryptionError: If decryption fails. + """ + from synthorg.config.schema import AgentConfig # noqa: PLC0415 + + return await self._resolve_list_setting( + "company", + "agents", + AgentConfig, + self._config.agents, + ) + + async def get_departments(self) -> tuple[Department, ...]: + """Resolve department configurations from settings. + + Falls back to ``RootConfig.departments`` if the setting value + is ``None``, contains invalid JSON, or fails schema validation. + An explicit empty list ``[]`` is a valid override. + + Raises: + SettingNotFoundError: If the departments key is not + in the registry. + SettingsEncryptionError: If decryption fails. + """ + from synthorg.core.company import Department # noqa: PLC0415 + + return await self._resolve_list_setting( + "company", + "departments", + Department, + self._config.departments, + ) async def get_provider_configs(self) -> dict[str, ProviderConfig]: """Resolve provider configurations from settings. Falls back to ``RootConfig.providers`` if the setting value is ``None``, contains invalid JSON, or fails schema validation. - An explicit empty dict ``{}`` is a valid override that returns - an empty dict. - - Returns: - A dict of provider name to ``ProviderConfig``. + An explicit empty dict ``{}`` is a valid override. Raises: SettingNotFoundError: If the ``configs`` key is not in the registry. SettingsEncryptionError: If decryption fails. """ - from pydantic import ValidationError # noqa: PLC0415 - from synthorg.config.schema import ProviderConfig # noqa: PLC0415 - try: - raw = await self.get_json("providers", "configs") - except ValueError: - logger.warning( - SETTINGS_FETCH_FAILED, - namespace="providers", - key="configs", - reason="invalid_json_fallback", - exc_info=True, - ) - return dict(self._config.providers) - if raw is None: - return dict(self._config.providers) - if not isinstance(raw, dict): - logger.warning( - SETTINGS_FETCH_FAILED, - namespace="providers", - key="configs", - reason="expected_dict_fallback", - value_type=type(raw).__name__, - ) - return dict(self._config.providers) - try: - return { - name: ProviderConfig.model_validate(conf) for name, conf in raw.items() - } - except ValidationError: - logger.warning( - SETTINGS_FETCH_FAILED, - namespace="providers", - key="configs", - reason="invalid_schema_fallback", - exc_info=True, - ) - return dict(self._config.providers) + return await self._resolve_dict_setting( + "providers", + "configs", + ProviderConfig, + dict(self._config.providers), + ) async def get_budget_config(self) -> BudgetConfig: """Assemble a ``BudgetConfig`` from individually resolved settings. diff --git a/tests/unit/api/controllers/test_company.py b/tests/unit/api/controllers/test_company.py index 1fdbf10da0..fcecf7eb24 100644 --- a/tests/unit/api/controllers/test_company.py +++ b/tests/unit/api/controllers/test_company.py @@ -83,6 +83,7 @@ async def test_db_company_departments_override( with TestClient(app) as client: client.headers.update(make_auth_headers("ceo")) resp = client.get("/api/v1/company/departments") + assert resp.status_code == 200 body = resp.json() assert body["success"] is True assert len(body["data"]) == 1 @@ -125,7 +126,8 @@ async def test_taskgroup_error_returns_clean_error_response( resp = client.get("/api/v1/company") assert resp.status_code == 500 body = resp.json() - assert body.get("success") is False or "detail" in body + assert body["success"] is False + assert body["error"] is not None async def test_db_company_overview_includes_db_agents( self, @@ -160,6 +162,7 @@ async def test_db_company_overview_includes_db_agents( with TestClient(app) as client: client.headers.update(make_auth_headers("ceo")) resp = client.get("/api/v1/company") + assert resp.status_code == 200 body = resp.json() assert body["success"] is True assert len(body["data"]["agents"]) == 1 diff --git a/tests/unit/api/controllers/test_providers.py b/tests/unit/api/controllers/test_providers.py index f962d0d339..5db8e9931a 100644 --- a/tests/unit/api/controllers/test_providers.py +++ b/tests/unit/api/controllers/test_providers.py @@ -97,6 +97,7 @@ async def test_db_providers_override_config( with TestClient(app) as client: client.headers.update(make_auth_headers("observer")) resp = client.get("/api/v1/providers") + assert resp.status_code == 200 body = resp.json() assert "db-provider" in body["data"] # api_key should be stripped diff --git a/tests/unit/settings/test_config_bridge.py b/tests/unit/settings/test_config_bridge.py index aec046af48..44de17aec2 100644 --- a/tests/unit/settings/test_config_bridge.py +++ b/tests/unit/settings/test_config_bridge.py @@ -130,6 +130,10 @@ def test_mixed_list_models_and_scalars(self) -> None: parsed = json.loads(result) assert parsed == [{"name": "x", "value": 1}, "plain", 42] + def test_unsupported_type_raises_type_error(self) -> None: + with pytest.raises(TypeError, match="set"): + _serialize_value({1, 2, 3}) + def test_mixed_dict_models_and_scalars(self) -> None: providers: dict[str, object] = { "a": _InnerConfig(daily_limit=5.0), diff --git a/tests/unit/settings/test_resolver_structural.py b/tests/unit/settings/test_resolver_structural.py index 2d3f036ba7..03bd6fb088 100644 --- a/tests/unit/settings/test_resolver_structural.py +++ b/tests/unit/settings/test_resolver_structural.py @@ -445,3 +445,25 @@ async def test_wrong_json_shape_falls_back_to_config( result = await resolver.get_provider_configs() assert "shape-safe" in result + + async def test_fallback_returns_defensive_copy( + self, mock_settings: AsyncMock + ) -> None: + """Returned dict must be a copy — mutating it must not affect config.""" + mock_settings.get.return_value = _make_value( + "null", + namespace=SettingNamespace.PROVIDERS, + key="configs", + ) + prov = FakeProviderConfig(driver="original") + config = _FakeRootConfig(providers={"p": prov}) + resolver = ConfigResolver( + settings_service=mock_settings, + config=config, # type: ignore[arg-type] + ) + result = await resolver.get_provider_configs() + result["injected"] = FakeProviderConfig(driver="evil") # type: ignore[assignment] + + fresh = await resolver.get_provider_configs() + assert "injected" not in fresh + assert "p" in fresh From 9e6f646bdd65b4439be1a9ea8c02e82fbff5a3ab Mon Sep 17 00:00:00 2001 From: Aurelio <19254254+Aureliolo@users.noreply.github.com> Date: Tue, 17 Mar 2026 23:35:35 +0100 Subject: [PATCH 6/7] fix(settings): address 6 CodeRabbit round-3 findings - Use API_REQUEST_ERROR instead of SETTINGS_FETCH_FAILED in analytics TaskGroup handler (mixed settings + non-settings calls) - Catch TypeError from _serialize_value in extract_from_config with yaml_path context logging before re-raise - Add HTTP status assertion in agents DB-override test - Mark all DbOverride test classes as @pytest.mark.integration - Add detail endpoint assertions in departments and providers DB-override tests (get_department, get_provider) --- src/synthorg/api/controllers/analytics.py | 7 +++---- src/synthorg/settings/config_bridge.py | 12 +++++++++++- tests/unit/api/controllers/test_agents.py | 3 ++- tests/unit/api/controllers/test_analytics.py | 2 +- tests/unit/api/controllers/test_company.py | 2 +- tests/unit/api/controllers/test_departments.py | 8 +++++++- tests/unit/api/controllers/test_providers.py | 8 +++++++- 7 files changed, 32 insertions(+), 10 deletions(-) diff --git a/src/synthorg/api/controllers/analytics.py b/src/synthorg/api/controllers/analytics.py index beb772a7fc..0e4d0aa912 100644 --- a/src/synthorg/api/controllers/analytics.py +++ b/src/synthorg/api/controllers/analytics.py @@ -12,7 +12,7 @@ from synthorg.api.state import AppState # noqa: TC001 from synthorg.core.enums import TaskStatus from synthorg.observability import get_logger -from synthorg.observability.events.settings import SETTINGS_FETCH_FAILED +from synthorg.observability.events.api import API_REQUEST_ERROR logger = get_logger(__name__) @@ -65,9 +65,8 @@ async def get_overview( t_agents = tg.create_task(app_state.config_resolver.get_agents()) except ExceptionGroup as eg: logger.warning( - SETTINGS_FETCH_FAILED, - namespace="analytics", - key="_overview", + API_REQUEST_ERROR, + endpoint="analytics.overview", error_count=len(eg.exceptions), exc_info=True, ) diff --git a/src/synthorg/settings/config_bridge.py b/src/synthorg/settings/config_bridge.py index 5a0172b739..e9f1f9dedb 100644 --- a/src/synthorg/settings/config_bridge.py +++ b/src/synthorg/settings/config_bridge.py @@ -101,4 +101,14 @@ def extract_from_config(config: object, yaml_path: str) -> str | None: return None if current is None: return None - return _serialize_value(current) + try: + return _serialize_value(current) + except TypeError: + logger.warning( + SETTINGS_CONFIG_PATH_MISS, + yaml_path=yaml_path, + reason="unsupported_type", + value_type=type(current).__name__, + exc_info=True, + ) + raise diff --git a/tests/unit/api/controllers/test_agents.py b/tests/unit/api/controllers/test_agents.py index 2d8890d946..f8351b6657 100644 --- a/tests/unit/api/controllers/test_agents.py +++ b/tests/unit/api/controllers/test_agents.py @@ -74,7 +74,7 @@ def test_get_agent_not_found(self, test_client: TestClient[Any]) -> None: assert resp.json()["success"] is False -@pytest.mark.unit +@pytest.mark.integration @pytest.mark.timeout(30) class TestAgentControllerDbOverride: """Test that DB-stored settings override YAML agents.""" @@ -118,6 +118,7 @@ async def test_db_agents_override_config( with TestClient(app) as client: client.headers.update(make_auth_headers("observer")) resp = client.get("/api/v1/agents") + assert resp.status_code == 200 body = resp.json() assert body["pagination"]["total"] == 2 names = {a["name"] for a in body["data"]} diff --git a/tests/unit/api/controllers/test_analytics.py b/tests/unit/api/controllers/test_analytics.py index 16ab10cace..1961a2f280 100644 --- a/tests/unit/api/controllers/test_analytics.py +++ b/tests/unit/api/controllers/test_analytics.py @@ -42,7 +42,7 @@ def test_overview_requires_read_access(self, test_client: TestClient[Any]) -> No assert resp.status_code == 401 -@pytest.mark.unit +@pytest.mark.integration @pytest.mark.timeout(30) class TestAnalyticsControllerDbOverride: """Test that DB-stored agents affect analytics agent count.""" diff --git a/tests/unit/api/controllers/test_company.py b/tests/unit/api/controllers/test_company.py index fcecf7eb24..177de57a3e 100644 --- a/tests/unit/api/controllers/test_company.py +++ b/tests/unit/api/controllers/test_company.py @@ -45,7 +45,7 @@ def test_company_requires_read_access(self, test_client: TestClient[Any]) -> Non assert resp.status_code == 401 -@pytest.mark.unit +@pytest.mark.integration @pytest.mark.timeout(30) class TestCompanyControllerDbOverride: """Test that DB-stored settings override YAML company data.""" diff --git a/tests/unit/api/controllers/test_departments.py b/tests/unit/api/controllers/test_departments.py index 8c6b5cb0b2..72c79be8cf 100644 --- a/tests/unit/api/controllers/test_departments.py +++ b/tests/unit/api/controllers/test_departments.py @@ -32,7 +32,7 @@ def test_get_department_not_found(self, test_client: TestClient[Any]) -> None: assert resp.json()["success"] is False -@pytest.mark.unit +@pytest.mark.integration @pytest.mark.timeout(30) class TestDepartmentControllerDbOverride: """Test that DB-stored settings override YAML departments.""" @@ -72,6 +72,12 @@ async def test_db_departments_override_config( with TestClient(app) as client: client.headers.update(make_auth_headers("observer")) resp = client.get("/api/v1/departments") + assert resp.status_code == 200 body = resp.json() assert body["pagination"]["total"] == 1 assert body["data"][0]["name"] == "db-dept" + + detail_resp = client.get("/api/v1/departments/db-dept") + assert detail_resp.status_code == 200 + detail = detail_resp.json() + assert detail["data"]["name"] == "db-dept" diff --git a/tests/unit/api/controllers/test_providers.py b/tests/unit/api/controllers/test_providers.py index 5db8e9931a..079069fdfd 100644 --- a/tests/unit/api/controllers/test_providers.py +++ b/tests/unit/api/controllers/test_providers.py @@ -51,7 +51,7 @@ def test_provider_api_key_stripped(self) -> None: assert safe.api_key is None -@pytest.mark.unit +@pytest.mark.integration @pytest.mark.timeout(30) class TestProviderControllerDbOverride: """Test that DB-stored settings override YAML providers.""" @@ -102,3 +102,9 @@ async def test_db_providers_override_config( assert "db-provider" in body["data"] # api_key should be stripped assert body["data"]["db-provider"].get("api_key") is None + + detail_resp = client.get("/api/v1/providers/db-provider") + assert detail_resp.status_code == 200 + detail = detail_resp.json() + assert detail["data"]["driver"] == "litellm" + assert detail["data"].get("api_key") is None From 4dbfdda0de0e5c55601c18c1039a5e9bf961049c Mon Sep 17 00:00:00 2001 From: Aurelio <19254254+Aureliolo@users.noreply.github.com> Date: Tue, 17 Mar 2026 23:45:28 +0100 Subject: [PATCH 7/7] fix(settings): address 4 CodeRabbit round-4 findings - Fix extract_from_config docstring to document boolean serialization - Add agent detail endpoint assertion in DB-override test - Add HTTP status assertion in analytics DB-override test - Extract shared db_override_app fixture in test_company.py (DRY) --- src/synthorg/settings/config_bridge.py | 5 +- tests/unit/api/controllers/test_agents.py | 5 + tests/unit/api/controllers/test_analytics.py | 1 + tests/unit/api/controllers/test_company.py | 109 +++++++------------ 4 files changed, 46 insertions(+), 74 deletions(-) diff --git a/src/synthorg/settings/config_bridge.py b/src/synthorg/settings/config_bridge.py index e9f1f9dedb..2acb0b1955 100644 --- a/src/synthorg/settings/config_bridge.py +++ b/src/synthorg/settings/config_bridge.py @@ -76,8 +76,9 @@ def extract_from_config(config: object, yaml_path: str) -> str | None: attribute exists and is not ``None``, otherwise ``None``. For Pydantic models, tuples/lists containing models, and - dicts with model values, the result is valid JSON. For - scalars, the result is ``str(value)``. + dicts with model values, the result is valid JSON. Scalar + booleans produce lowercase ``"true"``/``"false"``. Other + scalars (``str``, ``int``, ``float``) use ``str(value)``. Args: config: Root config object (typically ``RootConfig``). diff --git a/tests/unit/api/controllers/test_agents.py b/tests/unit/api/controllers/test_agents.py index f8351b6657..cf4516b3fe 100644 --- a/tests/unit/api/controllers/test_agents.py +++ b/tests/unit/api/controllers/test_agents.py @@ -123,3 +123,8 @@ async def test_db_agents_override_config( assert body["pagination"]["total"] == 2 names = {a["name"] for a in body["data"]} assert names == {"db-agent-1", "db-agent-2"} + + detail_resp = client.get("/api/v1/agents/db-agent-1") + assert detail_resp.status_code == 200 + detail = detail_resp.json() + assert detail["data"]["name"] == "db-agent-1" diff --git a/tests/unit/api/controllers/test_analytics.py b/tests/unit/api/controllers/test_analytics.py index 1961a2f280..0e79868598 100644 --- a/tests/unit/api/controllers/test_analytics.py +++ b/tests/unit/api/controllers/test_analytics.py @@ -84,6 +84,7 @@ async def test_db_agents_count_in_overview( with TestClient(app) as client: client.headers.update(make_auth_headers("ceo")) resp = client.get("/api/v1/analytics/overview") + assert resp.status_code == 200 body = resp.json() assert body["success"] is True assert body["data"]["total_agents"] == 3 diff --git a/tests/unit/api/controllers/test_company.py b/tests/unit/api/controllers/test_company.py index 177de57a3e..040e67f401 100644 --- a/tests/unit/api/controllers/test_company.py +++ b/tests/unit/api/controllers/test_company.py @@ -5,6 +5,7 @@ from unittest.mock import AsyncMock import pytest +from litestar import Litestar from litestar.testing import TestClient from synthorg.config.schema import RootConfig @@ -20,6 +21,36 @@ _HEADERS = make_auth_headers("ceo") +@pytest.fixture +async def db_override_app( + fake_persistence: FakePersistenceBackend, + fake_message_bus: FakeMessageBus, +) -> tuple[Litestar, SettingsService]: + """Build an app with a real SettingsService for DB-override tests.""" + from synthorg.api.app import create_app + from synthorg.api.auth.service import AuthService + from synthorg.budget.tracker import CostTracker + from tests.unit.api.conftest import _make_test_auth_service, _seed_test_users + + config = RootConfig(company_name="test") + auth_service: AuthService = _make_test_auth_service() + _seed_test_users(fake_persistence, auth_service) + settings_service = SettingsService( + repository=fake_persistence.settings, + registry=get_registry(), + config=config, + ) + app = create_app( + config=config, + persistence=fake_persistence, + message_bus=fake_message_bus, + cost_tracker=CostTracker(), + auth_service=auth_service, + settings_service=settings_service, + ) + return app, settings_service + + @pytest.mark.unit @pytest.mark.timeout(30) class TestCompanyController: @@ -52,34 +83,12 @@ class TestCompanyControllerDbOverride: async def test_db_company_departments_override( self, - fake_persistence: FakePersistenceBackend, - fake_message_bus: FakeMessageBus, + db_override_app: tuple[Litestar, SettingsService], ) -> None: - from synthorg.api.app import create_app - from synthorg.api.auth.service import AuthService - from synthorg.budget.tracker import CostTracker - from tests.unit.api.conftest import _make_test_auth_service, _seed_test_users - - config = RootConfig(company_name="test") - auth_service: AuthService = _make_test_auth_service() - _seed_test_users(fake_persistence, auth_service) - settings_service = SettingsService( - repository=fake_persistence.settings, - registry=get_registry(), - config=config, - ) - + app, settings_service = db_override_app db_depts = [{"name": "db-sales", "head": "bob"}] await settings_service.set("company", "departments", json.dumps(db_depts)) - app = create_app( - config=config, - persistence=fake_persistence, - message_bus=fake_message_bus, - cost_tracker=CostTracker(), - auth_service=auth_service, - settings_service=settings_service, - ) with TestClient(app) as client: client.headers.update(make_auth_headers("ceo")) resp = client.get("/api/v1/company/departments") @@ -91,32 +100,10 @@ async def test_db_company_departments_override( async def test_taskgroup_error_returns_clean_error_response( self, - fake_persistence: FakePersistenceBackend, - fake_message_bus: FakeMessageBus, + db_override_app: tuple[Litestar, SettingsService], ) -> None: """Verify TaskGroup exception unwraps to a clean API error.""" - from synthorg.api.app import create_app - from synthorg.api.auth.service import AuthService - from synthorg.budget.tracker import CostTracker - from tests.unit.api.conftest import _make_test_auth_service, _seed_test_users - - config = RootConfig(company_name="test") - auth_service: AuthService = _make_test_auth_service() - _seed_test_users(fake_persistence, auth_service) - settings_service = SettingsService( - repository=fake_persistence.settings, - registry=get_registry(), - config=config, - ) - - app = create_app( - config=config, - persistence=fake_persistence, - message_bus=fake_message_bus, - cost_tracker=CostTracker(), - auth_service=auth_service, - settings_service=settings_service, - ) + app, _settings_service = db_override_app with TestClient(app) as client: client.headers.update(make_auth_headers("ceo")) resolver = app.state.app_state.config_resolver @@ -131,34 +118,12 @@ async def test_taskgroup_error_returns_clean_error_response( async def test_db_company_overview_includes_db_agents( self, - fake_persistence: FakePersistenceBackend, - fake_message_bus: FakeMessageBus, + db_override_app: tuple[Litestar, SettingsService], ) -> None: - from synthorg.api.app import create_app - from synthorg.api.auth.service import AuthService - from synthorg.budget.tracker import CostTracker - from tests.unit.api.conftest import _make_test_auth_service, _seed_test_users - - config = RootConfig(company_name="test") - auth_service: AuthService = _make_test_auth_service() - _seed_test_users(fake_persistence, auth_service) - settings_service = SettingsService( - repository=fake_persistence.settings, - registry=get_registry(), - config=config, - ) - + app, settings_service = db_override_app db_agents = [{"name": "db-agent", "role": "dev", "department": "eng"}] await settings_service.set("company", "agents", json.dumps(db_agents)) - app = create_app( - config=config, - persistence=fake_persistence, - message_bus=fake_message_bus, - cost_tracker=CostTracker(), - auth_service=auth_service, - settings_service=settings_service, - ) with TestClient(app) as client: client.headers.update(make_auth_headers("ceo")) resp = client.get("/api/v1/company")