fix(agent): don't prompt for approval when the session permission layer will refuse the call

crates/openhuman-core/src/agent/tinyagents/harness_assembly.rs

@@ -558,12 +558,21 @@ pub(super) fn assemble_turn_harness(
     // `ApprovalGate`, a denial short-circuits with a model-consumable result, and
     // an approved call records a terminal audit row. Replaces the inline approval
     // block that used to live in the legacy tool adapter.
+    //
+    // The session permission snapshot rides along (issue #7649): this wrap is
+    // OUTSIDE `ToolPolicyMiddleware`, so `requires_approval` must pre-check the
+    // permission ceiling or a call the session refuses anyway prompts the human
+    // before it is denied.
     if !hosted_security_gate {
+        let session_permission = tool_policy
+            .as_ref()
+            .map(|enforcement| (enforcement.session.clone(), enforcement.channel.clone()));
         harness.push_tool_middleware(Arc::new(ApprovalGateMiddleware::new(
             "approval_security",
-            Arc::new(middleware::ApprovalSecurityMiddleware::new(
-                tool_sets.clone(),
-            )),
+            Arc::new(
+                middleware::ApprovalSecurityMiddleware::new(tool_sets.clone())
+                    .with_session_permission(session_permission),
+            ),
         )));
     }
 

crates/openhuman-core/src/agent/tinyagents/live_harness.rs

@@ -9,7 +9,8 @@
 //!
 //! 1. the SDK tool-policy projection (`require_sandbox`),
 //! 2. human-in-the-loop approval (`ApprovalSecurityMiddleware` through the
-//!    global `ApprovalGate`),
+//!    global `ApprovalGate`; it pre-checks the session permission snapshot so a
+//!    call the layer in 4 will refuse never raises a card — issue #7649),
 //! 3. the CLI/RPC-only scope gate,
 //! 4. the session's `ToolPolicyMiddleware` (deny / require-approval rules),
 //! 5. credential scrubbing of every tool result,
@@ -94,9 +95,13 @@ pub(crate) fn assemble_live_tool_harness(
     ));
     harness.push_tool_middleware(Arc::new(ApprovalGateMiddleware::new(
         "approval_security",
-        Arc::new(middleware::ApprovalSecurityMiddleware::new(
-            tool_sets.clone(),
-        )),
+        Arc::new(
+            middleware::ApprovalSecurityMiddleware::new(tool_sets.clone()).with_session_permission(
+                tool_policy
+                    .as_ref()
+                    .map(|enforcement| (enforcement.session.clone(), enforcement.channel.clone())),
+            ),
+        ),
     )));
     harness.push_tool_middleware(Arc::new(middleware::CliRpcOnlyMiddleware::new(
         tool_sets.clone(),

crates/openhuman-core/src/agent/tinyagents/middleware/approval.rs

@@ -1,5 +1,13 @@
 //! [`ApprovalSecurityMiddleware`]: route external-effect tool calls through
 //! OpenHuman's human-in-the-loop approval gate at the tool boundary.
+//!
+//! When the caller attaches the session's permission snapshot
+//! ([`ApprovalSecurityMiddleware::with_session_permission`]), `requires_approval`
+//! first asks whether the permission layer will refuse the call anyway
+//! (issue #7649): this wrap sits OUTSIDE `ToolPolicyMiddleware`, so a
+//! permission-refused call used to raise an approval card and was denied only
+//! after the human answered. A denied verdict skips the prompt — the inner
+//! permission layer answers with its own reason instead.
 
 use std::sync::Arc;
 
@@ -57,12 +65,37 @@ pub(crate) struct ApprovalSecurityMiddleware {
     /// The same `Arc`-shared tool sets the runner registers, used to resolve a
     /// call's OpenHuman `Tool` by name so `external_effect_with_args` can gate.
     tool_sets: Vec<Arc<Vec<Box<dyn Tool>>>>,
+    /// The session's channel-permission snapshot and channel name, when the
+    /// caller has one (the session chat / live paths pass
+    /// `ToolPolicyEnforcement`'s; CLI and sub-agent paths pass `None`).
+    ///
+    /// `requires_approval` consults it before prompting (issue #7649): this
+    /// middleware is the OUTER tool wrap, and the permission ceiling lives on
+    /// the inner `ToolPolicyMiddleware` — so without the pre-check a call the
+    /// session refuses anyway raises an approval card first and is denied only
+    /// after the human answers. A permission-refused call must fail with the
+    /// permission layer's own message instead of interrupting anyone.
+    permission: Option<(crate::tools::agent_policy::ToolPolicySession, String)>,
 }
 
 impl ApprovalSecurityMiddleware {
     /// Build the middleware over the runner's shared tool sets.
     pub(crate) fn new(tool_sets: Vec<Arc<Vec<Box<dyn Tool>>>>) -> Self {
-        Self { tool_sets }
+        Self {
+            tool_sets,
+            permission: None,
+        }
+    }
+
+    /// Attach the session permission boundary the pre-check consults
+    /// (`ToolPolicySession` + channel); `None` keeps the prompt-first
+    /// behaviour for paths with no session permission snapshot.
+    pub(crate) fn with_session_permission(
+        mut self,
+        permission: Option<(crate::tools::agent_policy::ToolPolicySession, String)>,
+    ) -> Self {
+        self.permission = permission;
+        self
     }
 
     /// Whether the named tool declares an external effect for these args.
@@ -107,12 +140,46 @@ impl ApprovalResolver<crate::agent::tinyagents::host::OpenHumanRunContext>
         // Resolve external-effect up front so no tool borrow is held across the
         // approval await.
         let has_ext = self.requires_approval(&call.name, &call.arguments).await;
+        if !has_ext {
+            tracing::debug!(
+                tool = %call.name,
+                has_external_effect = false,
+                "[tinyagents::mw] checking tool for approval"
+            );
+            return false;
+        }
+        // Issue #7649: the session's permission ceiling runs INSIDE this wrap
+        // (`ToolPolicyMiddleware::channel_permission_block`), so a call it will
+        // refuse reaches that refusal only after this prompt — approval-then-
+        // deny. Consult the identical verdict here: a call the session refuses
+        // regardless of the human's answer must not raise a card; returning
+        // `false` falls through to the permission layer, which denies with its
+        // own rendered reason. `false` for `desktop_approval_disabled` is exact:
+        // the inherent check above already returned `false` when desktop
+        // approvals are disabled, and that is the value the inner
+        // `ToolPolicyMiddleware` computes for the same tool on the same config.
+        if let Some((session, channel)) = &self.permission {
+            let view = super::tool_policy::SessionPermissionView {
+                session,
+                tool_sets: &self.tool_sets,
+                channel,
+            };
+            if view.channel_permission_block(call, false).is_some() {
+                tracing::debug!(
+                    tool = %call.name,
+                    channel = %channel,
+                    "[tinyagents::mw] skipping approval prompt: the session permission \
+                     layer will refuse this call"
+                );
+                return false;
+            }
+        }
         tracing::debug!(
             tool = %call.name,
-            has_external_effect = has_ext,
+            has_external_effect = true,
             "[tinyagents::mw] checking tool for approval"
         );
-        has_ext
+        true
     }
 
     async fn resolve(

crates/openhuman-core/src/agent/tinyagents/middleware/tool_policy.rs

@@ -50,6 +50,23 @@ struct SessionToolPolicy {
     agent_definition_id: String,
 }
 
+/// The borrowed session-scoped view every "can this session call this tool?"
+/// answer is rendered from: the channel-permission snapshot, the shared tool
+/// sets a call's `Tool` is resolved from, and the channel name.
+///
+/// [`ToolPolicyMiddleware`] builds one per call. [`crate::agent::tinyagents::middleware::ApprovalSecurityMiddleware`]
+/// builds the same view in its `requires_approval` pre-check (issue #7649):
+/// the approval wrap sits OUTSIDE this middleware, so the pre-check is the
+/// last chance to keep an approval card from firing for a call the
+/// permission layer will refuse regardless of the answer. Hosting the logic
+/// here — rather than copying the predicates — is what keeps the two gates
+/// from ever disagreeing ("two sources of truth ... is the bug").
+pub(crate) struct SessionPermissionView<'a> {
+    pub(crate) session: &'a crate::tools::agent_policy::ToolPolicySession,
+    pub(crate) tool_sets: &'a [Arc<Vec<Box<dyn Tool>>>],
+    pub(crate) channel: &'a str,
+}
+
 #[async_trait]
 impl ToolCallPolicy<OpenHumanRunContext> for SessionToolPolicy {
     fn name(&self) -> &str {
@@ -97,11 +114,116 @@ impl ToolPolicyMiddleware {
         }
     }
 
+    /// The borrowed session view this middleware renders its permission
+    /// answers from.
+    pub(crate) fn view(&self) -> SessionPermissionView<'_> {
+        SessionPermissionView {
+            session: &self.session,
+            tool_sets: &self.tool_sets,
+            channel: &self.channel,
+        }
+    }
+
     fn resolve_tool(&self, name: &str) -> Option<&Box<dyn Tool>> {
-        self.tool_sets
-            .iter()
-            .flat_map(|set| set.iter())
-            .find(|t| t.name() == name)
+        resolve_tool_in(&self.tool_sets, name)
+    }
+
+    /// The delegation tools this session can actually call that reach one of
+    /// `owners`, as tool names. See [`SessionPermissionView::callable_delegates_for`].
+    fn callable_delegates_for(&self, owners: &[&str]) -> Vec<String> {
+        self.view().callable_delegates_for(owners)
+    }
+
+    /// The answer to a `use_skill` call naming a tool `pack` does not contain:
+    /// the tools in it this session can call, or the pack's route when none.
+    pub(crate) fn no_such_pack_tool<'a>(
+        &self,
+        pack: &'static tinyagents_harness::tool::packs::ToolPack,
+        tool: &'a str,
+    ) -> tinyagents_harness::tool::packs::NoSuchPackTool<'a> {
+        self.view().no_such_pack_tool(pack, tool)
+    }
+
+    /// The route sentence for a pack, resolved against THIS session.
+    pub(crate) fn route_for_pack(
+        &self,
+        pack: &tinyagents_harness::tool::packs::ToolPack,
+    ) -> String {
+        self.view().route_for_pack(pack)
+    }
+
+    /// Render a `use_skill` listing scoped to what this session may call.
+    ///
+    /// This lives in the middleware because the middleware is the only layer
+    /// that holds the session — `UseSkillTool` is built once per registry and
+    /// has no idea who is calling it. Only the disclosure half is rendered here:
+    /// a call that names a `tool` is the execution half, which
+    /// `channel_permission_block` has already gated and the tool's own
+    /// `execute` dispatches. Returns `None` when there is nothing to scope (a
+    /// tool named, no `skill` argument, no pack handle), so the call falls
+    /// through to the tool's own `execute` unchanged.
+    pub(crate) fn render_skill_for_session(&self, call: &TaToolCall) -> Option<TaToolResult> {
+        if tinyagents_harness::tool::packs::named_tool(&call.arguments).is_some() {
+            return None;
+        }
+        let skill = call
+            .arguments
+            .get("skill")
+            .and_then(serde_json::Value::as_str)?;
+        let tool = self.resolve_tool(&call.name)?;
+        let handle = crate::tools::host_extensions::pack_registry_handle(tool.as_ref())?;
+        let is_callable = |name: &str| !self.session.decision_for(name).blocks_execution();
+        let route = crate::tools::toolpacks::pack(skill)
+            .map(|pack| self.route_for_pack(pack))
+            .unwrap_or_default();
+        let rendered = tinyagents_harness::tool::packs::render_pack_filtered(
+            &crate::tools::toolpacks::CATALOG,
+            skill,
+            handle,
+            // The same predicate the gate applies to `use_skill`'s inner tool.
+            // Two sources of truth for "can this session call it" is the bug.
+            &is_callable,
+            &route,
+        );
+        Some(match rendered {
+            Ok(text) => TaToolResult::success(text),
+            Err(message) => TaToolResult::error(message),
+        })
+    }
+
+    /// The channel-permission gate the engine ran before the builder policy: a
+    /// session-level deny, then a per-call permission-level ceiling check.
+    /// Returns the blocking message when the call must not execute. The
+    /// predicates live on [`SessionPermissionView`] so the approval
+    /// middleware's pre-check (issue #7649) consults the identical logic.
+    pub(crate) fn channel_permission_block(
+        &self,
+        call: &TaToolCall,
+        desktop_approval_disabled: bool,
+    ) -> Option<String> {
+        self.view()
+            .channel_permission_block(call, desktop_approval_disabled)
+    }
+}
+
+/// Finds the registered [`Tool`] named `name` across `tool_sets`, if any.
+/// Free-standing so both [`ToolPolicyMiddleware`] and the borrowed
+/// [`SessionPermissionView`] return references into the caller's tool sets
+/// rather than through a moved view value.
+fn resolve_tool_in<'a>(
+    tool_sets: &'a [Arc<Vec<Box<dyn Tool>>>],
+    name: &str,
+) -> Option<&'a Box<dyn Tool>> {
+    tool_sets
+        .iter()
+        .flat_map(|set| set.iter())
+        .find(|t| t.name() == name)
+}
+
+impl SessionPermissionView<'_> {
+    /// Finds the registered [`Tool`] named `name`, if any.
+    fn resolve_tool(&self, name: &str) -> Option<&Box<dyn Tool>> {
+        resolve_tool_in(self.tool_sets, name)
     }
 
     /// The delegation tools this session can actually call that reach one of
@@ -191,48 +313,9 @@ impl ToolPolicyMiddleware {
         )
     }
 
-    /// Render a `use_skill` listing scoped to what this session may call.
-    ///
-    /// This lives in the middleware because the middleware is the only layer
-    /// that holds the session — `UseSkillTool` is built once per registry and
-    /// has no idea who is calling it. Only the disclosure half is rendered here:
-    /// a call that names a `tool` is the execution half, which
-    /// `channel_permission_block` has already gated and the tool's own
-    /// `execute` dispatches. Returns `None` when there is nothing to scope (a
-    /// tool named, no `skill` argument, no pack handle), so the call falls
-    /// through to the tool's own `execute` unchanged.
-    pub(crate) fn render_skill_for_session(&self, call: &TaToolCall) -> Option<TaToolResult> {
-        if tinyagents_harness::tool::packs::named_tool(&call.arguments).is_some() {
-            return None;
-        }
-        let skill = call
-            .arguments
-            .get("skill")
-            .and_then(serde_json::Value::as_str)?;
-        let tool = self.resolve_tool(&call.name)?;
-        let handle = crate::tools::host_extensions::pack_registry_handle(tool.as_ref())?;
-        let is_callable = |name: &str| !self.session.decision_for(name).blocks_execution();
-        let route = crate::tools::toolpacks::pack(skill)
-            .map(|pack| self.route_for_pack(pack))
-            .unwrap_or_default();
-        let rendered = tinyagents_harness::tool::packs::render_pack_filtered(
-            &crate::tools::toolpacks::CATALOG,
-            skill,
-            handle,
-            // The same predicate the gate applies to `use_skill`'s inner tool.
-            // Two sources of truth for "can this session call it" is the bug.
-            &is_callable,
-            &route,
-        );
-        Some(match rendered {
-            Ok(text) => TaToolResult::success(text),
-            Err(message) => TaToolResult::error(message),
-        })
-    }
-
     /// The channel-permission gate the engine ran before the builder policy: a
-    /// session-level deny, then a per-call permission-level ceiling check. Returns
-    /// the blocking message when the call must not execute.
+    /// session-level deny, then a per-call permission-level ceiling check.
+    /// Returns the blocking message when the call must not execute.
     pub(crate) fn channel_permission_block(
         &self,
         call: &TaToolCall,
@@ -250,7 +333,7 @@ impl ToolPolicyMiddleware {
                     tool: &call.name,
                     required: decision.required_permission,
                     allowed: decision.allowed_permission,
-                    channel: &self.channel,
+                    channel: self.channel,
                 }
                 .render(),
             );
@@ -327,7 +410,7 @@ impl ToolPolicyMiddleware {
                     tool: &call.name,
                     required: call_required,
                     allowed: decision.allowed_permission,
-                    channel: &self.channel,
+                    channel: self.channel,
                 }
                 .render(),
             );