diff --git a/README.md b/README.md index d7b0bc1..45d3a1e 100644 --- a/README.md +++ b/README.md @@ -87,16 +87,29 @@ This writes the skill file to `~/.claude/skills/` and `~/.codex/skills/`. ### Skills from repositories -Install, list, and uninstall skills from GitHub repositories or local paths: +Install, update, sync, list, and uninstall skills from GitHub repositories or local paths: ```sh -clime skills install owner/repo # browse and install skills from a repo -clime skills install /local/path # install from a local directory -clime skills install # interactive mode — pick a source and skills -clime skills list # list installed skills -clime skills uninstall # remove a skill +clime skills install owner/repo # browse and install skills (same as @latest) +clime skills install owner/repo@latest # highest stable semver tag, like go get +clime skills install owner/repo@v1 # highest v1.x.y tag +clime skills install owner/repo@v1.2.3 # pin to a tag, branch, or commit SHA +clime skills install /local/path # install from a local directory +clime skills install # interactive mode — pick a source and skills +clime skills update # update every source to its latest version +clime skills update owner/repo # update one source to latest +clime skills update owner/repo@v1.2.3 # move one source to a specific version +clime skills sync # reinstall skills at the versions locked in the manifest +clime skills list # list installed skills +clime skills uninstall # remove a skill ``` +The manifest at `~/.clime/skills.yaml` records each source's locked version. `sync` +applies those versions as-is (from the local cache when available, so it works +offline), while `update` resolves a newer version and rewrites the lock. An update +is refused when the new version no longer provides an installed skill, so skills +are never removed implicitly. + ## Shell Completions Generate shell completion scripts for bash, zsh, fish, or PowerShell: diff --git a/cmd/plugin_skills.go b/cmd/plugin_skills.go index 83cd877..72a6ddc 100644 --- a/cmd/plugin_skills.go +++ b/cmd/plugin_skills.go @@ -4,7 +4,6 @@ import ( "fmt" "os/exec" "strings" - "time" uicli "github.com/alperdrsnn/clime" "github.com/git-hulk/clime/internal/plugin" @@ -18,6 +17,22 @@ const pluginSkillsOption = "Plugin Skills" // auto-install any skills the plugin provides. It's a variable for testing. var pluginSkillInstaller = tryInstallPluginSkills +// pluginSkillEvents reports only successful installs; every failure stays +// silent so plugin installation is never blocked. +type pluginSkillEvents struct { + skill.NopEvents +} + +// pluginSkillSource pairs a plugin name with its skill source path/repo. +type pluginSkillSource struct { + pluginName string + source string +} + +func (pluginSkillEvents) SkillInstalled(_ skill.Verb, name string, targets []string) { + terminal.Successf("Installed plugin skill %q to %s", name, strings.Join(targets, ", ")) +} + // tryInstallPluginSkills invokes `clime- skills` to discover a skill // source from the plugin. If the plugin provides skills, they are automatically // installed. Errors are silently ignored so plugin installation is never blocked. @@ -26,41 +41,28 @@ func tryInstallPluginSkills(name string) { if source == "" { return } - - manifest, err := skill.LoadManifest() + src, err := skill.ParseSource(source) if err != nil { return } - repoManifest, err := skill.FetchRepoManifest(source) + mgr, err := skill.Open(pluginSkillEvents{}) if err != nil { return } - dir, cleanup, err := skill.PrepareRepoDir(source) + snap, catalog, err := mgr.Fetch(src) if err != nil { return } - defer cleanup() - for _, entry := range repoManifest.Skills { - if _, installed := manifest.GetSkill(entry.Name); installed { - continue - } - targets, err := skill.InstallFromDir(entry.Name, dir, entry.Path) - if err != nil || len(targets) == 0 { - continue + var entries []skill.Entry + for _, entry := range catalog.Skills { + if _, installed := mgr.Manifest.GetSkill(entry.Name); !installed { + entries = append(entries, entry) } - manifest.AddSkill(skill.InstalledSkill{ - Name: entry.Name, - Description: entry.Description, - Source: source, - Path: entry.Path, - InstalledAt: time.Now(), - }) - manifest.Save() - terminal.Successf("Installed plugin skill %q to %s", entry.Name, strings.Join(targets, ", ")) } + _, _ = mgr.Install(snap, entries) } // getPluginSkillSource runs `clime- skills` and returns the trimmed @@ -78,12 +80,6 @@ func getPluginSkillSource(name string) string { return strings.TrimSpace(string(out)) } -// pluginSkillSource pairs a plugin name with its skill source path/repo. -type pluginSkillSource struct { - pluginName string - source string -} - // discoverPluginSkillSources iterates all discovered plugins and returns // those that support the `skills` subcommand with a valid source. func discoverPluginSkillSources() []pluginSkillSource { @@ -105,11 +101,7 @@ func discoverPluginSkillSources() []pluginSkillSource { // It scans all plugins for skill sources, presents available skills, and // installs the user's selections. func installFromPluginSkills(manifest *skill.Manifest) error { - spinner := uicli.NewSpinner(). - WithStyle(uicli.SpinnerDots). - WithColor(uicli.CyanColor). - WithMessage("Scanning plugins for skills..."). - Start() + spinner := startSpinner("Scanning plugins for skills...") sources := discoverPluginSkillSources() if len(sources) == 0 { @@ -118,24 +110,30 @@ func installFromPluginSkills(manifest *skill.Manifest) error { return nil } + mgr, err := newSkillsManager(manifest) + if err != nil { + spinner.Error("Failed to prepare skill manager") + return err + } + // Collect skills from all plugin sources. type skillCandidate struct { - entry skill.SkillEntry - source string - label string + entry skill.Entry + src skill.Source + label string } var candidates []skillCandidate - var dirs []struct { - dir string - cleanup func() - } for _, ps := range sources { - repoManifest, err := skill.FetchRepoManifest(ps.source) + src, err := skill.ParseSource(ps.source) + if err != nil { + continue + } + _, catalog, err := mgr.Fetch(src) if err != nil { continue } - for _, entry := range repoManifest.Skills { + for _, entry := range catalog.Skills { if _, installed := manifest.GetSkill(entry.Name); installed { continue } @@ -144,9 +142,9 @@ func installFromPluginSkills(manifest *skill.Manifest) error { label = fmt.Sprintf("%s — %s (%s)", entry.Name, uicli.TruncateString(entry.Description, 50), ps.pluginName) } candidates = append(candidates, skillCandidate{ - entry: entry, - source: ps.source, - label: label, + entry: entry, + src: src, + label: label, }) } } @@ -178,46 +176,35 @@ func installFromPluginSkills(manifest *skill.Manifest) error { return nil } - // Group selected skills by source to prepare repos efficiently. + // Group selected skills by source so each repository resolves once. type sourceSkills struct { - source string - entries []skill.SkillEntry + src skill.Source + entries []skill.Entry } sourceMap := make(map[string]*sourceSkills) for _, idx := range selectedIdxs { c := candidates[idx] - ss, ok := sourceMap[c.source] + ss, ok := sourceMap[c.src.Repo] if !ok { - ss = &sourceSkills{source: c.source} - sourceMap[c.source] = ss + ss = &sourceSkills{src: c.src} + sourceMap[c.src.Repo] = ss } ss.entries = append(ss.entries, c.entry) } fmt.Println() for _, ss := range sourceMap { - dir, cleanup, err := skill.PrepareRepoDir(ss.source) + snap, err := mgr.Store.Snapshot(ss.src) if err != nil { - terminal.Errorf("Failed to prepare %q: %v", ss.source, err) + terminal.Errorf("Failed to prepare %q: %v", ss.src, err) continue } - dirs = append(dirs, struct { - dir string - cleanup func() - }{dir, cleanup}) - manifest.AddSource(ss.source) + manifest.AddSource(ss.src) manifest.Save() - for _, entry := range ss.entries { - if err := installSkillEntry(manifest, &entry, ss.source, dir); err != nil { - terminal.Errorf("Failed to install %q: %v", entry.Name, err) - } - } - } - - for _, d := range dirs { - d.cleanup() + // Per-skill failures are already reported through the progress events. + _, _ = mgr.Install(snap, ss.entries) } return nil diff --git a/cmd/skill.go b/cmd/skill.go index e24e19c..001762a 100644 --- a/cmd/skill.go +++ b/cmd/skill.go @@ -2,9 +2,9 @@ package cmd import ( "fmt" - "os" "path/filepath" + "github.com/git-hulk/clime/internal/skill" "github.com/spf13/cobra" ) @@ -31,44 +31,28 @@ var installSkillCmd = &cobra.Command{ Use: "skill", Short: "Install the clime-cli skill into ~/.claude/skills and ~/.codex/skills", RunE: func(cmd *cobra.Command, args []string) error { - home, err := os.UserHomeDir() + targets, err := skill.Targets() if err != nil { - return fmt.Errorf("failed to get home directory: %w", err) + return err } installed := 0 - targets := []string{".claude", ".codex"} - for _, dir := range targets { - base := filepath.Join(home, dir) - info, err := os.Stat(base) - if err != nil || !info.IsDir() { - terminal.Warningf("Skipping %s (directory not found)", base) + for _, t := range targets { + if !t.Exists() { + terminal.Warningf("Skipping %s (directory not found)", t.Dir) continue } - skillDir := filepath.Join(base, "skills", skillDirName) - if err := os.MkdirAll(skillDir, 0o755); err != nil { - return fmt.Errorf("failed to create %s: %w", skillDir, err) + files := map[string][]byte{skillFileName: []byte(SkillContent)} + // Codex discovers skills through agents/openai.yaml. + if t.Name == "codex" { + files[filepath.Join("agents", "openai.yaml")] = []byte(AgentYAML) } - - dest := filepath.Join(skillDir, skillFileName) - if err := os.WriteFile(dest, []byte(SkillContent), 0o644); err != nil { - return fmt.Errorf("failed to write %s: %w", dest, err) - } - - // Write agents/openai.yaml for Codex skill discovery. - if dir == ".codex" { - agentsDir := filepath.Join(skillDir, "agents") - if err := os.MkdirAll(agentsDir, 0o755); err != nil { - return fmt.Errorf("failed to create %s: %w", agentsDir, err) - } - yamlDest := filepath.Join(agentsDir, "openai.yaml") - if err := os.WriteFile(yamlDest, []byte(AgentYAML), 0o644); err != nil { - return fmt.Errorf("failed to write %s: %w", yamlDest, err) - } + if err := t.Install(skillDirName, files); err != nil { + return fmt.Errorf("failed to install skill to %s: %w", t.Dir, err) } - terminal.Successf("Installed skill to %s", dest) + terminal.Successf("Installed skill to %s", filepath.Join(t.Dir, "skills", skillDirName, skillFileName)) installed++ } diff --git a/cmd/skills.go b/cmd/skills.go index 33ce228..664c510 100644 --- a/cmd/skills.go +++ b/cmd/skills.go @@ -5,7 +5,6 @@ import ( "fmt" "os" "strings" - "time" uicli "github.com/alperdrsnn/clime" "github.com/git-hulk/clime/internal/prompt" @@ -13,11 +12,49 @@ import ( "github.com/spf13/cobra" ) +type sourceAction int + +const ( + actionBrowseInstall sourceAction = iota + actionRemoveSource + actionUpdate +) + +const newRepoOption = "Enter a new repository..." + +var ( + selectPrompt = prompt.Select + multiSelectPrompt = prompt.MultiSelect + inputPrompt = prompt.Input + skillsActionRunner = runSkillsSourceAction + + skillsInstallForce bool +) + +// verbUI maps a skill verb to the wording the progress output uses. +var verbUI = map[skill.Verb]struct{ resolving, present, past, prep string }{ + skill.VerbInstall: {"Preparing", "Installing", "Installed", "at"}, + skill.VerbUpdate: {"Resolving", "Updating", "Updated", "to"}, + skill.VerbSync: {"Preparing", "Syncing", "Synced", "at"}, +} + +// skillsUI renders Manager progress events as terminal spinners. +type skillsUI struct { + spinner *uicli.Spinner +} + +type installCandidate struct { + entry skill.Entry + label string +} + func init() { skillsInstallCmd.Flags().BoolVarP(&skillsInstallForce, "force", "f", false, "when installing from a repo, also (re)install skills that are already installed and overwrite them") skillsCmd.AddCommand(skillsListCmd) skillsCmd.AddCommand(skillsInstallCmd) + skillsCmd.AddCommand(skillsUpdateCmd) + skillsCmd.AddCommand(skillsSyncCmd) skillsCmd.AddCommand(skillsUninstallCmd) rootCmd.AddCommand(skillsCmd) } @@ -51,88 +88,139 @@ var skillsListCmd = &cobra.Command{ uicli.DimColor.Sprintf("(%d total)", len(manifest.Skills)), ) - headers := []string{"NAME", "DESCRIPTION", "SOURCE"} - const descWidth = 50 + headers := []string{"NAME", "SOURCE", "VERSION"} var rows [][]string for _, s := range manifest.Skills { - desc := s.Description - if desc == "" { - desc = "—" - } - desc = uicli.TruncateString(desc, descWidth) - rows = append(rows, []string{s.Name, desc, s.Source}) + record, _ := manifest.GetSource(skill.Source{Repo: s.Source}) + rows = append(rows, []string{s.Name, s.Source, skill.DisplayVersion(record.Version)}) } + printTable(headers, rows) - colWidths := make([]int, len(headers)) - for i, h := range headers { - colWidths[i] = len(h) - } - for _, row := range rows { - for i, cell := range row { - if len(cell) > colWidths[i] { - colWidths[i] = len(cell) - } + return nil + }, +} + +func printTable(headers []string, rows [][]string) { + colWidths := make([]int, len(headers)) + for i, h := range headers { + colWidths[i] = len(h) + } + for _, row := range rows { + for i, cell := range row { + if len(cell) > colWidths[i] { + colWidths[i] = len(cell) } } + } - const gap = 2 - const indent = " " + const gap = 2 + const indent = " " - fmt.Print(indent) - for i, h := range headers { - if i > 0 { - fmt.Print(strings.Repeat(" ", gap)) - } - fmt.Print(uicli.BoldColor.Sprintf("%-*s", colWidths[i], h)) + fmt.Print(indent) + for i, h := range headers { + if i > 0 { + fmt.Print(strings.Repeat(" ", gap)) } - fmt.Println() + fmt.Print(uicli.BoldColor.Sprintf("%-*s", colWidths[i], h)) + } + fmt.Println() + fmt.Print(indent) + for i, w := range colWidths { + if i > 0 { + fmt.Print(strings.Repeat(" ", gap)) + } + fmt.Print(strings.Repeat("-", w)) + } + fmt.Println() + + for _, row := range rows { fmt.Print(indent) - for i, w := range colWidths { + for i, cell := range row { if i > 0 { fmt.Print(strings.Repeat(" ", gap)) } - fmt.Print(strings.Repeat("-", w)) + fmt.Printf("%-*s", colWidths[i], cell) } fmt.Println() + } +} - for _, row := range rows { - fmt.Print(indent) - for i, cell := range row { - if i > 0 { - fmt.Print(strings.Repeat(" ", gap)) - } - fmt.Printf("%-*s", colWidths[i], cell) +var skillsInstallCmd = &cobra.Command{ + Use: "install [owner/repo[@version]|path]", + Short: "Install skills from a GitHub repository or local path", + Long: "Install skills from a GitHub repository or local path. A repository source " + + "may carry a Go-style version suffix resolved like `go get`: owner/repo@latest " + + "(the default when no version is given) picks the highest stable semver tag, " + + "owner/repo@v1 the highest v1.x.y tag, and an exact tag, branch, or commit SHA " + + "pins that revision.", + Args: cobra.MaximumNArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + manifest, err := skill.LoadManifest() + if err != nil { + return fmt.Errorf("failed to load skills manifest: %w", err) + } + + if len(args) > 0 { + if skillsInstallForce { + return installFromRepo(manifest, args[0], true) } - fmt.Println() + return runSkillsSourceAction(manifest, args[0], actionBrowseInstall) } - return nil + return runInteractiveSkillsInstall(manifest) }, } -type sourceAction int - -const ( - actionBrowseInstall sourceAction = iota - actionRemoveSource - actionUpdate -) - -const newRepoOption = "Enter a new repository..." +var skillsUpdateCmd = &cobra.Command{ + Use: "update [owner/repo[@version]]", + Short: "Update installed skills to the latest version", + Long: "Update the skills installed from a source repository. With no argument every " + + "source is updated to its latest version. With a repository, only that source is " + + "updated: to latest, or to the version given by a Go-style suffix such as " + + "owner/repo@v1.2.3 or owner/repo@v1. The set of installed skills is preserved.", + Args: cobra.MaximumNArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + manifest, err := skill.LoadManifest() + if err != nil { + return fmt.Errorf("failed to load skills manifest: %w", err) + } -var ( - selectPrompt = prompt.Select - multiSelectPrompt = prompt.MultiSelect - inputPrompt = prompt.Input - skillsActionRunner = runSkillsSourceAction + if len(args) > 0 { + return runSkillsSourceAction(manifest, args[0], actionUpdate) + } + mgr, err := newSkillsManager(manifest) + if err != nil { + return err + } + return forEachInstalledSource(mgr, skill.VerbUpdate, mgr.Update) + }, +} - skillsInstallForce bool -) +var skillsSyncCmd = &cobra.Command{ + Use: "sync", + Short: "Reinstall skills at the versions locked in the manifest", + Long: "Reinstall every skill recorded in ~/.clime/skills.yaml from its source at the " + + "locked version, without looking for a newer one. Versions already cached under " + + "~/.clime/sources are applied without network access. Use `clime skills update` " + + "to move a source to a newer version.", + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, args []string) error { + manifest, err := skill.LoadManifest() + if err != nil { + return fmt.Errorf("failed to load skills manifest: %w", err) + } + mgr, err := newSkillsManager(manifest) + if err != nil { + return err + } + return forEachInstalledSource(mgr, skill.VerbSync, mgr.Sync) + }, +} -var skillsInstallCmd = &cobra.Command{ - Use: "install [owner/repo|path]", - Short: "Install skills from a GitHub repository or local path", +var skillsUninstallCmd = &cobra.Command{ + Use: "uninstall [skill-name]", + Short: "Uninstall a previously installed skill", Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { manifest, err := skill.LoadManifest() @@ -140,22 +228,100 @@ var skillsInstallCmd = &cobra.Command{ return fmt.Errorf("failed to load skills manifest: %w", err) } - if len(args) > 0 { - if skillsInstallForce { - if err := validateSkillRepoSource(args[0]); err != nil { - return err - } - return installFromRepo(manifest, args[0], true) - } - return runSkillsSourceAction(manifest, args[0], actionBrowseInstall) + if len(args) == 0 { + return interactiveUninstall(manifest) } - return runInteractiveSkillsInstall(manifest) + return uninstallByName(manifest, args[0]) }, } +// newSkillsManager assembles a Manager over an already-loaded manifest, +// reporting progress through terminal spinners. +func newSkillsManager(manifest *skill.Manifest) (*skill.Manager, error) { + store, err := skill.OpenStore() + if err != nil { + return nil, err + } + targets, err := skill.DetectTargets() + if err != nil { + return nil, err + } + return &skill.Manager{ + Manifest: manifest, + Store: store, + Targets: targets, + Events: &skillsUI{}, + }, nil +} + +func startSpinner(msg string) *uicli.Spinner { + return uicli.NewSpinner(). + WithStyle(uicli.SpinnerDots). + WithColor(uicli.CyanColor). + WithMessage(msg). + Start() +} + +// finish ends the active spinner with fn, tolerating events that arrive +// without a preceding start. +func (u *skillsUI) finish(fn func(*uicli.Spinner)) { + if u.spinner != nil { + fn(u.spinner) + u.spinner = nil + } +} + +func (u *skillsUI) SourceResolving(verb skill.Verb, src skill.Source) { + u.spinner = startSpinner(fmt.Sprintf("%s %s...", verbUI[verb].resolving, src)) +} + +func (u *skillsUI) SourceFailed(verb skill.Verb, src skill.Source, err error) { + u.finish(func(s *uicli.Spinner) { s.Error(fmt.Sprintf("Failed to %s %s", verb, src.Repo)) }) +} + +func (u *skillsUI) SourceUpToDate(src skill.Source, version string) { + u.finish(func(s *uicli.Spinner) { + s.Success(fmt.Sprintf("%s is already at %s", src.Repo, skill.DisplayVersion(version))) + }) +} + +func (u *skillsUI) SourceReady(verb skill.Verb, src skill.Source, version string) { + label := fmt.Sprintf("%s %s", verbUI[verb].present, src.Repo) + if version != "" { + label = fmt.Sprintf("%s %s %s", label, verbUI[verb].prep, skill.DisplayVersion(version)) + } + u.finish(func(s *uicli.Spinner) { s.Success(label) }) +} + +func (u *skillsUI) SkillInstalling(verb skill.Verb, name string, src skill.Source) { + u.spinner = startSpinner(fmt.Sprintf("%s skill %q from %s...", verbUI[verb].present, name, src.Repo)) +} + +func (u *skillsUI) SkillInstalled(verb skill.Verb, name string, targets []string) { + u.finish(func(s *uicli.Spinner) { + s.Success(fmt.Sprintf("%s skill %q to %s", verbUI[verb].past, name, strings.Join(targets, ", "))) + }) +} + +func (u *skillsUI) SkillFailed(verb skill.Verb, name string, err error) { + u.finish(func(s *uicli.Spinner) { + if errors.Is(err, os.ErrNotExist) { + s.Stop() + return + } + s.Error(fmt.Sprintf("Failed to %s skill %q", verb, name)) + }) + terminal.Errorf("Failed to %s %q: %v", verb, name, err) +} + +func (u *skillsUI) NoTargets() { + u.finish(func(s *uicli.Spinner) { s.Stop() }) + terminal.Warning("No skill directories were installed. Neither ~/.claude nor ~/.codex was found.") +} + func runInteractiveSkillsInstall(manifest *skill.Manifest) error { - sources := uniqueSkillSources(manifest) + sources := manifest.KnownSources() if len(sources) == 0 { fmt.Println() repo, err := inputPrompt("Enter repository (owner/repo)") @@ -165,7 +331,11 @@ func runInteractiveSkillsInstall(manifest *skill.Manifest) error { return skillsActionRunner(manifest, repo, actionBrowseInstall) } - options := append(sources, pluginSkillsOption, newRepoOption) + options := make([]string, 0, len(sources)+2) + for _, src := range sources { + options = append(options, src.Repo) + } + options = append(options, pluginSkillsOption, newRepoOption) showSourceSpacer := true for { if showSourceSpacer { @@ -224,54 +394,25 @@ func runInteractiveSkillsInstall(manifest *skill.Manifest) error { } } -func uniqueSkillSources(manifest *skill.Manifest) []string { - // Collect unique sources from installed skills and tracked sources, preserving order. - seen := make(map[string]bool) - var sources []string - for _, s := range manifest.Skills { - if s.Source != "" && !seen[s.Source] { - seen[s.Source] = true - sources = append(sources, s.Source) - } - } - for _, s := range manifest.Sources { - if s != "" && !seen[s] { - seen[s] = true - sources = append(sources, s) - } - } - - return sources -} - -func runSkillsSourceAction(manifest *skill.Manifest, repo string, action sourceAction) error { - if err := validateSkillRepoSource(repo); err != nil { +func runSkillsSourceAction(manifest *skill.Manifest, source string, action sourceAction) error { + src, err := skill.ParseSource(source) + if err != nil { return err } switch action { case actionRemoveSource: - return removeSource(manifest, repo) + return removeSource(manifest, src) case actionUpdate: - return updateSource(manifest, repo) - default: - return installFromRepo(manifest, repo, false) - } -} - -func validateSkillRepoSource(repo string) error { - if repo == "" { - return fmt.Errorf("invalid repo format: expected owner/repo or local path, got %q", repo) - } - if _, ok, err := skill.LocalRepoDir(repo); err != nil { + mgr, err := newSkillsManager(manifest) + if err != nil { + return err + } + _, err = mgr.Update(src) return err - } else if ok { - return nil - } - if !strings.Contains(repo, "/") { - return fmt.Errorf("invalid repo format: expected owner/repo or local path, got %q", repo) + default: + return installFromRepo(manifest, source, false) } - return nil } func pickSourceAction(repo string, showSpacer bool) (sourceAction, error) { @@ -303,12 +444,10 @@ func pickSourceAction(repo string, showSpacer bool) (sourceAction, error) { } // removeSource uninstalls all skills from the given source and removes it from the manifest. -func removeSource(manifest *skill.Manifest, repo string) error { +func removeSource(manifest *skill.Manifest, src skill.Source) error { var names []string - for _, s := range manifest.Skills { - if s.Source == repo { - names = append(names, s.Name) - } + for _, s := range manifest.SkillsFrom(src) { + names = append(names, s.Name) } fmt.Println() @@ -318,120 +457,62 @@ func removeSource(manifest *skill.Manifest, repo string) error { } } - manifest.RemoveSource(repo) + manifest.RemoveSource(src) if err := manifest.Save(); err != nil { return fmt.Errorf("failed to update manifest: %w", err) } - if len(names) == 0 { - terminal.Successf("Removed source %s.", repo) + store, err := skill.OpenStore() + if err == nil { + err = store.Remove(src) } - return nil -} - -// updateSource re-installs all skills from the given source with the latest files. -func updateSource(manifest *skill.Manifest, repo string) error { - var installed []skill.InstalledSkill - for _, s := range manifest.Skills { - if s.Source == repo { - installed = append(installed, s) - } - } - - if len(installed) == 0 { - terminal.Warningf("No skills installed from %s.", repo) - return nil - } - - // Resolve the repo once and reuse it for all skill updates. - dir, cleanup, err := skill.PrepareRepoDir(repo) if err != nil { - return err + terminal.Warningf("Failed to remove cached versions of %s: %v", src.Repo, err) } - defer cleanup() - fmt.Println() - var removed []string - for _, s := range installed { - entry := &skill.SkillEntry{ - Name: s.Name, - Description: s.Description, - Path: s.Path, - } - if err := installSkillEntry(manifest, entry, repo, dir); err != nil { - if errors.Is(err, os.ErrNotExist) { - removed = append(removed, s.Name) - continue - } - terminal.Errorf("Failed to update %q: %v", s.Name, err) - } - } - if len(removed) > 0 { - for _, name := range removed { - skill.Uninstall(name) - manifest.RemoveSkill(name) - } - if err := manifest.Save(); err != nil { - return fmt.Errorf("failed to update manifest after removing skills: %w", err) - } - terminal.Warningf("The following skills were removed from upstream: %s", strings.Join(removed, ", ")) + if len(names) == 0 { + terminal.Successf("Removed source %s.", src.Repo) } return nil } -func installSkillEntry(manifest *skill.Manifest, entry *skill.SkillEntry, repo string, localDir string) error { - spinner := uicli.NewSpinner(). - WithStyle(uicli.SpinnerDots). - WithColor(uicli.CyanColor). - WithMessage(fmt.Sprintf("Installing skill %q from %s...", entry.Name, repo)). - Start() - - var targets []string - var err error - if localDir != "" { - targets, err = skill.InstallFromDir(entry.Name, localDir, entry.Path) - } else { - targets, err = skill.Install(entry.Name, repo, entry.Path) +// forEachInstalledSource applies fn to every source with installed skills, +// continuing past failures so one unreachable source does not block the +// rest. fn returns how many skills it changed; the run ends with a summary. +func forEachInstalledSource(mgr *skill.Manager, verb skill.Verb, fn func(skill.Source) (int, error)) error { + sources := mgr.Manifest.InstalledSources() + if len(sources) == 0 { + terminal.Warning("No skills installed.") + return nil } - if err != nil { - if errors.Is(err, os.ErrNotExist) { - spinner.Stop() - } else { - spinner.Error(fmt.Sprintf("Failed to install skill %q", entry.Name)) + + failed, changed := 0, 0 + for _, src := range sources { + fmt.Println() + n, err := fn(src) + changed += n + if err != nil { + terminal.Errorf("Failed to %s %s: %v", verb, src.Repo, err) + failed++ } - return fmt.Errorf("failed to install skill %q: %w", entry.Name, err) } - if len(targets) == 0 { - spinner.Stop() - terminal.Warning("No skill directories were installed. Neither ~/.claude nor ~/.codex was found.") - return nil + fmt.Println() + if failed > 0 { + return fmt.Errorf("failed to %s %d of %d source(s)", verb, failed, len(sources)) } - - manifest.AddSkill(skill.InstalledSkill{ - Name: entry.Name, - Description: entry.Description, - Source: repo, - Path: entry.Path, - InstalledAt: time.Now(), - }) - if err := manifest.Save(); err != nil { - return fmt.Errorf("skill installed but failed to update manifest: %w", err) + if changed == 0 { + terminal.Success("All skills are up to date.") + return nil } - - spinner.Success(fmt.Sprintf("Installed skill %q to %s", entry.Name, strings.Join(targets, ", "))) + terminal.Successf("%s %d skill(s) from %d source(s).", verbUI[verb].past, changed, len(sources)) return nil } -type installCandidate struct { - entry skill.SkillEntry - label string -} - // selectInstallCandidates returns the repo skills that should be offered for // installation. Already-installed skills are skipped unless force is set, in // which case they are included and their label is marked "(reinstall)". -func selectInstallCandidates(repoSkills []skill.SkillEntry, manifest *skill.Manifest, force bool) []installCandidate { +func selectInstallCandidates(repoSkills []skill.Entry, manifest *skill.Manifest, force bool) []installCandidate { var candidates []installCandidate for _, s := range repoSkills { _, installed := manifest.GetSkill(s.Name) @@ -450,42 +531,44 @@ func selectInstallCandidates(repoSkills []skill.SkillEntry, manifest *skill.Mani return candidates } -// installFromRepo fetches skills from a repo and lets the user pick which to install. -// When force is true, skills that are already installed are kept in the list -// (instead of being filtered out) so they can be reinstalled and overwritten. -func installFromRepo(manifest *skill.Manifest, repo string, force bool) error { - spinner := uicli.NewSpinner(). - WithStyle(uicli.SpinnerDots). - WithColor(uicli.CyanColor). - WithMessage(fmt.Sprintf("Fetching skills from %q...", repo)). - Start() +// installFromRepo fetches skills from a source and lets the user pick which +// to install. When force is true, skills that are already installed are kept +// in the list (instead of being filtered out) so they can be reinstalled and +// overwritten. +func installFromRepo(manifest *skill.Manifest, source string, force bool) error { + src, err := skill.ParseSource(source) + if err != nil { + return err + } + mgr, err := newSkillsManager(manifest) + if err != nil { + return err + } - repoManifest, err := skill.FetchRepoManifest(repo) + spinner := startSpinner(fmt.Sprintf("Fetching skills from %q...", source)) + snap, catalog, err := mgr.Fetch(src) if err != nil { - spinner.Error(fmt.Sprintf("Failed to fetch %q", repo)) + spinner.Error(fmt.Sprintf("Failed to fetch %q", source)) return fmt.Errorf("failed to fetch skills: %w", err) } - - if len(repoManifest.Skills) == 0 { - spinner.Error(fmt.Sprintf("No skills found in %q", repo)) - return fmt.Errorf("repository %q has no skills defined", repo) + if len(catalog.Skills) == 0 { + spinner.Error(fmt.Sprintf("No skills found in %q", source)) + return fmt.Errorf("repository %q has no skills defined", source) } - - spinner.Success(fmt.Sprintf("Found %d skill(s) in %q", len(repoManifest.Skills), repo)) + spinner.Success(fmt.Sprintf("Found %d skill(s) in %q", len(catalog.Skills), source)) // Record the source so it appears in future interactive menus. - manifest.AddSource(repo) + manifest.AddSource(src) if err := manifest.Save(); err != nil { return fmt.Errorf("failed to save skill source: %w", err) } - candidates := selectInstallCandidates(repoManifest.Skills, manifest, force) + candidates := selectInstallCandidates(catalog.Skills, manifest, force) if len(candidates) == 0 { terminal.Info("All skills from this repository are already installed. Use --force to reinstall them.") return nil } - // Multi-select skills to install. options := make([]string, len(candidates)) for i, c := range candidates { options[i] = c.label @@ -505,66 +588,35 @@ func installFromRepo(manifest *skill.Manifest, repo string, force bool) error { return nil } - // Resolve the repo once and reuse it for all skill installations. - dir, cleanup, err := skill.PrepareRepoDir(repo) - if err != nil { - return err - } - defer cleanup() - - // Install each selected skill. - fmt.Println() + entries := make([]skill.Entry, 0, len(selectedIdxs)) for _, idx := range selectedIdxs { - entry := candidates[idx].entry - if err := installSkillEntry(manifest, &entry, repo, dir); err != nil { - terminal.Errorf("Failed to install %q: %v", entry.Name, err) - } + entries = append(entries, candidates[idx].entry) } + // Per-skill failures are already reported through the progress events. + fmt.Println() + _, _ = mgr.Install(snap, entries) return nil } -var skillsUninstallCmd = &cobra.Command{ - Use: "uninstall [skill-name]", - Short: "Uninstall a previously installed skill", - Args: cobra.MaximumNArgs(1), - RunE: func(cmd *cobra.Command, args []string) error { - manifest, err := skill.LoadManifest() - if err != nil { - return fmt.Errorf("failed to load skills manifest: %w", err) - } - - if len(args) == 0 { - return interactiveUninstall(manifest) - } - - return uninstallByName(manifest, args[0]) - }, -} - func uninstallByName(manifest *skill.Manifest, name string) error { if _, exists := manifest.GetSkill(name); !exists { return fmt.Errorf("skill %q is not installed", name) } + targets, err := skill.DetectTargets() + if err != nil { + return err + } + mgr := &skill.Manager{Manifest: manifest, Targets: targets} - spinner := uicli.NewSpinner(). - WithStyle(uicli.SpinnerDots). - WithColor(uicli.CyanColor). - WithMessage(fmt.Sprintf("Removing skill %q...", name)). - Start() - - targets, err := skill.Uninstall(name) + spinner := startSpinner(fmt.Sprintf("Removing skill %q...", name)) + removed, err := mgr.Uninstall(name) if err != nil { spinner.Error(fmt.Sprintf("Failed to remove skill %q", name)) return fmt.Errorf("failed to remove skill %q: %w", name, err) } - manifest.RemoveSkill(name) - if err := manifest.Save(); err != nil { - return fmt.Errorf("skill removed but failed to update manifest: %w", err) - } - - spinner.Success(fmt.Sprintf("Removed skill %q from %s", name, strings.Join(targets, ", "))) + spinner.Success(fmt.Sprintf("Removed skill %q from %s", name, strings.Join(removed, ", "))) return nil } @@ -576,11 +628,7 @@ func interactiveUninstall(manifest *skill.Manifest) error { options := make([]string, len(manifest.Skills)) for i, s := range manifest.Skills { - label := s.Name - if s.Description != "" { - label = fmt.Sprintf("%s — %s", s.Name, uicli.TruncateString(s.Description, 60)) - } - options[i] = label + options[i] = s.Name } showSpacer := true diff --git a/cmd/skills_force_test.go b/cmd/skills_force_test.go index 4da754a..f1ec965 100644 --- a/cmd/skills_force_test.go +++ b/cmd/skills_force_test.go @@ -13,7 +13,7 @@ func TestSelectInstallCandidatesSkipsInstalledByDefault(t *testing.T) { {Name: "alpha", Source: "owner/repo"}, }, } - repoSkills := []skill.SkillEntry{ + repoSkills := []skill.Entry{ {Name: "alpha", Description: "first"}, {Name: "beta", Description: "second"}, } @@ -36,7 +36,7 @@ func TestSelectInstallCandidatesForceIncludesInstalled(t *testing.T) { {Name: "alpha", Source: "owner/repo"}, }, } - repoSkills := []skill.SkillEntry{ + repoSkills := []skill.Entry{ {Name: "alpha", Description: "first"}, {Name: "beta", Description: "second"}, } diff --git a/cmd/skills_interactive_test.go b/cmd/skills_interactive_test.go index 1afc494..c11d343 100644 --- a/cmd/skills_interactive_test.go +++ b/cmd/skills_interactive_test.go @@ -312,9 +312,3 @@ func stubSkillPrompts(t *testing.T) func() { skillsActionRunner = origRunner } } - -func TestValidateSkillRepoSourceAllowsLocalCurrentDir(t *testing.T) { - if err := validateSkillRepoSource("."); err != nil { - t.Fatalf("validateSkillRepoSource(.) error = %v", err) - } -} diff --git a/go.mod b/go.mod index fcba405..42e0778 100644 --- a/go.mod +++ b/go.mod @@ -1,10 +1,11 @@ module github.com/git-hulk/clime -go 1.25 +go 1.25.0 require ( github.com/alperdrsnn/clime v1.1.2 github.com/spf13/cobra v1.10.2 + golang.org/x/mod v0.40.0 golang.org/x/term v0.33.0 gopkg.in/yaml.v3 v3.0.1 ) diff --git a/go.sum b/go.sum index 8102d8a..d4905d1 100644 --- a/go.sum +++ b/go.sum @@ -9,6 +9,8 @@ github.com/spf13/cobra v1.10.2/go.mod h1:7C1pvHqHw5A4vrJfjNwvOdzYu0Gml16OCs2GRiT github.com/spf13/pflag v1.0.9 h1:9exaQaMOCwffKiiiYk6/BndUBv+iRViNW+4lEMi0PvY= github.com/spf13/pflag v1.0.9/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg= +golang.org/x/mod v0.40.0 h1:hUv+3cXcdRHz08UmSiOob7sadHig73uo5bkXxQ/tvUs= +golang.org/x/mod v0.40.0/go.mod h1:0/weTWkPWGBikyTWAX3dkjVztMmBA5hM0DH6BElSupE= golang.org/x/sys v0.34.0 h1:H5Y5sJ2L2JRdyv7ROF1he/lPdvFsd0mJHFw2ThKHxLA= golang.org/x/sys v0.34.0/go.mod h1:BJP2sWEmIv4KK5OTEluFJCKSidICx8ciO85XgH3Ak8k= golang.org/x/term v0.33.0 h1:NuFncQrRcaRvVmgRkvM3j/F00gWIAlcmlB8ACEKmGIg= diff --git a/internal/skill/catalog.go b/internal/skill/catalog.go new file mode 100644 index 0000000..3caf9f3 --- /dev/null +++ b/internal/skill/catalog.go @@ -0,0 +1,201 @@ +package skill + +import ( + "encoding/json" + "fmt" + "os" + "path/filepath" + "strings" + + "gopkg.in/yaml.v3" +) + +// Entry is one skill as offered by a source: its name, description, and +// path within a snapshot. +type Entry struct { + Name string `yaml:"name"` + Description string `yaml:"description,omitempty"` + Path string `yaml:"path"` + Tags []string `yaml:"tags,omitempty"` +} + +// Catalog lists the skills a source offers at one version. +type Catalog struct { + Skills []Entry `yaml:"skills"` +} + +// marketplaceFile represents the .claude-plugin/marketplace.json format. +type marketplaceFile struct { + Plugins []marketplacePlugin `json:"plugins"` +} + +type marketplacePlugin struct { + Name string `json:"name"` + Description string `json:"description"` + Source string `json:"source"` + Skills []string `json:"skills"` +} + +// pluginFile represents the .claude-plugin/plugin.json format. +type pluginFile struct { + Name string `json:"name"` + Skills string `json:"skills"` +} + +// skillFrontmatter holds the YAML frontmatter from a SKILL.md file. +type skillFrontmatter struct { + Name string `yaml:"name"` + Description string `yaml:"description"` +} + +// Find returns the entry with the given skill name. +func (c *Catalog) Find(name string) (Entry, bool) { + for _, entry := range c.Skills { + if entry.Name == name { + return entry, true + } + } + return Entry{}, false +} + +// ReadCatalog reads the skill catalog of a checkout directory, trying +// skills.yaml, skills.yml, .claude-plugin/marketplace.json and +// .claude-plugin/plugin.json in that order. +func ReadCatalog(dir string) (*Catalog, error) { + var data []byte + var err error + for _, name := range []string{"skills.yaml", "skills.yml"} { + data, err = os.ReadFile(filepath.Join(dir, name)) + if err == nil { + break + } + } + if err == nil { + var catalog Catalog + if err := yaml.Unmarshal(data, &catalog); err != nil { + return nil, fmt.Errorf("failed to parse skills.yaml: %w", err) + } + return &catalog, nil + } + + if catalog, err := parseMarketplaceManifest(dir); err == nil && len(catalog.Skills) > 0 { + return catalog, nil + } + + if catalog, err := parsePluginManifest(dir); err == nil && len(catalog.Skills) > 0 { + return catalog, nil + } + + return nil, fmt.Errorf("no skills catalog found: tried skills.yaml, skills.yml, .claude-plugin/marketplace.json, and .claude-plugin/plugin.json") +} + +// parseMarketplaceManifest reads .claude-plugin/marketplace.json and builds +// a catalog by reading each skill's SKILL.md frontmatter. +func parseMarketplaceManifest(dir string) (*Catalog, error) { + data, err := os.ReadFile(filepath.Join(dir, ".claude-plugin", "marketplace.json")) + if err != nil { + return nil, err + } + + var mf marketplaceFile + if err := json.Unmarshal(data, &mf); err != nil { + return nil, fmt.Errorf("failed to parse marketplace.json: %w", err) + } + + var catalog Catalog + seen := make(map[string]bool) + for _, plugin := range mf.Plugins { + sourceDir := strings.TrimPrefix(plugin.Source, "./") + for _, skillPath := range plugin.Skills { + skillPath = strings.TrimPrefix(skillPath, "./") + if sourceDir != "" { + skillPath = filepath.Join(sourceDir, skillPath) + } + if seen[skillPath] { + continue + } + seen[skillPath] = true + + catalog.Skills = append(catalog.Skills, entryFromSkillDir(dir, skillPath)) + } + } + return &catalog, nil +} + +// parsePluginManifest reads .claude-plugin/plugin.json and discovers skills +// by scanning the skills directory it references. +func parsePluginManifest(dir string) (*Catalog, error) { + data, err := os.ReadFile(filepath.Join(dir, ".claude-plugin", "plugin.json")) + if err != nil { + return nil, err + } + + var pf pluginFile + if err := json.Unmarshal(data, &pf); err != nil { + return nil, fmt.Errorf("failed to parse plugin.json: %w", err) + } + if pf.Skills == "" { + return nil, fmt.Errorf("plugin.json has no skills directory") + } + + skillsDir := strings.TrimPrefix(pf.Skills, "./") + entries, err := os.ReadDir(filepath.Join(dir, skillsDir)) + if err != nil { + return nil, fmt.Errorf("failed to read skills directory: %w", err) + } + + var catalog Catalog + for _, e := range entries { + if !e.IsDir() { + continue + } + catalog.Skills = append(catalog.Skills, entryFromSkillDir(dir, filepath.Join(skillsDir, e.Name()))) + } + return &catalog, nil +} + +// entryFromSkillDir builds a catalog entry for a skill directory, taking +// name and description from SKILL.md frontmatter and falling back to the +// directory's basename. +func entryFromSkillDir(dir, skillPath string) Entry { + entry := Entry{Path: skillPath} + if fm, err := readSkillFrontmatter(filepath.Join(dir, skillPath, "SKILL.md")); err == nil { + entry.Name = fm.Name + entry.Description = fm.Description + } + if entry.Name == "" { + entry.Name = filepath.Base(skillPath) + } + return entry +} + +// parseSkillFrontmatter parses the YAML frontmatter between --- markers +// from SKILL.md content. +func parseSkillFrontmatter(data []byte) (*skillFrontmatter, error) { + lines := strings.Split(string(data), "\n") + if len(lines) == 0 || strings.TrimSpace(lines[0]) != "---" { + return nil, fmt.Errorf("no frontmatter found") + } + var fmLines []string + for _, line := range lines[1:] { + if strings.TrimSpace(line) == "---" { + break + } + fmLines = append(fmLines, line) + } + var fm skillFrontmatter + if err := yaml.Unmarshal([]byte(strings.Join(fmLines, "\n")), &fm); err != nil { + return nil, err + } + return &fm, nil +} + +// readSkillFrontmatter reads and parses the YAML frontmatter from a +// SKILL.md file on disk. +func readSkillFrontmatter(path string) (*skillFrontmatter, error) { + data, err := os.ReadFile(path) + if err != nil { + return nil, err + } + return parseSkillFrontmatter(data) +} diff --git a/internal/skill/catalog_test.go b/internal/skill/catalog_test.go new file mode 100644 index 0000000..227df32 --- /dev/null +++ b/internal/skill/catalog_test.go @@ -0,0 +1,181 @@ +package skill + +import ( + "path/filepath" + "testing" + + "gopkg.in/yaml.v3" +) + +func TestParseCatalogYAML(t *testing.T) { + t.Parallel() + + yamlContent := `skills: + - name: docker-helper + description: Docker management skill + path: skills/docker-helper + tags: + - devops + - name: git-wizard + description: Git workflow automation + path: skills/git-wizard +` + var catalog Catalog + if err := yaml.Unmarshal([]byte(yamlContent), &catalog); err != nil { + t.Fatalf("failed to parse yaml: %v", err) + } + + if len(catalog.Skills) != 2 { + t.Fatalf("expected 2 skills, got %d", len(catalog.Skills)) + } + if catalog.Skills[0].Name != "docker-helper" { + t.Fatalf("expected first skill name docker-helper, got %s", catalog.Skills[0].Name) + } + if catalog.Skills[0].Path != "skills/docker-helper" { + t.Fatalf("expected path skills/docker-helper, got %s", catalog.Skills[0].Path) + } + if len(catalog.Skills[0].Tags) != 1 || catalog.Skills[0].Tags[0] != "devops" { + t.Fatalf("unexpected tags: %v", catalog.Skills[0].Tags) + } + + entry, ok := catalog.Find("git-wizard") + if !ok || entry.Path != "skills/git-wizard" { + t.Fatalf("Find(git-wizard) = (%+v, %v)", entry, ok) + } + if _, ok := catalog.Find("missing"); ok { + t.Fatal("Find(missing) should report absence") + } +} + +func TestReadCatalogFromSkillsYAML(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "skills.yaml"), `skills: + - name: my-skill + description: A test skill + path: skills/my-skill +`) + writeFile(t, filepath.Join(dir, "skills", "my-skill", "SKILL.md"), "# My Skill") + + catalog, err := ReadCatalog(dir) + if err != nil { + t.Fatalf("ReadCatalog() error = %v", err) + } + if len(catalog.Skills) != 1 || catalog.Skills[0].Name != "my-skill" { + t.Fatalf("catalog = %+v, want the my-skill entry", catalog.Skills) + } +} + +func TestReadCatalogFromMarketplaceJSON(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + writeFile(t, filepath.Join(dir, ".claude-plugin", "marketplace.json"), `{ + "plugins": [ + { + "name": "Test Plugin", + "description": "A test plugin", + "skills": ["./skills/skill-a", "./skills/skill-b"] + } + ] +}`) + for _, name := range []string{"skill-a", "skill-b"} { + content := "---\nname: " + name + "\ndescription: " + name + " desc\n---\n# " + name + writeFile(t, filepath.Join(dir, "skills", name, "SKILL.md"), content) + } + + catalog, err := ReadCatalog(dir) + if err != nil { + t.Fatalf("ReadCatalog() error = %v", err) + } + if len(catalog.Skills) != 2 { + t.Fatalf("expected 2 skills, got %d", len(catalog.Skills)) + } + if catalog.Skills[0].Name != "skill-a" || catalog.Skills[0].Path != "skills/skill-a" { + t.Fatalf("first entry = %+v", catalog.Skills[0]) + } + if catalog.Skills[1].Name != "skill-b" { + t.Fatalf("second entry = %+v", catalog.Skills[1]) + } +} + +func TestReadCatalogFromPluginJSON(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + writeFile(t, filepath.Join(dir, ".claude-plugin", "plugin.json"), `{"name": "test-plugin", "skills": "./.claude/skills"}`) + for _, name := range []string{"skill-x", "skill-y"} { + content := "---\nname: " + name + "\ndescription: " + name + " desc\n---\n# " + name + writeFile(t, filepath.Join(dir, ".claude", "skills", name, "SKILL.md"), content) + } + + catalog, err := ReadCatalog(dir) + if err != nil { + t.Fatalf("ReadCatalog() error = %v", err) + } + if len(catalog.Skills) != 2 { + t.Fatalf("expected 2 skills, got %d", len(catalog.Skills)) + } + names := map[string]bool{} + for _, s := range catalog.Skills { + names[s.Name] = true + } + if !names["skill-x"] || !names["skill-y"] { + t.Fatalf("expected skill-x and skill-y, got %v", names) + } +} + +func TestReadCatalogPluginJSONFallbackFromEmptyMarketplace(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + // marketplace.json with plugins that have no skills arrays. + writeFile(t, filepath.Join(dir, ".claude-plugin", "marketplace.json"), + `{"plugins": [{"name": "test", "description": "test plugin", "source": "./"}]}`) + writeFile(t, filepath.Join(dir, ".claude-plugin", "plugin.json"), + `{"name": "test", "skills": "./.claude/skills"}`) + writeFile(t, filepath.Join(dir, ".claude", "skills", "my-skill", "SKILL.md"), + "---\nname: my-skill\ndescription: A skill\n---\n# My Skill") + + catalog, err := ReadCatalog(dir) + if err != nil { + t.Fatalf("ReadCatalog() error = %v", err) + } + if len(catalog.Skills) != 1 || catalog.Skills[0].Name != "my-skill" { + t.Fatalf("catalog = %+v, want my-skill via plugin.json fallback", catalog.Skills) + } +} + +func TestReadCatalogEmptyNameFallsBackToDirName(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + writeFile(t, filepath.Join(dir, ".claude-plugin", "marketplace.json"), + `{"plugins": [{"name": "P", "skills": ["./skills/my-skill"]}]}`) + // Frontmatter with no name field. + writeFile(t, filepath.Join(dir, "skills", "my-skill", "SKILL.md"), + "---\ndescription: has desc but no name\n---\n# Skill") + + catalog, err := ReadCatalog(dir) + if err != nil { + t.Fatalf("ReadCatalog() error = %v", err) + } + if len(catalog.Skills) != 1 { + t.Fatalf("expected 1 skill, got %d", len(catalog.Skills)) + } + if catalog.Skills[0].Name != "my-skill" { + t.Fatalf("expected name my-skill (from directory), got %q", catalog.Skills[0].Name) + } + if catalog.Skills[0].Description != "has desc but no name" { + t.Fatalf("expected description preserved, got %q", catalog.Skills[0].Description) + } +} + +func TestReadCatalogNoManifest(t *testing.T) { + t.Parallel() + + if _, err := ReadCatalog(t.TempDir()); err == nil { + t.Fatal("ReadCatalog() should fail when no catalog file exists") + } +} diff --git a/internal/skill/github.go b/internal/skill/github.go deleted file mode 100644 index 997996b..0000000 --- a/internal/skill/github.go +++ /dev/null @@ -1,418 +0,0 @@ -package skill - -import ( - "encoding/json" - "fmt" - "os" - "os/exec" - "path/filepath" - "strings" - - "gopkg.in/yaml.v3" -) - -// SkillEntry describes a skill in a repo's skills.yaml manifest. -type SkillEntry struct { - Name string `yaml:"name"` - Description string `yaml:"description,omitempty"` - Path string `yaml:"path"` - Tags []string `yaml:"tags,omitempty"` -} - -// RepoManifest is the top-level structure of a repo's skills.yaml. -type RepoManifest struct { - Skills []SkillEntry `yaml:"skills"` -} - -// LocalRepoDir resolves repo to a local directory when it already exists on disk. -// It returns the absolute path, whether the repo was resolved locally, and any error. -func LocalRepoDir(repo string) (string, bool, error) { - if repo == "" { - return "", false, nil - } - if strings.HasPrefix(repo, "https://") || strings.HasPrefix(repo, "git@") { - return "", false, nil - } - - info, err := os.Stat(repo) - if err == nil { - if !info.IsDir() { - return "", false, fmt.Errorf("local repo path %q is not a directory", repo) - } - dir, err := filepath.Abs(repo) - if err != nil { - return "", false, fmt.Errorf("failed to resolve local repo path %q: %w", repo, err) - } - return dir, true, nil - } - if !os.IsNotExist(err) { - return "", false, fmt.Errorf("failed to inspect local repo path %q: %w", repo, err) - } - - if filepath.IsAbs(repo) || repo == "." || repo == ".." || strings.HasPrefix(repo, "./") || strings.HasPrefix(repo, "../") { - return "", false, fmt.Errorf("local repo path %q does not exist", repo) - } - - return "", false, nil -} - -// sourceRepoDir returns the persistent local directory for a cached source repository. -// The directory is under ~/.clime/sources//. -func sourceRepoDir(repo string) (string, error) { - home, err := os.UserHomeDir() - if err != nil { - return "", fmt.Errorf("failed to get home directory: %w", err) - } - name := repo - name = strings.TrimPrefix(name, "https://") - name = strings.TrimPrefix(name, "http://") - name = strings.TrimPrefix(name, "git@") - name = strings.TrimSuffix(name, ".git") - name = strings.ReplaceAll(name, ":", "/") - return filepath.Join(home, ".clime", "sources", name), nil -} - -// RemoveSourceDir removes the persistent local cache for a source repository. -func RemoveSourceDir(repo string) error { - dir, err := sourceRepoDir(repo) - if err != nil { - return err - } - return os.RemoveAll(dir) -} - -// repoToCloneURL converts an "owner/repo" shorthand to a git clone URL. -// Full URLs (https://, git@) and local paths (absolute or relative) are returned as-is. -func repoToCloneURL(repo string) string { - if strings.HasPrefix(repo, "https://") || strings.HasPrefix(repo, "git@") { - return repo - } - if strings.HasPrefix(repo, "/") || strings.HasPrefix(repo, "./") || strings.HasPrefix(repo, "../") { - return repo - } - return fmt.Sprintf("https://github.com/%s.git", repo) -} - -// cloneViaGH attempts to clone a repo using the GitHub CLI (gh). -func cloneViaGH(repo, dir string) error { - cmd := exec.Command("gh", "repo", "clone", repo, dir, "--", "--depth", "1") - if out, err := cmd.CombinedOutput(); err != nil { - os.RemoveAll(dir) - return fmt.Errorf("gh repo clone failed: %w\n%s", err, out) - } - return nil -} - -// cloneViaGit performs a shallow clone (depth 1) using git directly. -func cloneViaGit(repo, dir string) error { - url := repoToCloneURL(repo) - cmd := exec.Command("git", "clone", "--depth", "1", url, dir) - cmd.Env = append(os.Environ(), "GIT_TERMINAL_PROMPT=0") - if out, err := cmd.CombinedOutput(); err != nil { - os.RemoveAll(dir) - return fmt.Errorf("git clone failed: %w\n%s", err, out) - } - return nil -} - -// cloneRepoTo performs a shallow clone (depth 1) into the specified directory. -// It first attempts to use the GitHub CLI (gh) for cloning, falling back to -// git if gh is unavailable or the clone fails. -func cloneRepoTo(repo, dir string) error { - if err := os.MkdirAll(filepath.Dir(dir), 0o755); err != nil { - return fmt.Errorf("failed to create parent directory: %w", err) - } - - if err := cloneViaGH(repo, dir); err == nil { - return nil - } - return cloneViaGit(repo, dir) -} - -// pullRepo updates an existing git repository by pulling the latest changes. -func pullRepo(dir string) error { - cmd := exec.Command("git", "pull") - cmd.Dir = dir - cmd.Env = append(os.Environ(), "GIT_TERMINAL_PROMPT=0") - if out, err := cmd.CombinedOutput(); err != nil { - return fmt.Errorf("git pull failed: %w\n%s", err, out) - } - return nil -} - -// PrepareRepoDir returns a directory that can be read for skill files. -// Existing local repos are reused directly; remote repos are cached in -// ~/.clime/sources/ and updated with git pull on subsequent calls. -// The returned cleanup function must always be called by the caller. -func PrepareRepoDir(repo string) (string, func(), error) { - if dir, ok, err := LocalRepoDir(repo); err != nil { - return "", nil, err - } else if ok { - return dir, func() {}, nil - } - - srcDir, err := sourceRepoDir(repo) - if err != nil { - return "", nil, err - } - - if info, err := os.Stat(srcDir); err == nil && info.IsDir() { - // Source exists locally, update it with the latest changes. - if err := pullRepo(srcDir); err != nil { - return "", nil, fmt.Errorf("failed to update %s: %w", repo, err) - } - return srcDir, func() {}, nil - } - - // Clone to persistent source directory. - if err := cloneRepoTo(repo, srcDir); err != nil { - return "", nil, fmt.Errorf("failed to clone %s: %w", repo, err) - } - return srcDir, func() {}, nil -} - -// FetchRepoManifest fetches skills from a repo's manifest. -// The repo is always cloned (or updated) into ~/.clime/sources/ so -// the source is cached locally for subsequent operations. -// Existing local paths are read directly without cloning. -func FetchRepoManifest(repo string) (*RepoManifest, error) { - dir, cleanup, err := PrepareRepoDir(repo) - if err != nil { - return nil, err - } - defer cleanup() - - return readRepoManifestFromDir(dir, repo) -} - -func readRepoManifestFromDir(dir, repo string) (*RepoManifest, error) { - // Try skills.yaml / skills.yml first. - var data []byte - var err error - for _, name := range []string{"skills.yaml", "skills.yml"} { - data, err = os.ReadFile(filepath.Join(dir, name)) - if err == nil { - break - } - } - if err == nil { - var manifest RepoManifest - if err := yaml.Unmarshal(data, &manifest); err != nil { - return nil, fmt.Errorf("failed to parse skills.yaml: %w", err) - } - return &manifest, nil - } - - // Fall back to .claude-plugin/marketplace.json. - if manifest, err := parseMarketplaceManifest(dir); err == nil && len(manifest.Skills) > 0 { - return manifest, nil - } - - // Fall back to .claude-plugin/plugin.json. - if manifest, err := parsePluginManifest(dir); err == nil && len(manifest.Skills) > 0 { - return manifest, nil - } - - return nil, fmt.Errorf("no skills manifest found in %s: tried skills.yaml, skills.yml, .claude-plugin/marketplace.json, and .claude-plugin/plugin.json", repo) -} - -// marketplaceFile represents the .claude-plugin/marketplace.json format. -type marketplaceFile struct { - Plugins []marketplacePlugin `json:"plugins"` -} - -type marketplacePlugin struct { - Name string `json:"name"` - Description string `json:"description"` - Source string `json:"source"` - Skills []string `json:"skills"` -} - -// parseMarketplaceManifest reads .claude-plugin/marketplace.json from a local directory -// and builds a RepoManifest by reading each skill's SKILL.md frontmatter. -func parseMarketplaceManifest(dir string) (*RepoManifest, error) { - data, err := os.ReadFile(filepath.Join(dir, ".claude-plugin", "marketplace.json")) - if err != nil { - return nil, err - } - - var mf marketplaceFile - if err := json.Unmarshal(data, &mf); err != nil { - return nil, fmt.Errorf("failed to parse marketplace.json: %w", err) - } - - var manifest RepoManifest - seen := make(map[string]bool) - for _, plugin := range mf.Plugins { - sourceDir := strings.TrimPrefix(plugin.Source, "./") - for _, skillPath := range plugin.Skills { - skillPath = strings.TrimPrefix(skillPath, "./") - if sourceDir != "" { - skillPath = filepath.Join(sourceDir, skillPath) - } - if seen[skillPath] { - continue - } - seen[skillPath] = true - - entry := SkillEntry{Path: skillPath} - if fm, err := readSkillFrontmatter(filepath.Join(dir, skillPath, "SKILL.md")); err == nil { - entry.Name = fm.Name - entry.Description = fm.Description - } - if entry.Name == "" { - entry.Name = filepath.Base(skillPath) - } - manifest.Skills = append(manifest.Skills, entry) - } - } - return &manifest, nil -} - -// pluginFile represents the .claude-plugin/plugin.json format. -type pluginFile struct { - Name string `json:"name"` - Skills string `json:"skills"` -} - -// parsePluginManifest reads .claude-plugin/plugin.json from a local directory -// and discovers skills by scanning the skills directory it references. -func parsePluginManifest(dir string) (*RepoManifest, error) { - data, err := os.ReadFile(filepath.Join(dir, ".claude-plugin", "plugin.json")) - if err != nil { - return nil, err - } - - var pf pluginFile - if err := json.Unmarshal(data, &pf); err != nil { - return nil, fmt.Errorf("failed to parse plugin.json: %w", err) - } - if pf.Skills == "" { - return nil, fmt.Errorf("plugin.json has no skills directory") - } - - skillsDir := strings.TrimPrefix(pf.Skills, "./") - entries, err := os.ReadDir(filepath.Join(dir, skillsDir)) - if err != nil { - return nil, fmt.Errorf("failed to read skills directory: %w", err) - } - - var manifest RepoManifest - for _, e := range entries { - if !e.IsDir() { - continue - } - skillPath := filepath.Join(skillsDir, e.Name()) - entry := SkillEntry{Path: skillPath} - if fm, err := readSkillFrontmatter(filepath.Join(dir, skillPath, "SKILL.md")); err == nil { - entry.Name = fm.Name - entry.Description = fm.Description - } - if entry.Name == "" { - entry.Name = e.Name() - } - manifest.Skills = append(manifest.Skills, entry) - } - return &manifest, nil -} - -// skillFrontmatter holds the YAML frontmatter from a SKILL.md file. -type skillFrontmatter struct { - Name string `yaml:"name"` - Description string `yaml:"description"` -} - -// parseSkillFrontmatter parses the YAML frontmatter between --- markers from SKILL.md content. -func parseSkillFrontmatter(data []byte) (*skillFrontmatter, error) { - lines := strings.Split(string(data), "\n") - if len(lines) == 0 || strings.TrimSpace(lines[0]) != "---" { - return nil, fmt.Errorf("no frontmatter found") - } - var fmLines []string - for _, line := range lines[1:] { - if strings.TrimSpace(line) == "---" { - break - } - fmLines = append(fmLines, line) - } - var fm skillFrontmatter - if err := yaml.Unmarshal([]byte(strings.Join(fmLines, "\n")), &fm); err != nil { - return nil, err - } - return &fm, nil -} - -// readSkillFrontmatter reads and parses the YAML frontmatter from a SKILL.md file on disk. -func readSkillFrontmatter(path string) (*skillFrontmatter, error) { - data, err := os.ReadFile(path) - if err != nil { - return nil, err - } - return parseSkillFrontmatter(data) -} - -// CloneRepo clones (or updates) a repo into ~/.clime/sources/ and returns the path. -func CloneRepo(repo string) (string, error) { - dir, _, err := PrepareRepoDir(repo) - return dir, err -} - -// ReadSkillFilesFromDir reads all files under skillPath from a local directory. -// Returns a map of relative file paths to their contents. -func ReadSkillFilesFromDir(dir, skillPath string) (map[string][]byte, error) { - root := filepath.Join(dir, skillPath) - info, err := os.Stat(root) - if err != nil { - return nil, fmt.Errorf("skill path %q not found: %w", skillPath, err) - } - - files := make(map[string][]byte) - if !info.IsDir() { - // Single file. - data, err := os.ReadFile(root) - if err != nil { - return nil, err - } - files[filepath.Base(root)] = data - return files, nil - } - - err = filepath.Walk(root, func(path string, fi os.FileInfo, err error) error { - if err != nil { - return err - } - if fi.IsDir() { - if fi.Name() == ".git" { - return filepath.SkipDir - } - return nil - } - rel, err := filepath.Rel(root, path) - if err != nil { - return err - } - data, err := os.ReadFile(path) - if err != nil { - return err - } - files[rel] = data - return nil - }) - if err != nil { - return nil, fmt.Errorf("failed to read skill files: %w", err) - } - - return files, nil -} - -// CloneAndReadSkillFiles clones the repo and reads all files under skillPath. -// Returns a map of relative file paths to their contents. -func CloneAndReadSkillFiles(repo, skillPath string) (map[string][]byte, error) { - dir, cleanup, err := PrepareRepoDir(repo) - if err != nil { - return nil, err - } - defer cleanup() - - return ReadSkillFilesFromDir(dir, skillPath) -} diff --git a/internal/skill/github_test.go b/internal/skill/github_test.go deleted file mode 100644 index e973d53..0000000 --- a/internal/skill/github_test.go +++ /dev/null @@ -1,268 +0,0 @@ -package skill - -import ( - "os" - "path/filepath" - "strings" - "testing" - - "gopkg.in/yaml.v3" -) - -func TestRepoToCloneURL(t *testing.T) { - t.Parallel() - - tests := []struct { - input string - want string - }{ - {"owner/repo", "https://github.com/owner/repo.git"}, - {"https://github.com/owner/repo.git", "https://github.com/owner/repo.git"}, - {"https://gitlab.com/group/repo.git", "https://gitlab.com/group/repo.git"}, - {"git@github.com:owner/repo.git", "git@github.com:owner/repo.git"}, - {"/tmp/local-repo", "/tmp/local-repo"}, - {"./relative-repo", "./relative-repo"}, - {"../parent-repo", "../parent-repo"}, - } - - for _, tt := range tests { - got := repoToCloneURL(tt.input) - if got != tt.want { - t.Errorf("repoToCloneURL(%q) = %q, want %q", tt.input, got, tt.want) - } - } -} - -func TestParseRepoManifestYAML(t *testing.T) { - t.Parallel() - - yamlContent := `skills: - - name: docker-helper - description: Docker management skill - path: skills/docker-helper - tags: - - devops - - name: git-wizard - description: Git workflow automation - path: skills/git-wizard -` - var manifest RepoManifest - if err := yaml.Unmarshal([]byte(yamlContent), &manifest); err != nil { - t.Fatalf("failed to parse yaml: %v", err) - } - - if len(manifest.Skills) != 2 { - t.Fatalf("expected 2 skills, got %d", len(manifest.Skills)) - } - if manifest.Skills[0].Name != "docker-helper" { - t.Fatalf("expected first skill name docker-helper, got %s", manifest.Skills[0].Name) - } - if manifest.Skills[0].Path != "skills/docker-helper" { - t.Fatalf("expected path skills/docker-helper, got %s", manifest.Skills[0].Path) - } - if manifest.Skills[1].Name != "git-wizard" { - t.Fatalf("expected second skill name git-wizard, got %s", manifest.Skills[1].Name) - } - if len(manifest.Skills[0].Tags) != 1 || manifest.Skills[0].Tags[0] != "devops" { - t.Fatalf("unexpected tags: %v", manifest.Skills[0].Tags) - } -} - -func TestCloneAndReadSkillFilesFromLocal(t *testing.T) { - t.Parallel() - - // Create a fake skill directory to simulate a cloned repo. - dir := t.TempDir() - skillDir := filepath.Join(dir, "my-skill") - agentsDir := filepath.Join(skillDir, "agents") - if err := os.MkdirAll(agentsDir, 0o755); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte("# Skill"), 0o644); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(filepath.Join(agentsDir, "openai.yaml"), []byte("model: gpt-4"), 0o644); err != nil { - t.Fatal(err) - } - - // Directly test the file-walking logic by calling filepath.Walk - // (CloneAndReadSkillFiles wraps cloneRepo + walk; we test the walk here). - files := make(map[string][]byte) - root := skillDir - err := filepath.Walk(root, func(path string, fi os.FileInfo, err error) error { - if err != nil { - return err - } - if fi.IsDir() { - return nil - } - rel, err := filepath.Rel(root, path) - if err != nil { - return err - } - data, err := os.ReadFile(path) - if err != nil { - return err - } - files[rel] = data - return nil - }) - if err != nil { - t.Fatalf("walk failed: %v", err) - } - - if len(files) != 2 { - t.Fatalf("expected 2 files, got %d", len(files)) - } - if string(files["SKILL.md"]) != "# Skill" { - t.Fatalf("unexpected SKILL.md content: %q", files["SKILL.md"]) - } - if string(files[filepath.Join("agents", "openai.yaml")]) != "model: gpt-4" { - t.Fatalf("unexpected agents/openai.yaml content: %q", files[filepath.Join("agents", "openai.yaml")]) - } -} - -func TestLocalRepoDir(t *testing.T) { - t.Parallel() - - dir := t.TempDir() - - got, ok, err := LocalRepoDir(dir) - if err != nil { - t.Fatalf("LocalRepoDir() error = %v", err) - } - if !ok { - t.Fatal("LocalRepoDir() did not detect local repo") - } - - want, err := filepath.Abs(dir) - if err != nil { - t.Fatalf("filepath.Abs() error = %v", err) - } - if got != want { - t.Fatalf("LocalRepoDir() = %q, want %q", got, want) - } -} - -func TestFetchRepoManifestFromLocalRepoWithoutGit(t *testing.T) { - dir := t.TempDir() - skillsDir := filepath.Join(dir, "skills", "alpha") - if err := os.MkdirAll(skillsDir, 0o755); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(filepath.Join(dir, "skills.yaml"), []byte("skills:\n - name: alpha\n path: skills/alpha\n"), 0o644); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(filepath.Join(skillsDir, "SKILL.md"), []byte("# Alpha"), 0o644); err != nil { - t.Fatal(err) - } - - t.Setenv("PATH", "") - - manifest, err := FetchRepoManifest(dir) - if err != nil { - t.Fatalf("FetchRepoManifest() error = %v", err) - } - if len(manifest.Skills) != 1 { - t.Fatalf("expected 1 skill, got %d", len(manifest.Skills)) - } - if manifest.Skills[0].Name != "alpha" { - t.Fatalf("skill name = %q, want %q", manifest.Skills[0].Name, "alpha") - } -} - -func TestCloneAndReadSkillFilesFromLocalRepoWithoutGit(t *testing.T) { - dir := t.TempDir() - skillDir := filepath.Join(dir, "skills", "alpha") - if err := os.MkdirAll(skillDir, 0o755); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte("# Alpha"), 0o644); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(filepath.Join(skillDir, "extra.txt"), []byte("extra"), 0o644); err != nil { - t.Fatal(err) - } - - t.Setenv("PATH", "") - - files, err := CloneAndReadSkillFiles(dir, "skills/alpha") - if err != nil { - t.Fatalf("CloneAndReadSkillFiles() error = %v", err) - } - if len(files) != 2 { - t.Fatalf("expected 2 files, got %d", len(files)) - } - if string(files["extra.txt"]) != "extra" { - t.Fatalf("extra.txt = %q, want %q", files["extra.txt"], "extra") - } -} - -func TestSourceRepoDir(t *testing.T) { - t.Parallel() - - tests := []struct { - input string - wantEnd string - }{ - {"owner/repo", filepath.Join(".clime", "sources", "owner", "repo")}, - {"https://github.com/owner/repo.git", filepath.Join(".clime", "sources", "github.com", "owner", "repo")}, - {"git@github.com:owner/repo.git", filepath.Join(".clime", "sources", "github.com", "owner", "repo")}, - {"http://example.com/foo/bar.git", filepath.Join(".clime", "sources", "example.com", "foo", "bar")}, - } - - for _, tt := range tests { - got, err := sourceRepoDir(tt.input) - if err != nil { - t.Errorf("sourceRepoDir(%q) error = %v", tt.input, err) - continue - } - if !filepath.IsAbs(got) { - t.Errorf("sourceRepoDir(%q) = %q, want absolute path", tt.input, got) - } - if !strings.HasSuffix(got, tt.wantEnd) { - t.Errorf("sourceRepoDir(%q) = %q, want suffix %q", tt.input, got, tt.wantEnd) - } - } -} - -func TestPrepareRepoDirUsesLocalRepo(t *testing.T) { - dir := t.TempDir() - - got, cleanup, err := PrepareRepoDir(dir) - if err != nil { - t.Fatalf("PrepareRepoDir() error = %v", err) - } - defer cleanup() - - want, _ := filepath.Abs(dir) - if got != want { - t.Fatalf("PrepareRepoDir() = %q, want %q", got, want) - } -} - -func TestRemoveSourceDir(t *testing.T) { - // Create a fake source dir. - srcDir, err := sourceRepoDir("test-owner/test-repo") - if err != nil { - t.Fatal(err) - } - if err := os.MkdirAll(srcDir, 0o755); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(filepath.Join(srcDir, "test.txt"), []byte("test"), 0o644); err != nil { - t.Fatal(err) - } - - if err := RemoveSourceDir("test-owner/test-repo"); err != nil { - t.Fatalf("RemoveSourceDir() error = %v", err) - } - - if _, err := os.Stat(srcDir); !os.IsNotExist(err) { - t.Fatalf("source dir still exists after RemoveSourceDir") - } - - // Cleanup parent dirs. - home, _ := os.UserHomeDir() - os.RemoveAll(filepath.Join(home, ".clime", "sources", "test-owner")) -} diff --git a/internal/skill/helpers_test.go b/internal/skill/helpers_test.go new file mode 100644 index 0000000..be5340b --- /dev/null +++ b/internal/skill/helpers_test.go @@ -0,0 +1,75 @@ +package skill + +import ( + "os" + "os/exec" + "path/filepath" + "strings" + "testing" +) + +// gitIn runs a git command in dir and returns its trimmed output. +func gitIn(t *testing.T, dir string, args ...string) string { + t.Helper() + cmd := exec.Command("git", args...) + cmd.Dir = dir + cmd.Env = append(os.Environ(), "GIT_TERMINAL_PROMPT=0") + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("git %v failed: %v\n%s", args, err, out) + } + return strings.TrimSpace(string(out)) +} + +// createTestGitRepo creates a temporary git repo with files under skillPath. +func createTestGitRepo(t *testing.T, skillPath string, files map[string]string) string { + t.Helper() + dir := t.TempDir() + + gitIn(t, dir, "init") + gitIn(t, dir, "config", "user.email", "test@test.com") + gitIn(t, dir, "config", "user.name", "Test") + + for relPath, content := range files { + fullPath := filepath.Join(dir, skillPath, relPath) + if err := os.MkdirAll(filepath.Dir(fullPath), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(fullPath, []byte(content), 0o644); err != nil { + t.Fatal(err) + } + } + + gitIn(t, dir, "add", "-A") + gitIn(t, dir, "commit", "-m", "init") + + return dir +} + +// checkoutVersion returns the version a checkout directory holds: a tag +// pointing exactly at HEAD, or the full commit SHA. +func checkoutVersion(t *testing.T, dir string) string { + t.Helper() + cmd := exec.Command("git", "describe", "--tags", "--exact-match", "HEAD") + cmd.Dir = dir + if out, err := cmd.Output(); err == nil { + return strings.TrimSpace(string(out)) + } + cmd = exec.Command("git", "rev-parse", "HEAD") + cmd.Dir = dir + out, err := cmd.Output() + if err != nil { + t.Fatalf("git rev-parse HEAD in %s failed: %v", dir, err) + } + return strings.TrimSpace(string(out)) +} + +func writeFile(t *testing.T, path, content string) { + t.Helper() + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte(content), 0o644); err != nil { + t.Fatal(err) + } +} diff --git a/internal/skill/install.go b/internal/skill/install.go deleted file mode 100644 index ef7b38f..0000000 --- a/internal/skill/install.go +++ /dev/null @@ -1,101 +0,0 @@ -package skill - -import ( - "fmt" - "os" - "path/filepath" -) - -// targetName maps a dot-directory name to a display-friendly target name. -var targetName = map[string]string{ - ".claude": "claude", - ".codex": "codex", -} - -// installFiles writes the given skill files to ~/.claude/skills// -// and ~/.codex/skills//. -// Returns the names of the targets installed to (e.g., ["claude", "codex"]). -func installFiles(name string, files map[string][]byte) ([]string, error) { - if len(files) == 0 { - return nil, fmt.Errorf("no files found for skill %q", name) - } - - if _, ok := files["SKILL.md"]; !ok { - return nil, fmt.Errorf("skill %q is missing required SKILL.md file", name) - } - - home, err := os.UserHomeDir() - if err != nil { - return nil, fmt.Errorf("failed to get home directory: %w", err) - } - - var installed []string - targets := []string{".claude", ".codex"} - for _, dir := range targets { - base := filepath.Join(home, dir) - info, err := os.Stat(base) - if err != nil || !info.IsDir() { - continue - } - - skillDir := filepath.Join(base, "skills", name) - for relPath, content := range files { - destPath := filepath.Join(skillDir, relPath) - if err := os.MkdirAll(filepath.Dir(destPath), 0o755); err != nil { - return installed, fmt.Errorf("failed to create directory for %s: %w", destPath, err) - } - if err := os.WriteFile(destPath, content, 0o644); err != nil { - return installed, fmt.Errorf("failed to write %s: %w", destPath, err) - } - } - installed = append(installed, targetName[dir]) - } - - return installed, nil -} - -// Install clones the repo, reads skill files, and writes them -// to ~/.claude/skills// and ~/.codex/skills//. -// Returns the names of the targets installed to (e.g., ["claude", "codex"]). -func Install(name, repo, skillPath string) ([]string, error) { - files, err := CloneAndReadSkillFiles(repo, skillPath) - if err != nil { - return nil, fmt.Errorf("failed to fetch skill files: %w", err) - } - return installFiles(name, files) -} - -// InstallFromDir reads skill files from a local directory and writes them -// to ~/.claude/skills// and ~/.codex/skills//. -// Use this when the repo has already been cloned to avoid redundant clones. -func InstallFromDir(name, dir, skillPath string) ([]string, error) { - files, err := ReadSkillFilesFromDir(dir, skillPath) - if err != nil { - return nil, fmt.Errorf("failed to read skill files: %w", err) - } - return installFiles(name, files) -} - -// Uninstall removes skill files from ~/.claude/skills// and ~/.codex/skills//. -// Returns the names of the targets removed from (e.g., ["claude", "codex"]). -func Uninstall(name string) ([]string, error) { - home, err := os.UserHomeDir() - if err != nil { - return nil, fmt.Errorf("failed to get home directory: %w", err) - } - - var removed []string - targets := []string{".claude", ".codex"} - for _, dir := range targets { - skillDir := filepath.Join(home, dir, "skills", name) - if _, err := os.Stat(skillDir); err != nil { - continue - } - if err := os.RemoveAll(skillDir); err != nil { - return removed, fmt.Errorf("failed to remove %s: %w", skillDir, err) - } - removed = append(removed, targetName[dir]) - } - - return removed, nil -} diff --git a/internal/skill/install_test.go b/internal/skill/install_test.go deleted file mode 100644 index 53346e0..0000000 --- a/internal/skill/install_test.go +++ /dev/null @@ -1,687 +0,0 @@ -package skill - -import ( - "os" - "os/exec" - "path/filepath" - "testing" -) - -// createTestGitRepo creates a temporary git repo with files under skillPath. -func createTestGitRepo(t *testing.T, skillPath string, files map[string]string) string { - t.Helper() - dir := t.TempDir() - - run := func(args ...string) { - t.Helper() - cmd := exec.Command("git", args...) - cmd.Dir = dir - cmd.Env = append(os.Environ(), "GIT_TERMINAL_PROMPT=0") - out, err := cmd.CombinedOutput() - if err != nil { - t.Fatalf("git %v failed: %v\n%s", args, err, out) - } - } - - run("init") - run("config", "user.email", "test@test.com") - run("config", "user.name", "Test") - - for relPath, content := range files { - fullPath := filepath.Join(dir, skillPath, relPath) - if err := os.MkdirAll(filepath.Dir(fullPath), 0o755); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(fullPath, []byte(content), 0o644); err != nil { - t.Fatal(err) - } - } - - run("add", "-A") - run("commit", "-m", "init") - - return dir -} - -func TestInstallWritesFiles(t *testing.T) { - repoDir := createTestGitRepo(t, "skills/test-skill", map[string]string{ - "SKILL.md": "---\nname: test-skill\ndescription: A test skill\n---\n# Test Skill", - "helper.sh": "#!/bin/bash\necho hello", - "sub/nested.txt": "nested content", - }) - - home := t.TempDir() - claudeDir := filepath.Join(home, ".claude") - codexDir := filepath.Join(home, ".codex") - if err := os.MkdirAll(claudeDir, 0o755); err != nil { - t.Fatal(err) - } - if err := os.MkdirAll(codexDir, 0o755); err != nil { - t.Fatal(err) - } - t.Setenv("HOME", home) - - installed, err := Install("test-skill", repoDir, "skills/test-skill") - if err != nil { - t.Fatalf("Install failed: %v", err) - } - if len(installed) != 2 { - t.Fatalf("expected 2 targets, got %d", len(installed)) - } - - for _, dir := range []string{claudeDir, codexDir} { - skillDir := filepath.Join(dir, "skills", "test-skill") - - data, err := os.ReadFile(filepath.Join(skillDir, "SKILL.md")) - if err != nil { - t.Fatalf("failed to read SKILL.md from %s: %v", dir, err) - } - if string(data) != "---\nname: test-skill\ndescription: A test skill\n---\n# Test Skill" { - t.Fatalf("unexpected SKILL.md content: %q", data) - } - - data, err = os.ReadFile(filepath.Join(skillDir, "helper.sh")) - if err != nil { - t.Fatalf("failed to read helper.sh from %s: %v", dir, err) - } - if string(data) != "#!/bin/bash\necho hello" { - t.Fatalf("unexpected helper.sh content: %q", data) - } - - data, err = os.ReadFile(filepath.Join(skillDir, "sub", "nested.txt")) - if err != nil { - t.Fatalf("failed to read sub/nested.txt from %s: %v", dir, err) - } - if string(data) != "nested content" { - t.Fatalf("unexpected nested.txt content: %q", data) - } - } -} - -func TestInstallMissingSKILLMd(t *testing.T) { - repoDir := createTestGitRepo(t, "skills/no-skill-md", map[string]string{ - "README.md": "# Not a skill", - }) - - home := t.TempDir() - if err := os.MkdirAll(filepath.Join(home, ".claude"), 0o755); err != nil { - t.Fatal(err) - } - t.Setenv("HOME", home) - - _, err := Install("no-skill-md", repoDir, "skills/no-skill-md") - if err == nil { - t.Fatal("expected error for missing SKILL.md") - } -} - -func TestInstallNoTargetDirs(t *testing.T) { - repoDir := createTestGitRepo(t, "skills/test-skill", map[string]string{ - "SKILL.md": "# Skill", - }) - - home := t.TempDir() - t.Setenv("HOME", home) - - installed, err := Install("test-skill", repoDir, "skills/test-skill") - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if len(installed) != 0 { - t.Fatalf("expected 0 targets, got %d", len(installed)) - } -} - -func TestInstallOnlyClaudeDir(t *testing.T) { - repoDir := createTestGitRepo(t, "skills/test-skill", map[string]string{ - "SKILL.md": "# Skill", - }) - - home := t.TempDir() - if err := os.MkdirAll(filepath.Join(home, ".claude"), 0o755); err != nil { - t.Fatal(err) - } - t.Setenv("HOME", home) - - installed, err := Install("test-skill", repoDir, "skills/test-skill") - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if len(installed) != 1 { - t.Fatalf("expected 1 target, got %d", len(installed)) - } -} - -func TestInstallInvalidSkillPath(t *testing.T) { - repoDir := createTestGitRepo(t, "skills/test-skill", map[string]string{ - "SKILL.md": "# Skill", - }) - - home := t.TempDir() - if err := os.MkdirAll(filepath.Join(home, ".claude"), 0o755); err != nil { - t.Fatal(err) - } - t.Setenv("HOME", home) - - _, err := Install("test-skill", repoDir, "skills/nonexistent") - if err == nil { - t.Fatal("expected error for invalid skill path") - } -} - -func TestUninstallRemovesFiles(t *testing.T) { - home := t.TempDir() - t.Setenv("HOME", home) - - for _, dir := range []string{".claude", ".codex"} { - skillDir := filepath.Join(home, dir, "skills", "test-skill") - if err := os.MkdirAll(skillDir, 0o755); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte("# Skill"), 0o644); err != nil { - t.Fatal(err) - } - } - - removed, err := Uninstall("test-skill") - if err != nil { - t.Fatalf("Uninstall failed: %v", err) - } - if len(removed) != 2 { - t.Fatalf("expected 2 removals, got %d", len(removed)) - } - - for _, dir := range []string{".claude", ".codex"} { - skillDir := filepath.Join(home, dir, "skills", "test-skill") - if _, err := os.Stat(skillDir); !os.IsNotExist(err) { - t.Fatalf("expected %s to be removed", skillDir) - } - } -} - -func TestUninstallNotFound(t *testing.T) { - home := t.TempDir() - t.Setenv("HOME", home) - - removed, err := Uninstall("nonexistent") - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if len(removed) != 0 { - t.Fatalf("expected 0 removals, got %d", len(removed)) - } -} - -func TestFetchRepoManifestFromSkillsYAML(t *testing.T) { - dir := t.TempDir() - - run := func(args ...string) { - t.Helper() - cmd := exec.Command("git", args...) - cmd.Dir = dir - cmd.Env = append(os.Environ(), "GIT_TERMINAL_PROMPT=0") - out, err := cmd.CombinedOutput() - if err != nil { - t.Fatalf("git %v failed: %v\n%s", args, err, out) - } - } - - run("init") - run("config", "user.email", "test@test.com") - run("config", "user.name", "Test") - - skillsYAML := `skills: - - name: my-skill - description: A test skill - path: skills/my-skill - tags: - - test -` - if err := os.WriteFile(filepath.Join(dir, "skills.yaml"), []byte(skillsYAML), 0o644); err != nil { - t.Fatal(err) - } - - skillDir := filepath.Join(dir, "skills", "my-skill") - if err := os.MkdirAll(skillDir, 0o755); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte("# My Skill"), 0o644); err != nil { - t.Fatal(err) - } - - run("add", "-A") - run("commit", "-m", "init") - - manifest, err := FetchRepoManifest(dir) - if err != nil { - t.Fatalf("FetchRepoManifest failed: %v", err) - } - if len(manifest.Skills) != 1 { - t.Fatalf("expected 1 skill, got %d", len(manifest.Skills)) - } - if manifest.Skills[0].Name != "my-skill" { - t.Fatalf("expected skill name my-skill, got %s", manifest.Skills[0].Name) - } -} - -func TestFetchRepoManifestFromMarketplaceJSON(t *testing.T) { - dir := t.TempDir() - - run := func(args ...string) { - t.Helper() - cmd := exec.Command("git", args...) - cmd.Dir = dir - cmd.Env = append(os.Environ(), "GIT_TERMINAL_PROMPT=0") - out, err := cmd.CombinedOutput() - if err != nil { - t.Fatalf("git %v failed: %v\n%s", args, err, out) - } - } - - run("init") - run("config", "user.email", "test@test.com") - run("config", "user.name", "Test") - - mpDir := filepath.Join(dir, ".claude-plugin") - if err := os.MkdirAll(mpDir, 0o755); err != nil { - t.Fatal(err) - } - mpJSON := `{ - "plugins": [ - { - "name": "Test Plugin", - "description": "A test plugin", - "skills": ["./skills/skill-a", "./skills/skill-b"] - } - ] -}` - if err := os.WriteFile(filepath.Join(mpDir, "marketplace.json"), []byte(mpJSON), 0o644); err != nil { - t.Fatal(err) - } - - for _, name := range []string{"skill-a", "skill-b"} { - skillDir := filepath.Join(dir, "skills", name) - if err := os.MkdirAll(skillDir, 0o755); err != nil { - t.Fatal(err) - } - content := "---\nname: " + name + "\ndescription: " + name + " desc\n---\n# " + name - if err := os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte(content), 0o644); err != nil { - t.Fatal(err) - } - } - - run("add", "-A") - run("commit", "-m", "init") - - manifest, err := FetchRepoManifest(dir) - if err != nil { - t.Fatalf("FetchRepoManifest failed: %v", err) - } - if len(manifest.Skills) != 2 { - t.Fatalf("expected 2 skills, got %d", len(manifest.Skills)) - } - if manifest.Skills[0].Name != "skill-a" { - t.Fatalf("expected skill-a, got %s", manifest.Skills[0].Name) - } - if manifest.Skills[0].Path != "skills/skill-a" { - t.Fatalf("expected path skills/skill-a, got %s", manifest.Skills[0].Path) - } - if manifest.Skills[1].Name != "skill-b" { - t.Fatalf("expected skill-b, got %s", manifest.Skills[1].Name) - } -} - -func TestFetchRepoManifestFromPluginJSON(t *testing.T) { - dir := t.TempDir() - - run := func(args ...string) { - t.Helper() - cmd := exec.Command("git", args...) - cmd.Dir = dir - cmd.Env = append(os.Environ(), "GIT_TERMINAL_PROMPT=0") - out, err := cmd.CombinedOutput() - if err != nil { - t.Fatalf("git %v failed: %v\n%s", args, err, out) - } - } - - run("init") - run("config", "user.email", "test@test.com") - run("config", "user.name", "Test") - - // Create .claude-plugin/plugin.json pointing to a skills directory. - pluginDir := filepath.Join(dir, ".claude-plugin") - if err := os.MkdirAll(pluginDir, 0o755); err != nil { - t.Fatal(err) - } - pluginJSON := `{"name": "test-plugin", "skills": "./.claude/skills"}` - if err := os.WriteFile(filepath.Join(pluginDir, "plugin.json"), []byte(pluginJSON), 0o644); err != nil { - t.Fatal(err) - } - - // Create two skill directories with SKILL.md files. - for _, name := range []string{"skill-x", "skill-y"} { - skillDir := filepath.Join(dir, ".claude", "skills", name) - if err := os.MkdirAll(skillDir, 0o755); err != nil { - t.Fatal(err) - } - content := "---\nname: " + name + "\ndescription: " + name + " desc\n---\n# " + name - if err := os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte(content), 0o644); err != nil { - t.Fatal(err) - } - } - - run("add", "-A") - run("commit", "-m", "init") - - manifest, err := FetchRepoManifest(dir) - if err != nil { - t.Fatalf("FetchRepoManifest failed: %v", err) - } - if len(manifest.Skills) != 2 { - t.Fatalf("expected 2 skills, got %d", len(manifest.Skills)) - } - - // Skills are returned in directory order; check both exist. - names := map[string]bool{} - for _, s := range manifest.Skills { - names[s.Name] = true - } - if !names["skill-x"] || !names["skill-y"] { - t.Fatalf("expected skill-x and skill-y, got %v", names) - } -} - -func TestFetchRepoManifestPluginJSONFallbackFromEmptyMarketplace(t *testing.T) { - dir := t.TempDir() - - run := func(args ...string) { - t.Helper() - cmd := exec.Command("git", args...) - cmd.Dir = dir - cmd.Env = append(os.Environ(), "GIT_TERMINAL_PROMPT=0") - out, err := cmd.CombinedOutput() - if err != nil { - t.Fatalf("git %v failed: %v\n%s", args, err, out) - } - } - - run("init") - run("config", "user.email", "test@test.com") - run("config", "user.name", "Test") - - pluginDir := filepath.Join(dir, ".claude-plugin") - if err := os.MkdirAll(pluginDir, 0o755); err != nil { - t.Fatal(err) - } - - // marketplace.json with plugins that have no skills arrays (like pbakaus/impeccable). - mpJSON := `{"plugins": [{"name": "test", "description": "test plugin", "source": "./"}]}` - if err := os.WriteFile(filepath.Join(pluginDir, "marketplace.json"), []byte(mpJSON), 0o644); err != nil { - t.Fatal(err) - } - - // plugin.json points to the skills directory. - pluginJSON := `{"name": "test", "skills": "./.claude/skills"}` - if err := os.WriteFile(filepath.Join(pluginDir, "plugin.json"), []byte(pluginJSON), 0o644); err != nil { - t.Fatal(err) - } - - // Create a skill directory. - skillDir := filepath.Join(dir, ".claude", "skills", "my-skill") - if err := os.MkdirAll(skillDir, 0o755); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte("---\nname: my-skill\ndescription: A skill\n---\n# My Skill"), 0o644); err != nil { - t.Fatal(err) - } - - run("add", "-A") - run("commit", "-m", "init") - - manifest, err := FetchRepoManifest(dir) - if err != nil { - t.Fatalf("FetchRepoManifest failed: %v", err) - } - if len(manifest.Skills) != 1 { - t.Fatalf("expected 1 skill, got %d", len(manifest.Skills)) - } - if manifest.Skills[0].Name != "my-skill" { - t.Fatalf("expected my-skill, got %s", manifest.Skills[0].Name) - } -} - -func TestFetchRepoManifestEmptyNameFallback(t *testing.T) { - dir := t.TempDir() - - run := func(args ...string) { - t.Helper() - cmd := exec.Command("git", args...) - cmd.Dir = dir - cmd.Env = append(os.Environ(), "GIT_TERMINAL_PROMPT=0") - out, err := cmd.CombinedOutput() - if err != nil { - t.Fatalf("git %v failed: %v\n%s", args, err, out) - } - } - - run("init") - run("config", "user.email", "test@test.com") - run("config", "user.name", "Test") - - mpDir := filepath.Join(dir, ".claude-plugin") - if err := os.MkdirAll(mpDir, 0o755); err != nil { - t.Fatal(err) - } - // Skill with empty name in frontmatter - mpJSON := `{"plugins": [{"name": "P", "skills": ["./skills/my-skill"]}]}` - if err := os.WriteFile(filepath.Join(mpDir, "marketplace.json"), []byte(mpJSON), 0o644); err != nil { - t.Fatal(err) - } - - skillDir := filepath.Join(dir, "skills", "my-skill") - if err := os.MkdirAll(skillDir, 0o755); err != nil { - t.Fatal(err) - } - // Frontmatter with no name field - if err := os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte("---\ndescription: has desc but no name\n---\n# Skill"), 0o644); err != nil { - t.Fatal(err) - } - - run("add", "-A") - run("commit", "-m", "init") - - manifest, err := FetchRepoManifest(dir) - if err != nil { - t.Fatalf("FetchRepoManifest failed: %v", err) - } - if len(manifest.Skills) != 1 { - t.Fatalf("expected 1 skill, got %d", len(manifest.Skills)) - } - // Should fall back to directory basename "my-skill" - if manifest.Skills[0].Name != "my-skill" { - t.Fatalf("expected name my-skill (from directory), got %q", manifest.Skills[0].Name) - } - if manifest.Skills[0].Description != "has desc but no name" { - t.Fatalf("expected description preserved, got %q", manifest.Skills[0].Description) - } -} - -func TestEndToEndInstallFromLocalRepo(t *testing.T) { - dir := t.TempDir() - run := func(args ...string) { - t.Helper() - cmd := exec.Command("git", args...) - cmd.Dir = dir - cmd.Env = append(os.Environ(), "GIT_TERMINAL_PROMPT=0") - out, err := cmd.CombinedOutput() - if err != nil { - t.Fatalf("git %v failed: %v\n%s", args, err, out) - } - } - - run("init") - run("config", "user.email", "test@test.com") - run("config", "user.name", "Test") - - mpDir := filepath.Join(dir, ".claude-plugin") - if err := os.MkdirAll(mpDir, 0o755); err != nil { - t.Fatal(err) - } - mpJSON := `{"plugins": [{"name": "P", "skills": ["./skills/alpha"]}]}` - if err := os.WriteFile(filepath.Join(mpDir, "marketplace.json"), []byte(mpJSON), 0o644); err != nil { - t.Fatal(err) - } - - skillDir := filepath.Join(dir, "skills", "alpha") - if err := os.MkdirAll(skillDir, 0o755); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte("---\nname: alpha\ndescription: Alpha skill\n---\n# Alpha"), 0o644); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(filepath.Join(skillDir, "extra.txt"), []byte("extra"), 0o644); err != nil { - t.Fatal(err) - } - - run("add", "-A") - run("commit", "-m", "init") - - home := t.TempDir() - t.Setenv("HOME", home) - if err := os.MkdirAll(filepath.Join(home, ".claude"), 0o755); err != nil { - t.Fatal(err) - } - - // Step 1: Fetch repo manifest - manifest, err := FetchRepoManifest(dir) - if err != nil { - t.Fatalf("FetchRepoManifest failed: %v", err) - } - if len(manifest.Skills) != 1 { - t.Fatalf("expected 1 skill, got %d", len(manifest.Skills)) - } - - entry := manifest.Skills[0] - if entry.Name != "alpha" { - t.Fatalf("expected name alpha, got %s", entry.Name) - } - if entry.Path != "skills/alpha" { - t.Fatalf("expected path skills/alpha, got %s", entry.Path) - } - - // Step 2: Install the skill - installed, err := Install(entry.Name, dir, entry.Path) - if err != nil { - t.Fatalf("Install failed: %v", err) - } - if len(installed) != 1 { - t.Fatalf("expected 1 target, got %d", len(installed)) - } - - // Step 3: Verify files - installedSkillDir := filepath.Join(home, ".claude", "skills", "alpha") - data, err := os.ReadFile(filepath.Join(installedSkillDir, "SKILL.md")) - if err != nil { - t.Fatalf("failed to read SKILL.md: %v", err) - } - if string(data) != "---\nname: alpha\ndescription: Alpha skill\n---\n# Alpha" { - t.Fatalf("unexpected SKILL.md content: %q", data) - } - - data, err = os.ReadFile(filepath.Join(installedSkillDir, "extra.txt")) - if err != nil { - t.Fatalf("failed to read extra.txt: %v", err) - } - if string(data) != "extra" { - t.Fatalf("unexpected extra.txt content: %q", data) - } - - // Step 4: Uninstall - removed, err := Uninstall("alpha") - if err != nil { - t.Fatalf("Uninstall failed: %v", err) - } - if len(removed) != 1 { - t.Fatalf("expected 1 removal, got %d", len(removed)) - } - - if _, err := os.Stat(installedSkillDir); !os.IsNotExist(err) { - t.Fatal("expected skill directory to be removed") - } -} - -func TestInstallFromDirSkipsClone(t *testing.T) { - // Create a local directory (not a git repo) with skill files. - dir := t.TempDir() - skillDir := filepath.Join(dir, "skills", "test-skill") - if err := os.MkdirAll(skillDir, 0o755); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte("---\nname: test-skill\ndescription: A test\n---\n# Test"), 0o644); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(filepath.Join(skillDir, "helper.sh"), []byte("#!/bin/bash\necho hello"), 0o644); err != nil { - t.Fatal(err) - } - - home := t.TempDir() - if err := os.MkdirAll(filepath.Join(home, ".claude"), 0o755); err != nil { - t.Fatal(err) - } - t.Setenv("HOME", home) - - // InstallFromDir should work without cloning. - installed, err := InstallFromDir("test-skill", dir, "skills/test-skill") - if err != nil { - t.Fatalf("InstallFromDir failed: %v", err) - } - if len(installed) != 1 || installed[0] != "claude" { - t.Fatalf("expected [claude], got %v", installed) - } - - data, err := os.ReadFile(filepath.Join(home, ".claude", "skills", "test-skill", "SKILL.md")) - if err != nil { - t.Fatalf("failed to read SKILL.md: %v", err) - } - if string(data) != "---\nname: test-skill\ndescription: A test\n---\n# Test" { - t.Fatalf("unexpected SKILL.md content: %q", data) - } - - data, err = os.ReadFile(filepath.Join(home, ".claude", "skills", "test-skill", "helper.sh")) - if err != nil { - t.Fatalf("failed to read helper.sh: %v", err) - } - if string(data) != "#!/bin/bash\necho hello" { - t.Fatalf("unexpected helper.sh content: %q", data) - } -} - -func TestReadSkillFilesFromDir(t *testing.T) { - dir := t.TempDir() - skillDir := filepath.Join(dir, "my-skill") - subDir := filepath.Join(skillDir, "sub") - if err := os.MkdirAll(subDir, 0o755); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte("# Skill"), 0o644); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(filepath.Join(subDir, "nested.txt"), []byte("nested"), 0o644); err != nil { - t.Fatal(err) - } - - files, err := ReadSkillFilesFromDir(dir, "my-skill") - if err != nil { - t.Fatalf("ReadSkillFilesFromDir failed: %v", err) - } - if len(files) != 2 { - t.Fatalf("expected 2 files, got %d", len(files)) - } - if string(files["SKILL.md"]) != "# Skill" { - t.Fatalf("unexpected SKILL.md: %q", files["SKILL.md"]) - } - if string(files[filepath.Join("sub", "nested.txt")]) != "nested" { - t.Fatalf("unexpected nested.txt: %q", files[filepath.Join("sub", "nested.txt")]) - } -} diff --git a/internal/skill/legacy.go b/internal/skill/legacy.go new file mode 100644 index 0000000..04ac691 --- /dev/null +++ b/internal/skill/legacy.go @@ -0,0 +1,125 @@ +package skill + +// Migration of manifest layouts that predate repository-keyed versions. +// Everything here exists only to read old ~/.clime/skills.yaml files and +// rewrite them into the current shape; steady-state code lives in +// manifest.go. + +import ( + "fmt" + "os" + "strings" + + "gopkg.in/yaml.v3" +) + +// UnmarshalYAML accepts both the mapping form and the legacy plain-string +// form ("owner/repo") of a sources entry. +func (r *SourceRecord) UnmarshalYAML(value *yaml.Node) error { + if value.Kind == yaml.ScalarNode { + return value.Decode(&r.Repo) + } + type plain SourceRecord + var p plain + if err := value.Decode(&p); err != nil { + return err + } + *r = SourceRecord(p) + return nil +} + +// parseManifest unmarshals manifest data and migrates legacy layouts, +// persisting the migrated form so the migration is a one-time cost. +func parseManifest(path string, data []byte) (*Manifest, error) { + var m Manifest + if err := yaml.Unmarshal(data, &m); err != nil { + return nil, fmt.Errorf("failed to parse %s: %w", path, err) + } + if m.normalize() { + // The manifest is still usable when saving fails, so the error + // is not fatal here. + backupManifest(path, data) + _ = m.Save() + } + return &m, nil +} + +// backupManifest keeps the pre-migration manifest beside the current one so a +// downgraded clime, which cannot parse the repository-keyed layout, has a file +// to restore. An existing backup is kept: it is the older, more original state. +func backupManifest(path string, data []byte) { + backup := path + ".bak" + if _, err := os.Stat(backup); err == nil { + return + } + _ = os.WriteFile(backup, data, 0o644) +} + +// normalize migrates older manifest layouts to the current one: the version +// lives on the source repository, never on a skill or in a source's name. +// Version queries persisted in a source string are stripped (a concrete pin +// becomes the source's version), legacy per-skill versions move to their +// source, duplicate sources fold into the first-seen spelling, and every +// installed skill's source is listed. Returns whether anything changed. +func (m *Manifest) normalize() bool { + changed := false + + // Version candidates for sources that have none, keyed by lowercased repo. + candidates := make(map[string]string) + noteCandidate := func(repo, version string) { + key := strings.ToLower(repo) + if version != "" && version != "latest" && candidates[key] == "" { + candidates[key] = version + } + } + + for i, s := range m.Skills { + repo, query := splitSource(s.Source) + if repo != s.Source { + m.Skills[i].Source = repo + changed = true + } + noteCandidate(repo, query) + if s.LegacyVersion != "" { + noteCandidate(repo, s.LegacyVersion) + m.Skills[i].LegacyVersion = "" + changed = true + } + } + + sources := m.Sources + m.Sources = nil + for _, r := range sources { + repo, query := splitSource(r.Repo) + noteCandidate(repo, query) + if i := m.sourceIndex(repo); i >= 0 { + if m.Sources[i].Version == "" { + m.Sources[i].Version = r.Version + } + changed = true + continue + } + if repo != r.Repo { + changed = true + } + m.Sources = append(m.Sources, SourceRecord{Repo: repo, Version: r.Version}) + } + + for _, s := range m.Skills { + if s.Source != "" && m.sourceIndex(s.Source) < 0 { + m.Sources = append(m.Sources, SourceRecord{Repo: s.Source}) + changed = true + } + } + + for i, r := range m.Sources { + if r.Version == "" { + if v := candidates[strings.ToLower(r.Repo)]; v != "" { + m.Sources[i].Version = v + changed = true + } + } + } + + return changed +} diff --git a/internal/skill/manager.go b/internal/skill/manager.go new file mode 100644 index 0000000..a7f1e31 --- /dev/null +++ b/internal/skill/manager.go @@ -0,0 +1,281 @@ +package skill + +import ( + "fmt" + "strings" +) + +// Verb names a skill operation, for progress reporting and error text. +type Verb int + +const ( + VerbInstall Verb = iota + VerbUpdate + VerbSync +) + +// Events receives progress notifications from Manager operations so the +// caller can render spinners or logs. Implementations run synchronously; +// a nil Manager.Events is valid and reports nothing. +type Events interface { + // SourceResolving fires before a source is resolved and materialized. + SourceResolving(verb Verb, src Source) + // SourceFailed fires when preparing a source fails, including an + // update refused because the new catalog drops an installed skill. + SourceFailed(verb Verb, src Source, err error) + // SourceUpToDate fires when an update finds the source already at + // the requested version. + SourceUpToDate(src Source, version string) + // SourceReady fires once a source is materialized and its skills are + // about to be installed. version is empty for local sources. + SourceReady(verb Verb, src Source, version string) + // SkillInstalling and SkillInstalled/SkillFailed bracket one skill. + SkillInstalling(verb Verb, name string, src Source) + SkillInstalled(verb Verb, name string, targets []string) + SkillFailed(verb Verb, name string, err error) + // NoTargets fires when no agent directory exists to install into. + NoTargets() +} + +// NopEvents implements Events with no-ops; embed it to implement only +// the notifications you care about. +type NopEvents struct{} + +// Manager composes the manifest, the source store, and the detected +// targets into the skill verbs: install, update, sync, and uninstall. +type Manager struct { + Manifest *Manifest + Store *Store + Targets []Target + Events Events +} + +func (v Verb) String() string { + switch v { + case VerbUpdate: + return "update" + case VerbSync: + return "sync" + default: + return "install" + } +} + +func (NopEvents) SourceResolving(Verb, Source) {} +func (NopEvents) SourceFailed(Verb, Source, error) {} +func (NopEvents) SourceUpToDate(Source, string) {} +func (NopEvents) SourceReady(Verb, Source, string) {} +func (NopEvents) SkillInstalling(Verb, string, Source) {} +func (NopEvents) SkillInstalled(Verb, string, []string) {} +func (NopEvents) SkillFailed(Verb, string, error) {} +func (NopEvents) NoTargets() {} + +// Open loads the manifest, opens the source store, and detects targets. +func Open(events Events) (*Manager, error) { + manifest, err := LoadManifest() + if err != nil { + return nil, fmt.Errorf("failed to load skills manifest: %w", err) + } + store, err := OpenStore() + if err != nil { + return nil, err + } + targets, err := DetectTargets() + if err != nil { + return nil, err + } + return &Manager{Manifest: manifest, Store: store, Targets: targets, Events: events}, nil +} + +func (m *Manager) events() Events { + if m.Events == nil { + return NopEvents{} + } + return m.Events +} + +// Fetch materializes a source and reads its catalog, for browsing before +// an install. It reports no events; wrap it with caller-side progress. +func (m *Manager) Fetch(src Source) (*Snapshot, *Catalog, error) { + snap, err := m.Store.Snapshot(src) + if err != nil { + return nil, nil, err + } + catalog, err := snap.Catalog() + if err != nil { + return nil, nil, err + } + return snap, catalog, nil +} + +// Install installs the given catalog entries from a snapshot into every +// target and records them in the manifest. It continues past per-skill +// failures and returns how many skills succeeded. +func (m *Manager) Install(snap *Snapshot, entries []Entry) (int, error) { + return m.install(VerbInstall, snap, entries) +} + +// Update moves one source to the version its query resolves to (latest +// when it carries none), re-installing its installed skills from the new +// catalog. The update is refused when the new catalog no longer lists an +// installed skill, so a skill is never removed implicitly. Returns how +// many skills changed; zero with a nil error means already up to date. +func (m *Manager) Update(src Source) (int, error) { + ev := m.events() + installed := m.Manifest.SkillsFrom(src) + if len(installed) == 0 { + return 0, fmt.Errorf("no skills installed from %s", src.Repo) + } + + ev.SourceResolving(VerbUpdate, src) + snap, err := m.Store.Snapshot(src) + if err != nil { + ev.SourceFailed(VerbUpdate, src, err) + return 0, err + } + + if current, _ := m.Manifest.GetSource(src); snap.Version != "" && snap.Version == current.Version { + ev.SourceUpToDate(src, snap.Version) + return 0, nil + } + + catalog, err := snap.Catalog() + if err != nil { + ev.SourceFailed(VerbUpdate, src, err) + return 0, err + } + entries := make([]Entry, 0, len(installed)) + var missing []string + for _, s := range installed { + entry, ok := catalog.Find(s.Name) + if !ok { + missing = append(missing, s.Name) + continue + } + entries = append(entries, entry) + } + if len(missing) > 0 { + err := fmt.Errorf("%s at %s no longer provides %s; uninstall them first to update", + src.Repo, DisplayVersion(snap.Version), strings.Join(missing, ", ")) + ev.SourceFailed(VerbUpdate, src, err) + return 0, err + } + + ev.SourceReady(VerbUpdate, src, snap.Version) + return m.install(VerbUpdate, snap, entries) +} + +// Sync re-installs a source's skills at the version locked in the +// manifest, using the stored skill paths, and returns how many skills it +// re-installed. A source without a locked version is installed at latest +// and the resolved version is recorded. +func (m *Manager) Sync(src Source) (int, error) { + ev := m.events() + installed := m.Manifest.SkillsFrom(src) + if len(installed) == 0 { + return 0, fmt.Errorf("no skills installed from %s", src.Repo) + } + + locked := src + if record, ok := m.Manifest.GetSource(src); ok && record.Version != "" && !src.IsLocal() { + locked = src.WithQuery(record.Version) + } + + ev.SourceResolving(VerbSync, locked) + snap, err := m.Store.Snapshot(locked) + if err != nil { + ev.SourceFailed(VerbSync, locked, err) + return 0, err + } + ev.SourceReady(VerbSync, src, snap.Version) + + entries := make([]Entry, 0, len(installed)) + for _, s := range installed { + entries = append(entries, Entry{Name: s.Name, Path: s.Path}) + } + return m.install(VerbSync, snap, entries) +} + +// Uninstall removes one skill from every target and from the manifest, +// returning the names of the targets it was removed from. +func (m *Manager) Uninstall(name string) ([]string, error) { + if _, ok := m.Manifest.GetSkill(name); !ok { + return nil, fmt.Errorf("skill %q is not installed", name) + } + + var removed []string + for _, t := range m.Targets { + ok, err := t.Remove(name) + if err != nil { + return removed, err + } + if ok { + removed = append(removed, t.Name) + } + } + + m.Manifest.RemoveSkill(name) + if err := m.Manifest.Save(); err != nil { + return removed, fmt.Errorf("skill removed but failed to update manifest: %w", err) + } + return removed, nil +} + +// install installs each entry in turn, reporting failures as they happen, +// and returns how many succeeded. +func (m *Manager) install(verb Verb, snap *Snapshot, entries []Entry) (int, error) { + failed := 0 + for _, entry := range entries { + if err := m.installEntry(verb, snap, entry); err != nil { + m.events().SkillFailed(verb, entry.Name, err) + failed++ + } + } + if failed > 0 { + return len(entries) - failed, fmt.Errorf("%d skill(s) failed", failed) + } + return len(entries), nil +} + +// installEntry writes one skill from the snapshot into every target and +// records it in the manifest. The resolved version is recorded on the +// source, so a floating query is never persisted. +func (m *Manager) installEntry(verb Verb, snap *Snapshot, entry Entry) error { + ev := m.events() + ev.SkillInstalling(verb, entry.Name, snap.Source) + + files, err := snap.SkillFiles(entry.Path) + if err != nil { + return fmt.Errorf("failed to %s skill %q: %w", verb, entry.Name, err) + } + if _, ok := files["SKILL.md"]; !ok { + return fmt.Errorf("skill %q is missing required SKILL.md file", entry.Name) + } + + if len(m.Targets) == 0 { + ev.NoTargets() + return nil + } + var targets []string + for _, t := range m.Targets { + if err := t.Install(entry.Name, files); err != nil { + return fmt.Errorf("failed to %s skill %q: %w", verb, entry.Name, err) + } + targets = append(targets, t.Name) + } + + if snap.Version != "" { + m.Manifest.SetSourceVersion(snap.Source, snap.Version) + } + m.Manifest.AddSkill(InstalledSkill{ + Name: entry.Name, + Source: snap.Source.Repo, + Path: entry.Path, + }) + if err := m.Manifest.Save(); err != nil { + return fmt.Errorf("skill installed but failed to update manifest: %w", err) + } + + ev.SkillInstalled(verb, entry.Name, targets) + return nil +} diff --git a/internal/skill/manager_test.go b/internal/skill/manager_test.go new file mode 100644 index 0000000..86e594c --- /dev/null +++ b/internal/skill/manager_test.go @@ -0,0 +1,317 @@ +package skill + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// skillRemote is a git repository used as a versioned skill source. Its +// file:// URL makes clime treat it as a remote rather than a local path, so +// versions resolve and cache exactly as they do for a hosted repository. +type skillRemote struct { + t *testing.T + dir string + URL string +} + +func newSkillRemote(t *testing.T) *skillRemote { + t.Helper() + dir := t.TempDir() + r := &skillRemote{t: t, dir: dir, URL: "file://" + dir} + gitIn(t, dir, "init") + gitIn(t, dir, "config", "user.email", "test@test.com") + gitIn(t, dir, "config", "user.name", "Test") + return r +} + +// release replaces the catalog with the given skills, writing each skill's +// SKILL.md body, and tags the resulting commit. +func (r *skillRemote) release(tag string, skills map[string]string) { + r.t.Helper() + if err := os.RemoveAll(filepath.Join(r.dir, "skills")); err != nil { + r.t.Fatal(err) + } + catalog := "skills:\n" + for name, body := range skills { + writeFile(r.t, filepath.Join(r.dir, "skills", name, "SKILL.md"), body) + catalog += " - name: " + name + "\n path: skills/" + name + "\n" + } + writeFile(r.t, filepath.Join(r.dir, "skills.yaml"), catalog) + gitIn(r.t, r.dir, "add", "-A") + gitIn(r.t, r.dir, "commit", "-m", tag) + gitIn(r.t, r.dir, "tag", tag) +} + +// newTestManager builds a Manager over a temp home with ~/.claude present. +func newTestManager(t *testing.T, manifest *Manifest) (*Manager, string) { + t.Helper() + home := t.TempDir() + t.Setenv("HOME", home) + if err := os.MkdirAll(filepath.Join(home, ".claude"), 0o755); err != nil { + t.Fatal(err) + } + targets, err := DetectTargets() + if err != nil { + t.Fatal(err) + } + return &Manager{ + Manifest: manifest, + Store: &Store{Root: filepath.Join(home, ".clime", "sources")}, + Targets: targets, + }, home +} + +func readInstalledSkill(t *testing.T, home, name string) string { + t.Helper() + data, err := os.ReadFile(filepath.Join(home, ".claude", "skills", name, "SKILL.md")) + if err != nil { + t.Fatalf("reading installed %s: %v", name, err) + } + return string(data) +} + +func TestManagerInstallEndToEnd(t *testing.T) { + repoDir := t.TempDir() + writeFile(t, filepath.Join(repoDir, "skills.yaml"), `skills: + - name: alpha + description: Alpha skill + path: skills/alpha +`) + writeFile(t, filepath.Join(repoDir, "skills", "alpha", "SKILL.md"), "# Alpha") + writeFile(t, filepath.Join(repoDir, "skills", "alpha", "extra.txt"), "extra") + + mgr, home := newTestManager(t, &Manifest{}) + src, err := ParseSource(repoDir) + if err != nil { + t.Fatalf("ParseSource() error = %v", err) + } + + snap, catalog, err := mgr.Fetch(src) + if err != nil { + t.Fatalf("Fetch() error = %v", err) + } + if len(catalog.Skills) != 1 { + t.Fatalf("catalog = %+v, want 1 skill", catalog.Skills) + } + + n, err := mgr.Install(snap, catalog.Skills) + if err != nil { + t.Fatalf("Install() error = %v", err) + } + if n != 1 { + t.Fatalf("Install() = %d, want 1", n) + } + + if got := readInstalledSkill(t, home, "alpha"); got != "# Alpha" { + t.Fatalf("SKILL.md = %q", got) + } + extra, err := os.ReadFile(filepath.Join(home, ".claude", "skills", "alpha", "extra.txt")) + if err != nil || string(extra) != "extra" { + t.Fatalf("extra.txt = %q, %v", extra, err) + } + + installed, ok := mgr.Manifest.GetSkill("alpha") + if !ok || installed.Source != repoDir || installed.Path != "skills/alpha" { + t.Fatalf("manifest entry = %+v", installed) + } + if record, ok := mgr.Manifest.GetSource(src); ok && record.Version != "" { + t.Fatalf("local source must not record a version, got %q", record.Version) + } + + removed, err := mgr.Uninstall("alpha") + if err != nil { + t.Fatalf("Uninstall() error = %v", err) + } + if len(removed) != 1 || removed[0] != "claude" { + t.Fatalf("Uninstall() = %v, want [claude]", removed) + } + if _, err := os.Stat(filepath.Join(home, ".claude", "skills", "alpha")); !os.IsNotExist(err) { + t.Fatal("skill directory still exists after Uninstall") + } + if _, ok := mgr.Manifest.GetSkill("alpha"); ok { + t.Fatal("manifest still lists the skill after Uninstall") + } + + if _, err := mgr.Uninstall("alpha"); err == nil { + t.Fatal("Uninstall() of a missing skill should fail") + } +} + +func TestManagerInstallRequiresSkillMd(t *testing.T) { + repoDir := t.TempDir() + writeFile(t, filepath.Join(repoDir, "skills", "broken", "README.md"), "# not a skill") + + mgr, _ := newTestManager(t, &Manifest{}) + snap := &Snapshot{Source: Source{Repo: repoDir}, Dir: repoDir} + + n, err := mgr.Install(snap, []Entry{{Name: "broken", Path: "skills/broken"}}) + if err == nil { + t.Fatal("Install() should fail for a skill without SKILL.md") + } + if n != 0 { + t.Fatalf("Install() = %d, want 0", n) + } + if _, ok := mgr.Manifest.GetSkill("broken"); ok { + t.Fatal("failed install must not be recorded in the manifest") + } +} + +func TestManagerInstallWithoutTargets(t *testing.T) { + repoDir := t.TempDir() + writeFile(t, filepath.Join(repoDir, "skills", "alpha", "SKILL.md"), "# Alpha") + + mgr, _ := newTestManager(t, &Manifest{}) + mgr.Targets = nil + events := &recordingEvents{} + mgr.Events = events + + snap := &Snapshot{Source: Source{Repo: repoDir}, Dir: repoDir} + if _, err := mgr.Install(snap, []Entry{{Name: "alpha", Path: "skills/alpha"}}); err != nil { + t.Fatalf("Install() error = %v", err) + } + if !events.noTargets { + t.Fatal("expected NoTargets event") + } + if _, ok := mgr.Manifest.GetSkill("alpha"); ok { + t.Fatal("a skill installed nowhere must not be recorded") + } +} + +type recordingEvents struct { + NopEvents + noTargets bool + upToDate string +} + +func (e *recordingEvents) NoTargets() { e.noTargets = true } +func (e *recordingEvents) SourceUpToDate(_ Source, v string) { e.upToDate = v } + +func TestSyncKeepsLockedVersionWhileUpdateFollowsLatest(t *testing.T) { + remote := newSkillRemote(t) + remote.release("v1.0.0", map[string]string{"test-skill": "# v1"}) + + manifest := &Manifest{ + Skills: []InstalledSkill{{Name: "test-skill", Source: remote.URL, Path: "skills/test-skill"}}, + Sources: []SourceRecord{{Repo: remote.URL, Version: "v1.0.0"}}, + } + mgr, home := newTestManager(t, manifest) + src := Source{Repo: remote.URL} + sourceVersion := func() string { + record, _ := manifest.GetSource(src) + return record.Version + } + + if _, err := mgr.Sync(src); err != nil { + t.Fatalf("Sync() error = %v", err) + } + if got := readInstalledSkill(t, home, "test-skill"); got != "# v1" { + t.Fatalf("after sync, SKILL.md = %q, want the locked v1.0.0 content", got) + } + + remote.release("v2.0.0", map[string]string{"test-skill": "# v2"}) + + if _, err := mgr.Sync(src); err != nil { + t.Fatalf("Sync() after upstream release error = %v", err) + } + if got := readInstalledSkill(t, home, "test-skill"); got != "# v1" { + t.Fatalf("sync must not pick up a newer release, SKILL.md = %q", got) + } + if got := sourceVersion(); got != "v1.0.0" { + t.Fatalf("sync changed the locked version to %q", got) + } + + if _, err := mgr.Update(src); err != nil { + t.Fatalf("Update() error = %v", err) + } + if got := readInstalledSkill(t, home, "test-skill"); got != "# v2" { + t.Fatalf("after update, SKILL.md = %q, want the v2.0.0 content", got) + } + if got := sourceVersion(); got != "v2.0.0" { + t.Fatalf("after update, locked version = %q, want v2.0.0", got) + } + + events := &recordingEvents{} + mgr.Events = events + if n, err := mgr.Update(src); err != nil || n != 0 { + t.Fatalf("Update() when already at latest = (%d, %v), want (0, nil)", n, err) + } + if events.upToDate != "v2.0.0" { + t.Fatalf("expected SourceUpToDate(v2.0.0), got %q", events.upToDate) + } + mgr.Events = nil + + if _, err := mgr.Update(src.WithQuery("v1.0.0")); err != nil { + t.Fatalf("Update() to an explicit older version error = %v", err) + } + if got := readInstalledSkill(t, home, "test-skill"); got != "# v1" { + t.Fatalf("after pinning v1.0.0, SKILL.md = %q", got) + } + if got := sourceVersion(); got != "v1.0.0" { + t.Fatalf("after pinning v1.0.0, locked version = %q", got) + } +} + +func TestUpdateRefusesWhenCatalogDropsInstalledSkill(t *testing.T) { + remote := newSkillRemote(t) + remote.release("v1.0.0", map[string]string{"alpha": "# alpha v1", "beta": "# beta v1"}) + + manifest := &Manifest{ + Skills: []InstalledSkill{ + {Name: "alpha", Source: remote.URL, Path: "skills/alpha"}, + {Name: "beta", Source: remote.URL, Path: "skills/beta"}, + }, + Sources: []SourceRecord{{Repo: remote.URL, Version: "v1.0.0"}}, + } + mgr, home := newTestManager(t, manifest) + src := Source{Repo: remote.URL} + + if _, err := mgr.Sync(src); err != nil { + t.Fatalf("Sync() error = %v", err) + } + + remote.release("v2.0.0", map[string]string{"alpha": "# alpha v2"}) + + _, err := mgr.Update(src) + if err == nil { + t.Fatal("Update() should fail when the new catalog drops an installed skill") + } + if !strings.Contains(err.Error(), "beta") { + t.Fatalf("error = %v, want it to name the missing skill", err) + } + if got := readInstalledSkill(t, home, "alpha"); got != "# alpha v1" { + t.Fatalf("a refused update must leave targets unchanged, alpha = %q", got) + } + if got := readInstalledSkill(t, home, "beta"); got != "# beta v1" { + t.Fatalf("a refused update must not remove beta, got %q", got) + } + if record, _ := manifest.GetSource(src); record.Version != "v1.0.0" { + t.Fatalf("a refused update must keep the locked version, got %q", record.Version) + } + if _, ok := manifest.GetSkill("beta"); !ok { + t.Fatal("a refused update must keep beta in the manifest") + } +} + +func TestSyncBackfillsMissingSourceVersion(t *testing.T) { + remote := newSkillRemote(t) + remote.release("v1.0.0", map[string]string{"test-skill": "# v1"}) + + // A manifest from before versions were tracked: no source record. + manifest := &Manifest{ + Skills: []InstalledSkill{{Name: "test-skill", Source: remote.URL, Path: "skills/test-skill"}}, + } + mgr, home := newTestManager(t, manifest) + src := Source{Repo: remote.URL} + + if _, err := mgr.Sync(src); err != nil { + t.Fatalf("Sync() error = %v", err) + } + if got := readInstalledSkill(t, home, "test-skill"); got != "# v1" { + t.Fatalf("SKILL.md = %q", got) + } + if record, ok := manifest.GetSource(src); !ok || record.Version != "v1.0.0" { + t.Fatalf("source record = (%+v, %v), want the resolved version recorded", record, ok) + } +} diff --git a/internal/skill/manifest.go b/internal/skill/manifest.go index 4393e68..8b2afaa 100644 --- a/internal/skill/manifest.go +++ b/internal/skill/manifest.go @@ -4,24 +4,37 @@ import ( "fmt" "os" "path/filepath" - "time" + "slices" + "strings" "gopkg.in/yaml.v3" ) // InstalledSkill tracks a skill that has been installed locally. type InstalledSkill struct { - Name string `yaml:"name"` - Description string `yaml:"description,omitempty"` - Source string `yaml:"source"` - Path string `yaml:"path"` - InstalledAt time.Time `yaml:"installed_at"` + Name string `yaml:"name"` + Source string `yaml:"source"` + // LegacyVersion carries the per-skill version that older manifests + // recorded. The version belongs to the source repository; the legacy + // migration moves this value there and clears it, so it is never + // written back. + LegacyVersion string `yaml:"version,omitempty"` + Path string `yaml:"path"` } -// Manifest holds installed skills and known sources. +// SourceRecord tracks a known skill source repository and the version its +// installed skills come from. +type SourceRecord struct { + Repo string `yaml:"repo"` + Version string `yaml:"version,omitempty"` +} + +// Manifest is the persistent record at ~/.clime/skills.yaml: which skills +// are installed, from which sources, and the concrete version each source +// is pinned to. Versions live on sources, never on skills. type Manifest struct { Skills []InstalledSkill `yaml:"skills"` - Sources []string `yaml:"sources,omitempty"` + Sources []SourceRecord `yaml:"sources,omitempty"` } func manifestPath() (string, error) { @@ -50,11 +63,11 @@ func LoadManifest() (*Manifest, error) { } return nil, err } - var m Manifest - if err := yaml.Unmarshal(data, &m); err != nil { + m, err := parseManifest(path, data) + if err != nil { return nil, err } - return &m, nil + return m, nil } // Save writes the manifest to disk. @@ -105,23 +118,94 @@ func (m *Manifest) GetSkill(name string) (InstalledSkill, bool) { return InstalledSkill{}, false } -// AddSource adds a source to the known sources list if not already present. -func (m *Manifest) AddSource(source string) { - for _, s := range m.Sources { - if s == source { - return +// SkillsFrom returns the installed skills recorded from a source. +func (m *Manifest) SkillsFrom(src Source) []InstalledSkill { + var installed []InstalledSkill + for _, s := range m.Skills { + if sameRepo(s.Source, src.Repo) { + installed = append(installed, s) } } - m.Sources = append(m.Sources, source) + return installed +} + +// sourceIndex returns the index of the record for a source repository, or -1. +func (m *Manifest) sourceIndex(repo string) int { + return slices.IndexFunc(m.Sources, func(r SourceRecord) bool { + return sameRepo(r.Repo, repo) + }) +} + +// GetSource returns the recorded entry for a source. +func (m *Manifest) GetSource(src Source) (SourceRecord, bool) { + if i := m.sourceIndex(src.Repo); i >= 0 { + return m.Sources[i], true + } + return SourceRecord{}, false +} + +// AddSource adds a source to the known sources list if not already +// present, keeping the spelling of an existing entry. +func (m *Manifest) AddSource(src Source) { + if m.sourceIndex(src.Repo) >= 0 { + return + } + m.Sources = append(m.Sources, SourceRecord{Repo: src.Repo}) +} + +// SetSourceVersion records the version a source's skills are installed +// from, adding the source when it is not yet listed. +func (m *Manifest) SetSourceVersion(src Source, version string) { + if i := m.sourceIndex(src.Repo); i >= 0 { + m.Sources[i].Version = version + return + } + m.Sources = append(m.Sources, SourceRecord{Repo: src.Repo, Version: version}) } // RemoveSource removes a source from the known sources list. -func (m *Manifest) RemoveSource(source string) bool { - for i, s := range m.Sources { - if s == source { - m.Sources = append(m.Sources[:i], m.Sources[i+1:]...) - return true - } +func (m *Manifest) RemoveSource(src Source) bool { + if i := m.sourceIndex(src.Repo); i >= 0 { + m.Sources = append(m.Sources[:i], m.Sources[i+1:]...) + return true } return false } + +// InstalledSources lists the sources that have at least one installed +// skill, in first-seen order and spelling. +func (m *Manifest) InstalledSources() []Source { + repos := make([]string, 0, len(m.Skills)) + for _, s := range m.Skills { + repos = append(repos, s.Source) + } + return dedupeSources(repos) +} + +// KnownSources lists the sources of installed skills followed by tracked +// sources, preserving order and first-seen spelling. +func (m *Manifest) KnownSources() []Source { + repos := make([]string, 0, len(m.Skills)+len(m.Sources)) + for _, s := range m.Skills { + repos = append(repos, s.Source) + } + for _, r := range m.Sources { + repos = append(repos, r.Repo) + } + return dedupeSources(repos) +} + +// dedupeSources folds case-insensitive duplicates, keeping first-seen +// order and spelling. +func dedupeSources(repos []string) []Source { + seen := make(map[string]bool) + var unique []Source + for _, repo := range repos { + key := strings.ToLower(repo) + if repo != "" && !seen[key] { + seen[key] = true + unique = append(unique, Source{Repo: repo}) + } + } + return unique +} diff --git a/internal/skill/manifest_test.go b/internal/skill/manifest_test.go index 67c2d28..f2ef418 100644 --- a/internal/skill/manifest_test.go +++ b/internal/skill/manifest_test.go @@ -1,6 +1,9 @@ package skill import ( + "os" + "path/filepath" + "strings" "testing" ) @@ -19,16 +22,15 @@ func TestAddSkill(t *testing.T) { // Update existing skill. m.AddSkill(InstalledSkill{ - Name: "my-skill", - Description: "updated", - Source: "owner/repo", - Path: "my-skill", + Name: "my-skill", + Source: "owner/repo", + Path: "my-skill/updated", }) if len(m.Skills) != 1 { t.Fatalf("expected 1 skill after update, got %d", len(m.Skills)) } - if m.Skills[0].Description != "updated" { - t.Fatalf("expected description 'updated', got %q", m.Skills[0].Description) + if m.Skills[0].Path != "my-skill/updated" { + t.Fatalf("expected path 'my-skill/updated', got %q", m.Skills[0].Path) } } @@ -72,3 +74,204 @@ func TestGetSkill(t *testing.T) { t.Fatal("expected not to find missing skill") } } + +func TestSkillsFrom(t *testing.T) { + t.Parallel() + m := &Manifest{ + Skills: []InstalledSkill{ + {Name: "alpha", Source: "owner/repo"}, + {Name: "beta", Source: "Owner/Repo"}, + {Name: "gamma", Source: "other/repo"}, + }, + } + + got := m.SkillsFrom(Source{Repo: "OWNER/REPO"}) + if len(got) != 2 || got[0].Name != "alpha" || got[1].Name != "beta" { + t.Fatalf("SkillsFrom() = %+v, want alpha and beta across spellings", got) + } +} + +func TestSourcesAreCaseInsensitive(t *testing.T) { + t.Parallel() + m := &Manifest{} + + m.AddSource(Source{Repo: "AfterShip/Skills"}) + m.AddSource(Source{Repo: "aftership/skills"}) + if len(m.Sources) != 1 || m.Sources[0].Repo != "AfterShip/Skills" { + t.Fatalf("sources = %v, want first-seen spelling only", m.Sources) + } + + if !m.RemoveSource(Source{Repo: "AFTERSHIP/SKILLS"}) { + t.Fatal("RemoveSource should match case-insensitively") + } + if len(m.Sources) != 0 { + t.Fatalf("sources = %v, want empty", m.Sources) + } +} + +func TestSetSourceVersion(t *testing.T) { + t.Parallel() + m := &Manifest{} + + m.SetSourceVersion(Source{Repo: "owner/repo"}, "v1.0.0") + m.SetSourceVersion(Source{Repo: "Owner/Repo"}, "v2.0.0") + + if len(m.Sources) != 1 { + t.Fatalf("sources = %v, want one entry across spellings", m.Sources) + } + record, ok := m.GetSource(Source{Repo: "OWNER/REPO"}) + if !ok || record.Repo != "owner/repo" || record.Version != "v2.0.0" { + t.Fatalf("source = %+v, want first-seen spelling with the updated version", record) + } +} + +func TestInstalledAndKnownSources(t *testing.T) { + t.Parallel() + m := &Manifest{ + Skills: []InstalledSkill{ + {Name: "alpha", Source: "owner/repo"}, + {Name: "beta", Source: "Owner/Repo"}, + }, + Sources: []SourceRecord{ + {Repo: "owner/repo", Version: "v1.0.0"}, + {Repo: "tracked/only"}, + }, + } + + installed := m.InstalledSources() + if len(installed) != 1 || installed[0].Repo != "owner/repo" { + t.Fatalf("InstalledSources() = %v, want the skills' shared source once", installed) + } + + known := m.KnownSources() + if len(known) != 2 || known[0].Repo != "owner/repo" || known[1].Repo != "tracked/only" { + t.Fatalf("KnownSources() = %v, want installed sources then tracked ones", known) + } +} + +func writeManifestFile(t *testing.T, home, content string) { + t.Helper() + dir := filepath.Join(home, ".clime") + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "skills.yaml"), []byte(content), 0o644); err != nil { + t.Fatal(err) + } +} + +func TestLoadManifestNormalizesVersionedSources(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + + writeManifestFile(t, home, `skills: + - name: pinned-skill + source: owner/repo@v1.2.3 + path: skills/pinned-skill + - name: legacy-skill + source: owner/repo@latest + version: v1.4.0 + path: skills/legacy-skill +sources: + - owner/repo + - owner/repo@latest + - owner/repo@v1.2.3 +`) + + m, err := LoadManifest() + if err != nil { + t.Fatalf("LoadManifest() error = %v", err) + } + + if len(m.Sources) != 1 || m.Sources[0].Repo != "owner/repo" { + t.Fatalf("sources = %v, want [owner/repo]", m.Sources) + } + if m.Sources[0].Version != "v1.2.3" { + t.Fatalf("source version = %q, want the migrated pin %q", m.Sources[0].Version, "v1.2.3") + } + + for _, name := range []string{"pinned-skill", "legacy-skill"} { + s, _ := m.GetSkill(name) + if s.Source != "owner/repo" || s.LegacyVersion != "" { + t.Fatalf("%s = {source: %q, legacy version: %q}, want stripped source and no per-skill version", name, s.Source, s.LegacyVersion) + } + } +} + +func TestLoadManifestListsSkillSources(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + writeManifestFile(t, home, `skills: + - name: old-skill + source: owner/repo + path: skills/old-skill + - name: sibling-skill + source: Owner/Repo + path: skills/sibling-skill +`) + + m, err := LoadManifest() + if err != nil { + t.Fatalf("LoadManifest() error = %v", err) + } + if len(m.Sources) != 1 { + t.Fatalf("sources = %v, want the skills' shared source listed once", m.Sources) + } + if record, ok := m.GetSource(Source{Repo: "owner/repo"}); !ok || record.Version != "" { + t.Fatalf("source = (%+v, %v), want listed without a version", record, ok) + } +} + +func TestLoadManifestBacksUpMigratedFile(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + legacy := `skills: + - name: old-skill + source: owner/repo@v1.2.3 + path: skills/old-skill +sources: + - owner/repo +` + writeManifestFile(t, home, legacy) + + if _, err := LoadManifest(); err != nil { + t.Fatalf("LoadManifest() error = %v", err) + } + + backup := filepath.Join(home, ".clime", "skills.yaml.bak") + got, err := os.ReadFile(backup) + if err != nil { + t.Fatalf("reading backup: %v", err) + } + if string(got) != legacy { + t.Fatalf("backup =\n%s\nwant the pre-migration manifest\n%s", got, legacy) + } + + // A later migrating load keeps the original backup rather than + // replacing it with already-migrated content. + writeManifestFile(t, home, "skills:\n - name: other\n source: Owner/Repo@v2\n path: skills/other\n") + if _, err := LoadManifest(); err != nil { + t.Fatalf("second LoadManifest() error = %v", err) + } + got, err = os.ReadFile(backup) + if err != nil { + t.Fatalf("reading backup after second load: %v", err) + } + if string(got) != legacy { + t.Fatalf("backup was overwritten:\n%s", got) + } +} + +func TestLoadManifestParseErrorNamesFile(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + writeManifestFile(t, home, "skills: [oops\n") + + _, err := LoadManifest() + if err == nil { + t.Fatal("LoadManifest() error = nil, want a parse error") + } + if !strings.Contains(err.Error(), filepath.Join(home, ".clime", "skills.yaml")) { + t.Fatalf("error = %v, want it to name the manifest path", err) + } +} diff --git a/internal/skill/resolve.go b/internal/skill/resolve.go new file mode 100644 index 0000000..ddbe6f4 --- /dev/null +++ b/internal/skill/resolve.go @@ -0,0 +1,206 @@ +package skill + +import ( + "fmt" + "os" + "os/exec" + "regexp" + "strings" + + "golang.org/x/mod/semver" +) + +var ( + fullSHAPattern = regexp.MustCompile(`^[0-9a-f]{40}$`) + shortSHAPattern = regexp.MustCompile(`^[0-9a-f]{7,39}$`) + semverQueryPattern = regexp.MustCompile(`^v\d+(\.\d+)?$`) +) + +// resolveVersion resolves a version query against the remote's advertised +// refs, following the same rules as `go get`: +// +// - "latest" (or an empty query) selects the highest stable semver tag, +// falling back to the highest prerelease when no stable release exists, +// and to the default branch's HEAD commit when the repo has no semver tags. +// - "v1" or "v1.2" selects the highest semver tag in that major or +// major.minor line. +// - An existing tag resolves to itself. +// - A branch resolves to its head commit SHA. +// - A full commit SHA resolves to itself; a shorter SHA prefix resolves +// when it uniquely matches an advertised ref. +// +// The result is always a concrete tag or full commit SHA, never a floating +// value. +func resolveVersion(src Source, query string) (string, error) { + if fullSHAPattern.MatchString(query) { + return query, nil + } + if query == "" || query == "latest" { + return resolveLatest(src) + } + + tags, err := remoteTags(src) + if err != nil { + return "", err + } + for _, tag := range tags { + if tag == query { + return tag, nil + } + } + if semverQueryPattern.MatchString(query) { + if v := maxSemverTag(tags, query); v != "" { + return v, nil + } + return "", fmt.Errorf("no version of %s matches query %q", src.Repo, query) + } + if sha, ok, err := remoteRefCommit(src, "refs/heads/"+query); err != nil { + return "", err + } else if ok { + return sha, nil + } + if shortSHAPattern.MatchString(query) { + return expandShortSHA(src, query) + } + return "", fmt.Errorf("unknown version %q for %s: no matching tag, branch, or commit", query, src.Repo) +} + +func resolveLatest(src Source) (string, error) { + tags, err := remoteTags(src) + if err != nil { + return "", err + } + if v := maxSemverTag(tags, ""); v != "" { + return v, nil + } + sha, ok, err := remoteRefCommit(src, "HEAD") + if err != nil { + return "", err + } + if !ok { + return "", fmt.Errorf("cannot resolve latest version of %s: no semver tags and no HEAD", src.Repo) + } + return sha, nil +} + +// maxSemverTag returns the highest semver tag, preferring stable releases +// over prereleases. A query of "v1" or "v1.2" narrows candidates to that +// major or major.minor line; an empty query considers every tag. Tags that +// are not valid semver (including those missing the "v" prefix) are ignored, +// as in Go modules. +func maxSemverTag(tags []string, query string) string { + var best, bestPre string + for _, tag := range tags { + if !semver.IsValid(tag) { + continue + } + switch strings.Count(query, ".") { + case 0: + if query != "" && semver.Major(tag) != query { + continue + } + case 1: + if semver.MajorMinor(tag) != query { + continue + } + } + if semver.Prerelease(tag) != "" { + if bestPre == "" || semver.Compare(tag, bestPre) > 0 { + bestPre = tag + } + continue + } + if best == "" || semver.Compare(tag, best) > 0 { + best = tag + } + } + if best != "" { + return best + } + return bestPre +} + +// lsRemote runs git ls-remote with the given arguments followed by the +// source's clone URL and any ref patterns, returning non-empty output lines. +func lsRemote(src Source, flags []string, refs ...string) ([]string, error) { + args := append([]string{"ls-remote"}, flags...) + args = append(args, src.CloneURL()) + args = append(args, refs...) + cmd := exec.Command("git", args...) + cmd.Env = append(os.Environ(), "GIT_TERMINAL_PROMPT=0") + out, err := cmd.Output() + if err != nil { + return nil, fmt.Errorf("failed to list remote refs of %s: %w", src.Repo, err) + } + var lines []string + for line := range strings.SplitSeq(string(out), "\n") { + if strings.TrimSpace(line) != "" { + lines = append(lines, line) + } + } + return lines, nil +} + +// remoteTags lists the remote's tag names, with annotated-tag peel entries +// ("^{}") folded into their tag. +func remoteTags(src Source) ([]string, error) { + lines, err := lsRemote(src, []string{"--tags"}) + if err != nil { + return nil, err + } + seen := make(map[string]bool) + var tags []string + for _, line := range lines { + fields := strings.Fields(line) + if len(fields) != 2 { + continue + } + name := strings.TrimSuffix(strings.TrimPrefix(fields[1], "refs/tags/"), "^{}") + if !seen[name] { + seen[name] = true + tags = append(tags, name) + } + } + return tags, nil +} + +// remoteRefCommit returns the commit SHA the given ref points to, and +// whether the remote advertises that ref. +func remoteRefCommit(src Source, ref string) (string, bool, error) { + lines, err := lsRemote(src, nil, ref) + if err != nil { + return "", false, err + } + for _, line := range lines { + fields := strings.Fields(line) + if len(fields) == 2 && fields[1] == ref { + return fields[0], true, nil + } + } + return "", false, nil +} + +// expandShortSHA expands a commit SHA prefix to the full SHA when it +// uniquely matches one of the remote's advertised refs. +func expandShortSHA(src Source, prefix string) (string, error) { + lines, err := lsRemote(src, nil) + if err != nil { + return "", err + } + matches := make(map[string]bool) + for _, line := range lines { + fields := strings.Fields(line) + if len(fields) == 2 && strings.HasPrefix(fields[0], prefix) { + matches[fields[0]] = true + } + } + switch len(matches) { + case 1: + for sha := range matches { + return sha, nil + } + case 0: + return "", fmt.Errorf("commit %q does not match any advertised ref of %s; use the full 40-character SHA", prefix, src.Repo) + } + return "", fmt.Errorf("commit %q is ambiguous in %s", prefix, src.Repo) +} diff --git a/internal/skill/resolve_test.go b/internal/skill/resolve_test.go new file mode 100644 index 0000000..44a8282 --- /dev/null +++ b/internal/skill/resolve_test.go @@ -0,0 +1,106 @@ +package skill + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func TestResolveVersionLikeGoGet(t *testing.T) { + remote := createTestGitRepo(t, "skills/test-skill", map[string]string{ + "SKILL.md": "---\nname: test-skill\n---\n# Test", + }) + src := Source{Repo: remote} + + firstSHA := gitIn(t, remote, "rev-parse", "HEAD") + gitIn(t, remote, "tag", "v1.0.0") + gitIn(t, remote, "tag", "v1.1.0") + gitIn(t, remote, "tag", "v1.2.0-rc.1") + gitIn(t, remote, "tag", "2.0.0") // no "v" prefix: not semver, ignored like Go + gitIn(t, remote, "branch", "feature-x") + + if err := os.WriteFile(filepath.Join(remote, "later.txt"), []byte("later"), 0o644); err != nil { + t.Fatal(err) + } + gitIn(t, remote, "add", "-A") + gitIn(t, remote, "commit", "-m", "second") + headSHA := gitIn(t, remote, "rev-parse", "HEAD") + + tests := []struct { + query string + want string + }{ + {"latest", "v1.1.0"}, // stable beats the newer prerelease, non-v tag ignored + {"", "v1.1.0"}, + {"v1", "v1.1.0"}, + {"v1.0", "v1.0.0"}, + {"v1.2.0-rc.1", "v1.2.0-rc.1"}, // exact tag, even a prerelease + {"2.0.0", "2.0.0"}, // exact tag, even non-semver + {"feature-x", firstSHA}, // branch resolves to its head commit + {headSHA, headSHA}, // full SHA resolves to itself + {headSHA[:12], headSHA}, // advertised short SHA expands + } + + for _, tt := range tests { + got, err := resolveVersion(src, tt.query) + if err != nil { + t.Errorf("resolveVersion(%q) error = %v", tt.query, err) + continue + } + if got != tt.want { + t.Errorf("resolveVersion(%q) = %q, want %q", tt.query, got, tt.want) + } + } + + for _, query := range []string{"v9", "no-such-ref", "deadbeefdead"} { + if got, err := resolveVersion(src, query); err == nil { + t.Errorf("resolveVersion(%q) = %q, want error", query, got) + } + } +} + +func TestResolveVersionLatestWithoutSemverTags(t *testing.T) { + remote := createTestGitRepo(t, "skills/test-skill", map[string]string{ + "SKILL.md": "---\nname: test-skill\n---\n# Test", + }) + gitIn(t, remote, "tag", "release-1") // not semver + headSHA := gitIn(t, remote, "rev-parse", "HEAD") + + got, err := resolveVersion(Source{Repo: remote}, "latest") + if err != nil { + t.Fatalf("resolveVersion(latest) error = %v", err) + } + if got != headSHA { + t.Fatalf("resolveVersion(latest) = %q, want default branch HEAD %q", got, headSHA) + } +} + +func TestResolveVersionLatestPrefersPrereleaseOverHead(t *testing.T) { + remote := createTestGitRepo(t, "skills/test-skill", map[string]string{ + "SKILL.md": "---\nname: test-skill\n---\n# Test", + }) + gitIn(t, remote, "tag", "v0.1.0-alpha") + gitIn(t, remote, "tag", "v0.2.0-beta.1") + + got, err := resolveVersion(Source{Repo: remote}, "latest") + if err != nil { + t.Fatalf("resolveVersion(latest) error = %v", err) + } + if got != "v0.2.0-beta.1" { + t.Fatalf("resolveVersion(latest) = %q, want highest prerelease %q", got, "v0.2.0-beta.1") + } +} + +func TestResolveVersionFullSHASkipsNetwork(t *testing.T) { + t.Parallel() + + sha := strings.Repeat("ab12", 10) + got, err := resolveVersion(Source{Repo: "no-such-owner/no-such-repo"}, sha) + if err != nil { + t.Fatalf("resolveVersion() error = %v", err) + } + if got != sha { + t.Fatalf("resolveVersion() = %q, want %q", got, sha) + } +} diff --git a/internal/skill/source.go b/internal/skill/source.go new file mode 100644 index 0000000..58355e5 --- /dev/null +++ b/internal/skill/source.go @@ -0,0 +1,161 @@ +package skill + +import ( + "fmt" + "os" + "path/filepath" + "strings" +) + +// Source identifies where skills come from: a remote git repository +// (owner/repo shorthand, full URL, or SSH) or an existing local directory, +// together with an optional version query. Parse raw input once with +// ParseSource at the boundary; every other API takes a Source value. +type Source struct { + // Repo is the repository identity: owner/repo, a clone URL, or a + // local path. It never carries a version query. + Repo string + // Query is the version query: "", "latest", a semver line such as + // "v1" or "v1.2", a tag, a branch, or a commit SHA prefix. + Query string +} + +// ParseSource validates and splits a source such as "owner/repo@v1.2.3", +// a clone URL, or a local path. The version separator is the last "@" +// after the last "/", so the user in an SSH URL like +// "git@github.com:owner/repo.git" is never treated as a version. A source +// naming an existing local directory is kept whole, even when it contains +// an "@". +func ParseSource(raw string) (Source, error) { + repo, query := splitSource(raw) + if repo == "" { + return Source{}, fmt.Errorf("invalid source: expected owner/repo or local path, got %q", raw) + } + if isRemoteURL(repo) { + return Source{Repo: repo, Query: query}, nil + } + + info, err := os.Stat(repo) + switch { + case err == nil: + if !info.IsDir() { + return Source{}, fmt.Errorf("local source path %q is not a directory", repo) + } + return Source{Repo: repo, Query: query}, nil + case !os.IsNotExist(err): + return Source{}, fmt.Errorf("failed to inspect local source path %q: %w", repo, err) + } + + if looksLikeLocalPath(repo) { + return Source{}, fmt.Errorf("local source path %q does not exist", repo) + } + if !strings.Contains(repo, "/") { + return Source{}, fmt.Errorf("invalid source: expected owner/repo or local path, got %q", raw) + } + return Source{Repo: repo, Query: query}, nil +} + +// splitSource separates a raw source into repository and version query. +// An existing local directory is returned whole so a directory name +// containing "@" is never split. +func splitSource(raw string) (repo, query string) { + if info, err := os.Stat(raw); err == nil && info.IsDir() { + return raw, "" + } + return splitQuery(raw) +} + +// splitQuery splits "repo@query" at the last "@" appearing after the last +// "/". A trailing "@" is kept as part of the repository. +func splitQuery(s string) (repo, query string) { + at := strings.LastIndex(s, "@") + if at <= strings.LastIndex(s, "/") { + return s, "" + } + if q := s[at+1:]; q != "" { + return s[:at], q + } + return s, "" +} + +func isRemoteURL(repo string) bool { + return strings.Contains(repo, "://") || strings.HasPrefix(repo, "git@") +} + +func looksLikeLocalPath(repo string) bool { + return filepath.IsAbs(repo) || repo == "." || repo == ".." || + strings.HasPrefix(repo, "./") || strings.HasPrefix(repo, "../") +} + +// IsLocal reports whether the source names an existing local directory. +func (s Source) IsLocal() bool { + if isRemoteURL(s.Repo) { + return false + } + info, err := os.Stat(s.Repo) + return err == nil && info.IsDir() +} + +// Dir returns the absolute path of a local source. +func (s Source) Dir() (string, error) { + dir, err := filepath.Abs(s.Repo) + if err != nil { + return "", fmt.Errorf("failed to resolve local source path %q: %w", s.Repo, err) + } + return dir, nil +} + +// CloneURL converts an "owner/repo" shorthand to a git clone URL. Full +// URLs (any scheme, or SSH git@) and local paths are returned as-is. +func (s Source) CloneURL() string { + if isRemoteURL(s.Repo) || looksLikeLocalPath(s.Repo) { + return s.Repo + } + return fmt.Sprintf("https://github.com/%s.git", s.Repo) +} + +// Equal reports whether two sources refer to the same repository. +// Version queries are per-install state, not part of a source's identity. +func (s Source) Equal(o Source) bool { + return sameRepo(s.Repo, o.Repo) +} + +// sameRepo is the one definition of source identity: repository names are +// case-insensitive on GitHub and other major hosts. +func sameRepo(a, b string) bool { + return strings.EqualFold(a, b) +} + +// WithQuery returns the source pinned to the given version query. +func (s Source) WithQuery(query string) Source { + return Source{Repo: s.Repo, Query: query} +} + +// String renders the source as "repo" or "repo@query". +func (s Source) String() string { + if s.Query == "" { + return s.Repo + } + return s.Repo + "@" + s.Query +} + +// DisplayVersion shortens a full commit SHA for display; tags are shown +// as-is and an unknown version renders as a dash. +func DisplayVersion(version string) string { + if version == "" { + return "—" + } + if len(version) == 40 && isHex(version) { + return version[:12] + } + return version +} + +func isHex(s string) bool { + for _, r := range s { + if (r < '0' || r > '9') && (r < 'a' || r > 'f') && (r < 'A' || r > 'F') { + return false + } + } + return true +} diff --git a/internal/skill/source_test.go b/internal/skill/source_test.go new file mode 100644 index 0000000..0bf8b76 --- /dev/null +++ b/internal/skill/source_test.go @@ -0,0 +1,162 @@ +package skill + +import ( + "os" + "path/filepath" + "testing" +) + +func TestParseSource(t *testing.T) { + t.Parallel() + + tests := []struct { + raw string + wantRepo string + wantQuery string + }{ + {"owner/repo", "owner/repo", ""}, + {"owner/repo@v1.2.3", "owner/repo", "v1.2.3"}, + {"owner/repo@8f9f4e0b67b9f6c627e93ab4e56ee48d623aa095", "owner/repo", "8f9f4e0b67b9f6c627e93ab4e56ee48d623aa095"}, + {"owner/repo@", "owner/repo@", ""}, + {"https://github.com/owner/repo.git@v1.2.3", "https://github.com/owner/repo.git", "v1.2.3"}, + {"git@github.com:owner/repo.git", "git@github.com:owner/repo.git", ""}, + {"git@github.com:owner/repo.git@v1.4.2", "git@github.com:owner/repo.git", "v1.4.2"}, + } + + for _, tt := range tests { + src, err := ParseSource(tt.raw) + if err != nil { + t.Errorf("ParseSource(%q) error = %v", tt.raw, err) + continue + } + if src.Repo != tt.wantRepo || src.Query != tt.wantQuery { + t.Errorf("ParseSource(%q) = (%q, %q), want (%q, %q)", + tt.raw, src.Repo, src.Query, tt.wantRepo, tt.wantQuery) + } + } +} + +func TestParseSourceRejectsInvalid(t *testing.T) { + t.Parallel() + + for _, raw := range []string{"", "noslash", "./does-not-exist", "../does-not-exist"} { + if src, err := ParseSource(raw); err == nil { + t.Errorf("ParseSource(%q) = %+v, want error", raw, src) + } + } +} + +func TestParseSourceAllowsCurrentDir(t *testing.T) { + src, err := ParseSource(".") + if err != nil { + t.Fatalf("ParseSource(.) error = %v", err) + } + if !src.IsLocal() { + t.Fatal("ParseSource(.) should be local") + } +} + +func TestParseSourceLocalDir(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + src, err := ParseSource(dir) + if err != nil { + t.Fatalf("ParseSource(%q) error = %v", dir, err) + } + if src.Repo != dir || src.Query != "" { + t.Fatalf("ParseSource(%q) = (%q, %q), want the path with no query", dir, src.Repo, src.Query) + } + if !src.IsLocal() { + t.Fatal("existing directory should be local") + } + + abs, err := src.Dir() + if err != nil { + t.Fatalf("Dir() error = %v", err) + } + want, _ := filepath.Abs(dir) + if abs != want { + t.Fatalf("Dir() = %q, want %q", abs, want) + } +} + +func TestParseSourceKeepsLocalDirWithAt(t *testing.T) { + t.Parallel() + + base := t.TempDir() + dir := filepath.Join(base, "skills@v2") + if err := os.Mkdir(dir, 0o755); err != nil { + t.Fatal(err) + } + + src, err := ParseSource(dir) + if err != nil { + t.Fatalf("ParseSource(%q) error = %v", dir, err) + } + if src.Repo != dir || src.Query != "" { + t.Fatalf("ParseSource(%q) = (%q, %q), want the existing path unchanged", dir, src.Repo, src.Query) + } +} + +func TestCloneURL(t *testing.T) { + t.Parallel() + + tests := []struct { + repo string + want string + }{ + {"owner/repo", "https://github.com/owner/repo.git"}, + {"https://github.com/owner/repo.git", "https://github.com/owner/repo.git"}, + {"https://gitlab.com/group/repo.git", "https://gitlab.com/group/repo.git"}, + {"git@github.com:owner/repo.git", "git@github.com:owner/repo.git"}, + {"file:///tmp/local-repo", "file:///tmp/local-repo"}, + {"/tmp/local-repo", "/tmp/local-repo"}, + {"./relative-repo", "./relative-repo"}, + {"../parent-repo", "../parent-repo"}, + } + + for _, tt := range tests { + if got := (Source{Repo: tt.repo}).CloneURL(); got != tt.want { + t.Errorf("CloneURL(%q) = %q, want %q", tt.repo, got, tt.want) + } + } +} + +func TestSourceEqual(t *testing.T) { + t.Parallel() + + if !(Source{Repo: "owner/repo", Query: "v1.0.0"}).Equal(Source{Repo: "Owner/Repo", Query: "latest"}) { + t.Fatal("Equal should ignore case and version queries") + } + if (Source{Repo: "owner/repo"}).Equal(Source{Repo: "owner/other"}) { + t.Fatal("Equal should not match different repos") + } +} + +func TestSourceString(t *testing.T) { + t.Parallel() + + if got := (Source{Repo: "owner/repo"}).String(); got != "owner/repo" { + t.Fatalf("String() = %q", got) + } + if got := (Source{Repo: "owner/repo"}).WithQuery("v1.2.3").String(); got != "owner/repo@v1.2.3" { + t.Fatalf("String() = %q", got) + } +} + +func TestDisplayVersion(t *testing.T) { + t.Parallel() + + sha := "8f9f4e0b67b9f6c627e93ab4e56ee48d623aa095" + tests := []struct{ in, want string }{ + {"", "—"}, + {"v1.2.3", "v1.2.3"}, + {sha, sha[:12]}, + } + for _, tt := range tests { + if got := DisplayVersion(tt.in); got != tt.want { + t.Errorf("DisplayVersion(%q) = %q, want %q", tt.in, got, tt.want) + } + } +} diff --git a/internal/skill/store.go b/internal/skill/store.go new file mode 100644 index 0000000..001d038 --- /dev/null +++ b/internal/skill/store.go @@ -0,0 +1,207 @@ +package skill + +import ( + "fmt" + "os" + "os/exec" + "path/filepath" + "strings" +) + +// Store owns the version cache under ~/.clime/sources/: one immutable +// directory per (source, resolved version), never mutated after clone. +type Store struct { + Root string +} + +// Snapshot is a Source materialized on disk at one concrete version. +type Snapshot struct { + Source Source + Dir string + // Version is the concrete tag or full commit SHA the directory holds. + // Local sources have no version identity, so their version is empty. + Version string +} + +// OpenStore returns the store rooted at ~/.clime/sources. +func OpenStore() (*Store, error) { + home, err := os.UserHomeDir() + if err != nil { + return nil, fmt.Errorf("failed to get home directory: %w", err) + } + return &Store{Root: filepath.Join(home, ".clime", "sources")}, nil +} + +// Snapshot materializes src at the version its query resolves to (latest +// when it carries none). A concrete version already in cache is returned +// without network access; floating queries (latest, a semver line, a +// branch) always resolve remotely because only resolved versions are +// cached. Local sources are used in place without cloning. +func (st *Store) Snapshot(src Source) (*Snapshot, error) { + if src.IsLocal() { + if src.Query != "" { + return nil, fmt.Errorf("version %q is not supported for local path %q", src.Query, src.Repo) + } + dir, err := src.Dir() + if err != nil { + return nil, err + } + return &Snapshot{Source: src, Dir: dir}, nil + } + + query := src.Query + if query == "" { + query = "latest" + } + base := st.repoDir(src) + if dir := versionDir(base, query); dirExists(dir) { + return &Snapshot{Source: src, Dir: dir, Version: query}, nil + } + resolved, err := resolveVersion(src, query) + if err != nil { + return nil, err + } + dir := versionDir(base, resolved) + if !dirExists(dir) { + if err := cloneAtVersion(src, resolved, dir); err != nil { + return nil, fmt.Errorf("failed to clone %s at %s: %w", src.Repo, resolved, err) + } + } + return &Snapshot{Source: src, Dir: dir, Version: resolved}, nil +} + +// Remove deletes every cached version of the source. +func (st *Store) Remove(src Source) error { + return os.RemoveAll(st.repoDir(src)) +} + +// repoDir returns the base cache path for a source repository; version +// directories live beside it, keyed by versionDir. +func (st *Store) repoDir(src Source) string { + name := src.Repo + name = strings.TrimPrefix(name, "https://") + name = strings.TrimPrefix(name, "http://") + name = strings.TrimPrefix(name, "git@") + name = strings.TrimSuffix(name, ".git") + name = strings.ReplaceAll(name, ":", "/") + return filepath.Join(st.Root, name) +} + +// versionDir returns the immutable cache directory for one version of a +// source repository. +func versionDir(base, version string) string { + return base + "@" + strings.ReplaceAll(version, "/", "-") +} + +func dirExists(path string) bool { + info, err := os.Stat(path) + return err == nil && info.IsDir() +} + +// Catalog reads the skills this snapshot offers. +func (s *Snapshot) Catalog() (*Catalog, error) { + catalog, err := ReadCatalog(s.Dir) + if err != nil { + return nil, fmt.Errorf("%s: %w", s.Source.Repo, err) + } + return catalog, nil +} + +// SkillFiles reads all files under one skill's path, keyed by path +// relative to it. A path naming a single file yields that file alone. +func (s *Snapshot) SkillFiles(path string) (map[string][]byte, error) { + root := filepath.Join(s.Dir, path) + info, err := os.Stat(root) + if err != nil { + return nil, fmt.Errorf("skill path %q not found: %w", path, err) + } + + files := make(map[string][]byte) + if !info.IsDir() { + data, err := os.ReadFile(root) + if err != nil { + return nil, err + } + files[filepath.Base(root)] = data + return files, nil + } + + err = filepath.Walk(root, func(p string, fi os.FileInfo, err error) error { + if err != nil { + return err + } + if fi.IsDir() { + if fi.Name() == ".git" { + return filepath.SkipDir + } + return nil + } + rel, err := filepath.Rel(root, p) + if err != nil { + return err + } + data, err := os.ReadFile(p) + if err != nil { + return err + } + files[rel] = data + return nil + }) + if err != nil { + return nil, fmt.Errorf("failed to read skill files: %w", err) + } + + return files, nil +} + +// cloneAtVersion clones a source checked out at the given version (a tag, +// branch, or commit SHA) into dir. Tags and branches are cloned directly; +// a commit SHA falls back to cloning the default branch and fetching the +// commit. +func cloneAtVersion(src Source, version, dir string) error { + if err := os.MkdirAll(filepath.Dir(dir), 0o755); err != nil { + return fmt.Errorf("failed to create parent directory: %w", err) + } + + cmd := exec.Command("git", "clone", "--depth", "1", "--branch", version, src.CloneURL(), dir) + cmd.Env = append(os.Environ(), "GIT_TERMINAL_PROMPT=0") + if _, err := cmd.CombinedOutput(); err == nil { + return nil + } + os.RemoveAll(dir) + + if err := cloneDefault(src, dir); err != nil { + return err + } + for _, args := range [][]string{ + {"fetch", "--depth", "1", "origin", version}, + {"checkout", "--detach", "FETCH_HEAD"}, + } { + cmd := exec.Command("git", args...) + cmd.Dir = dir + cmd.Env = append(os.Environ(), "GIT_TERMINAL_PROMPT=0") + if out, err := cmd.CombinedOutput(); err != nil { + os.RemoveAll(dir) + return fmt.Errorf("failed to check out version %q: %w\n%s", version, err, out) + } + } + return nil +} + +// cloneDefault performs a shallow clone of the default branch, trying the +// GitHub CLI (gh) first and falling back to git. +func cloneDefault(src Source, dir string) error { + cmd := exec.Command("gh", "repo", "clone", src.Repo, dir, "--", "--depth", "1") + if _, err := cmd.CombinedOutput(); err == nil { + return nil + } + os.RemoveAll(dir) + + cmd = exec.Command("git", "clone", "--depth", "1", src.CloneURL(), dir) + cmd.Env = append(os.Environ(), "GIT_TERMINAL_PROMPT=0") + if out, err := cmd.CombinedOutput(); err != nil { + os.RemoveAll(dir) + return fmt.Errorf("git clone failed: %w\n%s", err, out) + } + return nil +} diff --git a/internal/skill/store_test.go b/internal/skill/store_test.go new file mode 100644 index 0000000..082a557 --- /dev/null +++ b/internal/skill/store_test.go @@ -0,0 +1,218 @@ +package skill + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func newTestStore(t *testing.T) *Store { + t.Helper() + return &Store{Root: t.TempDir()} +} + +func TestStoreRepoDir(t *testing.T) { + t.Parallel() + + st := &Store{Root: "/root"} + tests := []struct { + repo string + want string + }{ + {"owner/repo", filepath.Join("/root", "owner", "repo")}, + {"https://github.com/owner/repo.git", filepath.Join("/root", "github.com", "owner", "repo")}, + {"git@github.com:owner/repo.git", filepath.Join("/root", "github.com", "owner", "repo")}, + {"http://example.com/foo/bar.git", filepath.Join("/root", "example.com", "foo", "bar")}, + } + + for _, tt := range tests { + if got := st.repoDir(Source{Repo: tt.repo}); got != tt.want { + t.Errorf("repoDir(%q) = %q, want %q", tt.repo, got, tt.want) + } + } +} + +func TestSnapshotLocalSource(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + st := newTestStore(t) + + snap, err := st.Snapshot(Source{Repo: dir}) + if err != nil { + t.Fatalf("Snapshot() error = %v", err) + } + want, _ := filepath.Abs(dir) + if snap.Dir != want { + t.Fatalf("Snapshot().Dir = %q, want %q", snap.Dir, want) + } + if snap.Version != "" { + t.Fatalf("Snapshot().Version = %q, want empty for a local path", snap.Version) + } +} + +func TestSnapshotRejectsVersionForLocalPath(t *testing.T) { + t.Parallel() + + st := newTestStore(t) + if _, err := st.Snapshot(Source{Repo: t.TempDir(), Query: "v1.0.0"}); err == nil { + t.Fatal("Snapshot() should reject a version query on a local path") + } +} + +func TestSnapshotResolvesLatestAndCaches(t *testing.T) { + remote := createTestGitRepo(t, "skills/test-skill", map[string]string{ + "SKILL.md": "---\nname: test-skill\n---\n# v1", + }) + gitIn(t, remote, "tag", "v1.0.0") + + if err := os.WriteFile(filepath.Join(remote, "later.txt"), []byte("later"), 0o644); err != nil { + t.Fatal(err) + } + gitIn(t, remote, "add", "-A") + gitIn(t, remote, "commit", "-m", "untagged head") + + st := newTestStore(t) + src := Source{Repo: "file://" + remote} + + snap, err := st.Snapshot(src) + if err != nil { + t.Fatalf("Snapshot() error = %v", err) + } + if snap.Version != "v1.0.0" { + t.Fatalf("resolved version = %q, want %q", snap.Version, "v1.0.0") + } + if !strings.HasSuffix(snap.Dir, "@v1.0.0") { + t.Fatalf("cache dir = %q, want it keyed by the resolved version v1.0.0", snap.Dir) + } + if got := checkoutVersion(t, snap.Dir); got != "v1.0.0" { + t.Fatalf("checkout version = %q, want %q", got, "v1.0.0") + } + if _, err := os.Stat(filepath.Join(snap.Dir, "later.txt")); err == nil { + t.Fatal("checkout should be the latest tag, not the untagged HEAD") + } + + // A second call reuses the resolved snapshot. + snap2, err := st.Snapshot(src.WithQuery("latest")) + if err != nil { + t.Fatalf("Snapshot(latest) error = %v", err) + } + if snap2.Dir != snap.Dir || snap2.Version != snap.Version { + t.Fatalf("second call = (%q, %q), want cached (%q, %q)", snap2.Dir, snap2.Version, snap.Dir, snap.Version) + } + + // A concrete cached version needs no network: point the source at a + // repo that no longer exists and ask for the pinned version. + gone := Source{Repo: src.Repo + "-gone"} + cached := versionDir(st.repoDir(gone), "v1.0.0") + if err := os.MkdirAll(cached, 0o755); err != nil { + t.Fatal(err) + } + snap3, err := st.Snapshot(gone.WithQuery("v1.0.0")) + if err != nil { + t.Fatalf("Snapshot(cached pin) error = %v", err) + } + if snap3.Dir != cached { + t.Fatalf("Snapshot(cached pin).Dir = %q, want %q", snap3.Dir, cached) + } +} + +func TestCloneAtVersion(t *testing.T) { + remote := createTestGitRepo(t, "skills/test-skill", map[string]string{ + "SKILL.md": "---\nname: test-skill\n---\n# v1", + }) + src := Source{Repo: remote} + + gitIn(t, remote, "tag", "v1.0.0") + taggedSHA := gitIn(t, remote, "rev-parse", "HEAD") + + // A later commit so the tag no longer points at the default branch HEAD. + if err := os.WriteFile(filepath.Join(remote, "later.txt"), []byte("later"), 0o644); err != nil { + t.Fatal(err) + } + gitIn(t, remote, "add", "-A") + gitIn(t, remote, "commit", "-m", "second") + gitIn(t, remote, "config", "uploadpack.allowAnySHA1InWant", "true") + + t.Run("tag", func(t *testing.T) { + dir := filepath.Join(t.TempDir(), "clone") + if err := cloneAtVersion(src, "v1.0.0", dir); err != nil { + t.Fatalf("cloneAtVersion() error = %v", err) + } + if got := checkoutVersion(t, dir); got != "v1.0.0" { + t.Fatalf("checkout version = %q, want %q", got, "v1.0.0") + } + if _, err := os.Stat(filepath.Join(dir, "later.txt")); err == nil { + t.Fatal("file from a later commit should not exist in the tagged checkout") + } + }) + + t.Run("commit SHA", func(t *testing.T) { + dir := filepath.Join(t.TempDir(), "clone") + if err := cloneAtVersion(src, taggedSHA, dir); err != nil { + t.Fatalf("cloneAtVersion() error = %v", err) + } + if _, err := os.Stat(filepath.Join(dir, "later.txt")); err == nil { + t.Fatal("file from a later commit should not exist in the pinned checkout") + } + }) + + t.Run("unknown version", func(t *testing.T) { + dir := filepath.Join(t.TempDir(), "clone") + if err := cloneAtVersion(src, "v9.9.9", dir); err == nil { + t.Fatal("cloneAtVersion() should fail for an unknown version") + } + if _, err := os.Stat(dir); !os.IsNotExist(err) { + t.Fatal("failed clone should not leave a cache directory behind") + } + }) +} + +func TestStoreRemove(t *testing.T) { + t.Parallel() + + st := newTestStore(t) + src := Source{Repo: "test-owner/test-repo"} + dir := versionDir(st.repoDir(src), "v1.0.0") + if err := os.MkdirAll(st.repoDir(src), 0o755); err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + + if err := st.Remove(src); err != nil { + t.Fatalf("Remove() error = %v", err) + } + if _, err := os.Stat(st.repoDir(src)); !os.IsNotExist(err) { + t.Fatal("source cache still exists after Remove") + } +} + +func TestSnapshotSkillFiles(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "my-skill", "SKILL.md"), "# Skill") + writeFile(t, filepath.Join(dir, "my-skill", "sub", "nested.txt"), "nested") + + snap := &Snapshot{Source: Source{Repo: dir}, Dir: dir} + files, err := snap.SkillFiles("my-skill") + if err != nil { + t.Fatalf("SkillFiles() error = %v", err) + } + if len(files) != 2 { + t.Fatalf("expected 2 files, got %d", len(files)) + } + if string(files["SKILL.md"]) != "# Skill" { + t.Fatalf("unexpected SKILL.md: %q", files["SKILL.md"]) + } + if string(files[filepath.Join("sub", "nested.txt")]) != "nested" { + t.Fatalf("unexpected nested.txt: %q", files[filepath.Join("sub", "nested.txt")]) + } + + if _, err := snap.SkillFiles("does-not-exist"); err == nil { + t.Fatal("SkillFiles() should fail for a missing path") + } +} diff --git a/internal/skill/target.go b/internal/skill/target.go new file mode 100644 index 0000000..a80c6fb --- /dev/null +++ b/internal/skill/target.go @@ -0,0 +1,86 @@ +package skill + +import ( + "fmt" + "os" + "path/filepath" +) + +// Target is one agent destination for installed skills, such as +// ~/.claude or ~/.codex. +type Target struct { + Name string + Dir string +} + +// targetHomes lists the known agent dot-directories and their display names. +var targetHomes = []struct{ name, dir string }{ + {"claude", ".claude"}, + {"codex", ".codex"}, +} + +// Targets returns every known target, whether or not its base directory +// exists. +func Targets() ([]Target, error) { + home, err := os.UserHomeDir() + if err != nil { + return nil, fmt.Errorf("failed to get home directory: %w", err) + } + targets := make([]Target, 0, len(targetHomes)) + for _, t := range targetHomes { + targets = append(targets, Target{Name: t.name, Dir: filepath.Join(home, t.dir)}) + } + return targets, nil +} + +// DetectTargets returns the targets whose base directory exists. +func DetectTargets() ([]Target, error) { + all, err := Targets() + if err != nil { + return nil, err + } + var detected []Target + for _, t := range all { + if t.Exists() { + detected = append(detected, t) + } + } + return detected, nil +} + +// Exists reports whether the target's base directory exists. +func (t Target) Exists() bool { + info, err := os.Stat(t.Dir) + return err == nil && info.IsDir() +} + +func (t Target) skillDir(name string) string { + return filepath.Join(t.Dir, "skills", name) +} + +// Install writes the given skill files under /skills//. +func (t Target) Install(name string, files map[string][]byte) error { + dir := t.skillDir(name) + for rel, content := range files { + dest := filepath.Join(dir, rel) + if err := os.MkdirAll(filepath.Dir(dest), 0o755); err != nil { + return fmt.Errorf("failed to create directory for %s: %w", dest, err) + } + if err := os.WriteFile(dest, content, 0o644); err != nil { + return fmt.Errorf("failed to write %s: %w", dest, err) + } + } + return nil +} + +// Remove deletes /skills//, reporting whether it existed. +func (t Target) Remove(name string) (bool, error) { + dir := t.skillDir(name) + if _, err := os.Stat(dir); err != nil { + return false, nil + } + if err := os.RemoveAll(dir); err != nil { + return false, fmt.Errorf("failed to remove %s: %w", dir, err) + } + return true, nil +} diff --git a/internal/skill/target_test.go b/internal/skill/target_test.go new file mode 100644 index 0000000..75fa446 --- /dev/null +++ b/internal/skill/target_test.go @@ -0,0 +1,87 @@ +package skill + +import ( + "os" + "path/filepath" + "testing" +) + +func TestDetectTargets(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + + targets, err := DetectTargets() + if err != nil { + t.Fatalf("DetectTargets() error = %v", err) + } + if len(targets) != 0 { + t.Fatalf("targets = %v, want none in an empty home", targets) + } + + if err := os.MkdirAll(filepath.Join(home, ".claude"), 0o755); err != nil { + t.Fatal(err) + } + targets, err = DetectTargets() + if err != nil { + t.Fatalf("DetectTargets() error = %v", err) + } + if len(targets) != 1 || targets[0].Name != "claude" { + t.Fatalf("targets = %v, want [claude]", targets) + } + + if err := os.MkdirAll(filepath.Join(home, ".codex"), 0o755); err != nil { + t.Fatal(err) + } + targets, err = DetectTargets() + if err != nil { + t.Fatalf("DetectTargets() error = %v", err) + } + if len(targets) != 2 { + t.Fatalf("targets = %v, want claude and codex", targets) + } +} + +func TestTargetInstallAndRemove(t *testing.T) { + target := Target{Name: "claude", Dir: filepath.Join(t.TempDir(), ".claude")} + if err := os.MkdirAll(target.Dir, 0o755); err != nil { + t.Fatal(err) + } + + files := map[string][]byte{ + "SKILL.md": []byte("# Skill"), + "helper.sh": []byte("#!/bin/bash\necho hello"), + filepath.Join("sub", "nest.txt"): []byte("nested"), + } + if err := target.Install("test-skill", files); err != nil { + t.Fatalf("Install() error = %v", err) + } + + for rel, want := range files { + got, err := os.ReadFile(filepath.Join(target.Dir, "skills", "test-skill", rel)) + if err != nil { + t.Fatalf("reading %s: %v", rel, err) + } + if string(got) != string(want) { + t.Fatalf("%s = %q, want %q", rel, got, want) + } + } + + removed, err := target.Remove("test-skill") + if err != nil { + t.Fatalf("Remove() error = %v", err) + } + if !removed { + t.Fatal("Remove() = false, want true for an installed skill") + } + if _, err := os.Stat(filepath.Join(target.Dir, "skills", "test-skill")); !os.IsNotExist(err) { + t.Fatal("skill directory still exists after Remove") + } + + removed, err = target.Remove("test-skill") + if err != nil { + t.Fatalf("Remove() second call error = %v", err) + } + if removed { + t.Fatal("Remove() = true for a skill that is not installed") + } +}