feat(agent): surface storage transfer tools (#4577)

This commit is contained in:
Steven Enamakel
2026-07-06 09:30:04 -07:00
committed by GitHub
parent 057c3ffbf5
commit 3a7330884f
9 changed files with 295 additions and 29 deletions
@@ -46,5 +46,9 @@ named = [
"todowrite",
"plan_exit",
"web_fetch",
"storage_upload_file",
"storage_download_file",
"storage_list_files",
"storage_get_link",
"lsp",
]
@@ -14,14 +14,19 @@ omit_skills_catalog = true
hint = "burst"
[tools]
# Composio-only surface: this agent drives a single Composio toolkit per
# spawn. Workflow discovery/execution is the orchestrator's job now
# Integration surface: this agent drives a single Composio toolkit per
# spawn, with managed file-storage transfer tools for attachments/artifacts.
# Workflow discovery/execution is the orchestrator's job now
# (`list_workflows` / `run_workflow`), so the old `workflow_load` /
# `workflow_phase` tools (removed with the `agent_workflows` domain) are gone.
named = [
"composio_list_tools",
"file_read",
"composio_execute",
"storage_upload_file",
"storage_download_file",
"storage_list_files",
"storage_get_link",
# Inline-in-chat OAuth connect card (#3993). When the bound toolkit is not
# connected — or an action fails with a true auth/connection error — raise
# the connect card with this tool and await the result, instead of bubbling
+41 -3
View File
@@ -733,10 +733,18 @@ mod tests {
// orchestrator must not mutate files or run commands; all
// modification is deferred to `run_code` / owning
// specialists where edits live next to build/test/verify.
for forbidden in ["shell", "edit", "file_write", "apply_patch"] {
for forbidden in [
"shell",
"edit",
"file_write",
"apply_patch",
"curl",
"storage_set_visibility",
"storage_delete_file",
] {
assert!(
!tools.iter().any(|t| t == forbidden),
"orchestrator must NOT have write/exec tool `{forbidden}`"
"orchestrator must NOT have write/exec/lifecycle tool `{forbidden}`"
);
}
// Basic READ-ONLY direct surface: quick lookups without
@@ -748,6 +756,7 @@ mod tests {
"list",
"web_search_tool",
"web_fetch",
"http_request",
] {
assert!(
tools.iter().any(|t| t == direct),
@@ -815,6 +824,35 @@ mod tests {
);
}
#[test]
fn broad_agent_surfaces_expose_storage_transfer_not_lifecycle_tools() {
for id in ["code_executor", "integrations_agent", "orchestrator"] {
let def = find(id);
match &def.tools {
ToolScope::Named(tools) => {
for required in [
"storage_upload_file",
"storage_download_file",
"storage_list_files",
"storage_get_link",
] {
assert!(
tools.iter().any(|t| t == required),
"{id} must expose storage transfer tool `{required}`"
);
}
for forbidden in ["storage_set_visibility", "storage_delete_file"] {
assert!(
!tools.iter().any(|t| t == forbidden),
"{id} must not expose storage lifecycle tool `{forbidden}`"
);
}
}
ToolScope::Wildcard => panic!("{id} must have Named tool scope"),
}
}
}
#[test]
fn tool_maker_is_sandboxed_with_max_2_iterations() {
let def = find("tool_maker");
@@ -1478,7 +1516,7 @@ mod tests {
fn orchestrator_does_not_get_curl() {
// Per design: curl is a `Write` permission tool that writes
// to the workspace. The orchestrator delegates rather than
// executing — code_executor / researcher own actual downloads.
// executing — code_executor / tools_agent own actual downloads.
let def = find("orchestrator");
if let ToolScope::Named(tools) = &def.tools {
assert!(
@@ -216,12 +216,21 @@ named = [
"grep",
"glob",
"list",
# Quick web lookups. A single fact ("what's the capital of X", "latest
# version of Y") is one `web_search_tool` / `web_fetch` call — spawning
# the `research` worker for that is overkill. Multi-source crawls,
# comparisons, and doc digests still delegate to `research`.
# Quick web/API lookups. A single fact ("what's the capital of X", "latest
# version of Y") is one `web_search_tool` / `web_fetch` / `http_request`
# call — spawning the `research` worker for that is overkill. Multi-source
# crawls, comparisons, and doc digests still delegate to `research`.
"web_search_tool",
"web_fetch",
"http_request",
# Managed file-storage transfer surface for passing artifacts/attachments
# between chat, users, and workers. Storage lifecycle mutators
# (`storage_set_visibility`, `storage_delete_file`) stay out of the broad
# chat surface.
"storage_upload_file",
"storage_download_file",
"storage_list_files",
"storage_get_link",
# NOTE: `spawn_worker_thread` is intentionally absent — the tool is
# disabled pending a worker-thread UI (#1624) and is not registered, so
# listing it here (and teaching it in prompt.md) only trained the model
@@ -1,6 +1,6 @@
# Orchestrator - Staff Engineer
You are the **Orchestrator**, the senior agent in a multi-agent system. Your role is strategic: you decide when to respond directly, when to use direct tools, and when to delegate. You have a small **read-only** direct surface for lookups (`file_read`, `grep`, `glob`, `list`, `web_search_tool`, `web_fetch`). You **never** write or edit files and **never** execute shell commands — every file modification, however small, is delegated to `run_code` (or the owning specialist).
You are the **Orchestrator**, the senior agent in a multi-agent system. Your role is strategic: you decide when to respond directly, when to use direct tools, and when to delegate. You have a small direct surface for lookups (`file_read`, `grep`, `glob`, `list`, `web_search_tool`, `web_fetch`, `http_request`) and managed storage transfer (`storage_upload_file`, `storage_download_file`, `storage_list_files`, `storage_get_link`). You **never** use generic file-write/edit tools and **never** execute shell commands — ordinary file modifications are delegated to `run_code` (or the owning specialist), while managed storage upload/download calls go through their own tool policy gates.
## Core Responsibilities
@@ -24,8 +24,8 @@ Follow this sequence for every user message:
- If the relevant toolkit is **not** in **Connected Integrations**, call `composio_connect { toolkit: "<slug>" }` **directly** to raise an **inline connect card** so the user can authorize in one click, then continue the task once it returns `connected: true`. Do **not** refuse based on the Connected Integrations list (that is only what is *already* connected, not what is *connectable*), do **not** make "go to Settings → Connections" your first move, and do **not** silently fall back to memory retrieval (see "Connecting external services" below).
3. **Can I solve this with direct tools?**
- Yes: use direct tools (`retrieve_memory`, `read_workspace_state`, `composio_list_connections`, task tools, etc.).
- **Quick lookups are direct work.** A single web fact (a version number, a definition, one page's contents) is one `web_search_tool` or `web_fetch` call — do it yourself and answer. Reserve `research` for multi-source crawls, comparisons, or doc digests.
- **Read-only file lookups are direct work.** Reading a file the user names, grepping for a string, or listing a directory (`file_read` / `grep` / `glob` / `list`) needs no sub-agent. But you cannot write: the moment the task requires *changing* a file — even a one-line edit — delegate it to `run_code` (see below). Never promise an edit you cannot make yourself.
- **Quick lookups are direct work.** Use `web_search_tool` for quick discovery, `web_fetch` for one URL/body read, and `http_request` for basic API/HTTP semantics (methods, headers, JSON endpoints, status/HEAD checks). Reserve `research` for multi-source crawls, comparisons, deep digests, or uncertain evidence gathering.
- **Read-only file lookups are direct work.** Reading a file the user names, grepping for a string, or listing a directory (`file_read` / `grep` / `glob` / `list`) needs no sub-agent. Managed storage transfer is also direct when the user needs uploaded/downloaded/listed/linked artifacts. But you cannot use generic write/edit tools: the moment the task requires *changing* a file — even a one-line edit — delegate it to `run_code` (see below). Never promise an edit you cannot make yourself.
- No: continue.
4. **Does this need other specialised execution?**
- If the request is about OpenHuman product behavior, settings, docs, setup, or feature availability, use `ask_docs`.
@@ -38,8 +38,8 @@ Follow this sequence for every user message:
- **Do not stall after reading code-repo files.** If you (or a worker you spawned) have *read* files in a repo and have not yet *acted* on them — edited, built, tested, run, or pushed — and the user expects an outcome rather than a summary, that's the signal the task should have gone to `run_code` from the start. Re-issue the entire task as one `run_code` call with the full intent and let the code executor own the lifecycle. Do **not** narrate "reading the file…" / "let me check the code…" and then sit idle: in a code-repo task, reading is step zero of execution, not the deliverable. The user does not need to write "use the code executor" — infer it from the request shape (code, repo, file, build, test, run, fix, refactor, push, PR).
- If the request is to find, browse, install, or manage agent skills from community registries — or to follow a SKILL.md URL — use `setup_skills`.
- If the request is to run or execute an installed agent skill by name, use `run_skill`. The skill runs in an isolated worker, so its instructions never enter this conversation — you get back only its result. If that result contains a `## Handoff Plan` (steps the worker's narrow toolset couldn't perform — e.g. sending email, writing memory), carry out those steps yourself with your full tool set, routing each through the normal delegation path, then report the combined outcome. Treat handoff steps as *proposed* actions: never bypass the approval gate for them, especially for third-party skills.
- If multi-source web/doc crawling is required, use `research`. For a single live fact (weather, one price, one page) prefer your direct `web_search_tool` / `web_fetch` first.
- If the user asks for live/current/time-sensitive facts — weather, forecasts, current temperatures, recent news, fresh web facts, or "use Grok/web/live data" — get them now: one quick fact via direct `web_search_tool`, anything broader via `research` with a prompt that asks for live sources. Do **not** stop at "on it", and do **not** wait for the exact named provider if it is not wired in. Use the available tools and then answer with the result.
- If multi-source web/doc crawling is required, use `research`. For a single live fact (weather, one price, one page) prefer your direct `web_search_tool` / `web_fetch` / `http_request` first.
- If the user asks for live/current/time-sensitive facts — weather, forecasts, current temperatures, recent news, fresh web facts, or "use Grok/web/live data" — get them now: one quick fact via direct `web_search_tool` / `web_fetch` / `http_request`, anything broader via `research` with a prompt that asks for live sources. Do **not** stop at "on it", and do **not** wait for the exact named provider if it is not wired in. Use the available tools and then answer with the result.
- If complex multi-step decomposition is required, use `plan`.
- If code review is requested, use `review_code`.
- If memory archiving or distillation is required, use `archive_session`.
+123 -10
View File
@@ -22,6 +22,7 @@ use serde_json::{json, Value};
use crate::openhuman::config::Config;
use crate::openhuman::integrations::IntegrationClient;
use crate::openhuman::security::SecurityPolicy;
use crate::openhuman::tools::traits::{
PermissionLevel, Tool, ToolCallOptions, ToolCategory, ToolResult,
};
@@ -214,19 +215,46 @@ fn file_path(file_id: &str, suffix: &str) -> String {
format!("{FILES_PATH}/{file_id}{suffix}")
}
fn readonly_autonomy_block(security: &SecurityPolicy) -> Option<ToolResult> {
if security.can_act() {
None
} else {
Some(ToolResult::error(
"[policy-blocked] Action blocked: autonomy is read-only",
))
}
}
// ── StorageUploadFileTool ───────────────────────────────────────────
pub struct StorageUploadFileTool {
client: Arc<IntegrationClient>,
action_dir: PathBuf,
security: Arc<SecurityPolicy>,
}
impl StorageUploadFileTool {
pub fn new(client: Arc<IntegrationClient>, action_dir: PathBuf) -> Self {
Self { client, action_dir }
Self::new_with_security(client, action_dir, Arc::new(SecurityPolicy::default()))
}
pub fn new_with_security(
client: Arc<IntegrationClient>,
action_dir: PathBuf,
security: Arc<SecurityPolicy>,
) -> Self {
Self {
client,
action_dir,
security,
}
}
async fn run(&self, args: Value, action_dir: &Path) -> anyhow::Result<ToolResult> {
if let Some(blocked) = readonly_autonomy_block(&self.security) {
return Ok(blocked);
}
let raw_path = match args.get("path").and_then(|v| v.as_str()) {
Some(p) if !p.trim().is_empty() => p.trim(),
_ => return Ok(ToolResult::error("path is required")),
@@ -384,14 +412,31 @@ impl Tool for StorageUploadFileTool {
pub struct StorageDownloadFileTool {
client: Arc<IntegrationClient>,
action_dir: PathBuf,
security: Arc<SecurityPolicy>,
}
impl StorageDownloadFileTool {
pub fn new(client: Arc<IntegrationClient>, action_dir: PathBuf) -> Self {
Self { client, action_dir }
Self::new_with_security(client, action_dir, Arc::new(SecurityPolicy::default()))
}
pub fn new_with_security(
client: Arc<IntegrationClient>,
action_dir: PathBuf,
security: Arc<SecurityPolicy>,
) -> Self {
Self {
client,
action_dir,
security,
}
}
async fn run(&self, args: Value, action_dir: &Path) -> anyhow::Result<ToolResult> {
if let Some(blocked) = readonly_autonomy_block(&self.security) {
return Ok(blocked);
}
let file_id = match validate_file_id(&args) {
Ok(id) => id,
Err(e) => return Ok(ToolResult::error(e)),
@@ -487,6 +532,14 @@ impl Tool for StorageDownloadFileTool {
ToolCategory::Workflow
}
fn permission_level(&self) -> PermissionLevel {
PermissionLevel::Write
}
fn external_effect(&self) -> bool {
true
}
async fn execute(&self, args: Value) -> anyhow::Result<ToolResult> {
self.run(args, &self.action_dir).await
}
@@ -576,11 +629,19 @@ impl Tool for StorageListFilesTool {
pub struct StorageGetLinkTool {
client: Arc<IntegrationClient>,
security: Arc<SecurityPolicy>,
}
impl StorageGetLinkTool {
pub fn new(client: Arc<IntegrationClient>) -> Self {
Self { client }
Self::new_with_security(client, Arc::new(SecurityPolicy::default()))
}
pub fn new_with_security(
client: Arc<IntegrationClient>,
security: Arc<SecurityPolicy>,
) -> Self {
Self { client, security }
}
}
@@ -612,7 +673,19 @@ impl Tool for StorageGetLinkTool {
ToolCategory::Workflow
}
fn permission_level(&self) -> PermissionLevel {
PermissionLevel::Write
}
fn external_effect(&self) -> bool {
true
}
async fn execute(&self, args: Value) -> anyhow::Result<ToolResult> {
if let Some(blocked) = readonly_autonomy_block(&self.security) {
return Ok(blocked);
}
let file_id = match validate_file_id(&args) {
Ok(id) => id,
Err(e) => return Ok(ToolResult::error(e)),
@@ -653,11 +726,19 @@ impl Tool for StorageGetLinkTool {
pub struct StorageSetVisibilityTool {
client: Arc<IntegrationClient>,
security: Arc<SecurityPolicy>,
}
impl StorageSetVisibilityTool {
pub fn new(client: Arc<IntegrationClient>) -> Self {
Self { client }
Self::new_with_security(client, Arc::new(SecurityPolicy::default()))
}
pub fn new_with_security(
client: Arc<IntegrationClient>,
security: Arc<SecurityPolicy>,
) -> Self {
Self { client, security }
}
}
@@ -697,6 +778,10 @@ impl Tool for StorageSetVisibilityTool {
}
async fn execute(&self, args: Value) -> anyhow::Result<ToolResult> {
if let Some(blocked) = readonly_autonomy_block(&self.security) {
return Ok(blocked);
}
let file_id = match validate_file_id(&args) {
Ok(id) => id,
Err(e) => return Ok(ToolResult::error(e)),
@@ -735,11 +820,19 @@ impl Tool for StorageSetVisibilityTool {
pub struct StorageDeleteFileTool {
client: Arc<IntegrationClient>,
security: Arc<SecurityPolicy>,
}
impl StorageDeleteFileTool {
pub fn new(client: Arc<IntegrationClient>) -> Self {
Self { client }
Self::new_with_security(client, Arc::new(SecurityPolicy::default()))
}
pub fn new_with_security(
client: Arc<IntegrationClient>,
security: Arc<SecurityPolicy>,
) -> Self {
Self { client, security }
}
}
@@ -777,6 +870,10 @@ impl Tool for StorageDeleteFileTool {
}
async fn execute(&self, args: Value) -> anyhow::Result<ToolResult> {
if let Some(blocked) = readonly_autonomy_block(&self.security) {
return Ok(blocked);
}
let file_id = match validate_file_id(&args) {
Ok(id) => id,
Err(e) => return Ok(ToolResult::error(e)),
@@ -811,19 +908,35 @@ pub fn build_file_storage_tools(root_config: &Config, action_dir: &Path) -> Vec<
};
let action_dir = action_dir.to_path_buf();
let security = Arc::new(SecurityPolicy::from_config(
&root_config.autonomy,
&root_config.workspace_dir,
&root_config.action_dir,
));
let tools: Vec<Box<dyn Tool>> = vec![
Box::new(StorageUploadFileTool::new(
Box::new(StorageUploadFileTool::new_with_security(
Arc::clone(&client),
action_dir.clone(),
Arc::clone(&security),
)),
Box::new(StorageDownloadFileTool::new(
Box::new(StorageDownloadFileTool::new_with_security(
Arc::clone(&client),
action_dir,
Arc::clone(&security),
)),
Box::new(StorageListFilesTool::new(Arc::clone(&client))),
Box::new(StorageGetLinkTool::new(Arc::clone(&client))),
Box::new(StorageSetVisibilityTool::new(Arc::clone(&client))),
Box::new(StorageDeleteFileTool::new(Arc::clone(&client))),
Box::new(StorageGetLinkTool::new_with_security(
Arc::clone(&client),
Arc::clone(&security),
)),
Box::new(StorageSetVisibilityTool::new_with_security(
Arc::clone(&client),
Arc::clone(&security),
)),
Box::new(StorageDeleteFileTool::new_with_security(
Arc::clone(&client),
Arc::clone(&security),
)),
];
tracing::debug!(
"[file_storage] registered {} file-storage tools",
+64 -3
View File
@@ -8,6 +8,7 @@ use super::{
StorageGetLinkTool, StorageListFilesTool, StorageSetVisibilityTool, StorageUploadFileTool,
};
use crate::openhuman::integrations::IntegrationClient;
use crate::openhuman::security::{AutonomyLevel, SecurityPolicy};
use crate::openhuman::tools::traits::{PermissionLevel, Tool, ToolCategory};
fn dummy_client() -> Arc<IntegrationClient> {
@@ -40,9 +41,9 @@ fn upload_tool_schema_and_metadata() {
fn download_tool_schema_and_metadata() {
let tool = StorageDownloadFileTool::new(dummy_client(), PathBuf::from("/tmp"));
assert_eq!(tool.name(), "storage_download_file");
assert_eq!(tool.permission_level(), PermissionLevel::ReadOnly);
assert_eq!(tool.permission_level(), PermissionLevel::Write);
assert_eq!(tool.category(), ToolCategory::Workflow);
assert!(!tool.external_effect());
assert!(tool.external_effect());
let schema = tool.parameters_schema();
assert_eq!(schema["required"], json!(["file_id"]));
@@ -65,8 +66,9 @@ fn list_tool_metadata() {
fn get_link_tool_schema_and_metadata() {
let tool = StorageGetLinkTool::new(dummy_client());
assert_eq!(tool.name(), "storage_get_link");
assert_eq!(tool.permission_level(), PermissionLevel::ReadOnly);
assert_eq!(tool.permission_level(), PermissionLevel::Write);
assert_eq!(tool.category(), ToolCategory::Workflow);
assert!(tool.external_effect());
let schema = tool.parameters_schema();
assert_eq!(schema["required"], json!(["file_id"]));
@@ -252,6 +254,65 @@ fn client_for(server: &MockServer) -> Arc<IntegrationClient> {
Arc::new(IntegrationClient::new(server.uri(), "tok".to_string()))
}
fn readonly_security() -> Arc<SecurityPolicy> {
Arc::new(SecurityPolicy {
autonomy: AutonomyLevel::ReadOnly,
..SecurityPolicy::default()
})
}
#[tokio::test]
async fn storage_transfer_tools_block_readonly_autonomy_before_network() {
let tmp = tempfile::tempdir().unwrap();
std::fs::write(tmp.path().join("a.txt"), b"hi").unwrap();
let upload = StorageUploadFileTool::new_with_security(
dummy_client(),
tmp.path().to_path_buf(),
readonly_security(),
);
let upload_res = upload.execute(json!({ "path": "a.txt" })).await.unwrap();
assert!(upload_res.is_error);
assert!(upload_res.output().contains("[policy-blocked]"));
let download = StorageDownloadFileTool::new_with_security(
dummy_client(),
tmp.path().to_path_buf(),
readonly_security(),
);
let download_res = download
.execute(json!({ "file_id": "file_123" }))
.await
.unwrap();
assert!(download_res.is_error);
assert!(download_res.output().contains("[policy-blocked]"));
let get_link = StorageGetLinkTool::new_with_security(dummy_client(), readonly_security());
let link_res = get_link
.execute(json!({ "file_id": "file_123" }))
.await
.unwrap();
assert!(link_res.is_error);
assert!(link_res.output().contains("[policy-blocked]"));
let set_visibility =
StorageSetVisibilityTool::new_with_security(dummy_client(), readonly_security());
let visibility_res = set_visibility
.execute(json!({ "file_id": "file_123", "visibility": "public" }))
.await
.unwrap();
assert!(visibility_res.is_error);
assert!(visibility_res.output().contains("[policy-blocked]"));
let delete = StorageDeleteFileTool::new_with_security(dummy_client(), readonly_security());
let delete_res = delete
.execute(json!({ "file_id": "file_123" }))
.await
.unwrap();
assert!(delete_res.is_error);
assert!(delete_res.output().contains("[policy-blocked]"));
}
#[tokio::test]
async fn upload_tool_posts_multipart_and_reports_file_id() {
let server = MockServer::start().await;
@@ -1,7 +1,7 @@
use super::url_guard::{normalize_allowed_domains, validate_url_with_dns_check};
use crate::openhuman::config::HttpRequestConfig;
use crate::openhuman::security::SecurityPolicy;
use crate::openhuman::tools::traits::{Tool, ToolResult};
use crate::openhuman::security::{CommandClass, GateDecision, SecurityPolicy};
use crate::openhuman::tools::traits::{PermissionLevel, Tool, ToolResult};
use async_trait::async_trait;
use base64::engine::Engine as _;
use serde_json::json;
@@ -322,6 +322,17 @@ impl Tool for HttpRequestTool {
})
}
/// Rich HTTP semantics (methods, headers, request bodies, and x402 retry)
/// are the same Network-class risk as `curl`: read-only autonomy is blocked
/// in `execute`, and supervised/full tiers route through ApprovalGate.
fn external_effect_with_args(&self, _args: &serde_json::Value) -> bool {
self.security.gate_decision(CommandClass::Network) == GateDecision::Prompt
}
fn permission_level(&self) -> PermissionLevel {
PermissionLevel::Write
}
async fn execute(&self, args: serde_json::Value) -> anyhow::Result<ToolResult> {
let url = args
.get("url")
@@ -180,3 +180,28 @@ fn redirect_policy_is_none() {
let tool = test_tool(vec!["example.com"]);
assert_eq!(tool.name(), "http_request");
}
#[test]
fn supervised_http_request_is_external_effect_for_approval_gate() {
let tool = test_tool(vec!["example.com"]);
assert_eq!(tool.permission_level(), PermissionLevel::Write);
assert!(tool.external_effect_with_args(&json!({
"url": "https://example.com/api",
"method": "POST",
"headers": { "Authorization": "Bearer token" },
"body": "{}"
})));
}
#[test]
fn readonly_http_request_is_not_external_effect_because_execute_blocks() {
let security = Arc::new(SecurityPolicy {
autonomy: AutonomyLevel::ReadOnly,
..SecurityPolicy::default()
});
let tool = HttpRequestTool::new(security, vec!["example.com".into()], 1_000_000, 30);
assert!(!tool.external_effect_with_args(&json!({
"url": "https://example.com/api",
"method": "GET"
})));
}