From ff8cd13ea74193927f7d9a902f824019a8ad1779 Mon Sep 17 00:00:00 2001 From: Domitrius Clark Date: Fri, 2 Oct 2026 14:35:48 -0400 Subject: [PATCH 1/3] feat(init): sync installed skills with the manifest Removes unedited deprecated skills, migrates unedited copies under a prior name to the current name, and cleans up staging and backup directories left by an interrupted install. Edited copies are kept and reported; --reset-context replaces, migrates or deletes them. A directory that stops early still reports what it finished. Co-Authored-By: Claude Fable 5.1 --- docs/commands/init.md | 1 + src/commands/init/index.ts | 9 +- src/commands/init/init.ts | 16 +- src/utils/init/agent-skills.ts | 407 ++++++++++++++----- tests/integration/commands/init/init.test.ts | 201 ++++++--- tests/unit/utils/init/agent-skills.test.ts | 343 ++++++++++++++-- 6 files changed, 787 insertions(+), 190 deletions(-) diff --git a/docs/commands/init.md b/docs/commands/init.md index 2ba0ddb24b2..c4827dceda6 100644 --- a/docs/commands/init.md +++ b/docs/commands/init.md @@ -23,6 +23,7 @@ netlify init - `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 +- `reset-context` (*boolean*) - Replace locally edited Netlify skills with the latest release, and migrate or delete edited copies under renamed or deprecated names - `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 b7e446cd590..39d14acfc31 100644 --- a/src/commands/init/index.ts +++ b/src/commands/init/index.ts @@ -12,6 +12,10 @@ 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') + .option( + '--reset-context', + 'Replace locally edited Netlify skills with the latest release, and migrate or delete edited copies under renamed or deprecated names', + ) .addHelpText('after', () => { const docsUrl = 'https://docs.netlify.com/cli/get-started/' return ` @@ -20,5 +24,8 @@ 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, { setupAgentSkills: !options.skipAgentSetup }) + await init(options, command, { + setupAgentSkills: !options.skipAgentSetup, + resetContext: Boolean(options.resetContext), + }) }) diff --git a/src/commands/init/init.ts b/src/commands/init/init.ts index ec4b054d6ff..2a66d01f514 100644 --- a/src/commands/init/init.ts +++ b/src/commands/init/init.ts @@ -225,15 +225,17 @@ type InitExtraOptions = { customizeExitMessage?: InitExitMessageCustomizer | undefined exitAfterConfiguringRepo?: boolean | undefined setupAgentSkills?: boolean | undefined + resetContext?: boolean | undefined } -const installAgentSkills = async (command: BaseCommand): Promise => { +const installAgentSkills = async (command: BaseCommand, reset: boolean): Promise => { log() - const result = await setupAgentSkills({ workingDir: command.netlify.repositoryRoot }) + const result = await setupAgentSkills({ workingDir: command.netlify.repositoryRoot, reset }) await track('sites_agentSkillsSetup', { installed: result.installed, directories: result.directories, skillsVersion: result.skillsVersion, + resetContext: reset, ...result.summary, }) } @@ -245,9 +247,15 @@ export const init = async ( customizeExitMessage, exitAfterConfiguringRepo = false, setupAgentSkills: shouldSetupAgentSkills = false, + resetContext = false, }: InitExtraOptions = {}, ): Promise => { - command.setAnalyticsPayload({ manual: options.manual, force: options.force, skipAgentSetup: options.skipAgentSetup }) + command.setAnalyticsPayload({ + manual: options.manual, + force: options.force, + skipAgentSetup: options.skipAgentSetup, + resetContext: options.resetContext, + }) const { repositoryRoot, state } = command.netlify const { siteInfo: existingSiteInfo } = command.netlify @@ -259,7 +267,7 @@ export const init = async ( await ensureNetlifyIgnore(repositoryRoot) if (shouldSetupAgentSkills) { - await installAgentSkills(command) + await installAgentSkills(command, resetContext) } const repoUrl = getRepoUrl(existingSiteInfo) diff --git a/src/utils/init/agent-skills.ts b/src/utils/init/agent-skills.ts index 9ffef043cba..2dd2203596e 100644 --- a/src/utils/init/agent-skills.ts +++ b/src/utils/init/agent-skills.ts @@ -17,6 +17,8 @@ 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 FETCH_TIMEOUT_MS = 10_000 const LOOPBACK_HOSTS = new Set(['localhost', '127.0.0.1', '[::1]']) @@ -49,8 +51,9 @@ 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 } - | { name: string; status: 'deprecated'; replacedBy: 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 { @@ -58,7 +61,7 @@ export interface SkillsClassification { missing: string[] } -export type SkillAction = 'current' | 'added' | 'updated' | 'kept' | 'ignored' +export type SkillAction = 'current' | 'added' | 'updated' | 'reset' | 'renamed' | 'removed' | 'kept' | 'ignored' export interface SkillActionRecord { name: string @@ -75,7 +78,17 @@ class SkillsError extends Error {} class SkillConflictError extends SkillsError {} -const errorMessage = (error: unknown): string => { +export class SkillsSyncInterrupted extends SkillsError { + partial: SkillsSyncResult + + constructor(cause: unknown, partial: SkillsSyncResult) { + super(errorMessage(cause), { cause }) + this.name = 'SkillsSyncInterrupted' + this.partial = partial + } +} + +function 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` } @@ -159,6 +172,9 @@ const indexManifest = (manifest: SkillsManifest): ManifestIndex => { return { exact, prior } } +const skillUnderName = ({ exact, prior }: ManifestIndex, name: string): ManifestSkill | undefined => + exact.get(name) ?? prior.get(name) + export const fetchSkillsManifest = async (host: string): Promise => { const url = urlFor(host, 'manifest.json') const bytes = await fetchBytes(url) @@ -250,9 +266,33 @@ const isDirectoryOrLinkToOne = async (dir: string): Promise => { } } +const isSameEntry = async (a: string, b: string): Promise => { + try { + const [statA, statB] = await Promise.all([fs.lstat(a), fs.lstat(b)]) + return statA.ino === statB.ino && statA.dev === statB.dev + } catch { + return false + } +} + const sortByName = (entries: T[]): T[] => [...entries].sort((a, b) => a.name.localeCompare(b.name, 'en')) +const hashIfPossible = async (dir: string): Promise => { + try { + return await hashSkillTree(dir) + } catch { + return null + } +} + +const isUneditedRelease = async (dir: string, skill: ManifestSkill): Promise => { + if (!(await isDirectory(dir))) { + return false + } + return lastMatching(skill, await hashIfPossible(dir)) !== undefined +} + export const classifySkillsDirectory = async ( root: string, manifest: SkillsManifest, @@ -269,44 +309,53 @@ export const classifySkillsDirectory = async ( for (const entry of entries) { const known = exact.get(entry.name) const renamed = prior.get(entry.name) - if (!entry.isDirectory() && !known && !renamed) continue + 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 = entry.isDirectory() && (await isFile(path.join(dir, 'SKILL.md'))) - if (!hasSkillMd && !known && !renamed) continue + if (!hasSkillMd && known?.status !== 'active') 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 }) + const treeHash = hasSkillMd ? await hashIfPossible(dir) : null + + if (retired) { + records.push({ + name: entry.name, + status: 'deprecated', + replacedBy: retired.deprecated?.replaced_by ?? null, + modified: lastMatching(retired, treeHash) === undefined, + }) continue } - if (!known && renamed) { - records.push({ name: entry.name, status: 'renamed', currentName: renamed.name }) + + if (!target) { + const twin = manifest.skills.find( + (skill) => skill.status === 'active' && lastMatching(skill, treeHash) !== undefined, + ) + records.push( + twin + ? { name: entry.name, status: 'duplicate', currentName: twin.name } + : { name: entry.name, status: 'unknown' }, + ) continue } - if (!known) { - records.push({ name: entry.name, status: 'unknown' }) + + const match = lastMatching(target, treeHash) + if (target === renamed) { + records.push({ name: entry.name, status: 'renamed', currentName: target.name, modified: match === undefined }) continue } - presentActive.add(known.name) - let treeHash: string | null = null - if (hasSkillMd) { - try { - treeHash = await hashSkillTree(dir) - } catch { - treeHash = null - } - } - const match = lastMatching(known, treeHash) - if (treeHash === known.tree_hash) { - records.push({ name: entry.name, status: 'current', version: known.version }) + 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: known.version, have: match.version }) + records.push({ name: entry.name, status: 'stale', version: target.version, have: match.version }) } else { - records.push({ name: entry.name, status: 'modified', version: known.version }) + records.push({ name: entry.name, status: 'modified', version: target.version }) } } @@ -319,20 +368,24 @@ export const classifySkillsDirectory = async ( } const replaceDirectory = async (staged: string, target: string): Promise => { - const exists = await isDirectory(target) + let existing = await fs.lstat(target).catch(() => null) + if (existing && !existing.isDirectory()) { + await fs.rm(target, { force: true }) + existing = null + } const retired = `${target}.old-${process.pid.toString()}-${randomBytes(6).toString('hex')}` - if (exists) { + if (existing) { await fs.rename(target, retired) } try { await fs.rename(staged, target) } catch (error) { - if (exists) { + if (existing) { await fs.rename(retired, target) } throw error } - if (exists) { + if (existing) { await fs.rm(retired, { recursive: true, force: true }) } } @@ -344,12 +397,7 @@ const isReplaceableCopy = async (target: string, skill: ManifestSkill): Promise< 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 - } + return isUneditedRelease(target, skill) } const downloadSkill = async (host: string, skill: ManifestSkill): Promise<[string, Uint8Array][]> => { @@ -372,10 +420,15 @@ const downloadSkill = async (host: string, skill: ManifestSkill): Promise<[strin return downloaded } -export const installSkill = async (host: string, dest: string, skill: ManifestSkill): Promise => { +export const installSkill = async ( + host: string, + dest: string, + skill: ManifestSkill, + { force = false }: { force?: boolean } = {}, +): Promise => { const downloaded = await downloadSkill(host, skill) const target = path.join(dest, skill.name) - if (!(await isReplaceableCopy(target, skill))) { + if (!force && !(await isReplaceableCopy(target, skill))) { throw new SkillConflictError(`${target} already exists and is not an unedited Netlify skill; left in place`) } @@ -396,71 +449,188 @@ export const installSkill = async (host: string, dest: string, skill: ManifestSk return downloaded.length } +const removeInstallLeftovers = async (root: string, index: ManifestIndex): Promise => { + if (!(await isDirectoryOrLinkToOne(root))) { + return [] + } + const removed: string[] = [] + for (const entry of sortByName(await fs.readdir(root, { withFileTypes: true }))) { + if (!entry.isDirectory()) continue + const leftover = path.join(root, entry.name) + const staging = STAGING_LEFTOVER.exec(entry.name) + if (staging) { + if (skillUnderName(index, staging[1])) { + await fs.rm(leftover, { recursive: true, force: true }) + removed.push(entry.name) + } + continue + } + const retired = RETIRED_LEFTOVER.exec(entry.name) + const skill = retired ? skillUnderName(index, retired[1]) : undefined + if (!retired || !skill) continue + const original = path.join(root, retired[1]) + if (!(await exists(original))) { + await fs.rename(leftover, original) + } else if (await isUneditedRelease(leftover, skill)) { + await fs.rm(leftover, { recursive: true, force: true }) + removed.push(entry.name) + } + } + return removed +} + export const syncSkills = async ({ host, directory, manifest, + reset = false, }: { host: string directory: string manifest: SkillsManifest + reset?: boolean }): Promise => { - const { exact } = indexManifest(manifest) - const before = await classifySkillsDirectory(directory, manifest) + const index = indexManifest(manifest) const actions: SkillActionRecord[] = [] 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) + const skill = skillUnderName(index, name) if (!skill) { throw new SkillsError(`manifest: unknown skill ${name}`) } return skill } + const stillUnedited = async (name: string, skill: ManifestSkill): Promise => + reset || isUneditedRelease(path.join(directory, name), skill) + const removeUnlessSame = async (name: string, keep: string): Promise => { + if (await isSameEntry(path.join(directory, name), path.join(directory, keep))) return + await fs.rm(path.join(directory, name), { recursive: true, force: true }) + } - 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) + const applySyncRules = async () => { + for (const leftover of await removeInstallLeftovers(directory, index)) { + act(leftover, 'removed', 'leftover from an interrupted install') + } + const before = await classifySkillsDirectory(directory, manifest) + + const foreign = before.skills + .filter(({ status }) => status === 'duplicate' || status === 'unknown') + .map(({ name }) => name) + const occupantOf = async (name: string): Promise => { + for (const other of foreign) { + if (await isSameEntry(path.join(directory, other), path.join(directory, name))) return other + } + return undefined } - } - for (const record of before.skills) { - switch (record.status) { - case 'current': - act(record.name, 'current', record.version ?? undefined) - break - 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 - case 'renamed': - act(record.name, 'kept', `now called ${record.currentName}; this copy can be removed`) - break - case 'deprecated': - act(record.name, 'kept', `deprecated${record.replacedBy ? `; use ${record.replacedBy}` : ''}`) - break - case 'unknown': - act(record.name, 'ignored', 'not a Netlify skill') - break + const installed = new Set() + const conflicted = new Set() + const install = async (skill: ManifestSkill, options: { force?: boolean } = {}): Promise => { + if (conflicted.has(skill.name)) return false + const occupant = await occupantOf(skill.name) + if (occupant) { + conflicted.add(skill.name) + act(skill.name, 'kept', `${occupant} already uses this name; left in place`) + return false + } + try { + await installSkill(host, directory, skill, options) + installed.add(skill.name) + return true + } catch (error) { + if (!(error instanceof SkillConflictError)) throw error + conflicted.add(skill.name) + act(skill.name, 'kept', error.message) + return false + } } - } - for (const name of before.missing) { - await install(name, (skill) => { - act(name, 'added', skill.version ?? undefined) - }) + 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) + if (await install(skill)) { + act(record.name, 'updated', `${record.have ?? 'unknown'} -> ${skill.version ?? 'latest'}`) + } + break + } + case 'modified': { + if (!reset) { + act(record.name, 'kept', 'edited locally') + break + } + const skill = skillByName(record.name) + await install(skill, { force: true }) + act(record.name, 'reset', `edited copy replaced with ${skill.version ?? 'latest'}`) + break + } + case 'renamed': { + const skill = skillByName(record.currentName) + const current = before.skills.find((other) => other.name === record.currentName) + const currentPresent = current !== undefined || installed.has(record.currentName) + if (!reset && record.modified) { + act(record.name, 'kept', `edited locally; now called ${record.currentName}`) + break + } + if (!reset && current?.status === 'modified') { + act(record.name, 'kept', `${record.currentName} is already installed and edited locally`) + break + } + if (!currentPresent && !(await install(skill, { force: reset }))) { + act(record.name, 'kept', `now called ${record.currentName}, which could not be installed`) + break + } + if (!(await stillUnedited(record.name, skill))) { + act(record.name, 'kept', `edited locally; now called ${record.currentName}`) + break + } + await removeUnlessSame(record.name, record.currentName) + if (currentPresent) { + act(record.name, 'removed', `superseded by ${record.currentName}`) + } else { + act(record.name, 'renamed', `-> ${record.currentName}`) + } + break + } + case 'deprecated': { + const retired = skillByName(record.name) + const replacement = record.replacedBy ? `; use ${record.replacedBy}` : '' + if ((record.modified && !reset) || !(await stillUnedited(record.name, retired))) { + act(record.name, 'kept', `deprecated${replacement}, but edited locally`) + break + } + await fs.rm(path.join(directory, record.name), { 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) + if (await install(skill)) { + act(name, 'added', skill.version ?? undefined) + } + } } + try { + await applySyncRules() + } catch (error) { + throw new SkillsSyncInterrupted(error, { directory, actions }) + } return { directory, actions } } @@ -483,7 +653,16 @@ export const resolveSkillsDirectories = async ( } const summarize = (actions: SkillActionRecord[]): Record => { - const summary: Record = { current: 0, added: 0, updated: 0, kept: 0, ignored: 0 } + const summary: Record = { + current: 0, + added: 0, + updated: 0, + reset: 0, + renamed: 0, + removed: 0, + kept: 0, + ignored: 0, + } for (const { action } of actions) { summary[action] += 1 } @@ -493,15 +672,33 @@ const summarize = (actions: SkillActionRecord[]): Record => const describeSync = ({ directory, actions }: SkillsSyncResult): string => { const summary = summarize(actions) const location = chalk.underline(directory) - if (summary.added + summary.updated === 0) { + const changed = summary.added + summary.updated + summary.reset + summary.renamed + summary.removed + 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.reset > 0 ? `${summary.reset.toString()} reset` : '', + summary.renamed > 0 ? `${summary.renamed.toString()} renamed` : '', + summary.removed > 0 ? `${summary.removed.toString()} removed` : '', summary.kept > 0 ? `${summary.kept.toString()} kept` : '', ].filter(Boolean) - return `Installed Netlify skills in ${location} (${parts.join(', ')}).` + const verb = changed === summary.added ? 'Installed' : 'Synced' + return `${verb} Netlify skills in ${location} (${parts.join(', ')}).` +} + +const logSync = (directory: string, result: SkillsSyncResult, reset: boolean): void => { + log(describeSync({ directory, actions: result.actions })) + const kept = result.actions.filter(({ action }) => action === 'kept') + for (const { name, detail } of kept) { + log(` ${chalk.dim(name)}: ${detail ?? 'kept'}`) + } + if (kept.length > 0 && !reset) { + log( + ` Run ${chalk.cyanBright.bold(`${netlifyCommand()} init --reset-context`)} to replace edited Netlify skills with the latest release.`, + ) + } } export interface AgentSkillsSetupSummary { @@ -515,25 +712,19 @@ export interface AgentSkillsSetupSummary { export const setupAgentSkills = async ({ workingDir, env = process.env, + reset = false, }: { workingDir: string env?: NodeJS.ProcessEnv + reset?: boolean }): Promise => { let directories: string[] = [] + let host: string + let manifest: SkillsManifest try { directories = await resolveSkillsDirectories(workingDir, env) - 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 })) - 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) } + host = resolveSkillsHost(env) + manifest = await fetchSkillsManifest(host) } catch (error) { const message = errorMessage(error) warn(`Could not set up Netlify skills for AI agents: ${message}`) @@ -542,4 +733,34 @@ export const setupAgentSkills = async ({ ) return { installed: false, directories, summary: summarize([]), error: message } } + + const actions: SkillActionRecord[] = [] + const errors: string[] = [] + for (const directory of directories) { + try { + const result = await syncSkills({ host, directory: path.resolve(workingDir, directory), manifest, reset }) + actions.push(...result.actions) + logSync(directory, result, reset) + } catch (error) { + if (error instanceof SkillsSyncInterrupted) { + actions.push(...error.partial.actions) + if (error.partial.actions.length > 0) { + logSync(directory, error.partial, reset) + } + } + const message = errorMessage(error) + errors.push(message) + warn(`Netlify skills sync in ${chalk.underline(directory)} stopped early: ${message}`) + } + } + if (errors.length > 0) { + log(`Run ${chalk.cyanBright.bold(`${netlifyCommand()} init`)} again to finish syncing Netlify skills.`) + } + return { + installed: errors.length === 0, + directories, + skillsVersion: manifest.version, + summary: summarize(actions), + ...(errors.length > 0 ? { error: errors.join('; ') } : {}), + } } diff --git a/tests/integration/commands/init/init.test.ts b/tests/integration/commands/init/init.test.ts index 0a403887c32..92462bfc446 100644 --- a/tests/integration/commands/init/init.test.ts +++ b/tests/integration/commands/init/init.test.ts @@ -20,36 +20,59 @@ 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 }], - }, - ], +type SkillFiles = Record + +const treeHashOf = (files: SkillFiles) => { + const hash = createHash('sha256') + for (const file of Object.keys(files).sort()) { + hash.update(`${file}\u0000100644\u0000${sha256(files[file]).replace(/^sha256:/, '')}\n`) } + return `sha256:${hash.digest('hex')}` +} + +interface HostedSkill { + files: SkillFiles + status?: 'active' | 'deprecated' + previous?: Record } -const withSkillsHost = async (handler: (host: { url: string; requests: string[] }) => Promise) => { +const DEFAULT_HOSTED_SKILLS: Record = { + 'netlify-functions': { files: { 'SKILL.md': SKILL_CONTENT } }, +} + +const skillsManifest = (skills: Record) => ({ + schema_version: 1, + version: '1.0.0', + skills: Object.entries(skills).map(([name, { files, status = 'active', previous = {} }]) => { + const current = status === 'active' ? treeHashOf(files) : null + return { + name, + status, + version: current ? '1.0.0' : null, + prior_names: [], + description: name, + tree_hash: current, + files: Object.fromEntries(Object.entries(files).map(([file, content]) => [file, sha256(content)])), + executable: [], + history: [ + ...Object.entries(previous).map(([version, oldFiles]) => ({ version, tree_hash: treeHashOf(oldFiles) })), + ...(current ? [{ version: '1.0.0', tree_hash: current }] : []), + ], + } + }), +}) + +const withSkillsHost = async ( + handler: (host: { url: string; requests: string[] }) => Promise, + skills: Record = DEFAULT_HOSTED_SKILLS, +) => { const requests: string[] = [] - const responses = new Map([ - ['/manifest.json', JSON.stringify(skillsManifest())], - ['/skills/netlify-functions/SKILL.md', SKILL_CONTENT], - ]) + const responses = new Map([['/manifest.json', JSON.stringify(skillsManifest(skills))]]) + for (const [name, { files }] of Object.entries(skills)) { + for (const [file, content] of Object.entries(files)) { + responses.set(`/skills/${name}/${file}`, content) + } + } const server = createServer((req, res) => { const url = req.url ?? '' requests.push(url) @@ -760,53 +783,56 @@ 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 linkedSiteInfo = { + 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 linkedSiteRoutes = [ + { path: 'accounts', response: [{ slug: 'test-account' }] }, + { path: 'sites/site_id/service-instances', response: [] }, + { path: 'sites/site_id', response: linkedSiteInfo }, + { path: 'sites', response: [linkedSiteInfo] }, + { 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 }, + ] + const initOnLinkedSite = + ({ apiUrl, cwd, skillsHost }: { apiUrl: string; cwd: string; skillsHost: string }) => + async (...flags: string[]) => { + const env = { + NETLIFY_API_URL: apiUrl, + NETLIFY_SITE_ID: 'site_id', + NETLIFY_AUTH_TOKEN: 'fake-token', + NETLIFY_SKILLS_HOST: skillsHost, + } + const childProcess = execa(cliPath, ['init', '--manual', ...flags], { cwd, env }) + if (process.env.DEBUG_TESTS) { + childProcess.stdout?.on('data', (data: Buffer) => { + process.stderr.write(data) + }) + } + handleQuestions(childProcess, manualQuestions()) + return await childProcess } - 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 }, - ] + test('netlify init installs Netlify skills for AI agents by default and is idempotent', async (t) => { await withSiteBuilder(t, async (builder) => { await builder.withGit().ensureDirectoryExists(path.join(builder.directory, '.agents')).build() - await withMockApi(routes, async ({ apiUrl }) => { + await withMockApi(linkedSiteRoutes, 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 runInit = initOnLinkedSite({ apiUrl, cwd: builder.directory, skillsHost: skillsHost.url }) const first = await runInit() t.expect(first.stdout).toContain('Installed Netlify skills') @@ -825,4 +851,47 @@ describe.concurrent('commands/init', () => { }) }) }) + + test('netlify init syncs installed skills: updates stale copies, removes deprecated ones, keeps edits until --reset-context', async (t) => { + await withSiteBuilder(t, async (builder) => { + const skillsDir = path.join('.agents', 'skills') + await builder + .withGit() + .withContentFiles([ + { path: path.join(skillsDir, 'netlify-functions', 'SKILL.md'), content: '# functions (old)\n' }, + { path: path.join(skillsDir, 'netlify-blobs', 'SKILL.md'), content: '# blobs plus my notes\n' }, + { path: path.join(skillsDir, 'netlify-db', 'SKILL.md'), content: '# db\n' }, + ]) + .build() + const skillFile = (name: string) => readFile(path.join(builder.directory, skillsDir, name, 'SKILL.md'), 'utf8') + + await withMockApi(linkedSiteRoutes, async ({ apiUrl }) => { + await withSkillsHost( + async (skillsHost) => { + const runInit = initOnLinkedSite({ apiUrl, cwd: builder.directory, skillsHost: skillsHost.url }) + + const first = await runInit() + t.expect(first.stdout).toMatch(/Synced Netlify skills in .*\(1 updated, 1 removed, 1 kept\)\./) + t.expect(first.stdout).toContain('netlify-blobs: edited locally') + t.expect(first.stdout).toContain('--reset-context') + await t.expect(skillFile('netlify-functions')).resolves.toBe('# functions\n') + await t.expect(skillFile('netlify-blobs')).resolves.toBe('# blobs plus my notes\n') + await t.expect(skillFile('netlify-db')).rejects.toThrow(/ENOENT/) + + const second = await runInit('--reset-context') + t.expect(second.stdout).toMatch(/Synced Netlify skills in .*\(1 reset\)\./) + await t.expect(skillFile('netlify-blobs')).resolves.toBe('# blobs\n') + }, + { + 'netlify-functions': { + files: { 'SKILL.md': '# functions\n' }, + previous: { '0.9.0': { 'SKILL.md': '# functions (old)\n' } }, + }, + 'netlify-blobs': { files: { 'SKILL.md': '# blobs\n' } }, + 'netlify-db': { files: {}, status: 'deprecated', previous: { '0.9.0': { 'SKILL.md': '# db\n' } } }, + }, + ) + }) + }) + }) }) diff --git a/tests/unit/utils/init/agent-skills.test.ts b/tests/unit/utils/init/agent-skills.test.ts index 5d327f4ab21..e35a76d0499 100644 --- a/tests/unit/utils/init/agent-skills.test.ts +++ b/tests/unit/utils/init/agent-skills.test.ts @@ -10,6 +10,7 @@ import { DEFAULT_SKILLS_HOST, type ManifestSkill, type SkillsManifest, + SkillsSyncInterrupted, classifySkillsDirectory, fetchSkillsManifest, hashSkillTree, @@ -19,6 +20,7 @@ import { setupAgentSkills, syncSkills, } from '../../../../src/utils/init/agent-skills.js' +import { log } from '../../../../src/utils/command-helpers.js' vi.mock('../../../../src/utils/command-helpers.js', async (importOriginal) => ({ ...(await importOriginal()), @@ -130,12 +132,15 @@ const listDirectories = async (root: string) => .map((entry) => entry.name) .sort() +const loggedLines = () => vi.mocked(log).mock.calls.map(([line]) => String(line)) + describe('agent skills', () => { let projectDir: string let skillsDir: string beforeEach(async () => { vi.stubGlobal('fetch', vi.fn()) + vi.mocked(log).mockClear() projectDir = await mkdtemp(join(tmpdir(), 'agent-skills-')) skillsDir = join(projectDir, '.agents', 'skills') }) @@ -200,6 +205,12 @@ describe('agent skills', () => { let manifest: SkillsManifest let responses: Map + const manifestSkill = (name: string): ManifestSkill => { + const skill = manifest.skills.find((candidate) => candidate.name === name) + if (!skill) throw new Error(`${name} missing from manifest`) + return skill + } + beforeEach(() => { manifest = buildManifest(skills, [deprecatedSkill('netlify-legacy', RETIRED_FILES, 'netlify-deploy')]) responses = hostedResponses(manifest, skills) @@ -266,15 +277,76 @@ describe('agent skills', () => { await expect(readFile(join(skillsDir, 'netlify-deploy', 'SKILL.md'), 'utf8')).resolves.toBe('# my own notes\n') }) - test('installs the new name beside a copy under a prior name and leaves the old copy alone', async () => { + test('--reset-context replaces a locally edited copy with the latest release', async () => { + await writeSkill(skillsDir, DEPLOY.name, { ...DEPLOY.files, 'SKILL.md': '# my own notes\n', 'notes.md': 'x\n' }) + + const { actions } = await syncSkills({ host: HOST, directory: skillsDir, manifest, reset: true }) + + expect(actions).toContainEqual({ + name: 'netlify-deploy', + action: 'reset', + detail: 'edited copy replaced with 2.0.0', + }) + await expect(readFile(join(skillsDir, 'netlify-deploy', 'SKILL.md'), 'utf8')).resolves.toBe('# deploy\n') + await expect(readdir(join(skillsDir, 'netlify-deploy'))).resolves.toEqual(['SKILL.md', 'scripts']) + }) + + test('--reset-context replaces a symlink standing in for a skill without touching its target', async () => { + const elsewhere = join(projectDir, 'elsewhere') + await writeSkill(elsewhere, 'functions-source', FUNCTIONS_V1) + await mkdir(skillsDir, { recursive: true }) + await symlink(join(elsewhere, 'functions-source'), join(skillsDir, 'netlify-functions')) + + const { actions } = await syncSkills({ host: HOST, directory: skillsDir, manifest, reset: true }) + + expect(actions).toContainEqual({ + name: 'netlify-functions', + action: 'reset', + detail: 'edited copy replaced with 2.0.0', + }) + expect((await lstat(join(skillsDir, 'netlify-functions'))).isDirectory()).toBe(true) + await expect(readFile(join(skillsDir, 'netlify-functions', 'SKILL.md'), 'utf8')).resolves.toBe('# functions v2\n') + await expect(readFile(join(elsewhere, 'functions-source', 'SKILL.md'), 'utf8')).resolves.toBe('# functions v1\n') + }) + + test('migrates an unedited copy under a prior name to the current name', async () => { await writeSkill(skillsDir, 'netlify-cli-and-deploy', DEPLOY.files, DEPLOY.executable) const { actions } = await syncSkills({ host: HOST, directory: skillsDir, manifest }) + expect(actions).toEqual([ + { name: 'netlify-cli-and-deploy', action: 'renamed', detail: '-> netlify-deploy' }, + { 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-deploy', 'SKILL.md'), 'utf8')).resolves.toBe('# deploy\n') + }) + + test('removes an unedited prior-name copy when the current name is already installed', async () => { + await writeSkill(skillsDir, 'netlify-cli-and-deploy', DEPLOY.files, DEPLOY.executable) + await writeSkill(skillsDir, DEPLOY.name, DEPLOY.files, DEPLOY.executable) + await writeSkill(skillsDir, FUNCTIONS.name, FUNCTIONS.files) + + const { actions } = await syncSkills({ host: HOST, directory: skillsDir, manifest }) + + expect(actions).toEqual([ + { name: 'netlify-cli-and-deploy', action: 'removed', detail: 'superseded by netlify-deploy' }, + { name: 'netlify-deploy', action: 'current', detail: '2.0.0' }, + { name: 'netlify-functions', action: 'current', detail: '2.0.0' }, + ]) + await expect(listDirectories(skillsDir)).resolves.toEqual(['netlify-deploy', 'netlify-functions']) + expect(requestedPaths()).toEqual([]) + }) + + test('keeps an edited prior-name copy and still installs the current name beside it', async () => { + await writeSkill(skillsDir, 'netlify-cli-and-deploy', { 'SKILL.md': '# my deploy notes\n' }) + + const { actions } = await syncSkills({ host: HOST, directory: skillsDir, manifest }) + expect(actions).toContainEqual({ name: 'netlify-cli-and-deploy', action: 'kept', - detail: 'now called netlify-deploy; this copy can be removed', + detail: 'edited locally; now called netlify-deploy', }) expect(actions).toContainEqual({ name: 'netlify-deploy', action: 'added', detail: '2.0.0' }) await expect(listDirectories(skillsDir)).resolves.toEqual([ @@ -284,26 +356,42 @@ describe('agent skills', () => { ]) }) - test('keeps reporting a renamed copy on the second run without extra downloads', async () => { + test('keeps a prior-name copy when the current name is installed and edited', async () => { await writeSkill(skillsDir, 'netlify-cli-and-deploy', DEPLOY.files, DEPLOY.executable) - await syncSkills({ host: HOST, directory: skillsDir, manifest }) - vi.mocked(fetch).mockClear() + await writeSkill(skillsDir, DEPLOY.name, { 'SKILL.md': '# my deploy notes\n' }) 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([]) + expect(actions).toContainEqual({ + name: 'netlify-cli-and-deploy', + action: 'kept', + detail: 'netlify-deploy is already installed and edited locally', + }) + expect(actions).toContainEqual({ name: 'netlify-deploy', action: 'kept', detail: 'edited locally' }) + await expect(readFile(join(skillsDir, 'netlify-cli-and-deploy', 'SKILL.md'), 'utf8')).resolves.toBe('# deploy\n') + }) + + test('--reset-context migrates an edited prior-name copy over an edited current one', async () => { + await writeSkill(skillsDir, 'netlify-cli-and-deploy', { 'SKILL.md': '# my deploy notes\n' }) + await writeSkill(skillsDir, DEPLOY.name, { 'SKILL.md': '# other notes\n' }) + + const { actions } = await syncSkills({ host: HOST, directory: skillsDir, manifest, reset: true }) + + expect(actions).toContainEqual({ + name: 'netlify-cli-and-deploy', + action: 'removed', + detail: 'superseded by netlify-deploy', + }) + expect(actions).toContainEqual({ + name: 'netlify-deploy', + action: 'reset', + detail: 'edited copy replaced with 2.0.0', + }) + await expect(listDirectories(skillsDir)).resolves.toEqual(['netlify-deploy', 'netlify-functions']) + await expect(readFile(join(skillsDir, 'netlify-deploy', 'SKILL.md'), 'utf8')).resolves.toBe('# deploy\n') }) - test('reports a deprecated skill and leaves it and unknown directories alone', async () => { + test('deletes an unedited deprecated skill and leaves unknown directories alone', async () => { await writeSkill(skillsDir, 'netlify-legacy', RETIRED_FILES) await writeSkill(skillsDir, 'my-team-skill', { 'SKILL.md': '# ours\n' }) @@ -312,17 +400,88 @@ describe('agent skills', () => { expect(actions).toContainEqual({ name: 'my-team-skill', action: 'ignored', detail: 'not a Netlify skill' }) expect(actions).toContainEqual({ name: 'netlify-legacy', - action: 'kept', + action: 'removed', detail: 'deprecated; use netlify-deploy', }) await expect(listDirectories(skillsDir)).resolves.toEqual([ 'my-team-skill', 'netlify-deploy', 'netlify-functions', - 'netlify-legacy', ]) }) + test('keeps an edited deprecated skill until --reset-context', async () => { + await writeSkill(skillsDir, 'netlify-legacy', { 'SKILL.md': '# retired, with my notes\n' }) + + const first = await syncSkills({ host: HOST, directory: skillsDir, manifest }) + expect(first.actions).toContainEqual({ + name: 'netlify-legacy', + action: 'kept', + detail: 'deprecated; use netlify-deploy, but edited locally', + }) + await expect(listDirectories(skillsDir)).resolves.toContain('netlify-legacy') + + const second = await syncSkills({ host: HOST, directory: skillsDir, manifest, reset: true }) + expect(second.actions).toContainEqual({ + name: 'netlify-legacy', + action: 'removed', + detail: 'deprecated; use netlify-deploy', + }) + await expect(listDirectories(skillsDir)).resolves.toEqual(['netlify-deploy', 'netlify-functions']) + }) + + test('reports a copy of a skill living under an unrelated name and never touches it', async () => { + await writeSkill(skillsDir, 'deploy-copy', DEPLOY.files, DEPLOY.executable) + + const { actions } = await syncSkills({ host: HOST, directory: skillsDir, manifest }) + + expect(actions).toContainEqual({ + name: 'deploy-copy', + action: 'ignored', + detail: 'copy of netlify-deploy under another name', + }) + await expect(listDirectories(skillsDir)).resolves.toEqual(['deploy-copy', 'netlify-deploy', 'netlify-functions']) + }) + + test('cleans up staging and backup directories left by an interrupted install', async () => { + await writeSkill(skillsDir, FUNCTIONS.name, FUNCTIONS.files) + await writeSkill(skillsDir, '.netlify-skill-netlify-deploy-Ab12Cd', { 'SKILL.md': '# half written\n' }) + await writeSkill(skillsDir, 'netlify-functions.old-123-0123456789ab', FUNCTIONS_V1) + await writeSkill(skillsDir, 'netlify-deploy.old-123-0123456789ab', DEPLOY.files, DEPLOY.executable) + await writeSkill(skillsDir, '.netlify-skill-someone-else-Ab12Cd', { 'SKILL.md': '# not ours\n' }) + + const { actions } = await syncSkills({ host: HOST, directory: skillsDir, manifest }) + + expect(actions.slice(0, 2)).toEqual([ + { + name: '.netlify-skill-netlify-deploy-Ab12Cd', + action: 'removed', + detail: 'leftover from an interrupted install', + }, + { + name: 'netlify-functions.old-123-0123456789ab', + action: 'removed', + detail: 'leftover from an interrupted install', + }, + ]) + expect(actions).toContainEqual({ name: 'netlify-deploy', action: 'current', detail: '2.0.0' }) + await expect(readdir(skillsDir)).resolves.toEqual([ + '.netlify-skill-someone-else-Ab12Cd', + 'netlify-deploy', + 'netlify-functions', + ]) + }) + + test('keeps a backup directory the user has edited', async () => { + await writeSkill(skillsDir, FUNCTIONS.name, FUNCTIONS.files) + await writeSkill(skillsDir, 'netlify-functions.old-123-0123456789ab', { 'SKILL.md': '# my recovery\n' }) + + const { actions } = await syncSkills({ host: HOST, directory: skillsDir, manifest }) + + expect(actions.map(({ action }) => action)).not.toContain('removed') + await expect(listDirectories(skillsDir)).resolves.toContain('netlify-functions.old-123-0123456789ab') + }) + test('reinstalls over an empty directory carrying a skill name', async () => { await mkdir(join(skillsDir, FUNCTIONS.name), { recursive: true }) @@ -378,10 +537,8 @@ 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 functions = manifest.skills.find(({ name }) => name === 'netlify-functions') - if (!functions) throw new Error('netlify-functions missing from manifest') - await expect(installSkill(HOST, skillsDir, functions)).rejects.toThrow(/already exists/) + await expect(installSkill(HOST, skillsDir, manifestSkill('netlify-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']) }) @@ -393,19 +550,125 @@ describe('agent skills', () => { 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) + const functions = actions.filter(({ name }) => name === 'netlify-functions') + expect(functions).toHaveLength(1) + const caseInsensitive = (await listDirectories(skillsDir)).length === 2 + expect(functions[0]).toEqual( + caseInsensitive + ? { + name: 'netlify-functions', + action: 'kept', + detail: 'Netlify-Functions already uses this name; left in place', + } + : { name: 'netlify-functions', action: 'added', detail: '2.0.0' }, + ) + }) + + test('never renames away a user directory that differs only by case but holds release content', async () => { + await writeSkill(skillsDir, 'Netlify-Deploy', DEPLOY.files, DEPLOY.executable) + + const { actions } = await syncSkills({ host: HOST, directory: skillsDir, manifest }) + + expect(actions).toContainEqual({ + name: 'Netlify-Deploy', + action: 'ignored', + detail: 'copy of netlify-deploy under another name', + }) + const directories = await listDirectories(skillsDir) + expect(directories).toContain('Netlify-Deploy') + if (!directories.includes('netlify-deploy')) { + expect(actions).toContainEqual({ + name: 'netlify-deploy', + action: 'kept', + detail: 'Netlify-Deploy already uses this name; left in place', + }) + } + }) + + test('migrates two prior names of one skill in a single run and is quiet afterwards', async () => { + const twice = buildManifest([FUNCTIONS, { ...DEPLOY, priorNames: ['netlify-cli-and-deploy', 'deploy-skill'] }]) + await writeSkill(skillsDir, 'netlify-cli-and-deploy', DEPLOY.files, DEPLOY.executable) + await writeSkill(skillsDir, 'deploy-skill', DEPLOY.files, DEPLOY.executable) + await writeSkill(skillsDir, FUNCTIONS.name, FUNCTIONS.files) + + const first = await syncSkills({ host: HOST, directory: skillsDir, manifest: twice }) + expect(first.actions).toEqual([ + { name: 'deploy-skill', action: 'renamed', detail: '-> netlify-deploy' }, + { name: 'netlify-cli-and-deploy', action: 'removed', detail: 'superseded by netlify-deploy' }, + { name: 'netlify-functions', action: 'current', detail: '2.0.0' }, + ]) + await expect(listDirectories(skillsDir)).resolves.toEqual(['netlify-deploy', 'netlify-functions']) + + const second = await syncSkills({ host: HOST, directory: skillsDir, manifest: twice }) + expect(second.actions.map(({ action }) => action)).toEqual(['current', 'current']) + }) + + test('deletes an unedited deprecated skill found under one of its prior names', async () => { + const legacy = { ...deprecatedSkill('netlify-legacy', RETIRED_FILES), prior_names: ['netlify-old'] } + const withLegacy = buildManifest(skills, [legacy]) + await writeSkill(skillsDir, 'netlify-old', RETIRED_FILES) + + const { actions } = await syncSkills({ host: HOST, directory: skillsDir, manifest: withLegacy }) + + expect(actions).toContainEqual({ name: 'netlify-old', action: 'removed', detail: 'deprecated' }) + await expect(listDirectories(skillsDir)).resolves.toEqual(['netlify-deploy', 'netlify-functions']) + }) + + test('leaves a directory without SKILL.md under a prior or deprecated name alone, even on --reset-context', async () => { + await writeSkill(skillsDir, 'netlify-cli-and-deploy', { 'notes.md': '# not a skill\n' }) + await writeSkill(skillsDir, 'netlify-legacy', { 'notes.md': '# not a skill either\n' }) + + const { actions } = await syncSkills({ host: HOST, directory: skillsDir, manifest, reset: true }) + + expect(actions.map(({ name }) => name)).not.toContain('netlify-cli-and-deploy') + 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', + 'netlify-legacy', + ]) + }) + + test('restores a backup left under a prior name and then migrates it', async () => { + await writeSkill(skillsDir, 'netlify-cli-and-deploy.old-123-0123456789ab', DEPLOY.files, DEPLOY.executable) + + const { actions } = await syncSkills({ host: HOST, directory: skillsDir, manifest }) + + expect(actions).toContainEqual({ name: 'netlify-cli-and-deploy', action: 'renamed', detail: '-> netlify-deploy' }) + await expect(listDirectories(skillsDir)).resolves.toEqual(['netlify-deploy', 'netlify-functions']) + }) + + test('--reset-context never forces over a directory that is not classified as a Netlify skill', async () => { + await writeSkill(skillsDir, 'Netlify-Functions', { 'SKILL.md': '# mine\n', 'notes.md': '# keep me\n' }) + + await syncSkills({ host: HOST, directory: skillsDir, manifest, reset: true }) + + await expect(readFile(join(skillsDir, 'Netlify-Functions', 'notes.md'), 'utf8')).resolves.toBe('# keep me\n') }) test('rejects a file whose bytes do not match the manifest and leaves no partial install', async () => { 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(HOST, skillsDir, functions)).rejects.toThrow(/hash mismatch/) + await expect(installSkill(HOST, skillsDir, manifestSkill('netlify-functions'))).rejects.toThrow(/hash mismatch/) await expect(stat(skillsDir)).rejects.toThrow(/ENOENT/) }) + test('a failure part way through carries the actions already taken', async () => { + await writeSkill(skillsDir, 'netlify-legacy', RETIRED_FILES) + responses.delete('/skills/netlify-functions/SKILL.md') + + const error = await syncSkills({ host: HOST, directory: skillsDir, manifest }).catch((caught: unknown) => caught) + + expect(error).toBeInstanceOf(SkillsSyncInterrupted) + const { partial } = error as SkillsSyncInterrupted + expect(partial.actions).toEqual([ + { name: 'netlify-legacy', action: 'removed', detail: 'deprecated; use netlify-deploy' }, + { name: 'netlify-deploy', action: 'added', detail: '2.0.0' }, + ]) + await expect(listDirectories(skillsDir)).resolves.toEqual(['netlify-deploy']) + }) + 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: HOST } }) @@ -418,6 +681,34 @@ describe('agent skills', () => { 'netlify-deploy', 'netlify-functions', ]) + expect(loggedLines()).toEqual([expect.stringContaining('Installed Netlify skills')]) + }) + + test('setupAgentSkills summarizes a sync and points edited copies at --reset-context', async () => { + await writeSkill(skillsDir, FUNCTIONS.name, FUNCTIONS_V1) + await writeSkill(skillsDir, DEPLOY.name, { 'SKILL.md': '# my own notes\n' }) + await writeSkill(skillsDir, 'netlify-legacy', RETIRED_FILES) + + const result = await setupAgentSkills({ workingDir: projectDir, env: { NETLIFY_SKILLS_HOST: HOST } }) + + expect(result.summary).toMatchObject({ updated: 1, removed: 1, kept: 1 }) + const lines = loggedLines() + expect(lines[0]).toMatch(/Synced Netlify skills in .*\(1 updated, 1 removed, 1 kept\)\./) + expect(lines[1]).toContain('netlify-deploy') + expect(lines[1]).toContain('edited locally') + expect(lines[2]).toContain('--reset-context') + }) + + test('setupAgentSkills reports what finished when a directory stops early', async () => { + await writeSkill(skillsDir, 'netlify-legacy', RETIRED_FILES) + responses.delete('/skills/netlify-functions/SKILL.md') + + const result = await setupAgentSkills({ workingDir: projectDir, env: { NETLIFY_SKILLS_HOST: HOST } }) + + expect(result.installed).toBe(false) + expect(result.error).toMatch(/netlify-functions\/SKILL.md: .*HTTP 404/) + expect(result.summary).toMatchObject({ added: 1, removed: 1 }) + expect(loggedLines()[0]).toMatch(/Synced Netlify skills in .*\(1 added, 1 removed\)\./) }) }) From 69886d06da6cce9a9aa48eef0d65eaf3d4495112 Mon Sep 17 00:00:00 2001 From: Domitrius Clark Date: Fri, 2 Oct 2026 14:51:58 -0400 Subject: [PATCH 2/3] fix(init): guard skill sync against case twins and races The occupant check now scans the directory itself, so a case twin without SKILL.md can no longer be migrated over on reset, and the renamed path never forces. Staging directories younger than ten minutes are left to the run that owns them, and the assembled tree must match the manifest tree_hash before it is swapped in. Co-Authored-By: Claude Fable 5.1 --- src/utils/init/agent-skills.ts | 27 +++++++++---- tests/unit/utils/init/agent-skills.test.ts | 44 +++++++++++++++++++++- 2 files changed, 63 insertions(+), 8 deletions(-) diff --git a/src/utils/init/agent-skills.ts b/src/utils/init/agent-skills.ts index 2dd2203596e..1a051f0edc9 100644 --- a/src/utils/init/agent-skills.ts +++ b/src/utils/init/agent-skills.ts @@ -20,6 +20,7 @@ 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 LEFTOVER_MIN_AGE_MS = 10 * 60_000 const LOOPBACK_HOSTS = new Set(['localhost', '127.0.0.1', '[::1]']) export interface SkillHistoryEntry { @@ -266,6 +267,14 @@ const isDirectoryOrLinkToOne = async (dir: string): Promise => { } } +const isOlderThan = async (file: string, ageMs: number): Promise => { + try { + return Date.now() - (await fs.lstat(file)).mtimeMs >= ageMs + } catch { + return false + } +} + const isSameEntry = async (a: string, b: string): Promise => { try { const [statA, statB] = await Promise.all([fs.lstat(a), fs.lstat(b)]) @@ -307,6 +316,7 @@ export const classifySkillsDirectory = async ( } for (const entry of entries) { + if (entry.name.startsWith(STAGING_PREFIX)) continue const known = exact.get(entry.name) const renamed = prior.get(entry.name) const target = known?.status === 'active' ? known : renamed?.status === 'active' ? renamed : null @@ -441,6 +451,10 @@ export const installSkill = async ( await fs.mkdir(path.dirname(output), { recursive: true }) await fs.writeFile(output, bytes, { mode: executable.has(file) ? 0o755 : 0o644 }) } + 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, target) } catch (error) { await fs.rm(staged, { recursive: true, force: true }) @@ -459,7 +473,7 @@ const removeInstallLeftovers = async (root: string, index: ManifestIndex): Promi const leftover = path.join(root, entry.name) const staging = STAGING_LEFTOVER.exec(entry.name) if (staging) { - if (skillUnderName(index, staging[1])) { + if (skillUnderName(index, staging[1]) && (await isOlderThan(leftover, LEFTOVER_MIN_AGE_MS))) { await fs.rm(leftover, { recursive: true, force: true }) removed.push(entry.name) } @@ -515,12 +529,11 @@ export const syncSkills = async ({ } const before = await classifySkillsDirectory(directory, manifest) - const foreign = before.skills - .filter(({ status }) => status === 'duplicate' || status === 'unknown') - .map(({ name }) => name) const occupantOf = async (name: string): Promise => { - for (const other of foreign) { - if (await isSameEntry(path.join(directory, other), path.join(directory, name))) return other + if (!(await isDirectoryOrLinkToOne(directory))) return undefined + for (const entry of await fs.readdir(directory)) { + if (entry === name) continue + if (await isSameEntry(path.join(directory, entry), path.join(directory, name))) return entry } return undefined } @@ -581,7 +594,7 @@ export const syncSkills = async ({ act(record.name, 'kept', `${record.currentName} is already installed and edited locally`) break } - if (!currentPresent && !(await install(skill, { force: reset }))) { + if (!currentPresent && !(await install(skill))) { act(record.name, 'kept', `now called ${record.currentName}, which could not be installed`) break } diff --git a/tests/unit/utils/init/agent-skills.test.ts b/tests/unit/utils/init/agent-skills.test.ts index e35a76d0499..ef80bd2cba5 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 { tmpdir } from 'node:os' import { join } from 'node:path' @@ -449,6 +449,8 @@ describe('agent skills', () => { await writeSkill(skillsDir, 'netlify-functions.old-123-0123456789ab', FUNCTIONS_V1) await writeSkill(skillsDir, 'netlify-deploy.old-123-0123456789ab', DEPLOY.files, DEPLOY.executable) await writeSkill(skillsDir, '.netlify-skill-someone-else-Ab12Cd', { 'SKILL.md': '# not ours\n' }) + const anHourAgo = new Date(Date.now() - 60 * 60_000) + await utimes(join(skillsDir, '.netlify-skill-netlify-deploy-Ab12Cd'), anHourAgo, anHourAgo) const { actions } = await syncSkills({ host: HOST, directory: skillsDir, manifest }) @@ -472,6 +474,46 @@ describe('agent skills', () => { ]) }) + test('leaves a fresh staging directory alone, since another run may still be writing it', async () => { + await writeSkill(skillsDir, FUNCTIONS.name, FUNCTIONS.files) + await writeSkill(skillsDir, DEPLOY.name, DEPLOY.files, DEPLOY.executable) + await writeSkill(skillsDir, '.netlify-skill-netlify-deploy-Ab12Cd', { 'SKILL.md': '# half written\n' }) + + const { actions } = await syncSkills({ host: HOST, directory: skillsDir, manifest }) + + expect(actions.map(({ action }) => action)).toEqual(['current', 'current']) + await expect(readdir(skillsDir)).resolves.toContain('.netlify-skill-netlify-deploy-Ab12Cd') + }) + + test('refuses to install when the assembled tree does not match the manifest tree_hash', async () => { + const broken = structuredClone(manifest) + const functions = broken.skills.find(({ name }) => name === 'netlify-functions') + if (functions) functions.tree_hash = 'sha256:not-what-the-files-hash-to' + + await expect(installSkill(HOST, skillsDir, functions ?? manifestSkill('netlify-functions'))).rejects.toThrow( + /staged tree hash .* does not match/, + ) + await expect(listDirectories(skillsDir)).resolves.toEqual([]) + }) + + test('--reset-context never migrates over a case twin that has no SKILL.md', async () => { + await writeSkill(skillsDir, 'Netlify-Deploy', { 'notes.md': '# keep me\n' }) + await writeSkill(skillsDir, 'netlify-cli-and-deploy', { 'SKILL.md': '# my deploy notes\n' }) + + const { actions } = await syncSkills({ host: HOST, directory: skillsDir, manifest, reset: true }) + + await expect(readFile(join(skillsDir, 'Netlify-Deploy', 'notes.md'), 'utf8')).resolves.toBe('# keep me\n') + await expect(listDirectories(skillsDir)).resolves.toContain('netlify-cli-and-deploy') + const caseInsensitive = !(await listDirectories(skillsDir)).includes('netlify-deploy') + if (caseInsensitive) { + expect(actions).toContainEqual({ + name: 'netlify-deploy', + action: 'kept', + detail: 'Netlify-Deploy already uses this name; left in place', + }) + } + }) + test('keeps a backup directory the user has edited', async () => { await writeSkill(skillsDir, FUNCTIONS.name, FUNCTIONS.files) await writeSkill(skillsDir, 'netlify-functions.old-123-0123456789ab', { 'SKILL.md': '# my recovery\n' }) From a623e2c3d2754c046397120c5be6d9db8f55aa41 Mon Sep 17 00:00:00 2001 From: Domitrius Clark Date: Fri, 2 Oct 2026 14:56:14 -0400 Subject: [PATCH 3/3] refactor(init): trim skill sync to what the output needs Drops the duplicate classification (an unknown directory is left alone either way), the restore-a-backup branch (a missing skill is reinstalled and the backup then removed as an unedited release), and scopes the --reset-context hint to copies reset would act on. Co-Authored-By: Claude Fable 5.1 --- src/utils/init/agent-skills.ts | 20 ++------- tests/unit/utils/init/agent-skills.test.ts | 49 +++++++++++----------- 2 files changed, 27 insertions(+), 42 deletions(-) diff --git a/src/utils/init/agent-skills.ts b/src/utils/init/agent-skills.ts index 1a051f0edc9..f8823cebc6b 100644 --- a/src/utils/init/agent-skills.ts +++ b/src/utils/init/agent-skills.ts @@ -54,7 +54,6 @@ export type SkillRecord = | { 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 { @@ -342,14 +341,7 @@ export const classifySkillsDirectory = async ( } if (!target) { - const twin = manifest.skills.find( - (skill) => skill.status === 'active' && lastMatching(skill, treeHash) !== undefined, - ) - records.push( - twin - ? { name: entry.name, status: 'duplicate', currentName: twin.name } - : { name: entry.name, status: 'unknown' }, - ) + records.push({ name: entry.name, status: 'unknown' }) continue } @@ -482,10 +474,7 @@ const removeInstallLeftovers = async (root: string, index: ManifestIndex): Promi const retired = RETIRED_LEFTOVER.exec(entry.name) const skill = retired ? skillUnderName(index, retired[1]) : undefined if (!retired || !skill) continue - const original = path.join(root, retired[1]) - if (!(await exists(original))) { - await fs.rename(leftover, original) - } else if (await isUneditedRelease(leftover, skill)) { + if (await isUneditedRelease(leftover, skill)) { await fs.rm(leftover, { recursive: true, force: true }) removed.push(entry.name) } @@ -621,9 +610,6 @@ export const syncSkills = async ({ 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 @@ -707,7 +693,7 @@ const logSync = (directory: string, result: SkillsSyncResult, reset: boolean): v for (const { name, detail } of kept) { log(` ${chalk.dim(name)}: ${detail ?? 'kept'}`) } - if (kept.length > 0 && !reset) { + if (!reset && kept.some(({ detail }) => detail?.includes('edited locally'))) { log( ` Run ${chalk.cyanBright.bold(`${netlifyCommand()} init --reset-context`)} to replace edited Netlify skills with the latest release.`, ) diff --git a/tests/unit/utils/init/agent-skills.test.ts b/tests/unit/utils/init/agent-skills.test.ts index ef80bd2cba5..e6d0cc3fbd9 100644 --- a/tests/unit/utils/init/agent-skills.test.ts +++ b/tests/unit/utils/init/agent-skills.test.ts @@ -430,16 +430,12 @@ describe('agent skills', () => { await expect(listDirectories(skillsDir)).resolves.toEqual(['netlify-deploy', 'netlify-functions']) }) - test('reports a copy of a skill living under an unrelated name and never touches it', async () => { + test('never touches a copy of a skill living under an unrelated name', async () => { await writeSkill(skillsDir, 'deploy-copy', DEPLOY.files, DEPLOY.executable) const { actions } = await syncSkills({ host: HOST, directory: skillsDir, manifest }) - expect(actions).toContainEqual({ - name: 'deploy-copy', - action: 'ignored', - detail: 'copy of netlify-deploy under another name', - }) + expect(actions).toContainEqual({ name: 'deploy-copy', action: 'ignored', detail: 'not a Netlify skill' }) await expect(listDirectories(skillsDir)).resolves.toEqual(['deploy-copy', 'netlify-deploy', 'netlify-functions']) }) @@ -454,19 +450,13 @@ describe('agent skills', () => { const { actions } = await syncSkills({ host: HOST, directory: skillsDir, manifest }) - expect(actions.slice(0, 2)).toEqual([ - { - name: '.netlify-skill-netlify-deploy-Ab12Cd', - action: 'removed', - detail: 'leftover from an interrupted install', - }, - { - name: 'netlify-functions.old-123-0123456789ab', - action: 'removed', - detail: 'leftover from an interrupted install', - }, + const leftover = { action: 'removed', detail: 'leftover from an interrupted install' } + expect(actions.slice(0, 3)).toEqual([ + { name: '.netlify-skill-netlify-deploy-Ab12Cd', ...leftover }, + { name: 'netlify-deploy.old-123-0123456789ab', ...leftover }, + { name: 'netlify-functions.old-123-0123456789ab', ...leftover }, ]) - expect(actions).toContainEqual({ name: 'netlify-deploy', action: 'current', detail: '2.0.0' }) + expect(actions).toContainEqual({ name: 'netlify-deploy', action: 'added', detail: '2.0.0' }) await expect(readdir(skillsDir)).resolves.toEqual([ '.netlify-skill-someone-else-Ab12Cd', 'netlify-deploy', @@ -611,11 +601,7 @@ describe('agent skills', () => { const { actions } = await syncSkills({ host: HOST, directory: skillsDir, manifest }) - expect(actions).toContainEqual({ - name: 'Netlify-Deploy', - action: 'ignored', - detail: 'copy of netlify-deploy under another name', - }) + expect(actions).toContainEqual({ name: 'Netlify-Deploy', action: 'ignored', detail: 'not a Netlify skill' }) const directories = await listDirectories(skillsDir) expect(directories).toContain('Netlify-Deploy') if (!directories.includes('netlify-deploy')) { @@ -672,15 +658,28 @@ describe('agent skills', () => { ]) }) - test('restores a backup left under a prior name and then migrates it', async () => { + test('removes an unedited backup left under a prior name and installs the current name', async () => { await writeSkill(skillsDir, 'netlify-cli-and-deploy.old-123-0123456789ab', DEPLOY.files, DEPLOY.executable) const { actions } = await syncSkills({ host: HOST, directory: skillsDir, manifest }) - expect(actions).toContainEqual({ name: 'netlify-cli-and-deploy', action: 'renamed', detail: '-> netlify-deploy' }) + expect(actions).toContainEqual({ + name: 'netlify-cli-and-deploy.old-123-0123456789ab', + action: 'removed', + detail: 'leftover from an interrupted install', + }) + expect(actions).toContainEqual({ name: 'netlify-deploy', action: 'added', detail: '2.0.0' }) await expect(listDirectories(skillsDir)).resolves.toEqual(['netlify-deploy', 'netlify-functions']) }) + test('does not point at --reset-context when the only kept copies are not edited Netlify skills', async () => { + await writeSkill(skillsDir, 'Netlify-Functions', { 'SKILL.md': '# mine\n', 'notes.md': '# keep me\n' }) + + await setupAgentSkills({ workingDir: projectDir, env: { NETLIFY_SKILLS_HOST: HOST } }) + + expect(loggedLines().join('\n')).not.toContain('--reset-context') + }) + test('--reset-context never forces over a directory that is not classified as a Netlify skill', async () => { await writeSkill(skillsDir, 'Netlify-Functions', { 'SKILL.md': '# mine\n', 'notes.md': '# keep me\n' })