From 1aac04eb4e7475a1842bd39d9518ca8f98d93a8a Mon Sep 17 00:00:00 2001 From: Jeremiah Lowin <153965+jlowin@users.noreply.github.com> Date: Sat, 15 Nov 2025 12:23:38 -0500 Subject: [PATCH] Revert "Fix self-referencing types not being recognized as object schemas" --- pyproject.toml | 2 +- src/fastmcp/server/auth/oauth_proxy.py | 19 +----- .../server/auth/providers/in_memory.py | 26 +-------- src/fastmcp/tools/tool.py | 58 ++----------------- tests/client/auth/test_oauth_client.py | 6 +- .../auth/test_github_provider_integration.py | 1 - tests/server/test_auth_integration.py | 6 -- tests/tools/test_tool.py | 57 ++++-------------- uv.lock | 23 ++------ 9 files changed, 26 insertions(+), 172 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 07b8a168f..e20f60047 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -7,7 +7,7 @@ dependencies = [ "python-dotenv>=1.1.0", "exceptiongroup>=1.2.2", "httpx>=0.28.1", - "mcp>=1.19.0,<2.0.0,!=1.21.1", + "mcp>=1.19.0,<2.0.0", "openapi-pydantic>=0.5.1", "platformdirs>=4.0.0", "rich>=13.9.4", diff --git a/src/fastmcp/server/auth/oauth_proxy.py b/src/fastmcp/server/auth/oauth_proxy.py index f8babfa2e..516fd592e 100644 --- a/src/fastmcp/server/auth/oauth_proxy.py +++ b/src/fastmcp/server/auth/oauth_proxy.py @@ -44,7 +44,6 @@ from mcp.server.auth.provider import ( AccessToken, AuthorizationCode, AuthorizationParams, - AuthorizeError, RefreshToken, TokenError, ) @@ -940,8 +939,6 @@ class OAuthProxy(OAuthProvider): """ # Create a ProxyDCRClient with configured redirect URI validation - if client_info.client_id is None: - raise ValueError("client_id is required for client registration") proxy_client: ProxyDCRClient = ProxyDCRClient( client_id=client_info.client_id, client_secret=client_info.client_secret, @@ -971,7 +968,7 @@ class OAuthProxy(OAuthProvider): logger.debug( "Registered client %s with %d redirect URIs", client_info.client_id, - len(proxy_client.redirect_uris) if proxy_client.redirect_uris else 0, + len(proxy_client.redirect_uris), ) # ------------------------------------------------------------------------- @@ -1008,10 +1005,6 @@ class OAuthProxy(OAuthProvider): ) # Store transaction data for IdP callback processing - if client.client_id is None: - raise AuthorizeError( - error="invalid_client", error_description="Client ID is required" - ) transaction = OAuthTransaction( txn_id=txn_id, client_id=client.client_id, @@ -1090,10 +1083,6 @@ class OAuthProxy(OAuthProvider): return None # Create authorization code object with PKCE challenge - if client.client_id is None: - raise AuthorizeError( - error="invalid_client", error_description="Client ID is required" - ) return AuthorizationCode( code=authorization_code, client_id=client.client_id, @@ -1179,7 +1168,7 @@ class OAuthProxy(OAuthProvider): expires_at=time.time() + expires_in, token_type=idp_tokens.get("token_type", "Bearer"), scope=" ".join(authorization_code.scopes), - client_id=client.client_id or "", + client_id=client.client_id, created_at=time.time(), raw_token_data=idp_tokens, ) @@ -1192,8 +1181,6 @@ class OAuthProxy(OAuthProvider): logger.debug("Stored encrypted upstream tokens (jti=%s)", access_jti[:8]) # Issue minimal FastMCP access token (just a reference via JTI) - if client.client_id is None: - raise TokenError("invalid_client", "Client ID is required") fastmcp_access_token = self._jwt_issuer.issue_access_token( client_id=client.client_id, scopes=authorization_code.scopes, @@ -1395,8 +1382,6 @@ class OAuthProxy(OAuthProvider): ) # Issue new minimal FastMCP access token (just a reference via JTI) - if client.client_id is None: - raise TokenError("invalid_client", "Client ID is required") new_access_jti = secrets.token_urlsafe(32) new_fastmcp_access = self._jwt_issuer.issue_access_token( client_id=client.client_id, diff --git a/src/fastmcp/server/auth/providers/in_memory.py b/src/fastmcp/server/auth/providers/in_memory.py index c0275faa4..9a1bd0c7d 100644 --- a/src/fastmcp/server/auth/providers/in_memory.py +++ b/src/fastmcp/server/auth/providers/in_memory.py @@ -66,22 +66,6 @@ class InMemoryOAuthProvider(OAuthProvider): return self.clients.get(client_id) async def register_client(self, client_info: OAuthClientInformationFull) -> None: - # Validate scopes against valid_scopes if configured (matches MCP SDK behavior) - if ( - client_info.scope is not None - and self.client_registration_options is not None - and self.client_registration_options.valid_scopes is not None - ): - requested_scopes = set(client_info.scope.split()) - valid_scopes = set(self.client_registration_options.valid_scopes) - invalid_scopes = requested_scopes - valid_scopes - if invalid_scopes: - raise ValueError( - f"Requested scopes are not valid: {', '.join(invalid_scopes)}" - ) - - if client_info.client_id is None: - raise ValueError("client_id is required for client registration") if client_info.client_id in self.clients: # As per RFC 7591, if client_id is already known, it's an update. # For this simple provider, we'll treat it as re-registration. @@ -107,7 +91,7 @@ class InMemoryOAuthProvider(OAuthProvider): # OAuthClientInformationFull should have a method like validate_redirect_uri # For this test provider, we assume it's valid if it matches one in client_info # The AuthorizationHandler already does robust validation using client.validate_redirect_uri - if client.redirect_uris and params.redirect_uri not in client.redirect_uris: + if params.redirect_uri not in client.redirect_uris: # This check might be too simplistic if redirect_uris can be patterns # or if params.redirect_uri is None and client has a default. # However, the AuthorizationHandler handles the primary validation. @@ -126,10 +110,6 @@ class InMemoryOAuthProvider(OAuthProvider): client_allowed_scopes = set(client.scope.split()) scopes_list = [s for s in scopes_list if s in client_allowed_scopes] - if client.client_id is None: - raise AuthorizeError( - error="invalid_client", error_description="Client ID is required" - ) auth_code = AuthorizationCode( code=auth_code_value, client_id=client.client_id, @@ -186,8 +166,6 @@ class InMemoryOAuthProvider(OAuthProvider): time.time() + DEFAULT_REFRESH_TOKEN_EXPIRY_SECONDS ) - if client.client_id is None: - raise TokenError("invalid_client", "Client ID is required") self.access_tokens[access_token_value] = AccessToken( token=access_token_value, client_id=client.client_id, @@ -258,8 +236,6 @@ class InMemoryOAuthProvider(OAuthProvider): time.time() + DEFAULT_REFRESH_TOKEN_EXPIRY_SECONDS ) - if client.client_id is None: - raise TokenError("invalid_client", "Client ID is required") self.access_tokens[new_access_token_value] = AccessToken( token=new_access_token_value, client_id=client.client_id, diff --git a/src/fastmcp/tools/tool.py b/src/fastmcp/tools/tool.py index aac5f3e3d..4517f9486 100644 --- a/src/fastmcp/tools/tool.py +++ b/src/fastmcp/tools/tool.py @@ -294,11 +294,10 @@ class FunctionTool(Tool): # Note: explicit schemas (dict) are used as-is without auto-wrapping # Validate that explicit schemas are object type for structured content - # (resolving $ref references for self-referencing types) if final_output_schema is not None and isinstance(final_output_schema, dict): - if not _is_object_schema(final_output_schema): + if final_output_schema.get("type") != "object": raise ValueError( - f"Output schemas must represent object types due to MCP spec limitations. Received: {final_output_schema!r}" + f'Output schemas must have "type" set to "object" due to MCP spec limitations. Received: {final_output_schema!r}' ) return cls( @@ -367,52 +366,6 @@ class FunctionTool(Tool): ) -def _is_object_schema(schema: dict[str, Any]) -> bool: - """Check if a JSON schema represents an object type, resolving $ref references.""" - # Direct object type - if schema.get("type") == "object": - return True - - # Schema with properties but no explicit type is treated as object - if "properties" in schema: - return True - - # Resolve $ref references to check the referenced schema - # Self-referencing types (e.g., list["ReturnThing"]) generate schemas with $ref - # at the root level instead of "type": "object" directly - if "$ref" in schema: - ref = schema["$ref"] - if ref.startswith("#/$defs/"): - # Resolve reference within the same schema document - # The schema dict contains both $ref and $defs - defs_path = ref.replace("#/$defs/", "").split("/") - if "$defs" in schema: - defs = schema["$defs"] - current = defs - # Navigate through the defs path - for part in defs_path: - if isinstance(current, dict) and part in current: - current = current[part] - else: - # Can't resolve, assume it might be an object - # (safer to assume object than to wrap incorrectly) - return True - # Recursively check the resolved schema - if isinstance(current, dict): - return _is_object_schema(current) - # If $defs not found but we have a $ref, assume object - # (self-referencing types are typically objects) - return True - elif ref == "#": - # Self-reference - treat as object (common for recursive types) - return True - # For other $ref patterns, assume object to be safe - # (most $refs in JSON schemas point to object types) - return True - - return False - - @dataclass class ParsedFunction: fn: Callable[..., Any] @@ -525,9 +478,10 @@ class ParsedFunction: # Generate schema for wrapped type if it's non-object # because MCP requires that output schemas are objects - # Check if schema is an object type, resolving $ref references - # (self-referencing types use $ref at root level) - if wrap_non_object_output_schema and not _is_object_schema(base_schema): + if ( + wrap_non_object_output_schema + and base_schema.get("type") != "object" + ): # Use the wrapped result schema directly wrapped_type = _WrappedResult[clean_output_type] wrapped_adapter = get_cached_typeadapter(wrapped_type) diff --git a/tests/client/auth/test_oauth_client.py b/tests/client/auth/test_oauth_client.py index f9e452503..dbf41dd0b 100644 --- a/tests/client/auth/test_oauth_client.py +++ b/tests/client/auth/test_oauth_client.py @@ -18,9 +18,7 @@ def fastmcp_server(issuer_url: str): "TestServer", auth=InMemoryOAuthProvider( base_url=issuer_url, - client_registration_options=ClientRegistrationOptions( - enabled=True, valid_scopes=["read", "write"] - ), + client_registration_options=ClientRegistrationOptions(enabled=True), ), ) @@ -56,7 +54,7 @@ def client_with_headless_oauth(streamable_http_server: str) -> Client: """Client with headless OAuth that bypasses browser interaction.""" return Client( transport=StreamableHttpTransport(streamable_http_server), - auth=HeadlessOAuth(mcp_url=streamable_http_server, scopes=["read", "write"]), + auth=HeadlessOAuth(mcp_url=streamable_http_server), ) diff --git a/tests/integration_tests/auth/test_github_provider_integration.py b/tests/integration_tests/auth/test_github_provider_integration.py index 8ad84dc65..7623a7690 100644 --- a/tests/integration_tests/auth/test_github_provider_integration.py +++ b/tests/integration_tests/auth/test_github_provider_integration.py @@ -360,7 +360,6 @@ async def test_github_oauth_unauthorized_access(github_server: str): async def test_github_oauth_with_mock(github_client_with_mock: Client): """Test complete GitHub OAuth flow with mocked callback.""" - async with github_client_with_mock: # Test that we can ping the server (requires successful OAuth) assert await github_client_with_mock.ping() diff --git a/tests/server/test_auth_integration.py b/tests/server/test_auth_integration.py index 1eb232f8b..3ef2b4018 100644 --- a/tests/server/test_auth_integration.py +++ b/tests/server/test_auth_integration.py @@ -49,8 +49,6 @@ class MockOAuthProvider(OAuthAuthorizationServerProvider): ) -> str: # toy authorize implementation which just immediately generates an authorization # code and completes the redirect - if client.client_id is None: - raise ValueError("client_id is required") code = AuthorizationCode( code=f"code_{int(time.time())}", client_id=client.client_id, @@ -81,8 +79,6 @@ class MockOAuthProvider(OAuthAuthorizationServerProvider): refresh_token = f"refresh_{secrets.token_hex(32)}" # Store the tokens - if client.client_id is None: - raise ValueError("client_id is required") self.tokens[access_token] = AccessToken( token=access_token, client_id=client.client_id, @@ -146,8 +142,6 @@ class MockOAuthProvider(OAuthAuthorizationServerProvider): new_refresh_token = f"refresh_{secrets.token_hex(32)}" # Store the new tokens - if client.client_id is None: - raise ValueError("client_id is required") self.tokens[new_access_token] = AccessToken( token=new_access_token, client_id=client.client_id, diff --git a/tests/tools/test_tool.py b/tests/tools/test_tool.py index 731bfed91..c779d0942 100644 --- a/tests/tools/test_tool.py +++ b/tests/tools/test_tool.py @@ -927,7 +927,7 @@ class TestToolFromFunctionOutputSchema: for schema in non_object_schemas: with pytest.raises( - ValueError, match="Output schemas must represent object types" + ValueError, match='Output schemas must have "type" set to "object"' ): Tool.from_function(func, output_schema=schema) @@ -1262,31 +1262,6 @@ class TestAutomaticStructuredContent: "verified": True, } - async def test_self_referencing_dataclass_not_wrapped(self): - """Test that self-referencing dataclasses are not wrapped in result field.""" - - @dataclass - class ReturnThing: - value: int - stuff: list["ReturnThing"] - - def return_things() -> ReturnThing: - return ReturnThing(value=123, stuff=[ReturnThing(value=456, stuff=[])]) - - tool = Tool.from_function(return_things) - - result = await tool.run({}) - - # Should have structured content without wrapping - assert result.structured_content is not None - # Should NOT be wrapped in "result" field - assert "result" not in result.structured_content - # Should have the actual data directly - assert result.structured_content == { - "value": 123, - "stuff": [{"value": 456, "stuff": []}], - } - async def test_int_return_no_structured_content_without_schema(self): """Test that int returns don't create structured content without output schema.""" @@ -1549,20 +1524,13 @@ class TestSerializationAlias: # not the first validation alias 'id' assert tool.output_schema is not None - # For object types, the schema may use $ref at root (self-referencing types) - # or have properties directly. Check both cases. - if "$ref" in tool.output_schema: - # Schema uses $ref - resolve to get the actual definition - assert "$defs" in tool.output_schema - ref_path = tool.output_schema["$ref"].replace("#/$defs/", "") - component_def = tool.output_schema["$defs"][ref_path] - else: - # Schema has properties directly (wrapped case) - assert "properties" in tool.output_schema - assert "result" in tool.output_schema["properties"] - assert "$defs" in tool.output_schema - # Find the Component definition - component_def = list(tool.output_schema["$defs"].values())[0] + # Check the wrapped result schema + assert "properties" in tool.output_schema + assert "result" in tool.output_schema["properties"] + assert "$defs" in tool.output_schema + + # Find the Component definition + component_def = list(tool.output_schema["$defs"].values())[0] # Should have 'componentId' not 'id' in properties assert "componentId" in component_def["properties"] @@ -1605,13 +1573,8 @@ class TestSerializationAlias: # The result should contain the serialized form with 'componentId' assert result.structured_content is not None - # Object types may be wrapped in "result" or not, depending on schema structure - if "result" in result.structured_content: - component_data = result.structured_content["result"] - else: - component_data = result.structured_content - assert component_data["componentId"] == "test123" - assert "id" not in component_data + assert result.structured_content["result"]["componentId"] == "test123" + assert "id" not in result.structured_content["result"] class TestToolTitle: diff --git a/uv.lock b/uv.lock index 6b50678b5..cd0f46d40 100644 --- a/uv.lock +++ b/uv.lock @@ -619,7 +619,7 @@ requires-dist = [ { name = "exceptiongroup", specifier = ">=1.2.2" }, { name = "httpx", specifier = ">=0.28.1" }, { name = "jsonschema-path", specifier = ">=0.3.4" }, - { name = "mcp", specifier = ">=1.19.0,!=1.21.1,<2.0.0" }, + { name = "mcp", specifier = ">=1.19.0,<2.0.0" }, { name = "openai", marker = "extra == 'openai'", specifier = ">=1.102.0" }, { name = "openapi-pydantic", specifier = ">=0.5.1" }, { name = "platformdirs", specifier = ">=4.0.0" }, @@ -1033,7 +1033,7 @@ wheels = [ [[package]] name = "mcp" -version = "1.21.0" +version = "1.19.0" source = { registry = "https://pypi.org/simple" } dependencies = [ { name = "anyio" }, @@ -1042,16 +1042,15 @@ dependencies = [ { name = "jsonschema" }, { name = "pydantic" }, { name = "pydantic-settings" }, - { name = "pyjwt", extra = ["crypto"] }, { name = "python-multipart" }, { name = "pywin32", marker = "sys_platform == 'win32'" }, { name = "sse-starlette" }, { name = "starlette" }, { name = "uvicorn", marker = "sys_platform != 'emscripten'" }, ] -sdist = { url = "https://files.pythonhosted.org/packages/33/54/dd2330ef4611c27ae59124820863c34e1d3edb1133c58e6375e2d938c9c5/mcp-1.21.0.tar.gz", hash = "sha256:bab0a38e8f8c48080d787233343f8d301b0e1e95846ae7dead251b2421d99855", size = 452697, upload-time = "2025-11-06T23:19:58.432Z" } +sdist = { url = "https://files.pythonhosted.org/packages/69/2b/916852a5668f45d8787378461eaa1244876d77575ffef024483c94c0649c/mcp-1.19.0.tar.gz", hash = "sha256:213de0d3cd63f71bc08ffe9cc8d4409cc87acffd383f6195d2ce0457c021b5c1", size = 444163, upload-time = "2025-10-24T01:11:15.839Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/39/47/850b6edc96c03bd44b00de9a0ca3c1cc71e0ba1cd5822955bc9e4eb3fad3/mcp-1.21.0-py3-none-any.whl", hash = "sha256:598619e53eb0b7a6513db38c426b28a4bdf57496fed04332100d2c56acade98b", size = 173672, upload-time = "2025-11-06T23:19:56.508Z" }, + { url = "https://files.pythonhosted.org/packages/ce/a3/3e71a875a08b6a830b88c40bc413bff01f1650f1efe8a054b5e90a9d4f56/mcp-1.19.0-py3-none-any.whl", hash = "sha256:f5907fe1c0167255f916718f376d05f09a830a215327a3ccdd5ec8a519f2e572", size = 170105, upload-time = "2025-10-24T01:11:14.151Z" }, ] [[package]] @@ -1486,20 +1485,6 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/aa/e8/691115aa790a2fa4bfad456287061a7439aaf877edfb0befd13486440de9/pyinstrument-5.1.0-cp314-cp314t-win_amd64.whl", hash = "sha256:c6711d53e600cfadb16bff68ba29c9e4f13e61196f185e32e8e29c8baa1dd606", size = 126064, upload-time = "2025-08-10T11:17:37.013Z" }, ] -[[package]] -name = "pyjwt" -version = "2.10.1" -source = { registry = "https://pypi.org/simple" } -sdist = { url = "https://files.pythonhosted.org/packages/e7/46/bd74733ff231675599650d3e47f361794b22ef3e3770998dda30d3b63726/pyjwt-2.10.1.tar.gz", hash = "sha256:3cc5772eb20009233caf06e9d8a0577824723b44e6648ee0a2aedb6cf9381953", size = 87785, upload-time = "2024-11-28T03:43:29.933Z" } -wheels = [ - { url = "https://files.pythonhosted.org/packages/61/ad/689f02752eeec26aed679477e80e632ef1b682313be70793d798c1d5fc8f/PyJWT-2.10.1-py3-none-any.whl", hash = "sha256:dcdd193e30abefd5debf142f9adfcdd2b58004e644f25406ffaebd50bd98dacb", size = 22997, upload-time = "2024-11-28T03:43:27.893Z" }, -] - -[package.optional-dependencies] -crypto = [ - { name = "cryptography" }, -] - [[package]] name = "pyperclip" version = "1.9.0"