Commit graph

68 commits

Author SHA1 Message Date
Danny Avila
6f45a9e32e
🔗 fix: Normalize MCP Tool Keys at Every Producer, Resolve Raw Names via Aliases (#14553)
* 🔗 fix: Normalize MCP Tool Keys at Every Producer, Resolve Raw Names via Aliases

Tool keys had two spellings that could diverge for any server whose name
contains characters outside [a-zA-Z0-9_.-]: the tool cache (and registry
inspector) built keys with the RAW server name, while runtime instances
are named with normalizeServerName(serverName). Three code comments
already asserted "tool keys embed the normalized server name" - no
producer honored it. For a special-character server that meant:

- definitions-only mode shipped raw def names the model echoed back,
  but the executor's tool map held the normalized instance name, so
  every call failed with "Tool not found";
- per-tool tool_options (defer_loading / allowed_callers /
  run_in_background / describe_intent) were persisted under raw keys
  that never matched the definition names the option passes resolve
  against, so builder settings were silently inert;
- tool-key parsing against normalized candidate lists silently fell
  back to last-delimiter splitting, which mis-parses delimiter-bearing
  tool names.

The reconciliation is one contract enforced in three moves:

1. PRODUCERS NORMALIZE. The tool cache (packages/api/src/mcp/tools.ts)
   and the registry inspector build keys with the normalized server
   name, matching the instance names MCP.js has always assigned. The
   builder's tool ids, agent.tools entries, tool_options keys, and
   definition names all flow from these keys, so every model-facing
   name now agrees. The cache STORE stays keyed by the raw config name.

2. CONFIG LOOKUPS RESOLVE ALIASES. New shared helpers in data-provider
   (buildServerNameAliases, normalizeMCPToolKey) map a parsed
   normalized name back to the raw config name that the registry,
   config maps, tool cache, and plugin-auth rows are keyed by. Applied
   in the definitions loader closure, handleTools grouping,
   createMCPTool's parsing fallback, getUserMCPAuthMap, and the MCP
   tools endpoint - matching both spellings so legacy raw keys keep
   resolving.

3. LEGACY DATA HEALS AT ONE BOUNDARY. initializeAgent rewrites
   raw-keyed agent.tools entries and tool_options keys to the
   normalized form (normalizeAgentToolKeys) before anything consumes
   them, so agents persisted under the old convention load their tools
   AND have all four per-tool options honored. Placeholder and
   server-pin tokens stay raw - they are config-identity references,
   not model-facing names.

Servers whose names are already in the safe character set (the common
case) produce byte-identical keys before and after; the fast path
allocates nothing. Stale Redis-cached raw keys self-heal via the
existing reconnect-on-missing path within one cache cycle.

* 🧯 fix: Deterministic Alias Collisions + Raw Names in Definition Metadata

Two review findings on the normalization contract:

- Two configured server names that normalize to the same segment (e.g.
  'Sales Force' and 'Sales:Force' -> 'Sales_Force') produce inherently
  ambiguous tool keys; the alias map silently resolved last-wins, so a
  tool selected from one server could execute against the other's
  config. buildServerNameAliases now resolves collisions to the FIRST
  configured name deterministically, and resolveMCPServerContext warns
  once per colliding pair per process so the operator can rename one
  server. A collision-resistant identifier would change every existing
  tool key, so detection + stable routing is the right treatment here;
  startup-time config validation can follow separately.

- The definitions loader resolved parsed (normalized) server names to
  raw only inside the ToolService closure, while the definition
  metadata (serverName -> mcpRawServerName) kept the normalized value.
  Server instructions are keyed by raw config names, so a
  special-character server's instructions were silently omitted in
  definitions-only mode. loadToolDefinitions now takes rawServerNames,
  resolves the boundary against both spellings, and stores the RAW
  name in definition metadata - consistent with the instance path.

* 🧯 fix: Heal Stale Caches, Skill Allowed-Tools, and Builder Selectors

Three review findings on the normalization rollout, all in the
transition class:

- Stale cache entries (P1): the definitions-only loader treats the
  per-server tool map as authoritative and never reconnects on a
  per-key miss, so a pre-change raw-keyed Redis entry would make a
  special-character server's tools vanish for up to the cache TTL.
  getMCPServerTools now heals legacy raw-keyed entries to the
  normalized format at read time (keys and function names), covering
  every consumer with no coordinated invalidation; safe names return
  the map untouched.

- Skill allowed-tools: a skill declaring a raw MCP key in
  allowed-tools bypassed the initialize-boundary heal (the union runs
  after it) and would neither dedupe against healed agent tools nor
  match the normalized tool map. The primes' allowedTools now pass
  through the same normalizeAgentToolKeys heal before unioning.

- Builder selectors: matchesMcpServer and useVisibleTools parsed tool
  ids against raw server names only, so an attached special-character
  server rendered as an unselected orphan card. Both now accept the
  normalized spelling and resolve it back to the raw map key, keeping
  legacy raw ids working.

* 🧯 fix: Fail Closed on Normalized Server-Name Collisions

Escalation of the collision finding: a deterministic first-wins alias
plus a warning still let the tools listing publish BOTH colliding
servers, so a tool selected under the shadowed second server would
silently execute against the first server's configuration (their
model-facing keys are identical, so routing cannot ever distinguish
them).

- findShadowedServerNames identifies later-configured names whose
  normalized form an earlier different name claimed.
- getMCPTools excludes shadowed servers from the published listing
  entirely (with a warn naming the collision), so their tools are
  never selectable - nothing ambiguous can be picked.
- Server creation reserves both spellings: a generated slug may not
  collide with a raw config name OR the normalized form its tool keys
  would carry.

Collision-resistant model-facing IDs remain out of scope: changing
normalizeServerName's output would rewrite every existing tool key
(agent documents, caches, instance names) for ALL servers to handle a
misconfiguration that is now blocked from exposure instead.

*  fix: Dedupe Reserved Server-Name Spellings at Creation

The reservation list appended normalized forms unconditionally, which
duplicated every safe name (raw === normalized) and broke the
route-level contract test pinning the exact list. Dedupe via a Set so
safe names contribute one entry, while special-character names still
reserve both spellings; adds the special-character reservation case.

* 🧯 fix: Never Heal a Shadowed Server's Keys; Align Authorization Tie-Break

Persisted references were the remaining collision vector: an agent or
skill saved with the shadowed later server's raw key was HEALED into
the shared normalized key, authorized through a last-wins map, and
routed first-wins - authorized as one server, executed as another.

- normalizeAgentToolKeys now refuses to rewrite keys of shadowed
  servers (findShadowedServerNames): rewriting would produce exactly
  the first server's key. Left raw, the key cannot match the
  normalized-keyed tool map and the tool fails visibly - broken beats
  misrouted. Covers agent.tools, tool_options, and skill
  allowed-tools through the shared heal.

- filterAuthorizedTools (agents/v1.js) builds its normalized-to-raw
  map via the shared buildServerNameAliases instead of a last-wins
  Map constructor, so authorization resolves a colliding key to the
  SAME first server execution routes to.

* 🧯 fix: Direct Identity Wins Over Aliases; Heal Client Forms and Degraded Contexts

Four review findings on the normalization edges:

- Alias hijack (P1): a user-DB server named exactly like an operator
  server's normalized form ('foo' vs YAML 'foo!') had its tools
  rerouted to the operator server by unconditional alias resolution.
  Resolution is now DIRECT-FIRST everywhere: the parsed name is tried
  as-is, and only when nothing resolves is it treated as a normalized
  spelling (definitions loader, handleTools grouping, createMCPTool
  fallback). buildServerNameAliases seats identity entries before
  derived ones so a literal name owns its slot regardless of config
  order, findShadowedServerNames and the collision warning derive from
  the same construction, and getUserMCPAuthMap fetches auth under both
  spellings so either owner finds its rows.

- Builder double-match: a normalized name containing the delimiter
  ('foo mcp bar' -> 'foo_mcp_bar') also suffix-matched a server named
  'bar', selecting both cards and making removal strip the wrong tool.
  matchesMcpServer now resolves the token ONCE against the full
  configured list (longest boundary, both spellings) when the caller
  supplies it; selection and removal share the resolution.

- Builder legacy ids: an agent saved with raw-keyed ids showed its
  tools unchecked while the runtime heal kept them active, and
  selection updates never replaced the legacy entries. McpSection maps
  legacy raw ids to their current normalized ids when deriving and
  rewriting this server's selection.

- Degraded context: a transient ensureConfigServers failure returned
  an entirely empty context, leaving normalized keys unresolvable for
  the request. resolveMCPServerContext now keeps the name lists (they
  derive from the config snapshot alone) and degrades only the
  lazy-init configs.

* 🧯 fix: Collision Detection Sees Accessible Servers; Shadowed Refs Fail Closed End to End

Round follow-ups on the collision design, all in the
DB-server-visibility class:

- The legacy-key heal detected collisions against operator-config
  names only, so healing could still produce a key that direct-first
  resolution routes to an invisible user-DB server. initializeAgent
  gains an optional getAccessibleMcpServerNames dep (wired through
  ToolService for controllers that mock it, directly elsewhere),
  consulted ONLY when a configured name needs normalization - zero
  cost for safe-name deployments. The heal then sees the full
  accessible set and skips shadowed servers' keys.

- Wildcard and legacy raw tokens bypassed catalog filtering, letting a
  shadowed server's instances join a run under the same normalized
  names as the winner's. filterAuthorizedTools rejects tools of
  shadowed servers at authorization (its merged map sees DB + config),
  and handleTools skips them at execution.

- The builder migrated only tool selection, not tool_options: legacy
  raw option keys showed disabled while the runtime honored them, and
  toggles could not clear them. McpSection now migrates option keys to
  the current normalized ids (existing normalized entries win).

- A transient ensureConfigServers failure degraded to an EMPTY server
  context, leaving normalized keys unresolvable for the request.
  resolveMCPServerContext keeps the name lists (derived from the
  config snapshot alone) and degrades only the lazy-init configs.

* 🧯 fix: Complete the Collision Audit at Every Gate; Safer Heal Semantics

Round follow-ups hardening the collision audit:

- Execution guards now consult the FULL accessible set: the caller's
  heal threads its already-fetched names through loadTools, and
  handleTools fetches them itself when a configured name needs
  normalization (never for safe-name deployments) - so a cross-tier
  collision (user-DB 'foo' vs operator 'foo!') fails closed at eager
  execution instead of joining the run under one normalized name.

- Healing is SKIPPED when the collision audit cannot complete
  (transient lookup failure, or no dep): un-healed raw keys still
  resolve through the direct-first candidates, so skipping is safe
  while rewriting against an incomplete audit is not.

- The audit lookup is gated on the agent actually carrying
  delimiter-bearing keys (tools, tool_options, or skill
  allowed-tools), so non-MCP agents never pay a registry round-trip
  even on specially named deployments.

- normalizeAgentToolKeys gives the CURRENT (normalized) entry
  precedence when both spellings carry options, matching the builder's
  migration semantics instead of letting insertion order decide.

- The builder's toCurrentToolId resolves entries boundary-exactly
  against every configured server (longest match, both spellings), so
  a raw suffix shared with a LONGER server name can no longer reassign
  that server's selection or options while another dialog is open.

* 🧯 fix: Shared Collision Audit for Definitions Loading; Fail Closed on Audit Failure

Round follow-ups closing the remaining audit gaps:

- The definitions-only loader now consumes the same collision audit as
  eager loading: shadowed servers' entries (wildcards included) are
  dropped before definitions are emitted, so the default execution
  path can never resolve a shadowed server's normalized function name
  to another server. The audit names thread from initializeAgent's
  heal; the loader self-fetches only when a configured name needs
  normalization.

- resolveCollisionAuditNames centralizes the audit-resolution policy
  (threaded set > self-fetch when needed > incomplete on failure), and
  BOTH loaders now fail closed under an incomplete audit: any
  normalization-sensitive reference (its own name needs normalizing,
  or it equals the normalized form of a configured special-character
  name) is skipped with a warning instead of being audited against
  operator names alone. isNormalizationSensitiveName lives in
  packages/api as a pure helper so test mocks use the real predicate.

- normalizeAgentToolKeys collapses duplicate ids after healing
  (order-preserving): a document carrying both spellings converges on
  one key, never two instances with the same function name.

* 🧯 fix: Thread the Audit Everywhere; Identity-Aware Alias Fallback

Round follow-ups on audit plumbing:

- The OpenAI-compatible and Responses tool loaders now forward the
  already-resolved accessibleMcpServerNames instead of discarding it,
  so the definitions loader neither repeats the registry lookup nor
  fails closed on a transient second lookup after the first succeeded.

- The skill-only path threads its audit: when the baseline agent has
  no MCP keys but a primed skill's allowed-tools fetched the complete
  set, that set (not the operator-only list) reaches the loader, so
  the collision remains visible and the shadowed reference stays
  rejected end to end.

- OAuth discovery iterates the collision-FILTERED tool list, so a
  request can no longer emit an OAuth prompt, wait out the connection
  timeout, and reconnect a server whose definitions were deliberately
  rejected.

- The definitions loader's alias fallback is identity-aware: when the
  parsed name IS a known accessible server, a null tool fetch means
  temporarily unavailable (OAuth pending, missing user variables,
  disconnected) and no longer reroutes to the raw alias - previously
  the aliased operator server's definitions could be emitted under the
  unavailable DB server's names.

* 🧯 fix: Legacy-Key Definition Lookup; Retain Audit for Deferred Execution

- createMCPTool resolves tool definitions by BOTH spellings: the key as
  persisted plus the canonical normalized key built from the resolved
  server name. Assistants and direct tool calls persisted before the
  rollout bypass the agent-boundary heal and arrive with raw keys, while
  availableTools is now indexed canonically - previously every such call
  missed the index, burned a reconnect, and returned the unavailable
  stub permanently via the negative cache.

- The initialized agent retains accessibleMcpServerNames (the COMPLETE
  collision audit this initialization resolved), buildAgentToolContext
  copies it into every per-agent tool context, and loadToolsForExecution
  threads it into the eager loader as bare options. Deferred/event-driven
  execution therefore reuses the snapshot instead of repeating the merged
  registry read - a transient failure there could fail-closed a tool the
  same turn already advertised from the successful first audit.

- MCP.spec.js keeps @librechat/api pure helpers REAL (requireActual
  spread) so normalization paths are exercised rather than mirrored.

* 🧯 fix: Parse Legacy Keys Against Both Server-Name Spellings

createMCPTool's boundary candidates were normalized-only, so a legacy
raw key whose server name contains the delimiter (foo_mcp_bar!) missed
the suffix match and fell to the generic last-delimiter split - the
canonical rebuild then produced a key that could never hit the index
and the persisted call stubbed out. The candidate list now carries the
RAW resolved name (and raw config names on the parse-only path) next
to the normalized spellings.

* 🧯 fix: Honest Audit Completeness; Shadowed-Server Form-Key Guard

- resolveAllMcpConfigs tolerates ensureConfigServers failures, so the
  merged registry read can silently omit config-only servers while the
  audit still reported complete: true - a foo/foo! collision would go
  unseen and a persisted key could route to the wrong server. Both
  audit consumers now union the snapshot-derived raw config names back
  in (resolveCollisionAuditNames unions the caller's rawServerNames;
  the initializeAgent heal unions configRawServerNames), keeping the
  completeness label honest without an extra read: operator names come
  from the registry-independent config snapshot, user-DB names from the
  merged read that fails loudly into the existing incomplete path.

- The client tool_options migration now mirrors the runtime heal's
  fail-closed rule for SHADOWED servers: when the dialog's server has
  lost its normalized slot to another catalog name, legacy raw keys
  stay raw instead of being rewritten onto the winning server's key,
  where a later save would apply the wrong server's per-tool settings.
  The dialog's own server joins the alias construction so a stale
  catalog map can't misread as a collision.

* 🧯 fix: Heal Legacy Assistant MCP Tool Names on Save

The assistants create/update controllers look tools up in the cached
definitions by exact key, and the cache is now normalized-keyed - an
assistant saved before the convention resubmits its raw-suffixed MCP
name on every edit, so any save silently removed the tool.

healMcpToolNames pre-heals the payload's tool list: a delimiter-bearing
string that misses the cache resolves through the configured raw names
(longest-suffix, boundary-exact) and rewrites to the normalized key
only when that key actually exists in the cache. SHADOWED raw names
stay raw and fail closed, mirroring the runtime heal; the config read
happens only when a delimiter-bearing name actually misses, and read
failures propagate (write path) rather than silently dropping tools.
v2's update loop also stops re-reading the tool cache per iteration.

* 🧯 fix: Full-Audit Shadow Set + Dedupe in the Assistant Key Heal

- The assistant-save heal built its shadow set from operator config
  names alone, so a cross-tier collision (user-DB `foo` owning the
  normalized slot of operator `foo!`) looked unshadowed and the legacy
  key healed into the shared normalized name - which direct-first
  execution then binds to the DB server. The shadow set now comes from
  resolveCollisionAuditNames' full accessible audit, and an incomplete
  audit skips healing outright (every rewrite candidate is
  normalization-sensitive by construction, so raw-and-fail-closed is
  the only safe answer).

- Healed string entries dedupe order-preserving: a payload carrying
  both spellings of the same tool collapses to one entry instead of
  expanding into duplicate function definitions the provider rejects.
2026-08-01 07:39:24 -04:00
Danny Avila
250aca375a
🔗 fix: Resolve MCP Tool-Key Boundary Against Configured Server Names (#14448)
* fix: resolve MCP tool-name delimiter collision at invocation time

MCP tool keys are identified internally as `${rawToolName}${mcp_delimiter}${serverName}`
(delimiter `_mcp_`). Several call sites parsed this back apart with a naive
`toolKey.split(Constants.mcp_delimiter)`, assuming the delimiter occurs exactly once.

When the raw upstream tool name itself contains the delimiter substring - which
happens whenever it's exposed through a gateway that prefixes aggregated tool names by
server (e.g. a gateway's own "gitlab-get_mcp_server_version" for GitLab's
"get_mcp_server_version" tool) - the combined key has the delimiter more than once.
`.split()` then produces more than two segments, and destructuring
`[toolName, serverName]` silently keeps only the first two, yielding a bogus server
name that matches no configured server. Tool listing still worked (a different code
path builds keys directly without re-splitting), but invocation failed with
`Tool {name} not found`, and `filterAuthorizedTools` rejected such keys outright as
malformed.

Add `splitMCPToolKey`, which splits on the *last* occurrence of the delimiter instead:
the server-name half is always LibreChat's own normalized suffix (guaranteed not to
contain the delimiter), while the raw tool-name half is untrusted and may legitimately
contain it. This matches `.split()`'s result whenever the delimiter occurs once, and
correctly resolves the collision case. Update the four call sites that parsed this
manually (`handleTools.js`, `MCP.js`, `mcp.js` controller, `filterAuthorizedTools` in
`v1.js`) plus one in the client (`useVisibleTools.ts`) to use it.

Fixes #14440

* fix: resolve MCP tool-key boundary against configured server names

splitMCPToolKey moves to librechat-data-provider so the client and backend
share one parser, and takes the configured server names when the caller has
them: the longest name the key actually ends with wins, which is exact.

Position alone cannot identify the boundary because both halves may contain
the delimiter. lastIndexOf alone fixes gateway-prefixed tool names but
regresses servers whose own name contains it, which ToolService.spec.js
already covered; the last-delimiter path now only serves as the fallback for
callers with no configured set.

Also converts the remaining first-occurrence parsers that the delimiter fix
missed - mcp/auth.ts (custom user vars silently unresolved), mcp/oauth/events.ts,
agents/initialize.ts, and the three client parsers that labelled tool calls
with the wrong server.

* fix: keep client tool-call labels on first-delimiter parsing

The three client parsers had deliberate, tested first-delimiter semantics
(ToolCall.test.tsx asserts the full server name for 'foo_mcp_bar' and the
synthetic 'oauth_mcp_server' call), and the client has no configured server
list in scope to resolve the boundary exactly, so they are left as they were.

Threads the configured names into the event-driven definition loader so it
resolves the same boundary as the authorization filter that admits the key,
and documents the one case that stays undecidable without provenance.

* fix: resolve tool-key boundary against all configured servers

resolveConfigServers only returns lazily-initialized config overrides -
ensureConfigServers skips unmodified YAML servers - so on a stock deployment
the known-name list was empty and suffix resolution never engaged. Adds
resolveMcpServerNames, which keeps every configured server in the normalized
form tool keys carry, and uses it at the loading, auth-map and definition
sites.

Background-tool eligibility now resolves against all configured names before
testing ephemeral membership, so a non-ephemeral server whose name ends in an
ephemeral one is no longer misclassified, and useVisibleTools resolves against
the server map it already receives.

* fix: use resolved server provenance and one app-config read

createMCPTool now uses the serverName loadTools already resolved for the key
and only parses as a fallback, so an unmodified YAML server whose name
contains the delimiter no longer resolves to the wrong server for auth,
reconnection and callTool.

resolveMcpServerContext derives config servers and all configured names from
a single getAppConfigForRequest, replacing two independent lookups on the
chat startup path, and degrades to empty like resolveConfigServers instead of
aborting tool loading when the config lookup fails.

* chore: drop unused resolveConfigServers import

* fix: forward server provenance on the all-tools path and read config once

createMCPTools builds each toolKey from the server name it already has but did
not forward it, so the sys__all__sys path re-derived it by parsing and bound
an unmodified YAML server whose name contains the delimiter to the wrong auth
and invocation context.

loadAgentTools now resolves the MCP server context once and threads it into
loadTools, replacing the second app-config read it had introduced on the
non-event-driven chat startup path.

* fix: carry resolved MCP server name through tool classification

definitions.ts resolves the server for each key and then dropped it when
building loadedTools, so buildToolClassification re-derived it with a
last-segment split and recorded 'Workspace' for a server configured as
'Google_mcp_Workspace'. The resolved name now rides along on the tool
instance and classification prefers it over re-parsing.

* fix: consume carried server name when extracting MCP servers

extractMCPServers re-derived the name with a last-segment split, so a server
configured as Google_mcp_Workspace resolved to Workspace and its instructions
were silently omitted. Prefers the name carried on the tool definition
instance, falling back to the split.

* fix: fail closed on ambiguous MCP keys when persisting server names

Persisted mcpServerNames grant agent-scoped access to a DB server by name
(ServerConfigsDB.getAccessibleServers), so a wrong guess exposes an unrelated
server to everyone who can view the agent. The last-segment split turned
search_mcp_Google_mcp_workspace into 'workspace'; such keys were previously
rejected outright at agent save, so admitting them opened this path.

Derives a name only from unambiguous single-delimiter keys. This is #12250's
guard moved to the boundary it was actually protecting, instead of blocking
tool admission.

* fix: keep DB server access for multi-delimiter tool keys

The fail-closed guard was wrong for the case this PR exists to fix. This index
only grants DB-backed servers, and DB names are slugs that cannot contain the
delimiter (generateServerNameFromTitle strips underscores), so the trailing
segment is always the real server for them - dropping it cost every consumer
of a gateway-prefixed tool their shared-agent access.

Also gates the MCP server-context lookup on the filtered MCP set, so an agent
with no MCP tools no longer pays an app-config read on startup.

* fix: resolve tool-call display names without breaking OAuth calls

The display parsers could not use the shared boundary parser because their
tested behavior depends on first-delimiter semantics. That constraint only
applies to synthetic MCP OAuth calls, whose tool half is always exactly
'oauth', so everything after the first delimiter is the server even when the
server name carries one.

splitToolCallName special-cases that form and defers to splitMCPToolKey for
real tool keys, so a gateway-prefixed tool now renders its own name and
server while oauth_mcp_foo_mcp_bar still resolves to foo_mcp_bar.

* fix: persist resolved MCP server provenance on agents

Deriving mcpServerNames from the tool key cannot tell a config server's
trailing segment from a real DB server name, so a config server named
a_mcp_b indexed an unrelated DB server b and shared the agent's viewers into
it. Neither string rule works: the suffix guess exposes, and failing closed
drops legitimate DB access for gateway-prefixed tools.

filterAuthorizedTools already resolves each tool's server against the merged
registry config, so it now collects those names and create, update and
duplicate persist them. No extra registry queries: the update path unions the
newly resolved names with what the agent already had, and duplicate replaces
the copied list rather than inheriting the source's servers.

Display parsing also takes the configured names, so a real tool call on a
delimiter-bearing server renders the right server and icon.

* test: teach MCP hook mocks about useMCPServerNames

Three specs mock ~/hooks/MCP with a hand-listed factory, so adding the hook
to ToolCall made useMCPServerNames undefined under test and every render
threw. Returns a stable array so the mock cannot perturb render counts.

* fix: rebuild agent MCP server index from surviving tools

Unioning the prior names kept a server indexed after its last tool was
detached, so viewers of a shared agent retained agent-scoped access to it.
The index is now rebuilt from the tools that survive the edit: a prior name
carries forward only while some retained tool still resolves to it, using the
agent's own persisted names as the candidate set, and the rebuild runs on any
tool change rather than only when a new MCP tool is added.

* fix: keep duplicate indexes on registry fallback and harden the oauth split

Duplication blanked mcpServerNames when the registry was unavailable, because
filterAuthorizedTools grandfathers the source's tools without resolving them -
the copy kept tools it could no longer resolve. Source names now carry forward
for the tools that still point at them.

splitToolCallName also treated any oauth_mcp_ prefix as a synthetic OAuth
call, so a genuine upstream tool by that name resolved to the wrong server. A
configured server name now decides when one matches, since a real key always
ends in its server, and the prefix only breaks ties for unconfigured servers.

* fix: thread configured server names through display parsing

parseToolName and getMCPServerName resolved context-free, so a configured
server whose name contains the delimiter showed the wrong server in grouped
tool summaries and subagent tool labels, and stacked icons missed its entry in
the icon map. Both take the configured names now, supplied by the components
that render them.

Adds the hook to SubagentCall's mock factory: the spec renders the real
component, so an unmocked useMCPServerNames would reach the query with no
provider.

* test: cover the auth-map boundary, server provenance and context fallback

Adds regression coverage for three behaviors this PR changed that no test
exercised: customUserVars resolving under the right plugin key for a
gateway-prefixed tool name (the failure that made these tools loadable but
unusable), the resolved server name reaching createMCPTool instead of being
re-parsed, and resolveMcpServerContext degrading to empty rather than
aborting tool loading when the config lookup fails.

Each was checked against a mutated source to confirm it fails when the
behavior is broken.

* fix: normalize server-name candidates and cover the boundary guard

Tool keys embed normalizeServerName's output while the config is keyed by the
raw name, so callers passing raw keys never matched a server whose name needs
normalizing and silently fell back to the last delimiter. filterAuthorizedTools
now maps normalized names back to their config key, and createMCPTool
normalizes its candidates.

Adds the cases an audit found surviving mutation: a configured name that is a
bare but not delimiter-aligned suffix must not match, an empty candidate list
behaves as no list, and splitToolCallName still falls back to the oauth prefix
when a list is supplied but nothing in it matches.

* fix: keep resolved server names when a non-owner retains MCP tools

The shared-agent path keeps an agent's existing MCP tools verbatim but supplied
no mcpServerNames, so persistence re-derived them and reduced a configured
server like Google_mcp_Workspace to Workspace - which ServerConfigsDB then
treats as a DB server, granting the agent's viewers access to an unrelated one.
Carries the existing resolved names across instead, and clears the index on the
owner path where every MCP tool is removed.

* fix: preserve resolved MCP names for every tools update

extractMCPServerNames was reachable from any caller that writes tools without
mcpServerNames - the Action edit path does exactly that - so a configured
Google_mcp_Workspace was reindexed as Workspace and ServerConfigsDB granted
shared-agent viewers an unrelated DB server by that name.

updateAgent now rebuilds the index from the agent's own resolved names: one
carries forward while a retained tool still resolves to it, and only keys
matching none of them fall back to derivation. Callers are safe by default
rather than by remembering to pass the set.

normalizeServerName moves to librechat-data-provider so the client can match
its candidates against tool keys, which embed the normalized form; the icon map
is keyed the same way since it is looked up with a parsed server name.

* refactor: move MCP context resolution into packages/api

New backend logic belongs in the TypeScript workspace per CLAUDE.md, with /api
kept to a thin wrapper. resolveMCPServerContext now lives in
packages/api/src/mcp/context.ts and takes ensureConfigServers by injection,
since the registry accessor is still legacy-only; the /api function is reduced
to loading the request app config and translating failures into the empty
degrade it already promised.

* test: teach the MCP service mock about resolveMCPServerContext

The spec mocks @librechat/api with a hand-listed factory, so moving the
resolver into that package left it undefined and the wrapper degraded into its
own catch, returning empty config servers. The stub mirrors the real resolver
so these tests still cover what the wrapper owns - loading the request config
and degrading on failure - while the resolution logic is unit-tested in
packages/api.

* fix: only persist an authoritative MCP server index on update

Assigning the resolved set unconditionally pinned the index to [] whenever
nothing authoritative was available - a legacy agent holding MCP tools with no
stored mcpServerNames - which suppressed updateAgent's derivation and stripped
agent-scoped access to its DB-backed server.

The field is now supplied only when the result is authoritative: names were
resolved, or no MCP tool survives so the index genuinely is empty. The
retained-tools branch likewise leaves it unset when the agent has none stored.

---------

Co-authored-by: Jens Schumann <schumajs@gmail.com>
2026-07-27 14:45:38 -04:00
Danny Avila
a53936d273
🧭 test: Cover Agent Handoffs End to End (#14428)
Some checks failed
Docker Dev Branch Images Build / build (Dockerfile, lc-dev, node) (push) Waiting to run
Docker Dev Branch Images Build / build (Dockerfile.multi, lc-dev-api, api-build) (push) Waiting to run
Docker Dev Images Build / build (Dockerfile, librechat-dev, node) (push) Has been cancelled
Docker Dev Images Build / build (Dockerfile.multi, librechat-dev-api, api-build) (push) Has been cancelled
GitNexus Index / index (push) Has been cancelled
Sync Locize Translations & Create Translation PR / Sync Translation Keys with Locize (push) Has been cancelled
Sync Helm Chart Tags / Ignore non-main push (push) Has been cancelled
Sync Helm Chart Tags / Sync chart tags (push) Has been cancelled
GitNexus Index / post-index (push) Has been cancelled
Sync Locize Translations & Create Translation PR / Create Translation PR on Version Published (push) Has been cancelled
* test: cover agent handoffs end to end

* style: sort handoff imports

* fix: normalize missing agent handoff edges

* chore: update package dependencies and versions in package-lock.json and package.json

* chore: bump agents SDK
2026-07-27 08:47:15 -04:00
Danny Avila
3e9f07976a
🧩 fix: Preserve Deployment Skill IDs on Agents (#14368)
* fix: preserve deployment skills on agents

* fix: expose deployment skills to agent viewers

* refactor: centralize deployment skill ID merging

---------

Co-authored-by: Dennis Schenk <dennis@gridonic.ch>
2026-07-21 19:44:27 -04:00
Serhii Zghama
96d213afe6
🧊 fix: Include Conversation Starters in Agent View and List Responses (#14142)
* fix: include conversation_starters in agent view and list responses

* test: cover conversation_starters in agent view and list projections

* test: include conversation_starters in agent list whitelist

---------

Co-authored-by: Danny Avila <danny@librechat.ai>
2026-07-08 12:54:54 -04:00
Danny Avila
96367828e1
🧷 fix: Align Agent File Attachment Ownership (#14149)
Some checks failed
Docker Dev Branch Images Build / build (Dockerfile, lc-dev, node) (push) Waiting to run
Docker Dev Branch Images Build / build (Dockerfile.multi, lc-dev-api, api-build) (push) Waiting to run
Docker Dev Images Build / build (Dockerfile, librechat-dev, node) (push) Has been cancelled
Docker Dev Images Build / build (Dockerfile.multi, librechat-dev-api, api-build) (push) Has been cancelled
GitNexus Index / index (push) Has been cancelled
Sync Locize Translations & Create Translation PR / Sync Translation Keys with Locize (push) Has been cancelled
Sync Helm Chart Tags / Ignore non-main push (push) Has been cancelled
Sync Helm Chart Tags / Sync chart tags (push) Has been cancelled
GitNexus Index / post-index (push) Has been cancelled
Sync Locize Translations & Create Translation PR / Create Translation PR on Version Published (push) Has been cancelled
* fix: Align agent file attachment ownership

* fix: Harden agent file unlink validation

* test: Align file preview agent attachment access

* test: Add agent file ownership e2e regression
2026-07-07 16:23:48 -04:00
Danny Avila
2d4ef52c22
🧮 fix: Prevent String Corruption of Numeric Agent Model Parameters (#14119)
* 🧮 fix: Prevent String Corruption of Numeric Agent Model Parameters

* 🧮 fix: Support Partial Numeric Input and Cover Parameter Aliases
2026-07-05 12:03:41 -04:00
Danny Avila
12fea693bb
🦥 perf: Lazy-Load Agent Version History in Editor (#13977)
Opening the agent editor fetched the full `versions` array (each a complete
config snapshot) alongside the agent, so agents with large histories were slow
to open. Version history is now loaded only when the user opens it.

- Add `getAgentWithVersionCount` (aggregation: version count, no versions array)
  and `getAgentVersions` data-schemas methods.
- `getAgentHandler` returns the version count without the heavy array; add
  `GET /agents/:id/versions` (EDIT-gated) for lazy retrieval.
- Add `useGetAgentVersionsQuery`; VersionPanel reads current config from the
  cached expanded query and fetches versions on open. Revert keeps the expanded
  cache and versions query in sync.
2026-06-26 12:19:54 -04:00
Peter
abf9fc307d
📇 feat: Agent Contact Visibility with Owner Fallback (#13663)
* Shared Contract

* Backend Resolution

* Frontend Display

* Contact Styling and more tests

* fix contact flicker when saving an agent

* fix display owner when contact deleted

* simplification of the last fixes

* github action fixes

* fixes failing tests

---------

Co-authored-by: Peter Rothlaender <peter.rothlaender@ginkgo.com>
2026-06-25 15:58:15 -04:00
Danny Avila
397ddc5366
🧠 feat: Add Memory as an Agent Capability with Inline Tools and Ephemeral Badge (#13869)
* 🧠 feat: Memory Agent Capability with Inline Tools and Ephemeral Badge

Add `AgentCapabilities.memory`, which expands into the inline set_memory/delete_memory tool pair (mirroring the execute_code expansion via registerMemoryTools) when a run-level memoryAvailable gate holds: capability enabled, memory configured, MEMORIES.USE permission, and personalization not opted out. Surfaces the memory artifact as an attachment in the agents tool-end callback.

Adds the ephemeral path (TEphemeralAgent.memory, load/added agent tool injection), a fully-gated memory badge plus tools-dropdown entry, the agent-builder Memory toggle with form round-trip, and a mock e2e test asserting the badge reaches the request payload. Additive to and independent of the existing post-turn memory extraction agent.

* 🩹 fix: Address Codex review on memory capability (gating, validKeys, usage guard)

- Strip the memory capability from the served agents capabilities when memory is not configured/enabled, so the badge, tools dropdown, agent-builder toggle, and backend capability gate stay consistent instead of exposing an inert toggle on default installs (where MEMORIES.USE defaults true).
- Surface configured memory.validKeys in the inline tool definitions so the model is told the allowed keys up front, matching the runtime createMemoryTool schema.
- Append a strict explicit-request usage guard to the agent instructions when inline memory tools are registered, preserving the memory-agent's privacy behavior.
- Add AppService tests covering memory-capability stripping.

*  test: Update AppService capability snapshots for memory strip

AppService now strips the memory capability from the served agents defaults when no memory block is configured; update the spec's expected capability lists to defaultAgentCapabilitiesWithoutMemory for the no-memory-config cases.

* 🛡️ fix: Address Codex re-review on memory capability (round 2)

- Strip the memory capability from the FINAL served agents config, not just defaults; loadEndpoints reparses any endpoints.agents block, so memory was still exposed in that common shape (packages/data-schemas/src/app/service.ts) + regression test.
- Re-check the full memory gate (config, opt-out, MEMORIES.USE) inside handleTools before constructing set_memory/delete_memory, so an unsolicited tool call from a model/custom endpoint can't bypass the runtime gates (api/app/clients/tools/util/handleTools.js).
- Restore the persisted memory toggle for model-spec conversations via applyModelSpecEphemeralAgent (client/src/utils/endpoints.ts).
- Clear LAST_MEMORY_TOGGLE_ on logout and clear-all-chats so a stale memory preference can't leak across users on a shared browser (client/src/utils/localStorage.ts).

* 🧠 fix: Address Codex re-review on memory capability (round 3)

- Serialize set_memory writes and advance a running token total inside createMemoryTool, so parallel batched calls in one event-driven turn can't each pass the limit check against a stale total and collectively exceed memory.tokenLimit (packages/api/src/agents/memory.ts) + tests.
- Inject the keyed memory context (withKeys) instead of withoutKeys when the running agent has the inline memory capability, so delete_memory has a visible key to target (api/server/controllers/agents/client.js).

* 🔐 fix: Address Codex re-review on memory capability (round 4)

- Detect inline memory by tool NAME (set_memory/delete_memory) across an initialized agent's tools + toolDefinitions, since the 'memory' marker is expanded at init and the prior string check never matched; inject the keyed memory context for any primary OR sub-agent that carries the inline memory tools (api/server/controllers/agents/client.js).
- Enforce memory WRITE permissions in the inline tool gate: set_memory requires CREATE+UPDATE and delete_memory requires UPDATE (matching the REST memory routes), so a USE-only role can't mutate/delete memories via agent tool calls (api/app/clients/tools/util/handleTools.js).

* 🔒 fix: Address Codex re-review on memory capability (round 5)

- Gate inline memory registration (memoryAvailable) on the memory WRITE permissions (USE+CREATE+UPDATE), so a read-only-memory role no longer has set_memory/delete_memory shown to the model only for the runtime loader to refuse them (api/server/services/Endpoints/agents/initialize.js).
- Enforce the per-agent memory opt-in at execution: handleTools now refuses to construct set_memory/delete_memory unless the agent actually declared them (toolDefinitions/tools), blocking hallucinated/undeclared memory tool calls from mutating memory.
- Fail closed when getFormattedMemories errors with a configured tokenLimit, instead of writing as if storage were empty and bypassing the cap (api/app/clients/tools/util/handleTools.js).

* 🩹 fix: Address Codex re-review on memory capability (round 6)

- Fix a P1 regression from the prior round: the execution-context agent keeps the raw 'memory' capability marker (not the expanded set_memory/delete_memory names), so the opt-in check now matches the marker. This restores memory writes/deletes AND avoids hijacking an MCP tool that merely shares the set_memory/delete_memory name (api/app/clients/tools/util/handleTools.js).
- Count repeated set_memory writes to the same key as replacements, not additions, against tokenLimit — set_memory upserts, so a same-key rewrite swaps its prior token contribution instead of double-counting (packages/api/src/agents/memory.ts) + test.
- Gate the memory badge, tools dropdown, and agent-builder toggle on the full memory write permissions (USE+CREATE+UPDATE) via a shared useHasMemoryAccess hook, so a read-only-memory role no longer sees an enabled Memory control the backend would refuse to wire up.

* 🧷 fix: Address Codex re-review on memory capability (round 7)

- Recognize inline memory across both execution-context agent shapes: initializeAgent now sets a LibreChat-only memoryToolsRegistered flag on the InitializedAgent, and the opt-in/detection checks accept that flag OR the raw 'memory' marker. Fixes memory failing for processAddedConvo agents (which store the initialized config, marker already expanded) while staying MCP-name-collision-safe (api/app/clients/tools/util/handleTools.js, packages/api/src/agents/initialize.ts, api/server/controllers/agents/client.js).
- Scope keyed memory context to memory-enabled agents only: useMemory now returns both keyed and unkeyed contexts, and buildMessages injects the keyed one (memory keys + token metadata) only to agents that can call delete_memory, while the primary/post-turn path keeps the unkeyed values — so a primary without memory tools no longer sees memory keys it doesn't need.

* 🔏 fix: Address Codex re-review on memory capability (round 8)

- Enforce memory size limits on inline writes: createMemoryTool now rejects keys over 1000 chars and values over memory.charLimit, matching the REST memory routes, so an inline-memory agent can't persist blobs the memory UI/API would reject (packages/api/src/agents/memory.ts, api/app/clients/tools/util/handleTools.js) + test.
- Recheck the agents 'memory' endpoint capability at execution time, so a stale/hallucinated set_memory/delete_memory call can't mutate memory after an admin removes the capability while the agent document still carries the marker (api/app/clients/tools/util/handleTools.js).

* ♻️ refactor: Move inline-memory backend logic into packages/api + share memory load

Workspace boundary: the inline-memory gating/detection logic that had crept into /api now lives in packages/api/src/agents/memory.ts (TS), with /api kept as thin wrappers.

- Add agentHasInlineMemoryTools, isMemoryToolAllowed, and buildInlineMemoryTool to packages/api; handleTools.js now calls buildInlineMemoryTool instead of constructing/gating the tools inline, and client.js imports agentHasInlineMemoryTools instead of redefining it.
- Optimize repeated memory loads: getRequestMemories memoizes getFormattedMemories per request (WeakMap keyed by req), so the run's memory-context load and every memory-enabled agent's set_memory token-usage load share a single DB fetch instead of one per agent.

* 🧠 fix: Invalidate request memory cache after inline writes

Inline set_memory/delete_memory now invalidate the request-scoped
getFormattedMemories cache on a successful write, so a later tool round
in the same response is seeded with the post-write usage total instead
of the stale pre-write one (multi-round writes no longer collectively
exceed tokenLimit, and a set after a delete is not over-counted). The
within-round sharing across multiple memory-enabled agents is preserved.

* 🧠 fix: Persist memory capability on saved agents; honor registration flag

- Add Tools.memory to the v1 systemTools allowlist so filterAuthorizedTools
  no longer silently drops the memory marker when an agent with the Memory
  capability is created/updated/duplicated through the builder (previously
  the capability only worked for ephemeral chats, not persisted agents).
- agentHasInlineMemoryTools now honors an explicit memoryToolsRegistered
  boolean before falling back to the raw `memory` marker, so an initialized
  config whose registration was denied (memoryAvailable false) is not given
  keyed memory context just because the marker survives in tools.

* 🧩 fix: Bring memory tool to parity with other ephemeral tools

- Add `memory` to the model-spec schema/type and honor `modelSpec.memory`
  in both ephemeral paths (load.ts, added.ts) and the frontend spec
  application, so admins can pre-enable Memory from a model spec exactly
  like webSearch/fileSearch/executeCode.
- Add LAST_MEMORY_TOGGLE_ to the timestamped-storage cleanup list so stale
  per-conversation memory toggles are purged on startup like the others.
- Hide the agent-builder Memory toggle for users who disabled memory in
  personalization (memories === false), mirroring the chat badge's opt-out
  gate, so the setting isn't shown as inert/misleading.

*  test: Cover memory in applyModelSpecEphemeralAgent spec defaults

Update the exact-object assertions to include the new `memory` field and
add positive coverage that `modelSpec.memory` maps to the ephemeral
agent's `memory` flag. Fixes the shard 2/4 failure from 672a03b05.
2026-06-24 17:14:13 -04:00
Danny Avila
1eb460eb03
🧾 fix: Harden Historical File Authorization (#13918)
* fix: Harden historical file authorization

* chore: Sort file authorization imports

* fix: Preserve authorized historical artifact refs

* chore: Format historical artifact hardening
2026-06-23 15:49:57 -04:00
Danny Avila
5118a566df
🧭 fix: Restore Empty Skill Allowlist Catalog (#13526) 2026-06-05 12:30:48 -04:00
Danny Avila
479e9d59b7
🧠 refactor: Memoize MCP Permission Checks Per Request (#13419) 2026-05-30 18:32:06 -04:00
Danny Avila
100871c3ec
🛂 fix: Enforce MCP Permissions for Agent Tools (#13174)
Some checks are pending
Docker Dev Branch Images Build / build (Dockerfile, lc-dev, node) (push) Waiting to run
Docker Dev Branch Images Build / build (Dockerfile.multi, lc-dev-api, api-build) (push) Waiting to run
GitNexus Index / index (push) Waiting to run
GitNexus Index / post-index (push) Blocked by required conditions
* fix: Enforce MCP Permissions for Agent Tools

* fix: Measure MCP Image Limit by Decoded Size

* fix: gate cached MCP tools and tighten remote image URL detection

Addresses Codex review findings on the MCP permissions PR:

- filterAuthorizedTools previously fast-accepted any tool present in the
  global tool cache before reaching the MCP-use permission gate. App-level
  MCP tools (keyed `name_mcp_server` by MCPServerInspector and merged into
  the cache via mergeAppTools) therefore bypassed the canUseMCP check,
  letting a user without MCP_SERVERS.USE persist/bind them. Route all
  MCP-delimited tools through the permission + server-access gate
  regardless of cache presence.

- assertImageDataWithinLimit / image formatter used startsWith("http")
  to skip the size cap, which also matched base64 payloads that happen to
  begin with those chars. Require http:// or https:// via a shared
  isRemoteImageUrl helper so oversized inline base64 can no longer bypass
  MCP_IMAGE_DATA_MAX_BYTES.

Adds regression tests for both paths.

* fix: address Codex round-2 findings on MCP permissions PR

- parsers.ts: parseAsString dropped the image payload for unrecognized
  providers, returning only `Image result: <mimeType>`. Pre-PR these
  items survived via JSON.stringify(item). Keep the size guard but fall
  through to JSON.stringify so the data/URL is preserved.

- MCP.js: the runtime MCP-use check only read `configurable.user`, so
  paths that propagate `user_id` only (e.g. the OpenAI-compatible API in
  agents/openai/service.ts) rejected every MCP tool call for an
  authenticated user. Add resolveMCPPermissionUser: use the safe user
  directly when it already carries a role (no extra DB call), otherwise
  fall back to loading the role by user_id. Update fail-closed tests to
  the resolved behavior.

- v1.js: the update path only re-filtered newly added MCP tools, so a
  user who lost MCP_SERVERS.USE kept existing MCP bindings on edit while
  create/duplicate/revert stripped them. Strip all MCP tools on update
  when the permission is revoked; keep the narrower new-tool gating (and
  disconnect/registry preservation) when it is intact.

Updates and adds regression tests for all three paths.

* fix: populate safe user at producer instead of resolving in runtime MCP check

Corrects the Finding B approach from the previous commit. Rather than
loading the user by id inside the runtime MCP permission check, populate
`configurable.user` (and createRun's `user`) with the full safe user at
the producer, matching the in-repo agent controllers
(responses.js / openai.js) which already pass `createSafeUser(req.user)`.

- service.ts: derive `safeUser` via createSafeUser(req.user) and pass it
  to both createRun and processStream's configurable, so the role-bearing
  identity reaches the runtime `userCanUseMCPServers(configurable.user)`
  check. Falls back to a bare id when the host app attached no user,
  which correctly leaves MCP gated (fail closed).
- MCP.js: revert the resolveMCPPermissionUser DB-load fallback; the
  runtime check again reads configurable.user directly and fails closed
  when absent (defense in depth).
- MCP.spec.js: revert to the matching runtime test expectations.

* test: cover safe-user propagation in createAgentChatCompletion

Adds a focused spec for the OpenAI-compatible chat completion service
(the producer fixed for Codex Finding B). Injects mocked deps and asserts
that createRun and processStream's configurable.user carry the role from
req.user (with sensitive fields stripped by createSafeUser), and that an
unauthenticated request falls back to a bare { id: 'api-user' } so the
runtime MCP check fails closed.

* fix: address Codex round-3 findings + TS6133

- MCP.js (P1): the assistants required-action path invokes tool._call(
  toolInput) with no LangChain config, so the runtime check saw no
  configurable.user and rejected authorized users. createToolInstance now
  captures the creation-time user (req.user via createMCPTool) and _call
  falls back to it for both the permission check and userId. Still fails
  closed when neither config nor captured user carries a role.

- v1.js (P2): the update-path isMCPTool used a bare mcp_delimiter substring
  check, misclassifying action tools whose operationId contains "_mcp_"
  (e.g. sync_mcp_state_action_...) as MCP and dropping them on a
  permission-revoked edit. Delegate to the canonical isActionTool so only
  real MCP tools are gated. Regression test added.

- service.ts: drop the now-unused IUser import (TS6133); derive reqUser's
  type from createSafeUser's own parameter instead.

* fix: resolve TS7022 self-reference in service.spec mock res

The mock response object referenced `res` inside its own `status`/`json`
initializers without a type annotation, so tsc inferred `res` as `any`
(TS7022). Annotate the object and assign the self-referencing chainable
methods after declaration.

* fix: correct round-4 findings (isActionTool import, captured user, partial-update)

- v1.js: import isActionTool from librechat-data-provider (its real export;
  @librechat/api does not export it, so the prior import was undefined and
  threw TypeError). Exclude action tools from MCP classification in both the
  main filterAuthorizedTools loop and the update path, so action tools whose
  operationId contains _mcp_ (e.g. sync_mcp_state_action_...) are preserved
  regardless of MCP permission.
- v1.js: evaluate the effective tool set (updateData.tools ?? existingAgent.tools)
  so a tools-less PATCH by a user who lost MCP_SERVERS.USE still strips stale
  MCP bindings, matching create/duplicate/revert.
- MCP.js: createToolInstance now receives the construction-time user and _call
  falls back to it (permissionUser) when configurable.user is absent, fixing the
  assistants required-action path that invokes _call without a config and
  resolving the capturedUser no-undef/ReferenceError.
- Tests: action-tool preservation (authorized + denied), tools-less revocation
  PATCH, updated revocation test to expect all MCP tools stripped.

Affected specs pass locally: MCP 49/49, filterAuthorizedTools 49/49.

* fix: guard isActionTool against non-string tools; correct actionDelimiter import

Two test regressions from the prior commit:
- The main filterAuthorizedTools loop called isActionTool(tool) directly,
  but isActionTool does toolName.indexOf(...) and throws on null/undefined.
  Compute isActionToolName = typeof tool === 'string' && isActionTool(tool)
  once and reuse it, restoring graceful null/undefined handling.
- The action-tool test referenced Constants.actionDelimiter (undefined);
  actionDelimiter is a standalone librechat-data-provider export. Import and
  use it directly.

filterAuthorizedTools 36/36 and MCP 40/40 pass locally.

* fix: address MCP permission review follow-ups

* fix: preserve shared agent MCP tools
2026-05-30 16:19:49 -04:00
Danny Avila
94c73123ee
📋 fix: Cap Default Limit on Agent List Queries (#13382)
* 🛡️ fix: Cap Default Limit on Agent List Queries (#13363)

`GET /api/agents` accepted unbounded requests: when the client omitted
`limit`, the value flowed straight into `getListAgentsByAccess`, which
set `isPaginated = false` and issued an uncapped MongoDB query. Combined
with the unindexed `findPubliclyAccessibleResources` AclEntry scan run
on every request, this produced 10-19s response times and stalled the
connection pool on instances with 100+ agents.

- Default `limit` to 100 in the route handler so client requests without
  `?limit=` paginate by default.
- Default `limit` to 100 in `getListAgentsByAccess` itself as
  defense-in-depth. The function already caps numeric limits at 100, so
  there is no client-facing change.
- Pass `limit: null` explicitly in the actions route, which legitimately
  needs the full editable-agent set, to preserve its existing behavior.
- Add regression tests covering the default cap and the explicit
  unbounded opt-out.

* 🛡️ fix: Avoid agent-list regression for users with 100+ agents

Codex review pointed out that capping `getListAgentsByAccess` at 100
silently truncated agents past the first page for the four consumers
(`useAgentsMap`, `AgentSelect`, `ModelSelectorContext`, `useMentions`)
that read `res.data` without following `has_more`/`after`.

- Raise the function's hard cap from 100 to 1000 to match
  `MAX_AVATAR_REFRESH_AGENTS`, the realistic upper bound the
  avatar-refresh path already assumes. (Side effect: the avatar refresh
  call site was silently being capped at 100 by the old normalize step.)
- In `useListAgentsQuery`, merge `limit: 1000` into params so the four
  consumers above get the user's full accessible set in a single
  round-trip instead of needing cursor pagination.
- Route handler default stays at 100 as defense-in-depth for any other
  caller that omits `limit`.
- Add a regression test asserting an explicit `limit` above 100 now
  returns the full set instead of being clipped.

* 🪢 fix: Keep agent-list cache key stable for mutations

Codex P2 review noted that folding `limit: 1000` into the cache key
broke `allAgentViewAndEditQueryKeys` in `Agents/mutations.ts`, which
references `[QueryKeys.agents, { requiredPermission }]` directly across
eight mutation handlers. After my prior change the cached entry lived
under `[QueryKeys.agents, { limit: 1000, requiredPermission }]`, so
create/update/delete/avatar/action mutations stopped updating the list
the four consumer hooks render — and with `refetchOnMount` and focus/
reconnect refetches disabled, the UI would stay stale until something
else triggered a fetch.

Split the merged limit out of the cache key: the request to
`dataService.listAgents` still uses `requestParams` (with the default
limit applied), but the React Query cache key uses the caller's `params`
as-is. The mutation cache updates land again, and the request still
returns the user's full accessible set in one round-trip.

* 🛡️ fix: Index AclEntry and paginate agent list internally (#13363)

Completes the perf fix for #13363 properly — resolves both the
unbounded ACL scans Copilot flagged and Codex's tension between "show
all agents" and "don't bypass the server cap".

Backend:
- Add a compound index on `{ principalType, resourceType, permBits,
  resourceId }` to the AclEntry schema. This is the index missing for
  `findPublicResourceIds` and the public branch of the `$or` in
  `findAccessibleResources`, both of which previously fell back to a
  collection scan on every `GET /api/agents`. Adds an `explain`-based
  regression test asserting the public query no longer COLLSCANs.

Client:
- Rewrite `useListAgentsQuery` to follow the server's cursor
  pagination internally and concatenate every page into a single flat
  `AgentListResponse`. Consumers (`useAgentsMap`, `AgentSelect`,
  `ModelSelectorContext`, `useMentions`) get the user's complete
  accessible-agent set without any of them needing to learn about
  cursors, and each individual request uses the server's default
  page size (so the route's 100-default defense-in-depth fires for
  real). Cache key shape is unchanged, so the eight mutation handlers
  in `Agents/mutations.ts` keep matching `allAgentViewAndEditQueryKeys`
  and update the cached list as before.
- Drop the `FULL_AGENT_LIST_LIMIT = 1000` injection added in the
  previous commit — no longer needed once pagination handles the full
  set, and removing it stops bypassing the route default.

* 🧹 fix: CI fallout from C-done-properly refactor

- Collapse multi-line `fetchAllAgentPages` signature in queries.ts so
  prettier stops complaining.
- In the new public-principal index test, grant one ACL entry before
  calling `.explain()` so the collection exists (otherwise mongo returns
  `nonExistentNamespace` and there is no winning plan to inspect).
- Cast the `.explain('queryPlanner')` result to a typed shape — the
  mongoose return type doesn't expose `queryPlanner` directly and was
  failing the TypeScript check.

* 🧪 fix: Test the AclEntry public-principal index via hint, not planner choice

The previous test asserted the query planner did not pick COLLSCAN for
the public-principal lookup. That assertion fails on small collections
(under the planner's collection-size heuristic) — the index exists and
is usable, but with a single document in the test the planner correctly
chooses COLLSCAN as the cheaper plan.

Reshape the assertion:
1. Confirm the new compound index is actually declared by inspecting
   `collection.indexes()` after `syncIndexes()`.
2. Force the planner to that index via `.hint()` and assert the winning
   plan is `IXSCAN` — proves the index is real and serves this query
   shape, without depending on collection-size heuristics.

* 🧹 chore: Slim down verbose comments

The JSDoc and inline comments added across the perf fix had drifted
into multi-paragraph rationale better suited to the PR description than
the source. Collapse to single-line JSDoc that just describes what each
piece does; drop the inline comment in `actions.js` entirely — the call
is self-evident.
2026-05-28 21:37:53 -07:00
Danny Avila
749eb06e67
🧭 fix: Reduce MCP Registry ACL Lookups (#13195) 2026-05-19 17:16:37 -04:00
Danny Avila
62da4c28ed
🛡️ fix: Sanitize Agent List Skill Scope (#13122) 2026-05-14 09:27:41 -04:00
Danny Avila
030dc98a1d
☁️ fix: Enable Azure Agent Provider Uploads (#13045) 2026-05-10 17:47:05 -04:00
Danny Avila
5c338a4642
🛂 fix: Harden Agent File Preview Access (#12981)
* fix: harden agent file access

* style: format agent file query

* fix: prune agent file refs on alternate writes

* test: fix agent pruning specs
2026-05-06 19:56:04 -04:00
Danny Avila
9c81792d25
🔐 feat: Add Signed CloudFront File Downloads (#12970)
* feat: add signed CloudFront downloads

* fix: preserve local IdP avatar paths

* fix: address signed download review findings

* fix: harden CloudFront cookie scope validation

* fix: preserve URL save API compatibility

* fix: store CDN SSO avatars under shared prefix

* fix: Harden CloudFront tenant file access

* fix: Preserve CloudFront download compatibility

* fix: Address CloudFront review follow-ups

* fix: Preserve file URL fallback user paths

* fix: Address download review hardening

* fix: Use file owner for S3 RAG cleanup

* fix: Address final download review nits

* fix: Clear stale avatar CloudFront cookies

* fix: Align download filename helpers with dev

* fix: Address final CloudFront review follow-ups

* fix: Stream S3 URL uploads

* fix: Set S3 stream upload length

* fix: Preserve download metadata filepath

* fix: Avoid remote content length for stream uploads

* fix: Use bounded multipart URL uploads

* fix: Harden S3 filename boundaries
2026-05-06 19:48:30 -04:00
Danny Avila
d83cb84f59 🪆 feat: Subagent configuration in Agent Builder (#12725)
* 🪆 feat: Subagents configuration (isolated-context child agents)

Surfaces the new @librechat/agents `SubagentConfig` primitive in the Agent
Builder. Subagents let a supervisor delegate a focused subtask to a child
graph running in an isolated context window: verbose tool output stays in
the child, only a filtered summary returns to the parent.

Data model: new `subagents: { enabled, allowSelf, agent_ids }` on Agent,
wired through the Zod, Mongoose, and form schemas plus a new
`AgentCapabilities.subagents` capability (enabled by default).

Backend: `initialize.js` loads explicit subagent configs alongside handoff
agents, and drops subagent-only references from the parallel/handoff maps
so they don't leak into the supervisor's graph. `run.ts` emits
`SubagentConfig[]` on the primary `AgentInputs` — a self-spawn entry when
`allowSelf` is enabled plus one entry per configured agent.

UI: an "Advanced" panel section with an enable toggle, a self-spawn
toggle, and an agent picker (capped at 10). Enabling without adding
agents still yields self-spawn; disabling self-spawn with no agents shows
a warning. A capability flag gates the whole section.

* 🪆 feat: Stream subagent progress to UI (dialog + inline ticker)

Pairs with the @librechat/agents SDK change that forwards child-graph
events through the parent's handler registry (danny-avila/agents#107):

- Self-spawn and explicit subagents can now use event-driven tools,
  because child `ON_TOOL_EXECUTE` dispatches reach our ToolService via
  the parent's registered handler.
- The same forwarding path wraps the child's run_step / run_step_delta
  / run_step_completed / message_delta / reasoning_delta dispatches in
  a new `ON_SUBAGENT_UPDATE` envelope, with start/stop/error bookends.

Backend: `callbacks.js` registers an `ON_SUBAGENT_UPDATE` handler that
forwards each envelope straight to the SSE stream.

Frontend:
- `useStepHandler` consumes `ON_SUBAGENT_UPDATE` events and merges them
  into a per-tool_call Recoil atom (`subagentProgressByToolCallId`).
  First-seen `subagentRunId` claims the most-recent unclaimed `subagent`
  tool call in the active response message — a temporal mapping, no SDK
  wire-format change needed to correlate child runs with parent tool
  calls.
- New `SubagentCall` part component replaces the default `ToolCall`
  rendering when `toolCall.name === Constants.SUBAGENT`: compact status
  ticker showing the 3 most recent update labels, clickable to open a
  dialog with the full activity log + final markdown-rendered result.
- Adds `Constants.SUBAGENT`, `StepEvents.ON_SUBAGENT_UPDATE`, and
  `SubagentUpdateEvent` type in data-provider.

Tests:
- `packages/api npx jest run-summarization` — 23 pass
- `api npx jest initialize` — 16 pass
- `npm run build` — clean

Dependency note: bumps `@librechat/agents` to `^3.1.67-dev.1` — requires
the SDK PR (danny-avila/agents#107) to be merged to dev and published
before this PR merges. `ON_SUBAGENT_UPDATE` is absent from dev.0, so the
handler registration would be a no-op with the older SDK but would not
crash.

* 🪆 fix: address Codex review and review audit on subagents

Stacks on top of the SDK change in danny-avila/agents#107 (bumped to
`^3.1.67-dev.2`).

- **P1 (`initialize.js`)**: subagent-only agents were being deleted from
  both `agentConfigs` AND `agentToolContexts`. The tool-execute handler
  resolves execution context (agent, tool_resources, skill ACLs) from
  `agentToolContexts`, so explicit subagents would run without their
  configured resources and skip action tools. Now only `agentConfigs`
  is pruned — tool context stays intact.
- **P2 (`AgentSubagents.tsx`)**: toggling subagents off set the form
  field to `undefined`; `removeNullishValues` stripped it from the
  PATCH, leaving the server copy enabled. Now it persists an explicit
  `{ enabled: false, ... }` so the update actually clears state.

- **Finding 1 (MAJOR)** — `agent_ids` Zod schema gains `.max()` via a
  new `MAX_SUBAGENTS` export from `data-provider` (shared with the UI
  cap). Crafted payloads can't trigger hundreds of `processAgent`
  calls.
- **Finding 2 (MAJOR)** — `subagentProgressByToolCallId` atomFamily
  atoms are now tracked in a ref and reset from `clearStepMaps` via a
  `useRecoilCallback({ reset })`. No monotonic growth across a session.
- **Finding 3 (MAJOR)** — early-arriving `ON_SUBAGENT_UPDATE` events
  whose parent `tool_call_id` is not yet mapped are now buffered in
  `pendingSubagentBuffer` (keyed by `subagentRunId`) and replayed in
  arrival order once correlation completes. Mirrors the existing
  `pendingDeltaBuffer` pattern.
- **Finding 4 (MAJOR)** — switched to deterministic correlation via
  the new `parentToolCallId` that SDK `3.1.67-dev.2` threads through
  from `ToolRunnableConfig.toolCall.id`. Temporal fallback now iterates
  oldest-unclaimed-first (forward), matching tool-call creation order,
  so concurrent spawns map correctly.
- **Finding 6 (MINOR)** — `agent_ids` are deduped on the backend via
  `new Set(...)` before the load loop. Duplicates no longer produce
  duplicate `SubagentConfig` entries visible to the LLM.
- **Finding 7 (MINOR)** — events array inside each Recoil atom is
  capped at 200 entries. Long-running subagents no longer replay O(n)
  spreads on every update; the dialog log still shows the cap window.
- **Finding 8 (MINOR)** — documented: subagents are loaded only for
  the primary agent this release (handoff children get self-spawn but
  not explicit sub-subagents). In-code comment added so the next
  maintainer doesn't wonder.
- **Finding 9 (NIT)** — removed `{!isSubmitting && null}` dead code
  and the misleading announce-polite comment in `SubagentCall`.

- New `validation.spec.ts` — 9 tests covering the cap on
  `agent_ids.length` at the subagent schema, agent-create, and
  agent-update layers.
- `run-summarization` — 23 pass, `initialize` — 16 pass, total backend
  package: 103 pass across touched areas.

Findings 5 (component tests) and 10 (micro-allocation) are tracked
but deferred; the former needs a Recoil-RenderHook harness that isn't
in this PR's scope, and the latter has negligible impact (one `Array.from`
per subagent run).

* 🧪 test: integration coverage for subagent correlation + backend loading

Addresses the follow-up audit on #12725 with real-code tests (no mock
handlers, only the existing setMessages/getMessages spies and the
standard mongodb-memory-server harness).

Six new tests under a dedicated `describe('subagent loading')`:
- loads a configured subagent, populates `subagentAgentConfigs`, keeps
  it out of `agentConfigs`
- **P1 regression guard**: drives the real `toolExecuteOptions.loadTools`
  closure with the subagent id and asserts `loadToolsForExecution` is
  called with `agent: <subagent>`, `tool_resources`, `actionsEnabled`.
  If anyone deletes `agentToolContexts` again, this fails.
- dedup: three copies of the same id load the agent once
- overlap: agent referenced both as handoff target and subagent stays in
  `agentConfigs`
- capability gate: admin disabling `subagents` suppresses loading even
  when the agent has a config
- per-agent disable: `subagents.enabled: false` skips loading entirely

Five new tests under `describe('on_subagent_update event')` using a
real `RecoilRoot` and a companion `useRecoilCallback` reader so writes
from the hook are observable:
- deterministic correlation via `parentToolCallId` (happy path with
  SDK dev.2+)
- fallback: oldest-unclaimed tool call wins for concurrent spawns
  without `parentToolCallId`
- early-arrival buffer: updates with no mapping get buffered and
  replayed once the tool call appears
- event cap: 205 updates collapse to 200 retained, oldest dropped
- `clearStepMaps` resets tracked atoms back to their null default

- F2 — added explicit `// TODO` marker for handoff-subagent-loading
  extension (matches the comment that referenced it).
- F3 — dropped the unnecessary `MAX_SUBAGENTS as MAX_SUBAGENTS_CAP`
  alias; just import the constant directly.
- Bumped `@librechat/agents` to `^3.1.67-dev.3` to pick up the SDK's
  paired test additions.

- `api/server/services/Endpoints/agents/initialize.spec.js` — 22 pass
  (6 new + 16 existing)
- `packages/api/src/agents/validation.spec.ts` +
  `run-summarization.test.ts` — 103 pass
- `client/src/hooks/SSE/__tests__/useStepHandler.spec.ts` — 48 pass
  (5 new + 43 existing)

* 🪆 fix: strip parent run summary + discovered tools from subagent inputs

Codex P1 on #12725: `buildSubagentConfigs` reused the shared
`buildAgentInput` factory for each explicit child, and that factory
always stamps the parent run's `initialSummary` (cross-run conversation
summary) and `discoveredTools` (tool names the parent's LLM searched
earlier) onto every `AgentInputs` it returns. When subagents were
enabled on a conversation that had already been summarized, every
child inherited that summary — silently defeating the isolated-context
contract and burning extra tokens on unrelated prior chat.

Fix in `run.ts`: after `buildAgentInput(child)`, explicitly clear
`childInputs.initialSummary` and `childInputs.discoveredTools` before
attaching to the `SubagentConfig`. The parent keeps both — that's how
the supervisor receives cross-turn context — but the child starts
fresh.

Paired with danny-avila/agents#107 (bumped to `^3.1.67-dev.4`), which
adds the equivalent strip inside `buildChildInputs` to cover the
self-spawn path where the SDK clones parent `_sourceInputs` directly
and LibreChat never sees the intermediate shape. Belt and suspenders.

Regression test (new):
- `does NOT leak the parent run initialSummary into an explicit child
  (Codex P1 regression)` — sets `initialSummary` on the run, enables
  subagents with an explicit child, asserts the parent still has the
  summary but `childConfig.agentInputs.initialSummary` is `undefined`.
  Same for `discoveredTools`. 24 pass.

* 🪆 fix: capability gate applies to handoff agents + parallel subagent test

### Codex P2 — handoff agents kept `subagents` after capability disabled
The endpoint-level `AgentCapabilities.subagents` gate only cleared
`subagents` on `primaryConfig`. Handoff agents loaded into
`agentConfigs` retained their persisted `subagents.enabled: true`,
and because `run.ts` calls `buildSubagentConfigs` for every agent
input, self-spawn would still fire on a handoff target even when the
admin had disabled the capability globally.

Fix in `initialize.js`: after the subagent loading block, when the
capability is off, iterate `agentConfigs.values()` and clear
`subagents` + `subagentAgentConfigs` on every loaded config.

Regression test: `clears subagents on handoff agents too when
capability is disabled (Codex P2 regression)` — seeds a handoff target
with its own `subagents.enabled: true`, disables the capability at
the endpoint, asserts both primary AND handoff have `subagents`
undefined in the client args. 23 init tests pass.

### Parallel subagent correlation — user-requested verification
Added `keeps parallel subagent streams independent when events
interleave` to `useStepHandler.spec.ts`. Two `subagent` tool calls
seeded side by side, 6 interleaved `ON_SUBAGENT_UPDATE` envelopes
dispatched (a-start, b-start, a-step, b-step, a-stop, b-step), each
carrying its own `parentToolCallId`. Asserts each `tool_call_id`'s
Recoil bucket accumulates only its own run's events, statuses reflect
each run independently (`call_a` → stop, `call_b` → run_step), no
cross-contamination. 49 step-handler tests pass.

* 🪆 fix: SubagentCall detects cancelled / errored states (Codex P2)

Codex P2 on #12725: the old `running` check only consulted
`initialProgress` and the subagent's phase. A user stop, dropped
stream, or backend crash before a terminal `stop`/`error` envelope
arrived would leave the ticker permanently stuck on "working…". Other
*Call components (ToolCall.tsx) already model this via
`!isSubmitting && !finished` → cancelled.

Mirror that pattern. Re-introduce `isSubmitting` on `SubagentCallProps`
(the prop was dropped earlier as 'unused' — that was a bug) and resolve
status as a tri-state:

- `finished`  — initialProgress >= 1, or subagent `stop`/`error`
- `cancelled` — `!isSubmitting && !finished`
- `running`   — neither

New locale keys `com_ui_subagent_cancelled` + `com_ui_subagent_errored`
swap in the right header text per state.

Tests: new `SubagentCall.test.tsx` covers all four states with a real
`RecoilRoot` and a `useRecoilCallback` seeder — no mocked store — 5/5
pass. Includes an explicit P2 regression test that simulates the
`isSubmitting=false, progress.status='run_step', initialProgress<1`
scenario and asserts the cancelled label renders.

* 🪆 feat: semantic ticker + aggregated content-part dialog for subagents

Two rounds of feedback on #12725:

### Ticker — user-readable lines, not raw event names

The old ticker showed \`on_run_step\`, \`on_message_delta\`, etc. — not
meaningful to users. Replaced with \`buildSubagentTickerLines\`, a pure
helper that walks the \`SubagentUpdateEvent\` stream and emits:

- message/reasoning deltas → a single live "Writing: <last 60 chars>"
  (or "Reasoning: …") line that updates in place as chunks arrive
- run_step with tool_calls → "Using calculator(expression=42*58)" for
  a single call, "Using tool: a, b" for parallel (args dropped when
  multiple so the line stays short)
- run_step_completed → "calculator → 42*58 = 2436" (output truncated
  to 48 chars; falls back to "Tool X complete" when output is empty)
- error → "Error: <message>"
- start / stop / run_step_delta → suppressed (too granular / lifecycle-only)

Args and output pass through \`summarizeArgs\` / \`summarizeOutput\`
which flatten JSON to \`key=value\` pairs and head-truncate long
strings so a 200-line tool output never bloats the ticker.

### Dialog — aggregated content parts via leaf renderers

\`aggregateSubagentContent\` folds the raw event stream into
\`TMessageContentParts[]\` — text/reasoning delta streaks collapse into
single \`TEXT\` / \`THINK\` parts, tool calls become \`TOOL_CALL\` parts,
and \`run_step\` boundaries correctly break text runs around tool
calls. The dialog iterates those parts through a \`SubagentDialogPart\`
renderer that delegates to the existing \`Text\`, \`Reasoning\`, and
\`ToolCall\` leaf components — the same sub-components \`<Part />\` uses
— wrapped in a minimal \`MessageContext\` so reasoning expand state and
cursor animation work.

Leaf components are used directly rather than importing \`<Part />\`
itself to avoid a module cycle (Part → Parts/index → SubagentCall →
Part) and to sidestep a hypothetical nested-subagent rendering.

### Tests

- \`subagentContent.test.ts\` — 19 pure-function tests covering the
  aggregator (text concat, reasoning concat, tool call lifecycle,
  interleaving, phase suppression, late-arriving completions) and the
  ticker builder (live preview truncation, args/output snippets,
  parallel-call handling, output truncation, i18n formatter override).
- \`SubagentCall.test.tsx\` — 9 component tests: 5 status-resolution
  (existing) + 2 ticker (semantic text, delta collapse) + 2 dialog
  (aggregated parts routed to leaf renderers, raw-output fallback).

### Locale keys

New: \`com_ui_subagent_ticker_writing\`, \`…_reasoning\`, \`…_error\`,
\`…_using\`, \`…_using_with_args\`, \`…_tool_complete\`,
\`…_tool_output\`. Preserves i18n at the display layer while the
helper stays pure.

* chore: drop unused com_ui_subagent_activity_log locale key

The dialog no longer renders an "Activity log" section — the new
content-parts renderer replaced it. Also tweaks the dialog description
copy to match.

* 🪆 fix: subagent dialog order, persistence, auto-scroll, width

Follow-up pass addressing the four issues observed in real runs
against a live subagent-using parent.

### Aggregator ordering (reasoning appearing after text it preceded)

Reproducible pattern: LLM emits reasoning → text → tool call in that
order, but the dialog rendered text BEFORE reasoning in the content
array. Root cause: `aggregateSubagentContent` maintained `currentText`
and `currentThink` buffers in parallel and only flushed them at a
`run_step` boundary in a fixed (text, think) order, losing the actual
arrival order.

Fix: when a text chunk arrives, close any open think buffer first
(pushes it into the content array right then); symmetric for think →
text. Two new regression tests cover the exact reasoning → text →
tool_call sequence from the screenshot and the repeated
reasoning ↔ text flow across a turn.

### Content persists after completion (markdown not rendering when done)

`clearStepMaps` was calling `resetSubagentAtoms()` at stream end,
which wiped every `subagentProgressByToolCallId` entry. Once reset,
`contentParts.length === 0` and the dialog fell back to rendering the
raw `output` string with plain text — hence the literal `##`/`**` in
the completed-state screenshot. Stopped resetting; the atoms are
bounded per-call (200-event cap) and per-conversation (one per
subagent spawn) so growth matches the rest of the conversation state.
`resetSubagentAtoms` is kept for a future conversation-switch caller.

Also: routed the raw-`output` fallback (older subagent runs recorded
before the event forwarder existed) through the same
`SubagentDialogPart` → `Text` leaf that content parts use, so its
markdown renders the same way.

### Auto-scroll to bottom while running

Added a `scrollRef` on the dialog body and a `useEffect` that pins
`scrollTop = scrollHeight` while the dialog is open AND the subagent
is running. Triggers on `contentParts.length` (new tool calls / part
boundaries) and `events.length` (intra-part deltas) so the cursor
tracks text streaming. Disabled post-completion so re-opening a
finished run doesn't yank to the bottom.

### Wider dialog

Went from `max-w-2xl` (42rem / 672px — too cramped on maximized
laptop windows) to `w-[min(95vw,64rem)] max-w-[min(95vw,64rem)]`.
Narrow on phones, scales up to 64rem on desktop, always leaves a bit
of margin from the viewport edge. Bumped `max-h-[65vh]` on the scroll
area to give the extra width room to breathe vertically too.

### Tests

- `subagentContent.test.ts` — 21 pass (2 new ordering regressions).
- `useStepHandler.spec.ts` — 49 pass (1 updated to assert atoms are
  *preserved* on clearStepMaps).
- `SubagentCall.test.tsx` — 9 pass (unchanged; aggregator-level tests
  cover the ordering).

* 🪆 feat: persist subagent_content via SDK createContentAggregator

Per-request map of createContentAggregator instances keyed by the
parent's tool_call_id. ON_SUBAGENT_UPDATE handler feeds each event
into the matching aggregator (phase → GraphEvent mapping); AgentClient
harvests contentParts onto the subagent tool_call at message save so
the child's reasoning / tool calls / final text survive a page refresh.

Reusing the SDK's battle-tested aggregator instead of a bespoke one
keeps the persisted shape identical to the parent graph's output and
drops ~100 lines of custom aggregation code.

* 🪆 fix: incremental subagent aggregation + dialog render parity

**Disappearing tool_calls**: the Recoil atom trimmed events to a 200-long
rolling window, so verbose subagents could shed the `run_step` that
originally created a tool_call part — rebuilding content from the trimmed
window then produced only the surviving text/reasoning. Fix: fold each
envelope into `contentParts` incrementally in the atom as it arrives
(new `foldSubagentEvent` + cursor state). Event trim window now affects
only the ticker, never the dialog.

**Render parity**: dialog now applies `groupSequentialToolCalls` and
renders single parts through `Container` + grouped batches through
`ToolCallGroup` — same spacing and "Used N tools" collapsing the main
message view uses.

**Width**: `min(96vw, 80rem)` — wider on big screens, still responsive.

**Labels**: "Subagent: X" is jargon. Named subagents render as
`Running "{name}" agent` / `Ran "{name}" agent` (past tense on
completion); self-spawns use `Running subtask` / `Ran subtask` since
`Running "self" agent` reads badly.

* 🪆 polish: subagent dialog parity + agent avatar in header

**Labels**: drop "subtask" framing. Self-spawn shows `Running agent` /
`Ran agent` (past tense on completion); named subagents stay
`Running "X" agent` / `Ran "X" agent`.

**Dialog render parity**: stop wrapping every part in `Container`.
TEXT keeps its `Container` (gap-3 + `mt-5` sibling margin), THINK and
TOOL_CALL render bare so their own wrappers set the full-column width
the regular message view gives them — matches the main `<Part>` dispatch.
Outer scroll region now uses `px-4 py-3` padding and a
`max-w-full flex-grow flex-col gap-0` inner wrapper, mirroring the
`MessageParts` container the main conversation uses.

**Avatar**: header icon now renders the subagent's configured avatar
via `MessageIcon` when `useAgentsMapContext()` has the child agent,
falling back to the `Users` SVG (which keeps its running-state pulse).
Same icon-left-of-label pattern the tool UI uses.

* 🪆 polish: subagent group label, ticker throttle + tail-ellipsis, scroll button

**Grouped label**: ToolCallGroup now detects all-subagent batches and
labels them "Running N agents" / "Ran N agents" instead of "Used N
tools". Mixed batches keep the existing label. The tool-name summary
is suppressed for all-subagent groups (every entry dedupes to
"subagent", which adds nothing).

**Ticker width + tail-ellipsis**: raise the preview cap to 300 chars so
wide containers aren't half-empty, and flip the ticker `<li>` to
`dir="rtl"` so `text-overflow: ellipsis` clips the *oldest* characters
(visually the left edge) — the newest tokens stay pinned to the right
regardless of container width. Bidi lays out the Latin text LTR
internally, the rtl only affects which side gets the ellipsis.

**Throttle**: `useThrottledValue` hook (trailing-edge, 1.2s) smooths
the live `Writing: …` preview so tokens no longer strobe past the
eye faster than they can be read. Ref-based internals (not `useState`)
avoid infinite-update loops when the upstream value is a new-reference
each render; `NEGATIVE_INFINITY` sentinel ensures the very first value
passes through synchronously so tests and first paint aren't delayed.

**Scroll-to-bottom**: dialog tracks `isAtBottom` with a 120px
threshold; auto-scroll only engages when the user is already following
along, and a persistent jump-to-latest button appears whenever they
scroll up — no more fighting the auto-scroll to read back.

* 🪆 polish: snappier ticker, prefix-safe labels, agents icon, readable lines

**Ticker lines are now incrementally aggregated in the atom** — same
pattern as contentParts. The raw-events rolling window is gone; event
volume no longer caps what the ticker can display. Verbose subagents
that used to drop early tool_call lines out of the window now keep the
full 3-line history (using_tool, tool_complete, writing).

**Discriminated-union ticker lines** split a constant prefix (e.g.
"Writing:") from a tail-truncatable body. The prefix lives in a
`shrink-0` span so it never gets clipped when the body overflows; the
body uses `dir="rtl"` only on itself — scoped so non-streaming lines
(e.g. "Waiting for first update…") can't get their trailing ellipsis
flipped by bidi.

**Content-aware throttle**: 800ms interval (down from 1200ms), skipped
entirely while the live buffer is below 120 chars. Early tokens now
appear immediately — no more "Reasoning: I" sitting blank for a full
second before the next heartbeat. Once the preview is long enough to
fill the container, throttling kicks in at the tighter interval.

**Header label** is now a constant verb + optional muted sub-label.
Base reads "Running agent" / "Ran agent" / "Cancelled agent" / "Agent
errored" for every subagent; named subagents get the configured agent
name rendered to the right in secondary text (self-spawns and
unresolved names omit it — "Running self agent" is nonsense).

**ToolCallGroup** now detects `allSubagents` and swaps `StackedToolIcons`
for a single `Users` glyph — otherwise the group header shows a wrench
("tool") icon next to "Ran 5 agents", which reads wrong.

* 🪆 feat: delimiter-aware tool labels in ticker + full-width tool lines

New shared `parseToolName` helper in `client/src/utils/toolLabels.ts`
— single source of truth for splitting `<tool>_mcp_<server>` ids and
mapping native tool names (web_search, execute_code, …) to their
friendly translation keys. `ToolCallGroup` drops its inline copy and
pulls from this helper.

Ticker tool lines now use the shared parser + a new `ToolIdentifier`
sub-renderer so the live log reads like the main tool UI:

  - MCP tool  → `<server> · <code-badge:tool>` (e.g. "github · `search_code`")
  - Native   → friendly name from `TOOL_FRIENDLY_NAME_KEYS`
  - Unknown  → bare `<code>` badge of the raw id

The `using_tool` / `tool_complete` rows now render with a
`flex w-full items-baseline gap-1 overflow-hidden` layout matching
the writing/reasoning rows — they take the full container width
instead of collapsing to content size. Output snippets on
`tool_complete` get the same tail-side `dir="rtl"` ellipsis so the
newest characters stay flush-right when the container is narrow.

Dropped the now-unused template i18n keys
(`com_ui_subagent_ticker_using_with_args`,
`com_ui_subagent_ticker_tool_complete`,
`com_ui_subagent_ticker_tool_output`) in favor of tokens the JSX
composes structurally. Only English is touched per the project rule;
other locales follow externally.

* 🪆 fix: dialog scroll button + auto-scroll during streaming deltas

Two race/trigger bugs in the dialog's scroll behavior:

**Button never showed**: `addEventListener('scroll', …)` in a
`useEffect` ran before Radix's portal had actually committed the
scroll container, so `scrollRef.current` was still null — the listener
never attached, `isAtBottom` stayed stuck at its initial `true`, and
the jump-to-latest button was never rendered. Swap to React's
`onScroll` prop on the element itself so the handler wires up as part
of DOM commit, not a post-commit effect.

**Auto-scroll stalled during text streaming**: the pin-to-bottom
effect only re-fired on `contentParts.length` changes. Message/reasoning
deltas extend the last TEXT/THINK part's `.text` without changing the
array length — so the view would drift up as tokens piled in and
never catch back up. Replace the length-dep effect with a
`ResizeObserver` on the inner content div; every height change (new
part or in-place growth) triggers a scroll-pin when the user is still
at the bottom.

* 🪆 fix: drop leading ellipsis from ticker body

truncatePreview was prepending ... to the tail when the buffer
exceeded 300 chars. The component's CSS already produces a left-side
ellipsis for overflow via dir=rtl + text-overflow: ellipsis — stacking
a data-level ellipsis on top renders a stray dot character right after
the Writing: / Reasoning: label (Writing: .Sure!), which looks like a
typo to the reader.

Data now returns just the last 300 chars when truncating; CSS handles
the visual cue whenever the body actually overflows its container.

* 🪆 fix: Codex review — subagent isolation + concurrent-safe throttle

Three findings from the @codex review pass, all valid:

**P1 — buildAgentInput leaks parent discovered-tool state into subagent
children.** `buildAgentInput` mutates `agent.toolRegistry`
(`overrideDeferLoadingForDiscoveredTools` flips `defer_loading:true→false`
on tools the parent previously searched for) and appends those tools'
definitions to the returned `toolDefinitions` before the function
returns. `buildSubagentConfigs` was clearing the reported
`initialSummary` / `discoveredTools` fields on the returned
AgentInputs, but that happened post-return — the registry writes and
extra tool definitions persisted on the child, silently defeating
context isolation and inflating the child's prompt.

Fix: `buildAgentInput` now takes an `isSubagent` flag that gates the
registry-mutation block and omits `initialSummary` /
`discoveredTools` at the source. `buildSubagentConfigs` passes
`{ isSubagent: true }` for every explicit child; no post-hoc cleanup
needed.

**P2 — ToolCallGroup labels a finished subagent group as still
running when the child returned no output.** `getToolMeta` computed
`hasOutput` as `!!tc.output`, which is `false` for a completed
subagent that returned empty text (the UI already has an "empty
result" fallback for that case). `allCompleted` would stay `false`
and the group header stuck on "Running N agents" forever.

Fix: treat `tc.progress === 1` as completion too — progress is the
authoritative lifecycle signal, output is just content.

**P2 — useThrottledValue schedules `setTimeout` during render.**
Discarded renders under Strict Mode / Concurrent rendering would
leave orphan timers firing against stale trees.

Fix: move `setTimeout` into a `useEffect` keyed on `[value, intervalMs,
enabled]`. Render-time still mutates refs (idempotent), but timer
scheduling lives post-commit. Cleanup on unmount and on passthrough
transitions is preserved.

* 🪆 fix: Codex P2 — wipe subagent atoms on conversation switch

`clearStepMaps()` intentionally doesn't reset `subagentProgressByToolCallId`
so a user can reopen a completed subagent's dialog mid-conversation,
but `resetSubagentAtoms` was defined and never exposed / called —
so each completed run's aggregated `contentParts` + `tickerState`
stayed resident in the `atomFamily` for the whole app session.
Unbounded growth across multi-conversation sessions.

Expose `resetSubagentAtoms` from `useStepHandler` and fire it from
`useEventHandlers` whenever the URL's `conversationId` changes.
That's the correct cleanup boundary: historical subagent dialogs
rehydrate from persisted `subagent_content` on each `tool_call` at
message-save time, so wiping live atoms on switch doesn't lose any
viewable history — it just releases per-tool-call state that the
old conversation's components no longer subscribe to.

* 🪆 fix: Codex round 3 — subagent registry isolation + post-run label

Two more valid findings.

**P1 — parent-order registry mutation leaks into subagent inputs.**
`overrideDeferLoadingForDiscoveredTools` mutates `agent.toolRegistry`
in place (the Map *and* the LCTool objects inside it). When an agent
appears both as a handoff target (normal graph node) AND an explicit
subagent child, a subagent build that ran before the parent's build
captures a reference to the same registry — the parent's later mutation
leaks through to the child.

Fix: for subagent children (`isSubagent`), clone the `toolRegistry`
Map and shallow-clone each LCTool inside before returning the inputs.
`defer_loading` flips on parent-graph registry mutations can't
propagate across the clone boundary. `toolDefinitions` also gets a
shallow-copy pass so the same isolation holds for definitions the
child carries directly.

**P2 — "Running N agents" label stuck after cancel/error.**
ToolCallGroup's all-subagent label was gated only on `allCompleted`,
which requires every child to have `hasOutput || progress === 1`.
A subagent that gets cancelled (stream ends, no `stop` phase, no
output) never satisfies that — so even after `isSubmitting` flips
false, the header stays on "Running N agents" while each individual
card correctly shows "Cancelled agent".

Fix: derive a `subagentsDone` flag as `allCompleted || !isSubmitting`
and gate the past-tense label on that. Matches the tri-state each
SubagentCall card already resolves (finished / cancelled / running).

* 🪆 fix: Codex P2 — ACL-check subagents.agent_ids on create/update

Codex flagged that `subagents.agent_ids` was accepted as arbitrary
strings on the create/update routes while `edges` got a
`validateEdgeAgentAccess` pass — so users could save subagent
references to agents they can't VIEW. At runtime `initializeClient`'s
`processAgent` ACL gate silently drops those, so the persisted
configuration and the actual behavior diverged in a way that is
difficult to diagnose.

Refactor: extract the id-set → unauthorized-ids check into a shared
`collectUnauthorizedAgentIds`, wrap it with a dedicated
`validateSubagentAccess`, and plumb the same 403-on-failure response
the edge path already returns. Applied on both POST /agents and
PATCH /agents/:id.

* 🪆 fix: Codex round 5 — ACL-disable escape hatch + ticker order

Two valid findings.

**P1 — can't disable subagents after losing access to a child.**
The subagent ACL check ran on every create/update that echoed back
the `agent_ids` list, even when the user was explicitly disabling
the feature. The UI keeps the list intact when toggling `enabled:
false`, so a user who subsequently lost VIEW on any child would be
locked in a 403 loop — every edit (including the one that turns
subagents off) bounces.

Fix: gate the ACL check on `subagents.enabled !== false` at both
the POST /agents and PATCH /agents/:id handlers. Empty list stays
a no-op. Disabling the feature is always permitted.

**P2 — ticker fold merges out-of-order previews across delta
switches.** `foldSubagentEventIntoTicker` carried `textLineIdx` /
`thinkLineIdx` across a reasoning → text → reasoning transition,
so the second reasoning chunk appended to the original reasoning
line instead of starting a new chronological one.

Fix: close the opposite buffer + cursor when a delta-type switch
is detected (same rule the content-parts reducer already applies).
Added a regression test.

* 🪆 fix: Codex round 6 — preserve mid-stream atoms + honor sequential suppression

Two valid findings.

**P2 — atom reset fires on initial chat URL assignment.**
`useEventHandlers` initialized `lastConversationIdRef` from the URL's
current `paramId`, then reset subagent atoms whenever the ref and
`paramId` disagreed. For a brand-new conversation the URL stamp goes
from `undefined → "abc123"` while the first response is still
streaming, which used to drop subagent ticker/content state mid-run
and leave dialogs missing earlier updates.

Fix: only reset when *both* the old and new IDs are non-null and
differ — i.e. a user-initiated switch between two established
conversations. The initial assignment passes through without
clearing.

**P2 — ON_SUBAGENT_UPDATE bypassed `hide_sequential_outputs`.**
Every other streaming handler in `callbacks.js`
(`ON_RUN_STEP`, `ON_MESSAGE_DELTA`, etc.) gates emission on
`checkIfLastAgent` + `metadata?.hide_sequential_outputs`, but the
subagent forwarder did an unconditional `emitEvent` — so intermediate
agents in a sequential chain were leaking their children's activity
to the client even when the chain was configured to suppress
intermediates.

Fix: accept `metadata` and apply the same `isLastAgent ||
!hide_sequential_outputs` gate. Aggregation still runs regardless of
visibility (persistence + dialog depend on it); only the SSE forward
is suppressed.

* 🪆 fix: Codex P2 — gate subagent ACL check on endpoint capability

`validateSubagentAccess` ran on every create/update where
`subagents.enabled !== false`, regardless of the endpoint-level
`subagents` capability. When the capability is off at the appConfig
level, `initializeClient` already strips the `subagents` block at
runtime — so persisted `agent_ids` are inert — but the validation
could still 403 on a legacy record whose referenced child is no
longer viewable, blocking unrelated edits.

Fix: add `isSubagentsCapabilityEnabled(req)` that reads the agents
endpoint's capabilities from `req.config` and gate both the create
and update ACL checks on it. Capability-off environments can update
agents with stale `subagents` data freely; capability-on keeps the
full ACL protection.

* 🪆 fix: Codex P2 — reset subagent atoms on id→null navigation too

Previous guard (both-established) skipped the reset whenever
`paramId` became null/undefined, so navigating from an existing
chat to a "new chat" route left stale subagent progress resident
in the `atomFamily` until the user picked a specific different
chat.

Swap the both-established check for a one-time flag: skip only the
very first `undefined → id` transition (the brand-new-chat URL
stamp that happens mid-stream), then reset on any subsequent
change — id→id, id→null, null→id-after-reset. If the user started
on an established chat the flag is true at mount, so the guard is
a no-op and every navigation resets normally.

* 🪆 fix: Codex round 9 — subagent persistence gate + handoff children

Two valid findings.

**P1 — hide_sequential_outputs also gates persistence.**
The previous fix gated the SSE forward on
`isLastAgent || !hide_sequential_outputs` but still ran the
per-tool-call `createContentAggregator` aggregation unconditionally.
`finalizeSubagentContent` would then attach the hidden intermediate
agent's child reasoning / tool output to the saved message, so a
page refresh could reveal activity that was intentionally suppressed
live. Move the visibility gate to the top of the handler — hidden
agents now skip both aggregation and emission, so
"hide_sequential_outputs" is a consistent "don't record" rule for
subagent traces.

**P2 — handoff agents' explicit subagents were silently dropped.**
`initializeClient` only resolved `subagentAgentConfigs` for the
primary config, so an agent used via handoff that had its own
`subagents.agent_ids` saved in the builder would get self-spawn
only; every explicit child was quietly ignored, creating a
saved-config / runtime mismatch the user couldn't diagnose.

Extract the resolution into a shared `loadSubagentsFor(config)`
helper and invoke it for the primary and every handoff agent in
`agentConfigs`. The `edgeAgentIds` precomputation stays outside
the helper (it's loop-invariant). Capability-off shortcuts return
empty early so the existing strip-on-capability-off path still
holds.

* 🪆 fix: Codex P2 — recursive subagent build for multi-level delegation

Previously only the outer `agents[]` loop attached `subagentConfigs`
to its inputs, so a child used as a subagent (invoked via the
`subagent` tool) lost every explicit spawn target of its own. A
user-valid configuration like A → B → C would only run the top
layer; B could never actually delegate to C from inside A's run.

Recursively build `subagentConfigs` for each child inside
`buildSubagentConfigs`, passing the child's freshly-constructed
`childInputs` down so its own `subagents.enabled` children get
resolved too. Added cycle protection via an `ancestors` Set — a
configuration like A → B → A is safely cut off at the second
encounter of A rather than recursing forever (the existing
`child.id === agent.id` guard already prevents the direct self-loop).

* 🪆 fix: Codex P2 — reset subagent atoms on useEventHandlers unmount

The effect that resets subagent atoms only fired on `paramId` change,
so unmounting the chat container (route change away from /c) never
flushed the atoms. `knownSubagentAtomKeys` lives in a ref inside
`useStepHandler` — once the hook unmounts the ref is gone, so a
subsequent remount can't clean atoms it never registered.

Added a second `useEffect` that only runs cleanup on unmount (empty
deps aside from the stable `resetSubagentAtoms` callback). Keeps
`atomFamily` bounded across full route teardowns too.

* 🪆 fix: Codex round 13 — cyclic subagent guard + prefer persisted

Two valid findings.

**P1 — cyclic subagent ref reloads the primary.** A configuration
like `A ↔ B` (B lists A as its own subagent) would send
`loadSubagentsFor` down a path that couldn't find A in
`agentConfigs` (the primary isn't stored there), so it called
`processAgent(A)` a second time. That inserts a fresh config for
the primary id, which downstream duplicates in
`[primary, ...agentConfigs.values()]` and can replace the primary's
tool context with the reloaded copy.

Fix: short-circuit when a subagent ref points back at
`primaryConfig.id` — reuse the already-loaded primary config.
Primary is always an edge id so no pruning bookkeeping needed.

**P2 — live atom preferred over canonical persisted trace.** The
dialog picked `progress.contentParts` ahead of
`persistedContent`, but the Recoil bucket is best-effort — after
a disconnect/reconnect it can be stale or partial. The server's
`subagent_content` on the `tool_call` is the canonical record
refreshed on sync. Preferring live could hide completed
tool/reasoning history that was actually persisted.

Fix: flip the preference order. Persisted wins when it's
non-empty; live covers the mid-stream window (before the parent
message saves, persisted is empty) and the older-runs fallback.
Updated the test that enforced the old order to lock the new
semantics in (separate mid-stream live-fallback assertion kept).

* 🪆 fix: Codex P2 — subagent atom reset rule simplified to 'leaving established id'

The `hasEstablishedConversationRef` + check for initial undefined→id
covered the first navigation but missed the equivalent mid-stream
URL stamp when a user goes from an existing chat to a new chat and
sends a message there (`id → null → newId`). The null → newId
transition was still hitting the reset branch and wiping the
in-flight subagent ticker/content for that first turn.

Simpler rule: only reset when the PREVIOUS paramId is an established
id. Every transition AWAY from an established chat clears (id→id2,
id→null, id→undefined); every transition FROM null/undefined passes
through (initial mount, new-chat URL stamp mid-stream). Drop the
`hasEstablishedConversationRef` machinery in favor of that single
condition.

* 🪆 fix: Codex P2 — match runtime's strict subagent enable check in ACL

Runtime (`initializeClient` + `run.ts`) treats `subagents?.enabled`
as a truthy predicate — `undefined`, `null`, missing, and `false`
all short-circuit. The ACL gate was using `!== false` which
accepted `undefined` / missing as "enabled" and could 403 a payload
whose subagent tool would be inert at runtime.

Swap both create and update to `enabled === true`. Only a strictly-
enabled payload triggers the ACL check; the disable path (`false`)
still passes through so a user who lost VIEW on a child can still
save the disable edit.

* 🪆 fix: Codex P2 — reject missing subagent references with 400

`validateSubagentAccess` collapsed through `collectUnauthorizedAgentIds`,
which returns an empty list for ids with no DB record — so typos and
references to deleted agents passed validation silently, and
`initializeClient` later dropped them at runtime. Saved config would
then list spawn targets that the backend never honored, a hard-to-
diagnose drift.

Refactor the helper into `classifyAgentReferences(ids, …)` which
returns `{ missing, unauthorized }` separately. `validateEdgeAgentAccess`
keeps its old semantics (missing is intentional — a self-referential
`from` names the agent being created). `validateSubagentReferences`
surfaces both buckets so the create/update handlers can 400 on
missing and 403 on unauthorized with distinct error messages and
`agent_ids` lists.

* 🪆 polish: tighten subagent dialog grid gap to gap-2

OGDialogContent's grid default is `gap-4`, which renders the title,
description, and scroll area as three visually separated panels.
Drop to `gap-2` so they read as one block.

* 🪆 polish: swap Subagents above Handoffs in Advanced panel

Subagents is the more common knob users reach for, so show it first.
Handoffs keep the same Controller wiring, just move below.
2026-04-25 04:02:01 -04:00
Danny Avila
181d705579
🧹 fix: Clean Up Orphaned Agent File Stubs After Deletion (#12781)
* 🧹 fix: Prune Orphaned File References on File Deletion

Deleting a file via the Manage Files tab left its file_id in every agent's
tool_resources.*.file_ids. Stubs accumulate until the frontend dedupe keys
them as duplicates and blocks all new uploads (issue #12776).

- Add removeAgentResourceFilesFromAllAgents in packages/data-schemas: a
  single updateMany/$pullAll across every EToolResources category.
- Invoke it from processDeleteRequest after db.deleteFiles so every
  referencing agent is cleaned up, not just the one passed in req.body.
- Wrap the cleanup in try/catch so a stale agent update cannot mask a
  successful file deletion.

* 🧼 fix: Prune Orphaned File References on Agent Update

Already-affected agents would stay broken even after the delete-time fix:
the stubs sit on the agent document until something strips them. Heal them
on the next save (issue #12776).

- Add collectToolResourceFileIds + stripFileIdsFromToolResources helpers
  in @librechat/api — centralizing the tool_resources traversal used by
  the controller and the follow-up migration script.
- In updateAgentHandler, check the effective tool_resources against the
  files collection. When orphans are found, either strip them from the
  incoming tool_resources (if the update sets them) or run the bulk
  cleanup (if the update leaves tool_resources untouched).

* 🧰 chore: Add Migration to Clean Up Orphaned Agent File References

Complements the delete-time and save-time fixes by healing agents that
already accumulated orphan stubs before the upgrade (issue #12776). The
script is idempotent — re-running it on a clean database is a no-op.

- Add config/migrate-orphaned-agent-files.js following the existing
  migrate-*.js convention: --dry-run by default omitted (writes by
  default) and --batch-size= tuning knob. Streams agents via cursor.
- Register migrate:orphaned-agent-files and :dry-run npm scripts.
- Reuse collectToolResourceFileIds from @librechat/api so migration and
  runtime share the same traversal logic.

* 🩹 fix: Address Codex/Copilot Review on Orphaned Agent File Cleanup

Refines the #12776 fix series based on automated review feedback.

- Scope save-time pruning to the current agent only. When a PATCH
  carries tool_resources, strip orphans from the incoming payload and
  pay the DB round-trip only then. Removes the collection-wide
  updateMany previously triggered when tool_resources was absent
  (Codex P2 / Copilot).
- Wrap the orphan check in try/catch so a transient db.getFiles
  failure can't turn a good save into a 500 (comprehensive review #1).
- Replace Object.values(EToolResources) casts with an explicit list of
  agent-side categories in both orphans.ts and agent.ts. code_interpreter
  belongs to the Assistants API and isn't a key of AgentToolResources —
  including it was a type lie and generated dead MongoDB clauses
  (comprehensive review #3, #8).
- Export TOOL_RESOURCE_KEYS from @librechat/api and consume it in the
  migration script, dropping one duplicated definition (#4).
- Cap migration results.details at 50 sample entries so the memory
  footprint stays bounded on deployments with thousands of corrupted
  agents (Codex P3).
- Add migrate:orphaned-agent-files:batch npm script to match the
  convention set by migrate-agent-permissions / migrate-prompt-permissions
  (#7).
- Add controller-level tests covering the three orphan-pruning paths:
  strip from incoming tool_resources, leave alone when tool_resources
  is absent, swallow db.getFiles errors and still save (#6).
- Back pre-existing "should validate tool_resources in updates" test's
  file_ids with real File docs — the new pruning would otherwise strip
  them, and that test is about OCR conversion / schema filtering, not
  file existence. Register the File model in beforeAll so the fixture
  works.

* 🩹 fix: Tighten TOOL_RESOURCE_KEYS Type and Align Migration Sample Output

Two follow-ups from the second review pass.

- Type data-schemas' TOOL_RESOURCE_KEYS as ReadonlyArray<keyof
  AgentToolResources> instead of readonly string[]. Data-schemas depends
  on data-provider, so the import is clean. Catches typos and aligns
  with the matching export in @librechat/api — doesn't guarantee
  exhaustiveness, but that's a TypeScript limitation, not a workspace
  one.
- Align the migration's console output with DETAIL_SAMPLE_LIMIT: print
  every collected detail (up to 50) and, when more agents were affected
  than the sample size allowed, show a truncation notice. The old hard
  cap of 25 meant affected agents in the 26-50 range were collected
  but never shown.

*  test: Add Integration Coverage for Orphan Cleanup Paths (#12776)

Exercise the delete-time and migration paths end-to-end against a real
in-memory Mongo. Catches integration bugs the isolated unit tests on
each layer couldn't.

- api/server/services/Files/process.integration.spec.js — the primary
  repro: seed an Agent + File, call processDeleteRequest, assert the
  file_id disappears from every referencing agent's tool_resources
  while unrelated agents stay untouched. Also covers the no-op case
  and confirms a failure in the new cleanup step cannot roll back the
  file deletion itself.
- api/test/migrate-orphaned-agent-files.spec.js — drives the migration
  module: --dry-run reports without writing, apply mode prunes across
  every tool_resource category, re-running is idempotent, and
  DETAIL_SAMPLE_LIMIT caps the in-memory sample on wide corruption.
  Mocks only the connect helper (the spec owns the mongoose instance)
  so the real migration code path — cursor, $pullAll, reduce — runs.

* 🔒 fix: Run Orphan Cleanup Migration in System Tenant Context

Codex P2 catch: under TENANT_ISOLATION_STRICT=true, the migration
throws on the very first Agent.countDocuments() because the tenant
isolation plugin fail-closes on queries without tenant context — which
makes migrate:orphaned-agent-files unusable on the exact deployments
most likely to have accumulated corruption.

- Wrap the scan/prune body in runAsSystem so queries bypass the tenant
  filter (SYSTEM_TENANT_ID sentinel). The migration legitimately needs
  cross-tenant visibility — this is the same pattern seedDatabase and
  the S3 refresh job already use.
- Add a regression test that spies on Agent.countDocuments() and
  asserts the active tenantStorage context is SYSTEM_TENANT_ID during
  the call. Pins the wrap against future regressions without the
  brittleness of toggling the strict-mode env var (which caches on
  first read).

Note: the delete-time and save-time paths already run inside an
authenticated HTTP request where tenantStorage.run is set by auth
middleware, so the cleanup naturally scopes to the current tenant —
which is the correct behavior there since file ownership is
tenant-scoped.

* 🧹 chore: Drop Unused path Import From Process Integration Spec

Leftover from an earlier iteration that resolved the migration path
via path.resolve before I switched to a relative require. The import
does nothing now — removing it.
2026-04-22 11:35:48 -07:00
Danny Avila
fda72ac621
🏗️ refactor: Remove Redundant Caching, Migrate Config Services to TypeScript (#12466)
* ♻️ refactor: Remove redundant scopedCacheKey caching, support user-provided key model fetching

Remove redundant cache layers that used `scopedCacheKey()` (tenant-only scoping)
on top of `getAppConfig()` which already caches per-principal (role+user+tenant).
This caused config overrides for different principals within the same tenant to
be invisible due to stale cached data.

Changes:
- Add `requireJwtAuth` to `/api/endpoints` route for proper user context
- Remove ENDPOINT_CONFIG, STARTUP_CONFIG, PLUGINS, TOOLS, and MODELS_CONFIG
  cache layers — all derive from `getAppConfig()` with cheap computation
- Enhance MODEL_QUERIES cache: hash(baseURL+apiKey) keys, 2-minute TTL,
  caching centralized in `fetchModels()` base function
- Support fetching models with user-provided API keys in `loadConfigModels`
  via `getUserKeyValues` lookup (no caching for user keys)
- Update all affected tests

Closes #1028

* ♻️ refactor: Migrate config services to TypeScript in packages/api

Move core config logic from CJS /api wrappers to typed TypeScript in
packages/api using dependency injection factories:

- `createEndpointsConfigService` — endpoint config merging + checkCapability
- `createLoadConfigModels` — custom endpoint model loading with user key support
- `createMCPToolCacheService` — MCP tool cache operations (update, merge, cache)

/api files become thin wrappers that wire dependencies (getAppConfig,
loadDefaultEndpointsConfig, getUserKeyValues, getCachedTools, etc.)
into the typed factories.

Also moves existing `endpoints/config.ts` → `endpoints/config/providers.ts`
to accommodate the new `config/` directory structure.

* 🔄 fix: Invalidate models query when user API key is set or revoked

Without this, users had to refresh the page after entering their API key
to see the updated model list fetched with their credentials.

- Invalidate QueryKeys.models in useUpdateUserKeysMutation onSuccess
- Invalidate QueryKeys.models in useRevokeUserKeyMutation onSuccess
- Invalidate QueryKeys.models in useRevokeAllUserKeysMutation onSuccess

* 🗺️ fix: Remap YAML-level override keys to AppConfig equivalents in mergeConfigOverrides

Config overrides stored in the DB use YAML-level keys (TCustomConfig),
but they're merged into the already-processed AppConfig where some fields
have been renamed by AppService. This caused mcpServers overrides to land
on a nonexistent key instead of mcpConfig, so config-override MCP servers
never appeared in the UI.

- Add OVERRIDE_KEY_MAP to remap mcpServers→mcpConfig, interface→interfaceConfig
- Apply remapping before deep merge in mergeConfigOverrides
- Add test for YAML-level key remapping behavior
- Update existing tests to use AppConfig field names in assertions

* 🧪 test: Update service.spec to use AppConfig field names after override key remapping

* 🛡️ fix: Address code review findings — reliability, types, tests, and performance

- Pass tenant context (getTenantId) in importers.js getEndpointsConfig call
- Add 5 tests for user-provided API key model fetching (key found, no key,
  DB error, missing userId, apiKey-only with fixed baseURL)
- Distinguish NO_USER_KEY (debug) from infrastructure errors (warn) in catch
- Switch fetchPromisesMap from Promise.all to Promise.allSettled so one
  failing provider doesn't kill the entire model config
- Parallelize getUserKeyValues DB lookups via batched Promise.allSettled
  instead of sequential awaits in the loop
- Hoist standardCache instance in fetchModels to avoid double instantiation
- Replace Record<string, unknown> types with Partial<TConfig>-based types;
  remove as unknown as T double-cast in endpoints config
- Narrow Bedrock availableRegions to typed destructure
- Narrow version field from string|number|undefined to string|undefined
- Fix import ordering in mcp/tools.ts and config/models.ts per AGENTS.md
- Add JSDoc to getModelsConfig alias clarifying caching semantics

* fix: Guard against null getCachedTools in mergeAppTools

* 🔍 fix: Address follow-up review — deduplicate extractEnvVariable, fix error discrimination, add log-level tests

- Deduplicate extractEnvVariable calls: resolve apiKey/baseURL once, reuse
  for both the entry and isUserProvided checks (Finding A)
- Move ResolvedEndpoint interface from function closure to module scope (Finding B)
- Replace fragile msg.includes('NO_USER_KEY') with ErrorTypes.NO_USER_KEY
  enum check against actual error message format (Finding C). Also handle
  ErrorTypes.INVALID_USER_KEY as an expected "no key" case.
- Add test asserting logger.warn is called for infra errors (not debug)
- Add test asserting logger.debug is called for NO_USER_KEY errors (not warn)

* fix: Preserve numeric assistants version via String() coercion

* 🐛 fix: Address secondary review — Ollama cache bypass, cache tests, type safety

- Fix Ollama success path bypassing cache write in fetchModels (CRITICAL):
  store result before returning so Ollama models benefit from 2-minute TTL
- Add 4 fetchModels cache behavior tests: cache write with TTL, cache hit
  short-circuits HTTP, skipCache bypasses read+write, empty results not cached
- Type-safe OVERRIDE_KEY_MAP: Partial<Record<keyof TCustomConfig, keyof AppConfig>>
  so compiler catches future field rename mismatches
- Fix import ordering in config/models.ts (package types longest→shortest)
- Rename ToolCacheDeps → MCPToolCacheDeps for naming consistency
- Expand getModelsConfig JSDoc to explain caching granularity

* fix: Narrow OVERRIDE_KEY_MAP index to satisfy strict tsconfig

* 🧩 fix: Add allowedProviders to TConfig, remove Record<string, unknown> from PartialEndpointEntry

The agents endpoint config includes allowedProviders (used by the frontend
AgentPanel to filter available providers), but it was missing from TConfig.
This forced PartialEndpointEntry to use & Record<string, unknown> as an
escape hatch, violating AGENTS.md type policy.

- Add allowedProviders?: (string | EModelEndpoint)[] to TConfig
- Remove Record<string, unknown> from PartialEndpointEntry — now just Partial<TConfig>

* 🛡️ fix: Isolate Ollama cache write from fetch try-catch, add Ollama cache tests

- Separate Ollama fetch and cache write into distinct scopes so a cache
  failure (e.g., Redis down) doesn't misattribute the error as an Ollama
  API failure and fall through to the OpenAI-compatible path (Issue A)
- Add 2 Ollama-specific cache tests: models written with TTL on fetch,
  cached models returned without hitting server (Issue B)
- Replace hardcoded 120000 with Time.TWO_MINUTES constant in cache TTL
  test assertion (Issue C)
- Fix OVERRIDE_KEY_MAP JSDoc to accurately describe runtime vs compile-time
  type enforcement (Issue D)
- Add global beforeEach for cache mock reset to prevent cross-test leakage

* 🧪 fix: Address third review — DI consistency, cache key width, MCP tests

- Inject loadCustomEndpointsConfig via EndpointsConfigDeps with default
  fallback, matching loadDefaultEndpointsConfig DI pattern (Finding 3)
- Widen modelsCacheKey from 64-bit (.slice(0,16)) to 128-bit (.slice(0,32))
  for collision-sensitive cross-credential cache key (Finding 4)
- Add fetchModels.mockReset() in loadConfigModels.spec beforeEach to
  prevent mock implementation leaks across tests (Finding 5)
- Add 11 unit tests for createMCPToolCacheService covering all three
  functions: null/empty input, successful ops, error propagation,
  cold-cache merge (Finding 2)
- Simplify getModelsConfig JSDoc to @see reference (Finding 10)

* ♻️ refactor: Address remaining follow-ups from reviews

OVERRIDE_KEY_MAP completeness:
- Add missing turnstile→turnstileConfig mapping
- Add exhaustiveness test verifying all three renamed keys are remapped
  and original YAML keys don't leak through

Import role context:
- Pass userRole through importConversations job → importLibreChatConvo
  so role-based endpoint overrides are honored during conversation import
- Update convos.js route to include req.user.role in the job payload

createEndpointsConfigService unit tests:
- Add 8 tests covering: default+custom merge, Azure/AzureAssistants/
  Anthropic Vertex/Bedrock config enrichment, assistants version
  coercion, agents allowedProviders, req.config bypass

Plugins/tools efficiency:
- Use Set for includedTools/filteredTools lookups (O(1) vs O(n) per plugin)
- Combine auth check + filter into single pass (eliminates intermediate array)
- Pre-compute toolDefKeys Set for O(1) tool definition lookups

* fix: Scope model query cache by user when userIdQuery is enabled

* fix: Skip model cache for userIdQuery endpoints, fix endpoints test types

- When userIdQuery is true, skip caching entirely (like user_provided keys)
  to avoid cross-user model list leakage without duplicating cache data
- Fix AgentCapabilities type error in endpoints.spec.ts — use enum values
  and appConfig() helper for partial mock typing

* 🐛 fix: Restore filteredTools+includedTools composition, add checkCapability tests

- Fix filteredTools regression: whitelist and blacklist are now applied
  independently (two flat guards), matching original behavior where
  includedTools=['a','b'] + filteredTools=['b'] produces ['a'] (Finding A)
- Fix Set spread in toolkit loop: pre-compute toolDefKeysList array once
  alongside the Set, reuse for .some() without per-plugin allocation (Finding B)
- Add 2 filteredTools tests: blacklist-only path and combined
  whitelist+blacklist composition (Finding C)
- Add 3 checkCapability tests: capability present, capability absent,
  fallback to defaultAgentCapabilities for non-agents endpoints (Finding D)

* 🔑 fix: Include config-override MCP servers in filterAuthorizedTools

Config-override MCP servers (defined via admin config overrides for
roles/groups) were rejected by filterAuthorizedTools because it called
getAllServerConfigs(userId) without the configServers parameter. Only
YAML and DB-backed user servers were included in the access check.

- Add configServers parameter to filterAuthorizedTools
- Resolve config servers via resolveConfigServers(req) at all 4 callsites
  (create, update, duplicate, revert) using parallel Promise.all
- Pass configServers through to getAllServerConfigs(userId, configServers)
  so the registry merges config-source servers into the access check
- Update filterAuthorizedTools.spec.js mock for resolveConfigServers

* fix: Skip model cache for userIdQuery endpoints, fix endpoints test types

For user-provided key endpoints (userProvide: true), skip the full model
list re-fetch during message validation — the user already selected from
a list we served them, and re-fetching with skipCache:true on every
message send is both slow and fragile (5s provider timeout = rejected model).

Instead, validate the model string format only:
- Must be a string, max 256 chars
- Must match [a-zA-Z0-9][a-zA-Z0-9_.:\-/@+ ]* (covers all known provider
  model ID formats while rejecting injection attempts)

System-configured endpoints still get full model list validation as before.

* 🧪 test: Add regression tests for filterAuthorizedTools configServers and validateModel

filterAuthorizedTools:
- Add test verifying configServers is passed to getAllServerConfigs and
  config-override server tools are allowed through
- Guard resolveConfigServers in createAgentHandler to only run when
  MCP tools are present (skip for tool-free agent creates)

validateModel (12 new tests):
- Format validation: missing model, non-string, length overflow, leading
  special char, script injection, standard model ID acceptance
- userProvide early-return: next() called immediately, getModelsConfig
  not invoked (regression guard for the exact bug this fixes)
- System endpoint list validation: reject unknown model, accept known
  model, handle null/missing models config

Also fix unnecessary backslash escape in MODEL_PATTERN regex.

* 🧹 fix: Remove space from MODEL_PATTERN, trim input, clean up nits

- Remove space character from MODEL_PATTERN regex — no real model ID
  uses spaces; prevents spurious violation logs from whitespace artifacts
- Add model.trim() before validation to handle accidental whitespace
- Remove redundant filterUniquePlugins call on already-deduplicated output
- Add comment documenting intentional whitelist+blacklist composition
- Add getUserKeyValues.mockReset() in loadConfigModels.spec beforeEach
- Remove narrating JSDoc from getModelsConfig one-liner
- Add 2 tests: trim whitespace handling, reject spaces in model ID

* fix: Match startup tool loader semantics — includedTools takes precedence over filteredTools

The startup tool loader (loadAndFormatTools) explicitly ignores
filteredTools when includedTools is set, with a warning log. The
PluginController was applying both independently, creating inconsistent
behavior where the same config produced different results at startup
vs plugin listing time.

Restored mutually exclusive semantics: when includedTools is non-empty,
filteredTools is not evaluated.

* 🧹 chore: Simplify validateModel flow, note auth requirement on endpoints route

- Separate missing-model from invalid-model checks cleanly: type+presence
  guard first, then trim+format guard (reviewer NIT)
- Add route comment noting auth is required for role/tenant scoping

* fix: Write trimmed model back to req.body.model for downstream consumers
2026-03-30 16:49:48 -04:00
Danny Avila
67db0c1cb3
🗑️ chore: Remove Action Test Suite and Update Mock Implementations (#12268)
- Deleted the Action test suite located in `api/models/Action.spec.js` to streamline the codebase.
- Updated various test files to reflect changes in model mocks, consolidating mock implementations for user-related actions and enhancing clarity.
- Improved consistency in test setups by aligning with the latest model updates and removing redundant mock definitions.
2026-03-21 14:28:55 -04:00
Atef Bellaaj
a0fed6173c
🗂️ refactor: Migrate S3 Storage to TypeScript in packages/api (#11947)
* Migrate S3 storage module with unit and integration tests

  - Migrate S3 CRUD and image operations to packages/api/src/storage/s3/
  - Add S3ImageService class with dependency injection
  - Add unit tests using aws-sdk-client-mock
  - Add integration tests with real s3 bucket (condition presence of  AWS_TEST_BUCKET_NAME)

* AI Review Findings Fixes

* chore: tests and refactor S3 storage types

- Added mock implementations for the 'sharp' library in various test files to improve image processing testing.
- Updated type references in S3 storage files from MongoFile to TFile for consistency and type safety.
- Refactored S3 CRUD operations to ensure proper handling of file types and improve code clarity.
- Enhanced integration tests to validate S3 file operations and error handling more effectively.

* chore: rename test file

* Remove duplicate import of refreshS3Url

* chore: imports order

* fix: remove duplicate imports for S3 URL handling in UserController

* fix: remove duplicate import of refreshS3FileUrls in files.js

* test: Add mock implementations for 'sharp' and '@librechat/api' in UserController tests

- Introduced mock functions for the 'sharp' library to facilitate image processing tests, including metadata retrieval and buffer conversion.
- Enhanced mocking for '@librechat/api' to ensure consistent behavior in tests, particularly for the needsRefresh and getNewS3URL functions.

---------

Co-authored-by: Danny Avila <danny@librechat.ai>
2026-03-21 14:28:55 -04:00
Danny Avila
8ba2bde5c1
📦 refactor: Consolidate DB models, encapsulating Mongoose usage in data-schemas (#11830)
* chore: move database model methods to /packages/data-schemas

* chore: add TypeScript ESLint rule to warn on unused variables

* refactor: model imports to streamline access

- Consolidated model imports across various files to improve code organization and reduce redundancy.
- Updated imports for models such as Assistant, Message, Conversation, and others to a unified import path.
- Adjusted middleware and service files to reflect the new import structure, ensuring functionality remains intact.
- Enhanced test files to align with the new import paths, maintaining test coverage and integrity.

* chore: migrate database models to packages/data-schemas and refactor all direct Mongoose Model usage outside of data-schemas

* test: update agent model mocks in unit tests

- Added `getAgent` mock to `client.test.js` to enhance test coverage for agent-related functionality.
- Removed redundant `getAgent` and `getAgents` mocks from `openai.spec.js` and `responses.unit.spec.js` to streamline test setup and reduce duplication.
- Ensured consistency in agent mock implementations across test files.

* fix: update types in data-schemas

* refactor: enhance type definitions in transaction and spending methods

- Updated type definitions in `checkBalance.ts` to use specific request and response types.
- Refined `spendTokens.ts` to utilize a new `SpendTxData` interface for better clarity and type safety.
- Improved transaction handling in `transaction.ts` by introducing `TransactionResult` and `TxData` interfaces, ensuring consistent data structures across methods.
- Adjusted unit tests in `transaction.spec.ts` to accommodate new type definitions and enhance robustness.

* refactor: streamline model imports and enhance code organization

- Consolidated model imports across various controllers and services to a unified import path, improving code clarity and reducing redundancy.
- Updated multiple files to reflect the new import structure, ensuring all functionalities remain intact.
- Enhanced overall code organization by removing duplicate import statements and optimizing the usage of model methods.

* feat: implement loadAddedAgent and refactor agent loading logic

- Introduced `loadAddedAgent` function to handle loading agents from added conversations, supporting multi-convo parallel execution.
- Created a new `load.ts` file to encapsulate agent loading functionalities, including `loadEphemeralAgent` and `loadAgent`.
- Updated the `index.ts` file to export the new `load` module instead of the deprecated `loadAgent`.
- Enhanced type definitions and improved error handling in the agent loading process.
- Adjusted unit tests to reflect changes in the agent loading structure and ensure comprehensive coverage.

* refactor: enhance balance handling with new update interface

- Introduced `IBalanceUpdate` interface to streamline balance update operations across the codebase.
- Updated `upsertBalanceFields` method signatures in `balance.ts`, `transaction.ts`, and related tests to utilize the new interface for improved type safety.
- Adjusted type imports in `balance.spec.ts` to include `IBalanceUpdate`, ensuring consistency in balance management functionalities.
- Enhanced overall code clarity and maintainability by refining type definitions related to balance operations.

* feat: add unit tests for loadAgent functionality and enhance agent loading logic

- Introduced comprehensive unit tests for the `loadAgent` function, covering various scenarios including null and empty agent IDs, loading of ephemeral agents, and permission checks.
- Enhanced the `initializeClient` function by moving `getConvoFiles` to the correct position in the database method exports, ensuring proper functionality.
- Improved test coverage for agent loading, including handling of non-existent agents and user permissions.

* chore: reorder memory method exports for consistency

- Moved `deleteAllUserMemories` to the correct position in the exported memory methods, ensuring a consistent and logical order of method exports in `memory.ts`.
2026-03-21 14:28:53 -04:00
Danny Avila
58f128bee7
🗑️ chore: Remove Deprecated Project Model and Associated Fields (#11773)
* chore: remove projects and projectIds usage

* chore: empty line linting

* chore: remove isCollaborative property across agent models and related tests

- Removed the isCollaborative property from agent models, controllers, and tests, as it is deprecated in favor of ACL permissions.
- Updated related validation schemas and data provider types to reflect this change.
- Ensured all references to isCollaborative were stripped from the codebase to maintain consistency and clarity.
2026-03-21 14:28:53 -04:00
Danny Avila
a26eeea592
🔏 fix: Enforce MCP Server Authorization on Agent Tool Persistence (#12250)
* 🛡️ fix: Validate MCP tool authorization on agent create/update

Agent creation and update accepted arbitrary MCP tool strings without
verifying the user has access to the referenced MCP servers. This allowed
a user to embed unauthorized server names in tool identifiers (e.g.
"anything_mcp_<victimServer>"), causing mcpServerNames to be stored on
the agent and granting consumeOnly access via hasAccessViaAgent().

Adds filterAuthorizedTools() that checks MCP tool strings against the
user's accessible server configs (via getAllServerConfigs) before
persisting. Applied to create, update, and duplicate agent paths.

* 🛡️ fix: Harden MCP tool authorization and add test coverage

Addresses review findings on the MCP agent tool authorization fix:

- Wrap getMCPServersRegistry() in try/catch so uninitialized registry
  gracefully filters all MCP tools instead of causing a 500 (DoS risk)
- Guard revertAgentVersionHandler: filter unauthorized MCP tools after
  reverting to a previous version snapshot
- Preserve existing MCP tools on collaborative updates: only validate
  newly added tools, preventing silent stripping of tools the editing
  user lacks direct access to
- Add audit logging (logger.warn) when MCP tools are rejected
- Refactor to single-pass lazy-fetch (registry queried only on first
  MCP tool encountered)
- Export filterAuthorizedTools for direct unit testing
- Add 18 tests covering: authorized/unauthorized/mixed tools, registry
  unavailable fallback, create/update/duplicate/revert handler paths,
  collaborative update preservation, and mcpServerNames persistence

* test: Add duplicate handler test, use Constants.mcp_delimiter, DB assertions

- N1: Add duplicateAgentHandler integration test verifying unauthorized
  MCP tools are stripped from the cloned agent and mcpServerNames are
  correctly persisted in the database
- N2: Replace all hardcoded '_mcp_' delimiter literals with
  Constants.mcp_delimiter to prevent silent false-positive tests if
  the delimiter value ever changes
- N3: Add DB state assertion to the revert-with-strip test confirming
  persisted tools match the response after unauthorized tools are
  removed

* fix: Enforce exact 2-segment format for MCP tool keys

Reject MCP tool keys with multiple delimiters to prevent
authorization/execution mismatch when `.pop()` vs `split[1]`
extract different server names from the same key.

* fix: Preserve existing MCP tools when registry is unavailable

When the MCP registry is uninitialized (e.g. server restart), existing
tools already persisted on the agent are preserved instead of silently
stripped. New MCP tools are still rejected when the registry cannot
verify them. Applies to duplicate and revert handlers via existingTools
param; update handler already preserves existing tools via its diff logic.
2026-03-15 20:08:34 -04:00
Danny Avila
bcf45519bd
🪪 fix: Enforce VIEW ACL on Agent Edge References at Write and Runtime (#12246)
* 🛡️ fix: Enforce ACL checks on handoff edge and added-convo agent loading

Edge-linked agents and added-convo agents were fetched by ID via
getAgent without verifying the requesting user's access permissions.
This allowed an authenticated user to reference another user's private
agent in edges or addedConvo and have it initialized at runtime.

Add checkPermission(VIEW) gate in processAgent before initializing
any handoff agent, and in processAddedConvo for non-ephemeral added
agents. Unauthorized agents are logged and added to skippedAgentIds
so orphaned-edge filtering removes them cleanly.

* 🛡️ fix: Validate edge agent access at agent create/update time

Reject agent create/update requests that reference agents in edges
the requesting user cannot VIEW. This provides early feedback and
prevents storing unauthorized agent references as defense-in-depth
alongside the runtime ACL gate in processAgent.

Add collectEdgeAgentIds utility to extract all unique agent IDs from
an edge array, and validateEdgeAgentAccess helper in the v1 handler.

* 🧪 test: Improve ACL gate test coverage and correctness

- Add processAgent ACL gate tests for initializeClient (skip/allow handoff agents)
- Fix addedConvo.spec.js to mock loadAddedAgent directly instead of getAgent
- Seed permMap with ownedAgent VIEW bits in v1.spec.js update-403 test

* 🧹 chore: Remove redundant addedConvo ACL gate (now in middleware)

PR #12243 moved the addedConvo agent ACL check upstream into
canAccessAgentFromBody middleware, making the runtime check in
processAddedConvo and its spec redundant.

* 🧪 test: Rewrite processAgent ACL test with real DB and minimal mocking

Replace heavy mock-based test (12 mocks, Providers.XAI crash) with
MongoMemoryServer-backed integration test that exercises real getAgent,
checkPermission, and AclEntry — only external I/O (initializeAgent,
ToolService, AgentClient) remains mocked. Load edge utilities directly
from packages/api/src/agents/edges to sidestep the config.ts barrel.

* 🧪 fix: Use requireActual spread for @librechat/agents and @librechat/api mocks

The Providers.XAI crash was caused by mocking @librechat/agents with
a minimal replacement object, breaking the @librechat/api initialization
chain. Match the established pattern from client.test.js and
recordCollectedUsage.spec.js: spread jest.requireActual for both
packages, overriding only the functions under test.
2026-03-15 18:08:57 -04:00
Danny Avila
0c27ad2d55
🛡️ refactor: Scope Action Mutations by Parent Resource Ownership (#12237)
* 🛡️ fix: Scope action mutations by parent resource ownership

Prevent cross-tenant action overwrites by validating that an existing
action's agent_id/assistant_id matches the URL parameter before allowing
updates or deletes. Without this, a user with EDIT access on their own
agent could reference a foreign action_id to hijack another agent's
action record.

* 🛡️ fix: Harden action ownership checks and scope write filters

- Remove && short-circuit that bypassed the guard when agent_id or
  assistant_id was falsy (e.g. assistant-owned actions have no agent_id,
  so the check was skipped entirely on the agents route).
- Include agent_id / assistant_id in the updateAction and deleteAction
  query filters so the DB write itself enforces ownership atomically.
- Log a warning when deleteAction returns null (silent no-op from
  data-integrity mismatch).

* 📝 docs: Update Action model JSDoc to reflect scoped query params

*  test: Add Action ownership scoping tests

Cover update, delete, and cross-type protection scenarios using
MongoMemoryServer to verify that scoped query filters (agent_id,
assistant_id) prevent cross-tenant overwrites and deletions at the
database level.

* 🛡️ fix: Scope updateAction filter in agent duplication handler

* 🐛 fix: Use action metadata domain instead of action_id when duplicating agent actions

The duplicate handler was splitting `action.action_id` by `actionDelimiter`
to extract the domain, but `action_id` is a bare nanoid that doesn't
contain the delimiter. This produced malformed entries in the duplicated
agent's actions array (nanoid_action_newNanoid instead of
domain_action_newNanoid). The domain is available on `action.metadata.domain`.

*  test: Add integration tests for agent duplication action handling

Uses MongoMemoryServer with real Agent and Action models to verify:
- Duplicated actions use metadata.domain (not action_id) for the
  agent actions array entries
- Sensitive metadata fields are stripped from duplicated actions
- Original action documents are not modified
2026-03-15 10:19:29 -04:00
Danny Avila
b349f2f876
🪣 fix: Serve Fresh Presigned URLs on Agent List Cache Hits (#11902)
* fix: serve cached presigned URLs on agent list cache hits

  On a cache hit the list endpoint was skipping the S3 refresh and
  returning whatever presigned URL was stored in MongoDB, which could be
  expired if the S3 URL TTL is shorter than the 30-minute cache window.

  refreshListAvatars now collects a urlCache map (agentId -> refreshed
  filepath) alongside its existing stats. The controller stores this map
  in the cache instead of a plain boolean and re-applies it to every
  paginated response, guaranteeing clients always receive a URL that was
  valid as of the last refresh rather than a potentially stale DB value.

* fix: improve avatar refresh cache handling and logging

Updated the avatar refresh logic to validate cached refresh data before proceeding with S3 URL updates. Enhanced logging to exclude sensitive `urlCache` details while still providing relevant statistics. Added error handling for cache invalidation during avatar updates to ensure robustness.

* fix: update avatar refresh logic to clear urlCache on no change

Modified the avatar refresh function to clear the urlCache when no new path is generated, ensuring that stale URLs are not retained. This change improves cache handling and aligns with the updated logic for avatar updates.

* fix: enhance avatar refresh logic to handle legacy cache entries

Updated the avatar refresh logic to accommodate legacy boolean cache entries, ensuring they are treated as cache misses and triggering a refresh. The cache now stores a structured `urlCache` map instead of a boolean, improving cache handling. Added tests to verify correct behavior for cache hits and misses, ensuring clients receive valid URLs based on the latest refresh.
2026-02-22 18:29:31 -05:00
Danny Avila
6279ea8dd7
🛸 feat: Remote Agent Access with External API Support (#11503)
* 🪪 feat: Microsoft Graph Access Token Placeholder for MCP Servers (#10867)

* feat: MCP Graph Token env var

* Addressing copilot remarks

* Addressed Copilot review remarks

* Fixed graphtokenservice mock in MCP test suite

* fix: remove unnecessary type check and cast in resolveGraphTokensInRecord

* ci: add Graph Token integration tests in MCPManager

* refactor: update user type definitions to use Partial<IUser> in multiple functions

* test: enhance MCP tests for graph token processing and user placeholder resolution

- Added comprehensive tests to validate the interaction between preProcessGraphTokens and processMCPEnv.
- Ensured correct resolution of graph tokens and user placeholders in various configurations.
- Mocked OIDC utilities to facilitate testing of token extraction and validation.
- Verified that original options remain unchanged after processing.

* chore: import order

* chore: imports

---------

Co-authored-by: Danny Avila <danny@librechat.ai>

* WIP: OpenAI-compatible API for LibreChat agents

- Added OpenAIChatCompletionController for handling chat completions.
- Introduced ListModelsController and GetModelController for listing and retrieving agent details.
- Created routes for OpenAI API endpoints, including /v1/chat/completions and /v1/models.
- Developed event handlers for streaming responses in OpenAI format.
- Implemented request validation and error handling for API interactions.
- Integrated content aggregation and response formatting to align with OpenAI specifications.

This commit establishes a foundational API for interacting with LibreChat agents in a manner compatible with OpenAI's chat completion interface.

* refactor: OpenAI-spec content aggregation for improved performance and clarity

* fix: OpenAI chat completion controller with safe user handling for correct tool loading

* refactor: Remove conversation ID from OpenAI response context and related handlers

* refactor: OpenAI chat completion handling with streaming support

- Introduced a lightweight tracker for streaming responses, allowing for efficient tracking of emitted content and usage metadata.
- Updated the OpenAIChatCompletionController to utilize the new tracker, improving the handling of streaming and non-streaming responses.
- Refactored event handlers to accommodate the new streaming logic, ensuring proper management of tool calls and content aggregation.
- Adjusted response handling to streamline error reporting during streaming sessions.

* WIP: Open Responses API with core service, types, and handlers

- Added Open Responses API module with comprehensive types and enums.
- Implemented core service for processing requests, including validation and input conversion.
- Developed event handlers for streaming responses and non-streaming aggregation.
- Established response building logic and error handling mechanisms.
- Created detailed types for input and output content, ensuring compliance with Open Responses specification.

* feat: Implement response storage and retrieval in Open Responses API

- Added functionality to save user input messages and assistant responses to the database when the `store` flag is set to true.
- Introduced a new endpoint to retrieve stored responses by ID, allowing users to access previous interactions.
- Enhanced the response creation process to include database operations for conversation and message storage.
- Implemented tests to validate the storage and retrieval of responses, ensuring correct behavior for both existing and non-existent response IDs.

* refactor: Open Responses API with additional token tracking and validation

- Added support for tracking cached tokens in response usage, improving token management.
- Updated response structure to include new properties for top log probabilities and detailed usage metrics.
- Enhanced tests to validate the presence and types of new properties in API responses, ensuring compliance with updated specifications.
- Refactored response handling to accommodate new fields and improve overall clarity and performance.

* refactor: Update reasoning event handlers and types for consistency

- Renamed reasoning text events to simplify naming conventions, changing `emitReasoningTextDelta` to `emitReasoningDelta` and `emitReasoningTextDone` to `emitReasoningDone`.
- Updated event types in the API to reflect the new naming, ensuring consistency across the codebase.
- Added `logprobs` property to output events for enhanced tracking of log probabilities.

* feat: Add validation for streaming events in Open Responses API tests

* feat: Implement response.created event in Open Responses API

- Added emitResponseCreated function to emit the response.created event as the first event in the streaming sequence, adhering to the Open Responses specification.
- Updated createResponse function to emit response.created followed by response.in_progress.
- Enhanced tests to validate the order of emitted events, ensuring response.created is triggered before response.in_progress.

* feat: Responses API with attachment event handling

- Introduced `createResponsesToolEndCallback` to handle attachment events in the Responses API, emitting `librechat:attachment` events as per the Open Responses extension specification.
- Updated the `createResponse` function to utilize the new callback for processing tool outputs and emitting attachments during streaming.
- Added helper functions for writing attachment events and defined types for attachment data, ensuring compatibility with the Open Responses protocol.
- Enhanced tests to validate the integration of attachment events within the Responses API workflow.

* WIP: remote agent auth

* fix: Improve loading state handling in AgentApiKeys component

- Updated the rendering logic to conditionally display loading spinner and API keys based on the loading state.
- Removed unnecessary imports and streamlined the component for better readability.

* refactor: Update API key access handling in routes

- Replaced `checkAccess` with `generateCheckAccess` for improved access control.
- Consolidated access checks into a single `checkApiKeyAccess` function, enhancing code readability and maintainability.
- Streamlined route definitions for creating, listing, retrieving, and deleting API keys.

* fix: Add permission handling for REMOTE_AGENT resource type

* feat: Enhance permission handling for REMOTE_AGENT resources

- Updated the deleteAgent and deleteUserAgents functions to handle permissions for both AGENT and REMOTE_AGENT resource types.
- Introduced new functions to enrich REMOTE_AGENT principals and backfill permissions for AGENT owners.
- Modified createAgentHandler and duplicateAgentHandler to grant permissions for REMOTE_AGENT alongside AGENT.
- Added utility functions for retrieving effective permissions for REMOTE_AGENT resources, ensuring consistent access control across the application.

* refactor: Rename and update roles for remote agent access

- Changed role name from API User to Editor in translation files for clarity.
- Updated default editor role ID from REMOTE_AGENT_USER to REMOTE_AGENT_EDITOR in resource configurations.
- Adjusted role localization to reflect the new Editor role.
- Modified access permissions to align with the updated role definitions across the application.

* feat: Introduce remote agent permissions and update access handling

- Added support for REMOTE_AGENTS in permission schemas, including use, create, share, and share_public permissions.
- Updated the interface configuration to include remote agent settings.
- Modified middleware and API key access checks to align with the new remote agent permission structure.
- Enhanced role defaults to incorporate remote agent permissions, ensuring consistent access control across the application.

* refactor: Update AgentApiKeys component and permissions handling

- Refactored the AgentApiKeys component to improve structure and readability, including the introduction of ApiKeysContent for better separation of concerns.
- Updated CreateKeyDialog to accept an onKeyCreated callback, enhancing its functionality.
- Adjusted permission checks in Data component to use REMOTE_AGENTS and USE permissions, aligning with recent permission schema changes.
- Enhanced loading state handling and dialog management for a smoother user experience.

* refactor: Update remote agent access checks in API routes

- Replaced existing access checks with `generateCheckAccess` for remote agents in the API keys and agents routes.
- Introduced specific permission checks for creating, listing, retrieving, and deleting API keys, enhancing access control.
- Improved code structure by consolidating permission handling for remote agents across multiple routes.

* fix: Correct query parameters in ApiKeysContent component

- Updated the useGetAgentApiKeysQuery call to include an object for the enabled parameter, ensuring proper functionality when the component is open.
- This change improves the handling of API key retrieval based on the component's open state.

* feat: Implement remote agents permissions and update API routes

- Added new API route for updating remote agents permissions, enhancing role management capabilities.
- Introduced remote agents permissions handling in the AgentApiKeys component, including a dedicated settings dialog.
- Updated localization files to include new remote agents permission labels for better user experience.
- Refactored data provider to support remote agents permissions updates, ensuring consistent access control across the application.

* feat: Add remote agents permissions to role schema and interface

- Introduced new permissions for REMOTE_AGENTS in the role schema, including USE, CREATE, SHARE, and SHARE_PUBLIC.
- Updated the IRole interface to reflect the new remote agents permissions structure, enhancing role management capabilities.

* feat: Add remote agents settings button to API keys dialog

* feat: Update AgentFooter to include remote agent sharing permissions

- Refactored access checks to incorporate permissions for sharing remote agents.
- Enhanced conditional rendering logic to allow sharing by users with remote agent permissions.
- Improved loading state handling for remote agent permissions, ensuring a smoother user experience.

* refactor: Update API key creation access check and localization strings

- Replaced the access check for creating API keys to use the existing remote agents access check.
- Updated localization strings to correct the descriptions for remote agent permissions, ensuring clarity in user interface.

* fix: resource permission mapping to include remote agents

- Changed the resourceToPermissionMap to use a Partial<Record> for better flexibility.
- Added mapping for REMOTE_AGENT permissions, enhancing the sharing capabilities for remote agents.

* feat: Implement remote access checks for agent models

- Enhanced ListModelsController and GetModelController to include checks for user permissions on remote agents.
- Integrated findAccessibleResources to filter agents based on VIEW permission for REMOTE_AGENT.
- Updated response handling to ensure users can only access agents they have permissions for, improving security and access control.

* fix: Update user parameter type in processUserPlaceholders function

- Changed the user parameter type in the processUserPlaceholders function from Partial<Partial<IUser>> to Partial<IUser> for improved type clarity and consistency.

* refactor: Simplify integration test structure by removing conditional describe

- Replaced conditional describeWithApiKey with a standard describe for all integration tests in responses.spec.js.
- This change enhances test clarity and ensures all tests are executed consistently, regardless of the SKIP_INTEGRATION_TESTS flag.

* test: Update AgentFooter tests to reflect new grant access dialog ID

- Changed test IDs for the grant access dialog in AgentFooter tests to include the resource type, ensuring accurate identification in the test cases.
- This update improves test clarity and aligns with recent changes in the component's implementation.

* test: Enhance integration tests for Open Responses API

- Updated integration tests in responses.spec.js to utilize an authRequest helper for consistent authorization handling across all test cases.
- Introduced a test user and API key creation to improve test setup and ensure proper permission checks for remote agents.
- Added checks for existing access roles and created necessary roles if they do not exist, enhancing test reliability and coverage.

* feat: Extend accessRole schema to include remoteAgent resource type

- Updated the accessRole schema to add 'remoteAgent' to the resourceType enum, enhancing the flexibility of role assignments and permissions management.

* test: refactored test setup to create a minimal Express app for responses routes, enhancing test structure and maintainability.

* test: Enhance abort.spec.js by mocking additional modules for improved test isolation

- Updated the test setup in abort.spec.js to include actual implementations of '@librechat/data-schemas' and '@librechat/api' while maintaining mock functionality.
- This change improves test reliability and ensures that the tests are more representative of the actual module behavior.

* refactor: Update conversation ID generation to use UUID

- Replaced the nanoid with uuidv4 for generating conversation IDs in the createResponse function, enhancing uniqueness and consistency in ID generation.

* test: Add remote agent access roles to AccessRole model tests

- Included additional access roles for remote agents (REMOTE_AGENT_EDITOR, REMOTE_AGENT_OWNER, REMOTE_AGENT_VIEWER) in the AccessRole model tests to ensure comprehensive coverage of role assignments and permissions management.

* chore: Add deletion of user agent API keys in user deletion process

- Updated the user deletion process in UserController and delete-user.js to include the removal of user agent API keys, ensuring comprehensive cleanup of user data upon account deletion.

* test: Add remote agents permissions to permissions.spec.ts

- Enhanced the permissions tests by including comprehensive permission settings for remote agents across various scenarios, ensuring accurate validation of access controls for remote agent roles.

* chore: Update remote agents translations for clarity and consistency

- Removed outdated remote agents translation entries and added revised entries to improve clarity on API key creation and sharing permissions for remote agents. This enhances user understanding of the available functionalities.

* feat: Add indexing and TTL for agent API keys

- Introduced an index on the `key` field for improved query performance.
- Added a TTL index on the `expiresAt` field to enable automatic cleanup of expired API keys, ensuring efficient management of stored keys.

* chore: Update API route documentation for clarity

- Revised comments in the agents route file to clarify the handling of API key authentication.
- Removed outdated endpoint listings to streamline the documentation and focus on current functionality.

---------

Co-authored-by: Max Sanna <max@maxsanna.com>
2026-01-28 17:44:33 -05:00
David Newman
a95fea19bb
🌅 fix: Agent Avatar S3 URL Refresh Pagination and Persistence (#11323)
* Refresh all S3 avatars for this user's accessible agent set, not the first page

* Cleaner debug messages

* Log errors as errors

* refactor: avatar refresh logic to process agents in batches and improve error handling. Introduced new utility functions for refreshing S3 avatars and updating agent records. Updated tests to cover various scenarios including cache hits, user ownership checks, and error handling. Added constants for maximum refresh limits.

* refactor: update avatar refresh logic to allow users with VIEW access to refresh avatars for all accessible agents. Removed checks for agent ownership and author presence, and updated related tests to reflect new behavior.

* chore: Remove YouTube toolkit due to #11331

---------

Co-authored-by: Danny Avila <danny@librechat.ai>
2026-01-13 13:01:11 -05:00
Danny Avila
cdffdd2926
🏞️ fix: Gemini Image Filenames and Add Tool Cache Safety (#11306)
* 🔧 fix: Handle undefined tool definitions in agent and assistant creation (#11295)

* Updated the tool fetching logic in createAgentHandler, createAssistant, and patchAssistant functions to use nullish coalescing, ensuring that an empty object is returned if no tools are available. This change improves robustness against undefined values in tool definitions across multiple controller files.

* Adjusted the ToolService to maintain consistency in tool definition handling.

* 🔧 fix: Update filename generation in createToolEndCallback function

* Modified the filename generation logic to remove the tool_call_id from the filename, simplifying the naming convention for saved images. This change enhances clarity and consistency in the generated filenames.
2026-01-12 09:01:23 -05:00
Danny Avila
24e8a258cd
🔧 fix: Clean empty strings from model_parameters for Agents/OpenAI (#11248) 2026-01-07 11:26:53 -05:00
Danny Avila
04a4a2aa44
🧵 refactor: Migrate Endpoint Initialization to TypeScript (#10794)
* refactor: move endpoint initialization methods to typescript

* refactor: move agent init to packages/api

- Introduced `initialize.ts` for agent initialization, including file processing and tool loading.
- Updated `resources.ts` to allow optional appConfig parameter.
- Enhanced endpoint configuration handling in various initialization files to support model parameters.
- Added new artifacts and prompts for React component generation.
- Refactored existing code to improve type safety and maintainability.

* refactor: streamline endpoint initialization and enhance type safety

- Updated initialization functions across various endpoints to use a consistent request structure, replacing `unknown` types with `ServerResponse`.
- Simplified request handling by directly extracting keys from the request body.
- Improved type safety by ensuring user IDs are safely accessed with optional chaining.
- Removed unnecessary parameters and streamlined model options handling for better clarity and maintainability.

* refactor: moved ModelService and extractBaseURL to packages/api

- Added comprehensive tests for the models fetching functionality, covering scenarios for OpenAI, Anthropic, Google, and Ollama models.
- Updated existing endpoint index to include the new models module.
- Enhanced utility functions for URL extraction and model data processing.
- Improved type safety and error handling across the models fetching logic.

* refactor: consolidate utility functions and remove unused files

- Merged `deriveBaseURL` and `extractBaseURL` into the `@librechat/api` module for better organization.
- Removed redundant utility files and their associated tests to streamline the codebase.
- Updated imports across various client files to utilize the new consolidated functions.
- Enhanced overall maintainability by reducing the number of utility modules.

* refactor: replace ModelService references with direct imports from @librechat/api and remove ModelService file

* refactor: move encrypt/decrypt methods and key db methods to data-schemas, use `getProviderConfig` from `@librechat/api`

* chore: remove unused 'res' from options in AgentClient

* refactor: file model imports and methods

- Updated imports in various controllers and services to use the unified file model from '~/models' instead of '~/models/File'.
- Consolidated file-related methods into a new file methods module in the data-schemas package.
- Added comprehensive tests for file methods including creation, retrieval, updating, and deletion.
- Enhanced the initializeAgent function to accept dependency injection for file-related methods.
- Improved error handling and logging in file methods.

* refactor: streamline database method references in agent initialization

* refactor: enhance file method tests and update type references to IMongoFile

* refactor: consolidate database method imports in agent client and initialization

* chore: remove redundant import of initializeAgent from @librechat/api

* refactor: move checkUserKeyExpiry utility to @librechat/api and update references across endpoints

* refactor: move updateUserPlugins logic to user.ts and simplify UserController

* refactor: update imports for user key management and remove UserService

* refactor: remove unused Anthropics and Bedrock endpoint files and clean up imports

* refactor: consolidate and update encryption imports across various files to use @librechat/data-schemas

* chore: update file model mock to use unified import from '~/models'

* chore: import order

* refactor: remove migrated to TS agent.js file and its associated logic from the endpoints

* chore: add reusable function to extract imports from source code in unused-packages workflow

* chore: enhance unused-packages workflow to include @librechat/api dependencies and improve dependency extraction

* chore: improve dependency extraction in unused-packages workflow with enhanced error handling and debugging output

* chore: add detailed debugging output to unused-packages workflow for better visibility into unused dependencies and exclusion lists

* chore: refine subpath handling in unused-packages workflow to correctly process scoped and non-scoped package imports

* chore: clean up unused debug output in unused-packages workflow and reorganize type imports in initialize.ts
2025-12-11 16:37:16 -05:00
Marco Beretta
8907bd5d7c
👤 feat: Agent Avatar Removal and Decouple upload/reset from Agent Updates (#10527)
*  feat: Enhance agent avatar management with upload and reset functionality

*  feat: Refactor AvatarMenu to use DropdownPopup for improved UI and functionality

*  feat: Improve avatar upload handling in AgentPanel to suppress misleading "no changes" toast

*  feat: Refactor toast message handling and payload composition in AgentPanel for improved clarity and functionality

*  feat: Enhance agent avatar functionality with upload, reset, and validation improvements

*  feat: Refactor agent avatar upload handling and enhance related components for improved functionality and user experience

* feat(agents): tighten ACL, harden GETs/search, and sanitize action metadata
stop persisting refreshed S3 URLs on GET; compute per-response only
enforce ACL EDIT on revert route; remove legacy admin/author/collab checks
sanitize action metadata before persisting during duplication (api_key, oauth_client_id, oauth_client_secret)
escape user search input, cap length (100), and use Set for public flag mapping
add explicit req.file guard in avatar upload; fix empty catch lint; remove unused imports

* feat: Remove outdated avatar-related translation keys

* feat: Improve error logging for avatar updates and streamline file input handling

* feat(agents): implement caching for S3 avatar refresh in agent list responses

* fix: replace unconventional 'void e' with explicit comment to clarify intentionally ignored error

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* feat(agents): enhance avatar handling and improve search functionality

* fix: clarify intentionally ignored error in agent list handler

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
2025-11-17 17:04:01 -05:00
Danny Avila
5b1a31ef4d
🔄 refactor: Optimize MCP Tool Initialization
🔄 refactor: Optimize MCP Tool Initialization

fix: update tool caching to use separated mcp logic

refactor: Replace `req.user` with `userId` in MCP handling functions

refactor: Replace `req` parameter with `userId` in file search tool functions

fix: Update user connection parameter to use object format in reinitMCPServer

refactor: Simplify MCP tool creation logic and improve handling of tool configurations to avoid capturing too much in closures

refactor: ensure MCP available tools are fetched from cache only when needed
2025-09-21 20:31:28 -04:00
Danny Avila
81139046e5
🔄 refactor: Convert OCR Tool Resource to Context (#9699)
* WIP: conversion of `ocr` to `context`

* refactor: make `primeResources` backwards-compatible for `ocr` tool_resources

* refactor: Convert legacy `ocr` tool resource to `context` in agent updates

- Implemented conversion logic to replace `ocr` with `context` in both incoming updates and existing agent data.
- Merged file IDs and files from `ocr` into `context` while ensuring deduplication.
- Updated tools array to reflect the change from `ocr` to `context`.

* refactor: Enhance context file handling in agent processing

- Updated the logic for managing context files by consolidating file IDs from both `ocr` and `context` resources.
- Improved backwards compatibility by ensuring that context files are correctly populated and handled.
- Simplified the iteration over context files for better readability and maintainability.

* refactor: Enhance tool_resources handling in primeResources

- Added tests to verify the deletion behavior of tool_resources fields, ensuring original objects remain unchanged.
- Implemented logic to delete `ocr` and `context` fields after fetching and re-categorizing files.
- Preserved context field when the context capability is disabled, ensuring correct behavior in various scenarios.

* refactor: Replace `ocrEnabled` with `contextEnabled` in AgentConfig

* refactor: Adjust legacy tool handling order for improved clarity

* refactor: Implement OCR to context conversion functions and remove original conversion logic in update agent handling

* refactor: Move contextEnabled declaration to maintain consistent order in capabilities

* refactor: Update localization keys for file context to improve clarity and accuracy

* chore: Update localization key for file context information to improve clarity
2025-09-18 20:06:59 -04:00
Dustin Healy
49e8443ec5
✂️ refactor: MCP UI Separation for Agents (#9237)
* refactor: MCP UI Separation for Agents (Dustin WIP)

feat: separate MCPs into their own lists away from tools + actions and add the status indicator functionality from chat to their dropdown ui

fix: spotify mcp was not persisting on agent creation

feat: show disconnected saved servers and their tools in agent mcp list in created agents

fix: select-all regression fixed (caused by deleting tools we were drawing from for rendering list)

fix: dont show all mcps, only those installed in agent in list

feat: separate ToolSelectDialog for MCPServerTools

fix: uninitialized mcp servers not showing as added in toolselectdialog

refactor: reduce looping in AgentPanelContext for categorizing groups and mcps

refactor: split ToolSelectDialog and MCPToolSelectDialog functionality (still needs customization for custom user vars)

chore: address ESLint comments

chore: address ESLint comments

feat: one-click initialization on MCP servers in agent builder

fix: stop propagation triggering reinit on caret click

refactor: split uninitialized MCPs component from initialized MCPs

feat: new mcp tool select dialog ui with custom user vars

feat: show initialization state for CUV configurable MCPs too

chore: remove unused localization string

fix: deselecting all tools caused a re-render

fix: remove subtools so removal from MCPToolSelectDialog works more consistently

feat: added servers have all tools enabled by default

feat: mcp server list now alphabetical to prevent annoying ui behavior of servers jumping around depending on tool selection

fix: filter out placeholder group mcp tools from any actual tool calls / definitions

feat: indicator now takes you to config dialog for uninitialized servers

feat: show previously configured mcp servers that are now missing from the yaml

feat: select all enabled by default on first add to mcp server list

chore: address ESLint comments

* refactor: MCP UI Separation for Agents (Danny WIP)

chore: remove use of `{serverName}_mcp_{serverName}`

chore: import order

WIP: separate component concerns

refactor: streamline agent mcp tools

refactor: unify MCP server handling and improve tool visibility logic, remove unnecessary normalization or sorting, remove nesting button, make variable names clear

refactor: rename mcpServerIds to mcpServerNames for clarity and consistency across components

refactor: remove groupedMCPTools and toolToServerMap, streamline MCP server handling in context and components to effectively utilize mcpServersMap

refactor: optimize tool selection logic by replacing array includes with Set for improved performance

chore: add error logging for failed auth URL parsing in ToolCall component

refactor: enhance MCP tool handling by improving server name management and updating UI elements for better clarity

* refactor: decouple connection status from useMCPServerManager with useMCPConnectionStatus

* fix: improve MCP tool validation logic to handle unconfigured servers

* chore: enhance log message clarity for MCP server disconnection in updateUserPluginsController

* refactor: simplify connection status extraction in useMCPConnectionStatus hook

* refactor: improve initializing UX

* chore: replace string literal with ResourceType constant in useResourcePermissions

* refactor: cleanup code, remove redundancies, rename variables for clarity

* chore: add back filtering and sorting for mcp tools dialog

* refactor: initializeServer to return response and early return

* refactor: enhance server initialization logic and improve UI for OAuth interaction

* chore: clarify warning message for unconfigured MCP server in handleTools

* refactor: prevent CustomUserVarsSection from submitting tools dialog form

* fix: nested button of button issue in UninitializedMCPTool

* feat: add functionality to revoke custom user variables in MCPToolSelectDialog

---------

Co-authored-by: Danny Avila <danny@librechat.ai>
2025-08-29 22:57:01 -04:00
Danny Avila
9a210971f5
🛜 refactor: Streamline App Config Usage (#9234)
* WIP: app.locals refactoring

WIP: appConfig

fix: update memory configuration retrieval to use getAppConfig based on user role

fix: update comment for AppConfig interface to clarify purpose

🏷️ refactor: Update tests to use getAppConfig for endpoint configurations

ci: Update AppService tests to initialize app config instead of app.locals

ci: Integrate getAppConfig into remaining tests

refactor: Update multer storage destination to use promise-based getAppConfig and improve error handling in tests

refactor: Rename initializeAppConfig to setAppConfig and update related tests

ci: Mock getAppConfig in various tests to provide default configurations

refactor: Update convertMCPToolsToPlugins to use mcpManager for server configuration and adjust related tests

chore: rename `Config/getAppConfig` -> `Config/app`

fix: streamline OpenAI image tools configuration by removing direct appConfig dependency and using function parameters

chore: correct parameter documentation for imageOutputType in ToolService.js

refactor: remove `getCustomConfig` dependency in config route

refactor: update domain validation to use appConfig for allowed domains

refactor: use appConfig registration property

chore: remove app parameter from AppService invocation

refactor: update AppConfig interface to correct registration and turnstile configurations

refactor: remove getCustomConfig dependency and use getAppConfig in PluginController, multer, and MCP services

refactor: replace getCustomConfig with getAppConfig in STTService, TTSService, and related files

refactor: replace getCustomConfig with getAppConfig in Conversation and Message models, update tempChatRetention functions to use AppConfig type

refactor: update getAppConfig calls in Conversation and Message models to include user role for temporary chat expiration

ci: update related tests

refactor: update getAppConfig call in getCustomConfigSpeech to include user role

fix: update appConfig usage to access allowedDomains from actions instead of registration

refactor: enhance AppConfig to include fileStrategies and update related file strategy logic

refactor: update imports to use normalizeEndpointName from @librechat/api and remove redundant definitions

chore: remove deprecated unused RunManager

refactor: get balance config primarily from appConfig

refactor: remove customConfig dependency for appConfig and streamline loadConfigModels logic

refactor: remove getCustomConfig usage and use app config in file citations

refactor: consolidate endpoint loading logic into loadEndpoints function

refactor: update appConfig access to use endpoints structure across various services

refactor: implement custom endpoints configuration and streamline endpoint loading logic

refactor: update getAppConfig call to include user role parameter

refactor: streamline endpoint configuration and enhance appConfig usage across services

refactor: replace getMCPAuthMap with getUserMCPAuthMap and remove unused getCustomConfig file

refactor: add type annotation for loadedEndpoints in loadEndpoints function

refactor: move /services/Files/images/parse to TS API

chore: add missing FILE_CITATIONS permission to IRole interface

refactor: restructure toolkits to TS API

refactor: separate manifest logic into its own module

refactor: consolidate tool loading logic into a new tools module for startup logic

refactor: move interface config logic to TS API

refactor: migrate checkEmailConfig to TypeScript and update imports

refactor: add FunctionTool interface and availableTools to AppConfig

refactor: decouple caching and DB operations from AppService, make part of consolidated `getAppConfig`

WIP: fix tests

* fix: rebase conflicts

* refactor: remove app.locals references

* refactor: replace getBalanceConfig with getAppConfig in various strategies and middleware

* refactor: replace appConfig?.balance with getBalanceConfig in various controllers and clients

* test: add balance configuration to titleConvo method in AgentClient tests

* chore: remove unused `openai-chat-tokens` package

* chore: remove unused imports in initializeMCPs.js

* refactor: update balance configuration to use getAppConfig instead of getBalanceConfig

* refactor: integrate configMiddleware for centralized configuration handling

* refactor: optimize email domain validation by removing unnecessary async calls

* refactor: simplify multer storage configuration by removing async calls

* refactor: reorder imports for better readability in user.js

* refactor: replace getAppConfig calls with req.config for improved performance

* chore: replace getAppConfig calls with req.config in tests for centralized configuration handling

* chore: remove unused override config

* refactor: add configMiddleware to endpoint route and replace getAppConfig with req.config

* chore: remove customConfig parameter from TTSService constructor

* refactor: pass appConfig from request to processFileCitations for improved configuration handling

* refactor: remove configMiddleware from endpoint route and retrieve appConfig directly in getEndpointsConfig if not in `req.config`

* test: add mockAppConfig to processFileCitations tests for improved configuration handling

* fix: pass req.config to hasCustomUserVars and call without await after synchronous refactor

* fix: type safety in useExportConversation

* refactor: retrieve appConfig using getAppConfig in PluginController and remove configMiddleware from plugins route, to avoid always retrieving when plugins are cached

* chore: change `MongoUser` typedef to `IUser`

* fix: Add `user` and `config` fields to ServerRequest and update JSDoc type annotations from Express.Request to ServerRequest

* fix: remove unused setAppConfig mock from Server configuration tests
2025-08-26 12:10:18 -04:00
Danny Avila
39346d6b8e
🛂 feat: Role as Permission Principal Type
WIP: Role as Permission Principal Type

WIP: add user role check optimization to user principal check, update type comparisons

WIP: cover edge cases for string vs ObjectId handling in permission granting and checking

chore: Update people picker access middleware to use PrincipalType constants

feat: Enhance people picker access control to include roles permissions

chore: add missing default role schema values for people picker perms, cleanup typing

feat: Enhance PeoplePicker component with role-specific UI and localization updates

chore: Add missing `VIEW_ROLES` permission to role schema
2025-08-13 16:24:23 -04:00
Danny Avila
49d1cefe71
🔧 refactor: Add and use PrincipalType Enum
- Replaced string literals for principal types ('user', 'group', 'public') with the new PrincipalType enum across various models, services, and tests for improved type safety and consistency.
- Updated permission handling in multiple files to utilize the PrincipalType enum, enhancing maintainability and reducing potential errors.
- Ensured all relevant tests reflect these changes to maintain coverage and functionality.
2025-08-13 16:24:22 -04:00
Danny Avila
81b32e400a
🔧 refactor: Organize Sharing/Agent Components and Improve Type Safety
refactor: organize Sharing/Agent components, improve type safety for resource types and access role ids, rename enums to PascalCase

refactor: organize Sharing/Agent components, improve type safety for resource types and access role ids

chore: move sharing related components to dedicated "Sharing" directory

chore: remove PublicSharingToggle component and update index exports

chore: move non-sidepanel agent components to `~/components/Agents`

chore: move AgentCategoryDisplay component with tests

chore: remove commented out code

refactor: change PERMISSION_BITS from const to enum for better type safety

refactor: reorganize imports in GenericGrantAccessDialog and update index exports for hooks

refactor: update type definitions to use ACCESS_ROLE_IDS for improved type safety

refactor: remove unused canAccessPromptResource middleware and related code

refactor: remove unused prompt access roles from createAccessRoleMethods

refactor: update resourceType in AclEntry type definition to remove unused 'prompt' value

refactor: introduce ResourceType enum and update resourceType usage across data provider files for improved type safety

refactor: update resourceType usage to ResourceType enum across sharing and permissions components for improved type safety

refactor: standardize resourceType usage to ResourceType enum across agent and prompt models, permissions controller, and middleware for enhanced type safety

refactor: update resourceType references from PROMPT_GROUP to PROMPTGROUP for consistency across models, middleware, and components

refactor: standardize access role IDs and resource type usage across agent, file, and prompt models for improved type safety and consistency

chore: add typedefs for TUpdateResourcePermissionsRequest and TUpdateResourcePermissionsResponse to enhance type definitions

chore: move SearchPicker to PeoplePicker dir

refactor: implement debouncing for query changes in SearchPicker for improved performance

chore: fix typing, import order for agent admin settings

fix: agent admin settings, prevent agent form submission

refactor: rename `ACCESS_ROLE_IDS` to `AccessRoleIds`

refactor: replace PermissionBits with PERMISSION_BITS

refactor: replace PERMISSION_BITS with PermissionBits
2025-08-13 16:24:20 -04:00
“Praneeth
ff54cbffd9
🔒 feat: Implement Granular File Storage Strategies and Access Control Middleware 2025-08-13 16:24:19 -04:00
“Praneeth
949682ef0f
🏪 feat: Agent Marketplace
bugfix: Enhance Agent and AgentCategory schemas with new fields for category, support contact, and promotion status

refactored and moved agent category methods and schema to data-schema package

🔧 fix: Merge and Rebase Conflicts

- Move AgentCategory from api/models to @packages/data-schemas structure
  - Add schema, types, methods, and model following codebase conventions
  - Implement auto-seeding of default categories during AppService startup
  - Update marketplace controller to use new data-schemas methods
  - Remove old model file and standalone seed script

refactor: unify agent marketplace to single endpoint with cursor pagination

  - Replace multiple marketplace routes with unified /marketplace endpoint
  - Add query string controls: category, search, limit, cursor, promoted, requiredPermission
  - Implement cursor-based pagination replacing page-based system
  - Integrate ACL permissions for proper access control
  - Fix ObjectId constructor error in Agent model
  - Update React components to use unified useGetMarketplaceAgentsQuery hook
  - Enhance type safety and remove deprecated useDynamicAgentQuery
  - Update tests for new marketplace architecture
  -Known issues:
  see more button after category switching + Unit tests

feat: add icon property to ProcessedAgentCategory interface

- Add useMarketplaceAgentsInfiniteQuery and useGetAgentCategoriesQuery to client/src/data-provider/Agents/
  - Replace manual pagination in AgentGrid with infinite query pattern
  - Update imports to use local data provider instead of librechat-data-provider
  - Add proper permission handling with PERMISSION_BITS.VIEW/EDIT constants
  - Improve agent access control by adding requiredPermission validation in backend
  - Remove manual cursor/state management in favor of infinite query built-ins
  - Maintain existing search and category filtering functionality

refactor: consolidate agent marketplace endpoints into main agents API and improve data management consistency

  - Remove dedicated marketplace controller and routes, merging functionality into main agents v1 API
  - Add countPromotedAgents function to Agent model for promoted agents count
  - Enhance getListAgents handler with marketplace filtering (category, search, promoted status)
  - Move getAgentCategories from marketplace to v1 controller with same functionality
  - Update agent mutations to invalidate marketplace queries and handle multiple permission levels
  - Improve cache management by updating all agent query variants (VIEW/EDIT permissions)
  - Consolidate agent data access patterns for better maintainability and consistency
  - Remove duplicate marketplace route definitions and middleware

selected view only agents injected in the drop down

fix: remove minlength validation for support contact name in agent schema

feat: add validation and error messages for agent name in AgentConfig and AgentPanel

fix: update agent permission check logic in AgentPanel to simplify condition

Fix linting WIP

Fix Unit tests WIP

ESLint fixes

eslint fix

refactor: enhance isDuplicateVersion function in Agent model for improved comparison logic

- Introduced handling for undefined/null values in array and object comparisons.
- Normalized array comparisons to treat undefined/null as empty arrays.
- Added deep comparison for objects and improved handling of primitive values.
- Enhanced projectIds comparison to ensure consistent MongoDB ObjectId handling.

refactor: remove redundant properties from IAgent interface in agent schema

chore: update localization for agent detail component and clean up imports

ci: update access middleware tests

chore: remove unused PermissionTypes import from Role model

ci: update AclEntry model tests

ci: update button accessibility labels in AgentDetail tests

refactor: update exhaustive dep. lint warning

🔧 fix: Fixed agent actions access

feat: Add role-level permissions for agent sharing people picker

  - Add PEOPLE_PICKER permission type with VIEW_USERS and VIEW_GROUPS permissions
  - Create custom middleware for query-aware permission validation
  - Implement permission-based type filtering in PeoplePicker component
  - Hide people picker UI when user lacks permissions, show only public toggle
  - Support granular access: users-only, groups-only, or mixed search modes

refactor: Replace marketplace interface config with permission-based system

  - Add MARKETPLACE permission type to handle marketplace access control
  - Update interface configuration to use role-based marketplace settings (admin/user)
  - Replace direct marketplace boolean config with permission-based checks
  - Modify frontend components to use marketplace permissions instead of interface config
  - Update agent query hooks to use marketplace permissions for determining permission levels
  - Add marketplace configuration structure similar to peoplePicker in YAML config
  - Backend now sets MARKETPLACE permissions based on interface configuration
  - When marketplace enabled: users get agents with EDIT permissions in dropdown lists  (builder mode)
  - When marketplace disabled: users get agents with VIEW permissions  in dropdown lists (browse mode)

🔧 fix: Redirect to New Chat if No Marketplace Access and Required Agent Name Placeholder (#8213)

* Fix: Fix the redirect to new chat page if access to marketplace is denied

* Fixed the required agent name placeholder

---------

Co-authored-by: Atef Bellaaj <slalom.bellaaj@external.daimlertruck.com>

chore: fix tests, remove unnecessary imports

refactor: Implement permission checks for file access via agents

- Updated `hasAccessToFilesViaAgent` to utilize permission checks for VIEW and EDIT access.
- Replaced project-based access validation with permission-based checks.
- Enhanced tests to cover new permission logic and ensure proper access control for files associated with agents.
- Cleaned up imports and initialized models in test files for consistency.

refactor: Enhance test setup and cleanup for file access control

- Introduced modelsToCleanup array to track models added during tests for proper cleanup.
- Updated afterAll hooks in test files to ensure all collections are cleared and only added models are deleted.
- Improved consistency in model initialization across test files.
- Added comments for clarity on cleanup processes and test data management.

chore: Update Jest configuration and test setup for improved timeout handling

- Added a global test timeout of 30 seconds in jest.config.js.
- Configured jest.setTimeout in jestSetup.js to allow individual test overrides if needed.
- Enhanced test reliability by ensuring consistent timeout settings across all tests.

refactor: Implement file access filtering based on agent permissions

- Introduced `filterFilesByAgentAccess` function to filter files based on user access through agents.
- Updated `getFiles` and `primeFiles` functions to utilize the new filtering logic.
- Moved `hasAccessToFilesViaAgent` function from the File model to permission services, adjusting imports accordingly
- Enhanced tests to ensure proper access control and filtering behavior for files associated with agents.

fix: make support_contact field a nested object rather than a sub-document

refactor: Update support_contact field initialization in agent model

- Removed handling for empty support_contact object in createAgent function.
- Changed default value of support_contact in agent schema to undefined.

test: Add comprehensive tests for support_contact field handling and versioning

refactor: remove unused avatar upload mutation field and add informational toast for success

chore: add missing SidePanelProvider for AgentMarketplace and organize imports

fix: resolve agent selection race condition in marketplace HandleStartChat
- Set agent in localStorage before newConversation to prevent useSelectorEffects from auto-selecting previous agent

fix: resolve agent dropdown showing raw ID instead of agent info from URL

  - Add proactive agent fetching when agent_id is present in URL parameters
  - Inject fetched agent into agents cache so dropdowns display proper name/avatar
  - Use useAgentsMap dependency to ensure proper cache initialization timing
  - Prevents raw agent IDs from showing in UI when visiting shared agent links

Fix: Agents endpoint renamed to "My Agent" for less confusion with the Marketplace agents.

chore: fix ESLint issues and Test Mocks

ci: update permissions structure in loadDefaultInterface tests

- Refactored permissions for MEMORY and added new permissions for MARKETPLACE and PEOPLE_PICKER.
- Ensured consistent structure for permissions across different types.

feat:  support_contact validation to allow empty email strings
2025-08-13 16:24:18 -04:00
Danny Avila
66bd419baa
🔐 feat: Granular Role-based Permissions + Entra ID Group Discovery (#7804)
WIP: pre-granular-permissions commit

feat: Add category and support contact fields to Agent schema and UI components

Revert "feat: Add category and support contact fields to Agent schema and UI components"

This reverts commit c43a52b4c9.

Fix: Update import for renderHook in useAgentCategories.spec.tsx

fix: Update icon rendering in AgentCategoryDisplay tests to use empty spans

refactor: Improve category synchronization logic and clean up AgentConfig component

refactor: Remove unused UI flow translations from translation.json

feat: agent marketplace features

🔐 feat: Granular Role-based Permissions + Entra ID Group Discovery (#7804)
2025-08-13 16:24:17 -04:00
Sebastien Bruel
0b071c06f6
🥞 refactor: Duplicate Agent Versions as Informational Instead of Errors (#8881)
* Fix error when updating an agent with no changes

* Add tests

* Revert translation file changes
2025-08-07 02:12:05 -04:00
Samuel Path
d2e1ca4c4a
🖼️ fix: Permission Checks for Agent Avatar Uploads (#8412)
Implements permission validation before allowing agent avatar uploads. Only admins, the agent's author, or users of collaborative agents can modify avatars. Also improves error handling by checking for agent existence upfront and simplifies avatar update logic.

Co-authored-by: Sai Nihas <sai.nihas@shopify.com>
2025-07-11 15:37:11 -04:00
Danny Avila
a37bf6719c
🧪 refactor: Add Validation for Agent Creation/Updates (#8261)
* refactor: Add validation schemas for agent creation and updates

* fix: Ensure author validation is applied in correct order for agent update handler

* ci: Add comprehensive unit tests for agent creation and update handlers with mass assignment protection

* fix: add missing  web_search tool in system tools configuration
2025-07-05 11:34:28 -04:00