Skip to content

Classify packager errors as 400 in skill build handler #5076

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
JAORMX merged 2 commits into from
Apr 28, 2026
+96 −10
Merged

Classify packager errors as 400 in skill build handler #5076

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ddb1329
Bump github.com/stacklok/toolhive-core to v0.0.17
samuv Apr 27, 2026
aabec78
Classify packager errors as 400 in skill build handler
samuv Apr 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ require (
github.com/shirou/gopsutil/v4 v4.26.2
github.com/spf13/viper v1.21.0
github.com/stacklok/toolhive-catalog v0.20260423.0
github.com/stacklok/toolhive-core v0.0.16
github.com/stacklok/toolhive-core v0.0.17
github.com/stretchr/testify v1.11.1
github.com/swaggo/swag/v2 v2.0.0-rc5
github.com/tailscale/hujson v0.0.0-20260302212456-ecc657c15afd
Expand Down Expand Up @@ -307,7 +307,7 @@ require (
modernc.org/libc v1.70.0 // indirect
modernc.org/mathutil v1.7.1 // indirect
modernc.org/memory v1.11.0 // indirect
oras.land/oras-go/v2 v2.6.0 // indirect
oras.land/oras-go/v2 v2.6.0
sigs.k8s.io/json v0.0.0-20250730193827-2d320260d730 // indirect
sigs.k8s.io/randfill v1.0.0 // indirect
sigs.k8s.io/structured-merge-diff/v6 v6.3.2-0.20260122202528-d9cc6641c482 // indirect
Expand Down
4 changes: 2 additions & 2 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -799,8 +799,8 @@ github.com/spf13/viper v1.21.0 h1:x5S+0EU27Lbphp4UKm1C+1oQO+rKx36vfCoaVebLFSU=
github.com/spf13/viper v1.21.0/go.mod h1:P0lhsswPGWD/1lZJ9ny3fYnVqxiegrlNrEmgLjbTCAY=
github.com/stacklok/toolhive-catalog v0.20260423.0 h1:W3qbLqbLhPqclYGrxYtm5jDYurjcEMRAhr7k/qfxm4s=
github.com/stacklok/toolhive-catalog v0.20260423.0/go.mod h1:bfIGimrf8SJJGT3+WcJmFKJ3JB2vz9RPykn2r6nKVNw=
github.com/stacklok/toolhive-core v0.0.16 h1:Td/o/zrO3Pbr6tkx4r0k8Bqn9DySlwgah2vknE8tj5A=
github.com/stacklok/toolhive-core v0.0.16/go.mod h1:iHp39rCHZxXzU5FmXKK9aoH48NE+K0ob+MtdTS9CvAk=
github.com/stacklok/toolhive-core v0.0.17 h1:yGKXntWyw5ZO5GMxfSHi9doJhSXA8w5ORSXWveJ3OGc=
github.com/stacklok/toolhive-core v0.0.17/go.mod h1:o/zVzleR/xNCNXdTwNx8A41hApu0GZsHZS42qcXYUr8=
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
github.com/stretchr/objx v0.2.0/go.mod h1:qt09Ya8vawLte6SNmTgCsAVtYtaKzEcn8ATUoHMkEqE=
Expand Down
12 changes: 12 additions & 0 deletions pkg/skills/skillsvc/build.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,18 @@ func (s *service) Build(ctx context.Context, opts skills.BuildOptions) (*skills.
}
result, err := s.packager.Package(ctx, opts.Path, ociskills.DefaultPackageOptions())
if err != nil {
// User-input failures (missing SKILL.md, bad frontmatter, symlinks,
// size/count limits, unreadable directory) are surfaced as 400 with
// the packager's message intact. Anything else is a real 500.
switch {
case errors.Is(err, ociskills.ErrSkillMDMissing),
errors.Is(err, ociskills.ErrInvalidFrontmatter),
errors.Is(err, ociskills.ErrInvalidSkillDir),
errors.Is(err, ociskills.ErrInvalidSkillFile),
errors.Is(err, ociskills.ErrTooManyFiles),
errors.Is(err, ociskills.ErrSkillTooLarge):
return nil, httperr.WithCode(err, http.StatusBadRequest)
}
return nil, fmt.Errorf("packaging skill: %w", err)
}

Expand Down
86 changes: 80 additions & 6 deletions pkg/skills/skillsvc/build_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,12 +83,13 @@ func TestBuild(t *testing.T) {
t.Parallel()

tests := []struct {
name string
opts skills.BuildOptions
setup func(*gomock.Controller) (ociskills.SkillPackager, *ociskills.Store)
wantCode int
wantRef string
wantErr string
name string
opts skills.BuildOptions
setup func(*gomock.Controller) (ociskills.SkillPackager, *ociskills.Store)
wantCode int
wantRef string
wantErr string
wantErrIs error
}{
{
name: "nil packager returns 500",
Expand Down Expand Up @@ -158,6 +159,76 @@ func TestBuild(t *testing.T) {
},
wantErr: "packaging skill",
},
{
name: "missing SKILL.md returns 400",
opts: skills.BuildOptions{Path: "/some/dir"},
setup: func(ctrl *gomock.Controller) (ociskills.SkillPackager, *ociskills.Store) {
ociStore, err := ociskills.NewStore(t.TempDir())
require.NoError(t, err)
p := ocimocks.NewMockSkillPackager(ctrl)
p.EXPECT().Package(gomock.Any(), "/some/dir", gomock.Any()).
Return(nil, fmt.Errorf("reading skill directory: %w", ociskills.ErrSkillMDMissing))
return p, ociStore
},
wantCode: http.StatusBadRequest,
wantErrIs: ociskills.ErrSkillMDMissing,
},
{
name: "invalid frontmatter returns 400",
opts: skills.BuildOptions{Path: "/some/dir"},
setup: func(ctrl *gomock.Controller) (ociskills.SkillPackager, *ociskills.Store) {
ociStore, err := ociskills.NewStore(t.TempDir())
require.NoError(t, err)
p := ocimocks.NewMockSkillPackager(ctrl)
p.EXPECT().Package(gomock.Any(), "/some/dir", gomock.Any()).
Return(nil, fmt.Errorf("parsing frontmatter YAML: %w", ociskills.ErrInvalidFrontmatter))
return p, ociStore
},
wantCode: http.StatusBadRequest,
wantErrIs: ociskills.ErrInvalidFrontmatter,
},
{
name: "empty name in frontmatter returns 400",
opts: skills.BuildOptions{Path: "/some/dir"},
setup: func(ctrl *gomock.Controller) (ociskills.SkillPackager, *ociskills.Store) {
ociStore, err := ociskills.NewStore(t.TempDir())
require.NoError(t, err)
p := ocimocks.NewMockSkillPackager(ctrl)
p.EXPECT().Package(gomock.Any(), "/some/dir", gomock.Any()).
Return(nil, fmt.Errorf("skill name is required in SKILL.md frontmatter: %w", ociskills.ErrInvalidFrontmatter))
return p, ociStore
},
wantCode: http.StatusBadRequest,
wantErrIs: ociskills.ErrInvalidFrontmatter,
},
{
name: "symlink in skill dir returns 400",
opts: skills.BuildOptions{Path: "/some/dir"},
setup: func(ctrl *gomock.Controller) (ociskills.SkillPackager, *ociskills.Store) {
ociStore, err := ociskills.NewStore(t.TempDir())
require.NoError(t, err)
p := ocimocks.NewMockSkillPackager(ctrl)
p.EXPECT().Package(gomock.Any(), "/some/dir", gomock.Any()).
Return(nil, fmt.Errorf("symlinks not allowed in skill directory: sub/link: %w", ociskills.ErrInvalidSkillFile))
return p, ociStore
},
wantCode: http.StatusBadRequest,
wantErrIs: ociskills.ErrInvalidSkillFile,
},
{
name: "oversized dir returns 400",
opts: skills.BuildOptions{Path: "/some/dir"},
setup: func(ctrl *gomock.Controller) (ociskills.SkillPackager, *ociskills.Store) {
ociStore, err := ociskills.NewStore(t.TempDir())
require.NoError(t, err)
p := ocimocks.NewMockSkillPackager(ctrl)
p.EXPECT().Package(gomock.Any(), "/some/dir", gomock.Any()).
Return(nil, fmt.Errorf("skill directory exceeds maximum total size: %w", ociskills.ErrSkillTooLarge))
return p, ociStore
},
wantCode: http.StatusBadRequest,
wantErrIs: ociskills.ErrSkillTooLarge,
},
{
name: "successful build with explicit tag",
opts: skills.BuildOptions{Path: "/some/dir", Tag: "v1.0.0"},
Expand Down Expand Up @@ -243,6 +314,9 @@ func TestBuild(t *testing.T) {
if tt.wantCode != 0 {
require.Error(t, err)
assert.Equal(t, tt.wantCode, httperr.Code(err))
if tt.wantErrIs != nil {
assert.ErrorIs(t, err, tt.wantErrIs)
}
return
}
if tt.wantErr != "" {
Expand Down
Loading