diff --git a/features/steps/tdd_migration_runner_get_current_revision_threading_10507_steps.py b/features/steps/tdd_migration_runner_get_current_revision_threading_10507_steps.py new file mode 100644 index 000000000..ca6cb4f95 --- /dev/null +++ b/features/steps/tdd_migration_runner_get_current_revision_threading_10507_steps.py @@ -0,0 +1,85 @@ +"""Steps for TDD Issue #10507 — get_current_revision() SQLite threading fix.""" + +from __future__ import annotations + +from typing import Any +from unittest.mock import MagicMock, patch + +from behave import then, when + + +def _run_get_current_revision_and_capture_kwargs( + context: Any, +) -> None: + """Shared helper: call get_current_revision() and capture create_engine kwargs. + + Mocks ``create_engine`` and ``MigrationContext.configure`` so the call + completes without a real database. The keyword arguments passed to + ``create_engine`` are stored on ``context.engine_creation_kwargs`` for + subsequent assertion steps. + """ + fake_engine = MagicMock() + fake_connection = MagicMock() + fake_connection.__enter__ = MagicMock(return_value=fake_connection) + fake_connection.__exit__ = MagicMock(return_value=False) + fake_engine.connect.return_value = fake_connection + + migration_ctx = MagicMock() + migration_ctx.get_current_revision.return_value = None + + captured_kwargs: list[dict[str, Any]] = [] + + def fake_create_engine(url: str, **kwargs: Any) -> MagicMock: + captured_kwargs.append(kwargs) + return fake_engine + + with ( + patch( + "cleveragents.infrastructure.database.migration_runner.create_engine", + side_effect=fake_create_engine, + ), + patch( + "cleveragents.infrastructure.database.migration_runner.MigrationContext.configure", + return_value=migration_ctx, + ), + ): + context.revision_result = context.runner.get_current_revision() + + context.engine_creation_kwargs = captured_kwargs + + +@when("I request the current revision and capture the engine creation args") +def step_when_capture_engine_args(context: Any) -> None: + """Call get_current_revision and capture the create_engine call arguments.""" + _run_get_current_revision_and_capture_kwargs(context) + + +@then("the SQLite engine should be created with check_same_thread set to False") +def step_then_sqlite_engine_has_check_same_thread(context: Any) -> None: + """Verify the SQLite engine was created with check_same_thread=False.""" + assert len(context.engine_creation_kwargs) == 1, ( + f"Expected exactly 1 create_engine call, got {len(context.engine_creation_kwargs)}" + ) + kwargs = context.engine_creation_kwargs[0] + assert "connect_args" in kwargs, ( + "Expected connect_args in create_engine kwargs for SQLite, " + f"but got kwargs: {kwargs}" + ) + assert kwargs["connect_args"].get("check_same_thread") is False, ( + "Expected check_same_thread=False in connect_args, " + f"but got: {kwargs['connect_args']}" + ) + + +@then("the non-SQLite engine should be created without check_same_thread") +def step_then_non_sqlite_engine_no_check_same_thread(context: Any) -> None: + """Verify non-SQLite engines are not given check_same_thread.""" + assert len(context.engine_creation_kwargs) == 1, ( + f"Expected exactly 1 create_engine call, got {len(context.engine_creation_kwargs)}" + ) + kwargs = context.engine_creation_kwargs[0] + connect_args = kwargs.get("connect_args", {}) + assert "check_same_thread" not in connect_args, ( + "Expected check_same_thread to be absent for non-SQLite engine, " + f"but got connect_args: {connect_args}" + ) diff --git a/features/tdd_migration_runner_get_current_revision_threading_10507.feature b/features/tdd_migration_runner_get_current_revision_threading_10507.feature new file mode 100644 index 000000000..81b500e9d --- /dev/null +++ b/features/tdd_migration_runner_get_current_revision_threading_10507.feature @@ -0,0 +1,31 @@ +@tdd_issue @tdd_issue_10507 +Feature: TDD Issue #10507 — get_current_revision() must pass check_same_thread=False for SQLite + As a developer using MigrationRunner in a multi-threaded application + I want get_current_revision() to work safely from background threads + So that async startup flows and background migration checks do not crash + + The root cause is that MigrationRunner.get_current_revision() calls + create_engine(self.database_url) without connect_args={"check_same_thread": False} + for SQLite databases. When called from a thread other than the one that + created the engine, SQLite raises: + ProgrammingError: SQLite objects created in a thread can only be + used in that same thread. + + The sibling method init_or_upgrade() already passes check_same_thread=False + for SQLite, making this an inconsistency in the same class. Because + get_pending_migrations() and check_migrations_needed() both delegate to + get_current_revision(), the threading bug propagates to all three methods. + + The fix adds connect_args={"check_same_thread": False} to the create_engine() + call in get_current_revision() when the database URL starts with "sqlite", + consistent with the existing pattern in init_or_upgrade(). + + Scenario: get_current_revision passes check_same_thread=False for SQLite engine + Given a migration runner configured for "sqlite:///:memory:" + When I request the current revision and capture the engine creation args + Then the SQLite engine should be created with check_same_thread set to False + + Scenario: get_current_revision does not pass check_same_thread for non-SQLite engine + Given a migration runner configured for "postgresql://user:pass@localhost/testdb" + When I request the current revision and capture the engine creation args + Then the non-SQLite engine should be created without check_same_thread diff --git a/src/cleveragents/infrastructure/database/migration_runner.py b/src/cleveragents/infrastructure/database/migration_runner.py index 8f8393cb3..4f91f0477 100644 --- a/src/cleveragents/infrastructure/database/migration_runner.py +++ b/src/cleveragents/infrastructure/database/migration_runner.py @@ -151,6 +151,12 @@ class MigrationRunner: def get_current_revision(self) -> str | None: """Get the current migration revision of the database. + For SQLite databases, the engine is created with + ``connect_args={"check_same_thread": False}`` so that this method + can be safely called from any thread — including background threads + used in async startup flows. This is consistent with the pattern + used in :meth:`init_or_upgrade`. + Returns: Current revision ID or None if no migrations have been applied """