Default to :latest tag for OCI skill install references#4430
Conversation
When a user runs `thv skill install ghcr.io/org/skill` without a tag, go-containerregistry defaults to "latest" internally but String() omits it. The raw user input was passed to oras-go's registry.Pull which requires an explicit tag, causing a confusing "Bad Gateway" error. Build the qualified OCI reference from the parsed nameref.Reference, appending `:latest` only when the user omitted a tag. Also change the pull error status from 502 to 400 since a bad reference is a client input error, not an upstream gateway failure. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4430 +/- ##
==========================================
+ Coverage 69.50% 69.54% +0.03%
==========================================
Files 486 486
Lines 50017 50024 +7
==========================================
+ Hits 34766 34790 +24
+ Misses 12570 12549 -21
- Partials 2681 2685 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
aponcedeleonch
left a comment
There was a problem hiding this comment.
LGTM! One minor note: according to the go-containerregistry docs, ref.String() already returns the fully-qualified reference (including the default :latest tag when none is specified), so qualifiedOCIRef could potentially be replaced by just using ref.String() from the already-parsed reference. That said, the strings.Contains(s, ":") check could be a false positive for references with a registry port (e.g. registry.example.com:5000/my-skill). If you've tested that case and it works correctly, all good!
|
I actually tried using the String method and it seems the docs were off. They did kot contain the tag. |
Summary
thv skill install ghcr.io/org/skill(without a tag) produced a confusingError: failed to install skill: Bad Gatewaybecause the raw user input was passed to oras-go which requires an explicit tag or digest, while go-containerregistry had already defaulted tolatestinternally but that parsed reference was ignored.nameref.Reference, appending:latestonly when the user omitted a tag. Explicit tags and digests pass through unchanged.Type of change
Test plan
task test)task lint-fix)Does this introduce a user-facing change?
thv skill install ghcr.io/org/skillnow defaults to the:latesttag instead of failing with "Bad Gateway". When OCI pull errors do occur, the error message is now surfaced to the user instead of being hidden behind a generic status text.Generated with Claude Code