mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-10-25 19:52:29 +00:00
d5819becf2
* 🎯 feat: Per-Tool Intent Labels for Model Specs A model spec could only turn intent labels on for ALL of its eligible tools. Saved agents have had per-tool control since the capability landed (`tool_options[id].describe_intent`, with the builder toggle following in the UI slice), but a model spec is admin YAML that produces an ephemeral agent — there is no agent document to hold per-tool options, so `describeIntent: true` synthesized an entry for every eligible tool. `describeIntent` now accepts a string array alongside the boolean, matching the `skills` field already in the same schema: describeIntent: true # every eligible tool describeIntent: ['web_search', 'search_code_mcp_github'] This matters because the label costs schema tokens on every request, so an admin may want it on a handful of illegible calls rather than the whole toolset. - Named tools still pass eligibility, so an excluded tool cannot be forced on by listing it. - An empty array reads as disabled. - Names that are not eligible or not equipped on the spec are logged rather than silently skipped — a typo in a spec would otherwise be undiagnosable. - The ephemeral toggle stays boolean and stays global even when a spec list is present: it has no per-tool UI to drive it, so narrowing it would silently cover fewer tools than the user asked for. `runInBackground` has the same all-or-nothing limitation and could take the same shape; left alone here to keep this change reviewable. * 🎯 feat: Per-Tool Background Dispatch for Model Specs Gives `runInBackground` the same `boolean | string[]` shape as `describeIntent`, so the two per-tool capabilities are configured identically from a model spec: runInBackground: true # every eligible tool runInBackground: ['slow_report_mcp_analytics'] # only this one Selectivity matters more here than for intent labels. An intent label is inert — it costs tokens and nothing else. Backgrounding changes execution semantics: the model gets a synthetic handle and must poll. Letting an admin detach one slow MCP call without making every other tool in the spec detachable is the difference between a usable setting and an all-or-nothing one. Same guarantees as the intent equivalent: - Named tools still pass eligibility, so the exclusion list still holds — a list cannot force on web_search, file_search, image gen, the HITL tool, or anything whose attachments/artifact continuity would break. - Empty array reads as disabled. - Unmatched names are logged rather than silently skipped. - The ephemeral toggle stays boolean and global; it has no per-tool UI. Also fixes a latent no-op: this function did not skip the lazily-expanded `mcp_all` placeholder, so a spec with an overlay MCP server recorded an option under a name `applyBackgroundToolCalls` can never match. The intent equivalent already skipped it; now both do. * ♻️ refactor: One Definition of the mcp_all Placeholder Guard Fixing the background no-op left the placeholder prefix declared twice — once per capability synthesizer — which is the same duplicated-literal shape that made the intent label marker fragile: two copies that must agree, with drift producing a silent no-op rather than an error. `MCP_ALL_PLACEHOLDER_PREFIX` and `isMCPAllPlaceholder` now live beside `mcpToolPattern` in mcp/utils, so both synthesizers cannot disagree about which tool entries to ignore, and anything added later that keys per-tool config by exact name has an obvious guard to reach for. Audited the rest of the capability family while here: only background and intent synthesize per-tool options from a model spec. `defer_loading` and `allowed_callers` have no model-spec path at all, so neither can carry this bug. Both synthesizers now have an explicit regression test naming the placeholder. * 🧯 fix: Treat a describeIntent List as a Selection Policy, Not a Filter Two build/behavior defects from the previous commits. **Narrowing did not actually narrow.** Omitting a tool from the synthesized options is not the same as opting it out, because intent has two default-on paths background does not: `isIntentOptedIn` treats every NATIVE_INTENT_TOOL_NAMES member as enabled when it finds no entry, and `sanitizeIntentLabels` keeps an SDK-native label unless it sees an explicit `describe_intent: false`. So `describeIntent: ['web_search']` still labelled `set_memory`, and an empty list — the most explicit way to say "none" — disabled nothing at all. A list is now a selection policy: selected eligible tools get true, unselected get an explicit false. Ineligible tools still get no entry at all. Background needs no equivalent change: it opts in on `run_in_background === true` only, with no default-on set, so omission there genuinely means off. **Fixed the CI build break.** `MCP_ALL_PLACEHOLDER_PREFIX` was exported without a type annotation, which `tsc --noEmit` accepts but the package build rejects under `--isolatedDeclarations` (TS9010) — the same reason `mcpToolPattern` beside it is annotated `: RegExp`. Verified with an actual `npm run build` this time, not just a typecheck. * 🧯 fix: Propagate Intent Selection Through Capability Marker Expansion A model spec's `tools` carries capability MARKERS, not the definition names initialization actually registers, so an option recorded under a marker never matches the tool it becomes — the same silent no-op as an `mcp_all` placeholder entry. Harmless for an opt-IN (the tool keeps its default) but not for the opt-OUTs the previous commit introduced: `memory` becomes `set_memory` + `delete_memory`, both default-on natives, and `execute_code` becomes `bash_tool`, which carries an SDK-native label that survives unless sanitize sees an explicit false. So `describeIntent: ['web_search']` on a spec with `memory: true` still labelled both memory tools, and `[]` disabled neither. `expandIntentToolOptions` propagates a marker's value to the names it expands into, mirroring `expandCodeToolOptions` in background.ts which solves the same problem for the code marker. Applied at APPLY time rather than synthesis, so hand-edited saved agents that key options by marker benefit too, not just synthesized specs. An explicit per-tool entry always wins — expansion only fills names the caller did not already decide — and it runs in both `applyIntentLabels` and `sanitizeIntentLabels`, since the SDK-native strip reads the same options. Verified with an actual package build, not just a typecheck. * ♻️ refactor: Resolve Spec Tool Selections Against Final Definitions Three review rounds hit the same root cause from three directions: a per-tool option synthesized at load time is keyed by names that may not exist at injection time. Spec tools carry capability markers (execute_code, memory), skills never reach the tools array at all, and lazy MCP servers expand after synthesis - every mismatch was a silent no-op, and each fix added another hand-maintained marker map that the next case leaked past. This removes the name-space gap instead of bridging it per case: - Synthesis records the selection as a policy: a wildcard '*' entry carries the default (true for "every tool", false for "only the named ones") and listed names are recorded verbatim. No load-time enumeration, eligibility checks, or placeholder special-casing. - Resolution happens at injection time, per final definition: explicit name -> capability marker projection -> wildcard. The projection maps a marker onto the names its registration actually produced this run - the registrars report their own tool names and initializeAgent accumulates them - so the mapping cannot drift from what gets registered. - The unmatched-name diagnosis moves to the apply passes, the one place the real definitions are known, so a selection naming a marker whose runtime expansion is entirely ineligible (runInBackground: ['memory']) is now warned about instead of recorded as a dead success. Covers all four round-3 findings: code opt-outs now reach create_file/edit_file/read_file, skill definitions are governed by narrowing selections, the memory marker is rejected and diagnosed for backgrounding, and the mcp_all placeholder predicate is no longer consulted for selections at all (its one remaining consumer is the definitions loader that defines the convention). * 🧯 fix: Reject Dead Ask-Tool Selections and the Reserved Wildcard Two review findings on the selection policy, both fixed at the point where they are knowably invalid: - ask_user_question joins EXCLUDED_INTENT_TOOL_NAMES: createRun strips its provisional definition and rebuilds the graph tool from its own Zod schema, so definition-level injection never reaches the model. A describeIntent selection naming it now warns as ineligible instead of crediting a label that gets discarded. Real intent support for the ask tool lands with the HITL slice via the interrupt payload. - A literal '*' in a describeIntent/runInBackground list is dropped with a warning at synthesis: it would overwrite the wildcard opt-out default and silently enable the capability for every eligible tool instead of selecting one named tool. The wildcard is reserved for the internal policy; boolean true is the supported way to cover everything. |
||
|---|---|---|
| .. | ||
| api | ||
| client | ||
| data-provider | ||
| data-schemas | ||