diff --git a/src/utils/init/agent-skills.ts b/src/utils/init/agent-skills.ts index 4c28a1a72af..4b8a218a445 100644 --- a/src/utils/init/agent-skills.ts +++ b/src/utils/init/agent-skills.ts @@ -613,10 +613,21 @@ export const syncSkills = async ({ break } const priorDir = path.join(directory, record.name) - if (!currentPresent && (await isSameEntry(priorDir, path.join(directory, record.currentName)))) { - await fs.rename(priorDir, path.join(directory, record.currentName)) - installed.add(record.currentName) + const currentDir = path.join(directory, record.currentName) + if (!currentPresent && (await isSameEntry(priorDir, currentDir))) { + await fs.rename(priorDir, currentDir) act(record.name, 'renamed', `-> ${record.currentName}`) + if ( + (await hashIfPossible(currentDir, skill)) !== skill.tree_hash && + (await install(skill, { force: reset })) + ) { + if (record.modified) { + act(record.currentName, 'reset', `edited copy replaced with ${skill.version ?? 'latest'}`) + } else { + act(record.currentName, 'updated', `-> ${skill.version ?? 'latest'}`) + } + } + installed.add(record.currentName) break } if (!currentPresent && !(await install(skill))) { @@ -715,7 +726,12 @@ const describeSync = ({ directory, actions }: SkillsSyncResult): string => { if (changed + summary.failed === 0) { return `Netlify skills in ${location} are up to date${parts.length > 0 ? ` (${parts.join(', ')})` : ''}.` } - const verb = changed === summary.added ? 'Installed' : 'Synced' + if (changed === 0) { + return summary.current + summary.kept > 0 + ? `Some Netlify skills in ${location} could not be synced (${parts.join(', ')}).` + : `Could not sync Netlify skills in ${location} (${parts.join(', ')}).` + } + const verb = changed === summary.added && summary.failed === 0 ? 'Installed' : 'Synced' return `${verb} Netlify skills in ${location} (${parts.join(', ')}).` } diff --git a/tests/unit/utils/init/agent-skills.test.ts b/tests/unit/utils/init/agent-skills.test.ts index 8e9edc42ed2..745912519ea 100644 --- a/tests/unit/utils/init/agent-skills.test.ts +++ b/tests/unit/utils/init/agent-skills.test.ts @@ -522,6 +522,64 @@ describe('agent skills', () => { expect(second.actions.map(({ action }) => action)).toEqual(['current', 'current']) }) + test('refreshes a stale copy in the same run when its old name differs only by case', async () => { + const deployV1: Files = { 'SKILL.md': '# deploy v1\n' } + const caseOnly = buildManifest([ + FUNCTIONS, + { ...DEPLOY, priorNames: ['Netlify-Deploy'], previous: [{ version: '1.0.0', files: deployV1 }] }, + ]) + await writeSkill(skillsDir, 'Netlify-Deploy', deployV1) + await writeSkill(skillsDir, FUNCTIONS.name, FUNCTIONS.files) + + const { actions } = await syncSkills({ host: HOST, directory: skillsDir, manifest: caseOnly }) + + expect(actions).toContainEqual({ name: 'Netlify-Deploy', action: 'renamed', detail: '-> netlify-deploy' }) + await expect(listDirectories(skillsDir)).resolves.toEqual(['netlify-deploy', 'netlify-functions']) + await expect(readFile(join(skillsDir, 'netlify-deploy', 'SKILL.md'), 'utf8')).resolves.toBe('# deploy\n') + + const second = await syncSkills({ host: HOST, directory: skillsDir, manifest: caseOnly }) + expect(second.actions.map(({ action }) => action)).toEqual(['current', 'current']) + }) + + test('keeps the old copy when the refresh after a case-only rename fails, and finishes on the next run', async () => { + const deployV1: Files = { 'SKILL.md': '# deploy v1\n' } + const caseOnly = buildManifest([ + FUNCTIONS, + { ...DEPLOY, priorNames: ['Netlify-Deploy'], previous: [{ version: '1.0.0', files: deployV1 }] }, + ]) + await writeSkill(skillsDir, 'Netlify-Deploy', deployV1) + await writeSkill(skillsDir, FUNCTIONS.name, FUNCTIONS.files) + const deploySkillMd = responses.get('/skills/netlify-deploy/SKILL.md') + responses.delete('/skills/netlify-deploy/SKILL.md') + + const { actions } = await syncSkills({ host: HOST, directory: skillsDir, manifest: caseOnly }) + + const deployFailures = actions.filter(({ name, action }) => name === 'netlify-deploy' && action === 'failed') + expect(deployFailures).toHaveLength(1) + expect(deployFailures[0].detail).toMatch(/SKILL\.md: .*HTTP 404/) + const [oldCopy] = (await listDirectories(skillsDir)).filter((name) => name.toLowerCase() === 'netlify-deploy') + await expect(readFile(join(skillsDir, oldCopy, 'SKILL.md'), 'utf8')).resolves.toBe('# deploy v1\n') + await expect(readdir(skillsDir)).resolves.not.toContainEqual(expect.stringMatching(/^\.netlify-skill-/)) + + if (deploySkillMd === undefined) throw new Error('netlify-deploy SKILL.md missing from the hosted fixture') + responses.set('/skills/netlify-deploy/SKILL.md', deploySkillMd) + await syncSkills({ host: HOST, directory: skillsDir, manifest: caseOnly }) + + 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('--reset-context replaces an edited copy in the same run when its old name differs only by case', async () => { + const caseOnly = buildManifest([FUNCTIONS, { ...DEPLOY, priorNames: ['Netlify-Deploy'] }]) + await writeSkill(skillsDir, 'Netlify-Deploy', { 'SKILL.md': '# my deploy notes\n' }) + await writeSkill(skillsDir, FUNCTIONS.name, FUNCTIONS.files) + + await syncSkills({ host: HOST, directory: skillsDir, manifest: caseOnly, reset: true }) + + 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('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) @@ -899,6 +957,29 @@ describe('agent skills', () => { expect(lines[2]).toContain('--reset-context') }) + test('setupAgentSkills does not say installed when every download failed', async () => { + responses.delete('/skills/netlify-functions/SKILL.md') + responses.delete('/skills/netlify-deploy/SKILL.md') + + const result = await setupAgentSkills({ workingDir: projectDir, env: { NETLIFY_SKILLS_HOST: HOST } }) + + expect(result.installed).toBe(false) + expect(result.summary).toMatchObject({ added: 0, failed: 2 }) + expect(loggedLines()[0]).toMatch(/Could not sync Netlify skills in .*\(2 failed\)\./) + }) + + test('setupAgentSkills says some skills could not be synced when the rest are already current', async () => { + await writeSkill(skillsDir, DEPLOY.name, DEPLOY.files, DEPLOY.executable) + await writeSkill(skillsDir, FUNCTIONS.name, FUNCTIONS_V1) + responses.delete('/skills/netlify-functions/SKILL.md') + + const result = await setupAgentSkills({ workingDir: projectDir, env: { NETLIFY_SKILLS_HOST: HOST } }) + + expect(result.installed).toBe(true) + expect(result.summary).toMatchObject({ current: 1, failed: 1 }) + expect(loggedLines()[0]).toMatch(/Some Netlify skills in .* could not be synced \(1 failed\)\./) + }) + test('setupAgentSkills reports a failed skill next to what landed', async () => { await writeSkill(skillsDir, 'netlify-legacy', RETIRED_FILES) responses.delete('/skills/netlify-functions/SKILL.md')