fix(mcp): store empty dict when MCP prompt load fails - #693
Conversation
MCPClient._server_prompts is typed Dict[str, Dict[str, Prompt]] and every
accessor (get_server_prompts, get_all_prompts, get_prompt_names,
get_all_prompt_names, get_prompt_metadata) calls dict methods (.values(),
.keys(), .get()) on the stored value.
_load_prompts_from_session runs on every server connect right after
list_prompts(). MCP servers that do not implement the prompts capability
make list_prompts() raise, and the except branch stored a list ([]) instead
of a dict, so any subsequent prompt lookup raised
"AttributeError: 'list' object has no attribute 'values'". Because
get_all_prompts()/get_all_prompt_names() iterate every server, a single
prompt-less server poisoned the aggregate call for all servers.
Store {} to match the declared type and the success path. Add regression
tests covering the failure and success paths.
Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com>
|
The |
There was a problem hiding this comment.
Pull request overview
Fixes a container-type bug in MCPClient prompt loading so prompt accessors remain safe when an MCP server does not support the prompts capability (or prompt loading otherwise fails). This aligns the failure-path storage with the declared _server_prompts: Dict[str, Dict[str, Prompt]] type and prevents AttributeError in aggregate prompt lookups.
Changes:
- Store
{}(empty dict) instead of[](list) in_server_prompts[server_name]whenlist_prompts()fails. - Add regression tests covering both prompt-loading failure and success paths for the prompt accessors.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
dapr_agents/tool/mcp/client.py |
Fixes prompt-load failure path to store an empty mapping (dict) so .values()/.keys()/.get() accessors remain valid. |
tests/tool/test_mcp_client.py |
Adds async regression tests to ensure prompt accessors behave correctly on both failure and success prompt-loading paths. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
sicoyle
left a comment
sicoyle
left a comment
There was a problem hiding this comment.
amazing, thank you!!! 🚀
Description
MCPClient._server_promptsis a dictionary of per-server prompt dictionaries, but the prompt-loading failure path stored a list. Prompt accessors then called.values(),.keys(), or.get()on that list and raisedAttributeError. A server without prompt support could also break aggregate prompt lookups across all servers.This changes the failure value from
[]to{}, matching the declared type and success path. Regression tests cover prompt-loading failure and success paths.Issue reference
No issue. This is a small, self-contained bug fix discovered while reviewing the MCP client.
Checklist
No documentation PR is needed because this fixes an internal container type without changing the public API, configuration, or documented behavior.
AGENTS.md Notes