mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-08-04 06:52:47 +00:00
🪜 chore: Plumb allowedTools through resolveManualSkills (#12744)
* 🪜 chore: Plumb `allowedTools` through `resolveManualSkills` Tiny shape-only precursor shared by Phase 5 (`always-apply`) and Phase 6 (frontmatter runtime enforcement). Adds `allowedTools?: string[]` to `ResolvedManualSkill` and widens the `getSkillByName` return type in `ResolveManualSkillsParams` and `InitializeAgentDbMethods` to carry the same field. The resolver forwards `skill.allowedTools` verbatim when present. No runtime behavior change — the field is populated but not yet consumed. Phase 6 will union the per-skill allowlist into the agent's effective tool set for the turn; Phase 5's `ResolvedAlwaysApplySkill` will mirror this shape. Landing this first eliminates the type-shape race between the two phases. * 🪜 style: Drop phase-name refs from `allowedTools` JSDoc for longevity JSDoc that cites "Phase N will X" goes stale the moment that phase ships. Swap for "future runtime enforcement" so the docs age with the code.
This commit is contained in:
parent
539c4c7e4d
commit
bb13d5b9ee
3 changed files with 65 additions and 1 deletions
|
|
@ -604,6 +604,7 @@ describe('resolveManualSkills', () => {
|
|||
name: string;
|
||||
body: string;
|
||||
author: Types.ObjectId;
|
||||
allowedTools?: string[];
|
||||
};
|
||||
|
||||
const buildGetSkillByName =
|
||||
|
|
@ -649,6 +650,45 @@ describe('resolveManualSkills', () => {
|
|||
expect(result).toEqual([{ name: 'my-skill', body: 'MY SKILL BODY' }]);
|
||||
});
|
||||
|
||||
it('passes allowedTools through when the skill doc carries the field', async () => {
|
||||
const owned: SkillDoc = {
|
||||
...mkSkill('with-tools', userOid, 'body'),
|
||||
allowedTools: ['execute_code', 'read_file'],
|
||||
};
|
||||
const result = await resolveManualSkills({
|
||||
names: ['with-tools'],
|
||||
getSkillByName: buildGetSkillByName({ 'with-tools': owned }),
|
||||
accessibleSkillIds: [owned._id],
|
||||
userId,
|
||||
});
|
||||
expect(result).toEqual([
|
||||
{ name: 'with-tools', body: 'body', allowedTools: ['execute_code', 'read_file'] },
|
||||
]);
|
||||
});
|
||||
|
||||
it('omits allowedTools when the skill doc does not declare it', async () => {
|
||||
const owned = mkSkill('no-tools', userOid, 'body');
|
||||
const [resolved] = await resolveManualSkills({
|
||||
names: ['no-tools'],
|
||||
getSkillByName: buildGetSkillByName({ 'no-tools': owned }),
|
||||
accessibleSkillIds: [owned._id],
|
||||
userId,
|
||||
});
|
||||
expect(resolved).toEqual({ name: 'no-tools', body: 'body' });
|
||||
expect(resolved).not.toHaveProperty('allowedTools');
|
||||
});
|
||||
|
||||
it('preserves an empty allowedTools array (distinguishes "declared none" from "undeclared")', async () => {
|
||||
const owned: SkillDoc = { ...mkSkill('empty-tools', userOid, 'body'), allowedTools: [] };
|
||||
const [resolved] = await resolveManualSkills({
|
||||
names: ['empty-tools'],
|
||||
getSkillByName: buildGetSkillByName({ 'empty-tools': owned }),
|
||||
accessibleSkillIds: [owned._id],
|
||||
userId,
|
||||
});
|
||||
expect(resolved).toEqual({ name: 'empty-tools', body: 'body', allowedTools: [] });
|
||||
});
|
||||
|
||||
it('silently skips names with no backing skill (typo / ACL miss) without failing the batch', async () => {
|
||||
const real = mkSkill('real', userOid);
|
||||
const result = await resolveManualSkills({
|
||||
|
|
|
|||
|
|
@ -201,6 +201,12 @@ export interface InitializeAgentDbMethods extends EndpointDbMethods {
|
|||
name: string;
|
||||
body: string;
|
||||
author: import('mongoose').Types.ObjectId;
|
||||
/**
|
||||
* Skill-declared tool allowlist, forwarded verbatim from the skill doc.
|
||||
* Surfaced so the resolver can carry it onto `ResolvedManualSkill` for
|
||||
* future runtime enforcement without a second round-trip.
|
||||
*/
|
||||
allowedTools?: string[];
|
||||
} | null>;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -316,6 +316,13 @@ export interface ResolveManualSkillsParams {
|
|||
name: string;
|
||||
body: string;
|
||||
author: Types.ObjectId | string;
|
||||
/**
|
||||
* Skill-declared tool allowlist, forwarded verbatim from the skill doc.
|
||||
* Surfaced on `ResolvedManualSkill` so future runtime enforcement can
|
||||
* union it into the agent's effective tool set for the turn without
|
||||
* re-fetching the document. Populated by the DB method when available.
|
||||
*/
|
||||
allowedTools?: string[];
|
||||
} | null>;
|
||||
/** ACL-accessible skill IDs for this user (already scoped by `scopeSkillIds`). */
|
||||
accessibleSkillIds: Types.ObjectId[];
|
||||
|
|
@ -330,6 +337,13 @@ export interface ResolveManualSkillsParams {
|
|||
export interface ResolvedManualSkill {
|
||||
name: string;
|
||||
body: string;
|
||||
/**
|
||||
* Skill-declared tool allowlist passed through from the skill doc. Present
|
||||
* only when the skill author declared `allowed-tools` in frontmatter.
|
||||
* Currently populated but not consumed — future runtime enforcement will
|
||||
* union these into the agent's effective tool set for the turn.
|
||||
*/
|
||||
allowedTools?: string[];
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -414,7 +428,11 @@ export async function resolveManualSkills(
|
|||
logger.warn(`[resolveManualSkills] Skill "${name}" is inactive for this user — skipping`);
|
||||
return null;
|
||||
}
|
||||
return { name: skill.name, body: skill.body };
|
||||
const resolved: ResolvedManualSkill = { name: skill.name, body: skill.body };
|
||||
if (skill.allowedTools !== undefined) {
|
||||
resolved.allowedTools = skill.allowedTools;
|
||||
}
|
||||
return resolved;
|
||||
} catch (err) {
|
||||
logger.warn(
|
||||
`[resolveManualSkills] Failed to resolve skill "${name}":`,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue