From d82ad2b9b19e41728be4f0f3819b8a009a9b423b Mon Sep 17 00:00:00 2001
From: Snider <snider@host.uk.com>
Date: Tue, 3 Mar 2026 09:47:42 +0000
Subject: [PATCH] fix(brain): address code quality review findings

- Move BrainMemory::create() inside BrainService::remember() for
  full atomicity (DB + Qdrant in single transaction)
- Add forWorkspace() scope to recall() MariaDB query (tenant isolation)
- Wrap forget() in DB::transaction (MariaDB first, then Qdrant)
- Check qdrantDelete() response and log warnings on failure
- Validate embed() response is a non-empty array

Co-Authored-By: Virgil <virgil@lethean.io>
---
 Mcp/Tools/Agent/Brain/BrainRemember.php |  4 +-
 Services/BrainService.php               | 59 +++++++++++++++++--------
 2 files changed, 41 insertions(+), 22 deletions(-)

diff --git a/Mcp/Tools/Agent/Brain/BrainRemember.php b/Mcp/Tools/Agent/Brain/BrainRemember.php
index 38a8e81..a01ec11 100644
--- a/Mcp/Tools/Agent/Brain/BrainRemember.php
+++ b/Mcp/Tools/Agent/Brain/BrainRemember.php
@@ -130,7 +130,7 @@ class BrainRemember extends AgentTool
         return $this->withCircuitBreaker('brain', function () use (
             $content, $type, $workspaceId, $agentId, $tags, $project, $confidence, $supersedes, $expiresIn
         ) {
-            $memory = BrainMemory::create([
+            $memory = app(BrainService::class)->remember([
                 'workspace_id' => $workspaceId,
                 'agent_id' => $agentId,
                 'type' => $type,
@@ -142,8 +142,6 @@ class BrainRemember extends AgentTool
                 'expires_at' => $expiresIn ? now()->addHours($expiresIn) : null,
             ]);
 
-            app(BrainService::class)->remember($memory);
-
             return $this->success([
                 'memory' => $memory->toMcpContext(),
             ]);
diff --git a/Services/BrainService.php b/Services/BrainService.php
index da27181..e71505d 100644
--- a/Services/BrainService.php
+++ b/Services/BrainService.php
@@ -40,37 +40,51 @@ class BrainService
             throw new \RuntimeException("Ollama embedding failed: {$response->status()}");
         }
 
-        return $response->json('embedding');
+        $embedding = $response->json('embedding');
+
+        if (! is_array($embedding) || empty($embedding)) {
+            throw new \RuntimeException('Ollama returned no embedding vector');
+        }
+
+        return $embedding;
     }
 
     /**
      * Store a memory in both MariaDB and Qdrant.
      *
-     * If the memory supersedes an older one, the old entry is
-     * removed from both stores.
+     * Creates the MariaDB record and upserts the Qdrant vector within
+     * a single DB transaction. If the memory supersedes an older one,
+     * the old entry is soft-deleted from MariaDB and removed from Qdrant.
+     *
+     * @param  array<string, mixed>  $attributes  Fillable attributes for BrainMemory
+     * @return BrainMemory The created memory
      */
-    public function remember(BrainMemory $memory): void
+    public function remember(array $attributes): BrainMemory
     {
-        $vector = $this->embed($memory->content);
+        $vector = $this->embed($attributes['content']);
 
-        $payload = $this->buildQdrantPayload($memory->id, [
-            'workspace_id' => $memory->workspace_id,
-            'agent_id' => $memory->agent_id,
-            'type' => $memory->type,
-            'tags' => $memory->tags ?? [],
-            'project' => $memory->project,
-            'confidence' => $memory->confidence,
-            'created_at' => $memory->created_at->toIso8601String(),
-        ]);
-        $payload['vector'] = $vector;
+        return DB::transaction(function () use ($attributes, $vector) {
+            $memory = BrainMemory::create($attributes);
+
+            $payload = $this->buildQdrantPayload($memory->id, [
+                'workspace_id' => $memory->workspace_id,
+                'agent_id' => $memory->agent_id,
+                'type' => $memory->type,
+                'tags' => $memory->tags ?? [],
+                'project' => $memory->project,
+                'confidence' => $memory->confidence,
+                'created_at' => $memory->created_at->toIso8601String(),
+            ]);
+            $payload['vector'] = $vector;
 
-        DB::transaction(function () use ($payload, $memory) {
             $this->qdrantUpsert([$payload]);
 
             if ($memory->supersedes_id) {
                 BrainMemory::where('id', $memory->supersedes_id)->delete();
                 $this->qdrantDelete([$memory->supersedes_id]);
             }
+
+            return $memory;
         });
     }
 
@@ -112,6 +126,7 @@ class BrainService
         }
 
         $memories = BrainMemory::whereIn('id', $ids)
+            ->forWorkspace($workspaceId)
             ->active()
             ->latestVersions()
             ->get()
@@ -129,8 +144,10 @@ class BrainService
      */
     public function forget(string $id): void
     {
-        $this->qdrantDelete([$id]);
-        BrainMemory::where('id', $id)->delete();
+        DB::transaction(function () use ($id) {
+            BrainMemory::where('id', $id)->delete();
+            $this->qdrantDelete([$id]);
+        });
     }
 
     /**
@@ -236,9 +253,13 @@ class BrainService
      */
     private function qdrantDelete(array $ids): void
     {
-        Http::timeout(10)
+        $response = Http::timeout(10)
             ->post("{$this->qdrantUrl}/collections/{$this->collection}/points/delete", [
                 'points' => $ids,
             ]);
+
+        if (! $response->successful()) {
+            Log::warning("Qdrant delete failed: {$response->status()}", ['ids' => $ids, 'body' => $response->body()]);
+        }
     }
 }