mcp-hmr#39
Conversation
PR Change SummaryAdded documentation for the mcp-hmr package.
Added Files
How can I customize these reviews?Check out the Hyperlint AI Reviewer docs for more information on how to customize the review. If you just want to ignore it on this PR, you can add the Note specifically for link checks, we only check the first 30 links in a file and we cache the results for several hours (for instance, if you just added a page, you might experience this). Our recommendation is to add |
|
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. 📝 WalkthroughWalkthroughAdds a new mcp-hmr package that provides hot-module-reload for MCP/FastMCP servers: new HMR runtime module with runner and CLI, package metadata and CLI entry, README, workspace registration, and CI publish matrix inclusion. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (2)
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 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- Verify that the hmr~=0.7.0 dependency actually provides the reactivity and reactivity.hmr modules you import (or add the correct package name).
- Rather than mutating private
_mounted_serverslists in mount(), see if there’s a public unmount API to reduce fragility if internals change. - Make sure your pyproject.toml explicitly includes the mcp_hmr module (via
packagesormodules) so it isn’t left out of the published distribution.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Verify that the hmr~=0.7.0 dependency actually provides the reactivity and reactivity.hmr modules you import (or add the correct package name).
- Rather than mutating private `_mounted_servers` lists in mount(), see if there’s a public unmount API to reduce fragility if internals change.
- Make sure your pyproject.toml explicitly includes the mcp_hmr module (via `packages` or `modules`) so it isn’t left out of the published distribution.
## Individual Comments
### Comment 1
<location> `packages/mcp-hmr/mcp_hmr.py:22-27` </location>
<code_context>
+
+ base_app = FastMCP(include_fastmcp_meta=False)
+
+ @contextmanager
+ def mount(app: FastMCP | mcp.server.FastMCP):
+ base_app.mount(proxy := FastMCP.as_proxy(ProxyClient(app)), as_proxy=False)
+ try:
+ yield
+ finally: # unmount
+ for mounted_server in base_app._mounted_servers: # noqa: SLF001
+ if mounted_server.server is proxy:
</code_context>
<issue_to_address>
**issue:** Direct manipulation of private attributes may be fragile.
Accessing private attributes like _mounted_servers and _tool_manager._mounted_servers may lead to breakage if FastMCP's internals change. Please check for a public API for unmounting, or document this risk if direct access is necessary.
</issue_to_address>
### Comment 2
<location> `packages/mcp-hmr/mcp_hmr.py:48` </location>
<code_context>
+ return getattr(import_module(module), attr)
+
+ stop_event: Event | None = None
+ finish_event: Event = ... # type: ignore
+
+ @async_effect(context=HMR_CONTEXT, call_immediately=False)
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Use of '...' as a placeholder for Event may be confusing.
Using '...' with 'type: ignore' for finish_event may cause runtime errors if accessed before assignment. Initialize finish_event to None and update type hints, or guarantee it is set before use.
Suggested implementation:
```python
stop_event: Event | None = None
finish_event: Event | None = None
```
```python
await stop_event.wait()
if finish_event is not None:
finish_event.set()
```
```python
if finish_event is not None:
await finish_event.wait()
```
```python
stop_event = Event()
finish_event = Event()
create_task(using(app, stop_event, finish_event)) # noqa: RUF006
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
packages/mcp-hmr/mcp_hmr.py (2)
43-46: Harden get_app() error reporting.Emit a clear message when module/attr is invalid.
- def get_app(): - return getattr(import_module(module), attr) + def get_app(): + try: + mod = import_module(module) + except Exception as e: # narrow if desired + raise ImportError(f"Failed to import module '{module}': {e}") from e + try: + return getattr(mod, attr) + except AttributeError as e: + raise AttributeError(f"Module '{module}' has no attribute '{attr}'") from e
63-66: Consider passing an explicit watch root to AsyncReloader.
""relies on internal defaults; using CWD is clearer and may avoid edge cases.- def __init__(self): - super().__init__("") + def __init__(self): + from pathlib import Path + super().__init__(str(Path.cwd()))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/mcp-hmr/mcp_hmr.py(1 hunks)packages/mcp-hmr/pyproject.toml(1 hunks)
⏰ 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). (2)
- GitHub Check: Link Check
- GitHub Check: Sourcery review
🔇 Additional comments (3)
packages/mcp-hmr/pyproject.toml (3)
15-15: fastmcp version range looks good.Allowing v2 minors while capping <3 aligns with current v2 line.
Based on learnings
4-4: No action required — README.md exists.The verification confirms that
packages/mcp-hmr/README.mdis present, so the build configuration is correct.
20-23: PDM version extraction verified—__version__present.The configuration is correct.
__version__ = "0.0.1"exists at line 3 ofmcp_hmr.py, and pdm-backend will successfully extract it.
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new package mcp-hmr that enables hot module reloading for MCP (Model Context Protocol) servers, allowing automatic server restarts when code changes are detected during development.
Key Changes:
- Added hot-module-reload functionality for MCP servers with automatic restart on file changes
- Implemented CLI interface for running MCP servers with HMR enabled
- Created package configuration with dependencies on fastmcp and hmr libraries
Reviewed Changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/mcp-hmr/pyproject.toml | Package configuration defining project metadata, dependencies (fastmcp, hmr), and CLI entry point |
| packages/mcp-hmr/mcp_hmr.py | Core implementation of HMR functionality including server mounting/unmounting, reloader class, and CLI interface |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
CNSeniorious000
temporarily deployed
to
release
GitHub Actions
Inactive
CNSeniorious000
temporarily deployed
to
release
GitHub Actions
Inactive
CNSeniorious000
temporarily deployed
to
release
GitHub Actions
Inactive
CNSeniorious000
temporarily deployed
to
release
GitHub Actions
Inactive
CNSeniorious000
temporarily deployed
to
release
GitHub Actions
Inactive
CNSeniorious000
temporarily deployed
to
release
GitHub Actions
Inactive
CNSeniorious000
temporarily deployed
to
release
GitHub Actions
Inactive
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/mcp-hmr/mcp_hmr.py (1)
44-46: Consider explicit error handling for import failures.
import_moduleandgetattrcan raiseModuleNotFoundErrororAttributeErrorif the target doesn't exist. While the reloader will catch these, explicit validation with clearer error messages would improve developer experience.Example:
@derived(context=HMR_CONTEXT) def get_app(): try: mod = import_module(module) except ModuleNotFoundError as e: raise RuntimeError(f"Cannot import module '{module}': {e}") from e try: return getattr(mod, attr) except AttributeError as e: raise RuntimeError(f"Module '{module}' has no attribute '{attr}'") from e
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.github/workflows/ci.yml(1 hunks)packages/mcp-hmr/mcp_hmr.py(1 hunks)
⏰ 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). (2)
- GitHub Check: Link Check
- GitHub Check: Sourcery review
🔇 Additional comments (3)
.github/workflows/ci.yml (1)
46-46: LGTM! Publish matrix entry added correctly.The addition of
mcp-hmrto the publish matrix is consistent with the existing package structure and enables automated publishing for the new package.packages/mcp-hmr/mcp_hmr.py (2)
63-79: Well-structured reloader lifecycle management.The custom
Reloaderclass appropriately excludes the current file from watching, coordinates reload hooks, and manages the watching task lifecycle. The design fits the HMR use case well.
85-111: Clean CLI implementation with proper validation.The CLI correctly validates the
module:attrformat, ensures the current directory is importable, and gracefully handles keyboard interrupts. The argument parsing and error messages are clear.
Summary by CodeRabbit
New Features
Documentation
Chores