feat: add ToolPolicy middleware for tool-loop allow/deny gate

[Liohtml]

Summary

Closes #2131.

• Adds a synchronous ToolPolicy trait in src/openhuman/tools/policy.rs with a PolicyDecision enum (Allow / Deny(String))
• Ships a DefaultToolPolicy that returns Allow unconditionally (backward compatible)
• Integrates the policy check into run_tool_call_loop (the bus/CLI tool execution path) -- denied tools return a ToolResult error to the model without executing, preventing any side effects
• Emits tracing::debug! with tool name and denial reason when a call is denied (updated from warn! per code review)
• Policy gate runs before the approval hook and external-effect gate so denied tools cannot trigger approval-side effects
• Includes 5 unit tests: default allows all, custom deny blocks matching tool, allows non-matching, deny-all blocks every tool, unknown tool names allowed by default

This complements the existing async agent::tool_policy::ToolPolicy (session layer) by covering the lower-level run_tool_call_loop path used by the bus and CLI dispatchers.

Scope

• Minimal: just the trait, default impl, one integration point, and tests
• No changes to existing tool behavior
• No approval workflows -- just Allow/Deny
• No per-tool config (that's Add per-server and per-tool allowlists to the generic MCP bridge #2134)

Test plan

• cargo check -p openhuman --lib passes
• cargo test -p openhuman "tool" --lib -- 1402 tests pass
• cargo test -p openhuman "policy" --lib -- 153 tests pass (includes 5 new)
• cargo fmt --all applied
• CI checks pass (TypeScript type-check failure is pre-existing i18n issue fixed in fix(i18n): remove duplicate German keys unblocking main's Type Check #2495, unrelated to this PR's Rust-only changes)

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Introduced a tool-execution policy system: each tool call is evaluated and may be Allowed or Denied. Denied calls produce clear rejection messages, are recorded as failed per-call results, and are skipped. A DefaultToolPolicy preserves previous allow-all behavior.

• Tests

  • Updated harness and tests to exercise policy evaluation across diverse tool-call scenarios and edge cases.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds a pluggable tool-policy gate: ToolPolicy + PolicyDecision, DefaultToolPolicy returning Allow, threaded into run_tool_call_loop so each parsed tool call is evaluated and denied calls emit failed progress and a recorded denial without executing the tool.

Changes

Tool execution policy gating

Layer / File(s)	Summary
Policy abstraction and default implementation src/openhuman/tools/policy.rs, src/openhuman/tools/mod.rs	PolicyDecision enum and ToolPolicy trait define the policy contract. DefaultToolPolicy always returns Allow. Module re-exports make types available at openhuman::tools scope. Tests validate default, custom deny-by-name, and deny-all policies.
Agent tool-loop policy evaluation src/openhuman/agent/harness/tool_loop.rs, src/openhuman/agent/harness/tool_loop_tests.rs	run_tool_call_loop adds a tool_policy parameter. Before each tool executes, the loop evaluates the policy; on Deny, emission of failed ToolCallCompleted progress and recorded denial in tool results replaces execution. Imports and signature updated accordingly. Tests cover summarization, streaming, error handling, max iterations, and stop-hook scenarios.
Native agent handler wiring src/openhuman/agent/bus.rs	register_agent_handlers passes DefaultToolPolicy to run_tool_call_loop in the native agent.run_turn handler.
Test harness call-site integration src/openhuman/agent/harness/bughunt_tests.rs, src/openhuman/agent/harness/harness_gap_tests.rs, src/openhuman/agent/harness/test_support_test.rs, src/openhuman/agent/harness/tests.rs, src/openhuman/agent/harness/tool_loop_tests.rs	All test call sites invoking run_tool_call_loop were updated to pass &crate::openhuman::tools::policy::DefaultToolPolicy. No test assertions or behavior were changed; only invocation signatures were updated.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• tinyhumansai/openhuman#2137: Related work implementing pre-execution ToolPolicy gating in Agent::execute_tool_call.
• tinyhumansai/openhuman#2100: Overlaps at run_tool_call_loop changes (context-window/token budgeting and history trimming).

Suggested reviewers

• graycyrus
• senamakel

"🐰 I hopped through code and found a gate,
A policy listens, patient and polite,
Default says yes, but some names must wait,
Denials are logged, the loop skips to the light,
Tools now knock before they write."

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely summarizes the main feature: adding a ToolPolicy middleware for tool-loop allow/deny gating, which is the core change across all modified files.
Linked Issues check	✅ Passed	The PR fully implements the core requirements from issue #2131: introduces ToolPolicy trait with PolicyDecision enum, integrates it into run_tool_call_loop before tool execution, provides DefaultToolPolicy preserving default behavior, includes unit tests proving denial works before execution, and emits structured denial reasons in logs.
Out of Scope Changes check	✅ Passed	All changes are tightly scoped to the stated objective: new policy module and trait definition, integration into tool-loop execution, re-exports in module roots, and test updates. No unrelated refactoring, behavior changes to existing tools, or approval workflow additions are present.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ 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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

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/agent/harness/tool_loop.rs (1)

608-740: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Policy gate runs too late in the call pipeline.

On Line 609 and Line 687, approval paths can execute before the policy check at Line 721. A call denied by policy can still trigger approval-side effects first. Move policy evaluation before approval handling (and before approval-gate interception) so denied calls short-circuit immediately.

Suggested reordering

-            // ── Approval hook ────────────────────────────────
-            if let Some(mgr) = approval {
-                ...
-            }
-
             let tool_opt: Option<&dyn Tool> = tools_registry
                 .iter()
                 .chain(extra_tools.iter())
                 .find(|t| t.name() == call.name && is_visible(t.name()))
                 .map(|b| b.as_ref());

+            if let PolicyDecision::Deny(reason) =
+                tool_policy.evaluate(&call.name, &call.arguments)
+            {
+                let denied = format!("Tool '{}' denied by policy: {reason}", call.name);
+                emit_failed_completion(&denied).await;
+                individual_results.push(denied.clone());
+                let _ = writeln!(
+                    tool_results,
+                    "<tool_result name=\"{}\">\n{denied}\n</tool_result>",
+                    call.name
+                );
+                continue;
+            }
+
+            // ── Approval hook ────────────────────────────────
+            if let Some(mgr) = approval {
+                ...
+            }

🤖 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/tool_loop.rs` around lines 608 - 740, The
PolicyDecision check currently runs after approval handling and the
external-effect ApprovalGate; move the policy evaluation to run immediately
after resolving tool_opt (i.e., right after the tool lookup/tracing block) so
denied tools short-circuit before any approval-side effects. Concretely:
relocate the block that calls tool_policy.evaluate(&call.name, &call.arguments)
/ matches PolicyDecision::Deny (and its
emit_failed_completion/individual_results/tool_results handling) to immediately
follow the `let tool_opt: Option<&dyn Tool> = ...` and tracing::debug(...)
lines; keep later checks (approval manager using ApprovalRequest,
mgr.prompt_cli, mgr.record_decision, ApprovalGate::try_global, and
tool.external_effect_with_args) after that repositioned policy block so
approvals and gate interception never run for policy-denied calls.

🤖 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/harness/tool_loop.rs`:
- Around line 724-729: The diagnostic log currently uses tracing::warn! for the
expected policy-denial branch; change that call to tracing::debug! (or
tracing::trace!) so it uses a lower verbosity level per repo logging rules.
Locate the tracing::warn! invocation that logs iteration, tool =
call.name.as_str(), and reason (the policy-denied branch in tool_loop.rs) and
replace the macro with tracing::debug! while preserving the structured fields
(iteration, tool, reason) and the message string so caller semantics and fields
remain identical.

---

Outside diff comments:
In `@src/openhuman/agent/harness/tool_loop.rs`:
- Around line 608-740: The PolicyDecision check currently runs after approval
handling and the external-effect ApprovalGate; move the policy evaluation to run
immediately after resolving tool_opt (i.e., right after the tool lookup/tracing
block) so denied tools short-circuit before any approval-side effects.
Concretely: relocate the block that calls tool_policy.evaluate(&call.name,
&call.arguments) / matches PolicyDecision::Deny (and its
emit_failed_completion/individual_results/tool_results handling) to immediately
follow the `let tool_opt: Option<&dyn Tool> = ...` and tracing::debug(...)
lines; keep later checks (approval manager using ApprovalRequest,
mgr.prompt_cli, mgr.record_decision, ApprovalGate::try_global, and
tool.external_effect_with_args) after that repositioned policy block so
approvals and gate interception never run for policy-denied calls.

🪄 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: 4a525d67-79cf-4de6-bb36-3a4f42e9ed20

📥 Commits

Reviewing files that changed from the base of the PR and between 65d92bf and c737377.

📒 Files selected for processing (9)

• src/openhuman/agent/bus.rs
• src/openhuman/agent/harness/bughunt_tests.rs
• src/openhuman/agent/harness/harness_gap_tests.rs
• src/openhuman/agent/harness/test_support_test.rs
• src/openhuman/agent/harness/tests.rs
• src/openhuman/agent/harness/tool_loop.rs
• src/openhuman/agent/harness/tool_loop_tests.rs
• src/openhuman/tools/mod.rs
• src/openhuman/tools/policy.rs

[Liohtml]

Credit to @vaddisrinivas for the well-specified issue (#2131) that made this implementation straightforward. The tool-policy middleware design and acceptance criteria in the issue guided the trait shape and test coverage directly.

[senamakel]

hey @Liohtml can u enable maintainers to make edits in this PR? and also patch the typecheck and e2e tests?