Merge branch 'master' into tdd/container-resolve-crash
CI / benchmark-publish (pull_request) Has been skipped
CI / lint (pull_request) Successful in 16s
CI / build (pull_request) Successful in 17s
CI / quality (pull_request) Successful in 18s
CI / security (pull_request) Successful in 38s
CI / typecheck (pull_request) Successful in 40s
CI / unit_tests (pull_request) Successful in 2m59s
CI / integration_tests (pull_request) Successful in 3m37s
CI / docker (pull_request) Successful in 39s
CI / coverage (pull_request) Successful in 6m35s
CI / benchmark-regression (pull_request) Successful in 35m33s
CI / benchmark-publish (pull_request) Has been skipped
CI / lint (pull_request) Successful in 16s
CI / build (pull_request) Successful in 17s
CI / quality (pull_request) Successful in 18s
CI / security (pull_request) Successful in 38s
CI / typecheck (pull_request) Successful in 40s
CI / unit_tests (pull_request) Successful in 2m59s
CI / integration_tests (pull_request) Successful in 3m37s
CI / docker (pull_request) Successful in 39s
CI / coverage (pull_request) Successful in 6m35s
CI / benchmark-regression (pull_request) Successful in 35m33s
This commit is contained in:
@@ -36,6 +36,16 @@
|
||||
prevented by a `contextvars` nesting guard. Per-service overrides are loaded
|
||||
from the `retry_service_overrides` JSON config key. Includes Behave BDD unit
|
||||
tests, Robot Framework integration tests, and ASV benchmarks. (#313)
|
||||
- Added container-aware tool execution and I/O forwarding via `ContainerToolExecutor`
|
||||
and `PathMapper`. Tools routed to `execution_environment: container` are executed
|
||||
inside a provisioned devcontainer with automatic host↔container path mapping,
|
||||
bounded output capture (50 MiB), structured error reporting, and container metadata
|
||||
on the `ToolInvocation` audit trail. Includes `ContainerConfig`, `ContainerMetadata`,
|
||||
`ContainerExecutionError`, and `ContainerTimeoutError` domain models, `ToolRunner`
|
||||
container routing integration, safe environment filtering, symlink/traversal
|
||||
protection, and `sync_results_to_host` for file-based result retrieval. Covered by
|
||||
Behave BDD scenarios, Robot Framework integration tests, ASV benchmarks, and
|
||||
`docs/reference/execution_environment.md`. (#515)
|
||||
|
||||
|
||||
### Added
|
||||
|
||||
@@ -0,0 +1,32 @@
|
||||
"""Add container_metadata_json column to tool_invocations table.
|
||||
|
||||
Revision ID: m6_004_container_metadata_column
|
||||
Revises: m6_004_resource_type_inherits
|
||||
Create Date: 2026-03-11 00:00:00
|
||||
|
||||
"""
|
||||
|
||||
from collections.abc import Sequence
|
||||
|
||||
from alembic import op
|
||||
import sqlalchemy as sa
|
||||
|
||||
# revision identifiers, used by Alembic.
|
||||
revision: str = "m6_004_container_metadata_column"
|
||||
down_revision: str | Sequence[str] | None = "m6_004_resource_type_inherits"
|
||||
branch_labels: str | Sequence[str] | None = None
|
||||
depends_on: str | Sequence[str] | None = None
|
||||
|
||||
|
||||
def upgrade() -> None:
|
||||
"""Add container_metadata_json column for container execution audit trail."""
|
||||
with op.batch_alter_table("tool_invocations") as batch_op:
|
||||
batch_op.add_column(
|
||||
sa.Column("container_metadata_json", sa.Text(), nullable=True),
|
||||
)
|
||||
|
||||
|
||||
def downgrade() -> None:
|
||||
"""Remove container_metadata_json column."""
|
||||
with op.batch_alter_table("tool_invocations") as batch_op:
|
||||
batch_op.drop_column("container_metadata_json")
|
||||
@@ -0,0 +1,153 @@
|
||||
"""ASV benchmarks for container tool execution overhead (#515).
|
||||
|
||||
Measures the performance cost of path mapping, input/output
|
||||
translation, and container metadata creation — the operations
|
||||
that happen on every container-routed tool call.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
|
||||
from cleveragents.tool.container_executor import (
|
||||
ContainerConfig,
|
||||
ContainerMetadata,
|
||||
ContainerToolExecutor,
|
||||
)
|
||||
from cleveragents.tool.path_mapper import PathMapper
|
||||
|
||||
|
||||
class PathMapperBench:
|
||||
"""Benchmark PathMapper host-to-container and container-to-host translations."""
|
||||
|
||||
def setup(self) -> None:
|
||||
self.mapper = PathMapper(
|
||||
host_root="/tmp/sandbox/project-abc",
|
||||
container_root="/workspace",
|
||||
)
|
||||
self.host_path = "/tmp/sandbox/project-abc/src/cleveragents/tool/runner.py"
|
||||
self.container_path = "/workspace/src/cleveragents/tool/runner.py"
|
||||
self.external_path = (
|
||||
"/usr/local/lib/python3.13/site-packages/pydantic/__init__.py"
|
||||
)
|
||||
|
||||
def time_host_to_container(self) -> None:
|
||||
self.mapper.host_to_container(self.host_path)
|
||||
|
||||
def time_container_to_host(self) -> None:
|
||||
self.mapper.container_to_host(self.container_path)
|
||||
|
||||
def time_external_path_passthrough(self) -> None:
|
||||
self.mapper.host_to_container(self.external_path)
|
||||
|
||||
def time_is_host_path_check(self) -> None:
|
||||
self.mapper.is_host_path(self.host_path)
|
||||
|
||||
def time_is_container_path_check(self) -> None:
|
||||
self.mapper.is_container_path(self.container_path)
|
||||
|
||||
|
||||
class InputPathMappingBench:
|
||||
"""Benchmark input path mapping with various nesting depths."""
|
||||
|
||||
def setup(self) -> None:
|
||||
config = ContainerConfig(
|
||||
workspace_folder="/workspace",
|
||||
host_sandbox_path="/tmp/sandbox",
|
||||
container_id="bench-ctr",
|
||||
)
|
||||
self.executor = ContainerToolExecutor(config)
|
||||
self.flat_inputs = {
|
||||
"path": "/tmp/sandbox/src/main.py",
|
||||
"output_dir": "/tmp/sandbox/build",
|
||||
"name": "my-tool",
|
||||
"count": 42,
|
||||
}
|
||||
self.nested_inputs = {
|
||||
"paths": {
|
||||
"source": "/tmp/sandbox/src/main.py",
|
||||
"config": "/tmp/sandbox/.config/settings.json",
|
||||
"nested": {
|
||||
"deep": "/tmp/sandbox/deep/path.txt",
|
||||
},
|
||||
},
|
||||
"list_paths": [
|
||||
"/tmp/sandbox/a.py",
|
||||
"/tmp/sandbox/b.py",
|
||||
"/tmp/sandbox/c.py",
|
||||
],
|
||||
}
|
||||
|
||||
def time_flat_input_mapping(self) -> None:
|
||||
self.executor._map_input_paths(self.flat_inputs)
|
||||
|
||||
def time_nested_input_mapping(self) -> None:
|
||||
self.executor._map_input_paths(self.nested_inputs)
|
||||
|
||||
|
||||
class OutputPathMappingBench:
|
||||
"""Benchmark output path mapping."""
|
||||
|
||||
def setup(self) -> None:
|
||||
config = ContainerConfig(
|
||||
workspace_folder="/workspace",
|
||||
host_sandbox_path="/tmp/sandbox",
|
||||
container_id="bench-ctr",
|
||||
)
|
||||
self.executor = ContainerToolExecutor(config)
|
||||
self.output = {
|
||||
"created_file": "/workspace/build/output.txt",
|
||||
"log_file": "/workspace/logs/run.log",
|
||||
"status": "success",
|
||||
"count": 10,
|
||||
}
|
||||
|
||||
def time_output_mapping(self) -> None:
|
||||
self.executor._map_output_paths(self.output)
|
||||
|
||||
|
||||
class ContainerMetadataBench:
|
||||
"""Benchmark container metadata creation and serialisation."""
|
||||
|
||||
def setup(self) -> None:
|
||||
self.metadata = ContainerMetadata(
|
||||
container_id="abc123def456",
|
||||
image="ghcr.io/org/devimage:latest",
|
||||
workspace_folder="/workspace",
|
||||
exec_time_ms=1500.0,
|
||||
exit_code=0,
|
||||
timed_out=False,
|
||||
)
|
||||
|
||||
def time_metadata_creation(self) -> None:
|
||||
ContainerMetadata(
|
||||
container_id="abc123def456",
|
||||
image="ghcr.io/org/devimage:latest",
|
||||
workspace_folder="/workspace",
|
||||
exec_time_ms=1500.0,
|
||||
exit_code=0,
|
||||
timed_out=False,
|
||||
)
|
||||
|
||||
def time_metadata_to_dict(self) -> None:
|
||||
self.metadata.model_dump()
|
||||
|
||||
|
||||
class OutputParsingBench:
|
||||
"""Benchmark output parsing (JSON vs plain text)."""
|
||||
|
||||
def setup(self) -> None:
|
||||
self.json_output = json.dumps(
|
||||
{"files": [f"file_{i}.py" for i in range(50)], "errors": 0}
|
||||
)
|
||||
self.plain_output = "Build completed successfully\n" * 20
|
||||
self.empty_output = ""
|
||||
|
||||
def time_parse_json_output(self) -> None:
|
||||
ContainerToolExecutor._parse_output(self.json_output)
|
||||
|
||||
def time_parse_plain_output(self) -> None:
|
||||
ContainerToolExecutor._parse_output(self.plain_output)
|
||||
|
||||
def time_parse_empty_output(self) -> None:
|
||||
ContainerToolExecutor._parse_output(self.empty_output)
|
||||
@@ -0,0 +1,225 @@
|
||||
# Execution Environment Reference
|
||||
|
||||
This document describes how CleverAgents routes tool execution between
|
||||
host and container environments, including the container-aware tool
|
||||
executor, path mapping, and I/O forwarding.
|
||||
|
||||
## Overview
|
||||
|
||||
When a plan or tool specifies `execution_environment: container`,
|
||||
the tool runner delegates execution to a **ContainerToolExecutor** that:
|
||||
|
||||
1. Maps host-side file paths to container-side equivalents.
|
||||
2. Executes the tool via `devcontainer exec`.
|
||||
3. Retrieves results and maps output paths back to the host.
|
||||
4. Records container metadata on the `ToolInvocation` audit trail.
|
||||
|
||||
## Execution Environment Resolution
|
||||
|
||||
The execution environment is resolved using a priority chain:
|
||||
|
||||
| Priority | Source | Example |
|
||||
|----------|--------|---------|
|
||||
| 1 (highest) | Tool-level override | `tool_env="container"` |
|
||||
| 2 | Plan-level override | `--execution-environment container` |
|
||||
| 3 | Project-level default | `ContextConfig.execution_environment` |
|
||||
| 4 (lowest) | System default | `HOST` |
|
||||
|
||||
If `CONTAINER` is resolved, the system validates that a container
|
||||
resource (devcontainer-instance, container-instance, or
|
||||
devcontainer-file) is linked to the project.
|
||||
|
||||
## Path Mapping
|
||||
|
||||
The `PathMapper` utility translates file paths between host and
|
||||
container filesystems:
|
||||
|
||||
```python
|
||||
from cleveragents.tool.path_mapper import PathMapper
|
||||
|
||||
mapper = PathMapper(
|
||||
host_root="/tmp/sandbox/project-1",
|
||||
container_root="/workspace",
|
||||
)
|
||||
|
||||
# Host to container
|
||||
mapper.host_to_container("/tmp/sandbox/project-1/src/main.py")
|
||||
# => "/workspace/src/main.py"
|
||||
|
||||
# Container to host
|
||||
mapper.container_to_host("/workspace/src/main.py")
|
||||
# => "/tmp/sandbox/project-1/src/main.py"
|
||||
|
||||
# Paths outside root are returned unchanged
|
||||
mapper.host_to_container("/usr/lib/libc.so")
|
||||
# => "/usr/lib/libc.so"
|
||||
```
|
||||
|
||||
### Path Mapping in Tool Inputs
|
||||
|
||||
File path arguments in tool inputs are automatically mapped before
|
||||
container execution. The executor recursively traverses the input
|
||||
dict, mapping any string values that fall under the host root:
|
||||
|
||||
```python
|
||||
# Before mapping
|
||||
{"path": "/tmp/sandbox/src/main.py", "count": 5}
|
||||
|
||||
# After mapping
|
||||
{"path": "/workspace/src/main.py", "count": 5}
|
||||
```
|
||||
|
||||
Nested dicts and lists are also traversed.
|
||||
|
||||
## Container Tool Executor
|
||||
|
||||
The `ContainerToolExecutor` wraps tool execution via `devcontainer exec`:
|
||||
|
||||
```python
|
||||
from cleveragents.tool.container_executor import (
|
||||
ContainerConfig,
|
||||
ContainerToolExecutor,
|
||||
)
|
||||
|
||||
config = ContainerConfig(
|
||||
workspace_folder="/workspace",
|
||||
container_id="my-container",
|
||||
image="ghcr.io/org/devimage:latest",
|
||||
host_sandbox_path="/tmp/sandbox/project-1",
|
||||
timeout_seconds=120,
|
||||
)
|
||||
|
||||
executor = ContainerToolExecutor(config)
|
||||
result = executor.execute_tool("file_read", {"path": "/tmp/sandbox/project-1/src/main.py"})
|
||||
```
|
||||
|
||||
### Configuration
|
||||
|
||||
| Parameter | Type | Default | Description |
|
||||
|-----------|------|---------|-------------|
|
||||
| `workspace_folder` | `str` | `/workspace` | Workspace root inside the container |
|
||||
| `container_id` | `str` | `""` | Docker/Podman container ID |
|
||||
| `image` | `str` | `""` | Container image reference |
|
||||
| `timeout_seconds` | `int` | `120` | Max execution time (seconds) |
|
||||
| `host_sandbox_path` | `str` | `""` | Host-side sandbox root |
|
||||
|
||||
### Timeout Handling
|
||||
|
||||
If a container tool execution exceeds `timeout_seconds`, the
|
||||
executor returns a `ToolResult` with `success=False` and an error
|
||||
message indicating the timeout. The `metadata.container` dict on
|
||||
the result will have `timed_out=True`.
|
||||
|
||||
### Error Reporting
|
||||
|
||||
Container execution failures produce structured `ToolResult` objects:
|
||||
|
||||
- **Timeout**: `error="Container execution timed out after Ns"`
|
||||
- **Non-zero exit**: `error="Container execution failed (exit code N): stderr"`
|
||||
- **OS error**: `error="Container execution failed (exit code -1): error message"`
|
||||
|
||||
All errors include `container_metadata` with execution details.
|
||||
|
||||
## Result Retrieval
|
||||
|
||||
After container execution, file-based outputs can be synced back to
|
||||
the host using `sync_results_to_host`:
|
||||
|
||||
```python
|
||||
host_path = executor.sync_results_to_host("/workspace/build/output.txt")
|
||||
# => "/tmp/sandbox/project-1/build/output.txt"
|
||||
```
|
||||
|
||||
## Audit Trail
|
||||
|
||||
When a tool executes inside a container, the `ToolResult.metadata`
|
||||
dict includes a `container` key with execution details, and the
|
||||
`ToolInvocation` record stores this as the `container_metadata` field:
|
||||
|
||||
```json
|
||||
{
|
||||
"metadata": {
|
||||
"container": {
|
||||
"container_id": "abc123",
|
||||
"image": "ghcr.io/org/devimage:latest",
|
||||
"workspace_folder": "/workspace",
|
||||
"exec_time_ms": 1500.0,
|
||||
"exit_code": 0,
|
||||
"timed_out": false
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
This provides full traceability for container-routed tool executions.
|
||||
|
||||
## Security Model
|
||||
|
||||
Container tool execution enforces several layers of protection:
|
||||
|
||||
### Environment Variable Filtering
|
||||
|
||||
Only a minimal allowlist of environment variables (`PATH`, `LANG`,
|
||||
`TERM`) is forwarded to the subprocess that invokes `devcontainer
|
||||
exec`. All other host environment variables—including secrets,
|
||||
credentials, and `HOME`—are excluded.
|
||||
|
||||
### Symlink and Path Traversal Protection
|
||||
|
||||
- **Sandbox root validation**: `sync_results_to_host` resolves the
|
||||
mapped host path and verifies it falls within the sandbox root
|
||||
*before* writing any data. Path traversal via `..` is rejected.
|
||||
- **Symlink guard**: The default sandbox fallback (`/tmp/sandbox`)
|
||||
is rejected if it is a symlink. Explicit `host_sandbox_path` is
|
||||
required in that case.
|
||||
- **`O_NOFOLLOW` writes**: Files written by `sync_results_to_host`
|
||||
use `O_NOFOLLOW` to prevent symlink-following at write time.
|
||||
- **Root mapping rejection**: `PathMapper` rejects `/` and `//` as
|
||||
host or container roots to prevent mapping every absolute path.
|
||||
|
||||
### Output Caps
|
||||
|
||||
Captured stdout and stderr are bounded to 50 MiB per stream.
|
||||
Output exceeding this limit is truncated with a warning log entry.
|
||||
This prevents a misbehaving container from exhausting host memory.
|
||||
|
||||
### Permission Model
|
||||
|
||||
- Files written by `sync_results_to_host` are created with mode
|
||||
`0o600` (owner read/write only).
|
||||
- Parent directories are created with mode `0o700` (owner only).
|
||||
- The `devcontainer exec` binary is resolved at init time via
|
||||
`shutil.which` to prevent PATH hijacking. If the binary is not
|
||||
found, all execution calls fail fast with a descriptive error.
|
||||
|
||||
### Input Validation
|
||||
|
||||
- Null bytes in paths are rejected at all entry points.
|
||||
- Tool inputs are validated as JSON-serialisable before container
|
||||
execution (with `allow_nan=False` per RFC 7159).
|
||||
- JSON inputs are piped via stdin rather than shell arguments to
|
||||
avoid `ARG_MAX` limits and shell injection.
|
||||
|
||||
## ToolRunner Integration
|
||||
|
||||
The `ToolRunner` accepts an optional `container_executor` parameter:
|
||||
|
||||
```python
|
||||
from cleveragents.tool.runner import ToolRunner
|
||||
from cleveragents.tool.container_executor import (
|
||||
ContainerConfig,
|
||||
ContainerToolExecutor,
|
||||
)
|
||||
|
||||
config = ContainerConfig(...)
|
||||
executor = ContainerToolExecutor(config)
|
||||
runner = ToolRunner(
|
||||
registry=registry,
|
||||
container_executor=executor,
|
||||
)
|
||||
```
|
||||
|
||||
When the execution environment resolves to `CONTAINER` and a
|
||||
`ContainerToolExecutor` is configured, the runner delegates to it
|
||||
automatically. If no executor is configured, the runner returns an
|
||||
error result indicating that container execution is not available.
|
||||
@@ -0,0 +1,517 @@
|
||||
@container-tool-exec
|
||||
Feature: Container-aware tool execution and I/O forwarding
|
||||
As a CleverAgents developer
|
||||
I want tools to execute inside containers when the execution environment is CONTAINER
|
||||
So that file-based tools operate on the container filesystem transparently
|
||||
|
||||
Background:
|
||||
Given I have the container tool execution module imported
|
||||
|
||||
# -- PathMapper ---------------------------------------------------------
|
||||
|
||||
Scenario: PathMapper maps host path to container path
|
||||
Given I have a PathMapper with host_root "/tmp/sandbox" and container_root "/workspace"
|
||||
When I map host path "/tmp/sandbox/src/main.py" to container
|
||||
Then the container path should be "/workspace/src/main.py"
|
||||
|
||||
Scenario: PathMapper maps container path to host path
|
||||
Given I have a PathMapper with host_root "/tmp/sandbox" and container_root "/workspace"
|
||||
When I map container path "/workspace/src/main.py" to host
|
||||
Then the host path should be "/tmp/sandbox/src/main.py"
|
||||
|
||||
Scenario: PathMapper returns unmapped path when outside root
|
||||
Given I have a PathMapper with host_root "/tmp/sandbox" and container_root "/workspace"
|
||||
When I map host path "/usr/lib/libc.so" to container
|
||||
Then the container path should be "/usr/lib/libc.so"
|
||||
|
||||
Scenario: PathMapper maps root path exactly
|
||||
Given I have a PathMapper with host_root "/tmp/sandbox" and container_root "/workspace"
|
||||
When I map host path "/tmp/sandbox" to container
|
||||
Then the container path should be "/workspace"
|
||||
|
||||
Scenario: PathMapper is_host_path returns true for host paths
|
||||
Given I have a PathMapper with host_root "/tmp/sandbox" and container_root "/workspace"
|
||||
Then "/tmp/sandbox/file.txt" should be a host path
|
||||
And "/usr/lib/libc.so" should not be a host path
|
||||
|
||||
Scenario: PathMapper is_container_path returns true for container paths
|
||||
Given I have a PathMapper with host_root "/tmp/sandbox" and container_root "/workspace"
|
||||
Then "/workspace/file.txt" should be a container path
|
||||
And "/tmp/other/file.txt" should not be a container path
|
||||
|
||||
# -- ContainerConfig ----------------------------------------------------
|
||||
|
||||
Scenario: ContainerConfig has sensible defaults
|
||||
When I create a default ContainerConfig
|
||||
Then the workspace_folder should be "/workspace"
|
||||
And the container timeout_seconds should be 120
|
||||
|
||||
# -- ContainerMetadata --------------------------------------------------
|
||||
|
||||
Scenario: ContainerMetadata is frozen and holds execution details
|
||||
When I create a ContainerMetadata with container_id "abc123" and image "node:20"
|
||||
Then the container_metadata container_id should be "abc123"
|
||||
And the container_metadata image should be "node:20"
|
||||
And the container_metadata timed_out should be false
|
||||
|
||||
# -- ContainerToolExecutor basic ----------------------------------------
|
||||
|
||||
Scenario: ContainerToolExecutor can be instantiated with config
|
||||
Given I have a ContainerConfig with workspace "/workspace" and container_id "test-ctr"
|
||||
When I create a ContainerToolExecutor with that config
|
||||
Then the executor config container_id should be "test-ctr"
|
||||
And the executor should have a path mapper
|
||||
|
||||
Scenario: ContainerToolExecutor maps input paths before execution
|
||||
Given I have a ContainerToolExecutor with host_root "/tmp/sandbox" and container_root "/workspace"
|
||||
When I map tool inputs with path "/tmp/sandbox/src/file.py"
|
||||
Then the mapped input path should be "/workspace/src/file.py"
|
||||
|
||||
Scenario: ContainerToolExecutor maps output paths after execution
|
||||
Given I have a ContainerToolExecutor with host_root "/tmp/sandbox" and container_root "/workspace"
|
||||
When I map tool output with path "/workspace/build/output.txt"
|
||||
Then the mapped output path should be "/tmp/sandbox/build/output.txt"
|
||||
|
||||
Scenario: ContainerToolExecutor handles nested input path mapping
|
||||
Given I have a ContainerToolExecutor with host_root "/tmp/sandbox" and container_root "/workspace"
|
||||
When I map tool inputs with nested paths under "/tmp/sandbox"
|
||||
Then all nested paths should be mapped to container paths
|
||||
|
||||
# -- Timeout handling ---------------------------------------------------
|
||||
|
||||
Scenario: ContainerToolExecutor reports timeout as structured error
|
||||
Given I have a ContainerToolExecutor with a mock that times out
|
||||
When I execute tool "file_read" with inputs in the container
|
||||
Then the container tool result should indicate failure
|
||||
And the container tool result error should mention "timed out"
|
||||
And the tool result metadata should contain container info with timed_out true
|
||||
|
||||
# -- Error reporting ----------------------------------------------------
|
||||
|
||||
Scenario: ContainerToolExecutor reports non-zero exit as structured error
|
||||
Given I have a ContainerToolExecutor with a mock that returns exit code 1
|
||||
When I execute tool "file_read" with inputs in the container
|
||||
Then the container tool result should indicate failure
|
||||
And the container tool result error should mention "exit code 1"
|
||||
|
||||
Scenario: ContainerToolExecutor reports OSError as structured error
|
||||
Given I have a ContainerToolExecutor with subprocess raising OSError
|
||||
When I execute tool "compile" with inputs in the container
|
||||
Then the container tool result should indicate failure
|
||||
|
||||
# -- Successful execution -----------------------------------------------
|
||||
|
||||
Scenario: ContainerToolExecutor returns parsed JSON output on success
|
||||
Given I have a ContainerToolExecutor with a mock that returns JSON output
|
||||
When I execute tool "lint" with inputs in the container
|
||||
Then the container tool result should indicate success
|
||||
And the tool result output should contain the parsed JSON
|
||||
And the tool result metadata should contain container info
|
||||
|
||||
Scenario: ContainerToolExecutor returns raw output when not JSON
|
||||
Given I have a ContainerToolExecutor with a mock that returns plain text
|
||||
When I execute tool "echo" with inputs in the container
|
||||
Then the container tool result should indicate success
|
||||
And the tool result output should contain raw_output
|
||||
|
||||
# -- Command building ---------------------------------------------------
|
||||
|
||||
Scenario: _build_exec_command places --container-id flag adjacent to its value
|
||||
Given I have a ContainerToolExecutor with host_root "/tmp/sandbox" and container_root "/workspace"
|
||||
When I build an exec command for tool "test_tool" with timeout 30
|
||||
Then the command should have "--container-id" adjacent to its value
|
||||
|
||||
# -- ToolRunner container routing ---------------------------------------
|
||||
|
||||
Scenario: ToolRunner delegates to ContainerToolExecutor when env is CONTAINER
|
||||
Given I have a ToolRunner with a ContainerToolExecutor mock
|
||||
When I execute a tool with container execution environment
|
||||
Then the ContainerToolExecutor should have been called
|
||||
And the resolver should have been called with the correct arguments
|
||||
|
||||
Scenario: ToolRunner returns error when container executor is not configured
|
||||
Given I have a ToolRunner without a ContainerToolExecutor
|
||||
When I execute a tool with container execution environment
|
||||
Then the container tool result error should mention "ContainerToolExecutor must be configured"
|
||||
|
||||
# -- ToolInvocation audit trail -----------------------------------------
|
||||
|
||||
Scenario: ToolInvocation has container_metadata field
|
||||
When I create a ToolInvocation with container_metadata
|
||||
Then the ToolInvocation container_metadata should contain "container_id"
|
||||
And the ToolInvocation container_metadata should contain "image"
|
||||
|
||||
Scenario: ToolInvocation container_metadata defaults to None
|
||||
When I create a ToolInvocation without container_metadata
|
||||
Then the ToolInvocation container_metadata should be None
|
||||
|
||||
# -- ContainerExecutionError --------------------------------------------
|
||||
|
||||
Scenario: ContainerExecutionError carries exit_code and stderr
|
||||
When I raise a ContainerExecutionError with exit_code 2 and stderr "oops"
|
||||
Then the error exit_code should be 2
|
||||
And the error stderr should be "oops"
|
||||
|
||||
Scenario: ContainerTimeoutError carries timeout_seconds
|
||||
When I raise a ContainerTimeoutError with timeout 30
|
||||
Then the timeout error message should mention "30"
|
||||
And the timeout error timed_out should be true
|
||||
|
||||
# -- Signal exit code preservation ----------------------------------------
|
||||
|
||||
Scenario: ContainerToolExecutor preserves negative exit codes from signal kills
|
||||
Given I have a ContainerToolExecutor with a mock that returns exit code -9
|
||||
When I execute tool "compile" with inputs in the container
|
||||
Then the container tool result should indicate failure
|
||||
And the tool result metadata should have exit_code -9
|
||||
|
||||
# -- Path mapping with spaces --------------------------------------------
|
||||
|
||||
Scenario: _looks_like_path accepts paths with spaces
|
||||
Given I have a ContainerToolExecutor with host_root "/tmp/sandbox" and container_root "/workspace"
|
||||
When I map tool inputs with path "/tmp/sandbox/My Documents/file.py"
|
||||
Then the mapped input path should be "/workspace/My Documents/file.py"
|
||||
|
||||
# -- Output truncation ---------------------------------------------------
|
||||
|
||||
Scenario: ContainerToolExecutor truncates output exceeding max bytes
|
||||
Given I have a ContainerToolExecutor with a mock that returns oversized output
|
||||
When I execute tool "big_output" with inputs in the container
|
||||
Then the container tool result stdout should be truncated
|
||||
|
||||
# -- Path traversal protection -------------------------------------------
|
||||
|
||||
Scenario: sync_results_to_host rejects path traversal attempts
|
||||
Given I have a ContainerToolExecutor with a mock that succeeds on sync using a temp dir
|
||||
When I attempt to sync a path that traverses outside the sandbox
|
||||
Then a ContainerExecutionError should be raised for path traversal
|
||||
|
||||
# -- Sync results -------------------------------------------------------
|
||||
|
||||
Scenario: sync_results_to_host maps container path to host
|
||||
Given I have a ContainerToolExecutor with a mock that succeeds on sync using a temp dir
|
||||
When I sync container path "/workspace/output.txt" to host
|
||||
Then the synced host path should be under the host sandbox root
|
||||
|
||||
# -- execute_tool input validation (P2-19) --------------------------------
|
||||
|
||||
Scenario: execute_tool rejects empty tool_name
|
||||
Given I have a ContainerToolExecutor with host_root "/tmp/sandbox" and container_root "/workspace"
|
||||
When I execute tool with empty tool_name
|
||||
Then a ValueError should be raised with message "non-empty"
|
||||
|
||||
Scenario: execute_tool rejects non-dict inputs
|
||||
Given I have a ContainerToolExecutor with host_root "/tmp/sandbox" and container_root "/workspace"
|
||||
When I execute tool with non-dict inputs
|
||||
Then a container TypeError should be raised with message "must be a dict"
|
||||
|
||||
Scenario: execute_tool rejects negative timeout_seconds
|
||||
Given I have a ContainerToolExecutor with host_root "/tmp/sandbox" and container_root "/workspace"
|
||||
When I execute tool with negative timeout
|
||||
Then a ValueError should be raised with message "must be > 0"
|
||||
|
||||
# -- extract_container_metadata helper (P1-9) -----------------------------
|
||||
|
||||
Scenario: extract_container_metadata returns container dict from ToolResult
|
||||
When I create a ToolResult with container metadata in metadata dict
|
||||
Then extract_container_metadata should return the container dict
|
||||
|
||||
# =========================================================================
|
||||
# Safe initialization — PathMapper validation guards
|
||||
# =========================================================================
|
||||
|
||||
Scenario: PathMapper rejects null bytes in host_root
|
||||
When I try to create a PathMapper with null bytes in host_root
|
||||
Then a PathMapper ValueError should be raised mentioning "null"
|
||||
|
||||
Scenario: PathMapper rejects null bytes in container_root
|
||||
When I try to create a PathMapper with null bytes in container_root
|
||||
Then a PathMapper ValueError should be raised mentioning "null"
|
||||
|
||||
Scenario: PathMapper rejects empty host_root
|
||||
When I try to create a PathMapper with empty host_root
|
||||
Then a PathMapper ValueError should be raised mentioning "empty"
|
||||
|
||||
Scenario: PathMapper rejects filesystem-root host_root
|
||||
When I try to create a PathMapper with host_root "/"
|
||||
Then a PathMapper ValueError should be raised mentioning "/"
|
||||
|
||||
Scenario: PathMapper rejects filesystem-root container_root
|
||||
When I try to create a PathMapper with container_root "/"
|
||||
Then a PathMapper ValueError should be raised mentioning "/"
|
||||
|
||||
Scenario: PathMapper rejects overlapping roots
|
||||
When I try to create a PathMapper with overlapping roots
|
||||
Then a PathMapper ValueError should be raised mentioning "overlap"
|
||||
|
||||
Scenario: PathMapper is_host_path returns false for null-byte paths
|
||||
Given I have a PathMapper with host_root "/tmp/sandbox" and container_root "/workspace"
|
||||
Then a path with null bytes should not be a host path
|
||||
|
||||
Scenario: PathMapper is_container_path returns false for null-byte paths
|
||||
Given I have a PathMapper with host_root "/tmp/sandbox" and container_root "/workspace"
|
||||
Then a path with null bytes should not be a container path
|
||||
|
||||
# =========================================================================
|
||||
# Safe initialization — ContainerConfig workspace_folder guards
|
||||
# =========================================================================
|
||||
|
||||
Scenario: ContainerConfig rejects relative workspace_folder
|
||||
When I try to create a ContainerConfig with workspace_folder "relative/path"
|
||||
Then a ContainerConfig ValueError should be raised mentioning "absolute"
|
||||
|
||||
Scenario: ContainerConfig rejects workspace_folder that normalises to root
|
||||
When I try to create a ContainerConfig with workspace_folder "/."
|
||||
Then a ContainerConfig ValueError should be raised mentioning "root"
|
||||
|
||||
Scenario: ContainerConfig rejects workspace_folder with dotdot traversal
|
||||
When I try to create a ContainerConfig with workspace_folder "/workspace/../etc"
|
||||
Then a ContainerConfig ValueError should be raised mentioning ".."
|
||||
|
||||
# =========================================================================
|
||||
# Safe initialization — ContainerToolExecutor binary resolution
|
||||
# =========================================================================
|
||||
|
||||
Scenario: ContainerToolExecutor without devcontainer binary uses workspace-folder target args
|
||||
Given I have a ContainerToolExecutor with no container_id configured
|
||||
When I request the target CLI arguments
|
||||
Then the target args should use "--workspace-folder"
|
||||
|
||||
Scenario: ContainerToolExecutor raises when devcontainer binary is missing at execution
|
||||
Given I have a ContainerToolExecutor with no devcontainer binary
|
||||
When I try to require the devcontainer binary
|
||||
Then a ContainerExecutionError should be raised about missing binary
|
||||
|
||||
# =========================================================================
|
||||
# Cleanup — sync_results_to_host error paths
|
||||
# =========================================================================
|
||||
|
||||
Scenario: sync_results_to_host rejects null bytes in container_path
|
||||
Given I have a ContainerToolExecutor with a mock that succeeds on sync using a temp dir
|
||||
When I try to sync a path containing null bytes
|
||||
Then a sync ValueError should be raised mentioning "null"
|
||||
|
||||
Scenario: sync_results_to_host rejects relative container_path
|
||||
Given I have a ContainerToolExecutor with a mock that succeeds on sync using a temp dir
|
||||
When I try to sync a relative container path
|
||||
Then a sync ValueError should be raised mentioning "absolute"
|
||||
|
||||
Scenario: sync_results_to_host raises on timeout during file sync
|
||||
Given I have a ContainerToolExecutor with a mock that times out on sync
|
||||
When I try to sync container path to host with timeout
|
||||
Then a ContainerTimeoutError should be raised from sync
|
||||
|
||||
Scenario: sync_results_to_host raises on non-zero exit during file sync
|
||||
Given I have a ContainerToolExecutor with a mock that returns non-zero exit on sync
|
||||
When I try to sync container path to host with failure
|
||||
Then a ContainerExecutionError should be raised from sync failure
|
||||
|
||||
Scenario: sync_results_to_host raises on OSError during file write
|
||||
Given I have a ContainerToolExecutor with a mock that triggers OSError on write
|
||||
When I try to sync container path to host triggering write error
|
||||
Then a ContainerExecutionError should be raised from write failure
|
||||
|
||||
# =========================================================================
|
||||
# Safe execution — _looks_like_path heuristic boundary checks
|
||||
# =========================================================================
|
||||
|
||||
Scenario: _looks_like_path rejects strings with newlines
|
||||
When I check whether a string with newlines looks like a path
|
||||
Then it should not look like a path
|
||||
|
||||
Scenario: _looks_like_path rejects strings with null bytes
|
||||
When I check whether a string with null bytes looks like a path
|
||||
Then it should not look like a path
|
||||
|
||||
Scenario: _looks_like_path rejects protocol-relative URIs
|
||||
When I check whether a protocol-relative URI looks like a path
|
||||
Then it should not look like a path
|
||||
|
||||
Scenario: _looks_like_path rejects strings with query parameters
|
||||
When I check whether a path with query string looks like a path
|
||||
Then it should not look like a path
|
||||
|
||||
Scenario: _looks_like_path rejects strings with fragment identifiers
|
||||
When I check whether a path with fragment identifier looks like a path
|
||||
Then it should not look like a path
|
||||
|
||||
Scenario: _looks_like_path rejects strings with scheme separators
|
||||
When I check whether a URL-like string looks like a path
|
||||
Then it should not look like a path
|
||||
|
||||
# =========================================================================
|
||||
# Safe execution — recursion depth guards on path mapping
|
||||
# =========================================================================
|
||||
|
||||
Scenario: _map_input_paths stops at max recursion depth
|
||||
Given I have a ContainerToolExecutor with host_root "/tmp/sandbox" and container_root "/workspace"
|
||||
When I map deeply nested input paths exceeding the recursion limit
|
||||
Then the input mapping should return without error
|
||||
|
||||
Scenario: _map_output_paths stops at max recursion depth
|
||||
Given I have a ContainerToolExecutor with host_root "/tmp/sandbox" and container_root "/workspace"
|
||||
When I map deeply nested output paths exceeding the recursion limit
|
||||
Then the output mapping should return without error
|
||||
|
||||
# =========================================================================
|
||||
# Safe execution — _parse_output edge cases
|
||||
# =========================================================================
|
||||
|
||||
Scenario: _parse_output returns result key for non-dict JSON
|
||||
When I parse stdout containing a JSON array
|
||||
Then the parsed output should wrap it under "result" key
|
||||
|
||||
Scenario: _parse_output returns empty dict for blank stdout
|
||||
When I parse empty stdout
|
||||
Then the parsed output should be an empty dict
|
||||
|
||||
# =========================================================================
|
||||
# Safe execution — _map_output_paths skips raw_output key
|
||||
# =========================================================================
|
||||
|
||||
Scenario: _map_output_paths preserves raw_output without path mapping
|
||||
Given I have a ContainerToolExecutor with host_root "/tmp/sandbox" and container_root "/workspace"
|
||||
When I map output containing a raw_output key with container path
|
||||
Then the raw_output value should remain unmapped
|
||||
|
||||
# =========================================================================
|
||||
# Safe execution — container tool non-JSON-serialisable inputs
|
||||
# =========================================================================
|
||||
|
||||
Scenario: execute_tool returns error for non-JSON-serialisable inputs
|
||||
Given I have a ContainerToolExecutor with host_root "/tmp/sandbox" and container_root "/workspace"
|
||||
When I execute a tool with non-serialisable inputs
|
||||
Then the container tool result should indicate failure
|
||||
And the container tool result error should mention "JSON-serialisable"
|
||||
|
||||
# =========================================================================
|
||||
# ToolRunner full lifecycle — discover, activate, deactivate
|
||||
# =========================================================================
|
||||
|
||||
Scenario: ToolRunner discover returns tools from registry
|
||||
Given I have a ToolRunner with a populated registry
|
||||
When I call discover on the runner
|
||||
Then the discovered tools list should not be empty
|
||||
|
||||
Scenario: ToolRunner activate marks a tool as ready
|
||||
Given I have a ToolRunner with a populated registry
|
||||
When I activate a tool on the runner
|
||||
Then the activated tool spec should be returned
|
||||
|
||||
Scenario: ToolRunner activate raises ToolError for unknown tool
|
||||
Given I have a ToolRunner with a populated registry
|
||||
When I try to activate an unknown tool
|
||||
Then a ToolError should be raised for activation
|
||||
|
||||
Scenario: ToolRunner deactivate cleans up active tool
|
||||
Given I have a ToolRunner with a populated registry
|
||||
When I activate and then deactivate a tool
|
||||
Then deactivate should return true
|
||||
And a second deactivate should return false
|
||||
|
||||
# =========================================================================
|
||||
# ToolRunner host execution path
|
||||
# =========================================================================
|
||||
|
||||
Scenario: ToolRunner executes tool on host and returns success
|
||||
Given I have a ToolRunner with a host-routed tool
|
||||
When I execute the host tool with valid inputs
|
||||
Then the host tool result should indicate success
|
||||
And the host tool result output should contain handler data
|
||||
|
||||
Scenario: ToolRunner normalises non-dict handler output
|
||||
Given I have a ToolRunner with a tool returning non-dict output
|
||||
When I execute the non-dict-output tool
|
||||
Then the result output should wrap the value under "result"
|
||||
|
||||
Scenario: ToolRunner catches handler exception and returns error
|
||||
Given I have a ToolRunner with a tool that raises an exception
|
||||
When I execute the raising tool
|
||||
Then the tool result should indicate failure with exception info
|
||||
|
||||
Scenario: ToolRunner returns error for non-JSON-serialisable host output
|
||||
Given I have a ToolRunner with a tool returning non-serialisable output
|
||||
When I execute the non-serialisable-output tool
|
||||
Then the tool result should indicate failure mentioning "not JSON-serialisable"
|
||||
|
||||
Scenario: ToolRunner returns error for non-JSON-serialisable host inputs
|
||||
Given I have a ToolRunner with a host-routed tool
|
||||
When I execute the host tool with non-serialisable inputs
|
||||
Then the tool result should indicate failure mentioning "not JSON-serialisable"
|
||||
|
||||
Scenario: ToolRunner raises ToolError for unregistered tool in execute
|
||||
Given I have a ToolRunner with an empty registry
|
||||
When I try to execute an unregistered tool
|
||||
Then a ToolError should be raised for execution
|
||||
|
||||
# =========================================================================
|
||||
# ToolRunner container routing — additional edge cases
|
||||
# =========================================================================
|
||||
|
||||
Scenario: ToolRunner returns error when container env resolver raises ContainerUnavailableError
|
||||
Given I have a ToolRunner with an unavailable-container resolver
|
||||
When I execute a tool through the unavailable-container runner
|
||||
Then the tool result should indicate container unavailable
|
||||
|
||||
Scenario: ToolRunner validates missing required input fields for container tools
|
||||
Given I have a ToolRunner with a container executor and required-field tool
|
||||
When I execute the container tool with missing required fields
|
||||
Then the tool result error should mention "Missing required input fields"
|
||||
|
||||
Scenario: ToolRunner returns error for non-JSON-serialisable container inputs
|
||||
Given I have a ToolRunner with a container executor for JSON validation
|
||||
When I execute a container tool with non-serialisable inputs
|
||||
Then the container tool result error should mention "not JSON-serialisable"
|
||||
|
||||
Scenario: ToolRunner catches container executor exception
|
||||
Given I have a ToolRunner with a failing container executor
|
||||
When I execute a tool through the failing container runner
|
||||
Then the tool result error should mention "Container execution error"
|
||||
|
||||
# =========================================================================
|
||||
# Safe execution — list-type value mapping branches
|
||||
# =========================================================================
|
||||
|
||||
Scenario: _map_input_paths handles list values containing host paths
|
||||
Given I have a ContainerToolExecutor with host_root "/tmp/sandbox" and container_root "/workspace"
|
||||
When I map tool inputs containing a list of host paths
|
||||
Then the list values should be mapped to container paths
|
||||
|
||||
Scenario: _map_output_paths handles list values containing container paths
|
||||
Given I have a ContainerToolExecutor with host_root "/tmp/sandbox" and container_root "/workspace"
|
||||
When I map tool output containing a list of container paths
|
||||
Then the list values should be mapped to host paths
|
||||
|
||||
# =========================================================================
|
||||
# Safe execution — PathMapper null byte rejection in mapping methods
|
||||
# =========================================================================
|
||||
|
||||
Scenario: PathMapper host_to_container rejects null bytes
|
||||
Given I have a PathMapper with host_root "/tmp/sandbox" and container_root "/workspace"
|
||||
When I try to map a host path with null bytes to container
|
||||
Then a mapping ValueError should be raised about null bytes
|
||||
|
||||
Scenario: PathMapper container_to_host rejects null bytes
|
||||
Given I have a PathMapper with host_root "/tmp/sandbox" and container_root "/workspace"
|
||||
When I try to map a container path with null bytes to host
|
||||
Then a mapping ValueError should be raised about null bytes
|
||||
|
||||
# =========================================================================
|
||||
# ToolResult validation — success/error consistency
|
||||
# =========================================================================
|
||||
|
||||
Scenario: ToolResult rejects success=True with error message
|
||||
When I try to create a ToolResult with success true and an error
|
||||
Then a ToolResult validation error should be raised
|
||||
|
||||
Scenario: ToolResult rejects success=False without error message
|
||||
When I try to create a ToolResult with success false and no error
|
||||
Then a ToolResult validation error should be raised
|
||||
|
||||
# =========================================================================
|
||||
# ToolError structured attributes
|
||||
# =========================================================================
|
||||
|
||||
Scenario: ToolError carries structured context attributes
|
||||
When I create a ToolError with known attributes
|
||||
Then the ToolError should expose tool_name, error_type, and details
|
||||
@@ -280,8 +280,12 @@ def step_start_changeset(context, plan_id):
|
||||
@when("I record a create entry in the changeset")
|
||||
def step_record_create_entry(context):
|
||||
"""Record a create entry."""
|
||||
# Use the plan_id from the changeset so the entry matches
|
||||
# (InMemoryChangeSetStore.record now validates via add_change).
|
||||
cs = context.store.get(context.last_cs_id)
|
||||
plan_id = cs.plan_id if cs is not None else "plan-abc"
|
||||
entry = ChangeEntry(
|
||||
plan_id="plan-abc",
|
||||
plan_id=plan_id,
|
||||
resource_id="res-1",
|
||||
tool_name="builtin/file-write",
|
||||
operation=ChangeOperation.CREATE,
|
||||
|
||||
File diff suppressed because it is too large
Load Diff
@@ -104,14 +104,30 @@ def step_builtin_registry(context: Context) -> None:
|
||||
|
||||
@when("I create a devcontainer-instance resource at that path")
|
||||
def step_create_dc_resource(context: Context) -> None:
|
||||
# Verify the type is actually recognised by the built-in registry
|
||||
# instead of just assigning a string and asserting it back (U7).
|
||||
context.resource_type = "devcontainer-instance"
|
||||
context.resource_id = "01TESTDEVCONTAINER00000000"
|
||||
assert context.resource_type in ResourceTypeSpec.BUILTIN_NAMES, (
|
||||
f"'{context.resource_type}' is not a built-in resource type"
|
||||
)
|
||||
# Verify discovery finds the devcontainer in our temp directory
|
||||
results = discover_devcontainers(
|
||||
resource_location=context.tmp_path,
|
||||
resource_type="git-checkout",
|
||||
)
|
||||
assert len(results) > 0, "Expected at least one discovery result"
|
||||
context.resource_id = f"01TEST{results[0].config_path.name}"
|
||||
context.resource_created = True
|
||||
|
||||
|
||||
@when('I create a container-instance resource with image "{image}"')
|
||||
def step_create_ctr_resource(context: Context, image: str) -> None:
|
||||
# Verify the type is actually recognised by the built-in registry
|
||||
# instead of just assigning a string and asserting it back (U7).
|
||||
context.container_type = "container-instance"
|
||||
assert context.container_type in ResourceTypeSpec.BUILTIN_NAMES, (
|
||||
f"'{context.container_type}' is not a built-in resource type"
|
||||
)
|
||||
context.container_image = image
|
||||
context.container_created = True
|
||||
|
||||
|
||||
@@ -217,4 +217,4 @@ Feature: Tool wrapping runtime
|
||||
Given a ToolRunner with a container-returning env resolver
|
||||
When I execute a tool through the runner with container env
|
||||
Then the tool result should have success false
|
||||
And the tool result error should contain "Container execution is not yet implemented"
|
||||
And the tool result error should contain "Container execution is not available"
|
||||
|
||||
@@ -0,0 +1,157 @@
|
||||
*** Settings ***
|
||||
Documentation Integration tests for container-aware tool execution (#515)
|
||||
Resource ${CURDIR}/common.resource
|
||||
Suite Setup Setup Test Environment
|
||||
Suite Teardown Cleanup Test Environment
|
||||
|
||||
*** Variables ***
|
||||
${TIMEOUT} 60s
|
||||
|
||||
*** Test Cases ***
|
||||
PathMapper Maps Host To Container
|
||||
[Documentation] Verify PathMapper translates host paths to container paths
|
||||
${script}= Catenate SEPARATOR=\n
|
||||
... from cleveragents.tool.path_mapper import PathMapper
|
||||
... m = PathMapper('/tmp/sandbox', '/workspace')
|
||||
... result = m.host_to_container('/tmp/sandbox/src/f.py')
|
||||
... assert result == '/workspace/src/f.py', f"got {result}"
|
||||
... print("OK")
|
||||
${result}= Run Process ${PYTHON} -c ${script}
|
||||
... timeout=${TIMEOUT} env:PYTHONPATH=${CURDIR}/../src
|
||||
Should Be Equal As Integers ${result.rc} 0 PathMapper host_to_container failed: ${result.stderr}
|
||||
Should Be Equal As Strings ${result.stdout.strip()} OK
|
||||
|
||||
PathMapper Maps Container To Host
|
||||
[Documentation] Verify PathMapper translates container paths to host paths
|
||||
${script}= Catenate SEPARATOR=\n
|
||||
... from cleveragents.tool.path_mapper import PathMapper
|
||||
... m = PathMapper('/tmp/sandbox', '/workspace')
|
||||
... result = m.container_to_host('/workspace/src/f.py')
|
||||
... assert result == '/tmp/sandbox/src/f.py', f"got {result}"
|
||||
... print("OK")
|
||||
${result}= Run Process ${PYTHON} -c ${script}
|
||||
... timeout=${TIMEOUT} env:PYTHONPATH=${CURDIR}/../src
|
||||
Should Be Equal As Integers ${result.rc} 0 PathMapper container_to_host failed: ${result.stderr}
|
||||
Should Be Equal As Strings ${result.stdout.strip()} OK
|
||||
|
||||
PathMapper Leaves External Paths Unmapped
|
||||
[Documentation] Paths outside root are returned unchanged
|
||||
${script}= Catenate SEPARATOR=\n
|
||||
... from cleveragents.tool.path_mapper import PathMapper
|
||||
... m = PathMapper('/tmp/sandbox', '/workspace')
|
||||
... result = m.host_to_container('/usr/lib/x')
|
||||
... assert result == '/usr/lib/x', f"got {result}"
|
||||
... print("OK")
|
||||
${result}= Run Process ${PYTHON} -c ${script}
|
||||
... timeout=${TIMEOUT} env:PYTHONPATH=${CURDIR}/../src
|
||||
Should Be Equal As Integers ${result.rc} 0 PathMapper external path mapping failed: ${result.stderr}
|
||||
Should Be Equal As Strings ${result.stdout.strip()} OK
|
||||
|
||||
ContainerConfig Has Defaults
|
||||
[Documentation] ContainerConfig defaults to /workspace and 120s timeout
|
||||
${script}= Catenate SEPARATOR=\n
|
||||
... from cleveragents.tool.container_executor import ContainerConfig
|
||||
... c = ContainerConfig()
|
||||
... assert c.workspace_folder == '/workspace', f"got {c.workspace_folder}"
|
||||
... assert c.timeout_seconds == 120, f"got {c.timeout_seconds}"
|
||||
... print("OK")
|
||||
${result}= Run Process ${PYTHON} -c ${script}
|
||||
... timeout=${TIMEOUT} env:PYTHONPATH=${CURDIR}/../src
|
||||
Should Be Equal As Integers ${result.rc} 0 ContainerConfig defaults test failed: ${result.stderr}
|
||||
Should Be Equal As Strings ${result.stdout.strip()} OK
|
||||
|
||||
ContainerMetadata Is Frozen
|
||||
[Documentation] ContainerMetadata is a frozen Pydantic model
|
||||
${script}= Catenate SEPARATOR=\n
|
||||
... from cleveragents.tool.container_executor import ContainerMetadata
|
||||
... m = ContainerMetadata(container_id='x', image='y')
|
||||
... assert m.container_id == 'x', f"got {m.container_id}"
|
||||
... assert m.image == 'y', f"got {m.image}"
|
||||
... assert m.timed_out is False
|
||||
... print("OK")
|
||||
${result}= Run Process ${PYTHON} -c ${script}
|
||||
... timeout=${TIMEOUT} env:PYTHONPATH=${CURDIR}/../src
|
||||
Should Be Equal As Integers ${result.rc} 0 ContainerMetadata test failed: ${result.stderr}
|
||||
Should Be Equal As Strings ${result.stdout.strip()} OK
|
||||
|
||||
ContainerToolExecutor Instantiation
|
||||
[Documentation] ContainerToolExecutor can be created with ContainerConfig
|
||||
${script}= Catenate SEPARATOR=\n
|
||||
... import structlog, sys; structlog.configure(logger_factory=structlog.PrintLoggerFactory(file=sys.stderr))
|
||||
... from cleveragents.tool.container_executor import ContainerConfig, ContainerToolExecutor
|
||||
... c = ContainerConfig(container_id='t')
|
||||
... e = ContainerToolExecutor(c)
|
||||
... assert e.config.container_id == 't', f"got {e.config.container_id}"
|
||||
... assert e.path_mapper is not None
|
||||
... print("OK")
|
||||
${result}= Run Process ${PYTHON} -c ${script}
|
||||
... timeout=${TIMEOUT} env:PYTHONPATH=${CURDIR}/../src
|
||||
Should Be Equal As Integers ${result.rc} 0 Executor instantiation failed: ${result.stderr}
|
||||
Should Be Equal As Strings ${result.stdout.strip()} OK
|
||||
|
||||
ContainerExecutionError Carries Details
|
||||
[Documentation] ContainerExecutionError stores exit_code and stderr
|
||||
${script}= Catenate SEPARATOR=\n
|
||||
... from cleveragents.tool.container_executor import ContainerExecutionError
|
||||
... e = ContainerExecutionError('fail', exit_code=2, stderr='bad')
|
||||
... assert e.exit_code == 2, f"got {e.exit_code}"
|
||||
... assert e.stderr == 'bad', f"got {e.stderr}"
|
||||
... print("OK")
|
||||
${result}= Run Process ${PYTHON} -c ${script}
|
||||
... timeout=${TIMEOUT} env:PYTHONPATH=${CURDIR}/../src
|
||||
Should Be Equal As Integers ${result.rc} 0 ContainerExecutionError test failed: ${result.stderr}
|
||||
Should Be Equal As Strings ${result.stdout.strip()} OK
|
||||
|
||||
ContainerTimeoutError Carries Timeout
|
||||
[Documentation] ContainerTimeoutError records timeout_seconds
|
||||
${script}= Catenate SEPARATOR=\n
|
||||
... from cleveragents.tool.container_executor import ContainerTimeoutError
|
||||
... e = ContainerTimeoutError(30)
|
||||
... assert e.timeout_seconds == 30, f"got {e.timeout_seconds}"
|
||||
... assert e.timed_out is True
|
||||
... assert '30' in str(e)
|
||||
... print("OK")
|
||||
${result}= Run Process ${PYTHON} -c ${script}
|
||||
... timeout=${TIMEOUT} env:PYTHONPATH=${CURDIR}/../src
|
||||
Should Be Equal As Integers ${result.rc} 0 ContainerTimeoutError test failed: ${result.stderr}
|
||||
Should Be Equal As Strings ${result.stdout.strip()} OK
|
||||
|
||||
ToolInvocation Has ContainerMetadata Field
|
||||
[Documentation] ToolInvocation accepts container_metadata dict
|
||||
${script}= Catenate SEPARATOR=\n
|
||||
... from cleveragents.domain.models.core.change import ToolInvocation
|
||||
... t = ToolInvocation(
|
||||
... plan_id='01TESTPLANID000000000000000',
|
||||
... tool_name='ns/t',
|
||||
... container_metadata={'container_id': 'x'},
|
||||
... )
|
||||
... assert t.container_metadata['container_id'] == 'x'
|
||||
... print("OK")
|
||||
${result}= Run Process ${PYTHON} -c ${script}
|
||||
... timeout=${TIMEOUT} env:PYTHONPATH=${CURDIR}/../src
|
||||
Should Be Equal As Integers ${result.rc} 0 ToolInvocation container_metadata test failed: ${result.stderr}
|
||||
Should Be Equal As Strings ${result.stdout.strip()} OK
|
||||
|
||||
ToolRunner Container Routing Without Executor Returns Error
|
||||
[Documentation] ToolRunner without ContainerToolExecutor returns error for container env
|
||||
${script}= Catenate SEPARATOR=\n
|
||||
... from unittest.mock import MagicMock
|
||||
... from cleveragents.tool.runner import ToolRunner
|
||||
... from cleveragents.tool.registry import ToolRegistry
|
||||
... from cleveragents.tool.runtime import ToolSpec
|
||||
... from cleveragents.domain.models.core.plan import ExecutionEnvironment
|
||||
... from cleveragents.application.services.execution_environment_resolver import ExecutionEnvironmentResolver
|
||||
... r = MagicMock(spec=ToolRegistry)
|
||||
... s = ToolSpec(name='t', description='d', input_schema={}, handler=lambda x: x)
|
||||
... r.get.return_value = s
|
||||
... ev = MagicMock(spec=ExecutionEnvironmentResolver)
|
||||
... ev.resolve_and_validate.return_value = ExecutionEnvironment.CONTAINER
|
||||
... runner = ToolRunner(registry=r, execution_environment_resolver=ev)
|
||||
... res = runner.execute('t', {})
|
||||
... assert not res.success, f"Expected failure but got success"
|
||||
... assert 'ContainerToolExecutor' in res.error, f"Expected ContainerToolExecutor in error: {res.error}"
|
||||
... print("OK")
|
||||
${result}= Run Process ${PYTHON} -c ${script}
|
||||
... timeout=${TIMEOUT} env:PYTHONPATH=${CURDIR}/../src
|
||||
Should Be Equal As Integers ${result.rc} 0 ToolRunner container routing test failed: ${result.stderr}
|
||||
Should Be Equal As Strings ${result.stdout.strip()} OK
|
||||
@@ -472,6 +472,14 @@ class ToolInvocation(BaseModel):
|
||||
default=None,
|
||||
description="Provider info (model name, latency, etc.)",
|
||||
)
|
||||
container_metadata: dict[str, Any] | None = Field(
|
||||
default=None,
|
||||
description=(
|
||||
"Container execution metadata when tool ran inside a container. "
|
||||
"Includes container_id, image, workspace_folder, exec_time_ms, "
|
||||
"exit_code, and timed_out."
|
||||
),
|
||||
)
|
||||
|
||||
model_config = ConfigDict(
|
||||
str_strip_whitespace=True,
|
||||
@@ -600,7 +608,7 @@ class InMemoryChangeSetStore:
|
||||
cs = self._store.get(changeset_id)
|
||||
if cs is None:
|
||||
raise KeyError(f"ChangeSet '{changeset_id}' not found")
|
||||
cs.entries.append(entry)
|
||||
cs.add_change(entry)
|
||||
|
||||
def get(self, changeset_id: str) -> SpecChangeSet | None:
|
||||
"""Return a changeset by ID or ``None``."""
|
||||
|
||||
@@ -11,6 +11,7 @@ All repositories follow the session-factory pattern (ADR-007).
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import logging
|
||||
from collections.abc import Callable
|
||||
from datetime import datetime
|
||||
from typing import Any, cast
|
||||
@@ -35,6 +36,32 @@ from cleveragents.infrastructure.database.models import (
|
||||
ToolInvocationModel,
|
||||
)
|
||||
|
||||
_logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
def _safe_json_loads(
|
||||
raw: str | None,
|
||||
*,
|
||||
column: str = "",
|
||||
default: Any = None,
|
||||
) -> Any:
|
||||
"""Deserialise a JSON string from the database.
|
||||
|
||||
Returns *default* (instead of raising) when the stored data is
|
||||
corrupt or not valid JSON so that a single bad row does not break
|
||||
bulk reads.
|
||||
"""
|
||||
if not raw:
|
||||
return default
|
||||
try:
|
||||
return json.loads(raw)
|
||||
except (json.JSONDecodeError, TypeError, ValueError):
|
||||
_logger.warning(
|
||||
"corrupt_json_column",
|
||||
extra={"column": column, "raw_length": len(raw)},
|
||||
)
|
||||
return default
|
||||
|
||||
|
||||
class ChangeSetEntryRepository:
|
||||
"""Repository for persisting ChangeEntry records.
|
||||
@@ -221,7 +248,7 @@ class ToolInvocationRepository:
|
||||
if invocation.result is not None:
|
||||
result_json = json.dumps(
|
||||
invocation.result,
|
||||
default=str,
|
||||
allow_nan=False,
|
||||
)
|
||||
|
||||
completed_iso: str | None = None
|
||||
@@ -232,7 +259,14 @@ class ToolInvocationRepository:
|
||||
if invocation.provider_metadata is not None:
|
||||
pm_json = json.dumps(
|
||||
invocation.provider_metadata,
|
||||
default=str,
|
||||
allow_nan=False,
|
||||
)
|
||||
|
||||
cm_json: str | None = None
|
||||
if invocation.container_metadata is not None:
|
||||
cm_json = json.dumps(
|
||||
invocation.container_metadata,
|
||||
allow_nan=False,
|
||||
)
|
||||
|
||||
model = ToolInvocationModel(
|
||||
@@ -241,7 +275,7 @@ class ToolInvocationRepository:
|
||||
plan_id=invocation.plan_id,
|
||||
tool_name=invocation.tool_name,
|
||||
skill_name=invocation.skill_name,
|
||||
arguments_json=json.dumps(invocation.arguments),
|
||||
arguments_json=json.dumps(invocation.arguments, allow_nan=False),
|
||||
result_json=result_json,
|
||||
error=invocation.error,
|
||||
success=invocation.success,
|
||||
@@ -255,6 +289,7 @@ class ToolInvocationRepository:
|
||||
invocation.resource_refs,
|
||||
),
|
||||
provider_metadata_json=pm_json,
|
||||
container_metadata_json=cm_json,
|
||||
)
|
||||
session.add(model)
|
||||
session.flush()
|
||||
@@ -304,15 +339,27 @@ class ToolInvocationRepository:
|
||||
def _to_domain(row: ToolInvocationModel) -> ToolInvocation:
|
||||
"""Convert a DB row to a ToolInvocation domain object."""
|
||||
args_raw = cast("str | None", row.arguments_json)
|
||||
args: dict[str, Any] = json.loads(args_raw) if args_raw else {}
|
||||
args: dict[str, Any] = _safe_json_loads(
|
||||
args_raw, column="arguments_json", default={}
|
||||
)
|
||||
res_raw = cast("str | None", row.result_json)
|
||||
result: dict[str, Any] | None = json.loads(res_raw) if res_raw else None
|
||||
result: dict[str, Any] | None = _safe_json_loads(res_raw, column="result_json")
|
||||
cids_raw = cast("str | None", row.change_ids_json)
|
||||
change_ids: list[str] = json.loads(cids_raw) if cids_raw else []
|
||||
change_ids: list[str] = _safe_json_loads(
|
||||
cids_raw, column="change_ids_json", default=[]
|
||||
)
|
||||
rr_raw = cast("str | None", row.resource_refs_json)
|
||||
resource_refs: list[str] = json.loads(rr_raw) if rr_raw else []
|
||||
resource_refs: list[str] = _safe_json_loads(
|
||||
rr_raw, column="resource_refs_json", default=[]
|
||||
)
|
||||
pm_raw = cast("str | None", row.provider_metadata_json)
|
||||
provider_meta: dict[str, Any] | None = json.loads(pm_raw) if pm_raw else None
|
||||
provider_meta: dict[str, Any] | None = _safe_json_loads(
|
||||
pm_raw, column="provider_metadata_json"
|
||||
)
|
||||
cm_raw = cast("str | None", row.container_metadata_json)
|
||||
container_meta: dict[str, Any] | None = _safe_json_loads(
|
||||
cm_raw, column="container_metadata_json"
|
||||
)
|
||||
|
||||
dur_raw = cast("float | None", row.duration_ms)
|
||||
seq_raw = cast("int | None", row.sequence_number)
|
||||
@@ -339,6 +386,7 @@ class ToolInvocationRepository:
|
||||
sandbox_path=cast("str | None", row.sandbox_path),
|
||||
resource_refs=resource_refs,
|
||||
provider_metadata=provider_meta,
|
||||
container_metadata=container_meta,
|
||||
)
|
||||
|
||||
|
||||
|
||||
@@ -2984,6 +2984,7 @@ class ToolInvocationModel(Base): # type: ignore[misc]
|
||||
sandbox_path = Column(String(1024), nullable=True)
|
||||
resource_refs_json = Column(Text, nullable=True)
|
||||
provider_metadata_json = Column(Text, nullable=True)
|
||||
container_metadata_json = Column(Text, nullable=True)
|
||||
|
||||
__table_args__ = (
|
||||
Index("ix_tool_invocations_changeset_id", "changeset_id"),
|
||||
|
||||
@@ -17,6 +17,13 @@ from cleveragents.tool.actor_runtime import (
|
||||
ToolCallingRuntime,
|
||||
ToolCallRunResult,
|
||||
)
|
||||
from cleveragents.tool.container_executor import (
|
||||
ContainerConfig,
|
||||
ContainerExecutionError,
|
||||
ContainerMetadata,
|
||||
ContainerTimeoutError,
|
||||
ContainerToolExecutor,
|
||||
)
|
||||
from cleveragents.tool.context import (
|
||||
BoundResource,
|
||||
CancellationToken,
|
||||
@@ -47,6 +54,7 @@ from cleveragents.tool.lifecycle import (
|
||||
from cleveragents.tool.lifecycle import (
|
||||
ToolResult as LifecycleToolResult,
|
||||
)
|
||||
from cleveragents.tool.path_mapper import PathMapper
|
||||
from cleveragents.tool.registry import ToolRegistry
|
||||
from cleveragents.tool.router import (
|
||||
NormalizedToolCallResult,
|
||||
@@ -85,12 +93,18 @@ __all__ = [
|
||||
"CancellationToken",
|
||||
"Change",
|
||||
"ChangeOperation",
|
||||
"ContainerConfig",
|
||||
"ContainerExecutionError",
|
||||
"ContainerMetadata",
|
||||
"ContainerTimeoutError",
|
||||
"ContainerToolExecutor",
|
||||
"LLMCaller",
|
||||
"LLMResponse",
|
||||
"LLMToolCall",
|
||||
"LifecycleToolResult",
|
||||
"MaxIterationsExceededError",
|
||||
"NormalizedToolCallResult",
|
||||
"PathMapper",
|
||||
"ProviderFormat",
|
||||
"StreamingStatus",
|
||||
"StreamingToolUpdate",
|
||||
|
||||
@@ -0,0 +1,770 @@
|
||||
"""Container-aware tool execution via ``devcontainer exec``.
|
||||
|
||||
Implements :class:`ContainerToolExecutor` which wraps tool execution
|
||||
inside a provisioned devcontainer. Arguments are escaped for shell
|
||||
safety, results are captured as JSON, and structured errors are
|
||||
produced on failure or timeout.
|
||||
|
||||
This module fulfils issue #515 — container-aware tool execution and
|
||||
I/O forwarding.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import os
|
||||
import posixpath
|
||||
import shlex
|
||||
import shutil
|
||||
import subprocess
|
||||
import time
|
||||
from pathlib import Path
|
||||
from typing import Any
|
||||
|
||||
import structlog
|
||||
from pydantic import BaseModel, ConfigDict, Field, field_validator
|
||||
|
||||
from cleveragents.tool.path_mapper import PathMapper
|
||||
from cleveragents.tool.runtime import ToolResult
|
||||
|
||||
logger = structlog.get_logger(__name__)
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Domain models
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
_DEFAULT_TIMEOUT_SECONDS = 120
|
||||
_MAX_RECURSION_DEPTH = 20
|
||||
_MAX_OUTPUT_BYTES = 50 * 1024 * 1024 # 50 MiB guard for captured output
|
||||
_SAFE_SUBPROCESS_ENV_KEYS = ("PATH", "LANG", "TERM")
|
||||
|
||||
|
||||
class ContainerMetadata(BaseModel):
|
||||
"""Metadata about a container execution.
|
||||
|
||||
Stored on :class:`ToolInvocation` for audit trail purposes.
|
||||
"""
|
||||
|
||||
model_config = ConfigDict(frozen=True)
|
||||
|
||||
container_id: str = ""
|
||||
image: str = ""
|
||||
workspace_folder: str = ""
|
||||
exec_time_ms: float = 0.0
|
||||
exit_code: int | None = None
|
||||
timed_out: bool = False
|
||||
|
||||
|
||||
class ContainerConfig(BaseModel):
|
||||
"""Configuration for the container execution environment.
|
||||
|
||||
Attributes:
|
||||
workspace_folder: Absolute path to the workspace inside the container.
|
||||
container_id: Docker/Podman container ID or name.
|
||||
image: Container image reference.
|
||||
timeout_seconds: Maximum execution time in seconds (must be > 0).
|
||||
host_sandbox_path: Host-side sandbox root for file sync.
|
||||
"""
|
||||
|
||||
model_config = ConfigDict(frozen=True, str_strip_whitespace=True)
|
||||
|
||||
workspace_folder: str = Field(default="/workspace", min_length=1)
|
||||
container_id: str = ""
|
||||
image: str = ""
|
||||
timeout_seconds: int = Field(default=_DEFAULT_TIMEOUT_SECONDS, gt=0)
|
||||
host_sandbox_path: str = ""
|
||||
|
||||
@field_validator("workspace_folder")
|
||||
@classmethod
|
||||
def _validate_workspace_folder(cls, value: str) -> str:
|
||||
if not value.startswith("/"):
|
||||
raise ValueError(
|
||||
"workspace_folder must be an absolute path (starting with '/')"
|
||||
)
|
||||
# Normalise to collapse redundant separators and resolve ".." so
|
||||
# the same path is used both by PathMapper and the devcontainer
|
||||
# CLI (P1-11).
|
||||
normalised = posixpath.normpath(value)
|
||||
if normalised == "/":
|
||||
raise ValueError("workspace_folder must not be the filesystem root '/'")
|
||||
# Reject traversal components that survive normalisation (e.g.
|
||||
# "/../etc" normalises to "/etc" — the intent is suspicious).
|
||||
if "/.." in value or value.endswith("/.."):
|
||||
raise ValueError("workspace_folder must not contain '..' path components")
|
||||
return normalised
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Execution errors
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class ContainerExecutionError(Exception):
|
||||
"""Raised when container execution fails."""
|
||||
|
||||
def __init__(
|
||||
self,
|
||||
message: str,
|
||||
exit_code: int | None = None,
|
||||
stderr: str = "",
|
||||
timed_out: bool = False,
|
||||
) -> None:
|
||||
super().__init__(message)
|
||||
self.exit_code = exit_code
|
||||
self.stderr = stderr
|
||||
self.timed_out = timed_out
|
||||
|
||||
|
||||
class ContainerTimeoutError(ContainerExecutionError):
|
||||
"""Raised when container execution exceeds the timeout."""
|
||||
|
||||
def __init__(self, timeout_seconds: int, stderr: str = "") -> None:
|
||||
super().__init__(
|
||||
f"Container execution timed out after {timeout_seconds}s",
|
||||
exit_code=None,
|
||||
stderr=stderr,
|
||||
timed_out=True,
|
||||
)
|
||||
self.timeout_seconds = timeout_seconds
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Executor
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class _ExecResult(BaseModel):
|
||||
"""Internal result from a ``devcontainer exec`` call."""
|
||||
|
||||
stdout: str = ""
|
||||
stderr: str = ""
|
||||
raw_stdout: bytes = b""
|
||||
exit_code: int = 0
|
||||
duration_ms: float = 0.0
|
||||
timed_out: bool = False
|
||||
|
||||
|
||||
class ContainerToolExecutor:
|
||||
"""Execute tools inside a devcontainer with I/O forwarding.
|
||||
|
||||
The executor translates host-side tool inputs (paths, arguments) into
|
||||
container-side equivalents using :class:`PathMapper`, executes the
|
||||
tool via ``devcontainer exec``, and retrieves results.
|
||||
|
||||
Parameters
|
||||
----------
|
||||
config:
|
||||
Container configuration describing the target environment.
|
||||
path_mapper:
|
||||
Optional path mapper override. If ``None`` a default mapper is
|
||||
created from the config.
|
||||
|
||||
Usage::
|
||||
|
||||
config = ContainerConfig(
|
||||
workspace_folder="/workspace",
|
||||
container_id="abc123",
|
||||
image="ghcr.io/org/devimage:latest",
|
||||
host_sandbox_path="/tmp/sandbox",
|
||||
)
|
||||
executor = ContainerToolExecutor(config)
|
||||
result = executor.execute_tool("file_read", {"path": "/tmp/sandbox/foo.txt"})
|
||||
"""
|
||||
|
||||
def __init__(
|
||||
self,
|
||||
config: ContainerConfig,
|
||||
path_mapper: PathMapper | None = None,
|
||||
) -> None:
|
||||
self._config = config
|
||||
|
||||
host_root = config.host_sandbox_path or "/tmp/sandbox"
|
||||
fallback_path = Path(host_root)
|
||||
if not config.host_sandbox_path and fallback_path.is_symlink():
|
||||
raise ContainerExecutionError(
|
||||
f"Default sandbox path {host_root!r} is a symlink — "
|
||||
"set host_sandbox_path explicitly on ContainerConfig",
|
||||
)
|
||||
|
||||
self._path_mapper = path_mapper or PathMapper(
|
||||
host_root=host_root,
|
||||
container_root=config.workspace_folder,
|
||||
)
|
||||
|
||||
# Resolve devcontainer binary at init to prevent PATH hijacking.
|
||||
# Store ``None`` when the binary is absent so that callers
|
||||
# receive a clear error at execution time rather than silently
|
||||
# falling back to a bare name that could be hijacked via PATH.
|
||||
resolved_bin = shutil.which("devcontainer")
|
||||
if resolved_bin is None:
|
||||
logger.warning(
|
||||
"devcontainer_binary_not_found",
|
||||
msg=(
|
||||
"devcontainer binary not found on PATH; "
|
||||
"execute_tool and sync_results_to_host will fail"
|
||||
),
|
||||
)
|
||||
self._devcontainer_bin: str | None = resolved_bin
|
||||
|
||||
@property
|
||||
def config(self) -> ContainerConfig:
|
||||
"""Return the container configuration."""
|
||||
return self._config
|
||||
|
||||
@property
|
||||
def path_mapper(self) -> PathMapper:
|
||||
"""Return the path mapper."""
|
||||
return self._path_mapper
|
||||
|
||||
# ------------------------------------------------------------------
|
||||
# Public API
|
||||
# ------------------------------------------------------------------
|
||||
|
||||
def execute_tool(
|
||||
self,
|
||||
tool_name: str,
|
||||
inputs: dict[str, Any],
|
||||
*,
|
||||
timeout_seconds: int | None = None,
|
||||
) -> ToolResult:
|
||||
"""Execute a tool inside the container.
|
||||
|
||||
Maps file paths in *inputs*, runs the tool via ``devcontainer
|
||||
exec``, and returns a :class:`ToolResult`.
|
||||
|
||||
Args:
|
||||
tool_name: Namespaced tool name.
|
||||
inputs: Tool input arguments.
|
||||
timeout_seconds: Override for execution timeout.
|
||||
|
||||
Returns:
|
||||
:class:`ToolResult` with execution output or error.
|
||||
|
||||
Raises:
|
||||
ValueError: If *tool_name* is empty or *timeout_seconds* <= 0.
|
||||
TypeError: If *inputs* is not a dict.
|
||||
"""
|
||||
if not tool_name:
|
||||
raise ValueError("tool_name must be a non-empty string")
|
||||
if not isinstance(inputs, dict):
|
||||
raise TypeError("inputs must be a dict")
|
||||
if timeout_seconds is not None and timeout_seconds <= 0:
|
||||
raise ValueError("timeout_seconds must be > 0")
|
||||
|
||||
timeout = timeout_seconds or self._config.timeout_seconds
|
||||
|
||||
# Map host paths to container paths in inputs
|
||||
mapped_inputs = self._map_input_paths(inputs)
|
||||
|
||||
# Build the command — validate JSON-serialisability
|
||||
try:
|
||||
cmd, stdin_data = self._build_exec_command(
|
||||
tool_name, mapped_inputs, timeout
|
||||
)
|
||||
except (TypeError, ValueError) as exc:
|
||||
return ToolResult(
|
||||
success=False,
|
||||
output={},
|
||||
error=f"Tool inputs are not JSON-serialisable: {exc}",
|
||||
duration_ms=0.0,
|
||||
)
|
||||
|
||||
logger.info(
|
||||
"container_tool_exec_start",
|
||||
tool_name=tool_name,
|
||||
container_id=self._config.container_id,
|
||||
timeout_seconds=timeout,
|
||||
)
|
||||
|
||||
exec_result = self._run_command(cmd, timeout=timeout, stdin_data=stdin_data)
|
||||
|
||||
metadata = ContainerMetadata(
|
||||
container_id=self._config.container_id,
|
||||
image=self._config.image,
|
||||
workspace_folder=self._config.workspace_folder,
|
||||
exec_time_ms=exec_result.duration_ms,
|
||||
exit_code=exec_result.exit_code,
|
||||
timed_out=exec_result.timed_out,
|
||||
)
|
||||
metadata_dict = metadata.model_dump()
|
||||
|
||||
if exec_result.timed_out:
|
||||
logger.warning(
|
||||
"container_tool_exec_timeout",
|
||||
tool_name=tool_name,
|
||||
timeout_seconds=timeout,
|
||||
)
|
||||
return ToolResult(
|
||||
success=False,
|
||||
output={},
|
||||
error=f"Container execution timed out after {timeout}s",
|
||||
duration_ms=exec_result.duration_ms,
|
||||
metadata={"container": metadata_dict},
|
||||
)
|
||||
|
||||
if exec_result.exit_code != 0:
|
||||
logger.warning(
|
||||
"container_tool_exec_failed",
|
||||
tool_name=tool_name,
|
||||
exit_code=exec_result.exit_code,
|
||||
)
|
||||
# Attempt best-effort parsing of stdout even on failure
|
||||
partial_output = self._parse_output(exec_result.stdout)
|
||||
return ToolResult(
|
||||
success=False,
|
||||
output=partial_output,
|
||||
error=(
|
||||
f"Container execution failed (exit code "
|
||||
f"{exec_result.exit_code}): "
|
||||
f"{exec_result.stderr[:500]}"
|
||||
),
|
||||
duration_ms=exec_result.duration_ms,
|
||||
metadata={"container": metadata_dict},
|
||||
)
|
||||
|
||||
# Parse output and map container paths back to host paths.
|
||||
# Metadata is placed in ToolResult.metadata (not output) so the
|
||||
# tool's semantic output is not polluted.
|
||||
output = self._parse_output(exec_result.stdout)
|
||||
output = self._map_output_paths(output)
|
||||
|
||||
logger.info(
|
||||
"container_tool_exec_success",
|
||||
tool_name=tool_name,
|
||||
duration_ms=exec_result.duration_ms,
|
||||
)
|
||||
|
||||
return ToolResult(
|
||||
success=True,
|
||||
output=output,
|
||||
duration_ms=exec_result.duration_ms,
|
||||
metadata={"container": metadata_dict},
|
||||
)
|
||||
|
||||
def sync_results_to_host(self, container_path: str) -> str:
|
||||
"""Sync a container file back to the host sandbox.
|
||||
|
||||
Uses ``devcontainer exec`` with ``cat`` to read the file
|
||||
content from the container and writes it to the mapped host
|
||||
path.
|
||||
|
||||
Args:
|
||||
container_path: Absolute path inside the container.
|
||||
|
||||
Returns:
|
||||
Host-side path where the content was written.
|
||||
|
||||
Raises:
|
||||
ContainerExecutionError: If the sync fails.
|
||||
ContainerTimeoutError: If the sync times out.
|
||||
ValueError: If *container_path* is not absolute or
|
||||
contains null bytes.
|
||||
"""
|
||||
if "\x00" in container_path:
|
||||
raise ValueError("container_path must not contain null bytes")
|
||||
if not container_path.startswith("/"):
|
||||
raise ValueError(f"container_path must be absolute, got {container_path!r}")
|
||||
|
||||
host_path = self._path_mapper.container_to_host(container_path)
|
||||
|
||||
# Validate host path falls within the sandbox root.
|
||||
sandbox_root = Path(self._path_mapper.host_root).resolve()
|
||||
resolved_host = Path(host_path).resolve()
|
||||
if not resolved_host.is_relative_to(sandbox_root):
|
||||
raise ContainerExecutionError(
|
||||
f"Mapped host path {host_path!r} resolves outside "
|
||||
f"sandbox root {sandbox_root}",
|
||||
exit_code=-1,
|
||||
)
|
||||
|
||||
cmd = self._build_sync_command(container_path)
|
||||
result = self._run_command(cmd, timeout=self._config.timeout_seconds)
|
||||
if result.timed_out:
|
||||
raise ContainerTimeoutError(
|
||||
timeout_seconds=self._config.timeout_seconds,
|
||||
stderr=result.stderr,
|
||||
)
|
||||
if result.exit_code != 0:
|
||||
raise ContainerExecutionError(
|
||||
f"Failed to sync {container_path} to host: {result.stderr}",
|
||||
exit_code=result.exit_code,
|
||||
stderr=result.stderr,
|
||||
)
|
||||
|
||||
# Write captured content to the *resolved* host path to avoid a
|
||||
# TOCTOU race between validation and actual file write.
|
||||
# Use raw bytes from _run_command to avoid binary-file
|
||||
# corruption from text-mode decode/re-encode (P1-10).
|
||||
dest = resolved_host
|
||||
try:
|
||||
dest.parent.mkdir(parents=True, exist_ok=True, mode=0o700)
|
||||
fd = os.open(
|
||||
str(dest),
|
||||
os.O_WRONLY | os.O_CREAT | os.O_TRUNC | os.O_NOFOLLOW,
|
||||
0o600,
|
||||
)
|
||||
try:
|
||||
os.write(fd, result.raw_stdout)
|
||||
finally:
|
||||
os.close(fd)
|
||||
except OSError as exc:
|
||||
raise ContainerExecutionError(
|
||||
f"Failed to write synced file to host path {dest}: {exc}",
|
||||
exit_code=-1,
|
||||
stderr=str(exc),
|
||||
) from exc
|
||||
|
||||
return str(resolved_host)
|
||||
|
||||
# ------------------------------------------------------------------
|
||||
# Command building
|
||||
# ------------------------------------------------------------------
|
||||
|
||||
def _build_exec_command(
|
||||
self,
|
||||
tool_name: str,
|
||||
inputs: dict[str, Any],
|
||||
timeout: int,
|
||||
) -> tuple[list[str], str]:
|
||||
"""Build the ``devcontainer exec`` command for tool execution.
|
||||
|
||||
Returns a (command, stdin_data) tuple. The JSON inputs are
|
||||
piped via stdin instead of embedded as a shell argument to
|
||||
avoid hitting the OS ``ARG_MAX`` limit.
|
||||
"""
|
||||
inputs_json = json.dumps(inputs, allow_nan=False)
|
||||
|
||||
devcontainer = self._require_devcontainer_bin()
|
||||
target_args = self._devcontainer_target_args()
|
||||
|
||||
# Enforce int type to prevent shell injection via malicious
|
||||
# objects with __str__ methods (P2-16).
|
||||
timeout = int(timeout)
|
||||
|
||||
# Use a slightly shorter container-side timeout so the
|
||||
# container process self-terminates before the host-side
|
||||
# deadline, avoiding orphan processes when the host kills
|
||||
# its Popen first.
|
||||
container_timeout = max(timeout - 5, 1)
|
||||
cmd = [
|
||||
devcontainer,
|
||||
"exec",
|
||||
*target_args,
|
||||
"--",
|
||||
"sh",
|
||||
"-c",
|
||||
f"timeout {int(container_timeout)} cleveragents-tool-exec "
|
||||
+ shlex.quote(tool_name),
|
||||
]
|
||||
return cmd, inputs_json
|
||||
|
||||
def _build_sync_command(self, container_path: str) -> list[str]:
|
||||
"""Build a command to sync a file from container to host."""
|
||||
devcontainer = self._require_devcontainer_bin()
|
||||
target_args = self._devcontainer_target_args()
|
||||
return [
|
||||
devcontainer,
|
||||
"exec",
|
||||
*target_args,
|
||||
"--",
|
||||
"sh",
|
||||
"-c",
|
||||
f"cat {shlex.quote(container_path)}",
|
||||
]
|
||||
|
||||
def _require_devcontainer_bin(self) -> str:
|
||||
"""Return the resolved devcontainer binary path or raise."""
|
||||
if self._devcontainer_bin is None:
|
||||
raise ContainerExecutionError(
|
||||
"devcontainer binary was not found on PATH at init time. "
|
||||
"Install devcontainer CLI or set host_sandbox_path explicitly.",
|
||||
exit_code=-1,
|
||||
)
|
||||
return self._devcontainer_bin
|
||||
|
||||
def _devcontainer_target_args(self) -> list[str]:
|
||||
"""Return CLI args to target the correct container."""
|
||||
if self._config.container_id:
|
||||
return ["--container-id", self._config.container_id]
|
||||
return ["--workspace-folder", self._config.workspace_folder]
|
||||
|
||||
# ------------------------------------------------------------------
|
||||
# Command execution
|
||||
# ------------------------------------------------------------------
|
||||
|
||||
def _run_command(
|
||||
self,
|
||||
cmd: list[str],
|
||||
timeout: int,
|
||||
*,
|
||||
stdin_data: str | None = None,
|
||||
) -> _ExecResult:
|
||||
"""Execute a command and capture output with bounded memory.
|
||||
|
||||
Uses :class:`subprocess.Popen` with incremental reads so that
|
||||
at most ``_MAX_OUTPUT_BYTES`` are buffered per stream at any
|
||||
time, preventing an unbounded memory allocation if the
|
||||
container produces excessive output.
|
||||
|
||||
Returns an :class:`_ExecResult` with stdout, stderr, exit code,
|
||||
duration, and timeout status.
|
||||
"""
|
||||
safe_env = {k: os.environ.get(k, "") for k in _SAFE_SUBPROCESS_ENV_KEYS}
|
||||
|
||||
start = time.monotonic()
|
||||
try:
|
||||
proc = subprocess.Popen(
|
||||
cmd,
|
||||
stdout=subprocess.PIPE,
|
||||
stderr=subprocess.PIPE,
|
||||
stdin=subprocess.PIPE if stdin_data else subprocess.DEVNULL,
|
||||
env=safe_env,
|
||||
)
|
||||
try:
|
||||
# Feed stdin and close it so the child can proceed.
|
||||
if stdin_data:
|
||||
assert proc.stdin is not None # mypy guard
|
||||
proc.stdin.write(stdin_data.encode("utf-8"))
|
||||
proc.stdin.close()
|
||||
|
||||
# Read stdout/stderr with a cap to avoid unbounded memory.
|
||||
raw_stdout = _read_bounded(proc.stdout, _MAX_OUTPUT_BYTES)
|
||||
raw_stderr = _read_bounded(proc.stderr, _MAX_OUTPUT_BYTES)
|
||||
|
||||
remaining = timeout - (time.monotonic() - start)
|
||||
proc.wait(timeout=max(remaining, 0))
|
||||
except subprocess.TimeoutExpired:
|
||||
proc.kill()
|
||||
proc.wait()
|
||||
elapsed = (time.monotonic() - start) * 1000.0
|
||||
return _ExecResult(
|
||||
stdout="",
|
||||
stderr=f"Command timed out after {timeout}s",
|
||||
exit_code=-1,
|
||||
duration_ms=elapsed,
|
||||
timed_out=True,
|
||||
)
|
||||
finally:
|
||||
# Ensure all handles are closed even on unexpected errors.
|
||||
for stream in (proc.stdout, proc.stderr, proc.stdin):
|
||||
if stream and not stream.closed:
|
||||
stream.close()
|
||||
|
||||
elapsed = (time.monotonic() - start) * 1000.0
|
||||
|
||||
if len(raw_stdout) > _MAX_OUTPUT_BYTES:
|
||||
logger.warning(
|
||||
"container_output_truncated",
|
||||
stream="stdout",
|
||||
original_bytes=len(raw_stdout),
|
||||
max_bytes=_MAX_OUTPUT_BYTES,
|
||||
)
|
||||
raw_stdout = raw_stdout[:_MAX_OUTPUT_BYTES]
|
||||
if len(raw_stderr) > _MAX_OUTPUT_BYTES:
|
||||
logger.warning(
|
||||
"container_output_truncated",
|
||||
stream="stderr",
|
||||
original_bytes=len(raw_stderr),
|
||||
max_bytes=_MAX_OUTPUT_BYTES,
|
||||
)
|
||||
raw_stderr = raw_stderr[:_MAX_OUTPUT_BYTES]
|
||||
|
||||
stdout_text = raw_stdout.decode("utf-8", errors="replace")
|
||||
stderr_text = raw_stderr.decode("utf-8", errors="replace")
|
||||
return _ExecResult(
|
||||
stdout=stdout_text,
|
||||
stderr=stderr_text,
|
||||
raw_stdout=raw_stdout,
|
||||
exit_code=proc.returncode,
|
||||
duration_ms=elapsed,
|
||||
timed_out=False,
|
||||
)
|
||||
except OSError as exc:
|
||||
elapsed = (time.monotonic() - start) * 1000.0
|
||||
return _ExecResult(
|
||||
stdout="",
|
||||
stderr=str(exc),
|
||||
exit_code=-1,
|
||||
duration_ms=elapsed,
|
||||
timed_out=False,
|
||||
)
|
||||
|
||||
# ------------------------------------------------------------------
|
||||
# Path mapping
|
||||
# ------------------------------------------------------------------
|
||||
|
||||
def _map_input_paths(
|
||||
self, inputs: dict[str, Any], *, _depth: int = 0
|
||||
) -> dict[str, Any]:
|
||||
"""Map host file paths in tool inputs to container paths."""
|
||||
if _depth > _MAX_RECURSION_DEPTH:
|
||||
logger.warning(
|
||||
"path_mapping_depth_exceeded",
|
||||
direction="host_to_container",
|
||||
max_depth=_MAX_RECURSION_DEPTH,
|
||||
)
|
||||
return inputs
|
||||
mapped: dict[str, Any] = {}
|
||||
for key, value in inputs.items():
|
||||
mapped[key] = self._map_value_host_to_container(value, _depth=_depth)
|
||||
return mapped
|
||||
|
||||
def _map_value_host_to_container(self, value: Any, *, _depth: int = 0) -> Any:
|
||||
"""Recursively map a single value from host to container paths."""
|
||||
if _depth > _MAX_RECURSION_DEPTH:
|
||||
logger.warning(
|
||||
"path_mapping_depth_exceeded",
|
||||
direction="host_to_container",
|
||||
max_depth=_MAX_RECURSION_DEPTH,
|
||||
)
|
||||
return value
|
||||
if isinstance(value, str) and _looks_like_path(value):
|
||||
if self._path_mapper.is_host_path(value):
|
||||
return self._path_mapper.host_to_container(value)
|
||||
return value
|
||||
if isinstance(value, dict):
|
||||
return {
|
||||
k: self._map_value_host_to_container(v, _depth=_depth + 1)
|
||||
for k, v in value.items()
|
||||
}
|
||||
if isinstance(value, list):
|
||||
return [
|
||||
self._map_value_host_to_container(v, _depth=_depth + 1) for v in value
|
||||
]
|
||||
return value
|
||||
|
||||
def _map_output_paths(
|
||||
self, output: dict[str, Any], *, _depth: int = 0
|
||||
) -> dict[str, Any]:
|
||||
"""Map container file paths in tool output to host paths."""
|
||||
if _depth > _MAX_RECURSION_DEPTH:
|
||||
logger.warning(
|
||||
"path_mapping_depth_exceeded",
|
||||
direction="container_to_host",
|
||||
max_depth=_MAX_RECURSION_DEPTH,
|
||||
)
|
||||
return output
|
||||
mapped: dict[str, Any] = {}
|
||||
for key, value in output.items():
|
||||
# Skip path mapping for raw_output to prevent nonsensical
|
||||
# remapping of multi-line stdout that happens to start with
|
||||
# the container root.
|
||||
if key == "raw_output":
|
||||
mapped[key] = value
|
||||
continue
|
||||
mapped[key] = self._map_value_container_to_host(value, _depth=_depth)
|
||||
return mapped
|
||||
|
||||
def _map_value_container_to_host(self, value: Any, *, _depth: int = 0) -> Any:
|
||||
"""Recursively map a single value from container to host paths."""
|
||||
if _depth > _MAX_RECURSION_DEPTH:
|
||||
logger.warning(
|
||||
"path_mapping_depth_exceeded",
|
||||
direction="container_to_host",
|
||||
max_depth=_MAX_RECURSION_DEPTH,
|
||||
)
|
||||
return value
|
||||
if isinstance(value, str) and _looks_like_path(value):
|
||||
if self._path_mapper.is_container_path(value):
|
||||
return self._path_mapper.container_to_host(value)
|
||||
return value
|
||||
if isinstance(value, dict):
|
||||
return {
|
||||
k: self._map_value_container_to_host(v, _depth=_depth + 1)
|
||||
for k, v in value.items()
|
||||
}
|
||||
if isinstance(value, list):
|
||||
return [
|
||||
self._map_value_container_to_host(v, _depth=_depth + 1) for v in value
|
||||
]
|
||||
return value
|
||||
|
||||
# ------------------------------------------------------------------
|
||||
# Output parsing
|
||||
# ------------------------------------------------------------------
|
||||
|
||||
@staticmethod
|
||||
def extract_container_metadata(result: ToolResult) -> dict[str, Any] | None:
|
||||
"""Extract container metadata from a :class:`ToolResult`.
|
||||
|
||||
Returns the ``container`` dict from ``result.metadata`` if
|
||||
present, or ``None`` otherwise. This is the bridge between the
|
||||
executor's ``ToolResult`` output and the
|
||||
:class:`ToolInvocation.container_metadata` field for audit trail
|
||||
persistence (P1-9).
|
||||
"""
|
||||
return result.metadata.get("container")
|
||||
|
||||
@staticmethod
|
||||
def _parse_output(stdout: str) -> dict[str, Any]:
|
||||
"""Parse JSON output from the container tool execution.
|
||||
|
||||
Falls back to wrapping raw output in a dict if JSON parsing
|
||||
fails.
|
||||
"""
|
||||
if not stdout.strip():
|
||||
return {}
|
||||
try:
|
||||
parsed = json.loads(stdout)
|
||||
if isinstance(parsed, dict):
|
||||
return parsed
|
||||
return {"result": parsed}
|
||||
except (json.JSONDecodeError, TypeError, RecursionError, MemoryError):
|
||||
return {"raw_output": stdout.strip()}
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Helpers
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
_READ_CHUNK_SIZE = 65_536 # 64 KiB chunks for incremental stream reads
|
||||
|
||||
|
||||
def _read_bounded(stream: Any, max_bytes: int) -> bytes:
|
||||
"""Read up to *max_bytes* from *stream*, then discard the rest.
|
||||
|
||||
Reads in fixed-size chunks to avoid allocating the entire output
|
||||
in a single ``read()`` call.
|
||||
"""
|
||||
chunks: list[bytes] = []
|
||||
total = 0
|
||||
while True:
|
||||
chunk = stream.read(_READ_CHUNK_SIZE)
|
||||
if not chunk:
|
||||
break
|
||||
remaining = max_bytes - total
|
||||
if remaining <= 0:
|
||||
# Already at the cap — drain the stream without keeping data
|
||||
continue
|
||||
if len(chunk) > remaining:
|
||||
chunks.append(chunk[:remaining])
|
||||
total += remaining
|
||||
else:
|
||||
chunks.append(chunk)
|
||||
total += len(chunk)
|
||||
return b"".join(chunks)
|
||||
|
||||
|
||||
def _looks_like_path(value: str) -> bool:
|
||||
"""Return ``True`` if *value* looks like a filesystem path.
|
||||
|
||||
Rejects strings containing newlines, tabs, or null bytes which are
|
||||
almost certainly not bare paths. Also rejects URL-like strings and
|
||||
strings with query/fragment markers to avoid false positives on API
|
||||
routes and URIs (P1-7). Spaces are allowed because they are valid
|
||||
in filesystem paths (e.g. ``/home/user/My Documents``).
|
||||
"""
|
||||
if not value.startswith("/"):
|
||||
return False
|
||||
if "\n" in value or "\r" in value or "\t" in value:
|
||||
return False
|
||||
if "\x00" in value:
|
||||
return False
|
||||
# Reject URL-like patterns: "//host/...", protocol-relative URIs,
|
||||
# and paths containing query strings or fragment identifiers.
|
||||
if value.startswith("//"):
|
||||
return False
|
||||
if "?" in value or "#" in value:
|
||||
return False
|
||||
# Reject strings that look like URL paths with scheme separators.
|
||||
return "://" not in value
|
||||
@@ -0,0 +1,177 @@
|
||||
"""Host-to-container path mapping utility.
|
||||
|
||||
Translates file paths between the host filesystem and the container
|
||||
workspace for file-based tool arguments and outputs.
|
||||
|
||||
Used by :class:`ContainerToolExecutor` to ensure file paths in tool
|
||||
inputs point to the correct locations inside the container, and that
|
||||
output paths are mapped back to the host sandbox.
|
||||
|
||||
Based on issue #515 — container-aware tool execution and I/O forwarding.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import posixpath
|
||||
from dataclasses import dataclass
|
||||
|
||||
|
||||
@dataclass(frozen=True)
|
||||
class PathMapper:
|
||||
"""Bi-directional path mapper between host and container filesystems.
|
||||
|
||||
Attributes:
|
||||
host_root: Absolute path to the host-side sandbox root.
|
||||
container_root: Absolute path to the container-side workspace root.
|
||||
|
||||
Example::
|
||||
|
||||
mapper = PathMapper(
|
||||
host_root="/tmp/sandbox/project-1",
|
||||
container_root="/workspace",
|
||||
)
|
||||
h2c = mapper.host_to_container(
|
||||
"/tmp/sandbox/project-1/src/main.py",
|
||||
)
|
||||
assert h2c == "/workspace/src/main.py"
|
||||
c2h = mapper.container_to_host("/workspace/src/main.py")
|
||||
assert c2h == "/tmp/sandbox/project-1/src/main.py"
|
||||
"""
|
||||
|
||||
host_root: str
|
||||
container_root: str
|
||||
|
||||
def __post_init__(self) -> None:
|
||||
"""Validate and normalise root paths."""
|
||||
if "\x00" in self.host_root or "\x00" in self.container_root:
|
||||
raise ValueError("Root paths must not contain null bytes")
|
||||
if not self.host_root or not self.container_root:
|
||||
raise ValueError("Root paths must not be empty")
|
||||
|
||||
normalised_host = _normalise(self.host_root)
|
||||
normalised_container = _normalise(self.container_root)
|
||||
|
||||
# ``posixpath.normpath("//")`` returns ``"//"`` per POSIX, so
|
||||
# we must guard against both ``"/"`` and ``"//"`` to prevent a
|
||||
# root-mapping bypass.
|
||||
if normalised_host in ("/", "//"):
|
||||
raise ValueError(
|
||||
"host_root must not be '/' — it would map every absolute path"
|
||||
)
|
||||
if normalised_container in ("/", "//"):
|
||||
raise ValueError(
|
||||
"container_root must not be '/' — it would map every absolute path"
|
||||
)
|
||||
|
||||
# Reject overlapping roots which produce corrupt mappings where
|
||||
# a relative-path component gets doubled (P2-14).
|
||||
if _is_under(normalised_host, normalised_container) or _is_under(
|
||||
normalised_container, normalised_host
|
||||
):
|
||||
raise ValueError(
|
||||
"host_root and container_root must not overlap: "
|
||||
f"{normalised_host!r} vs {normalised_container!r}"
|
||||
)
|
||||
|
||||
# Store normalised roots to avoid trailing-slash inconsistencies.
|
||||
object.__setattr__(self, "host_root", normalised_host)
|
||||
object.__setattr__(self, "container_root", normalised_container)
|
||||
|
||||
def host_to_container(self, host_path: str) -> str:
|
||||
"""Map a host path to the equivalent container path.
|
||||
|
||||
If *host_path* is not under :attr:`host_root`, it is returned
|
||||
unchanged (external paths cannot be mapped).
|
||||
|
||||
Args:
|
||||
host_path: Absolute path on the host filesystem.
|
||||
|
||||
Returns:
|
||||
Equivalent path inside the container, or *host_path* if
|
||||
it falls outside the host root.
|
||||
|
||||
Raises:
|
||||
ValueError: If *host_path* contains null bytes.
|
||||
"""
|
||||
_reject_null_bytes(host_path)
|
||||
normalised = _normalise(host_path)
|
||||
|
||||
if not _is_under(normalised, self.host_root):
|
||||
return normalised
|
||||
|
||||
relative = _relative_to(normalised, self.host_root)
|
||||
return (
|
||||
posixpath.join(self.container_root, relative)
|
||||
if relative
|
||||
else self.container_root
|
||||
)
|
||||
|
||||
def container_to_host(self, container_path: str) -> str:
|
||||
"""Map a container path to the equivalent host path.
|
||||
|
||||
If *container_path* is not under :attr:`container_root`, it is
|
||||
returned unchanged.
|
||||
|
||||
Args:
|
||||
container_path: Absolute path inside the container.
|
||||
|
||||
Returns:
|
||||
Equivalent path on the host filesystem, or *container_path*
|
||||
if it falls outside the container root.
|
||||
|
||||
Raises:
|
||||
ValueError: If *container_path* contains null bytes.
|
||||
"""
|
||||
_reject_null_bytes(container_path)
|
||||
normalised = _normalise(container_path)
|
||||
|
||||
if not _is_under(normalised, self.container_root):
|
||||
return normalised
|
||||
|
||||
relative = _relative_to(normalised, self.container_root)
|
||||
return posixpath.join(self.host_root, relative) if relative else self.host_root
|
||||
|
||||
def is_host_path(self, path: str) -> bool:
|
||||
"""Check whether *path* is under the host root."""
|
||||
if "\x00" in path:
|
||||
return False
|
||||
return _is_under(_normalise(path), self.host_root)
|
||||
|
||||
def is_container_path(self, path: str) -> bool:
|
||||
"""Check whether *path* is under the container root."""
|
||||
if "\x00" in path:
|
||||
return False
|
||||
return _is_under(_normalise(path), self.container_root)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Path helpers (use posixpath for container paths which are always Linux)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def _reject_null_bytes(path: str) -> None:
|
||||
"""Raise ``ValueError`` if *path* contains null bytes."""
|
||||
if "\x00" in path:
|
||||
raise ValueError(f"Path contains null bytes: {path!r}")
|
||||
|
||||
|
||||
def _normalise(path: str) -> str:
|
||||
"""Normalise a path — strip trailing slashes, collapse double slashes."""
|
||||
return posixpath.normpath(path)
|
||||
|
||||
|
||||
def _is_under(path: str, root: str) -> bool:
|
||||
"""Return ``True`` if *path* is equal to or a child of *root*."""
|
||||
if path == root:
|
||||
return True
|
||||
return path.startswith(root + "/")
|
||||
|
||||
|
||||
def _relative_to(path: str, root: str) -> str:
|
||||
"""Return the part of *path* relative to *root*.
|
||||
|
||||
Assumes :func:`_is_under` has already been checked.
|
||||
"""
|
||||
if path == root:
|
||||
return ""
|
||||
return path[len(root) + 1 :]
|
||||
@@ -23,10 +23,12 @@ and the actual handler callables.
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import threading
|
||||
import time
|
||||
from typing import TYPE_CHECKING, Any
|
||||
|
||||
from cleveragents.domain.models.core.plan import ExecutionEnvironment
|
||||
from cleveragents.tool.container_executor import ContainerToolExecutor
|
||||
from cleveragents.tool.registry import ToolRegistry
|
||||
from cleveragents.tool.runtime import ToolError, ToolResult, ToolSpec
|
||||
|
||||
@@ -51,6 +53,7 @@ class ToolRunner:
|
||||
self,
|
||||
registry: ToolRegistry,
|
||||
execution_environment_resolver: ExecutionEnvironmentResolver | None = None,
|
||||
container_executor: ContainerToolExecutor | None = None,
|
||||
) -> None:
|
||||
from cleveragents.application.services.execution_environment_resolver import (
|
||||
ExecutionEnvironmentResolver as _Resolver,
|
||||
@@ -58,9 +61,11 @@ class ToolRunner:
|
||||
|
||||
self._registry = registry
|
||||
self._active: dict[str, ToolSpec] = {}
|
||||
self._active_lock = threading.RLock()
|
||||
self._env_resolver: ExecutionEnvironmentResolver = (
|
||||
execution_environment_resolver or _Resolver()
|
||||
)
|
||||
self._container_executor = container_executor
|
||||
|
||||
# -- Stage 1: Discover ----------------------------------------------------
|
||||
|
||||
@@ -88,7 +93,8 @@ class ToolRunner:
|
||||
error_type="ActivationError",
|
||||
details=f"Tool '{tool_name}' not found in registry",
|
||||
)
|
||||
self._active[tool_name] = spec
|
||||
with self._active_lock:
|
||||
self._active[tool_name] = spec
|
||||
return spec
|
||||
|
||||
# -- Stage 3: Execute -----------------------------------------------------
|
||||
@@ -119,6 +125,7 @@ class ToolRunner:
|
||||
project_env: str | None = None,
|
||||
linked_resource_types: list[str] | None = None,
|
||||
project_name: str = "",
|
||||
timeout_seconds: int | None = None,
|
||||
) -> ToolResult:
|
||||
"""Run the tool with strict JSON-serialisable IO.
|
||||
|
||||
@@ -128,8 +135,17 @@ class ToolRunner:
|
||||
|
||||
Any exception raised by the handler is caught and normalised
|
||||
into a ``ToolResult`` with ``success=False``.
|
||||
|
||||
Parameters
|
||||
----------
|
||||
timeout_seconds:
|
||||
Optional timeout override forwarded to the container
|
||||
executor when the execution environment is ``CONTAINER``.
|
||||
"""
|
||||
spec = self._active.get(tool_name) or self._registry.get(tool_name)
|
||||
with self._active_lock:
|
||||
spec = self._active.get(tool_name)
|
||||
if spec is None:
|
||||
spec = self._registry.get(tool_name)
|
||||
if spec is None:
|
||||
raise ToolError(
|
||||
tool_name=tool_name,
|
||||
@@ -157,7 +173,7 @@ class ToolRunner:
|
||||
error=str(exc),
|
||||
duration_ms=0.0,
|
||||
)
|
||||
except ValueError as exc:
|
||||
except Exception as exc:
|
||||
return ToolResult(
|
||||
success=False,
|
||||
output={},
|
||||
@@ -167,18 +183,71 @@ class ToolRunner:
|
||||
|
||||
# Container routing — delegate to container handler
|
||||
if env == ExecutionEnvironment.CONTAINER:
|
||||
return ToolResult(
|
||||
success=False,
|
||||
output={},
|
||||
error=(
|
||||
"Container execution is not yet implemented. "
|
||||
"A container resource is linked but the container "
|
||||
"handler has not been wired."
|
||||
),
|
||||
duration_ms=0.0,
|
||||
)
|
||||
if self._container_executor is None:
|
||||
return ToolResult(
|
||||
success=False,
|
||||
output={},
|
||||
error=(
|
||||
"Container execution is not available. "
|
||||
"A ContainerToolExecutor must be configured on the "
|
||||
"ToolRunner before container-routed tools can execute."
|
||||
),
|
||||
duration_ms=0.0,
|
||||
)
|
||||
|
||||
# Validate that inputs are JSON-serialisable
|
||||
# Validate input_schema and capabilities (T2) before
|
||||
# delegating to the container executor so container-routed
|
||||
# tools receive the same spec validation as host-routed ones.
|
||||
try:
|
||||
json.dumps(inputs, allow_nan=False)
|
||||
except (TypeError, ValueError) as exc:
|
||||
return ToolResult(
|
||||
success=False,
|
||||
output={},
|
||||
error=f"Inputs are not JSON-serialisable: {exc}",
|
||||
duration_ms=0.0,
|
||||
)
|
||||
|
||||
if spec.input_schema:
|
||||
# Iterate the ``required`` list directly so that fields
|
||||
# listed in ``required`` but absent from ``properties``
|
||||
# (e.g. ``additionalProperties`` patterns) are still
|
||||
# detected (P2-20).
|
||||
missing = [
|
||||
k for k in spec.input_schema.get("required", []) if k not in inputs
|
||||
]
|
||||
if missing:
|
||||
return ToolResult(
|
||||
success=False,
|
||||
output={},
|
||||
error=(
|
||||
f"Missing required input fields for "
|
||||
f"'{tool_name}': {missing}"
|
||||
),
|
||||
duration_ms=0.0,
|
||||
)
|
||||
|
||||
try:
|
||||
return self._container_executor.execute_tool(
|
||||
tool_name,
|
||||
inputs,
|
||||
timeout_seconds=timeout_seconds,
|
||||
)
|
||||
except Exception as exc:
|
||||
# Intentionally broad: the runner's contract is to
|
||||
# normalise *any* handler failure into a ToolResult
|
||||
# with ``success=False`` so callers never see raw
|
||||
# exceptions from container execution.
|
||||
return ToolResult(
|
||||
success=False,
|
||||
output={},
|
||||
error=f"Container execution error: {type(exc).__name__}: {exc}",
|
||||
duration_ms=0.0,
|
||||
)
|
||||
|
||||
# Validate that inputs are JSON-serialisable. Host-routed tools
|
||||
# use the default ``allow_nan=True`` for backward compatibility;
|
||||
# only the container path enforces RFC 7159 (P1-1).
|
||||
try:
|
||||
json.dumps(inputs)
|
||||
except (TypeError, ValueError) as exc:
|
||||
@@ -193,6 +262,9 @@ class ToolRunner:
|
||||
try:
|
||||
raw_output = spec.handler(inputs)
|
||||
except Exception as exc:
|
||||
# Intentionally broad: the runner normalises handler
|
||||
# exceptions into a failed ToolResult so callers get a
|
||||
# uniform result type regardless of handler implementation.
|
||||
elapsed = (time.monotonic() - start) * 1000.0
|
||||
return ToolResult(
|
||||
success=False,
|
||||
@@ -207,7 +279,8 @@ class ToolRunner:
|
||||
if not isinstance(raw_output, dict):
|
||||
raw_output = {"result": raw_output}
|
||||
|
||||
# Validate that output is JSON-serialisable
|
||||
# Validate that output is JSON-serialisable. Host-routed tools
|
||||
# use the default ``allow_nan=True`` for backward compatibility.
|
||||
try:
|
||||
json.dumps(raw_output)
|
||||
except (TypeError, ValueError) as exc:
|
||||
@@ -232,7 +305,8 @@ class ToolRunner:
|
||||
Returns ``True`` if the tool was active and has been
|
||||
deactivated, ``False`` otherwise.
|
||||
"""
|
||||
if tool_name in self._active:
|
||||
del self._active[tool_name]
|
||||
return True
|
||||
return False
|
||||
with self._active_lock:
|
||||
if tool_name in self._active:
|
||||
del self._active[tool_name]
|
||||
return True
|
||||
return False
|
||||
|
||||
@@ -31,7 +31,7 @@ from __future__ import annotations
|
||||
from collections.abc import Callable
|
||||
from typing import Any
|
||||
|
||||
from pydantic import BaseModel, ConfigDict, Field
|
||||
from pydantic import BaseModel, ConfigDict, Field, model_validator
|
||||
|
||||
from cleveragents.domain.models.core.tool import ToolCapability
|
||||
|
||||
@@ -121,6 +121,24 @@ class ToolResult(BaseModel):
|
||||
validate_assignment=True,
|
||||
)
|
||||
|
||||
@model_validator(mode="after")
|
||||
def _validate_success_error_consistency(self) -> ToolResult:
|
||||
"""Ensure success and error fields are logically consistent.
|
||||
|
||||
A result marked as ``success=True`` must not carry an error
|
||||
message, and a result marked as ``success=False`` must have
|
||||
an error message explaining the failure.
|
||||
"""
|
||||
if self.success and self.error is not None:
|
||||
raise ValueError(
|
||||
"ToolResult with success=True must not have an error message"
|
||||
)
|
||||
if not self.success and self.error is None:
|
||||
raise ValueError(
|
||||
"ToolResult with success=False must include an error message"
|
||||
)
|
||||
return self
|
||||
|
||||
|
||||
class ToolError(Exception):
|
||||
"""Exception raised for tool execution failures.
|
||||
|
||||
@@ -943,6 +943,7 @@ score_detailed # noqa: B018, F821
|
||||
depth_fallback_steps # noqa: B018, F821
|
||||
min_fragment_tokens # noqa: B018, F821
|
||||
|
||||
|
||||
# LSP Server Stub — public API (issue #203)
|
||||
LspServer # noqa: B018, F821
|
||||
MAX_CONTENT_LENGTH # noqa: B018, F821
|
||||
@@ -1097,3 +1098,29 @@ _build_analyzer_registry # noqa: B018, F821
|
||||
|
||||
# Test doubles — public API for BDD/Robot test infrastructure
|
||||
TrackingEventBus # noqa: B018, F821
|
||||
|
||||
# Container Tool Execution (#515) — public API
|
||||
ContainerConfig # noqa: B018, F821
|
||||
ContainerExecutionError # noqa: B018, F821
|
||||
ContainerMetadata # noqa: B018, F821
|
||||
ContainerTimeoutError # noqa: B018, F821
|
||||
ContainerToolExecutor # noqa: B018, F821
|
||||
PathMapper # noqa: B018, F821
|
||||
container_metadata # noqa: B018, F821
|
||||
container_id # noqa: B018, F821
|
||||
exec_time_ms # noqa: B018, F821
|
||||
exit_code # noqa: B018, F821
|
||||
host_root # noqa: B018, F821
|
||||
container_root # noqa: B018, F821
|
||||
host_sandbox_path # noqa: B018, F821
|
||||
image # noqa: B018, F821
|
||||
is_container_path # noqa: B018, F821
|
||||
is_host_path # noqa: B018, F821
|
||||
host_to_container # noqa: B018, F821
|
||||
container_to_host # noqa: B018, F821
|
||||
sync_results_to_host # noqa: B018, F821
|
||||
timed_out # noqa: B018, F821
|
||||
timeout_seconds # noqa: B018, F821
|
||||
workspace_folder # noqa: B018, F821
|
||||
execute_tool # noqa: B018, F821
|
||||
wrap_service_method # noqa: B018, F821
|
||||
|
||||
Reference in New Issue
Block a user