fix(persistence): close session in AutomationProfileRepository auto_commit finally block

Add missing `finally: if self._auto_commit: session.close()` blocks to all four public session-creating methods in AutomationProfileRepository: get_by_name(), list_all(), upsert(), and delete(). Closes #987

CHANGELOG.md

@@ -88,6 +88,12 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
 
 ### Fixed
 
+- Fixed session leak in all `AutomationProfileRepository` public methods
+  (`get_by_name()`, `list_all()`, `upsert()`, and `delete()`): added
+  `finally: if self._auto_commit: session.close()` blocks matching the
+  pattern already used by `SessionRepository`, preventing database sessions
+  from being leaked when `auto_commit` mode is enabled. (#987)
+
 - **CLI — `agents actor add` rich output**: The `actor add` command now renders
   the full spec-required output including **Type**, **Config**, **Capabilities**,
   and **Tools** panels, matching the output format of `actor show`.

features/steps/tdd_automation_profile_session_leak_steps.py

@@ -1,14 +1,10 @@
-"""Step definitions for TDD Bug #987 — AutomationProfileRepository session leak.
+"""Step definitions for bug #987 regression coverage.
 
-This test captures bug #987: ``AutomationProfileRepository.upsert()`` and
-``delete()`` never call ``session.close()`` when using ``auto_commit`` mode.
-By contrast, ``SessionRepository`` correctly uses
-``finally: if self._auto_commit: db_session.close()`` in every method.
-
-The assertions here will **fail** until the bug is fixed, proving the bug
-exists.  The ``@tdd_expected_fail`` tag inverts the result so CI passes.
-
-This test uses ``@tdd_expected_fail`` until the fix for #987 is merged.
+These scenarios verify the fixed behavior for all public
+``AutomationProfileRepository`` methods (``get_by_name()``,
+``list_all()``, ``upsert()``, and ``delete()``): when ``auto_commit``
+mode is enabled, each method must close its session in a ``finally``
+block, matching the existing ``SessionRepository`` pattern.
 """
 
 from __future__ import annotations
@@ -71,6 +67,21 @@ class _FailingFlushTrackingSession(_TrackingSession):
         raise OperationalError("simulated failure", params=None, orig=Exception("boom"))
 
 
+class _FailingQueryTrackingSession(_TrackingSession):
+    """A ``_TrackingSession`` whose ``query()`` always raises.
+
+    Used to test the error path for read-only methods (``get_by_name``,
+    ``list_all``) without monkey-patching.
+    """
+
+    def query(self, *entities: Any, **kwargs: Any) -> Any:
+        raise OperationalError(
+            "simulated query failure",
+            params=None,
+            orig=Exception("boom"),
+        )
+
+
 # ---------------------------------------------------------------------------
 # Given steps
 # ---------------------------------------------------------------------------
@@ -202,6 +213,34 @@ def step_given_repo_with_prepopulated_and_failing(context: Context) -> None:
     context._cleanup_handlers.append(engine.dispose)
 
 
+@given(
+    "an AutomationProfileRepository with auto_commit enabled"
+    " and a failing-query session factory"
+)
+def step_given_repo_with_failing_query_factory(context: Context) -> None:
+    """Set up a repository whose session will raise on query."""
+    engine = create_engine("sqlite:///:memory:", echo=False)
+    Base.metadata.create_all(engine)
+
+    tracking_session = _FailingQueryTrackingSession(bind=engine)
+    context.tracking_session = tracking_session
+    context.db_engine = engine
+
+    def session_factory() -> Session:
+        return tracking_session
+
+    context.repo = AutomationProfileRepository(
+        session_factory=session_factory,
+        auto_commit=True,
+    )
+
+    # Register cleanup handlers for engine disposal and session close.
+    if not hasattr(context, "_cleanup_handlers"):
+        context._cleanup_handlers = []
+    context._cleanup_handlers.append(tracking_session.close)
+    context._cleanup_handlers.append(engine.dispose)
+
+
 # ---------------------------------------------------------------------------
 # When steps
 # ---------------------------------------------------------------------------
@@ -241,6 +280,38 @@ def step_when_delete_triggers_error(context: Context) -> None:
         context.delete_error = exc
 
 
+@when("I look up a profile by name via the repository")
+def step_when_get_by_name(context: Context) -> None:
+    """Call get_by_name on the repository under test."""
+    context.repo.get_by_name("local/nonexistent-profile")
+
+
+@when("I list all profiles via the repository")
+def step_when_list_all(context: Context) -> None:
+    """Call list_all on the repository under test."""
+    context.repo.list_all()
+
+
+@when("I attempt to look up a profile that triggers a database error")
+def step_when_get_by_name_triggers_error(context: Context) -> None:
+    """Attempt a get_by_name that will fail due to the failing query."""
+    context.get_by_name_error = None
+    try:
+        context.repo.get_by_name("local/nonexistent-profile")
+    except DatabaseError as exc:
+        context.get_by_name_error = exc
+
+
+@when("I attempt to list profiles that triggers a database error")
+def step_when_list_all_triggers_error(context: Context) -> None:
+    """Attempt a list_all that will fail due to the failing query."""
+    context.list_all_error = None
+    try:
+        context.repo.list_all()
+    except DatabaseError as exc:
+        context.list_all_error = exc
+
+
 # ---------------------------------------------------------------------------
 # Then steps
 # ---------------------------------------------------------------------------
@@ -250,9 +321,9 @@ def step_when_delete_triggers_error(context: Context) -> None:
 def step_then_session_closed(context: Context) -> None:
     """Assert that close() was called on the tracking session.
 
-    This assertion will FAIL on the current codebase because
-    AutomationProfileRepository does not call session.close() in a
-    finally block when auto_commit is True — proving bug #987.
+    Regression guard for bug #987: verifies that
+    AutomationProfileRepository calls session.close() in a finally
+    block when auto_commit is True.
     """
     assert context.tracking_session.close_called, (
         "Expected session.close() to have been called in auto_commit mode, "
@@ -293,3 +364,37 @@ def step_then_session_closed_despite_delete_error(context: Context) -> None:
         "error in auto_commit mode, but it was NOT called. This confirms "
         "bug #987: AutomationProfileRepository leaks sessions on error."
     )
+
+
+@then("the tracking session should have been closed despite the get_by_name error")
+def step_then_session_closed_despite_get_by_name_error(context: Context) -> None:
+    """Assert close() was called even when a get_by_name error occurred.
+
+    The finally block should ensure session cleanup regardless of
+    whether the query succeeded or failed.
+    """
+    assert context.get_by_name_error is not None, (
+        "Expected a DatabaseError from the failing get_by_name, but none was raised."
+    )
+    assert context.tracking_session.close_called, (
+        "Expected session.close() to have been called even after a database "
+        "error in auto_commit mode, but it was NOT called. This confirms "
+        "bug #987: AutomationProfileRepository leaks sessions on error."
+    )
+
+
+@then("the tracking session should have been closed despite the list_all error")
+def step_then_session_closed_despite_list_all_error(context: Context) -> None:
+    """Assert close() was called even when a list_all error occurred.
+
+    The finally block should ensure session cleanup regardless of
+    whether the query succeeded or failed.
+    """
+    assert context.list_all_error is not None, (
+        "Expected a DatabaseError from the failing list_all, but none was raised."
+    )
+    assert context.tracking_session.close_called, (
+        "Expected session.close() to have been called even after a database "
+        "error in auto_commit mode, but it was NOT called. This confirms "
+        "bug #987: AutomationProfileRepository leaks sessions on error."
+    )

features/tdd_automation_profile_session_leak.feature

@@ -1,25 +1,20 @@
-@tdd_expected_fail @tdd_issue @tdd_issue_987
+@tdd_issue @tdd_issue_987
 Feature: TDD Bug #987 — AutomationProfileRepository session leak
   As a developer
   I want to verify that AutomationProfileRepository closes sessions
   in auto_commit mode
-  So that the bug is captured and will be caught by a regression test
+  So that regressions in session cleanup are caught
 
-  AutomationProfileRepository.upsert() and delete() commit when
-  auto_commit is True but never call session.close() in a finally
-  block.  By contrast, SessionRepository correctly uses
-  ``finally: if self._auto_commit: db_session.close()`` in every
-  method.
+  Regression guard for bug #987.  AutomationProfileRepository methods
+  previously committed when auto_commit was True but never called
+  session.close() in a finally block.  The fix added
+  ``finally: if self._auto_commit: session.close()`` to all public
+  session-creating methods (get_by_name, list_all, upsert, delete),
+  matching the pattern already used by SessionRepository.
 
-  This inconsistency means AutomationProfileRepository leaks database
-  sessions over time, potentially exhausting the connection pool.
-
-  These tests assert the expected behaviour (session.close() IS called)
-  and will FAIL until the bug is fixed.  The @tdd_expected_fail tag
-  inverts the result so CI passes.
-
-  # This test captures bug #987 and uses @tdd_expected_fail until the
-  # fix is merged.
+  These scenarios verify that sessions are properly closed in
+  auto_commit mode for both success and error paths across all
+  public methods.
 
   Scenario: upsert closes session in auto_commit mode on success
     Given an AutomationProfileRepository with auto_commit enabled and a tracking session factory
@@ -41,3 +36,23 @@ Feature: TDD Bug #987 — AutomationProfileRepository session leak
     Given an AutomationProfileRepository with auto_commit enabled, a pre-populated profile, and a failing-flush tracking session
     When I attempt to delete a profile that triggers a database error
     Then the tracking session should have been closed despite the delete error
+
+  Scenario: get_by_name closes session in auto_commit mode on success
+    Given an AutomationProfileRepository with auto_commit enabled and a tracking session factory
+    When I look up a profile by name via the repository
+    Then the tracking session should have been closed
+
+  Scenario: list_all closes session in auto_commit mode on success
+    Given an AutomationProfileRepository with auto_commit enabled and a tracking session factory
+    When I list all profiles via the repository
+    Then the tracking session should have been closed
+
+  Scenario: get_by_name closes session in auto_commit mode on database error
+    Given an AutomationProfileRepository with auto_commit enabled and a failing-query session factory
+    When I attempt to look up a profile that triggers a database error
+    Then the tracking session should have been closed despite the get_by_name error
+
+  Scenario: list_all closes session in auto_commit mode on database error
+    Given an AutomationProfileRepository with auto_commit enabled and a failing-query session factory
+    When I attempt to list profiles that triggers a database error
+    Then the tracking session should have been closed despite the list_all error

src/cleveragents/infrastructure/database/repositories.py

@@ -4378,6 +4378,9 @@ class AutomationProfileRepository:
             SQLAlchemyDatabaseError,
         ) as exc:
             raise DatabaseError(f"Failed to get profile '{name}': {exc}") from exc
+        finally:
+            if self._auto_commit:
+                session.close()
 
     @database_retry
     def list_all(self) -> list[Any]:
@@ -4399,6 +4402,9 @@ class AutomationProfileRepository:
             SQLAlchemyDatabaseError,
         ) as exc:
             raise DatabaseError(f"Failed to list profiles: {exc}") from exc
+        finally:
+            if self._auto_commit:
+                session.close()
 
     @database_retry
     def upsert(
@@ -4460,6 +4466,9 @@ class AutomationProfileRepository:
             raise DatabaseError(
                 f"Failed to upsert profile '{profile.name}': {exc}"
             ) from exc
+        finally:
+            if self._auto_commit:
+                session.close()
 
     @database_retry
     def delete(self, name: str) -> None:
@@ -4489,6 +4498,9 @@ class AutomationProfileRepository:
         ) as exc:
             session.rollback()
             raise DatabaseError(f"Failed to delete profile '{name}': {exc}") from exc
+        finally:
+            if self._auto_commit:
+                session.close()
 
     @staticmethod
     def _to_domain(