Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
226 changes: 226 additions & 0 deletions crates/tools/src/fs/contract_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -889,6 +889,232 @@ async fn sandbox_write_via_registry_sends_base64_to_bridge() {
assert!(cmd.contains(&BASE64.encode(b"sandboxed write")));
}

#[tokio::test]
async fn sandbox_write_home_path_via_registry_uses_bridge_not_host_parent_resolution() {
use {
crate::{
exec::ExecResult,
fs::sandbox_bridge::test_helpers::MockSandbox,
sandbox::{Sandbox, SandboxConfig, SandboxRouter},
},
base64::{Engine as _, engine::general_purpose::STANDARD as BASE64},
std::sync::Arc,
};

let mock = MockSandbox::new(vec![ExecResult {
stdout: String::new(),
stderr: String::new(),
exit_code: 0,
}]);
let backend: Arc<dyn Sandbox> = mock.clone();
let router = Arc::new(SandboxRouter::with_backend(
SandboxConfig::default(),
backend,
));
router.set_override("sandboxed", true).await;

let mut registry = ToolRegistry::new();
register_fs_tools(&mut registry, FsToolsContext {
sandbox_router: Some(router),
..FsToolsContext::default()
});

let write = registry.get("Write").unwrap();
let value = write
.execute(json!({
"file_path": "/home/sandbox/write_probe.txt",
"content": "sandbox home write",
"_session_key": "sandboxed",
}))
.await
.unwrap();

assert_eq!(value["bytes_written"], 18);
let cmd = mock.last_command().unwrap();
assert!(cmd.contains("/home/sandbox/write_probe.txt"));
assert!(cmd.contains(&BASE64.encode(b"sandbox home write")));
}

#[tokio::test]
async fn sandbox_read_and_write_workspace_paths_via_registry_use_bridge() {
use {
crate::{
exec::ExecResult,
fs::sandbox_bridge::test_helpers::MockSandbox,
sandbox::{Sandbox, SandboxConfig, SandboxRouter},
},
base64::{Engine as _, engine::general_purpose::STANDARD as BASE64},
std::sync::Arc,
};

let workspace_file = moltis_config::data_dir().join("notes/workspace_probe.txt");
let workspace_file = workspace_file.display().to_string();
let mock = MockSandbox::new(vec![
ExecResult {
stdout: BASE64.encode(b"workspace read"),
stderr: String::new(),
exit_code: 0,
},
ExecResult {
stdout: String::new(),
stderr: String::new(),
exit_code: 0,
},
]);
let backend: Arc<dyn Sandbox> = mock.clone();
let router = Arc::new(SandboxRouter::with_backend(
SandboxConfig::default(),
backend,
));
router.set_override("sandboxed", true).await;

let mut registry = ToolRegistry::new();
register_fs_tools(&mut registry, FsToolsContext {
sandbox_router: Some(router),
..FsToolsContext::default()
});

let read = registry.get("Read").unwrap();
let value = read
.execute(json!({
"file_path": workspace_file,
"_session_key": "sandboxed",
}))
.await
.unwrap();
assert!(
value["content"]
.as_str()
.unwrap()
.contains("workspace read")
);

let write = registry.get("Write").unwrap();
let value = write
.execute(json!({
"file_path": workspace_file,
"content": "workspace write",
"_session_key": "sandboxed",
}))
.await
.unwrap();

assert_eq!(value["bytes_written"], 15);
let cmd = mock.last_command().unwrap();
assert!(cmd.contains("notes/workspace_probe.txt"));
assert!(cmd.contains(&BASE64.encode(b"workspace write")));
}

#[tokio::test]
async fn sandbox_edit_home_path_via_registry_reads_and_writes_through_bridge() {
use {
crate::{
exec::ExecResult,
fs::sandbox_bridge::test_helpers::MockSandbox,
sandbox::{Sandbox, SandboxConfig, SandboxRouter},
},
base64::{Engine as _, engine::general_purpose::STANDARD as BASE64},
std::sync::Arc,
};

let mock = MockSandbox::new(vec![
ExecResult {
stdout: BASE64.encode(b"before\n"),
stderr: String::new(),
exit_code: 0,
},
ExecResult {
stdout: String::new(),
stderr: String::new(),
exit_code: 0,
},
]);
let backend: Arc<dyn Sandbox> = mock.clone();
let router = Arc::new(SandboxRouter::with_backend(
SandboxConfig::default(),
backend,
));
router.set_override("sandboxed", true).await;

let mut registry = ToolRegistry::new();
register_fs_tools(&mut registry, FsToolsContext {
sandbox_router: Some(router),
..FsToolsContext::default()
});

let edit = registry.get("Edit").unwrap();
let value = edit
.execute(json!({
"file_path": "/home/sandbox/edit_probe.txt",
"old_string": "before",
"new_string": "after",
"_session_key": "sandboxed",
}))
.await
.unwrap();

assert_eq!(value["replacements"], 1);
let cmd = mock.last_command().unwrap();
assert!(cmd.contains("/home/sandbox/edit_probe.txt"));
assert!(cmd.contains(&BASE64.encode(b"after\n")));
}

#[tokio::test]
async fn sandbox_multi_edit_home_path_via_registry_reads_and_writes_through_bridge() {
use {
crate::{
exec::ExecResult,
fs::sandbox_bridge::test_helpers::MockSandbox,
sandbox::{Sandbox, SandboxConfig, SandboxRouter},
},
base64::{Engine as _, engine::general_purpose::STANDARD as BASE64},
std::sync::Arc,
};

let mock = MockSandbox::new(vec![
ExecResult {
stdout: BASE64.encode(b"alpha beta\n"),
stderr: String::new(),
exit_code: 0,
},
ExecResult {
stdout: String::new(),
stderr: String::new(),
exit_code: 0,
},
]);
let backend: Arc<dyn Sandbox> = mock.clone();
let router = Arc::new(SandboxRouter::with_backend(
SandboxConfig::default(),
backend,
));
router.set_override("sandboxed", true).await;

let mut registry = ToolRegistry::new();
register_fs_tools(&mut registry, FsToolsContext {
sandbox_router: Some(router),
..FsToolsContext::default()
});

let multi_edit = registry.get("MultiEdit").unwrap();
let value = multi_edit
.execute(json!({
"file_path": "/home/sandbox/multi_probe.txt",
"edits": [
{ "old_string": "alpha", "new_string": "ALPHA" },
{ "old_string": "beta", "new_string": "BETA" }
],
"_session_key": "sandboxed",
}))
.await
.unwrap();

assert_eq!(value["edits_applied"], 2);
let cmd = mock.last_command().unwrap();
assert!(cmd.contains("/home/sandbox/multi_probe.txt"));
assert!(cmd.contains(&BASE64.encode(b"ALPHA BETA\n")));
}

#[tokio::test]
async fn sandbox_write_serializes_same_file_mutations_via_registry() {
let probe = ConcurrentExecProbeSandbox::new();
Expand Down
4 changes: 3 additions & 1 deletion crates/tools/src/fs/edit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -476,7 +476,9 @@ impl AgentTool for EditTool {
`old_string` is not unique in the file unless `replace_all=true`. \
This uniqueness requirement prevents the most common class of \
edit mistakes. Supply enough context in `old_string` to make the \
match unique."
match unique. In sandboxed sessions, use absolute workspace/data paths \
or `/home/sandbox/...` paths; both are read and written through the \
sandbox when needed."
}

fn parameters_schema(&self) -> Value {
Expand Down
4 changes: 3 additions & 1 deletion crates/tools/src/fs/multi_edit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,9 @@ impl AgentTool for MultiEditTool {
"Apply multiple sequential edits to a single file as one atomic \
operation. Each edit sees the output of previous edits. Either all \
edits succeed or none are applied (full rollback on any failure). \
Each edit follows the same uniqueness rules as `Edit`."
Each edit follows the same uniqueness rules as `Edit`. In sandboxed \
sessions, use absolute workspace/data paths or `/home/sandbox/...` \
paths; both are read and written through the sandbox when needed."
}

fn parameters_schema(&self) -> Value {
Expand Down
4 changes: 3 additions & 1 deletion crates/tools/src/fs/read/tool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -528,7 +528,9 @@ impl AgentTool for ReadTool {
Supports `offset` (1-indexed line to start at) and `limit` (max lines \
to return) for paginating large files. Returns structured JSON with \
the file's content, total line count, and truncation flag. Binary \
files return a typed marker instead of garbage."
files return a typed marker instead of garbage. In sandboxed sessions, \
use absolute workspace/data paths or `/home/sandbox/...` paths; both \
are routed through the sandbox when needed."
}

fn parameters_schema(&self) -> Value {
Expand Down
4 changes: 3 additions & 1 deletion crates/tools/src/fs/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,9 @@ impl AgentTool for WriteTool {
fn description(&self) -> &str {
"Atomically write a file to the local filesystem. Parent directories \
must already exist. Refuses to follow symlinks. The entire file is \
replaced — use `Edit` for surgical changes."
replaced — use `Edit` for surgical changes. In sandboxed sessions, \
use absolute workspace/data paths or `/home/sandbox/...` paths; parent \
directories must already exist in the target namespace."
}

fn parameters_schema(&self) -> Value {
Expand Down
48 changes: 43 additions & 5 deletions crates/tools/src/sandbox/docker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,18 @@ impl DockerSandbox {
}
}

#[cfg(test)]
pub(crate) fn with_cli(config: SandboxConfig, cli: &'static str) -> Self {
Self {
config,
kind: BackendKind::Docker,
cli,
backend_label: "docker",
provisioned: Mutex::new(HashSet::new()),
startup_gates: Mutex::new(HashMap::new()),
}
}

fn image(&self) -> &str {
self.config
.image
Expand Down Expand Up @@ -758,7 +770,10 @@ impl Sandbox for DockerSandbox {
max_bytes,
)
.await?;
if !matches!(host_result, SandboxReadResult::NotFound) {
if matches!(
host_result,
SandboxReadResult::Ok(_) | SandboxReadResult::TooLarge(_)
) {
return Ok(host_result);
}

Expand All @@ -780,13 +795,24 @@ impl Sandbox for DockerSandbox {
content: &[u8],
) -> Result<Option<serde_json::Value>> {
if let Some(host_path) = self.mounted_host_path(id, file_path) {
return native_host_write_file(
let host_result = native_host_write_file(
host_path
.to_str()
.ok_or_else(|| Error::message("mounted host path contains invalid UTF-8"))?,
content,
)
.await;
match host_result {
Ok(payload) => return Ok(payload),
Err(error) => {
debug!(
guest_path = file_path,
host_path = %host_path.display(),
%error,
"mounted host write failed; falling back to container write"
);
},
}
}

let container_name = self.container_name(id);
Expand All @@ -795,13 +821,25 @@ impl Sandbox for DockerSandbox {

async fn list_files(&self, id: &SandboxId, root: &str) -> Result<SandboxListFilesResult> {
if let Some(host_path) = self.mounted_host_path(id, root) {
let host_files = native_host_list_files(
let host_result = native_host_list_files(
host_path
.to_str()
.ok_or_else(|| Error::message("mounted host path contains invalid UTF-8"))?,
)
.await?;
return remap_host_list_result_to_guest(root, &host_path, host_files);
.await;
match host_result {
Ok(host_files) => {
return remap_host_list_result_to_guest(root, &host_path, host_files);
},
Err(error) => {
debug!(
guest_path = root,
host_path = %host_path.display(),
%error,
"mounted host list failed; falling back to container list"
);
},
}
}
Comment on lines 822 to 843

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 No test for list_files host-mount fallback

The analogous write_file fallback has a dedicated regression test (test_docker_write_file_falls_back_to_container_copy_when_host_mount_is_inaccessible), but the list_files fallback added here has no corresponding test. native_host_list_files returns Err on non-NotFound IO errors (e.g., permission denied on the directory), so a similar fake-CLI test that sets host_data_dir to a non-existent path and asserts the container list_files call is reached would cover the new branch.


let container_name = self.container_name(id);
Expand Down
Loading
Loading