diff --git a/includes/class-saddle-mcp.php b/includes/class-saddle-mcp.php index 6ae3bc3..ca62ed5 100644 --- a/includes/class-saddle-mcp.php +++ b/includes/class-saddle-mcp.php @@ -300,10 +300,89 @@ public static function register_routes() { ), ) ); + } + /** + * Make the notification acknowledgement true whichever transport answered. + * + * THE FIX FOR #155, and the reason it is separate from register_routes(): + * that method runs only when Saddle serves MCP itself. When the official + * MCP Adapter plugin is installed, saddle.php hands the whole request to + * it, and the answer to `notifications/initialized` is then that plugin's + * code at whatever version the site has — not ours. A tester on Codex hit + * exactly that: 200 with an empty body, then `EOF while parsing a value`, + * which is what a strict client does when it tries to JSON-parse nothing. + * + * Saddle should not depend on a third-party plugin's spec compliance for + * the one step that sits between "connected" and `tools/list`. Both filters + * are our own and neither touches the vendored library. They are safe on + * both paths because the adapter registers under Saddle's OWN namespace and + * route ({@see self::register_adapter_server()}), so owns_route() matches + * either way — and everything here is scoped to that route. + */ + public static function register_spec_guards() { + // Before WP_REST_Server sets the status from the response object + // (rest_post_dispatch fires first), so correcting it here is what + // actually reaches the wire. + add_filter( 'rest_post_dispatch', array( __CLASS__, 'normalize_notification_status' ), 10, 3 ); add_filter( 'rest_pre_serve_request', array( __CLASS__, 'serve_empty_acknowledgement' ), 10, 3 ); } + /** + * Force 202 on a JSON-RPC notification aimed at Saddle's MCP route. + * + * Narrow twice over: our route only, and only when the body really is a + * notification — no `id`, and a `notifications/` method. A batch qualifies + * only if EVERY member does, because one real call in it expects a real + * response. An unparseable body is left alone: that is a 400 the transport + * already answered correctly, and guessing at it would hide the error. + * + * @param WP_HTTP_Response $response The dispatched response. + * @param mixed $server The REST server (unused). + * @param WP_REST_Request $request The request. + * @return mixed + */ + public static function normalize_notification_status( $response, $server = null, $request = null ) { + if ( ! $response instanceof WP_HTTP_Response || ! $request instanceof WP_REST_Request ) { + return $response; + } + + if ( ! self::owns_route( $request ) || ! self::is_notification_request( $request ) ) { + return $response; + } + + $response->set_status( 202 ); + return $response; + } + + /** + * Whether a request body is a JSON-RPC notification, or a batch of nothing + * but notifications. + * + * @param WP_REST_Request $request The request. + * @return bool + */ + private static function is_notification_request( WP_REST_Request $request ) { + $body = json_decode( (string) $request->get_body(), true ); + if ( ! is_array( $body ) || array() === $body ) { + return false; + } + + $messages = isset( $body['jsonrpc'] ) || isset( $body['method'] ) ? array( $body ) : $body; + + foreach ( $messages as $message ) { + if ( ! is_array( $message ) ) { + return false; + } + $method = isset( $message['method'] ) ? (string) $message['method'] : ''; + if ( isset( $message['id'] ) || 0 !== strpos( $method, 'notifications/' ) ) { + return false; + } + } + + return true; + } + /** * Answer GET and DELETE with the 405 the spec asks for. * diff --git a/languages/saddle.pot b/languages/saddle.pot index edec6ca..7495c65 100644 --- a/languages/saddle.pot +++ b/languages/saddle.pot @@ -9,7 +9,7 @@ msgstr "" "MIME-Version: 1.0\n" "Content-Type: text/plain; charset=UTF-8\n" "Content-Transfer-Encoding: 8bit\n" -"POT-Creation-Date: 2026-08-25T04:45:33+00:00\n" +"POT-Creation-Date: 2026-08-25T07:53:05+00:00\n" "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n" "X-Generator: WP-CLI 2.12.0\n" "X-Domain: saddle\n" @@ -2117,7 +2117,7 @@ msgid "Saddle sign-in keys cannot be used over XML-RPC." msgstr "" #: includes/class-saddle-connection.php:386 -#: includes/class-saddle-mcp.php:399 +#: includes/class-saddle-mcp.php:478 msgid "Your sign-in key was rejected — it was most likely revoked or removed. Reconnect the app from Saddle to issue a fresh key." msgstr "" @@ -2500,65 +2500,65 @@ msgstr "" msgid "Tiered, default-safe, approval-gated MCP access to posts, pages, and media." msgstr "" -#: includes/class-saddle-mcp.php:336 +#: includes/class-saddle-mcp.php:415 msgid "This MCP endpoint does not offer an event stream. Send JSON-RPC over POST instead." msgstr "" -#: includes/class-saddle-mcp.php:337 +#: includes/class-saddle-mcp.php:416 msgid "This MCP endpoint is stateless and issues no session to terminate." msgstr "" -#: includes/class-saddle-mcp.php:388 +#: includes/class-saddle-mcp.php:467 msgid "The access token was rejected — it has expired, been revoked, or was issued for a different site. Refresh it, or reconnect the app to authorize a new one." msgstr "" -#: includes/class-saddle-mcp.php:409 +#: includes/class-saddle-mcp.php:488 msgid "The request arrived without a sign-in key. If you did connect one, your host may be stripping the Authorization header — open Saddle and run the connection check to confirm and fix it." msgstr "" -#: includes/class-saddle-mcp.php:442 +#: includes/class-saddle-mcp.php:521 msgid "Parse error: request body is not valid JSON-RPC." msgstr "" -#: includes/class-saddle-mcp.php:451 +#: includes/class-saddle-mcp.php:530 msgid "Invalid request." msgstr "" #. translators: 1: protocol version the client asked for, 2: comma-separated list of supported versions. -#: includes/class-saddle-mcp.php:601 +#: includes/class-saddle-mcp.php:680 #, php-format msgid "Unsupported MCP protocol version \"%1$s\". This server speaks %2$s." msgstr "" #. translators: %s: JSON-RPC method name. #. translators: %s: method name -#: includes/class-saddle-mcp.php:679 +#: includes/class-saddle-mcp.php:758 #: includes/lib/wp-mcp/includes/Infrastructure/ErrorHandling/McpErrorFactory.php:117 #, php-format msgid "Method not found: %s" msgstr "" -#: includes/class-saddle-mcp.php:738 +#: includes/class-saddle-mcp.php:817 msgid "Saddle is paused on this site, so no tools will run. The site owner can resume it in Saddle → Settings. Do not retry until they do." msgstr "" -#: includes/class-saddle-mcp.php:750 +#: includes/class-saddle-mcp.php:829 msgid "Saddle exposes tiered, approval-gated access to this site's posts, pages, and media. Call saddle/get-instructions for the current scope and safety rules before acting." msgstr "" -#: includes/class-saddle-mcp.php:943 +#: includes/class-saddle-mcp.php:1022 msgid "Invalid params: a Saddle tool name is required." msgstr "" #. translators: %s: tool name. #. translators: %s: tool name -#: includes/class-saddle-mcp.php:953 +#: includes/class-saddle-mcp.php:1032 #: includes/lib/wp-mcp/includes/Infrastructure/ErrorHandling/McpErrorFactory.php:246 #, php-format msgid "Tool not found: %s" msgstr "" -#: includes/class-saddle-mcp.php:983 +#: includes/class-saddle-mcp.php:1062 msgid "None of Saddle’s site-wide gates (pause, access level, per-tool toggles) blocked this — the connected WordPress account likely lacks a capability this tool requires, or the specific item is protected. Do not retry the same call." msgstr "" @@ -4290,7 +4290,7 @@ msgid "This persisted attribute does nothing — rewrite it on a path the block msgstr "" #. translators: %s: minimum WordPress version. -#: saddle.php:345 +#: saddle.php:351 #, php-format msgid "Saddle requires WordPress %s or later (the core Abilities API). The MCP server is disabled until you upgrade." msgstr "" diff --git a/saddle.php b/saddle.php index 3f4766e..4741bc4 100644 --- a/saddle.php +++ b/saddle.php @@ -284,6 +284,12 @@ public static function setup_mcp_transport() { // the build most likely to need it is the one without the adapter. Saddle_MCP_Diagnostics::register(); + // Spec conformance Saddle owns on BOTH transports: a JSON-RPC + // notification on our route must be answered 202 with no body, and on + // the adapter path that answer would otherwise be the MCP Adapter + // plugin's, at whatever version the site happens to have (#155). + add_action( 'rest_api_init', array( 'Saddle_MCP', 'register_spec_guards' ) ); + if ( self::adapter_available() ) { // Third-party hook, owned by the MCP Adapter plugin — its name is // theirs, and this is the documented way to register a server with diff --git a/tests/mcp-notification-normalize-test.php b/tests/mcp-notification-normalize-test.php new file mode 100644 index 0000000..bd5cf1a --- /dev/null +++ b/tests/mcp-notification-normalize-test.php @@ -0,0 +1,173 @@ +set_header( 'content-type', 'application/json' ); + $req->set_body( wp_json_encode( $body ) ); + return $req; + } + + /** + * Stands in for a transport that answered the notification wrongly — the + * adapter's job on that path, and the exact shape the tester reported. + */ + private function dispatched( WP_REST_Request $request, $status = 200, $data = null ) { + return apply_filters( + 'rest_post_dispatch', + new WP_REST_Response( $data, $status ), + rest_get_server(), + $request + ); + } + + /* -------- the bug -------- */ + + public function test_a_notification_answered_200_is_corrected_to_202() { + $request = $this->request( + array( + 'jsonrpc' => '2.0', + 'method' => 'notifications/initialized', + ) + ); + $response = $this->dispatched( $request, 200 ); + + $this->assertSame( + 202, + $response->get_status(), + 'A notification must leave Saddle as 202 whichever transport answered it.' + ); + } + + public function test_an_all_notification_batch_answered_200_is_corrected() { + $request = $this->request( + array( + array( 'jsonrpc' => '2.0', 'method' => 'notifications/initialized' ), + array( 'jsonrpc' => '2.0', 'method' => 'notifications/cancelled' ), + ) + ); + $response = $this->dispatched( $request, 200 ); + + $this->assertSame( 202, $response->get_status() ); + } + + public function test_the_corrected_202_carries_no_body() { + $request = $this->request( + array( + 'jsonrpc' => '2.0', + 'method' => 'notifications/initialized', + ) + ); + $response = $this->dispatched( $request, 200 ); + + $served = apply_filters( 'rest_pre_serve_request', false, $response, $request, rest_get_server() ); + + $this->assertTrue( + $served, + 'The acknowledgement must be reported as already served, so WordPress prints nothing.' + ); + } + + /* -------- and nothing else moves -------- */ + + public function test_a_real_request_on_our_route_is_left_alone() { + $request = $this->request( + array( + 'jsonrpc' => '2.0', + 'id' => 1, + 'method' => 'tools/list', + ) + ); + $response = $this->dispatched( $request, 200, array( 'result' => array() ) ); + + $this->assertSame( 200, $response->get_status(), 'A call with an id is not a notification.' ); + $this->assertNotNull( $response->get_data(), 'A real response keeps its envelope.' ); + } + + public function test_a_batch_containing_a_real_call_is_left_alone() { + $request = $this->request( + array( + array( 'jsonrpc' => '2.0', 'method' => 'notifications/initialized' ), + array( 'jsonrpc' => '2.0', 'id' => 1, 'method' => 'tools/list' ), + ) + ); + $response = $this->dispatched( $request, 200 ); + + $this->assertSame( + 200, + $response->get_status(), + 'A batch that contains one real call expects a real response.' + ); + } + + public function test_a_notification_shaped_body_on_someone_elses_route_is_left_alone() { + $request = $this->request( + array( + 'jsonrpc' => '2.0', + 'method' => 'notifications/initialized', + ), + '/wp/v2/posts' + ); + $response = $this->dispatched( $request, 200 ); + + $this->assertSame( + 200, + $response->get_status(), + 'Saddle must only ever touch its own MCP route.' + ); + } + + public function test_a_non_json_body_on_our_route_is_left_alone() { + $req = new WP_REST_Request( 'POST', '/saddle/v1/mcp' ); + $req->set_header( 'content-type', 'application/json' ); + $req->set_body( 'not json at all' ); + + $response = $this->dispatched( $req, 400 ); + + $this->assertSame( 400, $response->get_status(), 'An unparseable body keeps its error status.' ); + } + + /** + * The guard that stops this emptying the next response in the same + * process — the failure mode the own-transport version was careful about. + */ + public function test_a_non_202_response_is_never_emptied() { + $request = $this->request( array( 'jsonrpc' => '2.0', 'id' => 1, 'method' => 'ping' ) ); + $response = $this->dispatched( $request, 200, array( 'result' => 'pong' ) ); + + $served = apply_filters( 'rest_pre_serve_request', false, $response, $request, rest_get_server() ); + + $this->assertFalse( $served, 'Only the 202 acknowledgement may be short-circuited.' ); + } +}