Cloud/deployable mcp agent server temporal #508
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughUpdates rename example entrypoints to main.py in docs and tooling, move nested server path resolution to local example directories, add two new Temporal nested servers (sampling and elicitation), update a Temporal worker import, add openai to Temporal requirements, and document/implement a change to Executor.wait_for_signal signature (now accepts signal_name, workflow_id, run_id). Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant MainApp as Main App (asyncio/temporal)
participant UV as uv subprocess
participant Nested as Nested MCP Server
participant Model as Model API
participant Executor as Temporal Executor
User->>MainApp: Trigger tool (sampling / elicitation / workflow pause/resume)
MainApp->>UV: Start "uv run <nested_*.py>" (launch nested server)
UV->>Nested: Launch FastMCP server
alt Sampling flow
MainApp->>Nested: get_haiku(topic)
Nested->>Model: SamplingRequest (system + user, prefs)
Model-->>Nested: TextContent result
Nested-->>MainApp: Haiku text
else Elicitation flow
MainApp->>Nested: confirm_action(action)
Nested->>User: Prompt with validation schema
User-->>Nested: Accepted/Rejected
Nested-->>MainApp: Confirmation/Decline message
end
alt Workflow resume
MainApp->>Executor: wait_for_signal(signal_name="resume", workflow_id, run_id)
Executor-->>MainApp: Resume confirmation / timeout
end
MainApp-->>User: Return result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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 |
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
examples/mcp_agent_server/temporal/README.md (2)
321-331: Fix typo and simplify example extracting workflow/run IDs.
exectionis misspelled and importing SDK types in docs is unnecessary. Parse JSON directly.- execution = WorkflowExecution( - **json.loads(pause_result.content[0].text) - ) - - run_id = execution.run_id - workflow_id = exection.workflow_id + import json + data = json.loads(pause_result.content[0].text) + run_id = data.get("run_id") + workflow_id = data.get("workflow_id")
274-276: Include timeout in wait_for_signal snippet.Prevents indefinite hangs and matches the example workflow.
- await app.context.executor.wait_for_signal( - signal_name="resume", workflow_id=self.id, run_id=self.run_id, - ) + await app.context.executor.wait_for_signal( + signal_name="resume", + workflow_id=self.id, + run_id=self.run_id, + timeout_seconds=60, + )examples/mcp_agent_server/temporal/main.py (1)
197-197: Prefer pathlib for nested script resolution; also guard for missing files.Tiny readability win and clearer errors if files are absent in Cloud bundle.
- nested_path = os.path.abspath( - os.path.join(os.path.dirname(__file__), "nested_sampling_server.py") - ) + from pathlib import Path + nested_path = str(Path(__file__).with_name("nested_sampling_server.py").resolve()) + assert Path(nested_path).is_file(), f"Missing nested server: {nested_path}"- nested_path = os.path.abspath( - os.path.join(os.path.dirname(__file__), "nested_elicitation_server.py") - ) + from pathlib import Path + nested_path = str(Path(__file__).with_name("nested_elicitation_server.py").resolve()) + assert Path(nested_path).is_file(), f"Missing nested server: {nested_path}"Operational note: ensure the Cloud image has
uvon PATH for nested servers. If not, consider makingcommandconfigurable viamcp_agent.config.yamlpaths.Also applies to: 235-235
examples/mcp_agent_server/temporal/requirements.txt (1)
5-6: Pin major versions for reproducible cloud deploysexamples/mcp_agent_server/temporal/requirements.txt (lines 5–6) lists unpinned packages; pin them to a safe major range to avoid drifting/breaking installs.
-openai -temporalio +openai>=1,<2 +temporalio>=1,<2examples/mcp_agent_server/asyncio/main.py (1)
131-131: Use pathlib for simpler, robust path resolution (fail fast if missing).Verified examples/mcp_agent_server/asyncio/nested_sampling_server.py and nested_elicitation_server.py exist — apply at examples/mcp_agent_server/asyncio/main.py lines 131 and 169.
- nested_path = os.path.abspath( - os.path.join(os.path.dirname(__file__), "nested_sampling_server.py") - ) + from pathlib import Path + nested_path = str(Path(__file__).with_name("nested_sampling_server.py").resolve())- nested_path = os.path.abspath( - os.path.join(os.path.dirname(__file__), "nested_elicitation_server.py") - ) + from pathlib import Path + nested_path = str(Path(__file__).with_name("nested_elicitation_server.py").resolve())
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
examples/mcp_agent_server/asyncio/README.md(5 hunks)examples/mcp_agent_server/asyncio/main.py(2 hunks)examples/mcp_agent_server/temporal/README.md(6 hunks)examples/mcp_agent_server/temporal/basic_agent_server_worker.py(1 hunks)examples/mcp_agent_server/temporal/main.py(3 hunks)examples/mcp_agent_server/temporal/nested_elicitation_server.py(1 hunks)examples/mcp_agent_server/temporal/nested_sampling_server.py(1 hunks)examples/mcp_agent_server/temporal/requirements.txt(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-07-22T18:59:49.368Z
Learnt from: CR
PR: lastmile-ai/mcp-agent#0
File: examples/usecases/reliable_conversation/CLAUDE.md:0-0
Timestamp: 2025-07-22T18:59:49.368Z
Learning: Applies to examples/usecases/reliable_conversation/examples/reliable_conversation/src/**/*.py : Use mcp-agent's Agent abstraction for ALL LLM interactions, including quality evaluation, to ensure consistent tool access, logging, and error handling.
Applied to files:
examples/mcp_agent_server/temporal/requirements.txtexamples/mcp_agent_server/asyncio/README.mdexamples/mcp_agent_server/temporal/README.md
📚 Learning: 2025-09-05T14:31:48.139Z
Learnt from: rholinshead
PR: lastmile-ai/mcp-agent#414
File: src/mcp_agent/logging/logger.py:18-19
Timestamp: 2025-09-05T14:31:48.139Z
Learning: In the mcp-agent logging module (src/mcp_agent/logging/logger.py), temporalio should be imported lazily with try/except ImportError to avoid making it a hard dependency. Use temporalio.workflow.in_workflow() instead of isinstance checks on internal classes like _WorkflowInstanceImpl.
Applied to files:
examples/mcp_agent_server/temporal/requirements.txtexamples/mcp_agent_server/temporal/basic_agent_server_worker.py
🧬 Code graph analysis (3)
examples/mcp_agent_server/temporal/nested_sampling_server.py (2)
examples/mcp_agent_server/asyncio/nested_sampling_server.py (1)
get_haiku(8-32)src/mcp_agent/executor/temporal/session_proxy.py (1)
create_message(250-289)
examples/mcp_agent_server/temporal/basic_agent_server_worker.py (1)
examples/mcp_agent_server/temporal/main.py (1)
main(394-406)
examples/mcp_agent_server/temporal/nested_elicitation_server.py (3)
src/mcp_agent/core/context.py (1)
mcp(102-103)examples/mcp_agent_server/temporal/nested_sampling_server.py (1)
main(35-36)examples/mcp_agent_server/asyncio/nested_sampling_server.py (1)
main(35-36)
🔇 Additional comments (4)
examples/mcp_agent_server/temporal/README.md (1)
144-144: LGTM on entrypoint rename in Temporal docs.References now point to
main.py.Also applies to: 218-218, 243-243
examples/mcp_agent_server/temporal/nested_elicitation_server.py (1)
1-31: LGTM: clear, minimal elicitation server.Good use of
elicit_with_validationand simple confirmation schema.examples/mcp_agent_server/temporal/nested_sampling_server.py (1)
1-41: LGTM: nested sampling server is straightforward and aligns with Session API.Nice defaults and model hints; fallback message is fine.
examples/mcp_agent_server/temporal/basic_agent_server_worker.py (1)
15-15: Import path change is correct for this folder layout.Given invocation from the same directory (
uv run basic_agent_server_worker.py),from main import appwill resolve.If you ever run the worker from another CWD, document using
PYTHONPATH=.or switch to a package layout.
…agent-server-temporal
Co-authored-by: graphite-app[bot] <96075541+graphite-app[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
examples/mcp_agent_server/temporal/README.md (3)
253-253: Naming consistency: consider renaming the worker file for symmetry.
Optional: rename basic_agent_server_worker.py to temporal_worker.py (or similar) in a follow‑up to match the main.py simplification and reduce cognitive load.
274-276: Updated wait_for_signal usage — validate API and consider timeout/cancel note.
The new signature looks correct; consider noting a timeout or cancel‑signal pattern so examples don’t imply indefinite blocking. Also, given the PR note that “resume is currently ignored” in cloud, add a short “Known issue” callout in this section to set expectations until fixed.
321-324: Client snippet: add missing imports and minimal parsing safety.
As written, WorkflowExecution and json aren’t imported; parsing assumes JSON text. Inline imports are fine in examples and keep the snippet runnable.Apply this diff within the snippet:
- execution = WorkflowExecution( - **json.loads(pause_result.content[0].text) - ) - - run_id = execution.run_id - workflow_id = execution.workflow_id + import json + from mcp_agent.temporal import WorkflowExecution + data = json.loads(pause_result.content[0].text) + execution = WorkflowExecution(**data) + run_id = execution.run_id + workflow_id = execution.workflow_idOptional: guard for empty content and raise a clear error if pause_result.content is missing.
Also applies to: 325-327, 330-330
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
examples/mcp_agent_server/temporal/README.md(6 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-22T18:59:49.368Z
Learnt from: CR
PR: lastmile-ai/mcp-agent#0
File: examples/usecases/reliable_conversation/CLAUDE.md:0-0
Timestamp: 2025-07-22T18:59:49.368Z
Learning: Applies to examples/usecases/reliable_conversation/examples/reliable_conversation/src/**/*.py : Use mcp-agent's Agent abstraction for ALL LLM interactions, including quality evaluation, to ensure consistent tool access, logging, and error handling.
Applied to files:
examples/mcp_agent_server/temporal/README.md
🔇 Additional comments (2)
examples/mcp_agent_server/temporal/README.md (2)
218-219: MCP Inspector example updated to main.py — LGTM.
242-243: Claude Desktop config updated to main.py — LGTM.
| uv run main.py | ||
| ``` |
There was a problem hiding this comment.
🧩 Analysis chain
Renamed entrypoint to main.py — looks good; please sweep for stale references.
Docs reflect the rename; confirm no remaining references to basic_agent_server.py across the repo.
🏁 Script executed:
#!/bin/bash
# Find stale references to the old entrypoint name(s)
rg -nP -S '\bbasic[_-]agent[_-]server(\.py)?\b|BasicAgentServer' -g '!**/node_modules/**'Length of output: 148
🏁 Script executed:
#!/bin/bash
set -euo pipefail
PATTERN=$'\bbasic[_-]agent[_-]server(\.py)?\b|basicAgentServer|BasicAgentServer'
echo "Searching repo for stale entrypoint references..."
if command -v rg >/dev/null 2>&1; then
rg -nP --hidden --no-ignore -S --ignore-case --glob '!node_modules/**' "$PATTERN" || true
else
echo "rg not found; falling back to git grep"
git grep -n --ignore-case -e 'basic_agent_server' -e 'basic-agent-server' -e 'basicAgentServer' || true
fiLength of output: 198
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "PWD: $(pwd)"
echo "Top-level listing:"
ls -la || true
echo "Git repo root:"
git rev-parse --show-toplevel || true
echo "Git status (short):"
git status --porcelain --untracked-files=all || true
echo "Count files under repo (first 200 shown):"
find . -type f | wc -l || true
find . -type f -print | sed -n '1,200p' || true
echo "rg --version:"
rg --version || true
echo "Running rg --debug to see why files are skipped..."
PATTERN='\bbasic[_-]agent[_-]server(\.py)?\b|basicAgentServer|BasicAgentServer'
rg --debug -nP --hidden --no-ignore -S --ignore-case --glob '!node_modules/**' "$PATTERN" || true
echo "Fallback: git grep for common variants"
git grep -n --ignore-case -e 'basic_agent_server' -e 'basic-agent-server' -e 'basicAgentServer' || trueLength of output: 181560
Stale references to basic_agent_server remain — update to main.py
Update these files (docs, examples, templates, and CLI code) to reference main.py (and rename worker script references where appropriate):
- docs/cli-reference.mdx
- examples/mcp_agent_server/README.md
- examples/mcp_agent_server/asyncio/README.md
- examples/mcp_agent_server/asyncio/client.py
- examples/mcp_agent_server/asyncio/main.py
- examples/mcp_agent_server/temporal/README.md
- examples/mcp_agent_server/temporal/client.py
- examples/mcp_agent_server/temporal/main.py
- examples/mcp_agent_server/temporal/basic_agent_server_worker.py
- examples/usecases/reliable_conversation/CLAUDE.md
- src/mcp_agent/cli/commands/init.py
- src/mcp_agent/cli/commands/quickstart.py
- src/mcp_agent/cli/commands/serve.py
- src/mcp_agent/data/templates/basic_agent_server.py
🤖 Prompt for AI Agents
In examples/mcp_agent_server/temporal/README.md around lines 144-145, there are
stale references to basic_agent_server; update this and all listed files to
reference main.py instead (and rename any worker script references where
appropriate). Specifically, replace occurrences of "basic_agent_server" with
"main.py" (or adjust to the new worker script name), update docs/CLI examples
and templates to call main.py, rename or update
examples/mcp_agent_server/temporal/basic_agent_server_worker.py to the new
worker filename and adjust imports/usages, and update src/mcp_agent/cli/*
commands and template src/mcp_agent/data/templates/basic_agent_server.py to
reflect the new filenames/entrypoints so all examples, docs, and CLI commands
consistently reference main.py.
2 participants
Summary
Get the
mcp_agent/temporalexample cloud deployable:basic_agent_server.pytomain.pyRetested locally and everything works. Deployed to cloud and everything works except PauseResumeWorkflow; still investigating why the resume signal in temporal is ignored.
Summary by CodeRabbit
New Features
Bug Fixes / Improvements
Documentation
Chores