Skip to content

Commit f61bc92

Browse files
chenjiahanCopilot
chenjiahan
and
Copilot
authored
fix: batch skill installations by source (#123)
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
1 parent 9cf48d2 commit f61bc92

2 files changed

Lines changed: 258 additions & 13 deletions

File tree

‎src/index.ts‎

Lines changed: 38 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -410,25 +410,32 @@ async function runCommand(command: string, cwd: string, packageManager: string)
410410
}
411411

412412
async function runSkillCommand(
413-
skill: ExtraSkill,
413+
skills: ExtraSkill[],
414414
cwd: string,
415415
) {
416+
const [firstSkill] = skills;
417+
// `skills add` accepts repeated `--skill` flags for a single source.
418+
const installArgs = skills.flatMap((skill) => [
419+
'--skill',
420+
skill.skill ?? skill.value,
421+
]);
416422
const args = [
417423
'-y',
418424
'skills',
419425
'add',
420-
skill.source,
426+
firstSkill.source,
421427
'--agent',
422428
'universal',
423429
'--yes',
424430
'--copy',
425-
'--skill',
426-
skill.skill ?? skill.value,
431+
...installArgs,
427432
];
428433
const command = `npx ${args.join(' ')}`;
429434
log.info(`Running skill install command: ${color.dim(command)}`);
435+
const skillLabel = skills.map((skill) => skill.value).join(', ');
436+
const skillNoun = skills.length === 1 ? 'skill' : 'skills';
430437
const installationTaskLog = taskLog({
431-
title: `Installing skill ${skill.value}`,
438+
title: `Installing ${skillNoun} ${skillLabel}`,
432439
});
433440

434441
const proc = x('npx', args, {
@@ -445,12 +452,13 @@ async function runSkillCommand(
445452
const result = await proc;
446453

447454
if (result.exitCode !== 0) {
448-
const message = `Failed to install skill "${skill.value}" from "${skill.source}" using command: ${command}`;
455+
const quotedSkillLabel = skills.map((skill) => `"${skill.value}"`).join(', ');
456+
const message = `Failed to install ${skillNoun} ${quotedSkillLabel} from "${firstSkill.source}" using command: ${command}`;
449457
installationTaskLog.error(message);
450458
throw new Error(message);
451459
}
452460

453-
installationTaskLog.success(`Installed skill ${skill.value}`);
461+
installationTaskLog.success(`Installed ${skillNoun} ${skillLabel}`);
454462
}
455463

456464
export async function create({
@@ -589,13 +597,31 @@ export async function create({
589597
skipFiles,
590598
});
591599

600+
const skillsByValue = new Map(
601+
(filteredExtraSkills ?? []).map((extraSkill) => [extraSkill.value, extraSkill]),
602+
);
603+
let currentSkillBatch: ExtraSkill[] = [];
604+
605+
// Batch only contiguous skills from the same source to preserve install order.
592606
for (const skillValue of skills) {
593-
const matchedSkill = filteredExtraSkills?.find(
594-
(extraSkill) => extraSkill.value === skillValue,
595-
);
596-
if (matchedSkill) {
597-
await runSkillCommand(matchedSkill, distFolder);
607+
const matchedSkill = skillsByValue.get(skillValue);
608+
if (!matchedSkill) {
609+
continue;
610+
}
611+
612+
if (
613+
currentSkillBatch.length > 0 &&
614+
currentSkillBatch[0].source !== matchedSkill.source
615+
) {
616+
await runSkillCommand(currentSkillBatch, distFolder);
617+
currentSkillBatch = [];
598618
}
619+
620+
currentSkillBatch.push(matchedSkill);
621+
}
622+
623+
if (currentSkillBatch.length > 0) {
624+
await runSkillCommand(currentSkillBatch, distFolder);
599625
}
600626

601627
const packageRoot = path.resolve(__dirname, '..');

‎test/skills.test.ts‎

Lines changed: 220 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -243,7 +243,75 @@ async function getCreateError(action: Promise<unknown>) {
243243
throw new Error('Expected create() to throw');
244244
}
245245

246-
test('should install selected extra skills from comma separated --skill option', async () => {
246+
test('should batch selected same-source extra skills from comma separated --skill option into a single install', async () => {
247+
const projectDir = path.join(testDir, 'skills-comma-separated-same-source');
248+
const calls = createExecCommand();
249+
const taskLogEvents = mocks.state.taskLogEvents;
250+
const commandLogs = mocks.state.commandLogs;
251+
252+
await create({
253+
name: 'test',
254+
root: fixturesDir,
255+
templates: ['vanilla'],
256+
getTemplateName: async () => 'vanilla',
257+
extraSkills: [
258+
{
259+
value: 'rstest-best-practices',
260+
label: 'Rstest Best Practices',
261+
source: 'rstackjs/agent-skills',
262+
},
263+
{
264+
value: 'rsbuild-best-practices',
265+
label: 'Rsbuild Best Practices',
266+
source: 'rstackjs/agent-skills',
267+
},
268+
],
269+
argv: [
270+
'node',
271+
'test',
272+
'--dir',
273+
projectDir,
274+
'--template',
275+
'vanilla',
276+
'--skill',
277+
'rstest-best-practices,rsbuild-best-practices',
278+
],
279+
});
280+
281+
expect(calls).toHaveLength(1);
282+
expect(calls[0]).toEqual({
283+
args: [
284+
'-y',
285+
'skills',
286+
'add',
287+
'rstackjs/agent-skills',
288+
'--agent',
289+
'universal',
290+
'--yes',
291+
'--copy',
292+
'--skill',
293+
'rstest-best-practices',
294+
'--skill',
295+
'rsbuild-best-practices',
296+
],
297+
command: 'npx',
298+
options: expect.objectContaining({
299+
nodeOptions: expect.objectContaining({
300+
cwd: projectDir,
301+
stdio: 'pipe',
302+
}),
303+
}),
304+
});
305+
expect(taskLogEvents).toEqual([
306+
'create:Installing skills rstest-best-practices, rsbuild-best-practices',
307+
'success:Installing skills rstest-best-practices, rsbuild-best-practices:Installed skills rstest-best-practices, rsbuild-best-practices',
308+
]);
309+
expect(commandLogs).toContain(
310+
`Running skill install command: ${color.dim('npx -y skills add rstackjs/agent-skills --agent universal --yes --copy --skill rstest-best-practices --skill rsbuild-best-practices')}`,
311+
);
312+
});
313+
314+
test('should install selected extra skills from comma separated --skill option across different sources', async () => {
247315
const projectDir = path.join(testDir, 'skills-comma-separated');
248316
const calls = createExecCommand();
249317
const taskLogEvents = mocks.state.taskLogEvents;
@@ -415,6 +483,110 @@ test('should install selected extra skills from repeated --skill flags', async (
415483
});
416484
});
417485

486+
test('should preserve skill install order when the same source appears non-contiguously', async () => {
487+
const projectDir = path.join(testDir, 'skills-preserve-order');
488+
const calls = createExecCommand();
489+
490+
await create({
491+
name: 'test',
492+
root: fixturesDir,
493+
templates: ['vanilla'],
494+
getTemplateName: async () => 'vanilla',
495+
extraSkills: [
496+
{
497+
value: 'rstest-best-practices',
498+
label: 'Rstest Best Practices',
499+
source: 'rstackjs/agent-skills',
500+
},
501+
{
502+
value: 'docs-writer',
503+
label: 'Docs Writer',
504+
source: 'acme/skills',
505+
},
506+
{
507+
value: 'rsbuild-best-practices',
508+
label: 'Rsbuild Best Practices',
509+
source: 'rstackjs/agent-skills',
510+
},
511+
],
512+
argv: [
513+
'node',
514+
'test',
515+
'--dir',
516+
projectDir,
517+
'--template',
518+
'vanilla',
519+
'--skill',
520+
'rstest-best-practices,docs-writer,rsbuild-best-practices',
521+
],
522+
});
523+
524+
expect(calls).toHaveLength(3);
525+
expect(calls[0]).toEqual({
526+
args: [
527+
'-y',
528+
'skills',
529+
'add',
530+
'rstackjs/agent-skills',
531+
'--agent',
532+
'universal',
533+
'--yes',
534+
'--copy',
535+
'--skill',
536+
'rstest-best-practices',
537+
],
538+
command: 'npx',
539+
options: expect.objectContaining({
540+
nodeOptions: expect.objectContaining({
541+
cwd: projectDir,
542+
stdio: 'pipe',
543+
}),
544+
}),
545+
});
546+
expect(calls[1]).toEqual({
547+
args: [
548+
'-y',
549+
'skills',
550+
'add',
551+
'acme/skills',
552+
'--agent',
553+
'universal',
554+
'--yes',
555+
'--copy',
556+
'--skill',
557+
'docs-writer',
558+
],
559+
command: 'npx',
560+
options: expect.objectContaining({
561+
nodeOptions: expect.objectContaining({
562+
cwd: projectDir,
563+
stdio: 'pipe',
564+
}),
565+
}),
566+
});
567+
expect(calls[2]).toEqual({
568+
args: [
569+
'-y',
570+
'skills',
571+
'add',
572+
'rstackjs/agent-skills',
573+
'--agent',
574+
'universal',
575+
'--yes',
576+
'--copy',
577+
'--skill',
578+
'rsbuild-best-practices',
579+
],
580+
command: 'npx',
581+
options: expect.objectContaining({
582+
nodeOptions: expect.objectContaining({
583+
cwd: projectDir,
584+
stdio: 'pipe',
585+
}),
586+
}),
587+
});
588+
});
589+
418590
test('should skip the skills prompt when --skill is provided', async () => {
419591
const projectDir = path.join(testDir, 'skills-skip-prompt-with-cli-option');
420592
const calls = createExecCommand();
@@ -663,6 +835,53 @@ test('should throw the install command context when installation fails', async (
663835
);
664836
});
665837

838+
test('should throw grouped install command context when batched installation fails', async () => {
839+
const projectDir = path.join(testDir, 'skills-batched-install-failure');
840+
createExecCommand(() => {
841+
return {
842+
stdout: '',
843+
stderr: 'install failed',
844+
exitCode: 1,
845+
};
846+
});
847+
848+
const error = await getCreateError(
849+
create({
850+
name: 'test',
851+
root: fixturesDir,
852+
templates: ['vanilla'],
853+
getTemplateName: async () => 'vanilla',
854+
extraSkills: [
855+
{
856+
value: 'rstest-best-practices',
857+
label: 'Rstest Best Practices',
858+
source: 'rstackjs/agent-skills',
859+
},
860+
{
861+
value: 'rsbuild-best-practices',
862+
label: 'Rsbuild Best Practices',
863+
source: 'rstackjs/agent-skills',
864+
},
865+
],
866+
argv: [
867+
'node',
868+
'test',
869+
'--dir',
870+
projectDir,
871+
'--template',
872+
'vanilla',
873+
'--skill',
874+
'rstest-best-practices,rsbuild-best-practices',
875+
],
876+
}),
877+
);
878+
879+
expect(error).toBeInstanceOf(Error);
880+
expect((error as Error).message).toBe(
881+
'Failed to install skills "rstest-best-practices", "rsbuild-best-practices" from "rstackjs/agent-skills" using command: npx -y skills add rstackjs/agent-skills --agent universal --yes --copy --skill rstest-best-practices --skill rsbuild-best-practices',
882+
);
883+
});
884+
666885
test('should omit noisy skills cli output from install errors', async () => {
667886
const projectDir = path.join(testDir, 'skills-install-noisy-error');
668887
const rawStdout = `███████╗██╗ ██╗██╗██╗ ██╗ ███████╗

0 commit comments

Comments
 (0)