diff --git a/src/Fields/OrganicField.php b/src/Fields/OrganicField.php index fbc51869..ec9f7779 100644 --- a/src/Fields/OrganicField.php +++ b/src/Fields/OrganicField.php @@ -6,6 +6,8 @@ use Binaryk\LaravelRestify\MCP\Requests\McpIndexRequest; use Binaryk\LaravelRestify\MCP\Requests\McpRequestable; use Binaryk\LaravelRestify\MCP\Requests\McpShowRequest; +use Binaryk\LaravelRestify\MCP\Requests\McpStoreRequest; +use Binaryk\LaravelRestify\MCP\Requests\McpUpdateRequest; use Binaryk\LaravelRestify\Traits\ProxiesCanSeeToGate; use Closure; use Illuminate\Http\Request; @@ -148,11 +150,16 @@ public function isHiddenOnIndex(RestifyRequest $request, $repository): bool public function isShownOnMcp(RestifyRequest $request, $repository): bool { - if ($this->isHidden($request)) { + // Check if field is hidden from MCP + if ($this->isHiddenFromMcp($request, $repository)) { return false; } - if ($this->isHiddenFromMcp($request, $repository)) { + // For store and update requests, hidden fields should still be writable + // So we check isHidden() only for show/index requests + $isReadOperation = $request instanceof McpShowRequest || $request instanceof McpIndexRequest; + + if ($isReadOperation && $this->isHidden($request)) { return false; } @@ -160,12 +167,32 @@ public function isShownOnMcp(RestifyRequest $request, $repository): bool return call_user_func($this->showOnMcp, $request, $repository); } + // For MCP show requests, check the base showOnShow property without recursion if ($request instanceof McpShowRequest) { - return $this->isHiddenOnShow($request, $repository) === false; + if (is_callable($this->showOnShow)) { + $this->showOnShow = call_user_func($this->showOnShow, $request, $repository); + } + + return $this->showOnShow; } + // For MCP index requests, check the base showOnIndex property without recursion if ($request instanceof McpIndexRequest) { - return $this->isHiddenOnIndex($request, $repository) === false; + if (is_callable($this->showOnIndex)) { + $this->showOnIndex = call_user_func($this->showOnIndex, $request, $repository); + } + + return $this->showOnIndex; + } + + // For MCP store requests, check if field is not readonly + if ($request instanceof McpStoreRequest) { + return ! $this->isReadonly($request); + } + + // For MCP update requests, check if field is not readonly + if ($request instanceof McpUpdateRequest) { + return ! $this->isReadonly($request); } return $this->showOnMcp; @@ -267,21 +294,41 @@ public function isReadonly(RestifyRequest $request) public function isShownOnUpdate(RestifyRequest $request, $repository): bool { + // Check MCP-specific visibility for MCP requests + if ($request instanceof McpRequestable) { + return $this->isShownOnMcp($request, $repository); + } + return ! $this->isReadonly($request); } public function isShownOnUpdateBulk(RestifyRequest $request, $repository): bool { + // Check MCP-specific visibility for MCP requests + if ($request instanceof McpRequestable) { + return $this->isShownOnMcp($request, $repository); + } + return ! $this->isReadonly($request); } public function isShownOnStore(RestifyRequest $request, $repository): bool { + // Check MCP-specific visibility for MCP requests + if ($request instanceof McpRequestable) { + return $this->isShownOnMcp($request, $repository); + } + return ! $this->isReadonly($request); } public function isShownOnStoreBulk(RestifyRequest $request, $repository): bool { + // Check MCP-specific visibility for MCP requests + if ($request instanceof McpRequestable) { + return $this->isShownOnMcp($request, $repository); + } + return ! $this->isReadonly($request); } diff --git a/src/MCP/Tools/Wrapper/ExecuteOperationTool.php b/src/MCP/Tools/Wrapper/ExecuteOperationTool.php index 4eb15aac..4b0e8bac 100644 --- a/src/MCP/Tools/Wrapper/ExecuteOperationTool.php +++ b/src/MCP/Tools/Wrapper/ExecuteOperationTool.php @@ -38,8 +38,7 @@ public function schema(JsonSchema $schema): array ->description('Required only for "action" and "getter" operation types. The URI key of the specific action or getter to execute.'), 'parameters' => $schema->object() - ->description('The parameters to pass to the operation. The required parameters depend on the operation type. Use get-operation-details to see the schema for the specific operation.') - ->required(), + ->description('The parameters to pass to the operation. The required parameters depend on the operation type. Use get-operation-details to see the schema for the specific operation.'), ]; } diff --git a/tests/Fields/FieldMcpVisibilityTest.php b/tests/Fields/FieldMcpVisibilityTest.php index 38bf5e76..428e5bc4 100644 --- a/tests/Fields/FieldMcpVisibilityTest.php +++ b/tests/Fields/FieldMcpVisibilityTest.php @@ -7,6 +7,9 @@ use Binaryk\LaravelRestify\Http\Requests\RestifyRequest; use Binaryk\LaravelRestify\MCP\Requests\McpIndexRequest; use Binaryk\LaravelRestify\MCP\Requests\McpRequest; +use Binaryk\LaravelRestify\MCP\Requests\McpShowRequest; +use Binaryk\LaravelRestify\MCP\Requests\McpStoreRequest; +use Binaryk\LaravelRestify\MCP\Requests\McpUpdateRequest; use Binaryk\LaravelRestify\Tests\Fixtures\Post\PostRepository; use Binaryk\LaravelRestify\Tests\IntegrationTestCase; @@ -172,4 +175,129 @@ public function test_field_collection_preserves_other_filters_with_mcp(): void $this->assertNotContains('mcp_hidden', $indexFieldNames); $this->assertNotContains('eager_field', $indexFieldNames); // EagerFields excluded from index } + + public function test_mcp_show_request_does_not_cause_infinite_loop(): void + { + $fields = new FieldCollection([ + Field::make('title'), + Field::make('secret_key')->hideFromMcp(), + Field::make('visible_field')->showOnShow(true), + ]); + + // This test ensures that McpShowRequest doesn't cause infinite recursion + // between isShownOnMcp() and isShownOnShow() methods + $mcpShowRequest = new McpShowRequest; + + // This should not cause infinite loop + $showFields = $fields->forShow($mcpShowRequest, $this->repository); + $fieldNames = $showFields->map(fn ($field) => $field->getAttribute())->toArray(); + + $this->assertContains('title', $fieldNames); + $this->assertNotContains('secret_key', $fieldNames); // Hidden from MCP + $this->assertContains('visible_field', $fieldNames); + } + + public function test_mcp_index_request_does_not_cause_infinite_loop(): void + { + $fields = new FieldCollection([ + Field::make('name'), + Field::make('internal_token')->hideFromMcp(), + Field::make('public_data')->showOnIndex(true), + ]); + + // This test ensures that McpIndexRequest doesn't cause infinite recursion + $mcpIndexRequest = new McpIndexRequest; + + // This should not cause infinite loop + $indexFields = $fields->forIndex($mcpIndexRequest, $this->repository); + $fieldNames = $indexFields->map(fn ($field) => $field->getAttribute())->toArray(); + + $this->assertContains('name', $fieldNames); + $this->assertNotContains('internal_token', $fieldNames); // Hidden from MCP + $this->assertContains('public_data', $fieldNames); + } + + public function test_mcp_store_request_respects_hide_from_mcp(): void + { + $fields = new FieldCollection([ + Field::make('title'), + Field::make('secret_api_key')->hideFromMcp(), + Field::make('content'), + Field::make('internal_metadata')->hideFromMcp(function ($request) { + return ! $request->get('is_admin', false); + }), + ]); + + // MCP store request without admin + $mcpStoreRequest = new McpStoreRequest; + $storeFields = $fields->forStore($mcpStoreRequest, $this->repository); + $fieldNames = $storeFields->map(fn ($field) => $field->getAttribute())->toArray(); + + $this->assertContains('title', $fieldNames); + $this->assertNotContains('secret_api_key', $fieldNames); // Hidden from MCP + $this->assertContains('content', $fieldNames); + $this->assertNotContains('internal_metadata', $fieldNames); // Hidden by callback + + // MCP store request with admin + $mcpStoreRequestAdmin = new McpStoreRequest(['is_admin' => true]); + $storeFieldsAdmin = $fields->forStore($mcpStoreRequestAdmin, $this->repository); + $fieldNamesAdmin = $storeFieldsAdmin->map(fn ($field) => $field->getAttribute())->toArray(); + + $this->assertContains('title', $fieldNamesAdmin); + $this->assertNotContains('secret_api_key', $fieldNamesAdmin); // Still hidden from MCP + $this->assertContains('content', $fieldNamesAdmin); + $this->assertContains('internal_metadata', $fieldNamesAdmin); // Visible to admin + + // Regular store request should show all fields except readonly + $regularStoreRequest = new RestifyRequest; + $regularStoreFields = $fields->forStore($regularStoreRequest, $this->repository); + $regularFieldNames = $regularStoreFields->map(fn ($field) => $field->getAttribute())->toArray(); + + $this->assertContains('title', $regularFieldNames); + $this->assertContains('secret_api_key', $regularFieldNames); // Visible in regular request + $this->assertContains('content', $regularFieldNames); + $this->assertContains('internal_metadata', $regularFieldNames); + } + + public function test_mcp_update_request_respects_hide_from_mcp(): void + { + $fields = new FieldCollection([ + Field::make('name'), + Field::make('password')->hideFromMcp(), + Field::make('email'), + Field::make('admin_notes')->hideFromMcp(function ($request) { + return $request->get('role') !== 'admin'; + }), + ]); + + // MCP update request without admin role + $mcpUpdateRequest = new McpUpdateRequest(['role' => 'user']); + $updateFields = $fields->forUpdate($mcpUpdateRequest, $this->repository); + $fieldNames = $updateFields->map(fn ($field) => $field->getAttribute())->toArray(); + + $this->assertContains('name', $fieldNames); + $this->assertNotContains('password', $fieldNames); // Hidden from MCP + $this->assertContains('email', $fieldNames); + $this->assertNotContains('admin_notes', $fieldNames); // Hidden by callback + + // MCP update request with admin role + $mcpUpdateRequestAdmin = new McpUpdateRequest(['role' => 'admin']); + $updateFieldsAdmin = $fields->forUpdate($mcpUpdateRequestAdmin, $this->repository); + $fieldNamesAdmin = $updateFieldsAdmin->map(fn ($field) => $field->getAttribute())->toArray(); + + $this->assertContains('name', $fieldNamesAdmin); + $this->assertNotContains('password', $fieldNamesAdmin); // Still hidden from MCP + $this->assertContains('email', $fieldNamesAdmin); + $this->assertContains('admin_notes', $fieldNamesAdmin); // Visible to admin + + // Regular update request should show all fields except readonly + $regularUpdateRequest = new RestifyRequest; + $regularUpdateFields = $fields->forUpdate($regularUpdateRequest, $this->repository); + $regularFieldNames = $regularUpdateFields->map(fn ($field) => $field->getAttribute())->toArray(); + + $this->assertContains('name', $regularFieldNames); + $this->assertContains('password', $regularFieldNames); // Visible in regular request + $this->assertContains('email', $regularFieldNames); + $this->assertContains('admin_notes', $regularFieldNames); + } }