feat(mongodb): implement complete MongoDB memory adapter#1000
feat(mongodb): implement complete MongoDB memory adapter#1000UmeshpJadhav wants to merge 9 commits intoVoltAgent:mainfrom
Conversation
🦋 Changeset detectedLatest commit: ee9c1ff The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new MongoDB-backed memory storage package for VoltAgent: implementation, public exports, build/test configs, integration/unit tests, Docker Compose for integration testing, README, example, and a changeset announcing the initial release. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Adapter as MongoDBMemoryAdapter
participant DB as MongoDB
Client->>Adapter: createConversation(input)
Adapter->>DB: insert conversation document (collectionPrefix + conversations)
DB-->>Adapter: inserted document ( _id, createdAt, updatedAt )
Adapter-->>Client: return Conversation (id, createdAt, updatedAt)
Client->>Adapter: addMessage(message, userId, conversationId)
Adapter->>DB: find conversation by id
alt conversation exists
Adapter->>DB: insert message (collectionPrefix + messages), enforce unique messageId
DB-->>Adapter: ack
Adapter-->>Client: success
else conversation missing
Adapter-->>Client: ConversationNotFoundError
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@packages/mongodb/README.md`:
- Around line 35-39: Replace the erroneous placeholder row "str | str | str |
str" that follows the header "Option | Type | Default | Description" with a
proper Markdown table separator row made of hyphens for each column separated by
pipes (i.e., a hyphen group under each header). Locate the header and the
placeholder row in packages/mongodb/README.md and update the second row so the
table renders correctly while keeping the existing header and subsequent rows
(like the `connection`, `database`, `collectionPrefix` lines) unchanged.
In `@packages/mongodb/src/memory-adapter.spec.ts`:
- Around line 570-608: In saveConversationSteps, the expression step.id ||
this.generateId() is evaluated twice when building the replaceOne operation
which can create mismatched _id values for records without step.id; fix by
computing a single id per step (e.g., const id = step.id || this.generateId())
before constructing the replacement object and use that id for both filter._id
and replacement._id so replaceOne.upsert uses a consistent identifier (refer to
saveConversationSteps, steps, generateId, getCollection, and stepsCollection to
locate the code).
In `@packages/mongodb/src/memory-adapter.ts`:
- Around line 571-609: In saveConversationSteps, the code calls step.id ||
this.generateId() twice which can yield different IDs for filter vs replacement;
for each step compute a single id (e.g., const id = step.id ??
this.generateId()) and use that id for both filter._id and replacement._id in
the operations array so the upsert targets the same document; update references
in the stepsCollection.bulkWrite payload to use that computed id.
- Around line 786-975: Remove the erroneous .toISOString() conversions so
createdAt and updatedAt are returned as Date objects to match
WorkflowStateEntry: in getWorkflowState replace createdAt: (state as
any).createdAt.toISOString() and updatedAt: (state as
any).updatedAt.toISOString() with the raw Date values, and in queryWorkflowRuns
replace createdAt: state.createdAt.toISOString() and updatedAt:
state.updatedAt.toISOString() with the Date values; keep
getSuspendedWorkflowStates as-is (it already returns Date objects).
🧹 Nitpick comments (5)
packages/mongodb/package.json (1)
49-52: Avoid fixed sleep for MongoDB readiness if possible.
A hardsleep 10can be flaky on slower CI. Consider a readiness/healthcheck ordocker compose --waitif supported.♻️ Possible update (verify docker compose supports --wait)
- "test:integration:setup": "docker compose -f docker-compose.test.yaml up -d && sleep 10", + "test:integration:setup": "docker compose -f docker-compose.test.yaml up -d --wait",packages/mongodb/src/index.integration.test.ts (2)
388-492: Tighten workflow state fixtures typing.
anycasts around Line 390/416/442/469 reduce compile-time coverage. PreferWorkflowStateEntry-typed fixtures (or a typed helper builder) overas any.
As per coding guidelines: Maintain type safety in TypeScript-first codebase.
498-583: Use typed fixtures for conversation steps.
any[]andas anyaround Line 508/553 weaken type safety. PreferConversationStepRecord[]fixtures to keep tests aligned with adapter contracts.
As per coding guidelines: Maintain type safety in TypeScript-first codebase.packages/mongodb/src/memory-adapter.ts (1)
195-350: Reduceanyusage in message operations for stronger type safety.
filter: anyandmsg: any(Line 286 onward) weaken compile-time checks. Consider defining MongoDB document interfaces and using typed filters/collections to avoidas any.
As per coding guidelines: Maintain type safety in TypeScript-first codebase.packages/mongodb/src/memory-adapter.spec.ts (1)
1-21: Avoid duplicating the adapter implementation in a.spec.ts.
Consider importing/re-exporting the adapter frompackages/mongodb/src/memory-adapter.tsso fixes don’t have to be duplicated and drift doesn’t occur.
|
Hey @UmeshpJadhav , thanks a lot for the PR! 🙌 |
There was a problem hiding this comment.
1 issue found across 13 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/mongodb/src/memory-adapter.ts">
<violation number="1" location="packages/mongodb/src/memory-adapter.ts:381">
P2: createConversation uses a check-then-insert pattern without handling MongoDB duplicate key errors; concurrent creation can raise an E11000 error instead of ConversationAlreadyExistsError.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/mongodb/src/memory-adapter.spec.ts (1)
1-980:⚠️ Potential issue | 🔴 CriticalRemove
memory-adapter.spec.tsor replace with unit tests; integration tests are properly implemented inindex.integration.test.ts.The spec file (lines 1–980) is an exact duplicate of the implementation in
memory-adapter.tswith only trivial formatting differences, containing no test code. Comprehensive integration tests already exist inindex.integration.test.ts(651 lines) covering all major operations. The.spec.tsfile should either be deleted or converted to unit tests. Per project conventions, test files should follow the**/*.test.tspattern.
🧹 Nitpick comments (3)
packages/mongodb/src/memory-adapter.ts (3)
121-128: Initialization guard has a logical issue.The check at lines 125-127 will never trigger because
this.initPromiseis assigned at line 87 beforeinitialize()completes, and wheninitialize()is called again,this.initPromisewill always be truthy. However, sinceinitialize()is private and only called once from the constructor, this redundant check is benign but misleading.The guard at line 122 (
if (this.initialized) return) provides the actual protection after initialization completes.♻️ Simplify the guard logic
private async initialize(): Promise<void> { if (this.initialized) return; - // Prevent multiple simultaneous initializations - if (this.initPromise && !this.initialized) { - return this.initPromise; - } - try {
556-564: Consider using a transaction for cascade delete.The delete operation removes from three collections sequentially. If the process crashes between deletions, orphaned records may remain. While not critical for most use cases, a transaction would ensure atomicity.
♻️ Wrap in a transaction for atomicity
async deleteConversation(id: string): Promise<void> { await this.initPromise; const conversationsCollection = this.getCollection("conversations"); - - // MongoDB will cascade delete messages and steps automatically via application logic const messagesCollection = this.getCollection("messages"); const stepsCollection = this.getCollection("steps"); - await messagesCollection.deleteMany({ conversationId: id } as any); - await stepsCollection.deleteMany({ conversationId: id } as any); - await conversationsCollection.deleteOne({ _id: id } as any); + const session = this.client.startSession(); + try { + await session.withTransaction(async () => { + await messagesCollection.deleteMany({ conversationId: id } as any, { session }); + await stepsCollection.deleteMany({ conversationId: id } as any, { session }); + await conversationsCollection.deleteOne({ _id: id } as any, { session }); + }); + } finally { + await session.endSession(); + } this.log(`Deleted conversation ${id}`); }
102-106: Consider using a more robust ID generation method.
Math.random()is not cryptographically secure and has a theoretical collision risk under high load. For production use with unique constraints, consider usingcrypto.randomUUID()or MongoDB'sObjectId.♻️ Use crypto.randomUUID() for better uniqueness
+import { randomUUID } from "crypto"; + /** * Generate a random ID */ private generateId(): string { - return ( - Math.random().toString(36).substring(2, 15) + Math.random().toString(36).substring(2, 15) - ); + return randomUUID(); }
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/mongodb/src/memory-adapter.ts">
<violation number="1" location="packages/mongodb/src/memory-adapter.ts:328">
P2: deleteMessages deletes by conversationId only without any userId/ownership check, allowing deletion of other users’ conversation data if the ID is known.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@packages/mongodb/src/memory-adapter.spec.ts`:
- Around line 1-980: The spec file contains the full MongoDBMemoryAdapter
implementation (class MongoDBMemoryAdapter and methods like addMessage,
addMessages, createConversation, getConversation, saveConversationSteps,
getWorkingMemory, setWorkflowMemory, setWorkflowState, etc.) instead of tests;
move this implementation into the proper source file (e.g., memory-adapter.ts)
or delete it from memory-adapter.spec.ts and replace with the actual test suite
for MongoDBMemoryAdapter, creating appropriate unit tests that import
MongoDBMemoryAdapter and exercise methods such as createConversation,
addMessage, getMessages, saveConversationSteps, and workflow state functions.
In `@packages/mongodb/src/memory-adapter.ts`:
- Around line 195-233: In addMessage, remove the explicit _id: undefined from
the document passed to messagesCollection.insertOne so MongoDB can auto-generate
ObjectIds; locate the insert in the addMessage method and delete the _id
property from the inserted object literal (keep messageId, conversationId,
userId, role, parts, metadata, formatVersion, createdAt intact) and keep the
existing 11000 duplicate-key catch logic as-is.
- Around line 235-272: The batch insert in addMessages is setting _id: undefined
which the MongoDB driver serializes as null causing duplicate key errors; update
the documentsToInsert construction in addMessages to NOT include an _id property
at all (so each document lets MongoDB generate a unique ObjectId) before calling
messagesCollection.insertMany, leaving messageId generation and other fields
unchanged.
- Around line 613-654: The saveConversationSteps method currently uses
replaceOne which resets createdAt on every upsert; change the bulk operations in
saveConversationSteps to use updateOne with upsert:true, using $set for mutable
fields (conversationId, userId, agentId, agentName, operationId, stepIndex,
type, role, content, arguments, result, usage, subAgentId, subAgentName, etc.)
and $setOnInsert to set createdAt (and _id when generated) so the original
creation timestamp is preserved; follow the same pattern used in
setWorkingMemory and ensure you still generate an id via this.generateId() when
step.id is missing and include it in the $setOnInsert._id.
🧹 Nitpick comments (2)
packages/mongodb/src/memory-adapter.ts (2)
99-106: Prefer a stronger ID source than Math.random.
Considercrypto.randomUUID()to reduce collision risk and align with common Node practices.♻️ Proposed update
- private generateId(): string { - return ( - Math.random().toString(36).substring(2, 15) + Math.random().toString(36).substring(2, 15) - ); - } + private generateId(): string { + return randomUUID(); + }Additional import (outside this line range):
+import { randomUUID } from "crypto";
377-416: Tighten collection typing to avoidany.
Consider defining collection document interfaces and usinggetCollection<ConversationDocument>()to preserve type safety and reduce casts.As per coding guidelines: Maintain type safety in TypeScript-first codebase.
|
@omeraplak Yes, I've just pushed the updates I fixed the build errors and addressed all the code review comments. It should be good to go now. |
| "@voltagent/mongodb": minor | ||
| --- | ||
|
|
||
| Initial release of MongoDB memory storage adapter |
There was a problem hiding this comment.
| Initial release of MongoDB memory storage adapter | |
| feat: initial release of MongoDB memory storage adapter |
There was a problem hiding this comment.
Can we also add minimal usage example?
|
Hey @UmeshpJadhav , |
|
@UmeshpJadhav It would be great to get this finalized and merged soon. Thank you so much 🙏 |
|
I opened a follow-up PR that is based on this branch and includes the requested docs updates + minimal usage example: #1127\n\nIt adds:\n- MongoDB mention in the memory overview\n- a dedicated MongoDB memory docs page\n- README minimal usage example\n- llms.txt update\n\nIf easier, maintainers can merge #1127 directly or cherry-pick the docs commit from that branch. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
examples/with-mongodb/src/index.ts (1)
41-41: Make server port configurable via environment variable.Line 41 hardcodes
3141; usingPORTimproves local/dev/CI portability.Suggested refactor
+const port = Number(process.env.PORT ?? 3141); + new VoltAgent({ agents: { agent, }, logger, - server: honoServer({ port: 3141 }), + server: honoServer({ port }), });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/with-mongodb/src/index.ts` at line 41, Change the hardcoded port in the honoServer configuration to read from an environment variable so the server is configurable; update the server: honoServer({ port: 3141 }) call to use process.env.PORT (or a similarly named env var) parsed as a number with a sensible default (e.g., 3141) so honoServer receives a numeric port; ensure any type conversion (Number or parseInt) and a fallback default are applied so server startup behavior remains unchanged when PORT is not set.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@examples/with-mongodb/README.md`:
- Line 30: The Contributor Covenant badge links to a relative CODE_OF_CONDUCT.md
that breaks for this example; update the badge link target (the Markdown
starting with [![Contributor Covenant] and pointing to CODE_OF_CONDUCT.md) to
reference the repo root (e.g. /CODE_OF_CONDUCT.md) so it resolves correctly from
the example folder.
In `@examples/with-mongodb/tsconfig.json`:
- Around line 6-8: The tsconfig sets "module": "ESNext" but uses
"moduleResolution": "node", which can ignore package.json "exports" and break
ESM conditional exports; update the tsconfig.json by changing the
moduleResolution field from "node" to "NodeNext" so TypeScript resolves the
correct ESM export variants when compiling with "module": "ESNext".
---
Nitpick comments:
In `@examples/with-mongodb/src/index.ts`:
- Line 41: Change the hardcoded port in the honoServer configuration to read
from an environment variable so the server is configurable; update the server:
honoServer({ port: 3141 }) call to use process.env.PORT (or a similarly named
env var) parsed as a number with a sensible default (e.g., 3141) so honoServer
receives a numeric port; ensure any type conversion (Number or parseInt) and a
fallback default are applied so server startup behavior remains unchanged when
PORT is not set.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 690ab3f2-b811-4988-9a1f-a0e179a14f7d
📒 Files selected for processing (6)
examples/with-mongodb/.env.exampleexamples/with-mongodb/.gitignoreexamples/with-mongodb/README.mdexamples/with-mongodb/package.jsonexamples/with-mongodb/src/index.tsexamples/with-mongodb/tsconfig.json
✅ Files skipped from review due to trivial changes (2)
- examples/with-mongodb/.gitignore
- examples/with-mongodb/.env.example
| <div align="center"> | ||
|
|
||
| [](https://www.npmjs.com/package/@voltagent/core) | ||
| [](CODE_OF_CONDUCT.md) |
There was a problem hiding this comment.
Fix broken Contributor Covenant link path.
Line 30 links to CODE_OF_CONDUCT.md relative to this example folder, which likely breaks. Point it to the repo-root file.
Suggested fix
-[](CODE_OF_CONDUCT.md)
+[](../../CODE_OF_CONDUCT.md)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| [](CODE_OF_CONDUCT.md) | |
| [](../../CODE_OF_CONDUCT.md) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/with-mongodb/README.md` at line 30, The Contributor Covenant badge
links to a relative CODE_OF_CONDUCT.md that breaks for this example; update the
badge link target (the Markdown starting with [![Contributor Covenant] and
pointing to CODE_OF_CONDUCT.md) to reference the repo root (e.g.
/CODE_OF_CONDUCT.md) so it resolves correctly from the example folder.
| "module": "ESNext", | ||
| "target": "ES2022", | ||
| "moduleResolution": "node" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n tsconfig.jsonRepository: VoltAgent/voltagent
Length of output: 902
🏁 Script executed:
cat -n packages/mongodb/package.json | head -50Repository: VoltAgent/voltagent
Length of output: 1780
🏁 Script executed:
cat -n examples/with-mongodb/tsconfig.jsonRepository: VoltAgent/voltagent
Length of output: 507
Align ESM resolution with NodeNext to properly resolve conditional package exports.
Line 8 uses "moduleResolution": "node", which ignores package.json exports field and can cause resolution mismatches with packages like @voltagent/mongodb that use conditional exports. Since the example uses "module": "ESNext" (ESM), set moduleResolution to "NodeNext" to ensure correct export variant resolution.
Suggested fix
"compilerOptions": {
"outDir": "./dist",
"rootDir": "./src",
"module": "ESNext",
"target": "ES2022",
- "moduleResolution": "node"
+ "moduleResolution": "NodeNext"
},🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/with-mongodb/tsconfig.json` around lines 6 - 8, The tsconfig sets
"module": "ESNext" but uses "moduleResolution": "node", which can ignore
package.json "exports" and break ESM conditional exports; update the
tsconfig.json by changing the moduleResolution field from "node" to "NodeNext"
so TypeScript resolves the correct ESM export variants when compiling with
"module": "ESNext".
PR Checklist
Please check if your PR fulfills the following requirements:
Bugs / Features
What is the current behavior?
There is currently no built-in support for MongoDB as a persistent storage backend for agent memory. Users are limited to in-memory or Postgres adapters.
What is the new behavior?
This PR implements the
@voltagent/mongodbpackage with a full MongoDBMemoryAdapter.StorageAdapterinterface.Datevsstringfields to match Core interfaces.fixes #505
Notes for reviewers
npm run test:integration).userId,conversationId, andcreatedAtto ensure query performance.Summary by cubic
Adds a new @voltagent/mongodb package with a complete MongoDBMemoryAdapter and a minimal example app to show usage. Addresses #505 by adding built-in MongoDB support with indexes and integration tests.
New Features
Bug Fixes
Written for commit ee9c1ff. Summary will update on new commits.
Summary by CodeRabbit
New Features
Documentation
Tests
Chores