* added remote MCP server support * trim * added tests * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * increased timeout * disabling MCP chat toggle * Fix MCP OpenAI function-name validation + cancel propagation for PR #5750 OpenAI requires function.name to match ^[a-zA-Z0-9_-]{1,64}$ before streaming starts. The existing 64-char length check is necessary but not sufficient: MCP servers can return tool names containing '.', '/', spaces, etc. that would 400 the whole chat request. Validate the composed mcp__<server_id>__<tool> name against the regex, skip + warn on miss, and drop duplicate tool names from the same server (which would also 400 the request as "duplicates"). Also propagate the agentic-loop cancel_event into MCP tool execution so a /cancel POST during a long-running MCP call (e.g. GitHub MCP search across a large repo) actually interrupts the in-flight HTTP call instead of waiting out the 300 s timeout. The watcher polls the threading.Event at 50 ms cadence inside the asyncio loop (matches routes/inference.py's existing cancel-watcher cadence) and races against the call task with asyncio.wait FIRST_COMPLETED. Tests added: - test_mcp_specs_skip_invalid_openai_function_names: drops bad chars - test_mcp_specs_skip_empty_tool_name - test_mcp_specs_drops_duplicate_names - test_call_tool_sync_respects_pre_set_cancel_event Also fix test_desktop_auth.py's router stub that listed every existing router but missed mcp_servers_router, so importing main.py fails after this PR adds it to routes/__init__.py. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * PR #5750 round 2: OAuth cleanup on delete/url-change + mcp_enabled standalone Round 2 of cross-platform validation surfaced two more P1 findings: 1. OAuth tokens never get cleared. fastmcp keys tokens by MCP URL, not by server row, and delete / URL change / use_oauth toggle only updated the SQLite row. Re-registering the same URL would silently reuse the old account's credentials. Adds clear_oauth_tokens_async() in mcp_client.py and calls it from the delete + put route handlers when the row had use_oauth=True and either the URL changes or OAuth is turned off. 2. mcp_enabled=true was ignored unless the caller also sent enable_tools=true. The frontend always sends both together so the UI path was fine, but a direct API caller sending only mcp_enabled would silently get no MCP tools, which contradicts the field's documented "append tools from every enabled MCP server" behavior. Loosens the use_tools gate in both the GGUF and safetensors paths so mcp_enabled opens the tool loop on its own; when the caller did not also opt into built-ins, the built-in list starts empty. Tests added: - test_clear_oauth_tokens_async_no_op_safe - test_delete_server_calls_oauth_cleanup_when_oauth_was_on - test_delete_server_skips_oauth_cleanup_when_oauth_off - test_update_server_clears_oauth_on_url_change - test_update_server_clears_oauth_when_oauth_disabled 26 backend MCP tests pass; full studio/backend suite 1710 passed locally. Cross-platform CI (Linux, macOS, Windows) green on staging fork. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * PR #5750 round 3: reject null bool updates + /test surfaces 400 Round 3 of cross-platform validation: 1. PUT /api/mcp/servers/<id> would 500 with TypeError when the body explicitly set is_enabled or use_oauth to null. Pydantic accepts None for an Optional[bool] and _changes_from_payload then passed None into mcp_servers_db.update_server, which int(None)d. Reject explicit null at the validation layer with 400 instead. 2. POST /api/mcp/servers/test caught HTTPException under "except Exception", so an invalid URL came back as HTTP 200 with {"ok": false, "error": "400: ..."} instead of a real 400. The create + update paths return 400 for the same input. Move validation outside the transport try/except so it surfaces 400. Tests added: - test_changes_from_payload_rejects_null_is_enabled - test_changes_from_payload_rejects_null_use_oauth - test_test_endpoint_surfaces_url_validation_as_400 * PR #5750 round 4: hyphenated MCP tool names + empty-tool-list gate Round 4 surfaces two more interaction bugs between the new MCP path and existing safetensors tool plumbing: 1. OpenAI accepts ^[a-zA-Z0-9_-]{1,64}$ for function.name, and round 1 widened the MCP regex to that set, so MCP tools can now be advertised as `mcp__srv__list-issues`. But the XML tool-call parser in tool_call_parser.py used `\w+` (no hyphen), so the model could call the tool but Studio could not parse the call. Same in routes/inference.py's `_TOOL_XML_RE` stripper, which would leave hyphenated tool-call XML in the visible content. Both regexes now use `[\w-]+`. 2. safetensors_agentic treats `tools=[]` as "allow all" (documented contract, exercised by test_empty_tools_list_does_not_enforce_allowlist). When a caller sends `enable_tools=true` + `enabled_tools=[]` + `mcp_enabled=true` and MCP discovery returns 0, the resolved tool list is genuinely empty and built-in tools (web_search / python / terminal) could execute via the model's emitted call. Fix at the route gate instead of breaking the documented contract: set `use_tools=False` when the resolved list is empty, in both GGUF and safetensors paths. Existing callers who omit `enabled_tools` still get ALL_TOOLS and are unaffected. Tests added (32 total): - test_tool_xml_parser_handles_hyphenated_function_names - test_tool_xml_strip_handles_hyphenated_function_names - test_safetensors_agentic_empty_allowlist_still_means_allow_all (documents the contract round 4 preserved) 1716 passed locally; cross-platform CI on staging fork still green. * PR #5750 round 5: GGUF allow-list + CLI policy + hyphenated params + cancel race Round 5 of parallel-reviewer aggregation surfaced six additional findings; five are real and fixed here: 1. Hyphenated MCP parameter names (`<parameter=issue-number>`) were dropped by the XML parser's `\w+` regex. Extended to `[\w-]+` in both core/inference/tool_call_parser.py and core/tool_healing.py. The latter is GGUF's own copy of the parser/strip patterns and was missed by round 4. 2. core/tool_healing.py's `strip_tool_call_markup` still used `<function=\w+>` so hyphenated MCP tool-call XML leaked into the GGUF visible content even after round 4 fixed the shared parser. 3+4. `mcp_enabled` re-opened the tool loop even when the operator passed `unsloth run --disable-tools` (CLI policy False). Round 2's `(_tools_on or payload.mcp_enabled)` gate ignored the raw process policy. Now reads `state.tool_policy.get_tool_policy()` and gates mcp_enabled on `_cli_policy is not False`. Applied to both GGUF and safetensors paths. 5. GGUF's agentic loop called `execute_tool(tool_name, ...)` without checking the model-emitted name against the per-request tool list, while the safetensors loop already enforces this. Added the same allow-list check so a model that hallucinates a filtered MCP name or a built-in the caller opted out of returns "not enabled" instead of executing. Bonus P2 fixes: - `call_tool_sync` now checks `cancel_event.is_set()` BEFORE creating the call task, so a pre-set cancellation does not open the HTTP transport. - `clear_oauth_tokens_async` moved the OAuth import + construction inside the protected try block; a fastmcp.client.auth load error used to escape and 500 the delete / update route. NOT fixed (verified false or out of scope): - finding #10 "structured_content vs structuredContent": fastmcp's CallToolResult dataclass uses snake_case (verified live against structured-only tool result; fields are `dict_keys(['content', 'structured_content', 'meta', 'data', 'is_error'])`). - finding #11 "asyncio.run from running loop": call_tool_sync is invoked from `asyncio.to_thread` worker threads which have no event loop; asyncio.run() is safe there. Tests added (37 total): hyphenated param names, tool_healing strip, GGUF allow-list gate, cancel pre-set short-circuit, OAuth cleanup constructor-error swallowing. 1721 passed locally, no regressions. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: danielhanchen <danielhanchen@gmail.com>
176 lines
7.2 KiB
Python
176 lines
7.2 KiB
Python
# SPDX-License-Identifier: AGPL-3.0-only
|
|
# Copyright 2026-present the Unsloth AI Inc. team. All rights reserved. See /studio/LICENSE.AGPL-3.0
|
|
|
|
"""Tool-call XML parsing and stripping helpers.
|
|
|
|
Extracted verbatim from studio/backend/core/inference/llama_cpp.py so that
|
|
external inference servers (llama-server wrappers, llama-swap, custom
|
|
shims) can reuse the same logic without importing the inference
|
|
orchestrator, structlog, httpx, or the rest of the studio backend.
|
|
|
|
The regexes and function bodies are byte-for-byte identical to the
|
|
original inline implementation in llama_cpp.py. Any change made here must
|
|
preserve that equivalence; tests/python/test_tool_healing_extraction_is_exact.py
|
|
verifies it with AST comparison.
|
|
"""
|
|
|
|
import json
|
|
import re
|
|
|
|
# Pre-compiled patterns for tool XML stripping. Hyphen in the
|
|
# function/parameter name char-class tracks OpenAI's allowed set so
|
|
# MCP tool names with dashes (mcp__srv__list-issues) and parameter
|
|
# names with dashes (`issue-number`) parse alongside the built-ins.
|
|
_TOOL_CLOSED_PATS = [
|
|
re.compile(r"<tool_call>.*?</tool_call>", re.DOTALL),
|
|
re.compile(r"<function=[\w-]+>.*?</function>", re.DOTALL),
|
|
]
|
|
_TOOL_ALL_PATS = _TOOL_CLOSED_PATS + [
|
|
re.compile(r"<tool_call>.*$", re.DOTALL),
|
|
re.compile(r"<function=[\w-]+>.*$", re.DOTALL),
|
|
]
|
|
|
|
# Pre-compiled patterns for tool-call XML parsing.
|
|
_TC_JSON_START_RE = re.compile(r"<tool_call>\s*\{")
|
|
_TC_FUNC_START_RE = re.compile(r"<function=([\w-]+)>\s*")
|
|
_TC_END_TAG_RE = re.compile(r"</tool_call>")
|
|
_TC_FUNC_CLOSE_RE = re.compile(r"\s*</function>\s*$")
|
|
_TC_PARAM_START_RE = re.compile(r"<parameter=([\w-]+)>\s*")
|
|
_TC_PARAM_CLOSE_RE = re.compile(r"\s*</parameter>\s*$")
|
|
|
|
|
|
def parse_tool_calls_from_text(content: str) -> list[dict]:
|
|
"""
|
|
Parse tool calls from XML markup in content text.
|
|
|
|
Handles formats like:
|
|
<tool_call>{"name":"web_search","arguments":{"query":"..."}}</tool_call>
|
|
<tool_call><function=web_search><parameter=query>...</parameter></function></tool_call>
|
|
Closing tags (</tool_call>, </function>, </parameter>) are all optional
|
|
since models frequently omit them.
|
|
"""
|
|
tool_calls = []
|
|
|
|
# Pattern 1: JSON inside <tool_call> tags.
|
|
# Use balanced-brace extraction that skips braces inside JSON strings.
|
|
for m in _TC_JSON_START_RE.finditer(content):
|
|
brace_start = m.end() - 1 # position of the opening {
|
|
depth, i = 0, brace_start
|
|
in_string = False
|
|
while i < len(content):
|
|
ch = content[i]
|
|
if in_string:
|
|
if ch == "\\" and i + 1 < len(content):
|
|
i += 2 # skip escaped character
|
|
continue
|
|
if ch == '"':
|
|
in_string = False
|
|
elif ch == '"':
|
|
in_string = True
|
|
elif ch == "{":
|
|
depth += 1
|
|
elif ch == "}":
|
|
depth -= 1
|
|
if depth == 0:
|
|
break
|
|
i += 1
|
|
if depth == 0:
|
|
json_str = content[brace_start : i + 1]
|
|
try:
|
|
obj = json.loads(json_str)
|
|
tc = {
|
|
"id": f"call_{len(tool_calls)}",
|
|
"type": "function",
|
|
"function": {
|
|
"name": obj.get("name", ""),
|
|
"arguments": obj.get("arguments", {}),
|
|
},
|
|
}
|
|
if isinstance(tc["function"]["arguments"], dict):
|
|
tc["function"]["arguments"] = json.dumps(
|
|
tc["function"]["arguments"]
|
|
)
|
|
tool_calls.append(tc)
|
|
except (json.JSONDecodeError, ValueError):
|
|
pass
|
|
|
|
# Pattern 2: XML-style <function=name><parameter=key>value</parameter></function>
|
|
# All closing tags optional -- models frequently omit </parameter>,
|
|
# </function>, and/or </tool_call>.
|
|
if not tool_calls:
|
|
# Step 1: Find all <function=name> positions and extract their bodies.
|
|
# Body boundary: use only </tool_call> or next <function= as hard
|
|
# boundaries. We avoid using </function> as a boundary because
|
|
# code parameter values can contain that literal string.
|
|
# After extracting, we trim a trailing </function> if present.
|
|
func_starts = list(_TC_FUNC_START_RE.finditer(content))
|
|
for idx, fm in enumerate(func_starts):
|
|
func_name = fm.group(1)
|
|
body_start = fm.end()
|
|
# Hard boundaries: next <function= tag or </tool_call>
|
|
next_func = (
|
|
func_starts[idx + 1].start()
|
|
if idx + 1 < len(func_starts)
|
|
else len(content)
|
|
)
|
|
end_tag = _TC_END_TAG_RE.search(content[body_start:])
|
|
if end_tag:
|
|
body_end = body_start + end_tag.start()
|
|
else:
|
|
body_end = len(content)
|
|
body_end = min(body_end, next_func)
|
|
body = content[body_start:body_end]
|
|
# Trim trailing </function> if present (it's the real closing tag)
|
|
body = _TC_FUNC_CLOSE_RE.sub("", body)
|
|
|
|
# Step 2: Extract parameters from body.
|
|
# For single-parameter functions (the common case: code, command,
|
|
# query), use body end as the only boundary to avoid false matches
|
|
# on </parameter> inside code strings.
|
|
arguments = {}
|
|
param_starts = list(_TC_PARAM_START_RE.finditer(body))
|
|
if len(param_starts) == 1:
|
|
# Single parameter: value is everything from after the tag
|
|
# to end of body, trimming any trailing </parameter>.
|
|
pm = param_starts[0]
|
|
val = body[pm.end() :]
|
|
val = _TC_PARAM_CLOSE_RE.sub("", val)
|
|
arguments[pm.group(1)] = val.strip()
|
|
else:
|
|
for pidx, pm in enumerate(param_starts):
|
|
param_name = pm.group(1)
|
|
val_start = pm.end()
|
|
# Value ends at next <parameter= or end of body
|
|
next_param = (
|
|
param_starts[pidx + 1].start()
|
|
if pidx + 1 < len(param_starts)
|
|
else len(body)
|
|
)
|
|
val = body[val_start:next_param]
|
|
# Trim trailing </parameter> if present
|
|
val = _TC_PARAM_CLOSE_RE.sub("", val)
|
|
arguments[param_name] = val.strip()
|
|
|
|
tc = {
|
|
"id": f"call_{len(tool_calls)}",
|
|
"type": "function",
|
|
"function": {
|
|
"name": func_name,
|
|
"arguments": json.dumps(arguments),
|
|
},
|
|
}
|
|
tool_calls.append(tc)
|
|
return tool_calls
|
|
|
|
|
|
def strip_tool_call_markup(text: str, *, final: bool = False) -> str:
|
|
"""Strip tool-call XML markup from text.
|
|
|
|
When ``final`` is False, only fully closed tool-call blocks are removed.
|
|
When ``final`` is True, trailing incomplete tool-call blocks are removed
|
|
too, and the result is stripped of surrounding whitespace.
|
|
"""
|
|
patterns = _TOOL_ALL_PATS if final else _TOOL_CLOSED_PATS
|
|
for pat in patterns:
|
|
text = pat.sub("", text)
|
|
return text.strip() if final else text
|