-
Notifications
You must be signed in to change notification settings - Fork 1
✨ Detect and help clean up duplicate local command/agent/skill files #13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Add detection logic that identifies when users have both marketplace plugins AND local copies of the same commands, agents, or skills. When duplicates are found, the update command now educates users about the plugin marketplace approach and offers to remove duplicates while preserving any custom files. This solves the common confusion where users keep old local copies that override marketplace versions, leading to version drift and unexpected behavior. Users get to choose: embrace the marketplace for easier updates, or keep both. But now they're aware of what's happening and can make an informed decision. Bumps core plugin version to 2.1.0 to reflect this UX improvement. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
Code ReviewI've reviewed PR #13 and have the following feedback: Positive Aspects✅ Clear problem statement: The PR addresses a real issue where users have outdated local copies overriding marketplace plugins ✅ User education: Excellent explanation of the plugin marketplace benefits and why duplicates are problematic ✅ Safety-first approach: The logic preserves custom files and has a special exception for ✅ User control: Uses ✅ Semantic versioning: Appropriate minor version bump (2.0.1 → 2.1.0) for new functionality Concerns and Suggestions1. Path Construction Relies on Wildcards (Medium Priority)Lines 318-321 use wildcard patterns like: Issue: The prompt says "Get the plugin version from installed_plugins.json to construct the correct path" but then shows wildcard usage. This creates ambiguity for the LLM executing this command. Recommendation: Be more explicit about path construction: Read the version from installed_plugins.json for each plugin (core, agents, skills).
Construct exact paths using the version number:
- Commands: ~/.claude/plugins/cache/ai-coding-config/core/{version}/commands/
- Agents: ~/.claude/plugins/cache/ai-coding-config/agents/{version}/agents/
- Skills: ~/.claude/plugins/cache/ai-coding-config/skills/{version}/skills/
If the version isn't in installed_plugins.json or the path doesn't exist, fall back to using Glob with wildcards to find the most recent version.2. Symlink Detection Method Unclear (Low Priority)Lines 301-304 specify checking for "real directory, not symlink" but don't explain HOW to detect this. Recommendation: Add explicit detection method: 3. Error Handling Not Specified (Medium Priority)The cleanup section doesn't specify what to do if:
Recommendation: Add error handling guidance: <error-handling>
- If installed_plugins.json doesn't exist: Skip this section entirely (no plugins installed)
- If file removal fails: Report the error to user and continue with remaining files
- If marketplace cache paths don't exist: Warn user their plugin installation may be corrupt
</error-handling>4. Alignment with Project Guidelines (Low Priority)Per
The current prompt follows these well with "Use Glob to list files" - good adherence to project standards. 5. Token Efficiency (Minor)The user communication block (lines 331-362) is quite verbose. Per Current: ~450 words of explanation Security Considerations✅ Safe deletion strategy: Only removes exact filename matches from a known list One concern: The prompt doesn't explicitly warn against using Testing RecommendationsThe PR mentions testing on mcp-hubby project. Consider adding test cases for:
Minor: Version ConsistencyThe marketplace.json only bumps RecommendationApprove with suggestions. The core functionality is sound and addresses a real user problem. The concerns above are mostly about edge case handling and clarity for LLM execution. None are blocking issues. Consider addressing the path construction ambiguity (concern #1) and adding error handling guidance (concern #3) before merging, as these could lead to confusing behavior in edge cases. |
| For each directory type (commands, agents, skills): | ||
| 1. Use Glob to list files in the marketplace plugin install path | ||
| 2. Use Glob to list files in the local project directory | ||
| 3. Compare the two lists to identify exact filename matches |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Filename matching may delete user-customized file versions
The "Critical constraint" on line 308 states to "Only remove files that are duplicates of marketplace content" and "Preserve any custom commands, agents, or skills the user created themselves." However, the implementation on line 315 uses "exact filename matches" rather than content comparison. This means if a user customized a marketplace file (same filename, modified content), it would be identified as a duplicate and offered for removal, potentially causing data loss. Users seeing a list of "duplicates" may not realize their customized version is included since they might assume duplicates have identical content.
Summary
ai-coding-config.md(always preserved even if duplicate)Changes
The
/ai-coding-config updatecommand now includes a<local-duplicate-cleanup>section that:~/.claude/plugins/installed_plugins.jsonto check if marketplace plugins exist, then uses Glob to compare files in marketplace cache paths vs local project directoriesAskUserQuestionto let users choose between removing duplicates (recommended) or keeping both.claude/commands/ai-coding-config.mdThis solves a common problem where projects set up before the marketplace existed have outdated local copies that override marketplace versions, causing version drift and preventing auto-updates.
Testing
Can be tested on the mcp-hubby project, which has the exact situation this addresses: local copies of core commands/agents/skills alongside marketplace plugins.
Steps to verify:
.claude/files/ai-coding-config updatein mcp-hubbyai-coding-config.mdis always preserved🤖 Generated with Claude Code