mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-08-04 06:52:47 +00:00
🎚️ feat: Per-User Skill Active/Inactive Toggle with Ownership-Aware Defaults (#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.
This commit is contained in:
Danny Avila
parent
3e064c2f2b
commit
9225a279eb
29 changed files with 1331 additions and 25 deletions
93
api/server/controllers/SkillStatesController.js
Normal file
93
api/server/controllers/SkillStatesController.js
Normal file
|
|
@ -0,0 +1,93 @@
|
|||
const mongoose = require('mongoose');
|
||||
const { logger } = require('@librechat/data-schemas');
|
||||
const {
|
||||
MAX_SKILL_STATES,
|
||||
toSkillStatesRecord,
|
||||
validateSkillStatesPayload,
|
||||
pruneOrphanSkillStates,
|
||||
} = require('@librechat/api');
|
||||
const { ResourceType, PermissionBits } = require('librechat-data-provider');
|
||||
const { findAccessibleResources } = require('~/server/services/PermissionService');
|
||||
const { updateUser, getUserById } = require('~/models');
|
||||
|
||||
/** Builds the injected deps for `pruneOrphanSkillStates` from live models. */
|
||||
function buildPruneDeps(user) {
|
||||
return {
|
||||
findExistingSkillIds: async (validIds) => {
|
||||
const Skill = mongoose.models.Skill;
|
||||
if (!Skill) {
|
||||
return validIds;
|
||||
}
|
||||
const existing = await Skill.find({ _id: { $in: validIds } })
|
||||
.select('_id')
|
||||
.lean();
|
||||
return existing.map((doc) => doc._id.toString());
|
||||
},
|
||||
findAccessibleSkillIds: () =>
|
||||
findAccessibleResources({
|
||||
userId: user.id,
|
||||
role: user.role,
|
||||
resourceType: ResourceType.SKILL,
|
||||
requiredPermissions: PermissionBits.VIEW,
|
||||
}),
|
||||
};
|
||||
}
|
||||
|
||||
const getSkillStatesController = async (req, res) => {
|
||||
try {
|
||||
const userId = req.user.id;
|
||||
const user = await getUserById(userId, 'skillStates');
|
||||
|
||||
if (!user) {
|
||||
return res.status(404).json({ message: 'User not found' });
|
||||
}
|
||||
|
||||
const states = toSkillStatesRecord(user.skillStates);
|
||||
const pruned = await pruneOrphanSkillStates(states, buildPruneDeps(req.user));
|
||||
return res.status(200).json(pruned);
|
||||
} catch (error) {
|
||||
logger.error('[SkillStatesController] Error fetching skill states:', error);
|
||||
return res.status(500).json({ message: 'Internal server error' });
|
||||
}
|
||||
};
|
||||
|
||||
const updateSkillStatesController = async (req, res) => {
|
||||
try {
|
||||
const { skillStates } = req.body;
|
||||
|
||||
const validationError = validateSkillStatesPayload(skillStates);
|
||||
if (validationError) {
|
||||
const { message, code, limit } = validationError;
|
||||
const payload = { message };
|
||||
if (code) payload.code = code;
|
||||
if (limit != null) payload.limit = limit;
|
||||
return res.status(400).json(payload);
|
||||
}
|
||||
|
||||
const pruned = await pruneOrphanSkillStates(skillStates, buildPruneDeps(req.user));
|
||||
|
||||
if (Object.keys(pruned).length > MAX_SKILL_STATES) {
|
||||
return res.status(400).json({
|
||||
code: 'MAX_SKILL_STATES_EXCEEDED',
|
||||
message: `Maximum ${MAX_SKILL_STATES} skill state overrides allowed`,
|
||||
limit: MAX_SKILL_STATES,
|
||||
});
|
||||
}
|
||||
|
||||
const user = await updateUser(req.user.id, { skillStates: pruned });
|
||||
|
||||
if (!user) {
|
||||
return res.status(404).json({ message: 'User not found' });
|
||||
}
|
||||
|
||||
return res.status(200).json(toSkillStatesRecord(user.skillStates));
|
||||
} catch (error) {
|
||||
logger.error('[SkillStatesController] Error updating skill states:', error);
|
||||
return res.status(500).json({ message: 'Internal server error' });
|
||||
}
|
||||
};
|
||||
|
||||
module.exports = {
|
||||
getSkillStatesController,
|
||||
updateSkillStatesController,
|
||||
};
|
||||
|
|
@ -41,6 +41,7 @@ jest.mock('@librechat/api', () => ({
|
|||
createChunk: jest.fn().mockReturnValue({}),
|
||||
buildToolSet: jest.fn().mockReturnValue(new Set()),
|
||||
scopeSkillIds: jest.fn().mockImplementation((ids) => ids),
|
||||
loadSkillStates: jest.fn().mockResolvedValue({ skillStates: {}, defaultActiveOnShare: false }),
|
||||
sendFinalChunk: jest.fn(),
|
||||
createSafeUser: jest.fn().mockReturnValue({ id: 'user-123' }),
|
||||
validateRequest: jest
|
||||
|
|
|
|||
|
|
@ -42,6 +42,7 @@ jest.mock('@librechat/api', () => ({
|
|||
}),
|
||||
buildToolSet: jest.fn().mockReturnValue(new Set()),
|
||||
scopeSkillIds: jest.fn().mockImplementation((ids) => ids),
|
||||
loadSkillStates: jest.fn().mockResolvedValue({ skillStates: {}, defaultActiveOnShare: false }),
|
||||
createSafeUser: jest.fn().mockReturnValue({ id: 'user-123' }),
|
||||
initializeAgent: jest.fn().mockResolvedValue({
|
||||
id: 'agent-123',
|
||||
|
|
|
|||
|
|
@ -14,6 +14,7 @@ const {
|
|||
createChunk,
|
||||
buildToolSet,
|
||||
scopeSkillIds,
|
||||
loadSkillStates,
|
||||
sendFinalChunk,
|
||||
createSafeUser,
|
||||
validateRequest,
|
||||
|
|
@ -256,6 +257,13 @@ const OpenAIChatCompletionController = async (req, res) => {
|
|||
})
|
||||
: [];
|
||||
|
||||
const { skillStates, defaultActiveOnShare } = await loadSkillStates({
|
||||
userId: req.user.id,
|
||||
appConfig,
|
||||
getUserById: db.getUserById,
|
||||
accessibleSkillIds,
|
||||
});
|
||||
|
||||
const primaryConfig = await initializeAgent(
|
||||
{
|
||||
req,
|
||||
|
|
@ -273,6 +281,8 @@ const OpenAIChatCompletionController = async (req, res) => {
|
|||
ephemeralSkillsToggle ? undefined : agent.skills,
|
||||
),
|
||||
codeEnvAvailable: enabledCapabilities.has(AgentCapabilities.execute_code),
|
||||
skillStates,
|
||||
defaultActiveOnShare,
|
||||
},
|
||||
dbMethods,
|
||||
);
|
||||
|
|
|
|||
|
|
@ -13,6 +13,7 @@ const {
|
|||
createRun,
|
||||
buildToolSet,
|
||||
scopeSkillIds,
|
||||
loadSkillStates,
|
||||
createSafeUser,
|
||||
initializeAgent,
|
||||
getBalanceConfig,
|
||||
|
|
@ -385,6 +386,13 @@ const createResponse = async (req, res) => {
|
|||
})
|
||||
: [];
|
||||
|
||||
const { skillStates, defaultActiveOnShare } = await loadSkillStates({
|
||||
userId: req.user.id,
|
||||
appConfig,
|
||||
getUserById: db.getUserById,
|
||||
accessibleSkillIds,
|
||||
});
|
||||
|
||||
const primaryConfig = await initializeAgent(
|
||||
{
|
||||
req,
|
||||
|
|
@ -402,6 +410,8 @@ const createResponse = async (req, res) => {
|
|||
ephemeralSkillsToggle ? undefined : agent.skills,
|
||||
),
|
||||
codeEnvAvailable: enabledCapabilities.has(AgentCapabilities.execute_code),
|
||||
skillStates,
|
||||
defaultActiveOnShare,
|
||||
},
|
||||
dbMethods,
|
||||
);
|
||||
|
|
|
|||
|
|
@ -3,11 +3,17 @@ const {
|
|||
updateFavoritesController,
|
||||
getFavoritesController,
|
||||
} = require('~/server/controllers/FavoritesController');
|
||||
const {
|
||||
getSkillStatesController,
|
||||
updateSkillStatesController,
|
||||
} = require('~/server/controllers/SkillStatesController');
|
||||
const { requireJwtAuth } = require('~/server/middleware');
|
||||
|
||||
const router = express.Router();
|
||||
|
||||
router.get('/favorites', requireJwtAuth, getFavoritesController);
|
||||
router.post('/favorites', requireJwtAuth, updateFavoritesController);
|
||||
router.get('/skills/active', requireJwtAuth, getSkillStatesController);
|
||||
router.post('/skills/active', requireJwtAuth, updateSkillStatesController);
|
||||
|
||||
module.exports = router;
|
||||
|
|
|
|||
|
|
@ -2,6 +2,7 @@ const { logger } = require('@librechat/data-schemas');
|
|||
const { EnvVar, createContentAggregator } = require('@librechat/agents');
|
||||
const {
|
||||
scopeSkillIds,
|
||||
loadSkillStates,
|
||||
initializeAgent,
|
||||
primeInvokedSkills,
|
||||
validateAgentModel,
|
||||
|
|
@ -124,6 +125,13 @@ const initializeClient = async ({ req, res, signal, endpointOption }) => {
|
|||
})
|
||||
: [];
|
||||
|
||||
const { skillStates, defaultActiveOnShare } = await loadSkillStates({
|
||||
userId: req.user.id,
|
||||
appConfig,
|
||||
getUserById: db.getUserById,
|
||||
accessibleSkillIds,
|
||||
});
|
||||
|
||||
// Resolve code API key once for the entire run (shared by primeInvokedSkills
|
||||
// and enrichWithSkillConfigurable) to avoid redundant auth lookups.
|
||||
let codeApiKey;
|
||||
|
|
@ -243,6 +251,8 @@ const initializeClient = async ({ req, res, signal, endpointOption }) => {
|
|||
ephemeralSkillsToggle ? undefined : primaryAgent.skills,
|
||||
),
|
||||
codeEnvAvailable: enabledCapabilities.has(AgentCapabilities.execute_code),
|
||||
skillStates,
|
||||
defaultActiveOnShare,
|
||||
},
|
||||
{
|
||||
getFiles: db.getFiles,
|
||||
|
|
@ -290,6 +300,8 @@ const initializeClient = async ({ req, res, signal, endpointOption }) => {
|
|||
parentMessageId,
|
||||
computeAccessibleSkillIds: (agent) =>
|
||||
scopeSkillIds(accessibleSkillIds, ephemeralSkillsToggle ? undefined : agent.skills),
|
||||
skillStates,
|
||||
defaultActiveOnShare,
|
||||
},
|
||||
{
|
||||
getAgent: db.getAgent,
|
||||
|
|
|
|||
40
client/src/components/Skills/buttons/SkillToggle.tsx
Normal file
40
client/src/components/Skills/buttons/SkillToggle.tsx
Normal file
|
|
@ -0,0 +1,40 @@
|
|||
import { memo } from 'react';
|
||||
import { cn } from '~/utils';
|
||||
|
||||
interface SkillToggleProps {
|
||||
enabled: boolean;
|
||||
onChange: () => void;
|
||||
ariaLabel: string;
|
||||
}
|
||||
|
||||
function SkillToggle({ enabled, onChange, ariaLabel }: SkillToggleProps) {
|
||||
return (
|
||||
<button
|
||||
type="button"
|
||||
role="switch"
|
||||
aria-checked={enabled}
|
||||
aria-label={ariaLabel}
|
||||
onClick={(e) => {
|
||||
e.stopPropagation();
|
||||
onChange();
|
||||
}}
|
||||
className="inline-flex h-9 items-center justify-center rounded-md px-1 transition-colors hover:bg-surface-hover"
|
||||
>
|
||||
<span
|
||||
className={cn(
|
||||
'relative inline-flex h-5 w-9 shrink-0 rounded-full transition-colors duration-200',
|
||||
enabled ? 'bg-green-500' : 'bg-border-medium',
|
||||
)}
|
||||
>
|
||||
<span
|
||||
className={cn(
|
||||
'pointer-events-none mt-0.5 inline-block size-4 rounded-full bg-white shadow-sm transition-transform duration-200',
|
||||
enabled ? 'translate-x-[1.125rem]' : 'translate-x-0.5',
|
||||
)}
|
||||
/>
|
||||
</span>
|
||||
</button>
|
||||
);
|
||||
}
|
||||
|
||||
export default memo(SkillToggle);
|
||||
|
|
@ -1,3 +1,4 @@
|
|||
export { default as AdminSettings } from './AdminSettings';
|
||||
export { default as SkillToggle } from './SkillToggle';
|
||||
export { default as ShareSkill } from './ShareSkill';
|
||||
export { default as CreateSkillMenu } from './CreateSkillMenu';
|
||||
|
|
|
|||
|
|
@ -3,11 +3,11 @@ import { format } from 'date-fns';
|
|||
import { Eye, Code, User, Calendar, EarthIcon, ScrollText } from 'lucide-react';
|
||||
import { TooltipAnchor } from '@librechat/client';
|
||||
import type { TSkill } from 'librechat-data-provider';
|
||||
import { useLocalize, useAuthContext, useSkillPermissions } from '~/hooks';
|
||||
import { useLocalize, useAuthContext, useSkillPermissions, useSkillActiveState } from '~/hooks';
|
||||
import { ShareSkill, SkillToggle } from '../buttons';
|
||||
import SkillMarkdownRenderer from './SkillMarkdownRenderer';
|
||||
import { parseFrontmatter } from '../utils';
|
||||
import DeleteSkill from '../dialogs/DeleteSkill';
|
||||
import { ShareSkill } from '../buttons';
|
||||
import { cn } from '~/utils';
|
||||
|
||||
interface SkillDetailProps {
|
||||
|
|
@ -68,7 +68,9 @@ export default function SkillDetail({ skill, onEdit, onDelete }: SkillDetailProp
|
|||
const localize = useLocalize();
|
||||
const { user } = useAuthContext();
|
||||
const permissions = useSkillPermissions(skill);
|
||||
const { isActive, toggle } = useSkillActiveState();
|
||||
const [viewMode, setViewMode] = useState<'rendered' | 'source'>('rendered');
|
||||
const skillEnabled = isActive(skill);
|
||||
|
||||
const isPublic = skill.isPublic === true;
|
||||
const isShared = skill.author !== user?.id && Boolean(skill.authorName);
|
||||
|
|
@ -130,6 +132,11 @@ export default function SkillDetail({ skill, onEdit, onDelete }: SkillDetailProp
|
|||
|
||||
{/* Actions */}
|
||||
<div className="flex shrink-0 items-center gap-2">
|
||||
<SkillToggle
|
||||
enabled={skillEnabled}
|
||||
onChange={() => toggle(skill)}
|
||||
ariaLabel={localize('com_ui_skill_toggle_active')}
|
||||
/>
|
||||
<ShareSkill skill={skill} />
|
||||
{permissions.canEdit && onEdit && (
|
||||
<button
|
||||
|
|
|
|||
|
|
@ -3,8 +3,8 @@ import { ChevronRight } from 'lucide-react';
|
|||
import { useSearchParams } from 'react-router-dom';
|
||||
import { Skeleton } from '@librechat/client';
|
||||
import type { TSkill } from 'librechat-data-provider';
|
||||
import SkillListItem from './SkillListItem';
|
||||
import { useLocalize } from '~/hooks';
|
||||
import SkillListItem from './SkillListItem';
|
||||
import { cn } from '~/utils';
|
||||
|
||||
interface SkillListProps {
|
||||
|
|
@ -13,9 +13,7 @@ interface SkillListProps {
|
|||
activeSkillId?: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* Claude.ai–style skill list with a collapsible "Personal skills" section.
|
||||
*/
|
||||
/** Collapsible skill list. Active/inactive toggling lives in the detail view. */
|
||||
export default function SkillList({ skills, isLoading, activeSkillId }: SkillListProps) {
|
||||
const localize = useLocalize();
|
||||
const [searchParams] = useSearchParams();
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
import { useQuery, useInfiniteQuery } from '@tanstack/react-query';
|
||||
import { useQuery, useMutation, useInfiniteQuery, useQueryClient } from '@tanstack/react-query';
|
||||
import { QueryKeys, dataService } from 'librechat-data-provider';
|
||||
import type {
|
||||
QueryObserverResult,
|
||||
|
|
@ -9,6 +9,7 @@ import type {
|
|||
TSkill,
|
||||
TSkillListRequest,
|
||||
TSkillListResponse,
|
||||
TSkillStatesResponse,
|
||||
TListSkillFilesResponse,
|
||||
TSkillFileContentResponse,
|
||||
} from 'librechat-data-provider';
|
||||
|
|
@ -149,3 +150,39 @@ export const useGetSkillFileContentQuery = (
|
|||
},
|
||||
);
|
||||
};
|
||||
|
||||
/** Per-user skill active/inactive overrides. */
|
||||
export const useGetSkillStatesQuery = (
|
||||
config?: Omit<UseQueryOptions<TSkillStatesResponse, Error>, 'queryKey' | 'queryFn'>,
|
||||
) => {
|
||||
return useQuery<TSkillStatesResponse, Error>(
|
||||
[QueryKeys.skillStates],
|
||||
() => dataService.getSkillStates(),
|
||||
{
|
||||
refetchOnWindowFocus: false,
|
||||
refetchOnReconnect: false,
|
||||
refetchOnMount: false,
|
||||
...config,
|
||||
},
|
||||
);
|
||||
};
|
||||
|
||||
export const useUpdateSkillStatesMutation = () => {
|
||||
const queryClient = useQueryClient();
|
||||
return useMutation(
|
||||
(skillStates: TSkillStatesResponse) => dataService.updateSkillStates(skillStates),
|
||||
{
|
||||
onMutate: async (next) => {
|
||||
await queryClient.cancelQueries([QueryKeys.skillStates]);
|
||||
const previous = queryClient.getQueryData<TSkillStatesResponse>([QueryKeys.skillStates]);
|
||||
queryClient.setQueryData([QueryKeys.skillStates], next);
|
||||
return { previous };
|
||||
},
|
||||
onError: (_err, _next, context) => {
|
||||
if (context?.previous !== undefined) {
|
||||
queryClient.setQueryData([QueryKeys.skillStates], context.previous);
|
||||
}
|
||||
},
|
||||
},
|
||||
);
|
||||
};
|
||||
|
|
|
|||
|
|
@ -1,2 +1,3 @@
|
|||
export { default as useSkillActiveState } from './useSkillActiveState';
|
||||
export { default as useSkillPermissions } from './useSkillPermissions';
|
||||
export type { SkillPermissions } from './useSkillPermissions';
|
||||
|
|
|
|||
214
client/src/hooks/Skills/useSkillActiveState.ts
Normal file
214
client/src/hooks/Skills/useSkillActiveState.ts
Normal file
|
|
@ -0,0 +1,214 @@
|
|||
import { useCallback, useEffect, useMemo } from 'react';
|
||||
import { useQueryClient } from '@tanstack/react-query';
|
||||
import { QueryKeys } from 'librechat-data-provider';
|
||||
import { useToastContext } from '@librechat/client';
|
||||
import type { TSkillStatesResponse } from 'librechat-data-provider';
|
||||
import {
|
||||
useGetSkillStatesQuery,
|
||||
useUpdateSkillStatesMutation,
|
||||
useGetStartupConfig,
|
||||
} from '~/data-provider';
|
||||
import { useAuthContext, useLocalize } from '~/hooks';
|
||||
import { logger } from '~/utils';
|
||||
|
||||
const EMPTY_STATES: TSkillStatesResponse = {};
|
||||
|
||||
interface WriteQueue {
|
||||
pending: TSkillStatesResponse | null;
|
||||
inFlight: boolean;
|
||||
/**
|
||||
* Set when the active user changes so any in-flight flush loop still
|
||||
* holding a reference to this queue exits before sending another request
|
||||
* under the new user's auth context. Once abandoned, the queue is single-
|
||||
* use-dead: callers must create a new one via `getWriteQueue`.
|
||||
*/
|
||||
abandoned: boolean;
|
||||
}
|
||||
|
||||
/**
|
||||
* Module-scoped, per-user write queues so every hook instance (SkillList,
|
||||
* SkillDetail, etc.) shares a single in-flight slot for the active user.
|
||||
* Per-instance refs let two components race full-map POSTs and drop toggles
|
||||
* via last-writer-wins. Keying by `userId` prevents pending writes from
|
||||
* leaking across account transitions in the same browser tab (logout/login
|
||||
* flushing a previous user's snapshot into the new session). `lastSeenUserId`
|
||||
* tracks the most recent active identity so we can evict and abandon a prior
|
||||
* user's queue entry when the active identity changes.
|
||||
*/
|
||||
const writeQueues = new Map<string, WriteQueue>();
|
||||
let lastSeenUserId: string | null = null;
|
||||
|
||||
/** Marks every queue whose userId does not match `activeUserId` as abandoned. */
|
||||
function abandonOtherQueues(activeUserId: string): void {
|
||||
for (const [id, queue] of writeQueues) {
|
||||
if (id !== activeUserId) {
|
||||
queue.abandoned = true;
|
||||
queue.pending = null;
|
||||
writeQueues.delete(id);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
function getWriteQueue(userId: string): WriteQueue {
|
||||
if (lastSeenUserId !== null && lastSeenUserId !== userId) {
|
||||
abandonOtherQueues(userId);
|
||||
}
|
||||
lastSeenUserId = userId;
|
||||
let queue = writeQueues.get(userId);
|
||||
if (!queue) {
|
||||
queue = { pending: null, inFlight: false, abandoned: false };
|
||||
writeQueues.set(userId, queue);
|
||||
}
|
||||
return queue;
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolves the default active state for a skill the user has never toggled.
|
||||
*
|
||||
* - Owned skills (author === currentUser) default to **active**.
|
||||
* - Shared skills default to the `defaultActiveOnShare` config value (default `false`).
|
||||
*/
|
||||
function resolveDefault(author: string, userId: string, defaultActiveOnShare: boolean): boolean {
|
||||
return author === userId ? true : defaultActiveOnShare;
|
||||
}
|
||||
|
||||
/**
|
||||
* Hook for managing per-user skill active/inactive state.
|
||||
*
|
||||
* The `skillStates` map stores explicit overrides (`{ [skillId]: boolean }`).
|
||||
* Skills absent from the map use the ownership-based default: owned -> active,
|
||||
* shared -> `defaultActiveOnShare` from the interface config. Toggles that
|
||||
* land on the resolved default remove the key from the map rather than
|
||||
* persisting a redundant entry, keeping `skillStates` strictly an exceptions
|
||||
* list (otherwise rapid round-trip toggles would exhaust the 200-entry cap).
|
||||
*
|
||||
* React Query is the single source of truth. Toggling drives an optimistic
|
||||
* mutation that updates the cache, identical to the favorites pattern.
|
||||
* Toggling is blocked until the initial fetch succeeds, preventing an empty
|
||||
* baseline (from isLoading or a failed GET) from wiping server-side overrides.
|
||||
* Writes are serialized via a module-scoped queue so rapid toggles from any
|
||||
* hook instance cannot race: only one request is ever in flight, and the
|
||||
* latest desired state is sent when the previous one settles. Each toggle
|
||||
* reads the latest optimistic state directly from the React Query cache so
|
||||
* rapid successive toggles cannot drop earlier changes via stale closure.
|
||||
* An `abandoned` flag on the queue guards against cross-session writes: on
|
||||
* logout/login, the previous user's queue is marked abandoned so an in-flight
|
||||
* flush loop exits instead of posting the stale snapshot under the new auth.
|
||||
*/
|
||||
export default function useSkillActiveState() {
|
||||
const localize = useLocalize();
|
||||
const { user } = useAuthContext();
|
||||
const { showToast } = useToastContext();
|
||||
const queryClient = useQueryClient();
|
||||
const configQuery = useGetStartupConfig();
|
||||
const getQuery = useGetSkillStatesQuery();
|
||||
const updateMutation = useUpdateSkillStatesMutation();
|
||||
|
||||
const userId = user?.id ?? '';
|
||||
|
||||
/**
|
||||
* Proactively abandon other users' queues whenever the active identity
|
||||
* changes. `getWriteQueue` already handles this on the next toggle, but an
|
||||
* in-flight flush loop from the previous user can fire its next iteration
|
||||
* before any toggle runs — that iteration would carry the stale payload
|
||||
* under the new user's cookies. Running this effect on every userId change
|
||||
* closes that window for any hook instance that renders post-login.
|
||||
*/
|
||||
useEffect(() => {
|
||||
if (!userId) {
|
||||
return;
|
||||
}
|
||||
abandonOtherQueues(userId);
|
||||
lastSeenUserId = userId;
|
||||
}, [userId]);
|
||||
|
||||
const defaultActiveOnShare = useMemo(() => {
|
||||
const skills = configQuery.data?.interface?.skills;
|
||||
if (typeof skills === 'object' && skills !== null && 'defaultActiveOnShare' in skills) {
|
||||
return skills.defaultActiveOnShare === true;
|
||||
}
|
||||
return false;
|
||||
}, [configQuery.data]);
|
||||
|
||||
const skillStates = useMemo<TSkillStatesResponse>(
|
||||
() => (getQuery.data && typeof getQuery.data === 'object' ? getQuery.data : EMPTY_STATES),
|
||||
[getQuery.data],
|
||||
);
|
||||
|
||||
const canToggle = !getQuery.isLoading && !getQuery.isError && getQuery.data !== undefined;
|
||||
|
||||
const flush = useCallback(async () => {
|
||||
if (!userId) {
|
||||
return;
|
||||
}
|
||||
const queue = getWriteQueue(userId);
|
||||
while (queue.pending !== null && !queue.abandoned) {
|
||||
const next = queue.pending;
|
||||
queue.pending = null;
|
||||
queue.inFlight = true;
|
||||
try {
|
||||
await updateMutation.mutateAsync(next);
|
||||
} catch (error) {
|
||||
logger.error('Error updating skill states:', error);
|
||||
const data = (error as { response?: { data?: { code?: string; limit?: number } } })
|
||||
?.response?.data;
|
||||
const message =
|
||||
data?.code === 'MAX_SKILL_STATES_EXCEEDED'
|
||||
? localize('com_ui_skill_states_limit', { 0: String(data.limit ?? '') })
|
||||
: localize('com_ui_error');
|
||||
showToast({ message, status: 'error' });
|
||||
queue.pending = null;
|
||||
break;
|
||||
}
|
||||
}
|
||||
queue.inFlight = false;
|
||||
}, [userId, updateMutation, showToast, localize]);
|
||||
|
||||
const isActive = useCallback(
|
||||
(skill: { _id: string; author: string }): boolean => {
|
||||
const override = skillStates[skill._id];
|
||||
if (override !== undefined) {
|
||||
return override;
|
||||
}
|
||||
return resolveDefault(skill.author, userId, defaultActiveOnShare);
|
||||
},
|
||||
[skillStates, userId, defaultActiveOnShare],
|
||||
);
|
||||
|
||||
const toggle = useCallback(
|
||||
(skill: { _id: string; author: string }) => {
|
||||
if (!canToggle || !userId) {
|
||||
return;
|
||||
}
|
||||
const queue = getWriteQueue(userId);
|
||||
const cached =
|
||||
queryClient.getQueryData<TSkillStatesResponse>([QueryKeys.skillStates]) ?? EMPTY_STATES;
|
||||
const baseline = queue.pending ?? cached;
|
||||
const defaultValue = resolveDefault(skill.author, userId, defaultActiveOnShare);
|
||||
const override = baseline[skill._id];
|
||||
const currentActive = override !== undefined ? override : defaultValue;
|
||||
const nextValue = !currentActive;
|
||||
const next = { ...baseline };
|
||||
if (nextValue === defaultValue) {
|
||||
delete next[skill._id];
|
||||
} else {
|
||||
next[skill._id] = nextValue;
|
||||
}
|
||||
queue.pending = next;
|
||||
if (!queue.inFlight) {
|
||||
flush();
|
||||
}
|
||||
},
|
||||
[queryClient, userId, defaultActiveOnShare, canToggle, flush],
|
||||
);
|
||||
|
||||
return {
|
||||
skillStates,
|
||||
defaultActiveOnShare,
|
||||
isActive,
|
||||
toggle,
|
||||
isLoading: getQuery.isLoading,
|
||||
isError: getQuery.isError,
|
||||
isUpdating: updateMutation.isLoading,
|
||||
};
|
||||
}
|
||||
|
|
@ -1463,6 +1463,8 @@
|
|||
"com_ui_size": "Size",
|
||||
"com_ui_size_sort": "Sort by Size",
|
||||
"com_ui_skill": "Skill",
|
||||
"com_ui_skill_states_limit": "You've reached the limit of {{0}} active/inactive skill overrides. Remove some overrides to toggle new skills.",
|
||||
"com_ui_skill_toggle_active": "Toggle skill active state",
|
||||
"com_ui_skill_content": "Skill Content",
|
||||
"com_ui_skill_content_placeholder": "Enter your skill instructions in markdown...",
|
||||
"com_ui_skill_create_error": "Failed to create skill",
|
||||
|
|
|
|||
|
|
@ -1,4 +1,8 @@
|
|||
/** Mock Constants.SKILL_TOOL since the installed SDK version may not include it yet */
|
||||
/**
|
||||
* Mock the pieces of `@librechat/agents` the installed SDK version may not
|
||||
* export yet. Includes both the `Constants.SKILL_TOOL` stub and the skill
|
||||
* catalog/tool-definition helpers needed to exercise `injectSkillCatalog`.
|
||||
*/
|
||||
jest.mock('@librechat/agents', () => ({
|
||||
...jest.requireActual('@librechat/agents'),
|
||||
Constants: {
|
||||
|
|
@ -6,12 +10,33 @@ jest.mock('@librechat/agents', () => ({
|
|||
.Constants,
|
||||
SKILL_TOOL: 'skill',
|
||||
},
|
||||
formatSkillCatalog: (skills: Array<{ name: string; description: string }>) =>
|
||||
skills.map((s) => `- ${s.name}: ${s.description}`).join('\n'),
|
||||
SkillToolDefinition: { name: 'skill', description: 'skill tool', parameters: {} },
|
||||
ReadFileToolDefinition: {
|
||||
name: 'read_file',
|
||||
description: 'read file',
|
||||
parameters: {},
|
||||
responseFormat: 'content',
|
||||
},
|
||||
BashExecutionToolDefinition: {
|
||||
name: 'bash_tool',
|
||||
description: 'bash',
|
||||
schema: {},
|
||||
},
|
||||
}));
|
||||
|
||||
import { Types } from 'mongoose';
|
||||
import { scopeSkillIds } from '../skills';
|
||||
import { scopeSkillIds, resolveSkillActive, injectSkillCatalog } from '../skills';
|
||||
import { extractInvokedSkillsFromPayload } from '../run';
|
||||
|
||||
type PageSkill = {
|
||||
_id: Types.ObjectId;
|
||||
name: string;
|
||||
description: string;
|
||||
author: Types.ObjectId;
|
||||
};
|
||||
|
||||
describe('extractInvokedSkillsFromPayload', () => {
|
||||
it('extracts skill names from assistant messages with skill tool_calls', () => {
|
||||
const payload = [
|
||||
|
|
@ -248,3 +273,289 @@ describe('scopeSkillIds', () => {
|
|||
expect(scopeSkillIds([], [new Types.ObjectId().toString()])).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('resolveSkillActive', () => {
|
||||
const makeSkill = (author: Types.ObjectId) => ({ _id: new Types.ObjectId(), author });
|
||||
|
||||
it('fails closed when userId is undefined and no override is set', () => {
|
||||
const skill = makeSkill(new Types.ObjectId());
|
||||
expect(
|
||||
resolveSkillActive({
|
||||
skill,
|
||||
skillStates: {},
|
||||
userId: undefined,
|
||||
defaultActiveOnShare: true,
|
||||
}),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it('fails closed when userId is undefined even if defaultActiveOnShare is true', () => {
|
||||
const skill = makeSkill(new Types.ObjectId());
|
||||
expect(
|
||||
resolveSkillActive({
|
||||
skill,
|
||||
userId: undefined,
|
||||
defaultActiveOnShare: true,
|
||||
}),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it('respects explicit override = true regardless of ownership or config', () => {
|
||||
const userId = new Types.ObjectId().toString();
|
||||
const sharedSkill = makeSkill(new Types.ObjectId());
|
||||
expect(
|
||||
resolveSkillActive({
|
||||
skill: sharedSkill,
|
||||
skillStates: { [sharedSkill._id.toString()]: true },
|
||||
userId,
|
||||
defaultActiveOnShare: false,
|
||||
}),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it('respects explicit override = false even for owned skills', () => {
|
||||
const userObjectId = new Types.ObjectId();
|
||||
const userId = userObjectId.toString();
|
||||
const ownedSkill = makeSkill(userObjectId);
|
||||
expect(
|
||||
resolveSkillActive({
|
||||
skill: ownedSkill,
|
||||
skillStates: { [ownedSkill._id.toString()]: false },
|
||||
userId,
|
||||
defaultActiveOnShare: true,
|
||||
}),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it('owned skills default to active when no override is present', () => {
|
||||
const userObjectId = new Types.ObjectId();
|
||||
const userId = userObjectId.toString();
|
||||
const ownedSkill = makeSkill(userObjectId);
|
||||
expect(
|
||||
resolveSkillActive({
|
||||
skill: ownedSkill,
|
||||
skillStates: {},
|
||||
userId,
|
||||
defaultActiveOnShare: false,
|
||||
}),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it('shared skills default to inactive when defaultActiveOnShare is false', () => {
|
||||
const userId = new Types.ObjectId().toString();
|
||||
const sharedSkill = makeSkill(new Types.ObjectId());
|
||||
expect(
|
||||
resolveSkillActive({
|
||||
skill: sharedSkill,
|
||||
skillStates: {},
|
||||
userId,
|
||||
defaultActiveOnShare: false,
|
||||
}),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it('shared skills default to active when defaultActiveOnShare is true', () => {
|
||||
const userId = new Types.ObjectId().toString();
|
||||
const sharedSkill = makeSkill(new Types.ObjectId());
|
||||
expect(
|
||||
resolveSkillActive({
|
||||
skill: sharedSkill,
|
||||
skillStates: {},
|
||||
userId,
|
||||
defaultActiveOnShare: true,
|
||||
}),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it('treats skillStates = undefined identically to an empty object', () => {
|
||||
const userObjectId = new Types.ObjectId();
|
||||
const userId = userObjectId.toString();
|
||||
const ownedSkill = makeSkill(userObjectId);
|
||||
const sharedSkill = makeSkill(new Types.ObjectId());
|
||||
|
||||
expect(resolveSkillActive({ skill: ownedSkill, userId, defaultActiveOnShare: false })).toBe(
|
||||
resolveSkillActive({
|
||||
skill: ownedSkill,
|
||||
skillStates: {},
|
||||
userId,
|
||||
defaultActiveOnShare: false,
|
||||
}),
|
||||
);
|
||||
expect(resolveSkillActive({ skill: sharedSkill, userId, defaultActiveOnShare: true })).toBe(
|
||||
resolveSkillActive({
|
||||
skill: sharedSkill,
|
||||
skillStates: {},
|
||||
userId,
|
||||
defaultActiveOnShare: true,
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('defaults defaultActiveOnShare to false when the param is omitted', () => {
|
||||
const userId = new Types.ObjectId().toString();
|
||||
const sharedSkill = makeSkill(new Types.ObjectId());
|
||||
expect(resolveSkillActive({ skill: sharedSkill, skillStates: {}, userId })).toBe(false);
|
||||
});
|
||||
|
||||
it('ignores defaultActiveOnShare for owned skills', () => {
|
||||
const userObjectId = new Types.ObjectId();
|
||||
const userId = userObjectId.toString();
|
||||
const ownedSkill = makeSkill(userObjectId);
|
||||
expect(
|
||||
resolveSkillActive({
|
||||
skill: ownedSkill,
|
||||
skillStates: {},
|
||||
userId,
|
||||
defaultActiveOnShare: false,
|
||||
}),
|
||||
).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('injectSkillCatalog', () => {
|
||||
const userId = new Types.ObjectId().toString();
|
||||
const userObjectId = new Types.ObjectId(userId);
|
||||
|
||||
/** Minimal `Agent` shape `injectSkillCatalog` actually reads/writes. */
|
||||
type MockAgent = Parameters<typeof injectSkillCatalog>[0]['agent'];
|
||||
|
||||
function makeAgent(): MockAgent {
|
||||
return { additional_instructions: undefined } as MockAgent;
|
||||
}
|
||||
|
||||
function makeSkill(name: string, author: Types.ObjectId = userObjectId): PageSkill {
|
||||
return {
|
||||
_id: new Types.ObjectId(),
|
||||
name,
|
||||
description: `desc-${name}`,
|
||||
author,
|
||||
};
|
||||
}
|
||||
|
||||
function buildPager(pages: PageSkill[][]) {
|
||||
return jest.fn().mockImplementation(async ({ cursor }: { cursor?: string | null }) => {
|
||||
const pageIndex = cursor ? Number(cursor) : 0;
|
||||
const skills = pages[pageIndex] ?? [];
|
||||
const has_more = pageIndex < pages.length - 1;
|
||||
return {
|
||||
skills,
|
||||
has_more,
|
||||
after: has_more ? String(pageIndex + 1) : null,
|
||||
};
|
||||
});
|
||||
}
|
||||
|
||||
function baseParams(overrides: Partial<Parameters<typeof injectSkillCatalog>[0]> = {}) {
|
||||
return {
|
||||
agent: makeAgent(),
|
||||
toolDefinitions: undefined,
|
||||
toolRegistry: undefined,
|
||||
accessibleSkillIds: [new Types.ObjectId()],
|
||||
contextWindowTokens: 200_000,
|
||||
listSkillsByAccess: jest.fn(),
|
||||
userId,
|
||||
skillStates: {},
|
||||
defaultActiveOnShare: false,
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
it('returns empty when no skills are accessible', async () => {
|
||||
const result = await injectSkillCatalog(
|
||||
baseParams({ accessibleSkillIds: [], listSkillsByAccess: jest.fn() }),
|
||||
);
|
||||
expect(result.skillCount).toBe(0);
|
||||
expect(result.activeSkillIds).toEqual([]);
|
||||
});
|
||||
|
||||
it('returns empty when listSkillsByAccess is not provided', async () => {
|
||||
const result = await injectSkillCatalog(baseParams({ listSkillsByAccess: undefined }));
|
||||
expect(result.skillCount).toBe(0);
|
||||
expect(result.activeSkillIds).toEqual([]);
|
||||
});
|
||||
|
||||
it('filters out inactive skills on a single page (shared, default inactive)', async () => {
|
||||
const owned = makeSkill('my-skill', userObjectId);
|
||||
const sharedInactive = makeSkill('other-skill', new Types.ObjectId());
|
||||
const listSkillsByAccess = buildPager([[owned, sharedInactive]]);
|
||||
const result = await injectSkillCatalog(baseParams({ listSkillsByAccess }));
|
||||
expect(result.skillCount).toBe(1);
|
||||
expect(result.activeSkillIds.map((id) => id.toString())).toEqual([owned._id.toString()]);
|
||||
});
|
||||
|
||||
it('includes shared skills when defaultActiveOnShare is true', async () => {
|
||||
const shared = makeSkill('other-skill', new Types.ObjectId());
|
||||
const listSkillsByAccess = buildPager([[shared]]);
|
||||
const result = await injectSkillCatalog(
|
||||
baseParams({ listSkillsByAccess, defaultActiveOnShare: true }),
|
||||
);
|
||||
expect(result.skillCount).toBe(1);
|
||||
expect(result.activeSkillIds.map((id) => id.toString())).toEqual([shared._id.toString()]);
|
||||
});
|
||||
|
||||
it('honors explicit overrides (deactivated owned skill absent from catalog)', async () => {
|
||||
const owned = makeSkill('off', userObjectId);
|
||||
const listSkillsByAccess = buildPager([[owned]]);
|
||||
const result = await injectSkillCatalog(
|
||||
baseParams({
|
||||
listSkillsByAccess,
|
||||
skillStates: { [owned._id.toString()]: false },
|
||||
}),
|
||||
);
|
||||
expect(result.skillCount).toBe(0);
|
||||
expect(result.activeSkillIds).toEqual([]);
|
||||
});
|
||||
|
||||
it('paginates across pages and collects active skills from later pages', async () => {
|
||||
const sharedInactive = Array.from({ length: 3 }, (_, i) =>
|
||||
makeSkill(`shared-${i}`, new Types.ObjectId()),
|
||||
);
|
||||
const owned = makeSkill('my-skill', userObjectId);
|
||||
const listSkillsByAccess = buildPager([sharedInactive, [owned]]);
|
||||
const result = await injectSkillCatalog(baseParams({ listSkillsByAccess }));
|
||||
expect(result.skillCount).toBe(1);
|
||||
expect(result.activeSkillIds.map((id) => id.toString())).toEqual([owned._id.toString()]);
|
||||
expect(listSkillsByAccess).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it('stops paginating at MAX_CATALOG_PAGES even if no active skills found', async () => {
|
||||
const inactivePage = Array.from({ length: 10 }, (_, i) =>
|
||||
makeSkill(`shared-${i}`, new Types.ObjectId()),
|
||||
);
|
||||
// Build 12 pages, all inactive — scanner should stop at page cap.
|
||||
const pages = Array.from({ length: 12 }, () => inactivePage);
|
||||
const listSkillsByAccess = buildPager(pages);
|
||||
const result = await injectSkillCatalog(baseParams({ listSkillsByAccess }));
|
||||
expect(result.skillCount).toBe(0);
|
||||
expect(result.activeSkillIds).toEqual([]);
|
||||
// MAX_CATALOG_PAGES = 10 — loop terminates after exactly 10 page fetches.
|
||||
expect(listSkillsByAccess).toHaveBeenCalledTimes(10);
|
||||
});
|
||||
|
||||
it('terminates early when has_more is false even below the catalog limit', async () => {
|
||||
const owned = makeSkill('solo', userObjectId);
|
||||
const listSkillsByAccess = buildPager([[owned]]);
|
||||
await injectSkillCatalog(baseParams({ listSkillsByAccess }));
|
||||
expect(listSkillsByAccess).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('appends the catalog text to agent.additional_instructions', async () => {
|
||||
const owned = makeSkill('my-skill', userObjectId);
|
||||
const listSkillsByAccess = buildPager([[owned]]);
|
||||
const agent = makeAgent();
|
||||
await injectSkillCatalog(baseParams({ listSkillsByAccess, agent }));
|
||||
expect(agent.additional_instructions).toContain('my-skill');
|
||||
expect(agent.additional_instructions).toContain('desc-my-skill');
|
||||
});
|
||||
|
||||
it('fails closed when userId is absent (shared skills drop, owned would need override)', async () => {
|
||||
const owned = makeSkill('my-skill', userObjectId);
|
||||
const shared = makeSkill('shared-skill', new Types.ObjectId());
|
||||
const listSkillsByAccess = buildPager([[owned, shared]]);
|
||||
const result = await injectSkillCatalog(
|
||||
baseParams({ listSkillsByAccess, userId: undefined, defaultActiveOnShare: true }),
|
||||
);
|
||||
expect(result.skillCount).toBe(0);
|
||||
expect(result.activeSkillIds).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -69,6 +69,10 @@ export interface DiscoverConnectedAgentsParams {
|
|||
* allowlist (or the full accessible set when scoping is disabled).
|
||||
*/
|
||||
computeAccessibleSkillIds?: (agent: Agent) => InitializeAgentParams['accessibleSkillIds'];
|
||||
/** Per-user skill active/inactive state, forwarded to each sub-agent. */
|
||||
skillStates?: InitializeAgentParams['skillStates'];
|
||||
/** Default active-on-share flag, forwarded to each sub-agent. */
|
||||
defaultActiveOnShare?: InitializeAgentParams['defaultActiveOnShare'];
|
||||
}
|
||||
|
||||
export interface DiscoverConnectedAgentsDeps {
|
||||
|
|
@ -134,6 +138,8 @@ export async function discoverConnectedAgents(
|
|||
parentMessageId,
|
||||
resourceType = ResourceType.AGENT,
|
||||
computeAccessibleSkillIds,
|
||||
skillStates,
|
||||
defaultActiveOnShare,
|
||||
} = params;
|
||||
|
||||
const {
|
||||
|
|
@ -232,6 +238,8 @@ export async function discoverConnectedAgents(
|
|||
endpointOption: subAgentEndpointOption,
|
||||
allowedProviders,
|
||||
accessibleSkillIds: computeAccessibleSkillIds?.(agent),
|
||||
skillStates,
|
||||
defaultActiveOnShare,
|
||||
},
|
||||
db,
|
||||
);
|
||||
|
|
|
|||
|
|
@ -123,6 +123,10 @@ export interface InitializeAgentParams {
|
|||
accessibleSkillIds?: import('mongoose').Types.ObjectId[];
|
||||
/** Whether the code execution environment is available (execute_code capability enabled) */
|
||||
codeEnvAvailable?: boolean;
|
||||
/** Per-user skill active/inactive overrides for filtering the skill catalog. */
|
||||
skillStates?: Record<string, boolean>;
|
||||
/** Admin-configured default for shared skills (`true` = shared skills auto-activate). */
|
||||
defaultActiveOnShare?: boolean;
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -158,7 +162,17 @@ export interface InitializeAgentDbMethods extends EndpointDbMethods {
|
|||
listSkillsByAccess?: (params: {
|
||||
accessibleIds: import('mongoose').Types.ObjectId[];
|
||||
limit: number;
|
||||
}) => Promise<{ skills: Array<{ name: string; description: string }> }>;
|
||||
cursor?: string | null;
|
||||
}) => Promise<{
|
||||
skills: Array<{
|
||||
_id: import('mongoose').Types.ObjectId;
|
||||
name: string;
|
||||
description: string;
|
||||
author: import('mongoose').Types.ObjectId;
|
||||
}>;
|
||||
has_more?: boolean;
|
||||
after?: string | null;
|
||||
}>;
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -433,6 +447,13 @@ export async function initializeAgent(
|
|||
}
|
||||
|
||||
let skillCount = 0;
|
||||
/**
|
||||
* IDs authorized for runtime skill execution — starts as the ACL-scoped set
|
||||
* and gets replaced with the active-filtered subset after catalog injection.
|
||||
* Ensures `getSkillByName` cannot resolve a deactivated skill even if the
|
||||
* LLM (or a direct-invocation path) names one.
|
||||
*/
|
||||
let executableSkillIds = params.accessibleSkillIds;
|
||||
const { accessibleSkillIds } = params;
|
||||
if (accessibleSkillIds && accessibleSkillIds.length > 0) {
|
||||
const skillResult = await injectSkillCatalog({
|
||||
|
|
@ -443,9 +464,13 @@ export async function initializeAgent(
|
|||
contextWindowTokens: Number(agentMaxContextTokens) || 200_000,
|
||||
listSkillsByAccess: db?.listSkillsByAccess,
|
||||
codeEnvAvailable: params.codeEnvAvailable,
|
||||
userId: req.user?.id,
|
||||
skillStates: params.skillStates,
|
||||
defaultActiveOnShare: params.defaultActiveOnShare,
|
||||
});
|
||||
toolDefinitions = skillResult.toolDefinitions;
|
||||
skillCount = skillResult.skillCount;
|
||||
executableSkillIds = skillResult.activeSkillIds;
|
||||
}
|
||||
|
||||
const agentMaxContextNum = Number(agentMaxContextTokens) || DEFAULT_MAX_CONTEXT_TOKENS;
|
||||
|
|
@ -480,7 +505,7 @@ export async function initializeAgent(
|
|||
actionsEnabled,
|
||||
baseContextTokens,
|
||||
skillCount,
|
||||
accessibleSkillIds: params.accessibleSkillIds,
|
||||
accessibleSkillIds: executableSkillIds,
|
||||
attachments: finalAttachments,
|
||||
toolContextMap: toolContextMap ?? {},
|
||||
useLegacyContent: !!options.useLegacyContent,
|
||||
|
|
|
|||
|
|
@ -11,6 +11,10 @@ import type { Agent } from 'librechat-data-provider';
|
|||
import type { InitializeAgentDbMethods } from './initialize';
|
||||
|
||||
const SKILL_CATALOG_LIMIT = 100;
|
||||
/** Max pages scanned per run when filtering out inactive skills. */
|
||||
const MAX_CATALOG_PAGES = 10;
|
||||
/** Page size used when paginating to fill the active-skill quota. */
|
||||
const CATALOG_PAGE_SIZE = 100;
|
||||
|
||||
/**
|
||||
* Scopes user-accessible skill IDs to only those configured on the agent.
|
||||
|
|
@ -37,6 +41,39 @@ export function scopeSkillIds(
|
|||
return accessibleSkillIds.filter((oid) => agentSet.has(oid.toString()));
|
||||
}
|
||||
|
||||
export interface ResolveSkillActiveParams {
|
||||
/** Skill being evaluated. Only `_id` and `author` matter for resolution. */
|
||||
skill: { _id: Types.ObjectId | string; author: Types.ObjectId | string };
|
||||
/** Per-user overrides: `{ [skillId]: boolean }`. Missing entries use the default. */
|
||||
skillStates?: Record<string, boolean>;
|
||||
/** Current user ID. When absent, the function fails closed for all non-overridden skills. */
|
||||
userId?: string;
|
||||
/** Admin-configured default for shared skills. `true` = shared skills auto-activate. */
|
||||
defaultActiveOnShare?: boolean;
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolves whether a skill should be injected into the agent catalog for the
|
||||
* current user. Precedence (pinned by unit tests):
|
||||
*
|
||||
* 1. Explicit override in `skillStates` wins above all.
|
||||
* 2. Absent `userId` → fail closed. The caller lost user context, so we do
|
||||
* not fall back to ownership-based defaults that could leak shared skills.
|
||||
* 3. Owned skills (author === userId) default to **active**.
|
||||
* 4. Shared skills default to `defaultActiveOnShare` (admin-configured, default `false`).
|
||||
*/
|
||||
export function resolveSkillActive(params: ResolveSkillActiveParams): boolean {
|
||||
const { skill, skillStates, userId, defaultActiveOnShare = false } = params;
|
||||
const override = skillStates?.[skill._id.toString()];
|
||||
if (override !== undefined) {
|
||||
return override;
|
||||
}
|
||||
if (!userId) {
|
||||
return false;
|
||||
}
|
||||
return skill.author.toString() === userId ? true : defaultActiveOnShare;
|
||||
}
|
||||
|
||||
export interface InjectSkillCatalogParams {
|
||||
agent: Agent;
|
||||
toolDefinitions: LCTool[] | undefined;
|
||||
|
|
@ -46,11 +83,24 @@ export interface InjectSkillCatalogParams {
|
|||
listSkillsByAccess: InitializeAgentDbMethods['listSkillsByAccess'];
|
||||
/** When true, registers bash_tool alongside skill + read_file. */
|
||||
codeEnvAvailable?: boolean;
|
||||
/** Current user ID — used to determine skill ownership for active-state resolution. */
|
||||
userId?: string;
|
||||
/** Per-user skill overrides: `{ [skillId]: boolean }`. Missing entries use the default. */
|
||||
skillStates?: Record<string, boolean>;
|
||||
/** Admin-configured default for shared skills. `true` = shared skills auto-activate. */
|
||||
defaultActiveOnShare?: boolean;
|
||||
}
|
||||
|
||||
export interface InjectSkillCatalogResult {
|
||||
toolDefinitions: LCTool[] | undefined;
|
||||
skillCount: number;
|
||||
/**
|
||||
* IDs of skills that passed the active-state filter and appear in the
|
||||
* injected catalog. Runtime tool execution must authorize against this set,
|
||||
* not the full `accessibleSkillIds`, so deactivated skills cannot be
|
||||
* invoked by name even if the LLM hallucinates them.
|
||||
*/
|
||||
activeSkillIds: Types.ObjectId[];
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -75,30 +125,62 @@ export async function injectSkillCatalog(
|
|||
contextWindowTokens,
|
||||
listSkillsByAccess,
|
||||
codeEnvAvailable,
|
||||
userId,
|
||||
skillStates,
|
||||
defaultActiveOnShare = false,
|
||||
} = params;
|
||||
|
||||
if (!listSkillsByAccess || accessibleSkillIds.length === 0) {
|
||||
return { toolDefinitions: inputDefs, skillCount: 0 };
|
||||
return { toolDefinitions: inputDefs, skillCount: 0, activeSkillIds: [] };
|
||||
}
|
||||
|
||||
const { skills } = await listSkillsByAccess({
|
||||
accessibleIds: accessibleSkillIds,
|
||||
limit: SKILL_CATALOG_LIMIT,
|
||||
});
|
||||
type SkillSummary = Awaited<ReturnType<NonNullable<typeof listSkillsByAccess>>>['skills'][number];
|
||||
|
||||
if (skills.length === SKILL_CATALOG_LIMIT) {
|
||||
const isActive = (s: SkillSummary): boolean =>
|
||||
resolveSkillActive({ skill: s, skillStates, userId, defaultActiveOnShare });
|
||||
|
||||
const activeSkills: SkillSummary[] = [];
|
||||
let cursor: string | null = null;
|
||||
let pages = 0;
|
||||
let reachedEnd = false;
|
||||
|
||||
while (activeSkills.length < SKILL_CATALOG_LIMIT && pages < MAX_CATALOG_PAGES) {
|
||||
const page = await listSkillsByAccess({
|
||||
accessibleIds: accessibleSkillIds,
|
||||
limit: CATALOG_PAGE_SIZE,
|
||||
cursor,
|
||||
});
|
||||
|
||||
for (const skill of page.skills) {
|
||||
if (activeSkills.length >= SKILL_CATALOG_LIMIT) {
|
||||
break;
|
||||
}
|
||||
if (isActive(skill)) {
|
||||
activeSkills.push(skill);
|
||||
}
|
||||
}
|
||||
|
||||
if (!page.has_more || !page.after) {
|
||||
reachedEnd = true;
|
||||
break;
|
||||
}
|
||||
cursor = page.after;
|
||||
pages += 1;
|
||||
}
|
||||
|
||||
if (activeSkills.length === 0) {
|
||||
return { toolDefinitions: inputDefs, skillCount: 0, activeSkillIds: [] };
|
||||
}
|
||||
|
||||
if (!reachedEnd && activeSkills.length < SKILL_CATALOG_LIMIT) {
|
||||
logger.warn(
|
||||
`[injectSkillCatalog] Skill catalog reached limit of ${SKILL_CATALOG_LIMIT}. Some skills may be excluded.`,
|
||||
`[injectSkillCatalog] Scanned ${MAX_CATALOG_PAGES} pages without filling the ${SKILL_CATALOG_LIMIT}-skill catalog. Some active skills may be excluded.`,
|
||||
);
|
||||
}
|
||||
|
||||
if (skills.length === 0) {
|
||||
return { toolDefinitions: inputDefs, skillCount: 0 };
|
||||
}
|
||||
|
||||
// Warn on duplicate names — model may invoke the wrong skill
|
||||
const nameCount = new Map<string, number>();
|
||||
for (const s of skills) {
|
||||
for (const s of activeSkills) {
|
||||
nameCount.set(s.name, (nameCount.get(s.name) ?? 0) + 1);
|
||||
}
|
||||
for (const [dupName, count] of nameCount) {
|
||||
|
|
@ -110,7 +192,7 @@ export async function injectSkillCatalog(
|
|||
}
|
||||
|
||||
const catalog = formatSkillCatalog(
|
||||
skills.map((s) => ({ name: s.name, description: s.description })),
|
||||
activeSkills.map((s) => ({ name: s.name, description: s.description })),
|
||||
{ contextWindowTokens: contextWindowTokens || 200_000 },
|
||||
);
|
||||
|
||||
|
|
@ -152,5 +234,9 @@ export async function injectSkillCatalog(
|
|||
}
|
||||
}
|
||||
|
||||
return { toolDefinitions, skillCount: skills.length };
|
||||
return {
|
||||
toolDefinitions,
|
||||
skillCount: activeSkills.length,
|
||||
activeSkillIds: activeSkills.map((s) => s._id),
|
||||
};
|
||||
}
|
||||
|
|
|
|||
244
packages/api/src/skills/__tests__/skillStates.test.ts
Normal file
244
packages/api/src/skills/__tests__/skillStates.test.ts
Normal file
|
|
@ -0,0 +1,244 @@
|
|||
import { Types } from 'mongoose';
|
||||
import {
|
||||
MAX_KEY_LENGTH,
|
||||
MAX_RAW_PAYLOAD,
|
||||
MAX_SKILL_STATES,
|
||||
loadSkillStates,
|
||||
pruneOrphanSkillStates,
|
||||
resolveDefaultActiveOnShare,
|
||||
toSkillStatesRecord,
|
||||
validateSkillStatesPayload,
|
||||
} from '../skillStates';
|
||||
|
||||
describe('toSkillStatesRecord', () => {
|
||||
it('converts a Mongoose Map to a plain record', () => {
|
||||
const map = new Map<string, boolean>([
|
||||
['a', true],
|
||||
['b', false],
|
||||
]);
|
||||
expect(toSkillStatesRecord(map)).toEqual({ a: true, b: false });
|
||||
});
|
||||
|
||||
it('returns a plain object unchanged', () => {
|
||||
const input = { x: true, y: false };
|
||||
expect(toSkillStatesRecord(input)).toBe(input);
|
||||
});
|
||||
|
||||
it('returns {} for null, undefined, or primitives', () => {
|
||||
expect(toSkillStatesRecord(null)).toEqual({});
|
||||
expect(toSkillStatesRecord(undefined)).toEqual({});
|
||||
expect(toSkillStatesRecord('not-an-object' as unknown as Record<string, boolean>)).toEqual({});
|
||||
});
|
||||
});
|
||||
|
||||
describe('resolveDefaultActiveOnShare', () => {
|
||||
it('returns true when config object has defaultActiveOnShare: true', () => {
|
||||
expect(resolveDefaultActiveOnShare({ defaultActiveOnShare: true })).toBe(true);
|
||||
});
|
||||
|
||||
it('returns false when defaultActiveOnShare is false, missing, or non-boolean', () => {
|
||||
expect(resolveDefaultActiveOnShare({ defaultActiveOnShare: false })).toBe(false);
|
||||
expect(resolveDefaultActiveOnShare({})).toBe(false);
|
||||
expect(resolveDefaultActiveOnShare({ defaultActiveOnShare: 'true' })).toBe(false);
|
||||
});
|
||||
|
||||
it('returns false for non-object config (boolean shorthand or missing)', () => {
|
||||
expect(resolveDefaultActiveOnShare(true)).toBe(false);
|
||||
expect(resolveDefaultActiveOnShare(null)).toBe(false);
|
||||
expect(resolveDefaultActiveOnShare(undefined)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('loadSkillStates', () => {
|
||||
it('returns the user\u2019s stored states and the admin-configured default', async () => {
|
||||
const getUserById = jest.fn().mockResolvedValue({
|
||||
skillStates: new Map([['skill-a', true]]),
|
||||
});
|
||||
const result = await loadSkillStates({
|
||||
userId: 'user-1',
|
||||
appConfig: { interfaceConfig: { skills: { defaultActiveOnShare: true } } },
|
||||
getUserById,
|
||||
});
|
||||
expect(result.skillStates).toEqual({ 'skill-a': true });
|
||||
expect(result.defaultActiveOnShare).toBe(true);
|
||||
expect(getUserById).toHaveBeenCalledWith('user-1', 'skillStates');
|
||||
});
|
||||
|
||||
it('returns an empty record when the user has no stored states', async () => {
|
||||
const getUserById = jest.fn().mockResolvedValue({ skillStates: undefined });
|
||||
const result = await loadSkillStates({
|
||||
userId: 'user-1',
|
||||
appConfig: null,
|
||||
getUserById,
|
||||
});
|
||||
expect(result.skillStates).toEqual({});
|
||||
expect(result.defaultActiveOnShare).toBe(false);
|
||||
});
|
||||
|
||||
it('handles a missing user doc gracefully', async () => {
|
||||
const getUserById = jest.fn().mockResolvedValue(null);
|
||||
const result = await loadSkillStates({
|
||||
userId: 'user-1',
|
||||
appConfig: {},
|
||||
getUserById,
|
||||
});
|
||||
expect(result.skillStates).toEqual({});
|
||||
expect(result.defaultActiveOnShare).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('validateSkillStatesPayload', () => {
|
||||
it('accepts a valid ObjectId-keyed boolean map', () => {
|
||||
const payload = {
|
||||
[new Types.ObjectId().toString()]: true,
|
||||
[new Types.ObjectId().toString()]: false,
|
||||
};
|
||||
expect(validateSkillStatesPayload(payload)).toBeNull();
|
||||
});
|
||||
|
||||
it('accepts an empty object', () => {
|
||||
expect(validateSkillStatesPayload({})).toBeNull();
|
||||
});
|
||||
|
||||
it('rejects non-object payloads', () => {
|
||||
expect(validateSkillStatesPayload(null)?.message).toMatch(/plain object/);
|
||||
expect(validateSkillStatesPayload(undefined)?.message).toMatch(/plain object/);
|
||||
expect(validateSkillStatesPayload('string')?.message).toMatch(/plain object/);
|
||||
expect(validateSkillStatesPayload([])?.message).toMatch(/plain object/);
|
||||
});
|
||||
|
||||
it('rejects payloads that exceed the raw sanity bound', () => {
|
||||
const payload: Record<string, boolean> = {};
|
||||
for (let i = 0; i < MAX_RAW_PAYLOAD + 1; i += 1) {
|
||||
payload[new Types.ObjectId().toString()] = true;
|
||||
}
|
||||
const error = validateSkillStatesPayload(payload);
|
||||
expect(error?.code).toBe('SKILL_STATES_PAYLOAD_TOO_LARGE');
|
||||
expect(error?.limit).toBe(MAX_RAW_PAYLOAD);
|
||||
});
|
||||
|
||||
it('does not reject a payload at the strict 200-cap (enforcement happens post-prune)', () => {
|
||||
const payload: Record<string, boolean> = {};
|
||||
for (let i = 0; i < MAX_SKILL_STATES + 1; i += 1) {
|
||||
payload[new Types.ObjectId().toString()] = true;
|
||||
}
|
||||
expect(validateSkillStatesPayload(payload)).toBeNull();
|
||||
});
|
||||
|
||||
it('rejects empty string keys', () => {
|
||||
expect(validateSkillStatesPayload({ '': true })?.message).toMatch(/non-empty string/);
|
||||
});
|
||||
|
||||
it('rejects overlong keys', () => {
|
||||
const long = 'a'.repeat(MAX_KEY_LENGTH + 1);
|
||||
expect(validateSkillStatesPayload({ [long]: true })?.message).toMatch(/non-empty string/);
|
||||
});
|
||||
|
||||
it('rejects keys that are not valid ObjectIds', () => {
|
||||
expect(validateSkillStatesPayload({ 'not-an-objectid': true })?.message).toMatch(
|
||||
/valid ObjectId/,
|
||||
);
|
||||
});
|
||||
|
||||
it('rejects non-boolean values', () => {
|
||||
const id = new Types.ObjectId().toString();
|
||||
expect(validateSkillStatesPayload({ [id]: 'true' })?.message).toMatch(/boolean/);
|
||||
expect(validateSkillStatesPayload({ [id]: 1 })?.message).toMatch(/boolean/);
|
||||
expect(validateSkillStatesPayload({ [id]: null })?.message).toMatch(/boolean/);
|
||||
});
|
||||
});
|
||||
|
||||
describe('pruneOrphanSkillStates', () => {
|
||||
const makeId = () => new Types.ObjectId().toString();
|
||||
|
||||
it('drops entries for skills that do not exist', async () => {
|
||||
const kept = makeId();
|
||||
const orphan = makeId();
|
||||
const pruned = await pruneOrphanSkillStates(
|
||||
{ [kept]: true, [orphan]: false },
|
||||
{
|
||||
findExistingSkillIds: async () => [kept],
|
||||
findAccessibleSkillIds: async () => [kept, orphan],
|
||||
},
|
||||
);
|
||||
expect(pruned).toEqual({ [kept]: true });
|
||||
});
|
||||
|
||||
it('drops entries whose user access was revoked (skill exists but not accessible)', async () => {
|
||||
const kept = makeId();
|
||||
const revoked = makeId();
|
||||
const pruned = await pruneOrphanSkillStates(
|
||||
{ [kept]: true, [revoked]: false },
|
||||
{
|
||||
findExistingSkillIds: async () => [kept, revoked],
|
||||
findAccessibleSkillIds: async () => [kept],
|
||||
},
|
||||
);
|
||||
expect(pruned).toEqual({ [kept]: true });
|
||||
});
|
||||
|
||||
it('drops malformed (non-ObjectId) keys before querying', async () => {
|
||||
const kept = makeId();
|
||||
const findExistingSkillIds = jest.fn().mockResolvedValue([kept]);
|
||||
const findAccessibleSkillIds = jest.fn().mockResolvedValue([kept]);
|
||||
const pruned = await pruneOrphanSkillStates(
|
||||
{ [kept]: true, 'not-an-objectid': false, '': true },
|
||||
{ findExistingSkillIds, findAccessibleSkillIds },
|
||||
);
|
||||
expect(pruned).toEqual({ [kept]: true });
|
||||
expect(findExistingSkillIds).toHaveBeenCalledWith([kept]);
|
||||
});
|
||||
|
||||
it('returns {} and skips DB calls when every key is malformed', async () => {
|
||||
const findExistingSkillIds = jest.fn();
|
||||
const findAccessibleSkillIds = jest.fn();
|
||||
const pruned = await pruneOrphanSkillStates(
|
||||
{ 'bad-1': true, 'bad-2': false },
|
||||
{ findExistingSkillIds, findAccessibleSkillIds },
|
||||
);
|
||||
expect(pruned).toEqual({});
|
||||
expect(findExistingSkillIds).not.toHaveBeenCalled();
|
||||
expect(findAccessibleSkillIds).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('issues existence and access queries in parallel (both called before either resolves)', async () => {
|
||||
const id = makeId();
|
||||
let resolveExisting: (value: string[]) => void = () => undefined;
|
||||
let resolveAccessible: (value: string[]) => void = () => undefined;
|
||||
const existingPromise = new Promise<string[]>((resolve) => {
|
||||
resolveExisting = resolve;
|
||||
});
|
||||
const accessiblePromise = new Promise<string[]>((resolve) => {
|
||||
resolveAccessible = resolve;
|
||||
});
|
||||
const findExistingSkillIds = jest.fn(() => existingPromise);
|
||||
const findAccessibleSkillIds = jest.fn(() => accessiblePromise);
|
||||
|
||||
const pending = pruneOrphanSkillStates(
|
||||
{ [id]: true },
|
||||
{ findExistingSkillIds, findAccessibleSkillIds },
|
||||
);
|
||||
|
||||
// Yield one microtask so any sequential-await implementation would have
|
||||
// invoked only the first dep. `Promise.all` invokes both synchronously.
|
||||
await Promise.resolve();
|
||||
expect(findExistingSkillIds).toHaveBeenCalledTimes(1);
|
||||
expect(findAccessibleSkillIds).toHaveBeenCalledTimes(1);
|
||||
|
||||
resolveExisting([id]);
|
||||
resolveAccessible([id]);
|
||||
await pending;
|
||||
});
|
||||
|
||||
it('accepts Types.ObjectId instances in the accessible list', async () => {
|
||||
const id = new Types.ObjectId();
|
||||
const pruned = await pruneOrphanSkillStates(
|
||||
{ [id.toString()]: true },
|
||||
{
|
||||
findExistingSkillIds: async () => [id.toString()],
|
||||
findAccessibleSkillIds: async () => [id],
|
||||
},
|
||||
);
|
||||
expect(pruned).toEqual({ [id.toString()]: true });
|
||||
});
|
||||
});
|
||||
|
|
@ -1,3 +1,4 @@
|
|||
export * from './binary';
|
||||
export * from './handlers';
|
||||
export * from './import';
|
||||
export * from './skillStates';
|
||||
|
|
|
|||
164
packages/api/src/skills/skillStates.ts
Normal file
164
packages/api/src/skills/skillStates.ts
Normal file
|
|
@ -0,0 +1,164 @@
|
|||
import { isValidObjectIdString } from '@librechat/data-schemas';
|
||||
import type { Types } from 'mongoose';
|
||||
|
||||
/** Hard cap on explicit override entries stored on a user document. */
|
||||
export const MAX_SKILL_STATES = 200;
|
||||
/** Max length of a skill-ID map key (matches ObjectId hex length with slack). */
|
||||
export const MAX_KEY_LENGTH = 64;
|
||||
/**
|
||||
* Generous upper bound on raw payload size to reject abusive inputs before
|
||||
* we spend cycles validating or querying the DB for orphan cleanup.
|
||||
*/
|
||||
export const MAX_RAW_PAYLOAD = MAX_SKILL_STATES * 2;
|
||||
|
||||
/** Map of skillId → explicit active state override. */
|
||||
export type SkillStatesRecord = Record<string, boolean>;
|
||||
|
||||
/**
|
||||
* Converts a Mongoose Map (non-lean) or plain object (lean) into a
|
||||
* `SkillStatesRecord`. Returns `{}` for any other shape.
|
||||
*/
|
||||
export function toSkillStatesRecord(
|
||||
raw: Map<string, boolean> | Record<string, boolean> | null | undefined,
|
||||
): SkillStatesRecord {
|
||||
if (raw instanceof Map) {
|
||||
return Object.fromEntries(raw);
|
||||
}
|
||||
if (raw && typeof raw === 'object') {
|
||||
return raw as SkillStatesRecord;
|
||||
}
|
||||
return {};
|
||||
}
|
||||
|
||||
/**
|
||||
* Reads `defaultActiveOnShare` out of the `interface.skills` config shape.
|
||||
* The shape is a Zod union (boolean | object) so we handle both gracefully.
|
||||
*/
|
||||
export function resolveDefaultActiveOnShare(skillsConfig: unknown): boolean {
|
||||
if (skillsConfig && typeof skillsConfig === 'object') {
|
||||
return (skillsConfig as { defaultActiveOnShare?: unknown }).defaultActiveOnShare === true;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
/** Return shape from `loadSkillStates`. */
|
||||
export interface LoadedSkillStates {
|
||||
skillStates: SkillStatesRecord;
|
||||
defaultActiveOnShare: boolean;
|
||||
}
|
||||
|
||||
export interface LoadSkillStatesParams {
|
||||
userId: string;
|
||||
appConfig?: { interfaceConfig?: { skills?: unknown } } | null;
|
||||
getUserById: (
|
||||
id: string,
|
||||
select: string,
|
||||
) => Promise<{ skillStates?: Map<string, boolean> | Record<string, boolean> } | null | undefined>;
|
||||
/**
|
||||
* When provided and empty, the helper short-circuits to defaults without
|
||||
* issuing the user query. Lets every agent entry point skip the DB round
|
||||
* trip when no skills are accessible, without re-implementing the guard.
|
||||
*/
|
||||
accessibleSkillIds?: ArrayLike<unknown>;
|
||||
}
|
||||
|
||||
/**
|
||||
* Loads a user's `skillStates` overrides and the admin-configured
|
||||
* `defaultActiveOnShare` in one call. Used by every agent entry point so the
|
||||
* same loading block is not duplicated across controllers.
|
||||
*/
|
||||
export async function loadSkillStates(params: LoadSkillStatesParams): Promise<LoadedSkillStates> {
|
||||
const { userId, appConfig, getUserById, accessibleSkillIds } = params;
|
||||
if (accessibleSkillIds !== undefined && accessibleSkillIds.length === 0) {
|
||||
return { skillStates: {}, defaultActiveOnShare: false };
|
||||
}
|
||||
const user = await getUserById(userId, 'skillStates');
|
||||
return {
|
||||
skillStates: toSkillStatesRecord(user?.skillStates),
|
||||
defaultActiveOnShare: resolveDefaultActiveOnShare(appConfig?.interfaceConfig?.skills),
|
||||
};
|
||||
}
|
||||
|
||||
export interface SkillStatesValidationError {
|
||||
code?: string;
|
||||
message: string;
|
||||
limit?: number;
|
||||
}
|
||||
|
||||
/**
|
||||
* Validates a raw skill-states update payload. Returns `null` on success or
|
||||
* a structured `SkillStatesValidationError` describing the first rejection
|
||||
* reason. Caller maps the error to an HTTP 400 response.
|
||||
*
|
||||
* Rejects: non-object payloads, oversize payloads (sanity bound for abuse),
|
||||
* non-string/empty/oversize keys, keys that are not valid ObjectIds, and
|
||||
* non-boolean values. The strict 200-entry cap is enforced *after* orphan
|
||||
* pruning, not here, so stale-client payloads near the cap do not get a
|
||||
* false-positive rejection.
|
||||
*/
|
||||
export function validateSkillStatesPayload(payload: unknown): SkillStatesValidationError | null {
|
||||
if (!payload || typeof payload !== 'object' || Array.isArray(payload)) {
|
||||
return { message: 'skillStates must be a plain object' };
|
||||
}
|
||||
const entries = Object.entries(payload as Record<string, unknown>);
|
||||
if (entries.length > MAX_RAW_PAYLOAD) {
|
||||
return {
|
||||
code: 'SKILL_STATES_PAYLOAD_TOO_LARGE',
|
||||
message: `Payload exceeds ${MAX_RAW_PAYLOAD} entries`,
|
||||
limit: MAX_RAW_PAYLOAD,
|
||||
};
|
||||
}
|
||||
for (const [key, value] of entries) {
|
||||
if (key.length === 0 || key.length > MAX_KEY_LENGTH) {
|
||||
return {
|
||||
message: `Each skill ID must be a non-empty string (max ${MAX_KEY_LENGTH} chars)`,
|
||||
};
|
||||
}
|
||||
if (!isValidObjectIdString(key)) {
|
||||
return { message: 'Each skill ID must be a valid ObjectId' };
|
||||
}
|
||||
if (typeof value !== 'boolean') {
|
||||
return { message: 'Each skill state value must be a boolean' };
|
||||
}
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
export interface PruneOrphansDeps {
|
||||
/** Returns the subset of the given IDs that exist as Skill documents. */
|
||||
findExistingSkillIds: (validIds: string[]) => Promise<string[]>;
|
||||
/** Returns IDs the current user has VIEW access to via ACL. */
|
||||
findAccessibleSkillIds: () => Promise<Array<Types.ObjectId | string>>;
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns a copy of `skillStates` containing only entries that: are valid
|
||||
* ObjectIds, point to a Skill that currently exists, AND the user still has
|
||||
* VIEW access to. Self-heals three classes of orphan without cascade logic:
|
||||
* malformed keys, deleted skills, and revoked shares.
|
||||
*
|
||||
* Deps are injected so the pure logic can be tested without Mongoose or ACL
|
||||
* wiring. Callers in `/api` adapt their live model + permission service.
|
||||
*/
|
||||
export async function pruneOrphanSkillStates(
|
||||
skillStates: SkillStatesRecord,
|
||||
deps: PruneOrphansDeps,
|
||||
): Promise<SkillStatesRecord> {
|
||||
const validIds = Object.keys(skillStates).filter((id) => isValidObjectIdString(id));
|
||||
if (validIds.length === 0) {
|
||||
return {};
|
||||
}
|
||||
const [existing, accessible] = await Promise.all([
|
||||
deps.findExistingSkillIds(validIds),
|
||||
deps.findAccessibleSkillIds(),
|
||||
]);
|
||||
const existingSet = new Set(existing);
|
||||
const accessibleSet = new Set(accessible.map((id) => id.toString()));
|
||||
const pruned: SkillStatesRecord = {};
|
||||
for (const id of validIds) {
|
||||
if (existingSet.has(id) && accessibleSet.has(id)) {
|
||||
pruned[id] = skillStates[id];
|
||||
}
|
||||
}
|
||||
return pruned;
|
||||
}
|
||||
|
|
@ -400,6 +400,9 @@ export const skillTree = ({ skillId, path = '' }: { skillId: string; path?: stri
|
|||
return url;
|
||||
};
|
||||
|
||||
/* Skill active states (per-user overrides) */
|
||||
export const skillStates = () => `${BASE_URL}/api/user/settings/skills/active`;
|
||||
|
||||
/* Roles */
|
||||
export const roles = () => `${BASE_URL}/api/roles`;
|
||||
export const adminRoles = () => `${BASE_URL}/api/admin/roles`;
|
||||
|
|
|
|||
|
|
@ -755,6 +755,7 @@ export const interfaceSchema = z
|
|||
create: z.boolean().optional(),
|
||||
share: z.boolean().optional(),
|
||||
public: z.boolean().optional(),
|
||||
defaultActiveOnShare: z.boolean().optional(),
|
||||
}),
|
||||
])
|
||||
.optional(),
|
||||
|
|
@ -808,6 +809,7 @@ export const interfaceSchema = z
|
|||
create: true,
|
||||
share: false,
|
||||
public: false,
|
||||
defaultActiveOnShare: false,
|
||||
},
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -47,6 +47,17 @@ export function updateSkillFavorites(skillFavorites: string[]): Promise<string[]
|
|||
return Promise.resolve(skillFavorites);
|
||||
}
|
||||
|
||||
/** Per-user skill active/inactive overrides. */
|
||||
export function getSkillStates(): Promise<sk.TSkillStatesResponse> {
|
||||
return request.get(endpoints.skillStates());
|
||||
}
|
||||
|
||||
export function updateSkillStates(
|
||||
skillStates: sk.TSkillStatesResponse,
|
||||
): Promise<sk.TSkillStatesResponse> {
|
||||
return request.post(endpoints.skillStates(), { skillStates });
|
||||
}
|
||||
|
||||
export function getSharedMessages(shareId: string): Promise<t.TSharedMessagesResponse> {
|
||||
return request.get(endpoints.shareMessages(shareId));
|
||||
}
|
||||
|
|
|
|||
|
|
@ -75,6 +75,8 @@ export enum QueryKeys {
|
|||
skillNodeContent = 'skillNodeContent',
|
||||
/* Skill favorites (star a skill in the sidebar) */
|
||||
skillFavorites = 'skillFavorites',
|
||||
/* Per-user skill active/inactive overrides */
|
||||
skillStates = 'skillStates',
|
||||
/* General user favorites */
|
||||
favorites = 'favorites',
|
||||
}
|
||||
|
|
|
|||
|
|
@ -259,3 +259,11 @@ export type TDeleteSkillFileVariables = {
|
|||
skillId: string;
|
||||
relativePath: string;
|
||||
};
|
||||
|
||||
/**
|
||||
* Per-user skill active/inactive overrides (GET response and POST body payload).
|
||||
* Key = skill ObjectId string, value = explicit active state.
|
||||
* Skills absent from the map use the ownership-based default:
|
||||
* owned = active, shared = `defaultActiveOnShare` from config.
|
||||
*/
|
||||
export type TSkillStatesResponse = Record<string, boolean>;
|
||||
|
|
|
|||
|
|
@ -145,6 +145,11 @@ const userSchema = new Schema<IUser>(
|
|||
],
|
||||
default: [],
|
||||
},
|
||||
skillStates: {
|
||||
type: Map,
|
||||
of: Boolean,
|
||||
default: () => new Map(),
|
||||
},
|
||||
/** Field for external source identification (for consistency with TPrincipal schema) */
|
||||
idOnTheSource: {
|
||||
type: String,
|
||||
|
|
|
|||
|
|
@ -43,6 +43,8 @@ export interface IUser extends Document {
|
|||
memories?: boolean;
|
||||
};
|
||||
favorites?: TUserFavorite[];
|
||||
/** Per-skill active/inactive overrides. Key = skillId, value = active state. */
|
||||
skillStates?: Record<string, boolean>;
|
||||
createdAt?: Date;
|
||||
updatedAt?: Date;
|
||||
/** Field for external source identification (for consistency with TPrincipal schema) */
|
||||
|
|
@ -85,6 +87,7 @@ export interface UpdateUserRequest {
|
|||
personalization?: {
|
||||
memories?: boolean;
|
||||
};
|
||||
skillStates?: Record<string, boolean>;
|
||||
}
|
||||
|
||||
export interface UserDeleteResult {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue