fix(clarify): unblock the agent when a prose reply is rejected against a multi-choice clarify

[andyst-dev]

Fixes #84608

Problem

When an agent calls clarify with a native multi-choice prompt and the user replies with prose (e.g. asking what the options mean), the reply is rejected as a clarify response and demoted to a queued steer — but the clarify wait is never resolved or cancelled. Since the agent thread is already blocked inside wait_for_response(), the "continue as a normal turn" path can't run, so the session parks for the full agent.clarify_timeout (default 3600 seconds) before the queued reply is delivered. Asking a clarifying question about a clarify prompt is precisely what deadlocks the session for up to an hour.

Root cause

resolve_text_response_for_session() (tools/clarify_gateway.py) returned False on a rejected prose reply (matching _coerce_text_response's docstring: "so the message continues as a normal turn") but never resolved or cancelled the clarify entry. The blocked agent thread had no way to unblock.

Fix

When a prose reply is rejected against a non-awaiting_text clarify (native multi-choice), resolve the clarify with the empty sentinel — the same sentinel clear_session() uses — so the blocked agent returns promptly. resolve_text_response_for_session still returns False, so the caller dispatches the message as a normal turn.

Open-ended clarifies and the "Other" text-capture path are unchanged (they already accept arbitrary text and never hit this branch).

Verification

• New test: a prose reply to a native multi-choice clarify returns False from resolve_text_response_for_session, resolves the wait with the empty sentinel (blocked thread unblocks), and consumes the entry from the pending index.
• tests/tools/test_clarify_gateway.py: 24/24 pass.
• ruff check clean on both edited files.

[ayushnangia]

Verified on your branch: scripts/run_tests.sh tests/tools/test_clarify_gateway.py tests/tools/test_clarify_tool.py → 44 passed, 0 failed. The fix point is the right one — the rejection path deciding what happens next, with the blocked wait in scope — rather than patching _coerce_text_response or wait_for_response alone, which (as @AlexxRussell's composition analysis on #84608 showed) are each correct in isolation. Also ran a merge check against #84560 (interrupt-path member of the same family, touches the same file): the two branches auto-merge cleanly, no conflict — they compose, in whichever order maintainers land them. With this, every non-desktop member of the clarify/approval family from the #84047 triage has a fix in flight: #25506/#84560 (interrupt), #83346 (session-key routing), #84642 (rejection-demotion). Nice turnaround — #84608 was 85 minutes old when you fixed it.

[Enough1122]

AI code review — automated review for reference, author can ignore or act on any point.

fix(clarify): unblock the blocked agent when a prose reply is rejected against a multi-choice clarify

1. tools/clarify_gateway.py (resolve_text_response_for_session): the return-value contract is now overloaded — False historically meant "rejected, nothing consumed, message continues as a normal turn", but it can now also mean "rejected and the clarify entry was resolved/cancelled with the empty sentinel". Confirm every caller treats the consumed-entry case correctly (the entry is gone from the pending index when it next checks), and document the new contract on the function.
2. tools/clarify_gateway.py: the if not entry.awaiting_text guard means a prose reply to a text-prompt clarify (awaiting_text=True) still leaves the agent parked for the full clarify_timeout. If the parked-agent symptom from [Bug]: Prose reply to a multi-choice clarify is demoted to a steer, so the clarify blocks for its full timeout #84608 applies there too, consider unblocking that path as well; at minimum a test locking the branch behavior would prevent regressions.
3. Minor: reusing the empty-string sentinel (same as clear_session) for cancellation makes a genuine empty user response indistinguishable from a cancel. A dedicated named sentinel constant would make the intent explicit.

[keeltrace]

AI-assisted state-machine review; reviewed before posting.

One ordering invariant may be worth pinning around the chosen “resolve sentinel + return False” behavior.

A rejected prose message now has two effects: it resolves the currently blocked clarify, waking the existing agent turn, while resolve_text_response_for_session() returns False so that same prose continues through normal message dispatch.

Could there be a race where the prose begins normal processing before the clarify-owning turn has observed the cancellation and relinquished its execution slot?

The safety property I’d want is:

A rejected clarify reply may unblock the owning turn and subsequently become normal input, but it must not create a second concurrently executing turn for that session.

A deterministic regression could hold the original clarify-owning turn immediately after the sentinel wakes it, allow the prose-dispatch path to proceed, and verify that the prose remains queued/serialized until the first turn has exited or yielded session ownership. Then assert the prose is delivered exactly once afterward.

It may also be useful to pin that the awakened turn cannot interpret the cancellation sentinel as permission to continue consequential work before the user's actual prose is processed.

This seems distinct from the existing return-contract/sentinel observations: those establish what the two branches mean; this would establish their causal ordering when both are triggered by one message.

[andyst-dev]

Good observation — this is a real invariant worth pinning, and let me be precise about where it holds in this PR versus where it lives upstream.

What this PR deterministically guarantees (inside clarify_gateway.py): the sentinel resolve happens before resolve_text_response_for_session() returns False, and it's a single-threaded, side-effect-ordered sequence inside the gateway (resolve entry → return). So by the time the caller sees False and re-dispatches the prose, the clarify-owning wait has already been released. There is no interleaving inside this module that lets the re-dispatch observe an unreleased wait. The existing test (test_clarify_gateway.py) pins the sentinel-with-empty-string + return-False contract.

What is NOT pinned here: the turn-serialization property you're pointing at — that the re-dispatched prose doesn't enter a second concurrently-executing turn for the same session before the awakened turn has yielded session ownership. That ordering lives in the agent execution loop / message dispatch above this module, which this utility doesn't import or control. clarify_gateway has no reference to the executor, so a unit test in this file cannot honestly assert turn concurrency without pulling in the whole agent runtime.

So I'd split it:

• The part this PR can and should own: "sentinel resolve strictly precedes the rejected-prose return" — already implemented and tested here.
• The genuine race window (awakened turn still holding the slot while prose reaches dispatch) is a property of the dispatch layer and is better covered by an integration-level test against the real agent loop, or pinned where that loop serializes per-session turns.

What would you prefer — (a) I open a follow-up issue capturing the turn-serialization invariant (with your suggested deterministic test sketch) so it's scoped to the executor and doesn't get lost, or (b) you'd like me to attempt an integration test in this PR against the run-agent loop (heavier setup, but covers it end-to-end)? I'm happy to add (a) now and do (b) if you think it belongs in this change.