From c315fc43c6f7ce97b95b60750fb87de7220455ac Mon Sep 17 00:00:00 2001 From: darbs-claude Date: Mon, 23 Feb 2026 11:39:01 +0000 Subject: [PATCH] fix: validate API keys on AgenticManager init (#29) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Log a warning for each AI provider registered without an API key so that misconfiguration is surfaced at boot time (not silently on the first API call). Each message names the environment variable to set: ANTHROPIC_API_KEY – Claude GOOGLE_AI_API_KEY – Gemini OPENAI_API_KEY – OpenAI Providers without a key remain registered but are marked unavailable via isAvailable(), preserving backward compatibility. - Add Log::warning() calls in registerProviders() for empty keys - Extend AgenticManagerTest with a dedicated 'API key validation warnings' describe block (7 new test cases) - Update DX-002 in TODO.md as resolved Closes #29 Co-Authored-By: Claude Sonnet 4.6 --- Services/AgenticManager.php | 31 +++++++++- TODO.md | 4 +- tests/Unit/AgenticManagerTest.php | 98 +++++++++++++++++++++++++++++++ 3 files changed, 128 insertions(+), 5 deletions(-) diff --git a/Services/AgenticManager.php b/Services/AgenticManager.php index 7130fd9..2e21077 100644 --- a/Services/AgenticManager.php +++ b/Services/AgenticManager.php @@ -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', ); } diff --git a/TODO.md b/TODO.md index d6fed66..dce93ac 100644 --- a/TODO.md +++ b/TODO.md @@ -120,10 +120,10 @@ Production-quality task list for the AI agent orchestration package. - Issue: "workspace_id is required" doesn't explain how to fix - Fix: Include context about authentication/session setup -- [ ] **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()` diff --git a/tests/Unit/AgenticManagerTest.php b/tests/Unit/AgenticManagerTest.php index 1efcb5d..e2ab9e5 100644 --- a/tests/Unit/AgenticManagerTest.php +++ b/tests/Unit/AgenticManagerTest.php @@ -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); + }); +});