Fix breaking type change in mcp 1.20.0 URL elicitation #623
Conversation
WalkthroughThis PR bumps the project version to 0.2.6 and updates the mcp dependency to >=1.20.0. It enhances elicitation parameter handling through safer attribute access, introduces Union-based type aliases for form and URL parameters, and implements pattern-matching-based parameter transformation to propagate Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 (beta)
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 |
![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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
pyproject.toml(2 hunks)src/mcp_agent/elicitation/handler.py(1 hunks)src/mcp_agent/elicitation/types.py(1 hunks)src/mcp_agent/mcp/mcp_agent_client_session.py(2 hunks)src/mcp_agent/server/app_server.py(4 hunks)src/mcp_agent/workflows/llm/augmented_llm.py(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-05-27T19:10:22.911Z
Learnt from: rholinshead
Repo: lastmile-ai/mcp-agent PR: 241
File: src/mcp_agent/mcp/mcp_agent_client_session.py:174-187
Timestamp: 2025-05-27T19:10:22.911Z
Learning: In the MCP Agent codebase, all request parameter types inherit from RequestParams or NotificationParams with consistent Meta class structures, so trace context injection using the base Meta classes doesn't cause type-mismatch issues.
Applied to files:
src/mcp_agent/elicitation/types.pysrc/mcp_agent/mcp/mcp_agent_client_session.py
📚 Learning: 2025-07-22T18:59:49.368Z
Learnt from: CR
Repo: lastmile-ai/mcp-agent PR: 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:
src/mcp_agent/workflows/llm/augmented_llm.pypyproject.tomlsrc/mcp_agent/mcp/mcp_agent_client_session.py
📚 Learning: 2025-08-28T15:07:10.015Z
Learnt from: saqadri
Repo: lastmile-ai/mcp-agent PR: 386
File: src/mcp_agent/mcp/mcp_server_registry.py:110-116
Timestamp: 2025-08-28T15:07:10.015Z
Learning: In MCP server registry methods, when client_session_factory parameters are updated to accept additional context parameters, ensure the type hints match what is actually passed (Context instance vs ServerSession) and that the default factory (MCPAgentClientSession) can handle the number of arguments being passed to avoid TypeError at runtime.
Applied to files:
src/mcp_agent/server/app_server.pysrc/mcp_agent/mcp/mcp_agent_client_session.py
📚 Learning: 2025-07-22T18:59:49.368Z
Learnt from: CR
Repo: lastmile-ai/mcp-agent PR: 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/mcp/mcp_agent_client_session.py
🧬 Code graph analysis (2)
src/mcp_agent/server/app_server.py (1)
src/mcp_agent/elicitation/types.py (2)
ElicitRequestFormParams(10-14)ElicitRequestURLParams(17-21)
src/mcp_agent/mcp/mcp_agent_client_session.py (1)
src/mcp_agent/elicitation/types.py (2)
ElicitRequestFormParams(10-14)ElicitRequestURLParams(17-21)
🔇 Additional comments (7)
pyproject.toml (2)
3-3: Version bump is appropriate for this patch release.Bumping from 0.2.5 to 0.2.6 aligns with semantic versioning for bug fixes.
21-21: Verify that implementation changes handle the mcp 1.20.0 ElicitRequestParams breaking change.Updating from
>=1.19.0to>=1.20.0is necessary to support the breaking type change in mcp. The AI summary indicates Union-based type aliases and pattern-matching parameter transformations have been added to handle this; however, the actual implementation files are not included in this review.Ensure that:
- All code paths handle
ElicitRequestParamsas a union type (form or URL variants) rather than a single type- Subclassing of the union member types is correctly implemented
- No runtime type errors occur when the new mcp version is deployed
src/mcp_agent/workflows/llm/augmented_llm.py (1)
34-34: LGTM!The
Toolimport with the# noqa: F401comment is a valid pattern for resolving forward references in Pydantic models. The comment clearly documents why this seemingly-unused import is necessary.src/mcp_agent/elicitation/handler.py (1)
91-94: LGTM - Safe attribute access for union type compatibility.The
getattr(request, "requestedSchema", None)pattern correctly handles the case whereElicitRequestParamscan now be either form or URL mode. The subsequent validation ensures the function fails gracefully with a clear error when called with URL-mode params.src/mcp_agent/elicitation/types.py (1)
10-25: Well-designed type hierarchy for the MCP 1.20 breaking change.The approach of subclassing both
MCPElicitRequestFormParamsandMCPElicitRequestURLParamsto add theserver_namefield, then exposing aUniontype alias, is a clean solution. This maintains type safety while accommodating the upstream breaking change.src/mcp_agent/server/app_server.py (2)
1049-1061: LGTM - Mode-based elicitation parameter construction.The branching logic correctly distinguishes between URL and form mode elicitation based on the
modeparameter, with a sensible default to"form"mode. This aligns with the new union type structure introduced in the elicitation types.
1172-1178: LGTM - Auth flow correctly uses form-based elicitation.The auth confirmation flow appropriately uses
ElicitRequestFormParamswith arequestedSchemaderived from theAuthTokenmodel, which is the correct type for form-based user input collection.
| # Convert MCP params to our subclass with server_name | ||
| elicitation_request: ( | ||
| AgentElicitRequestFormParams | AgentElicitRequestURLParams | ||
| ) | ||
| match params: | ||
| case MCPElicitRequestURLParams(): | ||
| elicitation_request = AgentElicitRequestURLParams( | ||
| message=params.message, | ||
| url=params.url, | ||
| elicitationId=params.elicitationId, | ||
| server_name=server_name, | ||
| ) | ||
| case MCPElicitRequestFormParams(): | ||
| elicitation_request = AgentElicitRequestFormParams( | ||
| message=params.message, | ||
| requestedSchema=params.requestedSchema, | ||
| server_name=server_name, | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check MCPElicitRequestFormParams definition
rg -n "class.*ElicitRequestFormParams" --type py -A 15Repository: lastmile-ai/mcp-agent
Length of output: 1284
🏁 Script executed:
# Check AgentElicitRequestFormParams definition
rg -n "class.*AgentElicitRequestFormParams" --type py -A 15Repository: lastmile-ai/mcp-agent
Length of output: 47
🏁 Script executed:
# Check if logger is imported in the file under review
rg -n "^import|^from" src/mcp_agent/mcp/mcp_agent_client_session.py | grep -i logRepository: lastmile-ai/mcp-agent
Length of output: 115
🏁 Script executed:
# Check the full context of lines 401-418 to understand the match statement better
sed -n '390,430p' src/mcp_agent/mcp/mcp_agent_client_session.pyRepository: lastmile-ai/mcp-agent
Length of output: 1814
🏁 Script executed:
# Find where MCPElicitRequestFormParams and MCPElicitRequestURLParams are imported from
rg -n "from mcp" src/mcp_agent/mcp/mcp_agent_client_session.py | head -20Repository: lastmile-ai/mcp-agent
Length of output: 769
🏁 Script executed:
# Check if there's any MCP types definition in the repo
fd "mcp.*types" --type fRepository: lastmile-ai/mcp-agent
Length of output: 47
🏁 Script executed:
# Look for MCP library documentation or check requirements
fd "requirements\|setup\|pyproject" --type f | head -10Repository: lastmile-ai/mcp-agent
Length of output: 47
🏁 Script executed:
# Check the local Agent types definition more thoroughly
cat -n src/mcp_agent/elicitation/types.pyRepository: lastmile-ai/mcp-agent
Length of output: 1625
🌐 Web query:
MCPElicitRequestFormParams MCPElicitRequestURLParams elicitationId field mcp types
💡 Result:
The elicitationId field is a string (typically a UUID). See the MCP elicitation spec and SDK examples showing elicitationId passed as a string/UUID in URL/form elicitation requests. [1][2]
Sources:
[1] Model Context Protocol — Elicitation (spec).
[2] modelcontextprotocol python/TypeScript SDK examples (elicit URL example using elicitationId as a UUID string).
🏁 Script executed:
# Search for any usage examples of these MCP types in tests or elsewhere
rg -n "MCPElicitRequestFormParams|MCPElicitRequestURLParams" --type py -B 2 -A 5Repository: lastmile-ai/mcp-agent
Length of output: 3683
🏁 Script executed:
# Check if there are any tests that show how these params are used
fd "test.*\.py" --type f | xargs rg -l "Elicit" 2>/dev/null | head -5Repository: lastmile-ai/mcp-agent
Length of output: 110
🏁 Script executed:
# Check the test file to see how elicitation is tested
cat -n tests/human_input/test_elicitation_handler.py | head -80Repository: lastmile-ai/mcp-agent
Length of output: 3758
🏁 Script executed:
# Check if AgentElicitRequestFormParams (or its parent) has elicitationId field
rg -n "class.*AgentElicitRequest" src/ -A 10Repository: lastmile-ai/mcp-agent
Length of output: 47
🏁 Script executed:
# Check the full match statement context and what comes after it
sed -n '400,430p' src/mcp_agent/mcp/mcp_agent_client_session.pyRepository: lastmile-ai/mcp-agent
Length of output: 1388
🏁 Script executed:
# Search more broadly for Agent* classes related to elicitation
rg -n "AgentElicit" src/ --type py -B 1 -A 5Repository: lastmile-ai/mcp-agent
Length of output: 2450
🏁 Script executed:
# Check what happens after the match statement - does it fail if no case matches?
sed -n '413,435p' src/mcp_agent/mcp/mcp_agent_client_session.pyRepository: lastmile-ai/mcp-agent
Length of output: 1016
🏁 Script executed:
# Look for the import of AgentElicitRequestFormParams and AgentElicitRequestURLParams
rg -n "from.*import.*AgentElicit" src/mcp_agent/mcp/mcp_agent_client_session.py -B 2 -A 2Repository: lastmile-ai/mcp-agent
Length of output: 47
Add missing elicitationId field to form params conversion.
The URL params conversion includes elicitationId (line 410), but the form params conversion omits it. According to the MCP elicitation spec, both MCPElicitRequestFormParams and MCPElicitRequestURLParams contain an elicitationId field that must be preserved when converting to the agent subclass.
case MCPElicitRequestFormParams():
elicitation_request = AgentElicitRequestFormParams(
message=params.message,
requestedSchema=params.requestedSchema,
+ elicitationId=params.elicitationId,
server_name=server_name,
)🤖 Prompt for AI Agents
In src/mcp_agent/mcp/mcp_agent_client_session.py around lines 401 to 418, the
form-params branch building AgentElicitRequestFormParams omits the elicitationId
field; add elicitationId=params.elicitationId to the
AgentElicitRequestFormParams constructor so the field is preserved when
converting MCPElicitRequestFormParams to the agent subclass, keeping parameter
naming consistent with the URL branch.
saqadri
left a comment
saqadri
left a comment
There was a problem hiding this comment.
Looks correct, thanks for fixing. Let's ship and fix forward from here
| SamplingMessage, | ||
| TextContent, | ||
| PromptMessage, | ||
| Tool, # noqa: F401 - Required to resolve forward reference in CreateMessageRequestParams |
There was a problem hiding this comment.
I get
pydantic.errors.PydanticUserError: `RequestParams` is not fully defined; you should define `Tool`, then call `RequestParams.model_rebuild()`
when running the elicitation example without this
| req = ElicitRequest( | ||
| method="elicitation/create", | ||
| params=ElicitRequestParams( | ||
| params=ElicitRequestFormParams( |
There was a problem hiding this comment.
@romanvanderkrogt should the auth flow now use url based elicitation?
Support MCP
ElicitRequestParamsas a union type by subclassing its two member types instead of the union itself.Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.