From e8cce1545b4bbeeab9cdfd7e608a736b6c6d6062 Mon Sep 17 00:00:00 2001 From: "Anthony Fu (via agent)" Date: Wed, 7 Oct 2026 06:53:14 +0000 Subject: [PATCH] feat(sync): install git sources from package.json skills fields Remote entries in a skills field (owner/repo, owner/repo@skill, installed by experimental_sync. Only git-hosted sources are allowed; local paths and plain URLs are errors, as in the skills-npm SPEC. Identical requests from several packages are installed once. Sync installs them in-process with installFromSource, which now takes a parsed source, records via in the lock, and lets the caller drop skills before installing. A skill shipped by a dependency or an earlier source wins, a skill installed with skills add is never shadowed, and a directory sync does not own is never replaced. A request whose skills are already in the lock and on disk is not fetched again; skills update refreshes them. Remote skills that no field requests anymore are removed. --no-remote skips remote entries for both installing and removing. A failing source is reported, the rest of the run continues, and the exit code is 1. --- src/add.ts | 27 +++-- src/cli.ts | 1 + src/install.ts | 6 +- src/skills-field.ts | 32 +++++- src/sync.ts | 199 +++++++++++++++++++++++++++++-------- tests/install.test.ts | 2 +- tests/skills-field.test.ts | 49 ++++++++- tests/sync.test.ts | 187 ++++++++++++++++++++++++++++++++++ 8 files changed, 445 insertions(+), 58 deletions(-) diff --git a/src/add.ts b/src/add.ts index 0b17a09b6..b6ead664c 100644 --- a/src/add.ts +++ b/src/add.ts @@ -1302,6 +1302,11 @@ function getSkillRepoPaths(resolved: ResolvedSkills, skills: Skill[]): Record Promise; + } ): Promise { - const parsed = parseSource(source); const spinner = p.spinner(); let resolved: ResolvedSkills | null = null; try { resolved = await resolveSkills(parsed, { includeInternal: options.skills.length > 0 }, spinner); - const selected = + let selected = options.skills.length > 0 ? filterSkills(resolved.skills, options.skills) : resolved.skills; if (selected.length === 0) { spinner.stop(pc.red('No matching skills found')); return { installed: [], failed: [], error: 'No matching skills found' }; } spinner.stop(`Found ${pc.green(selected.length)} skill${selected.length > 1 ? 's' : ''}`); + if (options.select) selected = await options.select(selected); const results = await installToTargets( resolved, @@ -1356,7 +1367,7 @@ export async function installFromSource( for (const skill of selected) { if (!installed.has(getSkillDisplayName(skill))) continue; const entry = projectLockEntry(parsed, repoPaths[skill.name], await sourceSkillHash(skill)); - await addSkillToLocalLock(skill.name, entry); + await addSkillToLocalLock(skill.name, { ...entry, ...(options.via && { via: options.via }) }); } return { installed: [...installed], diff --git a/src/cli.ts b/src/cli.ts index eda04a504..9b73c97e5 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -165,6 +165,7 @@ ${BOLD}Experimental Sync Options:${RESET} --copy Copy skills instead of linking them to node_modules --dry-run Show what would change without changing anything --no-cleanup Keep skills whose package no longer ships them + --no-remote Skip git sources listed in package.json skills fields --include Only sync matching packages () or skills (#) --exclude Skip matching packages () or skills (#) diff --git a/src/install.ts b/src/install.ts index 41b046e09..cdfec95f7 100644 --- a/src/install.ts +++ b/src/install.ts @@ -5,6 +5,7 @@ import { installFromSource, runAdd } from './add.ts'; import { runSync, parseSyncOptions } from './sync.ts'; import { getUniversalAgents } from './agents.ts'; import { buildLocalUpdateSource } from './update-source.ts'; +import { parseSource } from './source-parser.ts'; /** * Install all skills from the local skills-lock.json. @@ -74,7 +75,10 @@ export async function runInstallFromLock(args: string[]): Promise { await runAdd([source], { skill: skills, agent: universalAgentNames, yes: true }); continue; } - const result = await installFromSource(source, { skills, agents: universalAgentNames }); + const result = await installFromSource(parseSource(source), { + skills, + agents: universalAgentNames, + }); if (result.error) { p.log.error(`Failed to install from ${pc.cyan(source)}: ${result.error}`); process.exitCode = 1; diff --git a/src/skills-field.ts b/src/skills-field.ts index 3aeebf74d..aec509c1b 100644 --- a/src/skills-field.ts +++ b/src/skills-field.ts @@ -1,3 +1,6 @@ +import { parseSource } from './source-parser.ts'; +import type { ParsedSource } from './types.ts'; + /** * The `skills` field of package.json: skills a package wants installed * without shipping their files. Grammar: https://github.com/antfu/skills-npm/blob/main/SPEC.md @@ -11,12 +14,22 @@ interface NpmSkillsRequest { skills: string[]; } +/** A git source to install skills from. */ +export interface RemoteSkillsRequest { + parsed: ParsedSource; + /** Skill names to keep; empty means all. */ + skills: string[]; +} + interface ParsedSkillsField { npm: NpmSkillsRequest[]; + remote: RemoteSkillsRequest[]; errors: string[]; } const NPM_PREFIX = 'npm:'; +// the SPEC allows git-hosted sources only, not local paths or plain URLs +const REMOTE_SOURCE_TYPES = new Set(['github', 'gitlab', 'git']); function isSkillsFieldEntry(value: unknown): value is SkillsFieldEntry { if (typeof value === 'string') return true; @@ -33,7 +46,7 @@ function isSkillsFieldEntry(value: unknown): value is SkillsFieldEntry { /** Parse the entries of `declarer`'s `skills` field. Problems are returned, not thrown. */ export function parseSkillsField(entries: unknown[], declarer: string): ParsedSkillsField { - const parsed: ParsedSkillsField = { npm: [], errors: [] }; + const parsed: ParsedSkillsField = { npm: [], remote: [], errors: [] }; for (const raw of entries) { if (!isSkillsFieldEntry(raw)) { @@ -41,10 +54,19 @@ export function parseSkillsField(entries: unknown[], declarer: string): ParsedSk continue; } const { source, skills = [], ref } = typeof raw === 'string' ? { source: raw } : raw; - // remote (git) entries are not synced yet - if (!source.startsWith(NPM_PREFIX)) continue; - - if (ref !== undefined) { + if (!source.startsWith(NPM_PREFIX)) { + const remote = parseSource(source); + if (!REMOTE_SOURCE_TYPES.has(remote.type)) { + parsed.errors.push(`${declarer}: "${source}" is not a git source`); + } else if (ref !== undefined && remote.ref !== undefined) { + parsed.errors.push(`${declarer}: "${source}" already has a ref; remove "ref"`); + } else { + parsed.remote.push({ + parsed: { ...remote, ref: ref ?? remote.ref }, + skills: remote.skillFilter ? [...skills, remote.skillFilter] : skills, + }); + } + } else if (ref !== undefined) { parsed.errors.push(`${declarer}: "ref" cannot be used with "${source}"`); } else { parsed.npm.push({ package: source.slice(NPM_PREFIX.length), skills }); diff --git a/src/sync.ts b/src/sync.ts index da4a55b5f..ab7dbfa81 100644 --- a/src/sync.ts +++ b/src/sync.ts @@ -33,7 +33,8 @@ import type { Skill, AgentType } from './types.ts'; import { track } from './telemetry.ts'; import { detectAgent, getAgentType } from './detect-agent.ts'; import { getLastSelectedAgents, saveSelectedAgents } from './skill-lock.ts'; -import { parseSkillsField } from './skills-field.ts'; +import { parseSkillsField, type RemoteSkillsRequest } from './skills-field.ts'; +import { getProjectLockSource, installFromSource } from './add.ts'; const isCancelled = (value: unknown): value is symbol => typeof value === 'symbol'; @@ -45,6 +46,7 @@ export interface SyncOptions { cleanup?: boolean; include?: string[]; exclude?: string[]; + remote?: boolean; } /** @@ -162,13 +164,20 @@ function findInstalledPackage(from: string, name: string): string | undefined { * reaches the agent only when a direct dependency names it, and the package * manager's version resolution decides which copy of a package is seen. */ -async function discoverNodeModuleSkills( - cwd: string -): Promise<{ skills: PackageSkill[]; warnings: string[]; errors: string[] }> { +type FieldRemoteRequest = RemoteSkillsRequest & { via: string }; + +async function discoverNodeModuleSkills(cwd: string): Promise<{ + skills: PackageSkill[]; + remote: FieldRemoteRequest[]; + warnings: string[]; + errors: string[]; +}> { const warnings: string[] = []; const errors: string[] = []; + // identical requests from several packages are one + const remote = new Map(); const pkg = await readPackageJson(cwd); - if (!pkg) return { skills: [], warnings, errors }; + if (!pkg) return { skills: [], remote: [], warnings, errors }; const skills: PackageSkill[] = []; const seen = new Set(); @@ -212,6 +221,11 @@ async function discoverNodeModuleSkills( const parsed = parseSkillsField(field, declarer.name); problems.push(...parsed.errors); + for (const request of parsed.remote) { + const { url, ref, subpath } = request.parsed; + const key = JSON.stringify([url, ref, subpath, request.skills]); + if (!remote.has(key)) remote.set(key, { ...request, via: declarer.name }); + } for (const request of parsed.npm) { const target = findInstalledPackage(dir, request.package); if (!target) { @@ -235,17 +249,22 @@ async function discoverNodeModuleSkills( } } - return { skills, warnings, errors }; + return { skills, remote: [...remote.values()], warnings, errors }; } function isUnderNodeModules(path: string): boolean { return path.split(sep).includes('node_modules'); } +/** Sync installed this skill: shipped by a package, or requested by a `skills` field. */ +function installedBySync(entry: LocalSkillLockEntry | undefined): boolean { + return entry?.sourceType === 'node_modules' || entry?.via !== undefined; +} + /** * Why `dest` must not be touched, or null when it is free or already ours. * Sync owns a symlink whose target is in node_modules or is the canonical dir, - * and a real directory that the lock attributes to node_modules. + * and a real directory whose lock entry sync wrote. */ async function foreignDestination( dest: string, @@ -264,7 +283,7 @@ async function foreignDestination( if (isUnderNodeModules(target) || target === canonicalDir) return null; return `${shortenPath(dest, cwd)} is a symlink to ${shortenPath(target, cwd)}`; } - if (lockEntry?.sourceType === 'node_modules') return null; + if (installedBySync(lockEntry)) return null; return `${shortenPath(dest, cwd)} already exists and was not installed by sync`; } @@ -278,13 +297,15 @@ async function linksIntoNodeModules(path: string): Promise { } /** - * Remove skills that sync installed earlier and no dependency ships anymore. - * A name is a candidate when its lock entry comes from node_modules or its - * canonical dir links into node_modules. Only destinations sync owns are removed. + * Remove skills that sync installed earlier and nothing provides anymore. + * A name is a candidate when its lock entry comes from node_modules, or from a + * remote `skills` entry that `keepRemote` rejects, or when its canonical dir + * links into node_modules. Only destinations sync owns are removed. */ async function pruneStaleSkills( cwd: string, keep: Set, + keepRemote: (entry: LocalSkillLockEntry) => boolean, lock: LocalSkillLockFile, dryRun: boolean ): Promise { @@ -292,8 +313,12 @@ async function pruneStaleSkills( const lockKeys = new Map(); const explicit = new Set(); for (const [key, entry] of Object.entries(lock.skills)) { - if (entry.sourceType === 'node_modules') lockKeys.set(sanitizeName(key), key); - else explicit.add(sanitizeName(key)); + const remote = entry.sourceType !== 'node_modules'; + if (installedBySync(entry) && !(remote && keepRemote(entry))) { + lockKeys.set(sanitizeName(key), key); + } else { + explicit.add(sanitizeName(key)); + } } const candidates = new Set(lockKeys.keys()); @@ -323,17 +348,44 @@ async function pruneStaleSkills( return stale; } +/** + * Why sync must not install `name`, or null: a skill installed with + * `skills add` is never shadowed, and a destination sync does not own is + * never replaced. + */ +async function blockedReason( + name: string, + targetAgents: AgentType[], + lockEntry: LocalSkillLockEntry | undefined, + cwd: string +): Promise { + if (lockEntry && !installedBySync(lockEntry)) { + return `installed with \`skills add\` from ${lockEntry.source}`; + } + const canonicalDir = getCanonicalPath(name, { cwd }); + const destinations = new Set([ + canonicalDir, + ...targetAgents.map((agent) => getInstallPath(name, agent, { cwd })), + ]); + for (const dest of destinations) { + const reason = await foreignDestination(dest, canonicalDir, lockEntry, cwd); + if (reason) return reason; + } + return null; +} + interface SkippedSkill { skill: PackageSkill; reason: string; } /** - * Conflict rules, in order: - * 1. a skill installed with `skills add` is never shadowed - * 2. a directory or symlink sync does not own is never replaced - * 3. a skill from a direct dependency wins over one from a transitive package - * 4. two packages at the same depth shipping the same skill name install neither + * Conflict rules for shipped skills, in order: + * 1. a skill from a direct dependency wins over one from a transitive package + * 2. two packages at the same depth shipping the same skill name install neither + * 3. blockedReason: never shadow `skills add`, never replace what sync does not own + * + * A shipped skill replaces one that a remote `skills` entry installed. */ async function resolveConflicts( skills: PackageSkill[], @@ -367,22 +419,7 @@ async function resolveConflicts( } const skill = candidates[0]!; - const lockEntry = lockBySanitizedName.get(name); - if (lockEntry && lockEntry.sourceType !== 'node_modules') { - skipped.push({ skill, reason: `installed with \`skills add\` from ${lockEntry.source}` }); - continue; - } - - const canonicalDir = getCanonicalPath(name, { cwd }); - const destinations = new Set([ - canonicalDir, - ...targetAgents.map((agent) => getInstallPath(name, agent, { cwd })), - ]); - let reason: string | null = null; - for (const dest of destinations) { - reason = await foreignDestination(dest, canonicalDir, lockEntry, cwd); - if (reason) break; - } + const reason = await blockedReason(name, targetAgents, lockBySanitizedName.get(name), cwd); if (reason) { skipped.push({ skill, reason }); } else { @@ -426,6 +463,32 @@ async function promptForAgentChoice( return selected as AgentType[] | symbol; } +/** `entry` was installed by sync for the remote `skills` entry `request`. */ +function isFromRequest(entry: LocalSkillLockEntry, request: FieldRemoteRequest): boolean { + return ( + entry.via !== undefined && + entry.source === getProjectLockSource(request.parsed) && + entry.ref === request.parsed.ref + ); +} + +/** Every skill `request` asks for is in the lock and on disk; `skills update` refreshes them. */ +function isRemoteInstalled( + request: FieldRemoteRequest, + lock: LocalSkillLockFile, + cwd: string +): boolean { + const installed = Object.entries(lock.skills) + .filter( + ([name, entry]) => + isFromRequest(entry, request) && existsSync(getCanonicalPath(name, { cwd })) + ) + .map(([name]) => sanitizeName(name)); + return request.skills.length === 0 + ? installed.length > 0 + : request.skills.every((name) => installed.includes(sanitizeName(name))); +} + export async function runSync(args: string[], options: SyncOptions = {}): Promise { const cwd = process.cwd(); @@ -486,16 +549,23 @@ export async function runSync(args: string[], options: SyncOptions = {}): Promis const localLock = await readLocalLock(cwd); if (options.cleanup !== false) { const keep = new Set(discoveredSkills.map((skill) => sanitizeName(skill.name))); - const stale = await pruneStaleSkills(cwd, keep, localLock, options.dryRun ?? false); + // --no-remote leaves remote skills alone, both installing and removing + const keepRemote = (entry: LocalSkillLockEntry) => + options.remote === false || discovery.remote.some((r) => isFromRequest(entry, r)); + const stale = await pruneStaleSkills(cwd, keep, keepRemote, localLock, options.dryRun ?? false); for (const name of stale) { p.log.info( - `${options.dryRun ? 'Would remove' : 'Removed'} ${pc.cyan(name)} ${pc.dim('(no longer shipped by a dependency)')}` + `${options.dryRun ? 'Would remove' : 'Removed'} ${pc.cyan(name)} ${pc.dim('(no longer provided by a dependency)')}` ); } } - if (discoveredSkills.length === 0) { - p.outro(pc.dim('No SKILL.md files found in the dependencies listed in package.json.')); + const remoteRequests = (options.remote === false ? [] : discovery.remote).filter( + (request) => !isRemoteInstalled(request, localLock, cwd) + ); + + if (discoveredSkills.length === 0 && remoteRequests.length === 0) { + p.outro(pc.dim('Nothing to sync.')); return; } @@ -567,7 +637,7 @@ export async function runSync(args: string[], options: SyncOptions = {}): Promis for (const { skill, reason } of skipped) { p.log.warn(`Skipped ${pc.cyan(skill.name)} ${pc.dim(`from ${skill.packageName}`)}: ${reason}`); } - if (toInstall.length === 0) { + if (toInstall.length === 0 && remoteRequests.length === 0) { console.log(); p.outro(pc.yellow('Nothing to sync.')); return; @@ -584,6 +654,12 @@ export async function runSync(args: string[], options: SyncOptions = {}): Promis ); } + for (const request of remoteRequests) { + summaryLines.push( + `${pc.cyan(getProjectLockSource(request.parsed))} ${pc.dim(`← ${request.via} (remote)`)}` + ); + } + console.log(); p.note(summaryLines.join('\n'), 'Sync Summary'); @@ -663,7 +739,48 @@ export async function runSync(args: string[], options: SyncOptions = {}): Promis } } - // 7. Display results + // 7. Install skills that remote `skills` entries request + const claimed = new Set(discoveredSkills.map((skill) => sanitizeName(skill.name))); + const lockEntryFor = (name: string) => + Object.entries(localLock.skills).find(([key]) => sanitizeName(key) === name)?.[1]; + for (const request of remoteRequests) { + const label = getProjectLockSource(request.parsed); + const result = await installFromSource(request.parsed, { + skills: request.skills, + agents: targetAgents, + via: request.via, + select: async (skills) => { + const selected: Skill[] = []; + for (const skill of skills) { + const name = sanitizeName(skill.name); + const reason = claimed.has(name) + ? 'another source in this sync provides it' + : await blockedReason(name, targetAgents, lockEntryFor(name), cwd); + if (reason) { + p.log.warn(`Skipped ${pc.cyan(name)} from ${label}: ${reason}`); + continue; + } + claimed.add(name); + selected.push(skill); + } + return selected; + }, + }); + if (result.error) { + p.log.error(`Failed to install from ${pc.cyan(label)}: ${result.error}`); + process.exitCode = 1; + continue; + } + if (result.installed.length > 0) { + p.log.success( + `Installed ${result.installed.map((name) => pc.cyan(name)).join(', ')} from ${label}` + ); + } + for (const failure of result.failed) p.log.error(failure); + if (result.failed.length > 0) process.exitCode = 1; + } + + // 8. Display results console.log(); if (successful.length > 0) { @@ -735,6 +852,8 @@ export function parseSyncOptions(args: string[]): { options: SyncOptions } { options.dryRun = true; } else if (arg === '--no-cleanup') { options.cleanup = false; + } else if (arg === '--no-remote') { + options.remote = false; } else if (arg === '-a' || arg === '--agent') { options.agent = [...(options.agent ?? []), ...takeValues()]; } else if (arg === '--include') { diff --git a/tests/install.test.ts b/tests/install.test.ts index d0e8b4192..bc8039db5 100644 --- a/tests/install.test.ts +++ b/tests/install.test.ts @@ -36,7 +36,7 @@ describe('runInstallFromLock', () => { await runInstallFromLock([]); expect(add.installFromSource).toHaveBeenCalledWith( - 'https://gitlab.example.com/acme/skills.git', + expect.objectContaining({ url: 'https://gitlab.example.com/acme/skills.git' }), { skills: ['skill-a'], agents: ['cursor'] } ); }); diff --git a/tests/skills-field.test.ts b/tests/skills-field.test.ts index eb5eecc89..a2b529676 100644 --- a/tests/skills-field.test.ts +++ b/tests/skills-field.test.ts @@ -13,14 +13,57 @@ describe('parseSkillsField', () => { { package: '@vueuse/skills', skills: [] }, { package: 'my-lib', skills: ['a', 'b'] }, ], + remote: [], errors: [], }); }); - it('skips remote entries', () => { + it('parses git sources, folding @skill and ref into the request', () => { + const { remote, errors } = parseSkillsField( + [ + 'owner/repo@one', + { source: 'owner/repo', ref: 'v1', skills: ['two'] }, + 'https://gitlab.com/group/repo/-/tree/main/skills', + ], + '.' + ); + expect(errors).toEqual([]); + expect(remote).toEqual([ + { + parsed: expect.objectContaining({ + type: 'github', + url: 'https://github.com/owner/repo.git', + }), + skills: ['one'], + }, + { parsed: expect.objectContaining({ type: 'github', ref: 'v1' }), skills: ['two'] }, + { + parsed: expect.objectContaining({ type: 'gitlab', ref: 'main', subpath: 'skills' }), + skills: [], + }, + ]); + }); + + it('rejects sources that are not git-hosted', () => { + expect(parseSkillsField(['./local', 'https://example.com/skills'], 'my-pack').errors).toEqual([ + 'my-pack: "./local" is not a git source', + 'my-pack: "https://example.com/skills" is not a git source', + ]); + }); + + it('rejects a ref on a source that already has one', () => { expect( - parseSkillsField(['owner/repo@skill', { source: 'owner/repo', ref: 'v1' }, 'npm:x'], '.') - ).toEqual({ npm: [{ package: 'x', skills: [] }], errors: [] }); + parseSkillsField( + [ + { source: 'owner/repo#v1', ref: 'v2' }, + { source: 'https://github.com/o/r/tree/main/x', ref: 'v2' }, + ], + '.' + ).errors + ).toEqual([ + '.: "owner/repo#v1" already has a ref; remove "ref"', + '.: "https://github.com/o/r/tree/main/x" already has a ref; remove "ref"', + ]); }); it('rejects ref on an npm: entry', () => { diff --git a/tests/sync.test.ts b/tests/sync.test.ts index 251e19e9d..62f3a828e 100644 --- a/tests/sync.test.ts +++ b/tests/sync.test.ts @@ -11,6 +11,8 @@ import { writeFileSync, } from 'fs'; import { join, resolve } from 'path'; +import { execFileSync } from 'child_process'; +import { pathToFileURL } from 'url'; import { tmpdir } from 'os'; import { runCli } from '../src/test-utils.ts'; @@ -690,6 +692,191 @@ describe('experimental_sync command', () => { }); }); + describe('remote skills field entries', () => { + const sync = (...flags: string[]) => + runCli(['experimental_sync', '-y', '-a', 'claude-code', ...flags], testDir); + const canonical = (name: string) => join(testDir, '.agents', 'skills', name); + const readLock = () => JSON.parse(readFileSync(join(testDir, 'skills-lock.json'), 'utf-8')); + + /** A local git repository with one skills/ folder per name; returns its file:// URL. */ + function createSkillsRepo(dirName: string, names: string[]): string { + const repo = join(testDir, '..', `${testDir.split(/[\\/]/).pop()}-${dirName}`); + for (const name of names) writeSkill(join(repo, 'skills', name), name); + const git = (...args: string[]) => + execFileSync('git', ['-c', 'user.name=t', '-c', 'user.email=t@t', ...args], { + cwd: repo, + stdio: 'ignore', + }); + git('init', '-q'); + git('add', '.'); + git('commit', '-q', '-m', 'skills'); + return pathToFileURL(repo).href; + } + + function declareProject(deps: string[], skills: unknown): void { + writeFileSync( + join(testDir, 'package.json'), + JSON.stringify({ + name: 'test-project', + dependencies: Object.fromEntries(deps.map((d) => [d, '*'])), + skills, + }) + ); + } + + afterEach(() => { + for (const suffix of ['repo', 'other']) { + rmSync(join(testDir, '..', `${testDir.split(/[\\/]/).pop()}-${suffix}`), { + recursive: true, + force: true, + }); + } + }); + + it('installs skills from a git source and records via', () => { + const repo = createSkillsRepo('repo', ['remote-a']); + declareProject([], [repo]); + + const result = sync(); + + expect(result.exitCode).toBe(0); + expect(lstatSync(canonical('remote-a')).isDirectory()).toBe(true); + expect(readLock().skills['remote-a']).toMatchObject({ + source: repo, + sourceType: 'git', + via: '.', + }); + }); + + it('records the pack that requested the skill', () => { + const repo = createSkillsRepo('repo', ['remote-a']); + declareProject(['my-pack'], undefined); + const pack = join(testDir, 'node_modules', 'my-pack'); + mkdirSync(pack, { recursive: true }); + writeFileSync( + join(pack, 'package.json'), + JSON.stringify({ name: 'my-pack', skills: [repo] }) + ); + + sync(); + + expect(readLock().skills['remote-a'].via).toBe('my-pack'); + }); + + it('keeps only the skills an object entry names', () => { + const repo = createSkillsRepo('repo', ['remote-a', 'remote-b']); + declareProject([], [{ source: repo, skills: ['remote-a'] }]); + + sync(); + + expect(existsSync(canonical('remote-a'))).toBe(true); + expect(existsSync(canonical('remote-b'))).toBe(false); + }); + + it('does not fetch again once installed', () => { + const repo = createSkillsRepo('repo', ['remote-a']); + declareProject([], [repo]); + sync(); + rmSync(join(testDir, '..', `${testDir.split(/[\\/]/).pop()}-repo`), { + recursive: true, + force: true, + }); + + const result = sync(); + + expect(result.exitCode).toBe(0); + expect(result.stdout).toContain('Nothing to sync'); + expect(existsSync(canonical('remote-a'))).toBe(true); + }); + + it('installs again when the skill folder is gone', () => { + const repo = createSkillsRepo('repo', ['remote-a']); + declareProject([], [repo]); + sync(); + rmSync(canonical('remote-a'), { recursive: true, force: true }); + + sync(); + + expect(existsSync(join(canonical('remote-a'), 'SKILL.md'))).toBe(true); + }); + + it('removes skills that no field requests anymore', () => { + const repo = createSkillsRepo('repo', ['remote-a']); + declareProject([], [repo]); + sync(); + + declareProject([], []); + sync(); + + expect(existsSync(canonical('remote-a'))).toBe(false); + expect(readLock().skills['remote-a']).toBeUndefined(); + }); + + it('--no-remote neither installs nor removes remote skills', () => { + const repo = createSkillsRepo('repo', ['remote-a', 'remote-b']); + declareProject([], [{ source: repo, skills: ['remote-a'] }]); + sync(); + + declareProject([], [{ source: repo, skills: ['remote-b'] }]); + sync('--no-remote'); + + expect(existsSync(canonical('remote-a'))).toBe(true); + expect(existsSync(canonical('remote-b'))).toBe(false); + }); + + it('prefers a skill shipped by a dependency', () => { + const repo = createSkillsRepo('repo', ['shared']); + declareProject(['my-lib'], [repo]); + writeSkill(join(createPackage('my-lib'), 'skills', 'shared'), 'shared'); + + const result = sync(); + + expect(result.stdout).toContain('another source in this sync provides it'); + expect(lstatSync(canonical('shared')).isSymbolicLink()).toBe(true); + expect(readLock().skills.shared.sourceType).toBe('node_modules'); + }); + + it('never shadows a skill installed with skills add', () => { + const repo = createSkillsRepo('repo', ['remote-a']); + declareProject([], [repo]); + writeSkill(canonical('remote-a'), 'remote-a'); + writeFileSync(join(canonical('remote-a'), 'mine.md'), 'keep'); + writeFileSync( + join(testDir, 'skills-lock.json'), + JSON.stringify({ + version: 1, + skills: { 'remote-a': { source: 'owner/repo', sourceType: 'github', computedHash: 'x' } }, + }) + ); + + const result = sync(); + + expect(result.stdout).toContain('installed with `skills add`'); + expect(existsSync(join(canonical('remote-a'), 'mine.md'))).toBe(true); + }); + + it('reports a failing source and still installs the rest', () => { + const missing = pathToFileURL(join(testDir, 'no-such-repo')).href; + declareProject(['my-lib'], [missing]); + writeSkill(join(createPackage('my-lib'), 'skills', 'shipped'), 'shipped'); + + const result = sync(); + + expect(result.exitCode).toBe(1); + expect(result.stdout).toContain('Failed to install from'); + expect(lstatSync(canonical('shipped')).isSymbolicLink()).toBe(true); + }); + + it('stops on a project entry that is not a git source', () => { + declareProject([], ['./local-skills']); + + const result = sync(); + + expect(result.exitCode).toBe(1); + expect(result.stdout).toContain('is not a git source'); + }); + }); + describe('CLI routing', () => { it('shows experimental_sync in help output', () => { const result = runCli(['--help']);