fix(flows): make native oh: tool results usable and fail loudly, document the real file-attachment chain

src/openhuman/agent/harness/subagent_runner/ops/provider.rs

@@ -252,8 +252,9 @@ pub(crate) struct LazyToolkitResolver {
     /// for a slug, subsequent `resolve()` calls for the same slug reuse the
     /// cached instance — sharing its [`ContractGate`] state (#5119).
     #[allow(dead_code)] // used via pub(super) from tests
-    pub(super) resolved:
-        std::sync::Mutex<std::collections::HashMap<String, std::sync::Arc<dyn crate::openhuman::tools::Tool>>>,
+    pub(super) resolved: std::sync::Mutex<
+        std::collections::HashMap<String, std::sync::Arc<dyn crate::openhuman::tools::Tool>>,
+    >,
 }
 
 /// Minimum normalized-slug length before the prefix/superstring tier in
@@ -302,14 +303,13 @@ impl LazyToolkitResolver {
         }
 
         let action = self.find_action(name)?;
-        let tool: std::sync::Arc<dyn crate::openhuman::tools::Tool> = std::sync::Arc::new(
-            crate::openhuman::composio::ComposioActionTool::new(
+        let tool: std::sync::Arc<dyn crate::openhuman::tools::Tool> =
+            std::sync::Arc::new(crate::openhuman::composio::ComposioActionTool::new(
                 self.config.clone(),
                 action.name.clone(),
                 action.description.clone(),
                 action.parameters.clone(),
-            ),
-        );
+            ));
 
         // Store in cache for future lookups.
         {

src/openhuman/approval/redact.rs

@@ -69,6 +69,21 @@ const SENSITIVE_KEYS: &[&str] = &[
     "authorization",
     "auth",
     "code",
+    // File-handoff params. A presigned storage link (`storage_get_link`) is a
+    // BEARER CAPABILITY: anyone holding the URL can fetch the file until it
+    // expires. These land in `tool_call` args whenever a flow hands a produced
+    // file to an externally-executed action (a Composio `file_uploadable`
+    // param such as Gmail's `attachment` or Jira's `file_to_upload`). Redacted
+    // args are both rendered on the approval card and persisted with the
+    // approval record, so leaving these clear would leak the capability into
+    // the UI and durable storage.
+    "attachment",
+    "attachments",
+    "file_to_upload",
+    "file_url",
+    "public_url",
+    "signed_url",
+    "presigned_url",
 ];
 
 /// Produce a redacted clone of `args` suitable for persistence /
@@ -485,6 +500,41 @@ mod tests {
         assert_eq!(summary.matches("<HOME>").count(), 2);
     }
 
+    #[test]
+    fn file_handoff_links_are_redacted() {
+        // A presigned storage link is a bearer capability: anyone holding the
+        // URL can fetch the file until it expires. Redacted args are shown on
+        // the approval card AND persisted, so these must never appear clear.
+        let args = json!({
+            "attachment": "https://files.example.test/f_1?sig=SECRETSIGNATURE",
+            "file_to_upload": "https://files.example.test/f_2?sig=ANOTHERSIG",
+            "file_url": "https://files.example.test/f_3?sig=THIRDSIG",
+            "public_url": "https://files.example.test/f_4?sig=FOURTHSIG",
+            // A bare `url` (e.g. an `http_request` node's destination, or a
+            // Composio action's benign url arg) is NOT a file-handoff key and
+            // MUST stay visible so the human approver can judge the action.
+            "url": "https://webhook.site/VISIBLE-DESTINATION",
+        });
+        let red = redact_args(&args);
+        let blob = red.to_string();
+        for leaked in [
+            "SECRETSIGNATURE",
+            "ANOTHERSIG",
+            "THIRDSIG",
+            "FOURTHSIG",
+            "files.example.test",
+        ] {
+            assert!(
+                !blob.contains(leaked),
+                "presigned link leaked through redaction ({leaked}): {blob}"
+            );
+        }
+        assert!(
+            blob.contains("webhook.site/VISIBLE-DESTINATION"),
+            "a bare `url` (e.g. an http_request destination) must NOT be redacted: {blob}"
+        );
+    }
+
     #[test]
     fn summarize_action_pulls_safe_fields() {
         let args = json!({

src/openhuman/flows/agents/workflow_builder/prompt.md

@@ -551,6 +551,65 @@ And without `input_context`, don't reach for a jq expression woven into
 — that's prose, not jq, resolves to `null`, and both the `save_workflow` gate
 and `dry_run_workflow`'s `agent_prompt_nulls` will reject it.
 
+### Attaching a produced file to an external action
+
+When the user wants a file **attached** to something you send or create through
+a connected integration, the file must be handed over as a **URL the provider's
+servers can fetch**. Composio actions execute on Composio's backend, so a local filesystem
+path can never work: it fails at run time with `Error reading file at
+/Users/... ENOENT`. Putting the content in the message body does **not**
+satisfy an attachment request either.
+
+The working chain is four nodes. **Keep the file handle out of any agent's
+structured output.** Give the producer a fixed path you choose, then refer to
+that same literal path from a separate upload node. An agent that has to emit a
+`file_id` or `file_path` through `output_parser` is a schema mismatch waiting to
+fail the run; the file only needs to exist on disk, so the agent's *side effect*
+is what matters, not its output.
+
+1. **Produce the file at a workspace-relative path YOU chose.** An `agent` node
+   (usually `agent_ref: "code_executor"`) or a `code` node writes it. State the
+   path in the prompt, e.g. "write the page to `report.html`". Use a path
+   **relative to the working directory**, never an absolute path like
+   `/tmp/...`: writes and uploads are confined to the agent workspace, and an
+   absolute path outside it is rejected. Do **not** give this node an
+   `output_parser` schema for the file, and do **not** bind anything off it.
+2. **Upload it.** A `tool_call` on **`oh:storage_upload_file`** with that same
+   path as a **literal** string: `{ "path": "report.html" }`. Because this is a
+   real node, its `file_id` is a node output rather than model-authored JSON:
+   bind `=nodes.<upload>.item.json.file_id`.
+3. **Mint a link that outlives approval.** A `tool_call` on
+   **`oh:storage_get_link`** with
+   `{ "file_id": "=nodes.<upload>.item.json.file_id", "expires_in_seconds": 900 }`
+   returns `{ url, expires_at }`. Bind `=nodes.<link>.item.json.url`.
+   The send is an outbound action, so it may be parked for human approval for up
+   to ~10 minutes before it fires; the link's TTL must comfortably outlive that
+   window or the provider will fetch a dead URL. The URL is a bearer capability
+   for as long as it lives, so do not set it far longer than needed either.
+   **Do not** upload with `visibility: "public"` to get a `public_url` instead.
+   That leaves a permanently world-readable object; the presigned link expires.
+4. **Send it.** A `tool_call` on the provider action, binding the link URL into
+   that action's file parameter.
+
+**Find the file parameter by its marker, never by guessing a name.** Call
+`get_tool_contract` on the send action and look in `input_schema.properties`
+for the property carrying **`"file_uploadable": true`** (it also shows
+`"format": "path"`). That marker is the contract across toolkits; the property
+name is not, so read it off the contract every time rather than assuming a
+convention (one toolkit's is `attachment`, another's is `file_to_upload`).
+
+Everything else about that parameter also comes from the contract: whether it
+accepts one value or a list, and any size or type limits the provider enforces,
+are stated in its schema and `description`. Read them there. Do not rely on
+remembered provider limits.
+
+**Do not invent a dedicated attachment action.** Send actions generally take
+the file on the ordinary send, so search for an attachment-capable send before
+assuming a separate one exists. If `get_tool_contract` reports a slug is not a
+real action, that is a hard stop: go back to `search_tool_catalog` and pick a
+real one rather than wiring it anyway. A slug that merely looks plausible by
+naming convention is the single most expensive mistake you can make here.
+
 ### Trigger kinds — which ones actually fire
 
 Set `config.trigger_kind` on the trigger node. **Only three fire automatically

src/openhuman/flows/builder_tools.rs

@@ -2587,32 +2587,52 @@ impl Tool for GetNodeKindContractTool {
 ///
 /// The common case reports `{ node_id, location, expression }` — a wiring
 /// mistake the agent should fix. But when the null-resolved expression binds to
-/// the output of an upstream Composio `tool_call` node
-/// ([`ops::composio_tool_call_upstream_ref`]), the entry is instead marked
+/// the output of an upstream Composio-or-native `tool_call` node
+/// ([`ops::mock_opaque_tool_call_upstream_ref`]), the entry is instead marked
 /// `unverifiable: true` and carries an honest `suggestion`: the echo sandbox
-/// can NEVER produce a Composio tool's real output fields, so this particular
-/// null is expected here and does NOT prove the binding wrong (WS6 — the
-/// transcript audit where the agent re-wired an already-correct binding three
-/// times chasing this exact false negative). The message points at
-/// `get_tool_contract` / `get_tool_output_sample` as the real disambiguators.
+/// can NEVER produce a tool's real output fields, so this particular null is
+/// expected here and does NOT prove the binding wrong (WS6 — the transcript
+/// audit where the agent re-wired an already-correct binding three times
+/// chasing this exact false negative). The suggestion adapts to the upstream
+/// kind: a Composio upstream points at `get_tool_contract` /
+/// `get_tool_output_sample` and the `.item.json.data.` nesting; a native `oh:`
+/// upstream points at the flat `.item.json.<field>` shape instead.
 fn build_null_resolution_entry(
     node_id: &str,
     diag: &tinyflows::expr::NullResolution,
     graph: &WorkflowGraph,
 ) -> Value {
-    if let Some(upstream) = crate::openhuman::flows::ops::composio_tool_call_upstream_ref(
+    if let Some(upstream) = crate::openhuman::flows::ops::mock_opaque_tool_call_upstream_ref(
         &diag.expression,
         graph,
         node_id,
     ) {
         let field = diag.location.strip_prefix("args.").unwrap_or("args");
-        return json!({
-            "node_id": node_id,
-            "location": diag.location,
-            "expression": diag.expression,
-            "unverifiable": true,
-            "upstream_tool_call": upstream,
-            "suggestion": format!(
+        // The disambiguation advice differs by upstream kind: a native `oh:`
+        // tool's output binds FLAT (`.item.json.<field>`) after
+        // `native_tool_payload`'s unwrap — it has no `.data.` wrapper and no
+        // Composio `get_tool_contract` — whereas a Composio action nests under
+        // `.item.json.data.`. Emitting the Composio advice for a native
+        // upstream would send the agent chasing a `.data.` path that will
+        // never exist.
+        let upstream_is_native = graph
+            .nodes
+            .iter()
+            .find(|n| n.id == upstream)
+            .and_then(|n| n.config.get("slug").and_then(Value::as_str))
+            .is_some_and(|s| s.starts_with("oh:"));
+        let suggestion = if upstream_is_native {
+            format!(
+                "required arg `{field}` binds to the output of native tool_call node \
+                 `{upstream}` — the SANDBOX only echoes tool calls and can never produce \
+                 their real output fields, so this binding is UNVERIFIABLE here (not \
+                 necessarily wrong). A native `oh:` tool's real output binds FLAT at \
+                 `=nodes.{upstream}.item.json.<field>` (no `.data.` wrapper). Confirm the \
+                 field name against that tool's own output shape. It is a real bug only if \
+                 the path doesn't match the tool's actual output."
+            )
+        } else {
+            format!(
                 "required arg `{field}` binds to the output of Composio tool_call node \
                  `{upstream}` — the SANDBOX only echoes tool calls and can never produce \
                  their real output fields, so this binding is UNVERIFIABLE here (not \
@@ -2621,7 +2641,15 @@ fn build_null_resolution_entry(
                  `.item.json.data.`), or get_tool_output_sample {{ slug, args }} for the \
                  real shape. It is a real bug only if the path doesn't match the action's \
                  actual output."
-            ),
+            )
+        };
+        return json!({
+            "node_id": node_id,
+            "location": diag.location,
+            "expression": diag.expression,
+            "unverifiable": true,
+            "upstream_tool_call": upstream,
+            "suggestion": suggestion,
         });
     }
     json!({
@@ -2634,7 +2662,7 @@ fn build_null_resolution_entry(
 /// Every null-resolved `args.*` config expression that landed on a `tool_call`
 /// node, as `null_resolutions` diagnostic entries (see
 /// [`build_null_resolution_entry`] for the shape, including the WS6
-/// `unverifiable` Composio-upstream variant). Shared by the settled-run path
+/// `unverifiable` Composio-or-native-upstream variant). Shared by the settled-run path
 /// (which fails the dry run on these) and the errored-run path (which surfaces
 /// only the `unverifiable` ones so a stop-policy preflight abort explains
 /// itself honestly instead of via the generic required-arg text).