From cf057ecb9ec4b385b1dc879d4a8cbe4241c332c1 Mon Sep 17 00:00:00 2001 From: Christopher Tso Date: Tue, 19 May 2026 10:32:48 +0200 Subject: [PATCH 1/2] fix(skill): support standalone skill URLs --- src/cli/commands/plugin-skills.ts | 19 ++++++--- src/core/git-errors.ts | 9 ++++- src/core/git.ts | 41 +++++++++++++++----- src/core/skills.ts | 2 +- src/core/workspace-modify.ts | 4 ++ tests/unit/core/extract-plugin-names.test.ts | 23 +++++++++++ tests/unit/core/skills.test.ts | 22 +++++++++++ 7 files changed, 102 insertions(+), 18 deletions(-) diff --git a/src/cli/commands/plugin-skills.ts b/src/cli/commands/plugin-skills.ts index 396fff74..2b410aeb 100644 --- a/src/cli/commands/plugin-skills.ts +++ b/src/cli/commands/plugin-skills.ts @@ -110,6 +110,12 @@ async function recordSourceProvenance(opts: { }); } +function resolveFetchedSourcePath(source: string, cachePath: string): string { + if (!isGitHubUrl(source)) return cachePath; + const parsed = parseGitHubUrl(source); + return parsed?.subpath ? join(cachePath, parsed.subpath) : cachePath; +} + /** * Extract the inline `@` suffix from a plugin source spec, if present. * Only matches owner/repo-style sources (must have a slash before the `@`), @@ -585,9 +591,10 @@ async function installSkillDirect(opts: { cachePath: string; }): Promise { const { skill, from, isUser, workspacePath, cachePath } = opts; + const sourcePath = resolveFetchedSourcePath(from, cachePath); // Verify the skill exists in the cached plugin before installing - const availableSkills = await discoverSkillNames(cachePath); + const availableSkills = await discoverSkillNames(sourcePath); if (!availableSkills.includes(skill)) { return { success: false, @@ -608,7 +615,7 @@ async function installSkillDirect(opts: { } } - const pluginName = getPluginName(cachePath); + const pluginName = getPluginName(sourcePath); return applySkillAllowlist({ skill, pluginName, isUser, workspacePath }); } @@ -751,7 +758,8 @@ async function discoverSkillsFromSource(from: string): Promise< return { success: true, skills: all, isMarketplace: true }; } - const skills = await discoverSkillsWithMetadata(fetchResult.cachePath); + const sourcePath = resolveFetchedSourcePath(from, fetchResult.cachePath); + const skills = await discoverSkillsWithMetadata(sourcePath); return { success: true, skills, isMarketplace: false }; } @@ -788,7 +796,8 @@ async function installAllSkillsFromSource(opts: { } // Direct plugin install — enable every discovered skill - const skillNames = await discoverSkillNames(fetchResult.cachePath); + const sourcePath = resolveFetchedSourcePath(from, fetchResult.cachePath); + const skillNames = await discoverSkillNames(sourcePath); if (skillNames.length === 0) { return { success: false, error: `No skills found in '${from}'.` }; } @@ -803,7 +812,7 @@ async function installAllSkillsFromSource(opts: { } } - const pluginName = getPluginName(fetchResult.cachePath); + const pluginName = getPluginName(sourcePath); const setModeResult = isUser ? await setUserPluginSkillsMode(pluginName, 'allowlist', skillNames) diff --git a/src/core/git-errors.ts b/src/core/git-errors.ts index 0d42dd7a..92c11d3c 100644 --- a/src/core/git-errors.ts +++ b/src/core/git-errors.ts @@ -22,7 +22,11 @@ export class GitCloneError extends Error { } } -export function classifyError(error: unknown, url: string): GitCloneError { +export function classifyError( + error: unknown, + url: string, + timeoutMs = 60_000, +): GitCloneError { const errorMessage = error instanceof Error ? error.message : String(error); @@ -44,8 +48,9 @@ export function classifyError(error: unknown, url: string): GitCloneError { errorMessage.includes('Internal Server Error'); if (isTimeout) { + const timeoutSeconds = Math.round(timeoutMs / 1000); return new GitCloneError( - `Clone timed out after 60s for ${url}.\n Check your network connection and repository access.\n For SSH: ssh-add -l (to check loaded keys)\n For HTTPS: Check your git credentials`, + `Clone timed out after ${timeoutSeconds}s for ${url}.\n Check your network connection and repository access.\n For SSH: ssh-add -l (to check loaded keys)\n For HTTPS: Check your git credentials`, url, true, false, diff --git a/src/core/git.ts b/src/core/git.ts index 9111f532..9b4ccc3f 100644 --- a/src/core/git.ts +++ b/src/core/git.ts @@ -6,7 +6,30 @@ import { GitCloneError, classifyError } from './git-errors.js'; export { GitCloneError, classifyError }; -const CLONE_TIMEOUT_MS = 60_000; // 60 seconds +const DEFAULT_CLONE_TIMEOUT_MS = 300_000; +const CLONE_TIMEOUT_MS = (() => { + const raw = process.env.ALLAGENTS_CLONE_TIMEOUT_MS; + if (!raw) return DEFAULT_CLONE_TIMEOUT_MS; + const parsed = Number.parseInt(raw, 10); + return Number.isFinite(parsed) && parsed > 0 ? parsed : DEFAULT_CLONE_TIMEOUT_MS; +})(); + +function createGit(baseDir?: string) { + return simpleGit(baseDir, { + timeout: { block: CLONE_TIMEOUT_MS }, + env: { + ...process.env, + GIT_TERMINAL_PROMPT: '0', + GIT_LFS_SKIP_SMUDGE: '1', + }, + config: [ + 'filter.lfs.required=false', + 'filter.lfs.smudge=', + 'filter.lfs.clean=', + 'filter.lfs.process=', + ], + }); +} /** * Build an HTTPS GitHub URL from owner/repo. @@ -24,7 +47,7 @@ export async function cloneToTemp( ref?: string, ): Promise { const tempDir = await mkdtemp(join(tmpdir(), 'allagents-')); - const git = simpleGit({ timeout: { block: CLONE_TIMEOUT_MS } }); + const git = createGit(); const cloneOptions = ref ? ['--depth', '1', '--branch', ref] : ['--depth', '1']; @@ -34,7 +57,7 @@ export async function cloneToTemp( return tempDir; } catch (error) { await rm(tempDir, { recursive: true, force: true }).catch(() => {}); - throw classifyError(error, url); + throw classifyError(error, url, CLONE_TIMEOUT_MS); } } @@ -46,7 +69,7 @@ export async function cloneTo( dest: string, ref?: string, ): Promise { - const git = simpleGit({ timeout: { block: CLONE_TIMEOUT_MS } }); + const git = createGit(); const cloneOptions = ref ? ['--depth', '1', '--branch', ref] : ['--depth', '1']; @@ -54,7 +77,7 @@ export async function cloneTo( try { await git.clone(url, dest, cloneOptions); } catch (error) { - throw classifyError(error, url); + throw classifyError(error, url, CLONE_TIMEOUT_MS); } } @@ -62,9 +85,7 @@ export async function cloneTo( * Pull latest changes in an existing repository. */ export async function pull(repoPath: string): Promise { - const git = simpleGit(repoPath, { - timeout: { block: CLONE_TIMEOUT_MS }, - }); + const git = createGit(repoPath); await git.pull(); } @@ -73,7 +94,7 @@ export async function pull(repoPath: string): Promise { * Returns true if accessible, false otherwise. */ export async function repoExists(url: string): Promise { - const git = simpleGit({ timeout: { block: CLONE_TIMEOUT_MS } }); + const git = createGit(); try { await git.listRemote([url]); return true; @@ -89,7 +110,7 @@ export async function refExists( url: string, ref: string, ): Promise { - const git = simpleGit({ timeout: { block: CLONE_TIMEOUT_MS } }); + const git = createGit(); try { const result = await git.listRemote([ '--refs', diff --git a/src/core/skills.ts b/src/core/skills.ts index 0c08662d..867e8cfc 100644 --- a/src/core/skills.ts +++ b/src/core/skills.ts @@ -69,7 +69,7 @@ async function resolvePluginPath( const path = parsed?.subpath ? join(result.cachePath, parsed.subpath) : result.cachePath; - return { path }; + return existsSync(path) ? { path } : null; } // Local path diff --git a/src/core/workspace-modify.ts b/src/core/workspace-modify.ts index 09fa8719..85c9d006 100644 --- a/src/core/workspace-modify.ts +++ b/src/core/workspace-modify.ts @@ -410,6 +410,10 @@ export function extractPluginNames(pluginSource: string): string[] { const names = [parsed.repo]; const ownerRepo = `${parsed.owner}-${parsed.repo}`; if (ownerRepo !== parsed.repo) names.push(ownerRepo); + if (parsed.subpath) { + const subpathName = parsed.subpath.split('/').filter(Boolean).pop(); + if (subpathName && !names.includes(subpathName)) names.push(subpathName); + } return names; } } diff --git a/tests/unit/core/extract-plugin-names.test.ts b/tests/unit/core/extract-plugin-names.test.ts index c984f0b6..ce3e684b 100644 --- a/tests/unit/core/extract-plugin-names.test.ts +++ b/tests/unit/core/extract-plugin-names.test.ts @@ -19,6 +19,18 @@ describe('extractPluginNames', () => { expect(names).toContain('anthropics-skills'); }); + it('includes subpath basename for GitHub URLs with subpath', () => { + const names = extractPluginNames('https://github.com/NousResearch/hermes-agent/tree/main/skills/research/llm-wiki'); + expect(names).toContain('hermes-agent'); + expect(names).toContain('NousResearch-hermes-agent'); + expect(names).toContain('llm-wiki'); + }); + + it('includes subpath basename for GitHub shorthand with subpath', () => { + const names = extractPluginNames('NousResearch/hermes-agent/skills/research/llm-wiki'); + expect(names).toContain('llm-wiki'); + }); + it('handles plugin@marketplace spec', () => { const names = extractPluginNames('document-skills@anthropic-agent-skills'); expect(names).toContain('document-skills'); @@ -78,6 +90,17 @@ describe('findPluginEntryByName with GitHub URL entries', () => { expect(index).toBe(0); }); + it('matches subpath basename against URL entry with deep skill path', () => { + const config: WorkspaceConfig = { + plugins: ['https://github.com/NousResearch/hermes-agent/tree/main/skills/research/llm-wiki'], + repositories: [], + clients: ['copilot'], + version: 2, + }; + const index = findPluginEntryByName(config, 'llm-wiki'); + expect(index).toBe(0); + }); + it('returns -1 when no match', () => { const config: WorkspaceConfig = { plugins: ['https://github.com/other/repo'], diff --git a/tests/unit/core/skills.test.ts b/tests/unit/core/skills.test.ts index f1d13790..9c083750 100644 --- a/tests/unit/core/skills.test.ts +++ b/tests/unit/core/skills.test.ts @@ -4,6 +4,7 @@ import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { dump } from 'js-yaml'; import { getAllSkillsFromPlugins, type SkillInfo } from '../../../src/core/skills.js'; +import { getPluginCachePath } from '../../../src/utils/plugin-path.js'; describe('getAllSkillsFromPlugins', () => { let tmpDir: string; @@ -135,4 +136,25 @@ describe('getAllSkillsFromPlugins', () => { expect(skills).toHaveLength(1); expect(skills[0]!.name).toBe('sub-skill'); }); + + it('skips GitHub URL entries whose subpath no longer exists in cache', async () => { + const originalHome = process.env.HOME; + process.env.HOME = tmpDir; + try { + const cachePath = getPluginCachePath('owner', 'repo', 'main'); + await mkdir(cachePath, { recursive: true }); + + const config = { + repositories: [], + plugins: ['https://github.com/owner/repo/tree/main/missing/path'], + clients: ['claude'], + }; + await writeFile(join(tmpDir, '.allagents/workspace.yaml'), dump(config)); + + const skills = await getAllSkillsFromPlugins(tmpDir); + expect(skills).toEqual([]); + } finally { + process.env.HOME = originalHome; + } + }); }); From 34951929db98131b91af3c1712ab691692a4eccf Mon Sep 17 00:00:00 2001 From: Christopher Tso Date: Tue, 19 May 2026 10:35:53 +0200 Subject: [PATCH 2/2] fix(git): type safe clone env config --- src/core/git.ts | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/core/git.ts b/src/core/git.ts index 9b4ccc3f..2d3bb30c 100644 --- a/src/core/git.ts +++ b/src/core/git.ts @@ -17,17 +17,15 @@ const CLONE_TIMEOUT_MS = (() => { function createGit(baseDir?: string) { return simpleGit(baseDir, { timeout: { block: CLONE_TIMEOUT_MS }, - env: { - ...process.env, - GIT_TERMINAL_PROMPT: '0', - GIT_LFS_SKIP_SMUDGE: '1', - }, config: [ 'filter.lfs.required=false', 'filter.lfs.smudge=', 'filter.lfs.clean=', 'filter.lfs.process=', ], + }).env({ + GIT_TERMINAL_PROMPT: '0', + GIT_LFS_SKIP_SMUDGE: '1', }); }