diff --git a/.surface b/.surface index cacb7f11..735d22db 100644 --- a/.surface +++ b/.surface @@ -12838,6 +12838,7 @@ FLAG basecamp setup agents --no-stats type=bool FLAG basecamp setup agents --profile type=string FLAG basecamp setup agents --project type=string FLAG basecamp setup agents --quiet type=bool +FLAG basecamp setup agents --remove type=bool FLAG basecamp setup agents --stats type=bool FLAG basecamp setup agents --styled type=bool FLAG basecamp setup agents --todolist type=string diff --git a/README.md b/README.md index 29cb2643..a1c1a687 100644 --- a/README.md +++ b/README.md @@ -246,6 +246,15 @@ To pick up a newer plugin version later, refresh the marketplace with **Other agents:** Point your agent at [`skills/basecamp/SKILL.md`](skills/basecamp/SKILL.md) for Basecamp workflow coverage. +To remove the Basecamp-managed shared skill and coding-agent plugins without +removing Basecamp authentication, configuration, or data, run: + +```bash +basecamp setup agents --remove +``` + +The command leaves user-authored skill directories and additional files alone. + **Agent discovery:** Every command supports `--help --agent` for structured JSON output (flags, gotchas, subcommands). Use `basecamp commands --json` for the full catalog. See [install.md](install.md) for step-by-step setup instructions. diff --git a/internal/appctx/context.go b/internal/appctx/context.go index 29e569d0..d0f01ff2 100644 --- a/internal/appctx/context.go +++ b/internal/appctx/context.go @@ -3,6 +3,7 @@ package appctx import ( "context" + "errors" "fmt" "net/http" "os" @@ -258,6 +259,10 @@ func (a *App) OK(data any, opts ...output.ResponseOption) error { func (a *App) Err(err error) error { // Determine if we should include stats var opts []output.ErrorResponseOption + var carrier output.ErrorMetaProvider + if errors.As(err, &carrier) { + opts = append(opts, output.WithErrorMeta(carrier.ErrorMetadata())) + } if a.shouldIncludeStatsInError() { stats := a.Collector.Summary() opts = append(opts, output.WithErrorStats(&stats)) diff --git a/internal/commands/setup_agents_remove.go b/internal/commands/setup_agents_remove.go new file mode 100644 index 00000000..9cff68b9 --- /dev/null +++ b/internal/commands/setup_agents_remove.go @@ -0,0 +1,894 @@ +package commands + +import ( + "bytes" + "context" + "crypto/sha256" + "encoding/json" + "errors" + "fmt" + "os" + "os/exec" + "path/filepath" + "sort" + "strings" + "time" + + "github.com/spf13/cobra" + + "github.com/basecamp/basecamp-cli/internal/appctx" + "github.com/basecamp/basecamp-cli/internal/harness" + "github.com/basecamp/basecamp-cli/internal/output" + "github.com/basecamp/basecamp-cli/skills" +) + +const agentRemoveTimeout = 20 * time.Second + +var legacyManagedSkillHashes = map[string]struct{}{ + // Every unique skills/basecamp/SKILL.md payload shipped from v0.1.0 through + // v0.9.1, plus 17a00ac immediately before this command. Exact hashes let us + // recognize pre-marker wizard installs without claiming user-authored files. + "9ba73c37394e2f3fd41b1fb88dfcb5765c8d28f817a4d8c186cb3bc6eb9b7c0b": {}, + "5b8dbaee9258079695e078c2fffea835d53ac408aa1265fd5116fd4a8657aaed": {}, + "7f388068176a382e1b452452e88ebb9a4712265ca777c81f47350df0845c8839": {}, + "866ffee85417ea2d204efc5b49e4d6bf2c7f74fd3d3e0d4773fe2fbb70640e36": {}, + "ca0db118c4c69211dd9c0169cce36850c8cca1331e83544e0e4638435a157f43": {}, + "2c295c087b0110ca67c3c12ae8a4deda0d2f04390cefeef6719d924526f0e72a": {}, + "c5dfa70f7a7d9ce5ff4d6c780949bb821447c87db345a22e8f533004a2bd0b3f": {}, + "c8465eadd9f1c7cfae8235812d85b424f15f0a65b9fa3878be046deb58b9efcf": {}, + "21dbba9a6419d3bbf215976e591cafb9fdb3f2a4ca7b20a9953efec7f9691e96": {}, + "5bdfcb49c9808011087790c006f8665cc5d8a079961a1dcef760547d8dd9280c": {}, + "a5e60a1c55ec381dab3265625d97461b7c32edd49837a03642abba347852421d": {}, +} + +var runAgentRemoveCommand = func(ctx context.Context, path, dir string, args ...string) ([]byte, error) { + command := exec.CommandContext(ctx, path, args...) //nolint:gosec // path comes from exec.LookPath + command.Dir = dir + command.WaitDelay = time.Second + return command.CombinedOutput() +} + +type setupRemoveError struct { + err *output.Error + removed []string + failures []string +} + +var _ output.ErrorMetaProvider = (*setupRemoveError)(nil) + +func (e *setupRemoveError) Error() string { return e.err.Error() } + +func (e *setupRemoveError) Unwrap() error { return e.err } + +func (e *setupRemoveError) ErrorMetadata() map[string]any { + return map[string]any{ + "removed": e.removed, + "failures": e.failures, + } +} + +type claudePluginScope struct { + Name string + ProjectPath string +} + +type claudePluginInstallation struct { + Key string + Scopes []claudePluginScope + ScopesKnown bool +} + +var codexPluginInstalled = harness.CodexPluginInstalledContext + +// runRemoveAgentSetup removes only coding-agent integrations managed by the +// Basecamp CLI. Authentication, Basecamp configuration, and project data are +// intentionally outside this command's scope. +func runRemoveAgentSetup(cmd *cobra.Command, app *appctx.App) error { + home, err := harness.UserHomeDir() + homeAvailable := err == nil && home != "" + if !homeAvailable { + home = "" + } + + removed := make([]string, 0) + failures := make([]string, 0) + if !homeAvailable { + failures = append(failures, fmt.Sprintf("home-based skill cleanup: %v", err)) + } + + baseline := "" + if homeAvailable { + baseline = filepath.Join(home, ".agents", "skills", "basecamp") + } + claudeConfig, claudeConfigErr := harness.ClaudeConfigDir() + claudeLinksHandled := claudeConfigErr == nil + if claudeConfigErr != nil { + failures = append(failures, "Claude Code configuration: "+claudeConfigErr.Error()) + } else { + claudeRemoved, claudeFailures := removeClaudePlugin(cmd.Context(), claudeConfig) + if claudeRemoved { + removed = append(removed, "Claude Code plugin") + } + failures = append(failures, claudeFailures...) + + claudeSkill := filepath.Join(claudeConfig, "skills", "basecamp") + // A custom Claude config may place its skill at the shared baseline. + // Keep that baseline intact until every other managed link has been + // inspected, then remove it in the final baseline cleanup below. + if baseline == "" || !pathEntriesEquivalent(claudeSkill, baseline) { + claudeRoot := claudeConfig + if os.Getenv("CLAUDE_CONFIG_DIR") == "" { + claudeRoot = home + } + safe, safeErr := safeSkillTraversal(claudeRoot, claudeSkill) + if safeErr != nil { + claudeLinksHandled = false + failures = append(failures, "Claude Code skill: "+safeErr.Error()) + } else if !safe { + claudeLinksHandled = false + failures = append(failures, "Claude Code skill: unsafe symlink traversal skipped") + } else if didRemove, removeErr := removeClaudeSkill(claudeSkill, baseline); removeErr != nil { + claudeLinksHandled = false + failures = append(failures, "Claude Code skill: "+removeErr.Error()) + } else if didRemove { + removed = append(removed, "Claude Code skill") + } + } + } + + // The default path may be a managed link left by an older install. Clean it + // independently so a malformed custom CLAUDE_CONFIG_DIR cannot leave that + // link broken when the shared baseline is removed below. + legacyClaudeSkill := "" + if homeAvailable { + legacyClaudeSkill = filepath.Join(home, ".claude", "skills", "basecamp") + } + configuredClaudeSkill := "" + if claudeConfigErr == nil { + configuredClaudeSkill = filepath.Join(claudeConfig, "skills", "basecamp") + } + if legacyClaudeSkill != "" && (configuredClaudeSkill == "" || !pathEntriesEquivalent(legacyClaudeSkill, configuredClaudeSkill)) { + safe, safeErr := safeSkillTraversal(home, legacyClaudeSkill) + if safeErr != nil { + claudeLinksHandled = false + failures = append(failures, "legacy Claude Code skill: "+safeErr.Error()) + } else if !safe { + claudeLinksHandled = false + failures = append(failures, "legacy Claude Code skill: unsafe symlink traversal skipped") + } else if didRemove, removeErr := removeClaudeSkill(legacyClaudeSkill, baseline); removeErr != nil { + claudeLinksHandled = false + failures = append(failures, "legacy Claude Code skill: "+removeErr.Error()) + } else if didRemove { + removed = append(removed, "legacy Claude Code skill") + } + } + projectClaudeSkill := filepath.Join(".claude", "skills", "basecamp") + if !pathEntriesEquivalent(projectClaudeSkill, configuredClaudeSkill) && !pathEntriesEquivalent(projectClaudeSkill, legacyClaudeSkill) { + projectRoot, rootErr := os.Getwd() + symlinked := false + if rootErr == nil { + symlinked, rootErr = hasSymlinkComponent(projectRoot, projectClaudeSkill) + } + if rootErr != nil { + failures = append(failures, "project Claude Code skill: "+rootErr.Error()) + } else if symlinked { + failures = append(failures, "project Claude Code skill: unsafe symlink traversal skipped") + } else { + if didRemove, removeErr := removeOwnedOrLegacySkill(projectClaudeSkill); removeErr != nil { + failures = append(failures, "project Claude Code skill: "+removeErr.Error()) + } else if didRemove { + removed = append(removed, "project Claude Code skill") + } + } + } + + codexRemoved, codexFailure := removeCodexPlugin(cmd.Context()) + if codexRemoved { + removed = append(removed, "Codex plugin") + } + if codexFailure != "" { + failures = append(failures, codexFailure) + } + + resolvedCodexHome, codexHomeErr := harness.CodexHome() + if codexHomeErr != nil { + failures = append(failures, "Codex configuration: "+codexHomeErr.Error()) + if homeAvailable { + resolvedCodexHome = filepath.Join(home, ".codex") + } + } + codexSkill := "" + if resolvedCodexHome != "" { + codexSkill = filepath.Join(resolvedCodexHome, "skills", "basecamp") + } + if codexSkill != "" && !pathEntriesEquivalent(codexSkill, baseline) { + codexRoot := resolvedCodexHome + if os.Getenv("CODEX_HOME") == "" { + codexRoot = home + } + safe, safeErr := safeSkillTraversal(codexRoot, codexSkill) + if safeErr != nil { + failures = append(failures, "Codex skill: "+safeErr.Error()) + } else if !safe { + failures = append(failures, "Codex skill: unsafe symlink traversal skipped") + } else if didRemove, removeErr := removeOwnedOrLegacyCodexSkill(codexSkill); removeErr != nil { + failures = append(failures, "Codex skill: "+removeErr.Error()) + } else if didRemove { + removed = append(removed, "Codex skill") + } + } + legacyCodexSkill := "" + if homeAvailable { + legacyCodexSkill = filepath.Join(home, ".codex", "skills", "basecamp") + } + if legacyCodexSkill != "" && !pathEntriesEquivalent(legacyCodexSkill, codexSkill) && + !pathEntriesEquivalent(legacyCodexSkill, baseline) { + safe, safeErr := safeSkillTraversal(home, legacyCodexSkill) + if safeErr != nil { + failures = append(failures, "legacy Codex skill: "+safeErr.Error()) + } else if !safe { + failures = append(failures, "legacy Codex skill: unsafe symlink traversal skipped") + } else if didRemove, removeErr := removeOwnedOrLegacyCodexSkill(legacyCodexSkill); removeErr != nil { + failures = append(failures, "legacy Codex skill: "+removeErr.Error()) + } else if didRemove { + removed = append(removed, "legacy Codex skill") + } + } + + removeOpenCodeSkills(home, &removed, &failures) + + // A managed Claude link can only be recognized while its managed baseline + // remains intact. Retain the baseline after any link-slot inspection or + // removal failure so a retry can still prove and remove the link. + if baseline != "" && claudeLinksHandled { + safe, safeErr := safeSkillTraversal(home, baseline) + if safeErr != nil { + failures = append(failures, "agent skill: "+safeErr.Error()) + } else if !safe { + failures = append(failures, "agent skill: unsafe symlink traversal skipped") + } else if didRemove, removeErr := removeOwnedOrLegacySkill(baseline); removeErr != nil { + failures = append(failures, "agent skill: "+removeErr.Error()) + } else if didRemove { + removed = append(removed, "agent skill") + } + } + + result := map[string]any{ + "removed": removed, + "failures": failures, + } + if len(failures) > 0 { + return &setupRemoveError{ + err: &output.Error{ + Code: "setup_remove_failed", + Message: "coding-agent integration removal incomplete: " + strings.Join(failures, "; "), + Hint: "Resolve the reported item, then run: basecamp setup agents --remove", + }, + removed: removed, + failures: failures, + } + } + + return app.OK(result, output.WithSummary("Coding-agent integrations removed")) +} + +func removeOpenCodeSkills(home string, removed, failures *[]string) { + locations := append(append([]skillLocation{}, skillLocations...), legacySkillLocations...) + seen := make(map[string]struct{}) + projectRoot, projectRootErr := os.Getwd() + for _, location := range locations { + if !strings.HasPrefix(location.Name, "OpenCode") { + continue + } + path := location.Path + root := projectRoot + if strings.HasPrefix(path, "~/") || strings.HasPrefix(path, "~\\") { + if home == "" { + continue + } + path = filepath.Join(home, path[2:]) + root = home + } else if projectRootErr != nil { + *failures = append(*failures, location.Name+" skill: getting working directory: "+projectRootErr.Error()) + continue + } + dir := filepath.Clean(filepath.Dir(path)) + if absolute, err := filepath.Abs(dir); err == nil { + dir = absolute + } + // Deduplicate configured path entries, not their resolved destinations. + // An unmanaged symlink at one location may point at a managed directory + // that is also listed directly and still needs to be removed. + if _, duplicate := seen[dir]; duplicate { + continue + } + seen[dir] = struct{}{} + symlinked, inspectErr := hasSymlinkComponent(root, dir) + if inspectErr != nil { + *failures = append(*failures, location.Name+" skill: "+inspectErr.Error()) + continue + } + if symlinked { + *failures = append(*failures, location.Name+" skill: unsafe symlink traversal skipped") + continue + } + didRemove, err := removeOwnedOrLegacySkill(dir) + if err != nil { + *failures = append(*failures, location.Name+" skill: "+err.Error()) + } else if didRemove { + *removed = append(*removed, location.Name+" skill") + } + } +} + +// hasSymlinkComponent refuses traversal through user-controlled aliases below +// a trusted installation root. Cleanup may inspect the root itself, but it must +// never follow a symlink in a predefined path into an unrelated directory. +func hasSymlinkComponent(root, target string) (bool, error) { + root, rootErr := filepath.Abs(filepath.Clean(root)) + if rootErr != nil { + return false, fmt.Errorf("resolving cleanup root: %w", rootErr) + } + target, targetErr := filepath.Abs(filepath.Clean(target)) + if targetErr != nil { + return false, fmt.Errorf("resolving cleanup target: %w", targetErr) + } + relative, err := filepath.Rel(root, target) + if err != nil || relative == ".." || strings.HasPrefix(relative, ".."+string(filepath.Separator)) { + return false, fmt.Errorf("cleanup target %s is outside %s", target, root) + } + + current := root + for _, component := range strings.Split(relative, string(filepath.Separator)) { + if component == "" || component == "." { + continue + } + current = filepath.Join(current, component) + info, statErr := os.Lstat(current) + if os.IsNotExist(statErr) { + return false, nil + } + if statErr != nil { + return false, fmt.Errorf("inspecting %s: %w", current, statErr) + } + if info.Mode()&os.ModeSymlink != 0 { + return true, nil + } + } + return false, nil +} + +func safeSkillTraversal(root, target string) (bool, error) { + if root == "" { + return false, errors.New("cleanup root is unavailable") + } + symlinked, err := hasSymlinkComponent(root, filepath.Dir(target)) + return !symlinked, err +} + +func removeClaudePlugin(parent context.Context, configDir string) (bool, []string) { + installations, err := readClaudePluginInstallations(configDir) + if err != nil { + return false, []string{"Claude Code plugin: " + err.Error()} + } + if len(installations) == 0 { + return false, nil + } + claudePath := harness.FindClaudeBinary() + if claudePath == "" { + return false, []string{"Claude Code plugin: claude binary not found"} + } + + removed := false + var failures []string + for _, installation := range installations { + valid, needsFallback := validClaudeScopes(installation) + var scopedFailures []string + for _, scope := range valid { + if scope.Name != "user" && !validProjectPath(scope.ProjectPath) { + scopedFailures = append(scopedFailures, fmt.Sprintf("Claude Code plugin %s (%s): recorded project path is missing or invalid", installation.Key, scope.Name)) + continue + } + workingDir := scope.ProjectPath + if scope.Name == "user" { + workingDir = "" + } + commandOutput, commandErr := runRemoveStep(parent, claudePath, workingDir, "plugin", "uninstall", installation.Key, "--scope", scope.Name) + if commandErr != nil { + if !claudePluginAbsent(commandOutput, installation.Key) { + scopedFailures = append(scopedFailures, fmt.Sprintf("Claude Code plugin %s (%s): %s", installation.Key, scope.Name, commandOutputFailure(commandOutput, commandErr))) + } + } else { + removed = true + } + } + + if needsFallback { + fallbackRemoved, fallbackErr := uninstallClaudeUnscoped(parent, claudePath, installation.Key) + if fallbackErr == nil { + removed = removed || fallbackRemoved + continue + } + if fallbackRemoved { + removed = true + } + failures = append(failures, fmt.Sprintf("Claude Code plugin %s: %s", installation.Key, fallbackErr)) + failures = append(failures, scopedFailures...) + continue + } + failures = append(failures, scopedFailures...) + } + return removed, failures +} + +func validClaudeScopes(installation claudePluginInstallation) ([]claudePluginScope, bool) { + var valid []claudePluginScope + needsFallback := !installation.ScopesKnown || len(installation.Scopes) == 0 + for _, scope := range installation.Scopes { + if validPluginScope(scope.Name) { + valid = append(valid, scope) + } else { + needsFallback = true + } + } + return valid, needsFallback +} + +// uninstallClaudeUnscoped handles registry formats that cannot name every +// scope. Claude removes one matching installation per invocation, so retry up +// to a safety cap and stop only on an explicit absent-plugin response. +func uninstallClaudeUnscoped(parent context.Context, claudePath, key string) (bool, error) { + ctx, cancel := context.WithTimeout(parent, agentRemoveTimeout) + defer cancel() + + removed := false + for range 10 { + output, err := runRemoveStep(ctx, claudePath, "", "plugin", "uninstall", key) + if err != nil { + if claudePluginAbsent(output, key) { + return removed, nil + } + return removed, errors.New(commandOutputFailure(output, err)) + } + removed = true + } + return removed, errors.New("uninstall safety limit reached") +} + +func claudePluginAbsent(output []byte, key string) bool { + message := strings.ToLower(string(output)) + absent := strings.Contains(message, "not installed") || strings.Contains(message, "no installed plugin") || strings.Contains(message, "not found") + return absent && strings.Contains(message, strings.ToLower(key)) +} + +func removeCodexPlugin(parent context.Context) (bool, string) { + codexPath := harness.FindCodexBinary() + if codexPath == "" { + if codexHomeExists() { + return false, "Codex plugin: codex binary not found" + } + return false, "" + } + installed, queryErr := codexPluginInstalled(parent) + if queryErr != nil { + return false, "Codex plugin: checking installed state: " + queryErr.Error() + } + if !installed { + return false, "" + } + + commandOutput, err := runRemoveStep(parent, codexPath, "", "plugin", "remove", harness.CodexExpectedPluginKey, "--json") + if err == nil { + return true, "" + } + if codexPluginAbsent(commandOutput) { + return false, "" + } + return false, "Codex plugin: " + commandOutputFailure(commandOutput, err) +} + +func codexPluginAbsent(output []byte) bool { + message := strings.ToLower(string(output)) + return strings.Contains(message, strings.ToLower(harness.CodexExpectedPluginKey)) && + (strings.Contains(message, "not installed") || strings.Contains(message, "not found")) +} + +func runRemoveStep(parent context.Context, path, dir string, args ...string) ([]byte, error) { + ctx, cancel := context.WithTimeout(parent, agentRemoveTimeout) + defer cancel() + output, err := runAgentRemoveCommand(ctx, path, dir, args...) + if ctx.Err() != nil { + return output, ctx.Err() + } + return output, err +} + +func codexHomeExists() bool { + resolved, err := harness.CodexHome() + if err != nil { + return false + } + info, err := os.Stat(resolved) + return err == nil && info.IsDir() +} + +func pathsEquivalent(left, right string) bool { + if left == "" || right == "" { + return false + } + left = filepath.Clean(left) + right = filepath.Clean(right) + if left == right { + return true + } + leftInfo, leftErr := os.Stat(left) + rightInfo, rightErr := os.Stat(right) + return leftErr == nil && rightErr == nil && os.SameFile(leftInfo, rightInfo) +} + +// pathEntriesEquivalent compares installation slots without following the +// final path component. Parent directory aliases should collapse to one slot, +// but two distinct symlinks that happen to share a target must both be handled. +func pathEntriesEquivalent(left, right string) bool { + if left == "" || right == "" { + return false + } + left, leftErr := filepath.Abs(filepath.Clean(left)) + right, rightErr := filepath.Abs(filepath.Clean(right)) + if leftErr != nil || rightErr != nil { + return filepath.Clean(left) == filepath.Clean(right) + } + if left == right { + return true + } + if filepath.Base(left) != filepath.Base(right) { + return false + } + leftParent, leftErr := filepath.EvalSymlinks(filepath.Dir(left)) + rightParent, rightErr := filepath.EvalSymlinks(filepath.Dir(right)) + return leftErr == nil && rightErr == nil && pathsEquivalent(leftParent, rightParent) +} + +func validProjectPath(path string) bool { + if path == "" || !filepath.IsAbs(path) { + return false + } + info, err := os.Stat(path) + return err == nil && info.IsDir() +} + +func commandFailure(err error) string { + if err == nil { + return "unknown failure" + } + return err.Error() +} + +func commandOutputFailure(commandOutput []byte, err error) string { + message := strings.TrimSpace(string(commandOutput)) + if len(message) > 500 { + message = message[:500] + } + if message != "" { + return message + } + return commandFailure(err) +} + +func readClaudePluginInstallations(configDir string) ([]claudePluginInstallation, error) { + path := filepath.Join(configDir, "plugins", "installed_plugins.json") + data, err := os.ReadFile(path) //nolint:gosec // canonical user configuration path + if os.IsNotExist(err) { + return nil, nil + } + if err != nil { + return nil, fmt.Errorf("reading %s: %w", path, err) + } + installations, ok := parseClaudePluginInstallations(data) + if !ok { + return nil, fmt.Errorf("parsing %s", path) + } + return installations, nil +} + +func parseClaudePluginInstallations(data []byte) ([]claudePluginInstallation, bool) { + var envelope map[string]any + if json.Unmarshal(data, &envelope) == nil && envelope != nil { + if rawPlugins, exists := envelope["plugins"]; exists { + pluginMap, ok := rawPlugins.(map[string]any) + if !ok { + return nil, false + } + return installationsFromMap(pluginMap, true), true + } + return installationsFromMap(envelope, false), true + } + + var list []map[string]any + if json.Unmarshal(data, &list) != nil || list == nil { + return nil, false + } + byKey := make(map[string]*claudePluginInstallation) + for _, entry := range list { + key := pluginKeyFromEntry(entry) + if !basecampClaudePluginKey(key) { + continue + } + installation := byKey[key] + if installation == nil { + installation = &claudePluginInstallation{Key: key, ScopesKnown: true} + byKey[key] = installation + } + if scope, ok := entry["scope"].(string); ok && scope != "" { + installation.Scopes = appendUniqueClaudeScope(installation.Scopes, claudeScopeFromEntry(scope, entry)) + } else { + installation.ScopesKnown = false + } + } + return sortedInstallations(byKey), true +} + +func installationsFromMap(pluginMap map[string]any, v2 bool) []claudePluginInstallation { + byKey := make(map[string]*claudePluginInstallation) + for key, raw := range pluginMap { + if !basecampClaudePluginKey(key) { + continue + } + installation := &claudePluginInstallation{Key: key} + if entries, ok := raw.([]any); ok { + installation.ScopesKnown = true + for _, rawEntry := range entries { + entry, entryOK := rawEntry.(map[string]any) + if !entryOK { + installation.ScopesKnown = false + continue + } + if scope, scopeOK := entry["scope"].(string); scopeOK && scope != "" { + installation.Scopes = appendUniqueClaudeScope(installation.Scopes, claudeScopeFromEntry(scope, entry)) + } else { + installation.ScopesKnown = false + } + } + } else if entry, ok := raw.(map[string]any); ok { + if scope, ok := entry["scope"].(string); ok && scope != "" { + installation.ScopesKnown = true + installation.Scopes = appendUniqueClaudeScope(installation.Scopes, claudeScopeFromEntry(scope, entry)) + } + } + if v2 && raw == nil { + installation.ScopesKnown = false + } + byKey[key] = installation + } + return sortedInstallations(byKey) +} + +func sortedInstallations(byKey map[string]*claudePluginInstallation) []claudePluginInstallation { + keys := make([]string, 0, len(byKey)) + for key := range byKey { + keys = append(keys, key) + } + sort.Strings(keys) + out := make([]claudePluginInstallation, 0, len(keys)) + for _, key := range keys { + out = append(out, *byKey[key]) + } + return out +} + +func appendUniqueClaudeScope(values []claudePluginScope, value claudePluginScope) []claudePluginScope { + for _, existing := range values { + if existing == value { + return values + } + } + return append(values, value) +} + +func claudeScopeFromEntry(scope string, entry map[string]any) claudePluginScope { + projectPath, _ := entry["projectPath"].(string) + if projectPath == "" { + projectPath, _ = entry["project_path"].(string) + } + return claudePluginScope{Name: scope, ProjectPath: projectPath} +} + +func pluginKeyFromEntry(entry map[string]any) string { + for _, field := range []string{"package", "id", "name"} { + if value, ok := entry[field].(string); ok && basecampClaudePluginKey(value) { + return value + } + } + return "" +} + +func basecampClaudePluginKey(key string) bool { + return key == harness.ClaudePluginName || key == harness.ClaudeExpectedPluginKey || key == "basecamp@basecamp" +} + +// removeClaudeSkill removes only the canonical symlink written by Basecamp or +// a marked/legacy-managed copy created by the symlink fallback. +func removeClaudeSkill(path, baseline string) (bool, error) { + info, err := os.Lstat(path) + if os.IsNotExist(err) { + return false, nil + } + if err != nil { + return false, fmt.Errorf("inspecting %s: %w", path, err) + } + if info.Mode()&os.ModeSymlink != 0 { + target, readErr := os.Readlink(path) + if readErr != nil { + return false, fmt.Errorf("reading %s: %w", path, readErr) + } + managedBaseline := baseline != "" && ownedOrLegacySkillDir(baseline) + managedTarget := managedBaseline && pathsEquivalent(path, baseline) + if !managedTarget && managedBaseline { + managedTarget = brokenLinkTargetsPath(path, target, baseline) + } + if !managedTarget { + return false, nil + } + if removeErr := os.Remove(path); removeErr != nil && !os.IsNotExist(removeErr) { + return false, fmt.Errorf("removing %s: %w", path, removeErr) + } + return true, nil + } + return removeOwnedOrLegacySkill(path) +} + +func ownedOrLegacySkillDir(dir string) bool { + if ownedSkillDir(dir) { + return true + } + info, err := os.Lstat(dir) + if err != nil || info.Mode()&os.ModeSymlink != 0 || !info.IsDir() { + return false + } + entries, err := os.ReadDir(dir) + if err != nil || len(entries) != 1 || entries[0].Name() != skillFilename || !entries[0].Type().IsRegular() { + return false + } + installed, err := os.ReadFile(filepath.Join(dir, skillFilename)) //nolint:gosec // fixed skill path + return err == nil && recognizedManagedSkillPayload(installed) +} + +func brokenLinkTargetsPath(linkPath, target, expected string) bool { + linkParent, err := filepath.EvalSymlinks(filepath.Dir(linkPath)) + if err != nil { + linkParent = filepath.Dir(linkPath) + } + if !filepath.IsAbs(target) { + target = filepath.Join(linkParent, target) + } + return resolveExistingPathPrefix(target) == resolveExistingPathPrefix(expected) +} + +func resolveExistingPathPrefix(path string) string { + path = filepath.Clean(path) + missing := make([]string, 0) + for { + if resolved, err := filepath.EvalSymlinks(path); err == nil { + for i := len(missing) - 1; i >= 0; i-- { + resolved = filepath.Join(resolved, missing[i]) + } + return filepath.Clean(resolved) + } + parent := filepath.Dir(path) + if parent == path { + return filepath.Clean(path) + } + missing = append(missing, filepath.Base(path)) + path = parent + } +} + +// removeOwnedSkillFiles removes only files written by Basecamp from a skill +// directory carrying a current ownership marker or the legacy version marker. +// Additional files keep the now-unmanaged directory in place. +func removeOwnedSkillFiles(dir string) (bool, error) { + info, err := os.Lstat(dir) + if os.IsNotExist(err) { + return false, nil + } + if err != nil { + return false, fmt.Errorf("inspecting %s: %w", dir, err) + } + if info.Mode()&os.ModeSymlink != 0 || !info.IsDir() || !ownedSkillDir(dir) { + return false, nil + } + + paths := []string{ + filepath.Join(dir, skillFilename), + filepath.Join(dir, installedVersionFile), + filepath.Join(dir, ownershipMarkerFile), + } + for _, path := range paths { + entry, statErr := os.Lstat(path) + if os.IsNotExist(statErr) { + continue + } + if statErr != nil { + return false, fmt.Errorf("inspecting %s: %w", path, statErr) + } + if !entry.Mode().IsRegular() { + return false, fmt.Errorf("%s is not a regular file", path) + } + } + for _, path := range paths { + if removeErr := os.Remove(path); removeErr != nil && !os.IsNotExist(removeErr) { + return false, fmt.Errorf("removing %s: %w", path, removeErr) + } + } + _ = os.Remove(dir) + return true, nil +} + +// removeOwnedOrLegacyCodexSkill also recognizes the old wizard's direct Codex +// install, which predates ownership markers. Exact embedded content plus a flat, +// allowlisted directory is the required provenance; a merely similar skill is +// user state and remains untouched. +func removeOwnedOrLegacyCodexSkill(dir string) (bool, error) { + return removeOwnedOrLegacySkill(dir) +} + +func removeOwnedOrLegacySkill(dir string) (bool, error) { + if ownedSkillDir(dir) { + return removeOwnedSkillFiles(dir) + } + info, err := os.Lstat(dir) + if os.IsNotExist(err) { + return false, nil + } + if err != nil { + return false, fmt.Errorf("inspecting %s: %w", dir, err) + } + if info.Mode()&os.ModeSymlink != 0 || !info.IsDir() { + return false, nil + } + entries, err := os.ReadDir(dir) + if err != nil { + return false, fmt.Errorf("inspecting %s: %w", dir, err) + } + skillEntry := -1 + for i, entry := range entries { + if entry.Name() == skillFilename && entry.Type().IsRegular() { + skillEntry = i + break + } + } + if skillEntry == -1 { + return false, nil + } + installed, err := os.ReadFile(filepath.Join(dir, skillFilename)) //nolint:gosec // fixed legacy skill path + if err != nil { + return false, fmt.Errorf("reading legacy managed skill: %w", err) + } + if !recognizedManagedSkillPayload(installed) { + return false, nil + } + if err := os.Remove(filepath.Join(dir, skillFilename)); err != nil && !os.IsNotExist(err) { + return false, fmt.Errorf("removing legacy managed skill: %w", err) + } + _ = os.Remove(dir) + return true, nil +} + +func recognizedManagedSkillPayload(data []byte) bool { + embedded, err := skills.FS.ReadFile("basecamp/SKILL.md") + if err == nil && bytes.Equal(data, embedded) { + return true + } + sum := sha256.Sum256(data) + _, recognized := legacyManagedSkillHashes[fmt.Sprintf("%x", sum)] + return recognized +} + +func ownedSkillDir(dir string) bool { + return regularFile(filepath.Join(dir, ownershipMarkerFile)) || regularFile(filepath.Join(dir, installedVersionFile)) +} + +func regularFile(path string) bool { + info, err := os.Lstat(path) + return err == nil && info.Mode().IsRegular() +} diff --git a/internal/commands/setup_agents_remove_test.go b/internal/commands/setup_agents_remove_test.go new file mode 100644 index 00000000..25799efe --- /dev/null +++ b/internal/commands/setup_agents_remove_test.go @@ -0,0 +1,1055 @@ +package commands + +import ( + "bytes" + "context" + "crypto/sha256" + "encoding/json" + "errors" + "fmt" + "os" + "os/exec" + "path/filepath" + "slices" + "strconv" + "strings" + "testing" + "time" + + "github.com/spf13/cobra" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/basecamp/basecamp-cli/internal/appctx" + "github.com/basecamp/basecamp-cli/internal/output" + "github.com/basecamp/basecamp-cli/skills" +) + +func runSetupAgentsRemove(t *testing.T) ([]byte, error) { + t.Helper() + app, out := setupQuickstartTestApp(t, "", "") + app.Flags.JSON = true + t.Cleanup(app.Close) + + cmd := NewSetupCmd() + cmd.SetArgs([]string{"agents", "--remove"}) + cmd.SetContext(appctx.WithApp(context.Background(), app)) + cmd.SetOut(&bytes.Buffer{}) + cmd.SetErr(&bytes.Buffer{}) + err := cmd.Execute() + return out.Bytes(), err +} + +func runSetupAgentsRemoveWithErrorEnvelope(t *testing.T) ([]byte, error) { + t.Helper() + app, out := setupQuickstartTestApp(t, "", "") + app.Flags.JSON = true + t.Cleanup(app.Close) + + cmd := NewSetupCmd() + cmd.SetArgs([]string{"agents", "--remove"}) + cmd.SetContext(appctx.WithApp(context.Background(), app)) + cmd.SetOut(&bytes.Buffer{}) + cmd.SetErr(&bytes.Buffer{}) + err := cmd.Execute() + require.Error(t, err) + require.NoError(t, app.Err(err)) + return out.Bytes(), err +} + +func stubAgentRemoveCommand(t *testing.T, fn func(context.Context, string, string, ...string) ([]byte, error)) { + t.Helper() + original := runAgentRemoveCommand + runAgentRemoveCommand = fn + t.Cleanup(func() { runAgentRemoveCommand = original }) +} + +func stubCodexInstalled(t *testing.T, installed bool, err error) { + t.Helper() + original := codexPluginInstalled + codexPluginInstalled = func(context.Context) (bool, error) { return installed, err } + t.Cleanup(func() { codexPluginInstalled = original }) +} + +func installExecutableStub(t *testing.T, name string) string { + t.Helper() + binDir := t.TempDir() + path := filepath.Join(binDir, name) + require.NoError(t, os.WriteFile(path, []byte("#!/bin/sh\nexit 0\n"), 0o755)) + t.Setenv("PATH", binDir) + return path +} + +func writeClaudeRegistry(t *testing.T, configDir, content string) { + t.Helper() + dir := filepath.Join(configDir, "plugins") + require.NoError(t, os.MkdirAll(dir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "installed_plugins.json"), []byte(content), 0o600)) +} + +func TestSetupAgentsRemoveDeletesManagedSkillsAndPreservesUserFiles(t *testing.T) { + home := emptyHome(t) + _, err := installSkillFiles() + require.NoError(t, err) + _, _, err = linkSkillToClaude() + require.NoError(t, err) + + baseline := filepath.Join(home, ".agents", "skills", "basecamp") + require.NoError(t, os.WriteFile(filepath.Join(baseline, "notes.txt"), []byte("keep me"), 0o600)) + + response, err := runSetupAgentsRemove(t) + require.NoError(t, err) + var envelope struct { + Summary string `json:"summary"` + Data struct { + Removed []string `json:"removed"` + Failures []string `json:"failures"` + } `json:"data"` + } + require.NoError(t, json.Unmarshal(response, &envelope), string(response)) + assert.Equal(t, "Coding-agent integrations removed", envelope.Summary) + assert.ElementsMatch(t, []string{"Claude Code skill", "agent skill"}, envelope.Data.Removed) + assert.Empty(t, envelope.Data.Failures) + + for _, name := range []string{skillFilename, installedVersionFile, ownershipMarkerFile} { + _, statErr := os.Lstat(filepath.Join(baseline, name)) + assert.True(t, os.IsNotExist(statErr), name) + } + data, readErr := os.ReadFile(filepath.Join(baseline, "notes.txt")) + require.NoError(t, readErr) + assert.Equal(t, "keep me", string(data)) + _, statErr := os.Lstat(filepath.Join(home, ".claude", "skills", "basecamp")) + assert.True(t, os.IsNotExist(statErr)) +} + +func TestSetupAgentsRemoveDeletesManagedOpenCodeSkills(t *testing.T) { + home := emptyHome(t) + project := t.TempDir() + locations := []string{ + filepath.Join(home, ".config", "opencode", "skills", "basecamp"), + filepath.Join(home, ".config", "opencode", "skill", "basecamp"), + filepath.Join(project, ".opencode", "skills", "basecamp"), + } + t.Chdir(project) + for _, dir := range locations { + require.NoError(t, os.MkdirAll(dir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, skillFilename), []byte("managed"), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(dir, ownershipMarkerFile), []byte("managed"), 0o600)) + } + + _, err := runSetupAgentsRemove(t) + require.NoError(t, err) + for _, dir := range locations { + _, statErr := os.Lstat(filepath.Join(dir, skillFilename)) + assert.True(t, os.IsNotExist(statErr), dir) + } +} + +func TestSetupAgentsRemoveDoesNotDeduplicateOpenCodeSymlinkDestination(t *testing.T) { + home := emptyHome(t) + project := t.TempDir() + t.Chdir(project) + + projectSkill := filepath.Join(project, ".opencode", "skills", "basecamp") + require.NoError(t, os.MkdirAll(projectSkill, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(projectSkill, skillFilename), []byte("managed"), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(projectSkill, ownershipMarkerFile), []byte("managed"), 0o600)) + + globalSkill := filepath.Join(home, ".config", "opencode", "skills", "basecamp") + require.NoError(t, os.MkdirAll(filepath.Dir(globalSkill), 0o755)) + require.NoError(t, os.Symlink(projectSkill, globalSkill)) + + _, err := runSetupAgentsRemove(t) + require.ErrorContains(t, err, "OpenCode (Global) skill: unsafe symlink traversal skipped") + _, statErr := os.Lstat(filepath.Join(projectSkill, skillFilename)) + assert.True(t, os.IsNotExist(statErr), "the directly listed managed project skill must be removed") + linkInfo, statErr := os.Lstat(globalSkill) + require.NoError(t, statErr) + assert.NotZero(t, linkInfo.Mode()&os.ModeSymlink, "the unmanaged symlink itself must be preserved") +} + +func TestSetupAgentsRemovePreservesOpenCodeSkillBehindSymlinkedParent(t *testing.T) { + home := emptyHome(t) + project := t.TempDir() + t.Chdir(project) + + externalConfig := t.TempDir() + externalSkill := filepath.Join(externalConfig, "opencode", "skills", "basecamp") + require.NoError(t, os.MkdirAll(externalSkill, 0o755)) + embedded, err := skills.FS.ReadFile("basecamp/SKILL.md") + require.NoError(t, err) + require.NoError(t, os.WriteFile(filepath.Join(externalSkill, skillFilename), embedded, 0o600)) + require.NoError(t, os.Symlink(externalConfig, filepath.Join(home, ".config"))) + + _, err = runSetupAgentsRemove(t) + require.ErrorContains(t, err, "OpenCode (Global) skill: unsafe symlink traversal skipped") + data, readErr := os.ReadFile(filepath.Join(externalSkill, skillFilename)) + require.NoError(t, readErr) + assert.Equal(t, embedded, data, "cleanup must not follow a symlinked OpenCode parent") +} + +func TestSetupAgentsRemoveDeletesManagedProjectClaudeSkill(t *testing.T) { + emptyHome(t) + project := t.TempDir() + t.Chdir(project) + dir := filepath.Join(project, ".claude", "skills", "basecamp") + require.NoError(t, os.MkdirAll(dir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, skillFilename), []byte("managed"), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(dir, ownershipMarkerFile), []byte("managed"), 0o600)) + + _, err := runSetupAgentsRemove(t) + require.NoError(t, err) + _, statErr := os.Lstat(filepath.Join(dir, skillFilename)) + assert.True(t, os.IsNotExist(statErr)) +} + +func TestSetupAgentsRemovePreservesProjectClaudeSkillBehindSymlinkedParent(t *testing.T) { + emptyHome(t) + project := t.TempDir() + t.Chdir(project) + + externalClaude := t.TempDir() + externalSkill := filepath.Join(externalClaude, "skills", "basecamp") + require.NoError(t, os.MkdirAll(externalSkill, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(externalSkill, skillFilename), []byte("managed"), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(externalSkill, ownershipMarkerFile), []byte("managed"), 0o600)) + require.NoError(t, os.Symlink(externalClaude, filepath.Join(project, ".claude"))) + + _, err := runSetupAgentsRemove(t) + require.ErrorContains(t, err, "project Claude Code skill: unsafe symlink traversal skipped") + _, statErr := os.Lstat(filepath.Join(externalSkill, skillFilename)) + assert.NoError(t, statErr, "cleanup must not follow a symlinked project Claude parent") +} + +func TestSetupAgentsRemoveDeletesAuthenticMarkerlessBaseline(t *testing.T) { + home := emptyHome(t) + dir := filepath.Join(home, ".agents", "skills", "basecamp") + require.NoError(t, os.MkdirAll(dir, 0o755)) + embedded, err := skills.FS.ReadFile("basecamp/SKILL.md") + require.NoError(t, err) + require.NoError(t, os.WriteFile(filepath.Join(dir, skillFilename), embedded, 0o600)) + + _, err = runSetupAgentsRemove(t) + require.NoError(t, err) + _, statErr := os.Lstat(dir) + assert.True(t, os.IsNotExist(statErr)) +} + +func TestSetupAgentsRemoveReportsSkippedSymlinkedBaseline(t *testing.T) { + home := emptyHome(t) + externalAgents := t.TempDir() + baseline := filepath.Join(externalAgents, "skills", "basecamp") + require.NoError(t, os.MkdirAll(baseline, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(baseline, skillFilename), []byte("managed"), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(baseline, ownershipMarkerFile), []byte("managed"), 0o600)) + require.NoError(t, os.Symlink(externalAgents, filepath.Join(home, ".agents"))) + + _, err := runSetupAgentsRemove(t) + require.ErrorContains(t, err, "agent skill: unsafe symlink traversal skipped") + _, statErr := os.Stat(filepath.Join(baseline, skillFilename)) + assert.NoError(t, statErr, "cleanup must not follow a symlinked baseline parent") +} + +func TestSetupAgentsRemovePreservesUserFilesBesideMarkerlessManagedSkill(t *testing.T) { + home := emptyHome(t) + dir := filepath.Join(home, ".agents", "skills", "basecamp") + require.NoError(t, os.MkdirAll(dir, 0o755)) + embedded, err := skills.FS.ReadFile("basecamp/SKILL.md") + require.NoError(t, err) + require.NoError(t, os.WriteFile(filepath.Join(dir, skillFilename), embedded, 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "notes.txt"), []byte("keep me"), 0o600)) + + _, err = runSetupAgentsRemove(t) + require.NoError(t, err) + _, statErr := os.Lstat(filepath.Join(dir, skillFilename)) + assert.True(t, os.IsNotExist(statErr)) + data, readErr := os.ReadFile(filepath.Join(dir, "notes.txt")) + require.NoError(t, readErr) + assert.Equal(t, "keep me", string(data)) +} + +func TestSetupAgentsRemoveUsesAbsoluteClaudeConfigWithInvalidRelativeHome(t *testing.T) { + home := emptyHome(t) + app, _ := setupQuickstartTestApp(t, "", "") + t.Cleanup(app.Close) + config := filepath.Join(home, "absolute-claude") + t.Setenv("CLAUDE_CONFIG_DIR", config) + dir := filepath.Join(config, "skills", "basecamp") + require.NoError(t, os.MkdirAll(dir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, skillFilename), []byte("managed"), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(dir, ownershipMarkerFile), []byte("managed"), 0o600)) + t.Setenv("HOME", "relative-home") + + cmd := &cobra.Command{} + cmd.SetContext(appctx.WithApp(context.Background(), app)) + err := runRemoveAgentSetup(cmd, app) + require.Error(t, err, "unavailable home-based cleanup must remain visible") + assert.Contains(t, err.Error(), "home-based skill cleanup: getting home directory:") + assert.NotContains(t, err.Error(), "getting home directory: getting home directory:") + _, statErr := os.Lstat(filepath.Join(dir, skillFilename)) + assert.True(t, os.IsNotExist(statErr), "the self-contained Claude integration must still be removed") +} + +func TestSafeSkillTraversalRejectsSymlinkedDefaultParents(t *testing.T) { + home := t.TempDir() + external := t.TempDir() + for _, parent := range []string{".agents", ".claude", ".codex"} { + require.NoError(t, os.Symlink(external, filepath.Join(home, parent))) + safe, err := safeSkillTraversal(home, filepath.Join(home, parent, "skills", "basecamp")) + require.NoError(t, err) + assert.False(t, safe, parent) + } +} + +func TestRemoveCodexPluginReportsConfiguredHomeWithoutUserHome(t *testing.T) { + t.Setenv("HOME", "") + configured := filepath.Join(t.TempDir(), "codex") + t.Setenv("CODEX_HOME", configured) + require.NoError(t, os.MkdirAll(configured, 0o755)) + t.Setenv("PATH", t.TempDir()) + + removed, failure := removeCodexPlugin(context.Background()) + assert.False(t, removed) + assert.Contains(t, failure, "codex binary not found") +} + +func TestSetupAgentsRemoveDeletesClaudeLinkThroughConfigAlias(t *testing.T) { + home := emptyHome(t) + _, err := installSkillFiles() + require.NoError(t, err) + realConfig := filepath.Join(t.TempDir(), "claude") + require.NoError(t, os.MkdirAll(realConfig, 0o755)) + alias := filepath.Join(home, "claude-alias") + require.NoError(t, os.Symlink(realConfig, alias)) + t.Setenv("CLAUDE_CONFIG_DIR", alias) + link, _, err := linkSkillToClaude() + require.NoError(t, err) + + _, err = runSetupAgentsRemove(t) + require.NoError(t, err) + _, statErr := os.Lstat(link) + assert.True(t, os.IsNotExist(statErr)) +} + +func TestRemoveClaudeSkillPreservesLinkToUnmanagedBaseline(t *testing.T) { + home := t.TempDir() + baseline := filepath.Join(home, ".agents", "skills", "basecamp") + require.NoError(t, os.MkdirAll(baseline, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(baseline, skillFilename), []byte("user skill"), 0o644)) + link := filepath.Join(home, ".claude", "skills", "basecamp") + require.NoError(t, os.MkdirAll(filepath.Dir(link), 0o755)) + require.NoError(t, os.Symlink(claudeSkillLinkTarget, link)) + + removed, err := removeClaudeSkill(link, baseline) + require.NoError(t, err) + assert.False(t, removed) + _, statErr := os.Lstat(link) + assert.NoError(t, statErr) +} + +func TestSetupAgentsRemovePreservesUnmanagedSkillsAndLinks(t *testing.T) { + home := emptyHome(t) + baseline := filepath.Join(home, ".agents", "skills", "basecamp") + require.NoError(t, os.MkdirAll(baseline, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(baseline, skillFilename), []byte("user baseline"), 0o600)) + + target := filepath.Join(home, "my-basecamp-skill") + require.NoError(t, os.MkdirAll(target, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(target, skillFilename), []byte("user claude skill"), 0o600)) + claudeSkill := filepath.Join(home, ".claude", "skills", "basecamp") + require.NoError(t, os.MkdirAll(filepath.Dir(claudeSkill), 0o755)) + require.NoError(t, os.Symlink(target, claudeSkill)) + + _, err := runSetupAgentsRemove(t) + require.NoError(t, err) + baselineData, readErr := os.ReadFile(filepath.Join(baseline, skillFilename)) + require.NoError(t, readErr) + assert.Equal(t, "user baseline", string(baselineData)) + _, statErr := os.Lstat(claudeSkill) + assert.NoError(t, statErr) + claudeData, readErr := os.ReadFile(filepath.Join(claudeSkill, skillFilename)) + require.NoError(t, readErr) + assert.Equal(t, "user claude skill", string(claudeData)) +} + +func TestRemoveOwnedSkillFilesRecognizesLegacyVersionMarker(t *testing.T) { + dir := filepath.Join(t.TempDir(), "basecamp") + require.NoError(t, os.MkdirAll(dir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, skillFilename), []byte("legacy managed"), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(dir, installedVersionFile), []byte("0.9.1"), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "mine.txt"), []byte("keep"), 0o600)) + + removed, err := removeOwnedSkillFiles(dir) + require.NoError(t, err) + assert.True(t, removed) + _, err = os.Stat(filepath.Join(dir, "mine.txt")) + assert.NoError(t, err) + _, err = os.Stat(filepath.Join(dir, skillFilename)) + assert.True(t, os.IsNotExist(err)) +} + +func TestRemoveOwnedSkillFilesPreflightsEveryManagedPath(t *testing.T) { + dir := filepath.Join(t.TempDir(), "basecamp") + require.NoError(t, os.MkdirAll(filepath.Join(dir, installedVersionFile), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, skillFilename), []byte("do not partially remove"), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(dir, ownershipMarkerFile), []byte("managed"), 0o600)) + + removed, err := removeOwnedSkillFiles(dir) + assert.False(t, removed) + require.Error(t, err) + assert.Contains(t, err.Error(), "not a regular file") + _, statErr := os.Stat(filepath.Join(dir, skillFilename)) + assert.NoError(t, statErr, "preflight failure must not partially delete managed files") +} + +func TestRemoveClaudePluginUninstallsEveryRecordedValidScope(t *testing.T) { + home := t.TempDir() + claude := installExecutableStub(t, "claude") + project := t.TempDir() + writeClaudeRegistry(t, filepath.Join(home, ".claude"), fmt.Sprintf(`{"version":2,"plugins":{"basecamp@37signals":[{"scope":"project","projectPath":%q},{"scope":"user"}]}}`, project)) + + var calls []string + stubAgentRemoveCommand(t, func(_ context.Context, path, dir string, args ...string) ([]byte, error) { + assert.Equal(t, claude, path) + calls = append(calls, dir+"|"+strings.Join(args, " ")) + return nil, nil + }) + + removed, failures := removeClaudePlugin(context.Background(), filepath.Join(home, ".claude")) + assert.True(t, removed) + assert.Empty(t, failures) + assert.Equal(t, []string{ + project + "|plugin uninstall basecamp@37signals --scope project", + "|plugin uninstall basecamp@37signals --scope user", + }, calls) +} + +func TestRemoveClaudePluginReportsPartialScopeFailure(t *testing.T) { + home := t.TempDir() + installExecutableStub(t, "claude") + project := t.TempDir() + writeClaudeRegistry(t, filepath.Join(home, ".claude"), fmt.Sprintf(`{"version":2,"plugins":{"basecamp@37signals":[{"scope":"project","projectPath":%q},{"scope":"user"}]}}`, project)) + stubAgentRemoveCommand(t, func(_ context.Context, _, _ string, args ...string) ([]byte, error) { + if args[len(args)-1] == "project" { + return []byte("permission denied"), errors.New("exit status 1") + } + return nil, nil + }) + + removed, failures := removeClaudePlugin(context.Background(), filepath.Join(home, ".claude")) + assert.True(t, removed, "the user-scoped installation was still removed") + require.Len(t, failures, 1) + assert.Contains(t, failures[0], "project") + assert.Contains(t, failures[0], "permission denied") +} + +func TestRemoveClaudePluginIgnoresStaleProjectPathForUserScope(t *testing.T) { + home := t.TempDir() + installExecutableStub(t, "claude") + stale := filepath.Join(t.TempDir(), "deleted") + writeClaudeRegistry(t, filepath.Join(home, ".claude"), fmt.Sprintf(`{"version":2,"plugins":{"basecamp@37signals":[{"scope":"user","projectPath":%q}]}}`, stale)) + + stubAgentRemoveCommand(t, func(_ context.Context, _, dir string, _ ...string) ([]byte, error) { + assert.Empty(t, dir) + return nil, nil + }) + + removed, failures := removeClaudePlugin(context.Background(), filepath.Join(home, ".claude")) + assert.True(t, removed) + assert.Empty(t, failures) +} + +func TestRemoveClaudePluginFallsBackForUnknownScope(t *testing.T) { + home := t.TempDir() + installExecutableStub(t, "claude") + writeClaudeRegistry(t, filepath.Join(home, ".claude"), `{"version":2,"plugins":{"basecamp@37signals":[{"scope":"global"}]}}`) + + calls := 0 + stubAgentRemoveCommand(t, func(_ context.Context, _, _ string, args ...string) ([]byte, error) { + calls++ + assert.NotContains(t, args, "--scope") + if calls == 1 { + return nil, nil + } + return []byte("basecamp@37signals is not installed"), errors.New("exit status 1") + }) + + removed, failures := removeClaudePlugin(context.Background(), filepath.Join(home, ".claude")) + assert.True(t, removed) + assert.Empty(t, failures) + assert.Equal(t, 2, calls) +} + +func TestRemoveClaudePluginReportsFailureAfterUnscopedProgress(t *testing.T) { + home := t.TempDir() + installExecutableStub(t, "claude") + writeClaudeRegistry(t, filepath.Join(home, ".claude"), `{"plugins":{"basecamp@37signals":null}}`) + + calls := 0 + stubAgentRemoveCommand(t, func(_ context.Context, _, _ string, _ ...string) ([]byte, error) { + calls++ + if calls == 1 { + return nil, nil + } + return []byte("permission denied"), errors.New("exit status 1") + }) + + removed, failures := removeClaudePlugin(context.Background(), filepath.Join(home, ".claude")) + assert.True(t, removed) + require.Len(t, failures, 1) + assert.Contains(t, failures[0], "permission denied") +} + +func TestRemoveClaudePluginFallsBackWhenRegistryMixesKnownAndUnknownScopes(t *testing.T) { + home := t.TempDir() + installExecutableStub(t, "claude") + writeClaudeRegistry(t, filepath.Join(home, ".claude"), `{"version":2,"plugins":{"basecamp@37signals":[{"scope":"user"},{}]}}`) + + var calls []string + stubAgentRemoveCommand(t, func(_ context.Context, _, _ string, args ...string) ([]byte, error) { + calls = append(calls, strings.Join(args, " ")) + if len(args) > 0 && args[len(args)-1] == "user" { + return nil, nil + } + if len(calls) == 2 { + return nil, nil + } + return []byte("basecamp@37signals is not installed"), errors.New("exit status 1") + }) + + removed, failures := removeClaudePlugin(context.Background(), filepath.Join(home, ".claude")) + assert.True(t, removed) + assert.Empty(t, failures) + assert.Equal(t, []string{ + "plugin uninstall basecamp@37signals --scope user", + "plugin uninstall basecamp@37signals", + "plugin uninstall basecamp@37signals", + }, calls) +} + +func TestRemoveClaudePluginNeverForwardsUnknownScope(t *testing.T) { + home := t.TempDir() + installExecutableStub(t, "claude") + project := t.TempDir() + writeClaudeRegistry(t, filepath.Join(home, ".claude"), fmt.Sprintf(`{"version":2,"plugins":{"basecamp@37signals":[{"scope":"unexpected","projectPath":%q}]}}`, project)) + + stubAgentRemoveCommand(t, func(_ context.Context, _, _ string, args ...string) ([]byte, error) { + assert.NotContains(t, args, "--scope") + return []byte("basecamp@37signals is not installed"), errors.New("exit status 1") + }) + + removed, failures := removeClaudePlugin(context.Background(), filepath.Join(home, ".claude")) + assert.False(t, removed) + assert.Empty(t, failures) +} + +func TestRemoveClaudePluginAcceptsAbsentFallbackAfterScopedRemoval(t *testing.T) { + home := t.TempDir() + installExecutableStub(t, "claude") + writeClaudeRegistry(t, filepath.Join(home, ".claude"), `{"version":2,"plugins":{"basecamp@37signals":[{"scope":"user"},{}]}}`) + + stubAgentRemoveCommand(t, func(_ context.Context, _, _ string, args ...string) ([]byte, error) { + if slices.Contains(args, "--scope") { + return nil, nil + } + return []byte("basecamp@37signals is not installed"), errors.New("exit status 1") + }) + + removed, failures := removeClaudePlugin(context.Background(), filepath.Join(home, ".claude")) + assert.True(t, removed) + assert.Empty(t, failures) +} + +func TestRemoveClaudePluginAcceptsAbsentScopedInstallation(t *testing.T) { + home := t.TempDir() + installExecutableStub(t, "claude") + writeClaudeRegistry(t, filepath.Join(home, ".claude"), `{"version":2,"plugins":{"basecamp@37signals":[{"scope":"user"}]}}`) + + stubAgentRemoveCommand(t, func(_ context.Context, _, _ string, args ...string) ([]byte, error) { + assert.Equal(t, []string{"plugin", "uninstall", "basecamp@37signals", "--scope", "user"}, args) + return []byte("basecamp@37signals is not installed"), errors.New("exit status 1") + }) + + removed, failures := removeClaudePlugin(context.Background(), filepath.Join(home, ".claude")) + assert.False(t, removed) + assert.Empty(t, failures) +} + +func TestRemoveClaudePluginAcceptsNotFoundScopedInstallation(t *testing.T) { + home := t.TempDir() + installExecutableStub(t, "claude") + writeClaudeRegistry(t, filepath.Join(home, ".claude"), `{"version":2,"plugins":{"basecamp@37signals":[{"scope":"user"}]}}`) + + stubAgentRemoveCommand(t, func(_ context.Context, _, _ string, args ...string) ([]byte, error) { + assert.Equal(t, []string{"plugin", "uninstall", "basecamp@37signals", "--scope", "user"}, args) + return []byte("Plugin basecamp@37signals not found"), errors.New("exit status 1") + }) + + removed, failures := removeClaudePlugin(context.Background(), filepath.Join(home, ".claude")) + assert.False(t, removed) + assert.Empty(t, failures) +} + +func TestRemoveClaudePluginDoesNotSwallowUnrelatedAbsentPlugin(t *testing.T) { + home := t.TempDir() + installExecutableStub(t, "claude") + writeClaudeRegistry(t, filepath.Join(home, ".claude"), `{"version":2,"plugins":{"basecamp@37signals":[{"scope":"user"}]}}`) + stubAgentRemoveCommand(t, func(_ context.Context, _, _ string, _ ...string) ([]byte, error) { + return []byte("marketplace helper is not installed"), errors.New("exit status 1") + }) + + removed, failures := removeClaudePlugin(context.Background(), filepath.Join(home, ".claude")) + assert.False(t, removed) + require.Len(t, failures, 1) + assert.Contains(t, failures[0], "marketplace helper") +} + +func TestRemoveClaudePluginReportsUnscopedSafetyLimit(t *testing.T) { + home := t.TempDir() + installExecutableStub(t, "claude") + writeClaudeRegistry(t, filepath.Join(home, ".claude"), `{"plugins":{"basecamp@37signals":null}}`) + stubAgentRemoveCommand(t, func(_ context.Context, _, _ string, _ ...string) ([]byte, error) { + return nil, nil + }) + + removed, failures := removeClaudePlugin(context.Background(), filepath.Join(home, ".claude")) + assert.True(t, removed) + require.Len(t, failures, 1) + assert.Contains(t, failures[0], "safety limit reached") +} + +func TestUninstallClaudeUnscopedUsesSingleDeadline(t *testing.T) { + var deadlines []time.Time + calls := 0 + stubAgentRemoveCommand(t, func(ctx context.Context, _, _ string, _ ...string) ([]byte, error) { + deadline, ok := ctx.Deadline() + require.True(t, ok) + deadlines = append(deadlines, deadline) + calls++ + if calls < 3 { + return nil, nil + } + return []byte("basecamp@37signals not found"), errors.New("exit status 1") + }) + + removed, err := uninstallClaudeUnscoped(context.Background(), "claude", "basecamp@37signals") + require.NoError(t, err) + assert.True(t, removed) + require.Len(t, deadlines, 3) + assert.Equal(t, deadlines[0], deadlines[1]) + assert.Equal(t, deadlines[0], deadlines[2]) +} + +func TestRunAgentRemoveCommandOutlivingGrandchild(t *testing.T) { + sh, err := exec.LookPath("sh") + if err != nil { + t.Skip("sh not available") + } + sleep, err := exec.LookPath("sleep") + if err != nil { + t.Skip("sleep not available") + } + + pidFile := filepath.Join(t.TempDir(), "grandchild.pid") + script := strconv.Quote(sleep) + " 120 & echo $! > " + strconv.Quote(pidFile) + "; exit 0" + t.Cleanup(func() { + raw, readErr := os.ReadFile(pidFile) //nolint:gosec // path is this test's TempDir + if readErr != nil { + return + } + pid, convErr := strconv.Atoi(strings.TrimSpace(string(raw))) + if convErr != nil { + return + } + if proc, findErr := os.FindProcess(pid); findErr == nil { + _ = proc.Kill() + } + }) + + done := make(chan struct{}) + start := time.Now() + go func() { + defer close(done) + _, _ = runAgentRemoveCommand(context.Background(), sh, "", "-c", script) + }() + select { + case <-done: + assert.Less(t, time.Since(start), 10*time.Second) + case <-time.After(10 * time.Second): + t.Fatal("removal command remained blocked on a grandchild's output pipe") + } +} + +func TestRemoveClaudePluginReportsUntargetableProjectScope(t *testing.T) { + home := t.TempDir() + installExecutableStub(t, "claude") + writeClaudeRegistry(t, filepath.Join(home, ".claude"), `{"version":2,"plugins":{"basecamp@37signals":[{"scope":"local"}]}}`) + stubAgentRemoveCommand(t, func(_ context.Context, _, _ string, _ ...string) ([]byte, error) { + t.Fatal("an unlocated project installation must not be removed from the caller's cwd") + return nil, nil + }) + + removed, failures := removeClaudePlugin(context.Background(), filepath.Join(home, ".claude")) + assert.False(t, removed) + require.Len(t, failures, 1) + assert.Contains(t, failures[0], "project path is missing or invalid") +} + +func TestParseClaudePluginInstallationsRejectsNullRoot(t *testing.T) { + installations, ok := parseClaudePluginInstallations([]byte("null")) + assert.False(t, ok) + assert.Nil(t, installations) +} + +func TestParseClaudePluginInstallationsChecksEveryIdentityField(t *testing.T) { + installations, ok := parseClaudePluginInstallations([]byte(`[{"package":"unrelated","id":"basecamp@37signals","scope":"user"}]`)) + require.True(t, ok) + require.Len(t, installations, 1) + assert.Equal(t, "basecamp@37signals", installations[0].Key) + assert.Equal(t, []claudePluginScope{{Name: "user"}}, installations[0].Scopes) +} + +func TestSetupAgentsRemoveHonorsClaudeConfigDir(t *testing.T) { + home := emptyHome(t) + claude := installExecutableStub(t, "claude") + customConfig := filepath.Join(home, "custom-claude") + t.Setenv("CLAUDE_CONFIG_DIR", customConfig) + writeClaudeRegistry(t, customConfig, `{"version":2,"plugins":{"basecamp@37signals":[{"scope":"user"}]}}`) + skillDir := filepath.Join(customConfig, "skills", "basecamp") + require.NoError(t, os.MkdirAll(skillDir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(skillDir, skillFilename), []byte("managed"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(skillDir, ownershipMarkerFile), []byte("managed"), 0o644)) + defaultSkill := filepath.Join(home, ".claude", "skills", "basecamp") + require.NoError(t, os.MkdirAll(defaultSkill, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(defaultSkill, skillFilename), []byte("default-user-skill"), 0o644)) + + stubAgentRemoveCommand(t, func(_ context.Context, path, dir string, args ...string) ([]byte, error) { + assert.Equal(t, claude, path) + assert.Empty(t, dir) + assert.Equal(t, []string{"plugin", "uninstall", "basecamp@37signals", "--scope", "user"}, args) + return nil, nil + }) + + _, err := runSetupAgentsRemove(t) + require.NoError(t, err) + _, statErr := os.Stat(filepath.Join(skillDir, skillFilename)) + assert.True(t, os.IsNotExist(statErr)) + data, readErr := os.ReadFile(filepath.Join(defaultSkill, skillFilename)) + require.NoError(t, readErr) + assert.Equal(t, "default-user-skill", string(data)) +} + +func TestSetupAgentsRemoveCleansProvenDefaultLinkAfterCustomConfigMigration(t *testing.T) { + home := emptyHome(t) + _, err := installSkillFiles() + require.NoError(t, err) + customConfig := filepath.Join(home, "configs", "claude") + t.Setenv("CLAUDE_CONFIG_DIR", customConfig) + customLink, _, err := linkSkillToClaude() + require.NoError(t, err) + assert.Equal(t, filepath.Join(customConfig, "skills", "basecamp"), customLink) + + defaultLink := filepath.Join(home, ".claude", "skills", "basecamp") + require.NoError(t, os.MkdirAll(filepath.Dir(defaultLink), 0o755)) + require.NoError(t, os.Symlink(claudeSkillLinkTarget, defaultLink)) + + _, err = runSetupAgentsRemove(t) + require.NoError(t, err) + for _, path := range []string{customLink, defaultLink} { + _, statErr := os.Lstat(path) + assert.True(t, os.IsNotExist(statErr), path) + } +} + +func TestSetupAgentsRemoveDefersBaselineAliasedByClaudeConfig(t *testing.T) { + home := emptyHome(t) + baseline, err := installSkillFiles() + require.NoError(t, err) + t.Setenv("CLAUDE_CONFIG_DIR", filepath.Join(home, ".agents")) + + defaultLink := filepath.Join(home, ".claude", "skills", "basecamp") + require.NoError(t, os.MkdirAll(filepath.Dir(defaultLink), 0o755)) + require.NoError(t, os.Symlink(claudeSkillLinkTarget, defaultLink)) + + _, err = runSetupAgentsRemove(t) + require.NoError(t, err) + for _, path := range []string{baseline, defaultLink} { + _, statErr := os.Lstat(path) + assert.True(t, os.IsNotExist(statErr), path) + } +} + +func TestSetupAgentsRemoveCleansProvenDefaultLinkWhenClaudeConfigIsInvalid(t *testing.T) { + home := emptyHome(t) + baseline, err := installSkillFiles() + require.NoError(t, err) + defaultLink := filepath.Join(home, ".claude", "skills", "basecamp") + require.NoError(t, os.MkdirAll(filepath.Dir(defaultLink), 0o755)) + require.NoError(t, os.Symlink(claudeSkillLinkTarget, defaultLink)) + t.Setenv("CLAUDE_CONFIG_DIR", "relative/path") + + _, err = runSetupAgentsRemove(t) + require.Error(t, err) + _, statErr := os.Lstat(defaultLink) + assert.True(t, os.IsNotExist(statErr), "managed default link must be removed despite the invalid custom config") + _, statErr = os.Lstat(baseline) + assert.NoError(t, statErr, "baseline must remain when a configured Claude link slot cannot be inspected") +} + +func TestRemoveOwnedOrLegacyCodexSkillRecognizesAuthenticPremarkerInstall(t *testing.T) { + dir := filepath.Join(t.TempDir(), "basecamp") + require.NoError(t, os.MkdirAll(dir, 0o755)) + embedded, err := skills.FS.ReadFile("basecamp/SKILL.md") + require.NoError(t, err) + require.NoError(t, os.WriteFile(filepath.Join(dir, skillFilename), embedded, 0o644)) + + removed, err := removeOwnedOrLegacyCodexSkill(dir) + require.NoError(t, err) + assert.True(t, removed) + _, statErr := os.Stat(dir) + assert.True(t, os.IsNotExist(statErr)) +} + +func TestRemoveOwnedOrLegacyCodexSkillRecognizesAllowlistedPayload(t *testing.T) { + dir := filepath.Join(t.TempDir(), "basecamp") + require.NoError(t, os.MkdirAll(dir, 0o755)) + payload := []byte("synthetic allowlisted legacy skill") + sum := sha256.Sum256(payload) + hash := fmt.Sprintf("%x", sum) + legacyManagedSkillHashes[hash] = struct{}{} + t.Cleanup(func() { delete(legacyManagedSkillHashes, hash) }) + require.NoError(t, os.WriteFile(filepath.Join(dir, skillFilename), payload, 0o644)) + + removed, err := removeOwnedOrLegacyCodexSkill(dir) + require.NoError(t, err) + assert.True(t, removed) +} + +func TestLegacyManagedSkillHashAllowlistDoesNotShrink(t *testing.T) { + want := []string{ + "9ba73c37394e2f3fd41b1fb88dfcb5765c8d28f817a4d8c186cb3bc6eb9b7c0b", + "5b8dbaee9258079695e078c2fffea835d53ac408aa1265fd5116fd4a8657aaed", + "7f388068176a382e1b452452e88ebb9a4712265ca777c81f47350df0845c8839", + "866ffee85417ea2d204efc5b49e4d6bf2c7f74fd3d3e0d4773fe2fbb70640e36", + "ca0db118c4c69211dd9c0169cce36850c8cca1331e83544e0e4638435a157f43", + "2c295c087b0110ca67c3c12ae8a4deda0d2f04390cefeef6719d924526f0e72a", + "c5dfa70f7a7d9ce5ff4d6c780949bb821447c87db345a22e8f533004a2bd0b3f", + "c8465eadd9f1c7cfae8235812d85b424f15f0a65b9fa3878be046deb58b9efcf", + "21dbba9a6419d3bbf215976e591cafb9fdb3f2a4ca7b20a9953efec7f9691e96", + "5bdfcb49c9808011087790c006f8665cc5d8a079961a1dcef760547d8dd9280c", + "a5e60a1c55ec381dab3265625d97461b7c32edd49837a03642abba347852421d", + } + for _, hash := range want { + _, ok := legacyManagedSkillHashes[hash] + assert.True(t, ok, hash) + } +} + +func TestRemoveClaudeSkillRecognizesMarkerlessWizardPayload(t *testing.T) { + dir := filepath.Join(t.TempDir(), "basecamp") + require.NoError(t, os.MkdirAll(dir, 0o755)) + embedded, err := skills.FS.ReadFile("basecamp/SKILL.md") + require.NoError(t, err) + require.NoError(t, os.WriteFile(filepath.Join(dir, skillFilename), embedded, 0o644)) + + removed, err := removeClaudeSkill(dir, filepath.Join(t.TempDir(), "baseline")) + require.NoError(t, err) + assert.True(t, removed) + _, statErr := os.Lstat(dir) + assert.True(t, os.IsNotExist(statErr)) +} + +func TestRemoveOwnedOrLegacyCodexSkillPreservesNonmatchingUserSkill(t *testing.T) { + dir := filepath.Join(t.TempDir(), "basecamp") + require.NoError(t, os.MkdirAll(dir, 0o755)) + path := filepath.Join(dir, skillFilename) + require.NoError(t, os.WriteFile(path, []byte("user-authored"), 0o644)) + + removed, err := removeOwnedOrLegacyCodexSkill(dir) + require.NoError(t, err) + assert.False(t, removed) + data, readErr := os.ReadFile(path) + require.NoError(t, readErr) + assert.Equal(t, "user-authored", string(data)) +} + +func TestSetupAgentsRemoveUsesOfficialCodexCommandWithAliasedHome(t *testing.T) { + home := emptyHome(t) + codex := installExecutableStub(t, "codex") + stubCodexInstalled(t, true, nil) + _, err := installSkillFiles() + require.NoError(t, err) + + agentsHome := filepath.Join(home, ".agents") + alias := filepath.Join(home, "codex-home-alias") + require.NoError(t, os.Symlink(agentsHome, alias)) + t.Setenv("CODEX_HOME", alias) + userPluginFile := filepath.Join(agentsHome, "plugins", "keep.txt") + require.NoError(t, os.MkdirAll(filepath.Dir(userPluginFile), 0o755)) + require.NoError(t, os.WriteFile(userPluginFile, []byte("keep"), 0o600)) + + stubAgentRemoveCommand(t, func(_ context.Context, path, dir string, args ...string) ([]byte, error) { + assert.Equal(t, codex, path) + assert.Empty(t, dir) + assert.Equal(t, []string{"plugin", "remove", "basecamp@37signals", "--json"}, args) + return []byte(`{"removed":true}`), nil + }) + + response, err := runSetupAgentsRemove(t) + require.NoError(t, err) + var envelope struct { + Data struct { + Removed []string `json:"removed"` + } `json:"data"` + } + require.NoError(t, json.Unmarshal(response, &envelope), string(response)) + assert.Contains(t, envelope.Data.Removed, "Codex plugin") + data, readErr := os.ReadFile(userPluginFile) + require.NoError(t, readErr) + assert.Equal(t, "keep", string(data), "CODEX_HOME contents are owned by Codex, not removed directly") +} + +func TestRemoveCodexPluginTreatsAlreadyAbsentAsSuccess(t *testing.T) { + installExecutableStub(t, "codex") + stubCodexInstalled(t, false, nil) + stubAgentRemoveCommand(t, func(_ context.Context, _, _ string, _ ...string) ([]byte, error) { + t.Fatal("remove must not run when the plugin is absent") + return nil, nil + }) + + removed, failure := removeCodexPlugin(context.Background()) + assert.False(t, removed) + assert.Empty(t, failure) +} + +func TestRemoveCodexPluginReportsUnrelatedNotFoundFailure(t *testing.T) { + installExecutableStub(t, "codex") + stubCodexInstalled(t, true, nil) + stubAgentRemoveCommand(t, func(_ context.Context, _, _ string, _ ...string) ([]byte, error) { + return []byte("marketplace metadata not found"), errors.New("exit status 1") + }) + + removed, failure := removeCodexPlugin(context.Background()) + assert.False(t, removed) + assert.Contains(t, failure, "marketplace metadata not found") +} + +func TestSetupAgentsRemoveDeletesOwnedDirectCodexSkill(t *testing.T) { + home := emptyHome(t) + t.Setenv("PATH", t.TempDir()) + codexHome := filepath.Join(home, "custom-codex") + t.Setenv("CODEX_HOME", codexHome) + dir := filepath.Join(codexHome, "skills", "basecamp") + require.NoError(t, os.MkdirAll(dir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, skillFilename), []byte("managed"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(dir, ownershipMarkerFile), []byte("managed"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "mine.txt"), []byte("keep"), 0o600)) + + _, err := runSetupAgentsRemove(t) + require.Error(t, err, "the missing binary is still reported") + _, statErr := os.Stat(filepath.Join(dir, skillFilename)) + assert.True(t, os.IsNotExist(statErr)) + data, readErr := os.ReadFile(filepath.Join(dir, "mine.txt")) + require.NoError(t, readErr) + assert.Equal(t, "keep", string(data)) +} + +func TestSetupAgentsRemoveCleansDefaultCodexSkillWithCustomHome(t *testing.T) { + home := emptyHome(t) + t.Setenv("PATH", t.TempDir()) + t.Setenv("CODEX_HOME", filepath.Join(home, "custom-codex")) + legacy := filepath.Join(home, ".codex", "skills", "basecamp") + require.NoError(t, os.MkdirAll(legacy, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(legacy, skillFilename), []byte("managed"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(legacy, ownershipMarkerFile), []byte("managed"), 0o644)) + + _, err := runSetupAgentsRemove(t) + require.NoError(t, err) + _, statErr := os.Lstat(filepath.Join(legacy, skillFilename)) + assert.True(t, os.IsNotExist(statErr)) +} + +func TestSetupAgentsRemoveDeletesManagedProjectRelativeCodexSkill(t *testing.T) { + emptyHome(t) + t.Setenv("PATH", t.TempDir()) + project := t.TempDir() + t.Chdir(project) + t.Setenv("CODEX_HOME", ".codex") + dir := filepath.Join(project, ".codex", "skills", "basecamp") + require.NoError(t, os.MkdirAll(dir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, skillFilename), []byte("managed"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(dir, ownershipMarkerFile), []byte("managed"), 0o644)) + + _, err := runSetupAgentsRemove(t) + require.Error(t, err, "the missing Codex binary remains reportable") + _, statErr := os.Lstat(filepath.Join(dir, skillFilename)) + assert.True(t, os.IsNotExist(statErr), "verified managed project-relative Codex data must be removed") +} + +func TestSetupAgentsRemovePreservesUnownedDirectCodexSkill(t *testing.T) { + home := emptyHome(t) + t.Setenv("PATH", t.TempDir()) + codexHome := filepath.Join(home, "custom-codex") + t.Setenv("CODEX_HOME", codexHome) + dir := filepath.Join(codexHome, "skills", "basecamp") + require.NoError(t, os.MkdirAll(dir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, skillFilename), []byte("user"), 0o644)) + + _, err := runSetupAgentsRemove(t) + require.Error(t, err, "the missing binary is still reported") + data, readErr := os.ReadFile(filepath.Join(dir, skillFilename)) + require.NoError(t, readErr) + assert.Equal(t, "user", string(data)) +} + +func TestSetupAgentsRemoveReportsMissingCodexBinaryForCustomHome(t *testing.T) { + home := emptyHome(t) + t.Setenv("PATH", t.TempDir()) + customHome := filepath.Join(home, "custom-codex") + require.NoError(t, os.MkdirAll(customHome, 0o755)) + t.Setenv("CODEX_HOME", customHome) + + _, err := runSetupAgentsRemove(t) + var structured *output.Error + require.ErrorAs(t, err, &structured) + assert.Contains(t, structured.Message, "codex binary not found") +} + +func TestSetupAgentsRemoveContinuesAfterPluginFailure(t *testing.T) { + home := emptyHome(t) + t.Setenv("PATH", t.TempDir()) + require.NoError(t, os.MkdirAll(filepath.Join(home, ".codex"), 0o755)) + _, err := installSkillFiles() + require.NoError(t, err) + + _, err = runSetupAgentsRemove(t) + var structured *output.Error + require.ErrorAs(t, err, &structured) + assert.Equal(t, "setup_remove_failed", structured.Code) + assert.Contains(t, structured.Message, "codex binary not found") + _, statErr := os.Stat(filepath.Join(home, ".agents", "skills", "basecamp", skillFilename)) + assert.True(t, os.IsNotExist(statErr), "skill cleanup must continue after a plugin failure") +} + +func TestSetupAgentsRemovePartialFailureHasStructuredMetadata(t *testing.T) { + home := emptyHome(t) + t.Setenv("PATH", t.TempDir()) + require.NoError(t, os.MkdirAll(filepath.Join(home, ".codex"), 0o755)) + _, err := installSkillFiles() + require.NoError(t, err) + + response, err := runSetupAgentsRemoveWithErrorEnvelope(t) + require.Error(t, err) + var envelope struct { + OK bool `json:"ok"` + Meta struct { + Removed []string `json:"removed"` + Failures []string `json:"failures"` + } `json:"meta"` + } + require.NoError(t, json.Unmarshal(response, &envelope), string(response)) + assert.False(t, envelope.OK) + assert.Contains(t, envelope.Meta.Removed, "agent skill") + require.NotEmpty(t, envelope.Meta.Failures) + assert.Contains(t, envelope.Meta.Failures[0], "codex binary not found") +} diff --git a/internal/commands/skill.go b/internal/commands/skill.go index 027f3c41..14a942d6 100644 --- a/internal/commands/skill.go +++ b/internal/commands/skill.go @@ -21,6 +21,15 @@ import ( const skillFilename = "SKILL.md" const installedVersionFile = ".installed-version" +const ownershipMarkerFile = ".managed-by-basecamp-cli" + +var claudeSkillLinkTarget = filepath.Join("..", "..", ".agents", "skills", "basecamp") + +type unmanagedSkillDirError struct{ dir string } + +func (e *unmanagedSkillDirError) Error() string { + return fmt.Sprintf("%s exists but was not written by basecamp-cli; move it aside to let Basecamp install its skill there", e.dir) +} // skillLocation represents a predefined skill installation target. type skillLocation struct { @@ -88,7 +97,7 @@ func newSkillInstallCmd() *cobra.Command { return &cobra.Command{ Use: "install", Short: "Install the basecamp agent skill", - Long: "Copies the embedded SKILL.md to ~/.agents/skills/basecamp/ and creates a symlink in ~/.claude/skills/basecamp (if Claude Code is detected).", + Long: "Copies the embedded SKILL.md to ~/.agents/skills/basecamp/ and creates a symlink in Claude's configured skills directory (if Claude Code is detected).", RunE: func(cmd *cobra.Command, args []string) error { app := appctx.FromContext(cmd.Context()) @@ -127,9 +136,9 @@ func newSkillInstallCmd() *cobra.Command { // installSkillFiles writes the embedded SKILL.md to ~/.agents/skills/basecamp/ // and returns the path to the installed file. func installSkillFiles() (string, error) { - home, err := os.UserHomeDir() + home, err := harness.UserHomeDir() if err != nil { - return "", fmt.Errorf("getting home directory: %w", err) + return "", err } skillDir := filepath.Join(home, ".agents", "skills", "basecamp") @@ -140,19 +149,191 @@ func installSkillFiles() (string, error) { return "", fmt.Errorf("reading embedded skill: %w", err) } - if err := os.MkdirAll(skillDir, 0o755); err != nil { //nolint:gosec // G301: Skill files are not secrets - return "", fmt.Errorf("creating skill directory: %w", err) + created, err := claimSkillDirForWrite(skillDir) + if err != nil { + return "", err } - if err := os.WriteFile(skillFile, data, 0o644); err != nil { //nolint:gosec // G306: Skill files are not secrets + if err := writeSkillFile(skillFile, data); err != nil { + if created { + _ = os.Remove(skillDir) // succeeds only while the claimed directory is empty + } return "", fmt.Errorf("writing skill file: %w", err) } // Best-effort: stamp installed version - _ = os.WriteFile(filepath.Join(skillDir, installedVersionFile), []byte(version.Version), 0o644) //nolint:gosec // G306: not a secret + _ = writeSkillFile(filepath.Join(skillDir, installedVersionFile), []byte(version.Version)) + if err := writeSkillFile(filepath.Join(skillDir, ownershipMarkerFile), []byte("This skill is managed by basecamp-cli. Manual edits will be overwritten on upgrade.\n")); err != nil { + return "", fmt.Errorf("writing skill ownership marker: %w", err) + } return skillFile, nil } +// claimSkillDir is the ownership gate for the shared skill. A prior Basecamp +// install is recognized by either the current ownership marker or the legacy +// version sentinel. Populated unmarked directories and symlinks are user state. +func claimSkillDir(dir string) error { + _, err := claimSkillDirForWrite(dir) + return err +} + +// claimSkillDirForWrite also reports whether this call created the leaf +// directory, allowing a failed first write to roll back only its own empty +// claim and leave pre-existing directories untouched. +func claimSkillDirForWrite(dir string) (bool, error) { + home, err := harness.UserHomeDir() + if err != nil { + return false, err + } + if skillPathWithin(home, dir) { + symlinked, inspectErr := hasSymlinkComponent(home, filepath.Dir(dir)) + if inspectErr != nil { + return false, inspectErr + } + if symlinked { + return false, &unmanagedSkillDirError{dir: dir} + } + } + return claimSkillDirLeafForWrite(dir) +} + +func claimSkillDirLeafForWrite(dir string) (bool, error) { + info, err := os.Lstat(dir) + switch { + case os.IsNotExist(err): + if mkErr := os.MkdirAll(dir, 0o755); mkErr != nil { //nolint:gosec // G301: Skill files are public documentation + return false, fmt.Errorf("creating skill directory: %w", mkErr) + } + return true, nil + case err != nil: + return false, fmt.Errorf("inspecting skill directory: %w", err) + case info.Mode()&os.ModeSymlink != 0 || !info.IsDir(): + return false, &unmanagedSkillDirError{dir: dir} + case !ownedSkillDir(dir): + entries, readErr := os.ReadDir(dir) + if readErr != nil { + return false, fmt.Errorf("inspecting skill directory: %w", readErr) + } + legacyManaged := false + if len(entries) == 1 && entries[0].Name() == skillFilename && entries[0].Type().IsRegular() { + installed, readFileErr := os.ReadFile(filepath.Join(dir, skillFilename)) + if readFileErr != nil { + return false, fmt.Errorf("inspecting skill directory: %w", readFileErr) + } + legacyManaged = recognizedManagedSkillPayload(installed) + } + if !legacyManaged { + return false, &unmanagedSkillDirError{dir: dir} + } + } + return false, nil +} + +// claimPredefinedSkillDir accepts the managed Claude link that points back to +// the canonical Basecamp skill, while preserving claimSkillDir's refusal to +// follow any other symlink. +func claimPredefinedSkillDir(dir string) error { + _, err := claimPredefinedSkillDirForWrite(dir) + return err +} + +func claimPredefinedSkillDirForWrite(dir string) (bool, error) { + root, rootErr := predefinedSkillRoot(dir) + if rootErr != nil { + return false, rootErr + } + symlinked, inspectErr := hasSymlinkComponent(root, filepath.Dir(dir)) + if inspectErr != nil { + return false, inspectErr + } + if symlinked { + return false, &unmanagedSkillDirError{dir: dir} + } + + info, err := os.Lstat(dir) + if err != nil || info.Mode()&os.ModeSymlink == 0 { + return claimSkillDirLeafForWrite(dir) + } + + home, homeErr := harness.UserHomeDir() + if homeErr != nil { + return false, &unmanagedSkillDirError{dir: dir} + } + resolved, resolveErr := filepath.EvalSymlinks(dir) + canonical := filepath.Join(home, ".agents", "skills", "basecamp") + resolvedCanonical, canonicalErr := filepath.EvalSymlinks(canonical) + if resolveErr == nil && canonicalErr == nil && filepath.Clean(resolved) == filepath.Clean(resolvedCanonical) && ownedSkillDir(resolvedCanonical) { + return false, nil + } + return false, &unmanagedSkillDirError{dir: dir} +} + +func predefinedSkillRoot(dir string) (string, error) { + if !filepath.IsAbs(dir) { + cwd, err := os.Getwd() + if err != nil { + return "", fmt.Errorf("getting working directory: %w", err) + } + return cwd, nil + } + home, err := harness.UserHomeDir() + if err != nil { + return "", err + } + for _, configured := range []struct { + env string + resolve func() (string, error) + }{ + {env: "CLAUDE_CONFIG_DIR", resolve: harness.ClaudeConfigDir}, + {env: "CODEX_HOME", resolve: harness.CodexHome}, + } { + if os.Getenv(configured.env) == "" { + continue + } + root, resolveErr := configured.resolve() + if resolveErr != nil { + return "", resolveErr + } + if skillPathWithin(root, dir) { + return root, nil + } + } + if skillPathWithin(home, dir) { + return home, nil + } + // A path outside HOME and the configured agent roots is caller-supplied; + // only its leaf ownership is ours to validate here. + return filepath.Dir(dir), nil +} + +func skillPathWithin(root, target string) bool { + root, rootErr := filepath.Abs(filepath.Clean(root)) + target, targetErr := filepath.Abs(filepath.Clean(target)) + if rootErr != nil || targetErr != nil { + return false + } + relative, err := filepath.Rel(root, target) + return err == nil && relative != ".." && !strings.HasPrefix(relative, ".."+string(filepath.Separator)) +} + +func writeSkillFile(path string, data []byte) error { + if info, err := os.Lstat(path); err == nil { + if !info.Mode().IsRegular() { + return &unmanagedSkillDirError{dir: path} + } + } else if !os.IsNotExist(err) { + return fmt.Errorf("inspecting %s: %w", path, err) + } + return os.WriteFile(path, data, 0o644) //nolint:gosec // G306: Skill files are public documentation +} + +func writeWizardSkill(path string, data []byte, predefined bool) error { + if predefined { + return writeSkillFile(path, data) + } + return os.WriteFile(path, data, 0o644) //nolint:gosec // G306: user explicitly selected this custom path +} + // runSkillWizard runs the interactive skill installation wizard. // skillPromptFailed decides what a failed prompt means. Exactly one outcome is // a success: the user was asked and said no. Everything else is a failure and @@ -192,8 +373,12 @@ func runSkillWizard(cmd *cobra.Command, app *appctx.App) error { fmt.Fprintln(w) // Build options - options := make([]tui.SelectOption, 0, len(skillLocations)+1) - for _, loc := range skillLocations { + locations, locationErr := wizardSkillLocations() + if locationErr != nil { + return fmt.Errorf("resolving skill locations: %w", locationErr) + } + options := make([]tui.SelectOption, 0, len(locations)+1) + for _, loc := range locations { options = append(options, tui.SelectOption{ Value: loc.Path, Label: fmt.Sprintf("%s (%s)", loc.Name, loc.Path), @@ -209,6 +394,8 @@ func runSkillWizard(cmd *cobra.Command, app *appctx.App) error { return skillPromptFailed(w, styles, err) } + selectedPredefined := selectedPath != "other" + // Handle custom path if selectedPath == "other" { selectedPath, err = tui.Input(" Enter custom path", "/path/to/skills/basecamp/SKILL.md") @@ -247,34 +434,69 @@ func runSkillWizard(cmd *cobra.Command, app *appctx.App) error { // Write to selected location dir := filepath.Dir(expandedPath) - if mkErr := os.MkdirAll(dir, 0o755); mkErr != nil { //nolint:gosec // G301: Skill files are not secrets + created := false + if selectedPredefined { + var claimErr error + created, claimErr = claimPredefinedSkillDirForWrite(dir) + if claimErr != nil { + return claimErr + } + } else if mkErr := os.MkdirAll(dir, 0o755); mkErr != nil { //nolint:gosec // G301: Skill files are not secrets return fmt.Errorf("creating directory: %w", mkErr) } - if writeErr := os.WriteFile(expandedPath, data, 0o644); writeErr != nil { //nolint:gosec // G306: Skill files are not secrets + writeErr := writeWizardSkill(expandedPath, data, selectedPredefined) + if writeErr != nil { + if created { + _ = os.Remove(dir) // succeeds only while the claimed directory is empty + } return fmt.Errorf("writing skill file: %w", writeErr) } + if selectedPredefined { + if markerErr := writeSkillFile(filepath.Join(dir, ownershipMarkerFile), []byte("This skill is managed by basecamp-cli. Manual edits will be overwritten on upgrade.\n")); markerErr != nil { + return fmt.Errorf("writing skill ownership marker: %w", markerErr) + } + if versionErr := writeSkillFile(filepath.Join(dir, installedVersionFile), []byte(version.Version)); versionErr != nil { + return fmt.Errorf("writing installed skill version: %w", versionErr) + } + } // Also write to canonical location result := map[string]any{"skill_path": expandedPath} - home, homeErr := os.UserHomeDir() + home, homeErr := harness.UserHomeDir() if homeErr == nil { canonicalDir := filepath.Join(home, ".agents", "skills", "basecamp") canonicalFile := filepath.Join(canonicalDir, skillFilename) if canonicalFile != expandedPath { - if mkErr := os.MkdirAll(canonicalDir, 0o755); mkErr != nil { //nolint:gosec // G301: Skill files are not secrets - result["notice"] = fmt.Sprintf("could not write to %s: %v", canonicalFile, mkErr) - } else if wErr := os.WriteFile(canonicalFile, data, 0o644); wErr != nil { //nolint:gosec // G306: Skill files are not secrets - result["notice"] = fmt.Sprintf("could not write to %s: %v", canonicalFile, wErr) + if _, installErr := installSkillFiles(); installErr != nil { + result["notice"] = fmt.Sprintf("could not write to %s: %v", canonicalFile, installErr) } + } else { + // The user explicitly confirmed this exact destination above, so it + // is safe to mark the selected canonical directory as CLI-managed. + _ = writeSkillFile(filepath.Join(canonicalDir, ownershipMarkerFile), []byte("This skill is managed by basecamp-cli. Manual edits will be overwritten on upgrade.\n")) + _ = writeSkillFile(filepath.Join(canonicalDir, installedVersionFile), []byte(version.Version)) } - // Best-effort: stamp installed version in canonical location - _ = os.WriteFile(filepath.Join(canonicalDir, installedVersionFile), []byte(version.Version), 0o644) //nolint:gosec // G306: not a secret } return app.OK(result, output.WithSummary(fmt.Sprintf("Basecamp skill installed → %s", expandedPath))) } +func wizardSkillLocations() ([]skillLocation, error) { + locations := append([]skillLocation(nil), skillLocations...) + claudeConfig, err := harness.ClaudeConfigDir() + if err != nil { + return nil, err + } + for i := range locations { + if locations[i].Name == "Claude Code (Global)" { + locations[i].Path = filepath.Join(claudeConfig, "skills", "basecamp", skillFilename) + break + } + } + return locations, nil +} + // normalizeSkillPath appends basecamp/SKILL.md to directory paths. // Explicit file paths (any .md) are left as-is. func normalizeSkillPath(path string) string { @@ -315,33 +537,62 @@ func expandSkillPath(path string) string { } func codexGlobalSkillPath() string { - codexHome := strings.TrimSpace(os.Getenv("CODEX_HOME")) + codexHome := os.Getenv("CODEX_HOME") if codexHome == "" { return "~/.codex/skills/basecamp/SKILL.md" } return filepath.Join(codexHome, "skills", "basecamp", skillFilename) } -// linkSkillToClaude creates a symlink at ~/.claude/skills/basecamp pointing to -// the baseline skill directory. Returns (symlinkPath, notice, error). +// linkSkillToClaude creates a symlink in Claude's configured skill directory +// pointing to the baseline skill directory. Returns (symlinkPath, notice, error). func linkSkillToClaude() (string, string, error) { - home, err := os.UserHomeDir() + home, err := harness.UserHomeDir() if err != nil { - return "", "", fmt.Errorf("getting home directory: %w", err) + return "", "", err } skillDir := filepath.Join(home, ".agents", "skills", "basecamp") - symlinkDir := filepath.Join(home, ".claude", "skills") + claudeConfig, err := harness.ClaudeConfigDir() + if err != nil { + return "", "", err + } + symlinkDir := filepath.Join(claudeConfig, "skills") symlinkPath := filepath.Join(symlinkDir, "basecamp") + skillInfo, skillErr := os.Lstat(skillDir) + if skillErr != nil || skillInfo.Mode()&os.ModeSymlink != 0 || !skillInfo.IsDir() || + !ownedSkillDir(skillDir) || !regularFile(filepath.Join(skillDir, skillFilename)) { + return "", "", &unmanagedSkillDirError{dir: skillDir} + } + claudeRoot := claudeConfig + if os.Getenv("CLAUDE_CONFIG_DIR") == "" { + claudeRoot = home + } + symlinked, inspectErr := hasSymlinkComponent(claudeRoot, symlinkDir) + if inspectErr != nil { + return "", "", inspectErr + } + if symlinked { + return "", "", &unmanagedSkillDirError{dir: symlinkDir} + } + if err := os.MkdirAll(symlinkDir, 0o755); err != nil { //nolint:gosec // G301: Skill files are not secrets return "", "", fmt.Errorf("creating symlink directory: %w", err) } + symlinkTarget := skillDir + resolvedSymlinkDir, dirErr := filepath.EvalSymlinks(symlinkDir) + resolvedSkillDir, skillErr := filepath.EvalSymlinks(skillDir) + if dirErr == nil && skillErr == nil { + if relativeTarget, relErr := filepath.Rel(resolvedSymlinkDir, resolvedSkillDir); relErr == nil { + symlinkTarget = relativeTarget + } + } - // Remove existing entry at symlink path (idempotent) - _ = os.Remove(symlinkPath) + if err := removeExistingClaudeSkillLink(symlinkPath, symlinkTarget, skillDir); err != nil { + return "", "", err + } - symlinkTarget := filepath.Join("..", "..", ".agents", "skills", "basecamp") notice := "" if err := os.Symlink(symlinkTarget, symlinkPath); err != nil { // Fallback: copy skill files directly @@ -354,6 +605,33 @@ func linkSkillToClaude() (string, string, error) { return symlinkPath, notice, nil } +func removeExistingClaudeSkillLink(path, expectedTarget, expectedDestination string) error { + info, err := os.Lstat(path) + if os.IsNotExist(err) { + return nil + } + if err != nil { + return fmt.Errorf("inspecting existing skill link: %w", err) + } + if info.Mode()&os.ModeSymlink != 0 { + target, readErr := os.Readlink(path) + if readErr != nil || (target != expectedTarget && !pathsEquivalent(path, expectedDestination)) { + return &unmanagedSkillDirError{dir: path} + } + if removeErr := os.Remove(path); removeErr != nil { + return fmt.Errorf("removing existing skill link: %w", removeErr) + } + return nil + } + if !info.IsDir() || !ownedSkillDir(path) { + return &unmanagedSkillDirError{dir: path} + } + // A directory is the copy fallback from an earlier install. Leave its + // managed files in place until copySkillFiles has a replacement ready; + // this also preserves any additional user files in the directory. + return nil +} + // installedSkillVersion reads the .installed-version file from the baseline // skill directory. Returns "" if absent or unreadable. func installedSkillVersion() string { @@ -383,110 +661,277 @@ func RefreshSkillsIfVersionChanged() bool { return false } - refreshed := refreshAllInstalledSkills() + outcome := refreshInstalledSkills() // Repair Claude symlink if broken (e.g. baseline dir was recreated) if harness.DetectClaude() { - repairClaudeSkillLink() + if repairErr := repairClaudeSkillLink(); repairErr != nil { + outcome.failed++ + } } - // Update sentinel only when no refresh was needed or it succeeded. - // On transient failure, leave the sentinel stale so the next run retries. - needsRefresh := baselineSkillInstalled() - if !needsRefresh || refreshed { + // No work and a successful refresh both advance the sentinel. Any failure at + // any managed location leaves it stale so the next run retries. + if outcome.failed == 0 { // 0o700: GlobalConfigDir can hold credentials.json; keep it owner-only. _ = os.MkdirAll(filepath.Dir(sentinelPath), 0o700) _ = os.WriteFile(sentinelPath, []byte(version.Version), 0o644) //nolint:gosec // G306: not a secret } - return refreshed + return outcome.updated > 0 && outcome.failed == 0 } func refreshAllInstalledSkills() bool { + outcome := refreshInstalledSkills() + return outcome.updated > 0 && outcome.failed == 0 +} + +type skillRefreshOutcome struct { + updated int + failed int +} + +func refreshInstalledSkills() skillRefreshOutcome { embedded, err := skills.FS.ReadFile("basecamp/SKILL.md") if err != nil { - return false + return skillRefreshOutcome{failed: 1} } - updated := 0 - failed := 0 - for _, loc := range append(append([]skillLocation{}, skillLocations...), legacySkillLocations...) { + outcome := skillRefreshOutcome{} + locations := append(append([]skillLocation{}, skillLocations...), legacySkillLocations...) + claudeConfig, claudeConfigErr := harness.ClaudeConfigDir() + if claudeConfigErr != nil { + outcome.failed++ + } else { + configured := filepath.Join(claudeConfig, "skills", "basecamp", skillFilename) + locations = append(locations, skillLocation{Name: "Claude Code (configured)", Path: configured}) + } + for _, loc := range locations { // Skip project-relative paths — no reliable project root in PostRunE. if !strings.HasPrefix(loc.Path, "~") && !filepath.IsAbs(loc.Path) { continue } expanded := expandSkillPath(loc.Path) - if _, statErr := os.Stat(expanded); statErr != nil { - if !os.IsNotExist(statErr) { - failed++ // permission or IO error on a known location + dir := filepath.Dir(expanded) + root, rootErr := refreshLocationRoot(loc, expanded, claudeConfig) + if rootErr != nil { + outcome.failed++ + continue + } + symlinked, inspectErr := hasSymlinkComponent(root, dir) + if inspectErr != nil { + outcome.failed++ + continue + } + if symlinked { + // Claude's canonical installation is intentionally a symlink to the + // shared managed baseline. Leave it for repairClaudeSkillLink below; + // every other predefined symlink remains untrusted. + if strings.HasPrefix(loc.Name, "Claude Code") && managedClaudeSkillLink(dir) { + continue + } + if ownedOrLegacySkillDir(dir) || invalidSkillMarker(dir) { + outcome.failed++ + } + continue + } + dirInfo, dirErr := os.Lstat(dir) + if os.IsNotExist(dirErr) { + continue + } + if dirErr != nil { + outcome.failed++ + continue + } + // Never follow a parent symlink while refreshing a predefined location. + if dirInfo.Mode()&os.ModeSymlink != 0 || !dirInfo.IsDir() { + continue + } + fileInfo, statErr := os.Lstat(expanded) + if statErr != nil { + if os.IsNotExist(statErr) && ownedSkillDir(dir) { + if writeErr := writeSkillFile(expanded, embedded); writeErr == nil { + outcome.updated++ + } else { + outcome.failed++ + } + } else if os.IsNotExist(statErr) && invalidSkillMarker(dir) { + outcome.failed++ + } else if !os.IsNotExist(statErr) { + outcome.failed++ // permission or IO error on a known location } continue } + if !fileInfo.Mode().IsRegular() { + if ownedSkillDir(dir) { + outcome.failed++ + } + continue + } + if !ownedSkillDir(dir) { + if invalidSkillMarker(dir) { + outcome.failed++ + continue + } + entries, readDirErr := os.ReadDir(dir) + if readDirErr != nil { + outcome.failed++ + continue + } + if len(entries) != 1 || entries[0].Name() != skillFilename || !entries[0].Type().IsRegular() { + continue + } + installed, readErr := os.ReadFile(expanded) //nolint:gosec // predefined agent skill path + if readErr != nil { + outcome.failed++ + continue + } + if !recognizedManagedSkillPayload(installed) { + continue + } + if markerErr := writeSkillFile(filepath.Join(dir, ownershipMarkerFile), []byte("This skill is managed by basecamp-cli. Manual edits will be overwritten on upgrade.\n")); markerErr != nil { + outcome.failed++ + continue + } + } - if writeErr := os.WriteFile(expanded, embedded, 0o644); writeErr == nil { //nolint:gosec // G306: Skill files are not secrets - updated++ + if writeErr := writeSkillFile(expanded, embedded); writeErr == nil { + outcome.updated++ } else { - failed++ + outcome.failed++ } } // Stamp installed version in the baseline directory only on full success. - if failed == 0 && updated > 0 { - if home, err := os.UserHomeDir(); err == nil { + if outcome.failed == 0 && outcome.updated > 0 { + if home, err := harness.UserHomeDir(); err == nil { baselineDir := filepath.Join(home, ".agents", "skills", "basecamp") - _ = os.WriteFile(filepath.Join(baselineDir, installedVersionFile), []byte(version.Version), 0o644) //nolint:gosec // G306: not a secret + if ownedSkillDir(baselineDir) { + if stampErr := writeSkillFile(filepath.Join(baselineDir, installedVersionFile), []byte(version.Version)); stampErr != nil { + outcome.failed++ + } + } } } - return updated > 0 && failed == 0 + return outcome } -// repairClaudeSkillLink repairs a broken symlink at ~/.claude/skills/basecamp. -// If the path is a directory (copy fallback), the file refresh already handled it. -func repairClaudeSkillLink() { - home, err := os.UserHomeDir() +func refreshLocationRoot(loc skillLocation, expanded, claudeConfig string) (string, error) { + if loc.Name == "Claude Code (configured)" { + if os.Getenv("CLAUDE_CONFIG_DIR") == "" { + return harness.UserHomeDir() + } + return claudeConfig, nil + } + if loc.Name == "Codex (Global)" { + if os.Getenv("CODEX_HOME") != "" { + return harness.CodexHome() + } + return harness.UserHomeDir() + } + if strings.HasPrefix(loc.Path, "~") { + return harness.UserHomeDir() + } + return filepath.VolumeName(expanded) + string(filepath.Separator), nil +} + +func invalidSkillMarker(dir string) bool { + for _, marker := range []string{ownershipMarkerFile, installedVersionFile} { + markerInfo, markerErr := os.Lstat(filepath.Join(dir, marker)) + if markerErr == nil && !markerInfo.Mode().IsRegular() { + return true + } + if markerErr != nil && !os.IsNotExist(markerErr) { + return true + } + } + return false +} + +func managedClaudeSkillLink(path string) bool { + info, err := os.Lstat(path) + if err != nil || info.Mode()&os.ModeSymlink == 0 { + return false + } + target, err := os.Readlink(path) if err != nil { - return + return false + } + home, err := harness.UserHomeDir() + if err != nil { + return false + } + baseline := filepath.Join(home, ".agents", "skills", "basecamp") + if !ownedOrLegacySkillDir(baseline) { + return false + } + return pathsEquivalent(path, baseline) || brokenLinkTargetsPath(path, target, baseline) +} + +// repairClaudeSkillLink repairs a broken basecamp-cli symlink in Claude's +// configured skill directory. If the path is a directory (copy fallback), the +// file refresh already handled it. Unmanaged symlinks are preserved. +func repairClaudeSkillLink() error { + claudeConfig, err := harness.ClaudeConfigDir() + if err != nil { + return err } - symlinkPath := filepath.Join(home, ".claude", "skills", "basecamp") + symlinkPath := filepath.Join(claudeConfig, "skills", "basecamp") info, err := os.Lstat(symlinkPath) if err != nil { - return // doesn't exist, nothing to repair + if os.IsNotExist(err) { + return nil // doesn't exist, nothing to repair + } + return fmt.Errorf("inspecting Claude skill link: %w", err) } if info.Mode()&os.ModeSymlink == 0 { - return // not a symlink (directory copy fallback), file refresh handled it + return nil // not a symlink (directory copy fallback), file refresh handled it } - // It's a symlink — check if the target is reachable + // It's a symlink — check if the target is reachable. Only a missing target + // proves the link is broken; permission and I/O errors must not trigger a + // destructive repair attempt. if _, statErr := os.Stat(symlinkPath); statErr == nil { - return // symlink is healthy + return nil // symlink is healthy + } else if !os.IsNotExist(statErr) { + return fmt.Errorf("checking Claude skill link target: %w", statErr) } - // Broken symlink — repair it - _, _, _ = linkSkillToClaude() + target, err := os.Readlink(symlinkPath) + if err != nil { + return fmt.Errorf("reading Claude skill link: %w", err) + } + home, err := harness.UserHomeDir() + if err != nil { + return err + } + baseline := filepath.Join(home, ".agents", "skills", "basecamp") + if !brokenLinkTargetsPath(symlinkPath, target, baseline) { + return nil + } + + _, _, err = linkSkillToClaude() + return err } func copySkillFiles(src, dst string) error { if err := os.MkdirAll(dst, 0o755); err != nil { //nolint:gosec // G301: Skill files are not secrets return err } - entries, err := os.ReadDir(src) - if err != nil { - return err - } - for _, e := range entries { - if e.IsDir() { - return fmt.Errorf("skill directory contains subdirectory %q; copy fallback only supports flat files", e.Name()) - } - data, err := os.ReadFile(filepath.Join(src, e.Name())) + for _, name := range []string{skillFilename, installedVersionFile, ownershipMarkerFile} { + data, err := os.ReadFile(filepath.Join(src, name)) if err != nil { - return err + if os.IsNotExist(err) && name != skillFilename { + continue + } + return fmt.Errorf("reading managed skill file %s: %w", name, err) } - if err := os.WriteFile(filepath.Join(dst, e.Name()), data, 0o644); err != nil { //nolint:gosec // G306: Skill files are not secrets - return err + if err := writeSkillFile(filepath.Join(dst, name), data); err != nil { + return fmt.Errorf("writing managed skill file %s: %w", name, err) } } return nil diff --git a/internal/commands/skill_test.go b/internal/commands/skill_test.go index 57c8a777..e8255a54 100644 --- a/internal/commands/skill_test.go +++ b/internal/commands/skill_test.go @@ -7,6 +7,7 @@ import ( "fmt" "os" "path/filepath" + "slices" "strings" "testing" @@ -87,7 +88,7 @@ func TestSkillInstallIdempotent(t *testing.T) { } } -func TestSkillInstallFallbackOnNonEmptyDir(t *testing.T) { +func TestSkillInstallPreservesNonEmptyUnmanagedClaudeDir(t *testing.T) { home := t.TempDir() t.Setenv("HOME", home) // ~/.claude dir exists so DetectClaude() triggers symlink path @@ -95,9 +96,7 @@ func TestSkillInstallFallbackOnNonEmptyDir(t *testing.T) { t.Fatal(err) } - // Pre-create a non-empty directory where the symlink would go. - // os.Remove can't remove non-empty dirs, so symlink creation will fail, - // triggering the copy fallback. + // Pre-create a non-empty, unmarked directory where the symlink would go. symlinkPath := filepath.Join(home, ".claude", "skills", "basecamp") if err := os.MkdirAll(symlinkPath, 0o755); err != nil { t.Fatal(err) @@ -112,25 +111,105 @@ func TestSkillInstallFallbackOnNonEmptyDir(t *testing.T) { cmd.SetOut(&buf) err := cmd.RunE(cmd, nil) - if err != nil { - t.Fatalf("RunE() error = %v (fallback should have handled it)", err) - } + require.Error(t, err) + var unmanaged *unmanagedSkillDirError + require.ErrorAs(t, err, &unmanaged) + data, readErr := os.ReadFile(filepath.Join(symlinkPath, "blocker.txt")) + require.NoError(t, readErr) + assert.Equal(t, "x", string(data)) + _, statErr := os.Stat(filepath.Join(symlinkPath, "SKILL.md")) + assert.True(t, os.IsNotExist(statErr), "unmanaged directory must not be claimed or overwritten") +} - // Verify SKILL.md was copied (not symlinked) - copied, err := os.ReadFile(filepath.Join(symlinkPath, "SKILL.md")) - if err != nil { - t.Fatal("SKILL.md not found in fallback copy location") - } - embedded, _ := skills.FS.ReadFile("basecamp/SKILL.md") - if string(copied) != string(embedded) { - t.Error("fallback copy content does not match embedded") - } +func TestClaimSkillDirRejectsPopulatedUnmarkedPredefinedDestination(t *testing.T) { + dir := filepath.Join(t.TempDir(), "basecamp") + require.NoError(t, os.MkdirAll(dir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "notes.txt"), []byte("mine"), 0o644)) - // Output should mention fallback (via stdout since no app context) - output := buf.String() - if output == "" { - t.Error("expected fallback output, got empty") - } + err := claimSkillDir(dir) + var unmanaged *unmanagedSkillDirError + require.ErrorAs(t, err, &unmanaged) + _, statErr := os.Stat(filepath.Join(dir, ownershipMarkerFile)) + assert.True(t, os.IsNotExist(statErr)) +} + +func TestClaimSkillDirRejectsEmptyUnmarkedPredefinedDestination(t *testing.T) { + dir := filepath.Join(t.TempDir(), "basecamp") + require.NoError(t, os.MkdirAll(dir, 0o755)) + + err := claimSkillDir(dir) + var unmanaged *unmanagedSkillDirError + require.ErrorAs(t, err, &unmanaged) + entries, readErr := os.ReadDir(dir) + require.NoError(t, readErr) + assert.Empty(t, entries) +} + +func TestClaimSkillDirAcceptsMarkerlessManagedPayload(t *testing.T) { + dir := filepath.Join(t.TempDir(), "basecamp") + require.NoError(t, os.MkdirAll(dir, 0o755)) + embedded, err := skills.FS.ReadFile("basecamp/SKILL.md") + require.NoError(t, err) + require.NoError(t, os.WriteFile(filepath.Join(dir, skillFilename), embedded, 0o644)) + + require.NoError(t, claimSkillDir(dir)) +} + +func TestClaimPredefinedSkillDirAcceptsManagedCanonicalLinkOnly(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + _, err := installSkillFiles() + require.NoError(t, err) + canonical := filepath.Join(home, ".agents", "skills", "basecamp") + + managedLink := filepath.Join(t.TempDir(), "basecamp") + require.NoError(t, os.Symlink(canonical, managedLink)) + require.NoError(t, claimPredefinedSkillDir(managedLink)) + + unrelated := t.TempDir() + unrelatedLink := filepath.Join(t.TempDir(), "basecamp") + require.NoError(t, os.Symlink(unrelated, unrelatedLink)) + var unmanaged *unmanagedSkillDirError + require.ErrorAs(t, claimPredefinedSkillDir(unrelatedLink), &unmanaged) +} + +func TestWizardSkillLocationsUsesClaudeConfigDir(t *testing.T) { + custom := filepath.Join(t.TempDir(), "claude") + t.Setenv("CLAUDE_CONFIG_DIR", custom) + + locations, err := wizardSkillLocations() + require.NoError(t, err) + index := slices.IndexFunc(locations, func(location skillLocation) bool { + return location.Name == "Claude Code (Global)" + }) + require.NotEqual(t, -1, index) + assert.Equal(t, filepath.Join(custom, "skills", "basecamp", skillFilename), locations[index].Path) +} + +func TestLinkSkillToClaudeRefreshesManagedCopyWithUserFiles(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("CLAUDE_CONFIG_DIR", filepath.Join(home, ".claude")) + _, err := installSkillFiles() + require.NoError(t, err) + + dir := filepath.Join(home, ".claude", "skills", "basecamp") + require.NoError(t, os.MkdirAll(dir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, skillFilename), []byte("old"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(dir, ownershipMarkerFile), []byte("managed"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "notes.txt"), []byte("keep"), 0o644)) + + _, notice, err := linkSkillToClaude() + require.NoError(t, err) + assert.Contains(t, notice, "copied files instead") + embedded, readErr := skills.FS.ReadFile("basecamp/SKILL.md") + require.NoError(t, readErr) + got, readErr := os.ReadFile(filepath.Join(dir, skillFilename)) + require.NoError(t, readErr) + assert.Equal(t, embedded, got) + notes, readErr := os.ReadFile(filepath.Join(dir, "notes.txt")) + require.NoError(t, readErr) + assert.Equal(t, "keep", string(notes)) } func TestSkillInstallOutputKeys(t *testing.T) { @@ -182,19 +261,27 @@ func TestCopySkillFiles(t *testing.T) { src := t.TempDir() dst := filepath.Join(t.TempDir(), "dest") - // Create test files in source (flat — no subdirs) - if err := os.WriteFile(filepath.Join(src, "SKILL.md"), []byte("skill content"), 0o644); err != nil { + if err := os.WriteFile(filepath.Join(src, skillFilename), []byte("skill content"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(src, ownershipMarkerFile), []byte("managed"), 0o644); err != nil { t.Fatal(err) } if err := os.WriteFile(filepath.Join(src, "extra.txt"), []byte("extra"), 0o644); err != nil { t.Fatal(err) } + if err := os.MkdirAll(dst, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dst, "extra.txt"), []byte("keep"), 0o644); err != nil { + t.Fatal(err) + } if err := copySkillFiles(src, dst); err != nil { t.Fatalf("copySkillFiles() error = %v", err) } - got, err := os.ReadFile(filepath.Join(dst, "SKILL.md")) + got, err := os.ReadFile(filepath.Join(dst, skillFilename)) if err != nil { t.Fatalf("reading SKILL.md: %v", err) } @@ -205,25 +292,48 @@ func TestCopySkillFiles(t *testing.T) { if err != nil { t.Fatalf("reading extra.txt: %v", err) } - if string(got) != "extra" { - t.Errorf("extra.txt = %q, want %q", got, "extra") + if string(got) != "keep" { + t.Errorf("extra.txt = %q, want preserved user content", got) } } -func TestCopySkillFilesRejectsSubdirs(t *testing.T) { +func TestWriteWizardSkillRejectsSymlinkAtPredefinedDestination(t *testing.T) { + target := filepath.Join(t.TempDir(), "target") + require.NoError(t, os.WriteFile(target, []byte("keep"), 0o644)) + link := filepath.Join(t.TempDir(), skillFilename) + require.NoError(t, os.Symlink(target, link)) + + err := writeWizardSkill(link, []byte("replacement"), true) + require.Error(t, err) + got, readErr := os.ReadFile(target) + require.NoError(t, readErr) + assert.Equal(t, "keep", string(got)) +} + +func TestCopySkillFilesRejectsNonRegularManagedDestination(t *testing.T) { src := t.TempDir() dst := filepath.Join(t.TempDir(), "dest") - os.WriteFile(filepath.Join(src, "SKILL.md"), []byte("content"), 0o644) - os.MkdirAll(filepath.Join(src, "subdir"), 0o755) + require.NoError(t, os.WriteFile(filepath.Join(src, skillFilename), []byte("content"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(src, ownershipMarkerFile), []byte("managed"), 0o644)) + require.NoError(t, os.MkdirAll(filepath.Join(dst, skillFilename), 0o755)) err := copySkillFiles(src, dst) - if err == nil { - t.Fatal("expected error for subdirectory in source") - } - if !strings.Contains(err.Error(), "subdirectory") { - t.Errorf("error = %q, want subdirectory rejection message", err) - } + require.Error(t, err) + assert.Contains(t, err.Error(), "was not written by basecamp-cli") +} + +func TestCopySkillFilesAcceptsLegacyVersionOwnershipWithoutMarker(t *testing.T) { + src := t.TempDir() + dst := filepath.Join(t.TempDir(), "dest") + require.NoError(t, os.WriteFile(filepath.Join(src, skillFilename), []byte("content"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(src, installedVersionFile), []byte("0.9.1"), 0o644)) + + require.NoError(t, copySkillFiles(src, dst)) + assert.FileExists(t, filepath.Join(dst, skillFilename)) + assert.FileExists(t, filepath.Join(dst, installedVersionFile)) + _, err := os.Stat(filepath.Join(dst, ownershipMarkerFile)) + assert.True(t, os.IsNotExist(err)) } // Pin the literals rather than deriving them, so a test can't mirror a typo the @@ -272,11 +382,13 @@ func TestRefreshAllInstalledSkills_LegacyOpenCodePath(t *testing.T) { baseline := filepath.Join(home, ".agents", "skills", "basecamp") require.NoError(t, os.MkdirAll(baseline, 0o755)) require.NoError(t, os.WriteFile(filepath.Join(baseline, "SKILL.md"), []byte("old"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(baseline, installedVersionFile), []byte("4.0.0"), 0o644)) // Singular — what the wizard wrote before #624. legacy := filepath.Join(home, ".config", "opencode", "skill", "basecamp") require.NoError(t, os.MkdirAll(legacy, 0o755)) require.NoError(t, os.WriteFile(filepath.Join(legacy, "SKILL.md"), []byte("old"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(legacy, installedVersionFile), []byte("4.0.0"), 0o644)) require.True(t, refreshAllInstalledSkills()) @@ -541,6 +653,406 @@ func TestRefreshSkillsIfVersionChanged_NoSentinelUpdateOnFailure(t *testing.T) { assert.Equal(t, "2.0.0", string(sentinel), "sentinel should remain unchanged on failure") } +func TestRefreshSkillsIfVersionChangedRetriesInvalidClaudeConfig(t *testing.T) { + home := t.TempDir() + configDir := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("PATH", home) + t.Setenv("XDG_CONFIG_HOME", configDir) + t.Setenv("CLAUDE_CONFIG_DIR", "relative/invalid") + + origVersion := version.Version + version.Version = "3.0.0" + defer func() { version.Version = origVersion }() + _, err := installSkillFiles() + require.NoError(t, err) + sentinelDir := filepath.Join(configDir, "basecamp") + require.NoError(t, os.MkdirAll(sentinelDir, 0o755)) + sentinel := filepath.Join(sentinelDir, ".last-run-version") + require.NoError(t, os.WriteFile(sentinel, []byte("2.0.0"), 0o644)) + + assert.False(t, RefreshSkillsIfVersionChanged()) + got, err := os.ReadFile(sentinel) + require.NoError(t, err) + assert.Equal(t, "2.0.0", string(got)) +} + +func TestRefreshSkillsIfVersionChangedAdvancesSentinelForUnmanagedBaseline(t *testing.T) { + home := t.TempDir() + configDir := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("PATH", home) + t.Setenv("XDG_CONFIG_HOME", configDir) + + origVersion := version.Version + version.Version = "3.0.0" + defer func() { version.Version = origVersion }() + + dir := filepath.Join(home, ".agents", "skills", "basecamp") + require.NoError(t, os.MkdirAll(dir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, skillFilename), []byte("user"), 0o644)) + + assert.False(t, RefreshSkillsIfVersionChanged()) + sentinel := filepath.Join(configDir, "basecamp", ".last-run-version") + got, err := os.ReadFile(sentinel) + require.NoError(t, err) + assert.Equal(t, "3.0.0", string(got)) + data, err := os.ReadFile(filepath.Join(dir, skillFilename)) + require.NoError(t, err) + assert.Equal(t, "user", string(data)) +} + +func TestRefreshSkillsIfVersionChangedRetriesManagedLocationFailureWithoutManagedBaseline(t *testing.T) { + home := t.TempDir() + configDir := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("PATH", home) + t.Setenv("XDG_CONFIG_HOME", configDir) + + origVersion := version.Version + version.Version = "3.0.0" + defer func() { version.Version = origVersion }() + + baseline := filepath.Join(home, ".agents", "skills", "basecamp") + require.NoError(t, os.MkdirAll(baseline, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(baseline, skillFilename), []byte("user"), 0o644)) + + claudeDir := filepath.Join(home, ".claude", "skills", "basecamp") + require.NoError(t, os.MkdirAll(claudeDir, 0o755)) + embedded, err := skills.FS.ReadFile("basecamp/SKILL.md") + require.NoError(t, err) + require.NoError(t, os.WriteFile(filepath.Join(claudeDir, skillFilename), embedded, 0o644)) + require.NoError(t, os.Mkdir(filepath.Join(claudeDir, ownershipMarkerFile), 0o755)) + + assert.False(t, RefreshSkillsIfVersionChanged()) + _, err = os.Stat(filepath.Join(configDir, "basecamp", ".last-run-version")) + assert.True(t, os.IsNotExist(err), "a failed managed location must keep the sentinel stale") +} + +func TestRefreshSkillsIfVersionChangedRetriesFailedClaudeLinkRepair(t *testing.T) { + home := t.TempDir() + configDir := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("XDG_CONFIG_HOME", configDir) + installExecutableStub(t, "claude") + + origVersion := version.Version + version.Version = "3.0.0" + defer func() { version.Version = origVersion }() + + link := filepath.Join(home, ".claude", "skills", "basecamp") + require.NoError(t, os.MkdirAll(filepath.Dir(link), 0o755)) + require.NoError(t, os.Symlink(claudeSkillLinkTarget, link)) + opencode := filepath.Join(home, ".config", "opencode", "skills", "basecamp") + require.NoError(t, os.MkdirAll(opencode, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(opencode, skillFilename), []byte("old"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(opencode, ownershipMarkerFile), []byte("managed"), 0o644)) + + assert.False(t, RefreshSkillsIfVersionChanged(), "a partial refresh must not report success") + _, err := os.Stat(filepath.Join(configDir, "basecamp", ".last-run-version")) + assert.True(t, os.IsNotExist(err), "a failed managed link repair must keep the sentinel stale") +} + +func TestRefreshSkillsIfVersionChangedAcceptsManagedClaudeLink(t *testing.T) { + home := t.TempDir() + configDir := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("PATH", home) + t.Setenv("XDG_CONFIG_HOME", configDir) + + origVersion := version.Version + version.Version = "3.0.0" + defer func() { version.Version = origVersion }() + + _, err := installSkillFiles() + require.NoError(t, err) + _, _, err = linkSkillToClaude() + require.NoError(t, err) + + assert.True(t, RefreshSkillsIfVersionChanged()) + sentinel := filepath.Join(configDir, "basecamp", ".last-run-version") + got, err := os.ReadFile(sentinel) + require.NoError(t, err) + assert.Equal(t, "3.0.0", string(got)) +} + +func TestRefreshAllInstalledSkillsRejectsSymlinkedSkillDirectory(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("PATH", home) + + target := filepath.Join(t.TempDir(), "target") + require.NoError(t, os.MkdirAll(target, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(target, skillFilename), []byte("outside"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(target, ownershipMarkerFile), []byte("managed"), 0o644)) + link := filepath.Join(home, ".config", "opencode", "skills", "basecamp") + require.NoError(t, os.MkdirAll(filepath.Dir(link), 0o755)) + require.NoError(t, os.Symlink(target, link)) + + assert.False(t, refreshAllInstalledSkills()) + got, err := os.ReadFile(filepath.Join(target, skillFilename)) + require.NoError(t, err) + assert.Equal(t, "outside", string(got)) + linkInfo, err := os.Lstat(link) + require.NoError(t, err) + assert.NotZero(t, linkInfo.Mode()&os.ModeSymlink) + linkTarget, err := os.Readlink(link) + require.NoError(t, err) + assert.Equal(t, target, linkTarget) +} + +func TestRefreshAllInstalledSkillsRejectsSymlinkedSkillAncestor(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("PATH", home) + + externalConfig := t.TempDir() + target := filepath.Join(externalConfig, "opencode", "skills", "basecamp") + require.NoError(t, os.MkdirAll(target, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(target, skillFilename), []byte("outside"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(target, ownershipMarkerFile), []byte("managed"), 0o644)) + require.NoError(t, os.Symlink(externalConfig, filepath.Join(home, ".config"))) + + assert.False(t, refreshAllInstalledSkills()) + got, err := os.ReadFile(filepath.Join(target, skillFilename)) + require.NoError(t, err) + assert.Equal(t, "outside", string(got)) +} + +func TestLinkSkillToClaudeUsesResolvedConfigDirectoryForRelativeTarget(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + _, err := installSkillFiles() + require.NoError(t, err) + + realConfig := filepath.Join(t.TempDir(), "claude") + require.NoError(t, os.MkdirAll(realConfig, 0o755)) + configAlias := filepath.Join(home, "claude-alias") + require.NoError(t, os.Symlink(realConfig, configAlias)) + t.Setenv("CLAUDE_CONFIG_DIR", configAlias) + + link, _, err := linkSkillToClaude() + require.NoError(t, err) + info, err := os.Lstat(link) + require.NoError(t, err) + assert.NotZero(t, info.Mode()&os.ModeSymlink) + got, err := os.ReadFile(filepath.Join(link, skillFilename)) + require.NoError(t, err) + embedded, err := skills.FS.ReadFile("basecamp/SKILL.md") + require.NoError(t, err) + assert.Equal(t, embedded, got) +} + +func TestLinkSkillToClaudeAcceptsLegacyTargetWhenAgentsDirectoryIsSymlinked(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + realAgents := filepath.Join(t.TempDir(), "agents") + baseline := filepath.Join(realAgents, "skills", "basecamp") + require.NoError(t, os.MkdirAll(baseline, 0o755)) + embedded, err := skills.FS.ReadFile("basecamp/SKILL.md") + require.NoError(t, err) + require.NoError(t, os.WriteFile(filepath.Join(baseline, skillFilename), embedded, 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(baseline, ownershipMarkerFile), []byte("managed"), 0o644)) + require.NoError(t, os.Symlink(realAgents, filepath.Join(home, ".agents"))) + + link := filepath.Join(home, ".claude", "skills", "basecamp") + require.NoError(t, os.MkdirAll(filepath.Dir(link), 0o755)) + require.NoError(t, os.Symlink(claudeSkillLinkTarget, link)) + + got, _, err := linkSkillToClaude() + require.NoError(t, err) + assert.Equal(t, link, got) + data, err := os.ReadFile(filepath.Join(link, skillFilename)) + require.NoError(t, err) + assert.Equal(t, embedded, data) +} + +func TestInstallSkillFilesRejectsSymlinkedParent(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + external := t.TempDir() + require.NoError(t, os.Symlink(external, filepath.Join(home, ".agents"))) + + _, err := installSkillFiles() + var unmanaged *unmanagedSkillDirError + require.ErrorAs(t, err, &unmanaged) + _, statErr := os.Lstat(filepath.Join(external, "skills", "basecamp", skillFilename)) + assert.True(t, os.IsNotExist(statErr)) +} + +func TestClaimPredefinedSkillDirRejectsProjectRelativeSymlinkedParent(t *testing.T) { + project := t.TempDir() + t.Chdir(project) + external := t.TempDir() + require.NoError(t, os.Symlink(external, filepath.Join(project, ".opencode"))) + + _, err := claimPredefinedSkillDirForWrite(filepath.Join(".opencode", "skills", "basecamp")) + var unmanaged *unmanagedSkillDirError + require.ErrorAs(t, err, &unmanaged) + _, statErr := os.Lstat(filepath.Join(external, "skills", "basecamp")) + assert.True(t, os.IsNotExist(statErr)) +} + +func TestLinkSkillToClaudeRejectsSymlinkedSkillsParent(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + _, err := installSkillFiles() + require.NoError(t, err) + external := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(home, ".claude"), 0o755)) + require.NoError(t, os.Symlink(external, filepath.Join(home, ".claude", "skills"))) + + _, _, err = linkSkillToClaude() + var unmanaged *unmanagedSkillDirError + require.ErrorAs(t, err, &unmanaged) + _, statErr := os.Lstat(filepath.Join(external, "basecamp")) + assert.True(t, os.IsNotExist(statErr)) +} + +func TestLinkSkillToClaudeRejectsSymlinkedBaselineDirectory(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + target := filepath.Join(t.TempDir(), "basecamp") + require.NoError(t, os.MkdirAll(target, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(target, skillFilename), []byte("skill"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(target, ownershipMarkerFile), []byte("managed"), 0o644)) + baseline := filepath.Join(home, ".agents", "skills", "basecamp") + require.NoError(t, os.MkdirAll(filepath.Dir(baseline), 0o755)) + require.NoError(t, os.Symlink(target, baseline)) + + _, _, err := linkSkillToClaude() + var unmanaged *unmanagedSkillDirError + require.ErrorAs(t, err, &unmanaged) + _, statErr := os.Lstat(filepath.Join(home, ".claude", "skills", "basecamp")) + assert.True(t, os.IsNotExist(statErr)) +} + +func TestRefreshManagedSkillCountsNonRegularSkillAsFailure(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("PATH", home) + + dir := filepath.Join(home, ".config", "opencode", "skills", "basecamp") + require.NoError(t, os.MkdirAll(filepath.Join(dir, skillFilename), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, ownershipMarkerFile), []byte("managed"), 0o644)) + + outcome := refreshInstalledSkills() + assert.Equal(t, 0, outcome.updated) + assert.Equal(t, 1, outcome.failed) +} + +func TestRefreshManagedSkillRepairsMissingSkill(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("PATH", home) + + dir := filepath.Join(home, ".config", "opencode", "skills", "basecamp") + require.NoError(t, os.MkdirAll(dir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, ownershipMarkerFile), []byte("managed"), 0o644)) + + outcome := refreshInstalledSkills() + assert.Equal(t, 1, outcome.updated) + assert.Equal(t, 0, outcome.failed) + embedded, err := skills.FS.ReadFile("basecamp/SKILL.md") + require.NoError(t, err) + got, err := os.ReadFile(filepath.Join(dir, skillFilename)) + require.NoError(t, err) + assert.Equal(t, embedded, got) +} + +func TestRefreshManagedSkillCountsMalformedMarkerWithMissingSkillAsFailure(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("PATH", home) + + dir := filepath.Join(home, ".config", "opencode", "skills", "basecamp") + require.NoError(t, os.MkdirAll(filepath.Join(dir, ownershipMarkerFile), 0o755)) + + outcome := refreshInstalledSkills() + assert.Equal(t, 0, outcome.updated) + assert.Equal(t, 1, outcome.failed) +} + +func TestRefreshAllInstalledSkillsClaimsVerifiedMarkerlessWizardPayload(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("PATH", home) + + dir := filepath.Join(home, ".config", "opencode", "skills", "basecamp") + require.NoError(t, os.MkdirAll(dir, 0o755)) + embedded, err := skills.FS.ReadFile("basecamp/SKILL.md") + require.NoError(t, err) + require.NoError(t, os.WriteFile(filepath.Join(dir, skillFilename), embedded, 0o644)) + + assert.True(t, refreshAllInstalledSkills()) + assert.True(t, regularFile(filepath.Join(dir, ownershipMarkerFile))) +} + +func TestRefreshAllInstalledSkillsDoesNotClaimMarkerlessDirectoryWithUserFiles(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("PATH", home) + dir := filepath.Join(home, ".config", "opencode", "skills", "basecamp") + require.NoError(t, os.MkdirAll(dir, 0o755)) + embedded, err := skills.FS.ReadFile("basecamp/SKILL.md") + require.NoError(t, err) + require.NoError(t, os.WriteFile(filepath.Join(dir, skillFilename), embedded, 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "notes.txt"), []byte("mine"), 0o644)) + + assert.False(t, refreshAllInstalledSkills()) + _, statErr := os.Lstat(filepath.Join(dir, ownershipMarkerFile)) + assert.True(t, os.IsNotExist(statErr)) + data, err := os.ReadFile(filepath.Join(dir, "notes.txt")) + require.NoError(t, err) + assert.Equal(t, "mine", string(data)) +} + +func TestRefreshAllInstalledSkillsUsesConfiguredClaudeCopy(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + custom := filepath.Join(home, "custom-claude") + t.Setenv("CLAUDE_CONFIG_DIR", custom) + + dir := filepath.Join(custom, "skills", "basecamp") + require.NoError(t, os.MkdirAll(dir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, skillFilename), []byte("old"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(dir, ownershipMarkerFile), []byte("managed"), 0o644)) + + assert.True(t, refreshAllInstalledSkills()) + embedded, err := skills.FS.ReadFile("basecamp/SKILL.md") + require.NoError(t, err) + got, err := os.ReadFile(filepath.Join(dir, skillFilename)) + require.NoError(t, err) + assert.Equal(t, embedded, got) +} + +func TestRefreshAllInstalledSkillsSkipsRelativeCodexHome(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("PATH", home) + project := t.TempDir() + t.Chdir(project) + t.Setenv("CODEX_HOME", ".codex") + + index := slices.IndexFunc(skillLocations, func(location skillLocation) bool { + return location.Name == "Codex (Global)" + }) + require.NotEqual(t, -1, index) + original := skillLocations[index].Path + skillLocations[index].Path = codexGlobalSkillPath() + t.Cleanup(func() { skillLocations[index].Path = original }) + + dir := filepath.Join(project, ".codex", "skills", "basecamp") + require.NoError(t, os.MkdirAll(dir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, skillFilename), []byte("project-owned"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(dir, ownershipMarkerFile), []byte("managed"), 0o644)) + + assert.False(t, refreshAllInstalledSkills()) + data, err := os.ReadFile(filepath.Join(dir, skillFilename)) + require.NoError(t, err) + assert.Equal(t, "project-owned", string(data)) +} + func TestRefreshAllInstalledSkills_MultipleLocations(t *testing.T) { home := t.TempDir() t.Setenv("HOME", home) @@ -557,14 +1069,17 @@ func TestRefreshAllInstalledSkills_MultipleLocations(t *testing.T) { baseline := filepath.Join(home, ".agents", "skills", "basecamp") require.NoError(t, os.MkdirAll(baseline, 0o755)) require.NoError(t, os.WriteFile(filepath.Join(baseline, "SKILL.md"), []byte("old"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(baseline, installedVersionFile), []byte("4.0.0"), 0o644)) claudeSkill := filepath.Join(home, ".claude", "skills", "basecamp") require.NoError(t, os.MkdirAll(claudeSkill, 0o755)) require.NoError(t, os.WriteFile(filepath.Join(claudeSkill, "SKILL.md"), []byte("old"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(claudeSkill, installedVersionFile), []byte("4.0.0"), 0o644)) opencode := filepath.Join(home, ".config", "opencode", "skills", "basecamp") require.NoError(t, os.MkdirAll(opencode, 0o755)) require.NoError(t, os.WriteFile(filepath.Join(opencode, "SKILL.md"), []byte("old"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(opencode, installedVersionFile), []byte("4.0.0"), 0o644)) refreshed := refreshAllInstalledSkills() assert.True(t, refreshed) @@ -597,6 +1112,7 @@ func TestRefreshAllInstalledSkills_SkipsAbsentLocations(t *testing.T) { baseline := filepath.Join(home, ".agents", "skills", "basecamp") require.NoError(t, os.MkdirAll(baseline, 0o755)) require.NoError(t, os.WriteFile(filepath.Join(baseline, "SKILL.md"), []byte("old"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(baseline, installedVersionFile), []byte("4.0.0"), 0o644)) refreshed := refreshAllInstalledSkills() assert.True(t, refreshed) @@ -631,6 +1147,7 @@ func TestRefreshAllInstalledSkills_SkipsProjectRelativePaths(t *testing.T) { baseline := filepath.Join(home, ".agents", "skills", "basecamp") require.NoError(t, os.MkdirAll(baseline, 0o755)) require.NoError(t, os.WriteFile(filepath.Join(baseline, "SKILL.md"), []byte("old"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(baseline, installedVersionFile), []byte("4.0.0"), 0o644)) refreshAllInstalledSkills() @@ -640,31 +1157,47 @@ func TestRefreshAllInstalledSkills_SkipsProjectRelativePaths(t *testing.T) { assert.Equal(t, "project", string(got), "project-relative skill should not be refreshed") } -func TestRepairClaudeSkillLink_BrokenSymlink(t *testing.T) { +func TestRefreshAllInstalledSkillsPreservesUnmanagedSkill(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + + dir := filepath.Join(home, ".agents", "skills", "basecamp") + require.NoError(t, os.MkdirAll(dir, 0o755)) + skillPath := filepath.Join(dir, skillFilename) + require.NoError(t, os.WriteFile(skillPath, []byte("user-authored"), 0o644)) + + assert.False(t, refreshAllInstalledSkills()) + data, err := os.ReadFile(skillPath) + require.NoError(t, err) + assert.Equal(t, "user-authored", string(data)) + _, err = os.Stat(filepath.Join(dir, ownershipMarkerFile)) + assert.True(t, os.IsNotExist(err), "refresh must not claim an unmanaged directory") +} + +func TestRepairClaudeSkillLink_PreservesUnmanagedBrokenSymlink(t *testing.T) { home := t.TempDir() t.Setenv("HOME", home) // Ensure ~/.claude/skills exists so the symlink can be placed there require.NoError(t, os.MkdirAll(filepath.Join(home, ".claude", "skills"), 0o755)) - // Create baseline skill - baseline := filepath.Join(home, ".agents", "skills", "basecamp") - require.NoError(t, os.MkdirAll(baseline, 0o755)) - require.NoError(t, os.WriteFile(filepath.Join(baseline, "SKILL.md"), []byte("skill"), 0o644)) + _, err := installSkillFiles() + require.NoError(t, err) // Create a broken symlink symlinkPath := filepath.Join(home, ".claude", "skills", "basecamp") require.NoError(t, os.Symlink("/nonexistent/target", symlinkPath)) // Verify it's broken - _, err := os.Stat(symlinkPath) + _, err = os.Stat(symlinkPath) require.True(t, os.IsNotExist(err), "symlink should be broken") - repairClaudeSkillLink() + require.NoError(t, repairClaudeSkillLink()) - // Symlink should now be healthy - _, err = os.Stat(filepath.Join(symlinkPath, "SKILL.md")) - assert.NoError(t, err, "skill should be reachable through repaired symlink") + // An arbitrary target is user state, even when broken. + target, readErr := os.Readlink(symlinkPath) + require.NoError(t, readErr) + assert.Equal(t, "/nonexistent/target", target) } func TestRepairClaudeSkillLink_HealthySymlink(t *testing.T) { @@ -683,7 +1216,7 @@ func TestRepairClaudeSkillLink_HealthySymlink(t *testing.T) { // Read the symlink target before repair targetBefore, _ := os.Readlink(filepath.Join(symlinkDir, "basecamp")) - repairClaudeSkillLink() + require.NoError(t, repairClaudeSkillLink()) // Target should be unchanged (no unnecessary repair) targetAfter, _ := os.Readlink(filepath.Join(symlinkDir, "basecamp")) diff --git a/internal/commands/wizard_agents.go b/internal/commands/wizard_agents.go index fb5c3fb1..a8869f4d 100644 --- a/internal/commands/wizard_agents.go +++ b/internal/commands/wizard_agents.go @@ -144,6 +144,9 @@ var agentSetupHandlers = map[string]agentSetupHandler{ // runClaudeSetup performs the Claude Code-specific setup steps // (marketplace add + plugin install + skill symlink). func runClaudeSetup(cmd *cobra.Command, styles *tui.Styles) error { + if _, err := harness.ClaudeConfigDir(); err != nil { + return fmt.Errorf("resolving Claude configuration: %w", err) + } w := cmd.OutOrStdout() // Clean up stale plugin entries from old marketplaces before checking status. @@ -385,6 +388,9 @@ func claudeStaleIssues() []agentIssue { // runClaudeSetupNonInteractive attempts plugin install without prompts (for --json/--agent mode). func runClaudeSetupNonInteractive(cmd *cobra.Command) error { + if _, err := harness.ClaudeConfigDir(); err != nil { + return fmt.Errorf("resolving Claude configuration: %w", err) + } var errs []string // Clean up stale plugin entries from old marketplaces before checking status. @@ -685,10 +691,13 @@ const agentSetupEnv = "BASECAMP_SETUP_AGENT" // selector (or auto-detection), and emits a structured envelope. It never // prompts, so it is safe for the piped installer and coding-agent shells. func newSetupAgentsCmd() *cobra.Command { - return &cobra.Command{ + var remove bool + cmd := &cobra.Command{ Use: "agents", Short: "Install the Basecamp skill and connect detected coding agents", Long: "Install the baseline Basecamp agent skill and attempt to connect coding agents.\n\n" + + "Use --remove to uninstall Basecamp-managed coding-agent integrations without\n" + + "removing authentication, configuration, or Basecamp data.\n\n" + "Selection is controlled by " + agentSetupEnv + ": claude, codex, all, or none. When\n" + "unset, a single detected agent is connected; when several are detected none is\n" + "guessed — the per-agent `basecamp setup ` commands are surfaced instead.", @@ -700,9 +709,14 @@ func newSetupAgentsCmd() *cobra.Command { if app == nil { return fmt.Errorf("app not initialized") } + if remove { + return runRemoveAgentSetup(cmd, app) + } return runNonInteractiveAgentSetup(cmd, app) }, } + cmd.Flags().BoolVar(&remove, "remove", false, "Remove Basecamp-managed coding-agent integrations") + return cmd } // agentSetupRecord is the per-agent outcome captured while running handlers. diff --git a/internal/commands/wizard_test.go b/internal/commands/wizard_test.go index 7b5cecdd..955ac99b 100644 --- a/internal/commands/wizard_test.go +++ b/internal/commands/wizard_test.go @@ -799,6 +799,44 @@ func TestRunClaudeSetupRepairsSkillLink(t *testing.T) { assert.NoError(t, statErr, "skill link should exist after setup repairs it") } +func TestClaudeSetupRejectsRelativeConfigBeforeSideEffects(t *testing.T) { + for _, test := range []struct { + name string + run func(*cobra.Command) error + }{ + { + name: "interactive", + run: func(cmd *cobra.Command) error { + styles := tui.NewStylesWithTheme(tui.ResolveTheme(false)) + return runClaudeSetup(cmd, styles) + }, + }, + {name: "non-interactive", run: runClaudeSetupNonInteractive}, + } { + t.Run(test.name, func(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("CLAUDE_CONFIG_DIR", "relative/claude") + binDir := filepath.Join(home, "bin") + require.NoError(t, os.MkdirAll(binDir, 0o755)) + logFile := filepath.Join(home, "claude-calls.log") + script := "#!/bin/sh\necho \"$*\" >> \"" + logFile + "\"\n" + require.NoError(t, os.WriteFile(filepath.Join(binDir, "claude"), []byte(script), 0o755)) //nolint:gosec // test helper + t.Setenv("PATH", binDir) + + cmd := &cobra.Command{} + cmd.SetContext(context.Background()) + cmd.SetOut(&bytes.Buffer{}) + cmd.SetErr(&bytes.Buffer{}) + err := test.run(cmd) + require.Error(t, err) + assert.Contains(t, err.Error(), "CLAUDE_CONFIG_DIR must be an absolute path") + _, statErr := os.Stat(logFile) + assert.True(t, os.IsNotExist(statErr), "Claude must not run before config validation") + }) + } +} + // TestSetupClaudeNonInteractiveRemovesStalePlugins verifies that non-interactive // setup detects and removes stale plugin entries from old marketplaces. func TestSetupClaudeNonInteractiveRemovesStalePlugins(t *testing.T) { diff --git a/internal/harness/agent_config.go b/internal/harness/agent_config.go new file mode 100644 index 00000000..846ea374 --- /dev/null +++ b/internal/harness/agent_config.go @@ -0,0 +1,60 @@ +package harness + +import ( + "errors" + "fmt" + "os" + "path/filepath" + "strings" +) + +type relativeConfigPathPolicy bool + +const ( + rejectRelativeConfigPath relativeConfigPathPolicy = false + resolveRelativeConfigPath relativeConfigPathPolicy = true +) + +func resolveAgentConfigDir(envName, defaultDir string, relativePolicy relativeConfigPathPolicy) (string, error) { + configured := os.Getenv(envName) + if configured != "" && configured != "~" && !strings.HasPrefix(configured, "~/") && !strings.HasPrefix(configured, "~\\") { + if filepath.IsAbs(configured) { + return filepath.Clean(configured), nil + } + if relativePolicy == rejectRelativeConfigPath { + return "", fmt.Errorf(`%s must be an absolute path, ~, or start with ~/ or ~\`, envName) + } + cwd, err := os.Getwd() + if err != nil { + return "", fmt.Errorf("resolving relative %s: %w", envName, err) + } + return filepath.Join(cwd, configured), nil + } + + home, err := UserHomeDir() + if err != nil { + return "", err + } + if configured == "" { + return filepath.Join(home, defaultDir), nil + } + if configured == "~" || configured == "~/" || configured == "~\\" { + return home, nil + } + return filepath.Join(home, configured[2:]), nil +} + +// UserHomeDir returns the absolute home required for global agent paths. +func UserHomeDir() (string, error) { + home, err := os.UserHomeDir() + if err != nil { + return "", fmt.Errorf("getting home directory: %w", err) + } + if home == "" { + return "", errors.New("getting home directory: empty path") + } + if !filepath.IsAbs(home) { + return "", fmt.Errorf("getting home directory: path must be absolute: %s", home) + } + return filepath.Clean(home), nil +} diff --git a/internal/harness/claude.go b/internal/harness/claude.go index d94e900e..60299971 100644 --- a/internal/harness/claude.go +++ b/internal/harness/claude.go @@ -25,16 +25,21 @@ func init() { func claudeChecks() []*StatusCheck { checks := []*StatusCheck{CheckClaudePlugin()} - // Only check the skill link if ~/.claude exists (i.e. Claude is dir-detected) - home, err := os.UserHomeDir() - if err == nil { - if info, statErr := os.Stat(filepath.Join(home, ".claude")); statErr == nil && info.IsDir() { + // Only check the skill link if Claude's configured home exists. + if configDir, err := ClaudeConfigDir(); err == nil { + if info, statErr := os.Stat(configDir); statErr == nil && info.IsDir() { checks = append(checks, CheckClaudeSkillLink()) } } return checks } +// ClaudeConfigDir resolves Claude Code's configured home. A relative override +// is rejected because its meaning would otherwise depend on the caller's cwd. +func ClaudeConfigDir() (string, error) { + return resolveAgentConfigDir("CLAUDE_CONFIG_DIR", ".claude", rejectRelativeConfigPath) +} + // ClaudeMarketplaceSource is the marketplace repository for the Basecamp plugin. // Migrating from basecamp/basecamp-cli → basecamp/claude-plugins. const ClaudeMarketplaceSource = "basecamp/claude-plugins" @@ -49,12 +54,10 @@ const ClaudeMarketplaceName = "37signals" const ClaudeExpectedPluginKey = ClaudePluginName + "@" + ClaudeMarketplaceName // DetectClaude returns true if Claude Code is installed. -// Checks ~/.claude/ directory first, then falls back to binary on PATH. +// Checks Claude's configured home first, then falls back to binary on PATH. func DetectClaude() bool { - home, err := os.UserHomeDir() - if err == nil { - home = filepath.Clean(home) - info, statErr := os.Stat(filepath.Join(home, ".claude")) + if configDir, err := ClaudeConfigDir(); err == nil { + info, statErr := os.Stat(configDir) if statErr == nil && info.IsDir() { return true } @@ -89,16 +92,16 @@ func FindClaudeBinary() string { // CheckClaudePlugin checks whether the basecamp plugin is installed in Claude Code. func CheckClaudePlugin() *StatusCheck { - home, err := os.UserHomeDir() + configDir, err := ClaudeConfigDir() if err != nil { return &StatusCheck{ Name: "Claude Code Plugin", Status: "warn", - Message: "Cannot determine home directory", + Message: "Cannot determine Claude home: " + err.Error(), } } - pluginsPath := filepath.Join(filepath.Clean(home), ".claude", "plugins", "installed_plugins.json") + pluginsPath := filepath.Join(configDir, "plugins", "installed_plugins.json") data, err := os.ReadFile(pluginsPath) //nolint:gosec // G304: trusted path if err != nil { if os.IsNotExist(err) { @@ -136,18 +139,18 @@ func CheckClaudePlugin() *StatusCheck { } } -// CheckClaudeSkillLink checks whether ~/.claude/skills/basecamp contains a valid SKILL.md. +// CheckClaudeSkillLink checks Claude's configured home for a valid Basecamp SKILL.md. func CheckClaudeSkillLink() *StatusCheck { - home, err := os.UserHomeDir() + configDir, err := ClaudeConfigDir() if err != nil { return &StatusCheck{ Name: "Claude Code Skill", Status: "warn", - Message: "Cannot determine home directory", + Message: "Cannot determine Claude home: " + err.Error(), } } - skillPath := filepath.Join(filepath.Clean(home), ".claude", "skills", "basecamp", "SKILL.md") + skillPath := filepath.Join(configDir, "skills", "basecamp", "SKILL.md") if _, err := os.Stat(skillPath); err != nil { if os.IsNotExist(err) { return &StatusCheck{ @@ -213,14 +216,14 @@ func CheckClaudePluginVersion() *StatusCheck { } } -// InstalledPluginVersion reads the installed plugin version from -// ~/.claude/plugins/installed_plugins.json. Returns "" if unreadable. +// InstalledPluginVersion reads the installed plugin version from Claude's +// configured home. Returns "" if unreadable. func InstalledPluginVersion() string { - home, err := os.UserHomeDir() - if err != nil || home == "" { + configDir, err := ClaudeConfigDir() + if err != nil { return "" } - data, err := os.ReadFile(filepath.Join(filepath.Clean(home), ".claude", "plugins", "installed_plugins.json")) //nolint:gosec // G304: trusted path + data, err := os.ReadFile(filepath.Join(configDir, "plugins", "installed_plugins.json")) //nolint:gosec // G304: trusted path if err != nil { return "" } @@ -371,11 +374,11 @@ type StalePlugin struct { // StalePluginKeys returns stale plugin entries from installed_plugins.json // that belong to old/dead marketplaces. func StalePluginKeys() []StalePlugin { - home, err := os.UserHomeDir() + configDir, err := ClaudeConfigDir() if err != nil { return nil } - data, err := os.ReadFile(filepath.Join(filepath.Clean(home), ".claude", "plugins", "installed_plugins.json")) //nolint:gosec // G304: trusted path + data, err := os.ReadFile(filepath.Join(configDir, "plugins", "installed_plugins.json")) //nolint:gosec // G304: trusted path if err != nil { return nil } diff --git a/internal/harness/claude_test.go b/internal/harness/claude_test.go index 156c9613..658e5dc6 100644 --- a/internal/harness/claude_test.go +++ b/internal/harness/claude_test.go @@ -11,6 +11,47 @@ import ( "github.com/basecamp/basecamp-cli/internal/version" ) +func TestClaudeConfigDirUsesAbsoluteOverrideWithoutHome(t *testing.T) { + configured := filepath.Join(t.TempDir(), "claude-config") + t.Setenv("HOME", "") + t.Setenv("CLAUDE_CONFIG_DIR", configured) + + got, err := ClaudeConfigDir() + require.NoError(t, err) + assert.Equal(t, configured, got) +} + +func TestClaudeConfigDirPreservesWhitespaceInOverride(t *testing.T) { + configured := filepath.Join(t.TempDir(), "claude config ") + t.Setenv("CLAUDE_CONFIG_DIR", configured) + + got, err := ClaudeConfigDir() + require.NoError(t, err) + assert.Equal(t, configured, got) +} + +func TestClaudeConfigDirRejectsRelativeHome(t *testing.T) { + t.Setenv("HOME", "relative-home") + t.Setenv("CLAUDE_CONFIG_DIR", "") + + _, err := ClaudeConfigDir() + require.Error(t, err) + assert.Contains(t, err.Error(), "home directory: path must be absolute") +} + +func TestClaudeChecksReportConfigError(t *testing.T) { + t.Setenv("CLAUDE_CONFIG_DIR", "relative") + + plugin := CheckClaudePlugin() + assert.Equal(t, "warn", plugin.Status) + assert.Contains(t, plugin.Message, "CLAUDE_CONFIG_DIR must be an absolute path") + assert.Contains(t, plugin.Message, `~/ or ~\`) + + skill := CheckClaudeSkillLink() + assert.Equal(t, "warn", skill.Status) + assert.Contains(t, skill.Message, "CLAUDE_CONFIG_DIR must be an absolute path") +} + func TestPluginInstalled_ArrayFormat(t *testing.T) { data := []byte(`[{"name": "basecamp", "version": "1.0.0"}]`) assert.True(t, pluginInstalled(data)) diff --git a/internal/harness/codex.go b/internal/harness/codex.go index e1e29860..15591513 100644 --- a/internal/harness/codex.go +++ b/internal/harness/codex.go @@ -74,9 +74,9 @@ func init() { // DetectCodex returns true when Codex has a home directory or executable. func DetectCodex() bool { - home, err := os.UserHomeDir() + codexHome, err := CodexHome() if err == nil { - info, statErr := os.Stat(filepath.Join(filepath.Clean(home), ".codex")) + info, statErr := os.Stat(codexHome) if statErr == nil && info.IsDir() { return true } @@ -84,6 +84,18 @@ func DetectCodex() bool { return FindCodexBinary() != "" } +// CodexHome resolves CODEX_HOME, including shell-style home aliases. +func CodexHome() (string, error) { + return resolveAgentConfigDir("CODEX_HOME", ".codex", resolveRelativeConfigPath) +} + +// CodexPluginInstalledContext reports whether Basecamp is currently installed. +// Callers can use it to distinguish a successful no-op removal from a real removal. +func CodexPluginInstalledContext(ctx context.Context) (bool, error) { + state, found, err := queryCodexPlugin(ctx) + return found && state.Installed, err +} + // FindCodexBinary returns the Codex executable path, or an empty string. func FindCodexBinary() string { if path, err := codexLookPath("codex"); err == nil { diff --git a/internal/harness/codex_test.go b/internal/harness/codex_test.go index f28606e7..0fd0be5a 100644 --- a/internal/harness/codex_test.go +++ b/internal/harness/codex_test.go @@ -27,6 +27,69 @@ func TestDetectCodexDirectory(t *testing.T) { assert.True(t, DetectCodex()) } +func TestCodexHomeAliases(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + + tests := []struct { + configured string + want string + }{ + {"", filepath.Join(home, ".codex")}, + {"~", home}, + {"~/", home}, + {"~\\", home}, + {"~/custom", filepath.Join(home, "custom")}, + } + for _, test := range tests { + t.Run(strconv.Quote(test.configured), func(t *testing.T) { + t.Setenv("CODEX_HOME", test.configured) + got, err := CodexHome() + require.NoError(t, err) + assert.Equal(t, filepath.Clean(test.want), got) + }) + } +} + +func TestCodexHomePreservesWhitespaceInOverride(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + configured := " codex home " + t.Setenv("CODEX_HOME", configured) + cwd, err := os.Getwd() + require.NoError(t, err) + + got, err := CodexHome() + require.NoError(t, err) + assert.Equal(t, filepath.Join(cwd, configured), got) +} + +func TestCodexHomeResolvesRelativeOverrideFromWorkingDirectory(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + t.Setenv("CODEX_HOME", "project-local-codex") + cwd, err := os.Getwd() + require.NoError(t, err) + + got, err := CodexHome() + require.NoError(t, err) + assert.Equal(t, filepath.Join(cwd, "project-local-codex"), got) +} + +func TestCodexHomeDoesNotRequireHomeForSelfContainedOverride(t *testing.T) { + t.Setenv("HOME", "") + absolute := filepath.Join(t.TempDir(), "codex") + t.Setenv("CODEX_HOME", absolute) + got, err := CodexHome() + require.NoError(t, err) + assert.Equal(t, absolute, got) + + t.Setenv("CODEX_HOME", "project-local-codex") + cwd, err := os.Getwd() + require.NoError(t, err) + got, err = CodexHome() + require.NoError(t, err) + assert.Equal(t, filepath.Join(cwd, "project-local-codex"), got) +} + func TestDetectCodexBinary(t *testing.T) { home := t.TempDir() t.Setenv("HOME", home) @@ -36,6 +99,18 @@ func TestDetectCodexBinary(t *testing.T) { assert.Equal(t, "/usr/local/bin/codex", FindCodexBinary()) } +func TestDetectCodexCustomHomeWithoutBinary(t *testing.T) { + home := t.TempDir() + customHome := filepath.Join(t.TempDir(), "custom-codex") + t.Setenv("HOME", home) + t.Setenv("CODEX_HOME", customHome) + stubCodexLookPath(t, "", exec.ErrNotFound) + + assert.False(t, DetectCodex()) + require.NoError(t, os.MkdirAll(customHome, 0o755)) + assert.True(t, DetectCodex()) +} + func TestCheckCodexPluginMissingBinary(t *testing.T) { t.Setenv("HOME", t.TempDir()) stubCodexLookPath(t, "", exec.ErrNotFound) diff --git a/internal/output/envelope.go b/internal/output/envelope.go index 6903cf8d..16db928c 100644 --- a/internal/output/envelope.go +++ b/internal/output/envelope.go @@ -187,6 +187,26 @@ func (w *Writer) Err(err error, opts ...ErrorResponseOption) error { // ErrorResponseOption modifies an ErrorResponse. type ErrorResponseOption func(*ErrorResponse) +// ErrorMetaProvider supplies command-specific structured error metadata. +type ErrorMetaProvider interface { + ErrorMetadata() map[string]any +} + +// WithErrorMeta adds command-specific structured details to an error response. +func WithErrorMeta(meta map[string]any) ErrorResponseOption { + return func(r *ErrorResponse) { + if len(meta) == 0 { + return + } + if r.Meta == nil { + r.Meta = make(map[string]any) + } + for key, value := range meta { + r.Meta[key] = value + } + } +} + // WithErrorStats adds session metrics to the error response metadata. func WithErrorStats(metrics *observability.SessionMetrics) ErrorResponseOption { return func(r *ErrorResponse) { diff --git a/skills/basecamp/SKILL.md b/skills/basecamp/SKILL.md index ca867626..b8a9616c 100644 --- a/skills/basecamp/SKILL.md +++ b/skills/basecamp/SKILL.md @@ -1302,11 +1302,15 @@ basecamp doctor --json # Check CLI health, auth, conn ```bash basecamp setup agents # Install skill + connect detected agent(s) basecamp setup agents --json # Structured result envelope +basecamp setup agents --remove # Remove CLI-managed agent integrations only ``` `setup agents` installs the baseline skill and connects coding agents without prompting. Selection is driven by `BASECAMP_SETUP_AGENT` (`claude`, `codex`, `all`, or `none`); unset auto-detects — one detected agent is connected, several leave the skill only and surface the per-agent `basecamp setup ` commands. +`--remove` removes Basecamp-managed skills and Claude/Codex plugins without +removing authentication, configuration, or Basecamp data. Unmanaged skill +directories and additional user files are preserved. **Rate limiting (429):** The CLI handles backoff automatically. If you see 429 errors, reduce request frequency.