fix: install scripts/mcp/ for external research tools#95
Conversation
The setup wizard (claude_integration.py) and update wizard (update.py) install scripts from opc/scripts/ to ~/.claude/scripts/, but mcp/ was missing. This directory contains essential external API tools used by skills like /perplexity-search, /research-external, /firecrawl-scrape. Scripts in mcp/: - perplexity_search.py - AI web search/research - firecrawl_scrape.py - Web scraping - github_search.py - GitHub search - nia_docs.py - Documentation search - morph_apply.py, morph_search.py - Code search/apply This was an oversight from commit f36ce28 which added other script directories (core/, cc_math/, tldr/) but missed mcp/. ## Changes 1. Add .claude/scripts/mcp as symlink to ../../opc/scripts/mcp (avoids duplication since mcp/ scripts are standalone) 2. Update claude_integration.py to copy opc/scripts/mcp/ during install 3. Update update.py to include scripts/mcp in the checks list ## Before (fails from any project except CC) ``` $ cd ~/my-project && claude > /perplexity-search --search "test" Error: scripts/mcp/perplexity_search.py not found ``` ## After (works from anywhere) ``` $ cd ~/my-project && claude > /perplexity-search --search "test" ✓ Works - uses ~/.claude/scripts/mcp/perplexity_search.py ``` ## Architecture note Unlike core/ scripts which have separate opc/ (development) and .claude/ (distribution) versions, mcp/ scripts are standalone with no opc-specific dependencies. Using a symlink avoids maintaining duplicate files. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Skipped: This PR was not opened by one of your configured authors: ( |
📝 WalkthroughWalkthroughThe pull request introduces MCP (Model Context Protocol) script integration into the Claude setup process. A reference file is added, along with installation logic to copy MCP scripts from the OPC directory and update checking to track script changes. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 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 |
|
To be honest, it's really strange that we have so many files with the same name but mirror diffs in this repo. Can we make one refactor to clean them all? |
![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
🤖 Fix all issues with AI agents
In `@opc/scripts/setup/claude_integration.py`:
- Around line 522-527: The copy of opc_scripts_mcp to target_scripts_mcp can
fail if target_dir / "scripts" doesn't exist; before calling shutil.copytree for
opc_scripts_mcp (variables opc_scripts_mcp and target_scripts_mcp), ensure the
parent directory (target_scripts_mcp.parent or target_dir / "scripts") is
created with mkdir(parents=True, exist_ok=True); keep the existing logic that
removes an existing target with shutil.rmtree if present, then create the parent
and call shutil.copytree.
In `@opc/scripts/setup/update.py`:
- Around line 247-250: The parent "scripts" tuple currently overlaps with the
"scripts/mcp" tuple causing potential duplicate reports when compare_directories
uses recursive rglob; either remove the redundant ("scripts/mcp", claude_dir /
"scripts" / "mcp", {".py"}) tuple from the list if you don’t need separate
reporting, or alter the parent entry to only scan top-level files (e.g., change
the parent scan to non-recursive glob or add an exclusion for "scripts/mcp") so
compare_directories won’t double-count files.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (3)
.claude/scripts/mcpopc/scripts/setup/claude_integration.pyopc/scripts/setup/update.py
🔇 Additional comments (2)
opc/scripts/setup/claude_integration.py (1)
519-528: LGTM! MCP script copying follows established patterns.The implementation correctly mirrors the existing patterns for
core,cc_math, andtldrscript directories. Path resolution, directory cleanup, and script counting are all consistent..claude/scripts/mcp (1)
1-1: Symlink approach is correct. The relative path properly resolves from.claude/scripts/mcptoopc/scripts/mcp, and the target directory with all expected script files exists. This avoids maintaining duplicate script files.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| opc_scripts_mcp = opc_source.parent / "opc" / "scripts" / "mcp" | ||
| target_scripts_mcp = target_dir / "scripts" / "mcp" | ||
| if opc_scripts_mcp.exists(): | ||
| if target_scripts_mcp.exists(): | ||
| shutil.rmtree(target_scripts_mcp) | ||
| shutil.copytree(opc_scripts_mcp, target_scripts_mcp) |
There was a problem hiding this comment.
Edge case: Parent directory may not exist if scripts/core is absent.
The target_dir / "scripts" parent directory is only created at line 492-493 when opc_scripts_core.exists(). If scripts/core doesn't exist but scripts/mcp does, shutil.copytree will fail because the parent directory doesn't exist.
This is a pre-existing pattern issue, but since you're adding a new script directory, consider adding the parent mkdir here for robustness:
Proposed fix
opc_scripts_mcp = opc_source.parent / "opc" / "scripts" / "mcp"
target_scripts_mcp = target_dir / "scripts" / "mcp"
if opc_scripts_mcp.exists():
+ target_scripts_mcp.parent.mkdir(parents=True, exist_ok=True)
if target_scripts_mcp.exists():
shutil.rmtree(target_scripts_mcp)
shutil.copytree(opc_scripts_mcp, target_scripts_mcp)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| opc_scripts_mcp = opc_source.parent / "opc" / "scripts" / "mcp" | |
| target_scripts_mcp = target_dir / "scripts" / "mcp" | |
| if opc_scripts_mcp.exists(): | |
| if target_scripts_mcp.exists(): | |
| shutil.rmtree(target_scripts_mcp) | |
| shutil.copytree(opc_scripts_mcp, target_scripts_mcp) | |
| opc_scripts_mcp = opc_source.parent / "opc" / "scripts" / "mcp" | |
| target_scripts_mcp = target_dir / "scripts" / "mcp" | |
| if opc_scripts_mcp.exists(): | |
| target_scripts_mcp.parent.mkdir(parents=True, exist_ok=True) | |
| if target_scripts_mcp.exists(): | |
| shutil.rmtree(target_scripts_mcp) | |
| shutil.copytree(opc_scripts_mcp, target_scripts_mcp) |
🤖 Prompt for AI Agents
In `@opc/scripts/setup/claude_integration.py` around lines 522 - 527, The copy of
opc_scripts_mcp to target_scripts_mcp can fail if target_dir / "scripts" doesn't
exist; before calling shutil.copytree for opc_scripts_mcp (variables
opc_scripts_mcp and target_scripts_mcp), ensure the parent directory
(target_scripts_mcp.parent or target_dir / "scripts") is created with
mkdir(parents=True, exist_ok=True); keep the existing logic that removes an
existing target with shutil.rmtree if present, then create the parent and call
shutil.copytree.
| ("scripts", claude_dir / "scripts", {".py", ".sh"}), | ||
| ("scripts/core", claude_dir / "scripts" / "core", {".py"}), | ||
| ("scripts/mcp", claude_dir / "scripts" / "mcp", {".py"}), | ||
| ] |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Potential overlap between scripts and scripts/mcp checks.
The generic ("scripts", ...) entry at line 247 already scans for .py and .sh files recursively via rglob("*") in compare_directories. Adding scripts/mcp separately may cause files to be reported twice—once from the parent scripts check and once from the scripts/mcp check.
Consider whether this is intentional (for more granular status output) or if scripts/mcp should be excluded from the parent scan.
Option: Exclude subdirectories from parent scripts check
If you want separate reporting without double-counting, you could modify the scripts entry to exclude subdirectories that have their own entries, or remove the parent scripts entry entirely if all script subdirectories are listed individually.
🤖 Prompt for AI Agents
In `@opc/scripts/setup/update.py` around lines 247 - 250, The parent "scripts"
tuple currently overlaps with the "scripts/mcp" tuple causing potential
duplicate reports when compare_directories uses recursive rglob; either remove
the redundant ("scripts/mcp", claude_dir / "scripts" / "mcp", {".py"}) tuple
from the list if you don’t need separate reporting, or alter the parent entry to
only scan top-level files (e.g., change the parent scan to non-recursive glob or
add an exclusion for "scripts/mcp") so compare_directories won’t double-count
files.
2 participants
The setup wizard (claude_integration.py) and update wizard (update.py) install scripts from opc/scripts/ to ~/.claude/scripts/, but mcp/ was missing. This directory contains essential external API tools used by skills like /perplexity-search, /research-external, /firecrawl-scrape.
Scripts in mcp/:
This was an oversight from commit f36ce28 which added other script directories (core/, cc_math/, tldr/) but missed mcp/.
Changes
Add .claude/scripts/mcp as symlink to ../../opc/scripts/mcp (avoids duplication since mcp/ scripts are standalone)
Update claude_integration.py to copy opc/scripts/mcp/ during install
Update update.py to include scripts/mcp in the checks list
Before (fails from any project except CC)
After (works from anywhere)
Architecture note
Unlike core/ scripts which have separate opc/ (development) and .claude/ (distribution) versions, mcp/ scripts are standalone with no opc-specific dependencies. Using a symlink avoids maintaining duplicate files.
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.