Disable Experiments when no valid AI credentials are found

[dkotter]

What?

Closes #30

Ensure we have valid AI credentials before we allow any Experiment to be used

Why?

Right now you can enable any Experiment even if you haven't added any AI credentials. This can lead to confusion for users that then try and use that Experiment and it doesn't work properly. This is also the case if you add invalid credentials.

The AI Credentials setting screen is coming from the WP AI Client package and doesn't do any validation when credentials are saved. This PR adds that validation, when someone lands on our settings screen, and an error message will be shown if they either have no credentials set up or their credentials aren't valid. They are then prompted to go add valid credentials before proceeding.

How?

• Adds two new helper functions: has_ai_credentials, has_valid_ai_credentials
• Run those functions when we render our setting screen, showing a proper error message and hiding all other settings if either of those return false
• When we check to see if an Experiment is enabled, run has_valid_ai_credentials and return false appropriately, so an Experiment won't work if we don't have valid credentials

Note

If we ever introduce an Experiment that doesn't need to use AI, we'll want to update this logic.

Testing Instructions

1. In a clean environment, go to our settings page. Ensure an error message is shown that prompts you to go add credentials and that no other settings are shown
2. Add invalid credentials and come back. Ensure a different error message is shown that lets you know your credentials aren't valid. Ensure settings still aren't shown
3. Add at least one valid credential. Ensure our settings page now works
4. After enabling an Experiment, change the valid credentials to invalid and ensure the Experiment no longer works
5. Remove credentials all together and ensure the Experiment no longer works

Screenshots or screencast

[github-actions]

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.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: dkotter <dkotter@git.wordpress.org>
Co-authored-by: felixarntz <flixos90@git.wordpress.org>
Co-authored-by: jeffpaul <jeffpaul@git.wordpress.org>

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

[felixarntz]

@dkotter Looks solid, only two things I think need to be fixed.

[felixarntz]

This technically doesn't check whether AI credentials are valid. It only checks whether at least one model is available to satisfy the prompt requirements.

Now, in practice, all 3 default provider implementations require API keys even to call the endpoint to list the available models, which happens as part of that. So I suppose you can still use this here as a way to check. However, that'll only mean the API request that happens at some point will fail and throw an exception, so the code here doesn't cater for that.

I'd suggest the following:

• Immediate action: Wrap this in a try catch and return false if it throws.
• Separately: Consider whether/how we want to more formally enable checking for credential validity in the PHP/WP AI Client packages or whether this technique here is how we think it should be done.

[dkotter]

Added that try/catch here: 12b716a, let me know if that looks better

[felixarntz]

Shouldn't this be called before the filter? If you don't have any AI credentials, you certainly don't have valid AI credentials.

It makes sense to me to allow filtering/overriding the validity check logic, but the prerequisite should always be that there are AI credentials at all.

Otherwise, this also throws a wrench in things when e.g. for the admin notice you're also checking has_ai_credentials(), which would lead to a misleading notice if that filter here was overriding the result to be true.

[dkotter]

Yep, makes sense. Moved this filter down: 12b716a

[codecov]

Codecov Report

❌ Patch coverage is 43.75000% with 18 lines in your changes missing coverage. Please review.
✅ Project coverage is 29.61%. Comparing base (77ac717) to head (a6e50c2).
⚠️ Report is 25 commits behind head on trunk.

Files with missing lines	Patch %	Lines
includes/Settings/Settings_Page.php	0.00%	11 Missing ⚠️
includes/helpers.php	70.58%	5 Missing ⚠️
includes/Abstracts/Abstract_Experiment.php	66.66%	1 Missing ⚠️
includes/bootstrap.php	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##              trunk     #100      +/-   ##
============================================
+ Coverage     29.19%   29.61%   +0.41%     
- Complexity      142      145       +3     
============================================
  Files            14       14              
  Lines           846      878      +32     
============================================
+ Hits            247      260      +13     
- Misses          599      618      +19     

Flag	Coverage Δ
unit	29.61% <43.75%> (+0.41%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[felixarntz]

@dkotter LGTM!

One minor point, but not a blocker.