fix: test DB isolation, Discord recovery, and over-mocked tests

Test data was bleeding into production tasks.db because
swarm.task_queue.models.DB_PATH (relative path) was never patched in
conftest.clean_database. Fixed by switching to absolute paths via
settings.repo_root and adding the missing module to the patching list.

Discord bot could leak orphaned clients on retry after ERROR state.
Added _cleanup_stale() to close stale client/task before each start()
attempt, with improved logging in the token watcher.

Rewrote test_paperclip_client.py to use httpx.MockTransport instead of
patching _get/_post/_delete — tests now exercise real HTTP status codes,
error handling, and JSON parsing. Added end-to-end test for
capture_error → create_task DB isolation.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

src/dashboard/app.py

@@ -235,10 +235,18 @@ async def _discord_token_watcher() -> None:
 
         if token:
             try:
+                logger.info(
+                    "Discord watcher: token found, attempting start (state=%s)",
+                    discord_bot.state.name,
+                )
                 success = await discord_bot.start(token=token)
                 if success:
                     logger.info("Discord bot auto-started (token detected)")
                     return  # Done — stop watching
+                logger.warning(
+                    "Discord watcher: start() returned False (state=%s)",
+                    discord_bot.state.name,
+                )
             except Exception as exc:
                 logger.warning("Discord auto-start failed: %s", exc)
 

src/dashboard/routes/work_orders.py

@@ -9,13 +9,14 @@ from pathlib import Path
 from fastapi import APIRouter, Form, HTTPException, Request
 from fastapi.responses import HTMLResponse
 
+from config import settings
 from dashboard.templating import templates
 
 logger = logging.getLogger(__name__)
 
 router = APIRouter(tags=["work-orders"])
 
-DB_PATH = Path("data/work_orders.db")
+DB_PATH = Path(settings.repo_root) / "data" / "work_orders.db"
 
 PRIORITIES = ["low", "medium", "high", "critical"]
 CATEGORIES = ["bug", "feature", "suggestion", "maintenance", "security"]

src/integrations/chat_bridge/vendors/discord.py

@@ -134,6 +134,12 @@ class DiscordVendor(ChatPlatform):
             logger.warning("Discord bot: no token configured, skipping start.")
             return False
 
+        # Clean up any stale client/task from a previous failed attempt
+        # so we don't leak background tasks or hold orphaned connections.
+        # Must happen before the import check — the old client could exist
+        # from when discord.py was available.
+        await self._cleanup_stale()
+
         try:
             import discord
         except ImportError:
@@ -163,9 +169,10 @@ class DiscordVendor(ChatPlatform):
                     logger.info("Discord bot connected (%d guilds).", self._guild_count)
                     return True
                 if self._state == PlatformState.ERROR:
+                    logger.warning("Discord bot: entered ERROR state during connection.")
                     return False
 
-            logger.warning("Discord bot: connection timed out.")
+            logger.warning("Discord bot: connection timed out after 15s.")
             self._state = PlatformState.ERROR
             return False
 
@@ -176,6 +183,23 @@ class DiscordVendor(ChatPlatform):
             self._client = None
             return False
 
+    async def _cleanup_stale(self) -> None:
+        """Close any orphaned client/task from a previous failed start."""
+        if self._client and not self._client.is_closed():
+            try:
+                await self._client.close()
+            except Exception:
+                pass
+            self._client = None
+
+        if self._task and not self._task.done():
+            self._task.cancel()
+            try:
+                await self._task
+            except (asyncio.CancelledError, Exception):
+                pass
+            self._task = None
+
     async def stop(self) -> None:
         """Gracefully disconnect the Discord bot."""
         if self._client and not self._client.is_closed():

src/swarm/task_queue/models.py

@@ -13,7 +13,14 @@ from pathlib import Path
 
 logger = logging.getLogger(__name__)
 
-DB_PATH = Path("data/tasks.db")
+# Use absolute path via settings.repo_root so tests can reliably redirect it
+# and relative-path CWD differences don't cause DB leaks.
+try:
+    from config import settings as _settings
+
+    DB_PATH = Path(_settings.repo_root) / "data" / "tasks.db"
+except Exception:
+    DB_PATH = Path("data/tasks.db")
 
 
 @dataclass

tests/conftest.py

@@ -104,10 +104,14 @@ def clean_database(tmp_path):
         except Exception:
             pass
 
-    # Redirect task queue and work orders DBs to temp dir
+    # Redirect task queue and work orders DBs to temp dir.
+    # IMPORTANT: swarm.task_queue.models also has a DB_PATH that writes to
+    # tasks.db — it MUST be patched too, or error_capture.capture_error()
+    # will write test data to the production database.
     for mod_name, tmp_db in [
         ("dashboard.routes.tasks", tmp_tasks_db),
         ("dashboard.routes.work_orders", tmp_work_orders_db),
+        ("swarm.task_queue.models", tmp_tasks_db),
     ]:
         try:
             mod = __import__(mod_name, fromlist=["DB_PATH"])

tests/infrastructure/test_error_capture.py

@@ -1,5 +1,7 @@
 """Tests for infrastructure.error_capture module."""
 
+import sqlite3
+
 from infrastructure.error_capture import (
     _dedup_cache,
     _get_git_context,
@@ -117,5 +119,37 @@ class TestCaptureError:
         except RuntimeError as e:
             capture_error(e, source="test_module", context={"path": "/api/foo"})
 
+    def test_capture_creates_task_in_temp_db(self):
+        """capture_error should write a task to the isolated temp DB, not production.
+
+        This validates that conftest.clean_database properly redirects
+        swarm.task_queue.models.DB_PATH to the per-test temp directory.
+        """
+        _dedup_cache.clear()
+
+        try:
+            raise ValueError("Test error for DB isolation check")
+        except ValueError as e:
+            task_id = capture_error(e, source="test")
+
+        # task_id can be None if error_feedback_enabled is False — that's fine,
+        # but if it was created, verify it landed in the temp DB
+        if task_id:
+            import swarm.task_queue.models as tq_models
+
+            db_path = tq_models.DB_PATH
+            conn = sqlite3.connect(str(db_path))
+            try:
+                row = conn.execute(
+                    "SELECT id, title FROM tasks WHERE id = ?", (task_id,)
+                ).fetchone()
+                assert row is not None, (
+                    f"Task {task_id} not found in DB at {db_path} — "
+                    "check conftest.clean_database patches swarm.task_queue.models.DB_PATH"
+                )
+                assert "[BUG]" in row[1]
+            finally:
+                conn.close()
+
     def teardown_method(self):
         _dedup_cache.clear()

tests/integrations/test_discord_vendor.py

@@ -96,6 +96,42 @@ class TestDiscordVendor:
         await vendor.stop()
         assert vendor.state == PlatformState.DISCONNECTED
 
+    @pytest.mark.asyncio
+    async def test_cleanup_stale_closes_orphaned_client(self):
+        """_cleanup_stale should close any leftover client from a failed start."""
+        from integrations.chat_bridge.vendors.discord import DiscordVendor
+
+        vendor = DiscordVendor()
+        mock_client = MagicMock()
+        mock_client.is_closed.return_value = False
+        mock_client.close = AsyncMock()
+        vendor._client = mock_client
+
+        await vendor._cleanup_stale()
+        mock_client.close.assert_called_once()
+        assert vendor._client is None
+
+    @pytest.mark.asyncio
+    async def test_start_from_error_state_cleans_up(self):
+        """start() after ERROR should clean up stale state before retrying."""
+        from integrations.chat_bridge.vendors.discord import DiscordVendor
+
+        vendor = DiscordVendor()
+        vendor._state = PlatformState.ERROR
+        # Stale client from previous failed attempt
+        mock_old_client = MagicMock()
+        mock_old_client.is_closed.return_value = False
+        mock_old_client.close = AsyncMock()
+        vendor._client = mock_old_client
+
+        # start() should clean up the old client even though discord.py import
+        # will fail (we're in test mode with MagicMock stub)
+        with patch.dict("sys.modules", {"discord": None}):
+            result = await vendor.start(token="fake-token")
+
+        assert result is False
+        mock_old_client.close.assert_called_once()
+
     def test_get_oauth2_url_no_client(self):
         from integrations.chat_bridge.vendors.discord import DiscordVendor
 

tests/integrations/test_paperclip_client.py

@@ -1,140 +1,213 @@
-"""Tests for the Paperclip API client."""
+"""Tests for the Paperclip API client.
 
-from unittest.mock import AsyncMock, patch
+Uses httpx.MockTransport so every test exercises the real HTTP path
+(_get/_post/_delete, status-code handling, JSON parsing, error paths)
+instead of patching the transport methods away.
+"""
 
-import pytest
+import json
+from unittest.mock import patch
+
+import httpx
 
 from integrations.paperclip.client import PaperclipClient
 from integrations.paperclip.models import CreateIssueRequest
 
-
-@pytest.fixture
-def client():
-    return PaperclipClient(base_url="http://fake:3100", api_key="test-key")
+# ---------------------------------------------------------------------------
+# Helpers
+# ---------------------------------------------------------------------------
 
 
-# ── health ───────────────────────────────────────────────────────────────────
+def _mock_transport(routes: dict[str, tuple[int, dict | list | None]]):
+    """Build an httpx.MockTransport from a {method+path: (status, body)} map.
+
+    Example:
+        _mock_transport({
+            "GET /api/health": (200, {"status": "ok"}),
+            "DELETE /api/issues/i1": (204, None),
+        })
+    """
+
+    def handler(request: httpx.Request) -> httpx.Response:
+        key = f"{request.method} {request.url.path}"
+        if key in routes:
+            status, body = routes[key]
+            content = json.dumps(body).encode() if body is not None else b""
+            return httpx.Response(
+                status, content=content, headers={"content-type": "application/json"}
+            )
+        return httpx.Response(404, json={"error": "not found"})
+
+    return httpx.MockTransport(handler)
 
 
-async def test_healthy_returns_true_on_success(client):
-    with patch.object(client, "_get", new_callable=AsyncMock, return_value={"status": "ok"}):
-        assert await client.healthy() is True
+def _client_with(routes: dict[str, tuple[int, dict | list | None]]) -> PaperclipClient:
+    """Create a PaperclipClient whose internal httpx.AsyncClient uses a mock transport."""
+    client = PaperclipClient(base_url="http://fake:3100", api_key="test-key")
+    client._client = httpx.AsyncClient(
+        transport=_mock_transport(routes),
+        base_url="http://fake:3100",
+        headers={"Accept": "application/json", "Authorization": "Bearer test-key"},
+    )
+    return client
 
 
-async def test_healthy_returns_false_on_failure(client):
-    with patch.object(client, "_get", new_callable=AsyncMock, return_value=None):
-        assert await client.healthy() is False
+# ---------------------------------------------------------------------------
+# health
+# ---------------------------------------------------------------------------
 
 
-# ── agents ───────────────────────────────────────────────────────────────────
+async def test_healthy_returns_true_on_200():
+    client = _client_with({"GET /api/health": (200, {"status": "ok"})})
+    assert await client.healthy() is True
 
 
-async def test_list_agents_returns_list(client):
+async def test_healthy_returns_false_on_500():
+    client = _client_with({"GET /api/health": (500, {"error": "down"})})
+    assert await client.healthy() is False
+
+
+async def test_healthy_returns_false_on_404():
+    client = _client_with({})  # no routes → 404
+    assert await client.healthy() is False
+
+
+# ---------------------------------------------------------------------------
+# agents
+# ---------------------------------------------------------------------------
+
+
+async def test_list_agents_parses_response():
     raw = [{"id": "a1", "name": "Codex", "role": "engineer", "status": "active"}]
-    with patch.object(client, "_get", new_callable=AsyncMock, return_value=raw):
-        with patch("integrations.paperclip.client.settings") as mock_settings:
-            mock_settings.paperclip_company_id = "comp-1"
-            agents = await client.list_agents(company_id="comp-1")
+    client = _client_with({"GET /api/companies/comp-1/agents": (200, raw)})
+    agents = await client.list_agents(company_id="comp-1")
     assert len(agents) == 1
     assert agents[0].name == "Codex"
+    assert agents[0].id == "a1"
 
 
-async def test_list_agents_graceful_on_none(client):
-    with patch.object(client, "_get", new_callable=AsyncMock, return_value=None):
-        agents = await client.list_agents(company_id="comp-1")
+async def test_list_agents_empty_on_server_error():
+    client = _client_with({"GET /api/companies/comp-1/agents": (503, None)})
+    agents = await client.list_agents(company_id="comp-1")
     assert agents == []
 
 
-# ── issues ───────────────────────────────────────────────────────────────────
+async def test_list_agents_graceful_on_404():
+    client = _client_with({})
+    agents = await client.list_agents(company_id="comp-1")
+    assert agents == []
 
 
-async def test_list_issues(client):
+# ---------------------------------------------------------------------------
+# issues
+# ---------------------------------------------------------------------------
+
+
+async def test_list_issues():
     raw = [{"id": "i1", "title": "Fix bug"}]
-    with patch.object(client, "_get", new_callable=AsyncMock, return_value=raw):
-        issues = await client.list_issues(company_id="comp-1")
+    client = _client_with({"GET /api/companies/comp-1/issues": (200, raw)})
+    issues = await client.list_issues(company_id="comp-1")
     assert len(issues) == 1
     assert issues[0].title == "Fix bug"
 
 
-async def test_get_issue(client):
+async def test_get_issue():
     raw = {"id": "i1", "title": "Fix bug", "description": "It's broken"}
-    with patch.object(client, "_get", new_callable=AsyncMock, return_value=raw):
-        issue = await client.get_issue("i1")
+    client = _client_with({"GET /api/issues/i1": (200, raw)})
+    issue = await client.get_issue("i1")
     assert issue is not None
     assert issue.id == "i1"
 
 
-async def test_get_issue_not_found(client):
-    with patch.object(client, "_get", new_callable=AsyncMock, return_value=None):
-        issue = await client.get_issue("nonexistent")
+async def test_get_issue_not_found():
+    client = _client_with({"GET /api/issues/nonexistent": (404, None)})
+    issue = await client.get_issue("nonexistent")
     assert issue is None
 
 
-async def test_create_issue(client):
+async def test_create_issue():
     raw = {"id": "i2", "title": "New feature"}
-    with patch.object(client, "_post", new_callable=AsyncMock, return_value=raw):
-        req = CreateIssueRequest(title="New feature")
-        issue = await client.create_issue(req, company_id="comp-1")
+    client = _client_with({"POST /api/companies/comp-1/issues": (201, raw)})
+    req = CreateIssueRequest(title="New feature")
+    issue = await client.create_issue(req, company_id="comp-1")
     assert issue is not None
     assert issue.id == "i2"
 
 
-async def test_create_issue_no_company_id(client):
+async def test_create_issue_no_company_id():
+    """Missing company_id returns None without making any HTTP call."""
+    client = _client_with({})
     with patch("integrations.paperclip.client.settings") as mock_settings:
         mock_settings.paperclip_company_id = ""
-        issue = await client.create_issue(
-            CreateIssueRequest(title="Test"),
-        )
+        issue = await client.create_issue(CreateIssueRequest(title="Test"))
     assert issue is None
 
 
-async def test_delete_issue(client):
-    with patch.object(client, "_delete", new_callable=AsyncMock, return_value=True):
-        result = await client.delete_issue("i1")
+async def test_delete_issue_returns_true_on_success():
+    client = _client_with({"DELETE /api/issues/i1": (204, None)})
+    result = await client.delete_issue("i1")
     assert result is True
 
 
-# ── comments ─────────────────────────────────────────────────────────────────
+async def test_delete_issue_returns_false_on_error():
+    client = _client_with({"DELETE /api/issues/i1": (500, None)})
+    result = await client.delete_issue("i1")
+    assert result is False
 
 
-async def test_add_comment(client):
+# ---------------------------------------------------------------------------
+# comments
+# ---------------------------------------------------------------------------
+
+
+async def test_add_comment():
     raw = {"id": "c1", "issue_id": "i1", "content": "Done"}
-    with patch.object(client, "_post", new_callable=AsyncMock, return_value=raw):
-        comment = await client.add_comment("i1", "Done")
+    client = _client_with({"POST /api/issues/i1/comments": (201, raw)})
+    comment = await client.add_comment("i1", "Done")
     assert comment is not None
     assert comment.content == "Done"
 
 
-async def test_list_comments(client):
+async def test_list_comments():
     raw = [{"id": "c1", "issue_id": "i1", "content": "LGTM"}]
-    with patch.object(client, "_get", new_callable=AsyncMock, return_value=raw):
-        comments = await client.list_comments("i1")
+    client = _client_with({"GET /api/issues/i1/comments": (200, raw)})
+    comments = await client.list_comments("i1")
     assert len(comments) == 1
 
 
-# ── goals ────────────────────────────────────────────────────────────────────
+# ---------------------------------------------------------------------------
+# goals
+# ---------------------------------------------------------------------------
 
 
-async def test_list_goals(client):
+async def test_list_goals():
     raw = [{"id": "g1", "title": "Ship MVP"}]
-    with patch.object(client, "_get", new_callable=AsyncMock, return_value=raw):
-        goals = await client.list_goals(company_id="comp-1")
+    client = _client_with({"GET /api/companies/comp-1/goals": (200, raw)})
+    goals = await client.list_goals(company_id="comp-1")
     assert len(goals) == 1
     assert goals[0].title == "Ship MVP"
 
 
-async def test_create_goal(client):
+async def test_create_goal():
     raw = {"id": "g2", "title": "Scale to 1000 users"}
-    with patch.object(client, "_post", new_callable=AsyncMock, return_value=raw):
-        goal = await client.create_goal("Scale to 1000 users", company_id="comp-1")
+    client = _client_with({"POST /api/companies/comp-1/goals": (201, raw)})
+    goal = await client.create_goal("Scale to 1000 users", company_id="comp-1")
     assert goal is not None
 
 
-# ── heartbeat runs ───────────────────────────────────────────────────────────
+# ---------------------------------------------------------------------------
+# heartbeat runs
+# ---------------------------------------------------------------------------
 
 
-async def test_list_heartbeat_runs(client):
+async def test_list_heartbeat_runs():
     raw = [{"id": "r1", "agent_id": "a1", "status": "running"}]
-    with patch.object(client, "_get", new_callable=AsyncMock, return_value=raw):
-        runs = await client.list_heartbeat_runs(company_id="comp-1")
+    client = _client_with({"GET /api/companies/comp-1/heartbeat-runs": (200, raw)})
+    runs = await client.list_heartbeat_runs(company_id="comp-1")
     assert len(runs) == 1
+
+
+async def test_list_heartbeat_runs_server_error():
+    client = _client_with({"GET /api/companies/comp-1/heartbeat-runs": (500, None)})
+    runs = await client.list_heartbeat_runs(company_id="comp-1")
+    assert runs == []