core/php-agentic
8
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 1
php-agentic/changelog/2026/jan/code-review.md
Snider ad83825f93 refactor: rename namespace Core\Agentic to Core\Mod\Agentic 
Updates all classes to use the new modular namespace convention.
Adds Service/ layer with Core\Service\Agentic for service definition.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
2026-01-27 16:12:58 +00:00

7.2 KiB
Raw Blame History

Agentic Module Review

Updated: 2026-01-21 - OpenAI pricing, MCP token lookup, and Boot.php cleanup verified complete

Overview

The Agentic module provides AI agent infrastructure for Host Hub, including:

  1. Multi-provider AI abstraction - Unified interface for Claude, Gemini, and OpenAI APIs
  2. Agent work plans - Structured task tracking with phases, dependencies, and progress
  3. Session management - Track agent work sessions for handoff and context recovery
  4. API key management - Workspace-scoped keys with permissions and rate limiting
  5. Plan templates - YAML-based reusable plan structures
  6. Content generation pipeline - Batch content generation with quality validation
  7. Agent detection - Identify AI agents from HTTP requests (Trees for Agents)
  8. Admin dashboard - Hades-only admin panel for monitoring agent operations

Production Readiness Score: 88/100 (was 85/100 - Additional improvements 2026-01-21)

The module has solid foundational architecture. Critical issues fixed in Wave 2. All recommended improvements now implemented. OpenAI pricing, MCP token database lookup, and Boot.php cleanup completed.

Critical Issues (Must Fix)

  • Missing agent_api_keys table migration - FIXED: Migration exists at Migrations/2026_01_15_090000_create_agent_api_keys_table.php.
  • Missing tasks table migration - FIXED: Migration exists at Migrations/2026_01_15_090001_create_tasks_table.php. Task model updated with BelongsToWorkspace trait and workspace_id in fillable.
  • AgenticResponse::estimateCost() missing OpenAI pricing - FIXED: Pricing array now includes GPT-4o, GPT-4o-mini, GPT-4-turbo, GPT-4, GPT-3.5-turbo, o1, o1-mini, and o1-preview models.
  • AgentApiKey::getRecentCallCount() is a stub - FIXED: Now reads from cache key agent_api_key_rate:{id} which is incremented by AgentApiKeyService::recordUsage().
  • PlanCommand/TaskCommand missing workspace scope - FIXED: Both commands now require --workspace=ID option or authenticated user context. All queries scoped via forWorkspace() to prevent data leakage.

Recommended Improvements

  • Add retry logic to AI provider services - FIXED: HasRetry trait added with exponential backoff for transient failures.
  • Stream parsing is fragile - FIXED: HasStreamParsing trait added for robust chunked response handling.
  • ContentService uses hardcoded paths - FIXED: $batchPath, $promptPath, $draftsPath now use config values.
  • AgentApiKeyService rate limit TTL is incorrect - FIXED: Now uses Cache::add() with TTL to prevent race condition.
  • Missing validation on PlanTemplateService variable substitution - FIXED: JSON escaping added to prevent data corruption from special characters.
  • Dashboard queries not optimised - FIXED: cacheWithLock added to prevent cache stampede.
  • Add middleware for admin routes - FIXED: RequireHades middleware now applied to /hub/agents/* routes.
  • Add request validation to ForAgentsController - FIXED: Rate limiting and caching added.

Missing Features (Future)

  • MCP tool handlers commented out - FIXED: Boot::onMcpTools() now has clean documentation explaining that agent tools are registered in Mcp module via AgentToolRegistry.
  • MCP token lookup not implemented - FIXED: AgentDetection::identifyFromMcpToken() now supports opaque ak_ prefix tokens with database lookup via AgentApiKey::findByKey(), plus structured provider:model:secret format.
  • No tests for AgentApiKey model - Unlike other models (AgentPlan, AgentPhase, AgentSession), AgentApiKey has no dedicated test file.
  • No tests for AI provider services - ClaudeService, GeminiService, OpenAIService have no unit tests.
  • No tests for AgentApiKeyService - Service has complex authentication logic but no tests.
  • No tests for AgentDetection - User-Agent pattern matching logic is untested.
  • No tests for PlanTemplateService - Variable substitution and plan creation untested.
  • Missing admin panel tests for all Livewire components - Only UseCase/AdminPanelBasic.php exists but may not execute (uses non-standard test syntax).
  • GenerateCommand depends on Content module - Uses Mod\Content\Models\ContentBrief, Mod\Content\Jobs\GenerateContentJob, etc. Should verify Content module is always loaded.

Test Coverage Assessment

Current test files:

  • Tests/Feature/AgentPlanTest.php - 17 tests, good coverage of plan lifecycle
  • Tests/Feature/AgentPhaseTest.php - 17 tests, good coverage of phase operations
  • Tests/Feature/AgentSessionTest.php - 22 tests, thorough session tracking tests
  • Tests/Feature/ContentServiceTest.php - 4 tests, basic coverage using mocks
  • Tests/UseCase/AdminPanelBasic.php - UI acceptance tests (may not be functional)

Coverage gaps:

  • No unit tests for any of the 11 services
  • No tests for 3 console commands
  • No tests for AgentApiKey model
  • No tests for Task model
  • No tests for admin Livewire components
  • No integration tests for AI provider APIs

Estimated coverage: ~35% (models well-tested, services untested)

Security Concerns

  1. API key stored as SHA-256 hash only - No salt used in AgentApiKey::generate(). While SHA-256 is acceptable, using password_hash() with Argon2 would be more secure for credential storage.

  2. Rate limiting bypass - FIXED: getRecentCallCount() now reads from cache, making rate limiting functional when AgentApiKeyService::recordUsage() is called.

  3. No input validation on plan/template variables - PlanTemplateService::substituteVariables() uses regex replacement without sanitising input, potentially allowing injection.

  4. Admin routes lack middleware - FIXED: RequireHades middleware now applied to admin routes.

  5. ForAgentsController exposes internal URLs - Returns hardcoded URLs to MCP registry and documentation that could be enumerated by attackers.

  6. TaskCommand uses FIELD() with unescaped values - orderByRaw("FIELD(priority, 'urgent', 'high', 'normal', 'low')") - while current usage is safe (status/priority are validated), this pattern is SQL injection prone if extended.

Notes

Architecture: Well-structured following the modular monolith pattern. Boot.php properly uses event listeners for lazy loading. Services are appropriately separated.

Code quality: Consistent use of strict types, proper PHPDoc annotations, and Eloquent conventions. Factory pattern for test data is well implemented.

Dependencies on other modules:

  • Mod\Tenant - Workspace model and BelongsToWorkspace trait
  • Mod\Mcp - McpToolCallStat for dashboard analytics
  • Mod\Content - ContentBrief, AIUsage, GenerateContentJob (in GenerateCommand)

Files reviewed:

  • /app/Mod/Agentic/Boot.php
  • /app/Mod/Agentic/config.php
  • /app/Mod/Agentic/Migrations/2026_01_15_100000_create_agentic_tables.php
  • All 11 models in /app/Mod/Agentic/Models/
  • All 9 services in /app/Mod/Agentic/Services/
  • All 3 console commands in /app/Mod/Agentic/Console/Commands/
  • /app/Mod/Agentic/Controllers/ForAgentsController.php
  • /app/Mod/Agentic/Routes/admin.php
  • All 4 test files in /app/Mod/Agentic/Tests/
  • /app/Mod/Agentic/View/Modal/Admin/Dashboard.php