mirror of
https://github.com/turnstonelabs/turnstone.git
synced 2026-08-23 12:24:46 -06:00
feat(api): expose pending_approval on workstream detail response
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.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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):
|
||||
|
||||
@@ -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,
|
||||
}
|
||||
)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user