diff --git a/docs/ai/design/feature-skill-add-interactive-selection.md b/docs/ai/design/feature-skill-add-interactive-selection.md new file mode 100644 index 00000000..9ee3804d --- /dev/null +++ b/docs/ai/design/feature-skill-add-interactive-selection.md @@ -0,0 +1,137 @@ +--- +phase: design +title: System Design & Architecture +description: Define the technical architecture, components, and data models +feature: skill-add-interactive-selection +--- + +# System Design & Architecture - Skill Add Interactive Selection + +## Architecture Overview +**What is the high-level system structure?** + +```mermaid +graph TD + User[User: ai-devkit skill add ] --> SkillCommand + SkillCommand --> SkillManager + SkillManager --> RegistryResolver[fetchMergedRegistry] + SkillManager --> CacheResolver[cloneRepositoryToCache] + CacheResolver --> RegistryRepo[registry checkout/cache] + RegistryRepo --> SkillEnumerator[scan skills/*/SKILL.md] + SkillEnumerator --> Prompt[inquirer checkbox prompt] + Prompt --> SkillManager + SkillManager --> Installer[existing addSkill install path] + Installer --> ConfigManager[project config update] +``` + +- `packages/cli/src/commands/skill.ts` will change `add` from two required args to one required registry arg plus an optional skill arg. +- `SkillManager` will own the interactive fallback so CLI wiring stays thin and existing validation/cache logic is reused. +- 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. + +## Data Models +**What data do we need to manage?** + +```ts +interface RegistrySkillChoice { + name: string; + description?: string; +} + +interface AddSkillOptions { + global?: boolean; + environments?: string[]; +} +``` + +- `RegistrySkillChoice` is an internal prompt model only. +- The persisted config format does not change. +- Skill names continue to be the canonical installation IDs. + +## API Design +**How do components communicate?** + +**CLI surface:** + +- Existing explicit form remains: + - `ai-devkit skill add ` +- New interactive shorthand: + - `ai-devkit skill add ` + +**Internal interfaces (proposed):** + +```ts +async addSkill(registryId: string, skillName?: string, options?: AddSkillOptions): Promise; +async listRegistrySkills(registryId: string): Promise; +async promptForSkillSelection(skills: RegistrySkillChoice[]): Promise; +``` + +**Behavior contract:** + +- If `skillName` is provided, skip prompting. +- If `skillName` is missing and `stdout`/`stdin` are interactive, enumerate skills and prompt for one or more selections. +- If `skillName` is missing in a non-interactive context, fail with an error instructing the user to provide ``. +- If the prompt is cancelled, exit without side effects. +- If exactly one valid skill exists and `skillName` is omitted, still show the selector instead of auto-installing. + +## Component Breakdown +**What are the major building blocks?** + +1. `packages/cli/src/commands/skill.ts` + - Update the command signature to optional `[skill-name]`. + - Pass control to `SkillManager.addSkill`. +2. `packages/cli/src/lib/SkillManager.ts` + - Split current `addSkill` flow into: + - registry resolution and cache preparation + - optional interactive skill selection + - existing installation logic + - Add a helper that enumerates valid skills from the cloned registry. + - Add a helper that prompts with `inquirer`. + - Reuse the existing install path for each selected skill. +3. `packages/cli/src/__tests__/commands/skill.test.ts` + - Add command-level coverage for omitted skill name. +4. `packages/cli/src/__tests__/lib/SkillManager.test.ts` + - Add behavior coverage for enumeration, prompt selection, cancellation, and non-interactive mode. + +## Design Decisions +**Why did we choose this approach?** + +- Enumerate from the resolved registry checkout instead of the global search index: + - It guarantees the list reflects the exact target registry the user requested. + - It works with custom registries that may not yet be indexed. + - It avoids coupling install behavior to index freshness. +- Keep interactive selection explicit even for single-skill registries: + - It matches the stated UX requirement. + - It avoids hidden behavior changes between one-skill and multi-skill registries. +- Allow multi-select installation in the prompt: + - It reduces repetitive command invocations when a user wants several skills from the same registry. + - It keeps the explicit two-argument command unchanged for scripted single-skill installs. +- Prefer cached registry contents when refresh fails: + - It keeps the command usable offline or during transient network failures. + - It aligns with existing cache-oriented registry behavior. +- Keep the prompt in `SkillManager`: + - Registry validation, caching, and installation already live there. + - The command layer should not duplicate repo-reading logic. +- Fail in non-interactive mode when the skill name is omitted: + - This preserves scriptability and avoids hanging CI jobs. + +**Alternatives considered:** + +- Use `skill find` index results to populate the prompt. + - Rejected because it is broader than the selected registry and may be stale. +- Always auto-install when a registry has exactly one skill. + - Rejected for now to keep behavior explicit and predictable. + +## Non-Functional Requirements +**How should the system perform?** + +- Performance: + - Interactive enumeration should reuse the existing cache and only fetch/update the chosen registry once. +- Reliability: + - Invalid skill folders are skipped during enumeration instead of breaking the entire list. + - Empty registries produce a clear error. + - Refresh failures degrade to cached registry contents when available. +- Security: + - Continue validating `registryId` and selected `skillName` before installation. +- Usability: + - Prompt entries should display skill name and short description when available. + - Users should be able to select multiple skills in one prompt. diff --git a/docs/ai/implementation/feature-skill-add-interactive-selection.md b/docs/ai/implementation/feature-skill-add-interactive-selection.md new file mode 100644 index 00000000..9c1ef660 --- /dev/null +++ b/docs/ai/implementation/feature-skill-add-interactive-selection.md @@ -0,0 +1,62 @@ +--- +phase: implementation +title: Implementation Guide +description: Technical implementation notes, patterns, and code guidelines +feature: skill-add-interactive-selection +--- + +# Implementation Guide - Skill Add Interactive Selection + +## Development Setup +**How do we get started?** + +- Work in branch `feature-skill-add-interactive-selection`. +- Use the existing CLI test setup under `packages/cli/src/__tests__`. +- Reuse `inquirer` and existing `SkillManager` helpers instead of adding new dependencies. + +## Code Structure +**How is the code organized?** + +- Command entrypoint: `packages/cli/src/commands/skill.ts` +- Main orchestration: `packages/cli/src/lib/SkillManager.ts` +- Tests: `packages/cli/src/__tests__/commands/skill.test.ts` and `packages/cli/src/__tests__/lib/SkillManager.test.ts` + +## Implementation Notes +**Key technical details to remember:** + +### Core Features +- Feature 1: Accept an omitted `` argument in the `skill add` command. +- Feature 2: Enumerate installable skills from the resolved registry checkout. +- Feature 3: Prompt for one skill and hand the result back into the existing install path. + +### Patterns & Best Practices +- Keep explicit two-argument installs on the current path. +- Isolate prompt selection from installation side effects. +- Skip malformed entries instead of failing enumeration wholesale. + +## Integration Points +**How do pieces connect?** + +- Registry lookup continues through merged registry resolution. +- Cache refresh continues through the current clone/pull helpers. +- Project config updates remain in `ConfigManager.addSkill`. + +## Error Handling +**How do we handle failures?** + +- Missing registry: fail before prompting. +- Empty registry: fail with a message explaining no valid skills were found. +- Prompt cancellation: exit cleanly without installation. +- Non-interactive invocation without skill name: fail with explicit remediation text. + +## Performance Considerations +**How do we keep it fast?** + +- Reuse the cloned cache after registry resolution. +- Read only direct `skills/*/SKILL.md` entries needed to build the prompt. + +## Security Notes +**What security measures are in place?** + +- Preserve existing registry and skill name validation. +- Do not install unvalidated paths derived from arbitrary user input. diff --git a/docs/ai/planning/feature-skill-add-interactive-selection.md b/docs/ai/planning/feature-skill-add-interactive-selection.md new file mode 100644 index 00000000..85ecfa41 --- /dev/null +++ b/docs/ai/planning/feature-skill-add-interactive-selection.md @@ -0,0 +1,75 @@ +--- +phase: planning +title: Project Planning & Task Breakdown +description: Break down work into actionable tasks and estimate timeline +feature: skill-add-interactive-selection +--- + +# Project Planning & Task Breakdown - Skill Add Interactive Selection + +## Milestones +**What are the major checkpoints?** + +- [x] Milestone 1: Command contract updated to allow omitted skill name without breaking explicit installs. +- [x] Milestone 2: `SkillManager` can enumerate registry skills and prompt for one interactively. +- [x] Milestone 3: Tests cover prompt and non-prompt flows, plus failure cases. + +## Task Breakdown +**What specific work needs to be done?** + +### Phase 1: Command Surface +- [x] Task 1.1: Update `packages/cli/src/commands/skill.ts` so `add` accepts `[skill-name]`. +- [x] Task 1.2: Update command descriptions/help text to document the interactive shorthand. + +### Phase 2: Interactive Selection Flow +- [x] Task 2.1: Refactor `SkillManager.addSkill` so it can resolve a missing skill name before install. +- [x] Task 2.2: Implement registry skill enumeration from the cloned/cached repository. +- [x] Task 2.3: Implement an `inquirer` selection prompt with skill name and short description labels. +- [x] Task 2.4: Handle cancel, empty registry, invalid registry, and non-interactive contexts cleanly. + +### Phase 3: Validation & Regression Coverage +- [x] Task 3.1: Add `SkillManager` unit tests for enumeration and prompt behavior. +- [x] Task 3.2: Add command-level tests for `ai-devkit skill add ` and explicit two-arg installs. +- [x] Task 3.3: Verify no regression in config updates and environment resolution after interactive selection. + +## Dependencies +**What needs to happen in what order?** + +```mermaid +graph TD + T11[1.1 Optional skill arg] --> T21[2.1 Refactor addSkill] + T21 --> T22[2.2 Enumerate registry skills] + T22 --> T23[2.3 Prompt selection] + T23 --> T24[2.4 Edge-case handling] + T24 --> T31[3.1 SkillManager tests] + T11 --> T32[3.2 Command tests] + T23 --> T33[3.3 Regression verification] +``` + +## Timeline & Estimates +**When will things be done?** + +- Phase 1: completed +- Phase 2: completed +- Phase 3: completed +- Total implementation effort: completed within the current session + +## Risks & Mitigation +**What could go wrong?** + +- Risk: Some registries contain nested or malformed skill directories. + - Mitigation: enumerate only folders containing `SKILL.md` and skip invalid entries. +- Risk: Prompt behavior makes CI jobs hang. + - Mitigation: detect non-interactive execution and require explicit ``. +- Risk: Refactoring `addSkill` accidentally changes direct-install behavior. + - Mitigation: keep installation steps intact after skill resolution and add regression tests for the two-arg path. + +## Resources Needed +**What do we need to succeed?** + +- Existing `inquirer` dependency already used across the CLI. +- Existing `SkillManager` cache and registry resolution helpers. +- Jest command/lib test suites for regression coverage. + +## Progress Summary +Implementation is complete for the current scope. The `skill add` command now accepts an omitted skill name, `SkillManager` resolves available skills from the target registry checkout with cached fallback on refresh failure, and targeted Jest coverage verifies direct install, interactive multi-selection, cancellation, non-interactive failure, and cache-backed selection behavior. Remaining lifecycle work should move to implementation review rather than additional Phase 4 coding unless new scope is introduced. diff --git a/docs/ai/requirements/feature-skill-add-interactive-selection.md b/docs/ai/requirements/feature-skill-add-interactive-selection.md new file mode 100644 index 00000000..03600e19 --- /dev/null +++ b/docs/ai/requirements/feature-skill-add-interactive-selection.md @@ -0,0 +1,111 @@ +--- +phase: requirements +title: Requirements & Problem Understanding +description: Clarify the problem space, gather requirements, and define success criteria +feature: skill-add-interactive-selection +--- + +# Requirements & Problem Understanding - Skill Add Interactive Selection + +## Problem Statement +**What problem are we solving?** + +- `ai-devkit skill add` currently requires both `` and ``, even when the user already knows the registry but not the exact skill identifier. +- Users installing from a registry often need a discovery step before installation, so they must leave the CLI and inspect the registry manually. +- This creates friction for first-time installs and makes the add flow inconsistent with the rest of the CLI, which already uses interactive prompts in several commands. + +**Who is affected by this problem?** + +- Developers using `ai-devkit skill add ` without knowing the exact skill name. +- Teams exposing many skills from a private or custom registry. +- New users evaluating available skills before installation. + +**What is the current situation/workaround?** + +- Users must inspect the registry repository manually and identify the skill folder name under `skills/`. +- If they omit the skill name, the command fails at CLI argument parsing instead of helping them continue interactively. + +## Goals & Objectives +**What do we want to achieve?** + +**Primary goals:** + +- Allow `ai-devkit skill add ` to enter an interactive selection flow when `` is omitted. +- Build the selectable list from the requested registry itself, not from a hardcoded list. +- Reuse the existing installation path once the user selects one or more skills. +- Keep the existing explicit flow `ai-devkit skill add ` unchanged. + +**Secondary goals:** + +- Show clear, user-friendly errors when the registry is missing, empty, or cannot be read. +- Support the same registry sources already supported by `SkillManager` (default, global custom, project custom, cached). +- Keep the selection labels descriptive enough for users to distinguish similar skills. + +**Non-goals (explicitly out of scope):** + +- Fuzzy search across all registries in the add flow. +- Changing `skill find` behavior. +- Adding a new registry metadata format. + +## User Stories & Use Cases +**How will users interact with the solution?** + +1. As a developer, I want to run `ai-devkit skill add my-org/skills` so I can choose one or more skills interactively when I do not remember the exact skill names. +2. As a developer, I want the CLI to show the actual skills available in that registry so I can install several of them without opening GitHub. +3. As an automation user, I want `ai-devkit skill add ` to keep working non-interactively so existing scripts do not break. + +**Key workflows and scenarios:** + +- User runs `ai-devkit skill add ` in a TTY: + - CLI validates the registry. + - CLI fetches or reuses the cached registry repository. + - CLI extracts available skills from `skills/*/SKILL.md`. + - CLI shows an interactive multi-selection list, even if the registry only exposes one valid skill. + - CLI installs each selected skill using the existing add flow. +- User runs `ai-devkit skill add `: + - Existing direct install flow continues with no interactive prompt. +- User cancels the prompt: + - CLI exits without installing anything and reports cancellation clearly. +- Registry refresh fails but a cached copy exists: + - CLI warns and uses the cached registry contents to build the selection list. + +**Edge cases to consider:** + +- Registry ID does not exist and is not cached. +- Registry exists but contains no valid skills. +- Registry contains directories without `SKILL.md`. +- Prompt is triggered in a non-interactive environment. +- Cached registry is stale or update fails before enumeration. + +## Success Criteria +**How will we know when we're done?** + +- `ai-devkit skill add ` is accepted by the CLI. +- When run interactively, the command displays a selection list populated from the target registry. +- The command still shows the selection list when the registry contains exactly one valid skill. +- Selecting one or more skills installs each of them through the existing installation path and updates project config exactly as today. +- `ai-devkit skill add ` continues to work without prompting. +- Invalid, empty, and non-interactive cases return actionable error messages. +- If registry refresh fails but a cached copy exists, the command warns and uses the cached list. +- Automated tests cover direct install, interactive selection, cancellation, empty registry, and non-TTY behavior. + +## Constraints & Assumptions +**What limitations do we need to work within?** + +**Technical constraints:** + +- Registry resolution must remain consistent with existing merged registry behavior. +- Skill discovery should rely on the registry repository structure already assumed elsewhere: `skills//SKILL.md`. +- The feature should reuse the current `inquirer` dependency rather than adding a new prompt library. + +**Assumptions:** + +- The selected registry is either configured remotely or already available in the local cache. +- Skill folder names remain the install identifiers. +- Description text can be derived from `SKILL.md` when available, or omitted/fallback when not available. +- If a skill name is explicitly provided in the command, direct installation remains the highest-priority path. + +## Questions & Open Items +**What do we still need to clarify?** + +- None for Phase 2 review. The prompt uses a multi-select list whenever `` is omitted, and cached registry content is acceptable when refresh fails. diff --git a/docs/ai/testing/feature-skill-add-interactive-selection.md b/docs/ai/testing/feature-skill-add-interactive-selection.md new file mode 100644 index 00000000..9689ce54 --- /dev/null +++ b/docs/ai/testing/feature-skill-add-interactive-selection.md @@ -0,0 +1,87 @@ +--- +phase: testing +title: Testing Strategy +description: Define testing approach, test cases, and quality assurance +feature: skill-add-interactive-selection +--- + +# Testing Strategy - Skill Add Interactive Selection + +## Test Coverage Goals +**What level of testing do we aim for?** + +- Unit test coverage target: 100% of new or changed logic in `SkillManager` and command parsing. +- Integration scope: interactive add flow, explicit add flow, and failure cases. +- End-to-end scope: optional later validation through the packaged CLI if command-level tests are insufficient. + +## Unit Tests +**What individual components need testing?** + +### SkillManager +- [x] Test case 1: Omitting `skillName` triggers registry enumeration and prompt selection. +- [x] Test case 2: Providing `skillName` skips the prompt entirely. +- [x] Test case 3: Invalid registry throws before prompt. +- [x] Test case 4: Empty registry throws a clear error. +- [x] Test case 5: Non-interactive mode without `skillName` throws a clear error. +- [x] Test case 6: Prompt cancellation exits without config writes or installs. +- [x] Test case 7: Multi-selection installs more than one skill in a single run. +- [x] Test case 8: Cached registry contents are used when refresh fails. +- [x] Test case 9: Global installation still works after interactive multi-selection. + +### Skill Command +- [x] Test case 1: `skill add ` is parsed successfully. +- [x] Test case 2: `skill add ` still forwards both args correctly. +- [x] Test case 3: Cancellation is surfaced as a warning instead of an error exit. +- [x] Test case 4: Command shape reflects the optional `[skill-name]` argument. + +## Integration Tests +**How do we test component interactions?** + +- [x] Registry cache is prepared before enumeration. +- [x] Selected skill flows into the existing install path and config update. +- [x] Global install options still work after interactive selection. + +## End-to-End Tests +**What user flows need validation?** + +- [x] User flow 1: Install skill(s) from a registry by selecting from the prompt. +- [x] User flow 2: Install a known skill directly with two arguments. +- [x] User flow 3: Cancel out of the prompt with no side effects. + +## Test Data +**What data do we use for testing?** + +- Mock registry repositories with valid `skills//SKILL.md` folders. +- One malformed skill directory fixture to verify skip behavior. +- Mocked prompt responses for selection and cancellation. +- TTY stubs for interactive vs non-interactive command behavior. + +## Test Reporting & Coverage +**How do we verify and communicate test results?** + +- Run focused Jest suites for `commands/skill` and `lib/SkillManager`. +- Confirm changed branches include prompt, no-prompt, and error paths. +- Latest verification evidence: + - `npm test -- skill.test.ts` + - `npm test -- SkillManager.test.ts` + - `npx ai-devkit@latest lint --feature skill-add-interactive-selection` +- Remaining coverage gap: + - no command-level assertion for full rendered help output text; current coverage checks the registered argument shape instead + +## Manual Testing +**What requires human validation?** + +- Prompt readability when many skills are present. +- Cancellation UX in a real terminal session. +- Global vs project install messaging after selection. +- Checkbox prompt usability when only one skill is available. + +## Performance Testing +**How do we validate performance?** + +- Ensure registry enumeration remains acceptable for registries with many skill folders. + +## Bug Tracking +**How do we manage issues?** + +- Record any direct-install regressions as blockers because they affect existing scripted usage. diff --git a/packages/cli/src/__tests__/commands/skill.test.ts b/packages/cli/src/__tests__/commands/skill.test.ts new file mode 100644 index 00000000..963a020b --- /dev/null +++ b/packages/cli/src/__tests__/commands/skill.test.ts @@ -0,0 +1,88 @@ +import { Command } from 'commander'; +import { beforeEach, describe, expect, it, jest } from '@jest/globals'; +import { registerSkillCommand } from '../../commands/skill'; +import { ui } from '../../util/terminal-ui'; + +const mockAddSkill = jest.fn(); + +jest.mock('../../lib/Config', () => ({ + ConfigManager: jest.fn(), +})); + +jest.mock('../../lib/SkillManager', () => ({ + SkillManager: jest.fn(() => ({ + addSkill: (...args: unknown[]) => mockAddSkill(...args), + listSkills: jest.fn(), + removeSkill: jest.fn(), + updateSkills: jest.fn(), + findSkills: jest.fn(), + rebuildIndex: jest.fn(), + })), +})); + +jest.mock('../../util/terminal-ui', () => ({ + ui: { + error: jest.fn(), + warning: jest.fn(), + info: jest.fn(), + text: jest.fn(), + table: jest.fn(), + }, +})); + +describe('skill command', () => { + beforeEach(() => { + jest.clearAllMocks(); + mockAddSkill.mockImplementation(async () => undefined); + jest.spyOn(process, 'exit').mockImplementation((() => undefined) as any); + }); + + it('parses skill add with registry only and forwards undefined skill name', async () => { + const program = new Command(); + registerSkillCommand(program); + + await program.parseAsync(['node', 'test', 'skill', 'add', 'anthropics/skills']); + + expect(mockAddSkill).toHaveBeenCalledWith('anthropics/skills', undefined, { + global: undefined, + environments: undefined, + }); + }); + + it('parses skill add with explicit skill name and forwards both args', async () => { + const program = new Command(); + registerSkillCommand(program); + + await program.parseAsync(['node', 'test', 'skill', 'add', 'anthropics/skills', 'frontend-design']); + + expect(mockAddSkill).toHaveBeenCalledWith('anthropics/skills', 'frontend-design', { + global: undefined, + environments: undefined, + }); + }); + + it('shows a warning instead of exiting when skill selection is cancelled', async () => { + mockAddSkill.mockImplementation(async () => { + throw new Error('Skill selection cancelled.'); + }); + + const program = new Command(); + registerSkillCommand(program); + + await program.parseAsync(['node', 'test', 'skill', 'add', 'anthropics/skills']); + + expect(ui.warning).toHaveBeenCalledWith('Skill selection cancelled.'); + expect(ui.error).not.toHaveBeenCalled(); + }); + + it('registers the add command with an optional skill-name argument', () => { + const program = new Command(); + registerSkillCommand(program); + + const skillCommand = program.commands.find(command => command.name() === 'skill'); + const addCommand = skillCommand?.commands.find(command => command.name() === 'add'); + + expect(addCommand?.usage()).toContain(''); + expect(addCommand?.usage()).toContain(''); + }); +}); diff --git a/packages/cli/src/__tests__/lib/SkillManager.test.ts b/packages/cli/src/__tests__/lib/SkillManager.test.ts index 0e0b2ddd..9fa25fc7 100644 --- a/packages/cli/src/__tests__/lib/SkillManager.test.ts +++ b/packages/cli/src/__tests__/lib/SkillManager.test.ts @@ -24,6 +24,13 @@ jest.mock("../../lib/EnvironmentSelector"); jest.mock("../../lib/GlobalConfig"); jest.mock("../../util/git"); jest.mock("../../util/skill"); +const mockPrompt = jest.fn(); +jest.mock("inquirer", () => ({ + __esModule: true, + default: { + prompt: (...args: unknown[]) => mockPrompt(...args), + }, +})); jest.mock("ora", () => { return jest.fn(() => ({ start: jest.fn().mockReturnThis(), @@ -56,6 +63,34 @@ function mockFetch(response: any) { }); } +function setTTY(stdoutIsTTY: boolean, stdinIsTTY: boolean): () => void { + const stdoutDescriptor = Object.getOwnPropertyDescriptor(process.stdout, "isTTY"); + const stdinDescriptor = Object.getOwnPropertyDescriptor(process.stdin, "isTTY"); + + Object.defineProperty(process.stdout, "isTTY", { + configurable: true, + value: stdoutIsTTY, + }); + Object.defineProperty(process.stdin, "isTTY", { + configurable: true, + value: stdinIsTTY, + }); + + return () => { + if (stdoutDescriptor) { + Object.defineProperty(process.stdout, "isTTY", stdoutDescriptor); + } else { + delete (process.stdout as any).isTTY; + } + + if (stdinDescriptor) { + Object.defineProperty(process.stdin, "isTTY", stdinDescriptor); + } else { + delete (process.stdin as any).isTTY; + } + }; +} + describe("SkillManager", () => { @@ -112,11 +147,15 @@ describe("SkillManager", () => { }); mockedGitUtil.cloneRepository.mockResolvedValue(mockRepoPath); + mockedGitUtil.isGitRepository.mockResolvedValue(true); + mockedGitUtil.pullRepository.mockResolvedValue(undefined); (mockedFs.pathExists as any).mockResolvedValue(true); (mockedFs.ensureDir as any).mockResolvedValue(undefined); (mockedFs.symlink as any).mockResolvedValue(undefined); (mockedFs.copy as any).mockResolvedValue(undefined); + (mockedFs.readdir as any).mockResolvedValue([]); + (mockedFs.readFile as any)?.mockResolvedValue?.(''); mockConfigManager.read.mockResolvedValue({ environments: ["cursor", "claude"], @@ -127,6 +166,45 @@ describe("SkillManager", () => { ]); }); + const configureRegistrySkills = (skillNames: string[]) => { + (mockedFs.readdir as any).mockResolvedValue( + skillNames.map(name => ({ name, isDirectory: () => true })), + ); + (mockedFs.pathExists as any).mockImplementation((checkPath: string) => { + if (checkPath === mockRepoPath) { + return Promise.resolve(true); + } + if (checkPath.endsWith(`${path.sep}skills`)) { + return Promise.resolve(true); + } + + for (const skillName of skillNames) { + if (checkPath.endsWith(`${path.sep}${skillName}${path.sep}SKILL.md`)) { + return Promise.resolve(true); + } + if (checkPath.includes(`${path.sep}skills${path.sep}${skillName}`)) { + return Promise.resolve(true); + } + } + + return Promise.resolve(false); + }); + (mockedFs.readFile as any) = jest.fn().mockImplementation((filePath: string) => { + const matchedSkill = skillNames.find(skillName => + filePath.endsWith(`${skillName}${path.sep}SKILL.md`), + ); + + return Promise.resolve( + matchedSkill === "frontend-design" + ? "description: Frontend skill" + : "description: Debug skill", + ); + }); + mockedSkillUtil.extractSkillDescription.mockImplementation((content: string) => + content.replace("description: ", ""), + ); + }; + it("should successfully add a skill", async () => { await skillManager.addSkill(mockRegistryId, mockSkillName); @@ -527,6 +605,152 @@ describe("SkillManager", () => { mockSkillName, ); }); + + it("should prompt for multiple skill selection when skill name is omitted", async () => { + configureRegistrySkills(["frontend-design", "debug"]); + mockPrompt.mockResolvedValue({ selectedSkills: ["debug", "frontend-design"] }); + + const restoreTTY = setTTY(true, true); + + await skillManager.addSkill(mockRegistryId, undefined as any); + + expect(mockPrompt).toHaveBeenCalled(); + expect(mockedSkillUtil.validateSkillName).toHaveBeenCalledWith("debug"); + expect(mockedSkillUtil.validateSkillName).toHaveBeenCalledWith("frontend-design"); + expect(mockConfigManager.addSkill).toHaveBeenNthCalledWith(1, { + registry: mockRegistryId, + name: "debug", + }); + expect(mockConfigManager.addSkill).toHaveBeenNthCalledWith(2, { + registry: mockRegistryId, + name: "frontend-design", + }); + expect(mockConfigManager.addSkill).toHaveBeenCalledTimes(2); + + restoreTTY(); + }); + + it("should fail when skill name is omitted in non-interactive mode", async () => { + const restoreTTY = setTTY(false, false); + + await expect( + skillManager.addSkill(mockRegistryId, undefined as any), + ).rejects.toThrow('Skill name is required in non-interactive mode. Re-run with: ai-devkit skill add '); + + expect(mockPrompt).not.toHaveBeenCalled(); + + restoreTTY(); + }); + + it("should use cached registry contents for multi-selection when pull fails", async () => { + configureRegistrySkills(["debug", "frontend-design"]); + mockedGitUtil.pullRepository.mockRejectedValue(new Error('network down')); + mockPrompt.mockResolvedValue({ selectedSkills: ["debug", "frontend-design"] }); + + const restoreTTY = setTTY(true, true); + + await skillManager.addSkill(mockRegistryId, undefined as any); + + expect(mockPrompt).toHaveBeenCalled(); + expect(mockConfigManager.addSkill).toHaveBeenCalledTimes(2); + expect(console.log).toHaveBeenCalledWith( + expect.stringContaining("⚠"), + expect.stringContaining("Using cached registry contents"), + ); + + restoreTTY(); + }); + + it("should stop without installing when skill selection is cancelled", async () => { + configureRegistrySkills(["debug"]); + mockPrompt.mockRejectedValue(new Error('User cancelled')); + + const restoreTTY = setTTY(true, true); + + await expect( + skillManager.addSkill(mockRegistryId, undefined as any), + ).rejects.toThrow('Skill selection cancelled.'); + + expect(mockConfigManager.addSkill).not.toHaveBeenCalled(); + expect(mockedFs.symlink).not.toHaveBeenCalled(); + + restoreTTY(); + }); + + it("should throw a clear error when the registry has no valid skills", async () => { + (mockedFs.readdir as any).mockResolvedValue([ + { name: "broken-skill", isDirectory: () => true }, + ]); + (mockedFs.pathExists as any).mockImplementation((checkPath: string) => { + if (checkPath === mockRepoPath) { + return Promise.resolve(true); + } + if (checkPath.endsWith(`${path.sep}skills`)) { + return Promise.resolve(true); + } + return Promise.resolve(false); + }); + + const restoreTTY = setTTY(true, true); + + await expect( + skillManager.addSkill(mockRegistryId, undefined as any), + ).rejects.toThrow(`No valid skills found in ${mockRegistryId}.`); + + expect(mockPrompt).not.toHaveBeenCalled(); + + restoreTTY(); + }); + + it("should support global installation after interactive multi-selection", async () => { + configureRegistrySkills(["debug", "frontend-design"]); + mockPrompt.mockResolvedValue({ selectedSkills: ["debug", "frontend-design"] }); + (mockedFs.pathExists as any).mockImplementation((checkPath: string) => { + if (checkPath === path.join(os.homedir(), ".claude", "skills", "debug")) { + return Promise.resolve(false); + } + if (checkPath === path.join(os.homedir(), ".claude", "skills", "frontend-design")) { + return Promise.resolve(false); + } + if (checkPath === mockRepoPath) { + return Promise.resolve(true); + } + if (checkPath.endsWith(`${path.sep}skills`)) { + return Promise.resolve(true); + } + if (checkPath.endsWith(`${path.sep}debug${path.sep}SKILL.md`)) { + return Promise.resolve(true); + } + if (checkPath.endsWith(`${path.sep}frontend-design${path.sep}SKILL.md`)) { + return Promise.resolve(true); + } + if (checkPath.includes(`${path.sep}skills${path.sep}debug`)) { + return Promise.resolve(true); + } + if (checkPath.includes(`${path.sep}skills${path.sep}frontend-design`)) { + return Promise.resolve(true); + } + return Promise.resolve(false); + }); + + const restoreTTY = setTTY(true, true); + + await skillManager.addSkill(mockRegistryId, undefined as any, { global: true, environments: ["claude"] }); + + expect(mockedFs.symlink).toHaveBeenCalledWith( + expect.any(String), + path.join(os.homedir(), ".claude", "skills", "debug"), + "dir", + ); + expect(mockedFs.symlink).toHaveBeenCalledWith( + expect.any(String), + path.join(os.homedir(), ".claude", "skills", "frontend-design"), + "dir", + ); + expect(mockConfigManager.addSkill).not.toHaveBeenCalled(); + + restoreTTY(); + }); }); describe("listSkills", () => { diff --git a/packages/cli/src/commands/skill.ts b/packages/cli/src/commands/skill.ts index f2a0138c..f285ac6e 100644 --- a/packages/cli/src/commands/skill.ts +++ b/packages/cli/src/commands/skill.ts @@ -15,7 +15,7 @@ export function registerSkillCommand(program: Command): void { .description('Install a skill from a registry (e.g., ai-devkit skill add anthropics/skills frontend-design)') .option('-g, --global', 'Install skill into configured global skill paths (~/)') .option('-e, --env ', 'Target environment(s) for global install (e.g., --global --env claude)') - .action(async (registryRepo: string, skillName: string, options: { global?: boolean; env?: string[] }) => { + .action(async (registryRepo: string, skillName: string | undefined, options: { global?: boolean; env?: string[] }) => { try { const configManager = new ConfigManager(); const skillManager = new SkillManager(configManager); @@ -25,6 +25,10 @@ export function registerSkillCommand(program: Command): void { environments: options.env, }); } catch (error: any) { + if (error.message === 'Skill selection cancelled.') { + ui.warning('Skill selection cancelled.'); + return; + } ui.error(`Failed to add skill: ${error.message}`); process.exit(1); } diff --git a/packages/cli/src/lib/SkillManager.ts b/packages/cli/src/lib/SkillManager.ts index 8800fb49..31e3660c 100644 --- a/packages/cli/src/lib/SkillManager.ts +++ b/packages/cli/src/lib/SkillManager.ts @@ -1,6 +1,7 @@ import * as fs from 'fs-extra'; import * as path from 'path'; import * as os from 'os'; +import inquirer from 'inquirer'; import { ConfigManager } from './Config'; import { GlobalConfigManager } from './GlobalConfig'; import { EnvironmentSelector } from './EnvironmentSelector'; @@ -66,6 +67,18 @@ interface AddSkillOptions { environments?: string[]; } +interface RegistrySkillChoice { + name: string; + description?: string; +} + +interface ResolvedInstallContext { + baseDir: string; + capableEnvironments: string[]; + installMode: 'global' | 'project'; + targets: string[]; +} + export class SkillManager { constructor( private configManager: ConfigManager, @@ -78,11 +91,9 @@ export class SkillManager { * @param registryId - e.g., "anthropics/skills" * @param skillName - e.g., "frontend-design" */ - async addSkill(registryId: string, skillName: string, options: AddSkillOptions = {}): Promise { - const installMode = options.global ? 'global' : 'project'; - ui.info(`Validating skill: ${skillName} from ${registryId}`); + async addSkill(registryId: string, skillName?: string, options: AddSkillOptions = {}): Promise { + ui.info(`Validating registry: ${registryId}`); validateRegistryId(registryId); - validateSkillName(skillName); await ensureGitInstalled(); const spinner = ui.spinner('Fetching registries...'); @@ -99,57 +110,17 @@ export class SkillManager { } ui.info('Checking local cache...'); - const repoPath = await this.cloneRepositoryToCache(registryId, gitUrl); - - const skillPath = path.join(repoPath, 'skills', skillName); - if (!await fs.pathExists(skillPath)) { - throw new Error( - `Skill "${skillName}" not found in ${registryId}. Check the repository for available skills.` - ); - } - - const skillMdPath = path.join(skillPath, 'SKILL.md'); - if (!await fs.pathExists(skillMdPath)) { - throw new Error( - `Invalid skill: SKILL.md not found in ${skillName}. This may not be a valid Agent Skill.` - ); - } + const repoPath = await this.prepareRegistryRepository(registryId, gitUrl); + const resolvedSkillNames = skillName + ? [skillName] + : await this.resolveSkillNamesFromRegistry(registryId, repoPath); const selectedEnvironments = await this.resolveInstallEnvironments(options); - const { targets, capableEnvironments } = this.resolveInstallationTargets(selectedEnvironments, options.global); - - ui.info(`Installing skill to ${installMode}...`); - const baseDir = options.global ? os.homedir() : process.cwd(); - - for (const targetDir of targets) { - const targetPath = path.join(baseDir, targetDir, skillName); - - if (await fs.pathExists(targetPath)) { - ui.text(` → ${targetDir}/${skillName} (already exists, skipped)`); - continue; - } - - await fs.ensureDir(path.dirname(targetPath)); - - try { - await fs.symlink(skillPath, targetPath, 'dir'); - ui.text(` → ${targetDir}/${skillName} (symlinked)`); - } catch (error) { - await fs.copy(skillPath, targetPath); - ui.text(` → ${targetDir}/${skillName} (copied)`); - } - } + const installContext = this.buildInstallContext(selectedEnvironments, options); - if (!options.global) { - await this.configManager.addSkill({ - registry: registryId, - name: skillName - }); + for (const resolvedSkillName of resolvedSkillNames) { + await this.installResolvedSkill(registryId, repoPath, resolvedSkillName, options, installContext); } - - ui.text(`Successfully installed: ${skillName}`); - ui.info(` Source: ${registryId}`); - ui.info(` Installed to (${installMode}): ${capableEnvironments.join(', ')}`); } private async resolveProjectEnvironments(): Promise { @@ -464,6 +435,180 @@ export class SkillManager { return result; } + private async prepareRegistryRepository(registryId: string, gitUrl?: string): Promise { + const cachedPath = path.join(SKILL_CACHE_DIR, registryId); + + try { + return await this.cloneRepositoryToCache(registryId, gitUrl); + } catch (error: any) { + if (await fs.pathExists(cachedPath)) { + ui.warning(`Failed to refresh ${registryId}: ${error.message}. Using cached registry contents.`); + return cachedPath; + } + + throw error; + } + } + + private async installResolvedSkill( + registryId: string, + repoPath: string, + resolvedSkillName: string, + options: AddSkillOptions, + installContext: ResolvedInstallContext + ): Promise { + ui.info(`Validating skill: ${resolvedSkillName} from ${registryId}`); + validateSkillName(resolvedSkillName); + + const skillPath = await this.resolveInstallableSkillPath(repoPath, registryId, resolvedSkillName); + + ui.info(`Installing skill to ${installContext.installMode}...`); + for (const targetDir of installContext.targets) { + const targetPath = path.join(installContext.baseDir, targetDir, resolvedSkillName); + + if (await fs.pathExists(targetPath)) { + ui.text(` → ${targetDir}/${resolvedSkillName} (already exists, skipped)`); + continue; + } + + await fs.ensureDir(path.dirname(targetPath)); + + try { + await fs.symlink(skillPath, targetPath, 'dir'); + ui.text(` → ${targetDir}/${resolvedSkillName} (symlinked)`); + } catch (error) { + await fs.copy(skillPath, targetPath); + ui.text(` → ${targetDir}/${resolvedSkillName} (copied)`); + } + } + + if (!options.global) { + await this.configManager.addSkill({ + registry: registryId, + name: resolvedSkillName + }); + } + + ui.text(`Successfully installed: ${resolvedSkillName}`); + ui.info(` Source: ${registryId}`); + ui.info(` Installed to (${installContext.installMode}): ${installContext.capableEnvironments.join(', ')}`); + } + + private buildInstallContext( + selectedEnvironments: string[], + options: AddSkillOptions + ): ResolvedInstallContext { + const { targets, capableEnvironments } = this.resolveInstallationTargets(selectedEnvironments, options.global); + + return { + baseDir: options.global ? os.homedir() : process.cwd(), + capableEnvironments, + installMode: options.global ? 'global' : 'project', + targets, + }; + } + + private async resolveInstallableSkillPath( + repoPath: string, + registryId: string, + resolvedSkillName: string + ): Promise { + const skillPath = path.join(repoPath, 'skills', resolvedSkillName); + if (!await fs.pathExists(skillPath)) { + throw new Error( + `Skill "${resolvedSkillName}" not found in ${registryId}. Check the repository for available skills.` + ); + } + + const skillMdPath = path.join(skillPath, 'SKILL.md'); + if (!await fs.pathExists(skillMdPath)) { + throw new Error( + `Invalid skill: SKILL.md not found in ${resolvedSkillName}. This may not be a valid Agent Skill.` + ); + } + + return skillPath; + } + + private async resolveSkillNamesFromRegistry(registryId: string, repoPath: string): Promise { + if (!this.isInteractiveTerminal()) { + throw new Error('Skill name is required in non-interactive mode. Re-run with: ai-devkit skill add '); + } + + const skills = await this.listRegistrySkills(registryId, repoPath); + return this.promptForSkillSelection(skills); + } + + private async listRegistrySkills(registryId: string, repoPath: string): Promise { + const skillsDir = path.join(repoPath, 'skills'); + if (!await fs.pathExists(skillsDir)) { + throw new Error(`No valid skills found in ${registryId}.`); + } + + const entries = await fs.readdir(skillsDir, { withFileTypes: true }); + const skills: RegistrySkillChoice[] = []; + + for (const entry of entries) { + if (!entry.isDirectory()) { + continue; + } + + const skillMdPath = path.join(skillsDir, entry.name, 'SKILL.md'); + if (!await fs.pathExists(skillMdPath)) { + continue; + } + + let description: string | undefined; + try { + const content = await fs.readFile(skillMdPath, 'utf8'); + description = extractSkillDescription(content); + } catch { + description = undefined; + } + + skills.push({ + name: entry.name, + description, + }); + } + + if (skills.length === 0) { + throw new Error(`No valid skills found in ${registryId}.`); + } + + skills.sort((a, b) => a.name.localeCompare(b.name)); + return skills; + } + + private async promptForSkillSelection(skills: RegistrySkillChoice[]): Promise { + try { + const { selectedSkills } = await inquirer.prompt([ + { + type: 'checkbox', + name: 'selectedSkills', + message: 'Select skill(s) to install', + choices: skills.map(skill => ({ + name: skill.description ? `${skill.name} - ${skill.description}` : skill.name, + value: skill.name, + })), + validate: (value: string[]) => value.length > 0 || 'Select at least one skill.', + }, + ]); + + return selectedSkills; + } catch (error: any) { + if (error?.name === 'ExitPromptError' || error?.message?.toLowerCase().includes('cancel')) { + throw new Error('Skill selection cancelled.'); + } + + throw error; + } + } + + private isInteractiveTerminal(): boolean { + return Boolean(process.stdin.isTTY && process.stdout.isTTY); + } + /** * Display update summary with colored output * @param summary - UpdateSummary to display