fix(tui_gateway): tolerate late clarify + terminal.read replies after timeout (#69773)

`_block()` bridges four blocking request types — secret, sudo, clarify,
terminal.read — with an identical lifecycle: on timeout the tool gives up and
returns empty, but a slow renderer (or a WebSocket reconnect that dropped
tool.complete) can still answer afterward. Only secret and sudo tolerated that
late reply; clarify and terminal.read still hit the generic 4009 "no pending
request" error, which clients surface as a raw JSON-RPC string (and at least
one desktop fork re-armed the pending request on the error).

Bring the two stragglers in line with the pair that already works:
- `_block` now emits `{event}.expire` on timeout for all four request types.
- `clarify.respond` and `terminal.read.respond` pass `allow_expired=True`, so a
  late answer resolves to `{"status": "expired"}` instead of erroring.

Tests parametrize the timeout-expiry and late-idempotent-response cases over
all four bridges; the old test asserting clarify stays a 4009 error is updated
to the new graceful contract.

Supersedes #56571 (clarify) and #64886 (terminal.read) — same root cause, one fix.

Co-authored-by: liuhao1024 <liuhao1024@users.noreply.github.com>
Co-authored-by: pierrenode <pierrenode@users.noreply.github.com>
This commit is contained in:
brooklyn! 2026-07-22 23:24:42 -05:00 committed by GitHub
parent 390b03c455
commit 3dd9d5e692
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 36 additions and 18 deletions

View file

@ -265,7 +265,10 @@ def test_block_and_respond(capture):
assert result[0] == "my_answer"
@pytest.mark.parametrize("event", ["secret.request", "sudo.request"])
@pytest.mark.parametrize(
"event",
["secret.request", "sudo.request", "clarify.request", "terminal.read.request"],
)
def test_sensitive_prompt_timeout_emits_expiry(capture, event):
server, buf = capture
@ -281,9 +284,17 @@ def test_sensitive_prompt_timeout_emits_expiry(capture, event):
@pytest.mark.parametrize(
("method", "value_key"),
[("secret.respond", "value"), ("sudo.respond", "password")],
[
("secret.respond", "value"),
("sudo.respond", "password"),
("clarify.respond", "answer"),
("terminal.read.respond", "text"),
],
)
def test_late_sensitive_prompt_response_is_idempotent(server, method, value_key):
def test_late_prompt_response_is_idempotent(server, method, value_key):
"""All four blocking bridges tolerate a late reply after their request has
expired the `*.respond` returns a graceful `{"status": "expired"}` instead
of the raw 4009 protocol error a client would otherwise surface verbatim."""
response = server.handle_request(
{
"id": "late-response",
@ -295,18 +306,6 @@ def test_late_sensitive_prompt_response_is_idempotent(server, method, value_key)
assert response["result"] == {"status": "expired"}
def test_late_clarify_response_remains_protocol_error(server):
response = server.handle_request(
{
"id": "late-clarify",
"method": "clarify.respond",
"params": {"request_id": "expired-request", "answer": ""},
}
)
assert response["error"]["code"] == 4009
def test_clear_pending(server):
ev = threading.Event()
# _pending values are (sid, Event) tuples

View file

@ -2431,7 +2431,19 @@ def _block(event: str, sid: str, payload: dict, timeout: int = 300) -> str:
answer_present = rid in _answers
answer = _answers.pop(rid, "")
if not answered and not answer_present and event in {"secret.request", "sudo.request"}:
# Emit an `.expire` notification on timeout for every blocking request type
# whose `*.respond` handler tolerates a late reply (allow_expired=True).
# All four blocking bridges — secret, sudo, clarify, terminal.read — share
# the same lifecycle: the tool gives up on timeout and returns empty, but a
# slow renderer (or a reconnect that dropped tool.complete) can still answer
# afterward. Without this the late `*.respond` would hit the generic 4009
# "no pending request" error and clients would surface a raw JSON-RPC string.
if not answered and not answer_present and event in {
"secret.request",
"sudo.request",
"clarify.request",
"terminal.read.request",
}:
_emit(
f"{event.removesuffix('.request')}.expire",
sid,
@ -11745,13 +11757,20 @@ def _respond(rid, params, key, *, allow_expired=False):
@method("clarify.respond")
def _(rid, params: dict) -> dict:
return _respond(rid, params, "answer")
# allow_expired=True: a clarify can time out server-side (its entry is popped
# from _pending) while the card is still visible — common when a WebSocket
# reconnect during the wait drops tool.complete. A late answer must resolve
# gracefully instead of hitting the raw 4009 "no pending answer request".
return _respond(rid, params, "answer", allow_expired=True)
@method("terminal.read.respond")
def _(rid, params: dict) -> dict:
# `text` is a JSON string of the serialized terminal buffer + line metadata.
return _respond(rid, params, "text")
# allow_expired=True: the read_terminal tool's _block() uses a short 30s
# timeout, so a slow renderer losing the race is the common case — a late
# response must not error after the tool already returned empty.
return _respond(rid, params, "text", allow_expired=True)
@method("sudo.respond")