src/storage/protocols/vector/vector-store.ts (2)

45-60: ⚠️ Potential issue | 🔴 Critical

Preserve idempotent createBucket() once the bucket limit is reached.

Because the max-count check at Line 48 runs before the ConflictException fast-path at Lines 53-57, a retry for an already-created bucket starts failing with S3VectorMaxBucketsExceeded as soon as the limit is full. The insert needs to happen first under the lock, then the limit should be enforced only if a new row was actually created.

Possible fix

     await this.db.withTransaction(async (tnx) => {
       await tnx.lockResource('global', VECTOR_BUCKET_COUNT_LOCK)
 
-      const bucketCount = await tnx.countBuckets()
-      if (bucketCount >= this.config.maxBucketCount) {
-        throw ERRORS.S3VectorMaxBucketsExceeded(this.config.maxBucketCount)
-      }
-
       try {
         await tnx.createVectorBucket(bucketName)
       } catch (e) {
         if (e instanceof ConflictException) {
           return
         }
         throw e
       }
+
+      const bucketCount = await tnx.countBuckets()
+      if (bucketCount > this.config.maxBucketCount) {
+        throw ERRORS.S3VectorMaxBucketsExceeded(this.config.maxBucketCount)
+      }
     })

As per coding guidelines, "Highlight only issues that could cause runtime errors, data loss, or severe maintainability issues."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/storage/protocols/vector/vector-store.ts` around lines 45 - 60, The
transaction currently checks bucketCount via tnx.countBuckets() before
attempting tnx.createVectorBucket(bucketName), causing retries to throw
ERRORS.S3VectorMaxBucketsExceeded even when the bucket already exists; change
the logic inside withTransaction/tnx so you first attempt
tnx.createVectorBucket(bucketName) and handle ConflictException (return) as a
fast-path, then after a successful create check tnx.countBuckets() and throw
ERRORS.S3VectorMaxBucketsExceeded only if the create actually added a new row
and the count exceeds this.config.maxBucketCount; keep the
lockResource('global', VECTOR_BUCKET_COUNT_LOCK) and preserve the
ConflictException handling around createVectorBucket to maintain idempotency.

---

65-67: ⚠️ Potential issue | 🟠 Major

Don't let a busy bucket monopolize the global count lock.

deleteBucket() takes VECTOR_BUCKET_COUNT_LOCK at Line 66 before waiting on the per-bucket lock, while createVectorIndex() keeps that bucket lock until the transaction ends after reserve(), remote createVectorIndex(), and confirm(). One slow index creation on bucket A can therefore block every unrelated createBucket()/deleteBucket() behind the global lock. The lock scheme needs to avoid waiting on a bucket while already holding the global count lock.

As per coding guidelines, "Highlight only issues that could cause runtime errors, data loss, or severe maintainability issues."

Also applies to: 137-185

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/storage/protocols/vector/vector-store.ts` around lines 65 - 67,
deleteBucket() currently acquires VECTOR_BUCKET_COUNT_LOCK before waiting on the
per-bucket lock which can cause the global lock to be held while waiting for
slow bucket operations (and createVectorIndex() holds the bucket lock across
reserve()/createVectorIndex()/confirm()), so reorder locking to avoid holding
the global count lock while waiting on a bucket: change deleteBucket() (and the
similar path in createVectorIndex()/reserve()/confirm()) to acquire the
per-bucket lock (lockResource('bucket', bucketName)) first, then perform any
bucket-specific checks/operations, and only acquire VECTOR_BUCKET_COUNT_LOCK
when you actually need to update the global count (release the bucket lock if
necessary before obtaining the global lock and re-acquire to finalize if you
must modify both); alternatively use a two-phase approach: lock bucket → check
state → release bucket → lock global → re-lock bucket to perform count mutation,
ensuring no code path holds both locks while blocking on I/O or long operations.