From 4b1fc525cdf4219bc2628da17adb245f15786d5d Mon Sep 17 00:00:00 2001 From: Lev Neiman Date: Fri, 13 Mar 2026 21:49:56 -0700 Subject: [PATCH 1/7] refactor: standardize server env config prefixes --- docker-compose.yml | 8 +- sdks/python/Makefile | 9 +- sdks/python/tests/README.md | 4 +- server/Dockerfile | 4 +- server/Makefile | 5 +- server/README.md | 5 + server/src/agent_control_server/config.py | 206 ++++++++++++------ .../src/agent_control_server/logging_utils.py | 36 ++- server/src/agent_control_server/main.py | 17 +- server/tests/test_config.py | 59 ++++- server/tests/test_logging_utils.py | 27 ++- 11 files changed, 285 insertions(+), 95 deletions(-) diff --git a/docker-compose.yml b/docker-compose.yml index d933893a..e33354d9 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -44,16 +44,16 @@ services: environment: # Database connection (uses Docker service name 'postgres') # Use postgresql+psycopg:// (supports both sync migrations and async app code) - DATABASE_URL: postgresql+psycopg://agent_control:agent_control@postgres:5432/agent_control + AGENT_CONTROL_DB_URL: postgresql+psycopg://agent_control:agent_control@postgres:5432/agent_control # Server configuration - HOST: 0.0.0.0 - PORT: 8000 + AGENT_CONTROL_HOST: 0.0.0.0 + AGENT_CONTROL_PORT: 8000 # API authentication (override via host env or server/.env; see server/.env.example) AGENT_CONTROL_API_KEY_ENABLED: ${AGENT_CONTROL_API_KEY_ENABLED:-false} AGENT_CONTROL_API_KEYS: ${AGENT_CONTROL_API_KEYS:-} AGENT_CONTROL_ADMIN_API_KEYS: ${AGENT_CONTROL_ADMIN_API_KEYS:-} AGENT_CONTROL_SESSION_SECRET: ${AGENT_CONTROL_SESSION_SECRET:-} - CORS_ORIGINS: ${CORS_ORIGINS:-http://localhost:4000} + AGENT_CONTROL_CORS_ORIGINS: ${AGENT_CONTROL_CORS_ORIGINS:-http://localhost:4000} depends_on: postgres: condition: service_healthy diff --git a/sdks/python/Makefile b/sdks/python/Makefile index 991e1f93..1d2e62d0 100644 --- a/sdks/python/Makefile +++ b/sdks/python/Makefile @@ -1,6 +1,7 @@ .PHONY: help test lint lint-fix typecheck build publish TEST_DB ?= agent_control_test +TEST_DB_ENV := AGENT_CONTROL_DB_URL= AGENT_CONTROL_DB_HOST=localhost AGENT_CONTROL_DB_PORT=5432 AGENT_CONTROL_DB_USER=agent_control AGENT_CONTROL_DB_PASSWORD=agent_control AGENT_CONTROL_DB_DATABASE=$(TEST_DB) help: @echo "Agent Control SDK - Makefile commands" @@ -19,15 +20,15 @@ help: test: @echo "Starting server for tests..." - DB_DATABASE=$(TEST_DB) uv run --package agent-control-server python scripts/reset_test_db.py - DB_DATABASE=$(TEST_DB) $(MAKE) -C ../../ server-alembic-upgrade + $(TEST_DB_ENV) uv run --package agent-control-server python scripts/reset_test_db.py + $(TEST_DB_ENV) $(MAKE) -C ../../ server-alembic-upgrade @# Start server in background and save PID - @DB_DATABASE=$(TEST_DB) uv run --package agent-control-server uvicorn agent_control_server.main:app --port 8000 --host 0.0.0.0 > server.log 2>&1 & echo $$! > server.pid + @$(TEST_DB_ENV) uv run --package agent-control-server uvicorn agent_control_server.main:app --port 8000 --host 0.0.0.0 > server.log 2>&1 & echo $$! > server.pid @echo "Waiting for server..." @bash -c 'for i in {1..30}; do if curl -s http://localhost:8000/health >/dev/null; then echo "Server up!"; exit 0; fi; sleep 1; done; echo "Server failed"; cat server.log; exit 1' @# Run tests, capture exit code, and ensure cleanup @set -e; \ - DB_DATABASE=$(TEST_DB) uv run pytest --cov=src --cov-report=xml:../../coverage-sdk.xml -q; \ + $(TEST_DB_ENV) uv run pytest --cov=src --cov-report=xml:../../coverage-sdk.xml -q; \ TEST_EXIT_CODE=$$?; \ echo "Stopping server..."; \ if [ -f server.pid ]; then kill `cat server.pid` && rm server.pid; fi; \ diff --git a/sdks/python/tests/README.md b/sdks/python/tests/README.md index b1dada52..dbc3f945 100644 --- a/sdks/python/tests/README.md +++ b/sdks/python/tests/README.md @@ -75,7 +75,7 @@ For SQLite (local testing): ```bash cd server -echo "DB_URL=sqlite+aiosqlite:///./test_agent_control.db" > .env +echo "AGENT_CONTROL_DB_URL=sqlite+aiosqlite:///./test_agent_control.db" > .env uv run alembic upgrade head ``` @@ -231,7 +231,7 @@ jobs: uv run uvicorn agent_control_server.main:app & sleep 5 env: - DB_URL: postgresql+psycopg://postgres:postgres@localhost/agent_control_test + AGENT_CONTROL_DB_URL: postgresql+psycopg://postgres:postgres@localhost/agent_control_test AGENT_CONTROL_API_KEYS: test-api-key-ci AGENT_CONTROL_ADMIN_API_KEYS: test-api-key-ci diff --git a/server/Dockerfile b/server/Dockerfile index 404ab6f6..7ca7b30d 100644 --- a/server/Dockerfile +++ b/server/Dockerfile @@ -70,8 +70,8 @@ RUN uv sync \ ENV PATH="/app/.venv/bin:$PATH" # Set default environment variables -ENV HOST=0.0.0.0 -ENV PORT=8000 +ENV AGENT_CONTROL_HOST=0.0.0.0 +ENV AGENT_CONTROL_PORT=8000 # Expose the port EXPOSE 8000 diff --git a/server/Makefile b/server/Makefile index 3aae2898..5a7a233a 100644 --- a/server/Makefile +++ b/server/Makefile @@ -9,13 +9,14 @@ SHOW ?= head STAMP ?= head TEST_DB ?= agent_control_test +TEST_DB_ENV := AGENT_CONTROL_DB_URL= AGENT_CONTROL_DB_HOST=localhost AGENT_CONTROL_DB_PORT=5432 AGENT_CONTROL_DB_USER=agent_control AGENT_CONTROL_DB_PASSWORD=agent_control AGENT_CONTROL_DB_DATABASE=$(TEST_DB) .PHONY: help run start-dependencies test migrate alembic-migrate alembic-revision alembic-upgrade alembic-downgrade alembic-current alembic-history alembic-heads alembic-show alembic-stamp help: @echo "Available targets:" @echo " run - start FastAPI server (reload)" @echo " start-dependencies - docker compose up -d (start local dependencies)" - @echo " test - run server tests (uses DB_DATABASE=$(TEST_DB))" + @echo " test - run server tests (uses $(TEST_DB_ENV))" @echo " migrate - run database migrations (alembic upgrade head)" @echo " alembic-migrate MSG='message' - autogenerate alembic revision" @echo " alembic-upgrade UP=head - upgrade to revision" @@ -63,7 +64,7 @@ start-dependencies: @echo "PostgreSQL is ready!" test: - DB_DATABASE=$(TEST_DB) uv run --package agent-control-server pytest --cov=src --cov-report=xml:../coverage-server.xml -q + $(TEST_DB_ENV) uv run --package agent-control-server pytest --cov=src --cov-report=xml:../coverage-server.xml -q run: start-dependencies migrate uv run --package agent-control-server uvicorn agent_control_server.main:app --reload diff --git a/server/README.md b/server/README.md index 482f9034..3995c05a 100644 --- a/server/README.md +++ b/server/README.md @@ -25,4 +25,9 @@ Server runs on http://localhost:8000. The UI expects this base URL by default. Server configuration is driven by environment variables (database, auth, observability, evaluators). For the full list and examples, see the docs. +Server-owned configuration now uses `AGENT_CONTROL_`-prefixed environment variables. +Examples: `AGENT_CONTROL_DB_URL`, `AGENT_CONTROL_HOST`, `AGENT_CONTROL_PORT`, +`AGENT_CONTROL_CORS_ORIGINS`, `AGENT_CONTROL_LOG_LEVEL`, +`AGENT_CONTROL_OBSERVABILITY_ENABLED`. + Full guide: https://docs.agentcontrol.dev/components/server diff --git a/server/src/agent_control_server/config.py b/server/src/agent_control_server/config.py index 4abdecea..aaa5dba1 100644 --- a/server/src/agent_control_server/config.py +++ b/server/src/agent_control_server/config.py @@ -1,24 +1,27 @@ """Server configuration settings.""" + import logging -import os import secrets from functools import cached_property +from pydantic import AliasChoices, Field from pydantic_settings import BaseSettings, SettingsConfigDict _config_logger = logging.getLogger(__name__) +_COMMON_SETTINGS_CONFIG = SettingsConfigDict( + env_file=".env", + env_file_encoding="utf-8", + case_sensitive=False, + extra="ignore", + populate_by_name=True, +) + class AuthSettings(BaseSettings): """Authentication configuration for API key validation.""" - model_config = SettingsConfigDict( - env_file=".env", - env_file_encoding="utf-8", - case_sensitive=False, - extra="ignore", - env_prefix="AGENT_CONTROL_", - ) + model_config = SettingsConfigDict(**_COMMON_SETTINGS_CONFIG, env_prefix="AGENT_CONTROL_") # Master toggle for authentication (disabled by default for local development) # Enable in production: AGENT_CONTROL_API_KEY_ENABLED=true @@ -91,32 +94,55 @@ def get_session_secret(self) -> str: class AgentControlServerDatabaseConfig(BaseSettings): - model_config = SettingsConfigDict( - env_file=".env", - env_file_encoding="utf-8", - case_sensitive=False, - env_prefix="DB_", - extra="ignore", # Ignore extra fields in .env - ) + """Database configuration for the server. + + Canonical environment variables use the ``AGENT_CONTROL_DB_`` prefix. + Legacy ``DATABASE_URL`` / ``DB_*`` names are still accepted for + backwards compatibility. + """ + + model_config = _COMMON_SETTINGS_CONFIG # Allow direct URL override for SQLite in local dev - url: str | None = None + url: str | None = Field( + default=None, + validation_alias=AliasChoices( + "AGENT_CONTROL_DB_URL", + "DATABASE_URL", + "DB_URL", + ), + ) # PostgreSQL settings (only used if url is not set) - host: str = "localhost" - port: int = 5432 - user: str = "agent_control" - password: str = "agent_control" - database: str = "agent_control" - driver: str = "psycopg" + host: str = Field( + default="localhost", + validation_alias=AliasChoices("AGENT_CONTROL_DB_HOST", "DB_HOST"), + ) + port: int = Field( + default=5432, + validation_alias=AliasChoices("AGENT_CONTROL_DB_PORT", "DB_PORT"), + ) + user: str = Field( + default="agent_control", + validation_alias=AliasChoices("AGENT_CONTROL_DB_USER", "DB_USER"), + ) + password: str = Field( + default="agent_control", + validation_alias=AliasChoices("AGENT_CONTROL_DB_PASSWORD", "DB_PASSWORD"), + ) + database: str = Field( + default="agent_control", + validation_alias=AliasChoices("AGENT_CONTROL_DB_DATABASE", "DB_DATABASE"), + ) + driver: str = Field( + default="psycopg", + validation_alias=AliasChoices("AGENT_CONTROL_DB_DRIVER", "DB_DRIVER"), + ) def get_url(self) -> str: - """Get database URL, preferring explicit url if set.""" - - # Check for DATABASE_URL first (Docker standard), then DB_URL - database_url = os.getenv('DATABASE_URL') or self.url - if database_url: - return database_url + """Get database URL, preferring an explicit URL if configured.""" + if self.url: + return self.url return ( f"postgresql+{self.driver}://{self.user}:{self.password}@{self.host}:{self.port}/{self.database}" ) @@ -125,67 +151,120 @@ def get_url(self) -> str: class Settings(BaseSettings): """Server configuration settings.""" - model_config = SettingsConfigDict( - env_file=".env", - env_file_encoding="utf-8", - case_sensitive=False, - extra="ignore", # Ignore extra fields in .env (like DB_* fields) - ) + model_config = _COMMON_SETTINGS_CONFIG # Server settings - host: str = "0.0.0.0" - port: int = 8000 - debug: bool = False + host: str = Field( + default="0.0.0.0", + validation_alias=AliasChoices("AGENT_CONTROL_HOST", "HOST"), + ) + port: int = Field( + default=8000, + validation_alias=AliasChoices("AGENT_CONTROL_PORT", "PORT"), + ) + debug: bool = Field( + default=False, + validation_alias=AliasChoices("AGENT_CONTROL_DEBUG", "DEBUG"), + ) # API settings - api_version: str = "v1" - api_prefix: str = "/api" + api_version: str = Field( + default="v1", + validation_alias=AliasChoices("AGENT_CONTROL_API_VERSION", "API_VERSION"), + ) + api_prefix: str = Field( + default="/api", + validation_alias=AliasChoices("AGENT_CONTROL_API_PREFIX", "API_PREFIX"), + ) # Prometheus metrics settings - prometheus_metrics_prefix: str = "agent_control_server" + prometheus_metrics_prefix: str = Field( + default="agent_control_server", + validation_alias=AliasChoices( + "AGENT_CONTROL_PROMETHEUS_METRICS_PREFIX", + "PROMETHEUS_METRICS_PREFIX", + ), + ) # CORS settings - cors_origins: list[str] | str = "*" - allow_methods: list[str] = ["*"] - allow_headers: list[str] = ["*"] + cors_origins: list[str] | str = Field( + default="*", + validation_alias=AliasChoices("AGENT_CONTROL_CORS_ORIGINS", "CORS_ORIGINS"), + ) + allow_methods: list[str] | str = Field( + default=["*"], + validation_alias=AliasChoices("AGENT_CONTROL_ALLOW_METHODS", "ALLOW_METHODS"), + ) + allow_headers: list[str] | str = Field( + default=["*"], + validation_alias=AliasChoices("AGENT_CONTROL_ALLOW_HEADERS", "ALLOW_HEADERS"), + ) def get_cors_origins(self) -> list[str]: """Parse CORS origins from string or list.""" - if isinstance(self.cors_origins, str): - if self.cors_origins == "*": + return self._parse_list_setting(self.cors_origins) + + def get_allow_methods(self) -> list[str]: + """Parse allow_methods from string or list.""" + return self._parse_list_setting(self.allow_methods) + + def get_allow_headers(self) -> list[str]: + """Parse allow_headers from string or list.""" + return self._parse_list_setting(self.allow_headers) + + @staticmethod + def _parse_list_setting(value: list[str] | str) -> list[str]: + """Parse wildcard/comma-separated settings from string or list.""" + if isinstance(value, str): + if value == "*": return ["*"] - return [origin.strip() for origin in self.cors_origins.split(",")] - return self.cors_origins + return [item.strip() for item in value.split(",") if item.strip()] + return value class ObservabilitySettings(BaseSettings): """Observability configuration settings.""" - model_config = SettingsConfigDict( - env_file=".env", - env_file_encoding="utf-8", - case_sensitive=False, - env_prefix="OBSERVABILITY_", - extra="ignore", - ) + model_config = _COMMON_SETTINGS_CONFIG # Enable/disable observability features - enabled: bool = True + enabled: bool = Field( + default=True, + validation_alias=AliasChoices( + "AGENT_CONTROL_OBSERVABILITY_ENABLED", + "OBSERVABILITY_ENABLED", + ), + ) # Stdout logging of events - stdout: bool = False + stdout: bool = Field( + default=False, + validation_alias=AliasChoices( + "AGENT_CONTROL_OBSERVABILITY_STDOUT", + "OBSERVABILITY_STDOUT", + ), + ) + + +class LoggingSettings(BaseSettings): + """Server logging configuration settings.""" + + model_config = _COMMON_SETTINGS_CONFIG + + level: str | None = Field( + default=None, + validation_alias=AliasChoices("AGENT_CONTROL_LOG_LEVEL", "LOG_LEVEL"), + ) + json_logs: bool = Field( + default=False, + validation_alias=AliasChoices("AGENT_CONTROL_LOG_JSON", "LOG_JSON"), + ) class UISettings(BaseSettings): """Static UI hosting configuration settings.""" - model_config = SettingsConfigDict( - env_file=".env", - env_file_encoding="utf-8", - case_sensitive=False, - env_prefix="AGENT_CONTROL_UI_", - extra="ignore", - ) + model_config = SettingsConfigDict(**_COMMON_SETTINGS_CONFIG, env_prefix="AGENT_CONTROL_UI_") dist_dir: str | None = None @@ -194,4 +273,5 @@ class UISettings(BaseSettings): db_config = AgentControlServerDatabaseConfig() settings = Settings() observability_settings = ObservabilitySettings() +logging_settings = LoggingSettings() ui_settings = UISettings() diff --git a/server/src/agent_control_server/logging_utils.py b/server/src/agent_control_server/logging_utils.py index 13722f81..2660d96a 100644 --- a/server/src/agent_control_server/logging_utils.py +++ b/server/src/agent_control_server/logging_utils.py @@ -1,5 +1,6 @@ import logging -import os + +from .config import LoggingSettings _LEVELS = { "CRITICAL": logging.CRITICAL, @@ -16,19 +17,40 @@ def _parse_level(level: str | int | None) -> int: return level if isinstance(level, str): return _LEVELS.get(level.upper(), logging.INFO) - env = os.getenv("LOG_LEVEL", "INFO") - return _LEVELS.get(env.upper(), logging.INFO) + configured_level = LoggingSettings().level + if configured_level is not None: + return _LEVELS.get(configured_level.upper(), logging.INFO) + return logging.INFO + + +def get_log_level_name(default_level: str = "INFO") -> str: + """Resolve the configured log level name, falling back to the provided default.""" + configured_level = LoggingSettings().level + if configured_level is not None: + normalized = configured_level.upper() + if normalized in _LEVELS: + return normalized + return "INFO" + normalized_default = default_level.upper() + if normalized_default in _LEVELS: + return normalized_default + return "INFO" def _parse_json(json_flag: bool | None) -> bool: if isinstance(json_flag, bool): return json_flag - env = os.getenv("LOG_JSON", "false").lower() - return env in {"1", "true", "yes", "y"} + return LoggingSettings().json_logs -def configure_logging(*, level: str | int | None = None, json: bool | None = None) -> None: - lvl = _parse_level(level) +def configure_logging( + *, + level: str | int | None = None, + json: bool | None = None, + default_level: str = "INFO", +) -> None: + resolved_level = level if level is not None else get_log_level_name(default_level) + lvl = _parse_level(resolved_level) as_json = _parse_json(json) fmt = ( '{"time":"%(asctime)s","level":"%(levelname)s","name":"%(name)s","msg":"%(message)s"}' diff --git a/server/src/agent_control_server/main.py b/server/src/agent_control_server/main.py index 27509762..94a806de 100644 --- a/server/src/agent_control_server/main.py +++ b/server/src/agent_control_server/main.py @@ -32,7 +32,7 @@ http_exception_handler, validation_exception_handler, ) -from .logging_utils import configure_logging +from .logging_utils import configure_logging, get_log_level_name from .observability.ingest import DirectEventIngestor from .observability.store import PostgresEventStore from .ui_assets import configure_ui_routes @@ -78,8 +78,8 @@ def add_prometheus_metrics(app: FastAPI, metrics_prefix: str) -> None: async def lifespan(app: FastAPI) -> AsyncGenerator[None, None]: """Lifespan context manager for FastAPI app startup and shutdown.""" # Startup: Configure logging - log_level = "DEBUG" if settings.debug else "INFO" - configure_logging(level=log_level) + default_log_level = "DEBUG" if settings.debug else "INFO" + configure_logging(default_level=default_log_level) # Discover evaluators at startup discover_evaluators() @@ -152,13 +152,13 @@ async def lifespan(app: FastAPI) -> AsyncGenerator[None, None]: CORSMiddleware, allow_origins=settings.get_cors_origins(), allow_credentials=True, - allow_methods=settings.allow_methods, - allow_headers=settings.allow_headers, + allow_methods=settings.get_allow_methods(), + allow_headers=settings.get_allow_headers(), ) # Configure logging -log_level = "DEBUG" if settings.debug else "INFO" -configure_logging(level=log_level) +default_log_level = "DEBUG" if settings.debug else "INFO" +configure_logging(default_level=default_log_level) # ============================================================================= @@ -272,11 +272,12 @@ async def health_check() -> HealthResponse: def run() -> None: """Run the server application.""" + default_log_level = "DEBUG" if settings.debug else "INFO" uvicorn.run( app, host=settings.host, port=settings.port, - log_level="debug" if settings.debug else "info", + log_level=get_log_level_name(default_log_level).lower(), ) diff --git a/server/tests/test_config.py b/server/tests/test_config.py index 92a263cf..3dc736cc 100644 --- a/server/tests/test_config.py +++ b/server/tests/test_config.py @@ -1,6 +1,10 @@ """Tests for server configuration helpers.""" -from agent_control_server.config import AgentControlServerDatabaseConfig, Settings +from agent_control_server.config import ( + AgentControlServerDatabaseConfig, + ObservabilitySettings, + Settings, +) def test_db_config_prefers_explicit_url() -> None: @@ -15,6 +19,29 @@ def test_db_config_prefers_explicit_url() -> None: assert resolved == explicit_url +def test_db_config_prefers_agent_control_url_over_legacy_env(monkeypatch) -> None: + # Given: both canonical and legacy database URL env vars are set + monkeypatch.setenv("AGENT_CONTROL_DB_URL", "sqlite:///tmp/canonical.db") + monkeypatch.setenv("DATABASE_URL", "sqlite:///tmp/legacy.db") + + # When: loading DB config from the environment + config = AgentControlServerDatabaseConfig() + + # Then: the canonical Agent Control env var wins + assert config.get_url() == "sqlite:///tmp/canonical.db" + + +def test_db_config_supports_legacy_db_database_env(monkeypatch) -> None: + # Given: only the legacy DB_DATABASE env var is set + monkeypatch.setenv("DB_DATABASE", "agent_control_legacy") + + # When: loading DB config from the environment + config = AgentControlServerDatabaseConfig() + + # Then: the legacy value is still honored + assert config.database == "agent_control_legacy" + + def test_settings_parses_cors_origins_string() -> None: # Given: a comma-separated CORS origins string settings = Settings(cors_origins="https://a.example, https://b.example") @@ -26,6 +53,23 @@ def test_settings_parses_cors_origins_string() -> None: assert origins == ["https://a.example", "https://b.example"] +def test_settings_reads_agent_control_prefixed_env_vars(monkeypatch) -> None: + # Given: canonical Agent Control server env vars are set + monkeypatch.setenv("AGENT_CONTROL_HOST", "127.0.0.1") + monkeypatch.setenv("AGENT_CONTROL_CORS_ORIGINS", "https://a.example, https://b.example") + monkeypatch.setenv("AGENT_CONTROL_ALLOW_METHODS", "GET, POST") + monkeypatch.setenv("AGENT_CONTROL_ALLOW_HEADERS", "Authorization, Content-Type") + + # When: loading settings from the environment + config = Settings() + + # Then: the canonical env vars are parsed correctly + assert config.host == "127.0.0.1" + assert config.get_cors_origins() == ["https://a.example", "https://b.example"] + assert config.get_allow_methods() == ["GET", "POST"] + assert config.get_allow_headers() == ["Authorization", "Content-Type"] + + def test_settings_returns_cors_origins_list_unchanged() -> None: # Given: a CORS origins list settings = Settings(cors_origins=["https://a.example", "https://b.example"]) @@ -35,3 +79,16 @@ def test_settings_returns_cors_origins_list_unchanged() -> None: # Then: the list is returned as-is assert origins == ["https://a.example", "https://b.example"] + + +def test_observability_settings_support_prefixed_env_vars(monkeypatch) -> None: + # Given: canonical observability env vars are set + monkeypatch.setenv("AGENT_CONTROL_OBSERVABILITY_ENABLED", "false") + monkeypatch.setenv("AGENT_CONTROL_OBSERVABILITY_STDOUT", "true") + + # When: loading observability settings from the environment + config = ObservabilitySettings() + + # Then: the Agent Control-prefixed env vars are used + assert config.enabled is False + assert config.stdout is True diff --git a/server/tests/test_logging_utils.py b/server/tests/test_logging_utils.py index 43fd5c54..405a4ccb 100644 --- a/server/tests/test_logging_utils.py +++ b/server/tests/test_logging_utils.py @@ -17,8 +17,8 @@ def test_parse_level_accepts_int() -> None: def test_parse_level_uses_env_default(monkeypatch) -> None: - # Given: LOG_LEVEL set in the environment - monkeypatch.setenv("LOG_LEVEL", "ERROR") + # Given: AGENT_CONTROL_LOG_LEVEL set in the environment + monkeypatch.setenv("AGENT_CONTROL_LOG_LEVEL", "ERROR") # When: parsing with no explicit level parsed = _parse_level(None) @@ -27,6 +27,18 @@ def test_parse_level_uses_env_default(monkeypatch) -> None: assert parsed == logging.ERROR +def test_parse_level_prefers_canonical_env_over_legacy(monkeypatch) -> None: + # Given: both canonical and legacy logging env vars are set + monkeypatch.setenv("AGENT_CONTROL_LOG_LEVEL", "WARNING") + monkeypatch.setenv("LOG_LEVEL", "ERROR") + + # When: parsing with no explicit level + parsed = _parse_level(None) + + # Then: the canonical env var wins + assert parsed == logging.WARNING + + def test_parse_json_accepts_bool() -> None: # Given: an explicit JSON flag flag = True @@ -38,6 +50,17 @@ def test_parse_json_accepts_bool() -> None: assert parsed is True +def test_parse_json_uses_canonical_env_default(monkeypatch) -> None: + # Given: AGENT_CONTROL_LOG_JSON is enabled + monkeypatch.setenv("AGENT_CONTROL_LOG_JSON", "true") + + # When: parsing with no explicit flag + parsed = _parse_json(None) + + # Then: the canonical env var is used + assert parsed is True + + def test_configure_logging_resets_uvicorn_handlers() -> None: # Given: a uvicorn logger with a custom handler logger = logging.getLogger("uvicorn") From 737eb02ffac25e18c15bf099c6bc9cefcbb0007b Mon Sep 17 00:00:00 2001 From: Lev Neiman Date: Fri, 13 Mar 2026 22:00:12 -0700 Subject: [PATCH 2/7] refactor: remove legacy server env aliases --- server/src/agent_control_server/config.py | 131 +++++----------------- server/tests/test_config.py | 30 +++-- server/tests/test_logging_utils.py | 9 +- 3 files changed, 54 insertions(+), 116 deletions(-) diff --git a/server/src/agent_control_server/config.py b/server/src/agent_control_server/config.py index aaa5dba1..f4bdd1c3 100644 --- a/server/src/agent_control_server/config.py +++ b/server/src/agent_control_server/config.py @@ -4,7 +4,7 @@ import secrets from functools import cached_property -from pydantic import AliasChoices, Field +from pydantic import Field from pydantic_settings import BaseSettings, SettingsConfigDict _config_logger = logging.getLogger(__name__) @@ -94,50 +94,20 @@ def get_session_secret(self) -> str: class AgentControlServerDatabaseConfig(BaseSettings): - """Database configuration for the server. + """Database configuration for the server.""" - Canonical environment variables use the ``AGENT_CONTROL_DB_`` prefix. - Legacy ``DATABASE_URL`` / ``DB_*`` names are still accepted for - backwards compatibility. - """ - - model_config = _COMMON_SETTINGS_CONFIG + model_config = SettingsConfigDict(**_COMMON_SETTINGS_CONFIG, env_prefix="AGENT_CONTROL_DB_") # Allow direct URL override for SQLite in local dev - url: str | None = Field( - default=None, - validation_alias=AliasChoices( - "AGENT_CONTROL_DB_URL", - "DATABASE_URL", - "DB_URL", - ), - ) + url: str | None = None # PostgreSQL settings (only used if url is not set) - host: str = Field( - default="localhost", - validation_alias=AliasChoices("AGENT_CONTROL_DB_HOST", "DB_HOST"), - ) - port: int = Field( - default=5432, - validation_alias=AliasChoices("AGENT_CONTROL_DB_PORT", "DB_PORT"), - ) - user: str = Field( - default="agent_control", - validation_alias=AliasChoices("AGENT_CONTROL_DB_USER", "DB_USER"), - ) - password: str = Field( - default="agent_control", - validation_alias=AliasChoices("AGENT_CONTROL_DB_PASSWORD", "DB_PASSWORD"), - ) - database: str = Field( - default="agent_control", - validation_alias=AliasChoices("AGENT_CONTROL_DB_DATABASE", "DB_DATABASE"), - ) - driver: str = Field( - default="psycopg", - validation_alias=AliasChoices("AGENT_CONTROL_DB_DRIVER", "DB_DRIVER"), - ) + host: str = "localhost" + port: int = 5432 + user: str = "agent_control" + password: str = "agent_control" + database: str = "agent_control" + driver: str = "psycopg" def get_url(self) -> str: """Get database URL, preferring an explicit URL if configured.""" @@ -151,54 +121,24 @@ def get_url(self) -> str: class Settings(BaseSettings): """Server configuration settings.""" - model_config = _COMMON_SETTINGS_CONFIG + model_config = SettingsConfigDict(**_COMMON_SETTINGS_CONFIG, env_prefix="AGENT_CONTROL_") # Server settings - host: str = Field( - default="0.0.0.0", - validation_alias=AliasChoices("AGENT_CONTROL_HOST", "HOST"), - ) - port: int = Field( - default=8000, - validation_alias=AliasChoices("AGENT_CONTROL_PORT", "PORT"), - ) - debug: bool = Field( - default=False, - validation_alias=AliasChoices("AGENT_CONTROL_DEBUG", "DEBUG"), - ) + host: str = "0.0.0.0" + port: int = 8000 + debug: bool = False # API settings - api_version: str = Field( - default="v1", - validation_alias=AliasChoices("AGENT_CONTROL_API_VERSION", "API_VERSION"), - ) - api_prefix: str = Field( - default="/api", - validation_alias=AliasChoices("AGENT_CONTROL_API_PREFIX", "API_PREFIX"), - ) + api_version: str = "v1" + api_prefix: str = "/api" # Prometheus metrics settings - prometheus_metrics_prefix: str = Field( - default="agent_control_server", - validation_alias=AliasChoices( - "AGENT_CONTROL_PROMETHEUS_METRICS_PREFIX", - "PROMETHEUS_METRICS_PREFIX", - ), - ) + prometheus_metrics_prefix: str = "agent_control_server" # CORS settings - cors_origins: list[str] | str = Field( - default="*", - validation_alias=AliasChoices("AGENT_CONTROL_CORS_ORIGINS", "CORS_ORIGINS"), - ) - allow_methods: list[str] | str = Field( - default=["*"], - validation_alias=AliasChoices("AGENT_CONTROL_ALLOW_METHODS", "ALLOW_METHODS"), - ) - allow_headers: list[str] | str = Field( - default=["*"], - validation_alias=AliasChoices("AGENT_CONTROL_ALLOW_HEADERS", "ALLOW_HEADERS"), - ) + cors_origins: list[str] | str = "*" + allow_methods: list[str] | str = ["*"] + allow_headers: list[str] | str = ["*"] def get_cors_origins(self) -> list[str]: """Parse CORS origins from string or list.""" @@ -225,39 +165,27 @@ def _parse_list_setting(value: list[str] | str) -> list[str]: class ObservabilitySettings(BaseSettings): """Observability configuration settings.""" - model_config = _COMMON_SETTINGS_CONFIG + model_config = SettingsConfigDict( + **_COMMON_SETTINGS_CONFIG, + env_prefix="AGENT_CONTROL_OBSERVABILITY_", + ) # Enable/disable observability features - enabled: bool = Field( - default=True, - validation_alias=AliasChoices( - "AGENT_CONTROL_OBSERVABILITY_ENABLED", - "OBSERVABILITY_ENABLED", - ), - ) + enabled: bool = True # Stdout logging of events - stdout: bool = Field( - default=False, - validation_alias=AliasChoices( - "AGENT_CONTROL_OBSERVABILITY_STDOUT", - "OBSERVABILITY_STDOUT", - ), - ) + stdout: bool = False class LoggingSettings(BaseSettings): """Server logging configuration settings.""" - model_config = _COMMON_SETTINGS_CONFIG + model_config = SettingsConfigDict(**_COMMON_SETTINGS_CONFIG, env_prefix="AGENT_CONTROL_LOG_") - level: str | None = Field( - default=None, - validation_alias=AliasChoices("AGENT_CONTROL_LOG_LEVEL", "LOG_LEVEL"), - ) + level: str | None = None json_logs: bool = Field( default=False, - validation_alias=AliasChoices("AGENT_CONTROL_LOG_JSON", "LOG_JSON"), + validation_alias="AGENT_CONTROL_LOG_JSON", ) @@ -273,5 +201,4 @@ class UISettings(BaseSettings): db_config = AgentControlServerDatabaseConfig() settings = Settings() observability_settings = ObservabilitySettings() -logging_settings = LoggingSettings() ui_settings = UISettings() diff --git a/server/tests/test_config.py b/server/tests/test_config.py index 3dc736cc..b96b69f4 100644 --- a/server/tests/test_config.py +++ b/server/tests/test_config.py @@ -19,27 +19,26 @@ def test_db_config_prefers_explicit_url() -> None: assert resolved == explicit_url -def test_db_config_prefers_agent_control_url_over_legacy_env(monkeypatch) -> None: - # Given: both canonical and legacy database URL env vars are set +def test_db_config_reads_agent_control_url_from_env(monkeypatch) -> None: + # Given: the canonical database URL env var is set monkeypatch.setenv("AGENT_CONTROL_DB_URL", "sqlite:///tmp/canonical.db") - monkeypatch.setenv("DATABASE_URL", "sqlite:///tmp/legacy.db") # When: loading DB config from the environment config = AgentControlServerDatabaseConfig() - # Then: the canonical Agent Control env var wins + # Then: the canonical Agent Control env var is used assert config.get_url() == "sqlite:///tmp/canonical.db" -def test_db_config_supports_legacy_db_database_env(monkeypatch) -> None: - # Given: only the legacy DB_DATABASE env var is set - monkeypatch.setenv("DB_DATABASE", "agent_control_legacy") +def test_db_config_ignores_legacy_database_env(monkeypatch) -> None: + # Given: only the legacy database URL env var is set + monkeypatch.setenv("DATABASE_URL", "sqlite:///tmp/legacy.db") # When: loading DB config from the environment config = AgentControlServerDatabaseConfig() - # Then: the legacy value is still honored - assert config.database == "agent_control_legacy" + # Then: the legacy env var is ignored + assert config.get_url() != "sqlite:///tmp/legacy.db" def test_settings_parses_cors_origins_string() -> None: @@ -92,3 +91,16 @@ def test_observability_settings_support_prefixed_env_vars(monkeypatch) -> None: # Then: the Agent Control-prefixed env vars are used assert config.enabled is False assert config.stdout is True + + +def test_observability_settings_ignore_legacy_env_vars(monkeypatch) -> None: + # Given: only legacy observability env vars are set + monkeypatch.setenv("OBSERVABILITY_ENABLED", "false") + monkeypatch.setenv("OBSERVABILITY_STDOUT", "true") + + # When: loading observability settings from the environment + config = ObservabilitySettings() + + # Then: the legacy env vars are ignored + assert config.enabled is True + assert config.stdout is False diff --git a/server/tests/test_logging_utils.py b/server/tests/test_logging_utils.py index 405a4ccb..d8d2a0eb 100644 --- a/server/tests/test_logging_utils.py +++ b/server/tests/test_logging_utils.py @@ -27,16 +27,15 @@ def test_parse_level_uses_env_default(monkeypatch) -> None: assert parsed == logging.ERROR -def test_parse_level_prefers_canonical_env_over_legacy(monkeypatch) -> None: - # Given: both canonical and legacy logging env vars are set - monkeypatch.setenv("AGENT_CONTROL_LOG_LEVEL", "WARNING") +def test_parse_level_ignores_legacy_env(monkeypatch) -> None: + # Given: only the legacy logging env var is set monkeypatch.setenv("LOG_LEVEL", "ERROR") # When: parsing with no explicit level parsed = _parse_level(None) - # Then: the canonical env var wins - assert parsed == logging.WARNING + # Then: the legacy env var is ignored + assert parsed == logging.INFO def test_parse_json_accepts_bool() -> None: From 9c61455bde1b5524d8b0674ea9158f0c729b3fa8 Mon Sep 17 00:00:00 2001 From: Lev Neiman Date: Fri, 13 Mar 2026 22:21:38 -0700 Subject: [PATCH 3/7] fix: preserve server env compatibility --- sdks/python/Makefile | 2 +- server/.env.example | 28 ++++++++--------- server/Makefile | 2 +- server/src/agent_control_server/config.py | 37 ++++++++++++++++++----- server/tests/test_config.py | 36 +++++++++++++++++++--- 5 files changed, 77 insertions(+), 28 deletions(-) diff --git a/sdks/python/Makefile b/sdks/python/Makefile index 1d2e62d0..95069765 100644 --- a/sdks/python/Makefile +++ b/sdks/python/Makefile @@ -1,7 +1,7 @@ .PHONY: help test lint lint-fix typecheck build publish TEST_DB ?= agent_control_test -TEST_DB_ENV := AGENT_CONTROL_DB_URL= AGENT_CONTROL_DB_HOST=localhost AGENT_CONTROL_DB_PORT=5432 AGENT_CONTROL_DB_USER=agent_control AGENT_CONTROL_DB_PASSWORD=agent_control AGENT_CONTROL_DB_DATABASE=$(TEST_DB) +TEST_DB_ENV := env -u AGENT_CONTROL_DB_URL -u DATABASE_URL -u DB_URL AGENT_CONTROL_DB_HOST=localhost AGENT_CONTROL_DB_PORT=5432 AGENT_CONTROL_DB_USER=agent_control AGENT_CONTROL_DB_PASSWORD=agent_control AGENT_CONTROL_DB_DATABASE=$(TEST_DB) AGENT_CONTROL_DB_DRIVER=psycopg help: @echo "Agent Control SDK - Makefile commands" diff --git a/server/.env.example b/server/.env.example index 04656433..79abf3bd 100644 --- a/server/.env.example +++ b/server/.env.example @@ -3,8 +3,8 @@ ############################### # Server host/port -HOST=0.0.0.0 -PORT=8000 +AGENT_CONTROL_HOST=0.0.0.0 +AGENT_CONTROL_PORT=8000 ################################ # API key authentication (auth) # @@ -35,15 +35,15 @@ AGENT_CONTROL_SESSION_SECRET="change-me-to-a-long-random-string" # set this to the exact UI origin so cookie-based auth works cross-origin. # In production (static UI served by FastAPI on the same origin), this can # usually be left as "*". -CORS_ORIGINS="http://localhost:4000" - -################# -# Database (DB_) # -################# - -# Example PostgreSQL settings (used when DATABASE_URL is not set) -DB_HOST=localhost -DB_PORT=5432 -DB_USER=agent_control -DB_PASSWORD=agent_control -DB_DATABASE=agent_control +AGENT_CONTROL_CORS_ORIGINS="http://localhost:4000" + +######################################### +# Database (preferred AGENT_CONTROL_DB_) # +######################################### + +# Example PostgreSQL settings (legacy DB_* and DATABASE_URL are still accepted) +AGENT_CONTROL_DB_HOST=localhost +AGENT_CONTROL_DB_PORT=5432 +AGENT_CONTROL_DB_USER=agent_control +AGENT_CONTROL_DB_PASSWORD=agent_control +AGENT_CONTROL_DB_DATABASE=agent_control diff --git a/server/Makefile b/server/Makefile index 5a7a233a..7091bf9f 100644 --- a/server/Makefile +++ b/server/Makefile @@ -9,7 +9,7 @@ SHOW ?= head STAMP ?= head TEST_DB ?= agent_control_test -TEST_DB_ENV := AGENT_CONTROL_DB_URL= AGENT_CONTROL_DB_HOST=localhost AGENT_CONTROL_DB_PORT=5432 AGENT_CONTROL_DB_USER=agent_control AGENT_CONTROL_DB_PASSWORD=agent_control AGENT_CONTROL_DB_DATABASE=$(TEST_DB) +TEST_DB_ENV := env -u AGENT_CONTROL_DB_URL -u DATABASE_URL -u DB_URL AGENT_CONTROL_DB_HOST=localhost AGENT_CONTROL_DB_PORT=5432 AGENT_CONTROL_DB_USER=agent_control AGENT_CONTROL_DB_PASSWORD=agent_control AGENT_CONTROL_DB_DATABASE=$(TEST_DB) AGENT_CONTROL_DB_DRIVER=psycopg .PHONY: help run start-dependencies test migrate alembic-migrate alembic-revision alembic-upgrade alembic-downgrade alembic-current alembic-history alembic-heads alembic-show alembic-stamp help: diff --git a/server/src/agent_control_server/config.py b/server/src/agent_control_server/config.py index f4bdd1c3..ad87c8c3 100644 --- a/server/src/agent_control_server/config.py +++ b/server/src/agent_control_server/config.py @@ -4,7 +4,7 @@ import secrets from functools import cached_property -from pydantic import Field +from pydantic import AliasChoices, Field from pydantic_settings import BaseSettings, SettingsConfigDict _config_logger = logging.getLogger(__name__) @@ -99,15 +99,36 @@ class AgentControlServerDatabaseConfig(BaseSettings): model_config = SettingsConfigDict(**_COMMON_SETTINGS_CONFIG, env_prefix="AGENT_CONTROL_DB_") # Allow direct URL override for SQLite in local dev - url: str | None = None + url: str | None = Field( + default=None, + validation_alias=AliasChoices("AGENT_CONTROL_DB_URL", "DATABASE_URL", "DB_URL"), + ) # PostgreSQL settings (only used if url is not set) - host: str = "localhost" - port: int = 5432 - user: str = "agent_control" - password: str = "agent_control" - database: str = "agent_control" - driver: str = "psycopg" + host: str = Field( + default="localhost", + validation_alias=AliasChoices("AGENT_CONTROL_DB_HOST", "DB_HOST"), + ) + port: int = Field( + default=5432, + validation_alias=AliasChoices("AGENT_CONTROL_DB_PORT", "DB_PORT"), + ) + user: str = Field( + default="agent_control", + validation_alias=AliasChoices("AGENT_CONTROL_DB_USER", "DB_USER"), + ) + password: str = Field( + default="agent_control", + validation_alias=AliasChoices("AGENT_CONTROL_DB_PASSWORD", "DB_PASSWORD"), + ) + database: str = Field( + default="agent_control", + validation_alias=AliasChoices("AGENT_CONTROL_DB_DATABASE", "DB_DATABASE"), + ) + driver: str = Field( + default="psycopg", + validation_alias=AliasChoices("AGENT_CONTROL_DB_DRIVER", "DB_DRIVER"), + ) def get_url(self) -> str: """Get database URL, preferring an explicit URL if configured.""" diff --git a/server/tests/test_config.py b/server/tests/test_config.py index b96b69f4..5ef6116a 100644 --- a/server/tests/test_config.py +++ b/server/tests/test_config.py @@ -30,15 +30,43 @@ def test_db_config_reads_agent_control_url_from_env(monkeypatch) -> None: assert config.get_url() == "sqlite:///tmp/canonical.db" -def test_db_config_ignores_legacy_database_env(monkeypatch) -> None: - # Given: only the legacy database URL env var is set +def test_db_config_reads_database_url_from_env(monkeypatch) -> None: + # Given: only the legacy DATABASE_URL env var is set monkeypatch.setenv("DATABASE_URL", "sqlite:///tmp/legacy.db") # When: loading DB config from the environment config = AgentControlServerDatabaseConfig() - # Then: the legacy env var is ignored - assert config.get_url() != "sqlite:///tmp/legacy.db" + # Then: the legacy env var is still supported during migration + assert config.get_url() == "sqlite:///tmp/legacy.db" + + +def test_db_config_reads_legacy_db_prefix_from_env(monkeypatch) -> None: + # Given: only the legacy DB_* env vars are set + monkeypatch.setenv("DB_HOST", "db.example") + monkeypatch.setenv("DB_PORT", "15432") + monkeypatch.setenv("DB_USER", "legacy_user") + monkeypatch.setenv("DB_PASSWORD", "legacy_password") + monkeypatch.setenv("DB_DATABASE", "legacy_db") + monkeypatch.setenv("DB_DRIVER", "psycopg") + + # When: loading DB config from the environment + config = AgentControlServerDatabaseConfig() + + # Then: the legacy env vars remain compatible + assert config.get_url() == "postgresql+psycopg://legacy_user:legacy_password@db.example:15432/legacy_db" + + +def test_db_config_prefers_agent_control_env_over_legacy(monkeypatch) -> None: + # Given: both canonical and legacy database URLs are present + monkeypatch.setenv("AGENT_CONTROL_DB_URL", "sqlite:///tmp/canonical.db") + monkeypatch.setenv("DATABASE_URL", "sqlite:///tmp/legacy.db") + + # When: loading DB config from the environment + config = AgentControlServerDatabaseConfig() + + # Then: the canonical env var wins + assert config.get_url() == "sqlite:///tmp/canonical.db" def test_settings_parses_cors_origins_string() -> None: From 2550ad342c17b29a9d4cfc69e12b27a51973e5e3 Mon Sep 17 00:00:00 2001 From: Lev Neiman Date: Fri, 13 Mar 2026 22:32:21 -0700 Subject: [PATCH 4/7] fix: preserve legacy server env compatibility --- server/src/agent_control_server/config.py | 72 ++++++++++++++++--- .../src/agent_control_server/logging_utils.py | 9 +-- server/tests/test_config.py | 49 +++++++++++++ server/tests/test_logging_utils.py | 40 ++++++++++- 4 files changed, 155 insertions(+), 15 deletions(-) diff --git a/server/src/agent_control_server/config.py b/server/src/agent_control_server/config.py index ad87c8c3..9bd31644 100644 --- a/server/src/agent_control_server/config.py +++ b/server/src/agent_control_server/config.py @@ -4,7 +4,7 @@ import secrets from functools import cached_property -from pydantic import AliasChoices, Field +from pydantic import AliasChoices, Field, field_validator from pydantic_settings import BaseSettings, SettingsConfigDict _config_logger = logging.getLogger(__name__) @@ -145,21 +145,51 @@ class Settings(BaseSettings): model_config = SettingsConfigDict(**_COMMON_SETTINGS_CONFIG, env_prefix="AGENT_CONTROL_") # Server settings - host: str = "0.0.0.0" - port: int = 8000 - debug: bool = False + host: str = Field( + default="0.0.0.0", + validation_alias=AliasChoices("AGENT_CONTROL_HOST", "HOST"), + ) + port: int = Field( + default=8000, + validation_alias=AliasChoices("AGENT_CONTROL_PORT", "PORT"), + ) + debug: bool = Field( + default=False, + validation_alias=AliasChoices("AGENT_CONTROL_DEBUG", "DEBUG"), + ) # API settings - api_version: str = "v1" - api_prefix: str = "/api" + api_version: str = Field( + default="v1", + validation_alias=AliasChoices("AGENT_CONTROL_API_VERSION", "API_VERSION"), + ) + api_prefix: str = Field( + default="/api", + validation_alias=AliasChoices("AGENT_CONTROL_API_PREFIX", "API_PREFIX"), + ) # Prometheus metrics settings - prometheus_metrics_prefix: str = "agent_control_server" + prometheus_metrics_prefix: str = Field( + default="agent_control_server", + validation_alias=AliasChoices( + "AGENT_CONTROL_PROMETHEUS_METRICS_PREFIX", + "PROMETHEUS_METRICS_PREFIX", + ), + ) # CORS settings - cors_origins: list[str] | str = "*" - allow_methods: list[str] | str = ["*"] - allow_headers: list[str] | str = ["*"] + cors_origins: list[str] | str = Field( + default="*", + validation_alias=AliasChoices("AGENT_CONTROL_CORS_ORIGINS", "CORS_ORIGINS"), + ) + allow_methods: list[str] | str = Field( + default=["*"], + validation_alias=AliasChoices("AGENT_CONTROL_ALLOW_METHODS", "ALLOW_METHODS"), + ) + allow_headers: list[str] | str = Field( + default=["*"], + validation_alias=AliasChoices("AGENT_CONTROL_ALLOW_HEADERS", "ALLOW_HEADERS"), + ) def get_cors_origins(self) -> list[str]: """Parse CORS origins from string or list.""" @@ -209,6 +239,28 @@ class LoggingSettings(BaseSettings): validation_alias="AGENT_CONTROL_LOG_JSON", ) + @field_validator("level", mode="before") + @classmethod + def _normalize_level(cls, value: object) -> object: + """Treat blank strings as unset and trim whitespace from configured levels.""" + if isinstance(value, str): + stripped = value.strip() + if not stripped: + return None + return stripped + return value + + @field_validator("json_logs", mode="before") + @classmethod + def _normalize_json_logs(cls, value: object) -> object: + """Treat blank strings as false and trim whitespace before bool parsing.""" + if isinstance(value, str): + stripped = value.strip() + if not stripped: + return False + return stripped + return value + class UISettings(BaseSettings): """Static UI hosting configuration settings.""" diff --git a/server/src/agent_control_server/logging_utils.py b/server/src/agent_control_server/logging_utils.py index 2660d96a..ade03a24 100644 --- a/server/src/agent_control_server/logging_utils.py +++ b/server/src/agent_control_server/logging_utils.py @@ -25,16 +25,17 @@ def _parse_level(level: str | int | None) -> int: def get_log_level_name(default_level: str = "INFO") -> str: """Resolve the configured log level name, falling back to the provided default.""" + normalized_default = default_level.upper() + if normalized_default not in _LEVELS: + normalized_default = "INFO" + configured_level = LoggingSettings().level if configured_level is not None: normalized = configured_level.upper() if normalized in _LEVELS: return normalized - return "INFO" - normalized_default = default_level.upper() - if normalized_default in _LEVELS: return normalized_default - return "INFO" + return normalized_default def _parse_json(json_flag: bool | None) -> bool: diff --git a/server/tests/test_config.py b/server/tests/test_config.py index 5ef6116a..c53d68d0 100644 --- a/server/tests/test_config.py +++ b/server/tests/test_config.py @@ -32,6 +32,7 @@ def test_db_config_reads_agent_control_url_from_env(monkeypatch) -> None: def test_db_config_reads_database_url_from_env(monkeypatch) -> None: # Given: only the legacy DATABASE_URL env var is set + monkeypatch.delenv("AGENT_CONTROL_DB_URL", raising=False) monkeypatch.setenv("DATABASE_URL", "sqlite:///tmp/legacy.db") # When: loading DB config from the environment @@ -43,6 +44,12 @@ def test_db_config_reads_database_url_from_env(monkeypatch) -> None: def test_db_config_reads_legacy_db_prefix_from_env(monkeypatch) -> None: # Given: only the legacy DB_* env vars are set + monkeypatch.delenv("AGENT_CONTROL_DB_HOST", raising=False) + monkeypatch.delenv("AGENT_CONTROL_DB_PORT", raising=False) + monkeypatch.delenv("AGENT_CONTROL_DB_USER", raising=False) + monkeypatch.delenv("AGENT_CONTROL_DB_PASSWORD", raising=False) + monkeypatch.delenv("AGENT_CONTROL_DB_DATABASE", raising=False) + monkeypatch.delenv("AGENT_CONTROL_DB_DRIVER", raising=False) monkeypatch.setenv("DB_HOST", "db.example") monkeypatch.setenv("DB_PORT", "15432") monkeypatch.setenv("DB_USER", "legacy_user") @@ -97,6 +104,48 @@ def test_settings_reads_agent_control_prefixed_env_vars(monkeypatch) -> None: assert config.get_allow_headers() == ["Authorization", "Content-Type"] +def test_settings_reads_legacy_env_vars(monkeypatch) -> None: + # Given: only legacy server env vars are set + monkeypatch.setenv("HOST", "127.0.0.1") + monkeypatch.setenv("PORT", "9000") + monkeypatch.setenv("DEBUG", "true") + monkeypatch.setenv("API_VERSION", "v2") + monkeypatch.setenv("API_PREFIX", "/legacy") + monkeypatch.setenv("PROMETHEUS_METRICS_PREFIX", "legacy_metrics") + monkeypatch.setenv("CORS_ORIGINS", "https://legacy.example") + monkeypatch.setenv("ALLOW_METHODS", "GET, POST") + monkeypatch.setenv("ALLOW_HEADERS", "Authorization, Content-Type") + + # When: loading settings from the environment + config = Settings() + + # Then: the legacy env vars remain compatible + assert config.host == "127.0.0.1" + assert config.port == 9000 + assert config.debug is True + assert config.api_version == "v2" + assert config.api_prefix == "/legacy" + assert config.prometheus_metrics_prefix == "legacy_metrics" + assert config.get_cors_origins() == ["https://legacy.example"] + assert config.get_allow_methods() == ["GET", "POST"] + assert config.get_allow_headers() == ["Authorization", "Content-Type"] + + +def test_settings_prefers_agent_control_env_vars_over_legacy(monkeypatch) -> None: + # Given: both canonical and legacy server env vars are set + monkeypatch.setenv("AGENT_CONTROL_PORT", "7000") + monkeypatch.setenv("PORT", "9000") + monkeypatch.setenv("AGENT_CONTROL_CORS_ORIGINS", "https://canonical.example") + monkeypatch.setenv("CORS_ORIGINS", "https://legacy.example") + + # When: loading settings from the environment + config = Settings() + + # Then: the canonical env vars win + assert config.port == 7000 + assert config.get_cors_origins() == ["https://canonical.example"] + + def test_settings_returns_cors_origins_list_unchanged() -> None: # Given: a CORS origins list settings = Settings(cors_origins=["https://a.example", "https://b.example"]) diff --git a/server/tests/test_logging_utils.py b/server/tests/test_logging_utils.py index d8d2a0eb..033fe27d 100644 --- a/server/tests/test_logging_utils.py +++ b/server/tests/test_logging_utils.py @@ -2,7 +2,12 @@ import logging -from agent_control_server.logging_utils import _parse_json, _parse_level, configure_logging +from agent_control_server.logging_utils import ( + _parse_json, + _parse_level, + configure_logging, + get_log_level_name, +) def test_parse_level_accepts_int() -> None: @@ -60,6 +65,39 @@ def test_parse_json_uses_canonical_env_default(monkeypatch) -> None: assert parsed is True +def test_parse_json_treats_blank_env_value_as_false(monkeypatch) -> None: + # Given: AGENT_CONTROL_LOG_JSON is declared but blank + monkeypatch.setenv("AGENT_CONTROL_LOG_JSON", "") + + # When: parsing with no explicit flag + parsed = _parse_json(None) + + # Then: the blank env var is treated as false instead of raising + assert parsed is False + + +def test_get_log_level_name_falls_back_to_default_for_invalid_env(monkeypatch) -> None: + # Given: AGENT_CONTROL_LOG_LEVEL is present but invalid + monkeypatch.setenv("AGENT_CONTROL_LOG_LEVEL", "not-a-level") + + # When: resolving the log level with a DEBUG default + resolved = get_log_level_name("DEBUG") + + # Then: the provided default is used + assert resolved == "DEBUG" + + +def test_get_log_level_name_treats_blank_env_as_unset(monkeypatch) -> None: + # Given: AGENT_CONTROL_LOG_LEVEL is declared but blank + monkeypatch.setenv("AGENT_CONTROL_LOG_LEVEL", "") + + # When: resolving the log level with a DEBUG default + resolved = get_log_level_name("DEBUG") + + # Then: the provided default is used + assert resolved == "DEBUG" + + def test_configure_logging_resets_uvicorn_handlers() -> None: # Given: a uvicorn logger with a custom handler logger = logging.getLogger("uvicorn") From 9dbabe73055b89074c9e7816b24b9d1230227dff Mon Sep 17 00:00:00 2001 From: Lev Neiman Date: Fri, 13 Mar 2026 22:33:46 -0700 Subject: [PATCH 5/7] docs: remove stale server env note --- server/README.md | 5 ----- 1 file changed, 5 deletions(-) diff --git a/server/README.md b/server/README.md index 3995c05a..482f9034 100644 --- a/server/README.md +++ b/server/README.md @@ -25,9 +25,4 @@ Server runs on http://localhost:8000. The UI expects this base URL by default. Server configuration is driven by environment variables (database, auth, observability, evaluators). For the full list and examples, see the docs. -Server-owned configuration now uses `AGENT_CONTROL_`-prefixed environment variables. -Examples: `AGENT_CONTROL_DB_URL`, `AGENT_CONTROL_HOST`, `AGENT_CONTROL_PORT`, -`AGENT_CONTROL_CORS_ORIGINS`, `AGENT_CONTROL_LOG_LEVEL`, -`AGENT_CONTROL_OBSERVABILITY_ENABLED`. - Full guide: https://docs.agentcontrol.dev/components/server From 7d7bdd961d2d88406b2e0319594a8c1017d3817f Mon Sep 17 00:00:00 2001 From: Lev Neiman Date: Fri, 13 Mar 2026 23:14:05 -0700 Subject: [PATCH 6/7] fix: simplify server env compatibility --- server/src/agent_control_server/config.py | 131 ++++++------------ .../src/agent_control_server/logging_utils.py | 48 +++++-- server/src/agent_control_server/main.py | 15 +- server/tests/test_config.py | 24 ++++ server/tests/test_logging_utils.py | 12 ++ 5 files changed, 122 insertions(+), 108 deletions(-) diff --git a/server/src/agent_control_server/config.py b/server/src/agent_control_server/config.py index 9bd31644..63b7e78a 100644 --- a/server/src/agent_control_server/config.py +++ b/server/src/agent_control_server/config.py @@ -3,8 +3,9 @@ import logging import secrets from functools import cached_property +from typing import Any -from pydantic import AliasChoices, Field, field_validator +from pydantic import AliasChoices, Field from pydantic_settings import BaseSettings, SettingsConfigDict _config_logger = logging.getLogger(__name__) @@ -13,11 +14,17 @@ env_file=".env", env_file_encoding="utf-8", case_sensitive=False, + env_ignore_empty=True, extra="ignore", populate_by_name=True, ) +def _env_alias_field(default: Any, *env_names: str) -> Any: + """Create a field that accepts multiple environment variable names.""" + return Field(default=default, validation_alias=AliasChoices(*env_names)) + + class AuthSettings(BaseSettings): """Authentication configuration for API key validation.""" @@ -99,36 +106,23 @@ class AgentControlServerDatabaseConfig(BaseSettings): model_config = SettingsConfigDict(**_COMMON_SETTINGS_CONFIG, env_prefix="AGENT_CONTROL_DB_") # Allow direct URL override for SQLite in local dev - url: str | None = Field( - default=None, - validation_alias=AliasChoices("AGENT_CONTROL_DB_URL", "DATABASE_URL", "DB_URL"), - ) + url: str | None = _env_alias_field(None, "AGENT_CONTROL_DB_URL", "DATABASE_URL", "DB_URL") # PostgreSQL settings (only used if url is not set) - host: str = Field( - default="localhost", - validation_alias=AliasChoices("AGENT_CONTROL_DB_HOST", "DB_HOST"), - ) - port: int = Field( - default=5432, - validation_alias=AliasChoices("AGENT_CONTROL_DB_PORT", "DB_PORT"), - ) - user: str = Field( - default="agent_control", - validation_alias=AliasChoices("AGENT_CONTROL_DB_USER", "DB_USER"), + host: str = _env_alias_field("localhost", "AGENT_CONTROL_DB_HOST", "DB_HOST") + port: int = _env_alias_field(5432, "AGENT_CONTROL_DB_PORT", "DB_PORT") + user: str = _env_alias_field("agent_control", "AGENT_CONTROL_DB_USER", "DB_USER") + password: str = _env_alias_field( + "agent_control", + "AGENT_CONTROL_DB_PASSWORD", + "DB_PASSWORD", ) - password: str = Field( - default="agent_control", - validation_alias=AliasChoices("AGENT_CONTROL_DB_PASSWORD", "DB_PASSWORD"), - ) - database: str = Field( - default="agent_control", - validation_alias=AliasChoices("AGENT_CONTROL_DB_DATABASE", "DB_DATABASE"), - ) - driver: str = Field( - default="psycopg", - validation_alias=AliasChoices("AGENT_CONTROL_DB_DRIVER", "DB_DRIVER"), + database: str = _env_alias_field( + "agent_control", + "AGENT_CONTROL_DB_DATABASE", + "DB_DATABASE", ) + driver: str = _env_alias_field("psycopg", "AGENT_CONTROL_DB_DRIVER", "DB_DRIVER") def get_url(self) -> str: """Get database URL, preferring an explicit URL if configured.""" @@ -145,50 +139,36 @@ class Settings(BaseSettings): model_config = SettingsConfigDict(**_COMMON_SETTINGS_CONFIG, env_prefix="AGENT_CONTROL_") # Server settings - host: str = Field( - default="0.0.0.0", - validation_alias=AliasChoices("AGENT_CONTROL_HOST", "HOST"), - ) - port: int = Field( - default=8000, - validation_alias=AliasChoices("AGENT_CONTROL_PORT", "PORT"), - ) - debug: bool = Field( - default=False, - validation_alias=AliasChoices("AGENT_CONTROL_DEBUG", "DEBUG"), - ) + host: str = _env_alias_field("0.0.0.0", "AGENT_CONTROL_HOST", "HOST") + port: int = _env_alias_field(8000, "AGENT_CONTROL_PORT", "PORT") + debug: bool = _env_alias_field(False, "AGENT_CONTROL_DEBUG", "DEBUG") # API settings - api_version: str = Field( - default="v1", - validation_alias=AliasChoices("AGENT_CONTROL_API_VERSION", "API_VERSION"), - ) - api_prefix: str = Field( - default="/api", - validation_alias=AliasChoices("AGENT_CONTROL_API_PREFIX", "API_PREFIX"), - ) + api_version: str = _env_alias_field("v1", "AGENT_CONTROL_API_VERSION", "API_VERSION") + api_prefix: str = _env_alias_field("/api", "AGENT_CONTROL_API_PREFIX", "API_PREFIX") # Prometheus metrics settings - prometheus_metrics_prefix: str = Field( - default="agent_control_server", - validation_alias=AliasChoices( - "AGENT_CONTROL_PROMETHEUS_METRICS_PREFIX", - "PROMETHEUS_METRICS_PREFIX", - ), + prometheus_metrics_prefix: str = _env_alias_field( + "agent_control_server", + "AGENT_CONTROL_PROMETHEUS_METRICS_PREFIX", + "PROMETHEUS_METRICS_PREFIX", ) # CORS settings - cors_origins: list[str] | str = Field( - default="*", - validation_alias=AliasChoices("AGENT_CONTROL_CORS_ORIGINS", "CORS_ORIGINS"), + cors_origins: list[str] | str = _env_alias_field( + "*", + "AGENT_CONTROL_CORS_ORIGINS", + "CORS_ORIGINS", ) - allow_methods: list[str] | str = Field( - default=["*"], - validation_alias=AliasChoices("AGENT_CONTROL_ALLOW_METHODS", "ALLOW_METHODS"), + allow_methods: list[str] | str = _env_alias_field( + ["*"], + "AGENT_CONTROL_ALLOW_METHODS", + "ALLOW_METHODS", ) - allow_headers: list[str] | str = Field( - default=["*"], - validation_alias=AliasChoices("AGENT_CONTROL_ALLOW_HEADERS", "ALLOW_HEADERS"), + allow_headers: list[str] | str = _env_alias_field( + ["*"], + "AGENT_CONTROL_ALLOW_HEADERS", + "ALLOW_HEADERS", ) def get_cors_origins(self) -> list[str]: @@ -234,32 +214,7 @@ class LoggingSettings(BaseSettings): model_config = SettingsConfigDict(**_COMMON_SETTINGS_CONFIG, env_prefix="AGENT_CONTROL_LOG_") level: str | None = None - json_logs: bool = Field( - default=False, - validation_alias="AGENT_CONTROL_LOG_JSON", - ) - - @field_validator("level", mode="before") - @classmethod - def _normalize_level(cls, value: object) -> object: - """Treat blank strings as unset and trim whitespace from configured levels.""" - if isinstance(value, str): - stripped = value.strip() - if not stripped: - return None - return stripped - return value - - @field_validator("json_logs", mode="before") - @classmethod - def _normalize_json_logs(cls, value: object) -> object: - """Treat blank strings as false and trim whitespace before bool parsing.""" - if isinstance(value, str): - stripped = value.strip() - if not stripped: - return False - return stripped - return value + json_logs: bool = _env_alias_field(False, "AGENT_CONTROL_LOG_JSON") class UISettings(BaseSettings): diff --git a/server/src/agent_control_server/logging_utils.py b/server/src/agent_control_server/logging_utils.py index ade03a24..7c97744b 100644 --- a/server/src/agent_control_server/logging_utils.py +++ b/server/src/agent_control_server/logging_utils.py @@ -10,32 +10,54 @@ "DEBUG": logging.DEBUG, "NOTSET": logging.NOTSET, } +_UVICORN_LEVELS = {"CRITICAL", "ERROR", "WARNING", "INFO", "DEBUG", "TRACE"} + + +def _normalize_level_name(level: str | None) -> str | None: + if level is None: + return None + normalized = level.upper() + if normalized in _LEVELS: + return normalized + return None + + +def _configured_level_name() -> str | None: + return _normalize_level_name(LoggingSettings().level) def _parse_level(level: str | int | None) -> int: if isinstance(level, int): return level - if isinstance(level, str): - return _LEVELS.get(level.upper(), logging.INFO) - configured_level = LoggingSettings().level + normalized = _normalize_level_name(level) + if normalized is not None: + return _LEVELS[normalized] + configured_level = _configured_level_name() if configured_level is not None: - return _LEVELS.get(configured_level.upper(), logging.INFO) + return _LEVELS[configured_level] return logging.INFO def get_log_level_name(default_level: str = "INFO") -> str: """Resolve the configured log level name, falling back to the provided default.""" - normalized_default = default_level.upper() - if normalized_default not in _LEVELS: - normalized_default = "INFO" - - configured_level = LoggingSettings().level + configured_level = _configured_level_name() if configured_level is not None: - normalized = configured_level.upper() - if normalized in _LEVELS: - return normalized + return configured_level + normalized_default = _normalize_level_name(default_level) + if normalized_default is not None: + return normalized_default + return "INFO" + + +def get_uvicorn_log_level_name(default_level: str = "INFO") -> str: + """Resolve a uvicorn-compatible log level name.""" + normalized_level = get_log_level_name(default_level) + if normalized_level in _UVICORN_LEVELS: + return normalized_level + normalized_default = _normalize_level_name(default_level) + if normalized_default in _UVICORN_LEVELS: return normalized_default - return normalized_default + return "INFO" def _parse_json(json_flag: bool | None) -> bool: diff --git a/server/src/agent_control_server/main.py b/server/src/agent_control_server/main.py index 94a806de..c355f0df 100644 --- a/server/src/agent_control_server/main.py +++ b/server/src/agent_control_server/main.py @@ -32,7 +32,7 @@ http_exception_handler, validation_exception_handler, ) -from .logging_utils import configure_logging, get_log_level_name +from .logging_utils import configure_logging, get_uvicorn_log_level_name from .observability.ingest import DirectEventIngestor from .observability.store import PostgresEventStore from .ui_assets import configure_ui_routes @@ -60,6 +60,10 @@ ] +def _default_log_level() -> str: + return "DEBUG" if settings.debug else "INFO" + + def add_prometheus_metrics(app: FastAPI, metrics_prefix: str) -> None: """Configure Prometheus metrics for the FastAPI app.""" app.add_middleware( @@ -78,8 +82,7 @@ def add_prometheus_metrics(app: FastAPI, metrics_prefix: str) -> None: async def lifespan(app: FastAPI) -> AsyncGenerator[None, None]: """Lifespan context manager for FastAPI app startup and shutdown.""" # Startup: Configure logging - default_log_level = "DEBUG" if settings.debug else "INFO" - configure_logging(default_level=default_log_level) + configure_logging(default_level=_default_log_level()) # Discover evaluators at startup discover_evaluators() @@ -157,8 +160,7 @@ async def lifespan(app: FastAPI) -> AsyncGenerator[None, None]: ) # Configure logging -default_log_level = "DEBUG" if settings.debug else "INFO" -configure_logging(default_level=default_log_level) +configure_logging(default_level=_default_log_level()) # ============================================================================= @@ -272,12 +274,11 @@ async def health_check() -> HealthResponse: def run() -> None: """Run the server application.""" - default_log_level = "DEBUG" if settings.debug else "INFO" uvicorn.run( app, host=settings.host, port=settings.port, - log_level=get_log_level_name(default_log_level).lower(), + log_level=get_uvicorn_log_level_name(_default_log_level()).lower(), ) diff --git a/server/tests/test_config.py b/server/tests/test_config.py index c53d68d0..b5ff1682 100644 --- a/server/tests/test_config.py +++ b/server/tests/test_config.py @@ -76,6 +76,18 @@ def test_db_config_prefers_agent_control_env_over_legacy(monkeypatch) -> None: assert config.get_url() == "sqlite:///tmp/canonical.db" +def test_db_config_ignores_blank_agent_control_url_and_uses_legacy(monkeypatch) -> None: + # Given: the canonical URL is blank but a legacy URL is still configured + monkeypatch.setenv("AGENT_CONTROL_DB_URL", "") + monkeypatch.setenv("DATABASE_URL", "sqlite:///tmp/legacy.db") + + # When: loading DB config from the environment + config = AgentControlServerDatabaseConfig() + + # Then: the blank canonical env var is ignored + assert config.get_url() == "sqlite:///tmp/legacy.db" + + def test_settings_parses_cors_origins_string() -> None: # Given: a comma-separated CORS origins string settings = Settings(cors_origins="https://a.example, https://b.example") @@ -146,6 +158,18 @@ def test_settings_prefers_agent_control_env_vars_over_legacy(monkeypatch) -> Non assert config.get_cors_origins() == ["https://canonical.example"] +def test_settings_ignore_blank_agent_control_port_and_use_legacy(monkeypatch) -> None: + # Given: the canonical port is blank but the legacy port is still set + monkeypatch.setenv("AGENT_CONTROL_PORT", "") + monkeypatch.setenv("PORT", "9000") + + # When: loading settings from the environment + config = Settings() + + # Then: the blank canonical env var is ignored + assert config.port == 9000 + + def test_settings_returns_cors_origins_list_unchanged() -> None: # Given: a CORS origins list settings = Settings(cors_origins=["https://a.example", "https://b.example"]) diff --git a/server/tests/test_logging_utils.py b/server/tests/test_logging_utils.py index 033fe27d..750cd23d 100644 --- a/server/tests/test_logging_utils.py +++ b/server/tests/test_logging_utils.py @@ -7,6 +7,7 @@ _parse_level, configure_logging, get_log_level_name, + get_uvicorn_log_level_name, ) @@ -98,6 +99,17 @@ def test_get_log_level_name_treats_blank_env_as_unset(monkeypatch) -> None: assert resolved == "DEBUG" +def test_get_uvicorn_log_level_name_falls_back_from_notset(monkeypatch) -> None: + # Given: a configured level that Python logging accepts but uvicorn does not + monkeypatch.setenv("AGENT_CONTROL_LOG_LEVEL", "NOTSET") + + # When: resolving the uvicorn log level + resolved = get_uvicorn_log_level_name("DEBUG") + + # Then: uvicorn gets a supported fallback level + assert resolved == "DEBUG" + + def test_configure_logging_resets_uvicorn_handlers() -> None: # Given: a uvicorn logger with a custom handler logger = logging.getLogger("uvicorn") From ac5ee6b2057faaff183744e11dd90f5b83db768a Mon Sep 17 00:00:00 2001 From: Lev Neiman Date: Mon, 16 Mar 2026 13:21:29 -0700 Subject: [PATCH 7/7] docs: document preferred server db url env --- server/.env.example | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/server/.env.example b/server/.env.example index 79abf3bd..6d9aaef7 100644 --- a/server/.env.example +++ b/server/.env.example @@ -41,7 +41,10 @@ AGENT_CONTROL_CORS_ORIGINS="http://localhost:4000" # Database (preferred AGENT_CONTROL_DB_) # ######################################### -# Example PostgreSQL settings (legacy DB_* and DATABASE_URL are still accepted) +# Preferred single-variable form: +AGENT_CONTROL_DB_URL=postgresql+psycopg://agent_control:agent_control@localhost:5432/agent_control + +# Expanded PostgreSQL settings (legacy DB_* and DATABASE_URL are still accepted) AGENT_CONTROL_DB_HOST=localhost AGENT_CONTROL_DB_PORT=5432 AGENT_CONTROL_DB_USER=agent_control