🛡️ feat: Persist & enforce disable-model-invocation / user-invocable / allowed-tools

client/src/components/Chat/Input/SkillsCommand.tsx

@@ -2,7 +2,6 @@ import { memo, useState, useRef, useEffect, useMemo, useCallback } from 'react';
 import { ScrollText } from 'lucide-react';
 import { AutoSizer, List } from 'react-virtualized';
 import { Spinner, useCombobox } from '@librechat/client';
-import { InvocationMode } from 'librechat-data-provider';
 import { useRecoilValue, useSetRecoilState } from 'recoil';
 import type { TSkillSummary } from 'librechat-data-provider';
 import type { MentionOption } from '~/common';
@@ -22,16 +21,13 @@ const skillIcon = <ScrollText className="icon-md text-cyan-500" />;
 
 /**
  * Determines whether a skill should appear in the `$` command popover.
- * `manual` and `both` are user-invocable. `auto` is model-only and hidden.
- * Skills without an explicit mode (undefined) default to visible for
- * backward compatibility until the backend persists `invocationMode`.
+ * Reads the persisted `userInvocable` field (mirrors the `user-invocable`
+ * frontmatter). Defaults to visible when the field is absent so older
+ * skills authored before Phase 6 stay user-invocable without a migration;
+ * only an explicit `false` hides them.
  */
 export function isUserInvocable(skill: TSkillSummary): boolean {
-  const mode = skill.invocationMode;
-  if (mode == null || mode === InvocationMode.both) {
-    return true;
-  }
-  return mode === InvocationMode.manual;
+  return skill.userInvocable !== false;
 }
 
 /**

client/src/components/Chat/Input/__tests__/SkillsCommand.spec.tsx

@@ -14,7 +14,6 @@ import React from 'react';
 import { act } from '@testing-library/react';
 import userEvent from '@testing-library/user-event';
 import { render, screen } from '@testing-library/react';
-import { InvocationMode } from 'librechat-data-provider';
 import type { TSkillSummary } from 'librechat-data-provider';
 
 const CONVO_ID = 'convo-1';
@@ -412,7 +411,7 @@ describe('filterSkillsForPopover', () => {
   const inactive = () => false;
   const s1 = makeSkill({ _id: '1', name: 'a' });
   const s2 = makeSkill({ _id: '2', name: 'b' });
-  const s3 = makeSkill({ _id: '3', name: 'c', invocationMode: InvocationMode.auto });
+  const s3 = makeSkill({ _id: '3', name: 'c', userInvocable: false });
 
   it('passes everything through when agentSkillIds is undefined', () => {
     const out = filterSkillsForPopover([s1, s2], { agentSkillIds: undefined, isActive: active });
@@ -440,7 +439,7 @@ describe('filterSkillsForPopover', () => {
     expect(out.map((s) => s._id)).toEqual(['2']);
   });
 
-  it('excludes auto-only skills via isUserInvocable', () => {
+  it('excludes skills with userInvocable: false via isUserInvocable', () => {
     const out = filterSkillsForPopover([s1, s3], { agentSkillIds: null, isActive: active });
     expect(out.map((s) => s._id)).toEqual(['1']);
   });
@@ -456,8 +455,8 @@ describe('filterSkillsForPopover', () => {
       agentSkillIds: ['1', '2', '3'],
       isActive,
     });
-    /* s1 passes (active, manual-by-default, scoped), s2 drops (inactive),
-       s3 drops (auto-only invocation mode). */
+    /* s1 passes (active, user-invocable by default, scoped), s2 drops (inactive),
+       s3 drops (userInvocable: false). */
     expect(out.map((s) => s._id)).toEqual(['1']);
   });
 

packages/api/src/agents/__tests__/skills.test.ts

@@ -38,6 +38,7 @@ import {
   extractManualSkills,
   isSkillPrimeMessage,
   buildSkillPrimeContentParts,
+  unionPrimeAllowedTools,
   MAX_MANUAL_SKILLS,
 } from '../skills';
 import { extractInvokedSkillsFromPayload } from '../run';
@@ -47,6 +48,8 @@ type PageSkill = {
   name: string;
   description: string;
   author: Types.ObjectId;
+  disableModelInvocation?: boolean;
+  userInvocable?: boolean;
 };
 
 describe('extractInvokedSkillsFromPayload', () => {
@@ -570,6 +573,36 @@ describe('injectSkillCatalog', () => {
     expect(result.skillCount).toBe(0);
     expect(result.activeSkillIds).toEqual([]);
   });
+
+  it('excludes skills with disableModelInvocation=true from the catalog', async () => {
+    const open = makeSkill('open-skill', userObjectId);
+    const modelHidden: PageSkill = {
+      ...makeSkill('hidden-skill', userObjectId),
+      disableModelInvocation: true,
+    };
+    const listSkillsByAccess = buildPager([[open, modelHidden]]);
+    const agent = makeAgent();
+    const result = await injectSkillCatalog(baseParams({ listSkillsByAccess, agent }));
+    /* Only the open skill counts toward catalog injection — the model-hidden
+       one costs zero context tokens and is unreachable via the `skill` tool. */
+    expect(result.skillCount).toBe(1);
+    expect(result.activeSkillIds.map((id) => id.toString())).toEqual([open._id.toString()]);
+    expect(agent.additional_instructions).toContain('open-skill');
+    expect(agent.additional_instructions).not.toContain('hidden-skill');
+  });
+
+  it('disableModelInvocation filter wins even when isActive would have included the skill', async () => {
+    /* Owned skill defaults to active; without the disableModelInvocation
+       short-circuit, this would land in the catalog. */
+    const ownedHidden: PageSkill = {
+      ...makeSkill('owned-hidden', userObjectId),
+      disableModelInvocation: true,
+    };
+    const listSkillsByAccess = buildPager([[ownedHidden]]);
+    const result = await injectSkillCatalog(baseParams({ listSkillsByAccess }));
+    expect(result.skillCount).toBe(0);
+    expect(result.activeSkillIds).toEqual([]);
+  });
 });
 
 describe('buildSkillPrimeMessage', () => {
@@ -605,6 +638,7 @@ describe('resolveManualSkills', () => {
     body: string;
     author: Types.ObjectId;
     allowedTools?: string[];
+    userInvocable?: boolean;
   };
 
   const buildGetSkillByName =
@@ -700,6 +734,36 @@ describe('resolveManualSkills', () => {
     expect(result).toEqual([{ name: 'real', body: 'body of real' }]);
   });
 
+  it('silently skips skills with userInvocable: false, preserving the rest of the batch', async () => {
+    const open = mkSkill('open', userOid);
+    const modelOnly: SkillDoc = { ...mkSkill('model-only', userOid), userInvocable: false };
+    const result = await resolveManualSkills({
+      names: ['open', 'model-only'],
+      getSkillByName: buildGetSkillByName({ open, 'model-only': modelOnly }),
+      accessibleSkillIds: [open._id, modelOnly._id],
+      userId,
+    });
+    /* Defense-in-depth: even if a popover-bypassing API caller names a
+       userInvocable:false skill, the resolver drops it with a warn log
+       rather than priming SKILL.md. The rest of the batch survives. */
+    expect(result).toEqual([{ name: 'open', body: 'body of open' }]);
+  });
+
+  it('treats userInvocable: true (or absent) as user-invocable', async () => {
+    const explicit: SkillDoc = { ...mkSkill('explicit', userOid), userInvocable: true };
+    const implicit = mkSkill('implicit', userOid);
+    const result = await resolveManualSkills({
+      names: ['explicit', 'implicit'],
+      getSkillByName: buildGetSkillByName({ explicit, implicit }),
+      accessibleSkillIds: [explicit._id, implicit._id],
+      userId,
+    });
+    expect(result).toEqual([
+      { name: 'explicit', body: 'body of explicit' },
+      { name: 'implicit', body: 'body of implicit' },
+    ]);
+  });
+
   it('dedupes repeated names, preserving first occurrence order', async () => {
     const a = mkSkill('a', userOid);
     const b = mkSkill('b', userOid);
@@ -1083,3 +1147,77 @@ describe('buildSkillPrimeContentParts', () => {
     expect(first[0].tool_call.id).not.toBe(second[0].tool_call.id);
   });
 });
+
+describe('unionPrimeAllowedTools', () => {
+  it('returns no extras when no primes carry allowedTools', () => {
+    const result = unionPrimeAllowedTools({
+      primes: [{ name: 'a' }, { name: 'b', allowedTools: [] }],
+      agentToolNames: ['web_search'],
+    });
+    expect(result.extraToolNames).toEqual([]);
+    expect(result.perSkillExtras.size).toBe(0);
+  });
+
+  it('skips tools already configured on the agent (agent baseline wins)', () => {
+    const result = unionPrimeAllowedTools({
+      primes: [{ name: 'skill-a', allowedTools: ['web_search', 'execute_code'] }],
+      agentToolNames: ['web_search'],
+    });
+    /* web_search is already on the agent — only execute_code is "extra". */
+    expect(result.extraToolNames).toEqual(['execute_code']);
+    expect(result.perSkillExtras.get('skill-a')).toEqual(['execute_code']);
+  });
+
+  it('dedupes across skills (same tool requested by two primes counts once)', () => {
+    const result = unionPrimeAllowedTools({
+      primes: [
+        { name: 'skill-a', allowedTools: ['execute_code', 'read_file'] },
+        { name: 'skill-b', allowedTools: ['execute_code', 'web_search'] },
+      ],
+      agentToolNames: [],
+    });
+    expect(result.extraToolNames.sort()).toEqual(['execute_code', 'read_file', 'web_search']);
+    /* Per-skill attribution credits the FIRST skill that contributed
+       a tool name — keeps debug logs stable instead of duplicating. */
+    expect(result.perSkillExtras.get('skill-a')).toEqual(['execute_code', 'read_file']);
+    expect(result.perSkillExtras.get('skill-b')).toEqual(['web_search']);
+  });
+
+  it('filters out empty / non-string entries (defensive — frontmatter parser should already)', () => {
+    const result = unionPrimeAllowedTools({
+      primes: [
+        {
+          name: 'skill-a',
+          allowedTools: ['execute_code', '', 42 as unknown as string, 'web_search'],
+        },
+      ],
+      agentToolNames: [],
+    });
+    expect(result.extraToolNames).toEqual(['execute_code', 'web_search']);
+  });
+
+  it('omits skills with no contribution from the per-skill map', () => {
+    const result = unionPrimeAllowedTools({
+      primes: [
+        { name: 'skill-on-agent', allowedTools: ['web_search'] },
+        { name: 'skill-extra', allowedTools: ['execute_code'] },
+      ],
+      agentToolNames: ['web_search'],
+    });
+    expect(result.extraToolNames).toEqual(['execute_code']);
+    expect(result.perSkillExtras.get('skill-on-agent')).toBeUndefined();
+    expect(result.perSkillExtras.get('skill-extra')).toEqual(['execute_code']);
+  });
+
+  it('preserves first-occurrence order across skills (deterministic for log readability)', () => {
+    const result = unionPrimeAllowedTools({
+      primes: [
+        { name: 'a', allowedTools: ['z-tool'] },
+        { name: 'b', allowedTools: ['m-tool', 'a-tool'] },
+        { name: 'c', allowedTools: ['z-tool', 'b-tool'] },
+      ],
+      agentToolNames: [],
+    });
+    expect(result.extraToolNames).toEqual(['z-tool', 'm-tool', 'a-tool', 'b-tool']);
+  });
+});

packages/api/src/agents/handlers.spec.ts

@@ -175,4 +175,76 @@ describe('createToolExecuteHandler', () => {
       expect(capturedConfigs[0]._injected_files).toBeUndefined();
     });
   });
+
+  describe('skill tool model-invocation gate', () => {
+    function createSkillHandler(getSkillByName: ToolExecuteOptions['getSkillByName']) {
+      const loadTools: ToolExecuteOptions['loadTools'] = jest.fn(async () => ({
+        loadedTools: [],
+      }));
+      return createToolExecuteHandler({ loadTools, getSkillByName });
+    }
+
+    it('rejects with a clear error when the named skill has disableModelInvocation=true', async () => {
+      const getSkillByName = jest.fn(async () => ({
+        _id: 'skill-id' as unknown as never,
+        name: 'pii-redactor',
+        body: 'restricted body',
+        fileCount: 0,
+        disableModelInvocation: true,
+      }));
+      const handler = createSkillHandler(getSkillByName);
+
+      const [result] = await invokeHandler(handler, [
+        {
+          id: 'call_skill_1',
+          name: Constants.SKILL_TOOL,
+          args: { skillName: 'pii-redactor' },
+        },
+      ]);
+
+      expect(result.status).toBe('error');
+      expect(result.errorMessage).toContain('cannot be invoked by the model');
+      expect(result.errorMessage).toContain('pii-redactor');
+    });
+
+    it('returns the regular not-accessible error when the skill itself is missing (gate runs after lookup)', async () => {
+      const getSkillByName = jest.fn(async () => null);
+      const handler = createSkillHandler(getSkillByName);
+
+      const [result] = await invokeHandler(handler, [
+        {
+          id: 'call_skill_2',
+          name: Constants.SKILL_TOOL,
+          args: { skillName: 'ghost' },
+        },
+      ]);
+
+      expect(result.status).toBe('error');
+      /* Distinct error message — operators can tell "not in catalog" apart
+         from "exists but model-blocked". */
+      expect(result.errorMessage).toContain('not found or not accessible');
+      expect(result.errorMessage).not.toContain('cannot be invoked');
+    });
+
+    it('lets through skills without disableModelInvocation set (default behavior)', async () => {
+      const getSkillByName = jest.fn(async () => ({
+        _id: 'skill-id' as unknown as never,
+        name: 'normal-skill',
+        body: 'body',
+        fileCount: 0,
+      }));
+      const handler = createSkillHandler(getSkillByName);
+
+      const [result] = await invokeHandler(handler, [
+        {
+          id: 'call_skill_3',
+          name: Constants.SKILL_TOOL,
+          args: { skillName: 'normal-skill' },
+        },
+      ]);
+
+      expect(result.status).toBe('success');
+      expect(result.content).toContain('normal-skill');
+    });
+  });
 });

packages/api/src/agents/handlers.ts

@@ -58,6 +58,13 @@ export interface ToolExecuteOptions {
     name: string;
     _id: Types.ObjectId;
     fileCount: number;
+    /**
+     * Set when the skill author opted out of model invocation. The handler
+     * rejects the call and returns an instructive error so the model knows
+     * it can't reach the skill via the `skill` tool — manual `$` invocation
+     * is still allowed and goes through `resolveManualSkills` instead.
+     */
+    disableModelInvocation?: boolean;
   } | null>;
   /** Lists files bundled with a skill (for code env priming) */
   listSkillFiles?: (skillId: Types.ObjectId | string) => Promise<SkillFileRecord[]>;
@@ -433,6 +440,23 @@ async function handleSkillToolCall(
     };
   }
 
+  /**
+   * `disable-model-invocation: true` skills are excluded from the catalog
+   * the model sees, but a model that learned the name elsewhere (stale
+   * cache, hallucinated guess) could still try to invoke it. Reject
+   * explicitly so the error message tells the model exactly why and it
+   * doesn't loop retrying. Manual `$` invocation goes through
+   * `resolveManualSkills`, which is unaffected by this flag.
+   */
+  if (skill.disableModelInvocation === true) {
+    return {
+      toolCallId: tc.id,
+      status: 'error',
+      content: '',
+      errorMessage: `Skill "${args.skillName}" cannot be invoked by the model`,
+    };
+  }
+
   let body = skill.body;
   if (args.args) {
     body = body.replace(/\$ARGUMENTS/g, args.args);