Merge main into Epic 7 branch - resolve conflicts and fix linting

Co-authored-by: pvliesdonk <22190282+pvliesdonk@users.noreply.github.com>

Author: Copilot

src/mcp_devbench/managers/maintenance_manager.py

@@ -1,20 +1,27 @@
 """Background maintenance manager for periodic tasks."""
 
 import asyncio
-from datetime import datetime, timedelta, timezone
 
 from docker import DockerClient
+from docker.errors import NotFound
 
 from mcp_devbench.config import get_settings
 from mcp_devbench.models.database import get_db_manager
-from mcp_devbench.repositories.attachments import AttachmentRepository
 from mcp_devbench.repositories.containers import ContainerRepository
 from mcp_devbench.repositories.execs import ExecRepository
 from mcp_devbench.utils import get_logger
+from mcp_devbench.utils.cleanup import cleanup_orphaned_transients
 from mcp_devbench.utils.docker_client import get_docker_client
 
 logger = get_logger(__name__)
 
+# Maintenance task intervals (in seconds)
+MAINTENANCE_INTERVAL_SECONDS = 3600  # 1 hour
+MAINTENANCE_ERROR_RETRY_SECONDS = 60  # 1 minute
+
+# Cleanup retention periods
+EXEC_RETENTION_HOURS = 24  # 24 hours
+
 
 class MaintenanceManager:
     """Manager for background maintenance tasks."""
@@ -48,6 +55,7 @@ async def stop(self) -> None:
             try:
                 await self._task
             except asyncio.CancelledError:
+                # Task cancellation is expected during shutdown
                 pass
         logger.info("Maintenance manager stopped")
 
@@ -57,12 +65,12 @@ async def _run_maintenance_loop(self) -> None:
             try:
                 # Run maintenance tasks hourly
                 await self.run_maintenance()
-                await asyncio.sleep(3600)  # 1 hour
+                await asyncio.sleep(MAINTENANCE_INTERVAL_SECONDS)
             except asyncio.CancelledError:
                 break
             except Exception as e:
                 logger.error("Maintenance task failed", extra={"error": str(e)})
-                await asyncio.sleep(60)  # Retry after 1 minute on error
+                await asyncio.sleep(MAINTENANCE_ERROR_RETRY_SECONDS)
 
     async def run_maintenance(self) -> dict:
         """
@@ -119,40 +127,11 @@ async def _cleanup_orphaned_transients(self) -> int:
         logger.info("Cleaning up orphaned transient containers")
 
         try:
-            cutoff_days = self.settings.transient_gc_days
-            cutoff = datetime.now(timezone.utc) - timedelta(days=cutoff_days)
-
             async with self.db_manager.get_session() as session:
                 repo = ContainerRepository(session)
-                transients = await repo.list_by_status("stopped", persistent=False)
-
-                cleaned = 0
-                for container in transients:
-                    if container.last_seen < cutoff:
-                        try:
-                            # Try to remove Docker container if it exists
-                            try:
-                                docker_container = self.docker_client.containers.get(
-                                    container.docker_id
-                                )
-                                docker_container.remove(force=True)
-                            except Exception:
-                                pass  # Container may already be gone
-
-                            # Remove from database
-                            await repo.delete(container.id)
-                            cleaned += 1
-
-                            logger.info(
-                                "Cleaned up orphaned transient",
-                                extra={"container_id": container.id},
-                            )
-                        except Exception as e:
-                            logger.error(
-                                "Failed to clean up transient",
-                                extra={"container_id": container.id, "error": str(e)},
-                            )
-
+                cleaned = await cleanup_orphaned_transients(
+                    self.docker_client, repo, self.settings.transient_gc_days
+                )
                 return cleaned
 
         except Exception as e:
@@ -172,8 +151,8 @@ async def _cleanup_old_execs(self) -> int:
             async with self.db_manager.get_session() as session:
                 exec_repo = ExecRepository(session)
 
-                # Clean up execs older than 24 hours
-                cleaned = await exec_repo.cleanup_old(hours=24)
+                # Clean up execs older than configured retention period
+                cleaned = await exec_repo.cleanup_old(hours=EXEC_RETENTION_HOURS)
 
                 logger.info("Cleaned up old execs", extra={"count": cleaned})
                 return cleaned
@@ -192,14 +171,11 @@ async def _cleanup_abandoned_attachments(self) -> int:
         logger.info("Cleaning up abandoned attachments")
 
         try:
-            async with self.db_manager.get_session() as session:
-                attachment_repo = AttachmentRepository(session)
-
-                # Get all attachments
-                # In a full implementation, we would identify abandoned ones
-                # For now, just log
-                logger.info("Attachment cleanup completed")
-                return 0
+            # Get all attachments
+            # In a full implementation, we would identify abandoned ones
+            # For now, just log
+            logger.info("Attachment cleanup completed")
+            return 0
 
         except Exception as e:
             logger.error("Failed to clean up attachments", extra={"error": str(e)})
@@ -250,7 +226,7 @@ async def _sync_container_state(self) -> int:
 
                         synced += 1
 
-                    except Exception:
+                    except NotFound:
                         # Container doesn't exist, mark as stopped
                         if container.status != "stopped":
                             await repo.update_status(container.id, "stopped")
@@ -272,11 +248,12 @@ async def _vacuum_database(self) -> None:
         logger.info("Vacuuming database")
 
         try:
-            from sqlalchemy import text
-
             async with self.db_manager.get_session() as session:
-                # Execute VACUUM command
+                from sqlalchemy import text
+
+                # Execute VACUUM command using raw SQL
                 await session.execute(text("VACUUM"))
+                await session.commit()
                 logger.info("Database vacuumed successfully")
 
         except Exception as e:
@@ -307,7 +284,6 @@ async def check_health(self) -> dict:
                 health["containers_count"] = len(containers)
 
                 # Count active execs
-                exec_repo = ExecRepository(session)
                 # In a full implementation, would count incomplete execs
                 health["active_execs"] = 0
 

src/mcp_devbench/managers/reconciliation_manager.py

@@ -1,18 +1,18 @@
 """Reconciliation manager for boot recovery and state synchronization."""
 
-from datetime import datetime, timedelta, timezone
+from datetime import datetime, timezone
 from typing import List
 
 from docker import DockerClient
-from docker.errors import APIError, NotFound
+from docker.errors import APIError
 from docker.models.containers import Container as DockerContainer
 
 from mcp_devbench.config import get_settings
 from mcp_devbench.models.containers import Container
 from mcp_devbench.models.database import get_db_manager
 from mcp_devbench.repositories.containers import ContainerRepository
-from mcp_devbench.repositories.execs import ExecRepository
 from mcp_devbench.utils import get_logger
+from mcp_devbench.utils.cleanup import cleanup_orphaned_transients
 from mcp_devbench.utils.docker_client import get_docker_client
 
 logger = get_logger(__name__)
@@ -228,50 +228,10 @@ async def _handle_orphaned_transients(self, session) -> int:
         Returns:
             Number of containers cleaned up
         """
-        cutoff_days = self.settings.transient_gc_days
-        cutoff = datetime.now(timezone.utc) - timedelta(days=cutoff_days)
-
         repo = ContainerRepository(session)
-        transients = await repo.list_by_status("stopped", persistent=False)
-
-        cleaned = 0
-        for container in transients:
-            # Check if container is old enough
-            if container.last_seen < cutoff:
-                try:
-                    # Try to remove Docker container if it exists
-                    try:
-                        docker_container = self.docker_client.containers.get(
-                            container.docker_id
-                        )
-                        docker_container.remove(force=True)
-                        logger.info(
-                            "Removed orphaned Docker container",
-                            extra={
-                                "container_id": container.id,
-                                "docker_id": container.docker_id,
-                            },
-                        )
-                    except NotFound:
-                        pass
-
-                    # Remove from database
-                    await repo.delete(container.id)
-                    cleaned += 1
-
-                    logger.info(
-                        "Cleaned up orphaned transient container",
-                        extra={
-                            "container_id": container.id,
-                            "age_days": (datetime.now(timezone.utc) - container.last_seen).days,
-                        },
-                    )
-                except Exception as e:
-                    logger.error(
-                        "Failed to clean up orphaned container",
-                        extra={"container_id": container.id, "error": str(e)},
-                    )
-
+        cleaned = await cleanup_orphaned_transients(
+            self.docker_client, repo, self.settings.transient_gc_days
+        )
         return cleaned
 
     async def _cleanup_incomplete_execs(self, session) -> None:
@@ -281,14 +241,19 @@ async def _cleanup_incomplete_execs(self, session) -> None:
         Args:
             session: Database session
         """
-        exec_repo = ExecRepository(session)
-
         # Get all incomplete execs (no end time)
         # For simplicity, we'll just log this for now
         # A full implementation would query for incomplete execs and mark them
         logger.info("Incomplete exec cleanup completed")
 
 
+# Global instance
+_reconciliation_manager: ReconciliationManager | None = None
+
+
 def get_reconciliation_manager() -> ReconciliationManager:
-    """Get reconciliation manager instance."""
-    return ReconciliationManager()
+    """Get or create reconciliation manager instance."""
+    global _reconciliation_manager
+    if _reconciliation_manager is None:
+        _reconciliation_manager = ReconciliationManager()
+    return _reconciliation_manager

src/mcp_devbench/managers/shutdown_coordinator.py

@@ -1,8 +1,6 @@
 """Shutdown coordinator for graceful server shutdown."""
 
 import asyncio
-import signal
-from typing import Callable
 
 from mcp_devbench.config import get_settings
 from mcp_devbench.managers.container_manager import ContainerManager
@@ -160,24 +158,3 @@ def get_shutdown_coordinator() -> ShutdownCoordinator:
     if _shutdown_coordinator is None:
         _shutdown_coordinator = ShutdownCoordinator()
     return _shutdown_coordinator
-
-
-def setup_signal_handlers(shutdown_handler: Callable[[], None]) -> None:
-    """
-    Set up signal handlers for graceful shutdown.
-
-    Args:
-        shutdown_handler: Function to call on SIGTERM/SIGINT
-    """
-
-    def signal_handler(signum, frame):
-        """Handle shutdown signals."""
-        sig_name = signal.Signals(signum).name
-        logger.info(f"Received {sig_name} signal, initiating shutdown")
-        shutdown_handler()
-
-    # Register handlers for SIGTERM and SIGINT
-    signal.signal(signal.SIGTERM, signal_handler)
-    signal.signal(signal.SIGINT, signal_handler)
-
-    logger.info("Signal handlers registered for graceful shutdown")

src/mcp_devbench/repositories/containers.py

@@ -70,7 +70,10 @@ async def get_by_identifier(self, identifier: str) -> Container | None:
         return await self.get_by_alias(identifier)
 
     async def list_by_status(
-        self, status: str | None = None, include_stopped: bool = False, persistent: bool | None = None
+        self,
+        status: str | None = None,
+        include_stopped: bool = False,
+        persistent: bool | None = None,
     ) -> List[Container]:
         """
         List containers by status.

src/mcp_devbench/repositories/execs.py

@@ -104,12 +104,10 @@ async def cleanup_old(self, hours: int = 24) -> int:
         """
         from datetime import timedelta, timezone
 
+        from sqlalchemy import delete
+
         cutoff = datetime.now(timezone.utc) - timedelta(hours=hours)
-        stmt = select(Exec).where(Exec.ended_at.is_not(None), Exec.ended_at < cutoff)
+        stmt = delete(Exec).where(Exec.ended_at.is_not(None), Exec.ended_at < cutoff)
         result = await self.session.execute(stmt)
-        old_execs = list(result.scalars().all())
-
-        for exec_entry in old_execs:
-            await self.delete(exec_entry.id)
 
-        return len(old_execs)
+        return result.rowcount

src/mcp_devbench/server.py

@@ -50,7 +50,6 @@
 from mcp_devbench.models.database import close_db, get_db_manager, init_db
 from mcp_devbench.repositories.attachments import AttachmentRepository
 from mcp_devbench.repositories.containers import ContainerRepository
-from mcp_devbench.repositories.execs import ExecRepository
 from mcp_devbench.utils import get_logger, setup_logging
 from mcp_devbench.utils.audit_logger import AuditEventType, get_audit_logger
 from mcp_devbench.utils.docker_client import close_docker_client, get_docker_client