From 411b63159f2d12726b70537c542e1ffa0d8f4fb9 Mon Sep 17 00:00:00 2001 From: Graham Neubig Date: Sun, 5 Jan 2025 08:13:18 +0900 Subject: [PATCH 01/21] fix: Use _send_action_server_request in send_action_for_execution (#5951) Co-authored-by: openhands --- .../action_execution_client.py | 3 +- .../runtime/impl/remote/remote_runtime.py | 29 ++++--- tests/unit/test_runtime_reboot.py | 87 +++++++++++++++++++ 3 files changed, 106 insertions(+), 13 deletions(-) create mode 100644 tests/unit/test_runtime_reboot.py diff --git a/openhands/runtime/impl/action_execution/action_execution_client.py b/openhands/runtime/impl/action_execution/action_execution_client.py index 80f71ffe8ab1..4c9d5d52fae2 100644 --- a/openhands/runtime/impl/action_execution/action_execution_client.py +++ b/openhands/runtime/impl/action_execution/action_execution_client.py @@ -249,8 +249,7 @@ def send_action_for_execution(self, action: Action) -> Observation: assert action.timeout is not None try: - with send_request( - self.session, + with self._send_action_server_request( 'POST', f'{self._get_action_execution_server_host()}/execute_action', json={'action': event_to_dict(action)}, diff --git a/openhands/runtime/impl/remote/remote_runtime.py b/openhands/runtime/impl/remote/remote_runtime.py index 010ec8b0e938..a8c23bbde5fa 100644 --- a/openhands/runtime/impl/remote/remote_runtime.py +++ b/openhands/runtime/impl/remote/remote_runtime.py @@ -329,9 +329,14 @@ def _wait_until_alive_impl(self): elif pod_status in ('failed', 'unknown', 'crashloopbackoff'): # clean up the runtime self.close() - raise AgentRuntimeUnavailableError( - f'Runtime (ID={self.runtime_id}) failed to start. Current status: {pod_status}. Pod Logs:\n{runtime_data.get("pod_logs", "N/A")}' - ) + if pod_status == 'crashloopbackoff': + raise AgentRuntimeUnavailableError( + 'Runtime crashed and is being restarted, potentially due to memory usage. Please try again.' + ) + else: + raise AgentRuntimeUnavailableError( + f'Runtime is unavailable (status: {pod_status}). Please try again.' + ) else: # Maybe this should be a hard failure, but passing through in case the API changes self.log('warning', f'Unknown pod status: {pod_status}') @@ -371,15 +376,17 @@ def _send_action_server_request(self, method, url, **kwargs): f'No response received within the timeout period for url: {url}', ) raise + except requests.HTTPError as e: - if e.response.status_code == 404: - raise AgentRuntimeNotFoundError( - 'Runtime unavailable: System resources may be exhausted due to running commands. This may be fixed by retrying.' - ) from e - elif e.response.status_code == 502: - raise AgentRuntimeDisconnectedError( - 'Runtime disconnected: System resources may be exhausted due to running commands. This may be fixed by retrying.' - ) from e + if e.response.status_code in (404, 502): + if e.response.status_code == 404: + raise AgentRuntimeDisconnectedError( + 'Runtime is not responding. This may be temporary, please try again.' + ) from e + else: # 502 + raise AgentRuntimeDisconnectedError( + 'Runtime is temporarily unavailable. This may be due to a restart or network issue, please try again.' + ) from e elif e.response.status_code == 503: self.log('warning', 'Runtime appears to be paused. Resuming...') self._resume_runtime() diff --git a/tests/unit/test_runtime_reboot.py b/tests/unit/test_runtime_reboot.py new file mode 100644 index 000000000000..e3ae31815a3e --- /dev/null +++ b/tests/unit/test_runtime_reboot.py @@ -0,0 +1,87 @@ +from unittest.mock import Mock + +import pytest +import requests + +from openhands.core.exceptions import ( + AgentRuntimeDisconnectedError, + AgentRuntimeTimeoutError, +) +from openhands.events.action import CmdRunAction +from openhands.runtime.base import Runtime + + +@pytest.fixture +def mock_session(): + return Mock() + + +@pytest.fixture +def runtime(mock_session): + runtime = Mock(spec=Runtime) + runtime.session = mock_session + runtime.send_action_for_execution = Mock() + return runtime + + +def test_runtime_timeout_error(runtime, mock_session): + # Create a command action + action = CmdRunAction(command='test command') + action.timeout = 120 + + # Mock the runtime to raise a timeout error + runtime.send_action_for_execution.side_effect = AgentRuntimeTimeoutError( + 'Runtime failed to return execute_action before the requested timeout of 120s' + ) + + # Verify that the error message indicates a timeout + with pytest.raises(AgentRuntimeTimeoutError) as exc_info: + runtime.send_action_for_execution(action) + + assert ( + str(exc_info.value) + == 'Runtime failed to return execute_action before the requested timeout of 120s' + ) + + +@pytest.mark.parametrize( + 'status_code,expected_message', + [ + (404, 'Runtime is not responding. This may be temporary, please try again.'), + ( + 502, + 'Runtime is temporarily unavailable. This may be due to a restart or network issue, please try again.', + ), + ], +) +def test_runtime_disconnected_error( + runtime, mock_session, status_code, expected_message +): + # Mock the request to return the specified status code + mock_response = Mock() + mock_response.status_code = status_code + mock_response.raise_for_status = Mock( + side_effect=requests.HTTPError(response=mock_response) + ) + mock_response.json = Mock( + return_value={ + 'observation': 'run', + 'content': 'test', + 'extras': {'command_id': 'test_id', 'command': 'test command'}, + } + ) + + # Mock the runtime to raise the error + runtime.send_action_for_execution.side_effect = AgentRuntimeDisconnectedError( + expected_message + ) + + # Create a command action + action = CmdRunAction(command='test command') + action.timeout = 120 + + # Verify that the error message is correct + with pytest.raises(AgentRuntimeDisconnectedError) as exc_info: + runtime.send_action_for_execution(action) + + assert str(exc_info.value) == expected_message From 56d7dccec96d74f5f97abdcb67b93e428caa5a3e Mon Sep 17 00:00:00 2001 From: Xingyao Wang Date: Sat, 4 Jan 2025 18:38:34 -0500 Subject: [PATCH 02/21] fix(runtime): replace send_request with _send_action_server_request (#6035) --- .../impl/action_execution/action_execution_client.py | 9 +++------ openhands/runtime/impl/remote/remote_runtime.py | 4 +--- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/openhands/runtime/impl/action_execution/action_execution_client.py b/openhands/runtime/impl/action_execution/action_execution_client.py index 4c9d5d52fae2..24fb8250b30e 100644 --- a/openhands/runtime/impl/action_execution/action_execution_client.py +++ b/openhands/runtime/impl/action_execution/action_execution_client.py @@ -114,8 +114,7 @@ def list_files(self, path: str | None = None) -> list[str]: if path is not None: data['path'] = path - with send_request( - self.session, + with self._send_action_server_request( 'POST', f'{self._get_action_execution_server_host()}/list_files', json=data, @@ -132,8 +131,7 @@ def copy_from(self, path: str) -> Path: try: params = {'path': path} - with send_request( - self.session, + with self._send_action_server_request( 'GET', f'{self._get_action_execution_server_host()}/download_files', params=params, @@ -198,8 +196,7 @@ def get_vscode_token(self) -> str: if self.vscode_enabled and self._runtime_initialized: if self._vscode_token is not None: # cached value return self._vscode_token - with send_request( - self.session, + with self._send_action_server_request( 'GET', f'{self._get_action_execution_server_host()}/vscode/connection_token', timeout=10, diff --git a/openhands/runtime/impl/remote/remote_runtime.py b/openhands/runtime/impl/remote/remote_runtime.py index a8c23bbde5fa..cc2fdbf9391d 100644 --- a/openhands/runtime/impl/remote/remote_runtime.py +++ b/openhands/runtime/impl/remote/remote_runtime.py @@ -20,9 +20,7 @@ ) from openhands.runtime.plugins import PluginRequirement from openhands.runtime.utils.command import get_remote_startup_command -from openhands.runtime.utils.request import ( - send_request, -) +from openhands.runtime.utils.request import send_request from openhands.runtime.utils.runtime_build import build_runtime_image from openhands.utils.async_utils import call_sync_from_async from openhands.utils.tenacity_stop import stop_if_should_exit From 0c58f469b4d0a6ea1e21c75a9273468ee1b16357 Mon Sep 17 00:00:00 2001 From: Talut Salako <70220179+plutack@users.noreply.github.com> Date: Sun, 5 Jan 2025 01:25:45 +0100 Subject: [PATCH 03/21] fix: improve how llm models option (#6026) --- .../src/components/shared/modals/settings/settings-modal.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/src/components/shared/modals/settings/settings-modal.tsx b/frontend/src/components/shared/modals/settings/settings-modal.tsx index 011411f4748f..db8eb821d55c 100644 --- a/frontend/src/components/shared/modals/settings/settings-modal.tsx +++ b/frontend/src/components/shared/modals/settings/settings-modal.tsx @@ -16,7 +16,7 @@ export function SettingsModal({ onClose, settings }: SettingsModalProps) {
{aiConfigOptions.error && (

{aiConfigOptions.error.message}

From f5f988e552856276956d5cad911df3f55eb07a1b Mon Sep 17 00:00:00 2001 From: Xingyao Wang Date: Sat, 4 Jan 2025 20:08:47 -0500 Subject: [PATCH 04/21] fix(agent controller): state.metrics is missing on exception (#6036) --- openhands/controller/agent_controller.py | 5 +- tests/unit/test_agent_controller.py | 70 +++++++++++++++++++++++- tests/unit/test_llm.py | 6 +- 3 files changed, 76 insertions(+), 5 deletions(-) diff --git a/openhands/controller/agent_controller.py b/openhands/controller/agent_controller.py index 743decc4df94..625d64280c52 100644 --- a/openhands/controller/agent_controller.py +++ b/openhands/controller/agent_controller.py @@ -191,6 +191,7 @@ async def _react_to_exception( self, e: Exception, ): + """React to an exception by setting the agent state to error and sending a status message.""" await self.set_agent_state_to(AgentState.ERROR) if self.status_callback is not None: err_id = '' @@ -348,7 +349,6 @@ async def _handle_message_action(self, action: MessageAction) -> None: def _reset(self) -> None: """Resets the agent controller""" - # make sure there is an Observation with the tool call metadata to be recognized by the agent # otherwise the pending action is found in history, but it's incomplete without an obs with tool result if self._pending_action and hasattr(self._pending_action, 'tool_call_metadata'): @@ -389,6 +389,9 @@ async def set_agent_state_to(self, new_state: AgentState) -> None: return if new_state in (AgentState.STOPPED, AgentState.ERROR): + # sync existing metrics BEFORE resetting the agent + self.update_state_after_step() + self.state.metrics.merge(self.state.local_metrics) self._reset() elif ( new_state == AgentState.RUNNING diff --git a/tests/unit/test_agent_controller.py b/tests/unit/test_agent_controller.py index a2136c239366..0eba74edd138 100644 --- a/tests/unit/test_agent_controller.py +++ b/tests/unit/test_agent_controller.py @@ -39,7 +39,11 @@ def event_loop(): def mock_agent(): agent = MagicMock(spec=Agent) agent.llm = MagicMock(spec=LLM) - agent.llm.metrics = MagicMock(spec=Metrics) + metrics = MagicMock(spec=Metrics) + metrics.costs = [] + metrics.accumulated_cost = 0.0 + metrics.response_latencies = [] + agent.llm.metrics = metrics return agent @@ -292,6 +296,14 @@ async def test_delegate_step_different_states( async def test_max_iterations_extension(mock_agent, mock_event_stream): # Test with headless_mode=False - should extend max_iterations initial_state = State(max_iterations=10) + + # Set up proper metrics mock with required attributes + metrics = MagicMock(spec=Metrics) + metrics._costs = [] + metrics._response_latencies = [] + metrics.accumulated_cost = 0.0 + mock_agent.llm.metrics = metrics + controller = AgentController( agent=mock_agent, event_stream=mock_event_stream, @@ -544,3 +556,59 @@ def mock_hasattr(obj, name): # Verify that agent.reset() was called mock_agent.reset.assert_called_once() await controller.close() + + +@pytest.mark.asyncio +async def test_run_controller_max_iterations_has_metrics(): + config = AppConfig( + max_iterations=3, + ) + file_store = InMemoryFileStore({}) + event_stream = EventStream(sid='test', file_store=file_store) + + agent = MagicMock(spec=Agent) + agent.llm = MagicMock(spec=LLM) + agent.llm.metrics = Metrics() + agent.llm.config = config.get_llm_config() + + def agent_step_fn(state): + print(f'agent_step_fn received state: {state}') + # Mock the cost of the LLM + agent.llm.metrics.add_cost(10.0) + print( + f'agent.llm.metrics.accumulated_cost: {agent.llm.metrics.accumulated_cost}' + ) + return CmdRunAction(command='ls') + + agent.step = agent_step_fn + + runtime = MagicMock(spec=Runtime) + + def on_event(event: Event): + if isinstance(event, CmdRunAction): + non_fatal_error_obs = ErrorObservation( + 'Non fatal error. event id: ' + str(event.id) + ) + non_fatal_error_obs._cause = event.id + event_stream.add_event(non_fatal_error_obs, EventSource.ENVIRONMENT) + + event_stream.subscribe(EventStreamSubscriber.RUNTIME, on_event, str(uuid4())) + runtime.event_stream = event_stream + + state = await run_controller( + config=config, + initial_user_action=MessageAction(content='Test message'), + runtime=runtime, + sid='test', + agent=agent, + fake_user_response_fn=lambda _: 'repeat', + ) + assert state.iteration == 3 + assert state.agent_state == AgentState.ERROR + assert ( + state.last_error + == 'RuntimeError: Agent reached maximum iteration in headless mode. Current iteration: 3, max iteration: 3' + ) + assert ( + state.metrics.accumulated_cost == 10.0 * 3 + ), f'Expected accumulated cost to be 30.0, but got {state.metrics.accumulated_cost}' diff --git a/tests/unit/test_llm.py b/tests/unit/test_llm.py index 46923aa217b4..edf82d8aa41b 100644 --- a/tests/unit/test_llm.py +++ b/tests/unit/test_llm.py @@ -141,9 +141,9 @@ def test_llm_reset(): initial_metrics.add_cost(1.0) initial_metrics.add_response_latency(0.5, 'test-id') llm.reset() - assert llm.metrics._accumulated_cost != initial_metrics._accumulated_cost - assert llm.metrics._costs != initial_metrics._costs - assert llm.metrics._response_latencies != initial_metrics._response_latencies + assert llm.metrics.accumulated_cost != initial_metrics.accumulated_cost + assert llm.metrics.costs != initial_metrics.costs + assert llm.metrics.response_latencies != initial_metrics.response_latencies assert isinstance(llm.metrics, Metrics) From 79551e67f66aa7d6f74b38ea640f58b2b34018af Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Sun, 5 Jan 2025 03:43:26 +0100 Subject: [PATCH 05/21] chore(deps): bump docker/setup-qemu-action from 3.0.0 to 3.2.0 (#5798) Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/ghcr-build.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ghcr-build.yml b/.github/workflows/ghcr-build.yml index 26dccac08c30..addfd9610427 100644 --- a/.github/workflows/ghcr-build.yml +++ b/.github/workflows/ghcr-build.yml @@ -56,7 +56,7 @@ jobs: docker-images: false swap-storage: true - name: Set up QEMU - uses: docker/setup-qemu-action@v3.0.0 + uses: docker/setup-qemu-action@v3.2.0 with: image: tonistiigi/binfmt:latest - name: Login to GHCR @@ -119,7 +119,7 @@ jobs: docker-images: false swap-storage: true - name: Set up QEMU - uses: docker/setup-qemu-action@v3.0.0 + uses: docker/setup-qemu-action@v3.2.0 with: image: tonistiigi/binfmt:latest - name: Login to GHCR From e4cf2eee2d07ebe32f296056d4ba78f25c3e5db4 Mon Sep 17 00:00:00 2001 From: OpenHands Date: Sun, 5 Jan 2025 11:44:23 +0900 Subject: [PATCH 06/21] Fix issue #4864: [Bug]: make start-backend results in NotImplementedError: Non-relative patterns are unsupported (#5332) Co-authored-by: Engel Nyst --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index e1f14c410f43..9cae92e0456d 100644 --- a/Makefile +++ b/Makefile @@ -190,7 +190,7 @@ build-frontend: # Start backend start-backend: @echo "$(YELLOW)Starting backend...$(RESET)" - @poetry run uvicorn openhands.server.listen:app --host $(BACKEND_HOST) --port $(BACKEND_PORT) --reload --reload-exclude "$(shell pwd)/workspace" + @poetry run uvicorn openhands.server.listen:app --host $(BACKEND_HOST) --port $(BACKEND_PORT) --reload --reload-exclude "./workspace" # Start frontend start-frontend: From 3d2138d9ce8703aec87b4fe3d0c84b29238ae8fd Mon Sep 17 00:00:00 2001 From: Engel Nyst Date: Sun, 5 Jan 2025 03:58:26 +0100 Subject: [PATCH 07/21] Command line args fixes (#5990) --- config.template.toml | 16 +++++++++ openhands/core/cli.py | 46 ++++++++++++------------ openhands/core/config/__init__.py | 2 ++ openhands/core/config/app_config.py | 3 ++ openhands/core/config/utils.py | 54 +++++++++++++++++++++++++---- openhands/core/main.py | 42 ++++++++++------------ openhands/llm/llm.py | 13 ------- tests/unit/test_arg_parser.py | 7 ++-- tests/unit/test_cli.py | 27 +++++++++++++++ 9 files changed, 142 insertions(+), 68 deletions(-) create mode 100644 tests/unit/test_cli.py diff --git a/config.template.toml b/config.template.toml index 5890c5f301ff..de0ebf3a578f 100644 --- a/config.template.toml +++ b/config.template.toml @@ -198,6 +198,16 @@ model = "gpt-4o" # agent.CodeActAgent ############################################################################## [agent] + +# whether the browsing tool is enabled +codeact_enable_browsing = true + +# whether the LLM draft editor is enabled +codeact_enable_llm_editor = false + +# whether the IPython tool is enabled +codeact_enable_jupyter = true + # Name of the micro agent to use for this agent #micro_agent_name = "" @@ -210,6 +220,12 @@ model = "gpt-4o" # LLM config group to use #llm_config = 'your-llm-config-group' +# Whether to use microagents at all +#use_microagents = true + +# List of microagents to disable +#disabled_microagents = [] + [agent.RepoExplorerAgent] # Example: use a cheaper model for RepoExplorerAgent to reduce cost, especially # useful when an agent doesn't demand high quality but uses a lot of tokens diff --git a/openhands/core/cli.py b/openhands/core/cli.py index 7363a8af5a0d..f1f687fed52a 100644 --- a/openhands/core/cli.py +++ b/openhands/core/cli.py @@ -6,11 +6,10 @@ from termcolor import colored import openhands.agenthub # noqa F401 (we import this to get the agents registered) -from openhands import __version__ from openhands.core.config import ( AppConfig, - get_parser, - load_app_config, + parse_arguments, + setup_config_from_args, ) from openhands.core.logger import openhands_logger as logger from openhands.core.loop import run_agent_until_done @@ -84,27 +83,30 @@ def display_event(event: Event, config: AppConfig): display_confirmation(event.confirmation_state) -async def main(loop): - """Runs the agent in CLI mode""" +def read_input(config: AppConfig) -> str: + """Read input from user based on config settings.""" + if config.cli_multiline_input: + print('Enter your message (enter "/exit" on a new line to finish):') + lines = [] + while True: + line = input('>> ').rstrip() + if line == '/exit': # finish input + break + lines.append(line) + return '\n'.join(lines) + else: + return input('>> ').rstrip() - parser = get_parser() - # Add the version argument - parser.add_argument( - '-v', - '--version', - action='version', - version=f'{__version__}', - help='Show the version number and exit', - default=None, - ) - args = parser.parse_args() - if args.version: - print(f'OpenHands version: {__version__}') - return +async def main(loop: asyncio.AbstractEventLoop): + """Runs the agent in CLI mode""" + + args = parse_arguments() logger.setLevel(logging.WARNING) - config = load_app_config(config_file=args.config_file) + + config = setup_config_from_args(args) + sid = str(uuid4()) runtime = create_runtime(config, sid=sid, headless_mode=True) @@ -116,9 +118,7 @@ async def main(loop): async def prompt_for_next_task(): # Run input() in a thread pool to avoid blocking the event loop - next_message = await loop.run_in_executor( - None, lambda: input('How can I help? >> ') - ) + next_message = await loop.run_in_executor(None, read_input, config) if not next_message.strip(): await prompt_for_next_task() if next_message == 'exit': diff --git a/openhands/core/config/__init__.py b/openhands/core/config/__init__.py index b8fefb715cf3..2e0f87e32143 100644 --- a/openhands/core/config/__init__.py +++ b/openhands/core/config/__init__.py @@ -16,6 +16,7 @@ load_from_env, load_from_toml, parse_arguments, + setup_config_from_args, ) __all__ = [ @@ -34,4 +35,5 @@ 'get_field_info', 'get_parser', 'parse_arguments', + 'setup_config_from_args', ] diff --git a/openhands/core/config/app_config.py b/openhands/core/config/app_config.py index ccdd445d7732..2dbb4aeaa8c4 100644 --- a/openhands/core/config/app_config.py +++ b/openhands/core/config/app_config.py @@ -42,6 +42,8 @@ class AppConfig: file_uploads_max_file_size_mb: Maximum file upload size in MB. `0` means unlimited. file_uploads_restrict_file_types: Whether to restrict upload file types. file_uploads_allowed_extensions: Allowed file extensions. `['.*']` allows all. + cli_multiline_input: Whether to enable multiline input in CLI. When disabled, + input is read line by line. When enabled, input continues until /exit command. """ llms: dict[str, LLMConfig] = field(default_factory=dict) @@ -71,6 +73,7 @@ class AppConfig: file_uploads_restrict_file_types: bool = False file_uploads_allowed_extensions: list[str] = field(default_factory=lambda: ['.*']) runloop_api_key: str | None = None + cli_multiline_input: bool = False defaults_dict: ClassVar[dict] = {} diff --git a/openhands/core/config/utils.py b/openhands/core/config/utils.py index c375d2d55364..7719ce0d59b1 100644 --- a/openhands/core/config/utils.py +++ b/openhands/core/config/utils.py @@ -2,6 +2,7 @@ import os import pathlib import platform +import sys from dataclasses import is_dataclass from types import UnionType from typing import Any, MutableMapping, get_args, get_origin @@ -311,8 +312,14 @@ def get_llm_config_arg( # Command line arguments def get_parser() -> argparse.ArgumentParser: - """Get the parser for the command line arguments.""" - parser = argparse.ArgumentParser(description='Run an agent with a specific task') + """Get the argument parser.""" + parser = argparse.ArgumentParser(description='Run the agent via CLI') + + # Add version argument + parser.add_argument( + '-v', '--version', action='store_true', help='Show version information' + ) + parser.add_argument( '--config-file', type=str, @@ -406,16 +413,23 @@ def get_parser() -> argparse.ArgumentParser: parser.add_argument( '--no-auto-continue', action='store_true', - help='Disable automatic "continue" responses. Will read from stdin instead.', + help='Disable automatic "continue" responses in headless mode. Will read from stdin instead.', ) return parser def parse_arguments() -> argparse.Namespace: - """Parse the command line arguments.""" + """Parse command line arguments.""" parser = get_parser() - parsed_args, _ = parser.parse_known_args() - return parsed_args + args = parser.parse_args() + + if args.version: + from openhands import __version__ + + print(f'OpenHands version: {__version__}') + sys.exit(0) + + return args def load_app_config( @@ -435,3 +449,31 @@ def load_app_config( logger.DEBUG = config.debug logger.DISABLE_COLOR_PRINTING = config.disable_color return config + + +def setup_config_from_args(args: argparse.Namespace) -> AppConfig: + """Load config from toml and override with command line arguments. + + Common setup used by both CLI and main.py entry points. + """ + # Load base config from toml and env vars + config = load_app_config(config_file=args.config_file) + + # Override with command line arguments if provided + if args.llm_config: + llm_config = get_llm_config_arg(args.llm_config) + if llm_config is None: + raise ValueError(f'Invalid toml file, cannot read {args.llm_config}') + config.set_llm_config(llm_config) + + # Override default agent if provided + if args.agent_cls: + config.default_agent = args.agent_cls + + # Set max iterations and max budget per task if provided, otherwise fall back to config values + if args.max_iterations is not None: + config.max_iterations = args.max_iterations + if args.max_budget_per_task is not None: + config.max_budget_per_task = args.max_budget_per_task + + return config diff --git a/openhands/core/main.py b/openhands/core/main.py index d0436f20e4d9..65e328648358 100644 --- a/openhands/core/main.py +++ b/openhands/core/main.py @@ -9,9 +9,8 @@ from openhands.controller.state.state import State from openhands.core.config import ( AppConfig, - get_llm_config_arg, - load_app_config, parse_arguments, + setup_config_from_args, ) from openhands.core.logger import openhands_logger as logger from openhands.core.loop import run_agent_until_done @@ -51,6 +50,21 @@ def read_task_from_stdin() -> str: return sys.stdin.read() +def read_input(config: AppConfig) -> str: + """Read input from user based on config settings.""" + if config.cli_multiline_input: + print('Enter your message (enter "/exit" on a new line to finish):') + lines = [] + while True: + line = input('>> ').rstrip() + if line == '/exit': # finish input + break + lines.append(line) + return '\n'.join(lines) + else: + return input('>> ').rstrip() + + async def run_controller( config: AppConfig, initial_user_action: Action, @@ -120,9 +134,7 @@ def on_event(event: Event): if exit_on_message: message = '/exit' elif fake_user_response_fn is None: - # read until EOF (Ctrl+D on Unix, Ctrl+Z on Windows) - print('Request user input (press Ctrl+D/Z when done) >> ') - message = sys.stdin.read().rstrip() + message = read_input(config) else: message = fake_user_response_fn(controller.get_state()) action = MessageAction(content=message) @@ -195,31 +207,13 @@ def auto_continue_response( else: raise ValueError('No task provided. Please specify a task through -t, -f.') initial_user_action: MessageAction = MessageAction(content=task_str) - # Load the app config - # this will load config from config.toml in the current directory - # as well as from the environment variables - config = load_app_config(config_file=args.config_file) - # Override default LLM configs ([llm] section in config.toml) - if args.llm_config: - llm_config = get_llm_config_arg(args.llm_config) - if llm_config is None: - raise ValueError(f'Invalid toml file, cannot read {args.llm_config}') - config.set_llm_config(llm_config) - - # Set default agent - config.default_agent = args.agent_cls + config = setup_config_from_args(args) # Set session name session_name = args.name sid = generate_sid(config, session_name) - # if max budget per task is not sent on the command line, use the config value - if args.max_budget_per_task is not None: - config.max_budget_per_task = args.max_budget_per_task - if args.max_iterations is not None: - config.max_iterations = args.max_iterations - asyncio.run( run_controller( config=config, diff --git a/openhands/llm/llm.py b/openhands/llm/llm.py index cb0778765e24..743d6535ba3b 100644 --- a/openhands/llm/llm.py +++ b/openhands/llm/llm.py @@ -122,12 +122,6 @@ def __init__( if self.is_function_calling_active(): logger.debug('LLM: model supports function calling') - # Compatibility flag: use string serializer for DeepSeek models - # See this issue: https://github.com/All-Hands-AI/OpenHands/issues/5818 - self._use_string_serializer = False - if 'deepseek' in self.config.model: - self._use_string_serializer = True - # if using a custom tokenizer, make sure it's loaded and accessible in the format expected by litellm if self.config.custom_tokenizer is not None: self.tokenizer = create_pretrained_tokenizer(self.config.custom_tokenizer) @@ -449,21 +443,14 @@ def is_function_calling_active(self) -> bool: # Handle native_tool_calling user-defined configuration if self.config.native_tool_calling is None: - logger.debug( - f'Using default tool calling behavior based on model evaluation: {model_name_supported}' - ) return model_name_supported elif self.config.native_tool_calling is False: - logger.debug('Function calling explicitly disabled via configuration') return False else: # try to enable native tool calling if supported by the model supports_fn_call = litellm.supports_function_calling( model=self.config.model ) - logger.debug( - f'Function calling explicitly enabled, litellm support: {supports_fn_call}' - ) return supports_fn_call def _post_completion(self, response: ModelResponse) -> float: diff --git a/tests/unit/test_arg_parser.py b/tests/unit/test_arg_parser.py index ebfa629a5f00..51c736f19c05 100644 --- a/tests/unit/test_arg_parser.py +++ b/tests/unit/test_arg_parser.py @@ -26,6 +26,7 @@ def test_parser_custom_values(): parser = get_parser() args = parser.parse_args( [ + '-v', '-d', '/path/to/dir', '-t', @@ -67,6 +68,7 @@ def test_parser_custom_values(): assert args.llm_config == 'gpt4' assert args.name == 'test_session' assert args.no_auto_continue + assert args.version def test_parser_file_overrides_task(): @@ -110,8 +112,9 @@ def test_help_message(capsys): print(help_output) expected_elements = [ 'usage:', - 'Run an agent with a specific task', + 'Run the agent via CLI', 'options:', + '-v, --version', '-h, --help', '-d DIRECTORY, --directory DIRECTORY', '-t TASK, --task TASK', @@ -134,4 +137,4 @@ def test_help_message(capsys): assert element in help_output, f"Expected '{element}' to be in the help message" option_count = help_output.count(' -') - assert option_count == 16, f'Expected 16 options, found {option_count}' + assert option_count == 17, f'Expected 17 options, found {option_count}' diff --git a/tests/unit/test_cli.py b/tests/unit/test_cli.py new file mode 100644 index 000000000000..520d85d2aa7d --- /dev/null +++ b/tests/unit/test_cli.py @@ -0,0 +1,27 @@ +from unittest.mock import patch + +from openhands.core.cli import read_input +from openhands.core.config import AppConfig + + +def test_single_line_input(): + """Test that single line input works when cli_multiline_input is False""" + config = AppConfig() + config.cli_multiline_input = False + + with patch('builtins.input', return_value='hello world'): + result = read_input(config) + assert result == 'hello world' + + +def test_multiline_input(): + """Test that multiline input works when cli_multiline_input is True""" + config = AppConfig() + config.cli_multiline_input = True + + # Simulate multiple lines of input followed by /exit + mock_inputs = ['line 1', 'line 2', 'line 3', '/exit'] + + with patch('builtins.input', side_effect=mock_inputs): + result = read_input(config) + assert result == 'line 1\nline 2\nline 3' From d2790c8b212493918a4efb6afe81fd703a651be2 Mon Sep 17 00:00:00 2001 From: f-diao Date: Sat, 4 Jan 2025 20:10:51 -0800 Subject: [PATCH 08/21] docs: Update the referenced py filename. (#6043) --- openhands/server/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/openhands/server/README.md b/openhands/server/README.md index c2f104f04059..03f26a0812d2 100644 --- a/openhands/server/README.md +++ b/openhands/server/README.md @@ -116,9 +116,9 @@ The `session.py` file defines the `Session` class, which represents a WebSocket - Dispatching events between the client and the agent - Sending messages and errors to the client -### 2. session/agent.py +### 2. session/agent_session.py -The `agent.py` file contains the `AgentSession` class, which manages the lifecycle of an agent within a session. Key features include: +The `agent_session.py` file contains the `AgentSession` class, which manages the lifecycle of an agent within a session. Key features include: - Creating and managing the runtime environment - Initializing the agent controller From b7bbf0f5eb8523ceeb06d5d596cf47ad7aac4f94 Mon Sep 17 00:00:00 2001 From: Xingyao Wang Date: Sat, 4 Jan 2025 23:57:07 -0500 Subject: [PATCH 09/21] fix(agent controller): missing await (#6040) --- openhands/controller/agent_controller.py | 2 +- tests/unit/test_agent_controller.py | 13 +------------ 2 files changed, 2 insertions(+), 13 deletions(-) diff --git a/openhands/controller/agent_controller.py b/openhands/controller/agent_controller.py index 625d64280c52..9c5cae6faa5c 100644 --- a/openhands/controller/agent_controller.py +++ b/openhands/controller/agent_controller.py @@ -390,7 +390,7 @@ async def set_agent_state_to(self, new_state: AgentState) -> None: if new_state in (AgentState.STOPPED, AgentState.ERROR): # sync existing metrics BEFORE resetting the agent - self.update_state_after_step() + await self.update_state_after_step() self.state.metrics.merge(self.state.local_metrics) self._reset() elif ( diff --git a/tests/unit/test_agent_controller.py b/tests/unit/test_agent_controller.py index 0eba74edd138..72327570735b 100644 --- a/tests/unit/test_agent_controller.py +++ b/tests/unit/test_agent_controller.py @@ -39,11 +39,7 @@ def event_loop(): def mock_agent(): agent = MagicMock(spec=Agent) agent.llm = MagicMock(spec=LLM) - metrics = MagicMock(spec=Metrics) - metrics.costs = [] - metrics.accumulated_cost = 0.0 - metrics.response_latencies = [] - agent.llm.metrics = metrics + agent.llm.metrics = Metrics() return agent @@ -297,13 +293,6 @@ async def test_max_iterations_extension(mock_agent, mock_event_stream): # Test with headless_mode=False - should extend max_iterations initial_state = State(max_iterations=10) - # Set up proper metrics mock with required attributes - metrics = MagicMock(spec=Metrics) - metrics._costs = [] - metrics._response_latencies = [] - metrics.accumulated_cost = 0.0 - mock_agent.llm.metrics = metrics - controller = AgentController( agent=mock_agent, event_stream=mock_event_stream, From 150463e629e1f9f73f5c010a0b24b0f0ff44b0da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=AE=AE=E0=AE=A9=E0=AF=8B=E0=AE=9C=E0=AF=8D=E0=AE=95?= =?UTF-8?q?=E0=AF=81=E0=AE=AE=E0=AE=BE=E0=AE=B0=E0=AF=8D=20=E0=AE=AA?= =?UTF-8?q?=E0=AE=B4=E0=AE=A9=E0=AE=BF=E0=AE=9A=E0=AF=8D=E0=AE=9A=E0=AE=BE?= =?UTF-8?q?=E0=AE=AE=E0=AE=BF?= Date: Sun, 5 Jan 2025 11:58:05 +0530 Subject: [PATCH 10/21] feat: Add GPU support (#6042) --- openhands/core/config/sandbox_config.py | 2 ++ openhands/runtime/impl/docker/docker_runtime.py | 8 ++++++++ 2 files changed, 10 insertions(+) diff --git a/openhands/core/config/sandbox_config.py b/openhands/core/config/sandbox_config.py index 3c918444c4fe..a91c8b06a32a 100644 --- a/openhands/core/config/sandbox_config.py +++ b/openhands/core/config/sandbox_config.py @@ -34,6 +34,7 @@ class SandboxConfig: platform: The platform on which the image should be built. Default is None. remote_runtime_resource_factor: Factor to scale the resource allocation for remote runtime. Must be one of [1, 2, 4, 8]. Will only be used if the runtime is remote. + enable_gpu: Whether to enable GPU. """ remote_runtime_api_url: str = 'http://localhost:8000' @@ -59,6 +60,7 @@ class SandboxConfig: platform: str | None = None close_delay: int = 15 remote_runtime_resource_factor: int = 1 + enable_gpu: bool = False def defaults_to_dict(self) -> dict: """Serialize fields to a dict for the frontend, including type hints, defaults, and whether it's optional.""" diff --git a/openhands/runtime/impl/docker/docker_runtime.py b/openhands/runtime/impl/docker/docker_runtime.py index 62623d53a2fc..852e50f617d1 100644 --- a/openhands/runtime/impl/docker/docker_runtime.py +++ b/openhands/runtime/impl/docker/docker_runtime.py @@ -266,6 +266,14 @@ def _init_container(self): detach=True, environment=environment, volumes=volumes, + device_requests=( + [docker.types.DeviceRequest( + capabilities=[['gpu']], + count=-1 + )] + if self.config.sandbox.enable_gpu + else None + ), ) self.log('debug', f'Container started. Server url: {self.api_url}') self.send_status_message('STATUS$CONTAINER_STARTED') From 00d7395e09d32754cd3c96bf38100f5889873c31 Mon Sep 17 00:00:00 2001 From: Boxuan Li Date: Sun, 5 Jan 2025 14:43:05 -0800 Subject: [PATCH 11/21] Makefile: Fix poetry version detector (#6058) --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 9cae92e0456d..0317224d6847 100644 --- a/Makefile +++ b/Makefile @@ -106,7 +106,7 @@ check-poetry: @if command -v poetry > /dev/null; then \ POETRY_VERSION=$(shell poetry --version 2>&1 | sed -E 's/Poetry \(version ([0-9]+\.[0-9]+\.[0-9]+)\)/\1/'); \ IFS='.' read -r -a POETRY_VERSION_ARRAY <<< "$$POETRY_VERSION"; \ - if [ $${POETRY_VERSION_ARRAY[0]} -ge 1 ] && [ $${POETRY_VERSION_ARRAY[1]} -ge 8 ]; then \ + if [ $${POETRY_VERSION_ARRAY[0]} -gt 1 ] || ([ $${POETRY_VERSION_ARRAY[0]} -eq 1 ] && [ $${POETRY_VERSION_ARRAY[1]} -ge 8 ]); then \ echo "$(BLUE)$(shell poetry --version) is already installed.$(RESET)"; \ else \ echo "$(RED)Poetry 1.8 or later is required. You can install poetry by running the following command, then adding Poetry to your PATH:"; \ From f8735efadff01db35223a71e064ac59b0d45128a Mon Sep 17 00:00:00 2001 From: Xingyao Wang Date: Sun, 5 Jan 2025 18:02:42 -0500 Subject: [PATCH 12/21] chore: improve error logging for RuntimeError (#6055) --- openhands/controller/agent_controller.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/openhands/controller/agent_controller.py b/openhands/controller/agent_controller.py index 9c5cae6faa5c..f94b95bb84bb 100644 --- a/openhands/controller/agent_controller.py +++ b/openhands/controller/agent_controller.py @@ -209,10 +209,15 @@ async def _step_with_exception_handling(self): try: await self._step() except Exception as e: - traceback.print_exc() - self.log('error', f'Error while running the agent: {e}') + self.log( + 'error', + f'Error while running the agent (session ID: {self.id}): {e}. ' + f'Traceback: {traceback.format_exc()}', + ) reported = RuntimeError( - 'There was an unexpected error while running the agent.' + 'There was an unexpected error while running the agent. Please ' + f'report this error to the developers. Your session ID is {self.id}. ' + f'Exception: {e}.' ) if isinstance(e, litellm.AuthenticationError) or isinstance( e, litellm.BadRequestError From efd02679199615da7d0dfb9bd3c04e2e0e6488ee Mon Sep 17 00:00:00 2001 From: stefand678 Date: Sun, 5 Jan 2025 20:43:53 -0800 Subject: [PATCH 13/21] docs: Fix inconsistent comments (#6051) --- openhands/server/routes/files.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/openhands/server/routes/files.py b/openhands/server/routes/files.py index d048f5e61765..2efb85ceaddd 100644 --- a/openhands/server/routes/files.py +++ b/openhands/server/routes/files.py @@ -45,7 +45,7 @@ async def list_files(request: Request, conversation_id: str, path: str | None = To list files: ```sh - curl http://localhost:3000/api/list-files + curl http://localhost:3000/api/conversations/{conversation_id}/list-files ``` Args: @@ -110,7 +110,7 @@ async def select_file(file: str, request: Request): To select a file: ```sh - curl http://localhost:3000/api/select-file?file= + curl http://localhost:3000/api/conversations/{conversation_id}select-file?file= ``` Args: @@ -154,7 +154,7 @@ async def upload_file(request: Request, conversation_id: str, files: list[Upload To upload a files: ```sh - curl -X POST -F "file=@" -F "file=@" http://localhost:3000/api/upload-files + curl -X POST -F "file=@" -F "file=@" http://localhost:3000/api/conversations/{conversation_id}/upload-files ``` Args: From cde8aad47fc5a7be8f055073e8bd268247f5a2f5 Mon Sep 17 00:00:00 2001 From: tofarr Date: Mon, 6 Jan 2025 07:43:11 -0700 Subject: [PATCH 14/21] Feat multi conversations wiring (#6011) Co-authored-by: openhands --- .../conversation-card.test.tsx | 100 +++++++++--------- .../conversation-panel.test.tsx | 2 +- .../features/sidebar/sidebar.test.tsx | 6 +- frontend/__tests__/routes/_oh.app.test.tsx | 12 +-- frontend/src/api/open-hands.ts | 11 +- frontend/src/api/open-hands.types.ts | 15 ++- .../features/context-menu/context-menu.tsx | 2 +- .../conversation-panel/conversation-card.tsx | 60 ++++++----- .../conversation-panel/conversation-panel.tsx | 14 +-- .../conversation-repo-link.tsx | 10 +- .../conversation-state-indicator.tsx | 30 ++---- .../components/features/sidebar/sidebar.tsx | 39 +++++-- frontend/src/context/ws-client-provider.tsx | 4 + .../query/get-conversation-permissions.ts | 4 +- frontend/src/mocks/handlers.ts | 49 +++++---- frontend/src/routes/_oh.app/route.tsx | 4 +- frontend/src/utils/constants.ts | 1 - frontend/src/utils/feature-flags.ts | 15 +++ openhands/core/config/sandbox_config.py | 2 +- 19 files changed, 210 insertions(+), 170 deletions(-) delete mode 100644 frontend/src/utils/constants.ts create mode 100644 frontend/src/utils/feature-flags.ts diff --git a/frontend/__tests__/components/features/conversation-panel/conversation-card.test.tsx b/frontend/__tests__/components/features/conversation-panel/conversation-card.test.tsx index 749bc6c48de0..e07eb25a3d00 100644 --- a/frontend/__tests__/components/features/conversation-panel/conversation-card.test.tsx +++ b/frontend/__tests__/components/features/conversation-panel/conversation-card.test.tsx @@ -19,9 +19,9 @@ describe("ConversationCard", () => { onDelete={onDelete} onClick={onClick} onChangeTitle={onChangeTitle} - name="Conversation 1" - repo={null} - lastUpdated="2021-10-01T12:00:00Z" + title="Conversation 1" + selectedRepository={null} + lastUpdatedAt="2021-10-01T12:00:00Z" />, ); const expectedDate = `${formatTimeDelta(new Date("2021-10-01T12:00:00Z"))} ago`; @@ -33,20 +33,20 @@ describe("ConversationCard", () => { within(card).getByText(expectedDate); }); - it("should render the repo if available", () => { + it("should render the selectedRepository if available", () => { const { rerender } = render( , ); expect( - screen.queryByTestId("conversation-card-repo"), + screen.queryByTestId("conversation-card-selected-repository"), ).not.toBeInTheDocument(); rerender( @@ -54,13 +54,13 @@ describe("ConversationCard", () => { onDelete={onDelete} onClick={onClick} onChangeTitle={onChangeTitle} - name="Conversation 1" - repo="org/repo" - lastUpdated="2021-10-01T12:00:00Z" + title="Conversation 1" + selectedRepository="org/selectedRepository" + lastUpdatedAt="2021-10-01T12:00:00Z" />, ); - screen.getByTestId("conversation-card-repo"); + screen.getByTestId("conversation-card-selected-repository"); }); it("should call onClick when the card is clicked", async () => { @@ -70,9 +70,9 @@ describe("ConversationCard", () => { onDelete={onDelete} onClick={onClick} onChangeTitle={onChangeTitle} - name="Conversation 1" - repo={null} - lastUpdated="2021-10-01T12:00:00Z" + title="Conversation 1" + selectedRepository={null} + lastUpdatedAt="2021-10-01T12:00:00Z" />, ); @@ -89,9 +89,9 @@ describe("ConversationCard", () => { onDelete={onDelete} onClick={onClick} onChangeTitle={onChangeTitle} - name="Conversation 1" - repo={null} - lastUpdated="2021-10-01T12:00:00Z" + title="Conversation 1" + selectedRepository={null} + lastUpdatedAt="2021-10-01T12:00:00Z" />, ); @@ -114,9 +114,9 @@ describe("ConversationCard", () => { onClick={onClick} onDelete={onDelete} onChangeTitle={onChangeTitle} - name="Conversation 1" - repo={null} - lastUpdated="2021-10-01T12:00:00Z" + title="Conversation 1" + selectedRepository={null} + lastUpdatedAt="2021-10-01T12:00:00Z" />, ); @@ -131,21 +131,21 @@ describe("ConversationCard", () => { expect(onDelete).toHaveBeenCalled(); }); - test("clicking the repo should not trigger the onClick handler", async () => { + test("clicking the selectedRepository should not trigger the onClick handler", async () => { const user = userEvent.setup(); render( , ); - const repo = screen.getByTestId("conversation-card-repo"); - await user.click(repo); + const selectedRepository = screen.getByTestId("conversation-card-selected-repository"); + await user.click(selectedRepository); expect(onClick).not.toHaveBeenCalled(); }); @@ -156,9 +156,9 @@ describe("ConversationCard", () => { , ); @@ -180,9 +180,9 @@ describe("ConversationCard", () => { onClick={onClick} onDelete={onDelete} onChangeTitle={onChangeTitle} - name="Conversation 1" - repo={null} - lastUpdated="2021-10-01T12:00:00Z" + title="Conversation 1" + selectedRepository={null} + lastUpdatedAt="2021-10-01T12:00:00Z" />, ); @@ -202,9 +202,9 @@ describe("ConversationCard", () => { onClick={onClick} onDelete={onDelete} onChangeTitle={onChangeTitle} - name="Conversation 1" - repo={null} - lastUpdated="2021-10-01T12:00:00Z" + title="Conversation 1" + selectedRepository={null} + lastUpdatedAt="2021-10-01T12:00:00Z" />, ); @@ -221,9 +221,9 @@ describe("ConversationCard", () => { onClick={onClick} onDelete={onDelete} onChangeTitle={onChangeTitle} - name="Conversation 1" - repo={null} - lastUpdated="2021-10-01T12:00:00Z" + title="Conversation 1" + selectedRepository={null} + lastUpdatedAt="2021-10-01T12:00:00Z" />, ); @@ -239,19 +239,19 @@ describe("ConversationCard", () => { }); describe("state indicator", () => { - it("should render the 'cold' indicator by default", () => { + it("should render the 'STOPPED' indicator by default", () => { render( , ); - screen.getByTestId("cold-indicator"); + screen.getByTestId("STOPPED-indicator"); }); it("should render the other indicators when provided", () => { @@ -260,15 +260,15 @@ describe("ConversationCard", () => { onClick={onClick} onDelete={onDelete} onChangeTitle={onChangeTitle} - name="Conversation 1" - repo={null} - lastUpdated="2021-10-01T12:00:00Z" - state="warm" + title="Conversation 1" + selectedRepository={null} + lastUpdatedAt="2021-10-01T12:00:00Z" + status="RUNNING" />, ); - expect(screen.queryByTestId("cold-indicator")).not.toBeInTheDocument(); - screen.getByTestId("warm-indicator"); + expect(screen.queryByTestId("STOPPED-indicator")).not.toBeInTheDocument(); + screen.getByTestId("RUNNING-indicator"); }); }); }); diff --git a/frontend/__tests__/components/features/conversation-panel/conversation-panel.test.tsx b/frontend/__tests__/components/features/conversation-panel/conversation-panel.test.tsx index 5a1d703b22ce..262b2bd28e38 100644 --- a/frontend/__tests__/components/features/conversation-panel/conversation-panel.test.tsx +++ b/frontend/__tests__/components/features/conversation-panel/conversation-panel.test.tsx @@ -175,7 +175,7 @@ describe("ConversationPanel", () => { // Ensure the conversation is renamed expect(updateUserConversationSpy).toHaveBeenCalledWith("3", { - name: "Conversation 1 Renamed", + title: "Conversation 1 Renamed", }); }); diff --git a/frontend/__tests__/components/features/sidebar/sidebar.test.tsx b/frontend/__tests__/components/features/sidebar/sidebar.test.tsx index 40d0ea4a48bc..1e80607301b0 100644 --- a/frontend/__tests__/components/features/sidebar/sidebar.test.tsx +++ b/frontend/__tests__/components/features/sidebar/sidebar.test.tsx @@ -4,7 +4,7 @@ import { describe, expect, it } from "vitest"; import { renderWithProviders } from "test-utils"; import { createRoutesStub } from "react-router"; import { Sidebar } from "#/components/features/sidebar/sidebar"; -import { MULTI_CONVO_UI_IS_ENABLED } from "#/utils/constants"; +import { MULTI_CONVERSATION_UI } from "#/utils/feature-flags"; const renderSidebar = () => { const RouterStub = createRoutesStub([ @@ -18,7 +18,7 @@ const renderSidebar = () => { }; describe("Sidebar", () => { - it.skipIf(!MULTI_CONVO_UI_IS_ENABLED)( + it.skipIf(!MULTI_CONVERSATION_UI)( "should have the conversation panel open by default", () => { renderSidebar(); @@ -26,7 +26,7 @@ describe("Sidebar", () => { }, ); - it.skipIf(!MULTI_CONVO_UI_IS_ENABLED)( + it.skipIf(!MULTI_CONVERSATION_UI)( "should toggle the conversation panel", async () => { const user = userEvent.setup(); diff --git a/frontend/__tests__/routes/_oh.app.test.tsx b/frontend/__tests__/routes/_oh.app.test.tsx index 2addbc5fe604..e034c7476545 100644 --- a/frontend/__tests__/routes/_oh.app.test.tsx +++ b/frontend/__tests__/routes/_oh.app.test.tsx @@ -5,7 +5,7 @@ import { screen, waitFor } from "@testing-library/react"; import toast from "react-hot-toast"; import App from "#/routes/_oh.app/route"; import OpenHands from "#/api/open-hands"; -import { MULTI_CONVO_UI_IS_ENABLED } from "#/utils/constants"; +import { MULTI_CONVERSATION_UI } from "#/utils/feature-flags"; describe("App", () => { const RouteStub = createRoutesStub([ @@ -35,7 +35,7 @@ describe("App", () => { await screen.findByTestId("app-route"); }); - it.skipIf(!MULTI_CONVO_UI_IS_ENABLED)( + it.skipIf(!MULTI_CONVERSATION_UI)( "should call endSession if the user does not have permission to view conversation", async () => { const errorToastSpy = vi.spyOn(toast, "error"); @@ -59,10 +59,10 @@ describe("App", () => { getConversationSpy.mockResolvedValue({ conversation_id: "9999", - lastUpdated: "", - name: "", - repo: "", - state: "cold", + last_updated_at: "", + title: "", + selected_repository: "", + status: "STOPPED", }); const { rerender } = renderWithProviders( , diff --git a/frontend/src/api/open-hands.ts b/frontend/src/api/open-hands.ts index 1534be879ca5..08dec1ebe258 100644 --- a/frontend/src/api/open-hands.ts +++ b/frontend/src/api/open-hands.ts @@ -9,6 +9,7 @@ import { GetVSCodeUrlResponse, AuthenticateResponse, Conversation, + ResultSet, } from "./open-hands.types"; import { openHands } from "./open-hands-axios"; import { ApiSettings } from "#/services/settings"; @@ -222,8 +223,10 @@ class OpenHands { } static async getUserConversations(): Promise { - const { data } = await openHands.get("/api/conversations"); - return data; + const { data } = await openHands.get>( + "/api/conversations?limit=9", + ); + return data.results; } static async deleteUserConversation(conversationId: string): Promise { @@ -232,9 +235,9 @@ class OpenHands { static async updateUserConversation( conversationId: string, - conversation: Partial>, + conversation: Partial>, ): Promise { - await openHands.put(`/api/conversations/${conversationId}`, conversation); + await openHands.patch(`/api/conversations/${conversationId}`, conversation); } static async createConversation( diff --git a/frontend/src/api/open-hands.types.ts b/frontend/src/api/open-hands.types.ts index c17d2016816d..263bfb781162 100644 --- a/frontend/src/api/open-hands.types.ts +++ b/frontend/src/api/open-hands.types.ts @@ -1,4 +1,4 @@ -import { ProjectState } from "#/components/features/conversation-panel/conversation-state-indicator"; +import { ProjectStatus } from "#/components/features/conversation-panel/conversation-state-indicator"; export interface ErrorResponse { error: string; @@ -62,8 +62,13 @@ export interface AuthenticateResponse { export interface Conversation { conversation_id: string; - name: string; - repo: string | null; - lastUpdated: string; - state: ProjectState; + title: string; + selected_repository: string | null; + last_updated_at: string; + status: ProjectStatus; +} + +export interface ResultSet { + results: T[]; + next_page_id: string | null; } diff --git a/frontend/src/components/features/context-menu/context-menu.tsx b/frontend/src/components/features/context-menu/context-menu.tsx index d4d47708da4b..8350391a7b55 100644 --- a/frontend/src/components/features/context-menu/context-menu.tsx +++ b/frontend/src/components/features/context-menu/context-menu.tsx @@ -18,7 +18,7 @@ export function ContextMenu({
    {children}
diff --git a/frontend/src/components/features/conversation-panel/conversation-card.tsx b/frontend/src/components/features/conversation-panel/conversation-card.tsx index bba98607bb23..c10e90f005ba 100644 --- a/frontend/src/components/features/conversation-panel/conversation-card.tsx +++ b/frontend/src/components/features/conversation-panel/conversation-card.tsx @@ -2,7 +2,7 @@ import React from "react"; import { formatTimeDelta } from "#/utils/format-time-delta"; import { ConversationRepoLink } from "./conversation-repo-link"; import { - ProjectState, + ProjectStatus, ConversationStateIndicator, } from "./conversation-state-indicator"; import { ContextMenu } from "../context-menu/context-menu"; @@ -13,20 +13,20 @@ interface ProjectCardProps { onClick: () => void; onDelete: () => void; onChangeTitle: (title: string) => void; - name: string; - repo: string | null; - lastUpdated: string; // ISO 8601 - state?: ProjectState; + title: string; + selectedRepository: string | null; + lastUpdatedAt: string; // ISO 8601 + status?: ProjectStatus; } export function ConversationCard({ onClick, onDelete, onChangeTitle, - name, - repo, - lastUpdated, - state = "cold", + title, + selectedRepository, + lastUpdatedAt, + status = "STOPPED", }: ProjectCardProps) { const [contextMenuVisible, setContextMenuVisible] = React.useState(false); const inputRef = React.useRef(null); @@ -38,7 +38,13 @@ export function ConversationCard({ inputRef.current!.value = trimmed; } else { // reset the value if it's empty - inputRef.current!.value = name; + inputRef.current!.value = title; + } + }; + + const handleKeyUp = (event: React.KeyboardEvent) => { + if (event.key === "Enter") { + event.currentTarget.blur(); } }; @@ -55,47 +61,45 @@ export function ConversationCard({
-
+
- + { event.stopPropagation(); setContextMenuVisible((prev) => !prev); }} /> - {contextMenuVisible && ( - - - Delete - - - )}
- {repo && ( + {contextMenuVisible && ( + + + Delete + + + )} + {selectedRepository && ( e.stopPropagation()} /> )}

- +

); diff --git a/frontend/src/components/features/conversation-panel/conversation-panel.tsx b/frontend/src/components/features/conversation-panel/conversation-panel.tsx index 608856c68db0..fd22cb74399b 100644 --- a/frontend/src/components/features/conversation-panel/conversation-panel.tsx +++ b/frontend/src/components/features/conversation-panel/conversation-panel.tsx @@ -60,7 +60,7 @@ export function ConversationPanel({ onClose }: ConversationPanelProps) { if (oldTitle !== newTitle) updateConversation({ id: conversationId, - conversation: { name: newTitle }, + conversation: { title: newTitle }, }); }; @@ -72,7 +72,7 @@ export function ConversationPanel({ onClose }: ConversationPanelProps) { return (
{location.pathname.startsWith("/conversation") && ( @@ -98,12 +98,12 @@ export function ConversationPanel({ onClose }: ConversationPanelProps) { onClick={() => handleClickCard(project.conversation_id)} onDelete={() => handleDeleteProject(project.conversation_id)} onChangeTitle={(title) => - handleChangeTitle(project.conversation_id, project.name, title) + handleChangeTitle(project.conversation_id, project.title, title) } - name={project.name} - repo={project.repo} - lastUpdated={project.lastUpdated} - state={project.state} + title={project.title} + selectedRepository={project.selected_repository} + lastUpdatedAt={project.last_updated_at} + status={project.status} /> ))} diff --git a/frontend/src/components/features/conversation-panel/conversation-repo-link.tsx b/frontend/src/components/features/conversation-panel/conversation-repo-link.tsx index c2e527c2a6f9..28480277c3f4 100644 --- a/frontend/src/components/features/conversation-panel/conversation-repo-link.tsx +++ b/frontend/src/components/features/conversation-panel/conversation-repo-link.tsx @@ -1,21 +1,21 @@ interface ConversationRepoLinkProps { - repo: string; + selectedRepository: string; onClick?: (event: React.MouseEvent) => void; } export function ConversationRepoLink({ - repo, + selectedRepository, onClick, }: ConversationRepoLinkProps) { return ( - {repo} + {selectedRepository} ); } diff --git a/frontend/src/components/features/conversation-panel/conversation-state-indicator.tsx b/frontend/src/components/features/conversation-panel/conversation-state-indicator.tsx index b42a13606def..c0132c2496d8 100644 --- a/frontend/src/components/features/conversation-panel/conversation-state-indicator.tsx +++ b/frontend/src/components/features/conversation-panel/conversation-state-indicator.tsx @@ -1,39 +1,25 @@ import ColdIcon from "./state-indicators/cold.svg?react"; -import CoolingIcon from "./state-indicators/cooling.svg?react"; -import FinishedIcon from "./state-indicators/finished.svg?react"; import RunningIcon from "./state-indicators/running.svg?react"; -import WaitingIcon from "./state-indicators/waiting.svg?react"; -import WarmIcon from "./state-indicators/warm.svg?react"; type SVGIcon = React.FunctionComponent>; -export type ProjectState = - | "cold" - | "cooling" - | "finished" - | "running" - | "waiting" - | "warm"; +export type ProjectStatus = "RUNNING" | "STOPPED"; -const INDICATORS: Record = { - cold: ColdIcon, - cooling: CoolingIcon, - finished: FinishedIcon, - running: RunningIcon, - waiting: WaitingIcon, - warm: WarmIcon, +const INDICATORS: Record = { + STOPPED: ColdIcon, + RUNNING: RunningIcon, }; interface ConversationStateIndicatorProps { - state: ProjectState; + status: ProjectStatus; } export function ConversationStateIndicator({ - state, + status, }: ConversationStateIndicatorProps) { - const StateIcon = INDICATORS[state]; + const StateIcon = INDICATORS[status]; return ( -
+
); diff --git a/frontend/src/components/features/sidebar/sidebar.tsx b/frontend/src/components/features/sidebar/sidebar.tsx index 3d2e9a3e60b0..112375650487 100644 --- a/frontend/src/components/features/sidebar/sidebar.tsx +++ b/frontend/src/components/features/sidebar/sidebar.tsx @@ -1,6 +1,6 @@ import React from "react"; import { useLocation } from "react-router"; -import FolderIcon from "#/icons/docs.svg?react"; +import { FaListUl } from "react-icons/fa"; import { useAuth } from "#/context/auth-context"; import { useGitHubUser } from "#/hooks/query/use-github-user"; import { useIsAuthed } from "#/hooks/query/use-is-authed"; @@ -16,8 +16,7 @@ import { SettingsModal } from "#/components/shared/modals/settings/settings-moda import { useSettingsUpToDate } from "#/context/settings-up-to-date-context"; import { useSettings } from "#/hooks/query/use-settings"; import { ConversationPanel } from "../conversation-panel/conversation-panel"; -import { cn } from "#/utils/utils"; -import { MULTI_CONVO_UI_IS_ENABLED } from "#/utils/constants"; +import { MULTI_CONVERSATION_UI } from "#/utils/feature-flags"; export function Sidebar() { const location = useLocation(); @@ -32,9 +31,18 @@ export function Sidebar() { const [settingsModalIsOpen, setSettingsModalIsOpen] = React.useState(false); const [startNewProjectModalIsOpen, setStartNewProjectModalIsOpen] = React.useState(false); - const [conversationPanelIsOpen, setConversationPanelIsOpen] = React.useState( - MULTI_CONVO_UI_IS_ENABLED, - ); + const [conversationPanelIsOpen, setConversationPanelIsOpen] = + React.useState(false); + const conversationPanelRef = React.useRef(null); + + const handleClick = (event: MouseEvent) => { + const conversationPanel = conversationPanelRef.current; + if (conversationPanelIsOpen && conversationPanel) { + if (!conversationPanel.contains(event.target as Node)) { + setConversationPanelIsOpen(false); + } + } + }; React.useEffect(() => { // If the github token is invalid, open the account settings modal again @@ -43,6 +51,13 @@ export function Sidebar() { } }, [user.isError]); + React.useEffect(() => { + document.addEventListener("click", handleClick); + return () => { + document.removeEventListener("click", handleClick); + }; + }, [conversationPanelIsOpen]); + const handleAccountSettingsModalClose = () => { // If the user closes the modal without connecting to GitHub, // we need to log them out to clear the invalid token from the @@ -77,16 +92,17 @@ export function Sidebar() { /> )} setSettingsModalIsOpen(true)} /> - {MULTI_CONVO_UI_IS_ENABLED && ( + {MULTI_CONVERSATION_UI && ( )} @@ -97,6 +113,7 @@ export function Sidebar() { {conversationPanelIsOpen && (
{ + lastEventRef.current = null; + }, [conversationId]); + React.useEffect(() => { if (!conversationId) { throw new Error("No conversation ID provided"); diff --git a/frontend/src/hooks/query/get-conversation-permissions.ts b/frontend/src/hooks/query/get-conversation-permissions.ts index 83002a0ec206..721c2f38d79d 100644 --- a/frontend/src/hooks/query/get-conversation-permissions.ts +++ b/frontend/src/hooks/query/get-conversation-permissions.ts @@ -1,11 +1,11 @@ import { useQuery } from "@tanstack/react-query"; import OpenHands from "#/api/open-hands"; -import { MULTI_CONVO_UI_IS_ENABLED } from "#/utils/constants"; +import { MULTI_CONVERSATION_UI } from "#/utils/feature-flags"; export const useUserConversation = (cid: string | null) => useQuery({ queryKey: ["user", "conversation", cid], queryFn: () => OpenHands.getConversation(cid!), - enabled: MULTI_CONVO_UI_IS_ENABLED && !!cid, + enabled: MULTI_CONVERSATION_UI && !!cid, retry: false, }); diff --git a/frontend/src/mocks/handlers.ts b/frontend/src/mocks/handlers.ts index 0ad90741247d..37894e2da38e 100644 --- a/frontend/src/mocks/handlers.ts +++ b/frontend/src/mocks/handlers.ts @@ -17,26 +17,30 @@ const userPreferences = { const conversations: Conversation[] = [ { conversation_id: "1", - name: "My New Project", - repo: null, - lastUpdated: new Date().toISOString(), - state: "running", + title: "My New Project", + selected_repository: null, + last_updated_at: new Date().toISOString(), + status: "RUNNING", }, { conversation_id: "2", - name: "Repo Testing", - repo: "octocat/hello-world", + title: "Repo Testing", + selected_repository: "octocat/hello-world", // 2 days ago - lastUpdated: new Date(Date.now() - 2 * 24 * 60 * 60 * 1000).toISOString(), - state: "cold", + last_updated_at: new Date( + Date.now() - 2 * 24 * 60 * 60 * 1000, + ).toISOString(), + status: "STOPPED", }, { conversation_id: "3", - name: "Another Project", - repo: "octocat/earth", + title: "Another Project", + selected_repository: "octocat/earth", // 5 days ago - lastUpdated: new Date(Date.now() - 5 * 24 * 60 * 60 * 1000).toISOString(), - state: "finished", + last_updated_at: new Date( + Date.now() - 5 * 24 * 60 * 60 * 1000, + ).toISOString(), + status: "STOPPED", }, ]; @@ -182,8 +186,11 @@ export const handlers = [ http.get("/api/options/config", () => HttpResponse.json({ APP_MODE: "oss" })), - http.get("/api/conversations", async () => - HttpResponse.json(Array.from(CONVERSATIONS.values())), + http.get("/api/conversations?limit=9", async () => + HttpResponse.json({ + results: Array.from(CONVERSATIONS.values()), + next_page_id: null, + }), ), http.delete("/api/conversations/:conversationId", async ({ params }) => { @@ -197,7 +204,7 @@ export const handlers = [ return HttpResponse.json(null, { status: 404 }); }), - http.put( + http.patch( "/api/conversations/:conversationId", async ({ params, request }) => { const { conversationId } = params; @@ -207,10 +214,10 @@ export const handlers = [ if (conversation) { const body = await request.json(); - if (typeof body === "object" && body?.name) { + if (typeof body === "object" && body?.title) { CONVERSATIONS.set(conversationId, { ...conversation, - name: body.name, + title: body.title, }); return HttpResponse.json(null, { status: 200 }); } @@ -224,10 +231,10 @@ export const handlers = [ http.post("/api/conversations", () => { const conversation: Conversation = { conversation_id: (Math.random() * 100).toString(), - name: "New Conversation", - repo: null, - lastUpdated: new Date().toISOString(), - state: "warm", + title: "New Conversation", + selected_repository: null, + last_updated_at: new Date().toISOString(), + status: "RUNNING", }; CONVERSATIONS.set(conversation.conversation_id, conversation); diff --git a/frontend/src/routes/_oh.app/route.tsx b/frontend/src/routes/_oh.app/route.tsx index 73c50f5d8a7d..4fd5dec22eec 100644 --- a/frontend/src/routes/_oh.app/route.tsx +++ b/frontend/src/routes/_oh.app/route.tsx @@ -34,7 +34,7 @@ import { useUserConversation } from "#/hooks/query/get-conversation-permissions" import { CountBadge } from "#/components/layout/count-badge"; import { TerminalStatusLabel } from "#/components/features/terminal/terminal-status-label"; import { useSettings } from "#/hooks/query/use-settings"; -import { MULTI_CONVO_UI_IS_ENABLED } from "#/utils/constants"; +import { MULTI_CONVERSATION_UI } from "#/utils/feature-flags"; function AppContent() { const { gitHubToken } = useAuth(); @@ -73,7 +73,7 @@ function AppContent() { ); React.useEffect(() => { - if (MULTI_CONVO_UI_IS_ENABLED && isFetched && !conversation) { + if (MULTI_CONVERSATION_UI && isFetched && !conversation) { toast.error( "This conversation does not exist, or you do not have permission to access it.", ); diff --git a/frontend/src/utils/constants.ts b/frontend/src/utils/constants.ts deleted file mode 100644 index 1cc654131b6d..000000000000 --- a/frontend/src/utils/constants.ts +++ /dev/null @@ -1 +0,0 @@ -export const MULTI_CONVO_UI_IS_ENABLED = false; diff --git a/frontend/src/utils/feature-flags.ts b/frontend/src/utils/feature-flags.ts new file mode 100644 index 000000000000..8cdf711aadf5 --- /dev/null +++ b/frontend/src/utils/feature-flags.ts @@ -0,0 +1,15 @@ +function loadFeatureFlag( + flagName: string, + defaultValue: boolean = false, +): boolean { + try { + const stringValue = + localStorage.getItem(`FEATURE_${flagName}`) || defaultValue.toString(); + const value = !!JSON.parse(stringValue); + return value; + } catch (e) { + return defaultValue; + } +} + +export const MULTI_CONVERSATION_UI = loadFeatureFlag("MULTI_CONVERSATION_UI"); diff --git a/openhands/core/config/sandbox_config.py b/openhands/core/config/sandbox_config.py index a91c8b06a32a..d0785a640a34 100644 --- a/openhands/core/config/sandbox_config.py +++ b/openhands/core/config/sandbox_config.py @@ -58,7 +58,7 @@ class SandboxConfig: runtime_startup_env_vars: dict[str, str] = field(default_factory=dict) browsergym_eval_env: str | None = None platform: str | None = None - close_delay: int = 15 + close_delay: int = 900 remote_runtime_resource_factor: int = 1 enable_gpu: bool = False From 5626a22e420c9fe7e7e4e9d4f794246af7e7c233 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 6 Jan 2025 14:49:43 +0000 Subject: [PATCH 15/21] chore(deps-dev): bump @tanstack/eslint-plugin-query from 5.62.9 to 5.62.15 in /frontend in the eslint group (#6077) Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- frontend/package-lock.json | 9 ++++----- frontend/package.json | 2 +- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/frontend/package-lock.json b/frontend/package-lock.json index 70ab8e4e684d..aab2269cf542 100644 --- a/frontend/package-lock.json +++ b/frontend/package-lock.json @@ -52,7 +52,7 @@ "@playwright/test": "^1.49.1", "@react-router/dev": "^7.1.1", "@tailwindcss/typography": "^0.5.15", - "@tanstack/eslint-plugin-query": "^5.62.9", + "@tanstack/eslint-plugin-query": "^5.62.15", "@testing-library/jest-dom": "^6.6.1", "@testing-library/react": "^16.1.0", "@testing-library/user-event": "^14.5.2", @@ -5344,11 +5344,10 @@ } }, "node_modules/@tanstack/eslint-plugin-query": { - "version": "5.62.9", - "resolved": "https://registry.npmjs.org/@tanstack/eslint-plugin-query/-/eslint-plugin-query-5.62.9.tgz", - "integrity": "sha512-F3onhTcpBj7zQDo0NVtZwZQKRFx8BwpSabMJybl9no3+dFHUurvNMrH5M/6KNpkdDCf3zyHWadruZL6636B8Fw==", + "version": "5.62.15", + "resolved": "https://registry.npmjs.org/@tanstack/eslint-plugin-query/-/eslint-plugin-query-5.62.15.tgz", + "integrity": "sha512-24BHoF3LIzyptjrZXc1IpaISno+fhVD3zWWso/HPSB+ZVOyOXoiQSQc2K362T13JKJ07EInhHi1+KyNoRzCCfQ==", "dev": true, - "license": "MIT", "dependencies": { "@typescript-eslint/utils": "^8.18.1" }, diff --git a/frontend/package.json b/frontend/package.json index 1e3e4a04f587..9f519fa3524e 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -79,7 +79,7 @@ "@playwright/test": "^1.49.1", "@react-router/dev": "^7.1.1", "@tailwindcss/typography": "^0.5.15", - "@tanstack/eslint-plugin-query": "^5.62.9", + "@tanstack/eslint-plugin-query": "^5.62.15", "@testing-library/jest-dom": "^6.6.1", "@testing-library/react": "^16.1.0", "@testing-library/user-event": "^14.5.2", From e310f6b7769df56bdfd7ce314118893daa4968c0 Mon Sep 17 00:00:00 2001 From: tofarr Date: Mon, 6 Jan 2025 09:07:53 -0700 Subject: [PATCH 16/21] Feature - sort conversations by created at (#6079) --- openhands/server/routes/manage_conversations.py | 1 + .../storage/conversation/file_conversation_store.py | 6 +++--- openhands/storage/data_models/conversation_info.py | 3 ++- .../storage/data_models/conversation_metadata.py | 3 ++- tests/unit/test_conversation.py | 12 ++++++++---- 5 files changed, 16 insertions(+), 9 deletions(-) diff --git a/openhands/server/routes/manage_conversations.py b/openhands/server/routes/manage_conversations.py index 0d375229a50b..c613fa8d7422 100644 --- a/openhands/server/routes/manage_conversations.py +++ b/openhands/server/routes/manage_conversations.py @@ -189,6 +189,7 @@ async def _get_conversation_info( conversation_id=conversation.conversation_id, title=title, last_updated_at=conversation.last_updated_at, + created_at=conversation.created_at, selected_repository=conversation.selected_repository, status=ConversationStatus.RUNNING if is_running diff --git a/openhands/storage/conversation/file_conversation_store.py b/openhands/storage/conversation/file_conversation_store.py index 0e8983c01405..ee07aee9292c 100644 --- a/openhands/storage/conversation/file_conversation_store.py +++ b/openhands/storage/conversation/file_conversation_store.py @@ -96,7 +96,7 @@ async def get_instance(cls, config: AppConfig, token: str | None): def _sort_key(conversation: ConversationMetadata) -> str: - last_updated_at = conversation.last_updated_at - if last_updated_at: - return last_updated_at.isoformat() # YYYY-MM-DDTHH:MM:SS for sorting + created_at = conversation.created_at + if created_at: + return created_at.isoformat() # YYYY-MM-DDTHH:MM:SS for sorting return '' diff --git a/openhands/storage/data_models/conversation_info.py b/openhands/storage/data_models/conversation_info.py index cda7a2653bff..52464ec30f85 100644 --- a/openhands/storage/data_models/conversation_info.py +++ b/openhands/storage/data_models/conversation_info.py @@ -1,4 +1,4 @@ -from dataclasses import dataclass +from dataclasses import dataclass, field from datetime import datetime from openhands.storage.data_models.conversation_status import ConversationStatus @@ -13,3 +13,4 @@ class ConversationInfo: last_updated_at: datetime | None = None status: ConversationStatus = ConversationStatus.STOPPED selected_repository: str | None = None + created_at: datetime = field(default_factory=datetime.now) diff --git a/openhands/storage/data_models/conversation_metadata.py b/openhands/storage/data_models/conversation_metadata.py index 21c84be99eba..7761e077f976 100644 --- a/openhands/storage/data_models/conversation_metadata.py +++ b/openhands/storage/data_models/conversation_metadata.py @@ -1,4 +1,4 @@ -from dataclasses import dataclass +from dataclasses import dataclass, field from datetime import datetime @@ -9,3 +9,4 @@ class ConversationMetadata: selected_repository: str | None title: str | None = None last_updated_at: datetime | None = None + created_at: datetime = field(default_factory=datetime.now) diff --git a/tests/unit/test_conversation.py b/tests/unit/test_conversation.py index 709cb4cae65f..938193d108f0 100644 --- a/tests/unit/test_conversation.py +++ b/tests/unit/test_conversation.py @@ -29,7 +29,8 @@ def _patch_store(): 'selected_repository': 'foobar', 'conversation_id': 'some_conversation_id', 'github_user_id': 'github_user', - 'last_updated_at': '2025-01-01T00:00:00', + 'created_at': '2025-01-01T00:00:00', + 'last_updated_at': '2025-01-01T00:01:00', } ), ) @@ -55,7 +56,8 @@ async def test_search_conversations(): ConversationInfo( conversation_id='some_conversation_id', title='Some Conversation', - last_updated_at=datetime.fromisoformat('2025-01-01T00:00:00'), + created_at=datetime.fromisoformat('2025-01-01T00:00:00'), + last_updated_at=datetime.fromisoformat('2025-01-01T00:01:00'), status=ConversationStatus.STOPPED, selected_repository='foobar', ) @@ -73,7 +75,8 @@ async def test_get_conversation(): expected = ConversationInfo( conversation_id='some_conversation_id', title='Some Conversation', - last_updated_at=datetime.fromisoformat('2025-01-01T00:00:00'), + created_at=datetime.fromisoformat('2025-01-01T00:00:00'), + last_updated_at=datetime.fromisoformat('2025-01-01T00:01:00'), status=ConversationStatus.STOPPED, selected_repository='foobar', ) @@ -105,7 +108,8 @@ async def test_update_conversation(): expected = ConversationInfo( conversation_id='some_conversation_id', title='New Title', - last_updated_at=datetime.fromisoformat('2025-01-01T00:00:00'), + created_at=datetime.fromisoformat('2025-01-01T00:00:00'), + last_updated_at=datetime.fromisoformat('2025-01-01T00:01:00'), status=ConversationStatus.STOPPED, selected_repository='foobar', ) From 17d722f3b3d938fe7e2eb6198088a4ba858f015b Mon Sep 17 00:00:00 2001 From: Dmitry Kozlov Date: Mon, 6 Jan 2025 09:31:19 -0800 Subject: [PATCH 17/21] Update README.md (#6076) Co-authored-by: Xingyao Wang --- evaluation/benchmarks/swe_bench/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/evaluation/benchmarks/swe_bench/README.md b/evaluation/benchmarks/swe_bench/README.md index 08ec3427e6c5..4758a6616c81 100644 --- a/evaluation/benchmarks/swe_bench/README.md +++ b/evaluation/benchmarks/swe_bench/README.md @@ -204,7 +204,7 @@ Then, in a separate Python environment with `streamlit` library, you can run the ```bash # Make sure you are inside the cloned `evaluation` repo conda activate streamlit # if you follow the optional conda env setup above -streamlit app.py --server.port 8501 --server.address 0.0.0.0 +streamlit run app.py --server.port 8501 --server.address 0.0.0.0 ``` Then you can access the SWE-Bench trajectory visualizer at `localhost:8501`. From 09734467c06a9fdb305c7398d48bfe136b5fac11 Mon Sep 17 00:00:00 2001 From: "sp.wack" <83104063+amanape@users.noreply.github.com> Date: Mon, 6 Jan 2025 22:03:44 +0400 Subject: [PATCH 18/21] fix(frontend): Only render loading indicator if events are messages (#6082) --- frontend/src/context/ws-client-provider.tsx | 31 +++++++++++++++++---- 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/frontend/src/context/ws-client-provider.tsx b/frontend/src/context/ws-client-provider.tsx index 36d0ab74c1c6..12e354cee0ff 100644 --- a/frontend/src/context/ws-client-provider.tsx +++ b/frontend/src/context/ws-client-provider.tsx @@ -5,9 +5,12 @@ import EventLogger from "#/utils/event-logger"; import { handleAssistantMessage } from "#/services/actions"; import { useRate } from "#/hooks/use-rate"; import { OpenHandsParsedEvent } from "#/types/core"; -import { AgentStateChangeObservation } from "#/types/core/observations"; +import { + AssistantMessageAction, + UserMessageAction, +} from "#/types/core/actions"; -const isOpenHandsMessage = (event: unknown): event is OpenHandsParsedEvent => +const isOpenHandsEvent = (event: unknown): event is OpenHandsParsedEvent => typeof event === "object" && event !== null && "id" in event && @@ -15,10 +18,26 @@ const isOpenHandsMessage = (event: unknown): event is OpenHandsParsedEvent => "message" in event && "timestamp" in event; -const isAgentStateChangeObservation = ( +const isUserMessage = ( event: OpenHandsParsedEvent, -): event is AgentStateChangeObservation => - "observation" in event && event.observation === "agent_state_changed"; +): event is UserMessageAction => + "source" in event && + "type" in event && + event.source === "user" && + event.type === "message"; + +const isAssistantMessage = ( + event: OpenHandsParsedEvent, +): event is AssistantMessageAction => + "source" in event && + "type" in event && + event.source === "agent" && + event.type === "message"; + +const isMessageAction = ( + event: OpenHandsParsedEvent, +): event is UserMessageAction | AssistantMessageAction => + isUserMessage(event) || isAssistantMessage(event); export enum WsClientProviderStatus { CONNECTED, @@ -74,7 +93,7 @@ export function WsClientProvider({ } function handleMessage(event: Record) { - if (isOpenHandsMessage(event) && !isAgentStateChangeObservation(event)) { + if (isOpenHandsEvent(event) && isMessageAction(event)) { messageRateHandler.record(new Date().getTime()); } setEvents((prevEvents) => [...prevEvents, event]); From 343b86429ef28fe2c5b4ecd08bfc34893678bcda Mon Sep 17 00:00:00 2001 From: Robert Brennan Date: Mon, 6 Jan 2025 14:22:52 -0500 Subject: [PATCH 19/21] Retrieve GitHub IDs more efficiently (#6074) Co-authored-by: openhands --- frontend/src/context/ws-client-provider.tsx | 9 +--- frontend/src/routes/_oh.app/route.tsx | 2 +- openhands/server/auth.py | 5 ++ openhands/server/listen_socket.py | 25 +++++---- .../server/routes/manage_conversations.py | 52 +++++++++---------- openhands/server/routes/settings.py | 13 ++--- .../conversation/conversation_store.py | 4 +- .../conversation/file_conversation_store.py | 4 +- .../data_models/conversation_metadata.py | 2 +- .../storage/settings/file_settings_store.py | 2 +- openhands/storage/settings/settings_store.py | 2 +- tests/unit/test_conversation.py | 2 +- 12 files changed, 58 insertions(+), 64 deletions(-) diff --git a/frontend/src/context/ws-client-provider.tsx b/frontend/src/context/ws-client-provider.tsx index 12e354cee0ff..177a11e59032 100644 --- a/frontend/src/context/ws-client-provider.tsx +++ b/frontend/src/context/ws-client-provider.tsx @@ -62,16 +62,13 @@ const WsClientContext = React.createContext({ interface WsClientProviderProps { conversationId: string; - ghToken: string | null; } export function WsClientProvider({ - ghToken, conversationId, children, }: React.PropsWithChildren) { const sioRef = React.useRef(null); - const ghTokenRef = React.useRef(ghToken); const [status, setStatus] = React.useState( WsClientProviderStatus.DISCONNECTED, ); @@ -141,9 +138,6 @@ export function WsClientProvider({ sio = io(baseUrl, { transports: ["websocket"], - auth: { - github_token: ghToken || undefined, - }, query, }); sio.on("connect", handleConnect); @@ -153,7 +147,6 @@ export function WsClientProvider({ sio.on("disconnect", handleDisconnect); sioRef.current = sio; - ghTokenRef.current = ghToken; return () => { sio.off("connect", handleConnect); @@ -162,7 +155,7 @@ export function WsClientProvider({ sio.off("connect_failed", handleError); sio.off("disconnect", handleDisconnect); }; - }, [ghToken, conversationId]); + }, [conversationId]); React.useEffect( () => () => { diff --git a/frontend/src/routes/_oh.app/route.tsx b/frontend/src/routes/_oh.app/route.tsx index 4fd5dec22eec..ab3384f951ec 100644 --- a/frontend/src/routes/_oh.app/route.tsx +++ b/frontend/src/routes/_oh.app/route.tsx @@ -175,7 +175,7 @@ function AppContent() { } return ( - +
{renderMain()}
diff --git a/openhands/server/auth.py b/openhands/server/auth.py index 4b3fccdda7f1..b695cff89eab 100644 --- a/openhands/server/auth.py +++ b/openhands/server/auth.py @@ -1,9 +1,14 @@ import jwt +from fastapi import Request from jwt.exceptions import InvalidTokenError from openhands.core.logger import openhands_logger as logger +def get_user_id(request: Request) -> int: + return getattr(request.state, 'github_user_id', 0) + + def get_sid_from_token(token: str, jwt_secret: str) -> str: """Retrieves the session id from a JWT token. diff --git a/openhands/server/listen_socket.py b/openhands/server/listen_socket.py index 4bd7b8071960..4bb81aad6676 100644 --- a/openhands/server/listen_socket.py +++ b/openhands/server/listen_socket.py @@ -1,6 +1,6 @@ from urllib.parse import parse_qs -from github import Github +import jwt from socketio.exceptions import ConnectionRefusedError from openhands.core.logger import openhands_logger as logger @@ -18,7 +18,6 @@ from openhands.server.session.manager import ConversationDoesNotExistError from openhands.server.shared import config, openhands_config, session_manager, sio from openhands.server.types import AppMode -from openhands.utils.async_utils import call_sync_from_async @sio.event @@ -31,20 +30,20 @@ async def connect(connection_id: str, environ, auth): logger.error('No conversation_id in query params') raise ConnectionRefusedError('No conversation_id in query params') - github_token = '' + user_id = -1 if openhands_config.app_mode != AppMode.OSS: - user_id = '' - if auth and 'github_token' in auth: - github_token = auth['github_token'] - with Github(github_token) as g: - gh_user = await call_sync_from_async(g.get_user) - user_id = gh_user.id + cookies_str = environ.get('HTTP_COOKIE', '') + cookies = dict(cookie.split('=', 1) for cookie in cookies_str.split('; ')) + signed_token = cookies.get('github_auth', '') + if not signed_token: + logger.error('No github_auth cookie') + raise ConnectionRefusedError('No github_auth cookie') + decoded = jwt.decode(signed_token, config.jwt_secret, algorithms=['HS256']) + user_id = decoded['github_user_id'] logger.info(f'User {user_id} is connecting to conversation {conversation_id}') - conversation_store = await ConversationStoreImpl.get_instance( - config, github_token - ) + conversation_store = await ConversationStoreImpl.get_instance(config, user_id) metadata = await conversation_store.get_metadata(conversation_id) if metadata.github_user_id != user_id: logger.error( @@ -54,7 +53,7 @@ async def connect(connection_id: str, environ, auth): f'User {user_id} is not allowed to join conversation {conversation_id}' ) - settings_store = await SettingsStoreImpl.get_instance(config, github_token) + settings_store = await SettingsStoreImpl.get_instance(config, user_id) settings = await settings_store.load() if not settings: diff --git a/openhands/server/routes/manage_conversations.py b/openhands/server/routes/manage_conversations.py index c613fa8d7422..a48c7286879a 100644 --- a/openhands/server/routes/manage_conversations.py +++ b/openhands/server/routes/manage_conversations.py @@ -4,11 +4,11 @@ from fastapi import APIRouter, Body, Request from fastapi.responses import JSONResponse -from github import Github from pydantic import BaseModel from openhands.core.logger import openhands_logger as logger from openhands.events.stream import EventStreamSubscriber +from openhands.server.auth import get_user_id from openhands.server.routes.settings import ConversationStoreImpl, SettingsStoreImpl from openhands.server.session.conversation_init_data import ConversationInitData from openhands.server.shared import config, session_manager @@ -21,7 +21,6 @@ from openhands.utils.async_utils import ( GENERAL_TIMEOUT, call_async_from_sync, - call_sync_from_async, wait_all, ) @@ -43,10 +42,9 @@ async def new_conversation(request: Request, data: InitSessionRequest): using the returned conversation ID """ logger.info('Initializing new conversation') - github_token = data.github_token or '' logger.info('Loading settings') - settings_store = await SettingsStoreImpl.get_instance(config, github_token) + settings_store = await SettingsStoreImpl.get_instance(config, get_user_id(request)) settings = await settings_store.load() logger.info('Settings loaded') @@ -54,11 +52,14 @@ async def new_conversation(request: Request, data: InitSessionRequest): if settings: session_init_args = {**settings.__dict__, **session_init_args} + github_token = getattr(request.state, 'github_token', '') session_init_args['github_token'] = github_token session_init_args['selected_repository'] = data.selected_repository conversation_init_data = ConversationInitData(**session_init_args) logger.info('Loading conversation store') - conversation_store = await ConversationStoreImpl.get_instance(config, github_token) + conversation_store = await ConversationStoreImpl.get_instance( + config, get_user_id(request) + ) logger.info('Conversation store loaded') conversation_id = uuid.uuid4().hex @@ -67,18 +68,11 @@ async def new_conversation(request: Request, data: InitSessionRequest): conversation_id = uuid.uuid4().hex logger.info(f'New conversation ID: {conversation_id}') - user_id = '' - if data.github_token: - logger.info('Fetching Github user ID') - with Github(data.github_token) as g: - gh_user = await call_sync_from_async(g.get_user) - user_id = gh_user.id - logger.info(f'Saving metadata for conversation {conversation_id}') await conversation_store.save_metadata( ConversationMetadata( conversation_id=conversation_id, - github_user_id=user_id, + github_user_id=get_user_id(request), selected_repository=data.selected_repository, ) ) @@ -90,9 +84,7 @@ async def new_conversation(request: Request, data: InitSessionRequest): try: event_stream.subscribe( EventStreamSubscriber.SERVER, - _create_conversation_update_callback( - data.github_token or '', conversation_id - ), + _create_conversation_update_callback(get_user_id(request), conversation_id), UPDATED_AT_CALLBACK_ID, ) except ValueError: @@ -107,8 +99,9 @@ async def search_conversations( page_id: str | None = None, limit: int = 20, ) -> ConversationInfoResultSet: - github_token = getattr(request.state, 'github_token', '') or '' - conversation_store = await ConversationStoreImpl.get_instance(config, github_token) + conversation_store = await ConversationStoreImpl.get_instance( + config, get_user_id(request) + ) conversation_metadata_result_set = await conversation_store.search(page_id, limit) conversation_ids = set( conversation.conversation_id @@ -134,8 +127,9 @@ async def search_conversations( async def get_conversation( conversation_id: str, request: Request ) -> ConversationInfo | None: - github_token = getattr(request.state, 'github_token', '') or '' - conversation_store = await ConversationStoreImpl.get_instance(config, github_token) + conversation_store = await ConversationStoreImpl.get_instance( + config, get_user_id(request) + ) try: metadata = await conversation_store.get_metadata(conversation_id) is_running = await session_manager.is_agent_loop_running(conversation_id) @@ -149,8 +143,9 @@ async def get_conversation( async def update_conversation( request: Request, conversation_id: str, title: str = Body(embed=True) ) -> bool: - github_token = getattr(request.state, 'github_token', '') or '' - conversation_store = await ConversationStoreImpl.get_instance(config, github_token) + conversation_store = await ConversationStoreImpl.get_instance( + config, get_user_id(request) + ) metadata = await conversation_store.get_metadata(conversation_id) if not metadata: return False @@ -164,8 +159,9 @@ async def delete_conversation( conversation_id: str, request: Request, ) -> bool: - github_token = getattr(request.state, 'github_token', '') or '' - conversation_store = await ConversationStoreImpl.get_instance(config, github_token) + conversation_store = await ConversationStoreImpl.get_instance( + config, get_user_id(request) + ) try: await conversation_store.get_metadata(conversation_id) except FileNotFoundError: @@ -205,21 +201,21 @@ async def _get_conversation_info( def _create_conversation_update_callback( - github_token: str, conversation_id: str + user_id: int, conversation_id: str ) -> Callable: def callback(*args, **kwargs): call_async_from_sync( _update_timestamp_for_conversation, GENERAL_TIMEOUT, - github_token, + user_id, conversation_id, ) return callback -async def _update_timestamp_for_conversation(github_token: str, conversation_id: str): - conversation_store = await ConversationStoreImpl.get_instance(config, github_token) +async def _update_timestamp_for_conversation(user_id: int, conversation_id: str): + conversation_store = await ConversationStoreImpl.get_instance(config, user_id) conversation = await conversation_store.get_metadata(conversation_id) conversation.last_updated_at = datetime.now() await conversation_store.save_metadata(conversation) diff --git a/openhands/server/routes/settings.py b/openhands/server/routes/settings.py index 4fcdb42f02ba..1fa50aadb486 100644 --- a/openhands/server/routes/settings.py +++ b/openhands/server/routes/settings.py @@ -2,6 +2,7 @@ from fastapi.responses import JSONResponse from openhands.core.logger import openhands_logger as logger +from openhands.server.auth import get_user_id from openhands.server.settings import Settings from openhands.server.shared import config, openhands_config from openhands.storage.conversation.conversation_store import ConversationStore @@ -19,9 +20,10 @@ @app.get('/settings') async def load_settings(request: Request) -> Settings | None: - github_token = getattr(request.state, 'github_token', '') or '' try: - settings_store = await SettingsStoreImpl.get_instance(config, github_token) + settings_store = await SettingsStoreImpl.get_instance( + config, get_user_id(request) + ) settings = await settings_store.load() if not settings: return JSONResponse( @@ -45,11 +47,10 @@ async def store_settings( request: Request, settings: Settings, ) -> JSONResponse: - github_token = '' - if hasattr(request.state, 'github_token'): - github_token = request.state.github_token try: - settings_store = await SettingsStoreImpl.get_instance(config, github_token) + settings_store = await SettingsStoreImpl.get_instance( + config, get_user_id(request) + ) existing_settings = await settings_store.load() if existing_settings: diff --git a/openhands/storage/conversation/conversation_store.py b/openhands/storage/conversation/conversation_store.py index 2a09322574bf..1f0b41ea6c87 100644 --- a/openhands/storage/conversation/conversation_store.py +++ b/openhands/storage/conversation/conversation_store.py @@ -40,7 +40,5 @@ async def search( @classmethod @abstractmethod - async def get_instance( - cls, config: AppConfig, token: str | None - ) -> ConversationStore: + async def get_instance(cls, config: AppConfig, user_id: int) -> ConversationStore: """Get a store for the user represented by the token given""" diff --git a/openhands/storage/conversation/file_conversation_store.py b/openhands/storage/conversation/file_conversation_store.py index ee07aee9292c..622b72f4bbcf 100644 --- a/openhands/storage/conversation/file_conversation_store.py +++ b/openhands/storage/conversation/file_conversation_store.py @@ -90,7 +90,9 @@ def get_conversation_metadata_filename(self, conversation_id: str) -> str: return get_conversation_metadata_filename(conversation_id) @classmethod - async def get_instance(cls, config: AppConfig, token: str | None): + async def get_instance( + cls, config: AppConfig, user_id: int + ) -> FileConversationStore: file_store = get_file_store(config.file_store, config.file_store_path) return FileConversationStore(file_store) diff --git a/openhands/storage/data_models/conversation_metadata.py b/openhands/storage/data_models/conversation_metadata.py index 7761e077f976..e75bbf21f8d5 100644 --- a/openhands/storage/data_models/conversation_metadata.py +++ b/openhands/storage/data_models/conversation_metadata.py @@ -5,7 +5,7 @@ @dataclass class ConversationMetadata: conversation_id: str - github_user_id: int | str + github_user_id: int selected_repository: str | None title: str | None = None last_updated_at: datetime | None = None diff --git a/openhands/storage/settings/file_settings_store.py b/openhands/storage/settings/file_settings_store.py index 413376759c8d..c8703b304c11 100644 --- a/openhands/storage/settings/file_settings_store.py +++ b/openhands/storage/settings/file_settings_store.py @@ -30,6 +30,6 @@ async def store(self, settings: Settings): await call_sync_from_async(self.file_store.write, self.path, json_str) @classmethod - async def get_instance(cls, config: AppConfig, token: str | None): + async def get_instance(cls, config: AppConfig, user_id: int) -> FileSettingsStore: file_store = get_file_store(config.file_store, config.file_store_path) return FileSettingsStore(file_store) diff --git a/openhands/storage/settings/settings_store.py b/openhands/storage/settings/settings_store.py index 6e369b8f9739..a371720600ac 100644 --- a/openhands/storage/settings/settings_store.py +++ b/openhands/storage/settings/settings_store.py @@ -21,5 +21,5 @@ async def store(self, settings: Settings): @classmethod @abstractmethod - async def get_instance(cls, config: AppConfig, token: str | None) -> SettingsStore: + async def get_instance(cls, config: AppConfig, user_id: int) -> SettingsStore: """Get a store for the user represented by the token given""" diff --git a/tests/unit/test_conversation.py b/tests/unit/test_conversation.py index 938193d108f0..91731a601d6d 100644 --- a/tests/unit/test_conversation.py +++ b/tests/unit/test_conversation.py @@ -28,7 +28,7 @@ def _patch_store(): 'title': 'Some Conversation', 'selected_repository': 'foobar', 'conversation_id': 'some_conversation_id', - 'github_user_id': 'github_user', + 'github_user_id': 12345, 'created_at': '2025-01-01T00:00:00', 'last_updated_at': '2025-01-01T00:01:00', } From cebd391b7a64712e000e85392822e3f58f76b058 Mon Sep 17 00:00:00 2001 From: Xingyao Wang Date: Mon, 6 Jan 2025 15:45:59 -0500 Subject: [PATCH 20/21] fix: better handle bashlex error (#6090) --- openhands/runtime/utils/bash.py | 15 ++++++++++----- tests/runtime/test_bash.py | 14 ++++++++++++++ 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/openhands/runtime/utils/bash.py b/openhands/runtime/utils/bash.py index 84c5d0265957..70a24e2189f7 100644 --- a/openhands/runtime/utils/bash.py +++ b/openhands/runtime/utils/bash.py @@ -1,6 +1,7 @@ import os import re import time +import traceback import uuid from enum import Enum @@ -23,11 +24,11 @@ def split_bash_commands(commands): return [''] try: parsed = bashlex.parse(commands) - except bashlex.errors.ParsingError as e: + except (bashlex.errors.ParsingError, NotImplementedError): logger.debug( f'Failed to parse bash commands\n' f'[input]: {commands}\n' - f'[warning]: {e}\n' + f'[warning]: {traceback.format_exc()}\n' f'The original command will be returned as is.' ) # If parsing fails, return the original commands @@ -143,9 +144,13 @@ def visit_node(node): remaining = command[last_pos:] parts.append(remaining) return ''.join(parts) - except bashlex.errors.ParsingError: - # Fallback if parsing fails - logger.warning(f'Failed to parse command: {command}') + except (bashlex.errors.ParsingError, NotImplementedError): + logger.debug( + f'Failed to parse bash commands for special characters escape\n' + f'[input]: {command}\n' + f'[warning]: {traceback.format_exc()}\n' + f'The original command will be returned as is.' + ) return command diff --git a/tests/runtime/test_bash.py b/tests/runtime/test_bash.py index b97cdbc9266b..75f9085815fa 100644 --- a/tests/runtime/test_bash.py +++ b/tests/runtime/test_bash.py @@ -153,6 +153,20 @@ def test_multiple_multiline_commands(temp_dir, runtime_cls, run_as_openhands): _close_test_runtime(runtime) +def test_complex_commands(temp_dir, runtime_cls): + cmd = """count=0; tries=0; while [ $count -lt 3 ]; do result=$(echo "Heads"); tries=$((tries+1)); echo "Flip $tries: $result"; if [ "$result" = "Heads" ]; then count=$((count+1)); else count=0; fi; done; echo "Got 3 heads in a row after $tries flips!";""" + + runtime = _load_runtime(temp_dir, runtime_cls) + try: + obs = _run_cmd_action(runtime, cmd) + logger.info(obs, extra={'msg_type': 'OBSERVATION'}) + assert obs.exit_code == 0, 'The exit code should be 0.' + assert 'Got 3 heads in a row after 3 flips!' in obs.content + + finally: + _close_test_runtime(runtime) + + def test_no_ps2_in_output(temp_dir, runtime_cls, run_as_openhands): """Test that the PS2 sign is not added to the output of a multiline command.""" runtime = _load_runtime(temp_dir, runtime_cls, run_as_openhands) From 9515ac5e6231f88f907bfedfb8fcb567d9cc67f7 Mon Sep 17 00:00:00 2001 From: tofarr Date: Mon, 6 Jan 2025 14:26:48 -0700 Subject: [PATCH 21/21] Feat - browser client can now close sessions. (#6088) --- .../server/routes/manage_conversations.py | 2 +- openhands/server/session/manager.py | 22 ++++++++++++++++--- tests/unit/test_manager.py | 2 +- 3 files changed, 21 insertions(+), 5 deletions(-) diff --git a/openhands/server/routes/manage_conversations.py b/openhands/server/routes/manage_conversations.py index a48c7286879a..235b5801f24d 100644 --- a/openhands/server/routes/manage_conversations.py +++ b/openhands/server/routes/manage_conversations.py @@ -168,7 +168,7 @@ async def delete_conversation( return False is_running = await session_manager.is_agent_loop_running(conversation_id) if is_running: - return False + await session_manager.close_session(conversation_id) await conversation_store.delete_metadata(conversation_id) return True diff --git a/openhands/server/session/manager.py b/openhands/server/session/manager.py index 60b5bd2675af..cc08d87466e1 100644 --- a/openhands/server/session/manager.py +++ b/openhands/server/session/manager.py @@ -156,6 +156,10 @@ async def _process_message(self, message: dict): flag = self._has_remote_connections_flags.get(sid) if flag: flag.set() + elif message_type == 'close_session': + sid = data['sid'] + if sid in self._local_agent_loops_by_sid: + await self._on_close_session(sid) elif message_type == 'session_closing': # Session closing event - We only get this in the event of graceful shutdown, # which can't be guaranteed - nodes can simply vanish unexpectedly! @@ -419,7 +423,7 @@ async def disconnect_from_session(self, connection_id: str): if should_continue(): asyncio.create_task(self._cleanup_session_later(sid)) else: - await self._close_session(sid) + await self._on_close_session(sid) async def _cleanup_session_later(self, sid: str): # Once there have been no connections to a session for a reasonable period, we close it @@ -451,10 +455,22 @@ async def _cleanup_session(self, sid: str) -> bool: json.dumps({'sid': sid, 'message_type': 'session_closing'}), ) - await self._close_session(sid) + await self._on_close_session(sid) return True - async def _close_session(self, sid: str): + async def close_session(self, sid: str): + session = self._local_agent_loops_by_sid.get(sid) + if session: + await self._on_close_session(sid) + + redis_client = self._get_redis_client() + if redis_client: + await redis_client.publish( + 'oh_event', + json.dumps({'sid': sid, 'message_type': 'close_session'}), + ) + + async def _on_close_session(self, sid: str): logger.info(f'_close_session:{sid}') # Clear up local variables diff --git a/tests/unit/test_manager.py b/tests/unit/test_manager.py index c2a61104a864..144f79f9f491 100644 --- a/tests/unit/test_manager.py +++ b/tests/unit/test_manager.py @@ -286,7 +286,7 @@ async def test_cleanup_session_connections(): } ) - await session_manager._close_session('session1') + await session_manager._on_close_session('session1') remaining_connections = session_manager.local_connection_id_to_session_id assert 'conn1' not in remaining_connections