Compare commits

...

8 Commits
v1 ... ctrl-c

Author SHA1 Message Date
Engel Nyst
70ad153c2c Add comprehensive unit tests for Ctrl+C behavior
- test_single_ctrl_c_stops_agent: Verifies first Ctrl+C stops agent gracefully with helpful message
- test_double_ctrl_c_raises_keyboard_interrupt: Verifies second Ctrl+C within 2 seconds raises KeyboardInterrupt for CLI cleanup
- test_ctrl_p_pauses_agent: Verifies Ctrl+P still pauses agent as expected

Tests use proper mocking of prompt_toolkit's create_input, raw_mode, and attach context managers.
All tests pass and validate the improved Ctrl+C behavior implementation.

Co-authored-by: OpenHands-Claude <openhands-claude@all-hands.dev>
2025-06-28 16:17:48 +02:00
Engel Nyst
bded599449 Fix Ctrl+C behavior: use KeyboardInterrupt instead of signals
The previous approach using os.kill(os.getpid(), signal.SIGTERM) was too
aggressive and caused runtime crashes. The proper solution is to raise
KeyboardInterrupt and let the CLI main function handle it gracefully.

Key insights:
- CLI main function already has proper KeyboardInterrupt handling
- shutdown_listener is designed for server mode (uvicorn) primarily
- Raw input mode intercepts Ctrl+C before it becomes SIGINT
- Raising KeyboardInterrupt allows normal CLI shutdown flow

This approach:
- First Ctrl+C: stops agent gracefully with helpful message
- Second Ctrl+C: raises KeyboardInterrupt for clean application exit
- No more runtime crashes or 'system crashed and restarted' errors

Co-authored-by: OpenHands-Claude <openhands@all-hands.dev>
2025-06-28 16:02:22 +02:00
Engel Nyst
0bb43193d0 Use proper signal mechanism for double Ctrl+C shutdown
Instead of directly setting shutdown_listener._should_exit = True,
use os.kill(os.getpid(), signal.SIGTERM) to trigger the proper
shutdown signal handler.

This follows the established pattern where:
- shutdown_listener registers signal handlers for SIGINT/SIGTERM
- Signal handler sets _should_exit = True and calls shutdown listeners
- Components check should_continue()/should_exit() for coordinated shutdown

Benefits:
- Follows OpenHands' established shutdown architecture
- Proper signal handling instead of direct flag manipulation
- Consistent with how other shutdown scenarios work
- Cleaner separation of concerns

Co-authored-by: OpenHands-Claude <openhands-claude@all-hands.dev>
2025-06-28 15:41:02 +02:00
Engel Nyst
72b1aa6154 Fix double Ctrl+C to use global shutdown mechanism
Instead of raising KeyboardInterrupt which interrupts pending actions
and causes 'runtime system crashed' errors, use the existing global
shutdown_listener mechanism that gracefully shuts down all components.

This prevents:
- [Errno 21] Is a directory errors
- 'runtime system crashed and restarted' messages
- Delayed action execution after restart
- Missing 'Force quitting...' message

The shutdown_listener._should_exit flag is checked by EventStream
and other components for clean shutdown coordination.
2025-06-28 15:24:03 +02:00
Engel Nyst
db5f7a5744 Implement double Ctrl+C behavior for graceful vs force quit
Changed to a more user-friendly approach:
- First Ctrl+C: Stops agent gracefully (sets STOPPED state)
- Second Ctrl+C within 2 seconds: Force quits application

This provides better UX by allowing users to:
1. Stop the current agent task without quitting the CLI
2. Force quit if they really want to exit the application

Messages shown:
- First: 'Stopping agent... (press Ctrl+C again within 2 seconds to force quit)'
- Second: 'Force quitting...'

Co-authored-by: OpenHands-Claude <openhands@all-hands.dev>
2025-06-28 14:46:54 +02:00
Engel Nyst
fbe253f9e9 Fix Ctrl+C to cleanly stop agent instead of hard interrupt
Changed approach from raising KeyboardInterrupt to setting agent state
to STOPPED, which allows for clean shutdown without race conditions.

Changes:
- Ctrl+C now sets AgentState.STOPPED and signals done event
- Shows 'Keyboard interrupt, shutting down...' message
- Avoids 'cannot schedule new futures after interpreter shutdown' error
- Ctrl+P and Ctrl+D continue to pause the agent as before

This approach prevents the race condition where background tasks try to
schedule futures after the interpreter begins shutdown.

Co-authored-by: OpenHands-Claude <openhands@all-hands.dev>
2025-06-28 14:30:01 +02:00
Engel Nyst
f70f07e19a Fix Ctrl+C to raise KeyboardInterrupt instead of ignoring it
The previous fix removed Ctrl+C handling entirely, which caused it to be
consumed by the input handler but do nothing. This fix makes Ctrl+C
explicitly raise KeyboardInterrupt, which will properly terminate the
application.

Changes:
- Ctrl+C now raises KeyboardInterrupt in process_agent_pause
- Ctrl+P and Ctrl+D continue to pause the agent as before
- Application will properly terminate when Ctrl+C is pressed

Co-authored-by: OpenHands-Claude <openhands@all-hands.dev>
2025-06-28 14:21:48 +02:00
Engel Nyst
c9a2dec194 Fix Ctrl+C behavior in CLI to properly interrupt and stop application
Previously, Ctrl+C was intercepted by the process_agent_pause function
and treated the same as Ctrl+P (pause agent). This prevented normal
KeyboardInterrupt handling and made it impossible to stop the application
with Ctrl+C.

Changes:
- Remove Keys.ControlC from process_agent_pause function
- Now only Ctrl+P and Ctrl+D pause the agent
- Ctrl+C properly propagates as KeyboardInterrupt to main function
- Application can now be terminated normally with Ctrl+C

Co-authored-by: OpenHands-Claude <openhands@all-hands.dev>
2025-06-28 14:08:01 +02:00
2 changed files with 226 additions and 5 deletions

View File

@@ -586,15 +586,38 @@ async def read_confirmation_input(config: OpenHandsConfig) -> str:
async def process_agent_pause(done: asyncio.Event, event_stream: EventStream) -> None:
import time
input = create_input()
ctrl_c_pressed_time = None
def keys_ready() -> None:
nonlocal ctrl_c_pressed_time
for key_press in input.read_keys():
if (
key_press.key == Keys.ControlP
or key_press.key == Keys.ControlC
or key_press.key == Keys.ControlD
):
if key_press.key == Keys.ControlC:
current_time = time.time()
if ctrl_c_pressed_time and (current_time - ctrl_c_pressed_time) < 2.0:
# Double Ctrl+C within 2 seconds - force quit
print_formatted_text('')
print_formatted_text(HTML('<red>Force quitting...</red>'))
# Let the CLI main function handle the KeyboardInterrupt properly
raise KeyboardInterrupt()
else:
# First Ctrl+C - stop agent gracefully
ctrl_c_pressed_time = current_time
print_formatted_text('')
print_formatted_text(
HTML(
'<yellow>Stopping agent... (press Ctrl+C again within 2 seconds to force quit)</yellow>'
)
)
event_stream.add_event(
ChangeAgentStateAction(AgentState.STOPPED),
EventSource.USER,
)
done.set()
elif key_press.key == Keys.ControlP or key_press.key == Keys.ControlD:
print_formatted_text('')
print_formatted_text(HTML('<gold>Pausing the agent...</gold>'))
event_stream.add_event(

View File

@@ -16,6 +16,7 @@ from openhands.cli.tui import (
display_usage_metrics,
display_welcome_message,
get_session_duration,
process_agent_pause,
read_confirmation_input,
)
from openhands.core.config import OpenHandsConfig
@@ -385,3 +386,200 @@ class TestReadConfirmationInput:
result = await read_confirmation_input(config=MagicMock(spec=OpenHandsConfig))
assert result == 'no'
class TestProcessAgentPause:
@pytest.mark.asyncio
@patch('openhands.cli.tui.create_input')
@patch('openhands.cli.tui.print_formatted_text')
async def test_single_ctrl_c_stops_agent(self, mock_print, mock_create_input):
"""Test that a single Ctrl+C stops the agent gracefully."""
import asyncio
from prompt_toolkit.keys import Keys
# Mock the input to simulate a single Ctrl+C
mock_input = Mock()
mock_key_press = Mock()
mock_key_press.key = Keys.ControlC
mock_input.read_keys.return_value = [mock_key_press]
# Mock the context managers and simulate immediate key press
mock_input.raw_mode.return_value.__enter__ = Mock(return_value=None)
mock_input.raw_mode.return_value.__exit__ = Mock(return_value=None)
# Mock attach to immediately call the keys_ready function
def mock_attach(keys_ready_func):
# Simulate the key press by calling the function immediately
keys_ready_func()
# Return a mock context manager
mock_context = Mock()
mock_context.__enter__ = Mock(return_value=None)
mock_context.__exit__ = Mock(return_value=None)
return mock_context
mock_input.attach.side_effect = mock_attach
mock_create_input.return_value = mock_input
# Mock event stream
mock_event_stream = Mock()
mock_event_stream.add_event = Mock()
# Create done event
done = asyncio.Event()
# Run the function
await process_agent_pause(done, mock_event_stream)
# Verify agent was stopped gracefully
mock_event_stream.add_event.assert_called_once()
call_args = mock_event_stream.add_event.call_args[0]
assert call_args[0].agent_state.value == 'stopped'
# Verify the helpful message was displayed
mock_print.assert_called()
print_calls = [call.args[0] for call in mock_print.call_args_list]
helpful_message_found = any(
'Stopping agent' in str(call) and 'press Ctrl+C again' in str(call)
for call in print_calls
)
assert helpful_message_found, (
f'Expected helpful message not found in: {print_calls}'
)
# Verify done event was set
assert done.is_set()
@pytest.mark.asyncio
@patch('openhands.cli.tui.create_input')
@patch('openhands.cli.tui.print_formatted_text')
async def test_double_ctrl_c_raises_keyboard_interrupt(
self, mock_print, mock_create_input
):
"""Test that double Ctrl+C within 2 seconds raises KeyboardInterrupt."""
import asyncio
from prompt_toolkit.keys import Keys
# Mock the input to simulate double Ctrl+C
mock_input = Mock()
mock_key_press = Mock()
mock_key_press.key = Keys.ControlC
# Simulate two Ctrl+C presses within 2 seconds
call_count = 0
def mock_read_keys():
nonlocal call_count
call_count += 1
if call_count == 1:
# First call returns first Ctrl+C
return [mock_key_press]
elif call_count == 2:
# Second call returns second Ctrl+C (within 2 seconds)
return [mock_key_press]
else:
# Subsequent calls return empty to avoid infinite loop
return []
mock_input.read_keys.side_effect = mock_read_keys
# Mock the context managers and simulate double key press
mock_input.raw_mode.return_value.__enter__ = Mock(return_value=None)
mock_input.raw_mode.return_value.__exit__ = Mock(return_value=None)
# Mock attach to call the keys_ready function twice (simulating double Ctrl+C)
def mock_attach(keys_ready_func):
# Simulate first Ctrl+C
keys_ready_func()
# Simulate second Ctrl+C immediately (within 2 seconds)
keys_ready_func()
# Return a mock context manager
mock_context = Mock()
mock_context.__enter__ = Mock(return_value=None)
mock_context.__exit__ = Mock(return_value=None)
return mock_context
mock_input.attach.side_effect = mock_attach
mock_create_input.return_value = mock_input
# Mock event stream
mock_event_stream = Mock()
mock_event_stream.add_event = Mock()
# Create done event
done = asyncio.Event()
# Run the function and expect KeyboardInterrupt
with pytest.raises(KeyboardInterrupt):
await process_agent_pause(done, mock_event_stream)
# Verify force quit message was displayed
mock_print.assert_called()
print_calls = [call.args[0] for call in mock_print.call_args_list]
force_quit_message_found = any(
'Force quitting' in str(call) for call in print_calls
)
assert force_quit_message_found, (
f'Expected force quit message not found in: {print_calls}'
)
@pytest.mark.asyncio
@patch('openhands.cli.tui.create_input')
@patch('openhands.cli.tui.print_formatted_text')
async def test_ctrl_p_pauses_agent(self, mock_print, mock_create_input):
"""Test that Ctrl+P pauses the agent."""
import asyncio
from prompt_toolkit.keys import Keys
# Mock the input to simulate Ctrl+P
mock_input = Mock()
mock_key_press = Mock()
mock_key_press.key = Keys.ControlP
mock_input.read_keys.return_value = [mock_key_press]
# Mock the context managers and simulate immediate key press
mock_input.raw_mode.return_value.__enter__ = Mock(return_value=None)
mock_input.raw_mode.return_value.__exit__ = Mock(return_value=None)
# Mock attach to immediately call the keys_ready function
def mock_attach(keys_ready_func):
# Simulate the key press by calling the function immediately
keys_ready_func()
# Return a mock context manager
mock_context = Mock()
mock_context.__enter__ = Mock(return_value=None)
mock_context.__exit__ = Mock(return_value=None)
return mock_context
mock_input.attach.side_effect = mock_attach
mock_create_input.return_value = mock_input
# Mock event stream
mock_event_stream = Mock()
mock_event_stream.add_event = Mock()
# Create done event
done = asyncio.Event()
# Run the function
await process_agent_pause(done, mock_event_stream)
# Verify agent was paused
mock_event_stream.add_event.assert_called_once()
call_args = mock_event_stream.add_event.call_args[0]
assert call_args[0].agent_state.value == 'paused'
# Verify the pause message was displayed
mock_print.assert_called()
print_calls = [call.args[0] for call in mock_print.call_args_list]
pause_message_found = any(
'Pausing the agent' in str(call) for call in print_calls
)
assert pause_message_found, (
f'Expected pause message not found in: {print_calls}'
)
# Verify done event was set
assert done.is_set()