diff --git a/src/add.test.ts b/src/add.test.ts index 714ce74eb..e25166550 100644 --- a/src/add.test.ts +++ b/src/add.test.ts @@ -9,6 +9,7 @@ import { parseAddOptions, getLockSource, getProjectLockSourceUrl, + getProjectLockComputedHashScope, formatEveInstallPromptMessage, } from './add.ts'; @@ -932,6 +933,14 @@ metadata: }); }); +describe('getProjectLockComputedHashScope', () => { + it('marks only root-level blob installs as skill-file hashes', () => { + expect(getProjectLockComputedHashScope(true, 'SKILL.md')).toBe('skill-file'); + expect(getProjectLockComputedHashScope(true, 'skills/example/SKILL.md')).toBeUndefined(); + expect(getProjectLockComputedHashScope(false, 'SKILL.md')).toBeUndefined(); + }); +}); + describe('getLockSource', () => { it('preserves git@ SSH URLs for lock files', () => { expect(getLockSource('git@github.com:owner/repo.git', 'owner/repo')).toBe( diff --git a/src/add.ts b/src/add.ts index e80b4cc9a..875f3d963 100644 --- a/src/add.ts +++ b/src/add.ts @@ -110,6 +110,14 @@ export function getLockSource(parsedUrl: string, normalizedSource: string | null export function getProjectLockSourceUrl(sourceType: string, sourceUrl: string): string | undefined { return sourceType === 'git' || sourceType === 'gitlab' ? sourceUrl : undefined; } + +export function getProjectLockComputedHashScope( + isBlobInstall: boolean, + skillPath?: string +): 'skill-file' | undefined { + return isBlobInstall && skillPath && !skillPath.includes('/') ? 'skill-file' : undefined; +} + export function initTelemetry(version: string): void { setVersion(version); } @@ -1305,8 +1313,10 @@ function getSkillRepoPaths(resolved: ResolvedSkills, skills: Skill[]): Record const files: Array<{ relativePath: string; content: Buffer }> = []; await collectFiles(skillDir, skillDir, files); + return computeFilesHash(files); +} + +/** Compute the same deterministic hash for one skill entry file. */ +export async function computeSkillFileHash( + skillDir: string, + skillFileName = 'SKILL.md' +): Promise { + const content = await readFile(join(skillDir, skillFileName)); + return computeFilesHash([{ relativePath: skillFileName, content }]); +} + +function computeFilesHash(files: Array<{ relativePath: string; content: Buffer }>): string { // Sort by relative path for deterministic hashing files.sort((a, b) => a.relativePath.localeCompare(b.relativePath)); diff --git a/src/update.ts b/src/update.ts index b721dc358..48a5c433a 100644 --- a/src/update.ts +++ b/src/update.ts @@ -1,12 +1,17 @@ import { spawnSync } from 'child_process'; import { existsSync, readdirSync } from 'fs'; -import { join, dirname, relative, sep } from 'path'; +import { basename, join, dirname, relative, sep } from 'path'; import { fileURLToPath } from 'url'; import * as p from '@clack/prompts'; import pc from 'picocolors'; import { readSkillLock, getGitHubToken, type SkillLockEntry } from './skill-lock.ts'; -import { computeSkillFolderHash, readLocalLock, type LocalSkillLockEntry } from './local-lock.ts'; +import { + computeSkillFileHash, + computeSkillFolderHash, + readLocalLock, + type LocalSkillLockEntry, +} from './local-lock.ts'; import { formatSourceInput, buildUpdateInstallSource, @@ -893,7 +898,11 @@ export async function updateProjectSkills( // computedHash, so unchanged skills are not reinstalled. for (const [name, skillPath] of resolution.resolvedPaths) { try { - const hash = await computeSkillFolderHash(join(tempDir, dirname(skillPath))); + const skillDir = join(tempDir, dirname(skillPath)); + const hash = + localLock.skills[name]?.computedHashScope === 'skill-file' + ? await computeSkillFileHash(skillDir, basename(skillPath)) + : await computeSkillFolderHash(skillDir); if (hash) latestHashes.set(name, hash); } catch { // Unknown hash: fall through to reinstalling the skill. diff --git a/tests/blob-project-lock-hash.test.ts b/tests/blob-project-lock-hash.test.ts new file mode 100644 index 000000000..869e67292 --- /dev/null +++ b/tests/blob-project-lock-hash.test.ts @@ -0,0 +1,166 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { mkdir, mkdtemp, readFile, rm, writeFile } from 'node:fs/promises'; +import { dirname, join } from 'node:path'; +import { tmpdir } from 'node:os'; +import { spawnSync } from 'node:child_process'; + +vi.mock('@clack/prompts', () => { + const noop = () => {}; + return { + intro: noop, + outro: noop, + note: noop, + cancel: noop, + log: { info: noop, message: noop, warn: noop, error: noop, step: noop, success: noop }, + spinner: () => ({ start: noop, stop: noop, message: noop }), + }; +}); + +vi.mock('../src/telemetry.ts', () => ({ + track: vi.fn(), + setVersion: vi.fn(), + fetchAuditData: vi.fn().mockResolvedValue(null), +})); + +vi.mock('../src/detect-agent.ts', () => ({ + detectAgent: vi.fn().mockResolvedValue({ isAgent: false, agent: { name: 'none' } }), + getAgentType: vi.fn(), + ensureUniversalAgents: vi.fn((agents: string[]) => agents), +})); + +vi.mock('../src/source-parser.ts', async (importActual) => ({ + ...(await importActual()), + isRepoPrivate: vi.fn().mockResolvedValue(false), +})); + +vi.mock('../src/git.ts', async (importActual) => ({ + ...(await importActual()), + cloneRepo: vi.fn(), + cleanupTempDir: vi.fn().mockResolvedValue(undefined), +})); + +vi.mock('node:child_process', async (importActual) => ({ + ...(await importActual()), + spawnSync: vi.fn().mockReturnValue({ status: 0 }), +})); + +import { installFromSource, runAdd } from '../src/add.ts'; +import { resetRepoTreeAuthState } from '../src/blob.ts'; +import { cloneRepo } from '../src/git.ts'; +import { computeSkillFileHash, computeSkillFolderHash, readLocalLock } from '../src/local-lock.ts'; +import { updateProjectSkills } from '../src/update.ts'; + +const SKILL_MD = '---\nname: example\ndescription: Example skill\n---\n# Example\n'; + +describe('blob project lock hashes', () => { + let base: string; + let repo: string; + let originalCwd: string; + + beforeEach(async () => { + vi.clearAllMocks(); + resetRepoTreeAuthState(); + base = await mkdtemp(join(tmpdir(), 'blob-project-lock-')); + repo = join(base, 'repo'); + const project = join(base, 'project'); + await mkdir(project); + originalCwd = process.cwd(); + process.chdir(project); + vi.stubEnv('DISABLE_TELEMETRY', '1'); + vi.stubEnv('XDG_STATE_HOME', join(base, 'state')); + vi.spyOn(process, 'exit').mockImplementation((() => { + throw new Error('process.exit called'); + }) as never); + vi.mocked(cloneRepo).mockResolvedValue(repo); + }); + + afterEach(async () => { + process.chdir(originalCwd); + vi.restoreAllMocks(); + vi.unstubAllGlobals(); + vi.unstubAllEnvs(); + await rm(base, { recursive: true, force: true }); + }); + + it.each([ + { command: 'add', skillPath: 'SKILL.md' }, + { command: 'restore', skillPath: 'SKILL.md' }, + { command: 'add', skillPath: 'skills/example/SKILL.md' }, + { command: 'restore', skillPath: 'skills/example/SKILL.md' }, + ])( + 'keeps $command blob installs at $skillPath unchanged on update', + async ({ command, skillPath }) => { + const rootSkill = skillPath === 'SKILL.md'; + const supportingPath = rootSkill ? 'docs/guide.md' : 'skills/example/references/guide.md'; + const files = [ + { path: 'SKILL.md', contents: SKILL_MD }, + { path: rootSkill ? supportingPath : 'references/guide.md', contents: '# Guide\n' }, + ]; + for (const [path, contents] of [ + [skillPath, SKILL_MD], + [supportingPath, '# Guide\n'], + ]) { + await mkdir(dirname(join(repo, path!)), { recursive: true }); + await writeFile(join(repo, path!), contents!, 'utf-8'); + } + + vi.stubGlobal( + 'fetch', + vi.fn(async (input: string | URL) => { + const url = String(input); + if (url.includes('/git/trees/')) { + return Response.json({ + sha: 'tree-sha', + tree: [skillPath, supportingPath].map((path) => ({ path, type: 'blob', sha: path })), + }); + } + if (url.startsWith('https://raw.githubusercontent.com/')) return new Response(SKILL_MD); + if (url.includes('/api/download/')) return Response.json({ hash: 'server-hash', files }); + throw new Error(`Unexpected request: ${url}`); + }) + ); + + if (command === 'add') { + await runAdd(['vercel-labs/hash-fixture'], { yes: true, agent: ['codex'], global: false }); + } else { + const result = await installFromSource('vercel-labs/hash-fixture', { + skills: ['example'], + agents: ['codex'], + }); + expect(result).toEqual({ installed: ['example'], failed: [] }); + } + expect(cloneRepo).not.toHaveBeenCalled(); + + const entry = (await readLocalLock()).skills.example!; + expect(entry.skillPath).toBe(skillPath); + expect(entry.computedHashScope).toBe(rootSkill ? 'skill-file' : undefined); + expect(entry.computedHash).toBe( + rootSkill + ? await computeSkillFileHash(repo) + : await computeSkillFolderHash(join(repo, 'skills/example')) + ); + await expect( + readFile(join(process.cwd(), '.agents/skills/example/SKILL.md'), 'utf-8') + ).resolves.toBe(SKILL_MD); + + expect(await updateProjectSkills({ yes: true })).toEqual({ + successCount: 0, + failCount: 0, + foundCount: 1, + }); + expect(spawnSync).not.toHaveBeenCalled(); + + const changedPath = rootSkill ? skillPath : supportingPath; + await writeFile( + join(repo, changedPath), + rootSkill ? SKILL_MD + '\nChanged\n' : '# Changed\n' + ); + expect(await updateProjectSkills({ yes: true })).toEqual({ + successCount: 1, + failCount: 0, + foundCount: 1, + }); + expect(spawnSync).toHaveBeenCalledTimes(1); + } + ); +}); diff --git a/tests/blob-root-skill.test.ts b/tests/blob-root-skill.test.ts index 28afba431..ad5e8b9fe 100644 --- a/tests/blob-root-skill.test.ts +++ b/tests/blob-root-skill.test.ts @@ -1,5 +1,9 @@ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises'; +import { dirname, join } from 'node:path'; +import { tmpdir } from 'node:os'; import { resetRepoTreeAuthState, tryBlobInstall } from '../src/blob.ts'; +import { computeSkillFolderHash } from '../src/local-lock.ts'; const ROOT_SKILL_MD = `--- name: eve @@ -112,7 +116,18 @@ description: Nested skill. expect(result).not.toBeNull(); expect(result!.skills[0]!.files.map((file) => file.path)).toEqual(['SKILL.md', 'reference.md']); - expect(result!.skills[0]!.snapshotHash).toBe('nested-snapshot-hash'); + expect(result!.skills[0]!.snapshotHash).not.toBe('nested-snapshot-hash'); + + const snapshotDir = await mkdtemp(join(tmpdir(), 'blob-hash-')); + try { + for (const file of result!.skills[0]!.files) { + await mkdir(dirname(join(snapshotDir, file.path)), { recursive: true }); + await writeFile(join(snapshotDir, file.path), file.contents, 'utf-8'); + } + expect(result!.skills[0]!.snapshotHash).toBe(await computeSkillFolderHash(snapshotDir)); + } finally { + await rm(snapshotDir, { recursive: true, force: true }); + } }); it('falls back when a nested snapshot omits an installable repository file', async () => { diff --git a/tests/local-lock.test.ts b/tests/local-lock.test.ts index 6fd682d6b..90afa6eb6 100644 --- a/tests/local-lock.test.ts +++ b/tests/local-lock.test.ts @@ -7,6 +7,7 @@ import { writeLocalLock, addSkillToLocalLock, removeSkillFromLocalLock, + computeSkillFileHash, computeSkillFolderHash, getLocalLockPath, } from '../src/local-lock.ts'; @@ -467,6 +468,23 @@ describe('local-lock', () => { }); }); + describe('computeSkillFileHash', () => { + it('ignores supporting files while retaining the folder-hash format', async () => { + const dir = await mkdtemp(join(tmpdir(), 'lock-test-')); + try { + await writeFile(join(dir, 'SKILL.md'), 'skill contents', 'utf-8'); + const skillFileHash = await computeSkillFileHash(dir); + expect(skillFileHash).toBe(await computeSkillFolderHash(dir)); + + await writeFile(join(dir, 'supporting.md'), 'supporting contents', 'utf-8'); + expect(await computeSkillFileHash(dir)).toBe(skillFileHash); + expect(await computeSkillFolderHash(dir)).not.toBe(skillFileHash); + } finally { + await rm(dir, { recursive: true, force: true }); + } + }); + }); + describe('merge conflict friendliness', () => { it('produces no-conflict output when two skills are added independently', async () => { const dir = await mkdtemp(join(tmpdir(), 'lock-test-')); diff --git a/tests/update.test.ts b/tests/update.test.ts index fdb9c7926..59c2a9dfd 100644 --- a/tests/update.test.ts +++ b/tests/update.test.ts @@ -506,6 +506,33 @@ describe('Update Cleanup Unit Tests', () => { expect(argv).not.toContain('--full-depth'); }); + it('compares root blob installs using only the installed skill file', async () => { + vi.mocked(localLock.readLocalLock).mockResolvedValue({ + version: 1, + skills: { + 'root-skill': { + source: 'owner/repo', + sourceType: 'github', + skillPath: 'SKILL.md', + computedHash: 'same-hash', + computedHashScope: 'skill-file', + }, + }, + }); + vi.mocked(git.cloneRepo).mockResolvedValue('/tmp/repo'); + vi.mocked(skills.discoverSkills).mockResolvedValue([ + { name: 'root-skill', path: '/tmp/repo', description: 'Root', rawContent: '' }, + ]); + vi.mocked(localLock.computeSkillFileHash).mockResolvedValue('same-hash'); + + const result = await updateProjectSkills({ yes: true }); + + expect(localLock.computeSkillFileHash).toHaveBeenCalledWith(join('/tmp/repo'), 'SKILL.md'); + expect(localLock.computeSkillFolderHash).not.toHaveBeenCalled(); + expect(spawnSync).not.toHaveBeenCalled(); + expect(result).toEqual({ successCount: 0, failCount: 0, foundCount: 1 }); + }); + it('pins public GitHub project updates to github.com when GH_HOST points elsewhere', async () => { vi.stubEnv('GH_HOST', 'github.example.com'); vi.mocked(localLock.readLocalLock).mockResolvedValue({