Classify packager errors as 400 in skill build handler

[samuv]

Summary

• Follow-up on the same Sentry-noise / unhelpful-500 pattern fixed for the content/install paths in Classify OCI pull errors and fall back to git for skills content API #5014 and Return 502 Bad Gateway when OCI skill pull fails #4956 — now applied to POST /api/v1beta/skills/build. A user pointing thv skill build at a directory without a SKILL.md, or with malformed frontmatter, currently gets a bare 500 Internal Server Error and a *fmt.wrapError: packaging skill: reading skill directory: SKILL.md not found in skill directory exception in Sentry. That's plainly bad input, not a server bug.
• toolhive-core v0.0.17 (release, stacklok/toolhive-core#95) introduces six sentinel errors in oci/skills so the packager's user-input failures can be classified with errors.Is instead of matching error strings.
• Bump the dependency and wrap s.packager.Package with an errors.Is switch over those sentinels, mapping each one to 400 Bad Request with the packager's message preserved (e.g. SKILL.md missing, invalid SKILL.md frontmatter, skill directory too large). Anything else stays a real 500.
• Two independent commits so the dependency bump and the logic change can be reviewed / reverted in isolation.

Why this and not a ValidateSkillDir pre-flight

An earlier draft pre-flighted the existing skills.ValidateSkillDir validator before invoking the packager. That worked for the most common case (missing SKILL.md) but had two problems:

• Duplicate filesystem walk on every successful build (validator + packager both walk the directory).
• Validator drift: the toolhive-side validator does not enforce the packager's maxSkillFiles (1,000) and maxSkillTotalSize (100 MB) limits, nor the per-file validateSkillFile checks (symlinks, non-regular files). Those failure modes still fell through to 500.

The sentinel switch covers every packager failure mode in one place, with no double-validation. The packager's existing error messages are preserved verbatim, so the client still sees actionable detail in the 400 body.

Mapping

Sentinel (ociskills.*)	Cause	Status
ErrSkillMDMissing	SKILL.md not present in the directory	400
ErrInvalidFrontmatter	Malformed YAML, missing delimiter, oversized, empty name	400
ErrInvalidSkillDir	Directory missing / not-a-dir / contains traversal	400
ErrInvalidSkillFile	Symlink or non-regular file inside the dir	400
ErrTooManyFiles	Exceeds maxSkillFiles (1,000)	400
ErrSkillTooLarge	Exceeds maxSkillTotalSize (100 MB)	400
anything else	Unclassified packager failure	500

The pre-existing validateLocalPath checks (null bytes, traversal, relative paths) stay in place — they're cheap, run before any I/O, and double as a guard against packager bugs.

Type of change

• Bug fix (HTTP status semantics + observability of build failures)

Test plan

• Unit tests (go test ./pkg/skills/skillsvc/... -count=1)
• Linting (task lint-fix — 0 issues)
• Wider regression sweep (go test ./pkg/skills/... ./pkg/api/...)
• Five new TestBuild cases assert both httperr.Code(err) == 400 and errors.Is(err, ociskills.ErrXxx) for each sentinel: missing SKILL.md, invalid frontmatter, empty name in frontmatter, symlink in skill dir, oversized dir.
• Existing cases (nil packager returns 500, empty path returns 400, relative path returns 400, path traversal returns 400, invalid tag returns 400, packager error propagates, three success cases, invalid fallback config name returns 400) all still pass — none of them flow through the new switch.

Changes

File	Change
go.mod, go.sum	Bump github.com/stacklok/toolhive-core v0.0.16 → v0.0.17
pkg/skills/skillsvc/build.go	errors.Is switch over the six packager sentinels; each → httperr.WithCode(err, http.StatusBadRequest)
pkg/skills/skillsvc/build_test.go	Add wantErrIs error field + five new sentinel cases; runner asserts assert.ErrorIs when set

The OpenAPI annotation on buildSkill (pkg/api/v1/skills.go) already advertises @Failure 400 ..., and docs/server/{docs.go,swagger.json,swagger.yaml} already reflect it — no spec regeneration needed.

Does this introduce a user-facing change?

Yes — clients calling POST /api/v1beta/skills/build on a directory the packager rejects (missing SKILL.md, malformed frontmatter, symlinks, size/count limit) now receive 400 Bad Request with the packager's message in the body, instead of 500 Internal Server Error with an empty body. Successful builds and genuine server failures are unchanged.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 69.77%. Comparing base (68d29fb) to head (aabec78).
⚠️ Report is 8 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5076      +/-   ##
==========================================
- Coverage   69.81%   69.77%   -0.04%     
==========================================
  Files         562      564       +2     
  Lines       56647    56652       +5     
==========================================
- Hits        39548    39531      -17     
- Misses      14063    14092      +29     
+ Partials     3036     3029       -7     

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