fix: MCP server bd executable path resolution
Fixes issue where MCP tools failed with "bd executable not found" when BEADS_PATH was set to command name instead of absolute path. Changes: - Remove BEADS_PATH=bd from plugin.json (use auto-detection) - Enhance config validator to resolve command names via PATH - Add comprehensive config validation tests (11 new tests) The validator now accepts both: - Absolute paths: /usr/local/bin/bd - Command names: bd (resolved via shutil.which) This makes the MCP server more robust and user-friendly while maintaining backward compatibility. All 90 tests pass. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -26,7 +26,6 @@
|
||||
"beads-mcp"
|
||||
],
|
||||
"env": {
|
||||
"BEADS_PATH": "bd",
|
||||
"BEADS_ACTOR": "${USER}"
|
||||
}
|
||||
}
|
||||
|
||||
@@ -43,21 +43,27 @@ class Config(BaseSettings):
|
||||
"""Validate BEADS_PATH points to an executable bd binary.
|
||||
|
||||
Args:
|
||||
v: Path to bd executable
|
||||
v: Path to bd executable (can be command name or absolute path)
|
||||
|
||||
Returns:
|
||||
Validated path
|
||||
Validated absolute path
|
||||
|
||||
Raises:
|
||||
ValueError: If path is invalid or not executable
|
||||
"""
|
||||
path = Path(v)
|
||||
|
||||
# If not an absolute/existing path, try to find it in PATH
|
||||
if not path.exists():
|
||||
raise ValueError(
|
||||
f"bd executable not found at: {v}\n"
|
||||
+ "Please verify BEADS_PATH points to a valid bd executable."
|
||||
)
|
||||
found = shutil.which(v)
|
||||
if found:
|
||||
v = found
|
||||
path = Path(v)
|
||||
else:
|
||||
raise ValueError(
|
||||
f"bd executable not found at: {v}\n"
|
||||
+ "Please verify BEADS_PATH points to a valid bd executable."
|
||||
)
|
||||
|
||||
if not os.access(v, os.X_OK):
|
||||
raise ValueError(
|
||||
|
||||
129
integrations/beads-mcp/tests/test_config.py
Normal file
129
integrations/beads-mcp/tests/test_config.py
Normal file
@@ -0,0 +1,129 @@
|
||||
"""Tests for beads_mcp.config module."""
|
||||
|
||||
import os
|
||||
import shutil
|
||||
from pathlib import Path
|
||||
from unittest.mock import patch
|
||||
|
||||
import pytest
|
||||
|
||||
from beads_mcp.config import Config, ConfigError, load_config
|
||||
|
||||
|
||||
class TestConfig:
|
||||
"""Tests for Config class."""
|
||||
|
||||
def test_default_beads_path_auto_detection(self, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""Test that bd is auto-detected from PATH when BEADS_PATH not set."""
|
||||
# Clear BEADS_PATH if set
|
||||
monkeypatch.delenv("BEADS_PATH", raising=False)
|
||||
|
||||
# Mock shutil.which to return a test path
|
||||
with patch("shutil.which", return_value="/usr/local/bin/bd"):
|
||||
config = Config()
|
||||
assert config.beads_path == "/usr/local/bin/bd"
|
||||
|
||||
def test_beads_path_from_env(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""Test that BEADS_PATH environment variable is respected."""
|
||||
# Create a fake bd executable
|
||||
bd_path = tmp_path / "bd"
|
||||
bd_path.touch(mode=0o755)
|
||||
|
||||
monkeypatch.setenv("BEADS_PATH", str(bd_path))
|
||||
config = Config()
|
||||
assert config.beads_path == str(bd_path)
|
||||
|
||||
def test_beads_path_command_name_resolution(self, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""Test that command names like 'bd' are resolved via PATH."""
|
||||
# Set BEADS_PATH to just "bd" (command name, not path)
|
||||
monkeypatch.setenv("BEADS_PATH", "bd")
|
||||
|
||||
# Mock shutil.which to simulate finding bd in PATH
|
||||
with patch("shutil.which", return_value="/usr/local/bin/bd"):
|
||||
# Mock os.access to say the file is executable
|
||||
with patch("os.access", return_value=True):
|
||||
config = Config()
|
||||
assert config.beads_path == "/usr/local/bin/bd"
|
||||
|
||||
def test_beads_path_not_found(self, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""Test that invalid BEADS_PATH raises ValueError."""
|
||||
monkeypatch.setenv("BEADS_PATH", "/nonexistent/bd")
|
||||
|
||||
with pytest.raises(ValueError, match="bd executable not found"):
|
||||
Config()
|
||||
|
||||
def test_beads_path_not_executable(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""Test that non-executable bd raises ValueError."""
|
||||
# Create a non-executable file
|
||||
bd_path = tmp_path / "bd"
|
||||
bd_path.touch(mode=0o644) # rw-r--r--
|
||||
|
||||
monkeypatch.setenv("BEADS_PATH", str(bd_path))
|
||||
|
||||
with pytest.raises(ValueError, match="not executable"):
|
||||
Config()
|
||||
|
||||
def test_beads_db_validation(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""Test that BEADS_DB must point to existing file."""
|
||||
# Create valid bd executable
|
||||
bd_path = tmp_path / "bd"
|
||||
bd_path.touch(mode=0o755)
|
||||
monkeypatch.setenv("BEADS_PATH", str(bd_path))
|
||||
|
||||
# Set BEADS_DB to non-existent file
|
||||
monkeypatch.setenv("BEADS_DB", "/nonexistent/db.sqlite")
|
||||
|
||||
with pytest.raises(ValueError, match="non-existent file"):
|
||||
Config()
|
||||
|
||||
def test_beads_db_none_allowed(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""Test that BEADS_DB can be unset (None)."""
|
||||
# Create valid bd executable
|
||||
bd_path = tmp_path / "bd"
|
||||
bd_path.touch(mode=0o755)
|
||||
monkeypatch.setenv("BEADS_PATH", str(bd_path))
|
||||
monkeypatch.delenv("BEADS_DB", raising=False)
|
||||
|
||||
config = Config()
|
||||
assert config.beads_db is None
|
||||
|
||||
def test_beads_actor_from_env(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""Test that BEADS_ACTOR is read from environment."""
|
||||
bd_path = tmp_path / "bd"
|
||||
bd_path.touch(mode=0o755)
|
||||
monkeypatch.setenv("BEADS_PATH", str(bd_path))
|
||||
monkeypatch.setenv("BEADS_ACTOR", "test-user")
|
||||
|
||||
config = Config()
|
||||
assert config.beads_actor == "test-user"
|
||||
|
||||
def test_auto_flags_default_false(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""Test that auto-flush and auto-import default to False."""
|
||||
bd_path = tmp_path / "bd"
|
||||
bd_path.touch(mode=0o755)
|
||||
monkeypatch.setenv("BEADS_PATH", str(bd_path))
|
||||
|
||||
config = Config()
|
||||
assert config.beads_no_auto_flush is False
|
||||
assert config.beads_no_auto_import is False
|
||||
|
||||
|
||||
class TestLoadConfig:
|
||||
"""Tests for load_config function."""
|
||||
|
||||
def test_load_config_success(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""Test that load_config returns valid Config."""
|
||||
bd_path = tmp_path / "bd"
|
||||
bd_path.touch(mode=0o755)
|
||||
monkeypatch.setenv("BEADS_PATH", str(bd_path))
|
||||
|
||||
config = load_config()
|
||||
assert isinstance(config, Config)
|
||||
assert config.beads_path == str(bd_path)
|
||||
|
||||
def test_load_config_error_handling(self, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""Test that load_config raises ConfigError with helpful message."""
|
||||
monkeypatch.setenv("BEADS_PATH", "/nonexistent/bd")
|
||||
|
||||
with pytest.raises(ConfigError, match="Configuration Error"):
|
||||
load_config()
|
||||
Reference in New Issue
Block a user