From 169d5772b5701864169a82298a0f888f355aef5a Mon Sep 17 00:00:00 2001 From: Jan-Niclas Loosen Date: Sat, 26 Sep 2026 03:30:01 +0200 Subject: [PATCH] fix: discovery admin notice fires on cached 'recommended' too, guarded against oauth/permalink early returns Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01At3V6SjwbMe6eMzVGW3Hs7 --- src/Admin/AdminNotices.php | 18 +++++- src/Admin/ConnectionStatus.php | 37 +++++++++--- tests/Unit/Admin/AdminNoticesTest.php | 71 +++++++++++++++++++++-- tests/Unit/Admin/ConnectionStatusTest.php | 15 +++++ 4 files changed, 126 insertions(+), 15 deletions(-) diff --git a/src/Admin/AdminNotices.php b/src/Admin/AdminNotices.php index 5282a39..905654f 100644 --- a/src/Admin/AdminNotices.php +++ b/src/Admin/AdminNotices.php @@ -30,8 +30,24 @@ final class AdminNotices ) . ' ' . __('Claude kann sich nicht verbinden.', 'wp-lit-mcp'); } + /* + * Fire whenever the cached discovery result is not 'good' (this also covers + * 'recommended', which is how HealthCheck::run_self_check() itself reports both + * a genuine reachability problem and its acme-challenge-fingerprint "critical" + * case) — but only when oauth is enabled and permalinks are pretty. Those two + * conditions are exactly when run_self_check() short-circuits to its own + * disabled/permalink-missing early returns instead of a real check; guarding on + * them here (rather than trying to recognise their shape from the cached array) + * keeps this correct even though, in the vendor's current implementation, + * neither early return is ever persisted to the transient we read. + */ $discovery = $this->status->discovery(false); - if (is_array($discovery) && ($discovery['status'] ?? 'good') === 'critical') { + if ( + $this->status->oauthEnabled() + && $this->status->permalinksPretty() + && is_array($discovery) + && ($discovery['status'] ?? 'good') !== 'good' + ) { $messages[] = __( 'LIT MCP: Die OAuth-Discovery-Dokumente unter /.well-known/ sind nicht erreichbar.', 'wp-lit-mcp' diff --git a/src/Admin/ConnectionStatus.php b/src/Admin/ConnectionStatus.php index 836a6e0..89fe22e 100644 --- a/src/Admin/ConnectionStatus.php +++ b/src/Admin/ConnectionStatus.php @@ -46,6 +46,17 @@ class ConnectionStatus return str_starts_with((string) home_url(), 'https://'); } + /** + * Whether a pretty permalink structure is active. HealthCheck::run_self_check() + * short-circuits to a 'recommended' result (never persisted to its transient) + * when this is false; used to tell that early return apart from a genuine + * discovery failure without depending on that vendor implementation detail. + */ + public function permalinksPretty(): bool + { + return (string) get_option('permalink_structure') !== ''; + } + /** @return array|null mcp-oauth's discovery self check (label, status, …). */ public function discovery(bool $allowFetch): ?array { @@ -101,28 +112,36 @@ class ConnectionStatus $endpoint = $this->endpoint($allowFetch); return [ - ['label' => 'MCP-Adapter', 'ok' => $adapter, 'message' => $adapter ? 'geladen' : 'fehlt'], [ - 'label' => 'OAuth-Server', + 'label' => __('MCP-Adapter', 'wp-lit-mcp'), + 'ok' => $adapter, + 'message' => $adapter ? __('geladen', 'wp-lit-mcp') : __('fehlt', 'wp-lit-mcp'), + ], + [ + 'label' => __('OAuth-Server', 'wp-lit-mcp'), 'ok' => $oauth, - 'message' => $oauth ? 'aktiv' : 'deaktiviert (Filter wpmedia_mcp_oauth_server_enabled)', + 'message' => $oauth + ? __('aktiv', 'wp-lit-mcp') + : __('deaktiviert (Filter wpmedia_mcp_oauth_server_enabled)', 'wp-lit-mcp'), ], [ - 'label' => 'HTTPS', + 'label' => __('HTTPS', 'wp-lit-mcp'), 'ok' => $https, - 'message' => $https ? 'aktiv' : 'Website läuft ohne HTTPS – claude.ai verbindet sich nur über HTTPS', + 'message' => $https + ? __('aktiv', 'wp-lit-mcp') + : __('Website läuft ohne HTTPS – claude.ai verbindet sich nur über HTTPS', 'wp-lit-mcp'), ], [ - 'label' => 'Discovery (/.well-known/)', + 'label' => __('Discovery (/.well-known/)', 'wp-lit-mcp'), 'ok' => $discovery === null ? null : ($discovery['status'] ?? '') === 'good', 'message' => $discovery === null - ? 'noch nicht geprüft' + ? __('noch nicht geprüft', 'wp-lit-mcp') : wp_strip_all_tags((string) ($discovery['label'] ?? '')), ], [ - 'label' => 'Endpunkt ohne Token', + 'label' => __('Endpunkt ohne Token', 'wp-lit-mcp'), 'ok' => $endpoint['ok'] ?? null, - 'message' => $endpoint['message'] ?? 'noch nicht geprüft', + 'message' => $endpoint['message'] ?? __('noch nicht geprüft', 'wp-lit-mcp'), ], ]; } diff --git a/tests/Unit/Admin/AdminNoticesTest.php b/tests/Unit/Admin/AdminNoticesTest.php index d1a3312..e3151e1 100644 --- a/tests/Unit/Admin/AdminNoticesTest.php +++ b/tests/Unit/Admin/AdminNoticesTest.php @@ -18,23 +18,84 @@ final class AdminNoticesTest extends TestCase Functions\stubTranslationFunctions(); } - private function status(bool $oauth, ?array $discovery, bool $https): ConnectionStatus + /** + * The array shape WPMedia\MCP\OAuth\Auth\Discovery\HealthCheck::build_result() + * stores in its transient for a genuine (not early-return) check — see + * vendor/wp-media/mcp-oauth/inc/Auth/Discovery/HealthCheck.php. Its 'critical' + * internal classification (the acme-challenge fingerprint) is itself reported + * here as 'recommended' (never 'critical') by that class's own downgrade. + * + * @return array + */ + private function realDiscoveryResult(string $status): array + { + return [ + 'label' => 'MCP OAuth discovery documents', + 'status' => $status, + 'badge' => ['label' => 'Configuration', 'color' => 'blue'], + 'description' => '

The following .well-known discovery document(s) did not respond as expected: ' + . 'oauth-protected-resource.

', + 'actions' => '

Check your error/access logs for the requests to these paths…

', + 'test' => 'wpmedia_mcp_oauth_wellknown_discovery', + ]; + } + + private function status(bool $oauth, ?array $discovery, bool $https, bool $permalinksPretty = true): ConnectionStatus { $status = Mockery::mock(ConnectionStatus::class); $status->allows('oauthEnabled')->andReturn($oauth); $status->allows('discovery')->with(false)->andReturn($discovery); $status->allows('https')->andReturn($https); + $status->allows('permalinksPretty')->andReturn($permalinksPretty); return $status; } - public function testWarnsAboutDisabledOAuthAndBrokenDiscoveryEverywhere(): void + public function testWarnsAboutDisabledOAuth(): void { - $messages = (new AdminNotices($this->status(false, ['status' => 'critical'], true)))->messages(false); + $messages = (new AdminNotices($this->status(false, null, true)))->messages(false); - self::assertCount(2, $messages); + self::assertCount(1, $messages); self::assertStringContainsString('OAuth-Server ist deaktiviert', $messages[0]); - self::assertStringContainsString('/.well-known/', $messages[1]); + } + + public function testWarnsWhenTheCachedDiscoveryStatusIsCriticalOrRecommended(): void + { + foreach (['critical', 'recommended'] as $status) { + $messages = (new AdminNotices($this->status(true, $this->realDiscoveryResult($status), true))) + ->messages(false); + + self::assertCount(1, $messages, "status={$status}"); + self::assertStringContainsString('/.well-known/', $messages[0]); + } + } + + public function testGoodCachedDiscoveryNeverWarns(): void + { + $notices = new AdminNotices($this->status(true, $this->realDiscoveryResult('good'), true)); + + self::assertSame([], $notices->messages(false)); + } + + public function testDoesNotAddASeparateDiscoveryWarningWhenOAuthIsDisabled(): void + { + // HealthCheck::run_self_check()'s 'disabled' early return always reports status + // 'good' and (like the permalink one below) is never persisted to its transient — + // but we guard on the live oauthEnabled() state too rather than relying on that + // vendor implementation detail. A stale 'critical' result cached from before OAuth + // was disabled must not add a second, redundant notice. + $notices = new AdminNotices($this->status(false, $this->realDiscoveryResult('critical'), true)); + + $messages = $notices->messages(false); + self::assertCount(1, $messages); + self::assertStringContainsString('OAuth-Server ist deaktiviert', $messages[0]); + } + + public function testDoesNotWarnAboutDiscoveryWithoutPrettyPermalinks(): void + { + $notices = new AdminNotices($this->status(true, $this->realDiscoveryResult('recommended'), true, false)); + + self::assertSame([], $notices->messages(false)); } public function testHttpsHintOnlyOnTheSettingsPage(): void diff --git a/tests/Unit/Admin/ConnectionStatusTest.php b/tests/Unit/Admin/ConnectionStatusTest.php index c6cff7d..c8fb32e 100644 --- a/tests/Unit/Admin/ConnectionStatusTest.php +++ b/tests/Unit/Admin/ConnectionStatusTest.php @@ -11,6 +11,12 @@ use Mockery; final class ConnectionStatusTest extends TestCase { + protected function setUp(): void + { + parent::setUp(); + Functions\stubTranslationFunctions(); + } + public function testEndpointCheckExpects401WithBearerChallengeAndCachesIt(): void { Functions\when('get_transient')->justReturn(false); @@ -56,4 +62,13 @@ final class ConnectionStatusTest extends TestCase self::assertNull($byLabel['Discovery (/.well-known/)']['ok']); self::assertNull($byLabel['Endpunkt ohne Token']['ok']); } + + public function testPermalinksPrettyReflectsThePermalinkStructureOption(): void + { + Functions\when('get_option')->justReturn('/%postname%/'); + self::assertTrue((new ConnectionStatus())->permalinksPretty()); + + Functions\when('get_option')->justReturn(''); + self::assertFalse((new ConnectionStatus())->permalinksPretty()); + } }