Fix: Skip NULL bubble rows in workspace tabs loader (closes #50)

services/workspace_tabs.py

@@ -37,6 +37,16 @@ def _extract_chat_id_from_code_block_diff_key(key: str) -> str | None:
     return m.group(1) if m else None
 
 
+def _try_loads_kv_value(raw: str | None) -> Any | None:
+    """Parse a cursorDiskKV ``value`` column; ``None`` on missing or unparseable input (no raise)."""
+    if raw is None:
+        return None
+    try:
+        return json.loads(raw)
+    except (json.JSONDecodeError, TypeError, ValueError):
+        return None
+
+
 def assemble_workspace_tabs(
     workspace_id: str,
     workspace_path: str,
@@ -97,30 +107,30 @@ def _safe_fetchall(query: str, params: tuple = ()) -> list:
             parts = row["key"].split(":")
             if len(parts) >= 3:
                 bid = parts[2]
+                parsed = _try_loads_kv_value(row["value"])
+                if parsed is None:
+                    continue
                 try:
-                    bubble_obj = Bubble.from_dict(json.loads(row["value"]), bubble_id=bid)
+                    bubble_obj = Bubble.from_dict(parsed, bubble_id=bid)
                     bubble_map[bid] = bubble_obj.raw
                 except SchemaError as e:
                     # Drift logged so the operator can chase disappearing
                     # bubbles instead of guessing. Bad row still skipped so the
                     # tabs endpoint can't 500 on one malformed bubble.
                     print(f"Schema drift in bubble {bid}: {e}")
-                except (json.JSONDecodeError, ValueError):
-                    pass
 
         # Load codeBlockDiffs
         for row in _safe_fetchall("SELECT key, value FROM cursorDiskKV WHERE key LIKE 'codeBlockDiff:%'"):
             chat_id = _extract_chat_id_from_code_block_diff_key(row["key"])
             if not chat_id:
                 continue
-            try:
-                d = json.loads(row["value"])
-                code_block_diff_map.setdefault(chat_id, []).append({
-                    **d,
-                    "diffId": row["key"].split(":")[2] if len(row["key"].split(":")) > 2 else None,
-                })
-            except Exception:
-                pass
+            d = _try_loads_kv_value(row["value"])
+            if not isinstance(d, dict):
+                continue
+            code_block_diff_map.setdefault(chat_id, []).append({
+                **d,
+                "diffId": row["key"].split(":")[2] if len(row["key"].split(":")) > 2 else None,
+            })
 
         # Load messageRequestContext rows once; build both
         # message_request_context_map and project_layouts_map from the same pass.
@@ -130,10 +140,7 @@ def _safe_fetchall(query: str, params: tuple = ()) -> list:
             if len(parts) < 2:
                 continue
             chat_id = parts[1]
-            try:
-                ctx = json.loads(row["value"])
-            except Exception:
-                continue
+            ctx = _try_loads_kv_value(row["value"])
             if not isinstance(ctx, dict):
                 continue
 
@@ -151,9 +158,8 @@ def _safe_fetchall(query: str, params: tuple = ()) -> list:
                 project_layouts_map.setdefault(chat_id, [])
                 for layout in layouts:
                     if isinstance(layout, str):
-                        try:
-                            layout = json.loads(layout)
-                        except Exception:
+                        layout = _try_loads_kv_value(layout)
+                        if not isinstance(layout, dict):
                             continue
                     if isinstance(layout, dict) and layout.get("rootPath"):
                         project_layouts_map[chat_id].append(layout["rootPath"])
@@ -178,16 +184,17 @@ def _safe_fetchall(query: str, params: tuple = ()) -> list:
 
         for row in composer_rows:
             composer_id = row["key"].split(":")[1]
+            parsed = _try_loads_kv_value(row["value"])
+            if parsed is None:
+                continue
             try:
-                composer = Composer.from_dict(json.loads(row["value"]), composer_id=composer_id)
+                composer = Composer.from_dict(parsed, composer_id=composer_id)
             except SchemaError as e:
                 # Drift skipped + logged so the two primary conversation
                 # paths (list_workspaces + get_workspace_tabs) agree on what
                 # counts as a valid composer.
                 print(f"Schema drift in composer {composer_id}: {e}")
                 continue
-            except (json.JSONDecodeError, TypeError, ValueError):
-                continue
             try:
                 cd = composer.raw
 

tests/test_workspace_tabs_null_bubble.py

@@ -0,0 +1,114 @@
+"""Regression test for issue #50: NULL bubble value crashes GET /tabs.
+
+A cursorDiskKV row with a NULL value column previously caused
+json.loads(None) -> TypeError, which propagated as a 500 response.
+The fix uses ``_try_loads_kv_value`` in ``services/workspace_tabs.py`` so
+NULL / unparseable cursorDiskKV values are skipped without raising.
+"""
+
+import json
+import os
+import sqlite3
+import tempfile
+import unittest
+
+from services.workspace_tabs import assemble_workspace_tabs
+
+# cursorDiskKV keys use typed prefixes; tabs[].id is the bare suffix only
+# (assemble_workspace_tabs: composer_id = row["key"].split(":")[1]).
+COMPOSER_ID = "composer-abc"
+COMPOSER_KV_KEY = f"composerData:{COMPOSER_ID}"
+
+
+class TestNullBubbleValueDoesNotCrashTabs(unittest.TestCase):
+    def setUp(self):
+        self.tmp = tempfile.TemporaryDirectory()
+        base = self.tmp.name
+
+        # Minimal workspaceStorage layout expected by assemble_workspace_tabs.
+        ws_dir = os.path.join(base, "workspaceStorage")
+        os.makedirs(ws_dir)
+
+        # Global storage with a cursorDiskKV table containing a NULL-value bubble row.
+        global_dir = os.path.join(base, "globalStorage")
+        os.makedirs(global_dir)
+        db_path = os.path.join(global_dir, "state.vscdb")
+        conn = sqlite3.connect(db_path)
+        conn.execute("CREATE TABLE cursorDiskKV ([key] TEXT PRIMARY KEY, value TEXT)")
+        conn.execute(
+            "INSERT INTO cursorDiskKV ([key], value) VALUES (?, ?)",
+            (f"bubbleId:{COMPOSER_ID}:bubble-null", None),  # NULL value — the crash case
+        )
+        # Healthy bubble that should surface in the assembled tab.
+        conn.execute(
+            "INSERT INTO cursorDiskKV ([key], value) VALUES (?, ?)",
+            (
+                f"bubbleId:{COMPOSER_ID}:bubble-ok",
+                json.dumps({"type": 1, "text": "hello world", "createdAt": 1739200000000}),
+            ),
+        )
+        # Composer referencing the healthy bubble — required for a tab to be built.
+        conn.execute(
+            "INSERT INTO cursorDiskKV ([key], value) VALUES (?, ?)",
+            (
+                COMPOSER_KV_KEY,
+                json.dumps({
+                    "name": "Test Chat",
+                    "modelConfig": {"modelName": "gpt-4o"},
+                    "fullConversationHeadersOnly": [{"bubbleId": "bubble-ok", "type": 1}],
+                    "lastUpdatedAt": 1739300000000,
+                    "createdAt": 1739200000000,
+                }),
+            ),
+        )
+        conn.commit()
+        conn.close()
+
+        self.workspace_path = ws_dir
+
+    def tearDown(self):
+        self.tmp.cleanup()
+
+    def test_null_bubble_row_is_skipped_without_exception(self):
+        """assemble_workspace_tabs must not raise when a bubble row has NULL value."""
+        try:
+            _payload, status = assemble_workspace_tabs(
+                workspace_id="global",
+                workspace_path=self.workspace_path,
+                rules=[],
+            )
+        except TypeError as exc:
+            self.fail(f"NULL bubble row raised TypeError: {exc}")
+
+        self.assertEqual(status, 200, "NULL bubble row must not turn tabs load into an error response")
+
+    def test_healthy_bubbles_still_load_when_null_row_present(self):
+        """The healthy bubble surfaces in a tab even when a NULL row is present."""
+        payload, status = assemble_workspace_tabs(
+            workspace_id="global",
+            workspace_path=self.workspace_path,
+            rules=[],
+        )
+        self.assertEqual(status, 200, "tabs endpoint must succeed when only the null bubble row is bad")
+        self.assertIsInstance(payload, dict, "tabs response must be a JSON object envelope")
+        tabs = payload.get("tabs", [])
+        self.assertEqual(len(tabs), 1, f"Expected exactly one tab for {COMPOSER_ID}")
+
+        tab = tabs[0]
+        # GET /tabs and workspace.html ?tab= use bare composer id, not the KV key.
+        self.assertEqual(tab["id"], COMPOSER_ID, "tab id must be bare composer id (KV key suffix only)")
+        self.assertNotEqual(tab["id"], COMPOSER_KV_KEY, "tab id must not include composerData: prefix")
+        self.assertEqual(tab["title"], "Test Chat", "composer name from seeded cursorDiskKV row")
+        self.assertIn("bubbles", tab, "tab payload must include bubbles for the conversation view")
+        self.assertIn("codeBlockDiffs", tab, "tab payload must include codeBlockDiffs field (may be empty)")
+
+        bubbles = tab["bubbles"]
+        self.assertEqual(len(bubbles), 1, "Expected exactly one bubble (null row skipped)")
+        bubble = bubbles[0]
+        self.assertEqual(bubble["type"], "user", "header type 1 maps to user bubble")
+        self.assertEqual(bubble["text"], "hello world", "healthy bubble text must surface in the tab")
+        self.assertIn("timestamp", bubble, "bubble must carry a timestamp for ordering/display")
+
+
+if __name__ == "__main__":
+    unittest.main()