Skip to content

Commit de43d22

Browse files
committed
review: three more malformed-record + SQLite-error guards (#31, round 3)
- services/workspace_db.py: isinstance(c, dict) guard in _build_composer_id_to_workspace_id. A non-dict entry in ``allComposers`` would AttributeError on ``c.get("composerId")`` and silently discard the entire workspace's composer mapping. Test: tests/test_workspace_db_special_paths.py — list with None / string / int / valid dict, only the valid one maps. - services/workspace_resolver.py: wrap the per-composer ``messageRequestContext:*`` LIKE query in _infer_workspace_name_from_context with try/except sqlite3.Error. A corrupt cursorDiskKV table previously raised through and crashed listing routes that call this helper. Test: tests/test_workspace_name_db_errors.py — seeds globalStorage with cursorDiskKV missing, asserts helper returns None without propagating. - services/workspace_tabs.py: validate the parsed tool-call payload. ``_parse_tool_call`` may return None (or any non-dict); without the guard, ``tool_calls = [None]`` becomes a poison pill that crashes the display-text fallback (``NoneType.get('name')``) and drops the whole composer. Only store when isinstance(tool_call, dict); also guard the display-text fallback site. Test: tests/test_workspace_tabs_malformed_nested.py — patches _parse_tool_call to return None, asserts the composer still appears in the tabs response. 195 tests pass (was 192; +3 new).
1 parent ff25de3 commit de43d22

6 files changed

Lines changed: 170 additions & 8 deletions

services/workspace_db.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,8 @@ def _build_composer_id_to_workspace_id(workspace_path: str, workspace_entries: l
6060
all_composers = data.get("allComposers")
6161
if isinstance(all_composers, list):
6262
for c in all_composers:
63+
if not isinstance(c, dict):
64+
continue
6365
cid = c.get("composerId")
6466
if cid:
6567
mapping[cid] = entry["name"]

services/workspace_resolver.py

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -68,10 +68,13 @@ def _infer_workspace_name_from_context(workspace_path: str, workspace_id: str) -
6868
if not gconn:
6969
return None
7070
for cid in composer_ids:
71-
rows = gconn.execute(
72-
"SELECT value FROM cursorDiskKV WHERE key LIKE ?",
73-
(f"messageRequestContext:{cid}:%",),
74-
).fetchall()
71+
try:
72+
rows = gconn.execute(
73+
"SELECT value FROM cursorDiskKV WHERE key LIKE ?",
74+
(f"messageRequestContext:{cid}:%",),
75+
).fetchall()
76+
except sqlite3.Error:
77+
continue
7578
for row in rows:
7679
try:
7780
ctx = json.loads(row["value"])

services/workspace_tabs.py

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -250,7 +250,8 @@ def assemble_workspace_tabs(
250250
tfd = raw.get("toolFormerData")
251251
if isinstance(tfd, dict):
252252
tool_call = _parse_tool_call(tfd)
253-
tool_calls = [tool_call]
253+
if isinstance(tool_call, dict):
254+
tool_calls = [tool_call]
254255

255256
# Thinking
256257
thinking = None
@@ -271,9 +272,10 @@ def assemble_workspace_tabs(
271272
display_text = full_text.strip()
272273
if not display_text and tool_calls:
273274
tc = tool_calls[0]
274-
display_text = f"**Tool: {tc.get('name', 'unknown')}**"
275-
if tc.get("status"):
276-
display_text += f" ({tc['status']})"
275+
if isinstance(tc, dict):
276+
display_text = f"**Tool: {tc.get('name', 'unknown')}**"
277+
if tc.get("status"):
278+
display_text += f" ({tc['status']})"
277279
if not display_text and thinking:
278280
display_text = thinking
279281

tests/test_workspace_db_special_paths.py

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,34 @@ def test_open_global_db_handles_spaces(self):
7171
self.assertIsNotNone(row)
7272

7373

74+
class TestMalformedAllComposersEntry(unittest.TestCase):
75+
def test_non_dict_entries_skipped_healthy_one_mapped(self):
76+
with tempfile.TemporaryDirectory() as tmp:
77+
ws_dir = os.path.join(tmp, "ws-mixed")
78+
os.makedirs(ws_dir, exist_ok=True)
79+
db = os.path.join(ws_dir, "state.vscdb")
80+
conn = sqlite3.connect(db)
81+
conn.execute("CREATE TABLE ItemTable ([key] TEXT PRIMARY KEY, value TEXT)")
82+
conn.execute(
83+
"INSERT INTO ItemTable ([key], value) VALUES (?, ?)",
84+
(
85+
"composer.composerData",
86+
json.dumps({"allComposers": [
87+
None, # malformed: None
88+
"not a dict", # malformed: string
89+
42, # malformed: number
90+
{"composerId": "cid-real"}, # healthy
91+
]}),
92+
),
93+
)
94+
conn.commit()
95+
conn.close()
96+
97+
entries = [{"name": "ws-mixed", "workspaceJsonPath": ""}]
98+
mapping = _build_composer_id_to_workspace_id(tmp, entries)
99+
self.assertEqual(mapping, {"cid-real": "ws-mixed"})
100+
101+
74102
class TestOpenGlobalDbConnectFailure(unittest.TestCase):
75103
def test_sqlite_connect_error_yields_none_conn(self):
76104
with tempfile.TemporaryDirectory() as tmp:
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
from __future__ import annotations
2+
3+
import json
4+
import os
5+
import sqlite3
6+
import sys
7+
import tempfile
8+
import unittest
9+
from contextlib import contextmanager
10+
from unittest.mock import patch
11+
12+
REPO_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
13+
if REPO_ROOT not in sys.path:
14+
sys.path.insert(0, REPO_ROOT)
15+
16+
from services.workspace_resolver import _infer_workspace_name_from_context
17+
18+
19+
def _seed_local_state(workspace_path: str, workspace_id: str) -> None:
20+
"""Local state.vscdb with one composer.composerData row + composer ID."""
21+
ws_dir = os.path.join(workspace_path, workspace_id)
22+
os.makedirs(ws_dir, exist_ok=True)
23+
db = os.path.join(ws_dir, "state.vscdb")
24+
conn = sqlite3.connect(db)
25+
conn.execute("CREATE TABLE ItemTable ([key] TEXT PRIMARY KEY, value TEXT)")
26+
conn.execute(
27+
"INSERT INTO ItemTable VALUES (?, ?)",
28+
(
29+
"composer.composerData",
30+
json.dumps({"allComposers": [{"composerId": "cid-x"}]}),
31+
),
32+
)
33+
conn.commit()
34+
conn.close()
35+
36+
37+
class TestGlobalQueryErrorSwallowed(unittest.TestCase):
38+
def test_corrupt_cursordiskkv_does_not_propagate(self) -> None:
39+
with tempfile.TemporaryDirectory() as tmp:
40+
# _open_global_db reads ``<workspace_path>/../globalStorage`` — so
41+
# workspaceStorage must be a child of tmp, not tmp itself.
42+
ws_root = os.path.join(tmp, "workspaceStorage")
43+
os.makedirs(ws_root, exist_ok=True)
44+
_seed_local_state(ws_root, "ws-corrupt")
45+
46+
global_dir = os.path.join(tmp, "globalStorage")
47+
os.makedirs(global_dir, exist_ok=True)
48+
gdb = os.path.join(global_dir, "state.vscdb")
49+
conn = sqlite3.connect(gdb)
50+
# Schema deliberately missing cursorDiskKV so the LIKE query
51+
# inside _infer_workspace_name_from_context raises
52+
# sqlite3.OperationalError("no such table").
53+
conn.execute("CREATE TABLE other (x INTEGER)")
54+
conn.commit()
55+
conn.close()
56+
57+
try:
58+
result = _infer_workspace_name_from_context(ws_root, "ws-corrupt")
59+
except sqlite3.Error:
60+
self.fail("query error should be caught, not propagated")
61+
self.assertIsNone(result)
62+
63+
64+
if __name__ == "__main__":
65+
unittest.main()

tests/test_workspace_tabs_malformed_nested.py

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import sys
77
import tempfile
88
import unittest
9+
from unittest.mock import patch
910

1011
from flask import Flask
1112

@@ -149,5 +150,66 @@ def test_synthetic_falls_back_to_max_bubble_timestamp(self) -> None:
149150
self.assertLess(synthetic["timestamp"], now_ms - 10_000_000_000)
150151

151152

153+
def _seed_workspace_with_tool_former(parent: str) -> str:
154+
ws_root = os.path.join(parent, "workspaceStorage")
155+
global_root = os.path.join(parent, "globalStorage")
156+
os.makedirs(ws_root, exist_ok=True)
157+
os.makedirs(global_root, exist_ok=True)
158+
ws_dir = os.path.join(ws_root, "ws-a")
159+
os.makedirs(ws_dir, exist_ok=True)
160+
with open(os.path.join(ws_dir, "workspace.json"), "w") as f:
161+
json.dump({"folder": "/tmp/proj"}, f)
162+
sqlite3.connect(os.path.join(ws_dir, "state.vscdb")).close()
163+
164+
conn = sqlite3.connect(os.path.join(global_root, "state.vscdb"))
165+
conn.execute("CREATE TABLE cursorDiskKV ([key] TEXT PRIMARY KEY, value TEXT)")
166+
conn.execute(
167+
"INSERT INTO cursorDiskKV VALUES (?, ?)",
168+
(
169+
"composerData:cmp-t",
170+
json.dumps({
171+
"name": "Tab with toolFormerData",
172+
"createdAt": 1_715_000_000_000,
173+
"lastUpdatedAt": 1_715_000_500_000,
174+
"fullConversationHeadersOnly": [{"bubbleId": "b-t", "type": 2}],
175+
}),
176+
),
177+
)
178+
conn.execute(
179+
"INSERT INTO cursorDiskKV VALUES (?, ?)",
180+
(
181+
"bubbleId:cmp-t:b-t",
182+
json.dumps({
183+
"text": "assistant message",
184+
"createdAt": 1_715_000_400_000,
185+
"toolFormerData": {"name": "tool-x"},
186+
}),
187+
),
188+
)
189+
conn.commit()
190+
conn.close()
191+
return ws_root
192+
193+
194+
class TestParseToolCallNonDictReturn(unittest.TestCase):
195+
def test_non_dict_parse_result_does_not_drop_composer(self) -> None:
196+
app = Flask(__name__)
197+
app.config["TESTING"] = True
198+
app.config["EXCLUSION_RULES"] = []
199+
200+
with tempfile.TemporaryDirectory() as tmp:
201+
ws_root = _seed_workspace_with_tool_former(tmp)
202+
# Force _parse_tool_call to return None — the previous code
203+
# would have stored ``tool_calls = [None]`` and crashed in the
204+
# display-text fallback with ``NoneType.get``.
205+
with patch("services.workspace_tabs._parse_tool_call", return_value=None):
206+
with app.test_request_context("/api/workspaces/global/tabs"):
207+
payload, status = assemble_workspace_tabs("global", ws_root, rules=[])
208+
209+
self.assertEqual(status, 200)
210+
ids = [t["id"] for t in payload.get("tabs", [])]
211+
self.assertIn("cmp-t", ids)
212+
213+
152214
if __name__ == "__main__":
153215
unittest.main()

0 commit comments

Comments
 (0)