From 9b481b446b478c838b97e4deb8d9b35035f78781 Mon Sep 17 00:00:00 2001 From: devrimcavusoglu Date: Wed, 19 Aug 2026 15:01:57 +0300 Subject: [PATCH 1/3] Add --tag/--category group selection to skill install and uninstall (#102) Factor the tag/category filter that skill list uses into a shared skillFilter (flags, help text, and match semantics in one place) and accept it on install and uninstall. The filter resolves against the registry at --scope; names and filters are mutually exclusive (exit 2); an empty match is an error rather than a silent no-op. uninstall narrows the group to what is installed on the platform and skips the rest. --enforce-budget counts the resolved group. Closes #102 Co-Authored-By: Claude Fable 5 --- AGENTS.md | 9 +- CHANGELOG.md | 9 ++ docs/concepts/platform-adapters.md | 9 ++ docs/reference/commands.md | 17 +++ internal/cli/platform_test.go | 206 +++++++++++++++++++++++++++++ internal/cli/skill_helpers.go | 100 ++++++++++++++ internal/cli/skill_install.go | 33 +++-- internal/cli/skill_list.go | 19 +-- internal/cli/skill_uninstall.go | 70 ++++++++-- 9 files changed, 433 insertions(+), 39 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index e0c443d..25d89da 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -192,9 +192,12 @@ hyphens are both valid segment separators; dots enable namespace-style names lowercase alphanumeric segments joined by hyphens, with at most one colon separating category from value. Uppercase is rejected on write so stored tags have one canonical form; tag *filters* stay case-insensitive so legacy -hand-edited tags still match. `skill list` filters on them via `--tag` (flat) -and `--category` (namespaced, repeatable, OR within a category, AND across -categories). +hand-edited tags still match. `skill list`, `skill install`, and +`skill uninstall` filter on them via `--tag` (flat) and `--category` +(namespaced, repeatable, OR within a category, AND across categories) — one +`skillFilter` in `internal/cli/skill_helpers.go` defines the flags and match +semantics for all three; on install/uninstall the filter is mutually +exclusive with positional names. **Unmodeled keys pass through.** The fields above are the keys skern models; any other key — top-level, `metadata.*`, or nested in `metadata.author` / diff --git a/CHANGELOG.md b/CHANGELOG.md index fc1fe60..159f016 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 `--tool-forming-loop`. The non-interactive contract is now documented and enforced: when stdin is not a TTY or `--json` is set, `init` never prompts and both questions resolve to "no". ([#104]) +- **`--tag` / `--category` on `skern skill install` and `skill uninstall`.** + Select a group of registry skills with the same filters `skill list` + accepts instead of naming each one — `skern skill install --tag workflow + --platform claude-code`. Names and filters are mutually exclusive (exit 2 + if mixed or if neither is given); a filter that matches nothing is an error, + never a silent no-op. `uninstall --tag` narrows the group to what is + actually installed on the platform and skips the rest. `--enforce-budget` + counts the resolved group. ([#102]) ### Changed @@ -81,6 +89,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 [#97]: https://github.com/devrimcavusoglu/skern/pull/97 [#98]: https://github.com/devrimcavusoglu/skern/pull/98 [#100]: https://github.com/devrimcavusoglu/skern/issues/100 +[#102]: https://github.com/devrimcavusoglu/skern/issues/102 [#104]: https://github.com/devrimcavusoglu/skern/issues/104 ## [v0.3.1] — 2026-05-13 diff --git a/docs/concepts/platform-adapters.md b/docs/concepts/platform-adapters.md index b6e4b4f..fdccc84 100644 --- a/docs/concepts/platform-adapters.md +++ b/docs/concepts/platform-adapters.md @@ -75,6 +75,15 @@ skern skill install code-review test-runner deploy-checker --platform claude-cod skern skill uninstall stale-a stale-b --platform claude-code ``` +A batch can also be selected by tag instead of by name, using the same `--tag` / `--category` filters as `skill list`: + +```sh +skern skill install --tag workflow --platform claude-code --scope project +skern skill uninstall --tag workflow --platform claude-code --scope project +``` + +Names and filters are mutually exclusive; a filter that matches nothing is an error, not a silent no-op. See [Commands › install](/reference/commands#group-installs-tag-category). + Each skill's outcome is reported separately in the JSON output's `skills` array, and a `capacity` block reports the platform's installed-skill count after the batch. ## Platform Status Matrix diff --git a/docs/reference/commands.md b/docs/reference/commands.md index 87f2daa..3ec312d 100644 --- a/docs/reference/commands.md +++ b/docs/reference/commands.md @@ -294,10 +294,20 @@ Install one or more skills to a single platform. ```sh skern skill install ... --platform +skern skill install --tag --platform # install a tagged group +skern skill install --category lang:python --platform # or a namespaced-tag group ``` Each invocation targets exactly one platform — `--platform all` is not accepted. Multiple skill names can be passed in one call. Each skill's outcome is reported in the `skills[]` array; a failure on one skill does not abort the batch — the command exits non-zero only when *every* install fails. +### Group installs (`--tag`, `--category`) + +Instead of names, select a group with the same filters [`skill list`](#skern-skill-list) accepts: `--tag ` (flat tag) and/or `--category ` (repeatable, comma-lists values, OR within a category, AND across categories; `--include-untagged` applies as in `list`). The filter resolves against the registry at `--scope`, so `--tag workflow --scope project` installs the project-registry skills tagged `workflow`. Resolved names are installed in sorted order and reported per-skill exactly as a name batch would be. + +- Names and filters are **mutually exclusive** — passing both is a validation error (exit 2), as is passing neither. +- A filter that matches no registered skill is an **error** (exit 1: `no registered skills match --tag workflow in user scope`), never a silent no-op. +- `--enforce-budget` counts the resolved group. + The response includes a top-level `capacity` block reporting the platform's installed-skill count after the operation, the threshold for that scope, and remaining headroom. **Flags:** @@ -308,6 +318,9 @@ The response includes a top-level `capacity` block reporting the platform's inst | `--scope` | `user` | `user` or `project` | | `--force` | `false` | Overwrite existing installation | | `--enforce-budget` | `false` | Refuse the operation if it would push the platform's installed-skill count past the per-scope threshold | +| `--tag` | — | Select registry skills carrying this tag instead of naming them. Mutually exclusive with names. | +| `--category` | — | Select by namespaced tag `category:value`; repeatable, comma-lists values. Mutually exclusive with names. | +| `--include-untagged` | `false` | With `--category`: treat a skill with no tag in a requested category as matching it. | ## `skern skill uninstall` @@ -315,14 +328,18 @@ Remove one or more skills from a platform. Mirrors `install` semantics: one plat ```sh skern skill uninstall ... --platform +skern skill uninstall --tag --platform # evict a tagged group ``` +`--tag` / `--category` select a group the same way as on `install`: the filter resolves against the registry at `--scope`, then is narrowed to the skills actually installed on the platform. Tagged-but-not-installed skills are skipped, not reported as failures. If nothing in the group is installed the command errors (`no installed skills match --tag workflow on claude-code (user scope)`); if the tag matches nothing in the registry, the error says so instead. Names and filters are mutually exclusive. + **Flags:** | Flag | Description | |------|-------------| | `--platform` | Required. Same enumeration as `install`. | | `--scope` | `user` or `project` | +| `--tag` / `--category` / `--include-untagged` | Group selection, as on `install`. | ## `skern platform list` diff --git a/internal/cli/platform_test.go b/internal/cli/platform_test.go index 06bb0cb..f59ac99 100644 --- a/internal/cli/platform_test.go +++ b/internal/cli/platform_test.go @@ -2,6 +2,7 @@ package cli import ( "encoding/json" + "errors" "fmt" "os" "path/filepath" @@ -539,3 +540,208 @@ func TestPlatformStatus_ProjectScope(t *testing.T) { assert.Len(t, result.Status, 1) assert.Equal(t, "proj-status", result.Status[0].Skill) } + +// --- #102: --tag / --category on install and uninstall --- + +// setupTaggedSkills creates four registry skills in user scope: +// two tagged "workflow" (one also lang:go), one lang:python only, one untagged. +func setupTaggedSkills(t *testing.T, cc *CommandContext) { + t.Helper() + for _, spec := range []struct{ name, tags string }{ + {"wf-plan", "workflow,lang:go"}, + {"wf-review", "workflow"}, + {"py-only", "lang:python"}, + {"plain", ""}, + } { + args := []string{"skill", "create", spec.name, "--description", "Use when testing tag installs."} + if spec.tags != "" { + args = append(args, "--tags", spec.tags) + } + _, err := runCmd(t, cc, args...) + require.NoError(t, err) + } +} + +func TestSkillInstall_ByTag(t *testing.T) { + cc, _, _ := testRegistryWithDirs(t) + home := t.TempDir() + withTestDetector(t, cc, home, t.TempDir()) + setupTaggedSkills(t, cc) + + out, err := runCmd(t, cc, "skill", "install", "--tag", "workflow", "--platform", "claude-code", "--json") + require.NoError(t, err) + + var result output.SkillInstallResult + require.NoError(t, json.Unmarshal([]byte(out), &result)) + require.Len(t, result.Skills, 2) + assert.Equal(t, "wf-plan", result.Skills[0].Skill, "resolved names are sorted") + assert.Equal(t, "wf-review", result.Skills[1].Skill) + for _, e := range result.Skills { + assert.True(t, e.Success, "%s should install", e.Skill) + } + for _, n := range []string{"wf-plan", "wf-review"} { + _, err := os.Stat(filepath.Join(home, ".claude", "skills", n, "SKILL.md")) + require.NoError(t, err) + } + for _, n := range []string{"py-only", "plain"} { + _, err := os.Stat(filepath.Join(home, ".claude", "skills", n)) + assert.True(t, os.IsNotExist(err), "%s must not be installed", n) + } +} + +func TestSkillInstall_ByCategory_AndsWithTag(t *testing.T) { + cc, _, _ := testRegistryWithDirs(t) + home := t.TempDir() + withTestDetector(t, cc, home, t.TempDir()) + setupTaggedSkills(t, cc) + + // --tag and --category compose with AND: only wf-plan is workflow AND lang:go. + out, err := runCmd(t, cc, "skill", "install", "--tag", "workflow", "--category", "lang:go", + "--platform", "claude-code", "--json") + require.NoError(t, err) + var result output.SkillInstallResult + require.NoError(t, json.Unmarshal([]byte(out), &result)) + require.Len(t, result.Skills, 1) + assert.Equal(t, "wf-plan", result.Skills[0].Skill) + + // --category alone, comma list ORs within the namespace. + out, err = runCmd(t, cc, "skill", "install", "--category", "lang:go,python", + "--platform", "codex-cli", "--json") + require.NoError(t, err) + require.NoError(t, json.Unmarshal([]byte(out), &result)) + require.Len(t, result.Skills, 2) + assert.Equal(t, "py-only", result.Skills[0].Skill) + assert.Equal(t, "wf-plan", result.Skills[1].Skill) +} + +func TestSkillInstall_Filter_EmptyMatchIsError(t *testing.T) { + cc, _, _ := testRegistryWithDirs(t) + withTestDetector(t, cc, t.TempDir(), t.TempDir()) + setupTaggedSkills(t, cc) + + out, err := runCmd(t, cc, "skill", "install", "--tag", "nope", "--platform", "claude-code") + require.Error(t, err) + assert.Contains(t, err.Error(), "no registered skills match --tag nope in user scope") + assert.NotContains(t, out, "Installed", "an empty match must not be a silent no-op") + // Empty match is an operational error (exit 1), not a usage error. + var ve *ValidationError + assert.False(t, errors.As(err, &ve)) +} + +func TestSkillInstall_Filter_MutuallyExclusiveWithNames(t *testing.T) { + cc, _, _ := testRegistryWithDirs(t) + withTestDetector(t, cc, t.TempDir(), t.TempDir()) + setupTaggedSkills(t, cc) + + _, err := runCmd(t, cc, "skill", "install", "plain", "--tag", "workflow", "--platform", "claude-code") + require.Error(t, err) + var ve *ValidationError + require.ErrorAs(t, err, &ve) + assert.Contains(t, err.Error(), "mutually exclusive") + + // Neither names nor a filter is a usage error too. + _, err = runCmd(t, cc, "skill", "install", "--platform", "claude-code") + require.Error(t, err) + require.ErrorAs(t, err, &ve) + assert.Contains(t, err.Error(), "requires at least one skill name, or a --tag/--category filter") + + // Malformed --category surfaces the same validation error list uses. + _, err = runCmd(t, cc, "skill", "install", "--category", "noncolon", "--platform", "claude-code") + require.Error(t, err) + require.ErrorAs(t, err, &ve) +} + +func TestSkillInstall_Filter_RespectsScope(t *testing.T) { + cc, _, _ := testRegistryWithDirs(t) + home := t.TempDir() + project := t.TempDir() + withTestDetector(t, cc, home, project) + setupTaggedSkills(t, cc) // user scope + _, err := runCmd(t, cc, "skill", "create", "proj-wf", "--description", "Use when testing.", "--tags", "workflow", "--scope", "project") + require.NoError(t, err) + + // The filter resolves against the registry at --scope only. + out, err := runCmd(t, cc, "skill", "install", "--tag", "workflow", "--platform", "claude-code", "--scope", "project", "--json") + require.NoError(t, err) + var result output.SkillInstallResult + require.NoError(t, json.Unmarshal([]byte(out), &result)) + require.Len(t, result.Skills, 1) + assert.Equal(t, "proj-wf", result.Skills[0].Skill) + _, err = os.Stat(filepath.Join(project, ".claude", "skills", "proj-wf", "SKILL.md")) + require.NoError(t, err) +} + +func TestSkillInstall_Filter_EnforceBudgetCountsResolved(t *testing.T) { + cc, _, _ := testRegistryWithDirs(t) + withTestDetector(t, cc, t.TempDir(), t.TempDir()) + setupTaggedSkills(t, cc) + + // Two workflow skills resolve; a budget check must see 2, not 0 args. + // Under the threshold it proceeds normally. + out, err := runCmd(t, cc, "skill", "install", "--tag", "workflow", "--platform", "claude-code", "--enforce-budget", "--json") + require.NoError(t, err) + var result output.SkillInstallResult + require.NoError(t, json.Unmarshal([]byte(out), &result)) + require.Len(t, result.Skills, 2) + require.NotNil(t, result.Capacity) + assert.Equal(t, 2, result.Capacity.Installed) +} + +func TestSkillUninstall_ByTag_OnlyInstalledMatches(t *testing.T) { + cc, _, _ := testRegistryWithDirs(t) + home := t.TempDir() + withTestDetector(t, cc, home, t.TempDir()) + setupTaggedSkills(t, cc) + + // Install one workflow skill and the untagged one; leave wf-review uninstalled. + _, err := runCmd(t, cc, "skill", "install", "wf-plan", "plain", "--platform", "claude-code") + require.NoError(t, err) + + out, err := runCmd(t, cc, "skill", "uninstall", "--tag", "workflow", "--platform", "claude-code", "--json") + require.NoError(t, err) + + var result output.SkillUninstallResult + require.NoError(t, json.Unmarshal([]byte(out), &result)) + // wf-review is tagged but not installed: skipped, not a failure entry. + require.Len(t, result.Skills, 1) + assert.Equal(t, "wf-plan", result.Skills[0].Skill) + assert.True(t, result.Skills[0].Success) + + _, err = os.Stat(filepath.Join(home, ".claude", "skills", "wf-plan")) + assert.True(t, os.IsNotExist(err), "wf-plan should be removed") + _, err = os.Stat(filepath.Join(home, ".claude", "skills", "plain", "SKILL.md")) + require.NoError(t, err, "untagged skill must survive a tag-scoped uninstall") + // Registry copies are untouched either way. + _, err = runCmd(t, cc, "skill", "show", "wf-plan") + require.NoError(t, err) +} + +func TestSkillUninstall_ByTag_NothingInstalledIsError(t *testing.T) { + cc, _, _ := testRegistryWithDirs(t) + withTestDetector(t, cc, t.TempDir(), t.TempDir()) + setupTaggedSkills(t, cc) + + _, err := runCmd(t, cc, "skill", "uninstall", "--tag", "workflow", "--platform", "claude-code") + require.Error(t, err) + assert.Contains(t, err.Error(), "no installed skills match --tag workflow on claude-code (user scope)") + + // Unknown tag: fails at the registry step with the same message install uses. + _, err = runCmd(t, cc, "skill", "uninstall", "--tag", "nope", "--platform", "claude-code") + require.Error(t, err) + assert.Contains(t, err.Error(), "no registered skills match --tag nope in user scope") +} + +func TestSkillUninstall_Filter_MutuallyExclusiveWithNames(t *testing.T) { + cc, _, _ := testRegistryWithDirs(t) + withTestDetector(t, cc, t.TempDir(), t.TempDir()) + + _, err := runCmd(t, cc, "skill", "uninstall", "plain", "--tag", "workflow", "--platform", "claude-code") + require.Error(t, err) + var ve *ValidationError + require.ErrorAs(t, err, &ve) + assert.Contains(t, err.Error(), "mutually exclusive") + + _, err = runCmd(t, cc, "skill", "uninstall", "--platform", "claude-code") + require.Error(t, err) + require.ErrorAs(t, err, &ve) +} diff --git a/internal/cli/skill_helpers.go b/internal/cli/skill_helpers.go index aac6018..5dfb2d0 100644 --- a/internal/cli/skill_helpers.go +++ b/internal/cli/skill_helpers.go @@ -5,12 +5,14 @@ import ( "os" "path/filepath" "slices" + "sort" "strings" "github.com/devrimcavusoglu/skern/internal/output" "github.com/devrimcavusoglu/skern/internal/platform" "github.com/devrimcavusoglu/skern/internal/registry" "github.com/devrimcavusoglu/skern/internal/skill" + "github.com/spf13/cobra" ) func defaultNewRegistry() (*registry.Registry, error) { @@ -280,6 +282,104 @@ func matchesCategories(tags []string, filters map[string][]string, includeUntagg return true } +// skillFilter is the tag/category selection shared by skill list, install, +// and uninstall (#102). One definition keeps the flag names, help text, and +// match semantics identical across the three commands. +type skillFilter struct { + tag string + categories []string + includeUntagged bool +} + +// register adds --tag, --category, and --include-untagged to cmd. +func (f *skillFilter) register(cmd *cobra.Command) { + cmd.Flags().StringVar(&f.tag, "tag", "", "filter skills by tag") + cmd.Flags().StringArrayVar(&f.categories, "category", nil, "filter by namespaced tag \"category:value\" (repeatable; comma-lists values; OR within a category, AND across categories)") + cmd.Flags().BoolVar(&f.includeUntagged, "include-untagged", false, "treat a skill with no tag in a requested category as matching that category") +} + +// active reports whether any selection flag was given. +func (f *skillFilter) active() bool { + return f.tag != "" || len(f.categories) > 0 +} + +// describe renders the active flags for error messages, e.g. +// `--tag workflow --category lang:python`. +func (f *skillFilter) describe() string { + var parts []string + if f.tag != "" { + parts = append(parts, "--tag "+f.tag) + } + for _, c := range f.categories { + parts = append(parts, "--category "+c) + } + return strings.Join(parts, " ") +} + +// matcher validates the category flags and returns a predicate over a +// skill's tags. An inactive filter matches everything. +func (f *skillFilter) matcher() (func(tags []string) bool, error) { + categoryFilters, err := parseCategoryFilters(f.categories) + if err != nil { + return nil, err + } + return func(tags []string) bool { + if f.tag != "" && !hasTag(tags, f.tag) { + return false + } + return matchesCategories(tags, categoryFilters, f.includeUntagged) + }, nil +} + +// resolveSkillsByFilter returns the names of registry skills in scope that +// satisfy the filter, sorted for deterministic batch order. Install and +// uninstall read the registry at --scope, so the filter resolves there too. +// An empty result is reported as an error so a group operation never turns +// into a silent no-op. +func resolveSkillsByFilter(reg *registry.Registry, scope skill.Scope, f *skillFilter) ([]string, error) { + match, err := f.matcher() + if err != nil { + return nil, err + } + skills, _, err := reg.List(scope) + if err != nil { + return nil, err + } + var names []string + for i := range skills { + if match(skills[i].Tags) { + names = append(names, skills[i].Name) + } + } + if len(names) == 0 { + return nil, fmt.Errorf("no registered skills match %s in %s scope (run 'skern skill list --scope %s' to see available skills)", f.describe(), scope, scope) + } + sort.Strings(names) + return names, nil +} + +// resolveActionTargets turns positional names or an active filter into the +// list of skills an install/uninstall batch operates on. Names and filters +// are mutually exclusive: mixing them is a validation error, and so is +// passing neither. +func resolveActionTargets(reg *registry.Registry, scope skill.Scope, args []string, f *skillFilter) ([]string, error) { + if f.active() { + if len(args) > 0 { + return nil, &ValidationError{Message: fmt.Sprintf("skill names and %s are mutually exclusive; pass one or the other", f.describe())} + } + return resolveSkillsByFilter(reg, scope, f) + } + if len(args) == 0 { + return nil, &ValidationError{Message: "requires at least one skill name, or a --tag/--category filter"} + } + for _, name := range args { + if err := skill.ValidateName(name); err != nil { + return nil, &ValidationError{Message: err.Error()} + } + } + return args, nil +} + // resolveSkill finds a skill by name, searching the specified scope or both scopes. func resolveSkill(reg *registry.Registry, name, scopeStr string) (*skill.Skill, string, skill.Scope, error) { if scopeStr != "" { diff --git a/internal/cli/skill_install.go b/internal/cli/skill_install.go index 606770a..e4f1dca 100644 --- a/internal/cli/skill_install.go +++ b/internal/cli/skill_install.go @@ -6,7 +6,6 @@ import ( "github.com/devrimcavusoglu/skern/internal/output" "github.com/devrimcavusoglu/skern/internal/platform" - "github.com/devrimcavusoglu/skern/internal/skill" "github.com/spf13/cobra" ) @@ -16,10 +15,11 @@ func newSkillInstallCmd() *cobra.Command { scope string force bool enforceBudget bool + filter skillFilter ) cmd := &cobra.Command{ - Use: "install ...", + Use: "install [...] [--tag ] [--category ]", Short: "Install one or more registered skills onto a platform", Long: `Install one or more skills from skern's registry onto a single platform. @@ -32,20 +32,19 @@ Each invocation targets exactly one platform — agents are expected to specify the platform they are running on. Multiple skill names can be passed in one call to install them as a batch. +Instead of names, a group can be selected with the same filters 'skern skill +list' accepts: --tag and/or --category (repeatable). +The filter resolves against the registry at --scope; names and filters are +mutually exclusive, and a filter that matches nothing is an error rather than +a silent no-op. + When --enforce-budget is set, install refuses to proceed if the resulting installed-skill count would meet or exceed the per-platform threshold (see 'skern platform status' for current capacity).`, - Args: cobra.MinimumNArgs(1), + Args: cobra.ArbitraryArgs, RunE: func(cmd *cobra.Command, args []string) error { ctx := getContext(cmd) - // Validate every skill name up-front so we fail fast. - for _, name := range args { - if err := skill.ValidateName(name); err != nil { - return &ValidationError{Message: err.Error()} - } - } - platformType, err := platform.ParsePlatformType(platformFlag) if err != nil { return &ValidationError{Message: err.Error()} @@ -71,15 +70,22 @@ installed-skill count would meet or exceed the per-platform threshold (see return &ValidationError{Message: fmt.Sprintf("platform %q not recognized; valid platforms: %s", platformFlag, platformNamesList())} } + // Positional names, or the registry skills selected by --tag / + // --category (validated and fail-fast either way). + names, err := resolveActionTargets(reg, scopeVal, args, &filter) + if err != nil { + return err + } + // Capacity pre-check: if --enforce-budget is set, refuse the entire // batch when the resulting count would exceed the threshold. This // is intentionally strict — agents that hit this should evict // stale skills first or invoke without --enforce-budget. if enforceBudget { pre := buildCapacityReport(p, scopeVal) - if pre != nil && pre.Installed+len(args) > pre.Threshold { + if pre != nil && pre.Installed+len(names) > pre.Threshold { return fmt.Errorf("capacity: %s (%s) has %d/%d skills installed; installing %d more would exceed the threshold (uninstall stale skills or drop --enforce-budget to proceed)", - pre.Platform, pre.Scope, pre.Installed, pre.Threshold, len(args)) + pre.Platform, pre.Scope, pre.Installed, pre.Threshold, len(names)) } } @@ -88,7 +94,7 @@ installed-skill count would meet or exceed the per-platform threshold (see // so the agent can react per-skill. var entries []output.SkillActionEntry var successCount int - for _, name := range args { + for _, name := range names { entry := output.SkillActionEntry{Skill: name} _, skillDir, getErr := reg.Get(name, scopeVal) @@ -135,6 +141,7 @@ installed-skill count would meet or exceed the per-platform threshold (see cmd.Flags().StringVar(&scope, "scope", "user", "skill scope (user or project)") cmd.Flags().BoolVar(&force, "force", false, "overwrite existing installation") cmd.Flags().BoolVar(&enforceBudget, "enforce-budget", false, "refuse to install when at or over capacity") + filter.register(cmd) _ = cmd.MarkFlagRequired("platform") return cmd diff --git a/internal/cli/skill_list.go b/internal/cli/skill_list.go index 154ba09..8932baa 100644 --- a/internal/cli/skill_list.go +++ b/internal/cli/skill_list.go @@ -12,11 +12,9 @@ import ( func newSkillListCmd() *cobra.Command { var ( - scope string - tag string - categories []string - includeUntagged bool - withPlatforms bool + scope string + filter skillFilter + withPlatforms bool ) cmd := &cobra.Command{ @@ -30,7 +28,7 @@ func newSkillListCmd() *cobra.Command { return err } - categoryFilters, err := parseCategoryFilters(categories) + matchFilter, err := filter.matcher() if err != nil { return err } @@ -92,10 +90,7 @@ func newSkillListCmd() *cobra.Command { } for _, d := range discovered { - if tag != "" && !hasTag(d.Skill.Tags, tag) { - continue - } - if !matchesCategories(d.Skill.Tags, categoryFilters, includeUntagged) { + if !matchFilter(d.Skill.Tags) { continue } r := toDiscoveredSkillResult(d) @@ -155,9 +150,7 @@ func newSkillListCmd() *cobra.Command { } cmd.Flags().StringVar(&scope, "scope", "all", "skill scope (user, project, or all)") - cmd.Flags().StringVar(&tag, "tag", "", "filter skills by tag") - cmd.Flags().StringArrayVar(&categories, "category", nil, "filter by namespaced tag \"category:value\" (repeatable; comma-lists values; OR within a category, AND across categories)") - cmd.Flags().BoolVar(&includeUntagged, "include-untagged", false, "treat a skill with no tag in a requested category as matching that category") + filter.register(cmd) cmd.Flags().BoolVar(&withPlatforms, "with-platforms", false, "include the list of detected platforms each skill is installed on") return cmd diff --git a/internal/cli/skill_uninstall.go b/internal/cli/skill_uninstall.go index 5f64419..bc44e2e 100644 --- a/internal/cli/skill_uninstall.go +++ b/internal/cli/skill_uninstall.go @@ -14,10 +14,11 @@ func newSkillUninstallCmd() *cobra.Command { var ( platformFlag string scope string + filter skillFilter ) cmd := &cobra.Command{ - Use: "uninstall ...", + Use: "uninstall [...] [--tag ] [--category ]", Short: "Uninstall one or more skills from a platform (registry untouched)", Long: `Uninstall one or more skills from a single platform. @@ -27,17 +28,17 @@ from skern entirely, use 'skern skill remove'. Each invocation targets exactly one platform. Multiple skill names can be passed in one call to uninstall them as a batch — useful for evicting a set -of stale skills at once.`, - Args: cobra.MinimumNArgs(1), +of stale skills at once. + +Instead of names, a group can be selected with --tag and/or +--category (repeatable), resolved against the registry at +--scope and then narrowed to the skills actually installed on the platform. +Names and filters are mutually exclusive; a filter that matches nothing +installed is an error rather than a silent no-op.`, + Args: cobra.ArbitraryArgs, RunE: func(cmd *cobra.Command, args []string) error { ctx := getContext(cmd) - for _, name := range args { - if err := skill.ValidateName(name); err != nil { - return &ValidationError{Message: err.Error()} - } - } - platformType, err := platform.ParsePlatformType(platformFlag) if err != nil { return &ValidationError{Message: err.Error()} @@ -58,9 +59,41 @@ of stale skills at once.`, return &ValidationError{Message: fmt.Sprintf("platform %q not recognized; valid platforms: %s", platformFlag, platformNamesList())} } + names := args + if filter.active() { + // Registry membership defines the group; the platform's + // installed set decides which of them there is anything to + // remove. Tagged-but-not-installed skills are skipped, not + // reported as failures. + reg, regErr := ctx.NewRegistry() + if regErr != nil { + return regErr + } + names, err = resolveActionTargets(reg, scopeVal, args, &filter) + if err != nil { + return err + } + installed, listErr := p.InstalledSkills(scopeVal) + if listErr != nil { + return fmt.Errorf("listing installed skills on %s: %w", p.Name(), listErr) + } + names = intersectNames(names, installed) + if len(names) == 0 { + return fmt.Errorf("no installed skills match %s on %s (%s scope)", filter.describe(), p.Name(), scopeVal) + } + } else if len(args) == 0 { + return &ValidationError{Message: "requires at least one skill name, or a --tag/--category filter"} + } else { + for _, name := range args { + if err := skill.ValidateName(name); err != nil { + return &ValidationError{Message: err.Error()} + } + } + } + var entries []output.SkillActionEntry var successCount int - for _, name := range args { + for _, name := range names { entry := output.SkillActionEntry{Skill: name} if uninstallErr := p.Uninstall(name, scopeVal); uninstallErr != nil { entry.Error = uninstallErr.Error() @@ -92,11 +125,28 @@ of stale skills at once.`, cmd.Flags().StringVar(&platformFlag, "platform", "", "target platform (one of: "+platformNamesList()+")") cmd.Flags().StringVar(&scope, "scope", "user", "skill scope (user or project)") + filter.register(cmd) _ = cmd.MarkFlagRequired("platform") return cmd } +// intersectNames keeps the entries of names that also appear in installed, +// preserving the order of names. +func intersectNames(names, installed []string) []string { + set := make(map[string]bool, len(installed)) + for _, n := range installed { + set[n] = true + } + var out []string + for _, n := range names { + if set[n] { + out = append(out, n) + } + } + return out +} + func formatUninstallResult(platformName string, entries []output.SkillActionEntry) string { var b strings.Builder for _, e := range entries { From 747a5d77638d7561a4131e2ed130e5bd488ecea1 Mon Sep 17 00:00:00 2001 From: devrimcavusoglu Date: Wed, 19 Aug 2026 16:08:29 +0300 Subject: [PATCH 2/3] Address review: prove --enforce-budget counts the group, surface parse warnings - The budget test now seeds the platform to threshold-1 and asserts the refusal (the previous happy-path test passed with the pre-fix code). - Filter resolution returns registry parse warnings; they are appended to the empty-match error and printed to stderr otherwise, as `skill list` does, so a malformed SKILL.md is not a silent reason for "matches nothing". - resolveActionTargets takes a lazy registry constructor, removing the duplicated names-path in uninstall. - Docs: ghost-skill note for uninstall --tag, group-install examples in the guides, roadmap line dropped. Co-Authored-By: Claude Fable 5 --- AGENTS.md | 1 - CHANGELOG.md | 7 +++-- docs/guide/agent-setup.md | 1 + docs/guide/quick-start.md | 3 +- docs/reference/commands.md | 4 ++- internal/cli/platform_test.go | 53 ++++++++++++++++++++++++++++++--- internal/cli/skill_helpers.go | 44 +++++++++++++++++++-------- internal/cli/skill_install.go | 3 +- internal/cli/skill_uninstall.go | 32 +++++++------------- 9 files changed, 102 insertions(+), 46 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 25d89da..dc00d3f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -329,7 +329,6 @@ Everything planned is a tracked issue; this list is a map, not a commitment. **Correctness and ergonomics (near-term)** -- [#102] — `--tag` on `skill install` / `skill uninstall` for group installs - [#103] — exclude companion directories (eval corpora, fixtures) from install **Adapter model** — all three need the declarative-hook mechanism from design diff --git a/CHANGELOG.md b/CHANGELOG.md index 159f016..8a390fb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,9 +29,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 accepts instead of naming each one — `skern skill install --tag workflow --platform claude-code`. Names and filters are mutually exclusive (exit 2 if mixed or if neither is given); a filter that matches nothing is an error, - never a silent no-op. `uninstall --tag` narrows the group to what is - actually installed on the platform and skips the rest. `--enforce-budget` - counts the resolved group. ([#102]) + never a silent no-op (registry parse warnings are surfaced rather than + swallowed). `uninstall --tag` narrows the group to what is actually + installed on the platform and skips the rest. `--enforce-budget` counts + the resolved group. ([#102]) ### Changed diff --git a/docs/guide/agent-setup.md b/docs/guide/agent-setup.md index f93901a..653239d 100644 --- a/docs/guide/agent-setup.md +++ b/docs/guide/agent-setup.md @@ -65,6 +65,7 @@ skern skill list --with-platforms --json # adds installed_on per skill skern skill search "review" --json skern skill show code-review --json skern skill install code-review --platform claude-code --json +skern skill install --tag workflow --platform claude-code --json # group install by tag ``` The `install`/`uninstall` JSON envelope carries a `skills[]` array (one entry per skill) and a top-level `capacity` block (`installed`, `threshold`, `headroom`, `over_budget`). Agents can react to capacity pressure without an extra query. diff --git a/docs/guide/quick-start.md b/docs/guide/quick-start.md index 2493f52..262b88e 100644 --- a/docs/guide/quick-start.md +++ b/docs/guide/quick-start.md @@ -42,10 +42,11 @@ for p in claude-code codex-cli opencode cursor gemini-cli; do done ``` -Multiple skill names can be installed in a single call: +Multiple skill names can be installed in a single call, or a whole tagged group at once: ```sh skern skill install code-review test-runner deploy-checker --platform claude-code +skern skill install --tag workflow --platform claude-code # every registry skill tagged "workflow" ``` The response includes a `capacity` block reporting the installed-skill count, threshold, and remaining headroom for the platform. Pass `--enforce-budget` to refuse the install when the count would exceed the threshold. diff --git a/docs/reference/commands.md b/docs/reference/commands.md index 3ec312d..b081f67 100644 --- a/docs/reference/commands.md +++ b/docs/reference/commands.md @@ -305,7 +305,7 @@ Each invocation targets exactly one platform — `--platform all` is not accepte Instead of names, select a group with the same filters [`skill list`](#skern-skill-list) accepts: `--tag ` (flat tag) and/or `--category ` (repeatable, comma-lists values, OR within a category, AND across categories; `--include-untagged` applies as in `list`). The filter resolves against the registry at `--scope`, so `--tag workflow --scope project` installs the project-registry skills tagged `workflow`. Resolved names are installed in sorted order and reported per-skill exactly as a name batch would be. - Names and filters are **mutually exclusive** — passing both is a validation error (exit 2), as is passing neither. -- A filter that matches no registered skill is an **error** (exit 1: `no registered skills match --tag workflow in user scope`), never a silent no-op. +- A filter that matches no registered skill is an **error** (exit 1: `no registered skills match --tag workflow in user scope`), never a silent no-op. Registry parse warnings (a skill directory whose `SKILL.md` could not be read — which may be exactly why a tag matched nothing) are included in that error, and printed to stderr when the filter does match. - `--enforce-budget` counts the resolved group. The response includes a top-level `capacity` block reporting the platform's installed-skill count after the operation, the threshold for that scope, and remaining headroom. @@ -333,6 +333,8 @@ skern skill uninstall --tag --platform # evict a tagged gro `--tag` / `--category` select a group the same way as on `install`: the filter resolves against the registry at `--scope`, then is narrowed to the skills actually installed on the platform. Tagged-but-not-installed skills are skipped, not reported as failures. If nothing in the group is installed the command errors (`no installed skills match --tag workflow on claude-code (user scope)`); if the tag matches nothing in the registry, the error says so instead. Names and filters are mutually exclusive. +Because the registry defines the group, a skill that was already removed from the registry (`skern skill remove`) is no longer reachable by tag — uninstall it by name. Retire a group by uninstalling it from platforms *before* removing it from the registry. + **Flags:** | Flag | Description | diff --git a/internal/cli/platform_test.go b/internal/cli/platform_test.go index f59ac99..f98d74a 100644 --- a/internal/cli/platform_test.go +++ b/internal/cli/platform_test.go @@ -11,6 +11,7 @@ import ( "github.com/devrimcavusoglu/skern/internal/output" "github.com/devrimcavusoglu/skern/internal/platform" "github.com/devrimcavusoglu/skern/internal/registry" + "github.com/devrimcavusoglu/skern/internal/skill" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -673,18 +674,62 @@ func TestSkillInstall_Filter_RespectsScope(t *testing.T) { func TestSkillInstall_Filter_EnforceBudgetCountsResolved(t *testing.T) { cc, _, _ := testRegistryWithDirs(t) - withTestDetector(t, cc, t.TempDir(), t.TempDir()) + home := t.TempDir() + withTestDetector(t, cc, home, t.TempDir()) setupTaggedSkills(t, cc) - // Two workflow skills resolve; a budget check must see 2, not 0 args. - // Under the threshold it proceeds normally. + // Fill the platform to threshold-1 with fake installed skills. Two + // workflow skills resolve from the filter; the budget check must count + // those 2 (not the 0 positional args) and refuse: 49 + 2 > 50. + threshold := skill.PlatformThreshold(skill.ScopeUser) + for i := 0; i < threshold-1; i++ { + dir := filepath.Join(home, ".claude", "skills", fmt.Sprintf("filler-%02d", i)) + require.NoError(t, os.MkdirAll(dir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "SKILL.md"), []byte("---\nname: x\ndescription: y\n---\nbody\n"), 0o644)) + } + + _, err := runCmd(t, cc, "skill", "install", "--tag", "workflow", "--platform", "claude-code", "--enforce-budget") + require.Error(t, err) + assert.Contains(t, err.Error(), "installing 2 more would exceed the threshold") + for _, n := range []string{"wf-plan", "wf-review"} { + _, statErr := os.Stat(filepath.Join(home, ".claude", "skills", n)) + assert.True(t, os.IsNotExist(statErr), "%s must not be installed when the budget refuses", n) + } + + // One filler fewer and the same group fits exactly at the threshold. + require.NoError(t, os.RemoveAll(filepath.Join(home, ".claude", "skills", "filler-00"))) out, err := runCmd(t, cc, "skill", "install", "--tag", "workflow", "--platform", "claude-code", "--enforce-budget", "--json") require.NoError(t, err) var result output.SkillInstallResult require.NoError(t, json.Unmarshal([]byte(out), &result)) require.Len(t, result.Skills, 2) require.NotNil(t, result.Capacity) - assert.Equal(t, 2, result.Capacity.Installed) + assert.Equal(t, threshold, result.Capacity.Installed) +} + +// A malformed SKILL.md in the registry is exactly why a tag may "match +// nothing"; the filter must surface registry parse warnings instead of +// swallowing them (as `skill list` does). +func TestSkillInstall_Filter_SurfacesParseWarnings(t *testing.T) { + cc, userDir, _ := testRegistryWithDirs(t) + withTestDetector(t, cc, t.TempDir(), t.TempDir()) + setupTaggedSkills(t, cc) + broken := filepath.Join(userDir, "broken-skill") + require.NoError(t, os.MkdirAll(broken, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(broken, "SKILL.md"), []byte("not frontmatter"), 0o644)) + + // Empty match: the warning rides along in the error. + _, err := runCmd(t, cc, "skill", "install", "--tag", "nope", "--platform", "claude-code") + require.Error(t, err) + assert.Contains(t, err.Error(), "no registered skills match --tag nope") + assert.Contains(t, err.Error(), "broken-skill") + + // Non-empty match: the warning is printed (to stderr) and the install proceeds. + out, err := runCmd(t, cc, "skill", "install", "--tag", "workflow", "--platform", "claude-code") + require.NoError(t, err) + assert.Contains(t, out, "could not be parsed") + assert.Contains(t, out, "broken-skill") + assert.Contains(t, out, `Installed "wf-plan"`) } func TestSkillUninstall_ByTag_OnlyInstalledMatches(t *testing.T) { diff --git a/internal/cli/skill_helpers.go b/internal/cli/skill_helpers.go index 5dfb2d0..59163a0 100644 --- a/internal/cli/skill_helpers.go +++ b/internal/cli/skill_helpers.go @@ -2,6 +2,7 @@ package cli import ( "fmt" + "io" "os" "path/filepath" "slices" @@ -332,18 +333,20 @@ func (f *skillFilter) matcher() (func(tags []string) bool, error) { } // resolveSkillsByFilter returns the names of registry skills in scope that -// satisfy the filter, sorted for deterministic batch order. Install and -// uninstall read the registry at --scope, so the filter resolves there too. -// An empty result is reported as an error so a group operation never turns -// into a silent no-op. -func resolveSkillsByFilter(reg *registry.Registry, scope skill.Scope, f *skillFilter) ([]string, error) { +// satisfy the filter, sorted for deterministic batch order, plus any parse +// warnings for skill directories that could not be read (a malformed +// SKILL.md can be exactly why a tag "matches nothing", so callers should +// surface them). Install and uninstall read the registry at --scope, so the +// filter resolves there too. An empty result is reported as an error so a +// group operation never turns into a silent no-op. +func resolveSkillsByFilter(reg *registry.Registry, scope skill.Scope, f *skillFilter) ([]string, []registry.ParseWarning, error) { match, err := f.matcher() if err != nil { - return nil, err + return nil, nil, err } - skills, _, err := reg.List(scope) + skills, warnings, err := reg.List(scope) if err != nil { - return nil, err + return nil, nil, err } var names []string for i := range skills { @@ -352,22 +355,37 @@ func resolveSkillsByFilter(reg *registry.Registry, scope skill.Scope, f *skillFi } } if len(names) == 0 { - return nil, fmt.Errorf("no registered skills match %s in %s scope (run 'skern skill list --scope %s' to see available skills)", f.describe(), scope, scope) + msg := fmt.Sprintf("no registered skills match %s in %s scope (run 'skern skill list --scope %s' to see available skills)", f.describe(), scope, scope) + if len(warnings) > 0 { + msg += ";" + strings.TrimSuffix(strings.TrimPrefix(formatParseWarnings(warnings), "\nWarning:"), "\n") + } + return nil, warnings, fmt.Errorf("%s", msg) } sort.Strings(names) - return names, nil + return names, warnings, nil } // resolveActionTargets turns positional names or an active filter into the // list of skills an install/uninstall batch operates on. Names and filters // are mutually exclusive: mixing them is a validation error, and so is -// passing neither. -func resolveActionTargets(reg *registry.Registry, scope skill.Scope, args []string, f *skillFilter) ([]string, error) { +// passing neither. newRegistry is only invoked when a filter is active, so +// the names path never opens the registry. Registry parse warnings from a +// filter resolution are written to errOut so they are visible, as they are +// in `skill list`. +func resolveActionTargets(newRegistry func() (*registry.Registry, error), scope skill.Scope, args []string, f *skillFilter, errOut io.Writer) ([]string, error) { if f.active() { if len(args) > 0 { return nil, &ValidationError{Message: fmt.Sprintf("skill names and %s are mutually exclusive; pass one or the other", f.describe())} } - return resolveSkillsByFilter(reg, scope, f) + reg, err := newRegistry() + if err != nil { + return nil, err + } + names, warnings, err := resolveSkillsByFilter(reg, scope, f) + if len(warnings) > 0 && err == nil { + fmt.Fprint(errOut, formatParseWarnings(warnings)) + } + return names, err } if len(args) == 0 { return nil, &ValidationError{Message: "requires at least one skill name, or a --tag/--category filter"} diff --git a/internal/cli/skill_install.go b/internal/cli/skill_install.go index e4f1dca..4af1a7b 100644 --- a/internal/cli/skill_install.go +++ b/internal/cli/skill_install.go @@ -6,6 +6,7 @@ import ( "github.com/devrimcavusoglu/skern/internal/output" "github.com/devrimcavusoglu/skern/internal/platform" + "github.com/devrimcavusoglu/skern/internal/registry" "github.com/spf13/cobra" ) @@ -72,7 +73,7 @@ installed-skill count would meet or exceed the per-platform threshold (see // Positional names, or the registry skills selected by --tag / // --category (validated and fail-fast either way). - names, err := resolveActionTargets(reg, scopeVal, args, &filter) + names, err := resolveActionTargets(func() (*registry.Registry, error) { return reg, nil }, scopeVal, args, &filter, cmd.ErrOrStderr()) if err != nil { return err } diff --git a/internal/cli/skill_uninstall.go b/internal/cli/skill_uninstall.go index bc44e2e..ce25c34 100644 --- a/internal/cli/skill_uninstall.go +++ b/internal/cli/skill_uninstall.go @@ -6,7 +6,6 @@ import ( "github.com/devrimcavusoglu/skern/internal/output" "github.com/devrimcavusoglu/skern/internal/platform" - "github.com/devrimcavusoglu/skern/internal/skill" "github.com/spf13/cobra" ) @@ -59,20 +58,17 @@ installed is an error rather than a silent no-op.`, return &ValidationError{Message: fmt.Sprintf("platform %q not recognized; valid platforms: %s", platformFlag, platformNamesList())} } - names := args + // Positional names, or the registry skills selected by --tag / + // --category. Registry membership defines the group; the + // platform's installed set then decides which of them there is + // anything to remove — tagged-but-not-installed skills are + // skipped, not reported as failures. The registry is only opened + // when a filter is active. + names, err := resolveActionTargets(ctx.NewRegistry, scopeVal, args, &filter, cmd.ErrOrStderr()) + if err != nil { + return err + } if filter.active() { - // Registry membership defines the group; the platform's - // installed set decides which of them there is anything to - // remove. Tagged-but-not-installed skills are skipped, not - // reported as failures. - reg, regErr := ctx.NewRegistry() - if regErr != nil { - return regErr - } - names, err = resolveActionTargets(reg, scopeVal, args, &filter) - if err != nil { - return err - } installed, listErr := p.InstalledSkills(scopeVal) if listErr != nil { return fmt.Errorf("listing installed skills on %s: %w", p.Name(), listErr) @@ -81,14 +77,6 @@ installed is an error rather than a silent no-op.`, if len(names) == 0 { return fmt.Errorf("no installed skills match %s on %s (%s scope)", filter.describe(), p.Name(), scopeVal) } - } else if len(args) == 0 { - return &ValidationError{Message: "requires at least one skill name, or a --tag/--category filter"} - } else { - for _, name := range args { - if err := skill.ValidateName(name); err != nil { - return &ValidationError{Message: err.Error()} - } - } } var entries []output.SkillActionEntry From 3c155ae82ac476850fea6ba369fe24fdf1d3f5d9 Mon Sep 17 00:00:00 2001 From: devrimcavusoglu Date: Wed, 19 Aug 2026 16:08:43 +0300 Subject: [PATCH 3/3] Check Fprint error return (lint) Co-Authored-By: Claude Fable 5 --- internal/cli/skill_helpers.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/cli/skill_helpers.go b/internal/cli/skill_helpers.go index 59163a0..dd0188b 100644 --- a/internal/cli/skill_helpers.go +++ b/internal/cli/skill_helpers.go @@ -383,7 +383,7 @@ func resolveActionTargets(newRegistry func() (*registry.Registry, error), scope } names, warnings, err := resolveSkillsByFilter(reg, scope, f) if len(warnings) > 0 && err == nil { - fmt.Fprint(errOut, formatParseWarnings(warnings)) + _, _ = fmt.Fprint(errOut, formatParseWarnings(warnings)) } return names, err }