mirror of
https://github.com/OpenHands/OpenHands.git
synced 2026-03-22 13:47:19 +08:00
fix(backend): update planning agent to direct users to the build button instead of asking ready to proceed (#13139)
This commit is contained in:
@@ -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 = """<IMPORTANT_PLANNING_BOUNDARIES>
|
||||
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.
|
||||
</IMPORTANT_PLANNING_BOUNDARIES>"""
|
||||
|
||||
|
||||
@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})
|
||||
|
||||
|
||||
@@ -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'
|
||||
|
||||
Reference in New Issue
Block a user