From 4f241f7d32d501df0d164080395e8d46bdf398ab Mon Sep 17 00:00:00 2001 From: Jensen Zhang Date: Mon, 16 Mar 2026 02:33:46 +0800 Subject: [PATCH] Fix cm-cli auto-detection and add development guidelines - Update CMCLIExecutor to detect cm-cli as executable (not just .py script) - Use shutil.which() to find cm-cli in PATH (supports uv/pip installations) - Fall back to .py script detection for legacy installations - Add comprehensive .gitignore for Python, IDE, and ComfyUI artifacts - Expand AGENTS.md with build commands, code style, and architecture patterns - Remove trailing whitespace across all source files --- .gitignore | 89 ++++++++++++ AGENTS.md | 298 +++++++++++++++++++++++++++++++++++++- IMPLEMENTATION_PLAN.md | 8 +- __init__.py | 8 +- api/handlers/jobs.py | 14 +- api/handlers/nodes.py | 50 +++---- api/handlers/snapshots.py | 18 +-- api/routes.py | 6 +- cm_cli/executor.py | 61 +++++--- 9 files changed, 479 insertions(+), 73 deletions(-) create mode 100644 .gitignore diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..2227181 --- /dev/null +++ b/.gitignore @@ -0,0 +1,89 @@ +# Byte-compiled / optimized / DLL files +__pycache__/ +*.py[cod] +*$py.class + +# C extensions +*.so + +# Distribution / packaging +.Python +build/ +develop-eggs/ +dist/ +downloads/ +eggs/ +.eggs/ +lib/ +lib64/ +parts/ +sdist/ +var/ +wheels/ +*.egg-info/ +.installed.cfg +*.egg + +# PyInstaller +*.manifest +*.spec + +# Installer logs +pip-log.txt +pip-delete-this-directory.txt + +# Unit test / coverage reports +htmlcov/ +.tox/ +.nox/ +.coverage +.coverage.* +.cache +nosetests.xml +coverage.xml +*.cover +*.py,cover +.hypothesis/ +.pytest_cache/ + +# Translations +*.mo +*.pot + +# Environments +.env +.venv +env/ +venv/ +ENV/ +env.bak/ +venv.bak/ + +# IDE +.idea/ +.vscode/ +*.swp +*.swo +*~ + +# macOS +.DS_Store + +# Logs +*.log + +# Config (keep examples, ignore actual configs) +config/config.json +!config/config.json.example + +# ComfyUI specific +custom_nodes/ +models/ +input/ +output/ +temp/ +web/ + +# Test artifacts +tests/fixtures/ +*.snapshot diff --git a/AGENTS.md b/AGENTS.md index 5c3b33e..e4c4f4f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,4 +1,296 @@ -# A ComfyUI CustomNode to Expose REST API for `cm-cli` +# AGENTS.md - Development Guide for cm-cli-rest -ComfyUI Version: 0.15+ -ComfyUI Manager Version: 4.0+ +## Project Overview + +**cm-cli-rest** is a ComfyUI custom node that exposes REST API endpoints for managing ComfyUI custom nodes via `cm-cli` (ComfyUI-Manager CLI). + +- **ComfyUI Version**: 0.15+ +- **ComfyUI-Manager Version**: 4.0+ +- **Python Version**: 3.9+ (tested on 3.11) +- **Runtime**: Integrated with ComfyUI's aiohttp server (no separate port) + +--- + +## Build & Development Commands + +### Syntax Validation +```bash +# Check Python syntax for a single file +python -m py_compile cm_cli/executor.py + +# Check all Python files +find . -name "*.py" -not -path "./__pycache__/*" -exec python -m py_compile {} \; +``` + +### Type Checking (if type annotations added) +```bash +# Install mypy (optional - not in requirements.txt) +pip install mypy + +# Run type checking +mypy cm_cli/ api/ +``` + +### Linting (if linter added) +```bash +# Install ruff (recommended) +pip install ruff + +# Lint all files +ruff check . + +# Auto-fix issues +ruff check --fix . + +# Format code +ruff format . +``` + +### Testing +**No test suite exists yet** (as of 2026-03-16). Tests are marked as TODO in IMPLEMENTATION_PLAN.md. + +To add tests: +1. Create `tests/` directory +2. Use `pytest` as the test framework +3. Run with: `pytest tests/` or `pytest tests/test_specific_file.py -v` + +--- + +## Code Style Guidelines + +### Imports +- **Order**: Standard library → Third-party → Local imports +- **Grouping**: Separate import groups with blank lines +- **Type imports**: Use `from typing import ...` for type hints +- **Relative imports**: Use for local modules (`.`, `..`) + +```python +# ✅ Correct +import asyncio +import logging +from typing import Dict, List, Optional + +from aiohttp import web + +from ..cm_cli import CMCLIExecutor +``` + +### Formatting +- **Indentation**: 4 spaces (no tabs) +- **Line length**: 100 characters (soft limit) +- **Quotes**: Double quotes for strings +- **Trailing commas**: Use in multi-line structures + +### Type Hints +- **Required** for function parameters and return types +- **Use `Optional[T]`** for nullable values (Python 3.9+) +- **Use `Dict`, `List`, `Any`** from typing module + +```python +# ✅ Correct +async def execute( + self, + args: List[str], + timeout: Optional[int] = None, +) -> Dict[str, Any]: + ... + +# ❌ Avoid (no type hints) +async def execute(self, args, timeout=None): + ... +``` + +### Naming Conventions +- **Classes**: PascalCase (`CMCLIExecutor`, `NodesHandler`) +- **Functions/Methods**: snake_case (`list_nodes`, `execute_async`) +- **Constants**: UPPER_CASE (`JOB_NOT_FOUND`, `MAX_CONCURRENT_JOBS`) +- **Private methods**: Leading underscore (`_auto_detect_cm_cli`) +- **Exceptions**: End with `Error` (`CMCLIError`) + +### Error Handling +- **Custom exceptions**: Create specific exception classes for domain errors +- **Error responses**: Always return structured JSON with `success`, `error.code`, `error.message` +- **Logging**: Log errors with `logger.error()` before returning +- **Never suppress exceptions** without logging + +```python +# ✅ Correct pattern +try: + result = await self.executor.execute(["show", "installed"]) + if not result["success"]: + return web.json_response( + {"success": False, "error": {"code": "CM_CLI_ERROR", "message": "Failed"}}, + status=500, + ) +except CMCLIError as e: + logger.error(f"Error listing nodes: {e}") + return web.json_response( + {"success": False, "error": {"code": "CM_CLI_ERROR", "message": e.message}}, + status=500, + ) +``` + +### Async/Await +- **All I/O operations**: Use async (HTTP, subprocess, file I/O) +- **Timeout handling**: Always wrap with `asyncio.wait_for()` +- **Subprocess execution**: Use `asyncio.create_subprocess_exec()` + +### Docstrings +- **Required** for all public classes, methods, and functions +- **Format**: Google style (Args, Returns, Raises sections) +- **Include examples** for complex methods + +```python +# ✅ Correct +async def execute( + self, + args: List[str], + timeout: Optional[int] = None, +) -> Dict[str, Any]: + """ + Execute a cm-cli command synchronously. + + Args: + args: Command arguments (e.g., ["show", "installed"]). + timeout: Timeout in seconds. + capture_output: Whether to capture stdout/stderr. + + Returns: + Dictionary with 'success', 'stdout', 'stderr', 'exit_code'. + + Raises: + CMCLIError: If command execution fails. + """ +``` + +### File Organization +- **One class per file** (or tightly related classes) +- **Handlers**: One handler class per endpoint group (`nodes.py`, `snapshots.py`) +- **Exports**: Define `__all__` in `__init__.py` files + +--- + +## Architecture Patterns + +### Handler Pattern +```python +class NodesHandler: + def __init__(self, executor: CMCLIExecutor): + self.executor = executor + + async def list_nodes(self, request: web.Request) -> web.Response: + # Handler logic +``` + +### Executor Pattern +```python +class CMCLIExecutor: + def __init__(self, cm_cli_path: Optional[str] = None): + # Auto-detect cm-cli if not provided + self.cm_cli_path = self._auto_detect_cm_cli() + + async def execute(self, args: List[str]) -> Dict[str, Any]: + # Execute command synchronously +``` + +### Response Format +All responses follow this structure: + +**Success (2xx):** +```json +{ + "success": true, + "data": { ... } +} +``` + +**Error (4xx/5xx):** +```json +{ + "success": false, + "error": { + "code": "ERROR_CODE", + "message": "Human-readable message", + "details": "Optional additional context" + } +} +``` + +--- + +## Key Files + +| File | Purpose | +|------|---------| +| `__init__.py` | Entry point, initializes executor, registers routes | +| `api/routes.py` | Route definitions | +| `api/handlers/nodes.py` | Node management endpoints | +| `api/handlers/snapshots.py` | Snapshot endpoints | +| `api/handlers/jobs.py` | Job tracking endpoints | +| `cm_cli/executor.py` | cm-cli subprocess execution | +| `config/config.json` | Server configuration | + +--- + +## Configuration + +Edit `config/config.json`: + +```json +{ + "enabled": true, + "api_key": null, + "cm_cli_path": null, + "python_path": "python", + "timeout": 300, + "max_concurrent_jobs": 3, + "logging_level": "INFO" +} +``` + +--- + +## Common Tasks + +### Adding a New Endpoint +1. Add handler method in `api/handlers/nodes.py` (or appropriate handler file) +2. Register route in `api/routes.py` +3. Update README.md documentation +4. Add error codes to error response section + +### Modifying cm-cli Execution +1. Edit `cm_cli/executor.py` +2. Ensure both `execute()` and `execute_async()` are updated +3. Run syntax check: `python -m py_compile cm_cli/executor.py` + +### Testing Changes +1. Restart ComfyUI +2. Test endpoint with curl: + ```bash + curl http://localhost:8188/cm-cli-rest/health + ``` + +--- + +## Troubleshooting + +### cm-cli Not Found +- **Error**: `CM_CLI_NOT_FOUND` +- **Cause**: cm-cli executable not in PATH +- **Fix**: Set `cm_cli_path` in config.json to full path + +### Import Errors +- **Cause**: Circular imports or missing `__init__.py` +- **Fix**: Check import order, ensure all packages have `__init__.py` + +### Type Errors +- **Tool**: Use `lsp_diagnostics` in editor +- **Fix**: Never suppress with `as any` or `# type: ignore` + +--- + +## References + +- [ComfyUI Custom Node Docs](https://docs.comfy.org/custom-nodes/overview) +- [ComfyUI-Manager](https://github.com/ltdrdata/ComfyUI-Manager) +- [cm-cli Documentation](https://github.com/Comfy-Org/ComfyUI-Manager/blob/main/docs/en/cm-cli.md) diff --git a/IMPLEMENTATION_PLAN.md b/IMPLEMENTATION_PLAN.md index 45ed092..95b15b0 100644 --- a/IMPLEMENTATION_PLAN.md +++ b/IMPLEMENTATION_PLAN.md @@ -1,8 +1,8 @@ # cm-cli-rest Implementation Plan -**Project**: ComfyUI Custom Node - REST API for cm-cli -**Version**: 0.1.0 -**Last Updated**: 2026-03-15 +**Project**: ComfyUI Custom Node - REST API for cm-cli +**Version**: 0.1.0 +**Last Updated**: 2026-03-15 --- @@ -299,7 +299,7 @@ from cm_cli import CMCLIExecutor class NodesHandler: def __init__(self, executor: CMCLIExecutor): self.executor = executor - + async def list_nodes(self, request: web.Request) -> web.Response: result = await self.executor.execute(["show", "installed"]) nodes = await self.executor.parse_installed_nodes(result["stdout"]) diff --git a/__init__.py b/__init__.py index 6212393..0b7ff62 100644 --- a/__init__.py +++ b/__init__.py @@ -34,10 +34,10 @@ logger.info("cm-cli-rest initialized - REST API available at /cm-cli-rest/*") class CMCLIRestNode: """ ComfyUI node for triggering cm-cli REST API calls. - + This is optional - the REST API works independently. """ - + @classmethod def INPUT_TYPES(cls): return { @@ -60,13 +60,13 @@ class CMCLIRestNode: def execute(self, endpoint: str, trigger: bool, node_name: str = ""): """ Execute API call (synchronous, for simple endpoints only). - + Note: For complex operations (install, update), use the REST API directly. This node is mainly for health checks and simple queries. """ if not trigger: return ("", False) - + # For actual implementation, you would make HTTP requests to the API # This is a placeholder - in practice, use the REST API directly return (f"Use REST API at /cm-cli-rest/{endpoint}", True) diff --git a/api/handlers/jobs.py b/api/handlers/jobs.py index 23c55b3..42c9370 100644 --- a/api/handlers/jobs.py +++ b/api/handlers/jobs.py @@ -17,11 +17,11 @@ class JobsHandler: async def get_job(self, request: web.Request) -> web.Response: """ GET /cm-cli-rest/jobs/:id - + Get status of an async job. """ job_id = request.match_info.get("id") - + if not job_id: return web.json_response( { @@ -35,7 +35,7 @@ class JobsHandler: ) job = self.executor.get_job(job_id) - + if not job: return web.json_response( { @@ -58,11 +58,11 @@ class JobsHandler: async def list_jobs(self, request: web.Request) -> web.Response: """ GET /cm-cli-rest/jobs - + List all jobs (active and completed). """ jobs = self.executor.list_jobs() - + return web.json_response( { "success": True, @@ -76,11 +76,11 @@ class JobsHandler: async def cleanup_job(self, request: web.Request) -> web.Response: """ DELETE /cm-cli-rest/jobs/:id - + Remove a job from tracking (doesn't stop execution). """ job_id = request.match_info.get("id") - + if not job_id: return web.json_response( { diff --git a/api/handlers/nodes.py b/api/handlers/nodes.py index 17fdaf4..fbbeda2 100644 --- a/api/handlers/nodes.py +++ b/api/handlers/nodes.py @@ -18,12 +18,12 @@ class NodesHandler: async def list_nodes(self, request: web.Request) -> web.Response: """ GET /cm-cli-rest/nodes - + List all installed custom nodes. """ try: result = await self.executor.execute(["show", "installed"]) - + if not result["success"]: return web.json_response( { @@ -38,7 +38,7 @@ class NodesHandler: ) nodes = await self.executor.parse_installed_nodes(result["stdout"]) - + return web.json_response( { "success": True, @@ -66,11 +66,11 @@ class NodesHandler: async def get_node(self, request: web.Request) -> web.Response: """ GET /cm-cli-rest/nodes/:name - + Get details for a specific node. """ node_name = request.match_info.get("name") - + if not node_name: return web.json_response( { @@ -85,7 +85,7 @@ class NodesHandler: try: result = await self.executor.execute(["show", "installed"]) - + if not result["success"]: return web.json_response( { @@ -136,9 +136,9 @@ class NodesHandler: async def install_node(self, request: web.Request) -> web.Response: """ POST /cm-cli-rest/nodes/install - + Install a custom node (async operation). - + Request body: { "name": "ComfyUI-Impact-Pack", @@ -175,17 +175,17 @@ class NodesHandler: # Build command command = ["install", node_name] - + if body.get("channel"): command.extend(["--channel", body["channel"]]) - + if body.get("mode"): command.extend(["--mode", body["mode"]]) try: # Execute asynchronously job = await self.executor.execute_async(command, timeout=300) - + return web.json_response( { "success": True, @@ -214,7 +214,7 @@ class NodesHandler: async def uninstall_node(self, request: web.Request) -> web.Response: """ POST /cm-cli-rest/nodes/uninstall - + Uninstall a custom node. """ try: @@ -246,7 +246,7 @@ class NodesHandler: try: result = await self.executor.execute(["uninstall", node_name]) - + if result["success"]: return web.json_response( { @@ -285,7 +285,7 @@ class NodesHandler: async def update_node(self, request: web.Request) -> web.Response: """ POST /cm-cli-rest/nodes/update - + Update a custom node (async operation). """ try: @@ -317,7 +317,7 @@ class NodesHandler: try: job = await self.executor.execute_async(["update", node_name], timeout=300) - + return web.json_response( { "success": True, @@ -346,12 +346,12 @@ class NodesHandler: async def update_all_nodes(self, request: web.Request) -> web.Response: """ POST /cm-cli-rest/nodes/update-all - + Update all custom nodes (async operation). """ try: job = await self.executor.execute_async(["update", "all"], timeout=600) - + return web.json_response( { "success": True, @@ -380,7 +380,7 @@ class NodesHandler: async def enable_node(self, request: web.Request) -> web.Response: """ POST /cm-cli-rest/nodes/enable - + Enable a disabled custom node. """ try: @@ -412,7 +412,7 @@ class NodesHandler: try: result = await self.executor.execute(["enable", node_name]) - + if result["success"]: return web.json_response( { @@ -451,7 +451,7 @@ class NodesHandler: async def disable_node(self, request: web.Request) -> web.Response: """ POST /cm-cli-rest/nodes/disable - + Disable a custom node. """ try: @@ -483,7 +483,7 @@ class NodesHandler: try: result = await self.executor.execute(["disable", node_name]) - + if result["success"]: return web.json_response( { @@ -522,7 +522,7 @@ class NodesHandler: async def fix_node(self, request: web.Request) -> web.Response: """ POST /cm-cli-rest/nodes/fix - + Fix a custom node's dependencies. """ try: @@ -554,7 +554,7 @@ class NodesHandler: try: job = await self.executor.execute_async(["fix", node_name], timeout=300) - + return web.json_response( { "success": True, @@ -583,7 +583,7 @@ class NodesHandler: async def reinstall_node(self, request: web.Request) -> web.Response: """ POST /cm-cli-rest/nodes/reinstall - + Reinstall a custom node (async operation). """ try: @@ -615,7 +615,7 @@ class NodesHandler: try: job = await self.executor.execute_async(["reinstall", node_name], timeout=300) - + return web.json_response( { "success": True, diff --git a/api/handlers/snapshots.py b/api/handlers/snapshots.py index 70ae2e4..6544bc4 100644 --- a/api/handlers/snapshots.py +++ b/api/handlers/snapshots.py @@ -17,9 +17,9 @@ class SnapshotsHandler: async def save_snapshot(self, request: web.Request) -> web.Response: """ POST /cm-cli-rest/snapshots/save - + Save current custom node configuration as a snapshot. - + Request body (optional): { "name": "my-snapshot" # optional, defaults to timestamp @@ -31,14 +31,14 @@ class SnapshotsHandler: body = {} snapshot_name = body.get("name") - + try: command = ["save-snapshot"] if snapshot_name: command.append(snapshot_name) result = await self.executor.execute(command) - + if result["success"]: return web.json_response( { @@ -78,9 +78,9 @@ class SnapshotsHandler: async def restore_snapshot(self, request: web.Request) -> web.Response: """ POST /cm-cli-rest/snapshots/restore - + Restore custom node configuration from a snapshot. - + Request body: { "name": "my-snapshot" # required @@ -118,7 +118,7 @@ class SnapshotsHandler: ["restore-snapshot", snapshot_name], timeout=600 ) - + return web.json_response( { "success": True, @@ -147,9 +147,9 @@ class SnapshotsHandler: async def list_snapshots(self, request: web.Request) -> web.Response: """ GET /cm-cli-rest/snapshots - + List available snapshots. - + Note: cm-cli doesn't have a direct 'list snapshots' command. This reads from the snapshots directory if available. """ diff --git a/api/routes.py b/api/routes.py index b5eb1a9..1b0c012 100644 --- a/api/routes.py +++ b/api/routes.py @@ -14,7 +14,7 @@ logger = logging.getLogger(__name__) def setup_routes(app: web.Application, executor: CMCLIExecutor) -> None: """ Register all REST API routes with the aiohttp application. - + Args: app: aiohttp web application (PromptServer.instance.app) executor: CMCLIExecutor instance for command execution @@ -55,11 +55,11 @@ def setup_routes(app: web.Application, executor: CMCLIExecutor) -> None: async def health_check(request: web.Request) -> web.Response: """ GET /cm-cli-rest/health - + Health check endpoint. """ from datetime import datetime - + return web.json_response( { "status": "healthy", diff --git a/cm_cli/executor.py b/cm_cli/executor.py index 0a8134a..38c6766 100644 --- a/cm_cli/executor.py +++ b/cm_cli/executor.py @@ -72,11 +72,11 @@ class CMCLIExecutor: Executor for cm-cli commands with async support and job tracking. Usage: - executor = CMCLIExecutor(cm_cli_path="/path/to/cm-cli.py") - + executor = CMCLIExecutor(cm_cli_path="/path/to/cm-cli") + # Synchronous execution result = await executor.execute(["show", "installed"]) - + # Asynchronous job (for long-running operations) job = await executor.execute_async(["install", "ComfyUI-Impact-Pack"]) """ @@ -91,8 +91,8 @@ class CMCLIExecutor: Initialize the cm-cli executor. Args: - cm_cli_path: Path to cm-cli.py script. If None, auto-detects from ComfyUI-Manager. - python_path: Path to Python interpreter. + cm_cli_path: Path to cm-cli executable. If None, auto-detects from PATH or ComfyUI-Manager. + python_path: Path to Python interpreter (used only if cm-cli is a .py script). working_dir: Working directory for subprocess execution (usually ComfyUI root). """ self.python_path = python_path @@ -107,26 +107,41 @@ class CMCLIExecutor: def _auto_detect_cm_cli(self) -> Path: """ - Auto-detect cm-cli.py location from ComfyUI-Manager installation. + Auto-detect cm-cli executable location. - Searches in common locations: - 1. custom_nodes/ComfyUI-Manager/cm-cli.py - 2. custom_nodes/ComfyUI-Manager/cm_cli.py - 3. Current directory cm-cli.py + Searches in order: + 1. System PATH (cm-cli installed as console script by uv/pip) + 2. ComfyUI-Manager custom_nodes directory (cm-cli.py script) + 3. Current working directory + + Returns: + Path to cm-cli executable or script. """ + import shutil + + # First, try to find cm-cli in PATH (most common for uv/pip installs) + cm_cli_in_path = shutil.which("cm-cli") + if cm_cli_in_path: + return Path(cm_cli_in_path) + + # Fallback: check if it's cm_cli (underscore variant) + cm_cli_underscore = shutil.which("cm_cli") + if cm_cli_underscore: + return Path(cm_cli_underscore) + + # Last resort: check common ComfyUI-Manager locations for the script possible_paths = [ self.working_dir / "custom_nodes" / "ComfyUI-Manager" / "cm-cli.py", self.working_dir / "custom_nodes" / "ComfyUI-Manager" / "cm_cli.py", self.working_dir / "cm-cli.py", - Path(__file__).parent.parent.parent / "ComfyUI-Manager" / "cm-cli.py", ] for path in possible_paths: if path.exists(): return path - # If not found, return default path (will fail if cm-cli not installed) - return Path("cm-cli.py") + # If not found, return 'cm-cli' and let the subprocess fail with a clear error + return Path("cm-cli") async def execute( self, @@ -148,11 +163,15 @@ class CMCLIExecutor: Raises: CMCLIError: If command execution fails. """ - command = [str(self.cm_cli_path)] + args + # Build command: use cm-cli directly as executable + # If it's a .py file, prepend python interpreter + if self.cm_cli_path.suffix == ".py": + command = [self.python_path, str(self.cm_cli_path)] + args + else: + command = [str(self.cm_cli_path)] + args try: process = await asyncio.create_subprocess_exec( - self.python_path, *command, stdout=asyncio.subprocess.PIPE if capture_output else None, stderr=asyncio.subprocess.PIPE if capture_output else None, @@ -189,7 +208,8 @@ class CMCLIExecutor: except FileNotFoundError as e: raise CMCLIError( f"cm-cli not found at {self.cm_cli_path}. " - "Ensure ComfyUI-Manager is installed in custom_nodes/.", + "Ensure ComfyUI-Manager is installed and cm-cli is in PATH, " + "or set cm_cli_path in configuration.", command=command, ) from e except Exception as e: @@ -216,7 +236,13 @@ class CMCLIExecutor: CMCLIJob object for tracking execution. """ job_id = str(uuid.uuid4())[:8] - command = [str(self.cm_cli_path)] + args + + # Build command: use cm-cli directly as executable + # If it's a .py file, prepend python interpreter + if self.cm_cli_path.suffix == ".py": + command = [self.python_path, str(self.cm_cli_path)] + args + else: + command = [str(self.cm_cli_path)] + args job = CMCLIJob( job_id=job_id, @@ -229,7 +255,6 @@ class CMCLIExecutor: try: process = await asyncio.create_subprocess_exec( - self.python_path, *command, stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE,