Skip to content

Connectors: Use validate_plugin() for the connector install check #11701

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
t-hamano wants to merge 9 commits into
base: trunk
Choose a base branch
from
+57 −40
Open

Connectors: Use validate_plugin() for the connector install check #11701

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
aa8f9f2
Connectors: Defer `plugin.is_active` check to connector retrieval.
t-hamano May 4, 2026
6bcc1b8
Merge branch 'trunk' into 65020-update-is-active-check
t-hamano May 5, 2026
9cb5f08
Connectors: Harden plugin install detection and document `is_active` …
t-hamano May 5, 2026
bcf570c
Connectors: Normalize `plugin.is_active` default at registration.
t-hamano May 6, 2026
a80e2ad
Connectors: Restore tests for `plugin.is_active` default at registrat…
t-hamano May 6, 2026
097a005
Connectors: Correct PHPStan shape for the stored `plugin` array.
t-hamano May 6, 2026
c2bb4fc
Merge branch 'trunk' into 65020-update-is-active-check
t-hamano May 6, 2026
2307bf3
Connectors: Align `$file` assignment with surrounding statements.
t-hamano May 6, 2026
e15d155
Fix type issues identified by PHPStan rule level 10
westonruter May 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 21 additions & 5 deletions src/wp-includes/class-wp-connector-registry.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
*
* @phpstan-type Connector array{
* name: non-empty-string,
* description: non-empty-string,
* description: string,
* logo_url?: non-empty-string,
* type: non-empty-string,
* authentication: array{
Expand All @@ -39,9 +39,9 @@
* constant_name?: non-empty-string,
* env_var_name?: non-empty-string
* },
* plugin?: array{
* file: non-empty-string,
* is_active?: callable(): bool
* plugin: array{
* file?: non-empty-string,
* is_active: callable(): bool
* }
Comment on lines +42 to 45
Copy link
Copy Markdown
Contributor Author

@t-hamano t-hamano May 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Member

@westonruter westonruter May 6, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Member

@westonruter westonruter May 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in e15d155

* }
*/
Expand Down Expand Up @@ -120,7 +120,23 @@ final class WP_Connector_Registry {
* }
* @return array|null The registered connector data on success, null on failure.
*
* @phpstan-param Connector $args
* @phpstan-param array{
* name: non-empty-string,
* description?: 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
* }
* } $args
* @phpstan-return Connector|null
*/
public function register( string $id, array $args ): ?array {
Expand Down
71 changes: 36 additions & 35 deletions src/wp-includes/connectors.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ function wp_is_connector_registered( string $id ): bool {
* }
* @phpstan-return ?array{
* name: non-empty-string,
* description: non-empty-string,
* description: string,
Copy link
Copy Markdown
Member

@westonruter westonruter May 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Member

@westonruter westonruter May 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

* logo_url?: non-empty-string,
* type: non-empty-string,
* authentication: array{
Expand All @@ -75,7 +75,8 @@ function wp_is_connector_registered( string $id ): bool {
* env_var_name?: non-empty-string
* },
* plugin?: array{
* file: non-empty-string
* file?: non-empty-string,
* is_active: callable(): bool,
Comment on lines +78 to +79
Copy link
Copy Markdown
Member

@westonruter westonruter May 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to be consistent with Connector on WP_Connector_Registry.

* }
* }
*/
Expand Down Expand Up @@ -126,7 +127,7 @@ function wp_get_connector( string $id ): ?array {
* }
* @phpstan-return array<string, array{
* name: non-empty-string,
* description: non-empty-string,
* description: string,
* logo_url?: non-empty-string,
* type: non-empty-string,
* authentication: array{
Expand All @@ -137,7 +138,8 @@ function wp_get_connector( string $id ): ?array {
* env_var_name?: non-empty-string
* },
* plugin?: array{
* file: non-empty-string
* file?: non-empty-string,
* is_active: callable(): bool,
Comment on lines +141 to +142
Copy link
Copy Markdown
Member

@westonruter westonruter May 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to be consistent with Connector on WP_Connector_Registry.

* }
* }>
*/
Expand All @@ -160,7 +162,7 @@ function wp_get_connectors(): array {
* @access private
*
* @param string $path Absolute path to the logo file.
* @return string|null The URL to the logo file, or null if the path is invalid.
* @return non-empty-string|null The URL to the logo file, or null if the path is invalid.
*/
function _wp_connectors_resolve_ai_provider_logo_url( string $path ): ?string {
if ( ! $path ) {
Expand All @@ -175,12 +177,14 @@ function _wp_connectors_resolve_ai_provider_logo_url( string $path ): ?string {

$mu_plugin_dir = wp_normalize_path( WPMU_PLUGIN_DIR );
if ( str_starts_with( $path, $mu_plugin_dir . '/' ) ) {
return plugins_url( substr( $path, strlen( $mu_plugin_dir ) ), WPMU_PLUGIN_DIR . '/.' );
$logo_url = plugins_url( substr( $path, strlen( $mu_plugin_dir ) ), WPMU_PLUGIN_DIR . '/.' );
return $logo_url ? $logo_url : null;
}

$plugin_dir = wp_normalize_path( WP_PLUGIN_DIR );
if ( str_starts_with( $path, $plugin_dir . '/' ) ) {
return plugins_url( substr( $path, strlen( $plugin_dir ) ) );
$logo_url = plugins_url( substr( $path, strlen( $plugin_dir ) ) );
return $logo_url ? $logo_url : null;
}

_doing_it_wrong(
Expand Down Expand Up @@ -295,7 +299,7 @@ function _wp_connectors_register_default_ai_providers( WP_Connector_Registry $re
// Registry values (from provider plugins) take precedence over hardcoded fallbacks.
$ai_registry = AiClient::defaultRegistry();

foreach ( $ai_registry->getRegisteredProviderIds() as $connector_id ) {
foreach ( array_filter( $ai_registry->getRegisteredProviderIds() ) as $connector_id ) {
Copy link
Copy Markdown
Member

@westonruter westonruter May 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

$provider_class_name = $ai_registry->getProviderClassName( $connector_id );
$provider_metadata = $provider_class_name::metadata();

Expand All @@ -305,9 +309,11 @@ function _wp_connectors_register_default_ai_providers( WP_Connector_Registry $re
if ( $is_api_key ) {
$credentials_url = $provider_metadata->getCredentialsUrl();
$authentication = array(
'method' => 'api_key',
'credentials_url' => $credentials_url ? $credentials_url : null,
'method' => 'api_key',
);
if ( $credentials_url ) {
$authentication['credentials_url'] = $credentials_url;
Copy link
Copy Markdown
Member

@westonruter westonruter May 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

}
} else {
$authentication = array( 'method' => 'none' );
}
Expand Down Expand Up @@ -340,8 +346,10 @@ function _wp_connectors_register_default_ai_providers( WP_Connector_Registry $re
'description' => $description ? $description : '',
'type' => 'ai_provider',
'authentication' => $authentication,
'logo_url' => $logo_url,
);
if ( $logo_url ) {
$defaults[ $connector_id ]['logo_url'] = $logo_url;
Copy link
Copy Markdown
Member

@westonruter westonruter May 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

}
}
}

Expand All @@ -350,33 +358,22 @@ function _wp_connectors_register_default_ai_providers( WP_Connector_Registry $re
if ( 'api_key' === $args['authentication']['method'] ) {
$sanitized_id = str_replace( '-', '_', $id );

if ( ! isset( $args['authentication']['setting_name'] ) ) {
Copy link
Copy Markdown
Member

@westonruter westonruter May 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Member

@westonruter westonruter May 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

$args['authentication']['setting_name'] = "connectors_ai_{$sanitized_id}_api_key";
}
$args['authentication']['setting_name'] = "connectors_ai_{$sanitized_id}_api_key";

// All AI providers use the {CONSTANT_CASE_ID}_API_KEY naming convention.
if ( ! isset( $args['authentication']['constant_name'] ) || ! isset( $args['authentication']['env_var_name'] ) ) {
$constant_case_key = strtoupper( preg_replace( '/([a-z])([A-Z])/', '$1_$2', $sanitized_id ) ) . '_API_KEY';

if ( ! isset( $args['authentication']['constant_name'] ) ) {
$args['authentication']['constant_name'] = $constant_case_key;
}
$constant_case_key = strtoupper( (string) preg_replace( '/([a-z])([A-Z])/', '$1_$2', $sanitized_id ) ) . '_API_KEY';

if ( ! isset( $args['authentication']['env_var_name'] ) ) {
$args['authentication']['env_var_name'] = $constant_case_key;
}
}
$args['authentication']['constant_name'] = $constant_case_key;
$args['authentication']['env_var_name'] = $constant_case_key;
}

if ( ! isset( $args['plugin']['is_active'] ) ) {
$args['plugin']['is_active'] = static function () use ( $ai_registry, $id ): bool {
try {
return $ai_registry->hasProvider( $id );
} catch ( Exception $e ) {
return false;
}
};
}
$args['plugin']['is_active'] = static function () use ( $ai_registry, $id ): bool {
try {
return $ai_registry->hasProvider( $id );
} catch ( Exception $e ) {
return false;
}
};

$registry->register( $id, $args );
}
Expand Down Expand Up @@ -624,7 +621,7 @@ function _wp_connectors_pass_default_keys_to_ai_client(): void {
}

$api_key = get_option( $auth['setting_name'], '' );
if ( '' === $api_key ) {
if ( ! is_string( $api_key ) || '' === $api_key ) {
continue;
}

Expand All @@ -651,6 +648,10 @@ function _wp_connectors_pass_default_keys_to_ai_client(): void {
function _wp_connectors_get_connector_script_module_data( array $data ): array {
$registry = AiClient::defaultRegistry();

if ( ! function_exists( 'validate_plugin' ) ) {
require_once ABSPATH . 'wp-admin/includes/plugin.php';
}

$connectors = array();
foreach ( wp_get_connectors() as $connector_id => $connector_data ) {
$auth = $connector_data['authentication'];
Expand Down Expand Up @@ -684,7 +685,7 @@ function _wp_connectors_get_connector_script_module_data( array $data ): array {
if ( ! empty( $connector_data['plugin']['file'] ) ) {
$file = $connector_data['plugin']['file'];
$is_activated = (bool) call_user_func( $connector_data['plugin']['is_active'] );
$is_installed = $is_activated || file_exists( wp_normalize_path( WP_PLUGIN_DIR . '/' . $file ) );
$is_installed = $is_activated || 0 === validate_plugin( $file );

$connector_out['plugin'] = array(
'file' => $file,
Expand Down
Loading