fix(agent): execute fenced tool calls with inline args and route bare email tool names

src/agent_tools/__init__.py

@@ -14,6 +14,7 @@
 import logging
 from collections import namedtuple
 
+from src.tool_security import BUILTIN_EMAIL_TOOLS
 from src.tool_utils import _truncate, get_mcp_manager, set_mcp_manager
 
 logger = logging.getLogger(__name__)
@@ -86,9 +87,10 @@
              "manage_endpoints", "manage_mcp", "manage_webhooks",
              "manage_tokens", "manage_documents", "manage_settings",
              "manage_notes", "manage_calendar",
-             "resolve_contact", "manage_contact", "list_email_accounts", "send_email", "list_emails",
-             "read_email", "reply_to_email", "bulk_email", "archive_email",
-             "delete_email", "mark_email_read",
+             "resolve_contact", "manage_contact",
+             # Email tool names come from BUILTIN_EMAIL_TOOLS (unioned below)
+             # so the fence regex, dispatch, and non-admin blocklist all cover
+             # the same set.
              # Cookbook tools (LLM serving + downloads). Without these
              # entries, native function calls to e.g. list_served_models
              # are rejected as "Unknown function call" before reaching
@@ -105,7 +107,7 @@
              # Generic loopback to any UI-button endpoint (cookbook,
              # gallery, email folders, etc.) — agent uses this when
              # there's no named tool wrapper for the action.
-             "app_api"}
+             "app_api"} | BUILTIN_EMAIL_TOOLS
 
 ToolBlock = namedtuple("ToolBlock", ["tool_type", "content"])
 

src/agent_tools/admin_tools.py

@@ -14,6 +14,7 @@
 from typing import Optional, Dict
 
 from src.tool_utils import get_mcp_manager, _parse_tool_args
+from src.tool_security import BUILTIN_EMAIL_TOOLS
 
 logger = logging.getLogger(__name__)
 
@@ -706,7 +707,14 @@ def _mask(k, v):
                 "tasks": ["manage_tasks"],
                 "notes": ["manage_notes"],
                 "calendar": ["manage_calendar"],
-                "email": ["mcp__email__list_emails", "mcp__email__read_email", "mcp__email__send_email"],
+                # The full built-in email tool set, in BOTH spellings: the
+                # qualified mcp__email__* names drive MCP schema hiding, the
+                # bare names drive function-schema hiding, and the runtime
+                # gate accepts either — deriving from BUILTIN_EMAIL_TOOLS
+                # keeps the toggle covering every tool the email server
+                # exposes instead of a hand-picked subset.
+                "email": sorted(BUILTIN_EMAIL_TOOLS)
+                         + [f"mcp__email__{t}" for t in sorted(BUILTIN_EMAIL_TOOLS)],
                 "research": ["web_search", "web_fetch"],  # research is a per-request flag, not a tool (closest analog)
             }
 

src/agent_tools/filesystem_tools.py

@@ -186,6 +186,21 @@ async def execute(self, content: str, ctx: dict) -> dict:
         lines = content.split("\n", 1)
         raw_path = lines[0].strip()
         body = lines[1] if len(lines) > 1 else ""
+        # Decode JSON-object args (the fenced inline-args shape
+        # ```write_file {"path": "...", "content": "..."}```), matching
+        # ReadFileTool above. Without this the whole JSON string becomes the
+        # path and the file is written under a garbage name. This is the live
+        # path: there is no filesystem MCP server, so write_file always runs
+        # here via _direct_fallback, not through _build_mcp_args.
+        _stripped = content.strip()
+        if _stripped.startswith("{"):
+            try:
+                _a = json.loads(_stripped)
+                if isinstance(_a, dict) and "path" in _a:
+                    raw_path = str(_a.get("path", "")).strip()
+                    body = str(_a.get("content", ""))
+            except (json.JSONDecodeError, TypeError, ValueError):
+                pass
         try:
             path = _resolve_tool_path(raw_path)
         except ValueError as e:

src/tool_execution.py

@@ -21,7 +21,12 @@
 
 
 
-from src.tool_security import is_public_blocked_tool, owner_is_admin_or_single_user
+from src.tool_security import (
+    BUILTIN_EMAIL_TOOLS,
+    email_tool_policy_names,
+    is_public_blocked_tool,
+    owner_is_admin_or_single_user,
+)
 from src.tool_policy import ToolPolicy
 from src.constants import MAX_OUTPUT_CHARS, MAX_READ_CHARS, MAX_DIFF_LINES, DATA_DIR
 from src.tool_utils import _truncate, get_mcp_manager
@@ -390,8 +395,42 @@ def _parse_write_file(content: str) -> Dict:
 }
 
 
+# Primary argument key(s) for the legacy line-parsed tools. When a fenced
+# block's content is a JSON object carrying one of these keys, it's structured
+# inline args (the relaxed parser's ```web_search {"query": "..."}``` shape) —
+# use the object directly instead of letting the line-based parsers wrap the
+# whole JSON string as the query/url/path/prompt. Keyed off membership only
+# (the primary key never changes), so this can't drift; an unrecognized object
+# safely falls through to the line-based parser, i.e. the previous behavior.
+#
+# IMPORTANT — this only covers the MCP path. _build_mcp_args is reached via
+# _call_mcp_tool only for _MCP_TOOL_MAP tools (so an entry outside that map is
+# dead, as manage_memory was). And of these, only generate_image has a live MCP
+# server today; web_search/web_fetch/read_file/write_file have none, so they run
+# via _direct_fallback -> TOOL_HANDLERS, whose handlers decode JSON themselves
+# (see ReadFileTool/WriteFileTool/WebSearchTool/WebFetchTool). The entries here
+# are kept as defense-in-depth for if/when those servers are added. The live
+# fix for each server-less tool lives in its handler. test_write_file_inline_
+# json_args and test_mcp_json_primary_keys_are_all_live pin both halves.
+_MCP_JSON_PRIMARY_KEYS: Dict[str, tuple] = {
+    "web_search":     ("query", "queries"),
+    "web_fetch":      ("url",),
+    "read_file":      ("path",),
+    "write_file":     ("path",),
+    "generate_image": ("prompt",),
+}
+
+
 def _build_mcp_args(tool: str, content: str) -> Dict:
     """Convert fenced-block text content to structured MCP arguments."""
+    primaries = _MCP_JSON_PRIMARY_KEYS.get(tool)
+    if primaries and content.strip().startswith("{"):
+        try:
+            decoded = json.loads(content.strip())
+        except (json.JSONDecodeError, TypeError):
+            decoded = None
+        if isinstance(decoded, dict) and any(k in decoded for k in primaries):
+            return decoded
     parser = _MCP_ARG_PARSERS.get(tool)
     return parser(content) if parser else {}
 
@@ -596,6 +635,12 @@ async def _execute_tool_block_impl(
     tool = block.tool_type
     content = block.content
 
+    # The block/disable gates below must match every policy-equivalent
+    # spelling of the tool name (bare email names alias their mcp__email__
+    # form — see email_tool_policy_names), not just the spelling the model
+    # happened to emit.
+    policy_names = email_tool_policy_names(tool)
+
     # Misformatted tool call detection: model put JSON inside ```python``` (or
     # similar) without naming the tool. Common with MiniMax-style outputs.
     # Return a helpful error so the model retries with the correct format.
@@ -623,13 +668,13 @@ async def _execute_tool_block_impl(
             pass
 
     # Reject tools that the user has disabled for this request
-    if disabled_tools and tool in disabled_tools:
+    if disabled_tools and not policy_names.isdisjoint(disabled_tools):
         desc = f"{tool}: BLOCKED"
         result = {"error": f"Tool '{tool}' is disabled by user.", "exit_code": 1}
         logger.info(f"Tool blocked by user: {tool}")
         return desc, result
 
-    if tool_policy and tool_policy.blocks(tool):
+    if tool_policy and any(tool_policy.blocks(name) for name in policy_names):
         desc = f"{tool}: BLOCKED"
         result = {
             "error": f"Execution of tool '{tool}' is forbade by the active guide-only policy.",
@@ -823,6 +868,48 @@ async def _execute_tool_block_impl(
     elif tool == "vault_unlock":
         desc = "vault_unlock"
         result = await do_vault_unlock(content, owner=owner)
+    elif tool in BUILTIN_EMAIL_TOOLS:
+        # Bare email tool name from fenced-block models (e.g. Ollama) — route to MCP email server.
+        # Non-admin owners never reach here: BUILTIN_EMAIL_TOOLS ⊆ NON_ADMIN_BLOCKED_TOOLS,
+        # so is_public_blocked_tool() above already rejected them.
+        mcp = get_mcp_manager()
+        qualified = f"mcp__email__{tool}"
+        desc = f"email: {tool}"
+        if mcp:
+            _raw = content.strip()
+            args = {}
+            _args_error = None
+            if _raw:
+                # A non-empty body is always meant to be the call's arguments,
+                # and every email tool takes a JSON object. Anything that
+                # isn't one is a correctable error — NOT a silent empty-args
+                # call, which would read the DEFAULT mailbox/folder instead of
+                # the one the model meant (#3966 class). Only an EMPTY body
+                # keeps the no-arg path (e.g. ```list_email_accounts```).
+                try:
+                    parsed = json.loads(_raw)
+                except (json.JSONDecodeError, TypeError) as _je:
+                    # Covers both `{account: "work"}` (looks like JSON, bad)
+                    # and `account: work` (not JSON at all).
+                    _args_error = (
+                        f"'{tool}' arguments are not valid JSON ({_je}). "
+                        'Send a JSON object, e.g. {"account": "work"} — '
+                        "keys and string values need double quotes."
+                    )
+                else:
+                    if isinstance(parsed, dict):
+                        args = parsed
+                    else:
+                        _args_error = (
+                            f"'{tool}' arguments must be a JSON object, "
+                            'e.g. {"uid": "..."} — got a JSON array/value instead.'
+                        )
+            if _args_error is not None:
+                result = {"error": _args_error, "exit_code": 1}
+            else:
+                result = await mcp.call_tool(qualified, args)
+        else:
+            result = {"error": "MCP manager not available", "exit_code": 1}
     elif tool.startswith("mcp__"):
         # MCP tool dispatch
         mcp = get_mcp_manager()

src/tool_parsing.py

@@ -10,22 +10,74 @@
 import json
 import logging
 import re
-from typing import List, Optional
+from typing import List, Optional, Tuple
 
 from src.agent_tools import ToolBlock, TOOL_TAGS
+from src.tool_security import BUILTIN_EMAIL_TOOLS
 
 logger = logging.getLogger(__name__)
 
 # ---------------------------------------------------------------------------
 # Regex patterns
 # ---------------------------------------------------------------------------
 
-# Pattern 1: ```bash ... ``` fenced code blocks
+# Pattern 1: ```bash ... ``` fenced code blocks. The tag may be followed by a
+# newline (classic form) or by inline JSON args on the same line
+# (```list_email_accounts {}). The same-line part is captured separately
+# (group 2) and judged by _fenced_tool_call below — the regex alone only
+# requires it to start with { or [; anything else after the tag is a Markdown
+# info string (```python title="example.py") and the fence never matches.
+# (?![\w-]) keeps the alternation from prefix-matching longer fence tags:
+# without it, ```python3 would match as tool "python" with content "3\n..."
+# and execute as code.
 _TOOL_BLOCK_RE = re.compile(
-    r"```(" + "|".join(TOOL_TAGS) + r")\s*\n([\s\S]*?)```",
+    r"```(" + "|".join(TOOL_TAGS) + r")(?![\w-])"
+    r"[ \t]*([{\[][^\n]*?)?[ \t]*(?=\r?\n|```)\r?\n?([\s\S]*?)```",
     re.IGNORECASE,
 )
 
+# Tags whose fenced content is raw code, not JSON args. Same-line text after
+# these tags is Markdown fence metadata on a real language (```bash {title=
+# "setup"}), never inline tool args — only the classic tag-then-newline form
+# executes for them.
+_CODE_FENCE_TAGS = frozenset({"bash", "python"})
+
+
+def _fenced_tool_call(m) -> Optional[Tuple[str, str]]:
+    """Classify a Pattern-1 fence match: (tag, content) when it is an
+    executable tool call, None when the fence must stay display text.
+
+    Shared by parse_tool_blocks and strip_tool_blocks so the execute and
+    display decisions can never disagree: a fence that doesn't execute is
+    never stripped, and vice versa.
+
+    Same-line text after the tag only counts as inline tool args when the
+    tag's tool takes JSON args (not a code tag) AND the text is valid
+    standalone JSON. ```bash {title="setup"} and ```python {"x": 1} are
+    fence attributes on real languages, and {title="x"} on any tag is
+    metadata, not arguments — all of those stay visible and inert.
+    """
+    tag = m.group(1).lower()
+    inline = (m.group(2) or "").strip()
+    body = (m.group(3) or "").strip()
+    if not inline:
+        return tag, body
+    if tag in _CODE_FENCE_TAGS:
+        return None
+    # Inline args may continue onto following lines (a JSON object opened on
+    # the tag line); the combined text must parse as JSON or nothing runs.
+    content = f"{inline}\n{body}" if body else inline
+    try:
+        json.loads(content)
+    except (ValueError, TypeError):
+        return None
+    return tag, content
+
+
+def _strip_executed_fence(m) -> str:
+    """re.sub callback: remove only fences that parse as tool calls."""
+    return "" if _fenced_tool_call(m) is not None else m.group(0)
+
 # Pattern 2: [TOOL_CALL] ... [/TOOL_CALL] blocks (some models use this format)
 # Matches: {tool => "shell", args => {--command "ls -la"}} etc.
 _TOOL_CALL_RE = re.compile(
@@ -923,9 +975,20 @@ def parse_tool_blocks(text: str, skip_fenced: bool = False) -> List[ToolBlock]:
     # Pattern 1: fenced code blocks (skipped when `skip_fenced` — see docstring).
     if not skip_fenced:
         for m in _TOOL_BLOCK_RE.finditer(text):
-            tag = m.group(1).lower()
-            content = m.group(2).strip()
+            call = _fenced_tool_call(m)
+            if call is None:
+                continue
+            tag, content = call
             if not content:
+                # An empty fence is still an unambiguous call for the email
+                # tools — ```list_email_accounts``` with no body is a shape
+                # local models really emit for no-arg tools. Dispatch with
+                # empty args and let the tool's own validation answer;
+                # silently dropping the call left models concluding email was
+                # broken. Other tags (bash, python, ...) keep skipping: empty
+                # content is nothing to run.
+                if tag in BUILTIN_EMAIL_TOOLS:
+                    blocks.append(ToolBlock(tag, ""))
                 continue
             # If a code block's content is an <invoke> XML call (some models wrap
             # tool calls in ```python or ```xml fences), parse the invoke instead.
@@ -1037,7 +1100,10 @@ def strip_tool_blocks(text: str, skip_fenced: bool = False) -> str:
     # Normalize DSML first so its markup gets stripped by the <invoke>
     # / <tool_call> removers below instead of leaking to the user.
     text = _normalize_dsml(text)
-    cleaned = text if skip_fenced else _TOOL_BLOCK_RE.sub('', text)
+    # Keep the executed-vs-illustrative fence distinction (only strip fences
+    # that actually dispatched; leave example fences from native models inert
+    # but visible), then remove [TOOL_CALL]{...}[/TOOL_CALL] markup.
+    cleaned = text if skip_fenced else _TOOL_BLOCK_RE.sub(_strip_executed_fence, text)
     # Forward-only removal mirrors parse_tool_blocks: _strip_delimited pairs each
     # opener with a later closer and stops when none is reachable, so untrusted
     # output can't drive the O(n^2) lazy-rescan (ReDoS); see _iter_delimited.

src/tool_schemas.py

@@ -14,6 +14,7 @@
 
 from src.agent_tools import ToolBlock, TOOL_TAGS
 from src.tool_parsing import _TOOL_NAME_MAP
+from src.tool_security import BUILTIN_EMAIL_TOOLS
 
 logger = logging.getLogger(__name__)
 
@@ -1222,15 +1223,15 @@ def function_call_to_tool_block(name: str, arguments: str) -> Optional[ToolBlock
         return None
 
     tool_type = _TOOL_NAME_MAP.get(name, name)
-    _BUILTIN_EMAIL_TOOLS = {"list_email_accounts", "send_email", "list_emails", "read_email", "reply_to_email",
-                            "archive_email", "delete_email", "mark_email_read", "bulk_email", "download_attachment"}
 
     # Some models emit valid JSON that isn't an object (e.g. a bare array
     # ["ls -la"], string, or number) as function arguments. Most local tools keep
     # the legacy empty-object coercion for stream robustness, but email MCP tools
     # must fail closed so a malformed call cannot read the default mailbox.
+    # Uses the shared BUILTIN_EMAIL_TOOLS (single source of truth) so the
+    # fail-closed set can't drift from the dispatch/blocklist sets.
     if not isinstance(args, dict):
-        if tool_type.startswith("mcp__email__") or name in _BUILTIN_EMAIL_TOOLS:
+        if tool_type.startswith("mcp__email__") or name in BUILTIN_EMAIL_TOOLS:
             logger.warning(f"Non-object email function call arguments for {name}: {args!r}; rejecting")
             return None
         logger.warning(f"Non-object function call arguments for {name}: {args!r}; treating as empty")
@@ -1241,7 +1242,7 @@ def function_call_to_tool_block(name: str, arguments: str) -> Optional[ToolBlock
         content = json.dumps(args) if args else "{}"
         return ToolBlock(tool_type, content)
     # Email tools are implemented as MCP — route them to email
-    if name in _BUILTIN_EMAIL_TOOLS:
+    if name in BUILTIN_EMAIL_TOOLS:
         return ToolBlock(f"mcp__email__{name}", json.dumps(args) if args else "{}")
     if tool_type not in TOOL_TAGS:
         logger.warning(f"Unknown function call: {name}")