From 3299034297ac1708816eb6c46350dc09d46e7643 Mon Sep 17 00:00:00 2001 From: Jan-Niclas Loosen Date: Fri, 25 Sep 2026 15:50:51 +0200 Subject: [PATCH] fix: prevent mcp-oauth's duplicate server registration instead of tolerating it Proactively unhook wp-media/mcp-oauth's own mcp_adapter_init callback before creating our server, rather than letting it collide and rely on WP_DEBUG=true being harmless. The collision previously surfaced as a fatal-looking _doing_it_wrong notice under WP-CLI (breaking every wp command while the plugin is active) and could leak debug notices into HTTP responses on customer sites that run WP_DEBUG. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01At3V6SjwbMe6eMzVGW3Hs7 --- src/Server/ServerRegistrar.php | 45 +++++++++++++++++++++-- tests/Unit/Server/ServerRegistrarTest.php | 40 ++++++++++++++++++++ 2 files changed, 82 insertions(+), 3 deletions(-) diff --git a/src/Server/ServerRegistrar.php b/src/Server/ServerRegistrar.php index 9cbb326..55e9122 100644 --- a/src/Server/ServerRegistrar.php +++ b/src/Server/ServerRegistrar.php @@ -9,14 +9,19 @@ use LIT\MCP\Integrations\IntegrationRegistry; use WP\MCP\Core\McpAdapter; use WP\MCP\Infrastructure\ErrorHandling\ErrorLogMcpErrorHandler; use WPMedia\MCP\OAuth\Transport\OAuthHttpTransport; +use WPMedia\MCP\OAuth\Transport\ServerRegistrar as OAuthServerRegistrar; /** * Registers the OAuth-protected MCP server with our own tool list. * * wp-media/mcp-oauth hardcodes server id/route (token audience) and only three - * meta-tools. We register the same server id first (priority 5 < mcp-oauth's 10); - * mcp-oauth's later attempt fails with "duplicate server id" (logged via - * _doing_it_wrong in debug mode only) – see spec §4.2. + * meta-tools, and registers its own server unconditionally on the same + * `mcp_adapter_init` action (priority 10). We register the same server id + * first (priority 5) and, right before creating it, unhook mcp-oauth's own + * `ServerRegistrar::register_server` callback so it never runs – see + * `unhookOAuthDefaultServer()`. This avoids the duplicate-server-id WP_Error + * (and the `_doing_it_wrong()` notice it used to trigger) entirely instead of + * merely tolerating it – see spec §4.2. */ final class ServerRegistrar { @@ -79,6 +84,8 @@ final class ServerRegistrar return; } + $this->unhookOAuthDefaultServer(); + $result = McpAdapter::instance()->create_server( $args['id'], $args['namespace'], @@ -97,4 +104,36 @@ final class ServerRegistrar error_log($message); // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_error_log } } + + /** + * Removes wp-media/mcp-oauth's own `mcp_adapter_init` callback so it never attempts to + * register its (now redundant) copy of the `mcp-oauth-server` server. + * + * Reads the raw `WP_Hook` for `mcp_adapter_init` from `$GLOBALS['wp_filter']` – there is no + * public API to look up a hooked callback by the class of its bound object, only by the exact + * callable, which we don't have here. Removing a same-action, later-priority callback while an + * earlier-priority one for the same action is running is explicitly supported by + * `WP_Hook::remove_filter()` (it calls `resort_active_iterations()` for exactly this case). + * No-op if the hook has not fired yet, has no callbacks, or the mcp-oauth package is absent. + */ + public function unhookOAuthDefaultServer(): void + { + if (! class_exists(OAuthServerRegistrar::class)) { + return; + } + + $hook = $GLOBALS['wp_filter']['mcp_adapter_init'] ?? null; + if (! is_object($hook) || ! isset($hook->callbacks) || ! is_array($hook->callbacks)) { + return; + } + + foreach ($hook->callbacks as $priority => $callbacks) { + foreach ($callbacks as $callback) { + $function = $callback['function'] ?? null; + if (is_array($function) && ($function[0] ?? null) instanceof OAuthServerRegistrar) { + remove_action('mcp_adapter_init', $function, $priority); + } + } + } + } } diff --git a/tests/Unit/Server/ServerRegistrarTest.php b/tests/Unit/Server/ServerRegistrarTest.php index bc25cda..bd916dd 100644 --- a/tests/Unit/Server/ServerRegistrarTest.php +++ b/tests/Unit/Server/ServerRegistrarTest.php @@ -12,9 +12,16 @@ use LIT\MCP\Integrations\IntegrationRegistry; use LIT\MCP\Server\ServerRegistrar; use LIT\MCP\Tests\Unit\TestCase; use Mockery; +use WPMedia\MCP\OAuth\Transport\ServerRegistrar as OAuthServerRegistrar; final class ServerRegistrarTest extends TestCase { + protected function tearDown(): void + { + unset($GLOBALS['wp_filter']); + parent::tearDown(); + } + private function registry(): IntegrationRegistry { $direct = Mockery::mock(Ability::class); @@ -77,4 +84,37 @@ final class ServerRegistrarTest extends TestCase self::assertFalse($oauthChecked, 'createServer must bail out before touching the adapter'); } + + public function testUnhookOAuthDefaultServerRemovesOnlyTheMcpOauthCallback(): void + { + $oauthRegistrar = Mockery::mock(OAuthServerRegistrar::class); + $oauthCallback = [$oauthRegistrar, 'register_server']; + $unrelatedCallback = static function (): void { + }; + + $hook = new class { + /** @var array> */ + public array $callbacks = []; + }; + $hook->callbacks = [ + 5 => ['unrelated-id' => ['function' => $unrelatedCallback, 'accepted_args' => 0]], + 10 => ['oauth-id' => ['function' => $oauthCallback, 'accepted_args' => 0]], + ]; + $GLOBALS['wp_filter']['mcp_adapter_init'] = $hook; + + Functions\expect('remove_action') + ->once() + ->with('mcp_adapter_init', $oauthCallback, 10); + + (new ServerRegistrar($this->registry(), $this->instructions()))->unhookOAuthDefaultServer(); + } + + public function testUnhookOAuthDefaultServerIsNoOpWhenHookIsMissing(): void + { + unset($GLOBALS['wp_filter']['mcp_adapter_init']); + + Functions\expect('remove_action')->never(); + + (new ServerRegistrar($this->registry(), $this->instructions()))->unhookOAuthDefaultServer(); + } }