CLI(V1): expose advanced settings setup for first time users (#11288)

Co-authored-by: openhands <openhands@all-hands.dev>
This commit is contained in:
Rohit Malhotra
2025-10-09 11:12:37 -04:00
committed by GitHub
parent 36b174bfb4
commit 7de32b2579
7 changed files with 98 additions and 31 deletions

View File

@@ -29,11 +29,9 @@ from openhands_cli.user_actions import UserConfirmation, exit_session_confirmati
from openhands_cli.user_actions.utils import get_session_prompter from openhands_cli.user_actions.utils import get_session_prompter
def _start_fresh_conversation(resume_conversation_id: str | None = None) -> BaseConversation: def _start_fresh_conversation(resume_conversation_id: str | None = None) -> BaseConversation:
"""Start a fresh conversation by creating a new conversation instance. """Start a fresh conversation by creating a new conversation instance.
Handles the complete conversation setup process including settings screen Handles the complete conversation setup process including settings screen
if agent configuration is missing. if agent configuration is missing.
@@ -45,14 +43,16 @@ def _start_fresh_conversation(resume_conversation_id: str | None = None) -> Base
""" """
conversation = None conversation = None
settings_screen = SettingsScreen() 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: # Try once again after settings setup attempt
conversation = setup_conversation(resume_conversation_id) return setup_conversation(resume_conversation_id)
except MissingAgentSpec:
settings_screen.handle_basic_settings(escapable=False)
return conversation
def _restore_tty() -> None: def _restore_tty() -> None:
@@ -91,7 +91,14 @@ def run_cli_entry(resume_conversation_id: str | None = None) -> None:
EOFError: If EOF is encountered 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('\n<yellow>Setup is required to use OpenHands CLI.</yellow>'))
print_formatted_text(HTML('\n<yellow>Goodbye! 👋</yellow>'))
return
display_welcome(conversation.id, bool(resume_conversation_id)) display_welcome(conversation.id, bool(resume_conversation_id))
# Track session start time for uptime calculation # Track session start time for uptime calculation

View File

@@ -1,5 +1,12 @@
import os 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.llm_utils import get_llm_metadata
from openhands_cli.locations import AGENT_SETTINGS_PATH, PERSISTENCE_DIR from openhands_cli.locations import AGENT_SETTINGS_PATH, PERSISTENCE_DIR
from openhands_cli.pt_style import COLOR_GREY from openhands_cli.pt_style import COLOR_GREY
@@ -16,13 +23,6 @@ from openhands_cli.user_actions.settings_action import (
save_settings_confirmation, save_settings_confirmation,
settings_type_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: class SettingsScreen:
@@ -116,9 +116,9 @@ class SettingsScreen:
self.configure_settings() self.configure_settings()
def configure_settings(self): def configure_settings(self, first_time=False):
try: try:
settings_type = settings_type_confirmation() settings_type = settings_type_confirmation(first_time=first_time)
except KeyboardInterrupt: except KeyboardInterrupt:
return return
@@ -127,18 +127,18 @@ class SettingsScreen:
elif settings_type == SettingsType.ADVANCED: elif settings_type == SettingsType.ADVANCED:
self.handle_advanced_settings() self.handle_advanced_settings()
def handle_basic_settings(self, escapable=True): def handle_basic_settings(self):
step_counter = StepCounter(3) step_counter = StepCounter(3)
try: try:
provider = choose_llm_provider(step_counter, escapable=escapable) provider = choose_llm_provider(step_counter, escapable=True)
llm_model = choose_llm_model(step_counter, provider, escapable=escapable) llm_model = choose_llm_model(step_counter, provider, escapable=True)
api_key = prompt_api_key( api_key = prompt_api_key(
step_counter, step_counter,
provider, provider,
self.conversation.state.agent.llm.api_key self.conversation.state.agent.llm.api_key
if self.conversation if self.conversation
else None, else None,
escapable=escapable, escapable=True,
) )
save_settings_confirmation() save_settings_confirmation()
except KeyboardInterrupt: except KeyboardInterrupt:

View File

@@ -1,9 +1,9 @@
from enum import Enum from enum import Enum
from openhands.sdk.llm import UNVERIFIED_MODELS_EXCLUDING_BEDROCK, VERIFIED_MODELS
from prompt_toolkit.completion import FuzzyWordCompleter from prompt_toolkit.completion import FuzzyWordCompleter
from pydantic import SecretStr 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.tui.utils import StepCounter
from openhands_cli.user_actions.utils import ( from openhands_cli.user_actions.utils import (
NonEmptyValueValidator, NonEmptyValueValidator,
@@ -17,13 +17,19 @@ class SettingsType(Enum):
ADVANCED = 'advanced' ADVANCED = 'advanced'
def settings_type_confirmation() -> SettingsType: def settings_type_confirmation(first_time: bool = False) -> SettingsType:
question = 'Which settings would you like to modify?' question = (
'\nWelcome to OpenHands! Let\'s configure your LLM settings.\n'
'Choose your preferred setup method:'
)
choices = [ choices = [
'LLM (Basic)', 'LLM (Basic)',
'LLM (Advanced)', 'LLM (Advanced)'
'Go back',
] ]
if not first_time:
question = 'Which settings would you like to modify?'
choices.append('Go back')
index = cli_confirm(question, choices, escapable=True) index = cli_confirm(question, choices, escapable=True)

View File

@@ -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)

View File

@@ -51,8 +51,8 @@ def test_start_fresh_conversation_missing_agent_spec(
assert result == mock_conversation assert result == mock_conversation
# Should be called twice: first fails, second succeeds # Should be called twice: first fails, second succeeds
assert mock_setup_conversation.call_count == 2 assert mock_setup_conversation.call_count == 2
# Settings screen should be called once # Settings screen should be called once with first_time=True (new behavior)
mock_settings_screen.handle_basic_settings.assert_called_once_with(escapable=False) mock_settings_screen.configure_settings.assert_called_once_with(first_time=True)