From 7f616cbf29599c3fb21f94ebaf53a24641e1794e Mon Sep 17 00:00:00 2001 From: Hoang Nguyen Date: Mon, 7 Sep 2026 12:41:22 +0000 Subject: [PATCH 1/6] docs: define local registry lifecycle --- .../2026-09-07-feature-local-registry.md | 76 +++++++++++++++++++ .../2026-09-07-feature-local-registry.md | 39 ++++++++++ .../2026-09-07-feature-local-registry.md | 45 +++++++++++ .../2026-09-07-feature-local-registry.md | 72 ++++++++++++++++++ .../2026-09-07-feature-local-registry.md | 56 ++++++++++++++ 5 files changed, 288 insertions(+) create mode 100644 docs/ai/design/2026-09-07-feature-local-registry.md create mode 100644 docs/ai/implementation/2026-09-07-feature-local-registry.md create mode 100644 docs/ai/planning/2026-09-07-feature-local-registry.md create mode 100644 docs/ai/requirements/2026-09-07-feature-local-registry.md create mode 100644 docs/ai/testing/2026-09-07-feature-local-registry.md 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..5154b270 --- /dev/null +++ b/docs/ai/design/2026-09-07-feature-local-registry.md @@ -0,0 +1,76 @@ +--- +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 small parsed union centralizes type detection; SkillRegistry branches preparation/update and consumers receive the actual prepared root. + +## Data Model + + type RegistrySource = + | { type: 'git'; value: string } + | { type: 'local'; value: string; path: string }; + +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 injectable test limits and no user-facing flag. + +## 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..f9b7a029 --- /dev/null +++ b/docs/ai/implementation/2026-09-07-feature-local-registry.md @@ -0,0 +1,39 @@ +--- +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 + +Pending. Each task records red/green/refactor evidence here. + +## 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 + +None. 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..64de06d3 --- /dev/null +++ b/docs/ai/planning/2026-09-07-feature-local-registry.md @@ -0,0 +1,45 @@ +--- +phase: planning +title: Local Folder Skill Registry Plan +description: TDD plan for explicit read-only local sources +--- + +# Plan + +## Milestones and Tasks + +- [ ] **M1 Source boundary** + - [ ] T1: TDD source parser and explicit file:/Git classification (AC-01, SR-06, SR-12). + - [ ] T2: TDD cwd/config/template normalization and canonical duplicate rejection (AC-02–05). + - [ ] T3: Document canonical storage and move/re-add semantics. +- [ ] **M2 Runtime flows** + - [ ] T4: TDD read-only local preparation, no Git calls, and prep-once compatibility (AC-06, SR-01–05). + - [ ] T5: TDD fixture discovery/install, missing/empty errors, containment, direct-only and bounded reads (AC-07, SR-07–10). + - [ ] T6: TDD focused/full/seed/TTL index behavior using actual source roots (AC-08). + - [ ] T7: TDD selected/local-only/mixed update behavior (AC-09). +- [ ] **M3 Removal and surfaces** + - [ ] T8: TDD config/index cleanup and ID-derived cache-only deletion; snapshot local fixtures (AC-10, SR-01–04). + - [ ] T9: TDD status and installed provenance (AC-11–12, SR-11). + - [ ] T10: CLI e2e normalization/error/removal journeys and user docs. + - [ ] T11: Reconcile docs; implementation check, coverage, build, tests, lint, e2e, final review. + +## 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 + +Planning approved; implementation begins at T1 and this checklist is reconciled after completed tasks. 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..059b091b --- /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; tests use injectable low limits. + +## 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..dc3ee658 --- /dev/null +++ b/docs/ai/testing/2026-09-07-feature-local-registry.md @@ -0,0 +1,56 @@ +--- +phase: testing +title: Local Folder Skill Registry Testing +description: Safety-first testing strategy +--- + +# Testing + +## Goals + +100% new parser/containment coverage, unit coverage for SR-01–12, temp-directory adapter coverage, CLI e2e path/error coverage, and green Git registry regressions. + +## Source and Config + +- [ ] Absolute, ./, and ../ normalize to canonical file URLs against registration cwd. (AC-01–02) +- [ ] Template/config relative paths use their containing file. (AC-03) +- [ ] Root symlinks and trailing slashes deduplicate; different IDs are rejected. (AC-04) +- [ ] Git URL/SCP values remain Git; malformed/hosted file values fail locally. (AC-01, AC-05, SR-06, SR-12) + +## Preparation and Install + +- [ ] Local preparation returns the root with no Git/write call and memoizes once. (AC-06, SR-01–02) +- [ ] Missing/moved, non-directory, missing-skills, and empty roots error clearly without cache fallback. (AC-05, SR-05) +- [ ] Valid temp fixture installs without modifying source. (AC-07, SR-01) +- [ ] Escaping symlinks are rejected before read/install. (SR-07) +- [ ] Nested skills are ignored; candidate/metadata limits fail clearly. (SR-08–09) +- [ ] Fixture skill content is never executed. (SR-10) + +## Index and Update + +- [ ] Focused/full indexing uses local roots; seed/TTL paths refresh local entries. (AC-08) +- [ ] Selected/local-only/mixed updates report live local sources and make no Git call for them. (AC-09, SR-01–02) +- [ ] Missing local update errors without fallback. (SR-05) + +## Removal and Display + +- [ ] Project/global removal deletes config/index but not source. (AC-10, SR-01, SR-03) +- [ ] Global removal deletes only contained ID-derived cache data. (SR-04) +- [ ] Status displays local and redacts Git credentials. (AC-11, SR-11) +- [ ] 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 + +- [ ] Focused unit/coverage +- [ ] npm run build +- [ ] npm test +- [ ] npm run lint +- [ ] npm run test:e2e + +## Results + +Pending. From c76b4e1517f2b197688263666ae0e72a9f5b79a5 Mon Sep 17 00:00:00 2001 From: Hoang Nguyen Date: Mon, 7 Sep 2026 13:02:41 +0000 Subject: [PATCH 2/6] feat(cli): add local folder skill registries --- .../2026-09-07-feature-local-registry.md | 11 +- .../2026-09-07-feature-local-registry.md | 28 ++--- .../2026-09-07-feature-local-registry.md | 46 +++---- e2e/cli.e2e.ts | 35 ++++++ .../cli/src/__tests__/commands/skill.test.ts | 7 +- .../src/__tests__/lib/SkillManager.test.ts | 4 +- .../src/__tests__/lib/SkillRegistry.test.ts | 71 +++++++++++ .../services/status/status.service.test.ts | 2 + .../src/__tests__/util/local-registry.test.ts | 41 +++++++ .../src/__tests__/util/skill-registry.test.ts | 75 +++++++++++- packages/cli/src/commands/skill.ts | 27 +++-- packages/cli/src/lib/Config.ts | 4 +- packages/cli/src/lib/GlobalConfig.ts | 4 +- packages/cli/src/lib/InitTemplate.ts | 7 +- packages/cli/src/lib/SkillIndex.ts | 66 ++++++++-- packages/cli/src/lib/SkillManager.ts | 45 ++++--- packages/cli/src/lib/SkillRegistry.ts | 113 ++++++++++++++++-- .../cli/src/services/status/status.service.ts | 3 + packages/cli/src/util/local-registry.ts | 109 +++++++++++++++++ packages/cli/src/util/skill-registry.ts | 113 ++++++++++++++++++ web/content/docs/11-configuration-file.md | 3 +- web/content/docs/7-skills.md | 9 +- 22 files changed, 723 insertions(+), 100 deletions(-) create mode 100644 packages/cli/src/__tests__/util/local-registry.test.ts create mode 100644 packages/cli/src/util/local-registry.ts diff --git a/docs/ai/implementation/2026-09-07-feature-local-registry.md b/docs/ai/implementation/2026-09-07-feature-local-registry.md index f9b7a029..2790c698 100644 --- a/docs/ai/implementation/2026-09-07-feature-local-registry.md +++ b/docs/ai/implementation/2026-09-07-feature-local-registry.md @@ -24,7 +24,14 @@ description: Running implementation record ## Implementation Log -Pending. Each task records red/green/refactor evidence here. +- 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 @@ -36,4 +43,4 @@ Pending. Each task records red/green/refactor evidence here. ## Deviations -None. +None. Limits are 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. diff --git a/docs/ai/planning/2026-09-07-feature-local-registry.md b/docs/ai/planning/2026-09-07-feature-local-registry.md index 64de06d3..2af15646 100644 --- a/docs/ai/planning/2026-09-07-feature-local-registry.md +++ b/docs/ai/planning/2026-09-07-feature-local-registry.md @@ -8,19 +8,19 @@ description: TDD plan for explicit read-only local sources ## Milestones and Tasks -- [ ] **M1 Source boundary** - - [ ] T1: TDD source parser and explicit file:/Git classification (AC-01, SR-06, SR-12). - - [ ] T2: TDD cwd/config/template normalization and canonical duplicate rejection (AC-02–05). - - [ ] T3: Document canonical storage and move/re-add semantics. -- [ ] **M2 Runtime flows** - - [ ] T4: TDD read-only local preparation, no Git calls, and prep-once compatibility (AC-06, SR-01–05). - - [ ] T5: TDD fixture discovery/install, missing/empty errors, containment, direct-only and bounded reads (AC-07, SR-07–10). - - [ ] T6: TDD focused/full/seed/TTL index behavior using actual source roots (AC-08). - - [ ] T7: TDD selected/local-only/mixed update behavior (AC-09). -- [ ] **M3 Removal and surfaces** - - [ ] T8: TDD config/index cleanup and ID-derived cache-only deletion; snapshot local fixtures (AC-10, SR-01–04). - - [ ] T9: TDD status and installed provenance (AC-11–12, SR-11). - - [ ] T10: CLI e2e normalization/error/removal journeys and user docs. +- [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. - [ ] T11: Reconcile docs; implementation check, coverage, build, tests, lint, e2e, final review. ## Dependencies @@ -42,4 +42,4 @@ Every production change follows focused red, green, refactor commands recorded i ## Progress -Planning approved; implementation begins at T1 and this checklist is reconciled after completed tasks. +T1–T10 are complete. T11 final verification and review remain; no scope changes or blockers were discovered. diff --git a/docs/ai/testing/2026-09-07-feature-local-registry.md b/docs/ai/testing/2026-09-07-feature-local-registry.md index dc3ee658..032f41bc 100644 --- a/docs/ai/testing/2026-09-07-feature-local-registry.md +++ b/docs/ai/testing/2026-09-07-feature-local-registry.md @@ -12,32 +12,32 @@ description: Safety-first testing strategy ## Source and Config -- [ ] Absolute, ./, and ../ normalize to canonical file URLs against registration cwd. (AC-01–02) -- [ ] Template/config relative paths use their containing file. (AC-03) -- [ ] Root symlinks and trailing slashes deduplicate; different IDs are rejected. (AC-04) -- [ ] Git URL/SCP values remain Git; malformed/hosted file values fail locally. (AC-01, AC-05, SR-06, SR-12) +- [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 -- [ ] Local preparation returns the root with no Git/write call and memoizes once. (AC-06, SR-01–02) -- [ ] Missing/moved, non-directory, missing-skills, and empty roots error clearly without cache fallback. (AC-05, SR-05) -- [ ] Valid temp fixture installs without modifying source. (AC-07, SR-01) -- [ ] Escaping symlinks are rejected before read/install. (SR-07) -- [ ] Nested skills are ignored; candidate/metadata limits fail clearly. (SR-08–09) -- [ ] Fixture skill content is never executed. (SR-10) +- [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 -- [ ] Focused/full indexing uses local roots; seed/TTL paths refresh local entries. (AC-08) -- [ ] Selected/local-only/mixed updates report live local sources and make no Git call for them. (AC-09, SR-01–02) -- [ ] Missing local update errors without fallback. (SR-05) +- [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 -- [ ] Project/global removal deletes config/index but not source. (AC-10, SR-01, SR-03) -- [ ] Global removal deletes only contained ID-derived cache data. (SR-04) -- [ ] Status displays local and redacts Git credentials. (AC-11, SR-11) -- [ ] Listing does not infer escaped cache-relative IDs. (AC-12) +- [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 @@ -45,12 +45,12 @@ Tests create isolated temp roots with skills/name/SKILL.md, snapshot source cont ## Required Validation -- [ ] Focused unit/coverage -- [ ] npm run build -- [ ] npm test -- [ ] npm run lint -- [ ] npm run test:e2e +- [x] Focused unit/coverage +- [x] npm run build +- [x] npm test +- [x] npm run lint +- [x] npm run test:e2e ## Results -Pending. +Focused suites passed 63 tests. Full workspace tests passed all six projects, including 1,139 CLI tests. The six-project build passed. Lint passed with two unrelated pre-existing warnings. E2E passed 42 tests. A partial coverage run passed selected tests but failed the global threshold because unselected files count as zero; full coverage remains in T11. 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..3150d044 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(), @@ -77,6 +79,7 @@ describe('skill command', () => { mockRemoveSkill.mockImplementation(async () => undefined); mockCacheRegistry.mockImplementation(async () => undefined); 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', undefined); 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..c2713782 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,71 @@ 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); + mockedFs.readdir + .mockResolvedValueOnce([{ name: 'example', isDirectory: () => true }] as Awaited>) + .mockResolvedValueOnce([{ name: 'skills', isDirectory: () => true }] as Awaited>); + 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.isGitRepository).not.toHaveBeenCalled(); + expect(mockedGit.cloneRepository).not.toHaveBeenCalled(); + expect(mockUi.info).toHaveBeenCalledWith(`Using local registry ${registryId}: ${localPath}`); + }); + + 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 + .mockResolvedValueOnce([{ name: 'example', isDirectory: () => true }] as Awaited>) + .mockResolvedValueOnce([{ name: 'skills', isDirectory: () => true }] as Awaited>); + 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..d9fe7f5a --- /dev/null +++ b/packages/cli/src/__tests__/util/local-registry.test.ts @@ -0,0 +1,41 @@ +import fs from 'fs-extra'; +import os from 'node:os'; +import path from 'node:path'; +import { + discoverRegistrySkills, + 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' }), + ]); + await expect(discoverRegistrySkills('test/skills', root, { maxEntries: 0, maxSkillMdBytes: 1024 })) + .rejects.toThrow(/entry limit/i); + await expect(discoverRegistrySkills('test/skills', root, { maxEntries: 10, maxSkillMdBytes: 1 })) + .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); + }); +}); diff --git a/packages/cli/src/__tests__/util/skill-registry.test.ts b/packages/cli/src/__tests__/util/skill-registry.test.ts index 76d2ba3e..37683fef 100644 --- a/packages/cli/src/__tests__/util/skill-registry.test.ts +++ b/packages/cli/src/__tests__/util/skill-registry.test.ts @@ -1,4 +1,77 @@ -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, + parseRegistrySource, + planSkillRegistryAdd, + planSkillRegistryRemove, +} from '../../util/skill-registry.js'; + +describe('registry sources', () => { + it('classifies only file URLs as persisted local sources', () => { + expect(parseRegistrySource('https://example.com/skills.git')).toEqual({ + type: 'git', value: 'https://example.com/skills.git', + }); + expect(parseRegistrySource('git@example.com:org/skills.git').type).toBe('git'); + expect(parseRegistrySource('file:///tmp/skills')).toEqual({ + type: 'local', value: 'file:///tmp/skills', path: '/tmp/skills', + }); + }); + + it('rejects malformed and hosted file URLs instead of treating them as Git', () => { + expect(() => parseRegistrySource('file://remote/share')).toThrow(/host/i); + expect(() => parseRegistrySource('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..f8093ec5 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 { assertUniqueLocalRegistrySource, 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') .action(withErrorHandler('add registry', async ( id: string, - url: string, + source: string, options: { global?: boolean; force?: boolean }, ) => { validateRegistryId(id); @@ -76,13 +76,20 @@ 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 normalized = await normalizeRegistrySources({ ...registries, [id]: source }, process.cwd()); + const value = normalized[id]; + const [projectRegistries, globalRegistries] = await Promise.all([ + options.global ? new ConfigManager().getSkillRegistries() : Promise.resolve(registries), + options.global ? Promise.resolve(registries) : new GlobalConfigManager().getSkillRegistries(), + ]); + assertUniqueLocalRegistrySource({ ...globalRegistries, ...projectRegistries }, id, value); + 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 +122,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 (parseRegistrySource(gitUrl).type === 'local') { + return { registryId, error: 'local registry' }; + } const match = gitUrl.match(/github\.com\/([^/]+)\/([^/.]+)/); if (!match) return { registryId, error: 'not a GitHub URL' }; @@ -267,11 +281,33 @@ 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]) => parseRegistrySource(value).type === 'local') + .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 source = parseRegistrySource(value); + const registrySkills = source.type === 'local' + ? await this.readLocalRegistrySkills( + registryId, + await this.registry.prepareRegistryRepository(registryId, value), + ) + : await this.readLocalRegistrySkills(registryId); if (registrySkills) { skills.push(...registrySkills); } @@ -280,8 +316,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..29b85931 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 { parseRegistrySource } 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 && parseRegistrySource(gitUrl).type === 'local'); 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..ce19f1c9 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 { parseRegistrySource } 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 && parseRegistrySource(gitUrl).type === 'local' + ? 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 source = parseRegistrySource(value); + if (source.type !== 'local') { + throw new CliError(`Registry "${registryId}" is not a local source.`, 'INVALID_LOCAL_REGISTRY'); + } + + let root: string; + try { + root = await fs.realpath(source.path); + 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 ${source.path}: ${getErrorMessage(error)}. Recreate it or re-register the source.`, + { registryId, path: source.path }, + ); + } + + 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) && parseRegistrySource(value).type === 'local'); + const configuredLocalIds = new Set(Object.entries(configured.registries) + .filter(([, value]) => parseRegistrySource(value).type === 'local') + .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..16a822d2 --- /dev/null +++ b/packages/cli/src/util/local-registry.ts @@ -0,0 +1,109 @@ +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; + +export interface RegistryDiscoveryLimits { + maxEntries: number; + maxSkillMdBytes: number; +} + +export interface DiscoveredRegistrySkill { + name: string; + path: string; + content: string; + description: string; +} + +const DEFAULT_LIMITS: RegistryDiscoveryLimits = { + maxEntries: LOCAL_REGISTRY_MAX_ENTRIES, + maxSkillMdBytes: LOCAL_REGISTRY_MAX_SKILL_MD_BYTES, +}; + +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 }, + ); + } + return canonicalSkill; +} + +export async function discoverRegistrySkills( + registryId: string, + registryRoot: string, + limits: RegistryDiscoveryLimits = DEFAULT_LIMITS, +): 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 > limits.maxEntries) { + throw new CliError( + `Local registry "${registryId}" exceeds the ${limits.maxEntries} 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 stat = await fs.stat(metadataPath); + if (stat.size > limits.maxSkillMdBytes) { + throw new CliError( + `SKILL.md for "${entry.name}" is too large (${stat.size} bytes; limit ${limits.maxSkillMdBytes}).`, + 'LOCAL_REGISTRY_TOO_LARGE', + ); + } + const content = await fs.readFile(metadataPath, 'utf8'); + skills.push({ + name: entry.name, + path: skillPath, + content, + 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..220d145b 100644 --- a/packages/cli/src/util/skill-registry.ts +++ b/packages/cli/src/util/skill-registry.ts @@ -1,4 +1,117 @@ import { CliError } from './errors.js'; +import fs from 'fs-extra'; +import path from 'node:path'; +import { fileURLToPath, pathToFileURL } from 'node:url'; + +export type RegistrySource = + | { type: 'git'; value: string } + | { type: 'local'; value: string; path: string }; + +export function parseRegistrySource(value: string): RegistrySource { + if (!value.toLowerCase().startsWith('file:')) { + return { type: 'git', value }; + } + + 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 { type: 'local', value, path: 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 parsed = parseRegistrySource(value); + if (parsed.type === 'git' && !isPathShorthand(value)) { + return value; + } + + const requestedPath = parsed.type === 'local' + ? parsed.path + : 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 source = parseRegistrySource(nextValue); + if (source.type === 'local') { + const existingId = localOwners.get(source.path); + if (existingId && existingId !== id) { + throw new CliError( + `Local folder is already registered as "${existingId}": ${source.path}`, + 'REGISTRY_SOURCE_CONFLICT', + { id, existingId, path: source.path }, + ); + } + localOwners.set(source.path, id); + } + normalized[id] = nextValue; + } + return normalized; +} + +export function assertUniqueLocalRegistrySource( + registries: Record, + id: string, + value: string, +): void { + const requested = parseRegistrySource(value); + if (requested.type !== 'local') return; + for (const [existingId, existingValue] of Object.entries(registries)) { + if (existingId === id) continue; + const existing = parseRegistrySource(existingValue); + if (existing.type === 'local' && existing.path === requested.path) { + throw new CliError( + `Local folder is already registered as "${existingId}": ${requested.path}`, + 'REGISTRY_SOURCE_CONFLICT', + { id, existingId, path: requested.path }, + ); + } + } +} 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` From 121c9b6cef84d48eec987814518a3c33cc1e56a0 Mon Sep 17 00:00:00 2001 From: Hoang Nguyen Date: Mon, 7 Sep 2026 13:04:31 +0000 Subject: [PATCH 3/6] fix(cli): bound explicit local skill metadata --- packages/cli/src/__tests__/util/local-registry.test.ts | 9 +++++++++ packages/cli/src/commands/skill.ts | 2 +- packages/cli/src/util/local-registry.ts | 7 +++++++ 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/__tests__/util/local-registry.test.ts b/packages/cli/src/__tests__/util/local-registry.test.ts index d9fe7f5a..99133f5e 100644 --- a/packages/cli/src/__tests__/util/local-registry.test.ts +++ b/packages/cli/src/__tests__/util/local-registry.test.ts @@ -3,6 +3,7 @@ import os from 'node:os'; import path from 'node:path'; import { discoverRegistrySkills, + LOCAL_REGISTRY_MAX_SKILL_MD_BYTES, resolveContainedSkill, } from '../../util/local-registry.js'; @@ -38,4 +39,12 @@ describe('local registry filesystem boundary', () => { 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/commands/skill.ts b/packages/cli/src/commands/skill.ts index f8093ec5..ec4942a3 100644 --- a/packages/cli/src/commands/skill.ts +++ b/packages/cli/src/commands/skill.ts @@ -64,7 +64,7 @@ export function registerSkillCommand(program: Command): void { .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, source: string, diff --git a/packages/cli/src/util/local-registry.ts b/packages/cli/src/util/local-registry.ts index 16a822d2..b1633b46 100644 --- a/packages/cli/src/util/local-registry.ts +++ b/packages/cli/src/util/local-registry.ts @@ -59,6 +59,13 @@ export async function resolveContainedSkill( { 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; } From cc9e989552496169436ce465fbef648158216627 Mon Sep 17 00:00:00 2001 From: Hoang Nguyen Date: Mon, 7 Sep 2026 13:07:35 +0000 Subject: [PATCH 4/6] docs: record local registry validation --- docs/ai/implementation/2026-09-07-feature-local-registry.md | 4 ++++ docs/ai/planning/2026-09-07-feature-local-registry.md | 4 ++-- docs/ai/testing/2026-09-07-feature-local-registry.md | 2 +- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/docs/ai/implementation/2026-09-07-feature-local-registry.md b/docs/ai/implementation/2026-09-07-feature-local-registry.md index 2790c698..be6c9fe0 100644 --- a/docs/ai/implementation/2026-09-07-feature-local-registry.md +++ b/docs/ai/implementation/2026-09-07-feature-local-registry.md @@ -44,3 +44,7 @@ Red evidence included missing parser functions and a missing local-registry modu ## Deviations None. Limits are 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 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. 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 index 2af15646..e10e97e9 100644 --- a/docs/ai/planning/2026-09-07-feature-local-registry.md +++ b/docs/ai/planning/2026-09-07-feature-local-registry.md @@ -21,7 +21,7 @@ description: TDD plan for explicit read-only local sources - [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. - - [ ] T11: Reconcile docs; implementation check, coverage, build, tests, lint, e2e, final review. + - [x] T11: Reconcile docs; implementation check, coverage, build, tests, lint, e2e, final review. ## Dependencies @@ -42,4 +42,4 @@ Every production change follows focused red, green, refactor commands recorded i ## Progress -T1–T10 are complete. T11 final verification and review remain; no scope changes or blockers were discovered. +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/testing/2026-09-07-feature-local-registry.md b/docs/ai/testing/2026-09-07-feature-local-registry.md index 032f41bc..353b7f33 100644 --- a/docs/ai/testing/2026-09-07-feature-local-registry.md +++ b/docs/ai/testing/2026-09-07-feature-local-registry.md @@ -53,4 +53,4 @@ Tests create isolated temp roots with skills/name/SKILL.md, snapshot source cont ## Results -Focused suites passed 63 tests. Full workspace tests passed all six projects, including 1,139 CLI tests. The six-project build passed. Lint passed with two unrelated pre-existing warnings. E2E passed 42 tests. A partial coverage run passed selected tests but failed the global threshold because unselected files count as zero; full coverage remains in T11. +Final evidence: npm run build built six projects; npm test passed 2,188 tests across six projects, including 1,140 CLI tests; npm run lint passed with zero errors and two unrelated existing warnings; npm run test:e2e passed 42 tests. Focused TDD suites passed after every change. 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. From 5d61f46d2d49ba747b3cec00f5bc04043b24c28a Mon Sep 17 00:00:00 2001 From: Hoang Nguyen Date: Mon, 7 Sep 2026 17:01:33 +0000 Subject: [PATCH 5/6] refactor(cli): simplify local registry boundaries Replace the unused registry-source union with a local-path parser, reuse normalization for duplicate checks, and remove test-only discovery limits and unused result fields. Keep canonicalization, containment, bounded reads, and read-only local registry behavior covered by focused safety tests. --- .../cli/src/__tests__/commands/skill.test.ts | 4 +- .../src/__tests__/lib/SkillRegistry.test.ts | 9 +--- .../src/__tests__/util/local-registry.test.ts | 26 ++++++++-- .../src/__tests__/util/skill-registry.test.ts | 16 +++--- packages/cli/src/commands/skill.ts | 7 ++- packages/cli/src/lib/SkillIndex.ts | 11 ++-- packages/cli/src/lib/SkillManager.ts | 6 +-- packages/cli/src/lib/SkillRegistry.ts | 18 +++---- packages/cli/src/util/local-registry.ts | 28 ++--------- packages/cli/src/util/skill-registry.ts | 50 +++++-------------- 10 files changed, 66 insertions(+), 109 deletions(-) diff --git a/packages/cli/src/__tests__/commands/skill.test.ts b/packages/cli/src/__tests__/commands/skill.test.ts index 3150d044..95ed4a2d 100644 --- a/packages/cli/src/__tests__/commands/skill.test.ts +++ b/packages/cli/src/__tests__/commands/skill.test.ts @@ -77,7 +77,7 @@ 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); @@ -161,7 +161,7 @@ describe('skill command', () => { 'example/private-skills', 'git@example.com:example/private-skills.git', ); - expect(mockUpdateSkillIndexForRegistry).toHaveBeenCalledWith('example/private-skills', undefined); + expect(mockUpdateSkillIndexForRegistry).toHaveBeenCalledWith('example/private-skills', '/tmp/registry-cache'); expect(mockCacheRegistry.mock.invocationCallOrder[0]).toBeLessThan( mockUpdateSkillIndexForRegistry.mock.invocationCallOrder[0], ); diff --git a/packages/cli/src/__tests__/lib/SkillRegistry.test.ts b/packages/cli/src/__tests__/lib/SkillRegistry.test.ts index c2713782..0772843e 100644 --- a/packages/cli/src/__tests__/lib/SkillRegistry.test.ts +++ b/packages/cli/src/__tests__/lib/SkillRegistry.test.ts @@ -184,9 +184,6 @@ describe('SkillRegistry repository preparation', () => { }, } as Awaited>); mockedFs.pathExists.mockResolvedValue(true); - mockedFs.readdir - .mockResolvedValueOnce([{ name: 'example', isDirectory: () => true }] as Awaited>) - .mockResolvedValueOnce([{ name: 'skills', isDirectory: () => true }] as Awaited>); const registry = createRegistry(); await expect(registry.prepareRegistryRepository(registryId, 'file:///tmp/local-skills')) @@ -199,9 +196,7 @@ describe('SkillRegistry repository preparation', () => { expect(mockedGit.ensureGitInstalled).not.toHaveBeenCalled(); expect(mockedGit.isGitRepository).not.toHaveBeenCalled(); expect(mockedGit.pullRepository).not.toHaveBeenCalled(); - expect(mockedGit.isGitRepository).not.toHaveBeenCalled(); expect(mockedGit.cloneRepository).not.toHaveBeenCalled(); - expect(mockUi.info).toHaveBeenCalledWith(`Using local registry ${registryId}: ${localPath}`); }); it('does not use a same-ID cache when a local registry is missing', async () => { @@ -224,9 +219,7 @@ describe('SkillRegistry repository preparation', () => { }, } as Awaited>); mockedFs.pathExists.mockResolvedValue(true); - mockedFs.readdir - .mockResolvedValueOnce([{ name: 'example', isDirectory: () => true }] as Awaited>) - .mockResolvedValueOnce([{ name: 'skills', isDirectory: () => true }] as Awaited>); + 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, diff --git a/packages/cli/src/__tests__/util/local-registry.test.ts b/packages/cli/src/__tests__/util/local-registry.test.ts index 99133f5e..246c1a47 100644 --- a/packages/cli/src/__tests__/util/local-registry.test.ts +++ b/packages/cli/src/__tests__/util/local-registry.test.ts @@ -3,6 +3,7 @@ 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'; @@ -22,10 +23,27 @@ describe('local registry filesystem boundary', () => { await expect(discoverRegistrySkills('test/skills', root)).resolves.toEqual([ expect.objectContaining({ name: 'safe-skill' }), ]); - await expect(discoverRegistrySkills('test/skills', root, { maxEntries: 0, maxSkillMdBytes: 1024 })) - .rejects.toThrow(/entry limit/i); - await expect(discoverRegistrySkills('test/skills', root, { maxEntries: 10, maxSkillMdBytes: 1 })) - .rejects.toThrow(/too large/i); + }); + + 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 () => { diff --git a/packages/cli/src/__tests__/util/skill-registry.test.ts b/packages/cli/src/__tests__/util/skill-registry.test.ts index 37683fef..cca8a485 100644 --- a/packages/cli/src/__tests__/util/skill-registry.test.ts +++ b/packages/cli/src/__tests__/util/skill-registry.test.ts @@ -5,25 +5,21 @@ import { pathToFileURL } from 'node:url'; import { normalizeRegistrySourceInput, normalizeRegistrySources, - parseRegistrySource, + parseLocalRegistryPath, planSkillRegistryAdd, planSkillRegistryRemove, } from '../../util/skill-registry.js'; describe('registry sources', () => { it('classifies only file URLs as persisted local sources', () => { - expect(parseRegistrySource('https://example.com/skills.git')).toEqual({ - type: 'git', value: 'https://example.com/skills.git', - }); - expect(parseRegistrySource('git@example.com:org/skills.git').type).toBe('git'); - expect(parseRegistrySource('file:///tmp/skills')).toEqual({ - type: 'local', value: 'file:///tmp/skills', path: '/tmp/skills', - }); + 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(() => parseRegistrySource('file://remote/share')).toThrow(/host/i); - expect(() => parseRegistrySource('file:%')).toThrow(/local registry/i); + 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 () => { diff --git a/packages/cli/src/commands/skill.ts b/packages/cli/src/commands/skill.ts index ec4942a3..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 { assertUniqueLocalRegistrySource, normalizeRegistrySources, planSkillRegistryAdd } from '../util/skill-registry.js'; +import { normalizeRegistrySourceInput, normalizeRegistrySources, planSkillRegistryAdd } from '../util/skill-registry.js'; export function registerSkillCommand(program: Command): void { const skillCommand = program @@ -76,13 +76,12 @@ export function registerSkillCommand(program: Command): void { : new ConfigManager(); const registries = await configManager.getSkillRegistries(); - const normalized = await normalizeRegistrySources({ ...registries, [id]: source }, process.cwd()); - const value = normalized[id]; + 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(), ]); - assertUniqueLocalRegistrySource({ ...globalRegistries, ...projectRegistries }, id, value); + 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()); diff --git a/packages/cli/src/lib/SkillIndex.ts b/packages/cli/src/lib/SkillIndex.ts index 7bb693c6..9b2f1ab6 100644 --- a/packages/cli/src/lib/SkillIndex.ts +++ b/packages/cli/src/lib/SkillIndex.ts @@ -7,7 +7,7 @@ import { fetchGitHead } from '../util/git.js'; import { fetchGitHubSkillPaths, fetchRawGitHubFile } from '../util/github.js'; import { ui } from '../util/terminal-ui.js'; import { getErrorMessage } from '../util/text.js'; -import { parseRegistrySource } from '../util/skill-registry.js'; +import { parseLocalRegistryPath } from '../util/skill-registry.js'; import { discoverRegistrySkills } from '../util/local-registry.js'; const SEED_INDEX_URL = 'https://raw.githubusercontent.com/codeaholicguy/ai-devkit/main/skills/index.json'; @@ -68,7 +68,7 @@ export class SkillIndex { } } - async updateRegistryFromCache(registryId: string, registryPath?: string): Promise { + async updateRegistryFromCache(registryId: string, registryPath: string): Promise { const localSkills = await this.readLocalRegistrySkills(registryId, registryPath); if (!localSkills) { return; @@ -174,7 +174,7 @@ export class SkillIndex { const batchResults = await Promise.allSettled( batch.map(async (registryId) => { const gitUrl = registry.registries[registryId]; - if (parseRegistrySource(gitUrl).type === 'local') { + if (parseLocalRegistryPath(gitUrl) !== null) { return { registryId, error: 'local registry' }; } const match = gitUrl.match(/github\.com\/([^/]+)\/([^/.]+)/); @@ -284,7 +284,7 @@ export class SkillIndex { private async refreshLocalRegistryEntries(index: SkillIndexData): Promise { const registry = await this.registry.fetchMergedRegistry(); const localIds = Object.entries(registry.registries) - .filter(([, value]) => parseRegistrySource(value).type === 'local') + .filter(([, value]) => parseLocalRegistryPath(value) !== null) .map(([id]) => id); if (localIds.length === 0) return index; const localSkills = await this.readConfiguredLocalRegistrySkills(registry.registries); @@ -301,8 +301,7 @@ export class SkillIndex { const skills: SkillEntry[] = []; for (const [registryId, value] of Object.entries(registries)) { - const source = parseRegistrySource(value); - const registrySkills = source.type === 'local' + const registrySkills = parseLocalRegistryPath(value) !== null ? await this.readLocalRegistrySkills( registryId, await this.registry.prepareRegistryRepository(registryId, value), diff --git a/packages/cli/src/lib/SkillManager.ts b/packages/cli/src/lib/SkillManager.ts index 29b85931..02227565 100644 --- a/packages/cli/src/lib/SkillManager.ts +++ b/packages/cli/src/lib/SkillManager.ts @@ -8,7 +8,7 @@ import { SkillRegistry, SKILL_CACHE_DIR } from './SkillRegistry.js'; import { SkillIndex } from './SkillIndex.js'; import { getAllEnvironments, getGlobalSkillPath, getSkillCapableEnvironments, getSkillPath, validateEnvironmentCodes } from '../util/env.js'; import { validateRegistryId, validateSkillName, extractSkillDescription, isValidSkillName } from '../util/skill.js'; -import { parseRegistrySource } from '../util/skill-registry.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'; @@ -90,7 +90,7 @@ export class SkillManager { } const repoPath = await this.registry.prepareRegistryRepository(registryId, gitUrl); - const isLocal = Boolean(gitUrl && parseRegistrySource(gitUrl).type === 'local'); + const isLocal = Boolean(gitUrl && parseLocalRegistryPath(gitUrl) !== null); const resolvedSkillNames = skillName ? [skillName] @@ -376,7 +376,7 @@ export class SkillManager { return this.index.rebuildIndex(outputPath); } - async updateSkillIndexForRegistry(registryId: string, registryPath?: string): Promise { + async updateSkillIndexForRegistry(registryId: string, registryPath: string): Promise { return this.index.updateRegistryFromCache(registryId, registryPath); } diff --git a/packages/cli/src/lib/SkillRegistry.ts b/packages/cli/src/lib/SkillRegistry.ts index ce19f1c9..b8e8c784 100644 --- a/packages/cli/src/lib/SkillRegistry.ts +++ b/packages/cli/src/lib/SkillRegistry.ts @@ -7,7 +7,7 @@ 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 { parseRegistrySource } from '../util/skill-registry.js'; +import { parseLocalRegistryPath } from '../util/skill-registry.js'; import { isValidSkillName } from '../util/skill.js'; import { LOCAL_REGISTRY_MAX_ENTRIES } from '../util/local-registry.js'; @@ -116,7 +116,7 @@ export class SkillRegistry { return preparedRepository; } - const preparation = gitUrl && parseRegistrySource(gitUrl).type === 'local' + const preparation = gitUrl && parseLocalRegistryPath(gitUrl) !== null ? this.prepareLocalRegistry(registryId, gitUrl) : this.prepareGitRegistry(registryId, gitUrl); this.preparedRepositories.set(registryId, preparation); @@ -129,20 +129,20 @@ export class SkillRegistry { } private async prepareLocalRegistry(registryId: string, value: string): Promise { - const source = parseRegistrySource(value); - if (source.type !== 'local') { + 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(source.path); + 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 ${source.path}: ${getErrorMessage(error)}. Recreate it or re-register the source.`, - { registryId, path: source.path }, + `Local registry "${registryId}" is unavailable at ${localPath}: ${getErrorMessage(error)}. Recreate it or re-register the source.`, + { registryId, path: localPath }, ); } @@ -209,9 +209,9 @@ export class SkillRegistry { const cacheDir = SKILL_CACHE_DIR; const configured = await this.fetchMergedRegistry(); const localEntries = Object.entries(configured.registries) - .filter(([id, value]) => (!registryId || id === registryId) && parseRegistrySource(value).type === 'local'); + .filter(([id, value]) => (!registryId || id === registryId) && parseLocalRegistryPath(value) !== null); const configuredLocalIds = new Set(Object.entries(configured.registries) - .filter(([, value]) => parseRegistrySource(value).type === 'local') + .filter(([, value]) => parseLocalRegistryPath(value) !== null) .map(([id]) => id)); const results: UpdateResult[] = []; diff --git a/packages/cli/src/util/local-registry.ts b/packages/cli/src/util/local-registry.ts index b1633b46..e2323d6b 100644 --- a/packages/cli/src/util/local-registry.ts +++ b/packages/cli/src/util/local-registry.ts @@ -6,23 +6,11 @@ import { extractSkillDescription, isValidSkillName } from './skill.js'; export const LOCAL_REGISTRY_MAX_ENTRIES = 10_000; export const LOCAL_REGISTRY_MAX_SKILL_MD_BYTES = 1024 * 1024; -export interface RegistryDiscoveryLimits { - maxEntries: number; - maxSkillMdBytes: number; -} - -export interface DiscoveredRegistrySkill { +interface DiscoveredRegistrySkill { name: string; - path: string; - content: string; description: string; } -const DEFAULT_LIMITS: RegistryDiscoveryLimits = { - maxEntries: LOCAL_REGISTRY_MAX_ENTRIES, - maxSkillMdBytes: LOCAL_REGISTRY_MAX_SKILL_MD_BYTES, -}; - function isStrictlyContained(root: string, candidate: string): boolean { const relative = path.relative(root, candidate); return Boolean(relative) @@ -72,7 +60,6 @@ export async function resolveContainedSkill( export async function discoverRegistrySkills( registryId: string, registryRoot: string, - limits: RegistryDiscoveryLimits = DEFAULT_LIMITS, ): Promise { const canonicalRoot = await fs.realpath(registryRoot); const skillsRoot = await fs.realpath(path.join(canonicalRoot, 'skills')); @@ -88,27 +75,18 @@ export async function discoverRegistrySkills( let entries = 0; for await (const entry of directory) { entries += 1; - if (entries > limits.maxEntries) { + if (entries > LOCAL_REGISTRY_MAX_ENTRIES) { throw new CliError( - `Local registry "${registryId}" exceeds the ${limits.maxEntries} entry limit.`, + `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 stat = await fs.stat(metadataPath); - if (stat.size > limits.maxSkillMdBytes) { - throw new CliError( - `SKILL.md for "${entry.name}" is too large (${stat.size} bytes; limit ${limits.maxSkillMdBytes}).`, - 'LOCAL_REGISTRY_TOO_LARGE', - ); - } const content = await fs.readFile(metadataPath, 'utf8'); skills.push({ name: entry.name, - path: skillPath, - content, description: extractSkillDescription(content), }); } diff --git a/packages/cli/src/util/skill-registry.ts b/packages/cli/src/util/skill-registry.ts index 220d145b..b53ad381 100644 --- a/packages/cli/src/util/skill-registry.ts +++ b/packages/cli/src/util/skill-registry.ts @@ -3,13 +3,9 @@ import fs from 'fs-extra'; import path from 'node:path'; import { fileURLToPath, pathToFileURL } from 'node:url'; -export type RegistrySource = - | { type: 'git'; value: string } - | { type: 'local'; value: string; path: string }; - -export function parseRegistrySource(value: string): RegistrySource { +export function parseLocalRegistryPath(value: string): string | null { if (!value.toLowerCase().startsWith('file:')) { - return { type: 'git', value }; + return null; } try { @@ -24,7 +20,7 @@ export function parseRegistrySource(value: string): RegistrySource { if (!path.isAbsolute(localPath)) { throw new Error('path must be absolute'); } - return { type: 'local', value, path: localPath }; + return localPath; } catch (error: unknown) { throw new CliError( `Invalid local registry source "${value}": ${error instanceof Error ? error.message : String(error)}`, @@ -39,14 +35,12 @@ function isPathShorthand(value: string): boolean { } export async function normalizeRegistrySourceInput(value: string, baseDir: string): Promise { - const parsed = parseRegistrySource(value); - if (parsed.type === 'git' && !isPathShorthand(value)) { + const localPath = parseLocalRegistryPath(value); + if (localPath === null && !isPathShorthand(value)) { return value; } - const requestedPath = parsed.type === 'local' - ? parsed.path - : path.resolve(baseDir, value); + const requestedPath = localPath ?? path.resolve(baseDir, value); let canonicalPath: string; try { canonicalPath = await fs.realpath(requestedPath); @@ -76,43 +70,23 @@ export async function normalizeRegistrySources( const localOwners = new Map(); for (const [id, value] of Object.entries(registries)) { const nextValue = await normalizeRegistrySourceInput(value, baseDir); - const source = parseRegistrySource(nextValue); - if (source.type === 'local') { - const existingId = localOwners.get(source.path); + 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}": ${source.path}`, + `Local folder is already registered as "${existingId}": ${localPath}`, 'REGISTRY_SOURCE_CONFLICT', - { id, existingId, path: source.path }, + { id, existingId, path: localPath }, ); } - localOwners.set(source.path, id); + localOwners.set(localPath, id); } normalized[id] = nextValue; } return normalized; } -export function assertUniqueLocalRegistrySource( - registries: Record, - id: string, - value: string, -): void { - const requested = parseRegistrySource(value); - if (requested.type !== 'local') return; - for (const [existingId, existingValue] of Object.entries(registries)) { - if (existingId === id) continue; - const existing = parseRegistrySource(existingValue); - if (existing.type === 'local' && existing.path === requested.path) { - throw new CliError( - `Local folder is already registered as "${existingId}": ${requested.path}`, - 'REGISTRY_SOURCE_CONFLICT', - { id, existingId, path: requested.path }, - ); - } - } -} - export interface AddSkillRegistryOptions { force?: boolean; } From 592e5907b41397d6ef190c46ae4653b0be7392ce Mon Sep 17 00:00:00 2001 From: Hoang Nguyen Date: Mon, 7 Sep 2026 17:05:39 +0000 Subject: [PATCH 6/6] docs: reconcile local registry simplification Record the pre-merge simplification decisions, retained safety boundaries, production-limit tests, and fresh full-suite verification counts so lifecycle documents match the final implementation. --- docs/ai/design/2026-09-07-feature-local-registry.md | 8 +++----- .../implementation/2026-09-07-feature-local-registry.md | 4 ++-- docs/ai/planning/2026-09-07-feature-local-registry.md | 4 ++++ docs/ai/requirements/2026-09-07-feature-local-registry.md | 2 +- docs/ai/testing/2026-09-07-feature-local-registry.md | 6 ++++-- 5 files changed, 14 insertions(+), 10 deletions(-) diff --git a/docs/ai/design/2026-09-07-feature-local-registry.md b/docs/ai/design/2026-09-07-feature-local-registry.md index 5154b270..a316e41b 100644 --- a/docs/ai/design/2026-09-07-feature-local-registry.md +++ b/docs/ai/design/2026-09-07-feature-local-registry.md @@ -18,13 +18,11 @@ description: Explicit file URL sources with read-only preparation and contained Discover --> Index[skills.json] Remove --> Owned[config/index/contained cache only] -The string map remains the storage boundary. A small parsed union centralizes type detection; SkillRegistry branches preparation/update and consumers receive the actual prepared root. +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 - type RegistrySource = - | { type: 'git'; value: string } - | { type: 'local'; value: string; path: string }; + parseLocalRegistryPath(source): string | null Local storage is a canonical file:///absolute/path string. No object migration or provider class is introduced. @@ -45,7 +43,7 @@ prepareRegistryRepository retains its per-instance promise map. Git keeps clone/ 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 injectable test limits and no user-facing flag. +Production limits are documented constants based on measured repositories, with no user-facing or test-only configuration surface. ## Flow Integration diff --git a/docs/ai/implementation/2026-09-07-feature-local-registry.md b/docs/ai/implementation/2026-09-07-feature-local-registry.md index be6c9fe0..2fa5dc0b 100644 --- a/docs/ai/implementation/2026-09-07-feature-local-registry.md +++ b/docs/ai/implementation/2026-09-07-feature-local-registry.md @@ -43,8 +43,8 @@ Red evidence included missing parser functions and a missing local-registry modu ## Deviations -None. Limits are 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. +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 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. No blocking findings remain. +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 index e10e97e9..e19ad9d6 100644 --- a/docs/ai/planning/2026-09-07-feature-local-registry.md +++ b/docs/ai/planning/2026-09-07-feature-local-registry.md @@ -22,6 +22,10 @@ description: TDD plan for explicit read-only local sources - [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 diff --git a/docs/ai/requirements/2026-09-07-feature-local-registry.md b/docs/ai/requirements/2026-09-07-feature-local-registry.md index 059b091b..4a01f9d2 100644 --- a/docs/ai/requirements/2026-09-07-feature-local-registry.md +++ b/docs/ai/requirements/2026-09-07-feature-local-registry.md @@ -65,7 +65,7 @@ AI DevKit treats every registry string as Git and clones/pulls it into ~/.ai-dev - 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; tests use injectable low limits. +- Production limits use measured evidence and tests exercise those limits directly. ## Questions diff --git a/docs/ai/testing/2026-09-07-feature-local-registry.md b/docs/ai/testing/2026-09-07-feature-local-registry.md index 353b7f33..62af598f 100644 --- a/docs/ai/testing/2026-09-07-feature-local-registry.md +++ b/docs/ai/testing/2026-09-07-feature-local-registry.md @@ -8,7 +8,7 @@ description: Safety-first testing strategy ## Goals -100% new parser/containment coverage, unit coverage for SR-01–12, temp-directory adapter coverage, CLI e2e path/error coverage, and green Git registry regressions. +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 @@ -53,4 +53,6 @@ Tests create isolated temp roots with skills/name/SKILL.md, snapshot source cont ## Results -Final evidence: npm run build built six projects; npm test passed 2,188 tests across six projects, including 1,140 CLI tests; npm run lint passed with zero errors and two unrelated existing warnings; npm run test:e2e passed 42 tests. Focused TDD suites passed after every change. 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. +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.