mirror of
https://github.com/unslothai/unsloth.git
synced 2026-07-09 15:58:41 +00:00
* studio: tool calling for Llama-3, Mistral, Gemma 4 on safetensors + MLX (#5615)
Adds tool calling for Llama-3, Mistral (pre-v11 + v11+ + [ARGS]), and Gemma 4 to the safetensors / transformers and MLX backends. Parser patched against llama.cpp / vLLM / SGLang per-family parsers and normalises to OpenAI shape. 96 targeted unit tests + cross-OS staging CI (ubuntu / macos-14 / windows) green on the multi-format probe.
* studio: tool-call healing parity between safetensors / MLX and GGUF
After the multi-format parser landed in #5615, the safetensors / MLX
agentic loop and the GGUF loop still differed on healing behaviour.
This commit closes the gaps in both directions so the two backends
react the same way to identical model output.
Changes:
1. core/inference/llama_cpp.py -- the GGUF BUFFERING state machine
now wakes on every emission marker the shared parser knows. Was
("<tool_call>", "<function="); is now the five-tuple imported
from core.inference.tool_call_parser (Qwen / Qwen3.5 / Llama-3
<|python_tag|> / Mistral [TOOL_CALLS] / Gemma 4 <|tool_call>).
Stream cleanup is delegated to the same shared strip_tool_markup
so leaked markup from any family is removed from assistant
content.
2. core/inference/llama_cpp.py -- per-tool canonical heal key. When
a tool arguments field is a bare string and JSON parsing fails,
the GGUF path now heals to {"code": raw_args} for python,
{"command": raw_args} for terminal, and {"query": raw_args} for
everything else. Was hard-coded to {"query": raw_args}, which
silently routed every python / terminal emission through
web_search. Mirrors safetensors_agentic._CANONICAL_HEAL_ARG.
3. core/inference/safetensors_agentic.py -- re-prompt on plan-
without-action. When the model emits a short forward-looking
intent ("I'll search for that", "Let me check", "First, I
will...") and no tool call, the loop nudges the model to act
instead of silently returning a plan-only answer. Up to
_MAX_REPROMPTS=3 (matches GGUF). The intent regex, character
cap, and instruction text are byte-identical to the GGUF path.
The buffer-end fall-through is unified so a buffered intent
emission that never exits the BUFFERING state still triggers
the re-prompt.
4. core/inference/safetensors_agentic.py -- extra iteration slots
for re-prompts. The loop now budgets max_tool_iterations +
_MAX_REPROMPTS + 1 total iterations and tracks the tool-call
count separately, so a stalling model can be nudged 3x without
eating the caller's tool-call budget. Mirrors the _extra slot
reservation in the GGUF path.
Tests (14 new safetensors-side units; 5 GGUF parity pins):
TestLoopRePrompt -- intent-trigger, plain-answer,
no-tools, cap-at-three, budget
preserved, buffer-end intent.
TestLoopCanonicalHealKey -- python / terminal / unknown.
TestGGUFSafetensorsHealingParity -- shared markers used, shared
strip used, canonical heal keys
identical, intent regex matches
same phrases, _MAX_REPROMPTS
equal on both backends.
All 110 targeted tests pass locally; the broader tool / inference /
model-config / sandbox / anthropic / mlx suites stay green.
Why this matters
Without this parity, Llama-3.2 / Mistral / Gemma 4 emissions on Mac
(MLX) and Linux-safetensors stop the agentic loop as soon as the
model says "Let me...", because the GGUF re-prompt logic never
existed on these backends. The two-marker GGUF BUFFERING tuple also
let non-Qwen tool emissions stream out as plain prose when
llama-server's structured channel did not pick them up. Both paths
now drain the same way, heal the same way, and re-prompt the same
way -- so a tool call that works on GGUF works identically on
safetensors / MLX.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* studio: fix tool-call parser bugs from gemini review on #5620
Three high-priority gemini findings on the tool-call parsing additions:
1. unicode_escape on UTF-8 bytes corrupts non-ASCII literals
(e.g. ✨ becomes â\x9c¨). Replace with json.loads on a quoted
string -- preserves emoji / CJK / RTL while still handling
\n \t \uXXXX escapes.
2. Llama-3 sentinel stripping is order-dependent. A leading
`<|eot_id|><|begin_of_text|>` left `<|begin_of_text|>` behind
because the loop had already passed that sentinel. Loop until
no sentinel matches at the start.
3. Mistral v11+ `[TOOL_CALLS] name { json }` regex uses non-greedy
`\{.*?\}` which truncates at the first `}` of a nested JSON
argument, leaking the tail (e.g. `}}`) into user-visible
streamed text. Same problem for the v0.3 array pattern with
nested brackets. Strip those with balanced brace/bracket
scanning via a new `_strip_mistral_closed_calls` helper called
from `strip_tool_markup`.
Also fix the inference routes' parallel `_TOOL_XML_RE`:
- Same nested-JSON truncation in the Mistral patterns; route the
strip through the parser's balanced-scan helper via a thin
`_strip_tool_xml` wrapper that all existing callers now use.
- Llama-3 `<|python_tag|>[^\n<]*` stopped at any `<`, leaking the
tail of any tool call whose argument contained a literal `<`
(queries, code snippets). Relax to `[^\n]*` which keeps the
strip confined to the actual end-of-line.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* studio: tool calling for DeepSeek (R1/V3/V3.1), GLM 4.x, Kimi K2
Adds three more emission-family parsers to tool_call_parser.py so the
shared safetensors / MLX / GGUF agentic loop covers the major open-
weight reasoning families. Patterns ported from llama.cpp
(common/chat-parser.cpp legacy pre-PEG branch), vLLM
(tool_parsers/deepseekv3*, glm4_moe, kimi_k2), and SGLang
(function_call/deepseekv31_detector, glm4_moe_detector, kimik2_detector).
All three references are MIT (llama.cpp) or Apache-2.0 (vLLM, SGLang).
Formats covered:
DeepSeek R1 <|tool▁calls▁begin|><|tool▁call▁begin|>function
<|tool▁sep|>NAME\n```json\n{...}\n```<|tool▁call▁end|>
<|tool▁calls▁end|>
-- args wrapped in a Markdown json fence, ``function``
literal prefix per llama.cpp common_chat_parse_
deepseek_r1 (chat-parser.cpp:801-820)
DeepSeek V3/V3.1
<|tool▁calls▁begin|><|tool▁call▁begin|>NAME
<|tool▁sep|>{json}<|tool▁call▁end|><|tool▁calls▁end|>
-- bare JSON, no code fence, no ``function`` prefix
per llama.cpp common_chat_parse_deepseek_v3_1
(chat-parser.cpp:822-879)
GLM 4.5/4.6/4.7 <tool_call>NAME\n<arg_key>k1</arg_key>
\n<arg_value>v1</arg_value>...</tool_call>
-- strings raw, non-strings JSON-encoded per
chat_template.jinja; multi-call is back-to-back
blocks. Per llama.cpp common_chat_parse_glm_4_5
(chat-parser.cpp:1040-1052)
Kimi K2 <|tool_calls_section_begin|><|tool_call_begin|>
functions.NAME:IDX<|tool_call_argument_begin|>{json}
<|tool_call_end|><|tool_calls_section_end|>
-- bare name recovered by stripping ``functions.``
prefix and ``:IDX`` suffix; full id preserved as
tool_calls[i].id so the roundtrip replays verbatim.
Per llama.cpp common_chat_parse_kimi_k2
(chat-parser.cpp:896-913)
Marker collisions
GLM uses the same ``<tool_call>`` opener as Qwen but with a bare
function name + ``<arg_key>`` body (Qwen has ``\s*{`` after the tag).
The dispatch keeps Qwen first; Qwen's _TC_JSON_START_RE returns no
matches on a GLM emission, so the fall-through to _parse_glm_tool_
calls handles it correctly. Existing Qwen tests confirm zero
regression.
Streaming buffer
TOOL_XML_SIGNALS extended from 5 markers to 12 so the BUFFERING state
machine wakes on every new family's section opener. Added the
DeepSeek alternative markers (ASCII underscores, short ``<|tool▁calls|>``
form) because real checkpoints emit those variants.
Strip patterns
_TOOL_CLOSED_PATS adds DeepSeek envelope (``<|tool▁calls▁begin|>...
<|tool▁calls▁end|>``) and Kimi section (``<|tool_calls_section_begin|>
...<|tool_calls_section_end|>``). _TOOL_ALL_PATS adds the same plus
the unclosed-tail variants so a truncated stream does not leak
markup.
Route gate
_detect_safetensors_features._PARSER_MARKERS grows to include
DeepSeek and Kimi markers plus ``<arg_key>`` (the unique GLM signal).
_TOOL_XML_RE (the route-layer markup-strip regex) gets DeepSeek and
Kimi closed-pair patterns. _TOOL_TEMPLATE_MARKERS in llama_cpp.py
adds ``message['role'] == 'tool'``, ``message['tool_calls']``, and
``tool_calls is defined`` so the classifier recognises DeepSeek's
subscripted-access template style (it has no top-level
``{% if tools %}`` block).
Tests (39 new):
TestParserDeepSeek (7) -- R1 fence, short-form opener, V3.1 bare,
multi-call, with-reasoning, strip,
signal-wakes-streaming
TestParserGLM (6) -- single, mixed types, multi-call,
unclosed-heal, no-Qwen-regression, strip
TestParserKimi (6) -- single, multi-call, dotted-name, unclosed,
strip, signal-wakes-streaming
TestParserCrossFormatRouting (2) -- dispatch routing, signal coverage
TestLoopBasic loop integration (3) -- DeepSeek / GLM / Kimi end-to-end
Capability advertise (3) -- DeepSeek / GLM / Kimi templates flip
supports_tools=True
All 398 targeted tests pass locally (115 safetensors + 27 capability
+ rest of tool / inference / sandbox / model-config suites). Builds
on PR #5620 (parser + healing parity for Llama-3 / Mistral / Gemma 4);
will rebase cleanly onto main once #5620 lands. PR opened as draft -
do not merge until validated against real models for each family.
Sources
- llama.cpp common/chat-parser.cpp lines 801-913, 1040-1052 (MIT)
- vLLM vllm/tool_parsers/deepseekv31_tool_parser.py (Apache-2.0)
- vLLM vllm/tool_parsers/glm4_moe_tool_parser.py (Apache-2.0)
- vLLM vllm/tool_parsers/kimi_k2_tool_parser.py (Apache-2.0)
- SGLang python/sglang/srt/function_call/{deepseekv31,glm4_moe,kimik2}_
detector.py (Apache-2.0)
- Live chat templates: deepseek-ai/DeepSeek-V3.1, zai-org/GLM-4.6,
moonshotai/Kimi-K2-Instruct, unsloth/DeepSeek-V3-0324,
unsloth/GLM-4.5-Air, unsloth/Kimi-K2-Instruct
* studio/routes: make python_tag strip multi-line aware
Earlier revisions of _TOOL_XML_RE in studio.backend.routes.inference
oscillated between two bug shapes:
5615 r"<\|python_tag\|>[^\n<]*" -- stopped at any literal "<"
so code='if x < 10: pass'
leaked '< 10: pass)' to the
user.
5620.1 r"<\|python_tag\|>[^\n]*" -- single-line only; the second
line of
python.call(code="a\nb")
leaked.
The full parser (_parse_llama3_python_tag) already handles both via
balanced-brace scanning, so the parsing path was fine; the LEAK was
in the streaming strip path that runs on every cumulative emission
while content is still arriving.
Switch to r"<\|python_tag\|>(?:[^<]|<(?!\|))*" so the strip consumes:
* any character that is not a "<" (newlines, JSON, code, ...),
* a "<" only when it is NOT followed by "|" (i.e. NOT a Llama-3
sentinel start like <|eot_id|>, <|eom_id|>, <|begin_of_text|>).
This means:
* code='if x < 10' stays inside the strip (5615 fix preserved),
* multi-line code stays inside the strip (5620 round 2),
* the strip terminates at the next Llama-3 sentinel so trailing
assistant content survives.
Tests: TestRoutesPythonTagStrip (8 cases)
pytest test_safetensors_tool_loop.py test_safetensors_capability_advertise.py
-> 118 passed in 1.81s (was 110).
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* studio: review follow-ups for DeepSeek / GLM / Kimi tool calling
Four fixes addressing review of the parent commit:
1. GLM <arg_value> coercion: tighten the
json.loads -> ast.literal_eval -> raw cascade to only deserialize
when the body unambiguously looks like a JSON literal (object,
array, JSON-encoded string, true/false/null, or numeric). Strings
like ``True`` / ``None`` (Python literals, not JSON) and arbitrary
prose now stay raw. The bare-numeric / bare-boolean ambiguity with
string args remains an inherent limitation of the template without
schema access -- documented in the new comment. Drops the ast
import entirely (closes Gemini's :1036 suggestion).
2. Kimi K2 bare-counter ids (e.g. ``<|tool_call_begin|>3``) are now
dropped rather than surfaced as a tool literally named "3". Matches
vLLM behaviour; SGLang's schema-infer fallback is out of scope at
the parse site. Real Kimi K2 emissions use ``functions.NAME:IDX``
so this is the exception path.
3. Restore the elaborate ``<|python_tag|>(?:[^<]|<(?!\|))*`` clause in
routes.inference._TOOL_XML_RE -- the simpler ``[^\n<]*`` form
regressed PR #5620's multi-line / literal-``<`` python_tag fix.
Restore ``TestRoutesPythonTagStrip`` (8 tests) adapted to call
``_TOOL_XML_RE.sub`` directly since the ``_strip_tool_xml`` helper
was inlined this PR.
4. Add the spaced and backslash-escaped DeepSeek opener variants
(``<|tool calls begin|>``, ``<|tool\_calls\_begin|>``) to
``TOOL_XML_SIGNALS`` for streaming-gate parity with
``_DEEPSEEK_BEGIN_RE``.
Also updates the llama.cpp / vLLM citations in the parser docstrings:
``common/chat-parser.cpp`` was split into ``common/chat.cpp`` +
``common/chat-peg-parser.cpp`` by llama.cpp PR #18675, and vLLM
moved the tool parsers from ``vllm/entrypoints/openai/tool_parsers/``
to ``vllm/tool_parsers/``. Pin to pre-refactor commit ``51fa458a92d6``
where the cited line numbers still resolve.
New regression tests in ``test_pr5624_regressions.py`` cover the GLM
coercion heuristic shapes, GLM literal-``<`` in arg_value, Kimi K2
dotted name, Kimi K2 bare-counter drop, DeepSeek V3.1 truncated
mid-stream, and routes-layer strip across all three new families.
Tests:
pytest studio/backend/tests/test_safetensors_tool_loop.py
studio/backend/tests/test_safetensors_capability_advertise.py
studio/backend/tests/test_pr5624_regressions.py -q
-> 170 passed in 1.91s
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* studio: tighten verbose comments in tool-call parser sections
Comments were narrating what the code already says. Cut historical
"earlier revisions used X, then Y" narratives down to one-line WHY
notes where the footgun still matters (canonical heal-key parity,
balanced-brace vs non-greedy regex, ``(?:[^<]|<(?!\|))*`` over
``[^\n<]*``/``[^\n]*``). Drop section-header banners.
No behaviour change. Re-ran:
pytest studio/backend/tests/test_safetensors_tool_loop.py \
studio/backend/tests/test_safetensors_capability_advertise.py -q
-> 118 passed.
Regression replay (parser + _coerce_arguments on the 5 #5615 inputs)
-> 21/21.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* studio: GLM 4.7 no-newline emission + Kimi multi-section parity
Two fixes surfaced by triple-confirm verification against the live
HF chat templates and upstream llama.cpp / vLLM / SGLang parsers.
1. GLM 4.7 silent drop
``zai-org/GLM-4.7/chat_template.jinja`` line 65 uses
``{{- '<tool_call>' + tc.name -}}`` which Jinja strips trailing
whitespace from, so the first ``<arg_key>`` follows the function
name with NO ``\n`` between them. Real emissions look like
``<tool_call>get_weather<arg_key>city</arg_key><arg_value>London
</arg_value></tool_call>``. The previous ``_GLM_TC_OPEN_RE`` ended
the name with ``\n`` so GLM-4.7 calls were silently dropped
(parser returned ``[]``).
Fix: relax the name terminator to a lookahead that accepts EITHER
``\n`` OR the next ``<arg_key>``:
_GLM_TC_OPEN_RE = re.compile(
r"<tool_call>\s*([^\n<{][^\n<]*?)\s*(?=\n|<arg_key>)"
)
The first-char restriction ``[^\n<{]`` still excludes Qwen's
``<tool_call>{json}`` form so the Qwen-vs-GLM dispatch remains
mutually exclusive.
2. Kimi multi-section parity with vLLM / SGLang
``vllm/tool_parsers/kimi_k2_tool_parser.py`` and SGLang's
``kimik2_detector.py`` both use ``re.findall`` and so collect every
``<|tool_calls_section_begin|>...<|tool_calls_section_end|>`` block
in a single stream. The previous implementation stopped at the
first ``<|tool_calls_section_end|>``. Kimi K2 doesn't emit
multi-section in practice, but parity is cheap.
Fix: wrap the existing per-call body parser in an outer loop that
advances past each ``<|tool_calls_section_end|>`` and continues to
the next ``<|tool_calls_section_begin|>``. Body parsing extracted
to ``_parse_kimi_section_body`` for clarity. Truncated final
section is still surfaced via the existing in-body balanced-brace
walk.
Verified independently against the live HF templates:
* GLM-4.7 emission constructed from the live template parses to the
expected ``{name, arguments}`` shape.
* GLM-4.5 / 4.6 newline shape continues to parse (the lookahead also
matches ``\n``).
* Qwen ``<tool_call>{json}`` still dispatches to the Qwen path -- the
first-char restriction stops the GLM regex from biting JSON bodies.
* Kimi two-section stream surfaces both calls in order with full ids
preserved.
* Bare-counter Kimi ids still drop.
Tests added in ``test_pr5624_regressions.py``:
* ``test_glm_4_7_no_newlines_between_name_and_arg_key``
* ``test_glm_4_7_no_newlines_multi_call``
* ``test_glm_4_7_does_not_break_qwen_path``
* ``test_kimi_two_sections_in_one_stream_both_parse``
pytest studio/backend/tests/test_safetensors_tool_loop.py
studio/backend/tests/test_safetensors_capability_advertise.py
studio/backend/tests/test_pr5624_regressions.py -q
-> 174 passed in 1.93s
pytest studio/backend/tests/ -q -k 'not gpu and not llama_cpp_integration'
-> 2038 passed, 15 failed (pre-existing CI gaps).
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* studio: parser robustness fixes for PR #5620
Three surgical extensions to the multi-format tool-call parser, each
covering a real fine-tune / template emission shape that the current
parser silently drops. No path narrows; all changes widen what is
accepted.
1. `_parse_tool_call_json` now accepts both `arguments` and
`parameters` keys. A Hermes / Qwen `<tool_call>{json}</tool_call>`
wrapper around a Llama-3.2 fine-tune that emits the `parameters`
key was extracting the tool name and silently discarding the
args, producing a working-shaped call with an empty payload. The
bare-JSON and python_tag paths already accepted both keys; this
path now matches them.
2. `_TC_FUNC_START_RE`, `_TC_PARAM_START_RE`, and `_TC_PARAM_CLOSE_RE`
now also match the attribute form
`<function name="..."><param name="...">v</param></function>` used
by MiniCPM-5 and MiniMax-M2. Names land in either capture group,
and `</param>` is accepted as a short close.
3. `_parse_llama3_bare_json` sentinel-strip now consumes the role
label inserted between `<|start_header_id|>` and
`<|end_header_id|>` by Meta's official Llama-3.x chat template.
Without this, every assistant turn re-fed through the template
prefix `<|start_header_id|>assistant<|end_header_id|>\n\n{json}`
parsed to zero calls, so any history-with-tool-call round-trip
in production silently dropped.
Tests in `studio/backend/tests/test_safetensors_tool_loop.py`:
* `TestParserRobustness::test_tool_call_json_accepts_parameters_key`
* `TestParserRobustness::test_function_xml_attribute_form`
* `TestParserRobustness::test_function_xml_attribute_form_multi_param`
* `TestParserRobustness::test_function_xml_legacy_equals_form_still_works`
(regression guard for the existing `<function=name>` syntax)
* `TestParserRobustness::test_llama3_chat_template_round_trip`
* `TestParserRobustness::test_llama3_round_trip_all_roles`
* `TestParserRobustness::test_llama3_round_trip_with_eot_prefix`
`pytest studio/backend/tests/test_safetensors_tool_loop.py
studio/backend/tests/test_safetensors_capability_advertise.py -q`
goes from 118 to 125 passed.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Trim verbose comments in tool-call parser sections for PR #5624
Pure comment / docstring tightening on top of the GLM 4.7 + Kimi
multi-section fixes. No behavioural change.
* Drop multi-paragraph prelude and post-refactor citation chatter in
the DeepSeek, GLM and Kimi parser docstrings; keep the shape and
upstream-commit pin.
* Collapse ``parse_tool_calls_from_text``'s 9 per-family blocks into
a single ordered loop with one combined comment.
* Tighten the GLM coercion, Kimi bare-counter and ``_TOOL_XML_RE``
comments to one or two lines each.
* Same trim pass on ``_PARSER_MARKERS`` and the regression-test
docstrings.
Tests:
pytest studio/backend/tests/test_safetensors_tool_loop.py
studio/backend/tests/test_safetensors_capability_advertise.py
studio/backend/tests/test_pr5624_regressions.py -q
-> 174 passed in 2.00s
* Fix O(N^2) DeepSeek V3.1 backtracking for PR #5624
Adversarial input ``<|tool▁calls▁begin|><|tool▁call▁begin|>fn<|tool▁sep|>``
followed by a long body that does NOT contain a closing brace caused
the V3 path's ``([^\n<]+?)<|tool▁sep|>`` regex to backtrack
quadratically: at each position the lazy quantifier extends one char
at a time looking for a sep that isn't there, taking ~19s on 50k
chars.
Replace the regex search with ``str.find`` on the sep marker plus a
left-walk to recover the name. ``str.find`` is O(N); the walk stops
on ``\n`` (turn boundary), ``<`` (start of a tag), or ``>`` (end of
an optional ``<|tool▁call▁begin|>`` prefix). Same observable
behaviour as the regex on every canonical input.
Tests:
test_deepseek_v3_1_huge_truncated_body_is_linear (new) -- 50k chars
must parse in < 1s.
pytest studio/backend/tests/test_safetensors_tool_loop.py
studio/backend/tests/test_safetensors_capability_advertise.py
studio/backend/tests/test_pr5624_regressions.py -q
-> 175 passed in 1.97s
pytest studio/backend/tests/ -q -k 'not gpu and not llama_cpp_integration'
-> 2038 passed, 15 pre-existing failures unchanged.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* studio: terminate function-XML body at </function>, not just </tool_call>
`_parse_function_xml` was looking for `</tool_call>` (the Hermes
wrapper) as the body terminator. When a model emits a standalone
`<function=NAME><parameter=K>v</parameter></function>` followed by
explanatory prose (which models routinely do), no `</tool_call>` is
present, so the body extended to end-of-string and the trailing
prose leaked into the LAST parameter value.
Pre-existing on main (the legacy `<function=NAME>` form had this
bug too). Same affects PR #5620's new attribute-form
`<function name="NAME"><param name="K">v</param></function>`
emission used by MiniCPM-5 / MiniMax-M2.
Fix: `_TC_END_TAG_RE` now matches either `</tool_call>` OR
`</function>`. The existing `_TC_FUNC_CLOSE_RE` / `_TC_PARAM_CLOSE_RE`
strips are unchanged. Multi-call inputs still bound each function
at the next `<function=` start, so no over-eager consumption.
New tests:
* `test_function_xml_followed_by_prose` (legacy form + prose)
* `test_function_attribute_xml_followed_by_prose` (attribute form + prose)
Existing `test_code_with_embedded_xml` still passes (a parameter
value containing literal `<a></a>` is preserved because the
embedded close tag is `</a>`, not `</function>`).
`pytest studio/backend/tests/test_safetensors_tool_loop.py
studio/backend/tests/test_safetensors_capability_advertise.py -q`
goes from 125 to 127 passed.
* Studio: tighten Llama-3.2 bare-JSON guard
A fuzz pass on PR #5811 turned up that ``_parse_llama3_bare_json``
accepted ``parameters`` as a string, contradicting the docstring's
"parameters or arguments is a dict" guard. Prose JSON like
``{"name":"foo","parameters":"a sentence"}`` would wrongly fire the
parser, which the agentic loop would then heal into a real
``foo(query="a sentence")`` call.
Same code lives on this branch, so the same fix applies here.
Tightened guard:
- ``parameters`` must be a dict (Llama-3 spec).
- ``arguments`` may be a dict, or a JSON-encoded string that
decodes to a dict (OpenAI shape, e.g.
``"arguments":"{\"q\":\"x\"}"``). Plain non-JSON strings or
JSON-strings of lists / scalars / null no longer pass.
Mirrors the fix landed in PR #5811 commit 615b8608. Adds the same
4 regression tests under TestParserMultiFormat.
Existing test suite stays green: 127 -> 131 passing.
* Studio: skip non-scalar args in python_tag JSON form
The JSON sub-path of ``_parse_llama3_python_tag`` was fabricating
``{"value": args}`` when the model emitted a non-dict / non-string
``arguments`` value (e.g. ``42``, ``[1,2,3]``, ``null``, ``true``).
This silently turned a malformed emission into a real tool call,
which the agentic loop would then execute with arguments the model
never intended.
Tightened: skip the call instead of fabricating. The same
behaviour now matches the bare-JSON guard tightened earlier
(strict-guard merge from PR #5620, inherited via merge here).
Added a regression test covering the four non-scalar shapes.
Pass count on this branch: 158 -> 159.
Sites in ``_parse_tool_call_json`` and ``_consume_mistral_call``
keep the existing looser behaviour for now; both are reached
only after explicit ``<tool_call>`` / ``[TOOL_CALLS]`` markers
so the false-positive surface there is much narrower.
* studio: fix safetensors tool-call parser gaps vs llama.cpp (Mistral CALL_ID / THINK, attribute-form signal)
Three GGUF-parity fixes to the safetensors tool-call parser, each matching
llama.cpp's reference behaviour:
- Mistral Small 3.2 emits [TOOL_CALLS]name[CALL_ID]<id>[ARGS]{json}. The
parser stopped after the name on seeing [CALL_ID] (neither [ARGS] nor {),
dropping the call. Skip an optional [CALL_ID]<id> segment in both the
parse and strip paths. llama.cpp parses this (test-chat.cpp:4785).
- Magistral wraps reasoning in [THINK]...[/THINK]. A [TOOL_CALLS] inside the
reasoning was parsed as a real call, producing a phantom call. Strip a
leading [THINK] block before scanning so only the post-reasoning call
counts (test-chat.cpp:2285); a literal [THINK] inside a later argument is
left intact.
- The standalone MiniCPM-5 / MiniMax-M2 <function name="..."> attribute form
parsed correctly but was absent from TOOL_XML_SIGNALS and the markup strip
patterns, so the streaming safety-net parse was gated off (dropping the
call) and markup leaked into displayed text. Add the signal and broaden
the strip regexes.
Adds regression tests for all three.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* studio: fix GLM and Kimi K2 safetensors tool-call parser gaps vs llama.cpp
Four GGUF-parity fixes for the GLM and Kimi K2 families:
- GLM 4.7 zero-argument inline call <tool_call>name</tool_call> was dropped:
the open-tag lookahead only allowed \n or <arg_key> after the name. Allow
</tool_call> too so a no-arg call parses to empty args (vLLM / SGLang /
llama.cpp all parse it).
- GLM string argument values were stripped, losing significant leading /
trailing whitespace in code / diff arguments. Keep the raw value for the
string fallback and only strip the copy used to probe for a JSON literal,
matching vLLM glm4_moe which never strips string args.
- Kimi K2 calls emitted without the <|tool_calls_section_begin|> wrapper
were dropped. llama.cpp makes the section optional (Kimi can call a tool
straight after reasoning without opening a section); parse a bare
<|tool_call_begin|> when no section is present.
- Kimi K2 malformed / truncated JSON in one call dropped every later call in
the section. Skip the bad call and keep parsing so valid subsequent calls
are recovered (vLLM parity).
Adds regression tests for all four.
* studio: fire safetensors tool calls for the bare-JSON (Llama-3.2) form
The agentic loop's streaming safety-net parse was gated on
has_tool_signal(), which is False for the Llama-3.1 / 3.2 bare-JSON tool
form {"name":..,"parameters":..} (no XML marker). Real tool calls were
therefore dropped: the loop logged "model planned without calling tools",
re-prompted three times, then gave up with zero tool calls, while GGUF's
llama-server parses the same emission natively.
Run parse_tool_calls_from_text() unconditionally in the safety net. The
parser is strict (only fires on a valid tool-call shape) so plain answers
are unaffected. Reproduced on a real unsloth/Llama-3.1-8B-Instruct run:
the model emits {"name":"web_search","parameters":{...}} which now
executes the tool instead of being re-prompted into a no-op.
Adds a loop regression test for the bare-JSON form.
* studio: fire safetensors tool calls for Gemma 4 (native template + stripped parser)
Gemma-4 safetensors fired no tools while its GGUF fired reliably. Three gaps:
- The Studio swaps in the Unsloth "gemma-4" chat template, which does not
render the tools schema (the model's native template does), so the model
never saw the tools. Fall back to the model's native template when the
override template renders identically with and without tools. Same fix
helps any family whose override template drops tools.
- skip_special_tokens strips the <|tool_call> wrapper and <|"|> string
markers, so a streamed Gemma-4 call arrives as a bare call:NAME{k:v, ...}
with unquoted values. Parse that form, keeping commas/braces inside a
code or command value, normalising surrounding quotes, and stripping the
leaked markup from the final answer.
- Without a grammar a small model can loop, repeating one call for the whole
tool budget. Collapse exact-duplicate calls within a turn and force a final
answer after a turn that made no new tool progress (llama-server's lazy
grammar prevents this loop on the GGUF side).
Adds parser tests for the bare/stripped Gemma-4 form.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Studio: complete strict-mode contract and fix parser import paths
Address review findings on the multi-format tool-call parser:
- Honor allow_incomplete=False in the remaining sub-parsers. The Llama-3
<|python_tag|>NAME.call(...) parser, the pre-v11 Mistral [TOOL_CALLS] array
parser, and the Gemma 4 <|tool_call> parser ignored strict mode, so a
truncated call (missing closing paren, ], or <tool_call|>) was still healed
and executed with Auto-Heal disabled. Thread strictness through and reject
the unclosed forms, matching the JSON and function-XML paths.
- Drop the duplicate tool_call_parser import block in llama_cpp.py and the
redundant un-aliased TOOL_XML_SIGNALS; only the _SHARED_TOOL_XML_SIGNALS
alias is used as a value.
- Import _strip_mistral_closed_calls from core.inference.tool_call_parser in
routes/inference.py instead of studio.backend.core... The self-contained
run.py launch mode only puts studio/backend on sys.path, so the absolute
package path raised ModuleNotFoundError on the server-tool strip path.
Add strict-mode regression tests for the truncated Llama-3 dot-call and the
unclosed Mistral array.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Studio: harden DeepSeek/Kimi tool-call parsing and strip
Address review findings on the DeepSeek and Kimi parsers:
- Honor allow_incomplete=False for DeepSeek. An envelope with no closing
<|tool▁calls▁end|> is truncated mid-stream; reject it in strict mode
instead of healing the body out to EOF, matching the strict XML and Mistral
paths.
- Do not skip a following tool call when the current call's end marker is
missing. The DeepSeek V3 and Kimi loops advanced by searching forward for the
next <|tool▁call▁end|> / <|tool_call_end|>, which could land on a later
call's end marker and drop the call in between. Advance by the JSON end; the
loop re-locates the next call marker from there.
- Strip truncated DeepSeek and Kimi section blocks in the route-level display
regex. The patterns required the closing marker; add the end-of-text
alternative so a block truncated by EOS does not leak raw markup to the UI.
Add regression tests for the truncated DeepSeek envelope, and for DeepSeek and
Kimi multi-call recovery when the first call's end marker is missing.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Studio: preserve XML param indentation and alias Mistral array parameters
Two parser-correctness fixes found by auditing against the model chat templates
and the SGLang / vLLM reference parsers:
- Qwen3.5 XML parameter values lost their leading indentation. The chat template
emits <parameter=k>\nVALUE\n</parameter>, but the parameter-start regex ate the
wrapping newline AND the value's first-line indentation with a trailing \s*,
then str.strip() removed the rest. Narrow the trailing class to horizontal
whitespace only and trim exactly one wrapping newline (via _trim_param_value),
preserving indentation in code/diff arguments. Matches SGLang's qwen3_coder
detector. Applies to both _parse_function_xml (tool_call_parser.py) and the XML
path in tool_healing.py.
- Mistral pre-v11 array objects keyed on parameters dropped their payload.
_consume_mistral_call read only the arguments key; alias parameters the same way
the JSON/XML paths and SGLang's base detector do.
Add regression tests for preserved multi-line indentation and the array
parameters alias.
* Studio: DeepSeek strip sync, Gemma nested args, GLM/Kimi strict mode
Parser-correctness fixes found by auditing DeepSeek/GLM/Kimi against vLLM,
SGLang, and the model chat templates:
- DeepSeek: the short <|tool▁calls|> opener (and the space / escaped-underscore
spellings) was parsed but never stripped, so a short-opener envelope leaked raw
markup to the UI. Share one opener alternation between _DEEPSEEK_BEGIN_RE and
the strip patterns (and the route-level display regex) so a signal we parse can
never be left un-stripped.
- Gemma wrapper-less stream: a nested object/array argument (loc:{city:NYC},
labels:[bug,ui]) was kept as a literal string. Parse it recursively when the
bare value is a balanced {} / [], falling back to the raw string for a
truncated value.
- GLM and Kimi ignored allow_incomplete. With Auto-Heal off, a GLM block with no
</tool_call>, a Kimi section with no <|tool_calls_section_end|>, or a Kimi call
with no <|tool_call_end|> are truncated and must be rejected, matching the
strict behavior of the JSON/XML/Mistral/DeepSeek paths and vLLM/SGLang.
Add regression tests for the short-opener strip, the Gemma nested args, and GLM /
Kimi strict-mode rejection.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Studio: tighten tool-call parser comments
Make the comments in the multi-format tool-call parser and its callers succinct:
compress verbose docstrings/blocks to one or two lines, drop ones that restate the
code, and trim the tiny balanced-scanner helpers. Correctness rationale and
upstream provenance (SGLang/llama.cpp parity, the strict-mode / Auto-Heal
contract, whitespace-preservation, and the Unicode / full-width-pipe notes) are
kept in compact form.
Comment-only: no code or behavior change (verified with comment_tools.py check
--strip-docstrings; parser suite green).
* Studio: tighten DeepSeek/GLM/Kimi parser comments
Compress the comments added for the DeepSeek/GLM/Kimi parsers and the Gemma
wrapper-less helpers to one or two lines, keeping the upstream provenance
(llama.cpp 51fa458a92d6), the O(N^2) / strict-mode rationale, and the vLLM parity
notes intact.
Comment-only: no code or behavior change (verified with comment_tools.py check
--strip-docstrings; parser suite green).
* Studio: make DeepSeek R1 / GLM parsing linear and close routes strip gaps
Review follow-up for the DeepSeek/GLM/Kimi parser:
- DeepSeek R1 detection used a greedy ``([^\n]+)\n```json`` regex that backtracks
O(N^2) on a fence-less truncated body; scan with str.find instead (mirrors the
V3 path).
- GLM arg pairs used a lazy-group finditer that rescanned to EOF from each bare
<arg_key> in an unclosed body (O(N^2)); walk pairs with str.find.
- The route display strip (_TOOL_XML_RE) accepted fewer DeepSeek openers than the
parser (missed the space / escaped-underscore spellings) and missed bare
section-less Kimi calls, so a call we parse could leak raw markup to the UI.
Reuse the parser's shared _DEEPSEEK_OPEN_RE_SRC and add a bare-Kimi arm.
Add ReDoS-linearity regressions for the R1 and GLM paths, a positive R1
fenced-json parse test, and routes-strip tests for the space/escaped DeepSeek
openers and the bare Kimi call.
* Studio: fix test_mcp_servers _TOOL_XML_RE reconstruction after _DS_OPEN_SRC reuse
The routes strip fix made _TOOL_XML_RE reference the module-level
_DS_OPEN_SRC variable. test_mcp_servers reconstructs the regex by exec-ing
the extracted compile() source in a namespace that only defined _re, so it
raised NameError. Inject _DS_OPEN_SRC into that namespace, matching the same
fix already applied in test_tool_xml_strip.
* Studio: make Llama-3 .call and Mistral-array healing parsing linear
Two more O(n^2) ReDoS paths in the multi-format parser, both reachable from
the agentic loop on a long truncated body with no length cap:
- _LLAMA3_KV_RE.finditer over a .call(...) body retried at every offset of a
long word run / unterminated quote (40K -> 14s). Replace with a hand-scan
that reuses the same key/number/literal sub-regexes via anchored match and
walks the string body by hand, so an unterminated quote is O(n). Verified
byte-identical to the old regex over 200K fuzzed inputs.
- _parse_mistral_array healing ran _balanced_brace_end from every { in the
body (20K -> 17s). Walk top-level objects, advancing past each balanced
{...}; this also drops the phantom call the old scan emitted from a nested
argument object.
Add adversarial-length linearity regressions plus positive .call kwargs and
unclosed-array recovery coverage.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Studio: strengthen #5624 regression assertions and strip-test harness guards
- test_strip_tool_markup_handles_deepseek_envelope used `A or B` where B was the
preservation property the next line already asserts, masking the real check.
Replace with an explicit assertion that the call name and args are stripped.
- The test_tool_xml_strip source-extraction harness reconstructs _TOOL_XML_RE and
_strip_tool_xml_for_display from routes/inference.py via lazy regexes that could
silently grab a shorter slice. Assert the extracted regex carries the DeepSeek /
bare-Kimi arms and the helper body reached the _TOOL_XML_RE.sub call.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Studio: honor strict mode in safety-net, keep empty Gemma args, strip attribute-form function XML
- safetensors safety-net parser now forwards allow_incomplete=auto_heal_tool_calls,
matching the draining path, so a late incomplete tool call is not healed and
executed when Auto-Heal is off.
- Gemma empty bare value ({k:}) now serialises as "" instead of invalid {"k":},
which previously dropped the whole call.
- Route _TOOL_XML_RE also strips the <function name="..."> attribute form
(MiniCPM-5 / MiniMax-M2) so it no longer leaks to the UI.
* Studio: linearize wrapper-less Gemma nested-arg parsing and correct parser provenance
- _gemma_parse_value/_gemma_parse_mapping/_gemma_parse_array now parse nested
{}/[] in a single forward pass instead of pre-scanning each subtree with a
balanced-brace walk and re-parsing it. Deeply nested wrapper-less Gemma args
were O(n^2); they are now ~linear (and ~40x faster at depth 400).
- Correct the DeepSeek/GLM/Kimi provenance comments: the cited commit
51fa458a92d6 is unrelated, and GLM/Kimi were never standalone
common_chat_parse_* functions (llama.cpp uses common_chat_params_init_glm_4_5
plus a generalized XML parser, PRs #15904 / #16932).
- Add tests: Gemma deep-nesting linearity, nested object/array preservation,
same-turn distinct-call cap, and the native-template tool-render fallback.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Studio: guard Gemma value parser against non-advancement and missing tokenizer
Addresses Gemini review:
- _gemma_parse_value now consumes one character when a stray }/]/, sits where a
value is expected, so _gemma_parse_array can never stall at the same index on
malformed input (a latent infinite loop).
- _render_with_native_template returns None when neither a tokenizer nor a
processor is present instead of raising AttributeError.
- Tests for both.
* Studio: fix attribute-form function-XML literal close tag and zero-arg strict call
Addresses Codex review of the <function name="..."> attribute form in
_parse_function_xml (MiniCPM-5 / MiniMax-M2):
- End the call body at the LAST </function> / </tool_call> within the call's
window, so a literal close tag inside a code/search argument (e.g.
print("</function>")) is preserved instead of truncating the call.
- Accept a closed call with no parameters as a valid zero-argument call in strict
mode (the function close is already required), instead of rejecting it as a
truncated call.
- Tests for both, mirroring the legacy <function=...> coverage.
* Studio: drop scratch review/planning artifacts from the branch
* Studio: fix tool-call parser/loop review findings on the multi-format path
Address the live code-review findings on the safetensors/MLX + GGUF tool path:
- routes: include the attribute form <function name="..."> in the safetensors
capability whitelist so MiniCPM-5 / MiniMax-M2 templates keep the tool pill
(parser already handles the form; the post-filter wrongly suppressed it).
- safetensors loop: build the plan-without-action re-prompt from the active
tools instead of a hardcoded web_search/python string, and gate it on
auto_heal_tool_calls, matching the GGUF loop.
- safetensors loop: hold a leading bare-JSON object ({"name":..,"parameters":..})
during BUFFERING until it closes, then drain it as a tool call instead of
streaming the raw JSON to clients. The DRAINING/STREAMING resolvers still
recover a plain JSON answer, so this can never drop content.
- parser: anchor the Llama-3 <|python_tag|>NAME.call(...) scan to the tag and
chain ; -separated calls, so all semicolon-separated built-ins parse and a
literal <|python_tag|>x.call(...) inside a JSON string argument no longer
fires the wrong tool.
- parser: consume the optional trailing </s> after a named Mistral
[TOOL_CALLS]name{json} call, mirroring the array shape.
- GGUF streaming strip: use the shared parser patterns (which know
[TOOL_CALLS] and <|python_tag|>) so a textual tool call entering DRAINING is
stripped instead of leaking the marker to streaming clients.
- routes: hoist the _strip_mistral_closed_calls import to module level.
Adds regression tests covering each fix; existing parser suite stays green.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Studio: fix DeepSeek/GLM/Gemma tool-call review findings
Address the live code-review findings specific to the DeepSeek / GLM / Kimi
and native-template additions:
- parser: in strict mode (Auto-Heal off) require the per-call
<|tool▁call|end|> terminator for DeepSeek V3 calls instead of executing on
a bare balanced object closed only by the envelope end.
- parser: keep GLM string arguments that begin with a quote verbatim (drop
the leading-quote case from the JSON-decode probe) so a quoted search query
is not decoded down to its inner text.
- parser: reject a GLM call with an unclosed <arg_value> in strict mode, and
under Auto-Heal keep the partial value rather than dropping it to a no-arg
call.
- parser: add a balanced wrapper-less Gemma strip (call:NAME{...}) so a nested
object/array argument is removed whole instead of leaving a trailing brace;
run the balanced Mistral and Gemma strips on the streaming display paths too.
- safetensors loop: buffer a leading wrapper-less Gemma call:NAME{...} so it
drains and executes instead of streaming the raw call text.
- inference: render the native-template fallback on a shallow tokenizer copy
instead of mutating the shared tokenizer outside the generation lock, and
load the native template from base_model for LoRA adapters.
Adds regression tests for each; existing parser suite stays green.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Studio: harden multi-format tool-call detection from review findings
Apply five targeted fixes from the review pass over the multi-format tool
path:
- routes: route display strip delegates to _strip_tool_xml so Mistral
[TOOL_CALLS] blocks with nested JSON are removed from streamed display
text, not just the XML forms.
- tool_call_parser: skip function/parameter starts that fall inside an
already-open parameter block (_inside_open_parameter) so nested example
payloads are not mis-parsed as new calls; extract
strip_llama3_leading_sentinels so the bare-JSON guard is shared.
- safetensors_agentic: probe bare JSON through strip_llama3_leading_sentinels
before the balanced-brace check so a leaked header sentinel does not defeat
the guard.
- tool_healing: allow dotted tool names in the Gemma wrapped start pattern.
- llama_cpp (GGUF): buffer wrapper-less Llama-3.2 {"name":..} calls that carry
no XML signal, drain a complete object silently and hold an incomplete one,
and run the end-of-stream safety net unconditionally so markerless calls are
detected and never leak the raw JSON (including truncated fragments).
Adds regression tests for the GGUF bare-JSON streaming path and the Mistral
display strip.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Studio: stop bare-JSON tool calls leaking at EOF, oversized, and into history
The second review pass flagged that the Llama-3.2 bare-JSON tool-call handling
still leaked raw JSON in several spots; ``strip_tool_markup`` only knows
XML/bracket markup, so the bare-JSON form survived it. Fix them symmetrically
across the safetensors and GGUF loops:
- Safetensors stream-end resolver now routes a held bare-JSON fragment to
DRAINING (mirroring GGUF) so a truncated ``{"name":..`` cut off by the end of
the stream is dropped instead of flushed as assistant content. The 7/10
reviewer finding.
- Both loops now drain (suppress) an oversized still-open bare-JSON call once it
passes ``_MAX_BARE_JSON_BUFFER`` instead of streaming the raw prefix, gated on
a ``"name"`` key so a giant plain JSON answer still streams; a complete
oversized call still executes via the safety net.
- Add a shared ``strip_leading_bare_json_call`` helper and apply it to the
content kept for the assistant turn in both loops, so an executed bare-JSON
call is not replayed as visible text or fed back as next-turn history.
Plain JSON answers without a ``"name"`` key are untouched throughout. Adds
regression tests for the EOF, oversized, and next-turn cases on both backends
plus unit tests for the helper.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Studio: bound the Llama-3 python_tag strip on real control sentinels
The route display strip's <|python_tag|> arm ran to the next <| of any kind.
A tool-call argument carrying a literal <|...|> token (for example <|cite|>
inside a string value) truncated the strip early and leaked the call tail into
the visible response. Narrow the stop condition to the genuine Llama control
sentinels (eot_id, eom_id, python_tag, start/end_header_id, begin_of_text,
finetune_right_pad_id) so embedded markup and JSON are consumed while real
header/turn boundaries still bound the strip.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Studio: harden GLM/Gemma parsing, cap GGUF textual calls, share native-template fallback
GLM 4.x parser walked a body pre-bounded by the first </tool_call>, so a string
argument containing a literal </tool_call> (e.g. code that prints it) was
truncated. Walk arg_key/arg_value pairs against the full content instead, since
each <arg_value> is delimited by its own </arg_value> and the call's real close
is the </tool_call> that precedes the next <arg_key>.
Add a truncated wrapper-less Gemma pattern (call:NAME{... with no closing brace)
to the markup strip so a call cut off mid-arguments does not leak raw into the
visible stream. It runs after the closed form, so a complete call keeps trailing
prose.
Cap and dedup tool calls parsed from the GGUF TEXTUAL fallback at
_MAX_TOOL_CALLS_PER_TURN, mirroring the safetensors loop. Structured
delta.tool_calls are grammar-bounded by llama-server, but text parsed straight
from content is not, so one runaway turn could fan out into dozens of
executions.
Extract the native-chat-template fallback into chat_template_helpers
(render_native_template / render_with_native_template_fallback) so the
transformers and MLX text backends share one implementation. The MLX text path
now applies it too, so an Unsloth override template that drops the tools schema
no longer silently stops MLX from advertising tools. The MLX VLM path renders
via the processor for image tokens and is intentionally left on its own render.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Studio: gate markerless bare JSON on enabled tools and close parser/strip asymmetries
The Llama-3.2 custom_tools bare-JSON form has no marker, so any JSON object with a
name key was read as a tool call. An ordinary JSON answer like
{"name":"Alice","parameters":{"age":30}} was misclassified as a call to a
disabled tool and dropped from the visible response. Gate the markerless form on
the enabled tool names (threaded through parse_tool_calls_from_text and
strip_leading_bare_json_call, supplied by both streaming loops): an object whose
name is not an enabled tool is ordinary content. The marker-based forms keep
their name-agnostic behaviour (an explicit signal is a real call attempt), and
unrestricted mode stays ungated.
Also fix two parser/strip asymmetries the parser already tolerated:
- A literal </function> inside a parameter value (print("</function>")) truncated
both the core and route strips at the first close, leaking the tail. Extend the
strip to the call's real close (last </function> before the next opener),
mirroring the parser, without merging separate calls.
- The single-object Mistral [TOOL_CALLS]{...} shape parsed but _strip_mistral_closed_calls
left it, leaking the raw object into display. Strip the balanced object while
keeping trailing prose, matching the array and name shapes.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Studio tools: fix strip/parse symmetry and native-template token for DeepSeek/GLM/Kimi
Pass-3 review follow-ups on the multi-format tool parser:
- Bare Kimi call (<|tool_call_begin|>...<|tool_call_end|> with no section
wrapper) is accepted by the parser, so add it to the closed strip patterns
so the streaming (non-final) display strip removes it instead of leaking the
markup mid-generation.
- Route display strip now also runs the wrapper-less Gemma cleanup, so a
Gemma 4 call:NAME{..} no longer leaks into the visible answer.
- MLX model record carries base_model for a LoRA adapter so the native-template
fallback loads the base repo template rather than the adapter's
(often template-less) tokenizer.
- Native-template reload forwards the load-time HF token so a gated/private
model's repo template can still be fetched (transformers and MLX text paths).
- GGUF end-of-stream bare-call heuristic is gated on the enabled tool names so a
truncated ordinary JSON object ({"name":"Alice","age":) streams as the answer
instead of being dropped as a tool call.
Adds regression tests for each case.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Studio tools: gate GGUF bare-JSON suppression on enabled tools and fix python-tag exponent parsing
Pass-4 review follow-ups on the GGUF tool loop and Llama-3 parser:
- The GGUF bare-JSON suppression sites still keyed off a raw "name" substring,
so an ordinary JSON answer whose name is not an enabled tool was dropped when
it was truncated, oversized, or reached the no-tool DRAINING fallback (the
parser, helper, and safetensors paths were already gated). All three sites now
use the shared enabled-name gate, and a held bare-JSON buffer that turns out not
to be an enabled call is shown as the answer instead of dropped at stream end.
- The Llama-3 python-tag numeric kwarg regex matched only the mantissa, so
scientific notation was truncated to its leading digits (1e-3 parsed as 1) and a
tool executed with the wrong value. The regex now accepts exponent and decimal
forms, and the int/float classification keys off the exponent too.
Adds regression tests for the truncated / oversized disabled-name JSON cases (and
a counterpart that a truncated enabled call still does not leak) plus the
scientific-notation kwargs.
* Studio: drop accidentally committed async worker transcripts
Eight generated reviewer / async-worker transcripts were committed under
studio/backend/async_task_outputs/. They are not imported or referenced by any
code and carry only internal task state, so they should never ship in the repo.
Remove them and gitignore the directory so they cannot be re-added.
* Studio tools: gate safetensors bare-JSON drain, fix nested-name gate and function-XML strip
Pass-4 review follow-ups on the shared parser / safetensors loop:
- The safetensors oversized and end-of-stream bare-JSON drain branches keyed off
a raw "name" substring, so a large or truncated ordinary JSON answer whose name
is not an enabled tool was drained instead of streamed. Both now use the shared
enabled-tool-name gate, matching the GGUF path.
- strip_leading_bare_json_call matched the first "name" anywhere, so a plain JSON
answer with a nested name equal to an enabled tool ({"result":{"name":"web_search"}})
was wrongly suppressed. It now extracts the TOP-LEVEL name only, walking past
nested objects/arrays and keeping the text when a top-level value is truncated.
- The function-XML display strip used a regex negative-lookahead that stopped at a
literal <function=...> opener inside a parameter value and then dropped the rest
of the answer to EOF. A scan-based strip mirrors the parser (ignores openers
inside an open <parameter> via _inside_open_parameter) and closes each call at its
real </function>, so trailing assistant text after such a call survives.
Adds regression tests for each.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Studio: keep tools prompt when native-template probe raises; make helper tests hermetic
Pass-4 review follow-ups on the native-template fallback:
- render_with_native_template_fallback re-renders the live template with tools=None
to detect whether it dropped the schema. A template that requires tools can raise
on that probe; that must not discard the already-valid tools prompt. The probe is
now wrapped so any error returns the original formatted_prompt (transformers would
otherwise fall back to manual formatting and lose the schema; MLX would let the
exception escape).
- The native-template helper tests imported InferenceBackend just to reach the
thin wrapper, which pulls in unsloth and its optional vllm package metadata. They
now call the dependency-light render_native_template helper directly so they pass
in a backend/test environment without vllm. Adds a probe-raises regression test.
* Tool parsing: 3.9 import safety, disabled-Auto-Heal contract, capability gate
Round-2 review follow-ups on the multi-format tool-call parser:
- tool_call_parser: add `from __future__ import annotations`. The module
is dependency-light by design (external llama-server wrappers import it
standalone) and the package targets python >=3.9, where its PEP 604
`int | None` return annotations would raise TypeError on import.
- safetensors + GGUF drain fallback: gate the leading bare-JSON strip on
auto_heal_tool_calls. With Auto-Heal off, a truncated enabled-name
fragment that did not parse now stays visible, matching the XML strip
in the same branch and the disabled-Auto-Heal contract. With Auto-Heal
on it is still suppressed.
- safetensors capability gate: match the bare-JSON `{"name":` template
marker with a whitespace/escape-tolerant regex so a pretty-printed
`{ "name" :` or JSON-escaped `{\"name\":` template is not mis-classified
as tool-less. The parser already accepts that whitespace via
raw_decode, so the gate must too.
Regression tests added for each case.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* GLM tool-call display strip: treat literal close tag in arg value as data
Round-2 review follow-up on the GLM 4.x tool-call format.
The GLM call shape is <tool_call>NAME<arg_key>k</arg_key><arg_value>v
</arg_value>...</tool_call>. The parser was hardened to walk arg_key /
arg_value pairs so a literal </tool_call> inside an argument value (e.g.
print("</tool_call>")) is treated as data and the call's real close is the
</tool_call> that precedes the next <arg_key>. The display strips still used a
non-greedy <tool_call>.*?</tool_call> regex, which stopped at the literal and
leaked the call's tail into visible content and stale history.
Add _strip_glm_calls, a scan that mirrors the parser's close detection, and run
it before the regex arms in every strip pipeline: the core strip_tool_markup,
the route _strip_tool_xml display/history cleanup, and the safetensors + GGUF
streaming strips. Qwen / Hermes <tool_call>{json} has no NAME token after the
opener, so it is left to the regex arms unchanged.
Regression tests cover the literal-close-tag leak (core + route), normal GLM
calls, back-to-back GLM calls, zero-arg GLM, truncated GLM, and untouched Qwen.
* Tool parsing: symmetric "function" bare-JSON alias and route strip parity
Round-3 review follow-ups, all parser/strip symmetry fixes.
- Bare-JSON "function" alias: the markerless parser accepts a call name via
obj.get("name") or obj.get("function"), but the strip/gates only knew "name",
so a {"function":<enabled tool>} call executed while its raw JSON leaked. Teach
_top_level_bare_json_name the alias (with "name" precedence and the same nested
and truncated-name guards), and widen the guards in strip_leading_bare_json_call,
the safetensors and GGUF _looks_like_enabled_bare_json gates, and the route
capability marker regex.
- Route display/history cleanup: strip a tail-only </param> alias close (the
parser accepts <param name="...">...</param>), and run the parser's guarded
function-XML scan (_inside_open_parameter) before _TOOL_XML_RE so a literal
nested <function=...></function> inside an argument value does not truncate the
strip and leak the tail.
Regression tests added for each.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Studio tools: fix DeepSeek strict recovery, Kimi dotted names, Gemma spaced streaming
Round 3 review fixes for the DeepSeek / GLM / Kimi tool-call parsing path.
- DeepSeek R1 and V3/V3.1 strict parsing (Auto-Heal off): when a call is
truncated (missing closing fence or <tool_call_end> terminator), skip it
and keep scanning for later well-formed calls instead of breaking out and
dropping the rest of the envelope. This matches the Kimi strict parser's
recovery behaviour.
- Kimi dotted tool names: keep the full name after stripping only the
functions. prefix and :idx suffix, e.g. functions.mcp.server-list:0 stays
mcp.server-list. The previous split on "." truncated dotted MCP names to
their last segment. This matches current vLLM
(tool_id.split(":")[0].removeprefix("functions.")) and SGLang
(^(?:functions\.)?(?P<name>[\w.\-]+):(?P<index>\d+)$).
- Gemma wrapper-less call streaming: hold the whitespace-tolerant prefix
(call : NAME) in the streaming suppression buffer, matching the parser's
_GEMMA_BARE_TC_RE, so the spaced spelling split across chunks is buffered
instead of leaking as visible text. Applied to both the safetensors and
llama.cpp streaming paths.
- Remove dead _render_with_native_template method and the now-unused copy
import from inference.py; the live path uses render_with_native_template_fallback.
Adds regression tests for DeepSeek R1/V3 strict recovery, Kimi full dotted
name preservation, and the Gemma spaced-call streaming suppression.
* Studio tools: honor tool budget in GGUF loop and guard function-XML streaming strip
Round 4 review fixes. Both are asymmetric-fix bugs where the final/steady path got a
guard the analogous streaming/loop path did not.
- GGUF tool-call budget: the safetensors loop counts real tool-call turns against
max_tool_iterations (re-prompt stalls excepted), but the GGUF loop only bounded the
turn count by the enlarged range (max_tool_iterations + _MAX_REPROMPTS). Since this
PR raised _MAX_REPROMPTS from 1 to 3, a model that keeps making valid tool calls
could run up to three extra tool rounds (with max_tool_iterations=1, four rounds
instead of one). Add a _tool_iters_done counter that increments only when a tool
actually executed in the turn, and stop once the caller's budget is spent so the
post-loop final-answer nudge fires. A duplicate/disabled no-op turn is a correction
turn (like a plan-without-action re-prompt) and does not consume budget, preserving
the existing "already completed" re-prompt behavior.
- Streaming display strip: the final strip runs the guarded _strip_function_xml_calls
scanner (a literal <function=...> inside a parameter value is data, not a nested
call), but the GGUF and safetensors streaming strips still used only the open-ended
regex arms. When a tool-call argument contained literal function markup, the regex
tail ate everything to end-of-text and dropped the real trailing prose after the
call's true </function>. Run the guarded scanner (and the balanced Mistral strip)
before the regex arms in both streaming paths so streaming and final display agree.
Adds regression tests: GGUF valid tool calls respect max_tool_iterations, and the
streaming strip keeps trailing prose after a function-XML call with a literal marker.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Studio tools: safetensors tool budget counts only executed turns (GGUF parity)
Follow-up to the GGUF budget fix. The safetensors loop charged max_tool_iterations
per non-re-prompt iteration (iteration + 1 - reprompt_count), so a duplicate/disabled
no-op turn spent a budget slot even though no tool ran. With a small cap this dropped
real work: for max_tool_iterations=2, a model that made a valid call, repeated it (an
internal no-op correction turn), then made a distinct valid call executed only the
first -- the third turn was sent with no tools and the distinct call was ignored.
Track whether a turn actually executed a tool (set on record_result) and count only
those turns against the cap, matching the GGUF loop. A duplicate/disabled no-op is a
correction turn -- like a plan-without-action re-prompt -- and no longer consumes
budget, so the model still gets its "already completed" nudge and another tool-enabled
turn. Adds a regression test for the small-cap duplicate-then-distinct-call flow.
* Studio tools: fix stale Kimi dotted-name regression test
test_pr5624_regressions.py still expected functions.my.tool:0 to resolve to the last
segment (tool). The parser now preserves the full dotted name (my.tool) after removing
only the functions. prefix and :idx suffix, matching current vLLM/SGLang so dotted MCP
names like mcp.server-list survive. Update the assertion, name, and module docstring to
the corrected contract (the raw id is still preserved on the call).
* Studio: render the reasoning block for safetensors and MLX like GGUF
enable_thinking chat templates (Qwen3/Qwen3.5/GLM) prefill an unclosed <think>
into the generation prompt, so the model emits only the closing </think> then
the answer. The safetensors/MLX chat stream emitted that as plain content, so
the reasoning showed inline with no collapsible thinking block, while GGUF
(which surfaces reasoning via reasoning_content) rendered one. This brings
safetensors and MLX to parity.
- _ResponsesReasoningExtractor gains a reasoning_prefilled mode that starts
inside the reasoning block and splits on the first </think>; default False
keeps GGUF and every existing caller byte-identical. It suppresses a stray
re-emitted <think> and holds partial markers back across chunk boundaries.
- _sf_reasoning_prefill_mode gates the mode on reasoning being enabled for the
request, an enable_thinking or enable_thinking_effort style, and the template
actually using the standard <think>/</think> markers. Models with a bespoke
reasoning channel (e.g. gemma's <|think|>/<|channel>) are excluded so their
answer is never swallowed; gpt-oss (Harmony) and thinking-off requests are
excluded too.
- sf_tool_stream and stream_chunks (the latter also serves MLX) feed text
through the extractor, emitting reasoning_content then content deltas, with a
per-turn reset in the tool loop and a flush before each tool_start; only the
visible delta reaches the monitor reply. The two non-streaming drains split
reasoning_content the same way.
- Tests: extractor prefilled mode (streaming and edge cases), the gate matrix
including the gemma-style exclusion, and a route-replay of the tool-loop
reasoning stream.
* Studio: render the reasoning block for safetensors and MLX like GGUF
enable_thinking chat templates (Qwen3/Qwen3.5/GLM) prefill an unclosed <think>
into the generation prompt, so the model emits only the closing </think> then
the answer. The safetensors/MLX chat stream emitted that as plain content, so
the reasoning showed inline with no collapsible thinking block, while GGUF
(which surfaces reasoning via reasoning_content) rendered one. This brings
safetensors and MLX to parity.
- _ResponsesReasoningExtractor gains a reasoning_prefilled mode that starts
inside the reasoning block and splits on the first </think>; default False
keeps GGUF and every existing caller byte-identical. It suppresses a stray
re-emitted <think> and holds partial markers back across chunk boundaries.
- _sf_reasoning_prefill_mode gates the mode on reasoning being enabled for the
request, an enable_thinking or enable_thinking_effort style, and the template
actually using the standard <think>/</think> markers. Models with a bespoke
reasoning channel (e.g. gemma's <|think|>/<|channel>) are excluded so their
answer is never swallowed; gpt-oss (Harmony) and thinking-off requests are
excluded too.
- sf_tool_stream and stream_chunks (the latter also serves MLX) feed text
through the extractor, emitting reasoning_content then content deltas, with a
per-turn reset in the tool loop and a flush before each tool_start; only the
visible delta reaches the monitor reply. The two non-streaming drains split
reasoning_content the same way.
- Tests: extractor prefilled mode (streaming and edge cases), the gate matrix
including the gemma-style exclusion, and a route-replay of the tool-loop
reasoning stream.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* studio: don't force a tool re-prompt on a negated intent (safetensors parity)
The safetensors _INTENT_SIGNAL claimed to mirror GGUF but was missing the
negative lookahead, so a refusal like "I will not search the web for that"
matched the "i will" intent and triggered the plan-without-action re-prompt
(STOP... you MUST call a tool), overriding a valid no-tool answer. GGUF already
excludes not/never. Add the same (?!\s+(?:not|never)\b) lookahead so both
backends agree. Extends the intent parity test with negated refusals.
* studio: parse the outer envelope before DeepSeek/Kimi markers embedded in its args
parse_tool_calls_from_text ran the DeepSeek/Kimi marker pre-pass before the shared
<tool_call>/<function=...> parser. When a Qwen/Hermes call's argument contained
literal Kimi/DeepSeek markup (for example a user asking the model to explain that
syntax), the pre-pass matched the embedded marker and returned it, executing the
wrong tool and dropping the real call. Skip the pre-pass when a <tool_call> or
<function=...> envelope opens before the first DeepSeek/Kimi marker, so the shared
parser takes the outer call; a genuine marker-led call (no leading envelope) still
goes through the pre-pass. Tests for the embedded-marker case and the control.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Studio: trim redundant comments (comment-only, AST-verified)
* Studio: trim redundant comments (comment-only, AST-verified)
* Studio: prevent Gemma tool-parser DoS on stray delimiters
_gemma_parse_value returned the input index unchanged when text[i] was a
stray delimiter (,}]), so the list and mapping caller loops that advance
on the returned index spun forever at 100% CPU on malformed input such as
[},]. Advance past the delimiter so parsing always terminates.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Studio: strip Magistral [THINK] reasoning from final display/history
strip_tool_markup removed [TOOL_CALLS] and <function> markup but left a
leading Magistral [THINK]...[/THINK] block intact, so its bracket-form
reasoning (not the <think> the reasoning channel renders) leaked into the
safetensors display and conversation history while GGUF/llama.cpp routes
it natively. Drop the leading reasoning block at end-of-turn (final=True)
via the existing _strip_mistral_reasoning helper; streaming is untouched.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Studio: keep times in wrapper-less Gemma tool arguments
The wrapper-less Gemma value scanner used _GEMMA_KEY_RE = [\w.\-]+ for keys,
which also matches a digit-leading token, so a comma followed by a time or
ratio inside a value (call:web_search{query:meet at 10:00, 11:00 tomorrow})
was misread as a new 11: key, truncating the query and injecting a bogus
argument. Require keys to start with a letter or underscore, matching the
identifier-start rule the wrapped path already uses (_GEMMA_NEXT_KEY_RE).
Add a regression test.
* Studio: treat markers/close-tags inside tool-call arguments as data
Four parser correctness fixes where a valid argument string was mistaken for
structure:
- DeepSeek: find the envelope-end token outside JSON strings, so a query/code
argument containing the literal token no longer truncates the body and drops
the whole call.
- GLM: locate the real </arg_value> as the one whose next token is <arg_key> /
</tool_call> / end, so a value containing a literal </arg_value> (or
</tool_call>) is kept instead of executing the tool with corrupted arguments.
- Attribute-form <function name="..."> envelopes now count in the embedded-marker
guard, so a DeepSeek/Kimi marker inside a parameter value does not hijack the
outer call and run the wrong tool.
- Wrapper-less Gemma call:NAME{...} is gated on the enabled tool names (parse and
display strip), mirroring the Llama bare-JSON gate, so a disabled/example name in
prose is not stolen as a call and the real answer is preserved.
Add regression tests for each.
* Gate route Gemma wrapperless strip by enabled tools; make Kimi section-end search string-aware
Route-level display stripping now threads the enabled tool-name set into the
Gemma wrapperless-call strip, so prose that mentions a disabled tool
(call:foo{...}) is preserved while active tool calls are still stripped. This
mirrors the parser-level gate already used in tool_call_parser.
The Kimi section-end lookup now searches outside JSON string literals, so a
section-end marker appearing inside an argument string no longer triggers a
false truncation that drops a valid tool call.
* Run DeepSeek/Kimi pre-pass when a closed tool-call example precedes a real block
The marker pre-pass was skipped whenever any <tool_call>/<function> opener
appeared before the first DeepSeek/Kimi marker, even when that opener was a
CLOSED syntax example in prose that ends before the real block. In that case
parse_tool_calls_from_text skipped the DeepSeek/Kimi parsers and the genuine
tool call was dropped while a phantom tool named in the example ran instead.
Only treat a marker as embedded in a leading envelope when removing the closed
outer <tool_call>/<function> envelopes also removes every marker (the marker
actually sat inside one). A marker left standing is a real call, so the pre-pass
runs. The legitimate case of a marker inside a closed outer envelope's arguments
is preserved.
* Honor reasoning_effort none in safetensors prefill; strip Magistral reasoning while streaming
Two safetensors/MLX reasoning fixes surfaced in review:
_sf_reasoning_prefill_mode only checked enable_thinking, so an
enable_thinking_effort (GLM-5.2) request that disables thinking via
reasoning_effort=none (without enable_thinking=False) still began in
prefilled-<think> mode. A plain answer with no </think> was then swallowed
whole into reasoning_content and the visible response came back empty. Thread
reasoning_effort into the predicate and treat none as disabled, mirroring
_request_reasoning_kwargs.
strip_tool_markup_streaming stripped tool markup but not the leading Magistral
[THINK]...[/THINK] bracket block, so the raw chain-of-thought leaked into the
streamed safetensors content instead of the reasoning drawer (GGUF routes it
natively). Apply _strip_mistral_reasoning first, matching the final strip; an
unclosed [THINK] is held from the marker on so nothing flickers.
* Heal truncated outer tool envelopes and keep quoted Gemma args intact
Two follow-ups from review of the marker pre-pass and Gemma parsing:
The leading-envelope guard only removed CLOSED outer <tool_call>/<function>
envelopes before deciding whether a DeepSeek/Kimi marker was embedded, so a
truncated outer call missing its close tag (whose argument embeds a marker) was
treated as a standalone marker and the embedded sample ran instead of the
intended outer call being Auto-Healed. Decide on the last outer opener before the
marker and whether it closed before the marker instead, so a closed syntax
example still runs the pre-pass while a real closed-or-truncated outer call keeps
it.
The wrapper-less Gemma argument scan tracked bracket depth but not quotes, so a
quoted value containing a comma followed by a key-like token (a search query such
as "weather, location: Boston") was split mid-string, truncating the value and
fabricating an extra argument. Track quote state (with escapes) so the top-level
comma boundary is only taken outside quoted spans.
* Span outer envelopes to their real close when locating embedded markers
Locating the DeepSeek/Kimi marker relative to a leading outer envelope used the
FIRST close tag after the opener, so a literal </function> or </tool_call> inside
an argument value (for example python code that contains the text) was mistaken
for the envelope boundary. The marker after it was then treated as a standalone
call and the embedded sample ran instead of the intended outer call.
Match the closed outer envelopes with the shared patterns that already extend to
the real final close (a literal close inside a value is data), and treat a marker
that survives their removal as embedded only when a still-open (truncated) outer
opener precedes it, so Auto-Heal still repairs a truncated outer call. A closed
syntax example before a genuine block still runs the pre-pass.
* Span the tool_call outer envelope to its real close in the marker guard
The leading-envelope check reused the lazy <tool_call>.*?</tool_call> strip
pattern, so a Qwen/Hermes JSON argument containing a literal </tool_call> ended
the span early. A DeepSeek/Kimi sample later in that same string then survived
the closed-envelope removal, and the pre-pass executed the embedded call instead
of the outer <tool_call>. The <function> arm already spanned to its real close;
give <tool_call> the same real-close pattern (with the negative lookahead that
keeps back-to-back calls separate) so a literal close inside a value is data.
* Preserve no-tool Gemma prose and keep later R1 calls when healing a close
Two review follow-ups:
_gemma_strip_gate returned None when no tools were enabled, and None means
strip every markerless call:NAME{...} block, so a no-tool answer that documents
the syntax (or the Anthropic display path, which passes an empty tool list as
None) had that prose deleted. It is a display/history gate, so return the
enabled-name set instead -- an empty set when no tool is enabled, which strips
nothing because every call:NAME{...} is then prose.
The DeepSeek R1 heal path located the close fence with an unbounded forward
search, so when a first call had balanced JSON but omitted its fence the search
landed on a LATER call's terminator and pos advanced past that valid call,
dropping it. Match the close immediately after the JSON (whitespace-skipped) like
the strict path, and advance by just the JSON when it is absent, so a multi-call
turn keeps its later well-formed calls (heal is now a superset of strict).
* Resume wrapper-less Gemma scan past a consumed call's balanced body
The markerless call:NAME{...} scan used finditer, which resumes right after the
opening call: token, so a nested call:OTHER{...} mentioned inside the first
call's own quoted string argument (for example a web_search query that quotes the
Gemma tool syntax) was re-matched and returned as a spurious second tool call,
executing an unintended tool. Walk with a manual cursor that resumes after the
outer call's balanced body (brace matching already skips quoted braces), so a
call's arguments are never rescanned. Genuinely separate back-to-back calls and
disabled/example prose are unaffected.
* Mistral outer call wins over XML literals; align healer signals with its parser
Two follow-ups on the shared-parser ordering after the healing-passthrough
merge:
- A well-formed [TOOL_CALLS] call whose JSON arguments quote tool XML parsed
the literal instead of the outer call (executing the wrong tool). When the
first XML signal sits inside a leading balanced Mistral body it is argument
data, so the Mistral parser now runs first; an XML signal before the trigger
keeps the normal order, so a [TOOL_CALLS] literal inside an XML call's
arguments still stays data.
- passthrough_healing buffered streams on the parser module's broadened signal
list (now including <|python_tag|> and [TOOL_CALLS]) but promotes with
core.tool_healing, which does not parse those forms: a streamed Mistral or
Llama text call was held until finalization and flushed as prose. The healer
keeps its own signal list limited to the formats it can promote, restoring
immediate streaming for the rest.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Address review: Gemma wrapper-less marker literals and quotes, GLM embedded close pair
- The Gemma fallback deferral now keys on an actual wrapped opener
(_GEMMA_TC_RE), not the wrapper literal anywhere in content: a wrapper-less
call whose argument merely mentions <|tool_call> has nothing tool_healing
can parse, and deferring it lost the call entirely (not executed and
stripped from display).
- New _gemma_body_brace_end boundary scanner honors single- and double-quoted
strings like _gemma_parse_stripped_body, shared by parse and strip, so a
quoted brace in a code argument (code:print('}')) no longer truncates the
executed arguments or the strip span.
- _glm_value_close now requires a structural </arg_value> to sit at balanced
quote state: the full pair </arg_value></tool_call> embedded inside a string
literal is data, not an early close. When no candidate balances, the first
token-valid close wins as before.
* Address review: leading envelopes win over rehearsed literals
- New _first_foreign_tool_signal shared by the leading-envelope guards adds
<|python_tag|> to the protected signal set: the spelled-out literal inside a
Mistral call's arguments (a query about Llama built-in tool syntax) executed
the inner literal instead of the outer call.
- New _xml_signal_inside_leading_bare_json guard, sibling of the Mistral one:
a leading bare-JSON call whose string argument quotes tool XML (a code value
citing <function=...>) had the literal promoted by the shared XML pass
before the bare-JSON parser ran.
- Magistral [THINK]...[/THINK] is dropped once at parse entry instead of only
inside the Mistral parser, so a call rehearsed in the think block in a
foreign format can no longer be promoted while the real call after the
block is lost. Parse now agrees with the display strip.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Address review: a disabled leading bare-JSON object keeps its literals as data
When the leading bare-JSON object is ordinary content (name not an enabled
tool), the guard proved the first tool signal sits inside it, so falling
through to the XML/python_tag passes promoted quoted string data as a real
call. Drop the object and parse only the tail: a real call after the object
still parses, nothing inside it can be promoted.
* Address review: apostrophes in raw Gemma values, GLM strict key contract, per-model template token
- Quote openers in the wrapper-less Gemma boundary and body scanners now
require value-start context (after : { [ ( , =): an apostrophe inside an
unquoted value (query:what's the weather) opened quote mode, swallowed the
real closing brace, and lost the whole call on common contraction queries.
Quoted values keep hiding delimiters as before.
- A GLM <arg_key> with no <arg_value> tag now rejects the call in strict
mode, matching the unclosed-value contract, instead of executing the tool
with the argument silently dropped; Auto-Heal keeps the lenient skip.
- The native-template fallback reads the hf_token stored on the model record
instead of the instance-wide last-load token, so a later token-less load
cannot break template fetches for a previously loaded gated model (both
the transformers and MLX backends).
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Address review: Mistral literals inside leading JSON, whitespace-tolerant wrapped Gemma opener
- The leading bare-JSON guard now treats the [TOOL_CALLS] trigger as a
foreign signal: the Mistral parser runs before the bare-JSON one, so a
literal quoted inside the leading object's strings was promoted over the
outer call (or over ordinary JSON content).
- tool_healing's wrapped Gemma opener tolerates whitespace around call and
the colon: sampling drift emits call: name{ and call : name{, and
rejecting those lost the call entirely because no fallback re-parses the
wrapped form. Strict mode still requires the closing tag.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Address review: DeepSeek/Kimi markers inside leading JSON and Mistral envelopes stay data
The DeepSeek/Kimi pre-pass runs before the outer-call parsers, and
_marker_inside_leading_envelope only protected XML envelopes: a marker
quoted inside a leading bare-JSON or Mistral call's argument strings was
promoted as a separate no-arg call and the real outer call dropped. The
guard now recognizes those two leading envelopes as well; standalone
DeepSeek/Kimi calls keep parsing.
* Address review: accept dotted Gemma argument keys in the key-quoting scanner
The scanner quoted keys of [alnum_-] only, so a dotted key (user.name:...)
was left unquoted, json.loads failed, and the whole wrapped call was lost
(parse empty, strip wipes the markup). Dots now match the parser's own
key/name charset.
* Address review: a real DeepSeek/Kimi call after a disabled leading JSON object still parses
DeepSeek/Kimi markers are foreign signals for the leading bare-JSON guard
too: a marker literal inside a disabled leading object made the envelope
guard skip the pre-pass for the whole message, so a real DeepSeek/Kimi call
after the object was dropped. Routing the case through the guard's
drop-and-parse-the-tail recursion reaches the real call while the literal
inside the object stays data.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Address review: leading Mistral call owns the turn, dotted keys after bare values
- A LEADING parseable [TOOL_CALLS] call now runs the Mistral parser first
unconditionally: literal XML in trailing prose after the call was promoted
by the earlier shared XML pass, executing the quoted example instead of
the real leading call. XML leading keeps the normal order.
- _GEMMA_NEXT_KEY_RE accepts dots so a dotted key after a bare value
(query:foo,user.name:bob) ends the value at the comma instead of being
swallowed into it, matching the round-earlier key-quoting charset.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Address review: a leading wrapper-less Gemma call owns the turn
A quoted foreign literal inside a leading wrapper-less Gemma call's
argument (a query citing another tool syntax) was promoted by tool_healing
before the Gemma fallback ran, executing the quoted example and dropping
the outer call. New leading guard, sibling of the Mistral and bare-JSON
ones, gated on an enabled name since the form is markerless. Foreign markup
leading keeps the normal order.
* Fix merge resolution: restore both leading-guard test classes intact
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Address review: markup quoted inside a nameless leading JSON answer stays data
The leading bare-JSON guard required a top-level name, so a structured JSON
answer quoting tool markup in its strings (a response_format turn
documenting a tool's syntax) had the literal promoted by the later passes.
A nameless leading object that parses as real JSON now routes through the
same decline-then-parse-the-tail path; non-JSON braced prose keeps the old
behaviour, and a real call after the answer still parses.
* Address review: JSON answers stay data, nested Gemma quotes, earliest envelope, no failure caching
- A whole-content JSON value is a structured answer: the markerless Gemma
scan and its strip no longer promote or strip a quoted example of an
enabled tool's syntax inside it.
- Nested stripped-stream Gemma values now unquote quoted string leaves
recursively, so {loc:{city:"New York"}} hands the tool New York, matching
the top-level coercion.
- The DeepSeek/Kimi pre-pass dispatches by earliest envelope opener, so a
leading real call wins over a trailing example of the sibling format in
either direction.
- A failed native-template fetch is no longer cached as no-template: the
next call retries after the model record's token is fixed or a transient
Hub error clears; only definitive loads are cached.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Address review: closed calls precede the marker pre-pass, truncated Gemma scan stops, quoted nested delimiters
- A closed non-DeepSeek/Kimi call preceding the first DS/Kimi marker owns
the turn: a trailing syntax example, or one quoted inside a wrapped Gemma
argument, was promoted by the pre-pass and dropped the real leading call.
Wrapped Gemma joins the outer-envelope pattern sets.
- An unbalanced wrapper-less Gemma call now stops the scan (mirroring the
strip contract) instead of resuming inside its own argument text, where a
quoted enabled call would be promoted.
- Raw-quoted strings in nested stripped-stream Gemma values hide delimiters,
so {city:"New, York"} is one value instead of a split pair, returned
unquoted like the top-level coercion.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Address review: string-marker literals in wrapper-less args, mid-value quoted phrases
- The wrapper-less deferral guard no longer keys on the <|"|> literal: a
real call whose argument merely mentions the string marker was deferred to
tool_healing, which has no wrapped opener to parse, losing the call. The
wrapped-opener check alone owns the deferral.
- Double quotes now also open at the start of a word, so a quoted phrase
mid-value (query:find "weather, location: Boston", limit:3) hides its
delimiters instead of splitting the value into garbage keys; apostrophes
keep the value-start-only rule so contractions stay prose.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Address review: strict GLM refuses in-quote close fallback, Gemma guard covers preambles
- _glm_value_close gains a strict flag: a truncated value whose only close
candidates sit inside a string literal rejects the call in strict mode
(Auto-Heal keeps the lenient partial), restoring the strict contract the
quote-aware fallback had weakened.
- The leading wrapper-less Gemma guard no longer requires the call to open
the response: a visible preamble before call:NAME{...} is the normal
shape, and the quoted foreign literal inside the argument was promoted
again in that shape. An enabled balanced call beginning before the first
foreign signal owns it.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Address review: contextual GLM quote openers, disabled Gemma examples stay prose, JSON array answers
- The GLM value-close quote tracker uses the same contextual openers as the
Gemma scanners (single quote after punctuation context, double quote also
at word start), so strict mode accepts a normal apostrophe value again
while still rejecting a truncated value whose only close candidates sit
inside a string literal.
- A disabled wrapper-less Gemma call is prose by design, so a tool literal
quoted inside it no longer promotes: the span is dropped for parsing and
the tail parsed, mirroring the nameless-JSON guard.
- Leading JSON ARRAY answers join the leading-JSON envelope guard, so a
marker quoted inside a structured array response stays data.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Align closed-envelope regression test with the document-order contract
The test asserted the pre-round-13 behavior (trailing DeepSeek/Kimi block
wins over a leading closed envelope) while the shipped rule is document
order: the leading closed call owns the turn. Rename the test and assert
the leading call so the suite matches the contract exercised by
test_leading_xml_call_wins_over_trailing_kimi_example.
* Parse a leading Llama-3.2 bare-JSON call before the markerless Gemma scan
The bare-JSON form only ever matches a leading call object, and document
order says that call owns the turn. Running the Gemma wrapper-less scan
first let an enabled call:NAME{...} snippet quoted inside the leading
call's string arguments steal the turn when the JSON was not the whole
content (trailing prose or a second ;-separated call), executing the
quoted tool instead of the real one. Reordering cannot take a leading
Gemma call's turn since that content never starts with an object brace.
* Leading-call ownership: Mistral trigger in Gemma guards, closed bare JSON before markers, depth-aware nested Gemma values
Three parser gaps against the document-order contract:
The wrapperless Gemma leading guards did not count [TOOL_CALLS] as a
foreign signal, so a leading Gemma call quoting a Mistral snippet in its
argument lost the turn to the quoted literal. Both the enabled-call and
disabled-example guards now include the trigger, matching the bare-JSON
guard's local inclusion.
_marker_inside_leading_envelope required the DeepSeek/Kimi marker to sit
inside the first closed bare-JSON or Mistral call. A marker after that
closed call (a trailing example or data in a later ;-chained call's
strings) now also defers to the leading call, the same inside-or-after
rule the closed XML envelope patterns already applied.
The nested Gemma primitive value scan split on every comma, corrupting
arguments like opts:{code:print(1,2),lang:py}. It now applies the same
paren/brace depth, contextual quote openers, and comma-only-before-a-key
mapping rule as the top-level scan.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Gemma leading guard: a closed enabled call preceding the signal owns the turn
The wrapperless Gemma guard only claimed the turn when the first foreign
signal sat inside the first enabled balanced call. When that call closed
before the signal (a second call quoting a Mistral or Kimi literal, or a
trailing prose example), the guard forfeited the turn and the foreign
parser promoted the quoted literal, dropping the real Gemma calls. Apply
the same inside-or-after ownership rule as the closed bare-JSON and
Mistral envelopes, gated on an enabled name so the name-agnostic legacy
path is unchanged.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Marker guard: only an executable leading bare-JSON call owns the turn
The bare-JSON branch of the leading-envelope marker guard claimed the
turn for any NAMED leading object. A disabled-name object is prose by
design (the bare-JSON parser will not execute it), so deferring the
DeepSeek/Kimi pre-pass to it lost the real later call entirely. Gate the
ownership claim on the enabled set (or the name-agnostic None path). A
marker inside the disabled object's own strings stays data, matching the
tail-exclusion contract; a marker after it now falls through so the
pre-pass parses the real call. The Mistral branch stays ungated since
[TOOL_CALLS] parsing is never name-gated.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Gemma scan skips leading JSON answers; GLM heal bounds values at structural tags
Two fixes to the document-order data contracts:
The markerless Gemma scan only exempted whole-content JSON, so a leading
JSON answer followed by prose had an enabled call:NAME{...} snippet
inside its strings promoted to a real executed call and stripped from
the displayed answer. Both the parse and strip scans now start after a
balanced json-valid leading value span, keeping parse and strip
mirrored. Real calls after the answer still parse; mid-prose JSON gets
no exemption.
The GLM heal fallback for a missing closing arg_value tag took the
entire remainder as the value, executing markup-contaminated arguments
like city="NYC</tool_call>" and swallowing trailing prose. The healed
value now stops at the next arg_key or tool_call close and the pair walk
resumes there. EOF-truncated values keep the partial heal, strict mode
still rejects, and closed values holding a literal close tag in quotes
are untouched.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Compress docstrings in the multi-format tool parser to their contract essence
* Condense parser guard comments and test narration to contract essentials
* verify_import_hoist: exempt __future__ imports and same-diff relocations
Two false positives fired on this PR's refactor. A from __future__ import
is a compiler directive whose name never appears as a runtime load, so
HOISTED-IMPORT-UNUSED can never see it used, yet the file requires it for
PEP 604 annotations on Python 3.9. TARGET-CHANGED flagged the deliberate
move of the strip-pattern constants into core.inference.tool_call_parser
as a silent re-point even though the old module-level target was removed
and the new one added in the same diff. Both get narrow exemptions; a
re-point to a pre-existing target is still caught, and the self-test
negative controls all pass unchanged.
* Leading bare-JSON calls own the turn; function calls end at the first balanced close
The XML-signal guard for a leading bare-JSON call required the signal
strictly inside the object, so a trailing XML example stole the turn
from the leading call; it now applies the same inside-or-after rule as
the Mistral guard. Function-XML calls also ended at the LAST close tag,
which let prose after a closed call that mentions a literal close tag
get swallowed into the final parameter value; calls now end at the
first close tag that is not inside an open parameter, and the strip
mirrors the same rule so parse and strip agree.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Attribute-form calls end at the first balanced close; bare-JSON strip requires the call shape
The attribute form parser still kept the last close tag in the call
window, folding prose after a closed call into the final parameter
value. It now takes the first close not inside an open parameter, the
same rule the equals form and the strip already use.
The leading bare-JSON strip deleted any closed object whose top-level
name matched an enabled tool, including plain JSON answers the parser
correctly rejects as non-calls. The strip (and the drain gate that
delegates to it) now requires the parser's exact call shape, so answers
like {"name":"web_search","result":...} stream and display intact.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* False-alarm markers keep the answer; the bare-JSON strip consumes the whole chain
The trailing strip arms dropped everything from a bare marker to EOF,
so a normal answer that mentions [TOOL_CALLS] or another marker
literally was truncated (or fully swallowed when it started with the
literal) after the no-call drain fallback. Those arms now require a
call-shaped lookahead or marker-at-EOF before dropping; truncated real
calls still strip.
Chained bare-JSON turns executed both calls but stripped only the first
object, so the second call's raw JSON replayed into the next assistant
history message alongside the structured tool_calls. The strip now
consumes the entire chained run of call-shaped enabled objects while
non-call answers, disabled names, and trailing prose stay intact.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* DeepSeek and Kimi trailing strip arms require a call-shaped lookahead
Same false-alarm rule as the bare-word markers: a prose answer that
mentions a DeepSeek or Kimi marker literally keeps its tail, while
truncated real envelopes and bare end-of-text fragments still drop.
* Attribute-form containment, parameter-close-decides rule, preamble-tolerant Mistral guard, strict strip shape
Four document-order and containment fixes. A leading attribute-form
call now parses before the shared XML pass, so markup quoted in its
parameter stays data. The open-parameter scan lets the parameter's own
close tag decide, so any number of literal function closes inside one
value stay data, restoring the pre-close-scan behavior for multi-close
arguments. The leading-Mistral guard tolerates a visible preamble, with
the leading-bare-JSON guard running first so a trigger quoted inside a
leading JSON object stays data. The bare-JSON strip requires the
parser's top-level name in every mode, so nested-name JSON answers
survive name-agnostic stripping.
* Keep buffering long wrapper-less Gemma tool names instead of leaking the prefix
The streaming buffer stopped holding a call:NAME prefix at a fixed
32-char cap, so a Gemma wrapper-less call to a tool whose name exceeds
that (OpenAI allows 64 chars, MCP names run longer) streamed its raw
call:longname text as visible content before the end-of-turn parser
executed it. Hold the variable-length prefix while it still matches the
call: shape, bounded like the bare-JSON path and self-terminating into
prose, draining once the opening brace arrives.
* Keep prose that only mentions DeepSeek/Kimi markers in the route display strip
The route-level _TOOL_XML_RE DeepSeek/Kimi arms consumed from an opener up to
the end of text whenever the marker appeared, so an answer that merely refers
to a marker (for example "See <|tool_call_begin|> in the docs") had the rest
of the reply truncated. The parser-level _TOOL_ALL_PATS already gates these
arms with a call-shaped lookahead. Mirror it here so a marker is only stripped
when a real call follows it or it is a bare fragment at end of text.
* Tighten tool-calling parser and backend comments
* Pass trust_remote_code when reloading native tokenizers
The native-template fallback re-fetches a model's native chat template from
its repo when an Unsloth override template drops the tools schema. The
secondary AutoTokenizer.from_pretrained threaded hf_token but not
trust_remote_code, so for a model loaded with trust_remote_code=True whose
tokenizer repo carries custom code the reload raised, was swallowed, and the
request silently kept the tool-dropping prompt for a model that supports tools.
Store the loaded trust_remote_code on each backend's per-model info dict and
source it in render_native_template, so the reload re-uses exactly the consent
granted at load. For a LoRA adapter the reload targets the base model, whose
remote code was gated and loaded under the same stored flag, so re-passing it
executes no unconsented code. Falsy stored flag preserves the prior behaviour.
Adds a regression test that fails without the flag (custom-code reload raises,
returns None) and passes with it (tools-advertising native prompt returned).
* Treat <|python_tag|> as an outer marker envelope
A Llama-3 <|python_tag|> tool call (built-in NAME.call(...) or custom
{json} form) whose argument quotes a complete DeepSeek/Kimi example was
hijacked by the DeepSeek/Kimi marker pre-pass: the embedded example (for
example delete_all) executed instead of the real outer call. python_tag
is Llama-3's tool-call envelope, so a marker quoted inside its arguments
is data, the same as for <tool_call>, <function=...>, bare JSON, Mistral
and wrapper-less Gemma, which the guard already covers.
Add <|python_tag|> to _OUTER_ENVELOPE_OPEN_RE with a call-shaped
lookahead (mirroring the _TOOL_ALL_PATS python_tag arm) so the marker
pre-pass is suppressed when a python_tag call opens before the first
marker, while a bare prose <|python_tag|> mention is left untouched.
* Tighten tool-call parser comments
---------
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: danielhanchen <michaelhan2050@gmail.com>
Co-authored-by: Daniel Han <info@unsloth.ai>
Co-authored-by: danielhanchen <danielhanchen@users.noreply.github.com>
1253 lines
44 KiB
Python
1253 lines
44 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
|
|
|
|
import pytest
|
|
from fastapi import HTTPException
|
|
|
|
from storage import mcp_servers_db
|
|
|
|
|
|
def _reset_db(tmp_path, monkeypatch):
|
|
monkeypatch.setenv("UNSLOTH_STUDIO_HOME", str(tmp_path))
|
|
monkeypatch.setattr(mcp_servers_db, "_schema_ready", False)
|
|
|
|
|
|
# ── storage: mcp_servers_db ─────────────────────────────────────────
|
|
|
|
|
|
def test_create_and_get_server(tmp_path, monkeypatch):
|
|
_reset_db(tmp_path, monkeypatch)
|
|
mcp_servers_db.create_server(
|
|
id = "srv1",
|
|
display_name = "GitHub",
|
|
url = "https://example.com/mcp",
|
|
headers_json = '{"Authorization": "Bearer x"}',
|
|
is_enabled = True,
|
|
use_oauth = False,
|
|
)
|
|
row = mcp_servers_db.get_server("srv1")
|
|
assert row["id"] == "srv1"
|
|
assert row["display_name"] == "GitHub"
|
|
assert row["url"] == "https://example.com/mcp"
|
|
assert row["headers_json"] == '{"Authorization": "Bearer x"}'
|
|
assert row["is_enabled"] == 1
|
|
assert row["use_oauth"] == 0
|
|
|
|
|
|
def test_list_servers_ordered_by_created_at(tmp_path, monkeypatch):
|
|
_reset_db(tmp_path, monkeypatch)
|
|
mcp_servers_db.create_server(id = "a", display_name = "A", url = "https://a/m")
|
|
mcp_servers_db.create_server(id = "b", display_name = "B", url = "https://b/m")
|
|
rows = mcp_servers_db.list_servers()
|
|
assert [r["id"] for r in rows] == ["a", "b"]
|
|
|
|
|
|
def test_update_server_coerces_bools(tmp_path, monkeypatch):
|
|
_reset_db(tmp_path, monkeypatch)
|
|
mcp_servers_db.create_server(id = "srv1", display_name = "A", url = "https://a/m")
|
|
assert mcp_servers_db.update_server("srv1", {"is_enabled": False, "use_oauth": True})
|
|
row = mcp_servers_db.get_server("srv1")
|
|
assert row["is_enabled"] == 0
|
|
assert row["use_oauth"] == 1
|
|
|
|
|
|
def test_update_server_empty_changes_returns_false(tmp_path, monkeypatch):
|
|
_reset_db(tmp_path, monkeypatch)
|
|
mcp_servers_db.create_server(id = "srv1", display_name = "A", url = "https://a/m")
|
|
assert mcp_servers_db.update_server("srv1", {}) is False
|
|
|
|
|
|
def test_delete_server_roundtrip(tmp_path, monkeypatch):
|
|
_reset_db(tmp_path, monkeypatch)
|
|
mcp_servers_db.create_server(id = "srv1", display_name = "A", url = "https://a/m")
|
|
assert mcp_servers_db.delete_server("srv1") is True
|
|
assert mcp_servers_db.delete_server("srv1") is False
|
|
assert mcp_servers_db.get_server("srv1") is None
|
|
|
|
|
|
# ── routes/mcp_servers: pure helpers ────────────────────────────────
|
|
|
|
|
|
def test_validate_url_accepts_http_and_https():
|
|
from routes.mcp_servers import _validate_url
|
|
|
|
assert _validate_url("http://example.com/mcp") == "http://example.com/mcp"
|
|
assert _validate_url("https://example.com/mcp") == "https://example.com/mcp"
|
|
assert _validate_url(" https://example.com/mcp ") == "https://example.com/mcp"
|
|
|
|
|
|
@pytest.mark.parametrize("bad", ["", " ", "ftp://x", "http://", "noscheme.com"])
|
|
def test_validate_url_rejects_bad(bad):
|
|
from routes.mcp_servers import _validate_url
|
|
with pytest.raises(HTTPException) as exc:
|
|
_validate_url(bad)
|
|
assert exc.value.status_code == 400
|
|
|
|
|
|
def test_normalize_headers():
|
|
from routes.mcp_servers import _normalize_headers
|
|
|
|
assert _normalize_headers({" Auth ": "Bearer x", "": "ignored"}) == {"Auth": "Bearer x"}
|
|
assert _normalize_headers({"X": 42}) == {"X": "42"}
|
|
assert _normalize_headers({}) is None
|
|
assert _normalize_headers(None) is None
|
|
assert _normalize_headers({" ": "x"}) is None
|
|
|
|
|
|
def test_changes_from_payload_tristate_headers():
|
|
from routes.mcp_servers import _changes_from_payload
|
|
from models.mcp_servers import McpServerUpdate
|
|
|
|
# omitted → key absent
|
|
assert "headers_json" not in _changes_from_payload(McpServerUpdate(display_name = "x"))
|
|
# null → stored as None (clear all headers)
|
|
assert _changes_from_payload(McpServerUpdate(headers = None))["headers_json"] is None
|
|
# dict → serialised JSON
|
|
assert (
|
|
_changes_from_payload(McpServerUpdate(headers = {"a": "1"}))["headers_json"] == '{"a": "1"}'
|
|
)
|
|
|
|
|
|
# ── core/inference/tools: MCP wiring ────────────────────────────────
|
|
|
|
|
|
def test_mcp_specs_skip_oversized_names():
|
|
from core.inference.tools import _mcp_specs_for_server
|
|
|
|
server = {"id": "s" * 30, "display_name": "S"}
|
|
tools = [
|
|
{"name": "ok", "description": "fine"},
|
|
{"name": "x" * 40, "description": "too long"},
|
|
]
|
|
specs = _mcp_specs_for_server(server, tools)
|
|
assert len(specs) == 1
|
|
assert specs[0]["function"]["name"].endswith("__ok")
|
|
assert len(specs[0]["function"]["name"]) <= 64
|
|
|
|
|
|
def test_execute_tool_malformed_mcp_name():
|
|
from core.inference.tools import execute_tool
|
|
out = execute_tool("mcp__no_double_underscore", {})
|
|
assert out.startswith("Error: malformed MCP tool name")
|
|
|
|
|
|
def test_execute_tool_unknown_server(tmp_path, monkeypatch):
|
|
_reset_db(tmp_path, monkeypatch)
|
|
from core.inference.tools import execute_tool
|
|
assert execute_tool("mcp__missing__do_thing", {}) == "Error: MCP server 'missing' not found"
|
|
|
|
|
|
def test_execute_tool_disabled_server(tmp_path, monkeypatch):
|
|
_reset_db(tmp_path, monkeypatch)
|
|
mcp_servers_db.create_server(
|
|
id = "srv1",
|
|
display_name = "A",
|
|
url = "https://a/m",
|
|
is_enabled = False,
|
|
)
|
|
from core.inference.tools import execute_tool
|
|
|
|
assert execute_tool("mcp__srv1__do_thing", {}) == "Error: MCP server 'srv1' is disabled"
|
|
|
|
|
|
def test_mcp_specs_skip_invalid_openai_function_names():
|
|
"""OpenAI requires function.name ^[a-zA-Z0-9_-]{1,64}$; bad names 400 the request."""
|
|
from core.inference.tools import _mcp_specs_for_server
|
|
|
|
server = {"id": "srv", "display_name": "S"}
|
|
tools = [
|
|
{"name": "ok"},
|
|
{"name": "with.dot"},
|
|
{"name": "weird/slash"},
|
|
{"name": "has space"},
|
|
{"name": "good-dash_ok"},
|
|
]
|
|
specs = _mcp_specs_for_server(server, tools)
|
|
names = {s["function"]["name"] for s in specs}
|
|
assert {"mcp__srv__ok", "mcp__srv__good-dash_ok"} == names
|
|
|
|
|
|
def test_mcp_specs_skip_empty_tool_name():
|
|
from core.inference.tools import _mcp_specs_for_server
|
|
|
|
server = {"id": "srv", "display_name": "S"}
|
|
specs = _mcp_specs_for_server(server, [{"name": "", "description": "x"}])
|
|
assert specs == []
|
|
|
|
|
|
def test_mcp_specs_drops_duplicate_names():
|
|
"""Duplicate tool names from one server -> OpenAI rejects; drop before forwarding."""
|
|
from core.inference.tools import _mcp_specs_for_server
|
|
|
|
server = {"id": "srv", "display_name": "S"}
|
|
tools = [{"name": "echo"}, {"name": "echo"}]
|
|
specs = _mcp_specs_for_server(server, tools)
|
|
assert len(specs) == 1
|
|
|
|
|
|
def test_call_tool_sync_respects_pre_set_cancel_event(monkeypatch):
|
|
"""Pre-set cancel_event -> immediate cancellation, no network round-trip."""
|
|
import threading
|
|
from core.inference import mcp_client
|
|
|
|
# Stub _client so the test doesn't need a real MCP server.
|
|
class _StubClient:
|
|
async def __aenter__(self):
|
|
return self
|
|
|
|
async def __aexit__(self, *args):
|
|
return False
|
|
|
|
async def call_tool(self, name, args):
|
|
import asyncio as _asyncio
|
|
await _asyncio.sleep(30) # never finishes during the test
|
|
|
|
monkeypatch.setattr(mcp_client, "_client", lambda *a, **kw: _StubClient())
|
|
|
|
cancel = threading.Event()
|
|
cancel.set()
|
|
out = mcp_client.call_tool_sync(
|
|
url = "https://example/mcp",
|
|
headers = None,
|
|
name = "slow",
|
|
args = {},
|
|
timeout = 30.0,
|
|
cancel_event = cancel,
|
|
)
|
|
assert "cancelled" in out.lower()
|
|
|
|
|
|
def test_clear_oauth_tokens_async_no_op_safe(tmp_path, monkeypatch):
|
|
"""clear_oauth_tokens_async on a URL with no stored token must not raise;
|
|
the delete + update handlers call it best-effort regardless of state."""
|
|
import asyncio
|
|
|
|
monkeypatch.setenv("UNSLOTH_STUDIO_HOME", str(tmp_path))
|
|
from core.inference import mcp_client
|
|
|
|
monkeypatch.setattr(mcp_client, "_oauth_token_store", None)
|
|
asyncio.run(mcp_client.clear_oauth_tokens_async("https://example.com/mcp"))
|
|
|
|
|
|
def test_delete_server_calls_oauth_cleanup_when_oauth_was_on(tmp_path, monkeypatch):
|
|
"""delete_mcp_server route helper must call clear_oauth_tokens_async
|
|
when the deleted row had use_oauth=true."""
|
|
import asyncio
|
|
|
|
_reset_db(tmp_path, monkeypatch)
|
|
from core.inference import mcp_client
|
|
|
|
monkeypatch.setattr(mcp_client, "_oauth_token_store", None)
|
|
mcp_servers_db.create_server(
|
|
id = "oauth1",
|
|
display_name = "GH",
|
|
url = "https://gh-mcp.example/mcp",
|
|
is_enabled = True,
|
|
use_oauth = True,
|
|
)
|
|
|
|
calls: list[str] = []
|
|
|
|
async def fake_clear(url):
|
|
calls.append(url)
|
|
|
|
monkeypatch.setattr(mcp_client, "clear_oauth_tokens_async", fake_clear)
|
|
# Patch the route's module binding too so it's seen.
|
|
import routes.mcp_servers as routes_mcp
|
|
|
|
monkeypatch.setattr(routes_mcp, "clear_oauth_tokens_async", fake_clear)
|
|
asyncio.run(routes_mcp.delete_mcp_server("oauth1", current_subject = "u"))
|
|
assert calls == ["https://gh-mcp.example/mcp"]
|
|
assert mcp_servers_db.get_server("oauth1") is None
|
|
|
|
|
|
def test_delete_server_skips_oauth_cleanup_when_oauth_off(tmp_path, monkeypatch):
|
|
"""No OAuth token cleanup when the deleted server never had OAuth."""
|
|
import asyncio
|
|
|
|
_reset_db(tmp_path, monkeypatch)
|
|
from core.inference import mcp_client
|
|
import routes.mcp_servers as routes_mcp
|
|
|
|
monkeypatch.setattr(mcp_client, "_oauth_token_store", None)
|
|
mcp_servers_db.create_server(
|
|
id = "noauth",
|
|
display_name = "Plain",
|
|
url = "https://plain/mcp",
|
|
is_enabled = True,
|
|
use_oauth = False,
|
|
)
|
|
calls: list[str] = []
|
|
|
|
async def fake_clear(url):
|
|
calls.append(url)
|
|
|
|
monkeypatch.setattr(routes_mcp, "clear_oauth_tokens_async", fake_clear)
|
|
asyncio.run(routes_mcp.delete_mcp_server("noauth", current_subject = "u"))
|
|
assert calls == []
|
|
|
|
|
|
def test_update_server_clears_oauth_on_url_change(tmp_path, monkeypatch):
|
|
"""Changing the URL on an OAuth server must drop the old URL's tokens
|
|
so the new URL doesn't inherit credentials."""
|
|
import asyncio
|
|
|
|
_reset_db(tmp_path, monkeypatch)
|
|
from core.inference import mcp_client
|
|
from models.mcp_servers import McpServerUpdate
|
|
import routes.mcp_servers as routes_mcp
|
|
|
|
monkeypatch.setattr(mcp_client, "_oauth_token_store", None)
|
|
mcp_servers_db.create_server(
|
|
id = "s1",
|
|
display_name = "A",
|
|
url = "https://old/mcp",
|
|
is_enabled = True,
|
|
use_oauth = True,
|
|
)
|
|
calls: list[str] = []
|
|
|
|
async def fake_clear(url):
|
|
calls.append(url)
|
|
|
|
monkeypatch.setattr(routes_mcp, "clear_oauth_tokens_async", fake_clear)
|
|
asyncio.run(
|
|
routes_mcp.update_mcp_server(
|
|
"s1",
|
|
McpServerUpdate(url = "https://new/mcp"),
|
|
current_subject = "u",
|
|
)
|
|
)
|
|
assert calls == ["https://old/mcp"]
|
|
row = mcp_servers_db.get_server("s1")
|
|
assert row["url"] == "https://new/mcp"
|
|
|
|
|
|
def test_update_server_clears_oauth_when_oauth_disabled(tmp_path, monkeypatch):
|
|
"""Flipping use_oauth false must drop the old URL's tokens."""
|
|
import asyncio
|
|
|
|
_reset_db(tmp_path, monkeypatch)
|
|
from core.inference import mcp_client
|
|
from models.mcp_servers import McpServerUpdate
|
|
import routes.mcp_servers as routes_mcp
|
|
|
|
monkeypatch.setattr(mcp_client, "_oauth_token_store", None)
|
|
mcp_servers_db.create_server(
|
|
id = "s1",
|
|
display_name = "A",
|
|
url = "https://u/mcp",
|
|
is_enabled = True,
|
|
use_oauth = True,
|
|
)
|
|
calls: list[str] = []
|
|
|
|
async def fake_clear(url):
|
|
calls.append(url)
|
|
|
|
monkeypatch.setattr(routes_mcp, "clear_oauth_tokens_async", fake_clear)
|
|
asyncio.run(
|
|
routes_mcp.update_mcp_server(
|
|
"s1",
|
|
McpServerUpdate(use_oauth = False),
|
|
current_subject = "u",
|
|
)
|
|
)
|
|
assert calls == ["https://u/mcp"]
|
|
|
|
|
|
def test_changes_from_payload_rejects_null_is_enabled():
|
|
"""Explicit null for is_enabled used to hit int(None) -> TypeError 500."""
|
|
from routes.mcp_servers import _changes_from_payload
|
|
from models.mcp_servers import McpServerUpdate
|
|
|
|
with pytest.raises(HTTPException) as exc:
|
|
_changes_from_payload(McpServerUpdate(is_enabled = None))
|
|
assert exc.value.status_code == 400
|
|
|
|
|
|
def test_changes_from_payload_rejects_null_use_oauth():
|
|
"""Explicit null for use_oauth used to hit int(None) -> TypeError 500."""
|
|
from routes.mcp_servers import _changes_from_payload
|
|
from models.mcp_servers import McpServerUpdate
|
|
|
|
with pytest.raises(HTTPException) as exc:
|
|
_changes_from_payload(McpServerUpdate(use_oauth = None))
|
|
assert exc.value.status_code == 400
|
|
|
|
|
|
def test_test_endpoint_surfaces_url_validation_as_400(tmp_path, monkeypatch):
|
|
"""POST /api/mcp/servers/test must 400 on invalid URL like create/update;
|
|
it previously returned 200 with {"ok": false}."""
|
|
import asyncio
|
|
|
|
_reset_db(tmp_path, monkeypatch)
|
|
from routes.mcp_servers import test_mcp_server
|
|
from models.mcp_servers import McpServerTestRequest
|
|
|
|
with pytest.raises(HTTPException) as exc:
|
|
asyncio.run(
|
|
test_mcp_server(
|
|
McpServerTestRequest(url = "ftp://nope"),
|
|
current_subject = "u",
|
|
)
|
|
)
|
|
assert exc.value.status_code == 400
|
|
|
|
|
|
def test_tool_xml_parser_handles_hyphenated_parameter_names():
|
|
"""Hyphenated property names like `issue-number` must round-trip through the
|
|
XML parser (the old `<parameter=\\w+>` regex dropped them)."""
|
|
from core.inference.tool_call_parser import parse_tool_calls_from_text
|
|
import json as _json
|
|
|
|
calls = parse_tool_calls_from_text(
|
|
"<function=mcp__srv__create-issue>"
|
|
"<parameter=issue-title>Bug report</parameter>"
|
|
"<parameter=repo-name>octocat/hello</parameter>"
|
|
"</function>"
|
|
)
|
|
assert len(calls) == 1
|
|
args = _json.loads(calls[0]["function"]["arguments"])
|
|
assert args == {"issue-title": "Bug report", "repo-name": "octocat/hello"}
|
|
|
|
|
|
def test_tool_healing_strip_handles_hyphenated_function_names():
|
|
"""core/tool_healing.py has its own copy of the XML strip regex that the
|
|
shared-parser fix missed."""
|
|
from core.tool_healing import strip_tool_call_markup
|
|
|
|
out = strip_tool_call_markup(
|
|
"before <function=mcp__srv__list-issues><parameter=q>x</parameter></function> after"
|
|
)
|
|
assert out == "before after"
|
|
|
|
|
|
def test_tool_healing_strip_handles_gemma_native_tool_call():
|
|
from core.tool_healing import strip_tool_call_markup
|
|
out = strip_tool_call_markup(
|
|
'before <|tool_call>call:mcp__srv__list-issues{repo:"octocat/hello"}<tool_call|> after'
|
|
)
|
|
assert out == "before after"
|
|
|
|
|
|
def test_tool_healing_strip_handles_gemma_close_only_marker():
|
|
from core.tool_healing import strip_tool_call_markup
|
|
assert strip_tool_call_markup("before <tool_call|> after") == "before after"
|
|
assert strip_tool_call_markup("before <tool_call|> after", final = True) == "before after"
|
|
|
|
|
|
def test_tool_healing_parser_handles_gemma_native_windows_path():
|
|
from core.tool_healing import parse_tool_calls_from_text
|
|
import json as _json
|
|
|
|
calls = parse_tool_calls_from_text(
|
|
r'<|tool_call>call:ls{path:<|"|>C:\Users\wasim\repo<|"|>}<tool_call|>'
|
|
)
|
|
assert len(calls) == 1
|
|
assert calls[0]["function"]["name"] == "ls"
|
|
assert _json.loads(calls[0]["function"]["arguments"]) == {"path": r"C:\Users\wasim\repo"}
|
|
|
|
|
|
def test_tool_healing_json_parser_preserves_literal_gemma_quote_token():
|
|
from core.tool_healing import parse_tool_calls_from_text
|
|
import json as _json
|
|
|
|
text = (
|
|
"<tool_call>"
|
|
+ _json.dumps({"name": "python", "arguments": {"code": "print('<|\"|>')"}})
|
|
+ "</tool_call>"
|
|
)
|
|
calls = parse_tool_calls_from_text(text, allow_incomplete = False)
|
|
assert len(calls) == 1
|
|
assert _json.loads(calls[0]["function"]["arguments"]) == {"code": "print('<|\"|>')"}
|
|
|
|
|
|
def test_gguf_allow_list_blocks_unadvertised_tool(monkeypatch):
|
|
"""A tool call not in the per-request list must be refused by the GGUF
|
|
agentic loop (mirroring the safetensors path)."""
|
|
from core.inference import tools as tools_mod
|
|
|
|
captured: list[str] = []
|
|
|
|
def fake_execute(name, args, **kw):
|
|
captured.append(name)
|
|
return "executed"
|
|
|
|
monkeypatch.setattr(tools_mod, "execute_tool", fake_execute)
|
|
|
|
# Inline allow-list check to unit-test behavior without llama-server.
|
|
def _gate(tools_advertised, called_name, args):
|
|
allowed = {
|
|
(t.get("function") or {}).get("name")
|
|
for t in (tools_advertised or [])
|
|
if (t.get("function") or {}).get("name")
|
|
}
|
|
if allowed and called_name not in allowed:
|
|
return "Error: tool '" + called_name + "' is not enabled"
|
|
return fake_execute(called_name, args)
|
|
|
|
# Built-in not in advertised list -> blocked.
|
|
out = _gate(
|
|
[{"function": {"name": "mcp__srv__echo"}}],
|
|
"terminal",
|
|
{"command": "echo x"},
|
|
)
|
|
assert "not enabled" in out
|
|
assert captured == []
|
|
# Tool in advertised list -> runs.
|
|
out = _gate(
|
|
[{"function": {"name": "mcp__srv__echo"}}],
|
|
"mcp__srv__echo",
|
|
{"text": "hi"},
|
|
)
|
|
assert out == "executed"
|
|
assert captured == ["mcp__srv__echo"]
|
|
|
|
|
|
def test_call_tool_sync_short_circuits_on_pre_set_cancel(monkeypatch):
|
|
"""Pre-set cancel_event -> no HTTP request (task used to open a transport
|
|
before the cancel check)."""
|
|
from core.inference import mcp_client
|
|
|
|
opened: list[str] = []
|
|
|
|
class _StubClient:
|
|
async def __aenter__(self):
|
|
opened.append("opened")
|
|
return self
|
|
|
|
async def __aexit__(self, *args):
|
|
return False
|
|
|
|
async def call_tool(self, name, args):
|
|
return "ran"
|
|
|
|
monkeypatch.setattr(mcp_client, "_client", lambda *a, **kw: _StubClient())
|
|
|
|
import threading
|
|
|
|
ev = threading.Event()
|
|
ev.set()
|
|
out = mcp_client.call_tool_sync(
|
|
url = "https://example/mcp",
|
|
headers = None,
|
|
name = "x",
|
|
args = {},
|
|
timeout = 5.0,
|
|
cancel_event = ev,
|
|
)
|
|
assert "cancelled" in out.lower()
|
|
# The client must NOT have been opened.
|
|
assert opened == []
|
|
|
|
|
|
def test_clear_oauth_tokens_swallows_constructor_errors(tmp_path, monkeypatch):
|
|
"""clear_oauth_tokens_async is best-effort; an OAuth constructor failure
|
|
must not bubble into a 500 from the delete/update routes."""
|
|
import asyncio
|
|
from core.inference import mcp_client
|
|
|
|
monkeypatch.setenv("UNSLOTH_STUDIO_HOME", str(tmp_path))
|
|
monkeypatch.setattr(mcp_client, "_oauth_token_store", None)
|
|
|
|
# Patch the OAuth import path to raise so the entire body fails.
|
|
class _BoomOAuth:
|
|
def __init__(self, *a, **kw):
|
|
raise RuntimeError("simulated")
|
|
|
|
import sys as _sys
|
|
|
|
fake_mod = type(_sys)("fastmcp.client.auth")
|
|
fake_mod.OAuth = _BoomOAuth
|
|
monkeypatch.setitem(_sys.modules, "fastmcp.client.auth", fake_mod)
|
|
# Must not raise.
|
|
asyncio.run(mcp_client.clear_oauth_tokens_async("https://x/mcp"))
|
|
|
|
|
|
def test_tool_xml_parser_handles_hyphenated_function_names():
|
|
"""Hyphenated tool names like `mcp__srv__list-issues` must parse, else the
|
|
model can call the tool but Studio can't dispatch."""
|
|
from core.inference.tool_call_parser import parse_tool_calls_from_text
|
|
|
|
calls = parse_tool_calls_from_text(
|
|
"<function=mcp__srv__list-issues><parameter=repo>octocat/hello</parameter></function>"
|
|
)
|
|
assert len(calls) == 1
|
|
assert calls[0]["function"]["name"] == "mcp__srv__list-issues"
|
|
import json as _json
|
|
|
|
args = _json.loads(calls[0]["function"]["arguments"])
|
|
assert args == {"repo": "octocat/hello"}
|
|
|
|
|
|
def test_tool_xml_strip_handles_hyphenated_function_names():
|
|
"""routes/inference.py:_TOOL_XML_RE must strip a `<function=name-with-dash>`
|
|
block; else hyphenated MCP tool-call XML leaks into chat history."""
|
|
import re as _re
|
|
from pathlib import Path
|
|
|
|
from core.inference.tool_call_parser import _DEEPSEEK_OPEN_RE_SRC as _DS_OPEN_SRC
|
|
|
|
src = (Path(__file__).resolve().parent.parent / "routes/inference.py").read_text()
|
|
m = _re.search(r"_TOOL_XML_RE = _re\.compile\((.*?)\n\)", src, _re.DOTALL)
|
|
assert m, "could not extract _TOOL_XML_RE"
|
|
ns: dict = {"_re": _re, "_DS_OPEN_SRC": _DS_OPEN_SRC}
|
|
exec(f"_TOOL_XML_RE = _re.compile({m.group(1)})", ns)
|
|
rx = ns["_TOOL_XML_RE"]
|
|
stripped = rx.sub(
|
|
"",
|
|
"before <function=mcp__srv__list-issues><parameter=q>x</parameter></function> after",
|
|
)
|
|
assert stripped == "before after"
|
|
|
|
|
|
def test_safetensors_agentic_empty_allowlist_still_means_allow_all():
|
|
"""Contract: at the safetensors_agentic layer tools=[] means "no
|
|
constraint". The MCP-only-no-discovery fix lives at the route level in
|
|
inference.py, which refuses use_tools when the resolved list is empty."""
|
|
import threading
|
|
from core.inference.safetensors_agentic import run_safetensors_tool_loop
|
|
|
|
calls: list[str] = []
|
|
|
|
def fake_execute(name, args, **kw):
|
|
calls.append(name)
|
|
return "ran"
|
|
|
|
iteration = {"n": 0}
|
|
|
|
def fake_single_turn(messages):
|
|
iteration["n"] += 1
|
|
if iteration["n"] == 1:
|
|
txt = '<tool_call>{"name":"python","arguments":{"code":"1"}}</tool_call>'
|
|
buf = ""
|
|
for ch in txt:
|
|
buf += ch
|
|
yield buf
|
|
else:
|
|
yield "done"
|
|
|
|
list(
|
|
run_safetensors_tool_loop(
|
|
single_turn = fake_single_turn,
|
|
messages = [{"role": "user", "content": "x"}],
|
|
tools = [],
|
|
execute_tool = fake_execute,
|
|
cancel_event = threading.Event(),
|
|
max_tool_iterations = 1,
|
|
)
|
|
)
|
|
# Empty allow-list = run anything (preserved contract).
|
|
assert calls == [("python", {"code": "1"})] or len(calls) >= 1
|
|
|
|
|
|
# ── discovery cache ─────────────────────────────────────────────────
|
|
|
|
|
|
def _one_tool(name = "echo"):
|
|
return [{"name": name, "inputSchema": {"type": "object", "properties": {}}}]
|
|
|
|
|
|
def test_get_enabled_mcp_tools_caches_discovery(tmp_path, monkeypatch):
|
|
"""A second send must serve tools from cache instead of re-probing."""
|
|
import asyncio
|
|
|
|
_reset_db(tmp_path, monkeypatch)
|
|
from core.inference import mcp_client
|
|
from core.inference import tools as tools_mod
|
|
|
|
monkeypatch.setattr(mcp_client, "_tool_cache", {})
|
|
mcp_servers_db.create_server(id = "s1", display_name = "A", url = "https://x/mcp", is_enabled = True)
|
|
|
|
calls: list[str] = []
|
|
|
|
async def fake(
|
|
url,
|
|
headers = None,
|
|
timeout = None,
|
|
use_oauth = False,
|
|
):
|
|
calls.append(url)
|
|
return _one_tool()
|
|
|
|
monkeypatch.setattr(tools_mod, "list_tools_async", fake)
|
|
|
|
first = asyncio.run(tools_mod.get_enabled_mcp_tools())
|
|
second = asyncio.run(tools_mod.get_enabled_mcp_tools())
|
|
|
|
assert len(calls) == 1 # probed once, cache hit on the second send
|
|
assert [t["function"]["name"] for t in first] == ["mcp__s1__echo"]
|
|
assert first == second
|
|
|
|
|
|
def test_get_enabled_mcp_tools_does_not_cache_failures(tmp_path, monkeypatch):
|
|
"""A failed probe isn't cached: once the cool-off elapses, it's retried."""
|
|
import asyncio
|
|
|
|
_reset_db(tmp_path, monkeypatch)
|
|
from core.inference import mcp_client
|
|
from core.inference import tools as tools_mod
|
|
|
|
monkeypatch.setattr(mcp_client, "_tool_cache", {})
|
|
monkeypatch.setattr(mcp_client, "_probe_cooloff_until", {})
|
|
mcp_servers_db.create_server(id = "s1", display_name = "A", url = "https://x/mcp", is_enabled = True)
|
|
|
|
attempts = {"n": 0}
|
|
|
|
async def fake(
|
|
url,
|
|
headers = None,
|
|
timeout = None,
|
|
use_oauth = False,
|
|
):
|
|
attempts["n"] += 1
|
|
if attempts["n"] == 1:
|
|
raise RuntimeError("server down")
|
|
return _one_tool()
|
|
|
|
monkeypatch.setattr(tools_mod, "list_tools_async", fake)
|
|
|
|
assert asyncio.run(tools_mod.get_enabled_mcp_tools()) == [] # failure -> empty
|
|
# Expire the cool-off (an until-time in the past) so the server is retried.
|
|
mcp_client._probe_cooloff_until["s1"] = 0.0
|
|
second = asyncio.run(tools_mod.get_enabled_mcp_tools())
|
|
assert attempts["n"] == 2 # retried after the cool-off, not cached
|
|
assert [t["function"]["name"] for t in second] == ["mcp__s1__echo"]
|
|
|
|
|
|
def test_refresh_warms_tool_cache(tmp_path, monkeypatch):
|
|
"""Clicking Refresh must populate the cache the chat path reads."""
|
|
import asyncio
|
|
|
|
_reset_db(tmp_path, monkeypatch)
|
|
from core.inference import mcp_client
|
|
from core.inference import tools as tools_mod
|
|
import routes.mcp_servers as routes_mcp
|
|
|
|
monkeypatch.setattr(mcp_client, "_tool_cache", {})
|
|
mcp_servers_db.create_server(id = "s1", display_name = "A", url = "https://x/mcp", is_enabled = True)
|
|
|
|
async def fake_refresh(
|
|
url,
|
|
headers = None,
|
|
timeout = None,
|
|
use_oauth = False,
|
|
):
|
|
return _one_tool()
|
|
|
|
monkeypatch.setattr(routes_mcp, "list_tools_async", fake_refresh)
|
|
res = asyncio.run(routes_mcp.refresh_mcp_server_tools("s1", current_subject = "u"))
|
|
assert res.ok and res.tool_count == 1
|
|
|
|
def boom(*a, **k):
|
|
raise AssertionError("chat path re-probed despite a warm cache")
|
|
|
|
monkeypatch.setattr(tools_mod, "list_tools_async", boom)
|
|
specs = asyncio.run(tools_mod.get_enabled_mcp_tools())
|
|
assert [t["function"]["name"] for t in specs] == ["mcp__s1__echo"]
|
|
|
|
|
|
def test_update_url_evicts_tool_cache(tmp_path, monkeypatch):
|
|
"""Re-pointing the URL must drop the old endpoint's cached tools."""
|
|
import asyncio
|
|
|
|
_reset_db(tmp_path, monkeypatch)
|
|
from core.inference import mcp_client
|
|
from models.mcp_servers import McpServerUpdate
|
|
import routes.mcp_servers as routes_mcp
|
|
|
|
monkeypatch.setattr(mcp_client, "_tool_cache", {"s1": _one_tool("stale")})
|
|
mcp_servers_db.create_server(id = "s1", display_name = "A", url = "https://old/mcp", is_enabled = True)
|
|
|
|
asyncio.run(
|
|
routes_mcp.update_mcp_server(
|
|
"s1", McpServerUpdate(url = "https://new/mcp"), current_subject = "u"
|
|
)
|
|
)
|
|
assert mcp_client.get_cached_tools("s1") is None
|
|
|
|
|
|
def test_update_display_name_keeps_tool_cache(tmp_path, monkeypatch):
|
|
"""A rename touches no endpoint, so the cache must survive it."""
|
|
import asyncio
|
|
|
|
_reset_db(tmp_path, monkeypatch)
|
|
from core.inference import mcp_client
|
|
from models.mcp_servers import McpServerUpdate
|
|
import routes.mcp_servers as routes_mcp
|
|
|
|
cached = _one_tool()
|
|
monkeypatch.setattr(mcp_client, "_tool_cache", {"s1": cached})
|
|
mcp_servers_db.create_server(id = "s1", display_name = "A", url = "https://x/mcp", is_enabled = True)
|
|
|
|
asyncio.run(
|
|
routes_mcp.update_mcp_server("s1", McpServerUpdate(display_name = "B"), current_subject = "u")
|
|
)
|
|
assert mcp_client.get_cached_tools("s1") == cached
|
|
|
|
|
|
def test_update_disable_evicts_tool_cache(tmp_path, monkeypatch):
|
|
"""Disabling a server must drop its cached tools, not leave them unread."""
|
|
import asyncio
|
|
|
|
_reset_db(tmp_path, monkeypatch)
|
|
from core.inference import mcp_client
|
|
from models.mcp_servers import McpServerUpdate
|
|
import routes.mcp_servers as routes_mcp
|
|
|
|
monkeypatch.setattr(mcp_client, "_tool_cache", {"s1": _one_tool()})
|
|
mcp_servers_db.create_server(id = "s1", display_name = "A", url = "https://x/mcp", is_enabled = True)
|
|
|
|
asyncio.run(
|
|
routes_mcp.update_mcp_server("s1", McpServerUpdate(is_enabled = False), current_subject = "u")
|
|
)
|
|
assert mcp_client.get_cached_tools("s1") is None
|
|
|
|
|
|
def test_delete_evicts_tool_cache(tmp_path, monkeypatch):
|
|
"""Deleting a server must not leave its tools cached."""
|
|
import asyncio
|
|
|
|
_reset_db(tmp_path, monkeypatch)
|
|
from core.inference import mcp_client
|
|
import routes.mcp_servers as routes_mcp
|
|
|
|
monkeypatch.setattr(mcp_client, "_tool_cache", {"s1": _one_tool()})
|
|
mcp_servers_db.create_server(id = "s1", display_name = "A", url = "https://x/mcp", is_enabled = True)
|
|
asyncio.run(routes_mcp.delete_mcp_server("s1", current_subject = "u"))
|
|
assert mcp_client.get_cached_tools("s1") is None
|
|
|
|
|
|
def test_invalidate_tool_cache_clears_all(monkeypatch):
|
|
from core.inference import mcp_client
|
|
|
|
monkeypatch.setattr(mcp_client, "_tool_cache", {"a": _one_tool(), "b": _one_tool()})
|
|
mcp_client.invalidate_tool_cache()
|
|
assert mcp_client.get_cached_tools("a") is None
|
|
assert mcp_client.get_cached_tools("b") is None
|
|
|
|
|
|
def test_get_enabled_mcp_tools_probes_only_uncached(tmp_path, monkeypatch):
|
|
"""An already-cached server must not be re-probed alongside a cold one."""
|
|
import asyncio
|
|
|
|
_reset_db(tmp_path, monkeypatch)
|
|
from core.inference import mcp_client
|
|
from core.inference import tools as tools_mod
|
|
|
|
monkeypatch.setattr(mcp_client, "_tool_cache", {"s1": _one_tool("cached")})
|
|
mcp_servers_db.create_server(id = "s1", display_name = "A", url = "https://a/mcp", is_enabled = True)
|
|
mcp_servers_db.create_server(id = "s2", display_name = "B", url = "https://b/mcp", is_enabled = True)
|
|
|
|
probed: list[str] = []
|
|
|
|
async def fake(
|
|
url,
|
|
headers = None,
|
|
timeout = None,
|
|
use_oauth = False,
|
|
):
|
|
probed.append(url)
|
|
return _one_tool("fresh")
|
|
|
|
monkeypatch.setattr(tools_mod, "list_tools_async", fake)
|
|
|
|
specs = asyncio.run(tools_mod.get_enabled_mcp_tools())
|
|
assert probed == ["https://b/mcp"] # only the uncached server is probed
|
|
assert sorted(t["function"]["name"] for t in specs) == ["mcp__s1__cached", "mcp__s2__fresh"]
|
|
|
|
|
|
def test_get_enabled_mcp_tools_partial_failure_caches_healthy(tmp_path, monkeypatch):
|
|
"""One server failing must not stop the others from being cached/served."""
|
|
import asyncio
|
|
|
|
_reset_db(tmp_path, monkeypatch)
|
|
from core.inference import mcp_client
|
|
from core.inference import tools as tools_mod
|
|
|
|
monkeypatch.setattr(mcp_client, "_tool_cache", {})
|
|
monkeypatch.setattr(mcp_client, "_probe_cooloff_until", {})
|
|
mcp_servers_db.create_server(id = "s1", display_name = "A", url = "https://bad/mcp", is_enabled = True)
|
|
mcp_servers_db.create_server(id = "s2", display_name = "B", url = "https://good/mcp", is_enabled = True)
|
|
|
|
async def fake(
|
|
url,
|
|
headers = None,
|
|
timeout = None,
|
|
use_oauth = False,
|
|
):
|
|
if "bad" in url:
|
|
raise RuntimeError("down")
|
|
return _one_tool("ok")
|
|
|
|
monkeypatch.setattr(tools_mod, "list_tools_async", fake)
|
|
|
|
specs = asyncio.run(tools_mod.get_enabled_mcp_tools())
|
|
assert [t["function"]["name"] for t in specs] == ["mcp__s2__ok"]
|
|
assert mcp_client.get_cached_tools("s1") is None # failure not cached
|
|
assert mcp_client.get_cached_tools("s2") == _one_tool("ok") # healthy cached
|
|
|
|
|
|
def test_get_enabled_mcp_tools_caches_empty_tool_list(tmp_path, monkeypatch):
|
|
"""A server exposing zero tools is cached as [] (a hit), not re-probed."""
|
|
import asyncio
|
|
|
|
_reset_db(tmp_path, monkeypatch)
|
|
from core.inference import mcp_client
|
|
from core.inference import tools as tools_mod
|
|
|
|
monkeypatch.setattr(mcp_client, "_tool_cache", {})
|
|
mcp_servers_db.create_server(id = "s1", display_name = "A", url = "https://x/mcp", is_enabled = True)
|
|
|
|
calls: list[str] = []
|
|
|
|
async def fake(
|
|
url,
|
|
headers = None,
|
|
timeout = None,
|
|
use_oauth = False,
|
|
):
|
|
calls.append(url)
|
|
return []
|
|
|
|
monkeypatch.setattr(tools_mod, "list_tools_async", fake)
|
|
|
|
assert asyncio.run(tools_mod.get_enabled_mcp_tools()) == []
|
|
assert asyncio.run(tools_mod.get_enabled_mcp_tools()) == []
|
|
assert len(calls) == 1 # [] is a cache hit, not re-probed every send
|
|
assert mcp_client.get_cached_tools("s1") == []
|
|
|
|
|
|
def test_update_headers_evicts_tool_cache(tmp_path, monkeypatch):
|
|
"""Changing auth headers must drop tools discovered under the old headers."""
|
|
import asyncio
|
|
|
|
_reset_db(tmp_path, monkeypatch)
|
|
from core.inference import mcp_client
|
|
from models.mcp_servers import McpServerUpdate
|
|
import routes.mcp_servers as routes_mcp
|
|
|
|
monkeypatch.setattr(mcp_client, "_tool_cache", {"s1": _one_tool()})
|
|
mcp_servers_db.create_server(id = "s1", display_name = "A", url = "https://x/mcp", is_enabled = True)
|
|
|
|
asyncio.run(
|
|
routes_mcp.update_mcp_server(
|
|
"s1",
|
|
McpServerUpdate(headers = {"Authorization": "Bearer new"}),
|
|
current_subject = "u",
|
|
)
|
|
)
|
|
assert mcp_client.get_cached_tools("s1") is None
|
|
|
|
|
|
def test_get_enabled_mcp_tools_skips_cache_when_config_changes_mid_probe(tmp_path, monkeypatch):
|
|
"""A config edit landing during an in-flight probe must not be clobbered
|
|
by the now-stale probe result (TOCTOU on the cache write)."""
|
|
import asyncio
|
|
|
|
_reset_db(tmp_path, monkeypatch)
|
|
from core.inference import mcp_client
|
|
from core.inference import tools as tools_mod
|
|
|
|
monkeypatch.setattr(mcp_client, "_tool_cache", {})
|
|
mcp_servers_db.create_server(id = "s1", display_name = "A", url = "https://old/mcp", is_enabled = True)
|
|
|
|
async def fake(
|
|
url,
|
|
headers = None,
|
|
timeout = None,
|
|
use_oauth = False,
|
|
):
|
|
# Simulate a PUT landing while we are awaiting the probe.
|
|
mcp_servers_db.update_server("s1", {"url": "https://new/mcp"})
|
|
return _one_tool()
|
|
|
|
monkeypatch.setattr(tools_mod, "list_tools_async", fake)
|
|
|
|
specs = asyncio.run(tools_mod.get_enabled_mcp_tools())
|
|
assert specs == [] # stale result is neither served...
|
|
assert mcp_client.get_cached_tools("s1") is None # ...nor cached
|
|
|
|
|
|
def test_get_enabled_mcp_tools_no_cooloff_when_config_changes_mid_failed_probe(
|
|
tmp_path, monkeypatch
|
|
):
|
|
"""An edit landing while a probe of the OLD config is failing must not park
|
|
a cool-off on the now-fresh config -- else the re-pointed server the user
|
|
just fixed is needlessly skipped for the whole cool-off window."""
|
|
import asyncio
|
|
|
|
_reset_db(tmp_path, monkeypatch)
|
|
from core.inference import mcp_client
|
|
from core.inference import tools as tools_mod
|
|
|
|
monkeypatch.setattr(mcp_client, "_tool_cache", {})
|
|
monkeypatch.setattr(mcp_client, "_probe_cooloff_until", {})
|
|
mcp_servers_db.create_server(id = "s1", display_name = "A", url = "https://old/mcp", is_enabled = True)
|
|
|
|
async def fake(
|
|
url,
|
|
headers = None,
|
|
timeout = None,
|
|
use_oauth = False,
|
|
):
|
|
# The user re-points the server while the old endpoint's probe fails.
|
|
mcp_servers_db.update_server("s1", {"url": "https://new/mcp"})
|
|
raise RuntimeError("old endpoint down")
|
|
|
|
monkeypatch.setattr(tools_mod, "list_tools_async", fake)
|
|
|
|
assert asyncio.run(tools_mod.get_enabled_mcp_tools()) == []
|
|
# The failure was for the OLD config, so the new one must stay re-probable.
|
|
assert not mcp_client.in_failure_cooloff("s1")
|
|
|
|
|
|
def test_get_enabled_mcp_tools_no_cooloff_when_server_deleted_mid_failed_probe(
|
|
tmp_path, monkeypatch
|
|
):
|
|
"""A delete landing while a probe fails must not leave an orphan cool-off
|
|
entry keyed by the since-removed server id."""
|
|
import asyncio
|
|
|
|
_reset_db(tmp_path, monkeypatch)
|
|
from core.inference import mcp_client
|
|
from core.inference import tools as tools_mod
|
|
|
|
monkeypatch.setattr(mcp_client, "_tool_cache", {})
|
|
monkeypatch.setattr(mcp_client, "_probe_cooloff_until", {})
|
|
mcp_servers_db.create_server(id = "s1", display_name = "A", url = "https://x/mcp", is_enabled = True)
|
|
|
|
async def fake(
|
|
url,
|
|
headers = None,
|
|
timeout = None,
|
|
use_oauth = False,
|
|
):
|
|
mcp_servers_db.delete_server("s1")
|
|
raise RuntimeError("down")
|
|
|
|
monkeypatch.setattr(tools_mod, "list_tools_async", fake)
|
|
|
|
assert asyncio.run(tools_mod.get_enabled_mcp_tools()) == []
|
|
assert "s1" not in mcp_client._probe_cooloff_until # no orphan cool-off
|
|
|
|
|
|
def test_get_enabled_mcp_tools_skips_failed_server_during_cooloff(tmp_path, monkeypatch):
|
|
"""A down server is probed once, then skipped during the cool-off instead
|
|
of being re-probed (and re-hung) on every send."""
|
|
import asyncio
|
|
|
|
_reset_db(tmp_path, monkeypatch)
|
|
from core.inference import mcp_client
|
|
from core.inference import tools as tools_mod
|
|
|
|
monkeypatch.setattr(mcp_client, "_tool_cache", {})
|
|
monkeypatch.setattr(mcp_client, "_probe_cooloff_until", {})
|
|
mcp_servers_db.create_server(id = "s1", display_name = "A", url = "https://x/mcp", is_enabled = True)
|
|
|
|
attempts = {"n": 0}
|
|
|
|
async def fake(
|
|
url,
|
|
headers = None,
|
|
timeout = None,
|
|
use_oauth = False,
|
|
):
|
|
attempts["n"] += 1
|
|
raise RuntimeError("down")
|
|
|
|
monkeypatch.setattr(tools_mod, "list_tools_async", fake)
|
|
|
|
assert asyncio.run(tools_mod.get_enabled_mcp_tools()) == [] # probes, fails
|
|
assert asyncio.run(tools_mod.get_enabled_mcp_tools()) == [] # within cool-off
|
|
assert asyncio.run(tools_mod.get_enabled_mcp_tools()) == [] # still skipped
|
|
assert attempts["n"] == 1 # only the first send probed
|
|
|
|
|
|
def test_cache_tools_clears_failure_cooloff(monkeypatch):
|
|
"""A successful probe lifts a server's failure cool-off."""
|
|
from core.inference import mcp_client
|
|
|
|
monkeypatch.setattr(mcp_client, "_tool_cache", {})
|
|
monkeypatch.setattr(mcp_client, "_probe_cooloff_until", {})
|
|
mcp_client.record_probe_failure("s1")
|
|
assert mcp_client.in_failure_cooloff("s1")
|
|
mcp_client.cache_tools("s1", _one_tool())
|
|
assert not mcp_client.in_failure_cooloff("s1")
|
|
|
|
|
|
def test_oauth_failure_cools_off_longer_than_plain(monkeypatch):
|
|
"""An OAuth server's failure cools off longer than a plain server's, so its
|
|
multi-minute probe hang doesn't recur every minute."""
|
|
from core.inference import mcp_client
|
|
|
|
monkeypatch.setattr(mcp_client, "_probe_cooloff_until", {})
|
|
mcp_client.record_probe_failure("plain", use_oauth = False)
|
|
mcp_client.record_probe_failure("oauth", use_oauth = True)
|
|
assert mcp_client._probe_cooloff_until["oauth"] > mcp_client._probe_cooloff_until["plain"]
|
|
|
|
|
|
def test_invalidate_clears_failure_cooloff(monkeypatch):
|
|
"""Eviction drops the failure cool-off so an edited server re-probes at once."""
|
|
from core.inference import mcp_client
|
|
|
|
monkeypatch.setattr(mcp_client, "_tool_cache", {})
|
|
monkeypatch.setattr(mcp_client, "_probe_cooloff_until", {"s1": 1.0, "s2": 2.0})
|
|
mcp_client.invalidate_tool_cache("s1")
|
|
assert "s1" not in mcp_client._probe_cooloff_until
|
|
assert "s2" in mcp_client._probe_cooloff_until
|
|
mcp_client.invalidate_tool_cache()
|
|
assert mcp_client._probe_cooloff_until == {}
|
|
|
|
|
|
def test_refresh_failure_records_cooloff(tmp_path, monkeypatch):
|
|
"""A failed manual refresh starts the cool-off so the next chat send does
|
|
not immediately hang on the down server."""
|
|
import asyncio
|
|
|
|
_reset_db(tmp_path, monkeypatch)
|
|
from core.inference import mcp_client
|
|
import routes.mcp_servers as routes_mcp
|
|
|
|
monkeypatch.setattr(mcp_client, "_tool_cache", {})
|
|
monkeypatch.setattr(mcp_client, "_probe_cooloff_until", {})
|
|
mcp_servers_db.create_server(id = "s1", display_name = "A", url = "https://x/mcp", is_enabled = True)
|
|
|
|
async def boom(
|
|
url,
|
|
headers = None,
|
|
timeout = None,
|
|
use_oauth = False,
|
|
):
|
|
raise RuntimeError("down")
|
|
|
|
monkeypatch.setattr(routes_mcp, "list_tools_async", boom)
|
|
res = asyncio.run(routes_mcp.refresh_mcp_server_tools("s1", current_subject = "u"))
|
|
assert res.ok is False
|
|
assert mcp_client.in_failure_cooloff("s1")
|
|
|
|
|
|
def test_refresh_drops_result_when_config_changes_mid_probe(tmp_path, monkeypatch):
|
|
"""A manual refresh must not warm the chat cache with tools discovered
|
|
under an old config if the server is edited while the probe is in flight."""
|
|
import asyncio
|
|
|
|
_reset_db(tmp_path, monkeypatch)
|
|
from core.inference import mcp_client
|
|
import routes.mcp_servers as routes_mcp
|
|
|
|
monkeypatch.setattr(mcp_client, "_tool_cache", {})
|
|
mcp_servers_db.create_server(id = "s1", display_name = "A", url = "https://old/mcp", is_enabled = True)
|
|
|
|
async def fake_refresh(
|
|
url,
|
|
headers = None,
|
|
timeout = None,
|
|
use_oauth = False,
|
|
):
|
|
mcp_servers_db.update_server("s1", {"url": "https://new/mcp"})
|
|
return _one_tool("stale")
|
|
|
|
monkeypatch.setattr(routes_mcp, "list_tools_async", fake_refresh)
|
|
res = asyncio.run(routes_mcp.refresh_mcp_server_tools("s1", current_subject = "u"))
|
|
assert res.ok and res.tool_count == 1
|
|
assert mcp_client.get_cached_tools("s1") is None
|
|
|
|
|
|
def test_refresh_failure_no_cooloff_when_config_changes_mid_probe(tmp_path, monkeypatch):
|
|
"""A manual refresh failure for an old config must not cool off the freshly
|
|
edited server."""
|
|
import asyncio
|
|
|
|
_reset_db(tmp_path, monkeypatch)
|
|
from core.inference import mcp_client
|
|
import routes.mcp_servers as routes_mcp
|
|
|
|
monkeypatch.setattr(mcp_client, "_tool_cache", {})
|
|
monkeypatch.setattr(mcp_client, "_probe_cooloff_until", {})
|
|
mcp_servers_db.create_server(id = "s1", display_name = "A", url = "https://old/mcp", is_enabled = True)
|
|
|
|
async def boom(
|
|
url,
|
|
headers = None,
|
|
timeout = None,
|
|
use_oauth = False,
|
|
):
|
|
mcp_servers_db.update_server("s1", {"url": "https://new/mcp"})
|
|
raise RuntimeError("old endpoint down")
|
|
|
|
monkeypatch.setattr(routes_mcp, "list_tools_async", boom)
|
|
res = asyncio.run(routes_mcp.refresh_mcp_server_tools("s1", current_subject = "u"))
|
|
assert res.ok is False
|
|
assert not mcp_client.in_failure_cooloff("s1")
|
|
|
|
|
|
def test_get_enabled_mcp_tools_drops_result_when_server_deleted_mid_probe(tmp_path, monkeypatch):
|
|
"""A delete landing while a probe is in flight must drop the now-orphan
|
|
result -- the `fresh is None` arm of the mid-probe TOCTOU guard. The
|
|
result is neither served nor cached under the since-removed id."""
|
|
import asyncio
|
|
|
|
_reset_db(tmp_path, monkeypatch)
|
|
from core.inference import mcp_client
|
|
from core.inference import tools as tools_mod
|
|
|
|
monkeypatch.setattr(mcp_client, "_tool_cache", {})
|
|
monkeypatch.setattr(mcp_client, "_probe_cooloff_until", {})
|
|
mcp_servers_db.create_server(id = "s1", display_name = "A", url = "https://x/mcp", is_enabled = True)
|
|
|
|
async def fake(
|
|
url,
|
|
headers = None,
|
|
timeout = None,
|
|
use_oauth = False,
|
|
):
|
|
# Simulate a DELETE landing while we await the probe.
|
|
mcp_servers_db.delete_server("s1")
|
|
return _one_tool()
|
|
|
|
monkeypatch.setattr(tools_mod, "list_tools_async", fake)
|
|
|
|
specs = asyncio.run(tools_mod.get_enabled_mcp_tools())
|
|
assert specs == [] # orphan result not served
|
|
assert mcp_client.get_cached_tools("s1") is None # nor cached under a gone id
|
|
|
|
|
|
def test_oauth_probe_failure_in_chat_path_uses_long_cooloff(tmp_path, monkeypatch):
|
|
"""When an OAuth server fails discovery during a send, the chat path must
|
|
record the OAuth (long) cool-off, not the plain one -- otherwise its
|
|
multi-minute browser hang recurs every minute."""
|
|
import asyncio
|
|
import time
|
|
|
|
_reset_db(tmp_path, monkeypatch)
|
|
from core.inference import mcp_client
|
|
from core.inference import tools as tools_mod
|
|
|
|
monkeypatch.setattr(mcp_client, "_tool_cache", {})
|
|
monkeypatch.setattr(mcp_client, "_probe_cooloff_until", {})
|
|
mcp_servers_db.create_server(
|
|
id = "s1",
|
|
display_name = "A",
|
|
url = "https://x/mcp",
|
|
is_enabled = True,
|
|
use_oauth = True,
|
|
)
|
|
|
|
async def boom(
|
|
url,
|
|
headers = None,
|
|
timeout = None,
|
|
use_oauth = False,
|
|
):
|
|
raise RuntimeError("oauth down")
|
|
|
|
monkeypatch.setattr(tools_mod, "list_tools_async", boom)
|
|
|
|
assert asyncio.run(tools_mod.get_enabled_mcp_tools()) == []
|
|
assert mcp_client.in_failure_cooloff("s1")
|
|
# The recorded window must exceed the plain cool-off, proving the OAuth
|
|
# branch (use_oauth=True) fired -- not the 60 s default.
|
|
remaining = mcp_client._probe_cooloff_until["s1"] - time.monotonic()
|
|
assert remaining > mcp_client.FAILED_PROBE_COOLOFF_SECONDS
|