diff --git a/packages/views/locales/en/skills.json b/packages/views/locales/en/skills.json index 1526c330f..f0968d22e 100644 --- a/packages/views/locales/en/skills.json +++ b/packages/views/locales/en/skills.json @@ -48,7 +48,8 @@ "source_runtime_provider": "From {{provider}} runtime", "source_runtime_unknown": "From a runtime", "source_clawhub": "From ClawHub", - "source_skills_sh": "From Skills.sh" + "source_skills_sh": "From Skills.sh", + "source_github": "From GitHub" }, "detail": { "all_skills": "All skills", @@ -70,6 +71,7 @@ "origin_runtime_unknown": "Local runtime", "origin_clawhub": "Imported · ClawHub", "origin_skills_sh": "Imported · Skills.sh", + "origin_github": "Imported · GitHub", "origin_workspace": "Workspace", "updated_label": "Updated {{when}}", "by_creator": "by {{name}}" @@ -104,6 +106,7 @@ "imported_runtime": "Imported from local runtime", "imported_clawhub": "Imported from ClawHub", "imported_skills_sh": "Imported from Skills.sh", + "imported_github": "Imported from GitHub", "provider": "provider · {{provider}}" }, "not_found": { @@ -189,6 +192,7 @@ "importing": "Importing…", "importing_clawhub": "Importing from ClawHub…", "importing_skills_sh": "Importing from Skills.sh…", + "importing_github": "Importing from GitHub…", "name_conflict_hint": " The imported skill's name already exists — delete the existing one before retrying.", "fallback_error": "Import failed", "cancel": "Cancel", diff --git a/packages/views/locales/zh-Hans/skills.json b/packages/views/locales/zh-Hans/skills.json index ede9b461c..906943bfe 100644 --- a/packages/views/locales/zh-Hans/skills.json +++ b/packages/views/locales/zh-Hans/skills.json @@ -60,7 +60,8 @@ "source_runtime_provider": "来自 {{provider}} 运行时", "source_runtime_unknown": "来自某个运行时", "source_clawhub": "来自 ClawHub", - "source_skills_sh": "来自 Skills.sh" + "source_skills_sh": "来自 Skills.sh", + "source_github": "来自 GitHub" }, "detail": { "all_skills": "全部 skill", @@ -82,6 +83,7 @@ "origin_runtime_unknown": "本地运行时", "origin_clawhub": "导入自 · ClawHub", "origin_skills_sh": "导入自 · Skills.sh", + "origin_github": "导入自 · GitHub", "origin_workspace": "工作区", "updated_label": "{{when}}更新", "by_creator": "由 {{name}}" @@ -116,6 +118,7 @@ "imported_runtime": "从本地运行时导入", "imported_clawhub": "从 ClawHub 导入", "imported_skills_sh": "从 Skills.sh 导入", + "imported_github": "从 GitHub 导入", "provider": "provider · {{provider}}" }, "not_found": { @@ -201,6 +204,7 @@ "importing": "导入中...", "importing_clawhub": "正在从 ClawHub 导入...", "importing_skills_sh": "正在从 Skills.sh 导入...", + "importing_github": "正在从 GitHub 导入...", "name_conflict_hint": "导入的 skill 名称已存在——请先删除已有的再重试。", "fallback_error": "导入失败", "cancel": "取消", diff --git a/packages/views/skills/components/create-skill-dialog.tsx b/packages/views/skills/components/create-skill-dialog.tsx index 2c9963725..2a4e15d1c 100644 --- a/packages/views/skills/components/create-skill-dialog.tsx +++ b/packages/views/skills/components/create-skill-dialog.tsx @@ -241,12 +241,13 @@ function ManualForm({ // URL import form // --------------------------------------------------------------------------- -type DetectedSource = "clawhub" | "skills.sh" | null; +type DetectedSource = "clawhub" | "skills.sh" | "github" | null; function detectUrlSource(url: string): DetectedSource { const u = url.trim().toLowerCase(); if (u.includes("clawhub.ai")) return "clawhub"; if (u.includes("skills.sh")) return "skills.sh"; + if (u.includes("github.com")) return "github"; return null; } @@ -316,6 +317,7 @@ function UrlForm({ if (!loading) return t(($) => $.create.url.import); if (source === "clawhub") return t(($) => $.create.url.importing_clawhub); if (source === "skills.sh") return t(($) => $.create.url.importing_skills_sh); + if (source === "github") return t(($) => $.create.url.importing_github); return t(($) => $.create.url.importing); })(); @@ -350,7 +352,7 @@ function UrlForm({

{t(($) => $.create.url.supported_sources)}

-
+
+
diff --git a/packages/views/skills/components/skill-columns.tsx b/packages/views/skills/components/skill-columns.tsx index 021b0465c..a62f8704c 100644 --- a/packages/views/skills/components/skill-columns.tsx +++ b/packages/views/skills/components/skill-columns.tsx @@ -205,6 +205,9 @@ function SourceCell({ } else if (origin.type === "skills_sh") { icon = ; label = t(($) => $.table.source_skills_sh); + } else if (origin.type === "github") { + icon = ; + label = t(($) => $.table.source_github); } return ( diff --git a/packages/views/skills/components/skill-detail-page.tsx b/packages/views/skills/components/skill-detail-page.tsx index d6070586f..173b493c9 100644 --- a/packages/views/skills/components/skill-detail-page.tsx +++ b/packages/views/skills/components/skill-detail-page.tsx @@ -199,7 +199,9 @@ function OriginSidebarCard({ ? t(($) => $.detail.origin_card.imported_runtime) : origin.type === "clawhub" ? t(($) => $.detail.origin_card.imported_clawhub) - : t(($) => $.detail.origin_card.imported_skills_sh); + : origin.type === "github" + ? t(($) => $.detail.origin_card.imported_github) + : t(($) => $.detail.origin_card.imported_skills_sh); return (
@@ -542,6 +544,7 @@ export function SkillDetailPage({ skillId }: { skillId: string }) { } if (origin.type === "clawhub") return t(($) => $.detail.subline.origin_clawhub); if (origin.type === "skills_sh") return t(($) => $.detail.subline.origin_skills_sh); + if (origin.type === "github") return t(($) => $.detail.subline.origin_github); return t(($) => $.detail.subline.origin_workspace); })(); diff --git a/packages/views/skills/lib/origin.ts b/packages/views/skills/lib/origin.ts index 3b171a124..46ca49b2b 100644 --- a/packages/views/skills/lib/origin.ts +++ b/packages/views/skills/lib/origin.ts @@ -3,16 +3,11 @@ import type { Skill, SkillSummary } from "@multica/core/types"; /** * Discriminated view over `Skill.config.origin` — the JSONB blob the backend * writes when a skill was imported from outside (local runtime, ClawHub, - * Skills.sh). Manual creates have no origin, so we synthesize `{ type: - * "manual" }` for them to keep the consumer code uniform. - * - * NOTE: the backend currently only writes `runtime_local` origins. URL - * imports leave `config.origin` empty, so `clawhub`/`skills_sh` variants are - * declared here for forward compatibility but should never be rendered in - * the UI until the server fills them in. + * Skills.sh, GitHub). Manual creates have no origin, so we synthesize + * `{ type: "manual" }` for them to keep the consumer code uniform. */ export type OriginInfo = { - type: "runtime_local" | "clawhub" | "skills_sh" | "manual"; + type: "runtime_local" | "clawhub" | "skills_sh" | "github" | "manual"; provider?: string; runtime_id?: string; source_path?: string; @@ -26,6 +21,7 @@ export function readOrigin(skill: SkillSummary): OriginInfo { if (raw?.type === "runtime_local") return raw; if (raw?.type === "clawhub") return raw; if (raw?.type === "skills_sh") return raw; + if (raw?.type === "github") return raw; return { type: "manual" }; } diff --git a/server/cmd/multica/cmd_skill.go b/server/cmd/multica/cmd_skill.go index 7734b9eb3..1e30e1138 100644 --- a/server/cmd/multica/cmd_skill.go +++ b/server/cmd/multica/cmd_skill.go @@ -54,7 +54,7 @@ var skillDeleteCmd = &cobra.Command{ var skillImportCmd = &cobra.Command{ Use: "import", - Short: "Import a skill from a URL (clawhub.ai or skills.sh)", + Short: "Import a skill from a URL (clawhub.ai, skills.sh, or github.com)", RunE: runSkillImport, } diff --git a/server/internal/handler/skill.go b/server/internal/handler/skill.go index dfba8fa4c..ee32f6c47 100644 --- a/server/internal/handler/skill.go +++ b/server/internal/handler/skill.go @@ -2,6 +2,7 @@ package handler import ( "encoding/json" + "errors" "fmt" "io" "log/slog" @@ -441,12 +442,24 @@ type ImportSkillRequest struct { URL string `json:"url"` } +// Per-import bundle limits. These mirror the local-runtime importer so that +// URL imports cannot smuggle in payloads that the rest of the stack would +// reject. fetchRawFile enforces the per-file cap; importedSkill.addFile +// enforces the bundle-wide caps. +const ( + maxImportFileSize = 1 << 20 // 1 MiB per file + maxImportTotalSize = 8 << 20 // 8 MiB per import bundle (sum of supporting files) + maxImportFileCount = 128 // max number of supporting files +) + // importedSkill holds the data extracted from an external source. type importedSkill struct { name string description string content string // SKILL.md body files []importedFile + bundleSize int // running sum of file content bytes for cap enforcement + origin map[string]any // written into skill.config.origin so the UI can show provenance } type importedFile struct { @@ -454,6 +467,31 @@ type importedFile struct { content string } +// errImportCapExceeded marks an error caused by a per-file or per-bundle cap. +// Such errors must abort the import — silently dropping a file would otherwise +// produce an incomplete skill that looks valid to the user. +var errImportCapExceeded = errors.New("import cap exceeded") + +// isCapError reports whether err is (or wraps) errImportCapExceeded. +func isCapError(err error) bool { + return errors.Is(err, errImportCapExceeded) +} + +// addFile appends a supporting file while enforcing the per-bundle caps. It +// returns an error when either the file count or aggregate byte budget would +// be exceeded so the caller fails the import instead of silently truncating. +func (s *importedSkill) addFile(path, content string) error { + if len(s.files) >= maxImportFileCount { + return fmt.Errorf("%w: import bundle exceeds %d file limit", errImportCapExceeded, maxImportFileCount) + } + if s.bundleSize+len(content) > maxImportTotalSize { + return fmt.Errorf("%w: import bundle exceeds %d byte limit", errImportCapExceeded, maxImportTotalSize) + } + s.bundleSize += len(content) + s.files = append(s.files, importedFile{path: path, content: content}) + return nil +} + // --- ClawHub types --- type clawhubGetSkillResponse struct { @@ -539,6 +577,7 @@ type importSource int const ( sourceClawHub importSource = iota sourceSkillsSh + sourceGitHub ) // detectImportSource determines the source from a URL. @@ -565,12 +604,14 @@ func detectImportSource(raw string) (importSource, string, error) { return sourceSkillsSh, normalized, nil case host == "clawhub.ai" || host == "www.clawhub.ai": return sourceClawHub, normalized, nil + case host == "github.com" || host == "www.github.com": + return sourceGitHub, normalized, nil default: // If no host (bare slug), default to clawhub if !strings.Contains(raw, "/") || !strings.Contains(raw, ".") { return sourceClawHub, raw, nil } - return 0, "", fmt.Errorf("unsupported source: %s (supported: clawhub.ai, skills.sh)", host) + return 0, "", fmt.Errorf("unsupported source: %s (supported: clawhub.ai, skills.sh, github.com)", host) } } @@ -654,6 +695,11 @@ func fetchFromClawHub(httpClient *http.Client, rawURL string) (*importedSkill, e result := &importedSkill{ name: chSkill.DisplayName, description: chSkill.Summary, + origin: map[string]any{ + "type": "clawhub", + "source_url": rawURL, + "slug": slug, + }, } if result.name == "" { result.name = slug @@ -666,14 +712,26 @@ func fetchFromClawHub(httpClient *http.Client, rawURL string) (*importedSkill, e } body, err := fetchRawFile(httpClient, fileURL) if err != nil { + // Cap violations must abort: silently dropping a file would + // produce an incomplete bundle that looks valid. SKILL.md is + // load-bearing, so any failure on it is fatal too. + if isCapError(err) || fp == "SKILL.md" { + return nil, fmt.Errorf("clawhub import: %s: %w", fp, err) + } slog.Warn("clawhub import: file download failed", "path", fp, "error", err) continue } if fp == "SKILL.md" { result.content = string(body) - } else { - result.files = append(result.files, importedFile{path: fp, content: string(body)}) + continue } + if err := result.addFile(fp, string(body)); err != nil { + return nil, err + } + } + + if result.content == "" { + return nil, fmt.Errorf("clawhub import: SKILL.md is empty or missing for %s", slug) } return result, nil @@ -759,6 +817,13 @@ func fetchFromSkillsSh(httpClient *http.Client, rawURL string) (*importedSkill, name: name, description: description, content: string(skillMdBody), + origin: map[string]any{ + "type": "skills_sh", + "source_url": rawURL, + "owner": owner, + "repo": repo, + "skill": skillName, + }, } // 2. List supporting files via GitHub API @@ -775,15 +840,15 @@ func fetchFromSkillsSh(httpClient *http.Client, rawURL string) (*importedSkill, var entries []githubContentEntry if err := json.NewDecoder(dirResp.Body).Decode(&entries); err != nil { - slog.Warn("skills.sh import: failed to decode top-level directory listing", "url", apiURL, "error", err) + slog.Warn("github import: failed to decode top-level directory listing", "url", apiURL, "error", err) return result, nil } // 3. Recursively collect files (excluding SKILL.md and LICENSE) var allFiles []githubContentEntry - slog.Info("skills.sh import: collecting supporting files", "skill", skillName, "top_level_entries", len(entries)) + slog.Info("github import: collecting supporting files", "skill", skillName, "top_level_entries", len(entries)) collectGitHubFiles(httpClient, entries, &allFiles, apiURL) - slog.Info("skills.sh import: collected supporting files", "skill", skillName, "files", len(allFiles)) + slog.Info("github import: collected supporting files", "skill", skillName, "files", len(allFiles)) // 4. Download each file basePath := "" @@ -796,12 +861,17 @@ func fetchFromSkillsSh(httpClient *http.Client, rawURL string) (*importedSkill, } body, err := fetchRawFile(httpClient, entry.DownloadURL) if err != nil { - slog.Warn("skills.sh import: file download failed", "path", entry.Path, "error", err) + if isCapError(err) { + return nil, fmt.Errorf("github import: %s: %w", entry.Path, err) + } + slog.Warn("github import: file download failed", "path", entry.Path, "error", err) continue } // Convert absolute GitHub path to relative path within skill relPath := strings.TrimPrefix(entry.Path, basePath) - result.files = append(result.files, importedFile{path: relPath, content: string(body)}) + if err := result.addFile(relPath, string(body)); err != nil { + return nil, err + } } return result, nil @@ -836,7 +906,7 @@ func resolveGitHubSkillDirByName(httpClient *http.Client, owner, repo, defaultBr return "", nil, skillMdNotFoundError(owner, repo, skillName) } - slog.Warn("skills.sh import: repository tree listing truncated", "owner", owner, "repo", repo, "branch", defaultBranch) + slog.Warn("github import: repository tree listing truncated", "owner", owner, "repo", repo, "branch", defaultBranch) if dir, body, ok := findSkillDirFromConventionalPrefixes(httpClient, owner, repo, defaultBranch, rawPrefix, skillName); ok { return dir, body, nil } @@ -858,7 +928,7 @@ func collectGitHubFiles(httpClient *http.Client, entries []githubContentEntry, o if subURL == "" { parsed, err := url.Parse(parentURL) if err != nil { - slog.Warn("skills.sh import: invalid parent directory url", "url", parentURL, "error", err) + slog.Warn("github import: invalid parent directory url", "url", parentURL, "error", err) continue } parsed.Path = strings.TrimSuffix(parsed.Path, "/") + "/" + entry.Name @@ -874,13 +944,13 @@ func collectGitHubFiles(httpClient *http.Client, entries []githubContentEntry, o if err != nil { attrs = append(attrs, "error", err) } - slog.Warn("skills.sh import: failed to list subdirectory", attrs...) + slog.Warn("github import: failed to list subdirectory", attrs...) continue } var subEntries []githubContentEntry if err := json.NewDecoder(subResp.Body).Decode(&subEntries); err != nil { subResp.Body.Close() - slog.Warn("skills.sh import: failed to decode subdirectory listing", "url", subURL, "error", err) + slog.Warn("github import: failed to decode subdirectory listing", "url", subURL, "error", err) continue } subResp.Body.Close() @@ -895,7 +965,7 @@ func findSkillDirFromConventionalPrefixes(httpClient *http.Client, owner, repo, for _, prefix := range prefixes { paths, err := listGitHubSkillMdPaths(httpClient, owner, repo, prefix, defaultBranch) if err != nil { - slog.Warn("skills.sh import: failed to list conventional skill prefix", "prefix", prefix, "error", err) + slog.Warn("github import: failed to list conventional skill prefix", "prefix", prefix, "error", err) continue } skillPaths = append(skillPaths, paths...) @@ -949,7 +1019,7 @@ func collectGitHubSkillMdPaths(httpClient *http.Client, entries []githubContentE if subURL == "" { parsed, err := url.Parse(parentURL) if err != nil { - slog.Warn("skills.sh import: invalid parent directory url", "url", parentURL, "error", err) + slog.Warn("github import: invalid parent directory url", "url", parentURL, "error", err) continue } parsed.Path = strings.TrimSuffix(parsed.Path, "/") + "/" + entry.Name @@ -966,14 +1036,14 @@ func collectGitHubSkillMdPaths(httpClient *http.Client, entries []githubContentE if err != nil { attrs = append(attrs, "error", err) } - slog.Warn("skills.sh import: failed to list skill metadata subdirectory", attrs...) + slog.Warn("github import: failed to list skill metadata subdirectory", attrs...) continue } var subEntries []githubContentEntry if err := json.NewDecoder(subResp.Body).Decode(&subEntries); err != nil { subResp.Body.Close() - slog.Warn("skills.sh import: failed to decode skill metadata subdirectory", "url", subURL, "error", err) + slog.Warn("github import: failed to decode skill metadata subdirectory", "url", subURL, "error", err) continue } subResp.Body.Close() @@ -1007,7 +1077,7 @@ func findMatchingSkillDirByFrontmatter(httpClient *http.Client, rawPrefix, skill for _, skillPath := range skillPaths { body, err := fetchRawFile(httpClient, buildRawGitHubURL(rawPrefix, skillPath)) if err != nil { - slog.Warn("skills.sh import: fallback SKILL.md fetch failed", "path", skillPath, "error", err) + slog.Warn("github import: fallback SKILL.md fetch failed", "path", skillPath, "error", err) continue } name, _ := parseSkillFrontmatter(string(body)) @@ -1080,9 +1150,272 @@ func parseSkillFrontmatter(content string) (name, description string) { return name, description } +// --- GitHub import --- + +// githubSpec captures the parsed components of a github.com URL pointing at a +// skill (or single-skill repository). +type githubSpec struct { + owner string + repo string + ref string // empty → caller resolves the default branch + skillDir string // relative directory within the repo, "" for the repository root + + // refSegments holds the raw path segments after /tree/ or /blob/ that + // jointly encode (ref, skillDir). GitHub's web URLs do not delimit the + // boundary between branch/tag name and in-repo path, so when a ref + // contains '/' (e.g. "release/v2") segments[0] alone is not the ref. + // fetchFromGitHub uses resolveGitHubRefAndPath to walk these segments + // and ask the API which prefix is a real branch/tag/commit. When this + // slice is empty, ref/skillDir above are authoritative (root URL). + refSegments []string + // kind is "tree" or "blob"; "" for root URLs. blob requires the last + // segment to be SKILL.md, which is already stripped from refSegments. + kind string +} + +// parseGitHubURL extracts the owner, repo, and the raw post-/tree|/blob +// segments from a github.com URL. Supported forms: +// +// github.com/{owner}/{repo} → root, default branch +// github.com/{owner}/{repo}/tree/{ref}/{path...} → ref / skill dir +// github.com/{owner}/{repo}/blob/{ref}/{path.../SKILL.md} → ref / skill dir +// +// A simple-ref shortcut (segments[0] is the ref, the rest is the path) is +// stored in spec.ref/spec.skillDir; refSegments is also populated so that +// fetchFromGitHub can disambiguate refs containing '/' against the API. +func parseGitHubURL(raw string) (githubSpec, error) { + parsed, err := url.Parse(raw) + if err != nil { + return githubSpec{}, fmt.Errorf("invalid URL: %w", err) + } + parts := strings.Split(strings.Trim(parsed.Path, "/"), "/") + if len(parts) < 2 || parts[0] == "" || parts[1] == "" { + return githubSpec{}, fmt.Errorf("expected URL format: github.com/{owner}/{repo}[/tree/{ref}/{path}], got: %s", parsed.Path) + } + spec := githubSpec{owner: parts[0], repo: strings.TrimSuffix(parts[1], ".git")} + if len(parts) == 2 { + return spec, nil + } + kind := parts[2] + if kind != "tree" && kind != "blob" { + return githubSpec{}, fmt.Errorf("unsupported URL form: github.com/%s/%s/%s/... (use /tree/{ref}/... or /blob/{ref}/.../SKILL.md)", spec.owner, spec.repo, kind) + } + if len(parts) < 4 || parts[3] == "" { + return githubSpec{}, fmt.Errorf("missing ref after /%s/", kind) + } + spec.kind = kind + rest := parts[3:] + if kind == "blob" { + if !strings.EqualFold(rest[len(rest)-1], "SKILL.md") { + return githubSpec{}, fmt.Errorf("blob URL must point to a SKILL.md file") + } + rest = rest[:len(rest)-1] + if len(rest) == 0 { + return githubSpec{}, fmt.Errorf("missing ref after /blob/") + } + } + // Decode URL-escaped segments (e.g. spaces) so paths match the repo's + // real on-disk layout. Re-escaping happens in buildRawGitHubURL. + decoded := make([]string, len(rest)) + for i, p := range rest { + d, err := url.PathUnescape(p) + if err != nil { + return githubSpec{}, fmt.Errorf("invalid path segment %q: %w", p, err) + } + if d == "" { + return githubSpec{}, fmt.Errorf("empty path segment in URL") + } + decoded[i] = d + } + spec.refSegments = decoded + // Optimistic split: assume the simple case where the ref is one segment. + // fetchFromGitHub will re-resolve via the API and overwrite both fields + // when the optimistic guess does not validate (e.g. release/v2 refs). + spec.ref = decoded[0] + if len(decoded) > 1 { + spec.skillDir = strings.Join(decoded[1:], "/") + } + return spec, nil +} + +// resolveGitHubRefAndPath walks the parsed refSegments and asks the GitHub +// commits API which prefix corresponds to a real branch, tag, or commit. +// This is what makes refs containing '/' (e.g. "release/v2") work correctly: +// the URL github.com/o/r/tree/release/v2/skills/foo is ambiguous between +// (ref=release, path=v2/skills/foo) and (ref=release/v2, path=skills/foo), +// so we probe /repos/{o}/{r}/commits/{candidate} from longest to shortest +// and accept the first one the server confirms exists. +// +// On success spec.ref and spec.skillDir are overwritten with the resolved +// pair. On failure (no candidate resolves) a single error is returned that +// names every candidate that was tried. +func resolveGitHubRefAndPath(httpClient *http.Client, spec *githubSpec) error { + if len(spec.refSegments) == 0 { + return nil + } + // Try longest prefix first so that release/v2 wins over release. + tried := make([]string, 0, len(spec.refSegments)) + for n := len(spec.refSegments); n >= 1; n-- { + candidate := strings.Join(spec.refSegments[:n], "/") + tried = append(tried, candidate) + ok, err := githubRefExists(httpClient, spec.owner, spec.repo, candidate) + if err != nil { + // Network / transport errors should not be silently treated as + // "ref does not exist" — surface them so the caller can retry. + return fmt.Errorf("validating ref %q: %w", candidate, err) + } + if ok { + spec.ref = candidate + if n == len(spec.refSegments) { + spec.skillDir = "" + } else { + spec.skillDir = strings.Join(spec.refSegments[n:], "/") + } + return nil + } + } + return fmt.Errorf("could not resolve ref in github.com/%s/%s URL — tried: %s. Make sure the branch, tag, or commit exists and that the URL is the canonical /tree/{ref}/{path} or /blob/{ref}/{path}/SKILL.md form", + spec.owner, spec.repo, strings.Join(tried, ", ")) +} + +// githubRefExists returns true when GitHub recognizes ref as a branch, tag, +// or commit SHA on owner/repo. It uses the commits endpoint because that +// single call accepts all three ref kinds (unlike /branches or /tags which +// only match one). 404 means the ref does not exist; any other non-200 +// status is treated as an error so the caller can distinguish "missing" +// from "API down". +func githubRefExists(httpClient *http.Client, owner, repo, ref string) (bool, error) { + apiURL := fmt.Sprintf("https://api.github.com/repos/%s/%s/commits/%s", + url.PathEscape(owner), url.PathEscape(repo), escapeRefPath(ref)) + req, err := http.NewRequest(http.MethodGet, apiURL, nil) + if err != nil { + return false, err + } + // Per GitHub docs: Accept: application/vnd.github.v3.sha returns just + // the SHA when the ref resolves, which is the cheapest possible probe. + req.Header.Set("Accept", "application/vnd.github.v3.sha") + resp, err := httpClient.Do(req) + if err != nil { + return false, err + } + defer resp.Body.Close() + switch resp.StatusCode { + case http.StatusOK: + return true, nil + case http.StatusNotFound, http.StatusUnprocessableEntity: + return false, nil + default: + return false, fmt.Errorf("github API returned status %d for ref %q", resp.StatusCode, ref) + } +} + +func fetchFromGitHub(httpClient *http.Client, rawURL string) (*importedSkill, error) { + spec, err := parseGitHubURL(rawURL) + if err != nil { + return nil, err + } + if len(spec.refSegments) > 0 { + // Disambiguate slash-bearing refs (release/v2 etc.) against the API + // before issuing any raw or contents requests. + if err := resolveGitHubRefAndPath(httpClient, &spec); err != nil { + return nil, err + } + } + if spec.ref == "" { + spec.ref = fetchGitHubDefaultBranch(httpClient, spec.owner, spec.repo) + } + rawPrefix := fmt.Sprintf("https://raw.githubusercontent.com/%s/%s/%s", + url.PathEscape(spec.owner), url.PathEscape(spec.repo), escapeRefPath(spec.ref)) + + skillMdPath := "SKILL.md" + if spec.skillDir != "" { + skillMdPath = spec.skillDir + "/SKILL.md" + } + skillMdBody, err := fetchRawFile(httpClient, buildRawGitHubURL(rawPrefix, skillMdPath)) + if err != nil { + if spec.skillDir == "" { + return nil, fmt.Errorf("SKILL.md not found at the root of %s/%s@%s. For multi-skill repositories, point to a specific directory using github.com/%s/%s/tree/%s/", + spec.owner, spec.repo, spec.ref, spec.owner, spec.repo, spec.ref) + } + return nil, fmt.Errorf("SKILL.md not found at %s in %s/%s@%s: %w", + skillMdPath, spec.owner, spec.repo, spec.ref, err) + } + + name, description := parseSkillFrontmatter(string(skillMdBody)) + if name == "" { + if spec.skillDir != "" { + name = filepath.Base(spec.skillDir) + } else { + name = spec.repo + } + } + + result := &importedSkill{ + name: name, + description: description, + content: string(skillMdBody), + origin: map[string]any{ + "type": "github", + "source_url": rawURL, + "owner": spec.owner, + "repo": spec.repo, + "ref": spec.ref, + "path": spec.skillDir, + }, + } + + apiURL := buildGitHubContentsURL(spec.owner, spec.repo, spec.skillDir, spec.ref) + dirResp, err := httpClient.Get(apiURL) + if err != nil || dirResp.StatusCode != http.StatusOK { + // Cannot list the directory — return what we have (SKILL.md only). + // Keep this lenient: a private rate-limited request shouldn't fail + // an import that has already produced a valid SKILL.md. + if dirResp != nil { + dirResp.Body.Close() + } + return result, nil + } + defer dirResp.Body.Close() + + var entries []githubContentEntry + if err := json.NewDecoder(dirResp.Body).Decode(&entries); err != nil { + slog.Warn("github import: failed to decode top-level directory listing", "url", apiURL, "error", err) + return result, nil + } + + var allFiles []githubContentEntry + collectGitHubFiles(httpClient, entries, &allFiles, apiURL) + + basePath := "" + if spec.skillDir != "" { + basePath = spec.skillDir + "/" + } + for _, entry := range allFiles { + if entry.DownloadURL == "" { + continue + } + body, err := fetchRawFile(httpClient, entry.DownloadURL) + if err != nil { + if isCapError(err) { + return nil, fmt.Errorf("github import: %s: %w", entry.Path, err) + } + slog.Warn("github import: file download failed", "path", entry.Path, "error", err) + continue + } + relPath := strings.TrimPrefix(entry.Path, basePath) + if err := result.addFile(relPath, string(body)); err != nil { + return nil, err + } + } + + return result, nil +} + // --- Shared helpers --- -// fetchRawFile downloads a URL and returns the body bytes. Limit 1MB. +// fetchRawFile downloads a URL and returns the body bytes. Returns an error +// if the response exceeds maxImportFileSize so we never silently truncate a +// half-downloaded skill file into the workspace. func fetchRawFile(httpClient *http.Client, fileURL string) ([]byte, error) { resp, err := httpClient.Get(fileURL) if err != nil { @@ -1092,7 +1425,26 @@ func fetchRawFile(httpClient *http.Client, fileURL string) ([]byte, error) { if resp.StatusCode != http.StatusOK { return nil, fmt.Errorf("HTTP %d", resp.StatusCode) } - return io.ReadAll(io.LimitReader(resp.Body, 1<<20)) + body, err := io.ReadAll(io.LimitReader(resp.Body, maxImportFileSize+1)) + if err != nil { + return nil, err + } + if len(body) > maxImportFileSize { + return nil, fmt.Errorf("%w: file exceeds %d byte limit", errImportCapExceeded, maxImportFileSize) + } + return body, nil +} + +// escapeRefPath percent-encodes each segment of a git ref individually so +// that slash-bearing refs like "release/v2" are sent to GitHub as +// "release/v2" (path separators preserved) rather than "release%2Fv2" +// (which GitHub does not accept on the commits / raw endpoints). +func escapeRefPath(ref string) string { + parts := strings.Split(ref, "/") + for i, p := range parts { + parts[i] = url.PathEscape(p) + } + return strings.Join(parts, "/") } func buildRawGitHubURL(rawPrefix, repoPath string) string { @@ -1165,6 +1517,8 @@ func (h *Handler) ImportSkill(w http.ResponseWriter, r *http.Request) { imported, err = fetchFromClawHub(httpClient, normalized) case sourceSkillsSh: imported, err = fetchFromSkillsSh(httpClient, normalized) + case sourceGitHub: + imported, err = fetchFromGitHub(httpClient, normalized) } if err != nil { writeError(w, http.StatusBadGateway, err.Error()) @@ -1182,13 +1536,20 @@ func (h *Handler) ImportSkill(w http.ResponseWriter, r *http.Request) { }) } + // Persist provenance into skill.config.origin so list/detail UI can show + // "Imported from GitHub / ClawHub / Skills.sh" and link back to the source. + config := map[string]any{} + if imported.origin != nil { + config["origin"] = imported.origin + } + resp, err := h.createSkillWithFiles(r.Context(), skillCreateInput{ WorkspaceID: workspaceUUID, CreatorID: creatorUUID, Name: imported.name, Description: imported.description, Content: imported.content, - Config: map[string]any{}, + Config: config, Files: files, }) if err != nil { diff --git a/server/internal/handler/skill_test.go b/server/internal/handler/skill_test.go index a518015be..6cc81e9b0 100644 --- a/server/internal/handler/skill_test.go +++ b/server/internal/handler/skill_test.go @@ -221,7 +221,7 @@ func TestFetchFromSkillsSh_LogsSubdirectoryFailures(t *testing.T) { } logOutput := logs.String() - if !strings.Contains(logOutput, "skills.sh import: failed to list subdirectory") { + if !strings.Contains(logOutput, "github import: failed to list subdirectory") { t.Fatalf("expected warning log, got %q", logOutput) } if !strings.Contains(logOutput, "status=404") { @@ -540,6 +540,508 @@ func TestFetchFromSkillsSh_AnthropicPptxIntegration(t *testing.T) { } } +// --- GitHub source tests --- + +func TestParseGitHubURL(t *testing.T) { + cases := []struct { + name string + url string + want githubSpec + wantErr bool + }{ + { + name: "repo root", + url: "https://github.com/acme/skill", + want: githubSpec{owner: "acme", repo: "skill"}, + }, + { + name: "repo root with .git suffix", + url: "https://github.com/acme/skill.git", + want: githubSpec{owner: "acme", repo: "skill"}, + }, + { + name: "tree URL with directory", + url: "https://github.com/anthropics/skills/tree/main/document-skills/pptx", + want: githubSpec{owner: "anthropics", repo: "skills", ref: "main", skillDir: "document-skills/pptx"}, + }, + { + name: "tree URL ref only", + url: "https://github.com/anthropics/skills/tree/main", + want: githubSpec{owner: "anthropics", repo: "skills", ref: "main"}, + }, + { + name: "blob URL pointing at SKILL.md", + url: "https://github.com/acme/skills/blob/main/skills/foo/SKILL.md", + want: githubSpec{owner: "acme", repo: "skills", ref: "main", skillDir: "skills/foo"}, + }, + { + name: "blob URL with URL-escaped path segment", + url: "https://github.com/acme/skills/blob/main/my%20dir/SKILL.md", + want: githubSpec{owner: "acme", repo: "skills", ref: "main", skillDir: "my dir"}, + }, + { + name: "blob URL not pointing at SKILL.md", + url: "https://github.com/acme/skills/blob/main/skills/foo/README.md", + wantErr: true, + }, + { + name: "missing repo", + url: "https://github.com/acme", + wantErr: true, + }, + { + name: "unsupported segment", + url: "https://github.com/acme/skills/issues/1", + wantErr: true, + }, + { + name: "tree URL missing ref", + url: "https://github.com/acme/skills/tree/", + wantErr: true, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got, err := parseGitHubURL(tc.url) + if tc.wantErr { + if err == nil { + t.Fatalf("expected error, got %+v", got) + } + return + } + if err != nil { + t.Fatalf("parseGitHubURL: %v", err) + } + if got.owner != tc.want.owner || got.repo != tc.want.repo || + got.ref != tc.want.ref || got.skillDir != tc.want.skillDir { + t.Fatalf("got %+v, want %+v", got, tc.want) + } + }) + } +} + +func TestDetectImportSource_RecognizesGitHub(t *testing.T) { + src, _, err := detectImportSource("https://github.com/acme/skill") + if err != nil { + t.Fatalf("detectImportSource: %v", err) + } + if src != sourceGitHub { + t.Fatalf("source = %v, want sourceGitHub", src) + } +} + +func TestFetchFromGitHub_TreeURLImportsSkillDirectory(t *testing.T) { + client, requests := newGitHubFixtureClient(t, func(w http.ResponseWriter, r *http.Request) { + switch r.Header.Get("X-Test-Original-Host") { + case "api.github.com": + switch r.URL.Path { + case "/repos/anthropics/skills/commits/main": + w.Write([]byte("deadbeef")) + case "/repos/anthropics/skills/contents/document-skills/pptx": + if got := r.URL.Query().Get("ref"); got != "main" { + t.Fatalf("contents ref = %q, want main", got) + } + writeJSON(w, http.StatusOK, []githubContentEntry{ + { + Name: "editing.md", + Path: "document-skills/pptx/editing.md", + Type: "file", + DownloadURL: "https://raw.githubusercontent.com/anthropics/skills/main/document-skills/pptx/editing.md", + }, + { + Name: "scripts", + Path: "document-skills/pptx/scripts", + Type: "dir", + URL: "https://api.github.com/repos/anthropics/skills/contents/document-skills/pptx/scripts?ref=main", + }, + }) + case "/repos/anthropics/skills/contents/document-skills/pptx/scripts": + writeJSON(w, http.StatusOK, []githubContentEntry{ + { + Name: "add_slide.py", + Path: "document-skills/pptx/scripts/add_slide.py", + Type: "file", + DownloadURL: "https://raw.githubusercontent.com/anthropics/skills/main/document-skills/pptx/scripts/add_slide.py", + }, + }) + default: + http.NotFound(w, r) + } + case "raw.githubusercontent.com": + switch r.URL.Path { + case "/anthropics/skills/main/document-skills/pptx/SKILL.md": + w.Write([]byte("---\nname: pptx\ndescription: presentation tools\n---\nbody")) + case "/anthropics/skills/main/document-skills/pptx/editing.md": + w.Write([]byte("editing")) + case "/anthropics/skills/main/document-skills/pptx/scripts/add_slide.py": + w.Write([]byte("print('slide')")) + default: + http.NotFound(w, r) + } + default: + http.NotFound(w, r) + } + }) + + result, err := fetchFromGitHub(client, "https://github.com/anthropics/skills/tree/main/document-skills/pptx") + if err != nil { + t.Fatalf("fetchFromGitHub: %v", err) + } + if result.name != "pptx" { + t.Fatalf("name = %q, want pptx", result.name) + } + if result.description != "presentation tools" { + t.Fatalf("description = %q, want presentation tools", result.description) + } + gotPaths := importedFilePaths(result.files) + wantPaths := []string{"editing.md", "scripts/add_slide.py"} + if !equalStrings(gotPaths, wantPaths) { + t.Fatalf("files = %v (must be relative to skill dir), want %v", gotPaths, wantPaths) + } + // Verify the skill-relative path scheme: we never want supporting files + // to keep the in-repo prefix (document-skills/pptx/...). + for _, f := range result.files { + if strings.HasPrefix(f.path, "document-skills/") { + t.Fatalf("supporting file %q still carries skillDir prefix", f.path) + } + } + origin := result.origin + if origin == nil || origin["type"] != "github" { + t.Fatalf("origin = %v, want type=github", origin) + } + if origin["ref"] != "main" || origin["path"] != "document-skills/pptx" { + t.Fatalf("origin ref/path mismatch: %v", origin) + } + if !containsString(*requests, "api.github.com /repos/anthropics/skills/contents/document-skills/pptx?ref=main") { + t.Fatalf("expected contents listing, got %v", *requests) + } +} + +func TestFetchFromGitHub_RepoRootResolvesDefaultBranch(t *testing.T) { + client, requests := newGitHubFixtureClient(t, func(w http.ResponseWriter, r *http.Request) { + switch r.Header.Get("X-Test-Original-Host") { + case "api.github.com": + switch r.URL.Path { + case "/repos/alice/single-skill": + writeJSON(w, http.StatusOK, map[string]any{"default_branch": "master"}) + case "/repos/alice/single-skill/contents": + if got := r.URL.Query().Get("ref"); got != "master" { + t.Fatalf("contents ref = %q, want master", got) + } + writeJSON(w, http.StatusOK, []githubContentEntry{ + { + Name: "README.md", + Path: "README.md", + Type: "file", + DownloadURL: "https://raw.githubusercontent.com/alice/single-skill/master/README.md", + }, + }) + default: + http.NotFound(w, r) + } + case "raw.githubusercontent.com": + switch r.URL.Path { + case "/alice/single-skill/master/SKILL.md": + w.Write([]byte("---\nname: single-skill\n---\nbody")) + case "/alice/single-skill/master/README.md": + w.Write([]byte("readme")) + default: + http.NotFound(w, r) + } + default: + http.NotFound(w, r) + } + }) + + result, err := fetchFromGitHub(client, "https://github.com/alice/single-skill") + if err != nil { + t.Fatalf("fetchFromGitHub: %v", err) + } + if result.name != "single-skill" { + t.Fatalf("name = %q, want single-skill", result.name) + } + gotPaths := importedFilePaths(result.files) + if !equalStrings(gotPaths, []string{"README.md"}) { + t.Fatalf("files = %v", gotPaths) + } + if !containsString(*requests, "api.github.com /repos/alice/single-skill") { + t.Fatalf("expected default-branch lookup, got %v", *requests) + } +} + +func TestFetchFromGitHub_RepoRootMissingSKILLmdReturnsActionableError(t *testing.T) { + client, _ := newGitHubFixtureClient(t, func(w http.ResponseWriter, r *http.Request) { + switch r.Header.Get("X-Test-Original-Host") { + case "api.github.com": + if r.URL.Path == "/repos/alice/multi" { + writeJSON(w, http.StatusOK, map[string]any{"default_branch": "main"}) + return + } + http.NotFound(w, r) + case "raw.githubusercontent.com": + http.NotFound(w, r) + default: + http.NotFound(w, r) + } + }) + + _, err := fetchFromGitHub(client, "https://github.com/alice/multi") + if err == nil { + t.Fatal("expected error for missing root SKILL.md") + } + if !strings.Contains(err.Error(), "tree/main/") && !strings.Contains(err.Error(), "tree/main") { + t.Fatalf("error should hint at /tree/{ref}/, got %q", err.Error()) + } +} + +func TestFetchFromGitHub_BlobURLImportsSpecificSkill(t *testing.T) { + client, _ := newGitHubFixtureClient(t, func(w http.ResponseWriter, r *http.Request) { + switch r.Header.Get("X-Test-Original-Host") { + case "api.github.com": + switch r.URL.Path { + case "/repos/acme/skills/commits/main": + w.Write([]byte("deadbeef")) + case "/repos/acme/skills/contents/skills/foo": + writeJSON(w, http.StatusOK, []githubContentEntry{}) + default: + http.NotFound(w, r) + } + case "raw.githubusercontent.com": + if r.URL.Path == "/acme/skills/main/skills/foo/SKILL.md" { + w.Write([]byte("---\nname: foo\n---\nbody")) + return + } + http.NotFound(w, r) + default: + http.NotFound(w, r) + } + }) + + result, err := fetchFromGitHub(client, "https://github.com/acme/skills/blob/main/skills/foo/SKILL.md") + if err != nil { + t.Fatalf("fetchFromGitHub: %v", err) + } + if result.name != "foo" { + t.Fatalf("name = %q, want foo", result.name) + } + if result.origin["path"] != "skills/foo" { + t.Fatalf("origin path = %v, want skills/foo", result.origin["path"]) + } +} + +// --- Bundle / file size cap tests --- + +func TestFetchRawFile_ReturnsErrorOnOversizedFile(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Write(bytes.Repeat([]byte("a"), maxImportFileSize+1024)) + })) + t.Cleanup(server.Close) + + _, err := fetchRawFile(&http.Client{}, server.URL+"/big.bin") + if err == nil { + t.Fatal("expected error for oversized file, got nil") + } + if !strings.Contains(err.Error(), "byte limit") { + t.Fatalf("error = %q, want byte limit message", err.Error()) + } + if !isCapError(err) { + t.Fatalf("error %q must be classified as a cap error so callers fail-fast", err.Error()) + } +} + +func TestImportedSkill_AddFileEnforcesBundleLimits(t *testing.T) { + t.Run("file count", func(t *testing.T) { + s := &importedSkill{} + for i := 0; i < maxImportFileCount; i++ { + if err := s.addFile("f", "x"); err != nil { + t.Fatalf("addFile %d: %v", i, err) + } + } + err := s.addFile("overflow", "x") + if err == nil { + t.Fatal("expected file count cap error") + } + if !isCapError(err) { + t.Fatalf("error %q must be a cap error", err.Error()) + } + }) + t.Run("total bytes", func(t *testing.T) { + s := &importedSkill{} + big := strings.Repeat("y", maxImportTotalSize) + if err := s.addFile("a", big); err != nil { + t.Fatalf("addFile at cap: %v", err) + } + err := s.addFile("b", "x") + if err == nil { + t.Fatal("expected total bytes cap error") + } + if !isCapError(err) { + t.Fatalf("error %q must be a cap error", err.Error()) + } + }) +} + +// fetchFromGitHub must FAIL the import (not just log+continue) when a +// supporting file exceeds the per-file cap — silently dropping the file +// would leave a skill bundle that looks valid to the user but is missing +// content. +func TestFetchFromGitHub_OversizedSupportingFileFailsImport(t *testing.T) { + client, _ := newGitHubFixtureClient(t, func(w http.ResponseWriter, r *http.Request) { + switch r.Header.Get("X-Test-Original-Host") { + case "api.github.com": + switch r.URL.Path { + case "/repos/acme/skills/commits/main": + w.Write([]byte("deadbeef")) + case "/repos/acme/skills/contents/foo": + writeJSON(w, http.StatusOK, []githubContentEntry{ + { + Name: "huge.bin", + Path: "foo/huge.bin", + Type: "file", + DownloadURL: "https://raw.githubusercontent.com/acme/skills/main/foo/huge.bin", + }, + }) + default: + http.NotFound(w, r) + } + case "raw.githubusercontent.com": + switch r.URL.Path { + case "/acme/skills/main/foo/SKILL.md": + w.Write([]byte("---\nname: foo\n---\nbody")) + case "/acme/skills/main/foo/huge.bin": + w.Write(bytes.Repeat([]byte("z"), maxImportFileSize+512)) + default: + http.NotFound(w, r) + } + default: + http.NotFound(w, r) + } + }) + _, err := fetchFromGitHub(client, "https://github.com/acme/skills/tree/main/foo") + if err == nil { + t.Fatal("expected oversized supporting file to fail the whole import") + } + if !strings.Contains(err.Error(), "huge.bin") || !strings.Contains(err.Error(), "byte limit") { + t.Fatalf("error %q should name the file and the cap", err.Error()) + } +} + +// fetchFromSkillsSh has the same supporting-file loop and must also fail +// (not just warn) when one of those files exceeds the cap. +func TestFetchFromSkillsSh_OversizedSupportingFileFailsImport(t *testing.T) { + client, _ := newGitHubFixtureClient(t, func(w http.ResponseWriter, r *http.Request) { + switch r.Header.Get("X-Test-Original-Host") { + case "api.github.com": + switch r.URL.Path { + case "/repos/acme/skills": + writeJSON(w, http.StatusOK, map[string]any{"default_branch": "main"}) + case "/repos/acme/skills/contents/skills/foo": + writeJSON(w, http.StatusOK, []githubContentEntry{ + { + Name: "huge.bin", + Path: "skills/foo/huge.bin", + Type: "file", + DownloadURL: "https://raw.githubusercontent.com/acme/skills/main/skills/foo/huge.bin", + }, + }) + default: + http.NotFound(w, r) + } + case "raw.githubusercontent.com": + switch r.URL.Path { + case "/acme/skills/main/skills/foo/SKILL.md": + w.Write([]byte("---\nname: foo\n---\nbody")) + case "/acme/skills/main/skills/foo/huge.bin": + w.Write(bytes.Repeat([]byte("z"), maxImportFileSize+512)) + default: + http.NotFound(w, r) + } + default: + http.NotFound(w, r) + } + }) + _, err := fetchFromSkillsSh(client, "https://skills.sh/acme/skills/foo") + if err == nil { + t.Fatal("expected oversized supporting file to fail the whole import") + } + if !strings.Contains(err.Error(), "huge.bin") { + t.Fatalf("error %q should name the offending file", err.Error()) + } +} + +// Slash-bearing refs (e.g. release/v2) are now resolved against the API +// instead of being silently parsed as ref="release", path="v2/...". The +// resolver must walk longest→shortest and pick the prefix the API +// confirms exists. +func TestFetchFromGitHub_ResolvesSlashRefAgainstAPI(t *testing.T) { + client, requests := newGitHubFixtureClient(t, func(w http.ResponseWriter, r *http.Request) { + switch r.Header.Get("X-Test-Original-Host") { + case "api.github.com": + switch r.URL.Path { + case "/repos/acme/skills/commits/release/v2/skills/foo", + "/repos/acme/skills/commits/release/v2/skills": + http.NotFound(w, r) + case "/repos/acme/skills/commits/release/v2": + w.Write([]byte("deadbeef")) + case "/repos/acme/skills/contents/skills/foo": + if got := r.URL.Query().Get("ref"); got != "release/v2" { + t.Fatalf("contents called with ref=%q, want release/v2", got) + } + writeJSON(w, http.StatusOK, []githubContentEntry{}) + default: + http.NotFound(w, r) + } + case "raw.githubusercontent.com": + switch r.URL.Path { + case "/acme/skills/release/v2/skills/foo/SKILL.md": + w.Write([]byte("---\nname: foo\n---\nbody")) + default: + http.NotFound(w, r) + } + default: + http.NotFound(w, r) + } + }) + result, err := fetchFromGitHub(client, "https://github.com/acme/skills/tree/release/v2/skills/foo") + if err != nil { + t.Fatalf("fetchFromGitHub: %v", err) + } + if result.origin["ref"] != "release/v2" { + t.Fatalf("origin ref = %v, want release/v2", result.origin["ref"]) + } + if result.origin["path"] != "skills/foo" { + t.Fatalf("origin path = %v, want skills/foo", result.origin["path"]) + } + // Sanity-check that the resolver actually probed in the expected order. + if !containsString(*requests, "api.github.com /repos/acme/skills/commits/release/v2/skills/foo") { + t.Fatalf("resolver should probe longest prefix first, requests=%v", *requests) + } +} + +// When none of the candidate refs resolve, fail with a clear error that +// names what was tried — do not silently fall back to using the first +// segment as the ref (the previous behavior, which would import the wrong +// branch / wrong path). +func TestFetchFromGitHub_UnresolvableRefFailsLoudly(t *testing.T) { + client, _ := newGitHubFixtureClient(t, func(w http.ResponseWriter, r *http.Request) { + switch r.Header.Get("X-Test-Original-Host") { + case "api.github.com": + http.NotFound(w, r) + case "raw.githubusercontent.com": + t.Fatalf("must not hit raw.githubusercontent.com when ref unresolved: %s", r.URL.Path) + default: + http.NotFound(w, r) + } + }) + _, err := fetchFromGitHub(client, "https://github.com/acme/skills/tree/nope/skills/foo") + if err == nil { + t.Fatal("expected error when no candidate ref resolves") + } + if !strings.Contains(err.Error(), "could not resolve ref") { + t.Fatalf("error %q should mention ref resolution failure", err.Error()) + } +} + type rewriteGitHubTransport struct { target *url.URL base http.RoundTripper