mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-06-09 17:31:19 +00:00
🧭 feat: Scope Model Spec Skills (#13522)
* feat: scope model spec skills * style: format skill catalog limit * fix: serialize model spec skill resolution * test: satisfy model spec load config typing * fix: apply model spec skills to added conversations * fix: support alwaysApply frontmatter alias * fix: address model spec skills review
This commit is contained in:
Danny Avila
GitHub
parent
3fb48021f7
commit
6357ea10c1
28 changed files with 1398 additions and 139 deletions
|
|
@ -1,11 +1,16 @@
|
|||
import mongoose from 'mongoose';
|
||||
import { v4 as uuidv4 } from 'uuid';
|
||||
import { Constants } from 'librechat-data-provider';
|
||||
import { Constants, FileSources } from 'librechat-data-provider';
|
||||
import { MongoMemoryServer } from 'mongodb-memory-server';
|
||||
import { agentSchema, createMethods } from '@librechat/data-schemas';
|
||||
import type { AgentModelParameters } from 'librechat-data-provider';
|
||||
import type {
|
||||
Agent as LibreChatAgent,
|
||||
AgentModelParameters,
|
||||
TConversation,
|
||||
} from 'librechat-data-provider';
|
||||
import type { LoadAgentParams, LoadAgentDeps } from '../load';
|
||||
import { loadAgent } from '../load';
|
||||
import { loadAddedAgent } from '../added';
|
||||
|
||||
let Agent: mongoose.Model<unknown>;
|
||||
let createAgent: ReturnType<typeof createMethods>['createAgent'];
|
||||
|
|
@ -253,6 +258,184 @@ describe('loadAgent', () => {
|
|||
expect(result?.model_parameters).not.toHaveProperty('promptPrefix');
|
||||
});
|
||||
|
||||
test('should enable full skill scope for ephemeral model spec with skills true', async () => {
|
||||
const { EPHEMERAL_AGENT_ID } = Constants;
|
||||
|
||||
const result = await loadAgent(
|
||||
{
|
||||
req: {
|
||||
user: { id: 'user123' },
|
||||
body: {},
|
||||
config: {
|
||||
config: {},
|
||||
fileStrategy: FileSources.local,
|
||||
imageOutputType: 'png',
|
||||
modelSpecs: {
|
||||
list: [
|
||||
{
|
||||
name: 'skills-on',
|
||||
label: 'Skills On',
|
||||
preset: { endpoint: 'openai', model: 'gpt-4' },
|
||||
skills: true,
|
||||
},
|
||||
],
|
||||
},
|
||||
},
|
||||
},
|
||||
spec: 'skills-on',
|
||||
agent_id: EPHEMERAL_AGENT_ID as string,
|
||||
endpoint: 'openai',
|
||||
model_parameters: { model: 'gpt-4' } as unknown as AgentModelParameters,
|
||||
},
|
||||
deps,
|
||||
);
|
||||
|
||||
expect(result?.skills_enabled).toBe(true);
|
||||
expect(result?.skills).toBeUndefined();
|
||||
});
|
||||
|
||||
test('should initialize an empty allowlist for ephemeral model spec skill names', async () => {
|
||||
const { EPHEMERAL_AGENT_ID } = Constants;
|
||||
|
||||
const result = await loadAgent(
|
||||
{
|
||||
req: {
|
||||
user: { id: 'user123' },
|
||||
body: {},
|
||||
config: {
|
||||
config: {},
|
||||
fileStrategy: FileSources.local,
|
||||
imageOutputType: 'png',
|
||||
modelSpecs: {
|
||||
list: [
|
||||
{
|
||||
name: 'scoped-skills',
|
||||
label: 'Scoped Skills',
|
||||
preset: { endpoint: 'openai', model: 'gpt-4' },
|
||||
skills: ['finance-analyst', 'brand-writer'],
|
||||
},
|
||||
],
|
||||
},
|
||||
},
|
||||
},
|
||||
spec: 'scoped-skills',
|
||||
agent_id: EPHEMERAL_AGENT_ID as string,
|
||||
endpoint: 'openai',
|
||||
model_parameters: { model: 'gpt-4' } as unknown as AgentModelParameters,
|
||||
},
|
||||
deps,
|
||||
);
|
||||
|
||||
expect(result?.skills_enabled).toBe(true);
|
||||
expect(result?.skills).toEqual([]);
|
||||
});
|
||||
|
||||
test('should enable full skill scope for added ephemeral model spec with skills true', async () => {
|
||||
const result = await loadAddedAgent(
|
||||
{
|
||||
req: {
|
||||
user: { id: 'user123' },
|
||||
config: {
|
||||
config: {},
|
||||
fileStrategy: FileSources.local,
|
||||
imageOutputType: 'png',
|
||||
modelSpecs: {
|
||||
list: [
|
||||
{
|
||||
name: 'added-skills-on',
|
||||
label: 'Added Skills On',
|
||||
preset: { endpoint: 'openai', model: 'gpt-4' },
|
||||
skills: true,
|
||||
},
|
||||
],
|
||||
},
|
||||
},
|
||||
},
|
||||
conversation: {
|
||||
endpoint: 'openai',
|
||||
model: 'gpt-4',
|
||||
spec: 'added-skills-on',
|
||||
} as unknown as TConversation,
|
||||
},
|
||||
deps,
|
||||
);
|
||||
|
||||
expect(result?.skills_enabled).toBe(true);
|
||||
expect(result?.skills).toBeUndefined();
|
||||
});
|
||||
|
||||
test('should initialize an empty allowlist for added ephemeral model spec skill names', async () => {
|
||||
const result = await loadAddedAgent(
|
||||
{
|
||||
req: {
|
||||
user: { id: 'user123' },
|
||||
config: {
|
||||
config: {},
|
||||
fileStrategy: FileSources.local,
|
||||
imageOutputType: 'png',
|
||||
modelSpecs: {
|
||||
list: [
|
||||
{
|
||||
name: 'added-scoped-skills',
|
||||
label: 'Added Scoped Skills',
|
||||
preset: { endpoint: 'openai', model: 'gpt-4' },
|
||||
skills: ['finance-analyst', 'brand-writer'],
|
||||
},
|
||||
],
|
||||
},
|
||||
},
|
||||
},
|
||||
conversation: {
|
||||
endpoint: 'openai',
|
||||
model: 'gpt-4',
|
||||
spec: 'added-scoped-skills',
|
||||
} as unknown as TConversation,
|
||||
},
|
||||
deps,
|
||||
);
|
||||
|
||||
expect(result?.skills_enabled).toBe(true);
|
||||
expect(result?.skills).toEqual([]);
|
||||
});
|
||||
|
||||
test('should apply model spec skills when added agent mirrors ephemeral primary tools', async () => {
|
||||
const { EPHEMERAL_AGENT_ID } = Constants;
|
||||
|
||||
const result = await loadAddedAgent(
|
||||
{
|
||||
req: {
|
||||
user: { id: 'user123' },
|
||||
config: {
|
||||
config: {},
|
||||
fileStrategy: FileSources.local,
|
||||
imageOutputType: 'png',
|
||||
modelSpecs: {
|
||||
list: [
|
||||
{
|
||||
name: 'mirrored-scoped-skills',
|
||||
label: 'Mirrored Scoped Skills',
|
||||
preset: { endpoint: 'openai', model: 'gpt-4' },
|
||||
skills: ['brand-writer'],
|
||||
},
|
||||
],
|
||||
},
|
||||
},
|
||||
},
|
||||
conversation: {
|
||||
endpoint: 'openai',
|
||||
model: 'gpt-4',
|
||||
spec: 'mirrored-scoped-skills',
|
||||
} as unknown as TConversation,
|
||||
primaryAgent: { id: EPHEMERAL_AGENT_ID as string, tools: ['web_search'] } as LibreChatAgent,
|
||||
},
|
||||
deps,
|
||||
);
|
||||
|
||||
expect(result?.tools).toEqual(['web_search']);
|
||||
expect(result?.skills_enabled).toBe(true);
|
||||
expect(result?.skills).toEqual([]);
|
||||
});
|
||||
|
||||
test('should handle ephemeral agent with undefined ephemeralAgent in body', async () => {
|
||||
const { EPHEMERAL_AGENT_ID } = Constants;
|
||||
|
||||
|
|
|
|||
|
|
@ -38,6 +38,7 @@ import {
|
|||
scopeSkillIds,
|
||||
resolveSkillActive,
|
||||
resolveAgentScopedSkillIds,
|
||||
resolveModelSpecSkillIds,
|
||||
injectSkillCatalog,
|
||||
buildSkillPrimeMessage,
|
||||
resolveManualSkills,
|
||||
|
|
@ -312,9 +313,11 @@ describe('resolveAgentScopedSkillIds', () => {
|
|||
});
|
||||
const ephemeralAgent = (
|
||||
skills?: string[],
|
||||
skills_enabled?: boolean,
|
||||
): { id: string; skills?: string[]; skills_enabled?: boolean } => ({
|
||||
id: 'ephemeral_convo_xyz',
|
||||
skills,
|
||||
skills_enabled,
|
||||
});
|
||||
|
||||
it('returns [] when the skills capability is disabled, even with every other signal on', () => {
|
||||
|
|
@ -366,14 +369,50 @@ describe('resolveAgentScopedSkillIds', () => {
|
|||
expect(scoped.map((o) => o.toString()).sort()).toEqual([a.toString(), b.toString()].sort());
|
||||
});
|
||||
|
||||
it('ignores any `skills` field on an ephemeral agent (toggle is the only signal)', () => {
|
||||
it('returns the full accessible catalog when a model spec enables skills', () => {
|
||||
const a = makeId();
|
||||
const b = makeId();
|
||||
const scoped = resolveAgentScopedSkillIds({
|
||||
agent: ephemeralAgent(undefined, true),
|
||||
accessibleSkillIds: [a, b],
|
||||
skillsCapabilityEnabled: true,
|
||||
ephemeralSkillsToggle: false,
|
||||
});
|
||||
expect(scoped.map((o) => o.toString()).sort()).toEqual([a.toString(), b.toString()].sort());
|
||||
});
|
||||
|
||||
it('returns the model-spec allowlist intersection when configured', () => {
|
||||
const a = makeId();
|
||||
const b = makeId();
|
||||
const scoped = resolveAgentScopedSkillIds({
|
||||
agent: ephemeralAgent([a.toString()], true),
|
||||
accessibleSkillIds: [a, b],
|
||||
skillsCapabilityEnabled: true,
|
||||
ephemeralSkillsToggle: false,
|
||||
});
|
||||
expect(scoped.map((o) => o.toString())).toEqual([a.toString()]);
|
||||
});
|
||||
|
||||
it('treats an empty model-spec allowlist as explicit none', () => {
|
||||
const a = makeId();
|
||||
expect(
|
||||
resolveAgentScopedSkillIds({
|
||||
agent: ephemeralAgent([a.toString()]),
|
||||
agent: ephemeralAgent([], true),
|
||||
accessibleSkillIds: [a],
|
||||
skillsCapabilityEnabled: true,
|
||||
ephemeralSkillsToggle: false,
|
||||
ephemeralSkillsToggle: true,
|
||||
}),
|
||||
).toEqual([]);
|
||||
});
|
||||
|
||||
it('lets an explicit model-spec skills=false override the badge toggle', () => {
|
||||
const a = makeId();
|
||||
expect(
|
||||
resolveAgentScopedSkillIds({
|
||||
agent: ephemeralAgent(undefined, false),
|
||||
accessibleSkillIds: [a],
|
||||
skillsCapabilityEnabled: true,
|
||||
ephemeralSkillsToggle: true,
|
||||
}),
|
||||
).toEqual([]);
|
||||
});
|
||||
|
|
@ -470,6 +509,95 @@ describe('resolveAgentScopedSkillIds', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('resolveModelSpecSkillIds', () => {
|
||||
const userObjectId = new Types.ObjectId();
|
||||
|
||||
it('resolves configured names against accessible skills and skips misses without failing', async () => {
|
||||
const knownId = new Types.ObjectId();
|
||||
const getSkillByName = jest.fn(async (name: string) => {
|
||||
if (name === 'known-skill') {
|
||||
return {
|
||||
_id: knownId,
|
||||
name,
|
||||
body: 'body',
|
||||
author: userObjectId,
|
||||
};
|
||||
}
|
||||
if (name === 'throws') {
|
||||
throw new Error('lookup failed');
|
||||
}
|
||||
return null;
|
||||
});
|
||||
|
||||
const result = await resolveModelSpecSkillIds({
|
||||
names: [' known-skill ', 'missing-skill', 'throws', 'known-skill'],
|
||||
accessibleSkillIds: [knownId],
|
||||
getSkillByName,
|
||||
});
|
||||
|
||||
expect(result.map((id) => id.toString())).toEqual([knownId.toString()]);
|
||||
expect(getSkillByName).toHaveBeenCalledTimes(3);
|
||||
expect(getSkillByName).toHaveBeenCalledWith('known-skill', [knownId], {
|
||||
preferModelInvocable: true,
|
||||
});
|
||||
});
|
||||
|
||||
it('resolves configured names sequentially to avoid query bursts', async () => {
|
||||
const firstId = new Types.ObjectId();
|
||||
const secondId = new Types.ObjectId();
|
||||
const order: string[] = [];
|
||||
let releaseFirst!: () => void;
|
||||
const firstLookup = new Promise<void>((resolve) => {
|
||||
releaseFirst = resolve;
|
||||
});
|
||||
const getSkillByName = jest.fn(async (name: string) => {
|
||||
order.push(`start:${name}`);
|
||||
if (name === 'first') {
|
||||
await firstLookup;
|
||||
order.push(`end:${name}`);
|
||||
return {
|
||||
_id: firstId,
|
||||
name,
|
||||
body: 'body',
|
||||
author: userObjectId,
|
||||
};
|
||||
}
|
||||
order.push(`end:${name}`);
|
||||
return {
|
||||
_id: secondId,
|
||||
name,
|
||||
body: 'body',
|
||||
author: userObjectId,
|
||||
};
|
||||
});
|
||||
|
||||
const promise = resolveModelSpecSkillIds({
|
||||
names: ['first', 'second'],
|
||||
accessibleSkillIds: [firstId, secondId],
|
||||
getSkillByName,
|
||||
});
|
||||
|
||||
await Promise.resolve();
|
||||
expect(order).toEqual(['start:first']);
|
||||
|
||||
releaseFirst();
|
||||
const result = await promise;
|
||||
|
||||
expect(result.map((id) => id.toString())).toEqual([firstId.toString(), secondId.toString()]);
|
||||
expect(order).toEqual(['start:first', 'end:first', 'start:second', 'end:second']);
|
||||
});
|
||||
|
||||
it('returns [] when no skill lookup is available', async () => {
|
||||
const result = await resolveModelSpecSkillIds({
|
||||
names: ['known-skill'],
|
||||
accessibleSkillIds: [new Types.ObjectId()],
|
||||
getSkillByName: undefined,
|
||||
});
|
||||
|
||||
expect(result).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('resolveSkillActive', () => {
|
||||
const makeSkill = (author: Types.ObjectId) => ({ _id: new Types.ObjectId(), author });
|
||||
|
||||
|
|
@ -744,6 +872,26 @@ describe('injectSkillCatalog', () => {
|
|||
expect(agent.additional_instructions).toContain('desc-my-skill');
|
||||
});
|
||||
|
||||
it('honors a configured maxCatalogSkills below the default hard limit', async () => {
|
||||
const first = makeSkill('first-skill', userObjectId);
|
||||
const second = makeSkill('second-skill', userObjectId);
|
||||
const third = makeSkill('third-skill', userObjectId);
|
||||
const listSkillsByAccess = buildPager([[first, second, third]]);
|
||||
const agent = makeAgent();
|
||||
const result = await injectSkillCatalog(
|
||||
baseParams({ listSkillsByAccess, agent, maxCatalogSkills: 2 }),
|
||||
);
|
||||
|
||||
expect(result.skillCount).toBe(2);
|
||||
expect(result.activeSkillIds.map((id) => id.toString())).toEqual([
|
||||
first._id.toString(),
|
||||
second._id.toString(),
|
||||
]);
|
||||
expect(agent.additional_instructions).toContain('first-skill');
|
||||
expect(agent.additional_instructions).toContain('second-skill');
|
||||
expect(agent.additional_instructions).not.toContain('third-skill');
|
||||
});
|
||||
|
||||
it('fails closed when userId is absent (shared skills drop, owned would need override)', async () => {
|
||||
const owned = makeSkill('my-skill', userObjectId);
|
||||
const shared = makeSkill('shared-skill', new Types.ObjectId());
|
||||
|
|
|
|||
|
|
@ -8,13 +8,32 @@ import {
|
|||
appendAgentIdSuffix,
|
||||
encodeEphemeralAgentId,
|
||||
} from 'librechat-data-provider';
|
||||
import type { Agent, TConversation } from 'librechat-data-provider';
|
||||
import type { Agent, TConversation, TModelSpec } from 'librechat-data-provider';
|
||||
import { getCustomEndpointConfig } from '~/app/config';
|
||||
|
||||
const { mcp_all, mcp_delimiter } = Constants;
|
||||
|
||||
export const ADDED_AGENT_ID = 'added_agent';
|
||||
|
||||
function applyModelSpecSkills(
|
||||
result: Record<string, unknown>,
|
||||
modelSpec: Pick<TModelSpec, 'skills'> | null | undefined,
|
||||
): void {
|
||||
if (!modelSpec || !Object.prototype.hasOwnProperty.call(modelSpec, 'skills')) {
|
||||
return;
|
||||
}
|
||||
if (modelSpec.skills === true) {
|
||||
result.skills_enabled = true;
|
||||
delete result.skills;
|
||||
} else if (modelSpec.skills === false) {
|
||||
result.skills_enabled = false;
|
||||
result.skills = [];
|
||||
} else if (Array.isArray(modelSpec.skills)) {
|
||||
result.skills_enabled = true;
|
||||
result.skills = [];
|
||||
}
|
||||
}
|
||||
|
||||
export interface LoadAddedAgentDeps {
|
||||
getAgent: (searchParameter: { id: string }) => Promise<Agent | null>;
|
||||
getMCPServerTools: (
|
||||
|
|
@ -95,8 +114,7 @@ export async function loadAddedAgent(
|
|||
}
|
||||
}
|
||||
|
||||
const modelSpecs = (appConfig?.modelSpecs as { list?: Array<{ name: string; label?: string }> })
|
||||
?.list;
|
||||
const modelSpecs = (appConfig?.modelSpecs as { list?: TModelSpec[] })?.list;
|
||||
const modelSpec = spec != null && spec !== '' ? modelSpecs?.find((s) => s.name === spec) : null;
|
||||
const sender =
|
||||
rest.modelLabel ??
|
||||
|
|
@ -105,14 +123,16 @@ export async function loadAddedAgent(
|
|||
'';
|
||||
const ephemeralId = encodeEphemeralAgentId({ endpoint, model, sender, index: 1 });
|
||||
|
||||
return {
|
||||
const result: Record<string, unknown> = {
|
||||
id: ephemeralId,
|
||||
instructions: promptPrefix || '',
|
||||
provider: endpoint,
|
||||
model_parameters: {},
|
||||
model,
|
||||
tools: [...primaryAgent.tools],
|
||||
} as unknown as Agent;
|
||||
};
|
||||
applyModelSpecSkills(result, modelSpec);
|
||||
return result as unknown as Agent;
|
||||
}
|
||||
|
||||
const ephemeralAgent = rest.ephemeralAgent as
|
||||
|
|
@ -127,18 +147,7 @@ export async function loadAddedAgent(
|
|||
const mcpServers = new Set<string>(ephemeralAgent?.mcp);
|
||||
const userId = req.user?.id ?? '';
|
||||
|
||||
const modelSpecs = (
|
||||
appConfig?.modelSpecs as {
|
||||
list?: Array<{
|
||||
name: string;
|
||||
label?: string;
|
||||
mcpServers?: string[];
|
||||
executeCode?: boolean;
|
||||
fileSearch?: boolean;
|
||||
webSearch?: boolean;
|
||||
}>;
|
||||
}
|
||||
)?.list;
|
||||
const modelSpecs = (appConfig?.modelSpecs as { list?: TModelSpec[] })?.list;
|
||||
let modelSpec: (typeof modelSpecs extends Array<infer T> | undefined ? T : never) | null = null;
|
||||
if (spec != null && spec !== '') {
|
||||
modelSpec = modelSpecs?.find((s) => s.name === spec) ?? null;
|
||||
|
|
@ -225,6 +234,7 @@ export async function loadAddedAgent(
|
|||
if (ephemeralAgent?.artifacts != null && ephemeralAgent.artifacts) {
|
||||
result.artifacts = ephemeralAgent.artifacts;
|
||||
}
|
||||
applyModelSpecSkills(result, modelSpec);
|
||||
|
||||
return result as unknown as Agent;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -70,6 +70,13 @@ function appendAdditionalInstructions(agent: Agent, text?: string | null): void
|
|||
.join('\n\n');
|
||||
}
|
||||
|
||||
function getMaxCatalogSkills(req: ServerRequest): number | undefined {
|
||||
const endpoints = req.config?.endpoints as
|
||||
| Record<string, { skills?: { maxCatalogSkills?: number } } | undefined>
|
||||
| undefined;
|
||||
return endpoints?.[EModelEndpoint.agents]?.skills?.maxCatalogSkills;
|
||||
}
|
||||
|
||||
function getToolName(tool: unknown): string | undefined {
|
||||
if (tool == null || typeof tool !== 'object') {
|
||||
return undefined;
|
||||
|
|
@ -1022,6 +1029,7 @@ export async function initializeAgent(
|
|||
userId: req.user?.id,
|
||||
skillStates: params.skillStates,
|
||||
defaultActiveOnShare: params.defaultActiveOnShare,
|
||||
maxCatalogSkills: getMaxCatalogSkills(req),
|
||||
});
|
||||
toolDefinitions = skillResult.toolDefinitions;
|
||||
skillCount = skillResult.skillCount;
|
||||
|
|
|
|||
|
|
@ -135,6 +135,17 @@ export async function loadEphemeralAgent(
|
|||
if (ephemeralAgent?.artifacts) {
|
||||
result.artifacts = ephemeralAgent.artifacts;
|
||||
}
|
||||
if (modelSpec && Object.prototype.hasOwnProperty.call(modelSpec, 'skills')) {
|
||||
if (modelSpec.skills === true) {
|
||||
result.skills_enabled = true;
|
||||
} else if (modelSpec.skills === false) {
|
||||
result.skills_enabled = false;
|
||||
result.skills = [];
|
||||
} else if (Array.isArray(modelSpec.skills)) {
|
||||
result.skills_enabled = true;
|
||||
result.skills = [];
|
||||
}
|
||||
}
|
||||
return result as Agent;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -11,10 +11,13 @@ import { registerCodeExecutionTools } from './tools';
|
|||
import { logAxiosError } from '~/utils';
|
||||
|
||||
const SKILL_CATALOG_LIMIT = 100;
|
||||
const MIN_SKILL_CATALOG_LIMIT = 1;
|
||||
/** Max pages scanned per run when filtering out inactive skills. */
|
||||
const MAX_CATALOG_PAGES = 10;
|
||||
/** Page size used when paginating to fill the active-skill quota. */
|
||||
const CATALOG_PAGE_SIZE = 100;
|
||||
/** Hard ceiling on skill names a model spec can request by config. */
|
||||
const MAX_MODEL_SPEC_SKILLS = SKILL_CATALOG_LIMIT;
|
||||
/**
|
||||
* Hard ceiling on skill names resolved per request via `$` popover or
|
||||
* `always-apply`. The popover realistically surfaces only a few per turn;
|
||||
|
|
@ -123,6 +126,96 @@ export function scopeSkillIds(
|
|||
return accessibleSkillIds.filter((oid) => agentSet.has(oid.toString()));
|
||||
}
|
||||
|
||||
function normalizeSkillCatalogLimit(limit: number | undefined): number {
|
||||
if (typeof limit !== 'number' || !Number.isFinite(limit)) {
|
||||
return SKILL_CATALOG_LIMIT;
|
||||
}
|
||||
return Math.min(SKILL_CATALOG_LIMIT, Math.max(MIN_SKILL_CATALOG_LIMIT, Math.floor(limit)));
|
||||
}
|
||||
|
||||
export interface ResolveModelSpecSkillIdsParams {
|
||||
/** Skill names configured on a model spec. */
|
||||
names: string[];
|
||||
/** Full VIEW-accessible skill IDs for this user before model-spec scoping. */
|
||||
accessibleSkillIds: Types.ObjectId[];
|
||||
/** DB lookup: name → skill doc constrained to the user's accessible IDs. */
|
||||
getSkillByName?: InitializeAgentDbMethods['getSkillByName'];
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolves model-spec skill names against the current user's accessible skill
|
||||
* set. Config is advisory: unrecognized, inaccessible, malformed, or errored
|
||||
* names are skipped with a warning so a stale model-spec entry never blocks
|
||||
* the actual chat request.
|
||||
*/
|
||||
export async function resolveModelSpecSkillIds({
|
||||
names,
|
||||
accessibleSkillIds,
|
||||
getSkillByName,
|
||||
}: ResolveModelSpecSkillIdsParams): Promise<Types.ObjectId[]> {
|
||||
if (!names.length || accessibleSkillIds.length === 0 || !getSkillByName) {
|
||||
return [];
|
||||
}
|
||||
|
||||
const seenNames = new Set<string>();
|
||||
const uniqueNames: string[] = [];
|
||||
for (const name of names) {
|
||||
if (typeof name !== 'string') {
|
||||
continue;
|
||||
}
|
||||
const trimmed = name.trim();
|
||||
if (!trimmed || trimmed.length > MAX_SKILL_NAME_LENGTH || seenNames.has(trimmed)) {
|
||||
continue;
|
||||
}
|
||||
seenNames.add(trimmed);
|
||||
uniqueNames.push(trimmed);
|
||||
}
|
||||
|
||||
let boundedNames = uniqueNames;
|
||||
if (uniqueNames.length > MAX_MODEL_SPEC_SKILLS) {
|
||||
logger.warn(
|
||||
`[resolveModelSpecSkillIds] Truncating model spec skill list from ${uniqueNames.length} to ${MAX_MODEL_SPEC_SKILLS}.`,
|
||||
);
|
||||
boundedNames = uniqueNames.slice(0, MAX_MODEL_SPEC_SKILLS);
|
||||
}
|
||||
|
||||
const resolved: Array<Types.ObjectId | null> = [];
|
||||
for (const name of boundedNames) {
|
||||
try {
|
||||
const skill = await getSkillByName(name, accessibleSkillIds, {
|
||||
preferModelInvocable: true,
|
||||
});
|
||||
if (!skill) {
|
||||
logger.warn(
|
||||
`[resolveModelSpecSkillIds] Skill "${name}" not found or not accessible for this user`,
|
||||
);
|
||||
resolved.push(null);
|
||||
continue;
|
||||
}
|
||||
resolved.push(skill._id);
|
||||
} catch (err) {
|
||||
logger.warn(
|
||||
`[resolveModelSpecSkillIds] Failed to resolve skill "${name}":`,
|
||||
err instanceof Error ? err.message : err,
|
||||
);
|
||||
resolved.push(null);
|
||||
}
|
||||
}
|
||||
|
||||
const seenIds = new Set<string>();
|
||||
return resolved.filter((id): id is Types.ObjectId => {
|
||||
if (!id) {
|
||||
return false;
|
||||
}
|
||||
const key = id.toString();
|
||||
if (seenIds.has(key)) {
|
||||
return false;
|
||||
}
|
||||
seenIds.add(key);
|
||||
return true;
|
||||
});
|
||||
}
|
||||
|
||||
export interface ResolveAgentScopedSkillIdsParams {
|
||||
/** Agent being initialized. Reads `id`, `skills`, and `skills_enabled`. */
|
||||
agent: Pick<Agent, 'id' | 'skills' | 'skills_enabled'>;
|
||||
|
|
@ -137,8 +230,10 @@ export interface ResolveAgentScopedSkillIdsParams {
|
|||
/**
|
||||
* Strict opt-in resolver for per-agent skill scope. Activation requires an
|
||||
* explicit signal from the user or the agent author:
|
||||
* - Ephemeral agent → the skills badge toggle for this conversation.
|
||||
* Toggle ON = full accessible catalog; OFF = no skills.
|
||||
* - Ephemeral agent → model-spec `skills` wins when configured:
|
||||
* `true` = full accessible catalog, string list = scoped allowlist,
|
||||
* `false` = no skills. Otherwise the skills badge toggle controls
|
||||
* the full accessible catalog.
|
||||
* - Persisted agent → the builder's `skills_enabled` master switch.
|
||||
* Enabled + empty allowlist = full catalog; enabled + non-empty
|
||||
* allowlist = narrow to those ids; disabled (or undefined) = no skills.
|
||||
|
|
@ -158,6 +253,14 @@ export function resolveAgentScopedSkillIds(
|
|||
return [];
|
||||
}
|
||||
if (isEphemeralAgentId(agent.id)) {
|
||||
if (agent.skills_enabled === false) {
|
||||
return [];
|
||||
}
|
||||
if (agent.skills_enabled === true) {
|
||||
return Array.isArray(agent.skills)
|
||||
? scopeSkillIds(accessibleSkillIds, agent.skills)
|
||||
: scopeSkillIds(accessibleSkillIds, undefined);
|
||||
}
|
||||
return ephemeralSkillsToggle ? scopeSkillIds(accessibleSkillIds, undefined) : [];
|
||||
}
|
||||
if (agent.skills_enabled !== true) {
|
||||
|
|
@ -217,6 +320,8 @@ export interface InjectSkillCatalogParams {
|
|||
skillStates?: Record<string, boolean>;
|
||||
/** Admin-configured default for shared skills. `true` = shared skills auto-activate. */
|
||||
defaultActiveOnShare?: boolean;
|
||||
/** Admin-configured cap on the model-visible catalog. Defaults to 100. */
|
||||
maxCatalogSkills?: number;
|
||||
}
|
||||
|
||||
export interface InjectSkillCatalogResult {
|
||||
|
|
@ -268,7 +373,9 @@ export async function injectSkillCatalog(
|
|||
userId,
|
||||
skillStates,
|
||||
defaultActiveOnShare = false,
|
||||
maxCatalogSkills,
|
||||
} = params;
|
||||
const catalogLimit = normalizeSkillCatalogLimit(maxCatalogSkills);
|
||||
|
||||
if (!listSkillsByAccess || accessibleSkillIds.length === 0) {
|
||||
return {
|
||||
|
|
@ -299,7 +406,7 @@ export async function injectSkillCatalog(
|
|||
let pages = 0;
|
||||
let reachedEnd = false;
|
||||
|
||||
while (visibleCount < SKILL_CATALOG_LIMIT && pages < MAX_CATALOG_PAGES) {
|
||||
while (visibleCount < catalogLimit && pages < MAX_CATALOG_PAGES) {
|
||||
const page = await listSkillsByAccess({
|
||||
accessibleIds: accessibleSkillIds,
|
||||
limit: CATALOG_PAGE_SIZE,
|
||||
|
|
@ -307,7 +414,7 @@ export async function injectSkillCatalog(
|
|||
});
|
||||
|
||||
for (const skill of page.skills) {
|
||||
if (visibleCount >= SKILL_CATALOG_LIMIT) {
|
||||
if (visibleCount >= catalogLimit) {
|
||||
break;
|
||||
}
|
||||
/**
|
||||
|
|
@ -344,9 +451,9 @@ export async function injectSkillCatalog(
|
|||
};
|
||||
}
|
||||
|
||||
if (!reachedEnd && visibleCount < SKILL_CATALOG_LIMIT) {
|
||||
if (!reachedEnd && visibleCount < catalogLimit) {
|
||||
logger.warn(
|
||||
`[injectSkillCatalog] Scanned ${MAX_CATALOG_PAGES} pages without filling the ${SKILL_CATALOG_LIMIT}-skill catalog. Some active skills may be excluded.`,
|
||||
`[injectSkillCatalog] Scanned ${MAX_CATALOG_PAGES} pages without filling the ${catalogLimit}-skill catalog. Some active skills may be excluded.`,
|
||||
);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -170,18 +170,24 @@ export function sanitizeModelSpecs<T extends Partial<TSpecsConfig> | null | unde
|
|||
return {
|
||||
...modelSpecs,
|
||||
list: modelSpecs.list.map((modelSpec) => {
|
||||
const preset = modelSpec?.preset;
|
||||
if (!preset || typeof preset !== 'object') {
|
||||
if (!modelSpec || typeof modelSpec !== 'object') {
|
||||
return modelSpec;
|
||||
}
|
||||
|
||||
const preset = modelSpec?.preset;
|
||||
const sanitizedModelSpec = { ...modelSpec };
|
||||
delete (sanitizedModelSpec as { skills?: unknown }).skills;
|
||||
if (!preset || typeof preset !== 'object') {
|
||||
return sanitizedModelSpec;
|
||||
}
|
||||
|
||||
const sanitizedPreset = { ...preset };
|
||||
for (const field of PRIVATE_MODEL_SPEC_PRESET_FIELDS) {
|
||||
delete sanitizedPreset[field];
|
||||
}
|
||||
|
||||
return {
|
||||
...modelSpec,
|
||||
...sanitizedModelSpec,
|
||||
preset: sanitizedPreset,
|
||||
};
|
||||
}),
|
||||
|
|
|
|||
|
|
@ -17,6 +17,7 @@ describe('modelSpecs helpers', () => {
|
|||
{
|
||||
name: 'guarded-spec',
|
||||
label: 'Guarded Spec',
|
||||
skills: ['private-skill'],
|
||||
preset: {
|
||||
endpoint: EModelEndpoint.openAI,
|
||||
model: 'gpt-4o',
|
||||
|
|
@ -32,11 +33,13 @@ describe('modelSpecs helpers', () => {
|
|||
],
|
||||
};
|
||||
|
||||
expect(sanitizeModelSpecs(modelSpecs).list[0].preset).toEqual({
|
||||
const sanitizedModelSpecs = sanitizeModelSpecs(modelSpecs);
|
||||
expect(sanitizedModelSpecs.list[0].preset).toEqual({
|
||||
endpoint: EModelEndpoint.openAI,
|
||||
model: 'gpt-4o',
|
||||
greeting: 'Hello',
|
||||
});
|
||||
expect(sanitizedModelSpecs.list[0]).not.toHaveProperty('skills');
|
||||
});
|
||||
|
||||
it('should restore only private fields for non-enforced model specs', () => {
|
||||
|
|
|
|||
|
|
@ -121,6 +121,37 @@ describe('parseFrontmatter', () => {
|
|||
});
|
||||
});
|
||||
|
||||
it('extracts alwaysApply: true', () => {
|
||||
const raw = `---\nname: legal\ndescription: Legal rules.\nalwaysApply: true\n---\n\n# Legal body`;
|
||||
expect(parseFrontmatter(raw)).toEqual({
|
||||
name: 'legal',
|
||||
description: 'Legal rules.',
|
||||
alwaysApply: true,
|
||||
invalidBooleans: [],
|
||||
});
|
||||
});
|
||||
|
||||
it('lets always-apply win when both spellings are present', () => {
|
||||
const raw = `---\nname: legal\ndescription: Legal rules.\nalways-apply: false\nalwaysApply: true\n---\n\n# Legal body`;
|
||||
expect(parseFrontmatter(raw).alwaysApply).toBe(false);
|
||||
});
|
||||
|
||||
it('ignores invalid alwaysApply alias when canonical always-apply is valid', () => {
|
||||
const raw = `---\nname: legal\ndescription: Legal rules.\nalways-apply: true\nalwaysApply: yes\n---\n\n# Legal body`;
|
||||
const result = parseFrontmatter(raw);
|
||||
|
||||
expect(result.alwaysApply).toBe(true);
|
||||
expect(result.invalidBooleans).toEqual([]);
|
||||
});
|
||||
|
||||
it('ignores invalid alwaysApply alias before a valid canonical always-apply', () => {
|
||||
const raw = `---\nname: legal\ndescription: Legal rules.\nalwaysApply: yes\nalways-apply: false\n---\n\n# Legal body`;
|
||||
const result = parseFrontmatter(raw);
|
||||
|
||||
expect(result.alwaysApply).toBe(false);
|
||||
expect(result.invalidBooleans).toEqual([]);
|
||||
});
|
||||
|
||||
it('flags non-boolean always-apply values as invalid (no silent drop)', () => {
|
||||
const raw = `---\nname: n\ndescription: d\nalways-apply: yes\n---\n\nbody`;
|
||||
const result = parseFrontmatter(raw);
|
||||
|
|
@ -128,6 +159,13 @@ describe('parseFrontmatter', () => {
|
|||
expect(result.invalidBooleans).toEqual(['always-apply']);
|
||||
});
|
||||
|
||||
it('flags non-boolean alwaysApply values as invalid (no silent drop)', () => {
|
||||
const raw = `---\nname: n\ndescription: d\nalwaysApply: yes\n---\n\nbody`;
|
||||
const result = parseFrontmatter(raw);
|
||||
expect(result.alwaysApply).toBeUndefined();
|
||||
expect(result.invalidBooleans).toEqual(['alwaysApply']);
|
||||
});
|
||||
|
||||
it('does not flag always-apply when the key is absent', () => {
|
||||
const raw = `---\nname: n\ndescription: d\n---\n\nbody`;
|
||||
expect(parseFrontmatter(raw).invalidBooleans).toEqual([]);
|
||||
|
|
|
|||
|
|
@ -64,11 +64,12 @@ function parseBooleanScalar(value: string): boolean | undefined {
|
|||
* `packages/data-schemas/src/methods/skill.ts` covers the wire contract;
|
||||
* this parser only needs to hand `createSkill` the columns it populates.
|
||||
*
|
||||
* When a known boolean field (currently just `always-apply`) is present
|
||||
* When a known boolean field (currently `always-apply` plus the accepted
|
||||
* `alwaysApply` alias) is present
|
||||
* with a value that isn't recognizable as `true`/`false`, the parser
|
||||
* records it on `invalidBooleans[]` so the import handler can surface
|
||||
* a 400 instead of silently dropping the flag. Without this signal,
|
||||
* authoring mistakes like `always-apply: yes` would be lossy-converted
|
||||
* authoring mistakes like `alwaysApply: yes` would be lossy-converted
|
||||
* to "not always-applied" and the user would never learn their
|
||||
* frontmatter was malformed.
|
||||
*
|
||||
|
|
@ -94,19 +95,23 @@ export function parseFrontmatter(raw: string): {
|
|||
let name = '';
|
||||
let description = '';
|
||||
let alwaysApply: boolean | undefined;
|
||||
const invalidBooleans: string[] = [];
|
||||
let hasValidCanonicalAlwaysApply = false;
|
||||
const invalidCanonicalAlwaysApply: string[] = [];
|
||||
const invalidAliasAlwaysApply: string[] = [];
|
||||
for (const line of block.split('\n')) {
|
||||
const colon = line.indexOf(':');
|
||||
if (colon === -1) {
|
||||
continue;
|
||||
}
|
||||
const key = line.slice(0, colon).trim().toLowerCase();
|
||||
const rawKey = line.slice(0, colon).trim();
|
||||
const key = rawKey.toLowerCase();
|
||||
const rawValue = line.slice(colon + 1).trim();
|
||||
if (key === 'name') {
|
||||
name = unquoteYaml(rawValue);
|
||||
} else if (key === 'description') {
|
||||
description = unquoteYaml(rawValue);
|
||||
} else if (key === 'always-apply') {
|
||||
} else if (key === 'always-apply' || key === 'alwaysapply') {
|
||||
const isCanonical = key === 'always-apply';
|
||||
// Operate on the raw post-colon text (no outer `unquoteYaml`): a
|
||||
// line like `always-apply: "true" # note` must have its comment
|
||||
// stripped BEFORE unquoting. Running `unquoteYaml` on the whole
|
||||
|
|
@ -122,12 +127,24 @@ export function parseFrontmatter(raw: string): {
|
|||
}
|
||||
const parsed = parseBooleanScalar(unquoteYaml(stripped));
|
||||
if (parsed === undefined) {
|
||||
invalidBooleans.push(key);
|
||||
if (isCanonical) {
|
||||
invalidCanonicalAlwaysApply.push(rawKey);
|
||||
} else {
|
||||
invalidAliasAlwaysApply.push(rawKey);
|
||||
}
|
||||
} else {
|
||||
alwaysApply = parsed;
|
||||
if (isCanonical) {
|
||||
hasValidCanonicalAlwaysApply = true;
|
||||
alwaysApply = parsed;
|
||||
} else if (!hasValidCanonicalAlwaysApply && invalidCanonicalAlwaysApply.length === 0) {
|
||||
alwaysApply = parsed;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
const invalidBooleans = hasValidCanonicalAlwaysApply
|
||||
? invalidCanonicalAlwaysApply
|
||||
: [...invalidCanonicalAlwaysApply, ...invalidAliasAlwaysApply];
|
||||
return { name, description, alwaysApply, invalidBooleans };
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue