Commit 81a4010
authored
* refactor(api): split api/workspaces.py 1,407 → 142 LOC into services/ (closes #25)
The eval flagged api/workspaces.py as a 1,400+ LOC monolith co-locating
database access, dict transformation, workspace resolution, project
assignment heuristics, CLI session handling, and HTTP response formatting
in one file (Section 5.2). The get_workspace_tabs handler alone spanned
~450 lines; _determine_project_for_conversation was 120+ lines of
cascading fallbacks. Helpers (_read_json_file) were duplicated across
api/workspaces.py and api/composers.py. This refactor extracts the file
into focused service modules and leaves three thin route handlers.
Behaviour-preserving: same JSON response shapes, same URL routes, same
sort orders. All 178 existing tests pass without modification; a small
back-compat shim re-exports _infer_invalid_workspace_aliases,
_determine_project_for_conversation, and _infer_workspace_name_from_context
from api/workspaces so existing test imports continue to work
(production callers should import from services/ directly).
Extraction layout (matches the order in #25's spec for minimal merge-
conflict risk against parallel work):
utils/workspace_descriptor.py — pure helpers; one _read_json_file
consolidating the duplicate copies
from api/workspaces.py and
api/composers.py.
services/workspace_db.py — _open_global_db, _collect_workspace
_entries, _build_composer_id_to_
workspace_id, _collect_invalid_
workspace_ids.
services/workspace_resolver.py — _determine_project_for_conversation,
_infer_invalid_workspace_aliases,
_create_project_name_to_workspace_id
_map, _create_workspace_path_to_id_
map, _get_workspace_display_name,
_infer_workspace_name_from_context,
_get_project_from_file_path.
services/cli_tabs.py — _get_cli_workspace_tabs (the
CLI-session branch of the tabs route).
services/workspace_listing.py — list_workspace_projects (the entire
body of GET /api/workspaces).
services/workspace_tabs.py — assemble_workspace_tabs (the entire
body of GET /api/workspaces/<id>/tabs
for non-CLI workspaces).
api/workspaces.py is now 142 LOC: three route handlers that parse the
request, hand off to the service layer, and JSONify the result.
Verified locally:
- 178 unit tests pass without modification.
- mypy clean on the new modules (no new errors on touched files).
- Live smoke on real Cursor workspaceStorage:
* GET / -> HTTP 200
* GET /api/workspaces -> HTTP 200, 2 workspaces
* GET /api/workspaces/global/tabs -> HTTP 200, 5 tabs
* GET /api/search?q=test -> HTTP 200, 7 results
No exceptions, no behaviour change observed.
* review: address CodeRabbit findings on PR #31 (5 fixes + tests)
Five CodeRabbit findings on the api/workspaces.py split, addressed
together with regression coverage:
1. services/cli_tabs.py: wrap ``messages_to_bubbles(...)`` in try/except.
``traverse_blobs(...)`` was already guarded; one malformed session
from this second call would 500 the whole tabs endpoint instead of
being skipped like the traverse failures.
Test: tests/test_cli_tabs.py mocks one session to throw, verifies the
endpoint returns 200 with the failing session skipped and the healthy
session intact.
2. services/workspace_db.py: build SQLite ``file:`` URIs via
``Path(...).resolve().as_uri()`` instead of f-string interpolation.
The naive form breaks on paths containing spaces or other reserved
characters. The resolver code already used the safe form; aligning
the db helpers here.
Test: tests/test_workspace_db_special_paths.py creates a workspace
inside ``Cursor User/workspaceStorage/`` (path with a space),
verifies the mapping and global-DB open both succeed.
3. services/workspace_listing.py + workspace_tabs.py: accept
dict-shaped ``projectLayouts`` entries in addition to JSON-string
ones. The resolver code already handled both shapes; this brings
the listing/tabs paths in line so dict layouts no longer leave
``project_layouts_map`` empty (which silently fell composers
back to the "global" bucket).
Test: tests/test_project_layouts_dict_shape.py runs
``list_workspace_projects`` against both shapes, asserts the
composer is routed to its workspace in both.
4. services/workspace_resolver.py: ``_get_project_from_file_path``
now uses ``os.path.commonpath`` instead of ``startswith`` so a
workspace rooted at ``/repo/app`` doesn't sibling-match
``/repo/app2/...``. ``ValueError`` from ``commonpath`` (e.g.
mismatched drive on Windows) is caught as "not within".
Test: tests/test_project_path_boundary.py covers the sibling-prefix
case, the file-outside-any-workspace case, and the regular
inside-workspace case.
5. services/workspace_tabs.py: mark synthetic "Tool Action" bubbles
with ``synthetic=True`` and skip them in the response-time pass.
Synthetic bubbles get a fresh ``datetime.now()`` timestamp; without
the flag they were treated as AI responses from the last user
message, inflating ``responseTimeMs`` / ``totalResponseTimeMs``
with values unrelated to model latency. The flag stays internal —
the tab serializer only copies type/text/timestamp/metadata to
the wire payload.
186 tests pass (was 178; +8 new).
* review: harden malformed-record paths across services (#31, round 2)
Five CodeRabbit findings on PR #31, all the same shape — one bad nested
record (missing key, wrong type, transient I/O) was bubbling out to the
outermost ``except`` and dropping a whole workspace/tab/composer/CLI
project instead of just skipping the bad record.
1. services/cli_tabs.py: guard session.get("session_id") before use.
Previously session["session_id"] could KeyError and 500 the whole
tabs endpoint.
2. services/workspace_db.py: wrap sqlite3.connect(...) in try/except in
_open_global_db. A corrupt DB or transient I/O failure now yields
(None, path) — same shape as the missing-file branch — instead of
raising into the caller. All existing callers branch on
``if not conn:`` so they degrade to the no-global-storage path.
3. services/workspace_listing.py: CLI-section loop now uses
s.get("session_id") + s.get("meta") or {}; the original
s["session_id"] / s["meta"] would KeyError on a malformed session
and drop every CLI project. Also added isinstance(h, dict) to the
has_bubbles generator for the same reason.
4. services/workspace_tabs.py + services/workspace_resolver.py: 6
isinstance(..., dict) guards on nested-record loops (header,
terminalFiles, attachedFoldersListDirResults, files inside folder,
cursorRules, summarizedComposers, plus the two header loops in
_determine_project_for_conversation). One bad nested record now
skips that record instead of AttributeError-ing and dropping the
entire composer.
5. services/workspace_tabs.py: synthetic "Tool Action" bubbles no
longer get datetime.now() as their timestamp — they now use
to_epoch_ms(diff.timestamp) or to_epoch_ms(diff.createdAt) or
max(real_bubble_timestamps). The previous timestamp made every
synthetic bubble look newer than the entire transcript and
forced it to sort to the end.
Regression coverage:
- tests/test_cli_tabs.py: malformed session missing session_id
- tests/test_workspace_db_special_paths.py: sqlite3.connect raises
- tests/test_workspace_listing_cli.py: CLI listing malformed session
- tests/test_workspace_tabs_malformed_nested.py: non-dict header
doesn't drop composer; synthetic bubble uses diff timestamp;
synthetic bubble falls back to max real-bubble timestamp
192 tests pass (was 186; +6 new).
* 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).
* review: wrap cursorDiskKV reads in _safe_fetchall (#31, round 4)
CodeRabbit flagged that the five ``global_db.execute(...).fetchall()``
calls in assemble_workspace_tabs could raise sqlite3.Error (e.g. a
missing or corrupt cursorDiskKV table) and abort the whole tabs
endpoint. Matches the resilience pattern already applied to
_infer_workspace_name_from_context in round 3.
Add a local ``_safe_fetchall(query, params=()) -> list`` helper inside
assemble_workspace_tabs that catches sqlite3.Error and returns []. All
five call sites — bubbleId LIKE, codeBlockDiff LIKE, two
messageRequestContext LIKEs, and the composerData fetch — route
through it. The route now returns ``{"tabs": []}, 200`` instead of
500-ing when the schema is corrupt.
Regression: tests/test_workspace_tabs_sql_errors.py seeds a real
globalStorage/state.vscdb without cursorDiskKV → asserts the response
is ({"tabs": []}, 200) and no sqlite3.Error propagates.
196 tests pass (was 195; +1 new).
* review: narrow excepts + drop duplicate queries / synthetic bubbles (#31, round 5)
Eight findings from Brad's review, addressed together. Recurring shape:
broad ``except Exception`` blocks were swallowing not just the realistic
failure (KeyError on missing dict field, sqlite3.Error on corrupt table,
OSError on missing file) but also bugs we'd want to know about. Each
catch is now narrowed to the actual failure mode the surrounding code
intends to handle, with regression coverage to pin the contract.
1. services/workspace_db.py:_build_composer_id_to_workspace_id —
narrow sqlite3.Error catch on the per-workspace state.vscdb open,
json.JSONDecodeError / ValueError catch on the row parse. Round-3
work; one site Brad flagged in the same pass.
Test: tests/test_workspace_db_special_paths.py::
TestBuildComposerMappingCorruptDb.
2. services/workspace_resolver.py:_infer_workspace_name_from_context —
same narrowing on the local-DB query (lconn.execute fetchone).
Test: tests/test_workspace_name_db_errors.py::
TestLocalQueryErrorSwallowed.
3. services/workspace_listing.py:list_workspace_projects — local
_safe_fetchall(query, params=()) helper routes the three
cursorDiskKV LIKE queries (composerData, messageRequestContext,
bubbleId) through sqlite3.Error → []. Same pattern as
workspace_tabs.py round 4.
Test: tests/test_workspace_listing_sql_errors.py.
4. api/workspaces.py:get_workspace CLI branch — replaced
cp["workspace_name"] / cp["last_updated_ms"] / cp["workspace_path"]
with .get(); added isinstance(cp, dict) skip to the lookup
generator. Brad's "unsafe dict access in CLI branch" finding.
Test: tests/test_get_workspace_cli_malformed.py (two cases).
5. services/workspace_listing.py CLI section — the projects loop now
guards cp non-dict, missing project_id, non-list sessions, and
non-dict session entries. Brad's "unsafe dict access in CLI
project loop" finding.
Test: tests/test_workspace_listing_cli.py::
TestMalformedCliProjectRecordSkipped.
6. services/cli_tabs.py:_get_cli_workspace_tabs — lookup generator
filters isinstance(cp, dict); project field access switched to
.get(); session iteration uses isinstance(session, dict). Brad's
"unsafe dict access in project lookup" finding on lines 17/22/25.
Test: tests/test_cli_tabs.py::TestMalformedCliProjectsListLookup.
7. services/workspace_tabs.py — diffs no longer double-represented.
Previously each diff was emitted both as ``tab.codeBlockDiffs``
(which the frontend at dashboard/static/js/download.js:128,274
and templates/workspace.html:135 actually reads) AND as a
synthetic ``**Tool Action:**`` AI bubble in ``tab.bubbles`` (no
consumer). Dropped the synthesis loop + the now-unused
``synthetic`` flag and its filter in the response-time pass.
Brad's "diffs double-represented" finding.
Test: tests/test_workspace_tabs_malformed_nested.py::
TestDiffsEmittedOnlyAsCodeBlockDiffs replaces the prior
synthetic-timestamp tests.
8. services/workspace_tabs.py — merged the two
``messageRequestContext:%`` LIKE queries (built message_request_
context_map and project_layouts_map respectively) into a single
pass that fills both maps. Brad's "messageRequestContext queried
twice" finding.
9. services/workspace_db.py — narrowed the two remaining broad
excepts: _collect_workspace_entries → OSError;
_collect_invalid_workspace_ids → (OSError, ValueError, KeyError,
TypeError). Brad's "broad silent except Exception: pass" finding.
207 tests pass (was 196; +11 across this round). The recurring
heuristic across all four rounds on this PR: every nested iteration
site needs ``isinstance(..., dict)``, every DB read needs
``sqlite3.Error`` catch, and broad excepts swallow the bugs we want
to know about.
* fix(types): close mypy gate on services/ after post-#35 strict enforcement
Two narrow fixes that resolve all 27 mypy errors exposed by master's
stricter typecheck gate (newly enforced when #35 removed
continue-on-error):
* services/workspace_tabs.py:195 — annotate `bubbles: list[dict[str, Any]] = []`
so the iteration variables `b` / `m` get treated as `dict[str, Any]`
rather than `dict[str, object]`. That single annotation kills 26 of
the 27 errors (all the "object has no attribute X" / "Value of type
object is not indexable" cluster around the metadata-aggregation loop).
* services/workspace_resolver.py:84 — `# type: ignore[call-overload]` on
the `row["value"]` access plus a comment pointing at the underlying
cause. sqlite3.Row supports string-key access at runtime when
`row_factory = sqlite3.Row` is set (which `_open_global_db` does),
but mypy's stdlib stub types Row as `tuple[Any, ...]` which only
accepts SupportsIndex.
Verified: mypy → Success, no issues found in 52 source files; ruff →
All checks passed; unittest → 207 / 207 OK.
1 parent 849f696 commit 81a4010
18 files changed
Lines changed: 2502 additions & 1328 deletions
File tree
- api
- services
- tests
- utils
Large diffs are not rendered by default.
Whitespace-only changes.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
0 commit comments