Skip to content

Commit 33e4e20

Browse files
authored
feat!: add tools parameter to skill.when (#124)
1 parent d182350 commit 33e4e20

3 files changed

Lines changed: 208 additions & 15 deletions

File tree

src/index.ts

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,8 @@ function logHelpMessage(
124124
extraSkills?: ExtraSkill[],
125125
) {
126126
const toolsList = [...BUILTIN_TOOLS];
127+
// Keep help output exhaustive for discoverability. `skill.when` only gates
128+
// the interactive prompt, not the documented list of available skills.
127129
const skillsList = (extraSkills ?? [])
128130
.map((skill) => skill.value)
129131
.filter(Boolean);
@@ -234,10 +236,13 @@ async function getTools(
234236
function filterExtraSkills(
235237
extraSkills: ExtraSkill[] | undefined,
236238
templateName?: string,
239+
tools: string[] = [],
237240
) {
241+
// `skill.when` only affects the interactive prompt. Explicit `--skill`
242+
// values are handled separately in `getSkills`.
238243
return extraSkills?.filter((extraSkill) => {
239244
const when = extraSkill.when ?? (() => true);
240-
return templateName ? when(templateName) : true;
245+
return templateName ? when({ templateName, tools }) : true;
241246
});
242247
}
243248

@@ -257,14 +262,17 @@ async function getSkills(
257262
{ skill, dir, template }: Argv,
258263
extraSkills?: ExtraSkill[],
259264
templateName?: string,
265+
tools: string[] = [],
260266
promptMultiselect: typeof multiselect = multiselect,
261267
) {
262268
const parsedSkills = parseSkillsOption(skill);
263-
const filteredExtraSkills = filterExtraSkills(extraSkills, templateName);
269+
const filteredExtraSkills = filterExtraSkills(extraSkills, templateName, tools);
264270

265271
if (parsedSkills !== null) {
272+
// Treat explicit `--skill` values as authoritative as long as they refer to
273+
// a declared skill. `skill.when` only hides options from the prompt.
266274
return parsedSkills.filter((value: string) =>
267-
filteredExtraSkills?.some((extraSkill) => extraSkill.value === value),
275+
extraSkills?.some((extraSkill) => extraSkill.value === value),
268276
);
269277
}
270278

@@ -367,7 +375,12 @@ type ExtraSkill = {
367375
label: string;
368376
source: string;
369377
skill?: string;
370-
when?: (templateName: string) => boolean;
378+
/**
379+
* Controls whether the skill is shown in the interactive prompt for the
380+
* selected template/tools. Explicit `--skill` values and `--help` remain
381+
* unfiltered so CLI input stays authoritative and help stays discoverable.
382+
*/
383+
when?: (context: { templateName: string; tools: string[] }) => boolean;
371384
order?: 'pre' | 'post';
372385
};
373386

@@ -566,13 +579,7 @@ export async function create({
566579

567580
const templateName = await getTemplateName(argv);
568581
const tools = await getTools(argv, extraTools, templateName);
569-
const filteredExtraSkills = filterExtraSkills(extraSkills, templateName);
570-
const skills = await getSkills(
571-
argv,
572-
filteredExtraSkills,
573-
templateName,
574-
multiselect,
575-
);
582+
const skills = await getSkills(argv, extraSkills, templateName, tools, multiselect);
576583

577584
const srcFolder = path.join(root, `template-${templateName}`);
578585
const commonFolder = path.join(root, 'template-common');
@@ -598,7 +605,7 @@ export async function create({
598605
});
599606

600607
const skillsByValue = new Map(
601-
(filteredExtraSkills ?? []).map((extraSkill) => [extraSkill.value, extraSkill]),
608+
(extraSkills ?? []).map((extraSkill) => [extraSkill.value, extraSkill]),
602609
);
603610
let currentSkillBatch: ExtraSkill[] = [];
604611

test/help.test.ts

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,3 +96,55 @@ test('help message includes optional skills', async () => {
9696
expect(logOutput).toContain('Optional skills:');
9797
expect(logOutput).toContain('git-url');
9898
});
99+
100+
test('help message lists all optional skills even when template and tools are provided', async () => {
101+
const logs: string[] = [];
102+
const originalLog = logger.log;
103+
104+
logger.override({
105+
log: (message?: unknown) => {
106+
logs.push(String(message ?? ''));
107+
},
108+
});
109+
110+
try {
111+
await create({
112+
name: 'test',
113+
root: '.',
114+
templates: ['vanilla', 'react'],
115+
getTemplateName: async () => 'vanilla',
116+
extraSkills: [
117+
{
118+
value: 'shared-docs',
119+
label: 'Shared Docs',
120+
source: 'acme/skills',
121+
when: ({ templateName }) => templateName === 'vanilla',
122+
},
123+
{
124+
value: 'react-docs',
125+
label: 'React Docs',
126+
source: 'acme/skills',
127+
when: ({ templateName }) => templateName === 'react',
128+
},
129+
{
130+
value: 'rstest-best-practices',
131+
label: 'Rstest Best Practices',
132+
source: 'rstackjs/agent-skills',
133+
when: ({ tools }) => tools.includes('rstest'),
134+
},
135+
],
136+
argv: ['node', 'test', '--help', '--template', 'vanilla', '--tools', 'biome'],
137+
});
138+
} finally {
139+
logger.override({
140+
log: originalLog,
141+
});
142+
}
143+
144+
const logOutput = logs.join('\n');
145+
expect(logOutput).toContain('--skill <skill>');
146+
expect(logOutput).toContain('Optional skills:');
147+
expect(logOutput).toContain('shared-docs');
148+
expect(logOutput).toContain('react-docs');
149+
expect(logOutput).toContain('rstest-best-practices');
150+
});

test/skills.test.ts

Lines changed: 137 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -731,7 +731,7 @@ test('should prove --skill skips the skills prompt even without --dir and --temp
731731
});
732732
});
733733

734-
test('should filter extra skills by template and install using skill override', async () => {
734+
test('should honor explicit --skill values even when they are hidden by template gating', async () => {
735735
const projectDir = path.join(testDir, 'skills-template-filtering');
736736
const calls = createExecCommand();
737737

@@ -746,14 +746,14 @@ test('should filter extra skills by template and install using skill override',
746746
label: 'React Docs',
747747
source: 'acme/skills',
748748
skill: 'docs/react',
749-
when: (templateName) => templateName === 'react',
749+
when: ({ templateName }) => templateName === 'react',
750750
},
751751
{
752752
value: 'shared-docs',
753753
label: 'Shared Docs',
754754
source: 'acme/skills',
755755
skill: 'docs/shared',
756-
when: (templateName) => templateName === 'vanilla',
756+
when: ({ templateName }) => templateName === 'vanilla',
757757
},
758758
],
759759
argv: [
@@ -770,6 +770,140 @@ test('should filter extra skills by template and install using skill override',
770770

771771
expect(calls).toHaveLength(1);
772772
expect(calls[0]).toEqual({
773+
args: [
774+
'-y',
775+
'skills',
776+
'add',
777+
'acme/skills',
778+
'--agent',
779+
'universal',
780+
'--yes',
781+
'--copy',
782+
'--skill',
783+
'docs/react',
784+
'--skill',
785+
'docs/shared',
786+
],
787+
command: 'npx',
788+
options: expect.objectContaining({
789+
nodeOptions: expect.objectContaining({
790+
cwd: projectDir,
791+
stdio: 'pipe',
792+
}),
793+
}),
794+
});
795+
});
796+
797+
test('should show tool-gated skills in the prompt when the required tool is selected', async () => {
798+
const projectDir = path.join(testDir, 'skills-tools-filtering-prompt');
799+
800+
await create({
801+
name: 'test',
802+
root: fixturesDir,
803+
templates: ['vanilla'],
804+
getTemplateName: async () => 'vanilla',
805+
extraTools: [
806+
{
807+
value: 'rstest',
808+
label: 'Rstest',
809+
},
810+
],
811+
extraSkills: [
812+
{
813+
value: 'shared-docs',
814+
label: 'Shared Docs',
815+
source: 'acme/skills',
816+
},
817+
{
818+
value: 'rstest-best-practices',
819+
label: 'Rstest Best Practices',
820+
source: 'rstackjs/agent-skills',
821+
when: ({ tools }) => tools.includes('rstest'),
822+
},
823+
],
824+
argv: ['node', 'test', projectDir, '--tools', 'rstest'],
825+
});
826+
827+
expect(mocks.state.promptOptions).toEqual([
828+
{
829+
value: 'shared-docs',
830+
label: 'Shared Docs',
831+
hint: 'acme/skills',
832+
},
833+
{
834+
value: 'rstest-best-practices',
835+
label: 'Rstest Best Practices',
836+
hint: 'rstackjs/agent-skills',
837+
},
838+
]);
839+
});
840+
841+
test('should honor explicit --skill values even when the required tool is not selected', async () => {
842+
const projectDir = path.join(testDir, 'skills-tools-filtering-cli');
843+
const calls = createExecCommand();
844+
845+
await create({
846+
name: 'test',
847+
root: fixturesDir,
848+
templates: ['vanilla'],
849+
getTemplateName: async () => 'vanilla',
850+
extraTools: [
851+
{
852+
value: 'rstest',
853+
label: 'Rstest',
854+
},
855+
],
856+
extraSkills: [
857+
{
858+
value: 'shared-docs',
859+
label: 'Shared Docs',
860+
source: 'acme/skills',
861+
skill: 'docs/shared',
862+
},
863+
{
864+
value: 'rstest-best-practices',
865+
label: 'Rstest Best Practices',
866+
source: 'rstackjs/agent-skills',
867+
when: ({ tools }) => tools.includes('rstest'),
868+
},
869+
],
870+
argv: [
871+
'node',
872+
'test',
873+
'--dir',
874+
projectDir,
875+
'--template',
876+
'vanilla',
877+
'--tools',
878+
'biome',
879+
'--skill',
880+
'rstest-best-practices,shared-docs',
881+
],
882+
});
883+
884+
expect(calls).toHaveLength(2);
885+
expect(calls[0]).toEqual({
886+
args: [
887+
'-y',
888+
'skills',
889+
'add',
890+
'rstackjs/agent-skills',
891+
'--agent',
892+
'universal',
893+
'--yes',
894+
'--copy',
895+
'--skill',
896+
'rstest-best-practices',
897+
],
898+
command: 'npx',
899+
options: expect.objectContaining({
900+
nodeOptions: expect.objectContaining({
901+
cwd: projectDir,
902+
stdio: 'pipe',
903+
}),
904+
}),
905+
});
906+
expect(calls[1]).toEqual({
773907
args: [
774908
'-y',
775909
'skills',

0 commit comments

Comments
 (0)