feat(tests): pytest endpoint coverage via Flask test client (closes #26) (#32)

* feat(tests): pytest endpoint coverage via Flask test client (closes #26)

Adds pytest-based integration tests for the four conversation-browsing
HTTP routes the dashboard reads. Tests run alongside the existing
``unittest discover`` suite — both gates required in CI.

  - tests/conftest.py: pytest fixtures.
    * ``workspace_storage`` builds a temp layout matching what
      ``utils/workspace_path.resolve_workspace_path`` reads:
        <tmp>/workspaceStorage/<HAPPY_WORKSPACE_ID>/state.vscdb (ItemTable
            with composer.composerData → allComposers row)
        <tmp>/workspaceStorage/<HAPPY_WORKSPACE_ID>/workspace.json
            ({"folder": <tmp>/happy-project})
        <tmp>/globalStorage/state.vscdb (cursorDiskKV with one
            composerData:<id> row + matching bubbleId:<cid>:<bid> row;
            bubble text contains a searchable sentinel)
        <tmp>/cli_chats/ (empty — overrides CLI_CHATS_PATH so the live
            ~/.cursor/chats doesn't leak into tests)
      Sets WORKSPACE_PATH + CLI_CHATS_PATH env vars for the duration of
      the test; restores prior values on cleanup.
    * ``client`` returns a Flask test client bound to the temp
      workspace_storage (app.create_app() with TESTING=True and empty
      EXCLUSION_RULES).
    * ``empty_workspace_client`` returns a Flask test client over an
      empty workspaceStorage — exercises the 404 / empty-list paths.

  - tests/test_api_endpoints.py: 11 tests across 4 endpoints.
    * GET /api/workspaces — list contains the seeded workspace (happy);
      empty storage returns ``[]`` (empty).
    * GET /api/workspaces/<id> — returns full record (happy); unknown
      id returns 404; ``global`` returns the "Other chats" virtual
      workspace.
    * GET /api/workspaces/<id>/tabs — returns tabs list with the seeded
      composer (happy); global also returns tabs; missing global
      storage returns 404.
    * GET /api/search?q=... — finds the seeded sentinel (happy);
      non-matching query returns ``{"results": []}``; missing q is
      tolerated (200 with empty or 400 — pin whichever shipped).

  - .github/workflows/tests.yml: add ``pytest>=8`` to the dependency
    install + new "Run pytest integration suite" step
    (``python -m pytest tests/ -v --tb=short``) immediately after the
    unittest step. Both suites are merge gates; pytest does not
    replace unittest (existing 178 tests use unittest.TestCase classes
    which pytest also collects, so the new pytest run actually executes
    178 + 11 = 189 tests on top of the unittest run's 178).

Verified locally:
  - ``python -m unittest discover tests -v``         → 178 pass
  - ``python -m pytest tests/ -v --tb=short``        → 189 pass
  - ``mypy --ignore-missing-imports --no-strict-optional`` on the new
    files introduces zero new errors (the 4 pre-existing errors in
    utils/exclusion_rules.py and api/search.py are unchanged).

Production code (api/, services/, utils/, models/) is untouched — this
PR is test infrastructure only.

* review: pin /api/search missing-q contract to 400 (#32)

CodeRabbit flagged that the permissive ``assert status_code in (200, 400)``
let either contract pass, so a drift between the two behaviours would go
unnoticed. The shipped implementation (api/search.py:74-75) returns
``400 with {"error": "No search query provided"}`` for missing or empty q.

Replaced the single permissive test with three concrete ones, each
asserting the same 400 + error-body contract:

  - test_missing_q_returns_400          — GET /api/search       (no q at all)
  - test_empty_q_returns_400            — GET /api/search?q=    (empty)
  - test_whitespace_only_q_returns_400  — GET /api/search?q=%20%20%20
                                          (api/search.py strips before checking)

191 pytest tests pass (was 189; +2 from the split).

* chore: drop issue-number reference from tests.yml comment

The "Closes #26." header in the pytest step comment belongs in the PR
description, not in CI config — comments shouldn't reference the task
that introduced them.

* review: six follow-ups on Brad's PR #32 pass

1. Scope pytest step to tests/test_api_endpoints.py — `pytest tests/`
   was also re-collecting the 178 unittest.TestCase subclasses already
   covered by the unittest discover step, ~2× CI minutes for no signal.

2. Extract HAPPY_* ID constants into tests/_fixture_ids.py and import
   from there in both conftest.py and test_api_endpoints.py. conftest
   is special to pytest and is not guaranteed importable as
   `tests.conftest` under --import-mode=importlib; a regular helper
   module sidesteps that.

3. Wrap both DB-seeding helpers (_make_global_state_db, _make_workspace)
   in contextlib.closing(sqlite3.connect(...)) so a mid-setup exception
   can't leak the handle and lock the tempdir against cleanup.

4. Tighten test_global_returns_tabs from a shape-only check into an
   isolation assertion: HAPPY_COMPOSER_ID is assigned to
   HAPPY_WORKSPACE_ID, so it must NOT appear in the /global bucket.

5. New TestExclusionRules class — three tests covering the
   EXCLUSION_RULES path on /api/workspaces and /api/search: workspace
   filtered by matching rule, negative control (non-matching rule
   leaves it visible), seeded chat dropped from search by rule.

6. empty_workspace_client now annotates the yield type as
   Generator[FlaskClient, None, None] to match workspace_storage's
   parameterised Generator[str, None, None].

Verified locally:
  - pytest tests/test_api_endpoints.py: 16 passed
  - pytest --import-mode=importlib: 16 passed
  - unittest discover tests: 178 passed, OK

Author: timon0305

.github/workflows/tests.yml

@@ -46,11 +46,19 @@ jobs:
         # system packages on Linux — out of scope for the unittest suite.
         run: |
           python -m pip install --upgrade pip
-          python -m pip install 'flask>=3.0' 'fpdf2>=2.7'
+          python -m pip install 'flask>=3.0' 'fpdf2>=2.7' 'pytest>=8'
 
       - name: Run unittest suite
         run: python -m unittest discover tests -v
 
+      - name: Run pytest integration suite
+        # Pytest fixtures (tests/conftest.py) build a temp workspaceStorage
+        # and exercise the Flask routes via app.test_client(). Scoped to the
+        # new endpoint file because `pytest tests/` would also re-collect the
+        # 178 unittest.TestCase subclasses already run in the step above —
+        # ~2× the CI minutes for zero extra signal.
+        run: python -m pytest tests/test_api_endpoints.py -v --tb=short
+
   # ── Typecheck: mypy ───────────────────────────────────────────────────────
   # Codebase already has type hints across most of the surface (~70+ typed
   # functions). Mypy runs in lenient mode (--ignore-missing-imports for

tests/_fixture_ids.py

@@ -0,0 +1,11 @@
+"""Shared composer/bubble/workspace IDs used by both the pytest fixture
+(`tests/conftest.py`) and the tests that introspect the seeded data.
+
+Lives in a regular module rather than inside conftest because conftest is
+special to pytest and is not guaranteed to be importable as `tests.conftest`
+under non-default import modes (e.g. `--import-mode=importlib`)."""
+from __future__ import annotations
+
+HAPPY_COMPOSER_ID = "cmp-happy"
+HAPPY_BUBBLE_ID = "bub-happy"
+HAPPY_WORKSPACE_ID = "ws-happy"

tests/conftest.py

@@ -0,0 +1,164 @@
+from __future__ import annotations
+
+import contextlib
+import json
+import os
+import sqlite3
+import sys
+import tempfile
+from pathlib import Path
+from typing import Generator
+
+import pytest
+from flask.testing import FlaskClient
+
+REPO_ROOT = str(Path(__file__).resolve().parent.parent)
+if REPO_ROOT not in sys.path:
+    sys.path.insert(0, REPO_ROOT)
+
+from app import create_app
+from tests._fixture_ids import (  # noqa: E402,F401  (re-export for legacy importers)
+    HAPPY_BUBBLE_ID,
+    HAPPY_COMPOSER_ID,
+    HAPPY_WORKSPACE_ID,
+)
+
+
+def _make_global_state_db(path: str) -> None:
+    """globalStorage/state.vscdb with one composerData + one bubbleId row."""
+    # contextlib.closing guarantees conn.close() even if an exec/commit raises
+    # mid-setup, so a failed fixture build can't leak a handle and lock the
+    # tempdir against cleanup.
+    with contextlib.closing(sqlite3.connect(path)) as conn:
+        conn.execute("CREATE TABLE cursorDiskKV ([key] TEXT PRIMARY KEY, value TEXT)")
+        conn.execute(
+            "INSERT INTO cursorDiskKV ([key], value) VALUES (?, ?)",
+            (
+                f"composerData:{HAPPY_COMPOSER_ID}",
+                json.dumps({
+                    "name": "Happy conversation",
+                    "createdAt": 1_715_000_000_000,
+                    "lastUpdatedAt": 1_715_000_500_000,
+                    "fullConversationHeadersOnly": [
+                        {"bubbleId": HAPPY_BUBBLE_ID, "type": 1},
+                    ],
+                    "modelConfig": {"modelName": "gpt-4o"},
+                }),
+            ),
+        )
+        conn.execute(
+            "INSERT INTO cursorDiskKV ([key], value) VALUES (?, ?)",
+            (
+                f"bubbleId:{HAPPY_COMPOSER_ID}:{HAPPY_BUBBLE_ID}",
+                json.dumps({
+                    "text": "find me by search term sentinel-grep",
+                    "type": "user",
+                    "createdAt": 1_715_000_400_000,
+                }),
+            ),
+        )
+        conn.commit()
+
+
+def _make_workspace(parent: str, workspace_id: str, project_folder: str) -> None:
+    """One per-workspace directory: workspace.json + minimal state.vscdb."""
+    ws_dir = os.path.join(parent, workspace_id)
+    os.makedirs(ws_dir, exist_ok=True)
+    with open(os.path.join(ws_dir, "workspace.json"), "w", encoding="utf-8") as f:
+        json.dump({"folder": project_folder}, f)
+    db = os.path.join(ws_dir, "state.vscdb")
+    with contextlib.closing(sqlite3.connect(db)) as conn:
+        conn.execute("CREATE TABLE ItemTable ([key] TEXT PRIMARY KEY, value TEXT)")
+        conn.execute(
+            "INSERT INTO ItemTable ([key], value) VALUES (?, ?)",
+            (
+                "composer.composerData",
+                json.dumps({"allComposers": [{"composerId": HAPPY_COMPOSER_ID}]}),
+            ),
+        )
+        conn.commit()
+
+
+@pytest.fixture
+def workspace_storage() -> Generator[str, None, None]:
+    """Build a temp workspaceStorage layout and yield the workspace path.
+
+    Layout:
+        <tmp>/workspaceStorage/<HAPPY_WORKSPACE_ID>/workspace.json
+        <tmp>/workspaceStorage/<HAPPY_WORKSPACE_ID>/state.vscdb
+        <tmp>/globalStorage/state.vscdb
+        <tmp>/cli_chats/                    (empty — keeps live ~/.cursor leaking out)
+
+    Sets ``WORKSPACE_PATH`` and ``CLI_CHATS_PATH`` env vars for the duration of
+    the test and restores them on cleanup.
+    """
+    with tempfile.TemporaryDirectory() as tmp:
+        ws_root = os.path.join(tmp, "workspaceStorage")
+        global_root = os.path.join(tmp, "globalStorage")
+        cli_root = os.path.join(tmp, "cli_chats")
+        os.makedirs(ws_root, exist_ok=True)
+        os.makedirs(global_root, exist_ok=True)
+        os.makedirs(cli_root, exist_ok=True)
+
+        project_folder = os.path.join(tmp, "happy-project")
+        os.makedirs(project_folder, exist_ok=True)
+
+        _make_workspace(ws_root, HAPPY_WORKSPACE_ID, project_folder)
+        _make_global_state_db(os.path.join(global_root, "state.vscdb"))
+
+        prior_ws = os.environ.get("WORKSPACE_PATH")
+        prior_cli = os.environ.get("CLI_CHATS_PATH")
+        os.environ["WORKSPACE_PATH"] = ws_root
+        os.environ["CLI_CHATS_PATH"] = cli_root
+        try:
+            yield ws_root
+        finally:
+            if prior_ws is None:
+                os.environ.pop("WORKSPACE_PATH", None)
+            else:
+                os.environ["WORKSPACE_PATH"] = prior_ws
+            if prior_cli is None:
+                os.environ.pop("CLI_CHATS_PATH", None)
+            else:
+                os.environ["CLI_CHATS_PATH"] = prior_cli
+
+
+@pytest.fixture
+def client(workspace_storage: str):
+    """Flask test client bound to the temp workspace_storage fixture."""
+    app = create_app()
+    app.config["TESTING"] = True
+    app.config["EXCLUSION_RULES"] = []
+    return app.test_client()
+
+
+@pytest.fixture
+def empty_workspace_client() -> Generator[FlaskClient, None, None]:
+    """Flask test client bound to a workspaceStorage with no workspaces.
+
+    Useful for 404 tests where the workspace id is unknown.
+    """
+    with tempfile.TemporaryDirectory() as tmp:
+        ws_root = os.path.join(tmp, "workspaceStorage")
+        cli_root = os.path.join(tmp, "cli_chats")
+        os.makedirs(ws_root, exist_ok=True)
+        os.makedirs(cli_root, exist_ok=True)
+
+        prior_ws = os.environ.get("WORKSPACE_PATH")
+        prior_cli = os.environ.get("CLI_CHATS_PATH")
+        os.environ["WORKSPACE_PATH"] = ws_root
+        os.environ["CLI_CHATS_PATH"] = cli_root
+        try:
+            app = create_app()
+            app.config["TESTING"] = True
+            app.config["EXCLUSION_RULES"] = []
+            yield app.test_client()
+        finally:
+            if prior_ws is None:
+                os.environ.pop("WORKSPACE_PATH", None)
+            else:
+                os.environ["WORKSPACE_PATH"] = prior_ws
+            if prior_cli is None:
+                os.environ.pop("CLI_CHATS_PATH", None)
+            else:
+                os.environ["CLI_CHATS_PATH"] = prior_cli

tests/test_api_endpoints.py

@@ -0,0 +1,200 @@
+from __future__ import annotations
+
+from app import create_app
+from tests._fixture_ids import HAPPY_BUBBLE_ID, HAPPY_COMPOSER_ID, HAPPY_WORKSPACE_ID
+from utils.exclusion_rules import _tokenize_rule
+
+
+# ---------------------------------------------------------------------------
+# GET /api/workspaces
+# ---------------------------------------------------------------------------
+
+class TestListWorkspaces:
+    def test_happy_path_returns_workspace_list(self, client):
+        response = client.get("/api/workspaces")
+        assert response.status_code == 200
+        body = response.get_json()
+        assert isinstance(body, list)
+
+        ids = [p["id"] for p in body]
+        assert HAPPY_WORKSPACE_ID in ids, f"expected {HAPPY_WORKSPACE_ID} in {ids}"
+
+        ws = next(p for p in body if p["id"] == HAPPY_WORKSPACE_ID)
+        assert "name" in ws
+        assert "conversationCount" in ws and isinstance(ws["conversationCount"], int)
+        assert "lastModified" in ws and "T" in ws["lastModified"]
+
+    def test_empty_storage_returns_empty_list(self, empty_workspace_client):
+        response = empty_workspace_client.get("/api/workspaces")
+        assert response.status_code == 200
+        assert response.get_json() == []
+
+
+# ---------------------------------------------------------------------------
+# GET /api/workspaces/<id>
+# ---------------------------------------------------------------------------
+
+class TestGetWorkspace:
+    def test_happy_path_returns_workspace_details(self, client):
+        response = client.get(f"/api/workspaces/{HAPPY_WORKSPACE_ID}")
+        assert response.status_code == 200
+        body = response.get_json()
+        assert body["id"] == HAPPY_WORKSPACE_ID
+        assert "name" in body
+        assert "folder" in body
+        assert "lastModified" in body and "T" in body["lastModified"]
+
+    def test_unknown_id_returns_404(self, client):
+        response = client.get("/api/workspaces/nonexistent-workspace-id")
+        assert response.status_code == 404
+        body = response.get_json()
+        assert "error" in body
+
+    def test_global_returns_other_chats(self, client):
+        response = client.get("/api/workspaces/global")
+        assert response.status_code == 200
+        body = response.get_json()
+        assert body["id"] == "global"
+        assert body["name"] == "Other chats"
+
+
+# ---------------------------------------------------------------------------
+# GET /api/workspaces/<id>/tabs
+# ---------------------------------------------------------------------------
+
+class TestGetWorkspaceTabs:
+    def test_happy_path_returns_tabs(self, client):
+        response = client.get(f"/api/workspaces/{HAPPY_WORKSPACE_ID}/tabs")
+        assert response.status_code == 200
+        body = response.get_json()
+        assert "tabs" in body and isinstance(body["tabs"], list)
+
+        tab_ids = [t["id"] for t in body["tabs"]]
+        assert HAPPY_COMPOSER_ID in tab_ids, f"expected {HAPPY_COMPOSER_ID} in {tab_ids}"
+
+        tab = next(t for t in body["tabs"] if t["id"] == HAPPY_COMPOSER_ID)
+        assert "title" in tab
+        assert "timestamp" in tab and isinstance(tab["timestamp"], int)
+        assert "bubbles" in tab and isinstance(tab["bubbles"], list)
+        # The seeded user bubble must be present
+        bubble_types = [b["type"] for b in tab["bubbles"]]
+        assert "user" in bubble_types
+
+    def test_global_returns_tabs(self, client):
+        response = client.get("/api/workspaces/global/tabs")
+        assert response.status_code == 200
+        body = response.get_json()
+        assert "tabs" in body and isinstance(body["tabs"], list)
+        # Isolation: HAPPY_COMPOSER_ID is assigned to HAPPY_WORKSPACE_ID via the
+        # local ItemTable allComposers row, so it must NOT also surface in the
+        # /global bucket. If it does, workspace-assignment is leaking unassigned
+        # composers into both buckets.
+        global_tab_ids = [t["id"] for t in body["tabs"]]
+        assert HAPPY_COMPOSER_ID not in global_tab_ids, (
+            f"{HAPPY_COMPOSER_ID} leaked into /global tabs: {global_tab_ids}"
+        )
+
+    def test_missing_global_storage_returns_404(self, empty_workspace_client):
+        response = empty_workspace_client.get("/api/workspaces/global/tabs")
+        assert response.status_code == 404
+        body = response.get_json()
+        assert "error" in body
+
+
+# ---------------------------------------------------------------------------
+# GET /api/search?q=...
+# ---------------------------------------------------------------------------
+
+class TestSearch:
+    def test_happy_path_finds_seeded_term(self, client):
+        response = client.get("/api/search?q=sentinel-grep")
+        assert response.status_code == 200
+        body = response.get_json()
+        assert "results" in body and isinstance(body["results"], list)
+        assert len(body["results"]) >= 1, f"expected sentinel match, got {body}"
+
+    def test_no_match_returns_empty_results(self, client):
+        response = client.get("/api/search?q=does-not-match-any-content-xyzzy")
+        assert response.status_code == 200
+        body = response.get_json()
+        assert "results" in body and body["results"] == []
+
+    def test_missing_q_returns_400(self, client):
+        response = client.get("/api/search")
+        assert response.status_code == 400
+        body = response.get_json()
+        assert "error" in body
+        assert body["error"] == "No search query provided"
+
+    def test_empty_q_returns_400(self, client):
+        response = client.get("/api/search?q=")
+        assert response.status_code == 400
+        body = response.get_json()
+        assert body.get("error") == "No search query provided"
+
+    def test_whitespace_only_q_returns_400(self, client):
+        # api/search.py strips q before the empty-check, so "   " is rejected.
+        response = client.get("/api/search?q=%20%20%20")
+        assert response.status_code == 400
+        body = response.get_json()
+        assert body.get("error") == "No search query provided"
+
+
+# ---------------------------------------------------------------------------
+# Exclusion rules — must be applied across endpoints
+# ---------------------------------------------------------------------------
+
+def _client_with_rules(rule_lines):
+    """Build a Flask test client whose EXCLUSION_RULES match the given lines.
+
+    The standard `client` fixture sets EXCLUSION_RULES = [] because no
+    rules file exists under the temp workspace. This helper builds a fresh
+    app on top of the same env (already pointed at workspace_storage) and
+    overrides the config with parsed rules — exercising the same code path
+    a real `exclusion-rules.txt` file would.
+    """
+    parsed = [_tokenize_rule(line) for line in rule_lines]
+    app = create_app()
+    app.config["TESTING"] = True
+    app.config["EXCLUSION_RULES"] = [r for r in parsed if r]
+    return app.test_client()
+
+
+class TestExclusionRules:
+    def test_workspace_matching_rule_is_filtered_out_of_list(self, workspace_storage):
+        # The seeded workspace's display name resolves to "happy-project"
+        # (the basename of the folder linked from workspace.json). A rule of
+        # "happy-project" must drop it from /api/workspaces entirely.
+        excluded_client = _client_with_rules(["happy-project"])
+        response = excluded_client.get("/api/workspaces")
+        assert response.status_code == 200
+        body = response.get_json()
+        ids = [w["id"] for w in body]
+        assert HAPPY_WORKSPACE_ID not in ids, (
+            f"exclusion rule did not filter {HAPPY_WORKSPACE_ID}; got {ids}"
+        )
+
+    def test_workspace_not_matching_rule_still_listed(self, workspace_storage):
+        # Negative control: a rule that doesn't match must leave the workspace
+        # visible, so the test above can't pass for the wrong reason
+        # (e.g. listing always returning []).
+        kept_client = _client_with_rules(["unrelated-project-name-xyzzy"])
+        response = kept_client.get("/api/workspaces")
+        assert response.status_code == 200
+        body = response.get_json()
+        ids = [w["id"] for w in body]
+        assert HAPPY_WORKSPACE_ID in ids, (
+            f"non-matching rule filtered the workspace; got {ids}"
+        )
+
+    def test_search_skips_conversations_matching_rule(self, workspace_storage):
+        # The seeded conversation's name is "Happy conversation". Excluding by
+        # "Happy" must drop the seeded match from /api/search even though the
+        # bubble text still contains "sentinel-grep".
+        excluded_client = _client_with_rules(["Happy"])
+        response = excluded_client.get("/api/search?q=sentinel-grep")
+        assert response.status_code == 200
+        body = response.get_json()
+        assert body.get("results") == [], (
+            f"exclusion rule did not filter seeded chat from search: {body}"
+        )