feat: catalog metadata annotations, tag command, and remote inspect#4
feat: catalog metadata annotations, tag command, and remote inspect#4
Conversation
Add io.skillimage.tags, io.skillimage.compatibility, and io.skillimage.wordcount annotations. Compute SKILL.md word count during pack. Extend InspectResult to surface new fields. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Pavel Anni <panni@redhat.com>
Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Pavel Anni <panni@redhat.com>
Cover: no tags/compat/SKILL.md, empty SKILL.md produces no wordcount. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Pavel Anni <panni@redhat.com>
Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Pavel Anni <panni@redhat.com>
Add annotation reference table and catalog listing flow to the design spec for the frontend developer. Add skopeo inspect examples to README showing standards-compliant metadata access. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Pavel Anni <panni@redhat.com>
Two realistic example skills for working with technical documents. Demonstrates tags, compatibility, and word count annotations. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Pavel Anni <panni@redhat.com>
Works like "docker tag" / "podman tag": creates an additional
reference pointing to the same image. Enables the standard
tag-then-push workflow for remote registries.
skillctl tag examples/my-skill:1.0.0-draft \
quay.io/myorg/my-skill:1.0.0-draft
skillctl push quay.io/myorg/my-skill:1.0.0-draft
Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Pavel Anni <panni@redhat.com>
Remove digest deduplication so that images tagged for remote registries appear alongside their local references, matching the behavior of podman image ls. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Pavel Anni <panni@redhat.com>
Push/pull were making unauthenticated requests because newRemoteRepository never set up credentials. Now reads from ~/.docker/config.json with fallback to Podman's auth.json. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Pavel Anni <panni@redhat.com>
Inspect now tries the local store first, then falls back to the remote registry. Fixes inability to inspect tags that only exist remotely (e.g., after remote promote). Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Pavel Anni <panni@redhat.com>
Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Pavel Anni <panni@redhat.com>
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 46 minutes and 26 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis pull request introduces OCI manifest annotations to enable skill catalog UIs to read metadata directly from image manifests without downloading layers. It adds annotation constants for tags, compatibility, and word count; extends Pack to compute SKILL.md word count; implements local and remote inspection with annotation extraction; adds a Tag command for creating image aliases; and provides three new example skill definitions with metadata. Changes
Sequence DiagramssequenceDiagram
participant User as User / CLI
participant Pack as Pack()
participant FS as Filesystem
participant BA as buildAnnotations()
participant OCI as OCI Image<br/>(Manifest)
User->>Pack: Pack(skillDir, ...)
Pack->>FS: Read SKILL.md
FS-->>Pack: File content (or empty)
Pack->>Pack: Compute wordCount<br/>via strings.Fields()
Pack->>BA: buildAnnotations(sc,<br/>wordCount)
BA->>BA: Marshal tags to JSON
BA->>BA: Add AnnotationTags<br/>AnnotationCompatibility<br/>AnnotationWordCount
BA-->>Pack: Annotation map
Pack->>OCI: Embed annotations in<br/>OCI manifest
OCI-->>User: Packed image with<br/>metadata annotations
sequenceDiagram
participant User as User / CLI
participant Inspect as Inspect(ref)
participant Local as Local Store
participant Remote as Remote<br/>Repository
participant Manifest as OCI Manifest
participant Result as InspectResult
User->>Inspect: ref (image reference)
alt Local Inspection
Inspect->>Local: Resolve & Fetch
Local-->>Inspect: Manifest data
else Remote Fallback
Inspect->>Remote: Create remote repo
Remote-->>Inspect: Manifest data
end
Inspect->>Manifest: Read & unmarshal
Manifest->>Manifest: Extract annotations<br/>(Tags, Compatibility,<br/>WordCount)
Manifest-->>Inspect: Annotation values
Inspect->>Result: Populate Fields<br/>(existing + new)
Result-->>User: InspectResult with<br/>metadata
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Add comment explaining Podman credential fallback behavior. Document that json.Marshal of []string cannot fail in practice. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Pavel Anni <panni@redhat.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
internal/cli/inspect.go (1)
27-36: Consider preserving the local error when remote fallback also fails.Any local error (not just "not found") silently triggers a remote call, and when both fail only the remote error surfaces. This can make debugging confusing — e.g., a malformed local store or a typo in the ref will be reported as a registry/network failure. Consider wrapping both errors (e.g.,
errors.Join) so the user sees what happened in each path.♻️ Suggested tweak
- ctx := context.Background() - - // Try local first, fall back to remote. - result, err := client.Inspect(ctx, ref) - if err != nil { - result, err = client.InspectRemote(ctx, ref) - } - if err != nil { - return fmt.Errorf("inspecting %s: %w", ref, err) - } + ctx := context.Background() + + // Try local first, fall back to remote. + result, localErr := client.Inspect(ctx, ref) + if localErr != nil { + var remoteErr error + result, remoteErr = client.InspectRemote(ctx, ref) + if remoteErr != nil { + return fmt.Errorf("inspecting %s: local: %w; remote: %w", ref, localErr, remoteErr) + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/cli/inspect.go` around lines 27 - 36, The current fallback flow discards the original local error from client.Inspect when calling client.InspectRemote, so if both fail only the remote error is shown; change the logic to save the first error (e.g., localErr) after calling client.Inspect(ctx, ref), then call client.InspectRemote only if needed, and if that also fails return a combined error that includes both errors (use errors.Join(localErr, err) or wrap one into the other) so both client.Inspect and client.InspectRemote failures are reported; update the error return at the end to reference the combined error while keeping the same context string ("inspecting %s: ...").pkg/oci/pack.go (1)
138-162: Consider consolidatingListLocalandlistLocalWithTags— the core iteration logic is now identical.With dedup removal, both functions iterate through all tags without filtering by digest. However, they have incompatible signatures:
ListLocalcreates its owncontext.Background()and wraps errors, whilelistLocalWithTagsaccepts a context parameter and returns errors directly. Consolidation would require either changingListLocalto accept a context argument (affecting theinternal/cli/list.gocaller) or havingPrunecall a refactoredListLocalwithcontext.Background(). The old comment inprune.go(lines 74–75) describing the difference is now outdated. Worth evaluating as a cleanup, keeping in mind that showing all tags per digest without dedup—including duplicate entries like "1.0.0" and "latest" for the same image—appears to be the intended behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/oci/pack.go` around lines 138 - 162, The two functions ListLocal and listLocalWithTags share identical tag-iteration logic; consolidate by refactoring to a single implementation that accepts a context (e.g., rename listLocalWithTags -> listLocalWithCtx or keep listLocalWithTags signature) and have ListLocal become a thin wrapper that calls it with context.Background() and preserves the current error-wrapping behavior; update callers (Prune/internal/cli/list.go) to call the consolidated function as needed and remove the outdated comment in prune.go about digest filtering. Ensure you reference and update the symbols Client.ListLocal and listLocalWithTags (or the new name) so only one copy of the iteration logic remains.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/oci/pack.go`:
- Around line 55-60: The current SKILL.md word count uses raw file bytes
(skillMDPath) so YAML frontmatter and markdown tokens inflate wordCount and IO
errors are swallowed; change the read-and-count block so that after successfully
reading SKILL.md you strip YAML frontmatter if present (detect a leading "---"
and remove everything through the next "---" delimiter) then run strings.Fields
on the remaining markdown to compute wordCount, and when os.ReadFile returns an
error treat os.IsNotExist(err) as a missing file (leave wordCount 0) but surface
or log any other error instead of silently ignoring it.
In `@pkg/oci/push.go`:
- Around line 66-100: The code ignores REGISTRY_AUTH_FILE and only tries one
Podman path; update credentialStore and podmanAuthPath so Podman auth-file
overrides are honored and all candidate paths are tried: change podmanAuthPath
to return a slice (or add podmanAuthCandidates) that first yields
REGISTRY_AUTH_FILE if set, then the XDG_RUNTIME_DIR candidate and the $HOME
candidate (do not skip the $HOME candidate when XDG is set), then update
credentialStore to iterate those candidates—for each candidate check os.Stat and
attempt credentials.NewStore(candidate, ...) and fall back to dockerStore only
if no podman candidate yields a readable store; keep the existing dockerStore
fallback and preserve the behavior of returning
credentials.NewStoreWithFallbacks(dockerStore, podmanStore) when a podmanStore
is found.
---
Nitpick comments:
In `@internal/cli/inspect.go`:
- Around line 27-36: The current fallback flow discards the original local error
from client.Inspect when calling client.InspectRemote, so if both fail only the
remote error is shown; change the logic to save the first error (e.g., localErr)
after calling client.Inspect(ctx, ref), then call client.InspectRemote only if
needed, and if that also fails return a combined error that includes both errors
(use errors.Join(localErr, err) or wrap one into the other) so both
client.Inspect and client.InspectRemote failures are reported; update the error
return at the end to reference the combined error while keeping the same context
string ("inspecting %s: ...").
In `@pkg/oci/pack.go`:
- Around line 138-162: The two functions ListLocal and listLocalWithTags share
identical tag-iteration logic; consolidate by refactoring to a single
implementation that accepts a context (e.g., rename listLocalWithTags ->
listLocalWithCtx or keep listLocalWithTags signature) and have ListLocal become
a thin wrapper that calls it with context.Background() and preserves the current
error-wrapping behavior; update callers (Prune/internal/cli/list.go) to call the
consolidated function as needed and remove the outdated comment in prune.go
about digest filtering. Ensure you reference and update the symbols
Client.ListLocal and listLocalWithTags (or the new name) so only one copy of the
iteration logic remains.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 67646ef1-06be-4925-9bfe-2ee64a51d77c
📒 Files selected for processing (19)
README.mddocs/superpowers/plans/2026-04-20-catalog-metadata-annotations.mddocs/superpowers/specs/2026-04-20-catalog-metadata-annotations-design.mdexamples/document-reviewer/SKILL.mdexamples/document-reviewer/skill.yamlexamples/document-summarizer/SKILL.mdexamples/document-summarizer/skill.yamlexamples/hello-world/SKILL.mdexamples/hello-world/skill.yamlinternal/cli/inspect.gointernal/cli/root.gointernal/cli/tag.gopkg/oci/annotations.gopkg/oci/client.gopkg/oci/inspect.gopkg/oci/oci_test.gopkg/oci/pack.gopkg/oci/push.gopkg/oci/tag.go
| // 2b. Count words in SKILL.md if present. | ||
| var wordCount int | ||
| skillMDPath := filepath.Join(skillDir, "SKILL.md") | ||
| if data, err := os.ReadFile(skillMDPath); err == nil { | ||
| wordCount = len(strings.Fields(string(data))) | ||
| } |
There was a problem hiding this comment.
Word count includes YAML frontmatter and markdown syntax.
strings.Fields(string(data)) on the raw SKILL.md bytes counts the frontmatter block (---, name:, description:, etc.) and markdown markers (-, **, headings) as "words". For the example skills in this PR the frontmatter alone contributes ~15–20 tokens, noticeably inflating the catalog UI signal. Consider stripping the YAML frontmatter (between the first two --- lines) before counting, so the annotation reflects actual prompt length.
Also, an I/O error other than "file missing" (e.g., permission denied) is silently swallowed into wordCount == 0; at minimum, consider distinguishing os.IsNotExist(err) from other errors so real failures are not hidden.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/oci/pack.go` around lines 55 - 60, The current SKILL.md word count uses
raw file bytes (skillMDPath) so YAML frontmatter and markdown tokens inflate
wordCount and IO errors are swallowed; change the read-and-count block so that
after successfully reading SKILL.md you strip YAML frontmatter if present
(detect a leading "---" and remove everything through the next "---" delimiter)
then run strings.Fields on the remaining markdown to compute wordCount, and when
os.ReadFile returns an error treat os.IsNotExist(err) as a missing file (leave
wordCount 0) but surface or log any other error instead of silently ignoring it.
| // credentialStore returns a credential store that checks Docker config first, | ||
| // then falls back to Podman's auth.json. | ||
| func credentialStore() (credentials.Store, error) { | ||
| dockerStore, err := credentials.NewStoreFromDocker(credentials.StoreOptions{}) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| podmanPath := podmanAuthPath() | ||
| if podmanPath == "" { | ||
| return dockerStore, nil | ||
| } | ||
|
|
||
| if _, err := os.Stat(podmanPath); err != nil { | ||
| return dockerStore, nil | ||
| } | ||
|
|
||
| podmanStore, err := credentials.NewStore(podmanPath, credentials.StoreOptions{}) | ||
| if err != nil { | ||
| // Podman config unreadable but Docker config works — fall back gracefully. | ||
| return dockerStore, nil | ||
| } | ||
|
|
||
| return credentials.NewStoreWithFallbacks(dockerStore, podmanStore), nil | ||
| } | ||
|
|
||
| func podmanAuthPath() string { | ||
| if xdg := os.Getenv("XDG_RUNTIME_DIR"); xdg != "" { | ||
| return filepath.Join(xdg, "containers", "auth.json") | ||
| } | ||
| home, err := os.UserHomeDir() | ||
| if err != nil { | ||
| return "" | ||
| } | ||
| return filepath.Join(home, ".config", "containers", "auth.json") |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Podman login REGISTRY_AUTH_FILE authfile default XDG_RUNTIME_DIR containers auth.json
💡 Result:
In Podman on Linux, the default authentication file for podman login is ${XDG_RUNTIME_DIR}/containers/auth.json (typically /run/user/$UID/containers/auth.json), which is ephemeral and cleared on logout or reboot for security. Key details: - Use --authfile path to specify a custom file, e.g., podman login --authfile ~/.config/containers/auth.json quay.io for persistence. - Override default globally with environment variable REGISTRY_AUTH_FILE=path, e.g., export REGISTRY_AUTH_FILE=$HOME/.config/containers/auth.json. - Podman first checks the specified/primary auth file, then
Citations:
- 1: https://docs.podman.io/en/stable/markdown/podman-login.1.html
- 2: https://docs.podman.io/en/v5.8.0/markdown/podman-login.1.html
- 3: https://docs.podman.io/en/v5.7.0/markdown/podman-login.1.html
- 4: https://stackoverflow.com/questions/77412593/how-are-docker-credentials-saved-in-podman
- 5: https://docs.podman.io/en/latest/_sources/markdown/podman-login.1.md.txt
- 6: https://docs.podman.io/en/v3.3.0/markdown/podman-login.1.html
- 7: https://man.archlinux.org/man/podman-login.1.en.txt
- 8: why login registry auth.json always been cleared? containers/podman#9454
- 9: https://access.redhat.com/solutions/7002142
- 10: https://man.archlinux.org/man/containers-auth.json.5
- 11: https://docs.podman.io/en/v5.3.0/markdown/podman-login.1.html
🏁 Script executed:
# Search for credentialStore usage
rg "credentialStore" --type go -B 2 -A 2Repository: redhat-et/skillimage
Length of output: 642
🏁 Script executed:
# Search for newRemoteRepository usage to determine affected commands
rg "newRemoteRepository" --type go -B 2 -A 2Repository: redhat-et/skillimage
Length of output: 2048
Honor Podman auth-file overrides and try all Podman auth candidates.
podmanAuthPath() only returns one path and ignores REGISTRY_AUTH_FILE, so users with valid Podman credentials in a custom auth file—or in $HOME/.config/containers/auth.json when XDG_RUNTIME_DIR is set—can fail to authenticate. This affects Push(), Pull(), Promote(), and InspectRemote() which all use newRemoteRepository(). Podman documents REGISTRY_AUTH_FILE as an auth-file override: https://docs.podman.io/en/stable/markdown/podman-login.1.html.
🔐 Proposed fix
func credentialStore() (credentials.Store, error) {
dockerStore, err := credentials.NewStoreFromDocker(credentials.StoreOptions{})
if err != nil {
return nil, err
}
- podmanPath := podmanAuthPath()
- if podmanPath == "" {
- return dockerStore, nil
- }
-
- if _, err := os.Stat(podmanPath); err != nil {
- return dockerStore, nil
- }
-
- podmanStore, err := credentials.NewStore(podmanPath, credentials.StoreOptions{})
- if err != nil {
- // Podman config unreadable but Docker config works — fall back gracefully.
- return dockerStore, nil
+ var fallbackStores []credentials.Store
+ for _, podmanPath := range podmanAuthPaths() {
+ if _, err := os.Stat(podmanPath); err != nil {
+ continue
+ }
+
+ podmanStore, err := credentials.NewStore(podmanPath, credentials.StoreOptions{})
+ if err != nil {
+ // Podman config unreadable but Docker config works — fall back gracefully.
+ continue
+ }
+ fallbackStores = append(fallbackStores, podmanStore)
}
- return credentials.NewStoreWithFallbacks(dockerStore, podmanStore), nil
+ if len(fallbackStores) == 0 {
+ return dockerStore, nil
+ }
+ return credentials.NewStoreWithFallbacks(dockerStore, fallbackStores...), nil
}
-func podmanAuthPath() string {
+func podmanAuthPaths() []string {
+ var paths []string
+ if authFile := os.Getenv("REGISTRY_AUTH_FILE"); authFile != "" {
+ paths = append(paths, authFile)
+ }
if xdg := os.Getenv("XDG_RUNTIME_DIR"); xdg != "" {
- return filepath.Join(xdg, "containers", "auth.json")
+ paths = append(paths, filepath.Join(xdg, "containers", "auth.json"))
}
home, err := os.UserHomeDir()
if err != nil {
- return ""
+ return paths
}
- return filepath.Join(home, ".config", "containers", "auth.json")
+ return append(paths, filepath.Join(home, ".config", "containers", "auth.json"))
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/oci/push.go` around lines 66 - 100, The code ignores REGISTRY_AUTH_FILE
and only tries one Podman path; update credentialStore and podmanAuthPath so
Podman auth-file overrides are honored and all candidate paths are tried: change
podmanAuthPath to return a slice (or add podmanAuthCandidates) that first yields
REGISTRY_AUTH_FILE if set, then the XDG_RUNTIME_DIR candidate and the $HOME
candidate (do not skip the $HOME candidate when XDG is set), then update
credentialStore to iterate those candidates—for each candidate check os.Stat and
attempt credentials.NewStore(candidate, ...) and fall back to dockerStore only
if no podman candidate yields a readable store; keep the existing dockerStore
fallback and preserve the behavior of returning
credentials.NewStoreWithFallbacks(dockerStore, podmanStore) when a podmanStore
is found.
- Strip YAML frontmatter from SKILL.md before counting words so metadata fields don't inflate the complexity indicator. - Preserve both local and remote errors in inspect fallback using errors.Join so failures are fully reported. - Consolidate duplicate listLocalWithTags into shared listLocal; ListLocal and Prune now use the same implementation. - Add TestWordCountExcludesFrontmatter. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Pavel Anni <panni@redhat.com>
1 participant
Summary
io.skillimage.tags,io.skillimage.compatibility, andio.skillimage.wordcountto OCI manifest annotations so the catalog UI can build skill cards from the manifest alone — eliminating 10MB blob downloads per skill during listingskillctl tagcommand: Docker/Podman-style retagging for local images, enabling the familiar tag-then-push workflow for remote registriesskillctl inspectnow tries the local store first, then falls back to the remote registry — fixes inability to inspect tags that only exist remotely (e.g., after remote promote)skopeo inspectexamples showing standards-compliant metadata accessDesign spec
docs/superpowers/specs/2026-04-20-catalog-metadata-annotations-design.mdTest plan
TestAnnotationsIncludeTags— tags, compatibility, wordcount set correctlyTestAnnotationsOmittedWhenEmpty— no annotations when fields absentTestAnnotationsEmptySKILLmd— empty SKILL.md produces no wordcountTestTag— local retagging works, inspect via new tag resolvesmake lint— zero issuesSummary by CodeRabbit
Release Notes
New Features
skillctl tagcommand to create skill image tag aliasesDocumentation