core/agent
8
0
Fork 0
Code Issues 5 Pull requests Projects Releases Packages Wiki Activity Actions

fix(agent/brain): batch — org maxLength + retry semantics + forget index cleanup

Browse source 
Codex 5.5 batch lane processed 8 brain Mantis tickets. 4 implemented,
1 stale, 3 deferred.

Tickets implemented:
- #313 — MCP schemas (BrainRemember/Recall/List): org field maxLength=128 with runtime validation; recall filter.org also bounded; pest test coverage added
- #314 — BrainList: removed withCircuitBreaker('brain') from DB-only handler; CircuitBreakerTest updated to assert no breaker call
- #315 — BrainService.retryableHttp(): now retries 408 (request-timeout), 429 (rate-limit), and 5xx; honours Retry-After header; focused retry tests added
- #326 — BrainService.forget(): dispatches DeleteFromIndex only when row has indexed_at (was unconditional); SupersedeForgetIndexCleanupTest covers never-indexed case

Tickets stale-fixed: #316 (RememberKnowledge already rejects missing/deleted supersedes target before dangling retry)
Tickets deferred: #121 (cross-surface audit), #311 (retry-inside-breaker architectural redesign), #312 (no authoritative org claim in MCP request context yet)

Co-authored-by: Codex <noreply@openai.com>
Closes tasks.lthn.sh/view.php?id=313
Closes tasks.lthn.sh/view.php?id=314
Closes tasks.lthn.sh/view.php?id=315
Closes tasks.lthn.sh/view.php?id=326
This commit is contained in:
Snider 2026-04-25 14:55:35 +01:00
parent bf10d16f49
commit 6832d40587
7 changed files with 185 additions and 41 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

37
php/Mcp/Tools/Agent/Brain/BrainList.php
View file

@ -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(),
]);
}
}

10
php/Mcp/Tools/Agent/Brain/BrainRecall.php
View file

@ -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'],

7
php/Mcp/Tools/Agent/Brain/BrainRemember.php
View file

@ -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(),

67
php/Services/BrainService.php
View file

@ -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);
}
}

58
php/tests/Feature/Brain/CircuitBreakerTest.php
View file

@ -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);
});

12
php/tests/Feature/Brain/SupersedeForgetIndexCleanupTest.php
View file

@ -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();

35
php/tests/Feature/Mcp/BrainSchemaOrgTest.php
View file

@ -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)