From 606ec59b3359c0679384ef569f9e69942fc56153 Mon Sep 17 00:00:00 2001 From: Tim O'Farrell Date: Wed, 2 Jul 2025 10:48:43 -0600 Subject: [PATCH] Fix CLI confirmation input to handle invalid input properly (#9503) Co-authored-by: openhands --- openhands/cli/tui.py | 38 ++++++++++++-------- tests/unit/test_cli_tui.py | 74 +++++++++++++++++++++++++++++++++----- 2 files changed, 90 insertions(+), 22 deletions(-) diff --git a/openhands/cli/tui.py b/openhands/cli/tui.py index 1b84f8d491..17dbeb5017 100644 --- a/openhands/cli/tui.py +++ b/openhands/cli/tui.py @@ -568,22 +568,32 @@ async def read_confirmation_input(config: OpenHandsConfig) -> str: try: prompt_session = create_prompt_session(config) - with patch_stdout(): - print_formatted_text('') - confirmation: str = await prompt_session.prompt_async( - HTML('Proceed with action? (y)es/(n)o/(a)lways > '), - ) + while True: + with patch_stdout(): + print_formatted_text('') + confirmation: str = await prompt_session.prompt_async( + HTML('Proceed with action? (y)es/(n)o/(a)lways > '), + ) - confirmation = '' if confirmation is None else confirmation.strip().lower() + confirmation = ( + '' if confirmation is None else confirmation.strip().lower() + ) - if confirmation in ['y', 'yes']: - return 'yes' - elif confirmation in ['n', 'no']: - return 'no' - elif confirmation in ['a', 'always']: - return 'always' - else: - return 'no' + if confirmation in ['y', 'yes']: + return 'yes' + elif confirmation in ['n', 'no']: + return 'no' + elif confirmation in ['a', 'always']: + return 'always' + else: + # Display error message for invalid input + print_formatted_text('') + print_formatted_text( + HTML( + 'Invalid input. Please enter (y)es, (n)o, or (a)lways.' + ) + ) + # Continue the loop to re-prompt except (KeyboardInterrupt, EOFError): return 'no' diff --git a/tests/unit/test_cli_tui.py b/tests/unit/test_cli_tui.py index 1d6464ecaa..99d3190ece 100644 --- a/tests/unit/test_cli_tui.py +++ b/tests/unit/test_cli_tui.py @@ -335,34 +335,92 @@ class TestReadConfirmationInput: assert result == 'always' @pytest.mark.asyncio + @patch('openhands.cli.tui.print_formatted_text') @patch('openhands.cli.tui.create_prompt_session') - async def test_read_confirmation_input_invalid(self, mock_create_session): + async def test_read_confirmation_input_invalid_then_valid( + self, mock_create_session, mock_print + ): mock_session = AsyncMock() - mock_session.prompt_async.return_value = 'invalid' + # First return invalid input, then valid input + mock_session.prompt_async.side_effect = ['invalid', 'y'] mock_create_session.return_value = mock_session result = await read_confirmation_input(config=MagicMock(spec=OpenHandsConfig)) - assert result == 'no' + assert result == 'yes' + + # Verify error message was displayed + error_calls = [ + call + for call in mock_print.call_args_list + if len(call[0]) > 0 and 'Invalid input' in str(call[0][0]) + ] + assert len(error_calls) > 0 @pytest.mark.asyncio + @patch('openhands.cli.tui.print_formatted_text') @patch('openhands.cli.tui.create_prompt_session') - async def test_read_confirmation_input_empty(self, mock_create_session): + async def test_read_confirmation_input_empty_then_valid( + self, mock_create_session, mock_print + ): mock_session = AsyncMock() - mock_session.prompt_async.return_value = '' + # First return empty input, then valid input + mock_session.prompt_async.side_effect = ['', 'n'] mock_create_session.return_value = mock_session result = await read_confirmation_input(config=MagicMock(spec=OpenHandsConfig)) assert result == 'no' + # Verify error message was displayed + error_calls = [ + call + for call in mock_print.call_args_list + if len(call[0]) > 0 and 'Invalid input' in str(call[0][0]) + ] + assert len(error_calls) > 0 + @pytest.mark.asyncio + @patch('openhands.cli.tui.print_formatted_text') @patch('openhands.cli.tui.create_prompt_session') - async def test_read_confirmation_input_none(self, mock_create_session): + async def test_read_confirmation_input_none_then_valid( + self, mock_create_session, mock_print + ): mock_session = AsyncMock() - mock_session.prompt_async.return_value = None + # First return None, then valid input + mock_session.prompt_async.side_effect = [None, 'always'] mock_create_session.return_value = mock_session result = await read_confirmation_input(config=MagicMock(spec=OpenHandsConfig)) - assert result == 'no' + assert result == 'always' + + # Verify error message was displayed + error_calls = [ + call + for call in mock_print.call_args_list + if len(call[0]) > 0 and 'Invalid input' in str(call[0][0]) + ] + assert len(error_calls) > 0 + + @pytest.mark.asyncio + @patch('openhands.cli.tui.print_formatted_text') + @patch('openhands.cli.tui.create_prompt_session') + async def test_read_confirmation_input_multiple_invalid_then_valid( + self, mock_create_session, mock_print + ): + mock_session = AsyncMock() + # Multiple invalid inputs, then valid input + mock_session.prompt_async.side_effect = ['invalid1', 'invalid2', 'maybe', 'y'] + mock_create_session.return_value = mock_session + + result = await read_confirmation_input(config=MagicMock(spec=OpenHandsConfig)) + assert result == 'yes' + + # Verify error message was displayed multiple times + error_calls = [ + call + for call in mock_print.call_args_list + if len(call[0]) > 0 and 'Invalid input' in str(call[0][0]) + ] + assert len(error_calls) >= 3 # Should have at least 3 error messages @pytest.mark.asyncio @patch('openhands.cli.tui.create_prompt_session')