Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 14 additions & 5 deletions src/cli/commands/plugin-skills.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 `@<ref>` suffix from a plugin source spec, if present.
* Only matches owner/repo-style sources (must have a slash before the `@`),
Expand Down Expand Up @@ -585,9 +591,10 @@ async function installSkillDirect(opts: {
cachePath: string;
}): Promise<InstallSkillResult> {
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,
Expand All @@ -608,7 +615,7 @@ async function installSkillDirect(opts: {
}
}

const pluginName = getPluginName(cachePath);
const pluginName = getPluginName(sourcePath);
return applySkillAllowlist({ skill, pluginName, isUser, workspacePath });
}

Expand Down Expand Up @@ -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 };
}

Expand Down Expand Up @@ -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}'.` };
}
Expand All @@ -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)
Expand Down
9 changes: 7 additions & 2 deletions src/core/git-errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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,
Expand Down
39 changes: 29 additions & 10 deletions src/core/git.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,28 @@ 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 },
config: [
'filter.lfs.required=false',
'filter.lfs.smudge=',
'filter.lfs.clean=',
'filter.lfs.process=',
],
}).env({
GIT_TERMINAL_PROMPT: '0',
GIT_LFS_SKIP_SMUDGE: '1',
});
}

/**
* Build an HTTPS GitHub URL from owner/repo.
Expand All @@ -24,7 +45,7 @@ export async function cloneToTemp(
ref?: string,
): Promise<string> {
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'];
Expand All @@ -34,7 +55,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);
}
}

Expand All @@ -46,25 +67,23 @@ export async function cloneTo(
dest: string,
ref?: string,
): Promise<void> {
const git = simpleGit({ timeout: { block: CLONE_TIMEOUT_MS } });
const git = createGit();
const cloneOptions = ref
? ['--depth', '1', '--branch', ref]
: ['--depth', '1'];

try {
await git.clone(url, dest, cloneOptions);
} catch (error) {
throw classifyError(error, url);
throw classifyError(error, url, CLONE_TIMEOUT_MS);
}
}

/**
* Pull latest changes in an existing repository.
*/
export async function pull(repoPath: string): Promise<void> {
const git = simpleGit(repoPath, {
timeout: { block: CLONE_TIMEOUT_MS },
});
const git = createGit(repoPath);
await git.pull();
}

Expand All @@ -73,7 +92,7 @@ export async function pull(repoPath: string): Promise<void> {
* Returns true if accessible, false otherwise.
*/
export async function repoExists(url: string): Promise<boolean> {
const git = simpleGit({ timeout: { block: CLONE_TIMEOUT_MS } });
const git = createGit();
try {
await git.listRemote([url]);
return true;
Expand All @@ -89,7 +108,7 @@ export async function refExists(
url: string,
ref: string,
): Promise<boolean> {
const git = simpleGit({ timeout: { block: CLONE_TIMEOUT_MS } });
const git = createGit();
try {
const result = await git.listRemote([
'--refs',
Expand Down
2 changes: 1 addition & 1 deletion src/core/skills.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions src/core/workspace-modify.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand Down
23 changes: 23 additions & 0 deletions tests/unit/core/extract-plugin-names.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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'],
Expand Down
22 changes: 22 additions & 0 deletions tests/unit/core/skills.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
});
});