Skip to content

Commit 42c2f57

Browse files
committed
refactor: simplify skill install error handling
1 parent d3824cf commit 42c2f57

2 files changed

Lines changed: 95 additions & 41 deletions

File tree

src/index.ts

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -420,29 +420,32 @@ async function runSkillCommand(
420420
installationSpinner.start(`Installing skill ${skill.value}`);
421421

422422
try {
423-
const result = await x('npx', args, {
423+
await x('npx', args, {
424+
throwOnError: true,
424425
nodeOptions: {
425426
cwd,
426427
stdio: 'pipe',
427428
},
428429
});
429-
430-
if (result.exitCode !== 0) {
431-
installationSpinner.error(`Failed to install skill ${skill.value}`);
432-
const details = [result.stderr, result.stdout].filter(Boolean).join('\n').trim();
433-
throw new Error(
434-
`Failed to install skill "${skill.value}" from "${skill.source}" using command: ${command}${details ? `\n${details}` : ''}`,
435-
);
436-
}
437-
438-
installationSpinner.stop(`Installed skill ${skill.value}`);
439430
} catch (error) {
440431
installationSpinner.error(`Failed to install skill ${skill.value}`);
441-
const details = error instanceof Error ? error.message : String(error);
432+
const details = [
433+
error && typeof error === 'object' && 'output' in error
434+
? (error as { output?: { stderr?: string; stdout?: string } }).output?.stderr
435+
: undefined,
436+
error && typeof error === 'object' && 'output' in error
437+
? (error as { output?: { stderr?: string; stdout?: string } }).output?.stdout
438+
: undefined,
439+
]
440+
.filter(Boolean)
441+
.join('\n')
442+
.trim() || (error instanceof Error ? error.message : String(error));
442443
throw new Error(
443444
`Failed to install skill "${skill.value}" from "${skill.source}" using command: ${command}${details ? `\n${details}` : ''}`,
444445
);
445446
}
447+
448+
installationSpinner.stop(`Installed skill ${skill.value}`);
446449
}
447450

448451
export async function create({

test/skills.test.ts

Lines changed: 80 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,16 @@ function createExecCommand(
162162
return mocks.state.xCalls;
163163
}
164164

165+
async function getCreateError(action: Promise<unknown>) {
166+
try {
167+
await action;
168+
} catch (error) {
169+
return error;
170+
}
171+
172+
throw new Error('Expected create() to throw');
173+
}
174+
165175
test('should install selected extra skills from comma separated --skill option', async () => {
166176
const projectDir = path.join(testDir, 'skills-comma-separated');
167177
const calls = createExecCommand();
@@ -541,36 +551,11 @@ test('should filter extra skills by template and install using skill override',
541551

542552
test('should throw with skill context when installation fails', async () => {
543553
const projectDir = path.join(testDir, 'skills-install-failure');
544-
createExecCommand(({ command, args, options }) => {
545-
expect(command).toBe('npx');
546-
expect(args).toEqual([
547-
'-y',
548-
'skills',
549-
'add',
550-
'acme/skills',
551-
'--agent',
552-
'universal',
553-
'--yes',
554-
'--copy',
555-
'--skill',
556-
'docs/shared',
557-
]);
558-
expect(options).toEqual(
559-
expect.objectContaining({
560-
nodeOptions: expect.objectContaining({
561-
cwd: projectDir,
562-
stdio: 'pipe',
563-
}),
564-
}),
565-
);
566-
return {
567-
stdout: '',
568-
stderr: 'install failed',
569-
exitCode: 1,
570-
};
554+
createExecCommand(() => {
555+
throw new Error('install failed');
571556
});
572557

573-
await expect(
558+
const error = await getCreateError(
574559
create({
575560
name: 'test',
576561
root: fixturesDir,
@@ -595,11 +580,77 @@ test('should throw with skill context when installation fails', async () => {
595580
'shared-docs',
596581
],
597582
}),
598-
).rejects.toThrow(
583+
);
584+
585+
expect(error).toBeInstanceOf(Error);
586+
expect((error as Error).message).toBe(
599587
'Failed to install skill "shared-docs" from "acme/skills" using command: npx -y skills add acme/skills --agent universal --yes --copy --skill docs/shared\ninstall failed',
600588
);
601589
});
602590

591+
test('should trim noisy skills cli output in install errors', async () => {
592+
const projectDir = path.join(testDir, 'skills-install-noisy-error');
593+
const rawStdout = `███████╗██╗ ██╗██╗██╗ ██╗ ███████╗
594+
┌ skills
595+
596+
│ Tip: use the --yes (-y) and --global (-g) flags to install without prompts.
597+
598+
◇ Source: https://github.com/vercel-labs/agent-skills.git
599+
600+
◇ Repository cloned
601+
602+
◇ Found 6 skills
603+
604+
■ No matching skills found for: non-existent-skill`;
605+
createExecCommand(() => {
606+
const error = new Error('Process exited with non-zero status (1)') as Error & {
607+
output?: { stderr: string; stdout: string };
608+
};
609+
error.output = {
610+
stderr: '',
611+
stdout: rawStdout,
612+
};
613+
throw error;
614+
});
615+
616+
const error = await getCreateError(
617+
create({
618+
name: 'test',
619+
root: fixturesDir,
620+
templates: ['vanilla'],
621+
getTemplateName: async () => 'vanilla',
622+
extraSkills: [
623+
{
624+
value: 'missing-skill',
625+
label: 'Missing Skill',
626+
source: 'vercel-labs/agent-skills',
627+
skill: 'non-existent-skill',
628+
},
629+
],
630+
argv: [
631+
'node',
632+
'test',
633+
'--dir',
634+
projectDir,
635+
'--template',
636+
'vanilla',
637+
'--skill',
638+
'missing-skill',
639+
],
640+
}),
641+
);
642+
643+
expect(error).toBeInstanceOf(Error);
644+
const message = (error as Error).message;
645+
expect(
646+
message.match(/Failed to install skill "missing-skill"/g)?.length ?? 0,
647+
).toBe(1);
648+
expect(message).toContain(
649+
'Failed to install skill "missing-skill" from "vercel-labs/agent-skills" using command: npx -y skills add vercel-labs/agent-skills --agent universal --yes --copy --skill non-existent-skill',
650+
);
651+
expect(message).toContain(rawStdout);
652+
});
653+
603654
test('should include spawn errors when skill installation cannot start', async () => {
604655
const projectDir = path.join(testDir, 'skills-install-spawn-error');
605656
createExecCommand(({ command, args, options }) => {

0 commit comments

Comments
 (0)