feat(prompt): unified delegation guide + dynamic per-toolkit tool registration (#447)#570
Conversation
…istration (tinyhumansai#447) Cuts main agent prompt token cost ~98% on accounts with connected integrations and replaces the generic Composio dispatcher with native per-action tool calling inside skills_agent. ## Why Issue tinyhumansai#447: prompt payloads were ballooning because main/orchestrator re-shipped a full prose enumeration of every connected toolkit's actions on every turn (~13.7k tokens for one gmail integration alone), and skills_agent had no way to scope tools to a single toolkit so it inherited a flat dispatcher (composio_execute) that the LLM couldn't reliably call without first round-tripping composio_list_tools. ## What ### main / orchestrator prompt - Replace `ConnectedIntegrationsSection`'s per-action prose dump with a unified Delegation Guide: one bullet per integration (toolkit + one-line description + spawn snippet). Tokens added per integration: ~30. Savings vs old per-action listing: ~5k+ tokens per connected toolkit. - Drop every "Composio" reference from delegating-agent prose so the surface stays provider-neutral. ### skills_agent - Add a `toolkit` argument to `spawn_subagent` with three pre-flight validation modes resolved against the parent's connected integrations list before any LLM call: * missing toolkit -> ToolResult::error (model retries) * unknown toolkit -> ToolResult::error (model retries) * in allowlist, not connected -> ToolResult::success (model surfaces "authorize in Settings -> Integrations" to user; not flagged as a tool failure in the agent loop or web channel) - New `ComposioActionTool` (`src/openhuman/composio/action_tool.rs`) implements the `Tool` trait per Composio action. The sub-agent runner constructs ~N of these at spawn time from the cached integration overview and injects them via a new `extra_tools` parameter through `run_typed_mode` -> `run_inner_loop`. - `filter_tool_indices` drops every skill-category parent tool when `is_skills_agent_with_toolkit` is true so the only skill-category entries the sub-agent sees are the freshly-built per-action tools. This eliminates apify_*, composio_list_*, composio_authorize, and composio_execute from the toolkit-scoped surface. - Sub-agent renderer takes the parent's actual `tool_call_format` instead of hardcoding PFormat. Native dispatchers no longer carry a prose `## Tools` section at all (schemas already travel through the request body's `tools` field) — eliminates a ~30k-token duplication that was blowing past the model's context window. ### integration overview fetch - `fetch_connected_integrations_uncached` now merges Composio's toolkit allowlist with the user's active connections and returns one `ConnectedIntegration` per allowlisted toolkit with a `connected: bool` flag. Unconnected entries carry no schemas, just the toolkit name + description, so the orchestrator can mention them without trying to invoke them. - `ConnectedIntegrationTool` preserves the action's full JSON parameter schema so `ComposioActionTool` can advertise it through native function-calling. ### plumbing - `ParentExecutionContext` carries a `ComposioClient` and the parent's resolved `tool_call_format`, populated alongside `connected_integrations` in `Session::fetch_connected_integrations`. Triage and test paths pass `None` / `PFormat` defaults. - `dispatch_subagent` (the `SkillDelegationTool` path) plumbs its pre-bound skill_id through `toolkit_override` instead of the broken `skill_filter_override` (which used `{skill}__` prefix matching that never matched Composio's `TOOLKIT_*` naming). - `web::run_chat_task` failures now log the underlying error at WARN so debugging an in-flight failure no longer requires turning on TRACE for socket events. - `scripts/stage-core-sidecar.mjs` queries `cargo metadata` for the real target directory instead of assuming `<repo>/target` so the staging step works under workspace-level `target-dir` overrides (the vezures-workspace shared `.cargo-target` setup). ## Verified end-to-end Two RPC tests against a freshly-rebuilt sidecar (gmail connected, notion / slack / etc. allowlisted but not connected): | Test | Behavior | Tokens | Iterations | |---|---|---|---| | Connected (gmail, "fetch 5 unread emails") | spawn_subagent -> 62 dynamic gmail tools registered -> 1 GMAIL_FETCH_EMAILS call with smart args -> markdown table response | 42,911 input | 1 | | Not-connected (notion, "create a page") | main answers directly without spawning, tells user to authorize in Settings -> Integrations | 3,319 input | 1 | Both flows complete cleanly. The connected path proves dynamic per-action tool registration is working with full schema validation; the unconnected path proves the unified delegation guide + ToolResult::success return for not-connected toolkits keeps the model on a graceful path without polluting the chat with error styling. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThreaded a Composio client and tool-call format through Agent session and parent execution contexts, added dynamic per-action Composio tools and toolkit-scoped subagent wiring, and updated integration discovery and prompt rendering to reflect connected vs allowlisted toolkits. Changes
Sequence Diagram(s)sequenceDiagram
participant Agent as Agent(Session)
participant SubRunner as SubagentRunner
participant ToolDisp as ToolDispatcher
participant DynTool as ComposioActionTool
participant Composio as ComposioClient
participant EventBus as EventBus
Agent->>Agent: build_parent_execution_context(composio_client, tool_call_format)
Agent->>SubRunner: run_subagent(options{toolkit_override})
SubRunner->>SubRunner: run_typed_mode()
alt toolkit_override set & definition.id == "skills_agent"
SubRunner->>Agent: read parent.connected_integrations & composio_client
SubRunner->>DynTool: construct per-action ComposioActionTool(s)
SubRunner->>SubRunner: filter parent skill tools, append dynamic tools
end
SubRunner->>SubRunner: run_inner_loop(extra_tools)
loop tool call
SubRunner->>SubRunner: resolve tool name (extra_tools -> parent tools)
alt found in extra_tools
SubRunner->>DynTool: execute(args)
DynTool->>Composio: execute_tool(action_name, args)
Composio-->>DynTool: result
DynTool->>EventBus: publish ComposioActionExecuted(success, cost, error)
DynTool-->>SubRunner: ToolResult(success/json)
else found in parent tools
SubRunner->>ToolDisp: execute parent tool
else
SubRunner-->>SubRunner: unknown tool error
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/openhuman/context/prompt.rs (1)
907-927:⚠️ Potential issue | 🔴 CriticalAll test call sites for the modified renderer functions must be updated to match the new signatures.
The function signature now requires
extra_toolsandtool_call_formatparameters, plusconnected_integrations. Multiple tests throughout the file (lines 1598–2145) still call the old arity with arguments in the wrong order. For example, at line 1598–1606, the call passesarchetype_bodybeforeextra_tools, but the signature expectsextra_toolsbeforearchetype_body. All test calls are missing thetool_call_formatandconnected_integrationsarguments. The module will not compile until these call sites are updated.Test locations requiring updates
render_subagent_system_prompt: Lines 1598–1606, 1727–1741, 1818–1832, 1856–1864, 1907–1915, 1951–1959, 1989–2003, 2033–2041, 2071–2085, 2128–2145render_subagent_system_prompt_with_format: Lines 1662–1677, 1686–1695🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/context/prompt.rs` around lines 907 - 927, Update all test call sites to match the new signatures: ensure render_subagent_system_prompt takes parameters in this order (workspace_dir, model_name, allowed_indices, parent_tools, extra_tools, archetype_body, options, tool_call_format, connected_integrations) so move extra_tools to before archetype_body and add the missing tool_call_format and connected_integrations arguments at the end; likewise update render_subagent_system_prompt_with_format call sites to include the new extra_tools, tool_call_format and connected_integrations parameters. Search for the listed test locations (calls to render_subagent_system_prompt and render_subagent_system_prompt_with_format) and replace the old-arity calls with the new ordered argument list so the types align with the symbols render_subagent_system_prompt and render_subagent_system_prompt_with_format.
🧹 Nitpick comments (2)
src/openhuman/tools/impl/agent/spawn_subagent.rs (1)
57-62: Please document the newskills_agent(toolkit=...)contract outside this schema.This PR changes
spawn_subagent's caller contract in a user-visible way, but the requirement currently only exists in the inline tool description/schema. Please add the matching AGENTS/feature-doc update so prompt authors and non-LLM callers know thattoolkitis now required forskills_agent, and that an unconnected toolkit should surface an authorization prompt instead of retrying the spawn.As per coding guidelines, "Add concise rustdoc / code comments where the flow is not obvious; update
AGENTS.md, architecture docs, or feature docs when repository rules or user-visible behavior change".Also applies to: 109-112
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/tools/impl/agent/spawn_subagent.rs` around lines 57 - 62, The change to spawn_subagent alters the caller contract for skills_agent by requiring toolkit="..." and must be documented and surfaced beyond the inline tool schema: update AGENTS.md (or the agents/feature docs) to state that skills_agent requires a toolkit parameter naming the Composio integration (e.g., "gmail", "notion"), and that if the requested toolkit is not connected the runtime should surface an authorization prompt rather than silently retrying; add concise rustdoc/comments in spawn_subagent.rs (around the spawn_subagent function and the related handling code referenced near the other occurrence at lines ~109-112) describing the required toolkit param and the expected auth-prompt behavior so prompt authors and non-LLM callers can follow the contract.src/openhuman/composio/action_tool.rs (1)
84-119: Add structured debug logs around the Composio execution path.This new external-call hot path emits events, but it never logs entry/success/failure with the action name and elapsed time. Please add a stable-prefix debug log here so runtime failures can be traced even when no event subscriber is listening.
As per coding guidelines, "Add substantial, development-oriented logs on new/changed flows; include logs at entry/exit points, branch decisions, external calls, retries/timeouts, state transitions, and error handling paths".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/composio/action_tool.rs` around lines 84 - 119, Add stable-prefix debug logs around the Composio execution path in the async fn execute: log an entry debug right before calling self.client.execute_tool including the action name (self.action_name) and any input args, log a success debug in the Ok(resp) branch that includes the action name, elapsed_ms and resp (or resp.summary), and log a failure debug in the Err(e) branch that includes the action name, elapsed_ms and full error details; place logs near the started/elapsed_ms computation and before/after the publish_global DomainEvent::ComposioActionExecuted calls so entry/exit and branch decisions around self.client.execute_tool are clearly recorded with a stable prefix.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/stage-core-sidecar.mjs`:
- Around line 36-37: The CARGO_TARGET_DIR value is being resolved relative to
the current cwd which can differ from the repo root (root) used when running
cargo; update the branch that returns resolve(process.env.CARGO_TARGET_DIR) so
it resolves process.env.CARGO_TARGET_DIR relative to the repo root variable (use
resolve(root, process.env.CARGO_TARGET_DIR) or equivalent) so the path is stable
for the later binary lookup and cargo spawn that uses root and resolve.
In `@src/openhuman/agent/harness/session/turn.rs`:
- Around line 1032-1035: PromptContext.agent_id is using the wrong session field
(self.agent_definition_name) so built agents never get their real definition id;
update the session state to carry the resolved agent id and wire that into
PromptContext. Concretely, after resolving the agent id in
build_session_agent_inner() (or in AgentBuilder::build()), assign the resolved
id into the session field used for prompts (set the session's
agent_definition_name or add a session.agent_id field) and then construct
PromptContext with that resolved value (use the resolved agent id instead of
&self.agent_definition_name) so PromptContext.agent_id reflects the actual agent
definition.
In `@src/openhuman/agent/triage/escalation.rs`:
- Around line 160-169: The ParentExecutionContext snapshot currently drops the
real runtime values (setting composio_client: None and tool_call_format:
ToolCallFormat::PFormat); update the construction in dispatch_target_agent() to
copy the live values from the parent Agent/session instead of hard-coded
fallbacks—specifically populate composio_client with the parent Agent's composio
client instance and set tool_call_format from the parent runtime/dispatcher
setting used by the normal session path (see how ParentExecutionContext is
assembled in src/openhuman/agent/harness/session/turn.rs for the exact fields to
mirror), ensuring triage-spawned sub-agents keep composio access and the correct
tool-call dialect.
In `@src/openhuman/channels/providers/web.rs`:
- Around line 201-207: The current log in run_chat_task uses log::warn! and
interpolates the full err which can leak sensitive upstream details; change this
to log a stable error category or code instead of the raw err (e.g., map the
error to an ErrorKind / error_code or call a sanitizer function) and only
include redacted or non-sensitive detail if necessary; keep correlation fields
client_id_task, thread_id_task, request_id_task but replace err with something
like error_kind or redacted_error (derived from the error via a
sanitize_error/into_error_code helper) so logs contain a stable category rather
than raw provider/request text.
In `@src/openhuman/composio/ops.rs`:
- Around line 545-548: The current prefix check
t.function.name.starts_with(&slug.to_uppercase()) can false-match (e.g., "git" →
"GITHUB_*"); change the filter in ops.rs to require a delimiter after the slug,
e.g. starts_with(&format!("{}_", slug.to_uppercase())), or even better use
explicit toolkit metadata returned by the backend (e.g., a toolkit
slug/identifier field on the action object) to match actions to the toolkit
instead of string prefix matching; update the closure that references
t.function.name and slug to perform the new check or to read the toolkit
identifier from the action/metadata.
In `@src/openhuman/context/debug_dump.rs`:
- Around line 577-592: The debug dump currently forces ToolCallFormat::PFormat
and an empty no_extra_tools when calling render_subagent_system_prompt, which
breaks fidelity with real agent spawns; modify the dump path to accept and pass
through the actual tool_call_format and the runtime extra_tools (or a
toolkit-scoped dump option) instead of using the hard-coded
ToolCallFormat::PFormat and no_extra_tools: locate the
render_subagent_system_prompt call in debug_dump.rs and replace the fixed
arguments with the real format variable (e.g., tool_call_format or the caller’s
format) and the appropriate extra_tools vector (or add a new toolkit-scoped flag
that constructs the scoped extra_tools) so dump-prompt output matches
native/JSON and dynamic toolkit behaviors.
In `@src/openhuman/context/prompt.rs`:
- Around line 1000-1018: The current conditional only renders the `## Tools`
prose when ToolCallFormat::PFormat, which wrongly omits the tool catalog for the
`ToolCallFormat::Json` path; change the condition in prompt rendering (in
src/openhuman/context/prompt.rs around the block referencing
ToolCallFormat::PFormat) so that it includes Json as a prose-driven path (e.g.,
treat Json the same as PFormat) or invert the logic to skip the prose only for
ToolCallFormat::Native (use ToolCallFormat::Native as the sole case that relies
on request-body schemas), ensuring `ToolCallFormat::Json` and
`ToolCallFormat::PFormat` both produce the `## Tools` listing.
---
Outside diff comments:
In `@src/openhuman/context/prompt.rs`:
- Around line 907-927: Update all test call sites to match the new signatures:
ensure render_subagent_system_prompt takes parameters in this order
(workspace_dir, model_name, allowed_indices, parent_tools, extra_tools,
archetype_body, options, tool_call_format, connected_integrations) so move
extra_tools to before archetype_body and add the missing tool_call_format and
connected_integrations arguments at the end; likewise update
render_subagent_system_prompt_with_format call sites to include the new
extra_tools, tool_call_format and connected_integrations parameters. Search for
the listed test locations (calls to render_subagent_system_prompt and
render_subagent_system_prompt_with_format) and replace the old-arity calls with
the new ordered argument list so the types align with the symbols
render_subagent_system_prompt and render_subagent_system_prompt_with_format.
---
Nitpick comments:
In `@src/openhuman/composio/action_tool.rs`:
- Around line 84-119: Add stable-prefix debug logs around the Composio execution
path in the async fn execute: log an entry debug right before calling
self.client.execute_tool including the action name (self.action_name) and any
input args, log a success debug in the Ok(resp) branch that includes the action
name, elapsed_ms and resp (or resp.summary), and log a failure debug in the
Err(e) branch that includes the action name, elapsed_ms and full error details;
place logs near the started/elapsed_ms computation and before/after the
publish_global DomainEvent::ComposioActionExecuted calls so entry/exit and
branch decisions around self.client.execute_tool are clearly recorded with a
stable prefix.
In `@src/openhuman/tools/impl/agent/spawn_subagent.rs`:
- Around line 57-62: The change to spawn_subagent alters the caller contract for
skills_agent by requiring toolkit="..." and must be documented and surfaced
beyond the inline tool schema: update AGENTS.md (or the agents/feature docs) to
state that skills_agent requires a toolkit parameter naming the Composio
integration (e.g., "gmail", "notion"), and that if the requested toolkit is not
connected the runtime should surface an authorization prompt rather than
silently retrying; add concise rustdoc/comments in spawn_subagent.rs (around the
spawn_subagent function and the related handling code referenced near the other
occurrence at lines ~109-112) describing the required toolkit param and the
expected auth-prompt behavior so prompt authors and non-LLM callers can follow
the contract.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bf1c6180-d465-4bd3-94a8-29743ac5ea39
📒 Files selected for processing (16)
scripts/stage-core-sidecar.mjssrc/openhuman/agent/harness/fork_context.rssrc/openhuman/agent/harness/session/builder.rssrc/openhuman/agent/harness/session/turn.rssrc/openhuman/agent/harness/session/types.rssrc/openhuman/agent/harness/subagent_runner.rssrc/openhuman/agent/triage/escalation.rssrc/openhuman/channels/providers/web.rssrc/openhuman/composio/action_tool.rssrc/openhuman/composio/mod.rssrc/openhuman/composio/ops.rssrc/openhuman/context/debug_dump.rssrc/openhuman/context/prompt.rssrc/openhuman/learning/prompt_sections.rssrc/openhuman/tools/impl/agent/mod.rssrc/openhuman/tools/impl/agent/spawn_subagent.rs
…ture Follow-up to tinyhumansai#447. The main patch changed three signatures that test callers hadn't been updated for, and flipped one assertion that was validating the now-removed prose schema duplication. - `SubagentRunOptions` test constructors in subagent_runner.rs (2 sites) now pass `toolkit_override: None`. - `ConnectedIntegration` test constructor in orchestrator_tools.rs now passes `connected: true` (the default for test integrations — they're treated as authorized so delegation logic still runs). - 12 `render_subagent_system_prompt` test callers in prompt.rs now pass `&[]` for the new `extra_tools` slice and `ToolCallFormat::PFormat` for the new `tool_call_format` argument. - `render_subagent_system_prompt_honors_identity_safety_and_skills_flags` used to assert `rendered.contains("Parameters:")` on the Json dispatcher branch — that was valid in the old world where the prose `## Tools` section dumped full JSON schemas for Json/Native formats. The main patch deliberately removes that dump (it was the ~30k-token duplication of the native `tools` field), so the test now asserts the opposite: no `## Tools` header and no `Parameters:` line are emitted for Native/Json dispatchers. The schemas still travel through the provider request's `tools` field. Also picks up `rustfmt` rewraps in action_tool.rs and ops.rs from a background linter run — pure whitespace, no semantic change. Verified green against the full `cargo test --lib` suite for every test touched by this PR: - openhuman::context::prompt::tests (26 passed) - openhuman::agent::harness::subagent_runner::tests (19 passed) - openhuman::composio::ops::tests (2 passed) - openhuman::tools::impl::agent::tests (0 scoped) The 7 remaining failures in `cargo test --lib` are pre-existing Windows-path/filesystem flakes in subsystems this PR doesn't touch (self_healing polyfill path separator, cron scheduler shell spawning, local_ai::paths absolute-path detection, security::policy sandbox path handling, composio::trigger_history jsonl archive, and a real pre-existing `Option::unwrap()` panic in browser::screenshot). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…test stub Follow-up to tinyhumansai#447. The `tests/agent_harness_public.rs` integration test constructs a `ParentExecutionContext` in `stub_parent_context` that was missing the two fields added in the main patch (`composio_client` and `tool_call_format`). `cargo test --lib` doesn't compile integration tests, which is why this slipped past local verification — it only surfaced on Linux CI. - `composio_client: None` — the stub parent has no composio client because these tests don't exercise the integration-overview path. - `tool_call_format: ToolCallFormat::PFormat` — default legacy format; none of the tests in this file exercise the sub-agent renderer's format branching, so PFormat is the safe pick. Verified locally that `cargo test --no-run` compiles all integration test targets including `agent_harness_public`. (The `agent_memory_loader_public` link error in the local run is a pre-existing Windows `libucrt`/`fgets` C-runtime issue unrelated to this PR — won't reproduce on Linux CI.) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/openhuman/context/prompt.rs (1)
1000-1018:⚠️ Potential issue | 🟠 MajorKeep the tool catalog for
ToolCallFormat::Json.
Jsonis still the prompt-driven<tool_call>{"name": ...}</tool_call>path, not the provider-native channel. Skipping## Toolshere leaves Json-mode subagents with protocol instructions but no visible tool names or schemas whenever the provider isn't consuming nativetools. OnlyNativecan safely omit the prose catalog.Possible fix
- if matches!(tool_call_format, ToolCallFormat::PFormat) { + if !matches!(tool_call_format, ToolCallFormat::Native) { out.push_str("## Tools\n\n"); - let render_one = |out: &mut String, tool: &dyn Tool| { - let sig = render_pformat_signature_for_box_tool(tool); - let _ = writeln!( - out, - "- **{}**: {}\n Call as: `{}`", - tool.name(), - tool.description(), - sig - ); + let render_one = |out: &mut String, tool: &dyn Tool| match tool_call_format { + ToolCallFormat::PFormat => { + let sig = render_pformat_signature_for_box_tool(tool); + let _ = writeln!( + out, + "- **{}**: {}\n Call as: `{}`", + tool.name(), + tool.description(), + sig + ); + } + ToolCallFormat::Json => { + let _ = writeln!( + out, + "- **{}**: {}\n Parameters: `{}`", + tool.name(), + tool.description(), + tool.parameters_schema() + ); + } + ToolCallFormat::Native => unreachable!(), };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/context/prompt.rs` around lines 1000 - 1018, The current branch only keeps the prose tools catalog when tool_call_format == ToolCallFormat::PFormat, but Json also requires the prose `## Tools` section; update the conditional around the tools-catalog generation (the matches! check using tool_call_format and the ToolCallFormat enum) to include ToolCallFormat::Json as well (or conversely to only omit the catalog for the Native variant), so that both PFormat and Json emit the prose tool list including extra_tools and compact signatures.src/openhuman/composio/ops.rs (1)
538-542:⚠️ Potential issue | 🟠 MajorPrefix-match toolkit slugs with a delimiter.
starts_with(&slug.to_uppercase())still false-matches shared prefixes (git→GITHUB_*). That can attach the wrong action catalog to an integration and later register invalid dynamic tools.Possible fix
- t.function.name.starts_with(&slug.to_uppercase()) + { + let prefix = format!("{}_", slug.to_ascii_uppercase()); + t.function.name.starts_with(&prefix) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/composio/ops.rs` around lines 538 - 542, The filter currently uses starts_with(&slug.to_uppercase()) which false-matches shared prefixes; change the prefix check to require a delimiter after the toolkit slug (e.g., check starts_with(&format!("{}_", slug.to_uppercase()))) so only names like "GMAIL_" match and not "GMAILIFY" or "GITHUB" for "git"; update the closure that filters t.function.name (the filter on t.function.name.starts_with(...)) to use the slug + delimiter instead of the raw uppercase slug.
🧹 Nitpick comments (2)
src/openhuman/composio/ops.rs (1)
564-569: Usedebug!for this cache-miss summary.This runs on a normal prompt-assembly path, so
info!will be noisy in steady state.debug!is the better fit here.As per coding guidelines,
src/**/*.rs: Uselog/tracingatdebugortracelevel for development-oriented diagnostics in Rust.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/composio/ops.rs` around lines 564 - 569, Change the noisy info-level log in fetch_connected_integrations to a debug-level message: replace tracing::info! with tracing::debug! (preserving the structured fields total = integrations.len(), connected = connected_count, and the message "[composio] fetch_connected_integrations: done") so this cache-miss summary emits at debug level per the tracing/logging guidelines.src/openhuman/composio/action_tool.rs (1)
84-119: Add tracing around dynamic action execution.This new external-call path has no debug/trace logs at start, success, or failure, so troubleshooting broken dynamic tools depends entirely on the event stream. A small structured tracing envelope with
action_nameandelapsed_mswould make this much easier to follow.As per coding guidelines,
src/**/*.rs: Add substantial, development-oriented logs on new/changed flows; include logs at entry/exit points, branch decisions, external calls, retries/timeouts, state transitions, and error handling paths.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/composio/action_tool.rs` around lines 84 - 119, Add structured tracing around ComposioAction::execute: at the start log a trace/debug that the dynamic action is being invoked with the action_name and the input args, before calling self.client.execute_tool(&self.action_name, ...); after the call log success (including action_name, elapsed_ms, resp.successful, resp.error and cost_usd) on the Ok branch, and log failure (action_name, elapsed_ms and the error string) on the Err branch; use the existing function/method names (execute, self.action_name, self.client.execute_tool, and the DomainEvent publish sites) to locate the spots and emit trace/debug/error events consistent with the project's tracing macros so developers can follow entry, exit, and branch decisions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/openhuman/composio/action_tool.rs`:
- Around line 92-105: The match branch handling Ok(resp) should return a failure
ToolResult when the provider indicates unsuccessful: keep publishing the
ComposioActionExecuted event (using self.action_name, resp.successful,
resp.error, cost_usd, elapsed_ms) but if resp.successful is false return
ToolResult::error (including resp.error or the serialized resp as the error
payload) instead of always returning ToolResult::success; otherwise continue to
return ToolResult::success with the serialized resp when resp.successful is
true.
In `@src/openhuman/composio/ops.rs`:
- Around line 475-477: The Err branches in fetch_connected_integrations
currently swallow transient errors from
list_toolkits/list_connections/list_tools and return Some(Vec::new()), which
causes degraded snapshots to be cached; instead, keep the warning but propagate
the failure by returning None so callers and caching logic can avoid storing a
degraded empty snapshot. Update the Err(e) arms for the list_toolkits,
list_connections, and list_tools calls inside fetch_connected_integrations to
log the error (tracing::warn!) and then return None rather than
Some(Vec::new()); ensure you change the arms referenced around the existing
Err(e) blocks (the ones handling list_toolkits, list_connections, and
list_tools) so no transient backend error becomes an authoritative cached empty
integration list.
---
Duplicate comments:
In `@src/openhuman/composio/ops.rs`:
- Around line 538-542: The filter currently uses
starts_with(&slug.to_uppercase()) which false-matches shared prefixes; change
the prefix check to require a delimiter after the toolkit slug (e.g., check
starts_with(&format!("{}_", slug.to_uppercase()))) so only names like "GMAIL_"
match and not "GMAILIFY" or "GITHUB" for "git"; update the closure that filters
t.function.name (the filter on t.function.name.starts_with(...)) to use the slug
+ delimiter instead of the raw uppercase slug.
In `@src/openhuman/context/prompt.rs`:
- Around line 1000-1018: The current branch only keeps the prose tools catalog
when tool_call_format == ToolCallFormat::PFormat, but Json also requires the
prose `## Tools` section; update the conditional around the tools-catalog
generation (the matches! check using tool_call_format and the ToolCallFormat
enum) to include ToolCallFormat::Json as well (or conversely to only omit the
catalog for the Native variant), so that both PFormat and Json emit the prose
tool list including extra_tools and compact signatures.
---
Nitpick comments:
In `@src/openhuman/composio/action_tool.rs`:
- Around line 84-119: Add structured tracing around ComposioAction::execute: at
the start log a trace/debug that the dynamic action is being invoked with the
action_name and the input args, before calling
self.client.execute_tool(&self.action_name, ...); after the call log success
(including action_name, elapsed_ms, resp.successful, resp.error and cost_usd) on
the Ok branch, and log failure (action_name, elapsed_ms and the error string) on
the Err branch; use the existing function/method names (execute,
self.action_name, self.client.execute_tool, and the DomainEvent publish sites)
to locate the spots and emit trace/debug/error events consistent with the
project's tracing macros so developers can follow entry, exit, and branch
decisions.
In `@src/openhuman/composio/ops.rs`:
- Around line 564-569: Change the noisy info-level log in
fetch_connected_integrations to a debug-level message: replace tracing::info!
with tracing::debug! (preserving the structured fields total =
integrations.len(), connected = connected_count, and the message "[composio]
fetch_connected_integrations: done") so this cache-miss summary emits at debug
level per the tracing/logging guidelines.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0e453a1b-3ce9-4444-b83e-36ef47a441f0
📒 Files selected for processing (5)
src/openhuman/agent/harness/subagent_runner.rssrc/openhuman/composio/action_tool.rssrc/openhuman/composio/ops.rssrc/openhuman/context/prompt.rssrc/openhuman/tools/orchestrator_tools.rs
✅ Files skipped from review due to trivial changes (1)
- src/openhuman/tools/orchestrator_tools.rs
…inyhumansai#447) Four fixes from the CodeRabbit review of PR tinyhumansai#570, ranked by impact: ## `context/prompt.rs` — Json dispatcher needs prose tool catalog The initial patch gated the prose `## Tools` section behind `matches!(tool_call_format, ToolCallFormat::PFormat)`, which conflated Json with Native. Only `ToolCallFormat::Native` uses the provider's native function-calling channel where schemas travel via the request body's `tools` field. `ToolCallFormat::Json` is a prompt-driven format (the model wraps JSON tool calls in `<tool_call>` tags) and relies on the prose catalog the same way PFormat does — without it the model sees protocol instructions but no visible tool names or schemas. Inverted the guard to `!matches!(tool_call_format, Native)` so PFormat and Json both render the `## Tools` section with their respective per-entry shapes (compact `Call as:` signature for PFormat, inline `Parameters:` JSON schema for Json). Updated the `render_subagent_system_prompt_honors_identity_safety_and_skills_flags` test: the Json assertion now expects `Parameters:` to be present again (reverting commit 2's flip), and a new Native-branch assertion guards against the ~54k-token schema duplication the original PR fixed. ## `composio/ops.rs` — don't cache degraded snapshots All three backend calls in `fetch_connected_integrations_uncached` (`list_toolkits`, `list_connections`, `list_tools`) previously returned `Some(Vec::new())` or fell through to an empty inner vec on transient errors. The outer `fetch_connected_integrations` caches whatever the uncached path returns, so a single transient 5xx would silently hide every integration, mark connected toolkits as disconnected, or register dynamic Composio tools with zero callable actions — until the cache is invalidated or the process restarts. Changed all three branches to return `None`, which signals the caller to NOT cache the result and retry on the next call. ## `composio/ops.rs` — prefix match needs a delimiter `starts_with(&slug.to_uppercase())` false-matches when two toolkit slugs share a text prefix (e.g. `git` vs `github`). The current allowlist doesn't trigger this, but adding a new toolkit could silently leak actions between integrations. Anchored the prefix with an underscore so `GMAIL_SEND_EMAIL` matches `gmail_`, not just `gmail`. ## `scripts/stage-core-sidecar.mjs` — resolve CARGO_TARGET_DIR vs repo root `resolve(process.env.CARGO_TARGET_DIR)` uses the process's current working directory, but the `cargo build` spawn below runs with `cwd: root`. For an absolute env var this is identical, but a relative `CARGO_TARGET_DIR` and a cwd outside the repo would make the two paths disagree and the binary lookup would miss. Defensive fix: `resolve(root, process.env.CARGO_TARGET_DIR)` so both paths anchor to the same base. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/openhuman/context/prompt.rs (1)
642-720:⚠️ Potential issue | 🟠 MajorAdd structured debug/trace logs on the new branch-heavy prompt paths.
These changed flows now make key routing/rendering decisions (
agent_id,tool_call_format, spawn-vs-direct wording, connected integration counts) but currently have little instrumentation for debugging regressions.♻️ Suggested logging additions
impl PromptSection for ConnectedIntegrationsSection { @@ fn build(&self, ctx: &PromptContext<'_>) -> Result<String> { + tracing::debug!( + agent_id = ctx.agent_id, + integration_count = ctx.connected_integrations.len(), + "[context::prompt] rendering connected integrations section" + ); if ctx.connected_integrations.is_empty() { return Ok(String::new()); } @@ let is_skill_executor = ctx.agent_id == "skills_agent"; + tracing::debug!( + agent_id = ctx.agent_id, + is_skill_executor, + "[context::prompt] connected integrations render branch selected" + ); @@ pub fn render_subagent_system_prompt_with_format( @@ ) -> String { let mut out = String::new(); + tracing::debug!( + ?tool_call_format, + allowed_tool_count = allowed_indices.len(), + extra_tool_count = extra_tools.len(), + integration_count = connected_integrations.len(), + include_identity = options.include_identity, + include_safety_preamble = options.include_safety_preamble, + include_skills_catalog = options.include_skills_catalog, + "[context::prompt] rendering subagent system prompt" + ); @@ if !matches!(tool_call_format, ToolCallFormat::Native) { + tracing::debug!( + ?tool_call_format, + "[context::prompt] rendering prose tools catalog for subagent" + ); out.push_str("## Tools\n\n");As per coding guidelines
src/**/*.rs: “Add substantial, development-oriented logs on new/changed flows; include logs at … branch decisions … and error handling paths,” and “Use log/tracing at debug or trace level for development-oriented diagnostics in Rust.”Also applies to: 1000-1165
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/context/prompt.rs` around lines 642 - 720, Add debug/trace instrumentation to the new branching in Prompt::build: log the incoming ctx.agent_id and counts/states of ctx.connected_integrations, log the is_skill_executor decision, and log which integration toolkits are being rendered and whether they are filtered by connected==true; emit trace logs for the skills_agent branch (listing each toolkit/description emitted) and for the delegator branch (listing each toolkit and the spawn_subagent snippet chosen), and add a debug log when returning an empty prompt due to no connected integrations. Use the existing function/method names (build, PromptContext, connected_integrations, is_skill_executor, skills_agent, spawn_subagent) and appropriate trace/debug level from the project's tracing/log crate.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/openhuman/context/prompt.rs`:
- Around line 1151-1153: Replace the singular wording "following external
service" with the plural "following external services" in the prompt string that
begins with "You have direct access to the following external service..." in
src/openhuman/context/prompt.rs (locate the literal substring to find the exact
string) so the copy correctly covers multiple integrations; update only the word
"service" to "services" in that string.
---
Outside diff comments:
In `@src/openhuman/context/prompt.rs`:
- Around line 642-720: Add debug/trace instrumentation to the new branching in
Prompt::build: log the incoming ctx.agent_id and counts/states of
ctx.connected_integrations, log the is_skill_executor decision, and log which
integration toolkits are being rendered and whether they are filtered by
connected==true; emit trace logs for the skills_agent branch (listing each
toolkit/description emitted) and for the delegator branch (listing each toolkit
and the spawn_subagent snippet chosen), and add a debug log when returning an
empty prompt due to no connected integrations. Use the existing function/method
names (build, PromptContext, connected_integrations, is_skill_executor,
skills_agent, spawn_subagent) and appropriate trace/debug level from the
project's tracing/log crate.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 810f47ce-82de-455b-9e92-584b08f4f56c
📒 Files selected for processing (3)
scripts/stage-core-sidecar.mjssrc/openhuman/composio/ops.rssrc/openhuman/context/prompt.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- scripts/stage-core-sidecar.mjs
- src/openhuman/composio/ops.rs
| "You have direct access to the following external service. \ | ||
| Action tools are available in your tool list with their \ | ||
| typed parameter schemas — call them by name.\n\n", |
There was a problem hiding this comment.
Fix singular wording in Connected Integrations copy.
Line 1151 says “following external service” but this block can list multiple integrations.
✏️ Proposed text fix
- "You have direct access to the following external service. \
+ "You have direct access to the following external services. \
Action tools are available in your tool list with their \
typed parameter schemas — call them by name.\n\n",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "You have direct access to the following external service. \ | |
| Action tools are available in your tool list with their \ | |
| typed parameter schemas — call them by name.\n\n", | |
| "You have direct access to the following external services. \ | |
| Action tools are available in your tool list with their \ | |
| typed parameter schemas — call them by name.\n\n", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/openhuman/context/prompt.rs` around lines 1151 - 1153, Replace the
singular wording "following external service" with the plural "following
external services" in the prompt string that begins with "You have direct access
to the following external service..." in src/openhuman/context/prompt.rs (locate
the literal substring to find the exact string) so the copy correctly covers
multiple integrations; update only the word "service" to "services" in that
string.
Summary
Cuts main agent prompt token cost ~98% on accounts with connected integrations and replaces the generic Composio dispatcher with native per-action tool calling inside
skills_agent.Closes #447.
Why
Issue #447 reported that prompt payloads were ballooning because main/orchestrator re-shipped a full prose enumeration of every connected toolkit's actions on every turn (~13.7k tokens for one gmail integration alone), and
skills_agenthad no way to scope tools to a single toolkit so it inherited a flat dispatcher (composio_execute) that the LLM couldn't reliably call without first round-trippingcomposio_list_tools.What changed
main / orchestrator prompt
ConnectedIntegrationsSection's per-action prose dump with a unified Delegation Guide: one bullet per integration (toolkit + one-line description + spawn snippet). ~30 tokens added per integration; ~5k+ tokens saved per connected toolkit.skills_agent(parameterized by toolkit)toolkitargument onspawn_subagentwith three pre-flight validation modes resolved against the parent's connected integrations list before any LLM call:ToolResult::error(model retries with a valid arg)ToolResult::error(lists valid choices)ToolResult::success(model surfaces "authorize in Settings → Integrations" gracefully — not flagged as a tool failure in the agent loop or web channel)ComposioActionTool(src/openhuman/composio/action_tool.rs) implements theTooltrait per Composio action. The sub-agent runner constructs N of these at spawn time from the cached integration overview and injects them via a newextra_toolsparameter throughrun_typed_mode→run_inner_loop.filter_tool_indicesdrops every skill-category parent tool when the spawn is a toolkit-scopedskills_agent, leaving only the freshly-built per-action tools — eliminatesapify_*,composio_list_*,composio_authorize, andcomposio_executefrom the toolkit-scoped surface.tool_call_formatinstead of hardcoding PFormat. Native dispatchers no longer carry a prose## Toolssection at all — schemas already travel through the request body's `tools` field, and listing them again in prose was a ~30k-token duplication that was blowing past the model's context window.Integration overview fetch
Plumbing
Verified end-to-end
Two RPC tests against a freshly-rebuilt sidecar (gmail connected, notion / slack / etc. allowlisted but not connected):
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements
Bug Fixes