diff --git a/php/Services/BrainService.php b/php/Services/BrainService.php index b718099..10c1593 100644 --- a/php/Services/BrainService.php +++ b/php/Services/BrainService.php @@ -36,6 +36,8 @@ class BrainService private const MEMORY_LOCK_WAIT_SECONDS = 5; + private const MAX_SUPERSEDE_CHAIN_DEPTH = 100; + private string $qdrantApiKey; public function __construct( @@ -125,11 +127,11 @@ class BrainService $attributes['indexed_at'] = null; $cleanupIds = []; - $supersededId = $this->normaliseMemoryId($attributes['supersedes_id'] ?? null); - $remember = function () use ($attributes, &$cleanupIds): BrainMemory { - return DB::connection('brain')->transaction(function () use ($attributes, &$cleanupIds): BrainMemory { + $requestedSupersededId = $this->normaliseMemoryId($attributes['supersedes_id'] ?? null); + $remember = function (array $rememberAttributes) use (&$cleanupIds): BrainMemory { + return DB::connection('brain')->transaction(function () use ($rememberAttributes, &$cleanupIds): BrainMemory { $memory = new BrainMemory; - $memory->fill($attributes); + $memory->fill($rememberAttributes); $memory->save(); $this->deleteSupersededMemory($memory->supersedes_id, $cleanupIds); @@ -138,9 +140,20 @@ class BrainService }); }; - $memory = $supersededId !== null - ? $this->withMemoryIndexLock($supersededId, $remember) - : $remember(); + $memory = $requestedSupersededId !== null + ? $this->withMemoryIndexLock($requestedSupersededId, function () use ($requestedSupersededId, $attributes, $remember): BrainMemory { + $resolvedSupersededId = $this->resolveSupersedeHeadId($requestedSupersededId); + $rememberAttributes = $attributes; + // Retry calls should follow the current head rather than branch from a deleted ancestor. + $rememberAttributes['supersedes_id'] = $resolvedSupersededId; + + if ($resolvedSupersededId === $requestedSupersededId) { + return $remember($rememberAttributes); + } + + return $this->withMemoryIndexLock($resolvedSupersededId, fn (): BrainMemory => $remember($rememberAttributes)); + }) + : $remember($attributes); foreach ($cleanupIds as $cleanupId) { DeleteFromIndex::dispatch($cleanupId); @@ -859,13 +872,59 @@ class BrainService $superseded = BrainMemory::query()->find($supersededId); if (! $superseded instanceof BrainMemory) { - return; + throw new \RuntimeException("Superseded memory head vanished during delete: {$supersededId}"); } $cleanupIds[] = $superseded->id; $superseded->delete(); } + private function resolveSupersedeHeadId(string $supersededId): string + { + $currentId = $supersededId; + $visited = []; + $depth = 0; + + while (true) { + if (isset($visited[$currentId])) { + throw new \RuntimeException("Detected cycle while resolving supersede chain: {$supersededId}"); + } + + if ($depth >= self::MAX_SUPERSEDE_CHAIN_DEPTH) { + throw new \RuntimeException( + "Supersede chain exceeded ".self::MAX_SUPERSEDE_CHAIN_DEPTH." hops: {$supersededId}" + ); + } + + $visited[$currentId] = true; + + $successor = BrainMemory::withTrashed() + ->where('supersedes_id', $currentId) + ->orderByDesc('created_at') + ->orderByDesc('id') + ->first(); + + if ($successor instanceof BrainMemory) { + $currentId = $successor->id; + $depth++; + + continue; + } + + $current = BrainMemory::withTrashed()->find($currentId); + + if (! $current instanceof BrainMemory) { + throw new \InvalidArgumentException("Superseded memory not found: {$supersededId}"); + } + + if ($current->trashed()) { + throw new \InvalidArgumentException("Superseded memory has no live head: {$supersededId}"); + } + + return $current->id; + } + } + private function memoryExists(string $id): bool { return BrainMemory::query()->whereKey($id)->exists(); diff --git a/php/tests/Feature/Brain/SupersedeForgetIndexCleanupTest.php b/php/tests/Feature/Brain/SupersedeForgetIndexCleanupTest.php index 3150d00..630fb45 100644 --- a/php/tests/Feature/Brain/SupersedeForgetIndexCleanupTest.php +++ b/php/tests/Feature/Brain/SupersedeForgetIndexCleanupTest.php @@ -8,6 +8,7 @@ use Core\Mod\Agentic\Jobs\DeleteFromIndex; use Core\Mod\Agentic\Jobs\EmbedMemory; use Core\Mod\Agentic\Models\BrainMemory; use Core\Mod\Agentic\Services\BrainService; +use Illuminate\Support\Str; use Illuminate\Support\Facades\Http; use Illuminate\Support\Facades\Queue; @@ -120,7 +121,8 @@ test('SupersedeForgetIndexCleanup_supersede_Bad_dispatches_cleanup_for_old_index expect(BrainMemory::find($oldMemory->id))->toBeNull() ->and(BrainMemory::withTrashed()->find($oldMemory->id)?->trashed())->toBeTrue() - ->and($newMemory->indexed_at)->toBeNull(); + ->and($newMemory->indexed_at)->toBeNull() + ->and($newMemory->supersedes_id)->toBe($oldMemory->id); Queue::assertPushed(DeleteFromIndex::class, fn (DeleteFromIndex $job): bool => $job->memoryId === $oldMemory->id); Queue::assertPushed(EmbedMemory::class, fn (EmbedMemory $job): bool => $job->memoryId === $newMemory->id); @@ -148,12 +150,111 @@ test('SupersedeForgetIndexCleanup_supersede_Ugly_dispatches_cleanup_for_never_in expect(BrainMemory::find($oldMemory->id))->toBeNull() ->and(BrainMemory::withTrashed()->find($oldMemory->id)?->trashed())->toBeTrue() - ->and($newMemory->indexed_at)->toBeNull(); + ->and($newMemory->indexed_at)->toBeNull() + ->and($newMemory->supersedes_id)->toBe($oldMemory->id); Queue::assertPushed(DeleteFromIndex::class, fn (DeleteFromIndex $job): bool => $job->memoryId === $oldMemory->id); Queue::assertPushed(EmbedMemory::class, fn (EmbedMemory $job): bool => $job->memoryId === $newMemory->id); }); +test('SupersedeForgetIndexCleanup_supersede_Good_retries_follow_the_current_head', function (): void { + Queue::fake(); + $brain = cleanupBrainService(); + $workspace = createWorkspace(); + $originalMemory = cleanupMemory([ + 'workspace_id' => $workspace->id, + 'content' => 'Original memory for retry chain.', + ]); + + $replacement = $brain->remember([ + 'workspace_id' => $workspace->id, + 'agent_id' => 'virgil', + 'type' => 'observation', + 'content' => 'First replacement memory.', + 'confidence' => 0.91, + 'org' => 'core', + 'project' => 'agent', + 'supersedes_id' => $originalMemory->id, + ]); + + $retriedReplacement = $brain->remember([ + 'workspace_id' => $workspace->id, + 'agent_id' => 'virgil', + 'type' => 'observation', + 'content' => 'Retried replacement memory.', + 'confidence' => 0.92, + 'org' => 'core', + 'project' => 'agent', + 'supersedes_id' => $originalMemory->id, + ]); + + expect($replacement->supersedes_id)->toBe($originalMemory->id) + ->and($retriedReplacement->supersedes_id)->toBe($replacement->id) + ->and(BrainMemory::find($originalMemory->id))->toBeNull() + ->and(BrainMemory::withTrashed()->find($originalMemory->id)?->trashed())->toBeTrue() + ->and(BrainMemory::find($replacement->id))->toBeNull() + ->and(BrainMemory::withTrashed()->find($replacement->id)?->trashed())->toBeTrue() + ->and(BrainMemory::find($retriedReplacement->id))->not->toBeNull(); + + Queue::assertPushed(DeleteFromIndex::class, 2); + Queue::assertPushed(DeleteFromIndex::class, fn (DeleteFromIndex $job): bool => $job->memoryId === $originalMemory->id); + Queue::assertPushed(DeleteFromIndex::class, fn (DeleteFromIndex $job): bool => $job->memoryId === $replacement->id); + Queue::assertPushed(EmbedMemory::class, 2); +}); + +test('SupersedeForgetIndexCleanup_supersede_Bad_throws_for_unknown_memory', function (): void { + Queue::fake(); + $workspace = createWorkspace(); + $missingMemoryId = Str::uuid()->toString(); + + expect(fn () => cleanupBrainService()->remember([ + 'workspace_id' => $workspace->id, + 'agent_id' => 'virgil', + 'type' => 'observation', + 'content' => 'Memory with missing supersede target.', + 'confidence' => 0.88, + 'org' => 'core', + 'project' => 'agent', + 'supersedes_id' => $missingMemoryId, + ]))->toThrow(\InvalidArgumentException::class, "Superseded memory not found: {$missingMemoryId}"); + + expect(BrainMemory::count())->toBe(0); + Queue::assertNothingPushed(); +}); + +test('SupersedeForgetIndexCleanup_supersede_Ugly_detects_cycles_before_writing', function (): void { + Queue::fake(); + $workspace = createWorkspace(); + $firstMemory = cleanupMemory([ + 'workspace_id' => $workspace->id, + 'content' => 'Cycle root memory.', + ]); + $secondMemory = cleanupMemory([ + 'workspace_id' => $workspace->id, + 'content' => 'Cycle successor memory.', + 'supersedes_id' => $firstMemory->id, + ]); + + $firstMemory->supersedes_id = $secondMemory->id; + $firstMemory->save(); + + expect(fn () => cleanupBrainService()->remember([ + 'workspace_id' => $workspace->id, + 'agent_id' => 'virgil', + 'type' => 'observation', + 'content' => 'Memory that should fail on cycle.', + 'confidence' => 0.9, + 'org' => 'core', + 'project' => 'agent', + 'supersedes_id' => $firstMemory->id, + ]))->toThrow(\RuntimeException::class, "Detected cycle while resolving supersede chain: {$firstMemory->id}"); + + expect(BrainMemory::count())->toBe(2) + ->and(BrainMemory::withTrashed()->find($firstMemory->id)?->trashed())->toBeFalse() + ->and(BrainMemory::withTrashed()->find($secondMemory->id)?->trashed())->toBeFalse(); + Queue::assertNothingPushed(); +}); + test('SupersedeForgetIndexCleanup_supersede_Ugly_skips_late_index_writes_for_deleted_memory', function (): void { Queue::fake(); Http::fake();