diff --git a/openhands/core/config/utils.py b/openhands/core/config/utils.py index f90525585d..78301414aa 100644 --- a/openhands/core/config/utils.py +++ b/openhands/core/config/utils.py @@ -302,25 +302,33 @@ def finalize_config(cfg: AppConfig) -> None: # Split by commas to handle multiple mounts mounts = cfg.sandbox.volumes.split(',') - # Use the first volume for backward compatibility - if mounts: - primary_mount = mounts[0] - parts = primary_mount.split(':') - if len(parts) < 2 or len(parts) > 3: - raise ValueError( - f'Invalid sandbox.volumes format: {primary_mount}. ' - f"Expected format: 'host_path:container_path[:mode]', e.g. '/my/host/dir:/workspace:rw'" - ) + # Check if any mount explicitly targets /workspace + workspace_mount_found = False + for mount in mounts: + parts = mount.split(':') + if len(parts) >= 2 and parts[1] == '/workspace': + workspace_mount_found = True + host_path = os.path.abspath(parts[0]) - host_path = os.path.abspath(parts[0]) - container_path = parts[1] + # Set the workspace_mount_path and workspace_mount_path_in_sandbox + cfg.workspace_mount_path = host_path + cfg.workspace_mount_path_in_sandbox = '/workspace' - # Set the workspace_mount_path and workspace_mount_path_in_sandbox for backward compatibility - cfg.workspace_mount_path = host_path - cfg.workspace_mount_path_in_sandbox = container_path + # Also set workspace_base + cfg.workspace_base = host_path + break - # Also set workspace_base for backward compatibility - cfg.workspace_base = host_path + # If no explicit /workspace mount was found, don't set any workspace mount + # This allows users to mount volumes without affecting the workspace + if not workspace_mount_found: + logger.openhands_logger.debug( + 'No explicit /workspace mount found in SANDBOX_VOLUMES. ' + 'Using default workspace path in sandbox.' + ) + # Ensure workspace_mount_path and workspace_base are None to avoid + # unintended mounting behavior + cfg.workspace_mount_path = None + cfg.workspace_base = None # Validate all mounts for mount in mounts: @@ -619,7 +627,6 @@ def register_custom_agents(config: AppConfig) -> None: This function is called after configuration is loaded to ensure all custom agents specified in the config are properly imported and registered. """ - # Import here to avoid circular dependency from openhands.controller.agent import Agent diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py index ef77396b60..1454458b25 100644 --- a/tests/unit/test_config.py +++ b/tests/unit/test_config.py @@ -434,7 +434,7 @@ def test_defaults_dict_after_updates(default_config): def test_sandbox_volumes(monkeypatch, default_config): - # Test SANDBOX_VOLUMES with multiple mounts + # Test SANDBOX_VOLUMES with multiple mounts (no explicit /workspace mount) monkeypatch.setenv( 'SANDBOX_VOLUMES', '/host/path1:/container/path1,/host/path2:/container/path2:ro', @@ -449,14 +449,17 @@ def test_sandbox_volumes(monkeypatch, default_config): == '/host/path1:/container/path1,/host/path2:/container/path2:ro' ) - # Check that the old parameters are set from the first mount for backward compatibility - assert default_config.workspace_base == os.path.abspath('/host/path1') - assert default_config.workspace_mount_path == os.path.abspath('/host/path1') - assert default_config.workspace_mount_path_in_sandbox == '/container/path1' + # With the new behavior, workspace_base and workspace_mount_path should be None + # when no explicit /workspace mount is found + assert default_config.workspace_base is None + assert default_config.workspace_mount_path is None + assert ( + default_config.workspace_mount_path_in_sandbox == '/workspace' + ) # Default value def test_sandbox_volumes_with_mode(monkeypatch, default_config): - # Test SANDBOX_VOLUMES with read-only mode + # Test SANDBOX_VOLUMES with read-only mode (no explicit /workspace mount) monkeypatch.setenv('SANDBOX_VOLUMES', '/host/path1:/container/path1:ro') load_from_env(default_config, os.environ) @@ -465,10 +468,13 @@ def test_sandbox_volumes_with_mode(monkeypatch, default_config): # Check that sandbox.volumes is set correctly assert default_config.sandbox.volumes == '/host/path1:/container/path1:ro' - # Check that the old parameters are set for backward compatibility - assert default_config.workspace_base == os.path.abspath('/host/path1') - assert default_config.workspace_mount_path == os.path.abspath('/host/path1') - assert default_config.workspace_mount_path_in_sandbox == '/container/path1' + # With the new behavior, workspace_base and workspace_mount_path should be None + # when no explicit /workspace mount is found + assert default_config.workspace_base is None + assert default_config.workspace_mount_path is None + assert ( + default_config.workspace_mount_path_in_sandbox == '/workspace' + ) # Default value def test_invalid_toml_format(monkeypatch, temp_toml_file, default_config): @@ -639,6 +645,37 @@ def test_cache_dir_creation(default_config, tmpdir): assert os.path.exists(default_config.cache_dir) +def test_sandbox_volumes_with_workspace(default_config): + """Test that sandbox.volumes with explicit /workspace mount works correctly.""" + default_config.sandbox.volumes = '/home/user/mydir:/workspace:rw,/data:/data:ro' + finalize_config(default_config) + assert default_config.workspace_mount_path == '/home/user/mydir' + assert default_config.workspace_mount_path_in_sandbox == '/workspace' + assert default_config.workspace_base == '/home/user/mydir' + + +def test_sandbox_volumes_without_workspace(default_config): + """Test that sandbox.volumes without explicit /workspace mount doesn't set workspace paths.""" + default_config.sandbox.volumes = '/data:/data:ro,/models:/models:ro' + finalize_config(default_config) + assert default_config.workspace_mount_path is None + assert default_config.workspace_base is None + assert ( + default_config.workspace_mount_path_in_sandbox == '/workspace' + ) # Default value remains unchanged + + +def test_sandbox_volumes_with_workspace_not_first(default_config): + """Test that sandbox.volumes with /workspace mount not as first entry works correctly.""" + default_config.sandbox.volumes = ( + '/data:/data:ro,/home/user/mydir:/workspace:rw,/models:/models:ro' + ) + finalize_config(default_config) + assert default_config.workspace_mount_path == '/home/user/mydir' + assert default_config.workspace_mount_path_in_sandbox == '/workspace' + assert default_config.workspace_base == '/home/user/mydir' + + def test_agent_config_condenser_with_no_enabled(): """Test default agent condenser with enable_default_condenser=False.""" config = AppConfig(enable_default_condenser=False)