Fix: make connector validity capability-aware instead of text-only - #935
Open
whyisjake wants to merge 8 commits into
Open
Fix: make connector validity capability-aware instead of text-only#935whyisjake wants to merge 8 commits into
whyisjake wants to merge 8 commits into
Conversation
has_valid_ai_credentials() decided whether any configured connector was valid by asking a single question -- can it generate text. A site whose only connector provides speech or image generation was therefore told its connectors may be invalid, when they were correctly configured and working. Generalize the request-free scan already used by has_image_generation_support() into has_capability_support( $capability ), memoized per capability, and back has_valid_ai_credentials() with it for TEXT_GENERATION. The verdict for text-based features is unchanged; what changes is that the check no longer routes through a live prompt builder and the reason for a false result is now expressible per capability. has_image_generation_support() becomes a delegate. Its wpai_has_image_generation_support filter is applied after the new wpai_has_capability_support filter, so existing third-party integrations remain authoritative for image generation. The docblock on has_valid_ai_credentials() now states that it is a capability-specific gate rather than a judgement about connector validity.
Add get_supported_capabilities(), which reports the capabilities the configured connectors can actually provide, and pass it to the settings page script module as `capabilities` alongside the existing `hasCredentials` and `hasValidCredentials`. Together the three signals let the settings screen distinguish "no connectors configured" from "connectors configured, but none of them generate text" -- and name what the connectors do provide instead. Extract the script module data assembly out of the filter closure into a private method so the payload contract is testable. The route render function that guards filter registration is never defined under WPAI_IS_TEST, so testing through the filter itself would only ever skip.
…connector A site whose only connector provides speech or image generation was shown a red error notice reading "Please review the AI Connectors you have configured to ensure they are valid." The connector was valid; it simply did not do text. Replace that branch with copy that names the requirement and what the configured connectors actually provide -- "These AI features need an AI Connector that can generate text. The AI Connectors you have configured provide speech generation." -- and lower its intent from error to warning, because nothing is broken. The "no connectors configured" branch is unchanged and stays an error. Capability values are mapped to human-readable labels in the UI layer, and unrecognized values are dropped so a raw identifier can never reach user-visible copy. When the capability list is empty or absent -- including stale cached script data after an upgrade -- the notice falls back to a capability-free sentence. Adds e2e coverage for all three notice states, with a capability override endpoint in the e2e-testing plugin so specs can simulate a non-text connector without shipping a real speech provider.
`wp ai alt-text generate` reported "No valid AI credentials found" whenever has_valid_ai_credentials() was false, which reads as broken configuration even when the connector is authenticated and working but only does speech or images. Split the message: when no credentials exist at all, point at Settings > Connectors as before; when credentials exist but no connector generates text, say that instead. test_generate_errors_without_credentials() was already filtering has_ai_credentials to true, so despite its name it exercised the capability-mismatch path rather than the missing-credential path. Point it at the state it names and add separate coverage for the capability-mismatch message.
✅ WordPress Plugin Check Report
📊 ReportAll checks passed! No errors or warnings found. 🤖 Generated by WordPress Plugin Check Action • Learn more about Plugin Check |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #935 +/- ##
=============================================
+ Coverage 74.57% 74.76% +0.19%
- Complexity 3132 3134 +2
=============================================
Files 132 132
Lines 12213 12231 +18
=============================================
+ Hits 9108 9145 +37
+ Misses 3105 3086 -19
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
whyisjake
marked this pull request as ready for review
August 13, 2026 20:38
|
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. |
The settings page data filter is only registered when ai_ai_wp_admin_render_page() exists, and Requirements::check() short-circuits its asset check under WPAI_IS_TEST, so the generated route file that defines it is never loaded in PHPUnit. Nothing therefore asserted that the filter is attached under the hook the route script module actually reads -- the existing coverage invokes the payload builder directly. Add a global-namespace stand-in for that function, following the wp-cli-stubs.php pattern, and assert the wiring: the filter is registered under the page slug, existing script module data is preserved, and the payload carries the credential and capability keys the settings notice depends on.
Contributor
|
We were already considering just removing the So in looking at this PR, I guess where I'm struggling is if it's worth 1000+ lines of code to make that error message more useful or if the right answer is to just get rid of that entirely. If we were actually gating functionality based on that check we'd obviously want that to be more robust but when it's just used for an admin notice, trying to decide if this is worth keeping or not. Any thoughts? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What?
Closes #933
Makes the connector validity check capability-aware, so a site whose only connector provides speech or image generation is no longer told its connectors may be invalid.
Why?
has_valid_ai_credentials()decided whether any configured connector was valid by asking a single question — can it generate text:That verdict reached the settings screen as
PAGE_DATA.hasValidCredentialsand rendered a red error: "Please review the AI Connectors you have configured to ensure they are valid."A connector registering only
TEXT_TO_SPEECH_CONVERSION/SPEECH_GENERATIONis authenticated, registered, and working. It simply does not do text. The connector was detected correctly — the verdict and the wording were what mismatched.This repository had already solved the narrower version of the problem: #679 gated the image generation UI on real provider support, and #748 refined that into
has_image_generation_support(), a request-free scan over connector model metadata. The global credentials check never picked up the same capability awareness.How?
Direction 1 of the three the issue proposed — generalize the check and reword the notice — chosen because it is the smallest correct fix and mirrors the existing #679/#748 precedent.
has_valid_ai_credentials()keeps its name, signature, return type, and semantics as the "can this plugin do its text-based work" gate.has_capability_support( string $capability, bool $reset_cache = false )generalizes the scan out ofhas_image_generation_support(), memoized per capability, with a newwpai_has_capability_supportfilter.has_image_generation_support()becomes a delegate and applieswpai_has_image_generation_supportafter the general filter, so existing third-party integrations stay authoritative for image generation.has_valid_ai_credentials()is re-backed with that scan forTEXT_GENERATION. Thehas_ai_credentials()early return and thewpai_pre_has_valid_credentials_checkshort-circuit keep their exact positions. Its docblock now states that it is a capability-specific gate, not a judgement about connector validity.get_supported_capabilities()reports what the connectors can do, and is exposed to the settings page ascapabilitiesalongsidehasCredentialsandhasValidCredentials. Together the three signals distinguish "no connectors" from "connectors that don't generate text".warningrather thanerror— nothing is broken. The "no connectors configured" branch is unchanged and stays anerror. Capability values map to human-readable labels in the UI; unrecognized values are dropped so a raw identifier can never reach user-visible copy, and an empty or absent list falls back to a capability-free sentence.wp ai alt-text generatesplits its message the same way.CapabilityEnumis a constants class extendingAbstractEnum, not a native PHP enum, so the helper takes the constant'sstringvalue rather than an enum instance.Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5
Used for: tracing the behaviour to
has_valid_ai_credentials(), drafting the implementation plan, implementing the change, and writing the tests.Testing Instructions
Reproducing the original report needs the AI plugin plus exactly one non-text connector. The e2e suite simulates that without shipping a speech provider, via a capability override endpoint added to the
e2e-testingsupport plugin.Automated:
Manual:
add_filter( 'wpai_has_capability_support', fn( $s, $c ) => 'speech_generation' === $c, 10, 2 );Tests executed for this PR:
HelpersTest— newhas_capability_support,get_supported_capabilities, andhas_valid_ai_credentialscoverage; the 7 pre-existinghas_image_generation_supporttests pass with unmodified bodiesSettings_PageTest::test_script_module_data_includes_capability_statusAlt_Text_CommandTest— 28 testsconnector-capabilities.spec.js(3) andsettings.spec.js(17), 20/20 on a clean environmentphpcs,phpstan,eslint,tsc— all cleanScreenshots or screencast
Both captured on a speech-only connector.
Changelog Entry