feat(skills): add command creation skills for CLI development#68
feat(skills): add command creation skills for CLI development#68feloy merged 7 commits intokortex-hub:mainfrom
Conversation
Add four new skills to help developers create different types of CLI commands following established patterns: commands with JSON output, simple text-only commands, alias commands, and parent commands with subcommands. Each skill includes step-by-step instructions, code templates, testing patterns, and references to existing implementations. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Philippe Martin <phmartin@redhat.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
📝 WalkthroughWalkthroughAdds four new skill documentation pages under Changes
Sequence Diagram(s)(Skipped — changes are documentation and discovery references only.) Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📝 Coding Plan
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 |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@skills/add-command-simple/SKILL.md`:
- Around line 153-169: The test's subtest "validates arguments" incorrectly
expects c.preRun(cmd, args) to return an error even though the scaffolded preRun
has no argument validation; either remove the failing expectation or implement
validation. Fix by updating the test to call c.preRun(cmd, []string{}) (or nil)
and assert err == nil, or alternatively add argument checks inside the preRun
method of the <command>Cmd type to return an error for unexpected args; ensure
you reference the preRun function on the command instance c when making this
change.
- Around line 26-38: The scaffold references types and constructors from the
instances package (e.g., instances.Manager and instances.NewManager) but the
import block in this file is missing that package; add an import for the
instances package to the top-level import block and ensure any calls to
instances.NewManager(...) and the struct field using instances.Manager compile
against that package namespace so the generated code builds.
In `@skills/add-command-with-json/SKILL.md`:
- Around line 86-91: preRun currently validates args but never initializes the
command's manager, yet tests and later code dereference c.manager; modify preRun
to construct the manager (using the appropriate constructor or factory used
elsewhere in the repo) and assign it to c.manager before returning, handling and
logging any construction error and returning it so callers/tests see failures;
ensure you reference and set the struct field c.manager inside preRun so later
uses (e.g., the code paths around where c.manager is dereferenced) are valid.
- Around line 26-40: The import blocks are missing packages for referenced
symbols: add the package that defines instances.Manager (e.g., import
"your/module/pkg/instances" or the correct module path that exposes
instances.Manager) to the main file's imports so the type resolves, and add
"github.com/spf13/cobra" to the test files' import blocks so cobra.Command
references compile; update any existing import groups to include these entries
(referencing the type names instances.Manager and cobra.Command to locate the
spots to change).
In `@skills/add-parent-command/SKILL.md`:
- Around line 202-206: The test hard-codes expectedCount := 3 which conflicts
with the scaffolded template that provides 2 example commands; update the
assertion in Test<Parent>Cmd_Examples so expectedCount matches the template (set
expectedCount := 2) or derive the expected count from the actual examples slice
used to build commands (use len(examples) or similar) and keep the variables
expectedCount and commands consistent.
🪄 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
Run ID: a739e075-b86a-449b-b8ca-d7303c0d3a60
📒 Files selected for processing (8)
.claude/skills/add-alias-command.claude/skills/add-command-simple.claude/skills/add-command-with-json.claude/skills/add-parent-commandskills/add-alias-command/SKILL.mdskills/add-command-simple/SKILL.mdskills/add-command-with-json/SKILL.mdskills/add-parent-command/SKILL.md
The skill templates referenced the wrong module name (kortex-labs instead of kortex-hub) and the simple command skill was missing the instances package import. These errors would cause compilation failures when developers used the generated code templates. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Philippe Martin <phmartin@redhat.com>
The simple command skill had a test expecting argument validation in preRun, but the scaffolded preRun method didn't implement it. Added commented examples showing how to implement validation when needed, and clarified that for commands using Args: cobra.NoArgs, Cobra itself handles validation so custom preRun validation is not required. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Philippe Martin <phmartin@redhat.com>
The JSON command skill template was missing the instances package import in the main file and the cobra package import in the test file, causing compilation errors when developers used the generated code. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Philippe Martin <phmartin@redhat.com>
The JSON command template's preRun method was missing manager initialization code. The run method dereferences c.manager, but preRun never created or assigned it, which would cause nil pointer panics at runtime when developers used the generated code. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Philippe Martin <phmartin@redhat.com>
The parent command template's test expected 3 examples but the generic template only provides 2 example commands (--help and a subcommand example). Updated expectedCount from 3 to 2 to match the actual template. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Philippe Martin <phmartin@redhat.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@skills/add-command-simple/SKILL.md`:
- Around line 45-57: The Use string and Args validator are inconsistent: "Use:
\"<command> [args]\"" implies positional args but "Args: cobra.NoArgs" forbids
them; update either the Use/Example to reflect no positional args (remove
"[args]" and show only flags in Example) or change Args to a matching validator
such as cobra.ExactArgs(n) or cobra.MinimumNArgs(n) and update the Example
accordingly; ensure the same command declaration (fields Use, Args, Example, and
PreRunE: c.preRun) all consistently represent whether the command accepts
positional arguments.
🪄 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
Run ID: 5006ebd8-e68f-4389-8689-18f8f286d528
📒 Files selected for processing (3)
skills/add-command-simple/SKILL.mdskills/add-command-with-json/SKILL.mdskills/add-parent-command/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (1)
- skills/add-command-with-json/SKILL.md
Both command templates showed "Use: <command> [args]" but used Args: cobra.NoArgs, creating a conflict. Changed to ExactArgs(1) and updated all examples to include arg1 as a positional argument, making the Use field, Args validator, and examples consistent. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Philippe Martin <phmartin@redhat.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@skills/add-command-simple/SKILL.md`:
- Around line 26-29: The import block includes "path/filepath" but it is unused
causing a compile error; either delete the "filepath" import from the import
lists or actually use it by converting any path string to an absolute path
(e.g., call filepath.Abs on the path variable before using it) in the command
implementation. Locate the import blocks where "filepath" appears and remove
that entry, or update the command function that accepts/uses a path to call
filepath.Abs (and handle the returned path and error) so the import is used.
- Line 56: The Args validator is set to cobra.ExactArgs(1) but the E2E examples
run the command with no positional argument; update either the command
definition or the tests so they agree: either change the Args field (in the
command setup where Args: cobra.ExactArgs(1) appears) to cobra.NoArgs (or
another appropriate validator like cobra.MinimumNArgs(1)) if the command should
accept no args, or modify the E2E examples to pass the required single
positional argument to match ExactArgs(1) so tests and implementation are
consistent.
🪄 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
Run ID: 8eb779b9-d755-4fcc-8d41-0ba1184a5276
📒 Files selected for processing (2)
skills/add-command-simple/SKILL.mdskills/add-command-with-json/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (1)
- skills/add-command-with-json/SKILL.md
benoitf
left a comment
There was a problem hiding this comment.
I'm not sure to understand the difference between simple and with-json as to me it's only the output format (and all commands can support this output)
There are specific requisites to be sure that all output is in JSON format. I agree that all commands should be with JSON support, let's remove the simple command later if it is never used |
Add four new skills to help developers create different types of CLI commands following established patterns: commands with JSON output, simple text-only commands, alias commands, and parent commands with subcommands. Each skill includes step-by-step instructions, code templates, testing patterns, and references to existing implementations.
Part of #3