feat: figma mcp #38
feat: figma mcp #38
Conversation
- Introduced a new Figma MCP server configuration in `servers.yaml` with API token handling and timeout settings. - Updated `pyproject.toml` to include `aiohttp` as a dependency for Figma API integration. - Enhanced the configuration loader to inject environment variables for Figma API token. - Updated server orchestrator to recognize and manage the new Figma server type.
…time design manipulation - Developed a comprehensive Figma MCP server enabling real-time design manipulation through WebSocket communication with a Figma plugin. - Enhanced server capabilities with tools for document management, element creation, and modification. - Updated project configuration to include WebSocket dependencies and streamlined server setup. - Introduced a Figma plugin with a user interface for creating and managing design elements directly from Figma. - Improved documentation to reflect new features and usage instructions for the plugin and server integration.
…ents - Integrated WebSocket communication for real-time interaction between the Figma plugin and the server. - Updated the plugin UI to provide a more comprehensive user experience, including selection monitoring and improved error handling. - Modified server configuration to support WebSocket connections and adjusted timeout settings. - Enhanced the manifest to allow network access for local development, facilitating communication with the server. - Refactored code for better organization and maintainability, including utility functions for error handling and message processing.
- Changed WebSocket server URL from ws://localhost:9002 to ws://localhost:9003 in the plugin UI and configuration files. - Updated the Figma MCP server to use port 8003 for consistency with the new WebSocket settings. - Enhanced the manifest reasoning to clarify the purpose of the WebSocket communication.
- Added functionality to request and handle plugin status via WebSocket communication. - Updated the Figma MCP server to provide essential server status information for the plugin. - Enhanced the UI to log plugin status responses, improving user feedback and error handling. - Modified server configuration to include websocket port and plugin channel settings for better integration.
- Removed unnecessary dependencies from the server configuration. - Enhanced logging during server lifecycle events, including detailed error tracebacks for better debugging. - Implemented manual startup logic to ensure server initialization is robust across different transport types. - Added logging for environment configuration and server parameters to aid in troubleshooting.
- Reduced complexity in the Figma MCP plugin by removing advanced features and focusing on basic communication with the server. - Updated the plugin UI to streamline user interaction and enhance clarity. - Refactored server code to support a simplified architecture, removing unnecessary dependencies and tools. - Improved documentation to reflect the changes and provide clear instructions for the simplified plugin usage.
…t integration - Updated the Figma MCP plugin to support detailed document information retrieval and design command execution via WebSocket communication. - Improved the plugin UI to facilitate real-time interaction, including status indicators and enhanced user feedback. - Refactored server code to handle WebSocket connections, enabling robust communication with the Figma plugin for design operations. - Expanded documentation to reflect new features, including installation instructions and usage examples for the updated plugin and server functionalities.
- Transitioned the Figma MCP plugin to operate as a socket server, enhancing real-time communication capabilities with the MCP server. - Implemented manual connection controls in the plugin UI, allowing users to configure host and port settings for improved flexibility. - Refactored server code to utilize socket communication instead of WebSocket, streamlining the connection process and improving error handling. - Updated documentation to reflect new features, including connection management and configuration instructions for the plugin and server.
… command handling - Deleted the bridge server implementation from the Figma MCP plugin, simplifying the architecture. - Introduced a new command handling mechanism to process MCP commands directly within the plugin. - Updated the plugin UI to reflect changes in connection management and command execution. - Enhanced documentation to clarify the new command handling process and plugin functionality.
- Removed the previous bridge server implementation, simplifying the plugin's architecture. - Implemented a WebSocket server directly within the plugin for real-time communication with the MCP server. - Updated the plugin UI to manage WebSocket server controls, including start/stop functionality and connection testing. - Enhanced documentation to reflect the new WebSocket server architecture and usage instructions. - Refactored server code to utilize WebSocket connections, improving command handling and error management.
- Deleted the Figma MCP server implementation and associated files, simplifying the project structure. - Removed references to Figma in configuration files and dependencies, including the `pyproject.toml` and `uv.lock`. - Updated orchestration tools to eliminate Figma-related mappings and server types. - Cleaned up server configuration files to reflect the removal of the Figma server, enhancing maintainability.
- Introduced a new WebSocket mock server entry point in the `pyproject.toml` for enhanced testing capabilities. - Updated server access configurations to include the WebSocket mock server, improving modularity. - Enhanced orchestration tools to support the new WebSocket mock server, facilitating multi-server management.
- Refactored the Figma MCP plugin to operate as a WebSocket client, enabling real-time communication with the MCP server. - Updated the plugin UI to manage connection controls, including connect/disconnect functionality and server URL configuration. - Enhanced the WebSocket client implementation to handle connection status, message simulation, and command execution. - Added new server configuration for the Figma MCP server in `servers.yaml`, supporting design automation workflows. - Improved documentation to reflect the new client architecture and usage instructions for connecting to the MCP server.
…ementation - Removed the WebSocket client architecture from the plugin, streamlining the codebase. - Updated the plugin UI to eliminate connection management features, focusing on core functionality. - Refactored design command execution to operate independently of WebSocket communication. - Enhanced documentation to reflect the removal of WebSocket features and provide clearer usage instructions.
…uration updates - Added a new WebSocket mock server configuration in `servers.yaml` for testing WebSocket communications. - Updated the WebSocket mock server implementation to always start automatically upon MCP server startup, removing the need for manual start/stop tools. - Enhanced the server's retry logic for starting on available ports, improving reliability. - Removed obsolete auto-start configuration options from the server and related tools, streamlining the codebase. - Updated documentation to reflect the new automatic startup behavior and configuration options.
- Removed the auto-start WebSocket configuration from `servers.yaml`, as the WebSocket server now starts automatically with the MCP server. - Simplified the server startup logic by eliminating manual startup handling and integrating background startup for the WebSocket server. - Updated the Figma MCP server tools to remove obsolete start/stop functions, enhancing code clarity. - Enhanced documentation to reflect the new automatic startup behavior and configuration changes.
- Added logging for received messages in the plugin to improve debugging. - Implemented new message types in the UI for better communication feedback, including acknowledgment of plugin registration and WebSocket connection status. - Updated the UI to reflect connection test results and disconnection warnings, enhancing user awareness of plugin status.
- Added signal handlers for graceful WebSocket server shutdown on termination signals. - Enhanced the server's shutdown logic to keep the WebSocket server running during normal client disconnections. - Improved cleanup procedures for the WebSocket event loop and pending tasks during shutdown. - Introduced tracking for the WebSocket thread and loop for better resource management.
- Improved logging for WebSocket connection and disconnection events, providing clearer feedback in the UI. - Added acknowledgment messages for connection and disconnection events to improve communication reliability. - Introduced new message handlers for 'pong' and document-related responses, enhancing server-client interaction. - Enhanced error logging for better debugging of message handling issues.
- Changed the broadcast status message from "broadcast_sent" to "command_broadcast" for clarity in the Figma server's messaging. - Added a new fixture for sample Figma server configuration to streamline testing. - Enhanced WebSocket server status reporting by including total clients connected. - Updated end-to-end tests to reflect automatic server startup and improved error handling scenarios, ensuring accurate testing of server lifecycle and client interactions.
- Deleted the Figma WebSocket test client script to streamline the codebase. - Updated the Figma MCP server tools to include only essential functionalities, replacing obsolete methods with `get_state` and `execute_command`. - Enhanced documentation to reflect the changes in available tools and server capabilities.
- Refactored the Figma plugin to support executing arbitrary JavaScript code, enhancing its capabilities for design automation. - Updated messaging between the plugin and server to handle code execution requests and responses. - Replaced design command execution with a more flexible code execution model, allowing users to run custom scripts. - Enhanced error handling and logging for code execution results, improving debugging and user feedback. - Updated the plugin UI to reflect changes in functionality, including a new button for testing code execution.
- Streamlined the execution of JavaScript code by directly evaluating it in the main context, improving compatibility with Figma API calls. - Enhanced output capturing and error handling, providing clearer feedback on execution results. - Removed the sandbox context in favor of a more straightforward execution model, simplifying the code structure and improving performance.
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis update introduces a complete Figma integration for the Lightfast MCP platform, including a new Figma MCP server, a Figma plugin with a WebSocket client, comprehensive documentation, configuration, and extensive tests. The server enables real-time design automation by executing JavaScript and design commands in Figma via WebSocket. Supporting infrastructure, error handling, and mock servers for testing are included. Changes
Sequence Diagram(s)sequenceDiagram
participant Claude as AI Client
participant MCP as Figma MCP Server (HTTP)
participant WS as Figma WebSocket Server
participant Plugin as Figma Plugin (WebSocket Client)
participant Figma as Figma Editor
Claude->>MCP: HTTP request (get_state/execute_code)
MCP->>WS: Send command/request to plugin
WS->>Plugin: WebSocket message (command)
Plugin->>Figma: Execute JS/design command via Figma API
Plugin->>WS: WebSocket response (result/document info)
WS->>MCP: Response/result
MCP->>Claude: HTTP response (result/state)
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
🔭 Outside diff range comments (1)
pyproject.toml (1)
44-56:⚠️ Potential issueBroken entry-points – referenced modules do not exist
The new script targets use the
_mcp_serversuffix, but the actual Python packages live under...servers.websocket_mock.serverand...servers.figma.server.Trying to run
uv run lightfast-websocket-mock-serveroruv run lightfast-figma-serverwill therefore raiseModuleNotFoundError.Proposed diff:
-lightfast-websocket-mock-server = "lightfast_mcp.servers.websocket_mock_server:main" -lightfast-figma-server = "lightfast_mcp.servers.figma_mcp_server:main" +lightfast-websocket-mock-server = "lightfast_mcp.servers.websocket_mock.server:main" +lightfast-figma-server = "lightfast_mcp.servers.figma.server:main"(You already added the correct module paths further down for the “direct modular server” aliases; mirror those here.)
♻️ Duplicate comments (1)
src/lightfast_mcp/servers/figma/websocket_server.py (1)
442-449: Same unusedrequest_idissue in_handle_code_execution_responseDuplicate of the previous linter finding – either use it (e.g., in the log line) or drop it.
- request_id = data.get("request_id") + # request_id is available for correlation but currently unused +# request_id = data.get("request_id")🧰 Tools
🪛 Ruff (0.11.9)
446-446: Local variable
request_idis assigned to but never usedRemove assignment to unused variable
request_id(F841)
🪛 GitHub Actions: CI/CD Pipeline
[error] 446-446: ESLint: Local variable
request_idis assigned to but never used. (F841)
🧹 Nitpick comments (29)
tests/conftest.py (1)
56-65: Enhance test fixtures
Thesample_figma_configfixture aligns with existing patterns and will simplify Figma server tests. Consider also adding asample_websocket_mock_configfixture (withtype="websocket_mock",websocket_host, andwebsocket_port) to standardize setup for the new WebSocket Mock server tests.quick_test.py (1)
12-13: Consider making the WebSocket URL configurable.While the hardcoded URL is acceptable for a quick test script, consider making it configurable via command-line arguments or environment variables for better flexibility.
+import sys + +async def test(url: str = "ws://localhost:9003"): try: print("Attempting to connect...") websocket = await asyncio.wait_for( - websockets.connect("ws://localhost:9003"), timeout=5.0 + websockets.connect(url), timeout=5.0 )And update the main block:
if __name__ == "__main__": - asyncio.run(test()) + url = sys.argv[1] if len(sys.argv) > 1 else "ws://localhost:9003" + asyncio.run(test(url))src/tools/orchestration/config_loader.py (1)
174-202: Consider improving modularity and addressing potential shallow copy limitation.The implementation is functional but has some areas for improvement:
Modularity: The hard-coded
env_mappingsdictionary (acknowledged by the TODO comment) limits extensibility for new server types.Shallow copy limitation: Using
config.copy()creates a shallow copy, which could be problematic if the config contains nested dictionaries that also need environment variable injection.Consider these improvements:
- def _inject_environment_variables(self, config: dict[str, Any]) -> dict[str, Any]: - """Inject environment variables into configuration values.""" - # Create a copy to avoid modifying the original - config = config.copy() + def _inject_environment_variables(self, config: dict[str, Any]) -> dict[str, Any]: + """Inject environment variables into configuration values.""" + import copy + # Create a deep copy to avoid modifying the original and handle nested dicts + config = copy.deepcopy(config) - # Define environment variable mappings for different server types - # @todo more modular... - env_mappings = { - "github": { - "api_token": "GITHUB_TOKEN", - "personal_access_token": "GITHUB_PERSONAL_ACCESS_TOKEN", - }, - "openai": {"api_key": "OPENAI_API_KEY"}, - "anthropic": {"api_key": "ANTHROPIC_API_KEY"}, - } + # Load environment mappings from a more modular source + env_mappings = self._get_environment_mappings()Also consider adding a separate method
_get_environment_mappings()that could load mappings from a configuration file or registry pattern.addons/figma/package.json (1)
1-59: LGTM! Well-structured package.json for Figma plugin development.The configuration includes appropriate dependencies, comprehensive ESLint setup with TypeScript and Figma plugin support, and essential development scripts.
Consider pinning the
wsdependency to a specific version instead of using a range:- "ws": "^8.18.2" + "ws": "8.18.2"This ensures consistent behavior across different environments and aligns with the pinned package manager approach.
src/lightfast_mcp/servers/figma_mcp_server.py (1)
25-27: Consider using importlib for module resolution instead of sys.path manipulation.While the current approach works, directly manipulating
sys.pathcan be fragile. Consider usingimportlibor ensuring the package is properly installed instead.Alternative approach:
-# Add the project root to the Python path -project_root = Path(__file__).parent.parent.parent.parent -sys.path.insert(0, str(project_root)) +# Ensure proper package imports - consider using importlib or proper installationsrc/lightfast_mcp/servers/figma/README.md (3)
9-18: Add language specification to code block.The architecture diagram code block should specify a language for better rendering and consistency.
-``` +```text ┌─────────────────┐ WebSocket ┌─────────────────┐ MCP HTTP ┌─────────────────┐ │ Figma Plugin │ ←──────────────→ │ MCP Server │ ←──────────────→ │ AI Client │ │ (WebSocket │ localhost:9003 │ (WebSocket │ localhost:8003 │ (HTTP Client) │ │ Client) │ │ Server) │ │ │ │ - Figma API │ │ - WebSocket Srv │ │ - Tool Registry │ │ - Document Ops │ │ - MCP Tools │ │ - AI Integration│ │ - Design Cmds │ │ - State Mgmt │ │ - Conversation │ └─────────────────┘ └─────────────────┘ └─────────────────┘ -``` +``` <details> <summary>🧰 Tools</summary> <details> <summary>🪛 markdownlint-cli2 (0.17.2)</summary> 9-9: Fenced code blocks should have a language specified null (MD040, fenced-code-language) </details> </details> --- `292-299`: **Add language specification to directory structure.** The directory structure code block should specify a language for better rendering. ```diff -``` +```text figma/ ├── __init__.py # Module exports ├── server.py # Main MCP server implementation ├── websocket_server.py # WebSocket server implementation ├── tools.py # MCP tools implementation └── README.md # This file -``` +``` <details> <summary>🧰 Tools</summary> <details> <summary>🪛 markdownlint-cli2 (0.17.2)</summary> 292-292: Fenced code blocks should have a language specified null (MD040, fenced-code-language) </details> </details> --- `200-270`: **Consider using proper headings instead of emphasis.** The WebSocket protocol message type sections use emphasis (**bold**) instead of proper markdown headings, which affects document structure and accessibility. ```diff -**Plugin Info** +#### Plugin Info -**Document Update** +#### Document Update -**Design Command Response** +#### Design Command Response -**Welcome** +#### Welcome -**Execute Design Command** +#### Execute Design Command -**Ping** +#### Ping🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
202-202: Emphasis used instead of a heading
null(MD036, no-emphasis-as-heading)
214-214: Emphasis used instead of a heading
null(MD036, no-emphasis-as-heading)
227-227: Emphasis used instead of a heading
null(MD036, no-emphasis-as-heading)
241-241: Emphasis used instead of a heading
null(MD036, no-emphasis-as-heading)
254-254: Emphasis used instead of a heading
null(MD036, no-emphasis-as-heading)
264-264: Emphasis used instead of a heading
null(MD036, no-emphasis-as-heading)
src/lightfast_mcp/servers/websocket_mock/server.py (1)
64-103: Deduplicate retry logic & make port probing clearerThe current loop repeats identical
retry_countbookkeeping in both theif success … else …branch and theexceptbranch, leading to duplicated code paths and harder maintenance.A cleaner pattern:
max_retries = 3 while self.websocket_server.port < start_port + max_retries: try: if await self.websocket_server.start(): logger.info("✅ WebSocket server started") break except Exception as e: logger.warning(f"⚠️ Attempt failed on port {self.websocket_server.port}: {e}") self.websocket_server.port += 1 else: raise RuntimeError("Could not start WebSocket server")This eliminates the double counter and simultaneously documents the probing strategy (increment port until
max_retriesexhausted).addons/figma/websocket-server.js (1)
53-101: Add explicit UTF-8 validation / length checks for incoming JSON
JSON.parse(data.toString())assumes small, valid, UTF-8 payloads.
For robustness (and to reduce DoS vectors) consider:
- Rejecting messages above a reasonable size (e.g. 64 KB).
- Catching malformed UTF-8 before
JSON.parse.- Using a streamed JSON parser if very large payloads are expected.
src/lightfast_mcp/servers/websocket_mock/README.md (2)
360-369: Specify a language for the fenced directory-tree block
markdownlintflagged the “Architecture” code block because the opening triple-backtick has no language specifier (MD040).
Although this is only documentation, adding a language such astext(orbashif you prefer) prevents some renderers from falling back to generic formatting and removes the lint warning.-``` +```text websocket_mock/ ├── __init__.py # Module exports ...🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
362-362: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
415-436: Minor wording & punctuation improvementsThere are a handful of small grammar issues (“repeated verbs”, missing articles / commas) that distract from an otherwise excellent README.
Consider running the file through a linter likecodespellormarkdownlintwith rule MD036 disabled, or quickly editing the following:
- L.423 “WebSocket Connection Drops – Check network connectivity” (remove second verb repetition)
- L.431 “WebSocket Connection Drops – Verify firewall settings, monitor server logs” (comma before and)
No functional impact, purely polish.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
415-415: Emphasis used instead of a heading
null(MD036, no-emphasis-as-heading)
424-424: Emphasis used instead of a heading
null(MD036, no-emphasis-as-heading)
432-432: Emphasis used instead of a heading
null(MD036, no-emphasis-as-heading)
tests/integration/test_websocket_mock_integration.py (1)
39-43: Double-stop of the WebSocket server can hide errors
websocket_serveris stopped in the fixture’sfinally, but individual tests (e.g.test_mcp_server_websocket_integration) also callawait websocket_server.stop()inside their ownfinally.
Most implementations tolerate multiplestop()calls, yet if the method ever raises when the server is already closed the whole test session will fail unexpectedly.Either:
- Remove the extra
stop()in the test, or- Guard it with
if websocket_server.is_running:.addons/figma/code.ts (1)
125-136: Duplicate helper performs same work – consolidate
sendDocumentInfoToUIandsendDocumentInfoForServerare identical except for the optionalrequest_id.
Simplify maintenance by having one delegate to the other:-async function sendDocumentInfoToUI() { - ... -} - -async function sendDocumentInfoForServer(requestId?: string) { +async function sendDocumentInfoForServer(requestId?: string) { ... } + +// Back-compat wrapper +const sendDocumentInfoToUI = () => sendDocumentInfoForServer(undefined);src/lightfast_mcp/servers/figma/server.py (1)
178-205: Retry loop ignores asyncio-cancelled errorsIf
await self.websocket_server.start()is cancelled (e.g. during rapid shutdown), the except block treats it as an ordinary exception and retries, masking the cancellation.
Consider explicitly re-raisingasyncio.CancelledErrorto allow graceful exit.except asyncio.CancelledError: raise except Exception as e: ...tests/unit/test_websocket_mock_server.py (1)
60-71: Minor simplification: replace the custom async wrapper withAsyncMockSince
AsyncMockis await-able, you can mockwebsockets.servedirectly:-with patch( - "lightfast_mcp.servers.websocket_mock.websocket_server.websockets.serve" -) as mock_serve: - mock_server = AsyncMock() - - async def mock_serve_func(*args, **kwargs): - return mock_server - - mock_serve.side_effect = mock_serve_func +with patch( + "lightfast_mcp.servers.websocket_mock.websocket_server.websockets.serve", + new_callable=AsyncMock, +) as mock_serve: + mock_server = AsyncMock() + mock_serve.return_value = mock_serverThis shortens the setup and avoids an unnecessary nested coroutine.
tests/unit/test_figma_server.py (2)
208-216: Resolve Ruff F841 –_serveris never usedThe local
servervariable is only created to trigger the background-thread logic and is not referenced later, so Ruff flags it. Prefix it with_(or remove it) to silence the warning without altering behaviour:- server = FigmaMCPServer(sample_figma_config) + _server = FigmaMCPServer(sample_figma_config)🧰 Tools
🪛 Ruff (0.11.9)
210-210: Local variable
serveris assigned to but never usedRemove assignment to unused variable
server(F841)
🪛 GitHub Actions: CI/CD Pipeline
[error] 210-210: ESLint: Local variable
serveris assigned to but never used. (F841)
221-224: Same unused variable issue in signal-handler testApply the same rename here:
- server = FigmaMCPServer(sample_figma_config) + _server = FigmaMCPServer(sample_figma_config)This will clear the second F841 lint error reported by CI.
🧰 Tools
🪛 Ruff (0.11.9)
221-221: Local variable
serveris assigned to but never usedRemove assignment to unused variable
server(F841)
🪛 GitHub Actions: CI/CD Pipeline
[error] 221-221: ESLint: Local variable
serveris assigned to but never used. (F841)addons/figma/ui.html (2)
192-218: Add a style for the “disconnecting” state
disconnectFromServer()callsupdateConnectionStatus('disconnecting', …)but no corresponding CSS class exists, so the banner loses its background/colour when disconnecting:@@ .connection-status.warning { background: #fff3cd; color: #856404; border: 1px solid #ffeaa7; } + +.connection-status.disconnecting { + background: #fff3cd; + color: #856404; + border: 1px solid #ffeaa7; +}
290-295: Prevent unbounded log growth
log()keeps appending totextContent, which can balloon memory usage during long sessions. Consider capping the number of stored lines:function log(message) { const output = document.getElementById('output'); const timestamp = new Date().toLocaleTimeString(); - output.textContent += `\n[${timestamp}] ${message}`; + const lines = (output.textContent + `\n[${timestamp}] ${message}`).split('\n'); + const MAX_LINES = 1000; + output.textContent = lines.slice(-MAX_LINES).join('\n'); output.scrollTop = output.scrollHeight; }A rolling buffer keeps the UI responsive without losing recent context.
addons/figma/README.md (2)
88-88: Use a clearer preposition in the test step“Start the MCP server on
ws://localhost:9003” reads a little awkwardly.
Using “at” (or “on port”) is more idiomatic for network endpoints.-1. Start the MCP server on `ws://localhost:9003` +1. Start the MCP server at `ws://localhost:9003`🧰 Tools
🪛 LanguageTool
[uncategorized] ~88-~88: The preposition ‘to’ seems more likely in this position.
Context: ...`` ### Testing 1. Start the MCP server onws://localhost:90032. Load the plugi...(AI_HYDRA_LEO_REPLACE_ON_TO)
124-124: Add missing determiner for smoother readingAn article is expected before “plugin UI”.
-6. Verify WebSocket server settings in plugin UI +6. Verify the WebSocket server settings in the plugin UI🧰 Tools
🪛 LanguageTool
[uncategorized] ~124-~124: You might be missing the article “the” here.
Context: ... 6. Verify WebSocket server settings in plugin UI(AI_EN_LECTOR_MISSING_DETERMINER_THE)
src/lightfast_mcp/servers/figma/tools.py (1)
32-44:ctxparameter is never used – rename to_ctxor leverage itBoth tool functions accept
ctxbut ignore it, which may confuse static analysis and future readers.-async def get_state(ctx: Context) -> str: +async def get_state(_ctx: Context) -> str:Apply the same change to
execute_code.
Alternatively, make use ofctx(e.g., for logging request origin).src/lightfast_mcp/servers/websocket_mock/websocket_server.py (2)
67-84: Duplicate_is_websocket_closedlogic – consider centralising in shared utilThe exact same helper already lives in
servers/websocket_mock/tools.py. Maintaining two copies invites drift and inconsistencies when you eventually tweak the logic for newwebsocketsreleases.- def _is_websocket_closed(self, websocket) -> bool: - """Check if a WebSocket connection is closed (compatible with different websockets versions).""" - ... + # TODO: import from lightfast_mcp.servers.websocket_mock.toolsA tiny helper is better kept once and imported where needed.
99-128: Make server tuning parameters configurableHard-coding
ping_interval,ping_timeout, andclose_timeoutmakes it tough to adapt to slow networks or aggressive keep-alive requirements in production.Expose them via
__init__(..., *, ping_interval=30, ping_timeout=10, close_timeout=10)with sane defaults and pass through towebsockets.serve.tests/e2e/test_websocket_mock_e2e.py (3)
130-141: Repeated connection boiler-plate – extract into fixture for clarityThe manual loop that opens N clients appears multiple times. A small async fixture such as
multiple_clients(uri, count)would:
- reduce duplication
- guarantee proper cleanup
- centralise the welcome-message skip logic
Not critical, but it’ll make these sizeable tests easier to digest.
150-160: Tight 1 s timeouts may cause CI flakiness
await asyncio.wait_for(client.recv(), timeout=1.0)is aggressive on busy runners. Consider bumping to 3-5 s (or readingpytest’s--timeoutsetting) to avoid intermittent failures.
61-66: Comment does not match implementationThe step comment says “server should be running due to auto-start” but auto-start is disabled. Update the comment to avoid confusion for future maintainers.
tests/e2e/test_figma_e2e.py (1)
20-36: Globaltools._current_servernot reset between testsEach test sets
tools._current_serverimplicitly via theFigmaMCPServerconstructor but never restoresNone. Should a test fail early, later tests might interact with a half-configured server, causing cascading failures.Add a fixture or
try/finallyto save & restore the original value:orig_server = tools._current_server tools._current_server = server try: ... finally: tools._current_server = orig_serverThis keeps global state predictable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
addons/figma/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (37)
addons/figma/.gitignore(1 hunks)addons/figma/README.md(1 hunks)addons/figma/code.ts(1 hunks)addons/figma/manifest.json(1 hunks)addons/figma/package.json(1 hunks)addons/figma/tsconfig.json(1 hunks)addons/figma/ui.html(1 hunks)addons/figma/websocket-server.js(1 hunks)config/servers.yaml(1 hunks)pyproject.toml(4 hunks)quick_test.py(1 hunks)src/lightfast_mcp/core/base_server.py(3 hunks)src/lightfast_mcp/exceptions.py(1 hunks)src/lightfast_mcp/servers/__init__.py(1 hunks)src/lightfast_mcp/servers/figma/README.md(1 hunks)src/lightfast_mcp/servers/figma/__init__.py(1 hunks)src/lightfast_mcp/servers/figma/server.py(1 hunks)src/lightfast_mcp/servers/figma/tools.py(1 hunks)src/lightfast_mcp/servers/figma/websocket_server.py(1 hunks)src/lightfast_mcp/servers/figma_mcp_server.py(1 hunks)src/lightfast_mcp/servers/websocket_mock/README.md(1 hunks)src/lightfast_mcp/servers/websocket_mock/__init__.py(1 hunks)src/lightfast_mcp/servers/websocket_mock/server.py(1 hunks)src/lightfast_mcp/servers/websocket_mock/tools.py(1 hunks)src/lightfast_mcp/servers/websocket_mock/websocket_server.py(1 hunks)src/lightfast_mcp/servers/websocket_mock_server.py(1 hunks)src/tools/orchestration/config_loader.py(2 hunks)src/tools/orchestration/server_orchestrator.py(1 hunks)tests/conftest.py(1 hunks)tests/e2e/test_figma_e2e.py(1 hunks)tests/e2e/test_websocket_mock_e2e.py(1 hunks)tests/integration/test_figma_integration.py(1 hunks)tests/integration/test_websocket_mock_integration.py(1 hunks)tests/unit/test_figma_server.py(1 hunks)tests/unit/test_modular_servers.py(6 hunks)tests/unit/test_websocket_mock_server.py(1 hunks)tests/utils/websocket_test_client.py(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (10)
src/lightfast_mcp/core/base_server.py (1)
src/tools/common/logging.py (2)
info(104-106)error(112-126)
src/lightfast_mcp/servers/websocket_mock/__init__.py (1)
src/lightfast_mcp/servers/websocket_mock/server.py (1)
WebSocketMockMCPServer(13-139)
src/lightfast_mcp/servers/figma/__init__.py (1)
src/lightfast_mcp/servers/figma/server.py (1)
FigmaMCPServer(13-260)
tests/conftest.py (1)
src/lightfast_mcp/core/base_server.py (1)
ServerConfig(20-41)
src/lightfast_mcp/servers/__init__.py (3)
src/lightfast_mcp/servers/figma/server.py (1)
FigmaMCPServer(13-260)src/lightfast_mcp/servers/mock/server.py (1)
MockMCPServer(13-82)src/lightfast_mcp/servers/websocket_mock/server.py (1)
WebSocketMockMCPServer(13-139)
src/tools/orchestration/config_loader.py (1)
src/tools/common/logging.py (1)
info(104-106)
tests/unit/test_websocket_mock_server.py (4)
src/lightfast_mcp/core/base_server.py (1)
ServerConfig(20-41)src/lightfast_mcp/servers/websocket_mock/server.py (5)
WebSocketMockMCPServer(13-139)_register_tools(40-58)_on_startup(60-104)_on_shutdown(106-119)_perform_health_check(121-139)src/lightfast_mcp/servers/websocket_mock/websocket_server.py (11)
WebSocketClient(25-44)WebSocketMockServer(47-407)get_server_info(381-407)start(99-132)stop(134-160)_handle_ping(244-253)_handle_echo(255-264)_handle_get_stats(311-336)_handle_simulate_delay(338-352)_handle_error_test(354-379)to_dict(34-44)src/lightfast_mcp/servers/websocket_mock/tools.py (5)
set_current_server(23-26)get_websocket_server_status(48-83)send_websocket_message(86-178)get_websocket_clients(181-223)test_websocket_connection(226-329)
src/lightfast_mcp/servers/figma/tools.py (4)
src/lightfast_mcp/exceptions.py (3)
FigmaCommandError(70-73)FigmaConnectionError(64-67)FigmaMCPError(58-61)src/tools/common/logging.py (2)
info(104-106)error(112-126)tests/integration/test_websocket_mock_integration.py (1)
websocket_server(37-42)src/lightfast_mcp/servers/figma/websocket_server.py (1)
send_command_to_plugin(460-486)
tests/integration/test_figma_integration.py (4)
src/lightfast_mcp/core/base_server.py (1)
ServerConfig(20-41)src/lightfast_mcp/servers/figma/server.py (1)
FigmaMCPServer(13-260)src/lightfast_mcp/servers/figma/websocket_server.py (5)
FigmaClient(25-46)FigmaWebSocketServer(49-540)start(103-159)stop(161-187)to_dict(35-46)tests/conftest.py (1)
sample_figma_config(57-65)
src/lightfast_mcp/servers/websocket_mock/websocket_server.py (2)
src/lightfast_mcp/servers/websocket_mock/tools.py (1)
_is_websocket_closed(29-45)src/tools/common/logging.py (4)
warning(108-110)info(104-106)error(112-126)debug(100-102)
🪛 Biome (1.9.4)
addons/figma/code.ts
[error] 265-265: eval() exposes to security risks and performance issues.
See the MDN web docs for more details.
Refactor the code so that it doesn't need to call eval().
(lint/security/noGlobalEval)
addons/figma/websocket-server.js
[error] 134-135: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 138-141: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🪛 LanguageTool
src/lightfast_mcp/servers/figma/README.md
[uncategorized] ~182-~182: A punctuation mark might be missing here.
Context: ...ion | |--------|---------|-------------| | figma_host | localhost | Host for ...
(AI_EN_LECTOR_MISSING_PUNCTUATION)
[uncategorized] ~363-~363: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ...Executing** - Verify plugin is connected and active - Check plugin logs for errors -...
(COMMA_COMPOUND_SENTENCE_2)
addons/figma/README.md
[uncategorized] ~88-~88: The preposition ‘to’ seems more likely in this position.
Context: ...`` ### Testing 1. Start the MCP server on ws://localhost:9003 2. Load the plugi...
(AI_HYDRA_LEO_REPLACE_ON_TO)
[uncategorized] ~124-~124: You might be missing the article “the” here.
Context: ... 6. Verify WebSocket server settings in plugin UI
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
src/lightfast_mcp/servers/websocket_mock/README.md
[uncategorized] ~74-~74: You might be missing the article “a” here.
Context: ...clients - get_websocket_clients - Get list of connected clients - `test_websocket_...
(AI_EN_LECTOR_MISSING_DETERMINER_A)
[uncategorized] ~100-~100: A punctuation mark might be missing here.
Context: ...ion | |--------|---------|-------------| | websocket_host | localhost | Host ...
(AI_EN_LECTOR_MISSING_PUNCTUATION)
[grammar] ~399-~399: You’ve repeated a verb. Did you mean to only write one of them?
Context: ...unication testing ### 🔧 Development & Debugging - Debug WebSocket communication issues - Protot...
(REPEATED_VERBS)
🪛 markdownlint-cli2 (0.17.2)
src/lightfast_mcp/servers/figma/README.md
9-9: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
202-202: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
214-214: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
227-227: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
241-241: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
254-254: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
264-264: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
292-292: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
345-345: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
354-354: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
362-362: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
src/lightfast_mcp/servers/websocket_mock/README.md
120-120: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
128-128: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
137-137: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
145-145: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
152-152: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
159-159: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
167-167: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
177-177: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
191-191: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
201-201: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
211-211: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
362-362: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
415-415: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
424-424: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
432-432: Emphasis used instead of a heading
null
(MD036, no-emphasis-as-heading)
🪛 Ruff (0.11.9)
tests/unit/test_figma_server.py
210-210: Local variable server is assigned to but never used
Remove assignment to unused variable server
(F841)
221-221: Local variable server is assigned to but never used
Remove assignment to unused variable server
(F841)
src/lightfast_mcp/servers/figma/websocket_server.py
399-399: Local variable request_id is assigned to but never used
Remove assignment to unused variable request_id
(F841)
446-446: Local variable request_id is assigned to but never used
Remove assignment to unused variable request_id
(F841)
🪛 GitHub Actions: CI/CD Pipeline
tests/unit/test_figma_server.py
[error] 210-210: ESLint: Local variable server is assigned to but never used. (F841)
[error] 221-221: ESLint: Local variable server is assigned to but never used. (F841)
src/lightfast_mcp/servers/figma/websocket_server.py
[error] 399-399: ESLint: Local variable request_id is assigned to but never used. (F841)
[error] 446-446: ESLint: Local variable request_id is assigned to but never used. (F841)
🔇 Additional comments (47)
addons/figma/.gitignore (1)
1-9: Exclude typical build artifacts and logs
The.gitignorepatterns correctly ignore log files,node_modules, output directories (out/,dist/), and generated plugin code (code.js), preventing accidental commits of development artifacts.src/lightfast_mcp/servers/figma/__init__.py (1)
1-6: Expose Figma server as public API
The module-level docstring, import ofFigmaMCPServer, and__all__export establish a clear public interface for the Figma MCP server package.src/lightfast_mcp/servers/websocket_mock/__init__.py (1)
1-6: Expose WebSocket Mock server as public API
The initializer properly documents and exportsWebSocketMockMCPServer, making it easy to import and register in the MCP framework.src/lightfast_mcp/servers/__init__.py (1)
1-14: Aggregate MCP server exports
The package initializer cleanly imports and lists all MCP server classes—including the newly added Figma and WebSocket Mock servers—in__all__, centralizing server access and improving import ergonomics.src/tools/orchestration/server_orchestrator.py (1)
440-441: Good defensive programming practice.The addition of a safety check before deleting from
_running_serversprevents potentialKeyErrorexceptions that could occur in race conditions or ifstop_serveris called multiple times on the same server. This improves robustness of the server lifecycle management.src/lightfast_mcp/core/base_server.py (4)
82-82: Good addition for debugging server lifecycle.The entry log message helps track when the server lifespan context manager is entered, which is valuable for debugging server startup issues and understanding the server lifecycle flow.
99-101: Excellent improvement for error diagnostics.Adding detailed traceback logging during startup exceptions significantly improves debugging capabilities. The traceback information will help developers quickly identify the root cause of startup failures.
186-189: Helpful startup information logging.The detailed startup log provides valuable runtime information including server name, transport type, and URL/stdio mode. This improves operational visibility and makes it easier to verify server configuration.
191-191: Clarified comment improves understanding.The updated comment clearly explains that FastMCP manages the event loop, which helps developers understand the transport handling responsibilities.
quick_test.py (1)
9-21: Well-structured WebSocket test implementation.The test function properly handles timeouts, exceptions, and graceful connection closure. The 5-second timeouts are appropriate for a quick connectivity test.
config/servers.yaml (3)
29-43: Well-structured Figma server configuration.The configuration follows consistent patterns with existing servers and properly separates the MCP server port (8003) from the Figma WebSocket service port (9003). The required "Figma" app dependency is appropriate.
44-57: Consistent WebSocket mock server configuration.The configuration maintains consistency with other server entries and uses appropriate port separation (8004 for MCP, 9004 for WebSocket). No required apps makes sense for a mock server.
29-57:❓ Verification inconclusive
Verify port assignments don't conflict with existing services.
Please ensure the assigned ports (8003, 8004, 9003, 9004) don't conflict with other services in your environment or with any default ports used by the applications.
🏁 Script executed:
#!/bin/bash # Description: Check if any of the new ports are commonly used by other services # Expected: Information about potential port conflicts echo "Checking port assignments for potential conflicts..." echo "New ports: 8003 (Figma MCP), 8004 (WebSocket Mock MCP), 9003 (Figma WebSocket), 9004 (WebSocket Mock WebSocket)" echo "" # Check common port assignments echo "Common port usage for assigned ports:" echo "Port 8003: Often used by development servers" echo "Port 8004: Often used by development servers" echo "Port 9003: Less commonly used, good choice" echo "Port 9004: Less commonly used, good choice" echo "" # Check if ports are currently in use (this will work on Unix-like systems) for port in 8003 8004 9003 9004; do if command -v netstat >/dev/null 2>&1; then result=$(netstat -ln 2>/dev/null | grep ":$port " || echo "Port $port appears to be available") echo "Port $port: $result" elif command -v ss >/dev/null 2>&1; then result=$(ss -ln 2>/dev/null | grep ":$port " || echo "Port $port appears to be available") echo "Port $port: $result" else echo "Port $port: Cannot check availability (netstat/ss not available)" fi doneLength of output: 1674
Action Required: Manually Verify Port Availability
The sandbox check couldn’t determine if ports 8003, 8004, 9003, and 9004 are already in use. Before merging, please confirm these ports don’t conflict with any services in your deployment environment or with defaults used by Figma or your WebSocket mocks.• File: config/servers.yaml (lines 29–57)
• Ports to verify: 8003 (Figma MCP), 8004 (WebSocket Mock MCP), 9003 (Figma WebSocket), 9004 (WebSocket Mock WebSocket)Recommended commands on your host:
# Linux/macOS lsof -i:8003,8004,9003,9004 # or netstat -plnt | grep -E ':(8003|8004|9003|9004)\b'Adjust ports or update configurations if any are in use.
addons/figma/tsconfig.json (1)
1-23: LGTM! Well-configured TypeScript setup for Figma plugin development.The configuration follows best practices with strict type checking, appropriate ES2020 target, and proper inclusion of Figma plugin types. The output directory correctly aligns with the manifest.json main field.
addons/figma/manifest.json (1)
1-20: LGTM! Well-structured Figma plugin manifest with appropriate WebSocket permissions.The manifest correctly configures network access for WebSocket communication with the local MCP server. The reasoning field clearly explains the purpose of the network permissions.
Please verify that the plugin ID
1509155185436575957is properly registered with Figma and matches your development environment.src/tools/orchestration/config_loader.py (1)
154-156: LGTM! Environment variable injection enhances configuration flexibility.The placement of environment variable injection after setting the server type is logical and ensures proper processing order.
src/lightfast_mcp/servers/websocket_mock_server.py (2)
20-64: LGTM! Well-structured configuration handling.The main function demonstrates good practices:
- Environment configuration support for orchestrator integration
- Proper JSON parsing with error handling
- Graceful fallback to default configuration
- Clear logging of configuration source
67-81: Good default configuration setup.The default configuration is well-structured with appropriate defaults and clear documentation in comments.
src/lightfast_mcp/servers/figma_mcp_server.py (2)
43-91: Comprehensive CLI argument parsing implementation.The argument parser is well-structured with helpful descriptions, examples, and appropriate defaults. Good use of argparse features.
146-154: Excellent error handling with conditional verbosity.The exception handling properly distinguishes between user interruption and actual errors, with optional verbose traceback output.
src/lightfast_mcp/exceptions.py (1)
58-85: Excellent exception hierarchy following established patterns.The new Figma exception classes:
- Follow the same pattern as existing Blender exceptions
- Have proper inheritance (FigmaTimeoutError extends FigmaConnectionError)
- Include clear, descriptive docstrings
- Maintain consistency with project conventions
src/lightfast_mcp/servers/figma/README.md (1)
1-404: Comprehensive and well-structured documentation.This README provides excellent coverage of:
- Clear architecture diagrams and explanations
- Detailed feature descriptions
- Complete configuration options
- WebSocket protocol specifications
- Troubleshooting guidance
- Development instructions
The documentation effectively explains the server's purpose, capabilities, and usage patterns.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~182-~182: A punctuation mark might be missing here.
Context: ...ion | |--------|---------|-------------| |figma_host|localhost| Host for ...(AI_EN_LECTOR_MISSING_PUNCTUATION)
[uncategorized] ~363-~363: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ...Executing** - Verify plugin is connected and active - Check plugin logs for errors -...(COMMA_COMPOUND_SENTENCE_2)
🪛 markdownlint-cli2 (0.17.2)
9-9: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
202-202: Emphasis used instead of a heading
null(MD036, no-emphasis-as-heading)
214-214: Emphasis used instead of a heading
null(MD036, no-emphasis-as-heading)
227-227: Emphasis used instead of a heading
null(MD036, no-emphasis-as-heading)
241-241: Emphasis used instead of a heading
null(MD036, no-emphasis-as-heading)
254-254: Emphasis used instead of a heading
null(MD036, no-emphasis-as-heading)
264-264: Emphasis used instead of a heading
null(MD036, no-emphasis-as-heading)
292-292: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
345-345: Emphasis used instead of a heading
null(MD036, no-emphasis-as-heading)
354-354: Emphasis used instead of a heading
null(MD036, no-emphasis-as-heading)
362-362: Emphasis used instead of a heading
null(MD036, no-emphasis-as-heading)
tests/integration/test_websocket_mock_integration.py (1)
23-34:❌ Incorrect review comment
Use an ephemeral port to avoid CI flakiness
Hard-coding
websocket_port = 9998risks random “address already in use” failures when the test suite runs in parallel (or on developer machines with that port occupied).- "websocket_port": 9998, # Use different port for integration tests + "websocket_port": 0, # 0 => let OS choose a free portBecause the fixtures already refer to
server.portafter start-up, the remainder of the tests need no changes—the actual port will be filled in automatically.
🏁 Script executed:
#!/bin/bash # Locate the definition of ServerConfig rg -n "class ServerConfig" -A 5 # Find where the "websocket_mock" type is handled or the server implementation rg -n "websocket_mock" -A 5 # Verify how the integration test refers to server.port rg -n "server\\.port" -A 2 tests/integration/test_websocket_mock_integration.pyLength of output: 18300
🏁 Script executed:
#!/bin/bash # Display the WebSocketMockServer class definition and its start method rg -n "class WebSocketMockServer" -A 50 src/lightfast_mcp/servers/websocket_mock/websocket_server.pyLength of output: 2189
🏁 Script executed:
#!/bin/bash # Inspect WebSocketMockServer start method rg -n "def start" -A 40 src/lightfast_mcp/servers/websocket_mock/websocket_server.py # Inspect WebSocketMockMCPServer start method rg -n "def start" -A 80 src/lightfast_mcp/servers/websocket_mock/server.pyLength of output: 1665
🏁 Script executed:
#!/bin/bash # Show the beginning of the MCP server implementation to inspect port handling sed -n '1,200p' src/lightfast_mcp/servers/websocket_mock/server.pyLength of output: 6257
🏁 Script executed:
#!/bin/bash # Inspect the server_config fixture and related setup in integration tests rg -n "def server_config" -A 20 tests/integration/test_websocket_mock_integration.py # Show how the server fixture uses server_config and starts the server rg -n "pytest.fixture" -A 20 tests/integration/test_websocket_mock_integration.pyLength of output: 2786
🏁 Script executed:
#!/bin/bash # Search for get_server_info method in websocket_server.py rg -n "def get_server_info" -A 20 src/lightfast_mcp/servers/websocket_mock/websocket_server.pyLength of output: 993
Ignore ephemeral port recommendation
The
WebSocketMockServer.startmethod never updatesself.portafter binding to an OS‐assigned port, so passing"websocket_port": 0would leaveserver.portat 0 and all integration tests that dows://host:portwould fail. The hard-coded ports in the fixtures andserver_configmust remain unless you also updateWebSocketMockServer.startto read back the assigned port (for example viaself.server.sockets[0].getsockname()[1]).Likely an incorrect or invalid review comment.
addons/figma/code.ts (1)
327-329:figma.on('close')is deprecatedThe Figma plugin API removed the
'close'event. The recommended pattern is to perform cleanup in the promise returned byfigma.closePlugin()or in thefinallyblock of long-running tasks.Leaving this listener in may have no effect or could throw in future Figma releases.
src/lightfast_mcp/servers/websocket_mock/tools.py (6)
8-26: LGTM! Good imports and server reference pattern.The imports are well-organized and the global server reference pattern is appropriate for MCP tool modules.
29-46: Excellent defensive programming for WebSocket compatibility.This function handles different versions of the websockets library gracefully with appropriate fallbacks and safe error handling.
48-84: Well-structured status function with comprehensive information.Good error checking, detailed status reporting, and proper handling of missing configuration.
86-179: Excellent message sending implementation with robust error handling.The function properly handles both targeted and broadcast messaging with concurrent operations, comprehensive error tracking, and detailed result reporting.
181-224: Good client information retrieval with consistent error handling.The function follows established patterns and provides comprehensive client information including real-time connection status.
226-330: Comprehensive testing function with multiple test scenarios.The function supports various test types and properly reuses existing functionality. The stress test parameters are reasonable for testing purposes.
tests/integration/test_figma_integration.py (10)
1-24: Well-organized imports and test class structure.Good use of integration test markers and proper import organization for testing the Figma MCP server components.
25-54: Excellent lifecycle test with proper isolation.The test properly patches background operations and comprehensively tests the server startup/shutdown cycle with health checks.
55-123: Comprehensive tools integration test with realistic mocks.The test creates well-structured mock clients and thoroughly tests the integration between tools and the WebSocket server.
124-167: Thorough design command testing with proper verification.The test properly verifies command execution flow and parameter passing for both targeted and broadcast scenarios.
168-197: Good document state retrieval test.The test follows established patterns and properly verifies the document state request flow to the WebSocket server.
198-227: Excellent error handling coverage across scenarios.The test comprehensively covers error conditions and verifies graceful degradation when components are unavailable.
228-246: Good isolated WebSocket server lifecycle test.The test properly verifies the WebSocket server's independent behavior and safe operations.
247-283: Comprehensive FigmaClient dataclass testing.The test thoroughly covers initialization, default values, and serialization functionality of the FigmaClient dataclass.
284-324: Excellent comprehensive health check scenario testing.The test covers all combinations of server states and exception conditions, ensuring robust health monitoring.
325-431: Excellent comprehensive integration testing coverage.The remaining tests thoroughly cover error scenarios, configuration variations, and concurrent operations, providing robust integration test coverage.
tests/utils/websocket_test_client.py (4)
18-86: Well-designed WebSocket test client with excellent error handling.The class provides comprehensive WebSocket testing capabilities with proper async handling, clear user feedback, and robust error management.
87-116: Consistent and well-structured message method implementations.All message methods follow consistent patterns with appropriate parameters and good use of timestamps and test identifiers.
117-214: Excellent interactive session implementation with good UX.The interactive functionality provides a user-friendly interface with proper command parsing, help text, and graceful error handling.
216-282: Comprehensive test scenarios with excellent automation.The automated test scenarios provide thorough coverage and the main function offers a clean command-line interface with sensible defaults.
tests/unit/test_modular_servers.py (3)
179-269: Excellent Figma server test class following established patterns.The test class comprehensively covers the Figma server functionality with proper patching and follows the same excellent patterns as existing server tests.
304-358: Good integration of Figma server into existing test patterns.The integration tests properly incorporate the Figma server while maintaining consistency with existing server testing approaches.
390-467: Consistent validation and string representation test coverage.The updates properly extend existing test patterns to include Figma server validation and string representation testing.
| def _register_tools(self): | ||
| """Register WebSocket mock server tools.""" | ||
| if not self.mcp: | ||
| return | ||
|
|
||
| # Register tools from the tools module | ||
| self.mcp.tool()(tools.get_websocket_server_status) | ||
| self.mcp.tool()(tools.send_websocket_message) | ||
| self.mcp.tool()(tools.get_websocket_clients) | ||
| self.mcp.tool()(tools.test_websocket_connection) | ||
|
|
||
| # Update the server info with available tools | ||
| self.info.tools = [ | ||
| "get_websocket_server_status", | ||
| "send_websocket_message", | ||
| "get_websocket_clients", | ||
| "test_websocket_connection", | ||
| ] | ||
| logger.info(f"Registered {len(self.info.tools)} tools: {self.info.tools}") | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider deferring tool registration until self.mcp is definitely set
_register_tools() bails out early when self.mcp is falsy.
If BaseServer calls _register_tools() before injecting the MCP instance, none of the tools will ever be registered.
Please double-check the call-order in BaseServer; if uncertain, move the registration logic into _on_startup() (after the MCP is available) to guarantee predictable behaviour.
🤖 Prompt for AI Agents
In src/lightfast_mcp/servers/websocket_mock/server.py around lines 40 to 59, the
_register_tools method exits early if self.mcp is not set, which may cause tools
not to be registered if called before MCP injection. To fix this, move the tool
registration logic from _register_tools into the _on_startup method, ensuring it
runs only after self.mcp is definitely initialized, thus guaranteeing all tools
are registered predictably.
| SERVER_TYPE: ClassVar[str] = "websocket_mock" | ||
| SERVER_VERSION: ClassVar[str] = "1.0.0" | ||
| REQUIRED_DEPENDENCIES: ClassVar[list[str]] = ["websockets"] | ||
| REQUIRED_APPS: ClassVar[list[str]] = [] |
There was a problem hiding this comment.
Add ‘websockets’ to the package dependencies
The class declares REQUIRED_DEPENDENCIES = ["websockets"], but pyproject.toml currently does not list websockets under [project].dependencies.
Trying to start this server in a fresh environment will therefore raise ModuleNotFoundError.
Proposed diff (pyproject.toml shown for context):
dependencies = [
"fastmcp>=2.0.0",
"rich",
"pyyaml",
+ "websockets", # Needed by WebSocketMockMCPServer
"anthropic",
"openai",
"typer",
]📝 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.
| SERVER_TYPE: ClassVar[str] = "websocket_mock" | |
| SERVER_VERSION: ClassVar[str] = "1.0.0" | |
| REQUIRED_DEPENDENCIES: ClassVar[list[str]] = ["websockets"] | |
| REQUIRED_APPS: ClassVar[list[str]] = [] | |
| # pyproject.toml | |
| [project] | |
| dependencies = [ | |
| "fastmcp>=2.0.0", | |
| "rich", | |
| "pyyaml", | |
| "websockets", # Needed by WebSocketMockMCPServer | |
| "anthropic", | |
| "openai", | |
| "typer", | |
| ] |
🤖 Prompt for AI Agents
In src/lightfast_mcp/servers/websocket_mock/server.py lines 17 to 20, the class
specifies "websockets" as a required dependency, but this package is missing
from pyproject.toml under [project].dependencies. To fix this, add "websockets"
to the dependencies list in pyproject.toml to ensure it is installed in fresh
environments and prevent ModuleNotFoundError when starting the server.
| function processCommand(command) { | ||
| const requestId = command.id || Date.now(); | ||
|
|
||
| try { | ||
| switch (command.type) { | ||
| case 'ping': | ||
| return { | ||
| type: 'response', | ||
| requestId: requestId, | ||
| status: 'success', | ||
| result: { | ||
| message: 'pong', | ||
| timestamp: Date.now(), | ||
| server: 'figma-plugin-test', | ||
| serverRunning: true, | ||
| port: port | ||
| } | ||
| }; | ||
|
|
||
| case 'get_document_info': | ||
| return { | ||
| type: 'response', | ||
| requestId: requestId, | ||
| status: 'success', | ||
| result: mockDocumentState | ||
| }; | ||
|
|
||
| case 'execute_design_command': | ||
| const designCommand = command.params?.command || ''; | ||
| console.log(`🎨 Executing design command: ${designCommand}`); | ||
|
|
||
| let result = { | ||
| message: 'Command received but not implemented', | ||
| command: designCommand | ||
| }; | ||
|
|
||
| if (designCommand.toLowerCase().includes('create rectangle')) { | ||
| result = { | ||
| message: 'Rectangle created successfully', | ||
| command: designCommand, | ||
| created_node: { | ||
| id: `rect_${Date.now()}`, | ||
| name: 'AI Created Rectangle', | ||
| type: 'RECTANGLE' | ||
| } | ||
| }; | ||
| } else if (designCommand.toLowerCase().includes('create circle')) { | ||
| result = { | ||
| message: 'Circle created successfully', | ||
| command: designCommand, | ||
| created_node: { | ||
| id: `circle_${Date.now()}`, | ||
| name: 'AI Created Circle', | ||
| type: 'ELLIPSE' | ||
| } | ||
| }; | ||
| } else if (designCommand.toLowerCase().includes('create text')) { | ||
| result = { | ||
| message: 'Text created successfully', | ||
| command: designCommand, | ||
| created_node: { | ||
| id: `text_${Date.now()}`, | ||
| name: 'AI Created Text', | ||
| type: 'TEXT', | ||
| text: 'Hello from AI!' | ||
| } | ||
| }; | ||
| } | ||
|
|
||
| return { | ||
| type: 'response', | ||
| requestId: requestId, | ||
| status: 'success', | ||
| result: result | ||
| }; | ||
|
|
There was a problem hiding this comment.
Wrap variable declarations inside switch-case blocks to avoid scope bleed
Biome flags designCommand and result because declarations inside a case share the parent scope; subsequent cases can accidentally access them.
Wrap each case in braces or move the declarations to const statements inside a block.
Suggested fix:
- case 'execute_design_command':
- const designCommand = command.params?.command || '';
- console.log(`🎨 Executing design command: ${designCommand}`);
-
- let result = {
+ case 'execute_design_command': {
+ const designCommand = command.params?.command || '';
+ console.log(`🎨 Executing design command: ${designCommand}`);
+
+ let result = {
message: 'Command received but not implemented',
command: designCommand
};
…
- return {
+ return {
type: 'response',
requestId: requestId,
status: 'success',
result: result
};
-
+ }The surrounding braces create a block scope, satisfying noSwitchDeclarations and preventing accidental leakage.
📝 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.
| function processCommand(command) { | |
| const requestId = command.id || Date.now(); | |
| try { | |
| switch (command.type) { | |
| case 'ping': | |
| return { | |
| type: 'response', | |
| requestId: requestId, | |
| status: 'success', | |
| result: { | |
| message: 'pong', | |
| timestamp: Date.now(), | |
| server: 'figma-plugin-test', | |
| serverRunning: true, | |
| port: port | |
| } | |
| }; | |
| case 'get_document_info': | |
| return { | |
| type: 'response', | |
| requestId: requestId, | |
| status: 'success', | |
| result: mockDocumentState | |
| }; | |
| case 'execute_design_command': | |
| const designCommand = command.params?.command || ''; | |
| console.log(`🎨 Executing design command: ${designCommand}`); | |
| let result = { | |
| message: 'Command received but not implemented', | |
| command: designCommand | |
| }; | |
| if (designCommand.toLowerCase().includes('create rectangle')) { | |
| result = { | |
| message: 'Rectangle created successfully', | |
| command: designCommand, | |
| created_node: { | |
| id: `rect_${Date.now()}`, | |
| name: 'AI Created Rectangle', | |
| type: 'RECTANGLE' | |
| } | |
| }; | |
| } else if (designCommand.toLowerCase().includes('create circle')) { | |
| result = { | |
| message: 'Circle created successfully', | |
| command: designCommand, | |
| created_node: { | |
| id: `circle_${Date.now()}`, | |
| name: 'AI Created Circle', | |
| type: 'ELLIPSE' | |
| } | |
| }; | |
| } else if (designCommand.toLowerCase().includes('create text')) { | |
| result = { | |
| message: 'Text created successfully', | |
| command: designCommand, | |
| created_node: { | |
| id: `text_${Date.now()}`, | |
| name: 'AI Created Text', | |
| type: 'TEXT', | |
| text: 'Hello from AI!' | |
| } | |
| }; | |
| } | |
| return { | |
| type: 'response', | |
| requestId: requestId, | |
| status: 'success', | |
| result: result | |
| }; | |
| function processCommand(command) { | |
| const requestId = command.id || Date.now(); | |
| try { | |
| switch (command.type) { | |
| case 'ping': | |
| return { | |
| type: 'response', | |
| requestId: requestId, | |
| status: 'success', | |
| result: { | |
| message: 'pong', | |
| timestamp: Date.now(), | |
| server: 'figma-plugin-test', | |
| serverRunning: true, | |
| port: port | |
| } | |
| }; | |
| case 'get_document_info': | |
| return { | |
| type: 'response', | |
| requestId: requestId, | |
| status: 'success', | |
| result: mockDocumentState | |
| }; | |
| case 'execute_design_command': { | |
| const designCommand = command.params?.command || ''; | |
| console.log(`🎨 Executing design command: ${designCommand}`); | |
| let result = { | |
| message: 'Command received but not implemented', | |
| command: designCommand | |
| }; | |
| if (designCommand.toLowerCase().includes('create rectangle')) { | |
| result = { | |
| message: 'Rectangle created successfully', | |
| command: designCommand, | |
| created_node: { | |
| id: `rect_${Date.now()}`, | |
| name: 'AI Created Rectangle', | |
| type: 'RECTANGLE' | |
| } | |
| }; | |
| } else if (designCommand.toLowerCase().includes('create circle')) { | |
| result = { | |
| message: 'Circle created successfully', | |
| command: designCommand, | |
| created_node: { | |
| id: `circle_${Date.now()}`, | |
| name: 'AI Created Circle', | |
| type: 'ELLIPSE' | |
| } | |
| }; | |
| } else if (designCommand.toLowerCase().includes('create text')) { | |
| result = { | |
| message: 'Text created successfully', | |
| command: designCommand, | |
| created_node: { | |
| id: `text_${Date.now()}`, | |
| name: 'AI Created Text', | |
| type: 'TEXT', | |
| text: 'Hello from AI!' | |
| } | |
| }; | |
| } | |
| return { | |
| type: 'response', | |
| requestId: requestId, | |
| status: 'success', | |
| result: result | |
| }; | |
| } | |
| // other cases... | |
| } | |
| } catch (err) { | |
| // error handling | |
| } | |
| } |
🧰 Tools
🪛 Biome (1.9.4)
[error] 134-135: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 138-141: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🤖 Prompt for AI Agents
In addons/figma/websocket-server.js between lines 106 and 181, the variables
designCommand and result are declared directly inside a switch case, causing
scope leakage across cases. To fix this, wrap the entire code inside each case
block with curly braces to create a new block scope, ensuring that variable
declarations like designCommand and result are scoped only within their
respective cases and do not bleed into others.
| function executeCodeSync(code: string): any { | ||
| try { | ||
| console.log('Executing JavaScript code synchronously:', code); | ||
|
|
||
| // Capture console output | ||
| let output: string[] = []; | ||
| const originalLog = console.log; | ||
| console.log = (...args: any[]) => { | ||
| output.push(args.join(' ')); | ||
| originalLog(...args); | ||
| }; | ||
|
|
||
| let result: any = undefined; | ||
| let executed = true; | ||
| let error: string | undefined = undefined; | ||
|
|
||
| try { | ||
| // Execute code directly in the main context | ||
| // This is necessary for Figma API calls to work properly | ||
| eval(code); | ||
|
|
||
| // If code sets a result variable, use it | ||
| if (typeof result !== 'undefined') { | ||
| return { | ||
| code_executed: true, | ||
| original_code: code, | ||
| execution_result: { | ||
| executed: true, | ||
| result: result, | ||
| output: output.join('\n') || 'Code executed successfully' | ||
| }, | ||
| timestamp: Date.now() | ||
| }; | ||
| } | ||
|
|
||
| return { | ||
| code_executed: true, | ||
| original_code: code, | ||
| execution_result: { | ||
| executed: true, | ||
| result: 'Code executed successfully', | ||
| output: output.join('\n') || 'No output' | ||
| }, | ||
| timestamp: Date.now() | ||
| }; |
There was a problem hiding this comment.
eval poses security & stability risks – isolate or remove
executeCodeSync executes arbitrary strings with eval, exposing:
- XSS / code-injection if untrusted input reaches the plugin.
- Possible plugin termination or corrupted state (infinite loops,
figma.closePlugin, etc.). - Future review headaches (TypeScript compiler flags this with
noGlobalEval).
If executing arbitrary code is a hard requirement, at minimum:
- Wrap the evaluation in an
iframesandbox viafigma.showUIand communicate results withpostMessage. - Use a whitelisted API approach—parse the string into a restricted AST and allow only safe operations.
- Add an execution timeout (e.g.
Promise.race([worker, sleep(2500)])) to prevent hangs.
Otherwise, replace eval with specific helper functions that cover the required automation scenarios.
🧰 Tools
🪛 Biome (1.9.4)
[error] 265-265: eval() exposes to security risks and performance issues.
See the MDN web docs for more details.
Refactor the code so that it doesn't need to call eval().
(lint/security/noGlobalEval)
🤖 Prompt for AI Agents
In addons/figma/code.ts between lines 245 and 289, the use of eval to execute
arbitrary code poses security and stability risks. To fix this, remove the
direct eval call and instead run the code inside a sandboxed iframe using
figma.showUI, communicating results back via postMessage. Alternatively,
implement a whitelist-based parser that converts the code string into a safe AST
allowing only approved operations. Also, add an execution timeout mechanism to
prevent infinite loops or hangs. If arbitrary code execution is not essential,
replace eval with specific helper functions that perform the needed tasks
safely.
| logger.debug( | ||
| f"🔧 Server is_serving: {getattr(self.server, 'is_serving', 'unknown')()}" | ||
| ) |
There was a problem hiding this comment.
getattr(..., 'is_serving', 'unknown')() can raise TypeError
When the websockets version does not expose is_serving,
getattr returns the string 'unknown'. Calling the result ('unknown'()) raises a TypeError, breaking server start-up logging.
-logger.debug(
- f"🔧 Server is_serving: {getattr(self.server, 'is_serving', 'unknown')()}"
-)
+if hasattr(self.server, "is_serving"):
+ logger.debug(f"🔧 Server is_serving: {self.server.is_serving()}")
+else:
+ logger.debug("🔧 Server is_serving attribute not available on this websockets version")📝 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.
| logger.debug( | |
| f"🔧 Server is_serving: {getattr(self.server, 'is_serving', 'unknown')()}" | |
| ) | |
| if hasattr(self.server, "is_serving"): | |
| logger.debug(f"🔧 Server is_serving: {self.server.is_serving()}") | |
| else: | |
| logger.debug("🔧 Server is_serving attribute not available on this websockets version") |
🤖 Prompt for AI Agents
In src/lightfast_mcp/servers/figma/websocket_server.py around lines 148 to 150,
the code calls getattr(self.server, 'is_serving', 'unknown')() which raises a
TypeError if 'is_serving' is not present because it tries to call the string
'unknown' as a function. Fix this by first retrieving the attribute without
calling it, then check if it is callable before calling it; if not callable, use
a default value or the attribute itself for logging.
| for client in self.clients.values(): | ||
| if not self._is_websocket_closed(client.websocket): | ||
| close_tasks.append(client.websocket.close()) | ||
|
|
||
| if close_tasks: | ||
| await asyncio.gather(*close_tasks, return_exceptions=True) |
There was a problem hiding this comment.
Potential RuntimeError when closing clients during stop()
self.clients.values() returns a live view; if a client disconnects while you iterate (e.g. because await client.websocket.close() triggers the finally block in _handle_client) the dict size mutates and Python raises RuntimeError: dictionary changed size during iteration.
- for client in self.clients.values():
+ # Snapshot to avoid RuntimeError if dict mutates during iteration
+ for client in list(self.clients.values()):The same pattern appears in _handle_broadcast; consider applying the fix there too.
📝 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.
| for client in self.clients.values(): | |
| if not self._is_websocket_closed(client.websocket): | |
| close_tasks.append(client.websocket.close()) | |
| if close_tasks: | |
| await asyncio.gather(*close_tasks, return_exceptions=True) | |
| # Snapshot to avoid RuntimeError if dict mutates during iteration | |
| for client in list(self.clients.values()): | |
| if not self._is_websocket_closed(client.websocket): | |
| close_tasks.append(client.websocket.close()) | |
| if close_tasks: | |
| await asyncio.gather(*close_tasks, return_exceptions=True) |
🤖 Prompt for AI Agents
In src/lightfast_mcp/servers/websocket_mock/websocket_server.py around lines 146
to 151, iterating directly over self.clients.values() can cause a RuntimeError
if the dictionary changes size during iteration due to client disconnects. To
fix this, iterate over a static list copy of the clients by wrapping
self.clients.values() with list() before the loop. Apply the same fix to the
similar iteration pattern in the _handle_broadcast method to prevent the same
issue there.
| # Send to all other clients | ||
| broadcast_tasks = [] | ||
| for other_client in self.clients.values(): | ||
| if other_client.id != client.id and not self._is_websocket_closed( | ||
| other_client.websocket | ||
| ): | ||
| broadcast_tasks.append( | ||
| other_client.websocket.send(json.dumps(broadcast_data)) | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Broadcast loop vulnerable to concurrent dict mutation & adds needless awaits
- Use a snapshot to avoid the mutation issue noted above.
- Skip closed peers before scheduling
send, saving an unnecessary coroutine creation.
- broadcast_tasks = []
- for other_client in self.clients.values():
- if other_client.id != client.id and not self._is_websocket_closed(
- other_client.websocket
- ):
- broadcast_tasks.append(
- other_client.websocket.send(json.dumps(broadcast_data))
- )
+ recipients = [
+ c
+ for c in list(self.clients.values()) # snapshot
+ if c.id != client.id and not self._is_websocket_closed(c.websocket)
+ ]
+ await asyncio.gather(
+ *[c.websocket.send(json.dumps(broadcast_data)) for c in recipients],
+ return_exceptions=True,
+ )Remember to update the recipients count you send back to the sender.
📝 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.
| # Send to all other clients | |
| broadcast_tasks = [] | |
| for other_client in self.clients.values(): | |
| if other_client.id != client.id and not self._is_websocket_closed( | |
| other_client.websocket | |
| ): | |
| broadcast_tasks.append( | |
| other_client.websocket.send(json.dumps(broadcast_data)) | |
| ) | |
| # Send to all other clients | |
| recipients = [ | |
| c | |
| for c in list(self.clients.values()) # snapshot | |
| if c.id != client.id and not self._is_websocket_closed(c.websocket) | |
| ] | |
| await asyncio.gather( | |
| *[c.websocket.send(json.dumps(broadcast_data)) for c in recipients], | |
| return_exceptions=True, | |
| ) |
🤖 Prompt for AI Agents
In src/lightfast_mcp/servers/websocket_mock/websocket_server.py around lines 278
to 286, the broadcast loop iterates directly over self.clients.values(), which
can cause errors if the dictionary is mutated concurrently. Fix this by
iterating over a snapshot copy of the clients, such as a list of the values.
Also, check if the websocket is closed before scheduling the send coroutine to
avoid creating unnecessary coroutines. Finally, update the recipients count sent
back to the sender to reflect the actual number of clients the message was
broadcast to.
| @pytest.fixture | ||
| def server_config(self): | ||
| """Create a server configuration for E2E testing.""" | ||
| return ServerConfig( | ||
| name="E2ETestWebSocketMockMCP", | ||
| description="E2E Test WebSocket Mock MCP Server", | ||
| config={ | ||
| "type": "websocket_mock", | ||
| "websocket_host": "localhost", | ||
| "websocket_port": 9997, # Use different port for E2E tests | ||
| "auto_start_websocket": False, # Manual control for tests | ||
| }, | ||
| ) | ||
|
|
There was a problem hiding this comment.
auto_start_websocket=False contradicts later expectations
The fixture explicitly disables auto-start, yet the first test asserts that the WebSocket server is already running right after startup (lines 61-70). Either:
- change the fixture to
True, or - adjust the assertions to start the server explicitly.
Mismatched assumptions will create flaky tests the moment startup behaviour changes.
🤖 Prompt for AI Agents
In tests/e2e/test_websocket_mock_e2e.py around lines 23 to 36, the server_config
fixture sets auto_start_websocket to False, but later tests expect the WebSocket
server to be running immediately after startup. To fix this, either change
auto_start_websocket to True in the fixture to match the test assumptions or
modify the tests to explicitly start the WebSocket server before asserting it is
running, ensuring consistency between setup and test expectations.
- Included the `websockets` library in the project dependencies to support WebSocket server functionality. - Updated the `uv.lock` file to reflect the new dependency. - Registered the `figma` server type in the `ServerOrchestrator` for enhanced orchestration capabilities.
- Removed unnecessary retrieval of `request_id` from incoming data in the document info and code execution response handlers, simplifying the code. - Updated unit tests to reflect variable name changes for consistency and clarity.
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
src/lightfast_mcp/servers/figma/websocket_server.py (1)
149-150:⚠️ Potential issue
getattr(..., 'is_serving', 'unknown')()can raiseTypeErrorWhen the underlying
websocketsversion lacksis_serving, the fallback string'unknown'
is invoked as a callable, crashing the server right after start-up.- logger.debug( - f"🔧 Server is_serving: {getattr(self.server, 'is_serving', 'unknown')()}" - ) + if hasattr(self.server, "is_serving"): + logger.debug(f"🔧 Server is_serving: {self.server.is_serving()}") + else: + logger.debug("🔧 Server is_serving attribute not available on this websockets version")
🧹 Nitpick comments (1)
src/lightfast_mcp/servers/figma/websocket_server.py (1)
200-201:websocket.statemay not exist on all versionsAccessing
websocket.statewithout a feature check can break on newerwebsocketsversions.
Guard the debug line withhasattrto keep logs useful without risking anAttributeError.- logger.debug(f"🔧 Client {client_id} WebSocket state: {websocket.state}") + if hasattr(websocket, "state"): + logger.debug(f"🔧 Client {client_id} WebSocket state: {websocket.state}")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/lightfast_mcp/servers/figma/websocket_server.py(1 hunks)tests/unit/test_figma_server.py(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
tests/unit/test_figma_server.py (5)
src/lightfast_mcp/core/base_server.py (1)
ServerConfig(20-41)src/lightfast_mcp/servers/figma/server.py (5)
FigmaMCPServer(13-260)_check_application(63-69)_on_startup(71-87)_on_shutdown(89-98)_perform_health_check(100-120)src/lightfast_mcp/servers/figma/websocket_server.py (7)
FigmaClient(25-46)FigmaWebSocketServer(49-538)get_server_info(511-538)start(103-159)send_command_to_plugin(458-484)stop(161-187)to_dict(35-46)tests/conftest.py (1)
sample_figma_config(57-65)src/lightfast_mcp/servers/figma/tools.py (2)
set_current_server(26-29)get_state(32-137)
🪛 GitHub Actions: CI/CD Pipeline
tests/unit/test_figma_server.py
[error] 90-90: AssertionError: Expected tool 'execute_command' not found in registered tools ['get_state', 'execute_code'].
[error] 315-315: AttributeError: module 'lightfast_mcp.servers.figma.tools' has no attribute 'execute_command'. Did you mean: 'execute_code'?
[error] 334-334: AttributeError: module 'lightfast_mcp.servers.figma.tools' has no attribute 'execute_command'. Did you mean: 'execute_code'?
[error] 358-358: AttributeError: module 'lightfast_mcp.servers.figma.tools' has no attribute 'execute_command'. Did you mean: 'execute_code'?
[error] 386-386: AttributeError: module 'lightfast_mcp.servers.figma.tools' has no attribute 'execute_command'. Did you mean: 'execute_code'?
[error] 433-433: AssertionError: Expected message handler 'execute_design_command' not found in server.message_handlers.
[error] 538-538: AssertionError: Expected tool 'execute_command' not found in registered tools ['get_state', 'execute_code'].
| """Test that default message handlers are registered.""" | ||
| server = FigmaWebSocketServer() | ||
|
|
||
| expected_handlers = [ | ||
| "ping", | ||
| "pong", | ||
| "get_document_info", | ||
| "execute_design_command", | ||
| "get_server_status", | ||
| "plugin_info", | ||
| "document_update", | ||
| "document_info_response", | ||
| "design_command_response", | ||
| ] | ||
|
|
||
| for handler in expected_handlers: | ||
| assert handler in server.message_handlers | ||
|
|
There was a problem hiding this comment.
Handler name discrepancy (execute_design_command)
FigmaWebSocketServer.message_handlers currently registers execute_code; the test expects execute_design_command, causing assertions to fail.
Update the expectation or register an alias in production code:
- "execute_design_command",
+ "execute_code",(or register the alias as shown in the websocket-server diff below).
📝 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.
| """Test that default message handlers are registered.""" | |
| server = FigmaWebSocketServer() | |
| expected_handlers = [ | |
| "ping", | |
| "pong", | |
| "get_document_info", | |
| "execute_design_command", | |
| "get_server_status", | |
| "plugin_info", | |
| "document_update", | |
| "document_info_response", | |
| "design_command_response", | |
| ] | |
| for handler in expected_handlers: | |
| assert handler in server.message_handlers | |
| """Test that default message handlers are registered.""" | |
| server = FigmaWebSocketServer() | |
| expected_handlers = [ | |
| "ping", | |
| "pong", | |
| "get_document_info", | |
| "execute_code", | |
| "get_server_status", | |
| "plugin_info", | |
| "document_update", | |
| "document_info_response", | |
| "design_command_response", | |
| ] | |
| for handler in expected_handlers: | |
| assert handler in server.message_handlers |
🧰 Tools
🪛 GitHub Actions: CI/CD Pipeline
[error] 433-433: AssertionError: Expected message handler 'execute_design_command' not found in server.message_handlers.
🤖 Prompt for AI Agents
In tests/unit/test_figma_server.py around lines 417 to 434, the test expects the
handler name "execute_design_command" but the actual registered handler in
FigmaWebSocketServer.message_handlers is "execute_code". To fix this, either
update the test's expected_handlers list to use "execute_code" instead of
"execute_design_command" or modify the production code to register
"execute_design_command" as an alias for "execute_code" so both names are
recognized.
| """Register default message handlers.""" | ||
| self.message_handlers.update( | ||
| { | ||
| "ping": self._handle_ping, | ||
| "pong": self._handle_pong, | ||
| "get_document_info": self._handle_get_document_info, | ||
| "execute_code": self._handle_execute_code, | ||
| "get_server_status": self._handle_get_server_status, | ||
| "plugin_info": self._handle_plugin_info, | ||
| "document_update": self._handle_document_update, | ||
| "document_info_response": self._handle_document_info_response, | ||
| "code_execution_response": self._handle_code_execution_response, | ||
| } | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add alias for execute_design_command to avoid breaking callers/tests
Some callers (and the plugin codebase, per tests) use the older message name execute_design_command.
Register it as a thin wrapper to _handle_execute_code to maintain backward compatibility.
@@
"execute_code": self._handle_execute_code,
+ # Back-compat: older clients/tests still send this
+ "execute_design_command": self._handle_execute_code,📝 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.
| """Register default message handlers.""" | |
| self.message_handlers.update( | |
| { | |
| "ping": self._handle_ping, | |
| "pong": self._handle_pong, | |
| "get_document_info": self._handle_get_document_info, | |
| "execute_code": self._handle_execute_code, | |
| "get_server_status": self._handle_get_server_status, | |
| "plugin_info": self._handle_plugin_info, | |
| "document_update": self._handle_document_update, | |
| "document_info_response": self._handle_document_info_response, | |
| "code_execution_response": self._handle_code_execution_response, | |
| } | |
| ) | |
| """Register default message handlers.""" | |
| self.message_handlers.update( | |
| { | |
| "ping": self._handle_ping, | |
| "pong": self._handle_pong, | |
| "get_document_info": self._handle_get_document_info, | |
| "execute_code": self._handle_execute_code, | |
| # Back-compat: older clients/tests still send this | |
| "execute_design_command": self._handle_execute_code, | |
| "get_server_status": self._handle_get_server_status, | |
| "plugin_info": self._handle_plugin_info, | |
| "document_update": self._handle_document_update, | |
| "document_info_response": self._handle_document_info_response, | |
| "code_execution_response": self._handle_code_execution_response, | |
| } | |
| ) |
🤖 Prompt for AI Agents
In src/lightfast_mcp/servers/figma/websocket_server.py around lines 88 to 101,
add an alias for the message handler "execute_design_command" that points to the
existing _handle_execute_code method. This will maintain backward compatibility
for callers and tests that still use the older message name. Update the
self.message_handlers dictionary to include "execute_design_command":
self._handle_execute_code alongside the existing handlers.
- Introduced new images for the Figma plugin installation and interface, enhancing documentation and user guidance. - These images provide visual support for users to better understand the plugin setup and usage.
…-a580 docs: add figma docs
…in Figma tools - Simplified JSON error responses in `get_state` and `execute_code` functions by consolidating error handling logic. - Introduced a new `get_figma_server_status` function to provide detailed status information about the Figma WebSocket server, including running status and connected clients. - Enhanced logging for error scenarios to improve debugging and monitoring capabilities.
…ve error handling - Added stub functions for removed design command tools, providing clear error messages and alternatives for users. - Updated the `get_figma_server_status` function to return a dictionary when called directly, enhancing usability. - Modified WebSocket server to handle deprecated design command responses, ensuring backward compatibility with informative warnings. - Adjusted unit tests to reflect changes in available tools, replacing `execute_command` with `execute_code` for consistency.
- Deleted end-to-end and integration test files for the Figma MCP server to streamline the test suite. - Updated unit tests to reflect changes in function names and error handling for code execution. - Ensured consistency in testing methodologies across the codebase by replacing `execute_command` with `execute_code` in relevant tests.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
src/lightfast_mcp/servers/figma/server.py (2)
44-49:⚠️ Potential issueTools are still not registered –
_register_tools()must be invoked
TheFigmaMCPServernever calls its own_register_tools()method, so MCP clients will not seeget_stateorexecute_code.
This was raised in a previous review and remains unresolved.@@ class FigmaMCPServer(BaseServer): # Start WebSocket server immediately in a background task self._start_websocket_server_background() # Register signal handler for proper cleanup on process termination self._register_signal_handlers() + + # Expose MCP tools + self._register_tools() # <-- add this
161-174: 🛠️ Refactor suggestionEvent-loop cleanup after
loop.stop()can dead-lock / raise
Cancelling tasks after the loop has already been stopped (and possibly closed) is unsafe and can raiseRuntimeError: Event loop stopped before Future completed, exactly the issue flagged in the earlier review.
Move the cancellation logic before theloop.stop()call, or schedule it inside the running loop, then simplyreturnin thefinallyblock.src/lightfast_mcp/servers/figma/websocket_server.py (1)
151-152:⚠️ Potential issue
getattr(..., 'is_serving', 'unknown')()still triggersTypeError
Calling the fallback string'unknown'as a function recreates the crash noted last time.-logger.debug( - f"🔧 Server is_serving: {getattr(self.server, 'is_serving', 'unknown')()}" -) +if hasattr(self.server, "is_serving"): + logger.debug(f"🔧 Server is_serving: {self.server.is_serving()}") +else: + logger.debug("🔧 Server is_serving attribute not available on this websockets version")
🧹 Nitpick comments (2)
src/lightfast_mcp/servers/figma/server.py (1)
161-165:loopmay be undefined infinally
If an exception occurs beforeloopis assigned, the reference insidefinallywill raiseUnboundLocalError.
Guard withloop = locals().get("loop")(or similar) before use.src/lightfast_mcp/servers/figma/tools.py (1)
164-170: Allow caller to target a specific plugin
Currently the first connected client is arbitrarily chosen. Expose an optionalplugin_idparameter to avoid non-deterministic behaviour in multi-plugin sessions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/lightfast_mcp/servers/figma/server.py(1 hunks)src/lightfast_mcp/servers/figma/tools.py(1 hunks)src/lightfast_mcp/servers/figma/websocket_server.py(1 hunks)tests/unit/test_figma_server.py(1 hunks)tests/unit/test_modular_servers.py(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/unit/test_modular_servers.py
- tests/unit/test_figma_server.py
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Python 3.13
- GitHub Check: Python 3.10
- GitHub Check: Python 3.12
- GitHub Check: Python 3.11
- GitHub Check: Integration Tests
🔇 Additional comments (2)
src/lightfast_mcp/servers/figma/server.py (1)
225-263: Signal registration might run outside the main thread
Python raisesValueErrorifsignal.signal()is called from a non-main thread.
Please ensure the server instance is always created in the main thread or gate this registration:import threading, signal if threading.current_thread() is threading.main_thread(): signal.signal(signal.SIGTERM, signal_handler) signal.signal(signal.SIGINT, signal_handler)src/lightfast_mcp/servers/figma/websocket_server.py (1)
510-544:❌ Incorrect review comment
Potential payload mismatch –
codeends up nested underparams
send_command_to_plugin()wraps the suppliedparamsdict, so the Figma plugin will receive:{ "type": "execute_code", "params": { "code": "..." } }Yet
_handle_execute_code(and typical plugin samples) expect"code"at the top level.
Confirm the plugin side deserialisesparams.code; otherwise, send the code directly:success = await ws_server.send_command_to_plugin( - target_client.id, "execute_code", {"code": code_to_execute} + target_client.id, "execute_code", None # code at top level )
🏁 Script executed:
#!/bin/bash # Locate the definition of send_command_to_plugin and inspect its implementation rg -n "async def send_command_to_plugin" -A20 src/lightfast_mcp/servers/figma/websocket_server.pyLength of output: 887
🏁 Script executed:
#!/bin/bash # Search for plugin-side handler definitions and usage of execute_code rg -n "def _handle_execute_code" -A5 rg -n "execute_code" -A5Length of output: 20116
🏁 Script executed:
#!/bin/bash # Show the dispatch loop where handlers are applied in websocket_server.py around where handlers mapping is defined rg -n "handlers = {" -A30 -B5 src/lightfast_mcp/servers/figma/websocket_server.pyLength of output: 171
No changes needed – nested
params.codeis already supported
The server uniformly wraps payloads under"params", and the Figma UI code explicitly handlesmessage.params?.code(falling back frommessage.code) when forwarding to the plugin. The existing flow already works end-to-end:
- Server’s
send_command_to_plugin/broadcast_to_pluginsalways sends
{ type, params: { code }, … }addons/figma/ui.htmlusesmessage.code || message.params?.code- Plugin script (
addons/figma/code.ts) then receivescodeat top levelChanging to a bare top-level
codefield would break this consistency and isn’t necessary.Likely an incorrect or invalid review comment.
Summary by CodeRabbit
New Features
Documentation
Bug Fixes
Tests
Chores