Fix: register providers and load .env in cli.php#224
Conversation
|
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✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## trunk #224 +/- ##
=========================================
Coverage 88.12% 88.12%
Complexity 1213 1213
=========================================
Files 60 60
Lines 3934 3934
=========================================
Hits 3467 3467
Misses 467 467
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:
|
felixarntz
left a comment
There was a problem hiding this comment.
@sarthak-19 Thank you for the PR!
The .env integration looks good, but the provider registration needs a cleaner approach.
| // Register provider packages so the registry can discover them. | ||
| $registry = AiClient::defaultRegistry(); | ||
| if (class_exists(AnthropicProvider::class)) { | ||
| $registry->registerProvider(AnthropicProvider::class); | ||
| } | ||
| if (class_exists(GoogleProvider::class)) { | ||
| $registry->registerProvider(GoogleProvider::class); | ||
| } | ||
| if (class_exists(OpenAiProvider::class)) { | ||
| $registry->registerProvider(OpenAiProvider::class); | ||
| } |
There was a problem hiding this comment.
It's not ideal to hard-code specific providers here. What if you want to test the CLI with other ones?
I think this should be removed. It would be better to instead support a simple system where another user-controlled file handles the registration of providers. Could look like this:
- check if the file (e.g.
__DIR__ . '/cli-providers.php) exists - if so, implement a closure here in which that file is loaded, with only access to the
$registryinstance so that it cannot do anything else
The cli-providers.php file would be git-ignored. It would need to look something like this (with $registry being in the local scope of where it is required):
use WordPress\AnthropicAiProvider\Provider\AnthropicProvider;
use WordPress\GoogleAiProvider\Provider\GoogleProvider;
use WordPress\OpenAiAiProvider\Provider\OpenAiProvider;
$registry->registerProvider(AnthropicProvider::class);
$registry->registerProvider(GoogleProvider::class);
$registry->registerProvider(OpenAiProvider::class);
What
cli.phpwas missing two bootstrapping steps, causing every run to fail with:Why
The
ProviderRegistryhas no built-in knowledge of provider packages they must be explicitly registered. Additionally, API keys stored in.envwere never loaded into the environment, so providers could not authenticate even if registered.Changes
Added to
cli.php(mirroringtests/integration/bootstrap.php):.envloading viasymfony/dotenv(already a dev dependency).AnthropicProvider,GoogleProvider, andOpenAiProvider, guarded withclass_exists()so the script works even if only a subset of provider packages are installed.Testing
References
Fixes : #223