Skip to content

Default skills to the "default" group on install#4201

Merged
amirejaz merged 1 commit intomainfrom
jaosorior/skill-default-group
Mar 18, 2026
Merged

Default skills to the "default" group on install#4201
amirejaz merged 1 commit intomainfrom
jaosorior/skill-default-group

Conversation

@JAORMX
Copy link
Copy Markdown
Collaborator

@JAORMX JAORMX commented Mar 18, 2026

Summary

Workloads already default to the "default" group when no group is specified
(pkg/api/v1/workload_service.go), but skills did not. When
thv skill install <name> was called without --group, the group field stayed
empty and registerSkillInGroup was a no-op, meaning skills were never added to
any group unless the user explicitly passed --group.

  • Add the same defaulting logic in the skill service Install method so skills
    are automatically registered in the "default" group, matching workload behavior
  • Update the existing test case to verify default group registration occurs

Type of change

  • Bug fix

Test plan

  • Unit tests (task test)
  • Linting (task lint-fix)

Does this introduce a user-facing change?

Skills installed without --group are now automatically added to the "default"
group, matching the existing behavior for workloads. Previously they were not
added to any group.

Generated with Claude Code

Workloads already default to the "default" group when no group is
specified (pkg/api/v1/workload_service.go), but skills did not. When
`thv skill install <name>` was called without --group, the group field
stayed empty and registerSkillInGroup was a no-op, meaning skills were
never added to any group unless explicitly requested.

Add the same defaulting logic in the skill service Install method so
that skills are automatically registered in the "default" group,
matching workload behavior.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions github-actions bot added the size/XS Extra small PR: < 100 lines changed label Mar 18, 2026
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 18, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 68.92%. Comparing base (837906c) to head (d2feca8).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4201      +/-   ##
==========================================
- Coverage   69.04%   68.92%   -0.13%     
==========================================
  Files         468      468              
  Lines       47003    47032      +29     
==========================================
- Hits        32453    32415      -38     
- Misses      11974    11977       +3     
- Partials     2576     2640      +64     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Contributor

@amirejaz amirejaz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the skills are being used by folks with thv we need to have migration moving existing non-group skills to default group.

@JAORMX
Copy link
Copy Markdown
Collaborator Author

JAORMX commented Mar 18, 2026

@amirejaz skills are not yet exposed. All commands are hidden and we haven't been promoting them yet.

@amirejaz amirejaz merged commit 40f3dd6 into main Mar 18, 2026
43 checks passed
@amirejaz amirejaz deleted the jaosorior/skill-default-group branch March 18, 2026 13:59
Sanskarzz pushed a commit to Sanskarzz/toolhive that referenced this pull request Mar 23, 2026
Workloads already default to the "default" group when no group is
specified (pkg/api/v1/workload_service.go), but skills did not. When
`thv skill install <name>` was called without --group, the group field
stayed empty and registerSkillInGroup was a no-op, meaning skills were
never added to any group unless explicitly requested.

Add the same defaulting logic in the skill service Install method so
that skills are automatically registered in the "default" group,
matching workload behavior.

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/XS Extra small PR: < 100 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants