From 8059c18b5708d8dec285353cd59ff12ccc70352c Mon Sep 17 00:00:00 2001 From: Hiep Le <69354317+hieptl@users.noreply.github.com> Date: Tue, 3 Mar 2026 03:31:29 +0700 Subject: [PATCH] fix(backend): update planning agent to direct users to the build button instead of asking ready to proceed (#13139) --- .../live_status_app_conversation_service.py | 27 +++- ...st_live_status_app_conversation_service.py | 130 ++++++++++++++++++ 2 files changed, 156 insertions(+), 1 deletion(-) diff --git a/openhands/app_server/app_conversation/live_status_app_conversation_service.py b/openhands/app_server/app_conversation/live_status_app_conversation_service.py index 61ec63b15b..83abb472c3 100644 --- a/openhands/app_server/app_conversation/live_status_app_conversation_service.py +++ b/openhands/app_server/app_conversation/live_status_app_conversation_service.py @@ -99,6 +99,20 @@ from openhands.tools.preset.planning import ( _conversation_info_type_adapter = TypeAdapter(list[ConversationInfo | None]) _logger = logging.getLogger(__name__) +# Planning agent instruction to prevent "Ready to proceed?" behavior +PLANNING_AGENT_INSTRUCTION = """ +You are a Planning Agent that can ONLY create plans - you CANNOT execute code or make changes. + +After you finalize the plan in PLAN.md: +- Do NOT ask "Ready to proceed?" or offer to execute the plan +- Do NOT attempt to run any implementation commands +- Instead, inform the user they have two options to proceed: + 1. Click the **Build** button below the plan preview - this will automatically switch to the code agent and instruct it to execute the plan + 2. Switch to the code agent manually (click the agent selector button or press Shift+Tab), then send a message instructing it to execute the plan + +Your role ends when the plan is finalized. Implementation is handled by the code agent. +""" + @dataclass class LiveStatusAppConversationService(AppConversationServiceBase): @@ -953,9 +967,20 @@ class LiveStatusAppConversationService(AppConversationServiceBase): mcp_config=mcp_config, ) + # Prepare system message suffix based on agent type + effective_system_message_suffix = system_message_suffix + if agent_type == AgentType.PLAN: + # Prepend planning-specific instruction to prevent "Ready to proceed?" behavior + if system_message_suffix: + effective_system_message_suffix = ( + f'{PLANNING_AGENT_INSTRUCTION}\n\n{system_message_suffix}' + ) + else: + effective_system_message_suffix = PLANNING_AGENT_INSTRUCTION + # Add agent context agent_context = AgentContext( - system_message_suffix=system_message_suffix, secrets=secrets + system_message_suffix=effective_system_message_suffix, secrets=secrets ) agent = agent.model_copy(update={'agent_context': agent_context}) diff --git a/tests/unit/app_server/test_live_status_app_conversation_service.py b/tests/unit/app_server/test_live_status_app_conversation_service.py index 1157d8ef01..e374d45146 100644 --- a/tests/unit/app_server/test_live_status_app_conversation_service.py +++ b/tests/unit/app_server/test_live_status_app_conversation_service.py @@ -22,6 +22,7 @@ from openhands.app_server.app_conversation.app_conversation_models import ( AppConversationStartRequest, ) from openhands.app_server.app_conversation.live_status_app_conversation_service import ( + PLANNING_AGENT_INSTRUCTION, LiveStatusAppConversationService, ) from openhands.app_server.sandbox.sandbox_models import ( @@ -901,6 +902,135 @@ class TestLiveStatusAppConversationService: mock_llm, AgentType.DEFAULT, self.mock_user.condenser_max_size ) + @patch( + 'openhands.app_server.app_conversation.live_status_app_conversation_service.get_planning_tools' + ) + @patch( + 'openhands.app_server.app_conversation.app_conversation_service_base.AppConversationServiceBase._create_condenser' + ) + @patch( + 'openhands.app_server.app_conversation.live_status_app_conversation_service.format_plan_structure' + ) + def test_create_agent_with_context_planning_agent_applies_instruction( + self, mock_format_plan, mock_create_condenser, mock_get_tools + ): + """Test _create_agent_with_context applies PLANNING_AGENT_INSTRUCTION for plan agents.""" + # Arrange + mock_llm = Mock(spec=LLM) + mock_llm.model_copy.return_value = mock_llm + mock_get_tools.return_value = [] + mock_condenser = Mock() + mock_create_condenser.return_value = mock_condenser + mock_format_plan.return_value = 'test_plan_structure' + mcp_config = {} + + # Act + with patch( + 'openhands.app_server.app_conversation.live_status_app_conversation_service.Agent' + ) as mock_agent_class: + mock_agent_instance = Mock() + mock_agent_instance.model_copy.return_value = mock_agent_instance + mock_agent_class.return_value = mock_agent_instance + + self.service._create_agent_with_context( + mock_llm, + AgentType.PLAN, + None, # No existing suffix + mcp_config, + self.mock_user.condenser_max_size, + ) + + # Assert - verify model_copy was called with agent_context containing planning instruction + model_copy_call = mock_agent_instance.model_copy.call_args + agent_context = model_copy_call[1]['update']['agent_context'] + assert agent_context.system_message_suffix == PLANNING_AGENT_INSTRUCTION + + @patch( + 'openhands.app_server.app_conversation.live_status_app_conversation_service.get_planning_tools' + ) + @patch( + 'openhands.app_server.app_conversation.app_conversation_service_base.AppConversationServiceBase._create_condenser' + ) + @patch( + 'openhands.app_server.app_conversation.live_status_app_conversation_service.format_plan_structure' + ) + def test_create_agent_with_context_planning_agent_prepends_to_existing_suffix( + self, mock_format_plan, mock_create_condenser, mock_get_tools + ): + """Test _create_agent_with_context prepends planning instruction to existing suffix.""" + # Arrange + mock_llm = Mock(spec=LLM) + mock_llm.model_copy.return_value = mock_llm + mock_get_tools.return_value = [] + mock_condenser = Mock() + mock_create_condenser.return_value = mock_condenser + mock_format_plan.return_value = 'test_plan_structure' + mcp_config = {} + existing_suffix = 'Custom user instruction from integration' + + # Act + with patch( + 'openhands.app_server.app_conversation.live_status_app_conversation_service.Agent' + ) as mock_agent_class: + mock_agent_instance = Mock() + mock_agent_instance.model_copy.return_value = mock_agent_instance + mock_agent_class.return_value = mock_agent_instance + + self.service._create_agent_with_context( + mock_llm, + AgentType.PLAN, + existing_suffix, + mcp_config, + self.mock_user.condenser_max_size, + ) + + # Assert - verify planning instruction is prepended to existing suffix + model_copy_call = mock_agent_instance.model_copy.call_args + agent_context = model_copy_call[1]['update']['agent_context'] + assert agent_context.system_message_suffix.startswith( + PLANNING_AGENT_INSTRUCTION + ) + assert existing_suffix in agent_context.system_message_suffix + + @patch( + 'openhands.app_server.app_conversation.live_status_app_conversation_service.get_default_tools' + ) + @patch( + 'openhands.app_server.app_conversation.app_conversation_service_base.AppConversationServiceBase._create_condenser' + ) + def test_create_agent_with_context_default_agent_no_planning_instruction( + self, mock_create_condenser, mock_get_tools + ): + """Test _create_agent_with_context does NOT add planning instruction for default agent.""" + # Arrange + mock_llm = Mock(spec=LLM) + mock_llm.model_copy.return_value = mock_llm + mock_get_tools.return_value = [] + mock_condenser = Mock() + mock_create_condenser.return_value = mock_condenser + mcp_config = {} + + # Act + with patch( + 'openhands.app_server.app_conversation.live_status_app_conversation_service.Agent' + ) as mock_agent_class: + mock_agent_instance = Mock() + mock_agent_instance.model_copy.return_value = mock_agent_instance + mock_agent_class.return_value = mock_agent_instance + + self.service._create_agent_with_context( + mock_llm, + AgentType.DEFAULT, + None, + mcp_config, + self.mock_user.condenser_max_size, + ) + + # Assert - verify no planning instruction for default agent + model_copy_call = mock_agent_instance.model_copy.call_args + agent_context = model_copy_call[1]['update']['agent_context'] + assert agent_context.system_message_suffix is None + @pytest.mark.asyncio @patch( 'openhands.app_server.app_conversation.live_status_app_conversation_service.ExperimentManagerImpl'