From ed7823273bf692b806b4e783d15c18c00a701e8b Mon Sep 17 00:00:00 2001 From: Patrick Buckley Date: Tue, 28 Apr 2026 20:21:16 -0700 Subject: [PATCH] feat(api): expose pending_approval on workstream detail response MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR 447 / 93cb3d9 (Copilot autonomous follow-up) added a JS path that reads ``wsSnapshot.pending_approval_detail`` off the ``GET /v1/api/workstreams/{ws_id}`` snapshot in coordinator.js init() so a freshly-loaded chat tab can paint the inline approval gate immediately at reload, without waiting for the SSE approve_request replay (which leaves a brief --running flash on the inflight orphan placeholder). But the server's ``WorkstreamDetailResponse`` schema only declared ``{ws_id, name, state, user_id, kind}`` and the lifted ``make_detail_handler`` matched: nothing was populating ``pending_approval`` or ``pending_approval_detail`` on the wire. The frontend block silently no-op'd at runtime; Copilot's accompanying assertion only grep'd the JS source for the literal strings, so it stayed green while the actual contract was missing. Extend the contract to match the Copilot frontend: - Add ``pending_approval: bool`` + ``pending_approval_detail: PendingApprovalDetail | None`` to ``WorkstreamDetailResponse``, same shape as the dashboard / cluster live projection. - ``make_detail_handler`` reads ``ws.ui._pending_approval`` (only treats it as live when ``isinstance(_, dict)`` so MagicMock- based unit tests don't trip the path) and calls ``ui.serialize_pending_approval_detail()`` to fill the detail. A serializer raise falls back to ``pending_approval=True`` + ``detail=None`` instead of 500ing the whole response — SSE replay still carries the authoritative payload. - ``test_returns_workstream_fields`` updated for the two extra fields (False / None on a MagicMock UI). - ``test_pending_approval_fields_propagate_from_ui`` is the new behavioural test: stub a UI with a realistic ``_pending_approval`` dict + serializer return, assert the JSON surfaces ``pending_approval=True`` + the items list. - ``test_pending_serializer_failure_falls_back_to_bool_only`` pins the defensive degradation so a future serializer regression can't 500 every reload. Tests: 4822 pass (3 deselected live). Ruff + mypy clean. --- tests/test_workstream_endpoints.py | 98 ++++++++++++++++++++++++++++++ turnstone/api/server_schemas.py | 20 ++++++ turnstone/core/session_routes.py | 37 +++++++++++ 3 files changed, 155 insertions(+) diff --git a/tests/test_workstream_endpoints.py b/tests/test_workstream_endpoints.py index f83a09bc..d349bb78 100644 --- a/tests/test_workstream_endpoints.py +++ b/tests/test_workstream_endpoints.py @@ -906,6 +906,10 @@ class TestDetailInteractive: loaded_ws.state = ws_state loaded_ws.user_id = "test-user" loaded_ws.kind = "interactive" + # No pending approval — leave .ui's MagicMock attrs alone; the + # handler isinstance-checks ``_pending_approval`` against ``dict`` + # before treating it as live, so MagicMock attribute pollution + # doesn't trigger the pending path. mock_mgr = MagicMock() mock_mgr.get.return_value = loaded_ws client = _build_detail_app(mock_mgr) @@ -919,8 +923,102 @@ class TestDetailInteractive: "state": "idle", "user_id": "test-user", "kind": "interactive", + "pending_approval": False, + "pending_approval_detail": None, } + def test_pending_approval_fields_propagate_from_ui(self): + """When the workstream's UI is parked on an approval, the detail + response surfaces ``pending_approval=True`` + the serialized + ``pending_approval_detail`` so a freshly-loaded chat tab can + paint the inline gate without waiting for the SSE + ``approve_request`` replay (which would otherwise produce a + brief ``--running`` flash on reload).""" + ws_id = "ws-pending-1" + ws_state = MagicMock() + ws_state.value = "attention" + loaded_ws = MagicMock() + loaded_ws.id = ws_id + loaded_ws.name = "coord-1" + loaded_ws.state = ws_state + loaded_ws.user_id = "test-user" + loaded_ws.kind = "coordinator" + # Realistic _pending_approval shape (mirrors what + # SessionUIBase.approve_tools assigns) + a serializer that + # returns the merged-with-verdicts payload. + loaded_ws.ui._pending_approval = { + "type": "approve_request", + "items": [ + { + "call_id": "c-1", + "func_name": "spawn_workstream", + "needs_approval": True, + }, + ], + "judge_pending": True, + } + loaded_ws.ui.serialize_pending_approval_detail = MagicMock( + return_value={ + "call_id": "c-1", + "judge_pending": True, + "items": [ + { + "call_id": "c-1", + "func_name": "spawn_workstream", + "needs_approval": True, + "heuristic_verdict": { + "recommendation": "approve", + "risk_level": "low", + "confidence": 0.9, + }, + } + ], + } + ) + mock_mgr = MagicMock() + mock_mgr.get.return_value = loaded_ws + client = _build_detail_app(mock_mgr) + + r = client.get(f"/v1/api/workstreams/{ws_id}") + assert r.status_code == 200 + body = r.json() + assert body["pending_approval"] is True + assert body["pending_approval_detail"]["call_id"] == "c-1" + assert body["pending_approval_detail"]["judge_pending"] is True + items = body["pending_approval_detail"]["items"] + assert len(items) == 1 + assert items[0]["func_name"] == "spawn_workstream" + assert items[0]["needs_approval"] is True + + def test_pending_serializer_failure_falls_back_to_bool_only(self): + """A malformed verdict that crashes ``serialize_pending_approval_detail`` + must NOT fail the detail response — the boolean still informs + the UI that an approval is pending; SSE replay carries the + authoritative payload. Defensive against a future serializer + regression silently 500ing every page load.""" + ws_id = "ws-pending-broken" + ws_state = MagicMock() + ws_state.value = "attention" + loaded_ws = MagicMock() + loaded_ws.id = ws_id + loaded_ws.name = "coord-broken" + loaded_ws.state = ws_state + loaded_ws.user_id = "test-user" + loaded_ws.kind = "coordinator" + loaded_ws.ui._pending_approval = {"items": []} + loaded_ws.ui.serialize_pending_approval_detail = MagicMock( + side_effect=RuntimeError("verdict object is malformed"), + ) + mock_mgr = MagicMock() + mock_mgr.get.return_value = loaded_ws + client = _build_detail_app(mock_mgr) + + r = client.get(f"/v1/api/workstreams/{ws_id}") + assert r.status_code == 200 + body = r.json() + assert body["pending_approval"] is True + assert body["pending_approval_detail"] is None + def test_lazy_rehydrates_on_miss(self): """``mgr.get`` miss → ``mgr.open`` rehydrate. Same flow as coord; pre-lift interactive had no detail endpoint so this is the diff --git a/turnstone/api/server_schemas.py b/turnstone/api/server_schemas.py index d1bde837..7a1f399b 100644 --- a/turnstone/api/server_schemas.py +++ b/turnstone/api/server_schemas.py @@ -424,6 +424,26 @@ class WorkstreamDetailResponse(BaseModel): state: str user_id: str kind: WorkstreamKind = WorkstreamKind.INTERACTIVE + pending_approval: bool = Field( + default=False, + description=( + "True when the workstream is parked on ``_approval_event`` " + "awaiting an operator approve/deny. Mirrors the same field " + "on ``DashboardWorkstream`` / cluster live projections so a " + "freshly-loaded chat tab can render the inline approval gate " + "from the detail snapshot before SSE replay arrives." + ), + ) + pending_approval_detail: PendingApprovalDetail | None = Field( + default=None, + description=( + "Inline approval payload — same shape as ``DashboardWorkstream" + ".pending_approval_detail``. ``None`` when no approval is " + "pending. Lets a reload paint the action row + judge " + "verdicts immediately instead of relying on the SSE " + "approve_request replay timing window." + ), + ) class WorkstreamHistoryResponse(BaseModel): diff --git a/turnstone/core/session_routes.py b/turnstone/core/session_routes.py index 9bc29cfa..7746000e 100644 --- a/turnstone/core/session_routes.py +++ b/turnstone/core/session_routes.py @@ -2303,6 +2303,41 @@ def make_detail_handler(cfg: SessionEndpointConfig) -> Handler: # mismatch, and tombstoned rows — all surface as 404. return JSONResponse({"error": cfg.not_found_label}, status_code=404) + # Pending-approval snapshot — lets a freshly-loaded chat tab + # paint the inline approval gate from this single response + # instead of waiting for the SSE approve_request replay (which + # introduces a brief --running flash on reload). The UI + # protocol doesn't mandate the approval surface; CLI / channel + # UIs that don't expose ``serialize_pending_approval_detail`` + # leave the field as ``None`` and the JSON omits the section. + # ``_pending_approval`` is asserted as ``dict`` (its only real + # production shape — see ``SessionUIBase._pending_approval``) + # so a MagicMock-based unit test or other non-dict sentinel + # doesn't trip the path. + pending_approval = False + pending_approval_detail: Any = None + ui = ws.ui + pending_raw = getattr(ui, "_pending_approval", None) if ui is not None else None + if isinstance(pending_raw, dict): + pending_approval = True + serializer = getattr(ui, "serialize_pending_approval_detail", None) + if callable(serializer): + try: + serialized = serializer() + if isinstance(serialized, dict) or serialized is None: + pending_approval_detail = serialized + except Exception: + # Defensive: a malformed verdict object inside the + # serializer shouldn't fail the entire detail + # response. The boolean still informs the UI that + # an approval is pending; SSE replay carries the + # full payload. + log.warning( + "ws.detail.pending_serialize_failed ws_id=%s", + ws_id[:8] if ws_id else "", + exc_info=True, + ) + return JSONResponse( { "ws_id": ws.id, @@ -2310,6 +2345,8 @@ def make_detail_handler(cfg: SessionEndpointConfig) -> Handler: "state": ws.state.value, "user_id": ws.user_id, "kind": ws.kind, + "pending_approval": pending_approval, + "pending_approval_detail": pending_approval_detail, } )