[eric] tool-ui: asks pair by tool_use_id, early clicks held, stops release waits, dead asks go quiet

This commit is contained in:
ciregenz
2026-08-10 14:45:11 -07:00
parent b0b7df3f61
commit 4cf0971693
9 changed files with 213 additions and 26 deletions
+3
View File
@@ -194,6 +194,9 @@ async def send_message(session_id: str, body: dict):
@agents.router.post("/sessions/{session_id}/stop")
async def stop_agent(session_id: str):
await agent_manager.stop_agent(session_id)
# A stopped turn's parked AskUI waits would otherwise zombie for 600s and eat the next click (ENG-232).
from backend.apps.agents.ui_request_bridge import cancel_session_waits
cancel_session_waits(session_id)
return {"ok": True}
@agents.router.post("/approval")
@@ -136,6 +136,9 @@ async def post_tool_hook(ctx: HookContext, input_data: dict, tool_use_id, contex
content = f"{content}\n\n" + wrap_platform_note("\n\n".join(notes))
result_payload = {"text": content}
# Pairing by array index mispairs any parallel batch (first-completing result lands on the first call); the provider id lets the frontend pair by identity (ENG-232).
if tool_use_id:
result_payload["tool_use_id"] = str(tool_use_id)
hook_tool_name = input_data.get("tool_name", "")
if hook_tool_name:
result_payload["tool_name"] = hook_tool_name
+11 -2
View File
@@ -44,6 +44,7 @@ COMPONENT_SPECS.update({name: entry["hint"] for name, entry in GENERATED.items()
INTERACTIVE_COMPONENTS = (
"option-list", "question-flow", "parameter-slider", "preferences-panel", "approval-card",
"message-draft",
)
TOOLS = [
@@ -250,8 +251,9 @@ def handle_ask_ui(arguments: dict) -> dict:
return {"content": [{"type": "text", "text": "props must be an object."}], "isError": True}
if component not in INTERACTIVE_COMPONENTS:
return {"content": [{"type": "text", "text": f"AskUI only supports: {', '.join(INTERACTIVE_COMPONENTS)}. Use ShowUI for display-only components."}], "isError": True}
component_id = str(props.get("id", "")).strip()
if not component_id:
# Registered RAW, not stripped: the frontend responds with the untrimmed id, and a mismatched key means the answer never lands.
component_id = str(props.get("id", ""))
if not component_id.strip():
return {"content": [{"type": "text", "text": "props.id (a stable string) is required so the answer can be correlated."}], "isError": True}
problem = validate(component, props)
if problem:
@@ -273,6 +275,13 @@ def handle_tool_call(tool_name: str, arguments: dict) -> dict:
props = arguments.get("props")
if not isinstance(props, dict):
return {"content": [{"type": "text", "text": "props must be an object."}], "isError": True}
# A display-only render wires no click handlers, so an interactive component via ShowUI draws dead buttons (message-draft even animates a send that sent nothing); teach instead of rendering a lie.
if component in INTERACTIVE_COMPONENTS:
return {"content": [{"type": "text", "text": (
f"Not rendered: '{component}' is interactive and ShowUI is display-only, so its buttons "
"would be dead. Call AskUI with the same component and props (props.id required) to "
"collect the user's answer."
)}], "isError": True}
problem = validate(component, props)
if problem:
return {"content": [{"type": "text", "text": f"Not rendered: {problem}"}], "isError": True}
+56 -9
View File
@@ -3,13 +3,19 @@ answers in the transcript (or the wait times out). Keyed by (session_id, compone
so the frontend can respond without ever learning a server-side request id."""
import asyncio
from typing import Any, Dict, Optional, Tuple
import time
from typing import Any, Dict, Literal, Optional, Tuple
from pydantic import BaseModel, ConfigDict, InstanceOf
from typeguard import typechecked
MAX_PENDING = 50
MAX_WAIT_SECONDS = 600.0
# The card is clickable the moment its tool_call broadcasts, but the wait only parks after the CLI hook round-trip, stdio dispatch and an HTTP hop; an instant click landing in that gap must not be dropped (ENG-232 D4).
EARLY_ANSWER_TTL_SECONDS = 45.0
MAX_EARLY = 50
RespondOutcome = Literal["delivered", "buffered", "gone"]
class PendingUiRequest(BaseModel):
@@ -18,15 +24,32 @@ class PendingUiRequest(BaseModel):
response: Optional[Dict[str, Any]] = None
class EarlyAnswer(BaseModel):
model_config = ConfigDict(validate_assignment=True)
stamp: float
response: Dict[str, Any]
p_pending: Dict[Tuple[str, str], PendingUiRequest] = {}
p_early: Dict[Tuple[str, str], EarlyAnswer] = {}
@typechecked
def p_prune_early(now: float) -> None:
for key in [k for k, v in p_early.items() if now - v.stamp > EARLY_ANSWER_TTL_SECONDS]:
p_early.pop(key, None)
@typechecked
async def wait_for_ui_response(session_id: str, component_id: str, timeout_s: float) -> Optional[Dict[str, Any]]:
"""Registers the request and blocks until respond_to_ui_request fires it; None on timeout."""
key = (session_id, component_id)
p_prune_early(time.monotonic())
early = p_early.pop(key, None)
if early is not None:
return early.response
if len(p_pending) >= MAX_PENDING:
raise ValueError("too many pending UI requests")
key = (session_id, component_id)
# A retried tool call for the same component replaces the stale wait; the old waiter times out.
pending = PendingUiRequest(event=asyncio.Event())
p_pending[key] = pending
@@ -41,11 +64,35 @@ async def wait_for_ui_response(session_id: str, component_id: str, timeout_s: fl
@typechecked
def respond_to_ui_request(session_id: str, component_id: str, response: Dict[str, Any]) -> bool:
"""Delivers the user's answer to the parked wait; False when nothing is waiting."""
def respond_to_ui_request(session_id: str, component_id: str, response: Dict[str, Any]) -> RespondOutcome:
"""Delivers the user's answer to the parked wait, or holds it briefly for a wait still en route."""
pending = p_pending.get((session_id, component_id))
if pending is None:
return False
pending.response = response
pending.event.set()
return True
if pending is not None:
pending.response = response
pending.event.set()
return "delivered"
now = time.monotonic()
p_prune_early(now)
if len(p_early) >= MAX_EARLY:
return "gone"
p_early[(session_id, component_id)] = EarlyAnswer(stamp=now, response=response)
return "buffered"
@typechecked
def reset_ui_bridge() -> None:
p_pending.clear()
p_early.clear()
@typechecked
def cancel_session_waits(session_id: str) -> int:
"""Releases every parked wait for a stopped session so its cards can't eat later clicks (ENG-232 D5)."""
released = 0
for key, pending in list(p_pending.items()):
if key[0] == session_id:
pending.event.set()
released += 1
for key in [k for k in p_early if k[0] == session_id]:
p_early.pop(key, None)
return released
+2 -2
View File
@@ -939,10 +939,10 @@ async def ui_request_respond(request: Request):
if not session_id or not component_id or not isinstance(response, dict):
return JSONResponse({"error": "session_id, component_id and response object are required"}, status_code=400)
from backend.apps.agents.ui_request_bridge import respond_to_ui_request
delivered = respond_to_ui_request(session_id, component_id, response)
outcome = respond_to_ui_request(session_id, component_id, response)
# A consumed/expired request is a normal outcome (replayed transcript, agent moved on), not an
# error; 200 + gone keeps Chromium's console free of red 404 noise while the UI shows its orphaned state.
return JSONResponse({"ok": delivered, "gone": not delivered})
return JSONResponse({"ok": outcome != "gone", "buffered": outcome == "buffered", "gone": outcome == "gone"})
@app.post("/api/invoke-agent/run")
+64
View File
@@ -0,0 +1,64 @@
"""The AskUI bridge: early clicks are held, stops release zombies, answers land (ENG-232)."""
import asyncio
import pytest
from backend.apps.agents.ui_request_bridge import (
cancel_session_waits,
reset_ui_bridge,
respond_to_ui_request,
wait_for_ui_response,
)
@pytest.fixture(autouse=True)
def fresh_bridge():
reset_ui_bridge()
yield
reset_ui_bridge()
@pytest.mark.asyncio
async def test_answer_reaches_a_parked_wait():
task = asyncio.ensure_future(wait_for_ui_response("s1", "q1", 5.0))
await asyncio.sleep(0.05)
assert respond_to_ui_request("s1", "q1", {"action": "select", "value": "a"}) == "delivered"
assert await task == {"action": "select", "value": "a"}
@pytest.mark.asyncio
async def test_click_before_the_wait_parks_is_held_not_dropped():
"""The card is clickable before the wait registers; that click used to be discarded."""
assert respond_to_ui_request("s1", "q1", {"action": "confirm"}) == "buffered"
assert await wait_for_ui_response("s1", "q1", 5.0) == {"action": "confirm"}
# Consumed exactly once: the next wait for the same id parks and times out instead of replaying it.
assert await wait_for_ui_response("s1", "q1", 0.1) is None
@pytest.mark.asyncio
async def test_stop_releases_parked_waits_so_they_cannot_eat_later_clicks():
task = asyncio.ensure_future(wait_for_ui_response("s1", "q1", 30.0))
other = asyncio.ensure_future(wait_for_ui_response("s2", "q1", 30.0))
await asyncio.sleep(0.05)
assert cancel_session_waits("s1") == 1
assert await task is None
# The other session's wait is untouched and still answerable.
assert respond_to_ui_request("s2", "q1", {"action": "x"}) == "delivered"
assert await other == {"action": "x"}
@pytest.mark.asyncio
async def test_stop_also_drops_buffered_answers():
assert respond_to_ui_request("s1", "q1", {"action": "confirm"}) == "buffered"
cancel_session_waits("s1")
assert await wait_for_ui_response("s1", "q1", 0.1) is None
@pytest.mark.asyncio
async def test_a_stale_buffered_answer_expires(monkeypatch):
import backend.apps.agents.ui_request_bridge as B
monkeypatch.setattr(B, "EARLY_ANSWER_TTL_SECONDS", 0.0)
assert respond_to_ui_request("s1", "q1", {"action": "confirm"}) == "buffered"
# TTL zero: the next wait prunes it and parks instead of replaying a dead click.
assert await wait_for_ui_response("s1", "q1", 0.2) is None
+26 -7
View File
@@ -65,7 +65,7 @@ import ToolCallBubble, { ToolPair } from './tool-bubbles/ToolCallBubble';
import ToolGroupBubble, { RenderItem, ToolGroup, ToolGroupEntry, isToolGroup, isToolPair } from './tool-bubbles/ToolGroupBubble';
import ToolUiBubble from './tool-ui/ToolUiBubble';
import AskUiBubble from './tool-ui/AskUiBubble';
import { isShowUiPair, isAskUiPair, extractPendingAskUi } from './tool-ui/showUiPayload';
import { isShowUiPair, isAskUiPair, extractPendingAskUi, callToolUseId, resultToolUseId, isDeadAskResult } from './tool-ui/showUiPayload';
import { composerPlaceholder } from './composerPlaceholder';
import ApprovalBar, { BatchApprovalBar } from './shell/ApprovalBar';
import ForceStopAgentBar from './ForceStopAgentBar';
@@ -1190,12 +1190,22 @@ const AgentChat: React.FC<AgentChatProps> = ({ sessionId: sessionIdProp, onClose
const allCalls = group.filter((m) => m.role === 'tool_call');
const results = group.filter((m) => m.role === 'tool_result');
const allPairs: ToolPair[] = allCalls.map((call, idx) => ({
type: 'tool_pair' as const,
id: `pair-${call.id}`,
call,
result: results[idx] || null,
}));
// Index pairing mispairs any parallel batch (first-completing result lands on the first call, killing a live AskUI card, ENG-232); pair by tool_use_id when the result carries one, index only for legacy unkeyed results.
const resultById = new Map<string, (typeof results)[number]>();
for (const r of results) {
const rid = resultToolUseId(r);
if (rid && !resultById.has(rid)) resultById.set(rid, r);
}
const allPairs: ToolPair[] = allCalls.map((call, idx) => {
const byId = resultById.get(callToolUseId(call));
const indexed = results[idx] || null;
return {
type: 'tool_pair' as const,
id: `pair-${call.id}`,
call,
result: byId ?? (indexed && !resultToolUseId(indexed) ? indexed : null),
};
});
// ShowUI/AskUI calls render as inline components, never buried inside a collapsed group.
// They typically cap a run of work, so the quiet group row stays above the widget.
@@ -1803,6 +1813,15 @@ const AgentChat: React.FC<AgentChatProps> = ({ sessionId: sessionIdProp, onClose
if (isToolPair(item)) {
const isPending = item.result === null && sessionRunning;
if (isAskUiPair(item)) {
// A dead ask (timeout prose or a validation bounce) is not answerable; rendering it full-size is the "two identical cards, only one works" dupe (ENG-232).
if (isDeadAskResult(item)) {
return (
<Box key={item.id} data-window-item-id={item.id} ref={isLastVisibleItem ? lastVisibleItemRef : undefined}>
<ToolCallBubble call={item.call} result={item.result} isPending={false} sessionId={session.id} suppressReveal />
{compactionChip}
</Box>
);
}
// Only the LATEST unanswered question is live (the backend parks one component); an
// older pending ask renders as a quiet row instead of a second clickable form.
if (item.result === null && lastPendingAskCallId !== null && item.call.id !== lastPendingAskCallId) {
@@ -70,12 +70,14 @@ function AskUiBubble({ pair, sessionId, isPending, suppressReveal }: AskUiBubble
setOrphaned(true);
}
})
.catch(() => { setSubmitted(false); setLocalChoice(undefined); });
.catch(() => { setSubmitted(false); setLocalChoice(undefined); setOrphaned(true); });
},
[submitted, sessionId, componentId],
);
const waiting = pair.result === null && !submitted;
// A result that isn't the JSON answer envelope (timeout prose, validation bounce) means this ask is dead; it must not look answerable (ENG-232).
const expired = pair.result !== null && answered === null;
// Their embedded-actions contract: onAction(actionId, state) delivers the component's full state,
// and the components ship their own footer actions (Clear/Confirm), so we only wire the callback.
@@ -91,12 +93,17 @@ function AskUiBubble({ pair, sessionId, isPending, suppressReveal }: AskUiBubble
: { choice: (answered?.choice as string) ?? (localChoice as string | undefined) };
}
if (waiting) {
return {
const base = {
onAction: (actionId: string, state: unknown) => {
if (actionId === 'cancel') return;
respond({ action: actionId, value: state ?? null });
},
};
// message-draft's send flow fires onSend (after its undo grace), not onAction; without this the send animation completes while nothing reaches the agent.
if (payload.name === 'message-draft') {
return { ...base, onSend: () => respond({ action: 'send', value: null }) };
}
return base;
}
// A free-text answer isn't an option id; passing it as `choice` would fail their contract.
if (freeTextAnswer !== null) return {};
@@ -139,7 +146,14 @@ function AskUiBubble({ pair, sessionId, isPending, suppressReveal }: AskUiBubble
)}
</Box>
)}
<VendoredToolUi name={payload.name} props={payload.props} extraProps={extraProps} />
<Box sx={expired ? { opacity: 0.55, pointerEvents: 'none' } : undefined}>
<VendoredToolUi name={payload.name} props={payload.props} extraProps={extraProps} />
</Box>
{expired && (
<Box sx={{ fontSize: '0.75rem', opacity: 0.55, pt: 0.5 }}>
This question expired before it was answered; if the agent asked again, use the newer card.
</Box>
)}
{waiting && FREE_TEXT_COMPONENTS.has(payload.name) && (
<Box
component="form"
@@ -179,7 +193,7 @@ function AskUiBubble({ pair, sessionId, isPending, suppressReveal }: AskUiBubble
)}
{orphaned && (
<Box sx={{ fontSize: '0.75rem', opacity: 0.55, pt: 0.5 }}>
No agent is waiting for this answer (the request expired or this is an old transcript).
This answer didn't reach the agent (it may have stopped, expired, or the connection dropped). Try again.
</Box>
)}
</Box>
@@ -81,6 +81,26 @@ export function isAskUiPair(pair: ToolPair): boolean {
return /(^|__)AskUI$/.test(tool);
}
/** The provider tool_use id a call carries in its content; '' when absent. */
export function callToolUseId(msg: { content: unknown }): string {
const c = (typeof msg.content === 'object' && msg.content !== null ? msg.content : {}) as { id?: unknown };
return typeof c.id === 'string' ? c.id : '';
}
/** The provider tool_use id a result says it answers; '' on legacy results that carry none. */
export function resultToolUseId(msg: { content: unknown }): string {
const c = (typeof msg.content === 'object' && msg.content !== null ? msg.content : {}) as { tool_use_id?: unknown };
return typeof c.tool_use_id === 'string' ? c.tool_use_id : '';
}
/** A result that is not the JSON answer envelope (timeout prose, validation bounce, AskUI failure): the ask is dead, not answered, and must not render as a clickable form. */
export function isDeadAskResult(pair: ToolPair): boolean {
if (pair.result === null) return false;
const rc = pair.result.content as unknown;
const text = typeof rc === 'string' ? rc : (typeof rc === 'object' && rc !== null ? String((rc as { text?: unknown }).text ?? '') : '');
return !text.trim().startsWith('{');
}
/** Latest ShowUI payload anywhere in a transcript; the collapsed card pins this artifact under its pill. */
/** The newest UNANSWERED AskUI call, so a collapsed card can surface the live question under its
pill (a blocking question beats every other artifact; the agent is literally waiting on it). */
@@ -90,8 +110,16 @@ export function extractPendingAskUi(messages: Array<{ id: string; role: string;
if (msg.role !== 'tool_call') continue;
const body = (typeof msg.content === 'object' && msg.content !== null ? msg.content : {}) as { tool?: unknown };
if (!/(^|__)AskUI$/.test(String(body.tool || ''))) continue;
const next = messages[i + 1];
if (next && next.role === 'tool_result') return null;
// Parallel batches break next-message adjacency, so match the answering result by tool_use_id when results carry one; adjacency stays the legacy fallback.
const callId = callToolUseId(msg);
let answered = false;
for (let j = i + 1; j < messages.length && !answered; j++) {
const m = messages[j];
if (m.role !== 'tool_result') continue;
const rid = resultToolUseId(m);
answered = rid ? rid === callId : j === i + 1;
}
if (answered) return null;
return { type: 'tool_pair', id: msg.id, call: msg as ToolPair['call'], result: null };
}
return null;