mirror of
https://github.com/turnstonelabs/turnstone.git
synced 2026-08-12 23:12:23 -06:00
fix: replay plan review prompt on SSE reconnection (#281)
* fix: replay plan review prompt on SSE reconnection Plan approval prompts were lost when a user navigated to the server web UI from the console dashboard (triggering a new SSE connection). Tool approvals stored pending state in _pending_approval and replayed it on reconnection, but plan reviews used fire-and-forget _enqueue with no persistent state. Mirror the _pending_approval pattern: store _pending_plan_review before blocking, replay it in events_sse for new SSE clients, and clear it on resolution. Without this fix, plan reviews silently timed out after 1 hour and were treated as approval. * test: add plan review SSE replay regression tests Covers pending state lifecycle: stored during on_plan_review, cleared on resolve_plan, available for SSE reconnection replay.
This commit is contained in:
@@ -713,6 +713,41 @@ class TestWebUI:
|
||||
assert ui._plan_result == "approved"
|
||||
t.join()
|
||||
|
||||
def test_pending_plan_review_stored_and_replayed(self):
|
||||
"""Plan review state is stored for SSE reconnection replay."""
|
||||
from turnstone.server import WebUI
|
||||
|
||||
ui = WebUI(ws_id="test")
|
||||
assert ui._pending_plan_review is None
|
||||
|
||||
# Simulate on_plan_review in a background thread (it blocks)
|
||||
def review():
|
||||
ui.on_plan_review("Here is the plan")
|
||||
|
||||
t = threading.Thread(target=review)
|
||||
t.start()
|
||||
time.sleep(0.1)
|
||||
|
||||
# While blocking, pending state should be set
|
||||
assert ui._pending_plan_review is not None
|
||||
assert ui._pending_plan_review["type"] == "plan_review"
|
||||
assert ui._pending_plan_review["content"] == "Here is the plan"
|
||||
|
||||
# Resolve — pending state should be cleared
|
||||
ui.resolve_plan("looks good")
|
||||
t.join(timeout=2)
|
||||
assert ui._pending_plan_review is None
|
||||
assert ui._plan_result == "looks good"
|
||||
|
||||
def test_pending_plan_review_cleared_on_resolve_before_wait_returns(self):
|
||||
"""resolve_plan clears pending state immediately, not just after wait."""
|
||||
from turnstone.server import WebUI
|
||||
|
||||
ui = WebUI(ws_id="test")
|
||||
ui._pending_plan_review = {"type": "plan_review", "content": "test"}
|
||||
ui.resolve_plan("ok")
|
||||
assert ui._pending_plan_review is None
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# WebUI SSE fan-out
|
||||
|
||||
+8
-2
@@ -95,6 +95,7 @@ class WebUI:
|
||||
self._pending_approval: dict[str, Any] | None = None # re-sent on SSE reconnect
|
||||
self._plan_event = threading.Event()
|
||||
self._plan_result: str = ""
|
||||
self._pending_plan_review: dict[str, Any] | None = None # re-sent on SSE reconnect
|
||||
self.auto_approve = False
|
||||
self.auto_approve_tools: set[str] = set()
|
||||
# Per-workstream metrics accumulators (written by worker thread, read by metrics handler)
|
||||
@@ -476,10 +477,12 @@ class WebUI:
|
||||
|
||||
def on_plan_review(self, content: str) -> str:
|
||||
self._plan_event.clear()
|
||||
self._enqueue({"type": "plan_review", "content": content})
|
||||
self._pending_plan_review = {"type": "plan_review", "content": content}
|
||||
self._enqueue(self._pending_plan_review)
|
||||
if not self._plan_event.wait(timeout=3600):
|
||||
log.warning("Plan review timed out for ws_id=%s", self.ws_id)
|
||||
self._plan_result = ""
|
||||
self._pending_plan_review = None
|
||||
return self._plan_result
|
||||
|
||||
def on_info(self, message: str) -> None:
|
||||
@@ -621,6 +624,7 @@ class WebUI:
|
||||
|
||||
def resolve_plan(self, feedback: str) -> None:
|
||||
"""Called by the HTTP handler when the user responds to a plan."""
|
||||
self._pending_plan_review = None
|
||||
self._plan_result = feedback
|
||||
self._plan_event.set()
|
||||
|
||||
@@ -900,9 +904,11 @@ async def events_sse(request: Request) -> Response:
|
||||
history = _build_history(session, has_pending_approval=ui._pending_approval is not None)
|
||||
if history:
|
||||
yield {"data": json.dumps({"type": "history", "messages": history})}
|
||||
# Re-inject pending approval
|
||||
# Re-inject pending approval or plan review
|
||||
if ui._pending_approval is not None:
|
||||
yield {"data": json.dumps(ui._pending_approval)}
|
||||
if ui._pending_plan_review is not None:
|
||||
yield {"data": json.dumps(ui._pending_plan_review)}
|
||||
|
||||
_metrics.record_sse_connect()
|
||||
try:
|
||||
|
||||
Reference in New Issue
Block a user