diff --git a/src/add.ts b/src/add.ts index 0b17a09b6..66cc1d3af 100644 --- a/src/add.ts +++ b/src/add.ts @@ -1330,13 +1330,17 @@ interface SourceInstallResult { */ export async function installFromSource( source: string, - options: { skills: string[]; agents: AgentType[] } + options: { skills: string[]; agents: AgentType[]; fullDepth?: boolean } ): 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); + resolved = await resolveSkills( + parsed, + { includeInternal: options.skills.length > 0, fullDepth: options.fullDepth }, + spinner + ); const selected = options.skills.length > 0 ? filterSkills(resolved.skills, options.skills) : resolved.skills; if (selected.length === 0) { diff --git a/src/install.ts b/src/install.ts index 41b046e09..422bef3a7 100644 --- a/src/install.ts +++ b/src/install.ts @@ -4,7 +4,7 @@ import { readLocalLock } from './local-lock.ts'; import { installFromSource, runAdd } from './add.ts'; import { runSync, parseSyncOptions } from './sync.ts'; import { getUniversalAgents } from './agents.ts'; -import { buildLocalUpdateSource } from './update-source.ts'; +import { buildLocalUpdateSource, shouldUseFullDepthForUpdate } from './update-source.ts'; /** * Install all skills from the local skills-lock.json. @@ -34,7 +34,7 @@ export async function runInstallFromLock(args: string[]): Promise { // Separate node_modules skills from remote skills const nodeModuleSkills: string[] = []; - const bySource = new Map(); + const bySource = new Map(); for (const [skillName, entry] of skillEntries) { if (entry.sourceType === 'node_modules') { @@ -52,10 +52,12 @@ export async function runInstallFromLock(args: string[]): Promise { const existing = bySource.get(installSource); if (existing) { existing.skills.push(skillName); + existing.fullDepth ||= shouldUseFullDepthForUpdate(entry); } else { bySource.set(installSource, { sourceType: entry.sourceType, skills: [skillName], + fullDepth: shouldUseFullDepthForUpdate(entry), }); } } @@ -68,13 +70,17 @@ export async function runInstallFromLock(args: string[]): Promise { } // Install remote skills grouped by source - for (const [source, { sourceType, skills }] of bySource) { + for (const [source, { sourceType, skills, fullDepth }] of bySource) { // well-known sources go through their provider flow, which installs on its own if (sourceType === 'well-known') { await runAdd([source], { skill: skills, agent: universalAgentNames, yes: true }); continue; } - const result = await installFromSource(source, { skills, agents: universalAgentNames }); + const result = await installFromSource(source, { + skills, + agents: universalAgentNames, + fullDepth, + }); if (result.error) { p.log.error(`Failed to install from ${pc.cyan(source)}: ${result.error}`); process.exitCode = 1; diff --git a/src/source-parser.test.ts b/src/source-parser.test.ts index daf492c18..84a4a0533 100644 --- a/src/source-parser.test.ts +++ b/src/source-parser.test.ts @@ -186,6 +186,20 @@ describe('source-parser', () => { }); }); + it('preserves fragment skill filters on explicit GitHub Enterprise URLs', () => { + vi.stubEnv('GH_HOST', 'github.example.com'); + const result = parseSource( + 'https://github.example.com/acme/skills/tree/main/tools#other@review' + ); + expect(result).toEqual({ + type: 'git', + url: 'https://github.example.com/acme/skills.git', + ref: 'main', + subpath: 'tools', + skillFilter: 'review', + }); + }); + it('does not override explicit github.com URLs with GH_HOST', () => { vi.stubEnv('GH_HOST', 'github.example.com'); diff --git a/src/source-parser.ts b/src/source-parser.ts index df83b0f01..ab96482a5 100644 --- a/src/source-parser.ts +++ b/src/source-parser.ts @@ -257,6 +257,11 @@ function looksLikeGitSource(input: string): boolean { return /^\/.+?\/[^/]+(?:\.git)?(?:\/-\/tree\/[^/]+(?:\/.*)?)?\/?$/.test(pathname); } + // Self-hosted GitLab tree URLs have the same distinctive /-/tree/ marker. + if (/^\/.+?\/-\/tree\/[^/]+(?:\/.*)?$/.test(pathname)) { + return true; + } + // Azure Repos clone and web URLs use /_git/{repo} on any host // (dev.azure.com, *.visualstudio.com, Azure DevOps Server). const azureSegments = pathname.split('/').filter(Boolean); @@ -415,6 +420,7 @@ export function parseSource(input: string): ParsedSource { const isTreeUrl = marker === 'tree' && ref; return { type: 'git', + ...(fragmentSkillFilter ? { skillFilter: fragmentSkillFilter } : {}), url: `${parsedUrl.protocol}//${parsedUrl.host}/${owner}/${repo}.git`, ...(isTreeUrl ? { ref } : fragmentRef ? { ref: fragmentRef } : {}), ...(isTreeUrl && subpathSegments.length > 0 @@ -434,6 +440,7 @@ export function parseSource(input: string): ParsedSource { const [, owner, repo, ref, subpath] = githubTreeWithPathMatch; return { type: 'github', + ...(fragmentSkillFilter ? { skillFilter: fragmentSkillFilter } : {}), url: `https://github.com/${owner}/${repo}.git`, ref: ref || fragmentRef, subpath: subpath ? sanitizeSubpath(subpath) : subpath, @@ -446,6 +453,7 @@ export function parseSource(input: string): ParsedSource { const [, owner, repo, ref] = githubTreeMatch; return { type: 'github', + ...(fragmentSkillFilter ? { skillFilter: fragmentSkillFilter } : {}), url: `https://github.com/${owner}/${repo}.git`, ref: ref || fragmentRef, }; @@ -458,6 +466,7 @@ export function parseSource(input: string): ParsedSource { const cleanRepo = repo!.replace(/\.git$/, ''); return { type: 'github', + ...(fragmentSkillFilter ? { skillFilter: fragmentSkillFilter } : {}), url: `https://github.com/${owner}/${cleanRepo}.git`, ...(fragmentRef ? { ref: fragmentRef } : {}), }; @@ -474,6 +483,7 @@ export function parseSource(input: string): ParsedSource { if (hostname !== 'github.com' && repoPath) { return { type: 'gitlab', + ...(fragmentSkillFilter ? { skillFilter: fragmentSkillFilter } : {}), url: `${protocol}://${hostname}/${repoPath.replace(/\.git$/, '')}.git`, ref: ref || fragmentRef, subpath: subpath ? sanitizeSubpath(subpath) : subpath, @@ -488,6 +498,7 @@ export function parseSource(input: string): ParsedSource { if (hostname !== 'github.com' && repoPath) { return { type: 'gitlab', + ...(fragmentSkillFilter ? { skillFilter: fragmentSkillFilter } : {}), url: `${protocol}://${hostname}/${repoPath.replace(/\.git$/, '')}.git`, ref: ref || fragmentRef, }; @@ -504,6 +515,7 @@ export function parseSource(input: string): ParsedSource { if (repoPath.includes('/')) { return { type: 'gitlab', + ...(fragmentSkillFilter ? { skillFilter: fragmentSkillFilter } : {}), url: `https://gitlab.com/${repoPath}.git`, ...(fragmentRef ? { ref: fragmentRef } : {}), }; @@ -562,6 +574,7 @@ export function parseSource(input: string): ParsedSource { // Fallback: treat as direct git URL return { type: 'git', + ...(fragmentSkillFilter ? { skillFilter: fragmentSkillFilter } : {}), url: input, ...(fragmentRef ? { ref: fragmentRef } : {}), }; diff --git a/tests/install-from-lock.test.ts b/tests/install-from-lock.test.ts index 188c4bcf3..ad93c34c3 100644 --- a/tests/install-from-lock.test.ts +++ b/tests/install-from-lock.test.ts @@ -1,6 +1,8 @@ import { describe, it, expect, beforeEach, afterEach } from 'vitest'; import { existsSync, mkdirSync, readFileSync, rmSync, writeFileSync } from 'fs'; import { join } from 'path'; +import { execFileSync } from 'child_process'; +import { pathToFileURL } from 'url'; import { tmpdir } from 'os'; import { runCli } from '../src/test-utils.ts'; @@ -59,6 +61,57 @@ describe('experimental_install', () => { expect(lock.skills.alpha.computedHash).toMatch(/^[a-f0-9]{64}$/); }); + it('restores nested skills from a full-depth Git install without installing the root skill', () => { + const repo = join(testDir, 'fixture.git'); + const project = join(testDir, 'project'); + mkdirSync(project); + writeSkill(repo, 'root'); + writeSkill(join(repo, 'skills', 'demo'), 'demo'); + execFileSync('git', ['init', '-b', 'main', repo]); + execFileSync('git', ['add', '.'], { cwd: repo }); + execFileSync( + 'git', + [ + '-c', + 'user.name=Fixture', + '-c', + 'user.email=fixture@example.com', + '-c', + 'commit.gpgsign=false', + 'commit', + '-m', + 'fixture', + ], + { cwd: repo } + ); + const env = { DISABLE_TELEMETRY: '1' }; + const added = runCli( + [ + 'add', + pathToFileURL(repo).href, + '--full-depth', + '--skill', + 'demo', + '--agent', + 'universal', + '--yes', + ], + project, + env + ); + expect(added.exitCode, added.stdout + added.stderr).toBe(0); + rmSync(join(project, '.agents'), { recursive: true }); + + const restored = runCli(['experimental_install'], project, env); + + expect(restored.exitCode, restored.stdout + restored.stderr).toBe(0); + expect(existsSync(join(project, '.agents', 'skills', 'demo', 'SKILL.md'))).toBe(true); + expect(existsSync(join(project, '.agents', 'skills', 'root'))).toBe(false); + const lock = JSON.parse(readFileSync(join(project, 'skills-lock.json'), 'utf-8')); + expect(lock.skills.demo.skillPath).toBe('skills/demo/SKILL.md'); + expect(lock.skills.demo.sourceUrl).toBe(pathToFileURL(repo).href); + }); + it('keeps restoring after a source fails', () => { writeSkill(join(testDir, 'src-b', 'beta'), 'beta'); writeLock({ alpha: './missing', beta: './src-b' }); diff --git a/tests/install.test.ts b/tests/install.test.ts index d0e8b4192..d578e455b 100644 --- a/tests/install.test.ts +++ b/tests/install.test.ts @@ -37,10 +37,60 @@ describe('runInstallFromLock', () => { expect(add.installFromSource).toHaveBeenCalledWith( 'https://gitlab.example.com/acme/skills.git', - { skills: ['skill-a'], agents: ['cursor'] } + { skills: ['skill-a'], agents: ['cursor'], fullDepth: true } ); }); + it('searches full depth when any skill in a shared Git source requires it', async () => { + vi.mocked(localLock.readLocalLock).mockResolvedValue({ + version: 1, + skills: { + root: { + source: 'acme/skills', + sourceUrl: 'ssh://git@example.com/acme/skills.git', + sourceType: 'git', + computedHash: 'hash', + }, + nested: { + source: 'acme/skills', + sourceUrl: 'ssh://git@example.com/acme/skills.git', + sourceType: 'git', + skillPath: 'skills/nested/SKILL.md', + computedHash: 'hash', + }, + }, + }); + + await runInstallFromLock([]); + + expect(add.installFromSource).toHaveBeenCalledExactlyOnceWith( + 'ssh://git@example.com/acme/skills.git', + { skills: ['root', 'nested'], agents: ['cursor'], fullDepth: true } + ); + }); + + it('keeps path-targeted GitHub restores on ordinary discovery', async () => { + vi.mocked(localLock.readLocalLock).mockResolvedValue({ + version: 1, + skills: { + review: { + source: 'acme/skills', + sourceType: 'github', + skillPath: 'skills/review/SKILL.md', + computedHash: 'hash', + }, + }, + }); + + await runInstallFromLock([]); + + expect(add.installFromSource).toHaveBeenCalledExactlyOnceWith('acme/skills/skills/review', { + skills: ['review'], + agents: ['cursor'], + fullDepth: false, + }); + }); + it('does not restore generic git shorthands as GitHub without sourceUrl', async () => { vi.mocked(localLock.readLocalLock).mockResolvedValue({ version: 1, diff --git a/tests/source-parser.test.ts b/tests/source-parser.test.ts index 2873f188d..7f4a066a4 100644 --- a/tests/source-parser.test.ts +++ b/tests/source-parser.test.ts @@ -307,6 +307,33 @@ describe('parseSource', () => { }); }); + describe('fragment skill filters', () => { + it.each([ + ['https://github.com/owner/repo', undefined], + ['https://github.com/owner/repo.git', undefined], + ['https://github.com/owner/repo/tree/main', undefined], + ['https://github.com/owner/repo/tree/main/skills', 'skills'], + ['https://gitlab.com/group/subgroup/repo', undefined], + ['gitlab:owner/repo', undefined], + ['https://gitlab.example.com/group/repo/-/tree/main', undefined], + ['https://gitlab.example.com/group/repo/-/tree/main/skills', 'skills'], + ['ssh://git@example.com/owner/repo.git', undefined], + ['git@example.com:owner/repo.git', undefined], + ])('preserves the selected skill for %s', (source, subpath) => { + const result = parseSource(`${source}#main@my%20skill`); + expect(result.ref).toBe('main'); + expect(result.skillFilter).toBe('my skill'); + expect(result.subpath).toBe(subpath); + expect(result.url).not.toContain('#'); + }); + + it('keeps the tree URL ref when the fragment specifies another ref', () => { + const result = parseSource('https://github.com/owner/repo/tree/main#other@review'); + expect(result.ref).toBe('main'); + expect(result.skillFilter).toBe('review'); + }); + }); + describe('Local path tests', () => { it('Local path - relative with ./', () => { const result = parseSource('./my-skills');