Skip to content

Fix #355: Fix type error with lifespan context#368

Merged
dsp-ant merged 3 commits into
mainfrom
fix-355-type-error-lifespan-context
Mar 26, 2025
Merged

Fix #355: Fix type error with lifespan context#368
dsp-ant merged 3 commits into
mainfrom
fix-355-type-error-lifespan-context

Conversation

@dsp-ant

@dsp-ant dsp-ant commented Mar 25, 2025

Copy link
Copy Markdown
Member

Summary

Fix type error when using strongly typed custom lifespan contexts with FastMCP.

  • The TypeVar was incorrectly defined with
  • This change removes the default, allowing proper type propagation
  • Fixed type annotations in relevant places to maintain type safety
  • Added test case demonstrating the fixed functionality

The TypeVar LifespanContextT was incorrectly defined with a default=None, which caused type errors when using strongly typed custom context objects. This change properly propagates the type information from custom lifespans to request contexts.
@dsp-ant
dsp-ant requested a review from Kludex March 25, 2025 12:11
Comment thread src/mcp/shared/context.py

SessionT = TypeVar("SessionT", bound=BaseSession[Any, Any, Any, Any, Any])
LifespanContextT = TypeVar("LifespanContextT", default=None)
LifespanContextT = TypeVar("LifespanContextT")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

But doesn't it makes sense to default to None on FastMCP context?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

FastMCP sets the default lifespan which yields and empty dictionary. There is actually never a None. This one defaults to Any, if default is not set, which I think is correct.

@dsp-ant
dsp-ant requested a review from Kludex March 25, 2025 15:38
@Kludex
Kludex enabled auto-merge (squash) March 26, 2025 08:11
@Kludex
Kludex disabled auto-merge March 26, 2025 08:11
@Kludex

Kludex commented Mar 26, 2025

Copy link
Copy Markdown
Member

This PR doesn't fix the typing tho

@dsp-ant

dsp-ant commented Mar 26, 2025

Copy link
Copy Markdown
Member Author

This PR doesn't fix the typing tho

I am confused, it very much fixes the issue for me

@dsp-ant
dsp-ant merged commit df2d3a5 into main Mar 26, 2025
@dsp-ant
dsp-ant deleted the fix-355-type-error-lifespan-context branch March 26, 2025 14:18
@Kludex

Kludex commented Mar 26, 2025

Copy link
Copy Markdown
Member

🤔

Screenshot 2025-03-26 at 15 23 56

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants