meta-pytorch/OpenEnv was transferred to huggingface/OpenEnv. The old
URL still works via GitHub's redirect from a clean clone, but uv's git
cache can fail to follow it on some machines — surfacing as a
'could not read Username for github.com' credential prompt mid-install.
Point the requirement at the canonical huggingface/OpenEnv.git (same
HEAD), which also sidesteps any stale cache entry keyed on the old URL.
The completion toast reported only document count. Capture each job's
num_chunks from its complete event into the index-progress store and sum
across the batch, so the toast reads 'N documents and M chunks indexed'.
Already-indexed (deduped) files contribute 0 new chunks.
Uploading several documents (or a folder) produced a separate toast per
file, which piled up. Replace the per-job toast stack with a single
aggregate toast driven by a new index-progress-store that's populated at
addDoc time — so it counts queued files (held by the concurrency
semaphore) in the denominator, which the per-job rag-store map can't see.
- index-progress-store: one entry per file in the batch
(queued/indexing/ready/error + 0..1 progress).
- Both addDoc paths register each file on entry and update it through
the lifecycle (setIndexing after acquiring a slot; setProgress on job
progress events; setReady/setError on terminal).
- ingestion-toast-stack: renders ONE toast — 'Indexing document(s) · X/Y
· Z%' with a progress bar while in flight (overall = completed files +
in-flight fractions, over total), 'RAG index ready · N documents
indexed' (+ failures) when the last finishes, then auto-dismisses.
Single-file uploads still read naturally ('Indexing document' / '1
document indexed').
IngestionProgress / rag-store jobs are untouched (still used by the
KB detail panel). Not build/UI verified here (no bun).
Uploading many docs (or a folder) previously spawned an ingestion
subprocess per file all at once, thrashing the GPU/CPU. Add a
configurable concurrency limit and a folder picker.
- ragIndexConcurrency setting (default 1) in the chat runtime store,
persisted like the other RAG scalar settings; exposed as a 'Parallel
indexing' slider (1-8) at the bottom of the sidebar Retrieval section.
- New rag-index-queue.ts semaphore: each document upload acquires a slot
before it starts and releases it once its ingestion job finishes
(complete / error / already-indexed), so bulk uploads drain at the
configured rate. Wired into both composer upload paths
(use-thread-doc-uploads + shared-composer).
- Folder upload: a second 'Attach a folder' button on the RAG attach
control uses a webkitdirectory input; every compatible file is routed
through the same queue. Multi-file select already worked (the input has
'multiple' and loops addDoc).
- Content-hash dedup (shipped earlier) means re-scanning a folder skips
already-indexed files.
Not build/UI verified here (no bun); needs bun typecheck + a browser
check of bulk/folder upload draining at the set concurrency.
External providers (OpenAI/Anthropic/Gemini) can't run the local
search_knowledge_base tool loop, so give them RAG by prefetching:
studio retrieves before calling the provider, injects the chunks into
the user prompt, and surfaces it as a synthetic tool call. Local models
are untouched (they keep tool-based RAG + decomposition).
Backend:
- New POST /api/rag/prefetch: momentarily loads the pre-cached helper
(gemma-4-E2B-it-GGUF) via LlamaCppBackend(kill_orphans=False) to
decompose the question into up to 3 queries, retrieves+merges+dedups
per query, unloads the helper. Raw single-query fallback if the helper
can't load. New core/rag/query_decompose.py owns the helper lifecycle.
- Factored the retrieval body of /search into _execute_search, reused by
both endpoints.
Frontend:
- prefetchRag() client.
- chat-adapter external branch: gated on isExternalRequest + ragToolEnabled
+ scope!=off + ragScopeHasDocs (no docs -> no prefetch, prior behavior
preserved). Formats hits as <chunk id=N> (parseChunks shape), injects
into the last user message (send-only; not shown in the user bubble),
seeds a synthetic search_knowledge_base tool-call part so the existing
chunk-card UI + [N] citations + source badges all work unchanged.
- Extends PR #5674's disabled-tool guard: when RAG is off, reinforce
'no document search (RAG) capabilities'; when prefetch ran, point the
model at the injected excerpts instead.
- RAG pill enabled for external providers regardless of supports_tools.
Not build/UI verified here (no bun/GPU/keys); needs bun typecheck+test
and a browser round-trip with real provider keys.
RAG retrieval runs entirely through the local search_knowledge_base
tool. If the loaded model doesn't support tool calling (e.g. a
safetensors model whose template advertises tools in an unparsable
emission format, so supports_tools is suppressed), enabling RAG does
nothing — the model never calls the tool. The pill stayed lit and
clickable, which was misleading.
Gate the RAG pill on supportsTools (in addition to modelLoaded), the
same condition web/code use when there's no provider builtin. Applied
to both composer surfaces (shared-composer and the in-thread
RagToggle), with a 'RAG needs a model that supports tool calling'
tooltip on the disabled state.
The backend dedups re-uploads and the sidepanel shows the doc once, but
the composer's pending-doc chips are created per addDoc call, so each
re-upload of an already-indexed file appended another 'Ready' chip for
the same document. In the already_indexed branch, if a chip with the
returned documentId already exists, drop the chip we just added instead
of marking it ready — so the composer shows each document only once.
Re-uploading the same file into the same scope (KB or thread) used to
parse, chunk, caption and embed it all over again, creating a duplicate
set of chunks. Dedup by content hash instead:
- schema: add rag_documents.content_hash (sha256 of the bytes) via the
standard PRAGMA/ALTER migration, plus (scope, content_hash) indexes.
- upload: _save_upload now streams the bytes through sha256 and returns
the digest alongside path/name/size.
- _start_ingestion: before inserting, look for a COMPLETED row in the
same scope with the same hash. If found, delete the redundant upload
from disk and return the existing document_id with already_indexed=
true and an empty job_id — no ingestion job is started. Only
'completed' rows dedup, so a failed/in-flight prior attempt can still
retry. Scope-local: the same file in two KBs is indexed in each.
- frontend: UploadResponse.already_indexed flows through the rag-store
(skips job subscription) into both upload paths, which mark the chip
ready immediately and toast '<file> is already indexed'.
Pre-existing rows have NULL content_hash and won't dedup until
re-uploaded once under the new path. Not build/UI-verified here (no bun
in this env); needs typecheck + browser check.
docx-preview defaults to blob: URLs for embedded images, which DOMPurify
strips from img src (blob: isn't in its default allowed-URI list), so
figures vanished after sanitize. Switch docx-preview to useBase64URL so
images inline as data: URIs, and add ADD_DATA_URI_TAGS: ['img'] to the
DOMPurify config so those data: URIs survive sanitization. Script /
handler / javascript: stripping is unchanged.
Previously a DOCX citation only showed the extracted snippet + a
Download button (Risk #3: never render a user-supplied .docx inline).
Add a faithful inline render that keeps that guarantee:
- New PreviewDocxView renders the .docx with docx-preview into an
off-screen element, then injects DOMPurify-sanitized HTML into the
live DOM (keeping <style> for docx-preview's scoped layout CSS).
Script tags, event handlers and javascript: URLs are stripped, so
a malicious .docx can't execute in the app origin.
- preview-store now fetches the raw bytes for docx and exposes them
via previewBlob, but deliberately keeps previewBlobUrl = null — no
object URL is created, so the 'open raw original inline' path stays
disabled (Risk #3) and Download remains the only raw-file path.
- preview-panel routes docx -> PreviewDocxView when a blob is present,
falling back to the text-view snippet otherwise. isInlineBlobAllowed
still returns false for docx, so html/unknown behaviour is unchanged.
- Deps: docx-preview + dompurify added to package.json.
- Tests updated: docx now asserts bytes-fetched-without-object-URL.
Not build/UI-verified in this environment (deps not installed here);
needs bun install + browser check.
- Add human-readable stage labels for caption_images ('Captioning
images') and extract_images ('Extracting images') so the raw
underscore stage names no longer leak into the progress toast.
- On completion, the toast title is now 'RAG index ready' (was
'Indexed') and the body reads '1 document and N chunk(s) indexed'
(was 'Indexed N chunks'), with chunk pluralization.
CodeQL flagged information exposure through an exception in the /warmup
and /reranker/precache endpoints: both returned str(exc) in the JSON
body, exposing internal paths and stack details to the client. Keep
the full exception in the server-side warning log and return a generic
error message ('Failed to load embedder' / 'Failed to download
reranker') to the caller instead. The frontend only surfaces the
message in a toast, so a generic string is sufficient.
Two changes to the RAG captioning log output:
- Drop the noisy per-image and path-selection info lines
(using-chat-VLM, loading-helper, per-image done). Only the
'caption_images: invoked' and 'caption_images: complete' lines
remain; warnings for genuine failures (helper load, per-image
request, helper unload) are kept.
- Configure structlog at the top of the ingestion subprocess worker
with the same env the parent uses. The worker runs in a spawned
process where structlog was never set up, so its logs fell back to
structlog's dev ConsoleRenderer ([info] ...) instead of the JSON
renderer the rest of the app uses. Now captioner/parser logs from
the subprocess match the parent's JSON format.
The toast stack and the chat settings sheet were both z-50, so an open
side panel (rendered later in the DOM) covered the indexing toast. Bump
the stack to z-[9999] — comfortably above the sheet's z-50 — so the
ingestion toast stays visible like the Sonner reranker toast does.
The thumbnail-rail refactor hoisted <Document> to wrap both the rail and
the main page so the PDF loads once. That moved the width-measuring scroll
container INSIDE <Document>, which only renders its children after the PDF
finishes loading. The old `useEffect(..., [])` ran on component mount —
when the container was still absent — so the ResizeObserver never attached,
`width` stayed null, and the main <Page> collapsed to width 0.
Replace the mount-effect measurement with a callback ref: the
ResizeObserver now attaches the instant the container node mounts,
regardless of when that happens relative to PDF load. Disconnects cleanly
on unmount / re-attach.
Verified: tsc clean, vite build succeeds, preview-pdf-smoke (incl. the
resize/debounce case) passes.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix: honor --ctx-size and other forwarded args from `unsloth studio run` in Studio's context-fit logic
* refactor: extract resolve_requested_ctx as single source of truth
The test helper was reimplementing the two-line
'ctx_override = parse_ctx_override(...); requested_ctx = ctx_override
if ctx_override is not None else n_ctx' pattern locally, so the test
asserted against its own reimplementation rather than production logic.
Extract the conditional into resolve_requested_ctx and have both the
production caller and the test use it.
* fix(studio): honor pass-through cache type flags in KV VRAM estimate
Studio's KV cache VRAM estimate computed from the first-class
cache_type_kv even when the user passed -ctk/--cache-type-k/-ctv/
--cache-type-v via extras. Those flags reached llama-server fine
(last-wins on the CLI) but the pre-launch estimate kept using the
default f16 bytes-per-element, so GPU placement decisions could be
off when the user lowered cache precision via pass-through.
Adds parse_cache_override + resolve_cache_type_kv in llama_server_args.py
(mirroring parse_ctx_override / resolve_requested_ctx), wires both into
load_model alongside the existing ctx resolution, and adds focused
unit tests for the parser + resolver.
Follow-up to @rolandtannous review on #5815.
---------
Co-authored-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
Co-authored-by: Lee Jackson <130007945+Imagineer99@users.noreply.github.com>
assistant-ui mints a fresh __LOCALID_* draft id on every page load, so
RAG docs uploaded under the previous draft id were orphaned after a
refresh — the doc panel queries useThreadDocuments(activeThreadId) and
the new id had nothing.
Persist activeThreadId in localStorage and, on the first settled render
after load, have ActiveThreadSync ask aui to switchToThread(persisted)
when it differs from the freshly-minted draft. Because the draft was
already persisted to the backend by initialize()/ensureThreadRecord
when its first doc was uploaded, the adapter's fetch() resolves it and
aui adopts it as mainThreadId. That keeps aui's mainThreadId and our
activeThreadId unified, so the earlier divergence (uploads under the
persisted id vs chat-completion reading aui's fresh id) can't recur —
unlike the reverted localStorage-only attempt, the chat-adapter's
unstable_threadId now equals the persisted id after the switch.
A one-shot ref ensures we only re-adopt on initial load; user-driven
new-chat / thread switches still flow through normally. If the
persisted draft was never initialized (no doc/message, not in the
backend), switchToThread rejects and we fall back to the fresh draft.
Follow-ups after merging origin/main into feature/rag:
* chat-adapter.ts: drop `score` field from DocumentSourcePart and the
`chunk.score` copy — the remote "hide RAG retrieval scores from chunks,
citations, and side panel" commit removed `score` from ParsedChunk.
* chat-settings-sheet.tsx: remove the duplicate `const activeThreadId =`
introduced by the merge (kept the HEAD-side declaration at line 497).
* chat-settings-sheet.tsx: drop the "Min relevance" Slider that referenced
`ragMinScore` / `setRagMinScore` — same intent as the hide-scores commit
(these are still on the runtime store but the side-panel UI is gone).
* i18n locales (en, zh-CN): add `settings.tabs.knowledgeBases` translation
key so the new TabDef entry passes the TranslationKey union check.
Verified: `tsc --noEmit` clean, `vite build` succeeds.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Snapshot taken before fast-forwarding feature/rag to origin and merging main.
Bundles in-flight work so the merge has a clean tree:
Frontend
- PDF preview panel (preview-panel, preview-pdf-view, preview-text-view,
preview-unavailable) with lazy-rendered page thumbnail rail
- Resizable preview slot via useResizablePanelWidth hook (drag handle,
localStorage persistence, viewport clamping)
- Neutral scrollbar + Source Excerpt card restyle (no brand-coloured rail)
- Preview-store + chat-adapter / rag-api / kb-detail wiring
- Frontend test harness (vitest.config, setupTests, biome update) and the
paired __tests__ suites for preview, sources, document-row, chat-adapter,
rag-api, knowledge-bases-tab, search-knowledge-base-tool-ui
Backend
- RAG locator + authorization modules with chunking / retrieval / tool /
vector_store / studio_db updates
- Paired test_rag_* suites (authorization, locators, locator_backfill,
locator_migration, preview_routes, preview_target_locators, source_identity)
Other
- tests/fixtures/rag-preview for preview route fixtures (sample.pdf,
sample.txt, make_fixture_pdf.py)
- .gitignore + package(-lock).json adjustments for the new test runner
Will be squashed/reworked via interactive rebase after main is merged.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The two previous commits (41e43b6c7 and 3d3a00c2b) persisted
activeThreadId in localStorage so RAG docs would survive a page
reload. That broke fresh chats: stale localStorage values from a
prior session pinned activeThreadId to an old draft id, but the
chat-completion path reads aui's current mainThreadId via
unstable_threadId. The two diverged — uploads went under the stale
persisted id, the chat-completion turn looked up docs under the new
aui id, and nothing matched.
Revert the persistence + ActiveThreadSync guard. We're back to the
pre-fix behaviour where uploads-in-the-same-session work, and a
proper fix for the reload case (promote drafts to real chat_threads
rows on first doc upload so the id never changes) will land next.
ActiveThreadSync was reacting to aui's mainThreadId === null on mount
(aui hasn't booted yet) by calling setActiveThreadId(null), which
wiped the just-restored persisted draft id from localStorage and
emptied the doc panel for the user's thread. The previous fix only
covered the 'aui minted a different LOCALID' branch; it missed the
'mainThreadId is null while aui boots' branch.
Treat a null mainThreadId as a no-op for the sync. Explicit clears
(new chat, sidebar delete) keep going through setActiveThreadId(null)
directly, so this guard doesn't trap stale state — it just gives the
persisted draft id a chance to survive until aui finishes booting.
* style: remove dark mode upload circle
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
---------
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
* Detect CUDA UMD Version from newer nvidia-smi output (#5812)
Newer NVIDIA drivers (e.g. 610.x on Windows) print the driver's max
CUDA capability as "CUDA UMD Version: X.Y" instead of the legacy
"CUDA Version: X.Y" header. The installers and Studio setup scripts
were only matching the legacy spelling, so on a fresh RTX 5090
laptop with a 13.x driver they failed to detect any CUDA version
and fell through to the cu126 wheel default.
Accept both spellings everywhere we parse nvidia-smi output:
- install.ps1: Get-TorchIndexUrl regex now allows " UMD"
- install.sh: two-expression sed (POSIX BRE has no "?"); the two
patterns are mutually exclusive per line, head -1 picks the match
- studio/setup.ps1: Get-PytorchCudaTag and the $DriverMaxCuda
detector both relaxed
- studio/install_llama_prebuilt.py: substring scan replaced with a
regex search using the same pattern
- tests/sh/test_get_torch_index_url.sh: new make_mock_smi_umd helper
plus three UMD cases (13.3 -> cu130, 12.8 -> cu128, 11.8 -> cu118);
all 30 tests pass locally
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
---------
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
When the user uploads a doc to a brand-new chat (a draft thread with
an assistant-ui __LOCALID_* id), the backend stores rag_documents
rows scoped to that id. On page reload assistant-ui mints a fresh
__LOCALID_* for the new mainThreadId, so useThreadDocuments asks the
backend for docs under the NEW id and gets nothing, even though the
original rows are still on disk under the OLD id.
- chat-runtime-store: persist activeThreadId in localStorage via
a new CHAT_ACTIVE_THREAD_KEY, restore on init, save on every
setActiveThreadId call (including clears, which write '').
- ActiveThreadSync: when aui's freshly-minted mainThreadId is a
__LOCALID_* and we already have a persisted __LOCALID_* draft,
keep ours instead of overwriting. This only affects RAG/doc
lookup; aui's chat history for the new draft starts empty
either way, so there's no regression for users who don't have
attached docs.
User-initiated thread switches (new chat, switching to a saved
thread, deleting the current thread) all go through setActiveThreadId
with the new id (or null), so they correctly replace/clear the
persisted value.
RAG is opt-in but the chat store had a hydration-time migration that
silently flipped ragToolEnabled to true whenever a persisted ragSource
was anything other than 'off'. Plus there was no logic to reset the
pill across model loads, so the pill stayed on across sessions even
after the user explicitly disabled and re-enabled it.
- Remove the migration block in chat-runtime-store.hydrate — the
embedder warmup still runs when ragToolEnabled is genuinely
persisted true.
- In use-chat-model-runtime's load-success handler, call
setRagToolEnabled(false) (via the setter, so localStorage stays
in sync) immediately after the loaded-state setState batch. Every
fresh model load now starts with the pill off and the user must
toggle it explicitly.
get_reranker() acquires the module-level _lock and then, on first load,
calls unload() to clear any stale state before _load() instantiates the
CrossEncoder. unload() acquires the same _lock — but threading.Lock is
non-reentrant, so the second acquisition by the holding thread blocked
forever. Symptom: rerank=True hung the search_knowledge_base tool with
no further log output past 'rerank entered'.
Switch to threading.RLock so the same thread can re-enter without
blocking. unload()'s independent callers still work the same way; the
only behaviour change is that re-entrant acquisition from one thread
now succeeds.
When the reranker hung on rerank=True there were zero log lines after
'retrieved=N (no threshold)', which made it impossible to tell whether
the hang was in _load (CrossEncoder construction), in get_reranker's
lock acquisition, or in predict. Structlog routing may also be the
culprit since we never saw the 'Loading RAG reranker' info line.
Add unconditional stderr prints at each milestone — entered, device
resolved, before CrossEncoder, after CrossEncoder, rerank entered,
predict starting, predict done. These bypass any logger config and
show up directly in /tmp/studio.log next to the rest of the captured
stdout/stderr. Leaving structlog logger.info calls in place too so
the structured stream still gets the same data when routing works.
Auto-downloading a 1.1 GB cross-encoder at every studio start is
wrong for users who never use rerank — reranker is opt-in by design.
Move the precache from a startup daemon thread to an explicit
POST /api/rag/reranker/precache endpoint, and have the chat settings
sheet call it the moment the 'Use reranker' switch is flipped on.
- Backend: drop the startup _precache_reranker thread; add the
/api/rag/reranker/precache route that calls precache_reranker().
- Frontend: new precacheRagReranker() in rag-api, wired into the
Switch's onCheckedChange so the download runs synchronously
with a loading toast. On success: 'Reranker ready'. On failure:
error toast + auto-flip the switch back off so the next query
doesn't trigger another long hang.
First toggle-on pays the 1.1 GB download once; subsequent toggles
hit the HF cache and return ~instantly.
The reranker model (BAAI/bge-reranker-base by default, ~1.1 GB) was
never precached, so the first user-facing rerank call paid the full
download cost — which on slow connections looked like a hang and got
retried by upstream timeouts. The deprecation warning that surfaced
during the hang was actually from sentence-transformers internals
firing while the download was still in flight.
Mirror the precache_helper_gguf pattern: add precache_reranker() that
calls snapshot_download in a daemon thread at FastAPI startup. The
first opt-in rerank now finds the weights already on disk and only
pays the in-process model load.
Also tighten the loader:
- explicit device selection (cuda when torch.cuda.is_available,
else cpu) so we don't rely on sentence-transformers auto-detect
behaviour that has historically picked cpu under odd
CUDA_VISIBLE_DEVICES configs;
- structlog-shaped logs with elapsed_seconds around load + predict
so a real runtime hang is visible in /tmp/studio.log with
'RAG reranker predict starting' / 'RAG reranker predict done'.
* clear mrope state after generation
* move clear mrope to here
* [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
---------
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Daniel Han <danielhanchen@gmail.com>
* added remote MCP server support
* trim
* added tests
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* increased timeout
* disabling MCP chat toggle
* Fix MCP OpenAI function-name validation + cancel propagation for PR #5750
OpenAI requires function.name to match ^[a-zA-Z0-9_-]{1,64}$ before
streaming starts. The existing 64-char length check is necessary but
not sufficient: MCP servers can return tool names containing '.', '/',
spaces, etc. that would 400 the whole chat request. Validate the
composed mcp__<server_id>__<tool> name against the regex, skip + warn
on miss, and drop duplicate tool names from the same server (which
would also 400 the request as "duplicates").
Also propagate the agentic-loop cancel_event into MCP tool execution
so a /cancel POST during a long-running MCP call (e.g. GitHub MCP
search across a large repo) actually interrupts the in-flight HTTP
call instead of waiting out the 300 s timeout. The watcher polls the
threading.Event at 50 ms cadence inside the asyncio loop (matches
routes/inference.py's existing cancel-watcher cadence) and races
against the call task with asyncio.wait FIRST_COMPLETED.
Tests added:
- test_mcp_specs_skip_invalid_openai_function_names: drops bad chars
- test_mcp_specs_skip_empty_tool_name
- test_mcp_specs_drops_duplicate_names
- test_call_tool_sync_respects_pre_set_cancel_event
Also fix test_desktop_auth.py's router stub that listed every existing
router but missed mcp_servers_router, so importing main.py fails after
this PR adds it to routes/__init__.py.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* PR #5750 round 2: OAuth cleanup on delete/url-change + mcp_enabled standalone
Round 2 of cross-platform validation surfaced two more P1 findings:
1. OAuth tokens never get cleared. fastmcp keys tokens by MCP URL, not by
server row, and delete / URL change / use_oauth toggle only updated
the SQLite row. Re-registering the same URL would silently reuse the
old account's credentials. Adds clear_oauth_tokens_async() in
mcp_client.py and calls it from the delete + put route handlers when
the row had use_oauth=True and either the URL changes or OAuth is
turned off.
2. mcp_enabled=true was ignored unless the caller also sent
enable_tools=true. The frontend always sends both together so the UI
path was fine, but a direct API caller sending only mcp_enabled would
silently get no MCP tools, which contradicts the field's documented
"append tools from every enabled MCP server" behavior. Loosens the
use_tools gate in both the GGUF and safetensors paths so mcp_enabled
opens the tool loop on its own; when the caller did not also opt
into built-ins, the built-in list starts empty.
Tests added:
- test_clear_oauth_tokens_async_no_op_safe
- test_delete_server_calls_oauth_cleanup_when_oauth_was_on
- test_delete_server_skips_oauth_cleanup_when_oauth_off
- test_update_server_clears_oauth_on_url_change
- test_update_server_clears_oauth_when_oauth_disabled
26 backend MCP tests pass; full studio/backend suite 1710 passed locally.
Cross-platform CI (Linux, macOS, Windows) green on staging fork.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* PR #5750 round 3: reject null bool updates + /test surfaces 400
Round 3 of cross-platform validation:
1. PUT /api/mcp/servers/<id> would 500 with TypeError when the body
explicitly set is_enabled or use_oauth to null. Pydantic accepts
None for an Optional[bool] and _changes_from_payload then passed
None into mcp_servers_db.update_server, which int(None)d. Reject
explicit null at the validation layer with 400 instead.
2. POST /api/mcp/servers/test caught HTTPException under
"except Exception", so an invalid URL came back as HTTP 200 with
{"ok": false, "error": "400: ..."} instead of a real 400. The
create + update paths return 400 for the same input. Move
validation outside the transport try/except so it surfaces 400.
Tests added:
- test_changes_from_payload_rejects_null_is_enabled
- test_changes_from_payload_rejects_null_use_oauth
- test_test_endpoint_surfaces_url_validation_as_400
* PR #5750 round 4: hyphenated MCP tool names + empty-tool-list gate
Round 4 surfaces two more interaction bugs between the new MCP path
and existing safetensors tool plumbing:
1. OpenAI accepts ^[a-zA-Z0-9_-]{1,64}$ for function.name, and round 1
widened the MCP regex to that set, so MCP tools can now be advertised
as `mcp__srv__list-issues`. But the XML tool-call parser in
tool_call_parser.py used `\w+` (no hyphen), so the model could call
the tool but Studio could not parse the call. Same in
routes/inference.py's `_TOOL_XML_RE` stripper, which would leave
hyphenated tool-call XML in the visible content. Both regexes now
use `[\w-]+`.
2. safetensors_agentic treats `tools=[]` as "allow all" (documented
contract, exercised by test_empty_tools_list_does_not_enforce_allowlist).
When a caller sends `enable_tools=true` + `enabled_tools=[]` +
`mcp_enabled=true` and MCP discovery returns 0, the resolved tool
list is genuinely empty and built-in tools (web_search / python /
terminal) could execute via the model's emitted call. Fix at the
route gate instead of breaking the documented contract: set
`use_tools=False` when the resolved list is empty, in both GGUF and
safetensors paths. Existing callers who omit `enabled_tools` still
get ALL_TOOLS and are unaffected.
Tests added (32 total):
- test_tool_xml_parser_handles_hyphenated_function_names
- test_tool_xml_strip_handles_hyphenated_function_names
- test_safetensors_agentic_empty_allowlist_still_means_allow_all
(documents the contract round 4 preserved)
1716 passed locally; cross-platform CI on staging fork still green.
* PR #5750 round 5: GGUF allow-list + CLI policy + hyphenated params + cancel race
Round 5 of parallel-reviewer aggregation surfaced six additional
findings; five are real and fixed here:
1. Hyphenated MCP parameter names (`<parameter=issue-number>`) were
dropped by the XML parser's `\w+` regex. Extended to `[\w-]+` in
both core/inference/tool_call_parser.py and core/tool_healing.py.
The latter is GGUF's own copy of the parser/strip patterns and was
missed by round 4.
2. core/tool_healing.py's `strip_tool_call_markup` still used
`<function=\w+>` so hyphenated MCP tool-call XML leaked into the
GGUF visible content even after round 4 fixed the shared parser.
3+4. `mcp_enabled` re-opened the tool loop even when the operator
passed `unsloth run --disable-tools` (CLI policy False). Round 2's
`(_tools_on or payload.mcp_enabled)` gate ignored the raw process
policy. Now reads `state.tool_policy.get_tool_policy()` and gates
mcp_enabled on `_cli_policy is not False`. Applied to both GGUF
and safetensors paths.
5. GGUF's agentic loop called `execute_tool(tool_name, ...)` without
checking the model-emitted name against the per-request tool list,
while the safetensors loop already enforces this. Added the same
allow-list check so a model that hallucinates a filtered MCP name
or a built-in the caller opted out of returns "not enabled" instead
of executing.
Bonus P2 fixes:
- `call_tool_sync` now checks `cancel_event.is_set()` BEFORE
creating the call task, so a pre-set cancellation does not open
the HTTP transport.
- `clear_oauth_tokens_async` moved the OAuth import + construction
inside the protected try block; a fastmcp.client.auth load error
used to escape and 500 the delete / update route.
NOT fixed (verified false or out of scope):
- finding #10 "structured_content vs structuredContent": fastmcp's
CallToolResult dataclass uses snake_case (verified live against
structured-only tool result; fields are
`dict_keys(['content', 'structured_content', 'meta', 'data', 'is_error'])`).
- finding #11 "asyncio.run from running loop": call_tool_sync is
invoked from `asyncio.to_thread` worker threads which have no
event loop; asyncio.run() is safe there.
Tests added (37 total): hyphenated param names, tool_healing strip,
GGUF allow-list gate, cancel pre-set short-circuit, OAuth cleanup
constructor-error swallowing. 1721 passed locally, no regressions.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
---------
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: danielhanchen <danielhanchen@gmail.com>
