Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis PR introduces a comprehensive Storage Abstraction layer with multiple database adapters (Memory, PostgreSQL, MongoDB, Redis, LibSQL, S3, SQLite, File), migrates documentation from Docusaurus to MkDocs, updates conversation memory to be async and storage-aware, adds new CLI storage commands, and integrates storage capabilities into the MCP server and core SDK. Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application
participant Mgr as ConversationMemoryManager
participant Pool as ConnectionPool
participant Storage as StorageProvider
participant DB as Database Backend
App->>Mgr: storeConversationTurn()
Mgr->>Pool: acquire()
Pool->>Storage: createThread/getMessage
Storage->>DB: SQL/API Call
DB-->>Storage: Result
Storage-->>Pool: Connection
Pool-->>Mgr: Connection
Mgr->>Mgr: Format & Persist
Mgr->>Storage: createMessage()
Storage->>DB: Write Message
DB-->>Storage: Confirmation
Storage-->>Mgr: StorageMessage
Mgr->>Pool: release()
Pool-->>App: Success
sequenceDiagram
participant CLI as CLI User
participant Cmd as storage command
participant Factory as StorageFactory
participant Adapter as StorageAdapter
participant Health as HealthMonitor
CLI->>Cmd: storage status
Cmd->>Factory: createStorageFromEnv()
Factory->>Adapter: instantiate(type)
Adapter->>Adapter: init()
Adapter-->>Factory: initialized
Factory-->>Cmd: StorageProvider
Cmd->>Health: checkStorageHealth()
Health->>Adapter: healthCheck()
Adapter-->>Health: latency + status
Health-->>Cmd: HealthCheckResult
Cmd->>CLI: formatted output
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
✅ Single Commit Policy - COMPLIANTStatus: Policy requirements met • 1 commit • Valid format • Ready for merge 📊 View validation details📝 Commit Details
✅ Validation Results
🤖 Automated validation by NeuroLink Single Commit Enforcement |
4b58ea4 to
ada092a
Compare
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR introduces project metadata and automation around a proposed “storage abstraction layer” feature, including a status document, Serena project configuration, and a GitHub Pages docs deployment workflow.
Changes:
- Added
FEATURE-STATUS.mdto track storage abstraction completion status. - Added Serena project configuration and cache ignore rules under
.serena/. - Added a GitHub Actions workflow to build and deploy MkDocs documentation to GitHub Pages.
Reviewed changes
Copilot reviewed 4 out of 750 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| FEATURE-STATUS.md | Adds a human-readable status tracker for the storage abstraction work. |
| .serena/project.yml | Adds Serena configuration for TypeScript language server and project settings. |
| .serena/.gitignore | Ignores Serena cache directory. |
| .github/workflows/docs.yml | Adds CI workflow to lint docs markdown and deploy MkDocs site to GitHub Pages. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -0,0 +1,20 @@ | |||
| # Storage Abstraction - Status | |||
There was a problem hiding this comment.
The PR description/title claim a large new storage abstraction layer (adapters, middleware, transactions, CLI commands, tests), but the provided diff only adds a status markdown file plus Serena config and a docs workflow. Either the PR description/title should be updated to match these changes, or the missing implementation diffs should be included so reviewers can validate the actual storage feature.
| if: github.event_name != 'pull_request' | ||
| uses: actions/upload-pages-artifact@v3 | ||
| with: | ||
| path: _site |
There was a problem hiding this comment.
The workflow uploads _site, but mkdocs build defaults to outputting into site/ unless site_dir: _site is set in mkdocs.yml. If mkdocs.yml does not override site_dir, this will upload an empty/non-existent directory and deploy blank docs. Align the upload path with the actual MkDocs output directory (either change to site or ensure mkdocs.yml config sets site_dir: _site).
| path: _site | |
| path: site |
| - name: 📝 Markdown Linting | ||
| run: | | ||
| echo "📝 Running markdownlint on documentation files..." | ||
| npx markdownlint-cli2 "docs/**/*.md" --config .markdownlint.json || echo "⚠️ Markdownlint found formatting issues - consider running 'npx markdownlint-cli2 --fix \"docs/**/*.md\"' locally" |
There was a problem hiding this comment.
The || echo ... makes markdownlint failures non-blocking, so the job will still succeed even when lint finds issues. If the goal is to enforce documentation formatting in CI, remove the || echo ... (or explicitly use continue-on-error: true on the step if you want non-blocking behavior while being intentional/visible in the workflow).
| npx markdownlint-cli2 "docs/**/*.md" --config .markdownlint.json || echo "⚠️ Markdownlint found formatting issues - consider running 'npx markdownlint-cli2 --fix \"docs/**/*.md\"' locally" | |
| npx markdownlint-cli2 "docs/**/*.md" --config .markdownlint.json |
| - name: 📝 Markdown Linting | ||
| run: | | ||
| echo "📝 Running markdownlint on documentation files..." | ||
| npx markdownlint-cli2 "docs/**/*.md" --config .markdownlint.json || echo "⚠️ Markdownlint found formatting issues - consider running 'npx markdownlint-cli2 --fix \"docs/**/*.md\"' locally" |
There was a problem hiding this comment.
Running npx markdownlint-cli2 without pinning a version reduces supply-chain safety and reproducibility (the resolved version can change over time). Prefer pinning the tool version (e.g., npx markdownlint-cli2@<known-version> ...) or installing it via the repo’s package manager/lockfile and running it from there.
| npx markdownlint-cli2 "docs/**/*.md" --config .markdownlint.json || echo "⚠️ Markdownlint found formatting issues - consider running 'npx markdownlint-cli2 --fix \"docs/**/*.md\"' locally" | |
| npx markdownlint-cli2@0.14.0 "docs/**/*.md" --config .markdownlint.json || echo "⚠️ Markdownlint found formatting issues - consider running 'npx markdownlint-cli2@0.14.0 --fix \"docs/**/*.md\"' locally" |
🤖 AI Review & Build Compliance ✅Status: AI analysis complete • Build rules validated • Ready for review 📊 View detailed analysis results🛡️ Analysis Complete
📋 Ready for Merge When
🤖 AI analysis complete - check individual code comments for specific feedback |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
ada092a to
e87feda
Compare
🤖 AI Review & Build Compliance ✅Status: AI analysis complete • Build rules validated • Ready for review 📊 View detailed analysis results🛡️ Analysis Complete
📋 Ready for Merge When
🤖 AI analysis complete - check individual code comments for specific feedback |
e87feda to
1dde981
Compare
Documentation Validation Results
🚧 Please fix the failing checks before merging. Commit: |
🤖 AI Review & Build Compliance ✅Status: AI analysis complete • Build rules validated • Ready for review 📊 View detailed analysis results🛡️ Analysis Complete
📋 Ready for Merge When
🤖 AI analysis complete - check individual code comments for specific feedback |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/lib/core/conversationMemoryInitializer.ts (1)
101-105:⚠️ Potential issue | 🟠 MajorSerialize lazy conversation-memory initialization before making both branches fully async.
initializeConversationMemory()now awaits manager construction on every backend, butsrc/lib/neurolink.tsstill guards lazy init with onlyconversationMemoryNeedsInitand flips that flag after the await. Two concurrentgenerate()calls can therefore both enter this function and create separate memory managers / duplicate storage connections. Please memoize the in-flight initialization promise in the caller before landing this change.Also applies to: 130-133
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/core/conversationMemoryInitializer.ts` around lines 101 - 105, The lazy init race happens because initializeConversationMemory() is awaited without memoizing an in-flight promise, so concurrent generate() calls can each construct a manager; update the caller (the code that checks conversationMemoryNeedsInit and flips it) to store a single in-flight Promise before awaiting initializeConversationMemory() (e.g., a module-level or class-level pendingInitialization promise), reuse that promise for concurrent callers, and clear it once resolved/failed; specifically ensure the guarding logic around conversationMemoryNeedsInit in neurolink.ts (and the analogous block at the other occurrence around lines 130-133) sets and awaits the memoized promise instead of directly awaiting initializeConversationMemory() to prevent duplicate createConversationMemoryManager() calls.package.json (1)
270-306:⚠️ Potential issue | 🟠 MajorUnguarded module imports in storage adapters need error handling before optional peers ship.
With
pg,ioredis,mongodb, and@libsql/clientmarked optional, their dynamic imports in adapterinit()methods will throw rawMODULE_NOT_FOUNDerrors instead of typed storage failures. The following adapters lack try-catch protection around their driver imports:
postgresAdapter.ts:94—await import("pg")redisAdapter.ts:113—await import("ioredis")mongodbAdapter.ts:168—await import("mongodb")libsqlAdapter.ts:89—await import("@libsql/client")S3 has additional unguarded imports in post-init methods (
putObject,deleteObject,loadIndexes). While some call sites wrapinit()in try-catch, they log raw error strings rather than converting to typed storage errors. Wrap all driver imports in try-catch and throw a consistent storage error type.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` around lines 270 - 306, Wrap all dynamic driver imports in the adapters in try-catch and rethrow a consistent storage error: update postgresAdapter.init (the await import("pg")), redisAdapter.init (await import("ioredis")), mongodbAdapter.init (await import("mongodb")), libsqlAdapter.init (await import("@libsql/client")), and the S3 post-init methods (putObject, deleteObject, loadIndexes) so that the import is surrounded by try { const mod = await import(...) } catch (err) { throw new StorageError(`failed to load driver for <adapter>: ${String(err)}`, { cause: err }) } (or use your project’s canonical StorageError type/name), ensuring all raw MODULE_NOT_FOUND errors are converted to the typed storage error before bubbling up.
🟠 Major comments (26)
src/lib/storage/adapters/FileStorageAdapter.ts-321-323 (1)
321-323:⚠️ Potential issue | 🟠 MajorOther method signatures also differ from the base class.
Similar to
createThread, other methods use inlineOmit<...>types instead of the dedicated input types defined in the base class:
createMessage(Line 321-323): UsesOmit<StorageMessage, ...>instead ofCreateMessageInputupdateThread(Line 244-247): UsesPartial<Omit<...>>instead ofUpdateThreadInputupdateMessage(Line 365-368): UsesPartial<Omit<...>>instead ofUpdateMessageInputThese should be aligned with the abstract method signatures for interface consistency.
Also applies to: 244-247, 365-368
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/storage/adapters/FileStorageAdapter.ts` around lines 321 - 323, The method signatures in FileStorageAdapter use inline Omit/Partial types instead of the abstract/base input types, causing an interface mismatch; update the signatures for createMessage, updateThread, and updateMessage to use the dedicated types from the base class (e.g., CreateMessageInput for createMessage, UpdateThreadInput for updateThread, UpdateMessageInput for updateMessage), import those types if needed, and adjust any internal references/parameter destructuring to match the renamed parameter types so the class correctly implements the abstract interface (check method names createMessage, updateThread, updateMessage).src/lib/storage/adapters/S3StorageAdapter.ts-186-188 (1)
186-188:⚠️ Potential issue | 🟠 MajorMethod signatures differ from base class (same as FileStorageAdapter).
This adapter has the same signature inconsistencies as
FileStorageAdapter:
createThread(Line 186-188): UsesOmit<StorageThread, ...>instead ofCreateThreadInputcreateMessage(Line 284-286): UsesOmit<StorageMessage, ...>instead ofCreateMessageInputupdateThread(Line 210-213): UsesPartial<Omit<...>>instead ofUpdateThreadInputupdateMessage(Line 325-328): UsesPartial<Omit<...>>instead ofUpdateMessageInputupdateWorkflowRunStatus(Line 476-481): UsesStorageWorkflowRun["error"]instead ofStorageWorkflowErrorThese should match the base class signatures for adapter interchangeability.
Also applies to: 284-286, 210-213, 325-328, 476-481
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/storage/adapters/S3StorageAdapter.ts` around lines 186 - 188, The method signatures in S3StorageAdapter must match the base class: change createThread(thread: Omit<StorageThread, "id" | "createdAt" | "updatedAt">) to createThread(thread: CreateThreadInput), createMessage(...) to CreateMessageInput, updateThread(...) to UpdateThreadInput, updateMessage(...) to UpdateMessageInput, and updateWorkflowRunStatus(error: StorageWorkflowRun["error"]) to accept StorageWorkflowError; update the imports to bring in CreateThreadInput, CreateMessageInput, UpdateThreadInput, UpdateMessageInput, and StorageWorkflowError from the shared types, keep the existing implementation and return types intact, and adapt any internal property access if needed to account for the input shapes (e.g., mapping fields from the Create*/Update* inputs to StorageThread/StorageMessage).src/lib/storage/adapters/redisAdapter.ts-691-697 (1)
691-697:⚠️ Potential issue | 🟠 MajorSame
KEYScommand issue in other methods.The
KEYScommand is also used in:
listWorkflowRuns(lines 691-697)getStats(lines 1015-1021)clearAll(lines 1047-1053)All should be refactored to use
SCANor index sets.Also applies to: 1015-1021, 1047-1053
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/storage/adapters/redisAdapter.ts` around lines 691 - 697, Replace use of Redis KEYS in listWorkflowRuns, getStats, and clearAll with a cursor-based SCAN iteration or an indexed Redis set to avoid blocking the server: for listWorkflowRuns (uses this.client!.keys with pattern this.key(KEY_PREFIXES.workflowRun, "*")) implement an async SCAN loop (using this.client.scan/call or equivalent) to collect matching keys and then map them with k.replace(this.key(KEY_PREFIXES.workflowRun, ""), "") OR maintain a dedicated Redis set that stores workflow run IDs on creation/deletion and read from that set instead; apply the same SCAN-or-index-set refactor for getStats and clearAll where KEYS is used to enumerate keys so you no longer call this.client.keys(...) directly.src/lib/storage/adapters/FileStorageAdapter.ts-217-219 (1)
217-219:⚠️ Potential issue | 🟠 MajorInconsistent
createThreadparameter type across adapters.This adapter uses
Omit<StorageThread, "id" | "createdAt" | "updatedAt">while other adapters (e.g.,PostgresAdapter,MemoryAdapter,MongoDBAdapter) useCreateThreadInput. This inconsistency can cause issues when swapping adapters at runtime.Per the relevant snippet from
src/lib/storage/adapters/postgresAdapter.ts:278-296andsrc/lib/storage/storageProvider.ts,CreateThreadInputis the expected parameter type.Proposed fix
+import type { + ... + CreateThreadInput, + ... +} from "../../types/index.js"; -async createThread( - thread: Omit<StorageThread, "id" | "createdAt" | "updatedAt">, -): Promise<StorageThread> { +async createThread(input: CreateThreadInput): Promise<StorageThread> { this.ensureInitialized(); const now = new Date(); const newThread: StorageThread = { - ...thread, + ...input, id: randomUUID(), createdAt: now, updatedAt: now, - status: thread.status || "active", + status: input.status || "active", };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/storage/adapters/FileStorageAdapter.ts` around lines 217 - 219, The createThread signature in FileStorageAdapter.ts is using Omit<StorageThread, "id" | "createdAt" | "updatedAt"> which is inconsistent with other adapters; change the parameter type to CreateThreadInput, update the method signature async createThread(thread: CreateThreadInput): Promise<StorageThread>, add/import CreateThreadInput at top of the file, and adjust any internal references within the FileStorageAdapter.createThread implementation if they assume different field names so they match the CreateThreadInput shape used by PostgresAdapter/MemoryAdapter/MongoDBAdapter.src/lib/storage/adapters/redisAdapter.ts-614-654 (1)
614-654:⚠️ Potential issue | 🟠 Major
saveWorkflowRundoesn't update status index on upsert.When upserting an existing workflow run with a different status, the old status index entry is not removed. This can cause stale entries in the status index, leading to incorrect
listWorkflowRunsresults when filtering by status.Compare with
updateWorkflowRunStatus(lines 736-745) which correctly handles this case.Proposed fix
async saveWorkflowRun( input: SaveWorkflowRunInput, ): Promise<StorageWorkflowRun> { this.ensureInitialized(); const now = this.now(); const id = input.id || this.generateId(); // Check if exists const existing = input.id ? await this.getWorkflowRun(input.id) : null; + // Update status index if status changed + if (existing && existing.status !== input.status) { + await this.client!.srem( + this.indexKey("workflow", "status", existing.status), + id, + ); + } const run: StorageWorkflowRun = { // ... };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/storage/adapters/redisAdapter.ts` around lines 614 - 654, saveWorkflowRun currently always sadds the new status index but never removes the old one on upsert; modify saveWorkflowRun so that after loading existing (existing = await getWorkflowRun(...)) you check if existing && existing.status !== input.status and call this.client!.srem(this.indexKey("workflow","status", existing.status), id) before adding the new status index; similarly, if workflowId can change, remove the id from the old workflow index with srem(this.indexKey("workflow","workflow", existing.workflowId), id) when existing && existing.workflowId !== input.workflowId; keep the existing sadd calls to add the updated indexes.src/lib/storage/adapters/S3StorageAdapter.ts-730-747 (1)
730-747:⚠️ Potential issue | 🟠 Major
loadIndexesdoesn't handle pagination.The
ListObjectsV2Commandreturns up to 1000 objects by default. For buckets with more objects, subsequent pages are not fetched, causing incomplete indexes.Use
NextContinuationTokento paginate through all objects, similar to the pattern inclearAll.Proposed fix
private async loadIndexes(): Promise<void> { this.log("debug", "Loading indexes"); const { ListObjectsV2Command, GetObjectCommand } = await import("@aws-sdk/client-s3"); - // Load all objects and build indexes - const listResult = (await this.client!.send( - new ListObjectsV2Command({ - Bucket: this.config.bucket, - Prefix: this.keyPrefix, - }), - )) as { Contents?: Array<{ Key: string }> }; - - if (!listResult.Contents) { - return; - } + let continuationToken: string | undefined; + const allContents: Array<{ Key: string }> = []; - for (const obj of listResult.Contents) { + do { + const listResult = (await this.client!.send( + new ListObjectsV2Command({ + Bucket: this.config.bucket, + Prefix: this.keyPrefix, + ContinuationToken: continuationToken, + }), + )) as { Contents?: Array<{ Key: string }>; NextContinuationToken?: string }; + + if (listResult.Contents) { + allContents.push(...listResult.Contents); + } + continuationToken = listResult.NextContinuationToken; + } while (continuationToken); + + for (const obj of allContents) { // ... existing processing logic } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/storage/adapters/S3StorageAdapter.ts` around lines 730 - 747, loadIndexes currently calls ListObjectsV2Command once and misses additional pages; modify loadIndexes to paginate like clearAll by looping and re-sending ListObjectsV2Command with ContinuationToken/NextContinuationToken until no token remains, accumulating all Contents before building indexes (use the same this.client!.send pattern and preserve Prefix/keyPrefix handling), and ensure you handle an empty Contents on intermediate pages and exit the loop when NextContinuationToken is undefined.src/lib/storage/adapters/redisAdapter.ts-341-347 (1)
341-347:⚠️ Potential issue | 🟠 Major
KEYScommand is O(N) and blocks Redis.Using
KEYS *scans the entire keyspace and blocks the Redis server during execution. This is problematic in production with large datasets.Consider using
SCANfor iterative, non-blocking enumeration, or maintaining a dedicated set index for all thread IDs.Example fix using SCAN
} else { // Get all thread keys - const pattern = this.key(KEY_PREFIXES.thread, "*"); - const keys = await this.client!.keys(pattern); - threadIds = keys.map((k) => - k.replace(this.key(KEY_PREFIXES.thread, ""), ""), - ); + const pattern = this.key(KEY_PREFIXES.thread, "*"); + threadIds = []; + let cursor = "0"; + do { + const [nextCursor, keys] = await this.client!.scan( + cursor, + "MATCH", + pattern, + "COUNT", + 100, + ); + cursor = nextCursor; + threadIds.push( + ...keys.map((k) => k.replace(this.key(KEY_PREFIXES.thread, ""), "")), + ); + } while (cursor !== "0"); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/storage/adapters/redisAdapter.ts` around lines 341 - 347, Replace the blocking KEYS call with a non-blocking SCAN loop: use the Redis SCAN command (via this.client.scan or scanStream) with MATCH set to this.key(KEY_PREFIXES.thread, "*") and an appropriate COUNT, iterate until cursor returns 0, collect matched keys into an array, then map them to threadIds by stripping this.key(KEY_PREFIXES.thread, "") (same mapping as current keys.map). Update the code in the method that currently uses this.client!.keys(...) to perform this scan-based accumulation; alternatively consider adding/using a dedicated set index for thread IDs if available (e.g., a threads set) but prefer SCAN for immediate fix.src/lib/mcp/auth/tokenStorage.ts-218-230 (1)
218-230:⚠️ Potential issue | 🟠 MajorDon’t swallow storage-init failures for token persistence.
Returning
nullhere makessaveTokens(),deleteTokens(), andclearAll()silently no-op whenever storage setup fails. In the auth flow that means a login can succeed but the tokens are never persisted, so the next process starts unauthenticated with no clue why. Please at least surface write-path failures or fall back explicitly to a non-persistent storage implementation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/mcp/auth/tokenStorage.ts` around lines 218 - 230, The getKV() method currently swallows all storage-init errors and returns null which causes saveTokens(), deleteTokens(), and clearAll() to silently no-op; update getKV() to either rethrow the caught error (so calling code sees storage failures) or instantiate a clearly labeled non-persistent in-memory KeyValueStore fallback instead of returning null. Specifically, inside getKV() around the createStorageFromEnv()/KeyValueStore import, catch the error, and (a) log and throw a new Error with context referencing createStorageFromEnv and KeyValueStore so callers like saveTokens()/deleteTokens()/clearAll() fail loudly, or (b) assign this.kv to an in-memory KeyValueStore implementation (with the same API and a flag/note that it's non-persistent) and return it so persistence behavior is explicit. Ensure references to this.kv and this.namespace are preserved.docs/mastra-features-implementation/18-storage-abstraction.md-122-129 (1)
122-129:⚠️ Potential issue | 🟠 MajorThese code samples import from a package/export that this repo does not publish.
package.jsonexposes@juspay/neurolink, not@neurolink/storage, so readers copying this snippet will fail before they even reach the storage APIs. This bad import path repeats throughout the page and should be replaced with the actual public entry point.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/mastra-features-implementation/18-storage-abstraction.md` around lines 122 - 129, The examples import MemoryStorageAdapter from a non-published package ("@neurolink/storage"); update all occurrences to import the same symbols (e.g., MemoryStorageAdapter) from the repo's public entry point "@juspay/neurolink" instead, replacing the incorrect import path across this file (and similar pages) so the snippet uses the published export and will work for readers copying the sample.src/cli/factories/commandFactory.ts-3990-3996 (1)
3990-3996:⚠️ Potential issue | 🟠 MajorRemove or fix the unused
createStorageCommands()method.
package.jsonmarks the CLI as ESM ("type": "module"), sorequire("../commands/storage.js")will throw at runtime if this method is ever called. More importantly, this method is dead code—src/cli/parser.tsimports and registersstorageCommanddirectly without using this factory method. Therequire()pattern also contradicts other factory methods in this file, which either return inline objects or delegate to other factory classes. Either remove this unused method or refactor it to use dynamicimport()syntax if it's meant to serve as an alternative entry point.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/factories/commandFactory.ts` around lines 3990 - 3996, The createStorageCommands() method contains a CommonJS require and is dead code; either remove the method entirely or refactor it to use ESM dynamic import and an async signature so it won't throw at runtime: replace the require("../commands/storage.js") with await import("../commands/storage.js") and return the imported storageCommand, or delete createStorageCommands() if nothing calls it (confirm callers such as any references to createStorageCommands and the existing direct import of storageCommand in the CLI parser), and ensure any exports or callers are updated to match the new async function signature if you choose the import() approach.src/lib/mcp/servers/storage/storageServer.ts-62-65 (1)
62-65:⚠️ Potential issue | 🟠 MajorStorage instance created on every tool invocation — use singleton pattern from existing
getDefaultStorage()instead.Each tool execution calls
createStorageFromEnv()andawait storage.init(), creating a new storage instance per request. While the default storage type ismemory(which doesn't require cleanup), this pattern becomes a critical resource leak whenSTORAGE_TYPEis configured to use persistent backends like PostgreSQL (which maintains connection pools).Instead of caching locally, use the existing
getDefaultStorage()function fromsrc/lib/storage/storageFactory.ts, which implements proper singleton management:🔧 Apply singleton pattern
+const { getDefaultStorage } = + await import("../../../storage/index.js"); - const { createStorageFromEnv } = - await import("../../../storage/index.js"); - const storage = await createStorageFromEnv(); - await storage.init(); + const storage = await getDefaultStorage();Replace all four instances at lines 62–65, 152–155, 235–238, and 322–325.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/mcp/servers/storage/storageServer.ts` around lines 62 - 65, The code creates a new storage instance on every tool invocation by calling createStorageFromEnv() and await storage.init(), causing resource leaks for persistent backends; replace those calls with the singleton accessor getDefaultStorage() (from storageFactory) and use the returned instance without re-initializing it (or call init only if getDefaultStorage() requires it), i.e., stop invoking createStorageFromEnv() and storage.init() in functions referenced in this file and instead import and call getDefaultStorage() to obtain the shared storage instance managed as a singleton.src/lib/neurolink.ts-1078-1081 (1)
1078-1081:⚠️ Potential issue | 🟠 MajorDon't implicitly take ownership of caller-supplied storage.
Line 1079 stores a provider passed in by the caller, but the new cleanup paths later always call
close()on it. That makes shared providers unsafe across multipleNeuroLinkinstances and also makesshutdown()/dispose()non-idempotent because the same backend can be closed twice. Track ownership before auto-closing, and clearthis.storageafter a successful close sostorageProviderdoes not keep returning a closed backend.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/neurolink.ts` around lines 1078 - 1081, The code currently assigns caller-supplied storage to this.storage (from config.storage) but always calls close() in shutdown()/dispose(), which can close shared providers and break idempotency; change the constructor/wiring to record an ownership flag (e.g., this.ownsStorage = true/false) when assigning storage from config.storage or when creating a new backend, use that flag in shutdown()/dispose() to only call close() if ownsStorage is true, and after a successful close set this.storage = undefined (or null) and this.ownsStorage = false so subsequent shutdown/dispose/storageProvider calls are safe and idempotent; update any places referencing storageProvider/this.storage to handle the cleared value.src/lib/neurolink.ts-2385-2417 (1)
2385-2417:⚠️ Potential issue | 🟠 MajorSkip registering storage tools until a backend is configured and ready.
Line 2417 registers the storage server unconditionally, even though this instance may not have any
this.storageat all. That means storage lookup tools can be advertised via MCP on SDK instances that have no usable backend, so the failure only shows up when the model/tool call happens. Please guard this onthis.storage, and if NeuroLink is managing the provider lifecycle, initialize it before registration as well.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/neurolink.ts` around lines 2385 - 2417, registerStorageServerInternal currently registers storageServer unconditionally which advertises storage lookup tools even when no backend exists; update registerStorageServerInternal to first check this.storage and only proceed if a backend is present (e.g., if (!this.storage) return), and if NeuroLink is responsible for provider lifecycle ensure you initialize the provider (call the existing init/ensureProvider method or create one) and wait for it to be ready before constructing storageServerInfo and calling this.toolRegistry.registerServer; reference registerStorageServerInternal, storageServer, and this.storage when making the guard/initialization changes.src/lib/core/storageConversationMemoryManager.ts-503-511 (1)
503-511:⚠️ Potential issue | 🟠 MajorBug: JSON-serialized fields not being deserialized.
In
setSessionMessages(lines 417-431), complex fields likeargs,result, andeventsare JSON-stringified before storage:meta.args = JSON.stringify(msg.args);However, in
convertStorageMessages, the code checks for object types without parsing the JSON strings:if (meta.args && typeof meta.args === "object") { msg.args = meta.args as Record<string, unknown>; }This means
args,result,events, andchatMetadatawill never be restored since they're stored as strings but expected as objects.🐛 Proposed fix to parse JSON-serialized metadata
- if (meta.args && typeof meta.args === "object") { - msg.args = meta.args as Record<string, unknown>; + if (meta.args) { + msg.args = typeof meta.args === "string" + ? JSON.parse(meta.args) + : meta.args as Record<string, unknown>; } - if (meta.result) { - msg.result = meta.result as ChatMessage["result"]; + if (meta.result) { + msg.result = typeof meta.result === "string" + ? JSON.parse(meta.result) + : meta.result as ChatMessage["result"]; } - if (Array.isArray(meta.events)) { - msg.events = meta.events as ChatMessage["events"]; + if (meta.events) { + msg.events = typeof meta.events === "string" + ? JSON.parse(meta.events) + : meta.events as ChatMessage["events"]; } + + // Restore chatMetadata if present + if (meta.chatMetadata) { + const restored = typeof meta.chatMetadata === "string" + ? JSON.parse(meta.chatMetadata) + : meta.chatMetadata; + msg.metadata = { ...msg.metadata, ...restored }; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/core/storageConversationMemoryManager.ts` around lines 503 - 511, In convertStorageMessages, deserialize JSON-stringified metadata fields that were stored by setSessionMessages: detect when meta.args, meta.result, meta.events, or meta.chatMetadata are strings and JSON.parse them (wrap each parse in try/catch and fall back to the original value on failure) before assigning to msg.args, msg.result, msg.events, and msg.chatMetadata; update the logic around msg assignment in convertStorageMessages to handle both already-parsed objects and JSON strings so stored values are correctly restored for ChatMessage consumers.src/lib/storage/StorageRegistry.ts-302-304 (1)
302-304:⚠️ Potential issue | 🟠 MajorThese public lookups can return false negatives before the registry is initialized.
hasBackend()andcheckHealth()readentrieswithoutensureInitialized(), andunregisterBackend()also skips alias resolution. A first call like"pg"or"redis"can report “not registered” even though the backend is supported.Also applies to: 414-417, 431-436
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/storage/StorageRegistry.ts` around lines 302 - 304, The methods hasBackend, checkHealth, and unregisterBackend read StorageRegistry.entries directly and can return false negatives before the registry is initialized and when given an alias; call StorageRegistry.getInstance().ensureInitialized() at the start of each of these public lookup methods and use the registry's alias resolution (e.g., StorageRegistry.resolveAlias or the instance method that maps aliases to StorageBackendType) before accessing registry.entries so lookups like "pg" or "redis" correctly resolve and return the true registration/health status rather than a premature miss.src/lib/storage/adapters/postgresAdapter.ts-126-137 (1)
126-137:⚠️ Potential issue | 🟠 MajorTear down the pool when initialization fails.
If schema creation or migrations throw after
new Pool(...), the adapter leaves that pool open and the next retry allocates another one. Wrap the body after Line 126 in atry/catchandawait this.pool.end()before rethrowing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/storage/adapters/postgresAdapter.ts` around lines 126 - 137, After instantiating the connection pool (this.pool = new Pool(poolConfig)), wrap the subsequent initialization steps — the CREATE SCHEMA query, the conditional call to this.runMigrations(), setting this.initialized, and this.log — in a try/catch; if any step throws, call await this.pool.end() to tear down the pool, set this.pool to undefined (or null) if you keep that invariant, then rethrow the error so callers can retry without leaking connections. Ensure the catch targets errors from the schema creation and runMigrations() calls and performs the pool cleanup before rethrowing.src/lib/storage/adapters/SQLiteStorageAdapter.ts-401-429 (1)
401-429:⚠️ Potential issue | 🟠 Major
dateRangefiltering is missing here.
MessageQueryOptionsincludes date bounds, butlistMessages()only filters by thread, role, and type. The same query will return different results depending on which adapter is active.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/storage/adapters/SQLiteStorageAdapter.ts` around lines 401 - 429, The listMessages method is not applying MessageQueryOptions.dateRange, so listMessages returns messages outside the requested bounds; update SQLiteStorageAdapter.listMessages to honor the dateRange by adding WHERE clauses for the start/end bounds (e.g., "created_at >= ?" and/or "created_at <= ?") when options.dateRange.start and/or options.dateRange.end are present, push the corresponding parameters onto params in the correct order, and ensure the same whereClause/params are used for both the COUNT and SELECT queries so pagination and totals reflect the filtered date range.src/lib/storage/StorageRegistry.ts-190-197 (1)
190-197:⚠️ Potential issue | 🟠 MajorRegister
libsqlas its own backend instead of aliasing it tosqlite.The public storage surface exposes both backends separately, but this registry collapses
"libsql"into the"sqlite"entry. That makesgetAdapter("libsql")resolve to the wrong adapter and keeps"libsql"out ofgetAvailableBackends().🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/storage/StorageRegistry.ts` around lines 190 - 197, The registry currently folds "libsql" into the "sqlite" entry (see the entry with type: "sqlite" and aliases including "libsql"), so getAdapter("libsql") and getAvailableBackends() are incorrect; fix this by removing "libsql" from the aliases of the sqlite entry and add a separate registry entry where type: "libsql" (and appropriate metadata/name/features/persistent/distributed) is declared as its own backend; ensure the new entry uses the same canonical name used by getAdapter and that getAvailableBackends() will now include "libsql".src/lib/storage/adapters/SQLiteStorageAdapter.ts-657-665 (1)
657-665:⚠️ Potential issue | 🟠 MajorTTL is only enforced on point reads.
Lines 657-665 expire records inside
getRecord(), butlistRecords()andgetStats()read the table directly. Expired keys keep showing up in lists and counts until somebody fetches them individually.Also applies to: 678-700, 719-744
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/storage/adapters/SQLiteStorageAdapter.ts` around lines 657 - 665, The TTL check is only applied in getRecord(), so expired rows still appear in listRecords() and getStats(); update listRecords(), getStats(), and the other read methods (the ones around lines 678-700 and 719-744) to either (a) filter out expired records in their SQL/ORM queries by adding a condition comparing updated_at + ttl (or ttl IS NULL) against now, or (b) run a cleanup step that deletes expired rows before computing lists/counts; locate the methods named getRecord(), listRecords(), and getStats() in SQLiteStorageAdapter and modify their query logic to exclude records whose ttl has elapsed (or invoke a shared helper that checks/deletes expired records) so lists and stats never include expired entries.src/lib/storage/StorageRegistry.ts-348-357 (1)
348-357:⚠️ Potential issue | 🟠 MajorDeduplicate concurrent instance creation.
Two callers can observe
entry.instanceas empty and both runentry.factory(). The second completion wins, but the first instance is still created and leaked, which breaks the registry's singleton guarantee.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/storage/StorageRegistry.ts` around lines 348 - 357, Race condition: two callers can both see empty entry.instance and call entry.factory(), leaking one instance; fix by deduplicating concurrent creation using a creation promise. Implement a creation guard on the entry (e.g., entry.creatingPromise): if entry.instance return it; else if entry.creatingPromise await and return its resolved instance; otherwise set entry.creatingPromise = entry.factory() (do not await inline), await the promise, on success assign entry.instance = result, delete entry.creatingPromise, emit registry.events.emit("instance:created", resolvedType, instance) and return instance; on error ensure entry.creatingPromise is cleared before rethrowing so subsequent calls can retry.src/lib/storage/adapters/libsqlAdapter.ts-894-897 (1)
894-897:⚠️ Potential issue | 🟠 MajorUse one timestamp format for
expires_atcomparisons.
setRecord()storesexpires_atwithtoISOString(), while the read/delete queries compare it todatetime('now'). Because libSQL compares these as text, same-day expired records can still look unexpired, so TTL becomes unreliable.Also applies to: 942-945, 976-979, 1003-1006, 1093-1096
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/storage/adapters/libsqlAdapter.ts` around lines 894 - 897, The TTL logic mixes ISO strings (toISOString()) with SQL datetime('now') text comparisons, causing unreliable expiry checks; modify storage to use a single numeric timestamp format: change setRecord() to store expires_at as an integer epoch seconds (e.g., Math.floor(Date.now()/1000) + options.ttl) instead of toISOString(), and update all read/delete queries (the places that currently compare expires_at to datetime('now')) to compare against the UNIX epoch via strftime('%s','now') (e.g., WHERE expires_at IS NULL OR expires_at > strftime('%s','now')) so expires_at comparisons are numeric and consistent across setRecord, the read methods, delete/remove methods and any purgeExpired logic (affecting the other occurrences you noted).src/lib/storage/adapters/libsqlAdapter.ts-190-218 (1)
190-218:⚠️ Potential issue | 🟠 MajorPrefix the index names too.
tablePrefixnamespaces the tables, but theidx_*names stay global. A second prefixed adapter in the same database can skip or fail index creation because those names already belong to a different table.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/storage/adapters/libsqlAdapter.ts` around lines 190 - 218, The index names are global and collide across prefixed adapters; update each CREATE INDEX statement in libsqlAdapter (the calls to this.client.execute around tableName("threads"/"messages"/"workflow_runs"/"custom_records")) to include the adapter's table prefix (e.g. use this.tablePrefix or derive from this.tableName) in the index name so they're unique per namespace (for example change idx_threads_resource_id to `${this.tablePrefix}_idx_threads_resource_id` or include the resolved table name), keeping the same ON ${this.tableName(...)}(...) clauses.src/lib/storage/adapters/SQLiteStorageAdapter.ts-359-385 (1)
359-385:⚠️ Potential issue | 🟠 Major
updateMessage()ignorestypeandtoolInfo.The update builder only persists
content,role, andmetadata, so changing a message type or tool payload becomes a silent no-op on this backend.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/storage/adapters/SQLiteStorageAdapter.ts` around lines 359 - 385, The updateMessage method currently only updates content, role, and metadata; add handling for updates.type and updates.toolInfo so those changes persist: inside updateMessage (function name) add setClauses and params entries when updates.type !== undefined (push "type = ?" and the value) and when updates.toolInfo !== undefined (push "tool_info = ?" and push JSON.stringify(updates.toolInfo)), then run the same prepared UPDATE so the SQL includes those columns; ensure the column names match the table schema ("type" and "tool_info") and serialization for toolInfo is consistent with how metadata is stored.src/lib/storage/adapters/SQLiteStorageAdapter.ts-547-564 (1)
547-564:⚠️ Potential issue | 🟠 MajorDon't null out
outputanderrorwhen the caller omits them.This UPDATE always binds both columns, so
updateWorkflowRunStatus(runId, status)clears any previously stored output/error. Preserve the old values unless those optional arguments are explicitly provided.Suggested fix
async updateWorkflowRunStatus( runId: string, status: WorkflowRunStatus, output?: JsonValue, error?: StorageWorkflowRun["error"], ): Promise<StorageWorkflowRun | null> { this.ensureInitialized(); - const result = this.db!.prepare( - `UPDATE ${this.table("workflow_runs")} - SET status = ?, output = ?, error = ?, updated_at = ? - WHERE id = ?`, - ).run( - status, - output !== undefined ? JSON.stringify(output) : null, - error ? JSON.stringify(error) : null, - new Date().toISOString(), - runId, - ); + const now = new Date().toISOString(); + const setClauses = ["status = ?", "updated_at = ?"]; + const params: unknown[] = [status, now]; + + if (output !== undefined) { + setClauses.push("output = ?"); + params.push(JSON.stringify(output)); + } + if (error !== undefined) { + setClauses.push("error = ?"); + params.push(error ? JSON.stringify(error) : null); + } + + params.push(runId); + + const result = this.db!.prepare( + `UPDATE ${this.table("workflow_runs")} + SET ${setClauses.join(", ")} + WHERE id = ?`, + ).run(...params);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/storage/adapters/SQLiteStorageAdapter.ts` around lines 547 - 564, The UPDATE in SQLiteStorageAdapter.updateWorkflowRunStatus always binds output and error, causing omitted optional params to wipe existing values; change the method to only include "output = ?" and/or "error = ?" in the SET clause when those arguments are explicitly provided (distinguish undefined vs null), build the parameter array in the same order, JSON.stringify provided values, always include updated_at and runId, and execute the prepared statement so existing columns are preserved when callers omit output or error.src/lib/storage/adapters/postgresAdapter.ts-107-113 (1)
107-113:⚠️ Potential issue | 🟠 MajorMake TLS verification required, not disabled by default.
When
sslistrue, the code setsrejectUnauthorized: false, which disables PostgreSQL server certificate verification. This allows connections to any server with any certificate—including self-signed, expired, or forged ones—and leaves the connection vulnerable to man-in-the-middle attacks. Callers requesting SSL expect verification to remain enabled unless they explicitly opt out.Require callers to provide a
rejectUnauthorized: falseoption explicitly if they need to skip verification. For example:
- When
config.ssl === true, use{ rejectUnauthorized: true }(or justtrueif thepglibrary defaults totrue)- When
config.sslis an object, pass it through as-is and let callers control verification🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/storage/adapters/postgresAdapter.ts` around lines 107 - 113, The code in postgresAdapter.ts currently disables TLS verification by default when building poolConfig.ssl: change the logic in the block handling this.config.ssl so that when this.config.ssl === true you set poolConfig.ssl to require verification (e.g., truthy or an object with rejectUnauthorized: true) instead of { rejectUnauthorized: false }, and when this.config.ssl is an object pass it through unchanged so callers can explicitly set rejectUnauthorized: false only if they opt out; update the branch that assigns poolConfig.ssl (and any related comments) to reflect this behavior.src/lib/storage/StorageRegistry.ts-100-105 (1)
100-105:⚠️ Potential issue | 🟠 MajorReset failed initialization attempts.
If
initialize()rejects once,initPromisestays set to the rejected promise and every laterensureInitialized()call fails immediately. That makes transient startup failures unrecoverable until process restart.Suggested fix
async ensureInitialized(): Promise<void> { if (this.initialized) { return; } if (this.initPromise) { return this.initPromise; } - this.initPromise = this.initialize(); - await this.initPromise; - this.initialized = true; + this.initPromise = (async () => { + await this.initialize(); + this.initialized = true; + })(); + try { + await this.initPromise; + } catch (error) { + this.initPromise = null; + throw error; + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/storage/StorageRegistry.ts` around lines 100 - 105, The current logic leaves this.initPromise set to a rejected promise, making future calls fail; update the ensure-initialization flow (where this.initPromise is assigned to this.initialize()) to wrap the await in try/catch: set this.initPromise = this.initialize(); try { await this.initPromise; this.initialized = true; } catch (err) { this.initPromise = undefined; throw err; } — this ensures initialize() failures clear this.initPromise so subsequent calls can retry while preserving initialize() and this.initPromise identifiers.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bf3c3bf9-59f4-47a7-a24f-df70e7883684
⛔ Files ignored due to path filters (32)
docs-site/static/docs/assets/images/business-use-cases.pngis excluded by!**/*.pngdocs-site/static/docs/assets/images/cli-help-demo.pngis excluded by!**/*.pngdocs-site/static/docs/assets/images/creative-tools.pngis excluded by!**/*.pngdocs-site/static/docs/assets/images/developer-tools.pngis excluded by!**/*.pngdocs-site/static/docs/assets/images/mcp-tools.pngis excluded by!**/*.pngdocs-site/static/docs/assets/images/monitoring-analytics.pngis excluded by!**/*.pngdocs-site/static/docs/assets/images/provider-status.pngis excluded by!**/*.pngdocs-site/static/docs/assets/images/text-generation.pngis excluded by!**/*.pngdocs-site/static/docs/assets/images/web-demo-overview.pngis excluded by!**/*.pngdocs-site/static/docs/visual-content/screenshots/ai-workflow-tools-cli-demo.pngis excluded by!**/*.pngdocs-site/static/docs/visual-content/screenshots/mcp-cli/01-mcp-help-2025-06-09.pngis excluded by!**/*.pngdocs-site/static/docs/visual-content/screenshots/mcp-cli/01-mcp-help-2025-06-10.pngis excluded by!**/*.pngdocs-site/static/docs/visual-content/screenshots/mcp-cli/02-mcp-install-2025-06-09.pngis excluded by!**/*.pngdocs-site/static/docs/visual-content/screenshots/mcp-cli/02-mcp-install-2025-06-10.pngis excluded by!**/*.pngdocs-site/static/docs/visual-content/screenshots/mcp-cli/03-mcp-list-status-2025-06-09.pngis excluded by!**/*.pngdocs-site/static/docs/visual-content/screenshots/mcp-cli/03-mcp-list-status-2025-06-10.pngis excluded by!**/*.pngdocs-site/static/docs/visual-content/screenshots/mcp-cli/04-mcp-test-server-2025-06-09.pngis excluded by!**/*.pngdocs-site/static/docs/visual-content/screenshots/mcp-cli/04-mcp-test-server-2025-06-10.pngis excluded by!**/*.pngdocs-site/static/docs/visual-content/screenshots/mcp-cli/05-mcp-custom-server-2025-06-09.pngis excluded by!**/*.pngdocs-site/static/docs/visual-content/screenshots/mcp-cli/05-mcp-custom-server-2025-06-10.pngis excluded by!**/*.pngdocs-site/static/docs/visual-content/screenshots/mcp-cli/06-mcp-workflow-demo-2025-06-09.pngis excluded by!**/*.pngdocs-site/static/docs/visual-content/screenshots/mcp-cli/06-mcp-workflow-demo-2025-06-10.pngis excluded by!**/*.pngdocs-site/static/docs/visual-content/screenshots/phase-1-2-workflow/01-phase-1-2-overview.pngis excluded by!**/*.pngdocs-site/static/docs/visual-content/screenshots/phase-1-2-workflow/02-generate-test-cases.pngis excluded by!**/*.pngdocs-site/static/docs/visual-content/screenshots/phase-1-2-workflow/03-refactor-code.pngis excluded by!**/*.pngdocs-site/static/docs/visual-content/screenshots/phase-1-2-workflow/04-generate-documentation.pngis excluded by!**/*.pngdocs-site/static/docs/visual-content/screenshots/phase-1-2-workflow/05-debug-ai-output.pngis excluded by!**/*.pngdocs-site/static/docs/visual-content/screenshots/phase-1-2-workflow/06-workflow-integration.pngis excluded by!**/*.pngdocs-site/static/docs/visual-content/screenshots/phase-1-2-workflow/07-phase-1-2-metrics.pngis excluded by!**/*.pngdocs-site/static/img/favicon.icois excluded by!**/*.icodocs-site/static/img/neurolink-social-card.svgis excluded by!**/*.svgpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (132)
.gitignoredocs-site/.env.exampledocs-site/babel.config.jsdocs-site/content/docs/.gitkeepdocs-site/scripts/create-version.tsdocs-site/scripts/validate-frontmatter.tsdocs-site/scripts/visual-verification.pydocs-site/src/components/Button/Button.module.cssdocs-site/src/components/CardGrid/CardGrid.module.cssdocs-site/src/components/CopyPageButton/CopyPageButton.module.cssdocs-site/src/components/CopyPageButton/index.tsxdocs-site/src/components/DropdownMenu/DropdownMenu.module.cssdocs-site/src/components/DropdownMenu/index.tsxdocs-site/src/components/GithubLink/GithubLink.module.cssdocs-site/src/components/GithubLink/index.tsxdocs-site/src/components/PropertiesTable/index.tsxdocs-site/src/components/ProviderModelsTable/ProviderModelsTable.module.cssdocs-site/src/components/ProviderModelsTable/index.tsxdocs-site/src/components/Search/EmptySearch.tsxdocs-site/src/components/Search/SearchInput.tsxdocs-site/src/components/Search/index.tsdocs-site/src/components/Steps/Steps.module.cssdocs-site/src/components/Steps/index.tsxdocs-site/src/components/YouTube/YouTube.module.cssdocs-site/src/components/YouTube/index.tsxdocs-site/src/components/icons/ArrowIcon.tsxdocs-site/src/components/icons/CloseIcon.tsxdocs-site/src/components/icons/ReturnIcon.tsxdocs-site/src/components/icons/SearchIcon.tsxdocs-site/src/components/icons/index.tsdocs-site/src/components/ui/Kbd.module.cssdocs-site/src/components/ui/Kbd.tsxdocs-site/src/css/fonts.cssdocs-site/src/css/utilities.cssdocs-site/src/hooks/useAlgoliaSearch.tsdocs-site/src/theme/Root.tsxdocs-site/src/theme/prism-neurolink-light.tsdocs-site/static/CNAMEdocs-site/static/img/.gitkeepdocs-site/versions.jsondocs/advanced/mcp-integration.mddocs/cookbook/streaming-with-retry.mddocs/cookbook/structured-output.mddocs/features/multimodal.mddocs/features/structured-output.mddocs/features/thinking-configuration.mddocs/features/tts.mddocs/guides/examples/code-patterns.mddocs/mastra-features-implementation/18-storage-abstraction.mddocs/sdk-custom-tools.mddocs/storage-tests/CLI-COVERAGE.mddocs/storage-tests/CONFIGURATION.mddocs/storage-tests/TESTING.mddocs/storage-tests/VERIFICATION.mdmkdocs.ymlpackage.jsonpnpm-workspace.yamlrequirements.txtscripts/build-browser.mjssrc/cli/commands/storage.tssrc/cli/factories/commandFactory.tssrc/cli/parser.tssrc/lib/core/conversationMemoryFactory.tssrc/lib/core/conversationMemoryInitializer.tssrc/lib/core/storageConversationMemoryManager.tssrc/lib/index.tssrc/lib/mcp/auth/tokenStorage.tssrc/lib/mcp/servers/storage/storageServer.tssrc/lib/neurolink.tssrc/lib/storage/StorageRegistry.tssrc/lib/storage/adapters/FileStorageAdapter.tssrc/lib/storage/adapters/S3StorageAdapter.tssrc/lib/storage/adapters/SQLiteStorageAdapter.tssrc/lib/storage/adapters/libsqlAdapter.tssrc/lib/storage/adapters/memoryAdapter.tssrc/lib/storage/adapters/mongodbAdapter.tssrc/lib/storage/adapters/postgresAdapter.tssrc/lib/storage/adapters/redisAdapter.tssrc/lib/storage/connectionPool.tssrc/lib/storage/healthCheck.tssrc/lib/storage/index.tssrc/lib/storage/managers/index.tssrc/lib/storage/managers/keyValueStore.tssrc/lib/storage/managers/threadManager.tssrc/lib/storage/managers/workflowPersistenceManager.tssrc/lib/storage/middleware/CachingMiddleware.tssrc/lib/storage/middleware/CompressionMiddleware.tssrc/lib/storage/middleware/EncryptionMiddleware.tssrc/lib/storage/migrations/index.tssrc/lib/storage/migrations/runner.tssrc/lib/storage/storageFactory.tssrc/lib/storage/storageProvider.tssrc/lib/storage/transactions.tssrc/lib/telemetry/attributes.tssrc/lib/telemetry/tracers.tssrc/lib/types/common.tssrc/lib/types/config.tssrc/lib/types/index.tssrc/lib/types/optional-deps.d.tssrc/lib/types/storage.tssrc/lib/types/storageInternal.tssrc/lib/types/storageMastra.tssrc/lib/utils/conversationMemory.tssrc/lib/workflow/core/workflowRegistry.tssrc/lib/workflow/core/workflowRunner.tstest/continuous-test-suite-storage-sdk-cli.tstest/continuous-test-suite-storage.tstest/fixtures/storage/adapter-config.jsontest/fixtures/storage/middleware-config.jsontest/fixtures/storage/migration-scripts.jsontest/integration/storage/README.mdtest/storage/MigrationManager.test.tstest/storage/StorageFactory.test.tstest/storage/StorageRegistry.test.tstest/storage/adapters/FileStorageAdapter.test.tstest/storage/adapters/MemoryStorageAdapter.test.tstest/storage/adapters/MongoDBStorageAdapter.test.tstest/storage/adapters/PostgreSQLStorageAdapter.test.tstest/storage/adapters/RedisStorageAdapter.test.tstest/storage/adapters/S3StorageAdapter.test.tstest/storage/adapters/SQLiteStorageAdapter.test.tstest/storage/integration/storage.integration.test.tstest/storage/middleware/CachingMiddleware.test.tstest/storage/middleware/CompressionMiddleware.test.tstest/storage/middleware/EncryptionMiddleware.test.tstest/unit/storage/TEST_SUMMARY.mdtest/unit/storage/keyValueStore.test.tstest/unit/storage/memoryAdapter.test.tstest/unit/storage/storageFactory.test.tstest/unit/storage/threadManager.test.tstest/unit/storage/workflowPersistenceManager.test.tsvite.config.ts
💤 Files with no reviewable changes (37)
- docs-site/babel.config.js
- docs-site/src/components/YouTube/YouTube.module.css
- docs-site/static/CNAME
- docs-site/versions.json
- docs-site/src/components/icons/CloseIcon.tsx
- docs-site/src/components/GithubLink/GithubLink.module.css
- docs-site/src/components/icons/SearchIcon.tsx
- docs-site/src/components/icons/index.ts
- docs-site/src/theme/prism-neurolink-light.ts
- docs-site/src/components/ProviderModelsTable/ProviderModelsTable.module.css
- docs-site/src/components/Button/Button.module.css
- docs-site/src/components/ui/Kbd.tsx
- docs-site/.env.example
- docs-site/src/components/YouTube/index.tsx
- docs-site/src/components/DropdownMenu/DropdownMenu.module.css
- docs-site/src/components/icons/ArrowIcon.tsx
- docs-site/src/components/Steps/Steps.module.css
- docs-site/src/components/PropertiesTable/index.tsx
- docs-site/src/css/fonts.css
- docs-site/src/css/utilities.css
- docs-site/src/components/icons/ReturnIcon.tsx
- docs-site/src/components/GithubLink/index.tsx
- docs-site/src/components/CopyPageButton/index.tsx
- docs-site/src/components/CardGrid/CardGrid.module.css
- docs-site/src/components/Search/index.ts
- docs-site/scripts/validate-frontmatter.ts
- docs-site/scripts/create-version.ts
- docs-site/src/components/Steps/index.tsx
- docs-site/src/theme/Root.tsx
- docs-site/src/components/Search/EmptySearch.tsx
- docs-site/src/components/ProviderModelsTable/index.tsx
- docs-site/src/components/ui/Kbd.module.css
- docs-site/scripts/visual-verification.py
- docs-site/src/hooks/useAlgoliaSearch.ts
- docs-site/src/components/CopyPageButton/CopyPageButton.module.css
- docs-site/src/components/DropdownMenu/index.tsx
- docs-site/src/components/Search/SearchInput.tsx
| // Connect to StorageFactory — delegate actual adapter creation there | ||
| const { StorageFactory } = await import("./storageFactory.js"); | ||
| await StorageFactory.registerAllAdapters(); | ||
|
|
||
| // Register metadata for health monitoring (instances come from StorageFactory) | ||
| const defaults: Array<{ | ||
| type: StorageBackendType; | ||
| aliases: string[]; | ||
| metadata: Omit<StorageBackendMetadata, "type">; | ||
| }> = [ |
There was a problem hiding this comment.
Preserve the built-in factories when registering default metadata.
initialize() calls StorageFactory.registerAllAdapters() and then registerMetadata() replaces each built-in entry with a factory that always throws. After init, getAdapter("memory")/getAdapter("postgresql") can no longer construct built-in backends unless some other path manually injects an instance.
Also applies to: 201-203, 217-223
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/storage/StorageRegistry.ts` around lines 114 - 123, initialize()
calls StorageFactory.registerAllAdapters() but then registerMetadata()
overwrites built-in factories with placeholders that throw, breaking
getAdapter("memory")/getAdapter("postgresql"); update registerMetadata (or
StorageRegistry.initialize) to preserve any factories already registered on
StorageFactory: when iterating defaults, check StorageFactory.hasFactory(type)
or StorageFactory.getFactory(type) and skip replacing it (or merge metadata into
the existing factory) instead of blindly assigning a new throwing factory.
Ensure reference to StorageFactory.registerAllAdapters(),
StorageFactory.getFactory()/hasFactory(), StorageFactory.registerAllAdapters(),
StorageRegistry.initialize, registerMetadata(), and getAdapter() so the change
is made where defaults are applied.
1dde981 to
35c9796
Compare
🤖 AI Review & Build Compliance ✅Status: AI analysis complete • Build rules validated • Ready for review 📊 View detailed analysis results🛡️ Analysis Complete
📋 Ready for Merge When
🤖 AI analysis complete - check individual code comments for specific feedback |
35c9796 to
41b4639
Compare
Introduces a pluggable storage abstraction for NeuroLink with 8 backends, 3 middleware, transaction support, health monitoring, migration runner, and CLI commands. All types live in the canonical src/lib/types/ and follow the project's strict type-engineering rules. Adapters (8): Memory, File, SQLite, LibSQL, Redis, PostgreSQL, MongoDB, S3 Middleware (3): Caching (LRU), Encryption (AES-256-GCM), Compression (gzip/brotli) Core features: - StorageFactory + StorageRegistry (factory + registry pattern, mirrors AI providers) - TransactionManager with retry/backoff for serialization conflicts - StorageHealthMonitor with latency thresholds and aggregation - MigrationRunner with lock-based concurrency and functional migrations - ThreadManager, WorkflowPersistenceManager, KeyValueStore (domain facades) - CLI commands: storage info, storage status, storage health, storage migrate, storage clear Packaging: - Optional peer dependencies (optional: true) for DB drivers — zero runtime cost when unused, typed via ambient declarations in types/optional-deps.d.ts - Browser build stubs native Node APIs for all storage adapters - publint clean, 0 type errors, 0 lint errors Testing: - Continuous test suite (17/17 passing): all adapters, middleware, migration, factory/registry, and integration scenarios - SDK + CLI Generate/Stream test suite (8/8 passing): validates storage integration alongside the canonical NeuroLink entry points
41b4639 to
cad8ba8
Compare
🤖 AI Review & Build Compliance ✅Status: AI analysis complete • Build rules validated • Ready for review 📊 View detailed analysis results🛡️ Analysis Complete
📋 Ready for Merge When
🤖 AI analysis complete - check individual code comments for specific feedback |
Pull Request
Description
What does this PR do?
Adds a complete, pluggable storage abstraction layer to NeuroLink with 8 backend adapters, 3 middleware, transaction support, health monitoring, migration runner, and CLI commands. All types follow the project's strict type-engineering rules (canonical
src/lib/types/,typenotinterface, barrel imports, unique names).Type of Change
Motivation and Context
NeuroLink needed a unified persistence layer to support conversation threads, messages, workflow runs, and custom key-value records across multiple backends. The design mirrors the existing AI provider factory + registry pattern for consistency.
Changes Made
Adapters (8):
MemoryAdapter— zero-dependency in-process Map storage with LRU evictionFileStorageAdapter— debounced atomic file writes with in-memory working setSQLiteStorageAdapter— better-sqlite3 with WAL mode and foreign key cascadesLibSQLAdapter— @libsql/client supporting local SQLite + remote Turso replicasPostgresAdapter— pg with JSONB columns, proper upserts, schema isolationMongoDBAdapter— mongodb with TTL indexes and separate collectionsRedisAdapter— ioredis with sorted-set message ordering and pipeline batchingS3StorageAdapter— @aws-sdk/client-s3 with in-memory index and SSE supportMiddleware (3):
CachingMiddleware— LRU cache with TTL, stats, pattern invalidationEncryptionMiddleware— AES-256-GCM/CBC/ChaCha20, scrypt/pbkdf2 key derivationCompressionMiddleware— gzip/deflate/brotli with size thresholdInfrastructure:
StorageFactory+StorageRegistry(factory + registry pattern)TransactionManagerwith retry/backoff for serialization conflictsStorageHealthMonitorwith latency thresholds and aggregationMigrationRunnerwith lock-based concurrency and functional migrationsThreadManager,WorkflowPersistenceManager,KeyValueStore(domain facades)CLI Commands:
neurolink storage info— list available backendsneurolink storage status— show storage statisticsneurolink storage health— check backend healthneurolink storage migrate— run migrationsneurolink storage clear— clear all dataType System:
src/lib/types/(storage.ts, storageMastra.ts, storageInternal.ts)interface— alltypedeclarationsStorageprefix to avoid barrel collisionsPackaging:
Breaking Changes
Testing
Test Results
pnpm run check)pnpm run lint)pnpm run build)Adapter suite breakdown:
SDK+CLI suite breakdown:
Code Quality
Dependencies
peerDependencies (all
optional: true):@aws-sdk/client-s3^3.1030.0 — S3 storage adapter@libsql/client^0.17.0 — LibSQL/Turso adapterbetter-sqlite3^11.0.0 — SQLite adapterioredis^5.9.2 — Redis adaptermongodb^7.0.0 — MongoDB adapterpg^8.17.2 — PostgreSQL adapterdevDependencies:
@types/pg^8.11.10 — PostgreSQL type declarations@types/better-sqlite3^7.6.12 — SQLite type declarationsCommit Message Format
type(scope): descriptionPre-submission Checklist
pnpm buildSummary by CodeRabbit
Release Notes
New Features
status,migrate,health,check,clear, andinfoDocumentation