Merge pull request 'fix: validate API keys on AgenticManager init' (#57) from fix/validate-api-keys-on-init into main
Reviewed-on: #57
This commit is contained in:
commit
5fa46104f4
3 changed files with 128 additions and 5 deletions
|
|
@ -4,6 +4,7 @@ declare(strict_types=1);
|
|||
|
||||
namespace Core\Mod\Agentic\Services;
|
||||
|
||||
use Illuminate\Support\Facades\Log;
|
||||
use InvalidArgumentException;
|
||||
|
||||
class AgenticManager
|
||||
|
|
@ -91,22 +92,46 @@ class AgenticManager
|
|||
|
||||
/**
|
||||
* Register all AI providers.
|
||||
*
|
||||
* Logs a warning for each provider whose API key is absent so that
|
||||
* misconfiguration is surfaced at boot time rather than on the first
|
||||
* API call. Set the corresponding environment variable to silence it:
|
||||
*
|
||||
* ANTHROPIC_API_KEY – Claude
|
||||
* GOOGLE_AI_API_KEY – Gemini
|
||||
* OPENAI_API_KEY – OpenAI
|
||||
*/
|
||||
private function registerProviders(): void
|
||||
{
|
||||
// Use null coalescing since config() returns null for missing env vars
|
||||
$claudeKey = config('services.anthropic.api_key') ?? '';
|
||||
$geminiKey = config('services.google.ai_api_key') ?? '';
|
||||
$openaiKey = config('services.openai.api_key') ?? '';
|
||||
|
||||
if (empty($claudeKey)) {
|
||||
Log::warning("Agentic: 'claude' provider has no API key configured. Set ANTHROPIC_API_KEY to enable it.");
|
||||
}
|
||||
|
||||
if (empty($geminiKey)) {
|
||||
Log::warning("Agentic: 'gemini' provider has no API key configured. Set GOOGLE_AI_API_KEY to enable it.");
|
||||
}
|
||||
|
||||
if (empty($openaiKey)) {
|
||||
Log::warning("Agentic: 'openai' provider has no API key configured. Set OPENAI_API_KEY to enable it.");
|
||||
}
|
||||
|
||||
$this->providers['claude'] = new ClaudeService(
|
||||
apiKey: config('services.anthropic.api_key') ?? '',
|
||||
apiKey: $claudeKey,
|
||||
model: config('services.anthropic.model') ?? 'claude-sonnet-4-20250514',
|
||||
);
|
||||
|
||||
$this->providers['gemini'] = new GeminiService(
|
||||
apiKey: config('services.google.ai_api_key') ?? '',
|
||||
apiKey: $geminiKey,
|
||||
model: config('services.google.ai_model') ?? 'gemini-2.0-flash',
|
||||
);
|
||||
|
||||
$this->providers['openai'] = new OpenAIService(
|
||||
apiKey: config('services.openai.api_key') ?? '',
|
||||
apiKey: $openaiKey,
|
||||
model: config('services.openai.model') ?? 'gpt-4o-mini',
|
||||
);
|
||||
}
|
||||
|
|
|
|||
4
TODO.md
4
TODO.md
|
|
@ -121,10 +121,10 @@ Production-quality task list for the AI agent orchestration package.
|
|||
- Issue: "workspace_id is required" didn't explain how to fix
|
||||
- Fix: Updated error messages in PlanCreate, PlanGet, PlanList, StateSet, StateGet, StateList, SessionStart to include actionable guidance and link to documentation
|
||||
|
||||
- [ ] **DX-002: AgenticManager doesn't validate API keys on init**
|
||||
- [x] **DX-002: AgenticManager doesn't validate API keys on init** (FIXED 2026-02-23)
|
||||
- Location: `Services/AgenticManager.php::registerProviders()`
|
||||
- Issue: Empty API key creates provider that fails on first use
|
||||
- Fix: Log warning or throw if provider configured without key
|
||||
- Fix: `Log::warning()` emitted for each provider registered without an API key; message names the env var to set
|
||||
|
||||
- [ ] **DX-003: Plan template variable errors not actionable**
|
||||
- Location: `Services/PlanTemplateService.php::validateVariables()`
|
||||
|
|
|
|||
|
|
@ -15,6 +15,7 @@ use Core\Mod\Agentic\Services\ClaudeService;
|
|||
use Core\Mod\Agentic\Services\GeminiService;
|
||||
use Core\Mod\Agentic\Services\OpenAIService;
|
||||
use Illuminate\Support\Facades\Config;
|
||||
use Illuminate\Support\Facades\Log;
|
||||
use InvalidArgumentException;
|
||||
|
||||
// =========================================================================
|
||||
|
|
@ -343,6 +344,8 @@ describe('direct provider access methods', function () {
|
|||
|
||||
describe('edge cases', function () {
|
||||
it('handles missing configuration gracefully', function () {
|
||||
Log::spy();
|
||||
|
||||
Config::set('services.anthropic.api_key', null);
|
||||
Config::set('services.anthropic.model', null);
|
||||
Config::set('services.google.ai_api_key', null);
|
||||
|
|
@ -359,6 +362,9 @@ describe('edge cases', function () {
|
|||
|
||||
// But all should be unavailable
|
||||
expect($manager->availableProviders())->toBeEmpty();
|
||||
|
||||
// Warnings logged for all three unconfigured providers
|
||||
Log::shouldHaveReceived('warning')->times(3);
|
||||
});
|
||||
|
||||
it('provider retrieval is case-sensitive', function () {
|
||||
|
|
@ -387,3 +393,95 @@ describe('edge cases', function () {
|
|||
->toThrow(InvalidArgumentException::class);
|
||||
});
|
||||
});
|
||||
|
||||
// =========================================================================
|
||||
// API Key Validation Warning Tests
|
||||
// =========================================================================
|
||||
|
||||
describe('API key validation warnings', function () {
|
||||
it('logs a warning when Claude API key is not configured', function () {
|
||||
Log::spy();
|
||||
Config::set('services.anthropic.api_key', '');
|
||||
Config::set('services.google.ai_api_key', 'test-gemini-key');
|
||||
Config::set('services.openai.api_key', 'test-openai-key');
|
||||
|
||||
new AgenticManager;
|
||||
|
||||
Log::shouldHaveReceived('warning')
|
||||
->once()
|
||||
->withArgs(fn (string $message) => str_contains($message, 'claude') && str_contains($message, 'ANTHROPIC_API_KEY'));
|
||||
});
|
||||
|
||||
it('logs a warning when Gemini API key is not configured', function () {
|
||||
Log::spy();
|
||||
Config::set('services.anthropic.api_key', 'test-claude-key');
|
||||
Config::set('services.google.ai_api_key', '');
|
||||
Config::set('services.openai.api_key', 'test-openai-key');
|
||||
|
||||
new AgenticManager;
|
||||
|
||||
Log::shouldHaveReceived('warning')
|
||||
->once()
|
||||
->withArgs(fn (string $message) => str_contains($message, 'gemini') && str_contains($message, 'GOOGLE_AI_API_KEY'));
|
||||
});
|
||||
|
||||
it('logs a warning when OpenAI API key is not configured', function () {
|
||||
Log::spy();
|
||||
Config::set('services.anthropic.api_key', 'test-claude-key');
|
||||
Config::set('services.google.ai_api_key', 'test-gemini-key');
|
||||
Config::set('services.openai.api_key', '');
|
||||
|
||||
new AgenticManager;
|
||||
|
||||
Log::shouldHaveReceived('warning')
|
||||
->once()
|
||||
->withArgs(fn (string $message) => str_contains($message, 'openai') && str_contains($message, 'OPENAI_API_KEY'));
|
||||
});
|
||||
|
||||
it('logs a warning when API key is null', function () {
|
||||
Log::spy();
|
||||
Config::set('services.anthropic.api_key', null);
|
||||
Config::set('services.google.ai_api_key', 'test-gemini-key');
|
||||
Config::set('services.openai.api_key', 'test-openai-key');
|
||||
|
||||
new AgenticManager;
|
||||
|
||||
Log::shouldHaveReceived('warning')
|
||||
->once()
|
||||
->withArgs(fn (string $message) => str_contains($message, 'claude'));
|
||||
});
|
||||
|
||||
it('logs warnings for all three providers when no keys are configured', function () {
|
||||
Log::spy();
|
||||
Config::set('services.anthropic.api_key', '');
|
||||
Config::set('services.google.ai_api_key', '');
|
||||
Config::set('services.openai.api_key', '');
|
||||
|
||||
new AgenticManager;
|
||||
|
||||
Log::shouldHaveReceived('warning')->times(3);
|
||||
});
|
||||
|
||||
it('does not log warnings when all API keys are configured', function () {
|
||||
Log::spy();
|
||||
Config::set('services.anthropic.api_key', 'test-claude-key');
|
||||
Config::set('services.google.ai_api_key', 'test-gemini-key');
|
||||
Config::set('services.openai.api_key', 'test-openai-key');
|
||||
|
||||
new AgenticManager;
|
||||
|
||||
Log::shouldNotHaveReceived('warning');
|
||||
});
|
||||
|
||||
it('only warns for providers that have missing keys, not all providers', function () {
|
||||
Log::spy();
|
||||
Config::set('services.anthropic.api_key', 'test-key');
|
||||
Config::set('services.google.ai_api_key', '');
|
||||
Config::set('services.openai.api_key', '');
|
||||
|
||||
new AgenticManager;
|
||||
|
||||
// Only gemini and openai should warn – not claude
|
||||
Log::shouldHaveReceived('warning')->times(2);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue