Skip to content

Commit 7b8c53c

Browse files
committed
Follow up fixes
Signed-off-by: Radoslav Dimitrov <radoslav@stacklok.com>
1 parent 2d2650e commit 7b8c53c

10 files changed

Lines changed: 47 additions & 119 deletions

File tree

pkg/api/server.go

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@ import (
3939
"github.com/stacklok/toolhive/pkg/container/runtime"
4040
"github.com/stacklok/toolhive/pkg/groups"
4141
"github.com/stacklok/toolhive/pkg/recovery"
42-
"github.com/stacklok/toolhive/pkg/registry"
4342
"github.com/stacklok/toolhive/pkg/skills"
4443
"github.com/stacklok/toolhive/pkg/skills/skillsvc"
4544
"github.com/stacklok/toolhive/pkg/storage/sqlite"
@@ -252,29 +251,20 @@ func (b *ServerBuilder) createDefaultManagers(ctx context.Context) error {
252251
_ = store.Close()
253252
return fmt.Errorf("failed to create OCI skill store: %w", ociErr)
254253
}
255-
ociRegistry, regErr := ociskills.NewRegistry()
254+
registry, regErr := ociskills.NewRegistry()
256255
if regErr != nil {
257256
_ = store.Close()
258257
// ociStore is directory-backed with no open handles; no cleanup needed.
259258
return fmt.Errorf("failed to create OCI registry client: %w", regErr)
260259
}
261260
packager := ociskills.NewPackager(ociStore)
262261

263-
// Get registry provider for skill discovery (best-effort)
264-
regProvider, regProviderErr := registry.GetDefaultProvider()
265-
var regOpts []skillsvc.Option
266-
if regProviderErr == nil {
267-
regOpts = append(regOpts, skillsvc.WithRegistryProvider(regProvider))
268-
}
269-
270262
b.skillManager = skillsvc.New(store,
271-
append([]skillsvc.Option{
272-
skillsvc.WithPathResolver(&clientPathAdapter{cm: cm}),
273-
skillsvc.WithOCIStore(ociStore),
274-
skillsvc.WithPackager(packager),
275-
skillsvc.WithRegistryClient(ociRegistry),
276-
skillsvc.WithGroupManager(b.groupManager),
277-
}, regOpts...)...,
263+
skillsvc.WithPathResolver(&clientPathAdapter{cm: cm}),
264+
skillsvc.WithOCIStore(ociStore),
265+
skillsvc.WithPackager(packager),
266+
skillsvc.WithRegistryClient(registry),
267+
skillsvc.WithGroupManager(b.groupManager),
278268
)
279269
}
280270

pkg/api/v1/skills.go

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ func SkillsRouter(skillService skills.SkillService) http.Handler {
2828

2929
r := chi.NewRouter()
3030
r.Get("/", apierrors.ErrorHandler(routes.listSkills))
31-
r.Get("/available", apierrors.ErrorHandler(routes.listAvailableSkills))
3231
r.Post("/", apierrors.ErrorHandler(routes.installSkill))
3332
r.Delete("/{name}", apierrors.ErrorHandler(routes.uninstallSkill))
3433
r.Get("/{name}", apierrors.ErrorHandler(routes.getSkillInfo))
@@ -39,25 +38,6 @@ func SkillsRouter(skillService skills.SkillService) http.Handler {
3938
return r
4039
}
4140

42-
// listAvailableSkills returns skills available from the registry.
43-
//
44-
// @Summary List available skills
45-
// @Description Get a list of skills available from the registry
46-
// @Tags skills
47-
// @Produce json
48-
// @Success 200 {object} availableSkillsResponse
49-
// @Failure 500 {string} string "Internal Server Error"
50-
// @Router /api/v1beta/skills/available [get]
51-
func (s *SkillsRoutes) listAvailableSkills(w http.ResponseWriter, r *http.Request) error {
52-
result, err := s.skillService.ListAvailable(r.Context())
53-
if err != nil {
54-
return err
55-
}
56-
57-
w.Header().Set("Content-Type", "application/json")
58-
return json.NewEncoder(w).Encode(availableSkillsResponse{Skills: result})
59-
}
60-
6141
// listSkills returns a list of installed skills.
6242
//
6343
// @Summary List all installed skills

pkg/api/v1/skills_types.go

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,7 @@
33

44
package v1
55

6-
import (
7-
types "github.com/stacklok/toolhive-core/registry/types"
8-
"github.com/stacklok/toolhive/pkg/skills"
9-
)
6+
import "github.com/stacklok/toolhive/pkg/skills"
107

118
// skillListResponse represents the response for listing skills.
129
//
@@ -69,11 +66,3 @@ type pushSkillRequest struct {
6966
// OCI reference to push
7067
Reference string `json:"reference"`
7168
}
72-
73-
// availableSkillsResponse represents the response for listing available skills from the registry.
74-
//
75-
// @Description Response containing skills available from the registry
76-
type availableSkillsResponse struct {
77-
// List of available skills from the registry
78-
Skills []types.Skill `json:"skills"`
79-
}

pkg/registry/provider_local.go

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -105,10 +105,23 @@ func (p *LocalRegistryProvider) setSkills(skills []types.Skill) {
105105
}
106106

107107
// ListAvailableSkills returns skills discovered from the upstream registry data.
108+
// Triggers a registry load if skills haven't been populated yet.
108109
func (p *LocalRegistryProvider) ListAvailableSkills() ([]types.Skill, error) {
109110
p.skillsMu.RLock()
110-
defer p.skillsMu.RUnlock()
111-
return p.skills, nil
111+
skills := p.skills
112+
p.skillsMu.RUnlock()
113+
114+
if skills == nil {
115+
// Skills are populated as a side effect of GetRegistry
116+
if _, err := p.GetRegistry(); err != nil {
117+
return nil, err
118+
}
119+
p.skillsMu.RLock()
120+
skills = p.skills
121+
p.skillsMu.RUnlock()
122+
}
123+
124+
return skills, nil
112125
}
113126

114127
// parseRegistryData parses JSON data into a Registry struct

pkg/registry/provider_remote.go

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -175,10 +175,23 @@ func (p *RemoteRegistryProvider) GetRegistry() (*types.Registry, error) {
175175
}
176176

177177
// ListAvailableSkills returns skills discovered from the remote registry data.
178+
// Triggers a registry load if skills haven't been populated yet.
178179
func (p *RemoteRegistryProvider) ListAvailableSkills() ([]types.Skill, error) {
179180
p.skillsMu.RLock()
180-
defer p.skillsMu.RUnlock()
181-
return p.skills, nil
181+
skills := p.skills
182+
p.skillsMu.RUnlock()
183+
184+
if skills == nil {
185+
// Skills are populated as a side effect of GetRegistry
186+
if _, err := p.GetRegistry(); err != nil {
187+
return nil, err
188+
}
189+
p.skillsMu.RLock()
190+
skills = p.skills
191+
p.skillsMu.RUnlock()
192+
}
193+
194+
return skills, nil
182195
}
183196

184197
func (p *RemoteRegistryProvider) setSkills(skills []types.Skill) {

pkg/skills/client/client.go

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ import (
1818

1919
"github.com/stacklok/toolhive-core/env"
2020
"github.com/stacklok/toolhive-core/httperr"
21-
types "github.com/stacklok/toolhive-core/registry/types"
2221
"github.com/stacklok/toolhive/pkg/skills"
2322
)
2423

@@ -254,15 +253,6 @@ func (c *Client) doJSONRequest(
254253
return nil
255254
}
256255

257-
// ListAvailable returns skills available from the registry via the API.
258-
func (c *Client) ListAvailable(ctx context.Context) ([]types.Skill, error) {
259-
var resp availableSkillsResponse
260-
if err := c.doJSONRequest(ctx, http.MethodGet, "/available", nil, nil, &resp); err != nil {
261-
return nil, err
262-
}
263-
return resp.Skills, nil
264-
}
265-
266256
// handleErrorResponse reads the response body and returns an *httperr.CodedError.
267257
func handleErrorResponse(resp *http.Response) error {
268258
body, err := io.ReadAll(io.LimitReader(resp.Body, maxErrorBodySize))

pkg/skills/client/dto.go

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,7 @@
33

44
package client
55

6-
import (
7-
types "github.com/stacklok/toolhive-core/registry/types"
8-
"github.com/stacklok/toolhive/pkg/skills"
9-
)
6+
import "github.com/stacklok/toolhive/pkg/skills"
107

118
// --- request/response dto (mirror pkg/api/v1/skills_types.go) ---
129

@@ -39,7 +36,3 @@ type listResponse struct {
3936
type installResponse struct {
4037
Skill skills.InstalledSkill `json:"skill"`
4138
}
42-
43-
type availableSkillsResponse struct {
44-
Skills []types.Skill `json:"skills"`
45-
}

pkg/skills/mocks/mock_service.go

Lines changed: 0 additions & 16 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/skills/service.go

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,7 @@
33

44
package skills
55

6-
import (
7-
"context"
8-
9-
types "github.com/stacklok/toolhive-core/registry/types"
10-
)
6+
import "context"
117

128
//go:generate mockgen -destination=mocks/mock_service.go -package=mocks -source=service.go SkillService
139

@@ -27,6 +23,4 @@ type SkillService interface {
2723
Build(ctx context.Context, opts BuildOptions) (*BuildResult, error)
2824
// Push pushes a built skill artifact to a remote registry.
2925
Push(ctx context.Context, opts PushOptions) error
30-
// ListAvailable returns skills available from the registry.
31-
ListAvailable(ctx context.Context) ([]types.Skill, error)
3226
}

pkg/skills/skillsvc/skillsvc.go

Lines changed: 8 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,7 @@ import (
2323

2424
"github.com/stacklok/toolhive-core/httperr"
2525
ociskills "github.com/stacklok/toolhive-core/oci/skills"
26-
types "github.com/stacklok/toolhive-core/registry/types"
2726
"github.com/stacklok/toolhive/pkg/groups"
28-
"github.com/stacklok/toolhive/pkg/registry"
2927
"github.com/stacklok/toolhive/pkg/skills"
3028
"github.com/stacklok/toolhive/pkg/storage"
3129
)
@@ -75,13 +73,6 @@ func WithGroupManager(mgr groups.Manager) Option {
7573
}
7674
}
7775

78-
// WithRegistryProvider sets the registry provider for discovering available skills.
79-
func WithRegistryProvider(p registry.Provider) Option {
80-
return func(s *service) {
81-
s.registryProvider = p
82-
}
83-
}
84-
8576
// skillLock provides per-skill mutual exclusion keyed by scope/name/projectRoot.
8677
// Entries are never evicted. This is acceptable because the number of distinct
8778
// skills on a single machine is expected to remain small (< 1000).
@@ -110,15 +101,14 @@ func (sl *skillLock) lock(name string, scope skills.Scope, projectRoot string) f
110101

111102
// service is the default implementation of skills.SkillService.
112103
type service struct {
113-
locks skillLock
114-
store storage.SkillStore
115-
groupManager groups.Manager
116-
pathResolver skills.PathResolver
117-
installer skills.Installer
118-
ociStore *ociskills.Store
119-
packager ociskills.SkillPackager
120-
registry ociskills.RegistryClient
121-
registryProvider registry.Provider
104+
locks skillLock
105+
store storage.SkillStore
106+
groupManager groups.Manager
107+
pathResolver skills.PathResolver
108+
installer skills.Installer
109+
ociStore *ociskills.Store
110+
packager ociskills.SkillPackager
111+
registry ociskills.RegistryClient
122112
}
123113

124114
// New creates a new SkillService backed by the given store.
@@ -183,14 +173,6 @@ func (s *service) List(ctx context.Context, opts skills.ListOptions) ([]skills.I
183173
return filtered, nil
184174
}
185175

186-
// ListAvailable returns skills available from the registry provider.
187-
func (s *service) ListAvailable(_ context.Context) ([]types.Skill, error) {
188-
if s.registryProvider == nil {
189-
return nil, nil
190-
}
191-
return s.registryProvider.ListAvailableSkills()
192-
}
193-
194176
// Install installs a skill. When the Name field contains an OCI reference
195177
// (detected by the presence of '/', ':', or '@'), the artifact is pulled from
196178
// the registry and extracted. When LayerData is provided, the skill is extracted

0 commit comments

Comments
 (0)