Conversation
Fixes type errors when upgrading to ty 0.0.5: - Add base_url validation to auth providers (AWS, Azure, Discord, GitHub, Google, WorkOS) - Add partial[] to tool/prompt decorator return types - Add type ignores for PydanticAdapter generic aliases (pending strawgate/py-key-value#250) - Add type ignores for Depends() in tests (type checker doesn't understand DI unwrapping)
Added type ignores for Pydantic field aliases (_meta) and MCP SDK optional fields that ty doesn't yet recognize. Also removed useless code_challenge_method parameters from tests that were being silently ignored by Pydantic. - Added `# type: ignore[call-arg]` with explanatory comments for _meta (Pydantic alias) - Added type ignores for MCP SDK optional fields (limit, resource_owner, jwt_signing_key, code_challenge_method) - Removed code_challenge_method from AuthorizationParams test instantiations (not a real field) - Upgraded ruff pre-commit hook to v0.14.10 - Enabled unknown-argument rule in ty (was previously ignored)
- Add elicit() overloads for dict/list response_type patterns - Use cast for server_auth_class instead of type ignore - Add basedpyright config for future comparison
- Parse createdAt ISO string to datetime in tasks/get handler - Use explicit type annotations instead of bare type ignores - Add explanatory comments to remaining type ignores
marvin-context-protocol
Bot
added
enhancement
WalkthroughAdds local typing relaxations and a few runtime validations: many call sites now use Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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 |
Test Failure AnalysisSummary: 37 tests failing across authentication provider test suites because they're missing the newly-required Root Cause: This PR (#2676) adds Suggested Solution: Update all failing tests to include a Example fix for def test_init_defaults(self):
"""Test that default values are applied correctly."""
provider = AzureProvider(
client_id="test_client",
client_secret="test_secret",
tenant_id="test-tenant",
required_scopes=["read"],
jwt_signing_key="test-secret",
base_url="https://example.com", # Add this line
)Apply similar changes to all test files:
Detailed AnalysisThe validation was added in this PR to satisfy ty 0.0.5 type checking: if not settings.base_url:
raise ValueError(
"base_url is required - set via parameter or FASTMCP_SERVER_AUTH_{PROVIDER}_BASE_URL"
)Changed files:
The validation is correct - OAuth proxy providers need Related FilesSource files with new validation (from this PR):
Test files needing updates:
Comment updated after re-analyzing workflow run 20445082974 |
![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
🧹 Nitpick comments (5)
src/fastmcp/server/auth/oauth_proxy.py (1)
1396-1401: Consider adding explanatory comments for consistency.The
type: ignore[assignment]annotations are appropriate here, as authlib'srefresh_token()andfetch_token()methods likely return more specific types thandict[str, Any]. However, for consistency with lines 1024 and 1107, consider adding brief explanatory comments.Suggested improvement for consistency
- token_response: dict[str, Any] = await oauth_client.refresh_token( # type: ignore[assignment] + token_response: dict[str, Any] = await oauth_client.refresh_token( # type: ignore[assignment] # authlib returns specific token type url=self._upstream_token_endpoint,idp_tokens: dict[str, Any] = await oauth_client.fetch_token( **token_params - ) # type: ignore[assignment] + ) # type: ignore[assignment] # authlib returns specific token typeAlso applies to: 1803-1805
src/fastmcp/server/tasks/requests.py (1)
10-10: AligncreatedAtparsing between get and cancel handlersThe new
Literal[...]-typedmcp_stateand robustcreated_at_dtparsing intasks_get_handlerlook good and defensively handle missing/invalid Redis values.
tasks_cancel_handlerstill callsdatetime.fromisoformat(created_at)directly, which can raise if the stored string has a"Z"suffix or is otherwise malformed, whereastasks_get_handlernow normalizes and falls back tonow(). For consistency and resilience, consider sharing the parsing logic (e.g., a small_parse_created_at(created_at: str | None) -> datetimehelper) and using it in both places.Possible helper extraction
+def _parse_created_at(created_at: str | None) -> datetime: + """Parse createdAt from Redis, falling back to current UTC time.""" + if created_at: + try: + return datetime.fromisoformat(created_at.replace("Z", "+00:00")) + except (ValueError, AttributeError): + pass + return datetime.now(timezone.utc) + @@ - # createdAt is required per spec, but can be None from Redis - # Parse ISO string to datetime, or use current time as fallback - if created_at: - try: - created_at_dt = datetime.fromisoformat( - created_at.replace("Z", "+00:00") - ) - except (ValueError, AttributeError): - created_at_dt = datetime.now(timezone.utc) - else: - created_at_dt = datetime.now(timezone.utc) + created_at_dt = _parse_created_at(created_at) @@ - createdAt=datetime.fromisoformat(created_at) - if created_at - else datetime.now(timezone.utc), + createdAt=_parse_created_at(created_at),Also applies to: 148-151, 168-178, 416-420
src/fastmcp/server/server.py (1)
2472-2477: HTTP path logging hardening is correct; consider reusingserver_pathUsing
hasattr/getattr(app.state, "path", "")avoids attribute errors and ensures a sane default for logging. There’s a tiny duplication betweenserver_pathand the laterpath = getattr(app.state, "path", "").lstrip("/")that could be collapsed to one variable, but that’s purely cosmetic.Also applies to: 2504-2507
src/fastmcp/server/context.py (1)
883-904:elicitoverloads comprehensively cover new dict/list patternsThe additional overloads for:
dict[str, dict[str, str]](single-select with titles),list[list[str]](multi-select),list[dict[str, dict[str, str]]](multi-select with titles),plus the widened concrete signature, align with the documented return types (
strorlist[str]as appropriate) and still funnel throughparse_elicit_response_type. This gives callers much better type information for the supported response patterns without changing control flow.You might later expand the main docstring to briefly mention these dict/list selection shapes so the high-level docs match the overload comments.
Also applies to: 905-914, 915-921, 925-948, 950-966
src/fastmcp/client/client.py (1)
1576-1576: Approved with minor suggestion for comment clarity.The targeted type ignore with specific error code
[call-arg]is appropriate for working around upstream MCP SDK type signature issues. However, the comment "Optional field in MCP SDK" is slightly ambiguous since two parameters are being passed.Consider clarifying which parameter(s) cause the type error, for example:
params = PaginatedRequestParams(cursor=cursor, limit=limit) # type: ignore[call-arg] # cursor and limit are optional in MCP SDK but ty requires explicit handlingThis helps future maintainers understand exactly what type issue is being suppressed.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (9)
.pre-commit-config.yamlis excluded by none and included by nonepyproject.tomlis excluded by none and included by nonetests/client/test_elicitation.pyis excluded by none and included by nonetests/server/auth/test_oauth_consent_flow.pyis excluded by none and included by nonetests/server/auth/test_oauth_proxy.pyis excluded by none and included by nonetests/server/tasks/test_task_dependencies.pyis excluded by none and included by nonetests/server/test_dependencies.pyis excluded by none and included by nonetests/utilities/test_components.pyis excluded by none and included by noneuv.lockis excluded by!**/*.lockand included by none
📒 Files selected for processing (20)
src/fastmcp/client/client.pysrc/fastmcp/client/elicitation.pysrc/fastmcp/client/tasks.pysrc/fastmcp/prompts/prompt.pysrc/fastmcp/resources/resource.pysrc/fastmcp/resources/template.pysrc/fastmcp/server/auth/oauth_proxy.pysrc/fastmcp/server/auth/providers/aws.pysrc/fastmcp/server/auth/providers/azure.pysrc/fastmcp/server/auth/providers/discord.pysrc/fastmcp/server/auth/providers/github.pysrc/fastmcp/server/auth/providers/google.pysrc/fastmcp/server/auth/providers/workos.pysrc/fastmcp/server/context.pysrc/fastmcp/server/dependencies.pysrc/fastmcp/server/middleware/caching.pysrc/fastmcp/server/server.pysrc/fastmcp/server/tasks/handlers.pysrc/fastmcp/server/tasks/requests.pysrc/fastmcp/tools/tool.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Use Python ≥ 3.10 with full type annotations
Never use bareexcept- be specific with exception types
Files:
src/fastmcp/resources/template.pysrc/fastmcp/resources/resource.pysrc/fastmcp/tools/tool.pysrc/fastmcp/server/auth/providers/workos.pysrc/fastmcp/client/elicitation.pysrc/fastmcp/client/client.pysrc/fastmcp/server/auth/providers/azure.pysrc/fastmcp/server/middleware/caching.pysrc/fastmcp/prompts/prompt.pysrc/fastmcp/server/auth/providers/github.pysrc/fastmcp/client/tasks.pysrc/fastmcp/server/tasks/handlers.pysrc/fastmcp/server/server.pysrc/fastmcp/server/auth/providers/aws.pysrc/fastmcp/server/auth/oauth_proxy.pysrc/fastmcp/server/auth/providers/discord.pysrc/fastmcp/server/context.pysrc/fastmcp/server/tasks/requests.pysrc/fastmcp/server/auth/providers/google.pysrc/fastmcp/server/dependencies.py
🧠 Learnings (5)
📚 Learning: 2025-11-26T21:51:44.174Z
Learnt from: CR
Repo: jlowin/fastmcp PR: 0
File: .cursor/rules/core-mcp-objects.mdc:0-0
Timestamp: 2025-11-26T21:51:44.174Z
Learning: Review and update related Manager classes (ToolManager, ResourceManager, PromptManager) when modifying MCP object definitions
Applied to files:
src/fastmcp/prompts/prompt.py
📚 Learning: 2025-12-21T21:37:55.015Z
Learnt from: CR
Repo: jlowin/fastmcp PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-21T21:37:55.015Z
Learning: When modifying MCP functionality (Tools, Resources, Resource Templates, Prompts), changes typically need to be applied across all object types
Applied to files:
src/fastmcp/prompts/prompt.py
📚 Learning: 2025-12-21T21:37:55.015Z
Learnt from: CR
Repo: jlowin/fastmcp PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-21T21:37:55.015Z
Learning: Applies to src/fastmcp/**/__init__.py : Core types that define a module's purpose should be exported (e.g., `Middleware` from `fastmcp.server.middleware`), while specialized features can live in submodules
Applied to files:
src/fastmcp/server/server.py
📚 Learning: 2025-12-01T15:48:05.095Z
Learnt from: jlowin
Repo: jlowin/fastmcp PR: 0
File: :0-0
Timestamp: 2025-12-01T15:48:05.095Z
Learning: PR #2505 in fastmcp adds NEW functionality to get_access_token(): it now first checks request.scope["user"] for the token (which never existed before), then falls back to _sdk_get_access_token() (the only thing the original code did). This is not a reversal of order but entirely new functionality to fix stale token issues.
Applied to files:
src/fastmcp/server/dependencies.py
📚 Learning: 2025-12-01T15:53:29.709Z
Learnt from: jlowin
Repo: jlowin/fastmcp PR: 2505
File: src/fastmcp/server/dependencies.py:9-9
Timestamp: 2025-12-01T15:53:29.709Z
Learning: In the MCP SDK (mcp.server.auth.middleware.bearer_auth), the AuthenticatedUser class has an `access_token` attribute (not `token`) that returns an AccessToken object. To get the raw token string, you would use `user.access_token.token`.
Applied to files:
src/fastmcp/server/dependencies.py
🧬 Code graph analysis (4)
src/fastmcp/server/server.py (2)
src/fastmcp/settings.py (1)
server_auth_class(396-412)src/fastmcp/server/providers/proxy.py (1)
ProxyClient(683-717)
src/fastmcp/server/auth/oauth_proxy.py (1)
src/fastmcp/server/context.py (1)
error(473-487)
src/fastmcp/server/context.py (2)
src/fastmcp/server/dependencies.py (1)
message(426-427)src/fastmcp/server/elicitation.py (1)
AcceptedElicitation(105-109)
src/fastmcp/server/tasks/requests.py (2)
src/fastmcp/client/tasks.py (1)
status(171-199)src/fastmcp/client/client.py (1)
CallToolResult(117-124)
🪛 Ruff (0.14.10)
src/fastmcp/server/auth/providers/workos.py
238-240: Avoid specifying long messages outside the exception class
(TRY003)
src/fastmcp/server/auth/providers/github.py
266-268: Avoid specifying long messages outside the exception class
(TRY003)
src/fastmcp/server/auth/providers/aws.py
201-203: Avoid specifying long messages outside the exception class
(TRY003)
src/fastmcp/server/auth/providers/discord.py
271-273: Avoid specifying long messages outside the exception class
(TRY003)
src/fastmcp/server/auth/providers/google.py
290-292: Avoid specifying long messages outside the exception class
(TRY003)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run tests: Python 3.10 on windows-latest
🔇 Additional comments (24)
src/fastmcp/server/auth/providers/google.py (1)
289-292: Base URL validation is correctly implemented.The validation ensures
base_urlis provided before initializing the OAuth proxy, preventing type errors and providing clear feedback to users. The error message appropriately guides users to set the value via parameter or environment variable.src/fastmcp/server/auth/providers/workos.py (1)
237-240: Base URL validation is correctly implemented.The validation ensures
base_urlis provided before initializing the OAuth proxy. The error message is clear and consistent with other providers in this PR.src/fastmcp/server/auth/providers/azure.py (1)
236-240: Base URL validation is correctly implemented.The validation follows the established pattern in this file of using a
msgvariable for error messages. The check appropriately ensuresbase_urlis provided before proceeding with OAuth proxy initialization.src/fastmcp/server/auth/providers/aws.py (1)
200-203: Base URL validation is correctly implemented.The validation ensures
base_urlis provided before initializing the OIDC proxy. The error message correctly includesCOGNITOin the environment variable name, matching the provider's configuration prefix.src/fastmcp/server/auth/providers/discord.py (1)
270-273: Base URL validation is correctly implemented.The validation ensures
base_urlis provided before initializing the OAuth proxy. The implementation is consistent with other providers in this PR.src/fastmcp/server/auth/providers/github.py (1)
265-268: Base URL validation is correctly implemented.The validation ensures
base_urlis provided before initializing the OAuth proxy. This completes the consistent base_url validation pattern across all OAuth providers in this PR, improving type safety and providing clear error messages when configuration is missing.src/fastmcp/client/elicitation.py (1)
68-72: LGTM!The
type: ignore[call-arg]annotation is correctly scoped and the explanatory comment clearly documents that_metais a Pydantic alias for themetafield. This is a valid pattern for handling Pydantic's field aliasing with strict type checkers.src/fastmcp/tools/tool.py (2)
116-120: LGTM!The
type: ignore[call-arg]is correctly scoped with an explanatory comment. Consistent with the pattern used throughout the PR for handling Pydantic's_metaalias.
193-196: LGTM!The type ignore is properly applied to the
_metaparameter with the correctcall-argcode and explanatory comment.src/fastmcp/resources/resource.py (2)
121-134: LGTM!Both
TextResourceContentsandBlobResourceContentsconstructions correctly applytype: ignore[call-arg]with explanatory comments for the_metaPydantic alias.
293-296: LGTM!The
type: ignore[call-arg]is properly applied to theSDKResourceconstruction with a clear comment explaining the Pydantic alias behavior.src/fastmcp/server/tasks/handlers.py (1)
95-104: LGTM!The
type: ignore[call-arg]annotation is correctly applied to theJSONRPCNotificationconstruction with an explanatory comment. The_metastructure for task-related notifications is properly formatted per SEP-1686 spec.src/fastmcp/prompts/prompt.py (2)
109-115: LGTM!The
type: ignore[call-arg]is correctly scoped with an explanatory comment for theGetPromptResultconstruction.
157-166: LGTM!The type ignore is properly applied to the
SDKPromptconstruction with a clear comment. Based on learnings, this change is correctly applied across all MCP object types (Tools, Resources, and Prompts) in the PR.src/fastmcp/server/middleware/caching.py (1)
197-202: ScopedPydanticAdaptertype ignores look goodThe added comment plus
# type: ignore[arg-type]onpydantic_modelare narrowly scoped and keep runtime semantics unchanged while unblocking stricter type checking. Nothing else to fix here.Also applies to: 205-208, 211-214, 217-222, 225-228, 231-234
src/fastmcp/resources/template.py (1)
240-251:_metaoverride handling and typing clarification are correctUsing
overrides.get("_meta", self.get_meta(...))preserves existing defaulting while allowing explicit_metaoverrides, and the targeted# type: ignore[call-arg]is appropriate for the Pydantic alias. Looks solid.src/fastmcp/server/dependencies.py (1)
633-642: Narrowresource_ownertype-ignore is appropriateThe explicit
# type: ignore[call-arg]onresource_ownermatches it being optional in the SDK and keeps the conversion logic unchanged. No further changes needed.src/fastmcp/server/tasks/requests.py (1)
272-277:_metapropagation for task-related MCP results is consistentThe added
_meta=...arguments onCallToolResultandReadResourceResult, with clear# type: ignore[call-arg]notes about the Pydantic alias, nicely standardize how related-task metadata is attached across error, tuple, and plain-content branches. No issues here.Also applies to: 300-305, 307-310, 324-336
src/fastmcp/server/server.py (4)
246-257: Auth provider instantiation viaserver_auth_classcast is soundCasting
fastmcp.settings.server_auth_classtotype[AuthProvider]and instantiating it whenauth is NotSetkeeps behavior (lazy construction from settings) while avoiding call-non-callable issues from the property’s annotation. Guarding onserver_auth is not Noneensures this only runs when configured.
1850-1869: Expandedtooldecorator return type matches actual usage patternsIncluding
partial[Callable[[AnyFunction], FunctionTool] | FunctionTool]in the return type accurately models the decorator-usage cases where a partially applied function is returned. This should make stricter type checkers happier without changing runtime behavior.
2239-2255:promptdecorator return type union is now aligned with real-world useMirroring the
tooldecorator, the prompt decorator’s return type now covers both directFunctionPromptreturns and partially applied callables. This improves typing fidelity and doesn’t affect execution.
2911-2918:as_proxybackend handling viaProxyClient(cast(Any, backend))is reasonableFor non-
Clientbackends, wrapping the transport inProxyClientand then using.new()for fresh sessions matches the intended API, and thecast(Any, backend)pragmatically sidesteps complex union typing without changing runtime behavior.src/fastmcp/server/context.py (1)
675-685:tool_resultstyping with a focused ignore is acceptableAnnotating
tool_resultsaslist[SamplingMessageContentBlock]and using# type: ignore[assignment]keeps the local type precise while deferring to the upstreamrun_sampling_toolssignature. Given the downstream usage (content=tool_results), this is a safe and well-scoped typing workaround.Also applies to: 689-692
src/fastmcp/client/tasks.py (1)
376-383: Legacy ToolResult →CallToolResultconversion_metahandling is consistentPassing
_meta=raw_result.metawith a# type: ignore[call-arg]matches the Pydantic alias behavior and keeps legacy ToolResult conversions aligned with how server-side code attaches metadata.
| error="invalid_client", # type: ignore[arg-type] # "invalid_client" is valid OAuth error but not in Literal type | ||
| error_description="Client ID is required", | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the oauth_proxy.py file and examine lines around 1024, 1107, 1396, 1805
git ls-files | grep oauth_proxy.pyRepository: jlowin/fastmcp
Length of output: 134
🏁 Script executed:
#!/bin/bash
# Search for AuthorizeError class definition
rg -n "class AuthorizeError" --type=pyRepository: jlowin/fastmcp
Length of output: 40
🏁 Script executed:
#!/bin/bash
# Examine lines 1020-1030 to see the context
sed -n '1020,1030p' src/fastmcp/server/auth/oauth_proxy.py | cat -nRepository: jlowin/fastmcp
Length of output: 636
🏁 Script executed:
#!/bin/bash
# Examine lines 1100-1110 to see the context
sed -n '1100,1110p' src/fastmcp/server/auth/oauth_proxy.py | cat -nRepository: jlowin/fastmcp
Length of output: 579
🏁 Script executed:
#!/bin/bash
# Examine lines 1390-1400 to see if there are comments
sed -n '1390,1400p' src/fastmcp/server/auth/oauth_proxy.py | cat -nRepository: jlowin/fastmcp
Length of output: 781
🏁 Script executed:
#!/bin/bash
# Examine lines 1800-1810 to see if there are comments
sed -n '1800,1810p' src/fastmcp/server/auth/oauth_proxy.py | cat -nRepository: jlowin/fastmcp
Length of output: 553
🏁 Script executed:
#!/bin/bash
# Look for the AuthorizeError class definition
rg -n "^class AuthorizeError" src/fastmcp/server/auth/oauth_proxy.py -A 20Repository: jlowin/fastmcp
Length of output: 40
🏁 Script executed:
#!/bin/bash
# Search for where error parameter is typed in AuthorizeError
rg -n "class AuthorizeError" -A 30 --type=pyRepository: jlowin/fastmcp
Length of output: 40
🏁 Script executed:
#!/bin/bash
# Search for AuthorizeError class definition across the entire codebase
rg -n "class AuthorizeError" --type=pyRepository: jlowin/fastmcp
Length of output: 40
🏁 Script executed:
#!/bin/bash
# Search for AuthorizeError imports in oauth_proxy.py
rg -n "AuthorizeError" src/fastmcp/server/auth/oauth_proxy.py | head -5Repository: jlowin/fastmcp
Length of output: 158
🏁 Script executed:
#!/bin/bash
# Look at the imports section of oauth_proxy.py
head -100 src/fastmcp/server/auth/oauth_proxy.py | grep -n "import\|from"Repository: jlowin/fastmcp
Length of output: 1534
🏁 Script executed:
#!/bin/bash
# Find the AuthorizeError definition in mcp module
find . -name "*.py" -type f -exec grep -l "class AuthorizeError" {} \;Repository: jlowin/fastmcp
Length of output: 40
🏁 Script executed:
#!/bin/bash
# Check the imports more carefully
sed -n '39,50p' src/fastmcp/server/auth/oauth_proxy.py | cat -nRepository: jlowin/fastmcp
Length of output: 396
🏁 Script executed:
#!/bin/bash
# Search for AuthorizeError definition in installed packages or local code
python3 << 'EOF'
import inspect
try:
from mcp.server.auth.provider import AuthorizeError
print(f"Location: {inspect.getfile(AuthorizeError)}")
print(f"\nSignature:")
print(inspect.signature(AuthorizeError.__init__))
print(f"\nSource:")
print(inspect.getsource(AuthorizeError))
except Exception as e:
print(f"Error: {e}")
EOFRepository: jlowin/fastmcp
Length of output: 144
🏁 Script executed:
#!/bin/bash
# Look for .pyi stub files or search for AuthorizeError in a different way
find . -name "*.pyi" -type f | xargs grep -l "AuthorizeError" 2>/dev/null || echo "No stub files found"
# Also search for the error parameter type definition
rg -n "error.*Literal" --type=py src/fastmcp/ | head -20Repository: jlowin/fastmcp
Length of output: 427
🏁 Script executed:
#!/bin/bash
# Get more context around lines 1396 and 1805 to understand what needs comments
echo "=== Lines 1390-1400 context ==="
sed -n '1390,1400p' src/fastmcp/server/auth/oauth_proxy.py
echo ""
echo "=== Lines 1800-1810 context ==="
sed -n '1800,1810p' src/fastmcp/server/auth/oauth_proxy.pyRepository: jlowin/fastmcp
Length of output: 1188
🏁 Script executed:
#!/bin/bash
# Check the type annotations for the oauth_client methods to understand the assignment type ignore
rg -n "oauth_client.*refresh_token\|oauth_client.*fetch_token" src/fastmcp/server/auth/oauth_proxy.py -B 2 -A 2Repository: jlowin/fastmcp
Length of output: 40
🏁 Script executed:
#!/bin/bash
# Get the full function signatures around those lines to understand context better
sed -n '1370,1410p' src/fastmcp/server/auth/oauth_proxy.py | cat -nRepository: jlowin/fastmcp
Length of output: 2356
🏁 Script executed:
#!/bin/bash
# Check what the PR description or commit message says about these changes
git log --oneline -20Repository: jlowin/fastmcp
Length of output: 109
🏁 Script executed:
#!/bin/bash
# Check the git diff or changes in this branch
git diff main src/fastmcp/server/auth/oauth_proxy.py | grep -A 3 -B 3 "type: ignore" | head -60Repository: jlowin/fastmcp
Length of output: 1828
🏁 Script executed:
#!/bin/bash
# Look at the git show for the recent changes
git show HEAD:src/fastmcp/server/auth/oauth_proxy.py | sed -n '1020,1030p'Repository: jlowin/fastmcp
Length of output: 559
🏁 Script executed:
#!/bin/bash
# Check what was changed - look at line 1396 and 1805 more carefully
echo "Line 1396 area:"
sed -n '1394,1398p' src/fastmcp/server/auth/oauth_proxy.py
echo ""
echo "Line 1805 area:"
sed -n '1803,1807p' src/fastmcp/server/auth/oauth_proxy.pyRepository: jlowin/fastmcp
Length of output: 594
🏁 Script executed:
#!/bin/bash
# Verify the actual lines in the current state to confirm the review comment is accurate
echo "=== Lines 1024-1026 (line numbers) ==="
sed -n '1024,1026p' src/fastmcp/server/auth/oauth_proxy.py
echo ""
echo "=== Lines 1107-1109 ==="
sed -n '1107,1109p' src/fastmcp/server/auth/oauth_proxy.py
echo ""
echo "=== Line 1396 (type: ignore[assignment] area) ==="
sed -n '1396,1396p' src/fastmcp/server/auth/oauth_proxy.py
echo ""
echo "=== Line 1805 (type: ignore[assignment] area) ==="
sed -n '1805,1805p' src/fastmcp/server/auth/oauth_proxy.pyRepository: jlowin/fastmcp
Length of output: 784
Add explanatory comments to lines 1396 and 1805 for consistency.
Lines 1024 and 1107 correctly use type: ignore[arg-type] with clear explanatory comments for the "invalid_client" error, which is documented in RFC 6749 Section 5.2. However, lines 1396 and 1805 have been updated to the more specific type: ignore[assignment] but lack explanatory comments. For consistency with the PR objective and the pattern established in the file, add comments explaining why the assignment type ignores are needed:
- Line 1396: Explain the return type mismatch for
oauth_client.refresh_token() - Line 1805: Explain the return type mismatch for
oauth_client.fetch_token()
🤖 Prompt for AI Agents
In src/fastmcp/server/auth/oauth_proxy.py around lines 1396 and 1805, add
explanatory inline comments next to the existing type: ignore[assignment]
annotations to match the pattern used elsewhere: for line 1396, note that
oauth_client.refresh_token() returns a broader/untyped dict (or a type that
doesn't match our expected return type) so we intentionally ignore an assignment
type mismatch; for line 1805, note that oauth_client.fetch_token() similarly
returns a dynamic/untyped structure (per the underlying OAuth library) so we
intentionally ignore an assignment type mismatch. Keep the comments concise and
reference that the mismatch is due to the OAuth library return types (analogous
to the RFC comment pattern used in the file).
All OAuth providers (Google, GitHub, Azure, Discord, WorkOS) now properly validate that base_url is provided either via parameter or environment variable. Updated tests to always provide base_url and added tests for the missing base_url validation error.
Test Failure AnalysisSummary: The workflow failed due to a transient GitHub infrastructure issue (503 Service Unavailable) when downloading the prek installer, not due to any code problems. Root Cause: The prek installation step failed with a 503 HTTP error when trying to download from This is a temporary service availability issue on GitHub's side, unrelated to the PR changes. Suggested Solution: No action needed - the workflow has already been automatically retried (run #20445442408 is currently in progress). If the retry also fails with the same infrastructure error, GitHub Actions will likely auto-retry again, or you can manually re-run the workflow. Detailed AnalysisThe failure occurred during the "Run prek" step in the Timeline:
This confirms the issue is intermittent infrastructure, not code-related. Related FilesNo code files are implicated - this is purely an infrastructure/network issue with the CI environment's ability to download from GitHub releases. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/fastmcp/server/auth/providers/azure.py (1)
236-239: LGTM! Validation logic is correct and consistent.The
base_urlvalidation follows the same pattern as other required field checks in this file and is necessary sincebase_urlis passed to the parent class at line 294.The static analysis hint (TRY003) about long inline exception messages applies to all validation blocks in this file (lines 211-212, 214-215, 219-224, 228-234, and these new lines). If you want to address it, consider refactoring all validation messages file-wide or codebase-wide for consistency, rather than changing just this one.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
tests/server/auth/providers/test_azure.pyis excluded by none and included by nonetests/server/auth/providers/test_discord.pyis excluded by none and included by nonetests/server/auth/providers/test_github.pyis excluded by none and included by nonetests/server/auth/providers/test_google.pyis excluded by none and included by nonetests/server/auth/providers/test_workos.pyis excluded by none and included by none
📒 Files selected for processing (1)
src/fastmcp/server/auth/providers/azure.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Use Python ≥ 3.10 with full type annotations
Never use bareexcept- be specific with exception types
Files:
src/fastmcp/server/auth/providers/azure.py
🧬 Code graph analysis (1)
src/fastmcp/server/auth/providers/azure.py (1)
src/fastmcp/server/server.py (1)
settings(348-356)
🪛 Ruff (0.14.10)
src/fastmcp/server/auth/providers/azure.py
237-239: Avoid specifying long messages outside the exception class
(TRY003)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Run tests: Python 3.10 on windows-latest
- GitHub Check: Run tests with lowest-direct dependencies
1 participant
Upgrades to ty 0.0.5 and fixes all type errors, enabling stricter type checking rules that were previously disabled.
Key changes:
elicit()method overloads for dict/list response_type patterns (single-select with titles, multi-select)createdAtin tasks/get handler - was passing string where datetime expected# type: ignorewith explicit casts or specific ignore codes with explanationscast(type[AuthProvider], ...)for server_auth_class instead of ignoring call-non-callableAll
no-matching-overloadandcall-non-callablety rules now enabled.