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

@@ -53,9 +53,11 @@ jobs:
 
       - name: Run pytest integration suite
         # Pytest fixtures (tests/conftest.py) build a temp workspaceStorage
-        # and exercise the Flask routes via app.test_client(). Runs alongside
-        # unittest, not instead of — both suites are merge gates.
-        run: python -m pytest tests/ -v --tb=short
+        # 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

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

@@ -1,5 +1,6 @@
 from __future__ import annotations
 
+import contextlib
 import json
 import os
 import sqlite3
@@ -9,51 +10,54 @@
 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
-
-
-HAPPY_COMPOSER_ID = "cmp-happy"
-HAPPY_BUBBLE_ID = "bub-happy"
-HAPPY_WORKSPACE_ID = "ws-happy"
+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."""
-    conn = sqlite3.connect(path)
-    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()
-    conn.close()
+    # 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:
@@ -63,17 +67,16 @@ def _make_workspace(parent: str, workspace_id: str, project_folder: str) -> None
     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")
-    conn = sqlite3.connect(db)
-    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()
-    conn.close()
+    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
@@ -130,7 +133,7 @@ def client(workspace_storage: str):
 
 
 @pytest.fixture
-def empty_workspace_client() -> Generator:
+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.

tests/test_api_endpoints.py

@@ -1,6 +1,8 @@
 from __future__ import annotations
 
-from tests.conftest import HAPPY_BUBBLE_ID, HAPPY_COMPOSER_ID, HAPPY_WORKSPACE_ID
+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
 
 
 # ---------------------------------------------------------------------------
@@ -83,6 +85,14 @@ def test_global_returns_tabs(self, client):
         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")
@@ -128,3 +138,63 @@ def test_whitespace_only_q_returns_400(self, client):
         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}"
+        )