chore: move sqlite-vfs to separate package

[NathanFlurry]

Description

Please include a summary of the changes and the related issue. Please also include relevant motivation and context.

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes.

Checklist:

• My code follows the style guidelines of this project
• I have performed a self-review of my code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes

[NathanFlurry]

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

• feat: implement actor sandbox example #4140
• chore: migrate sqlite vfs to native rust #4143
• chore: move sqlite-vfs to separate package #4142 👈 (View in Graphite)
• feat: sqlite #3666
• feat: use cases #4100 : 1 other dependent PR (#4124 )
• feat(dashboard): traces timeline ui #4056 : 1 other dependent PR (#4081 )
• fix(traces): separate entrypoints #4119 : 1 other dependent PR (#4126 )
• refactor(rivetkit): migrate granularly to zod v4 #4121
• fix(rivekit): stub traces in browser #4120
• main

---

How to use the Graphite Merge Queue

Add the label merge-queue to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

[claude]

PR Review: Move sqlite-vfs to separate package

Summary

This PR extracts the SQLite VFS implementation from the main rivetkit package into a separate @rivetkit/sqlite-vfs package. This is a good refactoring that improves tree-shaking and modularization.

Critical Issues 🔴

1. Duplicate waitForNames method in queue-manager.ts

Location: rivetkit-typescript/packages/rivetkit/src/actor/instance/queue-manager.ts:316 and :410

There are two identical waitForNames methods defined in the same class. This will cause a TypeScript compilation error.

The first occurrence (lines 316-360):

• Has a simpler implementation with #nameWaiters map
• Takes abortSignal: AbortSignal as required parameter
• Returns early if messages already exist

The second occurrence (lines 410-460):

• Uses #messageListeners set
• Takes abortSignal?: AbortSignal as optional parameter
• Has more complex event cleanup logic

Recommendation: Remove one of these implementations. Based on the code structure, the second implementation (lines 410-460) appears to be the intended one as it uses the #messageListeners pattern that's consistent with the rest of the class methods like #notifyMessageListeners and #removeMessageListener.

Code Quality Issues ⚠️

2. AsyncMutex implementation

Location: rivetkit-typescript/packages/sqlite-vfs/src/index.ts:106-133

The AsyncMutex class has a potential edge case where if release() is called before any waiters are added to the queue, but after checking this.#locked, waiters could get stuck.

This implementation is functional but could be more robust. Consider using a standard async mutex library or adding documentation about thread safety guarantees.

3. VFS debug logging uses process.env at module level

Location: rivetkit-typescript/packages/sqlite-vfs/src/index.ts:14

This is evaluated once at module load time. If running in an environment where environment variables can change, this won't pick up changes. Consider making this a function or document this limitation.

4. Missing cleanup in xDelete

Location: rivetkit-typescript/packages/sqlite-vfs/src/index.ts:613-622

Issues:

• Uses void to discard the promise, making deletion fire-and-forget. This could lead to race conditions.
• Comment says "we can't easily delete all chunks" but metadata deletion should ideally be synchronous or at least tracked
• Should this return SQLITE_IOERR if options is not found?

5. Error handling in xOpen could leak file handles

Location: rivetkit-typescript/packages/sqlite-vfs/src/index.ts:348-389

If an error occurs after this.#openFiles.set(fileId, file) on line 378, the file entry is added to the map but the function returns SQLITE_CANTOPEN. The file entry should be cleaned up in the error case.

Performance Considerations 📊

6. Redundant metadata updates

Location: rivetkit-typescript/packages/rivetkit/src/actor/instance/queue-manager.ts:192-201

Metadata is being updated on every enqueue operation. For high-throughput scenarios, consider batching metadata updates or using a write-behind cache strategy.

7. Multiple sequential KV operations in xWrite

Location: rivetkit-typescript/packages/sqlite-vfs/src/index.ts:475-569

The xWrite operation does:

1. getBatch for chunks
2. putBatch for updated chunks
3. put for metadata (if size changed)

Consider combining the last two operations into a single batch if size changed, to reduce round trips.

Positive Aspects ✅

1. Good separation of concerns - Moving SQLite VFS to a separate package improves tree-shaking
2. Clean API design - The KvVfsOptions interface provides a clean abstraction over storage
3. Proper use of BARE schemas - Version-aware serialization with FILE_META_VERSIONED
4. Good test coverage - Basic test verifying persistence across VFS instances
5. Clear documentation - Good JSDoc comments throughout the VFS implementation
6. Follows repository conventions - Uses workspace dependencies correctly, follows naming patterns

Security Considerations 🔒

8. No bounds checking on chunk index

Location: rivetkit-typescript/packages/sqlite-vfs/src/kv.ts:38-50

The getChunkKey function doesn't validate the chunkIndex parameter. While JavaScript's setUint32 will throw on invalid values, explicit validation would be better.

Testing Recommendations 🧪

1. Add test for concurrent database operations with multiple SqliteVfs instances
2. Add test for file deletion (xDelete) functionality
3. Add test for large transactions that span multiple chunks
4. Add error case tests (invalid operations, corrupted metadata, etc.)
5. Add test for the metadata recovery path in queue-manager

Documentation 📝

The new package's README should include:

• Installation instructions
• Basic usage example
• Performance characteristics (chunk size, when to use this vs other storage)
• Limitations (Node.js only, no browser support as written)

Summary

Must fix before merge:

• Remove duplicate waitForNames method in queue-manager.ts

Should fix:

• Cleanup file handles on error in xOpen
• Make xDelete operations synchronous or properly tracked

Consider for follow-up:

• Add more comprehensive tests
• Optimize metadata update batching
• Add bounds checking for chunk indices

Overall, this is a well-structured refactoring that improves the codebase modularity. The critical duplicate method issue must be resolved, but the rest of the implementation is solid.

[graphite-app]

Merge activity

• Feb 6, 8:22 AM UTC: NathanFlurry added this pull request to the Graphite merge queue.
• Feb 6, 8:23 AM UTC: CI is running for this pull request on a draft pull request (#4146) due to your merge queue CI optimization settings.
• Feb 6, 8:24 AM UTC: Merged by the Graphite merge queue via draft PR: #4146.