Address PR feedback

Signed-off-by: Daniel Garnier-Moiroux <git@garnier.wf>

Author: Kehrlann

docs/server.md

@@ -489,7 +489,11 @@ The same `addToolFilter(...)` method is available on the stateless builders.
   legitimately see different results for two successive requests carrying different credentials.
 - Registration order is preserved; only omissions happen.
 - Returning `Mono.empty()` from an async filter omits the tool. An error fails the whole listing
-  request rather than silently hiding tools.
+  request rather than silently hiding tools: a client cannot tell a filtered-down listing from a
+  partial one, and MCP has no way to signal "this listing was incomplete, retry".
+- A filter that errors is logged server-side and reported to the client as an opaque
+  `-32603 Internal error` with no `data`. If you want the client to see a specific error, throw an
+  `McpError`, those are passed through.
 - Filters accumulate as a boolean **AND**: a tool is listed only when every registered filter accepts it, so a
   later `addToolFilter(...)` can never widen access. Evaluation follows registration order and
   short-circuits on the first filter that hides a tool.
@@ -504,16 +508,26 @@ The same `addToolFilter(...)` method is available on the stateless builders.
     ```
 
 - Tools are tested one at a time, so a filter that performs I/O per tool costs one round trip per
-  tool. Resolve per-request state **once** in your `contextExtractor` and read it in the filter:
+  tool. Sync filters also run on a shared scheduler thread — not the request thread — unless
+  `immediateExecution(true)` is set, so thread-bound request state (Spring Security's
+  `SecurityContextHolder`, MDC, custom `ThreadLocal` holders) is **not visible** inside the filter.
+  For both reasons, resolve per-request state **once** in the transport's `contextExtractor`,
+  which does run on the request thread, and read only the extracted context in the filter:
 
     ```java
-    // one authorization lookup, shared by every tool tested in this request
-    .contextExtractor(request -> McpTransportContext.create(
-            Map.of("perms", introspect(request.getHeader("Authorization")))))
+    // transport builder: one authorization lookup, on the request thread,
+    // shared by every tool tested in this request
+    var transportProvider = HttpServletStreamableServerTransportProvider.builder()
+        .contextExtractor(request -> McpTransportContext.create(
+                Map.of("perms", introspect(request.getHeader("Authorization")))))
+        // ...
+        .build();
 
-    .addToolFilter((context, tool) -> 
-            ((Set<String>) context.get("perms")).contains(tool.name())
-    )
+    // server builder: the filter reads only the extracted context
+    McpServer.sync(transportProvider)
+        .addToolFilter((context, tool) ->
+                ((Set<String>) context.get("perms")).contains(tool.name()))
+        .build();
     ```
 
 - `notifications/tools/list_changed` is **not** filtered. It is a server-initiated broadcast with

mcp-core/src/main/java/io/modelcontextprotocol/server/McpAsyncListFilter.java

@@ -65,10 +65,10 @@ static <T> McpAsyncListFilter<T> fromSync(McpSyncListFilter<T> filter, boolean i
 	 * contain {@code null} elements.
 	 */
 	static <T> McpAsyncListFilter<T> and(List<McpAsyncListFilter<T>> filters) {
-		Assert.noNullElements(filters, "filters must not contain null elements");
 		if (filters == null || filters.isEmpty()) {
 			return (transportContext, primitive) -> Mono.just(Boolean.TRUE);
 		}
+		Assert.noNullElements(filters, "filters must not contain null elements");
 		if (filters.size() == 1) {
 			return filters.get(0);
 		}

mcp-core/src/main/java/io/modelcontextprotocol/server/McpAsyncServer.java

@@ -545,12 +545,28 @@ private McpRequestHandler<McpSchema.ListToolsResult> toolsListRequestHandler() {
 			// view, otherwise page offsets leak the number of hidden tools.
 			return Flux.fromIterable(this.tools)
 				.map(McpServerFeatures.AsyncToolSpecification::tool)
-				.filterWhen(tool -> this.toolFilter.isVisible(exchange.transportContext(), tool))
+				.filterWhen(tool -> this.toolFilter.isVisible(exchange.transportContext(), tool)
+					.onErrorResume(error -> opaqueListFilterError(tool, error)))
 				.collectList()
 				.map(tools -> McpSchema.ListToolsResult.builder(tools).build());
 		};
 	}
 
+	/**
+	 * Report a list filter failure to the client as an opaque {@code -32603} error, so
+	 * that filter internals such as identity provider hostnames or the reason a principal
+	 * was rejected never leave the server. The actual cause is logged instead. An
+	 * {@link McpError} is deliberate on the filter's part and passes through untouched.
+	 */
+	private static Mono<Boolean> opaqueListFilterError(Tool tool, Throwable error) {
+		if (error instanceof McpError mcpError && mcpError.getJsonRpcError() != null) {
+			logger.debug("Tool list filter failed for tool '{}' with an explicit MCP error", tool.name(), error);
+			return Mono.error(mcpError);
+		}
+		logger.error("Tool list filter failed for tool '{}', failing the tools/list request", tool.name(), error);
+		return Mono.error(McpError.builder(ErrorCodes.INTERNAL_ERROR).message("Internal error").build());
+	}
+
 	private McpRequestHandler<CallToolResult> toolsCallRequestHandler() {
 		return (exchange, params) -> {
 			McpSchema.CallToolRequest callToolRequest = jsonMapper.convertValue(params,

mcp-core/src/main/java/io/modelcontextprotocol/server/McpServer.java

@@ -1671,10 +1671,7 @@ public StatelessAsyncSpecification validateToolInputs(boolean validate) {
 		 * accepts it, so a later registration can never widen access.
 		 * <p>
 		 * A hidden tool is omitted from listings only. It remains callable by name, so
-		 * enforce permissions in the tool's call handler. Tools are NOT hidden from
-		 * {@code notifications/tools/list_changed}, as it is a per-client context rather
-		 * than per-request. Consider disabling list changed notifications entirely when
-		 * using filters.
+		 * enforce permissions in the tool's call handler.
 		 * <p>
 		 * @param toolFilter the filter to add, must not be null
 		 * @return This builder instance for method chaining
@@ -2211,10 +2208,7 @@ public StatelessSyncSpecification validateToolInputs(boolean validate) {
 		 * accepts it, so a later registration can never widen access.
 		 * <p>
 		 * A hidden tool is omitted from listings only. It remains callable by name, so
-		 * enforce permissions in the tool's call handler. Tools are NOT hidden from
-		 * {@code notifications/tools/list_changed}, as it is a per-client context rather
-		 * than per-request. Consider disabling list changed notifications entirely when
-		 * using filters.
+		 * enforce permissions in the tool's call handler.
 		 * <p>
 		 * The filter is offloaded to a bounded elastic scheduler unless
 		 * {@link #immediateExecution(boolean)} is set.

mcp-core/src/main/java/io/modelcontextprotocol/server/McpStatelessAsyncServer.java

@@ -424,12 +424,28 @@ private McpStatelessRequestHandler<McpSchema.ListToolsResult> toolsListRequestHa
 			// view, otherwise page offsets leak the number of hidden tools.
 			return Flux.fromIterable(this.tools)
 				.map(McpStatelessServerFeatures.AsyncToolSpecification::tool)
-				.filterWhen(tool -> this.toolFilter.isVisible(ctx, tool))
+				.filterWhen(tool -> this.toolFilter.isVisible(ctx, tool)
+					.onErrorResume(error -> opaqueListFilterError(tool, error)))
 				.collectList()
 				.map(tools -> McpSchema.ListToolsResult.builder(tools).build());
 		};
 	}
 
+	/**
+	 * Report a list filter failure to the client as an opaque {@code -32603} error, so
+	 * that filter internals such as identity provider hostnames or the reason a principal
+	 * was rejected never leave the server. The actual cause is logged instead. An
+	 * {@link McpError} is deliberate on the filter's part and passes through untouched.
+	 */
+	private static Mono<Boolean> opaqueListFilterError(Tool tool, Throwable error) {
+		if (error instanceof McpError mcpError && mcpError.getJsonRpcError() != null) {
+			logger.debug("Tool list filter failed for tool '{}' with an explicit MCP error", tool.name(), error);
+			return Mono.error(mcpError);
+		}
+		logger.error("Tool list filter failed for tool '{}', failing the tools/list request", tool.name(), error);
+		return Mono.error(McpError.builder(ErrorCodes.INTERNAL_ERROR).message("Internal error").build());
+	}
+
 	private McpStatelessRequestHandler<CallToolResult> toolsCallRequestHandler() {
 		return (ctx, params) -> {
 			McpSchema.CallToolRequest callToolRequest = jsonMapper.convertValue(params,

mcp-test/src/test/java/io/modelcontextprotocol/server/McpSyncListFilteringIntegrationTests.java

@@ -174,16 +174,52 @@ void asyncFilter() {
 	}
 
 	@Test
-	void filterErrorPropagates() {
+	void filterErrorFailsTheListingWithAnOpaqueInternalError() {
 		var toolSpec = serverFactory.namedAsyncTool("tool");
 
 		serverFactory.asyncServer()
 			.tools(List.of(toolSpec.spec()))
-			.addToolFilter((ctx, tool) -> Mono.error(new RuntimeException("filter error")))
+			.addToolFilter((ctx, tool) -> Mono.error(new RuntimeException("private information")))
 			.build();
 
 		mcpClient.initialize();
-		assertThatThrownBy(mcpClient::listTools).isInstanceOf(McpError.class).hasMessage("filter error");
+
+		// The listing fails, rather than silently omitting the tool: the client has no
+		// way to tell a filtered-down list from a partial one.
+		assertThatThrownBy(mcpClient::listTools).isInstanceOf(McpError.class)
+			.hasMessage("Internal error")
+			.extracting(error -> ((McpError) error).getJsonRpcError())
+			.satisfies(jsonRpcError -> {
+				assertThat(jsonRpcError.code()).isEqualTo(McpSchema.ErrorCodes.INTERNAL_ERROR);
+				// The filter's own diagnostics MUST NOT reach the client.
+				assertThat(jsonRpcError.message()).doesNotContain("private information");
+				assertThat(jsonRpcError.data()).isNull();
+			});
+	}
+
+	@Test
+	void filterMcpErrorIsPassedThroughVerbatim() {
+		var toolSpec = serverFactory.namedAsyncTool("tool");
+
+		serverFactory.asyncServer()
+			.tools(List.of(toolSpec.spec()))
+			.addToolFilter((ctx,
+					tool) -> Mono.error(McpError.builder(McpSchema.ErrorCodes.INVALID_REQUEST)
+						.message("Step-up authentication required")
+						.data(Map.of("scope", "tools:read"))
+						.build()))
+			.build();
+
+		mcpClient.initialize();
+
+		// An McpError is a deliberate choice by the filter author, so it is not scrubbed.
+		assertThatThrownBy(mcpClient::listTools).isInstanceOf(McpError.class)
+			.extracting(error -> ((McpError) error).getJsonRpcError())
+			.satisfies(jsonRpcError -> {
+				assertThat(jsonRpcError.code()).isEqualTo(McpSchema.ErrorCodes.INVALID_REQUEST);
+				assertThat(jsonRpcError.message()).isEqualTo("Step-up authentication required");
+				assertThat(jsonRpcError.data()).isEqualTo(Map.of("scope", "tools:read"));
+			});
 	}
 
 	@Test