Fix workspace mount behavior with SANDBOX_VOLUMES (#8500)

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
 

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)