diff --git a/api/app/clients/BaseClient.js b/api/app/clients/BaseClient.js index 1a77c8cf82..41b6ea0db7 100644 --- a/api/app/clients/BaseClient.js +++ b/api/app/clients/BaseClient.js @@ -494,7 +494,7 @@ class BaseClient { } /** * Persist the user's manual skill picks onto the user message so the - * frontend `ManualSkillPills` component can render them in history + * frontend `SkillPills` component can render them in history * after reload. UI-only metadata — the runtime skill resolution * pipeline reads the top-level `req.body.manualSkills` separately. * Filter is defense-in-depth on top of Mongoose schema validation: @@ -510,7 +510,7 @@ class BaseClient { } /** * Persist the names of skills auto-primed this turn via `always-apply` - * frontmatter so `ManualSkillPills` can render pinned-variant badges + * frontmatter so `SkillPills` can render pinned-variant badges * on the user bubble that survive reload and history render. Frozen * at turn time (not reconstructed from `Skill.alwaysApply` at render * time) because the flag is mutable — historical turns must keep diff --git a/api/server/controllers/agents/client.js b/api/server/controllers/agents/client.js index a19a5bf628..3669933cae 100644 --- a/api/server/controllers/agents/client.js +++ b/api/server/controllers/agents/client.js @@ -948,11 +948,11 @@ class AgentClient extends BaseClient { * the card would cause the skill body to get primed twice per * turn starting on turn 2. The user-facing acknowledgement for * always-apply lives on the user bubble as the pinned - * `ManualSkillPills` row (`message.alwaysAppliedSkills`), which + * `SkillPills` row (`message.alwaysAppliedSkills`), which * is the durable signal the user wants: "this skill auto-primes". * * Live streaming display of manual user-bubble pills is handled - * by `ManualSkillPills` reading `message.manualSkills`. No + * by `SkillPills` reading `message.manualSkills`. No * separate SSE emit is needed here; trying to stream a mid-run * tool_call at index 0 collided with the LLM's first text * content, while emitting at a sparse offset pushed the card diff --git a/api/server/controllers/agents/openai.js b/api/server/controllers/agents/openai.js index 67b0221cc6..d26be8117e 100644 --- a/api/server/controllers/agents/openai.js +++ b/api/server/controllers/agents/openai.js @@ -480,6 +480,16 @@ const OpenAIChatCompletionController = async (req, res) => { alwaysApplySkillPrimes, }); indexTokenCountMap = primeResult.indexTokenCountMap; + /* Surface the cap-driven always-apply truncation at the controller + layer too — `injectSkillPrimes` already logs internally, but the + controller-level warn includes endpoint context so operators can + tell at a glance which path hit the cap. Mirrors AgentClient's + warn in `client.js`. */ + if (primeResult.alwaysApplyDropped > 0) { + logger.warn( + `[OpenAI API] Dropped ${primeResult.alwaysApplyDropped} always-apply prime(s) to stay within MAX_PRIMED_SKILLS_PER_TURN.`, + ); + } } /** diff --git a/api/server/controllers/agents/responses.js b/api/server/controllers/agents/responses.js index 2954f8973c..9c626234cd 100644 --- a/api/server/controllers/agents/responses.js +++ b/api/server/controllers/agents/responses.js @@ -378,6 +378,7 @@ const createResponse = async (req, res) => { getSkillByName: db.getSkillByName, }; + const appConfig = req.config; const enabledCapabilities = new Set( appConfig?.endpoints?.[EModelEndpoint.agents]?.capabilities, ); @@ -555,6 +556,16 @@ const createResponse = async (req, res) => { alwaysApplySkillPrimes, }); indexTokenCountMap = primeResult.indexTokenCountMap; + /* Surface the cap-driven always-apply truncation at the controller + layer too — `injectSkillPrimes` already logs internally, but the + controller-level warn includes endpoint context so operators can + tell at a glance which path hit the cap. Mirrors AgentClient's + warn in `client.js`. */ + if (primeResult.alwaysApplyDropped > 0) { + logger.warn( + `[Responses API] Dropped ${primeResult.alwaysApplyDropped} always-apply prime(s) to stay within MAX_PRIMED_SKILLS_PER_TURN.`, + ); + } } /* Stable for the turn: the capability set is the admin config, and diff --git a/client/src/components/Chat/Input/PendingManualSkillsChips.tsx b/client/src/components/Chat/Input/PendingManualSkillsChips.tsx index 3ff0c4b08b..5111ea985d 100644 --- a/client/src/components/Chat/Input/PendingManualSkillsChips.tsx +++ b/client/src/components/Chat/Input/PendingManualSkillsChips.tsx @@ -8,7 +8,7 @@ import store from '~/store'; * Chip row rendered above the textarea showing skills the user picked in * the `$` popover for the next submission. Each chip has an × button to * remove that skill before sending — primary purpose is new-chat UX where - * there's no submitted user message yet to render `ManualSkillPills` on. + * there's no submitted user message yet to render `SkillPills` on. * * Reads + writes `pendingManualSkillsByConvoId` atom directly. The atom * gets drained in `useChatFunctions.ask` on submit, so chips naturally diff --git a/client/src/components/Chat/Input/SkillsCommand.tsx b/client/src/components/Chat/Input/SkillsCommand.tsx index 364a0ff32e..aec1d1b80c 100644 --- a/client/src/components/Chat/Input/SkillsCommand.tsx +++ b/client/src/components/Chat/Input/SkillsCommand.tsx @@ -215,7 +215,7 @@ function SkillsCommandContent({ before the LLM turn — no textarea-level marker is needed, and injecting `$skill-name ` as text would mislead users into thinking free-form text invocation is supported. Visual confirmation after - submit comes from `ManualSkillPills` on the user message bubble + submit comes from `SkillPills` on the user message bubble until the live skill-card stream takes over. */ setPendingManualSkills((prev) => prev.includes(mention.value) ? prev : [...prev, mention.value], diff --git a/client/src/components/Chat/Input/__tests__/SkillsCommand.spec.tsx b/client/src/components/Chat/Input/__tests__/SkillsCommand.spec.tsx index 74ef28d996..064688552e 100644 --- a/client/src/components/Chat/Input/__tests__/SkillsCommand.spec.tsx +++ b/client/src/components/Chat/Input/__tests__/SkillsCommand.spec.tsx @@ -216,7 +216,7 @@ describe('SkillsCommand', () => { expect(agentUpdater({ skills: true })).toEqual({ skills: true }); /* Textarea is cleared of the `$` trigger but no `$skill-name ` cue is - inserted — visual confirmation is the `ManualSkillPills` row that + inserted — visual confirmation is the `SkillPills` row that renders on the submitted user message, and injecting text would mislead users into thinking free-form `$name` invocation works. */ expect(textAreaRef.current?.value).toBe(''); diff --git a/client/src/components/Chat/Messages/Content/Container.tsx b/client/src/components/Chat/Messages/Content/Container.tsx index 00d28b0536..7d5f842511 100644 --- a/client/src/components/Chat/Messages/Content/Container.tsx +++ b/client/src/components/Chat/Messages/Content/Container.tsx @@ -1,6 +1,6 @@ import { TMessage } from 'librechat-data-provider'; import Files from './Files'; -import ManualSkillPills from './ManualSkillPills'; +import SkillPills from './SkillPills'; const Container = ({ children, message }: { children: React.ReactNode; message?: TMessage }) => (
- - + + )} {children} diff --git a/client/src/components/Chat/Messages/Content/ContentParts.tsx b/client/src/components/Chat/Messages/Content/ContentParts.tsx index 0380e77f80..3fb23dc2a6 100644 --- a/client/src/components/Chat/Messages/Content/ContentParts.tsx +++ b/client/src/components/Chat/Messages/Content/ContentParts.tsx @@ -144,7 +144,7 @@ const ContentParts = memo(function ContentParts({ * response (no `manualSkills` field) → interim cards disappear and * the real `skill` tool_call part in `content` takes over. * - * Skipped on the user side (they get `ManualSkillPills` on the user + * Skipped on the user side (they get `SkillPills` on the user * bubble) and when no skills were invoked on this turn. */ const pendingSkills = useMemo( diff --git a/client/src/components/Chat/Messages/Content/ManualSkillPills.tsx b/client/src/components/Chat/Messages/Content/SkillPills.tsx similarity index 92% rename from client/src/components/Chat/Messages/Content/ManualSkillPills.tsx rename to client/src/components/Chat/Messages/Content/SkillPills.tsx index 7e56b048c4..b9b60f116a 100644 --- a/client/src/components/Chat/Messages/Content/ManualSkillPills.tsx +++ b/client/src/components/Chat/Messages/Content/SkillPills.tsx @@ -11,7 +11,7 @@ import { useLocalize } from '~/hooks'; * Exported so message-render code can import and pass the appropriate * value without hardcoding string literals. */ -export type ManualSkillPillsSource = 'manual' | 'always-apply'; +export type SkillPillsSource = 'manual' | 'always-apply'; /** * Compact pill row rendered on a submitted user message, one chip per skill @@ -25,12 +25,12 @@ export type ManualSkillPillsSource = 'manual' | 'always-apply'; * and pills survive page reloads / history renders. The `source` prop picks * the icon variant so both flavors render from the same component. */ -function ManualSkillPills({ +function SkillPills({ skills, source = 'manual', }: { skills?: string[]; - source?: ManualSkillPillsSource; + source?: SkillPillsSource; }) { const localize = useLocalize(); @@ -64,4 +64,4 @@ function ManualSkillPills({ ); } -export default memo(ManualSkillPills); +export default memo(SkillPills); diff --git a/client/src/components/Chat/Messages/Content/__tests__/ManualSkillPills.test.tsx b/client/src/components/Chat/Messages/Content/__tests__/SkillPills.test.tsx similarity index 78% rename from client/src/components/Chat/Messages/Content/__tests__/ManualSkillPills.test.tsx rename to client/src/components/Chat/Messages/Content/__tests__/SkillPills.test.tsx index a39e95e310..9b7a41b132 100644 --- a/client/src/components/Chat/Messages/Content/__tests__/ManualSkillPills.test.tsx +++ b/client/src/components/Chat/Messages/Content/__tests__/SkillPills.test.tsx @@ -6,21 +6,21 @@ jest.mock('~/hooks', () => ({ `${key}:${params?.[0] ?? ''}`, })); -import ManualSkillPills from '../ManualSkillPills'; +import SkillPills from '../SkillPills'; -describe('ManualSkillPills', () => { +describe('SkillPills', () => { it('renders nothing when skills is undefined', () => { - const { container } = render(); + const { container } = render(); expect(container.firstChild).toBeNull(); }); it('renders nothing when skills is empty', () => { - const { container } = render(); + const { container } = render(); expect(container.firstChild).toBeNull(); }); it('renders one pill per entry', () => { - render(); + render(); const items = screen.getAllByRole('listitem'); expect(items).toHaveLength(2); expect(items[0]).toHaveTextContent('brand-guidelines'); @@ -28,18 +28,18 @@ describe('ManualSkillPills', () => { }); it('localizes the list aria-label (manual default)', () => { - render(); + render(); expect(screen.getByRole('list')).toHaveAttribute('aria-label', 'com_ui_skills_manual_invoked:'); }); it('tags each pill with data-skill-source="manual" by default', () => { - render(); + render(); const items = screen.getAllByRole('listitem'); expect(items[0]).toHaveAttribute('data-skill-source', 'manual'); }); it('switches aria-label and data attribute for source="always-apply"', () => { - render(); + render(); expect(screen.getByRole('list')).toHaveAttribute( 'aria-label', 'com_ui_skills_always_apply_invoked:', @@ -50,9 +50,9 @@ describe('ManualSkillPills', () => { it('renders the pin-icon variant for always-apply (no ScrollText)', () => { const { container: alwaysApply } = render( - , + , ); - const { container: manual } = render(); + const { container: manual } = render(); // lucide-react icons render as SVGs with distinguishing class names. We // assert on class token presence rather than the full SVG markup so a // lucide internal change to path data doesn't break this contract. diff --git a/client/src/hooks/Chat/useChatFunctions.ts b/client/src/hooks/Chat/useChatFunctions.ts index 34c6990b07..36647908d2 100644 --- a/client/src/hooks/Chat/useChatFunctions.ts +++ b/client/src/hooks/Chat/useChatFunctions.ts @@ -259,7 +259,7 @@ export default function useChatFunctions({ error: false, /** * UI-only metadata. Survives reload because the backend persists the - * field on the message schema, and `ManualSkillPills` reads straight + * field on the message schema, and `SkillPills` reads straight * off the message so there's no Recoil state to clean up. Runtime * skill resolution reads the top-level `manualSkills` payload field. */ diff --git a/packages/data-provider/src/schemas.ts b/packages/data-provider/src/schemas.ts index 3e1f8a74ef..dd611250cd 100644 --- a/packages/data-provider/src/schemas.ts +++ b/packages/data-provider/src/schemas.ts @@ -682,7 +682,7 @@ export const tMessageSchema = z.object({ .optional(), /** * Skill names the user invoked manually via the `$` popover on this turn. - * Purely UI metadata — `ManualSkillPills` renders these above the message + * Purely UI metadata — `SkillPills` renders these above the message * bubble so users can see which skills they asked for in history and on * reload. Runtime resolution uses the top-level payload field with the * same name. Empty / absent for model-invoked skills (shown as tool_call diff --git a/packages/data-schemas/src/methods/skill.spec.ts b/packages/data-schemas/src/methods/skill.spec.ts index 4d8ea062a9..a3c62887eb 100644 --- a/packages/data-schemas/src/methods/skill.spec.ts +++ b/packages/data-schemas/src/methods/skill.spec.ts @@ -1187,6 +1187,80 @@ describe('Skill CRUD methods', () => { expect(result.skill.alwaysApply).toBe(true); } }); + + it('updateSkill accepts a body typo when an explicit top-level alwaysApply overrides it', async () => { + /* Validation respects the same precedence as derivation: a caller + sending `alwaysApply: true` alongside a body whose frontmatter + carries a typo (`always-apply: tru`) should NOT be rejected — + the body value is never consulted. Rejecting would be + user-hostile for programmatic callers that legitimately own the + explicit field. */ + const { skill } = await methods.createSkill(makeSkillInput({ name: 'typo-overridden' })); + const bodyWithTypo = `---\nname: typo-overridden\ndescription: body has a broken flag.\nalways-apply: tru\n---\n\n# Body`; + const result = await methods.updateSkill({ + id: skill._id.toString(), + expectedVersion: skill.version, + update: { alwaysApply: true, body: bodyWithTypo }, + }); + expect(result.status).toBe('updated'); + if (result.status === 'updated') { + expect(result.skill.alwaysApply).toBe(true); + } + }); + + it('updateSkill accepts a body typo when a structured frontmatter override carries a valid boolean', async () => { + /* Same precedence rule for the frontmatter-bag layer: a caller + that sends a structured `frontmatter['always-apply']: true` + plus a body with a typo is overriding the body at derivation + time, so the body typo should not reject the update. */ + const { skill } = await methods.createSkill(makeSkillInput({ name: 'fm-overrides-body' })); + const bodyWithTypo = `---\nname: fm-overrides-body\ndescription: broken body flag.\nalways-apply: yes\n---\n\n# Body`; + const result = await methods.updateSkill({ + id: skill._id.toString(), + expectedVersion: skill.version, + update: { + frontmatter: { + name: 'fm-overrides-body', + description: 'A small demo skill used in tests.', + 'always-apply': true, + }, + body: bodyWithTypo, + }, + }); + expect(result.status).toBe('updated'); + if (result.status === 'updated') { + expect(result.skill.alwaysApply).toBe(true); + } + }); + + it('updateSkill still rejects a body typo when no higher-precedence source is provided', async () => { + /* Body-inline validation must still fire when nothing else is + carrying the value — otherwise a body PATCH with `tru` would + silently drop the typo + keep the old column value. */ + const { skill } = await methods.createSkill(makeSkillInput({ name: 'typo-body-only' })); + const bodyWithTypo = `---\nname: typo-body-only\ndescription: nothing else carries the flag.\nalways-apply: tru\n---\n\n# Body`; + await expect( + methods.updateSkill({ + id: skill._id.toString(), + expectedVersion: skill.version, + update: { body: bodyWithTypo }, + }), + ).rejects.toMatchObject({ code: 'SKILL_VALIDATION_FAILED' }); + }); + + it('createSkill accepts a body typo when an explicit top-level alwaysApply overrides it', async () => { + /* Same precedence-aware validation on the create path: explicit + top-level `alwaysApply` short-circuits body-inline validation. */ + const bodyWithTypo = `---\nname: create-typo-overridden\ndescription: broken body flag.\nalways-apply: tru\n---\n\n# Body`; + const { skill } = await methods.createSkill( + makeSkillInput({ + name: 'create-typo-overridden', + alwaysApply: false, + body: bodyWithTypo, + }), + ); + expect(skill.alwaysApply).toBe(false); + }); }); describe('SkillFile methods', () => { diff --git a/packages/data-schemas/src/methods/skill.ts b/packages/data-schemas/src/methods/skill.ts index 55607b26ad..79387bc7a2 100644 --- a/packages/data-schemas/src/methods/skill.ts +++ b/packages/data-schemas/src/methods/skill.ts @@ -756,6 +756,10 @@ function resolveAlwaysApplyFromInput( frontmatter: Record | undefined, body: string | undefined, fallback: boolean, + /* Callers that have already parsed the body (e.g. because they also + ran body-level validation) can thread the result in to avoid a + second parse. Leave undefined to let the helper parse on demand. */ + precomputedBody?: BodyAlwaysApplyResult, ): boolean { if (typeof explicit === 'boolean') { return explicit; @@ -764,7 +768,7 @@ function resolveAlwaysApplyFromInput( if (typeof fromFrontmatter === 'boolean') { return fromFrontmatter; } - const fromBody = extractAlwaysApplyFromBody(body); + const fromBody = precomputedBody ?? extractAlwaysApplyFromBody(body); if (fromBody.status === 'valid') { return fromBody.value; } @@ -846,6 +850,11 @@ export function createSkillMethods(mongoose: typeof import('mongoose'), deps: Sk } async function createSkill(data: CreateSkillInput): Promise { + /* Parse body's always-apply status once — reused for validation + (below) and derivation in `resolveAlwaysApplyFromInput`. Avoids + parsing the same YAML frontmatter block twice per create. */ + const bodyAlwaysApply = + data.body !== undefined ? extractAlwaysApplyFromBody(data.body) : undefined; const issues: ValidationIssue[] = [ ...validateSkillName(data.name), ...validateSkillDescription(data.description), @@ -853,8 +862,25 @@ export function createSkillMethods(mongoose: typeof import('mongoose'), deps: Sk ...validateSkillDisplayTitle(data.displayTitle), ...validateSkillFrontmatter(data.frontmatter), ...validateAlwaysApply(data.alwaysApply), - ...validateAlwaysApplyInBody(data.body), ]; + /* Body-level `always-apply:` only needs to be well-formed when a + higher-precedence source won't override it (see + `resolveAlwaysApplyFromInput` for the cascade). A caller sending + an explicit top-level `alwaysApply` or a structured + `frontmatter['always-apply']` has the body value overridden at + derivation time, so rejecting them for a typo they aren't relying + on would be user-hostile. */ + if ( + bodyAlwaysApply?.status === 'invalid' && + typeof data.alwaysApply !== 'boolean' && + typeof data.frontmatter?.['always-apply'] !== 'boolean' + ) { + issues.push({ + field: 'body.frontmatter.always-apply', + code: 'INVALID_TYPE', + message: '"always-apply" in SKILL.md frontmatter must be a boolean (true or false)', + }); + } const { errors, warnings } = partitionIssues(issues); if (errors.length > 0) { const error = new Error('Skill validation failed'); @@ -901,6 +927,7 @@ export function createSkillMethods(mongoose: typeof import('mongoose'), deps: Sk data.frontmatter, data.body, false, + bodyAlwaysApply, ), tenantId: data.tenantId, ...derived, @@ -1128,6 +1155,12 @@ export function createSkillMethods(mongoose: typeof import('mongoose'), deps: Sk return { status: 'not_found' }; } + /* Parse body's always-apply status once — reused for validation + (precedence-aware, below) and the derivation cascade further + down. Avoids parsing the same YAML frontmatter block twice per + update. */ + const bodyAlwaysApply = + update.body !== undefined ? extractAlwaysApplyFromBody(update.body) : undefined; const issues: ValidationIssue[] = []; if (update.name !== undefined) issues.push(...validateSkillName(update.name)); if (update.description !== undefined) @@ -1138,7 +1171,23 @@ export function createSkillMethods(mongoose: typeof import('mongoose'), deps: Sk if (update.frontmatter !== undefined) issues.push(...validateSkillFrontmatter(update.frontmatter)); if (update.alwaysApply !== undefined) issues.push(...validateAlwaysApply(update.alwaysApply)); - if (update.body !== undefined) issues.push(...validateAlwaysApplyInBody(update.body)); + /* Body-level `always-apply:` only needs to be well-formed when a + higher-precedence source won't override it (see + `resolveAlwaysApplyFromInput` for precedence). Rejecting a typo + the caller is already overriding would be user-hostile, and the + body-inline derivation branch below is skipped for those + payloads anyway. */ + if ( + bodyAlwaysApply?.status === 'invalid' && + update.alwaysApply === undefined && + typeof update.frontmatter?.['always-apply'] !== 'boolean' + ) { + issues.push({ + field: 'body.frontmatter.always-apply', + code: 'INVALID_TYPE', + message: '"always-apply" in SKILL.md frontmatter must be a boolean (true or false)', + }); + } const { errors, warnings } = partitionIssues(issues); if (errors.length > 0) { const error = new Error('Skill validation failed'); @@ -1207,11 +1256,10 @@ export function createSkillMethods(mongoose: typeof import('mongoose'), deps: Sk derivedAlwaysApply = fromFrontmatter; } } - if (derivedAlwaysApply === undefined && update.body !== undefined) { - const fromBody = extractAlwaysApplyFromBody(update.body); - if (fromBody.status === 'valid') { - derivedAlwaysApply = fromBody.value; - } else if (fromBody.status === 'absent') { + if (derivedAlwaysApply === undefined && bodyAlwaysApply !== undefined) { + if (bodyAlwaysApply.status === 'valid') { + derivedAlwaysApply = bodyAlwaysApply.value; + } else if (bodyAlwaysApply.status === 'absent') { /* An `absent` result means the user submitted a new body that declares no `always-apply:` key (either the key was removed or no frontmatter block was ever there). The body is the diff --git a/packages/data-schemas/src/schema/message.ts b/packages/data-schemas/src/schema/message.ts index eb512129a9..d68edd0df3 100644 --- a/packages/data-schemas/src/schema/message.ts +++ b/packages/data-schemas/src/schema/message.ts @@ -124,7 +124,7 @@ const messageSchema: Schema = new Schema( attachments: { type: [{ type: mongoose.Schema.Types.Mixed }], default: undefined }, /** * Skill names the user invoked manually via the `$` popover on this turn. - * UI metadata only — `ManualSkillPills` on the frontend renders these on + * UI metadata only — `SkillPills` on the frontend renders these on * the user message bubble so the selection persists through reload and * shows in history. Runtime skill resolution lives separately on the * request body, not on the message itself. diff --git a/packages/data-schemas/src/types/message.ts b/packages/data-schemas/src/types/message.ts index 5eb47b8e25..cf2213d88b 100644 --- a/packages/data-schemas/src/types/message.ts +++ b/packages/data-schemas/src/types/message.ts @@ -44,7 +44,7 @@ export interface IMessage extends Document { encoding?: string; }; attachments?: unknown[]; - /** Skills the user invoked manually via the `$` popover on this turn. UI-only metadata for `ManualSkillPills`. */ + /** Skills the user invoked manually via the `$` popover on this turn. UI-only metadata for `SkillPills`. */ manualSkills?: string[]; /** * Skills auto-primed on this turn via `always-apply` frontmatter. Persisted