Change our AI connector check from looking at configuration to looking at if authentication is set#603
Conversation
…ce functions that are used to determine if a connector has authentication set, either via ENV var, constant or API key in the database. Use this instead of is_connector_configured as that makes actual API requests
|
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 If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #603 +/- ##
=============================================
+ Coverage 74.00% 74.01% +0.01%
Complexity 1738 1738
=============================================
Files 85 85
Lines 7490 7517 +27
=============================================
+ Hits 5543 5564 +21
- Misses 1947 1953 +6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@LukasFritzeDev wondering if you might have time to review this and ensure it still resolves your reported issue in #536? In testing some other things, realized the approach we took in #537 introduced some unintended side effects so I've switched things to the approach you mentioned here |
This approach resolves the described issue. I tested both, using env and constant. The warning in the AI settings page is not shown and the state in the status widget is shown correctly (if a valid token is set). I noticed a small regression, if an invalid token is set.
Just out of curiosity: What are the “unintended side effects” you noticed? |
Are these regressions based on how things currently work in the latest released version? Or are these regressions as compared to the changes made in #537? If it's the latter, that may be fine though I can look closer early next week when I'm back online.
So our Noticed this while testing some changes to the Connector Approval experiment as I was seeing multiple blocked requests on each page load whereas I wasn't seeing those previously. While these requests are meant to be lightweight and not use many tokens, still not ideal to fire off that many requests and potentially burn through a users token budget. |
the later one. As suggested, I guess we might simply use |
@LukasFritzeDev In giving it more thought, I think it's fine to make an API request on that dashboard page so I've reverted the dashboard widget to use the
@jeffpaul In theory, none of the code changes in this PR should be impacting that screen (in fact, the saving and validating of those credentials is entirely done by the Connectors API, not something we touch at all). That said, there was a bug with the Connectors Approval experiment that was fixed in #595 that is now merged into this PR that could have caught you. Basically if Connectors Approvals was turned on and the Connector wasn't approved for the AI plugin, validating credentials was being unintentionally blocked and would lead to the issue you were seeing. So assuming that is the issue you were having, that should be resolved in this PR now since |
jeffpaul
left a comment
There was a problem hiding this comment.
Retested via Playground and AI functionality continues to work as expected, thanks!



What?
Adds new helper functions that check if a connector has authentication in place, going from ENV var to constant to API key in the database. Use this in place of our new
is_connector_configuredcheck.Why?
In #537 we added a new
is_connector_configuredfunction and use that when determining if we have AI credentials. In testing something else, I was seeing lots of extra requests being made and realized that the$registry->isProviderConfigured( $connector_id )check that we run inis_connector_configuredmakes an actual API request to validate credentials.While this can be great in some circumstances, by attaching that to our
has_ai_credentialsfunction, this ends up running all the time (basically every admin page load).The original code here never verified credentials, it just checked if we had an API key set. As reported in #536, the issue with that check is it didn't consider ENV vars or constants and we fixed that with this new
is_connector_configuredfunction.But I don't think it's ideal to make all of these requests so this PR adds new functions that are now used instead of
is_connector_configuredthat bring us back to just checking for the existence of credentials, though now checking ENV var, constants and API keys in the database to resolve the original issue. This matches the original intent discussed here.How?
has_connector_authenticationandget_connector_api_key_source, the latter mirroring what WordPress Core does in_wp_connectors_get_api_key_sourceand the former being used as a replacement foris_connector_configuredis_connector_configuredfunction in case we want to use this at some point in the future. For example, likely could replace ourhas_valid_ai_credentialswith this, or use it within thatUse of AI Tools
AI assistance: Yes
Tool(s): Cursor
Model(s): GPT-5.5
Used for: Investigating the issue and researching a few approaches to resolving. Execution and testing done by me
Testing Instructions
See testing instructions on #537 to ensure that issue is still resolved
Changelog Entry