mirror of
https://github.com/OpenHands/OpenHands.git
synced 2025-12-26 13:52:43 +08:00
Update str_replace_editor tool to use dynamic workspace path from SANDBOX_VOLUMES (#10965)
Co-authored-by: openhands <openhands@all-hands.dev>
This commit is contained in:
parent
d3f3378a4c
commit
0f1780728e
@ -146,7 +146,8 @@ class CodeActAgent(Agent):
|
||||
elif self.config.enable_editor:
|
||||
tools.append(
|
||||
create_str_replace_editor_tool(
|
||||
use_short_description=use_short_tool_desc
|
||||
use_short_description=use_short_tool_desc,
|
||||
runtime_type=self.config.runtime,
|
||||
)
|
||||
)
|
||||
return tools
|
||||
|
||||
@ -1,9 +1,12 @@
|
||||
import os
|
||||
|
||||
from litellm import ChatCompletionToolParam, ChatCompletionToolParamFunctionChunk
|
||||
|
||||
from openhands.agenthub.codeact_agent.tools.security_utils import (
|
||||
RISK_LEVELS,
|
||||
SECURITY_RISK_DESC,
|
||||
)
|
||||
from openhands.core.config.config_utils import DEFAULT_WORKSPACE_MOUNT_PATH_IN_SANDBOX
|
||||
from openhands.llm.tool_names import STR_REPLACE_EDITOR_TOOL_NAME
|
||||
|
||||
_DETAILED_STR_REPLACE_EDITOR_DESCRIPTION = """Custom editing tool for viewing, creating and editing files in plain-text format
|
||||
@ -52,9 +55,51 @@ Notes for using the `str_replace` command:
|
||||
"""
|
||||
|
||||
|
||||
def _get_workspace_mount_path_from_env(runtime_type: str | None = None) -> str:
|
||||
"""Get the workspace mount path from SANDBOX_VOLUMES environment variable.
|
||||
|
||||
For LocalRuntime and CLIRuntime, returns the host path from SANDBOX_VOLUMES.
|
||||
For other runtimes, returns the default container path (/workspace).
|
||||
|
||||
Args:
|
||||
runtime_type: The runtime type ('local', 'cli', 'docker', etc.)
|
||||
|
||||
Returns:
|
||||
The workspace mount path in sandbox, defaults to '/workspace' if not found.
|
||||
"""
|
||||
# For LocalRuntime/CLIRuntime, try to get host path from SANDBOX_VOLUMES
|
||||
if runtime_type in ('local', 'cli'):
|
||||
sandbox_volumes = os.environ.get('SANDBOX_VOLUMES')
|
||||
if sandbox_volumes:
|
||||
# Split by commas to handle multiple mounts
|
||||
mounts = sandbox_volumes.split(',')
|
||||
|
||||
# Check if any mount explicitly targets /workspace
|
||||
for mount in mounts:
|
||||
parts = mount.split(':')
|
||||
if len(parts) >= 2 and parts[1] == '/workspace':
|
||||
host_path = os.path.abspath(parts[0])
|
||||
return host_path
|
||||
|
||||
# Fallback for local/CLI runtimes when SANDBOX_VOLUMES is not set:
|
||||
# Use current working directory as it's likely the workspace root
|
||||
return os.getcwd()
|
||||
|
||||
# For all other runtimes (docker, remote, etc.), use default container path
|
||||
return DEFAULT_WORKSPACE_MOUNT_PATH_IN_SANDBOX
|
||||
|
||||
|
||||
def create_str_replace_editor_tool(
|
||||
use_short_description: bool = False,
|
||||
workspace_mount_path_in_sandbox: str | None = None,
|
||||
runtime_type: str | None = None,
|
||||
) -> ChatCompletionToolParam:
|
||||
# If no workspace path is provided, try to get it from environment
|
||||
if workspace_mount_path_in_sandbox is None:
|
||||
workspace_mount_path_in_sandbox = _get_workspace_mount_path_from_env(
|
||||
runtime_type
|
||||
)
|
||||
|
||||
description = (
|
||||
_SHORT_STR_REPLACE_EDITOR_DESCRIPTION
|
||||
if use_short_description
|
||||
@ -80,7 +125,7 @@ def create_str_replace_editor_tool(
|
||||
'type': 'string',
|
||||
},
|
||||
'path': {
|
||||
'description': 'Absolute path to file or directory, e.g. `/workspace/file.py` or `/workspace`.',
|
||||
'description': f'Absolute path to file or directory, e.g. `{workspace_mount_path_in_sandbox}/file.py` or `{workspace_mount_path_in_sandbox}`.',
|
||||
'type': 'string',
|
||||
},
|
||||
'file_text': {
|
||||
|
||||
@ -62,6 +62,8 @@ class AgentConfig(BaseModel):
|
||||
"""Model routing configuration settings."""
|
||||
extended: ExtendedConfig = Field(default_factory=lambda: ExtendedConfig({}))
|
||||
"""Extended configuration for the agent."""
|
||||
runtime: str | None = Field(default=None)
|
||||
"""Runtime type (e.g., 'docker', 'local', 'cli') used for runtime-specific tool behavior."""
|
||||
|
||||
model_config = ConfigDict(extra='forbid')
|
||||
|
||||
|
||||
@ -202,6 +202,8 @@ def create_memory(
|
||||
def create_agent(config: OpenHandsConfig, llm_registry: LLMRegistry) -> Agent:
|
||||
agent_cls: type[Agent] = Agent.get_cls(config.default_agent)
|
||||
agent_config = config.get_agent_config(config.default_agent)
|
||||
# Pass the runtime information from the main config to the agent config
|
||||
agent_config.runtime = config.runtime
|
||||
config.get_llm_config_from_agent(config.default_agent)
|
||||
agent = agent_cls(config=agent_config, llm_registry=llm_registry)
|
||||
return agent
|
||||
|
||||
@ -212,6 +212,8 @@ class WebSession:
|
||||
|
||||
# TODO: override other LLM config & agent config groups (#2075)
|
||||
agent_config = self.config.get_agent_config(agent_cls)
|
||||
# Pass runtime information to agent config for runtime-specific tool behavior
|
||||
agent_config.runtime = self.config.runtime
|
||||
agent_name = agent_cls if agent_cls is not None else 'agent'
|
||||
llm_config = self.config.get_llm_config_from_agent(agent_name)
|
||||
if settings.enable_default_condenser:
|
||||
|
||||
225
tests/unit/agenthub/test_str_replace_editor_tool.py
Normal file
225
tests/unit/agenthub/test_str_replace_editor_tool.py
Normal file
@ -0,0 +1,225 @@
|
||||
"""Tests for str_replace_editor tool workspace path detection."""
|
||||
|
||||
import os
|
||||
import sys
|
||||
from unittest.mock import patch
|
||||
|
||||
import pytest
|
||||
|
||||
# Import the functions directly to avoid dependency issues
|
||||
sys.path.insert(0, os.path.join(os.path.dirname(__file__), '../../..'))
|
||||
|
||||
try:
|
||||
from openhands.agenthub.codeact_agent.tools.str_replace_editor import (
|
||||
_get_workspace_mount_path_from_env,
|
||||
create_str_replace_editor_tool,
|
||||
)
|
||||
except ImportError:
|
||||
# If import fails due to dependencies, skip these tests
|
||||
pytest.skip(
|
||||
'Cannot import str_replace_editor due to missing dependencies',
|
||||
allow_module_level=True,
|
||||
)
|
||||
|
||||
|
||||
class TestWorkspacePathDetection:
|
||||
"""Test workspace path detection logic."""
|
||||
|
||||
def test_docker_runtime_ignores_sandbox_volumes(self):
|
||||
"""Test that DockerRuntime ignores SANDBOX_VOLUMES and uses /workspace."""
|
||||
with patch.dict(os.environ, {'SANDBOX_VOLUMES': '/host/app:/workspace:rw'}):
|
||||
result = _get_workspace_mount_path_from_env(runtime_type='docker')
|
||||
assert result == '/workspace'
|
||||
|
||||
def test_remote_runtime_ignores_sandbox_volumes(self):
|
||||
"""Test that RemoteRuntime ignores SANDBOX_VOLUMES and uses /workspace."""
|
||||
with patch.dict(os.environ, {'SANDBOX_VOLUMES': '/host/app:/workspace:rw'}):
|
||||
result = _get_workspace_mount_path_from_env(runtime_type='remote')
|
||||
assert result == '/workspace'
|
||||
|
||||
def test_e2b_runtime_ignores_sandbox_volumes(self):
|
||||
"""Test that E2BRuntime ignores SANDBOX_VOLUMES and uses /workspace."""
|
||||
with patch.dict(os.environ, {'SANDBOX_VOLUMES': '/host/app:/workspace:rw'}):
|
||||
result = _get_workspace_mount_path_from_env(runtime_type='e2b')
|
||||
assert result == '/workspace'
|
||||
|
||||
def test_local_runtime_uses_sandbox_volumes(self):
|
||||
"""Test that LocalRuntime uses host path from SANDBOX_VOLUMES."""
|
||||
with patch.dict(os.environ, {'SANDBOX_VOLUMES': '/host/app:/workspace:rw'}):
|
||||
result = _get_workspace_mount_path_from_env(runtime_type='local')
|
||||
assert result == '/host/app'
|
||||
|
||||
def test_local_runtime_multiple_mounts(self):
|
||||
"""Test LocalRuntime with multiple mounts, finds /workspace mount."""
|
||||
with patch.dict(
|
||||
os.environ,
|
||||
{
|
||||
'SANDBOX_VOLUMES': '/tmp:/tmp:rw,/home/user/project:/workspace:rw,/var:/var:rw'
|
||||
},
|
||||
):
|
||||
result = _get_workspace_mount_path_from_env(runtime_type='local')
|
||||
assert result == '/home/user/project'
|
||||
|
||||
def test_cli_runtime_multiple_mounts(self):
|
||||
"""Test CLIRuntime with multiple mounts, finds /workspace mount."""
|
||||
with patch.dict(
|
||||
os.environ,
|
||||
{
|
||||
'SANDBOX_VOLUMES': '/tmp:/tmp:rw,/home/user/project:/workspace:rw,/var:/var:rw'
|
||||
},
|
||||
):
|
||||
result = _get_workspace_mount_path_from_env(runtime_type='cli')
|
||||
assert result == '/home/user/project'
|
||||
|
||||
def test_local_runtime_no_workspace_mount(self):
|
||||
"""Test LocalRuntime with no /workspace mount falls back to current directory."""
|
||||
with patch.dict(os.environ, {'SANDBOX_VOLUMES': '/tmp:/tmp:rw,/var:/var:rw'}):
|
||||
result = _get_workspace_mount_path_from_env(runtime_type='local')
|
||||
assert result == os.getcwd()
|
||||
|
||||
def test_local_runtime_no_sandbox_volumes(self):
|
||||
"""Test LocalRuntime with no SANDBOX_VOLUMES falls back to current directory."""
|
||||
with patch.dict(os.environ, {}, clear=True):
|
||||
result = _get_workspace_mount_path_from_env(runtime_type='local')
|
||||
assert result == os.getcwd()
|
||||
|
||||
def test_local_runtime_empty_sandbox_volumes(self):
|
||||
"""Test LocalRuntime with empty SANDBOX_VOLUMES falls back to current directory."""
|
||||
with patch.dict(os.environ, {'SANDBOX_VOLUMES': ''}):
|
||||
result = _get_workspace_mount_path_from_env(runtime_type='local')
|
||||
assert result == os.getcwd()
|
||||
|
||||
def test_relative_path_conversion(self):
|
||||
"""Test that relative paths in SANDBOX_VOLUMES are converted to absolute."""
|
||||
with patch.dict(
|
||||
os.environ, {'SANDBOX_VOLUMES': './relative/path:/workspace:rw'}
|
||||
):
|
||||
result = _get_workspace_mount_path_from_env(runtime_type='local')
|
||||
# Should be converted to absolute path
|
||||
assert os.path.isabs(result)
|
||||
assert result.endswith('relative/path')
|
||||
|
||||
def test_unknown_runtime_type(self):
|
||||
"""Test that unknown runtime types use default container path."""
|
||||
with patch.dict(os.environ, {'SANDBOX_VOLUMES': '/host/app:/workspace:rw'}):
|
||||
result = _get_workspace_mount_path_from_env(runtime_type='unknown')
|
||||
assert result == '/workspace'
|
||||
|
||||
def test_none_runtime_type(self):
|
||||
"""Test that None runtime type uses default container path."""
|
||||
with patch.dict(os.environ, {'SANDBOX_VOLUMES': '/host/app:/workspace:rw'}):
|
||||
result = _get_workspace_mount_path_from_env(runtime_type=None)
|
||||
assert result == '/workspace'
|
||||
|
||||
|
||||
class TestToolCreation:
|
||||
"""Test tool creation with dynamic workspace paths."""
|
||||
|
||||
def test_tool_creation_docker_runtime(self):
|
||||
"""Test tool creation in DockerRuntime shows /workspace in description."""
|
||||
with patch.dict(os.environ, {'SANDBOX_VOLUMES': '/host/app:/workspace:rw'}):
|
||||
tool = create_str_replace_editor_tool(runtime_type='docker')
|
||||
path_description = tool['function']['parameters']['properties']['path'][
|
||||
'description'
|
||||
]
|
||||
assert '/workspace/file.py' in path_description
|
||||
assert '/workspace`.' in path_description
|
||||
# Should not contain host paths
|
||||
assert '/host/app' not in path_description
|
||||
|
||||
def test_tool_creation_local_runtime(self):
|
||||
"""Test tool creation in LocalRuntime shows host path in description."""
|
||||
with patch.dict(os.environ, {'SANDBOX_VOLUMES': '/host/app:/workspace:rw'}):
|
||||
tool = create_str_replace_editor_tool(runtime_type='local')
|
||||
path_description = tool['function']['parameters']['properties']['path'][
|
||||
'description'
|
||||
]
|
||||
assert '/host/app/file.py' in path_description
|
||||
assert '/host/app`.' in path_description
|
||||
|
||||
def test_tool_creation_cli_runtime(self):
|
||||
"""Test tool creation in CLIRuntime shows host path in description."""
|
||||
with patch.dict(os.environ, {'SANDBOX_VOLUMES': '/host/app:/workspace:rw'}):
|
||||
tool = create_str_replace_editor_tool(runtime_type='cli')
|
||||
path_description = tool['function']['parameters']['properties']['path'][
|
||||
'description'
|
||||
]
|
||||
assert '/host/app/file.py' in path_description
|
||||
assert '/host/app`.' in path_description
|
||||
|
||||
def test_tool_creation_remote_runtime(self):
|
||||
"""Test tool creation in RemoteRuntime shows /workspace in description."""
|
||||
with patch.dict(os.environ, {'SANDBOX_VOLUMES': '/host/app:/workspace:rw'}):
|
||||
tool = create_str_replace_editor_tool(runtime_type='remote')
|
||||
path_description = tool['function']['parameters']['properties']['path'][
|
||||
'description'
|
||||
]
|
||||
assert '/workspace/file.py' in path_description
|
||||
assert '/workspace`.' in path_description
|
||||
# Should not contain host paths
|
||||
assert '/host/app' not in path_description
|
||||
|
||||
def test_tool_creation_e2b_runtime(self):
|
||||
"""Test tool creation in E2BRuntime shows /workspace in description."""
|
||||
with patch.dict(os.environ, {'SANDBOX_VOLUMES': '/host/app:/workspace:rw'}):
|
||||
tool = create_str_replace_editor_tool(runtime_type='e2b')
|
||||
path_description = tool['function']['parameters']['properties']['path'][
|
||||
'description'
|
||||
]
|
||||
assert '/workspace/file.py' in path_description
|
||||
assert '/workspace`.' in path_description
|
||||
# Should not contain host paths
|
||||
assert '/host/app' not in path_description
|
||||
|
||||
def test_tool_creation_explicit_workspace_path(self):
|
||||
"""Test tool creation with explicitly provided workspace path."""
|
||||
tool = create_str_replace_editor_tool(
|
||||
workspace_mount_path_in_sandbox='/custom/path'
|
||||
)
|
||||
path_description = tool['function']['parameters']['properties']['path'][
|
||||
'description'
|
||||
]
|
||||
assert '/custom/path/file.py' in path_description
|
||||
assert '/custom/path`.' in path_description
|
||||
|
||||
def test_tool_creation_short_description(self):
|
||||
"""Test tool creation with short description still has correct path."""
|
||||
with patch.dict(os.environ, {'SANDBOX_VOLUMES': '/host/app:/workspace:rw'}):
|
||||
tool = create_str_replace_editor_tool(
|
||||
use_short_description=True, runtime_type='local'
|
||||
)
|
||||
path_description = tool['function']['parameters']['properties']['path'][
|
||||
'description'
|
||||
]
|
||||
assert '/host/app/file.py' in path_description
|
||||
assert '/host/app`.' in path_description
|
||||
|
||||
|
||||
class TestEdgeCases:
|
||||
"""Test edge cases and error conditions."""
|
||||
|
||||
def test_malformed_sandbox_volumes(self):
|
||||
"""Test handling of malformed SANDBOX_VOLUMES."""
|
||||
# Missing colon separator - falls back to current directory for local/CLI
|
||||
with patch.dict(os.environ, {'SANDBOX_VOLUMES': '/host/app/workspace'}):
|
||||
result = _get_workspace_mount_path_from_env(runtime_type='local')
|
||||
assert result == os.getcwd()
|
||||
|
||||
# Only one part - falls back to current directory for local/CLI
|
||||
with patch.dict(os.environ, {'SANDBOX_VOLUMES': '/host/app:'}):
|
||||
result = _get_workspace_mount_path_from_env(runtime_type='local')
|
||||
assert result == os.getcwd()
|
||||
|
||||
def test_workspace_mount_with_different_permissions(self):
|
||||
"""Test /workspace mount with different permission strings."""
|
||||
test_cases = [
|
||||
'/host/app:/workspace:ro',
|
||||
'/host/app:/workspace:rw',
|
||||
'/host/app:/workspace', # No permissions
|
||||
'/host/app:/workspace:rw,size=100m', # Extra options
|
||||
]
|
||||
|
||||
for sandbox_volumes in test_cases:
|
||||
with patch.dict(os.environ, {'SANDBOX_VOLUMES': sandbox_volumes}):
|
||||
result = _get_workspace_mount_path_from_env(runtime_type='local')
|
||||
assert result == '/host/app', f'Failed for: {sandbox_volumes}'
|
||||
Loading…
x
Reference in New Issue
Block a user