diff --git a/php/Mcp/Tools/Agent/Brain/BrainList.php b/php/Mcp/Tools/Agent/Brain/BrainList.php index a676cff..fca47a8 100644 --- a/php/Mcp/Tools/Agent/Brain/BrainList.php +++ b/php/Mcp/Tools/Agent/Brain/BrainList.php @@ -45,6 +45,7 @@ class BrainList extends AgentTool 'org' => [ 'type' => 'string', 'description' => 'Filter by organisation scope', + 'maxLength' => 128, ], 'project' => [ 'type' => 'string', @@ -77,32 +78,30 @@ class BrainList extends AgentTool return $this->error('workspace_id is required. Ensure you have authenticated with a valid API key. See: https://host.uk.com/ai'); } - $org = $this->optionalString($args, 'org', null); + $org = $this->optionalString($args, 'org', null, 128); $project = $this->optionalString($args, 'project', null); $agentId = $this->optionalString($args, 'agent_id', null); $type = $this->optionalEnum($args, 'type', BrainMemory::VALID_TYPES); $limit = $this->optionalInt($args, 'limit', 20, 1, 100); - return $this->withCircuitBreaker('brain', function () use ($workspaceId, $org, $project, $agentId, $type, $limit) { - $query = BrainMemory::forWorkspace((int) $workspaceId) - ->active() - ->latestVersions() - ->forOrg($org) - ->forProject($project) - ->byAgent($agentId); + $query = BrainMemory::forWorkspace((int) $workspaceId) + ->active() + ->latestVersions() + ->forOrg($org) + ->forProject($project) + ->byAgent($agentId); - if ($type !== null) { - $query->ofType($type); - } + if ($type !== null) { + $query->ofType($type); + } - $memories = $query->orderByDesc('created_at') - ->limit($limit) - ->get(); + $memories = $query->orderByDesc('created_at') + ->limit($limit) + ->get(); - return $this->success([ - 'memories' => $memories->map(fn (BrainMemory $memory): array => $memory->toMcpContext())->all(), - 'count' => $memories->count(), - ]); - }, fn () => $this->error('Brain service temporarily unavailable. Memory list unavailable.', 'service_unavailable')); + return $this->success([ + 'memories' => $memories->map(fn (BrainMemory $memory): array => $memory->toMcpContext())->all(), + 'count' => $memories->count(), + ]); } } diff --git a/php/Mcp/Tools/Agent/Brain/BrainRecall.php b/php/Mcp/Tools/Agent/Brain/BrainRecall.php index 54af3de..837406c 100644 --- a/php/Mcp/Tools/Agent/Brain/BrainRecall.php +++ b/php/Mcp/Tools/Agent/Brain/BrainRecall.php @@ -63,6 +63,7 @@ class BrainRecall extends AgentTool 'org' => [ 'type' => 'string', 'description' => 'Filter by organisation scope', + 'maxLength' => 128, ], 'project' => [ 'type' => 'string', @@ -110,8 +111,13 @@ class BrainRecall extends AgentTool return $this->error('filter must be an object'); } - return $this->withCircuitBreaker('brain', function () use ($query, $workspaceId, $filter, $topK) { - $result = RecallKnowledge::run($query, (int) $workspaceId, $filter, $topK); + $validatedFilter = $filter; + if (array_key_exists('org', $validatedFilter)) { + $validatedFilter['org'] = $this->optionalString($validatedFilter, 'org', null, 128); + } + + return $this->withCircuitBreaker('brain', function () use ($query, $workspaceId, $validatedFilter, $topK) { + $result = RecallKnowledge::run($query, (int) $workspaceId, $validatedFilter, $topK); return $this->success([ 'count' => $result['count'], diff --git a/php/Mcp/Tools/Agent/Brain/BrainRemember.php b/php/Mcp/Tools/Agent/Brain/BrainRemember.php index 219f773..d474045 100644 --- a/php/Mcp/Tools/Agent/Brain/BrainRemember.php +++ b/php/Mcp/Tools/Agent/Brain/BrainRemember.php @@ -61,6 +61,7 @@ class BrainRemember extends AgentTool 'org' => [ 'type' => 'string', 'description' => 'Optional organisation scope', + 'maxLength' => 128, ], 'project' => [ 'type' => 'string', @@ -95,9 +96,11 @@ class BrainRemember extends AgentTool } $agentId = $context['agent_id'] ?? $context['session_id'] ?? 'anonymous'; + $payload = $args; + $payload['org'] = $this->optionalString($args, 'org', null, 128); - return $this->withCircuitBreaker('brain', function () use ($args, $workspaceId, $agentId) { - $memory = RememberKnowledge::run($args, (int) $workspaceId, $agentId); + return $this->withCircuitBreaker('brain', function () use ($payload, $workspaceId, $agentId) { + $memory = RememberKnowledge::run($payload, (int) $workspaceId, $agentId); return $this->success([ 'memory' => $memory->toMcpContext(), diff --git a/php/Services/BrainService.php b/php/Services/BrainService.php index d5e37ae..5bcee7b 100644 --- a/php/Services/BrainService.php +++ b/php/Services/BrainService.php @@ -24,6 +24,8 @@ class BrainService private const ELASTIC_INDEX = 'brain_memories'; + private const MAX_RETRY_DELAY_MS = 30000; + private string $qdrantApiKey; public function __construct( @@ -237,11 +239,22 @@ class BrainService */ public function forget(string $id): void { - $deleted = DB::connection('brain')->transaction(function () use ($id): int { - return BrainMemory::where('id', $id)->delete(); + $memory = null; + $deleted = DB::connection('brain')->transaction(function () use ($id, &$memory): int { + $memory = BrainMemory::query() + ->select(['id', 'indexed_at']) + ->find($id); + + if (! $memory instanceof BrainMemory) { + return 0; + } + + $memory->delete(); + + return 1; }); - if ($deleted > 0) { + if ($deleted > 0 && $memory instanceof BrainMemory && $memory->indexed_at !== null) { DeleteFromIndex::dispatch($id); } } @@ -651,14 +664,13 @@ class BrainService /** * Retry transient Qdrant HTTP failures with a small exponential backoff. * - * Retries 5xx responses and connection failures. 4xx responses are - * returned immediately so callers can fail fast without extra churn. + * Retries 408, 429, 5xx responses, and connection failures. Other 4xx + * responses are returned immediately so callers can fail fast. * * @param callable(PendingRequest): Response $buildRequest */ private function retryableHttp(int $timeout, callable $buildRequest, int $maxAttempts = 3): Response { - $delays = [100, 300, 900]; $lastConnectionException = null; for ($attempt = 1; $attempt <= $maxAttempts; $attempt++) { @@ -671,16 +683,16 @@ class BrainService break; } - $this->sleepMilliseconds($delays[$attempt - 1] ?? 900); + $this->sleepMilliseconds($this->retryDelayMilliseconds(null, $attempt)); continue; } - if ($response->status() < 500 || $attempt === $maxAttempts) { + if (! $this->shouldRetryResponse($response) || $attempt === $maxAttempts) { return $response; } - $this->sleepMilliseconds($delays[$attempt - 1] ?? 900); + $this->sleepMilliseconds($this->retryDelayMilliseconds($response, $attempt)); } throw new \RuntimeException( @@ -698,4 +710,41 @@ class BrainService { usleep($milliseconds * 1000); } + + private function shouldRetryResponse(Response $response): bool + { + $status = $response->status(); + + return $status === 408 || $status === 429 || $status >= 500; + } + + private function retryDelayMilliseconds(?Response $response, int $attempt): int + { + $retryAfter = $response?->header('Retry-After'); + if (is_string($retryAfter) && $retryAfter !== '') { + $delay = $this->parseRetryAfterMilliseconds($retryAfter); + + if ($delay !== null) { + return $delay; + } + } + + $delays = [100, 300, 900]; + + return $delays[$attempt - 1] ?? 900; + } + + private function parseRetryAfterMilliseconds(string $retryAfter): ?int + { + if (is_numeric($retryAfter)) { + return min(max((int) $retryAfter, 0) * 1000, self::MAX_RETRY_DELAY_MS); + } + + $retryAt = strtotime($retryAfter); + if ($retryAt === false) { + return null; + } + + return min(max($retryAt - time(), 0) * 1000, self::MAX_RETRY_DELAY_MS); + } } diff --git a/php/tests/Feature/Brain/CircuitBreakerTest.php b/php/tests/Feature/Brain/CircuitBreakerTest.php index 33131d5..907e476 100644 --- a/php/tests/Feature/Brain/CircuitBreakerTest.php +++ b/php/tests/Feature/Brain/CircuitBreakerTest.php @@ -6,6 +6,7 @@ declare(strict_types=1); use Core\Mcp\Services\CircuitBreaker; use Core\Mod\Agentic\Mcp\Tools\Agent\Brain\BrainList; +use Core\Mod\Agentic\Models\BrainMemory; use Core\Mod\Agentic\Services\BrainService; use Illuminate\Http\Client\Request; use Illuminate\Support\Facades\Http; @@ -23,16 +24,18 @@ function retryableBrainService(): BrainService }; } -test('CircuitBreaker_brain_list_Good_routes_failures_through_with_circuit_breaker', function (): void { +test('CircuitBreaker_brain_list_Good_executes_without_circuit_breaker_for_db_only_query', function (): void { $workspace = createWorkspace(); $breaker = Mockery::mock(CircuitBreaker::class); + BrainMemory::create([ + 'workspace_id' => $workspace->id, + 'agent_id' => 'virgil', + 'type' => 'fact', + 'content' => 'DB-backed list result.', + 'confidence' => 0.8, + ]); - $breaker->shouldReceive('call') - ->once() - ->with('brain', Mockery::type(Closure::class), Mockery::type(Closure::class)) - ->andReturnUsing(function (string $service, Closure $operation, ?Closure $fallback = null): array { - return $fallback instanceof Closure ? $fallback() : []; - }); + $breaker->shouldNotReceive('call'); $this->app->instance(CircuitBreaker::class, $breaker); @@ -40,8 +43,9 @@ test('CircuitBreaker_brain_list_Good_routes_failures_through_with_circuit_breake 'workspace_id' => $workspace->id, ]); - expect($result['code'])->toBe('service_unavailable') - ->and($result['error'])->toBe('Brain service temporarily unavailable. Memory list unavailable.'); + expect($result['success'])->toBeTrue() + ->and($result['count'])->toBe(1) + ->and($result['memories'][0]['content'])->toBe('DB-backed list result.'); }); test('CircuitBreaker_retryable_http_Bad_retries_qdrant_requests_on_503', function (): void { @@ -80,3 +84,39 @@ test('CircuitBreaker_retryable_http_Ugly_does_not_retry_qdrant_requests_on_401', expect($brain->sleepCalls)->toBe([]); Http::assertSentCount(1); }); + +test('CircuitBreaker_retryable_http_retries_qdrant_requests_on_429_using_retry_after_header', function (): void { + $brain = retryableBrainService(); + + Http::fake([ + 'http://localhost:6334/collections/openbrain/points' => Http::sequence() + ->push(['error' => 'rate limited'], 429, ['Retry-After' => '2']) + ->push(['result' => ['status' => 'ok']], 200), + ]); + + $brain->qdrantUpsert([ + ['id' => 'memory-3', 'vector' => [0.5, 0.6], 'payload' => ['type' => 'fact']], + ]); + + expect($brain->sleepCalls)->toBe([2000]); + + Http::assertSentCount(2); +}); + +test('CircuitBreaker_retryable_http_retries_qdrant_requests_on_408', function (): void { + $brain = retryableBrainService(); + + Http::fake([ + 'http://localhost:6334/collections/openbrain/points' => Http::sequence() + ->push(['error' => 'timeout'], 408) + ->push(['result' => ['status' => 'ok']], 200), + ]); + + $brain->qdrantUpsert([ + ['id' => 'memory-4', 'vector' => [0.7, 0.8], 'payload' => ['type' => 'fact']], + ]); + + expect($brain->sleepCalls)->toBe([100]); + + Http::assertSentCount(2); +}); diff --git a/php/tests/Feature/Brain/SupersedeForgetIndexCleanupTest.php b/php/tests/Feature/Brain/SupersedeForgetIndexCleanupTest.php index e4bada8..066cbe0 100644 --- a/php/tests/Feature/Brain/SupersedeForgetIndexCleanupTest.php +++ b/php/tests/Feature/Brain/SupersedeForgetIndexCleanupTest.php @@ -40,6 +40,18 @@ test('SupersedeForgetIndexCleanup_forget_Good_dispatches_delete_from_index', fun Queue::assertPushed(DeleteFromIndex::class, fn (DeleteFromIndex $job): bool => $job->memoryId === $memory->id); }); +test('SupersedeForgetIndexCleanup_forget_Bad_skips_delete_from_index_for_never_indexed_memory', function (): void { + Queue::fake(); + $memory = cleanupMemory(['indexed_at' => null]); + + cleanupBrainService()->forget($memory->id); + + expect(BrainMemory::find($memory->id))->toBeNull() + ->and(BrainMemory::withTrashed()->find($memory->id)?->trashed())->toBeTrue(); + + Queue::assertNotPushed(DeleteFromIndex::class, fn (DeleteFromIndex $job): bool => $job->memoryId === $memory->id); +}); + test('SupersedeForgetIndexCleanup_supersede_Bad_dispatches_cleanup_for_old_indexed_memory', function (): void { Queue::fake(); $workspace = createWorkspace(); diff --git a/php/tests/Feature/Mcp/BrainSchemaOrgTest.php b/php/tests/Feature/Mcp/BrainSchemaOrgTest.php index e00c8ad..f3449c1 100644 --- a/php/tests/Feature/Mcp/BrainSchemaOrgTest.php +++ b/php/tests/Feature/Mcp/BrainSchemaOrgTest.php @@ -66,11 +66,28 @@ test('BrainSchemaOrg_brain_remember_Good_accepts_org_and_forwards_it', function ]); expect($tool->inputSchema()['properties'])->toHaveKey('org') + ->and($tool->inputSchema()['properties']['org']['maxLength'])->toBe(128) ->and($result['success'])->toBeTrue() ->and($brain->remembered['org'])->toBe('core') ->and($result['memory']['org'])->toBe('core'); }); +test('BrainSchemaOrg_brain_remember_Bad_rejects_org_longer_than_128_chars', function (): void { + $workspace = createWorkspace(); + passThroughBrainCircuitBreaker($this->app); + + $tool = new BrainRemember; + + $tool->handle([ + 'content' => 'Shared organisation memory.', + 'type' => 'fact', + 'org' => str_repeat('o', 129), + ], [ + 'workspace_id' => $workspace->id, + 'session_id' => 'session-1', + ]); +})->throws(InvalidArgumentException::class, 'org exceeds maximum length of 128 characters'); + test('BrainSchemaOrg_brain_recall_Bad_accepts_org_filter_and_forwards_it', function (): void { $workspace = createWorkspace(); $brain = new class extends BrainService @@ -113,11 +130,28 @@ test('BrainSchemaOrg_brain_recall_Bad_accepts_org_filter_and_forwards_it', funct ]); expect($tool->inputSchema()['properties']['filter']['properties'])->toHaveKey('org') + ->and($tool->inputSchema()['properties']['filter']['properties']['org']['maxLength'])->toBe(128) ->and($result['success'])->toBeTrue() ->and($brain->captured['filter']['org'])->toBe('core') ->and($brain->captured['workspace_id'])->toBe($workspace->id); }); +test('BrainSchemaOrg_brain_recall_Ugly_rejects_filter_org_longer_than_128_chars', function (): void { + $workspace = createWorkspace(); + passThroughBrainCircuitBreaker($this->app); + + $tool = new BrainRecall; + + $tool->handle([ + 'query' => 'org-filtered recall', + 'filter' => [ + 'org' => str_repeat('o', 129), + ], + ], [ + 'workspace_id' => $workspace->id, + ]); +})->throws(InvalidArgumentException::class, 'org exceeds maximum length of 128 characters'); + test('BrainSchemaOrg_brain_list_Ugly_accepts_org_filter_without_validation_error', function (): void { $workspace = createWorkspace(); passThroughBrainCircuitBreaker($this->app); @@ -148,6 +182,7 @@ test('BrainSchemaOrg_brain_list_Ugly_accepts_org_filter_without_validation_error ]); expect($tool->inputSchema()['properties'])->toHaveKey('org') + ->and($tool->inputSchema()['properties']['org']['maxLength'])->toBe(128) ->and($result['success'])->toBeTrue() ->and($result['count'])->toBe(1) ->and($result['memories'][0]['id'])->toBe($matching->id)