diff --git a/docs/ai/design/2026-09-07-feature-local-registry.md b/docs/ai/design/2026-09-07-feature-local-registry.md new file mode 100644 index 00000000..a316e41b --- /dev/null +++ b/docs/ai/design/2026-09-07-feature-local-registry.md @@ -0,0 +1,74 @@ +--- +phase: design +title: Local Folder Skill Registry Design +description: Explicit file URL sources with read-only preparation and contained discovery +--- + +# Local Folder Skill Registry Design + +## Architecture + + flowchart TD + Input[CLI/config/template string] --> Parse[parse and normalize] + Parse -->|non-file| Git[Git cache preparation] + Parse -->|file URL| Local[read-only local preparation] + Git --> Discover[bounded discovery] + Local --> Discover + Discover --> Install[install target] + Discover --> Index[skills.json] + Remove --> Owned[config/index/contained cache only] + +The string map remains the storage boundary. A parser returns the local path for file URLs and null for Git sources; SkillRegistry branches preparation/update and consumers receive the actual prepared root. + +## Data Model + + parseLocalRegistryPath(source): string | null + +Local storage is a canonical file:///absolute/path string. No object migration or provider class is introduced. + +## Parsing and Normalization + +- Any file: prefix is local and validated strictly with fileURLToPath; malformed values throw. +- Every non-file persisted value is Git. Existence is never a discriminator. +- CLI shorthand is absolute, ./, or ../ and uses realpath then pathToFileURL. +- Templates use their directory as base; configs use the containing config directory. +- Hosted file URLs and unsupported Windows syntax are rejected. +- Canonical paths reject a different ID for the same folder. + +## Preparation and Freshness + +prepareRegistryRepository retains its per-instance promise map. Git keeps clone/pull/stale-cache behavior. Local validates its directory and skills, emits a local-source message, returns the path, calls no Git helper, and never falls back to cache. Reads remain live. + +## Discovery and Containment + +A shared routine canonicalizes root and skills, streams direct entries with opendir, enforces a candidate limit, validates names, canonicalizes skill and metadata paths, requires strict containment, stats metadata before a bounded read, and never recurses. Explicit install uses the same containment guard. + +Production limits are documented constants based on measured repositories, with no user-facing or test-only configuration surface. + +## Flow Integration + +- Add-registry normalizes and validates before mutation, rejects duplicates, prepares, then indexes the returned root. +- Add/reconcile requires Git only on the Git branch. +- Find refreshes local entries every call; rebuild partitions GitHub and local sources. +- Update pulls Git caches but validates/reports local sources as live. +- Removal deletes config, index entries, and optionally only an ID-derived contained cache. +- Status formats local file URLs and preserves Git credential sanitization. +- Installed listing infers cache provenance only after containment. + +## Errors and Security + +Errors cover missing/non-directory/missing-skills/empty sources, escaping symlinks, duplicates, hosted file URLs, and limits. Local operations are read-only by construction; destructive APIs accept IDs only. Registry flows never execute skill content. + +## Alternatives + +| Option | Decision | +|---|---| +| Existing-directory detection | Reject: ambiguous and state-dependent. | +| Raw relative storage | Reject: cwd-dependent. | +| --local only | Reject: config cannot express type. | +| Object config | Defer: broad migration. | +| Provider hierarchy | Defer: no current third source. | + +## Rollback + +Removing the parser branch restores Git-only behavior without migrating Git users. No local data migration or source mutation exists. diff --git a/docs/ai/implementation/2026-09-07-feature-local-registry.md b/docs/ai/implementation/2026-09-07-feature-local-registry.md new file mode 100644 index 00000000..2fa5dc0b --- /dev/null +++ b/docs/ai/implementation/2026-09-07-feature-local-registry.md @@ -0,0 +1,50 @@ +--- +phase: implementation +title: Local Folder Skill Registry Implementation +description: Running implementation record +--- + +# Implementation + +## Setup + +- Worktree: .worktrees/feature-local-registry +- Branch: feature-local-registry from fetched origin/main at 60c3bc1. +- npm ci completed; initial npm run build built six projects. +- Task tracing unavailable: npx ai-devkit@latest task list --name local-registry --json returned unknown command task. + +## Code Structure + +- Parsing: packages/cli/src/util/skill-registry.ts +- Config edges: Config.ts, GlobalConfig.ts, InitTemplate.ts +- Preparation/update: SkillRegistry.ts +- Discovery/install/removal: SkillManager.ts +- Search: SkillIndex.ts +- CLI/status: commands/skill.ts and status.service.ts + +## Implementation Log + +- T1–T2: Parser/canonicalization/duplicate tests drove the explicit source boundary and base-directory normalization. +- T4: Prep-once/no-Git/cache-fallback tests drove separate read-only local and Git preparation. +- T5: Real temp-directory tests drove direct discovery, entry/file limits, and symlink containment. +- T6–T7: Indexing uses actual local roots and update treats them as live; stale same-ID caches are excluded. +- T8–T9: Removal cleans focused index and only ID-derived cache paths; status/provenance are source-aware. +- T10: Built-CLI e2e registers a relative folder, installs, removes registration, and proves the source remains. + +Red evidence included missing parser functions and a missing local-registry module. Green evidence includes focused suites, 1,139 CLI tests, the full workspace suite, and e2e. + +## Invariants + +- file: is the only persisted local discriminator. +- Local preparation is read-only and never falls back to cache. +- Deletion derives only from ID beneath the owned cache. +- Local skill/metadata paths are canonically contained. +- Local index data is live rather than governed by remote TTL. + +## Deviations + +The pre-merge simplification audit replaced the exported parsed-source union with a local-path-or-null parser, consolidated cross-scope duplicate detection into the existing normalization pass, removed test-only discovery-limit injection, removed unused discovery fields, required the prepared path at the focused-index call site, and deleted a redundant metadata stat/limit check. Limits remain 10,000 direct entries and 1 MiB per SKILL.md, against a measured built-in baseline of 28 entries and a largest SKILL.md of 7,522 bytes. + +## Final Review + +The simplified implementation matches the requirements and design. All parser, config, preparation, discovery, install, index, update, removal, status, template, and CLI call sites were traced. No local source path reaches Git or deletion operations; removal remains ID-derived and cache-contained. The audit removed 43 net lines from production and tests in commit 5d61f46 without changing the safety contract. No blocking findings remain. diff --git a/docs/ai/planning/2026-09-07-feature-local-registry.md b/docs/ai/planning/2026-09-07-feature-local-registry.md new file mode 100644 index 00000000..e19ad9d6 --- /dev/null +++ b/docs/ai/planning/2026-09-07-feature-local-registry.md @@ -0,0 +1,49 @@ +--- +phase: planning +title: Local Folder Skill Registry Plan +description: TDD plan for explicit read-only local sources +--- + +# Plan + +## Milestones and Tasks + +- [x] **M1 Source boundary** + - [x] T1: TDD source parser and explicit file:/Git classification (AC-01, SR-06, SR-12). + - [x] T2: TDD cwd/config/template normalization and canonical duplicate rejection (AC-02–05). + - [x] T3: Document canonical storage and move/re-add semantics. +- [x] **M2 Runtime flows** + - [x] T4: TDD read-only local preparation, no Git calls, and prep-once compatibility (AC-06, SR-01–05). + - [x] T5: TDD fixture discovery/install, missing/empty errors, containment, direct-only and bounded reads (AC-07, SR-07–10). + - [x] T6: TDD focused/full/seed/TTL index behavior using actual source roots (AC-08). + - [x] T7: TDD selected/local-only/mixed update behavior (AC-09). +- [x] **M3 Removal and surfaces** + - [x] T8: TDD config/index cleanup and ID-derived cache-only deletion; snapshot local fixtures (AC-10, SR-01–04). + - [x] T9: TDD status and installed provenance (AC-11–12, SR-11). + - [x] T10: CLI e2e normalization/error/removal journeys and user docs. + - [x] T11: Reconcile docs; implementation check, coverage, build, tests, lint, e2e, final review. +- [x] **M4 Pre-merge simplification** + - [x] T12: Trace every new abstraction, guard, fallback, and test to a current caller or demonstrated safety trigger. + - [x] T13: Remove unused source/discovery surface, duplicate validation, redundant filesystem work, and implementation-detail assertions. + - [x] T14: Reconcile lifecycle docs and rerun build, full tests, lint, and e2e before push. + +## Dependencies + +T1 precedes all source-aware flows. T4–T5 precede index/update. T8 remains ID-derived. Documentation follows stable CLI behavior. No external API or database migration exists. + +## TDD Evidence + +Every production change follows focused red, green, refactor commands recorded in implementation/testing docs. Final evidence: npm run build, npm test, npm run lint, npm run test:e2e, and focused coverage. + +## Risks + +- Local mutation: snapshot fixtures and spy on Git/write/delete boundaries. +- Symlink escape: canonical containment before read/install. +- Stale search: local entries refresh independently of remote TTL/seed. +- Compatibility: non-file values stay on Git branch. +- Oversized sources: streamed direct iteration and metadata-size limits. +- Scope: no provider hierarchy, watcher, object schema, or Windows/UNC support. + +## Progress + +All tasks are complete. Final review found no blocking issues. The only validation limitation is that the repository's test:coverage script forwards --coverage as an npm config flag and produces no trustworthy percentage. diff --git a/docs/ai/requirements/2026-09-07-feature-local-registry.md b/docs/ai/requirements/2026-09-07-feature-local-registry.md new file mode 100644 index 00000000..4a01f9d2 --- /dev/null +++ b/docs/ai/requirements/2026-09-07-feature-local-registry.md @@ -0,0 +1,72 @@ +--- +phase: requirements +title: Local Folder Skill Registries +description: Support canonical, read-only local folders as skill registry sources +--- + +# Local Folder Skill Registries + +## Problem + +AI DevKit treats every registry string as Git and clones/pulls it into ~/.ai-devkit/skills. Skill authors cannot consume a registry directly from a local development folder. + +## Goals + +- Accept absolute paths, ./..., ../..., and explicit file: URLs in skill add-registry. +- Resolve shorthand at registration and persist one canonical absolute file: URL. +- Support local roots across add, reconciliation, discovery, find/index, update, remove, status, and templates. +- Preserve Git behavior and give clear errors for missing, moved, empty, or malformed folders. + +## Non-goals + +- Windows drive/UNC support in phase 1. +- Watchers, local registry caches, provider hierarchies, or execution of SKILL.md. +- Automatic repair after a folder moves. + +## Acceptance Criteria + +- **AC-01:** Persisted file: values are unambiguously local; non-file strings retain Git semantics. Invalid file: values never fall through to Git. +- **AC-02:** CLI absolute, ./, and ../ inputs resolve against registration cwd, pass through realpath, and store pathToFileURL(realPath).href. +- **AC-03:** Template-relative paths resolve against the template directory. Manually authored project/global relative paths resolve against their config directory. +- **AC-04:** Canonical identity rejects another registry ID for the same directory. Moving a folder requires re-adding it. +- **AC-05:** Hosted file URLs, unsupported Windows paths, missing/non-directory/unusable/empty roots fail clearly. +- **AC-06:** Local preparation memoizes once per SkillRegistry instance, returns the canonical path, and stays read-only. +- **AC-07:** Add and reconciliation discover/install local skills without requiring Git. +- **AC-08:** Focused/full indexing reads local roots directly; local entries supplement seeds and refresh independently of remote TTL. +- **AC-09:** Update validates local availability and reports a live-filesystem no-op without mutation. +- **AC-10:** Removal deletes config and focused index data; global removal may clean only owned cache data and never the source. +- **AC-11:** Status identifies local registries without misclassifying or redacting them. +- **AC-12:** Installed listing never infers ../ registry IDs from symlinks outside the cache. + +## Safety Rules + +- **SR-01:** Never clone, pull, checkout, clean, create, copy into, write into, or delete a local registry. +- **SR-02:** Local preparation bypasses all Git operations, including Git-installation checks. +- **SR-03:** Never derive a deletion target from a local source path. +- **SR-04:** Removal may delete only an ID-derived target proven strictly beneath SKILL_CACHE_DIR. +- **SR-05:** Never fall back from an unavailable local source to a same-ID cache. +- **SR-06:** Never detect source type by filesystem existence. +- **SR-07:** Skill directories and SKILL.md must canonically remain beneath registry/skills; reject escaping symlinks. +- **SR-08:** Examine direct skills children only; never recursively search. +- **SR-09:** Bound candidate enumeration and SKILL.md reads with documented limits. +- **SR-10:** Never execute SKILL.md in registry flows. +- **SR-11:** Continue sanitizing credential-bearing Git URLs in status. +- **SR-12:** Reject malformed file: sources as local errors, never Git. + +## Success Criteria + +- Unit tests cover every safety rule and parser branch. +- Temp-directory adapters prove sources remain unchanged through preparation, index, update, and removal. +- CLI e2e covers normalization and errors. +- Six-project build, full tests, lint, and e2e pass. + +## Constraints and Assumptions + +- Registry IDs keep org/repo validation; config remains Record. +- Installs keep symlink-first/copy-fallback after containment validation; writes target install locations only. +- Local sources are live; search re-enumerates them. +- Production limits use measured evidence and tests exercise those limits directly. + +## Questions + +All phase-1 choices are resolved by the approved design of record. Windows/UNC, watchers/fingerprints, and structured config are deferred. diff --git a/docs/ai/testing/2026-09-07-feature-local-registry.md b/docs/ai/testing/2026-09-07-feature-local-registry.md new file mode 100644 index 00000000..62af598f --- /dev/null +++ b/docs/ai/testing/2026-09-07-feature-local-registry.md @@ -0,0 +1,58 @@ +--- +phase: testing +title: Local Folder Skill Registry Testing +description: Safety-first testing strategy +--- + +# Testing + +## Goals + +Safety-contract coverage for SR-01–12, temp-directory adapter coverage, CLI e2e path/error coverage, and green Git registry regressions. Tests target observable boundaries and demonstrated guard triggers rather than private result shapes or injectable test-only limits. + +## Source and Config + +- [x] Absolute, ./, and ../ normalize to canonical file URLs against registration cwd. (AC-01–02) +- [x] Template/config relative paths use their containing file. (AC-03) +- [x] Root symlinks and trailing slashes deduplicate; different IDs are rejected. (AC-04) +- [x] Git URL/SCP values remain Git; malformed/hosted file values fail locally. (AC-01, AC-05, SR-06, SR-12) + +## Preparation and Install + +- [x] Local preparation returns the root with no Git/write call and memoizes once. (AC-06, SR-01–02) +- [x] Missing/moved, non-directory, missing-skills, and empty roots error clearly without cache fallback. (AC-05, SR-05) +- [x] Valid temp fixture installs without modifying source. (AC-07, SR-01) +- [x] Escaping symlinks are rejected before read/install. (SR-07) +- [x] Nested skills are ignored; candidate/metadata limits fail clearly. (SR-08–09) +- [x] Fixture skill content is never executed. (SR-10) + +## Index and Update + +- [x] Focused/full indexing uses local roots; seed/TTL paths refresh local entries. (AC-08) +- [x] Selected/local-only/mixed updates report live local sources and make no Git call for them. (AC-09, SR-01–02) +- [x] Missing local update errors without fallback. (SR-05) + +## Removal and Display + +- [x] Project/global removal deletes config/index but not source. (AC-10, SR-01, SR-03) +- [x] Global removal deletes only contained ID-derived cache data. (SR-04) +- [x] Status displays local and redacts Git credentials. (AC-11, SR-11) +- [x] Listing does not infer escaped cache-relative IDs. (AC-12) + +## Fixtures + +Tests create isolated temp roots with skills/name/SKILL.md, snapshot source content/metadata, and clean only the test-owned outer temp directory. Escape fixtures point to a second temp root. + +## Required Validation + +- [x] Focused unit/coverage +- [x] npm run build +- [x] npm test +- [x] npm run lint +- [x] npm run test:e2e + +## Results + +Post-simplification evidence: npm run build built six projects; npm test passed 2,190 tests across six projects, including 1,142 CLI tests; npm run lint passed with zero errors and two unrelated existing warnings; npm run test:e2e passed 42 tests. The focused local-registry suites passed 139 tests before the full gates. Coverage tooling limitation: npm run test:coverage exits successfully but Nx forwards --coverage as an npm config option, so Vitest runs without a coverage report; a selected-file direct coverage run is not representative because unselected files count as zero. + +The pre-merge simplification pass kept all named safety tests, changed the enumeration and metadata-limit tests to exercise production thresholds directly, and removed duplicate no-Git and UI-format assertions. diff --git a/e2e/cli.e2e.ts b/e2e/cli.e2e.ts index ba3178e8..e11483e0 100644 --- a/e2e/cli.e2e.ts +++ b/e2e/cli.e2e.ts @@ -377,6 +377,41 @@ describe('install command', () => { }); describe('skill command', () => { + it('registers, installs, and removes a relative local registry without deleting it', () => { + const projectDir = createTempProject(); + const homeDir = join(projectDir, 'home'); + const registryDir = join(projectDir, 'local-registry'); + mkdirSync(join(registryDir, 'skills', 'local-test'), { recursive: true }); + writeFileSync(join(registryDir, 'skills', 'local-test', 'SKILL.md'), '---\ndescription: local fixture\n---\n'); + writeConfigFile(projectDir, { + version: '1.0.0', environments: ['claude'], phases: [], createdAt: new Date().toISOString(), + }); + + try { + const added = run('skill add-registry local/skills ./local-registry', { + cwd: projectDir, env: { HOME: homeDir }, + }); + expect(added.exitCode).toBe(0); + const config = JSON.parse(readFileSync(join(projectDir, '.ai-devkit.json'), 'utf8')); + expect(config.registries['local/skills']).toBe(`file://${realpathSync(registryDir)}`); + + const installed = run('skill add local/skills local-test', { + cwd: projectDir, env: { HOME: homeDir }, + }); + expect(installed.exitCode).toBe(0); + expect(existsSync(join(projectDir, '.claude', 'skills', 'local-test', 'SKILL.md'))).toBe(true); + + const removed = run('skill remove-registry local/skills', { + cwd: projectDir, env: { HOME: homeDir }, + }); + expect(removed.exitCode).toBe(0); + expect(existsSync(join(registryDir, 'skills', 'local-test', 'SKILL.md'))).toBe(true); + expect(JSON.parse(readFileSync(join(projectDir, '.ai-devkit.json'), 'utf8')).registries).toEqual({}); + } finally { + cleanupTempProject(projectDir); + } + }); + it('should list skills (empty)', () => { const projectDir = createTempProject(); run('init -e claude -p requirements', { cwd: projectDir }); diff --git a/packages/cli/src/__tests__/commands/skill.test.ts b/packages/cli/src/__tests__/commands/skill.test.ts index 33f2d4fb..95ed4a2d 100644 --- a/packages/cli/src/__tests__/commands/skill.test.ts +++ b/packages/cli/src/__tests__/commands/skill.test.ts @@ -14,6 +14,7 @@ const mockListSkills = vi.fn(); const mockRemoveSkill = vi.fn(); const mockCacheRegistry = vi.fn(); const mockUpdateSkillIndexForRegistry = vi.fn(); +const mockRemoveSkillIndexForRegistry = vi.fn(); const mockProjectGetSkillRegistries = vi.fn(); const mockProjectAddSkillRegistry = vi.fn(); const mockProjectRemoveSkillRegistry = vi.fn(); @@ -45,6 +46,7 @@ vi.mock('../../lib/SkillManager.js', () => ({ removeSkill: (...args: unknown[]) => mockRemoveSkill(...args), cacheRegistry: (...args: unknown[]) => mockCacheRegistry(...args), updateSkillIndexForRegistry: (...args: unknown[]) => mockUpdateSkillIndexForRegistry(...args), + removeSkillIndexForRegistry: (...args: unknown[]) => mockRemoveSkillIndexForRegistry(...args), removeRegistryCache: (...args: unknown[]) => mockRemoveCache(...args), updateSkills: vi.fn(), findSkills: vi.fn(), @@ -75,8 +77,9 @@ describe('skill command', () => { mockListGlobalSkills.mockResolvedValue([]); mockListSkills.mockResolvedValue([]); mockRemoveSkill.mockImplementation(async () => undefined); - mockCacheRegistry.mockImplementation(async () => undefined); + mockCacheRegistry.mockResolvedValue('/tmp/registry-cache'); mockUpdateSkillIndexForRegistry.mockImplementation(async () => undefined); + mockRemoveSkillIndexForRegistry.mockImplementation(async () => undefined); mockRemoveCache.mockResolvedValue(undefined); mockProjectGetSkillRegistries.mockResolvedValue({}); mockProjectAddSkillRegistry.mockResolvedValue({}); @@ -158,7 +161,7 @@ describe('skill command', () => { 'example/private-skills', 'git@example.com:example/private-skills.git', ); - expect(mockUpdateSkillIndexForRegistry).toHaveBeenCalledWith('example/private-skills'); + expect(mockUpdateSkillIndexForRegistry).toHaveBeenCalledWith('example/private-skills', '/tmp/registry-cache'); expect(mockCacheRegistry.mock.invocationCallOrder[0]).toBeLessThan( mockUpdateSkillIndexForRegistry.mock.invocationCallOrder[0], ); @@ -263,7 +266,7 @@ describe('skill command', () => { const addRegistryCommand = skillCommand?.commands.find(command => command.name() === 'add-registry'); expect(addRegistryCommand?.usage()).toContain(''); - expect(addRegistryCommand?.usage()).toContain(''); + expect(addRegistryCommand?.usage()).toContain(''); expect(addRegistryCommand?.helpInformation()).toContain('-g, --global'); expect(addRegistryCommand?.helpInformation()).toContain('-f, --force'); const removeRegistryCommand = skillCommand?.commands.find(command => command.name() === 'remove-registry'); diff --git a/packages/cli/src/__tests__/lib/SkillManager.test.ts b/packages/cli/src/__tests__/lib/SkillManager.test.ts index 03fed7df..781ef6de 100644 --- a/packages/cli/src/__tests__/lib/SkillManager.test.ts +++ b/packages/cli/src/__tests__/lib/SkillManager.test.ts @@ -1180,12 +1180,12 @@ describe("SkillManager", () => { mockedGitUtil.ensureGitInstalled.mockResolvedValue(undefined); }); - it("should ensure git is installed before updating", async () => { + it("does not require git when there is no cached git registry to update", async () => { (mockedFs.pathExists as any).mockResolvedValue(false); await skillManager.updateSkills(); - expect(mockedGitUtil.ensureGitInstalled).toHaveBeenCalled(); + expect(mockedGitUtil.ensureGitInstalled).not.toHaveBeenCalled(); }); it("should return empty summary when cache directory does not exist", async () => { diff --git a/packages/cli/src/__tests__/lib/SkillRegistry.test.ts b/packages/cli/src/__tests__/lib/SkillRegistry.test.ts index cb704c11..0772843e 100644 --- a/packages/cli/src/__tests__/lib/SkillRegistry.test.ts +++ b/packages/cli/src/__tests__/lib/SkillRegistry.test.ts @@ -19,6 +19,10 @@ vi.mock('fs-extra', () => ({ default: { pathExists: vi.fn(), ensureDir: vi.fn(), + stat: vi.fn(), + readdir: vi.fn(), + opendir: vi.fn(), + realpath: vi.fn(), }, })); @@ -169,4 +173,64 @@ describe('SkillRegistry repository preparation', () => { `Cached registry ${registryId} is not a git repository, using as-is.`, ); }); + + it('prepares a local registry once without invoking Git or writing', async () => { + const localPath = '/tmp/local-skills'; + mockedFs.realpath.mockResolvedValue(localPath); + mockedFs.stat.mockResolvedValue({ isDirectory: () => true } as Awaited>); + mockedFs.opendir.mockResolvedValue({ + async *[Symbol.asyncIterator]() { + yield { name: 'test-skill', isDirectory: () => true, isSymbolicLink: () => false }; + }, + } as Awaited>); + mockedFs.pathExists.mockResolvedValue(true); + const registry = createRegistry(); + + await expect(registry.prepareRegistryRepository(registryId, 'file:///tmp/local-skills')) + .resolves.toBe(localPath); + await expect(registry.prepareRegistryRepository(registryId, 'file:///tmp/local-skills')) + .resolves.toBe(localPath); + + expect(mockedFs.realpath).toHaveBeenCalledTimes(1); + expect(mockedFs.ensureDir).not.toHaveBeenCalled(); + expect(mockedGit.ensureGitInstalled).not.toHaveBeenCalled(); + expect(mockedGit.isGitRepository).not.toHaveBeenCalled(); + expect(mockedGit.pullRepository).not.toHaveBeenCalled(); + expect(mockedGit.cloneRepository).not.toHaveBeenCalled(); + }); + + it('does not use a same-ID cache when a local registry is missing', async () => { + mockedFs.realpath.mockRejectedValue(new Error('ENOENT')); + mockedFs.pathExists.mockResolvedValue(true); + + await expect(createRegistry().prepareRegistryRepository(registryId, 'file:///missing')) + .rejects.toThrow(/unavailable/i); + expect(mockedGit.pullRepository).not.toHaveBeenCalled(); + }); + + it('treats update of a local registry as a read-only live-filesystem no-op', async () => { + const localPath = '/tmp/local-skills'; + vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ ok: true, json: async () => ({ registries: {} }) })); + mockedFs.realpath.mockResolvedValue(localPath); + mockedFs.stat.mockResolvedValue({ isDirectory: () => true } as Awaited>); + mockedFs.opendir.mockResolvedValue({ + async *[Symbol.asyncIterator]() { + yield { name: 'test-skill', isDirectory: () => true, isSymbolicLink: () => false }; + }, + } as Awaited>); + mockedFs.pathExists.mockResolvedValue(true); + mockedFs.readdir.mockResolvedValue([]); + const registry = new SkillRegistry( + { getSkillRegistries: vi.fn().mockResolvedValue({ [registryId]: 'file:///tmp/local-skills' }) } as unknown as ConfigManager, + { getSkillRegistries: vi.fn().mockResolvedValue({}) } as unknown as GlobalConfigManager, + ); + + await expect(registry.updateSkills(registryId)).resolves.toMatchObject({ + total: 1, successful: 0, skipped: 1, failed: 0, + }); + expect(mockedGit.ensureGitInstalled).not.toHaveBeenCalled(); + expect(mockedGit.pullRepository).not.toHaveBeenCalled(); + expect(mockedGit.isGitRepository).not.toHaveBeenCalled(); + expect(mockedFs.ensureDir).not.toHaveBeenCalled(); + }); }); 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 e1314db5..29f9c203 100644 --- a/packages/cli/src/__tests__/services/status/status.service.test.ts +++ b/packages/cli/src/__tests__/services/status/status.service.test.ts @@ -23,6 +23,7 @@ function fixture(overrides: Partial = {}) { version: '0.55.0', environments: ['codex', 'pi', 'claude'], phases: [], createdAt: 'now', registries: { project: 'https://example.test/project.git', + local: 'file:///work/local-registry', private: 'https://user:registry-secret@example.test/private.git?token=query-secret', }, }), @@ -126,6 +127,7 @@ describe('getStatusReport', () => { expect(report.agents.copilot.auth?.status).toBe('pass'); expect(report.tmux).toMatchObject({ path: 'tmux', available: true, version: '3.4' }); expect(report.registries.project.configured).toMatchObject({ project: 'https://example.test/project.git' }); + expect(report.registries.project.configured.local).toBe('local: /work/local-registry'); expect(report.registries.global.configured).toEqual({ global: 'https://example.test/global.git' }); expect(report.aiDevkit).toMatchObject({ installedVersion: '0.55.0', latestVersion: '0.56.0', updateAvailable: true }); expect(report.project.config).toMatchObject({ present: true, valid: true, environments: ['codex', 'pi', 'claude'] }); diff --git a/packages/cli/src/__tests__/util/local-registry.test.ts b/packages/cli/src/__tests__/util/local-registry.test.ts new file mode 100644 index 00000000..246c1a47 --- /dev/null +++ b/packages/cli/src/__tests__/util/local-registry.test.ts @@ -0,0 +1,68 @@ +import fs from 'fs-extra'; +import os from 'node:os'; +import path from 'node:path'; +import { + discoverRegistrySkills, + LOCAL_REGISTRY_MAX_ENTRIES, + LOCAL_REGISTRY_MAX_SKILL_MD_BYTES, + resolveContainedSkill, +} from '../../util/local-registry.js'; + +describe('local registry filesystem boundary', () => { + let temp: string; + let root: string; + beforeEach(async () => { + temp = await fs.mkdtemp(path.join(os.tmpdir(), 'local-registry-')); + root = path.join(temp, 'registry'); + await fs.outputFile(path.join(root, 'skills', 'safe-skill', 'SKILL.md'), '---\ndescription: safe\n---'); + }); + afterEach(async () => fs.remove(temp)); + + it('discovers direct valid skills within explicit bounds', async () => { + await fs.outputFile(path.join(root, 'nested', 'skills', 'hidden', 'SKILL.md'), 'hidden'); + await expect(discoverRegistrySkills('test/skills', root)).resolves.toEqual([ + expect.objectContaining({ name: 'safe-skill' }), + ]); + }); + + it('bounds direct-entry enumeration at the production limit', async () => { + const opendir = vi.spyOn(fs, 'opendir').mockResolvedValue({ + async *[Symbol.asyncIterator]() { + for (let index = 0; index <= LOCAL_REGISTRY_MAX_ENTRIES; index += 1) { + yield { name: 'invalid_name', isDirectory: () => true, isSymbolicLink: () => false }; + } + }, + } as Awaited>); + + await expect(discoverRegistrySkills('test/skills', root)).rejects.toThrow(/entry limit/i); + opendir.mockRestore(); + }); + + it('bounds metadata reads at the production limit', async () => { + await fs.writeFile( + path.join(root, 'skills', 'safe-skill', 'SKILL.md'), + Buffer.alloc(LOCAL_REGISTRY_MAX_SKILL_MD_BYTES + 1), + ); + await expect(discoverRegistrySkills('test/skills', root)).rejects.toThrow(/too large/i); + }); + + it('rejects skill and metadata symlinks that escape the registry', async () => { + const outside = path.join(temp, 'outside'); + await fs.outputFile(path.join(outside, 'SKILL.md'), 'outside'); + await fs.symlink(outside, path.join(root, 'skills', 'escape-skill'), 'dir'); + await expect(resolveContainedSkill('test/skills', root, 'escape-skill')).rejects.toThrow(/outside/i); + + const metadataEscape = path.join(root, 'skills', 'metadata-escape'); + await fs.ensureDir(metadataEscape); + await fs.symlink(path.join(outside, 'SKILL.md'), path.join(metadataEscape, 'SKILL.md')); + await expect(resolveContainedSkill('test/skills', root, 'metadata-escape')).rejects.toThrow(/outside/i); + }); + + it('rejects oversized metadata on an explicit skill install path', async () => { + await fs.writeFile( + path.join(root, 'skills', 'safe-skill', 'SKILL.md'), + Buffer.alloc(LOCAL_REGISTRY_MAX_SKILL_MD_BYTES + 1), + ); + await expect(resolveContainedSkill('test/skills', root, 'safe-skill')).rejects.toThrow(/too large/i); + }); +}); diff --git a/packages/cli/src/__tests__/util/skill-registry.test.ts b/packages/cli/src/__tests__/util/skill-registry.test.ts index 76d2ba3e..cca8a485 100644 --- a/packages/cli/src/__tests__/util/skill-registry.test.ts +++ b/packages/cli/src/__tests__/util/skill-registry.test.ts @@ -1,4 +1,73 @@ -import { planSkillRegistryAdd, planSkillRegistryRemove } from '../../util/skill-registry.js'; +import fs from 'fs-extra'; +import os from 'node:os'; +import path from 'node:path'; +import { pathToFileURL } from 'node:url'; +import { + normalizeRegistrySourceInput, + normalizeRegistrySources, + parseLocalRegistryPath, + planSkillRegistryAdd, + planSkillRegistryRemove, +} from '../../util/skill-registry.js'; + +describe('registry sources', () => { + it('classifies only file URLs as persisted local sources', () => { + expect(parseLocalRegistryPath('https://example.com/skills.git')).toBeNull(); + expect(parseLocalRegistryPath('git@example.com:org/skills.git')).toBeNull(); + expect(parseLocalRegistryPath('file:///tmp/skills')).toBe('/tmp/skills'); + }); + + it('rejects malformed and hosted file URLs instead of treating them as Git', () => { + expect(() => parseLocalRegistryPath('file://remote/share')).toThrow(/host/i); + expect(() => parseLocalRegistryPath('file:%')).toThrow(/local registry/i); + }); + + it('canonicalizes absolute and relative path input at registration time', async () => { + const temp = await fs.mkdtemp(path.join(os.tmpdir(), 'registry-source-')); + const root = path.join(temp, 'registry'); + await fs.ensureDir(path.join(root, 'skills')); + const alias = path.join(temp, 'alias'); + await fs.symlink(root, alias, 'dir'); + + try { + const expected = pathToFileURL(await fs.realpath(root)).href; + expect(await normalizeRegistrySourceInput(root, temp)).toBe(expected); + expect(await normalizeRegistrySourceInput('./registry/', temp)).toBe(expected); + expect(await normalizeRegistrySourceInput('../alias', path.join(temp, 'child'))).toBe(expected); + expect(await normalizeRegistrySourceInput(pathToFileURL(alias).href, temp)).toBe(expected); + expect(await normalizeRegistrySourceInput('https://example.com/skills.git', temp)) + .toBe('https://example.com/skills.git'); + } finally { + await fs.remove(temp); + } + }); + + it('reports missing and non-directory local sources clearly', async () => { + const temp = await fs.mkdtemp(path.join(os.tmpdir(), 'registry-source-')); + const file = path.join(temp, 'file'); + await fs.writeFile(file, 'x'); + try { + await expect(normalizeRegistrySourceInput('./missing', temp)).rejects.toThrow(/not found/i); + await expect(normalizeRegistrySourceInput(file, temp)).rejects.toThrow(/not a directory/i); + } finally { + await fs.remove(temp); + } + }); + + it('rejects duplicate canonical local folders under different IDs', async () => { + const temp = await fs.mkdtemp(path.join(os.tmpdir(), 'registry-source-')); + await fs.ensureDir(path.join(temp, 'skills')); + try { + const source = pathToFileURL(temp).href; + await expect(normalizeRegistrySources({ + 'one/skills': source, + 'two/skills': `${source}/`, + }, process.cwd())).rejects.toThrow(/already registered as "one\/skills"/i); + } finally { + await fs.remove(temp); + } + }); +}); describe('planSkillRegistryAdd', () => { it('covers existing add planner states used by the shared module', () => { diff --git a/packages/cli/src/commands/skill.ts b/packages/cli/src/commands/skill.ts index 03281c1f..7834327a 100644 --- a/packages/cli/src/commands/skill.ts +++ b/packages/cli/src/commands/skill.ts @@ -8,7 +8,7 @@ import { ui } from '../util/terminal-ui.js'; import { withErrorHandler } from '../util/errors.js'; import { truncate, getErrorMessage } from '../util/text.js'; import { validateRegistryId } from '../util/skill.js'; -import { planSkillRegistryAdd } from '../util/skill-registry.js'; +import { normalizeRegistrySourceInput, normalizeRegistrySources, planSkillRegistryAdd } from '../util/skill-registry.js'; export function registerSkillCommand(program: Command): void { const skillCommand = program @@ -61,13 +61,13 @@ export function registerSkillCommand(program: Command): void { }); skillCommand - .command('add-registry ') - .description('Register a third-party skill registry') + .command('add-registry ') + .description('Register a Git or local-folder skill registry') .option('-g, --global', 'Register in global config (~/.ai-devkit/.ai-devkit.json)') - .option('-f, --force', 'Overwrite a conflicting registry URL') + .option('-f, --force', 'Overwrite a conflicting registry source') .action(withErrorHandler('add registry', async ( id: string, - url: string, + source: string, options: { global?: boolean; force?: boolean }, ) => { validateRegistryId(id); @@ -76,13 +76,19 @@ export function registerSkillCommand(program: Command): void { : new ConfigManager(); const registries = await configManager.getSkillRegistries(); - const mutation = planSkillRegistryAdd(registries, id, url, { force: options.force }); - await configManager.addSkillRegistry(id, url, { force: options.force }); + const value = await normalizeRegistrySourceInput(source, process.cwd()); + const [projectRegistries, globalRegistries] = await Promise.all([ + options.global ? new ConfigManager().getSkillRegistries() : Promise.resolve(registries), + options.global ? Promise.resolve(registries) : new GlobalConfigManager().getSkillRegistries(), + ]); + await normalizeRegistrySources({ ...globalRegistries, ...projectRegistries, [id]: value }, process.cwd()); + const mutation = planSkillRegistryAdd(registries, id, value, { force: options.force }); if (mutation.status !== 'already-registered') { const skillManager = new SkillManager(new ConfigManager()); - await skillManager.cacheRegistry(id, url); - await skillManager.updateSkillIndexForRegistry(id); + const registryPath = await skillManager.cacheRegistry(id, value); + await skillManager.updateSkillIndexForRegistry(id, registryPath); } + await configManager.addSkillRegistry(id, value, { force: options.force }); if (mutation.status === 'already-registered') { ui.info(`Registry "${id}" is already registered.`); @@ -115,8 +121,10 @@ export function registerSkillCommand(program: Command): void { } await configManager.removeSkillRegistry(id); + const skillManager = new SkillManager(new ConfigManager()); + await skillManager.removeSkillIndexForRegistry(id); if (options.global) { - await new SkillManager(new ConfigManager()).removeRegistryCache(id); + await skillManager.removeRegistryCache(id); } const scope = options.global ? 'global' : 'project'; diff --git a/packages/cli/src/lib/Config.ts b/packages/cli/src/lib/Config.ts index c16448a9..01bdaf70 100644 --- a/packages/cli/src/lib/Config.ts +++ b/packages/cli/src/lib/Config.ts @@ -3,7 +3,7 @@ import * as path from 'path'; import { DevKitConfig, Phase, EnvironmentCode, ConfigSkill, DEFAULT_DOCS_DIR, DEFAULT_PHASES } from '../types.js'; import { filterStringRecord } from '../util/config.js'; import { ConfigNotFoundError } from '../util/errors.js'; -import { AddSkillRegistryOptions, planSkillRegistryAdd, planSkillRegistryRemove } from '../util/skill-registry.js'; +import { AddSkillRegistryOptions, normalizeRegistrySources, planSkillRegistryAdd, planSkillRegistryRemove } from '../util/skill-registry.js'; import { GlobalConfigManager } from './GlobalConfig.js'; import packageJson from '../../package.json' with { type: 'json' }; @@ -189,7 +189,7 @@ export class ConfigManager { async getSkillRegistries(): Promise> { const config = await this.read(); - return filterStringRecord(config?.registries); + return normalizeRegistrySources(filterStringRecord(config?.registries), path.dirname(this.configPath)); } async addSkillRegistry(id: string, url: string, options: AddSkillRegistryOptions = {}): Promise { diff --git a/packages/cli/src/lib/GlobalConfig.ts b/packages/cli/src/lib/GlobalConfig.ts index cd01902d..6b7b5372 100644 --- a/packages/cli/src/lib/GlobalConfig.ts +++ b/packages/cli/src/lib/GlobalConfig.ts @@ -4,7 +4,7 @@ import * as path from 'path'; import { GlobalDevKitConfig } from '../types.js'; import { filterStringRecord } from '../util/config.js'; import { CliError } from '../util/errors.js'; -import { AddSkillRegistryOptions, planSkillRegistryAdd, planSkillRegistryRemove } from '../util/skill-registry.js'; +import { AddSkillRegistryOptions, normalizeRegistrySources, planSkillRegistryAdd, planSkillRegistryRemove } from '../util/skill-registry.js'; import { ui } from '../util/terminal-ui.js'; export class GlobalConfigManager { @@ -28,7 +28,7 @@ export class GlobalConfigManager { async getSkillRegistries(): Promise> { const config = await this.read(); - return filterStringRecord(config?.registries); + return normalizeRegistrySources(filterStringRecord(config?.registries), path.dirname(this.getGlobalConfigPath())); } async addSkillRegistry(id: string, url: string, options: AddSkillRegistryOptions = {}): Promise { diff --git a/packages/cli/src/lib/InitTemplate.ts b/packages/cli/src/lib/InitTemplate.ts index cda381a7..c78d2589 100644 --- a/packages/cli/src/lib/InitTemplate.ts +++ b/packages/cli/src/lib/InitTemplate.ts @@ -3,6 +3,7 @@ import * as path from 'path'; import YAML from 'yaml'; import { AVAILABLE_PHASES, EnvironmentCode, MCP_TRANSPORTS, McpServerDefinition, McpTransport, Phase } from '../types.js'; import { isValidEnvironmentCode } from '../util/env.js'; +import { normalizeRegistrySources } from '../util/skill-registry.js'; export interface InitTemplateSkill { registry: string; @@ -279,5 +280,9 @@ export async function loadInitTemplate(templatePath: string): Promise { - const localSkills = await this.readLocalRegistrySkills(registryId); + async updateRegistryFromCache(registryId: string, registryPath: string): Promise { + const localSkills = await this.readLocalRegistrySkills(registryId, registryPath); if (!localSkills) { return; } @@ -90,6 +92,15 @@ export class SkillIndex { await fs.writeJson(SKILL_INDEX_PATH, nextIndex, { spaces: 2 }); } + async removeRegistry(registryId: string): Promise { + const existingIndex = await this.readExistingIndex(); + if (!existingIndex) return; + existingIndex.skills = existingIndex.skills.filter(skill => skill.registry !== registryId); + delete existingIndex.meta.registryHeads[registryId]; + existingIndex.meta.updatedAt = Date.now(); + await fs.writeJson(SKILL_INDEX_PATH, existingIndex, { spaces: 2 }); + } + private async ensureSkillIndex(forceRefresh = false): Promise { const indexExists = await fs.pathExists(SKILL_INDEX_PATH); @@ -99,7 +110,7 @@ export class SkillIndex { const age = Date.now() - (index.meta.updatedAt || 0); if (age < INDEX_TTL_MS) { - return index; + return this.refreshLocalRegistryEntries(index); } ui.info(`Index is older than 24h, checking for updates...`); } catch (ignore) { @@ -117,7 +128,7 @@ export class SkillIndex { await fs.ensureDir(path.dirname(SKILL_INDEX_PATH)); await fs.writeJson(SKILL_INDEX_PATH, seedIndex, { spaces: 2 }); spinner.succeed('Seed index fetched successfully'); - return seedIndex; + return this.refreshLocalRegistryEntries(seedIndex); } } catch (ignore) { spinner.fail('Failed to fetch seed index, falling back to build'); @@ -150,7 +161,7 @@ export class SkillIndex { const registryIds = Object.keys(registry.registries); const existingIndex = await this.readExistingIndex(); - const localSkills = await this.readConfiguredLocalRegistrySkills(registryIds); + const localSkills = await this.readConfiguredLocalRegistrySkills(registry.registries); ui.info(`Building skill index from ${registryIds.length} registries...`); @@ -163,6 +174,9 @@ export class SkillIndex { const batchResults = await Promise.allSettled( batch.map(async (registryId) => { const gitUrl = registry.registries[registryId]; + if (parseLocalRegistryPath(gitUrl) !== null) { + return { registryId, error: 'local registry' }; + } const match = gitUrl.match(/github\.com\/([^/]+)\/([^/.]+)/); if (!match) return { registryId, error: 'not a GitHub URL' }; @@ -267,11 +281,32 @@ export class SkillIndex { return null; } - private async readConfiguredLocalRegistrySkills(registryIds: string[]): Promise { + private async refreshLocalRegistryEntries(index: SkillIndexData): Promise { + const registry = await this.registry.fetchMergedRegistry(); + const localIds = Object.entries(registry.registries) + .filter(([, value]) => parseLocalRegistryPath(value) !== null) + .map(([id]) => id); + if (localIds.length === 0) return index; + const localSkills = await this.readConfiguredLocalRegistrySkills(registry.registries); + const next = { + ...index, + meta: { ...index.meta, updatedAt: Date.now() }, + skills: [...index.skills.filter(skill => !localIds.includes(skill.registry)), ...localSkills], + }; + await fs.writeJson(SKILL_INDEX_PATH, next, { spaces: 2 }); + return next; + } + + private async readConfiguredLocalRegistrySkills(registries: Record): Promise { const skills: SkillEntry[] = []; - for (const registryId of registryIds) { - const registrySkills = await this.readLocalRegistrySkills(registryId); + for (const [registryId, value] of Object.entries(registries)) { + const registrySkills = parseLocalRegistryPath(value) !== null + ? await this.readLocalRegistrySkills( + registryId, + await this.registry.prepareRegistryRepository(registryId, value), + ) + : await this.readLocalRegistrySkills(registryId); if (registrySkills) { skills.push(...registrySkills); } @@ -280,8 +315,18 @@ export class SkillIndex { return skills; } - private async readLocalRegistrySkills(registryId: string): Promise { - const registryPath = path.join(SKILL_CACHE_DIR, registryId); + private async readLocalRegistrySkills(registryId: string, sourcePath?: string): Promise { + const registryPath = sourcePath || path.join(SKILL_CACHE_DIR, registryId); + if (sourcePath) { + const discovered = await discoverRegistrySkills(registryId, sourcePath); + return discovered.map(skill => ({ + name: skill.name, + registry: registryId, + path: path.join('skills', skill.name).split(path.sep).join('/'), + description: skill.description, + lastIndexed: Date.now(), + })); + } const skillsPath = path.join(registryPath, 'skills'); if (!await fs.pathExists(registryPath) || !await fs.pathExists(skillsPath)) { diff --git a/packages/cli/src/lib/SkillManager.ts b/packages/cli/src/lib/SkillManager.ts index 2b86316b..02227565 100644 --- a/packages/cli/src/lib/SkillManager.ts +++ b/packages/cli/src/lib/SkillManager.ts @@ -7,8 +7,9 @@ import { EnvironmentSelector } from './EnvironmentSelector.js'; import { SkillRegistry, SKILL_CACHE_DIR } from './SkillRegistry.js'; import { SkillIndex } from './SkillIndex.js'; import { getAllEnvironments, getGlobalSkillPath, getSkillCapableEnvironments, getSkillPath, validateEnvironmentCodes } from '../util/env.js'; -import { ensureGitInstalled } from '../util/git.js'; import { validateRegistryId, validateSkillName, extractSkillDescription, isValidSkillName } from '../util/skill.js'; +import { parseLocalRegistryPath } from '../util/skill-registry.js'; +import { discoverRegistrySkills, resolveContainedSkill } from '../util/local-registry.js'; import { isInteractiveTerminal } from '../util/terminal.js'; import { ui } from '../util/terminal-ui.js'; import { ConfigNotFoundError, NotFoundError, ValidationError } from '../util/errors.js'; @@ -75,8 +76,6 @@ export class SkillManager { ): Promise<'installed' | 'matched'> { ui.info(`Validating registry: ${registryId}`); validateRegistryId(registryId); - await ensureGitInstalled(); - const spinner = ui.spinner('Fetching registries...'); spinner.start(); const registry = await this.registry.fetchMergedRegistry(); @@ -91,17 +90,18 @@ export class SkillManager { } const repoPath = await this.registry.prepareRegistryRepository(registryId, gitUrl); + const isLocal = Boolean(gitUrl && parseLocalRegistryPath(gitUrl) !== null); const resolvedSkillNames = skillName ? [skillName] - : await this.resolveSkillNamesFromRegistry(registryId, repoPath); + : await this.resolveSkillNamesFromRegistry(registryId, repoPath, isLocal); const selectedEnvironments = await this.resolveInstallEnvironments(options); const installContext = this.buildInstallContext(selectedEnvironments, options); let status: 'installed' | 'matched' = 'matched'; for (const resolvedSkillName of resolvedSkillNames) { const itemStatus = await this.installResolvedSkill( - registryId, repoPath, resolvedSkillName, options, installContext + registryId, repoPath, resolvedSkillName, options, installContext, isLocal ); if (itemStatus === 'installed') { status = 'installed'; @@ -148,7 +148,11 @@ export class SkillManager { const realPath = await fs.realpath(skillPath); const cacheRelative = path.relative(SKILL_CACHE_DIR, realPath); const parts = cacheRelative.split(path.sep); - if (parts.length >= 2) { + const insideCache = cacheRelative + && cacheRelative !== '..' + && !cacheRelative.startsWith(`..${path.sep}`) + && !path.isAbsolute(cacheRelative); + if (insideCache && parts.length >= 2) { registry = `${parts[0]}/${parts[1]}`; } } catch { @@ -354,9 +358,8 @@ export class SkillManager { return this.registry.updateSkills(registryId); } - async cacheRegistry(registryId: string, gitUrl: string): Promise { - await ensureGitInstalled(); - return this.registry.prepareRegistryRepository(registryId, gitUrl); + async cacheRegistry(registryId: string, source: string): Promise { + return this.registry.prepareRegistryRepository(registryId, source); } /** @@ -373,8 +376,12 @@ export class SkillManager { return this.index.rebuildIndex(outputPath); } - async updateSkillIndexForRegistry(registryId: string): Promise { - return this.index.updateRegistryFromCache(registryId); + async updateSkillIndexForRegistry(registryId: string, registryPath: string): Promise { + return this.index.updateRegistryFromCache(registryId, registryPath); + } + + async removeSkillIndexForRegistry(registryId: string): Promise { + return this.index.removeRegistry(registryId); } /** @@ -473,12 +480,15 @@ export class SkillManager { repoPath: string, resolvedSkillName: string, options: AddSkillOptions, - installContext: ResolvedInstallContext + installContext: ResolvedInstallContext, + isLocal: boolean, ): Promise<'installed' | 'matched'> { ui.info(`Validating skill: ${resolvedSkillName} from ${registryId}`); validateSkillName(resolvedSkillName); - const skillPath = await this.resolveInstallableSkillPath(repoPath, registryId, resolvedSkillName); + const skillPath = isLocal + ? await resolveContainedSkill(registryId, repoPath, resolvedSkillName) + : await this.resolveInstallableSkillPath(repoPath, registryId, resolvedSkillName); ui.info(`Installing skill to ${installContext.installMode}...`); let installed = false; @@ -553,12 +563,17 @@ export class SkillManager { return skillPath; } - private async resolveSkillNamesFromRegistry(registryId: string, repoPath: string): Promise { + private async resolveSkillNamesFromRegistry(registryId: string, repoPath: string, isLocal: boolean): Promise { if (!isInteractiveTerminal()) { throw new ValidationError('Skill name is required in non-interactive mode. Re-run with: ai-devkit skill add '); } - const skills = await this.listRegistrySkills(registryId, repoPath); + const skills = isLocal + ? (await discoverRegistrySkills(registryId, repoPath)).map(skill => ({ + name: skill.name, + description: skill.description, + })) + : await this.listRegistrySkills(registryId, repoPath); return this.promptForSkillSelection(skills); } diff --git a/packages/cli/src/lib/SkillRegistry.ts b/packages/cli/src/lib/SkillRegistry.ts index 26a9a07e..b8e8c784 100644 --- a/packages/cli/src/lib/SkillRegistry.ts +++ b/packages/cli/src/lib/SkillRegistry.ts @@ -7,6 +7,9 @@ import { ensureGitInstalled, cloneRepository, isGitRepository, pullRepository } import { ui } from '../util/terminal-ui.js'; import { getErrorMessage } from '../util/text.js'; import { CliError, NotFoundError } from '../util/errors.js'; +import { parseLocalRegistryPath } from '../util/skill-registry.js'; +import { isValidSkillName } from '../util/skill.js'; +import { LOCAL_REGISTRY_MAX_ENTRIES } from '../util/local-registry.js'; export const REGISTRY_URL = 'https://raw.githubusercontent.com/codeaholicguy/ai-devkit/main/skills/registry.json'; export const SKILL_CACHE_DIR = path.join(os.homedir(), '.ai-devkit', 'skills'); @@ -113,11 +116,72 @@ export class SkillRegistry { return preparedRepository; } - const preparation = this.refreshOrUseStaleCache(registryId, gitUrl); + const preparation = gitUrl && parseLocalRegistryPath(gitUrl) !== null + ? this.prepareLocalRegistry(registryId, gitUrl) + : this.prepareGitRegistry(registryId, gitUrl); this.preparedRepositories.set(registryId, preparation); return preparation; } + private async prepareGitRegistry(registryId: string, gitUrl?: string): Promise { + await ensureGitInstalled(); + return this.refreshOrUseStaleCache(registryId, gitUrl); + } + + private async prepareLocalRegistry(registryId: string, value: string): Promise { + const localPath = parseLocalRegistryPath(value); + if (localPath === null) { + throw new CliError(`Registry "${registryId}" is not a local source.`, 'INVALID_LOCAL_REGISTRY'); + } + + let root: string; + try { + root = await fs.realpath(localPath); + const stat = await fs.stat(root); + if (!stat.isDirectory()) throw new Error('source is not a directory'); + } catch (error: unknown) { + throw new NotFoundError( + `Local registry "${registryId}" is unavailable at ${localPath}: ${getErrorMessage(error)}. Recreate it or re-register the source.`, + { registryId, path: localPath }, + ); + } + + const skillsPath = path.join(root, 'skills'); + if (!await fs.pathExists(skillsPath)) { + throw new NotFoundError( + `Local registry "${registryId}" has no skills directory: ${skillsPath}`, + { registryId, path: skillsPath }, + ); + } + const directory = await fs.opendir(skillsPath); + let count = 0; + let hasSkill = false; + for await (const entry of directory) { + count += 1; + if (count > LOCAL_REGISTRY_MAX_ENTRIES) { + throw new CliError( + `Local registry "${registryId}" exceeds the ${LOCAL_REGISTRY_MAX_ENTRIES} entry limit.`, + 'LOCAL_REGISTRY_TOO_LARGE', + ); + } + if ((entry.isDirectory() || entry.isSymbolicLink()) + && isValidSkillName(entry.name) + && await fs.pathExists(path.join(skillsPath, entry.name, 'SKILL.md'))) { + hasSkill = true; + break; + } + } + if (!hasSkill) { + throw new NotFoundError( + `No valid skills found in local registry "${registryId}". Expected skills//SKILL.md.`, + { registryId, path: skillsPath }, + ); + } + + ui.info(`Using local registry ${registryId}: ${root}`); + return root; + } + private async refreshOrUseStaleCache(registryId: string, gitUrl?: string): Promise { const cachedPath = path.join(SKILL_CACHE_DIR, registryId); ui.info(`Refreshing registry ${registryId}...`); @@ -142,12 +206,33 @@ export class SkillRegistry { : 'Updating all skills...' ); - await ensureGitInstalled(); - const cacheDir = SKILL_CACHE_DIR; + const configured = await this.fetchMergedRegistry(); + const localEntries = Object.entries(configured.registries) + .filter(([id, value]) => (!registryId || id === registryId) && parseLocalRegistryPath(value) !== null); + const configuredLocalIds = new Set(Object.entries(configured.registries) + .filter(([, value]) => parseLocalRegistryPath(value) !== null) + .map(([id]) => id)); + + const results: UpdateResult[] = []; + for (const [id, value] of localEntries) { + await this.prepareRegistryRepository(id, value); + results.push({ + registryId: id, + status: 'skipped', + message: 'Local registry uses the live filesystem; nothing to update', + }); + ui.warning(`${id} skipped (Local registry uses the live filesystem; nothing to update)`); + } + if (!await fs.pathExists(cacheDir)) { + if (registryId && localEntries.length === 0) { + throw new NotFoundError(`Registry "${registryId}" not found.`, { registryId }); + } ui.warning('No skills cache found. Nothing to update.'); - return { total: 0, successful: 0, skipped: 0, failed: 0, results: [] }; + const summary = this.summarize(results); + this.displayUpdateSummary(summary); + return summary; } const entries = await fs.readdir(cacheDir, { withFileTypes: true }); @@ -162,7 +247,8 @@ export class SkillRegistry { if (repo.isDirectory()) { const fullRegistryId = `${entry.name}/${repo.name}`; - if (!registryId || fullRegistryId === registryId) { + if (!configuredLocalIds.has(fullRegistryId) + && (!registryId || fullRegistryId === registryId)) { registries.push({ path: path.join(ownerPath, repo.name), id: fullRegistryId, @@ -173,12 +259,10 @@ export class SkillRegistry { } } - if (registryId && registries.length === 0) { + if (registryId && registries.length === 0 && localEntries.length === 0) { throw new NotFoundError(`Registry "${registryId}" not found in cache.`, { registryId }); } - const results: UpdateResult[] = []; - for (const registry of registries) { ui.info(`Updating ${registry.id}...`); const result = await this.updateRegistry(registry.path, registry.id); @@ -192,19 +276,24 @@ export class SkillRegistry { } } - const summary: UpdateSummary = { + const summary = this.summarize(results); + this.displayUpdateSummary(summary); + + return summary; + } + + private summarize(results: UpdateResult[]): UpdateSummary { + return { total: results.length, successful: results.filter(r => r.status === 'success').length, skipped: results.filter(r => r.status === 'skipped').length, failed: results.filter(r => r.status === 'error').length, results, }; - this.displayUpdateSummary(summary); - - return summary; } private async updateRegistry(registryPath: string, registryId: string): Promise { + await ensureGitInstalled(); const isGit = await isGitRepository(registryPath); if (!isGit) { diff --git a/packages/cli/src/services/status/status.service.ts b/packages/cli/src/services/status/status.service.ts index fc3b7d11..2c25e59c 100644 --- a/packages/cli/src/services/status/status.service.ts +++ b/packages/cli/src/services/status/status.service.ts @@ -203,6 +203,9 @@ function safeRegistries(raw: unknown): Record { return Object.fromEntries(Object.entries(filterStringRecord(raw)).map(([id, value]) => { try { const url = new URL(value); + if (url.protocol === 'file:' && (!url.hostname || url.hostname === 'localhost')) { + return [id, `local: ${decodeURIComponent(url.pathname)}`]; + } url.username = ''; url.password = ''; url.search = ''; diff --git a/packages/cli/src/util/local-registry.ts b/packages/cli/src/util/local-registry.ts new file mode 100644 index 00000000..e2323d6b --- /dev/null +++ b/packages/cli/src/util/local-registry.ts @@ -0,0 +1,94 @@ +import fs from 'fs-extra'; +import path from 'node:path'; +import { CliError, NotFoundError } from './errors.js'; +import { extractSkillDescription, isValidSkillName } from './skill.js'; + +export const LOCAL_REGISTRY_MAX_ENTRIES = 10_000; +export const LOCAL_REGISTRY_MAX_SKILL_MD_BYTES = 1024 * 1024; + +interface DiscoveredRegistrySkill { + name: string; + description: string; +} + +function isStrictlyContained(root: string, candidate: string): boolean { + const relative = path.relative(root, candidate); + return Boolean(relative) + && relative !== '..' + && !relative.startsWith(`..${path.sep}`) + && !path.isAbsolute(relative); +} + +export async function resolveContainedSkill( + registryId: string, + registryRoot: string, + skillName: string, +): Promise { + const canonicalRoot = await fs.realpath(registryRoot); + const skillsRoot = await fs.realpath(path.join(canonicalRoot, 'skills')); + const skillPath = path.join(skillsRoot, skillName); + let canonicalSkill: string; + let canonicalMetadata: string; + try { + canonicalSkill = await fs.realpath(skillPath); + canonicalMetadata = await fs.realpath(path.join(skillPath, 'SKILL.md')); + } catch { + throw new NotFoundError( + `Skill "${skillName}" or its SKILL.md was not found in ${registryId}.`, + { registryId, skillName }, + ); + } + if (!isStrictlyContained(skillsRoot, canonicalSkill) + || !isStrictlyContained(skillsRoot, canonicalMetadata) + || !isStrictlyContained(canonicalSkill, canonicalMetadata)) { + throw new CliError( + `Skill "${skillName}" resolves outside local registry "${registryId}"; refusing to use it.`, + 'LOCAL_REGISTRY_ESCAPE', + { registryId, skillName }, + ); + } + const metadataSize = (await fs.stat(canonicalMetadata)).size; + if (metadataSize > LOCAL_REGISTRY_MAX_SKILL_MD_BYTES) { + throw new CliError( + `SKILL.md for "${skillName}" is too large (${metadataSize} bytes; limit ${LOCAL_REGISTRY_MAX_SKILL_MD_BYTES}).`, + 'LOCAL_REGISTRY_TOO_LARGE', + ); + } + return canonicalSkill; +} + +export async function discoverRegistrySkills( + registryId: string, + registryRoot: string, +): Promise { + const canonicalRoot = await fs.realpath(registryRoot); + const skillsRoot = await fs.realpath(path.join(canonicalRoot, 'skills')); + if (!isStrictlyContained(canonicalRoot, skillsRoot)) { + throw new CliError( + `Skills directory resolves outside local registry "${registryId}".`, + 'LOCAL_REGISTRY_ESCAPE', + ); + } + + const directory = await fs.opendir(skillsRoot); + const skills: DiscoveredRegistrySkill[] = []; + let entries = 0; + for await (const entry of directory) { + entries += 1; + if (entries > LOCAL_REGISTRY_MAX_ENTRIES) { + throw new CliError( + `Local registry "${registryId}" exceeds the ${LOCAL_REGISTRY_MAX_ENTRIES} entry limit.`, + 'LOCAL_REGISTRY_TOO_LARGE', + ); + } + if ((!entry.isDirectory() && !entry.isSymbolicLink()) || !isValidSkillName(entry.name)) continue; + const skillPath = await resolveContainedSkill(registryId, canonicalRoot, entry.name); + const metadataPath = path.join(skillPath, 'SKILL.md'); + const content = await fs.readFile(metadataPath, 'utf8'); + skills.push({ + name: entry.name, + description: extractSkillDescription(content), + }); + } + return skills; +} diff --git a/packages/cli/src/util/skill-registry.ts b/packages/cli/src/util/skill-registry.ts index a8331b11..b53ad381 100644 --- a/packages/cli/src/util/skill-registry.ts +++ b/packages/cli/src/util/skill-registry.ts @@ -1,4 +1,91 @@ import { CliError } from './errors.js'; +import fs from 'fs-extra'; +import path from 'node:path'; +import { fileURLToPath, pathToFileURL } from 'node:url'; + +export function parseLocalRegistryPath(value: string): string | null { + if (!value.toLowerCase().startsWith('file:')) { + return null; + } + + try { + const url = new URL(value); + if (url.protocol !== 'file:') { + throw new Error('invalid protocol'); + } + if (url.hostname && url.hostname !== 'localhost') { + throw new Error('file URL hosts are not supported'); + } + const localPath = fileURLToPath(url); + if (!path.isAbsolute(localPath)) { + throw new Error('path must be absolute'); + } + return localPath; + } catch (error: unknown) { + throw new CliError( + `Invalid local registry source "${value}": ${error instanceof Error ? error.message : String(error)}`, + 'INVALID_LOCAL_REGISTRY', + { value }, + ); + } +} + +function isPathShorthand(value: string): boolean { + return path.isAbsolute(value) || /^\.\.?[\\/]/.test(value); +} + +export async function normalizeRegistrySourceInput(value: string, baseDir: string): Promise { + const localPath = parseLocalRegistryPath(value); + if (localPath === null && !isPathShorthand(value)) { + return value; + } + + const requestedPath = localPath ?? path.resolve(baseDir, value); + let canonicalPath: string; + try { + canonicalPath = await fs.realpath(requestedPath); + } catch { + throw new CliError( + `Local registry source not found: ${requestedPath}`, + 'LOCAL_REGISTRY_NOT_FOUND', + { path: requestedPath }, + ); + } + const stat = await fs.stat(canonicalPath); + if (!stat.isDirectory()) { + throw new CliError( + `Local registry source is not a directory: ${canonicalPath}`, + 'INVALID_LOCAL_REGISTRY', + { path: canonicalPath }, + ); + } + return pathToFileURL(canonicalPath).href; +} + +export async function normalizeRegistrySources( + registries: Record, + baseDir: string, +): Promise> { + const normalized: Record = {}; + const localOwners = new Map(); + for (const [id, value] of Object.entries(registries)) { + const nextValue = await normalizeRegistrySourceInput(value, baseDir); + const localPath = parseLocalRegistryPath(nextValue); + if (localPath !== null) { + const existingId = localOwners.get(localPath); + if (existingId && existingId !== id) { + throw new CliError( + `Local folder is already registered as "${existingId}": ${localPath}`, + 'REGISTRY_SOURCE_CONFLICT', + { id, existingId, path: localPath }, + ); + } + localOwners.set(localPath, id); + } + normalized[id] = nextValue; + } + return normalized; +} export interface AddSkillRegistryOptions { force?: boolean; diff --git a/web/content/docs/11-configuration-file.md b/web/content/docs/11-configuration-file.md index a5c55cfb..b44bca44 100644 --- a/web/content/docs/11-configuration-file.md +++ b/web/content/docs/11-configuration-file.md @@ -35,7 +35,8 @@ Use this page as a reference for fields inside `.ai-devkit.json`. In most cases, "path": ".ai-devkit/memory.db" }, "registries": { - "codeaholicguy/ai-devkit": "https://github.com/codeaholicguy/ai-devkit.git" + "codeaholicguy/ai-devkit": "https://github.com/codeaholicguy/ai-devkit.git", + "my-org/local-skills": "file:///absolute/path/to/local-registry" }, "skills": [ { "registry": "codeaholicguy/ai-devkit", "name": "structured-debug" }, diff --git a/web/content/docs/7-skills.md b/web/content/docs/7-skills.md index 978fb8c5..b1a30bb1 100644 --- a/web/content/docs/7-skills.md +++ b/web/content/docs/7-skills.md @@ -187,9 +187,12 @@ Register a third-party skill registry in the current project or global configura ```bash ai-devkit skill add-registry my-org/skills https://github.com/my-org/agent-skills.git ai-devkit skill add-registry my-org/skills https://github.com/my-org/agent-skills.git --global +ai-devkit skill add-registry my-org/local-skills ../local-registry ``` -Use `--global` to write the registry to `~/.ai-devkit/.ai-devkit.json`. If the same registry ID already points to another URL, use `--force` to replace it: +Local sources accept absolute paths, `./`, `../`, or explicit `file:` URLs. AI DevKit resolves them when registered, stores a canonical absolute `file:` URL, and reads the folder in place without cloning, pulling, writing, or deleting it. If the folder moves, re-add it with `--force`. + +Use `--global` to write the registry to `~/.ai-devkit/.ai-devkit.json`. If the same registry ID already points to another source, use `--force` to replace it: ```bash ai-devkit skill add-registry my-org/skills https://github.com/my-org/new-skills.git --force @@ -206,9 +209,7 @@ ai-devkit skill remove-registry my-org/skills ai-devkit skill remove-registry my-org/skills --global ``` -Without `--global`, the command removes only the project configuration entry and keeps the cached repository. With `--global`, it removes the global configuration entry and recursively deletes that registry's cache directory under `~/.ai-devkit/skills/`. Registry IDs are validated and the resolved cache path must remain inside the skills cache root before deletion. - -The local discovery index is not modified. It is seeded with skills from registries that are not configured locally, so entries for a removed registry remain valid catalog entries. Default registries are structurally protected because they do not live in project or global configuration maps; the built-in `codeaholicguy/ai-devkit` registry is explicitly protected. +Removal also deletes that registry's focused search-index entries. With `--global`, it may delete only the registry's ID-derived cache directory under `~/.ai-devkit/skills/`. A configured local source folder is never a deletion target. Default registries are structurally protected; the built-in `codeaholicguy/ai-devkit` registry is explicitly protected. ### `ai-devkit skill list`