fix: resolve MCP binary paths to full absolute paths for PATH-independent startup#179
fix: resolve MCP binary paths to full absolute paths for PATH-independent startup#179marcusquinn merged 2 commits intomainfrom
Conversation
…dent startup OpenCode spawns MCP processes without inheriting shell profile PATH (~/.bun/bin not available). The previous migration from npx to bare binary names broke MCPs when launched from non-shell contexts. - Add resolve_mcp_binary_path() to check ~/.bun/bin, /opt/homebrew/bin, /usr/local/bin, ~/.local/bin, and ~/.npm-global/bin - Add update_mcp_paths_in_opencode() to rewrite all MCP commands to full paths in opencode.json - Update cleanup_deprecated_mcps migration to use full paths - Also resolve 'node' binary for node-script MCPs (quickfile, etc.) - Run path resolution both during MCP install and deprecated cleanup
Summary of ChangesHello @marcusquinn, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a critical issue preventing MCP servers from starting reliably in OpenCode, particularly in non-shell environments where the system's PATH variable might not include necessary binary locations. By implementing robust binary path resolution and persisting these absolute paths directly into the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds two helpers— Changes
Sequence Diagram(s)sequenceDiagram
participant Setup as Setup Script
participant FS as Filesystem
participant PATH as System PATH
participant OC as opencode.json
Setup->>FS: Check common install locations for <binary>
alt found in common location
FS-->>Setup: Return full path
else not found
Setup->>PATH: fallback `which` lookup
PATH-->>Setup: Return full path or empty
end
Setup->>OC: Read MCP entries
Setup->>Setup: Replace command[0] with resolved full path
Setup->>OC: Write updated opencode.json (if changed)
OC-->>Setup: Acknowledge update
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Sat Jan 24 19:22:42 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request is a great improvement for making MCP startups more robust by resolving binary paths to absolute paths. The new helper functions resolve_mcp_binary_path and update_mcp_paths_in_opencode are well-structured and handle many cases, including stale paths. I've found a couple of areas for improvement: one is a small refactoring to avoid a duplicate jq call and improve security in cleanup_deprecated_mcps, and the other is a potential bug in update_mcp_paths_in_opencode where some interpreters like python are skipped from path resolution, which seems to contradict the PR's goal. Overall, these are solid changes.
setup.sh
Outdated
|
|
||
| # Skip special commands (docker, node, etc.) | ||
| case "$current_cmd" in | ||
| docker|node|python|python3|bash|sh) continue ;; |
There was a problem hiding this comment.
The case statement skips resolving paths for python, python3, bash, and sh. This seems to contradict the goal of this pull request, which is to make MCP startup PATH-independent by using full absolute paths. If an MCP command uses a bare python3, it will not be resolved and might fail if python3 is not in the PATH of the spawned process. Since node is handled specially, these other interpreters could be removed from this case statement to be handled by the general path resolution logic.
| docker|node|python|python3|bash|sh) continue ;; | |
| docker|node) continue ;; |
setup.sh
Outdated
| if jq -e ".mcp | to_entries[] | select(.value.command != null) | select(.value.command | join(\" \") | test(\"npx.*${pkg}|bunx.*${pkg}|pipx.*run.*${pkg}\"))" "$tmp_config" > /dev/null 2>&1; then | ||
| if command -v "$bin_name" &> /dev/null; then | ||
| # Find the MCP key and update its command to bare binary | ||
| # Resolve full path for the binary | ||
| local full_path | ||
| full_path=$(resolve_mcp_binary_path "$bin_name") | ||
| if [[ -n "$full_path" ]]; then | ||
| # Find the MCP key and update its command to full path | ||
| local mcp_key | ||
| mcp_key=$(jq -r ".mcp | to_entries[] | select(.value.command != null) | select(.value.command | join(\" \") | test(\"npx.*${pkg}|bunx.*${pkg}|pipx.*run.*${pkg}\")) | .key" "$tmp_config" 2>/dev/null | head -1) | ||
| if [[ -n "$mcp_key" ]]; then | ||
| # Preserve --mcp flag for repomix | ||
| if [[ "$bin_name" == "repomix" ]]; then | ||
| jq ".mcp[\"$mcp_key\"].command = [\"$bin_name\", \"--mcp\"]" "$tmp_config" > "${tmp_config}.new" && mv "${tmp_config}.new" "$tmp_config" | ||
| jq --arg p "$full_path" ".mcp[\"$mcp_key\"].command = [\$p, \"--mcp\"]" "$tmp_config" > "${tmp_config}.new" && mv "${tmp_config}.new" "$tmp_config" | ||
| else | ||
| jq ".mcp[\"$mcp_key\"].command = [\"$bin_name\"]" "$tmp_config" > "${tmp_config}.new" && mv "${tmp_config}.new" "$tmp_config" | ||
| jq --arg p "$full_path" ".mcp[\"$mcp_key\"].command = [\$p]" "$tmp_config" > "${tmp_config}.new" && mv "${tmp_config}.new" "$tmp_config" | ||
| fi | ||
| ((cleaned++)) | ||
| fi | ||
| fi | ||
| fi |
There was a problem hiding this comment.
This block can be made more efficient and safer. It currently executes the same jq query twice to check for existence and then to get the key. Also, it uses shell variable interpolation ("$mcp_key") inside the jq script, which is not recommended. You can refactor this to run the query only once and use jq's --arg to pass the key safely.
| if jq -e ".mcp | to_entries[] | select(.value.command != null) | select(.value.command | join(\" \") | test(\"npx.*${pkg}|bunx.*${pkg}|pipx.*run.*${pkg}\"))" "$tmp_config" > /dev/null 2>&1; then | |
| if command -v "$bin_name" &> /dev/null; then | |
| # Find the MCP key and update its command to bare binary | |
| # Resolve full path for the binary | |
| local full_path | |
| full_path=$(resolve_mcp_binary_path "$bin_name") | |
| if [[ -n "$full_path" ]]; then | |
| # Find the MCP key and update its command to full path | |
| local mcp_key | |
| mcp_key=$(jq -r ".mcp | to_entries[] | select(.value.command != null) | select(.value.command | join(\" \") | test(\"npx.*${pkg}|bunx.*${pkg}|pipx.*run.*${pkg}\")) | .key" "$tmp_config" 2>/dev/null | head -1) | |
| if [[ -n "$mcp_key" ]]; then | |
| # Preserve --mcp flag for repomix | |
| if [[ "$bin_name" == "repomix" ]]; then | |
| jq ".mcp[\"$mcp_key\"].command = [\"$bin_name\", \"--mcp\"]" "$tmp_config" > "${tmp_config}.new" && mv "${tmp_config}.new" "$tmp_config" | |
| jq --arg p "$full_path" ".mcp[\"$mcp_key\"].command = [\$p, \"--mcp\"]" "$tmp_config" > "${tmp_config}.new" && mv "${tmp_config}.new" "$tmp_config" | |
| else | |
| jq ".mcp[\"$mcp_key\"].command = [\"$bin_name\"]" "$tmp_config" > "${tmp_config}.new" && mv "${tmp_config}.new" "$tmp_config" | |
| jq --arg p "$full_path" ".mcp[\"$mcp_key\"].command = [\$p]" "$tmp_config" > "${tmp_config}.new" && mv "${tmp_config}.new" "$tmp_config" | |
| fi | |
| ((cleaned++)) | |
| fi | |
| fi | |
| fi | |
| # Find MCP key using npx/bunx/pipx for this package | |
| local mcp_key | |
| mcp_key=$(jq -r ".mcp | to_entries[] | select(.value.command != null) | select(.value.command | join(\" \") | test(\"npx.*${pkg}|bunx.*${pkg}|pipx.*run.*${pkg}\")) | .key" "$tmp_config" 2>/dev/null | head -1) | |
| if [[ -n "$mcp_key" ]]; then | |
| # Resolve full path for the binary | |
| local full_path | |
| full_path=$(resolve_mcp_binary_path "$bin_name") | |
| if [[ -n "$full_path" ]]; then | |
| # Preserve --mcp flag for repomix | |
| if [[ "$bin_name" == "repomix" ]]; then | |
| jq --arg k "$mcp_key" --arg p "$full_path" '.mcp[$k].command = [$p, "--mcp"]' "$tmp_config" > "${tmp_config}.new" && mv "${tmp_config}.new" "$tmp_config" | |
| else | |
| jq --arg k "$mcp_key" --arg p "$full_path" '.mcp[$k].command = [$p]' "$tmp_config" > "${tmp_config}.new" && mv "${tmp_config}.new" "$tmp_config" | |
| fi | |
| ((cleaned++)) | |
| fi | |
| fi |
![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 `@setup.sh`:
- Around line 2219-2248: The resolve_mcp_binary_path function should use the
portable builtin command -v instead of which and explicitly return success;
replace the fallback assignment using which (inside resolve_mcp_binary_path)
with a command -v invocation (e.g., capture output of command -v "$bin_name"
2>/dev/null || true) and add an explicit "return 0" at the end of
resolve_mcp_binary_path so the function always returns success when finished.
…preters - Replace 'which' with 'command -v' (POSIX-portable) - Add explicit 'return 0' to resolve_mcp_binary_path - Remove python/bash/sh from skip list so all interpreters get resolved - Optimize cleanup_deprecated_mcps: single jq query instead of check+get - Use --arg for all jq key interpolation (safer than string interpolation)
|
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Sat Jan 24 19:28:25 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
Bash associative arrays mishandle @ in keys (e.g. @steipete/claude-code-mcp) during expansion. Using parallel indexed arrays and jq --arg for safe regex matching avoids both the bash @ conflict and regex escaping issues. Fixes syntax error at setup.sh:233 introduced in PR #179. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 participant
Summary
repomix,chrome-devtools-mcp) can't be found when~/.bun/binisn't in the spawned process's PATHresolve_mcp_binary_path()helper that checks~/.bun/bin,/opt/homebrew/bin,/usr/local/bin,~/.local/bin, and~/.npm-global/binupdate_mcp_paths_in_opencode()to rewrite all MCP commands inopencode.jsonto use full absolute pathsRoot Cause
The v2.79 migration (
perf: install MCP packages globally for instant startup) correctly installed packages globally viabun install -g, but the config migration wrote bare binary names. OpenCode usesStdioClientTransportwhich spawns processes withprocess.env- when launched from a non-shell context (desktop app, launchd, spotlight),~/.bun/binisn't in PATH.Changes
resolve_mcp_binary_path()update_mcp_paths_in_opencode()cleanup_deprecated_mcps()install_mcp_packages()Testing
Verified all 12 local MCPs resolve correctly:
~/.bun/bin/for bun-installed packages (chrome-devtools-mcp, repomix, playwriter, etc.)/opt/homebrew/bin/for brew-installed packages (osgrep, auggie, mcp-local-wp)~/.local/bin/for pipx/uv-installed packages (analytics-mcp, outscraper-mcp-server)/opt/homebrew/bin/nodefor node-script MCPs (quickfile, amazon-order-history)Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.