-
-
Notifications
You must be signed in to change notification settings - Fork 407
feat(libsql): add Edge/Cloudflare Workers support #887
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Add edge runtime compatibility for @voltagent/libsql package, enabling
use with Cloudflare Workers, Vercel Edge Functions, and other edge runtimes.
Changes:
- Refactor adapters into core classes with dependency injection
- Add edge-specific adapters using @libsql/client/web
- Add new ./edge export path for edge environments
- Use DataView/ArrayBuffer in core for cross-platform compatibility
- Node.js adapters override with Buffer for better performance
- Update Cloudflare Workers docs with Turso/LibSQL example
Usage:
```typescript
import { LibSQLMemoryAdapter } from "@voltagent/libsql/edge";
const adapter = new LibSQLMemoryAdapter({
url: "libsql://your-db.turso.io",
authToken: "your-token",
});
```
🦋 Changeset detectedLatest commit: f8cdfb5 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
5 issues found across 14 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/libsql/src/memory-v2-adapter-edge.ts">
<violation number="1" location="packages/libsql/src/memory-v2-adapter-edge.ts:72">
P2: The `debug` option is declared in the interface but never used when creating the logger. Users setting `debug: true` will not get debug logging. The similar `vector-adapter-edge.ts` correctly uses `level: options.debug ? "debug" : "info"` when creating the pino logger.</violation>
</file>
<file name="packages/libsql/src/memory-core.ts">
<violation number="1" location="packages/libsql/src/memory-core.ts:842">
P2: Potential SQL injection: `orderBy` and `orderDirection` are interpolated into SQL without runtime validation. TypeScript types provide compile-time safety, but runtime validation should be added for defense-in-depth, especially for a library that may be consumed by JavaScript code.</violation>
</file>
<file name="packages/libsql/src/vector-core.ts">
<violation number="1" location="packages/libsql/src/vector-core.ts:424">
P2: Cache is documented as LRU but implemented as FIFO. The `get()` method retrieves cached items without updating their position in the Map. JavaScript Map maintains insertion order, so `Map.keys().next().value` returns the oldest inserted item, not the least recently accessed. Frequently accessed items may be evicted while stale items remain.</violation>
</file>
<file name="packages/libsql/src/vector-adapter-edge.ts">
<violation number="1" location="packages/libsql/src/vector-adapter-edge.ts:64">
P2: The default createPinoLogger uses Node.js-specific APIs (process.env, pino streams) that may not be available in edge runtimes. Consider making the logger parameter required for edge environments, or adding a warning in the documentation that users should provide an edge-compatible logger.</violation>
</file>
<file name="packages/libsql/src/observability-core.ts">
<violation number="1" location="packages/libsql/src/observability-core.ts:65">
P2: Inconsistent initialization pattern - cached promise prevents recovery from transient failures. Unlike `memory-core.ts` and `vector-core.ts` which use a boolean flag allowing retry on failure, this adapter caches the initialization promise. If `initializeDatabase()` fails, subsequent operations will always reject with the same error, even if the database becomes available.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Ask questions if you need clarification on any suggestion
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
|
|
||
| const orderBy = options.orderBy || "updated_at"; | ||
| const orderDirection = options.orderDirection || "DESC"; | ||
| sql += ` ORDER BY ${orderBy} ${orderDirection}`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: Potential SQL injection: orderBy and orderDirection are interpolated into SQL without runtime validation. TypeScript types provide compile-time safety, but runtime validation should be added for defense-in-depth, especially for a library that may be consumed by JavaScript code.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/libsql/src/memory-core.ts, line 842:
<comment>Potential SQL injection: `orderBy` and `orderDirection` are interpolated into SQL without runtime validation. TypeScript types provide compile-time safety, but runtime validation should be added for defense-in-depth, especially for a library that may be consumed by JavaScript code.</comment>
<file context>
@@ -0,0 +1,1225 @@
+
+ const orderBy = options.orderBy || "updated_at";
+ const orderDirection = options.orderDirection || "DESC";
+ sql += ` ORDER BY ${orderBy} ${orderDirection}`;
+
+ if (options.limit) {
</file context>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pre-existing issue - this code was moved from the original memory-v2-adapter.ts (lines 1004-1006) without modification. The refactoring in this PR only extracted shared logic into core classes.
That said, the concern is valid. While the TypeScript types restrict orderBy to "created_at" | "updated_at" and orderDirection to "ASC" | "DESC", JavaScript consumers wouldn't have this protection.
Happy to add runtime validation if desired.
| async get(id: string): Promise<VectorItem | null> { | ||
| await this.initialize(); | ||
|
|
||
| if (this.vectorCache.has(id)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: Cache is documented as LRU but implemented as FIFO. The get() method retrieves cached items without updating their position in the Map. JavaScript Map maintains insertion order, so Map.keys().next().value returns the oldest inserted item, not the least recently accessed. Frequently accessed items may be evicted while stale items remain.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/libsql/src/vector-core.ts, line 424:
<comment>Cache is documented as LRU but implemented as FIFO. The `get()` method retrieves cached items without updating their position in the Map. JavaScript Map maintains insertion order, so `Map.keys().next().value` returns the oldest inserted item, not the least recently accessed. Frequently accessed items may be evicted while stale items remain.</comment>
<file context>
@@ -0,0 +1,532 @@
+ async get(id: string): Promise<VectorItem | null> {
+ await this.initialize();
+
+ if (this.vectorCache.has(id)) {
+ const cached = this.vectorCache.get(id);
+ if (cached) {
</file context>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pre-existing issue - this code was moved from the original adapter without modification. The refactoring in this PR only extracted shared logic into core classes; it didn't change the implementation behavior.
| // Initialize logger | ||
| const logger = | ||
| options.logger ?? | ||
| createPinoLogger({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: The default createPinoLogger uses Node.js-specific APIs (process.env, pino streams) that may not be available in edge runtimes. Consider making the logger parameter required for edge environments, or adding a warning in the documentation that users should provide an edge-compatible logger.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/libsql/src/vector-adapter-edge.ts, line 64:
<comment>The default createPinoLogger uses Node.js-specific APIs (process.env, pino streams) that may not be available in edge runtimes. Consider making the logger parameter required for edge environments, or adding a warning in the documentation that users should provide an edge-compatible logger.</comment>
<file context>
@@ -0,0 +1,77 @@
+ // Initialize logger
+ const logger =
+ options.logger ??
+ createPinoLogger({
+ name: "libsql-vector-adapter-edge",
+ level: options.debug ? "debug" : "info",
</file context>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not an issue. This follows the same pattern as @voltagent/supabase (line 121-123 of memory-adapter.ts) which is officially supported on Cloudflare Workers.
| maxSpansPerQuery: this.maxSpansPerQuery, | ||
| }); | ||
|
|
||
| this.initialized = this.initializeDatabase(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: Inconsistent initialization pattern - cached promise prevents recovery from transient failures. Unlike memory-core.ts and vector-core.ts which use a boolean flag allowing retry on failure, this adapter caches the initialization promise. If initializeDatabase() fails, subsequent operations will always reject with the same error, even if the database becomes available.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/libsql/src/observability-core.ts, line 65:
<comment>Inconsistent initialization pattern - cached promise prevents recovery from transient failures. Unlike `memory-core.ts` and `vector-core.ts` which use a boolean flag allowing retry on failure, this adapter caches the initialization promise. If `initializeDatabase()` fails, subsequent operations will always reject with the same error, even if the database becomes available.</comment>
<file context>
@@ -0,0 +1,880 @@
+ maxSpansPerQuery: this.maxSpansPerQuery,
+ });
+
+ this.initialized = this.initializeDatabase();
+ }
+
</file context>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pre-existing issue - this code was moved from the original adapter without modification. The refactoring in this PR only extracted shared logic into core classes; it didn't change the implementation behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
6 issues found across 14 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/libsql/src/vector-core.ts">
<violation number="1" location="packages/libsql/src/vector-core.ts:85">
P1: SQL injection risk: `tablePrefix` is interpolated directly into SQL statements without validation. Add input validation to ensure it only contains alphanumeric characters and underscores (e.g., `/^[a-zA-Z_][a-zA-Z0-9_]*$/`).</violation>
<violation number="2" location="packages/libsql/src/vector-core.ts:225">
P2: Cache mutation vulnerability: `store()` caches references to original vector/metadata without copying, but `get()` returns defensive copies. If caller mutates array/object after `store()`, cached data becomes corrupted.</violation>
<violation number="3" location="packages/libsql/src/vector-core.ts:424">
P2: Cache is documented as LRU but implements FIFO. When a cached item is accessed via `get()`, it should be moved to the end of the map to mark it as recently used. Currently, the eviction will remove the oldest inserted item rather than the least recently accessed.</violation>
</file>
<file name="packages/libsql/src/observability-adapter-edge.ts">
<violation number="1" location="packages/libsql/src/observability-adapter-edge.ts:64">
P1: Default logger `createPinoLogger` is not edge-compatible. Pino uses `process.env.NODE_ENV` and `pino-pretty` streams which don't work in Cloudflare Workers. Consider either requiring a logger to be provided in edge environments, or using a simpler edge-compatible console-based default logger.</violation>
</file>
<file name="packages/libsql/src/memory-core.ts">
<violation number="1" location="packages/libsql/src/memory-core.ts:842">
P1: Potential SQL injection: `orderBy` and `orderDirection` are interpolated directly into an SQL `ORDER BY` clause without runtime allowlisting or sanitization.</violation>
<violation number="2" location="packages/libsql/src/memory-core.ts:912">
P1: Orphan messages: `deleteConversation` only deletes from conversations table but messages table lacks CASCADE foreign key. Steps are auto-deleted (have CASCADE), but messages will remain as orphan records causing data inconsistency and storage bloat.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Ask questions if you need clarification on any suggestion
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
| async get(id: string): Promise<VectorItem | null> { | ||
| await this.initialize(); | ||
|
|
||
| if (this.vectorCache.has(id)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: Cache is documented as LRU but implements FIFO. When a cached item is accessed via get(), it should be moved to the end of the map to mark it as recently used. Currently, the eviction will remove the oldest inserted item rather than the least recently accessed.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/libsql/src/vector-core.ts, line 424:
<comment>Cache is documented as LRU but implements FIFO. When a cached item is accessed via `get()`, it should be moved to the end of the map to mark it as recently used. Currently, the eviction will remove the oldest inserted item rather than the least recently accessed.</comment>
<file context>
@@ -0,0 +1,532 @@
+ async get(id: string): Promise<VectorItem | null> {
+ await this.initialize();
+
+ if (this.vectorCache.has(id)) {
+ const cached = this.vectorCache.get(id);
+ if (cached) {
</file context>
| } | ||
|
|
||
| // Initialize the logger | ||
| const logger = options.logger || createPinoLogger({ name: "libsql-observability-edge" }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P1: Default logger createPinoLogger is not edge-compatible. Pino uses process.env.NODE_ENV and pino-pretty streams which don't work in Cloudflare Workers. Consider either requiring a logger to be provided in edge environments, or using a simpler edge-compatible console-based default logger.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/libsql/src/observability-adapter-edge.ts, line 64:
<comment>Default logger `createPinoLogger` is not edge-compatible. Pino uses `process.env.NODE_ENV` and `pino-pretty` streams which don't work in Cloudflare Workers. Consider either requiring a logger to be provided in edge environments, or using a simpler edge-compatible console-based default logger.</comment>
<file context>
@@ -0,0 +1,74 @@
+ }
+
+ // Initialize the logger
+ const logger = options.logger || createPinoLogger({ name: "libsql-observability-edge" });
+
+ // Initialize the LibSQL client using web-compatible import
</file context>
| const firstKey = this.vectorCache.keys().next().value; | ||
| if (firstKey) this.vectorCache.delete(firstKey); | ||
| } | ||
| this.vectorCache.set(id, { id, vector, metadata }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: Cache mutation vulnerability: store() caches references to original vector/metadata without copying, but get() returns defensive copies. If caller mutates array/object after store(), cached data becomes corrupted.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/libsql/src/vector-core.ts, line 225:
<comment>Cache mutation vulnerability: `store()` caches references to original vector/metadata without copying, but `get()` returns defensive copies. If caller mutates array/object after `store()`, cached data becomes corrupted.</comment>
<file context>
@@ -0,0 +1,532 @@
+ const firstKey = this.vectorCache.keys().next().value;
+ if (firstKey) this.vectorCache.delete(firstKey);
+ }
+ this.vectorCache.set(id, { id, vector, metadata });
+
+ this.logger.debug(`Vector stored: ${id} (${vector.length} dimensions)`);
</file context>
|
|
||
| constructor(client: Client, options: LibSQLVectorCoreOptions, logger: Logger) { | ||
| this.client = client; | ||
| this.tablePrefix = options.tablePrefix ?? "voltagent"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P1: SQL injection risk: tablePrefix is interpolated directly into SQL statements without validation. Add input validation to ensure it only contains alphanumeric characters and underscores (e.g., /^[a-zA-Z_][a-zA-Z0-9_]*$/).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/libsql/src/vector-core.ts, line 85:
<comment>SQL injection risk: `tablePrefix` is interpolated directly into SQL statements without validation. Add input validation to ensure it only contains alphanumeric characters and underscores (e.g., `/^[a-zA-Z_][a-zA-Z0-9_]*$/`).</comment>
<file context>
@@ -0,0 +1,532 @@
+
+ constructor(client: Client, options: LibSQLVectorCoreOptions, logger: Logger) {
+ this.client = client;
+ this.tablePrefix = options.tablePrefix ?? "voltagent";
+ this.maxVectorDimensions = options.maxVectorDimensions ?? 1536;
+ this.cacheSize = options.cacheSize ?? 100;
</file context>
| return updated; | ||
| } | ||
|
|
||
| async deleteConversation(id: string): Promise<void> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P1: Orphan messages: deleteConversation only deletes from conversations table but messages table lacks CASCADE foreign key. Steps are auto-deleted (have CASCADE), but messages will remain as orphan records causing data inconsistency and storage bloat.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/libsql/src/memory-core.ts, line 912:
<comment>Orphan messages: `deleteConversation` only deletes from conversations table but messages table lacks CASCADE foreign key. Steps are auto-deleted (have CASCADE), but messages will remain as orphan records causing data inconsistency and storage bloat.</comment>
<file context>
@@ -0,0 +1,1225 @@
+ return updated;
+ }
+
+ async deleteConversation(id: string): Promise<void> {
+ await this.initialize();
+
</file context>
|
|
||
| const orderBy = options.orderBy || "updated_at"; | ||
| const orderDirection = options.orderDirection || "DESC"; | ||
| sql += ` ORDER BY ${orderBy} ${orderDirection}`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P1: Potential SQL injection: orderBy and orderDirection are interpolated directly into an SQL ORDER BY clause without runtime allowlisting or sanitization.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/libsql/src/memory-core.ts, line 842:
<comment>Potential SQL injection: `orderBy` and `orderDirection` are interpolated directly into an SQL `ORDER BY` clause without runtime allowlisting or sanitization.</comment>
<file context>
@@ -0,0 +1,1225 @@
+
+ const orderBy = options.orderBy || "updated_at";
+ const orderDirection = options.orderDirection || "DESC";
+ sql += ` ORDER BY ${orderBy} ${orderDirection}`;
+
+ if (options.limit) {
</file context>
|
Rest of the comments: These are pre-existing issues in the original code - this PR only extracted shared logic into core classes without modifying behavior. |
omeraplak
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @nt9142 ⚡️
Summary
@voltagent/libsql/edgeexport for edge runtimes (Cloudflare Workers, Vercel Edge, etc.)@libsql/client/webfor fetch-based transportWhy this works
The edge adapters use
@libsql/client/web- Turso's official web-standard client that uses fetch/HTTP transport instead of Node.js sockets.Per Turso's SDK docs, it officially supports: "Node.js, Deno, Cloudflare Workers, Netlify & Vercel Edge Functions".
Usage
Test plan