From 01595f53fa2bb6c6dfbcef0a8413b50400e535f6 Mon Sep 17 00:00:00 2001 From: Domitrius Clark Date: Fri, 2 Oct 2026 11:53:32 -0400 Subject: [PATCH 1/6] feat(init): install Netlify agent skills by default netlify init now syncs the hosted Netlify skills manifest into the project's agent skills directory, verifying every file's SHA-256 and applying the stale/renamed/deprecated rules so repeat runs are no-ops. --skip-agent-setup opts out; failures warn and never block init. Co-Authored-By: Claude Fable 5.1 --- docs/commands/init.md | 1 + src/commands/init/index.ts | 3 +- src/commands/init/init.ts | 25 +- src/utils/init/agent-skills.ts | 519 +++++++++++++++++++ tests/integration/commands/init/init.test.ts | 141 ++++- tests/unit/utils/init/agent-skills.test.ts | 369 +++++++++++++ 6 files changed, 1047 insertions(+), 11 deletions(-) create mode 100644 src/utils/init/agent-skills.ts create mode 100644 tests/unit/utils/init/agent-skills.test.ts diff --git a/docs/commands/init.md b/docs/commands/init.md index 9aa21e7e4fe..2ba0ddb24b2 100644 --- a/docs/commands/init.md +++ b/docs/commands/init.md @@ -22,6 +22,7 @@ netlify init - `force` (*boolean*) - Reinitialize CI hooks if the linked project is already configured to use CI - `git-remote-name` (*string*) - Name of Git remote to use. e.g. "origin" - `manual` (*boolean*) - Manually configure a git remote for CI +- `skip-agent-setup` (*boolean*) - Skip installing Netlify skills for AI coding agents into the project - `debug` (*boolean*) - Print debugging information - `auth` (*string*) - Netlify auth token - can be used to run this command without logging in diff --git a/src/commands/init/index.ts b/src/commands/init/index.ts index ff9552ac350..b7e446cd590 100644 --- a/src/commands/init/index.ts +++ b/src/commands/init/index.ts @@ -11,6 +11,7 @@ export const createInitCommand = (program: BaseCommand) => ) .option('-m, --manual', 'Manually configure a git remote for CI') .option('--git-remote-name ', 'Name of Git remote to use. e.g. "origin"') + .option('--skip-agent-setup', 'Skip installing Netlify skills for AI coding agents into the project') .addHelpText('after', () => { const docsUrl = 'https://docs.netlify.com/cli/get-started/' return ` @@ -19,5 +20,5 @@ For more information about getting started with Netlify CLI, see ${terminalLink( }) .action(async (options: OptionValues, command: BaseCommand) => { const { init } = await import('./init.js') - await init(options, command) + await init(options, command, { setupAgentSkills: !options.skipAgentSetup }) }) diff --git a/src/commands/init/init.ts b/src/commands/init/init.ts index 76f53310b0e..48576193afc 100644 --- a/src/commands/init/init.ts +++ b/src/commands/init/init.ts @@ -11,6 +11,7 @@ import type BaseCommand from '../base-command.js' import { link } from '../link/link.js' import { sitesCreate } from '../sites/sites-create.js' import type { LocalState, SiteInfo } from '../../utils/types.js' +import { setupAgentSkills } from '../../utils/init/agent-skills.js' import { getBuildSettings, saveNetlifyToml } from '../../utils/init/utils.js' import { type InitExitCode, LINKED_EXISTING_SITE_EXIT_CODE, LINKED_NEW_SITE_EXIT_CODE } from './constants.js' @@ -223,14 +224,30 @@ type InitExitMessageCustomizer = (code: InitExitCode, defaultMessage: string) => type InitExtraOptions = { customizeExitMessage?: InitExitMessageCustomizer | undefined exitAfterConfiguringRepo?: boolean | undefined + setupAgentSkills?: boolean | undefined +} + +const installAgentSkills = async (command: BaseCommand): Promise => { + log() + const result = await setupAgentSkills({ workingDir: command.workingDir }) + await track('sites_agentSkillsSetup', { + installed: result.installed, + directories: result.directories, + skillsVersion: result.skillsVersion, + ...result.summary, + }) } export const init = async ( options: OptionValues, command: BaseCommand, - { customizeExitMessage, exitAfterConfiguringRepo = false }: InitExtraOptions = {}, + { + customizeExitMessage, + exitAfterConfiguringRepo = false, + setupAgentSkills: shouldSetupAgentSkills = false, + }: InitExtraOptions = {}, ): Promise => { - command.setAnalyticsPayload({ manual: options.manual, force: options.force }) + command.setAnalyticsPayload({ manual: options.manual, force: options.force, skipAgentSetup: options.skipAgentSetup }) const { repositoryRoot, state } = command.netlify const { siteInfo: existingSiteInfo } = command.netlify @@ -241,6 +258,10 @@ export const init = async ( // Add .netlify to .gitignore file await ensureNetlifyIgnore(repositoryRoot) + if (shouldSetupAgentSkills) { + await installAgentSkills(command) + } + const repoUrl = getRepoUrl(existingSiteInfo) if (repoUrl && !options.force) { logExistingAndExit({ siteInfo: existingSiteInfo }) diff --git a/src/utils/init/agent-skills.ts b/src/utils/init/agent-skills.ts new file mode 100644 index 00000000000..261d14a39ba --- /dev/null +++ b/src/utils/init/agent-skills.ts @@ -0,0 +1,519 @@ +import { createHash, randomBytes } from 'node:crypto' +import { promises as fs, type Dirent } from 'node:fs' +import path from 'node:path' + +import { getDrivingAgent } from '../agent-detection.js' +import { chalk, log, netlifyCommand, version, warn } from '../command-helpers.js' + +export const DEFAULT_SKILLS_HOST = 'https://netlify-agent-skills.netlify.app' +export const SKILLS_HOST_ENV = 'NETLIFY_SKILLS_HOST' +export const DEFAULT_SKILLS_DIRECTORY = path.join('.agents', 'skills') + +const AGENT_DIRECTORIES = ['.claude', '.agents', '.grok'] as const +const AGENT_DIRECTORY_BY_DRIVING_AGENT: Partial> = { + claude: '.claude', + claudeai: '.claude', +} + +const SKILL_NAME = /^[A-Za-z0-9_][A-Za-z0-9_.-]*$/ +const LOOPBACK_HOSTS = new Set(['localhost', '127.0.0.1', '[::1]']) + +export interface SkillHistoryEntry { + version: string | null + name?: string + tree_hash: string +} + +export interface ManifestSkill { + name: string + status: 'active' | 'deprecated' + version: string | null + prior_names?: string[] + description?: string + tree_hash: string | null + files?: Record + executable?: string[] + history?: SkillHistoryEntry[] + deprecated?: { since: string; replaced_by?: string } +} + +export interface SkillsManifest { + schema_version: number + version: string + skills: ManifestSkill[] +} + +export type SkillRecord = + | { name: string; status: 'current'; version: string | null } + | { name: string; status: 'stale'; version: string | null; have: string | null } + | { name: string; status: 'modified'; version: string | null } + | { name: string; status: 'renamed'; currentName: string; modified: boolean } + | { name: string; status: 'deprecated'; replacedBy: string | null; modified: boolean } + | { name: string; status: 'duplicate'; currentName: string } + | { name: string; status: 'unknown' } + +export interface SkillsClassification { + skills: SkillRecord[] + missing: string[] +} + +export type SkillAction = 'current' | 'added' | 'updated' | 'renamed' | 'removed' | 'kept' | 'ignored' + +export interface SkillActionRecord { + name: string + action: SkillAction + detail?: string +} + +export interface SkillsSyncResult { + directory: string + actions: SkillActionRecord[] +} + +class SkillsError extends Error {} + +const sha256 = (bytes: Uint8Array): string => `sha256:${createHash('sha256').update(bytes).digest('hex')}` + +export const resolveSkillsHost = (env: NodeJS.ProcessEnv = process.env): string => { + const raw = env[SKILLS_HOST_ENV] + if (!raw) { + return DEFAULT_SKILLS_HOST + } + const url = new URL(raw) + const loopback = LOOPBACK_HOSTS.has(url.hostname) + if (url.protocol !== 'https:' && !(url.protocol === 'http:' && loopback)) { + throw new SkillsError(`${SKILLS_HOST_ENV} must be https:// (http:// is accepted for localhost only): ${raw}`) + } + return url.toString().replace(/\/$/, '') +} + +const urlFor = (host: string, ...parts: string[]): string => + `${host}/${parts.map((part) => encodeURIComponent(part)).join('/')}` + +const fetchBytes = async (url: string): Promise => { + const response = await fetch(url, { headers: { 'user-agent': `NetlifyCLI ${version}` } }) + if (!response.ok) { + throw new SkillsError(`${url}: HTTP ${response.status.toString()}`) + } + return new Uint8Array(await response.arrayBuffer()) +} + +function assertSkillName(name: unknown, what: string): asserts name is string { + if (typeof name !== 'string' || !SKILL_NAME.test(name)) { + throw new SkillsError(`manifest: invalid ${what} ${JSON.stringify(name)}`) + } +} + +const isSafeFilePath = (file: string): boolean => + file.length > 0 && + !file.includes('\\') && + !path.posix.isAbsolute(file) && + file.split('/').every((part) => part && part !== '.' && part !== '..') + +interface ManifestIndex { + exact: Map + prior: Map +} + +const indexManifest = (manifest: SkillsManifest): ManifestIndex => { + if (!Array.isArray(manifest.skills)) { + throw new SkillsError('manifest has no skills array') + } + const exact = new Map() + const prior = new Map() + for (const skill of manifest.skills) { + assertSkillName(skill.name, 'skill name') + if (exact.has(skill.name)) { + throw new SkillsError(`manifest: duplicate skill name ${JSON.stringify(skill.name)}`) + } + exact.set(skill.name, skill) + for (const name of skill.prior_names ?? []) { + assertSkillName(name, `prior name of ${skill.name}`) + if (exact.has(name) || prior.has(name)) { + throw new SkillsError(`manifest: name ${JSON.stringify(name)} appears more than once`) + } + prior.set(name, skill) + } + } + for (const name of exact.keys()) { + if (prior.has(name)) { + throw new SkillsError(`manifest: name ${JSON.stringify(name)} is both a skill and a prior name`) + } + } + return { exact, prior } +} + +export const fetchSkillsManifest = async (host: string): Promise => { + const url = urlFor(host, 'manifest.json') + const bytes = await fetchBytes(url) + let manifest: SkillsManifest + try { + manifest = JSON.parse(Buffer.from(bytes).toString('utf8')) as SkillsManifest + } catch { + throw new SkillsError(`${url}: invalid JSON`) + } + indexManifest(manifest) + return manifest +} + +const historyOf = (skill: ManifestSkill): SkillHistoryEntry[] => { + if (Array.isArray(skill.history) && skill.history.length > 0) { + return skill.history + } + return skill.tree_hash ? [{ version: skill.version, tree_hash: skill.tree_hash }] : [] +} + +const lastMatching = (skill: ManifestSkill, treeHash: string | null): SkillHistoryEntry | undefined => + treeHash ? historyOf(skill).findLast((entry) => entry.tree_hash === treeHash) : undefined + +const listRegularFiles = async (dir: string): Promise => { + const files: string[] = [] + const walk = async (current: string, prefix: string) => { + const entries = await fs.readdir(current, { withFileTypes: true }) + for (const entry of entries) { + const relative = prefix ? `${prefix}/${entry.name}` : entry.name + if (entry.isDirectory()) { + await walk(path.join(current, entry.name), relative) + } else if (entry.isFile()) { + files.push(relative) + } else { + throw new SkillsError(`${path.join(current, entry.name)}: not a regular file`) + } + } + } + await walk(dir, '') + return files.sort() +} + +export const hashSkillTree = async (dir: string): Promise => { + const hash = createHash('sha256') + for (const relative of await listRegularFiles(dir)) { + const absolute = path.join(dir, ...relative.split('/')) + const [bytes, stat] = await Promise.all([fs.readFile(absolute), fs.stat(absolute)]) + const mode = stat.mode & 0o111 ? '100755' : '100644' + hash.update(`${relative}\0${mode}\0${sha256(bytes).replace(/^sha256:/, '')}\n`) + } + return `sha256:${hash.digest('hex')}` +} + +const isFile = async (file: string): Promise => { + try { + return (await fs.lstat(file)).isFile() + } catch { + return false + } +} + +const isDirectory = async (dir: string): Promise => { + try { + return (await fs.lstat(dir)).isDirectory() + } catch { + return false + } +} + +const sortByName = (entries: T[]): T[] => + [...entries].sort((a, b) => a.name.localeCompare(b.name, 'en')) + +export const classifySkillsDirectory = async ( + root: string, + manifest: SkillsManifest, +): Promise => { + const { exact, prior } = indexManifest(manifest) + const records: SkillRecord[] = [] + const presentActive = new Set() + + let entries: Dirent[] = [] + if (await isDirectory(root)) { + entries = sortByName(await fs.readdir(root, { withFileTypes: true })) + } + + for (const entry of entries) { + if (!entry.isDirectory()) continue + const known = exact.get(entry.name) + const renamed = prior.get(entry.name) + const target = known?.status === 'active' ? known : renamed?.status === 'active' ? renamed : null + const retired = known?.status === 'deprecated' ? known : renamed?.status === 'deprecated' ? renamed : null + const dir = path.join(root, entry.name) + const hasSkillMd = await isFile(path.join(dir, 'SKILL.md')) + if (!hasSkillMd && !target && !retired) continue + + let treeHash: string | null = null + if (hasSkillMd) { + try { + treeHash = await hashSkillTree(dir) + } catch { + treeHash = null + } + } + + if (retired) { + const match = lastMatching(retired, treeHash) + records.push({ + name: entry.name, + status: 'deprecated', + replacedBy: retired.deprecated?.replaced_by ?? null, + modified: !match, + }) + continue + } + + if (!target) { + const twin = treeHash + ? manifest.skills.find( + (skill) => skill.status === 'active' && historyOf(skill).some((item) => item.tree_hash === treeHash), + ) + : undefined + records.push( + twin + ? { name: entry.name, status: 'duplicate', currentName: twin.name } + : { name: entry.name, status: 'unknown' }, + ) + continue + } + + const match = lastMatching(target, treeHash) + if (target === renamed) { + records.push({ name: entry.name, status: 'renamed', currentName: target.name, modified: !match }) + continue + } + + presentActive.add(target.name) + if (treeHash === target.tree_hash) { + records.push({ name: entry.name, status: 'current', version: target.version }) + } else if (match) { + records.push({ name: entry.name, status: 'stale', version: target.version, have: match.version }) + } else { + records.push({ name: entry.name, status: 'modified', version: target.version }) + } + } + + const missing = manifest.skills + .filter((skill) => skill.status === 'active' && !presentActive.has(skill.name)) + .map((skill) => skill.name) + .sort() + + return { skills: records, missing } +} + +const replaceDirectory = async (staged: string, target: string): Promise => { + const exists = await isDirectory(target) + const retired = `${target}.old-${process.pid.toString()}-${randomBytes(6).toString('hex')}` + if (exists) { + await fs.rename(target, retired) + } + try { + await fs.rename(staged, target) + } catch (error) { + if (exists) { + await fs.rename(retired, target) + } + throw error + } + if (exists) { + await fs.rm(retired, { recursive: true, force: true }) + } +} + +export const installSkill = async (host: string, dest: string, skill: ManifestSkill): Promise => { + const files = Object.keys(skill.files ?? {}).sort() + const downloaded: [string, Uint8Array][] = [] + for (const file of files) { + if (!isSafeFilePath(file)) { + throw new SkillsError(`${skill.name}: unsafe manifest file path: ${file}`) + } + const bytes = await fetchBytes(urlFor(host, 'skills', skill.name, ...file.split('/'))) + if (sha256(bytes) !== skill.files?.[file]) { + throw new SkillsError(`${skill.name}/${file}: hash mismatch`) + } + downloaded.push([file, bytes]) + } + + await fs.mkdir(dest, { recursive: true }) + const staged = await fs.mkdtemp(path.join(dest, `.netlify-skill-${skill.name}-`)) + try { + const executable = new Set(skill.executable ?? []) + for (const [file, bytes] of downloaded) { + const output = path.join(staged, ...file.split('/')) + await fs.mkdir(path.dirname(output), { recursive: true }) + await fs.writeFile(output, bytes, { mode: executable.has(file) ? 0o755 : 0o644 }) + } + await replaceDirectory(staged, path.join(dest, skill.name)) + } catch (error) { + await fs.rm(staged, { recursive: true, force: true }) + throw new SkillsError(`${skill.name}: could not install: ${(error as Error).message}`) + } + return files.length +} + +export const syncSkills = async ({ + host, + directory, + manifest, +}: { + host: string + directory: string + manifest: SkillsManifest +}): Promise => { + const { exact } = indexManifest(manifest) + const before = await classifySkillsDirectory(directory, manifest) + const actions: SkillActionRecord[] = [] + const installed = new Set() + const act = (name: string, action: SkillAction, detail?: string) => { + actions.push(detail ? { name, action, detail } : { name, action }) + } + const skillByName = (name: string): ManifestSkill => { + const skill = exact.get(name) + if (!skill) { + throw new SkillsError(`manifest: unknown skill ${name}`) + } + return skill + } + + for (const record of before.skills) { + const dir = path.join(directory, record.name) + switch (record.status) { + case 'current': + act(record.name, 'current', record.version ?? undefined) + break + case 'stale': { + const skill = skillByName(record.name) + await installSkill(host, directory, skill) + act(record.name, 'updated', `${record.have ?? 'unknown'} -> ${skill.version ?? 'latest'}`) + break + } + case 'modified': + act(record.name, 'kept', 'edited locally') + break + case 'renamed': { + const current = before.skills.find((other) => other.name === record.currentName) + if (record.modified) { + act(record.name, 'kept', `edited locally; now called ${record.currentName}`) + } else if (current?.status === 'modified') { + act(record.name, 'kept', `${record.currentName} is already installed and edited locally`) + } else if (current) { + await fs.rm(dir, { recursive: true, force: true }) + act(record.name, 'removed', `superseded by ${record.currentName}`) + } else { + await installSkill(host, directory, skillByName(record.currentName)) + installed.add(record.currentName) + await fs.rm(dir, { recursive: true, force: true }) + act(record.name, 'renamed', `-> ${record.currentName}`) + } + break + } + case 'deprecated': { + const replacement = record.replacedBy ? `; use ${record.replacedBy}` : '' + if (record.modified) { + act(record.name, 'kept', `deprecated${replacement}, but edited locally`) + } else { + await fs.rm(dir, { recursive: true, force: true }) + act(record.name, 'removed', `deprecated${replacement}`) + } + break + } + case 'duplicate': + act(record.name, 'ignored', `copy of ${record.currentName} under another name`) + break + case 'unknown': + act(record.name, 'ignored', 'not a Netlify skill') + break + } + } + + for (const name of before.missing) { + if (installed.has(name)) continue + const skill = skillByName(name) + await installSkill(host, directory, skill) + act(name, 'added', skill.version ?? undefined) + } + + return { directory, actions } +} + +export const resolveSkillsDirectories = async ( + workingDir: string, + env: NodeJS.ProcessEnv = process.env, +): Promise => { + const present: string[] = [] + for (const agentDirectory of AGENT_DIRECTORIES) { + if (await isDirectory(path.join(workingDir, agentDirectory))) { + present.push(path.join(agentDirectory, 'skills')) + } + } + if (present.length > 0) { + return present + } + const drivingAgent = getDrivingAgent(env) + const agentDirectory = drivingAgent ? AGENT_DIRECTORY_BY_DRIVING_AGENT[drivingAgent.name] : undefined + return [agentDirectory ? path.join(agentDirectory, 'skills') : DEFAULT_SKILLS_DIRECTORY] +} + +const summarize = (actions: SkillActionRecord[]): Record => { + const summary: Record = { + current: 0, + added: 0, + updated: 0, + renamed: 0, + removed: 0, + kept: 0, + ignored: 0, + } + for (const { action } of actions) { + summary[action] += 1 + } + return summary +} + +const describeSync = ({ directory, actions }: SkillsSyncResult): string => { + const summary = summarize(actions) + const changed = summary.added + summary.updated + summary.renamed + summary.removed + const location = chalk.underline(directory) + if (changed === 0) { + return `Netlify skills in ${location} are up to date.` + } + const parts = [ + summary.added > 0 ? `${summary.added.toString()} added` : '', + summary.updated > 0 ? `${summary.updated.toString()} updated` : '', + summary.renamed > 0 ? `${summary.renamed.toString()} renamed` : '', + summary.removed > 0 ? `${summary.removed.toString()} removed` : '', + summary.kept > 0 ? `${summary.kept.toString()} kept (edited locally)` : '', + ].filter(Boolean) + return `Installed Netlify skills in ${location} (${parts.join(', ')}).` +} + +export interface AgentSkillsSetupSummary { + installed: boolean + directories: string[] + skillsVersion?: string + summary: Record + error?: string +} + +export const setupAgentSkills = async ({ + workingDir, + env = process.env, +}: { + workingDir: string + env?: NodeJS.ProcessEnv +}): Promise => { + const directories = await resolveSkillsDirectories(workingDir, env) + try { + const host = resolveSkillsHost(env) + const manifest = await fetchSkillsManifest(host) + const actions: SkillActionRecord[] = [] + for (const directory of directories) { + const result = await syncSkills({ host, directory: path.resolve(workingDir, directory), manifest }) + actions.push(...result.actions) + log(describeSync({ directory, actions: result.actions })) + } + return { installed: true, directories, skillsVersion: manifest.version, summary: summarize(actions) } + } catch (error) { + const message = error instanceof Error ? error.message : String(error) + warn(`Could not set up Netlify skills for AI agents: ${message}`) + log( + `Run ${chalk.cyanBright.bold(`${netlifyCommand()} init`)} again later, or use ${chalk.cyan('--skip-agent-setup')} to opt out.`, + ) + return { installed: false, directories, summary: summarize([]), error: message } + } +} diff --git a/tests/integration/commands/init/init.test.ts b/tests/integration/commands/init/init.test.ts index dc1c3a1ce48..0a403887c32 100644 --- a/tests/integration/commands/init/init.test.ts +++ b/tests/integration/commands/init/init.test.ts @@ -1,5 +1,8 @@ -import path from 'node:path' +import { createHash } from 'node:crypto' import { readFile } from 'node:fs/promises' +import { createServer } from 'node:http' +import type { AddressInfo } from 'node:net' +import path from 'node:path' import cleanDeep from 'clean-deep' import execa from 'execa' @@ -13,6 +16,62 @@ import { withSiteBuilder } from '../../utils/site-builder.js' const defaultFunctionsDirectory = 'netlify/functions' +const SKILL_CONTENT = '# netlify-functions\n' + +const sha256 = (content: string) => `sha256:${createHash('sha256').update(content).digest('hex')}` + +const skillsManifest = () => { + const fileHash = sha256(SKILL_CONTENT) + const treeHash = `sha256:${createHash('sha256') + .update(`SKILL.md\u0000100644\u0000${fileHash.replace(/^sha256:/, '')}\n`) + .digest('hex')}` + return { + schema_version: 1, + version: '1.0.0', + skills: [ + { + name: 'netlify-functions', + status: 'active', + version: '1.0.0', + prior_names: [], + description: 'Netlify Functions', + tree_hash: treeHash, + files: { 'SKILL.md': fileHash }, + executable: [], + history: [{ version: '1.0.0', tree_hash: treeHash }], + }, + ], + } +} + +const withSkillsHost = async (handler: (host: { url: string; requests: string[] }) => Promise) => { + const requests: string[] = [] + const responses = new Map([ + ['/manifest.json', JSON.stringify(skillsManifest())], + ['/skills/netlify-functions/SKILL.md', SKILL_CONTENT], + ]) + const server = createServer((req, res) => { + const url = req.url ?? '' + requests.push(url) + const body = responses.get(url) + res.statusCode = body === undefined ? 404 : 200 + res.end(body ?? 'not found') + }) + await new Promise((resolve) => { + server.listen(0, '127.0.0.1', resolve) + }) + const { port } = server.address() as AddressInfo + try { + await handler({ url: `http://127.0.0.1:${port.toString()}`, requests }) + } finally { + await new Promise((resolve) => { + server.close(() => { + resolve() + }) + }) + } +} + const assertNetlifyToml = async ( t: TestContext, tomlDir: string, @@ -102,7 +161,7 @@ describe.concurrent('commands/init', () => { await withMockApi(routes, async ({ apiUrl }) => { // --force is required since we return an existing site in the `sites` route // --manual is used to avoid the config-github flow that uses GitHub API - const childProcess = execa(cliPath, ['init', '--force', '--manual'], { + const childProcess = execa(cliPath, ['init', '--force', '--manual', '--skip-agent-setup'], { cwd: builder.directory, // NETLIFY_SITE_ID and NETLIFY_AUTH_TOKEN are required for @netlify/config to retrieve site info env: { NETLIFY_API_URL: apiUrl, NETLIFY_SITE_ID: 'site_id', NETLIFY_AUTH_TOKEN: 'fake-token' }, @@ -198,7 +257,7 @@ describe.concurrent('commands/init', () => { await withMockApi(routes, async ({ apiUrl }) => { // --manual is used to avoid the config-github flow that uses GitHub API - const childProcess = execa(cliPath, ['init', '--manual'], { + const childProcess = execa(cliPath, ['init', '--manual', '--skip-agent-setup'], { cwd: builder.directory, env: { NETLIFY_API_URL: apiUrl, NETLIFY_AUTH_TOKEN: 'fake-token' }, encoding: 'utf8', @@ -277,7 +336,7 @@ describe.concurrent('commands/init', () => { await builder.build() await withMockApi(routes, async ({ apiUrl }) => { - const childProcess = execa(cliPath, ['init'], { + const childProcess = execa(cliPath, ['init', '--skip-agent-setup'], { cwd: builder.directory, env: { NETLIFY_API_URL: apiUrl, NETLIFY_AUTH_TOKEN: 'fake-token' }, encoding: 'utf8', @@ -385,7 +444,7 @@ describe.concurrent('commands/init', () => { await withMockApi(routes, async ({ apiUrl }) => { // --manual is used to avoid the config-github flow that uses GitHub API - const childProcess = execa(cliPath, ['init', '--manual'], { + const childProcess = execa(cliPath, ['init', '--manual', '--skip-agent-setup'], { cwd: builder.directory, env: { NETLIFY_API_URL: apiUrl, NETLIFY_AUTH_TOKEN: 'fake-token' }, }) @@ -490,7 +549,7 @@ describe.concurrent('commands/init', () => { await withMockApi(routes, async ({ apiUrl }) => { // --manual is used to avoid the config-github flow that uses GitHub API - const childProcess = execa(cliPath, ['init', '--manual'], { + const childProcess = execa(cliPath, ['init', '--manual', '--skip-agent-setup'], { cwd: builder.directory, env: { NETLIFY_API_URL: apiUrl, NETLIFY_AUTH_TOKEN: 'fake-token' }, }) @@ -581,7 +640,7 @@ describe.concurrent('commands/init', () => { await withMockApi(routes, async ({ apiUrl }) => { // --force is required since we return an existing site in the `sites` route // --manual is used to avoid the config-github flow that uses GitHub API - const childProcess = execa(cliPath, ['init', '--force', '--manual'], { + const childProcess = execa(cliPath, ['init', '--force', '--manual', '--skip-agent-setup'], { cwd: builder.directory, // NETLIFY_SITE_ID and NETLIFY_AUTH_TOKEN are required for @netlify/config to retrieve site info env: { NETLIFY_API_URL: apiUrl, NETLIFY_SITE_ID: 'site_id', NETLIFY_AUTH_TOKEN: 'fake-token' }, @@ -687,7 +746,7 @@ describe.concurrent('commands/init', () => { await withMockApi(routes, async ({ apiUrl }) => { // --manual is used to avoid the config-github flow that uses GitHub API - const childProcess = execa(cliPath, ['init', '--manual'], { + const childProcess = execa(cliPath, ['init', '--manual', '--skip-agent-setup'], { cwd: builder.directory, env: { NETLIFY_API_URL: apiUrl, NETLIFY_AUTH_TOKEN: 'fake-token' }, }) @@ -700,4 +759,70 @@ describe.concurrent('commands/init', () => { }) }) }) + + test('netlify init installs Netlify skills for AI agents by default and is idempotent', async (t) => { + const siteInfo = { + admin_url: 'https://app.netlify.com/projects/site-name/overview', + ssl_url: 'https://site-name.netlify.app/', + id: 'site_id', + name: 'site-name', + build_settings: { repo_url: 'https://github.com/owner/repo' }, + } + const routes = [ + { path: 'accounts', response: [{ slug: 'test-account' }] }, + { path: 'sites/site_id/service-instances', response: [] }, + { path: 'sites/site_id', response: siteInfo }, + { path: 'sites', response: [siteInfo] }, + { path: 'deploy_keys', method: 'POST' as const, response: { public_key: 'public_key' } }, + { path: 'sites/site_id', method: 'PATCH' as const, response: { deploy_hook: 'deploy_hook' } }, + ] + const manualQuestions = () => [ + { question: 'Your build command (hugo build/yarn run build/etc)', answer: answerWithValue('npm run build') }, + { question: 'Directory to deploy (blank for current dir)', answer: answerWithValue('dist') }, + { question: 'No netlify.toml detected', answer: CONFIRM }, + { question: 'Give this Netlify SSH public key access to your repository', answer: CONFIRM }, + { question: 'The SSH URL of the remote git repo', answer: CONFIRM }, + { question: 'Configure the following webhook for your repository', answer: CONFIRM }, + ] + + await withSiteBuilder(t, async (builder) => { + await builder.withGit().ensureDirectoryExists(path.join(builder.directory, '.agents')).build() + + await withMockApi(routes, async ({ apiUrl }) => { + await withSkillsHost(async (skillsHost) => { + const env = { + NETLIFY_API_URL: apiUrl, + NETLIFY_SITE_ID: 'site_id', + NETLIFY_AUTH_TOKEN: 'fake-token', + NETLIFY_SKILLS_HOST: skillsHost.url, + } + const skillPath = path.join(builder.directory, '.agents', 'skills', 'netlify-functions', 'SKILL.md') + const runInit = async (...flags: string[]) => { + const childProcess = execa(cliPath, ['init', '--manual', ...flags], { cwd: builder.directory, env }) + if (process.env.DEBUG_TESTS) { + childProcess.stdout?.on('data', (data: Buffer) => { + process.stderr.write(data) + }) + } + handleQuestions(childProcess, manualQuestions()) + return await childProcess + } + + const first = await runInit() + t.expect(first.stdout).toContain('Installed Netlify skills') + await t.expect(readFile(skillPath, 'utf8')).resolves.toBe(SKILL_CONTENT) + const requestsAfterFirstRun = skillsHost.requests.length + + const second = await runInit() + t.expect(second.stdout).toContain('are up to date') + t.expect(skillsHost.requests.slice(requestsAfterFirstRun)).toEqual(['/manifest.json']) + await t.expect(readFile(skillPath, 'utf8')).resolves.toBe(SKILL_CONTENT) + + const skipped = await runInit('--skip-agent-setup') + t.expect(skipped.stdout).not.toContain('Netlify skills') + t.expect(skillsHost.requests.length).toBe(requestsAfterFirstRun + 1) + }) + }) + }) + }) }) diff --git a/tests/unit/utils/init/agent-skills.test.ts b/tests/unit/utils/init/agent-skills.test.ts new file mode 100644 index 00000000000..16dc803646b --- /dev/null +++ b/tests/unit/utils/init/agent-skills.test.ts @@ -0,0 +1,369 @@ +import { createHash } from 'node:crypto' +import { chmod, mkdir, mkdtemp, readFile, readdir, rm, stat, writeFile } from 'node:fs/promises' +import { createServer, type Server } from 'node:http' +import type { AddressInfo } from 'node:net' +import { tmpdir } from 'node:os' +import { join } from 'node:path' + +import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest' + +import { + DEFAULT_SKILLS_DIRECTORY, + DEFAULT_SKILLS_HOST, + type ManifestSkill, + type SkillsManifest, + classifySkillsDirectory, + fetchSkillsManifest, + hashSkillTree, + installSkill, + resolveSkillsDirectories, + resolveSkillsHost, + setupAgentSkills, + syncSkills, +} from '../../../../src/utils/init/agent-skills.js' + +vi.mock('../../../../src/utils/command-helpers.js', async (importOriginal) => ({ + ...(await importOriginal()), + log: vi.fn(), + warn: vi.fn(), +})) + +type Files = Record + +interface SkillSpec { + name: string + files: Files + executable?: string[] + priorNames?: string[] + previous?: { version: string; files: Files }[] +} + +const sha256 = (content: string | Uint8Array) => `sha256:${createHash('sha256').update(content).digest('hex')}` + +const treeHashOf = (files: Files, executable: string[] = []) => { + const hash = createHash('sha256') + for (const file of Object.keys(files).sort()) { + const mode = executable.includes(file) ? '100755' : '100644' + hash.update(`${file}\0${mode}\0${sha256(files[file]).replace(/^sha256:/, '')}\n`) + } + return `sha256:${hash.digest('hex')}` +} + +const activeSkill = ({ name, files, executable = [], priorNames = [], previous = [] }: SkillSpec): ManifestSkill => ({ + name, + status: 'active', + version: '2.0.0', + prior_names: priorNames, + description: `${name} skill`, + tree_hash: treeHashOf(files, executable), + files: Object.fromEntries(Object.entries(files).map(([file, content]) => [file, sha256(content)])), + executable, + history: [ + ...previous.map(({ version, files: oldFiles }) => ({ version, tree_hash: treeHashOf(oldFiles) })), + { version: '2.0.0', tree_hash: treeHashOf(files, executable) }, + ], +}) + +const deprecatedSkill = (name: string, shipped: Files, replacedBy?: string): ManifestSkill => ({ + name, + status: 'deprecated', + version: null, + prior_names: [], + description: `${name} is retired`, + tree_hash: null, + files: {}, + executable: [], + history: [{ version: '1.0.0', tree_hash: treeHashOf(shipped) }], + deprecated: { since: '2.0.0', ...(replacedBy ? { replaced_by: replacedBy } : {}) }, +}) + +const FUNCTIONS_V1: Files = { 'SKILL.md': '# functions v1\n' } + +const FUNCTIONS: SkillSpec = { + name: 'netlify-functions', + files: { 'SKILL.md': '# functions v2\n', 'references/routing.md': '# routing\n' }, + previous: [{ version: '1.0.0', files: FUNCTIONS_V1 }], +} + +const DEPLOY: SkillSpec = { + name: 'netlify-deploy', + files: { 'SKILL.md': '# deploy\n', 'scripts/deploy.sh': '#!/bin/sh\necho deploy\n' }, + executable: ['scripts/deploy.sh'], + priorNames: ['netlify-cli-and-deploy'], +} + +const RETIRED_FILES: Files = { 'SKILL.md': '# retired\n' } + +class SkillsHost { + private server: Server | undefined + private readonly responses = new Map() + readonly requests: string[] = [] + readonly manifest: SkillsManifest + + constructor(skills: SkillSpec[], extraSkills: ManifestSkill[] = []) { + this.manifest = { + schema_version: 1, + version: '2.0.0', + skills: [...skills.map(activeSkill), ...extraSkills], + } + this.responses.set('/manifest.json', Buffer.from(JSON.stringify(this.manifest))) + for (const { name, files } of skills) { + for (const [file, content] of Object.entries(files)) { + this.responses.set(`/skills/${name}/${file}`, Buffer.from(content)) + } + } + } + + corrupt(path: string, content: string) { + this.responses.set(path, Buffer.from(content)) + } + + async start(): Promise { + this.server = createServer((req, res) => { + const url = decodeURIComponent(req.url ?? '') + this.requests.push(url) + const body = this.responses.get(url) + if (!body) { + res.statusCode = 404 + res.end('not found') + return + } + res.end(body) + }) + await new Promise((resolve) => this.server?.listen(0, '127.0.0.1', resolve)) + const { port } = this.server.address() as AddressInfo + return `http://127.0.0.1:${port.toString()}` + } + + async stop() { + await new Promise((resolve, reject) => { + this.server?.close((error) => { + if (error) { + reject(error) + } else { + resolve() + } + }) + }) + } +} + +const writeSkill = async (root: string, name: string, files: Files, executable: string[] = []) => { + for (const [file, content] of Object.entries(files)) { + const target = join(root, name, ...file.split('/')) + await mkdir(join(target, '..'), { recursive: true }) + await writeFile(target, content) + await chmod(target, executable.includes(file) ? 0o755 : 0o644) + } +} + +const listDirectories = async (root: string) => + (await readdir(root, { withFileTypes: true })) + .filter((entry) => entry.isDirectory()) + .map((entry) => entry.name) + .sort() + +describe('agent skills', () => { + let projectDir: string + let skillsDir: string + + beforeEach(async () => { + projectDir = await mkdtemp(join(tmpdir(), 'agent-skills-')) + skillsDir = join(projectDir, '.agents', 'skills') + }) + + afterEach(async () => { + await rm(projectDir, { recursive: true, force: true }) + }) + + describe('resolveSkillsHost', () => { + test('defaults to the hosted skills site', () => { + expect(resolveSkillsHost({})).toBe(DEFAULT_SKILLS_HOST) + }) + + test('accepts an https override and strips the trailing slash', () => { + expect(resolveSkillsHost({ NETLIFY_SKILLS_HOST: 'https://skills.example.com/' })).toBe( + 'https://skills.example.com', + ) + }) + + test('accepts http for loopback only', () => { + expect(resolveSkillsHost({ NETLIFY_SKILLS_HOST: 'http://localhost:9999' })).toBe('http://localhost:9999') + expect(() => resolveSkillsHost({ NETLIFY_SKILLS_HOST: 'http://skills.example.com' })).toThrow(/https/) + }) + }) + + describe('resolveSkillsDirectories', () => { + test('defaults to .agents/skills when no agent directory exists', async () => { + await expect(resolveSkillsDirectories(projectDir, {})).resolves.toEqual([DEFAULT_SKILLS_DIRECTORY]) + }) + + test('uses every agent directory already present in the project', async () => { + await mkdir(join(projectDir, '.claude')) + await mkdir(join(projectDir, '.grok')) + await expect(resolveSkillsDirectories(projectDir, {})).resolves.toEqual([ + join('.claude', 'skills'), + join('.grok', 'skills'), + ]) + }) + + test('falls back to the driving agent location when nothing exists yet', async () => { + await expect(resolveSkillsDirectories(projectDir, { NETLIFY_AGENT: 'claude-code' })).resolves.toEqual([ + join('.claude', 'skills'), + ]) + await expect(resolveSkillsDirectories(projectDir, { NETLIFY_AGENT: 'cursor' })).resolves.toEqual([ + DEFAULT_SKILLS_DIRECTORY, + ]) + }) + }) + + describe('hashSkillTree', () => { + test('matches the manifest tree_hash formula including the executable bit', async () => { + await writeSkill(skillsDir, DEPLOY.name, DEPLOY.files, DEPLOY.executable) + await expect(hashSkillTree(join(skillsDir, DEPLOY.name))).resolves.toBe( + treeHashOf(DEPLOY.files, DEPLOY.executable), + ) + }) + }) + + describe('syncing against a hosted release', () => { + let host: SkillsHost + let hostUrl: string + + beforeEach(async () => { + host = new SkillsHost([FUNCTIONS, DEPLOY], [deprecatedSkill('netlify-legacy', RETIRED_FILES, 'netlify-deploy')]) + hostUrl = await host.start() + }) + + afterEach(async () => { + await host.stop() + }) + + test('installs every active skill into an empty directory with verified bytes and modes', async () => { + const manifest = await fetchSkillsManifest(hostUrl) + const { actions } = await syncSkills({ host: hostUrl, directory: skillsDir, manifest }) + + expect(actions).toEqual([ + { name: 'netlify-deploy', action: 'added', detail: '2.0.0' }, + { name: 'netlify-functions', action: 'added', detail: '2.0.0' }, + ]) + await expect(listDirectories(skillsDir)).resolves.toEqual(['netlify-deploy', 'netlify-functions']) + await expect(readFile(join(skillsDir, 'netlify-functions', 'references', 'routing.md'), 'utf8')).resolves.toBe( + '# routing\n', + ) + const script = await stat(join(skillsDir, 'netlify-deploy', 'scripts', 'deploy.sh')) + expect(script.mode & 0o111).not.toBe(0) + }) + + test('is idempotent: a second run reports everything current and changes nothing', async () => { + const manifest = await fetchSkillsManifest(hostUrl) + await syncSkills({ host: hostUrl, directory: skillsDir, manifest }) + const before = await stat(join(skillsDir, 'netlify-functions', 'SKILL.md')) + const requestsAfterInstall = host.requests.length + + const { actions } = await syncSkills({ host: hostUrl, directory: skillsDir, manifest }) + + expect(actions).toEqual([ + { name: 'netlify-deploy', action: 'current', detail: '2.0.0' }, + { name: 'netlify-functions', action: 'current', detail: '2.0.0' }, + ]) + const after = await stat(join(skillsDir, 'netlify-functions', 'SKILL.md')) + expect(after.mtimeMs).toBe(before.mtimeMs) + expect(host.requests.length).toBe(requestsAfterInstall) + }) + + test('replaces a stale copy and keeps a locally edited one', async () => { + await writeSkill(skillsDir, FUNCTIONS.name, FUNCTIONS_V1) + await writeSkill(skillsDir, DEPLOY.name, { ...DEPLOY.files, 'SKILL.md': '# my own notes\n' }) + const manifest = await fetchSkillsManifest(hostUrl) + + const classification = await classifySkillsDirectory(skillsDir, manifest) + expect(classification.skills).toEqual([ + { name: 'netlify-deploy', status: 'modified', version: '2.0.0' }, + { name: 'netlify-functions', status: 'stale', version: '2.0.0', have: '1.0.0' }, + ]) + + const { actions } = await syncSkills({ host: hostUrl, directory: skillsDir, manifest }) + expect(actions).toEqual([ + { name: 'netlify-deploy', action: 'kept', detail: 'edited locally' }, + { name: 'netlify-functions', action: 'updated', detail: '1.0.0 -> 2.0.0' }, + ]) + await expect(readFile(join(skillsDir, 'netlify-functions', 'SKILL.md'), 'utf8')).resolves.toBe('# functions v2\n') + await expect(readFile(join(skillsDir, 'netlify-deploy', 'SKILL.md'), 'utf8')).resolves.toBe('# my own notes\n') + }) + + test('migrates a skill installed under a prior name', async () => { + await writeSkill(skillsDir, 'netlify-cli-and-deploy', DEPLOY.files, DEPLOY.executable) + const manifest = await fetchSkillsManifest(hostUrl) + + const { actions } = await syncSkills({ host: hostUrl, directory: skillsDir, manifest }) + + expect(actions).toContainEqual({ name: 'netlify-cli-and-deploy', action: 'renamed', detail: '-> netlify-deploy' }) + expect(actions.filter(({ name }) => name === 'netlify-deploy')).toEqual([]) + await expect(listDirectories(skillsDir)).resolves.toEqual(['netlify-deploy', 'netlify-functions']) + }) + + test('removes an unedited deprecated skill and leaves edited or unknown directories alone', async () => { + await writeSkill(skillsDir, 'netlify-legacy', RETIRED_FILES) + await writeSkill(skillsDir, 'my-team-skill', { 'SKILL.md': '# ours\n' }) + const manifest = await fetchSkillsManifest(hostUrl) + + const { actions } = await syncSkills({ host: hostUrl, directory: skillsDir, manifest }) + + expect(actions).toContainEqual({ name: 'my-team-skill', action: 'ignored', detail: 'not a Netlify skill' }) + expect(actions).toContainEqual({ + name: 'netlify-legacy', + action: 'removed', + detail: 'deprecated; use netlify-deploy', + }) + await expect(listDirectories(skillsDir)).resolves.toEqual([ + 'my-team-skill', + 'netlify-deploy', + 'netlify-functions', + ]) + + await writeSkill(skillsDir, 'netlify-legacy', { 'SKILL.md': '# retired but edited\n' }) + const second = await syncSkills({ host: hostUrl, directory: skillsDir, manifest }) + expect(second.actions).toContainEqual({ + name: 'netlify-legacy', + action: 'kept', + detail: 'deprecated; use netlify-deploy, but edited locally', + }) + }) + + test('rejects a file whose bytes do not match the manifest and leaves no partial install', async () => { + host.corrupt('/skills/netlify-functions/SKILL.md', '# tampered\n') + const manifest = await fetchSkillsManifest(hostUrl) + const functions = manifest.skills.find(({ name }) => name === 'netlify-functions') + if (!functions) throw new Error('netlify-functions missing from manifest') + + await expect(installSkill(hostUrl, skillsDir, functions)).rejects.toThrow(/hash mismatch/) + await expect(stat(skillsDir)).rejects.toThrow(/ENOENT/) + }) + + test('setupAgentSkills installs into the detected directories and reports a summary', async () => { + await mkdir(join(projectDir, '.claude')) + const result = await setupAgentSkills({ workingDir: projectDir, env: { NETLIFY_SKILLS_HOST: hostUrl } }) + + expect(result.installed).toBe(true) + expect(result.directories).toEqual([join('.claude', 'skills')]) + expect(result.skillsVersion).toBe('2.0.0') + expect(result.summary.added).toBe(2) + await expect(listDirectories(join(projectDir, '.claude', 'skills'))).resolves.toEqual([ + 'netlify-deploy', + 'netlify-functions', + ]) + }) + }) + + test('setupAgentSkills does not throw when the host is unreachable', async () => { + const result = await setupAgentSkills({ + workingDir: projectDir, + env: { NETLIFY_SKILLS_HOST: 'http://127.0.0.1:1' }, + }) + + expect(result.installed).toBe(false) + expect(result.error).toBeDefined() + await expect(readdir(projectDir)).resolves.toEqual([]) + }) +}) From a4ce37d04bda33e341e3d4a61be4ef70f61ed907 Mon Sep 17 00:00:00 2001 From: Domitrius Clark Date: Fri, 2 Oct 2026 12:02:25 -0400 Subject: [PATCH 2/6] fix(init): bound skill downloads, handle symlinks and leftovers Review fixes: 10s per-request and 120s overall fetch deadline, follow a symlinked skills root and never touch symlinked skill entries, sweep staging and backup directories from an interrupted install, reinstall over an empty skill directory, refuse unknown manifest schemas, and install relative to the repository root. Co-Authored-By: Claude Fable 5.1 --- src/commands/init/init.ts | 2 +- src/utils/init/agent-skills.ts | 105 ++++++++++++++++----- tests/unit/utils/init/agent-skills.test.ts | 100 +++++++++++++++++++- 3 files changed, 183 insertions(+), 24 deletions(-) diff --git a/src/commands/init/init.ts b/src/commands/init/init.ts index 48576193afc..ec4b054d6ff 100644 --- a/src/commands/init/init.ts +++ b/src/commands/init/init.ts @@ -229,7 +229,7 @@ type InitExtraOptions = { const installAgentSkills = async (command: BaseCommand): Promise => { log() - const result = await setupAgentSkills({ workingDir: command.workingDir }) + const result = await setupAgentSkills({ workingDir: command.netlify.repositoryRoot }) await track('sites_agentSkillsSetup', { installed: result.installed, directories: result.directories, diff --git a/src/utils/init/agent-skills.ts b/src/utils/init/agent-skills.ts index 261d14a39ba..1303b599372 100644 --- a/src/utils/init/agent-skills.ts +++ b/src/utils/init/agent-skills.ts @@ -12,10 +12,15 @@ export const DEFAULT_SKILLS_DIRECTORY = path.join('.agents', 'skills') const AGENT_DIRECTORIES = ['.claude', '.agents', '.grok'] as const const AGENT_DIRECTORY_BY_DRIVING_AGENT: Partial> = { claude: '.claude', - claudeai: '.claude', } +const SUPPORTED_SCHEMA_VERSION = 1 const SKILL_NAME = /^[A-Za-z0-9_][A-Za-z0-9_.-]*$/ +const STAGING_PREFIX = '.netlify-skill-' +const STAGING_LEFTOVER = /^\.netlify-skill-.+-[A-Za-z0-9]{6}$/ +const RETIRED_LEFTOVER = /^(.+)\.old-\d+-[0-9a-f]{12}$/ +const FETCH_TIMEOUT_MS = 10_000 +const SETUP_TIMEOUT_MS = 120_000 const LOOPBACK_HOSTS = new Set(['localhost', '127.0.0.1', '[::1]']) export interface SkillHistoryEntry { @@ -72,6 +77,8 @@ export interface SkillsSyncResult { class SkillsError extends Error {} +const errorMessage = (error: unknown): string => (error instanceof Error ? error.message : String(error)) + const sha256 = (bytes: Uint8Array): string => `sha256:${createHash('sha256').update(bytes).digest('hex')}` export const resolveSkillsHost = (env: NodeJS.ProcessEnv = process.env): string => { @@ -90,8 +97,12 @@ export const resolveSkillsHost = (env: NodeJS.ProcessEnv = process.env): string const urlFor = (host: string, ...parts: string[]): string => `${host}/${parts.map((part) => encodeURIComponent(part)).join('/')}` -const fetchBytes = async (url: string): Promise => { - const response = await fetch(url, { headers: { 'user-agent': `NetlifyCLI ${version}` } }) +const fetchBytes = async (url: string, signal?: AbortSignal): Promise => { + const requestTimeout = AbortSignal.timeout(FETCH_TIMEOUT_MS) + const response = await fetch(url, { + headers: { 'user-agent': `NetlifyCLI ${version}` }, + signal: signal ? AbortSignal.any([requestTimeout, signal]) : requestTimeout, + }) if (!response.ok) { throw new SkillsError(`${url}: HTTP ${response.status.toString()}`) } @@ -143,15 +154,20 @@ const indexManifest = (manifest: SkillsManifest): ManifestIndex => { return { exact, prior } } -export const fetchSkillsManifest = async (host: string): Promise => { +export const fetchSkillsManifest = async (host: string, signal?: AbortSignal): Promise => { const url = urlFor(host, 'manifest.json') - const bytes = await fetchBytes(url) + const bytes = await fetchBytes(url, signal) let manifest: SkillsManifest try { manifest = JSON.parse(Buffer.from(bytes).toString('utf8')) as SkillsManifest } catch { throw new SkillsError(`${url}: invalid JSON`) } + if (manifest.schema_version !== SUPPORTED_SCHEMA_VERSION) { + throw new SkillsError( + `${url}: manifest schema ${String(manifest.schema_version)} is not supported by this CLI version; update the Netlify CLI`, + ) + } indexManifest(manifest) return manifest } @@ -212,6 +228,14 @@ const isDirectory = async (dir: string): Promise => { } } +const isDirectoryOrLinkToOne = async (dir: string): Promise => { + try { + return (await fs.stat(dir)).isDirectory() + } catch { + return false + } +} + const sortByName = (entries: T[]): T[] => [...entries].sort((a, b) => a.name.localeCompare(b.name, 'en')) @@ -224,19 +248,20 @@ export const classifySkillsDirectory = async ( const presentActive = new Set() let entries: Dirent[] = [] - if (await isDirectory(root)) { + if (await isDirectoryOrLinkToOne(root)) { entries = sortByName(await fs.readdir(root, { withFileTypes: true })) } for (const entry of entries) { - if (!entry.isDirectory()) continue const known = exact.get(entry.name) const renamed = prior.get(entry.name) const target = known?.status === 'active' ? known : renamed?.status === 'active' ? renamed : null const retired = known?.status === 'deprecated' ? known : renamed?.status === 'deprecated' ? renamed : null + if (!entry.isDirectory() && !target && !retired) continue const dir = path.join(root, entry.name) - const hasSkillMd = await isFile(path.join(dir, 'SKILL.md')) + const hasSkillMd = entry.isDirectory() && (await isFile(path.join(dir, 'SKILL.md'))) if (!hasSkillMd && !target && !retired) continue + if (entry.isDirectory() && !hasSkillMd && target && (await fs.readdir(dir)).length === 0) continue let treeHash: string | null = null if (hasSkillMd) { @@ -315,14 +340,19 @@ const replaceDirectory = async (staged: string, target: string): Promise = } } -export const installSkill = async (host: string, dest: string, skill: ManifestSkill): Promise => { +export const installSkill = async ( + host: string, + dest: string, + skill: ManifestSkill, + signal?: AbortSignal, +): Promise => { const files = Object.keys(skill.files ?? {}).sort() const downloaded: [string, Uint8Array][] = [] for (const file of files) { if (!isSafeFilePath(file)) { throw new SkillsError(`${skill.name}: unsafe manifest file path: ${file}`) } - const bytes = await fetchBytes(urlFor(host, 'skills', skill.name, ...file.split('/'))) + const bytes = await fetchBytes(urlFor(host, 'skills', skill.name, ...file.split('/')), signal) if (sha256(bytes) !== skill.files?.[file]) { throw new SkillsError(`${skill.name}/${file}: hash mismatch`) } @@ -330,7 +360,7 @@ export const installSkill = async (host: string, dest: string, skill: ManifestSk } await fs.mkdir(dest, { recursive: true }) - const staged = await fs.mkdtemp(path.join(dest, `.netlify-skill-${skill.name}-`)) + const staged = await fs.mkdtemp(path.join(dest, `${STAGING_PREFIX}${skill.name}-`)) try { const executable = new Set(skill.executable ?? []) for (const [file, bytes] of downloaded) { @@ -341,22 +371,46 @@ export const installSkill = async (host: string, dest: string, skill: ManifestSk await replaceDirectory(staged, path.join(dest, skill.name)) } catch (error) { await fs.rm(staged, { recursive: true, force: true }) - throw new SkillsError(`${skill.name}: could not install: ${(error as Error).message}`) + throw new SkillsError(`${skill.name}: could not install: ${errorMessage(error)}`) } return files.length } +const isInstallLeftover = (name: string, { exact, prior }: ManifestIndex): boolean => { + if (STAGING_LEFTOVER.test(name)) { + return true + } + const retired = RETIRED_LEFTOVER.exec(name) + return retired !== null && (exact.has(retired[1]) || prior.has(retired[1])) +} + +const removeInstallLeftovers = async (root: string, index: ManifestIndex): Promise => { + if (!(await isDirectoryOrLinkToOne(root))) { + return [] + } + const removed: string[] = [] + for (const entry of await fs.readdir(root, { withFileTypes: true })) { + if (entry.isDirectory() && isInstallLeftover(entry.name, index)) { + await fs.rm(path.join(root, entry.name), { recursive: true, force: true }) + removed.push(entry.name) + } + } + return removed.sort() +} + export const syncSkills = async ({ host, directory, manifest, + signal, }: { host: string directory: string manifest: SkillsManifest + signal?: AbortSignal }): Promise => { - const { exact } = indexManifest(manifest) - const before = await classifySkillsDirectory(directory, manifest) + const index = indexManifest(manifest) + const { exact } = index const actions: SkillActionRecord[] = [] const installed = new Set() const act = (name: string, action: SkillAction, detail?: string) => { @@ -370,6 +424,11 @@ export const syncSkills = async ({ return skill } + for (const leftover of await removeInstallLeftovers(directory, index)) { + act(leftover, 'removed', 'leftover from an interrupted install') + } + const before = await classifySkillsDirectory(directory, manifest) + for (const record of before.skills) { const dir = path.join(directory, record.name) switch (record.status) { @@ -378,7 +437,7 @@ export const syncSkills = async ({ break case 'stale': { const skill = skillByName(record.name) - await installSkill(host, directory, skill) + await installSkill(host, directory, skill, signal) act(record.name, 'updated', `${record.have ?? 'unknown'} -> ${skill.version ?? 'latest'}`) break } @@ -395,7 +454,7 @@ export const syncSkills = async ({ await fs.rm(dir, { recursive: true, force: true }) act(record.name, 'removed', `superseded by ${record.currentName}`) } else { - await installSkill(host, directory, skillByName(record.currentName)) + await installSkill(host, directory, skillByName(record.currentName), signal) installed.add(record.currentName) await fs.rm(dir, { recursive: true, force: true }) act(record.name, 'renamed', `-> ${record.currentName}`) @@ -424,7 +483,7 @@ export const syncSkills = async ({ for (const name of before.missing) { if (installed.has(name)) continue const skill = skillByName(name) - await installSkill(host, directory, skill) + await installSkill(host, directory, skill, signal) act(name, 'added', skill.version ?? undefined) } @@ -437,7 +496,7 @@ export const resolveSkillsDirectories = async ( ): Promise => { const present: string[] = [] for (const agentDirectory of AGENT_DIRECTORIES) { - if (await isDirectory(path.join(workingDir, agentDirectory))) { + if (await isDirectoryOrLinkToOne(path.join(workingDir, agentDirectory))) { present.push(path.join(agentDirectory, 'skills')) } } @@ -497,19 +556,21 @@ export const setupAgentSkills = async ({ workingDir: string env?: NodeJS.ProcessEnv }): Promise => { - const directories = await resolveSkillsDirectories(workingDir, env) + let directories: string[] = [] try { + directories = await resolveSkillsDirectories(workingDir, env) const host = resolveSkillsHost(env) - const manifest = await fetchSkillsManifest(host) + const signal = AbortSignal.timeout(SETUP_TIMEOUT_MS) + const manifest = await fetchSkillsManifest(host, signal) const actions: SkillActionRecord[] = [] for (const directory of directories) { - const result = await syncSkills({ host, directory: path.resolve(workingDir, directory), manifest }) + const result = await syncSkills({ host, directory: path.resolve(workingDir, directory), manifest, signal }) actions.push(...result.actions) log(describeSync({ directory, actions: result.actions })) } return { installed: true, directories, skillsVersion: manifest.version, summary: summarize(actions) } } catch (error) { - const message = error instanceof Error ? error.message : String(error) + const message = errorMessage(error) warn(`Could not set up Netlify skills for AI agents: ${message}`) log( `Run ${chalk.cyanBright.bold(`${netlifyCommand()} init`)} again later, or use ${chalk.cyan('--skip-agent-setup')} to opt out.`, diff --git a/tests/unit/utils/init/agent-skills.test.ts b/tests/unit/utils/init/agent-skills.test.ts index 16dc803646b..6c362fe4114 100644 --- a/tests/unit/utils/init/agent-skills.test.ts +++ b/tests/unit/utils/init/agent-skills.test.ts @@ -1,5 +1,5 @@ import { createHash } from 'node:crypto' -import { chmod, mkdir, mkdtemp, readFile, readdir, rm, stat, writeFile } from 'node:fs/promises' +import { chmod, lstat, mkdir, mkdtemp, readFile, readdir, rm, stat, symlink, writeFile } from 'node:fs/promises' import { createServer, type Server } from 'node:http' import type { AddressInfo } from 'node:net' import { tmpdir } from 'node:os' @@ -331,6 +331,104 @@ describe('agent skills', () => { }) }) + test('removes the old directory when a renamed skill is already installed under its new name', async () => { + await writeSkill(skillsDir, 'netlify-cli-and-deploy', DEPLOY.files, DEPLOY.executable) + await writeSkill(skillsDir, DEPLOY.name, DEPLOY.files, DEPLOY.executable) + const manifest = await fetchSkillsManifest(hostUrl) + + const { actions } = await syncSkills({ host: hostUrl, directory: skillsDir, manifest }) + + expect(actions).toContainEqual({ + name: 'netlify-cli-and-deploy', + action: 'removed', + detail: 'superseded by netlify-deploy', + }) + expect(actions).toContainEqual({ name: 'netlify-deploy', action: 'current', detail: '2.0.0' }) + await expect(listDirectories(skillsDir)).resolves.toEqual(['netlify-deploy', 'netlify-functions']) + }) + + test('reinstalls over an empty directory carrying a skill name', async () => { + await mkdir(join(skillsDir, FUNCTIONS.name), { recursive: true }) + const manifest = await fetchSkillsManifest(hostUrl) + + const { actions } = await syncSkills({ host: hostUrl, directory: skillsDir, manifest }) + + expect(actions).toContainEqual({ name: 'netlify-functions', action: 'added', detail: '2.0.0' }) + await expect(readFile(join(skillsDir, 'netlify-functions', 'SKILL.md'), 'utf8')).resolves.toBe('# functions v2\n') + }) + + test('sweeps staging and backup directories left by an interrupted install', async () => { + await writeSkill(skillsDir, '.netlify-skill-netlify-functions-Ab12Cd', FUNCTIONS.files) + await writeSkill(skillsDir, 'netlify-deploy.old-4242-0123456789ab', DEPLOY.files) + await writeSkill(skillsDir, 'my-skill.old-4242-0123456789ab', { 'SKILL.md': '# mine\n' }) + const manifest = await fetchSkillsManifest(hostUrl) + + const { actions } = await syncSkills({ host: hostUrl, directory: skillsDir, manifest }) + + expect(actions.filter(({ action }) => action === 'removed')).toEqual([ + { + name: '.netlify-skill-netlify-functions-Ab12Cd', + action: 'removed', + detail: 'leftover from an interrupted install', + }, + { + name: 'netlify-deploy.old-4242-0123456789ab', + action: 'removed', + detail: 'leftover from an interrupted install', + }, + ]) + await expect(listDirectories(skillsDir)).resolves.toEqual([ + 'my-skill.old-4242-0123456789ab', + 'netlify-deploy', + 'netlify-functions', + ]) + }) + + test('refuses a manifest with an unsupported schema version', async () => { + host.corrupt('/manifest.json', JSON.stringify({ ...host.manifest, schema_version: 2 })) + + await expect(fetchSkillsManifest(hostUrl)).rejects.toThrow(/schema 2 is not supported/) + }) + + test('follows a symlinked skills root and still keeps edited copies there', async () => { + const sharedDir = join(projectDir, 'shared-skills') + await writeSkill(sharedDir, DEPLOY.name, { ...DEPLOY.files, 'SKILL.md': '# my own notes\n' }) + await mkdir(join(projectDir, '.agents')) + await symlink(sharedDir, skillsDir) + const manifest = await fetchSkillsManifest(hostUrl) + + const { actions } = await syncSkills({ host: hostUrl, directory: skillsDir, manifest }) + + expect(actions).toEqual([ + { name: 'netlify-deploy', action: 'kept', detail: 'edited locally' }, + { name: 'netlify-functions', action: 'added', detail: '2.0.0' }, + ]) + await expect(readFile(join(sharedDir, 'netlify-deploy', 'SKILL.md'), 'utf8')).resolves.toBe('# my own notes\n') + await expect(listDirectories(sharedDir)).resolves.toEqual(['netlify-deploy', 'netlify-functions']) + }) + + test('never replaces or deletes a skill entry that is a symlink', async () => { + const elsewhere = join(projectDir, 'elsewhere') + await writeSkill(elsewhere, 'functions-source', FUNCTIONS_V1) + await writeSkill(elsewhere, 'legacy-source', RETIRED_FILES) + await mkdir(skillsDir, { recursive: true }) + await symlink(join(elsewhere, 'functions-source'), join(skillsDir, 'netlify-functions')) + await symlink(join(elsewhere, 'legacy-source'), join(skillsDir, 'netlify-legacy')) + const manifest = await fetchSkillsManifest(hostUrl) + + const { actions } = await syncSkills({ host: hostUrl, directory: skillsDir, manifest }) + + expect(actions).toContainEqual({ name: 'netlify-functions', action: 'kept', detail: 'edited locally' }) + expect(actions).toContainEqual({ + name: 'netlify-legacy', + action: 'kept', + detail: 'deprecated; use netlify-deploy, but edited locally', + }) + await expect(lstat(join(skillsDir, 'netlify-functions'))).resolves.toMatchObject({}) + await expect(readFile(join(elsewhere, 'functions-source', 'SKILL.md'), 'utf8')).resolves.toBe('# functions v1\n') + await expect(readFile(join(elsewhere, 'legacy-source', 'SKILL.md'), 'utf8')).resolves.toBe('# retired\n') + }) + test('rejects a file whose bytes do not match the manifest and leaves no partial install', async () => { host.corrupt('/skills/netlify-functions/SKILL.md', '# tampered\n') const manifest = await fetchSkillsManifest(hostUrl) From dda854e33dfb18dd6cf9b342680d6c73d83426f0 Mon Sep 17 00:00:00 2001 From: Domitrius Clark Date: Fri, 2 Oct 2026 12:54:27 -0400 Subject: [PATCH 3/6] fix(init): prove ownership before sweeping skill leftovers Leftover staging and backup directories are removed only when they carry this command's ownership marker and are at least ten minutes old, so a user directory with a matching name or another run's in-flight install is never deleted. The staged tree is hash-verified before it replaces anything, and active manifest skills must carry a tree hash. Co-Authored-By: Claude Fable 5.1 --- src/utils/init/agent-skills.ts | 42 ++++++++++++++++++-- tests/unit/utils/init/agent-skills.test.ts | 45 ++++++++++++++++++++-- 2 files changed, 79 insertions(+), 8 deletions(-) diff --git a/src/utils/init/agent-skills.ts b/src/utils/init/agent-skills.ts index 1303b599372..06027204db1 100644 --- a/src/utils/init/agent-skills.ts +++ b/src/utils/init/agent-skills.ts @@ -19,6 +19,8 @@ const SKILL_NAME = /^[A-Za-z0-9_][A-Za-z0-9_.-]*$/ const STAGING_PREFIX = '.netlify-skill-' const STAGING_LEFTOVER = /^\.netlify-skill-.+-[A-Za-z0-9]{6}$/ const RETIRED_LEFTOVER = /^(.+)\.old-\d+-[0-9a-f]{12}$/ +const OWNERSHIP_MARKER = '.netlify-skills-install' +const LEFTOVER_MIN_AGE_MS = 10 * 60_000 const FETCH_TIMEOUT_MS = 10_000 const SETUP_TIMEOUT_MS = 120_000 const LOOPBACK_HOSTS = new Set(['localhost', '127.0.0.1', '[::1]']) @@ -77,7 +79,12 @@ export interface SkillsSyncResult { class SkillsError extends Error {} -const errorMessage = (error: unknown): string => (error instanceof Error ? error.message : String(error)) +const errorMessage = (error: unknown): string => { + if (error instanceof Error && (error.name === 'TimeoutError' || error.name === 'AbortError')) { + return `the download timed out after ${String(FETCH_TIMEOUT_MS / 1000)}s` + } + return error instanceof Error ? error.message : String(error) +} const sha256 = (bytes: Uint8Array): string => `sha256:${createHash('sha256').update(bytes).digest('hex')}` @@ -137,6 +144,9 @@ const indexManifest = (manifest: SkillsManifest): ManifestIndex => { if (exact.has(skill.name)) { throw new SkillsError(`manifest: duplicate skill name ${JSON.stringify(skill.name)}`) } + if (skill.status === 'active' && typeof skill.tree_hash !== 'string') { + throw new SkillsError(`manifest: active skill ${skill.name} has no tree_hash`) + } exact.set(skill.name, skill) for (const name of skill.prior_names ?? []) { assertSkillName(name, `prior name of ${skill.name}`) @@ -261,7 +271,9 @@ export const classifySkillsDirectory = async ( const dir = path.join(root, entry.name) const hasSkillMd = entry.isDirectory() && (await isFile(path.join(dir, 'SKILL.md'))) if (!hasSkillMd && !target && !retired) continue - if (entry.isDirectory() && !hasSkillMd && target && (await fs.readdir(dir)).length === 0) continue + if (entry.isDirectory() && !hasSkillMd && known?.status === 'active' && (await fs.readdir(dir)).length === 0) { + continue + } let treeHash: string | null = null if (hasSkillMd) { @@ -321,16 +333,20 @@ export const classifySkillsDirectory = async ( return { skills: records, missing } } +const markOwned = (dir: string): Promise => fs.writeFile(path.join(dir, OWNERSHIP_MARKER), '') + const replaceDirectory = async (staged: string, target: string): Promise => { const exists = await isDirectory(target) const retired = `${target}.old-${process.pid.toString()}-${randomBytes(6).toString('hex')}` if (exists) { await fs.rename(target, retired) + await markOwned(retired) } try { await fs.rename(staged, target) } catch (error) { if (exists) { + await fs.rm(path.join(retired, OWNERSHIP_MARKER), { force: true }) await fs.rename(retired, target) } throw error @@ -362,12 +378,18 @@ export const installSkill = async ( await fs.mkdir(dest, { recursive: true }) const staged = await fs.mkdtemp(path.join(dest, `${STAGING_PREFIX}${skill.name}-`)) try { + await markOwned(staged) const executable = new Set(skill.executable ?? []) for (const [file, bytes] of downloaded) { const output = path.join(staged, ...file.split('/')) await fs.mkdir(path.dirname(output), { recursive: true }) await fs.writeFile(output, bytes, { mode: executable.has(file) ? 0o755 : 0o644 }) } + await fs.rm(path.join(staged, OWNERSHIP_MARKER)) + const stagedHash = await hashSkillTree(staged) + if (stagedHash !== skill.tree_hash) { + throw new SkillsError(`staged tree hash ${stagedHash} does not match the manifest`) + } await replaceDirectory(staged, path.join(dest, skill.name)) } catch (error) { await fs.rm(staged, { recursive: true, force: true }) @@ -384,14 +406,26 @@ const isInstallLeftover = (name: string, { exact, prior }: ManifestIndex): boole return retired !== null && (exact.has(retired[1]) || prior.has(retired[1])) } +const isAbandonedInstallDirectory = async (dir: string, now: number): Promise => { + try { + const marker = await fs.lstat(path.join(dir, OWNERSHIP_MARKER)) + return marker.isFile() && now - marker.mtimeMs >= LEFTOVER_MIN_AGE_MS + } catch { + return false + } +} + const removeInstallLeftovers = async (root: string, index: ManifestIndex): Promise => { if (!(await isDirectoryOrLinkToOne(root))) { return [] } + const now = Date.now() const removed: string[] = [] for (const entry of await fs.readdir(root, { withFileTypes: true })) { - if (entry.isDirectory() && isInstallLeftover(entry.name, index)) { - await fs.rm(path.join(root, entry.name), { recursive: true, force: true }) + if (!entry.isDirectory() || !isInstallLeftover(entry.name, index)) continue + const dir = path.join(root, entry.name) + if (await isAbandonedInstallDirectory(dir, now)) { + await fs.rm(dir, { recursive: true, force: true }) removed.push(entry.name) } } diff --git a/tests/unit/utils/init/agent-skills.test.ts b/tests/unit/utils/init/agent-skills.test.ts index 6c362fe4114..8900747b7c1 100644 --- a/tests/unit/utils/init/agent-skills.test.ts +++ b/tests/unit/utils/init/agent-skills.test.ts @@ -1,5 +1,5 @@ import { createHash } from 'node:crypto' -import { chmod, lstat, mkdir, mkdtemp, readFile, readdir, rm, stat, symlink, writeFile } from 'node:fs/promises' +import { chmod, lstat, mkdir, mkdtemp, readFile, readdir, rm, stat, symlink, utimes, writeFile } from 'node:fs/promises' import { createServer, type Server } from 'node:http' import type { AddressInfo } from 'node:net' import { tmpdir } from 'node:os' @@ -357,9 +357,21 @@ describe('agent skills', () => { await expect(readFile(join(skillsDir, 'netlify-functions', 'SKILL.md'), 'utf8')).resolves.toBe('# functions v2\n') }) - test('sweeps staging and backup directories left by an interrupted install', async () => { - await writeSkill(skillsDir, '.netlify-skill-netlify-functions-Ab12Cd', FUNCTIONS.files) + test('sweeps only aged, marker-owned staging and backup directories left by an interrupted install', async () => { + const elevenMinutesAgo = new Date(Date.now() - 11 * 60_000) + const markOwned = async (name: string, at?: Date) => { + const marker = join(skillsDir, name, '.netlify-skills-install') + await writeFile(marker, '') + if (at) await utimes(marker, at, at) + } + await writeSkill(skillsDir, '.netlify-skill-netlify-functions-Ab12Cd', FUNCTIONS_V1) + await markOwned('.netlify-skill-netlify-functions-Ab12Cd', elevenMinutesAgo) await writeSkill(skillsDir, 'netlify-deploy.old-4242-0123456789ab', DEPLOY.files) + await markOwned('netlify-deploy.old-4242-0123456789ab', elevenMinutesAgo) + await writeSkill(skillsDir, '.netlify-skill-netlify-deploy-Zz99Yy', { 'SKILL.md': '# in flight\n' }) + await markOwned('.netlify-skill-netlify-deploy-Zz99Yy') + await writeSkill(skillsDir, '.netlify-skill-my-notes-Ab12Cd', { 'SKILL.md': '# a user dir\n' }) + await writeSkill(skillsDir, 'netlify-deploy.old-1111-abcdefabcdef', { 'SKILL.md': '# a user dir\n' }) await writeSkill(skillsDir, 'my-skill.old-4242-0123456789ab', { 'SKILL.md': '# mine\n' }) const manifest = await fetchSkillsManifest(hostUrl) @@ -378,12 +390,34 @@ describe('agent skills', () => { }, ]) await expect(listDirectories(skillsDir)).resolves.toEqual([ + '.netlify-skill-my-notes-Ab12Cd', + '.netlify-skill-netlify-deploy-Zz99Yy', 'my-skill.old-4242-0123456789ab', 'netlify-deploy', + 'netlify-deploy.old-1111-abcdefabcdef', 'netlify-functions', ]) }) + test('does not leave its ownership marker inside an installed skill', async () => { + const manifest = await fetchSkillsManifest(hostUrl) + await syncSkills({ host: hostUrl, directory: skillsDir, manifest }) + + await expect(readdir(join(skillsDir, 'netlify-functions'))).resolves.toEqual(['SKILL.md', 'references']) + await expect(hashSkillTree(join(skillsDir, 'netlify-deploy'))).resolves.toBe( + treeHashOf(DEPLOY.files, DEPLOY.executable), + ) + }) + + test('refuses an active skill without a tree hash', async () => { + const broken = structuredClone(host.manifest) + const functions = broken.skills.find(({ name }) => name === 'netlify-functions') + if (functions) functions.tree_hash = null + host.corrupt('/manifest.json', JSON.stringify(broken)) + + await expect(fetchSkillsManifest(hostUrl)).rejects.toThrow(/has no tree_hash/) + }) + test('refuses a manifest with an unsupported schema version', async () => { host.corrupt('/manifest.json', JSON.stringify({ ...host.manifest, schema_version: 2 })) @@ -414,6 +448,7 @@ describe('agent skills', () => { await mkdir(skillsDir, { recursive: true }) await symlink(join(elsewhere, 'functions-source'), join(skillsDir, 'netlify-functions')) await symlink(join(elsewhere, 'legacy-source'), join(skillsDir, 'netlify-legacy')) + await symlink(join(elsewhere, 'legacy-source'), join(skillsDir, 'netlify-deploy.old-4242-0123456789ab')) const manifest = await fetchSkillsManifest(hostUrl) const { actions } = await syncSkills({ host: hostUrl, directory: skillsDir, manifest }) @@ -424,7 +459,9 @@ describe('agent skills', () => { action: 'kept', detail: 'deprecated; use netlify-deploy, but edited locally', }) - await expect(lstat(join(skillsDir, 'netlify-functions'))).resolves.toMatchObject({}) + expect(actions.filter(({ action }) => action === 'removed')).toEqual([]) + expect((await lstat(join(skillsDir, 'netlify-functions'))).isSymbolicLink()).toBe(true) + expect((await lstat(join(skillsDir, 'netlify-deploy.old-4242-0123456789ab'))).isSymbolicLink()).toBe(true) await expect(readFile(join(elsewhere, 'functions-source', 'SKILL.md'), 'utf8')).resolves.toBe('# functions v1\n') await expect(readFile(join(elsewhere, 'legacy-source', 'SKILL.md'), 'utf8')).resolves.toBe('# retired\n') }) From 0c19dda53ab3e07ad1c289eeb3c364ac92fe4038 Mon Sep 17 00:00:00 2001 From: Domitrius Clark Date: Fri, 2 Oct 2026 13:57:55 -0400 Subject: [PATCH 4/6] refactor(init): drop skill deletion and cleanup from init Init only installs and refreshes skills. Copies under a prior or deprecated name are reported and left in place, and leftover directories are not swept; those actions move to the sync work in EX-3055. Removes the ownership marker, overall deadline and signal plumbing, duplicate detection and staged re-hash that defended them. Co-Authored-By: Claude Fable 5.1 --- src/utils/init/agent-skills.ts | 212 +++++---------------- tests/unit/utils/init/agent-skills.test.ts | 119 ++---------- 2 files changed, 68 insertions(+), 263 deletions(-) diff --git a/src/utils/init/agent-skills.ts b/src/utils/init/agent-skills.ts index 06027204db1..4d292d2e917 100644 --- a/src/utils/init/agent-skills.ts +++ b/src/utils/init/agent-skills.ts @@ -17,12 +17,7 @@ const AGENT_DIRECTORY_BY_DRIVING_AGENT: Partial> = { const SUPPORTED_SCHEMA_VERSION = 1 const SKILL_NAME = /^[A-Za-z0-9_][A-Za-z0-9_.-]*$/ const STAGING_PREFIX = '.netlify-skill-' -const STAGING_LEFTOVER = /^\.netlify-skill-.+-[A-Za-z0-9]{6}$/ -const RETIRED_LEFTOVER = /^(.+)\.old-\d+-[0-9a-f]{12}$/ -const OWNERSHIP_MARKER = '.netlify-skills-install' -const LEFTOVER_MIN_AGE_MS = 10 * 60_000 const FETCH_TIMEOUT_MS = 10_000 -const SETUP_TIMEOUT_MS = 120_000 const LOOPBACK_HOSTS = new Set(['localhost', '127.0.0.1', '[::1]']) export interface SkillHistoryEntry { @@ -54,9 +49,8 @@ export type SkillRecord = | { name: string; status: 'current'; version: string | null } | { name: string; status: 'stale'; version: string | null; have: string | null } | { name: string; status: 'modified'; version: string | null } - | { name: string; status: 'renamed'; currentName: string; modified: boolean } - | { name: string; status: 'deprecated'; replacedBy: string | null; modified: boolean } - | { name: string; status: 'duplicate'; currentName: string } + | { name: string; status: 'renamed'; currentName: string } + | { name: string; status: 'deprecated'; replacedBy: string | null } | { name: string; status: 'unknown' } export interface SkillsClassification { @@ -64,7 +58,7 @@ export interface SkillsClassification { missing: string[] } -export type SkillAction = 'current' | 'added' | 'updated' | 'renamed' | 'removed' | 'kept' | 'ignored' +export type SkillAction = 'current' | 'added' | 'updated' | 'kept' | 'ignored' export interface SkillActionRecord { name: string @@ -104,11 +98,10 @@ export const resolveSkillsHost = (env: NodeJS.ProcessEnv = process.env): string const urlFor = (host: string, ...parts: string[]): string => `${host}/${parts.map((part) => encodeURIComponent(part)).join('/')}` -const fetchBytes = async (url: string, signal?: AbortSignal): Promise => { - const requestTimeout = AbortSignal.timeout(FETCH_TIMEOUT_MS) +const fetchBytes = async (url: string): Promise => { const response = await fetch(url, { headers: { 'user-agent': `NetlifyCLI ${version}` }, - signal: signal ? AbortSignal.any([requestTimeout, signal]) : requestTimeout, + signal: AbortSignal.timeout(FETCH_TIMEOUT_MS), }) if (!response.ok) { throw new SkillsError(`${url}: HTTP ${response.status.toString()}`) @@ -164,9 +157,9 @@ const indexManifest = (manifest: SkillsManifest): ManifestIndex => { return { exact, prior } } -export const fetchSkillsManifest = async (host: string, signal?: AbortSignal): Promise => { +export const fetchSkillsManifest = async (host: string): Promise => { const url = urlFor(host, 'manifest.json') - const bytes = await fetchBytes(url, signal) + const bytes = await fetchBytes(url) let manifest: SkillsManifest try { manifest = JSON.parse(Buffer.from(bytes).toString('utf8')) as SkillsManifest @@ -265,16 +258,29 @@ export const classifySkillsDirectory = async ( for (const entry of entries) { const known = exact.get(entry.name) const renamed = prior.get(entry.name) - const target = known?.status === 'active' ? known : renamed?.status === 'active' ? renamed : null - const retired = known?.status === 'deprecated' ? known : renamed?.status === 'deprecated' ? renamed : null - if (!entry.isDirectory() && !target && !retired) continue + if (!entry.isDirectory() && !known && !renamed) continue const dir = path.join(root, entry.name) const hasSkillMd = entry.isDirectory() && (await isFile(path.join(dir, 'SKILL.md'))) - if (!hasSkillMd && !target && !retired) continue + if (!hasSkillMd && !known && !renamed) continue if (entry.isDirectory() && !hasSkillMd && known?.status === 'active' && (await fs.readdir(dir)).length === 0) { continue } + if (known?.status === 'deprecated' || renamed?.status === 'deprecated') { + const retired = known?.status === 'deprecated' ? known : renamed + records.push({ name: entry.name, status: 'deprecated', replacedBy: retired?.deprecated?.replaced_by ?? null }) + continue + } + if (!known && renamed) { + records.push({ name: entry.name, status: 'renamed', currentName: renamed.name }) + continue + } + if (!known) { + records.push({ name: entry.name, status: 'unknown' }) + continue + } + + presentActive.add(known.name) let treeHash: string | null = null if (hasSkillMd) { try { @@ -283,45 +289,13 @@ export const classifySkillsDirectory = async ( treeHash = null } } - - if (retired) { - const match = lastMatching(retired, treeHash) - records.push({ - name: entry.name, - status: 'deprecated', - replacedBy: retired.deprecated?.replaced_by ?? null, - modified: !match, - }) - continue - } - - if (!target) { - const twin = treeHash - ? manifest.skills.find( - (skill) => skill.status === 'active' && historyOf(skill).some((item) => item.tree_hash === treeHash), - ) - : undefined - records.push( - twin - ? { name: entry.name, status: 'duplicate', currentName: twin.name } - : { name: entry.name, status: 'unknown' }, - ) - continue - } - - const match = lastMatching(target, treeHash) - if (target === renamed) { - records.push({ name: entry.name, status: 'renamed', currentName: target.name, modified: !match }) - continue - } - - presentActive.add(target.name) - if (treeHash === target.tree_hash) { - records.push({ name: entry.name, status: 'current', version: target.version }) + const match = lastMatching(known, treeHash) + if (treeHash === known.tree_hash) { + records.push({ name: entry.name, status: 'current', version: known.version }) } else if (match) { - records.push({ name: entry.name, status: 'stale', version: target.version, have: match.version }) + records.push({ name: entry.name, status: 'stale', version: known.version, have: match.version }) } else { - records.push({ name: entry.name, status: 'modified', version: target.version }) + records.push({ name: entry.name, status: 'modified', version: known.version }) } } @@ -333,20 +307,16 @@ export const classifySkillsDirectory = async ( return { skills: records, missing } } -const markOwned = (dir: string): Promise => fs.writeFile(path.join(dir, OWNERSHIP_MARKER), '') - const replaceDirectory = async (staged: string, target: string): Promise => { const exists = await isDirectory(target) const retired = `${target}.old-${process.pid.toString()}-${randomBytes(6).toString('hex')}` if (exists) { await fs.rename(target, retired) - await markOwned(retired) } try { await fs.rename(staged, target) } catch (error) { if (exists) { - await fs.rm(path.join(retired, OWNERSHIP_MARKER), { force: true }) await fs.rename(retired, target) } throw error @@ -356,19 +326,14 @@ const replaceDirectory = async (staged: string, target: string): Promise = } } -export const installSkill = async ( - host: string, - dest: string, - skill: ManifestSkill, - signal?: AbortSignal, -): Promise => { +export const installSkill = async (host: string, dest: string, skill: ManifestSkill): Promise => { const files = Object.keys(skill.files ?? {}).sort() const downloaded: [string, Uint8Array][] = [] for (const file of files) { if (!isSafeFilePath(file)) { throw new SkillsError(`${skill.name}: unsafe manifest file path: ${file}`) } - const bytes = await fetchBytes(urlFor(host, 'skills', skill.name, ...file.split('/')), signal) + const bytes = await fetchBytes(urlFor(host, 'skills', skill.name, ...file.split('/'))) if (sha256(bytes) !== skill.files?.[file]) { throw new SkillsError(`${skill.name}/${file}: hash mismatch`) } @@ -378,18 +343,12 @@ export const installSkill = async ( await fs.mkdir(dest, { recursive: true }) const staged = await fs.mkdtemp(path.join(dest, `${STAGING_PREFIX}${skill.name}-`)) try { - await markOwned(staged) const executable = new Set(skill.executable ?? []) for (const [file, bytes] of downloaded) { const output = path.join(staged, ...file.split('/')) await fs.mkdir(path.dirname(output), { recursive: true }) await fs.writeFile(output, bytes, { mode: executable.has(file) ? 0o755 : 0o644 }) } - await fs.rm(path.join(staged, OWNERSHIP_MARKER)) - const stagedHash = await hashSkillTree(staged) - if (stagedHash !== skill.tree_hash) { - throw new SkillsError(`staged tree hash ${stagedHash} does not match the manifest`) - } await replaceDirectory(staged, path.join(dest, skill.name)) } catch (error) { await fs.rm(staged, { recursive: true, force: true }) @@ -398,55 +357,18 @@ export const installSkill = async ( return files.length } -const isInstallLeftover = (name: string, { exact, prior }: ManifestIndex): boolean => { - if (STAGING_LEFTOVER.test(name)) { - return true - } - const retired = RETIRED_LEFTOVER.exec(name) - return retired !== null && (exact.has(retired[1]) || prior.has(retired[1])) -} - -const isAbandonedInstallDirectory = async (dir: string, now: number): Promise => { - try { - const marker = await fs.lstat(path.join(dir, OWNERSHIP_MARKER)) - return marker.isFile() && now - marker.mtimeMs >= LEFTOVER_MIN_AGE_MS - } catch { - return false - } -} - -const removeInstallLeftovers = async (root: string, index: ManifestIndex): Promise => { - if (!(await isDirectoryOrLinkToOne(root))) { - return [] - } - const now = Date.now() - const removed: string[] = [] - for (const entry of await fs.readdir(root, { withFileTypes: true })) { - if (!entry.isDirectory() || !isInstallLeftover(entry.name, index)) continue - const dir = path.join(root, entry.name) - if (await isAbandonedInstallDirectory(dir, now)) { - await fs.rm(dir, { recursive: true, force: true }) - removed.push(entry.name) - } - } - return removed.sort() -} - export const syncSkills = async ({ host, directory, manifest, - signal, }: { host: string directory: string manifest: SkillsManifest - signal?: AbortSignal }): Promise => { - const index = indexManifest(manifest) - const { exact } = index + const { exact } = indexManifest(manifest) + const before = await classifySkillsDirectory(directory, manifest) const actions: SkillActionRecord[] = [] - const installed = new Set() const act = (name: string, action: SkillAction, detail?: string) => { actions.push(detail ? { name, action, detail } : { name, action }) } @@ -458,55 +380,25 @@ export const syncSkills = async ({ return skill } - for (const leftover of await removeInstallLeftovers(directory, index)) { - act(leftover, 'removed', 'leftover from an interrupted install') - } - const before = await classifySkillsDirectory(directory, manifest) - for (const record of before.skills) { - const dir = path.join(directory, record.name) switch (record.status) { case 'current': act(record.name, 'current', record.version ?? undefined) break case 'stale': { const skill = skillByName(record.name) - await installSkill(host, directory, skill, signal) + await installSkill(host, directory, skill) act(record.name, 'updated', `${record.have ?? 'unknown'} -> ${skill.version ?? 'latest'}`) break } case 'modified': act(record.name, 'kept', 'edited locally') break - case 'renamed': { - const current = before.skills.find((other) => other.name === record.currentName) - if (record.modified) { - act(record.name, 'kept', `edited locally; now called ${record.currentName}`) - } else if (current?.status === 'modified') { - act(record.name, 'kept', `${record.currentName} is already installed and edited locally`) - } else if (current) { - await fs.rm(dir, { recursive: true, force: true }) - act(record.name, 'removed', `superseded by ${record.currentName}`) - } else { - await installSkill(host, directory, skillByName(record.currentName), signal) - installed.add(record.currentName) - await fs.rm(dir, { recursive: true, force: true }) - act(record.name, 'renamed', `-> ${record.currentName}`) - } + case 'renamed': + act(record.name, 'kept', `now called ${record.currentName}; this copy can be removed`) break - } - case 'deprecated': { - const replacement = record.replacedBy ? `; use ${record.replacedBy}` : '' - if (record.modified) { - act(record.name, 'kept', `deprecated${replacement}, but edited locally`) - } else { - await fs.rm(dir, { recursive: true, force: true }) - act(record.name, 'removed', `deprecated${replacement}`) - } - break - } - case 'duplicate': - act(record.name, 'ignored', `copy of ${record.currentName} under another name`) + case 'deprecated': + act(record.name, 'kept', `deprecated${record.replacedBy ? `; use ${record.replacedBy}` : ''}`) break case 'unknown': act(record.name, 'ignored', 'not a Netlify skill') @@ -515,9 +407,8 @@ export const syncSkills = async ({ } for (const name of before.missing) { - if (installed.has(name)) continue const skill = skillByName(name) - await installSkill(host, directory, skill, signal) + await installSkill(host, directory, skill) act(name, 'added', skill.version ?? undefined) } @@ -543,15 +434,7 @@ export const resolveSkillsDirectories = async ( } const summarize = (actions: SkillActionRecord[]): Record => { - const summary: Record = { - current: 0, - added: 0, - updated: 0, - renamed: 0, - removed: 0, - kept: 0, - ignored: 0, - } + const summary: Record = { current: 0, added: 0, updated: 0, kept: 0, ignored: 0 } for (const { action } of actions) { summary[action] += 1 } @@ -560,17 +443,14 @@ const summarize = (actions: SkillActionRecord[]): Record => const describeSync = ({ directory, actions }: SkillsSyncResult): string => { const summary = summarize(actions) - const changed = summary.added + summary.updated + summary.renamed + summary.removed const location = chalk.underline(directory) - if (changed === 0) { + if (summary.added + summary.updated === 0) { return `Netlify skills in ${location} are up to date.` } const parts = [ summary.added > 0 ? `${summary.added.toString()} added` : '', summary.updated > 0 ? `${summary.updated.toString()} updated` : '', - summary.renamed > 0 ? `${summary.renamed.toString()} renamed` : '', - summary.removed > 0 ? `${summary.removed.toString()} removed` : '', - summary.kept > 0 ? `${summary.kept.toString()} kept (edited locally)` : '', + summary.kept > 0 ? `${summary.kept.toString()} kept` : '', ].filter(Boolean) return `Installed Netlify skills in ${location} (${parts.join(', ')}).` } @@ -594,13 +474,15 @@ export const setupAgentSkills = async ({ try { directories = await resolveSkillsDirectories(workingDir, env) const host = resolveSkillsHost(env) - const signal = AbortSignal.timeout(SETUP_TIMEOUT_MS) - const manifest = await fetchSkillsManifest(host, signal) + const manifest = await fetchSkillsManifest(host) const actions: SkillActionRecord[] = [] for (const directory of directories) { - const result = await syncSkills({ host, directory: path.resolve(workingDir, directory), manifest, signal }) + const result = await syncSkills({ host, directory: path.resolve(workingDir, directory), manifest }) actions.push(...result.actions) log(describeSync({ directory, actions: result.actions })) + for (const { name, detail } of result.actions.filter(({ action }) => action === 'kept')) { + log(` ${chalk.dim(name)}: ${detail ?? 'kept'}`) + } } return { installed: true, directories, skillsVersion: manifest.version, summary: summarize(actions) } } catch (error) { diff --git a/tests/unit/utils/init/agent-skills.test.ts b/tests/unit/utils/init/agent-skills.test.ts index 8900747b7c1..40c797e9f6f 100644 --- a/tests/unit/utils/init/agent-skills.test.ts +++ b/tests/unit/utils/init/agent-skills.test.ts @@ -1,5 +1,5 @@ import { createHash } from 'node:crypto' -import { chmod, lstat, mkdir, mkdtemp, readFile, readdir, rm, stat, symlink, utimes, writeFile } from 'node:fs/promises' +import { chmod, lstat, mkdir, mkdtemp, readFile, readdir, rm, stat, symlink, writeFile } from 'node:fs/promises' import { createServer, type Server } from 'node:http' import type { AddressInfo } from 'node:net' import { tmpdir } from 'node:os' @@ -253,6 +253,7 @@ describe('agent skills', () => { ) const script = await stat(join(skillsDir, 'netlify-deploy', 'scripts', 'deploy.sh')) expect(script.mode & 0o111).not.toBe(0) + await expect(readdir(join(skillsDir, 'netlify-functions'))).resolves.toEqual(['SKILL.md', 'references']) }) test('is idempotent: a second run reports everything current and changes nothing', async () => { @@ -292,18 +293,26 @@ describe('agent skills', () => { await expect(readFile(join(skillsDir, 'netlify-deploy', 'SKILL.md'), 'utf8')).resolves.toBe('# my own notes\n') }) - test('migrates a skill installed under a prior name', async () => { + test('installs the new name beside a copy under a prior name and leaves the old copy alone', async () => { await writeSkill(skillsDir, 'netlify-cli-and-deploy', DEPLOY.files, DEPLOY.executable) const manifest = await fetchSkillsManifest(hostUrl) const { actions } = await syncSkills({ host: hostUrl, directory: skillsDir, manifest }) - expect(actions).toContainEqual({ name: 'netlify-cli-and-deploy', action: 'renamed', detail: '-> netlify-deploy' }) - expect(actions.filter(({ name }) => name === 'netlify-deploy')).toEqual([]) - await expect(listDirectories(skillsDir)).resolves.toEqual(['netlify-deploy', 'netlify-functions']) + expect(actions).toContainEqual({ + name: 'netlify-cli-and-deploy', + action: 'kept', + detail: 'now called netlify-deploy; this copy can be removed', + }) + expect(actions).toContainEqual({ name: 'netlify-deploy', action: 'added', detail: '2.0.0' }) + await expect(listDirectories(skillsDir)).resolves.toEqual([ + 'netlify-cli-and-deploy', + 'netlify-deploy', + 'netlify-functions', + ]) }) - test('removes an unedited deprecated skill and leaves edited or unknown directories alone', async () => { + test('reports a deprecated skill and leaves it and unknown directories alone', async () => { await writeSkill(skillsDir, 'netlify-legacy', RETIRED_FILES) await writeSkill(skillsDir, 'my-team-skill', { 'SKILL.md': '# ours\n' }) const manifest = await fetchSkillsManifest(hostUrl) @@ -313,38 +322,15 @@ describe('agent skills', () => { expect(actions).toContainEqual({ name: 'my-team-skill', action: 'ignored', detail: 'not a Netlify skill' }) expect(actions).toContainEqual({ name: 'netlify-legacy', - action: 'removed', + action: 'kept', detail: 'deprecated; use netlify-deploy', }) await expect(listDirectories(skillsDir)).resolves.toEqual([ 'my-team-skill', 'netlify-deploy', 'netlify-functions', + 'netlify-legacy', ]) - - await writeSkill(skillsDir, 'netlify-legacy', { 'SKILL.md': '# retired but edited\n' }) - const second = await syncSkills({ host: hostUrl, directory: skillsDir, manifest }) - expect(second.actions).toContainEqual({ - name: 'netlify-legacy', - action: 'kept', - detail: 'deprecated; use netlify-deploy, but edited locally', - }) - }) - - test('removes the old directory when a renamed skill is already installed under its new name', async () => { - await writeSkill(skillsDir, 'netlify-cli-and-deploy', DEPLOY.files, DEPLOY.executable) - await writeSkill(skillsDir, DEPLOY.name, DEPLOY.files, DEPLOY.executable) - const manifest = await fetchSkillsManifest(hostUrl) - - const { actions } = await syncSkills({ host: hostUrl, directory: skillsDir, manifest }) - - expect(actions).toContainEqual({ - name: 'netlify-cli-and-deploy', - action: 'removed', - detail: 'superseded by netlify-deploy', - }) - expect(actions).toContainEqual({ name: 'netlify-deploy', action: 'current', detail: '2.0.0' }) - await expect(listDirectories(skillsDir)).resolves.toEqual(['netlify-deploy', 'netlify-functions']) }) test('reinstalls over an empty directory carrying a skill name', async () => { @@ -357,56 +343,10 @@ describe('agent skills', () => { await expect(readFile(join(skillsDir, 'netlify-functions', 'SKILL.md'), 'utf8')).resolves.toBe('# functions v2\n') }) - test('sweeps only aged, marker-owned staging and backup directories left by an interrupted install', async () => { - const elevenMinutesAgo = new Date(Date.now() - 11 * 60_000) - const markOwned = async (name: string, at?: Date) => { - const marker = join(skillsDir, name, '.netlify-skills-install') - await writeFile(marker, '') - if (at) await utimes(marker, at, at) - } - await writeSkill(skillsDir, '.netlify-skill-netlify-functions-Ab12Cd', FUNCTIONS_V1) - await markOwned('.netlify-skill-netlify-functions-Ab12Cd', elevenMinutesAgo) - await writeSkill(skillsDir, 'netlify-deploy.old-4242-0123456789ab', DEPLOY.files) - await markOwned('netlify-deploy.old-4242-0123456789ab', elevenMinutesAgo) - await writeSkill(skillsDir, '.netlify-skill-netlify-deploy-Zz99Yy', { 'SKILL.md': '# in flight\n' }) - await markOwned('.netlify-skill-netlify-deploy-Zz99Yy') - await writeSkill(skillsDir, '.netlify-skill-my-notes-Ab12Cd', { 'SKILL.md': '# a user dir\n' }) - await writeSkill(skillsDir, 'netlify-deploy.old-1111-abcdefabcdef', { 'SKILL.md': '# a user dir\n' }) - await writeSkill(skillsDir, 'my-skill.old-4242-0123456789ab', { 'SKILL.md': '# mine\n' }) - const manifest = await fetchSkillsManifest(hostUrl) - - const { actions } = await syncSkills({ host: hostUrl, directory: skillsDir, manifest }) - - expect(actions.filter(({ action }) => action === 'removed')).toEqual([ - { - name: '.netlify-skill-netlify-functions-Ab12Cd', - action: 'removed', - detail: 'leftover from an interrupted install', - }, - { - name: 'netlify-deploy.old-4242-0123456789ab', - action: 'removed', - detail: 'leftover from an interrupted install', - }, - ]) - await expect(listDirectories(skillsDir)).resolves.toEqual([ - '.netlify-skill-my-notes-Ab12Cd', - '.netlify-skill-netlify-deploy-Zz99Yy', - 'my-skill.old-4242-0123456789ab', - 'netlify-deploy', - 'netlify-deploy.old-1111-abcdefabcdef', - 'netlify-functions', - ]) - }) - - test('does not leave its ownership marker inside an installed skill', async () => { - const manifest = await fetchSkillsManifest(hostUrl) - await syncSkills({ host: hostUrl, directory: skillsDir, manifest }) + test('refuses a manifest with an unsupported schema version', async () => { + host.corrupt('/manifest.json', JSON.stringify({ ...host.manifest, schema_version: 2 })) - await expect(readdir(join(skillsDir, 'netlify-functions'))).resolves.toEqual(['SKILL.md', 'references']) - await expect(hashSkillTree(join(skillsDir, 'netlify-deploy'))).resolves.toBe( - treeHashOf(DEPLOY.files, DEPLOY.executable), - ) + await expect(fetchSkillsManifest(hostUrl)).rejects.toThrow(/schema 2 is not supported/) }) test('refuses an active skill without a tree hash', async () => { @@ -418,12 +358,6 @@ describe('agent skills', () => { await expect(fetchSkillsManifest(hostUrl)).rejects.toThrow(/has no tree_hash/) }) - test('refuses a manifest with an unsupported schema version', async () => { - host.corrupt('/manifest.json', JSON.stringify({ ...host.manifest, schema_version: 2 })) - - await expect(fetchSkillsManifest(hostUrl)).rejects.toThrow(/schema 2 is not supported/) - }) - test('follows a symlinked skills root and still keeps edited copies there', async () => { const sharedDir = join(projectDir, 'shared-skills') await writeSkill(sharedDir, DEPLOY.name, { ...DEPLOY.files, 'SKILL.md': '# my own notes\n' }) @@ -441,29 +375,18 @@ describe('agent skills', () => { await expect(listDirectories(sharedDir)).resolves.toEqual(['netlify-deploy', 'netlify-functions']) }) - test('never replaces or deletes a skill entry that is a symlink', async () => { + test('never replaces a skill entry that is a symlink', async () => { const elsewhere = join(projectDir, 'elsewhere') await writeSkill(elsewhere, 'functions-source', FUNCTIONS_V1) - await writeSkill(elsewhere, 'legacy-source', RETIRED_FILES) await mkdir(skillsDir, { recursive: true }) await symlink(join(elsewhere, 'functions-source'), join(skillsDir, 'netlify-functions')) - await symlink(join(elsewhere, 'legacy-source'), join(skillsDir, 'netlify-legacy')) - await symlink(join(elsewhere, 'legacy-source'), join(skillsDir, 'netlify-deploy.old-4242-0123456789ab')) const manifest = await fetchSkillsManifest(hostUrl) const { actions } = await syncSkills({ host: hostUrl, directory: skillsDir, manifest }) expect(actions).toContainEqual({ name: 'netlify-functions', action: 'kept', detail: 'edited locally' }) - expect(actions).toContainEqual({ - name: 'netlify-legacy', - action: 'kept', - detail: 'deprecated; use netlify-deploy, but edited locally', - }) - expect(actions.filter(({ action }) => action === 'removed')).toEqual([]) expect((await lstat(join(skillsDir, 'netlify-functions'))).isSymbolicLink()).toBe(true) - expect((await lstat(join(skillsDir, 'netlify-deploy.old-4242-0123456789ab'))).isSymbolicLink()).toBe(true) await expect(readFile(join(elsewhere, 'functions-source', 'SKILL.md'), 'utf8')).resolves.toBe('# functions v1\n') - await expect(readFile(join(elsewhere, 'legacy-source', 'SKILL.md'), 'utf8')).resolves.toBe('# retired\n') }) test('rejects a file whose bytes do not match the manifest and leaves no partial install', async () => { From 9a006257143e8a8e36b35ab23b094a62cce9ecd0 Mon Sep 17 00:00:00 2001 From: Domitrius Clark Date: Fri, 2 Oct 2026 14:04:16 -0400 Subject: [PATCH 5/6] fix(init): verify the target right before replacing a skill A skill directory is replaced only if, at swap time, it is empty or its tree hash matches a release we shipped. This stops a user directory that differs only by case on a case-insensitive filesystem, or one edited during the download, from being renamed aside and deleted. A refused swap is reported and the rest of the sync goes on. Co-Authored-By: Claude Fable 5.1 --- src/utils/init/agent-skills.ts | 77 ++++++++++++++++++---- tests/unit/utils/init/agent-skills.test.ts | 43 ++++++++++++ 2 files changed, 106 insertions(+), 14 deletions(-) diff --git a/src/utils/init/agent-skills.ts b/src/utils/init/agent-skills.ts index 4d292d2e917..9ffef043cba 100644 --- a/src/utils/init/agent-skills.ts +++ b/src/utils/init/agent-skills.ts @@ -73,6 +73,8 @@ export interface SkillsSyncResult { class SkillsError extends Error {} +class SkillConflictError extends SkillsError {} + const errorMessage = (error: unknown): string => { if (error instanceof Error && (error.name === 'TimeoutError' || error.name === 'AbortError')) { return `the download timed out after ${String(FETCH_TIMEOUT_MS / 1000)}s` @@ -231,6 +233,15 @@ const isDirectory = async (dir: string): Promise => { } } +const exists = async (file: string): Promise => { + try { + await fs.lstat(file) + return true + } catch { + return false + } +} + const isDirectoryOrLinkToOne = async (dir: string): Promise => { try { return (await fs.stat(dir)).isDirectory() @@ -326,19 +337,47 @@ const replaceDirectory = async (staged: string, target: string): Promise = } } -export const installSkill = async (host: string, dest: string, skill: ManifestSkill): Promise => { - const files = Object.keys(skill.files ?? {}).sort() +const isReplaceableCopy = async (target: string, skill: ManifestSkill): Promise => { + if (!(await isDirectory(target))) { + return !(await exists(target)) + } + if ((await fs.readdir(target)).length === 0) { + return true + } + try { + const treeHash = await hashSkillTree(target) + return historyOf(skill).some((entry) => entry.tree_hash === treeHash) + } catch { + return false + } +} + +const downloadSkill = async (host: string, skill: ManifestSkill): Promise<[string, Uint8Array][]> => { const downloaded: [string, Uint8Array][] = [] - for (const file of files) { + for (const file of Object.keys(skill.files ?? {}).sort()) { if (!isSafeFilePath(file)) { throw new SkillsError(`${skill.name}: unsafe manifest file path: ${file}`) } - const bytes = await fetchBytes(urlFor(host, 'skills', skill.name, ...file.split('/'))) + let bytes: Uint8Array + try { + bytes = await fetchBytes(urlFor(host, 'skills', skill.name, ...file.split('/'))) + } catch (error) { + throw new SkillsError(`${skill.name}/${file}: ${errorMessage(error)}`) + } if (sha256(bytes) !== skill.files?.[file]) { throw new SkillsError(`${skill.name}/${file}: hash mismatch`) } downloaded.push([file, bytes]) } + return downloaded +} + +export const installSkill = async (host: string, dest: string, skill: ManifestSkill): Promise => { + const downloaded = await downloadSkill(host, skill) + const target = path.join(dest, skill.name) + if (!(await isReplaceableCopy(target, skill))) { + throw new SkillConflictError(`${target} already exists and is not an unedited Netlify skill; left in place`) + } await fs.mkdir(dest, { recursive: true }) const staged = await fs.mkdtemp(path.join(dest, `${STAGING_PREFIX}${skill.name}-`)) @@ -349,12 +388,12 @@ export const installSkill = async (host: string, dest: string, skill: ManifestSk await fs.mkdir(path.dirname(output), { recursive: true }) await fs.writeFile(output, bytes, { mode: executable.has(file) ? 0o755 : 0o644 }) } - await replaceDirectory(staged, path.join(dest, skill.name)) + await replaceDirectory(staged, target) } catch (error) { await fs.rm(staged, { recursive: true, force: true }) throw new SkillsError(`${skill.name}: could not install: ${errorMessage(error)}`) } - return files.length + return downloaded.length } export const syncSkills = async ({ @@ -380,17 +419,27 @@ export const syncSkills = async ({ return skill } + const install = async (name: string, onInstalled: (skill: ManifestSkill) => void) => { + const skill = skillByName(name) + try { + await installSkill(host, directory, skill) + onInstalled(skill) + } catch (error) { + if (!(error instanceof SkillConflictError)) throw error + act(name, 'kept', error.message) + } + } + for (const record of before.skills) { switch (record.status) { case 'current': act(record.name, 'current', record.version ?? undefined) break - case 'stale': { - const skill = skillByName(record.name) - await installSkill(host, directory, skill) - act(record.name, 'updated', `${record.have ?? 'unknown'} -> ${skill.version ?? 'latest'}`) + case 'stale': + await install(record.name, (skill) => { + act(record.name, 'updated', `${record.have ?? 'unknown'} -> ${skill.version ?? 'latest'}`) + }) break - } case 'modified': act(record.name, 'kept', 'edited locally') break @@ -407,9 +456,9 @@ export const syncSkills = async ({ } for (const name of before.missing) { - const skill = skillByName(name) - await installSkill(host, directory, skill) - act(name, 'added', skill.version ?? undefined) + await install(name, (skill) => { + act(name, 'added', skill.version ?? undefined) + }) } return { directory, actions } diff --git a/tests/unit/utils/init/agent-skills.test.ts b/tests/unit/utils/init/agent-skills.test.ts index 40c797e9f6f..86aabb99b39 100644 --- a/tests/unit/utils/init/agent-skills.test.ts +++ b/tests/unit/utils/init/agent-skills.test.ts @@ -389,6 +389,49 @@ describe('agent skills', () => { await expect(readFile(join(elsewhere, 'functions-source', 'SKILL.md'), 'utf8')).resolves.toBe('# functions v1\n') }) + test('refuses to replace a directory that is not an unedited Netlify skill, even when asked directly', async () => { + await writeSkill(skillsDir, FUNCTIONS.name, { 'SKILL.md': '# mine\n', 'notes.md': '# keep me\n' }) + const manifest = await fetchSkillsManifest(hostUrl) + const functions = manifest.skills.find(({ name }) => name === 'netlify-functions') + if (!functions) throw new Error('netlify-functions missing from manifest') + + await expect(installSkill(hostUrl, skillsDir, functions)).rejects.toThrow(/already exists/) + await expect(readFile(join(skillsDir, 'netlify-functions', 'notes.md'), 'utf8')).resolves.toBe('# keep me\n') + await expect(listDirectories(skillsDir)).resolves.toEqual(['netlify-functions']) + }) + + test('survives a user directory that differs only by case and keeps syncing the rest', async () => { + await writeSkill(skillsDir, 'Netlify-Functions', { 'SKILL.md': '# mine\n', 'notes.md': '# keep me\n' }) + const manifest = await fetchSkillsManifest(hostUrl) + + const { actions } = await syncSkills({ host: hostUrl, directory: skillsDir, manifest }) + + await expect(readFile(join(skillsDir, 'Netlify-Functions', 'notes.md'), 'utf8')).resolves.toBe('# keep me\n') + expect(actions).toContainEqual({ name: 'netlify-deploy', action: 'added', detail: '2.0.0' }) + const functions = actions.find(({ name }) => name === 'netlify-functions') + expect(functions?.action === 'added' || functions?.detail?.includes('already exists')).toBe(true) + }) + + test('keeps reporting a renamed copy on the second run without extra downloads', async () => { + await writeSkill(skillsDir, 'netlify-cli-and-deploy', DEPLOY.files, DEPLOY.executable) + const manifest = await fetchSkillsManifest(hostUrl) + await syncSkills({ host: hostUrl, directory: skillsDir, manifest }) + const requestsAfterInstall = host.requests.length + + const { actions } = await syncSkills({ host: hostUrl, directory: skillsDir, manifest }) + + expect(actions).toEqual([ + { + name: 'netlify-cli-and-deploy', + action: 'kept', + detail: 'now called netlify-deploy; this copy can be removed', + }, + { name: 'netlify-deploy', action: 'current', detail: '2.0.0' }, + { name: 'netlify-functions', action: 'current', detail: '2.0.0' }, + ]) + expect(host.requests.length).toBe(requestsAfterInstall) + }) + test('rejects a file whose bytes do not match the manifest and leaves no partial install', async () => { host.corrupt('/skills/netlify-functions/SKILL.md', '# tampered\n') const manifest = await fetchSkillsManifest(hostUrl) From c37f843b5373bed8b3d1a86c26bd1da918cf8c9f Mon Sep 17 00:00:00 2001 From: Domitrius Clark Date: Fri, 2 Oct 2026 14:12:12 -0400 Subject: [PATCH 6/6] test(init): stub fetch in the agent skills unit tests Matches the fetch-stubbing pattern the other unit tests use instead of starting a local http server. Co-Authored-By: Claude Fable 5.1 --- tests/unit/utils/init/agent-skills.test.ts | 208 +++++++++------------ 1 file changed, 90 insertions(+), 118 deletions(-) diff --git a/tests/unit/utils/init/agent-skills.test.ts b/tests/unit/utils/init/agent-skills.test.ts index 86aabb99b39..5d327f4ab21 100644 --- a/tests/unit/utils/init/agent-skills.test.ts +++ b/tests/unit/utils/init/agent-skills.test.ts @@ -1,7 +1,5 @@ import { createHash } from 'node:crypto' import { chmod, lstat, mkdir, mkdtemp, readFile, readdir, rm, stat, symlink, writeFile } from 'node:fs/promises' -import { createServer, type Server } from 'node:http' -import type { AddressInfo } from 'node:net' import { tmpdir } from 'node:os' import { join } from 'node:path' @@ -38,6 +36,8 @@ interface SkillSpec { previous?: { version: string; files: Files }[] } +const HOST = 'https://skills.test' + const sha256 = (content: string | Uint8Array) => `sha256:${createHash('sha256').update(content).digest('hex')}` const treeHashOf = (files: Files, executable: string[] = []) => { @@ -94,59 +94,26 @@ const DEPLOY: SkillSpec = { const RETIRED_FILES: Files = { 'SKILL.md': '# retired\n' } -class SkillsHost { - private server: Server | undefined - private readonly responses = new Map() - readonly requests: string[] = [] - readonly manifest: SkillsManifest - - constructor(skills: SkillSpec[], extraSkills: ManifestSkill[] = []) { - this.manifest = { - schema_version: 1, - version: '2.0.0', - skills: [...skills.map(activeSkill), ...extraSkills], - } - this.responses.set('/manifest.json', Buffer.from(JSON.stringify(this.manifest))) - for (const { name, files } of skills) { - for (const [file, content] of Object.entries(files)) { - this.responses.set(`/skills/${name}/${file}`, Buffer.from(content)) - } - } - } +const buildManifest = (skills: SkillSpec[], extraSkills: ManifestSkill[] = []): SkillsManifest => ({ + schema_version: 1, + version: '2.0.0', + skills: [...skills.map(activeSkill), ...extraSkills], +}) - corrupt(path: string, content: string) { - this.responses.set(path, Buffer.from(content)) +const hostedResponses = (manifest: SkillsManifest, skills: SkillSpec[]) => { + const responses = new Map([['/manifest.json', JSON.stringify(manifest)]]) + for (const { name, files } of skills) { + for (const [file, content] of Object.entries(files)) { + responses.set(`/skills/${name}/${file}`, content) + } } + return responses +} - async start(): Promise { - this.server = createServer((req, res) => { - const url = decodeURIComponent(req.url ?? '') - this.requests.push(url) - const body = this.responses.get(url) - if (!body) { - res.statusCode = 404 - res.end('not found') - return - } - res.end(body) - }) - await new Promise((resolve) => this.server?.listen(0, '127.0.0.1', resolve)) - const { port } = this.server.address() as AddressInfo - return `http://127.0.0.1:${port.toString()}` - } +const requestPath = (input: string | URL | Request) => + decodeURIComponent(new URL(input instanceof Request ? input.url : input).pathname) - async stop() { - await new Promise((resolve, reject) => { - this.server?.close((error) => { - if (error) { - reject(error) - } else { - resolve() - } - }) - }) - } -} +const requestedPaths = () => vi.mocked(fetch).mock.calls.map(([input]) => requestPath(input)) const writeSkill = async (root: string, name: string, files: Files, executable: string[] = []) => { for (const [file, content] of Object.entries(files)) { @@ -168,11 +135,13 @@ describe('agent skills', () => { let skillsDir: string beforeEach(async () => { + vi.stubGlobal('fetch', vi.fn()) projectDir = await mkdtemp(join(tmpdir(), 'agent-skills-')) skillsDir = join(projectDir, '.agents', 'skills') }) afterEach(async () => { + vi.unstubAllGlobals() await rm(projectDir, { recursive: true, force: true }) }) @@ -227,21 +196,25 @@ describe('agent skills', () => { }) describe('syncing against a hosted release', () => { - let host: SkillsHost - let hostUrl: string - - beforeEach(async () => { - host = new SkillsHost([FUNCTIONS, DEPLOY], [deprecatedSkill('netlify-legacy', RETIRED_FILES, 'netlify-deploy')]) - hostUrl = await host.start() - }) - - afterEach(async () => { - await host.stop() + const skills = [FUNCTIONS, DEPLOY] + let manifest: SkillsManifest + let responses: Map + + beforeEach(() => { + manifest = buildManifest(skills, [deprecatedSkill('netlify-legacy', RETIRED_FILES, 'netlify-deploy')]) + responses = hostedResponses(manifest, skills) + vi.mocked(fetch).mockImplementation((input) => { + const body = responses.get(requestPath(input)) + return Promise.resolve(body === undefined ? new Response('not found', { status: 404 }) : new Response(body)) + }) }) test('installs every active skill into an empty directory with verified bytes and modes', async () => { - const manifest = await fetchSkillsManifest(hostUrl) - const { actions } = await syncSkills({ host: hostUrl, directory: skillsDir, manifest }) + const { actions } = await syncSkills({ + host: HOST, + directory: skillsDir, + manifest: await fetchSkillsManifest(HOST), + }) expect(actions).toEqual([ { name: 'netlify-deploy', action: 'added', detail: '2.0.0' }, @@ -254,15 +227,16 @@ describe('agent skills', () => { const script = await stat(join(skillsDir, 'netlify-deploy', 'scripts', 'deploy.sh')) expect(script.mode & 0o111).not.toBe(0) await expect(readdir(join(skillsDir, 'netlify-functions'))).resolves.toEqual(['SKILL.md', 'references']) + const [, init] = vi.mocked(fetch).mock.calls[0] + expect(new Headers(init?.headers).get('user-agent')).toMatch(/^NetlifyCLI /) }) test('is idempotent: a second run reports everything current and changes nothing', async () => { - const manifest = await fetchSkillsManifest(hostUrl) - await syncSkills({ host: hostUrl, directory: skillsDir, manifest }) + await syncSkills({ host: HOST, directory: skillsDir, manifest }) const before = await stat(join(skillsDir, 'netlify-functions', 'SKILL.md')) - const requestsAfterInstall = host.requests.length + vi.mocked(fetch).mockClear() - const { actions } = await syncSkills({ host: hostUrl, directory: skillsDir, manifest }) + const { actions } = await syncSkills({ host: HOST, directory: skillsDir, manifest }) expect(actions).toEqual([ { name: 'netlify-deploy', action: 'current', detail: '2.0.0' }, @@ -270,13 +244,12 @@ describe('agent skills', () => { ]) const after = await stat(join(skillsDir, 'netlify-functions', 'SKILL.md')) expect(after.mtimeMs).toBe(before.mtimeMs) - expect(host.requests.length).toBe(requestsAfterInstall) + expect(requestedPaths()).toEqual([]) }) test('replaces a stale copy and keeps a locally edited one', async () => { await writeSkill(skillsDir, FUNCTIONS.name, FUNCTIONS_V1) await writeSkill(skillsDir, DEPLOY.name, { ...DEPLOY.files, 'SKILL.md': '# my own notes\n' }) - const manifest = await fetchSkillsManifest(hostUrl) const classification = await classifySkillsDirectory(skillsDir, manifest) expect(classification.skills).toEqual([ @@ -284,7 +257,7 @@ describe('agent skills', () => { { name: 'netlify-functions', status: 'stale', version: '2.0.0', have: '1.0.0' }, ]) - const { actions } = await syncSkills({ host: hostUrl, directory: skillsDir, manifest }) + const { actions } = await syncSkills({ host: HOST, directory: skillsDir, manifest }) expect(actions).toEqual([ { name: 'netlify-deploy', action: 'kept', detail: 'edited locally' }, { name: 'netlify-functions', action: 'updated', detail: '1.0.0 -> 2.0.0' }, @@ -295,9 +268,8 @@ describe('agent skills', () => { test('installs the new name beside a copy under a prior name and leaves the old copy alone', async () => { await writeSkill(skillsDir, 'netlify-cli-and-deploy', DEPLOY.files, DEPLOY.executable) - const manifest = await fetchSkillsManifest(hostUrl) - const { actions } = await syncSkills({ host: hostUrl, directory: skillsDir, manifest }) + const { actions } = await syncSkills({ host: HOST, directory: skillsDir, manifest }) expect(actions).toContainEqual({ name: 'netlify-cli-and-deploy', @@ -312,12 +284,30 @@ describe('agent skills', () => { ]) }) + test('keeps reporting a renamed copy on the second run without extra downloads', async () => { + await writeSkill(skillsDir, 'netlify-cli-and-deploy', DEPLOY.files, DEPLOY.executable) + await syncSkills({ host: HOST, directory: skillsDir, manifest }) + vi.mocked(fetch).mockClear() + + const { actions } = await syncSkills({ host: HOST, directory: skillsDir, manifest }) + + expect(actions).toEqual([ + { + name: 'netlify-cli-and-deploy', + action: 'kept', + detail: 'now called netlify-deploy; this copy can be removed', + }, + { name: 'netlify-deploy', action: 'current', detail: '2.0.0' }, + { name: 'netlify-functions', action: 'current', detail: '2.0.0' }, + ]) + expect(requestedPaths()).toEqual([]) + }) + test('reports a deprecated skill and leaves it and unknown directories alone', async () => { await writeSkill(skillsDir, 'netlify-legacy', RETIRED_FILES) await writeSkill(skillsDir, 'my-team-skill', { 'SKILL.md': '# ours\n' }) - const manifest = await fetchSkillsManifest(hostUrl) - const { actions } = await syncSkills({ host: hostUrl, directory: skillsDir, manifest }) + const { actions } = await syncSkills({ host: HOST, directory: skillsDir, manifest }) expect(actions).toContainEqual({ name: 'my-team-skill', action: 'ignored', detail: 'not a Netlify skill' }) expect(actions).toContainEqual({ @@ -335,27 +325,26 @@ describe('agent skills', () => { test('reinstalls over an empty directory carrying a skill name', async () => { await mkdir(join(skillsDir, FUNCTIONS.name), { recursive: true }) - const manifest = await fetchSkillsManifest(hostUrl) - const { actions } = await syncSkills({ host: hostUrl, directory: skillsDir, manifest }) + const { actions } = await syncSkills({ host: HOST, directory: skillsDir, manifest }) expect(actions).toContainEqual({ name: 'netlify-functions', action: 'added', detail: '2.0.0' }) await expect(readFile(join(skillsDir, 'netlify-functions', 'SKILL.md'), 'utf8')).resolves.toBe('# functions v2\n') }) test('refuses a manifest with an unsupported schema version', async () => { - host.corrupt('/manifest.json', JSON.stringify({ ...host.manifest, schema_version: 2 })) + responses.set('/manifest.json', JSON.stringify({ ...manifest, schema_version: 2 })) - await expect(fetchSkillsManifest(hostUrl)).rejects.toThrow(/schema 2 is not supported/) + await expect(fetchSkillsManifest(HOST)).rejects.toThrow(/schema 2 is not supported/) }) test('refuses an active skill without a tree hash', async () => { - const broken = structuredClone(host.manifest) + const broken = structuredClone(manifest) const functions = broken.skills.find(({ name }) => name === 'netlify-functions') if (functions) functions.tree_hash = null - host.corrupt('/manifest.json', JSON.stringify(broken)) + responses.set('/manifest.json', JSON.stringify(broken)) - await expect(fetchSkillsManifest(hostUrl)).rejects.toThrow(/has no tree_hash/) + await expect(fetchSkillsManifest(HOST)).rejects.toThrow(/has no tree_hash/) }) test('follows a symlinked skills root and still keeps edited copies there', async () => { @@ -363,9 +352,8 @@ describe('agent skills', () => { await writeSkill(sharedDir, DEPLOY.name, { ...DEPLOY.files, 'SKILL.md': '# my own notes\n' }) await mkdir(join(projectDir, '.agents')) await symlink(sharedDir, skillsDir) - const manifest = await fetchSkillsManifest(hostUrl) - const { actions } = await syncSkills({ host: hostUrl, directory: skillsDir, manifest }) + const { actions } = await syncSkills({ host: HOST, directory: skillsDir, manifest }) expect(actions).toEqual([ { name: 'netlify-deploy', action: 'kept', detail: 'edited locally' }, @@ -380,9 +368,8 @@ describe('agent skills', () => { await writeSkill(elsewhere, 'functions-source', FUNCTIONS_V1) await mkdir(skillsDir, { recursive: true }) await symlink(join(elsewhere, 'functions-source'), join(skillsDir, 'netlify-functions')) - const manifest = await fetchSkillsManifest(hostUrl) - const { actions } = await syncSkills({ host: hostUrl, directory: skillsDir, manifest }) + const { actions } = await syncSkills({ host: HOST, directory: skillsDir, manifest }) expect(actions).toContainEqual({ name: 'netlify-functions', action: 'kept', detail: 'edited locally' }) expect((await lstat(join(skillsDir, 'netlify-functions'))).isSymbolicLink()).toBe(true) @@ -391,20 +378,18 @@ describe('agent skills', () => { test('refuses to replace a directory that is not an unedited Netlify skill, even when asked directly', async () => { await writeSkill(skillsDir, FUNCTIONS.name, { 'SKILL.md': '# mine\n', 'notes.md': '# keep me\n' }) - const manifest = await fetchSkillsManifest(hostUrl) const functions = manifest.skills.find(({ name }) => name === 'netlify-functions') if (!functions) throw new Error('netlify-functions missing from manifest') - await expect(installSkill(hostUrl, skillsDir, functions)).rejects.toThrow(/already exists/) + await expect(installSkill(HOST, skillsDir, functions)).rejects.toThrow(/already exists/) await expect(readFile(join(skillsDir, 'netlify-functions', 'notes.md'), 'utf8')).resolves.toBe('# keep me\n') await expect(listDirectories(skillsDir)).resolves.toEqual(['netlify-functions']) }) test('survives a user directory that differs only by case and keeps syncing the rest', async () => { await writeSkill(skillsDir, 'Netlify-Functions', { 'SKILL.md': '# mine\n', 'notes.md': '# keep me\n' }) - const manifest = await fetchSkillsManifest(hostUrl) - const { actions } = await syncSkills({ host: hostUrl, directory: skillsDir, manifest }) + const { actions } = await syncSkills({ host: HOST, directory: skillsDir, manifest }) await expect(readFile(join(skillsDir, 'Netlify-Functions', 'notes.md'), 'utf8')).resolves.toBe('# keep me\n') expect(actions).toContainEqual({ name: 'netlify-deploy', action: 'added', detail: '2.0.0' }) @@ -412,39 +397,18 @@ describe('agent skills', () => { expect(functions?.action === 'added' || functions?.detail?.includes('already exists')).toBe(true) }) - test('keeps reporting a renamed copy on the second run without extra downloads', async () => { - await writeSkill(skillsDir, 'netlify-cli-and-deploy', DEPLOY.files, DEPLOY.executable) - const manifest = await fetchSkillsManifest(hostUrl) - await syncSkills({ host: hostUrl, directory: skillsDir, manifest }) - const requestsAfterInstall = host.requests.length - - const { actions } = await syncSkills({ host: hostUrl, directory: skillsDir, manifest }) - - expect(actions).toEqual([ - { - name: 'netlify-cli-and-deploy', - action: 'kept', - detail: 'now called netlify-deploy; this copy can be removed', - }, - { name: 'netlify-deploy', action: 'current', detail: '2.0.0' }, - { name: 'netlify-functions', action: 'current', detail: '2.0.0' }, - ]) - expect(host.requests.length).toBe(requestsAfterInstall) - }) - test('rejects a file whose bytes do not match the manifest and leaves no partial install', async () => { - host.corrupt('/skills/netlify-functions/SKILL.md', '# tampered\n') - const manifest = await fetchSkillsManifest(hostUrl) + responses.set('/skills/netlify-functions/SKILL.md', '# tampered\n') const functions = manifest.skills.find(({ name }) => name === 'netlify-functions') if (!functions) throw new Error('netlify-functions missing from manifest') - await expect(installSkill(hostUrl, skillsDir, functions)).rejects.toThrow(/hash mismatch/) + await expect(installSkill(HOST, skillsDir, functions)).rejects.toThrow(/hash mismatch/) await expect(stat(skillsDir)).rejects.toThrow(/ENOENT/) }) test('setupAgentSkills installs into the detected directories and reports a summary', async () => { await mkdir(join(projectDir, '.claude')) - const result = await setupAgentSkills({ workingDir: projectDir, env: { NETLIFY_SKILLS_HOST: hostUrl } }) + const result = await setupAgentSkills({ workingDir: projectDir, env: { NETLIFY_SKILLS_HOST: HOST } }) expect(result.installed).toBe(true) expect(result.directories).toEqual([join('.claude', 'skills')]) @@ -458,13 +422,21 @@ describe('agent skills', () => { }) test('setupAgentSkills does not throw when the host is unreachable', async () => { - const result = await setupAgentSkills({ - workingDir: projectDir, - env: { NETLIFY_SKILLS_HOST: 'http://127.0.0.1:1' }, - }) + vi.mocked(fetch).mockRejectedValue(new TypeError('fetch failed')) + + const result = await setupAgentSkills({ workingDir: projectDir, env: { NETLIFY_SKILLS_HOST: HOST } }) expect(result.installed).toBe(false) - expect(result.error).toBeDefined() + expect(result.error).toBe('fetch failed') await expect(readdir(projectDir)).resolves.toEqual([]) }) + + test('setupAgentSkills reports a timed out download in plain words', async () => { + vi.mocked(fetch).mockRejectedValue(new DOMException('The operation was aborted due to timeout', 'TimeoutError')) + + const result = await setupAgentSkills({ workingDir: projectDir, env: { NETLIFY_SKILLS_HOST: HOST } }) + + expect(result.installed).toBe(false) + expect(result.error).toBe('the download timed out after 10s') + }) })