From 8a5c6d3befe11c48293485c1be75fd47e8301198 Mon Sep 17 00:00:00 2001 From: Graham Neubig Date: Sat, 26 Apr 2025 12:27:54 -0400 Subject: [PATCH] Fix long setup scripts failing (#8101) Co-authored-by: openhands --- openhands/core/setup.py | 5 +- openhands/events/action/commands.py | 4 +- openhands/runtime/base.py | 32 +++++---- .../action_execution_client.py | 2 + tests/runtime/test_setup.py | 69 +++++++++++++++++++ 5 files changed, 92 insertions(+), 20 deletions(-) create mode 100644 tests/runtime/test_setup.py diff --git a/openhands/core/setup.py b/openhands/core/setup.py index 219f9dd9f3..2ebd2ba524 100644 --- a/openhands/core/setup.py +++ b/openhands/core/setup.py @@ -100,9 +100,8 @@ def initialize_repository_for_runtime( The repository directory path if a repository was cloned, None otherwise. """ # clone selected repository if provided - github_token = ( - SecretStr(os.environ.get('GITHUB_TOKEN')) if not github_token else github_token - ) + if github_token is None and 'GITHUB_TOKEN' in os.environ: + github_token = SecretStr(os.environ['GITHUB_TOKEN']) secret_store = ( SecretStore( diff --git a/openhands/events/action/commands.py b/openhands/events/action/commands.py index 395531ae49..df67d8b9ef 100644 --- a/openhands/events/action/commands.py +++ b/openhands/events/action/commands.py @@ -16,11 +16,9 @@ class CmdRunAction(Action): ) is_input: bool = False # if True, the command is an input to the running process thought: str = '' - blocking: bool = False + blocking: bool = False # if True, the command will be run in a blocking manner, but a timeout must be set through _set_hard_timeout is_static: bool = False # if True, runs the command in a separate process cwd: str | None = None # current working directory, only used if is_static is True - # If blocking is True, the command will be run in a blocking manner. - # e.g., it will NOT return early due to soft timeout. hidden: bool = False action: str = ActionType.RUN runnable: ClassVar[bool] = True diff --git a/openhands/runtime/base.py b/openhands/runtime/base.py index fb065647bd..0168e39280 100644 --- a/openhands/runtime/base.py +++ b/openhands/runtime/base.py @@ -97,7 +97,7 @@ class Runtime(FileEditRuntimeMixin): config: AppConfig initial_env_vars: dict[str, str] attach_to_existing: bool - status_callback: Callable | None + status_callback: Callable[[str, str, str], None] | None def __init__( self, @@ -106,7 +106,7 @@ class Runtime(FileEditRuntimeMixin): sid: str = 'default', plugins: list[PluginRequirement] | None = None, env_vars: dict[str, str] | None = None, - status_callback: Callable | None = None, + status_callback: Callable[[str, str, str], None] | None = None, attach_to_existing: bool = False, headless_mode: bool = False, user_id: str | None = None, @@ -344,12 +344,6 @@ class Runtime(FileEditRuntimeMixin): else selected_repository.git_provider ) - if not git_provider_tokens: - raise RuntimeError('Need git provider tokens to clone repo') - git_token = git_provider_tokens[chosen_provider].token - if not git_token: - raise RuntimeError('Need a valid git token to clone repo') - domain = provider_domains[chosen_provider] repository = ( selected_repository @@ -357,12 +351,18 @@ class Runtime(FileEditRuntimeMixin): else selected_repository.full_name ) - if chosen_provider == ProviderType.GITLAB: - remote_repo_url = f'https://oauth2:{git_token.get_secret_value()}@{domain}/{repository}.git' + # Try to use token if available, otherwise use public URL + if git_provider_tokens and chosen_provider in git_provider_tokens: + git_token = git_provider_tokens[chosen_provider].token + if git_token: + if chosen_provider == ProviderType.GITLAB: + remote_repo_url = f'https://oauth2:{git_token.get_secret_value()}@{domain}/{repository}.git' + else: + remote_repo_url = f'https://{git_token.get_secret_value()}@{domain}/{repository}.git' + else: + remote_repo_url = f'https://{domain}/{repository}.git' else: - remote_repo_url = ( - f'https://{git_token.get_secret_value()}@{domain}/{repository}.git' - ) + remote_repo_url = f'https://{domain}/{repository}.git' if not remote_repo_url: raise ValueError('Missing either Git token or valid repository') @@ -409,7 +409,11 @@ class Runtime(FileEditRuntimeMixin): 'info', 'STATUS$SETTING_UP_WORKSPACE', 'Setting up workspace...' ) - action = CmdRunAction(f'chmod +x {setup_script} && source {setup_script}') + # setup scripts time out after 10 minutes + action = CmdRunAction( + f'chmod +x {setup_script} && source {setup_script}', blocking=True + ) + action.set_hard_timeout(600) obs = self.run_action(action) if isinstance(obs, CmdOutputObservation) and obs.exit_code != 0: self.log('error', f'Setup script failed: {obs.content}') diff --git a/openhands/runtime/impl/action_execution/action_execution_client.py b/openhands/runtime/impl/action_execution/action_execution_client.py index 60f18dcd7c..f780ab63c5 100644 --- a/openhands/runtime/impl/action_execution/action_execution_client.py +++ b/openhands/runtime/impl/action_execution/action_execution_client.py @@ -253,6 +253,8 @@ class ActionExecutionClient(Runtime): # set timeout to default if not set if action.timeout is None: + if isinstance(action, CmdRunAction) and action.blocking: + raise RuntimeError('Blocking command with no timeout set') # We don't block the command if this is a default timeout action action.set_hard_timeout(self.config.sandbox.timeout, blocking=False) diff --git a/tests/runtime/test_setup.py b/tests/runtime/test_setup.py new file mode 100644 index 0000000000..6639ceeb09 --- /dev/null +++ b/tests/runtime/test_setup.py @@ -0,0 +1,69 @@ +"""Tests for the setup script.""" + +from conftest import ( + _load_runtime, +) + +from openhands.core.setup import initialize_repository_for_runtime +from openhands.events.action import FileReadAction, FileWriteAction +from openhands.events.observation import FileReadObservation, FileWriteObservation + + +def test_initialize_repository_for_runtime(temp_dir, runtime_cls, run_as_openhands): + """Test that the initialize_repository_for_runtime function works.""" + runtime, config = _load_runtime(temp_dir, runtime_cls, run_as_openhands) + repository_dir = initialize_repository_for_runtime( + runtime, 'https://github.com/All-Hands-AI/OpenHands' + ) + assert repository_dir is not None + assert repository_dir == 'OpenHands' + + +def test_maybe_run_setup_script(temp_dir, runtime_cls, run_as_openhands): + """Test that setup script is executed when it exists.""" + runtime, config = _load_runtime(temp_dir, runtime_cls, run_as_openhands) + + setup_script = '.openhands/setup.sh' + write_obs = runtime.write( + FileWriteAction( + path=setup_script, content="#!/bin/bash\necho 'Hello World' >> README.md\n" + ) + ) + assert isinstance(write_obs, FileWriteObservation) + + # Run setup script + runtime.maybe_run_setup_script() + + # Verify script was executed by checking output + read_obs = runtime.read(FileReadAction(path='README.md')) + assert isinstance(read_obs, FileReadObservation) + assert read_obs.content == 'Hello World\n' + + +def test_maybe_run_setup_script_with_long_timeout( + temp_dir, runtime_cls, run_as_openhands +): + """Test that setup script is executed when it exists.""" + runtime, config = _load_runtime( + temp_dir, + runtime_cls, + run_as_openhands, + runtime_startup_env_vars={'NO_CHANGE_TIMEOUT_SECONDS': '1'}, + ) + + setup_script = '.openhands/setup.sh' + write_obs = runtime.write( + FileWriteAction( + path=setup_script, + content="#!/bin/bash\nsleep 3 && echo 'Hello World' >> README.md\n", + ) + ) + assert isinstance(write_obs, FileWriteObservation) + + # Run setup script + runtime.maybe_run_setup_script() + + # Verify script was executed by checking output + read_obs = runtime.read(FileReadAction(path='README.md')) + assert isinstance(read_obs, FileReadObservation) + assert read_obs.content == 'Hello World\n'