From d08be750d962608039ea4324be5eae968937fe59 Mon Sep 17 00:00:00 2001 From: Mason Daugherty <61371264+mdrxy@users.noreply.github.com> Date: Wed, 29 Jul 2026 15:45:01 +0000 Subject: [PATCH 1/2] fix(code): keep the `/goal` criteria prompt responsive `/goal` is dispatched from `on_chat_input_submitted`, which is awaited inline on the Textual message pump, and the handler awaited the one-time Auto criteria preference modal in that chain. The pump therefore stayed blocked while the modal was open, so it never received the Enter/Esc keys it needs to resolve and the terminal looked frozen until the ten-minute watchdog fired. Run the flow off the pump when the prompt is still owed, so the handler returns and keys reach the modal. Co-authored-by: open-swe[bot] --- libs/code/deepagents_code/app.py | 158 +++++++++++++++++++++---- libs/code/tests/unit_tests/test_app.py | 136 ++++++++++++++++++++- 2 files changed, 266 insertions(+), 28 deletions(-) diff --git a/libs/code/deepagents_code/app.py b/libs/code/deepagents_code/app.py index 1aa2cabe291..f7e4c14f706 100644 --- a/libs/code/deepagents_code/app.py +++ b/libs/code/deepagents_code/app.py @@ -3327,6 +3327,14 @@ def __init__( self._goal_proposal_worker: Worker[None] | None = None """Active worker drafting or mounting a goal criteria proposal.""" + self._goal_preference_task: asyncio.Task[None] | None = None + """`/goal` flow detached from the App message pump. + + Set when the one-time Auto criteria preference modal is still owed, so + the flow cannot be awaited on the pump (see `_start_goal_proposal`). + Holding the reference keeps the task from being GC'd mid-flight and lets + tests await the continuation deterministically.""" + self._goal_review_task: asyncio.Task[None] | None = None """Active task awaiting a mounted goal criteria review decision.""" @@ -8496,25 +8504,36 @@ def _dismiss_orphaned_screen(self, screen: ModalScreen[Any]) -> None: if self.screen is screen: screen.dismiss() - async def _ensure_goal_auto_accept_preference(self) -> None: - """Ask once, on the first Auto-mode goal, how to handle criteria. + def _goal_auto_accept_prompt_pending(self) -> bool: + """Return whether a `/goal` action still owes the Auto criteria prompt. - Only prompts in Auto mode: the preference has no effect in Manual + Only Auto mode is prompted: the preference has no effect in Manual (always reviews) or YOLO (always applies), so a Manual/YOLO user is asked the first time they act on a goal after switching to Auto instead. - Fail closed to review when the modal times out or fails to mount. Saving - still records the one-time marker so a wedged prompt cannot loop, and the - orphaned modal is dismissed so it cannot linger over goal drafting. + Callers use this to decide whether the goal flow must run off the App + message pump before it can open the modal. """ if self._session_state is None: - return + return False from deepagents_code.approval_mode import ApprovalMode mode = getattr(self._session_state, "approval_mode", None) if mode is not ApprovalMode.AUTO: - return - if not self._should_prompt_goal_auto_accept_preference(): + return False + return self._should_prompt_goal_auto_accept_preference() + + async def _ensure_goal_auto_accept_preference(self) -> None: + """Ask once, on the first Auto-mode goal, how to handle criteria. + + Fail closed to review when the modal times out or fails to mount. Saving + still records the one-time marker so a wedged prompt cannot loop, and the + orphaned modal is dismissed so it cannot linger over goal drafting. + + Must not be awaited on the App message pump — see + `_start_goal_proposal`. + """ + if not self._goal_auto_accept_prompt_pending(): return from deepagents_code.tui.widgets.launch_init import ( @@ -11041,15 +11060,9 @@ async def _handle_goal_command(self, command: str) -> None: if not grader_arg: await self._mount_message(AppMessage("Usage: /goal amend ")) return - await self._ensure_goal_auto_accept_preference() - self._cancel_goal_proposal_worker() - await self._cancel_pending_goal_review(context="goal-amend cleanup") - async with self._goal_state_mutation_boundary(): - self._clear_pending_goal_rubric() - await self._persist_goal_rubric_state() - self._goal_proposal_worker = self.run_worker( - self._propose_goal_amendment(grader_arg), - exclusive=False, + await self._start_goal_proposal( + lambda: self._propose_goal_amendment(grader_arg), + cleanup_context="goal-amend cleanup", ) return @@ -11092,16 +11105,108 @@ async def _handle_goal_command(self, command: str) -> None: objective = remainder await self._mount_message(UserMessage(command)) - await self._ensure_goal_auto_accept_preference() + await self._start_goal_proposal( + lambda: self._propose_goal_rubric(objective), + cleanup_context="goal replacement cleanup", + ) + + async def _start_goal_proposal( + self, + proposal: Callable[[], Coroutine[Any, Any, None]], + *, + cleanup_context: str, + ) -> None: + """Begin a goal proposal, asking for the Auto criteria policy first. + + The one-time preference modal is awaited, and `/goal` is dispatched from + `on_chat_input_submitted`, which is itself awaited inline on the App + message pump. Awaiting the modal there blocks the pump, so it never + receives the Enter/Esc key events it needs to resolve and the whole app + looks frozen until the modal watchdog fires. When the prompt is still + owed, the flow therefore runs as a detached task so the handler returns + and the pump stays free to route keys to the modal. This mirrors the + install-then-switch flow, which runs its credential modal off the pump + for the same reason. + + Without a pending prompt (the overwhelmingly common case) nothing opens a + modal, so the flow stays inline and `/goal` keeps drafting within the + submission that asked for it. + + Args: + proposal: Builds the drafting coroutine to hand to the goal proposal + worker. Deferred so a preflight failure cannot strand an + un-awaited coroutine. + cleanup_context: Label for the pending-review cancellation log. + """ + if self._goal_auto_accept_prompt_pending(): + # Returning while the prompt is open means another `/goal` can arrive + # (an external caller, or a queue drain), so refuse a second flow + # rather than stacking modals over an unanswered one. + pending = self._goal_preference_task + if pending is not None and not pending.done(): + self.notify( + "Answer the goal criteria prompt first.", + severity="warning", + timeout=5, + ) + return + task = asyncio.create_task( + self._prompt_then_start_goal_proposal( + proposal, + cleanup_context=cleanup_context, + ), + name="goal-criteria-preference", + ) + self._goal_preference_task = task + task.add_done_callback(_log_task_exception) + return + await self._start_goal_proposal_now(proposal, cleanup_context=cleanup_context) + + async def _prompt_then_start_goal_proposal( + self, + proposal: Callable[[], Coroutine[Any, Any, None]], + *, + cleanup_context: str, + ) -> None: + """Answer the Auto criteria prompt, then start the goal proposal. + + Runs off the App message pump (see `_start_goal_proposal`) so the + preference modal can receive keys. + + Args: + proposal: Builds the drafting coroutine for the proposal worker. + cleanup_context: Label for the pending-review cancellation log. + """ + try: + await self._ensure_goal_auto_accept_preference() + await self._start_goal_proposal_now( + proposal, + cleanup_context=cleanup_context, + ) + except Exception: + logger.exception("Failed to start goal proposal after criteria prompt") + await self._mount_message( + ErrorMessage("Could not start the goal. Please try again."), + ) + + async def _start_goal_proposal_now( + self, + proposal: Callable[[], Coroutine[Any, Any, None]], + *, + cleanup_context: str, + ) -> None: + """Clear stale goal proposal state and start the drafting worker. + + Args: + proposal: Builds the drafting coroutine for the proposal worker. + cleanup_context: Label for the pending-review cancellation log. + """ self._cancel_goal_proposal_worker() - await self._cancel_pending_goal_review(context="goal replacement cleanup") + await self._cancel_pending_goal_review(context=cleanup_context) async with self._goal_state_mutation_boundary(): self._clear_pending_goal_rubric() await self._persist_goal_rubric_state() - self._goal_proposal_worker = self.run_worker( - self._propose_goal_rubric(objective), - exclusive=False, - ) + self._goal_proposal_worker = self.run_worker(proposal(), exclusive=False) @staticmethod def _goal_usage_text() -> str: @@ -16638,6 +16743,11 @@ def exit( self._agent_worker.cancel() if self._git_branch_refresh_task is not None: self._git_branch_refresh_task.cancel() + # A `/goal` flow parked on the criteria preference modal is detached from + # the message pump, so nothing else would cancel it before the loop is + # torn down. + if self._goal_preference_task is not None: + self._goal_preference_task.cancel() if self._external_event_source_task is not None: self._external_event_source_task.cancel() # Cancellation alone is not enough: the task's `finally` block runs diff --git a/libs/code/tests/unit_tests/test_app.py b/libs/code/tests/unit_tests/test_app.py index e61a21cfa0e..c00ab46a2e9 100644 --- a/libs/code/tests/unit_tests/test_app.py +++ b/libs/code/tests/unit_tests/test_app.py @@ -965,9 +965,9 @@ async def test_goal_create_prompts_for_auto_accept_preference( async with app.run_test() as pilot: await pilot.pause() self._force_auto_mode(app) - command_task = asyncio.create_task( - app._handle_goal_command("/goal ship passkeys"), - ) + # The command returns once the prompt is detached from the message + # pump, so the rest of the flow is awaited via its task. + await app._handle_goal_command("/goal ship passkeys") await pilot.pause() assert isinstance(app.screen, LaunchGoalCriteriaPreferenceScreen) @@ -976,7 +976,8 @@ async def test_goal_create_prompts_for_auto_accept_preference( assert options.highlighted == 0 await pilot.press("escape") - await asyncio.wait_for(command_task, timeout=2) + assert app._goal_preference_task is not None + await asyncio.wait_for(app._goal_preference_task, timeout=2) await pilot.pause() assert "auto_accept_criteria = false" in DEFAULT_CONFIG_PATH.read_text( @@ -986,6 +987,133 @@ async def test_goal_create_prompts_for_auto_accept_preference( assert DeepAgentsApp._should_prompt_goal_auto_accept_preference() is False assert app._goal_proposal_worker is not None + async def test_goal_preference_prompt_responsive_through_message_pump( + self, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + """The criteria prompt must accept keys via the real submission path. + + Regression test: `/goal ` is dispatched from the App's + `on_chat_input_submitted` handler, which is awaited inline on the App + message pump. Awaiting the preference modal anywhere in that chain blocks + the pump, so the modal never receives the Enter/Esc key events it needs + to resolve and the whole app looks frozen until the watchdog fires. The + flow now runs off the pump, so submitting through the real + `ChatInput.Submitted` path and pressing Escape must resolve the modal. + """ + from deepagents_code._env_vars import GOAL_AUTO_ACCEPT_CRITERIA + from deepagents_code.tui.widgets.chat_input import ChatInput + + monkeypatch.delenv(GOAL_AUTO_ACCEPT_CRITERIA, raising=False) + self._clear_goal_auto_accept_prompt_marker() + app = DeepAgentsApp(agent=MagicMock()) + app.run_worker = MagicMock() # ty: ignore + + async with app.run_test() as pilot: + await pilot.pause() + self._force_auto_mode(app) + # Idle session, so the submission is processed instead of queued. + app._agent_running = False + app._connecting = False + app._startup_sequence_running = False + app._server_startup_error = None + + assert app._chat_input is not None + app._chat_input.post_message( + ChatInput.Submitted("/goal ship passkeys", "command"), + ) + for _ in range(60): + await pilot.pause(0.05) + if isinstance(app.screen, LaunchGoalCriteriaPreferenceScreen): + break + assert isinstance(app.screen, LaunchGoalCriteriaPreferenceScreen) + + # With the pump free, Escape must reach the modal and resolve it. + await pilot.press("escape") + for _ in range(60): + await pilot.pause(0.05) + if not isinstance(app.screen, LaunchGoalCriteriaPreferenceScreen): + break + assert not isinstance(app.screen, LaunchGoalCriteriaPreferenceScreen) + assert app._goal_preference_task is not None + await asyncio.wait_for(app._goal_preference_task, timeout=2) + + assert app._goal_proposal_worker is not None + + async def test_second_goal_refused_while_criteria_prompt_is_open( + self, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + """An unanswered criteria prompt is not stacked under a second `/goal`.""" + from deepagents_code._env_vars import GOAL_AUTO_ACCEPT_CRITERIA + + monkeypatch.delenv(GOAL_AUTO_ACCEPT_CRITERIA, raising=False) + self._clear_goal_auto_accept_prompt_marker() + app = DeepAgentsApp(agent=MagicMock()) + app.run_worker = MagicMock() # ty: ignore + + async with app.run_test() as pilot: + await pilot.pause() + self._force_auto_mode(app) + await app._handle_goal_command("/goal ship passkeys") + await pilot.pause() + first_task = app._goal_preference_task + assert first_task is not None + + await app._handle_goal_command("/goal ship magic links") + await pilot.pause() + + assert app._goal_preference_task is first_task + assert isinstance(app.screen, LaunchGoalCriteriaPreferenceScreen) + assert len(app.screen_stack) == 2 + + await pilot.press("escape") + await asyncio.wait_for(first_task, timeout=2) + + async def test_goal_amend_prompt_responsive_through_message_pump( + self, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + """`/goal amend` shares the create path's off-pump preference prompt.""" + from deepagents_code._env_vars import GOAL_AUTO_ACCEPT_CRITERIA + from deepagents_code.tui.widgets.chat_input import ChatInput + + monkeypatch.delenv(GOAL_AUTO_ACCEPT_CRITERIA, raising=False) + self._clear_goal_auto_accept_prompt_marker() + app = DeepAgentsApp(agent=MagicMock()) + app.run_worker = MagicMock() # ty: ignore + + async with app.run_test() as pilot: + await pilot.pause() + self._force_auto_mode(app) + app._agent_running = False + app._connecting = False + app._startup_sequence_running = False + app._server_startup_error = None + app._active_goal = "ship passkeys" + app._goal_status = "active" + + assert app._chat_input is not None + app._chat_input.post_message( + ChatInput.Submitted("/goal amend also cover recovery", "command"), + ) + for _ in range(60): + await pilot.pause(0.05) + if isinstance(app.screen, LaunchGoalCriteriaPreferenceScreen): + break + assert isinstance(app.screen, LaunchGoalCriteriaPreferenceScreen) + + await pilot.press("escape") + for _ in range(60): + await pilot.pause(0.05) + if not isinstance(app.screen, LaunchGoalCriteriaPreferenceScreen): + break + assert not isinstance(app.screen, LaunchGoalCriteriaPreferenceScreen) + assert app._goal_preference_task is not None + await asyncio.wait_for(app._goal_preference_task, timeout=2) + + assert app._goal_proposal_worker is not None + async def test_goal_create_skips_preference_prompt_when_decided( self, monkeypatch: pytest.MonkeyPatch, From 590f4c4f6dd80da835f250ddc2d86467fba3e883 Mon Sep 17 00:00:00 2001 From: Mason Daugherty Date: Thu, 30 Jul 2026 14:40:40 -0400 Subject: [PATCH 2/2] fix(code): route goal criteria prompt through _schedule_off_message_pump MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the bespoke _goal_preference_task mechanism with the shared _schedule_off_message_pump helper. This registers the continuation in _modal_command_tasks synchronously, so the queue drain's busy check holds queued messages while the criteria prompt is unanswered — closing a gap where a dequeued /goal released the queue and let a following agent turn start concurrently with goal drafting. Also gains centralized exit cancellation (awaited in teardown), named failure logging, and the drain-after-modal-command queue resume. Generalize the helper's refusal toast now that it covers goal commands, and add a regression test driving a queued /goal + normal message through the real drain path. --- libs/code/deepagents_code/app.py | 59 +++----- libs/code/tests/unit_tests/test_app.py | 197 ++++++++++++++++++++----- 2 files changed, 175 insertions(+), 81 deletions(-) diff --git a/libs/code/deepagents_code/app.py b/libs/code/deepagents_code/app.py index 65278d7ded4..ee45ead650c 100644 --- a/libs/code/deepagents_code/app.py +++ b/libs/code/deepagents_code/app.py @@ -3453,14 +3453,6 @@ def __init__( self._goal_proposal_worker: Worker[None] | None = None """Active worker drafting or mounting a goal criteria proposal.""" - self._goal_preference_task: asyncio.Task[None] | None = None - """`/goal` flow detached from the App message pump. - - Set when the one-time Auto criteria preference modal is still owed, so - the flow cannot be awaited on the pump (see `_start_goal_proposal`). - Holding the reference keeps the task from being GC'd mid-flight and lets - tests await the continuation deterministically.""" - self._goal_review_task: asyncio.Task[None] | None = None """Active task awaiting a mounted goal criteria review decision.""" @@ -11655,10 +11647,10 @@ async def _start_goal_proposal( message pump. Awaiting the modal there blocks the pump, so it never receives the Enter/Esc key events it needs to resolve and the whole app looks frozen until the modal watchdog fires. When the prompt is still - owed, the flow therefore runs as a detached task so the handler returns - and the pump stays free to route keys to the modal. This mirrors the - install-then-switch flow, which runs its credential modal off the pump - for the same reason. + owed, the flow therefore hands off to `_schedule_off_message_pump` so + the handler returns and the pump stays free to route keys to the modal. + This mirrors the `/update` and `/install --package` confirmations, which + detach for the same reason. Without a pending prompt (the overwhelmingly common case) nothing opens a modal, so the flow stays inline and `/goal` keeps drafting within the @@ -11671,26 +11663,13 @@ async def _start_goal_proposal( cleanup_context: Label for the pending-review cancellation log. """ if self._goal_auto_accept_prompt_pending(): - # Returning while the prompt is open means another `/goal` can arrive - # (an external caller, or a queue drain), so refuse a second flow - # rather than stacking modals over an unanswered one. - pending = self._goal_preference_task - if pending is not None and not pending.done(): - self.notify( - "Answer the goal criteria prompt first.", - severity="warning", - timeout=5, - ) - return - task = asyncio.create_task( + self._schedule_off_message_pump( self._prompt_then_start_goal_proposal( proposal, cleanup_context=cleanup_context, ), - name="goal-criteria-preference", + context="goal:criteria-preference", ) - self._goal_preference_task = task - task.add_done_callback(_log_task_exception) return await self._start_goal_proposal_now(proposal, cleanup_context=cleanup_context) @@ -11702,8 +11681,10 @@ async def _prompt_then_start_goal_proposal( ) -> None: """Answer the Auto criteria prompt, then start the goal proposal. - Runs off the App message pump (see `_start_goal_proposal`) so the - preference modal can receive keys. + Must be scheduled via `_schedule_off_message_pump`, never awaited from a + command handler, so the preference modal can receive keys. It therefore + catches its own exceptions, because the handler's `try/except` no longer + wraps it. Args: proposal: Builds the drafting coroutine for the proposal worker. @@ -17329,11 +17310,6 @@ def exit( self._agent_worker.cancel() if self._git_branch_refresh_task is not None: self._git_branch_refresh_task.cancel() - # A `/goal` flow parked on the criteria preference modal is detached from - # the message pump, so nothing else would cancel it before the loop is - # torn down. - if self._goal_preference_task is not None: - self._goal_preference_task.cancel() if self._external_event_source_task is not None: self._external_event_source_task.cancel() # Cancellation alone is not enough: the task's `finally` block runs @@ -21858,9 +21834,9 @@ def _schedule_off_message_pump( Because the command handler now returns while the modal is still open, the continuation participates in the app's busy state until it ends. - Only one continuation is allowed globally, so differently keyed install - and update commands cannot show overlapping modals or mutate the tool - environment concurrently. + Only one continuation is allowed globally, so differently keyed install, + update, and goal commands cannot show overlapping modals or mutate + shared state concurrently. The continuation runs outside the command handler's `try/except` and is cancelled at app exit, possibly mid-install, so each one must catch its @@ -21880,12 +21856,11 @@ def _schedule_off_message_pump( if self._modal_command_running(): coro.close() # Covers the whole continuation, not just its modal: after the - # prompt is answered the install or refresh can still be running for - # minutes, and pointing the user at a prompt that has already closed - # would read as a bug. + # prompt is answered the install, refresh, or goal proposal can + # still be running for minutes, and pointing the user at a prompt + # that has already closed would read as a bug. self.notify( - "Another install or update is in progress — answer its prompt if " - "one is open.", + "Another command is waiting on a prompt — answer it first.", severity="warning", timeout=5, ) diff --git a/libs/code/tests/unit_tests/test_app.py b/libs/code/tests/unit_tests/test_app.py index 6cb7da2bbc3..67720117e31 100644 --- a/libs/code/tests/unit_tests/test_app.py +++ b/libs/code/tests/unit_tests/test_app.py @@ -33,6 +33,12 @@ from deepagents_code.sessions import ThreadInfo from deepagents_code.tui.widgets.messages import ToolCallMessage + # `conftest`'s fixture protocols cannot be imported here: `tool.ty` + # `extra-paths` puts `libs/deepagents` on the path too, so + # `tests.unit_tests.conftest` is ambiguous across packages. + DrainModalCommands = Callable[..., Awaitable[None]] + WaitForModal = Callable[..., Awaitable[None]] + import pytest from textual import events from textual.app import App, ComposeResult, ScreenStackError @@ -995,6 +1001,7 @@ def _force_auto_mode(app: DeepAgentsApp) -> None: async def test_goal_create_prompts_for_auto_accept_preference( self, monkeypatch: pytest.MonkeyPatch, + drain_modal_commands: DrainModalCommands, ) -> None: """First Auto-mode create/amend should ask before drafting criteria.""" from deepagents_code._env_vars import GOAL_AUTO_ACCEPT_CRITERIA @@ -1018,7 +1025,7 @@ async def test_goal_create_prompts_for_auto_accept_preference( await pilot.pause() self._force_auto_mode(app) # The command returns once the prompt is detached from the message - # pump, so the rest of the flow is awaited via its task. + # pump, so the rest of the flow is awaited via the continuation. await app._handle_goal_command("/goal ship passkeys") await pilot.pause() @@ -1028,8 +1035,7 @@ async def test_goal_create_prompts_for_auto_accept_preference( assert options.highlighted == 0 await pilot.press("escape") - assert app._goal_preference_task is not None - await asyncio.wait_for(app._goal_preference_task, timeout=2) + await drain_modal_commands(app) await pilot.pause() assert "auto_accept_criteria = false" in DEFAULT_CONFIG_PATH.read_text( @@ -1042,6 +1048,8 @@ async def test_goal_create_prompts_for_auto_accept_preference( async def test_goal_preference_prompt_responsive_through_message_pump( self, monkeypatch: pytest.MonkeyPatch, + drain_modal_commands: DrainModalCommands, + wait_for_modal: WaitForModal, ) -> None: """The criteria prompt must accept keys via the real submission path. @@ -1050,8 +1058,9 @@ async def test_goal_preference_prompt_responsive_through_message_pump( message pump. Awaiting the preference modal anywhere in that chain blocks the pump, so the modal never receives the Enter/Esc key events it needs to resolve and the whole app looks frozen until the watchdog fires. The - flow now runs off the pump, so submitting through the real - `ChatInput.Submitted` path and pressing Escape must resolve the modal. + flow now runs off the pump (`_schedule_off_message_pump`), so submitting + through the real `ChatInput.Submitted` path and pressing Escape must + resolve the modal. """ from deepagents_code._env_vars import GOAL_AUTO_ACCEPT_CRITERIA from deepagents_code.tui.widgets.chat_input import ChatInput @@ -1074,27 +1083,23 @@ async def test_goal_preference_prompt_responsive_through_message_pump( app._chat_input.post_message( ChatInput.Submitted("/goal ship passkeys", "command"), ) - for _ in range(60): - await pilot.pause(0.05) - if isinstance(app.screen, LaunchGoalCriteriaPreferenceScreen): - break - assert isinstance(app.screen, LaunchGoalCriteriaPreferenceScreen) + await wait_for_modal( + pilot, LaunchGoalCriteriaPreferenceScreen, present=True + ) # With the pump free, Escape must reach the modal and resolve it. await pilot.press("escape") - for _ in range(60): - await pilot.pause(0.05) - if not isinstance(app.screen, LaunchGoalCriteriaPreferenceScreen): - break - assert not isinstance(app.screen, LaunchGoalCriteriaPreferenceScreen) - assert app._goal_preference_task is not None - await asyncio.wait_for(app._goal_preference_task, timeout=2) + await wait_for_modal( + pilot, LaunchGoalCriteriaPreferenceScreen, present=False + ) + await drain_modal_commands(app) assert app._goal_proposal_worker is not None async def test_second_goal_refused_while_criteria_prompt_is_open( self, monkeypatch: pytest.MonkeyPatch, + drain_modal_commands: DrainModalCommands, ) -> None: """An unanswered criteria prompt is not stacked under a second `/goal`.""" from deepagents_code._env_vars import GOAL_AUTO_ACCEPT_CRITERIA @@ -1109,22 +1114,98 @@ async def test_second_goal_refused_while_criteria_prompt_is_open( self._force_auto_mode(app) await app._handle_goal_command("/goal ship passkeys") await pilot.pause() - first_task = app._goal_preference_task - assert first_task is not None + first_tasks = dict(app._modal_command_tasks) + assert list(first_tasks) == ["goal:criteria-preference"] await app._handle_goal_command("/goal ship magic links") await pilot.pause() - assert app._goal_preference_task is first_task + assert dict(app._modal_command_tasks) == first_tasks assert isinstance(app.screen, LaunchGoalCriteriaPreferenceScreen) assert len(app.screen_stack) == 2 await pilot.press("escape") - await asyncio.wait_for(first_task, timeout=2) + await drain_modal_commands(app) + + async def test_pending_criteria_prompt_holds_queue_until_answered( + self, + monkeypatch: pytest.MonkeyPatch, + drain_modal_commands: DrainModalCommands, + wait_for_modal: WaitForModal, + ) -> None: + """A dequeued `/goal` does not release the queue while its prompt is up. + + Regression test: when `/goal` is dequeued with a normal message behind + it, detaching the criteria prompt returns the handler before any busy + flag is set. The continuation is registered in `_modal_command_tasks` + synchronously, so the drain's busy check holds the queue instead of + starting an agent turn that would overlap the goal drafting the prompt + is about to start. Answering the prompt resumes the queue only after the + proposal worker is running. + """ + from deepagents_code._env_vars import GOAL_AUTO_ACCEPT_CRITERIA + + monkeypatch.delenv(GOAL_AUTO_ACCEPT_CRITERIA, raising=False) + app = DeepAgentsApp(agent=MagicMock()) + app.run_worker = MagicMock() # ty: ignore + processed: list[str] = [] + + async def _process(text: str, mode: str) -> None: + if mode == "command" and text.startswith("/goal"): + await app._handle_goal_command(text) + else: + processed.append(text) + + app._process_message = _process # ty: ignore + + async with app.run_test() as pilot: + await pilot.pause() + self._clear_goal_auto_accept_prompt_marker() + self._force_auto_mode(app) + # Idle session, so the drain processes instead of deferring. + app._agent_running = False + app._connecting = False + app._startup_sequence_running = False + app._server_startup_error = None + # Seed the queue inside the running app: startup drains any messages + # queued before `run_test()`. + app._pending_messages.extend( + [ + QueuedMessage(text="/goal ship passkeys", mode="command"), + QueuedMessage(text="now do the thing", mode="normal"), + ] + ) + + await app._process_next_from_queue() + await wait_for_modal( + pilot, LaunchGoalCriteriaPreferenceScreen, present=True + ) + # The normal message is still queued: the modal continuation counts + # as busy, so the drain did not start the agent turn behind the + # unanswered prompt. + assert [m.text for m in app._pending_messages] == ["now do the thing"] + assert processed == [] + + await pilot.press("escape") + await wait_for_modal( + pilot, LaunchGoalCriteriaPreferenceScreen, present=False + ) + await drain_modal_commands(app) + # Let the drain-after-modal-command task advance the queue. + for _ in range(20): + if processed: + break + await pilot.pause(0.05) + + assert app._goal_proposal_worker is not None + assert processed == ["now do the thing"] + assert not app._pending_messages async def test_goal_amend_prompt_responsive_through_message_pump( self, monkeypatch: pytest.MonkeyPatch, + drain_modal_commands: DrainModalCommands, + wait_for_modal: WaitForModal, ) -> None: """`/goal amend` shares the create path's off-pump preference prompt.""" from deepagents_code._env_vars import GOAL_AUTO_ACCEPT_CRITERIA @@ -1149,20 +1230,15 @@ async def test_goal_amend_prompt_responsive_through_message_pump( app._chat_input.post_message( ChatInput.Submitted("/goal amend also cover recovery", "command"), ) - for _ in range(60): - await pilot.pause(0.05) - if isinstance(app.screen, LaunchGoalCriteriaPreferenceScreen): - break - assert isinstance(app.screen, LaunchGoalCriteriaPreferenceScreen) + await wait_for_modal( + pilot, LaunchGoalCriteriaPreferenceScreen, present=True + ) await pilot.press("escape") - for _ in range(60): - await pilot.pause(0.05) - if not isinstance(app.screen, LaunchGoalCriteriaPreferenceScreen): - break - assert not isinstance(app.screen, LaunchGoalCriteriaPreferenceScreen) - assert app._goal_preference_task is not None - await asyncio.wait_for(app._goal_preference_task, timeout=2) + await wait_for_modal( + pilot, LaunchGoalCriteriaPreferenceScreen, present=False + ) + await drain_modal_commands(app) assert app._goal_proposal_worker is not None @@ -30647,12 +30723,11 @@ async def _work() -> None: # noqa: RUF029 # scheduled as a coroutine assert "demo" not in app._modal_command_tasks async def test_different_context_is_refused(self) -> None: - """A second mutation continuation is refused while one is active. + """A second continuation is refused while one is active. The command handler returns as soon as the continuation is detached, so - another install or update can arrive while its prompt is still up. The - global guard prevents differently keyed environment mutations from - racing each other. + another command can arrive while its prompt is still up. The global + guard prevents differently keyed continuations from racing each other. """ app = DeepAgentsApp() started = asyncio.Event() @@ -30677,8 +30752,7 @@ async def _second() -> None: # noqa: RUF029 # scheduled as a coroutine # Pin the wording and severity: these tests never call `run_test()`, so # the real `notify` never renders and a broken message would go unnoticed. notify.assert_called_once_with( - "Another install or update is in progress — answer its prompt if " - "one is open.", + "Another command is waiting on a prompt — answer it first.", severity="warning", timeout=5, ) @@ -30820,6 +30894,51 @@ async def _blocked() -> None: assert task.cancelled() + async def test_goal_is_refused_while_another_continuation_is_active( + self, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + """The global guard covers `/goal` across domains, not just installs. + + Admission is global, so a `/goal` that owes the criteria preference + prompt cannot stack its modal over an install or update continuation + that is still in flight. + """ + from deepagents_code._env_vars import GOAL_AUTO_ACCEPT_CRITERIA + + monkeypatch.delenv(GOAL_AUTO_ACCEPT_CRITERIA, raising=False) + app = DeepAgentsApp(agent=MagicMock()) + started = asyncio.Event() + release = asyncio.Event() + + async def _install() -> None: + started.set() + await release.wait() + + async with app.run_test() as pilot: + await pilot.pause() + TestStartupSequence._force_auto_mode(app) + TestStartupSequence._clear_goal_auto_accept_prompt_marker() + install = app._schedule_off_message_pump( + _install(), context="install:package" + ) + assert install is not None + await asyncio.wait_for(started.wait(), timeout=2.0) + + with patch.object(app, "notify") as notify: + await app._handle_goal_command("/goal ship passkeys") + + notify.assert_called_once_with( + "Another command is waiting on a prompt — answer it first.", + severity="warning", + timeout=5, + ) + assert app._goal_proposal_worker is None + assert not isinstance(app.screen, LaunchGoalCriteriaPreferenceScreen) + + release.set() + await asyncio.wait_for(install, timeout=2.0) + def _banner_query_raiser(app: DeepAgentsApp, exc: Exception) -> Callable[..., Widget]: """Return a `query_one` stand-in that raises for the welcome banner only.