Draft
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Introduces a first-class Skills resource across the data schemas, API, and client data-provider layers, including ACL-aware access/sharing integration and initial CRUD + pagination + optimistic concurrency support.
Changes:
- Adds Skill/SkillFile schemas + models + CRUD methods (including cursor pagination, validation, optimistic concurrency, and cascade deletes).
- Wires new
/api/skillsroutes and typed handler factory with ACL middleware + capability gates, plus integration tests. - Extends permissions/roles/capabilities and adds client data-service + React Query hooks/types for skills.
Reviewed changes
Copilot reviewed 47 out of 47 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/data-schemas/src/types/skill.ts | Defines Skill/SkillFile TS interfaces and summary projection type. |
| packages/data-schemas/src/types/role.ts | Extends role permission typing to include SKILLS permission block. |
| packages/data-schemas/src/types/index.ts | Re-exports skill types from the package type index. |
| packages/data-schemas/src/schema/skillFile.ts | Adds SkillFile Mongoose schema with relativePath validation and indexes. |
| packages/data-schemas/src/schema/skill.ts | Adds Skill Mongoose schema with reserved-name validation, metadata fields, and indexes. |
| packages/data-schemas/src/schema/role.ts | Extends role permissions schema to include SKILLS permission flags. |
| packages/data-schemas/src/schema/accessRole.ts | Allows skill in accessRole resourceType enum. |
| packages/data-schemas/src/models/skillFile.ts | Registers SkillFile model and applies tenant isolation. |
| packages/data-schemas/src/models/skill.ts | Registers Skill model and applies tenant isolation. |
| packages/data-schemas/src/models/index.ts | Adds Skill and SkillFile to the created model set. |
| packages/data-schemas/src/methods/skill.ts | Implements skill CRUD, validation helpers, cursor pagination, optimistic concurrency, and file metadata methods. |
| packages/data-schemas/src/methods/skill.spec.ts | Adds in-memory Mongo tests covering validators, CRUD, concurrency conflicts, pagination, and fileCount/version bumping. |
| packages/data-schemas/src/methods/index.ts | Exposes skill methods from the main methods factory and re-exports skill method types. |
| packages/data-schemas/src/methods/accessRole.ts | Seeds SKILL_VIEWER/EDITOR/OWNER access roles. |
| packages/data-schemas/src/methods/accessRole.spec.ts | Updates access role tests to include skill roles. |
| packages/data-schemas/src/admin/capabilities.ts | Adds READ_SKILLS / MANAGE_SKILLS capabilities and maps SKILL resource to MANAGE_SKILLS. |
| packages/data-provider/src/types/skills.ts | Defines the wire types for skills, including list responses, conflicts, and file metadata. |
| packages/data-provider/src/types/mutations.ts | Adds skill mutation option/context types and cache entry typing. |
| packages/data-provider/src/roles.ts | Adds SKILLS permission defaults to role defaults schema and values. |
| packages/data-provider/src/roles.spec.ts | Updates role defaults tests to include SKILLS in restricted/resource-managed sets. |
| packages/data-provider/src/permissions.ts | Adds PermissionTypes.SKILLS and corresponding zod schema + mapping entry. |
| packages/data-provider/src/keys.ts | Adds query keys for skills, skill detail, and skill files. |
| packages/data-provider/src/index.ts | Re-exports skill wire types from package entrypoint. |
| packages/data-provider/src/data-service.ts | Adds dataService methods for skills CRUD and skill file list/upload/delete. |
| packages/data-provider/src/api-endpoints.ts | Adds /api/skills endpoint builders including filter query construction. |
| packages/data-provider/src/accessPermissions.ts | Adds ResourceType.SKILL and SKILL_* access roles, mapping them to permission bits. |
| packages/api/src/skills/index.ts | Exports skills handlers module. |
| packages/api/src/skills/handlers.ts | Adds typed Express handlers factory for skills CRUD, listing, and stubbed file endpoints. |
| packages/api/src/index.ts | Re-exports skills API module. |
| client/src/hooks/Sharing/useCanSharePublic.ts | Enables public-share gating for the SKILL resource type. |
| client/src/data-provider/Skills/queries.ts | Adds React Query hooks for listing skills (flat + infinite), fetching skill detail, and listing skill files. |
| client/src/data-provider/Skills/mutations.ts | Adds React Query mutations for skill create/update/delete and file upload/delete (upload is stubbed server-side). |
| client/src/data-provider/Skills/index.ts | Re-exports Skills queries/mutations. |
| client/src/data-provider/index.ts | Re-exports Skills provider module from the client data-provider index. |
| api/server/routes/skills.test.js | Adds supertest integration coverage for skills routes, ACL behavior, and optimistic concurrency. |
| api/server/routes/skills.js | Adds Express router wiring for /api/skills using typed handlers + ACL/capability middleware. |
| api/server/routes/index.js | Registers skills routes module in the server route index. |
| api/server/routes/accessPermissions.js | Enables generic permissions endpoint support for ResourceType.SKILL. |
| api/server/middleware/checkSharePublicAccess.js | Adds SKILL -> PermissionTypes.SKILLS mapping for public-share checks. |
| api/server/middleware/accessResources/index.js | Exports new canAccessSkillResource middleware. |
| api/server/middleware/accessResources/canAccessSkillResource.js | Implements skill-specific access middleware wrapper around canAccessResource. |
| api/server/index.js | Mounts /api/skills on the main server. |
| api/server/experimental.js | Mounts /api/skills on the experimental server entry. |
| api/server/controllers/UserController.spec.js | Updates mocks to include deleteUserSkills. |
| api/server/controllers/UserController.js | Deletes a user’s sole-owned skills as part of user deletion flow. |
| api/server/controllers/tests/deleteUserResourceCoverage.spec.js | Adds SKILL to the “handled resource types” coverage test. |
| api/server/controllers/tests/deleteUser.spec.js | Asserts deleteUserSkills is invoked during user deletion. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+817
to
+836
| const previous = await SkillFile.findOneAndUpdate( | ||
| { skillId: row.skillId, relativePath: row.relativePath }, | ||
| { | ||
| $set: { | ||
| skillId: row.skillId, | ||
| relativePath: row.relativePath, | ||
| file_id: row.file_id, | ||
| filename: row.filename, | ||
| filepath: row.filepath, | ||
| source: row.source, | ||
| mimeType: row.mimeType, | ||
| bytes: row.bytes, | ||
| category, | ||
| isExecutable: row.isExecutable ?? false, | ||
| author: row.author, | ||
| tenantId: row.tenantId, | ||
| }, | ||
| }, | ||
| { new: false, upsert: true }, | ||
| ).lean(); |
There was a problem hiding this comment.
upsertSkillFile() uses findOneAndUpdate(..., { new: false, upsert: true }) without runValidators: true, so schema constraints like bytes: { min: 0 }, required fields, and relativePath maxlength can be bypassed (Mongoose does not run validators on update ops by default). Add runValidators: true (and, if needed, context: 'query') so SkillFile writes can’t persist invalid metadata.
Comment on lines
+784
to
+790
| const Skill = mongoose.models.Skill as Model<ISkillDocument>; | ||
| const updateOps: Record<string, Record<string, number>> = { $inc: { version: 1 } }; | ||
| if (delta !== 0) { | ||
| updateOps.$inc.fileCount = delta; | ||
| } | ||
| await Skill.findByIdAndUpdate(skillId, updateOps); | ||
| } |
There was a problem hiding this comment.
bumpSkillVersionAndAdjustFileCount() increments fileCount via $inc without guarding against drift. If fileCount is already inconsistent (the comment above explicitly calls this out), a delete can push it negative, despite the schema intending min: 0 (and validators won’t run on this update). Consider clamping at 0 (e.g., update pipeline with $max, or recompute fileCount from SkillFile.countDocuments() when applying a negative delta).
Comment on lines
+380
to
+395
| const segments = relativePath.split('/'); | ||
| if (segments.some((s) => s === '' || s === '.' || s === '..')) { | ||
| issues.push({ | ||
| field: 'relativePath', | ||
| code: 'TRAVERSAL', | ||
| message: 'Relative path cannot contain empty segments or "." / ".."', | ||
| }); | ||
| } | ||
| if (relativePath === 'SKILL.md' || segments[0] === 'SKILL.md') { | ||
| issues.push({ | ||
| field: 'relativePath', | ||
| code: 'RESERVED', | ||
| message: 'SKILL.md is managed via the skill body, not file uploads', | ||
| }); | ||
| } | ||
| return issues; |
There was a problem hiding this comment.
validateRelativePath() claims SKILL.md is reserved, but it only rejects relativePath === 'SKILL.md' or when the first segment is SKILL.md. Paths like docs/SKILL.md currently pass validation, which doesn’t match the error message/intent. Either reject any segment named SKILL.md or update the validation + message to explicitly reserve only the top-level SKILL.md.
Comment on lines
+36
to
+47
| const segments = value.split('/'); | ||
| if (segments.some((s) => s === '' || s === '.' || s === '..')) { | ||
| return false; | ||
| } | ||
| if (value === 'SKILL.md' || segments[0] === 'SKILL.md') { | ||
| return false; | ||
| } | ||
| return true; | ||
| }, | ||
| message: | ||
| 'Relative path must be relative, cannot contain ".." or absolute paths, and cannot be SKILL.md.', | ||
| }, |
There was a problem hiding this comment.
The SkillFile schema validator only blocks SKILL.md at the root/first segment (value === 'SKILL.md' || segments[0] === 'SKILL.md'), so docs/SKILL.md is still allowed even though the validation message says it cannot be SKILL.md. Align the check with the intended rule (reject any segment named SKILL.md, or clarify the message to “top-level SKILL.md”).
Comment on lines
+331
to
+346
|
|
||
| const authorId = (user._id ?? user.id) as unknown as Types.ObjectId; | ||
| const authorName = user.name ?? user.username ?? 'Unknown'; | ||
|
|
||
| let createResult: CreateSkillResult; | ||
| try { | ||
| createResult = await createSkill({ | ||
| name: body.name, | ||
| displayTitle: body.displayTitle, | ||
| description: body.description, | ||
| body: body.body, | ||
| frontmatter: body.frontmatter as Record<string, unknown> | undefined, | ||
| category: body.category, | ||
| author: authorId, | ||
| authorName, | ||
| tenantId: user.tenantId, |
There was a problem hiding this comment.
authorId is derived as (user._id ?? user.id) as Types.ObjectId. If req.user._id is absent (some auth flows/tests only set id as a string), this will pass a string where the Skill model expects an ObjectId and can lead to cast errors or incorrectly-typed writes. Prefer validating user.id and constructing new Types.ObjectId(user.id) (or returning 400 if it’s not a valid ObjectId) instead of an unchecked cast.
danny-avila
force-pushed
the
feat/agent-skills
branch
from
8515ac7 to
2f9df1f
Compare
Contributor
GitNexus: 🚀 deployedThe |
1 task
danny-avila
force-pushed
the
feat/agent-skills
branch
from
c11a723 to
f224bad
Compare
Contributor
GitNexus: 🚀 deployedThe |
Contributor
GitNexus: 🚀 deployedThe |
Contributor
GitNexus: 🚀 deployedThe |
Contributor
GitNexus: 🚀 deployedThe |
Contributor
GitNexus: 🚀 deployedThe |
danny-avila
added a commit
that referenced
this pull request
Apr 17, 2026
Phase 4 of Agent Skills umbrella (#12625): gate bash_tool and skill-file priming on the execute_code capability only. LIBRECHAT_CODE_API_KEY is the LibreChat-hosted sandbox service key — system-level, not a per-user secret — so the per-user loadAuthValues lookup was legacy plumbing. The agents library reads process.env[EnvVar.CODE_API_KEY] itself; this change removes the redundant resolution paths.
This was referenced Apr 17, 2026
* 🧬 feat: Scaffold Skills CRUD with ACL Sharing and File Schema
Adds Skills as a new first-class resource modeled on Anthropic's Agent
Skills, reusing the existing Prompt ACL stack for sharing. Lays the
groundwork for multi-file skills (SkillFile schema + metadata routes)
without wiring upload processing — single-file skills (inline SKILL.md
body) work end-to-end, multi-file uploads are stubbed for phase 2.
* 🔬 fix: Wire Skill Cleanup, AccessRole Enum, and Express 5 Path Params
CI surfaced four follow-ups from the initial Skills scaffolding commit
that local builds missed:
- AccessRole's resourceType field had a hardcoded enum that didn't
include `'skill'`, blocking SKILL_OWNER/EDITOR/VIEWER role creation
in every test that hit the AccessRole model.
- The seedDefaultRoles assertion in accessRole.spec.ts hard-listed the
expected role IDs and needed the new SKILL_* entries.
- deleteUserController had no cleanup for skills, and the
deleteUserResourceCoverage guard test enforces every ResourceType has
a documented handler — wired in db.deleteUserSkills(user._id) and
added the entry to HANDLED_RESOURCE_TYPES.
- Express 5's path-to-regexp v6 rejects the legacy `(*)` named-group
glob syntax. The two skill file routes now use a plain `:relativePath`
param; the client already encodeURIComponents the path, so a single
param is sufficient and decoded server-side.
* 🪡 fix: Make Skill Name Uniqueness Application-Level
Resolve three more CI failures from the Skills scaffolding PR:
- Mongoose creates indexes asynchronously and mongodb-memory-server
tests can race ahead of the unique (name, author, tenantId) index
being built, so the duplicate-name uniqueness test was flaky.
Added an explicit findOne pre-check inside createSkill that throws
with code 11000 (mimicking the index violation), giving deterministic
behavior. The unique index stays as the persistent guarantee.
- The deleteUser.spec.js and UserController.spec.js suites mock the
~/models module directly and were missing deleteUserSkills, causing
deleteUserController to throw and return 500 instead of 200.
- Removed two doc-comment claims that the SKILL_NAME_MAX_LENGTH and
SKILL_DESCRIPTION_MAX_LENGTH constants "match Anthropic's API". The
values themselves are reasonable but the comments were misleading
about who enforces them.
* 🪢 fix: Address Code Review Findings on Skills Scaffolding
Resolve all 15 findings from the comprehensive PR review:
Critical:
- Rollback the created skill when grantPermission throws so a transient
ACL failure cannot leave an orphaned, inaccessible skill in the DB.
- Fix infinite query cache corruption in useUpdateSkillMutation helpers.
setQueriesData([QueryKeys.skills]) matches useSkillsInfiniteQuery's
InfiniteData cache entries, which have { pages, pageParams } shape —
spreading data.skills on those would throw. Added an isInfiniteSkillData
guard and per-page transform so both flat and infinite caches update
correctly.
Major:
- Fix TUpdateSkillContext type: the public type declared previousListData
but onMutate actually returns previousListSnapshots (a [key, value]
tuple array). Updated the type + added TSkillCacheEntry as a shared
export from data-provider.
- Add cancelQueries calls before optimistic update in onMutate so
in-flight refetches cannot clobber the optimistic state.
- Parallelize deleteUserSkills ACL removal via Promise.allSettled instead
of a sequential await loop — O(1) round-trip vs O(n).
- Stub mockDeleteUserSkills in stubDeletionMocks() and assert it's called
with user.id in the deleteUser.spec.js happy-path test.
- Add idResolver: getSkillById to the SKILL branch in accessPermissions.js
so GET /api/permissions/skill/<missing-id> returns 404 instead of 403.
Minor:
- Reuse resolved skill from req.resourceAccess.resourceInfo in getHandler
to eliminate a redundant getSkillById call per GET /api/skills/:id.
- Reject PATCH /api/skills/:id requests whose body contains only
expectedVersion — previously they silently bumped version with no
changes, triggering spurious 409s for collaborators.
- Make TSkill.frontmatter optional (wire type) and add serializeFrontmatter
/ serializeSourceMetadata helpers that return undefined for empty
objects instead of casting incomplete data to SkillFrontmatter.
- Standardize deleteUserSkills to accept string | ObjectId and convert
internally, matching deleteUserPrompts's signature; UserController now
passes user.id consistently.
- Replace bumpSkillVersionAndRecount (read-then-write, racy) with
bumpSkillVersionAndAdjustFileCount using atomic $inc. upsertSkillFile
pre-checks existence to distinguish insert (+1) from replace (0).
- Add DELETE /api/skills/:id/files/:relativePath integration tests
covering success, 404, and 403 paths.
Nits:
- Drop trivial resolveSkillId wrapper — pass getSkillById directly.
- Remove dead staleTime: 1000 * 10 from useListSkillsQuery since all
refetch triggers are already disabled.
* 🧭 fix: Resolve Second Skills Review Pass — Cache, Gate, TOCTOU
Address 13 of 14 findings from the second code review; reject #13 as
misread of the AGENTS.md import-order rule (package types correctly
precede local types regardless of length).
Major:
- Fix addSkillToCachedLists closure bug: a hoisted `prepended` flag
was shared across every cache entry matched by setQueriesData, so
concurrent flat + infinite caches would silently drop the prepend
on whichever was processed second. Replaced the shared helper with
three per-entry inline updaters that handle InfiniteData at the
page level (page 0 only for prepend, all pages for replace/remove).
- Tighten patchHandler's expectedVersion validation: NaN passes
`typeof === 'number'` and would previously leak current skill state
via a misleading 409. Now requires finite positive integer and
returns 400 otherwise.
- Guard decodeURIComponent in deleteFileHandler with try/catch —
malformed percent encoding now returns 400 instead of 500.
- Add PermissionTypes.SKILLS + skillPermissionsSchema +
TSkillPermissions in data-provider; seed default SKILLS permissions
for ADMIN (all true) and USER (use + create only); wire
checkSkillAccess / checkSkillCreate via generateCheckAccess onto
the skills router mirroring the prompts pattern. Skills route now
enforces role-based capability gates alongside per-resource ACLs.
Test suite adds a mocked getRoleByName returning permissive SKILLS.
- Fix upsertSkillFile TOCTOU: replaced the pre-check + upsert pair
with a single `findOneAndUpdate({ new: false, upsert: true })` call
that atomically returns the pre-update doc (null ⇒ insert) so
fileCount delta can't double-count on concurrent same-path uploads.
Minor:
- Add `sourceMetadata` to listSkillsByAccess .select() so summaries
no longer silently drop the field for GitHub/Notion-synced skills.
- Include `cursor` in useListSkillsQuery's query key so manual
pagination doesn't alias across pages.
- Clean up TSkillSummary to `Omit<TSkill, 'body' | 'frontmatter'>`
matching what serializeSkillSummary actually emits; drop the
Omit-then-re-add noise.
- Skip getPublicSkillIdSet in createHandler; a newly-created skill
cannot have a PUBLIC ACL entry, so pass an empty set directly
instead of paying a DB round-trip.
- Trim SkillMethods public surface: drop internal helpers
countSkillFiles / deleteSkillFilesBySkillId / getSkillFile from the
return object; inline the file cascade into deleteSkill.
- Use TSkillConflictResponse at the PATCH 409 call site instead of
an inline ad-hoc object literal.
- Drop the now-unused EXPECTED_VERSION_ERROR module constant.
* 🧩 fix: Extend Role Schema + Types with SKILLS PermissionType
CI type-check and unit test failures from the PermissionTypes.SKILLS
addition surfaced three unrelated places that all hardcode the
permission-type set:
- IRole.permissions in data-schemas/types/role.ts enumerates every
PermissionTypes key as an optional field. Adding SKILLS to the enum
without updating the interface caused TS7053 'expression of type
PermissionTypes can't be used to index type' errors in
role.methods.spec.ts (lines 407-408, 477-478) because
Object.values(PermissionTypes) now yielded a value the interface
didn't cover.
- schema/role.ts rolePermissionsSchema mirrors the interface at the
Mongoose layer; also needed SKILLS added so the persisted role
document can actually store skill permissions.
- data-provider/roles.spec.ts has a guard test that every permission
type carrying CREATE/SHARE/SHARE_PUBLIC must be explicitly "tracked"
either in RESOURCE_PERMISSION_TYPES or in the PROMPTS/AGENTS/MEMORIES
exemption list. Added SKILLS to the exemption list since skills
follow the same default model as prompts/agents (USE + CREATE on for
USER, SHARE / SHARE_PUBLIC off).
All three are additive pass-throughs with no behavior change.
* 🏷️ refactor: Introduce ISkillSummary for Narrow List Projection
Follow-up NITs from the second review pass on the Skills PR:
- Define ISkillSummary = Omit<ISkill, 'body' | 'frontmatter'> and use
it as the element type in ListSkillsByAccessResult. The list query's
.select() intentionally omits body and frontmatter for payload size,
but the previous type claimed both fields were present — a type lie
that would mislead future readers even though serializeSkillSummary
never touches those fields at runtime. handlers.ts's signature for
serializeSkillSummary now accepts ISkillSummary too.
- Document the intentional second-round-trip `findOne` in
upsertSkillFile. Switching to `findOneAndUpdate({ new: false })`
was required for TOCTOU-safe insert-vs-replace detection, which
means the handler needs a follow-up query to return the post-upsert
document. A comment now explains the tradeoff so future readers
don't silently "optimize" it away.
No behavior change.
* 🌐 fix: Wire SKILL into SHARE_PUBLIC Resource Maps
Address codex comment #1 — making a skill public was blocked on two
hardcoded resource→permission-type maps that didn't know about SKILL:
- api/server/middleware/checkSharePublicAccess.js's
resourceToPermissionType map was missing ResourceType.SKILL, so
PUT /api/permissions/skill/:id with { public: true } would fall
through to the 400 "Unsupported resource type for public sharing"
path even though PermissionTypes.SKILLS exists and ADMIN has
SHARE_PUBLIC configured. Added the mapping.
- client/src/hooks/Sharing/useCanSharePublic.ts has an identical
client-side map used to gate the "Make Public" UI toggle. Without
the SKILL mapping the hook returned false for everyone, so the
toggle wouldn't render for skills once the sharing UI lands in
phase 2. Added the mapping.
Codex comment #2 (create/update cache writes inject skills into
unrelated filtered lists) is invalid — it flags a pattern that
mirrors useUpdatePromptGroup (which the PR description explicitly
cites as the model) and is a deliberate optimistic-update tradeoff.
Trying to match each cache key's embedded filter would couple the
mutation callback to query-key internals, which is exactly what
setQueriesData is designed to avoid. No change there.
* 🧪 feat: Frontmatter Validation, Reserved-Name Fixes, Coaching Warnings
Address the follow-up review notes on the Skills PR. This commit closes
the gap between the wire-type promise and what the backend actually
enforces, tightens the reserved-name rules, and adds a non-blocking
coaching tier for validators.
Frontmatter validation (new):
- Add `validateSkillFrontmatter` in data-schemas/methods/skill.ts with
strict mode — unknown keys are rejected so expanding the allowed set
is an intentional code change. Known keys are type-checked against a
`FrontmatterKind` table derived from Anthropic's Agent Skills spec
(name, description, when-to-use, allowed-tools, arguments,
argument-hint, user-invocable, disable-model-invocation, model,
effort, context, agent, paths, shell, hooks, version, metadata).
- `hooks` and `metadata` get a shallow JSON-safety check (max depth 4,
max string 2000, max array 100) instead of a full schema, since their
full shapes live outside this module.
- Wired into BOTH createSkill AND updateSkill so the PATCH path can't
smuggle invalid frontmatter past the validator.
Validation warning tier (new):
- Add optional `severity: 'error' | 'warning'` to `ValidationIssue`
(defaults to error). `partitionIssues` splits an issue list into
blocking errors and non-blocking warnings.
- `createSkill` / `updateSkill` filter on errors for the throw check
and return warnings in a new `warnings: ValidationIssue[]` field on
their result objects (`CreateSkillResult` / `UpdateSkillResult`).
- `validateSkillDescription` now emits a `TOO_SHORT` warning for
descriptions under 20 chars — the primary triggering field, so a
little coaching goes a long way.
- `createHandler` / `patchHandler` in packages/api surface the warnings
via a new `attachWarnings` helper that decorates the serialized
response with a `warnings?: TSkillWarning[]` field.
- `TSkill` gains an optional `warnings?: TSkillWarning[]` field
documented as "present on POST/PATCH, never on GET".
Reserved-name filter (tightened):
- Replace the substring match (`.includes('anthropic')`) with prefix
matching on `anthropic-` and `claude-` plus exact-match rejection of
CLI slash-command collisions (help, clear, compact, model, exit,
quit, settings, plus the bare `anthropic` / `claude` words). Both
the pure validator (`methods/skill.ts`) and the Mongoose schema
validator (`schema/skill.ts`) updated in lockstep; comments on
each reference the other to prevent drift.
- `research-anthropic-helper` and `about-claude` are now allowed;
`anthropic-helper`, `claude-bot`, and `settings` are still rejected.
Documentation:
- Add docstrings on `ISkill`, `schema/skill.ts`, and `TSkill` explaining
the semantics of `name` (Claude-visible identifier, kebab-case,
stable), `displayTitle` (UI-only cosmetic label, NOT sent to Claude),
`description` (highest-leverage trigger field), and `source` /
`sourceMetadata` (reserved for phase 2+ external sync).
- Add a detailed consistency comment on `bumpSkillVersionAndAdjustFileCount`
explaining that it runs as a separate MongoDB operation from
upsertSkillFile/deleteSkillFile, so `fileCount` can drift if the
second op fails — options listed, tradeoff documented, phase 1
risk window noted as closed because upload is still stubbed.
Tests:
- data-schemas skill.spec.ts: destructure `{ skill, warnings }` from
createSkill at every call site; add a TOO_SHORT warning test, a
frontmatter strict-mode test, reserved-prefix tests (including
positive cases for substring names that should pass), CLI reserved
word tests, and a full `validateSkillFrontmatter` describe block
covering unknown keys, type mismatches, and deep-nesting rejection.
- api/server/routes/skills.test.js: bump default test description
above the 20-char threshold, add a warning-emission test, add
reserved-prefix + reserved-CLI-word tests, add an unknown-frontmatter-
key test asserting the 400 response carries `issues` with `UNKNOWN_KEY`.
* 📦 fix: Export CreateSkillResult from data-schemas Methods Index
`CreateSkillResult` was defined in `methods/skill.ts` and consumed by
`packages/api/src/skills/handlers.ts` but never re-exported from the
methods barrel, so the type-check job failed with TS2724
"'@librechat/data-schemas' has no exported member named 'CreateSkillResult'".
Rollup's bundle-mode build picked up the type via its internal resolver,
but the standalone `tsc --noEmit` type-check ran against the package's
public entrypoint and couldn't see it. Added the type import + export
alongside the existing `UpdateSkillResult` export, which fixes the
CI type-check without any runtime change.
* 🎨 feat: Skills UI — Create/Edit/Share/List with Conditional File Tree
First-pass UI on top of the CRUD API scaffolding (#12613). Ships the full
user-facing flow for inline, single-SKILL.md skills and leaves a clean
drop-in for phase-2 multi-file support.
- Create a skill from /skills/new with name (kebab-case, validated),
description, and SKILL.md body — wired to the real `useCreateSkillMutation`
and `TCreateSkill` payload.
- List skills in a sidebar (SkillsSidePanel) via `useListSkillsQuery` with
live search filtering.
- Edit any skill the caller has EDIT permission on — `useUpdateSkillMutation`
passes `expectedVersion` for optimistic concurrency and surfaces 409
conflicts as a warning toast + cache refetch.
- Non-blocking `TSkillWarning[]` (e.g. "description too short") are shown
inline above the form after a successful create/patch.
- Read-only mode when the current user lacks EDIT — the form still renders
but inputs are marked `readOnly` and the save/reset buttons are hidden.
- Share via ACL using the existing `GenericGrantAccessDialog` — the
`ShareSkill` button is gated on the SHARE permission.
- Delete with confirmation, driven by `useDeleteSkillMutation({ id })`.
- Conditional file tree: only rendered when `useListSkillFilesQuery`
returns > 0 files. The tree groups flat `relativePath` strings into a
nested view (no `react-arborist` dependency) and supports per-file
deletion via `useDeleteSkillFileMutation`. Upload is intentionally
deferred — the backend stubs it at 501 in phase 1.
- New routes: `/skills`, `/skills/new`, `/skills/:skillId`.
- Sidebar accordion (`SkillsAccordion` wrapping `SkillsSidePanel`) added
to `useSideNavLinks` gated on `PermissionTypes.SKILLS` USE.
The initial UI branch (#12580) shipped a lot of exploration code on top of
a now-superseded placeholder backend. Kept as complementary: the `Skills/`
component tree, translation keys, role descriptions, `PublicSharingToggle`
SKILL mapping, `resources.ts` SKILL config, `useCanSharePublic` SKILL
mapping, and `data-provider/roles.ts` `useUpdateSkillPermissionsMutation`.
Deferred out of this first pass:
- Skill favorites (`useSkillFavorites`, `getSkillFavorites` endpoint) —
the backend route doesn't exist yet; saving for a follow-up.
- AgentConfig `SkillSelectDialog` integration — the UI branch had this
gated behind `false &&`; rolled back with the config.
- `InvocationMode` / `CategorySelector` / `parseSkillMd` / tree-node
mutations — not in the Anthropic skill spec and not in the CRUD API.
- `react-arborist` dependency — replaced with a hand-rolled recursive
tree built from flat `TSkillFile[]`.
- 38 data-schemas skill model tests: pass
- 25 api skill route tests: pass
- 16 user-controller cleanup tests: pass
* 🔐 feat: Default-On Skills in Interface Config and Role Seeder
The skills accordion was registered in the side nav gated on
`PermissionTypes.SKILLS` USE, but no one was actually seeding that
permission on startup, so a fresh install had the USER role with
zero skill permissions and the accordion never rendered.
Fixes three gaps:
1. `interfaceSchema` in data-provider's `config.ts` had no `skills`
field at all. Added it alongside the existing agents/prompts shape
(boolean | { use, create, share, public }) and a default of
`{ use: true, create: true, share: false, public: false }`.
2. `loadDefaultInterface` in data-schemas passed every interface key
through to the loaded config EXCEPT `skills`. Added the one-line
passthrough so `appConfig.interfaceConfig.skills` is actually
populated on boot.
3. `updateInterfacePermissions` in packages/api/src/app/permissions.ts
seeds role permissions from the interface config on every restart.
Added:
- `SKILLS` case to `hasExplicitConfig`
- `skillsDefaultUse/Create/Share/Public` extraction (mirrors
prompts/agents)
- `PermissionTypes.SKILLS` block in `allPermissions` that falls
through config → roleDefaults → schema default, same pattern as
AGENTS and PROMPTS
- `SKILLS` entry in the share-backfill array so that pre-existing
SKILL role docs missing SHARE/SHARE_PUBLIC get them filled on the
next restart
Test expectations updated: seven `expectedPermissionsFor(User|Admin)`
blocks in `permissions.spec.ts` now include SKILLS, matching the
role-default values (USER: use+create true, share/public false;
ADMIN: all true).
Result: on a fresh install, a regular USER gets skill USE/CREATE
and the "Skills" accordion shows up in the chat side panel without
any yaml config. Admins can lock it down per role or per tenant via
`interface.skills` in librechat.yaml.
Tests:
- 34 packages/api permissions.spec.ts: pass
- 151 packages/api app tests: pass
- 38 data-schemas skill.spec.ts: pass
- 928 data-provider tests: pass
- 25 api skills.test.js: pass
* ♻️ fix: Resolve Skills UI Review Findings
Addresses the 13 findings from the PR review against the prior commit.
1. **canEdit consistency** — extracted `useSkillPermissions(skill)` as the
single source of truth for owner/admin/ACL gating. `SkillsView`,
`SkillForm`, `ShareSkill` all consume it; `SkillFileTree`'s per-file
delete button now honors admin + EDIT-bit permissions instead of just
ownership. Unit tests cover owner, admin, editor-ACL, viewer-ACL,
owner-ACL, loading, and undefined-skill cases.
2. **Disabled submit buttons** — create/edit form submit buttons now set
native `disabled` (not just `aria-disabled`) during `isLoading`.
`onSubmit` also guards with an early return when the mutation is still
in-flight so a duplicate enter-key submit can't create two skills.
3. **Wrong maxLength error message** — description/name `maxLength` rules
no longer re-use `com_ui_skill_*_required`. Added dedicated
`com_ui_skill_name_too_long` and `com_ui_skill_description_too_long`
keys with the literal limit interpolated (`{{0}}`).
4. **Search debouncing** — `SkillsSidePanel` now threads the filter input
through the existing `useDebounce` hook (250ms) so typing "skills" no
longer fires six separate list queries.
5. **Frontend test coverage** — added:
- `tree.test.ts` (9 tests) covering `buildTree` / `nodeKey` edge cases:
empty input, single root file, multiple roots, nested folders,
deeply-nested trees, lexicographic sort, empty paths, stable keys
- `useSkillPermissions.test.ts` (7 tests) covering every precedence
branch (owner / admin / EDIT / VIEW / owner-ACL / loading / undef)
Form integration tests proved flaky against react-hook-form's async
`isValid` with our jest-dom mock setup; deferred to a follow-up PR
with a proper `@librechat/client` test harness.
6. **Shared `SKILL_NAME_PATTERN`** — promoted the regex plus the four
length constants (`SKILL_NAME_MAX_LENGTH`,
`SKILL_DESCRIPTION_MAX_LENGTH`, `SKILL_DESCRIPTION_SHORT_THRESHOLD`,
`SKILL_DISPLAY_TITLE_MAX_LENGTH`, `SKILL_BODY_MAX_LENGTH`) out of
`packages/data-schemas/src/methods/skill.ts` and into
`packages/data-provider/src/types/skills.ts`. The data-schemas
module now aliases the shared exports so the backend validator and
the frontend form share one source of truth. Also fixed a latent bug:
the client regex was stricter than the backend
(`^[a-z0-9]+(?:-[a-z0-9]+)*$` vs. the real `^[a-z0-9][a-z0-9-]*$`),
which would have rejected valid names like `foo--bar` client-side.
7. **Removed hardcoded "Claude"** — replaced `com_ui_skill_description_help`
("Claude uses this to...") with a new `com_ui_skill_create_subtitle`
for the form header and `com_ui_skill_description_field_hint`
("This is what the model reads to decide...") for the inline hint.
LibreChat is LLM-agnostic; the old copy misled GPT/Gemini users.
8. **Lifted tree mutation hook** — `useDeleteSkillFileMutation` is now
instantiated once in `SkillFileTree` (not per `TreeRow`). A
`TreeContext` provides `onDeleteFile` + `isDeleting` + `canEdit` to
rows. A 60-node tree used to instantiate 60 mutation hooks; it now
instantiates one.
9. **List O(n) re-render** — `SkillListItem` no longer reads
`useParams()` directly. `SkillList` reads the active id once and
passes `isActive` as a prop, so navigation only re-renders the two
items whose `isActive` flipped (memo'd), not all N items.
10. **Deduped help text** — the field-level hint and form-level subtitle
now use different translation keys with distinct copy instead of
showing the same sentence twice on the same page.
11. **Removed ineffective `useCallback`** — `DeleteSkill.handleDelete`,
`CreateSkillForm.onSubmit` / `.handleCancel`, `SkillForm.onSubmit`,
and `SkillFileTree.handleDeleteFile` all wrapped closures around
React Query `mutation` refs, whose identities change every render.
Their dep arrays invalidated every render, making the memo a no-op
with extra overhead. `SkillFileTree` now destructures the stable
`mutate` function and inlines the arrow inside the memoized
`contextValue` — one stable reference per deps change.
12. **Import order** — fixed shortest→longest package ordering and
longest→shortest local ordering across all touched skill files per
AGENTS.md. `react` always first where imported.
13. **Memoization principle** — documented the rule with inline comments:
`memo` on components that appear in repeated contexts (`TreeRow`,
`SkillListItem`) or as children of frequently-re-rendering parents
(`ShareSkill` / `DeleteSkill` under `SkillForm`'s per-keystroke
form-state updates). Removed `memo` from `SkillFileTree` since its
parent `SkillDetailPanel` only re-renders on query-data changes.
- 38 data-schemas skill.spec.ts
- 34 packages/api permissions.spec.ts
- 25 api skills.test.js
- 16 client unit tests (9 buildTree + 7 useSkillPermissions)
- All type-checks + eslint clean on touched files
* 🧹 fix: Skills Duplication, Input Styling, Remove LLM-specific Copy
Three UI fixes from an in-chat review pass:
1. **Sidebar duplication** — `SkillsView` was rendering its own
`SkillsSidePanel` aside alongside the chat side panel's
`SkillsAccordion`, so on `/skills` the user saw the skill list
twice. Fixed by mirroring the `InlinePromptsView` pattern: the
route content is now just the detail / create panel and the
chat side panel is the sole list. Added `/skills → /skills/new`
redirect and a `/skills/new` literal route so `useParams().skillId`
is `undefined` for "new" (matches prompts).
2. **Name Input styling** — the big floating-label pattern used by
prompts/agents for the primary name field was replaced with a
conventional `<Label>` + `<Input>` above it, diverging from the
rest of the app. Restored the prompts-style `text-2xl` input with
the peer-focus animated label on both `CreateSkillForm` and
`SkillForm`. Kept the conventional pattern for description and
body since they're textareas.
3. **Remove LLM-specific copy from skill translations** — dropped
`com_ui_skill_description_help` ("Claude uses this to...") and
the transitional "This is what the model reads..." phrasing.
Field hint is now a neutral "Be specific about when this skill
should apply." and the create-page subtitle is a neutral "Author
a new skill your agents can invoke." LibreChat is LLM-agnostic;
baking product names into user-facing copy is wrong outside the
`com_endpoint_anthropic_*` keys where the setting actually only
applies to Claude models.
Side-effect: the `SkillDetailView` wrapper in `SkillsView` now only
renders the file-tree aside when the skill has > 0 files — same
conditional-tree behavior as before, just scoped to this route
instead of also trying to also render a list sidebar.
- 16 client skill tests still pass
- Type-check + eslint clean on touched files
* 🎁 feat: Restore Skills UI from PR #12580
Brings back everything the original UI PR (#12580, commit da039917c)
shipped that my earlier rebase dropped. Verbatim restores where possible;
adapts the new hooks/types where the backend contract has shifted.
**Scoped-out / gated-off (now restored as inert UI scaffolding):**
- `hooks/useSkillFavorites.ts` + `utils/favoritesError.ts` + the
`useGetSkillFavoritesQuery` / `useUpdateSkillFavoritesMutation` additions
in `data-provider/Favorites.ts`. The backend route doesn't exist yet —
the data-service functions resolve with empty arrays so the Star UI is a
visual-only no-op until phase 2.
- `dialogs/SkillSelectDialog.tsx` + the "Add Skills" section in
`SidePanel/Agents/AgentConfig.tsx` (still gated behind the original
`false &&`) + `skills?: string[]` on `AgentForm` / `Agent` /
`AgentCreateParams` / `AgentUpdateParams` + the `skills: []` entry in
`defaultAgentFormValues`.
- `TUserFavorite.skillId` reserved on the shared favorites type.
**Concept-is-gone / deleted-types (restored as UI-only types + stubs):**
- `InvocationMode` enum and `TSkillNode`, `TSkillTreeResponse`,
`TCreateSkillNodeRequest`, `TUpdateSkillNodeRequest` types in
`packages/data-provider/src/types.ts`. UI-facing only; the backend flat
`TSkillFile[]` contract is unchanged.
- `TSkill.invocationMode?: InvocationMode` as an optional field. Forms
read/write it in local state and deliberately drop it from the PATCH
payload until the backend column lands.
- `tree/SkillFileTree.tsx` (`react-arborist`-based), `SkillTreeNode.tsx`,
`TreeToolbar.tsx`, `SkillFileEditor.tsx`, `SkillFilePreview.tsx` — full
filesystem-style browser UI restored verbatim.
- `data-provider/Skills/tree-queries.ts` + `tree-mutations.ts` hooks
(`useGetSkillTreeQuery`, `useCreateSkillNodeMutation`, etc.). The
`data-service` stubs them: `getSkillTree` returns `{ nodes: [] }`,
`createSkillNode` / `updateSkillNode` / `updateSkillNodeContent` return
synthetic node shapes, `deleteSkillNode` resolves void. Hooks compile
and run; tree is empty until phase 2 wires a real backend.
- `MutationKeys.createSkillNode` / `updateSkillNode` / `deleteSkillNode` /
`updateSkillNodeContent` + `CreateSkillNodeBody` /
`UpdateSkillNodeVariables` / `DeleteSkillNodeBody` /
`UpdateSkillNodeContentVariables` types.
- `QueryKeys.skillTree` / `skillNodeContent` / `skillFavorites` /
`favorites` and the `skillTree()` endpoint helper.
**Scope-simplified (restored with minimal adaptation):**
- `display/SkillDetailHeader.tsx` + `display/SkillDetail.tsx`. Header now
falls back to `InvocationMode.auto` when `skill.invocationMode` is
undefined.
- `forms/SkillContentEditor.tsx` — click-to-edit markdown preview toggle
for the SKILL.md body field. Wired into both `CreateSkillForm` and
`SkillForm` replacing the plain `<TextareaAutosize>`.
(Needed `@ts-ignore` on `remarkPlugins` / `rehypePlugins` for the same
`PluggableList` vs `Pluggable[]` shape drift `MarkdownLite.tsx` already
works around.)
- `forms/InvocationModePicker.tsx` + `forms/CategorySelector.tsx` — the
auto/manual/both dropdown and the skill category selector. Wired into
both forms inside a `FormProvider` so the Controller-based widgets can
read `useFormContext`. `category` flows to the PATCH / POST payload as
before; `invocationMode` is UI-only per the type note above.
- `buttons/CreateSkillMenu.tsx` + `utils/parseSkillMd.ts` — dropdown with
AI / Manual / Upload SKILL.md entries + the YAML frontmatter parser for
the upload path. `CreateSkillForm.defaultValues` now accepts the parsed
shape, so the upload → redirect → pre-populated form flow works again.
- `buttons/AdminSettings.tsx` — admin permissions dialog. Uses the
existing `useUpdateSkillPermissionsMutation` which was already wired.
- `sidebar/FilterSkills.tsx` — restored filter + AdminSettings +
CreateSkillMenu wrapper. `SkillsSidePanel.tsx` is back to the original
`FilterSkills`-based layout.
- `lists/SkillList.tsx` + `lists/SkillListItem.tsx` — restored verbatim.
- `layouts/SkillsView.tsx` — restored the full tree + file editor + file
preview layout. The chat side panel keeps its own accordion list; this
view is the inline detail experience.
- `hooks/Generic/useUnsavedChangesPrompt.ts` — route-leave guard hook.
- `useGetSkillByIdQuery` is aliased to `useGetSkillQuery` so restored
components (`SkillsView`, `SkillForm`) that import the old name resolve
to the new hook.
- `SkillSelectDialog` + `AgentConfig` coerce `skillsData?.skills` instead
of `.data` (list response shape drift from the CRUD PR).
- `CreateSkillForm` / `SkillForm` wrap their JSX in `FormProvider` so the
restored `CategorySelector` and `SkillContentEditor` components —
which read `useFormContext` — work inside the existing forms without
another refactor.
- `CreateSkillForm.defaultValues` prop accepts `Partial<Values> &
{ invocationMode?: unknown }` so the upload flow's
`{ name, description, invocationMode }` shape passes through cleanly.
- `SkillsView` route map gains `/skills/:skillId/edit` and
`/skills/:skillId/file/:nodeId` so the tree-navigation URLs the original
view produces actually resolve.
- `client/package.json` gains `react-arborist@^3.4.3`.
- ~60 translation keys the restored files reference — invocation labels,
edit/create page titles, file editor chrome, tree toolbar tooltips,
favorites, admin allow-settings, unknown-file-type, sr_public_skill,
delete/rename _var variants — all added to `en/translation.json`.
- Prompts-style floating-label name input — kept from my earlier commit
so it matches the rest of the app (user reviewed and approved that
styling). Hidden skill-body textarea is replaced by `SkillContentEditor`
in both forms.
- 38 data-schemas skill.spec.ts
- 34 packages/api permissions.spec.ts
- 25 api skills.test.js
- 7 client useSkillPermissions.test.ts
- Type-check: pre-existing error count (188) dropped to 120 because my
restorations fixed some previously-broken field types.
* chore: Update package-lock.json to include react-arborist and memoize-one
* feat: Add support for react-arborist in Vite configuration
This update introduces a new condition in the Vite configuration to handle the 'react-arborist' package, ensuring it is properly recognized during the build process. This change enhances compatibility with the recently added 'react-arborist' dependency in the project.
* 🩹 fix: Hide InvocationMode, Fix SkillContentEditor Click-to-Edit
1. Hide InvocationModePicker from both CreateSkillForm and SkillForm.
Component stays on disk for when the backend lands the column.
2. Fix "Click to edit" doing nothing on SkillContentEditor. The
`onBlur={() => setIsEditing(false)}` on the TextareaAutosize was
racing with `autoFocus` — React renders the textarea, autoFocus
fires, then a layout/reconciliation blur fires immediately,
bouncing back to preview mode before the user can interact.
Removed onBlur; users toggle via the header button or Escape key.
* 🎨 feat: Reader-First Skills UI — Match Claude.ai Layout
Reworks the Skills UI from form-first to reader-first, matching
Claude.ai's skill detail pattern.
**Default view is now read-only.** Clicking a skill in the sidebar
navigates to `/skills/:id` which renders `SkillDetail` — a clean
content view with:
- Skill name as the primary heading
- Metadata row: "Added by" + "Last updated" (formatted date)
- Description block
- Rendered SKILL.md body in a bordered card with a source/rendered
toggle (eye + code icons, matching Claude.ai's segmented control)
No form fields, no save/cancel buttons. The user reads the skill
first and takes action deliberately.
**Create is now a dialog.** The `/skills/new` route is gone.
`CreateSkillMenu` (the + dropdown in the sidebar) now opens
`CreateSkillDialog` — a minimal modal with name, description, and
instructions fields. Upload-from-file still works: parse → populate
dialog → create. Matches Claude.ai's "Write skill instructions"
modal.
**Edit is behind an action.** The detail view shows an "Edit" button
(permission-gated) that navigates to `/skills/:id/edit`, rendering
the existing `SkillForm`. The edit route is preserved for direct
linking.
**Navigation goes to detail, not edit.** `SkillListItem` now
navigates to `/skills/:id` (detail) instead of `/skills/:id/edit`.
- `display/SkillMarkdownRenderer.tsx` — shared ReactMarkdown
component extracted from `SkillContentEditor`. Same remark/rehype
plugins, no form dependency.
- `display/SkillDetail.tsx` — the reader-first view (replaces the
old thin wrapper).
- `dialogs/CreateSkillDialog.tsx` — OGDialog modal for skill
creation.
- `layouts/SkillsView.tsx` — gutted and rebuilt. Three states:
no-skill (empty state), skillId (SkillDetail), skillId+edit
(SkillForm). Removed full-page CreateSkillForm, removed TreeView.
- `buttons/CreateSkillMenu.tsx` — opens dialog instead of navigating
to `/skills/new`. Upload flow: parse → set dialog defaults → open.
- `lists/SkillListItem.tsx` — navigate to detail, not edit.
- `routes/index.tsx` — removed `/skills/new` and file/nodeId routes;
`/skills` renders SkillsView directly (empty state).
- `display/index.ts`, `dialogs/index.ts` — added new exports.
- `locales/en/translation.json` — added ~10 new keys for metadata,
toggle labels, dialog title, empty state.
* 🩹 fix: SkillContentEditor click-to-edit z-index — button was z-0 behind rendered content
* 🩹 fix: Align Edit button size with Share/Delete (size-9)
* 🎨 feat: Claude.ai-Style Skill List Panel
Rewrites the skills sidebar to match Claude.ai's panel layout:
- Header: "Skills" title + search icon (toggles input) + add icon
(opens CreateSkillDialog directly, no dropdown menu)
- Collapsible "Skills" section with chevron toggle
- Skill items: 24px icon badge (rounded square with ScrollText icon)
+ name only. No description text in the list — that lives in the
detail view. Active item gets highlighted bg + bold font.
- Removed AdminSettings button from sidebar header — admin config
is accessible via the admin dashboard, not cluttering every user's
skill list.
- Removed FilterSkills wrapper (was Filter + AdminSettings +
CreateSkillMenu). The search + create are now inline in the panel
header.
Files changed:
- sidebar/SkillsSidePanel.tsx — full rewrite
- sidebar/SkillsAccordion.tsx — simplified wrapper
- lists/SkillList.tsx — collapsible section, no description
- lists/SkillListItem.tsx — icon badge + name, memo'd
* 🎨 fix: Align Skills UI Styling with Prompts Patterns
Style alignment pass based on direct comparison with claude.ai and
the existing prompts preview dialog.
SkillsSidePanel search now replaces the title in the header row when
toggled (search icon + input + X close), matching Claude.ai's pattern.
Previously it pushed a separate input below the header, wasting
vertical space. Close button clears the search term.
Replaced `text-text-tertiary` with `text-text-secondary` across
SkillDetail, SkillList, SkillForm, CreateSkillForm, CreateSkillDialog,
SkillContentEditor. Tertiary was too dark / low contrast.
SkillList section chevron label now reads "Personal skills" (matching
Claude.ai) via the existing `com_ui_my_skills` key, instead of the
generic "Skills" which duplicated the header.
Aligned with `PromptDetailHeader` styling:
- 48px round icon (ScrollText in bg-surface-secondary circle)
- Name + public badge in the icon row
- Metadata below the icon: User icon + author, Calendar icon + date
(text-xs text-text-secondary with gap-3, matching prompts exactly)
- Description uses the same label-above-text pattern as prompts
- Content card uses `bg-transparent` border (not bg-surface-primary-alt)
- Toggle buttons use size-5 icons and text-text-secondary for inactive
Changed from `max-w-lg p-0` to `max-w-5xl` with the same max-height
and padding pattern as the prompts PreviewPrompt dialog:
`max-h-[80vh] p-1 sm:p-2 gap-3 sm:gap-4`. Close button now renders
via default OGDialogContent behavior (removed showCloseButton=false).
* 🩹 fix: SkillDetail fills parent height, tighter spacing (px-6 pb-6 gap-2)
* 🩹 fix: Align Skills panel header padding (px-4) with list content below
* 🩹 fix: Reduce Skills header top padding (pt-2) to align with sidebar icon strip
* 🩹 fix: Tighten Skills header (py-2) and detail top (py-2) to align with sidebar icons and match edit view
* 🩹 fix: Offset SidePanel Nav pt-2 with -mt-2 on SkillsAccordion so Skills header aligns with icon strip
* 🛠️ fix: Increase Node memory limit for production build in package.json
* 🩹 fix: Remove top padding from SkillDetail header row (py-2 → pb-2)
* 🏗️ refactor: Move pt-2 from SidePanel/Nav wrapper to each panel
Removed the global `pt-2` from `SidePanel/Nav.tsx` and pushed it
into each panel's own top-level wrapper. This lets each panel own
its vertical alignment independently — Skills can sit flush at the
top to align with the sidebar icon strip, while other panels keep
their original spacing.
Panels updated with `pt-2`:
- PromptsAccordion (via className on PromptSidePanel)
- BookmarkPanel
- FilesPanel
- MemoryPanel
- MCPBuilderPanel
- AgentPanel (form wrapper)
- AssistantPanel (form wrapper)
- ParametersPanel (already had pt-2)
SkillsAccordion: removed the -mt-2 hack, now naturally flush.
* 🧹 fix: Align CreateSkillDialog field styling + remove 19 unused i18n keys
Dialog fields: all three inputs now use consistent `rounded-xl
border-border-medium px-3 py-2 text-sm` styling. Replaced the
`<Input>` component with a plain `<input>` to avoid the component's
built-in `rounded-lg border-border-light` overriding the dialog's
border style. Labels use `font-medium` for consistency.
Removed 19 unused translation keys from translation.json:
com_ui_skill_body, com_ui_skill_body_placeholder,
com_ui_skill_create_subtitle, com_ui_skill_file_delete_confirm,
com_ui_skill_file_delete_error, com_ui_skill_file_deleted,
com_ui_skill_files_empty, com_ui_skill_files_multi_hint,
com_ui_skill_list, com_ui_skill_load_error,
com_ui_skill_resize_file_tree, com_ui_skill_select_file,
com_ui_skill_select_file_desc, com_ui_skills_load_error,
com_ui_add_first_skill, com_ui_create_skill_page,
com_ui_edit_skill_page, com_ui_save_skill, com_ui_no_skills_title
* 🎁 feat: Upload Skill Dialog + Simplified Create Menu
New `UploadSkillDialog` matching Claude.ai's upload modal:
- Dashed drop zone with drag-and-drop support
- Accepts .md, .zip, .skill files
- Phase 1: processes .md files (parses YAML frontmatter → creates
skill with body as the full file content)
- Shows file requirements below the drop zone
- On success: navigates to the new skill's detail view
`CreateSkillMenu` now has two flat options (no sub-menu):
- "Write skill instructions" → opens `CreateSkillDialog`
- "Upload a skill" → opens `UploadSkillDialog`
Removed the disabled "Create with AI" option and the old file input
hidden-element approach. The sidebar `+` button now renders
`CreateSkillMenu` directly instead of a standalone create dialog.
- Removed 5 unused i18n keys (com_ui_skill_added_by,
com_ui_skill_last_updated, com_ui_skills_add_first,
com_ui_skills_filter_placeholder, com_ui_skills_new)
- Tightened metadata gap in SkillDetail (mt-1 → mt-0.5)
- Added 7 new upload-related i18n keys
* 🔒 feat: Zip/Skill File Upload Support with Safety Limits
Rewrites UploadSkillDialog to properly handle all three accepted
file types:
- `.md` — reads as text, parses YAML frontmatter, creates skill
- `.zip` / `.skill` — reads as ArrayBuffer, extracts with JSZip,
finds SKILL.md (at root or one level deep), parses its content,
creates skill. Shows spinner during processing.
Security guards against zip bombs:
- MAX_ZIP_SIZE: 50MB compressed file limit
- MAX_ENTRIES: 500 file limit inside the archive
- Path traversal rejection: skips entries with `..` or leading `/`
- SKILL.md search limited to depth ≤ 2 segments
Added `jszip@^3.10.1` to client dependencies (already in the
monorepo's node_modules from backend usage).
The name is inferred from the zip filename if SKILL.md frontmatter
doesn't have one (e.g. `skills-autofix.zip` → `skills-autofix`).
* 🚀 feat: Backend Skill Import + Live File Upload Endpoints
New endpoint that accepts a single multipart file (.md, .zip, .skill)
and creates a skill with all its files in one request:
- **.md**: parse YAML frontmatter → create skill with body
- **.zip / .skill**: extract with JSZip, find SKILL.md (root or one
level deep), create skill from its content, then persist every
additional file via `upsertSkillFile` + local file storage strategy.
Returns the created skill + an `_importSummary` with per-file
results.
Security:
- 50MB compressed file size limit (multer)
- 500 max entries in archive
- 10MB per individual file
- Path traversal rejection (no `..`, no absolute, validated charset)
- File type filter: only .md/.zip/.skill accepted
- Rate limited via existing `fileUploadIpLimiter` + `fileUploadUserLimiter`
Handler lives in `packages/api/src/skills/import.ts` with injectable
deps (`createSkill`, `upsertSkillFile`, `saveBuffer`) for testability.
Replaced the 501 stub with a real handler:
- Accepts multipart FormData with `file` + `relativePath`
- Saves file via local storage strategy
- Calls `upsertSkillFile` to persist the SkillFile record
- Returns the upserted document
- Rate limited, ACL-gated (EDIT permission required)
- 10MB per file limit
`UploadSkillDialog` now sends the file to `/api/skills/import` via
`dataService.importSkill(formData)` — no more client-side JSZip.
Removed `jszip` from client dependencies (only backend needs it).
Added `importSkill()` in data-service + `importSkill()` endpoint
builder in api-endpoints.
Updated the file upload test from expecting 501 stub to expecting
400 "no file provided" (live validation). All 25 skill route tests
pass.
* 🔒 fix: Complete Import Handler — Validation, Ownership, Error Surfacing
Fixes several gaps in the skill import flow:
1. **Skill validation now runs and surfaces properly.** The import
handler calls the real `createSkill(CreateSkillInput)` which runs
`validateSkillName`, `validateSkillDescription`, `validateSkillBody`.
Validation errors (SKILL_VALIDATION_FAILED) are caught and returned
as 400 with the issue messages. Duplicate-key errors return 409.
Previously all errors were swallowed into a generic 500.
2. **`authorName` is now populated.** The `CreateSkillInput` requires
`authorName` which was missing — resolved from `req.user.name ??
req.user.username ?? 'Unknown'`, matching the existing create handler.
3. **SKILL_OWNER permission is granted after import.** Calls
`grantPermission` with `AccessRoleIds.SKILL_OWNER` so the uploader
can edit/delete/share the imported skill. This was entirely missing —
imported skills would have been ownerless.
4. **`tenantId` propagated.** Both the skill and each SkillFile record
receive `req.user.tenantId` for multi-tenant deployments.
5. **SkillFile records are created in the DB.** Each non-SKILL.md file
in the zip is saved to file storage via `saveBuffer` and recorded
via `upsertSkillFile`, which validates the relativePath, infers the
category from the path prefix, and atomically bumps the skill's
`fileCount` and `version`.
Import deps now include `grantPermission` from PermissionService,
injected in `api/server/routes/skills.js`.
* 🐛 fix: Import grant uses accessRoleId (not roleId) — fixes skill not appearing in list
* 🎨 fix: Cache invalidation, file tree, frontmatter rendering
Three fixes for the skill detail view:
1. **Cache invalidation after import.** UploadSkillDialog now calls
`queryClient.invalidateQueries([QueryKeys.skills])` after a
successful import so the sidebar list picks up the new skill
without requiring a page refresh.
2. **File tree in detail view.** When a skill has `fileCount > 0`,
the detail view now queries `useListSkillFilesQuery` and renders
a file list below the body card — SKILL.md first, then folders
and root files. Icons: Folder for directories, FileText for files.
3. **Frontmatter stripped and rendered as metadata.** YAML frontmatter
(`---\nversion: 0.1.0\ntriggers: ...\n---`) is now parsed out of
the body before markdown rendering. The `name` and `description`
fields are skipped (already shown in the header). Remaining fields
(version, triggers, dependencies, etc.) are displayed in a
Claude.ai–style grid: label on the left, value on the right,
above the rendered markdown content. Source view still shows the
full raw body including frontmatter.
* 🩹 fix: Always fetch skill files — fileCount may be stale in cached skill object
* 🌳 feat: Inline File Tree in Sidebar Skill List
Moves the file tree from the bottom of SkillDetail into the sidebar
list, matching Claude.ai's pattern:
- Multi-file skills show a chevron toggle on the right side of the
skill list item
- Clicking the chevron expands an inline file tree below the skill
name: SKILL.md first, then folders (with folder icon + right
chevron) and root files
- File list is fetched lazily (only when expanded) via
useListSkillFilesQuery
- Clicking a file navigates to the skill detail view
- Files section removed from SkillDetail — the sidebar is now the
sole file tree location, keeping the detail panel clean
SkillDetail cleaned up: removed groupFiles helper, file-related
state, useListSkillFilesQuery import, FileText/Folder icon imports.
* 🌲 feat: Virtualized inline file tree with react-vtree
Replace hand-rolled recursive FolderRow/FileRow buttons with a proper
virtualized FixedSizeTree from react-vtree for the sidebar skill list.
Dynamic height tracks open folders; capped at 350px with smooth
expand/collapse transitions.
* chore: Remove no longer used SkillFileTree and SkillTreeNode components
* chore: Update Vite config to replace 'react-arborist' with 'react-vtree' for module resolution
* feat: Skill file content viewing with lazy DB caching
- Add `skills` field to `fileStrategiesSchema` so operators can
configure a dedicated storage backend for skill files. Falls back
by type (image/document) when unset.
- Fix hardcoded `FileSources.local` in skill save/import — now uses
the resolved strategy via `getFileStrategy(req.config, { context })`.
- Replace 501 download stub with real handler that streams from any
storage backend and returns JSON `{ content, mimeType, isBinary }`.
- Binary detection (null-byte + non-printable ratio on first 8 KB)
flags files on first read so they're never re-fetched.
- Text content ≤ 512 KB is cached in the SkillFile MongoDB document;
subsequent reads skip storage entirely.
- Clicking a skill row now expands inline files (not just chevron).
- Clicking a file navigates to `?file=<path>` and renders content
in a new SkillFileViewer (markdown, code, images, binary placeholder).
* chore: Remove react-window and its type definitions from package.json and package-lock.json
- Deleted `react-window` and `@types/react-window` dependencies from both `package.json` and `package-lock.json` to streamline the project and reduce unnecessary bloat.
* fix: Build errors — remove endpoints import, fix Uint8Array cast
- Replace `import { endpoints }` (not public) with inline URL in
SkillFileViewer
- Remove `as Uint8Array` cast in stream chunk handling
- Extend getSkillFileByPath return type with content/isBinary to
decouple from data-schemas build artifact resolution
* chore: Remove 8 unused i18next keys
com_ui_create_skill_ai, com_ui_create_skill_manual,
com_ui_delete_folder_confirm_var, com_ui_delete_skill,
com_ui_delete_skill_confirm_var, com_ui_delete_var,
com_ui_rename_var, com_ui_skill_files
* fix: Add configMiddleware to skills router, handle SKILL.md in viewer
- Add configMiddleware to skills router so req.config is populated
when getLocalFileStream (or any strategy) reads file paths.
- Handle SKILL.md in download handler — serves skill.body directly
from the Skill document instead of looking for a SkillFile record.
- Clicking SKILL.md in sidebar tree now opens the file viewer
(matching Claude.ai behavior: file view vs default detail view).
* ci: Run unit tests on PRs to any branch
Remove the branches filter from both test workflows so contributor
PRs targeting feature branches (not just main/dev) get CI coverage.
Path filters are kept so tests only run when relevant files change.
* fix: Update skills route tests for download handler changes
- Mock configMiddleware (sets req.config for file storage access)
- Mock getStrategyFunctions and getFileStrategy (storage strategy deps)
- Replace 501 stub test with SKILL.md content test + 404 test
* fix: Auto-expand files, frontmatter parsing, select-none, prefetch
- Auto-expand file tree when navigating directly to a skill URL
- Prefetch files for the active skill (eliminates first-expand lag)
- Fix frontmatter parser to handle multi-line YAML list values
(triggers field was missing because it uses list syntax)
- SkillFileViewer now parses frontmatter for .md files — shows
structured grid + rendered body (matching SkillDetail's display)
with source/rendered toggle
- Add select-none to all sidebar skill and file tree buttons
* refactor: Derive expanded state from isActive instead of useEffect
Replace useEffect sync with deterministic derivation:
expanded = hasFiles && (isActive || !collapsed)
Active skill is always open. collapsed is a manual toggle that
only takes effect on non-active items.
* fix: Remove empty space above body card — overlay view toggle
Move the rendered/source toggle from a dedicated row (40px of empty
space) to an absolute-positioned overlay in the card's top-right
corner, matching Claude.ai's layout.
* fix: Remove header bars from content editors — overlay action buttons
Collapse the full-width header bars ("Skill Content", "Text") in
SkillContentEditor, PromptTextCard, and PromptEditor. Action buttons
(edit/save toggle, copy, variables) are now absolute-positioned in
the card's top-right corner, reclaiming ~46px of vertical space.
* fix: Spinner visibility in file viewer — use text-text-secondary
* fix: Address review findings — security, correctness, code quality
Codex P1: Use $unset instead of undefined to clear cached content
and isBinary fields on file re-upload (Mongoose strips undefined).
Codex P2: Match skill-file validation errors by error.code instead
of error.message substring.
F1: Zip bomb defense — track cumulative decompressed bytes (500 MB
cap), check declared uncompressed size before buffering each entry.
F2: Remove misleading "atomically" from import handler JSDoc.
F3: Static import for isBinaryBuffer instead of dynamic import().
F4: Replace console.error with logger in upload handler.
F6: Add multer error handler middleware to skills router.
F7: Move React import to top of SkillDetail.tsx.
F9: Fix variable shadowing (trimmed → item) in parseFrontmatter.
F11: Replace JSON.parse(JSON.stringify()) with toJSON() for
Mongoose document serialization.
F12: Remove dead dynamic import('fs') fallback (memoryStorage
always provides file.buffer).
F13: Hoist MIME_MAP to module scope to avoid per-call allocation.
F16: Share single multer.memoryStorage() instance.
* fix: Follow-up review — close zip bomb gap, fix error handler
F1: Add post-decompression cumulative byte check with break (the
pre-decompression check relies on undocumented JSZip internals
that may be absent; this closes the gap unconditionally).
F2+F3: Multer error handler now forwards non-multer errors via
next(err) instead of swallowing them. Also catches file filter
rejections (plain Error, not MulterError) by message prefix.
F4: Move isBinaryBuffer import to local imports section per
CLAUDE.md import order rules.
F5: Simplify dead toJSON branch — createSkill returns a POJO.
* nit: Link filter error message to handler prefix check
* feat: Accordion expansion + active file highlight in sidebar
- Only one skill's file tree can be expanded at a time (accordion).
Expansion state lifted from SkillListItem to SkillList.
- Selected file gets bg-surface-active highlight in the tree.
Skill row uses subtle style (no background) when a file is active,
matching Claude.ai's pattern where the file — not the skill —
carries the selection state.
* style: Adjust margin for file tree in SkillListItem component
- Reduced left margin from 10 to 5 for improved layout consistency in the file tree display.
* fix: TS error on FileTreeNode, nested ternary, chevron collapse
- Make style prop optional to match react-vtree's NodeComponentProps
- Flatten nested ternary for skill row active styles
- Skill row click expands (but doesn't collapse) files + navigates
- Chevron click explicitly toggles collapse (matching Claude.ai
where clicking the chevron is how you collapse files)
* fix: Upload basePath, reject SKILL.md uploads, add skills permission route
- Pass basePath: 'uploads' in per-file upload handler (was defaulting
to 'images' path, inconsistent with the import flow).
- Reject uploads targeting SKILL.md (reserved path — download handler
special-cases it to return skill.body, making an uploaded file
unreachable via the API).
- Add skills entry to roles router permissionConfigs so PUT
/api/roles/:roleName/skills actually reaches a handler instead
of returning 404.
* feat: Expand content area, move controls to header, reduce padding
Default detail view:
- Remove rounded-xl bordered card wrapper — content flows directly
into the article, capitalizing on full screen width
- Move eye/code toggle inline with the divider row
- Reduce px-6/pb-6 to px-4/pb-4
File viewer:
- Move eye/code toggle from card overlay to the header bar
- Add copy-to-clipboard button for text files in the header bar
- Remove rounded-xl bordered card wrapper for markdown content
- Remove bordered pre wrapper for non-markdown text
- Reduce px-6/py-4 to px-4/py-3
Both views maximize content space over decorative chrome.
* fix: Stable header height, restore some padding
- Fix layout shift in file viewer header: use fixed h-10 so the
bar height stays constant whether the eye/code toggle renders
(markdown) or not (plain text).
- Bump content padding from px-4/py-3 back to px-5/py-4 in both
views — the previous reduction was too aggressive.
* fix: Grant rollback, path validation, error format, dead code cleanup
F2: grantOwnership now rolls back (compensating delete) on failure,
matching the create handler. Both markdown and zip import paths
check the result and return 500 on grant failure.
F4: Upload handler validates relativePath with regex + traversal
check before calling downstream upsertSkillFile.
F5: Document JSZip _data.uncompressedSize as best-effort; the
post-decompression cumulative check is the real safety net.
F10: Standardize all upload handler error responses to { error }
(was { message }, inconsistent with handlers.ts).
F13: Single-pass fileResults accumulation in import handler.
F1-5: Remove dead uploadFileStubHandler (no route references it).
Codex P2: Fix delete nav from /skills/new to /skills.
F12: Use cn() in UploadSkillDialog instead of template literals.
* perf: Stream-first binary detection + O(1) public skill check
F1: Download handler now reads only the first 8 KB for binary
detection. If binary, the stream is destroyed immediately without
buffering the remaining file. Text files continue reading for
caching. Eliminates buffering up to 10 MB per request for binary
files under concurrent load.
F7: Single-skill GET and PATCH now use hasPublicPermission (O(1)
ACL lookup) instead of getPublicSkillIdSet (queries ALL public
skill IDs). The list handler still uses the Set approach since it
serializes multiple skills. serializeSkill/serializeSkillSummary
now accept boolean | Set for flexibility.
* fix: Update test to match { error } response format
* fix: Critical stream truncation bug, grantedBy, error format
NF-1 (CRITICAL): Rewrite binary detection to single for-await loop.
Breaking out of for-await-of destroys the stream via iterator.return(),
so the previous two-loop approach silently truncated text files > 8KB.
Now: one loop collects chunks, checks binary after 8KB accumulated,
and either destroys+returns (binary) or continues reading (text).
NF-2: Add grantedBy to import handler's grantPermission call and
interface (was missing, inconsistent with create handler).
NF-3: Standardize all import handler error responses from { message }
to { error }, matching handlers.ts convention. Update client's
UploadSkillDialog to read response.data.error accordingly.
* fix: Prefer specific validation message over generic error field
* fix: YAML quote stripping, saveBuffer null guard, dot segment rejection
- Strip surrounding YAML quotes from frontmatter values so
name: "my-skill" parses as my-skill (not "my-skill" with quotes
that fails the name validator).
- Guard resolveSkillStorage against backends with saveBuffer: null
(e.g. OpenAI/vector strategies) — throws a descriptive error
caught by the handler's try/catch instead of a TypeError.
- Tighten upload path validation to reject . segments (e.g.
docs/./a.md) matching the model-layer validator, preventing
storage writes for paths the DB will reject.
* fix: Orphan cleanup, stream errors, malformed zip, cache latency
F1: Upload handler now deletes the stored blob if the subsequent
DB upsert fails, preventing orphaned files on disk/cloud.
F2: Multer error handler returns { error } (was { message }).
F3: Wrap JSZip.loadAsync in try/catch — malformed zip returns 400
instead of falling through to 500.
F4: Raw download stream gets an error handler — logs the error and
destroys the response if headers were already sent.
F8: Strip leading hyphens from inferred skill name so filenames
like _my-skill.zip don't produce -my-skill (invalid name pattern).
F9: Fire-and-forget all updateSkillFileContent cache writes so the
response is sent immediately. Cache failures are logged but don't
block or fail the read.
* fix: Import orphan cleanup + Content-Disposition sanitization
Finding A: Add deleteFile dep to ImportSkillDeps. The per-file loop
in handleZip now cleans up stored blobs when upsertSkillFile fails,
closing the second half of the F1 orphan fix (upload handler was
already fixed).
Finding B: Sanitize filename in Content-Disposition header for raw
downloads — strip quotes, backslashes, and newlines to prevent
header injection from user-uploaded filenames.
* security: Prevent stored XSS via raw file downloads
Non-image files served via ?raw=true now use Content-Disposition:
attachment (force download) instead of inline. An uploaded .html or
.svg file served inline from the LibreChat origin could execute
scripts with access to the user's session — this closes that vector.
Images stay inline (needed for <img> rendering in SkillFileViewer).
X-Content-Type-Options: nosniff added to prevent MIME sniffing.
* security: Block SVG XSS — allowlist safe raster MIME types for inline
SVG (image/svg+xml) passed the startsWith('image/') check and was
served inline, but SVG is a scriptable format — embedded <script>
tags execute in the LibreChat origin. Replace the prefix match with
a Set of safe raster-only MIME types (png, jpeg, gif, webp, avif,
bmp). SVGs and any future scriptable image/* subtypes now get
Content-Disposition: attachment (forced download).
* fix: Cap JSON text response at 1MB, consistent md name inference
F3: Text files > 1MB now return { isBinary: false } with no content
field, forcing the client to use ?raw=true for download. Prevents
buffering 10MB files into heap for JSON serialization. Frontend
shows a download fallback when content is absent.
F4: handleMarkdown now infers skill name from filename (same as
handleZip) when frontmatter has no name, instead of rejecting
with 400. Consistent behavior across import paths.
F1 (reviewer concern): upsertSkillFile is NOT affected — it uses
{ new: false } for insert-vs-replace detection but does a follow-up
findOne (lines 855-859) to return the post-upsert document.
* fix: deleteFile arg shape, raw URL base path, hoist SAFE_INLINE_MIMES
Codex P2: deleteFile expects { filepath } object, not a raw string.
Both upload handler cleanup and import handler cleanup now pass
{ filepath } to match the strategy contract (deleteLocalFile,
deleteFileFromS3 all expect a file object).
Codex P2: Raw download URL in SkillFileViewer now uses apiBaseUrl
prefix so subpath deployments (/chat, etc.) resolve correctly.
NIT: Hoist SAFE_INLINE_MIMES Set to factory scope — was re-allocated
per raw download request inside the if block.
* fix: Remove inert cache write for large text files, localize aria-label
N2: The { isBinary: false } cache write for text files > 1MB had no
effect — subsequent requests still fell through to stream read since
neither isBinary nor content provided a fast-path short-circuit.
Removed the pointless DB updateOne per request.
N4: Replace hardcoded "Back to skill" aria-label with localize().
* refactor: Extract shared parseFrontmatter, widen deleteFile type
N3: Extract parseFrontmatter into Skills/utils/frontmatter.ts —
single implementation shared by SkillDetail and SkillFileViewer.
Accepts optional skipKeys set so callers control which frontmatter
fields are excluded (SkillDetail skips name/description since
they're shown in the header; other .md files show all fields).
N5: Widen ImportSkillDeps.deleteFile file param from { filepath }
to { filepath; [key: string]: unknown } to signal extensibility
if strategies start accessing additional file properties.
* fix: Advance i past list items for skipped keys, DRY parseSkillMd
Finding A: parseFrontmatter now consumes multi-line YAML list items
before checking skipKeys — prevents list lines from leaking into
subsequent key parsing as spurious fields.
Finding B: parseSkillMd now delegates to the shared parseFrontmatter
instead of re-implementing the same frontmatter scanning loop.
Reduces client-side parseFrontmatter implementations from 3 to 1.
* fix: Call apiBaseUrl(), delete storage blob on file removal
- apiBaseUrl is a function, not a string — call it in the template
literal so raw download URLs resolve correctly.
- deleteFileHandler now looks up the file record before deleting,
then fire-and-forget deletes the storage blob via the strategy's
deleteFile. Previously only the DB record was removed, leaving
orphaned blobs in local/S3/Firebase/Azure storage.
* fix: Clean up storage blobs when deleting an entire skill
deleteHandler now lists all files for the skill before calling
deleteSkill, then fire-and-forget deletes each blob via the
storage strategy. Previously only per-file deletion cleaned up
blobs — deleting a whole skill left all associated files orphaned
in local/S3/Firebase/Azure storage.
* refactor: useImportSkillMutation hook, fix TSkill[] unsafe cast
- Create useImportSkillMutation in mutations.ts + ImportSkillOptions
type. UploadSkillDialog now uses the mutation hook instead of
calling dataService.importSkill directly with manual useState
loading management. Eliminates unmounted-component state update
risk and aligns with the React Query mutation pattern used by
every other mutation in the codebase.
- SkillSelectDialog: replace as unknown as TSkill[] with proper
TSkillSummary typing. SkillCard props updated to TSkillSummary.
The dialog only uses summary-level fields (name, description,
category, author) — the cast was hiding a type mismatch.
* fix: Use saved source for import cleanup, delete old blob on replace
Codex P2: Import cleanup now uses file.source (the backend the file
was actually saved to) instead of re-resolving from config. In mixed
strategy setups, the previous approach could target the wrong backend.
Codex P2: When re-uploading a file to an existing relativePath, the
old blob is now deleted after successful upsert. Previously only the
DB record was replaced, leaving the old storage object orphaned.
* fix: Register PUT /:roleName/skills route in roles router
* fix: Re-read skill after zip file processing for fresh metadata
The import response was built from the skill object created before
the file loop, but each upsertSkillFile bumps version and fileCount.
Clients caching the stale response would get 409 conflicts on first
edit and see incorrect file counts.
Now re-reads the skill via getSkillById after the loop so the
response reflects the current version, fileCount, and updatedAt.
* fix: Size-check SKILL.md before decompression, don't gate on fileCount
P1: SKILL.md was decompressed before any size accounting. A crafted
archive could expand SKILL.md past 10MB before validation ran. Now
checks declared size pre-decompression and actual size post, both
against MAX_SINGLE_FILE_BYTES.
P2: File list query was gated on cached fileCount which can be stale
after mutations. Now fetches files for the active skill regardless
of fileCount. hasFiles derived from fetched data with fileCount as
fallback, so newly uploaded files appear without hard refresh.
* fix: Move files declaration before hasFiles to avoid TDZ error
* security: Stream-decompress zip entries with enforced byte cap
Replace zipEntry.async('nodebuffer') (buffers entire entry before
checking limits) with zipEntry.nodeStream('nodebuffer') piped
through a byte counter that destroys the stream when the per-file
or cumulative limit is exceeded.
Previously, when JSZip's _data.uncompressedSize was absent (the
common case), a high-ratio entry could allocate hundreds of MB
before the post-decompression check caught it. Now decompression
is aborted mid-stream at the exact byte threshold — no entry can
exceed its limit regardless of compression ratio.
* refactor: Reorganize access check for prompts in useSideNavLinks hook
Moved the prompts access check to a new position in the code to improve readability and maintainability. This change ensures that the prompts link is added to the navigation only if the user has the appropriate access, without altering the existing functionality.
---------
Co-authored-by: Danny Avila <danny@librechat.ai>
…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)
* feat: Custom UI renderers for skill, read_file, and bash_tool Add specialized tool call components for the three skill tools, replacing the generic ToolCall fallback with contextual UI. * fix: Address review findings for skill tool UI renderers - Fix Codex P2: read skillName (camelCase) matching agent pipeline - Fix Codex P2: remove error regex from ReadFileCall to avoid false positives on normal file content containing "Error:" tokens - Extract useToolCallState hook to eliminate ~60% boilerplate duplication across SkillCall, ReadFileCall, and BashCall - Extract parseJsonField utility with consistent escaped-char-aware regex fallback, shared by all three components - Gate SkillCall bordered card on hasOutput to prevent empty card when expanded before output arrives - Skip highlightAuto for plaintext lang to avoid expensive auto-detection on files with unknown extensions - Expand LANG_MAP with php, cs, kt, swift, scss, less, lua, r; add FILENAME_MAP for Makefile and Dockerfile - Export langFromPath for testability - Add unit tests for parseJsonField, langFromPath, and ToolIcon skill type branches * refactor: Redesign BashCall as minimal terminal widget Replace the ExecuteCode-clone pattern with a purpose-built terminal UI: $ prompt prefix, dark background command zone, icon-only copy button, and raw monospace output. Drops useLazyHighlight, CodeWindowHeader, Stdout, and the "Output" label in favor of a cleaner two-zone layout that feels native to the terminal. * fix: parseJsonField unescape ordering and ReadFileCall empty card Replace the sequential .replace() chain in parseJsonField's regex fallback with a single-pass /\(.)/g replacement. The old chain processed \n before \, so \n (JSON-escaped literal backslash + n) was incorrectly decoded as a newline instead of \n. Gate ReadFileCall's bordered card on hasOutput (matching SkillCall's pattern) so the card does not render as an empty rounded box during streaming before output arrives. Add regression tests for \n decoding and unknown escape sequences. * fix: Followup review fixes - Refactor ExecuteCode to use shared useToolCallState hook, eliminating the last copy of the inline state machine - Escape regex metacharacters in parseJsonField to prevent injection from field names containing ., +, (, etc. - Fix contradictory test description in langFromPath tests * fix: Surface tool failure state in skill tool renderers Add error detection to useToolCallState via the shared isError check so tool calls that complete with an error prefix show a "failed" suffix instead of a success label. Prevents misleading users when read_file, skill, or bash_tool returns an error (e.g. file not found, skill not accessible). Matches the error handling pattern already used by the generic ToolCall component. * feat: Add bash syntax highlighting to BashCall command zone Reuse the shared useLazyHighlight singleton (already loaded by ReadFileCall and ExecuteCode) to highlight the command with bash grammar. Falls back to plain text while lowlight is loading. * fix: Align BashCall scrollbar to span full card width Move max-h/overflow-auto from the inner pre to the outer container so the scrollbar spans the full width like the output zone. Float the copy button with sticky positioning so it stays visible while scrolling long commands. * feat: Use GNU Bash icon for bash_tool progress header and ToolIcon Replace the generic SquareTerminal lucide icon with the GNU Bash logo (already in the project via LangIcon/langIconPaths) for both the BashCall progress header and the ToolIcon stacked icon mapping. * fix: Render raw content while highlighter loads, preserve command text on copy - ReadFileCall: fall back to raw output when useLazyHighlight returns null, preventing a blank code panel on first render before lowlight finishes its dynamic import - BashCall: drop .trim() from the copy handler so the clipboard receives exactly what's displayed (WYSIWYG copy) * fix: Alphabetize new translation keys within en/translation.json Relocate read_file, skill_finished, and skill_running into their correct alphabetical positions within the overall key list. * fix: Surface error state in ExecuteCode, fix BashCall import order - ExecuteCode now uses hasError from useToolCallState to show the "failed" suffix on failed code executions, matching the three new renderers - Reorder BashCall local imports to longest-to-shortest per project style
* feat: add $ command popover for manual skill invocation
Adds a new `$` command trigger in the chat textarea that opens a
searchable popover listing user-invocable skills. Selecting a skill
inserts `$skill-name` into the message and enables the skills badge
on the ephemeral agent. Follows the same patterns as `@` mentions
and `/` prompts.
* test: update useHandleKeyUp tests for $ skill command
Add showSkillsPopoverFamily, dollarCommand, and SKILLS permission
to the store/recoil/access mocks. Include test cases for the $
trigger, toggle gating, and permission gating.
* fix: address review findings in SkillsCommand
- Exclude auto-mode skills from popover (isUserInvocable now
returns false for InvocationMode.auto)
- Rewrite JSDoc to match the corrected filter logic
- Guard ArrowUp/ArrowDown against NaN when matches is empty
- Replace useRecoilState with useSetRecoilState for ephemeralAgent
to avoid subscribing to unrelated agent state changes
- Move skills guard into setter callback and drop ephemeralAgent
from handleSelect dependency array
- Add e.preventDefault() for Tab key alongside Enter
- Render error state (com_ui_skills_load_error) on query failure
- Render empty state (com_ui_skills_empty) when no matches
- Hoist ScrollText icon to module-level constant
- Export isUserInvocable for testability
* fix: address follow-up review findings
- Differentiate empty-catalog ("No skills yet") from no-match
search ("No skills found") using existing com_ui_no_skills_found
- Clamp activeIndex when matches shrink from search filtering to
prevent silent Enter/Tab failures
- Add early return after Escape handler to skip redundant checks
- Reorder package imports shortest-to-longest after react
- Add fast-typing test case for $ command ("$sk" at position 3)
* fix: gate $ command on assistants endpoint, fix Tab on empty matches
- Block $ popover on assistants/azureAssistants endpoints where
ephemeralAgent is not sent in the submission payload
- Allow Tab to close the popover when matches is empty instead of
trapping keyboard focus
- Add endpoint gating tests for $ on both assistants endpoints
* fix: reset skills popover on assistants switch + paginate skills query
- Close $ skills popover when endpoint switches to assistants or
azureAssistants, mirroring the existing + command reset
- Switch SkillsCommand from a single-page list query to the
cursor-paginated useSkillsInfiniteQuery and auto-fetch all pages
so client-side search covers the full catalog instead of only
the first 100 entries
- Show the spinner during background page fetches and suppress the
empty-state copy until paging completes
- Add test for popover reset on endpoint switch
* fix: prevent currency hijack and form submit on $ command
- Reject the $ trigger when fast-typed text after $ does not start
with a lowercase letter, so currency input like $100 or $5.99
no longer opens the skills popover and clears the textarea
- preventDefault() on Enter when the popover has no matches so
the surrounding chat form does not submit when the user dismisses
the popover via Enter
- Add tests for $100 and $5.99 currency inputs
* fix: defer $ popover to second keystroke and stop pagination on errors
- Defer opening the skills popover until a letter follows the $
character. Bare $ no longer triggers the popover, so starting a
message with $100, $5.99, or $EUR is fully preserved (the
textarea is not cleared by useInitPopoverInput on the first
keystroke). $a, $skill, $my-skill still open as expected.
- Add a sticky paginationBlockedRef circuit breaker on the auto
fetchNextPage effect so a transient page request error cannot
spin into an unbounded retry loop when isError flips back to
false on the next attempt.
- Update tests: bare $ no longer triggers, $a does, currency cases
remain blocked.
* feat: scaffold structured manual-skill channel for follow-up PR
Add a per-conversation pendingManualSkillsByConvoId atom family
that SkillsCommand appends to on selection. This is the writer
half of the structured channel that will let a follow-up PR
deterministically prime SKILL.md as a meta user message before
the LLM turn (mirroring Claude Code's `/skill` invocation), so
the backend never has to regex-parse `$name` out of user text.
The submit pipeline does not yet read this atom; the textual
`$skill-name ` insertion remains the authoritative signal until
the follow-up wires the read + reset on submit. Reset is already
wired into useClearStates so the atom does not leak across
conversation switches.
* test: cover SkillsCommand selection-flow contract
Add a component test that locks in the contract the follow-up
manualSkills PR has to honor when a user picks a skill in the $
popover:
- Pushes the skill name onto pendingManualSkillsByConvoId (the
per-conversation structured channel), with dedup
- Flips ephemeralAgent.skills to true via the callback-form setter
- Inserts $skill-name into the textarea as cosmetic confirmation
- Closes the popover via setShowSkillsPopover(false)
- Renders nothing when the popover atom is false
Mocks recoil setters and the skills query so the test exercises
the real component logic without spinning up the full provider
stack.
* revert: drop $ defer-until-letter guard for sibling consistency
Bring $ in line with @, /, and +: open the popover on the bare
trigger character. The earlier defer guard kept currency input
like \$100 from clobbering the textarea, but it broke the natural
"type \$ to browse skills" UX and was inconsistent with the other
trigger chars, none of which gate on the follow-up character
(/path/to/file, @username, +1 all clear the textarea the same way).
The currency hijack remains recoverable via Escape.
Drop the corresponding paste-protection tests for \$100 and \$5.99,
restore the bare-\$ trigger test.
) * feat: per-agent skill selection in builder and runtime scoping Wire skills persistence on the Agent model and enable the skills section in the agents builder panel. At runtime, scope the skill catalog to only the skills configured on each agent (intersected with user ACL). When no skills are configured, the full user catalog is used as the default. The ephemeral chat toggle overrides per-agent scoping to provide the full catalog. * fix: add scopeSkillIds to @librechat/api mock in responses unit test The test mocks @librechat/api but was missing the newly imported scopeSkillIds, causing createResponse to throw before reaching the assertions. Added a passthrough mock that returns the input array. * fix: scope primeInvokedSkills by agent's configured skills primeInvokedSkills was receiving the full unscoped accessibleSkillIds, bypassing the per-agent skill scoping applied to initializeAgent. This allowed previously invoked skills from message history to be resolved and primed even when excluded from the agent's configured skill set. Apply the same scopeSkillIds filtering to match the initializeAgent calls, so skill resolution is consistent across catalog injection and history priming. * fix: preserve agent skills through form reset and union prime scope Two related bugs in the per-agent skill selection flow: 1. resetAgentForm dropped the persisted skills array because the generic fall-through at the end of the loop excludes object/array values. Combined with composeAgentUpdatePayload always emitting skills, this caused any save of a previously-configured agent to silently overwrite skills with an empty array. Add an explicit case for skills mirroring the agent_ids handling. 2. primeInvokedSkills processes the full conversation payload, including prior handoff-agent invocations. Scoping it to only primaryAgent.skills meant a skill invoked by a handoff agent in a prior turn could not be resolved when the current primary agent had a different scope, leaving message history reconstruction incomplete. Union the per-agent scoped accessibleSkillIds across primary plus all loaded handoff agents so any skill any active agent could invoke is resolvable from history. * fix: mark inline skill removals as dirty The inline X button on the skills list called setValue without shouldDirty: true, so removing a skill via this control did not mark the skills field as dirty in react-hook-form state. When a user removed a skill with the X button and also staged an avatar upload in the same save, isAvatarUploadOnlyDirty returned true and onSubmit short-circuited to avatar-only upload, silently dropping the PATCH that would persist the skill removal. The dialog path (SkillSelectDialog) already passes shouldDirty: true on add/remove; this aligns the inline control with that behavior. * fix: restore full ACL scope for primeInvokedSkills history reconstruction Reverting the earlier scoping of primeInvokedSkills to the active-agent union. That change conflated runtime invocation scoping (which correctly gates what the model can call now) with history reconstruction (which restores bodies the model already saw in prior turns). Per-agent scoping still applies at: - Catalog injection (injectSkillCatalog via initializeAgent) - Runtime invocation (handleSkillToolCall via enrichWithSkillConfigurable, using each agent's scoped accessibleSkillIds in agentToolContexts) History priming is a read of past context, not a grant of new capability. Scoping it causes historical skill bodies to vanish from formatAgentMessages when an agent's skills list is edited mid-conversation or when the ephemeral toggle flips, which breaks message reconstruction and drops code-env file continuity for /mnt/data/{skillName}/ references. The user's ACL-accessible set is the correct and sufficient gate for history reconstruction. * fix: close openai.js skill gap and pin undefined vs [] semantics Three related gaps surfaced in review: 1. api/server/controllers/agents/openai.js was a third skill resolution site alongside responses.js and initialize.js, but still used the old activation gate (required ephemeralAgent.skills === true) and never passed accessibleSkillIds through scopeSkillIds. Per-agent scoping silently did not apply on this route. Mirror the same pattern used in responses.js so all three routes behave identically. 2. scopeSkillIds previously collapsed undefined and [] into the same "full catalog" fallback, making it impossible for a user to express "this agent has no skills." Tighten the semantics before any data is written under the old behavior: - undefined / null = not configured, full catalog - [] = explicitly none, returns [] - non-empty = intersection with ACL-accessible set Update defaultAgentFormValues.skills from [] to undefined so a brand new agent whose skills UI was never touched does not accidentally persist "explicit none" on first save (removeNullishValues strips undefined from the payload server side). 3. Add direct unit tests for scopeSkillIds covering all five cases (undefined, null, empty, disjoint, overlap, exact match, empty accessible set). 16 tests total in skills.test.ts pass. * fix: add scopeSkillIds to @librechat/api mock in openai unit test Same pattern as the earlier responses.unit.spec.js fix: the test mocks @librechat/api with an explicit object, so each newly imported symbol must be added to the mock. Without scopeSkillIds, OpenAIChatCompletion controller throws on destructuring before reaching recordCollectedUsage, causing the token usage assertions to fail.
…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.
…2708) * feat: compose per-agent scope and per-user active-state filters in $ popover Stack two runtime-truth filters on top of the existing `isUserInvocable` check so the `$` skill popover matches what will actually be available at turn time: - Per-agent skill scope from `agent.skills` (resolved via useChatContext + useAgentsMapContext), mirroring backend `scopeSkillIds` semantics: `undefined`/`null` → no scope, `[]` → empty, non-empty → intersection. - Per-user ownership-aware active state via `useSkillActiveState().isActive`. The filters are composed in a pure, exported `filterSkillsForPopover` helper so the agent-scope ∩ active ∩ invocable matrix can be unit-tested without rendering the component. Short-circuits on cheapest check first (agent scope → active → invocation mode). Backend still enforces both filters at runtime; this PR is a UX mirror so users do not see popover entries that would be filtered out by the time the LLM turn begins. * refactor: thread agentId into SkillsCommand as a prop Drop the direct `useChatContext()` call inside SkillsCommand in favor of receiving `agentId` from ChatForm. The parent already subscribes to the conversation via its single useChatContext call, and SkillsCommand is wrapped in React.memo — threading the id as a prop means the popover only re-renders when agent_id actually changes instead of on every unrelated conversation-shape mutation. Mirrors the pattern AttachFileChat already uses for `conversation` / `agentId`. No behavior change; filter semantics and test coverage are identical. * fix: fail closed when agent skill scope cannot be resolved Previously, if `conversation.agent_id` was set but the agents map had no entry for it (hydration pending, query failure, or missing VIEW access), the popover treated the scope as `undefined` and showed the full ACL catalog. That leaks options the backend will reject at turn time, the opposite of what this phase is meant to do. Distinguish the unresolved cases ("map undefined" and "agent not in map") from the intentionally-unconfigured case ("agent exists, no `skills` field") and return `[]` for the former, preserving the backend semantics of `scopeSkillIds` only for the latter. Adds two tests covering both fail-closed branches. * fix: surface agent.skills in list projection and treat ephemeral ids as unscoped Two holes flagged on the earlier fail-closed commit: 1. The list-agents projection in `getListAgentsByAccess` omitted the `skills` field, so `agentsMap[agentId].skills` was always undefined and the popover fell back to the full ACL catalog for every scoped agent — the opposite of this phase's intent. Add `skills: 1` to the projection and lock it in with a new test. 2. Conversations can carry ephemeral agent ids (e.g. `ephemeral` after switching off the agents endpoint) that intentionally don't live in the agents map. The prior fail-closed branch blanked the popover in those cases. Treat anything that doesn't start with `agent_` as unscoped via the existing `isEphemeralAgent` helper so the popover shows the full ACL-visible catalog, matching how no-agent convos already behave. Frontend: 18 tests pass (adds one ephemeral-id case). Backend: 10 getListAgentsByAccess tests pass (adds one projection case). * refactor: pass through hydration race, only fail closed when map is authoritative Split the two "cannot resolve scope" cases that were previously collapsed onto the same fail-closed branch: - `agentsMap === undefined` means the agents list query has not settled yet. Return `undefined` (full catalog). The map typically hydrates well before the first `$` open, and the backend still scopes at turn time — blanking the popover during a sub-second race produces worse UX with no security benefit. - `agentsMap` is populated but the agent is absent means the agent was deleted or the user's VIEW access was revoked mid-session. That is authoritative missing state, so keep the fail-closed behavior — the full catalog would be misleading. Updates the associated test case to assert the hydration-race path now shows the catalog, and rewrites the in-code comment to distinguish the two branches. * refactor: drop `as string` in agent scope memo by tightening the guard `isEphemeralAgent` returns true for null/undefined so the original code was runtime-safe, but its signature returns plain `boolean` rather than a type predicate, so TypeScript never narrowed `agentId` to `string` and the subsequent map lookup required an `as string` assertion. Split the guard into `!agentId || isEphemeralAgent(agentId)` so the narrowing falls out naturally and the assertion can be removed. No behavior change; 18 tests pass.
danny-avila
force-pushed
the
feat/agent-skills
branch
from
75029e9 to
d519c82
Compare
Contributor
GitNexus: 🚀 deployedThe |
danny-avila
added a commit
that referenced
this pull request
Apr 17, 2026
Phase 4 of Agent Skills umbrella (#12625): gate bash_tool and skill-file priming on the execute_code capability only. LIBRECHAT_CODE_API_KEY is the LibreChat-hosted sandbox service key — system-level, not a per-user secret — so the per-user loadAuthValues lookup was legacy plumbing. The agents library reads process.env[EnvVar.CODE_API_KEY] itself; this change removes the redundant resolution paths.
Contributor
GitNexus: 🚀 deployedThe |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.