diff --git a/src/cli/commands/plugin-skills.ts b/src/cli/commands/plugin-skills.ts index 5c3d986..1acd50f 100644 --- a/src/cli/commands/plugin-skills.ts +++ b/src/cli/commands/plugin-skills.ts @@ -113,7 +113,7 @@ async function recordSourceProvenance(opts: { }); } -function resolveFetchedSourcePath(source: string, cachePath: string): string { +export function resolveFetchedSourcePath(source: string, cachePath: string): string { if (!isGitHubUrl(source)) return cachePath; const parsed = parseGitHubUrl(source); return parsed?.subpath ? join(cachePath, parsed.subpath) : cachePath; @@ -458,13 +458,24 @@ type InstallSkillResult = * * In both cases, set the plugin to allowlist mode with only the requested skill. */ -async function installSkillFromSource(opts: { +type InstallSkillFromSourceDeps = { + fetchPlugin?: typeof fetchPlugin; + parseMarketplaceManifest?: typeof parseMarketplaceManifest; + installSkillViaMarketplace?: typeof installSkillViaMarketplace; + installSkillDirect?: typeof installSkillDirect; +}; + +export async function installSkillFromSource(opts: { skill: string; from: string; isUser: boolean; workspacePath: string; -}): Promise { +}, deps: InstallSkillFromSourceDeps = {}): Promise { const { skill, from, isUser, workspacePath } = opts; + const fetchPluginFn = deps.fetchPlugin ?? fetchPlugin; + const parseMarketplaceManifestFn = deps.parseMarketplaceManifest ?? parseMarketplaceManifest; + const installSkillViaMarketplaceFn = deps.installSkillViaMarketplace ?? installSkillViaMarketplace; + const installSkillDirectFn = deps.installSkillDirect ?? installSkillDirect; if (!isJsonMode()) { console.log(`Installing skill '${skill}' from ${from}...`); @@ -472,7 +483,7 @@ async function installSkillFromSource(opts: { // Fetch the source to a local cache so we can inspect it const parsed = isGitHubUrl(from) ? parseGitHubUrl(from) : null; - const fetchResult = await fetchPlugin(from, { + const fetchResult = await fetchPluginFn(from, { ...(parsed?.branch && { branch: parsed.branch }), }); if (!fetchResult.success) { @@ -482,14 +493,14 @@ async function installSkillFromSource(opts: { const sourcePath = resolveFetchedSourcePath(from, fetchResult.cachePath); // Check if the source is a marketplace - const manifestResult = await parseMarketplaceManifest(sourcePath); + const manifestResult = await parseMarketplaceManifestFn(sourcePath); if (manifestResult.success) { - return installSkillViaMarketplace({ skill, from, isUser, workspacePath }); + return installSkillViaMarketplaceFn({ skill, from, isUser, workspacePath }); } // Not a marketplace — install as a direct plugin - return installSkillDirect({ skill, from, isUser, workspacePath, cachePath: sourcePath }); + return installSkillDirectFn({ skill, from, isUser, workspacePath, sourcePath }); } /** @@ -593,10 +604,9 @@ async function installSkillDirect(opts: { from: string; isUser: boolean; workspacePath: string; - cachePath: string; + sourcePath: string; }): Promise { - const { skill, from, isUser, workspacePath, cachePath } = opts; - const sourcePath = resolveFetchedSourcePath(from, cachePath); + const { skill, from, isUser, workspacePath, sourcePath } = opts; // Verify the skill exists in the cached plugin before installing const availableSkills = await discoverSkillNames(sourcePath); diff --git a/tests/unit/cli/skills-add-standalone-install.test.ts b/tests/unit/cli/skills-add-standalone-install.test.ts new file mode 100644 index 0000000..94688fb --- /dev/null +++ b/tests/unit/cli/skills-add-standalone-install.test.ts @@ -0,0 +1,54 @@ +import { describe, expect, it, mock } from 'bun:test'; +import { installSkillFromSource } from '../../../src/cli/commands/plugin-skills.js'; + +describe('installSkillFromSource', () => { + it('passes the resolved GitHub skill subpath to direct install without resolving it twice', async () => { + const source = 'https://github.com/NousResearch/hermes-agent/tree/main/skills/research/llm-wiki'; + const installSkillDirectMock = mock(async (_opts: { + skill: string; + from: string; + isUser: boolean; + workspacePath: string; + sourcePath: string; + }) => ({ + success: true as const, + pluginName: 'llm-wiki', + syncResult: { copied: 1, failed: 0 }, + })); + + const result = await installSkillFromSource( + { + skill: 'llm-wiki', + from: source, + isUser: false, + workspacePath: '/tmp/workspace', + }, + { + fetchPlugin: async () => ({ + success: true as const, + action: 'fetched' as const, + cachePath: '/tmp/fetched-hermes-agent', + }), + parseMarketplaceManifest: async () => ({ + success: false as const, + error: 'not a marketplace', + }), + installSkillViaMarketplace: async () => ({ + success: false as const, + error: 'unexpected marketplace path', + }), + installSkillDirect: installSkillDirectMock, + }, + ); + + expect(result.success).toBe(true); + expect(installSkillDirectMock).toHaveBeenCalledTimes(1); + expect(installSkillDirectMock).toHaveBeenCalledWith({ + skill: 'llm-wiki', + from: source, + isUser: false, + workspacePath: '/tmp/workspace', + sourcePath: '/tmp/fetched-hermes-agent/skills/research/llm-wiki', + }); + }); +}); diff --git a/tests/unit/cli/skills-add-url-detection.test.ts b/tests/unit/cli/skills-add-url-detection.test.ts index 97ad84a..02065a3 100644 --- a/tests/unit/cli/skills-add-url-detection.test.ts +++ b/tests/unit/cli/skills-add-url-detection.test.ts @@ -2,7 +2,11 @@ import { describe, it, expect } from 'bun:test'; import { mkdtemp, writeFile, rm } from 'node:fs/promises'; import { join } from 'node:path'; import { tmpdir } from 'node:os'; -import { resolveSkillFromUrl, resolveSkillNameFromRepo } from '../../../src/cli/commands/plugin-skills.js'; +import { + resolveFetchedSourcePath, + resolveSkillFromUrl, + resolveSkillNameFromRepo, +} from '../../../src/cli/commands/plugin-skills.js'; import type { FetchResult } from '../../../src/core/plugin.js'; describe('resolveSkillFromUrl', () => { @@ -99,3 +103,18 @@ describe('resolveSkillNameFromRepo', () => { expect(result).toBe('fallback-name'); }); }); + +describe('resolveFetchedSourcePath', () => { + it('appends the GitHub subpath to the fetched repo root once', () => { + expect( + resolveFetchedSourcePath( + 'https://github.com/NousResearch/hermes-agent/tree/main/skills/research/llm-wiki', + '/tmp/hermes-agent-cache', + ), + ).toBe('/tmp/hermes-agent-cache/skills/research/llm-wiki'); + }); + + it('leaves non-GitHub sources unchanged', () => { + expect(resolveFetchedSourcePath('/tmp/local-skill', '/tmp/local-skill')).toBe('/tmp/local-skill'); + }); +});