fix(agent): preserve bare email tool parity (#5075)

This commit is contained in:
RaresKeY
2026-06-30 20:20:56 +02:00
committed by GitHub
parent 7522b02034
commit d85afd5d72
4 changed files with 102 additions and 13 deletions
+3
View File
@@ -907,6 +907,9 @@ async def _execute_tool_block_impl(
if _args_error is not None: if _args_error is not None:
result = {"error": _args_error, "exit_code": 1} result = {"error": _args_error, "exit_code": 1}
else: else:
if owner:
args = dict(args)
args[_EMAIL_MCP_OWNER_ARG] = owner
result = await mcp.call_tool(qualified, args) result = await mcp.call_tool(qualified, args)
else: else:
result = {"error": "MCP manager not available", "exit_code": 1} result = {"error": "MCP manager not available", "exit_code": 1}
+22 -2
View File
@@ -425,6 +425,23 @@ const TOOL_CALL_RE = /\[TOOL_CALL\][\s\S]*?\[\/TOOL_CALL\]/gi;
let EXEC_FENCE_RE = null; let EXEC_FENCE_RE = null;
const EXEC_FENCE_NON_TOOL = new Set(['bash', 'python']); const EXEC_FENCE_NON_TOOL = new Set(['bash', 'python']);
function escapeRegex(source) {
return String(source).replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
}
function stripExecutedFence(match, tag, inline, body) {
const inlineArgs = (inline || '').trim();
if (!inlineArgs) return '';
const bodyText = (body || '').trim();
const content = bodyText ? `${inlineArgs}\n${bodyText}` : inlineArgs;
try {
JSON.parse(content);
} catch {
return match;
}
return '';
}
async function loadExecFenceRegex() { async function loadExecFenceRegex() {
try { try {
const res = await fetch('/api/tools', { credentials: 'same-origin' }); const res = await fetch('/api/tools', { credentials: 'same-origin' });
@@ -434,7 +451,10 @@ async function loadExecFenceRegex() {
.filter((id) => id && !EXEC_FENCE_NON_TOOL.has(id)); .filter((id) => id && !EXEC_FENCE_NON_TOOL.has(id));
if (tags.length) { if (tags.length) {
EXEC_FENCE_RE = new RegExp( EXEC_FENCE_RE = new RegExp(
'```(?:' + tags.join('|') + ')\\s*\\n[\\s\\S]*?```', 'gi' '```(' + tags.map(escapeRegex).join('|') + ')(?![\\w-])' +
'[ \\t]*([\\[{][^\\n]*?)?[ \\t]*(?=\\r?\\n|```)' +
'\\r?\\n?([\\s\\S]*?)```',
'gi'
); );
} }
} catch (err) { } catch (err) {
@@ -889,7 +909,7 @@ export function roleTimestamp(when) {
*/ */
export function stripToolBlocks(text) { export function stripToolBlocks(text) {
let cleaned = text.replace(TOOL_CALL_RE, ''); let cleaned = text.replace(TOOL_CALL_RE, '');
if (EXEC_FENCE_RE) cleaned = cleaned.replace(EXEC_FENCE_RE, ''); if (EXEC_FENCE_RE) cleaned = cleaned.replace(EXEC_FENCE_RE, stripExecutedFence);
cleaned = cleaned.replace(DSML_TOOL_RE, ''); cleaned = cleaned.replace(DSML_TOOL_RE, '');
cleaned = cleaned.replace(DSML_STRAY_RE, ''); cleaned = cleaned.replace(DSML_STRAY_RE, '');
cleaned = cleaned.replace(XML_TOOL_CALL_RE, ''); cleaned = cleaned.replace(XML_TOOL_CALL_RE, '');
+50 -9
View File
@@ -20,6 +20,7 @@ the behavioral tests exercise an equivalent Python regex built straight from the
backend ``TOOL_TAGS`` — the same source the live regex now derives from — and backend ``TOOL_TAGS`` — the same source the live regex now derives from — and
source-level guards assert the frontend keeps no hard-coded list. source-level guards assert the frontend keeps no hard-coded list.
""" """
import json
import re import re
from pathlib import Path from pathlib import Path
@@ -46,18 +47,48 @@ def _exec_fence_regex() -> re.Pattern:
derives from: the backend TOOL_TAGS (served via /api/tools) minus bash/python.""" derives from: the backend TOOL_TAGS (served via /api/tools) minus bash/python."""
tags = _tool_tags() - _NON_STRIPPED tags = _tool_tags() - _NON_STRIPPED
assert tags, "TOOL_TAGS is empty" assert tags, "TOOL_TAGS is empty"
return re.compile(r"```(?:" + "|".join(sorted(tags)) + r")\s*\n[\s\S]*?```", re.IGNORECASE) return re.compile(
r"```(" + "|".join(re.escape(tag) for tag in sorted(tags)) + r")(?![\w-])"
r"[ \t]*([{\[][^\n]*?)?[ \t]*(?=\r?\n|```)\r?\n?([\s\S]*?)```",
re.IGNORECASE,
)
def _strip_live_exec_fences(text: str) -> str:
rx = _exec_fence_regex()
def repl(match: re.Match) -> str:
inline = (match.group(2) or "").strip()
if not inline:
return ""
body = (match.group(3) or "").strip()
content = f"{inline}\n{body}" if body else inline
try:
json.loads(content)
except (TypeError, ValueError):
return match.group(0)
return ""
return rx.sub(repl, text)
def test_strips_executed_email_tool_fences(): def test_strips_executed_email_tool_fences():
rx = _exec_fence_regex()
# The exact shape the reporter observed lingering in the live bubble. # The exact shape the reporter observed lingering in the live bubble.
text = 'Here are emails\n\n```list_emails\n{"max_results":10}\n```' text = 'Here are emails\n\n```list_emails\n{"max_results":10}\n```'
assert rx.sub("", text).strip() == "Here are emails" assert _strip_live_exec_fences(text).strip() == "Here are emails"
def test_strips_executed_inline_email_tool_fences():
text = 'Here are accounts\n\n```list_email_accounts {}\n```'
assert _strip_live_exec_fences(text).strip() == "Here are accounts"
def test_strips_multiline_inline_json_email_fences():
text = 'Here are emails\n\n```list_emails {"folder": "INBOX",\n"max_results": 2}\n```'
assert _strip_live_exec_fences(text).strip() == "Here are emails"
def test_strips_every_named_email_tool_fence(): def test_strips_every_named_email_tool_fence():
rx = _exec_fence_regex()
email_tools = [ email_tools = [
"list_email_accounts", "send_email", "list_emails", "read_email", "list_email_accounts", "send_email", "list_emails", "read_email",
"reply_to_email", "bulk_email", "archive_email", "delete_email", "reply_to_email", "bulk_email", "archive_email", "delete_email",
@@ -65,22 +96,28 @@ def test_strips_every_named_email_tool_fence():
] ]
for tool in email_tools: for tool in email_tools:
fence = f"```{tool}\n{{}}\n```" fence = f"```{tool}\n{{}}\n```"
assert rx.sub("", fence).strip() == "", f"{tool} fence not stripped" assert _strip_live_exec_fences(fence).strip() == "", f"{tool} fence not stripped"
def test_preserves_existing_web_search_stripping(): def test_preserves_existing_web_search_stripping():
rx = _exec_fence_regex()
fence = '```web_search\n{"q":"x"}\n```' fence = '```web_search\n{"q":"x"}\n```'
assert rx.sub("", fence).strip() == "" assert _strip_live_exec_fences(fence).strip() == ""
def test_does_not_strip_bash_or_python_code_examples(): def test_does_not_strip_bash_or_python_code_examples():
"""bash/python fences are deliberately excluded — they are legitimate code """bash/python fences are deliberately excluded — they are legitimate code
examples a user may have asked the model to show, not tool invocations.""" examples a user may have asked the model to show, not tool invocations."""
rx = _exec_fence_regex()
for lang in sorted(_NON_STRIPPED): for lang in sorted(_NON_STRIPPED):
example = f"```{lang}\nls -la\n```" example = f"```{lang}\nls -la\n```"
assert rx.sub("", example) == example, f"{lang} example wrongly stripped" assert _strip_live_exec_fences(example) == example, f"{lang} example wrongly stripped"
def test_does_not_strip_invalid_inline_json_metadata():
for example in (
'```list_email_accounts {title="setup"}\n```',
'```web_search {query="odysseus"}\n```',
):
assert _strip_live_exec_fences(example) == example
def test_frontend_keeps_no_hardcoded_tool_list(): def test_frontend_keeps_no_hardcoded_tool_list():
@@ -99,6 +136,10 @@ def test_frontend_keeps_no_hardcoded_tool_list():
"chatRenderer.js must fetch the tool set from /api/tools to build " "chatRenderer.js must fetch the tool set from /api/tools to build "
"EXEC_FENCE_RE." "EXEC_FENCE_RE."
) )
assert "JSON.parse(content)" in source, (
"chatRenderer.js must validate inline JSON before stripping same-line "
"tool fences so Markdown metadata stays visible."
)
# The bash/python carve-out must survive the move to the runtime list. # The bash/python carve-out must survive the move to the runtime list.
m = re.search(r"EXEC_FENCE_NON_TOOL\s*=\s*new Set\(\[(?P<body>.*?)\]\)", source, re.DOTALL) m = re.search(r"EXEC_FENCE_NON_TOOL\s*=\s*new Set\(\[(?P<body>.*?)\]\)", source, re.DOTALL)
assert m, "bash/python carve-out (EXEC_FENCE_NON_TOOL) not found in chatRenderer.js" assert m, "bash/python carve-out (EXEC_FENCE_NON_TOOL) not found in chatRenderer.js"
+27 -2
View File
@@ -918,7 +918,9 @@ async def test_plan_mode_blocks_mutating_email_aliases_without_mcp_inventory(mon
disabled_tools=denied, disabled_tools=denied,
) )
assert result["exit_code"] == 0 assert result["exit_code"] == 0
assert mcp.calls == [("mcp__email__search_emails", {"query": "x"})] assert mcp.calls == [
("mcp__email__search_emails", {"query": "x", "_odysseus_owner": "admin-user"}),
]
@pytest.mark.asyncio @pytest.mark.asyncio
@@ -937,7 +939,9 @@ async def test_bare_email_dispatch_empty_content_calls_with_empty_args(monkeypat
owner="admin-user", owner="admin-user",
) )
assert result["exit_code"] == 0 assert result["exit_code"] == 0
assert mcp.calls == [("mcp__email__list_email_accounts", {})] assert mcp.calls == [
("mcp__email__list_email_accounts", {"_odysseus_owner": "admin-user"}),
]
@pytest.mark.asyncio @pytest.mark.asyncio
@@ -997,6 +1001,27 @@ async def test_email_mcp_dispatch_includes_hidden_owner(monkeypatch):
] ]
@pytest.mark.asyncio
async def test_bare_email_mcp_dispatch_includes_hidden_owner(monkeypatch):
import src.tool_execution as tool_execution
from src.tool_execution import execute_tool_block
fake = _FakeMcpManager()
monkeypatch.setattr(tool_execution, "_owner_is_admin", lambda owner: True)
monkeypatch.setattr(tool_execution, "get_mcp_manager", lambda: fake)
desc, result = await execute_tool_block(
SimpleNamespace(tool_type="list_emails", content='{"folder":"INBOX"}'),
owner="alice",
)
assert desc == "email: list_emails"
assert result["exit_code"] == 0
assert fake.calls == [
("mcp__email__list_emails", {"folder": "INBOX", "_odysseus_owner": "alice"}),
]
def test_public_agent_policy_hides_sensitive_tools(monkeypatch): def test_public_agent_policy_hides_sensitive_tools(monkeypatch):
auth_mod = _install_core_auth_stub(monkeypatch) auth_mod = _install_core_auth_stub(monkeypatch)
from src.tool_security import blocked_tools_for_owner from src.tool_security import blocked_tools_for_owner