XDE-85: registry compiled-core client (JSON, semver, read bearer) - #1
Conversation
- Parse GET /skills/{name} versions as map (yanked, semver keys)
- Resolve unpinned installs to highest non-yanked semver
- Send RegistryReadBearer on API GET and archive fetch when set
- Add docs/REGISTRY_CLIENT_CONTRACT.md; depend on golang.org/x/mod/semver
Co-Authored-By: Paperclip <noreply@paperclip.ing>
Made-with: Cursor
There was a problem hiding this comment.
Pull request overview
Updates the Go registry client contract and implementation to match the canonical compiled-core registry API, including the new GET /skills/{name} JSON shape, “latest” resolution behavior, and optional read bearer authentication for registry/archives.
Changes:
- Updates
SkillDetailto useversionsas a map keyed by version withyanked, plus timestamp fields. - Changes unpinned
ResolveInstallTargetto select the highest non-yanked semver. - Adds
RegistryReadBearerand applies it to registry GET requests and archive downloads; adds a client contract doc.
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| types.go | Aligns registry response types with canonical compiled-core shapes (versions map, yanked, timestamps). |
| resolve.go | Implements highest-non-yanked semver selection for unpinned installs using x/mod/semver. |
| resolve_integration_test.go | Updates integration tests to the new SkillDetail.versions map shape and new “latest” behavior. |
| client.go | Adds optional Authorization: Bearer <read> header to registry JSON GET requests. |
| config.go | Introduces RegistryReadBearer() with read-token override and write-token fallback. |
| download.go | Adds optional read bearer header to archive download HTTP requests. |
| download_test.go | Updates tests for new SkillDetail shape; adds coverage for read bearer propagation and JSON decoding. |
| registry_errors.go | Updates 401 hint text to reference read-token behavior and env vars. |
| docs/REGISTRY_CLIENT_CONTRACT.md | Documents the expected HTTP/JSON contract and env var behavior for integrators. |
| README.md | Updates public docs for new “latest” resolution and read-bearer behavior; links to contract doc. |
| CHANGELOG.md | Adds 0.1.4 entry documenting breaking JSON contract change and new behaviors. |
| go.mod | Adds golang.org/x/mod dependency and introduces go patch + toolchain directives. |
| go.sum | Records checksums for the new golang.org/x/mod dependency. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if t := RegistryReadBearer(); t != "" { | ||
| req.Header.Set("Authorization", "Bearer "+t) | ||
| } |
There was a problem hiding this comment.
This unconditionally adds the registry bearer to whatever absolute archive_url is returned. If archive_url can point to a different host (e.g., CDN/S3 presigned URLs), this can leak read/write tokens cross-origin. Consider only attaching Authorization when the archive URL host matches the registry host (or when an explicit allowlist/opt-in env var is set).
| var keys []string | ||
| for v, info := range d.Versions { | ||
| v = strings.TrimSpace(v) | ||
| if v == "" || info.Yanked { | ||
| continue | ||
| } | ||
| if v.Version != "" { | ||
| return v.Version | ||
| } | ||
| keys = append(keys, v) | ||
| } | ||
| return "" | ||
| if len(keys) == 0 { | ||
| return "" | ||
| } | ||
| sort.Slice(keys, func(i, j int) bool { | ||
| return semver.Compare(canonicalSemver(keys[i]), canonicalSemver(keys[j])) < 0 | ||
| }) | ||
| return keys[len(keys)-1] |
There was a problem hiding this comment.
Selecting the highest semver currently sorts all version keys (O(n log n) + alloc). Since you only need the max, this can be done in a single pass by tracking the best version with semver.Compare, which is simpler and more efficient for skills with many versions.
| Data []struct { | ||
| Name string `json:"name"` | ||
| Description string `json:"description,omitempty"` | ||
| Author string `json:"author,omitempty"` | ||
| LatestVersion string `json:"latest_version,omitempty"` | ||
| Name string `json:"name"` | ||
| Description string `json:"description,omitempty"` | ||
| Author string `json:"author,omitempty"` | ||
| LatestVersion string `json:"latest_version,omitempty"` | ||
| CreatedAt time.Time `json:"created_at,omitempty"` | ||
| } `json:"data"` |
There was a problem hiding this comment.
time.Time values tagged with omitempty will still be marshaled (structs are never considered empty by encoding/json), so these fields will emit "0001-..." when zero. If the intent is to omit missing timestamps to match the registry contract, use *time.Time (or remove omitempty and always send a real value).
| type VersionPublicInfo struct { | ||
| Manifest json.RawMessage `json:"manifest,omitempty"` | ||
| Checksum string `json:"checksum,omitempty"` | ||
| ArchiveURL string `json:"archive_url,omitempty"` | ||
| PublishedAt time.Time `json:"published_at,omitempty"` | ||
| Yanked bool `json:"yanked,omitempty"` |
There was a problem hiding this comment.
Same omitempty issue here: PublishedAt time.Time will not be omitted when zero and will serialize as the year-0001 timestamp. Consider switching to *time.Time (or a custom type) if published_at is optional.
Summary
SkillDetailwith canonicalGET /skills/{name}from registry-api.md:versionsas a map,yankedflag.ResolveInstallTarget: highest non-yanked semver (same idea as referencelatestNonYanked).Authorizationon registry GET and archive download viaSKILLGET_REGISTRY_READ_TOKEN/ fallback to write token (RegistryReadBearer).docs/REGISTRY_CLIENT_CONTRACT.mdfor CLI/integrators.golang.org/x/modfor semver sort.Breaking
Callers unmarshaling
SkillDetailfrom the old array/latest_versionshape must use the registry JSON shape.Paperclip: XDE-85.