Connectors: Use validate_plugin() for the connector install check by t-hamano · Pull Request #11701 · WordPress/wordpress-develop

t-hamano · 2026-05-04T06:20:47Z

This PR backports the changes from Gutenberg to the core: WordPress/gutenberg#77897

Trac ticket: https://core.trac.wordpress.org/ticket/65020

Use of AI Tools

None

This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

Previously, `WP_Connector_Registry::register()` silently filled in `__return_true` when no `is_active` callback was provided, which made it impossible for consumers to distinguish "no callback supplied" from "callback always returns true." Defer the check to call sites instead, so the stored connector data reflects what was actually registered. Update `_wp_register_default_connector_settings()` and `_wp_connectors_get_connector_script_module_data()` to treat a missing callback as active, matching the documented `Defaults to __return_true` semantics. See #65020. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

github-actions · 2026-05-04T06:24:51Z

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

All changes will be lost when closing a tab with a Playground instance.
All changes will be lost when refreshing the page.
A fresh instance is created each time the link below is clicked.
Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

mukeshpanchal27 · 2026-05-04T06:35:29Z

-		if ( ! isset( $connector['plugin']['is_active'] ) ) {
-			$connector['plugin']['is_active'] = '__return_true';
-		}
-


Remove Defaults to __return_true. from line no 118

github-actions · 2026-05-05T08:30:43Z

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props wildworks, westonruter, mukesh27, jorgefilipecosta.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

…default. Replace the raw `file_exists()` install probe in `_wp_connectors_get_connector_script_module_data()` with `validate_plugin()`, which adds path traversal protection and confirms the file is a recognised plugin rather than just any file at that path. Restores the `require_once` for `wp-admin/includes/plugin.php` that the function depends on. Also updates the `is_active` docblock in `WP_Connector_Registry::register()` to spell out that an omitted callback paired with a `file` is treated as active on the assumption that the plugin must be loaded in order to register itself, so callers do not mistake the omission for a no-op. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

westonruter · 2026-05-05T23:43:51Z

See feedback from @jorgefilipecosta at https://github.com/WordPress/gutenberg/pull/77897/changes#r3183140237 and https://github.com/WordPress/gutenberg/pull/77897/changes#r3189900673

If is_active is always then set among the \WP_Connector_Registry::$registered_connectors, then the Connector type modified in b3b40be should be updated as follows:

diff --git a/src/wp-includes/class-wp-connector-registry.php b/src/wp-includes/class-wp-connector-registry.php
index fbf35ad73e..4fd451b2b0 100644
--- a/src/wp-includes/class-wp-connector-registry.php
+++ b/src/wp-includes/class-wp-connector-registry.php
@@ -41,7 +41,7 @@
  *     },
  *     plugin?: array{
  *         file: non-empty-string,
- *         is_active?: callable(): bool
+ *         is_active: callable(): bool
  *     }
  * }
  */

Move the missing-callback fallback from `_wp_connectors_get_connector_script_module_data()` into `WP_Connector_Registry::register()`, where the registered connector now always has `plugin.is_active` set to `__return_true` when omitted. This lets every consumer call the callback unconditionally instead of repeating an `isset()` guard, and tightens the PHPStan shape so `is_active` is no longer optional on the stored array. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…ion. Follow-up to the previous commit, which moved the `__return_true` fallback back into `WP_Connector_Registry::register()`. Restore the test assertions so they once again expect `plugin.is_active` to be set to `__return_true` when a callback is omitted, matching the registered shape. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

t-hamano · 2026-05-06T02:14:06Z

Based on the feedback in Gutenberg, I have updated this PR. Ultimately, this PR only leaves minor code quality improvements 😄

`WP_Connector_Registry::register()` always initializes `$connector['plugin']` as an array and always defaults `is_active` to `__return_true`, while `file` is only set when supplied. Reflect that in the type: `plugin` is required on the stored connector and `file` is the optional key, not the other way around. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

t-hamano · 2026-05-06T02:18:18Z

+ *     plugin: array{
+ *         file?: non-empty-string,
+ *         is_active: callable(): bool
 *     }


I believe this is the correct type to match the current specifications. The plugins should always be an array, and the file field should be optional.

There is a small discrepancy here, it seems. Namely, the the register() method says that $args is a Connector. But this isn't true, because for Connector the plugin key is optional and both the file and is_active keys are optional. I suggest we replace this line:

* @phpstan-param Connector $args

With the actual shape that is allowed to be passed into register() for $args.

Addressed in e15d155

jorgefilipecosta

The changes look good, thank you @t-hamano!

Why: The variable was flagged by `Generic.Formatting.MultipleStatementAlignment` because the equals sign was not aligned with the adjacent `$is_activated` and `$is_installed` assignments. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

westonruter · 2026-05-06T20:11:30Z

This was not committed yet.

westonruter · 2026-05-06T21:19:56Z

I found several PHPStan issues at rule level 10 when looking at src/wp-includes/class-wp-connector-registry.php and src/wp-includes/connectors.php:

 ------ ----------------------------------------------------------------------- 
  Line   connectors.php                                                         
 ------ ----------------------------------------------------------------------- 
  88     Function wp_get_connector() should return array{name:                  
         non-empty-string, description: non-empty-string, logo_url?:            
         non-empty-string, type: non-empty-string, authentication:              
         array{method: 'api_key'|'none', credentials_url?: non-empty-string,    
         setting_name?: non-empty-string, constant_name?: non-empty-string,     
         env_var_name?: non-empty-string}, plugin?: array{file:                 
         non-empty-string}}|null but returns array{name: non-empty-string,      
         description: non-empty-string, logo_url?: non-empty-string, type:      
         non-empty-string, authentication: array{method: 'api_key'|'none',      
         credentials_url?: non-empty-string, setting_name?: non-empty-string,   
         constant_name?: non-empty-string, env_var_name?: non-empty-string},    
         plugin: array{file?: non-empty-string, is_active: callable():          
         bool}}|null.                                                           
         🪪  return.type                                                        
         💡  Type #1 from the union: Offset 'plugin' (array{file:               
         non-empty-string}) does not accept type array{file?:                   
         non-empty-string, is_active: callable(): bool}: Array might not have   
         offset 'file'.                                                         
         at src/wp-includes/connectors.php:88                                   
  150    Function wp_get_connectors() should return array<string, array{name:   
         non-empty-string, description: non-empty-string, logo_url?:            
         non-empty-string, type: non-empty-string, authentication:              
         array{method: 'api_key'|'none', credentials_url?: non-empty-string,    
         setting_name?: non-empty-string, constant_name?: non-empty-string,     
         env_var_name?: non-empty-string}, plugin?: array{file:                 
         non-empty-string}}> but returns array<string, array{name: non-empty-s  
         tring, description: non-empty-string, logo_url?: non-empty-string,     
         type: non-empty-string, authentication: array{method:                  
         'api_key'|'none', credentials_url?: non-empty-string, setting_name?:   
         non-empty-string, constant_name?: non-empty-string, env_var_name?:     
         non-empty-string}, plugin: array{file?: non-empty-string, is_active:   
         callable(): bool}}>.                                                   
         🪪  return.type                                                        
         💡  Offset 'plugin' (array{file: non-empty-string}) does not accept    
         type array{file?: non-empty-string, is_active: callable(): bool}:      
         Array might not have offset 'file'.                                    
         at src/wp-includes/connectors.php:150                                  
  353    Offset 'setting_name' on array{method: 'api_key'|'none',               
         credentials_url?: mixed} in isset() does not exist.                    
         🪪  isset.offset                                                       
         at src/wp-includes/connectors.php:353                                  
  358    Offset 'constant_name' on array{method: 'api_key'|'none',              
         credentials_url?: mixed, setting_name: non-falsy-string} in isset()    
         does not exist.                                                        
         🪪  isset.offset                                                       
         at src/wp-includes/connectors.php:358                                  
  358    Offset 'env_var_name' on *NEVER* in isset() always exists and is not   
         nullable.                                                              
         🪪  isset.offset                                                       
         at src/wp-includes/connectors.php:358                                  
  358    Result of || is always true.                                           
         🪪  booleanOr.alwaysTrue                                               
         at src/wp-includes/connectors.php:358                                  
  359    Parameter #1 $str of function strtoupper expects string, string|null   
         given.                                                                 
         🪪  argument.type                                                      
         at src/wp-includes/connectors.php:359                                  
  361    Offset 'constant_name' on array{method: 'api_key'|'none',              
         credentials_url?: mixed, setting_name: non-falsy-string} in isset()    
         does not exist.                                                        
         🪪  isset.offset                                                       
         at src/wp-includes/connectors.php:361                                  
  365    Offset 'env_var_name' on array{method: 'api_key'|'none',               
         credentials_url?: mixed, setting_name: non-falsy-string,               
         constant_name: non-falsy-string} in isset() does not exist.            
         🪪  isset.offset                                                       
         at src/wp-includes/connectors.php:365                                  
  371    Offset 'is_active' on array{file: 'ai-provider-for…'} in isset() does  
         not exist.                                                             
         🪪  isset.offset                                                       
         at src/wp-includes/connectors.php:371                                  
  381    Parameter #2 $args of method WP_Connector_Registry::register()         
         expects array{name: non-empty-string, description: non-empty-string,   
         logo_url?: non-empty-string, type: non-empty-string, authentication:   
         array{method: 'api_key'|'none', credentials_url?: non-empty-string,    
         setting_name?: non-empty-string, constant_name?: non-empty-string,     
         env_var_name?: non-empty-string}, plugin: array{file?:                 
         non-empty-string, is_active: callable(): bool}}, array{name: string,   
         description: string, type: 'ai_provider', plugin: array{file?:         
         'ai-provider-for…', is_active: Closure(): bool}, authentication:       
         array{method: 'api_key', credentials_url?: non-falsy-string|null,      
         setting_name: non-falsy-string, constant_name: non-falsy-string,       
         env_var_name: non-falsy-string}|array{method: 'none',                  
         credentials_url?: non-falsy-string|null}, logo_url?: string|null}      
         given.                                                                 
         🪪  argument.type                                                      
         💡  Offset 'name' (non-empty-string) does not accept type string.      
         💡  Offset 'description' (non-empty-string) does not accept type       
         string.                                                                
         💡  Offset 'logo_url' (non-empty-string) does not accept type          
         string|null.                                                           
         💡  Offset 'authentication' (array{method: 'api_key'|'none',           
         credentials_url?: non-empty-string, setting_name?: non-empty-string,   
         constant_name?: non-empty-string, env_var_name?: non-empty-string})    
         does not accept type array{method: 'api_key', credentials_url?:        
         non-falsy-string|null, setting_name: non-falsy-string, constant_name:  
         non-falsy-string, env_var_name: non-falsy-string}|array{method:        
         'none', credentials_url?: non-falsy-string|null}: Offset               
         'credentials_url' (non-empty-string) does not accept type              
         non-falsy-string|null.                                                 
         at src/wp-includes/connectors.php:381                                  
  581    Parameter #2 ...$values of function sprintf expects                    
         bool|float|int|string|null, mixed given.                               
         🪪  argument.type                                                      
         at src/wp-includes/connectors.php:581                                  
  586    Parameter #2 ...$values of function sprintf expects                    
         bool|float|int|string|null, mixed given.                               
         🪪  argument.type                                                      
         at src/wp-includes/connectors.php:586                                  
  633    Parameter #1 $apiKey of class                                          
         WordPress\AiClient\Providers\Http\DTO\ApiKeyRequestAuthentication      
         constructor expects string, mixed given.                               
         🪪  argument.type                                                      
         at src/wp-includes/connectors.php:633                                  
  690    Offset 'is_active' does not exist on array{file: non-falsy-string}.    
         🪪  offsetAccess.notFound                                              
         at src/wp-includes/connectors.php:690                                  
  690    Parameter #1 $function of function call_user_func expects callable():  
         mixed, mixed given.                                                    
         🪪  argument.type                                                      
         at src/wp-includes/connectors.php:690                                  
 ------ ----------------------------------------------------------------------- 


 [ERROR] Found 16 errors

I've fixed these in e15d155 by amending this PR.

westonruter · 2026-05-06T21:21:06Z

+ *         file?: non-empty-string,
+ *         is_active: callable(): bool,


Updated to be consistent with Connector on WP_Connector_Registry.

westonruter · 2026-05-06T21:21:10Z

+ *         file?: non-empty-string,
+ *         is_active: callable(): bool,


Updated to be consistent with Connector on WP_Connector_Registry.

westonruter · 2026-05-06T21:21:59Z

 * @phpstan-return ?array{
 *     name: non-empty-string,
- *     description: non-empty-string,
+ *     description: string,


Because the default is an empty string.

However, it would seem preferable to remain non-empty-string and to instead have the property be optional as with the other properties.

For reference, the underlying description is nullable: https://github.com/WordPress/php-ai-client/blob/631704201d15ffeff7091ad3bc7156db74054956/src/Providers/DTO/ProviderMetadata.php#L57

westonruter · 2026-05-06T21:44:59Z

 	$ai_registry = AiClient::defaultRegistry();

-	foreach ( $ai_registry->getRegisteredProviderIds() as $connector_id ) {
+	foreach ( array_filter( $ai_registry->getRegisteredProviderIds() ) as $connector_id ) {


This is because returns list<string> and not list<non-empty-string>. The array_filter() can be removed with an upstream fix: WordPress/php-ai-client#232

westonruter · 2026-05-06T21:45:39Z

+				'method' => 'api_key',
 			);
+			if ( $credentials_url ) {
+				$authentication['credentials_url'] = $credentials_url;


This is because credentials_url is either a non-empty-string or undefined. It doesn't accept null.

westonruter · 2026-05-06T21:46:11Z

-				'logo_url'       => $logo_url,
 			);
+			if ( $logo_url ) {
+				$defaults[ $connector_id ]['logo_url'] = $logo_url;


Again, because the type is non-empty-string when provided, not non-empty-string|null

westonruter · 2026-05-06T21:47:21Z

 		if ( 'api_key' === $args['authentication']['method'] ) {
 			$sanitized_id = str_replace( '-', '_', $id );

-			if ( ! isset( $args['authentication']['setting_name'] ) ) {


For this change and the other changes in this block, the if conditions were removed. This is because it was checking isset() for array keys which are never set on $defaults.

(Suppress whitespace changes to make the review of the diff easier.)

westonruter · 2026-05-06T21:52:53Z

For reference, the command I used to obtain the above PHPStan errors:

composer phpstan -- --configuration=phpstan.neon.dist --level=10 src/wp-includes/class-wp-connector-registry.php src/wp-includes/connectors.php

t-hamano mentioned this pull request May 4, 2026

Connectors: Add is_active callback support to plugin registration WordPress/gutenberg#77897

Open

mukeshpanchal27 reviewed May 4, 2026

View reviewed changes

t-hamano marked this pull request as ready for review May 5, 2026 08:30

t-hamano and others added 2 commits May 5, 2026 18:47

Merge branch 'trunk' into 65020-update-is-active-check

6bcc1b8

t-hamano and others added 2 commits May 6, 2026 11:11

t-hamano changed the title ~~Connectors: Stop auto-defaulting plugin.is_active in the registry~~ Connectors: Use validate_plugin() for the connector install check May 6, 2026

t-hamano commented May 6, 2026

View reviewed changes

jorgefilipecosta approved these changes May 6, 2026

View reviewed changes

mukeshpanchal27 reviewed May 6, 2026

View reviewed changes

Comment thread src/wp-includes/connectors.php Outdated

t-hamano and others added 2 commits May 6, 2026 17:58

Merge branch 'trunk' into 65020-update-is-active-check

c2bb4fc

This comment was marked as off-topic.

Sign in to view

github-actions Bot closed this May 6, 2026

westonruter reopened this May 6, 2026

Fix type issues identified by PHPStan rule level 10

e15d155

westonruter reviewed May 6, 2026

View reviewed changes

westonruter mentioned this pull request May 6, 2026

Use non-empty-string instead of string for Provider properties WordPress/php-ai-client#232

Open

westonruter reviewed May 6, 2026

View reviewed changes

Conversation

t-hamano commented May 4, 2026

Use of AI Tools

Uh oh!

github-actions Bot commented May 4, 2026

Test using WordPress Playground

Some things to be aware of

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented May 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

westonruter commented May 5, 2026

Uh oh!

t-hamano commented May 6, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

westonruter May 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jorgefilipecosta left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

This comment was marked as off-topic.

westonruter commented May 6, 2026

Uh oh!

westonruter commented May 6, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

westonruter commented May 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

github-actions Bot commented May 5, 2026 •

edited

Loading

westonruter May 6, 2026 •

edited

Loading