From 6d8c45101aaf1936d47340bd2a38e6ae3d4c8c30 Mon Sep 17 00:00:00 2001 From: Jeremy Gooch Date: Fri, 9 Oct 2026 14:59:55 -0500 Subject: [PATCH 1/3] CSTM-566: Adding functionality to setup skills for both managed and unmanaged plugins --- cmd/ui_plugins/init.go | 131 +++++--- cmd/ui_plugins/init_skills.go | 232 ++++++++++++++ cmd/ui_plugins/init_skills_test.go | 374 ++++++++++++++++++++++ cmd/ui_plugins/init_test.go | 51 +-- internal/initialize/fetch.go | 87 +++++ internal/initialize/fetch_subtree_test.go | 93 ++++++ 6 files changed, 910 insertions(+), 58 deletions(-) create mode 100644 cmd/ui_plugins/init_skills.go create mode 100644 cmd/ui_plugins/init_skills_test.go diff --git a/cmd/ui_plugins/init.go b/cmd/ui_plugins/init.go index 2f663bfd..babc155b 100644 --- a/cmd/ui_plugins/init.go +++ b/cmd/ui_plugins/init.go @@ -2,6 +2,7 @@ package ui_plugins import ( "bufio" + "bytes" "context" _ "embed" "fmt" @@ -38,25 +39,26 @@ const ( // initOptions holds the resolved flag values for the init command. type initOptions struct { - name string - alias string - path string - outDir string - port int - portSet bool - force bool + name string + alias string + path string + outDir string + port int + portSet bool + force bool + noSkills bool } -// initDeps carries the command's I/O plus the network-touching seams (scaffold -// and fetchGuide) so the orchestration is testable without GitHub or a live -// tenant. +// initDeps carries the command's I/O plus the network-touching seams +// (fetchArchive and fetchGuide) so the orchestration — including skills +// delivery — is testable without GitHub or a live tenant. type initDeps struct { - client client.Client - in io.Reader - out io.Writer - errOut io.Writer - scaffold func(destDir string) error // fetch + extract the angular starter subtree - fetchGuide func(destPath string) error // fetch + write the generic guide + client client.Client + in io.Reader + out io.Writer + errOut io.Writer + fetchArchive func() (io.ReadCloser, error) // fetch the templates repo tarball + fetchGuide func(destPath string) error // fetch + write the generic guide } func newInitCommand() *cobra.Command { @@ -90,12 +92,12 @@ func newInitCommand() *cobra.Command { } deps := initDeps{ - client: spClient, - in: cmd.InOrStdin(), - out: cmd.OutOrStdout(), - errOut: cmd.ErrOrStderr(), - scaffold: defaultScaffold, - fetchGuide: defaultFetchGuide, + client: spClient, + in: cmd.InOrStdin(), + out: cmd.OutOrStdout(), + errOut: cmd.ErrOrStderr(), + fetchArchive: defaultFetchArchive, + fetchGuide: defaultFetchGuide, } return runInit(context.Background(), deps, opts) }, @@ -107,6 +109,7 @@ func newInitCommand() *cobra.Command { cmd.Flags().StringVar(&opts.outDir, "out-dir", "", "Build output directory (required when attaching with --path)") cmd.Flags().IntVar(&opts.port, "port", defaultDevServerPort, "Local dev server port (used when attaching with --path)") cmd.Flags().BoolVar(&opts.force, "force", false, "Overwrite plugin files this command manages if they already exist") + cmd.Flags().BoolVar(&opts.noSkills, "no-skills", false, "Do not deliver the framework-agnostic agent skills") return cmd } @@ -134,28 +137,51 @@ func runInit(ctx context.Context, deps initDeps, opts initOptions) error { if opts.path != "" { return runPathAttach(r, deps, opts, name, alias, interactive) } - return runNewWorkspace(deps, name, alias) + return runNewWorkspace(deps, opts, name, alias) } -// runNewWorkspace scaffolds the Angular starter into .// and personalizes -// it. It hard-fails if the destination already exists. -func runNewWorkspace(deps initDeps, name, alias string) error { +// runNewWorkspace scaffolds the Angular starter into .//, personalizes +// it, and merges the framework-agnostic agent skills on top. It hard-fails if +// the destination already exists, and is all-or-nothing: any error after the +// destination is created removes it so a failed run leaves nothing behind. +func runNewWorkspace(deps initDeps, opts initOptions, name, alias string) error { destDir := alias if _, err := os.Stat(destDir); err == nil { return fmt.Errorf("error: project '%s' already exists", destDir) } - if err := deps.scaffold(destDir); err != nil { + // Fetch before creating anything so a download failure leaves no partial + // directory behind. + archive, err := fetchArchiveBytes(deps) + if err != nil { + return err + } + + if err := initialize.ExtractSubtree(bytes.NewReader(archive), destDir, angularStarterSubdir); err != nil { // Clean up a partially extracted directory; it did not exist before. _ = os.RemoveAll(destDir) return err } if err := personalizeScaffold(destDir, alias, name); err != nil { + _ = os.RemoveAll(destDir) return err } - fmt.Fprintf(deps.out, "\nCreated plugin workspace %q.\n\nNext steps:\n cd %s\n npm install\n sail ui-plugins create\n", alias, destDir) + var skills skillSummary + if !opts.noSkills { + // Framework flow: path-only skills are skipped; a name collision with a + // starter skill is fatal. Keep the combined copy all-or-nothing. + skills, err = deliverSkills(nil, deps, archive, destDir, true, opts.force) + if err != nil { + _ = os.RemoveAll(destDir) + return err + } + } + + fmt.Fprintf(deps.out, "\nCreated plugin workspace %q.\n", alias) + printSkillSummary(deps.out, opts.noSkills, skills) + fmt.Fprintf(deps.out, "\nNext steps:\n cd %s\n npm install\n sail ui-plugins create\n", destDir) return nil } @@ -192,7 +218,24 @@ func runPathAttach(r *bufio.Reader, deps initDeps, opts initOptions, name, alias return err } - fmt.Fprintf(deps.out, "\nPrepared %q as a UI plugin workspace.\n\nNext steps:\n Review %s and %s\n Install @sailpoint/ui-plugin-sdk per the guide\n sail ui-plugins create\n", opts.path, manifestFileName, genericGuideFileName) + // Deliver the framework-agnostic skills, merging into any existing + // .agents/skills/ without clobbering unrelated user skills. Skills are an + // additive convenience: a delivery failure warns but does not fail the + // attach, which has already written the manifest and guide. + var skills skillSummary + if !opts.noSkills && deps.fetchArchive != nil { + if archive, err := fetchArchiveBytes(deps); err != nil { + fmt.Fprintf(deps.errOut, "Warning: could not fetch agent skills: %v\n", err) + } else if s, err := deliverSkills(r, deps, archive, opts.path, false, opts.force); err != nil { + fmt.Fprintf(deps.errOut, "Warning: could not deliver agent skills: %v\n", err) + } else { + skills = s + } + } + + fmt.Fprintf(deps.out, "\nPrepared %q as a UI plugin workspace.\n", opts.path) + printSkillSummary(deps.out, opts.noSkills, skills) + fmt.Fprintf(deps.out, "\nNext steps:\n Review %s and %s\n Install @sailpoint/ui-plugin-sdk per the guide\n sail ui-plugins create\n", manifestFileName, genericGuideFileName) return nil } @@ -381,16 +424,32 @@ func checkAlias(ctx context.Context, c client.Client, errOut io.Writer, alias st // user knows the alias is re-checked later and how to resolve an unexpected miss. const aliasNotValidatedHint = "The alias will be checked again by `sail ui-plugins create`; fix your CLI authentication if this is unexpected." -// defaultScaffold fetches the templates repo archive and extracts the Angular -// starter subtree into destDir (no template substitution — personalization -// happens afterward). -func defaultScaffold(destDir string) error { - body, err := initialize.FetchArchive(uiPluginTemplatesRepoOwner, uiPluginTemplatesRepoName, uiPluginTemplatesBranch) +// defaultFetchArchive downloads the templates repo tarball. The caller buffers +// it so both the starter extraction and the skills delivery read from a single +// download. +func defaultFetchArchive() (io.ReadCloser, error) { + return initialize.FetchArchive(uiPluginTemplatesRepoOwner, uiPluginTemplatesRepoName, uiPluginTemplatesBranch) +} + +// fetchArchiveBytes fetches and buffers the templates tarball so it can be read +// more than once (starter extraction, then skills delivery). +func fetchArchiveBytes(deps initDeps) ([]byte, error) { + body, err := deps.fetchArchive() if err != nil { - return err + return nil, err } defer body.Close() - return initialize.ExtractSubtree(body, destDir, angularStarterSubdir) + return io.ReadAll(body) +} + +// printSkillSummary surfaces what skills delivery did, so brand-new (unprompted) +// writes are still visible. Nothing is printed when no skills were delivered +// (disabled, or none present in the template) — the absence is unremarkable. +func printSkillSummary(out io.Writer, noSkills bool, s skillSummary) { + if noSkills || s.empty() { + return + } + fmt.Fprintf(out, "Agent skills: %s.\n", s) } // defaultFetchGuide downloads the generic plugin guide from the templates repo diff --git a/cmd/ui_plugins/init_skills.go b/cmd/ui_plugins/init_skills.go new file mode 100644 index 00000000..a371fd69 --- /dev/null +++ b/cmd/ui_plugins/init_skills.go @@ -0,0 +1,232 @@ +package ui_plugins + +import ( + "bufio" + "bytes" + "fmt" + "io" + "os" + "path/filepath" + "regexp" + "strings" + + "github.com/sailpoint-oss/sailpoint-cli/internal/initialize" +) + +const ( + // agentSkillsSubdir is the repository-root skills location inside the + // templates archive (framework-agnostic skills). + agentSkillsSubdir = ".agents/skills" +) + +// agentSkillsDir is the OS-specific destination path (relative to a project +// root) the skills are delivered into. +var agentSkillsDir = filepath.Join(".agents", "skills") + +// pathOnlyFrontmatter matches the per-skill opt-out that keeps a repo-wide skill +// out of framework starters (delivered only by `init --path`). It is matched +// anywhere in the SKILL.md YAML frontmatter, nesting-agnostic, to stay +// consistent with the templates-repo PRB skill-collision check. +var pathOnlyFrontmatter = regexp.MustCompile(`(?m)^\s*sailpoint-merge:\s*path-only\s*$`) + +// skillSummary tallies what a skills-delivery pass did, so silent (non-prompted) +// writes are still surfaced to the user. +type skillSummary struct { + added int + skipped int + overwritten int +} + +func (s skillSummary) empty() bool { + return s.added == 0 && s.skipped == 0 && s.overwritten == 0 +} + +func (s skillSummary) String() string { + return fmt.Sprintf("added %d, skipped %d (already present), overwritten %d", s.added, s.skipped, s.overwritten) +} + +// deliverSkills merges the repository-wide (framework-agnostic) agent skills from +// the templates archive into destDir/.agents/skills/. +// +// - framework == true (new-workspace flow): skills whose frontmatter sets +// `sailpoint-merge: path-only` are skipped (a framework-specific version is +// authored in the starter instead). A name already present in the +// destination is a collision with a scaffolded starter skill — a template +// authoring error — and fails loudly. +// - framework == false (attach flow): every repo-wide skill is delivered, +// including path-only ones (this is their only delivery path). A name already +// present belongs to the user; it is left untouched unless overwrite is +// confirmed (prompt, or force). +// +// A template that ships no repo-wide skills is handled gracefully: the pass is a +// no-op returning an empty summary and no error. Only skill folders the CLI +// ships are ever touched. +func deliverSkills(r *bufio.Reader, deps initDeps, archive []byte, destDir string, framework, force bool) (skillSummary, error) { + var sum skillSummary + + staging, err := os.MkdirTemp("", "sp-ui-plugin-skills-") + if err != nil { + return sum, fmt.Errorf("failed to create skills staging directory: %w", err) + } + defer os.RemoveAll(staging) + + counts, err := initialize.ExtractSubtrees(bytes.NewReader(archive), staging, []initialize.SubtreeMapping{ + {Subdir: agentSkillsSubdir, Dest: "."}, + }) + if err != nil { + return sum, err + } + if counts[agentSkillsSubdir] == 0 { + // The template carries no repository-wide skills — skip gracefully. + return sum, nil + } + + folders, err := listSkillFolders(staging) + if err != nil { + return sum, err + } + + destSkills := filepath.Join(destDir, agentSkillsDir) + for _, name := range folders { + src := filepath.Join(staging, name) + + if framework && isPathOnlySkill(filepath.Join(src, "SKILL.md")) { + // Opted out of framework starters; delivered only by `init --path`. + continue + } + + target := filepath.Join(destSkills, name) + _, statErr := os.Stat(target) + exists := statErr == nil + + if !exists { + if err := copyTree(src, target); err != nil { + return sum, err + } + sum.added++ + continue + } + + if framework { + // A starter scaffolded this same skill name — the root and framework + // sources collide and would silently overwrite. Fail loudly rather + // than interleave; mark the repo-wide skill `sailpoint-merge: + // path-only` if it should not merge into frameworks. + return sum, fmt.Errorf("agent skill %q is delivered by both the repository-wide skills and the scaffolded starter; rename one, or mark the repo-wide skill `sailpoint-merge: path-only`", name) + } + + // Attach flow: the existing folder is the user's. Confirm before touching. + if !force { + overwrite, err := promptYesNoShared(r, deps.out, fmt.Sprintf("Agent skill %q already exists. Overwrite?", name)) + if err != nil { + return sum, err + } + if !overwrite { + fmt.Fprintf(deps.out, "Skipped skill %s\n", name) + sum.skipped++ + continue + } + } + if err := os.RemoveAll(target); err != nil { + return sum, fmt.Errorf("failed to replace skill %q: %w", name, err) + } + if err := copyTree(src, target); err != nil { + return sum, err + } + sum.overwritten++ + } + + return sum, nil +} + +// listSkillFolders returns the names of the immediate skill directories staged +// under dir, in sorted order (os.ReadDir sorts), so delivery and prompting are +// deterministic. +func listSkillFolders(dir string) ([]string, error) { + entries, err := os.ReadDir(dir) + if err != nil { + return nil, fmt.Errorf("failed to read staged skills: %w", err) + } + names := make([]string, 0, len(entries)) + for _, e := range entries { + if e.IsDir() { + names = append(names, e.Name()) + } + } + return names, nil +} + +// isPathOnlySkill reports whether the skill's SKILL.md frontmatter opts out of +// framework-starter delivery. A missing or unreadable SKILL.md is treated as not +// opted out (the default is to merge everywhere). +func isPathOnlySkill(skillMdPath string) bool { + data, err := os.ReadFile(skillMdPath) + if err != nil { + return false + } + fm, ok := yamlFrontmatter(data) + if !ok { + return false + } + return pathOnlyFrontmatter.Match(fm) +} + +// yamlFrontmatter returns the bytes between the leading `---` fence and the next +// `---` line. ok is false when the document has no frontmatter block. +func yamlFrontmatter(data []byte) ([]byte, bool) { + lines := strings.Split(string(data), "\n") + if len(lines) == 0 || strings.TrimSpace(lines[0]) != "---" { + return nil, false + } + for i := 1; i < len(lines); i++ { + if strings.TrimSpace(lines[i]) == "---" { + return []byte(strings.Join(lines[1:i], "\n")), true + } + } + return nil, false +} + +// copyTree recursively copies the directory src to dst, creating parents as +// needed. Regular files and directories only (skills are Markdown); this mirrors +// extractGzipTar, which drops symlinks and special files. +func copyTree(src, dst string) error { + info, err := os.Stat(src) + if err != nil { + return err + } + if info.IsDir() { + if err := os.MkdirAll(dst, 0755); err != nil { + return err + } + entries, err := os.ReadDir(src) + if err != nil { + return err + } + for _, e := range entries { + if err := copyTree(filepath.Join(src, e.Name()), filepath.Join(dst, e.Name())); err != nil { + return err + } + } + return nil + } + if !info.Mode().IsRegular() { + return nil + } + if err := os.MkdirAll(filepath.Dir(dst), 0755); err != nil { + return err + } + in, err := os.Open(src) + if err != nil { + return err + } + defer in.Close() + out, err := os.OpenFile(dst, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0644) + if err != nil { + return err + } + if _, err := io.Copy(out, in); err != nil { + out.Close() + return err + } + return out.Close() +} diff --git a/cmd/ui_plugins/init_skills_test.go b/cmd/ui_plugins/init_skills_test.go new file mode 100644 index 00000000..5a4598aa --- /dev/null +++ b/cmd/ui_plugins/init_skills_test.go @@ -0,0 +1,374 @@ +package ui_plugins + +import ( + "archive/tar" + "bytes" + "compress/gzip" + "context" + "io" + "os" + "path/filepath" + "strings" + "testing" +) + +const skillsTarballRoot = "ui-plugin-templates-main" + +// buildTemplatesArchive builds a gzipped tar archive rooted at skillsTarballRoot +// from files keyed by repo-root-relative path (forward slashes). Parent +// directories are created implicitly by extraction, so only file entries are +// written. +func buildTemplatesArchive(files map[string]string) []byte { + var buf bytes.Buffer + gw := gzip.NewWriter(&buf) + tw := tar.NewWriter(gw) + root := skillsTarballRoot + "/" + _ = tw.WriteHeader(&tar.Header{Name: root, Typeflag: tar.TypeDir, Mode: 0755}) + for name, body := range files { + _ = tw.WriteHeader(&tar.Header{Name: root + name, Typeflag: tar.TypeReg, Size: int64(len(body)), Mode: 0644}) + _, _ = tw.Write([]byte(body)) + } + _ = tw.Close() + _ = gw.Close() + return buf.Bytes() +} + +// archiveFetcher returns a fetchArchive seam that serves a fresh reader over the +// given archive on each call (callers buffer it, but model independent reads). +func archiveFetcher(t *testing.T, archive []byte) func() (io.ReadCloser, error) { + t.Helper() + return func() (io.ReadCloser, error) { + return io.NopCloser(bytes.NewReader(archive)), nil + } +} + +// starterTarFiles builds the file set for a templates archive: the Angular +// starter fixture under angular/starter/, plus any extra repo-relative files +// (e.g. root or starter skills) supplied by the caller. +func starterTarFiles(extra map[string]string) map[string]string { + files := map[string]string{} + for rel, body := range starterFixtureFiles() { + files["angular/starter/"+rel] = body + } + for rel, body := range extra { + files[rel] = body + } + return files +} + +func mustExist(t *testing.T, path string) { + t.Helper() + if _, err := os.Stat(path); err != nil { + t.Errorf("expected %s to exist: %v", path, err) + } +} + +func mustNotExist(t *testing.T, path string) { + t.Helper() + if _, err := os.Stat(path); err == nil { + t.Errorf("expected %s NOT to exist", path) + } +} + +func skillMD(name, extraFrontmatter string) string { + fm := "name: " + name + "\ndescription: test skill " + name + if extraFrontmatter != "" { + fm += "\n" + extraFrontmatter + } + return "---\n" + fm + "\n---\n\n# " + name + "\n\nbody\n" +} + +func TestNewWorkspace_DeliversRootSkills(t *testing.T) { + workdir := initTestChdir(t) + archive := buildTemplatesArchive(starterTarFiles(map[string]string{ + ".agents/skills/plugin-lifecycle-cli/SKILL.md": skillMD("plugin-lifecycle-cli", ""), + "angular/starter/.agents/skills/plugin-routing/SKILL.md": skillMD("plugin-routing", ""), + })) + out := &strings.Builder{} + deps := initDeps{ + client: &fakeClient{getStatus: 200}, + in: strings.NewReader(""), + out: out, + errOut: &strings.Builder{}, + fetchArchive: archiveFetcher(t, archive), + } + + if err := runInit(context.Background(), deps, initOptions{name: "My Plugin"}); err != nil { + t.Fatalf("runInit: %v", err) + } + + dest := filepath.Join(workdir, "my-plugin") + // Root agnostic skill merged in. + mustExist(t, filepath.Join(dest, ".agents", "skills", "plugin-lifecycle-cli", "SKILL.md")) + // Starter's own skill came along with the starter subtree. + mustExist(t, filepath.Join(dest, ".agents", "skills", "plugin-routing", "SKILL.md")) + if !strings.Contains(out.String(), "Agent skills: added 1") { + t.Errorf("summary should report one added root skill, got: %q", out.String()) + } +} + +func TestNewWorkspace_SkipsPathOnlySkill(t *testing.T) { + workdir := initTestChdir(t) + archive := buildTemplatesArchive(starterTarFiles(map[string]string{ + ".agents/skills/agnostic/SKILL.md": skillMD("agnostic", ""), + ".agents/skills/path-only/SKILL.md": skillMD("path-only", "metadata:\n sailpoint-merge: path-only"), + })) + deps := initDeps{ + client: &fakeClient{getStatus: 200}, + in: strings.NewReader(""), + out: &strings.Builder{}, + errOut: &strings.Builder{}, + fetchArchive: archiveFetcher(t, archive), + } + + if err := runInit(context.Background(), deps, initOptions{name: "My Plugin"}); err != nil { + t.Fatalf("runInit: %v", err) + } + + dest := filepath.Join(workdir, "my-plugin") + mustExist(t, filepath.Join(dest, ".agents", "skills", "agnostic", "SKILL.md")) + mustNotExist(t, filepath.Join(dest, ".agents", "skills", "path-only", "SKILL.md")) +} + +func TestNewWorkspace_SkillCollision_FailsLoudly(t *testing.T) { + workdir := initTestChdir(t) + // Same folder name at the repo root and in the starter. + archive := buildTemplatesArchive(starterTarFiles(map[string]string{ + ".agents/skills/dup/SKILL.md": skillMD("dup", ""), + "angular/starter/.agents/skills/dup/SKILL.md": skillMD("dup", ""), + })) + deps := initDeps{ + client: &fakeClient{getStatus: 200}, + in: strings.NewReader(""), + out: &strings.Builder{}, + errOut: &strings.Builder{}, + fetchArchive: archiveFetcher(t, archive), + } + + err := runInit(context.Background(), deps, initOptions{name: "My Plugin"}) + if err == nil || !strings.Contains(err.Error(), "both") { + t.Fatalf("expected a loud collision error, got %v", err) + } + // All-or-nothing: the destination must be removed on failure. + mustNotExist(t, filepath.Join(workdir, "my-plugin")) +} + +func TestNewWorkspace_PathOnlyCollisionAllowed(t *testing.T) { + workdir := initTestChdir(t) + // A path-only root skill may share a name with a starter skill: it is never + // merged into the framework, so there is no collision. + archive := buildTemplatesArchive(starterTarFiles(map[string]string{ + ".agents/skills/dup/SKILL.md": skillMD("dup", "metadata:\n sailpoint-merge: path-only"), + "angular/starter/.agents/skills/dup/SKILL.md": skillMD("dup", ""), + })) + deps := initDeps{ + client: &fakeClient{getStatus: 200}, + in: strings.NewReader(""), + out: &strings.Builder{}, + errOut: &strings.Builder{}, + fetchArchive: archiveFetcher(t, archive), + } + + if err := runInit(context.Background(), deps, initOptions{name: "My Plugin"}); err != nil { + t.Fatalf("path-only name reuse must not collide, got %v", err) + } + // The starter's version is the one that lands. + mustExist(t, filepath.Join(workdir, "my-plugin", ".agents", "skills", "dup", "SKILL.md")) +} + +func TestNewWorkspace_NoSkillsFlag(t *testing.T) { + workdir := initTestChdir(t) + archive := buildTemplatesArchive(starterTarFiles(map[string]string{ + ".agents/skills/agnostic/SKILL.md": skillMD("agnostic", ""), + })) + out := &strings.Builder{} + deps := initDeps{ + client: &fakeClient{getStatus: 200}, + in: strings.NewReader(""), + out: out, + errOut: &strings.Builder{}, + fetchArchive: archiveFetcher(t, archive), + } + + if err := runInit(context.Background(), deps, initOptions{name: "My Plugin", noSkills: true}); err != nil { + t.Fatalf("runInit: %v", err) + } + mustNotExist(t, filepath.Join(workdir, "my-plugin", ".agents", "skills", "agnostic")) + if strings.Contains(out.String(), "Agent skills:") { + t.Errorf("no-skills run should not print a skills summary, got: %q", out.String()) + } +} + +func TestNewWorkspace_TemplateWithoutSkills_Skips(t *testing.T) { + workdir := initTestChdir(t) + // Starter only; no .agents/skills anywhere in the archive. + archive := buildTemplatesArchive(starterTarFiles(nil)) + out := &strings.Builder{} + deps := initDeps{ + client: &fakeClient{getStatus: 200}, + in: strings.NewReader(""), + out: out, + errOut: &strings.Builder{}, + fetchArchive: archiveFetcher(t, archive), + } + + if err := runInit(context.Background(), deps, initOptions{name: "My Plugin"}); err != nil { + t.Fatalf("missing .agents/skills must not fail init, got %v", err) + } + mustExist(t, filepath.Join(workdir, "my-plugin", "package.json")) + if strings.Contains(out.String(), "Agent skills:") { + t.Errorf("no skills in template should print no skills message, got: %q", out.String()) + } +} + +// --- Attach flow --- + +func attachDeps(t *testing.T, stdin string, out, errOut *strings.Builder, extraSkills map[string]string) initDeps { + t.Helper() + return initDeps{ + client: &fakeClient{getStatus: 200}, + in: strings.NewReader(stdin), + out: out, + errOut: errOut, + fetchArchive: archiveFetcher(t, buildTemplatesArchive(starterTarFiles(extraSkills))), + fetchGuide: func(destPath string) error { return os.WriteFile(destPath, []byte("guide"), 0644) }, + } +} + +func TestAttach_MergesWithoutClobbering(t *testing.T) { + target := t.TempDir() + // A pre-existing unrelated user skill must survive untouched. + userSkill := filepath.Join(target, ".agents", "skills", "my-own-skill") + if err := os.MkdirAll(userSkill, 0755); err != nil { + t.Fatalf("seed: %v", err) + } + if err := os.WriteFile(filepath.Join(userSkill, "SKILL.md"), []byte("user content"), 0644); err != nil { + t.Fatalf("seed: %v", err) + } + + out := &strings.Builder{} + deps := attachDeps(t, "", out, &strings.Builder{}, map[string]string{ + ".agents/skills/plugin-lifecycle-cli/SKILL.md": skillMD("plugin-lifecycle-cli", ""), + ".agents/skills/path-only/SKILL.md": skillMD("path-only", "metadata:\n sailpoint-merge: path-only"), + }) + opts := initOptions{name: "My Plugin", path: target, outDir: "./dist/app"} + + if err := runInit(context.Background(), deps, opts); err != nil { + t.Fatalf("runInit: %v", err) + } + + // Repo-wide skills delivered, including the path-only one (attach is its only path). + mustExist(t, filepath.Join(target, ".agents", "skills", "plugin-lifecycle-cli", "SKILL.md")) + mustExist(t, filepath.Join(target, ".agents", "skills", "path-only", "SKILL.md")) + // Unrelated user skill untouched. + if data := readFile(t, filepath.Join(userSkill, "SKILL.md")); data != "user content" { + t.Errorf("unrelated user skill modified: %q", data) + } + if !strings.Contains(out.String(), "Agent skills: added 2") { + t.Errorf("summary should report two added skills, got: %q", out.String()) + } +} + +func TestAttach_OverwritePromptDeclined(t *testing.T) { + target := t.TempDir() + existing := filepath.Join(target, ".agents", "skills", "plugin-lifecycle-cli") + if err := os.MkdirAll(existing, 0755); err != nil { + t.Fatalf("seed: %v", err) + } + if err := os.WriteFile(filepath.Join(existing, "SKILL.md"), []byte("original"), 0644); err != nil { + t.Fatalf("seed: %v", err) + } + + out := &strings.Builder{} + // Decline the single overwrite prompt. + deps := attachDeps(t, "n\n", out, &strings.Builder{}, map[string]string{ + ".agents/skills/plugin-lifecycle-cli/SKILL.md": skillMD("plugin-lifecycle-cli", ""), + }) + opts := initOptions{name: "My Plugin", path: target, outDir: "./dist/app"} + + if err := runInit(context.Background(), deps, opts); err != nil { + t.Fatalf("runInit: %v", err) + } + if data := readFile(t, filepath.Join(existing, "SKILL.md")); data != "original" { + t.Errorf("declined overwrite should leave the skill untouched, got %q", data) + } + if !strings.Contains(out.String(), "skipped 1") { + t.Errorf("summary should report a skip, got: %q", out.String()) + } +} + +func TestAttach_ForceOverwrites(t *testing.T) { + target := t.TempDir() + existing := filepath.Join(target, ".agents", "skills", "plugin-lifecycle-cli") + if err := os.MkdirAll(existing, 0755); err != nil { + t.Fatalf("seed: %v", err) + } + if err := os.WriteFile(filepath.Join(existing, "SKILL.md"), []byte("original"), 0644); err != nil { + t.Fatalf("seed: %v", err) + } + + out := &strings.Builder{} + deps := attachDeps(t, "", out, &strings.Builder{}, map[string]string{ + ".agents/skills/plugin-lifecycle-cli/SKILL.md": skillMD("plugin-lifecycle-cli", ""), + }) + opts := initOptions{name: "My Plugin", path: target, outDir: "./dist/app", force: true} + + if err := runInit(context.Background(), deps, opts); err != nil { + t.Fatalf("runInit: %v", err) + } + if data := readFile(t, filepath.Join(existing, "SKILL.md")); !strings.Contains(data, "# plugin-lifecycle-cli") { + t.Errorf("force should overwrite with the delivered skill, got %q", data) + } + if !strings.Contains(out.String(), "overwritten 1") { + t.Errorf("summary should report an overwrite, got: %q", out.String()) + } +} + +func TestAttach_IdempotentReRun(t *testing.T) { + target := t.TempDir() + extra := map[string]string{ + ".agents/skills/plugin-lifecycle-cli/SKILL.md": skillMD("plugin-lifecycle-cli", ""), + } + + // First run: fresh add. + deps1 := attachDeps(t, "", &strings.Builder{}, &strings.Builder{}, extra) + opts := initOptions{name: "My Plugin", path: target, outDir: "./dist/app"} + if err := runInit(context.Background(), deps1, opts); err != nil { + t.Fatalf("first runInit: %v", err) + } + + // Second run: folder exists; decline the overwrite. No error, no duplicate. + out := &strings.Builder{} + deps2 := attachDeps(t, "n\n", out, &strings.Builder{}, extra) + if err := runInit(context.Background(), deps2, opts); err != nil { + t.Fatalf("second runInit: %v", err) + } + mustExist(t, filepath.Join(target, ".agents", "skills", "plugin-lifecycle-cli", "SKILL.md")) + if !strings.Contains(out.String(), "skipped 1") { + t.Errorf("re-run should skip the already-present skill, got: %q", out.String()) + } +} + +func TestDeliverSkills_PathOnlyUnit(t *testing.T) { + dir := t.TempDir() + md := filepath.Join(dir, "SKILL.md") + if err := os.WriteFile(md, []byte(skillMD("x", "metadata:\n sailpoint-merge: path-only")), 0644); err != nil { + t.Fatalf("seed: %v", err) + } + if !isPathOnlySkill(md) { + t.Error("expected path-only to be detected") + } + if err := os.WriteFile(md, []byte(skillMD("x", "")), 0644); err != nil { + t.Fatalf("seed: %v", err) + } + if isPathOnlySkill(md) { + t.Error("unflagged skill must not be path-only") + } + // Nesting-agnostic: a top-level key (no metadata parent) still counts, matching the PRB. + if err := os.WriteFile(md, []byte(skillMD("x", "sailpoint-merge: path-only")), 0644); err != nil { + t.Fatalf("seed: %v", err) + } + if !isPathOnlySkill(md) { + t.Error("expected nesting-agnostic path-only detection") + } +} diff --git a/cmd/ui_plugins/init_test.go b/cmd/ui_plugins/init_test.go index 9661f6f0..dbad33da 100644 --- a/cmd/ui_plugins/init_test.go +++ b/cmd/ui_plugins/init_test.go @@ -5,6 +5,7 @@ import ( "context" "encoding/json" "errors" + "io" "os" "path/filepath" "strings" @@ -51,11 +52,11 @@ func TestGenerateWorkspaceManifest_Valid(t *testing.T) { } } -// writeStarterFixture creates a minimal copy of the Angular starter's -// personalization-relevant files (with the sentinel token) under dir. -func writeStarterFixture(t *testing.T, dir string) { - t.Helper() - files := map[string]string{ +// starterFixtureFiles returns the Angular starter's personalization-relevant +// files (keyed by starter-relative path, with the sentinel token) shared by the +// on-disk fixture and the in-memory tarball builder. +func starterFixtureFiles() map[string]string { + return map[string]string{ "angular.json": `{ "projects": { "starter": { @@ -76,7 +77,13 @@ func writeStarterFixture(t *testing.T, dir string) { "src/app/app.spec.ts": "expect(x).toContain('Hello, starter');", "src/index.html": "Starter", } - for rel, content := range files { +} + +// writeStarterFixture creates a minimal copy of the Angular starter's +// personalization-relevant files (with the sentinel token) under dir. +func writeStarterFixture(t *testing.T, dir string) { + t.Helper() + for rel, content := range starterFixtureFiles() { full := filepath.Join(dir, filepath.FromSlash(rel)) if err := os.MkdirAll(filepath.Dir(full), 0755); err != nil { t.Fatalf("mkdir: %v", err) @@ -398,14 +405,11 @@ func TestRunInit_PathConflict_SkipWithoutForce(t *testing.T) { func TestScaffoldFlow_PersonalizesFromStub(t *testing.T) { workdir := initTestChdir(t) deps := initDeps{ - client: &fakeClient{getStatus: 200}, - in: strings.NewReader(""), - out: &strings.Builder{}, - errOut: &strings.Builder{}, - scaffold: func(destDir string) error { - writeStarterFixture(t, destDir) - return nil - }, + client: &fakeClient{getStatus: 200}, + in: strings.NewReader(""), + out: &strings.Builder{}, + errOut: &strings.Builder{}, + fetchArchive: archiveFetcher(t, buildTemplatesArchive(starterTarFiles(nil))), } opts := initOptions{name: "My Plugin"} // new-workspace, headless; alias slug = my-plugin @@ -425,21 +429,24 @@ func TestScaffoldFlow_DestExists_HardFail(t *testing.T) { if err := os.Mkdir(filepath.Join(workdir, "my-plugin"), 0755); err != nil { t.Fatalf("mkdir: %v", err) } - scaffoldCalled := false + fetchCalled := false deps := initDeps{ - client: &fakeClient{getStatus: 200}, - in: strings.NewReader(""), - out: &strings.Builder{}, - errOut: &strings.Builder{}, - scaffold: func(string) error { scaffoldCalled = true; return nil }, + client: &fakeClient{getStatus: 200}, + in: strings.NewReader(""), + out: &strings.Builder{}, + errOut: &strings.Builder{}, + fetchArchive: func() (io.ReadCloser, error) { + fetchCalled = true + return nil, errors.New("must not fetch") + }, } opts := initOptions{name: "My Plugin"} if err := runInit(context.Background(), deps, opts); err == nil || !strings.Contains(err.Error(), "already exists") { t.Fatalf("expected already-exists error, got %v", err) } - if scaffoldCalled { - t.Error("scaffold must not run when destination already exists") + if fetchCalled { + t.Error("archive must not be fetched when destination already exists") } } diff --git a/internal/initialize/fetch.go b/internal/initialize/fetch.go index f69b1ecf..265260fa 100644 --- a/internal/initialize/fetch.go +++ b/internal/initialize/fetch.go @@ -203,6 +203,93 @@ func ExtractSubtree(tarball io.Reader, destDir, subdir string) error { return nil } +// SubtreeMapping routes entries under the archive's / (relative to the +// archive root) to , a path relative to the extraction destination root. +// Dest "." places the subtree's contents at the destination root. +type SubtreeMapping struct { + Subdir string // archive subdir below the archive root, e.g. "angular/starter" or ".agents/skills" + Dest string // destination-relative path, e.g. "." or ".agents/skills" +} + +// ExtractSubtrees extracts, in a single streaming pass, the entries under each +// mapping's Subdir into that mapping's Dest (relative to destRoot). Each archive +// entry is routed to the first mapping it matches. It returns a per-Subdir count +// of matched entries so callers can tell which subtrees were present. +// +// Unlike ExtractSubtree it is merge-tolerant: destRoot may already exist and is +// written into in place, and a mapping that matches nothing is not an error (its +// count is simply zero) — the caller decides whether an absent subtree is fatal +// or a graceful skip. The path-traversal / Zip-Slip guards in extractGzipTar are +// preserved. +func ExtractSubtrees(tarball io.Reader, destRoot string, mappings []SubtreeMapping) (map[string]int, error) { + if destRoot == "" { + return nil, errors.New("destination directory cannot be empty") + } + if len(mappings) == 0 { + return nil, errors.New("at least one mapping is required") + } + + absRoot, err := filepath.Abs(destRoot) + if err != nil { + return nil, fmt.Errorf("failed to resolve destination path: %w", err) + } + + sep := string(filepath.Separator) + type prepared struct { + key string + osSubdir string + prefix string + osDest string + } + preps := make([]prepared, 0, len(mappings)) + counts := make(map[string]int, len(mappings)) + for _, m := range mappings { + if strings.TrimSpace(m.Subdir) == "" { + return nil, errors.New("mapping subdir cannot be empty") + } + osSubdir := filepath.Clean(filepath.FromSlash(strings.Trim(m.Subdir, "/"))) + osDest := filepath.Clean(filepath.FromSlash(strings.Trim(m.Dest, "/"))) + preps = append(preps, prepared{ + key: m.Subdir, + osSubdir: osSubdir, + prefix: osSubdir + sep, + osDest: osDest, + }) + counts[m.Subdir] = 0 + } + + // strip routes each entry (whose first path component is the archive root) + // to the first matching mapping, returning its destination-relative path. + // The subdir's own directory entry maps to the mapping's dest directory. + strip := func(osName string) (string, bool) { + parts := strings.SplitN(osName, sep, 2) + if len(parts) < 2 { + return "", false + } + below := parts[1] + for _, p := range preps { + var rel string + switch { + case below == p.osSubdir: + rel = "" + case strings.HasPrefix(below, p.prefix): + rel = strings.TrimPrefix(below, p.prefix) + default: + continue + } + counts[p.key]++ + return filepath.Join(p.osDest, rel), true + } + return "", false + } + + log.Info("Extracting subtrees", "mappings", len(mappings), "dest", destRoot) + if _, err := extractGzipTar(tarball, absRoot, strip); err != nil { + return counts, err + } + return counts, nil +} + // stripArchiveRoot removes the single top-level directory component that GitHub // archives wrap everything in (e.g. "repo-main/"). It is the default strip used // by ExtractAndInitProject. diff --git a/internal/initialize/fetch_subtree_test.go b/internal/initialize/fetch_subtree_test.go index 3917d7cd..bd190839 100644 --- a/internal/initialize/fetch_subtree_test.go +++ b/internal/initialize/fetch_subtree_test.go @@ -157,6 +157,99 @@ func TestExtractSubtree_RejectsZipSlip(t *testing.T) { } } +func TestExtractSubtrees_RoutesTwoSubtrees(t *testing.T) { + workdir := chdirTemp(t) + entries := append(starterFixtureEntries(), + tarEntry{name: ".agents", dir: true}, + tarEntry{name: ".agents/skills", dir: true}, + tarEntry{name: ".agents/skills/agnostic", dir: true}, + tarEntry{name: ".agents/skills/agnostic/SKILL.md", body: "root skill"}, + ) + dest := "proj" + + counts, err := ExtractSubtrees(bytes.NewReader(buildTemplatesTarball(entries)), dest, []SubtreeMapping{ + {Subdir: "angular/starter", Dest: "."}, + {Subdir: ".agents/skills", Dest: ".agents/skills"}, + }) + if err != nil { + t.Fatalf("ExtractSubtrees: %v", err) + } + + root := filepath.Join(workdir, dest) + // Starter subtree lands at the destination root, prefix stripped. + mustExist(t, filepath.Join(root, "package.json")) + mustExist(t, filepath.Join(root, "src", "app", "app.ts")) + // Root skills land under .agents/skills, not at the root. + mustExist(t, filepath.Join(root, ".agents", "skills", "agnostic", "SKILL.md")) + // Repo-root siblings outside both mappings are ignored. + mustNotExist(t, filepath.Join(root, "README.md")) + + if counts["angular/starter"] == 0 { + t.Error("starter mapping should have matched entries") + } + if counts[".agents/skills"] == 0 { + t.Error("skills mapping should have matched entries") + } +} + +func TestExtractSubtrees_AbsentMappingIsZeroNotError(t *testing.T) { + chdirTemp(t) + counts, err := ExtractSubtrees(bytes.NewReader(buildTemplatesTarball(starterFixtureEntries())), "proj", []SubtreeMapping{ + {Subdir: ".agents/skills", Dest: ".agents/skills"}, + }) + if err != nil { + t.Fatalf("absent subtree must not error: %v", err) + } + if counts[".agents/skills"] != 0 { + t.Errorf("absent subtree count = %d, want 0", counts[".agents/skills"]) + } +} + +func TestExtractSubtrees_MergeIntoExistingDest(t *testing.T) { + workdir := chdirTemp(t) + dest := "proj" + // Pre-create the destination with an unrelated file: ExtractSubtrees must + // merge in place rather than refuse an existing directory. + if err := os.MkdirAll(filepath.Join(workdir, dest), 0755); err != nil { + t.Fatalf("mkdir: %v", err) + } + if err := os.WriteFile(filepath.Join(workdir, dest, "keep.txt"), []byte("keep"), 0644); err != nil { + t.Fatalf("seed: %v", err) + } + + entries := []tarEntry{ + {name: ".agents", dir: true}, + {name: ".agents/skills", dir: true}, + {name: ".agents/skills/x", dir: true}, + {name: ".agents/skills/x/SKILL.md", body: "x"}, + } + if _, err := ExtractSubtrees(bytes.NewReader(buildTemplatesTarball(entries)), dest, []SubtreeMapping{ + {Subdir: ".agents/skills", Dest: ".agents/skills"}, + }); err != nil { + t.Fatalf("ExtractSubtrees into existing dest: %v", err) + } + mustExist(t, filepath.Join(workdir, dest, "keep.txt")) + mustExist(t, filepath.Join(workdir, dest, ".agents", "skills", "x", "SKILL.md")) +} + +func TestExtractSubtrees_RejectsZipSlip(t *testing.T) { + workdir := chdirTemp(t) + entries := []tarEntry{ + {name: ".agents", dir: true}, + {name: ".agents/skills", dir: true}, + {name: ".agents/skills/../../../evil.txt", body: "must not be written"}, + } + _, err := ExtractSubtrees(bytes.NewReader(buildTemplatesTarball(entries)), "proj", []SubtreeMapping{ + {Subdir: ".agents/skills", Dest: ".agents/skills"}, + }) + if err == nil || !contains(err.Error(), "unsafe path") { + t.Fatalf("expected unsafe-path error, got %v", err) + } + if _, statErr := os.Stat(filepath.Join(workdir, "evil.txt")); statErr == nil { + t.Fatal("Zip Slip must not create a file outside the destination") + } +} + func mustExist(t *testing.T, path string) { t.Helper() if _, err := os.Stat(path); err != nil { From e76efd38c75108ef0e4b7c3d89a4a995f14ca9c9 Mon Sep 17 00:00:00 2001 From: Jeremy Gooch Date: Fri, 9 Oct 2026 15:35:57 -0500 Subject: [PATCH 2/3] CSTM-566: Fixing an issue with --no-skills in an angular plugin copying agent skills --- cmd/ui_plugins/init.go | 9 ++++++++- cmd/ui_plugins/init_skills.go | 13 +++++++++++++ cmd/ui_plugins/init_skills_test.go | 14 ++++++++++++-- 3 files changed, 33 insertions(+), 3 deletions(-) diff --git a/cmd/ui_plugins/init.go b/cmd/ui_plugins/init.go index babc155b..acff3e05 100644 --- a/cmd/ui_plugins/init.go +++ b/cmd/ui_plugins/init.go @@ -169,7 +169,14 @@ func runNewWorkspace(deps initDeps, opts initOptions, name, alias string) error } var skills skillSummary - if !opts.noSkills { + if opts.noSkills { + // The starter subtree bundles its own framework skills; --no-skills means + // deliver none, so remove what the starter carried in. + if err := removeStarterSkills(destDir); err != nil { + _ = os.RemoveAll(destDir) + return err + } + } else { // Framework flow: path-only skills are skipped; a name collision with a // starter skill is fatal. Keep the combined copy all-or-nothing. skills, err = deliverSkills(nil, deps, archive, destDir, true, opts.force) diff --git a/cmd/ui_plugins/init_skills.go b/cmd/ui_plugins/init_skills.go index a371fd69..cb06f7b2 100644 --- a/cmd/ui_plugins/init_skills.go +++ b/cmd/ui_plugins/init_skills.go @@ -139,6 +139,19 @@ func deliverSkills(r *bufio.Reader, deps initDeps, archive []byte, destDir strin return sum, nil } +// removeStarterSkills deletes the skills the scaffolded starter bundled under +// destDir/.agents/skills/, used to honor --no-skills in the new-workspace flow. +// The empty .agents wrapper is removed too when nothing else remains in it. +func removeStarterSkills(destDir string) error { + if err := os.RemoveAll(filepath.Join(destDir, agentSkillsDir)); err != nil { + return err + } + // os.Remove removes .agents only when it is now empty; a non-empty wrapper + // (other agent config) is left intact, so the error is intentionally ignored. + _ = os.Remove(filepath.Join(destDir, ".agents")) + return nil +} + // listSkillFolders returns the names of the immediate skill directories staged // under dir, in sorted order (os.ReadDir sorts), so delivery and prompting are // deterministic. diff --git a/cmd/ui_plugins/init_skills_test.go b/cmd/ui_plugins/init_skills_test.go index 5a4598aa..328cfb89 100644 --- a/cmd/ui_plugins/init_skills_test.go +++ b/cmd/ui_plugins/init_skills_test.go @@ -178,8 +178,11 @@ func TestNewWorkspace_PathOnlyCollisionAllowed(t *testing.T) { func TestNewWorkspace_NoSkillsFlag(t *testing.T) { workdir := initTestChdir(t) + // Both a root agnostic skill and a starter-bundled framework skill: --no-skills + // must deliver neither. archive := buildTemplatesArchive(starterTarFiles(map[string]string{ - ".agents/skills/agnostic/SKILL.md": skillMD("agnostic", ""), + ".agents/skills/agnostic/SKILL.md": skillMD("agnostic", ""), + "angular/starter/.agents/skills/framework-x/SKILL.md": skillMD("framework-x", ""), })) out := &strings.Builder{} deps := initDeps{ @@ -193,7 +196,14 @@ func TestNewWorkspace_NoSkillsFlag(t *testing.T) { if err := runInit(context.Background(), deps, initOptions{name: "My Plugin", noSkills: true}); err != nil { t.Fatalf("runInit: %v", err) } - mustNotExist(t, filepath.Join(workdir, "my-plugin", ".agents", "skills", "agnostic")) + dest := filepath.Join(workdir, "my-plugin") + // Neither the root agnostic skill nor the starter's own framework skill lands, + // and the empty .agents wrapper is cleaned up. + mustNotExist(t, filepath.Join(dest, ".agents", "skills", "agnostic")) + mustNotExist(t, filepath.Join(dest, ".agents", "skills", "framework-x")) + mustNotExist(t, filepath.Join(dest, ".agents")) + // The starter itself is still scaffolded. + mustExist(t, filepath.Join(dest, "package.json")) if strings.Contains(out.String(), "Agent skills:") { t.Errorf("no-skills run should not print a skills summary, got: %q", out.String()) } From 9f9ab619c099e6ddbcb274b8f483e78fba49f8a9 Mon Sep 17 00:00:00 2001 From: Jeremy Gooch Date: Fri, 9 Oct 2026 16:04:36 -0500 Subject: [PATCH 3/3] CSTM-566: Fixing an issue with the total skills count for the angular template --- cmd/ui_plugins/init.go | 10 +++++++++- cmd/ui_plugins/init_skills.go | 19 +++++++++++++++++++ cmd/ui_plugins/init_skills_test.go | 5 +++-- 3 files changed, 31 insertions(+), 3 deletions(-) diff --git a/cmd/ui_plugins/init.go b/cmd/ui_plugins/init.go index acff3e05..e9cb44a5 100644 --- a/cmd/ui_plugins/init.go +++ b/cmd/ui_plugins/init.go @@ -179,11 +179,19 @@ func runNewWorkspace(deps initDeps, opts initOptions, name, alias string) error } else { // Framework flow: path-only skills are skipped; a name collision with a // starter skill is fatal. Keep the combined copy all-or-nothing. - skills, err = deliverSkills(nil, deps, archive, destDir, true, opts.force) + if _, err = deliverSkills(nil, deps, archive, destDir, true, opts.force); err != nil { + _ = os.RemoveAll(destDir) + return err + } + // Report every skill now in the fresh workspace — the starter's own + // bundled skills plus the repo-wide ones merged on top — not just the + // repo-wide delta. A brand-new workspace never skips or overwrites. + total, err := countSkillFolders(filepath.Join(destDir, agentSkillsDir)) if err != nil { _ = os.RemoveAll(destDir) return err } + skills = skillSummary{added: total} } fmt.Fprintf(deps.out, "\nCreated plugin workspace %q.\n", alias) diff --git a/cmd/ui_plugins/init_skills.go b/cmd/ui_plugins/init_skills.go index cb06f7b2..78ba5ddd 100644 --- a/cmd/ui_plugins/init_skills.go +++ b/cmd/ui_plugins/init_skills.go @@ -152,6 +152,25 @@ func removeStarterSkills(destDir string) error { return nil } +// countSkillFolders returns the number of immediate skill directories in dir, +// or 0 if dir does not exist (no skills were delivered). +func countSkillFolders(dir string) (int, error) { + entries, err := os.ReadDir(dir) + if err != nil { + if os.IsNotExist(err) { + return 0, nil + } + return 0, err + } + n := 0 + for _, e := range entries { + if e.IsDir() { + n++ + } + } + return n, nil +} + // listSkillFolders returns the names of the immediate skill directories staged // under dir, in sorted order (os.ReadDir sorts), so delivery and prompting are // deterministic. diff --git a/cmd/ui_plugins/init_skills_test.go b/cmd/ui_plugins/init_skills_test.go index 328cfb89..c8ce0f73 100644 --- a/cmd/ui_plugins/init_skills_test.go +++ b/cmd/ui_plugins/init_skills_test.go @@ -102,8 +102,9 @@ func TestNewWorkspace_DeliversRootSkills(t *testing.T) { mustExist(t, filepath.Join(dest, ".agents", "skills", "plugin-lifecycle-cli", "SKILL.md")) // Starter's own skill came along with the starter subtree. mustExist(t, filepath.Join(dest, ".agents", "skills", "plugin-routing", "SKILL.md")) - if !strings.Contains(out.String(), "Agent skills: added 1") { - t.Errorf("summary should report one added root skill, got: %q", out.String()) + // Summary reports the full set: 1 starter-bundled + 1 repo-wide = 2. + if !strings.Contains(out.String(), "Agent skills: added 2") { + t.Errorf("summary should report all delivered skills, got: %q", out.String()) } }