diff --git a/src/commands/install.ts b/src/commands/install.ts index b4c015e..616bfe0 100644 --- a/src/commands/install.ts +++ b/src/commands/install.ts @@ -8,7 +8,7 @@ import { checkbox, confirm } from '@inquirer/prompts'; import { ExitPromptError } from '@inquirer/core'; import { hasValidFrontmatter, extractYamlField } from '../utils/yaml.js'; import { ANTHROPIC_MARKETPLACE_SKILLS } from '../utils/marketplace-skills.js'; -import { writeSkillMetadata } from '../utils/skill-metadata.js'; +import { writeSkillMetadata, buildCommitUrl } from '../utils/skill-metadata.js'; import type { InstallOptions } from '../types.js'; import type { SkillSourceMetadata, SkillSourceType } from '../utils/skill-metadata.js'; @@ -17,6 +17,8 @@ interface InstallSourceInfo { sourceType: SkillSourceType; repoUrl?: string; localRoot?: string; + commitSha?: string; + commitUrl?: string; } /** @@ -158,6 +160,26 @@ export async function installSkill(source: string, options: InstallOptions): Pro stdio: 'pipe', }); spinner.succeed('Repository cloned'); + + const commitSha = execSync(`git -C "${tempDir}/repo" rev-parse HEAD`, { + encoding: 'utf-8', + stdio: 'pipe', + }).trim(); + sourceInfo.commitSha = commitSha; + sourceInfo.commitUrl = buildCommitUrl(repoUrl, commitSha); + if (!sourceInfo.commitUrl) { + // Clone worked — only the clickable-URL builder couldn't recognize + // the host. Surface it as a warning, not an error, so the user knows + // the install succeeded and the missing field isn't a sign of failure. + // We don't echo the raw repoUrl here: a crafted input could trick an + // editor's auto-linkifier into rendering attacker-controlled text as + // clickable. The repoUrl is already visible above in the spinner log. + console.log( + chalk.yellow( + 'Warning: clone succeeded, but the `commitUrl` field was not written to .openskills.json (host not yet supported for clickable URL generation).' + ) + ); + } } catch (error) { spinner.fail('Failed to clone repository'); const err = error as { stderr?: Buffer }; @@ -502,6 +524,8 @@ function buildGitMetadata(sourceInfo: InstallSourceInfo, subpath: string): Skill repoUrl: sourceInfo.repoUrl, subpath, installedAt: new Date().toISOString(), + commitSha: sourceInfo.commitSha, + commitUrl: sourceInfo.commitUrl, }; } diff --git a/src/utils/skill-metadata.ts b/src/utils/skill-metadata.ts index bf720b3..0dcbbfa 100644 --- a/src/utils/skill-metadata.ts +++ b/src/utils/skill-metadata.ts @@ -12,6 +12,119 @@ export interface SkillSourceMetadata { subpath?: string; localPath?: string; installedAt: string; + commitSha?: string; + commitUrl?: string; +} + +const GIT_SUFFIX = '.git'; +const SSH_PREFIX = 'git@'; + +/** + * Per-host commit-URL builders. To support a new git host, add an entry + * here keyed by `URL.hostname`. Each builder receives the full `repoPath` + * (everything between the hostname and the optional `.git` suffix) plus + * the SHA, and returns a clickable `https://` URL — or undefined if the + * repoPath isn't valid for that host (e.g. wrong number of segments). + * + * Only github is registered today. To add another host, add one entry + * below with that host's commit-URL pattern. + */ +type CommitUrlBuilder = (repoPath: string, commitSha: string) => string | undefined; +const COMMIT_URL_BUILDERS: Record = { + 'github.com': (repoPath, commitSha) => { + // Github repos are always exactly `/` — no nested orgs or + // subpaths. Anything else is either malformed or not a clone URL. + const parts = repoPath.split('/'); + if (parts.length !== 2 || !parts[0] || !parts[1]) { + return undefined; + } + return `https://github.com/${repoPath}/commit/${commitSha}`; + }, +}; + +/** + * Build a browser-clickable commit URL from a clone-URL + SHA. + * Returns undefined when the host has no registered builder in + * COMMIT_URL_BUILDERS (or its builder rejects the repo path) so callers + * can omit the field. + */ +export function buildCommitUrl(repoUrl: string, commitSha: string): string | undefined { + const parsed = parseCloneUrl(repoUrl); + if (!parsed) { + // Couldn't parse the URL at all (malformed or non-clone string). + return undefined; + } + + const buildUrl = COMMIT_URL_BUILDERS[parsed.host]; + if (!buildUrl) { + // No builder registered for this host — we don't know its commit-URL pattern. + return undefined; + } + + return buildUrl(parsed.repoPath, commitSha); +} + +/** + * Parse a clone URL into host + repoPath. Returns null when parsing fails. + */ +function parseCloneUrl(repoUrl: string): { host: string; repoPath: string } | null { + const parsed = sshToHttps(repoUrl.trim()); + if (!parsed) { + // Either malformed SSH or `new URL()` rejected the input. + return null; + } + + // `pathname` always starts with '/'. Strip the leading slash to get the + // repo path (e.g. `anthropics/skills` or `anthropics/skills.git`). + let repoPath = parsed.pathname.slice(1); + if (repoPath.endsWith(GIT_SUFFIX)) { + // Trailing `.git` is conventional in clone URLs but isn't part of the + // repo's identity — strip it so builders get a clean path. + repoPath = repoPath.slice(0, -GIT_SUFFIX.length); + } + + return { host: parsed.hostname, repoPath }; +} + +/** + * Convert any clone URL into a parsed `URL` object, rewriting SSH form + * (`git@host:owner/repo[.git]`) to HTTPS first since SSH isn't a valid URI. + * Non-SSH inputs are handed straight to `new URL()`. + * + * Returns null when the input can't be parsed: malformed SSH (starts with + * `git@` but missing the ':' separator), or anything `new URL()` rejects + * (bare local paths, free-form strings, unsupported schemes). + * + * Case of host and path is preserved during the SSH rewrite — `URL()` will + * lowercase the host on parse, and path is case-sensitive on github + * (Anthropics ≠ anthropics), so we hand the path through untouched. + */ +function sshToHttps(input: string): URL | null { + let candidate = input; + + // Case-insensitive prefix check so weird-cased inputs like `Git@host:...` + // are still recognized as SSH. + const isSshForm = input.slice(0, SSH_PREFIX.length).toLowerCase() === SSH_PREFIX; + if (isSshForm) { + // Convert "git@host:owner/repo" → "https://host/owner/repo" by swapping + // the first ':' for '/' and prepending the scheme. + const afterPrefix = input.slice(SSH_PREFIX.length); + const colonIdx = afterPrefix.indexOf(':'); + if (colonIdx === -1) { + // SSH-shaped input missing the ':' separator — can't be rewritten. + return null; + } + const host = afterPrefix.slice(0, colonIdx); + const path = afterPrefix.slice(colonIdx + 1); + candidate = `https://${host}/${path}`; + } + + try { + return new URL(candidate); + } catch { + // URL constructor rejected the candidate (bare local path, garbage, etc). + return null; + } } export function readSkillMetadata(skillDir: string): SkillSourceMetadata | null { diff --git a/tests/integration/e2e.test.ts b/tests/integration/e2e.test.ts index b4d328d..27d0cd9 100644 --- a/tests/integration/e2e.test.ts +++ b/tests/integration/e2e.test.ts @@ -149,44 +149,6 @@ describe('End-to-end CLI tests', () => { }); }); - describe('openskills install (local paths)', () => { - it('should install from absolute local path', () => { - // Create a source skill - const sourceDir = join(testTempDir, 'source-skills'); - createTestSkill(sourceDir, 'local-skill', 'Local skill'); - - // Install to project - const result = runCli(`install ${join(sourceDir, 'local-skill')} -y`); - - expect(result.exitCode).toBe(0); - expect(result.stdout).toContain('Installed'); - - // Verify skill was copied - const installedPath = join(testTempDir, '.claude', 'skills', 'local-skill', 'SKILL.md'); - expect(existsSync(installedPath)).toBe(true); - }); - - it('should install directory of skills from local path', () => { - // Create multiple source skills - const sourceDir = join(testTempDir, 'multi-skills'); - createTestSkill(sourceDir, 'skill-one', 'First skill'); - createTestSkill(sourceDir, 'skill-two', 'Second skill'); - - const result = runCli(`install ${sourceDir} -y`); - - expect(result.exitCode).toBe(0); - expect(result.stdout).toContain('skill-one'); - expect(result.stdout).toContain('skill-two'); - }); - - it('should error for non-existent local path', () => { - const result = runCli(`install /non/existent/path -y`); - - expect(result.exitCode).toBe(1); - expect(result.stderr).toContain('does not exist'); - }); - }); - describe('openskills remove', () => { it('should remove installed skill', () => { const skillsDir = join(testTempDir, '.claude', 'skills'); diff --git a/tests/integration/install.test.ts b/tests/integration/install.test.ts new file mode 100644 index 0000000..2c97b0b --- /dev/null +++ b/tests/integration/install.test.ts @@ -0,0 +1,141 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { mkdirSync, writeFileSync, readFileSync, rmSync, existsSync } from 'fs'; +import { join } from 'path'; +import { tmpdir } from 'os'; +import { execSync } from 'child_process'; +import { buildCommitUrl } from '../../src/utils/skill-metadata.js'; + +const testId = Math.random().toString(36).slice(2); +const testTempDir = join(tmpdir(), `openskills-install-${testId}`); +const cliPath = join(process.cwd(), 'dist', 'cli.js'); + +function runCli(args: string, cwd?: string): { stdout: string; stderr: string; exitCode: number } { + try { + const stdout = execSync(`node ${cliPath} ${args}`, { + cwd: cwd || testTempDir, + encoding: 'utf-8', + stdio: ['pipe', 'pipe', 'pipe'], + }); + return { stdout, stderr: '', exitCode: 0 }; + } catch (error: unknown) { + const err = error as { stdout?: string; stderr?: string; status?: number }; + return { + stdout: err.stdout || '', + stderr: err.stderr || '', + exitCode: err.status || 1, + }; + } +} + +function createTestSkill(dir: string, name: string, description: string = 'Test skill'): void { + const skillDir = join(dir, name); + mkdirSync(skillDir, { recursive: true }); + writeFileSync( + join(skillDir, 'SKILL.md'), + `--- +name: ${name} +description: ${description} +--- + +# ${name} + +Instructions for ${name}. +` + ); +} + +describe('openskills install', () => { + beforeEach(() => { + mkdirSync(testTempDir, { recursive: true }); + }); + + afterEach(() => { + rmSync(testTempDir, { recursive: true, force: true }); + }); + + describe('local paths', () => { + it('should install from absolute local path', () => { + const sourceDir = join(testTempDir, 'source-skills'); + createTestSkill(sourceDir, 'local-skill', 'Local skill'); + + const result = runCli(`install ${join(sourceDir, 'local-skill')} -y`); + + expect(result.exitCode).toBe(0); + expect(result.stdout).toContain('Installed'); + + const installedPath = join(testTempDir, '.claude', 'skills', 'local-skill', 'SKILL.md'); + expect(existsSync(installedPath)).toBe(true); + }); + + it('should install directory of skills from local path', () => { + const sourceDir = join(testTempDir, 'multi-skills'); + createTestSkill(sourceDir, 'skill-one', 'First skill'); + createTestSkill(sourceDir, 'skill-two', 'Second skill'); + + const result = runCli(`install ${sourceDir} -y`); + + expect(result.exitCode).toBe(0); + expect(result.stdout).toContain('skill-one'); + expect(result.stdout).toContain('skill-two'); + }); + + it('should error for non-existent local path', () => { + const result = runCli(`install /non/existent/path -y`); + + expect(result.exitCode).toBe(1); + expect(result.stderr).toContain('does not exist'); + }); + }); + + describe('git install records commitSha + commitUrl (real github.com)', () => { + it('records 40-char hex commitSha and matching commitUrl for a github skill', () => { + const result = runCli(`install anthropics/skills/skills/pdf -y`); + + expect(result.exitCode).toBe(0); + + const metadataPath = join(testTempDir, '.claude', 'skills', 'pdf', '.openskills.json'); + expect(existsSync(metadataPath)).toBe(true); + + const metadata = JSON.parse(readFileSync(metadataPath, 'utf-8')); + expect(metadata.sourceType).toBe('git'); + expect(metadata.commitSha).toMatch(/^[0-9a-f]{40}$/); + expect(metadata.commitUrl).toBe( + `https://github.com/anthropics/skills/commit/${metadata.commitSha}` + ); + }); + }); + + describe('buildCommitUrl normalization (pure helper)', () => { + const sha = 'f458cee31a7577a47ba0c9a101976fa599385174'; + const expectedUrl = `https://github.com/anthropics/skills/commit/${sha}`; + + it('passes through plain HTTPS', () => { + expect(buildCommitUrl('https://github.com/anthropics/skills', sha)).toBe(expectedUrl); + }); + + it('strips trailing .git from HTTPS', () => { + expect(buildCommitUrl('https://github.com/anthropics/skills.git', sha)).toBe(expectedUrl); + }); + + it('rewrites SSH form to HTTPS and strips .git', () => { + expect(buildCommitUrl('git@github.com:anthropics/skills.git', sha)).toBe(expectedUrl); + }); + + it('recognizes SSH form regardless of git@ prefix casing', () => { + // Rare in practice but harmless to support — pasted/munged URLs sometimes + // arrive with weird casing. + expect(buildCommitUrl('Git@github.com:anthropics/skills.git', sha)).toBe(expectedUrl); + expect(buildCommitUrl('GIT@github.com:anthropics/skills.git', sha)).toBe(expectedUrl); + }); + + it('rewrites git:// scheme to https and strips .git', () => { + expect(buildCommitUrl('git://github.com/anthropics/skills.git', sha)).toBe(expectedUrl); + }); + + it('returns undefined for non-github hosts (e.g. gitlab.com is not yet supported)', () => { + // Real gitlab skills repo — picked deliberately so this test stays + // meaningful if/when someone adds gitlab support and needs to update it. + expect(buildCommitUrl('https://gitlab.com/gitlab-org/ai/skills', sha)).toBeUndefined(); + }); + }); +});