Skip to content
This repository was archived by the owner on Jan 23, 2026. It is now read-only.

Jumpstarter MCP server integration Phase 1#721

Closed
RakeshMK90 wants to merge 1 commit intojumpstarter-dev:mainfrom
RakeshMK90:rakesh/jmp-mcp-server
Closed

Jumpstarter MCP server integration Phase 1#721
RakeshMK90 wants to merge 1 commit intojumpstarter-dev:mainfrom
RakeshMK90:rakesh/jmp-mcp-server

Conversation

@RakeshMK90
Copy link
Copy Markdown

@RakeshMK90 RakeshMK90 commented Oct 24, 2025

Summary

Migrated the Jumpstarter MCP server from the standard MCP library to FastMCP framework, resolving validation errors and implementing working lease creation functionality.

Key Changes

  • FastMCP Migration: Replaced standard MCP with FastMCP for better reliability and simpler tool definitions
  • Fixed Validation Errors: Resolved 20+ CallToolResult validation errors when used with Claude/Cursor AI
  • Working Lease Creation: Implemented real create_lease() API integration with robust error handling
  • Container Updates: Updated Containerfile to use FastMCP entry point and fixed version detection
  • Enhanced Documentation: Comprehensive README highlighting FastMCP benefits and current capabilities

Working Tools (Production Ready)

✅ jumpstarter_get_config - Configuration information✅ jumpstarter_list_exporters - Hardware discovery✅ jumpstarter_list_leases - Lease monitoring✅ jumpstarter_create_lease - Real lease creation

Technical Improvements

  • Simplified Tool Definition: Clean @mcp.tool decorators instead of complex handler registration
  • Automatic Error Handling: FastMCP handles serialization and validation automatically
  • API Compatibility: Robust fallback logic for different Jumpstarter API versions
  • Container Deployment: Ready for production with Podman/Docker

Files Changed

  • jumpstarter_mcp_server/fastmcp_main.py - New FastMCP implementation
  • pyproject.toml - Updated dependencies and entry points
  • Containerfile - FastMCP entry point and version fixes
  • README.md - Comprehensive documentation update

Testing

  • ✅ Container builds successfully
  • ✅ All 4 core tools tested and working
  • ✅ Lease creation API integration verified
  • ✅ Error handling for parameter compatibility

Summary by CodeRabbit

Release Notes

  • New Features

    • Introduced Jumpstarter MCP Server for Claude Desktop integration, enabling hardware control and automation workflows directly from Claude.
    • Added container-based deployment support with pre-configured setup.
    • Integrated multiple MCP tools for device management, including configuration retrieval, exporter and lease management, power control, and storage operations.
  • Documentation

    • Added comprehensive README covering deployment, installation, usage, and troubleshooting.
  • Chores

    • Added configuration files and deployment scripts for local and containerized setups.

@netlify
Copy link
Copy Markdown

netlify Bot commented Oct 24, 2025

Deploy Preview for jumpstarter-docs ready!

Name Link
🔨 Latest commit a8a4db4
🔍 Latest deploy log https://app.netlify.com/projects/jumpstarter-docs/deploys/68faf46b231103000809bb8b
😎 Deploy Preview https://deploy-preview-721--jumpstarter-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Oct 24, 2025

Walkthrough

A new MCP server package is introduced for Jumpstarter, enabling Claude and other MCP clients to interact with hardware devices through tools for leasing, power control, serial console, storage flashing, and SSH forwarding. The package includes container deployment, configuration, and comprehensive tooling.

Changes

Cohort / File(s) Summary
Package Configuration
packages/jumpstarter-mcp-server/pyproject.toml, pyproject.toml
Adds new project metadata, dependencies (FastMCP, Pydantic, AnyIO), and console script entry points; registers package as workspace member
Containerization
packages/jumpstarter-mcp-server/Containerfile
Defines Docker/Podman container image with Python 3.11-slim base, system dependencies, non-root user, and FastMCP server entry point
Claude Integration
packages/jumpstarter-mcp-server/claude_desktop_config.json, packages/jumpstarter-mcp-server/claude_desktop_config_podman.json
Configuration templates for running MCP server locally via uv and via Podman, specifying endpoint credentials and mount points
Documentation & Init
packages/jumpstarter-mcp-server/README.md, packages/jumpstarter-mcp-server/jumpstarter_mcp_server/__init__.py
Comprehensive usage guide covering features, architecture, installation, tool descriptions, and configuration; package initializer with version metadata
MCP Server Implementation
packages/jumpstarter-mcp-server/jumpstarter_mcp_server/fastmcp_main.py
Core MCP server exposing ten tools: config retrieval, exporter/lease listing, lease creation, shell execution, power control, serial console, storage flashing, SSH forwarding, and generic j-command execution; includes config loading, error handling, and subprocess management
Deployment Helper
packages/jumpstarter-mcp-server/run-podman.sh
Shell script automating Podman container build and execution with config/share directory setup and environment variable injection

Sequence Diagram

sequenceDiagram
    participant Claude as Claude / MCP Client
    participant Server as MCP Server
    participant Config as Config Loader
    participant Client as Jumpstarter Client
    participant CLI as CLI / Subprocess

    Claude->>Server: Request tool (e.g., jumpstarter_list_leases)
    activate Server
    
    alt Config Not Cached
        Server->>Config: _load_client_config()
        activate Config
        Config->>Config: Try ClientConfigV1Alpha1
        alt Fails
            Config->>Config: Load UserConfigV1Alpha1
            Config->>Config: Extract current_client
        end
        Config-->>Server: config
        deactivate Config
    end
    
    Server->>Client: Call client API (e.g., list_leases)
    activate Client
    Client-->>Server: Response object / data structure
    deactivate Client
    
    Server->>Server: Normalize heterogeneous shape<br/>to uniform dict list
    Server->>Server: Format as JSON string
    Server-->>Claude: Tool result with JSON payload
    deactivate Server
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

The addition of a full MCP server with ten tools requires careful review of logic correctness, error handling robustness, API integration patterns, and subprocess safety. The main implementation file (fastmcp_main.py) exhibits moderate-to-high logic density with multiple fallback strategies, configuration attribute probing, and varied tool implementations. While some tools follow repetitive patterns, each requires individual reasoning for correctness and security. Container and deployment configurations introduce infrastructure concerns. The scope is focused within a new package but touches root configuration.

Possibly related PRs

Suggested reviewers

  • NickCao
  • kirkbrauer

Poem

🐰 A server hops to life with FastMCP dreams,
Tools for leases, power, and storage streams,
Claude whispers commands through the protocol wire,
Hardware automation climbing higher and higher!
From container to config, the journey's complete,
Device management made bittersweet. 🚀✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "Jumpstarter MCP server integration Phase 1" is related to the main work in this changeset, which involves migrating the Jumpstarter MCP server to the FastMCP framework. The title correctly identifies the component being modified (Jumpstarter MCP server) and frames it as a significant integration milestone (Phase 1). While the term "integration" is somewhat generic and doesn't explicitly mention the FastMCP migration, it is specific enough that a teammate would recognize this involves substantial MCP server work, and the "Phase 1" qualifier appropriately indicates this is part of a planned effort. The title avoids misleading claims and remains focused on the primary area of change.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@RakeshMK90 RakeshMK90 closed this Oct 24, 2025
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🧹 Nitpick comments (18)
packages/jumpstarter-mcp-server/pyproject.toml (1)

11-18: Consider adding upper bounds to critical dependencies.

The dependencies use lower bounds only, which could lead to unexpected breakage when newer versions introduce breaking changes. Consider pinning or adding upper bounds for production stability, especially for:

  • fastmcp>=2.0.0 (relatively new framework)
  • pydantic>=2.8.2 (known for breaking changes between minor versions)

Example:

-    "fastmcp>=2.0.0",
-    "pydantic>=2.8.2",
+    "fastmcp>=2.0.0,<3.0.0",
+    "pydantic>=2.8.2,<3.0.0",
packages/jumpstarter-mcp-server/Containerfile (3)

1-8: LGTM! Base image and dependencies configured correctly.

The use of python:3.11-slim with proper apt cache cleanup is appropriate. Git is necessary for VCS-based versioning.

If curl is not actively used in the build or runtime, consider removing it to reduce the attack surface:

 RUN apt-get update && apt-get install -y \
     git \
-    curl \
     && rm -rf /var/lib/apt/lists/*

19-28: Version handling looks reasonable with fallback strategy.

The approach correctly:

  • Provides fallback versions via environment variables
  • Preserves Git metadata for VCS-based versioning when available
  • Checks for .git directory presence

Consider documenting why all packages use version 0.1.0 as fallback. In production builds, these versions should ideally match the actual package versions to avoid confusion.


36-40: LGTM! Entry point correctly configured.

The dependency synchronization and entry point are properly configured to run the FastMCP server from the workspace.

The file ends without a trailing newline. While not critical for Dockerfiles, adding one improves POSIX compliance:

 ENTRYPOINT ["uv", "run", "--directory", "packages/jumpstarter-mcp-server", "jumpstarter-fastmcp"]
+
packages/jumpstarter-mcp-server/claude_desktop_config_podman.json (1)

16-19: Document that these are placeholder credentials.

The static analysis tool flagged the auth token as a potential secret. While this is clearly a placeholder (truncated JWT, example.com domain), it would be helpful to add a comment in the file or adjacent README to make it explicit that users must replace these with their actual credentials.

Consider adding a comment block before the env section:

       "jumpstarter-mcp-server"
     ],
+    "_comment": "Replace env values with your actual Jumpstarter credentials",
     "env": {
       "JUMPSTARTER_CLIENT_ENDPOINT": "grpc.jumpstarter.example.com:443",
       "JUMPSTARTER_AUTH_TOKEN": "eyJhbGciOiJSUzI1NiIsImtpZCI6IjEifQ..."
packages/jumpstarter-mcp-server/README.md (1)

203-213: Consider using proper headings instead of bold text.

The markdown linter suggests using heading syntax (###) instead of bold emphasis for these subsection titles.

Apply this formatting fix:

-**"No client configuration available"**
+### "No client configuration available"
+
 - Run `jmp login` to set up authentication
 - Or set `JUMPSTARTER_ENDPOINT` and `JUMPSTARTER_TOKEN` environment variables
 
-**"Failed to list exporters"**
+### "Failed to list exporters"
+
 - Check network connectivity to Jumpstarter endpoint
 - Verify authentication token is valid and not expired
 
-**Container build issues**
+### Container build issues
+
 - Ensure you're building from the root jumpstarter directory
 - Check that all git metadata is available for version detection
packages/jumpstarter-mcp-server/claude_desktop_config.json (1)

11-14: Document placeholder credentials.

Similar to the Podman config, these are placeholder credentials that users must replace. Consider adding a comment to make this explicit.

       "jumpstarter-mcp"
     ],
+    "_comment": "Replace these with your actual Jumpstarter credentials",
     "env": {
       "JUMPSTARTER_CLIENT_ENDPOINT": "grpc.jumpstarter.example.com:443",
packages/jumpstarter-mcp-server/run-podman.sh (1)

26-37: LGTM! Container execution is properly configured.

The script correctly:

  • Creates necessary directories
  • Uses appropriate container flags (-it, --rm)
  • Mounts volumes as read-only for security
  • Provides environment variable defaults
  • Uses a descriptive container name

Consider adding a trailing newline for POSIX compliance:

     jumpstarter-mcp-server
+
packages/jumpstarter-mcp-server/jumpstarter_mcp_server/fastmcp_main.py (10)

109-130: Downgrade noisy introspection logs to debug.

Info-level dir()/type() logs will spam MCP clients. Use debug.

-        logger.info(f"Exporters type: {type(exporters)}")
-        logger.info(f"Exporters dir: {dir(exporters)}")
+        logger.debug(f"Exporters type: {type(exporters)}")
+        logger.debug(f"Exporters dir: {dir(exporters)}")
@@
-        logger.info(f"Found {len(exporter_list)} exporters")
+        logger.debug(f"Found {len(exporter_list)} exporters")
         if exporter_list:
-            logger.info(f"First exporter type: {type(exporter_list[0])}")
-            logger.info(f"First exporter dir: {dir(exporter_list[0])}")
+            logger.debug(f"First exporter type: {type(exporter_list[0])}")
+            logger.debug(f"First exporter dir: {dir(exporter_list[0])}")

196-216: Same logging nit: prefer debug for lease introspection.

-        logger.info(f"Leases type: {type(leases)}")
-        logger.info(f"Leases dir: {dir(leases)}")
+        logger.debug(f"Leases type: {type(leases)}")
+        logger.debug(f"Leases dir: {dir(leases)}")
@@
-        logger.info(f"Found {len(lease_list)} leases")
+        logger.debug(f"Found {len(lease_list)} leases")
         if lease_list:
-            logger.info(f"First lease type: {type(lease_list[0])}")
-            logger.info(f"First lease dir: {dir(lease_list[0])}")
+            logger.debug(f"First lease type: {type(lease_list[0])}")
+            logger.debug(f"First lease dir: {dir(lease_list[0])}")

363-390: Add timeout to subprocess to avoid hanging MCP requests.

Power actions can stall; enforce a timeout and kill on expiry.

-async def jumpstarter_power_control(action: str, lease_id: Optional[str] = None) -> str:
+async def jumpstarter_power_control(
+    action: str, lease_id: Optional[str] = None, timeout_seconds: int = 60
+) -> str:
@@
-        stdout, stderr = await process.communicate()
+        try:
+            stdout, stderr = await asyncio.wait_for(process.communicate(), timeout=timeout_seconds)
+        except asyncio.TimeoutError:
+            process.kill()
+            await process.wait()
+            return f"Power {action} command timed out after {timeout_seconds}s"

470-507: Validate SSH local_port and clarify outputs.

Reject invalid ports early; keeps errors crisp.

 async def jumpstarter_ssh_forward(
     local_port: int = 2222,
     action: str = "start",
     lease_id: Optional[str] = None
 ) -> str:
@@
     if action not in ["start", "stop", "status"]:
         raise ValueError("Action must be one of: start, stop, status")
 
     try:
+        if not (1 <= local_port <= 65535):
+            raise ValueError("local_port must be between 1 and 65535")
         env_vars = {"JMP_LEASE": lease_id} if lease_id else {}

512-540: Add timeout to generic j command execution.

Prevent indefinite hangs; return clear timeout message.

-async def jumpstarter_run_j_command(command: List[str], lease_id: Optional[str] = None) -> str:
+async def jumpstarter_run_j_command(
+    command: List[str], lease_id: Optional[str] = None, timeout_seconds: int = 300
+) -> str:
@@
-        stdout, stderr = await process.communicate()
+        try:
+            stdout, stderr = await asyncio.wait_for(process.communicate(), timeout=timeout_seconds)
+        except asyncio.TimeoutError:
+            process.kill()
+            await process.wait()
+            return f"J command timed out after {timeout_seconds}s: {' '.join(['j'] + command)}"

41-83: Reduce complexity in jumpstarter_get_config (ruff C901) by extracting helpers.

Endpoint/driver extraction can be small helpers; keeps tool handler lean and testable.

Example outline (minimal, not exhaustive):

+mdef _endpoint_from_client(cfg: ClientConfigV1Alpha1) -> str:
+    return getattr(cfg, "endpoint", None) or "unknown"
+
+mdef _driver_flags(cfg: ClientConfigV1Alpha1) -> tuple[list, bool]:
+    d = getattr(cfg, "drivers", None)
+    return (getattr(d, "allow", []) if d else [], getattr(d, "unsafe", False) if d else False)
@@
-    try:
-        config = _load_client_config()
-        config_type = type(config).__name__
-        ...
-        if isinstance(config, ClientConfigV1Alpha1):
-            ...
-            config_info.update({...})
-        return f"Jumpstarter Configuration:\n{json.dumps(config_info, indent=2)}"
+    try:
+        cfg = _load_client_config()
+        info = {"type": type(cfg).__name__, "is_client_config": isinstance(cfg, ClientConfigV1Alpha1),
+                "is_exporter_config": isinstance(cfg, ExporterConfigV1Alpha1)}
+        if isinstance(cfg, ClientConfigV1Alpha1):
+            allow, unsafe = _driver_flags(cfg)
+            info.update({"endpoint": _endpoint_from_client(cfg),
+                         "driver_allow_list": allow, "unsafe_drivers": unsafe,
+                         "config_attributes": [a for a in dir(cfg) if not a.startswith('_')]})
+        return f"Jumpstarter Configuration:\n{json.dumps(info, indent=2)}"

270-276: Clarify lease_name parameter behavior.

The tool accepts lease_name but does not pass it to the client API (which currently takes selector/duration). Consider documenting that lease_name is informational for now or remove it to avoid confusion.


90-178: Complexity in jumpstarter_list_exporters (ruff C901).

Consider extracting two tiny helpers: safe_to_list(collection) and normalize_exporter(exp) to shrink branches and improve readability. Similar for leases with normalize_lease.


371-379: Minor: surface stderr on success as debug, not in user-visible string.

Today success may append stdout but ignore stderr unless failure. Keep user result clean; log stderr at debug.


293-314: Use asyncio.to_thread for create_lease; consistency with list_exporters and list_leases.

create_lease is decorated with @blocking_compat identically to list_exporters and list_leases. Both list* methods are already called via asyncio.to_thread; apply the same pattern to create_lease for consistency.

-        try:
-            # Try with selector and duration first
-            lease_request = await config.create_lease(
-                selector=selector,
-                duration=duration
-            )
+        try:
+            # Try with selector and duration first
+            lease_request = await asyncio.to_thread(
+                config.create_lease,
+                selector=selector,
+                duration=duration,
+            )
         except TypeError as e:
             if "unexpected keyword argument" in str(e):
                 # If duration or other parameters are not supported, try with just selector
                 logger.warning(f"Parameter not supported, trying with selector only: {e}")
                 try:
-                    lease_request = await config.create_lease(selector=selector)
+                    lease_request = await asyncio.to_thread(config.create_lease, selector=selector)
                 except TypeError as e2:
                     if "unexpected keyword argument" in str(e2):
                         # If even selector is not supported as keyword, try positional
                         logger.warning(f"Selector as keyword not supported, trying positional: {e2}")
-                        lease_request = await config.create_lease(selector)
+                        lease_request = await asyncio.to_thread(config.create_lease, selector)
                     else:
                         raise
             else:
                 raise
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 31d0d8f and a8a4db4.

📒 Files selected for processing (9)
  • packages/jumpstarter-mcp-server/Containerfile (1 hunks)
  • packages/jumpstarter-mcp-server/README.md (1 hunks)
  • packages/jumpstarter-mcp-server/claude_desktop_config.json (1 hunks)
  • packages/jumpstarter-mcp-server/claude_desktop_config_podman.json (1 hunks)
  • packages/jumpstarter-mcp-server/jumpstarter_mcp_server/__init__.py (1 hunks)
  • packages/jumpstarter-mcp-server/jumpstarter_mcp_server/fastmcp_main.py (1 hunks)
  • packages/jumpstarter-mcp-server/pyproject.toml (1 hunks)
  • packages/jumpstarter-mcp-server/run-podman.sh (1 hunks)
  • pyproject.toml (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/jumpstarter-mcp-server/jumpstarter_mcp_server/fastmcp_main.py (3)
packages/jumpstarter/jumpstarter/config/client.py (2)
  • ClientConfigV1Alpha1 (89-370)
  • lease (123-131)
packages/jumpstarter/jumpstarter/config/exporter.py (1)
  • ExporterConfigV1Alpha1 (73-197)
packages/jumpstarter/jumpstarter/config/user.py (2)
  • UserConfigV1Alpha1 (48-102)
  • load_or_create (74-82)
🪛 GitHub Actions: Lint
packages/jumpstarter-mcp-server/jumpstarter_mcp_server/__init__.py

[warning] 7-7: W292 No newline at end of file.

🪛 GitHub Check: ruff
packages/jumpstarter-mcp-server/jumpstarter_mcp_server/__init__.py

[failure] 7-7: Ruff (W292)
packages/jumpstarter-mcp-server/jumpstarter_mcp_server/init.py:7:22: W292 No newline at end of file

packages/jumpstarter-mcp-server/jumpstarter_mcp_server/fastmcp_main.py

[failure] 181-181: Ruff (B904)
packages/jumpstarter-mcp-server/jumpstarter_mcp_server/fastmcp_main.py:181:9: B904 Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling


[failure] 90-90: Ruff (C901)
packages/jumpstarter-mcp-server/jumpstarter_mcp_server/fastmcp_main.py:90:11: C901 jumpstarter_list_exporters is too complex (19 > 10)


[failure] 86-86: Ruff (B904)
packages/jumpstarter-mcp-server/jumpstarter_mcp_server/fastmcp_main.py:86:9: B904 Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling


[failure] 41-41: Ruff (C901)
packages/jumpstarter-mcp-server/jumpstarter_mcp_server/fastmcp_main.py:41:11: C901 jumpstarter_get_config is too complex (11 > 10)


[failure] 32-32: Ruff (E501)
packages/jumpstarter-mcp-server/jumpstarter_mcp_server/fastmcp_main.py:32:121: E501 Line too long (121 > 120)


[failure] 32-32: Ruff (B904)
packages/jumpstarter-mcp-server/jumpstarter_mcp_server/fastmcp_main.py:32:13: B904 Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling


[failure] 11-11: Ruff (F401)
packages/jumpstarter-mcp-server/jumpstarter_mcp_server/fastmcp_main.py:11:25: F401 typing.Dict imported but unused


[failure] 11-11: Ruff (F401)
packages/jumpstarter-mcp-server/jumpstarter_mcp_server/fastmcp_main.py:11:20: F401 typing.Any imported but unused


[failure] 6-20: Ruff (I001)
packages/jumpstarter-mcp-server/jumpstarter_mcp_server/fastmcp_main.py:6:1: I001 Import block is un-sorted or un-formatted

🪛 Gitleaks (8.28.0)
packages/jumpstarter-mcp-server/claude_desktop_config_podman.json

[high] 18-18: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

packages/jumpstarter-mcp-server/claude_desktop_config.json

[high] 13-13: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

🪛 markdownlint-cli2 (0.18.1)
packages/jumpstarter-mcp-server/README.md

203-203: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


207-207: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


211-211: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
  • GitHub Check: Redirect rules - jumpstarter-docs
  • GitHub Check: pytest-matrix (macos-15, 3.12)
  • GitHub Check: pytest-matrix (macos-15, 3.13)
  • GitHub Check: pytest-matrix (ubuntu-24.04, 3.13)
  • GitHub Check: pytest-matrix (ubuntu-24.04, 3.11)
  • GitHub Check: pytest-matrix (ubuntu-24.04, 3.12)
  • GitHub Check: pytest-matrix (macos-15, 3.11)
  • GitHub Check: build
  • GitHub Check: e2e
  • GitHub Check: Header rules - jumpstarter-docs
  • GitHub Check: Pages changed - jumpstarter-docs
🔇 Additional comments (10)
pyproject.toml (1)

39-39: LGTM! Workspace member correctly added.

The new jumpstarter-mcp-server package is properly registered as a workspace member, following the existing convention.

packages/jumpstarter-mcp-server/pyproject.toml (1)

39-48: LGTM! Build configuration is appropriate for monorepo.

The VCS-based versioning with custom root path and the pin_jumpstarter hook are correctly configured for the workspace structure.

packages/jumpstarter-mcp-server/Containerfile (1)

13-34: LGTM! Excellent security practices.

The container correctly:

  • Creates a non-root user (UID 1000)
  • Sets environment variables before copying files
  • Changes ownership appropriately
  • Switches to non-root user before dependency installation
packages/jumpstarter-mcp-server/claude_desktop_config_podman.json (1)

10-11: LGTM! Read-only volume mounts enhance security.

The configuration correctly mounts Jumpstarter config directories as read-only (:ro), preventing the container from modifying local configuration.

packages/jumpstarter-mcp-server/jumpstarter_mcp_server/__init__.py (1)

1-5: LGTM! Clear module documentation.

The docstring appropriately describes the MCP server's purpose and capabilities.

packages/jumpstarter-mcp-server/README.md (3)

1-58: LGTM! Clear and comprehensive overview.

The documentation effectively communicates:

  • FastMCP framework benefits
  • Current implementation status
  • Architecture decisions
  • Container deployment approach

104-180: LGTM! Tool documentation is thorough and clear.

Each tool is well-documented with:

  • Purpose description
  • Parameter specifications
  • Example responses
  • Feature highlights

262-275: LGTM! Clear roadmap for future development.

The roadmap effectively communicates planned features and future enhancements, helping users understand the project's direction.

packages/jumpstarter-mcp-server/run-podman.sh (2)

1-9: LGTM! Script initialization and build are correct.

The script properly:

  • Uses set -e for error handling
  • Navigates to repository root
  • Builds the container from the correct location

11-24: LGTM! Excellent configuration validation.

The script provides helpful diagnostics:

  • Checks for existing configuration
  • Warns about missing user-specific config
  • Lists available clients
  • Creates directories as needed

Comment on lines +8 to +9
"/path/to/jumpstarter/packages/jumpstarter-mcp-server",
"jumpstarter-mcp"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Document path customization requirement and verify entry point.

Two concerns with this configuration:

  1. The hard-coded path /path/to/jumpstarter/packages/jumpstarter-mcp-server must be customized by users. Consider adding a comment or using a more obvious placeholder like <PATH_TO_JUMPSTARTER>.

  2. This uses the jumpstarter-mcp entry point, but earlier review found that jumpstarter_mcp_server.main:main_sync may not exist. The Containerfile and README use jumpstarter-fastmcp instead. Consider aligning this configuration to use jumpstarter-fastmcp for consistency.

Verify which entry point should be used:


🏁 Script executed:

#!/bin/bash
# Check if main.py exists and which entry point is intended for desktop use
fd -t f "main.py" packages/jumpstarter-mcp-server/
echo "---"
echo "Containerfile uses:"
grep "jumpstarter-fastmcp" packages/jumpstarter-mcp-server/Containerfile
echo "---"
echo "Podman config uses:"
grep "jumpstarter-mcp-server" packages/jumpstarter-mcp-server/claude_desktop_config_podman.json

Length of output: 293


I need to check the entry points defined in the project configuration to verify which one is correct for the desktop configuration.

is not needed


🏁 Script executed:

#!/bin/bash
# Find and inspect pyproject.toml for entry points
fd -t f "pyproject.toml" | head -5
echo "---"
echo "Checking jumpstarter-mcp-server pyproject.toml for entry points:"
grep -A 15 "\[project.scripts\]" packages/jumpstarter-mcp-server/pyproject.toml 2>/dev/null || echo "Not found with grep"
echo "---"
echo "Alternative check with full file context:"
cat packages/jumpstarter-mcp-server/pyproject.toml | grep -A 15 "scripts"

Length of output: 426


🏁 Script executed:

#!/bin/bash
# Search for entry point definitions in root and packages level
echo "Root pyproject.toml entry points:"
grep -A 20 "\[project.scripts\]" pyproject.toml 2>/dev/null | head -30
echo "---"
echo "Packages pyproject.toml entry points:"
grep -A 20 "\[project.scripts\]" packages/pyproject.toml 2>/dev/null | head -30
echo "---"
echo "Search for jumpstarter-mcp and jumpstarter-fastmcp definitions:"
rg -n "jumpstarter-mcp|jumpstarter-fastmcp" --type toml -C 2 | head -50

Length of output: 1225


🏁 Script executed:

#!/bin/bash
# Verify if main.py and main_sync exist
echo "Looking for main.py in jumpstarter_mcp_server:"
find packages/jumpstarter-mcp-server/jumpstarter_mcp_server -name "main.py" -o -name "*.py" | sort
echo "---"
echo "Searching for main_sync function:"
rg -n "def main_sync" packages/jumpstarter-mcp-server/
echo "---"
echo "All Python files in jumpstarter_mcp_server:"
fd -t f "\.py$" packages/jumpstarter-mcp-server/jumpstarter_mcp_server/

Length of output: 408


Change entry point from jumpstarter-mcp to jumpstarter-fastmcp and document path customization.

Two issues with this configuration:

  1. The hard-coded path /path/to/jumpstarter/packages/jumpstarter-mcp-server must be customized by users. Add a comment documenting this or use a more obvious placeholder.

  2. Line 9 uses jumpstarter-mcp entry point, but jumpstarter_mcp_server.main:main_sync does not exist—only jumpstarter_mcp_server.fastmcp_main:main is implemented. Change this to jumpstarter-fastmcp to match the Containerfile and ensure the configuration works.

🤖 Prompt for AI Agents
In packages/jumpstarter-mcp-server/claude_desktop_config.json around lines 8 to
9, the config uses a hard-coded path and the wrong entrypoint; replace the
literal "/path/to/jumpstarter/packages/jumpstarter-mcp-server" with a clear
customizable placeholder (e.g.
"<LOCAL_JUMPSTARTER_PATH>/packages/jumpstarter-mcp-server") and add a comment or
inline note indicating users must update it to their local repo path, and change
the entrypoint token from "jumpstarter-mcp" to "jumpstarter-fastmcp" so it
matches the implemented fastmcp_main entry (and the Containerfile).

Provides tools for hardware testing, device management, and automation workflows.
"""

__version__ = "0.1.0" No newline at end of file
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Fix missing newline to resolve CI failure.

The file is missing a newline at the end, causing the Ruff linter to fail (W292). This is blocking the pipeline.

Apply this fix:

 __version__ = "0.1.0"
+
📝 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.

Suggested change
__version__ = "0.1.0"
__version__ = "0.1.0"
🧰 Tools
🪛 GitHub Actions: Lint

[warning] 7-7: W292 No newline at end of file.

🪛 GitHub Check: ruff

[failure] 7-7: Ruff (W292)
packages/jumpstarter-mcp-server/jumpstarter_mcp_server/init.py:7:22: W292 No newline at end of file

🤖 Prompt for AI Agents
In packages/jumpstarter-mcp-server/jumpstarter_mcp_server/__init__.py around
line 7, the file ends without a trailing newline causing Ruff W292; fix it by
adding a single newline character at EOF (ensure the file ends with a blank line
after the existing __version__ = "0.1.0" line) and save the file so the linter
no longer reports the missing newline.

Comment on lines +6 to +18
import asyncio
import json
import logging
import os
import sys
from typing import Any, Dict, List, Optional

from fastmcp import FastMCP

from jumpstarter.config.client import ClientConfigV1Alpha1
from jumpstarter.config.exporter import ExporterConfigV1Alpha1
from jumpstarter.config.user import UserConfigV1Alpha1

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Fix import order and remove unused typing imports (ruff I001, F401).

Sort imports and drop unused Any/Dict to satisfy Ruff and keep the block clean.

Apply this diff:

-import asyncio
-import json
-import logging
-import os
-import sys
-from typing import Any, Dict, List, Optional
-
-from fastmcp import FastMCP
-
-from jumpstarter.config.client import ClientConfigV1Alpha1
-from jumpstarter.config.exporter import ExporterConfigV1Alpha1
-from jumpstarter.config.user import UserConfigV1Alpha1
+import asyncio
+import json
+import logging
+import os
+import sys
+from typing import List, Optional
+
+from fastmcp import FastMCP
+
+from jumpstarter.config.client import ClientConfigV1Alpha1
+from jumpstarter.config.exporter import ExporterConfigV1Alpha1
+from jumpstarter.config.user import UserConfigV1Alpha1
📝 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.

Suggested change
import asyncio
import json
import logging
import os
import sys
from typing import Any, Dict, List, Optional
from fastmcp import FastMCP
from jumpstarter.config.client import ClientConfigV1Alpha1
from jumpstarter.config.exporter import ExporterConfigV1Alpha1
from jumpstarter.config.user import UserConfigV1Alpha1
import asyncio
import json
import logging
import os
import sys
from typing import List, Optional
from fastmcp import FastMCP
from jumpstarter.config.client import ClientConfigV1Alpha1
from jumpstarter.config.exporter import ExporterConfigV1Alpha1
from jumpstarter.config.user import UserConfigV1Alpha1
🧰 Tools
🪛 GitHub Check: ruff

[failure] 11-11: Ruff (F401)
packages/jumpstarter-mcp-server/jumpstarter_mcp_server/fastmcp_main.py:11:25: F401 typing.Dict imported but unused


[failure] 11-11: Ruff (F401)
packages/jumpstarter-mcp-server/jumpstarter_mcp_server/fastmcp_main.py:11:20: F401 typing.Any imported but unused


[failure] 6-20: Ruff (I001)
packages/jumpstarter-mcp-server/jumpstarter_mcp_server/fastmcp_main.py:6:1: I001 Import block is un-sorted or un-formatted

Comment on lines +23 to +34
def _load_client_config() -> ClientConfigV1Alpha1:
"""Load client configuration following the same logic as CLI tools"""
try:
# Try to create a config directly (will succeed if env vars are set)
return ClientConfigV1Alpha1()
except Exception:
# Fall back to user config
user_config = UserConfigV1Alpha1.load_or_create()
if user_config.config.current_client is None:
raise RuntimeError("No client configuration available. Please run 'jmp login' or set environment variables.")
return user_config.config.current_client

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Use exception chaining and wrap long line (ruff B904, E501) in _load_client_config().

Chain the cause and break the long error message.

 def _load_client_config() -> ClientConfigV1Alpha1:
     """Load client configuration following the same logic as CLI tools"""
     try:
         # Try to create a config directly (will succeed if env vars are set)
         return ClientConfigV1Alpha1()
-    except Exception:
+    except Exception as e:
         # Fall back to user config
         user_config = UserConfigV1Alpha1.load_or_create()
         if user_config.config.current_client is None:
-            raise RuntimeError("No client configuration available. Please run 'jmp login' or set environment variables.")
+            raise RuntimeError(
+                "No client configuration available. Please run 'jmp login' "
+                "or set environment variables."
+            ) from e
         return user_config.config.current_client
🧰 Tools
🪛 GitHub Check: ruff

[failure] 32-32: Ruff (E501)
packages/jumpstarter-mcp-server/jumpstarter_mcp_server/fastmcp_main.py:32:121: E501 Line too long (121 > 120)


[failure] 32-32: Ruff (B904)
packages/jumpstarter-mcp-server/jumpstarter_mcp_server/fastmcp_main.py:32:13: B904 Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling

🤖 Prompt for AI Agents
In packages/jumpstarter-mcp-server/jumpstarter_mcp_server/fastmcp_main.py around
lines 23 to 34, the exception handler should use exception chaining and avoid a
too-long single-line error string; change the except block to catch the
exception as e (except Exception as e), then when current_client is None raise a
RuntimeError with a shorter message or a parenthesized multi-line string for
wrapping and include the original exception via "raise RuntimeError(... ) from
e" so the cause is preserved.

Comment on lines +85 to +86
except Exception as e:
raise RuntimeError(f"Failed to load configuration: {str(e)}")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Adopt raise ... from e in all except blocks (ruff B904).

This preserves root causes and satisfies lint.

-    except Exception as e:
-        raise RuntimeError(f"Failed to load configuration: {str(e)}")
+    except Exception as e:
+        raise RuntimeError(f"Failed to load configuration: {e}") from e
@@
-    except Exception as e:
-        logger.exception("Error listing exporters")
-        raise RuntimeError(f"Failed to list exporters: {str(e)}")
+    except Exception as e:
+        logger.exception("Error listing exporters")
+        raise RuntimeError(f"Failed to list exporters: {e}") from e
@@
-    except Exception as e:
-        logger.exception("Error listing leases")
-        raise RuntimeError(f"Failed to list leases: {str(e)}")
+    except Exception as e:
+        logger.exception("Error listing leases")
+        raise RuntimeError(f"Failed to list leases: {e}") from e
@@
-    except Exception as e:
-        logger.exception("Error creating lease")
-        raise RuntimeError(f"Failed to create lease: {str(e)}")
+    except Exception as e:
+        logger.exception("Error creating lease")
+        raise RuntimeError(f"Failed to create lease: {e}") from e
@@
-    except Exception as e:
-        raise RuntimeError(f"Failed to execute shell command: {str(e)}")
+    except Exception as e:
+        raise RuntimeError(f"Failed to execute shell command: {e}") from e
@@
-    except Exception as e:
-        raise RuntimeError(f"Failed to execute power {action}: {str(e)}")
+    except Exception as e:
+        raise RuntimeError(f"Failed to execute power {action}: {e}") from e
@@
-    except Exception as e:
-        raise RuntimeError(f"Failed to execute serial console action: {str(e)}")
+    except Exception as e:
+        raise RuntimeError(f"Failed to execute serial console action: {e}") from e
@@
-    except Exception as e:
-        raise RuntimeError(f"Failed to execute storage flash: {str(e)}")
+    except Exception as e:
+        raise RuntimeError(f"Failed to execute storage flash: {e}") from e
@@
-    except Exception as e:
-        raise RuntimeError(f"Failed to execute SSH forwarding: {str(e)}")
+    except Exception as e:
+        raise RuntimeError(f"Failed to execute SSH forwarding: {e}") from e
@@
-    except Exception as e:
-        raise RuntimeError(f"Failed to execute j command: {str(e)}")
+    except Exception as e:
+        raise RuntimeError(f"Failed to execute j command: {e}") from e

Also applies to: 179-181, 265-267, 335-337, 358-359, 391-393, 435-436, 466-467, 508-509, 541-542

🧰 Tools
🪛 GitHub Check: ruff

[failure] 86-86: Ruff (B904)
packages/jumpstarter-mcp-server/jumpstarter_mcp_server/fastmcp_main.py:86:9: B904 Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling

🤖 Prompt for AI Agents
In packages/jumpstarter-mcp-server/jumpstarter_mcp_server/fastmcp_main.py around
lines 85-86, 179-181, 265-267, 335-337, 358-359, 391-393, 435-436, 466-467,
508-509 and 541-542, the except blocks currently re-raise new RuntimeError (or
similar) without exception chaining; update each to re-raise using "raise
<RuntimeError(...)> from e" (keeping the same error message) so the original
exception is preserved and chained; ensure the except clause binds the exception
variable (e) and use it in the "from e" clause for every listed block.

Comment on lines +28 to +30
[project.scripts]
jumpstarter-mcp = "jumpstarter_mcp_server.main:main_sync"
jumpstarter-fastmcp = "jumpstarter_mcp_server.fastmcp_main:main"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify the jumpstarter-mcp entry point.

The jumpstarter-mcp entry point references jumpstarter_mcp_server.main:main_sync, but this module (main.py) is not included in the PR. Either this is a legacy entry point that should be removed, or the module exists outside this PR scope.

Run this script to check if the main.py module exists:

If the module doesn't exist, consider removing this entry point or documenting its future purpose.


🏁 Script executed:

#!/bin/bash
# Check for main.py in the jumpstarter-mcp-server package
fd -t f "main.py" packages/jumpstarter-mcp-server/

Length of output: 70


The jumpstarter-mcp entry point references a non-existent module.

The entry point jumpstarter-mcp = "jumpstarter_mcp_server.main:main_sync" points to jumpstarter_mcp_server.main:main_sync, but the main.py file does not exist in the package. This will cause a runtime error if the command is invoked.

Remove this entry point or create the missing jumpstarter_mcp_server/main.py module with a main_sync function.

🤖 Prompt for AI Agents
In packages/jumpstarter-mcp-server/pyproject.toml around lines 28 to 30, the
entry point jumpstarter-mcp = "jumpstarter_mcp_server.main:main_sync" references
a non-existent module/function; either remove this entry from project.scripts or
add a jumpstarter_mcp_server/main.py that defines a main_sync() function
(exported at module level) and ensure package __init__ and packaging include the
file so the entry point resolves at runtime.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant