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

[botinate]

Summary

Local models that use the fenced-block tool channel (Ollama, gemma, etc.) could not call any of the email tools in chat/agent mode. The model emits ```list_email_accounts {}, nothing executes, the raw fence leaks into the chat as text, and the model concludes it has no email access. Two bugs caused this: (1) the fence regex in src/tool_parsing.py required a newline directly after the tool name, so a call with arguments on the same line never matched; (2) even when a fence did parse, bare email tool names had no dispatch branch in src/tool_execution.py and fell through to "Unknown tool type".

Review follow-up (thanks @vgalin): routing bare email names had a security gap — is_public_blocked_tool() checks the bare name before dispatch, and NON_ADMIN_BLOCKED_TOOLS listed only 4 of the 14 email tools, so non-admin owners could have executed the other 10 (delete_email, bulk_email, …). The full email tool set is now defined once (BUILTIN_EMAIL_TOOLS in src/tool_security.py) and the non-admin blocklist, fence tags, bare-name dispatch, and native-call mapping all derive from it, so the sets can't drift apart. This also fixed the native-call mapping for 4 tools (search_emails, draft_email, draft_email_reply, ai_draft_email_reply) missing from the old tool_schemas.py copy; advertising them to models (schemas, prompt sections, RAG index) is part of the follow-up #4053. Additionally, the relaxed fence regex from the first commit could prefix-match longer fence tags (```python3 would have executed 3\nprint(...) as a python tool call — the same false-positive class as #3604); a (?![\w-]) boundary now prevents that.

Second review round (thanks @RaresKeY): two more gaps fixed. (1) The runtime denylist (disabled_tools) and the turn ToolPolicy were checked against the incoming block name only, while plan mode and the MCP settings toggle write the qualified mcp__email__... name — a bare list_emails fence bypassed a qualified block entry. Both gates now match bare and qualified spellings, in either direction. (2) The relaxed fence regex accepted any same-line text after a recognized tag, which made Markdown info strings executable (python title="example.py" ran as a python tool call). Same-line content now only counts as tool input when it starts with {/[ (JSON args); anything else stays display text, with parse/strip mirrored.

Third round (thanks @RaresKeY): (1) the same-line rule is now tag-aware and JSON-validated through one predicate shared by parse and strip — bash/python never take same-line args, other tags require valid standalone JSON — so brace-shaped fence attributes like bash {title="setup"} stay visible and inert. (2) The friendly disable_tool email toggle derives from BUILTIN_EMAIL_TOOLS in both bare and mcp__email__* spellings. Scope: per review, the wider registry/advertising consolidation has been moved out to #4053 (reverted here in ee35896); this PR keeps only the parser fix, the bare-email routing, and the directly required safety gates. Final hardening from corpus replay: empty email fences (list_email_accounts with no body) now dispatch with empty args instead of vanishing, and non-object JSON args return a correctable error.

Target branch

• This PR targets dev, not main.

Linked Issue

Part of #337 — fixes the email-in-chat failure reported there (UI test button works, but the model claims no email access). The calendar half of that issue needs retesting after this lands; the regex fix applies to every fenced tool with inline args (including manage_calendar), but that is unconfirmed.

Also related: #2447 (this fixes the fenced inline-args variant; the bare unfenced variant is still open), #1789 (possibly the same dispatch gap for user-added MCP servers), #3141 (closed; same symptom surface on Windows/Ollama).

Type of Change

• Bug fix (non-breaking — fixes a confirmed issue)

Checklist

• I searched open issues and open PRs — this is not a duplicate.
• This PR targets dev
• My changes are limited to the scope described above — no unrelated refactors or whitespace changes mixed in.
• I actually ran the app (docker compose up or uvicorn app:app) and verified the change works end-to-end. (Manual UAT on macOS with a local Ollama gemma model — results in the comment below. One failing scenario (chat-phrase email kill-switch) decomposes entirely into pre-existing defects outside this PR's files, tracked separately.)

How to Test

1. Automated: python -m pytest tests/test_fenced_inline_args.py tests/test_review_regressions.py -q — covers inline-args parsing, the ```python3/hyphenated-tag non-matches, parse/strip display mirroring, and non-admin blocking of all 14 bare email names plus the mcp__email__... form.
2. Dispatch check: start the app (uvicorn app:app), configure an email account (Settings → Email; confirm the test button lists your inbox).
3. In chat/agent mode, select a local Ollama model (no native function calling — e.g. a gemma variant) and ask: "can you see my email accounts?"
4. Before this PR: the reply is a raw ```list_email_accounts {} fence and no account data. After: the tool executes and the model answers with the configured account(s).
5. Repeat with "list my latest emails" (list_emails) to confirm coverage beyond the accounts tool.
6. Security check (multi-user deployments): as a non-admin user, ask the agent to delete/archive an email — every email tool must come back BLOCKED ("restricted to admin users"), bare or mcp__email__-qualified.

Visual / UI changes — REQUIRED if you touched anything that renders

N/A — no UI/rendering changes; only backend tool parsing/dispatch/policy (src/tool_parsing.py, src/tool_execution.py, src/tool_security.py, src/tool_schemas.py, src/agent_tools/__init__.py) and tests.

---

This PR was written via Claude and verified by a human.

🤖 Generated with Claude Code

[vgalin]

This PR may also needs to update src/tool_security.py.

On current dev, execute_tool_block() checks is_public_blocked_tool(tool) before dispatch. Routing bare email names makes additional raw tool names executable, but NON_ADMIN_BLOCKED_TOOLS currently blocks only some of them (send_email, reply_to_email, list_emails, read_email). Names like list_email_accounts, bulk_email, archive_email, delete_email, mark_email_read, and download_attachment should probably be added too, matching SECURITY.md’s guidance that email/MCP capabilities are privileged admin functionality.

A regression test could extend test_public_agent_policy_blocks_sensitive_tools to cover those bare names as well as mcp__email__....

[botinate]

Good catch — confirmed and fixed in 544a664, plus a bit more than you listed.

The gap was real: execute_tool_block() runs is_public_blocked_tool() on the bare name before my dispatch branch qualifies it to mcp__email__<name>, so the mcp__ prefix rule never applied. And it was slightly wider than your list — search_emails, draft_email, draft_email_reply, and ai_draft_email_reply were also routable but unblocked, so 10 of the 14 email tools were reachable by non-admins.

Rather than appending names to NON_ADMIN_BLOCKED_TOOLS (a fourth hand-maintained copy of the email tool list), I defined the set once — BUILTIN_EMAIL_TOOLS in src/tool_security.py — and derived the blocklist, the fence tags (TOOL_TAGS), the bare-name dispatch, and the native-call mapping from it. A tool added to the email server now can't become routable without also being blocked for non-admins. Side effect: the four names missing from the old tool_schemas.py copy now work for native function-calling models too.

Your test suggestion is in: test_public_agent_policy_blocks_sensitive_tools now iterates all 14 bare names (spelled out rather than imported, so accidentally dropping one from the constant fails the test) plus the mcp__email__... form.

While re-reviewing the parsing change I also found and fixed a false-positive risk it introduced: the relaxed fence regex could prefix-match longer tags, so ```python3 would have parsed as a python tool call with content 3\nprint(...) and executed as code. There's now a (?![\w-]) boundary after the tag, with regression tests in tests/test_fenced_inline_args.py.

This comment was written via Claude and verified by a human.

[RaresKeY]

Findings

issue (tool-policy): Bare email aliases bypass qualified disabled-tool blocks

• Problem: execute_tool_block() checks disabled_tools only against the incoming block name before dispatch. The new bare-email branch then maps that block to mcp__email__{tool} and calls the MCP manager without checking the qualified name. The email settings toggle and plan-mode MCP runtime blocks use qualified mcp__... names, so a bare block like list_emails or download_attachment can still reach the email MCP server.

• Impact: This breaks the hard runtime block for sensitive email tools. Users who turn off email access can still have bare aliases execute, and plan-mode MCP blocks can be bypassed for MCP-only email tools.

• Ask: Normalize bare email names to their qualified MCP names before applying runtime blocks, or check both bare and qualified names before calling mcp.call_tool(). Please add a regression where a bare email block is rejected when disabled_tools contains the qualified MCP name.

• Location: src/tool_execution.py:770

issue (security): Do not execute Markdown fence metadata as tool input

• Problem: The relaxed fenced-tool regex now accepts arbitrary same-line text after a recognized tag. That fixes inline tool args, but it also makes ordinary Markdown info strings executable on the local/fenced-tool path, such as python title="example.py" or bash title="setup".

• Impact: Local agent turns can execute code or other privileged tools from code fences the model meant to display.

• Ask: Keep inline-args support narrow enough that Markdown fence metadata is left visible/non-executable. Add regression tests for python title="..." and bash title="...".

• Location: src/tool_parsing.py:28

Validation

Ran focused parser/tool-policy tests, git diff --check, isolated disabled-tool probes, and parser false-positive probes. Full suite and live email integration were not run.

[botinate]

Both confirmed and fixed in 9076be3; the branch is also merged up to current dev.

P1 (disabled-tools bypass): exactly as described — execute_tool_block() gated on the incoming block name, and the bare-email branch qualified it to mcp__email__<name> only afterwards, so qualified denylist entries (plan mode's runtime MCP blocks, the settings toggle) never matched a bare fence. I also checked there's no enforcement deeper down: MCPManager.call_tool() doesn't consult disabled tools, and the loop dropping mcp_mgr only hides schemas. The fix computes the alias pair once at the top of the dispatcher ({bare, mcp__email__<bare>}) and both the disabled_tools gate and the turn ToolPolicy gate now match either spelling — in both directions, so a bare denylist entry also blocks the qualified form. The regression tests assert blocked calls never reach the MCP manager.

P2 (fence metadata): the same-line allowance is narrowed to JSON args only — after the tag, the rest of the opening line must be empty or start with {/[. Fences like python title="example.py" and bash title="setup" no longer parse, and since strip mirrors parse they stay visible as display text instead of vanishing. The inline-args forms this PR exists for ({}, {"max_results": 5}, arrays) keep working, with tests in both directions.

Full suite after the dev merge: 3265 passed, 1 skipped.

This comment was written via Claude and verified by a human.

[botinate]

One field note from human testing of this PR build (gemma4:26b-a4b-it via Ollama, macOS), for anyone following along: now that fenced email calls actually execute, two pre-existing behaviours became visible that are not caused by this PR — the parsing/dispatch/persistence layers all behave correctly — but its testing exposed them:

1. The model answers before the tool result. gemma often emits a full prose answer in the same round as its tool call — including entirely fabricated results (e.g. a made-up 10-row email table) — then answers again correctly in the next round once the real output is fed back. Both round texts are persisted by design, so the user sees two contradictory answers. This looks like an agent-prompt fix ("when you call a tool, output only the call — never answer before the result arrives"), not a parser change.

2. The streaming view keeps the raw fence visible after execution. The persisted round_texts are stripped correctly, and reloading the session renders the clean layout (tool chip + text). But the live streamed bubble is never re-rendered from the cleaned text when the turn completes, so the executed fence lingers as a code box until reload. Frontend-only gap in the stream-finish path.

Keeping these here for now since this PR's testing surfaced them — happy to split them into their own issues if maintainers prefer.

This comment was written via Claude and verified by a human.

[RaresKeY]

I rechecked the latest head after the follow-up fixes. The earlier disabled-tool alias and simple metadata cases are covered now, but two gaps remain in the narrowed parser/policy surface.

Findings

issue (security): Brace-style fence metadata still executes as tool input

• Problem: The new fenced-tool regex still treats any same-line { or [ after a recognized tag as executable tool content. On the current branch, a display fence like bash {title="setup"} parses as a bash block with {title="setup"}\necho hi, and strip_tool_blocks() removes the fence from display. python {"title":"example.py"} has the same shape.

• Impact: Local/fenced-tool models can still run ordinary Markdown code examples that use brace/bracket fence attributes, which keeps part of the accidental fence-execution class from Agent mode executes ordinary Markdown code fences as tool calls, causing accidental commands and loops (again) #3604 open.

• Ask: Parse the fence header separately and only treat same-line content as inline tool args when it is a valid standalone JSON argument for tools that actually use JSON args; keep bash/python and other Markdown metadata fences visible and non-executable. Please add brace/bracket metadata regressions for parse and strip symmetry.

• Location: src/tool_parsing.py:32

issue (tool-policy): The email category toggle leaves most bare email aliases enabled

• Problem: disable_tool email still expands to only mcp__email__list_emails, mcp__email__read_email, and mcp__email__send_email. This PR now routes all 14 built-in email tools by bare fenced names, and the execution gate only blocks aliases whose bare or qualified spelling is actually present in disabled_tools. With the current category toggle's disabled set, reply_to_email, delete_email, draft_email, download_attachment, and search_emails still reach the email MCP server.

• Impact: An operator who disables email tools can still have sensitive read/mutate/send-adjacent email tools execute through the new bare aliases.

• Ask: Derive the friendly email toggle from the shared built-in email tool set and cover the full set consistently, preferably including both bare and mcp__email__... spellings so schema hiding and the runtime backstop agree. Please add a regression for disable_tool email covering the full built-in email set.

• Location: src/tool_implementations.py:1100

Validation

Ran focused parser/tool-policy tests, chat-route tool-policy tests, adjacent fenced-parser tests, git diff --check, isolated parser probes, and disabled-tool probes. Full suite and live email integration were not run.

PR Hygiene

• GitHub currently shows the branch behind the latest dev after fix: read allow_bash/allow_web_search from JSON body (#3229) #3281. I checked it against the latest dev without conflicts, but it still needs an update before merge.

[RaresKeY]

Re: botinate's field note on PR #3681: https://github.com/pewdiepie-archdaemon/odysseus/pull/3681#issuecomment-4683549687

That is the comment calling out the two pre-existing behaviors:

The model answers before the tool result.
The streaming view keeps the raw fence visible after execution.

I rechecked both against latest dev (c0cc0f9) with source inspection, a fake two-round stream_agent_loop probe, and a focused frontend strip-regex probe. I agree these are pre-existing follow-ups exposed by this PR's testing, not parser/dispatch regressions from the branch.

Confirmed and split out:

• Agent persists pre-tool prose before the real tool result #3992 tracks the pre-tool prose path: first-round text can persist before the tool result arrives, then the later tool-grounded answer is also saved.
• Live chat leaves executed email tool fences visible until reload #3993 tracks the live stream/reload mismatch for email fences: persisted round_texts reload cleanly, but the browser-side live stripper does not cover email tags such as list_emails, so the executed fence can remain visible until reload.

So this PR can stay scoped to the inline-args and bare-email routing change.

[botinate]

Both round-3 findings are fixed, and a deeper self-audit closed the rest of the drift class. Branch is merged up to current dev (includes #3281).

Brace-style metadata (b3d43ad): the fence header is captured separately and judged by one predicate shared between parse_tool_blocks and strip_tool_blocks, so execute/display decisions can't disagree by construction. Same-line content only counts as inline args when (a) the tag is not a code tag — bash/python never take same-line args — and (b) the inline text (plus continuation lines) parses as standalone JSON. bash {title="setup"}, python {"title":"example.py"}, and list_emails {title="x"} stay visible and inert; {}, {"max_results": 5}, and array args keep working. A grid regression asserts the executed⇔stripped invariant across 192 fence shapes.

Email category toggle (b3d43ad): disable_tool email derives from BUILTIN_EMAIL_TOOLS in both spellings (14 bare + 14 mcp__email__*), so schema hiding and the runtime backstop agree; enable_tool email removes the full set. Regression iterates all 14 names in both spellings.

Drift audit (db29046, 016ce47): since each round found another hand-maintained email list, I audited the whole tree for copies. Seven more surfaces were missing the same 5 tools (search_emails, draft_email, draft_email_reply, ai_draft_email_reply, download_attachment): native function schemas, the fenced-prompt tool sections, the RAG description index and email keyword hints, the assistant always-available set, the guide-only known-names set, the frontend tool-selector groups, and the default assistant seed — meaning those tools were dispatchable but never advertised. All now derive from / are pinned to BUILTIN_EMAIL_TOOLS; the bare↔qualified alias rule is a tested helper (email_tool_policy_names); and plan mode classifies every email tool explicitly (list_email_accounts/search_emails join the read-only allowlist — without that, the alias gate would have blocked their qualified read-only calls in plan mode; the draft tools and download_attachment join the fail-closed mutator backstop). tests/test_email_registry_sync.py pins all of it — including the email server source and assistant.js — so the next email tool can't drift silently.

Validation: full suite 3294+ passed. Additionally, I replayed a corpus of real captured gemma/Ollama chat traffic through the parser: every legitimate fenced call from the previous revision still executes (including tag{ with no space and multi-line JSON opened on the tag line), two garbage-arg fences that used to execute with mangled args are now correctly display-only, and the 18 inline-args calls this PR exists for all execute. A human will do additional manual end-to-end testing tomorrow — the "ran the app" checklist box stays honest until then.

This comment was written via Claude and verified by a human.

[RaresKeY]

Findings

issue (email): Draft email tools skip saved writing-style context

• Problem: _EMAIL_TOOL_HINTS is still the old hand-maintained email list. This PR exposes draft_email, draft_email_reply, and ai_draft_email_reply, but when tool selection returns only one of those new draft tools, _build_system_prompt() does not enter the email-context branch. That skips the saved email writing style plus the hard identity/mechanical email rules. I reproduced that with a saved style sentinel: send_email and list_emails include it, while draft_email and ai_draft_email_reply do not.

• Impact: The new preferred draft path can generate emails that ignore the user's configured writing style/signature and the identity guardrails, even though the older send/read paths still get that context.

• Ask: Derive _EMAIL_TOOL_HINTS from BUILTIN_EMAIL_TOOLS plus their mcp__email__... aliases, keeping the existing contact/UI helpers, and add a regression where a draft-only selected tool receives the saved writing-style context.

• Location: src/agent_loop.py:1049

Validation

• Ran focused parser/email/tool-policy regression tests, Python compile checks for the changed backend modules, and node --check static/js/assistant.js.
• Not run: full local suite or live model/email-provider flow.

[RaresKeY]

One reviewer/triage scope note before this grows further.

The original #337 symptom is broad, but the narrow fix here looked like: local/fenced models need inline-arg parsing and bare email tool dispatch so configured email tools actually execute. Some follow-up broadening was necessary once bare email names became executable, especially the non-admin blocklist, disabled-tool alias checks, and fence metadata hardening.

The later registry “drift audit” is useful cleanup, but it moves this PR into a broader email tool registry/policy/prompt/UI consolidation. That may be worth doing, but it changes the review and validation surface.

Can we pick one framing?

• Keep fix(agent): execute fenced tool calls with inline args and route bare email tool names #3681 narrow: fenced inline args + bare email routing + directly required safety gates, and split the wider registry consolidation into a follow-up.
• Or keep the broader consolidation here, but update the PR framing/validation to say this is now an email tool registry and policy consolidation, with Email/calendar not working properly #337 only partially addressed until email is manually retested and the calendar half is confirmed or split.

Either way, I would avoid closing #337 solely from this PR until the email path has a real app retest and the calendar part has a clear disposition.

[botinate]

Two more caught during self-audits and fixed (branch now at c1b1a3d, suite 3304):

• cd2463b — round-6's manage_memory decode was dead code: _build_mcp_args only runs for _MCP_TOOL_MAP tools, and manage_memory routes through dispatch_ai_tool → do_manage_memory instead, so the decode never executed (the unit test passed on the dead path). Removed it and added test_mcp_json_primary_keys_are_all_live, asserting every JSON-primary tool is in _MCP_TOOL_MAP.
• c1b1a3d — same class, sharper: there's no filesystem MCP server, so write_file always runs via _direct_fallback → WriteFileTool, never through _build_mcp_args. WriteFileTool — alone among its siblings (ReadFileTool/WebSearchTool/WebFetchTool all decode JSON) — took lines[0] as the path, so write_file {"path": "...", "content": "..."} wrote to a file literally named with the JSON. Fixed in the handler (matching ReadFileTool), with a regression that drives the live path (execute_tool_block, no MCP) and is verified to fail without the fix. web_search/web_fetch/read_file were already correct via their handlers; write_file was the gap.

Beyond those, the manual UAT and the audits turned up four pre-existing items not in this PR's core change. Rather than open issues on my own read, I'd value your call on whether any are worth filing — happy to write clean repros + tracebacks and/or send fixes:

1. Positional dispatchers mishandle JSON content. Tools that parse fenced content positionally don't decode a JSON object. Verified at the dispatch boundary: search_chats {"query": "X"} passes do_search_chats the literal string {"query": "X"} rather than X (searches the literal JSON, no error). The document tools take a JSON object as title/body (visible junk artifact); manage_memory/create_session/ui_control/send_to_session instead return correctable errors. Pre-existing — dev mishandles their newline-JSON form the same way — and this PR's relaxed parser only adds the same-line trigger. The clean general fix would route fenced content through function_call_to_tool_block.
2. do_ui_control 500 on empty content — empty ui_control hits parts[0] on "".split() at ai_interaction.py:1304 → IndexError → agent-run 500.
3. streamingTTS ReferenceError in the stream-error handler — declared inside the streaming try in static/js/chat.js (~1048), referenced in its catch (~2858), so the handler throws on any stream error and the status indicator sticks.
4. "disable my email tools" routes to ui_control (which has no email toggle) instead of manage_settings disable_tool email, so the chat-phrase kill-switch never engages.

Which, if any, would you like as issues?

This comment was written via Claude and verified by a human.

[RaresKeY]

While live-testing the model-driven email path here, I found a separate current-dev blocker: configured AUTH_ENABLED=false instances treat the agent request owner as non-admin, so email/MCP/local tool schemas are stripped before the model sees them. Direct email MCP worked, and Gemini can emit the list_emails tool call when the schema is provided, but the Odysseus agent path only sent ui_control, ask_user, and update_plan.

I split that into a narrow stabilization fix: #4195. It is not caused by this PR's fenced/native dispatch changes, but it affects live model-driven email validation for this PR.

I also retested this PR on a branch stacked over the separate auth-disabled tool-exposure fix. On that stack:

• focused parser/email/security regression set: 63 passed
• full pytest suite: 3305 passed, 1 skipped
• Docker app started cleanly with AUTH_ENABLED=false
• GreenMail direct MCP list_emails returned the seeded message
• Gemini 2.5 Pro through /api/chat_stream emitted mcp__email__list_emails, the tool output contained the seeded inbox subject, and the final answer was that subject

Not run on the stacked branch: browser/UI E2E, mutating email actions, GitHub CI, and a direct retest of unstacked #3681 after the separate fix lands.

[RaresKeY]

I rechecked the current state after #4195 landed. #4195 is merged into dev now, so the auth-disabled tool-exposure blocker from the last validation pass is no longer an external open dependency for this PR.

The remaining blocker is that GitHub now marks #3681 stale/conflicting with current dev. Replaying the merge against current dev conflicts in src/tool_schemas.py and tests/test_review_regressions.py. The important bit during the rebase is to preserve the newer dev behavior in the conflict resolution:

• keep the current fail-closed handling for non-object email native-function arguments, but wire it to the shared email tool set from this PR;
• keep the newer MCP owner-propagation and auth-disabled regression tests, alongside this PR's parser/policy tests.

After that, can you rerun the unstacked validation on the rebased branch? The useful minimum would be the focused parser/email/security tests, full pytest, and the live email-agent path now that #4195 is in dev.

For your last list of follow-ups: streamingTTS already has #3603. The empty ui_control 500 and the "disable my email tools" phrase routing still look like separate issues, and the positional JSON dispatcher item only partly overlaps #4088. I would file/fix those separately unless maintainers want them folded into another tracker.

[botinate]

Rebased onto current dev (2620f6c). Conflicts resolved as you suggested: the fail-closed non-object email-arg handling now uses the shared BUILTIN_EMAIL_TOOLS set (dropped the stale local copy), and I kept your MCP owner-propagation + auth-disabled regression tests alongside the parser/policy ones. Focused parser/email/security set: 65 passed; full suite: 3870 passed, 2 skipped. The live email-agent retest on the rebased branch is still pending — I'll run that next before this is merge-ready.

This comment was written via Claude and verified by a human.

[RaresKeY]

Rechecked latest head 2620f6cd: it still conflicts with current dev, now in src/tool_implementations.py after manage_settings moved to src/agent_tools/admin_tools.py.

Please rebase, port the full disable_tool email coverage into admin_tools.py, preserve current search/web toggle behavior, then rerun focused parser/email/security tests, full pytest, and the live unstacked email-agent smoke.

[botinate]

Brought the branch up to current dev (ebead80) and resolved the conflict (head now 40f1726); PR shows MERGEABLE again. As you called out, the only textual conflict was in src/tool_implementations.py from manage_settings moving to src/agent_tools/admin_tools.py (#4423/#3629):

• tool_implementations.py: took dev's new facade. This branch's only change to that file was the manage_settings email tool-category expansion, which moved with do_manage_settings.
• admin_tools.py: ported the full disable_tool email coverage there — "email" now derives from BUILTIN_EMAIL_TOOLS in both spellings (bare + mcp__email__*) instead of the hand-picked 3-tool subset. test_disable_tool_email_covers_full_builtin_set passes against the new location (re-exported through the facade, so the existing import path still resolves).
• search/web/research toggles: left at current dev behavior (["web_search", "web_fetch"]) — only the email entry changed.

One semantic conflict that textual auto-merge did not flag, caught on the test run: dev added tests/test_live_strip_email_tool_fences.py, whose _tool_tags() helper source-scrapes only the TOOL_TAGS literal {...}. That worked on dev because the email names were inline there, but this branch makes TOOL_TAGS = {...} | BUILTIN_EMAIL_TOOLS the single source, so the names aren't in the literal anymore and the scraper missed them — the email-fence strip assertions failed even though TOOL_TAGS contains them at runtime. Fixed in 40f1726 by importing the real TOOL_TAGS (the exact set GET /api/tools serves and the live EXEC_FENCE_RE derives from) rather than scraping source; the frontend/route source-guards in that file are untouched.

Validation — focused parser/email/security/admin batch (22 files incl. test_email_registry_sync, test_review_regressions, test_admin_tools_registry, test_fenced_inline_args, test_plan_mode, test_live_strip_email_tool_fences, test_function_call_non_object_args, test_security_regressions, test_prompt_security): 309 passed. Caveat on full coverage: I couldn't run the whole suite in this sandbox — some unrelated route modules don't import here (the 6 model_routes discovery tests in test_review_regressions fail on a core.log_safety import that resolves fine standalone; they fail identically on a clean dev checkout, so they're environmental, not from this merge). Leaving the full pytest run to CI, and the live unstacked email-agent smoke is still pending — a human will run that before this is merge-ready.

This comment was written via Claude and verified by a human.

[botinate]

dev moved again and re-conflicted — this time one hunk in src/tool_parsing.py, from dev's tool-call-parser ReDoS hardening (#4941) overlapping this branch's strip_tool_blocks changes. Brought the branch up to current dev (3021569); head now 215bb94, PR back to MERGEABLE.

The two edits were independent, so the resolution keeps both:

• This branch: _TOOL_BLOCK_RE.sub(_strip_executed_fence, ...) for fenced ```tag blocks — only fences that actually dispatched get stripped; illustrative example fences from native function-calling models stay visible/inert (the executed⇔stripped invariant).
• dev (fix(security): prevent ReDoS in XML and args tool-call parsers #4941): the [TOOL_CALL]{…}[/TOOL_CALL] removal switches from _TOOL_CALL_RE.sub('') to the forward-only _strip_delimited(_TOOL_CALL_OPEN_RE, _TOOL_CALL_CLOSE_RE), so untrusted output can't drive the O(n²) lazy rescan. This mirrors the parse side, which already scans the same delimiters via _iter_delimited.

Validation: parser/strip/ReDoS set — test_fenced_inline_args, test_fenced_example_not_executed_for_native_models, test_redos_llm_parsers, test_redos_xml_tool_parsers, test_tool_parsing_nonstring, test_live_strip_email_tool_fences and the rest — 146 passed, and the parse↔strip invariant tests (89) are green. The broader focused parser/email/security/admin batch still 309 passed (same 6 unrelated model_routes env failures deselected, fail identically on clean dev). Full pytest + live email-agent smoke remain for CI / a human.

This comment was written via Claude and verified by a human.

[RaresKeY]

Thanks for keeping this branch current through the rebases. I re-reviewed the latest head (215bb944) against current dev; the earlier parser/policy issues look addressed, but I found two remaining gaps in the new bare/inline email path that should be fixed before merge.

Findings

issue (email/owner): Bare email aliases drop the authenticated owner before calling MCP

• Problem: The new bare-email branch calls mcp.call_tool(qualified, args) without adding the hidden _odysseus_owner argument, while the existing qualified mcp__email__... branch adds it before calling MCP. The email MCP server uses that hidden owner to filter visible accounts and to fail closed when multiple account owners are configured.

• Impact: Local/fenced-model calls through the new bare aliases can run as ownerless calls. Multi-owner installs can reject them with the owner-required error even though the chat request has an owner, and single-owner/legacy layouts can use the default/global account instead of matching the authenticated owner behavior of the qualified MCP path.

• Ask: Mirror the qualified email path before the bare branch calls MCP, for example by copying args and setting _odysseus_owner when owner is present. Please also add a regression for a bare alias such as list_emails with owner="alice" reaching the MCP manager with that hidden owner.

• Location: src/tool_execution.py:910

issue (frontend): Live stripping misses executed inline-argument fences

• Problem: The backend now executes same-line JSON fences such as an opener of list_email_accounts {}, but the live frontend stripper still builds EXEC_FENCE_RE with a required newline immediately after the tool tag.

• Impact: The backend can execute and persistently strip the new inline-args shape, while the final live bubble still renders the raw executed fence until the session is reloaded. The existing live-strip tests cover classic newline body-form fences, but not this new inline form.

• Ask: Update the live strip pattern to match the backend’s executable inline-args shape, and add a regression for a valid inline email fence such as list_email_accounts {}.

• Location: static/js/chatRenderer.js:436

Validation

• Visible GitHub checks: Passing at review time, with the Trivy SARIF upload job skipped.
• Ran: Focused parser/email/security/policy test slices, git diff --check, owner-propagation probes, and live-strip regex probes.
• Not run: Full local pytest on this prepared head, live app/browser/email/IMAP/SMTP validation, and live unstacked model-driven email-agent smoke.

[alteixeira20]

LGTM.

Reviewed the PR body, full discussion, changed files, staged merge-result diff, and focused validation. This fixes the local/fenced-tool path for inline JSON arguments and bare email tool names, while preserving the required safety gates around email/MCP tools.

Validated:

• PR discussion has no unresolved review threads, no review comments, and no formal blocking reviews.
• Expected head guard passed.
• Changed-file allowlist passed.
• Merge probe is clean against current dev.
• GitHub checks are green.
• git diff --check is clean.
• The 6 model/default-chat failures in tests/test_review_regressions.py reproduce on clean dev with the same core.log_safety import error, so they are not PR-caused.
• Focused PR-relevant validation passed: py_compile plus pytest on email registry, fenced inline args, live strip email fences, and review regressions with the known unrelated model/default-chat failures deselected.

Source-level review notes:

• parse_tool_blocks and strip_tool_blocks now share the same executed-vs-illustrative fence decision.
• bash/python Markdown fence metadata remains visible and inert.
• bare and mcp__email__ tool spellings are checked together by disabled-tool and ToolPolicy gates.
• non-admin users are blocked from all built-in email tools.
• plan mode explicitly classifies all built-in email tools.
• disable_tool email covers all bare and qualified email tool names.
• bare email calls reject non-object or invalid JSON args instead of silently falling back to default mailbox args.