From 842f54d7c8df2365050ba05cdbd62d5e58d0a405 Mon Sep 17 00:00:00 2001 From: Kunal Kushwaha Date: Mon, 22 Jun 2026 00:24:07 +0900 Subject: [PATCH] feat(template): support `agk template remove` by name Previously `template remove` only accepted the full source path; passing a manifest name (e.g. "rag-agent") failed. This was a documented TODO. - Add CacheManager.FindByName / RemoveByName: match cached templates by manifest name or source, removing all cached versions. - `template remove ` now tries name/source lookup first, then falls back to source-path removal, with a clear "not found" error. - Adds the registry package's first unit tests (find/remove/not-found). Co-Authored-By: Claude Opus 4.8 --- cmd/template.go | 20 +++++---- pkg/registry/cache.go | 37 +++++++++++++++ pkg/registry/cache_test.go | 92 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 140 insertions(+), 9 deletions(-) create mode 100644 pkg/registry/cache_test.go diff --git a/cmd/template.go b/cmd/template.go index 00bf66b..1ead009 100644 --- a/cmd/template.go +++ b/cmd/template.go @@ -77,24 +77,26 @@ var templateRemoveCmd = &cobra.Command{ Short: "Remove a template from the cache", Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - source := args[0] + ref := args[0] cm, err := registry.NewCacheManager("") if err != nil { return err } - // Try to remove by exact source match first, then maybe by name? - // CacheManager.Remove takes source. - // If user passes "rag-agent" (name) but source is "github.com/...", Remove might fail. - // TODO: Implement lookup by name in CacheManager to support removing by name. - // For now, assume source. + // Prefer lookup by manifest name or source (so `agk template remove rag-agent` + // works, not just the full source path). + if n, err := cm.RemoveByName(ref); err == nil { + color.Green("Removed template: %s (%d cached version(s))", ref, n) + return nil + } - if err := cm.Remove(source, ""); err != nil { - return err + // Fall back to treating the argument as a source path and removing all versions. + if err := cm.Remove(ref, ""); err != nil { + return fmt.Errorf("template %q not found in cache", ref) } - color.Green("Removed template: %s", source) + color.Green("Removed template: %s", ref) return nil }, } diff --git a/pkg/registry/cache.go b/pkg/registry/cache.go index 9e718ca..e3bf135 100644 --- a/pkg/registry/cache.go +++ b/pkg/registry/cache.go @@ -147,6 +147,43 @@ func (c *CacheManager) Remove(source, version string) error { return nil } +// FindByName returns all cached templates whose manifest name or source matches ref. +func (c *CacheManager) FindByName(ref string) ([]CachedTemplate, error) { + all, err := c.List() + if err != nil { + return nil, err + } + + var matches []CachedTemplate + for _, t := range all { + if t.Name == ref || filepath.ToSlash(t.Source) == ref { + matches = append(matches, t) + } + } + return matches, nil +} + +// RemoveByName removes cached templates identified by manifest name (or source), +// covering all cached versions, and returns how many entries were removed. +func (c *CacheManager) RemoveByName(ref string) (int, error) { + matches, err := c.FindByName(ref) + if err != nil { + return 0, err + } + if len(matches) == 0 { + return 0, fmt.Errorf("no cached template matches %q", ref) + } + + removed := 0 + for _, t := range matches { + if err := os.RemoveAll(t.LocalPath); err != nil { + return removed, fmt.Errorf("failed to remove %s: %w", t.LocalPath, err) + } + removed++ + } + return removed, nil +} + // Clear removes all cached templates. func (c *CacheManager) Clear() error { if err := os.RemoveAll(c.BaseDir); err != nil { diff --git a/pkg/registry/cache_test.go b/pkg/registry/cache_test.go new file mode 100644 index 0000000..048832e --- /dev/null +++ b/pkg/registry/cache_test.go @@ -0,0 +1,92 @@ +package registry + +import ( + "os" + "path/filepath" + "testing" +) + +// writeFakeTemplate creates a cached template at BaseDir///agk-template.toml. +func writeFakeTemplate(t *testing.T, baseDir, source, version, name string) { + t.Helper() + dir := filepath.Join(baseDir, filepath.FromSlash(source), version) + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatalf("mkdir: %v", err) + } + manifest := "[template]\nname = \"" + name + "\"\nversion = \"" + version + "\"\ndescription = \"test template\"\n" + if err := os.WriteFile(filepath.Join(dir, "agk-template.toml"), []byte(manifest), 0o644); err != nil { + t.Fatalf("write manifest: %v", err) + } +} + +func newTestCache(t *testing.T) *CacheManager { + t.Helper() + cm, err := NewCacheManager(t.TempDir()) + if err != nil { + t.Fatalf("NewCacheManager: %v", err) + } + return cm +} + +func TestFindByName(t *testing.T) { + cm := newTestCache(t) + writeFakeTemplate(t, cm.BaseDir, "github.com/acme/rag", "latest", "rag-agent") + writeFakeTemplate(t, cm.BaseDir, "github.com/acme/chat", "latest", "chat-agent") + + // Match by manifest name. + byName, err := cm.FindByName("rag-agent") + if err != nil { + t.Fatalf("FindByName: %v", err) + } + if len(byName) != 1 || byName[0].Name != "rag-agent" { + t.Fatalf("byName = %+v, want one rag-agent", byName) + } + + // Match by source path. + bySource, err := cm.FindByName("github.com/acme/chat") + if err != nil { + t.Fatalf("FindByName: %v", err) + } + if len(bySource) != 1 || bySource[0].Name != "chat-agent" { + t.Fatalf("bySource = %+v, want one chat-agent", bySource) + } + + // No match. + none, err := cm.FindByName("does-not-exist") + if err != nil { + t.Fatalf("FindByName: %v", err) + } + if len(none) != 0 { + t.Fatalf("expected no matches, got %+v", none) + } +} + +func TestRemoveByName(t *testing.T) { + cm := newTestCache(t) + writeFakeTemplate(t, cm.BaseDir, "github.com/acme/rag", "latest", "rag-agent") + writeFakeTemplate(t, cm.BaseDir, "github.com/acme/rag", "v1.0.0", "rag-agent") + + // Two cached versions share the same manifest name; both should be removed. + n, err := cm.RemoveByName("rag-agent") + if err != nil { + t.Fatalf("RemoveByName: %v", err) + } + if n != 2 { + t.Fatalf("removed %d, want 2", n) + } + + remaining, err := cm.List() + if err != nil { + t.Fatalf("List: %v", err) + } + if len(remaining) != 0 { + t.Fatalf("expected empty cache, got %+v", remaining) + } +} + +func TestRemoveByNameNotFound(t *testing.T) { + cm := newTestCache(t) + if _, err := cm.RemoveByName("ghost"); err == nil { + t.Fatal("expected error removing a non-existent template") + } +}