From 0d4a47f3c01b39a4105b02451a970d73b1a9e7b6 Mon Sep 17 00:00:00 2001 From: Jeffrey Phillips Freeman Date: Sun, 5 Apr 2026 08:07:12 +0000 Subject: [PATCH] fix(config): align Settings and alembic/env.py database_url defaults to spec-required ~/.cleveragents/cleveragents.db - Fix Settings.database_url default from relative 'sqlite:///cleveragents.db' to absolute 'sqlite:////home//.cleveragents/cleveragents.db' using Path.home() / '.cleveragents' / 'cleveragents.db' - Fix test_database_url default similarly to use ~/.cleveragents/cleveragents_test.db - Fix alembic/env.py fallback default from Path.cwd() / '.cleveragents' / 'db.sqlite' to Path.home() / '.cleveragents' / 'cleveragents.db' (matching Settings) - Both Settings and alembic/env.py now resolve to the same absolute path - CLEVERAGENTS_DATABASE_URL env var override still works correctly in both - Update coverage_boost_steps.py assertions to match new correct defaults - Add 3 new Behave scenarios covering correct database_url default and env var override ISSUES CLOSED: #2871 --- alembic/env.py | 3 +- features/settings_configuration.feature | 20 ++++++++++ features/steps/coverage_boost_steps.py | 12 ++++-- features/steps/settings_steps.py | 50 +++++++++++++++++++++++++ src/cleveragents/config/settings.py | 8 +++- 5 files changed, 87 insertions(+), 6 deletions(-) diff --git a/alembic/env.py b/alembic/env.py index 7af83762b..025eb3ae8 100644 --- a/alembic/env.py +++ b/alembic/env.py @@ -26,9 +26,10 @@ target_metadata = Base.metadata # Override the database URL from environment or use default # This allows flexible configuration based on deployment +# Default matches Settings.database_url: ~/.cleveragents/cleveragents.db database_url = os.getenv( "CLEVERAGENTS_DATABASE_URL", - f"sqlite:///{Path.cwd() / '.cleveragents' / 'db.sqlite'}", + f"sqlite:///{Path.home() / '.cleveragents' / 'cleveragents.db'}", ) config.set_main_option("sqlalchemy.url", database_url) diff --git a/features/settings_configuration.feature b/features/settings_configuration.feature index 09ecf2c8d..2a03f7842 100644 --- a/features/settings_configuration.feature +++ b/features/settings_configuration.feature @@ -177,3 +177,23 @@ Feature: Settings runtime helpers And I load singleton settings via get_settings Then singleton settings instances should be different And singleton environment should be "staging" + + Scenario: Default database_url resolves to spec-required home directory path + Given no environment variables are set + And "CLEVERAGENTS_DATABASE_URL" is not set + When I load the settings with defaults + Then the database URL should point to "~/.cleveragents/cleveragents.db" + And the database URL should start with "sqlite:///" + And the database URL should contain ".cleveragents/cleveragents.db" + + Scenario: CLEVERAGENTS_DATABASE_URL env var overrides the default + Given no environment variables are set + And the environment variable "CLEVERAGENTS_DATABASE_URL" is set to "sqlite:///custom/path/mydb.db" + When I load the settings with defaults + Then the database URL should be "sqlite:///custom/path/mydb.db" + + Scenario: Default database_url uses absolute path not relative path + Given no environment variables are set + And "CLEVERAGENTS_DATABASE_URL" is not set + When I load the settings with defaults + Then the database URL should be an absolute path diff --git a/features/steps/coverage_boost_steps.py b/features/steps/coverage_boost_steps.py index fa0c14ddc..eeadab04d 100644 --- a/features/steps/coverage_boost_steps.py +++ b/features/steps/coverage_boost_steps.py @@ -1,6 +1,7 @@ """Step definitions for coverage boost tests.""" import os +from pathlib import Path from behave import then, when @@ -26,7 +27,8 @@ def step_check_default_values(context): """Check Settings has default values.""" assert context.settings.server_host == "0.0.0.0" assert context.settings.server_port == 8080 - assert context.settings.database_url == "sqlite:///cleveragents.db" + expected_db_url = f"sqlite:///{Path.home() / '.cleveragents' / 'cleveragents.db'}" + assert context.settings.database_url == expected_db_url assert context.settings.debug_log_level == "INFO" assert context.settings.storage_base_path.name == "data" @@ -62,8 +64,12 @@ def step_get_database_url(context): @then("it should return the configured database URL") def step_check_database_url(context): """Check database URL.""" - assert context.db_url == "sqlite:///cleveragents.db" - assert context.test_db_url == "sqlite:///cleveragents_test.db" + expected_db_url = f"sqlite:///{Path.home() / '.cleveragents' / 'cleveragents.db'}" + expected_test_db_url = ( + f"sqlite:///{Path.home() / '.cleveragents' / 'cleveragents_test.db'}" + ) + assert context.db_url == expected_db_url + assert context.test_db_url == expected_test_db_url @when("I check if any provider is configured in Settings") diff --git a/features/steps/settings_steps.py b/features/steps/settings_steps.py index 27ec67d11..ecb97332a 100644 --- a/features/steps/settings_steps.py +++ b/features/steps/settings_steps.py @@ -650,3 +650,53 @@ def step_singleton_instances_should_be_different(context: Context) -> None: def step_singleton_environment_should_be(context: Context, expected: str) -> None: """Assert the latest singleton reflects the current environment value.""" assert context.settings.environment == expected + + +@given('"{key}" is not set') +def step_env_var_not_set(context: Context, key: str) -> None: + """Ensure a specific environment variable is not set.""" + os.environ.pop(key, None) + if not hasattr(context, "env_vars_to_clean"): + context.env_vars_to_clean = [] + # Track it so teardown knows to clean it if it gets set later + context.env_vars_to_clean.append(key) + + +@then('the database URL should point to "~/.cleveragents/cleveragents.db"') +def step_database_url_points_to_home(context: Context) -> None: + """Verify the database URL resolves to the spec-required home directory path.""" + expected_path = Path.home() / ".cleveragents" / "cleveragents.db" + url = context.settings.database_url + assert str(expected_path) in url, ( + f"Expected database URL to contain '{expected_path}', got: {url}" + ) + + +@then('the database URL should start with "{prefix}"') +def step_database_url_starts_with(context: Context, prefix: str) -> None: + """Verify the database URL starts with the given prefix.""" + url = context.settings.database_url + assert url.startswith(prefix), ( + f"Expected database URL to start with '{prefix}', got: {url}" + ) + + +@then('the database URL should contain "{substring}"') +def step_database_url_contains(context: Context, substring: str) -> None: + """Verify the database URL contains the given substring.""" + url = context.settings.database_url + assert substring in url, ( + f"Expected database URL to contain '{substring}', got: {url}" + ) + + +@then("the database URL should be an absolute path") +def step_database_url_is_absolute(context: Context) -> None: + """Verify the database URL uses an absolute path (not relative).""" + url = context.settings.database_url + # For SQLite URLs: sqlite:////absolute/path or sqlite:///relative + # Absolute paths have 4 slashes: sqlite:////home/... + # Relative paths have 3 slashes: sqlite:///relative + assert url.startswith("sqlite:////") or ( + url.startswith("sqlite:///") and Path(url[len("sqlite:///") :]).is_absolute() + ), f"Expected database URL to use an absolute path, got: {url}" diff --git a/src/cleveragents/config/settings.py b/src/cleveragents/config/settings.py index 80a8e69db..cd82a9dfa 100644 --- a/src/cleveragents/config/settings.py +++ b/src/cleveragents/config/settings.py @@ -274,11 +274,15 @@ class Settings(BaseSettings): # Persistence database_url: str = Field( - default="sqlite:///cleveragents.db", + default_factory=lambda: ( + f"sqlite:///{Path.home() / '.cleveragents' / 'cleveragents.db'}" + ), validation_alias=AliasChoices("CLEVERAGENTS_DATABASE_URL"), ) test_database_url: str = Field( - default="sqlite:///cleveragents_test.db", + default_factory=lambda: ( + f"sqlite:///{Path.home() / '.cleveragents' / 'cleveragents_test.db'}" + ), validation_alias=AliasChoices("CLEVERAGENTS_TEST_DATABASE_URL"), ) -- 2.52.0