diff --git a/cmd/prime_context.md b/cmd/prime_context.md index 38e322e..23002f9 100644 --- a/cmd/prime_context.md +++ b/cmd/prime_context.md @@ -16,7 +16,7 @@ Auth uses service account tokens exchanged for JWTs. Check status with: cio auth status ``` -If auth fails or there is no token yet, read `cio skills read cli/auth.md` +If auth fails or there is no token yet, read `cio skills read cio/auth.md` and follow its login flow. Never have the user paste a token into the conversation or pass one as a shell argument. @@ -76,7 +76,7 @@ Skills provide behavioral guidance, multi-step workflows, and gotchas that are N cio skills # list available skills cio skills read fly-api # Customer.io API guidance — routes to sub-files cio skills read fly-api/campaigns.md # campaign workflows, edge wiring, gotchas -cio skills read cli # skills unique to the CLI (onboarding, auth/login, integration, go-live) +cio skills read cio # skills unique to the CLI (onboarding, auth/login, integration, go-live) cio skills read design-studio # Design Studio email creation workflow cio skills read design-studio/nodes.md # node creation, component markup ``` diff --git a/cmd/skills_hint.go b/cmd/skills_hint.go index 7285a3e..9fc4721 100644 --- a/cmd/skills_hint.go +++ b/cmd/skills_hint.go @@ -39,10 +39,14 @@ func shouldHintSkillsInstall(cmd *cobra.Command) bool { return false } - skillPath := filepath.Join(home, ".claude", "skills", "cli", "SKILL.md") - if _, err := os.Stat(skillPath); err == nil { - return false - } + return !bootstrapSkillInstalled(home) +} - return true +// bootstrapSkillInstalled reports whether the bootstrap skill is present in the +// global Claude Code skills directory. Only the current name counts, so a +// leftover under the old generic name keeps the hint firing rather than +// suppressing the re-install that would replace it. +func bootstrapSkillInstalled(home string) bool { + _, err := os.Stat(filepath.Join(home, ".claude", "skills", bootstrapSkillName, "SKILL.md")) + return err == nil } diff --git a/cmd/skills_hint_test.go b/cmd/skills_hint_test.go new file mode 100644 index 0000000..ad10a36 --- /dev/null +++ b/cmd/skills_hint_test.go @@ -0,0 +1,43 @@ +package cmd + +import ( + "os" + "path/filepath" + "testing" +) + +func TestBootstrapSkillInstalled(t *testing.T) { + write := func(home, name string) { + dir := filepath.Join(home, ".claude", "skills", name) + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "SKILL.md"), []byte("---\n---\n"), 0o644); err != nil { + t.Fatal(err) + } + } + + t.Run("nothing installed", func(t *testing.T) { + if bootstrapSkillInstalled(t.TempDir()) { + t.Error("expected the hint to fire on a home with no skills") + } + }) + + t.Run("current name installed", func(t *testing.T) { + home := t.TempDir() + write(home, bootstrapSkillName) + if !bootstrapSkillInstalled(home) { + t.Error("expected the hint to be suppressed once the bootstrap is installed") + } + }) + + // A leftover under the old name is the thing we want replaced, so it must + // not pass as installed and silence the nudge to re-run install. + t.Run("only the legacy name installed", func(t *testing.T) { + home := t.TempDir() + write(home, legacyBootstrapDir) + if bootstrapSkillInstalled(home) { + t.Error("expected a legacy leftover not to count as installed") + } + }) +} diff --git a/cmd/skills_install.go b/cmd/skills_install.go index 62375a9..4703662 100644 --- a/cmd/skills_install.go +++ b/cmd/skills_install.go @@ -4,6 +4,7 @@ import ( "bufio" "context" _ "embed" + "errors" "fmt" "io" "os" @@ -147,10 +148,17 @@ func runSkillsInstall(cmd *cobra.Command, args []string) error { } } + removed, err := removeLegacyInstalls(base, targets, dryRun) + if err != nil { + // The skill itself is installed; a leftover we couldn't clear is worth + // saying out loud but not worth failing the command over. + fmt.Fprintf(cmd.ErrOrStderr(), "cio: %v\n", err) + } + // This CLI speaks JSON for agents and pipes, but a person who ran install at // a terminal gets a readable summary instead. if installWantsHumanOutput(cmd) { - printInstallSummary(cmd.OutOrStdout(), dryRun, installed) + printInstallSummary(cmd.OutOrStdout(), dryRun, installed, removed) return nil } @@ -165,9 +173,43 @@ func runSkillsInstall(cmd *cobra.Command, args []string) error { "dry_run": dryRun, "action": action, "installed": installed, + "removed": removed, }) } +// legacyBootstrapDir is the directory an earlier install wrote the bootstrap +// skill into. Left in place it keeps claiming the generic `cli` name in the +// agent's skill list, which is how it ends up loaded for unrelated CLI work. +const legacyBootstrapDir = "cli" + +// bootstrapBodyMarker is a phrase unique to the SKILL.md body this CLI writes. +// Only a directory whose SKILL.md carries it is ours to replace; a skill +// somebody else happens to have named `cli` is left alone. +const bootstrapBodyMarker = "Customer.io ships an agent-first CLI" + +// removeLegacyInstalls deletes the pre-rename bootstrap directory for each +// target and returns the paths removed. One target failing does not strand the +// others: each is attempted, and the failures come back joined. +func removeLegacyInstalls(base string, targets []installTarget, dryRun bool) ([]string, error) { + var removed []string + var failures error + for _, t := range targets { + dir := filepath.Join(base, t.subdir, legacyBootstrapDir) + body, err := os.ReadFile(filepath.Join(dir, "SKILL.md")) + if err != nil || !strings.Contains(string(body), bootstrapBodyMarker) { + continue + } + if !dryRun { + if err := os.RemoveAll(dir); err != nil { + failures = errors.Join(failures, fmt.Errorf("could not remove the earlier install at %s: %w", dir, err)) + continue + } + } + removed = append(removed, dir) + } + return removed, failures +} + // installWantsHumanOutput is true only at an interactive terminal with no // machine-output flag — --jq and --raw-output both mean the caller wants JSON. func installWantsHumanOutput(cmd *cobra.Command) bool { @@ -179,7 +221,7 @@ func installWantsHumanOutput(cmd *cobra.Command) bool { // printInstallSummary renders the install result for a human, in place of the // JSON that agents get. -func printInstallSummary(w io.Writer, dryRun bool, installed []installedFile) { +func printInstallSummary(w io.Writer, dryRun bool, installed []installedFile, removed []string) { wrote := false width := 0 for _, f := range installed { @@ -219,6 +261,27 @@ func printInstallSummary(w io.Writer, dryRun bool, installed []installedFile) { default: fmt.Fprintln(w, "Your agent picks it up on its next run. Try: cio prime") } + + printRemovedLegacy(w, dryRun, removed) +} + +// printRemovedLegacy tells a human which earlier installs were cleaned up. +func printRemovedLegacy(w io.Writer, dryRun bool, removed []string) { + if len(removed) == 0 { + return + } + + verb := "Removed" + if dryRun { + verb = "Would remove" + } + + fmt.Fprintln(w) + fmt.Fprintf(w, "%s an earlier install that claimed the generic \"cli\" name:\n", verb) + fmt.Fprintln(w) + for _, dir := range removed { + fmt.Fprintf(w, " %s\n", tildeAbbrev(dir)) + } } func targetLabel(name string) string { @@ -316,29 +379,30 @@ func safeRelPath(name string) (string, error) { return clean, nil } -// bootstrapSkillNames are the candidate paths for the entry/bootstrap skill, in -// preference order. The bootstrap is the only skill installed locally; its -// routing index points the agent at every other reference, which is fetched -// from the backend on demand via `cio skills read `. -var bootstrapSkillNames = []string{"cli", "cio"} +// bootstrapSkillName is the path the bootstrap skill is served under, and so the +// directory and frontmatter name it installs as. The bootstrap is the only skill +// installed locally; its routing index points the agent at every other +// reference, fetched from the backend on demand via `cio skills read `. +const bootstrapSkillName = "cio" // selectBootstrap returns the single bootstrap skill to install. The other // skills are intentionally not written to disk — they are served by the API // and read at runtime so their content stays current. +// +// A backend that serves the bootstrap under the retired name is an error rather +// than something to fall back on: installing under that name is what makes an +// agent load this skill for unrelated CLI work, so failing here is better than +// quietly writing it. func selectBootstrap(all []skills.Skill) ([]skills.Skill, error) { - byPath := make(map[string]skills.Skill, len(all)) available := make([]string, 0, len(all)) for _, s := range all { - byPath[s.Path] = s - available = append(available, s.Path) - } - for _, want := range bootstrapSkillNames { - if s, ok := byPath[want]; ok { + if s.Path == bootstrapSkillName { return []skills.Skill{s}, nil } + available = append(available, s.Path) } - err := fmt.Errorf("could not find the bootstrap skill (looked for %s)", - strings.Join(bootstrapSkillNames, ", ")) + + err := fmt.Errorf("could not find the bootstrap skill %q", bootstrapSkillName) output.PrintError(output.CodeGeneralError, err.Error(), map[string]any{ "available_skills": available, }) diff --git a/cmd/skills_install_test.go b/cmd/skills_install_test.go index d211aae..6c87915 100644 --- a/cmd/skills_install_test.go +++ b/cmd/skills_install_test.go @@ -11,6 +11,8 @@ import ( "strings" "testing" "time" + + "github.com/customerio/cli/internal/skills" ) // runSkillsInstall drives `cio skills install` against a test server with a @@ -66,12 +68,12 @@ func TestSkillsInstallBootstrapOnly(t *testing.T) { if result.Scope != "global" { t.Errorf("expected scope global, got %q", result.Scope) } - // Only the bootstrap skill (cli), once per target (claude+codex). + // Only the bootstrap skill (cio), once per target (claude+codex). if len(result.Installed) != 2 { - t.Fatalf("expected 2 entries (cli x claude+codex), got %d: %+v", len(result.Installed), result.Installed) + t.Fatalf("expected 2 entries (cio x claude+codex), got %d: %+v", len(result.Installed), result.Installed) } for _, e := range result.Installed { - if e.Skill != "cli" { + if e.Skill != "cio" { t.Errorf("expected only the bootstrap skill, got %q", e.Skill) } } @@ -87,12 +89,12 @@ func TestSkillsInstallBootstrapOnly(t *testing.T) { // Bootstrap SKILL.md exists for both targets: server-tuned frontmatter plus // a minimal body that just points the agent at `cio prime`. for _, target := range []string{".claude", ".agents"} { - p := filepath.Join(home, target, "skills", "cli", "SKILL.md") + p := filepath.Join(home, target, "skills", "cio", "SKILL.md") data, err := os.ReadFile(p) if err != nil { t.Fatalf("expected bootstrap SKILL.md at %s: %v", p, err) } - if !strings.HasPrefix(string(data), "---\nname: \"cli\"\ndescription: \"Builder onboarding.\"\n---\n") { + if !strings.HasPrefix(string(data), "---\nname: \"cio\"\ndescription: \"Builder onboarding.\"\n---\n") { t.Errorf("expected frontmatter on %s, got:\n%s", p, data) } if !strings.Contains(string(data), "cio prime") { @@ -100,7 +102,7 @@ func TestSkillsInstallBootstrapOnly(t *testing.T) { } } // Bootstrap sub-files are fetched at runtime, not installed. - if _, err := os.Stat(filepath.Join(home, ".claude", "skills", "cli", "onboarding.md")); !os.IsNotExist(err) { + if _, err := os.Stat(filepath.Join(home, ".claude", "skills", "cio", "onboarding.md")); !os.IsNotExist(err) { t.Errorf("bootstrap sub-files must not be installed, got err=%v", err) } } @@ -158,7 +160,7 @@ func TestSkillsInstallTargetSelection(t *testing.T) { if _, err := runInstallCommand(t, srv, home, "--target", "codex"); err != nil { t.Fatalf("install failed: %v", err) } - if _, err := os.Stat(filepath.Join(home, ".agents", "skills", "cli", "SKILL.md")); err != nil { + if _, err := os.Stat(filepath.Join(home, ".agents", "skills", "cio", "SKILL.md")); err != nil { t.Errorf("expected codex install: %v", err) } if _, err := os.Stat(filepath.Join(home, ".claude")); !os.IsNotExist(err) { @@ -187,7 +189,7 @@ func TestSkillsInstallProject(t *testing.T) { if _, err := runInstallCommand(t, srv, home, "--project", "--target", "claude"); err != nil { t.Fatalf("install failed: %v", err) } - if _, err := os.Stat(filepath.Join(proj, ".claude", "skills", "cli", "SKILL.md")); err != nil { + if _, err := os.Stat(filepath.Join(proj, ".claude", "skills", "cio", "SKILL.md")); err != nil { t.Errorf("expected project install under cwd: %v", err) } } @@ -197,7 +199,7 @@ func TestSkillsInstallNoForceSkipsExisting(t *testing.T) { defer srv.Close() home := t.TempDir() - skillDir := filepath.Join(home, ".claude", "skills", "cli") + skillDir := filepath.Join(home, ".claude", "skills", "cio") if err := os.MkdirAll(skillDir, 0o755); err != nil { t.Fatal(err) } @@ -231,8 +233,8 @@ func TestSkillsInstallNoForceSkipsExisting(t *testing.T) { } func TestPrintInstallSummary(t *testing.T) { - claude := installedFile{Skill: "cli", Target: "claude", Dir: "/home/u/.claude/skills/cli", Files: []string{"SKILL.md"}} - codex := installedFile{Skill: "cli", Target: "codex", Dir: "/home/u/.agents/skills/cli", Files: []string{"SKILL.md"}} + claude := installedFile{Skill: "cio", Target: "claude", Dir: "/home/u/.claude/skills/cio", Files: []string{"SKILL.md"}} + codex := installedFile{Skill: "cio", Target: "codex", Dir: "/home/u/.agents/skills/cio", Files: []string{"SKILL.md"}} claudeSkipped := installedFile{Skill: "cli", Target: "claude", Dir: "/home/u/.claude/skills/cli", Files: nil} codexSkipped := installedFile{Skill: "cli", Target: "codex", Dir: "/home/u/.agents/skills/cli", Files: nil} @@ -269,7 +271,7 @@ func TestPrintInstallSummary(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { var b bytes.Buffer - printInstallSummary(&b, tc.dryRun, tc.installed) + printInstallSummary(&b, tc.dryRun, tc.installed, nil) out := b.String() t.Logf("\n%s", out) @@ -354,3 +356,148 @@ func TestReadLineContextCancel(t *testing.T) { t.Fatal("readLineContext did not return on cancellation (hang)") } } + +func TestSelectBootstrap(t *testing.T) { + served := []skills.Skill{ + {Path: "fly-api", Name: "Fly"}, + {Path: bootstrapSkillName, Name: "Current"}, + } + got, err := selectBootstrap(served) + if err != nil { + t.Fatalf("selectBootstrap: %v", err) + } + if len(got) != 1 || got[0].Path != bootstrapSkillName { + t.Fatalf("expected the bootstrap skill, got %+v", got) + } + + // The retired name is not a fallback: installing under it is the bug, so a + // backend still serving it must fail loudly instead. + legacyOnly := []skills.Skill{{Path: legacyBootstrapDir, Name: "Legacy"}} + if _, err := selectBootstrap(legacyOnly); err == nil { + t.Fatal("expected an error when only the retired name is served") + } +} + +func TestRemoveLegacyInstalls(t *testing.T) { + base := t.TempDir() + + writeSKILL := func(subdir, name, body string) string { + dir := filepath.Join(base, subdir, name) + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "SKILL.md"), []byte(body), 0o644); err != nil { + t.Fatal(err) + } + return dir + } + ourBody := "---\nname: cli\n---\n\n" + bootstrapBodyMarker + ", `cio`.\n" + + // Nothing left over. + removed, err := removeLegacyInstalls(base, installTargets, false) + if err != nil { + t.Fatalf("removeLegacyInstalls: %v", err) + } + if len(removed) != 0 { + t.Fatalf("expected nothing removed, got %v", removed) + } + + // A dry run reports the directory without touching it. + ours := writeSKILL(filepath.Join(".claude", "skills"), legacyBootstrapDir, ourBody) + removed, err = removeLegacyInstalls(base, installTargets, true) + if err != nil { + t.Fatalf("removeLegacyInstalls dry run: %v", err) + } + if len(removed) != 1 || removed[0] != ours { + t.Fatalf("expected the dry run to report %q, got %v", ours, removed) + } + if _, err := os.Stat(ours); err != nil { + t.Fatalf("dry run must not delete, got err=%v", err) + } + + // Somebody else's skill of the same name is left alone; ours is removed. + theirs := writeSKILL(filepath.Join(".agents", "skills"), legacyBootstrapDir, "---\nname: cli\n---\n\n# My own CLI notes\n") + removed, err = removeLegacyInstalls(base, installTargets, false) + if err != nil { + t.Fatalf("removeLegacyInstalls: %v", err) + } + if len(removed) != 1 || removed[0] != ours { + t.Fatalf("expected only our own leftover removed, got %v", removed) + } + if _, err := os.Stat(ours); !os.IsNotExist(err) { + t.Errorf("expected our leftover gone, got err=%v", err) + } + if _, err := os.Stat(theirs); err != nil { + t.Errorf("expected an unrelated same-named skill untouched, got err=%v", err) + } +} + +func TestPrintInstallSummaryReportsRemoved(t *testing.T) { + var b bytes.Buffer + installed := []installedFile{{Skill: "cio", Target: "claude", Dir: "/home/u/.claude/skills/cio", Files: []string{"SKILL.md"}}} + printInstallSummary(&b, false, installed, []string{"/home/u/.agents/skills/cli"}) + + out := b.String() + if !strings.Contains(out, "Removed an earlier install") || !strings.Contains(out, "/home/u/.agents/skills/cli") { + t.Errorf("expected the summary to report the removal, got:\n%s", out) + } +} + +// The cleanup recognizes its own leftovers by a phrase from the body install +// writes, so editing that paragraph must not silently stop it matching. +func TestBootstrapBodyCarriesMarker(t *testing.T) { + if !strings.Contains(bootstrapSkillBody, bootstrapBodyMarker) { + t.Fatalf("the installed body must contain %q, or removeLegacyInstalls stops recognizing installs this CLI wrote", bootstrapBodyMarker) + } +} + +// A target we cannot clean must not strand the others. +func TestRemoveLegacyInstallsContinuesPastAFailure(t *testing.T) { + if len(installTargets) < 2 { + t.Skip("needs at least two targets to show one failure not stranding another") + } + // The failure is forced with directory permissions, which root ignores. + if os.Geteuid() == 0 { + t.Skip("running as root: a sealed directory is still removable") + } + + base := t.TempDir() + body := "---\nname: cli\n---\n\n" + bootstrapBodyMarker + ", `cio`.\n" + + dirs := make([]string, 0, len(installTargets)) + for _, target := range installTargets { + dir := filepath.Join(base, target.subdir, legacyBootstrapDir) + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "SKILL.md"), []byte(body), 0o644); err != nil { + t.Fatal(err) + } + dirs = append(dirs, dir) + } + + // Seal the first target's parent so its removal fails. + sealed := filepath.Dir(dirs[0]) + if err := os.Chmod(sealed, 0o500); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.Chmod(sealed, 0o755) }) + + removed, err := removeLegacyInstalls(base, installTargets, false) + if err == nil { + t.Fatal("expected the blocked target to surface an error") + } + + reported := make(map[string]bool, len(removed)) + for _, r := range removed { + reported[r] = true + } + for _, dir := range dirs[1:] { + if _, statErr := os.Stat(dir); !os.IsNotExist(statErr) { + t.Errorf("expected %s removed despite the earlier failure, got err=%v", dir, statErr) + } + if !reported[dir] { + t.Errorf("expected %s reported as removed, got %v", dir, removed) + } + } +} diff --git a/cmd/skills_test.go b/cmd/skills_test.go index 18f1233..e4567c8 100644 --- a/cmd/skills_test.go +++ b/cmd/skills_test.go @@ -46,7 +46,7 @@ func skillsFixture(accountPlan string) *skills.SkillsResponse { { // Entrypoint-less skill: empty Content, routing index is // synthesized client-side from each sub-file's frontmatter. - Path: "cli", + Path: "cio", Name: "Customer.io CLI Onboarding", Description: "Builder onboarding.", Content: "", @@ -189,12 +189,12 @@ func TestSkillsListIncludesFileDescriptions(t *testing.T) { } `json:"files"` } for i := range result { - if result[i].Path == "cli" { + if result[i].Path == "cio" { cli = &result[i] } } if cli == nil { - t.Fatal("expected cli skill in list") + t.Fatal("expected cio skill in list") } // Files come back sorted, each carrying its frontmatter description. // With no stored plan info, nothing is filtered. @@ -210,7 +210,7 @@ func TestSkillsReadSynthesizesIndex(t *testing.T) { srv := testSkillsServer(t) defer srv.Close() - out, err := runSkillsCommand(t, srv, "read", "cli") + out, err := runSkillsCommand(t, srv, "read", "cio") if err != nil { t.Fatal(err) } @@ -223,7 +223,7 @@ func TestSkillsReadSynthesizesIndex(t *testing.T) { content, _ := result["content"].(string) for _, want := range []string{ "# Customer.io CLI Onboarding", - "cio skills read cli/", + "cio skills read cio/", "- **auth.md** - cio CLI authentication reference.", "- **onboarding.md** - Builder onboarding entry point.", } { diff --git a/skills/cio/SKILL.md b/skills/cio/SKILL.md index 166caa2..43797fe 100644 --- a/skills/cio/SKILL.md +++ b/skills/cio/SKILL.md @@ -30,7 +30,7 @@ Use runtime references instead of copying large playbooks into this skill: - `cio schema` for commands, flags, generic API routes, and payload shapes. - `cio skills` to list currently available Customer.io reference skills. -- `cio skills read cli` for skills unique to the CLI (auth/login, integration, onboarding/new user setup); read it for the full index. +- `cio skills read cio` for skills unique to the CLI (auth/login, integration, onboarding/new user setup); read it for the full index. - `cio skills read ` for the specific service-hosted reference needed. - `cio api ` for API coverage that is not wrapped by a bespoke command. @@ -38,7 +38,7 @@ Common routing: - Builder/startup setup, signup, sandbox, first app integration, transactional proof, billing activation checks, and go-live start with - `cio prime` and `cio skills read cli`. + `cio prime` and `cio skills read cio`. - Existing Journeys resources such as campaigns, segments, templates, customers, newsletters, and subscription topics should use `cio skills read fly-api`.