Skip to content

fix: batch skill installations by source #123

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
chenjiahan merged 4 commits into from
Apr 14, 2026
Merged

fix: batch skill installations by source #123

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0c0b2a1
fix: batch skill installations by source
chenjiahan Apr 14, 2026
c99215e
Potential fix for pull request finding
chenjiahan Apr 14, 2026
cb18072
fix: preserve skill install order when batching
chenjiahan Apr 14, 2026
6adcaba
refactor: simplify skill batching flow
chenjiahan Apr 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 38 additions & 12 deletions src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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, {
Expand All @@ -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({
Expand Down Expand Up @@ -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, '..');
Expand Down
221 changes: 220 additions & 1 deletion test/skills.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,75 @@ async function getCreateError(action: Promise<unknown>) {
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;
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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 = `███████╗██╗ ██╗██╗██╗ ██╗ ███████╗
Expand Down
Loading