feat: spill long bash output to worker temp storage
This commit is contained in:
+134
-6
@@ -21,6 +21,7 @@ struct BashParams {
|
||||
|
||||
pub(crate) struct BashTool {
|
||||
session: WorkdirSessionHandle,
|
||||
output_dir: PathBuf,
|
||||
state: Arc<Mutex<BashExecutionState>>,
|
||||
}
|
||||
|
||||
@@ -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<dyn Tool> = 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);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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<String>,
|
||||
#[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() {
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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}"
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -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]
|
||||
|
||||
Reference in New Issue
Block a user