-
Notifications
You must be signed in to change notification settings - Fork 781
Refactor workflow runs list to use MCP tool calls #439
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor workflow runs list to use MCP tool calls #439
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughImplements an async CLI workflow-runs listing that resolves server URL from direct input or config, calls an embedded MCPApp tool client to retrieve runs, parses and filters results by status and limit, and prints json/yaml/text; also removes workflow models from the MCP App API client and adds log_entries_list. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant CLI as CLI:list_workflow_runs
participant Runner as run_async
participant Impl as _list_workflow_runs_async
participant Resolver as ServerResolver
participant App as MCPApp
participant Client as GeneratedClient
participant Server as WorkflowServer
User->>CLI: list_workflow_runs --server/--server-id --status --limit --format
CLI->>Runner: run_async(_list_workflow_runs_async)
Runner->>Impl: invoke/await
Impl->>Resolver: resolve server-id or accept URL
Resolver-->>Impl: serverUrl or raise CLIError
Impl->>App: init MCPApp, register workflow_server (SSE)
Impl->>Client: call tool "workflows-runs-list"
Client->>Server: request runs
Server-->>Client: runs response
Client-->>Impl: result.content[0].text
Impl->>Impl: parse text → JSON (or default [])
Impl->>Impl: apply _get_status_filter / _matches_status
Impl->>Impl: apply limit
Impl-->>CLI: formatted output (json|yaml|text)
CLI-->>User: printed results
note over Impl: On errors -> raise CLIError (handled by CLI)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This stack of pull requests is managed by Graphite. Learn more about stacking. |
0df7155 to
9f12b3a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (8)
src/mcp_agent/cli/cloud/commands/workflows/runs/main.py (8)
50-52: Normalize SSE URL to avoid double slashes and '/sse/sse' edge cases.Using rstrip prevents '...//sse' and handles trailing '/sse/' cleanly.
- sse_url = ( - f"{server_url}/sse" if not server_url.endswith("/sse") else server_url - ) + base_url = server_url.rstrip("/") + sse_url = base_url if base_url.endswith("/sse") else f"{base_url}/sse"
53-58: Set sane timeouts and least-privilege tool access for the transient server entry.Prevents hanging on dead SSE streams and avoids exposing unnecessary tools.
context.server_registry.registry["workflow_server"] = MCPServerSettings( name="workflow_server", description=f"Deployed MCP server {server_url}", url=sse_url, transport="sse", + read_timeout_seconds=30, + http_timeout_seconds=10, + allowed_tools={"workflows-runs-list"}, )
77-78: Validate that --limit is a positive integer.Avoids surprising slices when negative (tail of list) or zero (empty).
- if limit: - workflows = workflows[:limit] + if limit is not None: + if limit <= 0: + raise CLIError("--limit must be a positive integer") + workflows = workflows[:limit]
85-85: Pass the normalized status to the printer.Keeps the “Filtered by status” display consistent with the filter actually applied.
- _print_workflows_text(workflows, status, server_id_or_url) + _print_workflows_text( + workflows, + status_filter if status else None, + server_id_or_url, + )
123-136: Add common status aliases (success/succeeded/complete → completed).Improves UX parity with typical user input.
status_map = { "running": "running", "failed": "failed", "timed_out": "timed_out", "timeout": "timed_out", # alias "canceled": "canceled", "cancelled": "canceled", # alias "terminated": "terminated", "completed": "completed", + "success": "completed", + "succeeded": "completed", + "complete": "completed", "continued": "continued", "continued_as_new": "continued", }
146-151: Prefer exact match (with a safe fallback) for status matching to reduce false positives.Substring matching alone can misclassify unexpected values.
- workflow_status = workflow.get("execution_status", "") - if isinstance(workflow_status, str): - return status_filter.lower() in workflow_status.lower() + workflow_status = workflow.get("execution_status", "") + if isinstance(workflow_status, str): + status_val = workflow_status.lower() + target = status_filter.lower() + return status_val == target or status_val.endswith(target) return False
270-271: Rich color name: use orange1 instead of orange.'orange' isn't a standard Rich color name; 'orange1' is.
- elif "timeout" in status_str or "timed_out" in status_str: - return "[orange]⏰ Timed Out[/orange]" + elif "timeout" in status_str or "timed_out" in status_str: + return "[orange1]⏰ Timed Out[/orange1]"
233-236: Normalize created_at for dict inputs to ensure consistent JSON/YAML output.Prevents non-serializable datetime objects sneaking into dict-based paths.
- if isinstance(workflow, dict): - return workflow + if isinstance(workflow, dict): + w = dict(workflow) + ca = w.get("created_at") + if hasattr(ca, "isoformat"): + w["created_at"] = ca.isoformat() + return w
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/mcp_agent/cli/cloud/commands/workflows/runs/main.py(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/mcp_agent/cli/cloud/commands/workflows/runs/main.py (5)
src/mcp_agent/cli/core/utils.py (1)
run_async(16-31)src/mcp_agent/cli/exceptions.py (1)
CLIError(4-9)src/mcp_agent/config.py (2)
MCPServerSettings(52-113)LoggerSettings(535-580)src/mcp_agent/mcp/gen_client.py (1)
gen_client(16-41)src/mcp_agent/cli/cloud/commands/utils.py (3)
setup_authenticated_client(18-32)validate_output_format(35-48)resolve_server(51-77)
| def _get_status_filter(status: str) -> str: | ||
| """Convert status string to normalized status.""" | ||
| status_map = { | ||
| "running": "running", | ||
| "failed": "failed", | ||
| "timed_out": "timed_out", | ||
| "timeout": "timed_out", # alias | ||
| "canceled": "canceled", | ||
| "cancelled": "canceled", # alias | ||
| "terminated": "terminated", | ||
| "completed": "completed", | ||
| "continued": "continued", | ||
| "continued_as_new": "continued", | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like our WorkflowExecutionStatus got nuked? Can you restore?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One important comment, rest lgtm
Merge activity
|
| "workflow_id": getattr(workflow, "workflow_id", None), | ||
| "run_id": getattr(workflow, "run_id", None), | ||
| "name": getattr(workflow, "name", None), | ||
| "created_at": getattr(workflow, "created_at", None).isoformat() if getattr(workflow, "created_at", None) else None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line contains a potential bug due to redundant attribute access. The expression calls getattr(workflow, "created_at", None) twice, which is inefficient and could cause errors if the first call returns a falsy value but the attribute doesn't exist.
Consider refactoring to:
created_at_value = getattr(workflow, "created_at", None)
"created_at": created_at_value.isoformat() if created_at_value and hasattr(created_at_value, 'isoformat') else None,This approach safely handles all cases: when the attribute doesn't exist, when it exists but is None/falsy, and when it exists and has an isoformat method.
| "created_at": getattr(workflow, "created_at", None).isoformat() if getattr(workflow, "created_at", None) else None, | |
| "created_at": created_at_value.isoformat() if (created_at_value := getattr(workflow, "created_at", None)) and hasattr(created_at_value, 'isoformat') else None, |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
| workflow_status = workflow.get("execution_status", "") | ||
| if isinstance(workflow_status, str): | ||
| return status_filter.lower() in workflow_status.lower() | ||
| return False |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current status matching logic has a potential issue with false positives. Using status_filter.lower() in workflow_status.lower() performs substring matching, which means a status filter of running would incorrectly match statuses like not_running_anymore.
Consider using exact matching instead:
# More precise matching options:
return workflow_status.lower() == status_filter.lower()
# Or if matching against enum-style values:
return workflow_status.lower().endswith(status_filter.lower())This would ensure that only workflows with the exact requested status are returned to the user.
| workflow_status = workflow.get("execution_status", "") | |
| if isinstance(workflow_status, str): | |
| return status_filter.lower() in workflow_status.lower() | |
| return False | |
| workflow_status = workflow.get("execution_status", "") | |
| if isinstance(workflow_status, str): | |
| return workflow_status.lower() == status_filter.lower() | |
| return False | |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
* Temporarily exclude CLI from test coverage (#429) ### TL;DR Exclude CLI code from test coverage metrics for now. Will add tests when we're done sprinting 10000 mph  <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Adjusted test coverage collection to exclude non-critical CLI components, resulting in more accurate coverage metrics for core functionality. * **Chores** * Updated coverage reporting configuration to align with the new exclusion rules, ensuring consistent results across local and CI runs. <!-- end of auto-generated comment: release notes by coderabbit.ai --> * Add workflow commands to CLI (#424) ### TL;DR Added workflow management commands to the MCP Agent CLI, including describe, suspend, resume, and cancel operations. ### What changed? - Added four new workflow management commands: - `describe_workflow`: Shows detailed information about a workflow execution - `suspend_workflow`: Pauses a running workflow execution - `resume_workflow`: Resumes a previously suspended workflow - `cancel_workflow`: Permanently stops a workflow execution - Implemented corresponding API client methods in `WorkflowAPIClient`: - `suspend_workflow` - `resume_workflow` - `cancel_workflow` - Updated the CLI structure to expose these commands under `mcp-agent cloud workflows` - Added an alias for `describe_workflow` as `status` for backward compatibility ### How to test? Test the new workflow commands with a running workflow: ``` # Get workflow details mcp-agent cloud workflows describe run_abc123 mcp-agent cloud workflows status run_abc123 # alias # Suspend a workflow mcp-agent cloud workflows suspend run_abc123 # Resume a workflow (with optional payload) mcp-agent cloud workflows resume run_abc123 mcp-agent cloud workflows resume run_abc123 --payload '{"data": "value"}' # Cancel a workflow (with optional reason) mcp-agent cloud workflows cancel run_abc123 mcp-agent cloud workflows cancel run_abc123 --reason "User requested cancellation" ``` ### Why make this change? These commands provide essential workflow lifecycle management capabilities to users, allowing them to monitor and control workflow executions through the CLI. The ability to suspend, resume, and cancel workflows gives users more control over long-running operations and helps manage resources more efficiently. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - New Features - Introduced “workflows” CLI group with commands: describe (alias: status), resume, suspend, and cancel. - Describe supports text, JSON, and YAML output; all commands work with server ID or URL and include improved error messages. - Refactor - Renamed CLI group from “workflow” to “workflows” and reorganized command registrations. - Consolidated internal utility imports (no behavior change). - Chores - Updated module descriptions. - Removed legacy workflow status package/exports in favor of the new workflows commands. <!-- end of auto-generated comment: release notes by coderabbit.ai --> * add servers workflow subcommand (#428) # Add servers workflows subcommand This PR adds a new `workflows` subcommand to the `mcp-agent cloud servers` command that allows users to list workflows associated with a specific server. The command supports: - Filtering by workflow status - Limiting the number of results - Multiple output formats (text, JSON, YAML) - Accepting server IDs, app config IDs, or server URLs as input Examples: ``` mcp-agent cloud servers workflows app_abc123 mcp-agent cloud servers workflows https://server.example.com --status running mcp-agent cloud servers workflows apcnf_xyz789 --limit 10 --format json ``` The PR also cleans up the examples in the existing workflow commands and adds the necessary API client support for listing workflows. * add workflow list and runs (#430) ### TL;DR Reorganized workflow commands `mcp-agent cloud workflows runs` `mcp-agent cloud workflows list` `mcp-agent cloud server workflows` (alias of workflows list) ### What changed? - Moved `list_workflows_for_server` from the servers module to the workflows module as `list_workflow_runs` - Added new workflow commands: `list_workflows` and `list_workflow_runs` - Updated CLI command structure to make workflows commands more intuitive - Applied consistent code formatting with black across all server and workflow related files ### How to test? Test the new and reorganized workflow commands: ```bash # List available workflow definitions mcp-agent cloud workflows list app_abc123 # List workflow runs (previously under servers workflows) mcp-agent cloud workflows runs app_abc123 # Test with different output formats mcp-agent cloud workflows list app_abc123 --format json mcp-agent cloud workflows runs app_abc123 --format yaml # Verify existing commands still work mcp-agent cloud servers list mcp-agent cloud workflows describe app_abc123 run_xyz789 ``` * [ez] Move deploy command to cloud namespace (#431) ### TL;DR Added `cloud deploy` command as an alias for the existing `deploy` command. * First pass at implementing the mcp-agent CLI (#409) * Initial scaffolding * initial CLI * checkpoint * checkpoint 2 * various updates to cli * fix lint and format * fix: should load secrets.yaml template instead when running init cli command * fix: prevent None values in either mcp-agent secrets and config yaml files from overwriting one another when merging both * fix: when running config check, use get_settings() instead of Settings() to ensure settings are loaded. * fix: handle None values for servers in MCPSettings so it defaults to empty dict and update secrets.yaml template so it does not overwrite mcp servers in config * Inform users to save and close editor to continue when running config edit command * fix: Update openai, anthropic and azure regex for keys cli command * Sort model list by provider and model name * Add filtering support for models list cli command * disable untested commands * lint, format, gen_schema * get rid of accidental otlp exporter changes from another branch * get rid of accidental commit from other branch --------- Co-authored-by: StreetLamb <[email protected]> * Docs MVP (#436) * Initial scaffolding * initial CLI * checkpoint * checkpoint 2 * various updates to cli * fix lint and format * fix: should load secrets.yaml template instead when running init cli command * fix: prevent None values in either mcp-agent secrets and config yaml files from overwriting one another when merging both * fix: when running config check, use get_settings() instead of Settings() to ensure settings are loaded. * fix: handle None values for servers in MCPSettings so it defaults to empty dict and update secrets.yaml template so it does not overwrite mcp servers in config * Inform users to save and close editor to continue when running config edit command * fix: Update openai, anthropic and azure regex for keys cli command * Sort model list by provider and model name * Add filtering support for models list cli command * disable untested commands * Fixes to docs * Updating the main.py and !developer_secrets for secrets * updating python entry files to main.py * Fix tracer.py --------- Co-authored-by: StreetLamb <[email protected]> Co-authored-by: Andrew Hoh <[email protected]> * fix: max complete token for openai gen structured (#438) * Fix regression in CLI ("cloud cloud") * docs fixes * Fix top-level cli cloud commands (deploy, login, etc) * Add eager tool validation to ensure json serializability of input params/result types * More docs updates * Refactor workflow runs list to use MCP tool calls (#439) ### TL;DR Refactored the workflow runs listing command to use MCP tool calls instead of direct API client calls. ### What changed? - Replaced the direct API client approach with MCP tool calls to retrieve workflow runs - Added a new `_list_workflow_runs_async` function that uses the MCP App and gen_client to communicate with the server - Improved status filtering and display logic to work with both object and dictionary response formats - Enhanced error handling and formatting of workflow run information - Updated the workflow data processing to handle different response formats more robustly ### How to test? ```bash # List workflow runs from a server mcp-agent cloud workflows runs <server_id_or_url> # Filter by status mcp-agent cloud workflows runs <server_id_or_url> --status running # Limit results mcp-agent cloud workflows runs <server_id_or_url> --limit 10 # Change output format mcp-agent cloud workflows runs <server_id_or_url> --format json ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - New Features - Add status filtering for workflow runs, with common aliases (e.g., timeout → timed_out). - Add an optional limit to constrain the number of results. - Allow server selection via direct URL or config-based server ID. - Refactor - Update text output: columns now show Workflow ID, Name, Status, Run ID, Created; Principal removed. - Improve date formatting and consistent JSON/YAML/Text rendering. - Bug Fixes - Clearer error messages and safer handling when server info is missing or no data is returned. <!-- end of auto-generated comment: release notes by coderabbit.ai --> * Update workflows commands UI to be more consistant with the rest of the CLI (#432) ### TL;DR Improved CLI workflow command output formatting with better visual indicators and consistent styling. ### How to test? ``` mcp-agent cloud workflows cancel <run-id> mcp-agent cloud workflows describe <run-id> mcp-agent cloud workflows resume <run-id> ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Style** * Cancel workflow: added a blank line before the status and changed the success icon to 🚫 (yellow). * Describe workflow: replaced panel UI with a clean, header-based text layout (“🔍 Workflow Details”), showing name with colorized status and fields for Workflow ID, Run ID, and Created. Updated status indicators with emojis and colors; timestamp is now plain text on its own line. * Resume workflow: success message now applies consistent coloring to the entire line for improved readability. <!-- end of auto-generated comment: release notes by coderabbit.ai --> * Feature: Update Workflow Tool Calls to Work with workflow_id (#435) * Support for workflow_id and run_id * Update temporal workflow registry * tests * Update LLMS.txt * Fix config * Return bool for cancel result * Validate ids provided * Fix cancel workflow id * Fix workflows-resume response * Add workflow-name specific resume and cancel tools * Fix return type * Fix examples * Remove redundant workflows-{name}-tool tool calls * Add _workflow_status back * Use registry helper * Changes from review * Add back evaluator_optimizer enum fix * Fix a hang that can happen at shutdown (#440) * Fix a shutdown hang * Fix tests * fix taskgroup closed in a different context than when it was started in error * some PR feedback fixes * PR feedback * Fix random failures of server aggregator not found for agent in temporal (#441) * Fix a shutdown hang * Fix tests * fix taskgroup closed in a different context than when it was started in error * some PR feedback fixes * Fix random failures of server aggregator not found for agent in temporal environment * Bump pyproject version * Fix gateway URL resolution (#443) * Fix gateway URL resolution Removed incorrect dependence on ServerRegistry for gateway URLs; the gateway is not an MCP server. App server (src/mcp_agent/server/app_server.py) builds workflow memo with: - gateway_url precedence: X-MCP-Gateway-URL or X-Forwarded-Url → reconstruct X-Forwarded-Proto/Host/Prefix → request.base_url → MCP_GATEWAY_URL env. - gateway_token precedence: X-MCP-Gateway-Token → MCP_GATEWAY_TOKEN env. Worker-side (SystemActivities/SessionProxy) uses memo.gateway_url and gateway_token; falls back to worker env. Client proxy helpers (src/mcp_agent/mcp/client_proxy.py): - _resolve_gateway_url: explicit param → context → env → local default. - Updated public signatures to drop server_registry parameter. * Cloud/deployable temporal example (#395) * Move workflows to workflows.py file * Fix router example * Add remaining dependencies * Update orchestrator to @app.async_tool example * Changes from review * Fix interactive_workflow to be runnable via tool * Fix resume tool params * Fix: Use helpful typer and invoke for root cli commands (#444) * Use helpful typer and invoke for root cli commands * Fix lint * Fix enum check (#445) * Fix/swap relative mcp agent dependency on deploy (#446) * Update wrangler wrapper to handle requirements.txt processing * Fix backup handling * pass api key to workflow (#447) * pass api key to workflow * guard against settings not existing --------- Co-authored-by: John Corbett <[email protected]> Co-authored-by: Sarmad Qadri <[email protected]> Co-authored-by: StreetLamb <[email protected]> Co-authored-by: Yi <[email protected]> Co-authored-by: Ryan Holinshead <[email protected]> Co-authored-by: roman-van-der-krogt <[email protected]>

TL;DR
Refactored the workflow runs listing command to use MCP tool calls instead of direct API client calls.
What changed?
_list_workflow_runs_asyncfunction that uses the MCP App and gen_client to communicate with the serverHow to test?
Summary by CodeRabbit
New Features
Refactor
Bug Fixes