feat: skillctl MVP — CLI with lifecycle management#1
Merged
Conversation
- Add SkillCard v1 JSON Schema with validation rules - Create schemas/embed.go for embedding schema at build time - Add hello-world example skill for testing and documentation - Initialize go.sum (dependencies will be added in subsequent tasks) Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Pavel Anni <panni@redhat.com>
Implements SkillCard Go types matching the YAML schema from Task 1. Provides Parse() to read YAML from io.Reader and Serialize() to write YAML to io.Writer. Uses gopkg.in/yaml.v3 with strict field checking (KnownFields=true) to catch typos in SkillCard YAML. Test coverage includes valid YAML parsing, invalid YAML error handling, and roundtrip serialization. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Pavel Anni <panni@redhat.com>
Implements validation against the embedded JSON Schema and additional semantic version validation using Masterminds/semver. Returns structured validation errors with field paths and messages for better UX. 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 cobra-based CLI framework with skillctl validate command. The validate command checks SkillCard YAML files against the JSON Schema and semantic rules (semver, name patterns). Supports both directory and file paths as input. When given a directory, automatically looks for skill.yaml within it. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Pavel Anni <panni@redhat.com>
Signed-off-by: Pavel Anni <panni@redhat.com>
Implements the lifecycle state machine for skill versioning: - Five states: draft, testing, published, deprecated, archived - Valid transitions: draft->testing->published->deprecated->archived - OCI tag rules per state (draft: v-draft, testing: v-rc, published: v) - State parsing and validation Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Pavel Anni <panni@redhat.com>
Implement the core OCI package using oras-go v2 to build standard OCI images from skill directories. Pack reads a skill directory, validates the SkillCard, creates a tar.gz layer, builds an OCI image manifest with proper config and annotations, and stores in a local OCI layout. ListLocal reads tags from the store and returns image metadata. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Pavel Anni <panni@redhat.com>
Add thin CLI wrappers around pkg/oci.Client for local pack and list operations. The pack command creates OCI images from skill directories and stores them in ~/.local/share/skillctl/store. The images command displays all locally packed skills. Also fix missing crypto/sha256 import in pkg/oci/pack.go to register the hash algorithm for go-digest. Signed-off-by: Pavel Anni <panni@redhat.com> Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Pavel Anni <panni@redhat.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>
Implement push and pull commands for skillctl: - push: push a skill image from local store to remote registry - pull: pull a skill image from remote registry to local store - pull supports -o flag to unpack skill files to a directory These are thin wrappers around pkg/oci.Client.Push and Pull. Signed-off-by: Pavel Anni <panni@redhat.com> Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Pavel Anni <panni@redhat.com>
Implements OCI inspect operation and skillctl inspect command to show detailed SkillCard metadata and OCI image details for locally stored skill images. Extracts annotations from manifest and computes layer statistics. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Pavel Anni <panni@redhat.com>
Implement OCI promote operation that transitions a skill image through lifecycle states (draft -> testing -> published -> deprecated -> archived) by updating manifest annotations and tags while keeping image layers immutable. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Pavel Anni <panni@redhat.com>
Signed-off-by: Pavel Anni <panni@redhat.com>
The -testing suffix is more descriptive than -rc for our audience. Semver allows any alphanumeric pre-release label. Also added the TAG column to skillctl images so users see the actual ref to use. Updated design docs and README to match. Signed-off-by: Pavel Anni <panni@redhat.com>
- Renamed 'skillctl images' to 'skillctl list' with 'ls' alias - Added 'skillctl prune' to remove images superseded by promotion - Extracted imageFromManifest helper for shared manifest reading Signed-off-by: Pavel Anni <panni@redhat.com>
Install unpacks skills from local store to agent-specific directories. Supports --target (claude, cursor, windsurf, opencode, openclaw) and -o for custom paths. Pull now detects local-looking refs and suggests install instead. Signed-off-by: Pavel Anni <panni@redhat.com>
- Go module: github.com/redhat-et/oci-skill-registry -> github.com/redhat-et/skillimage - apiVersion: skills.redhat.io/v1alpha1 -> skillimage.io/v1alpha1 - JSON Schema $id updated to skillimage.io - CLI binary stays as skillctl - Domains skillimage.io and skillimage.dev purchased Signed-off-by: Pavel Anni <panni@redhat.com>
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
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 38 minutes and 16 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 (7)
📝 WalkthroughWalkthroughIntroduces a complete OCI skill registry framework called "skillimage" with a CLI tool ( Changes
Sequence DiagramssequenceDiagram
participant CLI as skillctl pack
participant Client as OCI Client
participant Validator as SkillCard Validator
participant Store as Local OCI Store
CLI->>Client: Pack(skillDir, opts)
Client->>Validator: Read & Parse skill.yaml
Validator->>Validator: Validate SkillCard
Validator-->>Client: SkillCard + SKILL.md path
Client->>Client: Create tar.gz layer from dir
Client->>Store: Push layer blob
Store-->>Client: Layer digest
Client->>Client: Build image config
Client->>Store: Push config blob
Store-->>Client: Config digest
Client->>Client: Build OCI manifest with annotations
Client->>Store: Push manifest blob
Store-->>Client: Manifest digest
Client->>Store: Tag manifest as namespace/name:version-draft
Store-->>Client: Success
Client-->>CLI: Manifest descriptor + digest
sequenceDiagram
participant CLI as skillctl push/pull
participant Client as OCI Client
participant Store as Local OCI Store
participant Registry as Remote Registry
alt Push Flow
CLI->>Client: Push(ref, opts)
Client->>Registry: Create remote repository
Registry-->>Client: Repository connected
Client->>Store: Resolve image in local store
Store-->>Client: Manifest descriptor
Client->>Registry: Copy image via ORAS
Registry-->>Client: Copy complete
Client->>Store: Tag as ref
Store-->>Client: Success
Client-->>CLI: Success
else Pull Flow
CLI->>Client: Pull(ref, opts)
Client->>Registry: Create remote repository
Registry-->>Client: Repository connected
Client->>Registry: Fetch image via ORAS
Registry-->>Client: Manifest + layers
Client->>Store: Push to local store
Store-->>Client: Image stored
Client->>Client: Extract to OutputDir if set
Client-->>CLI: Descriptor + unpack status
end
sequenceDiagram
participant CLI as skillctl promote
participant Client as OCI Client
participant Store as Local OCI Store
participant Registry as Remote Registry
alt Local Promotion
CLI->>Client: PromoteLocal(ref, testing)
Client->>Store: Resolve ref to manifest
Store-->>Client: Manifest descriptor
Client->>Store: Fetch manifest JSON
Store-->>Client: Manifest content
Client->>Client: Parse lifecycle status from annotation
Client->>Client: Validate draft→testing transition
Client->>Client: Update status annotation
Client->>Store: Push updated manifest
Store-->>Client: New manifest digest
Client->>Store: Tag as version-testing
Store-->>Client: Tagged
Client-->>CLI: Promotion complete
else Remote Promotion
CLI->>Client: Promote(ref, testing)
Client->>Registry: Create remote repository
Client->>Registry: Resolve ref
Client->>Registry: Fetch manifest
Client->>Client: Update annotations
Client->>Registry: Push updated manifest
Client->>Registry: Tag with new state tag
Client-->>CLI: Promotion complete
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 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 |
Signed-off-by: Pavel Anni <panni@redhat.com>
Fixes: - #2: Tag stripping now handles registry ports correctly (splitRefTag parses after last / to avoid port:tag confusion) - #4: File permissions from tar clamped to 0755/0644 max - #6: JSON Schema $id updated to skillimage.io - #7: Annotation key changed from io.skillregistry to io.skillimage - #10: validate no longer calls os.Exit(1) inside RunE - #11: Removed unused flagFormat/flagVerbose declarations - #12: tabwriter now uses cmd.OutOrStdout() instead of os.Stdout - #15: Moved defaultClient/defaultStoreDir to helpers.go - #16: Added --version flag via Cobra's Version field Declined: - #1: Platform stays amd64/linux — K8s ImageVolumes require valid platform; skill/skill would break mounting - #8: init() panic is standard Go for embedded resources - #13: Configurable timestamps deferred to reproducible builds phase 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: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/superpowers/plans/2026-04-16-skillctl-mvp.md (1)
954-975:⚠️ Potential issue | 🟡 MinorPlan doc is inconsistent with the new
-testingtag convention.This plan file still documents the
Testingstate tag as<version>-rcin multiple places, but the implementation inpkg/lifecycle/lifecycle.go(TagForStatereturnsversion + "-testing") and the updated design spec use-testing. Affected locations in this file include:
- Line 961:
{"1.2.0", lifecycle.Testing, "1.2.0-rc"}inTestTagForState- Line 1055: the inline
TagForStateimplementation returnsversion + "-rc"forTesting- Lines 1171–1172:
ListLocaltest expects tag1.0.0-rc... actually that one checks1.0.0-draft, but promote tests at lines 2523, 2532, 2537, 2798, 2833–2834 all reference1.0.0-rc- Lines 1859, 1865 in the Unpack test reference
test/test-skill:1.0.0-rcAnyone following this plan verbatim will produce tests that fail against the shipped lifecycle package. Please update all
-rcoccurrences to-testingfor consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/superpowers/plans/2026-04-16-skillctl-mvp.md` around lines 954 - 975, Update the plan doc tests and inline implementations to match the shipped lifecycle convention that uses "-testing" (not "-rc"): replace the "1.2.0-rc" expectation in TestTagForState, change the inline TagForState implementation that returns version + "-rc" to return version + "-testing", and update all other test expectations referenced (ListLocal, promote tests, Unpack tests, etc.) that currently expect "1.0.0-rc" or similar to "1.0.0-testing" so the plan matches the actual pkg/lifecycle.TagForState behavior.
🧹 Nitpick comments (10)
internal/cli/helpers.go (1)
18-32: Usefilepath.Joininstead of string concatenation for path construction.
home + "/.local/share"anddataDir + "/skillctl/store"hardcode forward slashes. While this happens to work on Linux/macOS andskillctlis primarily a Unix tool, usingfilepath.Joinis idiomatic Go and avoids any surprises ifXDG_DATA_HOMEever contains a trailing slash (which would produce a double-slash path).Proposed fix
import ( "fmt" "os" + "path/filepath" "github.com/redhat-et/skillimage/pkg/oci" ) @@ func defaultStoreDir() (string, error) { dataDir := os.Getenv("XDG_DATA_HOME") if dataDir == "" { home, err := os.UserHomeDir() if err != nil { return "", fmt.Errorf("finding home directory: %w", err) } - dataDir = home + "/.local/share" + dataDir = filepath.Join(home, ".local", "share") } - dir := dataDir + "/skillctl/store" + dir := filepath.Join(dataDir, "skillctl", "store") if err := os.MkdirAll(dir, 0o755); err != nil { return "", fmt.Errorf("creating store directory: %w", err) } return dir, nil }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/cli/helpers.go` around lines 18 - 32, The defaultStoreDir function builds filesystem paths by concatenating strings; replace those concatenations with filepath.Join to construct paths portably and correctly (handle XDG_DATA_HOME and home dir cases). Specifically, in defaultStoreDir use filepath.Join(home, ".local", "share") instead of home + "/.local/share" and filepath.Join(dataDir, "skillctl", "store") instead of dataDir + "/skillctl/store"; ensure you import "path/filepath" and keep the same error handling and permissions when calling os.MkdirAll.pkg/oci/client.go (1)
15-24: Wrapoci.Newerror with context.Every other error in this package is wrapped with
fmt.Errorf(... %w ...); this one propagates the raw error, so callers see a bare message without knowing which store path failed. Minor, but a small improvement for debuggability.Proposed fix
func NewClient(storePath string) (*Client, error) { store, err := oci.New(storePath) if err != nil { - return nil, err + return nil, fmt.Errorf("opening OCI store at %s: %w", storePath, err) } return &Client{ store: store, storePath: storePath, }, nil }(Requires adding
"fmt"to imports.)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/oci/client.go` around lines 15 - 24, The NewClient function returns the raw error from oci.New; wrap that error with context including the failing storePath so callers can see which path failed. Update NewClient (function NewClient and the call to oci.New) to return fmt.Errorf("creating OCI store at %s: %w", storePath, err) instead of returning err directly and add the "fmt" import.internal/cli/pack.go (1)
25-37: Usecmd.Context()instead ofcontext.Background().Cobra propagates a context (signal-cancellable when set up) via
cmd.Context(). Usingcontext.Background()here (and inpush.go/promote.go) prevents future cancellation/signal handling from working. Low priority but worth fixing consistently across the new CLI commands.Proposed fix
- desc, err := client.Pack(context.Background(), dir, oci.PackOptions{Tag: tag}) + desc, err := client.Pack(cmd.Context(), dir, oci.PackOptions{Tag: tag})🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/cli/pack.go` around lines 25 - 37, The runPack function (and analogous functions in push.go/promote.go) currently calls client.Pack with context.Background(), which bypasses Cobra's cancellable command context; replace context.Background() with cmd.Context() in the client.Pack call inside runPack (and similarly update client.Push/client.Promote calls in runPush/runPromote) so the command respects cancellation/signals propagated by Cobra's command context.internal/cli/promote.go (1)
40-65: Consider validating the transition before calling out to the registry.
lifecycle.ParseStateonly validates thattois a known state, not that the transition from the current state is valid. For the remote (non---local) path, an invalid transition will only be detected after a network round-trip. Ifpkg/lifecycleexposes a transition validator, consider using it earlier; otherwise, this is fine for the MVP.Also: pass
cmd.Context()instead ofcontext.Background()so signal cancellation works.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/cli/promote.go` around lines 40 - 65, The runPromote function currently only validates that toState is a known state via lifecycle.ParseState and uses context.Background(); update it to validate the proposed transition before making remote calls (use any transition validator exposed by pkg/lifecycle — e.g., lifecycle.IsValidTransition or lifecycle.ValidateTransition if available) and return an error early for invalid transitions, and replace context.Background() with cmd.Context() so cancellation/signals propagate; ensure both client.PromoteLocal(ctx, ref, to) and client.Promote(ctx, ref, to, ...) use the new ctx and that the transition check runs before either call.pkg/oci/inspect.go (1)
23-37: Minor: uncheckedrc.Close(matches golangci-lint errcheck hint).Since the body has already been read successfully, a Close error is informational only, but wiring it through keeps the linter happy and mirrors typical Go idiom for read-then-close of small payloads.
🔧 Suggested tweak
- rc, err := c.store.Fetch(ctx, desc) - if err != nil { - return nil, fmt.Errorf("fetching manifest: %w", err) - } - defer rc.Close() - - manifestBytes, err := io.ReadAll(rc) + rc, err := c.store.Fetch(ctx, desc) + if err != nil { + return nil, fmt.Errorf("fetching manifest: %w", err) + } + manifestBytes, err := io.ReadAll(rc) + _ = rc.Close() if err != nil { return nil, fmt.Errorf("reading manifest: %w", err) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/oci/inspect.go` around lines 23 - 37, The code currently defers rc.Close() and ignores its return value; replace the blind defer with an explicit close after reading the body so any Close error is captured and returned (i.e., remove defer rc.Close(), call err = io.ReadAll(rc) as now, then after that call do if cerr := rc.Close(); cerr != nil { return nil, fmt.Errorf("closing manifest reader: %w", cerr) } before unmarshalling). Refer to the Fetch call that yields rc, the rc variable, io.ReadAll(manifestBytes), and the json.Unmarshal(manifestBytes, &manifest) sequence when making this change.pkg/skillcard/skillcard_test.go (1)
162-162: Don't discardValidate's error return in tests.At lines 162, 185, and 211, the error from
skillcard.Validate(sc)is dropped with_. IfValidateever fails internally (e.g., YAML marshal error on a refactor), these tests would silently still inspecterrsand give misleading results. At.Fatalfon non-nil error would make failures actionable.Also applies to: 185-185, 211-211
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/skillcard/skillcard_test.go` at line 162, The tests call skillcard.Validate(sc) but ignore its error return; update each test invocation (the ones calling skillcard.Validate(sc) that assign errs, _ := ...) to capture the error and fail the test on non-nil (e.g., errs, err := skillcard.Validate(sc); if err != nil { t.Fatalf("Validate returned error: %v", err) }) before asserting on errs so internal failures (like YAML marshal errors) surface immediately.internal/cli/install.go (1)
89-96: DuplicatesskillNameFromReflogic inpkg/oci/pull.go.The exact same tag/path stripping lives in
skillNameFromRef(pkg/oci/pull.go:93-104). Export it (e.g.,oci.SkillNameFromRef) and reuse it here so the two implementations can't drift — especially important sinceUnpackuses its version to pick the destination directory (filepath.Join(outputDir, skillName)at line 98 must match whatUnpackactually created).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/cli/install.go` around lines 89 - 96, The skill name extraction logic in internal/cli/install.go duplicates skillNameFromRef from pkg/oci/pull.go; export the shared implementation (e.g., rename to SkillNameFromRef in the oci package) and call oci.SkillNameFromRef(ref) from install.go instead of reimplementing the parsing there so the value used for filepath.Join(outputDir, skillName) matches Unpack's behavior; update any imports/usages to reference the exported function and remove the duplicated code in install.go.pkg/oci/prune.go (1)
39-69: Parse status once per image.Each image's
Statusis parsed twice (first loop for computingmaxRank, second for filtering). A single pass that records(img, rank)pairs would avoid the duplication and make the skip-on-parse-error behavior explicit/consistent.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/oci/prune.go` around lines 39 - 69, The code currently calls lifecycle.ParseState(img.Status) twice per image (once to compute maxRank and again to decide removal), causing duplicate work and inconsistent skip behavior; refactor the loops in the prune logic to a single pass that records each image's parse result and its rank (e.g., build a slice or map of (LocalImage -> rank) while skipping images that fail ParseState), compute maxRank per group from those recorded ranks, then iterate the recorded pairs to call c.store.Untag(ctx, fullRef) for images whose recorded rank < maxRank; update references to removed, groups, stateRank, lifecycle.ParseState, and c.store.Untag so parsing is done exactly once per img and skip-on-parse-error is explicit.pkg/oci/pull.go (2)
149-149: UncheckedClose()returns (flagged by golangci-lint/errcheck).
- Line 149:
defer gz.Close()— for a gzipReader,Closecan report trailing-data/checksum issues; at minimum, assign to_explicitly or wrap to satisfy the linter.- Line 180:
f.Close()on the error path is unchecked, which is acceptable here since we're already returning an error, but the linter will still flag it. Use_ = f.Close()to make the intent explicit.🛠 Minimal fix
- defer gz.Close() + defer func() { _ = gz.Close() }() ... - if _, err := io.Copy(f, tr); err != nil { - f.Close() - return fmt.Errorf("writing file %s: %w", target, err) - } - f.Close() + if _, err := io.Copy(f, tr); err != nil { + _ = f.Close() + return fmt.Errorf("writing file %s: %w", target, err) + } + if err := f.Close(); err != nil { + return fmt.Errorf("closing file %s: %w", target, err) + }Also applies to: 180-180
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/oci/pull.go` at line 149, The linter flags unchecked Close() calls for the gzip reader and file; update the defer and explicit close sites to make intent explicit: change the deferred gz.Close() to capture or discard the returned error (e.g., use _ = gz.Close() or wrap it in a closure that assigns the error) and replace the unchecked f.Close() on the error path with an explicit discard assignment (_ = f.Close()); locate the calls to gz.Close() and f.Close() in this file (symbols gz and f in the pull handling function) and apply those changes so errcheck/golangci-lint is satisfied.
166-184: Unsupported tar entry types are silently skipped.The
switchonly handlesTypeDirandTypeReg. Symlinks, hardlinks, and other types are dropped without warning. For skill packages this is probably fine (and safer than supporting symlinks throughsanitizeTarPath), but consider either (a) returning an explicit error on unexpected types so malformed/malicious layers fail loudly, or (b) at least documenting the intent in the comment above.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/oci/pull.go` around lines 166 - 184, The extraction switch handling header.Typeflag in pkg/oci/pull.go currently only handles tar.TypeDir and tar.TypeReg and silently skips other entry types; add a default case to the switch that returns an explicit error (e.g., fmt.Errorf("unsupported tar entry type %v for %s", header.Typeflag, header.Name)) so unexpected types cause the pull to fail loudly, and ensure the error bubbles up from the enclosing function that performs the tar extraction (the block that uses header, tr, io.Copy, os.OpenFile, clampDirMode, clampFileMode, sanitizeTarPath).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/cli/install.go`:
- Around line 47-48: The output path flag (outputDir set via
cmd.Flags().StringVarP) can contain an unexpanded leading tilde so when users
pass "~" or "~/..." it becomes a literal folder; update the command handler that
reads outputDir (the Run/PreRun where outputDir is used) to detect a leading "~"
or "~/" and replace it with the current user home from os.UserHomeDir(),
preserving the rest of the path and using filepath.Clean; ensure this
normalization happens before any mkdir/create or path joins that use outputDir
so quoted or programmatic flag values behave like shell-expanded ones.
In `@internal/cli/pull.go`:
- Around line 35-43: The looksLocal heuristic is too strict and misclassifies
short-hostname registries; change looksLocal(ref string) to treat any ref
containing a '/' as remote by returning len(strings.SplitN(ref, "/", 2)) < 2 and
remove the dot/colon checks on parts[0]; this keeps rejecting single-segment
refs like "myskill:1.0" while allowing "localhost/foo:tag" or
"registry/foo:tag". Update the looksLocal function (referenced by runPull)
accordingly and adjust/add tests if present to cover hostname-without-dot cases.
In `@pkg/oci/annotations.go`:
- Around line 26-31: The current byte-slice truncation of the description
(variable desc) before assigning to ann[ocispec.AnnotationDescription] can cut a
UTF-8 rune and produce invalid UTF-8; update the truncation logic in
pkg/oci/annotations.go (where desc is set from sc.Metadata.Description) to
truncate on a rune boundary instead of raw bytes — either convert to []rune and
limit the rune count to 256 runes, or trim the 256-byte substring back to the
previous valid UTF-8 boundary using utf8.ValidString (or
utf8.DecodeLastRuneInString) so the final desc is always valid UTF-8 before
assigning to the annotation.
- Around line 14-16: Update the function comment for buildAnnotations to reflect
the renamed annotation key: change the phrase "skillregistry status annotation"
to "skillimage status annotation" so it matches lifecycle.StatusAnnotation and
the current io.skillimage.status name; ensure the comment mentions
buildAnnotations and lifecycle.StatusAnnotation to make the connection clear.
In `@pkg/oci/pack.go`:
- Around line 220-254: The walk callback that uses tar.FileInfoHeader in the
anonymous func passed to filepath.Walk currently produces empty Linkname for
symlinks and may try to read non-regular files; update that callback to (1)
detect symlinks via info.Mode()&os.ModeSymlink and, for symlinks, call
os.Readlink(path) and pass the link target to tar.FileInfoHeader (set
header.Linkname) before tw.WriteHeader, and (2) explicitly skip non-regular file
types (sockets, devices, pipes) by checking ModeType bits (e.g.,
!info.Mode().IsRegular() && !(symlink case handled)) and returning nil (or
filepath.SkipDir for directories as already done) so you don't call
os.Open/io.Copy on unsupported entries. Ensure header.Name still uses
filepath.ToSlash(rel).
- Around line 31-35: Replace the plain deferred Close calls with explicit
error-suppression closures to satisfy errcheck: for the file handle opened as f
when reading skillPath (defer f.Close()), the reader rc closed at the later read
(defer rc.Close()), and the other deferred f.Close() in the loop, change each to
a deferred anonymous func that assigns the Close error to the blank identifier
(e.g., defer func() { _ = <var>.Close() }()) so the code keeps the read-only
close but silences errcheck; locate these variables (f and rc) in
pkg/oci/pack.go and apply the same pattern to all three defer calls.
- Around line 256-265: The callback in pkg/oci/pack.go opens files during
filepath.Walk and uses defer f.Close(), which keeps all FDs open until the walk
finishes; change this so each file is closed immediately after copying by
scoping the open/copy/close in an inline closure or by explicitly calling
f.Close() before returning from the callback. Locate the block that opens f
(using os.Open(path)), performs io.Copy(tw, f) and currently defers f.Close();
replace the defer with a per-file closure (e.g., an anonymous func that defers
f.Close() inside it and is invoked immediately) or call f.Close() right after
io.Copy and handle any close error, ensuring errors from os.Open, io.Copy, and
f.Close() are propagated (use the same fmt.Errorf wrapping style as the existing
returns).
In `@pkg/oci/promote.go`:
- Around line 58-62: The reader returned as rc from the Fetch call is being
closed without checking errors; change the code in promote.go around the
manifest reading (where manifestBytes and rc are used) to ensure rc.Close() is
deferred immediately after obtaining rc (e.g., defer rc.Close()) or capture and
handle the Close error (assign err = rc.Close() and wrap/log it) so the close is
not ignored; update the block where manifestBytes, err := io.ReadAll(rc) is used
to rely on the deferred/handled Close to satisfy errcheck.
- Around line 106-116: The manifest's version annotation
(manifest.Annotations[ocispec.AnnotationVersion]) can be empty and produce an
invalid tag from lifecycle.TagForState; add a guard to check that version is
non-empty before calling lifecycle.TagForState (or adjust TagForState to return
"" for empty input) and only build tagRef and call store.Tag when newTag is
non-empty and derived from a non-empty version; locate the logic around
parseNameFromTag(ref), lifecycle.TagForState(version, to), newTag, and store.Tag
to implement this validation.
- Line 107: The current parseNameFromTag call (used to set namespaceName)
mishandles digest-form references like registry.io/ns/name@sha256:abcd by
splitting on the last ':' and producing malformed tag refs; update the logic so
parseNameFromTag (or the caller) first detects an '@' digest and either (a)
splits on '@' and uses the portion before '@' as the repository name when
creating new tag references, or (b) explicitly rejects digest-style inputs with
a clear error; modify parseNameFromTag or add validation in the promote flow
(where namespaceName is computed and later used to build the new tag) to
implement one of these behaviors and ensure tag construction never combines a
digest fragment with a new :tag.
In `@pkg/oci/prune.go`:
- Around line 76-99: listLocalWithTags currently ignores the caller's context by
creating context.Background(), which prevents Prune(ctx) cancellations from
propagating; change func (c *Client) listLocalWithTags() to accept ctx
context.Context and use that ctx when calling c.store.Tags, c.store.Resolve and
c.imageFromManifest, and update the Prune caller to pass its ctx into
listLocalWithTags so tag enumeration and per-tag operations respect
cancellation/timeouts.
In `@pkg/oci/pull.go`:
- Around line 26-35: The pull implementation currently calls splitRefTag and
passes only the bare tag to oras.Copy (creating a bare, un-namespaced tag) and
then calls c.store.Tag to add the full ref, causing tag collisions; change the
call to oras.Copy to use the full ref string (pass ref instead of tag) so the
stored entry is already namespaced, and remove the subsequent c.store.Tag call
that tags with the full ref; this will eliminate the duplicate bare-tag entry
that ListLocal/parseNameFromTag mis-parses.
In `@pkg/oci/push.go`:
- Around line 47-66: splitRefTag currently treats digest refs like
"quay.io/org/name@sha256:..." as part of the repo; change splitRefTag to first
detect and strip a digest by looking for '@' (e.g., idx := strings.Index(ref,
"@")); if found, set ref = ref[:idx] before the existing colon/slash logic so
the "@sha256:..." is not included in the repo passed to remote.NewRepository;
this preserves tag handling and avoids misbehavior downstream (alternatively, if
digests are out of scope, explicitly detect '@' and return an error from
splitRefTag by changing its signature to (repo, tag string, err error) and
propagate that error).
In `@pkg/skillcard/validate.go`:
- Around line 102-107: The current fragile cleanup of ve.Error() by slicing on
the first ": " should be replaced by using the structured error kind; instead of
calling ve.Error() and performing string surgery on the result (the code around
the message variable and the strings.Index slice), obtain the clean text from
ve.ErrorKind (e.g., call ve.ErrorKind.LocalizedString() from the
jsonschema/v6/kind package) and use that as the message; add the kind package
import if missing and remove the idx-based trimming so the message comes
directly from ve.ErrorKind.LocalizedString().
In `@README.md`:
- Around line 58-59: Update the stale CLI example in the README: replace the old
command invocation "bin/skillctl images" with the current CLI name "bin/skillctl
list" (or "bin/skillctl ls") so the "List local images" section and example
reflect the actual command surface; ensure the heading and the example command
string under it are both updated to match the renamed subcommand.
---
Outside diff comments:
In `@docs/superpowers/plans/2026-04-16-skillctl-mvp.md`:
- Around line 954-975: Update the plan doc tests and inline implementations to
match the shipped lifecycle convention that uses "-testing" (not "-rc"): replace
the "1.2.0-rc" expectation in TestTagForState, change the inline TagForState
implementation that returns version + "-rc" to return version + "-testing", and
update all other test expectations referenced (ListLocal, promote tests, Unpack
tests, etc.) that currently expect "1.0.0-rc" or similar to "1.0.0-testing" so
the plan matches the actual pkg/lifecycle.TagForState behavior.
---
Nitpick comments:
In `@internal/cli/helpers.go`:
- Around line 18-32: The defaultStoreDir function builds filesystem paths by
concatenating strings; replace those concatenations with filepath.Join to
construct paths portably and correctly (handle XDG_DATA_HOME and home dir
cases). Specifically, in defaultStoreDir use filepath.Join(home, ".local",
"share") instead of home + "/.local/share" and filepath.Join(dataDir,
"skillctl", "store") instead of dataDir + "/skillctl/store"; ensure you import
"path/filepath" and keep the same error handling and permissions when calling
os.MkdirAll.
In `@internal/cli/install.go`:
- Around line 89-96: The skill name extraction logic in internal/cli/install.go
duplicates skillNameFromRef from pkg/oci/pull.go; export the shared
implementation (e.g., rename to SkillNameFromRef in the oci package) and call
oci.SkillNameFromRef(ref) from install.go instead of reimplementing the parsing
there so the value used for filepath.Join(outputDir, skillName) matches Unpack's
behavior; update any imports/usages to reference the exported function and
remove the duplicated code in install.go.
In `@internal/cli/pack.go`:
- Around line 25-37: The runPack function (and analogous functions in
push.go/promote.go) currently calls client.Pack with context.Background(), which
bypasses Cobra's cancellable command context; replace context.Background() with
cmd.Context() in the client.Pack call inside runPack (and similarly update
client.Push/client.Promote calls in runPush/runPromote) so the command respects
cancellation/signals propagated by Cobra's command context.
In `@internal/cli/promote.go`:
- Around line 40-65: The runPromote function currently only validates that
toState is a known state via lifecycle.ParseState and uses context.Background();
update it to validate the proposed transition before making remote calls (use
any transition validator exposed by pkg/lifecycle — e.g.,
lifecycle.IsValidTransition or lifecycle.ValidateTransition if available) and
return an error early for invalid transitions, and replace context.Background()
with cmd.Context() so cancellation/signals propagate; ensure both
client.PromoteLocal(ctx, ref, to) and client.Promote(ctx, ref, to, ...) use the
new ctx and that the transition check runs before either call.
In `@pkg/oci/client.go`:
- Around line 15-24: The NewClient function returns the raw error from oci.New;
wrap that error with context including the failing storePath so callers can see
which path failed. Update NewClient (function NewClient and the call to oci.New)
to return fmt.Errorf("creating OCI store at %s: %w", storePath, err) instead of
returning err directly and add the "fmt" import.
In `@pkg/oci/inspect.go`:
- Around line 23-37: The code currently defers rc.Close() and ignores its return
value; replace the blind defer with an explicit close after reading the body so
any Close error is captured and returned (i.e., remove defer rc.Close(), call
err = io.ReadAll(rc) as now, then after that call do if cerr := rc.Close(); cerr
!= nil { return nil, fmt.Errorf("closing manifest reader: %w", cerr) } before
unmarshalling). Refer to the Fetch call that yields rc, the rc variable,
io.ReadAll(manifestBytes), and the json.Unmarshal(manifestBytes, &manifest)
sequence when making this change.
In `@pkg/oci/prune.go`:
- Around line 39-69: The code currently calls lifecycle.ParseState(img.Status)
twice per image (once to compute maxRank and again to decide removal), causing
duplicate work and inconsistent skip behavior; refactor the loops in the prune
logic to a single pass that records each image's parse result and its rank
(e.g., build a slice or map of (LocalImage -> rank) while skipping images that
fail ParseState), compute maxRank per group from those recorded ranks, then
iterate the recorded pairs to call c.store.Untag(ctx, fullRef) for images whose
recorded rank < maxRank; update references to removed, groups, stateRank,
lifecycle.ParseState, and c.store.Untag so parsing is done exactly once per img
and skip-on-parse-error is explicit.
In `@pkg/oci/pull.go`:
- Line 149: The linter flags unchecked Close() calls for the gzip reader and
file; update the defer and explicit close sites to make intent explicit: change
the deferred gz.Close() to capture or discard the returned error (e.g., use _ =
gz.Close() or wrap it in a closure that assigns the error) and replace the
unchecked f.Close() on the error path with an explicit discard assignment (_ =
f.Close()); locate the calls to gz.Close() and f.Close() in this file (symbols
gz and f in the pull handling function) and apply those changes so
errcheck/golangci-lint is satisfied.
- Around line 166-184: The extraction switch handling header.Typeflag in
pkg/oci/pull.go currently only handles tar.TypeDir and tar.TypeReg and silently
skips other entry types; add a default case to the switch that returns an
explicit error (e.g., fmt.Errorf("unsupported tar entry type %v for %s",
header.Typeflag, header.Name)) so unexpected types cause the pull to fail
loudly, and ensure the error bubbles up from the enclosing function that
performs the tar extraction (the block that uses header, tr, io.Copy,
os.OpenFile, clampDirMode, clampFileMode, sanitizeTarPath).
In `@pkg/skillcard/skillcard_test.go`:
- Line 162: The tests call skillcard.Validate(sc) but ignore its error return;
update each test invocation (the ones calling skillcard.Validate(sc) that assign
errs, _ := ...) to capture the error and fail the test on non-nil (e.g., errs,
err := skillcard.Validate(sc); if err != nil { t.Fatalf("Validate returned
error: %v", err) }) before asserting on errs so internal failures (like YAML
marshal errors) surface immediately.
🪄 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: f06c823f-23eb-4ee4-9189-ad95001ec6ab
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (36)
.gitignoreREADME.mdcmd/skillctl/main.godocs/design/2026-04-15-oci-skill-registry-design.mddocs/design/2026-04-16-implementation-spec.mddocs/superpowers/plans/2026-04-16-skillctl-mvp.mdexamples/hello-world/SKILL.mdexamples/hello-world/skill.yamlgo.modinternal/cli/helpers.gointernal/cli/inspect.gointernal/cli/install.gointernal/cli/list.gointernal/cli/pack.gointernal/cli/promote.gointernal/cli/prune.gointernal/cli/pull.gointernal/cli/push.gointernal/cli/root.gointernal/cli/validate.gopkg/lifecycle/lifecycle.gopkg/lifecycle/lifecycle_test.gopkg/oci/annotations.gopkg/oci/client.gopkg/oci/inspect.gopkg/oci/oci_test.gopkg/oci/pack.gopkg/oci/promote.gopkg/oci/prune.gopkg/oci/pull.gopkg/oci/push.gopkg/skillcard/skillcard.gopkg/skillcard/skillcard_test.gopkg/skillcard/validate.goschemas/embed.goschemas/skillcard-v1.json
Comment on lines
+47
to
+48
| cmd.Flags().StringVarP(&target, "target", "t", "", "agent name (claude, cursor, windsurf, opencode, openclaw)") | ||
| cmd.Flags().StringVarP(&outputDir, "output", "o", "", "custom output directory") |
There was a problem hiding this comment.
-o values with ~ won't expand when not quoted-for-shell.
The help example shows -o ~/custom/skills/, which relies on shell tilde expansion. When users pass "~/custom/skills/" (quoted) or the flag is set programmatically, ~ is treated literally and a directory named ~ is created under the current working directory. Consider expanding a leading ~ or ~/ against os.UserHomeDir() for the -o path, matching the --target behavior.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/cli/install.go` around lines 47 - 48, The output path flag
(outputDir set via cmd.Flags().StringVarP) can contain an unexpanded leading
tilde so when users pass "~" or "~/..." it becomes a literal folder; update the
command handler that reads outputDir (the Run/PreRun where outputDir is used) to
detect a leading "~" or "~/" and replace it with the current user home from
os.UserHomeDir(), preserving the rest of the path and using filepath.Clean;
ensure this normalization happens before any mkdir/create or path joins that use
outputDir so quoted or programmatic flag values behave like shell-expanded ones.
Comment on lines
+31
to
+35
| f, err := os.Open(skillPath) | ||
| if err != nil { | ||
| return ocispec.Descriptor{}, fmt.Errorf("opening skill.yaml: %w", err) | ||
| } | ||
| defer f.Close() |
There was a problem hiding this comment.
Unchecked Close errors (errcheck).
golangci-lint flags f.Close() on line 35, rc.Close() on line 172, and f.Close() on line 261 (the loop case is addressed separately above). For the read-only opens on lines 35/172 these are benign, but silencing errcheck explicitly keeps the lint clean:
defer func() { _ = f.Close() }()🧰 Tools
🪛 golangci-lint (2.11.4)
[error] 35-35: Error return value of f.Close is not checked
(errcheck)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/oci/pack.go` around lines 31 - 35, Replace the plain deferred Close calls
with explicit error-suppression closures to satisfy errcheck: for the file
handle opened as f when reading skillPath (defer f.Close()), the reader rc
closed at the later read (defer rc.Close()), and the other deferred f.Close() in
the loop, change each to a deferred anonymous func that assigns the Close error
to the blank identifier (e.g., defer func() { _ = <var>.Close() }()) so the code
keeps the read-only close but silences errcheck; locate these variables (f and
rc) in pkg/oci/pack.go and apply the same pattern to all three defer calls.
Comment on lines
+47
to
+66
| // splitRefTag splits an OCI reference into repository and tag. | ||
| // Handles registry ports correctly: localhost:5000/ns/name:tag splits | ||
| // at the tag colon (after the last /), not the port colon. | ||
| func splitRefTag(ref string) (repo, tag string) { | ||
| // Find the last slash to isolate the name:tag portion. | ||
| lastSlash := strings.LastIndex(ref, "/") | ||
| if lastSlash < 0 { | ||
| // No slash: the whole ref might be name:tag. | ||
| if idx := strings.LastIndex(ref, ":"); idx >= 0 { | ||
| return ref[:idx], ref[idx+1:] | ||
| } | ||
| return ref, "" | ||
| } | ||
| // Look for a colon only after the last slash. | ||
| tail := ref[lastSlash+1:] | ||
| if idx := strings.LastIndex(tail, ":"); idx >= 0 { | ||
| return ref[:lastSlash+1+idx], tail[idx+1:] | ||
| } | ||
| return ref, "" | ||
| } |
There was a problem hiding this comment.
splitRefTag doesn't handle digest refs (@sha256:...).
A reference like quay.io/org/name@sha256:abc... will be treated as an untagged ref, and the @sha256:... portion will be included in the repo path passed to remote.NewRepository, which likely fails or misbehaves downstream. If digest refs are out of scope for MVP, consider explicitly rejecting them here with a clear error; otherwise add an @ split before the colon handling.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/oci/push.go` around lines 47 - 66, splitRefTag currently treats digest
refs like "quay.io/org/name@sha256:..." as part of the repo; change splitRefTag
to first detect and strip a digest by looking for '@' (e.g., idx :=
strings.Index(ref, "@")); if found, set ref = ref[:idx] before the existing
colon/slash logic so the "@sha256:..." is not included in the repo passed to
remote.NewRepository; this preserves tag handling and avoids misbehavior
downstream (alternatively, if digests are out of scope, explicitly detect '@'
and return an error from splitRefTag by changing its signature to (repo, tag
string, err error) and propagate that error).
Comment on lines
+102
to
+107
| message := ve.Error() | ||
| // Clean up the error message to remove the instance location prefix | ||
| // The error format is typically: "instanceLocation: message" | ||
| if idx := strings.Index(message, ": "); idx != -1 { | ||
| message = message[idx+2:] | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
github.com/santhosh-tekuri/jsonschema/v6 ValidationError Error() format and ErrorKind API
💡 Result:
In github.com/santhosh-tekuri/jsonschema/v6, ValidationError is the primary error type returned by schema validation methods like Schema.Validate. It embeds or has a field ErrorKind jsonschema.ErrorKind, which is an interface providing structured, introspectable information about the specific validation failure (e.g., required property missing, type mismatch). ErrorKind types are defined in the "github.com/santhosh-tekuri/jsonschema/v6/kind" subpackage, such as kind.Required{}, kind.Type{}, kind.AnyOf{}, etc. ValidationError implements the standard Go error interface via its Error method, which generates a human-readable string describing the error, including hierarchy, instance/schema locations (using JSON pointers or # notation), and details derived from ErrorKind. The output format can be customized using methods like DetailedOutput, FlagOutput, BasicOutput, which return OutputUnit structs marshalable to JSON in various formats (detailed, flag, basic). ErrorKind supports localization via ErrorKind.LocalizedString(printer), using golang.org/x/text/message, allowing internationalized error messages. Examples from usage: - Compare errors: assert.Equal(expected.ErrorKind, actual.ErrorKind) - Hierarchy: ValidationError.Causes []*ValidationError (recursive) - Stable comparison: Avoid string Error due to non-deterministic map order; use DetailedOutput and custom sorting. Migration notes from v5: InstanceLocation now []string (was string), Message removed (use ErrorKind), AbsoluteKeywordLocation removed. Official documentation: pkg.go.dev/github.com/santhosh-tekuri/jsonschema/v6 (Godoc links to full API). Repository default branch "boon" contains v6 source code.
Citations:
- 1: https://pkg.go.dev/github.com/santhosh-tekuri/jsonschema/v6
- 2: https://pkg.go.dev/github.com/santhosh-tekuri/jsonschema/v6@v6.0.1
- 3: Some questions about migrating from v5 to v6 santhosh-tekuri/jsonschema#172
- 4: Error output not stable santhosh-tekuri/jsonschema#213
- 5: https://github.com/santhosh-tekuri/jsonschema
🏁 Script executed:
cd pkg/skillcard && wc -l validate.goRepository: redhat-et/skillimage
Length of output: 79
🏁 Script executed:
cat -n pkg/skillcard/validate.go | sed -n '95,115p'Repository: redhat-et/skillimage
Length of output: 718
🏁 Script executed:
# Also find the broader context of the collectValidationErrors function
cat -n pkg/skillcard/validate.go | sed -n '80,120p'Repository: redhat-et/skillimage
Length of output: 1261
🏁 Script executed:
# Check imports in validate.go and go.mod
head -20 pkg/skillcard/validate.goRepository: redhat-et/skillimage
Length of output: 550
🏁 Script executed:
# Check go.mod for jsonschema version
grep -i jsonschema go.modRepository: redhat-et/skillimage
Length of output: 112
🏁 Script executed:
# Search for any use of ErrorKind in the codebase
rg "ErrorKind" --type goRepository: redhat-et/skillimage
Length of output: 46
Fragile message cleanup via first ": " occurrence.
Since ve.Error() includes instance/schema locations in its formatted output and you're already extracting the structured InstanceLocation separately, the string surgery to remove the location prefix is fragile. If the underlying error message itself contains ": ", this will truncate legitimate content. Instead, access ve.ErrorKind directly and use its structured representation (e.g., ErrorKind.LocalizedString() from the jsonschema/v6/kind subpackage) to get a clean message without string parsing.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/skillcard/validate.go` around lines 102 - 107, The current fragile
cleanup of ve.Error() by slicing on the first ": " should be replaced by using
the structured error kind; instead of calling ve.Error() and performing string
surgery on the result (the code around the message variable and the
strings.Index slice), obtain the clean text from ve.ErrorKind (e.g., call
ve.ErrorKind.LocalizedString() from the jsonschema/v6/kind package) and use that
as the message; add the kind package import if missing and remove the idx-based
trimming so the message comes directly from ve.ErrorKind.LocalizedString().
- Fix FD leak: extract file copy to helper with scoped defer - Skip non-regular files (symlinks, devices) in tar creation - Fix bare-tag collision: pass full ref to oras.Copy destination - Guard against empty version annotation in promote - Simplify looksLocal: any ref with / is treated as remote - Fix UTF-8 truncation in description annotation - Thread context through prune/listLocalWithTags - Update stale comment and README command name Signed-off-by: Pavel Anni <panni@redhat.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
skillctlCLI with 10 commands:validate,pack,push,pull,inspect,list/ls,promote,install,prunepkg/(skillcard, oci, lifecycle), CLI is a thin consumerinstallcommand deploys skills to agent-specific directories (Claude, Cursor, Windsurf, OpenCode, OpenClaw)Key design decisions
skillimage.io/v1alpha1: project-owned domain, vendor-neutral-testingtag suffix instead of-rc: clearer for our audience, valid semver pre-release labelTest plan
make test— all unit tests pass (skillcard, lifecycle, oci packages)make lint— golangci-lint cleanvalidate→pack→list→inspect→promote(draft→testing→published) →pruneinstall --target claudedeploys skill to~/.claude/skills/pullwith local-looking ref shows helpful error pointing toinstall🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
skillctlcommand-line tool for creating, validating, and managing AI agent skills as OCI images.Documentation
Examples