|
| 1 | +--- |
| 2 | +phase: design |
| 3 | +title: System Design & Architecture |
| 4 | +description: Define the technical architecture, components, and data models |
| 5 | +feature: skill-add-interactive-selection |
| 6 | +--- |
| 7 | + |
| 8 | +# System Design & Architecture - Skill Add Interactive Selection |
| 9 | + |
| 10 | +## Architecture Overview |
| 11 | +**What is the high-level system structure?** |
| 12 | + |
| 13 | +```mermaid |
| 14 | +graph TD |
| 15 | + User[User: ai-devkit skill add <registry>] --> SkillCommand |
| 16 | + SkillCommand --> SkillManager |
| 17 | + SkillManager --> RegistryResolver[fetchMergedRegistry] |
| 18 | + SkillManager --> CacheResolver[cloneRepositoryToCache] |
| 19 | + CacheResolver --> RegistryRepo[registry checkout/cache] |
| 20 | + RegistryRepo --> SkillEnumerator[scan skills/*/SKILL.md] |
| 21 | + SkillEnumerator --> Prompt[inquirer list prompt] |
| 22 | + Prompt --> SkillManager |
| 23 | + SkillManager --> Installer[existing addSkill install path] |
| 24 | + Installer --> ConfigManager[project config update] |
| 25 | +``` |
| 26 | + |
| 27 | +- `packages/cli/src/commands/skill.ts` will change `add` from two required args to one required registry arg plus an optional skill arg. |
| 28 | +- `SkillManager` will own the interactive fallback so CLI wiring stays thin and existing validation/cache logic is reused. |
| 29 | +- The prompt list will be built from the selected registry checkout after registry resolution; if refresh fails but cache exists, the cached checkout is still used with a warning. |
| 30 | + |
| 31 | +## Data Models |
| 32 | +**What data do we need to manage?** |
| 33 | + |
| 34 | +```ts |
| 35 | +interface RegistrySkillChoice { |
| 36 | + name: string; |
| 37 | + description?: string; |
| 38 | + skillPath: string; |
| 39 | +} |
| 40 | + |
| 41 | +interface AddSkillOptions { |
| 42 | + global?: boolean; |
| 43 | + environments?: string[]; |
| 44 | + interactive?: boolean; |
| 45 | +} |
| 46 | +``` |
| 47 | + |
| 48 | +- `RegistrySkillChoice` is an internal prompt model only. |
| 49 | +- The persisted config format does not change. |
| 50 | +- Skill names continue to be the canonical installation IDs. |
| 51 | + |
| 52 | +## API Design |
| 53 | +**How do components communicate?** |
| 54 | + |
| 55 | +**CLI surface:** |
| 56 | + |
| 57 | +- Existing explicit form remains: |
| 58 | + - `ai-devkit skill add <registry> <skill-name>` |
| 59 | +- New interactive shorthand: |
| 60 | + - `ai-devkit skill add <registry>` |
| 61 | + |
| 62 | +**Internal interfaces (proposed):** |
| 63 | + |
| 64 | +```ts |
| 65 | +async addSkill(registryId: string, skillName?: string, options?: AddSkillOptions): Promise<void>; |
| 66 | +async listRegistrySkills(registryId: string): Promise<RegistrySkillChoice[]>; |
| 67 | +async promptForSkillSelection(skills: RegistrySkillChoice[]): Promise<string>; |
| 68 | +``` |
| 69 | + |
| 70 | +**Behavior contract:** |
| 71 | + |
| 72 | +- If `skillName` is provided, skip prompting. |
| 73 | +- If `skillName` is missing and `stdout`/`stdin` are interactive, enumerate skills and prompt. |
| 74 | +- If `skillName` is missing in a non-interactive context, fail with an error instructing the user to provide `<skill-name>`. |
| 75 | +- If the prompt is cancelled, exit without side effects. |
| 76 | +- If exactly one valid skill exists and `skillName` is omitted, still show the selector instead of auto-installing. |
| 77 | + |
| 78 | +## Component Breakdown |
| 79 | +**What are the major building blocks?** |
| 80 | + |
| 81 | +1. `packages/cli/src/commands/skill.ts` |
| 82 | + - Update the command signature to optional `[skill-name]`. |
| 83 | + - Pass control to `SkillManager.addSkill`. |
| 84 | +2. `packages/cli/src/lib/SkillManager.ts` |
| 85 | + - Split current `addSkill` flow into: |
| 86 | + - registry resolution and cache preparation |
| 87 | + - optional interactive skill selection |
| 88 | + - existing installation logic |
| 89 | + - Add a helper that enumerates valid skills from the cloned registry. |
| 90 | + - Add a helper that prompts with `inquirer`. |
| 91 | +3. `packages/cli/src/__tests__/commands/skill.test.ts` |
| 92 | + - Add command-level coverage for omitted skill name. |
| 93 | +4. `packages/cli/src/__tests__/lib/SkillManager.test.ts` |
| 94 | + - Add behavior coverage for enumeration, prompt selection, cancellation, and non-interactive mode. |
| 95 | + |
| 96 | +## Design Decisions |
| 97 | +**Why did we choose this approach?** |
| 98 | + |
| 99 | +- Enumerate from the resolved registry checkout instead of the global search index: |
| 100 | + - It guarantees the list reflects the exact target registry the user requested. |
| 101 | + - It works with custom registries that may not yet be indexed. |
| 102 | + - It avoids coupling install behavior to index freshness. |
| 103 | +- Keep interactive selection explicit even for single-skill registries: |
| 104 | + - It matches the stated UX requirement. |
| 105 | + - It avoids hidden behavior changes between one-skill and multi-skill registries. |
| 106 | +- Prefer cached registry contents when refresh fails: |
| 107 | + - It keeps the command usable offline or during transient network failures. |
| 108 | + - It aligns with existing cache-oriented registry behavior. |
| 109 | +- Keep the prompt in `SkillManager`: |
| 110 | + - Registry validation, caching, and installation already live there. |
| 111 | + - The command layer should not duplicate repo-reading logic. |
| 112 | +- Fail in non-interactive mode when the skill name is omitted: |
| 113 | + - This preserves scriptability and avoids hanging CI jobs. |
| 114 | + |
| 115 | +**Alternatives considered:** |
| 116 | + |
| 117 | +- Use `skill find` index results to populate the prompt. |
| 118 | + - Rejected because it is broader than the selected registry and may be stale. |
| 119 | +- Always auto-install when a registry has exactly one skill. |
| 120 | + - Rejected for now to keep behavior explicit and predictable. |
| 121 | + |
| 122 | +## Non-Functional Requirements |
| 123 | +**How should the system perform?** |
| 124 | + |
| 125 | +- Performance: |
| 126 | + - Interactive enumeration should reuse the existing cache and only fetch/update the chosen registry once. |
| 127 | +- Reliability: |
| 128 | + - Invalid skill folders are skipped during enumeration instead of breaking the entire list. |
| 129 | + - Empty registries produce a clear error. |
| 130 | + - Refresh failures degrade to cached registry contents when available. |
| 131 | +- Security: |
| 132 | + - Continue validating `registryId` and selected `skillName` before installation. |
| 133 | +- Usability: |
| 134 | + - Prompt entries should display skill name and short description when available. |
0 commit comments