diff --git a/docs/ai/design/2026-09-05-feature-builtin-remote.md b/docs/ai/design/2026-09-05-feature-builtin-remote.md new file mode 100644 index 00000000..b0894070 --- /dev/null +++ b/docs/ai/design/2026-09-05-feature-builtin-remote.md @@ -0,0 +1,71 @@ +--- +phase: design +title: Remote Built-in Skills Manifest Design +description: Load and validate the live curated skill list with a safe process-local fallback +--- + +# Remote Built-in Skills Manifest Design + +## Architecture Overview + +```mermaid +flowchart LR + F[Built-in flow] --> L[getBuiltinSkillNames] + L -->|first call| R[raw GitHub main manifest] + L --> V[validate bare array] + V -->|valid| N[readonly string list] + R -->|network/HTTP/JSON failure| B[embedded 21-name fallback] + V -->|invalid| B + N --> C[init / skill / setup / status] + B --> C + L -->|later calls| P[shared process promise] +``` + +`packages/cli/src/lib/BuiltinSkills.ts` is the only fetch, parse, validation, fallback, and promise-cache boundary. Consumers remain responsible only for using the resolved names. + +## Data Model + +`skills/built-in.json` is a bare JSON array: + +```json +["agent-communication", "agent-management"] +``` + +The internal API is: + +```ts +getBuiltinSkillNames(): Promise +``` + +Runtime names are strings. The unused compile-time `BuiltinSkillName` union is deleted because remote data cannot truthfully define a literal union. + +## Component Breakdown + +- `BuiltinSkills.ts` owns the raw `main` URL, trusted registry identifier, embedded fallback, validation, warning, and process-local promise. +- `init.ts` resolves built-ins only when the existing flow elects to install them, then maps names to template entries. +- `skill.ts` resolves names before the `--built-in` loop. +- `setup.service.ts` resolves names in the default installer; the cache prevents repeated requests across agents. +- `status.service.ts` resolves names before readiness checks; counts describe the selected live or fallback set. +- `SkillManager` remains unchanged and resolves `skills//SKILL.md` from the refreshed repository. + +## Failure Contract + +The loader treats a non-OK response, invalid JSON, or invalid manifest as one failure class: emit a warning containing the reason and return the embedded fallback. Consumers do not branch on source or fail setup. + +Validation requires a non-empty array whose elements are non-empty, unique strings accepted by the existing skill-name rules. Validation is all-or-nothing. + +## Design Decisions + +- Fetch from `main` so maintainers can add built-ins without a CLI release. +- Use a bare array because there is no current caller for metadata or schema fields. +- Use a process-local promise cache, not persistent storage, because one stable result per invocation is sufficient. +- Keep the fallback beside the loader so there is one compiled list and no consumer duplication. +- Keep the registry identifier compiled because the remote manifest controls membership, not installation origin. +- Do not change `SkillManager`; its existing runtime path lookup already supports skills unknown to the compiled CLI. + +## Non-Functional Requirements + +- No built-in flow may contact the manifest more than once per process. +- Setup and status remain usable when GitHub is unavailable. +- Untrusted remote data is validated before it controls installation paths. +- Tests replace global `fetch` and never rely on network state. diff --git a/docs/ai/implementation/2026-09-05-feature-builtin-remote.md b/docs/ai/implementation/2026-09-05-feature-builtin-remote.md new file mode 100644 index 00000000..82a53668 --- /dev/null +++ b/docs/ai/implementation/2026-09-05-feature-builtin-remote.md @@ -0,0 +1,64 @@ +--- +phase: implementation +title: Remote Built-in Skills Manifest Implementation +description: Track implementation decisions, files, and design alignment +--- + +# Remote Built-in Skills Manifest Implementation + +## Development Setup + +- Worktree: `.worktrees/feature-builtin-remote` +- Branch: `feature-builtin-remote`, created from latest `origin/main` +- Bootstrap: `npm ci` and the six-project `npm run build` +- Method: TDD for each loader behavior and consumer migration + +## Planned Code Structure + +- `skills/built-in.json`: live bare-array manifest. +- `packages/cli/src/lib/BuiltinSkills.ts`: remote boundary, validation, promise cache, fallback, registry identity. +- Four existing consumers: asynchronous runtime list resolution. +- Focused loader and consumer tests: mocked fetch and fixture lists. + +## Implementation Notes + +- Added the live bare-array manifest with the 21 names present on the latest base. +- Added a single loader that shares its promise, validates remote data, and warns before returning the embedded fallback. +- Migrated init, skill add, setup, and status to await runtime names. +- Deleted the obsolete constants module and literal union. +- Left `SkillManager` unchanged because it already resolves validated runtime names from `skills//SKILL.md`. + +## Error Handling + +All remote failures warn once and return the embedded fallback. Invalid manifests are rejected wholesale. + +## Security Notes + +The remote list controls installation membership. The loader validates the full array at the network boundary before exposing names internally. + +## Progress + +- [x] Loader and manifest +- [x] Consumer migration +- [x] Implementation alignment check +- [x] Full validation + +## Verification Evidence + +- Focused suite: 7 files, 97 tests passed. +- CLI build: TypeScript declarations and 221 source files compiled successfully. +- Full build: all 6 projects passed. +- Full unit suite: all 6 projects passed serially, 2177 tests total. +- Full lint: all 6 projects passed. +- E2E: 41 tests passed. +- Optional task tracing was unavailable: `npx ai-devkit@latest task list --name builtin-remote --json` returned `unknown command 'task'`. + +## Design Alignment + +The implementation matches the approved single-loader data flow, all-or-nothing validation, process-local promise caching, embedded fallback, runtime string typing, and four consumer boundaries. No design deviations or follow-up work were identified. + +## Publication + +- Branch: `feature-builtin-remote` +- Pull request: https://github.com/codeaholicguy/ai-devkit/pull/214 +- Merge intentionally left to reviewers. diff --git a/docs/ai/planning/2026-09-05-feature-builtin-remote.md b/docs/ai/planning/2026-09-05-feature-builtin-remote.md new file mode 100644 index 00000000..7606d7ec --- /dev/null +++ b/docs/ai/planning/2026-09-05-feature-builtin-remote.md @@ -0,0 +1,45 @@ +--- +phase: planning +title: Remote Built-in Skills Manifest Plan +description: Ordered TDD tasks for the manifest loader and four consumers +--- + +# Remote Built-in Skills Manifest Plan + +## Milestones + +- [x] Milestone 1: Manifest loader is specified and implemented with fallback semantics. +- [x] Milestone 2: All four built-in consumers use runtime names. +- [x] Milestone 3: Documentation and full verification gates are complete. + +## Task Breakdown + +### Phase 1: Manifest and loader + +- [x] Task 1.1: Add loader tests for a valid bare array and one-fetch promise caching. Evidence: focused loader test fails before implementation and passes afterward. +- [x] Task 1.2: Add loader tests for HTTP, parse, and invalid-shape fallback behavior, including empty, blank, duplicate, and invalid names. Dependency: Task 1.1. Evidence: focused tests. +- [x] Task 1.3: Add `skills/built-in.json` with the current 21 names from the latest base and implement the minimal loader. Dependency: failing tests. Evidence: focused tests and manifest parsing. + +### Phase 2: Consumer migration + +- [x] Task 2.1: Migrate `skill add --built-in` and setup using mocked runtime lists. Dependency: loader API. Evidence: focused command/service tests. +- [x] Task 2.2: Migrate init so the list is resolved only for a triggered built-in flow. Dependency: loader API. Evidence: focused init tests, including no-fetch skip behavior. +- [x] Task 2.3: Migrate status to report fixture-driven live counts and fallback warnings. Dependency: loader API. Evidence: focused status tests. +- [x] Task 2.4: Delete the compiled primary list and unused literal union; confirm no remaining references. Dependency: all consumers migrated. Evidence: `rg` and build. + +### Phase 3: Verification and publication + +- [x] Task 3.1: Reconcile implementation/testing docs and perform design-alignment review. Evidence: feature lint and diff review. +- [x] Task 3.2: Run `npm run build`, `npm test`, `npm run lint`, and E2E; fix regressions. Evidence: fresh exit-zero output. +- [x] Task 3.3: Create logical commits, rebase onto latest `origin/main`, rerun gates, push, and open the PR. Evidence: clean branch and PR URL. + +## Dependencies and Risks + +- Remote `main` may list a skill before its directory is available. Maintainers should land the directory and manifest atomically; `SkillManager` gives a clear not-found error. +- A rejected manifest uses the embedded fallback as one complete set; partial remote data is never installed. +- Existing tests with literal `20/20` output must use injected fixture counts where they test rendering rather than policy. +- All test suites must mock manifest fetches to preserve determinism. + +## Progress Summary + +The loader, manifest, and four consumers are complete and aligned with the approved design. The latest base added `ai-devkit-setup` after the original brief, so the manifest and safe fallback preserve the current 21-name set rather than regressing offline setup. Focused tests, the six-project build/lint/unit gates, E2E, and feature-doc lint pass. The branch is published for review in PR #214. diff --git a/docs/ai/requirements/2026-09-05-feature-builtin-remote.md b/docs/ai/requirements/2026-09-05-feature-builtin-remote.md new file mode 100644 index 00000000..7c4c765e --- /dev/null +++ b/docs/ai/requirements/2026-09-05-feature-builtin-remote.md @@ -0,0 +1,62 @@ +--- +phase: requirements +title: Remote Built-in Skills Manifest Requirements +description: Define live built-in skill discovery without requiring a CLI release +--- + +# Remote Built-in Skills Manifest Requirements + +## Problem Statement + +AI DevKit compiles its curated built-in skill names into the CLI. Adding a new built-in therefore requires a CLI code change and release even though the skill itself is already installable from the AI DevKit repository. + +Maintainers need a hand-managed list on `main` so a newly added skill can become a built-in immediately for existing CLI versions. + +## Goals & Objectives + +- Make `skills/built-in.json` on `main` the live source of truth for built-in skill names. +- Fetch the manifest whenever an init, setup, built-in install, or status flow needs the list. +- Fetch at most once per CLI process. +- Preserve working setup and built-in flows during network or manifest failures by using the current 21-name list from the latest base. +- Keep the manifest and implementation deliberately small. + +### Non-goals + +- Versioning or pinning the manifest or registry repository. +- Code generation or compile-time literal types derived from the manifest. +- Descriptions, compatibility metadata, or registry selection in the manifest. +- Persisting a downloaded manifest cache across CLI invocations. +- Uninstalling skills removed from the live list. + +## User Stories & Use Cases + +- As a maintainer, I can add `skills//SKILL.md` and append `` to the JSON array on `main`, making it available to old CLIs without a release. +- As a CLI user, I receive the live curated set through `init`, `setup`, or `skill add --built-in`. +- As an offline user, setup continues with the known bundled fallback set. +- As a user running `status`, I see presence counts against the live set, or against the fallback set when the live manifest is unavailable. + +## Success Criteria + +- [x] `skills/built-in.json` is a bare JSON array seeded with the existing 21 names from the latest base. +- [x] A single loader fetches the raw `main` manifest and returns `Promise`. +- [x] Repeated loader calls in one process share one fetch promise. +- [x] The loader accepts only a non-empty array of non-empty, unique, valid skill-name strings. +- [x] Network, HTTP, JSON, or validation failure emits a clear warning and returns the embedded current list. +- [x] `init`, `skill add --built-in`, setup, and status obtain names through the loader. +- [x] A runtime name is passed directly to `SkillManager.addSkill`, which resolves `skills//SKILL.md` from the refreshed registry without another compiled allowlist. +- [x] Status reports `required` and `present` against the live list; on fetch failure it reports against the fallback and warns. +- [x] `BUILTIN_SKILL_NAMES` and the unused `BuiltinSkillName` literal union are removed. +- [x] Tests never use the real network and cover loader success, promise caching, invalid manifests, fallback, and consumer integration. +- [x] Build, unit tests, lint, and the E2E suite pass. + +## Constraints & Assumptions + +- The URL points directly to `raw.githubusercontent.com/codeaholicguy/ai-devkit/main/skills/built-in.json`. +- The repository `main` branch is intentionally the rollout boundary; old CLIs may install newer skills. +- `BUILTIN_SKILL_REGISTRY` remains trusted compiled configuration. +- The current registry refresh behavior makes newly committed skill directories visible to existing CLIs. +- The fallback may become stale; it exists only to preserve safe offline behavior. + +## Questions & Open Items + +None. Product and failure semantics were explicitly approved. diff --git a/docs/ai/testing/2026-09-05-feature-builtin-remote.md b/docs/ai/testing/2026-09-05-feature-builtin-remote.md new file mode 100644 index 00000000..27a76557 --- /dev/null +++ b/docs/ai/testing/2026-09-05-feature-builtin-remote.md @@ -0,0 +1,56 @@ +--- +phase: testing +title: Remote Built-in Skills Manifest Testing +description: Deterministic coverage for remote loading, fallback, and built-in consumers +--- + +# Remote Built-in Skills Manifest Testing + +## Test Coverage Goals + +- Cover every new loader branch and all four changed consumer boundaries. +- Mock global fetch in loader tests and mock the loader in consumer tests. +- Preserve deterministic unit and E2E execution with no live manifest requests. + +## Unit Tests + +### Built-in loader + +- [x] Returns a valid bare-array manifest. +- [x] Reuses one in-flight/resolved promise across calls. +- [x] Falls back and warns for network failure. +- [x] Falls back and warns for non-OK HTTP responses. +- [x] Falls back and warns for invalid JSON. +- [x] Rejects non-array, empty-array, blank-name, duplicate-name, and invalid-name manifests as complete responses. + +### Consumers + +- [x] `skill add --built-in` installs every name from a mocked runtime list. +- [x] Setup installs every name from a mocked runtime list for the selected agent. +- [x] Init adds mocked runtime names when built-ins are selected and does not fetch when skipped. +- [x] Status passes a mocked runtime list to readiness checks and renders fixture-derived counts. +- [x] Status uses loader fallback behavior without failing the command. + +## Integration and End-to-End Tests + +- [x] Existing CLI command and service suites pass with deterministic mocks. +- [x] Search E2E tests for compiled skill-count assumptions and update any affected assertions. +- [x] Full E2E suite passes without accessing the live manifest. + +## Verification Gates + +- [x] `npm run build` +- [x] `npm test` (equivalent Nx target run serially after a shared temporary-filesystem quota failure) +- [x] `npm run lint` +- [x] `npx vitest run --config e2e/vitest.config.ts` +- [x] `npx ai-devkit@latest lint --feature builtin-remote` + +## Test Data + +- Small fixture lists such as `['remote-one', 'remote-two']` for consumer behavior. +- The current 21 names from the latest base only in the manifest and loader fallback. +- Mocked successful and failing `Response` objects for loader boundaries. + +## Results + +Focused verification passed 97 tests in seven files. The full six-project build and lint gates passed. The full unit suite passed 2177 tests across six projects when run serially to avoid the shared temporary-filesystem quota. E2E passed 41 tests. No hardcoded E2E skill-count assertions existed. diff --git a/packages/cli/src/__tests__/commands/init.test.ts b/packages/cli/src/__tests__/commands/init.test.ts index 317ff5dd..f378a37c 100644 --- a/packages/cli/src/__tests__/commands/init.test.ts +++ b/packages/cli/src/__tests__/commands/init.test.ts @@ -13,6 +13,7 @@ const { mockIsInteractiveTerminal, mockReconcileAndInstall, mockGetInstallExitCode, + mockGetBuiltinSkillNames, } = vi.hoisted(() => ({ mockConfigManager: { exists: vi.fn(), @@ -52,8 +53,11 @@ const { mockIsInteractiveTerminal: vi.fn() as any, mockReconcileAndInstall: vi.fn() as any, mockGetInstallExitCode: vi.fn() as any, + mockGetBuiltinSkillNames: vi.fn() as any, })); +const BUILTIN_SKILL_FIXTURE = ['remote-one', 'remote-two']; + vi.mock('../../services/install/install.service.js', () => ({ reconcileAndInstall: (...args: unknown[]) => mockReconcileAndInstall(...args), getInstallExitCode: (...args: unknown[]) => mockGetInstallExitCode(...args) @@ -87,6 +91,11 @@ vi.mock('../../lib/SkillManager.js', () => ({ SkillManager: vi.fn(function () { return mockSkillManager; }) })); +vi.mock('../../lib/BuiltinSkills.js', () => ({ + BUILTIN_SKILL_REGISTRY: 'codeaholicguy/ai-devkit', + getBuiltinSkillNames: (...args: unknown[]) => mockGetBuiltinSkillNames(...args), +})); + vi.mock('../../lib/InitTemplate.js', () => ({ loadInitTemplate: (...args: unknown[]) => mockLoadInitTemplate(...args) })); @@ -100,8 +109,7 @@ vi.mock('../../util/terminal.js', () => ({ })); import { initCommand } from '../../commands/init.js'; -import { BUILTIN_SKILL_NAMES, BUILTIN_SKILL_REGISTRY } from '../../constants.js'; -import { SkillManager } from '../../lib/SkillManager.js'; +import { BUILTIN_SKILL_REGISTRY } from '../../lib/BuiltinSkills.js'; function confirmCallsMatching(pattern: RegExp): any[] { return mockConfirm.mock.calls.filter(([config]: any[]) => @@ -149,6 +157,7 @@ describe('init command', () => { warnings: [], items: [], complete: true }); mockGetInstallExitCode.mockReturnValue(0); + mockGetBuiltinSkillNames.mockResolvedValue(BUILTIN_SKILL_FIXTURE); }); afterEach(() => { @@ -287,9 +296,9 @@ describe('init command', () => { await initCommand({ template: './init.yaml', builtIn: true }); - expect(appliedConfig().skills).toHaveLength(BUILTIN_SKILL_NAMES.length + 1); + expect(appliedConfig().skills).toHaveLength(BUILTIN_SKILL_FIXTURE.length + 1); expect(appliedConfig().skills).toContainEqual({ registry: BUILTIN_SKILL_REGISTRY, name: 'debug' }); - for (const skill of BUILTIN_SKILL_NAMES) { + for (const skill of BUILTIN_SKILL_FIXTURE) { expect(appliedConfig().skills).toContainEqual({ registry: BUILTIN_SKILL_REGISTRY, name: skill }); } const builtinPrompts = confirmCallsMatching(/Install AI DevKit built-in skills/); @@ -304,8 +313,8 @@ describe('init command', () => { await initCommand({ template: './init.yaml', builtIn: true }); - expect(appliedConfig().skills).toHaveLength(BUILTIN_SKILL_NAMES.length); - for (const skill of BUILTIN_SKILL_NAMES) { + expect(appliedConfig().skills).toHaveLength(BUILTIN_SKILL_FIXTURE.length); + for (const skill of BUILTIN_SKILL_FIXTURE) { expect(appliedConfig().skills).toContainEqual({ registry: BUILTIN_SKILL_REGISTRY, name: skill }); } const builtinPrompts = confirmCallsMatching(/Install AI DevKit built-in skills/); @@ -336,6 +345,7 @@ describe('init command', () => { const builtinPromptCalls = confirmCallsMatching(/Install AI DevKit built-in skills/); expect(builtinPromptCalls.length).toBe(1); expect(mockSkillManager.addSkill).not.toHaveBeenCalled(); + expect(mockGetBuiltinSkillNames).not.toHaveBeenCalled(); }); it('does not prompt for built-in skills when running in template mode', async () => { @@ -376,6 +386,7 @@ describe('init command', () => { const builtinPrompts = confirmCallsMatching(/Install AI DevKit built-in skills/); expect(builtinPrompts).toHaveLength(0); expect(mockSkillManager.addSkill).not.toHaveBeenCalled(); + expect(mockGetBuiltinSkillNames).not.toHaveBeenCalled(); expect(mockUi.info).toHaveBeenCalledWith( expect.stringMatching(/non-interactive|--built-in/) ); diff --git a/packages/cli/src/__tests__/commands/skill.test.ts b/packages/cli/src/__tests__/commands/skill.test.ts index b08e591c..33f2d4fb 100644 --- a/packages/cli/src/__tests__/commands/skill.test.ts +++ b/packages/cli/src/__tests__/commands/skill.test.ts @@ -5,6 +5,7 @@ import { ui } from '../../util/terminal-ui.js'; import { SkillManager } from '../../lib/SkillManager.js'; const mockRemoveCache = vi.hoisted(() => vi.fn()); +const mockGetBuiltinSkillNames = vi.hoisted(() => vi.fn()); const mockAddSkill = vi.fn(); @@ -51,6 +52,11 @@ vi.mock('../../lib/SkillManager.js', () => ({ }; }), })); +vi.mock('../../lib/BuiltinSkills.js', () => ({ + BUILTIN_SKILL_REGISTRY: 'codeaholicguy/ai-devkit', + getBuiltinSkillNames: (...args: unknown[]) => mockGetBuiltinSkillNames(...args), +})); + vi.mock('../../util/terminal-ui.js', () => ({ ui: { error: vi.fn(), @@ -78,6 +84,7 @@ describe('skill command', () => { mockGlobalAddSkillRegistry.mockResolvedValue({}); mockProjectRemoveSkillRegistry.mockResolvedValue({}); mockGlobalRemoveSkillRegistry.mockResolvedValue({}); + mockGetBuiltinSkillNames.mockResolvedValue(['remote-one', 'remote-two']); vi.spyOn(process, 'exit').mockImplementation((() => undefined) as any); vi.spyOn(process.stderr, 'write').mockImplementation((() => true) as any); }); @@ -311,34 +318,16 @@ describe('skill command', () => { await program.parseAsync(['node', 'test', 'skill', 'add', '--built-in']); - expect(mockAddSkill).toHaveBeenCalledWith('codeaholicguy/ai-devkit', 'agent-communication', { - global: undefined, - environments: undefined, - }); - expect(mockAddSkill).toHaveBeenCalledWith('codeaholicguy/ai-devkit', 'agent-management', { - global: undefined, - environments: undefined, - }); - expect(mockAddSkill).toHaveBeenCalledWith('codeaholicguy/ai-devkit', 'dev-commit', { - global: undefined, - environments: undefined, - }); - expect(mockAddSkill).toHaveBeenCalledWith('codeaholicguy/ai-devkit', 'dev-worktree', { - global: undefined, - environments: undefined, - }); - expect(mockAddSkill).toHaveBeenCalledWith('codeaholicguy/ai-devkit', 'dev-requirements', { - global: undefined, - environments: undefined, - }); - expect(mockAddSkill).toHaveBeenCalledWith('codeaholicguy/ai-devkit', 'dev-review', { + expect(mockAddSkill).toHaveBeenCalledTimes(2); + expect(mockAddSkill).toHaveBeenCalledWith('codeaholicguy/ai-devkit', 'remote-one', { global: undefined, environments: undefined, }); - expect(mockAddSkill).toHaveBeenCalledWith('codeaholicguy/ai-devkit', 'dev-pr', { + expect(mockAddSkill).toHaveBeenCalledWith('codeaholicguy/ai-devkit', 'remote-two', { global: undefined, environments: undefined, }); + expect(mockGetBuiltinSkillNames).toHaveBeenCalledOnce(); expect(SkillManager).toHaveBeenCalledTimes(1); }); diff --git a/packages/cli/src/__tests__/commands/status.test.ts b/packages/cli/src/__tests__/commands/status.test.ts index 00805afc..04ebea8c 100644 --- a/packages/cli/src/__tests__/commands/status.test.ts +++ b/packages/cli/src/__tests__/commands/status.test.ts @@ -28,7 +28,7 @@ function agent(status: 'pass' | 'warn' | 'fail', options: { globalConfig: { ...base, path: '~/.agent', present: true, readable: true }, builtInSkills: options.builtInSkills ? { status: options.builtInSkills.status, errors: ['required built-in skills are missing'], path: '~/.agent/skills', ...options.builtInSkills } - : { status: 'info' as const, errors: [], path: '~/.agent/skills', required: 20, present: 20, missing: [] }, + : { status: 'info' as const, errors: [], path: '~/.agent/skills', required: 2, present: 2, missing: [] }, ...(options.auth ? { auth: { ...base, state: 'authenticated', source: 'test', provider: null, availableProviders: [] } } : {}), ...(options.integration ? { integration: { ...base, ...options.integration } } : {}), }; @@ -51,7 +51,7 @@ const report = { opencode: { ...agent('pass', { auth: true, - builtInSkills: { status: 'info', present: 19, required: 20, missing: ['verify'] }, + builtInSkills: { status: 'info', present: 1, required: 2, missing: ['remote-two'] }, }), type: 'opencode', auth: { ...base, state: 'authenticated', source: 'opencode auth list', provider: null, availableProviders: ['OpenAI', 'litellm'] }, @@ -91,18 +91,18 @@ describe('status command', () => { expect(ui.table).toHaveBeenCalledWith(expect.objectContaining({ headers: ['Check', 'Status', 'Evidence'], rows: expect.arrayContaining([ - ['codex: ai-devkit built-in skills', 'info', '20/20'], + ['codex: ai-devkit built-in skills', 'info', '2/2'], ['codex: ai-devkit hook', 'ready', 'installed'], ['pi: ai-devkit plugin', 'ready', 'installed'], ['pi: providers', 'info', 'anthropic'], - ['opencode: ai-devkit built-in skills', 'info', '19/20'], + ['opencode: ai-devkit built-in skills', 'info', '1/2'], ['opencode: auth', 'ready', 'authenticated'], ['opencode: providers', 'info', 'OpenAI, litellm'], ]), })); const checkRows = (vi.mocked(ui.table).mock.calls[2][0].rows ?? []) as Array<[string, string, string]>; expect(checkRows.some(([label]) => label.startsWith('grok_cli:'))).toBe(false); - expect(checkRows).not.toContainEqual(['codex: ai-devkit built-in skills', 'pass', '20/20']); + expect(checkRows).not.toContainEqual(['codex: ai-devkit built-in skills', 'pass', '2/2']); expect(ui.warning).not.toHaveBeenCalledWith(expect.stringContaining('missing built-in skills')); }); diff --git a/packages/cli/src/__tests__/lib/BuiltinSkills.test.ts b/packages/cli/src/__tests__/lib/BuiltinSkills.test.ts new file mode 100644 index 00000000..edd40a1d --- /dev/null +++ b/packages/cli/src/__tests__/lib/BuiltinSkills.test.ts @@ -0,0 +1,120 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { readFile } from 'node:fs/promises'; + +const mockWarning = vi.fn(); + +vi.mock('../../util/terminal-ui.js', () => ({ + ui: { + warning: (...args: unknown[]) => mockWarning(...args), + }, +})); + +describe('getBuiltinSkillNames', () => { + beforeEach(() => { + vi.resetModules(); + vi.unstubAllGlobals(); + mockWarning.mockReset(); + }); + + it('returns the live bare-array manifest and fetches it once per process', async () => { + const fetchMock = vi.fn().mockResolvedValue({ + ok: true, + json: async () => ['remote-one', 'remote-two'], + }); + vi.stubGlobal('fetch', fetchMock); + + const { getBuiltinSkillNames } = await import('../../lib/BuiltinSkills.js'); + + await expect(getBuiltinSkillNames()).resolves.toEqual(['remote-one', 'remote-two']); + await expect(getBuiltinSkillNames()).resolves.toEqual(['remote-one', 'remote-two']); + expect(fetchMock).toHaveBeenCalledOnce(); + expect(fetchMock).toHaveBeenCalledWith( + 'https://raw.githubusercontent.com/codeaholicguy/ai-devkit/main/skills/built-in.json' + ); + expect(mockWarning).not.toHaveBeenCalled(); + }); + + it('falls back to the bundled list when the manifest cannot be fetched', async () => { + vi.stubGlobal('fetch', vi.fn().mockRejectedValue(new Error('network unavailable'))); + + const { getBuiltinSkillNames } = await import('../../lib/BuiltinSkills.js'); + + const names = await getBuiltinSkillNames(); + expect(names).toHaveLength(21); + expect(names).toContain('agent-communication'); + expect(names).toContain('tdd'); + expect(mockWarning).toHaveBeenCalledWith( + 'Failed to load built-in skills manifest: network unavailable. Using bundled fallback.' + ); + }); + + it('falls back to the bundled list for an unsuccessful response', async () => { + vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ + ok: false, + status: 404, + })); + + const { getBuiltinSkillNames } = await import('../../lib/BuiltinSkills.js'); + + await expect(getBuiltinSkillNames()).resolves.toHaveLength(21); + expect(mockWarning).toHaveBeenCalledWith( + 'Failed to load built-in skills manifest: HTTP 404. Using bundled fallback.' + ); + }); + + it('falls back to the bundled list when response JSON cannot be parsed', async () => { + vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ + ok: true, + json: async () => { + throw new SyntaxError('Unexpected token'); + }, + })); + + const { getBuiltinSkillNames } = await import('../../lib/BuiltinSkills.js'); + + await expect(getBuiltinSkillNames()).resolves.toHaveLength(21); + expect(mockWarning).toHaveBeenCalledWith( + 'Failed to load built-in skills manifest: Unexpected token. Using bundled fallback.' + ); + }); + + it.each([ + { label: 'an object', manifest: { skills: ['valid-name'] } }, + { label: 'an empty array', manifest: [] }, + { label: 'a non-string item', manifest: ['valid-name', 1] }, + { label: 'a blank name', manifest: ['valid-name', ' '] }, + { label: 'a duplicate name', manifest: ['valid-name', 'valid-name'] }, + { label: 'an invalid skill name', manifest: ['../escape'] }, + ])('rejects $label as a complete manifest', async ({ manifest }) => { + vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ + ok: true, + json: async () => manifest, + })); + + const { getBuiltinSkillNames } = await import('../../lib/BuiltinSkills.js'); + + await expect(getBuiltinSkillNames()).resolves.toHaveLength(21); + expect(mockWarning).toHaveBeenCalledWith( + expect.stringMatching(/^Failed to load built-in skills manifest: .+ Using bundled fallback\.$/) + ); + }); +}); + +describe('built-in skills manifest', () => { + it('starts with the current built-in skill names', async () => { + const manifestPath = new URL('../../../../../skills/built-in.json', import.meta.url); + const manifest = JSON.parse(await readFile(manifestPath, 'utf8')); + + expect(manifest).toHaveLength(21); + expect(manifest).toEqual(expect.arrayContaining([ + 'agent-communication', + 'agent-management', + 'ai-devkit-setup', + 'dev-lifecycle', + 'structured-debug', + 'memory', + 'verify', + 'tdd', + ])); + }); +}); diff --git a/packages/cli/src/__tests__/services/setup/setup-builtins.test.ts b/packages/cli/src/__tests__/services/setup/setup-builtins.test.ts new file mode 100644 index 00000000..58ab66f6 --- /dev/null +++ b/packages/cli/src/__tests__/services/setup/setup-builtins.test.ts @@ -0,0 +1,68 @@ +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +const { mockAddSkill, mockGetBuiltinSkillNames } = vi.hoisted(() => ({ + mockAddSkill: vi.fn(), + mockGetBuiltinSkillNames: vi.fn(), +})); + +vi.mock('../../../lib/SkillManager.js', () => ({ + SkillManager: vi.fn(function () { + return { addSkill: (...args: unknown[]) => mockAddSkill(...args) }; + }), +})); + +vi.mock('../../../lib/BuiltinSkills.js', () => ({ + BUILTIN_SKILL_REGISTRY: 'codeaholicguy/ai-devkit', + getBuiltinSkillNames: (...args: unknown[]) => mockGetBuiltinSkillNames(...args), +})); + +import { createSetupService } from '../../../services/setup/setup.service.js'; + +describe('setup built-in skills', () => { + let homeDir: string; + let assetRoot: string; + + beforeEach(() => { + homeDir = mkdtempSync(join(tmpdir(), 'ai-devkit-setup-builtins-home-')); + assetRoot = mkdtempSync(join(tmpdir(), 'ai-devkit-setup-builtins-assets-')); + mkdirSync(join(homeDir, '.claude'), { recursive: true }); + mkdirSync(join(assetRoot, 'claude'), { recursive: true }); + writeFileSync(join(assetRoot, 'claude', 'claude-prompt-hook.js'), '// hook'); + writeFileSync(join(assetRoot, 'claude', 'settings-hook.json'), JSON.stringify({ + hooks: [{ type: 'command', command: 'echo hook' }], + })); + mockAddSkill.mockReset(); + mockAddSkill.mockResolvedValue('installed'); + mockGetBuiltinSkillNames.mockReset(); + mockGetBuiltinSkillNames.mockResolvedValue(['remote-one', 'remote-two']); + }); + + afterEach(() => { + rmSync(homeDir, { recursive: true, force: true }); + rmSync(assetRoot, { recursive: true, force: true }); + }); + + it('installs every runtime built-in for the selected agent', async () => { + const service = createSetupService({ homeDir, assetRoot }); + + await service.run({ agents: ['claude'] }); + + expect(mockGetBuiltinSkillNames).toHaveBeenCalledOnce(); + expect(mockAddSkill).toHaveBeenCalledTimes(2); + expect(mockAddSkill).toHaveBeenNthCalledWith( + 1, + 'codeaholicguy/ai-devkit', + 'remote-one', + { global: true, environments: ['claude'] } + ); + expect(mockAddSkill).toHaveBeenNthCalledWith( + 2, + 'codeaholicguy/ai-devkit', + 'remote-two', + { global: true, environments: ['claude'] } + ); + }); +}); diff --git a/packages/cli/src/__tests__/services/status/status.service.test.ts b/packages/cli/src/__tests__/services/status/status.service.test.ts index fcfe4fd4..e1314db5 100644 --- a/packages/cli/src/__tests__/services/status/status.service.test.ts +++ b/packages/cli/src/__tests__/services/status/status.service.test.ts @@ -1,5 +1,14 @@ import path from 'node:path'; import { describe, expect, it, vi } from 'vitest'; + +const mockGetBuiltinSkillNames = vi.hoisted(() => + vi.fn(async () => ['remote-one', 'remote-two']) +); + +vi.mock('../../../lib/BuiltinSkills.js', () => ({ + getBuiltinSkillNames: (...args: unknown[]) => mockGetBuiltinSkillNames(...args), +})); + import { getStatusReport, type StatusServiceOptions } from '../../../services/status/status.service.js'; type Files = Record; @@ -8,12 +17,7 @@ function fixture(overrides: Partial = {}) { const homeDir = '/home/test'; const cwd = '/repo'; const assetRoot = '/assets'; - const builtIns = [ - 'agent-communication', 'agent-management', 'dev-commit', 'dev-lifecycle', 'dev-worktree', - 'dev-requirements', 'dev-design', 'dev-planning', 'dev-implementation', 'dev-testing', - 'dev-review', 'dev-pr', 'structured-debug', 'document-code', 'memory', 'task', - 'simplify-implementation', 'brainstorm', 'verify', 'tdd', - ]; + const builtIns = ['remote-one', 'remote-two']; const files: Files = { [path.join(cwd, '.ai-devkit.json')]: JSON.stringify({ version: '0.55.0', environments: ['codex', 'pi', 'claude'], phases: [], createdAt: 'now', @@ -152,9 +156,15 @@ describe('getStatusReport', () => { } }; - await getStatusReport({ ...options, access }); + const report = await getStatusReport({ ...options, access }); expect(maxActiveSkillChecks).toBeGreaterThan(3); + expect(report.agents.codex.builtInSkills).toMatchObject({ + required: 2, + present: 2, + missing: [], + }); + expect(mockGetBuiltinSkillNames).toHaveBeenCalled(); }); it('uses the shared tmux inspection without a PATH preflight', async () => { diff --git a/packages/cli/src/commands/init.ts b/packages/cli/src/commands/init.ts index 95fd7437..6ad49049 100644 --- a/packages/cli/src/commands/init.ts +++ b/packages/cli/src/commands/init.ts @@ -1,5 +1,5 @@ import { execFileSync } from 'child_process'; -import { BUILTIN_SKILL_NAMES, BUILTIN_SKILL_REGISTRY } from '../constants.js'; +import { BUILTIN_SKILL_REGISTRY, getBuiltinSkillNames } from '../lib/BuiltinSkills.js'; import { ConfigManager } from '../lib/Config.js'; import { TemplateManager } from '../lib/TemplateManager.js'; import { EnvironmentSelector } from '../lib/EnvironmentSelector.js'; @@ -72,11 +72,6 @@ function normalizeEnvironmentOption( .filter((value): value is EnvironmentCode => value.length > 0); } -const BUILTIN_SKILLS: InitTemplateSkill[] = BUILTIN_SKILL_NAMES.map((skill: string) => ({ - registry: BUILTIN_SKILL_REGISTRY, - skill -})); - async function shouldInstallBuiltinSkills(options: InitOptions): Promise { if (options.builtIn) { return true; @@ -246,7 +241,11 @@ export async function initCommand(options: InitOptions) { if (options.builtIn || !hasTemplate) { const shouldInstall = await shouldInstallBuiltinSkills(options); if (shouldInstall) { - desiredSkillEntries.push(...BUILTIN_SKILLS); + const builtInSkills: InitTemplateSkill[] = (await getBuiltinSkillNames()).map(skill => ({ + registry: BUILTIN_SKILL_REGISTRY, + skill, + })); + desiredSkillEntries.push(...builtInSkills); } } const desiredSkills = normalizeSkills(desiredSkillEntries); diff --git a/packages/cli/src/commands/skill.ts b/packages/cli/src/commands/skill.ts index c5180dba..03281c1f 100644 --- a/packages/cli/src/commands/skill.ts +++ b/packages/cli/src/commands/skill.ts @@ -3,7 +3,7 @@ import chalk from 'chalk'; import { ConfigManager } from '../lib/Config.js'; import { GlobalConfigManager } from '../lib/GlobalConfig.js'; import { SkillManager } from '../lib/SkillManager.js'; -import { BUILTIN_SKILL_NAMES, BUILTIN_SKILL_REGISTRY } from '../constants.js'; +import { BUILTIN_SKILL_REGISTRY, getBuiltinSkillNames } from '../lib/BuiltinSkills.js'; import { ui } from '../util/terminal-ui.js'; import { withErrorHandler } from '../util/errors.js'; import { truncate, getErrorMessage } from '../util/text.js'; @@ -35,7 +35,7 @@ export function registerSkillCommand(program: Command): void { ui.warning('Ignoring registry and skill arguments because --built-in installs the curated AI DevKit set.'); } - for (const builtInSkill of BUILTIN_SKILL_NAMES) { + for (const builtInSkill of await getBuiltinSkillNames()) { await skillManager.addSkill(BUILTIN_SKILL_REGISTRY, builtInSkill, installOptions); } diff --git a/packages/cli/src/constants.ts b/packages/cli/src/constants.ts deleted file mode 100644 index e4b89d03..00000000 --- a/packages/cli/src/constants.ts +++ /dev/null @@ -1,37 +0,0 @@ -/** - * Registry identifier for the AI DevKit built-in skills. - */ -export const BUILTIN_SKILL_REGISTRY = 'codeaholicguy/ai-devkit'; - -/** - * Canonical list of built-in skills that ship with AI DevKit. Keep in sync - * with the skills published under the {@link BUILTIN_SKILL_REGISTRY} - * registry. Commands that need to install or reference the curated set - * (e.g., `ai-devkit init`, future `doctor`/`upgrade` commands) should import - * from here rather than hard-coding names locally. - */ -export const BUILTIN_SKILL_NAMES = [ - 'agent-communication', - 'agent-management', - 'ai-devkit-setup', - 'dev-commit', - 'dev-lifecycle', - 'dev-worktree', - 'dev-requirements', - 'dev-design', - 'dev-planning', - 'dev-implementation', - 'dev-testing', - 'dev-review', - 'dev-pr', - 'structured-debug', - 'document-code', - 'memory', - 'task', - 'simplify-implementation', - 'brainstorm', - 'verify', - 'tdd' -] as const; - -export type BuiltinSkillName = typeof BUILTIN_SKILL_NAMES[number]; diff --git a/packages/cli/src/lib/BuiltinSkills.ts b/packages/cli/src/lib/BuiltinSkills.ts new file mode 100644 index 00000000..c34c7bd8 --- /dev/null +++ b/packages/cli/src/lib/BuiltinSkills.ts @@ -0,0 +1,67 @@ +import { isValidSkillName } from '../util/skill.js'; +import { getErrorMessage } from '../util/text.js'; +import { ui } from '../util/terminal-ui.js'; + +const BUILTIN_SKILLS_URL = + 'https://raw.githubusercontent.com/codeaholicguy/ai-devkit/main/skills/built-in.json'; + +export const BUILTIN_SKILL_REGISTRY = 'codeaholicguy/ai-devkit'; + +const FALLBACK_BUILTIN_SKILL_NAMES = [ + 'agent-communication', + 'agent-management', + 'ai-devkit-setup', + 'dev-commit', + 'dev-lifecycle', + 'dev-worktree', + 'dev-requirements', + 'dev-design', + 'dev-planning', + 'dev-implementation', + 'dev-testing', + 'dev-review', + 'dev-pr', + 'structured-debug', + 'document-code', + 'memory', + 'task', + 'simplify-implementation', + 'brainstorm', + 'verify', + 'tdd', +] as const; + +let builtInSkillNamesPromise: Promise | undefined; + +export function getBuiltinSkillNames(): Promise { + builtInSkillNamesPromise ??= loadBuiltinSkillNames(); + + return builtInSkillNamesPromise; +} + +async function loadBuiltinSkillNames(): Promise { + try { + const response = await fetch(BUILTIN_SKILLS_URL); + if (!response.ok) { + throw new Error(`HTTP ${response.status}`); + } + + const manifest: unknown = await response.json(); + if (!Array.isArray(manifest) || manifest.length === 0) { + throw new Error('manifest must be a non-empty array'); + } + if (!manifest.every(name => typeof name === 'string' && isValidSkillName(name))) { + throw new Error('manifest entries must be valid, non-empty skill names'); + } + if (new Set(manifest).size !== manifest.length) { + throw new Error('manifest skill names must be unique'); + } + + return manifest; + } catch (error: unknown) { + ui.warning( + `Failed to load built-in skills manifest: ${getErrorMessage(error)}. Using bundled fallback.` + ); + return FALLBACK_BUILTIN_SKILL_NAMES; + } +} diff --git a/packages/cli/src/services/setup/setup.service.ts b/packages/cli/src/services/setup/setup.service.ts index 4ddbdea3..9135104c 100644 --- a/packages/cli/src/services/setup/setup.service.ts +++ b/packages/cli/src/services/setup/setup.service.ts @@ -4,7 +4,7 @@ import { homedir } from 'os'; import { dirname, join, resolve } from 'path'; import { fileURLToPath } from 'url'; import { promisify } from 'util'; -import { BUILTIN_SKILL_NAMES, BUILTIN_SKILL_REGISTRY } from '../../constants.js'; +import { BUILTIN_SKILL_REGISTRY, getBuiltinSkillNames } from '../../lib/BuiltinSkills.js'; import { ConfigManager } from '../../lib/Config.js'; import { SkillManager } from '../../lib/SkillManager.js'; import { getErrorMessage } from '../../util/text.js'; @@ -311,7 +311,7 @@ async function defaultRunCommand(command: string, args: string[]): Promise async function defaultInstallBuiltInSkills(agent: SetupAgent): Promise { const skillManager = new SkillManager(new ConfigManager()); - for (const builtInSkill of BUILTIN_SKILL_NAMES) { + for (const builtInSkill of await getBuiltinSkillNames()) { await skillManager.addSkill(BUILTIN_SKILL_REGISTRY, builtInSkill, { global: true, environments: [agent], diff --git a/packages/cli/src/services/status/status.service.ts b/packages/cli/src/services/status/status.service.ts index 8d0d4a43..fc3b7d11 100644 --- a/packages/cli/src/services/status/status.service.ts +++ b/packages/cli/src/services/status/status.service.ts @@ -13,7 +13,7 @@ import { type ReadinessAgentType, type ReadinessStatus, } from '@ai-devkit/agent-manager'; -import { BUILTIN_SKILL_NAMES } from '../../constants.js'; +import { getBuiltinSkillNames } from '../../lib/BuiltinSkills.js'; import { filterStringRecord } from '../../util/config.js'; import { getGlobalSkillPath, isValidEnvironmentCode } from '../../util/env.js'; import { inspectTmux } from '../../util/tmux.js'; @@ -346,12 +346,13 @@ function leafStatuses(report: Omit): CheckSt export async function getStatusReport(options: StatusServiceOptions = {}): Promise { const rt = runtime(options); + const builtInSkillNames = await getBuiltinSkillNames(); const projectPromise = projectConfigCheck(rt); const agentOptions: AgentReadinessOptions = { homeDir: rt.homeDir, path: rt.path, assetRoot: rt.assetRoot, - builtInSkillNames: BUILTIN_SKILL_NAMES, + builtInSkillNames, skillRoots: STATUS_SKILL_ROOTS, readFile: rt.readFile, access: rt.access, diff --git a/skills/built-in.json b/skills/built-in.json new file mode 100644 index 00000000..63871cd4 --- /dev/null +++ b/skills/built-in.json @@ -0,0 +1,23 @@ +[ + "agent-communication", + "agent-management", + "ai-devkit-setup", + "dev-commit", + "dev-lifecycle", + "dev-worktree", + "dev-requirements", + "dev-design", + "dev-planning", + "dev-implementation", + "dev-testing", + "dev-review", + "dev-pr", + "structured-debug", + "document-code", + "memory", + "task", + "simplify-implementation", + "brainstorm", + "verify", + "tdd" +]