Skip to content

Commit 46c6ca2

Browse files
committed
fix(cli): remove skill in .ai-devkit.json when skill is removed
1 parent cfb49e0 commit 46c6ca2

File tree

5 files changed

+213
-1
lines changed

5 files changed

+213
-1
lines changed

e2e/cli.e2e.ts

Lines changed: 69 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { existsSync, readFileSync } from 'fs';
1+
import { existsSync, mkdirSync, readFileSync } from 'fs';
22
import { join } from 'path';
33
import { run, createTempProject, cleanupTempProject, writeConfigFile } from './helpers';
44

@@ -331,6 +331,74 @@ describe('skill command', () => {
331331

332332
cleanupTempProject(projectDir);
333333
});
334+
335+
describe('skill remove (issue #63)', () => {
336+
let projectDir: string;
337+
338+
beforeEach(() => {
339+
projectDir = createTempProject();
340+
});
341+
342+
afterEach(() => {
343+
cleanupTempProject(projectDir);
344+
});
345+
346+
it('should remove the skill entry from .ai-devkit.json after removal', () => {
347+
writeConfigFile(projectDir, {
348+
version: '1.0.0',
349+
environments: ['claude'],
350+
phases: [],
351+
skills: {
352+
installed: [
353+
{ registry: 'codeaholicguy/ai-devkit', name: 'dev-lifecycle' }
354+
]
355+
},
356+
createdAt: new Date().toISOString(),
357+
updatedAt: new Date().toISOString()
358+
});
359+
360+
// Create the skill directory so the remove command finds it
361+
const skillDir = join(projectDir, '.claude', 'skills', 'dev-lifecycle');
362+
mkdirSync(skillDir, { recursive: true });
363+
364+
const result = run('skill remove dev-lifecycle', { cwd: projectDir });
365+
expect(result.exitCode).toBe(0);
366+
367+
// Skill directory should be gone
368+
expect(existsSync(skillDir)).toBe(false);
369+
370+
// .ai-devkit.json should no longer list the skill
371+
const config = JSON.parse(readFileSync(join(projectDir, '.ai-devkit.json'), 'utf-8'));
372+
const installed = (config.skills?.installed ?? config.skills ?? []) as Array<{ name: string }>;
373+
expect(installed.some((s) => s.name === 'dev-lifecycle')).toBe(false);
374+
});
375+
376+
it('should preserve remaining skills in .ai-devkit.json when removing one', () => {
377+
writeConfigFile(projectDir, {
378+
version: '1.0.0',
379+
environments: ['claude'],
380+
phases: [],
381+
skills: {
382+
installed: [
383+
{ registry: 'codeaholicguy/ai-devkit', name: 'dev-lifecycle' },
384+
{ registry: 'codeaholicguy/ai-devkit', name: 'memory' }
385+
]
386+
},
387+
createdAt: new Date().toISOString(),
388+
updatedAt: new Date().toISOString()
389+
});
390+
391+
const skillDir = join(projectDir, '.claude', 'skills', 'dev-lifecycle');
392+
mkdirSync(skillDir, { recursive: true });
393+
394+
run('skill remove dev-lifecycle', { cwd: projectDir });
395+
396+
const config = JSON.parse(readFileSync(join(projectDir, '.ai-devkit.json'), 'utf-8'));
397+
const installed = (config.skills?.installed ?? config.skills ?? []) as Array<{ name: string }>;
398+
expect(installed.some((s) => s.name === 'dev-lifecycle')).toBe(false);
399+
expect(installed.some((s) => s.name === 'memory')).toBe(true);
400+
});
401+
});
334402
});
335403

336404
describe('Node.js compatibility', () => {

packages/cli/src/__tests__/lib/Config.test.ts

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -536,6 +536,121 @@ describe('ConfigManager', () => {
536536
});
537537
});
538538

539+
describe('removeSkill', () => {
540+
it('removes the skill entry from the installed array', async () => {
541+
const config: DevKitConfig = {
542+
version: '1.0.0',
543+
environments: ['claude'],
544+
phases: [],
545+
skills: {
546+
installed: [
547+
{ registry: 'codeaholicguy/ai-devkit', name: 'dev-lifecycle' },
548+
{ registry: 'codeaholicguy/ai-devkit', name: 'memory' }
549+
]
550+
},
551+
createdAt: '2024-01-01T00:00:00.000Z',
552+
updatedAt: '2024-01-01T00:00:00.000Z'
553+
};
554+
555+
(mockFs.pathExists as any).mockResolvedValue(true);
556+
(mockFs.readJson as any).mockResolvedValue(config);
557+
(mockFs.writeJson as any).mockResolvedValue(undefined);
558+
559+
const result = await configManager.removeSkill('dev-lifecycle');
560+
561+
expect(result.skills).toEqual({
562+
installed: [
563+
{ registry: 'codeaholicguy/ai-devkit', name: 'memory' }
564+
]
565+
});
566+
expect(mockFs.writeJson).toHaveBeenCalled();
567+
});
568+
569+
it('removes skill when skills is an array', async () => {
570+
const config: DevKitConfig = {
571+
version: '1.0.0',
572+
environments: ['claude'],
573+
phases: [],
574+
skills: [
575+
{ registry: 'codeaholicguy/ai-devkit', name: 'dev-lifecycle' },
576+
{ registry: 'codeaholicguy/ai-devkit', name: 'memory' }
577+
],
578+
createdAt: '2024-01-01T00:00:00.000Z',
579+
updatedAt: '2024-01-01T00:00:00.000Z'
580+
};
581+
582+
(mockFs.pathExists as any).mockResolvedValue(true);
583+
(mockFs.readJson as any).mockResolvedValue(config);
584+
(mockFs.writeJson as any).mockResolvedValue(undefined);
585+
586+
const result = await configManager.removeSkill('dev-lifecycle');
587+
588+
expect(result.skills).toEqual({
589+
installed: [
590+
{ registry: 'codeaholicguy/ai-devkit', name: 'memory' }
591+
]
592+
});
593+
expect(mockFs.writeJson).toHaveBeenCalled();
594+
});
595+
596+
it('results in an empty installed array when last skill is removed', async () => {
597+
const config: DevKitConfig = {
598+
version: '1.0.0',
599+
environments: ['claude'],
600+
phases: [],
601+
skills: {
602+
installed: [
603+
{ registry: 'codeaholicguy/ai-devkit', name: 'dev-lifecycle' }
604+
]
605+
},
606+
createdAt: '2024-01-01T00:00:00.000Z',
607+
updatedAt: '2024-01-01T00:00:00.000Z'
608+
};
609+
610+
(mockFs.pathExists as any).mockResolvedValue(true);
611+
(mockFs.readJson as any).mockResolvedValue(config);
612+
(mockFs.writeJson as any).mockResolvedValue(undefined);
613+
614+
const result = await configManager.removeSkill('dev-lifecycle');
615+
616+
expect(result.skills).toEqual({ installed: [] });
617+
expect(mockFs.writeJson).toHaveBeenCalled();
618+
});
619+
620+
it('is a no-op when skill name does not exist in installed', async () => {
621+
const config: DevKitConfig = {
622+
version: '1.0.0',
623+
environments: ['claude'],
624+
phases: [],
625+
skills: {
626+
installed: [
627+
{ registry: 'codeaholicguy/ai-devkit', name: 'memory' }
628+
]
629+
},
630+
createdAt: '2024-01-01T00:00:00.000Z',
631+
updatedAt: '2024-01-01T00:00:00.000Z'
632+
};
633+
634+
(mockFs.pathExists as any).mockResolvedValue(true);
635+
(mockFs.readJson as any).mockResolvedValue(config);
636+
(mockFs.writeJson as any).mockResolvedValue(undefined);
637+
638+
const result = await configManager.removeSkill('nonexistent');
639+
640+
expect(result.skills).toEqual({
641+
installed: [{ registry: 'codeaholicguy/ai-devkit', name: 'memory' }]
642+
});
643+
});
644+
645+
it('throws when config file is not found', async () => {
646+
(mockFs.pathExists as any).mockResolvedValue(false);
647+
648+
await expect(configManager.removeSkill('dev-lifecycle')).rejects.toThrow(
649+
'Config file not found'
650+
);
651+
});
652+
});
653+
539654
describe('normalizeSkillsConfig', () => {
540655
it('normalizes an array to SkillsConfig', () => {
541656
const result = configManager.normalizeSkillsConfig([

packages/cli/src/__tests__/lib/SkillManager.test.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -891,6 +891,7 @@ describe("SkillManager", () => {
891891

892892
(mockedFs.pathExists as any).mockResolvedValue(true);
893893
(mockedFs.remove as any).mockResolvedValue(undefined);
894+
mockConfigManager.removeSkill.mockResolvedValue({} as any);
894895
});
895896

896897
it("should validate skill name", async () => {
@@ -920,6 +921,20 @@ describe("SkillManager", () => {
920921
);
921922
});
922923

924+
it("should update config to remove skill entry after successful removal", async () => {
925+
await skillManager.removeSkill(mockSkillName);
926+
927+
expect(mockConfigManager.removeSkill).toHaveBeenCalledWith(mockSkillName);
928+
});
929+
930+
it("should not update config when skill files are not found", async () => {
931+
(mockedFs.pathExists as any).mockResolvedValue(false);
932+
933+
await skillManager.removeSkill(mockSkillName);
934+
935+
expect(mockConfigManager.removeSkill).not.toHaveBeenCalled();
936+
});
937+
923938
it("should handle skill not found gracefully", async () => {
924939
(mockedFs.pathExists as any).mockResolvedValue(false);
925940

packages/cli/src/lib/Config.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,19 @@ export class ConfigManager {
163163
return this.update({ skills: normalized });
164164
}
165165

166+
async removeSkill(skillName: string): Promise<DevKitConfig> {
167+
const config = await this.read();
168+
if (!config) {
169+
throw new Error('Config file not found. Run ai-devkit init first.');
170+
}
171+
172+
const normalized = this.normalizeSkillsConfig(config.skills);
173+
const installed = normalized.installed || [];
174+
175+
normalized.installed = installed.filter(entry => entry.name !== skillName);
176+
return this.update({ skills: normalized });
177+
}
178+
166179
async getSkillRegistries(): Promise<Record<string, string>> {
167180
const config = await this.read();
168181
if (!config) {

packages/cli/src/lib/SkillManager.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -260,6 +260,7 @@ export class SkillManager {
260260
ui.warning(`Skill "${skillName}" not found. Nothing to remove.`);
261261
ui.info('Tip: Run "ai-devkit skill list" to see installed skills.');
262262
} else {
263+
await this.configManager.removeSkill(skillName);
263264
ui.success(`Successfully removed from ${removedCount} location(s).`);
264265
ui.info(`Note: Cached copy in ~/.ai-devkit/skills/ preserved for other projects.`);
265266
}

0 commit comments

Comments
 (0)