fix(cli): accept source prefixes in the wizard package prompt (#88)

Co-authored-by: Harlan Wilton <harlan@harlanzw.com>

Author: mrrobertkent

README.md

@@ -146,6 +146,8 @@ skilld
 # Add skills for specific package(s) — npm: prefix for registry packages
 skilld add npm:vue npm:nuxt npm:pinia
 
+# The same prefixes work in the interactive wizard's package prompt
+
 # Add a pre-authored skill from a GitHub repo
 skilld add gh:vercel-labs/agent-skills
 

src/cache/internal/version.ts

@@ -11,6 +11,14 @@ const VALID_PKG_NAME = /^(?:@[a-z0-9][-a-z0-9._]*\/)?[a-z0-9][-a-z0-9._]*$/
 /** Validate version string (semver-ish, no path separators) */
 const VALID_VERSION = /^[a-z0-9][-\w.+]*$/i
 
+export function isValidCachePackageName(name: string): boolean {
+  return VALID_PKG_NAME.test(name)
+}
+
+export function isValidCacheVersion(version: string): boolean {
+  return VALID_VERSION.test(version)
+}
+
 /**
  * Get exact version key for cache keying
  */
@@ -30,9 +38,9 @@ export function getCacheKey(name: string, version: string): string {
  * Validates name/version to prevent path traversal.
  */
 export function getCacheDir(name: string, version: string): string {
-  if (!VALID_PKG_NAME.test(name))
+  if (!isValidCachePackageName(name))
     throw new Error(`Invalid package name: ${name}`)
-  if (!VALID_VERSION.test(version))
+  if (!isValidCacheVersion(version))
     throw new Error(`Invalid version: ${version}`)
 
   const dir = resolve(REFERENCES_DIR, getCacheKey(name, version))

src/cli.ts

@@ -21,6 +21,7 @@ import { runWizard } from './commands/wizard.ts'
 import { timedSpinner } from './core/formatting.ts'
 import { getProjectState, hasCompletedWizard, isOutdated, readConfig, semverGt } from './core/index.ts'
 import { readPackageJsonSafe } from './core/package-json.ts'
+import { parseNpmPackageInputs } from './core/prefix.ts'
 import { COMMA_OR_WHITESPACE_RE, VERSION_RANGE_PREFIX_RE } from './core/regex.ts'
 import { iterateSkills } from './core/skills.ts'
 import { fetchLatestVersion, fetchNpmRegistryMeta } from './sources/index.ts'
@@ -277,7 +278,7 @@ const main = defineCommand({
         if (source === 'manual') {
           const input = await p.text({
             message: 'Enter package names (space or comma-separated)',
-            placeholder: 'vue nuxt pinia',
+            placeholder: 'vue npm:nuxt pinia',
           })
           if (p.isCancel(input)) {
             if (!hasPkgJson) {
@@ -290,7 +291,12 @@ const main = defineCommand({
             p.log.warn('No packages entered')
             continue
           }
-          selected = input.split(COMMA_OR_WHITESPACE_RE).map(s => s.trim()).filter(Boolean)
+          const parsed = parseNpmPackageInputs(input.split(COMMA_OR_WHITESPACE_RE).map(s => s.trim()).filter(Boolean))
+          if (parsed._tag === 'Err') {
+            p.log.error(`${parsed.input} is not an npm package. Install it with \`skilld add ${parsed.input}\`.`)
+            continue
+          }
+          selected = parsed.packageSpecs
           if (selected.length === 0) {
             p.log.warn('No valid packages entered')
             continue
@@ -534,11 +540,16 @@ const main = defineCommand({
             if (source === 'manual') {
               const input = guard(await p.text({
                 message: 'Enter package names (space or comma-separated)',
-                placeholder: 'vue nuxt pinia',
+                placeholder: 'vue npm:nuxt pinia',
               }))
               if (!input)
                 return
-              selected = input.split(COMMA_OR_WHITESPACE_RE).map(s => s.trim()).filter(Boolean)
+              const parsed = parseNpmPackageInputs(input.split(COMMA_OR_WHITESPACE_RE).map(s => s.trim()).filter(Boolean))
+              if (parsed._tag === 'Err') {
+                p.log.error(`${parsed.input} is not an npm package. Install it with \`skilld add ${parsed.input}\`.`)
+                return
+              }
+              selected = parsed.packageSpecs
               if (selected.length === 0)
                 return
             }

src/core/prefix.ts

@@ -16,6 +16,7 @@ import type { GitSkillSource } from '../sources/git-skills.ts'
 import { parseGitSkillInput } from '../sources/git-skills.ts'
 
 const STATIC_REGEX_1 = /^[\w.-]+\/[\w.-]+/
+const EXPLICIT_NON_NPM_PREFIX_RE = /^(?:crate|gh|github):/
 
 export type SkillSource
   = | { type: 'npm', package: string, tag?: string }
@@ -25,6 +26,25 @@ export type SkillSource
     | { type: 'collection', handle: string, name: string }
     | { type: 'bare', package: string, tag?: string }
 
+export type NpmPackageInputResult
+  = | { _tag: 'Ok', packageSpecs: string[] }
+    | { _tag: 'Err', input: string }
+
+export function parseNpmPackageInputs(inputs: string[]): NpmPackageInputResult {
+  const packageSpecs: string[] = []
+
+  for (const input of inputs) {
+    const source = parseSkillInput(input)
+    const isMalformedExplicitSource = source.type === 'bare' && EXPLICIT_NON_NPM_PREFIX_RE.test(input)
+    if ((source.type !== 'npm' && source.type !== 'bare') || isMalformedExplicitSource || !source.package)
+      return { _tag: 'Err', input }
+
+    packageSpecs.push(source.tag ? `${source.package}@${source.tag}` : source.package)
+  }
+
+  return { _tag: 'Ok', packageSpecs }
+}
+
 /**
  * Parse a single CLI input token into a typed SkillSource.
  *

src/core/prepare.ts

@@ -9,7 +9,7 @@
 import type { SkillInfo } from './lockfile.ts'
 import { existsSync, lstatSync, mkdirSync, readdirSync, rmSync, symlinkSync, unlinkSync } from 'node:fs'
 import { basename, join } from 'pathe'
-import { getCacheDir } from '../cache/internal/version.ts'
+import { getCacheDir, isValidCachePackageName, isValidCacheVersion } from '../cache/internal/version.ts'
 import { readPackageJsonSafe } from './package-json.ts'
 import { README_FILENAME_RE } from './regex.ts'
 
@@ -24,11 +24,16 @@ function toStorageName(name: string): string {
 
 /** Resolve package directory: node_modules first, then global cache */
 export function resolvePkgDir(name: string, cwd: string, version?: string): string | null {
+  if (!isValidCachePackageName(name))
+    return null
+
   const nodeModulesPath = join(cwd, 'node_modules', name)
   if (existsSync(nodeModulesPath))
     return nodeModulesPath
 
   if (version) {
+    if (!isValidCacheVersion(version))
+      return null
     const cachedPkgDir = join(getCacheDir(name, version), 'pkg')
     if (existsSync(join(cachedPkgDir, 'package.json')))
       return cachedPkgDir

test/unit/pkg-dir-probe.test.ts

@@ -0,0 +1,40 @@
+import { mkdirSync, mkdtempSync, rmSync } from 'node:fs'
+import { tmpdir } from 'node:os'
+import { join } from 'pathe'
+import { afterEach, describe, expect, it } from 'vitest'
+import { getShippedSkills, resolvePkgDir } from '../../src/core/prepare.ts'
+
+describe('package dir probing', () => {
+  const fixtureDirs: string[] = []
+
+  afterEach(() => {
+    for (const dir of fixtureDirs)
+      rmSync(dir, { recursive: true, force: true })
+    fixtureDirs.length = 0
+  })
+
+  it.each(['npm:vue', 'gh:owner/repo', ''])('returns null for %j', (name) => {
+    expect(resolvePkgDir(name, process.cwd(), '1.0.0')).toBeNull()
+  })
+
+  it('rejects traversal when the escaped directory exists', () => {
+    const cwd = mkdtempSync(join(tmpdir(), 'skilld-pkg-probe-'))
+    fixtureDirs.push(cwd)
+    mkdirSync(join(cwd, 'escape'))
+
+    expect(resolvePkgDir('../escape', cwd, '1.0.0')).toBeNull()
+  })
+
+  it('returns an installed package before validating the cache version', () => {
+    const cwd = mkdtempSync(join(tmpdir(), 'skilld-pkg-probe-'))
+    fixtureDirs.push(cwd)
+    const packageDir = join(cwd, 'node_modules', 'vue')
+    mkdirSync(packageDir, { recursive: true })
+
+    expect(resolvePkgDir('vue', cwd, '../invalid')).toBe(packageDir)
+  })
+
+  it.each(['npm:vue', '../escape'])('reports no shipped skills for %j', (name) => {
+    expect(getShippedSkills(name, process.cwd(), '1.0.0')).toEqual([])
+  })
+})

test/unit/prefix.test.ts

@@ -1,7 +1,20 @@
 import { describe, expect, it } from 'vitest'
-import { parseSkillInput, resolveSkillName } from '../../src/core/prefix'
+import { parseNpmPackageInputs, parseSkillInput, resolveSkillName } from '../../src/core/prefix'
 
 describe('prefix parser', () => {
+  describe('wizard npm inputs', () => {
+    it('normalizes prefixes without dropping npm tags', () => {
+      expect(parseNpmPackageInputs(['npm:vue@beta', '@nuxt/ui@3.0.0', 'pinia'])).toEqual({
+        _tag: 'Ok',
+        packageSpecs: ['vue@beta', '@nuxt/ui@3.0.0', 'pinia'],
+      })
+    })
+
+    it.each(['gh:owner/repo', 'gh:not-a-repo', 'crate:serde', '@curator'])('rejects non-npm input %s', (input) => {
+      expect(parseNpmPackageInputs([input])).toEqual({ _tag: 'Err', input })
+    })
+  })
+
   describe('npm: prefix', () => {
     it('parses simple package name', () => {
       expect(parseSkillInput('npm:vue')).toEqual({

test/unit/prepare-restore.test.ts

@@ -12,9 +12,13 @@ vi.mock('node:fs', async () => {
   }
 })
 
-vi.mock('../../src/cache/internal/version', () => ({
-  getCacheDir: (name: string, version: string) => `/home/.skilld/references/${name}@${version}`,
-}))
+vi.mock('../../src/cache/internal/version', async (importOriginal) => {
+  const actual = await importOriginal<typeof import('../../src/cache/internal/version')>()
+  return {
+    ...actual,
+    getCacheDir: (name: string, version: string) => `/home/.skilld/references/${name}@${version}`,
+  }
+})
 
 describe('restorePkgSymlink', () => {
   beforeEach(() => vi.resetAllMocks())