From a7245f2de22af604831a4e226ef59a2cd5b94c9a Mon Sep 17 00:00:00 2001 From: Xingyao Wang Date: Mon, 21 Jul 2025 17:45:14 -0400 Subject: [PATCH] fix(CLI): alias persistence issue (#9828) Co-authored-by: openhands --- openhands/cli/main.py | 166 ++++++++++++++--------------- openhands/cli/shell_config.py | 18 ++++ tests/unit/test_cli_alias_setup.py | 124 ++++++++++++++++++++- 3 files changed, 219 insertions(+), 89 deletions(-) diff --git a/openhands/cli/main.py b/openhands/cli/main.py index b5a3e72900..1bba8592f1 100644 --- a/openhands/cli/main.py +++ b/openhands/cli/main.py @@ -17,7 +17,9 @@ from openhands.cli.settings import modify_llm_settings_basic from openhands.cli.shell_config import ( ShellConfigManager, add_aliases_to_shell_config, + alias_setup_declined, aliases_exist_in_shell_config, + mark_alias_setup_declined, ) from openhands.cli.tui import ( UsageMetrics, @@ -387,106 +389,86 @@ def run_alias_setup_flow(config: OpenHandsConfig) -> None: Prompts the user to set up aliases for 'openhands' and 'oh' commands. Handles existing aliases by offering to keep or remove them. + + Args: + config: OpenHands configuration """ print_formatted_text('') print_formatted_text(HTML('🚀 Welcome to OpenHands CLI!')) print_formatted_text('') - # Check if aliases already exist - if aliases_exist_in_shell_config(): - print_formatted_text( - HTML( - 'We detected existing OpenHands aliases in your shell configuration.' - ) + # Show the normal setup flow + print_formatted_text( + HTML('Would you like to set up convenient shell aliases?') + ) + print_formatted_text('') + print_formatted_text( + HTML('This will add the following aliases to your shell profile:') + ) + print_formatted_text( + HTML( + ' • openhands → uvx --python 3.12 --from openhands-ai openhands' ) - print_formatted_text('') - print_formatted_text( - HTML( - ' • openhands → uvx --python 3.12 --from openhands-ai openhands' - ) + ) + print_formatted_text( + HTML( + ' • oh → uvx --python 3.12 --from openhands-ai openhands' ) - print_formatted_text( - HTML( - ' • oh → uvx --python 3.12 --from openhands-ai openhands' - ) + ) + print_formatted_text('') + print_formatted_text( + HTML( + '⚠️ Note: This requires uv to be installed first.' ) - print_formatted_text('') - print_formatted_text( - HTML('✅ Aliases are already configured.') + ) + print_formatted_text( + HTML( + ' Installation guide: https://docs.astral.sh/uv/getting-started/installation' ) - return # Exit early since aliases already exist - else: - # No existing aliases, show the normal setup flow - print_formatted_text( - HTML('Would you like to set up convenient shell aliases?') - ) - print_formatted_text('') - print_formatted_text( - HTML( - 'This will add the following aliases to your shell profile:' - ) - ) - print_formatted_text( - HTML( - ' • openhands → uvx --python 3.12 --from openhands-ai openhands' - ) - ) - print_formatted_text( - HTML( - ' • oh → uvx --python 3.12 --from openhands-ai openhands' - ) - ) - print_formatted_text('') - print_formatted_text( - HTML( - '⚠️ Note: This requires uv to be installed first.' - ) - ) - print_formatted_text( - HTML( - ' Installation guide: https://docs.astral.sh/uv/getting-started/installation' - ) - ) - print_formatted_text('') + ) + print_formatted_text('') - # Use cli_confirm to get user choice - choice = cli_confirm( - config, - 'Set up shell aliases?', - ['Yes, set up aliases', 'No, skip this step'], - ) + # Use cli_confirm to get user choice + choice = cli_confirm( + config, + 'Set up shell aliases?', + ['Yes, set up aliases', 'No, skip this step'], + ) - if choice == 0: # User chose "Yes" - success = add_aliases_to_shell_config() - if success: - print_formatted_text('') - print_formatted_text( - HTML('✅ Aliases added successfully!') + if choice == 0: # User chose "Yes" + success = add_aliases_to_shell_config() + if success: + print_formatted_text('') + print_formatted_text( + HTML('✅ Aliases added successfully!') + ) + + # Get the appropriate reload command using the shell config manager + shell_manager = ShellConfigManager() + reload_cmd = shell_manager.get_reload_command() + + print_formatted_text( + HTML( + f'Run {reload_cmd} (or restart your terminal) to use the new aliases.' ) - - # Get the appropriate reload command using the shell config manager - shell_manager = ShellConfigManager() - reload_cmd = shell_manager.get_reload_command() - - print_formatted_text( - HTML( - f'Run {reload_cmd} (or restart your terminal) to use the new aliases.' - ) - ) - else: - print_formatted_text('') - print_formatted_text( - HTML( - '❌ Failed to add aliases. You can set them up manually later.' - ) - ) - else: # User chose "No" + ) + else: print_formatted_text('') print_formatted_text( HTML( - 'Skipped alias setup. You can run this setup again anytime.' + '❌ Failed to add aliases. You can set them up manually later.' ) ) + else: # User chose "No" + # Mark that the user has declined alias setup + mark_alias_setup_declined() + + print_formatted_text('') + print_formatted_text( + HTML( + 'Skipped alias setup. You can run this setup again anytime.' + ) + ) print_formatted_text('') @@ -583,15 +565,23 @@ async def main_with_loop(loop: asyncio.AbstractEventLoop) -> None: finalize_config(config) # Check if we should show the alias setup flow - # Only show it if aliases don't exist in the shell configuration - # and we're in an interactive environment (not during tests or CI) - if not aliases_exist_in_shell_config() and sys.stdin.isatty(): - # Clear the terminal if we haven't shown a banner yet + # Only show it if: + # 1. Aliases don't exist in the shell configuration + # 2. User hasn't previously declined alias setup + # 3. We're in an interactive environment (not during tests or CI) + should_show_alias_setup = ( + not aliases_exist_in_shell_config() + and not alias_setup_declined() + and sys.stdin.isatty() + ) + + if should_show_alias_setup: + # Clear the terminal if we haven't shown a banner yet (i.e., setup flow didn't run) if not banner_shown: clear() run_alias_setup_flow(config) - banner_shown = True + # Don't set banner_shown = True here, so the ASCII art banner will still be shown # TODO: Set working directory from config or use current working directory? current_dir = config.workspace_base diff --git a/openhands/cli/shell_config.py b/openhands/cli/shell_config.py index 06ae0de021..a27e8e2feb 100644 --- a/openhands/cli/shell_config.py +++ b/openhands/cli/shell_config.py @@ -277,3 +277,21 @@ def get_shell_config_path() -> Path: """Get the path to the shell configuration file.""" manager = ShellConfigManager() return manager.get_shell_config_path() + + +def alias_setup_declined() -> bool: + """Check if the user has previously declined alias setup. + + Returns: + True if user has declined alias setup, False otherwise. + """ + marker_file = Path.home() / '.openhands' / '.cli_alias_setup_declined' + return marker_file.exists() + + +def mark_alias_setup_declined() -> None: + """Mark that the user has declined alias setup.""" + openhands_dir = Path.home() / '.openhands' + openhands_dir.mkdir(exist_ok=True) + marker_file = openhands_dir / '.cli_alias_setup_declined' + marker_file.touch() diff --git a/tests/unit/test_cli_alias_setup.py b/tests/unit/test_cli_alias_setup.py index d2c4eb3bbb..f2feeffe9e 100644 --- a/tests/unit/test_cli_alias_setup.py +++ b/tests/unit/test_cli_alias_setup.py @@ -4,12 +4,16 @@ import tempfile from pathlib import Path from unittest.mock import patch +from openhands.cli.main import alias_setup_declined as main_alias_setup_declined +from openhands.cli.main import aliases_exist_in_shell_config, run_alias_setup_flow from openhands.cli.shell_config import ( ShellConfigManager, add_aliases_to_shell_config, - aliases_exist_in_shell_config, + alias_setup_declined, get_shell_config_path, + mark_alias_setup_declined, ) +from openhands.core.config import OpenHandsConfig def test_get_shell_config_path_no_files_fallback(): @@ -244,3 +248,121 @@ def test_shell_config_manager_template_rendering(): assert 'test-command' in content assert 'alias openhands="test-command"' in content assert 'alias oh="test-command"' in content + + +def test_alias_setup_declined_false(): + """Test alias setup declined check when marker file doesn't exist.""" + with tempfile.TemporaryDirectory() as temp_dir: + with patch('openhands.cli.shell_config.Path.home', return_value=Path(temp_dir)): + assert alias_setup_declined() is False + + +def test_alias_setup_declined_true(): + """Test alias setup declined check when marker file exists.""" + with tempfile.TemporaryDirectory() as temp_dir: + with patch('openhands.cli.shell_config.Path.home', return_value=Path(temp_dir)): + # Create the marker file + mark_alias_setup_declined() + assert alias_setup_declined() is True + + +def test_mark_alias_setup_declined(): + """Test marking alias setup as declined creates the marker file.""" + with tempfile.TemporaryDirectory() as temp_dir: + with patch('openhands.cli.shell_config.Path.home', return_value=Path(temp_dir)): + # Initially should be False + assert alias_setup_declined() is False + + # Mark as declined + mark_alias_setup_declined() + + # Should now be True + assert alias_setup_declined() is True + + # Verify the file exists + marker_file = Path(temp_dir) / '.openhands' / '.cli_alias_setup_declined' + assert marker_file.exists() + + +def test_alias_setup_declined_persisted(): + """Test that when user declines alias setup, their choice is persisted.""" + config = OpenHandsConfig() + + with tempfile.TemporaryDirectory() as temp_dir: + with patch('openhands.cli.shell_config.Path.home', return_value=Path(temp_dir)): + with patch('shellingham.detect_shell', return_value=('bash', 'bash')): + with patch( + 'openhands.cli.shell_config.aliases_exist_in_shell_config', + return_value=False, + ): + with patch( + 'openhands.cli.main.cli_confirm', return_value=1 + ): # User chooses "No" + with patch('prompt_toolkit.print_formatted_text'): + # Initially, user hasn't declined + assert not alias_setup_declined() + + # Run the alias setup flow + run_alias_setup_flow(config) + + # After declining, the marker should be set + assert alias_setup_declined() + + +def test_alias_setup_skipped_when_previously_declined(): + """Test that alias setup is skipped when user has previously declined.""" + OpenHandsConfig() + + with tempfile.TemporaryDirectory() as temp_dir: + with patch('openhands.cli.shell_config.Path.home', return_value=Path(temp_dir)): + # Mark that user has previously declined + mark_alias_setup_declined() + assert alias_setup_declined() + + with patch('shellingham.detect_shell', return_value=('bash', 'bash')): + with patch( + 'openhands.cli.shell_config.aliases_exist_in_shell_config', + return_value=False, + ): + with patch('openhands.cli.main.cli_confirm'): + with patch('prompt_toolkit.print_formatted_text'): + # This should not show the setup flow since user previously declined + # We test this by checking the main logic conditions + + should_show = ( + not aliases_exist_in_shell_config() + and not main_alias_setup_declined() + ) + + assert not should_show, ( + 'Alias setup should be skipped when user previously declined' + ) + + +def test_alias_setup_accepted_does_not_set_declined_flag(): + """Test that when user accepts alias setup, no declined marker is created.""" + config = OpenHandsConfig() + + with tempfile.TemporaryDirectory() as temp_dir: + with patch('openhands.cli.shell_config.Path.home', return_value=Path(temp_dir)): + with patch('shellingham.detect_shell', return_value=('bash', 'bash')): + with patch( + 'openhands.cli.shell_config.aliases_exist_in_shell_config', + return_value=False, + ): + with patch( + 'openhands.cli.main.cli_confirm', return_value=0 + ): # User chooses "Yes" + with patch( + 'openhands.cli.shell_config.add_aliases_to_shell_config', + return_value=True, + ): + with patch('prompt_toolkit.print_formatted_text'): + # Initially, user hasn't declined + assert not alias_setup_declined() + + # Run the alias setup flow + run_alias_setup_flow(config) + + # After accepting, the declined marker should still be False + assert not alias_setup_declined()