From 7de32b25794fda743fa2a19e622155fe78f4c387 Mon Sep 17 00:00:00 2001 From: Rohit Malhotra Date: Thu, 9 Oct 2025 11:12:37 -0400 Subject: [PATCH] CLI(V1): expose advanced settings setup for first time users (#11288) Co-authored-by: openhands --- openhands-cli/openhands_cli/agent_chat.py | 29 ++++++---- .../tui/settings/settings_screen.py | 26 ++++----- .../user_actions/settings_action.py | 16 ++++-- .../settings/test_first_time_user_settings.py | 54 +++++++++++++++++++ .../{ => settings}/test_settings_input.py | 0 .../{ => settings}/test_settings_workflow.py | 0 openhands-cli/tests/test_new_command.py | 4 +- 7 files changed, 98 insertions(+), 31 deletions(-) create mode 100644 openhands-cli/tests/settings/test_first_time_user_settings.py rename openhands-cli/tests/{ => settings}/test_settings_input.py (100%) rename openhands-cli/tests/{ => settings}/test_settings_workflow.py (100%) diff --git a/openhands-cli/openhands_cli/agent_chat.py b/openhands-cli/openhands_cli/agent_chat.py index 4208169709..1782463502 100644 --- a/openhands-cli/openhands_cli/agent_chat.py +++ b/openhands-cli/openhands_cli/agent_chat.py @@ -29,11 +29,9 @@ from openhands_cli.user_actions import UserConfirmation, exit_session_confirmati from openhands_cli.user_actions.utils import get_session_prompter - - def _start_fresh_conversation(resume_conversation_id: str | None = None) -> BaseConversation: """Start a fresh conversation by creating a new conversation instance. - + Handles the complete conversation setup process including settings screen if agent configuration is missing. @@ -45,14 +43,16 @@ def _start_fresh_conversation(resume_conversation_id: str | None = None) -> Base """ conversation = None settings_screen = SettingsScreen() + try: + conversation = setup_conversation(resume_conversation_id) + return conversation + except MissingAgentSpec: + # For first-time users, show the full settings flow with choice between basic/advanced + settings_screen.configure_settings(first_time=True) - while not conversation: - try: - conversation = setup_conversation(resume_conversation_id) - except MissingAgentSpec: - settings_screen.handle_basic_settings(escapable=False) - - return conversation + + # Try once again after settings setup attempt + return setup_conversation(resume_conversation_id) def _restore_tty() -> None: @@ -91,7 +91,14 @@ def run_cli_entry(resume_conversation_id: str | None = None) -> None: EOFError: If EOF is encountered """ - conversation = _start_fresh_conversation(resume_conversation_id) + try: + conversation = _start_fresh_conversation(resume_conversation_id) + except MissingAgentSpec: + print_formatted_text(HTML('\nSetup is required to use OpenHands CLI.')) + print_formatted_text(HTML('\nGoodbye! 👋')) + return + + display_welcome(conversation.id, bool(resume_conversation_id)) # Track session start time for uptime calculation diff --git a/openhands-cli/openhands_cli/tui/settings/settings_screen.py b/openhands-cli/openhands_cli/tui/settings/settings_screen.py index edfdecba93..4aa36cd0e2 100644 --- a/openhands-cli/openhands_cli/tui/settings/settings_screen.py +++ b/openhands-cli/openhands_cli/tui/settings/settings_screen.py @@ -1,5 +1,12 @@ import os +from openhands.sdk import LLM, BaseConversation, LocalFileStore +from openhands.sdk.security.confirmation_policy import NeverConfirm +from openhands.tools.preset.default import get_default_agent +from prompt_toolkit import HTML, print_formatted_text +from prompt_toolkit.shortcuts import print_container +from prompt_toolkit.widgets import Frame, TextArea + from openhands_cli.llm_utils import get_llm_metadata from openhands_cli.locations import AGENT_SETTINGS_PATH, PERSISTENCE_DIR from openhands_cli.pt_style import COLOR_GREY @@ -16,13 +23,6 @@ from openhands_cli.user_actions.settings_action import ( save_settings_confirmation, settings_type_confirmation, ) -from prompt_toolkit import HTML, print_formatted_text -from prompt_toolkit.shortcuts import print_container -from prompt_toolkit.widgets import Frame, TextArea - -from openhands.sdk import LLM, BaseConversation, LocalFileStore -from openhands.sdk.security.confirmation_policy import NeverConfirm -from openhands.tools.preset.default import get_default_agent class SettingsScreen: @@ -116,9 +116,9 @@ class SettingsScreen: self.configure_settings() - def configure_settings(self): + def configure_settings(self, first_time=False): try: - settings_type = settings_type_confirmation() + settings_type = settings_type_confirmation(first_time=first_time) except KeyboardInterrupt: return @@ -127,18 +127,18 @@ class SettingsScreen: elif settings_type == SettingsType.ADVANCED: self.handle_advanced_settings() - def handle_basic_settings(self, escapable=True): + def handle_basic_settings(self): step_counter = StepCounter(3) try: - provider = choose_llm_provider(step_counter, escapable=escapable) - llm_model = choose_llm_model(step_counter, provider, escapable=escapable) + provider = choose_llm_provider(step_counter, escapable=True) + llm_model = choose_llm_model(step_counter, provider, escapable=True) api_key = prompt_api_key( step_counter, provider, self.conversation.state.agent.llm.api_key if self.conversation else None, - escapable=escapable, + escapable=True, ) save_settings_confirmation() except KeyboardInterrupt: diff --git a/openhands-cli/openhands_cli/user_actions/settings_action.py b/openhands-cli/openhands_cli/user_actions/settings_action.py index 58632e3956..b2a0d85a37 100644 --- a/openhands-cli/openhands_cli/user_actions/settings_action.py +++ b/openhands-cli/openhands_cli/user_actions/settings_action.py @@ -1,9 +1,9 @@ from enum import Enum +from openhands.sdk.llm import UNVERIFIED_MODELS_EXCLUDING_BEDROCK, VERIFIED_MODELS from prompt_toolkit.completion import FuzzyWordCompleter from pydantic import SecretStr -from openhands.sdk.llm import UNVERIFIED_MODELS_EXCLUDING_BEDROCK, VERIFIED_MODELS from openhands_cli.tui.utils import StepCounter from openhands_cli.user_actions.utils import ( NonEmptyValueValidator, @@ -17,13 +17,19 @@ class SettingsType(Enum): ADVANCED = 'advanced' -def settings_type_confirmation() -> SettingsType: - question = 'Which settings would you like to modify?' +def settings_type_confirmation(first_time: bool = False) -> SettingsType: + question = ( + '\nWelcome to OpenHands! Let\'s configure your LLM settings.\n' + 'Choose your preferred setup method:' + ) choices = [ 'LLM (Basic)', - 'LLM (Advanced)', - 'Go back', + 'LLM (Advanced)' ] + if not first_time: + question = 'Which settings would you like to modify?' + choices.append('Go back') + index = cli_confirm(question, choices, escapable=True) diff --git a/openhands-cli/tests/settings/test_first_time_user_settings.py b/openhands-cli/tests/settings/test_first_time_user_settings.py new file mode 100644 index 0000000000..64d4a59a2f --- /dev/null +++ b/openhands-cli/tests/settings/test_first_time_user_settings.py @@ -0,0 +1,54 @@ +from unittest.mock import patch +from openhands_cli.agent_chat import run_cli_entry +import pytest + + +@patch("openhands_cli.agent_chat.print_formatted_text") +@patch("openhands_cli.tui.settings.settings_screen.save_settings_confirmation") +@patch("openhands_cli.tui.settings.settings_screen.prompt_api_key") +@patch("openhands_cli.tui.settings.settings_screen.choose_llm_model") +@patch("openhands_cli.tui.settings.settings_screen.choose_llm_provider") +@patch("openhands_cli.tui.settings.settings_screen.settings_type_confirmation") +@patch("openhands_cli.tui.settings.store.AgentStore.load") +@pytest.mark.parametrize("interrupt_step", ["settings_type", "provider", "model", "api_key", "save"]) +def test_first_time_users_can_escape_settings_flow_and_exit_app( + mock_agentstore_load, + mock_type, + mock_provider, + mock_model, + mock_api_key, + mock_save, + mock_print, + interrupt_step, +): + """Test that KeyboardInterrupt is handled at each step of basic settings.""" + + # Force first-time user: no saved agent + mock_agentstore_load.return_value = None + + # Happy path defaults + mock_type.return_value = "basic" + mock_provider.return_value = "openai" + mock_model.return_value = "gpt-4o-mini" + mock_api_key.return_value = "sk-test" + mock_save.return_value = True + + # Inject KeyboardInterrupt at the specified step + if interrupt_step == "settings_type": + mock_type.side_effect = KeyboardInterrupt() + elif interrupt_step == "provider": + mock_provider.side_effect = KeyboardInterrupt() + elif interrupt_step == "model": + mock_model.side_effect = KeyboardInterrupt() + elif interrupt_step == "api_key": + mock_api_key.side_effect = KeyboardInterrupt() + elif interrupt_step == "save": + mock_save.side_effect = KeyboardInterrupt() + + # Run + run_cli_entry() + + # Assert graceful messaging + calls = [call.args[0] for call in mock_print.call_args_list] + assert any("Setup is required" in str(c) for c in calls) + assert any("Goodbye!" in str(c) for c in calls) diff --git a/openhands-cli/tests/test_settings_input.py b/openhands-cli/tests/settings/test_settings_input.py similarity index 100% rename from openhands-cli/tests/test_settings_input.py rename to openhands-cli/tests/settings/test_settings_input.py diff --git a/openhands-cli/tests/test_settings_workflow.py b/openhands-cli/tests/settings/test_settings_workflow.py similarity index 100% rename from openhands-cli/tests/test_settings_workflow.py rename to openhands-cli/tests/settings/test_settings_workflow.py diff --git a/openhands-cli/tests/test_new_command.py b/openhands-cli/tests/test_new_command.py index 82d20dea29..33682c416b 100644 --- a/openhands-cli/tests/test_new_command.py +++ b/openhands-cli/tests/test_new_command.py @@ -51,8 +51,8 @@ def test_start_fresh_conversation_missing_agent_spec( assert result == mock_conversation # Should be called twice: first fails, second succeeds assert mock_setup_conversation.call_count == 2 - # Settings screen should be called once - mock_settings_screen.handle_basic_settings.assert_called_once_with(escapable=False) + # Settings screen should be called once with first_time=True (new behavior) + mock_settings_screen.configure_settings.assert_called_once_with(first_time=True)