diff --git a/src/index.ts b/src/index.ts index aa29f14..0aa3997 100644 --- a/src/index.ts +++ b/src/index.ts @@ -410,25 +410,32 @@ async function runCommand(command: string, cwd: string, packageManager: string) } async function runSkillCommand( - skill: ExtraSkill, + skills: ExtraSkill[], cwd: string, ) { + const [firstSkill] = skills; + // `skills add` accepts repeated `--skill` flags for a single source. + const installArgs = skills.flatMap((skill) => [ + '--skill', + skill.skill ?? skill.value, + ]); const args = [ '-y', 'skills', 'add', - skill.source, + firstSkill.source, '--agent', 'universal', '--yes', '--copy', - '--skill', - skill.skill ?? skill.value, + ...installArgs, ]; const command = `npx ${args.join(' ')}`; log.info(`Running skill install command: ${color.dim(command)}`); + const skillLabel = skills.map((skill) => skill.value).join(', '); + const skillNoun = skills.length === 1 ? 'skill' : 'skills'; const installationTaskLog = taskLog({ - title: `Installing skill ${skill.value}`, + title: `Installing ${skillNoun} ${skillLabel}`, }); const proc = x('npx', args, { @@ -445,12 +452,13 @@ async function runSkillCommand( const result = await proc; if (result.exitCode !== 0) { - const message = `Failed to install skill "${skill.value}" from "${skill.source}" using command: ${command}`; + const quotedSkillLabel = skills.map((skill) => `"${skill.value}"`).join(', '); + const message = `Failed to install ${skillNoun} ${quotedSkillLabel} from "${firstSkill.source}" using command: ${command}`; installationTaskLog.error(message); throw new Error(message); } - installationTaskLog.success(`Installed skill ${skill.value}`); + installationTaskLog.success(`Installed ${skillNoun} ${skillLabel}`); } export async function create({ @@ -589,13 +597,31 @@ export async function create({ skipFiles, }); + const skillsByValue = new Map( + (filteredExtraSkills ?? []).map((extraSkill) => [extraSkill.value, extraSkill]), + ); + let currentSkillBatch: ExtraSkill[] = []; + + // Batch only contiguous skills from the same source to preserve install order. for (const skillValue of skills) { - const matchedSkill = filteredExtraSkills?.find( - (extraSkill) => extraSkill.value === skillValue, - ); - if (matchedSkill) { - await runSkillCommand(matchedSkill, distFolder); + const matchedSkill = skillsByValue.get(skillValue); + if (!matchedSkill) { + continue; + } + + if ( + currentSkillBatch.length > 0 && + currentSkillBatch[0].source !== matchedSkill.source + ) { + await runSkillCommand(currentSkillBatch, distFolder); + currentSkillBatch = []; } + + currentSkillBatch.push(matchedSkill); + } + + if (currentSkillBatch.length > 0) { + await runSkillCommand(currentSkillBatch, distFolder); } const packageRoot = path.resolve(__dirname, '..'); diff --git a/test/skills.test.ts b/test/skills.test.ts index 088a755..2306fba 100644 --- a/test/skills.test.ts +++ b/test/skills.test.ts @@ -243,7 +243,75 @@ async function getCreateError(action: Promise) { throw new Error('Expected create() to throw'); } -test('should install selected extra skills from comma separated --skill option', async () => { +test('should batch selected same-source extra skills from comma separated --skill option into a single install', async () => { + const projectDir = path.join(testDir, 'skills-comma-separated-same-source'); + const calls = createExecCommand(); + const taskLogEvents = mocks.state.taskLogEvents; + const commandLogs = mocks.state.commandLogs; + + await create({ + name: 'test', + root: fixturesDir, + templates: ['vanilla'], + getTemplateName: async () => 'vanilla', + extraSkills: [ + { + value: 'rstest-best-practices', + label: 'Rstest Best Practices', + source: 'rstackjs/agent-skills', + }, + { + value: 'rsbuild-best-practices', + label: 'Rsbuild Best Practices', + source: 'rstackjs/agent-skills', + }, + ], + argv: [ + 'node', + 'test', + '--dir', + projectDir, + '--template', + 'vanilla', + '--skill', + 'rstest-best-practices,rsbuild-best-practices', + ], + }); + + expect(calls).toHaveLength(1); + expect(calls[0]).toEqual({ + args: [ + '-y', + 'skills', + 'add', + 'rstackjs/agent-skills', + '--agent', + 'universal', + '--yes', + '--copy', + '--skill', + 'rstest-best-practices', + '--skill', + 'rsbuild-best-practices', + ], + command: 'npx', + options: expect.objectContaining({ + nodeOptions: expect.objectContaining({ + cwd: projectDir, + stdio: 'pipe', + }), + }), + }); + expect(taskLogEvents).toEqual([ + 'create:Installing skills rstest-best-practices, rsbuild-best-practices', + 'success:Installing skills rstest-best-practices, rsbuild-best-practices:Installed skills rstest-best-practices, rsbuild-best-practices', + ]); + expect(commandLogs).toContain( + `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')}`, + ); +}); + +test('should install selected extra skills from comma separated --skill option across different sources', async () => { const projectDir = path.join(testDir, 'skills-comma-separated'); const calls = createExecCommand(); const taskLogEvents = mocks.state.taskLogEvents; @@ -415,6 +483,110 @@ test('should install selected extra skills from repeated --skill flags', async ( }); }); +test('should preserve skill install order when the same source appears non-contiguously', async () => { + const projectDir = path.join(testDir, 'skills-preserve-order'); + const calls = createExecCommand(); + + await create({ + name: 'test', + root: fixturesDir, + templates: ['vanilla'], + getTemplateName: async () => 'vanilla', + extraSkills: [ + { + value: 'rstest-best-practices', + label: 'Rstest Best Practices', + source: 'rstackjs/agent-skills', + }, + { + value: 'docs-writer', + label: 'Docs Writer', + source: 'acme/skills', + }, + { + value: 'rsbuild-best-practices', + label: 'Rsbuild Best Practices', + source: 'rstackjs/agent-skills', + }, + ], + argv: [ + 'node', + 'test', + '--dir', + projectDir, + '--template', + 'vanilla', + '--skill', + 'rstest-best-practices,docs-writer,rsbuild-best-practices', + ], + }); + + expect(calls).toHaveLength(3); + expect(calls[0]).toEqual({ + args: [ + '-y', + 'skills', + 'add', + 'rstackjs/agent-skills', + '--agent', + 'universal', + '--yes', + '--copy', + '--skill', + 'rstest-best-practices', + ], + command: 'npx', + options: expect.objectContaining({ + nodeOptions: expect.objectContaining({ + cwd: projectDir, + stdio: 'pipe', + }), + }), + }); + expect(calls[1]).toEqual({ + args: [ + '-y', + 'skills', + 'add', + 'acme/skills', + '--agent', + 'universal', + '--yes', + '--copy', + '--skill', + 'docs-writer', + ], + command: 'npx', + options: expect.objectContaining({ + nodeOptions: expect.objectContaining({ + cwd: projectDir, + stdio: 'pipe', + }), + }), + }); + expect(calls[2]).toEqual({ + args: [ + '-y', + 'skills', + 'add', + 'rstackjs/agent-skills', + '--agent', + 'universal', + '--yes', + '--copy', + '--skill', + 'rsbuild-best-practices', + ], + command: 'npx', + options: expect.objectContaining({ + nodeOptions: expect.objectContaining({ + cwd: projectDir, + stdio: 'pipe', + }), + }), + }); +}); + test('should skip the skills prompt when --skill is provided', async () => { const projectDir = path.join(testDir, 'skills-skip-prompt-with-cli-option'); const calls = createExecCommand(); @@ -663,6 +835,53 @@ test('should throw the install command context when installation fails', async ( ); }); +test('should throw grouped install command context when batched installation fails', async () => { + const projectDir = path.join(testDir, 'skills-batched-install-failure'); + createExecCommand(() => { + return { + stdout: '', + stderr: 'install failed', + exitCode: 1, + }; + }); + + const error = await getCreateError( + create({ + name: 'test', + root: fixturesDir, + templates: ['vanilla'], + getTemplateName: async () => 'vanilla', + extraSkills: [ + { + value: 'rstest-best-practices', + label: 'Rstest Best Practices', + source: 'rstackjs/agent-skills', + }, + { + value: 'rsbuild-best-practices', + label: 'Rsbuild Best Practices', + source: 'rstackjs/agent-skills', + }, + ], + argv: [ + 'node', + 'test', + '--dir', + projectDir, + '--template', + 'vanilla', + '--skill', + 'rstest-best-practices,rsbuild-best-practices', + ], + }), + ); + + expect(error).toBeInstanceOf(Error); + expect((error as Error).message).toBe( + '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', + ); +}); + test('should omit noisy skills cli output from install errors', async () => { const projectDir = path.join(testDir, 'skills-install-noisy-error'); const rawStdout = `███████╗██╗ ██╗██╗██╗ ██╗ ███████╗