fix: harden npm registry installs

Author: SoonIter

packages/skills-package-manager/README.md

@@ -125,13 +125,14 @@ const skills = await listRepoSkills('vercel-labs', 'skills')
 
 ## Specifier Format
 
-```
-<source>#[ref&]path:<skill-path>
+```text
+git/file/npm: <source>#[ref&]path:<skill-path>
+link: link:<path-to-skill-dir>
 ```
 
 | Part | Description | Example |
 |------|-------------|---------|
-| `source` | Git URL, direct `link:` skill path, `file:` tarball, or `npm:` package | `https://github.com/o/r.git`, `link:./local/skills/my-skill`, `file:./skills.tgz`, `npm:@scope/pkg` |
+| `source` | Git URL, direct `link:` skill path, `file:` tarball, or `npm:` package name | `https://github.com/o/r.git`, `link:./local/skills/my-skill`, `file:./skills.tgz`, `npm:@scope/pkg` |
 | `ref` | Optional git ref | `main`, `v1.0.0`, `HEAD`, `6cb0992`, `6cb0992a176f2ca142e19f64dca8ac12025b035e` |
 | `path` | Path to skill directory within source | `/skills/my-skill` |
 
@@ -142,7 +143,9 @@ const skills = await listRepoSkills('vercel-labs', 'skills')
 - **`git`** — Clones the repo, resolves commit hash, copies skill files
 - **`link`** — Reads from a local directory and copies the selected skill
 - **`file`** — Extracts a local `tgz` package and copies the selected skill
-- **`npm`** — Packs an npm package source, locks the resolved version, and installs from the package contents
+- **`npm`** — Resolves a package from the configured npm registry, locks the tarball URL/version/integrity, and installs from the downloaded tarball
+
+`npm:` reads `registry` and scoped `@scope:registry` values from `.npmrc`. Matching `:_authToken`, `:_auth`, or `username` + `:_password` entries are also used for private registry requests.
 
 ## Architecture
 

packages/skills-package-manager/package.json

@@ -24,6 +24,7 @@
     "@clack/prompts": "^1.1.0",
     "cac": "^7.0.0",
     "picocolors": "^1.1.1",
+    "semver": "^7.7.2",
     "tar": "^7.4.3",
     "yaml": "^2.8.1"
   },

packages/skills-package-manager/src/commands/update.ts

@@ -91,6 +91,7 @@ export async function updateCommand(options: UpdateCommandOptions): Promise<Upda
         previous.resolution.packageName === entry.resolution.packageName &&
         previous.resolution.version === entry.resolution.version &&
         previous.resolution.path === entry.resolution.path &&
+        previous.resolution.tarball === entry.resolution.tarball &&
         previous.resolution.integrity === entry.resolution.integrity
       ) {
         result.unchanged.push(skillName)

packages/skills-package-manager/src/config/syncSkillsLock.ts

@@ -157,7 +157,9 @@ export async function resolveLockEntry(
           integrity: resolved.integrity,
           registry: resolved.registry,
         },
-        digest: sha256(`${resolved.name}:${resolved.version}:${resolved.tarballUrl}:${normalized.path}`),
+        digest: sha256(
+          `${resolved.name}:${resolved.version}:${resolved.tarballUrl}:${normalized.path}`,
+        ),
       },
     }
   }

packages/skills-package-manager/src/config/types.ts

@@ -20,7 +20,15 @@ export type SkillsLockEntry = {
     | { type: 'link'; path: string }
     | { type: 'file'; tarball: string; path: string }
     | { type: 'git'; url: string; commit: string; path: string }
-    | { type: 'npm'; packageName: string; version: string; path: string; integrity?: string }
+    | {
+        type: 'npm'
+        packageName: string
+        version: string
+        path: string
+        tarball: string
+        integrity?: string
+        registry?: string
+      }
   digest: string
 }
 

packages/skills-package-manager/src/install/installSkills.ts

@@ -1,12 +1,12 @@
+import { access } from 'node:fs/promises'
 import path from 'node:path'
 import { isLockInSync } from '../config/compareSkillsLock'
 import { readSkillsLock } from '../config/readSkillsLock'
 import { readSkillsManifest } from '../config/readSkillsManifest'
 import { syncSkillsLock } from '../config/syncSkillsLock'
 import type { InstallProgressListener, SkillsLock, SkillsManifest } from '../config/types'
 import { writeSkillsLock } from '../config/writeSkillsLock'
-import { cleanupPackedNpmPackage, packNpmPackage } from '../npm/packPackage'
-import { normalizeSpecifier } from '../specifiers/normalizeSpecifier'
+import { cleanupPackedNpmPackage, downloadNpmPackageTarball } from '../npm/packPackage'
 import { sha256 } from '../utils/hash'
 import { readInstallState, writeInstallState } from './installState'
 import { linkSkill } from './links'
@@ -19,19 +19,20 @@ export const installStageHooks = {
   beforeFetch: async (_rootDir: string, _manifest: SkillsManifest, _lockfile: SkillsLock) => {},
 }
 
-function resolveNpmPackSource(specifier: string, packageName: string, version: string): string {
-  const normalized = normalizeSpecifier(specifier)
-  const source = normalized.source.slice('npm:'.length)
-
-  if (source.startsWith('.') || source.startsWith('/') || source.startsWith('~')) {
-    return source
-  }
-
-  if (source.includes(':')) {
-    return source
+async function areManagedSkillsInstalled(
+  rootDir: string,
+  installDir: string,
+  skillNames: string[],
+): Promise<boolean> {
+  for (const skillName of skillNames) {
+    try {
+      await access(path.join(rootDir, installDir, skillName, 'SKILL.md'))
+    } catch {
+      return false
+    }
   }
 
-  return `${packageName}@${version}`
+  return true
 }
 
 export async function fetchSkillsFromLock(
@@ -44,87 +45,107 @@ export async function fetchSkillsFromLock(
 ) {
   await installStageHooks.beforeFetch(rootDir, manifest, lockfile)
 
-  const lockDigest = sha256(JSON.stringify(lockfile))
-  const state = await readInstallState(rootDir)
-  if (state?.lockDigest === lockDigest) {
-    return { status: 'skipped', reason: 'up-to-date' } as const
-  }
-
   const installDir = manifest.installDir ?? '.agents/skills'
   const linkTargets = manifest.linkTargets ?? []
 
   await pruneManagedSkills(rootDir, installDir, linkTargets, Object.keys(lockfile.skills))
 
-  for (const [skillName, entry] of Object.entries(lockfile.skills)) {
-    if (entry.resolution.type === 'link') {
-      await materializeLocalSkill(
-        rootDir,
-        skillName,
-        path.resolve(rootDir, entry.resolution.path),
-        '/',
-        installDir,
-      )
-      continue
-    }
+  const lockDigest = sha256(JSON.stringify(lockfile))
+  const state = await readInstallState(rootDir, installDir)
+  if (
+    state?.lockDigest === lockDigest &&
+    (await areManagedSkillsInstalled(rootDir, installDir, Object.keys(lockfile.skills)))
+  ) {
+    return { status: 'skipped', reason: 'up-to-date' } as const
+  }
 
-    if (entry.resolution.type === 'file') {
-      await materializePackedSkill(
-        rootDir,
-        skillName,
-        path.resolve(rootDir, entry.resolution.tarball),
-        entry.resolution.path,
-        installDir,
-      )
-      options?.onProgress?.({ type: 'added', skillName })
-      continue
-    }
+  const downloadedTarballs = new Map<string, Promise<string>>()
 
-    if (entry.resolution.type === 'git') {
-      await materializeGitSkill(
-        rootDir,
-        skillName,
-        entry.resolution.url,
-        entry.resolution.commit,
-        entry.resolution.path,
-        installDir,
-      )
-      options?.onProgress?.({ type: 'added', skillName })
-      continue
-    }
+  try {
+    for (const [skillName, entry] of Object.entries(lockfile.skills)) {
+      if (entry.resolution.type === 'link') {
+        await materializeLocalSkill(
+          rootDir,
+          skillName,
+          path.resolve(rootDir, entry.resolution.path),
+          '/',
+          installDir,
+        )
+        options?.onProgress?.({ type: 'added', skillName })
+        continue
+      }
 
-    if (entry.resolution.type === 'npm') {
-      const packed = await packNpmPackage(
-        resolveNpmPackSource(
-          entry.specifier,
-          entry.resolution.packageName,
-          entry.resolution.version,
-        ),
-      )
+      if (entry.resolution.type === 'file') {
+        await materializePackedSkill(
+          rootDir,
+          skillName,
+          path.resolve(rootDir, entry.resolution.tarball),
+          entry.resolution.path,
+          installDir,
+        )
+        options?.onProgress?.({ type: 'added', skillName })
+        continue
+      }
 
-      try {
+      if (entry.resolution.type === 'git') {
+        await materializeGitSkill(
+          rootDir,
+          skillName,
+          entry.resolution.url,
+          entry.resolution.commit,
+          entry.resolution.path,
+          installDir,
+        )
+        options?.onProgress?.({ type: 'added', skillName })
+        continue
+      }
+
+      if (entry.resolution.type === 'npm') {
+        const cacheKey = `${entry.resolution.tarball}\0${entry.resolution.integrity ?? ''}`
+        let tarballPathPromise = downloadedTarballs.get(cacheKey)
+        if (!tarballPathPromise) {
+          tarballPathPromise = downloadNpmPackageTarball(
+            rootDir,
+            entry.resolution.tarball,
+            entry.resolution.integrity,
+          )
+          downloadedTarballs.set(cacheKey, tarballPathPromise)
+        }
+
+        const tarballPath = await tarballPathPromise
         await materializePackedSkill(
           rootDir,
           skillName,
-          packed.tarballPath,
+          tarballPath,
           entry.resolution.path,
           installDir,
         )
-      } finally {
-        await cleanupPackedNpmPackage(packed.tarballPath)
+        options?.onProgress?.({ type: 'added', skillName })
+        continue
       }
-      continue
+
+      throw new Error(`Unsupported resolution type in 0.1.0 core flow: ${entry.resolution.type}`)
     }
 
-    throw new Error(`Unsupported resolution type in 0.1.0 core flow: ${entry.resolution.type}`)
-  }
+    await writeInstallState(rootDir, installDir, {
+      lockDigest,
+      installDir,
+      linkTargets,
+      installerVersion: '0.1.0',
+      installedAt: new Date().toISOString(),
+    })
+  } finally {
+    const settledTarballs = await Promise.allSettled(downloadedTarballs.values())
+    const downloadedPaths = new Set(
+      settledTarballs
+        .filter(
+          (result): result is PromiseFulfilledResult<string> => result.status === 'fulfilled',
+        )
+        .map((result) => result.value),
+    )
 
-  await writeInstallState(rootDir, {
-    lockDigest,
-    installDir,
-    linkTargets,
-    installerVersion: '0.1.0',
-    installedAt: new Date().toISOString(),
-  })
+    await Promise.all([...downloadedPaths].map((tarballPath) => cleanupPackedNpmPackage(tarballPath)))
+  }
 
   return { status: 'fetched', fetched: Object.keys(lockfile.skills) } as const
 }
@@ -164,7 +185,6 @@ export async function installSkills(
   let lockfile: SkillsLock
 
   if (options?.frozenLockfile) {
-    // Frozen mode: lock must exist and be in sync
     if (!currentLock) {
       throw new Error('Lockfile is required in frozen mode but none was found')
     }
@@ -178,7 +198,6 @@ export async function installSkills(
       options?.onProgress?.({ type: 'resolved', skillName })
     }
   } else {
-    // Normal mode: sync lock with manifest (may trigger network requests)
     lockfile = await syncSkillsLock(rootDir, manifest, currentLock, {
       onProgress: options?.onProgress,
     })
@@ -187,7 +206,6 @@ export async function installSkills(
   await fetchSkillsFromLock(rootDir, manifest, lockfile, { onProgress: options?.onProgress })
   await linkSkillsFromLock(rootDir, manifest, lockfile, { onProgress: options?.onProgress })
 
-  // Write lockfile only after all operations succeed (atomicity)
   if (!options?.frozenLockfile) {
     await writeSkillsLock(rootDir, lockfile)
   }

packages/skills-package-manager/src/install/installState.ts

@@ -2,8 +2,10 @@ import { readFile } from 'node:fs/promises'
 import path from 'node:path'
 import { ensureDir, writeJson } from '../utils/fs'
 
-export async function readInstallState(rootDir: string) {
-  const filePath = path.join(rootDir, '.agents/skills/.skills-pm-install-state.json')
+const INSTALL_STATE_FILE = '.skills-pm-install-state.json'
+
+export async function readInstallState(rootDir: string, installDir: string) {
+  const filePath = path.join(rootDir, installDir, INSTALL_STATE_FILE)
 
   try {
     return JSON.parse(await readFile(filePath, 'utf8'))
@@ -12,9 +14,9 @@ export async function readInstallState(rootDir: string) {
   }
 }
 
-export async function writeInstallState(rootDir: string, value: unknown) {
-  const dirPath = path.join(rootDir, '.agents/skills')
+export async function writeInstallState(rootDir: string, installDir: string, value: unknown) {
+  const dirPath = path.join(rootDir, installDir)
   await ensureDir(dirPath)
-  const filePath = path.join(dirPath, '.skills-pm-install-state.json')
+  const filePath = path.join(dirPath, INSTALL_STATE_FILE)
   await writeJson(filePath, value)
 }

packages/skills-package-manager/src/install/materializeLocalSkill.ts

@@ -1,6 +1,6 @@
-import { cp, readFile } from 'node:fs/promises'
+import { readFile } from 'node:fs/promises'
 import path from 'node:path'
-import { ensureDir, writeJson } from '../utils/fs'
+import { ensureDir, replaceDir, writeJson } from '../utils/fs'
 
 export async function materializeLocalSkill(
   rootDir: string,
@@ -26,7 +26,7 @@ export async function materializeLocalSkill(
 
   const targetDir = path.join(rootDir, installDir, skillName)
   await ensureDir(path.dirname(targetDir))
-  await cp(absoluteSkillPath, targetDir, { recursive: true, force: true })
+  await replaceDir(absoluteSkillPath, targetDir)
   await writeJson(path.join(targetDir, '.skills-pm.json'), {
     name: skillName,
     installedBy: 'skills-package-manager',