zeph_subagent/

filter.rs

// SPDX-FileCopyrightText: 2026 Andrei G <bug-ops>
// SPDX-License-Identifier: MIT OR Apache-2.0

//! Tool and skill filtering for sub-agents.
//!
//! [`FilteredToolExecutor`] wraps any [`ErasedToolExecutor`] and enforces a [`ToolPolicy`]
//! plus an optional extra denylist on every tool invocation.
//!
//! [`PlanModeExecutor`] wraps any executor to allow catalog inspection while blocking all
//! execution — implementing the read-only planning permission mode.
//!
//! [`filter_skills`] applies glob-based include/exclude patterns against a skill registry.

use std::collections::HashMap;
use std::pin::Pin;
use std::sync::Arc;

use zeph_skills::loader::Skill;
use zeph_skills::registry::SkillRegistry;
use zeph_tools::ToolCall;
use zeph_tools::executor::{ErasedToolExecutor, ToolError, ToolOutput, extract_fenced_blocks};
use zeph_tools::registry::{InvocationHint, ToolDef};

use super::def::{SkillFilter, ToolPolicy};
use super::error::SubAgentError;

// ── Helpers ───────────────────────────────────────────────────────────────────

/// Collect all fenced-block language tags from an executor's tool definitions.
fn collect_fenced_tags(executor: &dyn ErasedToolExecutor) -> Vec<&'static str> {
    executor
        .tool_definitions_erased()
        .into_iter()
        .filter_map(|def| match def.invocation {
            InvocationHint::FencedBlock(tag) => Some(tag),
            _ => None,
        })
        .collect()
}

// ── Tool ID normalization ─────────────────────────────────────────────────────

/// Normalize a tool ID for policy matching: lowercase, strip everything from the first `(` onward.
///
/// Examples: `"Read"` → `"read"`, `"Bash(cargo *)"` → `"bash"`, `"bash"` → `"bash"`.
pub(crate) fn normalize_tool_id(s: &str) -> String {
    let base = s.split('(').next().unwrap_or(s);
    base.trim().to_lowercase()
}

// ── Tool filtering ────────────────────────────────────────────────────────────

/// Wraps an [`ErasedToolExecutor`] and enforces a [`ToolPolicy`] plus an optional
/// additional denylist (`disallowed`).
///
/// All calls are checked against the policy and the denylist before being forwarded
/// to the inner executor. The denylist is evaluated first — a tool in `disallowed`
/// is blocked even if `policy` would allow it (deny wins). Rejected calls return a
/// descriptive [`ToolError`].
pub struct FilteredToolExecutor {
    inner: Arc<dyn ErasedToolExecutor>,
    policy: ToolPolicy,
    disallowed: Vec<String>,
    /// Fenced-block language tags collected from `inner` at construction time.
    /// Used to detect actual fenced-block tool invocations in LLM responses.
    fenced_tags: Vec<&'static str>,
}

impl FilteredToolExecutor {
    /// Create a new filtered executor with the given policy and no additional denylist.
    ///
    /// Use [`with_disallowed`][Self::with_disallowed] when the agent definition also
    /// specifies `tools.except` entries.
    #[must_use]
    pub fn new(inner: Arc<dyn ErasedToolExecutor>, policy: ToolPolicy) -> Self {
        let fenced_tags = collect_fenced_tags(&*inner);
        Self {
            inner,
            policy,
            disallowed: Vec::new(),
            fenced_tags,
        }
    }

    /// Create a new filtered executor with an additional denylist.
    ///
    /// Tools in `disallowed` are blocked regardless of the base `policy`
    /// (deny wins over allow).
    #[must_use]
    pub fn with_disallowed(
        inner: Arc<dyn ErasedToolExecutor>,
        policy: ToolPolicy,
        disallowed: Vec<String>,
    ) -> Self {
        let fenced_tags = collect_fenced_tags(&*inner);
        Self {
            inner,
            policy,
            disallowed,
            fenced_tags,
        }
    }

    /// Return `true` if `response` contains at least one fenced block matching a registered tool.
    fn has_fenced_tool_invocation(&self, response: &str) -> bool {
        self.fenced_tags
            .iter()
            .any(|tag| !extract_fenced_blocks(response, tag).is_empty())
    }

    /// Check whether `tool_id` is allowed under the current policy and denylist.
    ///
    /// Matching is case-insensitive and strips argument suffixes (e.g. `"Bash(cargo *)"` matches
    /// runtime ID `"bash"`). MCP compound tool IDs (`mcp__server__tool`) must still be listed in
    /// full in `tools.except` — partial names or prefixes are not matched.
    fn is_allowed(&self, tool_id: &str) -> bool {
        let normalized = normalize_tool_id(tool_id);
        if self
            .disallowed
            .iter()
            .any(|t| normalize_tool_id(t) == normalized)
        {
            return false;
        }
        match &self.policy {
            ToolPolicy::AllowList(list) => list.iter().any(|t| normalize_tool_id(t) == normalized),
            ToolPolicy::DenyList(list) => !list.iter().any(|t| normalize_tool_id(t) == normalized),
            _ => true,
        }
    }
}

impl ErasedToolExecutor for FilteredToolExecutor {
    fn execute_erased<'a>(
        &'a self,
        response: &'a str,
    ) -> Pin<Box<dyn std::future::Future<Output = Result<Option<ToolOutput>, ToolError>> + Send + 'a>>
    {
        // Sub-agents must use structured tool calls (execute_tool_call_erased).
        // Fenced-block execution is disabled to prevent policy bypass (SEC-03).
        //
        // However, this method is also called for plain-text LLM responses that
        // contain markdown code fences unrelated to tool invocations. Returning
        // Err unconditionally causes the agent loop to treat every text response
        // as a failed tool call and exhaust all turns without producing output.
        //
        // Only block when the response actually contains a fenced block that
        // matches a registered fenced-block tool language tag.
        if self.has_fenced_tool_invocation(response) {
            tracing::warn!("sub-agent attempted fenced-block tool invocation — blocked by policy");
            return Box::pin(std::future::ready(Err(ToolError::Blocked {
                command: "fenced-block".into(),
            })));
        }
        Box::pin(std::future::ready(Ok(None)))
    }

    fn execute_confirmed_erased<'a>(
        &'a self,
        response: &'a str,
    ) -> Pin<Box<dyn std::future::Future<Output = Result<Option<ToolOutput>, ToolError>> + Send + 'a>>
    {
        // Same policy as execute_erased: only block actual fenced-block invocations.
        if self.has_fenced_tool_invocation(response) {
            tracing::warn!(
                "sub-agent attempted confirmed fenced-block tool invocation — blocked by policy"
            );
            return Box::pin(std::future::ready(Err(ToolError::Blocked {
                command: "fenced-block".into(),
            })));
        }
        Box::pin(std::future::ready(Ok(None)))
    }

    fn tool_definitions_erased(&self) -> Vec<ToolDef> {
        // Filter the visible tool definitions according to the policy.
        self.inner
            .tool_definitions_erased()
            .into_iter()
            .filter(|def| self.is_allowed(&def.id))
            .collect()
    }

    fn execute_tool_call_erased<'a>(
        &'a self,
        call: &'a ToolCall,
    ) -> Pin<Box<dyn std::future::Future<Output = Result<Option<ToolOutput>, ToolError>> + Send + 'a>>
    {
        if !self.is_allowed(call.tool_id.as_str()) {
            tracing::warn!(
                tool_id = %call.tool_id,
                "sub-agent tool call rejected by policy"
            );
            return Box::pin(std::future::ready(Err(ToolError::Blocked {
                command: call.tool_id.to_string(),
            })));
        }
        Box::pin(self.inner.execute_tool_call_erased(call))
    }

    /// Same policy as `execute_tool_call_erased`: the confirmed path must go through the
    /// same allow/deny check, not bypass it. Mirrors the removed trait default's behavior
    /// (delegate to `execute_tool_call_erased`) while preserving the policy enforcement
    /// already present there.
    fn execute_tool_call_confirmed_erased<'a>(
        &'a self,
        call: &'a ToolCall,
    ) -> Pin<Box<dyn std::future::Future<Output = Result<Option<ToolOutput>, ToolError>> + Send + 'a>>
    {
        self.execute_tool_call_erased(call)
    }

    fn set_skill_env(&self, env: Option<HashMap<String, String>>) {
        self.inner.set_skill_env(env);
    }

    fn set_effective_trust(&self, level: zeph_tools::SkillTrustLevel) {
        self.inner.set_effective_trust(level);
    }

    fn is_tool_retryable_erased(&self, tool_id: &str) -> bool {
        self.inner.is_tool_retryable_erased(tool_id)
    }

    fn requires_confirmation_erased(&self, call: &ToolCall) -> bool {
        self.inner.requires_confirmation_erased(call)
    }

    zeph_tools::erased_tool_executor_forward!(inner);
}

// ── Plan mode executor ────────────────────────────────────────────────────────

/// Wraps an [`ErasedToolExecutor`] for `Plan` permission mode.
///
/// Exposes the real tool catalog via `tool_definitions_erased()` so the LLM can
/// reference existing tools in its plan, but blocks all execution methods with
/// [`ToolError::Blocked`]. This implements read-only planning: the agent sees what
/// tools exist but cannot invoke them.
pub struct PlanModeExecutor {
    inner: Arc<dyn ErasedToolExecutor>,
}

impl PlanModeExecutor {
    /// Wrap `inner` with plan-mode restrictions.
    #[must_use]
    pub fn new(inner: Arc<dyn ErasedToolExecutor>) -> Self {
        Self { inner }
    }
}

impl ErasedToolExecutor for PlanModeExecutor {
    fn execute_erased<'a>(
        &'a self,
        _response: &'a str,
    ) -> Pin<Box<dyn std::future::Future<Output = Result<Option<ToolOutput>, ToolError>> + Send + 'a>>
    {
        Box::pin(std::future::ready(Err(ToolError::Blocked {
            command: "plan_mode".into(),
        })))
    }

    fn execute_confirmed_erased<'a>(
        &'a self,
        _response: &'a str,
    ) -> Pin<Box<dyn std::future::Future<Output = Result<Option<ToolOutput>, ToolError>> + Send + 'a>>
    {
        Box::pin(std::future::ready(Err(ToolError::Blocked {
            command: "plan_mode".into(),
        })))
    }

    fn tool_definitions_erased(&self) -> Vec<ToolDef> {
        self.inner.tool_definitions_erased()
    }

    fn execute_tool_call_erased<'a>(
        &'a self,
        call: &'a ToolCall,
    ) -> Pin<Box<dyn std::future::Future<Output = Result<Option<ToolOutput>, ToolError>> + Send + 'a>>
    {
        tracing::debug!(
            tool_id = %call.tool_id,
            "tool execution blocked in plan mode"
        );
        Box::pin(std::future::ready(Err(ToolError::Blocked {
            command: call.tool_id.to_string(),
        })))
    }

    /// Plan mode blocks all execution, confirmed or not — reuse the same block as the
    /// unconfirmed path rather than forwarding to `inner`.
    fn execute_tool_call_confirmed_erased<'a>(
        &'a self,
        call: &'a ToolCall,
    ) -> Pin<Box<dyn std::future::Future<Output = Result<Option<ToolOutput>, ToolError>> + Send + 'a>>
    {
        self.execute_tool_call_erased(call)
    }

    fn set_skill_env(&self, env: Option<std::collections::HashMap<String, String>>) {
        self.inner.set_skill_env(env);
    }

    /// Read-only in plan mode: trust level is metadata, not an execution capability,
    /// so it is safe (and necessary for parity with other read paths) to forward.
    fn set_effective_trust(&self, level: zeph_tools::SkillTrustLevel) {
        self.inner.set_effective_trust(level);
    }

    fn is_tool_retryable_erased(&self, _tool_id: &str) -> bool {
        false
    }

    fn requires_confirmation_erased(&self, _call: &ToolCall) -> bool {
        false
    }

    // Deliberate: this forwards the checkpoint trio to `inner` rather than reporting
    // "unsupported" (the pre-#6019 behavior). Plan mode blocks new tool *execution*
    // (`execute_tool_call_erased` above always returns `Blocked`); checkpoint undo/redo/list
    // act on already-executed side effects and are administrative/read operations, not new
    // execution, so exposing them while plan mode is active is in scope.
    zeph_tools::erased_tool_executor_forward!(inner);
}

// ── Network egress denial ─────────────────────────────────────────────────────

/// Tool IDs that are pure network-egress tools — any call is blocked outright, no command
/// inspection needed (unlike `bash`, which is dual-use).
///
/// `web_scrape`/`fetch` are the native `WebScrapeExecutor` tool IDs (see `zeph-tools`
/// `scrape.rs`) — the tool an LLM reaches for by default to retrieve a URL, and therefore
/// the highest-likelihood egress vector for a `Deny`-scoped task (#6030 critic finding S2).
const NETWORK_ONLY_TOOL_IDS: &[&str] = &["web_scrape", "fetch"];

/// Blocks network-egress tool calls for a single sub-agent spawn.
///
/// Wraps an [`ErasedToolExecutor`] and rejects two classes of call with
/// [`ToolError::Blocked`]:
/// - Any call to a network-only tool (`web_scrape`, `fetch`) — blocked unconditionally,
///   since these tools have no non-network purpose.
/// - `bash` tool calls whose command matches [`zeph_tools::NETWORK_COMMANDS`] (`curl`,
///   `wget`, `nc`, `ncat`, `netcat`).
///
/// All other tool calls pass through unchanged. **Known gap**: MCP-provided tools (which may
/// perform their own HTTP egress) are not inspected — see `specs/069-threat-model/spec.md`
/// INVARIANT-5. This is a best-effort, tool/command-identity block, not a sandbox boundary.
///
/// Installed by `build_filtered_executor` (`crate::manager::spawn`) when the spawning
/// task carries `NetworkScope::Deny` (spec `069-threat-model` OQ-1). Unlike mutating
/// [`ShellConfig`](zeph_tools::ShellConfig)'s `allow_network` field directly, this
/// wrapper scopes the restriction to a single spawn without affecting the shared
/// `tool_executor` used by the parent agent and sibling tasks.
pub struct NetworkDenyToolExecutor {
    inner: Arc<dyn ErasedToolExecutor>,
    blocklist: Vec<String>,
}

impl NetworkDenyToolExecutor {
    /// Wrap `inner`, blocking network-egress tool calls for every call.
    #[must_use]
    pub fn new(inner: Arc<dyn ErasedToolExecutor>) -> Self {
        Self {
            inner,
            blocklist: zeph_tools::NETWORK_COMMANDS
                .iter()
                .map(|s| (*s).to_owned())
                .collect(),
        }
    }

    /// Returns `Err` if `call` targets a network-only tool (`web_scrape`, `fetch`) or is a
    /// `bash` invocation whose command matches the network-command blocklist; `Ok(())`
    /// otherwise.
    fn check_call(&self, call: &ToolCall) -> Result<(), ToolError> {
        let tool_id = normalize_tool_id(call.tool_id.as_str());

        if NETWORK_ONLY_TOOL_IDS.contains(&tool_id.as_str()) {
            tracing::warn!(
                tool_id = %tool_id,
                "network egress denied for sub-agent task (NetworkScope::Deny)"
            );
            return Err(ToolError::Blocked { command: tool_id });
        }

        if tool_id != "bash" {
            return Ok(());
        }
        let Some(command) = call.params.get("command").and_then(|v| v.as_str()) else {
            return Ok(());
        };
        if let Some(matched) = zeph_tools::check_blocklist(command, &self.blocklist) {
            tracing::warn!(
                command = %matched,
                "network egress denied for sub-agent task (NetworkScope::Deny)"
            );
            return Err(ToolError::Blocked { command: matched });
        }
        Ok(())
    }
}

impl ErasedToolExecutor for NetworkDenyToolExecutor {
    fn execute_erased<'a>(
        &'a self,
        response: &'a str,
    ) -> Pin<Box<dyn std::future::Future<Output = Result<Option<ToolOutput>, ToolError>> + Send + 'a>>
    {
        self.inner.execute_erased(response)
    }

    fn execute_confirmed_erased<'a>(
        &'a self,
        response: &'a str,
    ) -> Pin<Box<dyn std::future::Future<Output = Result<Option<ToolOutput>, ToolError>> + Send + 'a>>
    {
        self.inner.execute_confirmed_erased(response)
    }

    fn tool_definitions_erased(&self) -> Vec<ToolDef> {
        self.inner.tool_definitions_erased()
    }

    fn execute_tool_call_erased<'a>(
        &'a self,
        call: &'a ToolCall,
    ) -> Pin<Box<dyn std::future::Future<Output = Result<Option<ToolOutput>, ToolError>> + Send + 'a>>
    {
        if let Err(e) = self.check_call(call) {
            return Box::pin(std::future::ready(Err(e)));
        }
        Box::pin(self.inner.execute_tool_call_erased(call))
    }

    fn execute_tool_call_confirmed_erased<'a>(
        &'a self,
        call: &'a ToolCall,
    ) -> Pin<Box<dyn std::future::Future<Output = Result<Option<ToolOutput>, ToolError>> + Send + 'a>>
    {
        if let Err(e) = self.check_call(call) {
            return Box::pin(std::future::ready(Err(e)));
        }
        Box::pin(self.inner.execute_tool_call_confirmed_erased(call))
    }

    fn set_skill_env(&self, env: Option<HashMap<String, String>>) {
        self.inner.set_skill_env(env);
    }

    fn set_effective_trust(&self, level: zeph_tools::SkillTrustLevel) {
        self.inner.set_effective_trust(level);
    }

    fn is_tool_retryable_erased(&self, tool_id: &str) -> bool {
        self.inner.is_tool_retryable_erased(tool_id)
    }

    fn requires_confirmation_erased(&self, call: &ToolCall) -> bool {
        self.inner.requires_confirmation_erased(call)
    }

    zeph_tools::erased_tool_executor_forward!(inner);
}

// ── Skill filtering ───────────────────────────────────────────────────────────

/// Filter skills from a registry according to a [`SkillFilter`].
///
/// Include patterns are glob-matched against skill names. If `include` is empty,
/// all skills pass (unless excluded). Exclude patterns always take precedence.
///
/// Supported glob syntax:
/// - `*` — wildcard matching any substring (e.g., `"git-*"`)
/// - Literal strings — exact match only
/// - `**` is **not** supported and returns [`SubAgentError::Invalid`]
///
/// # Errors
///
/// Returns [`SubAgentError::Invalid`] if any glob pattern is syntactically invalid.
///
/// # Examples
///
/// ```rust,no_run
/// use zeph_skills::registry::SkillRegistry;
/// use zeph_subagent::filter_skills;
/// use zeph_subagent::SkillFilter;
///
/// let registry = SkillRegistry::load(&[] as &[&str]);
/// let filter = SkillFilter { include: vec![], exclude: vec![] };
/// let skills = filter_skills(&registry, &filter).unwrap();
/// assert!(skills.is_empty());
/// ```
pub fn filter_skills(
    registry: &SkillRegistry,
    filter: &SkillFilter,
) -> Result<Vec<Skill>, SubAgentError> {
    let compiled_include = compile_globs(&filter.include)?;
    let compiled_exclude = compile_globs(&filter.exclude)?;

    let all: Vec<Skill> = registry
        .all_meta()
        .into_iter()
        .filter(|meta| {
            let name = &meta.name;
            let included =
                compiled_include.is_empty() || compiled_include.iter().any(|p| glob_match(p, name));
            let excluded = compiled_exclude.iter().any(|p| glob_match(p, name));
            included && !excluded
        })
        .filter_map(|meta| registry.skill(&meta.name).ok())
        .collect();

    Ok(all)
}

/// Compiled glob pattern: literal prefix + optional `*` wildcard suffix.
struct GlobPattern {
    raw: String,
    prefix: String,
    suffix: Option<String>,
    is_star: bool,
}

fn compile_globs(patterns: &[String]) -> Result<Vec<GlobPattern>, SubAgentError> {
    patterns.iter().map(|p| compile_glob(p)).collect()
}

fn compile_glob(pattern: &str) -> Result<GlobPattern, SubAgentError> {
    // Simple glob: supports `*` as a wildcard anywhere in the string.
    // For MVP we only need prefix-star patterns like "git-*" or "*".
    if pattern.contains("**") {
        return Err(SubAgentError::Invalid(format!(
            "glob pattern '{pattern}' uses '**' which is not supported"
        )));
    }

    let is_star = pattern == "*";

    let (prefix, suffix) = if let Some(pos) = pattern.find('*') {
        let before = pattern[..pos].to_owned();
        let after = pattern[pos + 1..].to_owned();
        (before, Some(after))
    } else {
        (pattern.to_owned(), None)
    };

    Ok(GlobPattern {
        raw: pattern.to_owned(),
        prefix,
        suffix,
        is_star,
    })
}

fn glob_match(pattern: &GlobPattern, name: &str) -> bool {
    if pattern.is_star {
        return true;
    }

    match &pattern.suffix {
        None => name == pattern.raw,
        Some(suf) => {
            name.starts_with(&pattern.prefix) && name.ends_with(suf.as_str()) && {
                // Ensure the wildcard section isn't negative-length.
                name.len() >= pattern.prefix.len() + suf.len()
            }
        }
    }
}

// ── Tests ─────────────────────────────────────────────────────────────────────

#[cfg(test)]
mod tests {
    #![allow(clippy::default_trait_access)]
    use std::assert_matches;

    use super::*;
    use crate::def::ToolPolicy;

    // ── FilteredToolExecutor tests ─────────────────────────────────────────

    struct StubExecutor {
        tools: Vec<&'static str>,
    }

    /// Stub executor that exposes tools with `InvocationHint::FencedBlock(tag)`.
    struct StubFencedExecutor {
        tag: &'static str,
    }

    impl ErasedToolExecutor for StubFencedExecutor {
        fn execute_erased<'a>(
            &'a self,
            _response: &'a str,
        ) -> Pin<
            Box<
                dyn std::future::Future<Output = Result<Option<ToolOutput>, ToolError>> + Send + 'a,
            >,
        > {
            Box::pin(std::future::ready(Ok(None)))
        }

        fn execute_confirmed_erased<'a>(
            &'a self,
            _response: &'a str,
        ) -> Pin<
            Box<
                dyn std::future::Future<Output = Result<Option<ToolOutput>, ToolError>> + Send + 'a,
            >,
        > {
            Box::pin(std::future::ready(Ok(None)))
        }

        fn tool_definitions_erased(&self) -> Vec<ToolDef> {
            use zeph_tools::registry::InvocationHint;
            vec![ToolDef {
                id: self.tag.into(),
                description: "fenced stub".into(),
                schema: schemars::Schema::default(),
                invocation: InvocationHint::FencedBlock(self.tag),
                output_schema: None,
                server_id: None,
            }]
        }

        fn execute_tool_call_erased<'a>(
            &'a self,
            call: &'a ToolCall,
        ) -> Pin<
            Box<
                dyn std::future::Future<Output = Result<Option<ToolOutput>, ToolError>> + Send + 'a,
            >,
        > {
            let result = Ok(Some(ToolOutput {
                tool_name: call.tool_id.clone(),
                summary: "ok".into(),
                blocks_executed: 1,
                filter_stats: None,
                diff: None,
                streamed: false,
                terminal_id: None,
                locations: None,
                raw_response: None,
                claim_source: None,
                ..Default::default()
            }));
            Box::pin(std::future::ready(result))
        }

        fn is_tool_retryable_erased(&self, _tool_id: &str) -> bool {
            false
        }

        fn requires_confirmation_erased(&self, _call: &ToolCall) -> bool {
            false
        }

        fn execute_tool_call_confirmed_erased<'a>(
            &'a self,
            call: &'a ToolCall,
        ) -> Pin<
            Box<
                dyn std::future::Future<Output = Result<Option<ToolOutput>, ToolError>> + Send + 'a,
            >,
        > {
            self.execute_tool_call_erased(call)
        }

        fn checkpoint_undo_erased(&self, _n: usize) -> zeph_tools::CheckpointActionResult {
            zeph_tools::CheckpointActionResult::unsupported()
        }

        fn checkpoint_redo_erased(&self) -> zeph_tools::CheckpointActionResult {
            zeph_tools::CheckpointActionResult::unsupported()
        }

        fn checkpoint_list_erased(&self) -> zeph_tools::CheckpointListResult {
            zeph_tools::CheckpointListResult::default()
        }

        fn is_tool_speculatable_erased(&self, _tool_id: &str) -> bool {
            false
        }
    }

    fn fenced_stub_box(tag: &'static str) -> Arc<dyn ErasedToolExecutor> {
        Arc::new(StubFencedExecutor { tag })
    }

    impl ErasedToolExecutor for StubExecutor {
        fn execute_erased<'a>(
            &'a self,
            _response: &'a str,
        ) -> Pin<
            Box<
                dyn std::future::Future<Output = Result<Option<ToolOutput>, ToolError>> + Send + 'a,
            >,
        > {
            Box::pin(std::future::ready(Ok(None)))
        }

        fn execute_confirmed_erased<'a>(
            &'a self,
            _response: &'a str,
        ) -> Pin<
            Box<
                dyn std::future::Future<Output = Result<Option<ToolOutput>, ToolError>> + Send + 'a,
            >,
        > {
            Box::pin(std::future::ready(Ok(None)))
        }

        fn tool_definitions_erased(&self) -> Vec<ToolDef> {
            // Return stub definitions for each tool name.
            use zeph_tools::registry::InvocationHint;
            self.tools
                .iter()
                .map(|id| ToolDef {
                    id: (*id).into(),
                    description: "stub".into(),
                    schema: schemars::Schema::default(),
                    invocation: InvocationHint::ToolCall,
                    output_schema: None,
                    server_id: None,
                })
                .collect()
        }

        fn execute_tool_call_erased<'a>(
            &'a self,
            call: &'a ToolCall,
        ) -> Pin<
            Box<
                dyn std::future::Future<Output = Result<Option<ToolOutput>, ToolError>> + Send + 'a,
            >,
        > {
            let result = Ok(Some(ToolOutput {
                tool_name: call.tool_id.clone(),
                summary: "ok".into(),
                blocks_executed: 1,
                filter_stats: None,
                diff: None,
                streamed: false,
                terminal_id: None,
                locations: None,
                raw_response: None,
                claim_source: None,
                ..Default::default()
            }));
            Box::pin(std::future::ready(result))
        }

        fn is_tool_retryable_erased(&self, _tool_id: &str) -> bool {
            false
        }

        fn requires_confirmation_erased(&self, _call: &ToolCall) -> bool {
            false
        }

        fn execute_tool_call_confirmed_erased<'a>(
            &'a self,
            call: &'a ToolCall,
        ) -> Pin<
            Box<
                dyn std::future::Future<Output = Result<Option<ToolOutput>, ToolError>> + Send + 'a,
            >,
        > {
            self.execute_tool_call_erased(call)
        }

        fn checkpoint_undo_erased(&self, _n: usize) -> zeph_tools::CheckpointActionResult {
            zeph_tools::CheckpointActionResult::unsupported()
        }

        fn checkpoint_redo_erased(&self) -> zeph_tools::CheckpointActionResult {
            zeph_tools::CheckpointActionResult::unsupported()
        }

        fn checkpoint_list_erased(&self) -> zeph_tools::CheckpointListResult {
            zeph_tools::CheckpointListResult::default()
        }

        fn is_tool_speculatable_erased(&self, _tool_id: &str) -> bool {
            false
        }
    }

    fn stub_box(tools: &[&'static str]) -> Arc<dyn ErasedToolExecutor> {
        Arc::new(StubExecutor {
            tools: tools.to_vec(),
        })
    }

    #[tokio::test]
    async fn allow_list_permits_listed_tool() {
        let exec = FilteredToolExecutor::new(
            stub_box(&["shell", "web"]),
            ToolPolicy::AllowList(vec!["shell".into()]),
        );
        let call = ToolCall {
            tool_id: "shell".into(),
            params: serde_json::Map::default(),
            caller_id: None,
            context: None,

            tool_call_id: String::new(),
            skill_name: None,
        };
        let res = exec.execute_tool_call_erased(&call).await.unwrap();
        assert!(res.is_some());
    }

    #[tokio::test]
    async fn allow_list_blocks_unlisted_tool() {
        let exec = FilteredToolExecutor::new(
            stub_box(&["shell", "web"]),
            ToolPolicy::AllowList(vec!["shell".into()]),
        );
        let call = ToolCall {
            tool_id: "web".into(),
            params: serde_json::Map::default(),
            caller_id: None,
            context: None,

            tool_call_id: String::new(),
            skill_name: None,
        };
        let res = exec.execute_tool_call_erased(&call).await;
        assert!(res.is_err());
    }

    #[tokio::test]
    async fn deny_list_blocks_listed_tool() {
        let exec = FilteredToolExecutor::new(
            stub_box(&["shell", "web"]),
            ToolPolicy::DenyList(vec!["shell".into()]),
        );
        let call = ToolCall {
            tool_id: "shell".into(),
            params: serde_json::Map::default(),
            caller_id: None,
            context: None,

            tool_call_id: String::new(),
            skill_name: None,
        };
        let res = exec.execute_tool_call_erased(&call).await;
        assert!(res.is_err());
    }

    #[tokio::test]
    async fn inherit_all_permits_any_tool() {
        let exec = FilteredToolExecutor::new(stub_box(&["shell"]), ToolPolicy::InheritAll);
        let call = ToolCall {
            tool_id: "shell".into(),
            params: serde_json::Map::default(),
            caller_id: None,
            context: None,

            tool_call_id: String::new(),
            skill_name: None,
        };
        let res = exec.execute_tool_call_erased(&call).await.unwrap();
        assert!(res.is_some());
    }

    #[test]
    fn tool_definitions_filtered_by_allow_list() {
        let exec = FilteredToolExecutor::new(
            stub_box(&["shell", "web"]),
            ToolPolicy::AllowList(vec!["shell".into()]),
        );
        let defs = exec.tool_definitions_erased();
        assert_eq!(defs.len(), 1);
        assert_eq!(defs[0].id, "shell");
    }

    // ── glob_match tests ───────────────────────────────────────────────────

    fn matches(pattern: &str, name: &str) -> bool {
        let p = compile_glob(pattern).unwrap();
        glob_match(&p, name)
    }

    #[test]
    fn glob_star_matches_all() {
        assert!(matches("*", "anything"));
        assert!(matches("*", ""));
    }

    #[test]
    fn glob_prefix_star() {
        assert!(matches("git-*", "git-commit"));
        assert!(matches("git-*", "git-status"));
        assert!(!matches("git-*", "rust-fmt"));
    }

    #[test]
    fn glob_literal_exact_match() {
        assert!(matches("shell", "shell"));
        assert!(!matches("shell", "shell-extra"));
    }

    #[test]
    fn glob_star_suffix() {
        assert!(matches("*-review", "code-review"));
        assert!(!matches("*-review", "code-reviewer"));
    }

    #[test]
    fn glob_double_star_is_error() {
        assert!(compile_glob("**").is_err());
    }

    #[test]
    fn glob_mid_string_wildcard() {
        // "a*b" — prefix="a", suffix=Some("b")
        assert!(matches("a*b", "axb"));
        assert!(matches("a*b", "aXYZb"));
        assert!(!matches("a*b", "ab-extra"));
        assert!(!matches("a*b", "xab"));
    }

    // ── FilteredToolExecutor additional tests ──────────────────────────────

    #[tokio::test]
    async fn deny_list_permits_unlisted_tool() {
        let exec = FilteredToolExecutor::new(
            stub_box(&["shell", "web"]),
            ToolPolicy::DenyList(vec!["shell".into()]),
        );
        let call = ToolCall {
            tool_id: "web".into(), // not in deny list → allowed
            params: serde_json::Map::default(),
            caller_id: None,
            context: None,

            tool_call_id: String::new(),
            skill_name: None,
        };
        let res = exec.execute_tool_call_erased(&call).await.unwrap();
        assert!(res.is_some());
    }

    #[test]
    fn tool_definitions_filtered_by_deny_list() {
        let exec = FilteredToolExecutor::new(
            stub_box(&["shell", "web"]),
            ToolPolicy::DenyList(vec!["shell".into()]),
        );
        let defs = exec.tool_definitions_erased();
        assert_eq!(defs.len(), 1);
        assert_eq!(defs[0].id, "web");
    }

    #[test]
    fn tool_definitions_inherit_all_returns_all() {
        let exec = FilteredToolExecutor::new(stub_box(&["shell", "web"]), ToolPolicy::InheritAll);
        let defs = exec.tool_definitions_erased();
        assert_eq!(defs.len(), 2);
    }

    // ── fenced-block detection tests (fix for #1432) ──────────────────────

    #[tokio::test]
    async fn fenced_block_matching_tag_is_blocked() {
        // Executor has a FencedBlock("bash") tool; response contains ```bash block.
        let exec = FilteredToolExecutor::new(fenced_stub_box("bash"), ToolPolicy::InheritAll);
        let res = exec.execute_erased("```bash\nls\n```").await;
        assert!(
            res.is_err(),
            "actual fenced-block invocation must be blocked"
        );
    }

    #[tokio::test]
    async fn fenced_block_matching_tag_confirmed_is_blocked() {
        let exec = FilteredToolExecutor::new(fenced_stub_box("bash"), ToolPolicy::InheritAll);
        let res = exec.execute_confirmed_erased("```bash\nls\n```").await;
        assert!(
            res.is_err(),
            "actual fenced-block invocation (confirmed) must be blocked"
        );
    }

    #[tokio::test]
    async fn no_fenced_tools_plain_text_returns_ok_none() {
        // No fenced-block tools registered → plain text must return Ok(None).
        let exec = FilteredToolExecutor::new(stub_box(&["shell"]), ToolPolicy::InheritAll);
        let res = exec.execute_erased("This is a plain text response.").await;
        assert!(
            res.unwrap().is_none(),
            "plain text must not be treated as a tool call"
        );
    }

    #[tokio::test]
    async fn markdown_non_tool_fence_returns_ok_none() {
        // Response has a ```rust fence but no FencedBlock tool with tag "rust" is registered.
        let exec = FilteredToolExecutor::new(fenced_stub_box("bash"), ToolPolicy::InheritAll);
        let res = exec
            .execute_erased("Here is some code:\n```rust\nfn main() {}\n```")
            .await;
        assert!(
            res.unwrap().is_none(),
            "non-tool code fence must not trigger blocking"
        );
    }

    #[tokio::test]
    async fn no_fenced_tools_plain_text_confirmed_returns_ok_none() {
        let exec = FilteredToolExecutor::new(stub_box(&["shell"]), ToolPolicy::InheritAll);
        let res = exec
            .execute_confirmed_erased("Plain response without any fences.")
            .await;
        assert!(res.unwrap().is_none());
    }

    /// Regression test for #1432: fenced executor + plain text (no fences at all) must return
    /// Ok(None) so the agent loop can break. Previously this returned Err(Blocked)
    /// unconditionally, exhausting all sub-agent turns.
    #[tokio::test]
    async fn fenced_executor_plain_text_returns_ok_none() {
        let exec = FilteredToolExecutor::new(fenced_stub_box("bash"), ToolPolicy::InheritAll);
        let res = exec
            .execute_erased("Here is my analysis of the code. No shell commands needed.")
            .await;
        assert!(
            res.unwrap().is_none(),
            "plain text with fenced executor must not be treated as a tool call"
        );
    }

    /// Unclosed fence (no closing ```) must not trigger blocking — it is not an executable
    /// tool invocation. Verified by debugger as an intentional false-negative.
    #[tokio::test]
    async fn unclosed_fenced_block_returns_ok_none() {
        let exec = FilteredToolExecutor::new(fenced_stub_box("bash"), ToolPolicy::InheritAll);
        let res = exec.execute_erased("```bash\nls -la\n").await;
        assert!(
            res.unwrap().is_none(),
            "unclosed fenced block must not be treated as a tool invocation"
        );
    }

    /// Multiple fenced blocks where one matches a registered tag — must block.
    #[tokio::test]
    async fn multiple_fences_one_matching_tag_is_blocked() {
        let exec = FilteredToolExecutor::new(fenced_stub_box("bash"), ToolPolicy::InheritAll);
        let response = "Here is an example:\n```python\nprint('hello')\n```\nAnd the fix:\n```bash\nrm -rf /tmp/old\n```";
        let res = exec.execute_erased(response).await;
        assert!(
            res.is_err(),
            "response containing a matching fenced block must be blocked"
        );
    }

    // ── disallowed_tools (tools.except) tests ─────────────────────────────

    #[tokio::test]
    async fn disallowed_blocks_tool_from_allow_list() {
        let exec = FilteredToolExecutor::with_disallowed(
            stub_box(&["shell", "web"]),
            ToolPolicy::AllowList(vec!["shell".into(), "web".into()]),
            vec!["shell".into()],
        );
        let call = ToolCall {
            tool_id: "shell".into(),
            params: serde_json::Map::default(),
            caller_id: None,
            context: None,

            tool_call_id: String::new(),
            skill_name: None,
        };
        let res = exec.execute_tool_call_erased(&call).await;
        assert!(
            res.is_err(),
            "disallowed tool must be blocked even if in allow list"
        );
    }

    #[tokio::test]
    async fn disallowed_allows_non_disallowed_tool() {
        let exec = FilteredToolExecutor::with_disallowed(
            stub_box(&["shell", "web"]),
            ToolPolicy::AllowList(vec!["shell".into(), "web".into()]),
            vec!["shell".into()],
        );
        let call = ToolCall {
            tool_id: "web".into(),
            params: serde_json::Map::default(),
            caller_id: None,
            context: None,

            tool_call_id: String::new(),
            skill_name: None,
        };
        let res = exec.execute_tool_call_erased(&call).await;
        assert!(res.is_ok(), "non-disallowed tool must be allowed");
    }

    #[test]
    fn disallowed_empty_list_no_change() {
        let exec = FilteredToolExecutor::with_disallowed(
            stub_box(&["shell", "web"]),
            ToolPolicy::InheritAll,
            vec![],
        );
        let defs = exec.tool_definitions_erased();
        assert_eq!(defs.len(), 2);
    }

    #[test]
    fn tool_definitions_filters_disallowed_tools() {
        let exec = FilteredToolExecutor::with_disallowed(
            stub_box(&["shell", "web", "dangerous"]),
            ToolPolicy::InheritAll,
            vec!["dangerous".into()],
        );
        let defs = exec.tool_definitions_erased();
        assert_eq!(defs.len(), 2);
        assert!(!defs.iter().any(|d| d.id == "dangerous"));
    }

    // ── NetworkDenyToolExecutor tests (issue #6030) ────────────────────────

    fn bash_call(command: &str) -> ToolCall {
        let mut params = serde_json::Map::new();
        params.insert("command".into(), serde_json::Value::from(command));
        ToolCall {
            tool_id: "bash".into(),
            params,
            caller_id: None,
            context: None,
            tool_call_id: String::new(),
            skill_name: None,
        }
    }

    #[tokio::test]
    async fn network_deny_blocks_curl() {
        let exec = NetworkDenyToolExecutor::new(stub_box(&["bash"]));
        let res = exec
            .execute_tool_call_erased(&bash_call("curl https://evil.example"))
            .await;
        assert_matches!(res, Err(ToolError::Blocked { .. }));
    }

    #[tokio::test]
    async fn network_deny_blocks_wget_and_nc() {
        let exec = NetworkDenyToolExecutor::new(stub_box(&["bash"]));
        assert!(
            exec.execute_tool_call_erased(&bash_call("wget https://evil.example"))
                .await
                .is_err()
        );
        assert!(
            exec.execute_tool_call_erased(&bash_call("nc -l 4444"))
                .await
                .is_err()
        );
    }

    #[tokio::test]
    async fn network_deny_permits_non_network_bash() {
        let exec = NetworkDenyToolExecutor::new(stub_box(&["bash"]));
        let res = exec.execute_tool_call_erased(&bash_call("ls -la")).await;
        assert!(res.is_ok(), "non-network command must pass through");
    }

    #[tokio::test]
    async fn network_deny_ignores_non_bash_tools() {
        let exec = NetworkDenyToolExecutor::new(stub_box(&["web"]));
        let call = ToolCall {
            tool_id: "web".into(),
            params: serde_json::Map::default(),
            caller_id: None,
            context: None,
            tool_call_id: String::new(),
            skill_name: None,
        };
        let res = exec.execute_tool_call_erased(&call).await;
        assert!(res.is_ok(), "non-bash tool calls must not be inspected");
    }

    #[tokio::test]
    async fn network_deny_confirmed_path_also_enforces() {
        let exec = NetworkDenyToolExecutor::new(stub_box(&["bash"]));
        let res = exec
            .execute_tool_call_confirmed_erased(&bash_call("curl https://evil.example"))
            .await;
        assert_matches!(res, Err(ToolError::Blocked { .. }));
    }

    fn tool_call(tool_id: &str) -> ToolCall {
        ToolCall {
            tool_id: tool_id.into(),
            params: serde_json::Map::default(),
            caller_id: None,
            context: None,
            tool_call_id: String::new(),
            skill_name: None,
        }
    }

    #[tokio::test]
    async fn network_deny_blocks_web_scrape_unconditionally() {
        let exec = NetworkDenyToolExecutor::new(stub_box(&["web_scrape"]));
        let res = exec
            .execute_tool_call_erased(&tool_call("web_scrape"))
            .await;
        assert_matches!(res, Err(ToolError::Blocked { .. }));
    }

    #[tokio::test]
    async fn network_deny_blocks_fetch_unconditionally() {
        let exec = NetworkDenyToolExecutor::new(stub_box(&["fetch"]));
        let res = exec.execute_tool_call_erased(&tool_call("fetch")).await;
        assert_matches!(res, Err(ToolError::Blocked { .. }));
    }

    #[tokio::test]
    async fn network_deny_blocks_fetch_confirmed_path_too() {
        let exec = NetworkDenyToolExecutor::new(stub_box(&["fetch"]));
        let res = exec
            .execute_tool_call_confirmed_erased(&tool_call("fetch"))
            .await;
        assert_matches!(res, Err(ToolError::Blocked { .. }));
    }

    // ── #1184: PlanModeExecutor + disallowed_tools catalog test ───────────

    #[test]
    fn plan_mode_with_disallowed_excludes_from_catalog() {
        // FilteredToolExecutor wrapping PlanModeExecutor must exclude disallowed tools from
        // tool_definitions_erased(), verifying that deny-list is enforced in plan mode catalog.
        let inner = Arc::new(PlanModeExecutor::new(stub_box(&["shell", "web"])));
        let exec = FilteredToolExecutor::with_disallowed(
            inner,
            ToolPolicy::InheritAll,
            vec!["shell".into()],
        );
        let defs = exec.tool_definitions_erased();
        assert!(
            !defs.iter().any(|d| d.id == "shell"),
            "shell must be excluded from catalog"
        );
        assert!(
            defs.iter().any(|d| d.id == "web"),
            "web must remain in catalog"
        );
    }

    // ── PlanModeExecutor tests ─────────────────────────────────────────────

    #[tokio::test]
    async fn plan_mode_blocks_execute_erased() {
        let exec = PlanModeExecutor::new(stub_box(&["shell"]));
        let res = exec.execute_erased("response").await;
        assert!(res.is_err());
    }

    #[tokio::test]
    async fn plan_mode_blocks_execute_confirmed_erased() {
        let exec = PlanModeExecutor::new(stub_box(&["shell"]));
        let res = exec.execute_confirmed_erased("response").await;
        assert!(res.is_err());
    }

    #[tokio::test]
    async fn plan_mode_blocks_tool_call() {
        let exec = PlanModeExecutor::new(stub_box(&["shell"]));
        let call = ToolCall {
            tool_id: "shell".into(),
            params: serde_json::Map::default(),
            caller_id: None,
            context: None,

            tool_call_id: String::new(),
            skill_name: None,
        };
        let res = exec.execute_tool_call_erased(&call).await;
        assert!(res.is_err(), "plan mode must block all tool execution");
    }

    #[test]
    fn plan_mode_exposes_real_tool_definitions() {
        let exec = PlanModeExecutor::new(stub_box(&["shell", "web"]));
        let defs = exec.tool_definitions_erased();
        // Real tool catalog exposed — LLM can reference tools in its plan.
        assert_eq!(defs.len(), 2);
        assert!(defs.iter().any(|d| d.id == "shell"));
        assert!(defs.iter().any(|d| d.id == "web"));
    }

    // ── #6019: checkpoint/speculatable/trust forwarding regression ─────────

    /// Inner executor whose checkpoint/speculatable/trust methods return distinguishable
    /// non-default values, used to prove `FilteredToolExecutor` and `PlanModeExecutor`
    /// forward to `inner` rather than falling through to the "unsupported"/`false`
    /// defaults the removed trait defaults used to provide silently (#6019).
    struct CheckpointingStub {
        trust_recorded: std::sync::Mutex<Option<zeph_tools::SkillTrustLevel>>,
    }

    impl ErasedToolExecutor for CheckpointingStub {
        fn execute_erased<'a>(
            &'a self,
            _response: &'a str,
        ) -> Pin<
            Box<
                dyn std::future::Future<Output = Result<Option<ToolOutput>, ToolError>> + Send + 'a,
            >,
        > {
            Box::pin(std::future::ready(Ok(None)))
        }

        fn execute_confirmed_erased<'a>(
            &'a self,
            _response: &'a str,
        ) -> Pin<
            Box<
                dyn std::future::Future<Output = Result<Option<ToolOutput>, ToolError>> + Send + 'a,
            >,
        > {
            Box::pin(std::future::ready(Ok(None)))
        }

        fn tool_definitions_erased(&self) -> Vec<ToolDef> {
            vec![]
        }

        fn execute_tool_call_erased<'a>(
            &'a self,
            _call: &'a ToolCall,
        ) -> Pin<
            Box<
                dyn std::future::Future<Output = Result<Option<ToolOutput>, ToolError>> + Send + 'a,
            >,
        > {
            Box::pin(std::future::ready(Ok(None)))
        }

        fn execute_tool_call_confirmed_erased<'a>(
            &'a self,
            call: &'a ToolCall,
        ) -> Pin<
            Box<
                dyn std::future::Future<Output = Result<Option<ToolOutput>, ToolError>> + Send + 'a,
            >,
        > {
            self.execute_tool_call_erased(call)
        }

        fn is_tool_retryable_erased(&self, _tool_id: &str) -> bool {
            false
        }

        fn requires_confirmation_erased(&self, _call: &ToolCall) -> bool {
            false
        }

        fn is_tool_speculatable_erased(&self, _tool_id: &str) -> bool {
            true
        }

        fn set_effective_trust(&self, level: zeph_tools::SkillTrustLevel) {
            *self.trust_recorded.lock().unwrap() = Some(level);
        }

        fn checkpoint_undo_erased(&self, n: usize) -> zeph_tools::CheckpointActionResult {
            zeph_tools::CheckpointActionResult {
                reverted_commands: n,
                restored: 0,
                deleted: 0,
                supported: true,
                message: "stub-undo".into(),
            }
        }

        fn checkpoint_redo_erased(&self) -> zeph_tools::CheckpointActionResult {
            zeph_tools::CheckpointActionResult {
                reverted_commands: 0,
                restored: 0,
                deleted: 0,
                supported: true,
                message: "stub-redo".into(),
            }
        }

        fn checkpoint_list_erased(&self) -> zeph_tools::CheckpointListResult {
            zeph_tools::CheckpointListResult {
                entries: vec![],
                redo_depth: 7,
                supported: true,
            }
        }
    }

    fn checkpointing_stub() -> Arc<CheckpointingStub> {
        Arc::new(CheckpointingStub {
            trust_recorded: std::sync::Mutex::new(None),
        })
    }

    #[test]
    fn filtered_executor_forwards_checkpoint_trio_and_speculatable() {
        let inner = checkpointing_stub();
        let exec = FilteredToolExecutor::new(Arc::clone(&inner) as _, ToolPolicy::InheritAll);

        let undo = exec.checkpoint_undo_erased(7);
        assert!(
            undo.supported,
            "checkpoint_undo_erased must forward to inner"
        );
        assert_eq!(
            undo.reverted_commands, 7,
            "n must be forwarded, not hardcoded"
        );
        assert!(
            exec.checkpoint_redo_erased().supported,
            "checkpoint_redo_erased must forward to inner"
        );
        assert_eq!(
            exec.checkpoint_list_erased().redo_depth,
            7,
            "checkpoint_list_erased must forward to inner"
        );
        assert!(
            exec.is_tool_speculatable_erased("anything"),
            "is_tool_speculatable_erased must forward to inner, not default to false"
        );
    }

    #[tokio::test]
    async fn filtered_executor_confirmed_erased_still_enforces_policy() {
        // execute_tool_call_confirmed_erased must delegate through execute_tool_call_erased
        // (preserving the policy check), not blind-forward straight to inner.
        let inner = checkpointing_stub();
        let exec = FilteredToolExecutor::with_disallowed(
            Arc::clone(&inner) as _,
            ToolPolicy::InheritAll,
            vec!["blocked_tool".into()],
        );
        let call = ToolCall {
            tool_id: "blocked_tool".into(),
            params: serde_json::Map::default(),
            caller_id: None,
            context: None,

            tool_call_id: String::new(),
            skill_name: None,
        };
        // confirmed path must still enforce the denylist, not bypass it
        let res = exec.execute_tool_call_confirmed_erased(&call).await;
        assert_matches!(res, Err(ToolError::Blocked { .. }));
    }

    #[test]
    fn plan_mode_forwards_checkpoint_trio_and_speculatable() {
        let inner = checkpointing_stub();
        let exec = PlanModeExecutor::new(Arc::clone(&inner) as _);

        let undo = exec.checkpoint_undo_erased(3);
        assert!(
            undo.supported,
            "checkpoint_undo_erased must forward to inner"
        );
        assert_eq!(undo.reverted_commands, 3);
        assert!(exec.checkpoint_redo_erased().supported);
        assert_eq!(exec.checkpoint_list_erased().redo_depth, 7);
        assert!(
            exec.is_tool_speculatable_erased("anything"),
            "is_tool_speculatable_erased must forward to inner, not default to false"
        );
    }

    #[test]
    fn plan_mode_forwards_set_effective_trust() {
        let inner = checkpointing_stub();
        let exec = PlanModeExecutor::new(Arc::clone(&inner) as _);
        exec.set_effective_trust(zeph_tools::SkillTrustLevel::Quarantined);
        assert_eq!(
            *inner.trust_recorded.lock().unwrap(),
            Some(zeph_tools::SkillTrustLevel::Quarantined),
            "set_effective_trust must forward to inner"
        );
    }

    #[tokio::test]
    async fn plan_mode_confirmed_erased_still_blocks_execution() {
        let inner = checkpointing_stub();
        let exec = PlanModeExecutor::new(Arc::clone(&inner) as _);
        let call = ToolCall {
            tool_id: "shell".into(),
            params: serde_json::Map::default(),
            caller_id: None,
            context: None,

            tool_call_id: String::new(),
            skill_name: None,
        };
        let res = exec.execute_tool_call_confirmed_erased(&call).await;
        assert!(
            res.is_err(),
            "plan mode must block confirmed execution too, not just unconfirmed"
        );
    }

    // ── normalize_tool_id tests ────────────────────────────────────────────

    #[test]
    fn normalize_tool_id_lowercases() {
        assert_eq!(normalize_tool_id("Read"), "read");
        assert_eq!(normalize_tool_id("Write"), "write");
        assert_eq!(normalize_tool_id("Edit"), "edit");
    }

    #[test]
    fn normalize_tool_id_strips_args() {
        assert_eq!(normalize_tool_id("Bash(cargo *)"), "bash");
        assert_eq!(normalize_tool_id("Bash(git *)"), "bash");
        assert_eq!(normalize_tool_id("bash"), "bash");
    }

    #[test]
    fn allow_list_pascal_case_permits_lowercase_runtime_id() {
        let exec = FilteredToolExecutor::new(
            stub_box(&["read", "write", "bash"]),
            ToolPolicy::AllowList(vec!["Read".into(), "Write".into(), "Bash(cargo *)".into()]),
        );
        // Runtime IDs are lowercase; policy entries use PascalCase / argument form.
        assert!(exec.is_allowed("read"));
        assert!(exec.is_allowed("write"));
        assert!(exec.is_allowed("bash"));
        assert!(!exec.is_allowed("web"));
        // tool_definitions_erased must also filter correctly.
        let defs = exec.tool_definitions_erased();
        assert_eq!(
            defs.len(),
            3,
            "read, write, bash must all appear in catalog"
        );
    }

    // ── filter_skills tests ────────────────────────────────────────────────

    #[test]
    fn filter_skills_empty_registry_returns_empty() {
        let registry = zeph_skills::registry::SkillRegistry::load(&[] as &[&str]);
        let filter = SkillFilter::default();
        let result = filter_skills(&registry, &filter).unwrap();
        assert!(result.is_empty());
    }

    #[test]
    fn filter_skills_empty_include_passes_all() {
        // Empty include list means "include everything".
        // With an empty registry, result is still empty — logic is correct.
        let registry = zeph_skills::registry::SkillRegistry::load(&[] as &[&str]);
        let filter = SkillFilter {
            include: vec![],
            exclude: vec![],
        };
        let result = filter_skills(&registry, &filter).unwrap();
        assert!(result.is_empty());
    }

    #[test]
    fn filter_skills_double_star_pattern_is_error() {
        let registry = zeph_skills::registry::SkillRegistry::load(&[] as &[&str]);
        let filter = SkillFilter {
            include: vec!["**".into()],
            exclude: vec![],
        };
        let err = filter_skills(&registry, &filter).unwrap_err();
        assert_matches!(err, SubAgentError::Invalid(_));
    }

    mod proptest_glob {
        use proptest::prelude::*;

        use super::{compile_glob, glob_match};

        proptest! {
            #![proptest_config(proptest::test_runner::Config::with_cases(500))]

            /// glob_match must never panic for any valid (non-**) pattern and any name string.
            #[test]
            fn glob_match_never_panics(
                pattern in "[a-z*-]{1,10}",
                name in "[a-z-]{0,15}",
            ) {
                // Skip patterns with ** (those are compile errors by design).
                if !pattern.contains("**")
                    && let Ok(p) = compile_glob(&pattern)
                {
                    let _ = glob_match(&p, &name);
                }
            }

            /// A literal pattern (no `*`) must match only exact strings.
            #[test]
            fn glob_literal_matches_only_exact(
                name in "[a-z-]{1,10}",
            ) {
                // A literal pattern equal to `name` must match.
                let p = compile_glob(&name).unwrap();
                prop_assert!(glob_match(&p, &name));

                // A different name must not match.
                let other = format!("{name}-x");
                prop_assert!(!glob_match(&p, &other));
            }

            /// The `*` pattern must match every input.
            #[test]
            fn glob_star_matches_everything(name in ".*") {
                let p = compile_glob("*").unwrap();
                prop_assert!(glob_match(&p, &name));
            }
        }
    }
}