feat(skills): add configure-ecc interactive installation wizard#147
feat(skills): add configure-ecc interactive installation wizard#147affaan-m merged 1 commit intoaffaan-m:mainfrom
Conversation
Add a new skill that guides users through selective installation of ECC skills and rules via AskUserQuestion. Clones the repo to /tmp, lets users choose components and install level (user/project), verifies path correctness, offers optimization, and cleans up on completion. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughA new SKILL.md file documents an interactive ECC installer with seven steps: repository cloning, installation level selection, skill selection with multi-category support, rule set installation, post-install verification, optional file optimization, and summary reporting, plus troubleshooting guidance. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8150ccb608
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| - Both: `TARGET_USER=~/.claude`, `TARGET_PROJECT=.claude` | ||
|
|
||
| Create the target directories if they don't exist: | ||
| ```bash | ||
| mkdir -p $TARGET/skills $TARGET/rules |
There was a problem hiding this comment.
When the user chooses "Both", you only set TARGET_USER/TARGET_PROJECT, but the subsequent install commands still use $TARGET (e.g., mkdir -p $TARGET/skills $TARGET/rules). That means $TARGET is unset in the “Both” path, so the directories and later copies run against an empty path (often creating /skills or failing). To avoid a broken “Both” install, the steps here need to branch and create/copy into both targets explicitly or set TARGET appropriately.
Useful? React with 👍 / 👎.
| | `backend-patterns` | Backend architecture, API design, server-side best practices for Node.js/Express/Next.js | | ||
| | `coding-standards` | Universal coding standards for TypeScript, JavaScript, React, Node.js | | ||
| | `django-patterns` | Django architecture, REST API with DRF, ORM, caching, signals, middleware | | ||
| | `django-security` | Django security: auth, CSRF, SQL injection, XSS prevention | | ||
| | `django-tdd` | Django testing with pytest-django, factory_boy, mocking, coverage | |
There was a problem hiding this comment.
The catalog claims 27 skills and lists multiple Django/Spring Boot/Java/Python skills (e.g., django-patterns, django-security, django-tdd) that are not present under skills/ in this repository. If a user selects these, the copy step (cp -r $ECC_ROOT/skills/<skill-name>) will fail and the verification section will produce false errors. The list should match actual directories in skills/ or the installer should dynamically enumerate existing skills.
Useful? React with 👍 / 👎.
|
@affaan-m This PR only resolves the installation issues of rules and skills, but does not address the configuration problems related to MCP and slash commands. Therefore, it needs further improvement and can be included in the future TODO plan. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@skills/configure-ecc/SKILL.md`:
- Around line 52-61: When INSTALL_LEVEL="Both" the document sets TARGET_USER and
TARGET_PROJECT but never TARGET, and later steps 2–5 reference $TARGET only;
update the install steps to handle Both by either (A) setting TARGET to a
primary location in the "Both" branch and explicitly performing secondary
actions against TARGET_USER/TARGET_PROJECT, or (B) change steps referenced by
functions/commands that use $TARGET (the copy/install/remove steps in Steps 2–5)
to detect INSTALL_LEVEL="Both" and loop over both TARGET_USER and TARGET_PROJECT
(e.g., iterate targets and perform the existing actions for each). Ensure all
references to TARGET in commands (cp/mkdir/ln/remove operations described in
Steps 2–5) are adjusted to use the chosen approach and clarify which components
go to user vs project targets.
🧹 Nitpick comments (4)
skills/configure-ecc/SKILL.md (4)
81-82: Clarify the skill confirmation flow for large lists.The instruction to "use
AskUserQuestionwith an 'Install all listed' option plus 'Other' for the user to paste specific names" is somewhat ambiguous. How would a user "paste specific names" throughAskUserQuestion? Consider providing a more concrete example of the question structure and options.💡 Suggested clarification
-For each selected category, print the full list of skills below and ask the user to confirm or deselect specific ones. If the list exceeds 4 items, print the list as text and use `AskUserQuestion` with an "Install all listed" option plus "Other" for the user to paste specific names. +For each selected category, print the full list of skills. Then use `AskUserQuestion` with: +- Option 1: "Install all [count] skills from this category" +- Option 2: "Let me choose specific skills" — then present a multi-select list of skill names
197-206: Add concrete verification commands for cross-reference checking.Step 4c describes which dependencies to verify but doesn't provide actual commands to perform these checks, unlike Steps 4a and 4b. This makes it difficult for the implementation to be consistent.
🔍 Suggested verification commands
Add concrete commands after the dependency list:
Verify these dependencies with targeted searches: ```bash # Check django-tdd → django-patterns reference if [ -d "$TARGET/skills/django-tdd" ]; then grep -q "django-patterns" "$TARGET/skills/django-tdd/SKILL.md" && \ [ ! -d "$TARGET/skills/django-patterns" ] && \ echo "⚠️ django-tdd references django-patterns but it's not installed" fi # Check springboot-tdd → springboot-patterns reference if [ -d "$TARGET/skills/springboot-tdd" ]; then grep -q "springboot-patterns" "$TARGET/skills/springboot-tdd/SKILL.md" && \ [ ! -d "$TARGET/skills/springboot-patterns" ] && \ echo "⚠️ springboot-tdd references springboot-patterns but it's not installed" fi # Check continuous-learning-v2 → homunculus directory if [ -d "$TARGET/skills/continuous-learning-v2" ]; then grep -q "homunculus" "$TARGET/skills/continuous-learning-v2/SKILL.md" && \ [ ! -d ~/.claude/homunculus ] && \ echo "ℹ️ continuous-learning-v2 requires ~/.claude/homunculus/ directory" fi # Similar checks for python-testing, golang-testing, language-specific rules...```
</details> --- `44-44`: **Consider adding language identifiers to code blocks.** The markdown linter suggests adding language identifiers to fenced code blocks for better rendering. For the pseudo-code/example blocks at these lines, consider using `text` or `yaml` identifiers. <details> <summary>📝 Example fix</summary> ```diff -``` +```text Question: "Where should ECC components be installed?" Options: - "User-level (~/.claude/)" — "Applies to all your Claude Code projects"Also applies to: 70-70, 146-146, 221-221, 260-260
83-83: Consider using proper markdown headings instead of bold text.The category labels (e.g., "Category: Framework & Language (16 skills)") at these lines use emphasis/bold rather than proper markdown headings, which may affect rendering and document navigation.
📝 Suggested fix
-**Category: Framework & Language (16 skills)** +#### Category: Framework & Language (16 skills)Apply similar changes to lines 104, 112, and 125.
Also applies to: 104-104, 112-112, 125-125
| Store the choice as `INSTALL_LEVEL`. Set the target directory: | ||
| - User-level: `TARGET=~/.claude` | ||
| - Project-level: `TARGET=.claude` (relative to current project root) | ||
| - Both: `TARGET_USER=~/.claude`, `TARGET_PROJECT=.claude` | ||
|
|
||
| Create the target directories if they don't exist: | ||
| ```bash | ||
| mkdir -p $TARGET/skills $TARGET/rules | ||
| ``` | ||
|
|
There was a problem hiding this comment.
Logic error: "Both" mode not properly handled in subsequent steps.
When the user selects "Both", lines 55-56 set TARGET_USER and TARGET_PROJECT, but do not set TARGET. However, all subsequent steps (2, 3, 4, 5) reference $TARGET exclusively, which will be undefined in "Both" mode, causing installation failures.
🔧 Suggested approach to fix "Both" mode
You need to handle "Both" mode consistently throughout the document. Here are two approaches:
Option 1: Modify Step 1 to set TARGET to a primary location in "Both" mode, then handle the secondary location separately in each step.
- Both: `TARGET_USER=~/.claude`, `TARGET_PROJECT=.claude`
+ Both: `TARGET=~/.claude`, `SECONDARY_TARGET=.claude` (common items to user-level, project-specific to project-level)Option 2: Update Steps 2-5 to check for "Both" mode and loop through both targets. For example, in Step 2c:
if [ "$INSTALL_LEVEL" = "Both" ]; then
for TARGET in "$TARGET_USER" "$TARGET_PROJECT"; do
cp -r $ECC_ROOT/skills/<skill-name> $TARGET/skills/
done
else
cp -r $ECC_ROOT/skills/<skill-name> $TARGET/skills/
fiYou'll need to apply similar logic to Steps 3, 4, and 5. Also clarify which components go where in "Both" mode (e.g., common skills to user-level, project-specific to project-level).
🤖 Prompt for AI Agents
In `@skills/configure-ecc/SKILL.md` around lines 52 - 61, When
INSTALL_LEVEL="Both" the document sets TARGET_USER and TARGET_PROJECT but never
TARGET, and later steps 2–5 reference $TARGET only; update the install steps to
handle Both by either (A) setting TARGET to a primary location in the "Both"
branch and explicitly performing secondary actions against
TARGET_USER/TARGET_PROJECT, or (B) change steps referenced by functions/commands
that use $TARGET (the copy/install/remove steps in Steps 2–5) to detect
INSTALL_LEVEL="Both" and loop over both TARGET_USER and TARGET_PROJECT (e.g.,
iterate targets and perform the existing actions for each). Ensure all
references to TARGET in commands (cp/mkdir/ln/remove operations described in
Steps 2–5) are adjusted to use the chosen approach and clarify which components
go to user vs project targets.
affaan-m
left a comment
affaan-m
left a comment
There was a problem hiding this comment.
Automated review: doc-only changes look good. Approving.
affaan-m
left a comment
affaan-m
left a comment
There was a problem hiding this comment.
✅ Approved!
Nice UX improvement! The interactive installation wizard addresses a real pain point.
Highlights:
- Step-by-step guided setup
- Uses
AskUserQuestionfor interactivity - Supports both user-level and project-level installation
- Includes verification and cleanup
Suggestion: Could be great to add this to the README's installation section as an alternative to manual setup.
Thanks for the contribution!
2 participants
Summary
configure-eccskill that provides an interactive, step-by-step installation wizard for the ECC project/tmp, usesAskUserQuestionto let users selectively install skills and rules to user-level (~/.claude/) or project-level (.claude/)/tmpon completionTest plan
skills/configure-ecc/SKILL.mdhas correct frontmatter (name, description)🤖 Generated with Claude Code
Summary by CodeRabbit