fix: addSkill() crashes when skills is an object with registries field

Author: codeaholicguy

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

@@ -425,7 +425,7 @@ describe('ConfigManager', () => {
   });
 
   describe('addSkill', () => {
-    it('adds a new skill entry to config', async () => {
+    it('adds a new skill entry to config when skills is an array', async () => {
       const config: DevKitConfig = {
         version: '1.0.0',
         environments: ['cursor'],
@@ -444,10 +444,46 @@ describe('ConfigManager', () => {
         name: 'memory'
       });
 
-      expect(result.skills).toEqual([
-        { registry: 'codeaholicguy/ai-devkit', name: 'debug' },
-        { registry: 'codeaholicguy/ai-devkit', name: 'memory' }
-      ]);
+      expect(result.skills).toEqual({
+        installed: [
+          { registry: 'codeaholicguy/ai-devkit', name: 'debug' },
+          { registry: 'codeaholicguy/ai-devkit', name: 'memory' }
+        ]
+      });
+      expect(mockFs.writeJson).toHaveBeenCalled();
+    });
+
+    it('adds a new skill entry when skills is an object with registries', async () => {
+      const config = {
+        version: '1.0.0',
+        environments: ['cursor'],
+        phases: [],
+        skills: {
+          registries: {
+            'custom/repo': 'https://github.com/custom/repo.git'
+          }
+        },
+        createdAt: '2024-01-01T00:00:00.000Z',
+        updatedAt: '2024-01-01T00:00:00.000Z'
+      };
+
+      (mockFs.pathExists as any).mockResolvedValue(true);
+      (mockFs.readJson as any).mockResolvedValue(config);
+      (mockFs.writeJson as any).mockResolvedValue(undefined);
+
+      const result = await configManager.addSkill({
+        registry: 'custom/repo',
+        name: 'my-skill'
+      });
+
+      expect(result.skills).toEqual({
+        registries: {
+          'custom/repo': 'https://github.com/custom/repo.git'
+        },
+        installed: [
+          { registry: 'custom/repo', name: 'my-skill' }
+        ]
+      });
       expect(mockFs.writeJson).toHaveBeenCalled();
     });
 
@@ -472,6 +508,114 @@ describe('ConfigManager', () => {
       expect(result.skills).toEqual([{ registry: 'codeaholicguy/ai-devkit', name: 'debug' }]);
       expect(mockFs.writeJson).not.toHaveBeenCalled();
     });
+
+    it('adds skill when skills is undefined', async () => {
+      const config: DevKitConfig = {
+        version: '1.0.0',
+        environments: ['cursor'],
+        phases: [],
+        createdAt: '2024-01-01T00:00:00.000Z',
+        updatedAt: '2024-01-01T00:00:00.000Z'
+      };
+
+      (mockFs.pathExists as any).mockResolvedValue(true);
+      (mockFs.readJson as any).mockResolvedValue(config);
+      (mockFs.writeJson as any).mockResolvedValue(undefined);
+
+      const result = await configManager.addSkill({
+        registry: 'codeaholicguy/ai-devkit',
+        name: 'memory'
+      });
+
+      expect(result.skills).toEqual({
+        installed: [
+          { registry: 'codeaholicguy/ai-devkit', name: 'memory' }
+        ]
+      });
+      expect(mockFs.writeJson).toHaveBeenCalled();
+    });
+  });
+
+  describe('normalizeSkillsConfig', () => {
+    it('normalizes an array to SkillsConfig', () => {
+      const result = configManager.normalizeSkillsConfig([
+        { registry: 'r', name: 's' }
+      ]);
+      expect(result).toEqual({
+        installed: [{ registry: 'r', name: 's' }]
+      });
+    });
+
+    it('normalizes an object with registries and installed', () => {
+      const result = configManager.normalizeSkillsConfig({
+        registries: { 'r': 'https://example.com' },
+        installed: [{ registry: 'r', name: 's' }]
+      });
+      expect(result).toEqual({
+        registries: { 'r': 'https://example.com' },
+        installed: [{ registry: 'r', name: 's' }]
+      });
+    });
+
+    it('normalizes an object with only registries', () => {
+      const result = configManager.normalizeSkillsConfig({
+        registries: { 'r': 'https://example.com' }
+      });
+      expect(result).toEqual({
+        registries: { 'r': 'https://example.com' },
+        installed: []
+      });
+    });
+
+    it('normalizes undefined to empty SkillsConfig', () => {
+      const result = configManager.normalizeSkillsConfig(undefined);
+      expect(result).toEqual({ installed: [] });
+    });
+
+    it('normalizes null to empty SkillsConfig', () => {
+      const result = configManager.normalizeSkillsConfig(null);
+      expect(result).toEqual({ installed: [] });
+    });
+  });
+
+  describe('getInstalledSkills', () => {
+    it('returns skills from array format', () => {
+      const config: DevKitConfig = {
+        version: '1.0.0',
+        environments: [],
+        phases: [],
+        skills: [{ registry: 'r', name: 's' }],
+        createdAt: '',
+        updatedAt: ''
+      };
+      expect(configManager.getInstalledSkills(config)).toEqual([{ registry: 'r', name: 's' }]);
+    });
+
+    it('returns skills from object format', () => {
+      const config = {
+        version: '1.0.0',
+        environments: [],
+        phases: [],
+        skills: {
+          registries: { 'r': 'https://example.com' },
+          installed: [{ registry: 'r', name: 's' }]
+        },
+        createdAt: '',
+        updatedAt: ''
+      } as DevKitConfig;
+      expect(configManager.getInstalledSkills(config)).toEqual([{ registry: 'r', name: 's' }]);
+    });
+
+    it('returns empty array when skills is undefined', () => {
+      const config: DevKitConfig = {
+        version: '1.0.0',
+        environments: [],
+        phases: [],
+        createdAt: '',
+        updatedAt: ''
+      };
+      expect(configManager.getInstalledSkills(config)).toEqual([]);
+    });
   });
 
   describe('getSkillRegistries', () => {

packages/cli/src/lib/Config.ts

@@ -1,6 +1,6 @@
 import * as fs from 'fs-extra';
 import * as path from 'path';
-import { DevKitConfig, Phase, EnvironmentCode, ConfigSkill, DEFAULT_DOCS_DIR } from '../types';
+import { DevKitConfig, Phase, EnvironmentCode, ConfigSkill, SkillsConfig, DEFAULT_DOCS_DIR } from '../types';
 import packageJson from '../../package.json';
 
 const CONFIG_FILE_NAME = '.ai-devkit.json';
@@ -127,34 +127,50 @@ export class ConfigManager {
     return environments.includes(envId);
   }
 
+  normalizeSkillsConfig(raw: unknown): SkillsConfig {
+    if (Array.isArray(raw)) {
+      return { installed: raw };
+    }
+    if (raw && typeof raw === 'object' && !Array.isArray(raw)) {
+      const obj = raw as Record<string, unknown>;
+      return {
+        registries: obj.registries as Record<string, string> | undefined,
+        installed: Array.isArray(obj.installed) ? obj.installed as ConfigSkill[] : []
+      };
+    }
+    return { installed: [] };
+  }
+
   async addSkill(skill: ConfigSkill): Promise<DevKitConfig> {
     const config = await this.read();
     if (!config) {
       throw new Error('Config file not found. Run ai-devkit init first.');
     }
 
-    const skills = config.skills || [];
-    const exists = skills.some(
+    const normalized = this.normalizeSkillsConfig(config.skills);
+    const installed = normalized.installed || [];
+
+    const exists = installed.some(
       entry => entry.registry === skill.registry && entry.name === skill.name
     );
 
     if (exists) {
       return config;
     }
 
-    skills.push(skill);
-    return this.update({ skills });
+    installed.push(skill);
+    normalized.installed = installed;
+    return this.update({ skills: normalized });
   }
 
   async getSkillRegistries(): Promise<Record<string, string>> {
-    const config = await this.read() as any;
-    const rootRegistries = config?.registries;
-    const nestedRegistries =
-      config?.skills && !Array.isArray(config.skills)
-        ? config.skills.registries
-        : undefined;
+    const config = await this.read();
+    if (!config) {
+      return {};
+    }
 
-    const registries = rootRegistries ?? nestedRegistries;
+    const normalized = this.normalizeSkillsConfig(config.skills);
+    const registries = config.registries ?? normalized.registries;
 
     if (!registries || typeof registries !== 'object' || Array.isArray(registries)) {
       return {};
@@ -164,4 +180,8 @@ export class ConfigManager {
       Object.entries(registries).filter(([, value]) => typeof value === 'string')
     ) as Record<string, string>;
   }
+
+  getInstalledSkills(config: DevKitConfig): ConfigSkill[] {
+    return this.normalizeSkillsConfig(config.skills).installed || [];
+  }
 }

packages/cli/src/types.ts

@@ -24,6 +24,11 @@ export type EnvironmentCode = 'cursor' | 'claude' | 'github' | 'gemini' | 'codex
 
 export const DEFAULT_DOCS_DIR = 'docs/ai';
 
+export interface SkillsConfig {
+  registries?: Record<string, string>;
+  installed?: ConfigSkill[];
+}
+
 export interface DevKitConfig {
   version: string;
   paths?: {
@@ -34,7 +39,8 @@ export interface DevKitConfig {
   };
   environments: EnvironmentCode[];
   phases: Phase[];
-  skills?: ConfigSkill[];
+  skills?: ConfigSkill[] | SkillsConfig;
+  registries?: Record<string, string>;
   createdAt: string;
   updatedAt: string;
 }
@@ -44,12 +50,8 @@ export interface ConfigSkill {
   name: string;
 }
 
-export interface SkillRegistriesConfig {
-  registries?: Record<string, string>;
-}
-
 export interface GlobalDevKitConfig {
-  skills?: SkillRegistriesConfig;
+  skills?: SkillsConfig;
 }
 
 export interface PhaseMetadata {