feat(agent): add tool policy session boundary#2166
Merged
senamakel merged 6 commits intoMay 20, 2026
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
💤 Files with no reviewable changes (2)
📝 WalkthroughWalkthroughAdds a per-session tool policy subsystem (types, engine, prompt renderer), integrates policy snapshots into Agent build and runtime, enforces per-tool decisions at execution, updates prompt generation and dispatcher hook, and moves composio/config loads to per-tool live-config reloads with normalization and tests. ChangesTool Policy System
Sequence DiagramssequenceDiagram
participant AgentBuilder
participant ToolPolicyEngine
participant ToolPolicySession
participant Agent
AgentBuilder->>AgentBuilder: derive config, agent_definition_name, event_channel
AgentBuilder->>ToolPolicyEngine: build_session(channel, permissions, tools, visible_names)
ToolPolicyEngine->>ToolPolicySession: return session snapshot
AgentBuilder->>AgentBuilder: filter specs via visible_tool_specs_for_policy
AgentBuilder->>Agent: construct Agent with tool_policy, config, agent_definition_name, session_key
sequenceDiagram
participant Agent
participant ToolPolicySession
participant Tool
Agent->>Agent: execute_tool_call(call)
Agent->>ToolPolicySession: decision_for(call.name)
alt Decision Denied
ToolPolicySession-->>Agent: ToolPolicyDecision(denied)
Agent-->>Agent: return "blocked by tool policy" error (success=false)
else Decision Allowed
ToolPolicySession-->>Agent: ToolPolicyDecision(allowed)
Agent->>Tool: execute_with_options(...)
Tool-->>Agent: ToolResult
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. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
src/openhuman/agent/harness/session/turn.rs (1)
978-991: ⚡ Quick winLog the resolved policy denial.
This branch returns a blocked-tool result but leaves no structured trace of why the call was denied. A
debug!/trace!withtool,channel,required_permission, andallowed_permissionwould make policy decisions auditable when a session starts failing tool calls.As per coding guidelines "Use
log/tracingatdebugortracelevel on RPC entry and exit, error paths, state transitions, and any branch that is hard to infer from tests alone."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/agent/harness/session/turn.rs` around lines 978 - 991, Add a debug/trace log when a tool call is denied in the branch that checks self.tool_policy.decision_for(&call.name) and decision.is_denied(); log the tool name (call.name), the event channel (self.event_channel), the resolved required permission (decision.required_permission mapped to string or "unknown"), and the allowed permission (decision.allowed_permission) before returning the blocked result so policy decisions are auditable and follow the project's logging guideline.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/openhuman/agent_tool_policy/engine.rs`:
- Around line 58-67: The match arm in engine.rs that handles ToolPolicyAction
currently treats HideFromPrompt the same as Deny/RequireApproval by inserting
name.clone() into blocked_tool_names; change this so HideFromPrompt is not added
to blocked_tool_names — either remove HideFromPrompt from that arm and simply
skip inserting it, or add a separate hidden_tool_names set and insert into that
(while leaving RequireApproval and Deny inserting into blocked_tool_names); keep
allowed_tool_names behavior unchanged and update any downstream checks to
consult hidden_tool_names if you add it.
- Around line 15-119: Add structured debug/trace logging to trace policy
resolution: in permission_for_channel log the incoming channel and whether
channel_permissions was empty, the raw permission string looked up, and when
parse_permission_level returns None (the fallback to PermissionLevel::ReadOnly);
in parse_permission_level log the normalized input and when it fails to match
any case; and in build_session log per-tool decisions (tool.name(),
required_permission, allowed_permission, explicitly_hidden, exceeds_permission,
and the resulting ToolPolicyAction) before inserting into decisions/capabilities
so you can see state transitions for ToolPolicyDecision and ToolCapability. Use
the existing function names (build_session, permission_for_channel,
parse_permission_level) and enums (ToolPolicyAction, ToolPolicyDecision,
ToolCapability) and emit logs at debug/trace level via the project's
logging/tracing crate.
In `@src/openhuman/agent_tool_policy/prompt.rs`:
- Around line 39-49: The current prompt code writes out
session.blocked_tool_names into rendered (the writeln call that prints "-
Blocked tools: {}" using session.blocked_tool_names.iter()...), which exposes
restricted tool names to the model; change that write to emit a generic summary
instead (e.g. "- Some tools are blocked" or "- Blocked tools: <redacted> (N
tools)") and stop iterating/printing the actual names, leaving
session.blocked_tool_names only for internal logic and not for prompt text.
In `@src/openhuman/agent/harness/session/builder.rs`:
- Around line 357-371: The policy snapshot created by
ToolPolicyEngine::build_session is only used to filter provider tool specs via
visible_tool_specs_for_policy, but the earlier prompt builder still uses the
original static visible set and freezes that into ContextManager (so
set_visible_tool_names or a stricter channel policy won't prevent the model
being prompted to use disallowed tools); update the prompt assembly and
ContextManager initialization to consume the policy-derived visibility (use the
tool_policy / visible_tool_specs_unfiltered result instead of the original
visible_names when building tool-specific guidance and when calling
ContextManager constructors or setters like set_visible_tool_names), and ensure
any function that assembles prompt guidance (the prompt builder code path)
filters tool guidance through the same tool_policy before freezing state.
In `@src/openhuman/agent/harness/session/runtime.rs`:
- Around line 222-236: The session's tool policy isn't recomputed when the agent
identity is renamed because set_agent_definition_name() doesn't refresh the
snapshot taken by ToolPolicyEngine::build_session; update
set_agent_definition_name to call the existing rebuild_tool_policy_session()
after assigning self.agent_definition_name so self.tool_policy (and derived
values like self.visible_tool_specs based on self.visible_tool_names) are
rebuilt to reflect the new agent identity.
In `@src/openhuman/config/schema/agent.rs`:
- Around line 198-203: The docs and tests disagree on the permission token
spelling ("readonly" vs "read_only"); pick one canonical token and make the code
and docs consistent, or accept both by normalizing at parse time. Update the
documentation comment under "Per-channel maximum permission level" to use the
chosen canonical value (or list both as aliases), and add normalization in the
permission parsing logic (e.g., the function/method that parses per-channel
permission strings—implement a normalize_permission_level or map in the parser
that treats "read_only" and "readonly" as equivalent and converts to the
canonical internal value). Ensure any enum/variant names and tests/policy
fixtures use the same canonical token.
---
Nitpick comments:
In `@src/openhuman/agent/harness/session/turn.rs`:
- Around line 978-991: Add a debug/trace log when a tool call is denied in the
branch that checks self.tool_policy.decision_for(&call.name) and
decision.is_denied(); log the tool name (call.name), the event channel
(self.event_channel), the resolved required permission
(decision.required_permission mapped to string or "unknown"), and the allowed
permission (decision.allowed_permission) before returning the blocked result so
policy decisions are auditable and follow the project's logging guideline.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 098b86d4-8d50-4b66-a14c-a15ed941a162
📒 Files selected for processing (11)
src/openhuman/agent/harness/session/builder.rssrc/openhuman/agent/harness/session/runtime.rssrc/openhuman/agent/harness/session/turn.rssrc/openhuman/agent/harness/session/turn_tests.rssrc/openhuman/agent/harness/session/types.rssrc/openhuman/agent_tool_policy/engine.rssrc/openhuman/agent_tool_policy/mod.rssrc/openhuman/agent_tool_policy/prompt.rssrc/openhuman/agent_tool_policy/types.rssrc/openhuman/config/schema/agent.rssrc/openhuman/mod.rs
# Conflicts: # src/openhuman/composio/action_tool.rs
Contributor
Author
|
Current state after the latest update:
Ready for human maintainer review/merge. |
…icy-layer # Conflicts: # src/openhuman/agent/harness/session/builder.rs
Contributor
Author
|
@senamakel Updated #2166 with latest Conflict resolution kept both sides: Local validation on
PR is mergeable again; CI and CodeRabbit are rerunning on the new head. Please take another look when checks settle. |
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/openhuman/agent/harness/session/runtime.rs (1)
223-238: ⚡ Quick winAdd a structured debug/trace log for policy rebuilds.
This helper is now the central state-transition point for
tool_policyandvisible_tool_specs, but the rebuild is silent. A single structured breadcrumb here would make later policy denials or prompt/spec mismatches much easier to trace.Suggested diff
pub(super) fn rebuild_tool_policy_session(&mut self) { self.tool_policy = ToolPolicyEngine::build_session( &self.agent_definition_name, &self.event_channel, "session", &self.config.channel_permissions, self.tools.as_slice(), &self.visible_tool_names, ); let visible_specs = super::builder::visible_tool_specs_for_policy( self.tool_specs.as_slice(), &self.visible_tool_names, &self.tool_policy, ); self.visible_tool_specs = Arc::new(super::builder::dedup_visible_tool_specs(visible_specs)); + log::debug!( + target: "agent", + "[agent.tool_policy] rebuilt agent_definition_name={} channel={} visible_name_count={} visible_spec_count={} restricted_tool_count={}", + self.agent_definition_name, + self.event_channel, + self.visible_tool_names.len(), + self.visible_tool_specs.len(), + self.tool_policy.restricted_tool_count(), + ); }As per coding guidelines, "Use
log/tracingatdebugortracelevel on RPC entry and exit, error paths, state transitions, and any branch that is hard to infer from tests alone. Use structured, grep-friendly context with stable prefixes."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/agent/harness/session/runtime.rs` around lines 223 - 238, Add a structured debug/trace log entry to rebuild_tool_policy_session to record the policy rebuild event and key state before/after the change: log at debug or trace level inside rebuild_tool_policy_session (around the call to ToolPolicyEngine::build_session and after computing visible_tool_specs via visible_tool_specs_for_policy and dedup_visible_tool_specs) including stable fields like agent_definition_name, channel (string "session"), count/list of visible_tool_names, a short identifier or summary of the previous self.tool_policy and new tool_policy (or a hash/ID), and the lengths of previous and new self.visible_tool_specs; ensure the log uses structured key/value fields and runs both on entry and after successful rebuild to make policy denials and spec mismatches traceable.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/openhuman/agent/harness/session/runtime.rs`:
- Around line 223-238: Add a structured debug/trace log entry to
rebuild_tool_policy_session to record the policy rebuild event and key state
before/after the change: log at debug or trace level inside
rebuild_tool_policy_session (around the call to ToolPolicyEngine::build_session
and after computing visible_tool_specs via visible_tool_specs_for_policy and
dedup_visible_tool_specs) including stable fields like agent_definition_name,
channel (string "session"), count/list of visible_tool_names, a short identifier
or summary of the previous self.tool_policy and new tool_policy (or a hash/ID),
and the lengths of previous and new self.visible_tool_specs; ensure the log uses
structured key/value fields and runs both on entry and after successful rebuild
to make policy denials and spec mismatches traceable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1e57aa08-5ac1-4237-a503-5e1d7e770946
📒 Files selected for processing (6)
src/openhuman/agent/harness/session/builder.rssrc/openhuman/agent/harness/session/runtime.rssrc/openhuman/agent/harness/session/turn.rssrc/openhuman/agent/harness/session/turn_tests.rssrc/openhuman/agent/harness/session/types.rssrc/openhuman/config/schema/load.rs
🚧 Files skipped from review as they are similar to previous changes (5)
- src/openhuman/agent/harness/session/types.rs
- src/openhuman/agent/harness/session/turn_tests.rs
- src/openhuman/agent/harness/session/turn.rs
- src/openhuman/config/schema/load.rs
- src/openhuman/agent/harness/session/builder.rs
Contributor
Author
|
@senamakel CI is green on |
# Conflicts: # src/openhuman/agent/harness/session/turn.rs # src/openhuman/agent/harness/session/turn_tests.rs # src/openhuman/agent/harness/session/types.rs # src/openhuman/mod.rs
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
Author
|
@senamakel #2166 已同步最新 main 并解决冲突；当前 head |
12 tasks
mtkik
pushed a commit
to mtkik/openhuman-meet
that referenced
this pull request
May 21, 2026
CodeGhost21
pushed a commit
to CodeGhost21/openhuman
that referenced
this pull request
May 22, 2026
AusAgentSmith
pushed a commit
to AusAgentSmith/openhuman
that referenced
this pull request
May 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
agent_tool_policycore domain for per-session task profiles and tool decisions.Tool Policy Boundaryprompt section only when a configured channel policy actually restricts tools.channel_permissionsis empty; once configured, unknown channels fall back toreadonly.Problem
Solution
ToolPolicyEngineandToolPolicySessionto resolve allowed/blocked tools from channel policy, visible-tool allowlists, and each tool'sPermissionLevel.Agentand rebuild it when event context or visible tool names change.execute_tool_callbefore invoking the tool, returning a structured failure message.OPENHUMAN_WORKSPACEmutations.Submission Checklist
pnpm test:rust, and CI coverage remains the merge gate.## Related— N/A: no feature ID applies.docs/RELEASE-MANUAL-SMOKE.md) — N/A: core policy guard only, no release manual smoke surface changed.Closes #NNNin the## Relatedsection — N/A: no GitHub issue was provided for this B1 follow-up.Impact
channel_permissionspreserves the legacy unrestricted tool surface and does not inject a policy prompt boundary.Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
feat/agent-tool-policy-layer8604c3bd3f9ab042289b84e6d30ae20c79a34025Validation Run
pnpm --filter openhuman-app format:check— N/A: no frontend/app TypeScript files changed.pnpm typecheck— N/A: no frontend/app TypeScript files changed.GGML_NATIVE=OFF cargo test --manifest-path Cargo.toml agent_tool_policy --libGGML_NATIVE=OFF cargo test --manifest-path Cargo.toml execute_tool_call --libGGML_NATIVE=OFF cargo test --manifest-path Cargo.toml system_prompt_includes_tool_policy_boundary --libGGML_NATIVE=OFF cargo test --manifest-path Cargo.toml set_agent_definition_name_refreshes_tool_policy_identity --libGGML_NATIVE=OFF cargo test --manifest-path Cargo.toml factory_routes_through_direct_when_mode_is_direct --lib -- --nocaptureGGML_NATIVE=OFF cargo test --manifest-path Cargo.toml composio::action_tool --libGGML_NATIVE=OFF cargo test --manifest-path Cargo.toml composio::tools::tests --libcargo fmt --manifest-path Cargo.toml --all --checkGGML_NATIVE=OFF cargo check --manifest-path Cargo.tomlpnpm test:rust(passed; emitted existing Rust warnings and a local Node engine warning: repo wants Node >=24, current v22.14.0)git diff --checkGGML_NATIVE=OFF cargo test --manifest-path Cargo.toml composio::action_tool --libGGML_NATIVE=OFF cargo test --manifest-path Cargo.toml agent_tool_policy --libGGML_NATIVE=OFF cargo check --manifest-path Cargo.tomlcargo fmt --manifest-path Cargo.toml --all --check(after merge from upstream/main)Validation Blocked
command:N/Aerror:N/Aimpact:N/ABehavior Changes
Parity Contract
channel_permissionskeeps the previous unrestricted tool surface and omits the new prompt boundary.readonlyonly after at least one channel policy is configured; hidden or denied tools fail before execution.Duplicate / Superseded PR Handling
Summary by CodeRabbit
New Features
Documentation
Tests