fix(agent/brain): walk supersede chain to current head + cycle guard
remember() now resolves a stale supersedes_id to the current live head
before writing — when X has been superseded by Y, a retried call with
supersedes_id=X automatically links the new memory to Y instead of
silently dropping the supersede.
- Walk the chain from supplied supersedes_id to find the active head
- Cap the walk at depth 100 (cycle/runaway protection)
- Throw RuntimeException("Detected cycle while resolving supersede chain")
on detected cycle, BEFORE any DB write
- Throw InvalidArgumentException("Superseded memory not found") when
the original supersedes_id never existed
- deleteSupersededMemory no longer silently no-ops once the resolved
head is expected to exist
Pest coverage extended:
- Direct chain link (X exists, succeeds with X→linked)
- Retry path (X→Y, then retry on X produces Z→Y, walks chain)
- Never-existed target (graceful error)
- Synthetic X↔Y cycle (caps walk + throws, no writes leak)
Co-authored-by: Codex <noreply@openai.com>
Closes tasks.lthn.sh/view.php?id=316
This commit is contained in:
parent
167ce9783e
commit
dea64f4099
2 changed files with 170 additions and 10 deletions
|
|
@ -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();
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue