Skip to content

Fix MCPServer error handling with Temporal #3299

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 9 commits into from
Oct 31, 2025
Merged

Fix MCPServer error handling with Temporal #3299

merged 9 commits into from
Oct 31, 2025

Conversation

@wreed4
Copy link
Contributor

@wreed4 wreed4 commented Oct 30, 2025 •
edited
Loading

fixes #3298

@wreed4 wreed4 mentioned this pull request Oct 30, 2025
Closed
2 tasks
@wreed4
Copy link
Contributor Author

wreed4 commented Oct 30, 2025

this actually just fails the workflow.. so no good.

@wreed4 wreed4 closed this Oct 30, 2025
@wreed4 wreed4 reopened this Oct 30, 2025
@wreed4 wreed4 changed the title fix(3298): add ModelRetry error as non-retryable to return control back to agent on tool failure fix(3298): Control not being returned to the agent on ModelRetry exception in temporal worker Oct 30, 2025
DouweM
DouweM requested changes Oct 30, 2025
View reviewed changes
pydantic_ai_slim/pydantic_ai/durable_exec/temporal/_mcp_server.py Outdated
self.tool_for_tool_def(params.tool_def),
)
return _ToolReturn(result=result)
except ApprovalRequired:
Copy link
Collaborator

@DouweM DouweM Oct 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please move this logic and the types to TemporalWrapperToolset, so that both TemporalFunctionToolset and TemporalMCPServer can use them?

@DouweM DouweM self-assigned this Oct 30, 2025
@wreed4
Copy link
Contributor Author

wreed4 commented Oct 31, 2025

@DouweM I've made the changes.. I'm not sure why coverage is failing.. running it locally (which I might be doing wrong..) it seems like I have 100% coverage. I'm not actually sure where the tests for this file live either though..

@wreed4 wreed4 requested a review from DouweM October 31, 2025 14:14
@wreed4
Copy link
Contributor Author

wreed4 commented Oct 31, 2025

unfortunately... now I'm getting this

    raise UnexpectedModelBehavior(f'Tool {name!r} exceeded max retries count of {max_retries}') from e
pydantic_ai.exceptions.UnexpectedModelBehavior: Tool 'fetch' exceeded max retries count of 1

that doesn't seem expected either...

@wreed4
Copy link
Contributor Author

wreed4 commented Oct 31, 2025

well, maybe that is expected.. I get that when running the plain pydantic agent for this query as well.

@wreed4
Copy link
Contributor Author

wreed4 commented Oct 31, 2025

yeah that was just me holding it wrong. :) I think the PR is good, minus the failed coverage check that I'm not sure how to resolve.

@DouweM DouweM changed the title fix(3298): Control not being returned to the agent on ModelRetry exception in temporal worker Fix MCPServer error handling with Temporal Oct 31, 2025
@DouweM
Copy link
Collaborator

DouweM commented Oct 31, 2025

@wreed4 Thanks William, I made a couple of tweaks that should also resolve the coverage issue.

@DouweM DouweM enabled auto-merge (squash) October 31, 2025 17:19
@DouweM DouweM merged commit 9c31521 into pydantic:main Oct 31, 2025
30 checks passed
@wreed4 wreed4 deleted the fix-issue-3298 branch October 31, 2025 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@DouweM DouweM Awaiting requested review from DouweM

Assignees

@DouweM DouweM

Labels

awaiting author revision

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Control not being returned to the agent on ModelRetry exception in temporal worker

2 participants

@wreed4 @DouweM