diff --git a/src/Admin/ConnectionUsage.php b/src/Admin/ConnectionUsage.php index 8e51666..351e1a8 100644 --- a/src/Admin/ConnectionUsage.php +++ b/src/Admin/ConnectionUsage.php @@ -55,7 +55,7 @@ final class ConnectionUsage public function record(mixed $response, array $handler, WP_REST_Request $request): mixed { $route = '/' . ServerRegistrar::NAMESPACE . '/' . ServerRegistrar::ROUTE; - if ($request->get_route() !== $route || is_wp_error($response)) { + if ($request->get_route() !== $route || ! self::succeeded($response)) { return $response; } @@ -78,4 +78,17 @@ final class ConnectionUsage return $response; } + + /** + * Only successful answers count as use: mcp-oauth answers auth failures with a + * WP_REST_Response(401), not a WP_Error (OAuthHttpTransport::build_auth_error_response). + */ + private static function succeeded(mixed $response): bool + { + if (is_wp_error($response)) { + return false; + } + + return ! $response instanceof \WP_HTTP_Response || $response->get_status() < 400; + } } diff --git a/src/Admin/LogListTable.php b/src/Admin/LogListTable.php index b07c966..ad27788 100644 --- a/src/Admin/LogListTable.php +++ b/src/Admin/LogListTable.php @@ -130,7 +130,7 @@ final class LogListTable extends \WP_List_Table [$icon, $word] = match ($result) { 'ok' => ['yes-alt', __('erfolgreich', 'wp-lit-mcp')], 'partial' => ['warning', __('teilweise', 'wp-lit-mcp')], - default => ['dismiss', $result], + default => ['dismiss', __('unbekannt', 'wp-lit-mcp')], }; return sprintf( diff --git a/src/Admin/ToolsTab.php b/src/Admin/ToolsTab.php index 0008558..8e0be67 100644 --- a/src/Admin/ToolsTab.php +++ b/src/Admin/ToolsTab.php @@ -22,6 +22,13 @@ final class ToolsTab implements AdminTab /** More direct tools than this cost noticeable context in every claude.ai conversation. */ public const DIRECT_WARNING_THRESHOLD = 25; + /** Tools that remove something: "delete" (for good) or "trash" (to the trash). */ + private const DELETING = [ + 'lit/media-delete' => 'delete', + 'lit/divi-delete' => 'delete', + 'lit/post-trash' => 'trash', + ]; + public function __construct(private IntegrationRegistry $registry, private ExposurePolicy $policy) { } @@ -336,12 +343,19 @@ final class ToolsTab implements AdminTab echo ''; } - /** "liest" / "schreibt" / "löscht" from the ability's annotations, with a dashicon. */ + /** + * "liest" / "schreibt" / "überschreibt" / "löscht" / "Papierkorb" with a dashicon. The + * "destructive" annotation is a broad hint (deleting, replacing, swapping, ruling S47), + * so only the tools in DELETING say "löscht"; other destructive tools "überschreiben". + */ private function kind(Ability $ability): string { $annotations = $ability->annotations(); [$icon, $text] = match (true) { - ($annotations['destructive'] ?? false) === true => ['trash', __('löscht', 'wp-lit-mcp')], + isset(self::DELETING[$ability->name()]) => ['trash', self::DELETING[$ability->name()] === 'trash' + ? __('Papierkorb', 'wp-lit-mcp') + : __('löscht', 'wp-lit-mcp')], + ($annotations['destructive'] ?? false) === true => ['update', __('überschreibt', 'wp-lit-mcp')], ($annotations['readonly'] ?? false) === true => ['visibility', __('liest', 'wp-lit-mcp')], default => ['edit', __('schreibt', 'wp-lit-mcp')], }; diff --git a/tests/Unit/Admin/ConnectionUsageTest.php b/tests/Unit/Admin/ConnectionUsageTest.php index 9d60ef6..ff36cef 100644 --- a/tests/Unit/Admin/ConnectionUsageTest.php +++ b/tests/Unit/Admin/ConnectionUsageTest.php @@ -8,7 +8,11 @@ use Brain\Monkey\Functions; use LIT\MCP\Admin\ConnectionUsage; use LIT\MCP\Tests\Unit\TestCase; use WP_Error; +use WP_HTTP_Response; use WP_REST_Request; +use WP_REST_Response; + +require_once __DIR__ . '/Fixtures/HttpResponses.php'; final class ConnectionUsageTest extends TestCase { @@ -64,6 +68,37 @@ final class ConnectionUsageTest extends TestCase self::assertSame([], $this->recorded); } + /** @dataProvider failedStatuses */ + public function testDoesNotRecordRequestsAnsweredWithAnErrorStatus(int $status): void + { + Functions\when('get_current_user_id')->justReturn(7); + $claims = ['sub' => 7, 'app_pass_id' => 'uuid-1']; + // mcp-oauth answers auth failures with a WP_REST_Response (OAuthHttpTransport::build_auth_error_response). + $rest = new WP_REST_Response(['error' => 'invalid_token'], $status); + + self::assertSame($rest, $this->usage($claims)->record($rest, [], $this->request())); + $this->usage($claims)->record(new WP_HTTP_Response(null, $status), [], $this->request()); + + self::assertSame([], $this->recorded); + } + + /** @return list> */ + public static function failedStatuses(): array + { + return [[401], [403], [500]]; + } + + public function testRecordsSuccessfulRestResponses(): void + { + Functions\when('get_current_user_id')->justReturn(7); + $claims = ['sub' => 7, 'app_pass_id' => 'uuid-1']; + + $this->usage($claims)->record(new WP_REST_Response([], 200), [], $this->request()); + $this->usage($claims)->record(new WP_REST_Response([], 202), [], $this->request()); + + self::assertSame([[7, 'uuid-1'], [7, 'uuid-1']], $this->recorded); + } + public function testNeverBreaksTheResponseWhenDecodingOrRecordingFails(): void { Functions\when('get_current_user_id')->justReturn(7); diff --git a/tests/Unit/Admin/Fixtures/HttpResponses.php b/tests/Unit/Admin/Fixtures/HttpResponses.php new file mode 100644 index 0000000..fbc675e --- /dev/null +++ b/tests/Unit/Admin/Fixtures/HttpResponses.php @@ -0,0 +1,25 @@ +status; + } + } +} + +if (! class_exists('WP_REST_Response')) { + class WP_REST_Response extends WP_HTTP_Response + { + } +} diff --git a/tests/Unit/Admin/LogListTableTest.php b/tests/Unit/Admin/LogListTableTest.php index 4b8ebd5..cee62d2 100644 --- a/tests/Unit/Admin/LogListTableTest.php +++ b/tests/Unit/Admin/LogListTableTest.php @@ -218,6 +218,7 @@ final class LogListTableTest extends TestCase self::assertStringContainsString('dashicons-' . $icon, $html); self::assertStringContainsString($word, $html); + self::assertStringNotContainsString('error', $html); } /** @return list> */ @@ -226,7 +227,7 @@ final class LogListTableTest extends TestCase return [ ['ok', 'yes-alt', 'erfolgreich'], ['partial', 'warning', 'teilweise'], - ['error', 'dismiss', 'error<b>'], + ['error', 'dismiss', 'unbekannt'], ]; } diff --git a/tests/Unit/Admin/ToolsTabTest.php b/tests/Unit/Admin/ToolsTabTest.php index 39870ab..7e43185 100644 --- a/tests/Unit/Admin/ToolsTabTest.php +++ b/tests/Unit/Admin/ToolsTabTest.php @@ -255,16 +255,45 @@ final class ToolsTabTest extends TestCase $registry = new IntegrationRegistry($this->integration('media', [ ['lit/media-preview', true, true], ['lit/media-upload', false, false], - ['lit/media-delete', false, false, true], ])); $html = $this->render([], $registry); self::assertStringContainsString('Art', $html); - self::assertStringContainsString('liest', $this->row($html, 'lit/media-preview')); - self::assertStringContainsString('schreibt', $this->row($html, 'lit/media-upload')); - self::assertStringContainsString('löscht', $this->row($html, 'lit/media-delete')); - self::assertStringNotContainsString('schreibt', $this->row($html, 'lit/media-delete')); + self::assertStringContainsString('dashicons-visibility', $this->row($html, 'lit/media-preview')); + self::assertStringContainsString('>liest<', $this->row($html, 'lit/media-preview')); + self::assertStringContainsString('dashicons-edit', $this->row($html, 'lit/media-upload')); + self::assertStringContainsString('>schreibt<', $this->row($html, 'lit/media-upload')); + } + + /** + * "destructive" is a broad hint (deleting, replacing, swapping) – only explicit deletes say "löscht". + * + * @dataProvider destructiveAbilities + */ + public function testArtColumnForEveryDestructiveAbility(string $name, string $icon, string $word): void + { + $registry = new IntegrationRegistry($this->integration('wordpress', [[$name, false, false, true]])); + + $row = $this->row($this->render([], $registry), $name); + + self::assertStringContainsString('dashicons-' . $icon, $row); + self::assertStringContainsString('>' . $word . '<', $row); + } + + /** @return list> */ + public static function destructiveAbilities(): array + { + return [ + ['lit/media-delete', 'trash', 'löscht'], + ['lit/divi-delete', 'trash', 'löscht'], + ['lit/post-trash', 'trash', 'Papierkorb'], + ['lit/post-replace-with-copy', 'update', 'überschreibt'], + ['lit/revision-restore', 'update', 'überschreibt'], + ['lit/menu-set-items', 'update', 'überschreibt'], + ['lit/menu-assign-location', 'update', 'überschreibt'], + ['lit/divi-replace-text', 'update', 'überschreibt'], + ]; } public function testToolCellShowsLabelAndTheNameAsMutedTextWithALabelledSelect(): void