From 62eaefb1fa6b18b07e519acb6c5816b085b0128f Mon Sep 17 00:00:00 2001 From: Hare Date: Mon, 31 Aug 2026 16:52:27 +0900 Subject: [PATCH] feat: spill long bash output to worker temp storage --- crates/fs-operation/src/lib.rs | 2 + crates/fs-operation/src/operation.rs | 24 +- crates/standalone/src/host.rs | 16 +- crates/tools/src/bash.rs | 140 +++++++++++- crates/tools/src/grep.rs | 4 +- crates/tools/src/read.rs | 6 +- crates/tools/tests/edge_cases.rs | 13 +- crates/tools/tests/integration.rs | 13 +- crates/workdir/src/delegation.rs | 5 + crates/workdir/src/local.rs | 234 +++++++++++++++++++- crates/workdir/src/operation.rs | 7 + crates/worker-runtime/src/http_server.rs | 1 + crates/worker-runtime/src/worker_backend.rs | 17 +- crates/worker/examples/worker_protocol.rs | 4 +- crates/worker/src/bootstrap.rs | 69 +++++- crates/worker/src/controller.rs | 75 +++++-- crates/worker/src/entrypoint.rs | 2 + crates/worker/src/lib.rs | 2 +- crates/worker/src/spawn/tool.rs | 58 ++++- crates/worker/tests/compact_events_test.rs | 3 +- crates/worker/tests/controller_test.rs | 46 +++- crates/workspace-server/src/server.rs | 1 + 22 files changed, 664 insertions(+), 78 deletions(-) diff --git a/crates/fs-operation/src/lib.rs b/crates/fs-operation/src/lib.rs index fb2cd54f..1488d505 100644 --- a/crates/fs-operation/src/lib.rs +++ b/crates/fs-operation/src/lib.rs @@ -177,6 +177,8 @@ mod tests { fn logical_paths_reject_absolute_parent_and_backslash_forms() { assert!(FsPath::new("src/lib.rs").is_ok()); assert!(FsPath::new("/tmp/file").is_err()); + assert!(FsPath::new_scoped("/tmp/file").is_ok()); + assert!(FsPath::new_scoped("/tmp/../secret").is_err()); assert!(FsPath::new("../file").is_err()); assert!(FsPath::new("src\\lib.rs").is_err()); } diff --git a/crates/fs-operation/src/operation.rs b/crates/fs-operation/src/operation.rs index b69ce30b..a883b8ee 100644 --- a/crates/fs-operation/src/operation.rs +++ b/crates/fs-operation/src/operation.rs @@ -5,7 +5,8 @@ use serde::{Deserialize, Serialize}; use crate::FsError; -/// Logical path relative to the bound Workdir root. +/// Scope-checked filesystem path. Relative paths resolve below the bound +/// Workdir root; absolute paths require an explicit matching scope rule. #[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize)] #[serde(transparent)] pub struct FsPath(String); @@ -16,11 +17,30 @@ impl<'de> Deserialize<'de> for FsPath { D: serde::Deserializer<'de>, { let value = String::deserialize(deserializer)?; - Self::new(&value).map_err(serde::de::Error::custom) + Self::new_scoped(&value).map_err(serde::de::Error::custom) } } impl FsPath { + /// Construct a path for a scope-checked operation that may target an + /// explicitly granted absolute path outside the provider root. + pub fn new_scoped(value: impl Into) -> Result { + let value = value.into(); + if !Path::new(&value).is_absolute() { + return Self::new(value); + } + if value.contains('\\') { + return Err(FsError::InvalidPath(value)); + } + if Path::new(&value) + .components() + .any(|component| component == Component::ParentDir) + { + return Err(FsError::InvalidPath(value)); + } + Ok(Self(value)) + } + pub fn root() -> Self { Self(String::new()) } diff --git a/crates/standalone/src/host.rs b/crates/standalone/src/host.rs index 7bfc1e51..405ad8af 100644 --- a/crates/standalone/src/host.rs +++ b/crates/standalone/src/host.rs @@ -10,7 +10,9 @@ use session_store::{ CombinedStore, FsStore, FsWorkerStore, WorkerActiveSegmentRef, WorkerMetadataStore, }; use thiserror::Error; -use worker::bootstrap::{WorkerBootstrap, WorkerBootstrapError, WorkerBootstrapLayout}; +use worker::bootstrap::{ + WorkerBootstrap, WorkerBootstrapError, WorkerBootstrapLayout, bash_output_dir_for_worker_id, +}; use worker::controller::WorkerControllerTransport; use worker::ipc::protocol_session::{ WorkerProtocolSessionStreams, dispatch_worker_protocol_method, live_log_entry_event, @@ -115,6 +117,7 @@ impl StandaloneHost { WorkerFilesystemAuthority::local(launch.cwd.clone(), launch.cwd.clone()); let workspace_context = WorkerWorkspaceContext::local_filesystem(None); let runtime_base = store.runtime_dir(worker_id); + let bash_output_dir = bash_output_dir_for_worker_id(worker_id); let mut bootstrap = WorkerBootstrap::new( bootstrap_manifest, @@ -122,7 +125,10 @@ impl StandaloneHost { launch.prompt_catalog, workspace_context, filesystem_authority, - WorkerBootstrapLayout::Direct { runtime_base }, + WorkerBootstrapLayout::Direct { + runtime_base, + bash_output_dir, + }, WorkerControllerTransport::InProcess, ); if let Some(model_client) = model_client { @@ -208,6 +214,7 @@ impl StandaloneHost { ); let workspace_context = WorkerWorkspaceContext::local_filesystem(None); let runtime_base = store.runtime_dir(worker_id); + let bash_output_dir = bash_output_dir_for_worker_id(worker_id); let mut bootstrap = WorkerBootstrap::new( manifest, @@ -215,7 +222,10 @@ impl StandaloneHost { worker::PromptCatalogSource::builtins_only(), workspace_context, filesystem_authority, - WorkerBootstrapLayout::Direct { runtime_base }, + WorkerBootstrapLayout::Direct { + runtime_base, + bash_output_dir, + }, WorkerControllerTransport::InProcess, ); if let Some(model_client) = model_client { diff --git a/crates/tools/src/bash.rs b/crates/tools/src/bash.rs index bc573849..fb68fcbc 100644 --- a/crates/tools/src/bash.rs +++ b/crates/tools/src/bash.rs @@ -21,6 +21,7 @@ struct BashParams { pub(crate) struct BashTool { session: WorkdirSessionHandle, + output_dir: PathBuf, state: Arc>, } @@ -117,6 +118,7 @@ impl Tool for BashTool { command: params.command, timeout_secs, output_limit: INLINE_BYTE_BUDGET, + spill_dir: Some(self.output_dir.clone()), tool_call_id: Some(call_id.clone()), }) .await @@ -183,10 +185,15 @@ impl Tool for BashTool { let content = if output.content.is_empty() { None } else if output.truncated { - Some(format!( - "[showing bounded WorkdirSession command output; additional output was truncated]\n{}", - output.content - )) + let notice = match output.output_path { + Some(path) => format!( + "[showing bounded WorkdirSession command output; full output saved to {}]", + path.display() + ), + None => "[showing bounded WorkdirSession command output; additional output was truncated]" + .to_owned(), + }; + Some(format!("{notice}\n{}", output.content)) } else { Some(output.content) }; @@ -259,16 +266,137 @@ fn truncate_for_summary(command: &str) -> String { summary } -pub fn bash_tool(session: WorkdirSessionHandle, _output_dir: PathBuf) -> ToolDefinition { +pub fn bash_tool(session: WorkdirSessionHandle, output_dir: PathBuf) -> ToolDefinition { Arc::new(move || { let schema = schemars::schema_for!(BashParams); let meta = ToolMeta::new("Bash") - .description("Execute a shell command in the bound Workdir. Process start, bounded output, timeout and cancellation are owned by the WorkdirSession provider. This is not a sandbox.") + .description("Execute a shell command in the bound Workdir. Process start, bounded inline output, full-output spill, timeout and cancellation are owned by the WorkdirSession provider. This is not a sandbox.") .input_schema(serde_json::to_value(schema).expect("Bash schema serialization")); let tool: Arc = Arc::new(BashTool { session: session.clone(), + output_dir: output_dir.clone(), state: Arc::new(Mutex::new(BashExecutionState::default())), }); (meta, tool) }) } + +#[cfg(test)] +mod tests { + use std::sync::Arc; + + use manifest::{Permission, Scope, ScopeConfig, ScopeRule}; + use tempfile::TempDir; + use workdir::{LocalWorkdirSession, WorkdirSessionHandle}; + + use super::bash_tool; + use crate::{grep::grep_tool, read::read_tool, tracker::Tracker}; + + fn session_with_output_scope(root: &TempDir, output: &TempDir) -> WorkdirSessionHandle { + let scope = Scope::from_config(&ScopeConfig { + allow: vec![ + ScopeRule { + target: root.path().to_path_buf(), + permission: Permission::Write, + recursive: true, + }, + ScopeRule { + target: output.path().to_path_buf(), + permission: Permission::Read, + recursive: true, + }, + ], + deny: Vec::new(), + }) + .unwrap(); + Arc::new(LocalWorkdirSession::new(scope, root.path().to_path_buf())) + } + + #[tokio::test] + async fn long_output_is_spilled_and_available_to_read_and_grep() { + let root = TempDir::new().unwrap(); + let output = TempDir::new().unwrap(); + let session = session_with_output_scope(&root, &output); + let (_, bash) = bash_tool(session.clone(), output.path().to_path_buf())(); + let command = "i=0; while [ $i -lt 2000 ]; do printf 'line-%04d\\n' \"$i\"; i=$((i+1)); done; printf 'FINAL-NEEDLE\\n'"; + let result = bash + .execute( + &serde_json::json!({ "command": command }).to_string(), + Default::default(), + ) + .await + .unwrap(); + let rendered = result.content.expect("bounded Bash output"); + let artifact = std::fs::read_dir(output.path()) + .unwrap() + .next() + .expect("artifact entry") + .unwrap() + .path(); + + assert!(rendered.contains("full output saved to")); + assert!(rendered.contains(&artifact.display().to_string())); + let retained = std::fs::read_to_string(&artifact).unwrap(); + assert!(retained.starts_with("line-0000\n")); + assert!(retained.ends_with("FINAL-NEEDLE\n")); + assert_eq!(retained.lines().count(), 2001); + + let (_, read) = read_tool(session.clone(), Tracker::new())(); + let read_result = read + .execute( + &serde_json::json!({ + "file_path": artifact, + "offset": 2000, + "limit": 1, + }) + .to_string(), + Default::default(), + ) + .await + .unwrap(); + assert!( + read_result + .content + .expect("Read content") + .contains("FINAL-NEEDLE") + ); + + let (_, grep) = grep_tool(session)(); + let grep_result = grep + .execute( + &serde_json::json!({ + "pattern": "FINAL-NEEDLE", + "path": artifact, + "output_mode": "content", + }) + .to_string(), + Default::default(), + ) + .await + .unwrap(); + let grep_content = grep_result.content.expect("Grep content"); + assert!( + grep_content.contains("FINAL-NEEDLE"), + "unexpected Grep content: {grep_content:?}" + ); + } + + #[tokio::test] + async fn short_output_does_not_leave_a_spill_artifact() { + let root = TempDir::new().unwrap(); + let output = TempDir::new().unwrap(); + let session = session_with_output_scope(&root, &output); + let (_, bash) = bash_tool(session, output.path().to_path_buf())(); + + let result = bash + .execute( + &serde_json::json!({ "command": "printf short" }).to_string(), + Default::default(), + ) + .await + .unwrap(); + + assert_eq!(result.content.as_deref(), Some("short")); + assert_eq!(std::fs::read_dir(output.path()).unwrap().count(), 0); + } +} diff --git a/crates/tools/src/grep.rs b/crates/tools/src/grep.rs index 785470d4..4774e6ff 100644 --- a/crates/tools/src/grep.rs +++ b/crates/tools/src/grep.rs @@ -22,7 +22,7 @@ enum OutputMode { #[derive(Debug, Deserialize, JsonSchema)] struct GrepParams { pattern: String, - /// Logical Workdir-relative file or directory to search. Defaults to the Workdir root. + /// Workdir-relative path, or an absolute path covered by readable scope. Defaults to the Workdir root. #[serde(default)] path: Option, #[serde(default)] @@ -61,7 +61,7 @@ impl Tool for GrepTool { let params: GrepParams = serde_json::from_str(input_json) .map_err(|error| ToolError::InvalidArgument(format!("invalid Grep input: {error}")))?; let path = match params.path { - Some(path) => WorkdirPath::new(&path).map_err(ToolsError::from)?, + Some(path) => WorkdirPath::new_scoped(&path).map_err(ToolsError::from)?, None => WorkdirPath::root(), }; let mode = match params.output_mode.unwrap_or_default() { diff --git a/crates/tools/src/read.rs b/crates/tools/src/read.rs index e743c56f..668f4a90 100644 --- a/crates/tools/src/read.rs +++ b/crates/tools/src/read.rs @@ -13,14 +13,14 @@ use workdir::{ReadRequest, WorkdirPath, WorkdirSessionHandle}; const DESCRIPTION: &str = "Read a text file from the local filesystem. \ Supports offset/limit for large files. Returns line-numbered output (1-based). \ Directories cannot be read. The file must be read before Write or Edit can \ -modify it. Paths are relative to the bound Workdir."; +modify it. Paths are Workdir-relative unless an absolute path is explicitly readable."; const DEFAULT_LIMIT: usize = 2000; const PROVIDER_BYTE_LIMIT: usize = 256 * 1024; #[derive(Debug, Deserialize, schemars::JsonSchema)] pub(crate) struct ReadParams { - /// Logical path relative to the bound Workdir root. + /// Workdir-relative path, or an absolute path covered by readable scope. pub file_path: String, /// 0-based line offset from the start. Defaults to 0. #[serde(default)] @@ -47,7 +47,7 @@ impl Tool for ReadTool { let offset = params.offset.unwrap_or(0); let limit = params.limit.unwrap_or(DEFAULT_LIMIT).max(1); - let path = WorkdirPath::new(¶ms.file_path).map_err(ToolsError::from)?; + let path = WorkdirPath::new_scoped(¶ms.file_path).map_err(ToolsError::from)?; tracing::debug!(path = %path, offset, limit, "Read"); let result = self diff --git a/crates/tools/tests/edge_cases.rs b/crates/tools/tests/edge_cases.rs index 6e139d07..e00549aa 100644 --- a/crates/tools/tests/edge_cases.rs +++ b/crates/tools/tests/edge_cases.rs @@ -224,20 +224,23 @@ async fn very_long_single_line() { } #[tokio::test] -async fn absolute_path_is_rejected() { - let (dir, _spill, reg) = setup(); +async fn absolute_path_requires_matching_read_scope() { + let (_dir, _spill, reg) = setup(); + let outside = tempfile::tempdir().unwrap(); + let outside_file = outside.path().join("outside.txt"); + std::fs::write(&outside_file, "secret").unwrap(); let read = reg.get("Read"); let err = read .execute( - &json!({ "file_path": dir.path().join("outside.txt") }).to_string(), + &json!({ "file_path": outside_file }).to_string(), Default::default(), ) .await .unwrap_err(); let msg = format!("{err}"); assert!( - msg.contains("invalid logical filesystem path"), - "absolute path was not rejected as invalid: {msg}" + msg.contains("outside allowed scope"), + "absolute path escaped readable scope: {msg}" ); } diff --git a/crates/tools/tests/integration.rs b/crates/tools/tests/integration.rs index 98e2dab6..3888c659 100644 --- a/crates/tools/tests/integration.rs +++ b/crates/tools/tests/integration.rs @@ -394,14 +394,21 @@ async fn bash_inherits_workdir_cwd() { } #[tokio::test] -async fn bash_provider_output_does_not_expose_internal_paths() { +async fn bash_provider_output_exposes_readable_retained_path() { let (_dir, spill, reg) = setup(); let bash = reg.get("Bash"); let out = call(&bash, json!({ "command": "printf 'x%.0s' {1..20480}" })).await; let body = out.content.unwrap(); assert!(body.contains("bounded WorkdirSession command output")); - assert!(!body.contains(spill.path().to_str().unwrap())); - assert_eq!(std::fs::read_dir(spill.path()).unwrap().count(), 0); + assert!(body.contains("full output saved to")); + assert!(body.contains(spill.path().to_str().unwrap())); + let artifact = std::fs::read_dir(spill.path()) + .unwrap() + .next() + .expect("retained output") + .unwrap() + .path(); + assert_eq!(std::fs::metadata(artifact).unwrap().len(), 20_480); } #[tokio::test] diff --git a/crates/workdir/src/delegation.rs b/crates/workdir/src/delegation.rs index 031ac9b5..17598dab 100644 --- a/crates/workdir/src/delegation.rs +++ b/crates/workdir/src/delegation.rs @@ -743,6 +743,7 @@ mod tests { command: command.into(), timeout_secs: 5, output_limit: 1024, + spill_dir: None, tool_call_id: Some(tool_call_id.into()), }) .await @@ -770,6 +771,7 @@ mod tests { command: "printf ready; sleep 0.2; printf done".into(), timeout_secs: 5, output_limit: 1024, + spill_dir: None, tool_call_id: Some("tool-delegated".into()), }) .await @@ -858,6 +860,7 @@ mod tests { command: "printf denied".into(), timeout_secs: 5, output_limit: 1024, + spill_dir: None, tool_call_id: Some("read-only-command".into()), }) .await, @@ -993,6 +996,7 @@ mod tests { command: "printf revoked".into(), timeout_secs: 5, output_limit: 1024, + spill_dir: None, tool_call_id: Some("revoked-child-command".into()), }) .await, @@ -1171,6 +1175,7 @@ mod tests { command: "printf closed".into(), timeout_secs: 5, output_limit: 1024, + spill_dir: None, tool_call_id: Some("closed-parent-command".into()), }) .await, diff --git a/crates/workdir/src/local.rs b/crates/workdir/src/local.rs index a897076f..8f9556dd 100644 --- a/crates/workdir/src/local.rs +++ b/crates/workdir/src/local.rs @@ -10,9 +10,7 @@ use std::collections::{BTreeMap, HashMap}; use std::fmt::Debug; -#[cfg(test)] -use std::io::Write as _; -use std::io::{Read as _, Seek as _, SeekFrom}; +use std::io::{Read as _, Seek as _, SeekFrom, Write as _}; use std::path::{Path, PathBuf}; use std::process::Stdio; use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; @@ -691,6 +689,11 @@ impl WorkdirSession for LocalWorkdirSession { async fn start_command(&self, request: CommandRequest) -> Result { self.ensure_capability(WorkdirSessionCapability::Command)?; self.ensure_open()?; + if let Some(spill_dir) = request.spill_dir.as_deref() + && !self.inner.scope.snapshot().is_readable(spill_dir) + { + return Err(WorkdirError::OutOfScope(spill_dir.to_path_buf())); + } let id = self.inner.next_command_id.fetch_add(1, Ordering::Relaxed); let handle = CommandHandle(format!("command-{id}")); let cwd = self.inner.cwd.clone(); @@ -776,6 +779,7 @@ impl WorkdirSession for LocalWorkdirSession { content: String::new(), next_cursor: None, truncated: false, + output_path: None, }); } drop(commands); @@ -792,6 +796,7 @@ impl WorkdirSession for LocalWorkdirSession { content: String::new(), next_cursor: None, truncated: false, + output_path: None, }); } break commands @@ -901,6 +906,7 @@ fn command_output_page(output: &CommandOutput, cursor: usize, limit: usize) -> C content, next_cursor: (end < total_chars).then_some(end), truncated: output.truncated || end < total_chars, + output_path: output.output_path.clone(), } } @@ -1059,6 +1065,22 @@ async fn run_command( let (content, truncated) = read_command_output_files(&stdout_path, &stderr_path, request.output_limit.max(1))?; + let output_path = match (truncated, request.spill_dir) { + (true, Some(spill_dir)) => { + let stdout_path = stdout_path.to_path_buf(); + let stderr_path = stderr_path.to_path_buf(); + Some( + tokio::task::spawn_blocking(move || { + persist_command_output(&stdout_path, &stderr_path, &spill_dir) + }) + .await + .map_err(|error| { + WorkdirError::Unavailable(format!("Bash output spill task failed: {error}")) + })??, + ) + } + _ => None, + }; Ok(CommandOutput { status, exit_code, @@ -1066,6 +1088,7 @@ async fn run_command( content, next_cursor: None, truncated, + output_path, }) } @@ -1154,6 +1177,59 @@ fn stable_utf8_prefix_len(bytes: &[u8]) -> usize { inspected } +fn persist_command_output( + stdout_path: &Path, + stderr_path: &Path, + spill_dir: &Path, +) -> Result { + std::fs::create_dir_all(spill_dir).map_err(|error| WorkdirError::io(spill_dir, error))?; + let mut artifact = tempfile::Builder::new() + .prefix("bash-") + .suffix(".log") + .tempfile_in(spill_dir) + .map_err(|error| WorkdirError::io(spill_dir, error))?; + let artifact_path = artifact.path().to_path_buf(); + + let mut stdout = + std::fs::File::open(stdout_path).map_err(|error| WorkdirError::io(stdout_path, error))?; + let stdout_len = stdout + .metadata() + .map_err(|error| WorkdirError::io(stdout_path, error))? + .len(); + std::io::copy(&mut stdout, &mut artifact) + .map_err(|error| WorkdirError::io(&artifact_path, error))?; + + let mut stderr = + std::fs::File::open(stderr_path).map_err(|error| WorkdirError::io(stderr_path, error))?; + let stderr_len = stderr + .metadata() + .map_err(|error| WorkdirError::io(stderr_path, error))? + .len(); + if stdout_len > 0 && stderr_len > 0 { + stdout + .seek(SeekFrom::End(-1)) + .map_err(|error| WorkdirError::io(stdout_path, error))?; + let mut last = [0_u8; 1]; + stdout + .read_exact(&mut last) + .map_err(|error| WorkdirError::io(stdout_path, error))?; + if last[0] != b'\n' { + artifact + .write_all(b"\n") + .map_err(|error| WorkdirError::io(&artifact_path, error))?; + } + } + std::io::copy(&mut stderr, &mut artifact) + .map_err(|error| WorkdirError::io(&artifact_path, error))?; + artifact + .flush() + .map_err(|error| WorkdirError::io(&artifact_path, error))?; + artifact + .keep() + .map(|(_, path)| path) + .map_err(|error| WorkdirError::io(&artifact_path, error.error)) +} + fn read_command_output_files( stdout_path: &Path, stderr_path: &Path, @@ -1440,6 +1516,7 @@ mod tests { command: "sleep 30".to_owned(), timeout_secs: 60, output_limit: 1024, + spill_dir: None, tool_call_id: None, }, ) @@ -1966,6 +2043,7 @@ mod tests { command: "pwd && printf provider-command".into(), timeout_secs: 5, output_limit: 4096, + spill_dir: None, tool_call_id: None, }, ) @@ -1991,6 +2069,151 @@ mod tests { ); } + #[tokio::test] + async fn explicitly_scoped_absolute_artifact_can_be_read_and_grepped() { + let dir = TempDir::new().unwrap(); + let spill = TempDir::new().unwrap(); + let artifact = spill.path().join("bash-output.log"); + std::fs::write(&artifact, "first\nFINAL-NEEDLE\nlast\n").unwrap(); + let scope = Scope::from_config(&ScopeConfig { + allow: vec![ + ScopeRule { + target: dir.path().to_path_buf(), + permission: Permission::Write, + recursive: true, + }, + ScopeRule { + target: spill.path().to_path_buf(), + permission: Permission::Read, + recursive: true, + }, + ], + deny: Vec::new(), + }) + .unwrap(); + let workdir = LocalWorkdirSession::new(scope, dir.path().to_path_buf()); + let artifact_path = WorkdirPath::new_scoped(artifact.to_string_lossy()).unwrap(); + + let read = WorkdirSession::read( + &workdir, + ReadRequest { + path: artifact_path.clone(), + offset: 1, + limit: 1, + max_bytes: 1024, + }, + ) + .await + .unwrap(); + assert_eq!(String::from_utf8(read.bytes).unwrap(), "FINAL-NEEDLE\n"); + + let grep = WorkdirSession::grep( + &workdir, + GrepRequest { + pattern: "FINAL-NEEDLE".into(), + path: artifact_path, + glob: None, + file_type: None, + case_insensitive: false, + before_context: 0, + after_context: 0, + multiline: false, + output_mode: crate::GrepOutputMode::Content, + limit: 10, + offset: 0, + }, + ) + .await + .unwrap(); + assert_eq!(grep.match_count, 1); + assert!(grep.output.contains("FINAL-NEEDLE")); + } + + #[tokio::test] + async fn command_rejects_spill_directory_without_read_scope() { + let dir = TempDir::new().unwrap(); + let spill = TempDir::new().unwrap(); + let workdir = make_fs(&dir); + + let error = WorkdirSession::start_command( + &workdir, + CommandRequest { + command: "printf hidden".into(), + timeout_secs: 5, + output_limit: 1, + spill_dir: Some(spill.path().to_path_buf()), + tool_call_id: None, + }, + ) + .await + .unwrap_err(); + + assert!(matches!(error, WorkdirError::OutOfScope(path) if path == spill.path())); + } + + #[tokio::test] + async fn truncated_command_output_is_retained_in_the_requested_spill_directory() { + let dir = TempDir::new().unwrap(); + let spill = TempDir::new().unwrap(); + let scope = Scope::from_config(&ScopeConfig { + allow: vec![ + ScopeRule { + target: dir.path().to_path_buf(), + permission: Permission::Write, + recursive: true, + }, + ScopeRule { + target: spill.path().to_path_buf(), + permission: Permission::Read, + recursive: true, + }, + ], + deny: Vec::new(), + }) + .unwrap(); + let workdir = LocalWorkdirSession::new(scope, dir.path().to_path_buf()); + let handle = WorkdirSession::start_command( + &workdir, + CommandRequest { + command: "i=0; while [ $i -lt 200 ]; do printf 'line-%03d\\n' \"$i\"; i=$((i+1)); done; printf 'FINAL-NEEDLE\\n'".into(), + timeout_secs: 5, + output_limit: 64, + spill_dir: Some(spill.path().to_path_buf()), + tool_call_id: None, + }, + ) + .await + .unwrap(); + let output = WorkdirSession::command_output( + &workdir, + CommandOutputRequest { + handle, + cursor: 0, + limit: 4096, + wait: true, + }, + ) + .await + .unwrap(); + + assert!(output.truncated); + let output_path = output.output_path.expect("retained output path"); + assert_eq!(output_path.parent(), Some(spill.path())); + let retained = std::fs::read_to_string(&output_path).unwrap(); + assert!(retained.starts_with("line-000\n")); + assert!(retained.ends_with("FINAL-NEEDLE\n")); + assert_eq!(retained.lines().count(), 201); + + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt as _; + assert_eq!( + std::fs::metadata(output_path).unwrap().permissions().mode() & 0o777, + 0o600 + ); + } + } + #[tokio::test] async fn completed_command_output_can_be_read_in_bounded_unicode_pages() { let dir = TempDir::new().unwrap(); @@ -2001,6 +2224,7 @@ mod tests { command: "printf 'aéz'".into(), timeout_secs: 5, output_limit: 1024, + spill_dir: None, tool_call_id: None, }, ) @@ -2120,6 +2344,7 @@ mod tests { content: "done".into(), next_cursor: None, truncated: false, + output_path: None, }) }); workdir.inner.commands.lock().await.insert( @@ -2224,6 +2449,7 @@ mod tests { command: "printf ready; printf warning >&2; sleep 0.2; printf done".into(), timeout_secs: 5, output_limit: 1024, + spill_dir: None, tool_call_id: Some("tool-7".into()), }, ) @@ -2327,6 +2553,7 @@ mod tests { command: "sleep 30".into(), timeout_secs: 1, output_limit: 1024, + spill_dir: None, tool_call_id: None, }, ) @@ -2396,6 +2623,7 @@ mod tests { command: "sleep 30".into(), timeout_secs: 60, output_limit: 1024, + spill_dir: None, tool_call_id: None, }, ) diff --git a/crates/workdir/src/operation.rs b/crates/workdir/src/operation.rs index 47527ad8..67858685 100644 --- a/crates/workdir/src/operation.rs +++ b/crates/workdir/src/operation.rs @@ -1,3 +1,5 @@ +use std::path::PathBuf; + use serde::{Deserialize, Serialize}; #[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)] @@ -9,6 +11,9 @@ pub struct CommandRequest { pub command: String, pub timeout_secs: u64, pub output_limit: usize, + /// Provider-local directory where complete output is retained when the + /// inline result exceeds `output_limit`. + pub spill_dir: Option, /// Optional caller-owned correlation id. Bash supplies its tool-call id so /// user-facing command telemetry can update the corresponding Console row /// without exposing provider/session handles. @@ -96,4 +101,6 @@ pub struct CommandOutput { pub content: String, pub next_cursor: Option, pub truncated: bool, + /// Complete output retained by the provider when `truncated` is true. + pub output_path: Option, } diff --git a/crates/worker-runtime/src/http_server.rs b/crates/worker-runtime/src/http_server.rs index 610ea770..a8486523 100644 --- a/crates/worker-runtime/src/http_server.rs +++ b/crates/worker-runtime/src/http_server.rs @@ -703,6 +703,7 @@ async fn run_workdir_session_operation( content: String::new(), next_cursor: Some(cursor), truncated: false, + output_path: None, }, }; WorkdirSessionOperationResult::CommandOutput(output) diff --git a/crates/worker-runtime/src/worker_backend.rs b/crates/worker-runtime/src/worker_backend.rs index a35114e5..5f587c1b 100644 --- a/crates/worker-runtime/src/worker_backend.rs +++ b/crates/worker-runtime/src/worker_backend.rs @@ -60,6 +60,7 @@ use worker::{ Worker, WorkerBootstrap, WorkerBootstrapError, WorkerBootstrapLayout, WorkerControllerTransport, WorkerError, WorkerFilesystemAuthority, WorkerHandle, WorkerSharedState, WorkerWorkspaceContext, WorkspaceClient, WorkspaceId, + bash_output_dir_for_worker_id, }; const DEFAULT_BACKEND_ID: &str = "worker-crate"; @@ -886,6 +887,7 @@ impl RuntimeWorkerFactory for ProfileRuntimeWorkerFactory { let run_dir = worker_aggregate_dir .join("runs") .join(request.run_generation.to_string()); + let bash_output_dir = bash_output_dir_for_worker_id(&request.worker_ref.worker_id); let mut prepared = WorkerBootstrap::new( manifest, store, @@ -894,6 +896,7 @@ impl RuntimeWorkerFactory for ProfileRuntimeWorkerFactory { filesystem_authority, WorkerBootstrapLayout::RuntimeManagedRun { run_dir: run_dir.clone(), + bash_output_dir, }, self.controller_transport, ) @@ -1131,10 +1134,12 @@ impl RuntimeWorkerFactory for ProfileRuntimeWorkerFactory { let run_dir = worker_aggregate_dir .join("runs") .join(request.run_generation.to_string()); + let bash_output_dir = bash_output_dir_for_worker_id(&request.worker_ref.worker_id); let started = PreparedWorker::new( worker, WorkerBootstrapLayout::RuntimeManagedRun { run_dir: run_dir.clone(), + bash_output_dir, }, self.controller_transport, ) @@ -2552,10 +2557,14 @@ mod tests { ) .await .map_err(|err| err.to_string())?; - let (handle, shutdown_rx) = - WorkerController::spawn_runtime_managed(worker, &self.runtime_base) - .await - .map_err(|err| err.to_string())?; + let bash_output_dir = self.runtime_base.join("bash-output"); + let (handle, shutdown_rx) = WorkerController::spawn_runtime_managed( + worker, + &self.runtime_base, + &bash_output_dir, + ) + .await + .map_err(|err| err.to_string())?; Ok(RuntimeWorkerController { handle, shutdown: Arc::new(tokio::sync::Mutex::new(Some(shutdown_rx))), diff --git a/crates/worker/examples/worker_protocol.rs b/crates/worker/examples/worker_protocol.rs index fc54a1d0..1e2c618f 100644 --- a/crates/worker/examples/worker_protocol.rs +++ b/crates/worker/examples/worker_protocol.rs @@ -47,7 +47,9 @@ async fn main() -> Result<(), Box> { let worker = worker::Worker::from_manifest_toml(&toml, store).await?; let runtime_tmp = tempfile::tempdir()?; - let (handle, _shutdown_rx) = WorkerController::spawn(worker, runtime_tmp.path()).await?; + let bash_output_dir = runtime_tmp.path().join("bash-output"); + let (handle, _shutdown_rx) = + WorkerController::spawn(worker, runtime_tmp.path(), &bash_output_dir).await?; // Check initial status via shared state println!("[shared_state] {}", handle.shared_state.status_json()); diff --git a/crates/worker/src/bootstrap.rs b/crates/worker/src/bootstrap.rs index c5ec07c5..ad292df1 100644 --- a/crates/worker/src/bootstrap.rs +++ b/crates/worker/src/bootstrap.rs @@ -17,9 +17,28 @@ use manifest::WorkerManifest; #[derive(Debug, Clone)] pub enum WorkerBootstrapLayout { /// A direct Worker rooted below the supplied runtime base directory. - Direct { runtime_base: PathBuf }, + Direct { + runtime_base: PathBuf, + bash_output_dir: PathBuf, + }, /// A runtime-managed Worker with an exact persisted run directory. - RuntimeManagedRun { run_dir: PathBuf }, + RuntimeManagedRun { + run_dir: PathBuf, + bash_output_dir: PathBuf, + }, +} + +/// Return the temporary Bash spill directory owned by a stable Worker identity. +/// +/// The directory deliberately lives outside session/run-generation storage so a +/// restarted controller for the same Worker keeps the same readable artifact +/// boundary. +pub fn bash_output_dir_for_worker_id(worker_id: impl std::fmt::Display) -> PathBuf { + std::env::temp_dir() + .join("yoi") + .join("workers") + .join(worker_id.to_string()) + .join("bash-output") } /// Construction and controller inputs that are stable for one Worker launch. @@ -204,12 +223,29 @@ where { let cleanup_session = worker.workdir_session().cloned(); let controller = match layout { - WorkerBootstrapLayout::Direct { runtime_base } => { - WorkerController::spawn_with_transport(worker, &runtime_base, transport).await + WorkerBootstrapLayout::Direct { + runtime_base, + bash_output_dir, + } => { + WorkerController::spawn_with_transport( + worker, + &runtime_base, + &bash_output_dir, + transport, + ) + .await } - WorkerBootstrapLayout::RuntimeManagedRun { run_dir } => { - WorkerController::spawn_runtime_managed_run_with_transport(worker, &run_dir, transport) - .await + WorkerBootstrapLayout::RuntimeManagedRun { + run_dir, + bash_output_dir, + } => { + WorkerController::spawn_runtime_managed_run_with_transport( + worker, + &run_dir, + &bash_output_dir, + transport, + ) + .await } }; @@ -227,3 +263,22 @@ where } } } + +#[cfg(test)] +mod tests { + use super::bash_output_dir_for_worker_id; + + #[test] + fn bash_output_directory_is_stable_per_worker_below_system_temp() { + let path = bash_output_dir_for_worker_id("019c1234-worker"); + + assert_eq!( + path, + std::env::temp_dir() + .join("yoi") + .join("workers") + .join("019c1234-worker") + .join("bash-output") + ); + } +} diff --git a/crates/worker/src/controller.rs b/crates/worker/src/controller.rs index 5af8355b..7d35739e 100644 --- a/crates/worker/src/controller.rs +++ b/crates/worker/src/controller.rs @@ -222,6 +222,7 @@ impl WorkerController { pub async fn spawn( worker: Worker, runtime_base: &Path, + bash_output_dir: &Path, ) -> Result<(WorkerHandle, ShutdownReceiver), std::io::Error> where C: LlmClient + Clone + 'static, @@ -230,6 +231,7 @@ impl WorkerController { Self::spawn_inner( worker, runtime_base, + bash_output_dir, false, None, WorkerControllerTransport::UnixSocket, @@ -242,24 +244,9 @@ impl WorkerController { pub async fn spawn_with_transport( worker: Worker, runtime_base: &Path, + bash_output_dir: &Path, transport: WorkerControllerTransport, ) -> Result<(WorkerHandle, ShutdownReceiver), std::io::Error> - where - C: LlmClient + Clone + 'static, - St: Store + WorkerMetadataStore + Clone + Send + Sync + 'static, - { - Self::spawn_inner(worker, runtime_base, false, None, transport).await - } - - /// Spawn a Worker owned by `worker-runtime`. - /// - /// The controller still uses an ephemeral directory for Unix sockets and - /// tool spill artifacts, but does not write legacy pid/status/manifest - /// liveness projections. - pub async fn spawn_runtime_managed( - worker: Worker, - runtime_base: &Path, - ) -> Result<(WorkerHandle, ShutdownReceiver), std::io::Error> where C: LlmClient + Clone + 'static, St: Store + WorkerMetadataStore + Clone + Send + Sync + 'static, @@ -267,6 +254,33 @@ impl WorkerController { Self::spawn_inner( worker, runtime_base, + bash_output_dir, + false, + None, + transport, + ) + .await + } + + /// Spawn a Worker owned by `worker-runtime`. + /// + /// The controller uses an ephemeral directory for Unix sockets while tool + /// spill artifacts use the separately supplied Worker-owned temporary path. + /// Runtime-managed Workers do not write legacy pid/status/manifest liveness + /// projections. + pub async fn spawn_runtime_managed( + worker: Worker, + runtime_base: &Path, + bash_output_dir: &Path, + ) -> Result<(WorkerHandle, ShutdownReceiver), std::io::Error> + where + C: LlmClient + Clone + 'static, + St: Store + WorkerMetadataStore + Clone + Send + Sync + 'static, + { + Self::spawn_inner( + worker, + runtime_base, + bash_output_dir, true, None, WorkerControllerTransport::UnixSocket, @@ -278,6 +292,7 @@ impl WorkerController { pub async fn spawn_runtime_managed_run( worker: Worker, run_dir: &Path, + bash_output_dir: &Path, ) -> Result<(WorkerHandle, ShutdownReceiver), std::io::Error> where C: LlmClient + Clone + 'static, @@ -286,6 +301,7 @@ impl WorkerController { Self::spawn_runtime_managed_run_with_transport( worker, run_dir, + bash_output_dir, WorkerControllerTransport::UnixSocket, ) .await @@ -296,6 +312,7 @@ impl WorkerController { pub async fn spawn_runtime_managed_run_with_transport( worker: Worker, run_dir: &Path, + bash_output_dir: &Path, transport: WorkerControllerTransport, ) -> Result<(WorkerHandle, ShutdownReceiver), std::io::Error> where @@ -305,12 +322,21 @@ impl WorkerController { let parent = run_dir .parent() .ok_or_else(|| std::io::Error::other("run path has no parent"))?; - Self::spawn_inner(worker, parent, true, Some(run_dir), transport).await + Self::spawn_inner( + worker, + parent, + bash_output_dir, + true, + Some(run_dir), + transport, + ) + .await } async fn spawn_inner( worker: Worker, runtime_base: &Path, + bash_output_dir: &Path, runtime_managed: bool, runtime_run: Option<&Path>, transport: WorkerControllerTransport, @@ -323,6 +349,7 @@ impl WorkerController { let result = Self::spawn_initialized( worker, runtime_base, + bash_output_dir, runtime_managed, runtime_run, transport, @@ -340,6 +367,7 @@ impl WorkerController { async fn spawn_initialized( mut worker: Worker, runtime_base: &Path, + bash_output_dir: &Path, runtime_managed: bool, runtime_run: Option<&Path>, transport: WorkerControllerTransport, @@ -397,11 +425,11 @@ impl WorkerController { worker.attach_internal_worker_registry(spawned_registry.clone()); worker.attach_working_event_tx(working_event_tx.clone()); - // Bash spills long outputs to a per-worker subdir under the runtime - // dir. Push a recursive `allow(Read)` for that path into the - // Worker's runtime scope so the agent can `Read` saved files - // without polluting the workspace. - let bash_output_dir = runtime_dir.path().join("bash-output"); + // Bash spill artifacts are owned by the stable Worker identity rather + // than a controller session/run generation. Push a recursive + // `allow(Read)` for the exact tool output path into the Worker's shared + // runtime scope so the Workdir session and system prompt stay aligned. + let bash_output_dir = bash_output_dir.to_path_buf(); std::fs::create_dir_all(&bash_output_dir).map_err(|e| { std::io::Error::other(format!( "create bash output dir {}: {e}", @@ -880,7 +908,7 @@ where .register_tools(tools::core_builtin_tools( workdir.clone(), tracker.clone(), - bash_output_dir, + bash_output_dir.clone(), )); if feature_config.image.enabled && model_supports_image_attachments(&spawner_manifest.model) { @@ -1103,6 +1131,7 @@ where spawner_workspace_context, parent_notifications, runtime_base.clone(), + bash_output_dir.clone(), spawner_workspace_root, source_workdir_session, spawned_registry.clone(), diff --git a/crates/worker/src/entrypoint.rs b/crates/worker/src/entrypoint.rs index 2b258df6..562e7b67 100644 --- a/crates/worker/src/entrypoint.rs +++ b/crates/worker/src/entrypoint.rs @@ -634,10 +634,12 @@ async fn run_cli_inner(cli: Cli) -> ExitCode { return ExitCode::FAILURE; } }; + let bash_output_dir = crate::bash_output_dir_for_worker_id(&worker_name); let started = match start_worker_controller( worker, WorkerBootstrapLayout::Direct { runtime_base: runtime_base.clone(), + bash_output_dir, }, WorkerControllerTransport::UnixSocket, ) diff --git a/crates/worker/src/lib.rs b/crates/worker/src/lib.rs index 72212e87..166bdeb3 100644 --- a/crates/worker/src/lib.rs +++ b/crates/worker/src/lib.rs @@ -27,7 +27,7 @@ mod worker; pub use bootstrap::{ BootstrappedWorker, PreparedWorker, WorkerBootstrap, WorkerBootstrapError, - WorkerBootstrapLayout, start_worker_controller, + WorkerBootstrapLayout, bash_output_dir_for_worker_id, start_worker_controller, }; pub use compact::token_counter::{EstimateSource, SplitPoint, TokenEstimate}; pub use controller::{ShutdownReceiver, WorkerController, WorkerControllerTransport, WorkerHandle}; diff --git a/crates/worker/src/spawn/tool.rs b/crates/worker/src/spawn/tool.rs index 603ed190..8c3ee231 100644 --- a/crates/worker/src/spawn/tool.rs +++ b/crates/worker/src/spawn/tool.rs @@ -22,7 +22,7 @@ use manifest::{ use serde::Deserialize; use tokio::sync::mpsc; use workdir::{ - WorkdirDelegationPermission, WorkdirDelegationRequest, WorkdirDelegationRule, + WorkdirDelegationPermission, WorkdirDelegationRequest, WorkdirDelegationRule, WorkdirPath, WorkdirSessionHandle, }; @@ -258,9 +258,10 @@ pub struct SubWorkerSpawnTool { spawner_name: String, workspace_context: crate::worker::WorkerWorkspaceContext, parent_notifications: ParentNotificationTarget, - /// Runtime-owned root used only for bounded Internal Worker tool artifacts such as Bash spill - /// output. It is not an Internal Worker identity or catalog location. + /// Runtime-owned root used for Internal Worker controller state. runtime_base: PathBuf, + /// Parent Worker-owned temporary root used for bounded Bash spill output. + bash_output_dir: PathBuf, /// Inherited runtime workspace root for Profile/project/Ticket/workflow/ /// memory context. SubWorkerSpawn `cwd` must not affect this value. workspace_root: PathBuf, @@ -292,6 +293,7 @@ impl SubWorkerSpawnTool { workspace_context: crate::worker::WorkerWorkspaceContext, parent_notifications: ParentNotificationTarget, runtime_base: PathBuf, + bash_output_dir: PathBuf, workspace_root: PathBuf, source_workdir_session: Option, registry: Arc, @@ -304,6 +306,7 @@ impl SubWorkerSpawnTool { workspace_context, parent_notifications, runtime_base, + bash_output_dir, workspace_root, source_workdir_session, registry, @@ -367,7 +370,22 @@ impl Tool for SubWorkerSpawnTool { .reserve_internal_name(input.name.clone()) .map_err(|error| ToolError::InvalidArgument(error.to_string()))?; - let workdir_rules = parse_workdir_scope(&input.scope)?; + let mut workdir_rules = parse_workdir_scope(&input.scope)?; + let child_bash_output_dir = self.bash_output_dir.join("sub-workers").join(&input.name); + tokio::fs::create_dir_all(&child_bash_output_dir) + .await + .map_err(|error| { + ToolError::ExecutionFailed(format!( + "create Internal Worker Bash output directory {}: {error}", + child_bash_output_dir.display() + )) + })?; + workdir_rules.push(WorkdirDelegationRule { + target: WorkdirPath::new_scoped(child_bash_output_dir.to_string_lossy()) + .map_err(|error| ToolError::ExecutionFailed(error.to_string()))?, + permission: WorkdirDelegationPermission::Read, + recursive: true, + }); let source_workdir_session = require_active_workdir_session(self.source_workdir_session.as_ref())?; let delegation_request = workdir_delegation_request(input.cwd.as_deref(), workdir_rules)?; @@ -464,14 +482,22 @@ impl Tool for SubWorkerSpawnTool { .await .map_err(|error| ToolError::ExecutionFailed(format!("build Internal Worker: {error}")))?; child.bind_workdir_session(Some(workdir_delegation.scoped_session.clone())); + child + .add_scope_rules([ScopeRule { + target: child_bash_output_dir.clone(), + permission: manifest::Permission::Read, + recursive: true, + }]) + .map_err(|error| { + ToolError::ExecutionFailed(format!( + "grant Internal Worker Bash output scope: {error}" + )) + })?; let child_scope = child.scope().clone(); let child_registry = SpawnedWorkerRegistry::new_internal(input.name.clone(), child_scope); register_worker_tools( &mut child, - self.runtime_base - .join("internal-workers") - .join(&input.name) - .join("bash-output"), + child_bash_output_dir, self.runtime_base.clone(), child_registry.clone(), None, @@ -883,6 +909,7 @@ pub(crate) fn sub_worker_spawn_tool( workspace_context: crate::worker::WorkerWorkspaceContext, parent_notifications: ParentNotificationTarget, runtime_base: PathBuf, + bash_output_dir: PathBuf, workspace_root: PathBuf, source_workdir_session: Option, registry: Arc, @@ -894,6 +921,7 @@ pub(crate) fn sub_worker_spawn_tool( workspace_context, parent_notifications, runtime_base, + bash_output_dir, workspace_root, source_workdir_session, registry, @@ -907,6 +935,7 @@ fn sub_worker_spawn_tool_impl( workspace_context: crate::worker::WorkerWorkspaceContext, parent_notifications: ParentNotificationTarget, runtime_base: PathBuf, + bash_output_dir: PathBuf, workspace_root: PathBuf, source_workdir_session: Option, registry: Arc, @@ -938,6 +967,7 @@ fn sub_worker_spawn_tool_impl( workspace_context.clone(), parent_notifications.clone(), runtime_base.clone(), + bash_output_dir.clone(), workspace_root.clone(), source_workdir_session.clone(), registry.clone(), @@ -1082,12 +1112,17 @@ extract_threshold = 4000 async fn reviewer_profile_write_scope_exposes_command_tools_and_notifies_parent_controller() { let runtime = TempDir::new().unwrap(); let workspace_root = runtime.path().join("project"); + let bash_output_dir = runtime.path().join("bash-output"); let available_profiles = write_project_profile_registry( &workspace_root, Some("reviewer"), &[("reviewer", "reviewer.toml", INTERNAL_REVIEWER_PROFILE)], ); let mut manifest = parent_manifest(&workspace_root, None); + manifest + .scope + .allow + .push(abs_rule(&bash_output_dir, Permission::Read)); manifest.delegation_scope = ScopeConfig { allow: vec![abs_rule(&workspace_root, Permission::Write)], deny: Vec::new(), @@ -1118,6 +1153,7 @@ extract_threshold = 4000 workspace_context, ParentNotificationTarget::Controller(parent_method_tx.downgrade()), runtime.path().to_path_buf(), + bash_output_dir.clone(), workspace_root.clone(), Some(source_workdir_session), registry.clone(), @@ -1167,6 +1203,12 @@ extract_threshold = 4000 .await .expect("spawn project reviewer as Internal Worker"); assert!(output.summary.contains("internal worker `reviewer-child`")); + assert!( + bash_output_dir + .join("sub-workers") + .join("reviewer-child") + .is_dir() + ); assert!(spawner_scope.snapshot().is_writable(&workspace_root)); let record = registry .get_internal("reviewer-child") diff --git a/crates/worker/tests/compact_events_test.rs b/crates/worker/tests/compact_events_test.rs index 708fd60a..afef80bb 100644 --- a/crates/worker/tests/compact_events_test.rs +++ b/crates/worker/tests/compact_events_test.rs @@ -861,7 +861,8 @@ async fn controller_compact_method_emits_start_and_done() { ]); let worker = make_worker_with_manifest(POST_RUN_MANIFEST_TOML, client).await; let runtime_tmp = tempfile::tempdir().unwrap(); - let (handle, _shutdown) = WorkerController::spawn(worker, runtime_tmp.path()) + let bash_output_dir = runtime_tmp.path().join("bash-output"); + let (handle, _shutdown) = WorkerController::spawn(worker, runtime_tmp.path(), &bash_output_dir) .await .unwrap(); let mut rx = handle.subscribe(); diff --git a/crates/worker/tests/controller_test.rs b/crates/worker/tests/controller_test.rs index 968f8292..7b13e752 100644 --- a/crates/worker/tests/controller_test.rs +++ b/crates/worker/tests/controller_test.rs @@ -276,12 +276,38 @@ async fn spawn_controller(worker: Worker) -> WorkerHandle let tmp = tempfile::tempdir().unwrap(); let runtime_base = tmp.path().to_owned(); std::mem::forget(tmp); - let (handle, _shutdown_rx) = WorkerController::spawn(worker, &runtime_base) + let bash_output_dir = runtime_base.join("bash-output"); + let (handle, _shutdown_rx) = WorkerController::spawn(worker, &runtime_base, &bash_output_dir) .await .unwrap(); handle } +#[tokio::test] +async fn controller_grants_read_scope_for_exact_bash_output_directory() { + let worker = make_worker(MockClient::new(simple_text_events())).await; + let shared_scope = worker.scope().clone(); + let runtime_base = tempfile::tempdir().unwrap(); + let worker_tmp = tempfile::tempdir().unwrap(); + let bash_output_dir = worker_tmp.path().join("worker-1").join("bash-output"); + + let (handle, shutdown_rx) = + WorkerController::spawn(worker, runtime_base.path(), &bash_output_dir) + .await + .unwrap(); + + assert!(bash_output_dir.is_dir()); + assert!(shared_scope.snapshot().allow_rules().iter().any(|rule| { + rule.target == bash_output_dir + && rule.permission == manifest::Permission::Read + && rule.recursive + })); + assert!(!handle.runtime_dir.path().join("bash-output").exists()); + + handle.send(Method::Shutdown).await.unwrap(); + shutdown_rx.await.unwrap(); +} + #[tokio::test] async fn shutdown_closes_bound_workdir_session() { let (mut worker, pwd) = make_worker_with_pwd(MockClient::new(simple_text_events())).await; @@ -297,6 +323,7 @@ async fn shutdown_closes_bound_workdir_session() { command: "sleep 30".to_owned(), timeout_secs: 60, output_limit: 1024, + spill_dir: None, tool_call_id: None, }) .await @@ -304,9 +331,11 @@ async fn shutdown_closes_bound_workdir_session() { worker.bind_workdir_session(Some(Arc::clone(&session))); let runtime_base = tempfile::tempdir().unwrap(); - let (handle, shutdown_rx) = WorkerController::spawn(worker, runtime_base.path()) - .await - .unwrap(); + let bash_output_dir = runtime_base.path().join("bash-output"); + let (handle, shutdown_rx) = + WorkerController::spawn(worker, runtime_base.path(), &bash_output_dir) + .await + .unwrap(); handle.send(Method::Shutdown).await.unwrap(); tokio::time::timeout(std::time::Duration::from_secs(5), shutdown_rx) .await @@ -338,6 +367,7 @@ async fn controller_projects_workdir_command_events_and_snapshot_state() { command: "printf ready; sleep 0.3; printf done".to_owned(), timeout_secs: 5, output_limit: 1024, + spill_dir: None, tool_call_id: Some("tool-command-1".into()), }) .await @@ -445,6 +475,7 @@ async fn controller_refreshes_command_snapshot_after_high_output_provider_lag() .to_owned(), timeout_secs: 10, output_limit: 1024, + spill_dir: None, tool_call_id: Some("tool-high-output".into()), }) .await @@ -508,8 +539,9 @@ async fn controller_startup_failure_closes_bound_workdir_session() { let invalid_runtime_base = runtime_base.path().join("not-a-directory"); std::fs::write(&invalid_runtime_base, "file").unwrap(); + let bash_output_dir = runtime_base.path().join("bash-output"); assert!( - WorkerController::spawn(worker, &invalid_runtime_base) + WorkerController::spawn(worker, &invalid_runtime_base, &bash_output_dir) .await .is_err() ); @@ -519,6 +551,7 @@ async fn controller_startup_failure_closes_bound_workdir_session() { command: "printf unreachable".to_owned(), timeout_secs: 5, output_limit: 1024, + spill_dir: None, tool_call_id: None, }) .await, @@ -863,7 +896,8 @@ permission = "write" let client = MockClient::new(simple_text_events()); let worker = make_worker_with_pwd_and_manifest(client, manifest).await.0; let tmp = tempfile::tempdir().unwrap(); - let result = WorkerController::spawn(worker, tmp.path()).await; + let bash_output_dir = tmp.path().join("bash-output"); + let result = WorkerController::spawn(worker, tmp.path(), &bash_output_dir).await; assert!( result.is_ok(), "feature exposure must not imply delegation authority" diff --git a/crates/workspace-server/src/server.rs b/crates/workspace-server/src/server.rs index 615073d0..2ec12ea0 100644 --- a/crates/workspace-server/src/server.rs +++ b/crates/workspace-server/src/server.rs @@ -15430,6 +15430,7 @@ mod tests { command: "printf ready; sleep 30".to_string(), timeout_secs: 60, output_limit: 4096, + spill_dir: None, tool_call_id: Some("tool-call-command-session".to_string()), }) .await