Connectors: Add wp_connectors_settings filter for extensibility#11175
Connectors: Add wp_connectors_settings filter for extensibility#11175gziolo wants to merge 7 commits intoWordPress:trunkfrom
wp_connectors_settings filter for extensibility#11175Conversation
|
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 Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
wp_connectors_settings filter for extensibility
55f0e81 to
555bb8a
Compare
Test using WordPress PlaygroundThe 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
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
|
It's something I still feel comfortable shipping in WordPress 7.0 if we can agree that it would be a valuable addition. Nearly the same structure gets passed to the client as the filter |
Introduce a `wp_connectors_settings` filter in `_wp_connectors_get_connector_settings()` so plugins can add, modify, or remove connectors on the Connectors screen. The filter runs after built-in and AI Client registry providers are collected but before `setting_name` is auto-generated for API-key connectors. Also tighten the `setting_name` generation to only apply when `type === 'ai_provider'`, preventing non-AI connectors from receiving an incorrect `connectors_ai_*` setting name. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…leanup. Use structured PHPDoc for the `$connectors` param in `apply_filters()` to mirror the function's return documentation. In tests, use closure references for filter removal before assertions to prevent leaking to other tests. Follow-up to [555bb8a]. See #64791. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
c8f329a to
e0a086e
Compare
…ng keys. Skip `register_setting` in `_wp_register_default_connector_settings()` when the AI provider is not in the registry, preventing REST-exposed settings that silently reject values. Reorder the `hasProvider` check in `_wp_connectors_pass_default_keys_to_ai_client()` to run before reading the option. Update the REST settings test to reflect that connector settings are only registered when their provider is active. Follow-up to [555bb8a], [bd6c4c8]. See #64791. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
e0a086e to
bd4090d
Compare
There was a problem hiding this comment.
@gziolo As for the purpose of the PR so far, this looks solid.
However, I wonder, is this how we want to handle the external registration of connectors? This is a pretty big decision we shouldn't take lightly.
So far, this has all been locked down, and it's all private code. While I agree we need to allow registering connectors in 7.0 already, I'm not sure a filter is the way to go. Filters for registration can be a bad pattern for DX, especially with nested structured data like here. A declarative API, like wp_register_connector would be much more intuitive to use.
Alternatively, I think something that would work well with the current state of the code here would be a pattern where we have a registry class instance, and we expose it to an action.
In other words:
- Introduce
WP_Connector_Registryclass - Move the 3 hard-coded connectors in there
- Have a method on it
register_connectorthat receives the data for a single connector - Create an instance of the class in
_wp_connectors_get_connector_settings(), then run an action (NOT filter) where you pass that instance; action could be called e.g.wp_connectors_initor something like that - Have a method on the class
get_registered_connectors, to call at the end of_wp_connectors_get_connector_settings()
This would lead to better DX and encapsulate registration nicely, without creating any timing concerns because all registration would be forced to happen in the action hook. And it's a single class, nothing too complicated to add even with the little time we have.
WDYT?
… generation. Addresses review feedback to filter the final, fully populated value. The filter now receives connectors with `setting_name` already set for API-key connectors, and runs as the last step before returning. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@felixarntz Thanks for the thoughtful review! I agree that a registry class with an action hook would be the better pattern here — it's consistent with how Core handles registration elsewhere ( I also realized that we already have a way to filter what reaches the frontend via the I'm going to explore your proposed approach with |
…d registration. Introduces `WP_Connector_Registry` class with `register()`, `unregister()`, `is_registered()`, `get_registered()`, and `get_all_registered()` methods. Adds public API functions: `wp_register_connector()`, `wp_unregister_connector()`, `wp_has_connector()`, `wp_get_connector()`, `wp_get_connectors()`. Connectors must be registered on the `wp_connectors_init` action hook. Default connectors (Anthropic, Google, OpenAI) and AI Client registry providers are registered via `_wp_register_default_connectors()`. Removes the `wp_connectors_settings` filter in favor of the registry pattern, consistent with `WP_Abilities_Registry` and other Core registration APIs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…d defaults. Merges AI Client registry values on top of hardcoded defaults before registering, so provider plugin data takes precedence while hardcoded values serve as fallbacks. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@felixarntz, I quickly prototyped the idea you outlined. Does it align with what you had in mind? |
westonruter
left a comment
There was a problem hiding this comment.
Suggestions for attaining PHPStan rule level 8 in src/wp-includes/class-wp-connector-registry.php along with leveraging some other features in PHP 7.4.
| * @since 7.0.0 | ||
| * @var self|null | ||
| */ | ||
| private static $instance = null; |
There was a problem hiding this comment.
| private static $instance = null; | |
| private static ?WP_Connector_Registry $instance = null; |
| * @since 7.0.0 | ||
| * @var array[] | ||
| */ | ||
| private $registered_connectors = array(); |
There was a problem hiding this comment.
| private $registered_connectors = array(); | |
| private array $registered_connectors = array(); |
| * name, description, type, authentication, and optionally plugin. | ||
| * | ||
| * @since 7.0.0 | ||
| * @var array[] |
There was a problem hiding this comment.
| * @var array[] | |
| * @var array<string, array> | |
| * @phpstan-var array<string, Connector> |
| * The singleton instance of the registry. | ||
| * | ||
| * @since 7.0.0 | ||
| * @var self|null |
There was a problem hiding this comment.
| * @var self|null |
| ); | ||
|
|
||
| if ( 'api_key' === $args['authentication']['method'] ) { | ||
| $connector['authentication']['credentials_url'] = isset( $args['authentication']['credentials_url'] ) ? $args['authentication']['credentials_url'] : null; |
There was a problem hiding this comment.
| $connector['authentication']['credentials_url'] = isset( $args['authentication']['credentials_url'] ) ? $args['authentication']['credentials_url'] : null; | |
| $connector['authentication']['credentials_url'] = $args['authentication']['credentials_url'] ?? null; |
Trac ticket: https://core.trac.wordpress.org/ticket/64791
Summary
wp_connectors_settingsfilter in_wp_connectors_get_connector_settings()so plugins can add, modify, or remove connectors displayed on the Connectors screen.setting_nameis auto-generated — so connectors added via the filter withtype => 'ai_provider'andmethod => 'api_key'automatically get theirsetting_namepopulated.setting_namegeneration to requiretype === 'ai_provider', preventing non-AI connectors from receiving an incorrectconnectors_ai_*setting name.Use cases
api_keytonone)ksort)Follows from the discussion in WordPress/gutenberg#76014 (comment).
Test plan
npm run test:php -- --group connectorspasses (20 tests, 98 assertions)api_keyauth getssetting_nameauto-generatedapi_keyauth does not getsetting_name🤖 Generated with Claude Code