XDE-87: CLI uses skillget-manager v0.1.4 + contract-aligned docs#2
Conversation
- Document SKILLGET_REGISTRY_READ_TOKEN in skillget --help - skillget config reports how the read bearer is resolved - QUICKSTART links REGISTRY_CLIENT_CONTRACT and READ token flow Co-Authored-By: Paperclip <noreply@paperclip.ing> Made-with: Cursor
There was a problem hiding this comment.
Pull request overview
Updates the CLI user-facing documentation and skillget config output to align with the skillget-manager registry client contract, especially around read-token behavior for read-protected registries.
Changes:
- Documented the registry client contract as the source of truth for env vars/routes/status codes.
- Added QUICKSTART guidance for
SKILLGET_REGISTRY_READ_TOKENand fallback-to-write-token behavior. - Enhanced
skillget configto report how the read bearer is being resolved.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| docs/QUICKSTART.md | Links the client contract doc and explains read-token resolution behavior. |
| cmd/skillget/main.go | Updates --help env var docs and prints read-bearer resolution details in skillget config. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| SKILLGET_REGISTRY_READ_TOKEN bearer for read-only registries (GET API + archive_url); | ||
| if unset, falls back to the write token when set | ||
| SKILLGET_REGISTRY_TOKEN bearer for skillget publish (or SKILLGET_TOKEN) | ||
| search/install use anonymous GETs unless a read/write token is set |
There was a problem hiding this comment.
The help text describes SKILLGET_REGISTRY_READ_TOKEN as for “read-only registries”, but the manager contract uses it for registries that require auth for reads (read-protected), not necessarily read-only. Consider rewording to avoid misleading users about registry capabilities.
| SKILLGET_REGISTRY_READ_TOKEN bearer for read-only registries (GET API + archive_url); | |
| if unset, falls back to the write token when set | |
| SKILLGET_REGISTRY_TOKEN bearer for skillget publish (or SKILLGET_TOKEN) | |
| search/install use anonymous GETs unless a read/write token is set | |
| SKILLGET_REGISTRY_READ_TOKEN bearer for registries that require auth for reads (read-protected | |
| GET API + archive_url); if unset, falls back to the write token | |
| SKILLGET_REGISTRY_TOKEN bearer for skillget publish (or SKILLGET_TOKEN); also used for | |
| authenticated search/install when no read token is set (otherwise | |
| search/install use anonymous GETs) |
| fmt.Println("override: export SKILLGET_REGISTRY_URL=… (or legacy SKPKG_REGISTRY_URL)") | ||
| } | ||
| switch { | ||
| case os.Getenv("SKILLGET_REGISTRY_READ_TOKEN") != "": |
There was a problem hiding this comment.
runConfig checks os.Getenv("SKILLGET_REGISTRY_READ_TOKEN") != "" to decide whether a read bearer is set. This treats whitespace-only values as set, but the underlying skillget-manager trims tokens and would treat whitespace as unset, so skillget config can report an incorrect state. Use strings.TrimSpace(os.Getenv(...)) (or compare against skillgetmanager.RegistryReadBearer() and/or RegistryToken() with trimming) to match actual resolution.
| case os.Getenv("SKILLGET_REGISTRY_READ_TOKEN") != "": | |
| case strings.TrimSpace(os.Getenv("SKILLGET_REGISTRY_READ_TOKEN")) != "": |
Summary
main).skillget configwith REGISTRY_CLIENT_CONTRACT.md: documentsSKILLGET_REGISTRY_READ_TOKENand how the read bearer is resolved.Paperclip
Test plan
go test -mod=vendor -short ./...