Add basic SkillService implementation and API handlers

[JAORMX]

Summary

This PR delivers the service layer and API wiring needed to make the skills
endpoints functional, building on the SQLite SkillStore merged in #3801.

Why a sub-package? pkg/skills and pkg/storage have a circular import
(storage uses skills types, skills needs the store interface). The service
implementation lives in pkg/skills/skillsvc/ to break this cycle cleanly.

What each endpoint does now

Endpoint	Before	After
GET /api/v1beta/skills	501	Lists installed skills, optional ?scope= filter
POST /api/v1beta/skills/install	501	Creates a "pending" skill record (no OCI pull yet)
POST /api/v1beta/skills/uninstall	501	Deletes a skill by name+scope
GET /api/v1beta/skills/{name}	501	Returns skill info or {installed: false}
POST /api/v1beta/skills/validate	501	Still 501 (deferred)
POST /api/v1beta/skills/build	501	Still 501 (deferred)
POST /api/v1beta/skills/push	501	Still 501 (deferred)

Server lifecycle changes

createDefaultManagers() now opens a SkillStore via sqlite.NewDefaultSkillStore()
and passes it to skillsvc.New(). The store reference is tracked as an io.Closer
on the Server struct and closed in cleanup() — before unix socket removal, even
on crash paths. When callers inject their own service via WithSkillManager(), they
own the store lifecycle (documented in the method comment).

Service design decisions

• Install creates "pending" only — no OCI pull in this phase, that's Skill service implementation #3650
• Info returns 200 always — {installed: false} for missing skills, not 404
• Info hardcodes ScopeUser — documented in code comment; project-scoped lookup
  comes when InfoOptions gains a Scope field
• All name-accepting methods validate via ValidateSkillName before touching the store
• Scope defaults to ScopeUser via extracted defaultScope() helper

Handler patterns

Handlers follow the same pattern as groups.go: decode JSON → validate required
fields → call service → encode response. Error mapping uses httperr.WithCode()
for 4xx and lets ErrorHandler sanitize 5xx to generic messages.

Closes #3741

Large PR Justification

The PR is ~980 lines total but only ~200 lines are production code (across 3 modified
files and 1 new file). The remaining ~780 lines are test files:

• pkg/skills/skillsvc/skillsvc_test.go (367 lines) — service unit tests
• pkg/api/v1/skills_test.go (380 lines) — handler tests
• docs/server/ — auto-generated swagger docs

The production code cannot be split further — the service implementation, API handlers,
and server wiring are tightly coupled (handlers call the service, server creates the
service). Shipping them separately would leave broken 501 stubs between PRs.

Test plan

• 24 service unit tests — table-driven with MockSkillStore covering happy paths,
  validation errors, error propagation, scope defaulting, and version passthrough
• 24 handler tests — table-driven with MockSkillService and httptest round-trips
  covering JSON decoding, empty/malformed input, status code mapping (400/404/409/501),
  scope+version passthrough, and response format validation
• task lint-fix clean, task build compiles, race detector passes

🤖 Generated with Claude Code

[github-actions]

Large PR Detected

This PR exceeds 1000 lines of changes and requires justification before it can be reviewed.

How to unblock this PR:

Add a section to your PR description with the following format:

## Large PR Justification

[Explain why this PR must be large, such as:]
- Generated code that cannot be split
- Large refactoring that must be atomic
- Multiple related changes that would break if separated
- Migration or data transformation

Alternative:

Consider splitting this PR into smaller, focused changes (< 1000 lines each) for easier review and reduced risk.

See our Contributing Guidelines for more details.

---

This review will be automatically dismissed once you add the justification section.

[codecov]

Codecov Report

❌ Patch coverage is 86.91589% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 66.99%. Comparing base (e4ad673) to head (9e08f09).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/api/server.go	0.00%	10 Missing ⚠️
pkg/api/v1/skills.go	92.45%	2 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3804      +/-   ##
==========================================
+ Coverage   66.89%   66.99%   +0.09%     
==========================================
  Files         439      441       +2     
  Lines       43561    43689     +128     
==========================================
+ Hits        29142    29270     +128     
+ Misses      12166    12163       -3     
- Partials     2253     2256       +3     

☔ 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.

addressed

[ChrisJBurns]

I brought up some of the issues below in Slack but I'll summarise here for the wider audience.

This PR builds on top of an API contract that may need some tweaks. Currently, as it is, is a mix between REST and RPC like conventions which is inconsistent with the rest of ToolHives API.

I believe how other ToolHive resources do it:

• Groups: POST / (create), DELETE /{name} (delete), GET / (list), GET /{name} (get)
• Workloads: POST / (create), DELETE /{name} (delete), GET / (list), GET /{name} (get)

How skills does it:

• POST /api/v1beta/skills/install (create) - should be POST /api/v1beta/skills/ (although it was mentioned the issue on long running requests)
• POST /api/v1beta/skills/uninstall (delete) - should be DELETE /api/v1beta/skills/{name} or DELETE /api/v1beta/skills/{name}?scope=X
• GET /api/v1beta/skills/ (list) - fine
• GET /api/v1beta/skills/{name} (get) - fine

Install and uninstall seems like CREATE and DELETE with verb-named endpoints. validate, build and push aren't really REST (not CRUD on the skills resources), however given its probably more complexity and hassle to support RPC for those, maybe they can be done as sub-resources on the skills resources? Something like /skills/{id}/builds etc? Am aware we won't be able to make it perfect

There is also a route conflict risk between the GET /api/v1beta/skills{name} and GET /api/v1beta/skills/install if someone names a skill "install", "uninstall", "validate", "build" or "push". Doing some googling, Chi may resolve this by preferring statics routes over parametersed ones, but its a collision none the less in standard REST patterns.

Apologies for the late response on the skills stuff in general, I've not taken a look at any of it until now due to other priorities.

[aponcedeleonch]

A few nits after re-reviewing with the latest commit applied.