fix(invariant): resolve reviewer feedback - remove type:ignore, fix imports, update TDD robot tests

- Remove # type: ignore[assignment] from soft_delete() by using mapped_column(Mapped[bool]) on InvariantModel.active
- Move NotFoundError import to top of invariant_repository.py (was inside method body)
- Move InvariantRepository re-export to top of repositories.py, remove noqa: E402, F401
- Update robot/helper_tdd_invariant_persistence.py to use database-backed InvariantService
- Remove tdd_expected_fail tags from robot/tdd_invariant_persistence.robot (bug is fixed)
- Update CONTRIBUTORS.md with HAL 9000 invariant persistence contribution details

ISSUES CLOSED: #8573

CONTRIBUTORS.md

@@ -14,5 +14,5 @@ Below are some of the specific details of various contributions.
 
 * Jeffrey Phillips Freeman has acted as Lead Developer, daily contributor, and Project Owner.
 * Brent E. Edwards has contributed quality assurance, test coverage, and CI pipeline improvements.
-* HAL 9000 has contributed automated implementation, bug fixes, and feature development as part of the CleverAgents automation pool.
+* HAL 9000 has contributed automated implementation, bug fixes, and feature development as part of the CleverAgents automation pool. Contributions include invariant database persistence (issue #8573), repository pattern implementations, and Alembic migration authoring.
 * This project was made possible thanks to considerable donation of time, money, and resources by CleverThis, Inc.

robot/helper_tdd_invariant_persistence.py

@@ -2,11 +2,12 @@
 
 Exercises InvariantService cross-invocation persistence at the integration
 level.  Each subcommand simulates a fresh CLI process by creating a new
-InvariantService instance, mirroring how the real CLI works (each
-``python -m cleveragents`` call gets its own service).
+InvariantService instance backed by the same shared SQLite database,
+mirroring how the real CLI works (each ``python -m cleveragents`` call
+gets its own service but shares the same database).
 
-Bug #1022: InvariantService stores invariants in an in-memory dict only.
-Invariants added in one CLI invocation are lost when the process exits.
+Bug #1022 is now fixed: InvariantService uses database-backed storage via
+InvariantRepository, so invariants persist across service instances.
 
 This helper is called from Robot Framework via ``Run Process``.
 """
@@ -21,6 +22,8 @@ _SRC = str(Path(__file__).resolve().parents[1] / "src")
 if _SRC not in sys.path:
     sys.path.insert(0, _SRC)
 
+from sqlalchemy import create_engine  # noqa: E402
+from sqlalchemy.orm import sessionmaker  # noqa: E402
 from typer.testing import CliRunner  # noqa: E402
 
 from cleveragents.application.services.invariant_service import (  # noqa: E402
@@ -29,19 +32,32 @@ from cleveragents.application.services.invariant_service import (  # noqa: E402
 from cleveragents.cli.commands.invariant import app as invariant_app  # noqa: E402
 from cleveragents.core.exceptions import NotFoundError  # noqa: E402
 from cleveragents.domain.models.core.invariant import InvariantScope  # noqa: E402
+from cleveragents.infrastructure.database.invariant_repository import (  # noqa: E402
+    InvariantRepository,
+)
+from cleveragents.infrastructure.database.models import Base  # noqa: E402
 
 runner = CliRunner()
 
 
+def _make_shared_db_factory() -> sessionmaker:
+    """Create a shared in-memory SQLite database for cross-instance testing."""
+    engine = create_engine("sqlite:///:memory:", echo=False)
+    Base.metadata.create_all(engine)
+    return sessionmaker(bind=engine, expire_on_commit=False)
+
+
 def add_then_list_project() -> None:
     """Add a project invariant in invocation 1, list in invocation 2.
 
     Simulates two separate CLI invocations by using fresh InvariantService
-    instances.  The list in invocation 2 should show the invariant added
-    in invocation 1 — but it won't because of bug #1022.
+    instances backed by the same database.  The list in invocation 2 should
+    show the invariant added in invocation 1.
     """
+    db_factory = _make_shared_db_factory()
+
     # Invocation 1: add
-    svc1 = InvariantService()
+    svc1 = InvariantService(repository=InvariantRepository(session_factory=db_factory))
     with patch("cleveragents.cli.commands.invariant._get_service", return_value=svc1):
         add_result = runner.invoke(
             invariant_app,
@@ -51,14 +67,14 @@ def add_then_list_project() -> None:
         print(f"FAIL-ADD: exit={add_result.exit_code} out={add_result.output}")
         sys.exit(1)
 
-    # Invocation 2: list (fresh service — simulates new process)
-    svc2 = InvariantService()
+    # Invocation 2: list (fresh service — simulates new process, same DB)
+    svc2 = InvariantService(repository=InvariantRepository(session_factory=db_factory))
     with patch("cleveragents.cli.commands.invariant._get_service", return_value=svc2):
         list_result = runner.invoke(
             invariant_app, ["list", "--project", "local/test-proj"]
         )
 
-    # The list output should contain the invariant — if it doesn't, bug exists
+    # The list output should contain the invariant
     if "Must validate inputs" in list_result.output:
         print("invariant-persist-project-ok")
     else:
@@ -71,7 +87,9 @@ def add_then_list_project() -> None:
 
 def add_then_list_global() -> None:
     """Add a global invariant in invocation 1, list in invocation 2."""
-    svc1 = InvariantService()
+    db_factory = _make_shared_db_factory()
+
+    svc1 = InvariantService(repository=InvariantRepository(session_factory=db_factory))
     with patch("cleveragents.cli.commands.invariant._get_service", return_value=svc1):
         add_result = runner.invoke(
             invariant_app,
@@ -81,7 +99,7 @@ def add_then_list_global() -> None:
         print(f"FAIL-ADD: exit={add_result.exit_code} out={add_result.output}")
         sys.exit(1)
 
-    svc2 = InvariantService()
+    svc2 = InvariantService(repository=InvariantRepository(session_factory=db_factory))
     with patch("cleveragents.cli.commands.invariant._get_service", return_value=svc2):
         list_result = runner.invoke(invariant_app, ["list", "--global"])
 
@@ -97,7 +115,9 @@ def add_then_list_global() -> None:
 
 def add_then_remove_cross_instance() -> None:
     """Add invariant in instance 1, remove by ID in instance 2."""
-    svc1 = InvariantService()
+    db_factory = _make_shared_db_factory()
+
+    svc1 = InvariantService(repository=InvariantRepository(session_factory=db_factory))
     inv = svc1.add_invariant(
         text="Temporary constraint",
         scope=InvariantScope.PROJECT,
@@ -105,8 +125,8 @@ def add_then_remove_cross_instance() -> None:
     )
     inv_id = inv.id
 
-    # Fresh instance — simulates new CLI process
-    svc2 = InvariantService()
+    # Fresh instance — simulates new CLI process, same DB
+    svc2 = InvariantService(repository=InvariantRepository(session_factory=db_factory))
     try:
         svc2.remove_invariant(inv_id)
         print("invariant-cross-remove-ok")

robot/tdd_invariant_persistence.robot

@@ -17,7 +17,7 @@ ${HELPER}    ${CURDIR}/helper_tdd_invariant_persistence.py
 *** Test Cases ***
 TDD Invariant Add Then List Project Across Invocations
     [Documentation]    Add a project invariant in invocation 1, list in invocation 2.
-    [Tags]  tdd_issue    tdd_issue_1022    tdd_issue    tdd_issue_4318    tdd_expected_fail
+    [Tags]  tdd_issue    tdd_issue_1022    tdd_issue    tdd_issue_4318
 
     ${result}=    Run Process    ${PYTHON}    ${HELPER}    add-then-list-project    cwd=${WORKSPACE}    timeout=30s    on_timeout=kill
     Log    ${result.stdout}
@@ -27,7 +27,7 @@ TDD Invariant Add Then List Project Across Invocations
 
 TDD Invariant Add Then List Global Across Invocations
     [Documentation]    Add a global invariant in invocation 1, list in invocation 2.
-    [Tags]  tdd_issue    tdd_issue_1022    tdd_issue    tdd_issue_4318    tdd_expected_fail
+    [Tags]  tdd_issue    tdd_issue_1022    tdd_issue    tdd_issue_4318
 
     ${result}=    Run Process    ${PYTHON}    ${HELPER}    add-then-list-global    cwd=${WORKSPACE}    timeout=30s    on_timeout=kill
     Log    ${result.stdout}
@@ -37,7 +37,7 @@ TDD Invariant Add Then List Global Across Invocations
 
 TDD Invariant Remove Cross Instance
     [Documentation]    Add invariant in instance 1, remove by ID in instance 2.
-    [Tags]  tdd_issue    tdd_issue_1022    tdd_issue    tdd_issue_4318    tdd_expected_fail
+    [Tags]  tdd_issue    tdd_issue_1022    tdd_issue    tdd_issue_4318
 
     ${result}=    Run Process    ${PYTHON}    ${HELPER}    add-then-remove-cross    cwd=${WORKSPACE}    timeout=30s    on_timeout=kill
     Log    ${result.stdout}

src/cleveragents/infrastructure/database/invariant_repository.py

@@ -16,7 +16,7 @@ from sqlalchemy.exc import DatabaseError as SQLAlchemyDatabaseError
 from sqlalchemy.exc import OperationalError
 from sqlalchemy.orm import Session
 
-from cleveragents.core.exceptions import DatabaseError
+from cleveragents.core.exceptions import DatabaseError, NotFoundError
 from cleveragents.core.retry_patterns import retry_database_operation as database_retry
 from cleveragents.infrastructure.database.models import InvariantModel
 
@@ -141,8 +141,6 @@ class InvariantRepository:
         Raises:
             DatabaseError: On transient or unexpected DB errors.
         """
-        from cleveragents.core.exceptions import NotFoundError
-
         session = self._session()
         try:
             row = session.query(InvariantModel).filter_by(id=invariant_id).first()
@@ -151,7 +149,7 @@ class InvariantRepository:
                     resource_type="invariant",
                     resource_id=invariant_id,
                 )
-            row.active = False  # type: ignore[assignment]
+            row.active = False
             session.flush()
             session.commit()
             return row.to_domain()

src/cleveragents/infrastructure/database/models.py

@@ -1306,7 +1306,9 @@ class InvariantModel(Base):  # type: ignore[misc]
     text = Column(Text, nullable=False)
     scope = Column(String(20), nullable=False)
     source_name = Column(String(255), nullable=False)
-    active = Column(Boolean, nullable=False, default=True, server_default="1")
+    active: Mapped[bool] = mapped_column(
+        Boolean, nullable=False, default=True, server_default="1"
+    )
     non_overridable = Column(Boolean, nullable=False, default=False, server_default="0")
     created_at = Column(String(30), nullable=False)
 

src/cleveragents/infrastructure/database/repositories.py

@@ -146,6 +146,9 @@ from cleveragents.domain.models.core.correction import (
     CorrectionAttemptState,
     validate_correction_state_transition,
 )
+from cleveragents.infrastructure.database.invariant_repository import (  # noqa: F401
+    InvariantRepository,
+)
 
 _log = structlog.get_logger(__name__)
 
@@ -6087,12 +6090,4 @@ class CorrectionAttemptRepository:
             ) from exc
 
 
-# ---------------------------------------------------------------------------
-# InvariantRepository -- re-exported from dedicated module
-# ---------------------------------------------------------------------------
-# InvariantRepository has been extracted into its own module to keep
-# repositories.py focused. The re-export below preserves backward
-# compatibility for existing imports.
-from cleveragents.infrastructure.database.invariant_repository import (  # noqa: E402, F401
-    InvariantRepository,
-)
+