add human_interaction handler using MCP elicitation #507
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. WalkthroughAdds an elicitation-based human input handler and Temporal example (MCP app, worker, client, configs). Updates multiple examples to import console_input_callback from mcp_agent.human_input.console_handler. Introduces tests for the new elicitation handler. Minor README edits and new example requirements/secrets files. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Client as Temporal Client
participant Worker as Temporal Worker
participant App as MCP App (Server)
participant LLM as LLM/Agent
rect rgb(240,248,255)
note over Client,Worker: Start workflow
Client->>Worker: Start greet workflow
Worker->>App: Call tool "greet" (MCP)
App->>LLM: Prompt: "Ask the user for their name and greet them."
end
rect rgb(245,255,240)
note right of App: Elicitation (new)
LLM-->>App: Requires user input
App-->>Worker: Elicit(response schema, related_request_id)
Worker-->>Client: Signal/Request input
Client->>User: Console prompt
User-->>Client: Name
Client-->>Worker: Provide input
Worker-->>App: Elicitation result {action: accept, content:{response}}
end
App->>LLM: Provide user's response
LLM-->>App: Final message
App-->>Worker: Tool result (greeting)
Worker-->>Client: Workflow completes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
…stmile-ai/mcp-agent into feat/user_interaction_via_elicitation
There was a problem hiding this comment.
Looks great overall! Have you seen if this would work with the human input tool that gets added to agent.py? I'm guessing that would be the underlying mechanism by which an agent would be able to pause the workflow by invoking the human input request as a tool, but wondering if that needs to be updated to instead be an elicitation request
|
|
||
| signal_name = execution_info.get("signal_name", "") | ||
|
|
||
| if signal_name == "": |
There was a problem hiding this comment.
Same question as in other PR if we need to check for None
…nto feat/user_interaction_via_elicitation
roman-van-der-krogt
changed the base branch from
feat/pause-resume-elicitation
to
main
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
| name="workflow_mcp_client", | ||
| # Disable sampling approval prompts entirely to keep flows non-interactive. | ||
| # Elicitation remains interactive via console_elicitation_callback. | ||
| human_input_callback=console_input_callback, |
There was a problem hiding this comment.
There appears to be a discrepancy between the code implementation and the documentation. The README mentions that the client should use console_elicitation_callback, but the code is using console_input_callback. For consistency, consider changing this line to use elicitation_callback=console_elicitation_callback to align with the documentation.
| human_input_callback=console_input_callback, | |
| elicitation_callback=console_elicitation_callback, |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
![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
♻️ Duplicate comments (1)
examples/human_input/temporal/client.py (1)
16-24: Consider adding type hints for fallback variables.Similar to the earlier comment in the other client file, consider adding explicit type annotations for clarity.
- _ExceptionGroup = None # type: ignore + _ExceptionGroup: type[BaseException] | None = None # type: ignore- _BrokenResourceError = None # type: ignore + _BrokenResourceError: type[BaseException] | None = None # type: ignore
🧹 Nitpick comments (6)
src/mcp_agent/human_input/elicitation_handler.py (2)
19-51: Consider extracting slash command constants.The function handles the response conversion well, but the hardcoded slash commands could be made more maintainable.
Consider extracting the slash commands as module-level constants:
+# Slash commands that can be used as responses +DECLINE_COMMAND = "/decline" +CANCEL_COMMAND = "/cancel" + def _handle_elicitation_response( result: types.ElicitResult, request: HumanInputRequest ) -> HumanInputResponse: """Convert ElicitResult back to HumanInputResponse.""" request_id = request.request_id or "" # Handle different action types if result.action == "accept": if result.content and isinstance(result.content, dict): response_text = result.content.get("response", "") # Handle slash commands that might be in the response response_text = response_text.strip() - if response_text.lower() in ["/decline", "/cancel"]: + if response_text.lower() in [DECLINE_COMMAND, CANCEL_COMMAND]: return HumanInputResponse(request_id=request_id, response=response_text.lower())
59-66: Consider simplifying the context retrieval error handling.The nested try-except blocks could be simplified since both paths raise the same
RuntimeError.# Try to get the context and session proxy - try: - from mcp_agent.core.context import get_current_context - context = get_current_context() - if context is None: - raise RuntimeError("No context available for elicitation") - except Exception: - raise RuntimeError("No context available for elicitation") + from mcp_agent.core.context import get_current_context + try: + context = get_current_context() + if context is None: + raise RuntimeError("No context available for elicitation") + except Exception as e: + raise RuntimeError("No context available for elicitation") from eThis preserves the original exception chain while simplifying the structure.
examples/mcp/mcp_elicitation/temporal/client.py (1)
19-27: Consider adding type hints for fallback variables.The fallback patterns for optional dependencies are good for robustness. However, consider adding explicit type annotations for clarity:
- _ExceptionGroup = None # type: ignore + _ExceptionGroup: type[BaseException] | None = None # type: ignore- _BrokenResourceError = None # type: ignore + _BrokenResourceError: type[BaseException] | None = None # type: ignoreexamples/human_input/temporal/README.md (1)
30-30: Wrap bare URL in angle brackets.For better Markdown formatting, consider wrapping the URL in angle brackets.
-1. Install the Temporal CLI by following the instructions at: https://docs.temporal.io/cli/ +1. Install the Temporal CLI by following the instructions at: <https://docs.temporal.io/cli/>examples/human_input/temporal/client.py (2)
61-68: Add type hints to the nested function.The
_deep_mergefunction lacks type hints, which would improve code maintainability.- def _deep_merge(base: dict, overlay: dict) -> dict: + def _deep_merge(base: dict[str, Any], overlay: dict[str, Any]) -> dict[str, Any]:Also add the import at the top of the file:
+from typing import Any
180-190: Remove unused helper function.The
_tool_result_to_jsonfunction is defined but never used in this file. Consider removing it to reduce code clutter.-def _tool_result_to_json(tool_result: CallToolResult): - if tool_result.content and len(tool_result.content) > 0: - text = tool_result.content[0].text - try: - # Try to parse the response as JSON if it's a string - import json - - return json.loads(text) - except (json.JSONDecodeError, TypeError): - # If it's not valid JSON, just use the text - return None
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
README.md(2 hunks)examples/human_input/temporal/README.md(1 hunks)examples/human_input/temporal/client.py(1 hunks)examples/human_input/temporal/main.py(1 hunks)examples/human_input/temporal/mcp_agent.config.yaml(1 hunks)examples/human_input/temporal/mcp_agent.secrets.yaml.example(1 hunks)examples/human_input/temporal/requirements.txt(1 hunks)examples/human_input/temporal/worker.py(1 hunks)examples/mcp/mcp_elicitation/main.py(1 hunks)examples/mcp/mcp_elicitation/temporal/client.py(1 hunks)examples/mcp_agent_server/asyncio/client.py(1 hunks)examples/mcp_agent_server/asyncio/main.py(1 hunks)examples/mcp_agent_server/temporal/main.py(1 hunks)examples/usecases/mcp_realtor_agent/main.py(1 hunks)examples/workflows/workflow_swarm/main.py(1 hunks)src/mcp_agent/data/examples/mcp_agent_server/elicitation/client.py(1 hunks)src/mcp_agent/data/examples/mcp_agent_server/elicitation/server.py(1 hunks)src/mcp_agent/data/examples/mcp_agent_server/reference/client.py(1 hunks)src/mcp_agent/data/examples/mcp_agent_server/reference/server.py(1 hunks)src/mcp_agent/human_input/elicitation_handler.py(1 hunks)tests/human_input/test_elicitation_handler.py(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 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/workflows/workflow_swarm/main.pyexamples/human_input/temporal/requirements.txtexamples/mcp_agent_server/asyncio/main.pyexamples/mcp/mcp_elicitation/main.pysrc/mcp_agent/data/examples/mcp_agent_server/elicitation/client.pyexamples/usecases/mcp_realtor_agent/main.pysrc/mcp_agent/data/examples/mcp_agent_server/reference/server.pyexamples/human_input/temporal/client.pyexamples/mcp/mcp_elicitation/temporal/client.py
📚 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/utils/config.py : Configuration values such as quality_threshold, max_refinement_attempts, consolidation_interval, and evaluator_model_provider must be loaded from mcp_agent.config.yaml.
Applied to files:
src/mcp_agent/data/examples/mcp_agent_server/elicitation/client.pyexamples/human_input/temporal/mcp_agent.config.yaml
📚 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/mcp_elicitation/temporal/client.py
🧬 Code graph analysis (16)
examples/workflows/workflow_swarm/main.py (1)
src/mcp_agent/human_input/console_handler.py (1)
console_input_callback(68-78)
examples/mcp_agent_server/asyncio/main.py (1)
src/mcp_agent/human_input/console_handler.py (1)
console_input_callback(68-78)
examples/mcp/mcp_elicitation/main.py (1)
src/mcp_agent/human_input/console_handler.py (1)
console_input_callback(68-78)
src/mcp_agent/data/examples/mcp_agent_server/elicitation/client.py (1)
src/mcp_agent/human_input/console_handler.py (1)
console_input_callback(68-78)
tests/human_input/test_elicitation_handler.py (3)
src/mcp_agent/executor/temporal/session_proxy.py (1)
elicit(324-340)src/mcp_agent/human_input/types.py (2)
HumanInputRequest(7-29)HumanInputResponse(32-42)src/mcp_agent/human_input/elicitation_handler.py (3)
elicitation_input_callback(53-123)_create_elicitation_message(10-16)_handle_elicitation_response(19-50)
examples/usecases/mcp_realtor_agent/main.py (1)
src/mcp_agent/human_input/console_handler.py (1)
console_input_callback(68-78)
src/mcp_agent/data/examples/mcp_agent_server/reference/server.py (1)
src/mcp_agent/human_input/console_handler.py (1)
console_input_callback(68-78)
examples/human_input/temporal/client.py (5)
src/mcp_agent/config.py (4)
LoggerSettings(535-580)MCPSettings(116-124)MCPServerSettings(52-113)find_secrets(647-649)src/mcp_agent/elicitation/handler.py (1)
console_elicitation_callback(138-167)src/mcp_agent/mcp/gen_client.py (1)
gen_client(17-49)src/mcp_agent/mcp/mcp_agent_client_session.py (1)
MCPAgentClientSession(73-428)src/mcp_agent/human_input/console_handler.py (1)
console_input_callback(68-78)
src/mcp_agent/data/examples/mcp_agent_server/reference/client.py (1)
src/mcp_agent/human_input/console_handler.py (1)
console_input_callback(68-78)
src/mcp_agent/data/examples/mcp_agent_server/elicitation/server.py (1)
src/mcp_agent/human_input/console_handler.py (1)
console_input_callback(68-78)
examples/human_input/temporal/worker.py (2)
src/mcp_agent/executor/temporal/__init__.py (1)
create_temporal_worker_for_app(498-554)examples/human_input/temporal/main.py (1)
main(67-79)
examples/mcp_agent_server/asyncio/client.py (1)
src/mcp_agent/human_input/console_handler.py (1)
console_input_callback(68-78)
examples/mcp/mcp_elicitation/temporal/client.py (1)
src/mcp_agent/human_input/console_handler.py (1)
console_input_callback(68-78)
examples/human_input/temporal/main.py (3)
src/mcp_agent/server/app_server.py (1)
app(135-137)src/mcp_agent/human_input/elicitation_handler.py (1)
elicitation_input_callback(53-123)src/mcp_agent/core/context.py (1)
Context(57-103)
examples/mcp_agent_server/temporal/main.py (1)
src/mcp_agent/human_input/console_handler.py (1)
console_input_callback(68-78)
src/mcp_agent/human_input/elicitation_handler.py (4)
src/mcp_agent/core/context.py (2)
mcp(102-103)get_current_context(270-300)src/mcp_agent/human_input/types.py (2)
HumanInputRequest(7-29)HumanInputResponse(32-42)src/mcp_agent/logging/logger.py (4)
get_logger(513-541)warning(281-289)debug(261-269)error(291-299)src/mcp_agent/executor/temporal/session_proxy.py (1)
elicit(324-340)
🪛 markdownlint-cli2 (0.17.2)
examples/human_input/temporal/README.md
30-30: Bare URL used
(MD034, no-bare-urls)
🔇 Additional comments (25)
src/mcp_agent/data/examples/mcp_agent_server/elicitation/client.py (1)
19-19: LGTM! Import path correctly updated.The import path change from
mcp_agent.human_input.handlertomcp_agent.human_input.console_handleraligns with the PR's refactoring to clarify handler names.src/mcp_agent/data/examples/mcp_agent_server/elicitation/server.py (1)
18-18: LGTM! Consistent import path update.The import path change maintains consistency with the client.py update and the broader refactoring effort.
examples/human_input/temporal/mcp_agent.secrets.yaml.example (1)
1-7: LGTM! Standard secrets template structure.The example secrets file follows the expected schema reference pattern and provides placeholder API keys for both OpenAI and Anthropic providers, matching the pattern used across other examples in the repository.
examples/human_input/temporal/requirements.txt (1)
1-7: LGTM! Well-organized dependencies.The requirements file is properly structured with:
- Clear separation between core and example-specific dependencies
- All necessary dependencies for the Temporal-based human input example
- Helpful comments to distinguish dependency categories
README.md (3)
571-571: LGTM! Import path updated correctly.The import statement has been properly updated to use the new
console_handlermodule path.
577-580: Verify the indentation change impact on the prompt formatting.The
FLY_AIR_AGENT_PROMPT.format(...)block indentation has changed within the triple-quoted string. While this is likely intentional for formatting consistency, the indentation change will affect the actual content of the instruction string passed to the agent.Could you confirm that the indentation change in the instruction string is intentional and won't affect the agent's behavior? The prompt content will now have different whitespace formatting.
589-589: LGTM! Consistent formatting improvement.The extra space after the comment improves readability and maintains consistency with Python style guidelines.
src/mcp_agent/human_input/elicitation_handler.py (3)
10-16: LGTM! Clean message construction logic.The helper function correctly builds the elicitation message with optional description prepending.
53-124: Well-structured elicitation handler with proper error handling.The main callback function is well-implemented with:
- Proper context and session validation
- Comprehensive logging at debug level
- Clear error messages for different failure scenarios
- Appropriate exception handling and re-raising
86-98: Elicitation schema is MCP‑compliant — verify no sensitive dataThe hardcoded requestedSchema (type: "object" with a primitive string property "response" and required ["response"]) matches MCP elicitation requirements.
- Optional: add a "title" and any tighter constraints (format/minLength/maxLength) for better UI/validation.
- Confirm the server is not requesting sensitive data and that clients advertise the "elicitation" capability.
Location: src/mcp_agent/human_input/elicitation_handler.py (lines 86–98).
tests/human_input/test_elicitation_handler.py (1)
1-160: LGTM - Comprehensive test coverage with solid implementation.The test suite provides excellent coverage for the elicitation handler with proper mocking patterns. The tests validate all key scenarios including success paths, error conditions, and edge cases. The usage of
pytest.MonkeyPatch.context()is appropriate for limiting the patching scope, which is especially important when patching core import functionality.Key strengths:
- Tests cover all three helper functions thoroughly
- Async test patterns are properly implemented with
@pytest.mark.asyncio- Mock objects are correctly configured with proper specs
- Error scenarios include appropriate exception matching with
matchparameter- Context patching correctly isolates the
get_current_contextimportThe test validates the integration between the elicitation system and the human input callback mechanism, ensuring the new functionality works correctly with the existing MCP architecture.
src/mcp_agent/data/examples/mcp_agent_server/reference/client.py (1)
22-22: Import path migration is consistent and correct.The update to import
console_input_callbackfrommcp_agent.human_input.console_handlerinstead ofmcp_agent.human_input.handleraligns with the repository-wide refactoring. The function signature and usage remain unchanged, ensuring backward compatibility while improving code organization.examples/mcp_agent_server/asyncio/client.py (1)
15-15: Consistent import path update - well executed.The import path change from
mcp_agent.human_input.handlertomcp_agent.human_input.console_handleris part of the broader refactoring effort across the codebase. Function usage and behavior remain unchanged.src/mcp_agent/data/examples/mcp_agent_server/reference/server.py (1)
28-28: Import migration completed correctly.The import path update maintains consistency across the example files while preserving all existing functionality. The change from
mcp_agent.human_input.handlertomcp_agent.human_input.console_handleris well-executed and aligns with the project's module restructuring.examples/mcp_agent_server/asyncio/main.py (1)
28-28: Import path updated correctly - maintains full functionality.The change to import
console_input_callbackfrommcp_agent.human_input.console_handleris consistent with the repository-wide migration. All downstream usage patterns remain intact, ensuring no breaking changes to the example functionality.examples/workflows/workflow_swarm/main.py (1)
8-8: Import path consistently updated across workflow examples.The migration to
mcp_agent.human_input.console_handlerfor theconsole_input_callbackimport is properly applied. The usage throughout the swarm agents (lines 11, 120, 140, 160, 180, 199) remains unchanged, preserving the existing human input functionality.examples/mcp/mcp_elicitation/main.py (1)
6-6: Import refactoring applied consistently.The update to import
console_input_callbackfrommcp_agent.human_input.console_handlerinstead of the previoushandlermodule maintains consistency with the broader codebase changes. The function usage in the MCPApp initialization (line 13) remains unchanged.examples/usecases/mcp_realtor_agent/main.py (1)
15-15: Import migration successfully applied to realtor example.The change from
mcp_agent.human_input.handlertomcp_agent.human_input.console_handlerfor theconsole_input_callbackimport is consistent with the repository-wide refactoring. The usage in the MCPApp initialization (line 33) and throughout the application remains unchanged, maintaining the existing human input capabilities for the real estate analysis workflow.examples/mcp_agent_server/temporal/main.py (1)
24-24: LGTM!The import path update to
mcp_agent.human_input.console_handleris correct and aligns with the PR-wide refactoring to rename the handler module for clarity.examples/mcp/mcp_elicitation/temporal/client.py (1)
17-17: LGTM!The import path update to
mcp_agent.human_input.console_handlercorrectly aligns with the PR-wide module renaming.examples/human_input/temporal/worker.py (1)
26-27: LGTM!The worker implementation correctly uses the
create_temporal_worker_for_apputility, and the structure follows the established pattern for Temporal worker scripts.examples/human_input/temporal/mcp_agent.config.yaml (1)
1-22: LGTM!The configuration file is well-structured and includes appropriate settings for Temporal execution, logging, and model selection. The practice of separating secrets into a separate file is commendable for security.
examples/human_input/temporal/main.py (3)
30-64: LGTM!The
greettool implementation follows the established pattern for workflow tools in the codebase. The agent setup, LLM attachment, and logging are all handled correctly.
67-79: LGTM!The main function correctly sets up the MCP server for the app, logs registered workflows, and runs the server using SSE transport.
45-45: Fix incorrect variable reference.The variable
app_ctxcontains the context, not the app. The app should be accessed viaapp_ctx.app.- app = app_ctx.app + if app_ctx is None: + raise ValueError("Context is required for greet tool") + app = app_ctx.app + if app is None: + raise ValueError("App not found in context")Likely an incorrect or invalid review comment.
| Notice how in `main.py` the `human_input_callback` is set to `elicitation_input_callback`. | ||
| This makes sure that human input is sought via elicitation. | ||
| In `client.py`, on the other hand, it is set to `console_elicitation_callback`. | ||
| This way, the client will prompt for input in the console whenever an upstream request for human input is made. |
There was a problem hiding this comment.
🧩 Analysis chain
Verify documentation consistency with implementation.
The README states that main.py uses elicitation_input_callback (Line 64) and client.py uses console_elicitation_callback (Line 66). Let me verify this matches the actual implementation.
🏁 Script executed:
#!/bin/bash
# Check the actual callback configurations in main.py
echo "=== Checking main.py for human_input_callback ==="
rg -n "human_input_callback\s*=" examples/human_input/temporal/main.py
# Check if elicitation_input_callback is imported in main.py
echo -e "\n=== Checking main.py for elicitation_input_callback import ==="
rg -n "from.*elicitation_handler.*import.*elicitation_input_callback" examples/human_input/temporal/main.py
# Check the actual callback configurations in client.py
echo -e "\n=== Checking client.py for callbacks ==="
rg -n "(human_input_callback|elicitation_callback)\s*=" examples/human_input/temporal/client.pyLength of output: 478
Update README: reflect actual callbacks in client.py
main.py uses elicitation_input_callback (examples/human_input/temporal/main.py:11,25); client.py sets human_input_callback=console_input_callback and elicitation_callback=console_elicitation_callback (examples/human_input/temporal/client.py:79–80). Update README (examples/human_input/temporal/README.md lines 64–67) to state this distinction.
🤖 Prompt for AI Agents
In examples/human_input/temporal/README.md around lines 64 to 67, the README
incorrectly states the callbacks: update the text to accurately reflect that
main.py sets human_input_callback=elicitation_input_callback (lines 11,25 of
main.py), while client.py sets human_input_callback=console_input_callback and
elicitation_callback=console_elicitation_callback (lines ~79–80 of client.py);
edit the paragraph to clearly state this distinction so the README matches the
actual callbacks used in the code.
2 participants
human_input/elicitation_handler.py, a human interaction handler that uses MCP elicitation to request user input. This builds on the existing elicitation handling.examples/human_input/temporalthat shows how to use it in an MCP app, including a README with more details.human_input/handler.pytohuman_input/console_handler.pyfor claritySummary by CodeRabbit
New Features
Documentation
Chores
Tests