From 53f7165844caea7106ebf7b7467ca8744af91a64 Mon Sep 17 00:00:00 2001 From: Peter Pak Date: Wed, 22 Apr 2026 15:33:56 -0400 Subject: [PATCH 1/2] Resolves #19 and #22. --- README.md | 38 ++++++++++++++++++++++++++++++++---- src/rocketsmith/mcp/setup.py | 4 ++-- src/rocketsmith/mcp/utils.py | 17 +++++++++++++++- 3 files changed, 52 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 49e0131..34dcc5d 100644 --- a/README.md +++ b/README.md @@ -11,22 +11,34 @@ RocketSmith is an end-to-end model-rocket toolchain exposed as an MCP extension. ## Install -### Claude Code (plugin) +**Prerequisites (all platforms):** +- [uv](https://docs.astral.sh/uv/getting-started/installation/) — RocketSmith uses `uv` to manage its Python environment. Install it before proceeding. +- A GitHub SSH key added to your account — required to clone the plugin repository. See [Generating a new SSH key](https://docs.github.com/en/authentication/connecting-to-github-with-ssh/generating-a-new-ssh-key-and-adding-it-to-the-ssh-agent). -**Prerequisites:** Create a GitHub SSH key and add it to your account before proceeding — see [Generating a new SSH key](https://docs.github.com/en/authentication/connecting-to-github-with-ssh/generating-a-new-ssh-key-and-adding-it-to-the-ssh-agent). +### Claude Code (plugin) -Inside a Claude Code session, add the marketplace and install the plugin: +Inside a Claude Code session, add the marketplace and install the plugin at **user scope**: ``` /plugin marketplace add ppak10/RocketSmith ``` ``` -/plugin install rocketsmith@rocketsmith +/plugin install rocketsmith@rocketsmith --scope user ``` After installing, reload plugins and verify the MCP server is connected with `/reload-plugins` and `/mcp` +**Updating to a new version:** Uninstall the existing plugin first, then reinstall: + +``` +/plugin uninstall rocketsmith@rocketsmith +``` + +``` +/plugin install rocketsmith@rocketsmith --scope user +``` + ### [Gemini CLI (extension)](https://geminicli.com/extensions/?name=ppak10RocketSmith) ```bash @@ -41,6 +53,24 @@ gemini extensions install https://github.com/ppak10/RocketSmith See [Installation](https://github.com/ppak10/RocketSmith/wiki/Installation) for platform-specific setup and troubleshooting. +## Troubleshooting + +### Windows — OpenRocket calls are very slow or appear to hang + +OpenRocket runs on a JVM. On Windows, the first `openrocket_*` call in a session pays a JVM cold-start cost of **67 seconds to 6 minutes**. This is normal — the tool will eventually return. Do not cancel the call or retry early; let it finish. Subsequent calls in the same session are faster once the JVM is warm. + +### Windows — `rocketsmith_setup` fails with `[WinError 87] The parameter is incorrect` + +This was a bug in path resolution on Windows where `Path.resolve()` raised `OSError` for certain path types. It is fixed in the current version. If you hit it on an older version, call `rocketsmith_setup(action="check")` first (no `project_dir`), confirm `ready: true`, then call again with `project_dir`. + +### Claude Code — agent continuation fails ("SendMessage not found") + +Claude Code requires **v2.1.77 or later** for inter-agent `SendMessage` to work. On older versions, attempting to continue a spawned subagent by `agentId` will fail because the tool doesn't exist yet. Update Claude Code to the latest release to resolve this. + +### MCP server disconnects mid-session + +Rejecting a pending rocketsmith tool call can cause the MCP server to disconnect, disabling all rocketsmith tools for the rest of the session. If this happens, restart the MCP server: in Claude Code run `/mcp`, find the rocketsmith server, and reconnect. In Gemini CLI, restart the session. + ## Documentation - [Home](https://github.com/ppak10/RocketSmith/wiki/Home) — pipeline overview, domain agents, MCP tool list diff --git a/src/rocketsmith/mcp/setup.py b/src/rocketsmith/mcp/setup.py index 5aea65c..e08a856 100644 --- a/src/rocketsmith/mcp/setup.py +++ b/src/rocketsmith/mcp/setup.py @@ -105,10 +105,10 @@ def rocketsmith_setup( gui_error = None if project_dir is not None: - from rocketsmith.mcp.utils import set_project_dir + from rocketsmith.mcp.utils import safe_resolve, set_project_dir set_project_dir(project_dir) - gui_url, gui_pid, gui_error = _start_gui(project_dir.resolve()) + gui_url, gui_pid, gui_error = _start_gui(safe_resolve(project_dir)) if action == "check": status = _check() diff --git a/src/rocketsmith/mcp/utils.py b/src/rocketsmith/mcp/utils.py index 00e4820..95743ff 100644 --- a/src/rocketsmith/mcp/utils.py +++ b/src/rocketsmith/mcp/utils.py @@ -19,6 +19,21 @@ def _cleanup_pid_file() -> None: pass +def safe_resolve(path: Path) -> Path: + """Resolve a path to absolute form without raising on Windows. + + ``Path.resolve()`` on Windows calls ``GetFinalPathNameByHandle``, which can + raise ``OSError: [WinError 87] The parameter is incorrect`` for certain path + types (long paths, paths with trailing characters, non-existent roots). + ``os.path.abspath`` performs the same ``..``-collapse and absolutisation + without that API call and is safe on all platforms. + """ + try: + return path.resolve() + except OSError: + return Path(os.path.abspath(path)) + + def set_project_dir(path: Path) -> None: """Persist the project directory for this MCP server process (PID-scoped). @@ -28,7 +43,7 @@ def set_project_dir(path: Path) -> None: """ global _atexit_registered _ROCKETSMITH_DIR.mkdir(parents=True, exist_ok=True) - _pid_file().write_text(str(path.resolve())) + _pid_file().write_text(str(safe_resolve(path))) if not _atexit_registered: atexit.register(_cleanup_pid_file) _atexit_registered = True From 6455e4dbdcaa4845c74330245df242d2c5f4f2d2 Mon Sep 17 00:00:00 2001 From: Peter Pak Date: Wed, 22 Apr 2026 15:43:56 -0400 Subject: [PATCH 2/2] Adds tests for mc setup and utils. --- tests/mcp_server/test_setup.py | 313 +++++++++++++++++++++++++++++++++ tests/mcp_server/test_utils.py | 199 +++++++++++++++++++++ 2 files changed, 512 insertions(+) create mode 100644 tests/mcp_server/test_setup.py create mode 100644 tests/mcp_server/test_utils.py diff --git a/tests/mcp_server/test_setup.py b/tests/mcp_server/test_setup.py new file mode 100644 index 0000000..3f9474b --- /dev/null +++ b/tests/mcp_server/test_setup.py @@ -0,0 +1,313 @@ +"""Tests for rocketsmith.mcp.setup.""" + +from pathlib import Path +from unittest.mock import MagicMock, patch + +import pytest + +import rocketsmith.mcp.setup as setup_mod +from rocketsmith.mcp.setup import DependencyStatus, _check, _start_gui, register_setup + + +# ── helpers ─────────────────────────────────────────────────────────────────── + + +def _all_found_status(**overrides): + defaults = dict( + java="installed (/usr/bin/java)", + openrocket="installed (/opt/OpenRocket.jar)", + prusaslicer="installed (/usr/bin/prusa-slicer)", + ready=True, + ) + defaults.update(overrides) + return DependencyStatus(**defaults) + + +def _missing_status(**overrides): + defaults = dict( + java="not found", + openrocket="not found", + prusaslicer="not found", + ready=False, + ) + defaults.update(overrides) + return DependencyStatus(**defaults) + + +# ── _check ──────────────────────────────────────────────────────────────────── + + +def test_check_all_installed(monkeypatch): + monkeypatch.setattr( + "rocketsmith.mcp.setup.get_openrocket_jvm", + lambda _: "/usr/bin/java", + raising=False, + ) + with ( + patch( + "rocketsmith.openrocket.utils.get_openrocket_jvm", + return_value="/usr/bin/java", + ), + patch( + "rocketsmith.openrocket.utils.get_openrocket_path", + return_value=Path("/opt/OpenRocket.jar"), + ), + patch( + "rocketsmith.prusaslicer.utils.get_prusaslicer_path", + return_value=Path("/usr/bin/prusa-slicer"), + ), + ): + status = _check() + + assert status.ready is True + assert "installed" in status.java + assert "installed" in status.openrocket + assert "installed" in status.prusaslicer + + +def test_check_java_missing(monkeypatch): + with ( + patch("rocketsmith.openrocket.utils.get_openrocket_jvm", return_value=None), + patch( + "rocketsmith.openrocket.utils.get_openrocket_path", + return_value=Path("/opt/OpenRocket.jar"), + ), + patch( + "rocketsmith.prusaslicer.utils.get_prusaslicer_path", + return_value=Path("/usr/bin/prusa-slicer"), + ), + ): + status = _check() + + assert status.ready is False + assert status.java == "not found" + + +def test_check_openrocket_missing(monkeypatch): + with ( + patch( + "rocketsmith.openrocket.utils.get_openrocket_jvm", + return_value="/usr/bin/java", + ), + patch( + "rocketsmith.openrocket.utils.get_openrocket_path", + side_effect=FileNotFoundError, + ), + patch( + "rocketsmith.prusaslicer.utils.get_prusaslicer_path", + return_value=Path("/usr/bin/prusa-slicer"), + ), + ): + status = _check() + + assert status.ready is False + assert status.openrocket == "not found" + + +def test_check_prusaslicer_missing(): + with ( + patch( + "rocketsmith.openrocket.utils.get_openrocket_jvm", + return_value="/usr/bin/java", + ), + patch( + "rocketsmith.openrocket.utils.get_openrocket_path", + return_value=Path("/opt/OpenRocket.jar"), + ), + patch( + "rocketsmith.prusaslicer.utils.get_prusaslicer_path", + side_effect=FileNotFoundError, + ), + ): + status = _check() + + assert status.ready is False + assert status.prusaslicer == "not found" + + +def test_check_ready_false_when_all_missing(): + with ( + patch("rocketsmith.openrocket.utils.get_openrocket_jvm", return_value=None), + patch( + "rocketsmith.openrocket.utils.get_openrocket_path", + side_effect=FileNotFoundError, + ), + patch( + "rocketsmith.prusaslicer.utils.get_prusaslicer_path", + side_effect=FileNotFoundError, + ), + ): + status = _check() + + assert status.ready is False + + +# ── _start_gui ──────────────────────────────────────────────────────────────── + + +def test_start_gui_success(tmp_path, monkeypatch): + monkeypatch.setattr(setup_mod, "_gui_teardown_registered", False) + mock_start = MagicMock( + return_value={"server_url": "http://localhost:5000", "pid": 1234} + ) + with patch("rocketsmith.gui.lifecycle.start_gui_server", mock_start): + url, pid, error = _start_gui(tmp_path) + + assert url == "http://localhost:5000" + assert pid == 1234 + assert error is None + + +def test_start_gui_returns_error_on_failure(tmp_path, monkeypatch): + monkeypatch.setattr(setup_mod, "_gui_teardown_registered", False) + mock_start = MagicMock(return_value={"error": "bundle not found", "pid": None}) + with patch("rocketsmith.gui.lifecycle.start_gui_server", mock_start): + url, pid, error = _start_gui(tmp_path) + + assert url is None + assert pid is None + assert error == "bundle not found" + + +def test_start_gui_registers_atexit_once(tmp_path, monkeypatch): + monkeypatch.setattr(setup_mod, "_gui_teardown_registered", False) + mock_start = MagicMock( + return_value={"server_url": "http://localhost:5000", "pid": 99} + ) + + with patch("rocketsmith.gui.lifecycle.start_gui_server", mock_start): + with patch("atexit.register") as mock_atexit: + _start_gui(tmp_path) + _start_gui(tmp_path) + + assert mock_atexit.call_count == 1 + + +# ── rocketsmith_setup (via register_setup) ──────────────────────────────────── + + +@pytest.fixture +def setup_tool(monkeypatch): + """Return the unwrapped rocketsmith_setup callable.""" + app = MagicMock() + captured = {} + + def fake_tool(**kwargs): + def decorator(fn): + captured["fn"] = fn + return fn + + return decorator + + app.tool = fake_tool + register_setup(app) + return captured["fn"] + + +def test_setup_check_no_project_dir(setup_tool, monkeypatch): + with patch("rocketsmith.mcp.setup._check", return_value=_all_found_status()): + status = setup_tool(action="check", project_dir=None) + + assert status.ready is True + assert status.gui_url is None + assert status.gui_pid is None + assert status.gui_error is None + + +def test_setup_check_with_project_dir(tmp_path, setup_tool, monkeypatch): + monkeypatch.setattr(setup_mod, "_gui_teardown_registered", False) + + import rocketsmith.mcp.utils as utils_mod + + rocketsmith_dir = tmp_path / ".rocketsmith" + monkeypatch.setattr(utils_mod, "_ROCKETSMITH_DIR", rocketsmith_dir) + monkeypatch.setattr(utils_mod, "_atexit_registered", False) + + mock_gui = MagicMock(return_value=("http://localhost:5000", 42, None)) + + with ( + patch("rocketsmith.mcp.setup._check", return_value=_all_found_status()), + patch("rocketsmith.mcp.setup._start_gui", mock_gui), + ): + status = setup_tool(action="check", project_dir=tmp_path) + + assert status.gui_url == "http://localhost:5000" + assert status.gui_pid == 42 + assert status.gui_error is None + + +def test_setup_check_propagates_gui_error(tmp_path, setup_tool, monkeypatch): + monkeypatch.setattr(setup_mod, "_gui_teardown_registered", False) + + import rocketsmith.mcp.utils as utils_mod + + rocketsmith_dir = tmp_path / ".rocketsmith" + monkeypatch.setattr(utils_mod, "_ROCKETSMITH_DIR", rocketsmith_dir) + monkeypatch.setattr(utils_mod, "_atexit_registered", False) + + mock_gui = MagicMock(return_value=(None, None, "bundle not found")) + + with ( + patch("rocketsmith.mcp.setup._check", return_value=_all_found_status()), + patch("rocketsmith.mcp.setup._start_gui", mock_gui), + ): + status = setup_tool(action="check", project_dir=tmp_path) + + assert status.gui_url is None + assert status.gui_error == "bundle not found" + + +def test_setup_install_calls_installers_when_missing(setup_tool, monkeypatch): + missing = _missing_status() + ready = _all_found_status() + + mock_install_or = MagicMock() + mock_install_ps = MagicMock() + + with ( + patch("rocketsmith.mcp.setup._check", side_effect=[missing, ready]), + patch("rocketsmith.openrocket.install.install", mock_install_or), + patch("rocketsmith.prusaslicer.install.install", mock_install_ps), + ): + status = setup_tool(action="install", project_dir=None) + + mock_install_or.assert_called_once() + mock_install_ps.assert_called_once() + assert status.ready is True + + +def test_setup_install_skips_installers_when_all_present(setup_tool): + found = _all_found_status() + + mock_install_or = MagicMock() + mock_install_ps = MagicMock() + + with ( + patch("rocketsmith.mcp.setup._check", side_effect=[found, found]), + patch("rocketsmith.openrocket.install.install", mock_install_or), + patch("rocketsmith.prusaslicer.install.install", mock_install_ps), + ): + setup_tool(action="install", project_dir=None) + + mock_install_or.assert_not_called() + mock_install_ps.assert_not_called() + + +def test_setup_install_only_installs_openrocket_when_prusaslicer_present(setup_tool): + partial = _missing_status( + prusaslicer="installed (/usr/bin/prusa-slicer)", ready=False + ) + ready = _all_found_status() + + mock_install_or = MagicMock() + mock_install_ps = MagicMock() + + with ( + patch("rocketsmith.mcp.setup._check", side_effect=[partial, ready]), + patch("rocketsmith.openrocket.install.install", mock_install_or), + patch("rocketsmith.prusaslicer.install.install", mock_install_ps), + ): + setup_tool(action="install", project_dir=None) + + mock_install_or.assert_called_once() + mock_install_ps.assert_not_called() diff --git a/tests/mcp_server/test_utils.py b/tests/mcp_server/test_utils.py new file mode 100644 index 0000000..0a2f305 --- /dev/null +++ b/tests/mcp_server/test_utils.py @@ -0,0 +1,199 @@ +"""Tests for rocketsmith.mcp.utils.""" + +import os +from pathlib import Path +from unittest.mock import patch + +import pytest + +from rocketsmith.mcp.utils import ( + get_project_dir, + resolve_path, + safe_resolve, + set_project_dir, + tool_error, + tool_success, +) +from rocketsmith.mcp.types import ToolError, ToolSuccess + + +# ── safe_resolve ────────────────────────────────────────────────────────────── + + +def test_safe_resolve_returns_absolute_path(tmp_path): + p = tmp_path / "subdir" + p.mkdir() + result = safe_resolve(p) + assert result.is_absolute() + + +def test_safe_resolve_collapses_dotdot(tmp_path): + p = tmp_path / "a" / ".." / "b" + result = safe_resolve(p) + assert ".." not in result.parts + + +def test_safe_resolve_falls_back_on_oserror(tmp_path): + p = tmp_path / "some" / "path" + with patch.object(Path, "resolve", side_effect=OSError("WinError 87")): + result = safe_resolve(p) + assert result.is_absolute() + assert isinstance(result, Path) + + +def test_safe_resolve_fallback_matches_abspath(tmp_path): + p = tmp_path / "some" / "path" + expected = Path(os.path.abspath(p)) + with patch.object(Path, "resolve", side_effect=OSError): + result = safe_resolve(p) + assert result == expected + + +# ── set_project_dir / get_project_dir ───────────────────────────────────────── + + +def test_set_and_get_project_dir(tmp_path, monkeypatch): + monkeypatch.delenv("ROCKETSMITH_PROJECT_DIR", raising=False) + + import rocketsmith.mcp.utils as utils_mod + + rocketsmith_dir = tmp_path / ".rocketsmith" + monkeypatch.setattr(utils_mod, "_ROCKETSMITH_DIR", rocketsmith_dir) + monkeypatch.setattr(utils_mod, "_atexit_registered", False) + + project = tmp_path / "myproject" + project.mkdir() + + set_project_dir(project) + + result = get_project_dir() + assert result == project.resolve() + + +def test_set_project_dir_creates_rocketsmith_dir(tmp_path, monkeypatch): + monkeypatch.delenv("ROCKETSMITH_PROJECT_DIR", raising=False) + + import rocketsmith.mcp.utils as utils_mod + + rocketsmith_dir = tmp_path / ".rocketsmith" + monkeypatch.setattr(utils_mod, "_ROCKETSMITH_DIR", rocketsmith_dir) + monkeypatch.setattr(utils_mod, "_atexit_registered", False) + + project = tmp_path / "proj" + project.mkdir() + + assert not rocketsmith_dir.exists() + set_project_dir(project) + assert rocketsmith_dir.exists() + + +def test_get_project_dir_prefers_env_var(tmp_path, monkeypatch): + env_dir = tmp_path / "env_project" + env_dir.mkdir() + monkeypatch.setenv("ROCKETSMITH_PROJECT_DIR", str(env_dir)) + + result = get_project_dir() + assert result == env_dir.resolve() + + +def test_get_project_dir_ignores_unresolved_env_substitution(tmp_path, monkeypatch): + monkeypatch.setenv("ROCKETSMITH_PROJECT_DIR", "${cwd}/project") + + import rocketsmith.mcp.utils as utils_mod + + rocketsmith_dir = tmp_path / ".rocketsmith" + monkeypatch.setattr(utils_mod, "_ROCKETSMITH_DIR", rocketsmith_dir) + # No pid file written → falls back to cwd + result = get_project_dir() + assert "${" not in str(result) + + +def test_get_project_dir_falls_back_to_cwd(tmp_path, monkeypatch): + monkeypatch.delenv("ROCKETSMITH_PROJECT_DIR", raising=False) + + import rocketsmith.mcp.utils as utils_mod + + # Point to a dir with no pid file + rocketsmith_dir = tmp_path / ".rocketsmith_empty" + monkeypatch.setattr(utils_mod, "_ROCKETSMITH_DIR", rocketsmith_dir) + + result = get_project_dir() + assert result == Path.cwd().resolve() + + +def test_get_project_dir_ignores_nonexistent_env_path(tmp_path, monkeypatch): + monkeypatch.setenv("ROCKETSMITH_PROJECT_DIR", str(tmp_path / "does_not_exist")) + + import rocketsmith.mcp.utils as utils_mod + + rocketsmith_dir = tmp_path / ".rocketsmith" + monkeypatch.setattr(utils_mod, "_ROCKETSMITH_DIR", rocketsmith_dir) + + # Falls through to cwd since the env path doesn't exist + result = get_project_dir() + assert result == Path.cwd().resolve() + + +# ── tool_error / tool_success ───────────────────────────────────────────────── + + +def test_tool_error_returns_tool_error_model(): + result = tool_error("something went wrong", "ERR_001", detail="value") + assert isinstance(result, ToolError) + assert result.success is False + assert result.error == "something went wrong" + assert result.error_code == "ERR_001" + assert result.details["detail"] == "value" + + +def test_tool_error_no_details(): + result = tool_error("oops", "ERR_002") + assert result.details == {} + + +def test_tool_success_returns_tool_success_model(): + result = tool_success({"key": "value"}) + assert isinstance(result, ToolSuccess) + assert result.success is True + assert result.data == {"key": "value"} + + +def test_tool_success_with_primitive(): + result = tool_success(42) + assert result.data == 42 + + +# ── resolve_path ────────────────────────────────────────────────────────────── + + +def test_resolve_path_absolute_returned_unchanged(tmp_path): + p = tmp_path / "file.txt" + p.touch() + result = resolve_path(p) + assert result == p.resolve() + + +def test_resolve_path_relative_resolved_against_project_dir(tmp_path, monkeypatch): + monkeypatch.setenv("ROCKETSMITH_PROJECT_DIR", str(tmp_path)) + result = resolve_path("subdir/file.txt") + assert result == (tmp_path / "subdir" / "file.txt").resolve() + + +def test_resolve_path_expands_tilde(monkeypatch): + result = resolve_path("~/somefile.txt") + assert not str(result).startswith("~") + assert result.is_absolute() + + +def test_resolve_path_must_exist_raises_when_missing(tmp_path, monkeypatch): + monkeypatch.setenv("ROCKETSMITH_PROJECT_DIR", str(tmp_path)) + with pytest.raises(FileNotFoundError): + resolve_path("nonexistent.txt", must_exist=True) + + +def test_resolve_path_must_exist_passes_when_present(tmp_path, monkeypatch): + monkeypatch.setenv("ROCKETSMITH_PROJECT_DIR", str(tmp_path)) + f = tmp_path / "present.txt" + f.touch() + result = resolve_path("present.txt", must_exist=True) + assert result == f.resolve()