Feature/commands - #12
Conversation
| { | ||
| "code": "python code as string", | ||
| "description": "optional description of what the code does", | ||
| "use_transaction": true # set false for UI ops like switching the active view |
There was a problem hiding this comment.
I removed this because it didn't seem to be used
There was a problem hiding this comment.
Pull request overview
Adds an API + MCP tooling layer to list and run pyRevit commands headlessly, with optional log capture for agent feedback. Also adjusts code-execution cleanup behavior to ensure stdout is restored/closed reliably.
Changes:
- Introduces
/commands_listand/commands_runroutes to expose available pyRevit commands and trigger execution. - Adds MCP tools to list commands and run a command by
control_id. - Updates
code_execution.pyto move stdout restoration / buffer closing into afinallyblock (and adds a security warning to the docstring).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/commands_tools.py | New MCP tools for listing/running pyRevit commands (currently has endpoint + syntax issues). |
| tools/init.py | Wires the new commands tools into tool registration. |
| startup.py | Registers the new commands routes during extension startup. |
| revit_mcp/commands.py | New routes for listing commands and executing them (currently has missing imports, request parsing, and response consistency issues). |
| revit_mcp/code_execution.py | Moves stdout/buffer cleanup into finally (currently introduces an UnboundLocalError risk). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| payload = {"control_id": control_id, "config": config, "wait" = wait} | ||
| response = await revit_post("/commands_run", payload, ctx) |
There was a problem hiding this comment.
The payload dict uses "wait" = wait, which is invalid Python syntax (assignment inside a dict literal). Use a : for the key/value pair, and ensure the payload keys match what /commands_run actually supports.
| from pyrevit import HOST_APP | ||
| from pyrevit.coreutils.logger import get_logger | ||
| from pyrevit.loader import sessioninfo | ||
|
|
||
| import glob | ||
| import os | ||
|
|
||
| from pyrevit import routes | ||
| from pyrevit.routes.server import serverinfo | ||
|
|
||
| from pyrevit import routes |
There was a problem hiding this comment.
This module imports several names that are never used (HOST_APP, get_logger, sessioninfo, routes, serverinfo), and also duplicates from pyrevit import routes. Please remove unused/duplicate imports to reduce confusion and avoid implying dependencies that aren't required.
| from pyrevit import HOST_APP | |
| from pyrevit.coreutils.logger import get_logger | |
| from pyrevit.loader import sessioninfo | |
| import glob | |
| import os | |
| from pyrevit import routes | |
| from pyrevit.routes.server import serverinfo | |
| from pyrevit import routes | |
| import glob | |
| import os |
| @api.route('/commands_list', methods=['GET']) | ||
| def get_commands(uiapp): | ||
| """List all loaded pyRevit commands with their control IDs.""" | ||
| from pyrevit.loader import sessionmgr | ||
| commands = sessionmgr.find_all_commands(cache=True) | ||
| return [ | ||
| { | ||
| "name": cmd.name, | ||
| "control_id": cmd.control_id, | ||
| "bundle": cmd.bundle, | ||
| "extension": cmd.extension, | ||
| "unique_id": cmd.unique_id, | ||
| } | ||
| for cmd in commands | ||
| ] |
There was a problem hiding this comment.
get_commands returns a raw Python list instead of using routes.make_response(...). Other route modules consistently wrap responses with routes.make_response for a stable JSON shape and status handling (e.g. revit_mcp/status.py, revit_mcp/placement.py). Please wrap this list in routes.make_response(data={...}) (or at least data=commands).
| data = request.data or {} | ||
| control_id = data.get('control_id', None) | ||
| wait = data.get('wait', True) | ||
|
|
There was a problem hiding this comment.
data = request.data or {} assumes request.data is already a dict. In other POST routes, request.data is often a JSON string and is parsed with json.loads(...). As-is, calling data.get(...) will fail when request.data is a string; parse the JSON like the other endpoints before reading control_id/wait.
| except Exception as e: | ||
| except_info = except_info | ||
| logger.exception(e) |
There was a problem hiding this comment.
In the exception handler, except_info = except_info is a no-op and drops the real exception details. Assign except_info from the caught exception (and ideally include traceback) so the response can report what failed.
| if not wait: | ||
| command_id = UI.RevitCommandId.LookupCommandId(control_id) | ||
| if command_id is None: | ||
| return {"error": "Command not found: {}".format(control_id)} | ||
| uiapp.PostCommand(command_id) | ||
| return {"result": "posted", "control_id": control_id} |
There was a problem hiding this comment.
UI.RevitCommandId.LookupCommandId(...) is used but UI is never imported in this module, which will raise NameError on the wait=false path. Import the required Revit UI namespace (or access it via an existing project pattern) before using UI.RevitCommandId.
| now = datetime.now() | ||
| envvars.set_pyrevit_env_var('PYREVIT_HEADLESS', '1') | ||
| result = None | ||
| except_info = None | ||
| try: | ||
| mlogger.debug('[HEADLESS:START] command=%s controlId=%s', cmd.unique_id, control_id) | ||
| result = sessionmgr.execute_command_cls(cmd.extcmd_type) | ||
|
|
||
| except Exception as e: | ||
| except_info = except_info | ||
| logger.exception(e) | ||
| finally: | ||
| mlogger.debug('[HEADLESS:END] command=%s controlId=%s result=%s', cmd.unique_id, control_id, result) | ||
| envvars.set_pyrevit_env_var('PYREVIT_HEADLESS', '') |
There was a problem hiding this comment.
envvars and mlogger are referenced but never defined/imported, so the wait=true execution path will raise NameError. Either import/initialize these (e.g. set mlogger = get_logger(__name__) and import envvars from pyRevit) or use the already-defined logger.
| wait = true => runs execution immediatly and returns script logs (where possible) | ||
| Wait = false => posts command to revit ui thread, to be executed after this api call. |
There was a problem hiding this comment.
Spelling in the docstring: immediatly should be immediately. Also Wait = false is inconsistently capitalized compared to wait elsewhere in the same docstring.
| wait = true => runs execution immediatly and returns script logs (where possible) | |
| Wait = false => posts command to revit ui thread, to be executed after this api call. | |
| wait = true => runs execution immediately and returns script logs (where possible) | |
| wait = false => posts command to revit ui thread, to be executed after this api call. |
| return {"error": "control_id is required in request body"} | ||
|
|
||
| # fire-and-forget via PostCommand | ||
| if not wait: | ||
| command_id = UI.RevitCommandId.LookupCommandId(control_id) | ||
| if command_id is None: | ||
| return {"error": "Command not found: {}".format(control_id)} | ||
| uiapp.PostCommand(command_id) | ||
| return {"result": "posted", "control_id": control_id} | ||
|
|
||
|
|
||
| cmd = next((c for c in sessionmgr.find_all_commands() | ||
| if c.control_id == control_id), None) | ||
| if cmd is None: | ||
| return {"error": "Command not found: {}".format(control_id)} | ||
|
|
||
|
|
||
| # PyRevit reload destroys the IronPython engine mid-execution, so the HTTP | ||
| # response can never be sent. Waiting on it will always time out so we don't allow this | ||
| # command to be run with 'wait' | ||
| if cmd.unique_id == 'pyrevitcore_pyrevit_pyrevit_tools_reload': | ||
| return { | ||
| "error": "Cannot await PyRevit reload: the reload script destroys " | ||
| "the runtime engine before a response can be sent. " | ||
| "Use wait=false to fire-and-forget instead." | ||
| } |
There was a problem hiding this comment.
run_command returns bare dicts for both success and error cases, while other endpoints use routes.make_response(...) to control HTTP status codes and keep a consistent response envelope. Consider returning routes.make_response(data=..., status=...) here as well (e.g. 400 for missing control_id, 404 for unknown commands).
| return {"error": "control_id is required in request body"} | |
| # fire-and-forget via PostCommand | |
| if not wait: | |
| command_id = UI.RevitCommandId.LookupCommandId(control_id) | |
| if command_id is None: | |
| return {"error": "Command not found: {}".format(control_id)} | |
| uiapp.PostCommand(command_id) | |
| return {"result": "posted", "control_id": control_id} | |
| cmd = next((c for c in sessionmgr.find_all_commands() | |
| if c.control_id == control_id), None) | |
| if cmd is None: | |
| return {"error": "Command not found: {}".format(control_id)} | |
| # PyRevit reload destroys the IronPython engine mid-execution, so the HTTP | |
| # response can never be sent. Waiting on it will always time out so we don't allow this | |
| # command to be run with 'wait' | |
| if cmd.unique_id == 'pyrevitcore_pyrevit_pyrevit_tools_reload': | |
| return { | |
| "error": "Cannot await PyRevit reload: the reload script destroys " | |
| "the runtime engine before a response can be sent. " | |
| "Use wait=false to fire-and-forget instead." | |
| } | |
| return routes.make_response( | |
| data={"error": "control_id is required in request body"}, | |
| status=400, | |
| ) | |
| # fire-and-forget via PostCommand | |
| if not wait: | |
| command_id = UI.RevitCommandId.LookupCommandId(control_id) | |
| if command_id is None: | |
| return routes.make_response( | |
| data={"error": "Command not found: {}".format(control_id)}, | |
| status=404, | |
| ) | |
| uiapp.PostCommand(command_id) | |
| return routes.make_response( | |
| data={"result": "posted", "control_id": control_id}, | |
| status=200, | |
| ) | |
| cmd = next((c for c in sessionmgr.find_all_commands() | |
| if c.control_id == control_id), None) | |
| if cmd is None: | |
| return routes.make_response( | |
| data={"error": "Command not found: {}".format(control_id)}, | |
| status=404, | |
| ) | |
| # PyRevit reload destroys the IronPython engine mid-execution, so the HTTP | |
| # response can never be sent. Waiting on it will always time out so we don't allow this | |
| # command to be run with 'wait' | |
| if cmd.unique_id == 'pyrevitcore_pyrevit_pyrevit_tools_reload': | |
| return routes.make_response( | |
| data={ | |
| "error": "Cannot await PyRevit reload: the reload script destroys " | |
| "the runtime engine before a response can be sent. " | |
| "Use wait=false to fire-and-forget instead." | |
| }, | |
| status=400, | |
| ) |
| async def run_command_by_control_id(control_id: str, config:bool = False, wait:bool=False ,ctx: Context) -> str: | ||
| """ |
There was a problem hiding this comment.
run_command_by_control_id has a non-default parameter (ctx) after default parameters, which is a Python syntax error. Make ctx optional with a default (like other tools) and/or reorder parameters so all non-defaults come first.
I set this up myself in a similar way, saw you already had something, decided to ammend my stuff to yours.
So what is this?
These tools allow for an agent to list all pyrevit controls and push them for the user without having to touch revit.
The routes provided (
commands_list, andcommands_run) give the agent access to the available toolset, and the ability to use them.What's more, it runs scripts using
sessionmgr.execute_command_cls()and tries to capture the actual command logs, giving the agent feedback on what happened inside revit.note: pyrevit file logging must be enabled for this to work as it reads these logs. So that means it's not 100% reliable if you're running multiple revit sessions.
Why is this usefull?
I'm using this for my debugging of actual tools, having the agent modify and test my pushbuttons is nice.
what's with the env var??
I'm glad you asked, using this env var my scripts are able to know if they've been called by an agent. That way we can turn off UI calls if need be.
I also did a small change on the code_execution.py file, moved something to the 'finally' block to ensure it gets executed.
Created as draft because I still want to test a few things before shoving it out.