diff --git a/services/workspace_tabs.py b/services/workspace_tabs.py index 62199e3..5b8d559 100644 --- a/services/workspace_tabs.py +++ b/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 diff --git a/tests/test_workspace_tabs_null_bubble.py b/tests/test_workspace_tabs_null_bubble.py new file mode 100644 index 0000000..3ce69ce --- /dev/null +++ b/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()