fix(rivetkit): harden sqlite-vfs lifecycle, remove getSqliteVfs

[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

[railway-app]

🚅 Deployed to the rivet-pr-4277 environment in rivet-frontend

Service	Status	Web	Updated (UTC)
frontend-cloud	😴 Sleeping (View Logs)	Web	Feb 24, 2026 at 4:13 am
frontend-inspector	😴 Sleeping (View Logs)	Web	Feb 24, 2026 at 4:13 am
website	😴 Sleeping (View Logs)	Web	Feb 24, 2026 at 4:11 am
mcp-hub	✅ Success (View Logs)	Web	Feb 24, 2026 at 4:01 am
ladle	❌ Build Failed (View Logs)	Web	Feb 24, 2026 at 3:59 am

[NathanFlurry]

• fix(rivetkit): harden sqlite-vfs lifecycle, remove getSqliteVfs #4277 👈 (View in Graphite)
• fix(sqlite-vfs): derive vfs types from Factory #4276
• fix(rivetkit): use gatewayId instead of requestId in connection persistence log #4275
• chore(rivetkit): fix sqlite-vfs for next-js & cf workers #4271 : 1 other dependent PR (#4272 )
• chore(rivetkit): fix run engine config #4268 : 1 other dependent PR (#4273 )
• chore: multiplayer cookbook patterns #4261 : 1 other dependent PR (#4266 )
• docs: document queue with interval pattern #4265
• chore(rivetkit): migrate to sqlite instance per-actor for concurrency optimizations #4264
• docs: workflows #4256
• chore(rivetkit): update conns to be hibernatable #4260
• chore(inspector): add back database tab #4255
• chore: impl canInvoke for matchmaker #4253
• chore: multiplayer cookbook #4200 : 1 other dependent PR (#4201 )
• feat(dashboard): bring new tabs back #4249
• fix(rivetkit): fix sqlite teardown logic #4259
• 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: fix(rivetkit): harden sqlite-vfs lifecycle, remove getSqliteVfs

Summary

This PR makes three related improvements to the SQLite VFS layer: lifecycle hardening (destroy/close, error retry, destroyed guard), key layout refactoring (removes closure-based factory in favor of direct inline functions), and input validation hardening. It also fixes a SQL injection vulnerability in a test fixture.

Overall this is a solid improvement. The lifecycle changes address real bugs and the changes are well-structured. A few minor issues worth noting.

---

Security

Test fixture SQL injection fix (db-lifecycle.ts) — Good catch. The change from string interpolation to parameterized queries is correct. Even in test code, this sets a better example and prevents test failures when value contains single quotes or other SQL-special characters.

---

Positive Changes

createSqliteVfs rename — The rename from getSqliteVfs correctly signals that each call produces a new instance (factory, not getter/cache). The docstring update from "Returns" to "Creates" aligns with this.

#destroyed guard in #ensureInitialized — The double check (before and inside the IIFE) correctly handles the race where destroy() is called while WASM is still loading. The inside-IIFE early return prevents the VFS from completing setup after being torn down.

#initPromise error reset — Previously a failed #initPromise would permanently poison the instance. Resetting this.#initPromise = null in the catch block correctly allows the next caller to retry initialization (e.g., for transient WASM load failures).

destroy() idempotency — The if (#destroyed) return guard makes multiple calls safe. Setting #destroyed = true before awaiting init ensures no new operations can start during teardown.

VFS teardown on setup failure (instance/mod.ts) — The new try/catch in #setupDatabase that destroys the VFS on createClient or onMigrate failure prevents orphaned VFS instances. Previously a partial setup failure would leave the VFS alive with no cleanup path.

xClose ordering fix — Skipping the metadata flush for SQLITE_OPEN_DELETEONCLOSE files is correct. Writing metadata that is immediately deleted was wasteful and potentially masking errors.

delegalize unsigned coercion — Using >>> 0 to reinterpret both halves as unsigned before the bounds check is correct. The previous implementation was susceptible to signed-integer bugs from Emscripten's i64 legalization output.

---

Issues

getSqliteVfs singleton still exists (src/db/sqlite-vfs.ts)

This file was not touched in the PR but exports a function named getSqliteVfs that returns a shared singleton:

let sqliteVfsInstance: SqliteVfs | null = null;

export async function getSqliteVfs(): Promise<SqliteVfs> {
    if (!sqliteVfsInstance) {
        sqliteVfsInstance = new SqliteVfs();
    }
    return sqliteVfsInstance;
}

This creates a naming inconsistency: getSqliteVfs now means "singleton getter" in one place and the driver interface uses createSqliteVfs to mean "create a new instance." The singleton also contradicts the re-entrancy concern documented in the driver interface comments. Worth a follow-up to either rename/remove the singleton or update it to match the new semantics.

Open database handles not closed on destroy()

SqliteSystem.close() clears #openFiles and internal state, but does not call sqlite3.close() on any open SQLite database handles. If a caller invokes vfs.destroy() without first closing all open databases, those handles will leak.

The expected call order (close all DBs, then destroy VFS) should be documented explicitly, or SqliteSystem.close() should iterate over open handles and close them:

async close(): Promise<void> {
    for (const [, file] of this.#openFiles) {
        // close open db handles before destroying
    }
    this.#openFiles.clear();
    // ...
}

OpenFile union type (vfs.ts)

The useLegacyChunkKeys: boolean + legacyFileNameBytes?: Uint8Array pairing relies on a runtime invariant that TypeScript cannot enforce, leading to ! non-null assertions at call sites:

getChunkKey(file.legacyFileNameBytes!, chunkIndex)

A discriminated union would make this statically safe:

type OpenFile = CompactKeyFile | LegacyKeyFile;
type CompactKeyFile = { useLegacyChunkKeys: false; fileTag: SqliteFileTag; /* ... */ };
type LegacyKeyFile = { useLegacyChunkKeys: true; legacyFileNameBytes: Uint8Array; fileTag: SqliteFileTag; /* ... */ };

This is a minor style issue — the ! assertions are safe given the current call sites — but a union would eliminate them and make the invariant self-documenting.

close() alias

Having close() as a public alias for destroy() is pragmatic but may create ambiguity about which method callers should prefer. A brief comment indicating destroy() is canonical and close() is provided for ergonomic consistency with Database.close() would help clarify intent. The JSDoc comment is present but could make this more explicit.

---

Issues Summary

Severity	Location	Issue
Minor / Follow-up	src/db/sqlite-vfs.ts	getSqliteVfs still exports a singleton under the old name, inconsistent with new createSqliteVfs semantics
Minor	vfs.ts SqliteSystem.close()	Open database handles not explicitly closed before VFS teardown; expected call order should be documented or enforced
Minor	vfs.ts OpenFile	useLegacyChunkKeys + legacyFileNameBytes? pairing uses ! assertions; discriminated union would be safer
Informational	vfs.ts	destroy() / close() are identical; document which is canonical

None of these block merging. The lifecycle hardening, input validation, and SQL injection fix are all improvements worth shipping.

[claude]

PR Review: fix(rivetkit): harden sqlite-vfs lifecycle, remove getSqliteVfs

This PR makes meaningful improvements to VFS lifecycle management, delegalize correctness, and bounds checking. Below are the issues found, from highest to lowest severity.

---

Bug: DELETEONCLOSE leaves orphaned chunks in KV

File: rivetkit-typescript/packages/sqlite-vfs/src/vfs.ts — xClose and #delete

The new xClose ordering skips the metadata flush for DELETEONCLOSE files and delegates directly to #delete. However, #delete reads the file size from KV to determine how many chunk keys to erase. If file.metaDirty === true (i.e., writes happened after the last xSync), the KV still has a stale (smaller) size, so #delete leaves the extra chunks orphaned.

The worst case is a brand-new DELETEONCLOSE file that is never synced:

1. xOpen — file created in memory, KV has no metadata entry (metaDirty = false, size = 0)
2. xWrite — chunk written to KV, file.size updated in memory, metaDirty = true
3. xClose — new code calls #delete(file.path)
4. #delete calls options.get(metaKey) → returns null (metadata was never flushed) → returns early
5. The chunk written in step 2 is now permanently orphaned in KV

Even for files that are synced mid-life, any writes after the final xSync will produce orphaned chunks on close.

The old ordering (flush metadata first, then delete) was correct. The comment says the flush is unnecessary because the file is being removed immediately — but that reasoning only holds if #delete also uses the in-memory file.size rather than re-reading from KV. One clean fix: compute the chunk list in xClose directly from file.size instead of going through #delete, or pass the known size as a parameter.

---

Stale singleton getSqliteVfs not removed

File: rivetkit-typescript/packages/rivetkit/src/db/sqlite-vfs.ts

This file exports a module-level singleton:

let sqliteVfsInstance: SqliteVfs | null = null;

export async function getSqliteVfs(): Promise<SqliteVfs> {
    if (\!sqliteVfsInstance) {
        sqliteVfsInstance = new SqliteVfs();
    }
    return sqliteVfsInstance;
}

The function is no longer called anywhere in the codebase (the driver interface was renamed to createSqliteVfs and the file-system driver correctly creates a new instance per call). However, the old singleton is still exported, which means it exists as a public API surface. Now that SqliteVfs has a destroy() method, any external caller that acquires the singleton and destroys it would permanently break any other code that holds a reference to the same instance. The file should be cleaned up: remove or update getSqliteVfs and the singleton, and keep only the KvVfsOptions type re-export if it is still needed.

---

Type safety: non-null assertions on legacyFileNameBytes

File: rivetkit-typescript/packages/sqlite-vfs/src/vfs.ts — multiple sites

legacyFileNameBytes is typed as Uint8Array | undefined on OpenFile, and every usage in xRead, xWrite, xTruncate, #delete, and #setupDatabase silences the compiler with \!. This couples compile-time safety to an implicit invariant that is only enforced by the boolean flag useLegacyChunkKeys.

A discriminated union makes the invariant structural and removes the need for all those assertions:

type OpenFile = OpenFileBase & (
    | { useLegacyChunkKeys: false; fileTag: SqliteFileTag }
    | { useLegacyChunkKeys: true; legacyFileNameBytes: Uint8Array }
);

With this shape the TypeScript compiler will narrow legacyFileNameBytes to Uint8Array inside the useLegacyChunkKeys === true branch without any cast.

---

Minor: per-call Uint8Array allocation in hot path

File: rivetkit-typescript/packages/sqlite-vfs/src/kv.ts

getChunkKey and getLegacyChunkKey now allocate a new Uint8Array on every call. The previous factory pattern pre-allocated a prefix buffer once and only appended the 4-byte index per call, so large sequential reads/writes produced O(numChunks) fewer short-lived allocations. For typical SQLite workloads (frequent small reads) this is likely not the bottleneck, but it is a regression worth noting if performance is measured.

---

Positives

• delegalize rewrite is correct. Using >>> 0 to force unsigned interpretation and then comparing against MAX_FILE_SIZE_HI32/LO32 properly handles the signed-integer artifact from Emscripten legalization that the old formula papered over.
• Bounds checks on xRead, xWrite, and xTruncate are good defensive additions.
• #setupDatabase catch block now calls destroy() on the VFS before clearing the reference, which was a resource leak.
• #cleanupDatabase decoupling the VFS teardown from the onDestroy callback is the right structure — a failure in either one no longer silently suppresses the other.
• destroy() idempotency (early return on #destroyed) and the destroyed-check inside the async init IIFE handle the race correctly in JS's single-threaded event loop.
• SQL injection fix in the test fixture is correct and good hygiene, even in a test context.
• #initPromise reset on catch so transient WASM load failures are retryable is a useful improvement.