diff --git a/libs/code/deepagents_code/_ask_user_types.py b/libs/code/deepagents_code/_ask_user_types.py index b5ad6fcdde..dc79f2c6d5 100644 --- a/libs/code/deepagents_code/_ask_user_types.py +++ b/libs/code/deepagents_code/_ask_user_types.py @@ -195,9 +195,11 @@ class AskUserCancelled(TypedDict): Rewording it changes that hook contract. Not rendered on any row: a live cancel calls `set_rejected` (which records no output), and a transcript of `(cancelled)` placeholders from a non-TUI client is summarized from the recorded status like -any other, so it reads as `ASK_USER_ANSWERED_SUMMARY`. The cancel banner in -`textual_adapter` shares this wording but deliberately not this constant — it is -user-facing prose, not the hook contract. +any other, so it reads as `ASK_USER_ANSWERED_SUMMARY`. The dismissal banner in +`textual_adapter` deliberately does not use this constant, and no longer even +shares its wording — the banner says "dismissed" where this says "cancelled". +That divergence is intentional: the banner is user-facing prose free to be +reworded, this is the hook contract. Do not "de-duplicate" them. """ ASK_USER_FAILED_SUMMARY: AskUserRowSummary = "Question failed" diff --git a/libs/code/deepagents_code/tui/textual_adapter.py b/libs/code/deepagents_code/tui/textual_adapter.py index 69f2afe061..1b18b90f77 100644 --- a/libs/code/deepagents_code/tui/textual_adapter.py +++ b/libs/code/deepagents_code/tui/textual_adapter.py @@ -2313,6 +2313,7 @@ async def _after_automatic_compact() -> None: if interrupt_occurred: any_rejected = False ask_user_cancelled = False + dismissed_question_count = 0 resume_payload: dict[str, Any] = dict(pending_hook_resumes) # Tools mounted above start their spinner immediately, but a @@ -2485,6 +2486,12 @@ async def _after_automatic_compact() -> None: # Halt the turn on cancel; error branches still # resume so the agent can react to the failure. ask_user_cancelled = True + # Counts questions, not calls, purely so the banner + # below can pick a singular or plural subject — the + # halt reads the flag above, never this. A widget + # dismisses its whole prompt, so every question in a + # cancelled call went with it. + dismissed_question_count += len(questions) tool_msg = adapter._current_tool_messages.pop(tool_id, None) output = ASK_USER_CANCELLED_SUMMARY _dispatch_tool_error_hook("ask_user") @@ -2948,18 +2955,31 @@ async def _after_automatic_compact() -> None: tool_id, ) + dismissed_subject = ( + "Questions" if dismissed_question_count > 1 else "Question" + ) message = ( - "Question cancelled. Tell the agent what you'd like instead." + f"{dismissed_subject} dismissed. Tell the agent what you'd " + "like instead." if ask_user_cancelled else "Command rejected. Tell the agent what you'd like instead." ) if undelivered: # The user typed answers and they are now gone; saying so # is the only way they learn not to wait for a response. + # Which event destroyed them differs: a dismissal in this + # batch, or — when `pending_ask_user` is empty because it + # resets each stream iteration — a rejection in a later + # iteration discarding an earlier one's answered row. + cause = ( + f"{dismissed_subject} dismissed" + if ask_user_cancelled + else "Command rejected" + ) message = ( - "Question cancelled, so answers to the other " - "question(s) in this batch were not sent. Tell the " - "agent what you'd like instead." + f"{cause}, so answers to the other question(s) in this " + "batch were not sent. Tell the agent what you'd like " + "instead." ) await adapter._mount_message(AppMessage(message)) turn_stats.wall_time_seconds = time.monotonic() - start_time diff --git a/libs/code/tests/unit_tests/tui/test_textual_adapter.py b/libs/code/tests/unit_tests/tui/test_textual_adapter.py index badb95380e..be98611e20 100644 --- a/libs/code/tests/unit_tests/tui/test_textual_adapter.py +++ b/libs/code/tests/unit_tests/tui/test_textual_adapter.py @@ -5383,7 +5383,17 @@ async def request_ask_user( tool_rows = [w for w in mounted if isinstance(w, ToolCallMessage)] assert len(tool_rows) == 1 - async def test_ask_user_cancelled_marks_row_rejected_and_halts(self) -> None: + @pytest.mark.parametrize( + ("question_count", "expected_message"), + [ + (0, "Question dismissed. Tell the agent what you'd like instead."), + (1, "Question dismissed. Tell the agent what you'd like instead."), + (2, "Questions dismissed. Tell the agent what you'd like instead."), + ], + ) + async def test_ask_user_cancelled_marks_row_rejected_and_halts( + self, question_count: int, expected_message: str + ) -> None: """Cancelled result should reject the row and not resume generation.""" mounted: list[object] = [] token_events: list[str] = [] @@ -5406,7 +5416,10 @@ async def request_ask_user( _ask_user_interrupt_chunk( { "type": "ask_user", - "questions": [{"question": "Name?", "type": "text"}], + "questions": [ + {"question": f"Question {index}?", "type": "text"} + for index in range(1, question_count + 1) + ], "tool_call_id": "tool-1", } ) @@ -5437,9 +5450,166 @@ async def request_ask_user( assert "tool-1" not in adapter._current_tool_messages app_messages = [widget for widget in mounted if isinstance(widget, AppMessage)] assert len(app_messages) == 1 - assert "Question cancelled" in str(app_messages[0]._content) + assert str(app_messages[0]._content) == expected_message assert token_events == ["pending", "show:False"] + async def test_dismissed_questions_accumulate_across_cancelled_calls(self) -> None: + """Two dismissed calls of one question each read as plural. + + The subject counts questions across every cancelled call in the batch, not + questions within one call, so two single-question prompts still say + "Questions". Pins the accumulation: overwriting instead of adding would + leave the count at 1 and silently read as singular. + """ + mounted: list[object] = [] + + async def mount_message(widget: object) -> None: + await asyncio.sleep(0) + mounted.append(widget) + + async def request_ask_user( + _questions: list[Question], + ) -> asyncio.Future[AskUserWidgetResult] | None: + await asyncio.sleep(0) + future: asyncio.Future[AskUserWidgetResult] = asyncio.Future() + future.set_result({"type": "cancelled"}) + return future + + agent = _SequencedAgent( + streams_by_call=[ + [ + ( + (), + "updates", + { + "__interrupt__": [ + SimpleNamespace( + id=f"interrupt-{index}", + value={ + "type": "ask_user", + "questions": [ + { + "question": f"Deploy {index}?", + "type": "text", + } + ], + "tool_call_id": f"ask-{index}", + }, + ) + for index in (1, 2) + ] + }, + ) + ], + ] + ) + adapter = TextualUIAdapter( + mount_message=mount_message, + update_status=_noop_status, + request_approval=_mock_approval, + request_ask_user=request_ask_user, + ) + + await execute_task_textual( + user_input="hello", + agent=agent, + assistant_id="assistant", + session_state=_session_state(auto_approve=False), + adapter=adapter, + ) + + app_messages = [widget for widget in mounted if isinstance(widget, AppMessage)] + assert len(app_messages) == 1 + assert str(app_messages[0]._content) == ( + "Questions dismissed. Tell the agent what you'd like instead." + ) + + async def test_undelivered_banner_names_a_rejection_when_nothing_was_dismissed( + self, + ) -> None: + """A later-iteration rejection discards earlier answers and says so. + + `pending_ask_user` resets each stream iteration, so a rejection in a second + iteration enters the halt branch via `not pending_ask_user` while the first + iteration's answered row is still awaiting its deferred result. Those + answers are discarded exactly as a dismissal would discard them, but no + question was dismissed — the banner must name the rejection instead. + """ + mounted: list[object] = [] + approval: asyncio.Future[object] = asyncio.Future() + approval.set_result({"type": "reject"}) + + async def mount_message(widget: object) -> None: + await asyncio.sleep(0) + mounted.append(widget) + + async def request_approval( + _action_requests: list[dict[str, Any]], + _assistant_id: str | None, + ) -> asyncio.Future[object]: + await asyncio.sleep(0) + return approval + + async def request_ask_user( + _questions: list[Question], + ) -> asyncio.Future[AskUserWidgetResult] | None: + await asyncio.sleep(0) + future: asyncio.Future[AskUserWidgetResult] = asyncio.Future() + future.set_result({"type": "answered", "answers": ["Alice"]}) + return future + + agent = _SequencedAgent( + streams_by_call=[ + [ + _ask_user_interrupt_chunk( + { + "type": "ask_user", + "questions": [{"question": "Name?", "type": "text"}], + "tool_call_id": "ask-1", + } + ) + ], + [ + _hitl_interrupt_chunk( + { + "action_requests": [ + {"name": "read_file", "args": {"path": "notes.txt"}} + ], + "review_configs": [ + { + "action_name": "read_file", + "allowed_decisions": ["approve", "reject"], + } + ], + } + ) + ], + [], + ] + ) + adapter = TextualUIAdapter( + mount_message=mount_message, + update_status=_noop_status, + request_approval=request_approval, + request_ask_user=request_ask_user, + ) + + await execute_task_textual( + user_input="hello", + agent=agent, + assistant_id="assistant", + session_state=_session_state(auto_approve=False), + adapter=adapter, + ) + + app_messages = [widget for widget in mounted if isinstance(widget, AppMessage)] + assert [str(widget._content) for widget in app_messages] == [ + ( + "Command rejected, so answers to the other question(s) in this " + "batch were not sent. Tell the agent what you'd like instead." + ) + ] + async def test_hitl_rejection_restores_token_display_before_halt(self) -> None: """Rejected approval should restore tokens before returning early.""" mounted: list[object] = [] @@ -8369,8 +8539,14 @@ async def request_approval( if c[0][0] == "tool.error" ] == [["execute"]] + @pytest.mark.parametrize( + ("cancelled_question_count", "expected_subject"), + [(1, "Question"), (2, "Questions")], + ) async def test_answered_ask_user_settles_when_a_sibling_is_cancelled( self, + cancelled_question_count: int, + expected_subject: str, ) -> None: """Cancelling one `ask_user` call reports an answered sibling as undelivered. @@ -8391,13 +8567,20 @@ async def test_answered_ask_user_settles_when_a_sibling_is_cancelled( `ASK_USER_ANSWERED_NOT_DELIVERED_SUMMARY`, distinct from `ASK_USER_ANSWERED_NO_RESULT_SUMMARY` (answers delivered, tool never completed). + + The banner naming that loss is asserted here too, parametrized so the + dismissed subject is exercised in both singular and plural: this is the + only path that renders it inside the longer sentence. """ mounted: list[ToolCallMessage] = [] + app_messages: list[AppMessage] = [] async def mount_message(widget: object) -> None: await asyncio.sleep(0) if isinstance(widget, ToolCallMessage): mounted.append(widget) + elif isinstance(widget, AppMessage): + app_messages.append(widget) results: list[AskUserWidgetResult] = [ {"type": "answered", "answers": ["Alice"]}, @@ -8413,7 +8596,10 @@ async def request_ask_user( return future answered_qs: list[Question] = [{"question": "Name?", "type": "text"}] - cancelled_qs: list[Question] = [{"question": "Deploy?", "type": "text"}] + cancelled_qs: list[Question] = [ + {"question": f"Deploy {index}?", "type": "text"} + for index in range(1, cancelled_question_count + 1) + ] agent = _SequencedAgent( streams_by_call=[ [ @@ -8496,6 +8682,13 @@ async def request_ask_user( assert "Alice" not in str(mock_dispatch_background.call_args_list) assert payloads["ask-2"]["tool_status"] == "error" assert payloads["ask-2"]["tool_output"] == ASK_USER_CANCELLED_SUMMARY + # The banner is the user's only signal that the answers are gone, so pin + # it exactly — a dropped space in the implicit concatenation would show. + assert len(app_messages) == 1 + assert str(app_messages[0]._content) == ( + f"{expected_subject} dismissed, so answers to the other question(s) " + "in this batch were not sent. Tell the agent what you'd like instead." + ) @pytest.mark.parametrize( ("tool_status", "transcript"),