Skip to content

chore: misc. fixes/improvements before containerizing - #6

Merged
danny-avila merged 6 commits into
masterfrom
easy-config-setup
Mar 6, 2023
Merged

danny-avila merged 6 commits into
masterfrom
easy-config-setup

Conversation

@danny-avila

Copy link
Copy Markdown
Collaborator

No description provided.

@danny-avila
danny-avila merged commit ed6de18 into master Mar 6, 2023
danny-avila added a commit that referenced this pull request Apr 6, 2023
chore: misc. fixes/improvements before containerizing
ClaraLeigh pushed a commit to ClaraLeigh/chatgpt-clone that referenced this pull request May 4, 2023
danny-avila added a commit that referenced this pull request Jun 20, 2024
* WIP: MERGE prompts/frontend (#1)

* added schema for prompt and promptgroup, added model methods for prompts, added routes for prompts

* * updated promptGroup Schema

* updated model methods for prompts (get, add, delete)

* slight fixes in prompt routes

* * Created Files Management components

* Created Vector Stores components

* Added file management route in the routes folder

* Completed UI for Files list, Compeleted UI for vector stores list, Completed UI for upload file modal, Completed UI for preview file, Completed UI for preview vector store

* Fixed style and UI fixes for file dashboard, file list and vector stores list

* added responsiveness classes for vector store page

* fixed responsiveness of file page, dashboard page, and main page

* fixed styling and responsiveness issues on dashboard page, file list page and vector store page

* added queries and mutations for prompts and promptGroups, added relevant endpoints in data-provider, added relevant components prompts, added and updated relevant APIs

* added types on mutation queries data service, updated prompt attributes

* feature: Prompts and prompt groups management, added relevant APIs, added types for data service/queries/mutations, added relevant mutation and queries

* chore: typing clarifications

* added drop down on prompts mgmt dashboard

* Fixes: fixed version switching issue on tags update or labels update, added cross button on create prompt group, fixed list updation on prompt group renaiming, added CSV upload button

* Feature: Added oneliner and category attributes in prompt group, added schema for categories, added schema methods and route for categories

* chore: typing and lint issues

* chore: more type and linter fixes

* chore: linting

* chore: prompt controller and backend typing example; MOVE TO CONTROLLER DIRECTORY

* chore: more type fixes

* style: prompt name changes

* chore: more type changes, and stateful prompt name change without flickering

* fix: Return result of savePrompt in patchPrompt API endpoint

* fix: navigation prompt queries; refactor: name 'prompt-groups' to just 'groups'

* refactor: fetch prompt groups rewrite

* refactor(prompts): query/mutation statefulness

* refactor: remove `isActive` field

* refactor: remove labels, consolidate logic

* style: width, layout shift

* refactor: improve hover toggle behavior and styling

* refactor: add useParams hook to PromptListItem for dynamic rendering and add timeout ref for blur timeout

* chore: hide upload button

* refactor: import Button component from correct location in PromptSidePanel

* style: prompt editor styling

* style: fix more layout shifts

* style: container scroll

* refactor: Rename CreatePrompt component to CreatePromptForm

* refactor: use react-hook-form

* refactor: Add Prompts components and routes to Dashboard

* style: skeletons for loading

* fix: optimize makePromptProduction

* refactor: consolidate variables

* feat: create prompt form validation

* refactor: Consolidate variables and update mutation hooks

* style: minor touchups

* chore: Update lucide-react npm dependency to version 0.394.0 and npm audit fix

* refactor: add a new icon for the Prompts heading.

* style: Update PromptsView heading to use h1 instead of h2 and other minor margin issues

* chore: wording

* refactor: Update PromptsView heading to use h1 instead of h2, consolidate variables, and add new icons

* refactor: Prompts Button for Mobile

* feature: added category field in prompt group, added relevant API and static data on BE to support FE UI for category in prompt group

* chore: template for prompt cards

---------

Co-authored-by: Fawadpot <contactfawada@gmail.com>

* WIP: Prompts/frontend Continued (#2)

* chore: loading style, remove unused component

* feat: Add CategorySelector component for prompt group category selection

* feat: add categories to create prompt

* feat: prompt versions styling

* feat: optimistic updates for prompt production state

* refactor: optimize form state and show if prompt field is dirty with cross icon, also other styling changes

* chore: remove unused code and localizations

* fix: light mode styling

* WIP: SidePanel Prompts

* refactor: move to groups directory

* refactor: rename GroupsSidePanel to GroupSidePanel and update imports

* style: ListCard

* refactor: isProduction changes

* refactor: infinite query with productionPrompt

* refactor: optimize snippets and prompts, and styling

* refactor: Update getSnippet function to accept a length parameter

* chore: localizations

* feat: prompts navigation to chat and vice versa

* fix: create prompt

* feat: remember last selected category for creating prompts

* fix(promptGroups): fix pagination and add usePromptGroupsNav hook

* Prompts/frontend 3 (#3)

* fix: stateful issues with prompt groups

* style: improved layout

* refactor: improve variable naming in Eng.ts

* refactor: theme selector styling improvements

* added prompt cards on chat new page, with dark mode, added API to fetch random prompts, added types for useQuery

Slightly improved usePromptGroupNav logic to fetch updated result for pageSize, updated prompt cards view with darkmode and responsiveness

fixed page size option buttons styling to match the theme

added dark mode on create prompt page and prompt edit/preview page

fixed page size option buttons styling to match the theme

added dark mode on create prompt page and prompt edit/preview page

* WIP: Prompts/frontend (#4)

* fix: optimize and fix paginated query

* fix: remove unique constraint on names

* refactor: button links and styling

* style: menu border light mode

* feat: Add Auto-Send Switch component for prompts groups

* refactor(ChatView): use form context for submission text

* chore: clear convo state on navigation to dashboard routes

* chore: save prompt edit name on tab, remove console log

* feat: basic prompt submission

* refactor: move Auto-Send Switch

* style(ListCard): border styling

* feat: Add function to detect variables in text

* feat: Add OriginalDialog component to UI library

* chore(ui): Update SelectDropDown options list class to use text-xs size

* refactor: submitMessage hook now includes submitPrompt, make compatible to document query selector

* WIP: Variable Dialog

* feat: variable submission working for both auto-send and non-autosend

* feat: dashboard breadcrumbs and prompts/chat navigation

* refactor: dashboard breadcrumb and dashboard link to chat navigation

* refactor: Update VariableDialog and VariableForm styles

* Prompts: Admin features (#5)

* fix: link issue

* fix: usePromptGroupsNav add missing dep.

* style: dashbreadcrumb and sidepanel text color

* temp fix: remove refetch on pageNumber change

* fix: handle multiple variable replacement

* WIP: create project schema and add project groups to fetch

* feat: Add functionality to add prompt group IDs to a project

* feat: Add caching for startup config in config route

* chore: remove prompt landing

* style: Update Skeleton component with additional background styling

* chore: styling and types

* WIP: SharePrompt first draft

* feat(SharePrompt): form validation

* feat: shared global indicators

* refactor: prompt details

* refactor: change NoPromptGroup directory

* feat: preview prompt

* feat: remove/add global prompts, add rbac-related enums

* refactor: manage prompts location

* WIP: first draft admin settings for prompts

* feat: SystemRoles enum

* refactor: update PromptDetails component styling

* style: ellipsis custom class for showing more preview text

* WIP: initial role schema and initialization

* style: improved margins for single unordered lists

* fix: use custom chat form context to prevent re-renders from FormProvider

* feat: Role mutations for Prompt Permissions

* feat: fetch user role

* feat: update AdminSettings form default values from user role values

* refactor: rename PromptPermissions to Permissions for general definitions

* feat: initial role checks

* feat: Add optional `bodyProps` parameter to generateCheckAccess middleware

* refactor: UI access checks

* Prompts: delete (#6)

* Fixed delete prompt version API, fixed types and logic for prompt version deletion, updated prompt delete mutation logic

* chore: Update return type of deletePrompt function in Prompt.js

---------

Co-authored-by: Fawadpot <contactfawada@gmail.com>

* chore: Update package-lock.json version to 0.7.4-rc1 and fast-xml-parser to 4.4.0

* feat: toast for saving admin settings, add timer no-access navigation

* feat: always make prod

* feat: Add localization to category labels in CategorySelector component

* feat: Update category label localization in CategorySelector component

* fix: Enable making prompt production in Prompt API

---------

Co-authored-by: Fawadpot <contactfawada@gmail.com>
danny-avila added a commit that referenced this pull request Jun 21, 2024
* 🗨️ feat: Prompts (#7)

* WIP: MERGE prompts/frontend (#1)

* added schema for prompt and promptgroup, added model methods for prompts, added routes for prompts

* * updated promptGroup Schema

* updated model methods for prompts (get, add, delete)

* slight fixes in prompt routes

* * Created Files Management components

* Created Vector Stores components

* Added file management route in the routes folder

* Completed UI for Files list, Compeleted UI for vector stores list, Completed UI for upload file modal, Completed UI for preview file, Completed UI for preview vector store

* Fixed style and UI fixes for file dashboard, file list and vector stores list

* added responsiveness classes for vector store page

* fixed responsiveness of file page, dashboard page, and main page

* fixed styling and responsiveness issues on dashboard page, file list page and vector store page

* added queries and mutations for prompts and promptGroups, added relevant endpoints in data-provider, added relevant components prompts, added and updated relevant APIs

* added types on mutation queries data service, updated prompt attributes

* feature: Prompts and prompt groups management, added relevant APIs, added types for data service/queries/mutations, added relevant mutation and queries

* chore: typing clarifications

* added drop down on prompts mgmt dashboard

* Fixes: fixed version switching issue on tags update or labels update, added cross button on create prompt group, fixed list updation on prompt group renaiming, added CSV upload button

* Feature: Added oneliner and category attributes in prompt group, added schema for categories, added schema methods and route for categories

* chore: typing and lint issues

* chore: more type and linter fixes

* chore: linting

* chore: prompt controller and backend typing example; MOVE TO CONTROLLER DIRECTORY

* chore: more type fixes

* style: prompt name changes

* chore: more type changes, and stateful prompt name change without flickering

* fix: Return result of savePrompt in patchPrompt API endpoint

* fix: navigation prompt queries; refactor: name 'prompt-groups' to just 'groups'

* refactor: fetch prompt groups rewrite

* refactor(prompts): query/mutation statefulness

* refactor: remove `isActive` field

* refactor: remove labels, consolidate logic

* style: width, layout shift

* refactor: improve hover toggle behavior and styling

* refactor: add useParams hook to PromptListItem for dynamic rendering and add timeout ref for blur timeout

* chore: hide upload button

* refactor: import Button component from correct location in PromptSidePanel

* style: prompt editor styling

* style: fix more layout shifts

* style: container scroll

* refactor: Rename CreatePrompt component to CreatePromptForm

* refactor: use react-hook-form

* refactor: Add Prompts components and routes to Dashboard

* style: skeletons for loading

* fix: optimize makePromptProduction

* refactor: consolidate variables

* feat: create prompt form validation

* refactor: Consolidate variables and update mutation hooks

* style: minor touchups

* chore: Update lucide-react npm dependency to version 0.394.0 and npm audit fix

* refactor: add a new icon for the Prompts heading.

* style: Update PromptsView heading to use h1 instead of h2 and other minor margin issues

* chore: wording

* refactor: Update PromptsView heading to use h1 instead of h2, consolidate variables, and add new icons

* refactor: Prompts Button for Mobile

* feature: added category field in prompt group, added relevant API and static data on BE to support FE UI for category in prompt group

* chore: template for prompt cards

---------

Co-authored-by: Fawadpot <contactfawada@gmail.com>

* WIP: Prompts/frontend Continued (#2)

* chore: loading style, remove unused component

* feat: Add CategorySelector component for prompt group category selection

* feat: add categories to create prompt

* feat: prompt versions styling

* feat: optimistic updates for prompt production state

* refactor: optimize form state and show if prompt field is dirty with cross icon, also other styling changes

* chore: remove unused code and localizations

* fix: light mode styling

* WIP: SidePanel Prompts

* refactor: move to groups directory

* refactor: rename GroupsSidePanel to GroupSidePanel and update imports

* style: ListCard

* refactor: isProduction changes

* refactor: infinite query with productionPrompt

* refactor: optimize snippets and prompts, and styling

* refactor: Update getSnippet function to accept a length parameter

* chore: localizations

* feat: prompts navigation to chat and vice versa

* fix: create prompt

* feat: remember last selected category for creating prompts

* fix(promptGroups): fix pagination and add usePromptGroupsNav hook

* Prompts/frontend 3 (#3)

* fix: stateful issues with prompt groups

* style: improved layout

* refactor: improve variable naming in Eng.ts

* refactor: theme selector styling improvements

* added prompt cards on chat new page, with dark mode, added API to fetch random prompts, added types for useQuery

Slightly improved usePromptGroupNav logic to fetch updated result for pageSize, updated prompt cards view with darkmode and responsiveness

fixed page size option buttons styling to match the theme

added dark mode on create prompt page and prompt edit/preview page

fixed page size option buttons styling to match the theme

added dark mode on create prompt page and prompt edit/preview page

* WIP: Prompts/frontend (#4)

* fix: optimize and fix paginated query

* fix: remove unique constraint on names

* refactor: button links and styling

* style: menu border light mode

* feat: Add Auto-Send Switch component for prompts groups

* refactor(ChatView): use form context for submission text

* chore: clear convo state on navigation to dashboard routes

* chore: save prompt edit name on tab, remove console log

* feat: basic prompt submission

* refactor: move Auto-Send Switch

* style(ListCard): border styling

* feat: Add function to detect variables in text

* feat: Add OriginalDialog component to UI library

* chore(ui): Update SelectDropDown options list class to use text-xs size

* refactor: submitMessage hook now includes submitPrompt, make compatible to document query selector

* WIP: Variable Dialog

* feat: variable submission working for both auto-send and non-autosend

* feat: dashboard breadcrumbs and prompts/chat navigation

* refactor: dashboard breadcrumb and dashboard link to chat navigation

* refactor: Update VariableDialog and VariableForm styles

* Prompts: Admin features (#5)

* fix: link issue

* fix: usePromptGroupsNav add missing dep.

* style: dashbreadcrumb and sidepanel text color

* temp fix: remove refetch on pageNumber change

* fix: handle multiple variable replacement

* WIP: create project schema and add project groups to fetch

* feat: Add functionality to add prompt group IDs to a project

* feat: Add caching for startup config in config route

* chore: remove prompt landing

* style: Update Skeleton component with additional background styling

* chore: styling and types

* WIP: SharePrompt first draft

* feat(SharePrompt): form validation

* feat: shared global indicators

* refactor: prompt details

* refactor: change NoPromptGroup directory

* feat: preview prompt

* feat: remove/add global prompts, add rbac-related enums

* refactor: manage prompts location

* WIP: first draft admin settings for prompts

* feat: SystemRoles enum

* refactor: update PromptDetails component styling

* style: ellipsis custom class for showing more preview text

* WIP: initial role schema and initialization

* style: improved margins for single unordered lists

* fix: use custom chat form context to prevent re-renders from FormProvider

* feat: Role mutations for Prompt Permissions

* feat: fetch user role

* feat: update AdminSettings form default values from user role values

* refactor: rename PromptPermissions to Permissions for general definitions

* feat: initial role checks

* feat: Add optional `bodyProps` parameter to generateCheckAccess middleware

* refactor: UI access checks

* Prompts: delete (#6)

* Fixed delete prompt version API, fixed types and logic for prompt version deletion, updated prompt delete mutation logic

* chore: Update return type of deletePrompt function in Prompt.js

---------

Co-authored-by: Fawadpot <contactfawada@gmail.com>

* chore: Update package-lock.json version to 0.7.4-rc1 and fast-xml-parser to 4.4.0

* feat: toast for saving admin settings, add timer no-access navigation

* feat: always make prod

* feat: Add localization to category labels in CategorySelector component

* feat: Update category label localization in CategorySelector component

* fix: Enable making prompt production in Prompt API

---------

Co-authored-by: Fawadpot <contactfawada@gmail.com>

* feat: Add helper fn for dark mode detection in ThemeProvider

* style: surface-primary definition

* fix(useHasAccess): utilize user.role and not just USER role

* fix: empty category and role fetch

* refactort: increase max height to options list and use label if no localization is found

* fix: update CategorySelector to handle empty category value and improve localization

* refactor: move prompts to own store/reactquery modules, add in filter WIP

* refactor: Rename AutoSendSwitch to AutoSendPrompt

* style: theming commit

* style: fix slight coloring issue for convos in dark mode

* style: better composition for prompts side panel

* style: remove gray-750 and make it gray-850

* chore: adjust theming

* feat: filter all prompt groups and properly remove prompts from projects

* refactor: optimize delete prompt groups further

* chore: localization

* feat: Add uniqueProperty filtering to normalizeData function

* WIP: filter prompts

* chore: Update FilterPrompts component to include User icon in FilterItem

* feat(FilterPrompts): set categories

* feat: more system filters and show selected category icon

* style: always make prod, flips switch to avoid mis-clicks

* style: ui/ux loading/no prompts

* chore: style FilterPrompts ChatView

* fix: handle missing role edge case

* style: special variables

* feat: special variables

* refactor: improve replaceSpecialVars function in prompts.ts

* feat: simple/advanced editor modes

* chore: bump versions

* feat: localizations and hide production button on simple mode

* fix: error connecting layout shift

* fix: prompts CRUD for admins

* fix: secure single group fetch

* style: sidepanel styling

* style(PromptName): bring edit button closer to name

* style: mobile prompts header

* style: mobile prompts header continued

* style: align send prompts switch right

* feat: description

* Update special variables description in Eng.ts

* feat: update/create/preview oneliner

* fix: allow empty oneliner update

* style: loading improvement and always make selected prompt Production if simple mode

* fix: production index set and remove unused props

* fix(ci): mock initializeRoles

* fix: address #3128

* fix: address #3128

* feat: add deletion confirmation dialog

* fix: mobile UI issues

* style: prompt library UI update

* style: focus, logcal tab order

* style: Refactor SelectDropDown component to improve code readability and maintainability

* chore: bump data-provider

* chore: fix labels

* refactor: confirm delete prompt version

---------

Co-authored-by: Marco Beretta <81851188+berry-13@users.noreply.github.com>
kenshinsamue referenced this pull request in intelequia/LibreChat Sep 17, 2024
* 🗨️ feat: Prompts (#7)

* WIP: MERGE prompts/frontend (#1)

* added schema for prompt and promptgroup, added model methods for prompts, added routes for prompts

* * updated promptGroup Schema

* updated model methods for prompts (get, add, delete)

* slight fixes in prompt routes

* * Created Files Management components

* Created Vector Stores components

* Added file management route in the routes folder

* Completed UI for Files list, Compeleted UI for vector stores list, Completed UI for upload file modal, Completed UI for preview file, Completed UI for preview vector store

* Fixed style and UI fixes for file dashboard, file list and vector stores list

* added responsiveness classes for vector store page

* fixed responsiveness of file page, dashboard page, and main page

* fixed styling and responsiveness issues on dashboard page, file list page and vector store page

* added queries and mutations for prompts and promptGroups, added relevant endpoints in data-provider, added relevant components prompts, added and updated relevant APIs

* added types on mutation queries data service, updated prompt attributes

* feature: Prompts and prompt groups management, added relevant APIs, added types for data service/queries/mutations, added relevant mutation and queries

* chore: typing clarifications

* added drop down on prompts mgmt dashboard

* Fixes: fixed version switching issue on tags update or labels update, added cross button on create prompt group, fixed list updation on prompt group renaiming, added CSV upload button

* Feature: Added oneliner and category attributes in prompt group, added schema for categories, added schema methods and route for categories

* chore: typing and lint issues

* chore: more type and linter fixes

* chore: linting

* chore: prompt controller and backend typing example; MOVE TO CONTROLLER DIRECTORY

* chore: more type fixes

* style: prompt name changes

* chore: more type changes, and stateful prompt name change without flickering

* fix: Return result of savePrompt in patchPrompt API endpoint

* fix: navigation prompt queries; refactor: name 'prompt-groups' to just 'groups'

* refactor: fetch prompt groups rewrite

* refactor(prompts): query/mutation statefulness

* refactor: remove `isActive` field

* refactor: remove labels, consolidate logic

* style: width, layout shift

* refactor: improve hover toggle behavior and styling

* refactor: add useParams hook to PromptListItem for dynamic rendering and add timeout ref for blur timeout

* chore: hide upload button

* refactor: import Button component from correct location in PromptSidePanel

* style: prompt editor styling

* style: fix more layout shifts

* style: container scroll

* refactor: Rename CreatePrompt component to CreatePromptForm

* refactor: use react-hook-form

* refactor: Add Prompts components and routes to Dashboard

* style: skeletons for loading

* fix: optimize makePromptProduction

* refactor: consolidate variables

* feat: create prompt form validation

* refactor: Consolidate variables and update mutation hooks

* style: minor touchups

* chore: Update lucide-react npm dependency to version 0.394.0 and npm audit fix

* refactor: add a new icon for the Prompts heading.

* style: Update PromptsView heading to use h1 instead of h2 and other minor margin issues

* chore: wording

* refactor: Update PromptsView heading to use h1 instead of h2, consolidate variables, and add new icons

* refactor: Prompts Button for Mobile

* feature: added category field in prompt group, added relevant API and static data on BE to support FE UI for category in prompt group

* chore: template for prompt cards

---------

Co-authored-by: Fawadpot <contactfawada@gmail.com>

* WIP: Prompts/frontend Continued (#2)

* chore: loading style, remove unused component

* feat: Add CategorySelector component for prompt group category selection

* feat: add categories to create prompt

* feat: prompt versions styling

* feat: optimistic updates for prompt production state

* refactor: optimize form state and show if prompt field is dirty with cross icon, also other styling changes

* chore: remove unused code and localizations

* fix: light mode styling

* WIP: SidePanel Prompts

* refactor: move to groups directory

* refactor: rename GroupsSidePanel to GroupSidePanel and update imports

* style: ListCard

* refactor: isProduction changes

* refactor: infinite query with productionPrompt

* refactor: optimize snippets and prompts, and styling

* refactor: Update getSnippet function to accept a length parameter

* chore: localizations

* feat: prompts navigation to chat and vice versa

* fix: create prompt

* feat: remember last selected category for creating prompts

* fix(promptGroups): fix pagination and add usePromptGroupsNav hook

* Prompts/frontend 3 (#3)

* fix: stateful issues with prompt groups

* style: improved layout

* refactor: improve variable naming in Eng.ts

* refactor: theme selector styling improvements

* added prompt cards on chat new page, with dark mode, added API to fetch random prompts, added types for useQuery

Slightly improved usePromptGroupNav logic to fetch updated result for pageSize, updated prompt cards view with darkmode and responsiveness

fixed page size option buttons styling to match the theme

added dark mode on create prompt page and prompt edit/preview page

fixed page size option buttons styling to match the theme

added dark mode on create prompt page and prompt edit/preview page

* WIP: Prompts/frontend (#4)

* fix: optimize and fix paginated query

* fix: remove unique constraint on names

* refactor: button links and styling

* style: menu border light mode

* feat: Add Auto-Send Switch component for prompts groups

* refactor(ChatView): use form context for submission text

* chore: clear convo state on navigation to dashboard routes

* chore: save prompt edit name on tab, remove console log

* feat: basic prompt submission

* refactor: move Auto-Send Switch

* style(ListCard): border styling

* feat: Add function to detect variables in text

* feat: Add OriginalDialog component to UI library

* chore(ui): Update SelectDropDown options list class to use text-xs size

* refactor: submitMessage hook now includes submitPrompt, make compatible to document query selector

* WIP: Variable Dialog

* feat: variable submission working for both auto-send and non-autosend

* feat: dashboard breadcrumbs and prompts/chat navigation

* refactor: dashboard breadcrumb and dashboard link to chat navigation

* refactor: Update VariableDialog and VariableForm styles

* Prompts: Admin features (#5)

* fix: link issue

* fix: usePromptGroupsNav add missing dep.

* style: dashbreadcrumb and sidepanel text color

* temp fix: remove refetch on pageNumber change

* fix: handle multiple variable replacement

* WIP: create project schema and add project groups to fetch

* feat: Add functionality to add prompt group IDs to a project

* feat: Add caching for startup config in config route

* chore: remove prompt landing

* style: Update Skeleton component with additional background styling

* chore: styling and types

* WIP: SharePrompt first draft

* feat(SharePrompt): form validation

* feat: shared global indicators

* refactor: prompt details

* refactor: change NoPromptGroup directory

* feat: preview prompt

* feat: remove/add global prompts, add rbac-related enums

* refactor: manage prompts location

* WIP: first draft admin settings for prompts

* feat: SystemRoles enum

* refactor: update PromptDetails component styling

* style: ellipsis custom class for showing more preview text

* WIP: initial role schema and initialization

* style: improved margins for single unordered lists

* fix: use custom chat form context to prevent re-renders from FormProvider

* feat: Role mutations for Prompt Permissions

* feat: fetch user role

* feat: update AdminSettings form default values from user role values

* refactor: rename PromptPermissions to Permissions for general definitions

* feat: initial role checks

* feat: Add optional `bodyProps` parameter to generateCheckAccess middleware

* refactor: UI access checks

* Prompts: delete (#6)

* Fixed delete prompt version API, fixed types and logic for prompt version deletion, updated prompt delete mutation logic

* chore: Update return type of deletePrompt function in Prompt.js

---------

Co-authored-by: Fawadpot <contactfawada@gmail.com>

* chore: Update package-lock.json version to 0.7.4-rc1 and fast-xml-parser to 4.4.0

* feat: toast for saving admin settings, add timer no-access navigation

* feat: always make prod

* feat: Add localization to category labels in CategorySelector component

* feat: Update category label localization in CategorySelector component

* fix: Enable making prompt production in Prompt API

---------

Co-authored-by: Fawadpot <contactfawada@gmail.com>

* feat: Add helper fn for dark mode detection in ThemeProvider

* style: surface-primary definition

* fix(useHasAccess): utilize user.role and not just USER role

* fix: empty category and role fetch

* refactort: increase max height to options list and use label if no localization is found

* fix: update CategorySelector to handle empty category value and improve localization

* refactor: move prompts to own store/reactquery modules, add in filter WIP

* refactor: Rename AutoSendSwitch to AutoSendPrompt

* style: theming commit

* style: fix slight coloring issue for convos in dark mode

* style: better composition for prompts side panel

* style: remove gray-750 and make it gray-850

* chore: adjust theming

* feat: filter all prompt groups and properly remove prompts from projects

* refactor: optimize delete prompt groups further

* chore: localization

* feat: Add uniqueProperty filtering to normalizeData function

* WIP: filter prompts

* chore: Update FilterPrompts component to include User icon in FilterItem

* feat(FilterPrompts): set categories

* feat: more system filters and show selected category icon

* style: always make prod, flips switch to avoid mis-clicks

* style: ui/ux loading/no prompts

* chore: style FilterPrompts ChatView

* fix: handle missing role edge case

* style: special variables

* feat: special variables

* refactor: improve replaceSpecialVars function in prompts.ts

* feat: simple/advanced editor modes

* chore: bump versions

* feat: localizations and hide production button on simple mode

* fix: error connecting layout shift

* fix: prompts CRUD for admins

* fix: secure single group fetch

* style: sidepanel styling

* style(PromptName): bring edit button closer to name

* style: mobile prompts header

* style: mobile prompts header continued

* style: align send prompts switch right

* feat: description

* Update special variables description in Eng.ts

* feat: update/create/preview oneliner

* fix: allow empty oneliner update

* style: loading improvement and always make selected prompt Production if simple mode

* fix: production index set and remove unused props

* fix(ci): mock initializeRoles

* fix: address LibreChat-AI#3128

* fix: address LibreChat-AI#3128

* feat: add deletion confirmation dialog

* fix: mobile UI issues

* style: prompt library UI update

* style: focus, logcal tab order

* style: Refactor SelectDropDown component to improve code readability and maintainability

* chore: bump data-provider

* chore: fix labels

* refactor: confirm delete prompt version

---------

Co-authored-by: Marco Beretta <81851188+berry-13@users.noreply.github.com>
BertKiv pushed a commit to BertKiv/LibreChat that referenced this pull request Dec 10, 2024
* 🗨️ feat: Prompts (LibreChat-AI#7)

* WIP: MERGE prompts/frontend (LibreChat-AI#1)

* added schema for prompt and promptgroup, added model methods for prompts, added routes for prompts

* * updated promptGroup Schema

* updated model methods for prompts (get, add, delete)

* slight fixes in prompt routes

* * Created Files Management components

* Created Vector Stores components

* Added file management route in the routes folder

* Completed UI for Files list, Compeleted UI for vector stores list, Completed UI for upload file modal, Completed UI for preview file, Completed UI for preview vector store

* Fixed style and UI fixes for file dashboard, file list and vector stores list

* added responsiveness classes for vector store page

* fixed responsiveness of file page, dashboard page, and main page

* fixed styling and responsiveness issues on dashboard page, file list page and vector store page

* added queries and mutations for prompts and promptGroups, added relevant endpoints in data-provider, added relevant components prompts, added and updated relevant APIs

* added types on mutation queries data service, updated prompt attributes

* feature: Prompts and prompt groups management, added relevant APIs, added types for data service/queries/mutations, added relevant mutation and queries

* chore: typing clarifications

* added drop down on prompts mgmt dashboard

* Fixes: fixed version switching issue on tags update or labels update, added cross button on create prompt group, fixed list updation on prompt group renaiming, added CSV upload button

* Feature: Added oneliner and category attributes in prompt group, added schema for categories, added schema methods and route for categories

* chore: typing and lint issues

* chore: more type and linter fixes

* chore: linting

* chore: prompt controller and backend typing example; MOVE TO CONTROLLER DIRECTORY

* chore: more type fixes

* style: prompt name changes

* chore: more type changes, and stateful prompt name change without flickering

* fix: Return result of savePrompt in patchPrompt API endpoint

* fix: navigation prompt queries; refactor: name 'prompt-groups' to just 'groups'

* refactor: fetch prompt groups rewrite

* refactor(prompts): query/mutation statefulness

* refactor: remove `isActive` field

* refactor: remove labels, consolidate logic

* style: width, layout shift

* refactor: improve hover toggle behavior and styling

* refactor: add useParams hook to PromptListItem for dynamic rendering and add timeout ref for blur timeout

* chore: hide upload button

* refactor: import Button component from correct location in PromptSidePanel

* style: prompt editor styling

* style: fix more layout shifts

* style: container scroll

* refactor: Rename CreatePrompt component to CreatePromptForm

* refactor: use react-hook-form

* refactor: Add Prompts components and routes to Dashboard

* style: skeletons for loading

* fix: optimize makePromptProduction

* refactor: consolidate variables

* feat: create prompt form validation

* refactor: Consolidate variables and update mutation hooks

* style: minor touchups

* chore: Update lucide-react npm dependency to version 0.394.0 and npm audit fix

* refactor: add a new icon for the Prompts heading.

* style: Update PromptsView heading to use h1 instead of h2 and other minor margin issues

* chore: wording

* refactor: Update PromptsView heading to use h1 instead of h2, consolidate variables, and add new icons

* refactor: Prompts Button for Mobile

* feature: added category field in prompt group, added relevant API and static data on BE to support FE UI for category in prompt group

* chore: template for prompt cards

---------

Co-authored-by: Fawadpot <contactfawada@gmail.com>

* WIP: Prompts/frontend Continued (LibreChat-AI#2)

* chore: loading style, remove unused component

* feat: Add CategorySelector component for prompt group category selection

* feat: add categories to create prompt

* feat: prompt versions styling

* feat: optimistic updates for prompt production state

* refactor: optimize form state and show if prompt field is dirty with cross icon, also other styling changes

* chore: remove unused code and localizations

* fix: light mode styling

* WIP: SidePanel Prompts

* refactor: move to groups directory

* refactor: rename GroupsSidePanel to GroupSidePanel and update imports

* style: ListCard

* refactor: isProduction changes

* refactor: infinite query with productionPrompt

* refactor: optimize snippets and prompts, and styling

* refactor: Update getSnippet function to accept a length parameter

* chore: localizations

* feat: prompts navigation to chat and vice versa

* fix: create prompt

* feat: remember last selected category for creating prompts

* fix(promptGroups): fix pagination and add usePromptGroupsNav hook

* Prompts/frontend 3 (LibreChat-AI#3)

* fix: stateful issues with prompt groups

* style: improved layout

* refactor: improve variable naming in Eng.ts

* refactor: theme selector styling improvements

* added prompt cards on chat new page, with dark mode, added API to fetch random prompts, added types for useQuery

Slightly improved usePromptGroupNav logic to fetch updated result for pageSize, updated prompt cards view with darkmode and responsiveness

fixed page size option buttons styling to match the theme

added dark mode on create prompt page and prompt edit/preview page

fixed page size option buttons styling to match the theme

added dark mode on create prompt page and prompt edit/preview page

* WIP: Prompts/frontend (LibreChat-AI#4)

* fix: optimize and fix paginated query

* fix: remove unique constraint on names

* refactor: button links and styling

* style: menu border light mode

* feat: Add Auto-Send Switch component for prompts groups

* refactor(ChatView): use form context for submission text

* chore: clear convo state on navigation to dashboard routes

* chore: save prompt edit name on tab, remove console log

* feat: basic prompt submission

* refactor: move Auto-Send Switch

* style(ListCard): border styling

* feat: Add function to detect variables in text

* feat: Add OriginalDialog component to UI library

* chore(ui): Update SelectDropDown options list class to use text-xs size

* refactor: submitMessage hook now includes submitPrompt, make compatible to document query selector

* WIP: Variable Dialog

* feat: variable submission working for both auto-send and non-autosend

* feat: dashboard breadcrumbs and prompts/chat navigation

* refactor: dashboard breadcrumb and dashboard link to chat navigation

* refactor: Update VariableDialog and VariableForm styles

* Prompts: Admin features (LibreChat-AI#5)

* fix: link issue

* fix: usePromptGroupsNav add missing dep.

* style: dashbreadcrumb and sidepanel text color

* temp fix: remove refetch on pageNumber change

* fix: handle multiple variable replacement

* WIP: create project schema and add project groups to fetch

* feat: Add functionality to add prompt group IDs to a project

* feat: Add caching for startup config in config route

* chore: remove prompt landing

* style: Update Skeleton component with additional background styling

* chore: styling and types

* WIP: SharePrompt first draft

* feat(SharePrompt): form validation

* feat: shared global indicators

* refactor: prompt details

* refactor: change NoPromptGroup directory

* feat: preview prompt

* feat: remove/add global prompts, add rbac-related enums

* refactor: manage prompts location

* WIP: first draft admin settings for prompts

* feat: SystemRoles enum

* refactor: update PromptDetails component styling

* style: ellipsis custom class for showing more preview text

* WIP: initial role schema and initialization

* style: improved margins for single unordered lists

* fix: use custom chat form context to prevent re-renders from FormProvider

* feat: Role mutations for Prompt Permissions

* feat: fetch user role

* feat: update AdminSettings form default values from user role values

* refactor: rename PromptPermissions to Permissions for general definitions

* feat: initial role checks

* feat: Add optional `bodyProps` parameter to generateCheckAccess middleware

* refactor: UI access checks

* Prompts: delete (LibreChat-AI#6)

* Fixed delete prompt version API, fixed types and logic for prompt version deletion, updated prompt delete mutation logic

* chore: Update return type of deletePrompt function in Prompt.js

---------

Co-authored-by: Fawadpot <contactfawada@gmail.com>

* chore: Update package-lock.json version to 0.7.4-rc1 and fast-xml-parser to 4.4.0

* feat: toast for saving admin settings, add timer no-access navigation

* feat: always make prod

* feat: Add localization to category labels in CategorySelector component

* feat: Update category label localization in CategorySelector component

* fix: Enable making prompt production in Prompt API

---------

Co-authored-by: Fawadpot <contactfawada@gmail.com>

* feat: Add helper fn for dark mode detection in ThemeProvider

* style: surface-primary definition

* fix(useHasAccess): utilize user.role and not just USER role

* fix: empty category and role fetch

* refactort: increase max height to options list and use label if no localization is found

* fix: update CategorySelector to handle empty category value and improve localization

* refactor: move prompts to own store/reactquery modules, add in filter WIP

* refactor: Rename AutoSendSwitch to AutoSendPrompt

* style: theming commit

* style: fix slight coloring issue for convos in dark mode

* style: better composition for prompts side panel

* style: remove gray-750 and make it gray-850

* chore: adjust theming

* feat: filter all prompt groups and properly remove prompts from projects

* refactor: optimize delete prompt groups further

* chore: localization

* feat: Add uniqueProperty filtering to normalizeData function

* WIP: filter prompts

* chore: Update FilterPrompts component to include User icon in FilterItem

* feat(FilterPrompts): set categories

* feat: more system filters and show selected category icon

* style: always make prod, flips switch to avoid mis-clicks

* style: ui/ux loading/no prompts

* chore: style FilterPrompts ChatView

* fix: handle missing role edge case

* style: special variables

* feat: special variables

* refactor: improve replaceSpecialVars function in prompts.ts

* feat: simple/advanced editor modes

* chore: bump versions

* feat: localizations and hide production button on simple mode

* fix: error connecting layout shift

* fix: prompts CRUD for admins

* fix: secure single group fetch

* style: sidepanel styling

* style(PromptName): bring edit button closer to name

* style: mobile prompts header

* style: mobile prompts header continued

* style: align send prompts switch right

* feat: description

* Update special variables description in Eng.ts

* feat: update/create/preview oneliner

* fix: allow empty oneliner update

* style: loading improvement and always make selected prompt Production if simple mode

* fix: production index set and remove unused props

* fix(ci): mock initializeRoles

* fix: address LibreChat-AI#3128

* fix: address LibreChat-AI#3128

* feat: add deletion confirmation dialog

* fix: mobile UI issues

* style: prompt library UI update

* style: focus, logcal tab order

* style: Refactor SelectDropDown component to improve code readability and maintainability

* chore: bump data-provider

* chore: fix labels

* refactor: confirm delete prompt version

---------

Co-authored-by: Marco Beretta <81851188+berry-13@users.noreply.github.com>
bl1nkker added a commit to bl1nkker/LibreChat that referenced this pull request Nov 24, 2025
…at/LC-2

fix: issue 2: added agent questions repr
jflores1 added a commit to superwebpros/LibreChat that referenced this pull request Jan 25, 2026
Phase 0 - Task 6: Register groups API in main application

Changes:
- Add groups route import to api/server/routes/index.js
- Register /api/groups endpoint in api/server/index.js
- Routes now accessible at http://localhost:3080/api/groups

Route registration:
  app.use('/api/groups', routes.groups);

All 9 group endpoints now active:
  GET    /api/groups/me
  GET    /api/groups
  POST   /api/groups
  GET    /api/groups/:groupId
  PATCH  /api/groups/:groupId
  DELETE /api/groups/:groupId
  GET    /api/groups/:groupId/members
  PATCH  /api/groups/:groupId/members
  POST   /api/groups/:groupId/members/bulk

Protected by:
- requireJwtAuth middleware
- checkGroupAccess / checkGroupCreate permission checks

Related: LibreChat-AI#6

Co-Authored-By: Claude Sonnet 4.5 (1M context) <noreply@anthropic.com>

@colin-theraday colin-theraday left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was added in error by an automatic tool. My sincere apologies.

abhishek-barnwal01 pushed a commit to abhishek-barnwal01/LibreChat that referenced this pull request Feb 24, 2026
LibreChat-AI#1 Merge intent classification + enrichment into a single LLM call
   (UnifiedSemanticOutput) — eliminates separate round-trips for
   direct and semantic_specific intents.

LibreChat-AI#3 Fold Step 2D final extraction into tool-calling loop — skip the
   redundant text-only iteration before structured extraction.

LibreChat-AI#4 Batch PostgresStore writes via ThreadPoolExecutor — parallel DB
   round-trips instead of serial N × latency.

LibreChat-AI#6 Already implemented (execute_tool_calls uses ThreadPoolExecutor).

https://claude.ai/code/session_01JjJYeRf8NGx4GarZkcbJob
danny-avila added a commit that referenced this pull request Mar 7, 2026
… review findings

- Extract shared provider resolution logic into useAgentFileConfig hook
  (Finding #2: DRY violation across FileContext, FileSearch, Code/Files)
- Restore 18 deleted test cases in AttachFileMenu.spec.tsx covering
  agent capabilities, SharePoint, edge cases, and button state
  (Finding #1: accidental test deletion)
- Wrap fileConfigEndpoint in useMemo in AttachFileChat (Finding #3)
- Fix misleading test name in AgentFileConfig.spec.tsx (Finding #4)
- Fix import order in FileSearch.tsx, FileContext.tsx, Code/Files.tsx (Finding #5)
- Add comment about cache gap in useDragHelpers (Finding #6)
- Clarify resolveEndpointType JSDoc (Finding #7)
danny-avila added a commit that referenced this pull request Mar 7, 2026
…12117)

* chore: Remove unused setValueOnChange prop from MCPServerMenuItem component

* fix: Resolve agent provider endpoint type for file upload support

When using the agents endpoint with a custom provider (e.g., Moonshot),
the endpointType was resolving to "agents" instead of the provider's
actual type ("custom"), causing "Upload to Provider" to not appear in
the file attach menu.

Adds `resolveEndpointType` utility in data-provider that follows the
chain: endpoint (if not agents) → agent.provider → agents. Applied
consistently across AttachFileChat, DragDropContext, useDragHelpers,
and AgentPanel file components (FileContext, FileSearch, Code/Files).

* refactor: Extract useAgentFileConfig hook, restore deleted tests, fix review findings

- Extract shared provider resolution logic into useAgentFileConfig hook
  (Finding #2: DRY violation across FileContext, FileSearch, Code/Files)
- Restore 18 deleted test cases in AttachFileMenu.spec.tsx covering
  agent capabilities, SharePoint, edge cases, and button state
  (Finding #1: accidental test deletion)
- Wrap fileConfigEndpoint in useMemo in AttachFileChat (Finding #3)
- Fix misleading test name in AgentFileConfig.spec.tsx (Finding #4)
- Fix import order in FileSearch.tsx, FileContext.tsx, Code/Files.tsx (Finding #5)
- Add comment about cache gap in useDragHelpers (Finding #6)
- Clarify resolveEndpointType JSDoc (Finding #7)

* refactor: Memoize Footer component for performance optimization

- Converted Footer component to a memoized version to prevent unnecessary re-renders.
- Improved import structure by adding memo to the React import statement for clarity.

* chore: Fix remaining review nits

- Widen useAgentFileConfig return type to EModelEndpoint | string
- Fix import order in FileContext.tsx and FileSearch.tsx
- Remove dead endpointType param from setupMocks in AttachFileMenu test

* fix: Pass resolved provider endpoint to file upload validation

AgentPanel file components (FileContext, FileSearch, Code/Files) were
hardcoding endpointOverride to "agents", causing both client-side
validation (file limits, MIME types) and server-side validation to
use the agents config instead of the provider-specific config.

Adds endpointTypeOverride to UseFileHandling params so endpoint and
endpointType can be set independently. Components now pass the
resolved provider name and type from useAgentFileConfig, so the full
fallback chain (provider → custom → agents → default) applies to
file upload validation on both client and server.

* test: Verify any custom endpoint is document-supported regardless of name

Adds parameterized tests with arbitrary endpoint names (spaces, hyphens,
colons, etc.) confirming that all custom endpoints resolve to
document-supported through resolveEndpointType, both as direct
endpoints and as agent providers.

* fix: Use || for provider fallback, test endpointOverride wiring

- Change providerValue ?? to providerValue || so empty string is
  treated as "no provider" consistently with resolveEndpointType
- Add wiring tests to CodeFiles, FileContext, FileSearch verifying
  endpointOverride and endpointTypeOverride are passed correctly
- Update endpointOverride JSDoc to document endpointType fallback
danny-avila added a commit that referenced this pull request Mar 22, 2026
- Fix #5: Gate DeletePrompt in HeaderActions behind canDelete permission
- Fix #8: BackToChat navigates to last conversation instead of /c/new
- Fix #7: Restore useLiveAnnouncer for screen reader feedback on delete/rename
- Fix #1: Use isPublic (set by API) instead of deprecated projectIds for globe icon
- Fix #4: Optimistic cache update in useRecordPromptUsage instead of full invalidation
- Fix #6: Add migration to drop superseded { createdAt, updatedAt } compound index
- Fix #9: Single-pass reduce in PromptVariables instead of triple filter
- Fix #10: Rename PromptLabelsForm internal component to avoid collision with PromptForm
- Fix #14: Remove redundant aria-label from aria-hidden Checkbox in AutoSendPrompt
danny-avila added a commit that referenced this pull request Mar 22, 2026
* style: enhance prompts UI with new components and improved structure; add CreatePromptButton and AutoSendPrompt; refactor GroupSidePanel and PromptsAccordion

* refactor(Prompts): move button components to buttons/ subdirectory

* refactor(Prompts): move dialog components to dialogs/ subdirectory

* refactor(Prompts): move display components to display/ subdirectory

* refactor(Prompts): move editor components to editor/ subdirectory

* refactor(Prompts): move field components to fields/ subdirectory

* refactor(Prompts): move form components to forms/ subdirectory

* refactor(Prompts): move layout components to layouts/ subdirectory

* refactor(Prompts): move list components to lists/ subdirectory

* refactor(Prompts): move sidebar components to sidebar/ subdirectory

* refactor(Prompts): move utility components to utils/ subdirectory

* refactor(Prompts): update main exports and external imports

* refactor(Prompts): fix class name typo in AutoSendPrompt

* refactor(Prompts): reorganize exports and imports order across components

* refactor(Prompts): reorder exports for better organization and clarity

* refactor(Buttons): enhance prompts accessibility with aria-labels and update translations

* refactor(AdminSettings): reorganize imports and improve form structure for clarity

* refactor(Dialogs): reorganize imports for consistency and clarity across DeleteVersion, SharePrompt, and VariableDialog components

* refactor(Dialogs): enhance prompts accessibility with aria-labels

* refactor(Display): enhance prompt components and accessibility features

* refactor(.gitignore): add Playwright MCP directory

* refactor(Preview): enhance prompt components, improve layout, and add accessibility features

* refactor(Prompts): enhance variable handling, improve accessibility, and update UI components

* refactor(Prompts): enhance loading state handling and improve accessibility in PromptName component

* refactor(Prompts): streamline special variable handling, improve icon management, and enhance UI components

* refactor(Prompts): update AdvancedSwitch component to use Radio for mode selection, enhance PromptName with tooltips, and improve layout in PromptForm

* refactor(Prompts): enhance VersionCard and VersionBadge components for improved UI and accessibility, update loading state handling in VersionsPanel

* refactor(Prompts): improve layout and styling of VersionCard component for better visual alignment and clarity

* refactor(DeleteVersion): update text color for confirmation prompt in DeleteConfirmDialog

* refactor(Prompts): add configurations for always make production and auto-send prompts, update localization strings for clarity

* refactor(Prompts): enhance layout and styling in CategorySelector, CreatePromptForm, and List components for improved responsiveness and clarity

* refactor(Prompts): enhance PromptDetailHeader and ChatGroupItem components, add shared prompt indication, and remove unused PromptMetadata component

* refactor(Prompts): implement prompt group usage tracking, update sorting logic, and enhance related components

* fix(Prompts): security, performance, and pagination fixes

- Fix cursor pagination skipping/duplicating items by including
  numberOfGenerations in cursor condition to match sort order
- Close NoSQL injection vector via otherFilters rest spread in
  GET /all, GET /groups, and buildPromptGroupFilter
- Validate groupId as ObjectId before passing to query (GET /)
- Add prompt body validation in addPromptToGroup (type + text)
- Return 404 instead of 500 for missing group in POST /use
- Combine data + count into single $facet aggregation
- Add compound index {numberOfGenerations, updatedAt, _id}
- Add index on prompt.author for deleteUserPrompts
- Update useRecordPromptUsage to refresh client caches
- Replace console.error with logger.error

* refactor(PromptForm): remove console warning for unselected prompt in VersionsPanel

* refactor(Prompts): improve error handling for groupId and streamline usage tracking

* refactor(.gitignore): add CLAUDE.md to ignore list

* refactor(Prompts): streamline prompt components by removing unused variables and enhancing props structure

* refactor(Prompts): fix sort stability, keyboard handling, and remove dead code

Add _id tiebreaker to prompt group sort pipelines for deterministic
pagination ordering. Prevent default browser scroll on Space key in
PromptEditor preview mode. Remove unused blurTimeoutRef and its
onMutate callback from DashGroupItem.

* refactor(Prompts): enhance groupId validation and improve prompt group aggregation handling

* fix: aria-hidden, API fixes, accessibility improvements

* fix: ACL author filter, mobile guard, semantic HTML, and add useFocusTrap hook

- Remove author filter from patchPromptGroup so ACL-granted editors
  can update prompt groups (aligns with deletePromptGroupController)
- Add missing group guard to mobile HeaderActions in PromptForm
- Replace div with article in DashGroupItem, remove redundant
  stopPropagation and onClick on outer container
- Add useFocusTrap hook for keyboard focus management
- Add numberOfGenerations to default projection
- Deduplicate ObjectId validation, remove console.warn,
  fix aria-labelledby, localize search announcements

* refactor(Prompts): adjust UI and improve a11y

* refactor(Prompts): reorder imports for consistency and clarity

* refactor(Prompts): implement updateFieldsInPlace for efficient data updates and add related tests

* refactor(Prompts): reorder imports to include updateFieldsInPlace for better organization

* refactor(Prompts): enhance DashGroupItem with toast notifications for prompt updates and add click-to-edit functionality in PromptEditor

* style: use self-closing TooltipAnchor in CreatePromptButton

Replace ></TooltipAnchor> with /> for consistency with the rest of the Prompts directory.

* fix(i18n): replace placeholder text for com_ui_global_group translation key

The value was left as 'something needs to go here. was empty' which
would be visible to users as an aria-label in DashGroupItem.

* fix(DashGroupItem): sync rename input with group.name on external changes

nameInputValue was initialized via useState(group.name) but never
synced when group.name changed from a background refetch. Added
useEffect that updates the input when the dialog is closed.

* perf(useFocusTrap): store onEscape in ref to avoid listener churn

onEscape was in the useEffect dependency array, causing the keydown
listener to be torn down and re-attached on every render when callers
passed an inline function. Now stored in a ref so the effect only
re-runs when active or containerRef changes.

* fix(a11y): replace role=button div with layered button overlay in ListCard

The card used role='button' on a div that contained nested Button
elements — an invalid ARIA pattern. Replaced with a hidden button
at z-0 for the card action while child interactive elements sit
at z-10, eliminating nested interactive element violations.

* fix(PromptForm): reset selectionIndex on route change, guard auto-save, and fix a11y

- Reset selectionIndex to 0 and isEditing to false when promptId
  changes, preventing out-of-bounds index when navigating between
  groups with different version counts.
- Track selectedPrompt in a ref so the auto-save effect doesn't
  fire against a stale prompt when the selection changed mid-edit.
- Stabilize useFocusTrap onEscape via useCallback to avoid
  unnecessary listener re-attachment.
- Conditionally render mobile overlay instead of always-present
  button with aria-hidden/pointer-events toggling.

* refactor: extract isValidObjectIdString to shared utility in data-schemas

The same regex helper was duplicated in api/server/routes/prompts.js
and packages/data-schemas/src/methods/prompt.ts. Moved to
packages/data-schemas/src/utils/objectId.ts and imported from both
consumers. Also removed a duplicate router.use block introduced
during the extraction.

* perf(updateFieldsInPlace): replace JSON deep clone with targeted spread

Instead of JSON.parse(JSON.stringify(data)) which serializes the
entire paginated data structure, use targeted immutable spreads
that only copy the affected page and collection array. Returns the
original data reference unchanged when the item is not found.

* perf(VariablesDropdown): memoize items array and stabilize handleAddVariable

The items array containing JSX elements was rebuilt on every render.
Wrapped in useMemo keyed on usedVariables and localize. Also wrapped
handleAddVariable in useCallback and memoized usedCount to avoid
redundant array filtering.

* perf(DashGroupItem): stabilize mutation callbacks via refs

handleSaveRename and handleDelete had updateGroup/deleteGroup mutation
objects in their useCallback dependency arrays. Since mutation objects
are new references each render, the callbacks were recreated every
render, defeating memoization. Now store mutation objects in refs and
call via ref.current in the callbacks.

* fix(security): validate groupId in incrementPromptGroupUsage

The data-schema method passed the groupId string directly to
findByIdAndUpdate without validation. If called from a different
entrypoint without the route-level check, Mongoose would throw a
CastError. Now validates with isValidObjectIdString before the
DB call and throws a clean 'Invalid groupId' error.

* fix(security): add rate limiter to prompt usage tracking endpoint

POST /groups/:groupId/use had no rate limiting — a user could spam
it to inflate numberOfGenerations, which controls sort order for all
users. Added promptUsageLimiter (30 req/user/min) following the same
pattern as toolCallLimiter. Also handle 'Invalid groupId' error from
the data layer in the route error handler.

* fix(updateFieldsInPlace): guard against undefined identifier value

If updatedItem[identifierField] is null/undefined, findIndex could
match unintended items where that field is also undefined. Added
early return when the identifier value is nullish.

* fix(a11y): use React useId for stable unique IDs in ListCard

aria-describedby/id values were derived from prompt name which can
contain spaces and special characters, producing invalid HTML IDs
and potential collisions. Now uses React.useId() for guaranteed
unique, valid IDs per component instance.

* fix: Align prompts panel styling with other sidebar panels and fix test

- Match FilterPrompts first row to Memory/Bookmark pattern (items-center gap-2)
- Remove items-stretch override from PromptsAccordion
- Add missing promptUsageLimiter mock to prompts route test

* fix: Address code review findings for prompts refactor PR

- Fix #5: Gate DeletePrompt in HeaderActions behind canDelete permission
- Fix #8: BackToChat navigates to last conversation instead of /c/new
- Fix #7: Restore useLiveAnnouncer for screen reader feedback on delete/rename
- Fix #1: Use isPublic (set by API) instead of deprecated projectIds for globe icon
- Fix #4: Optimistic cache update in useRecordPromptUsage instead of full invalidation
- Fix #6: Add migration to drop superseded { createdAt, updatedAt } compound index
- Fix #9: Single-pass reduce in PromptVariables instead of triple filter
- Fix #10: Rename PromptLabelsForm internal component to avoid collision with PromptForm
- Fix #14: Remove redundant aria-label from aria-hidden Checkbox in AutoSendPrompt

* fix: Align prompts panel filter row element sizes with other panels

- Override Dropdown trigger to size-9 (36px) to match FilterInput height
- Set CreatePromptButton to size-9 shrink-0 bg-transparent matching
  Memory/Bookmark panel button pattern

* fix(prompts): Shared Prompts filter ignores direct shares, only returns PUBLIC

Folds fix from PR #11882 into the refactored codebase.

Bug A: filterAccessibleIdsBySharedLogic now accepts ownedPromptGroupIds:
- MY_PROMPTS: accessible intersect owned
- SHARED_PROMPTS: (accessible union public) minus owned
- ALL: accessible union public (deduplicated)
Legacy fallback preserved when ownedPromptGroupIds is omitted.

Bug B: getPromptGroup uses $lookup aggregation to populate productionPrompt,
fixing empty text on direct URL navigation to shared prompts.

Also adds getOwnedPromptGroupIds to data-schemas methods and passes it
from both /all and /groups route handlers.

* fix: Add missing canDelete to mobile HeaderActions, remove dead instanceProjectId prop

- Pass canDelete to mobile HeaderActions row (was only on desktop)
- Remove instanceProjectId prop from ChatGroupItem and DashGroupItem
  since global check now uses group.isPublic
- Remove useGetStartupConfig from List.tsx (no longer needed)

* fix: Use runtime ObjectId instead of type-only Types.ObjectId, fix i18next interpolation

- getPromptGroup and getOwnedPromptGroupIds were using Types.ObjectId
  (imported as type-only), which is erased at compile time. Use the
  runtime ObjectId from mongoose.Types (already destructured at line 20).
  This fixes the 404s in PATCH /groups/:groupId tests.
- Fix com_ui_prompt_deleted_group translation to use {{0}} (i18next
  double-brace syntax) instead of {0}.

* chore: Fix translation key ordering, add sideEffects: false to data-provider

- Reorder new translation keys to maintain alphabetical order:
  com_ui_click_to_edit, com_ui_labels, com_ui_live, com_ui_prompt_delete_confirm,
  com_ui_prompt_deleted_group, com_ui_prompt_details, com_ui_prompt_renamed,
  com_ui_prompt_update_error, com_ui_prompt_variables_list
- Add "sideEffects": false to librechat-data-provider package.json to
  enable tree-shaking of unused exports (types, constants, pure functions)

* fix: Reduce prompts panel spacing, align memory toggle with checkbox pattern

- Remove unnecessary wrapper div around AutoSendPrompt in PromptsAccordion,
  reducing vertical space between the toggle and the first prompt item
- Replace Memory panel's Switch toggle with Checkbox+Button pattern
  matching the prompts panel's AutoSendPrompt for visual consistency

* fix: Reduce gap between AutoSendPrompt and first prompt item

Change ChatGroupItem margin from my-2 to mb-2 to eliminate the
doubled spacing (gap-2 from parent + top margin from first item).
Restore wrapper div around AutoSendPrompt for right-alignment.

* fix: Restore prompt name on empty save, remove dead bodyProps from checkGlobalPromptShare

- PromptName: reset newName to name when save is cancelled due to empty
  or unchanged input, preventing blank title in read mode
- checkGlobalPromptShare: remove dead bodyProps config — Permissions.SHARE
  was not in the permissions array so the bodyProps rule was never evaluated.
  Per-resource share checks are handled by canAccessPromptGroupResource.

---------

Co-authored-by: Danny Avila <danny@librechat.ai>
danny-avila added a commit that referenced this pull request Mar 25, 2026
- Fix cross-user cache contamination: overrideCacheKey now handles
  userId-without-role case with its own cache key (#1)
- Add broad capability check before DB lookup in getConfig to prevent
  config existence enumeration (#2/#3)
- Move deleteConfigField fieldPath from request body to query parameter
  for proxy/load balancer compatibility (#5)
- Derive BaseSystemCapability from SystemCapabilities const instead of
  manual string union (#6)
- Return 201 on upsert creation, 200 on update (#11)
- Remove inline narration comments per AGENTS.md (#12)
- Type overrides as Partial<TCustomConfig> in DB methods and handler
  deps (#13)
- Replace double as-unknown-as casts in resolution.ts with generic
  deepMerge<T> (#14)
- Make override cache TTL injectable via AppConfigServiceDeps (#16)
- Add exhaustive never check in principalModel switch (#17)
danny-avila added a commit that referenced this pull request Mar 25, 2026
Service (service.ts):
- Restore HAS_DB_CONFIGS=false for base-only queries (no role/userId)
  so deployments with zero DB configs skip DB queries (#1)
- Resolve cache once at factory init instead of per-invocation (#8)
- Use BASE_CONFIG_PRINCIPAL_ID constant in overrideCacheKey (#10)
- Add JSDoc to clearAppConfigCache documenting stale-window (#4)
- Fix log message to not say "from YAML" (#14)

Admin handlers (config.ts):
- Use configVersion===1 for 201 vs 200, eliminating TOCTOU race (#2)
- Add Array.isArray guard on overrides body (#5)
- Import CapabilityUser from capabilities.ts, remove duplicate (#6)
- Replace as-unknown-as cast with targeted type assertion (#7)
- Add MAX_PATCH_ENTRIES=100 cap on entries array (#15)
- Reorder deleteConfigField to validate principalType first (#12)
- Export CapabilityUser from middleware/capabilities.ts

DB methods (config.ts):
- Remove isActive:true from patchConfigFields to prevent silent
  reactivation of disabled configs (#3)

Schema (config.ts):
- Change principalId from Schema.Types.Mixed to String (#11)

Tests:
- Add patchConfigField unsafe fieldPath rejection test (#9)
- Add base-only HAS_DB_CONFIGS=false test (#1)
- Update 201/200 tests to use configVersion instead of findConfig (#2)
danny-avila added a commit that referenced this pull request Mar 25, 2026
* ✨ feat: Add Config schema, model, and methods for role-based DB config overrides

Add the database foundation for principal-based configuration overrides
(user, group, role) in data-schemas. Includes schema with tenantId and
tenant isolation, CRUD methods, and barrel exports.

* 🔧 fix: Add shebang and enforce LF line endings for git hooks

The pre-commit hook was missing #!/bin/sh, and core.autocrlf=true was
converting it to CRLF, both causing "Exec format error" on Windows.
Add .gitattributes to force LF for .husky/* and *.sh files.

* ✨ feat: Add admin config API routes with section-level capability checks

Add /api/admin/config endpoints for managing per-principal config
overrides (user, group, role). Handlers in @librechat/api use DI pattern
with section-level hasConfigCapability checks for granular access control.

Supports full overrides replacement, per-field PATCH via dot-paths, field
deletion, toggle active, and listing.

* 🐛 fix: Move deleteConfigField fieldPath from URL param to request body

The path-to-regexp wildcard syntax (:fieldPath(*)) is not supported by
the version used in Express. Send fieldPath in the DELETE request body
instead, which also avoids URL-encoding issues with dotted paths.

* ✨ feat: Wire config resolution into getAppConfig with override caching

Add mergeConfigOverrides utility in data-schemas for deep-merging DB
config overrides into base AppConfig by priority order.

Update getAppConfig to query DB for applicable configs when role/userId
is provided, with short-TTL caching and a hasAnyConfigs feature flag
for zero-cost when no DB configs exist.

Also: add unique compound index on Config schema, pass userId from
config middleware, and signal config changes from admin API handlers.

* 🔄 refactor: Extract getAppConfig logic into packages/api as TS service

Move override resolution, caching strategy, and signalConfigChange from
api/server/services/Config/app.js into packages/api/src/app/appConfigService.ts
using the DI factory pattern (createAppConfigService). The JS file becomes
a thin wiring layer injecting loadBaseConfig, cache, and DB dependencies.

* 🧹 chore: Rename configResolution.ts to resolution.ts

* ✨ feat: Move admin types & capabilities to librechat-data-provider

Move SystemCapabilities, CapabilityImplications, and utility functions
(hasImpliedCapability, expandImplications) from data-schemas to
data-provider so they are available to external consumers like the
admin panel without a data-schemas dependency.

Add API-friendly admin types: TAdminConfig, TAdminSystemGrant,
TAdminAuditLogEntry, TAdminGroup, TAdminMember, TAdminUserSearchResult,
TCapabilityCategory, and CAPABILITY_CATEGORIES.

data-schemas re-exports these from data-provider and extends with
config-schema-derived types (ConfigSection, SystemCapability union).

Bump version to 0.8.500.

* feat: Add JSON-serializable admin config API response types to data-schemas

Add AdminConfig, AdminConfigListResponse, AdminConfigResponse, and
AdminConfigDeleteResponse types so both LibreChat API handlers and the
admin panel can share the same response contract. Bump version to 0.0.41.

* refactor: Move admin capabilities & types from data-provider to data-schemas

SystemCapabilities, CapabilityImplications, utility functions,
CAPABILITY_CATEGORIES, and admin API response types should not be in
data-provider as it gets compiled into the frontend bundle, exposing
the capability surface. Moved everything to data-schemas (server-only).

All consumers already import from @librechat/data-schemas, so no
import changes needed elsewhere. Consolidated duplicate AdminConfig
type (was in both config.ts and admin.ts).

* chore: Bump @librechat/data-schemas to 0.0.42

* refactor: Reorganize admin capabilities into admin/ and types/admin.ts

Split systemCapabilities.ts following data-schemas conventions:
- Types (BaseSystemCapability, SystemCapability, AdminConfig, etc.)
  → src/types/admin.ts
- Runtime code (SystemCapabilities, CapabilityImplications, utilities)
  → src/admin/capabilities.ts

Revert data-provider version to 0.8.401 (no longer modified).

* chore: Fix import ordering, rename appConfigService to service

- Rename app/appConfigService.ts → app/service.ts (directory provides context)
- Fix import order in admin/config.ts, types/admin.ts, types/config.ts
- Add naming convention to AGENTS.md

* feat: Add DB base config support (role/__base__)

- Add BASE_CONFIG_PRINCIPAL_ID constant for reserved base config doc
- getApplicableConfigs always includes __base__ in queries
- getAppConfig queries DB even without role/userId when DB configs exist
- Bump @librechat/data-schemas to 0.0.43

* fix: Address PR review issues for admin config

- Add listAllConfigs method; listConfigs endpoint returns all active
  configs instead of only __base__
- Normalize principalId to string in all config methods to prevent
  ObjectId vs string mismatch on user/group lookups
- Block __proto__ and all dunder-prefixed segments in field path
  validation to prevent prototype pollution
- Fix configVersion off-by-one: default to 0, guard pre('save') with
  !isNew, use $inc on findOneAndUpdate
- Remove unused getApplicableConfigs from admin handler deps

* fix: Enable tree-shaking for data-schemas, bump packages

- Switch data-schemas Rollup output to preserveModules so each source
  file becomes its own chunk; consumers (admin panel) can now import
  just the modules they need without pulling in winston/mongoose/etc.
- Add sideEffects: false to data-schemas package.json
- Bump data-schemas to 0.0.44, data-provider to 0.8.402

* feat: add capabilities subpath export to data-schemas

Adds `@librechat/data-schemas/capabilities` subpath export so browser
consumers can import BASE_CONFIG_PRINCIPAL_ID and capability constants
without pulling in Node.js-only modules (winston, async_hooks, etc.).

Bump version to 0.0.45.

* fix: include dist/ in data-provider npm package

Add explicit files field so npm includes dist/types/ in the published
package. Without this, the root .gitignore exclusion of dist/ causes
npm to omit type declarations, breaking TypeScript consumers.

* chore: bump librechat-data-provider to 0.8.403

* feat: add GET /api/admin/config/base for raw AppConfig

Returns the full AppConfig (YAML + DB base merged) so the admin panel
can display actual config field values and structure. The startup config
endpoint (/api/config) returns TStartupConfig which is a different shape
meant for the frontend app.

* chore: imports order

* fix: address code review findings for admin config

Critical:
- Fix clearAppConfigCache: was deleting from wrong cache store (CONFIG_STORE
  instead of APP_CONFIG), now clears BASE and HAS_DB_CONFIGS keys
- Eliminate race condition: patchConfigField and deleteConfigField now use
  atomic MongoDB $set/$unset with dot-path notation instead of
  read-modify-write cycles, removing the lost-update bug entirely
- Add patchConfigFields and unsetConfigField atomic DB methods

Major:
- Reorder cache check before principal resolution in getAppConfig so
  getUserPrincipals DB query only fires on cache miss
- Replace '' as ConfigSection with typed BROAD_CONFIG_ACCESS constant
- Parallelize capability checks with Promise.all instead of sequential
  awaits in for loops
- Use loose equality (== null) for cache miss check to handle both null
  and undefined returns from cache implementations
- Set HAS_DB_CONFIGS_KEY to true on successful config fetch

Minor:
- Remove dead pre('save') hook from config schema (all writes use
  findOneAndUpdate which bypasses document hooks)
- Consolidate duplicate type imports in resolution.ts
- Remove dead deepGet/deepSet/deepUnset functions (replaced by atomic ops)
- Add .sort({ priority: 1 }) to getApplicableConfigs query
- Rename _impliedBy to impliedByMap

* fix: self-referencing BROAD_CONFIG_ACCESS constant

* fix: replace type-cast sentinel with proper null parameter

Update hasConfigCapability to accept ConfigSection | null where null
means broad access check (MANAGE_CONFIGS or READ_CONFIGS only).
Removes the '' as ConfigSection type lie from admin config handlers.

* fix: remaining review findings + add tests

- listAllConfigs accepts optional { isActive } filter so admin listing
  can show inactive configs (#9)
- Standardize session application to .session(session ?? null) across
  all config DB methods (#15)
- Export isValidFieldPath and getTopLevelSection for testability
- Add 38 tests across 3 spec files:
  - config.spec.ts (api): path validation, prototype pollution rejection
  - resolution.spec.ts: deep merge, priority ordering, array replacement
  - config.spec.ts (data-schemas): full CRUD, ObjectId normalization,
    atomic $set/$unset, configVersion increment, toggle, __base__ query

* fix: address second code review findings

- Fix cross-user cache contamination: overrideCacheKey now handles
  userId-without-role case with its own cache key (#1)
- Add broad capability check before DB lookup in getConfig to prevent
  config existence enumeration (#2/#3)
- Move deleteConfigField fieldPath from request body to query parameter
  for proxy/load balancer compatibility (#5)
- Derive BaseSystemCapability from SystemCapabilities const instead of
  manual string union (#6)
- Return 201 on upsert creation, 200 on update (#11)
- Remove inline narration comments per AGENTS.md (#12)
- Type overrides as Partial<TCustomConfig> in DB methods and handler
  deps (#13)
- Replace double as-unknown-as casts in resolution.ts with generic
  deepMerge<T> (#14)
- Make override cache TTL injectable via AppConfigServiceDeps (#16)
- Add exhaustive never check in principalModel switch (#17)

* fix: remaining review findings — tests, rename, semantics

- Rename signalConfigChange → markConfigsDirty with JSDoc documenting
  the stale-window tradeoff and overrideCacheTtl knob
- Fix DEFAULT_OVERRIDE_CACHE_TTL naming convention
- Add createAppConfigService tests (14 cases): cache behavior, feature
  flag, cross-user key isolation, fallback on error, markConfigsDirty
- Add admin handler integration tests (13 cases): auth ordering,
  201/200 on create/update, fieldPath from query param, markConfigsDirty
  calls, capability checks

* fix: global flag corruption + empty overrides auth bypass

- Remove HAS_DB_CONFIGS_KEY=false optimization: a scoped query returning
  no configs does not mean no configs exist globally. Setting the flag
  false from a per-principal query short-circuited all subsequent users.
- Add broad manage capability check before section checks in
  upsertConfigOverrides: empty overrides {} no longer bypasses auth.

* test: add regression and invariant tests for config system

Regression tests:
- Bug 1: User A's empty result does not short-circuit User B's overrides
- Bug 2: Empty overrides {} returns 403 without MANAGE_CONFIGS

Invariant tests (applied across ALL handlers):
- All 5 mutation handlers call markConfigsDirty on success
- All 5 mutation handlers return 401 without auth
- All 5 mutation handlers return 403 without capability
- All 3 read handlers return 403 without capability

* fix: third review pass — all findings addressed

Service (service.ts):
- Restore HAS_DB_CONFIGS=false for base-only queries (no role/userId)
  so deployments with zero DB configs skip DB queries (#1)
- Resolve cache once at factory init instead of per-invocation (#8)
- Use BASE_CONFIG_PRINCIPAL_ID constant in overrideCacheKey (#10)
- Add JSDoc to clearAppConfigCache documenting stale-window (#4)
- Fix log message to not say "from YAML" (#14)

Admin handlers (config.ts):
- Use configVersion===1 for 201 vs 200, eliminating TOCTOU race (#2)
- Add Array.isArray guard on overrides body (#5)
- Import CapabilityUser from capabilities.ts, remove duplicate (#6)
- Replace as-unknown-as cast with targeted type assertion (#7)
- Add MAX_PATCH_ENTRIES=100 cap on entries array (#15)
- Reorder deleteConfigField to validate principalType first (#12)
- Export CapabilityUser from middleware/capabilities.ts

DB methods (config.ts):
- Remove isActive:true from patchConfigFields to prevent silent
  reactivation of disabled configs (#3)

Schema (config.ts):
- Change principalId from Schema.Types.Mixed to String (#11)

Tests:
- Add patchConfigField unsafe fieldPath rejection test (#9)
- Add base-only HAS_DB_CONFIGS=false test (#1)
- Update 201/200 tests to use configVersion instead of findConfig (#2)

* fix: add read handler 401 invariant tests + document flag behavior

- Add invariant: all 3 read handlers return 401 without auth
- Document on markConfigsDirty that HAS_DB_CONFIGS stays true after
  all configs are deleted until clearAppConfigCache or restart

* fix: remove HAS_DB_CONFIGS false optimization entirely

getApplicableConfigs([]) only queries for __base__, not all configs.
A deployment with role/group configs but no __base__ doc gets the
flag poisoned to false by a base-only query, silently ignoring all
scoped overrides. The optimization is not safe without a comprehensive
Config.exists() check, which adds its own DB cost. Removed entirely.

The flag is now write-once-true (set when configs are found or by
markConfigsDirty) and only cleared by clearAppConfigCache/restart.

* chore: reorder import statements in app.js for clarity

* refactor: remove HAS_DB_CONFIGS_KEY machinery entirely

The three-state flag (false/null/true) was the source of multiple bugs
across review rounds. Every attempt to safely set it to false was
defeated by getApplicableConfigs querying only a subset of principals.

Removed: HAS_DB_CONFIGS_KEY constant, all reads/writes of the flag,
markConfigsDirty (now a no-op concept), notifyChange wrapper, and all
tests that seeded false manually.

The per-user/role TTL cache (overrideCacheTtl, default 60s) is the
sole caching mechanism. On cache miss, getApplicableConfigs queries
the DB. This is one indexed query per user per TTL window — acceptable
for the config override use case.

* docs: rewrite admin panel remaining work with current state

* perf: cache empty override results to avoid repeated DB queries

When getApplicableConfigs returns no configs for a principal, cache
baseConfig under their override key with TTL. Without this, every
user with no per-principal overrides hits MongoDB on every request
after the 60s cache window expires.

* fix: add tenantId to cache keys + reject PUBLIC principal type

- Include tenantId in override cache keys to prevent cross-tenant
  config contamination. Single-tenant deployments (tenantId undefined)
  use '_' as placeholder — no behavior change for them.
- Reject PrincipalType.PUBLIC in admin config validation — PUBLIC has
  no PrincipalModel and is never resolved by getApplicableConfigs,
  so config docs for it would be dead data.
- Config middleware passes req.user.tenantId to getAppConfig.

* fix: fourth review pass findings

DB methods (config.ts):
- findConfigByPrincipal accepts { includeInactive } option so admin
  GET can retrieve inactive configs (#5)
- upsertConfig catches E11000 duplicate key on concurrent upserts and
  retries without upsert flag (#2)
- unsetConfigField no longer filters isActive:true, consistent with
  patchConfigFields (#11)
- Typed filter objects replace Record<string, unknown> (#12)

Admin handlers (config.ts):
- patchConfigField: serial broad capability check before Promise.all
  to pre-warm ALS principal cache, preventing N parallel DB calls (#3)
- isValidFieldPath rejects leading/trailing dots and consecutive
  dots (#7)
- Duplicate fieldPaths in patch entries return 400 (#8)
- DEFAULT_PRIORITY named constant replaces hardcoded 10 (#14)
- Admin getConfig and patchConfigField pass includeInactive to
  findConfigByPrincipal (#5)
- Route import uses barrel instead of direct file path (#13)

Resolution (resolution.ts):
- deepMerge has MAX_MERGE_DEPTH=10 guard to prevent stack overflow
  from crafted deeply nested configs (#4)

* fix: final review cleanup

- Remove ADMIN_PANEL_REMAINING.md (local dev notes with Windows paths)
- Add empty-result caching regression test
- Add tenantId to AdminConfigDeps.getAppConfig type
- Restore exhaustive never check in principalModel switch
- Standardize toggleConfigActive session handling to options pattern

* fix: validate priority in patchConfigField handler

Add the same non-negative number validation for priority that
upsertConfigOverrides already has. Without this, invalid priority
values could be stored via PATCH and corrupt merge ordering.

* chore: remove planning doc from PR

* fix: correct stale cache key strings in service tests

* fix: clean up service tests and harden tenant sentinel

- Remove no-op cache delete lines from regression tests
- Change no-tenant sentinel from '_' to '__default__' to avoid
  collision with a real tenant ID when multi-tenancy is enabled
- Remove unused CONFIG_STORE from AppConfigServiceDeps

* chore: bump @librechat/data-schemas to 0.0.46

* fix: block prototype-poisoning keys in deepMerge

Skip __proto__, constructor, and prototype keys during config merge
to prevent prototype pollution via PUT /api/admin/config overrides.
danny-avila added a commit that referenced this pull request Mar 27, 2026
… DRY

- #2: getAllServerConfigs now always uses getBaseServerConfigs (cached via
  readThroughCacheAll) instead of bypassing it when configServers is present.
  Extracts user-DB entries from cached base by diffing against YAML keys
  to maintain YAML → Config → User DB merge order without extra MongoDB calls.

- #3: Add configServers param to ToolDiscoveryOptions and thread it through
  discoverServerTools → getServerConfig so config-source servers are
  discoverable during OAuth reconnection flows.

- #6: Replace inline import() type annotations in context.ts with proper
  import type { ParsedServerConfig } per AGENTS.md conventions.

- #7: Extract resolveConfigServers(req) helper in MCP.js and use it from
  handleTools.js and client.js, eliminating the duplicated 6-line config
  resolution pattern.

- #10: Restore removed "why" comment explaining getLoaded() vs getAll()
  choice in getMCPSetupData — documents non-obvious correctness constraint.

- #11: Fix incomplete JSDoc param type on resolveAllMcpConfigs.
danny-avila added a commit that referenced this pull request Apr 16, 2026
- Fail-closed when userId is absent: filter rejects all shared skills
  instead of passing them through unfiltered (Codex P1)
- Validate Mongoose Map key characters (reject . and $) in controller
  to return 400 instead of a 500 from schema validation (Codex P2)
- Block toggle while initial skill states query is loading to prevent
  overwriting server-side overrides with an empty snapshot (Codex P2)
- Extract shared SkillToggle component, eliminating duplicate toggle
  markup in SkillListItem and SkillDetail (Finding #3)
- Move skill state query/mutation hooks from Favorites.ts to
  Skills/queries.ts per feature-directory convention (Finding #4)
- Fix hardcoded English aria-label in SkillListItem by passing the
  localized string from the parent SkillList (Finding #5)
- Fix inline arrow in SkillList render loop: pass stable callback
  reference so SkillListItem memo() is not invalidated (Finding #1)
- Extract toRecord() helper in controller to DRY the Map-to-Object
  conversion (Finding #6)
- Remove Promise.resolve wrapping synchronous config read (Finding #8)
- Remove unused TUpdateSkillStatesRequest type (Finding #12)
danny-avila added a commit that referenced this pull request Apr 16, 2026
- Fail-closed when userId is absent: filter rejects all shared skills
  instead of passing them through unfiltered (Codex P1)
- Validate Mongoose Map key characters (reject . and $) in controller
  to return 400 instead of a 500 from schema validation (Codex P2)
- Block toggle while initial skill states query is loading to prevent
  overwriting server-side overrides with an empty snapshot (Codex P2)
- Extract shared SkillToggle component, eliminating duplicate toggle
  markup in SkillListItem and SkillDetail (Finding #3)
- Move skill state query/mutation hooks from Favorites.ts to
  Skills/queries.ts per feature-directory convention (Finding #4)
- Fix hardcoded English aria-label in SkillListItem by passing the
  localized string from the parent SkillList (Finding #5)
- Fix inline arrow in SkillList render loop: pass stable callback
  reference so SkillListItem memo() is not invalidated (Finding #1)
- Extract toRecord() helper in controller to DRY the Map-to-Object
  conversion (Finding #6)
- Remove Promise.resolve wrapping synchronous config read (Finding #8)
- Remove unused TUpdateSkillStatesRequest type (Finding #12)
danny-avila added a commit that referenced this pull request Apr 17, 2026
Address the followup review findings:

- Add optional `accessibleSkillIds` param to loadSkillStates so the
  helper short-circuits to defaults when no skills are accessible.
  All three controllers drop the residual 7-line conditional wrapper
  in favor of a single destructured call (Review #2)

- Remove the unreachable `typeof key !== 'string'` check from
  validateSkillStatesPayload: Object.entries always yields string
  keys per the JS spec (Review #3)

- Replace the two `as unknown as` agent casts in the injectSkillCatalog
  tests with a `makeAgent()` factory typed directly as the function's
  parameter shape (Review #4)

- Tighten the MAX_CATALOG_PAGES assertion from `toBeLessThanOrEqual(11)`
  to `toHaveBeenCalledTimes(10)` — the loop deterministically makes
  exactly 10 page fetches before hitting the cap (Review #1)

- Rewrite the parallel-execution test for pruneOrphanSkillStates using
  deferred promises instead of microtask-order assertions. The test
  now inspects `toHaveBeenCalledTimes(1)` on both mocks after a single
  Promise.resolve() yield, pinning Promise.all usage without relying
  on push-order into a shared array (Review #5)

- Evict stale writeQueue entries on user change via a module-scoped
  `lastSeenUserId` sentinel. When a different user's toggle is the
  first one after a logout/login, the previous user's queue entry is
  deleted. Keeps the Map bounded without adding hook-instance effect
  cleanup (Review #6)
danny-avila added a commit that referenced this pull request Apr 17, 2026
…efaults (#12692)

* feat: per-user skill active/inactive toggle with ownership-aware defaults

- Add `skillStates` map (Record<string, boolean>) to user schema for
  per-user active/inactive overrides on skills
- Add `defaultActiveOnShare` to interface.skills config (default: false)
  so admins can control whether shared skills auto-activate
- Add GET/POST /api/user/settings/skills/active endpoints with validation
- Add React Query hooks with optimistic mutations for skill states
- Add useSkillActiveState hook with ownership-aware resolution:
  owned skills default active, shared skills default inactive
- Add toggle switch UI to SkillListItem and SkillDetail components
- Filter inactive skills in injectSkillCatalog before agent injection
- Add localization keys for active/inactive labels

* fix: use Record instead of Map for IUser.skillStates

Mongoose .lean() flattens Map to a plain object, causing type
incompatibility with IUser in methods that return lean documents.

* fix: address review findings for skill active states

- Fail-closed when userId is absent: filter rejects all shared skills
  instead of passing them through unfiltered (Codex P1)
- Validate Mongoose Map key characters (reject . and $) in controller
  to return 400 instead of a 500 from schema validation (Codex P2)
- Block toggle while initial skill states query is loading to prevent
  overwriting server-side overrides with an empty snapshot (Codex P2)
- Extract shared SkillToggle component, eliminating duplicate toggle
  markup in SkillListItem and SkillDetail (Finding #3)
- Move skill state query/mutation hooks from Favorites.ts to
  Skills/queries.ts per feature-directory convention (Finding #4)
- Fix hardcoded English aria-label in SkillListItem by passing the
  localized string from the parent SkillList (Finding #5)
- Fix inline arrow in SkillList render loop: pass stable callback
  reference so SkillListItem memo() is not invalidated (Finding #1)
- Extract toRecord() helper in controller to DRY the Map-to-Object
  conversion (Finding #6)
- Remove Promise.resolve wrapping synchronous config read (Finding #8)
- Remove unused TUpdateSkillStatesRequest type (Finding #12)

* fix: forward tabIndex on SkillToggle to preserve list keyboard nav

The original inline toggle had tabIndex={-1} so the row itself
remained the sole tab target. The extraction into SkillToggle
dropped this prop, making every list toggle a tab stop. Add an
optional tabIndex prop and pass -1 from SkillListItem.

* fix: plumb skillStates to all agent entry points, isolate toggle keydown

- Add skillStates/defaultActiveOnShare loading to openai.js and
  responses.js controllers so shared-skill activation is respected
  across all agent entry points, not just initialize.js (Codex P1)
- Stop keydown propagation on SkillToggle so Enter/Space does not
  bubble to the parent row's navigation handler (Codex P2)

* fix: paginate catalog fetch and serialize toggle writes

- Paginate listSkillsByAccess (up to 10 pages of 100) until the active
  catalog quota is filled, so inactive shared skills in recent positions
  do not starve active owned skills past the first page (Codex P1)
- Extend listSkillsByAccess interface with cursor/has_more/after for
  catalog pagination
- Serialize skill-state writes via a ref queue: one in-flight request
  at a time, with the latest desired state sent when the previous one
  settles. Prevents last-response-wins races where an older request
  overwrites newer toggles (Codex P2)

* fix: share write queue across hook instances, block toggle on fetch error

- Move the write queue from a per-instance useRef to a module-scoped
  object so every mount of useSkillActiveState (SkillList, SkillDetail,
  etc.) serializes against the same in-flight slot. Prior per-instance
  queues allowed two components to race full-map POSTs (Codex P1)
- Extend the toggle guard beyond isLoading: also block when isError is
  true or data is undefined. Prevents a failed GET from seeding a
  toggle with an empty baseline that would wipe server-side overrides
  on the next successful POST (Codex P1)

* fix: stale closure, orphan cleanup, and cap-error UX

- Read toggle baseline from React Query cache via queryClient.getQueryData
  instead of the captured skillStates closure. The closure can be stale
  between onMutate's setQueryData and the next render, so rapid successive
  toggles would build on old state and drop earlier changes (Codex P1)
- Surface the MAX_SKILL_STATES_EXCEEDED error code with a specific toast
  key (com_ui_skill_states_limit) so users understand the 200-cap rather
  than seeing a generic error
- Prune orphaned entries (skillIds whose Skill doc no longer exists) on
  both GET and POST in SkillStatesController. Self-heals over time
  without needing cascade-delete hooks or a migration job. Uses one
  indexed Skill._id query per request

* test: pin skill active-state precedence with unit tests

Extract the active-state resolution logic from a closure inside
injectSkillCatalog into an exported resolveSkillActive helper, then
cover every branch of the precedence matrix:

- Fails closed when userId is absent (even with defaultActiveOnShare=true)
- Explicit override wins over ownership and config (both true and false)
- Owned skills default to active when no override is set
- Shared skills default to defaultActiveOnShare value
- Undefined skillStates behaves identically to an empty object
- defaultActiveOnShare defaults to false when omitted
- Owned skills ignore defaultActiveOnShare entirely

Closes Finding #2 from the pre-rebase comprehensive review. Mirrors
the existing scopeSkillIds test style; injectSkillCatalog now calls
resolveSkillActive instead of inlining the closure.

* refactor: limit skill active toggle to detail header, drop label

- Remove the per-row toggle from SkillListItem and the active-state
  plumbing (hook call, isSkillEnabled/onToggleEnabled/toggleAriaLabel
  props) from SkillList. The detail view is now the single place to
  change a skill's active state
- Drop dim/muted styling for inactive skills in the sidebar: without
  a control there, the visual indication has nowhere to land
- Resize SkillToggle to match neighbor buttons: outer h-9 container,
  h-6 w-11 track with size-5 knob, no label span. The 'Active' /
  'Inactive' text that accompanied the detail-view toggle is removed
- Remove the now-unused label prop and tabIndex prop (the tabIndex
  existed only for the list-row context) from SkillToggle. Drop the
  onKeyDown stopPropagation for the same reason
- Remove now-orphaned com_ui_skill_active / com_ui_skill_inactive
  translation keys

* style: shrink SkillToggle track to h-5 w-9 with size-4 knob

Container stays at h-9 to match neighbor button heights. The toggle
track itself drops from h-6 w-11 to h-5 w-9, with a size-4 knob
travelling 1.125rem on activation. Visually lighter inside the row.

* fix: remove redundant skillStates entries that match the resolved default

When a toggle lands on the ownership/config default, delete the key
from the map instead of persisting `{id: defaultValue}`. Without this,
a user toggling a skill off and back on would leave `{id: true}` for
an owned skill (whose default is already true), silently consuming a
slot against the 200-entry cap. Repeated round-trip toggles could
exhaust the quota with zero meaningful overrides (Codex P2).

Preserves the exceptions-list invariant that the runtime-resolution
design depends on.

* fix: prune before enforcing skill-state cap; reject non-ObjectId keys

Reorder the update controller so pruneOrphans runs before the 200-cap
check. Without this, a user near the cap with some orphaned entries
(skills deleted since their last GET) could send a payload that would
pass after pruning but gets rejected by the raw-size check first.

Add a sanity cap on raw payload size (2 * MAX_SKILL_STATES) so abusive
inputs do not reach the DB query, and enforce the real cap on the
pruned result instead.

Harden pruneOrphans: the earlier early-return path could pass
non-ObjectId keys through unchanged. Now only valid ObjectIds are
returned, and the Skill-model-unavailable fallback filters by format.

Also add isValidObjectIdString validation at the input boundary so
malformed (but otherwise non-Mongo-unsafe) keys never reach persistence
(Codex P2 x2).

* fix: enforce active filter at execute time, prune revoked shares, scope queue per user

P1: injectSkillCatalog now returns activeSkillIds (the filtered set
that appears in the catalog). initializeAgent uses that set as the
stored accessibleSkillIds on the initialized agent, so getSkillByName
at runtime cannot resolve a deactivated skill — even if the LLM
hallucinates a name or the user invokes by direct-invocation shorthand.
Previously the executor authorized against the full ACL set, bypassing
the active-state guarantee (Codex P1).

P2: pruneOrphans now checks user access via findAccessibleResources
in addition to skill existence. When a share is revoked, the user's
skillStates entry for that skill had no cleanup path and silently
consumed the 200-cap. Self-heals on both GET and POST. One extra ACL
query per settings read/write; scoped to a single user so no N-user
amplification (Codex P2).

P2: the write queue moves from a single module-scoped object to a Map
keyed by userId. Logout/login in the same tab can no longer flush the
previous user's pending snapshot under the new session's auth. Each
userId gets its own pending/inFlight slot; the in-flight request
retains its original auth via the cookie already attached when sent,
so the race window closes (Codex P2).

* refactor: extract skillStates helpers to packages/api; add tests; polish

Address the remaining valid findings from the comprehensive review:

- Extract toRecord, loadSkillStates, validateSkillStatesPayload, and
  pruneOrphanSkillStates into packages/api/src/skills/skillStates.ts
  as TypeScript. The controller in /api shrinks to a ~90-line thin
  wrapper that builds live dependency adapters for Mongoose + the
  permission service (Review #2 DRY, #3 workspace boundary)

- Replace the triplicated 12-line skillStates loading block in
  initialize.js, openai.js, and responses.js with a single call to
  loadSkillStates from @librechat/api. One helper, three sites

- Swap console.error for the project logger in the controller
  (Review #7)

- Remove the redundant INVALID_KEY_PATTERN regex: a valid ObjectId
  cannot contain . or $, so isValidObjectIdString already covers it
  (Review #11)

- Parameterize the 200-cap error toast with {{0}} interpolation
  driven by the error response's `limit` field, so future changes to
  MAX_SKILL_STATES update the UI message automatically (Review #12)

- Add 24 unit tests for the new skillStates helpers (toRecord,
  resolveDefaultActiveOnShare, loadSkillStates, validateSkillStates-
  Payload, pruneOrphanSkillStates) covering success paths, malformed
  input, cap boundaries, and parallel-query behavior (Review #4)

- Add 10 tests for injectSkillCatalog pagination covering empty
  accessible set, missing listSkillsByAccess, single-page filter,
  owned-vs-shared defaults, explicit-override precedence, multi-page
  collection, MAX_CATALOG_PAGES safety cap, early termination on
  has_more=false, additional_instructions injection, and fail-closed
  without userId (Review #5)

Total test count: 60 (was 26 on this surface).

* fix: rename skillStates ValidationError to avoid barrel-export collision

packages/api/src/types/error.ts already exports a ValidationError
(MongooseError extension). Re-exporting a different shape from
skills/skillStates.ts through the skills barrel caused TS2308 in CI
because the root index re-exports both. Rename to
SkillStatesValidationError to keep the exports disjoint.

* refactor: tighten tests and absorb caller guard into loadSkillStates

Address the followup review findings:

- Add optional `accessibleSkillIds` param to loadSkillStates so the
  helper short-circuits to defaults when no skills are accessible.
  All three controllers drop the residual 7-line conditional wrapper
  in favor of a single destructured call (Review #2)

- Remove the unreachable `typeof key !== 'string'` check from
  validateSkillStatesPayload: Object.entries always yields string
  keys per the JS spec (Review #3)

- Replace the two `as unknown as` agent casts in the injectSkillCatalog
  tests with a `makeAgent()` factory typed directly as the function's
  parameter shape (Review #4)

- Tighten the MAX_CATALOG_PAGES assertion from `toBeLessThanOrEqual(11)`
  to `toHaveBeenCalledTimes(10)` — the loop deterministically makes
  exactly 10 page fetches before hitting the cap (Review #1)

- Rewrite the parallel-execution test for pruneOrphanSkillStates using
  deferred promises instead of microtask-order assertions. The test
  now inspects `toHaveBeenCalledTimes(1)` on both mocks after a single
  Promise.resolve() yield, pinning Promise.all usage without relying
  on push-order into a shared array (Review #5)

- Evict stale writeQueue entries on user change via a module-scoped
  `lastSeenUserId` sentinel. When a different user's toggle is the
  first one after a logout/login, the previous user's queue entry is
  deleted. Keeps the Map bounded without adding hook-instance effect
  cleanup (Review #6)

* fix(test): mock loadSkillStates in openai and responses controller specs

The prior refactor replaced the inline 12-line skillStates loading
block with a call to loadSkillStates from @librechat/api. Both
controller spec files mock @librechat/api as a flat object, so any
new named import from that package is undefined in the test env.
Calling `await loadSkillStates(...)` threw before recordCollectedUsage
ran, surfacing as "undefined is not iterable" on the test's array
destructure of `mockRecordCollectedUsage.mock.calls[0]`.

Add the missing mock to both spec files alongside the existing
scopeSkillIds stub.

* fix: abandon stale skillStates write queues on user switch

Close the cross-session leak window where an in-flight flush loop
still holds a reference to a previous user's queue: it could fire its
next mutateAsync under the new session's auth cookies and persist
the stale snapshot to the new user's document (Codex P1).

Add an `abandoned` flag on `WriteQueue`. Three mechanisms cooperate:

- `getWriteQueue` marks every non-active queue abandoned when the
  user differs from the last-seen identity (pre-existing eviction
  site, now more aggressive).
- A `useEffect` on `userId` calls the same abandonment pass on every
  render with a new active identity, covering the window between
  logout/login and the new user's first toggle (when `getWriteQueue`
  would otherwise not fire).
- The flush loop checks `!queue.abandoned` in its while condition so
  the second and later iterations exit without firing another
  `mutateAsync` after the session changes.

The first iteration's in-flight request (already dispatched under the
original user's cookies) still runs to completion or failure on its
own — only the subsequent iterations, which are the dangerous ones,
are blocked.
danny-avila added a commit that referenced this pull request Apr 17, 2026
…ing (#12649)

* feat: Skill runtime integration — catalog injection, tool registration, execute handler

Wires the @librechat/agents SkillTool primitive into LibreChat's agent runtime:

**Enums:**
- Add `skills` to AgentCapabilities + defaultAgentCapabilities

**Data layer:**
- Add `getSkillByName(name, accessibleIds)` — compound query that
  combines name lookup + ACL check in one findOne

**Agent initialization (packages/api/src/agents/initialize.ts):**
- Accept `accessibleSkillIds` param and `listSkillsByAccess` db method
- Query accessible skills, format catalog via `formatSkillCatalog()`,
  append to `additional_instructions` (appears in agent system prompt)
- Register `SkillToolDefinition` + `createSkillTool()` when catalog
  is non-empty (tool appears in model's tool list)
- Store `accessibleSkillIds` and `skillCount` on InitializedAgent

**Execute handler (packages/api/src/agents/handlers.ts):**
- Add `getSkillByName` to `ToolExecuteOptions`
- `handleSkillToolCall()` intercepts `Constants.SKILL_TOOL`:
  extracts skillName, loads body from DB with ACL check,
  substitutes $ARGUMENTS, returns ToolExecuteResult with
  injectedMessages (skill body as isMeta user message)

**Caller wiring:**
- initialize.js: query skill IDs via findAccessibleResources,
  pass to initializeAgent + store on agentToolContexts,
  add getSkillByName to toolExecuteOptions,
  pass accessibleSkillIds through loadTools configurable
- openai.js + responses.js: same pattern for their flows

Requires @librechat/agents >= 3.1.65 (PR #91 exports).

* feat: Skills toggle in tools menu + backend capability gating

Frontend:
- Add skills?: boolean to TEphemeralAgent type
- Add LAST_SKILLS_TOGGLE_ to LocalStorageKeys for persistence
- Add skillsEnabled to useAgentCapabilities hook
- Add skills useToolToggle to BadgeRowContext with localStorage init
- New Skills.tsx badge component (Scroll icon, cyan theme,
  permission-gated via PermissionTypes.SKILLS)
- Add skills entry to ToolsDropdown with toggle + pin
- Render Skills badge in BadgeRow ephemeral section

Backend:
- Extract injectSkillCatalog() into packages/api/src/agents/skills.ts
  (reduces initializeAgent module size, reusable helper)
- initializeAgent delegates to helper instead of inline block
- Capability-gate the findAccessibleResources query:
  - Agents endpoint: checks AgentCapabilities.skills in admin config
  - OpenAI/Responses controllers: checks ephemeralAgent.skills toggle
- ACL query runs once per run, result shared across all agents

* refactor: remove createSkillTool() instance from injectSkillCatalog

SkillTool is event-driven only. The tool definition in toolDefinitions
is sufficient for the LLM to see the tool schema. No tool instance is
needed since the host handler intercepts via ON_TOOL_EXECUTE before
tool.invoke() is ever called.

Removes tools from InjectSkillCatalogParams/Result, drops the
createSkillTool import.

* feat: skill file priming, bash tool, and invoked skills state

Multi-file skill support:
- New primeSkillFiles() helper (packages/api/src/agents/skillFiles.ts)
  uploads skill files + SKILL.md body to code execution environment
- handleSkillToolCall primes files on invocation when skill.fileCount > 0,
  returns session info as artifact so ToolNode stores the session
- Skill-primed files available to subsequent bash/code tool calls

Bash tool auto-registration:
- BashExecutionToolDefinition added alongside SkillToolDefinition when
  skills are enabled, giving the model a bash tool for running scripts

Conversation state:
- Add invokedSkillIds field to conversation schema (Mongoose + Zod)
- handleSkillToolCall updates conversation with $addToSet on success
- Enables re-priming skill files on subsequent runs (future)

Dependency wiring:
- Pass listSkillFiles, getStrategyFunctions, uploadCodeEnvFile,
  updateConversation through ToolExecuteOptions
- Pass req and codeApiKey through mergedConfigurable
- All three controller entry points wired (initialize.js, openai.js,
  responses.js)

* fix: load bash_tool instance in loadToolsForExecution, remove file listing

- Add createBashExecutionTool to loadToolsForExecution alongside PTC/ToolSearch
  pattern: loads CODE_API_KEY, creates bash tool instance on demand
- Add BASH_TOOL and SKILL_TOOL to specialToolNames set so they don't go
  through the generic loadTools path (bash is created here, skill is
  intercepted in handler before tool.invoke)
- Remove file name listing from skill content text — it's the skill
  author's responsibility to disclose files in SKILL.md, not the framework

* feat: batch upload for skill files, replace sequential uploads

- Add batchUploadCodeEnvFiles() to crud.js: single POST to /upload/batch
  with all files in one multipart request, returns shared session_id
- Rewrite primeSkillFiles to collect all streams (SKILL.md + bundled files)
  then do one batch upload instead of N sequential uploads
- Replace uploadCodeEnvFile with batchUploadCodeEnvFiles across all callers
  (handlers.ts, initialize.js, openai.js, responses.js)

* refactor: remove invokedSkillIds from conversation schema

Skills aren't re-loaded between runs, so conversation-level state for
invoked skills doesn't help. Skill state will live on messages instead
(like tool_search discoveredTools and summaries), enabling in-place
re-injection on follow-up runs.

Removes invokedSkillIds from: convo Mongoose schema, IConversation
interface, Zod schema, ToolExecuteOptions.updateConversation, and
all three caller wiring points.

* feat: smart skill file re-priming with session freshness checking

Schema:
- Add codeEnvIdentifier field to ISkillFile (type + Mongoose schema)
- Add updateSkillFileCodeEnvIds batch method (uses tenantSafeBulkWrite)
- Export checkIfActive from Code/process.js

Extraction:
- Add extractInvokedSkillsFromHistory() to run.ts — scans message
  history for AIMessage tool_calls where name === 'skill', extracts
  skillName args. Follows same pattern as extractDiscoveredToolsFromHistory.

Smart re-priming in primeSkillFiles:
- Before batch uploading, checks if existing codeEnvIdentifiers are
  still active via getSessionInfo + checkIfActive (23h threshold)
- If session is still active, returns cached references (zero uploads)
- If stale or missing, batch-uploads everything and persists new
  identifiers on SkillFile documents (fire-and-forget)
- Single session check covers all files (batch shares one session_id)

Wiring:
- Pass getSessionInfo, checkIfActive, updateSkillFileCodeEnvIds
  through ToolExecuteOptions and all three controller entry points

* feat: wire skill file re-priming at run start via initialSessions

Flow:
1. initialize.js creates primeInvokedSkills callback with all deps
2. client.js calls it with message history before createRun
3. extractInvokedSkillsFromHistory scans for skill tool calls
4. For each invoked skill with files, primeSkillFiles uploads/checks
5. Returns initialSessions map passed to createRun
6. createRun passes initialSessions to Run.create (via RunConfig)
7. Run constructor seeds Graph.sessions, making skill files available
   to subsequent bash/code tool calls via ToolNode session injection

Requires @librechat/agents with initialSessions on RunConfig (PR #94).

* refactor: use CODE_EXECUTION_TOOLS set for code tool checks

Import CODE_EXECUTION_TOOLS from @librechat/agents and replace inline
constant checks in handlers.ts and callbacks.js. Fixes missing bash
tool coverage in the session context injection (handlers.ts) and code
output processing (callbacks.js).

* refactor: move primeInvokedSkills to packages/api, add skill body re-injection

Moves primeInvokedSkills from an inline closure in initialize.js (with
dynamic requires) to a proper exported function in packages/api
skillFiles.ts with explicit typed dependencies.

Key changes:
- primeInvokedSkills now returns both initialSessions (for file priming)
  AND injectedMessages (skill bodies for context continuity)
- createRun accepts invokedSkillMessages and appends skill bodies to
  systemContent so the model retains skill instructions across runs
- initialize.js calls the packaged function with all deps passed explicitly
- client.js passes both initialSessions and injectedMessages to createRun

* fix: move dynamic requires to top-level module imports

Move primeInvokedSkills, getStrategyFunctions, batchUploadCodeEnvFiles,
getSessionInfo, and checkIfActive from inline requires to top-level
module requires where they belong.

* refactor: skill body reconstruction via formatAgentMessages, not systemContent

Replaces the lazy systemContent approach with proper message-level
reconstruction:

SDK (formatAgentMessages):
- New invokedSkillBodies param (Map<string, string>)
- Reconstructs HumanMessages after skill ToolMessages at the correct
  position in the message sequence, matching where ToolNode originally
  injected them

LibreChat:
- extractInvokedSkillsFromPayload replaces extractInvokedSkillsFromHistory
  (works with raw TPayload before formatAgentMessages, not BaseMessage[])
- primeInvokedSkills now takes payload instead of messages, returns
  skillBodies Map instead of injectedMessages
- client.js calls primeInvokedSkills BEFORE formatAgentMessages, passes
  skillBodies through as the 4th param
- Removed invokedSkillMessages from createRun (no more systemContent hack)
- Single-pass: skill detection happens inside formatAgentMessages' existing
  tool_call processing loop, zero extra message iterations

* refactor: rename skillBodies to skills for consistency with SDK param

* refactor: move auth loading into primeInvokedSkills, pass loadAuthValues as dep

The payload/accessibleSkillIds guard and CODE_API_KEY loading now live
inside primeInvokedSkills (packages/api) rather than in the CJS caller.
initialize.js passes loadAuthValues as a dependency and the callback
is only created when skillsCapabilityEnabled.

* feat: ReadFile tool + conditional bash registration + skill path namespacing

ReadFile tool (read_file):
- General-purpose file reader, event-driven (ON_TOOL_EXECUTE)
- Schema: { file_path: string } — "{skillName}/{path}" convention
- handleReadFileCall: resolves skill name from path, ACL check, reads
  from DB cache or storage, binary detection, size limits (256KB),
  lazy caching (512KB), line numbers in output
- SKILL.md special case: reads skill.body directly
- Dispatched alongside SKILL_TOOL in createToolExecuteHandler
- Added to specialToolNames in ToolService

Conditional tool registration:
- ReadFile + SkillTool: always registered when skills enabled
- BashTool: only registered when codeEnvAvailable === true
- codeEnvAvailable passed through InitializeAgentParams from caller

Skill file path namespacing:
- primeSkillFiles now uploads as "{skillName}/SKILL.md" and
  "{skillName}/{relativePath}" instead of flat names
- Prevents file collisions when multiple skills are invoked

Wiring:
- getSkillFileByPath + updateSkillFileContent passed through
  ToolExecuteOptions in all three callers

* feat: return images/PDFs as artifacts from read_file, tighten caching

Binary artifact support:
- Images (png, jpeg, gif, webp) returned as base64 in artifact.content
  with type: 'image_url', processed by existing callback attachment flow
- PDFs returned as base64 artifact similarly
- Binary size limit: 10MB (MAX_BINARY_BYTES)
- Other binary files still return metadata + bash fallback

Caching:
- Text cached only on first read (file.content == null check)
- Binary flag cached only on first detection (file.isBinary == null)
- Skill files are immutable; no redundant cache writes

Registration:
- ReadFileToolDefinition now includes responseFormat: 'content_and_artifact'

* chore: update @librechat/agents to version 3.1.66-dev.0 and add peer dependencies in package-lock.json and package.json files

* fix: resolve review findings #1,#2,#4,#5,#6,#10,#13

Critical:
- #1: primeInvokedSkills now accumulates files across all skills into
  one session entry instead of overwriting. Parallel processing via
  Promise.allSettled.
- #2: codeEnvAvailable now computed and passed in openai.js and
  responses.js (was missing, bash tool never registered in those flows)

Major:
- #4: relativePath in updateSkillFileCodeEnvIds now strips the
  {skillName}/ prefix to match SkillFile documents. SKILL.md filter
  uses endsWith instead of exact match.
- #5: File priming guarded on apiKey being non-empty (skip when not
  configured instead of failing with auth error)
- #6: Skills processed in parallel via Promise.allSettled instead of
  sequential for-of loop

Minor:
- #10: Use top-level imports in initialize.js instead of inline requires
- #13: Log warning when skill catalog reaches the 100-skill limit

* fix: resolve followup review findings N1,N2,N4

N1 (CRITICAL): Wire skill deps into responses.js non-streaming path.
Was completely missing getSkillByName, file strategy functions, etc.

N2 (MAJOR): Single batch upload for ALL skills' files. Resolves skills
in parallel (Phase 1), then collects all file streams across skills
and does ONE batchUploadCodeEnvFiles call (Phase 2). All files share
one session_id, eliminating cross-session isolation issues.

N4 (MINOR): Move inline require() to top-level in openai.js and
responses.js, consistent with initialize.js.

* fix: add mocks for new file strategy imports in controller tests

* fix: restore session freshness check, parallelize file lookups, add warnings

R1: Re-add session freshness check before batch upload. Checks any
existing codeEnvIdentifier via getSessionInfo + checkIfActive. If the
session is still active (23h window), returns cached file references
with zero re-uploads.

R2: listSkillFiles calls parallelized via Promise.all (were sequential
in the for-of loop).

R3: Log warning when skill record lookup fails during identifier
persistence (was a silent empty-string fallback).

* fix: guard freshness cache on single-session consistency

* fix: multi-session freshness check (code env handles mixed sessions natively)

The code execution environment fetches each file by its own
{session_id, fileId} pair independently — no single-session
requirement. Removed the sessionIds.size === 1 guard.

Now checks ALL distinct sessions for freshness. If every session
is still active (23h window), returns cached references with per-file
session_ids preserved. If any session expired, falls through to
re-upload everything in a single batch.

* perf: parallelize session freshness checks via Promise.all

* fix: add optional chaining for session info retrieval in primeInvokedSkills

Updated the primeInvokedSkills function to use optional chaining for getSessionInfo and checkIfActive methods, ensuring safer access and preventing potential runtime errors when these methods are undefined.

* fix: address review findings #1-#9 + Codex P1/P2 + session probe

Critical:
- #1/Codex P1: Add codeApiKey loading to openai.js and responses.js
  loadTools configurable (was missing, file priming broken in 2/3 paths)
- Codex P1: Fix cached file name prefix in primeSkillFiles cache path
  (was sf.relativePath, now ${skill.name}/${sf.relativePath})

Major:
- Codex P2: Honor ephemeral skills toggle in agents endpoint
  (check ephemeralAgent?.skills !== false alongside admin capability)
- #4: Early size check using file.bytes from DB before streaming
  (prevents full-file buffer for oversized files)

Minor:
- #5: Replace Record<string, any> with Record<string, boolean | string>
- #6: Localize Pin/Unpin aria-labels with com_ui_pin/com_ui_unpin
- #8: Parallelize stream acquisition in primeSkillFiles via
  Promise.allSettled
- #9: Log warning for partial batch upload failures with filenames

Performance:
- Session probe optimization: getSessionInfo now hits per-object
  endpoint (GET /sessions/{sid}/objects/{fid}) instead of listing
  entire session (GET /files/{sid}?detail=summary). O(1) stat vs
  O(N) list + linear scan.

* refactor: extract shared skill wiring helper + add unit tests

DRY (#3):
- New skillDeps.js exports getSkillToolDeps() with all 9 skill-related
  deps (getSkillByName, listSkillFiles, getStrategyFunctions, etc.)
- Replaces 5 identical copy-paste blocks across initialize.js, openai.js,
  responses.js (streaming + non-streaming paths)
- One place to maintain when skill deps change

Tests (#2):
- 8 unit tests for extractInvokedSkillsFromPayload covering:
  string args, object args, missing skill tool_calls, non-assistant
  messages, malformed JSON, empty skillName, empty payload, dedup

* fix: remove @jest/globals import, use global jest env

* fix: resolve round 2 review findings R2-1 through R2-7

R2-1 (toggle semantics): openai.js + responses.js now check admin
  capability (AgentCapabilities.skills) alongside ephemeral toggle.
  Aligns with initialize.js.

R2-2 (swallowed error): primeInvokedSkills now logs
  updateSkillFileCodeEnvIds failures (was .catch(() => {}))

R2-4 (test cast): Record<string, string> → Record<string, unknown>

R2-5 (DRY regression): Extract enrichWithSkillConfigurable() into
  skillDeps.js. Replaces 4 identical loadAuthValues blocks.
  Each loadTools callback is now a one-liner. JSDoc added (R2-6).

R2-7 (sequential streams): primeInvokedSkills now uses
  Promise.allSettled for parallel stream acquisition.

* fix: require explicit skills toggle + treat partial cache as miss

- initialize.js: change ephemeralSkillsToggle !== false to === true
  (unset toggle no longer enables skills)
- primeSkillFiles cache: require ALL files to have codeEnvIdentifier
  before using cache (partial persistence = cache miss = re-upload)
- primeInvokedSkills cache: same check (allFilesWithIds.length must
  equal total file count)

* fix: pass entity_id=skillId on batch upload, eliminates per-user cache thrashing

primeSkillFiles now passes entity_id: skill._id.toString() to
batchUploadCodeEnvFiles. This scopes the code env session to the
skill, not the user. All users sharing a skill share the same
uploaded files — no more cache thrashing from overwriting each
other's codeEnvIdentifier.

The stored codeEnvIdentifier now includes ?entity_id= suffix so
freshness checks pass the entity_id through to the per-object
stat endpoint. Both primeSkillFiles and primeInvokedSkills
store consistent identifier formats.

* fix: pass entity_id on multi-skill batch upload, consistent identifier format

* Revert "fix: pass entity_id on multi-skill batch upload, consistent identifier format"

This reverts commit c85ce21.

* refactor: per-skill upload in primeInvokedSkills, eliminate multi-skill batch

Replace the monolithic multi-skill batch upload with per-skill
primeSkillFiles calls. Each skill gets its own session with
entity_id=skillId, ensuring:

- Correct session auth (entity_id matches on freshness checks)
- Per-skill freshness caching (only expired skills re-upload)
- Shared skill sessions work across users (same entity_id=skillId)
- Code env handles mixed session_ids natively

The big batch block (stream collection, single upload, identifier
mapping) is replaced by a simple loop over primeSkillFiles, which
already handles freshness caching, batch upload, and identifier
persistence per-skill.

* fix: resolve review findings #1,#3-5,#7,#9-11

Critical:
- #1: Strip ?entity_id= query string before splitting codeEnvIdentifier
  into session_id/fileId (was corrupting cached file IDs in 4 locations)

Major:
- #4: Parallelize per-skill primeSkillFiles via Promise.allSettled
- #5: Add logger.warn to all empty .catch(() => {}) on cache writes

Minor:
- #7: Add logger.debug to enrichWithSkillConfigurable catch block
- #9: Use error instanceof Error guard in batchUploadCodeEnvFiles
- #10: Move enrichWithSkillConfigurable to TypeScript in packages/api
  (skillConfigurable.ts), skillDeps.js wraps with loadAuthValues dep
- #11: Reduce MAX_BINARY_BYTES from 10MB to 5MB (~11.5MB peak with b64)

* fix: forward entity_id in session probe + always register bash tool

Codex P2 (entity_id in probe): getSessionInfo now preserves and
forwards query params (including entity_id) to the per-object stat
endpoint. Without this, identifiers stored as ...?entity_id=... would
fail auth checks because the entity_id scope was dropped.

Codex P2 (bash tool availability): Remove codeEnvAvailable gate from
injectSkillCatalog. Bash tool definition is now always registered when
skills are enabled. Actual tool instance creation still happens at
execution time in loadToolsForExecution (which loads per-user
credentials). This ensures users with per-user CODE_API_KEY get
bash without requiring a global env var at init time.

Removes codeEnvAvailable from InjectSkillCatalogParams,
InitializeAgentParams, and all three controller entry points.

* fix: add debug logging to primeInvokedSkills catch, rename export alias

* fix: stub bash tool when no key + remove PDF artifact path

Codex P1 (bash tool): When CODE_API_KEY is unavailable, create a stub
tool that returns "Code execution is not available. Use read_file
instead." This prevents "tool not found" errors from the model
repeatedly calling bash_tool in no-code-env deployments while still
registering the definition for per-user credential users.

Codex P2 (PDF artifacts): Remove PDF image_url artifact path. The
host artifact pipeline processes image_url via saveBase64Image which
fails for PDFs. PDFs now fall through to the generic binary handler
("Use bash to process"). TODO comment for future document artifact
support.

Also: isImageOrPdf → isImage in early size checks (PDFs are no
longer treated as artifact candidates).

* fix: remove dead PDF_MIME constant, hoist skillToolDeps, document session_id

- #7: Remove unused PDF_MIME constant (dead code after PDF artifact removal)
- #11: Hoist skillToolDeps to module-level constant (avoid per-call allocation)
- #6: Document that CodeSessionContext.session_id is a representative value;
  ToolNode uses per-file session_id from the files array

* fix: call toolEndCallback for skill/read_file artifacts + clear codeEnvIdentifier on re-upload

Codex P1 (toolEndCallback bypass): skill and read_file handler branches
returned early, bypassing the toolEndCallback that processes artifacts
(image attachments). Now calls toolEndCallback when the result has an
artifact, using the same metadata pattern as the normal tool.invoke path.

Codex P1 (stale identifiers): upsertSkillFile now $unset's
codeEnvIdentifier alongside content and isBinary when a file is
re-uploaded. Prevents the freshness cache from returning references
to old file content after a skill file is replaced.

* fix: add session_id comment at cached path, rename skillResult to handlerResult

* fix: return content_and_artifact from bash stub so result.content is populated

* fix: deterministic skill lookup, dedup warning, and multi-session freshness check

- getSkillByName: add sort({updatedAt:-1}) so name collisions resolve
  deterministically to the most recently updated skill
- injectSkillCatalog: warn when multiple accessible skills share a name
- primeSkillFiles: check ALL distinct sessions for freshness, not just
  the first file's session, preventing stale refs after partial bulkWrite

* refactor: update icon import in Skills component

- Replaced the Scroll icon with ScrollText in the Skills component for improved clarity and consistency in the UI.

* fix: SKILL.md cache parity, gate bash_tool on code env, fix read_file too-large message

- primeSkillFiles: filter SKILL.md from returned files array on fresh
  upload so cached and non-cached paths return identical file sets
  (SKILL.md is still on disk in the session for bash access)
- injectSkillCatalog: only register bash_tool when codeEnvAvailable is
  true; thread the flag from all three CJS callers via execute_code
  capability check
- handleReadFileCall: tell the model to invoke the skill first before
  suggesting /mnt/data paths for oversized files

* fix: use EnvVar constant, deduplicate auth lookup, validate batch upload, stream byte limit

- Replace hardcoded 'LIBRECHAT_CODE_API_KEY' with EnvVar.CODE_API_KEY
  in skillConfigurable.ts and skillFiles.ts
- Resolve code API key once at run start in initialize.js and pass to
  both primeInvokedSkills and enrichWithSkillConfigurable via optional
  preResolvedCodeApiKey param, eliminating redundant loadAuthValues calls
- Add response structure validation in batchUploadCodeEnvFiles before
  accessing session_id/files to surface unexpected responses early
- Add streaming byte counter in handleReadFileCall that aborts and
  destroys the stream when accumulated bytes exceed MAX_BINARY_BYTES,
  preventing full file buffering when DB metadata is inaccurate

* refactor: update icon import in ToolsDropdown component

- Replaced the Scroll icon with ScrollText in the ToolsDropdown component for improved clarity and consistency in the UI.

* fix: partial upload failure detection, EnvVar in initialize.js, declaration ordering

- primeSkillFiles: return null (failure) when batch upload partially
  succeeds — missing bundled files would cause runtime bash/read
  failures with missing paths in code env
- initialize.js: replace hardcoded 'LIBRECHAT_CODE_API_KEY' with
  EnvVar.CODE_API_KEY imported from @librechat/agents
- initialize.js: move enabledCapabilities, accessibleSkillIds, and
  codeApiKey declarations before the toolExecuteOptions closure that
  references them (eliminates reliance on temporal dead zone hoisting)
danny-avila added a commit that referenced this pull request Apr 17, 2026
…efaults (#12692)

* feat: per-user skill active/inactive toggle with ownership-aware defaults

- Add `skillStates` map (Record<string, boolean>) to user schema for
  per-user active/inactive overrides on skills
- Add `defaultActiveOnShare` to interface.skills config (default: false)
  so admins can control whether shared skills auto-activate
- Add GET/POST /api/user/settings/skills/active endpoints with validation
- Add React Query hooks with optimistic mutations for skill states
- Add useSkillActiveState hook with ownership-aware resolution:
  owned skills default active, shared skills default inactive
- Add toggle switch UI to SkillListItem and SkillDetail components
- Filter inactive skills in injectSkillCatalog before agent injection
- Add localization keys for active/inactive labels

* fix: use Record instead of Map for IUser.skillStates

Mongoose .lean() flattens Map to a plain object, causing type
incompatibility with IUser in methods that return lean documents.

* fix: address review findings for skill active states

- Fail-closed when userId is absent: filter rejects all shared skills
  instead of passing them through unfiltered (Codex P1)
- Validate Mongoose Map key characters (reject . and $) in controller
  to return 400 instead of a 500 from schema validation (Codex P2)
- Block toggle while initial skill states query is loading to prevent
  overwriting server-side overrides with an empty snapshot (Codex P2)
- Extract shared SkillToggle component, eliminating duplicate toggle
  markup in SkillListItem and SkillDetail (Finding #3)
- Move skill state query/mutation hooks from Favorites.ts to
  Skills/queries.ts per feature-directory convention (Finding #4)
- Fix hardcoded English aria-label in SkillListItem by passing the
  localized string from the parent SkillList (Finding #5)
- Fix inline arrow in SkillList render loop: pass stable callback
  reference so SkillListItem memo() is not invalidated (Finding #1)
- Extract toRecord() helper in controller to DRY the Map-to-Object
  conversion (Finding #6)
- Remove Promise.resolve wrapping synchronous config read (Finding #8)
- Remove unused TUpdateSkillStatesRequest type (Finding #12)

* fix: forward tabIndex on SkillToggle to preserve list keyboard nav

The original inline toggle had tabIndex={-1} so the row itself
remained the sole tab target. The extraction into SkillToggle
dropped this prop, making every list toggle a tab stop. Add an
optional tabIndex prop and pass -1 from SkillListItem.

* fix: plumb skillStates to all agent entry points, isolate toggle keydown

- Add skillStates/defaultActiveOnShare loading to openai.js and
  responses.js controllers so shared-skill activation is respected
  across all agent entry points, not just initialize.js (Codex P1)
- Stop keydown propagation on SkillToggle so Enter/Space does not
  bubble to the parent row's navigation handler (Codex P2)

* fix: paginate catalog fetch and serialize toggle writes

- Paginate listSkillsByAccess (up to 10 pages of 100) until the active
  catalog quota is filled, so inactive shared skills in recent positions
  do not starve active owned skills past the first page (Codex P1)
- Extend listSkillsByAccess interface with cursor/has_more/after for
  catalog pagination
- Serialize skill-state writes via a ref queue: one in-flight request
  at a time, with the latest desired state sent when the previous one
  settles. Prevents last-response-wins races where an older request
  overwrites newer toggles (Codex P2)

* fix: share write queue across hook instances, block toggle on fetch error

- Move the write queue from a per-instance useRef to a module-scoped
  object so every mount of useSkillActiveState (SkillList, SkillDetail,
  etc.) serializes against the same in-flight slot. Prior per-instance
  queues allowed two components to race full-map POSTs (Codex P1)
- Extend the toggle guard beyond isLoading: also block when isError is
  true or data is undefined. Prevents a failed GET from seeding a
  toggle with an empty baseline that would wipe server-side overrides
  on the next successful POST (Codex P1)

* fix: stale closure, orphan cleanup, and cap-error UX

- Read toggle baseline from React Query cache via queryClient.getQueryData
  instead of the captured skillStates closure. The closure can be stale
  between onMutate's setQueryData and the next render, so rapid successive
  toggles would build on old state and drop earlier changes (Codex P1)
- Surface the MAX_SKILL_STATES_EXCEEDED error code with a specific toast
  key (com_ui_skill_states_limit) so users understand the 200-cap rather
  than seeing a generic error
- Prune orphaned entries (skillIds whose Skill doc no longer exists) on
  both GET and POST in SkillStatesController. Self-heals over time
  without needing cascade-delete hooks or a migration job. Uses one
  indexed Skill._id query per request

* test: pin skill active-state precedence with unit tests

Extract the active-state resolution logic from a closure inside
injectSkillCatalog into an exported resolveSkillActive helper, then
cover every branch of the precedence matrix:

- Fails closed when userId is absent (even with defaultActiveOnShare=true)
- Explicit override wins over ownership and config (both true and false)
- Owned skills default to active when no override is set
- Shared skills default to defaultActiveOnShare value
- Undefined skillStates behaves identically to an empty object
- defaultActiveOnShare defaults to false when omitted
- Owned skills ignore defaultActiveOnShare entirely

Closes Finding #2 from the pre-rebase comprehensive review. Mirrors
the existing scopeSkillIds test style; injectSkillCatalog now calls
resolveSkillActive instead of inlining the closure.

* refactor: limit skill active toggle to detail header, drop label

- Remove the per-row toggle from SkillListItem and the active-state
  plumbing (hook call, isSkillEnabled/onToggleEnabled/toggleAriaLabel
  props) from SkillList. The detail view is now the single place to
  change a skill's active state
- Drop dim/muted styling for inactive skills in the sidebar: without
  a control there, the visual indication has nowhere to land
- Resize SkillToggle to match neighbor buttons: outer h-9 container,
  h-6 w-11 track with size-5 knob, no label span. The 'Active' /
  'Inactive' text that accompanied the detail-view toggle is removed
- Remove the now-unused label prop and tabIndex prop (the tabIndex
  existed only for the list-row context) from SkillToggle. Drop the
  onKeyDown stopPropagation for the same reason
- Remove now-orphaned com_ui_skill_active / com_ui_skill_inactive
  translation keys

* style: shrink SkillToggle track to h-5 w-9 with size-4 knob

Container stays at h-9 to match neighbor button heights. The toggle
track itself drops from h-6 w-11 to h-5 w-9, with a size-4 knob
travelling 1.125rem on activation. Visually lighter inside the row.

* fix: remove redundant skillStates entries that match the resolved default

When a toggle lands on the ownership/config default, delete the key
from the map instead of persisting `{id: defaultValue}`. Without this,
a user toggling a skill off and back on would leave `{id: true}` for
an owned skill (whose default is already true), silently consuming a
slot against the 200-entry cap. Repeated round-trip toggles could
exhaust the quota with zero meaningful overrides (Codex P2).

Preserves the exceptions-list invariant that the runtime-resolution
design depends on.

* fix: prune before enforcing skill-state cap; reject non-ObjectId keys

Reorder the update controller so pruneOrphans runs before the 200-cap
check. Without this, a user near the cap with some orphaned entries
(skills deleted since their last GET) could send a payload that would
pass after pruning but gets rejected by the raw-size check first.

Add a sanity cap on raw payload size (2 * MAX_SKILL_STATES) so abusive
inputs do not reach the DB query, and enforce the real cap on the
pruned result instead.

Harden pruneOrphans: the earlier early-return path could pass
non-ObjectId keys through unchanged. Now only valid ObjectIds are
returned, and the Skill-model-unavailable fallback filters by format.

Also add isValidObjectIdString validation at the input boundary so
malformed (but otherwise non-Mongo-unsafe) keys never reach persistence
(Codex P2 x2).

* fix: enforce active filter at execute time, prune revoked shares, scope queue per user

P1: injectSkillCatalog now returns activeSkillIds (the filtered set
that appears in the catalog). initializeAgent uses that set as the
stored accessibleSkillIds on the initialized agent, so getSkillByName
at runtime cannot resolve a deactivated skill — even if the LLM
hallucinates a name or the user invokes by direct-invocation shorthand.
Previously the executor authorized against the full ACL set, bypassing
the active-state guarantee (Codex P1).

P2: pruneOrphans now checks user access via findAccessibleResources
in addition to skill existence. When a share is revoked, the user's
skillStates entry for that skill had no cleanup path and silently
consumed the 200-cap. Self-heals on both GET and POST. One extra ACL
query per settings read/write; scoped to a single user so no N-user
amplification (Codex P2).

P2: the write queue moves from a single module-scoped object to a Map
keyed by userId. Logout/login in the same tab can no longer flush the
previous user's pending snapshot under the new session's auth. Each
userId gets its own pending/inFlight slot; the in-flight request
retains its original auth via the cookie already attached when sent,
so the race window closes (Codex P2).

* refactor: extract skillStates helpers to packages/api; add tests; polish

Address the remaining valid findings from the comprehensive review:

- Extract toRecord, loadSkillStates, validateSkillStatesPayload, and
  pruneOrphanSkillStates into packages/api/src/skills/skillStates.ts
  as TypeScript. The controller in /api shrinks to a ~90-line thin
  wrapper that builds live dependency adapters for Mongoose + the
  permission service (Review #2 DRY, #3 workspace boundary)

- Replace the triplicated 12-line skillStates loading block in
  initialize.js, openai.js, and responses.js with a single call to
  loadSkillStates from @librechat/api. One helper, three sites

- Swap console.error for the project logger in the controller
  (Review #7)

- Remove the redundant INVALID_KEY_PATTERN regex: a valid ObjectId
  cannot contain . or $, so isValidObjectIdString already covers it
  (Review #11)

- Parameterize the 200-cap error toast with {{0}} interpolation
  driven by the error response's `limit` field, so future changes to
  MAX_SKILL_STATES update the UI message automatically (Review #12)

- Add 24 unit tests for the new skillStates helpers (toRecord,
  resolveDefaultActiveOnShare, loadSkillStates, validateSkillStates-
  Payload, pruneOrphanSkillStates) covering success paths, malformed
  input, cap boundaries, and parallel-query behavior (Review #4)

- Add 10 tests for injectSkillCatalog pagination covering empty
  accessible set, missing listSkillsByAccess, single-page filter,
  owned-vs-shared defaults, explicit-override precedence, multi-page
  collection, MAX_CATALOG_PAGES safety cap, early termination on
  has_more=false, additional_instructions injection, and fail-closed
  without userId (Review #5)

Total test count: 60 (was 26 on this surface).

* fix: rename skillStates ValidationError to avoid barrel-export collision

packages/api/src/types/error.ts already exports a ValidationError
(MongooseError extension). Re-exporting a different shape from
skills/skillStates.ts through the skills barrel caused TS2308 in CI
because the root index re-exports both. Rename to
SkillStatesValidationError to keep the exports disjoint.

* refactor: tighten tests and absorb caller guard into loadSkillStates

Address the followup review findings:

- Add optional `accessibleSkillIds` param to loadSkillStates so the
  helper short-circuits to defaults when no skills are accessible.
  All three controllers drop the residual 7-line conditional wrapper
  in favor of a single destructured call (Review #2)

- Remove the unreachable `typeof key !== 'string'` check from
  validateSkillStatesPayload: Object.entries always yields string
  keys per the JS spec (Review #3)

- Replace the two `as unknown as` agent casts in the injectSkillCatalog
  tests with a `makeAgent()` factory typed directly as the function's
  parameter shape (Review #4)

- Tighten the MAX_CATALOG_PAGES assertion from `toBeLessThanOrEqual(11)`
  to `toHaveBeenCalledTimes(10)` — the loop deterministically makes
  exactly 10 page fetches before hitting the cap (Review #1)

- Rewrite the parallel-execution test for pruneOrphanSkillStates using
  deferred promises instead of microtask-order assertions. The test
  now inspects `toHaveBeenCalledTimes(1)` on both mocks after a single
  Promise.resolve() yield, pinning Promise.all usage without relying
  on push-order into a shared array (Review #5)

- Evict stale writeQueue entries on user change via a module-scoped
  `lastSeenUserId` sentinel. When a different user's toggle is the
  first one after a logout/login, the previous user's queue entry is
  deleted. Keeps the Map bounded without adding hook-instance effect
  cleanup (Review #6)

* fix(test): mock loadSkillStates in openai and responses controller specs

The prior refactor replaced the inline 12-line skillStates loading
block with a call to loadSkillStates from @librechat/api. Both
controller spec files mock @librechat/api as a flat object, so any
new named import from that package is undefined in the test env.
Calling `await loadSkillStates(...)` threw before recordCollectedUsage
ran, surfacing as "undefined is not iterable" on the test's array
destructure of `mockRecordCollectedUsage.mock.calls[0]`.

Add the missing mock to both spec files alongside the existing
scopeSkillIds stub.

* fix: abandon stale skillStates write queues on user switch

Close the cross-session leak window where an in-flight flush loop
still holds a reference to a previous user's queue: it could fire its
next mutateAsync under the new session's auth cookies and persist
the stale snapshot to the new user's document (Codex P1).

Add an `abandoned` flag on `WriteQueue`. Three mechanisms cooperate:

- `getWriteQueue` marks every non-active queue abandoned when the
  user differs from the last-seen identity (pre-existing eviction
  site, now more aggressive).
- A `useEffect` on `userId` calls the same abandonment pass on every
  render with a new active identity, covering the window between
  logout/login and the new user's first toggle (when `getWriteQueue`
  would otherwise not fire).
- The flush loop checks `!queue.abandoned` in its while condition so
  the second and later iterations exit without firing another
  `mutateAsync` after the session changes.

The first iteration's in-flight request (already dispatched under the
original user's cookies) still runs to completion or failure on its
own — only the subsequent iterations, which are the dangerous ones,
are blocked.
wilfoa referenced this pull request in whiletrue-industries/LibreChat Apr 19, 2026
Addresses RTL audit sections #2, #3, #4, #5, #6.

- Nav.tsx: mobile sidebar now uses `right-0` + positive translateX
  offset under RTL so it slides in from the right edge; desktop
  animation offset is also mirrored. Uses the new `useIsRTL` hook.
- NavToggle.tsx: toggle button now translates in the correct
  direction for both LTR and RTL (side-aware and dir-aware). The
  rotation is driven by a CSS transform instead of the rotate-180
  class so it composes with the translate offset.
- Convo.tsx: active-conversation bar swapped from `before:left-0`
  to `before:start-0` so it sits on the leading edge.
- ConvoLink.tsx: title fade-out gradient uses `end-0.5` + `rounded-e-md`;
  gradient direction flips between `bg-gradient-to-l` (LTR) and
  `bg-gradient-to-r` (RTL) so the fade covers the truncated side.
- HoverToggle.tsx: default position is `absolute bottom-0 end-0 top-0`
  with `rounded-e-lg` and logical ps-/pe-.
- SearchBar.tsx: search icon (`start-3`), input (`ps-7`, `me-0`), and
  clear button (`end-[7px]`/`end-[16px]`) use logical utilities so the
  icons stay on the right sides under RTL.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
danny-avila added a commit that referenced this pull request Apr 19, 2026
Codex P2 + reviewer #1: Single `loadTools` call with the union of
`agent.tools + allowed-tools`. The earlier two-call approach dropped
`userMCPAuthMap` / `toolContextMap` / `actionsEnabled` from the
skill-added pass — an MCP tool gained via `allowed-tools` would be
visible to the model but fail at execution without per-user auth
context. Resolution of `manualSkillPrimes` is hoisted before
`loadTools` so the union can be computed up-front; the dropped-tools
debug log now compares loaded vs. requested across the single call.

Codex P3 + reviewer #2: `injectSkillCatalog.activeSkillIds` now
includes `disable-model-invocation: true` skills. The runtime ACL
check in `handleSkillToolCall` previously couldn't reach the explicit
"cannot be invoked by the model" rejection because the broader access
set excluded those skills. Catalog text and tool registration still
gate on the visible subset (zero-context-token guarantee preserved);
only the per-user `isActive` filter is a hard exclusion now.

Reviewer #1 (try/catch around loadTools, MAJOR): A single bad
`allowed-tools` entry from a shared skill could crash the entire turn.
Now wrapped — on failure with extras, retry with just `agent.tools`
and continue (the dropped-tools debug log surfaces what vanished). If
the retry-without-extras still throws, propagate; the agent's own
tools are the load-bearing surface.

Reviewer #3 (integration tests, MAJOR): Added six tests in
`initialize.test.ts` covering the full `allowed-tools` loading path:
union pass-through, no-extras short-circuit, agent-baseline dedup,
loadTools throw + retry, propagated throw without extras, and the
empty-tools edge case.

Smaller cleanups bundled in:
- Reviewer #4: Moved `logger` import to the package-imports section
  (was wedged among local imports).
- Reviewer #5: Removed unused index on `disableModelInvocation`
  (filtering happens application-side in `injectSkillCatalog`; index
  cost write overhead for zero query benefit).
- Reviewer #6: Swapped order of `userInvocable` and body checks in
  `resolveManualSkills` so the more authoritative author-decision
  reason surfaces first when both apply.
- Reviewer #8: Documented the `allowedTools` enforcement gap on the
  schema + type — model-invoked skills (mid-turn `skill` tool calls)
  do NOT trigger tool union, since adding tools after the graph
  starts would require a rebuild. Manual / always-apply (Phase 5)
  primes are the supported paths.
- Reviewer #9: Renamed `dmi` / `ui` / `at` locals to
  `disableModelInvocationRaw` / `userInvocableRaw` / `allowedToolsRaw`
  in `deriveStructuredFrontmatterFields`.

Reviewer #7 (DRY shared `getSkillByName` return type) deferred —
field sets diverge meaningfully across the three call sites (handler
needs `body + fileCount`; resolver needs `author + allowedTools +
userInvocable`; the InitializeAgentDbMethods contract needs the
superset). A `Pick<>`-based consolidation is a follow-up cleanup.
danny-avila added a commit that referenced this pull request Apr 20, 2026
…ocs)

Sanity-pass review surfaced 7 findings; addressed 6 (the 7th — DRY
on inline `getSkillByName` return types — is acknowledged tech debt
deferred to a follow-up).

#1 [MAJOR, DRY]: The 4-line `manualSkillPrimedIdsByName` map
construction was duplicated across 4 CJS call sites (openai.js,
responses.js x2, initialize.js). Extracted `buildManualSkillPrimedIdsByName`
helper in `skillDeps.js`; all four sites now call the helper. If
`ResolvedManualSkill` ever renames `_id` or gains identifying fields,
only the helper changes.

#2 [MINOR, type safety]: `handleReadFileCall` was casting a hex string
to `Types.ObjectId[]` via `as unknown as`, relying on mongoose's
auto-cast in `$in` queries. Replaced with `new Types.ObjectId(...)`
so any future consumer comparing with `.equals()` / `===` gets the
correct value type. Imported `Types` as a value (was type-only).

#5 [MINOR, test gap]: Added a test for the worst-case silent-failure
path — both the union and base-only `loadTools` calls return undefined.
The agent gets no tools but the turn doesn't crash hard; pinning
that contract.

#4 [MINOR, performance]: Added a TODO on the `listSkillsByAccess`
projection noting the `frontmatter` field can be dropped once a
write migration backfills all pre-Phase-6 skills' columns. ~2KB/skill
× 100/page is wasted bandwidth post-backfill.

#6 [NIT, docs]: `backfillDerivedFromFrontmatter` JSDoc said "Pure"
right before "mutates its undefined fields in place". Replaced with
"Side-effect-free w.r.t. the DB (no writes), but mutates its argument
in place" which describes both halves accurately.

#7 [NIT, test determinism]: Replaced `await new Promise(r => setTimeout(r, 5))`
in two same-name collision tests with explicit `updateOne` setting
`updatedAt: new Date(Date.now() - 1000)` on the older doc. Removes
the wall-clock race on fast CI runners. The pagination test (line
480) still uses setTimeout — that test is pre-existing and order
is incidental, not load-bearing.

Existing test fixtures updated to use valid 24-char hex ObjectIds
(required by the iter-9 test that constructs a real `ObjectId`).

#3 [MINOR, deferred]: Inline `getSkillByName` return type duplicated
across `handlers.ts`, `initialize.ts`, `skills.ts`. Reviewer
acknowledged this as deferred; field sets diverge across call sites
(handler needs `fileCount`, resolver needs `author`/`allowedTools`).
A `Pick<>`-based consolidation is a clean follow-up.
danny-avila added a commit that referenced this pull request Apr 20, 2026
…le` / `allowed-tools` (#12745)

* 🧬 feat: Persist `disable-model-invocation` / `user-invocable` / `allowed-tools`

Adds first-class columns mirroring the three runtime-enforced frontmatter
fields, with a `deriveStructuredFrontmatterFields` helper that maps from
frontmatter at create/update time and re-syncs (via `$unset`) when fields
are removed. `listSkillsByAccess` projection includes them so the Phase 6
catalog filter and popover filter can both read off the summary row.

Marks `invocationMode` as @deprecated on `TSkill` and the
`InvocationMode` enum — the runtime now reads the persisted pair instead.

* 🛡️ feat: Enforce frontmatter at runtime (catalog, skill tool, manual resolver, tool union)

Wires the persisted columns into actual runtime behavior across all four
invocation paths:

- `injectSkillCatalog` excludes `disableModelInvocation: true` skills
  before catalog formatting — they cost zero context tokens and stay
  invisible to the model.
- `handleSkillToolCall` rejects with a clear error when the model names
  a skill marked `disable-model-invocation: true` (defends against a
  stale-cache or hallucinated invocation getting past the catalog
  filter).
- `resolveManualSkills` skips `userInvocable: false` skills with a warn
  log so an API-direct caller can't bypass the popover-side filter.
- `unionPrimeAllowedTools` collects skill-declared `allowed-tools` minus
  what's already on the agent; `initialize.ts` re-runs `loadTools` for
  the extras and merges resulting `toolDefinitions` into the agent's
  effective set for the turn. Tool-name resolution is tolerant —
  unknown names silently drop with a debug log so cross-ecosystem
  skills referencing yet-to-be-implemented tools (Claude Code's
  `edit_file`, etc.) import without breaking. The agent document is
  never modified; the union is turn-scoped.

Helper exports (`unionPrimeAllowedTools`) are structured so Phase 5's
always-apply primes flow through the same union (combined
`[...manualPrimes, ...alwaysApplyPrimes]`) once the resolver lands.

Skill handler wire format gains the three fields so clients can render
them on detail / list views.

* 🎛️ feat: `$` popover reads `userInvocable` instead of UI-only `invocationMode`

Replaces the phase-1 UI-only `invocationMode` check with the persisted
`userInvocable` field (mirrors the `user-invocable` frontmatter). Skills
authored with `user-invocable: false` no longer surface in the popover;
the backend resolver enforces the same rule for defense-in-depth.

Default-visible behavior is preserved: skills without an explicit
`userInvocable` value (older rows, freshly imported skills that don't
declare the field) stay visible — only an explicit `false` hides them.

Test fixture updated to reflect the new field.

* 🔧 fix: Address Phase 6 review findings

Codex P2 + reviewer #1: Single `loadTools` call with the union of
`agent.tools + allowed-tools`. The earlier two-call approach dropped
`userMCPAuthMap` / `toolContextMap` / `actionsEnabled` from the
skill-added pass — an MCP tool gained via `allowed-tools` would be
visible to the model but fail at execution without per-user auth
context. Resolution of `manualSkillPrimes` is hoisted before
`loadTools` so the union can be computed up-front; the dropped-tools
debug log now compares loaded vs. requested across the single call.

Codex P3 + reviewer #2: `injectSkillCatalog.activeSkillIds` now
includes `disable-model-invocation: true` skills. The runtime ACL
check in `handleSkillToolCall` previously couldn't reach the explicit
"cannot be invoked by the model" rejection because the broader access
set excluded those skills. Catalog text and tool registration still
gate on the visible subset (zero-context-token guarantee preserved);
only the per-user `isActive` filter is a hard exclusion now.

Reviewer #1 (try/catch around loadTools, MAJOR): A single bad
`allowed-tools` entry from a shared skill could crash the entire turn.
Now wrapped — on failure with extras, retry with just `agent.tools`
and continue (the dropped-tools debug log surfaces what vanished). If
the retry-without-extras still throws, propagate; the agent's own
tools are the load-bearing surface.

Reviewer #3 (integration tests, MAJOR): Added six tests in
`initialize.test.ts` covering the full `allowed-tools` loading path:
union pass-through, no-extras short-circuit, agent-baseline dedup,
loadTools throw + retry, propagated throw without extras, and the
empty-tools edge case.

Smaller cleanups bundled in:
- Reviewer #4: Moved `logger` import to the package-imports section
  (was wedged among local imports).
- Reviewer #5: Removed unused index on `disableModelInvocation`
  (filtering happens application-side in `injectSkillCatalog`; index
  cost write overhead for zero query benefit).
- Reviewer #6: Swapped order of `userInvocable` and body checks in
  `resolveManualSkills` so the more authoritative author-decision
  reason surfaces first when both apply.
- Reviewer #8: Documented the `allowedTools` enforcement gap on the
  schema + type — model-invoked skills (mid-turn `skill` tool calls)
  do NOT trigger tool union, since adding tools after the graph
  starts would require a rebuild. Manual / always-apply (Phase 5)
  primes are the supported paths.
- Reviewer #9: Renamed `dmi` / `ui` / `at` locals to
  `disableModelInvocationRaw` / `userInvocableRaw` / `allowedToolsRaw`
  in `deriveStructuredFrontmatterFields`.

Reviewer #7 (DRY shared `getSkillByName` return type) deferred —
field sets diverge meaningfully across the three call sites (handler
needs `body + fileCount`; resolver needs `author + allowedTools +
userInvocable`; the InitializeAgentDbMethods contract needs the
superset). A `Pick<>`-based consolidation is a follow-up cleanup.

* 🔧 fix: Address codex iter 2 — catalog quota + duplicate-name dedup

P1: `injectSkillCatalog` cap now counts only model-visible skills, not
the merged active set. The previous behavior let a tenant with many
`disable-model-invocation: true` rows near the top of the cursor
exhaust the 100-slot quota before any invocable skill got scanned —
the catalog could end up empty even though invocable skills existed
further down the paginated results. `MAX_CATALOG_PAGES` stays the
ceiling on scan budget; only `visibleCount` drives the early-exit on
quota fill.

P2: When an invocable and a `disable-model-invocation: true` skill
share a name, drop the disabled doc(s) from `activeSkillIds`. Without
this dedup, `getSkillByName` (which sorts by `updatedAt` desc) could
pick the disabled doc and every model call to the cataloged name
would fail with "cannot be invoked by the model" instead of executing
the visible skill. When ONLY a disabled doc exists for a name, it
stays in `activeSkillIds` so the explicit-rejection error path still
fires for hallucinated invocations.

Tests: 3 new cases in `injectSkillCatalog` covering (a) cap counted
on visible skills only, (b) same-name collision drops disabled doc,
(c) sole-disabled-name case keeps the disabled doc.

* 🔒 fix: Apply `disable-model-invocation` gate to read_file too (codex iter 3 P1)

`activeSkillIds` is shared between the `skill` and `read_file` handlers.
The skill-tool gate was applied last iteration, but `handleReadFileCall`
authorized purely on `getSkillByName(..., accessibleIds)` — so a model
that learned a hidden skill's name (stale catalog or hallucination)
could still read its `SKILL.md` body or bundled files via `read_file`,
defeating the contract. Same explicit rejection now fires from both
handlers; no change needed to the ACL set itself (disabled docs stay
in `activeSkillIds` so the explicit error path keeps firing).

Two new tests in `handlers.spec.ts` cover the read_file gate and
regression-protect the happy path.

* 🔧 fix: Address codex iter 4 — manual-prime exception + legacy frontmatter backfill

P1: Scope the `read_file` `disableModelInvocation` gate to AUTONOMOUS
model probes only. A user-invoked `$` skill that is also marked
`disable-model-invocation: true` had its bundled `references/*` /
`scripts/*` files unreadable, leaving the manually-primed skill body
referencing files the model couldn't load. Now the handler bypasses
the gate when the skill name appears in `manualSkillNames` (the
per-turn allowlist threaded from `manualSkillPrimes` →
`agentToolContexts` → `enrichWithSkillConfigurable` →
`mergedConfigurable`). Defense-in-depth: the bypass is scoped to the
specific names in the allowlist; a different disabled skill name is
still rejected.

P2: Read-time fallback for legacy skills authored before Phase 6
landed the structured columns. `user-invocable: false` /
`disable-model-invocation: true` set in `frontmatter` (the validator
already accepted those keys) but with no derived column would
incorrectly evaluate as "user-invocable / model-allowed" until a save
backfilled the columns. New `backfillDerivedFromFrontmatter` helper
fills undefined columns from frontmatter at read time in both
`getSkillByName` and `listSkillsByAccess` — column wins when both are
set, frontmatter fills the gap when only it's set. No DB writes; the
next `updateSkill` naturally persists. `listSkillsByAccess` projection
expanded to include `frontmatter` (bounded by validator, payload
impact small) so summaries can also be backfilled.

Sticky-primed disabled skills (ones invoked in prior turns of the
same conversation) are not yet in the manual-prime allowlist — same-
turn manual invocation is the load-bearing path codex flagged; the
sticky-turn case is a known limitation tracked for a follow-up.

Tests: 2 new in handlers.spec.ts (manual-prime allows + name-scoped
block holds), 3 new in skill.spec.ts (legacy backfill via
getSkillByName + listSkillsByAccess + column-wins precedence).

* 🔧 fix: Address codex iter 5 — propagate manualSkillNames + keep read_file

P1: `enrichWithSkillConfigurable` is also called from `openai.js` and
`responses.js` (the OpenAI Responses + completions endpoints). Both
were ignoring the new `manualSkillNames` parameter, which meant the
manual-prime exception in the `read_file` gate (iter 4) only worked
on the agents endpoint. Now all three call sites pass
`primaryConfig.manualSkillPrimes?.map(p => p.name)` so manual `$`
invocations of disabled skills work consistently across endpoints.

P2: When every accessible skill is `disable-model-invocation: true`,
the catalog text and `skill` tool are correctly omitted (no model-
reachable targets) — but `read_file` and `bash_tool` MUST still be
registered. A user manually invoking such a skill gets its SKILL.md
body primed into context; if the body references `references/foo.md`
or `scripts/run.sh`, those reads need a registered tool. Restructured
`injectSkillCatalog` so `skill` registration is gated on
`catalogVisibleSkills.length > 0` while `read_file` (always) and
`bash_tool` (when codeEnvAvailable) register whenever any active
skill is in scope.

Tests: existing all-disabled test rewritten to assert read_file IS
registered + skill is NOT; new test confirms bash_tool joins it
when codeEnvAvailable.

* 🔧 fix: Address codex iter 6 — name-collision consistency via preferInvocable

P2a (resolveManualSkills): a name collision between an older
user-invocable doc and a newer non-user-invocable doc made manual `$`
invocation silently no-op. The popover surfaced the older invocable
doc; resolver looked it up by name; `getSkillByName` returned the
newer non-invocable doc; resolver skipped on `userInvocable: false`.

P2b (handler / runtime ACL): with same-name duplicates (e.g. older
invocable + newer disabled), the manual prime resolved to one doc
while later `read_file` / `skill` execution resolved a different doc
through `activeSkillIds`. Model could follow one SKILL.md body while
reading files from a different skill.

Both root-cause: `getSkillByName` always returned the newest match
and let the caller filter, but with collisions the newest can be
something the caller didn't want.

Fix: extend `getSkillByName` with `options.preferInvocable`. When
true, prefer the newest doc satisfying BOTH `userInvocable !== false`
AND `disableModelInvocation !== true` (with frontmatter backfill);
fall back to the newest match otherwise. Fast path preserved when
caller doesn't opt in.

Callers passing `preferInvocable: true`:
- `resolveManualSkills` — picks the popover-visible invocable doc
  even when a newer disabled / non-user-invocable duplicate exists.
- `handleSkillToolCall` — keeps execution aligned with the catalog;
  falls back to the disabled doc only when no invocable variant
  exists (so the explicit "cannot be invoked by the model" gate
  still fires for the hallucinated-disabled-name case).
- `handleReadFileCall` — same alignment, plus the manual-prime
  exception added in iter 4 still applies.

Tests:
- 2 new in skill.spec.ts (preferInvocable picks invocable when
  collision exists; falls back to newest when no clean-invocable
  exists).
- 1 new in skills.test.ts (resolver passes preferInvocable through).
- 2 new in handlers.spec.ts (skill tool + read_file pass it).
- Existing initialize.test.ts assertion updated for the new option.

* 🔧 fix: Address codex iter 7 — split preferInvocable into per-axis flags

The previous unified `preferInvocable` filter required BOTH
`userInvocable !== false` AND `disableModelInvocation !== true`. That
was wrong for the model paths: `userInvocable: false` skills are
model-only and remain valid `skill` / `read_file` invocation targets.
A duplicate-name scenario where the newer cataloged doc was model-
only would let the older user-invocable variant shadow it on every
model call.

Split the option into two independent axes:
- `preferUserInvocable` — for manual paths (`$` popover). Skips docs
  with `userInvocable: false`. Disable-model-invocation status is
  irrelevant; iter 4 explicitly supports manual prime of disabled
  skills.
- `preferModelInvocable` — for model paths (`skill` / `read_file`
  handlers). Skips docs with `disableModelInvocation: true`. User-
  invocable status is irrelevant; model-only skills are valid here.

Both flags fall back to the newest match when no preferred doc
exists, so the explicit-rejection error paths still fire correctly
in the sole-disabled-name case.

Callers updated:
- `resolveManualSkills` → `preferUserInvocable: true`
- `handleSkillToolCall` / `handleReadFileCall` → `preferModelInvocable: true`

Tests:
- New spec test for preferModelInvocable not filtering on userInvocable.
- Existing preferInvocable test renamed/split to cover the new axes.
- New test asserts preferUserInvocable still returns disabled docs
  (preserves iter 4 manual-disabled support).
- Caller tests assert each path passes the right single flag and
  does NOT pass the wrong one.

* 🔧 fix: TypeScript type-check failure in handlers.spec.ts (CI green)

`jest.fn(async () => ...)` without explicit args infers an empty tuple
for the call signature, so `mock.calls[0][2]` flagged as "Tuple type
'[]' has no element at index '2'." Cast to `unknown[]` then narrow to
the expected option shape. Behavior unchanged.

Caught by the `Type check @librechat/api` CI step
(.github/workflows/backend-review.yml).

* 🔧 fix: Address codex iter 8 — undefined-result fallback + read_file alignment

P1 (loadTools returning undefined): Production loaders
(`createToolLoader` in `initialize.js` / `openai.js` /
`responses.js`) wrap `loadAgentTools` in try/catch and return
`undefined` on failure rather than throwing. Without explicit
handling, my iter-1 try/catch only fired for thrown errors — a
silent-failure on a skill-added tool would fall through to the
empty fallback and silently DROP the agent's baseline tools for
the turn (much worse than just losing the extras). Added an
`undefined`-result branch that retries with just `agent.tools`,
mirroring the throw branch. Test pins both behaviors.

P2 (read_file alignment with manual prime): When a skill is in
this turn's `manualSkillNames`, the `read_file` handler now uses
`preferUserInvocable` instead of `preferModelInvocable`. Same
name-collision rule as `resolveManualSkills`, so the doc whose
files get read is the same doc whose body got primed. For
autonomous probes (skill not in `manualSkillNames`), the handler
keeps `preferModelInvocable` to align with the catalog the model
saw. Two new tests cover both branches and regression-protect that
the wrong flag isn't passed.

* 🔧 fix: Address codex iter 9 — pin read_file lookup to primed skill _id

P1 (manually-primed disabled IDs were dropped from activeSkillIds):
The `executableSkills` dedup in `injectSkillCatalog` correctly drops
`disable-model-invocation: true` duplicates when an invocable doc
shares the name — but `resolveManualSkills` legitimately primes
disabled docs (iter 4 supports manual `$` invocation of disabled
skills). When the resolver primed a disabled doc, the read_file
handler couldn't find it in the (deduped) `activeSkillIds` and
either resolved a different same-name skill or returned not-found.

Fix: `ResolvedManualSkill` now carries `_id`; the legacy `initialize.js`
/ `openai.js` / `responses.js` controllers build a
`manualSkillPrimedIdsByName` map and `enrichWithSkillConfigurable`
passes it into `mergedConfigurable`. `handleReadFileCall` now pins
its lookup's `accessibleIds` to `[primedId]` whenever the requested
skill is in the map. The constrained set guarantees the lookup
returns the EXACT doc the resolver primed — body/files come from the
same source even when same-name duplicates exist or the dedup
removed the prime's id from `activeSkillIds`.

Autonomous read_file probes (skill not in the manual-primed map)
keep the full ACL set + `preferModelInvocable` so they align with
the catalog the model saw and the disabled-only case still fires
the explicit-rejection gate.

Test fixture changes flow from `_id` becoming required on
`ResolvedManualSkill`. `buildSkillPrimeContentParts` /
`injectManualSkillPrimes` widen their param types to `Pick<...>`
because they only read `name` / `body` and shouldn't force test
literals to invent placeholder ids.

* 🧹 fix: Address independent reviewer findings (DRY + types + tests + docs)

Sanity-pass review surfaced 7 findings; addressed 6 (the 7th — DRY
on inline `getSkillByName` return types — is acknowledged tech debt
deferred to a follow-up).

#1 [MAJOR, DRY]: The 4-line `manualSkillPrimedIdsByName` map
construction was duplicated across 4 CJS call sites (openai.js,
responses.js x2, initialize.js). Extracted `buildManualSkillPrimedIdsByName`
helper in `skillDeps.js`; all four sites now call the helper. If
`ResolvedManualSkill` ever renames `_id` or gains identifying fields,
only the helper changes.

#2 [MINOR, type safety]: `handleReadFileCall` was casting a hex string
to `Types.ObjectId[]` via `as unknown as`, relying on mongoose's
auto-cast in `$in` queries. Replaced with `new Types.ObjectId(...)`
so any future consumer comparing with `.equals()` / `===` gets the
correct value type. Imported `Types` as a value (was type-only).

#5 [MINOR, test gap]: Added a test for the worst-case silent-failure
path — both the union and base-only `loadTools` calls return undefined.
The agent gets no tools but the turn doesn't crash hard; pinning
that contract.

#4 [MINOR, performance]: Added a TODO on the `listSkillsByAccess`
projection noting the `frontmatter` field can be dropped once a
write migration backfills all pre-Phase-6 skills' columns. ~2KB/skill
× 100/page is wasted bandwidth post-backfill.

#6 [NIT, docs]: `backfillDerivedFromFrontmatter` JSDoc said "Pure"
right before "mutates its undefined fields in place". Replaced with
"Side-effect-free w.r.t. the DB (no writes), but mutates its argument
in place" which describes both halves accurately.

#7 [NIT, test determinism]: Replaced `await new Promise(r => setTimeout(r, 5))`
in two same-name collision tests with explicit `updateOne` setting
`updatedAt: new Date(Date.now() - 1000)` on the older doc. Removes
the wall-clock race on fast CI runners. The pagination test (line
480) still uses setTimeout — that test is pre-existing and order
is incidental, not load-bearing.

Existing test fixtures updated to use valid 24-char hex ObjectIds
(required by the iter-9 test that constructs a real `ObjectId`).

#3 [MINOR, deferred]: Inline `getSkillByName` return type duplicated
across `handlers.ts`, `initialize.ts`, `skills.ts`. Reviewer
acknowledged this as deferred; field sets diverge across call sites
(handler needs `fileCount`, resolver needs `author`/`allowedTools`).
A `Pick<>`-based consolidation is a clean follow-up.
danny-avila added a commit that referenced this pull request Apr 20, 2026
Audit review identified several MINOR/NIT items on top of the codex
rounds. This commit closes the actionable ones:

- **JSDoc (#1, #2)**: `extractMetaObject` no longer claims to filter
  underscore-prefixed keys (that filter was removed intentionally for
  MongoDB `_id`). `debugTraverse`'s docblock now describes the three
  code paths (warn/error compact trailer, debug multi-line traversal,
  other levels).
- **Case-insensitive api-key regex (#6)**: `/gi` so the Azure style
  `Api-Key:` / `API-KEY:` also gets redacted. Pre-existing behavior
  was lowercase-only.
- **Consolidated redundant branch (#5)**: `consoleFormat` printf was
  checking `isError || isWarn` twice; merged into one block.
- **Pre-compiled regex (#9)**: `NUMERIC_KEY_RE` moved to module scope.
- **Test coverage (#3, #4)**: Added regression tests for
  - per-field serialization fallback when a value's `toJSON` throws,
  - sensitive strings nested inside metadata objects,
  - the Azure-style `Api-Key:` header.
danny-avila added a commit that referenced this pull request Apr 20, 2026
* 🔊 fix: Preserve Log Metadata on Console for Warn/Error Levels

The default console formatter discarded every structured metadata key on the
winston info object — only `CONSOLE_JSON=true` preserved it. That meant
failures emitted by the agents SDK (e.g. "Summarization LLM call failed")
reached stdout without the provider, model, or underlying error attached,
leaving users unable to diagnose the root cause.

- Add `formatConsoleMeta` helper to serialize non-reserved metadata as a
  compact JSON trailer, with per-value string truncation and safe handling
  of circular references.
- Append the metadata trailer to warn/error console lines; info/debug
  behavior is unchanged.
- Relax `debugTraverse`'s debug-only gate so warn/error messages routed
  through the debug formatter also surface their metadata.
- Add a `formatConsoleMeta` stub to the shared logger mock so existing
  tests keep working.

* 🔐 fix: Also Redact Sensitive Patterns on Warn Console Lines

The warn-level console output now includes a metadata trailer that may
contain provider-returned error strings with embedded tokens or keys
(e.g. `Bearer ...`, `sk-...`). Apply `redactMessage` to warn lines in
addition to error, matching the new surface area.

* 🔐 fix: Redact Sensitive Tokens Embedded in JSON Metadata

Two gaps in the existing console redaction that became user-visible once
warn/error lines started emitting structured metadata:

1. The OpenAI-key regex (`/^(sk-)[^\s]+/`) was anchored to start-of-line,
   so keys embedded inside JSON payloads (e.g. `{"apiKey":"sk-..."}`)
   were never redacted. Every console line begins with a timestamp, so
   the anchor effectively made this pattern dead code.
2. `formatConsoleMeta` stringified metadata values verbatim; a sensitive
   string value was only redacted by the whole-line regex pass, which
   missed the anchored `sk-` case above.

Fix:
- Drop the `^` anchor; add `/g` so every occurrence is redacted, not just
  the first.
- Also exclude `"` and `'` from the token body so JSON-embedded values
  terminate at the closing quote rather than chewing into the next field.
- Simplify `redactMessage` to apply patterns directly (dropping the
  `getMatchingSensitivePatterns` filter) — the filter used `.test()`
  which has stateful behavior on `/g` regexes and is no longer needed.
- `formatConsoleMeta` now runs `redactMessage` over every string value
  before JSON serialization, so the metadata trailer is safe even on the
  warn path.
- Add regression tests covering both fixes.

Reviewed-by: Codex (P1 finding on PR #12737, commit 68c31b6).

* 🔐 fix: Redact Metadata in debugTraverse for Warn and Error

Relaxing the debug-only gate in debugTraverse (in commit 59371be)
routed warn/error records through the traversal path, which emits leaf
string values verbatim (via truncateLongStrings only). Because
DEBUG_LOGGING defaults to true, those records are also written to the
rotating debug log file — which means payloads like
`{ auth: 'Bearer ...' }` or `{ openaiKey: 'sk-...' }` were persisted
unredacted once my earlier change took effect.

Apply redactMessage to the final formatted string when the level is
warn or error. Debug-level behavior is unchanged (matching prior art).

Includes regression tests covering error/warn redaction and
debug-level preservation.

Reviewed-by: Codex (P1 finding on PR #12737, commit e288f7f).

* 🔐 fix: Anchor Secret Regexes at Word Boundaries to Prevent Over-Redaction

Removing the `^` anchor in commit e288f7f let the OpenAI-key regex match
anywhere in the line — including inside ordinary words like `task-runner`
or `mask-value`, where `sk-` appears mid-word. Non-secret text was being
rewritten to `task-[REDACTED]`, hiding real log content from operators.

- Anchor every sensitive-key pattern with `\b` so matches only fire at
  word boundaries.
- Constrain the OpenAI-key body to the documented charset
  (`[a-zA-Z0-9_-]+`) instead of the broader "not whitespace or quote"
  character class.
- Add `&` to the `key=` exclusion so a query-string value stops at the
  next parameter separator.
- Regression tests covering both the over-redaction cases (`task-runner`,
  `monkey=10`) and the intended redactions still firing.

Reviewed-by: Codex (P2 finding on PR #12737, commit c09d293).

* 🔐 fix: Redact Before Colorize To Survive ANSI Word-Boundary Interference

The console pipeline runs `redactFormat → colorize({ all: true }) → printf`.
With `all: true`, winston wraps `info.message` in ANSI escapes whose
trailing `m` is a word character. That means `\b(Bearer )…` placed at the
start of a colorized segment can fall on a (word,word) boundary and miss —
the earlier line-wise `redactMessage(line)` pass in printf suffers the
same issue because it runs after colorize.

Extend `redactFormat` to run for `warn` in addition to `error`, operating
on the raw pre-colorize `info.message` + `Symbol.for('message')` strings.
The later in-printf `redactMessage(line)` stays as a backstop, but the
primary redaction now happens where the regex can actually see the text.

Metadata redaction already operates on the raw info object via
`formatConsoleMeta`, so it was never affected by ANSI — no change there.

Includes regression tests for the new warn-level behavior and for the
info/debug no-op path.

Reviewed-by: Codex (P2 finding on PR #12737, commit fdb6b36).

* 🧹 fix: Prefer Structured Metadata Over Consumed Splat Args in Traversal

`debugTraverse` previously read `metadata[Symbol.for('splat')][0]` first
and only fell back to the structured metadata object. When a caller uses
printf interpolation alongside a metadata object — for example
`logger.warn('failed for %s', tenant, { provider })` — winston leaves the
*consumed* positional arg (`tenant`) in `SPLAT[0]` after interpolation.
The formatter would then append the tenant a second time and skip the
real metadata, regressing debug-file and `DEBUG_CONSOLE` output quality
now that warn/error share this path.

Prefer the structured metadata object (via `extractMetaObject`) and only
fall back to `SPLAT[0]` when there's nothing else, so the surviving log
line surfaces the actual key/value pairs regardless of call shape.

Reviewed-by: Codex (P2 finding on PR #12737, commit 1e43d63).

* 🧹 fix: Skip Consumed Splat Primitives in Warn/Error Debug Traversal

When no structured metadata is attached, winston still leaves consumed
`%s` / `%d` arguments in `Symbol.for('splat')`. Previous fix preferred
the structured object but still fell back to whatever sat at `SPLAT[0]`
— so `logger.warn('failed for %s', tenantId)` emitted
`failed for tenant-7 tenant-7` in the traversal path (debug file and
`DEBUG_CONSOLE`), now regressed outside of `debug` level because
warn/error share the path.

Only accept the splat fallback when the value is a plain object or an
array (structural data worth surfacing). Primitives there are almost
certainly consumed printf args and get skipped.

Regression tests cover the single-%s case and the array-as-metadata case
(which still surfaces through splat).

Reviewed-by: Codex (P2 finding on PR #12737, commit bccbf11).

* 🧹 fix: Skip Numeric Splat Keys When Extracting Log Metadata

When a caller passes a primitive as the second argument — e.g.
`logger.warn('Unhandled step creation type:', step.type)` — winston /
`format.splat()` can leave character-index keys (`"0"`, `"1"`, …) on the
`info` object. With the warn/error metadata trailer in play, those
synthetic artifacts were being surfaced as bogus metadata, producing
noisy console and debug-file output.

Filter out numeric-string keys in `extractMetaObject` so only real
metadata fields reach the trailer. Added a regression test.

Reviewed-by: Codex (P2 finding on PR #12737, commit b34628d).

* 🧹 fix: Preserve Unconsumed Primitive Splat Args in Debug Traversal

The previous round dropped every primitive SPLAT[0] value to avoid
duplicating consumed %s args, but that removed useful context from
calls like \`logger.debug('prefix:', detail)\` where the primitive was
never interpolated — users lost the \`detail\` value.

Refine the heuristic: skip a primitive splat value only when it already
appears inside the (post-interpolation) \`info.message\`; otherwise
surface it. Arrays and objects continue to surface unconditionally.

Regression test covers the 'prefix:', detail case.

Reviewed-by: Codex (P2 finding on PR #12737, commit 6bf9548).

* 🧹 fix: Traverse Filtered Metadata, Not Raw Metadata, In debugTraverse

`debugTraverse` computes `extracted = extractMetaObject(metadata)` to
strip reserved keys, underscore-prefixed internals, and numeric splat
artifacts — but the later \`klona(metadata)\` + \`traverse\` path still
read the raw object, putting all the filtered junk back into the
rendered multi-line output.

Clone and traverse \`debugValue\` (the already-filtered object) instead.
Regression test exercises the case where numeric splat artifacts sit
alongside a real metadata field.

Reviewed-by: Codex (P2 finding on PR #12737, commit c29c18e).

* 🧹 refactor: Split Warn/Error From Debug-Level Traversal in Parsers

Retrofitting `debugTraverse`'s multi-line object walker to cover
warn/error created a minefield of splat-interaction edge cases
(numeric artifacts, consumed %s args, bogus `_`-prefix filtering,
over-eager suppression of unconsumed primitives). Each fix kept
introducing new corner cases.

Split the two concerns instead:

- Warn/error now emit a compact single-line JSON metadata trailer via
  `formatConsoleMeta`, then pass the full line through `redactMessage`.
  This mirrors what the console formatter already does, so behavior
  between the console and debug-file outputs stays consistent for
  warn/error — and none of the splat/traversal edge cases apply.
- Debug level keeps its original code path verbatim (including the
  raw `metadata` traversal and SPLAT\[0\] fallback). No regressions from
  my earlier iterations.
- `extractMetaObject` no longer filters underscore-prefixed keys, so
  legitimate fields like MongoDB `_id` still appear. Reserved winston
  keys and numeric splat artifacts remain filtered.

Updated tests reflect the simpler contract (underscore preservation,
single-line trailer expectations already covered).

Reviewed-by: Codex (two P2 findings on PR #12737, commit 9ea1152:
`_id` regression and over-eager primitive suppression).

* 🧹 fix: Preserve Scalar Metadata When One Value Is Circular

`formatConsoleMeta` previously wrapped a single `JSON.stringify` in
try/catch — any circular reference inside any field (e.g. an attached
request/response object) caused the entire trailer to be dropped. That
defeats the goal of making failures diagnosable: one malformed field
would mask the provider/model/status we wanted to surface.

Use a `WeakSet`-based replacer that emits `[Circular]` for repeated
object visits. On the whole-object serialization failing, fall back to
per-field serialization so scalar keys always land and only the
offending field is replaced with \`"[Unserializable]"\`.

Reviewed-by: Codex (P2 finding on PR #12737, commit d63742a).

* 🧹 fix: Address Audit Findings (JSDoc, Case-Insensitive Api-Key, Tests)

Audit review identified several MINOR/NIT items on top of the codex
rounds. This commit closes the actionable ones:

- **JSDoc (#1, #2)**: `extractMetaObject` no longer claims to filter
  underscore-prefixed keys (that filter was removed intentionally for
  MongoDB `_id`). `debugTraverse`'s docblock now describes the three
  code paths (warn/error compact trailer, debug multi-line traversal,
  other levels).
- **Case-insensitive api-key regex (#6)**: `/gi` so the Azure style
  `Api-Key:` / `API-KEY:` also gets redacted. Pre-existing behavior
  was lowercase-only.
- **Consolidated redundant branch (#5)**: `consoleFormat` printf was
  checking `isError || isWarn` twice; merged into one block.
- **Pre-compiled regex (#9)**: `NUMERIC_KEY_RE` moved to module scope.
- **Test coverage (#3, #4)**: Added regression tests for
  - per-field serialization fallback when a value's `toJSON` throws,
  - sensitive strings nested inside metadata objects,
  - the Azure-style `Api-Key:` header.
krgokul referenced this pull request in syedhabib39/LibreChat Apr 20, 2026
…hat-AI#12737)

* 🔊 fix: Preserve Log Metadata on Console for Warn/Error Levels

The default console formatter discarded every structured metadata key on the
winston info object — only `CONSOLE_JSON=true` preserved it. That meant
failures emitted by the agents SDK (e.g. "Summarization LLM call failed")
reached stdout without the provider, model, or underlying error attached,
leaving users unable to diagnose the root cause.

- Add `formatConsoleMeta` helper to serialize non-reserved metadata as a
  compact JSON trailer, with per-value string truncation and safe handling
  of circular references.
- Append the metadata trailer to warn/error console lines; info/debug
  behavior is unchanged.
- Relax `debugTraverse`'s debug-only gate so warn/error messages routed
  through the debug formatter also surface their metadata.
- Add a `formatConsoleMeta` stub to the shared logger mock so existing
  tests keep working.

* 🔐 fix: Also Redact Sensitive Patterns on Warn Console Lines

The warn-level console output now includes a metadata trailer that may
contain provider-returned error strings with embedded tokens or keys
(e.g. `Bearer ...`, `sk-...`). Apply `redactMessage` to warn lines in
addition to error, matching the new surface area.

* 🔐 fix: Redact Sensitive Tokens Embedded in JSON Metadata

Two gaps in the existing console redaction that became user-visible once
warn/error lines started emitting structured metadata:

1. The OpenAI-key regex (`/^(sk-)[^\s]+/`) was anchored to start-of-line,
   so keys embedded inside JSON payloads (e.g. `{"apiKey":"sk-..."}`)
   were never redacted. Every console line begins with a timestamp, so
   the anchor effectively made this pattern dead code.
2. `formatConsoleMeta` stringified metadata values verbatim; a sensitive
   string value was only redacted by the whole-line regex pass, which
   missed the anchored `sk-` case above.

Fix:
- Drop the `^` anchor; add `/g` so every occurrence is redacted, not just
  the first.
- Also exclude `"` and `'` from the token body so JSON-embedded values
  terminate at the closing quote rather than chewing into the next field.
- Simplify `redactMessage` to apply patterns directly (dropping the
  `getMatchingSensitivePatterns` filter) — the filter used `.test()`
  which has stateful behavior on `/g` regexes and is no longer needed.
- `formatConsoleMeta` now runs `redactMessage` over every string value
  before JSON serialization, so the metadata trailer is safe even on the
  warn path.
- Add regression tests covering both fixes.

Reviewed-by: Codex (P1 finding on PR LibreChat-AI#12737, commit 68c31b6).

* 🔐 fix: Redact Metadata in debugTraverse for Warn and Error

Relaxing the debug-only gate in debugTraverse (in commit 59371be)
routed warn/error records through the traversal path, which emits leaf
string values verbatim (via truncateLongStrings only). Because
DEBUG_LOGGING defaults to true, those records are also written to the
rotating debug log file — which means payloads like
`{ auth: 'Bearer ...' }` or `{ openaiKey: 'sk-...' }` were persisted
unredacted once my earlier change took effect.

Apply redactMessage to the final formatted string when the level is
warn or error. Debug-level behavior is unchanged (matching prior art).

Includes regression tests covering error/warn redaction and
debug-level preservation.

Reviewed-by: Codex (P1 finding on PR LibreChat-AI#12737, commit e288f7f).

* 🔐 fix: Anchor Secret Regexes at Word Boundaries to Prevent Over-Redaction

Removing the `^` anchor in commit e288f7f let the OpenAI-key regex match
anywhere in the line — including inside ordinary words like `task-runner`
or `mask-value`, where `sk-` appears mid-word. Non-secret text was being
rewritten to `task-[REDACTED]`, hiding real log content from operators.

- Anchor every sensitive-key pattern with `\b` so matches only fire at
  word boundaries.
- Constrain the OpenAI-key body to the documented charset
  (`[a-zA-Z0-9_-]+`) instead of the broader "not whitespace or quote"
  character class.
- Add `&` to the `key=` exclusion so a query-string value stops at the
  next parameter separator.
- Regression tests covering both the over-redaction cases (`task-runner`,
  `monkey=10`) and the intended redactions still firing.

Reviewed-by: Codex (P2 finding on PR LibreChat-AI#12737, commit c09d293).

* 🔐 fix: Redact Before Colorize To Survive ANSI Word-Boundary Interference

The console pipeline runs `redactFormat → colorize({ all: true }) → printf`.
With `all: true`, winston wraps `info.message` in ANSI escapes whose
trailing `m` is a word character. That means `\b(Bearer )…` placed at the
start of a colorized segment can fall on a (word,word) boundary and miss —
the earlier line-wise `redactMessage(line)` pass in printf suffers the
same issue because it runs after colorize.

Extend `redactFormat` to run for `warn` in addition to `error`, operating
on the raw pre-colorize `info.message` + `Symbol.for('message')` strings.
The later in-printf `redactMessage(line)` stays as a backstop, but the
primary redaction now happens where the regex can actually see the text.

Metadata redaction already operates on the raw info object via
`formatConsoleMeta`, so it was never affected by ANSI — no change there.

Includes regression tests for the new warn-level behavior and for the
info/debug no-op path.

Reviewed-by: Codex (P2 finding on PR LibreChat-AI#12737, commit fdb6b36).

* 🧹 fix: Prefer Structured Metadata Over Consumed Splat Args in Traversal

`debugTraverse` previously read `metadata[Symbol.for('splat')][0]` first
and only fell back to the structured metadata object. When a caller uses
printf interpolation alongside a metadata object — for example
`logger.warn('failed for %s', tenant, { provider })` — winston leaves the
*consumed* positional arg (`tenant`) in `SPLAT[0]` after interpolation.
The formatter would then append the tenant a second time and skip the
real metadata, regressing debug-file and `DEBUG_CONSOLE` output quality
now that warn/error share this path.

Prefer the structured metadata object (via `extractMetaObject`) and only
fall back to `SPLAT[0]` when there's nothing else, so the surviving log
line surfaces the actual key/value pairs regardless of call shape.

Reviewed-by: Codex (P2 finding on PR LibreChat-AI#12737, commit 1e43d63).

* 🧹 fix: Skip Consumed Splat Primitives in Warn/Error Debug Traversal

When no structured metadata is attached, winston still leaves consumed
`%s` / `%d` arguments in `Symbol.for('splat')`. Previous fix preferred
the structured object but still fell back to whatever sat at `SPLAT[0]`
— so `logger.warn('failed for %s', tenantId)` emitted
`failed for tenant-7 tenant-7` in the traversal path (debug file and
`DEBUG_CONSOLE`), now regressed outside of `debug` level because
warn/error share the path.

Only accept the splat fallback when the value is a plain object or an
array (structural data worth surfacing). Primitives there are almost
certainly consumed printf args and get skipped.

Regression tests cover the single-%s case and the array-as-metadata case
(which still surfaces through splat).

Reviewed-by: Codex (P2 finding on PR LibreChat-AI#12737, commit bccbf11).

* 🧹 fix: Skip Numeric Splat Keys When Extracting Log Metadata

When a caller passes a primitive as the second argument — e.g.
`logger.warn('Unhandled step creation type:', step.type)` — winston /
`format.splat()` can leave character-index keys (`"0"`, `"1"`, …) on the
`info` object. With the warn/error metadata trailer in play, those
synthetic artifacts were being surfaced as bogus metadata, producing
noisy console and debug-file output.

Filter out numeric-string keys in `extractMetaObject` so only real
metadata fields reach the trailer. Added a regression test.

Reviewed-by: Codex (P2 finding on PR LibreChat-AI#12737, commit b34628d).

* 🧹 fix: Preserve Unconsumed Primitive Splat Args in Debug Traversal

The previous round dropped every primitive SPLAT[0] value to avoid
duplicating consumed %s args, but that removed useful context from
calls like \`logger.debug('prefix:', detail)\` where the primitive was
never interpolated — users lost the \`detail\` value.

Refine the heuristic: skip a primitive splat value only when it already
appears inside the (post-interpolation) \`info.message\`; otherwise
surface it. Arrays and objects continue to surface unconditionally.

Regression test covers the 'prefix:', detail case.

Reviewed-by: Codex (P2 finding on PR LibreChat-AI#12737, commit 6bf9548).

* 🧹 fix: Traverse Filtered Metadata, Not Raw Metadata, In debugTraverse

`debugTraverse` computes `extracted = extractMetaObject(metadata)` to
strip reserved keys, underscore-prefixed internals, and numeric splat
artifacts — but the later \`klona(metadata)\` + \`traverse\` path still
read the raw object, putting all the filtered junk back into the
rendered multi-line output.

Clone and traverse \`debugValue\` (the already-filtered object) instead.
Regression test exercises the case where numeric splat artifacts sit
alongside a real metadata field.

Reviewed-by: Codex (P2 finding on PR LibreChat-AI#12737, commit c29c18e).

* 🧹 refactor: Split Warn/Error From Debug-Level Traversal in Parsers

Retrofitting `debugTraverse`'s multi-line object walker to cover
warn/error created a minefield of splat-interaction edge cases
(numeric artifacts, consumed %s args, bogus `_`-prefix filtering,
over-eager suppression of unconsumed primitives). Each fix kept
introducing new corner cases.

Split the two concerns instead:

- Warn/error now emit a compact single-line JSON metadata trailer via
  `formatConsoleMeta`, then pass the full line through `redactMessage`.
  This mirrors what the console formatter already does, so behavior
  between the console and debug-file outputs stays consistent for
  warn/error — and none of the splat/traversal edge cases apply.
- Debug level keeps its original code path verbatim (including the
  raw `metadata` traversal and SPLAT\[0\] fallback). No regressions from
  my earlier iterations.
- `extractMetaObject` no longer filters underscore-prefixed keys, so
  legitimate fields like MongoDB `_id` still appear. Reserved winston
  keys and numeric splat artifacts remain filtered.

Updated tests reflect the simpler contract (underscore preservation,
single-line trailer expectations already covered).

Reviewed-by: Codex (two P2 findings on PR LibreChat-AI#12737, commit 9ea1152:
`_id` regression and over-eager primitive suppression).

* 🧹 fix: Preserve Scalar Metadata When One Value Is Circular

`formatConsoleMeta` previously wrapped a single `JSON.stringify` in
try/catch — any circular reference inside any field (e.g. an attached
request/response object) caused the entire trailer to be dropped. That
defeats the goal of making failures diagnosable: one malformed field
would mask the provider/model/status we wanted to surface.

Use a `WeakSet`-based replacer that emits `[Circular]` for repeated
object visits. On the whole-object serialization failing, fall back to
per-field serialization so scalar keys always land and only the
offending field is replaced with \`"[Unserializable]"\`.

Reviewed-by: Codex (P2 finding on PR LibreChat-AI#12737, commit d63742a).

* 🧹 fix: Address Audit Findings (JSDoc, Case-Insensitive Api-Key, Tests)

Audit review identified several MINOR/NIT items on top of the codex
rounds. This commit closes the actionable ones:

- **JSDoc (#1, #2)**: `extractMetaObject` no longer claims to filter
  underscore-prefixed keys (that filter was removed intentionally for
  MongoDB `_id`). `debugTraverse`'s docblock now describes the three
  code paths (warn/error compact trailer, debug multi-line traversal,
  other levels).
- **Case-insensitive api-key regex (#6)**: `/gi` so the Azure style
  `Api-Key:` / `API-KEY:` also gets redacted. Pre-existing behavior
  was lowercase-only.
- **Consolidated redundant branch (#5)**: `consoleFormat` printf was
  checking `isError || isWarn` twice; merged into one block.
- **Pre-compiled regex (#9)**: `NUMERIC_KEY_RE` moved to module scope.
- **Test coverage (#3, #4)**: Added regression tests for
  - per-field serialization fallback when a value's `toJSON` throws,
  - sensitive strings nested inside metadata objects,
  - the Azure-style `Api-Key:` header.
danny-avila added a commit that referenced this pull request Apr 21, 2026
Review follow-ups on #12756:

- #1 timer leak: stash the copy-state `setTimeout` in a ref and clear it
  from a `useEffect` cleanup so unmounting the Settings dialog mid-toast
  doesn't fire `setCopied(false)` on an unmounted component.
- #3 flash of About tab: gate `aboutEnabled` on `startupConfig != null`
  so the tab stays hidden until `/api/config` returns. For admins who
  disabled `interface.buildInfo`, the tab no longer briefly appears and
  vanishes on page load.
- #6 aria-live placement: move the live region off the interactive
  button onto a dedicated `<span role="status" aria-live="polite">` so
  screen readers announce the copied state, not the full button content
  on every re-render.
- #2 missing coverage: add `About.spec.tsx` exercising populated/empty
  buildInfo rendering, invalid-date handling, diagnostics clipboard
  payload, copy-state toggling, unmount cleanup, and the live region.
krgokul referenced this pull request in syedhabib39/LibreChat Apr 21, 2026
…hat-AI#12737)

* 🔊 fix: Preserve Log Metadata on Console for Warn/Error Levels

The default console formatter discarded every structured metadata key on the
winston info object — only `CONSOLE_JSON=true` preserved it. That meant
failures emitted by the agents SDK (e.g. "Summarization LLM call failed")
reached stdout without the provider, model, or underlying error attached,
leaving users unable to diagnose the root cause.

- Add `formatConsoleMeta` helper to serialize non-reserved metadata as a
  compact JSON trailer, with per-value string truncation and safe handling
  of circular references.
- Append the metadata trailer to warn/error console lines; info/debug
  behavior is unchanged.
- Relax `debugTraverse`'s debug-only gate so warn/error messages routed
  through the debug formatter also surface their metadata.
- Add a `formatConsoleMeta` stub to the shared logger mock so existing
  tests keep working.

* 🔐 fix: Also Redact Sensitive Patterns on Warn Console Lines

The warn-level console output now includes a metadata trailer that may
contain provider-returned error strings with embedded tokens or keys
(e.g. `Bearer ...`, `sk-...`). Apply `redactMessage` to warn lines in
addition to error, matching the new surface area.

* 🔐 fix: Redact Sensitive Tokens Embedded in JSON Metadata

Two gaps in the existing console redaction that became user-visible once
warn/error lines started emitting structured metadata:

1. The OpenAI-key regex (`/^(sk-)[^\s]+/`) was anchored to start-of-line,
   so keys embedded inside JSON payloads (e.g. `{"apiKey":"sk-..."}`)
   were never redacted. Every console line begins with a timestamp, so
   the anchor effectively made this pattern dead code.
2. `formatConsoleMeta` stringified metadata values verbatim; a sensitive
   string value was only redacted by the whole-line regex pass, which
   missed the anchored `sk-` case above.

Fix:
- Drop the `^` anchor; add `/g` so every occurrence is redacted, not just
  the first.
- Also exclude `"` and `'` from the token body so JSON-embedded values
  terminate at the closing quote rather than chewing into the next field.
- Simplify `redactMessage` to apply patterns directly (dropping the
  `getMatchingSensitivePatterns` filter) — the filter used `.test()`
  which has stateful behavior on `/g` regexes and is no longer needed.
- `formatConsoleMeta` now runs `redactMessage` over every string value
  before JSON serialization, so the metadata trailer is safe even on the
  warn path.
- Add regression tests covering both fixes.

Reviewed-by: Codex (P1 finding on PR LibreChat-AI#12737, commit 68c31b6).

* 🔐 fix: Redact Metadata in debugTraverse for Warn and Error

Relaxing the debug-only gate in debugTraverse (in commit 59371be)
routed warn/error records through the traversal path, which emits leaf
string values verbatim (via truncateLongStrings only). Because
DEBUG_LOGGING defaults to true, those records are also written to the
rotating debug log file — which means payloads like
`{ auth: 'Bearer ...' }` or `{ openaiKey: 'sk-...' }` were persisted
unredacted once my earlier change took effect.

Apply redactMessage to the final formatted string when the level is
warn or error. Debug-level behavior is unchanged (matching prior art).

Includes regression tests covering error/warn redaction and
debug-level preservation.

Reviewed-by: Codex (P1 finding on PR LibreChat-AI#12737, commit e288f7f).

* 🔐 fix: Anchor Secret Regexes at Word Boundaries to Prevent Over-Redaction

Removing the `^` anchor in commit e288f7f let the OpenAI-key regex match
anywhere in the line — including inside ordinary words like `task-runner`
or `mask-value`, where `sk-` appears mid-word. Non-secret text was being
rewritten to `task-[REDACTED]`, hiding real log content from operators.

- Anchor every sensitive-key pattern with `\b` so matches only fire at
  word boundaries.
- Constrain the OpenAI-key body to the documented charset
  (`[a-zA-Z0-9_-]+`) instead of the broader "not whitespace or quote"
  character class.
- Add `&` to the `key=` exclusion so a query-string value stops at the
  next parameter separator.
- Regression tests covering both the over-redaction cases (`task-runner`,
  `monkey=10`) and the intended redactions still firing.

Reviewed-by: Codex (P2 finding on PR LibreChat-AI#12737, commit c09d293).

* 🔐 fix: Redact Before Colorize To Survive ANSI Word-Boundary Interference

The console pipeline runs `redactFormat → colorize({ all: true }) → printf`.
With `all: true`, winston wraps `info.message` in ANSI escapes whose
trailing `m` is a word character. That means `\b(Bearer )…` placed at the
start of a colorized segment can fall on a (word,word) boundary and miss —
the earlier line-wise `redactMessage(line)` pass in printf suffers the
same issue because it runs after colorize.

Extend `redactFormat` to run for `warn` in addition to `error`, operating
on the raw pre-colorize `info.message` + `Symbol.for('message')` strings.
The later in-printf `redactMessage(line)` stays as a backstop, but the
primary redaction now happens where the regex can actually see the text.

Metadata redaction already operates on the raw info object via
`formatConsoleMeta`, so it was never affected by ANSI — no change there.

Includes regression tests for the new warn-level behavior and for the
info/debug no-op path.

Reviewed-by: Codex (P2 finding on PR LibreChat-AI#12737, commit fdb6b36).

* 🧹 fix: Prefer Structured Metadata Over Consumed Splat Args in Traversal

`debugTraverse` previously read `metadata[Symbol.for('splat')][0]` first
and only fell back to the structured metadata object. When a caller uses
printf interpolation alongside a metadata object — for example
`logger.warn('failed for %s', tenant, { provider })` — winston leaves the
*consumed* positional arg (`tenant`) in `SPLAT[0]` after interpolation.
The formatter would then append the tenant a second time and skip the
real metadata, regressing debug-file and `DEBUG_CONSOLE` output quality
now that warn/error share this path.

Prefer the structured metadata object (via `extractMetaObject`) and only
fall back to `SPLAT[0]` when there's nothing else, so the surviving log
line surfaces the actual key/value pairs regardless of call shape.

Reviewed-by: Codex (P2 finding on PR LibreChat-AI#12737, commit 1e43d63).

* 🧹 fix: Skip Consumed Splat Primitives in Warn/Error Debug Traversal

When no structured metadata is attached, winston still leaves consumed
`%s` / `%d` arguments in `Symbol.for('splat')`. Previous fix preferred
the structured object but still fell back to whatever sat at `SPLAT[0]`
— so `logger.warn('failed for %s', tenantId)` emitted
`failed for tenant-7 tenant-7` in the traversal path (debug file and
`DEBUG_CONSOLE`), now regressed outside of `debug` level because
warn/error share the path.

Only accept the splat fallback when the value is a plain object or an
array (structural data worth surfacing). Primitives there are almost
certainly consumed printf args and get skipped.

Regression tests cover the single-%s case and the array-as-metadata case
(which still surfaces through splat).

Reviewed-by: Codex (P2 finding on PR LibreChat-AI#12737, commit bccbf11).

* 🧹 fix: Skip Numeric Splat Keys When Extracting Log Metadata

When a caller passes a primitive as the second argument — e.g.
`logger.warn('Unhandled step creation type:', step.type)` — winston /
`format.splat()` can leave character-index keys (`"0"`, `"1"`, …) on the
`info` object. With the warn/error metadata trailer in play, those
synthetic artifacts were being surfaced as bogus metadata, producing
noisy console and debug-file output.

Filter out numeric-string keys in `extractMetaObject` so only real
metadata fields reach the trailer. Added a regression test.

Reviewed-by: Codex (P2 finding on PR LibreChat-AI#12737, commit b34628d).

* 🧹 fix: Preserve Unconsumed Primitive Splat Args in Debug Traversal

The previous round dropped every primitive SPLAT[0] value to avoid
duplicating consumed %s args, but that removed useful context from
calls like \`logger.debug('prefix:', detail)\` where the primitive was
never interpolated — users lost the \`detail\` value.

Refine the heuristic: skip a primitive splat value only when it already
appears inside the (post-interpolation) \`info.message\`; otherwise
surface it. Arrays and objects continue to surface unconditionally.

Regression test covers the 'prefix:', detail case.

Reviewed-by: Codex (P2 finding on PR LibreChat-AI#12737, commit 6bf9548).

* 🧹 fix: Traverse Filtered Metadata, Not Raw Metadata, In debugTraverse

`debugTraverse` computes `extracted = extractMetaObject(metadata)` to
strip reserved keys, underscore-prefixed internals, and numeric splat
artifacts — but the later \`klona(metadata)\` + \`traverse\` path still
read the raw object, putting all the filtered junk back into the
rendered multi-line output.

Clone and traverse \`debugValue\` (the already-filtered object) instead.
Regression test exercises the case where numeric splat artifacts sit
alongside a real metadata field.

Reviewed-by: Codex (P2 finding on PR LibreChat-AI#12737, commit c29c18e).

* 🧹 refactor: Split Warn/Error From Debug-Level Traversal in Parsers

Retrofitting `debugTraverse`'s multi-line object walker to cover
warn/error created a minefield of splat-interaction edge cases
(numeric artifacts, consumed %s args, bogus `_`-prefix filtering,
over-eager suppression of unconsumed primitives). Each fix kept
introducing new corner cases.

Split the two concerns instead:

- Warn/error now emit a compact single-line JSON metadata trailer via
  `formatConsoleMeta`, then pass the full line through `redactMessage`.
  This mirrors what the console formatter already does, so behavior
  between the console and debug-file outputs stays consistent for
  warn/error — and none of the splat/traversal edge cases apply.
- Debug level keeps its original code path verbatim (including the
  raw `metadata` traversal and SPLAT\[0\] fallback). No regressions from
  my earlier iterations.
- `extractMetaObject` no longer filters underscore-prefixed keys, so
  legitimate fields like MongoDB `_id` still appear. Reserved winston
  keys and numeric splat artifacts remain filtered.

Updated tests reflect the simpler contract (underscore preservation,
single-line trailer expectations already covered).

Reviewed-by: Codex (two P2 findings on PR LibreChat-AI#12737, commit 9ea1152:
`_id` regression and over-eager primitive suppression).

* 🧹 fix: Preserve Scalar Metadata When One Value Is Circular

`formatConsoleMeta` previously wrapped a single `JSON.stringify` in
try/catch — any circular reference inside any field (e.g. an attached
request/response object) caused the entire trailer to be dropped. That
defeats the goal of making failures diagnosable: one malformed field
would mask the provider/model/status we wanted to surface.

Use a `WeakSet`-based replacer that emits `[Circular]` for repeated
object visits. On the whole-object serialization failing, fall back to
per-field serialization so scalar keys always land and only the
offending field is replaced with \`"[Unserializable]"\`.

Reviewed-by: Codex (P2 finding on PR LibreChat-AI#12737, commit d63742a).

* 🧹 fix: Address Audit Findings (JSDoc, Case-Insensitive Api-Key, Tests)

Audit review identified several MINOR/NIT items on top of the codex
rounds. This commit closes the actionable ones:

- **JSDoc (#1, #2)**: `extractMetaObject` no longer claims to filter
  underscore-prefixed keys (that filter was removed intentionally for
  MongoDB `_id`). `debugTraverse`'s docblock now describes the three
  code paths (warn/error compact trailer, debug multi-line traversal,
  other levels).
- **Case-insensitive api-key regex (#6)**: `/gi` so the Azure style
  `Api-Key:` / `API-KEY:` also gets redacted. Pre-existing behavior
  was lowercase-only.
- **Consolidated redundant branch (#5)**: `consoleFormat` printf was
  checking `isError || isWarn` twice; merged into one block.
- **Pre-compiled regex (#9)**: `NUMERIC_KEY_RE` moved to module scope.
- **Test coverage (#3, #4)**: Added regression tests for
  - per-field serialization fallback when a value's `toJSON` throws,
  - sensitive strings nested inside metadata objects,
  - the Azure-style `Api-Key:` header.
danny-avila added a commit that referenced this pull request Apr 21, 2026
…ing (#12649)

* feat: Skill runtime integration — catalog injection, tool registration, execute handler

Wires the @librechat/agents SkillTool primitive into LibreChat's agent runtime:

**Enums:**
- Add `skills` to AgentCapabilities + defaultAgentCapabilities

**Data layer:**
- Add `getSkillByName(name, accessibleIds)` — compound query that
  combines name lookup + ACL check in one findOne

**Agent initialization (packages/api/src/agents/initialize.ts):**
- Accept `accessibleSkillIds` param and `listSkillsByAccess` db method
- Query accessible skills, format catalog via `formatSkillCatalog()`,
  append to `additional_instructions` (appears in agent system prompt)
- Register `SkillToolDefinition` + `createSkillTool()` when catalog
  is non-empty (tool appears in model's tool list)
- Store `accessibleSkillIds` and `skillCount` on InitializedAgent

**Execute handler (packages/api/src/agents/handlers.ts):**
- Add `getSkillByName` to `ToolExecuteOptions`
- `handleSkillToolCall()` intercepts `Constants.SKILL_TOOL`:
  extracts skillName, loads body from DB with ACL check,
  substitutes $ARGUMENTS, returns ToolExecuteResult with
  injectedMessages (skill body as isMeta user message)

**Caller wiring:**
- initialize.js: query skill IDs via findAccessibleResources,
  pass to initializeAgent + store on agentToolContexts,
  add getSkillByName to toolExecuteOptions,
  pass accessibleSkillIds through loadTools configurable
- openai.js + responses.js: same pattern for their flows

Requires @librechat/agents >= 3.1.65 (PR #91 exports).

* feat: Skills toggle in tools menu + backend capability gating

Frontend:
- Add skills?: boolean to TEphemeralAgent type
- Add LAST_SKILLS_TOGGLE_ to LocalStorageKeys for persistence
- Add skillsEnabled to useAgentCapabilities hook
- Add skills useToolToggle to BadgeRowContext with localStorage init
- New Skills.tsx badge component (Scroll icon, cyan theme,
  permission-gated via PermissionTypes.SKILLS)
- Add skills entry to ToolsDropdown with toggle + pin
- Render Skills badge in BadgeRow ephemeral section

Backend:
- Extract injectSkillCatalog() into packages/api/src/agents/skills.ts
  (reduces initializeAgent module size, reusable helper)
- initializeAgent delegates to helper instead of inline block
- Capability-gate the findAccessibleResources query:
  - Agents endpoint: checks AgentCapabilities.skills in admin config
  - OpenAI/Responses controllers: checks ephemeralAgent.skills toggle
- ACL query runs once per run, result shared across all agents

* refactor: remove createSkillTool() instance from injectSkillCatalog

SkillTool is event-driven only. The tool definition in toolDefinitions
is sufficient for the LLM to see the tool schema. No tool instance is
needed since the host handler intercepts via ON_TOOL_EXECUTE before
tool.invoke() is ever called.

Removes tools from InjectSkillCatalogParams/Result, drops the
createSkillTool import.

* feat: skill file priming, bash tool, and invoked skills state

Multi-file skill support:
- New primeSkillFiles() helper (packages/api/src/agents/skillFiles.ts)
  uploads skill files + SKILL.md body to code execution environment
- handleSkillToolCall primes files on invocation when skill.fileCount > 0,
  returns session info as artifact so ToolNode stores the session
- Skill-primed files available to subsequent bash/code tool calls

Bash tool auto-registration:
- BashExecutionToolDefinition added alongside SkillToolDefinition when
  skills are enabled, giving the model a bash tool for running scripts

Conversation state:
- Add invokedSkillIds field to conversation schema (Mongoose + Zod)
- handleSkillToolCall updates conversation with $addToSet on success
- Enables re-priming skill files on subsequent runs (future)

Dependency wiring:
- Pass listSkillFiles, getStrategyFunctions, uploadCodeEnvFile,
  updateConversation through ToolExecuteOptions
- Pass req and codeApiKey through mergedConfigurable
- All three controller entry points wired (initialize.js, openai.js,
  responses.js)

* fix: load bash_tool instance in loadToolsForExecution, remove file listing

- Add createBashExecutionTool to loadToolsForExecution alongside PTC/ToolSearch
  pattern: loads CODE_API_KEY, creates bash tool instance on demand
- Add BASH_TOOL and SKILL_TOOL to specialToolNames set so they don't go
  through the generic loadTools path (bash is created here, skill is
  intercepted in handler before tool.invoke)
- Remove file name listing from skill content text — it's the skill
  author's responsibility to disclose files in SKILL.md, not the framework

* feat: batch upload for skill files, replace sequential uploads

- Add batchUploadCodeEnvFiles() to crud.js: single POST to /upload/batch
  with all files in one multipart request, returns shared session_id
- Rewrite primeSkillFiles to collect all streams (SKILL.md + bundled files)
  then do one batch upload instead of N sequential uploads
- Replace uploadCodeEnvFile with batchUploadCodeEnvFiles across all callers
  (handlers.ts, initialize.js, openai.js, responses.js)

* refactor: remove invokedSkillIds from conversation schema

Skills aren't re-loaded between runs, so conversation-level state for
invoked skills doesn't help. Skill state will live on messages instead
(like tool_search discoveredTools and summaries), enabling in-place
re-injection on follow-up runs.

Removes invokedSkillIds from: convo Mongoose schema, IConversation
interface, Zod schema, ToolExecuteOptions.updateConversation, and
all three caller wiring points.

* feat: smart skill file re-priming with session freshness checking

Schema:
- Add codeEnvIdentifier field to ISkillFile (type + Mongoose schema)
- Add updateSkillFileCodeEnvIds batch method (uses tenantSafeBulkWrite)
- Export checkIfActive from Code/process.js

Extraction:
- Add extractInvokedSkillsFromHistory() to run.ts — scans message
  history for AIMessage tool_calls where name === 'skill', extracts
  skillName args. Follows same pattern as extractDiscoveredToolsFromHistory.

Smart re-priming in primeSkillFiles:
- Before batch uploading, checks if existing codeEnvIdentifiers are
  still active via getSessionInfo + checkIfActive (23h threshold)
- If session is still active, returns cached references (zero uploads)
- If stale or missing, batch-uploads everything and persists new
  identifiers on SkillFile documents (fire-and-forget)
- Single session check covers all files (batch shares one session_id)

Wiring:
- Pass getSessionInfo, checkIfActive, updateSkillFileCodeEnvIds
  through ToolExecuteOptions and all three controller entry points

* feat: wire skill file re-priming at run start via initialSessions

Flow:
1. initialize.js creates primeInvokedSkills callback with all deps
2. client.js calls it with message history before createRun
3. extractInvokedSkillsFromHistory scans for skill tool calls
4. For each invoked skill with files, primeSkillFiles uploads/checks
5. Returns initialSessions map passed to createRun
6. createRun passes initialSessions to Run.create (via RunConfig)
7. Run constructor seeds Graph.sessions, making skill files available
   to subsequent bash/code tool calls via ToolNode session injection

Requires @librechat/agents with initialSessions on RunConfig (PR #94).

* refactor: use CODE_EXECUTION_TOOLS set for code tool checks

Import CODE_EXECUTION_TOOLS from @librechat/agents and replace inline
constant checks in handlers.ts and callbacks.js. Fixes missing bash
tool coverage in the session context injection (handlers.ts) and code
output processing (callbacks.js).

* refactor: move primeInvokedSkills to packages/api, add skill body re-injection

Moves primeInvokedSkills from an inline closure in initialize.js (with
dynamic requires) to a proper exported function in packages/api
skillFiles.ts with explicit typed dependencies.

Key changes:
- primeInvokedSkills now returns both initialSessions (for file priming)
  AND injectedMessages (skill bodies for context continuity)
- createRun accepts invokedSkillMessages and appends skill bodies to
  systemContent so the model retains skill instructions across runs
- initialize.js calls the packaged function with all deps passed explicitly
- client.js passes both initialSessions and injectedMessages to createRun

* fix: move dynamic requires to top-level module imports

Move primeInvokedSkills, getStrategyFunctions, batchUploadCodeEnvFiles,
getSessionInfo, and checkIfActive from inline requires to top-level
module requires where they belong.

* refactor: skill body reconstruction via formatAgentMessages, not systemContent

Replaces the lazy systemContent approach with proper message-level
reconstruction:

SDK (formatAgentMessages):
- New invokedSkillBodies param (Map<string, string>)
- Reconstructs HumanMessages after skill ToolMessages at the correct
  position in the message sequence, matching where ToolNode originally
  injected them

LibreChat:
- extractInvokedSkillsFromPayload replaces extractInvokedSkillsFromHistory
  (works with raw TPayload before formatAgentMessages, not BaseMessage[])
- primeInvokedSkills now takes payload instead of messages, returns
  skillBodies Map instead of injectedMessages
- client.js calls primeInvokedSkills BEFORE formatAgentMessages, passes
  skillBodies through as the 4th param
- Removed invokedSkillMessages from createRun (no more systemContent hack)
- Single-pass: skill detection happens inside formatAgentMessages' existing
  tool_call processing loop, zero extra message iterations

* refactor: rename skillBodies to skills for consistency with SDK param

* refactor: move auth loading into primeInvokedSkills, pass loadAuthValues as dep

The payload/accessibleSkillIds guard and CODE_API_KEY loading now live
inside primeInvokedSkills (packages/api) rather than in the CJS caller.
initialize.js passes loadAuthValues as a dependency and the callback
is only created when skillsCapabilityEnabled.

* feat: ReadFile tool + conditional bash registration + skill path namespacing

ReadFile tool (read_file):
- General-purpose file reader, event-driven (ON_TOOL_EXECUTE)
- Schema: { file_path: string } — "{skillName}/{path}" convention
- handleReadFileCall: resolves skill name from path, ACL check, reads
  from DB cache or storage, binary detection, size limits (256KB),
  lazy caching (512KB), line numbers in output
- SKILL.md special case: reads skill.body directly
- Dispatched alongside SKILL_TOOL in createToolExecuteHandler
- Added to specialToolNames in ToolService

Conditional tool registration:
- ReadFile + SkillTool: always registered when skills enabled
- BashTool: only registered when codeEnvAvailable === true
- codeEnvAvailable passed through InitializeAgentParams from caller

Skill file path namespacing:
- primeSkillFiles now uploads as "{skillName}/SKILL.md" and
  "{skillName}/{relativePath}" instead of flat names
- Prevents file collisions when multiple skills are invoked

Wiring:
- getSkillFileByPath + updateSkillFileContent passed through
  ToolExecuteOptions in all three callers

* feat: return images/PDFs as artifacts from read_file, tighten caching

Binary artifact support:
- Images (png, jpeg, gif, webp) returned as base64 in artifact.content
  with type: 'image_url', processed by existing callback attachment flow
- PDFs returned as base64 artifact similarly
- Binary size limit: 10MB (MAX_BINARY_BYTES)
- Other binary files still return metadata + bash fallback

Caching:
- Text cached only on first read (file.content == null check)
- Binary flag cached only on first detection (file.isBinary == null)
- Skill files are immutable; no redundant cache writes

Registration:
- ReadFileToolDefinition now includes responseFormat: 'content_and_artifact'

* chore: update @librechat/agents to version 3.1.66-dev.0 and add peer dependencies in package-lock.json and package.json files

* fix: resolve review findings #1,#2,#4,#5,#6,#10,#13

Critical:
- #1: primeInvokedSkills now accumulates files across all skills into
  one session entry instead of overwriting. Parallel processing via
  Promise.allSettled.
- #2: codeEnvAvailable now computed and passed in openai.js and
  responses.js (was missing, bash tool never registered in those flows)

Major:
- #4: relativePath in updateSkillFileCodeEnvIds now strips the
  {skillName}/ prefix to match SkillFile documents. SKILL.md filter
  uses endsWith instead of exact match.
- #5: File priming guarded on apiKey being non-empty (skip when not
  configured instead of failing with auth error)
- #6: Skills processed in parallel via Promise.allSettled instead of
  sequential for-of loop

Minor:
- #10: Use top-level imports in initialize.js instead of inline requires
- #13: Log warning when skill catalog reaches the 100-skill limit

* fix: resolve followup review findings N1,N2,N4

N1 (CRITICAL): Wire skill deps into responses.js non-streaming path.
Was completely missing getSkillByName, file strategy functions, etc.

N2 (MAJOR): Single batch upload for ALL skills' files. Resolves skills
in parallel (Phase 1), then collects all file streams across skills
and does ONE batchUploadCodeEnvFiles call (Phase 2). All files share
one session_id, eliminating cross-session isolation issues.

N4 (MINOR): Move inline require() to top-level in openai.js and
responses.js, consistent with initialize.js.

* fix: add mocks for new file strategy imports in controller tests

* fix: restore session freshness check, parallelize file lookups, add warnings

R1: Re-add session freshness check before batch upload. Checks any
existing codeEnvIdentifier via getSessionInfo + checkIfActive. If the
session is still active (23h window), returns cached file references
with zero re-uploads.

R2: listSkillFiles calls parallelized via Promise.all (were sequential
in the for-of loop).

R3: Log warning when skill record lookup fails during identifier
persistence (was a silent empty-string fallback).

* fix: guard freshness cache on single-session consistency

* fix: multi-session freshness check (code env handles mixed sessions natively)

The code execution environment fetches each file by its own
{session_id, fileId} pair independently — no single-session
requirement. Removed the sessionIds.size === 1 guard.

Now checks ALL distinct sessions for freshness. If every session
is still active (23h window), returns cached references with per-file
session_ids preserved. If any session expired, falls through to
re-upload everything in a single batch.

* perf: parallelize session freshness checks via Promise.all

* fix: add optional chaining for session info retrieval in primeInvokedSkills

Updated the primeInvokedSkills function to use optional chaining for getSessionInfo and checkIfActive methods, ensuring safer access and preventing potential runtime errors when these methods are undefined.

* fix: address review findings #1-#9 + Codex P1/P2 + session probe

Critical:
- #1/Codex P1: Add codeApiKey loading to openai.js and responses.js
  loadTools configurable (was missing, file priming broken in 2/3 paths)
- Codex P1: Fix cached file name prefix in primeSkillFiles cache path
  (was sf.relativePath, now ${skill.name}/${sf.relativePath})

Major:
- Codex P2: Honor ephemeral skills toggle in agents endpoint
  (check ephemeralAgent?.skills !== false alongside admin capability)
- #4: Early size check using file.bytes from DB before streaming
  (prevents full-file buffer for oversized files)

Minor:
- #5: Replace Record<string, any> with Record<string, boolean | string>
- #6: Localize Pin/Unpin aria-labels with com_ui_pin/com_ui_unpin
- #8: Parallelize stream acquisition in primeSkillFiles via
  Promise.allSettled
- #9: Log warning for partial batch upload failures with filenames

Performance:
- Session probe optimization: getSessionInfo now hits per-object
  endpoint (GET /sessions/{sid}/objects/{fid}) instead of listing
  entire session (GET /files/{sid}?detail=summary). O(1) stat vs
  O(N) list + linear scan.

* refactor: extract shared skill wiring helper + add unit tests

DRY (#3):
- New skillDeps.js exports getSkillToolDeps() with all 9 skill-related
  deps (getSkillByName, listSkillFiles, getStrategyFunctions, etc.)
- Replaces 5 identical copy-paste blocks across initialize.js, openai.js,
  responses.js (streaming + non-streaming paths)
- One place to maintain when skill deps change

Tests (#2):
- 8 unit tests for extractInvokedSkillsFromPayload covering:
  string args, object args, missing skill tool_calls, non-assistant
  messages, malformed JSON, empty skillName, empty payload, dedup

* fix: remove @jest/globals import, use global jest env

* fix: resolve round 2 review findings R2-1 through R2-7

R2-1 (toggle semantics): openai.js + responses.js now check admin
  capability (AgentCapabilities.skills) alongside ephemeral toggle.
  Aligns with initialize.js.

R2-2 (swallowed error): primeInvokedSkills now logs
  updateSkillFileCodeEnvIds failures (was .catch(() => {}))

R2-4 (test cast): Record<string, string> → Record<string, unknown>

R2-5 (DRY regression): Extract enrichWithSkillConfigurable() into
  skillDeps.js. Replaces 4 identical loadAuthValues blocks.
  Each loadTools callback is now a one-liner. JSDoc added (R2-6).

R2-7 (sequential streams): primeInvokedSkills now uses
  Promise.allSettled for parallel stream acquisition.

* fix: require explicit skills toggle + treat partial cache as miss

- initialize.js: change ephemeralSkillsToggle !== false to === true
  (unset toggle no longer enables skills)
- primeSkillFiles cache: require ALL files to have codeEnvIdentifier
  before using cache (partial persistence = cache miss = re-upload)
- primeInvokedSkills cache: same check (allFilesWithIds.length must
  equal total file count)

* fix: pass entity_id=skillId on batch upload, eliminates per-user cache thrashing

primeSkillFiles now passes entity_id: skill._id.toString() to
batchUploadCodeEnvFiles. This scopes the code env session to the
skill, not the user. All users sharing a skill share the same
uploaded files — no more cache thrashing from overwriting each
other's codeEnvIdentifier.

The stored codeEnvIdentifier now includes ?entity_id= suffix so
freshness checks pass the entity_id through to the per-object
stat endpoint. Both primeSkillFiles and primeInvokedSkills
store consistent identifier formats.

* fix: pass entity_id on multi-skill batch upload, consistent identifier format

* Revert "fix: pass entity_id on multi-skill batch upload, consistent identifier format"

This reverts commit c85ce21.

* refactor: per-skill upload in primeInvokedSkills, eliminate multi-skill batch

Replace the monolithic multi-skill batch upload with per-skill
primeSkillFiles calls. Each skill gets its own session with
entity_id=skillId, ensuring:

- Correct session auth (entity_id matches on freshness checks)
- Per-skill freshness caching (only expired skills re-upload)
- Shared skill sessions work across users (same entity_id=skillId)
- Code env handles mixed session_ids natively

The big batch block (stream collection, single upload, identifier
mapping) is replaced by a simple loop over primeSkillFiles, which
already handles freshness caching, batch upload, and identifier
persistence per-skill.

* fix: resolve review findings #1,#3-5,#7,#9-11

Critical:
- #1: Strip ?entity_id= query string before splitting codeEnvIdentifier
  into session_id/fileId (was corrupting cached file IDs in 4 locations)

Major:
- #4: Parallelize per-skill primeSkillFiles via Promise.allSettled
- #5: Add logger.warn to all empty .catch(() => {}) on cache writes

Minor:
- #7: Add logger.debug to enrichWithSkillConfigurable catch block
- #9: Use error instanceof Error guard in batchUploadCodeEnvFiles
- #10: Move enrichWithSkillConfigurable to TypeScript in packages/api
  (skillConfigurable.ts), skillDeps.js wraps with loadAuthValues dep
- #11: Reduce MAX_BINARY_BYTES from 10MB to 5MB (~11.5MB peak with b64)

* fix: forward entity_id in session probe + always register bash tool

Codex P2 (entity_id in probe): getSessionInfo now preserves and
forwards query params (including entity_id) to the per-object stat
endpoint. Without this, identifiers stored as ...?entity_id=... would
fail auth checks because the entity_id scope was dropped.

Codex P2 (bash tool availability): Remove codeEnvAvailable gate from
injectSkillCatalog. Bash tool definition is now always registered when
skills are enabled. Actual tool instance creation still happens at
execution time in loadToolsForExecution (which loads per-user
credentials). This ensures users with per-user CODE_API_KEY get
bash without requiring a global env var at init time.

Removes codeEnvAvailable from InjectSkillCatalogParams,
InitializeAgentParams, and all three controller entry points.

* fix: add debug logging to primeInvokedSkills catch, rename export alias

* fix: stub bash tool when no key + remove PDF artifact path

Codex P1 (bash tool): When CODE_API_KEY is unavailable, create a stub
tool that returns "Code execution is not available. Use read_file
instead." This prevents "tool not found" errors from the model
repeatedly calling bash_tool in no-code-env deployments while still
registering the definition for per-user credential users.

Codex P2 (PDF artifacts): Remove PDF image_url artifact path. The
host artifact pipeline processes image_url via saveBase64Image which
fails for PDFs. PDFs now fall through to the generic binary handler
("Use bash to process"). TODO comment for future document artifact
support.

Also: isImageOrPdf → isImage in early size checks (PDFs are no
longer treated as artifact candidates).

* fix: remove dead PDF_MIME constant, hoist skillToolDeps, document session_id

- #7: Remove unused PDF_MIME constant (dead code after PDF artifact removal)
- #11: Hoist skillToolDeps to module-level constant (avoid per-call allocation)
- #6: Document that CodeSessionContext.session_id is a representative value;
  ToolNode uses per-file session_id from the files array

* fix: call toolEndCallback for skill/read_file artifacts + clear codeEnvIdentifier on re-upload

Codex P1 (toolEndCallback bypass): skill and read_file handler branches
returned early, bypassing the toolEndCallback that processes artifacts
(image attachments). Now calls toolEndCallback when the result has an
artifact, using the same metadata pattern as the normal tool.invoke path.

Codex P1 (stale identifiers): upsertSkillFile now $unset's
codeEnvIdentifier alongside content and isBinary when a file is
re-uploaded. Prevents the freshness cache from returning references
to old file content after a skill file is replaced.

* fix: add session_id comment at cached path, rename skillResult to handlerResult

* fix: return content_and_artifact from bash stub so result.content is populated

* fix: deterministic skill lookup, dedup warning, and multi-session freshness check

- getSkillByName: add sort({updatedAt:-1}) so name collisions resolve
  deterministically to the most recently updated skill
- injectSkillCatalog: warn when multiple accessible skills share a name
- primeSkillFiles: check ALL distinct sessions for freshness, not just
  the first file's session, preventing stale refs after partial bulkWrite

* refactor: update icon import in Skills component

- Replaced the Scroll icon with ScrollText in the Skills component for improved clarity and consistency in the UI.

* fix: SKILL.md cache parity, gate bash_tool on code env, fix read_file too-large message

- primeSkillFiles: filter SKILL.md from returned files array on fresh
  upload so cached and non-cached paths return identical file sets
  (SKILL.md is still on disk in the session for bash access)
- injectSkillCatalog: only register bash_tool when codeEnvAvailable is
  true; thread the flag from all three CJS callers via execute_code
  capability check
- handleReadFileCall: tell the model to invoke the skill first before
  suggesting /mnt/data paths for oversized files

* fix: use EnvVar constant, deduplicate auth lookup, validate batch upload, stream byte limit

- Replace hardcoded 'LIBRECHAT_CODE_API_KEY' with EnvVar.CODE_API_KEY
  in skillConfigurable.ts and skillFiles.ts
- Resolve code API key once at run start in initialize.js and pass to
  both primeInvokedSkills and enrichWithSkillConfigurable via optional
  preResolvedCodeApiKey param, eliminating redundant loadAuthValues calls
- Add response structure validation in batchUploadCodeEnvFiles before
  accessing session_id/files to surface unexpected responses early
- Add streaming byte counter in handleReadFileCall that aborts and
  destroys the stream when accumulated bytes exceed MAX_BINARY_BYTES,
  preventing full file buffering when DB metadata is inaccurate

* refactor: update icon import in ToolsDropdown component

- Replaced the Scroll icon with ScrollText in the ToolsDropdown component for improved clarity and consistency in the UI.

* fix: partial upload failure detection, EnvVar in initialize.js, declaration ordering

- primeSkillFiles: return null (failure) when batch upload partially
  succeeds — missing bundled files would cause runtime bash/read
  failures with missing paths in code env
- initialize.js: replace hardcoded 'LIBRECHAT_CODE_API_KEY' with
  EnvVar.CODE_API_KEY imported from @librechat/agents
- initialize.js: move enabledCapabilities, accessibleSkillIds, and
  codeApiKey declarations before the toolExecuteOptions closure that
  references them (eliminates reliance on temporal dead zone hoisting)
danny-avila added a commit that referenced this pull request Apr 21, 2026
…efaults (#12692)

* feat: per-user skill active/inactive toggle with ownership-aware defaults

- Add `skillStates` map (Record<string, boolean>) to user schema for
  per-user active/inactive overrides on skills
- Add `defaultActiveOnShare` to interface.skills config (default: false)
  so admins can control whether shared skills auto-activate
- Add GET/POST /api/user/settings/skills/active endpoints with validation
- Add React Query hooks with optimistic mutations for skill states
- Add useSkillActiveState hook with ownership-aware resolution:
  owned skills default active, shared skills default inactive
- Add toggle switch UI to SkillListItem and SkillDetail components
- Filter inactive skills in injectSkillCatalog before agent injection
- Add localization keys for active/inactive labels

* fix: use Record instead of Map for IUser.skillStates

Mongoose .lean() flattens Map to a plain object, causing type
incompatibility with IUser in methods that return lean documents.

* fix: address review findings for skill active states

- Fail-closed when userId is absent: filter rejects all shared skills
  instead of passing them through unfiltered (Codex P1)
- Validate Mongoose Map key characters (reject . and $) in controller
  to return 400 instead of a 500 from schema validation (Codex P2)
- Block toggle while initial skill states query is loading to prevent
  overwriting server-side overrides with an empty snapshot (Codex P2)
- Extract shared SkillToggle component, eliminating duplicate toggle
  markup in SkillListItem and SkillDetail (Finding #3)
- Move skill state query/mutation hooks from Favorites.ts to
  Skills/queries.ts per feature-directory convention (Finding #4)
- Fix hardcoded English aria-label in SkillListItem by passing the
  localized string from the parent SkillList (Finding #5)
- Fix inline arrow in SkillList render loop: pass stable callback
  reference so SkillListItem memo() is not invalidated (Finding #1)
- Extract toRecord() helper in controller to DRY the Map-to-Object
  conversion (Finding #6)
- Remove Promise.resolve wrapping synchronous config read (Finding #8)
- Remove unused TUpdateSkillStatesRequest type (Finding #12)

* fix: forward tabIndex on SkillToggle to preserve list keyboard nav

The original inline toggle had tabIndex={-1} so the row itself
remained the sole tab target. The extraction into SkillToggle
dropped this prop, making every list toggle a tab stop. Add an
optional tabIndex prop and pass -1 from SkillListItem.

* fix: plumb skillStates to all agent entry points, isolate toggle keydown

- Add skillStates/defaultActiveOnShare loading to openai.js and
  responses.js controllers so shared-skill activation is respected
  across all agent entry points, not just initialize.js (Codex P1)
- Stop keydown propagation on SkillToggle so Enter/Space does not
  bubble to the parent row's navigation handler (Codex P2)

* fix: paginate catalog fetch and serialize toggle writes

- Paginate listSkillsByAccess (up to 10 pages of 100) until the active
  catalog quota is filled, so inactive shared skills in recent positions
  do not starve active owned skills past the first page (Codex P1)
- Extend listSkillsByAccess interface with cursor/has_more/after for
  catalog pagination
- Serialize skill-state writes via a ref queue: one in-flight request
  at a time, with the latest desired state sent when the previous one
  settles. Prevents last-response-wins races where an older request
  overwrites newer toggles (Codex P2)

* fix: share write queue across hook instances, block toggle on fetch error

- Move the write queue from a per-instance useRef to a module-scoped
  object so every mount of useSkillActiveState (SkillList, SkillDetail,
  etc.) serializes against the same in-flight slot. Prior per-instance
  queues allowed two components to race full-map POSTs (Codex P1)
- Extend the toggle guard beyond isLoading: also block when isError is
  true or data is undefined. Prevents a failed GET from seeding a
  toggle with an empty baseline that would wipe server-side overrides
  on the next successful POST (Codex P1)

* fix: stale closure, orphan cleanup, and cap-error UX

- Read toggle baseline from React Query cache via queryClient.getQueryData
  instead of the captured skillStates closure. The closure can be stale
  between onMutate's setQueryData and the next render, so rapid successive
  toggles would build on old state and drop earlier changes (Codex P1)
- Surface the MAX_SKILL_STATES_EXCEEDED error code with a specific toast
  key (com_ui_skill_states_limit) so users understand the 200-cap rather
  than seeing a generic error
- Prune orphaned entries (skillIds whose Skill doc no longer exists) on
  both GET and POST in SkillStatesController. Self-heals over time
  without needing cascade-delete hooks or a migration job. Uses one
  indexed Skill._id query per request

* test: pin skill active-state precedence with unit tests

Extract the active-state resolution logic from a closure inside
injectSkillCatalog into an exported resolveSkillActive helper, then
cover every branch of the precedence matrix:

- Fails closed when userId is absent (even with defaultActiveOnShare=true)
- Explicit override wins over ownership and config (both true and false)
- Owned skills default to active when no override is set
- Shared skills default to defaultActiveOnShare value
- Undefined skillStates behaves identically to an empty object
- defaultActiveOnShare defaults to false when omitted
- Owned skills ignore defaultActiveOnShare entirely

Closes Finding #2 from the pre-rebase comprehensive review. Mirrors
the existing scopeSkillIds test style; injectSkillCatalog now calls
resolveSkillActive instead of inlining the closure.

* refactor: limit skill active toggle to detail header, drop label

- Remove the per-row toggle from SkillListItem and the active-state
  plumbing (hook call, isSkillEnabled/onToggleEnabled/toggleAriaLabel
  props) from SkillList. The detail view is now the single place to
  change a skill's active state
- Drop dim/muted styling for inactive skills in the sidebar: without
  a control there, the visual indication has nowhere to land
- Resize SkillToggle to match neighbor buttons: outer h-9 container,
  h-6 w-11 track with size-5 knob, no label span. The 'Active' /
  'Inactive' text that accompanied the detail-view toggle is removed
- Remove the now-unused label prop and tabIndex prop (the tabIndex
  existed only for the list-row context) from SkillToggle. Drop the
  onKeyDown stopPropagation for the same reason
- Remove now-orphaned com_ui_skill_active / com_ui_skill_inactive
  translation keys

* style: shrink SkillToggle track to h-5 w-9 with size-4 knob

Container stays at h-9 to match neighbor button heights. The toggle
track itself drops from h-6 w-11 to h-5 w-9, with a size-4 knob
travelling 1.125rem on activation. Visually lighter inside the row.

* fix: remove redundant skillStates entries that match the resolved default

When a toggle lands on the ownership/config default, delete the key
from the map instead of persisting `{id: defaultValue}`. Without this,
a user toggling a skill off and back on would leave `{id: true}` for
an owned skill (whose default is already true), silently consuming a
slot against the 200-entry cap. Repeated round-trip toggles could
exhaust the quota with zero meaningful overrides (Codex P2).

Preserves the exceptions-list invariant that the runtime-resolution
design depends on.

* fix: prune before enforcing skill-state cap; reject non-ObjectId keys

Reorder the update controller so pruneOrphans runs before the 200-cap
check. Without this, a user near the cap with some orphaned entries
(skills deleted since their last GET) could send a payload that would
pass after pruning but gets rejected by the raw-size check first.

Add a sanity cap on raw payload size (2 * MAX_SKILL_STATES) so abusive
inputs do not reach the DB query, and enforce the real cap on the
pruned result instead.

Harden pruneOrphans: the earlier early-return path could pass
non-ObjectId keys through unchanged. Now only valid ObjectIds are
returned, and the Skill-model-unavailable fallback filters by format.

Also add isValidObjectIdString validation at the input boundary so
malformed (but otherwise non-Mongo-unsafe) keys never reach persistence
(Codex P2 x2).

* fix: enforce active filter at execute time, prune revoked shares, scope queue per user

P1: injectSkillCatalog now returns activeSkillIds (the filtered set
that appears in the catalog). initializeAgent uses that set as the
stored accessibleSkillIds on the initialized agent, so getSkillByName
at runtime cannot resolve a deactivated skill — even if the LLM
hallucinates a name or the user invokes by direct-invocation shorthand.
Previously the executor authorized against the full ACL set, bypassing
the active-state guarantee (Codex P1).

P2: pruneOrphans now checks user access via findAccessibleResources
in addition to skill existence. When a share is revoked, the user's
skillStates entry for that skill had no cleanup path and silently
consumed the 200-cap. Self-heals on both GET and POST. One extra ACL
query per settings read/write; scoped to a single user so no N-user
amplification (Codex P2).

P2: the write queue moves from a single module-scoped object to a Map
keyed by userId. Logout/login in the same tab can no longer flush the
previous user's pending snapshot under the new session's auth. Each
userId gets its own pending/inFlight slot; the in-flight request
retains its original auth via the cookie already attached when sent,
so the race window closes (Codex P2).

* refactor: extract skillStates helpers to packages/api; add tests; polish

Address the remaining valid findings from the comprehensive review:

- Extract toRecord, loadSkillStates, validateSkillStatesPayload, and
  pruneOrphanSkillStates into packages/api/src/skills/skillStates.ts
  as TypeScript. The controller in /api shrinks to a ~90-line thin
  wrapper that builds live dependency adapters for Mongoose + the
  permission service (Review #2 DRY, #3 workspace boundary)

- Replace the triplicated 12-line skillStates loading block in
  initialize.js, openai.js, and responses.js with a single call to
  loadSkillStates from @librechat/api. One helper, three sites

- Swap console.error for the project logger in the controller
  (Review #7)

- Remove the redundant INVALID_KEY_PATTERN regex: a valid ObjectId
  cannot contain . or $, so isValidObjectIdString already covers it
  (Review #11)

- Parameterize the 200-cap error toast with {{0}} interpolation
  driven by the error response's `limit` field, so future changes to
  MAX_SKILL_STATES update the UI message automatically (Review #12)

- Add 24 unit tests for the new skillStates helpers (toRecord,
  resolveDefaultActiveOnShare, loadSkillStates, validateSkillStates-
  Payload, pruneOrphanSkillStates) covering success paths, malformed
  input, cap boundaries, and parallel-query behavior (Review #4)

- Add 10 tests for injectSkillCatalog pagination covering empty
  accessible set, missing listSkillsByAccess, single-page filter,
  owned-vs-shared defaults, explicit-override precedence, multi-page
  collection, MAX_CATALOG_PAGES safety cap, early termination on
  has_more=false, additional_instructions injection, and fail-closed
  without userId (Review #5)

Total test count: 60 (was 26 on this surface).

* fix: rename skillStates ValidationError to avoid barrel-export collision

packages/api/src/types/error.ts already exports a ValidationError
(MongooseError extension). Re-exporting a different shape from
skills/skillStates.ts through the skills barrel caused TS2308 in CI
because the root index re-exports both. Rename to
SkillStatesValidationError to keep the exports disjoint.

* refactor: tighten tests and absorb caller guard into loadSkillStates

Address the followup review findings:

- Add optional `accessibleSkillIds` param to loadSkillStates so the
  helper short-circuits to defaults when no skills are accessible.
  All three controllers drop the residual 7-line conditional wrapper
  in favor of a single destructured call (Review #2)

- Remove the unreachable `typeof key !== 'string'` check from
  validateSkillStatesPayload: Object.entries always yields string
  keys per the JS spec (Review #3)

- Replace the two `as unknown as` agent casts in the injectSkillCatalog
  tests with a `makeAgent()` factory typed directly as the function's
  parameter shape (Review #4)

- Tighten the MAX_CATALOG_PAGES assertion from `toBeLessThanOrEqual(11)`
  to `toHaveBeenCalledTimes(10)` — the loop deterministically makes
  exactly 10 page fetches before hitting the cap (Review #1)

- Rewrite the parallel-execution test for pruneOrphanSkillStates using
  deferred promises instead of microtask-order assertions. The test
  now inspects `toHaveBeenCalledTimes(1)` on both mocks after a single
  Promise.resolve() yield, pinning Promise.all usage without relying
  on push-order into a shared array (Review #5)

- Evict stale writeQueue entries on user change via a module-scoped
  `lastSeenUserId` sentinel. When a different user's toggle is the
  first one after a logout/login, the previous user's queue entry is
  deleted. Keeps the Map bounded without adding hook-instance effect
  cleanup (Review #6)

* fix(test): mock loadSkillStates in openai and responses controller specs

The prior refactor replaced the inline 12-line skillStates loading
block with a call to loadSkillStates from @librechat/api. Both
controller spec files mock @librechat/api as a flat object, so any
new named import from that package is undefined in the test env.
Calling `await loadSkillStates(...)` threw before recordCollectedUsage
ran, surfacing as "undefined is not iterable" on the test's array
destructure of `mockRecordCollectedUsage.mock.calls[0]`.

Add the missing mock to both spec files alongside the existing
scopeSkillIds stub.

* fix: abandon stale skillStates write queues on user switch

Close the cross-session leak window where an in-flight flush loop
still holds a reference to a previous user's queue: it could fire its
next mutateAsync under the new session's auth cookies and persist
the stale snapshot to the new user's document (Codex P1).

Add an `abandoned` flag on `WriteQueue`. Three mechanisms cooperate:

- `getWriteQueue` marks every non-active queue abandoned when the
  user differs from the last-seen identity (pre-existing eviction
  site, now more aggressive).
- A `useEffect` on `userId` calls the same abandonment pass on every
  render with a new active identity, covering the window between
  logout/login and the new user's first toggle (when `getWriteQueue`
  would otherwise not fire).
- The flush loop checks `!queue.abandoned` in its while condition so
  the second and later iterations exit without firing another
  `mutateAsync` after the session changes.

The first iteration's in-flight request (already dispatched under the
original user's cookies) still runs to completion or failure on its
own — only the subsequent iterations, which are the dangerous ones,
are blocked.
danny-avila added a commit that referenced this pull request Apr 21, 2026
…le` / `allowed-tools` (#12745)

* 🧬 feat: Persist `disable-model-invocation` / `user-invocable` / `allowed-tools`

Adds first-class columns mirroring the three runtime-enforced frontmatter
fields, with a `deriveStructuredFrontmatterFields` helper that maps from
frontmatter at create/update time and re-syncs (via `$unset`) when fields
are removed. `listSkillsByAccess` projection includes them so the Phase 6
catalog filter and popover filter can both read off the summary row.

Marks `invocationMode` as @deprecated on `TSkill` and the
`InvocationMode` enum — the runtime now reads the persisted pair instead.

* 🛡️ feat: Enforce frontmatter at runtime (catalog, skill tool, manual resolver, tool union)

Wires the persisted columns into actual runtime behavior across all four
invocation paths:

- `injectSkillCatalog` excludes `disableModelInvocation: true` skills
  before catalog formatting — they cost zero context tokens and stay
  invisible to the model.
- `handleSkillToolCall` rejects with a clear error when the model names
  a skill marked `disable-model-invocation: true` (defends against a
  stale-cache or hallucinated invocation getting past the catalog
  filter).
- `resolveManualSkills` skips `userInvocable: false` skills with a warn
  log so an API-direct caller can't bypass the popover-side filter.
- `unionPrimeAllowedTools` collects skill-declared `allowed-tools` minus
  what's already on the agent; `initialize.ts` re-runs `loadTools` for
  the extras and merges resulting `toolDefinitions` into the agent's
  effective set for the turn. Tool-name resolution is tolerant —
  unknown names silently drop with a debug log so cross-ecosystem
  skills referencing yet-to-be-implemented tools (Claude Code's
  `edit_file`, etc.) import without breaking. The agent document is
  never modified; the union is turn-scoped.

Helper exports (`unionPrimeAllowedTools`) are structured so Phase 5's
always-apply primes flow through the same union (combined
`[...manualPrimes, ...alwaysApplyPrimes]`) once the resolver lands.

Skill handler wire format gains the three fields so clients can render
them on detail / list views.

* 🎛️ feat: `$` popover reads `userInvocable` instead of UI-only `invocationMode`

Replaces the phase-1 UI-only `invocationMode` check with the persisted
`userInvocable` field (mirrors the `user-invocable` frontmatter). Skills
authored with `user-invocable: false` no longer surface in the popover;
the backend resolver enforces the same rule for defense-in-depth.

Default-visible behavior is preserved: skills without an explicit
`userInvocable` value (older rows, freshly imported skills that don't
declare the field) stay visible — only an explicit `false` hides them.

Test fixture updated to reflect the new field.

* 🔧 fix: Address Phase 6 review findings

Codex P2 + reviewer #1: Single `loadTools` call with the union of
`agent.tools + allowed-tools`. The earlier two-call approach dropped
`userMCPAuthMap` / `toolContextMap` / `actionsEnabled` from the
skill-added pass — an MCP tool gained via `allowed-tools` would be
visible to the model but fail at execution without per-user auth
context. Resolution of `manualSkillPrimes` is hoisted before
`loadTools` so the union can be computed up-front; the dropped-tools
debug log now compares loaded vs. requested across the single call.

Codex P3 + reviewer #2: `injectSkillCatalog.activeSkillIds` now
includes `disable-model-invocation: true` skills. The runtime ACL
check in `handleSkillToolCall` previously couldn't reach the explicit
"cannot be invoked by the model" rejection because the broader access
set excluded those skills. Catalog text and tool registration still
gate on the visible subset (zero-context-token guarantee preserved);
only the per-user `isActive` filter is a hard exclusion now.

Reviewer #1 (try/catch around loadTools, MAJOR): A single bad
`allowed-tools` entry from a shared skill could crash the entire turn.
Now wrapped — on failure with extras, retry with just `agent.tools`
and continue (the dropped-tools debug log surfaces what vanished). If
the retry-without-extras still throws, propagate; the agent's own
tools are the load-bearing surface.

Reviewer #3 (integration tests, MAJOR): Added six tests in
`initialize.test.ts` covering the full `allowed-tools` loading path:
union pass-through, no-extras short-circuit, agent-baseline dedup,
loadTools throw + retry, propagated throw without extras, and the
empty-tools edge case.

Smaller cleanups bundled in:
- Reviewer #4: Moved `logger` import to the package-imports section
  (was wedged among local imports).
- Reviewer #5: Removed unused index on `disableModelInvocation`
  (filtering happens application-side in `injectSkillCatalog`; index
  cost write overhead for zero query benefit).
- Reviewer #6: Swapped order of `userInvocable` and body checks in
  `resolveManualSkills` so the more authoritative author-decision
  reason surfaces first when both apply.
- Reviewer #8: Documented the `allowedTools` enforcement gap on the
  schema + type — model-invoked skills (mid-turn `skill` tool calls)
  do NOT trigger tool union, since adding tools after the graph
  starts would require a rebuild. Manual / always-apply (Phase 5)
  primes are the supported paths.
- Reviewer #9: Renamed `dmi` / `ui` / `at` locals to
  `disableModelInvocationRaw` / `userInvocableRaw` / `allowedToolsRaw`
  in `deriveStructuredFrontmatterFields`.

Reviewer #7 (DRY shared `getSkillByName` return type) deferred —
field sets diverge meaningfully across the three call sites (handler
needs `body + fileCount`; resolver needs `author + allowedTools +
userInvocable`; the InitializeAgentDbMethods contract needs the
superset). A `Pick<>`-based consolidation is a follow-up cleanup.

* 🔧 fix: Address codex iter 2 — catalog quota + duplicate-name dedup

P1: `injectSkillCatalog` cap now counts only model-visible skills, not
the merged active set. The previous behavior let a tenant with many
`disable-model-invocation: true` rows near the top of the cursor
exhaust the 100-slot quota before any invocable skill got scanned —
the catalog could end up empty even though invocable skills existed
further down the paginated results. `MAX_CATALOG_PAGES` stays the
ceiling on scan budget; only `visibleCount` drives the early-exit on
quota fill.

P2: When an invocable and a `disable-model-invocation: true` skill
share a name, drop the disabled doc(s) from `activeSkillIds`. Without
this dedup, `getSkillByName` (which sorts by `updatedAt` desc) could
pick the disabled doc and every model call to the cataloged name
would fail with "cannot be invoked by the model" instead of executing
the visible skill. When ONLY a disabled doc exists for a name, it
stays in `activeSkillIds` so the explicit-rejection error path still
fires for hallucinated invocations.

Tests: 3 new cases in `injectSkillCatalog` covering (a) cap counted
on visible skills only, (b) same-name collision drops disabled doc,
(c) sole-disabled-name case keeps the disabled doc.

* 🔒 fix: Apply `disable-model-invocation` gate to read_file too (codex iter 3 P1)

`activeSkillIds` is shared between the `skill` and `read_file` handlers.
The skill-tool gate was applied last iteration, but `handleReadFileCall`
authorized purely on `getSkillByName(..., accessibleIds)` — so a model
that learned a hidden skill's name (stale catalog or hallucination)
could still read its `SKILL.md` body or bundled files via `read_file`,
defeating the contract. Same explicit rejection now fires from both
handlers; no change needed to the ACL set itself (disabled docs stay
in `activeSkillIds` so the explicit error path keeps firing).

Two new tests in `handlers.spec.ts` cover the read_file gate and
regression-protect the happy path.

* 🔧 fix: Address codex iter 4 — manual-prime exception + legacy frontmatter backfill

P1: Scope the `read_file` `disableModelInvocation` gate to AUTONOMOUS
model probes only. A user-invoked `$` skill that is also marked
`disable-model-invocation: true` had its bundled `references/*` /
`scripts/*` files unreadable, leaving the manually-primed skill body
referencing files the model couldn't load. Now the handler bypasses
the gate when the skill name appears in `manualSkillNames` (the
per-turn allowlist threaded from `manualSkillPrimes` →
`agentToolContexts` → `enrichWithSkillConfigurable` →
`mergedConfigurable`). Defense-in-depth: the bypass is scoped to the
specific names in the allowlist; a different disabled skill name is
still rejected.

P2: Read-time fallback for legacy skills authored before Phase 6
landed the structured columns. `user-invocable: false` /
`disable-model-invocation: true` set in `frontmatter` (the validator
already accepted those keys) but with no derived column would
incorrectly evaluate as "user-invocable / model-allowed" until a save
backfilled the columns. New `backfillDerivedFromFrontmatter` helper
fills undefined columns from frontmatter at read time in both
`getSkillByName` and `listSkillsByAccess` — column wins when both are
set, frontmatter fills the gap when only it's set. No DB writes; the
next `updateSkill` naturally persists. `listSkillsByAccess` projection
expanded to include `frontmatter` (bounded by validator, payload
impact small) so summaries can also be backfilled.

Sticky-primed disabled skills (ones invoked in prior turns of the
same conversation) are not yet in the manual-prime allowlist — same-
turn manual invocation is the load-bearing path codex flagged; the
sticky-turn case is a known limitation tracked for a follow-up.

Tests: 2 new in handlers.spec.ts (manual-prime allows + name-scoped
block holds), 3 new in skill.spec.ts (legacy backfill via
getSkillByName + listSkillsByAccess + column-wins precedence).

* 🔧 fix: Address codex iter 5 — propagate manualSkillNames + keep read_file

P1: `enrichWithSkillConfigurable` is also called from `openai.js` and
`responses.js` (the OpenAI Responses + completions endpoints). Both
were ignoring the new `manualSkillNames` parameter, which meant the
manual-prime exception in the `read_file` gate (iter 4) only worked
on the agents endpoint. Now all three call sites pass
`primaryConfig.manualSkillPrimes?.map(p => p.name)` so manual `$`
invocations of disabled skills work consistently across endpoints.

P2: When every accessible skill is `disable-model-invocation: true`,
the catalog text and `skill` tool are correctly omitted (no model-
reachable targets) — but `read_file` and `bash_tool` MUST still be
registered. A user manually invoking such a skill gets its SKILL.md
body primed into context; if the body references `references/foo.md`
or `scripts/run.sh`, those reads need a registered tool. Restructured
`injectSkillCatalog` so `skill` registration is gated on
`catalogVisibleSkills.length > 0` while `read_file` (always) and
`bash_tool` (when codeEnvAvailable) register whenever any active
skill is in scope.

Tests: existing all-disabled test rewritten to assert read_file IS
registered + skill is NOT; new test confirms bash_tool joins it
when codeEnvAvailable.

* 🔧 fix: Address codex iter 6 — name-collision consistency via preferInvocable

P2a (resolveManualSkills): a name collision between an older
user-invocable doc and a newer non-user-invocable doc made manual `$`
invocation silently no-op. The popover surfaced the older invocable
doc; resolver looked it up by name; `getSkillByName` returned the
newer non-invocable doc; resolver skipped on `userInvocable: false`.

P2b (handler / runtime ACL): with same-name duplicates (e.g. older
invocable + newer disabled), the manual prime resolved to one doc
while later `read_file` / `skill` execution resolved a different doc
through `activeSkillIds`. Model could follow one SKILL.md body while
reading files from a different skill.

Both root-cause: `getSkillByName` always returned the newest match
and let the caller filter, but with collisions the newest can be
something the caller didn't want.

Fix: extend `getSkillByName` with `options.preferInvocable`. When
true, prefer the newest doc satisfying BOTH `userInvocable !== false`
AND `disableModelInvocation !== true` (with frontmatter backfill);
fall back to the newest match otherwise. Fast path preserved when
caller doesn't opt in.

Callers passing `preferInvocable: true`:
- `resolveManualSkills` — picks the popover-visible invocable doc
  even when a newer disabled / non-user-invocable duplicate exists.
- `handleSkillToolCall` — keeps execution aligned with the catalog;
  falls back to the disabled doc only when no invocable variant
  exists (so the explicit "cannot be invoked by the model" gate
  still fires for the hallucinated-disabled-name case).
- `handleReadFileCall` — same alignment, plus the manual-prime
  exception added in iter 4 still applies.

Tests:
- 2 new in skill.spec.ts (preferInvocable picks invocable when
  collision exists; falls back to newest when no clean-invocable
  exists).
- 1 new in skills.test.ts (resolver passes preferInvocable through).
- 2 new in handlers.spec.ts (skill tool + read_file pass it).
- Existing initialize.test.ts assertion updated for the new option.

* 🔧 fix: Address codex iter 7 — split preferInvocable into per-axis flags

The previous unified `preferInvocable` filter required BOTH
`userInvocable !== false` AND `disableModelInvocation !== true`. That
was wrong for the model paths: `userInvocable: false` skills are
model-only and remain valid `skill` / `read_file` invocation targets.
A duplicate-name scenario where the newer cataloged doc was model-
only would let the older user-invocable variant shadow it on every
model call.

Split the option into two independent axes:
- `preferUserInvocable` — for manual paths (`$` popover). Skips docs
  with `userInvocable: false`. Disable-model-invocation status is
  irrelevant; iter 4 explicitly supports manual prime of disabled
  skills.
- `preferModelInvocable` — for model paths (`skill` / `read_file`
  handlers). Skips docs with `disableModelInvocation: true`. User-
  invocable status is irrelevant; model-only skills are valid here.

Both flags fall back to the newest match when no preferred doc
exists, so the explicit-rejection error paths still fire correctly
in the sole-disabled-name case.

Callers updated:
- `resolveManualSkills` → `preferUserInvocable: true`
- `handleSkillToolCall` / `handleReadFileCall` → `preferModelInvocable: true`

Tests:
- New spec test for preferModelInvocable not filtering on userInvocable.
- Existing preferInvocable test renamed/split to cover the new axes.
- New test asserts preferUserInvocable still returns disabled docs
  (preserves iter 4 manual-disabled support).
- Caller tests assert each path passes the right single flag and
  does NOT pass the wrong one.

* 🔧 fix: TypeScript type-check failure in handlers.spec.ts (CI green)

`jest.fn(async () => ...)` without explicit args infers an empty tuple
for the call signature, so `mock.calls[0][2]` flagged as "Tuple type
'[]' has no element at index '2'." Cast to `unknown[]` then narrow to
the expected option shape. Behavior unchanged.

Caught by the `Type check @librechat/api` CI step
(.github/workflows/backend-review.yml).

* 🔧 fix: Address codex iter 8 — undefined-result fallback + read_file alignment

P1 (loadTools returning undefined): Production loaders
(`createToolLoader` in `initialize.js` / `openai.js` /
`responses.js`) wrap `loadAgentTools` in try/catch and return
`undefined` on failure rather than throwing. Without explicit
handling, my iter-1 try/catch only fired for thrown errors — a
silent-failure on a skill-added tool would fall through to the
empty fallback and silently DROP the agent's baseline tools for
the turn (much worse than just losing the extras). Added an
`undefined`-result branch that retries with just `agent.tools`,
mirroring the throw branch. Test pins both behaviors.

P2 (read_file alignment with manual prime): When a skill is in
this turn's `manualSkillNames`, the `read_file` handler now uses
`preferUserInvocable` instead of `preferModelInvocable`. Same
name-collision rule as `resolveManualSkills`, so the doc whose
files get read is the same doc whose body got primed. For
autonomous probes (skill not in `manualSkillNames`), the handler
keeps `preferModelInvocable` to align with the catalog the model
saw. Two new tests cover both branches and regression-protect that
the wrong flag isn't passed.

* 🔧 fix: Address codex iter 9 — pin read_file lookup to primed skill _id

P1 (manually-primed disabled IDs were dropped from activeSkillIds):
The `executableSkills` dedup in `injectSkillCatalog` correctly drops
`disable-model-invocation: true` duplicates when an invocable doc
shares the name — but `resolveManualSkills` legitimately primes
disabled docs (iter 4 supports manual `$` invocation of disabled
skills). When the resolver primed a disabled doc, the read_file
handler couldn't find it in the (deduped) `activeSkillIds` and
either resolved a different same-name skill or returned not-found.

Fix: `ResolvedManualSkill` now carries `_id`; the legacy `initialize.js`
/ `openai.js` / `responses.js` controllers build a
`manualSkillPrimedIdsByName` map and `enrichWithSkillConfigurable`
passes it into `mergedConfigurable`. `handleReadFileCall` now pins
its lookup's `accessibleIds` to `[primedId]` whenever the requested
skill is in the map. The constrained set guarantees the lookup
returns the EXACT doc the resolver primed — body/files come from the
same source even when same-name duplicates exist or the dedup
removed the prime's id from `activeSkillIds`.

Autonomous read_file probes (skill not in the manual-primed map)
keep the full ACL set + `preferModelInvocable` so they align with
the catalog the model saw and the disabled-only case still fires
the explicit-rejection gate.

Test fixture changes flow from `_id` becoming required on
`ResolvedManualSkill`. `buildSkillPrimeContentParts` /
`injectManualSkillPrimes` widen their param types to `Pick<...>`
because they only read `name` / `body` and shouldn't force test
literals to invent placeholder ids.

* 🧹 fix: Address independent reviewer findings (DRY + types + tests + docs)

Sanity-pass review surfaced 7 findings; addressed 6 (the 7th — DRY
on inline `getSkillByName` return types — is acknowledged tech debt
deferred to a follow-up).

#1 [MAJOR, DRY]: The 4-line `manualSkillPrimedIdsByName` map
construction was duplicated across 4 CJS call sites (openai.js,
responses.js x2, initialize.js). Extracted `buildManualSkillPrimedIdsByName`
helper in `skillDeps.js`; all four sites now call the helper. If
`ResolvedManualSkill` ever renames `_id` or gains identifying fields,
only the helper changes.

#2 [MINOR, type safety]: `handleReadFileCall` was casting a hex string
to `Types.ObjectId[]` via `as unknown as`, relying on mongoose's
auto-cast in `$in` queries. Replaced with `new Types.ObjectId(...)`
so any future consumer comparing with `.equals()` / `===` gets the
correct value type. Imported `Types` as a value (was type-only).

#5 [MINOR, test gap]: Added a test for the worst-case silent-failure
path — both the union and base-only `loadTools` calls return undefined.
The agent gets no tools but the turn doesn't crash hard; pinning
that contract.

#4 [MINOR, performance]: Added a TODO on the `listSkillsByAccess`
projection noting the `frontmatter` field can be dropped once a
write migration backfills all pre-Phase-6 skills' columns. ~2KB/skill
× 100/page is wasted bandwidth post-backfill.

#6 [NIT, docs]: `backfillDerivedFromFrontmatter` JSDoc said "Pure"
right before "mutates its undefined fields in place". Replaced with
"Side-effect-free w.r.t. the DB (no writes), but mutates its argument
in place" which describes both halves accurately.

#7 [NIT, test determinism]: Replaced `await new Promise(r => setTimeout(r, 5))`
in two same-name collision tests with explicit `updateOne` setting
`updatedAt: new Date(Date.now() - 1000)` on the older doc. Removes
the wall-clock race on fast CI runners. The pagination test (line
480) still uses setTimeout — that test is pre-existing and order
is incidental, not load-bearing.

Existing test fixtures updated to use valid 24-char hex ObjectIds
(required by the iter-9 test that constructs a real `ObjectId`).

#3 [MINOR, deferred]: Inline `getSkillByName` return type duplicated
across `handlers.ts`, `initialize.ts`, `skills.ts`. Reviewer
acknowledged this as deferred; field sets diverge across call sites
(handler needs `fileCount`, resolver needs `author`/`allowedTools`).
A `Pick<>`-based consolidation is a clean follow-up.
danny-avila added a commit that referenced this pull request Apr 21, 2026
…ps, no-op allocation

Addresses the comprehensive review of PR #12767:

- **Finding #1** (MINOR, observability): `initializeAgent` now emits a
  debug log when an agent lists `execute_code` in its tools but the
  runtime gate is off (`params.codeEnvAvailable` !== true). The
  event-driven `loadToolDefinitionsWrapper` path doesn't log
  capability-disabled warnings, so without this the tool silently
  vanishes from the LLM's definitions with zero trace. Operators
  debugging "why isn't code interpreter working?" now get a signal at
  the initialize layer.

- **Finding #5** (NIT, allocation): `registerCodeExecutionTools` now
  returns the input `toolDefinitions` array by reference on the no-op
  path (both tools already registered by a prior caller in the same
  run) instead of allocating a fresh spread array every time. The
  common dual-call scenario — `initializeAgent` then
  `injectSkillCatalog` — saves one O(n) copy per request.

- **Finding #4** (NIT, DRY): Collapsed the duplicated 6-line JSDoc
  comment in `openai.js`, `responses.js`, and `addedConvo.js` into
  either a one-line `@see DiscoverConnectedAgentsParams.codeEnvAvailable`
  pointer (the two JS call sites) or a compact 3-line block referring
  back to the canonical source (addedConvo's @PARAM).

- **Finding #2** (MINOR, test gap): Added
  `api/server/services/Endpoints/agents/addedConvo.spec.js` with three
  cases covering `codeEnvAvailable=true`, `codeEnvAvailable=false`,
  and omitted (undefined) passthrough. A future refactor that drops
  the param from destructuring now surfaces here instead of silently
  regressing multi-convo parallel agents with `execute_code`.

- **Finding #3** (MINOR, test gap): Added
  `api/server/controllers/agents/__tests__/client.memory.spec.js`
  pinning the capability-flag derivation that `AgentClient::useMemory`
  uses — six cases covering present/absent/null/undefined config shapes
  plus an enum-literal pin (`'execute_code'` / `'agents'`). Catches
  enum renames or config-path shifts that would otherwise silently
  strip `bash_tool` + `read_file` from memory agents.

Finding #7 (jest.mock scoping, confidence 40) left as-is: the
reviewer's own risk assessment noted `buildToolSet` doesn't touch
the mocked exports, and restructuring a file-level `jest.mock` to
`jest.doMock` + dynamic `import()` introduces more complexity than
the speculative risk justifies. The existing mock is scoped to the
test file and contains the same stubs the adjacent
`skills.test.ts` already uses.

Finding #6 (PR description commit count) addressed out-of-band via
PR description update.

All existing tests pass, typecheck clean, lint clean across touched
files. New tests: 9 cases across 2 new spec files.
OnyxBishop pushed a commit to OnyxBishop/LibreChat that referenced this pull request Apr 22, 2026
…ain Collision (LibreChat-AI#12594)

* 🐛 fix: resolve Action tools by exact tool name to prevent multi-action collision

When two OpenAPI Actions on the same Agent share a hostname, the second
action's entry overwrote the first in the encoded-domain Map and one
action's tools silently disappeared from the LLM payload. The buggy
resolution loop also used substring matching, which caused similar
shadowing for any encoded-domain prefix overlap.

This change builds a Map keyed on the full tool name
(`<operationId>_action_<encoded-domain>`) directly, mirroring the exact
lookup pattern that getActionToolDefinitions already uses. Each function
in an action's spec gets its own slot, so two actions sharing a hostname
no longer collide. Both the new and the legacy domain encodings are
registered for each function so agents whose stored tool names predate
the current encoding still resolve.

Applied at all three call sites that had the buggy pattern:

  - processRequiredActions (assistants/threads path)
  - loadAgentTools (agent build path)
  - loadActionToolsForExecution (agent execution path)

Adds three regression tests covering both ordering directions and the
execution path. Tests fail without the fix and pass with it.

* 🐛 fix: Normalize action tool name at lookup + cover assistants path

Follow-up to the multi-action domain collision fix. Addresses PR LibreChat-AI#12594
review feedback:

**Must-fix LibreChat-AI#1 — short-hostname lookup mismatch.** The toolToAction map
is keyed on the `_`-collapsed domain, but `agent.tools` and
`currentAction.tool` persist the raw `domainParser(..., true)` output,
which for hostnames ≤ ENCODED_DOMAIN_LENGTH is a `---`-separated string
(e.g. `medium---com`). Exact-match `Map.get()` missed those keys and
silently dropped the tool. Fix: normalize every incoming tool name
through a new `normalizeActionToolName` helper before the lookup in
`loadAgentTools`, `processRequiredActions`, and
`loadActionToolsForExecution`.

**Must-fix LibreChat-AI#2 — assistants path coverage.** `processRequiredActions`
received the same structural rewrite but had zero tests. Added a
regression test under `multi-action domain collision regression` that
drives two shared-hostname actions through the assistants path and
asserts each tool reaches its own request builder.

**Must-fix LibreChat-AI#3 — legacy encoding branch coverage.** The
`if (legacyNormalized !== normalizedDomain)` registration was never
exercised by any test. Added a test where `agent.tools` stores the
legacy-format name and asserts it still resolves.

**Should-fix LibreChat-AI#4 — DRY the registration loop.** Extracted
`registerActionTools({ toolToAction, functionSignatures,
normalizedDomain, legacyNormalized, makeEntry })`. All three call sites
now share the same key-building logic; the key template lives in one
place.

**Should-fix LibreChat-AI#5 — remove stale optional chaining.** In
`loadActionToolsForExecution`, `functionSignature?.description ?? ''`
became `functionSignature.description` — `sig` is always defined by the
iterator, matching the style of `loadAgentTools`.

**Should-fix LibreChat-AI#6 — drop unreachable `!requestBuilder` guard.** Entries
in `processRequiredActions` are now pre-built with
`requestBuilder: requestBuilders[sig.name]`, which `openapiToFunction`
always produces alongside the signature, so the guard is dead.

**Should-fix LibreChat-AI#7 — unwrap `actionSetsData`.** It now holds a bare
`Map` instead of `{ toolToAction }`; the sentinel `!actionSetsData`
check still works because `new Map()` is truthy.

Also added a short-hostname regression test
(`loadAgentTools resolves raw ---separated tool names`) that reproduces
Must-fix LibreChat-AI#1: it fails against the previous commit (0 create calls) and
passes with the normalization in place.

41 tests, all passing. The 3 new regression tests are under
`multi-action domain collision regression` and cover the assistants
path, the legacy encoding branch, and the short-hostname lookup path.

* 🐛 fix: Tighten registerActionTools key handling and assistants test

Follow-up to d643444 addressing the second review pass on PR LibreChat-AI#12594.

**ESLint** — Two prettier errors in the spec file (multi-line arrow
function bodies that should fit on one line). Auto-fixed.

**[MINOR] operationId containing `---` → key mismatch.** The lookup
path collapsed every `actionDomainSeparator` sequence in the full tool
name, but the registration path passed `sig.name` through unchanged.
A `---` that survived into an operationId would shift the underscore
boundary at lookup and miss its own key. Fix in `registerActionTools`:
normalize `sig.name` with the same helper so registration and lookup
always agree on the canonical form. `sanitizeOperationId` strips the
characters that produce `---` in practice, so this is theoretical
hardening, not a fix for a known reproducer.

**[MINOR] Same-operationId + same-hostname silent overwrite.** Two
actions sharing both an operationId and a hostname still produced a
silent `Map.set()` overwrite (the new key is identical, so neither the
operationId nor the domain disambiguates). Added a `setKey` helper
inside `registerActionTools` that logs a `[Actions] operationId
collision: ...` warning whenever a key is already present, naming the
overwriting action_id. The silent-overwrite mode from the original bug
cannot reappear under a different disguise without surfacing in the
logs.

**[NIT] processRequiredActions test simulated a runtime crash.**
`mockCreateActionTool` returned a tool with `_call: jest.fn()`, which
resolves to `undefined`. `processRequiredActions` chains
`.then(handleToolOutput).catch(handleToolError)` directly onto that
return, so `undefined.then(...)` threw synchronously and the outer
try/catch funneled the error into `handleToolError`. Creation count
assertions still passed because `createActionTool` runs before the
crash, but the test was silently exercising the failure path. Updated
the global mock to `_call: jest.fn().mockResolvedValue('{"status":"ok"}')`
so the success path runs end-to-end. The assistants regression test
now executes in ~5ms instead of ~90ms, which corroborates that it's
no longer hitting the synchronous throw.

**[NIT] Duplicated rationale comments.** All three call sites carried
multi-line comment blocks restating why we key on the full tool name.
That rationale now lives canonically in `registerActionTools`'s JSDoc;
the inline blocks collapsed to `// See registerActionTools for the
key-shape rationale.` Net -22 lines of comments.

41/41 tests still pass; lint is clean.

* 🐛 fix: Scope tool-name normalization to the encoded-domain suffix

Follow-up to f22228e addressing the Codex P1 on PR LibreChat-AI#12594.

**Regression.** The previous commit normalized the entire tool name
(`normalizeActionToolName(sig.name)` at registration, full-name
`.replace()` at lookup) to handle operationIds that theoretically
contained `---`. But `openapiToFunction` uses user-supplied
operationIds verbatim and the fallback `sanitizeOperationId` only
strips characters outside `[a-zA-Z0-9_-]`, so specs can legitimately
produce operationIds like `get_foo---bar` and `get_foo_bar` side by
side. Collapsing `---` to `_` across the entire key merged those two
into a single map slot — one silently overwrote the other, and both
tool requests routed to the surviving entry's request builder.

**Fix.** Limit normalization to the encoded-domain portion of the
full tool name, i.e. the substring after the last `actionDelimiter`.
The operationId half is left verbatim, so hyphens-vs-underscores
remain disambiguating. The short-hostname bug (Must-fix LibreChat-AI#1 from the
original review) is still covered because the `---` → `_` collapse
still happens on the domain suffix where it matters:

```js
const normalizeActionToolName = (toolName) => {
  const delimiterIndex = toolName.lastIndexOf(actionDelimiter);
  if (delimiterIndex === -1) return toolName;
  const prefixEnd = delimiterIndex + actionDelimiter.length;
  const encodedDomain = toolName.slice(prefixEnd);
  return toolName.slice(0, prefixEnd) + encodedDomain.replace(domainSeparatorRegex, '_');
};
```

`registerActionTools` reverts to `sig.name` verbatim — no more
`normalizeActionToolName(sig.name)` ahead of the key build.

**Regression test.** Added
`loadAgentTools distinguishes operationIds that differ only by
---` vs `_`` under `multi-action domain collision regression`. It
loads two actions sharing a hostname whose operationIds are
`get_foo---bar` and `get_foo_bar` respectively, each pointing at a
different path (`/foo-bar`, `/foo_bar`), and asserts that each tool
resolves to its own request builder. Verified the test fails against
f22228e (`hyphenTool` resolves to `/foo_bar` — the sibling's builder)
and passes with this commit.

42/42 tests pass; lint clean.
danny-avila added a commit that referenced this pull request Apr 22, 2026
…ToolAuth contract, doc appConfig

Addresses the comprehensive review of Phase 8. Findings mapped:

**#1 (MINOR): `verifyToolAuth` unconditional auth for execute_code**
- Added doc comment explicitly stating the deployment contract
  (admin capability → reachable sandbox; no per-check health probe
  to keep UI-gate queries O(1)).
- New `api/server/controllers/__tests__/tools.verifyToolAuth.spec.js`
  with 4 regression tests pinning the contract:
  1. `authenticated: true` + `SYSTEM_DEFINED` for execute_code.
  2. 404 for unknown tool IDs.
  3. `loadAuthValues` is never consulted (catches a future revert
     that would resurface the per-user key-entry dialog).
  4. Response `message` is never `USER_PROVIDED`.

**#2 (MINOR): `openai/service.ts` undocumented `appConfig` dependency**
- Expanded the `ChatCompletionDependencies.appConfig` JSDoc to spell
  out that omitting it silently disables code execution for agents
  with `execute_code` in their tools. External consumers of
  `createAgentChatCompletion` now have the contract documented at
  the type boundary.

**#5 (NIT): `registerCodeExecutionTools` re-allocates tool defs**
- Hoisted `READ_FILE_DEF` and `BASH_TOOL_DEF` to module-level
  `Object.freeze`d constants. The shapes derive entirely from
  static `@librechat/agents` exports, so a single frozen object per
  tool is safe to share across every agent init. Eliminates the
  ~4-property allocations on every call (including the common
  second-call no-op path).

**#6 (NIT): Verbose history-priming comment in initialize.js**
- Trimmed the 16-line `handlePrimeInvokedSkills` block to a 5-line
  summary with `@see InitializedAgent.codeEnvAvailable` pointer.
  The canonical narrowing explanation lives on the type; the
  controller comment is just the ACL-vs-capability rationale.

**Skipped:**

- #3 (memory spec tests a mirror function): reviewer self-dismissed
  as a design tradeoff; the enum-literal pin already catches the
  highest-risk drift vector.
- #4 (cross-repo contract for `createCodeExecutionTool`): user will
  explicitly install the latest `@librechat/agents` dev version
  once the companion PR publishes, so the version pin will be
  authoritative.
- #7 (migration/deprecation note for self-hosters): out of scope
  per user direction — release notes handle this.

Tests verified: 679 packages/api + 840 backend api tests pass.
Typecheck + lint clean.
danny-avila added a commit that referenced this pull request Apr 22, 2026
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.
danny-avila added a commit that referenced this pull request Apr 22, 2026
* 🧹 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants