diff --git a/CHANGELOG.md b/CHANGELOG.md index df784e8f..be71806c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,80 @@ Three release tracks are maintained: ### Changed +- **`list` / `saved` verb bodies lifted across both kinds** ([Stage 2 + Verb Lift — `list` / `saved`]). The interactive + ``GET /v1/api/workstreams`` + ``GET /v1/api/workstreams/saved`` + and coord ``GET /v1/api/workstreams`` + ``GET /v1/api/workstreams/saved`` + handlers now share two factory bodies via + ``make_list_handler(cfg)`` and ``make_saved_handler(cfg)``. Three + new ``SessionEndpointConfig`` fields capture the per-kind + divergence: + + - ``list_resolve_title: ListResolveTitle | None`` — interactive + wires :func:`turnstone.core.memory.get_workstream_display_name` + so user-set aliases override the auto-generated ws-XXXX name in + the active list. Coord wires ``None`` (no alias surface). + - ``saved_state_filter: str | None`` — coord wires ``"closed"`` + so only explicitly-closed coordinators surface in the + saved-card grid. Interactive wires ``None`` (the storage + layer already excludes ``state='deleted'`` tombstones). + - ``saved_loaded_lookup: SavedLoadedLookup | None`` — coord-only + defence-in-depth filter that excludes ws_ids currently in the + in-memory pool (a row can be ``state='closed'`` for a few + seconds while the close-emit sequence races the in-memory pop). + Interactive wires ``None``. + + Five observable behaviour changes (all documented per kind): + + - **Active-list top-level key converges on ``"workstreams"``.** + Pre-lift coord returned ``{"coordinators": [...]}``; the lifted + body returns ``{"workstreams": [...]}`` for response-shape + parity with interactive. Coord is a 1.5.0aN-only surface — never + shipped stable — so SDK / frontend consumers swap once and + there's no compat shim or fallback (the convergence MUST land + before v1.5.0 stable per + ``project_unification_before_stable.md``). + - **Saved-list top-level key converges on ``"workstreams"``.** + Same shape change as the active list, applied to + ``GET /v1/api/workstreams/saved`` on coord. Coord-only surface; + no compat shim. + - **Active-list row key renames ``"id"`` → ``"ws_id"``** on + interactive. Pre-lift interactive used the bare ``id`` field + while every other shared verb on this surface (cancel, open, + events, create, saved-list) uses ``ws_id``. Convergence + eliminates the internal inconsistency. Frontend consumers + reading ``ws.id`` from the active-list response swap to + ``ws.ws_id``. Interactive HAS shipped stable across 1.0 / 1.3 / + 1.4, but the active-list endpoint is consumed by the bundled + JS only — there's no external SDK on those stable lines reading + the field. Browser-cache staleness is bounded by normal + static-asset reload on next page load. + - **Active-list row gains always-include fields.** ``user_id`` + was coord-only; ``kind`` + ``parent_ws_id`` were + interactive-only. Both kinds now populate all three. + ``parent_ws_id`` defaults to ``None`` for coord (coordinators + have no parent). + - **Storage / manager-lock work moved off the event loop on + interactive.** The lifted ``saved`` body always uses + ``asyncio.to_thread`` for ``list_workstreams_with_history``; + pre-lift interactive ran it inline (correlated COUNT subquery + can stall every other async handler on a cluster with thousands + of saved rows). Coord already used ``to_thread`` (perf-2 from + the saved-coordinators review); convergence lifts interactive + up. The active-list body also moves ``mgr.list_all`` + + per-row title resolution off the event loop on both kinds. + + Pydantic schemas: ``WorkstreamInfo.id`` renamed → ``ws_id``, + ``WorkstreamInfo.user_id`` field added. ``CoordinatorInfo`` and + ``CoordinatorListResponse`` removed (folded into the unified + ``WorkstreamInfo`` / ``ListWorkstreamsResponse``); ``console_spec`` + active-list endpoint now points at ``ListWorkstreamsResponse``. + OpenAPI spec snapshots regenerated. + + ``GET /v1/api/dashboard`` is **not** in the lift's scope and + still returns rows keyed on ``id``. A separate cleanup PR will + converge the dashboard row shape with the rest of the v1 surface. + - **`SessionManager.create` gains a deferred-emit option; lifted ``create`` HTTP handler eliminates the phantom create→close pair on coord rollback.** ``SessionManager.create`` now accepts diff --git a/sdk/typescript/openapi-console.json b/sdk/typescript/openapi-console.json index 9a9f2bd6..dc294361 100644 --- a/sdk/typescript/openapi-console.json +++ b/sdk/typescript/openapi-console.json @@ -4603,7 +4603,7 @@ "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/CoordinatorListResponse" + "$ref": "#/components/schemas/ListWorkstreamsResponse" } } } @@ -7550,49 +7550,6 @@ "title": "CoordinatorHistoryResponse", "type": "object" }, - "CoordinatorInfo": { - "description": "Per-coordinator row in the list response.", - "properties": { - "ws_id": { - "title": "Ws Id", - "type": "string" - }, - "name": { - "title": "Name", - "type": "string" - }, - "state": { - "title": "State", - "type": "string" - }, - "user_id": { - "title": "User Id", - "type": "string" - } - }, - "required": [ - "ws_id", - "name", - "state", - "user_id" - ], - "title": "CoordinatorInfo", - "type": "object" - }, - "CoordinatorListResponse": { - "description": "Response body for GET /v1/api/workstreams.", - "properties": { - "coordinators": { - "items": { - "$ref": "#/components/schemas/CoordinatorInfo" - }, - "title": "Coordinators", - "type": "array" - } - }, - "title": "CoordinatorListResponse", - "type": "object" - }, "CoordinatorOpenResponse": { "description": "Response body for POST /v1/api/workstreams/{ws_id}/open.", "properties": { diff --git a/sdk/typescript/openapi-server.json b/sdk/typescript/openapi-server.json index 2935575b..e9253ee9 100644 --- a/sdk/typescript/openapi-server.json +++ b/sdk/typescript/openapi-server.json @@ -2014,6 +2014,7 @@ "type": "object" }, "ListWorkstreamsResponse": { + "description": "Response body for ``GET /v1/api/workstreams`` on either kind.\n\nTop-level key is ``workstreams`` regardless of the kind serving\nthe request \u2014 pre-lift coord returned ``{\"coordinators\": [...]}``;\nconvergence lifted both kinds onto the same shape. Coord SDK /\nfrontend consumers branching on ``data.coordinators`` swap to\n``data.workstreams``.", "properties": { "workstreams": { "items": { @@ -2030,9 +2031,10 @@ "type": "object" }, "WorkstreamInfo": { + "description": "Active-list row shape, shared across both kinds.\n\nRenamed ``id`` \u2192 ``ws_id`` and added ``user_id`` in the Stage 2\n``list``/``saved`` verb lift so the active-list response shape\nmatches the rest of the v1 surface (every other shared verb's\npayload uses ``ws_id``). ``user_id`` was previously coord-only;\ninteractive now populates it too. SDK consumers reading\n``row.id`` should swap to ``row.ws_id``.", "properties": { - "id": { - "title": "Id", + "ws_id": { + "title": "Ws Id", "type": "string" }, "name": { @@ -2058,10 +2060,15 @@ ], "default": null, "title": "Parent Ws Id" + }, + "user_id": { + "default": "", + "title": "User Id", + "type": "string" } }, "required": [ - "id", + "ws_id", "name", "state" ], diff --git a/sdk/typescript/src/types.ts b/sdk/typescript/src/types.ts index c7ba9349..f0425bbf 100644 --- a/sdk/typescript/src/types.ts +++ b/sdk/typescript/src/types.ts @@ -166,9 +166,15 @@ export interface CloseWorkstreamRequest { } export interface WorkstreamInfo { - id: string; + // Renamed `id` → `ws_id` and added kind/parent_ws_id/user_id in + // the Stage 2 list-verb lift. Pre-1.5 readers branching on + // `row.id` should swap to `row.ws_id`. + ws_id: string; name: string; state: string; + kind: string; + parent_ws_id: string | null; + user_id: string; } export interface ListWorkstreamsResponse { diff --git a/sdk/typescript/tests/server.test.ts b/sdk/typescript/tests/server.test.ts index 5bf24fb1..58330b5e 100644 --- a/sdk/typescript/tests/server.test.ts +++ b/sdk/typescript/tests/server.test.ts @@ -26,7 +26,16 @@ function mockFetchError( describe("TurnstoneServer", () => { it("listWorkstreams returns parsed response", async () => { const fetchFn = mockFetch({ - workstreams: [{ id: "ws1", name: "test", state: "idle" }], + workstreams: [ + { + ws_id: "ws1", + name: "test", + state: "idle", + kind: "interactive", + parent_ws_id: null, + user_id: "u1", + }, + ], }); const client = new TurnstoneServer({ baseUrl: "http://test", @@ -34,7 +43,9 @@ describe("TurnstoneServer", () => { }); const resp = await client.listWorkstreams(); expect(resp.workstreams).toHaveLength(1); - expect(resp.workstreams[0].id).toBe("ws1"); + // Row key renamed id → ws_id in the Stage 2 list-verb lift. + expect(resp.workstreams[0].ws_id).toBe("ws1"); + expect(resp.workstreams[0].kind).toBe("interactive"); expect(fetchFn).toHaveBeenCalledWith( "http://test/v1/api/workstreams", expect.objectContaining({ method: "GET" }), diff --git a/tests/test_coordinator_end_to_end.py b/tests/test_coordinator_end_to_end.py index c011232f..a6c57598 100644 --- a/tests/test_coordinator_end_to_end.py +++ b/tests/test_coordinator_end_to_end.py @@ -39,7 +39,6 @@ from turnstone.console.server import ( _require_admin_coordinator, _require_coord_mgr, coordinator_detail, - coordinator_list, ) from turnstone.core.auth import AuthResult from turnstone.core.session_manager import SessionManager @@ -47,6 +46,7 @@ from turnstone.core.session_routes import ( SessionEndpointConfig, make_close_handler, make_create_handler, + make_list_handler, ) from turnstone.core.storage._sqlite import SQLiteBackend @@ -148,7 +148,11 @@ def _make_client( make_create_handler(_coord_endpoint_config, audit_emit=_audit_coordinator_create), methods=["POST"], ), - Route("/v1/api/workstreams", coordinator_list, methods=["GET"]), + Route( + "/v1/api/workstreams", + make_list_handler(_coord_endpoint_config), + methods=["GET"], + ), Route( "/v1/api/workstreams/{ws_id}/close", make_close_handler( @@ -204,7 +208,7 @@ def test_create_list_detail_lifecycle(tmp_path): # --- List: caller sees their own coordinator --- resp = client.get("/v1/api/workstreams", headers=_COORD_HEADERS) assert resp.status_code == 200, resp.text - coordinators = resp.json()["coordinators"] + coordinators = resp.json()["workstreams"] ids = {c["ws_id"] for c in coordinators} assert ws_id in ids @@ -213,7 +217,7 @@ def test_create_list_detail_lifecycle(tmp_path): mgr.create(user_id="other-user", name="not-mine") resp = client.get("/v1/api/workstreams", headers=_COORD_HEADERS) assert resp.status_code == 200 - names = {c["name"] for c in resp.json()["coordinators"]} + names = {c["name"] for c in resp.json()["workstreams"]} assert "not-mine" in names # --- Detail --- diff --git a/tests/test_coordinator_endpoints.py b/tests/test_coordinator_endpoints.py index 8db4d940..bd59588f 100644 --- a/tests/test_coordinator_endpoints.py +++ b/tests/test_coordinator_endpoints.py @@ -37,14 +37,13 @@ from turnstone.console.server import ( _coord_create_build_kwargs, _coord_create_post_install, _coord_create_validate_request, + _coord_saved_loaded_lookup, _require_admin_coordinator, _require_coord_mgr, cluster_ws_detail, coordinator_children, coordinator_detail, coordinator_history, - coordinator_list, - coordinator_saved, coordinator_tasks, ) from turnstone.core.attachments import ( @@ -65,10 +64,13 @@ from turnstone.core.session_routes import ( make_cancel_handler, make_close_handler, make_create_handler, + make_list_handler, make_open_handler, + make_saved_handler, make_send_handler, ) from turnstone.core.storage._sqlite import SQLiteBackend +from turnstone.core.workstream import WorkstreamKind # --------------------------------------------------------------------------- # Fixtures @@ -115,6 +117,10 @@ _coord_endpoint_config = SessionEndpointConfig( create_validate_request=_coord_create_validate_request, create_build_kwargs=_coord_create_build_kwargs, create_post_install=_coord_create_post_install, + list_resolve_titles=None, + list_kind=WorkstreamKind.COORDINATOR, + saved_state_filter="closed", + saved_loaded_lookup=_coord_saved_loaded_lookup, ) @@ -142,12 +148,16 @@ def _make_client( coord_create_handler, methods=["POST"], ), - Route("/v1/api/workstreams", coordinator_list, methods=["GET"]), + Route( + "/v1/api/workstreams", + make_list_handler(_coord_endpoint_config), + methods=["GET"], + ), # Literal path before the /{ws_id} routes below so Starlette # matches "saved" as the literal, not as a ws_id. Route( "/v1/api/workstreams/saved", - coordinator_saved, + make_saved_handler(_coord_endpoint_config), methods=["GET"], ), Route( @@ -338,6 +348,42 @@ def test_unresolvable_alias_returns_503(storage): _COORD_HEADERS = {"X-Test-User": "user-1", "X-Test-Perms": "admin.coordinator"} +def test_active_list_row_shape_includes_unified_fields(storage): + """Stage 2 list-verb-lift parity regression — coord active-list row + carries the always-include fields (ws_id, name, state, kind, + parent_ws_id, user_id) that the lifted ``make_list_handler`` + produces on every kind. SDK consumers don't have to branch on + kind to read any of these. Pre-lift coord returned a smaller + row ({ws_id, name, state, user_id}) under a different top-level + key (``coordinators`` vs ``workstreams``).""" + mgr = _build_mgr(storage) + mgr.create(user_id="u1", name="lifted-coord") + client = _make_client(storage, coord_mgr=mgr, registry=_fake_registry()) + resp = client.get("/v1/api/workstreams", headers=_COORD_HEADERS) + assert resp.status_code == 200 + body = resp.json() + # Top-level key converged on `workstreams`. + assert "workstreams" in body + assert "coordinators" not in body + rows = body["workstreams"] + assert len(rows) == 1 + row = rows[0] + # Always-include row shape — coord populates kind=COORDINATOR + # and parent_ws_id=None (coordinators have no parent today). + assert set(row.keys()) == { + "ws_id", + "name", + "state", + "kind", + "parent_ws_id", + "user_id", + } + assert row["name"] == "lifted-coord" + assert row["kind"] == "coordinator" + assert row["parent_ws_id"] is None + assert row["user_id"] == "u1" + + def test_create_returns_ws_id_and_records_audit(storage): mgr = _build_mgr(storage) client = _make_client(storage, coord_mgr=mgr, registry=_fake_registry()) @@ -561,7 +607,7 @@ def test_list_returns_cluster_wide(storage): resp = client.get("/v1/api/workstreams", headers=_COORD_HEADERS) assert resp.status_code == 200 body = resp.json() - names = {c["name"] for c in body["coordinators"]} + names = {c["name"] for c in body["workstreams"]} assert names == {"mine", "theirs"} @@ -616,7 +662,7 @@ def test_saved_returns_cluster_wide(saved_storage): client = _make_client(storage, coord_mgr=mgr, registry=_fake_registry()) resp = client.get("/v1/api/workstreams/saved", headers=_COORD_HEADERS) assert resp.status_code == 200 - assert {c["ws_id"] for c in resp.json()["coordinators"]} == {a, b} + assert {c["ws_id"] for c in resp.json()["workstreams"]} == {a, b} def test_saved_excludes_currently_loaded(saved_storage): @@ -638,7 +684,7 @@ def test_saved_excludes_currently_loaded(saved_storage): client = _make_client(storage, coord_mgr=mgr, registry=_fake_registry()) resp = client.get("/v1/api/workstreams/saved", headers=_COORD_HEADERS) assert resp.status_code == 200 - saved_ids = {c["ws_id"] for c in resp.json()["coordinators"]} + saved_ids = {c["ws_id"] for c in resp.json()["workstreams"]} assert closed_id in saved_ids assert loaded_ws.id not in saved_ids @@ -665,7 +711,7 @@ def test_saved_excludes_active_state_rows(saved_storage): client = _make_client(storage, coord_mgr=mgr, registry=_fake_registry()) resp = client.get("/v1/api/workstreams/saved", headers=_COORD_HEADERS) assert resp.status_code == 200 - saved_ids = {c["ws_id"] for c in resp.json()["coordinators"]} + saved_ids = {c["ws_id"] for c in resp.json()["workstreams"]} assert saved_ids == {closed_id} diff --git a/tests/test_sdk_server.py b/tests/test_sdk_server.py index 610ab5ce..86f87d2d 100644 --- a/tests/test_sdk_server.py +++ b/tests/test_sdk_server.py @@ -40,7 +40,7 @@ async def test_list_workstreams(): transport = _mock_transport( { "GET /v1/api/workstreams": _json_response( - {"workstreams": [{"id": "ws1", "name": "test", "state": "idle"}]} + {"workstreams": [{"ws_id": "ws1", "name": "test", "state": "idle"}]} ) } ) @@ -48,7 +48,8 @@ async def test_list_workstreams(): client = AsyncTurnstoneServer(httpx_client=hc) resp = await client.list_workstreams() assert len(resp.workstreams) == 1 - assert resp.workstreams[0].id == "ws1" + # Row key renamed id → ws_id in the Stage 2 list-verb lift. + assert resp.workstreams[0].ws_id == "ws1" @pytest.mark.anyio diff --git a/tests/test_sdk_sync.py b/tests/test_sdk_sync.py index 02947e8a..9e08e77c 100644 --- a/tests/test_sdk_sync.py +++ b/tests/test_sdk_sync.py @@ -73,7 +73,7 @@ def test_sync_server_list_workstreams(): """Sync server client delegates to async and returns correct model.""" def handler(request: httpx.Request) -> httpx.Response: - return _json_response({"workstreams": [{"id": "ws1", "name": "test", "state": "idle"}]}) + return _json_response({"workstreams": [{"ws_id": "ws1", "name": "test", "state": "idle"}]}) # We need to create the async client with a mock transport, # then wrap it in the sync client @@ -88,7 +88,8 @@ def test_sync_server_list_workstreams(): try: resp = server.list_workstreams() assert len(resp.workstreams) == 1 - assert resp.workstreams[0].id == "ws1" + # Row key renamed id → ws_id in the Stage 2 list-verb lift. + assert resp.workstreams[0].ws_id == "ws1" finally: server.close() diff --git a/tests/test_server_authz.py b/tests/test_server_authz.py index 1b022884..74e64c92 100644 --- a/tests/test_server_authz.py +++ b/tests/test_server_authz.py @@ -430,9 +430,50 @@ class TestListWorkstreamsTrustedTeamVisibility: # user-a now sees both. resp = client.get("/v1/api/workstreams", headers=_auth("user-a")) assert resp.status_code == 200 - ids = {w["id"] for w in resp.json()["workstreams"]} + # Row key renamed id → ws_id in the Stage 2 list-verb lift. + ids = {w["ws_id"] for w in resp.json()["workstreams"]} assert {ws_a, ws_b}.issubset(ids), ids + def test_active_list_row_shape_includes_unified_fields(self, app_client): + """Stage 2 list-verb-lift parity regression — interactive + active-list row carries the always-include fields (ws_id, + name, state, kind, parent_ws_id, user_id) that the lifted + ``make_list_handler`` produces on every kind. Mirrors the + coord-side ``test_active_list_row_shape_includes_unified_fields`` + in ``test_coordinator_endpoints.py`` so a future regression + that drops a field on either branch is caught.""" + client, _mgr = app_client + create_resp = client.post( + "/v1/api/workstreams/new", + json={"name": "shape-check"}, + headers=_auth("user-shape"), + ) + assert create_resp.status_code == 200 + ws_id = create_resp.json()["ws_id"] + + resp = client.get("/v1/api/workstreams", headers=_auth("user-shape")) + assert resp.status_code == 200 + body = resp.json() + assert "workstreams" in body + rows = [w for w in body["workstreams"] if w["ws_id"] == ws_id] + assert len(rows) == 1 + row = rows[0] + # Always-include row shape — interactive populates kind= + # INTERACTIVE; user_id is post-lift parity (was coord-only). + assert set(row.keys()) == { + "ws_id", + "name", + "state", + "kind", + "parent_ws_id", + "user_id", + } + assert row["kind"] == "interactive" + assert row["user_id"] == "user-shape" + # parent_ws_id is None for top-level interactive workstreams + # (only coord-spawned children carry it). + assert row["parent_ws_id"] is None + class TestDashboardTrustedTeamVisibility: def test_dashboard_aggregate_includes_all_owners(self, app_client): diff --git a/turnstone/api/console_schemas.py b/turnstone/api/console_schemas.py index 7502ba96..6f065699 100644 --- a/turnstone/api/console_schemas.py +++ b/turnstone/api/console_schemas.py @@ -1024,21 +1024,6 @@ class CoordinatorCreateResponse(BaseModel): attachment_ids: list[str] = Field(default_factory=list) -class CoordinatorInfo(BaseModel): - """Per-coordinator row in the list response.""" - - ws_id: str - name: str - state: str - user_id: str - - -class CoordinatorListResponse(BaseModel): - """Response body for GET /v1/api/workstreams.""" - - coordinators: list[CoordinatorInfo] = Field(default_factory=list) - - class CoordinatorDetailResponse(BaseModel): """Response body for GET /v1/api/workstreams/{ws_id}.""" diff --git a/turnstone/api/console_spec.py b/turnstone/api/console_spec.py index 2e52501f..94e18a09 100644 --- a/turnstone/api/console_spec.py +++ b/turnstone/api/console_spec.py @@ -31,8 +31,6 @@ from turnstone.api.console_schemas import ( CoordinatorCreateResponse, CoordinatorDetailResponse, CoordinatorHistoryResponse, - CoordinatorInfo, - CoordinatorListResponse, CoordinatorOpenResponse, CoordinatorRestrictRequest, CoordinatorRestrictResponse, @@ -134,6 +132,7 @@ from turnstone.api.schemas import ( from turnstone.api.server_schemas import ( ListAttachmentsResponse, ListSkillSummaryResponse, + ListWorkstreamsResponse, SkillSummary, UploadAttachmentResponse, ) @@ -1159,7 +1158,7 @@ CONSOLE_ENDPOINTS: list[EndpointSpec] = [ "Returns coordinators owned by the caller. Callers with " "``admin.system`` see every coordinator across tenants." ), - response_model=CoordinatorListResponse, + response_model=ListWorkstreamsResponse, error_codes=[403, 503], tags=["Coordinator"], ), @@ -1510,8 +1509,6 @@ _ALL_MODELS: list[type[BaseModel]] = [ CoordinatorCreateResponse, CoordinatorDetailResponse, CoordinatorHistoryResponse, - CoordinatorInfo, - CoordinatorListResponse, CoordinatorOpenResponse, CoordinatorRestrictRequest, CoordinatorRestrictResponse, diff --git a/turnstone/api/server_schemas.py b/turnstone/api/server_schemas.py index d8f4e41d..ecccab18 100644 --- a/turnstone/api/server_schemas.py +++ b/turnstone/api/server_schemas.py @@ -188,14 +188,34 @@ class CloseWorkstreamRequest(BaseModel): class WorkstreamInfo(BaseModel): - id: str + """Active-list row shape, shared across both kinds. + + Renamed ``id`` → ``ws_id`` and added ``user_id`` in the Stage 2 + ``list``/``saved`` verb lift so the active-list response shape + matches the rest of the v1 surface (every other shared verb's + payload uses ``ws_id``). ``user_id`` was previously coord-only; + interactive now populates it too. SDK consumers reading + ``row.id`` should swap to ``row.ws_id``. + """ + + ws_id: str name: str state: str kind: WorkstreamKind = WorkstreamKind.INTERACTIVE parent_ws_id: str | None = None + user_id: str = "" class ListWorkstreamsResponse(BaseModel): + """Response body for ``GET /v1/api/workstreams`` on either kind. + + Top-level key is ``workstreams`` regardless of the kind serving + the request — pre-lift coord returned ``{"coordinators": [...]}``; + convergence lifted both kinds onto the same shape. Coord SDK / + frontend consumers branching on ``data.coordinators`` swap to + ``data.workstreams``. + """ + workstreams: list[WorkstreamInfo] diff --git a/turnstone/console/server.py b/turnstone/console/server.py index 8ec309ff..c6215c75 100644 --- a/turnstone/console/server.py +++ b/turnstone/console/server.py @@ -65,7 +65,9 @@ from turnstone.core.session_routes import ( make_close_handler, make_create_handler, make_events_handler, + make_list_handler, make_open_handler, + make_saved_handler, make_send_handler, register_coord_verbs, register_session_routes, @@ -2620,6 +2622,36 @@ def _audit_coordinator_create( ) +async def _coord_saved_loaded_lookup(request: Request) -> set[str]: + """Return ws_ids currently held in ``coord_mgr``'s warm pool. + + Wired onto :attr:`SessionEndpointConfig.saved_loaded_lookup`. + Defence-in-depth filter for the saved-coordinators list — a row + can be ``state='closed'`` on disk for a few seconds while the + close-emit sequence races the in-memory pop, and we don't want + the saved card grid showing a coord that's still loaded. + + Empty set when ``coord_mgr`` isn't attached (subsystem unavailable) + or empty (zero-element snapshot — skip the executor hop too). + Errors are swallowed by the lifted body's outer ``try/except``; + returning ``set()`` here on a missing manager keeps the caller + happy without trampling the lifted body's error log. + """ + coord_mgr = getattr(request.app.state, "coord_mgr", None) + if coord_mgr is None: + return set() + # Cheap probe: an empty pool can answer without paying the + # ``asyncio.to_thread`` round-trip. ``count`` reads under the + # manager lock but doesn't block; if the manager isn't empty we + # still need ``list_all`` under to_thread because the snapshot + # itself acquires the same lock. + if coord_mgr.count == 0: + return set() + return await asyncio.to_thread( + lambda: {ws.id for ws in coord_mgr.list_all()}, + ) + + async def coordinator_history(request: Request) -> JSONResponse: """GET /v1/api/workstreams/{ws_id}/history — message history for page load. @@ -2656,113 +2688,6 @@ async def coordinator_history(request: Request) -> JSONResponse: return JSONResponse({"ws_id": ws_id, "messages": messages}) -async def coordinator_list(request: Request) -> JSONResponse: - """GET /v1/api/workstreams — list active coordinator sessions for the caller.""" - err = _require_admin_coordinator(request) - if err is not None: - return err - coord_mgr, err503 = _require_coord_mgr(request) - if err503 is not None: - return err503 - # Trusted-team visibility: any admin.coordinator-scoped caller - # sees cluster-wide active coordinators. ``user_id`` stays on the - # response row as metadata for the UI. - rows = coord_mgr.list_all() - return JSONResponse( - { - "coordinators": [ - { - "ws_id": r.id, - "name": r.name, - "state": r.state.value, - "user_id": r.user_id, - } - for r in rows - ] - } - ) - - -async def coordinator_saved(request: Request) -> JSONResponse: - """GET /v1/api/workstreams/saved — list saved (closed-but-persisted) coordinator - sessions for the caller. - - Mirrors :func:`turnstone.server.list_saved_workstreams` for the - coordinator surface so the frontend can render a "saved coordinators" - card grid identically to the interactive "saved workstreams" sidebar. - Same response item shape, same tenant/admin scoping. - - Returns ONLY rows with ``state='closed'`` — explicitly closed - coordinators that the user can re-open via the saved card. Active / - in-flight rows live in the active list above; deleted rows are - tombstones that ``_open_impl`` refuses to resurrect. - - Also filters out coordinators currently loaded into ``coord_mgr`` - (defence-in-depth: a coordinator could be 'closed' on disk but - still in the warm pool right after a restart of the close-emit - sequence races the in-memory pop). - """ - from turnstone.core.memory import list_workstreams_with_history - from turnstone.core.workstream import WorkstreamKind - - err = _require_admin_coordinator(request) - if err is not None: - return err - - # Trusted-team visibility (post-#400): any caller with the - # ``admin.coordinator`` scope sees cluster-wide saved coordinators; - # ``user_id`` stays as metadata on the persisted row but isn't a - # filter here. - # Offload the blocking storage call + the lock-acquiring list_all - # off the event loop, matching coordinator_create's pattern (#perf-2 - # from the saved-coordinators review). list_workstreams_with_history - # runs a correlated COUNT subquery; coord_mgr.list_all() takes the - # manager lock. Either can stall every other async handler if run - # inline. - rows = await asyncio.to_thread( - list_workstreams_with_history, - limit=50, - kind=WorkstreamKind.COORDINATOR, - user_id=None, - state="closed", - ) - - coord_mgr = getattr(request.app.state, "coord_mgr", None) - loaded: set[str] = set() - if coord_mgr is not None: - try: - loaded = await asyncio.to_thread( - lambda: {ws.id for ws in coord_mgr.list_all()}, - ) - except Exception: - log.debug("coordinator_saved.list_all_failed", exc_info=True) - - # Column order from list_workstreams_with_history is - # (ws_id, alias, title, name, created, updated, count, node_id) — see - # the SELECT in turnstone/core/storage/_sqlite.py:list_workstreams_with_history. - # ``*_extra`` swallows the trailing node_id (and any future columns - # appended at the tail); keep this comment in sync if the SELECT - # changes the prefix order. - result = [ - { - "ws_id": wid, - "alias": alias, - "title": title, - "name": name, - "created": created, - "updated": updated, - "message_count": count, - } - for wid, alias, title, name, created, updated, count, *_extra in rows - if wid not in loaded - ] - # Key is ``coordinators`` (not ``workstreams``) for consistency with - # the sibling coordinator_list endpoint. Item shape matches the - # interactive saved-workstreams response so the frontend card renderer - # stays a 1:1 mirror. - return JSONResponse({"coordinators": result}) - - async def coordinator_page(request: Request) -> Response: """GET /coordinator/{ws_id} — serve the one-pane coordinator HTML. @@ -10143,14 +10068,27 @@ def create_app( create_validate_request=_coord_create_validate_request, create_build_kwargs=_coord_create_build_kwargs, create_post_install=_coord_create_post_install, + # No alias surface on coord today — the lifted body falls + # back to ``ws.name`` when ``list_resolve_titles`` is None. + list_resolve_titles=None, + # Explicit kind classifier for the lifted list/saved factory's + # storage filter (drops the pre-fix ``audit_action_prefix`` + # string compare that would have silently leaked interactive + # rows for any future kind). + list_kind=WorkstreamKind.COORDINATOR, + # Coord saved cards show only explicitly-closed coordinators — + # active / in-flight rows live in the active list and + # tombstones are non-resurrectable. + saved_state_filter="closed", + saved_loaded_lookup=_coord_saved_loaded_lookup, ) coord_workstream_routes: list[Any] = [] register_session_routes( coord_workstream_routes, prefix="/api/workstreams", handlers=SharedSessionVerbHandlers( - list_workstreams=coordinator_list, - list_saved=coordinator_saved, + list_workstreams=make_list_handler(coord_endpoint_config), # lifted: shared body + list_saved=make_saved_handler(coord_endpoint_config), # lifted: shared body create=make_create_handler( # lifted: shared body coord_endpoint_config, audit_emit=_audit_coordinator_create, diff --git a/turnstone/console/static/app.js b/turnstone/console/static/app.js index 8573b093..37de6be4 100644 --- a/turnstone/console/static/app.js +++ b/turnstone/console/static/app.js @@ -2246,10 +2246,10 @@ function loadSavedCoordinators() { _savedCoordsInFlight = true; authFetch("/v1/api/workstreams/saved") .then(function (r) { - return r.ok ? r.json() : { coordinators: [] }; + return r.ok ? r.json() : { workstreams: [] }; }) .then(function (data) { - renderSavedCoordinators(data.coordinators || []); + renderSavedCoordinators(data.workstreams || []); }) .catch(function () { /* silent — saved list is informational, not load-bearing */ diff --git a/turnstone/core/memory.py b/turnstone/core/memory.py index da4521e3..92e78745 100644 --- a/turnstone/core/memory.py +++ b/turnstone/core/memory.py @@ -472,6 +472,25 @@ def get_workstream_display_name(ws_id: str) -> str | None: return None +def get_workstream_display_names(ws_ids: list[str]) -> dict[str, str | None]: + """Bulk variant of :func:`get_workstream_display_name`. + + One ``SELECT ... WHERE ws_id IN (...)`` instead of N. Used by the + lifted ``list`` verb to resolve aliases for every active row in a + single round-trip. Returns a dict with every requested ws_id — + missing rows map to ``None``; the caller falls back to ``ws.name`` + per-row. Errors return an empty dict so the caller falls back to + ``ws.name`` on every row. + """ + if not ws_ids: + return {} + try: + return get_storage().get_workstream_display_names(ws_ids) + except Exception: + log.warning("Failed to get display names count=%d", len(ws_ids), exc_info=True) + return {} + + def get_workstream_metadata(ws_id: str) -> dict[str, Any] | None: """Return workstream metadata dict or None if not found.""" try: diff --git a/turnstone/core/session_routes.py b/turnstone/core/session_routes.py index 74f7696b..75574265 100644 --- a/turnstone/core/session_routes.py +++ b/turnstone/core/session_routes.py @@ -46,7 +46,7 @@ if TYPE_CHECKING: from starlette.routing import BaseRoute from turnstone.core.session_manager import SessionManager - from turnstone.core.workstream import Workstream + from turnstone.core.workstream import Workstream, WorkstreamKind log = get_logger(__name__) @@ -207,6 +207,29 @@ CreateAuditEmitter = Callable[ ] +# (ws_ids) -> {ws_id: title-or-None} bulk lookup. Interactive wires +# :func:`turnstone.core.memory.get_workstream_display_names` so the +# active-list endpoint resolves every alias in one storage round-trip +# instead of the pre-lift N+1 (one SELECT per row). Coord wires +# ``None`` (coord doesn't have an alias surface today) and the lifted +# body uses ``ws.name`` directly. Returns a dict keyed on every +# requested ws_id; missing rows map to ``None``, and the caller +# falls back to ``ws.name`` per-row. +ListResolveTitles = Callable[[list[str]], dict[str, str | None]] +# (request) -> set of ws_ids currently held in memory by the kind's +# manager. Coord wires a callable that returns +# ``{ws.id for ws in coord_mgr.list_all()}`` so the saved-card list +# can defence-in-depth filter out coordinators currently in the warm +# pool (a coord can be ``state='closed'`` on disk briefly while the +# close-emit sequence races the in-memory pop). Interactive wires +# ``None``: an interactive workstream that's both saved and loaded +# is a normal display state, not a race the saved card needs to +# hide. Async because the coord-side implementation runs through +# ``asyncio.to_thread`` (the manager lock is acquired in +# ``coord_mgr.list_all``). +SavedLoadedLookup = Callable[["Request"], Awaitable[set[str]]] + + @dataclass(frozen=True) class AttachmentUploadHelpers: """Process-local hooks the lifted attachment factories call into. @@ -378,6 +401,34 @@ class SessionEndpointConfig: # ``None`` skips the post-install entirely (response is just # ``{ws_id, name, ...}`` with empty parity fields). create_post_install: CreatePostInstall | None = None + # (ws_ids) -> {ws_id: title-or-None} — bulk title lookup for the + # active-list endpoint. Interactive wires + # ``get_workstream_display_names`` so every row's alias resolves + # in one storage round-trip; coord wires ``None`` (no alias + # surface today). See :data:`ListResolveTitles`. + list_resolve_titles: ListResolveTitles | None = None + # Kind classifier for the lifted ``list``/``saved`` factories' + # storage filter. Required when a kind mounts either handler — + # the factories pass it straight through to + # ``list_workstreams_with_history(kind=...)``. Distinct from + # ``audit_action_prefix`` (audit-action namespacing) so adding a + # third kind doesn't have to overload the audit prefix as a + # filter. ``None`` is allowed for kinds that don't mount a + # list/saved handler. + list_kind: WorkstreamKind | None = None + # Storage-side state filter for the saved-list endpoint. Interactive + # wires ``None`` — saved sidebar shows every persisted interactive + # workstream regardless of state (the storage layer already + # excludes ``state='deleted'`` tombstones). Coord wires + # ``"closed"`` so only explicitly-closed coordinators surface in + # the saved-card grid; active / in-flight rows live in the active + # list. + saved_state_filter: str | None = None + # (request) -> set of ws_ids in the kind's in-memory pool. Coord + # wires a coroutine that returns ``{ws.id for ws in + # coord_mgr.list_all()}`` (defence-in-depth filter — see + # :data:`SavedLoadedLookup`). Interactive wires ``None``. + saved_loaded_lookup: SavedLoadedLookup | None = None @dataclass(frozen=True) @@ -1824,6 +1875,212 @@ def make_create_handler( return create +def make_list_handler(cfg: SessionEndpointConfig) -> Handler: + """Lifted body for ``GET {prefix}`` — list workstreams in memory. + + Both kinds share the listing sequence (auth → manager lookup → + ``mgr.list_all()`` → row serialisation → respond). Per-kind + divergence captured by: + + - ``cfg.permission_gate`` — coord's ``admin.coordinator`` check; + interactive ``None`` (auth middleware covers it). + - ``cfg.manager_lookup`` — already used by every other lifted + verb. + - ``cfg.list_resolve_title`` — interactive's user-alias override; + coord ``None``. + + Always-include row shape: ``{ws_id, name, state, kind, + parent_ws_id, user_id}``. SDK consumers don't branch on kind. + + Behaviour changes vs the pre-lift handlers (documented in + CHANGELOG): + + - **Top-level response key converges on ``"workstreams"``.** + Pre-lift coord returned ``{"coordinators": [...]}``; the lifted + body returns ``{"workstreams": [...]}`` for response-shape + parity with interactive. Coord SDK / frontend consumers + branching on ``data.coordinators`` swap to ``data.workstreams``. + - **Interactive row key renames ``"id"`` → ``"ws_id"``.** Pre- + lift interactive used the bare ``id`` field while every other + shared verb on this surface (cancel, open, events, create, + saved-list) uses ``ws_id``. Convergence eliminates the + internal inconsistency. Frontend consumers reading + ``ws.id`` from the active-list response swap to ``ws.ws_id``. + - **Always-include row fields.** ``user_id`` was coord-only; + ``kind`` + ``parent_ws_id`` were interactive-only. Both + kinds now populate all three. ``parent_ws_id`` defaults to + ``None`` for coord (coordinators have no parent). + - **Storage / manager-lock work moved off the event loop.** + ``mgr.list_all()`` acquires the manager mutex; the title + resolution may dip into storage for the alias lookup. Both + now run via ``asyncio.to_thread`` (matching coord's pre- + existing perf-2 pattern from the saved-coordinators review). + + Args: + cfg: per-kind policy bundle. + """ + + async def list_workstreams_handler(request: Request) -> Response: + import asyncio + + if cfg.permission_gate is not None: + err = cfg.permission_gate(request) + if err is not None: + return err + mgr_opt, err503 = cfg.manager_lookup(request) + if err503 is not None: + return err503 + # See ``make_approve_handler`` for the cast rationale. + mgr = cast("SessionManager", mgr_opt) + + # Manager-lock + bulk title resolution off the event loop. + # ``list_all`` snapshots under the manager lock; the bulk + # alias lookup hits storage for every row in a single + # ``SELECT ... WHERE ws_id IN (...)`` (replaces the pre-lift + # per-row N+1). Running inline would stall every other async + # handler for the duration of the listing. + resolve_titles = cfg.list_resolve_titles + + def _build_rows() -> list[dict[str, Any]]: + wss = mgr.list_all() + titles: dict[str, str | None] = {} + if resolve_titles is not None and wss: + titles = resolve_titles([ws.id for ws in wss]) + rows: list[dict[str, Any]] = [] + for ws in wss: + title = titles.get(ws.id) or ws.name + rows.append( + { + "ws_id": ws.id, + "name": title, + "state": ws.state.value, + "kind": ws.kind, + "parent_ws_id": ws.parent_ws_id, + "user_id": ws.user_id, + } + ) + return rows + + rows = await asyncio.to_thread(_build_rows) + return JSONResponse({"workstreams": rows}) + + return list_workstreams_handler + + +def make_saved_handler(cfg: SessionEndpointConfig) -> Handler: + """Lifted body for ``GET {prefix}/saved`` — list persisted workstreams. + + Both kinds share the storage-backed listing sequence (auth → + ``list_workstreams_with_history`` filtered by kind → optional + in-memory exclusion filter → row serialisation → respond). + Per-kind divergence: + + - ``cfg.permission_gate`` — coord's ``admin.coordinator`` check. + - ``cfg.audit_action_prefix`` — used to derive the kind filter + ("workstream" → INTERACTIVE; "coordinator" → COORDINATOR). + Already wired on every endpoint cfg; reuse here keeps the + cfg surface tight. + - ``cfg.saved_state_filter`` — coord wires ``"closed"`` so only + explicitly-closed coordinators surface; interactive wires + ``None`` (any state except the tombstoned ``deleted`` rows the + storage layer already filters). + - ``cfg.saved_loaded_lookup`` — coord-only defence-in-depth + filter that excludes ws_ids currently in the in-memory pool + (a row can be ``state='closed'`` briefly while the close-emit + sequence races the in-memory pop). Interactive ``None``. + + Always-include row shape: ``{ws_id, alias, title, name, + created, updated, message_count}``. Identical between kinds + pre-lift; the lift just moves the row construction into one + place. + + Behaviour changes vs the pre-lift handlers: + + - **Top-level response key converges on ``"workstreams"``.** + Pre-lift coord returned ``{"coordinators": [...]}``; the + lifted body returns ``{"workstreams": [...]}``. Mirrors the + active-list convergence. + - **Interactive's storage call moves to ``asyncio.to_thread``.** + Pre-lift interactive ran ``list_workstreams_with_history`` + inline — under heavy load the SQL (which includes a + correlated COUNT subquery) stalled every other async + handler. Coord already used ``to_thread`` (perf-2 from the + saved-coordinators review); convergence lifts interactive up. + + Args: + cfg: per-kind policy bundle. + """ + + async def saved_workstreams_handler(request: Request) -> Response: + import asyncio + + from turnstone.core.memory import list_workstreams_with_history + + if cfg.permission_gate is not None: + err = cfg.permission_gate(request) + if err is not None: + return err + + if cfg.list_kind is None: + # Misconfig: a kind mounted the saved handler without + # wiring ``cfg.list_kind``. Fail loud instead of silently + # filtering for the wrong kind — pre-fix the lifted body + # defaulted to INTERACTIVE on any non-"coordinator" + # ``audit_action_prefix``, which would have leaked + # interactive rows on any future kind that forgot the + # cfg field. + log.error("ws.saved.misconfigured_no_list_kind") + return JSONResponse( + {"error": "saved handler misconfigured"}, + status_code=500, + ) + + rows = await asyncio.to_thread( + list_workstreams_with_history, + limit=50, + kind=cfg.list_kind, + user_id=None, + state=cfg.saved_state_filter, + ) + + # Coord-only: exclude ws_ids currently in the warm pool. + loaded: set[str] = set() + if cfg.saved_loaded_lookup is not None: + try: + loaded = await cfg.saved_loaded_lookup(request) + except Exception: + # Defence-in-depth filter — never let a lookup error + # block the saved list. Log + continue with empty + # set (worst case: a duplicate row in the saved list + # for a few seconds during a close-emit race). + log.debug( + "ws.saved.loaded_lookup_failed", + exc_info=True, + ) + + # Column order from list_workstreams_with_history is + # (ws_id, alias, title, name, created, updated, count, node_id) — + # ``*_extra`` swallows the trailing node_id (and any future + # columns the SELECT may grow). Keep this comment in sync if + # the SELECT changes the prefix order. + result = [ + { + "ws_id": wid, + "alias": alias, + "title": title, + "name": name, + "created": created, + "updated": updated, + "message_count": count, + } + for wid, alias, title, name, created, updated, count, *_extra in rows + if wid not in loaded + ] + return JSONResponse({"workstreams": result}) + + return saved_workstreams_handler + + def make_send_handler(cfg: SessionEndpointConfig) -> Handler: """Lifted body for ``POST {prefix}/{ws_id}/send`` — message dispatch. diff --git a/turnstone/core/storage/_postgresql.py b/turnstone/core/storage/_postgresql.py index ca6869c4..52717534 100644 --- a/turnstone/core/storage/_postgresql.py +++ b/turnstone/core/storage/_postgresql.py @@ -469,6 +469,24 @@ class PostgreSQLBackend: return str(value) if value is not None else None return None + def get_workstream_display_names(self, ws_ids: list[str]) -> dict[str, str | None]: + if not ws_ids: + return {} + with self._conn() as conn: + rows = conn.execute( + sa.select( + workstreams.c.ws_id, + workstreams.c.alias, + workstreams.c.title, + workstreams.c.name, + ).where(workstreams.c.ws_id.in_(ws_ids)) + ).fetchall() + result: dict[str, str | None] = dict.fromkeys(ws_ids) + for r in rows: + value = r[1] or r[2] or r[3] + result[r[0]] = str(value) if value is not None else None + return result + def get_workstream_owner(self, ws_id: str) -> str | None: with self._conn() as conn: row = conn.execute( diff --git a/turnstone/core/storage/_protocol.py b/turnstone/core/storage/_protocol.py index 00a9a1ca..1a0ddc7f 100644 --- a/turnstone/core/storage/_protocol.py +++ b/turnstone/core/storage/_protocol.py @@ -289,6 +289,16 @@ class StorageBackend(Protocol): """Return the alias (or title) for a workstream, or None if unset.""" ... + def get_workstream_display_names(self, ws_ids: list[str]) -> dict[str, str | None]: + """Bulk variant of :meth:`get_workstream_display_name`. + + Returns a dict keyed on every requested ws_id. Missing rows + map to ``None``; the caller falls back to ``ws.name`` per-row. + Used by the lifted ``list`` verb to avoid the per-row + N+1 storage round-trip pre-lift had. + """ + ... + def get_workstream_metadata(self, ws_id: str) -> dict[str, Any] | None: """Return workstream metadata dict or None if not found.""" ... diff --git a/turnstone/core/storage/_sqlite.py b/turnstone/core/storage/_sqlite.py index 1e9ae4d1..7404bfce 100644 --- a/turnstone/core/storage/_sqlite.py +++ b/turnstone/core/storage/_sqlite.py @@ -578,6 +578,24 @@ class SQLiteBackend: return str(value) if value is not None else None return None + def get_workstream_display_names(self, ws_ids: list[str]) -> dict[str, str | None]: + if not ws_ids: + return {} + with self._conn() as conn: + rows = conn.execute( + sa.select( + workstreams.c.ws_id, + workstreams.c.alias, + workstreams.c.title, + workstreams.c.name, + ).where(workstreams.c.ws_id.in_(ws_ids)) + ).fetchall() + result: dict[str, str | None] = dict.fromkeys(ws_ids) + for r in rows: + value = r[1] or r[2] or r[3] + result[r[0]] = str(value) if value is not None else None + return result + def get_workstream_owner(self, ws_id: str) -> str | None: with self._conn() as conn: row = conn.execute( diff --git a/turnstone/server.py b/turnstone/server.py index e5c06d24..5ee30ef8 100644 --- a/turnstone/server.py +++ b/turnstone/server.py @@ -71,7 +71,9 @@ from turnstone.core.session_routes import ( make_events_handler, make_legacy_body_keyed_adapter, make_legacy_query_keyed_adapter, + make_list_handler, make_open_handler, + make_saved_handler, make_send_handler, register_session_routes, ) @@ -1211,52 +1213,15 @@ async def global_events_sse(request: Request) -> Response: return EventSourceResponse(event_generator(), ping=5) -async def list_workstreams(request: Request) -> JSONResponse: - """GET /v1/api/workstreams — list workstreams visible to any - authenticated caller. - - Trusted-team visibility: listing returns the full cluster set - across all owners (no per-user filter). Self-hosted deployments - are the assumed shape; console operators already see cluster-wide - state via service scope, and a per-user filter would also hide - ownerless rows like the auto-created ``name="default"`` startup - workstream from every authenticated caller. - - Per-workstream mutations (``/send``, ``/close``, ``/open``, - ``/title``, ``/delete``, ``/refresh-title``) keep their independent - ownership checks — see those handlers for the cross-tenant guards - that stay in force. This endpoint exposes metadata only (name, - state, kind, parent_ws_id); message history requires the per- - workstream ownership gate on ``/history``. - - For a multi-tenant SaaS deployment, the right boundary is a real - ``tenant_id`` column with row-level filtering at the storage - layer, not an empty-user_id heuristic. - """ - from turnstone.core.memory import get_workstream_display_name - - mgr: SessionManager = request.app.state.workstreams - result = [] - for ws in mgr.list_all(): - title = get_workstream_display_name(ws.id) or ws.name - # kind + parent_ws_id mirror the shape /dashboard returns below so - # client consumers (SDK, frontend, integrators) see one consistent - # row schema across adjacent endpoints instead of a subset here - # and a superset there. - result.append( - { - "id": ws.id, - "name": title, - "state": ws.state.value, - "kind": ws.kind, - "parent_ws_id": ws.parent_ws_id, - } - ) - return JSONResponse({"workstreams": result}) - - async def dashboard(request: Request) -> JSONResponse: - """GET /v1/api/dashboard — enriched workstream data + aggregate stats.""" + """GET /v1/api/dashboard — enriched workstream data + aggregate stats. + + NOTE: row shape still uses ``"id"`` (not ``"ws_id"``) — Stage 2's + list-verb lift converged ``/v1/api/workstreams`` and + ``/saved`` on ``ws_id`` but left dashboard alone to keep that + PR's diff focused. The same rename should land here as a separate + cleanup so the v1 row shape is consistent across the family. + """ from turnstone.core.memory import get_workstream_display_name mgr: SessionManager = request.app.state.workstreams @@ -1317,50 +1282,6 @@ async def dashboard(request: Request) -> JSONResponse: ) -async def list_saved_workstreams(request: Request) -> JSONResponse: - """GET /v1/api/workstreams/saved — list saved interactive workstream - metadata, visible to any authenticated caller. - - Trusted-team visibility: returns the cluster-wide set across all - owners (no per-user filter) — see ``list_workstreams`` for the - rationale. This endpoint exposes summary fields only (ws_id, - alias, title, name, created, updated, message_count); message - history requires the per-workstream ownership gate on - ``/v1/api/workstreams/{ws_id}/history``. - - Resuming an owned saved workstream goes through ``/open``'s - ownership check; ownerless persisted rows (legacy / migration / - startup ``name="default"``) are claimable by any authenticated - caller via ``/open``, consistent with the same trusted-team model. - - Restricted to ``kind="interactive"`` — the interactive UI's "saved - workstreams" sidebar is not a coordinator surface, and coordinator - rows (which persist conversation history too) would otherwise leak - into it. - """ - from turnstone.core.memory import list_workstreams_with_history - from turnstone.core.workstream import WorkstreamKind - - rows = list_workstreams_with_history( - limit=50, - kind=WorkstreamKind.INTERACTIVE, - user_id=None, - ) - result = [ - { - "ws_id": wid, - "alias": alias, - "title": title, - "name": name, - "created": created, - "updated": updated, - "message_count": count, - } - for wid, alias, title, name, created, updated, count, *_extra in rows - ] - return JSONResponse({"workstreams": result}) - - async def list_skills_summary(request: Request) -> JSONResponse: """GET /v1/api/skills — list available skills (summary).""" import json as _json @@ -3624,6 +3545,9 @@ def create_app( classify_text_attachment=_classify_text_attachment, upload_lock=_attachment_upload_lock, ) + from turnstone.core.memory import ( + get_workstream_display_names as _get_ws_display_names, + ) from turnstone.core.memory import resolve_workstream as _resolve_workstream_alias interactive_endpoint_config = SessionEndpointConfig( @@ -3653,6 +3577,22 @@ def create_app( create_validate_request=_interactive_create_validate_request, create_build_kwargs=_interactive_create_build_kwargs, create_post_install=_interactive_create_post_install, + # Bulk display-name resolution for the active list — one + # ``SELECT ... WHERE ws_id IN (...)`` for the whole snapshot + # instead of N per-row queries. Returns a {ws_id: title-or-None} + # dict; the lifted body falls back to ``ws.name`` per-row. + list_resolve_titles=_get_ws_display_names, + # Explicit kind classifier for the lifted list/saved factory's + # storage filter — required to avoid silently filtering for + # the wrong kind when a future kind is added. + list_kind=WorkstreamKind.INTERACTIVE, + # No state filter: the interactive saved sidebar shows every + # persisted workstream the storage layer doesn't already + # tombstone (deleted rows are excluded at the SQL level). + saved_state_filter=None, + # No in-memory exclusion: an interactive workstream that's + # both saved AND loaded is a normal display state. + saved_loaded_lookup=None, ) approve_handler = make_approve_handler(interactive_endpoint_config) close_handler = make_close_handler( @@ -3673,6 +3613,8 @@ def create_app( interactive_endpoint_config, audit_emit=_audit_workstream_created, ) + list_handler = make_list_handler(interactive_endpoint_config) + saved_handler = make_saved_handler(interactive_endpoint_config) v1_routes: list[Any] = [ Route("/api/events", make_legacy_query_keyed_adapter(events_handler)), Route("/api/events/global", global_events_sse), @@ -3681,8 +3623,8 @@ def create_app( v1_routes, prefix="/api/workstreams", handlers=SharedSessionVerbHandlers( - list_workstreams=list_workstreams, - list_saved=list_saved_workstreams, + list_workstreams=list_handler, # lifted: shared body + list_saved=saved_handler, # lifted: shared body create=create_handler, # lifted: shared body close_legacy=make_legacy_body_keyed_adapter(close_handler), delete=delete_workstream_endpoint, diff --git a/turnstone/ui/static/app.js b/turnstone/ui/static/app.js index 505ae133..0d815be8 100644 --- a/turnstone/ui/static/app.js +++ b/turnstone/ui/static/app.js @@ -516,7 +516,7 @@ Pane.prototype.connectSSE = function (wsId) { return r.json().then(function (data) { workstreams = {}; (data.workstreams || []).forEach(function (ws) { - workstreams[ws.id] = { name: ws.name, state: ws.state }; + workstreams[ws.ws_id] = { name: ws.name, state: ws.state }; }); renderTabBar(); // Reconnect all disconnected panes, reassigning stale ws_ids. @@ -5816,7 +5816,7 @@ function initWorkstreams() { }) .then(function (data) { data.workstreams.forEach(function (ws) { - workstreams[ws.id] = { name: ws.name, state: ws.state }; + workstreams[ws.ws_id] = { name: ws.name, state: ws.state }; }); connectGlobalSSE(); var wsIds = Object.keys(workstreams);