diff --git a/src/cli/commands/add.ts b/src/cli/commands/add.ts index ebb8c70..92f93b1 100644 --- a/src/cli/commands/add.ts +++ b/src/cli/commands/add.ts @@ -34,12 +34,18 @@ export async function runAdd(opts: AddOptions): Promise { // Load config early so we can check trust before any network work const config = await loadConfig(configPath); - // Validate trust before resolution - validateTrustedSource(specifier, config.trust); - // Parse the specifier const parsed = parseSource(specifier); + // Normalize GitHub URLs to owner/repo form for storage + const canonicalSource = + parsed.type === "github" + ? `${parsed.owner}/${parsed.repo}${parsed.ref ? `@${parsed.ref}` : ""}` + : specifier; + + // Validate trust against the canonical source (owner/repo form for GitHub) + validateTrustedSource(canonicalSource, config.trust); + // Determine ref (flag overrides inline @ref) const effectiveRef = ref ?? parsed.ref; @@ -49,13 +55,13 @@ export async function runAdd(opts: AddOptions): Promise { throw new AddError("Cannot use --all with --name. Use one or the other."); } - if (config.skills.some((s) => isWildcardDep(s) && s.source === specifier)) { + if (config.skills.some((s) => isWildcardDep(s) && s.source === canonicalSource)) { throw new AddError( - `A wildcard entry for "${specifier}" already exists in agents.toml.`, + `A wildcard entry for "${canonicalSource}" already exists in agents.toml.`, ); } - await addWildcardToConfig(configPath, specifier, { + await addWildcardToConfig(configPath, canonicalSource, { ...(effectiveRef ? { ref: effectiveRef } : {}), exclude: [], }); @@ -96,8 +102,8 @@ export async function runAdd(opts: AddOptions): Promise { const found = await discoverSkill(cached.repoDir, nameOverride); if (!found) { throw new AddError( - `Skill "${nameOverride}" not found in ${specifier}. ` + - `Use 'dotagents add ${specifier}' without --name to see available skills.`, + `Skill "${nameOverride}" not found in ${canonicalSource}. ` + + `Use 'dotagents add ${canonicalSource}' without --name to see available skills.`, ); } skillName = nameOverride; @@ -105,7 +111,7 @@ export async function runAdd(opts: AddOptions): Promise { // Discover all skills and pick const skills = await discoverAllSkills(cached.repoDir); if (skills.length === 0) { - throw new AddError(`No skills found in ${specifier}.`); + throw new AddError(`No skills found in ${canonicalSource}.`); } if (skills.length === 1) { skillName = skills[0]!.meta.name; @@ -113,7 +119,7 @@ export async function runAdd(opts: AddOptions): Promise { // Multiple skills found — list them and ask user to pick with --name or --all const names = skills.map((s) => s.meta.name).sort(); throw new AddError( - `Multiple skills found in ${specifier}: ${names.join(", ")}. ` + + `Multiple skills found in ${canonicalSource}: ${names.join(", ")}. ` + `Use --name to specify which one, or --all for all skills.`, ); } @@ -129,7 +135,7 @@ export async function runAdd(opts: AddOptions): Promise { // Add to config await addSkillToConfig(configPath, skillName, { - source: specifier, + source: canonicalSource, ...(effectiveRef ? { ref: effectiveRef } : {}), }); diff --git a/src/config/schema.test.ts b/src/config/schema.test.ts index 3a37de4..633689b 100644 --- a/src/config/schema.test.ts +++ b/src/config/schema.test.ts @@ -131,6 +131,22 @@ describe("agentsConfigSchema", () => { it("rejects three-part path (not a valid format)", () => { expect(parseSkill("a/b/c").success).toBe(false); }); + + it("accepts GitHub HTTPS URL", () => { + expect(parseSkill("https://github.com/owner/repo").success).toBe(true); + }); + + it("accepts GitHub HTTPS URL with .git suffix", () => { + expect(parseSkill("https://github.com/owner/repo.git").success).toBe(true); + }); + + it("accepts GitHub SSH URL", () => { + expect(parseSkill("git@github.com:owner/repo.git").success).toBe(true); + }); + + it("rejects GitHub URL with dash-prefixed owner", () => { + expect(parseSkill("https://github.com/-bad/repo").success).toBe(false); + }); }); describe("skill name validation", () => { diff --git a/src/config/schema.ts b/src/config/schema.ts index 4c14646..4235ae4 100644 --- a/src/config/schema.ts +++ b/src/config/schema.ts @@ -9,6 +9,13 @@ import { z } from "zod/v4"; */ const GIT_URL_VALID = /^git:(https:\/\/|git:\/\/|ssh:\/\/|git@|file:\/\/|\/)/; +/** GitHub HTTPS URL pattern — owner/repo must start with alphanumeric (no dash prefix). */ +export const GITHUB_HTTPS_URL = + /^https?:\/\/github\.com\/([a-zA-Z0-9][^/]*)\/([a-zA-Z0-9][^/@]*?)(?:\.git)?(?:\/)?(?:@(.+))?$/; +/** GitHub SSH URL pattern — owner/repo must start with alphanumeric (no dash prefix). */ +export const GITHUB_SSH_URL = + /^git@github\.com:([a-zA-Z0-9][^/]*)\/([a-zA-Z0-9][^/@]*?)(?:\.git)?(?:@(.+))?$/; + const skillSourceSchema = z.string().check( z.refine((s) => { if (s.startsWith("git:")) { @@ -16,11 +23,14 @@ const skillSourceSchema = z.string().check( return GIT_URL_VALID.test(s); } if (s.startsWith("path:")) return true; + // GitHub HTTPS or SSH URLs + if (GITHUB_HTTPS_URL.test(s)) return true; + if (GITHUB_SSH_URL.test(s)) return true; // owner/repo or owner/repo@ref const base = s.includes("@") ? s.slice(0, s.indexOf("@")) : s; const parts = base.split("/"); return parts.length === 2 && parts.every((p) => p.length > 0 && !p.startsWith("-")); - }, "Must be owner/repo, owner/repo@ref, git: (with https/git/ssh protocol), or path:"), + }, "Must be owner/repo, owner/repo@ref, GitHub URL, git: (with https/git/ssh protocol), or path:"), ); export type SkillSource = z.infer; diff --git a/src/skills/resolver.test.ts b/src/skills/resolver.test.ts index 739a544..7cd5ba4 100644 --- a/src/skills/resolver.test.ts +++ b/src/skills/resolver.test.ts @@ -39,4 +39,79 @@ describe("parseSource", () => { expect(result.type).toBe("local"); expect(result.path).toBe("../shared/my-skill"); }); + + it("parses HTTPS GitHub URL", () => { + const result = parseSource("https://github.com/getsentry/skills"); + expect(result.type).toBe("github"); + expect(result.owner).toBe("getsentry"); + expect(result.repo).toBe("skills"); + expect(result.url).toBe("https://github.com/getsentry/skills.git"); + expect(result.ref).toBeUndefined(); + }); + + it("parses HTTPS GitHub URL with .git suffix", () => { + const result = parseSource("https://github.com/getsentry/skills.git"); + expect(result.type).toBe("github"); + expect(result.owner).toBe("getsentry"); + expect(result.repo).toBe("skills"); + expect(result.url).toBe("https://github.com/getsentry/skills.git"); + }); + + it("parses HTTPS GitHub URL with trailing slash", () => { + const result = parseSource("https://github.com/getsentry/skills/"); + expect(result.type).toBe("github"); + expect(result.owner).toBe("getsentry"); + expect(result.repo).toBe("skills"); + }); + + it("parses HTTPS GitHub URL with @ref", () => { + const result = parseSource("https://github.com/getsentry/skills@v1.0.0"); + expect(result.type).toBe("github"); + expect(result.owner).toBe("getsentry"); + expect(result.repo).toBe("skills"); + expect(result.ref).toBe("v1.0.0"); + expect(result.url).toBe("https://github.com/getsentry/skills.git"); + }); + + it("parses SSH GitHub URL", () => { + const result = parseSource("git@github.com:getsentry/skills"); + expect(result.type).toBe("github"); + expect(result.owner).toBe("getsentry"); + expect(result.repo).toBe("skills"); + expect(result.url).toBe("https://github.com/getsentry/skills.git"); + expect(result.ref).toBeUndefined(); + }); + + it("parses SSH GitHub URL with .git suffix", () => { + const result = parseSource("git@github.com:getsentry/skills.git"); + expect(result.type).toBe("github"); + expect(result.owner).toBe("getsentry"); + expect(result.repo).toBe("skills"); + expect(result.url).toBe("https://github.com/getsentry/skills.git"); + }); + + it("parses SSH GitHub URL with @ref", () => { + const result = parseSource("git@github.com:getsentry/skills@v2.0"); + expect(result.type).toBe("github"); + expect(result.owner).toBe("getsentry"); + expect(result.repo).toBe("skills"); + expect(result.ref).toBe("v2.0"); + expect(result.url).toBe("https://github.com/getsentry/skills.git"); + }); + + it("parses HTTPS GitHub URL with dotted repo name", () => { + const result = parseSource("https://github.com/vercel/next.js"); + expect(result.type).toBe("github"); + expect(result.owner).toBe("vercel"); + expect(result.repo).toBe("next.js"); + expect(result.url).toBe("https://github.com/vercel/next.js.git"); + }); + + it("parses HTTPS GitHub URL with dotted repo name and .git suffix", () => { + const result = parseSource("https://github.com/vercel/next.js.git"); + expect(result.type).toBe("github"); + expect(result.owner).toBe("vercel"); + expect(result.repo).toBe("next.js"); + expect(result.url).toBe("https://github.com/vercel/next.js.git"); + }); }); diff --git a/src/skills/resolver.ts b/src/skills/resolver.ts index 3d63374..73353e0 100644 --- a/src/skills/resolver.ts +++ b/src/skills/resolver.ts @@ -1,5 +1,6 @@ import { join } from "node:path"; import type { WildcardSkillDependency } from "../config/schema.js"; +import { GITHUB_HTTPS_URL, GITHUB_SSH_URL } from "../config/schema.js"; import { ensureCached } from "../sources/cache.js"; import { resolveLocalSource } from "../sources/local.js"; import { discoverSkill, discoverAllSkills } from "./discovery.js"; @@ -56,6 +57,20 @@ export function parseSource(source: string): { return { type: "git", url: source.slice(4) }; } + // GitHub HTTPS or SSH URL + const githubUrlMatch = + source.match(GITHUB_HTTPS_URL) || source.match(GITHUB_SSH_URL); + if (githubUrlMatch) { + const [, owner, repo, ref] = githubUrlMatch; + return { + type: "github", + owner, + repo, + ref, + url: `https://github.com/${owner}/${repo}.git`, + }; + } + // owner/repo or owner/repo@ref const atIdx = source.indexOf("@"); const base = atIdx !== -1 ? source.slice(0, atIdx) : source;