From 2465d67154df62d00816cd2180e00985a64536e0 Mon Sep 17 00:00:00 2001 From: bradjin8 Date: Mon, 18 May 2026 16:02:58 -0400 Subject: [PATCH 1/4] fix: added none check for row["value"] to continue without crashing --- services/workspace_tabs.py | 2 + tests/test_workspace_tabs_null_bubble.py | 81 ++++++++++++++++++++++++ 2 files changed, 83 insertions(+) create mode 100644 tests/test_workspace_tabs_null_bubble.py diff --git a/services/workspace_tabs.py b/services/workspace_tabs.py index 62199e3..41350f9 100644 --- a/services/workspace_tabs.py +++ b/services/workspace_tabs.py @@ -97,6 +97,8 @@ def _safe_fetchall(query: str, params: tuple = ()) -> list: parts = row["key"].split(":") if len(parts) >= 3: bid = parts[2] + if row["value"] is None: + continue try: bubble_obj = Bubble.from_dict(json.loads(row["value"]), bubble_id=bid) bubble_map[bid] = bubble_obj.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..546afbf --- /dev/null +++ b/tests/test_workspace_tabs_null_bubble.py @@ -0,0 +1,81 @@ +"""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 adds an explicit None-guard before json.loads in the bubble +loading loop of services/workspace_tabs.py. +""" + +import json +import os +import sqlite3 +import tempfile +import unittest + + +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 (?, ?)", + ("bubbleId:composer-abc:bubble-null", None), # NULL value — the crash case + ) + # Also insert a healthy bubble so we verify good rows still load. + conn.execute( + "INSERT INTO cursorDiskKV ([key], value) VALUES (?, ?)", + ( + "bubbleId:composer-abc:bubble-ok", + json.dumps({"type": 1, "text": "hello", "createdAt": 0}), + ), + ) + 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.""" + from services.workspace_tabs import assemble_workspace_tabs + + 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) + + def test_healthy_bubbles_still_load_when_null_row_present(self): + """Healthy bubble rows in the same table are not dropped by the None-guard.""" + from services.workspace_tabs import assemble_workspace_tabs + + payload, status = assemble_workspace_tabs( + workspace_id="global", + workspace_path=self.workspace_path, + rules=[], + ) + self.assertEqual(status, 200) + self.assertIsInstance(payload, dict) + self.assertIn("tabs", payload) + + +if __name__ == "__main__": + unittest.main() From 2554b5cdba238a41b5ad1937bd59cb45743e1e52 Mon Sep 17 00:00:00 2001 From: bradjin8 Date: Mon, 18 May 2026 16:15:08 -0400 Subject: [PATCH 2/4] fix: review comments --- tests/test_workspace_tabs_null_bubble.py | 38 ++++++++++++++++++++---- 1 file changed, 33 insertions(+), 5 deletions(-) diff --git a/tests/test_workspace_tabs_null_bubble.py b/tests/test_workspace_tabs_null_bubble.py index 546afbf..d88c2ab 100644 --- a/tests/test_workspace_tabs_null_bubble.py +++ b/tests/test_workspace_tabs_null_bubble.py @@ -32,12 +32,26 @@ def setUp(self): "INSERT INTO cursorDiskKV ([key], value) VALUES (?, ?)", ("bubbleId:composer-abc:bubble-null", None), # NULL value — the crash case ) - # Also insert a healthy bubble so we verify good rows still load. + # Healthy bubble that should surface in the assembled tab. conn.execute( "INSERT INTO cursorDiskKV ([key], value) VALUES (?, ?)", ( "bubbleId:composer-abc:bubble-ok", - json.dumps({"type": 1, "text": "hello", "createdAt": 0}), + 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 (?, ?)", + ( + "composerData:composer-abc", + json.dumps({ + "name": "Test Chat", + "modelConfig": {"modelName": "gpt-4o"}, + "fullConversationHeadersOnly": [{"bubbleId": "bubble-ok", "type": 1}], + "lastUpdatedAt": 1739300000000, + "createdAt": 1739200000000, + }), ), ) conn.commit() @@ -53,7 +67,7 @@ def test_null_bubble_row_is_skipped_without_exception(self): from services.workspace_tabs import assemble_workspace_tabs try: - payload, status = assemble_workspace_tabs( + _payload, status = assemble_workspace_tabs( workspace_id="global", workspace_path=self.workspace_path, rules=[], @@ -64,7 +78,7 @@ def test_null_bubble_row_is_skipped_without_exception(self): self.assertEqual(status, 200) def test_healthy_bubbles_still_load_when_null_row_present(self): - """Healthy bubble rows in the same table are not dropped by the None-guard.""" + """The healthy bubble surfaces in a tab even when a NULL row is present.""" from services.workspace_tabs import assemble_workspace_tabs payload, status = assemble_workspace_tabs( @@ -74,7 +88,21 @@ def test_healthy_bubbles_still_load_when_null_row_present(self): ) self.assertEqual(status, 200) self.assertIsInstance(payload, dict) - self.assertIn("tabs", payload) + tabs = payload.get("tabs", []) + self.assertEqual(len(tabs), 1, "Expected exactly one tab for composer-abc") + + tab = tabs[0] + self.assertEqual(tab["id"], "composer-abc") + self.assertEqual(tab["title"], "Test Chat") + self.assertIn("bubbles", tab) + self.assertIn("codeBlockDiffs", tab) + + bubbles = tab["bubbles"] + self.assertEqual(len(bubbles), 1, "Expected exactly one bubble (null row skipped)") + bubble = bubbles[0] + self.assertEqual(bubble["type"], "user") + self.assertEqual(bubble["text"], "hello world") + self.assertIn("timestamp", bubble) if __name__ == "__main__": From 79de6ac9d53e98979614d16a9764c15fed4e3bcc Mon Sep 17 00:00:00 2001 From: bradjin8 Date: Tue, 19 May 2026 01:29:58 -0400 Subject: [PATCH 3/4] fix: review comments --- services/workspace_tabs.py | 49 +++++++++++++----------- tests/test_workspace_tabs_null_bubble.py | 45 ++++++++++++---------- 2 files changed, 52 insertions(+), 42 deletions(-) diff --git a/services/workspace_tabs.py b/services/workspace_tabs.py index 41350f9..ade2dc2 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 _loads_disk_kv_value(raw: Any) -> Any | None: + """Parse a cursorDiskKV ``value`` column; ``None`` if missing or unparseable.""" + 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,32 +107,30 @@ def _safe_fetchall(query: str, params: tuple = ()) -> list: parts = row["key"].split(":") if len(parts) >= 3: bid = parts[2] - if row["value"] is None: + parsed = _loads_disk_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 = _loads_disk_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. @@ -132,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 = _loads_disk_kv_value(row["value"]) if not isinstance(ctx, dict): continue @@ -153,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 = _loads_disk_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"]) @@ -180,16 +184,17 @@ def _safe_fetchall(query: str, params: tuple = ()) -> list: for row in composer_rows: composer_id = row["key"].split(":")[1] + parsed = _loads_disk_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 index d88c2ab..ddd2ba1 100644 --- a/tests/test_workspace_tabs_null_bubble.py +++ b/tests/test_workspace_tabs_null_bubble.py @@ -2,8 +2,8 @@ A cursorDiskKV row with a NULL value column previously caused json.loads(None) -> TypeError, which propagated as a 500 response. -The fix adds an explicit None-guard before json.loads in the bubble -loading loop of services/workspace_tabs.py. +The fix uses ``_loads_disk_kv_value`` in ``services/workspace_tabs.py`` so +NULL / unparseable cursorDiskKV values are skipped without raising. """ import json @@ -12,6 +12,13 @@ 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): @@ -30,13 +37,13 @@ def setUp(self): conn.execute("CREATE TABLE cursorDiskKV ([key] TEXT PRIMARY KEY, value TEXT)") conn.execute( "INSERT INTO cursorDiskKV ([key], value) VALUES (?, ?)", - ("bubbleId:composer-abc:bubble-null", None), # NULL value — the crash case + (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 (?, ?)", ( - "bubbleId:composer-abc:bubble-ok", + f"bubbleId:{COMPOSER_ID}:bubble-ok", json.dumps({"type": 1, "text": "hello world", "createdAt": 1739200000000}), ), ) @@ -44,7 +51,7 @@ def setUp(self): conn.execute( "INSERT INTO cursorDiskKV ([key], value) VALUES (?, ?)", ( - "composerData:composer-abc", + COMPOSER_KV_KEY, json.dumps({ "name": "Test Chat", "modelConfig": {"modelName": "gpt-4o"}, @@ -64,8 +71,6 @@ def tearDown(self): def test_null_bubble_row_is_skipped_without_exception(self): """assemble_workspace_tabs must not raise when a bubble row has NULL value.""" - from services.workspace_tabs import assemble_workspace_tabs - try: _payload, status = assemble_workspace_tabs( workspace_id="global", @@ -75,34 +80,34 @@ def test_null_bubble_row_is_skipped_without_exception(self): except TypeError as exc: self.fail(f"NULL bubble row raised TypeError: {exc}") - self.assertEqual(status, 200) + 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.""" - from services.workspace_tabs import assemble_workspace_tabs - payload, status = assemble_workspace_tabs( workspace_id="global", workspace_path=self.workspace_path, rules=[], ) - self.assertEqual(status, 200) - self.assertIsInstance(payload, dict) + 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, "Expected exactly one tab for composer-abc") + self.assertEqual(len(tabs), 1, f"Expected exactly one tab for {COMPOSER_ID}") tab = tabs[0] - self.assertEqual(tab["id"], "composer-abc") - self.assertEqual(tab["title"], "Test Chat") - self.assertIn("bubbles", tab) - self.assertIn("codeBlockDiffs", tab) + # 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") - self.assertEqual(bubble["text"], "hello world") - self.assertIn("timestamp", bubble) + 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__": From 29c310f37b16726e0179089ca6e728f6466380b6 Mon Sep 17 00:00:00 2001 From: bradjin8 Date: Tue, 19 May 2026 09:24:05 -0400 Subject: [PATCH 4/4] fix: _loads_disk_kv_value to _try_loads_kv_value --- services/workspace_tabs.py | 14 +++++++------- tests/test_workspace_tabs_null_bubble.py | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/services/workspace_tabs.py b/services/workspace_tabs.py index ade2dc2..5b8d559 100644 --- a/services/workspace_tabs.py +++ b/services/workspace_tabs.py @@ -37,8 +37,8 @@ def _extract_chat_id_from_code_block_diff_key(key: str) -> str | None: return m.group(1) if m else None -def _loads_disk_kv_value(raw: Any) -> Any | None: - """Parse a cursorDiskKV ``value`` column; ``None`` if missing or unparseable.""" +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: @@ -107,7 +107,7 @@ def _safe_fetchall(query: str, params: tuple = ()) -> list: parts = row["key"].split(":") if len(parts) >= 3: bid = parts[2] - parsed = _loads_disk_kv_value(row["value"]) + parsed = _try_loads_kv_value(row["value"]) if parsed is None: continue try: @@ -124,7 +124,7 @@ def _safe_fetchall(query: str, params: tuple = ()) -> list: chat_id = _extract_chat_id_from_code_block_diff_key(row["key"]) if not chat_id: continue - d = _loads_disk_kv_value(row["value"]) + d = _try_loads_kv_value(row["value"]) if not isinstance(d, dict): continue code_block_diff_map.setdefault(chat_id, []).append({ @@ -140,7 +140,7 @@ def _safe_fetchall(query: str, params: tuple = ()) -> list: if len(parts) < 2: continue chat_id = parts[1] - ctx = _loads_disk_kv_value(row["value"]) + ctx = _try_loads_kv_value(row["value"]) if not isinstance(ctx, dict): continue @@ -158,7 +158,7 @@ def _safe_fetchall(query: str, params: tuple = ()) -> list: project_layouts_map.setdefault(chat_id, []) for layout in layouts: if isinstance(layout, str): - layout = _loads_disk_kv_value(layout) + layout = _try_loads_kv_value(layout) if not isinstance(layout, dict): continue if isinstance(layout, dict) and layout.get("rootPath"): @@ -184,7 +184,7 @@ def _safe_fetchall(query: str, params: tuple = ()) -> list: for row in composer_rows: composer_id = row["key"].split(":")[1] - parsed = _loads_disk_kv_value(row["value"]) + parsed = _try_loads_kv_value(row["value"]) if parsed is None: continue try: diff --git a/tests/test_workspace_tabs_null_bubble.py b/tests/test_workspace_tabs_null_bubble.py index ddd2ba1..3ce69ce 100644 --- a/tests/test_workspace_tabs_null_bubble.py +++ b/tests/test_workspace_tabs_null_bubble.py @@ -2,7 +2,7 @@ A cursorDiskKV row with a NULL value column previously caused json.loads(None) -> TypeError, which propagated as a 500 response. -The fix uses ``_loads_disk_kv_value`` in ``services/workspace_tabs.py`` so +The fix uses ``_try_loads_kv_value`` in ``services/workspace_tabs.py`` so NULL / unparseable cursorDiskKV values are skipped without raising. """