diff --git a/src/agent_tools/__init__.py b/src/agent_tools/__init__.py index dccb771d5..848acf695 100644 --- a/src/agent_tools/__init__.py +++ b/src/agent_tools/__init__.py @@ -14,6 +14,7 @@ Sub-modules: 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 @@ TOOL_TAGS = {"bash", "python", "web_search", "web_fetch", "read_file", "write_fi "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 @@ TOOL_TAGS = {"bash", "python", "web_search", "web_fetch", "read_file", "write_fi # 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"]) diff --git a/src/agent_tools/admin_tools.py b/src/agent_tools/admin_tools.py index bb54b6cbd..2cd6dc1a8 100644 --- a/src/agent_tools/admin_tools.py +++ b/src/agent_tools/admin_tools.py @@ -14,6 +14,7 @@ import logging 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 @@ async def do_manage_settings(content: str, owner: Optional[str] = None) -> Dict: "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) } diff --git a/src/agent_tools/filesystem_tools.py b/src/agent_tools/filesystem_tools.py index a55944ae5..8b37278bc 100644 --- a/src/agent_tools/filesystem_tools.py +++ b/src/agent_tools/filesystem_tools.py @@ -186,6 +186,21 @@ class WriteFileTool: 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: diff --git a/src/tool_execution.py b/src/tool_execution.py index d498e1f98..1680d18c4 100644 --- a/src/tool_execution.py +++ b/src/tool_execution.py @@ -21,7 +21,12 @@ from typing import Any, Awaitable, Callable, Dict, Optional, Tuple -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 @@ _MCP_ARG_PARSERS: Dict[str, Callable[[str], Dict[str, str]]] = { } +# 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() diff --git a/src/tool_parsing.py b/src/tool_parsing.py index 218b53105..d1a3b7d6e 100644 --- a/src/tool_parsing.py +++ b/src/tool_parsing.py @@ -10,9 +10,10 @@ import bisect 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__) @@ -20,12 +21,63 @@ 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 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 # / 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. diff --git a/src/tool_schemas.py b/src/tool_schemas.py index 5e2ad2045..76e790fc8 100644 --- a/src/tool_schemas.py +++ b/src/tool_schemas.py @@ -14,6 +14,7 @@ from typing import Optional 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}") diff --git a/src/tool_security.py b/src/tool_security.py index 2a7dca3c0..bd8a6f66c 100644 --- a/src/tool_security.py +++ b/src/tool_security.py @@ -8,10 +8,36 @@ from typing import Optional, Set logger = logging.getLogger(__name__) +# Every tool exposed by the built-in email MCP server +# (mcp_servers/email_server.py). Single source of truth: the fence tags +# (TOOL_TAGS), bare-name dispatch (tool_execution), native-call mapping +# (tool_schemas), and the non-admin blocklist below all derive from this set, +# so a tool added to the email server can't become reachable under its bare +# name without also being blocked for non-admins. +BUILTIN_EMAIL_TOOLS = frozenset({ + "list_email_accounts", + "list_emails", + "read_email", + "search_emails", + "send_email", + "reply_to_email", + "draft_email", + "draft_email_reply", + "ai_draft_email_reply", + "archive_email", + "delete_email", + "mark_email_read", + "bulk_email", + "download_attachment", +}) + + # Tools regular/public users must not execute directly. These either expose # server/runtime access, sensitive user data, external messaging, persistent -# state changes, or generic loopback/integration surfaces. -NON_ADMIN_BLOCKED_TOOLS = { +# state changes, or generic loopback/integration surfaces. All email tools are +# included (SECURITY.md: email/MCP capabilities are privileged admin +# functionality). +NON_ADMIN_BLOCKED_TOOLS = BUILTIN_EMAIL_TOOLS | { "bash", "python", "manage_bg_jobs", @@ -34,10 +60,6 @@ NON_ADMIN_BLOCKED_TOOLS = { "manage_settings", "api_call", "app_api", - "send_email", - "reply_to_email", - "list_emails", - "read_email", "resolve_contact", "manage_contact", "manage_calendar", @@ -74,8 +96,20 @@ PLAN_MODE_READONLY_TOOLS = { "search_chats", "list_models", "list_sessions", + # Read-only email tools. list_email_accounts must be here because the + # bare/qualified alias gate in execute_tool_block works both ways: it has + # a native function schema, so plan mode's schema-derived bare denylist + # contains it — and without this allowlist entry that bare entry would + # also block the qualified mcp__email__list_email_accounts call that the + # MCP read-only filter deliberately allows. + "list_email_accounts", "list_emails", "read_email", + # Explicitly read-only rather than allowed-by-omission: this PR makes + # every BUILTIN_EMAIL_TOOLS name fence-taggable, so each one must be + # classified — see the plan-mode partition test in + # tests/test_email_registry_sync.py. + "search_emails", "list_served_models", "list_downloads", "list_cached_models", @@ -109,7 +143,14 @@ _PLAN_MODE_KNOWN_MUTATORS = { "manage_webhooks", "manage_tokens", "manage_settings", "manage_contact", "manage_calendar", "api_call", "app_api", "ui_control", "send_email", "reply_to_email", "bulk_email", "delete_email", - "archive_email", "mark_email_read", "download_model", "serve_model", + "archive_email", "mark_email_read", + # The draft tools create documents and download_attachment writes to + # disk — mutating. They have no native schemas (yet), so without these + # static entries plan-mode safety for their bare fence tags would depend + # entirely on the MCP read-only inventory being present and current. + "draft_email", "draft_email_reply", "ai_draft_email_reply", + "download_attachment", + "download_model", "serve_model", "stop_served_model", "cancel_download", "adopt_served_model", "serve_preset", "generate_image", "edit_image", "trigger_research", "manage_research", # Shell is never read-only-safe; block it explicitly so it stays out of plan @@ -151,6 +192,28 @@ def plan_mode_disabled_tools() -> Set[str]: return (all_names | _PLAN_MODE_KNOWN_MUTATORS) - PLAN_MODE_READONLY_TOOLS +def email_tool_policy_names(tool_name: str) -> frozenset: + """All policy-equivalent spellings of a tool name. + + A bare built-in email tool name and its MCP-qualified mcp__email__ + form dispatch to the same email server tool, but policy sources spell + them either way — plan mode and the MCP settings toggle write qualified + names into denylists, chat-level toggles write bare ones. Every gate must + match against the full alias set, or a call in one spelling slips past a + denylist entry written in the other. Non-email names alias only to + themselves. + """ + if not isinstance(tool_name, str): + return frozenset((tool_name,)) + if tool_name in BUILTIN_EMAIL_TOOLS: + return frozenset((tool_name, f"mcp__email__{tool_name}")) + if tool_name.startswith("mcp__email__"): + bare = tool_name[len("mcp__email__"):] + if bare in BUILTIN_EMAIL_TOOLS: + return frozenset((tool_name, bare)) + return frozenset((tool_name,)) + + def is_public_blocked_tool(tool_name: Optional[str]) -> bool: """Return True when a non-admin/public user must not execute this tool. diff --git a/tests/test_email_registry_sync.py b/tests/test_email_registry_sync.py new file mode 100644 index 000000000..ed02bc703 --- /dev/null +++ b/tests/test_email_registry_sync.py @@ -0,0 +1,86 @@ +"""PR #3681 — the surfaces this PR derives from BUILTIN_EMAIL_TOOLS stay in sync. + +The review rounds on #3681 each found a hand-maintained copy of the email tool +list that had drifted. This PR's scope pins the SECURITY-RELEVANT surfaces to +the single source of truth (the email MCP server itself, the fence tags, the +non-admin blocklist, the bare<->qualified alias rule, and the plan-mode +read-only fix the alias gate requires). The wider advertising/registry +consolidation (schemas, prompt sections, RAG index, UI selector, assistant +seed) lives in a follow-up PR with its own sync tests. +""" +import re +from pathlib import Path + +import src.agent_tools # noqa: F401 — resolve the circular-import cluster first +from src.tool_security import ( + BUILTIN_EMAIL_TOOLS, + NON_ADMIN_BLOCKED_TOOLS, + PLAN_MODE_READONLY_TOOLS, +) + +_REPO_ROOT = Path(__file__).resolve().parent.parent + + +def test_email_server_tools_match_builtin_set(): + """BUILTIN_EMAIL_TOOLS must equal exactly what the email server exposes.""" + source = (_REPO_ROOT / "mcp_servers" / "email_server.py").read_text() + served = set(re.findall(r'Tool\(\s*name="(\w+)"', source)) + assert served == set(BUILTIN_EMAIL_TOOLS), ( + f"email_server tools != BUILTIN_EMAIL_TOOLS; " + f"server-only: {sorted(served - BUILTIN_EMAIL_TOOLS)}, " + f"set-only: {sorted(BUILTIN_EMAIL_TOOLS - served)}" + ) + + +def test_fence_tags_cover_email_tools(): + from src.agent_tools import TOOL_TAGS + + assert BUILTIN_EMAIL_TOOLS <= set(TOOL_TAGS) + + +def test_non_admin_blocklist_covers_email_tools(): + assert BUILTIN_EMAIL_TOOLS <= NON_ADMIN_BLOCKED_TOOLS + + +def test_plan_mode_classifies_every_email_tool(): + """Every fence-taggable email tool must be EXPLICITLY classified for plan + mode: read-only (allowlisted) or mutating (in the static denylist via the + fail-closed backstop). Allowed-by-omission is not a classification — it + silently flips when schemas/backstop change, and it leaves bare-alias + safety depending on the MCP read-only inventory being present.""" + from src.tool_security import plan_mode_disabled_tools + + denied = plan_mode_disabled_tools() + readonly = {"list_email_accounts", "list_emails", "read_email", "search_emails"} + for tool in sorted(BUILTIN_EMAIL_TOOLS): + if tool in readonly: + assert tool in PLAN_MODE_READONLY_TOOLS, f"{tool} must be explicit read-only" + assert tool not in denied, f"read-only {tool} must not be denied in plan mode" + else: + assert tool in denied, f"mutating {tool} missing from the plan-mode denylist" + + +def test_plan_mode_allows_qualified_readonly_email_discovery(): + """list_email_accounts has a native schema, so plan mode's schema-derived + bare denylist contains it; with the bidirectional alias gate, the bare + entry would also block the qualified mcp__email__ call that the MCP + read-only filter deliberately allows — unless it's in the read-only + allowlist (which subtracts it from the denylist).""" + assert "list_email_accounts" in PLAN_MODE_READONLY_TOOLS + + +def test_email_policy_name_aliases(): + """The alias rule every execution gate relies on.""" + from src.tool_security import email_tool_policy_names + + assert email_tool_policy_names("list_emails") == { + "list_emails", "mcp__email__list_emails", + } + assert email_tool_policy_names("mcp__email__delete_email") == { + "delete_email", "mcp__email__delete_email", + } + # Non-email names alias only to themselves — including mcp__email__ + # spellings of tools the email server doesn't expose. + assert email_tool_policy_names("bash") == {"bash"} + assert email_tool_policy_names("mcp__email__not_a_tool") == {"mcp__email__not_a_tool"} + assert email_tool_policy_names("mcp__other__list_emails") == {"mcp__other__list_emails"} diff --git a/tests/test_fenced_inline_args.py b/tests/test_fenced_inline_args.py new file mode 100644 index 000000000..0e9bd3c22 --- /dev/null +++ b/tests/test_fenced_inline_args.py @@ -0,0 +1,186 @@ +"""PR #3681 — fenced tool calls with inline args, and the fence-tag boundary. + +Local fenced-block models (Ollama etc.) emit calls like ```list_email_accounts {} +with the args on the same line as the tag; the parser must execute those. The +relaxed tag pattern must NOT prefix-match longer fence tags: ```python3 is a +language hint, not a "python" tool call with content "3\n...". +""" +import sys +from unittest.mock import MagicMock + +for mod in ['src.agent_tools', 'src.tool_parsing', 'src.tool_schemas', 'src.tool_execution']: + sys.modules.pop(mod, None) +for mod in [ + 'sqlalchemy', 'sqlalchemy.orm', 'sqlalchemy.ext', 'sqlalchemy.ext.declarative', + 'sqlalchemy.ext.hybrid', 'sqlalchemy.sql', 'sqlalchemy.sql.expression', + 'src.database', 'core.models', 'core.database', 'core.auth' +]: + if mod not in sys.modules: + sys.modules[mod] = MagicMock() + +import src.agent_tools # noqa: E402, F401 +from src.tool_parsing import parse_tool_blocks, strip_tool_blocks # noqa: E402 + + +def test_inline_args_on_tag_line_parse(): + # The original bug: ```list_email_accounts {} (args on the tag line) + # never matched because the regex required a newline right after the tag. + blocks = parse_tool_blocks('```list_email_accounts {}\n```') + assert [(b.tool_type, b.content) for b in blocks] == [("list_email_accounts", "{}")] + + +def test_inline_json_args_parse_for_email_tools(): + blocks = parse_tool_blocks('```list_emails {"max_results": 5}\n```') + assert [(b.tool_type, b.content) for b in blocks] == [("list_emails", '{"max_results": 5}')] + + +def test_next_line_content_still_parses(): + # No regression for the classic shape: tag, newline, content. + blocks = parse_tool_blocks('```manage_memory\nadd\nsome text\n```') + assert [(b.tool_type, b.content) for b in blocks] == [("manage_memory", "add\nsome text")] + + +def test_plain_bash_fence_still_parses(): + blocks = parse_tool_blocks('```bash\necho hello\n```') + assert [(b.tool_type, b.content) for b in blocks] == [("bash", "echo hello")] + + +def test_python3_language_hint_is_not_a_python_tool_call(): + # ```python3 must not prefix-match the "python" fence tag — without the + # (?![\w-]) boundary it parsed as tool "python" with content "3\nprint(...)" + # and executed as code. + blocks = parse_tool_blocks('```python3\nprint("hi")\n```') + assert blocks == [], blocks + + +def test_hyphenated_tag_is_not_a_tool_call(): + blocks = parse_tool_blocks('```bash-session\n$ ls\n```') + assert blocks == [], blocks + + +def test_markdown_info_string_is_not_executable_python(): + # ```python title="example.py" is Markdown fence metadata, not tool args. + # Same-line content other than JSON args ({...}/[...]) must not execute — + # otherwise a fence the model meant to display runs as code. + blocks = parse_tool_blocks('```python title="example.py"\nprint("hi")\n```') + assert blocks == [], blocks + + +def test_markdown_info_string_is_not_executable_bash(): + blocks = parse_tool_blocks('```bash title="setup"\necho hi\n```') + assert blocks == [], blocks + + +def test_empty_email_fence_is_an_executable_call(): + # ```list_email_accounts``` with no body is a real shape local models emit + # for no-arg tools — it must dispatch (with empty args), not vanish. + blocks = parse_tool_blocks('```list_email_accounts\n```') + assert [(b.tool_type, b.content) for b in blocks] == [("list_email_accounts", "")] + + +def test_empty_non_email_fence_still_skipped(): + # Empty bash/python/other fences stay inert: empty content is nothing to run. + for tag in ("bash", "python", "manage_memory"): + assert parse_tool_blocks(f'```{tag}\n```') == [] + + +def test_empty_email_fence_is_stripped_from_display(): + # Executed (empty-args) email fences mirror like any executed fence. + text = 'One sec.\n```list_email_accounts\n```\nDone.' + assert strip_tool_blocks(text) == 'One sec.\n\nDone.' + + +def test_inline_json_array_args_still_parse(): + # The narrowed same-line rule must keep accepting JSON args: { or [. + blocks = parse_tool_blocks('```bulk_email {"action": "archive", "uids": [1, 2]}\n```') + assert [(b.tool_type, b.content) for b in blocks] == [ + ("bulk_email", '{"action": "archive", "uids": [1, 2]}') + ] + + +def test_brace_metadata_on_bash_is_not_executable(): + # ```bash {title="setup"} is a Markdown fence attribute on a real + # language. Code tags (bash/python) never take same-line args — even a + # brace-shaped info string must stay display text. + blocks = parse_tool_blocks('```bash {title="setup"}\necho hi\n```') + assert blocks == [], blocks + + +def test_valid_json_metadata_on_python_is_not_executable(): + # Same rule when the attribute happens to BE valid JSON: the tag decides. + blocks = parse_tool_blocks('```python {"title": "example.py"}\nprint("hi")\n```') + assert blocks == [], blocks + + +def test_invalid_inline_json_on_email_tool_is_not_executable(): + # JSON-args tools only execute same-line content that parses as JSON — + # {title="x"} is metadata/garbage, not arguments. + blocks = parse_tool_blocks('```list_emails {title="x"}\n```') + assert blocks == [], blocks + + +def test_inline_json_continuing_on_next_lines_still_parses(): + # A JSON object opened on the tag line may close on a later line. + blocks = parse_tool_blocks('```list_emails {"folder": "INBOX",\n"max_results": 5}\n```') + assert [(b.tool_type, b.content) for b in blocks] == [ + ("list_emails", '{"folder": "INBOX",\n"max_results": 5}') + ] + + +def test_brace_metadata_fences_left_intact_in_display(): + # strip must mirror parse for every rejected fence shape. + for text in ( + 'Example:\n```bash {title="setup"}\necho hi\n```', + 'Example:\n```python {"title": "example.py"}\nprint("hi")\n```', + 'Example:\n```list_emails {title="x"}\n```', + ): + assert strip_tool_blocks(text) == text + + +def test_inline_args_fence_is_stripped_from_display(): + # strip must mirror parse: an executed inline-args fence must not leak + # into the displayed text. + text = 'Checking now.\n```list_email_accounts {}\n```\nDone.' + assert strip_tool_blocks(text) == 'Checking now.\n\nDone.' + + +def test_python3_fence_is_left_intact_in_display(): + # ...and a fence that did NOT parse as a tool call must stay visible. + text = 'Example:\n```python3\nprint("hi")\n```' + assert strip_tool_blocks(text) == text + + +def test_markdown_info_string_fence_is_left_intact_in_display(): + # strip must mirror parse for info-string fences too: not executed, + # so not stripped from the displayed text. + text = 'Example:\n```python title="example.py"\nprint("hi")\n```' + assert strip_tool_blocks(text) == text + + +def test_parse_strip_mirror_across_fence_shape_grid(): + # Invariant for ANY single fence: either it executes AND is stripped, or + # it doesn't execute AND stays fully visible. The one allowed exception is + # an empty NON-EMAIL tool fence (no header, no body): never executed, but + # stripped as noise — pre-PR behavior, kept deliberately. (Empty EMAIL + # fences execute with empty args, so they fall under the first branch.) + from src.agent_tools import TOOL_TAGS + + tags = ["bash", "python", "list_emails", "bulk_email", "manage_memory", + "python3", "bash-session", "notatool"] + headers = ["", " ", ' title="x"', ' {title="x"}', ' {"a": 1}', " [1, 2]", + " {bad json", ' {"a": 1} extra'] + bodies = ["", "content line\n", '{"k": "v"}\n'] + + for tag in tags: + for header in headers: + for body in bodies: + text = f"before\n```{tag}{header}\n{body}```\nafter" + blocks = parse_tool_blocks(text) + stripped = strip_tool_blocks(text) + case = (tag, header, body) + if blocks: + assert stripped == "before\n\nafter", case + elif stripped != text: + assert ( + tag in TOOL_TAGS and not header.strip() and not body.strip() + ), f"non-executed fence was stripped: {case}" diff --git a/tests/test_live_strip_email_tool_fences.py b/tests/test_live_strip_email_tool_fences.py index 1ddad60cf..1e066cb10 100644 --- a/tests/test_live_strip_email_tool_fences.py +++ b/tests/test_live_strip_email_tool_fences.py @@ -24,7 +24,6 @@ import re from pathlib import Path _SRC = Path("static/js/chatRenderer.js") -_TOOLS_SRC = Path("src/agent_tools/__init__.py") _ROUTES_SRC = Path("routes/model_routes.py") # Deliberately NOT stripped: legitimate code-example languages, not tool @@ -33,11 +32,13 @@ _NON_STRIPPED = {"bash", "python"} def _tool_tags() -> set[str]: - """Extract the backend TOOL_TAGS set from src/agent_tools/__init__.py (source-level).""" - source = _TOOLS_SRC.read_text(encoding="utf-8") - m = re.search(r"TOOL_TAGS\s*=\s*\{(?P.*?)\}", source, re.DOTALL) - assert m, "TOOL_TAGS literal not found in src/agent_tools/__init__.py" - return set(re.findall(r'"([a-z_]+)"', m.group("body"))) + """The backend TOOL_TAGS set — the same authoritative set GET /api/tools + serves (sorted) and the live EXEC_FENCE_RE derives from. Imported rather + than source-scraped so it reflects the real set however it is composed: the + literal plus the ``| BUILTIN_EMAIL_TOOLS`` union (email tool names live in + that single source, not inline in the literal).""" + from src.agent_tools import TOOL_TAGS + return set(TOOL_TAGS) def _exec_fence_regex() -> re.Pattern: diff --git a/tests/test_review_regressions.py b/tests/test_review_regressions.py index c10aaba85..08333b2d1 100644 --- a/tests/test_review_regressions.py +++ b/tests/test_review_regressions.py @@ -616,7 +616,16 @@ async def test_public_agent_policy_blocks_sensitive_tools(monkeypatch): monkeypatch.setattr(auth_mod, "AuthManager", lambda: FakeAuth()) - for tool_name in ("send_email", "read_file", "mcp__email__send_email"): + # Every bare email tool name is spelled out (not imported from + # BUILTIN_EMAIL_TOOLS) so accidentally dropping one from that set fails + # here instead of silently shrinking the blocklist. + bare_email_tools = ( + "list_email_accounts", "list_emails", "read_email", "search_emails", + "send_email", "reply_to_email", "draft_email", "draft_email_reply", + "ai_draft_email_reply", "archive_email", "delete_email", + "mark_email_read", "bulk_email", "download_attachment", + ) + for tool_name in bare_email_tools + ("read_file", "mcp__email__send_email"): desc, result = await execute_tool_block( SimpleNamespace(tool_type=tool_name, content="{}"), owner="regular-user", @@ -626,6 +635,311 @@ async def test_public_agent_policy_blocks_sensitive_tools(monkeypatch): assert "restricted to admin users" in result["error"] +@pytest.mark.asyncio +async def test_disabled_qualified_email_tool_blocks_bare_alias(monkeypatch): + """A bare email fence is an alias for its mcp__email__ form. Plan mode and + the MCP settings toggle write the QUALIFIED name into disabled_tools, so + the gate must block the bare spelling too — and never reach the MCP + manager (PR #3681 review follow-up).""" + import src.tool_execution as tool_execution + from src.tool_execution import execute_tool_block + + def fail_get_mcp_manager(): + raise AssertionError("blocked email tool must not reach the MCP manager") + + monkeypatch.setattr(tool_execution, "get_mcp_manager", fail_get_mcp_manager) + + for bare, disabled in ( + # qualified denylist entry blocks the bare alias… + ("list_emails", {"mcp__email__list_emails"}), + ("download_attachment", {"mcp__email__download_attachment"}), + # …and a bare denylist entry blocks the qualified spelling. + ("mcp__email__delete_email", {"delete_email"}), + ): + desc, result = await execute_tool_block( + SimpleNamespace(tool_type=bare, content="{}"), + owner="admin-user", + disabled_tools=disabled, + ) + assert desc == f"{bare}: BLOCKED" + assert result["exit_code"] == 1 + assert "disabled by user" in result["error"] + + +@pytest.mark.asyncio +async def test_tool_policy_qualified_email_block_covers_bare_alias(monkeypatch): + """Same aliasing rule for the turn ToolPolicy denylist.""" + import src.tool_execution as tool_execution + from src.tool_execution import execute_tool_block + from src.tool_policy import ToolPolicy + + def fail_get_mcp_manager(): + raise AssertionError("blocked email tool must not reach the MCP manager") + + monkeypatch.setattr(tool_execution, "get_mcp_manager", fail_get_mcp_manager) + + policy = ToolPolicy(disabled_tools=frozenset({"mcp__email__send_email"})) + desc, result = await execute_tool_block( + SimpleNamespace(tool_type="send_email", content="{}"), + owner="admin-user", + tool_policy=policy, + ) + assert desc == "send_email: BLOCKED" + assert result["exit_code"] == 1 + + +@pytest.mark.asyncio +async def test_disable_tool_email_covers_full_builtin_set(monkeypatch): + """The friendly `disable_tool email` toggle must cover every built-in + email tool, in BOTH spellings — bare names (function-schema hiding, + bare-fence dispatch) and mcp__email__* (MCP schema hiding, runtime + qualified blocks). Hand-picking a subset left tools like delete_email + and download_attachment enabled (PR #3681 review follow-up).""" + # Import first so the module loads against the real core package; only + # the call-time SessionLocal import below sees the stub. + from src.tool_implementations import do_manage_settings + import src.settings as settings_mod + + db_mod = types.ModuleType("core.database") + + class _Db: + def close(self): + pass + + db_mod.SessionLocal = lambda: _Db() + monkeypatch.setitem(sys.modules, "core.database", db_mod) + + store = {} + + def fake_load_settings(): + return dict(store) + + def fake_save_settings(s): + store.clear() + store.update(s) + + monkeypatch.setattr(settings_mod, "load_settings", fake_load_settings) + monkeypatch.setattr(settings_mod, "save_settings", fake_save_settings) + + result = await do_manage_settings( + '{"action": "disable_tool", "tool": "email"}', owner="admin" + ) + + assert result["exit_code"] == 0 + disabled = set(store["disabled_tools"]) + # Spelled out (not imported from BUILTIN_EMAIL_TOOLS) so dropping a name + # from the constant fails here instead of silently shrinking the toggle. + bare_email_tools = ( + "list_email_accounts", "list_emails", "read_email", "search_emails", + "send_email", "reply_to_email", "draft_email", "draft_email_reply", + "ai_draft_email_reply", "archive_email", "delete_email", + "mark_email_read", "bulk_email", "download_attachment", + ) + for tool_name in bare_email_tools: + assert tool_name in disabled, tool_name + assert f"mcp__email__{tool_name}" in disabled, tool_name + + # enable_tool email must remove the full set again. + result = await do_manage_settings( + '{"action": "enable_tool", "tool": "email"}', owner="admin" + ) + assert result["exit_code"] == 0 + assert store["disabled_tools"] == [] + + +def _install_admin_auth_stub(monkeypatch): + auth_mod = _install_core_auth_stub(monkeypatch) + + class FakeAdminAuth: + is_configured = True + + def is_admin(self, username): + return True + + monkeypatch.setattr(auth_mod, "AuthManager", lambda: FakeAdminAuth()) + + +class _FakeMcpManager: + def __init__(self): + self.calls = [] + + async def call_tool(self, name, args): + self.calls.append((name, args)) + return {"output": "ok", "exit_code": 0} + + +@pytest.mark.asyncio +async def test_bare_email_dispatch_rejects_non_object_json_args(monkeypatch): + """The fence parser accepts JSON arrays as inline args, but email tools + take objects — a correctable error must come back instead of a silent + empty-args call (same class as #3966).""" + _install_admin_auth_stub(monkeypatch) + import src.tool_execution as tool_execution + from src.tool_execution import execute_tool_block + + mcp = _FakeMcpManager() + monkeypatch.setattr(tool_execution, "get_mcp_manager", lambda: mcp) + + desc, result = await execute_tool_block( + SimpleNamespace(tool_type="bulk_email", content='["10", "11"]'), + owner="admin-user", + ) + assert result["exit_code"] == 1 + assert "JSON object" in result["error"] + assert mcp.calls == [], "non-object args must never reach the MCP server" + + +@pytest.mark.asyncio +async def test_bare_email_dispatch_rejects_invalid_json_body(monkeypatch): + """The classic tag/body form reaches execution unvalidated (only INLINE + args are JSON-checked by the parser). A non-JSON-object body must return a + correctable parse error — silently becoming {} args would read the DEFAULT + mailbox instead of the one the model meant. Covers both the brace-looking + `{account: "work"}` and the bare `account: work` shapes.""" + _install_admin_auth_stub(monkeypatch) + import src.tool_execution as tool_execution + from src.tool_execution import execute_tool_block + + for bad_body in ('{account: "work"}', "account: work"): + mcp = _FakeMcpManager() + monkeypatch.setattr(tool_execution, "get_mcp_manager", lambda: mcp) + desc, result = await execute_tool_block( + SimpleNamespace(tool_type="list_emails", content=bad_body), + owner="admin-user", + ) + assert result["exit_code"] == 1, bad_body + assert "not valid JSON" in result["error"], bad_body + assert mcp.calls == [], f"malformed args must never reach MCP: {bad_body!r}" + + +@pytest.mark.asyncio +async def test_legacy_mcp_tools_decode_inline_json_args(monkeypatch): + """The relaxed parser accepts inline JSON for non-code tags, but the legacy + line-based arg builders (web_search/web_fetch/read_file/write_file/ + generate_image) would wrap the whole JSON string as the query/path/prompt. + A JSON object carrying the tool's primary key must be used directly.""" + import src.tool_execution as tool_execution + from src.tool_execution import _build_mcp_args + + cases = { + "web_search": ('{"query": "odysseus pr 3681"}', {"query": "odysseus pr 3681"}), + "web_fetch": ('{"url": "https://example.com"}', {"url": "https://example.com"}), + "read_file": ('{"path": "/tmp/x.txt"}', {"path": "/tmp/x.txt"}), + "write_file": ('{"path": "/tmp/x", "content": "hi"}', {"path": "/tmp/x", "content": "hi"}), + "generate_image": ('{"prompt": "a cat"}', {"prompt": "a cat"}), + } + for tool, (content, expected) in cases.items(): + assert _build_mcp_args(tool, content) == expected, tool + + # Freeform (non-JSON) content keeps the line-based behavior. + assert _build_mcp_args("web_search", "latest python release") == {"query": "latest python release"} + # A JSON object WITHOUT the tool's primary key is not args — fall back + # (write_file content the model happened to write as a bare object). + assert _build_mcp_args("write_file", '{"config": "value"}') == { + "path": '{"config": "value"}', "content": "", + } + + +def test_mcp_json_primary_keys_are_all_live(): + """Every _MCP_JSON_PRIMARY_KEYS entry must be reachable: _build_mcp_args is + only called from _call_mcp_tool, which only runs for _MCP_TOOL_MAP tools. + An entry outside _MCP_TOOL_MAP is dead code whose inline-JSON decode never + executes — manage_memory was exactly that (it routes through + dispatch_ai_tool), and a unit test on _build_mcp_args passed on the dead + path while the real call still corrupted. This pins it so it can't recur.""" + from src.tool_execution import _MCP_JSON_PRIMARY_KEYS, _MCP_TOOL_MAP + + dead = set(_MCP_JSON_PRIMARY_KEYS) - set(_MCP_TOOL_MAP) + assert not dead, f"dead JSON-primary entries (never reach _build_mcp_args): {sorted(dead)}" + + +@pytest.mark.asyncio +async def test_write_file_inline_json_args(monkeypatch): + """write_file has no MCP server, so it runs via _direct_fallback -> + WriteFileTool, NOT _build_mcp_args. Inline JSON must therefore be decoded + by the handler itself: drive the LIVE path (execute_tool_block, no MCP) and + assert the file is written to the intended path with the intended content, + not a file literally named with the JSON blob. A _build_mcp_args unit test + can't catch this — it's on the dead MCP path for write_file.""" + import src.tool_execution as tool_execution + from src.tool_execution import execute_tool_block + + monkeypatch.setattr(tool_execution, "_owner_is_admin", lambda owner: True) + monkeypatch.setattr(tool_execution, "is_public_blocked_tool", lambda t: False) + monkeypatch.setattr(tool_execution, "get_mcp_manager", lambda: None) + + captured = {} + import src.agent_tools.filesystem_tools as fst + + def fake_resolve(p): + captured["path"] = p + raise ValueError("probe-stop-before-disk") + + monkeypatch.setattr(tool_execution, "_resolve_tool_path", fake_resolve) + + from src.tool_parsing import parse_tool_blocks + blocks = parse_tool_blocks('```write_file {"path": "/tmp/wf.txt", "content": "hi"}\n```') + for b in blocks: + await execute_tool_block(b, owner="admin") + + assert captured.get("path") == "/tmp/wf.txt", ( + f"write_file did not decode inline JSON args; got path {captured.get('path')!r}" + ) + + +@pytest.mark.asyncio +async def test_plan_mode_blocks_mutating_email_aliases_without_mcp_inventory(monkeypatch): + """Plan-mode safety for bare email aliases must hold from the STATIC + partition alone — no MCP read-only inventory involved: mutators (the + draft/download tools included) are blocked before dispatch, while the + explicitly read-only search_emails goes through.""" + _install_admin_auth_stub(monkeypatch) + import src.tool_execution as tool_execution + from src.tool_execution import execute_tool_block + from src.tool_security import plan_mode_disabled_tools + + mcp = _FakeMcpManager() + monkeypatch.setattr(tool_execution, "get_mcp_manager", lambda: mcp) + denied = plan_mode_disabled_tools() + + for tool_name in ("draft_email", "draft_email_reply", "ai_draft_email_reply", + "download_attachment", "send_email", "delete_email"): + desc, result = await execute_tool_block( + SimpleNamespace(tool_type=tool_name, content="{}"), + owner="admin-user", + disabled_tools=denied, + ) + assert result["exit_code"] == 1, tool_name + assert mcp.calls == [], f"{tool_name} reached the MCP server in plan mode" + + desc, result = await execute_tool_block( + SimpleNamespace(tool_type="search_emails", content='{"query": "x"}'), + owner="admin-user", + disabled_tools=denied, + ) + assert result["exit_code"] == 0 + assert mcp.calls == [("mcp__email__search_emails", {"query": "x"})] + + +@pytest.mark.asyncio +async def test_bare_email_dispatch_empty_content_calls_with_empty_args(monkeypatch): + """An empty fence (```list_email_accounts``` with no body) dispatches with + {} args — the no-arg call shape local models really emit.""" + _install_admin_auth_stub(monkeypatch) + import src.tool_execution as tool_execution + from src.tool_execution import execute_tool_block + + mcp = _FakeMcpManager() + monkeypatch.setattr(tool_execution, "get_mcp_manager", lambda: mcp) + + desc, result = await execute_tool_block( + SimpleNamespace(tool_type="list_email_accounts", content=""), + owner="admin-user", + ) + assert result["exit_code"] == 0 + assert mcp.calls == [("mcp__email__list_email_accounts", {})] + + @pytest.mark.asyncio async def test_email_mcp_non_object_args_fail_before_dispatch(monkeypatch): import src.tool_execution as tool_execution