fix(sqlite-native): gate native read cache behind env var

[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]

• chore(engine): publish engine bases in ci #4649 : 3 dependent PRs (#4650 , #4652 , #4666 )
• chore: lockfile lefthook #4647
• fix(rivetkit): surface native sqlite kv errors to callers #4645
• fix(rivetkit): prevent sleep races during disconnect and db work #4644
• fix(rivetkit): isolate engine envoys and propagate startup failures #4643
• test(rivetkit): remove wasm sqlite-only fixtures and coverage #4642
• chore(sqlite-vfs): remove wasm sqlite packages and workspace wiring #4641
• chore(rivetkit): remove wasm sqlite runtime #4614
• fix(sqlite-native): restore native startup kv preload #4639
• fix(sqlite-native): delete metadata before chunk range #4638
• fix(sqlite-native): keep truncate cache coherent #4637
• fix(sqlite-vfs): use delete range for truncate cleanup #4636
• fix(sqlite-native): restore kv error hook #4635
• perf(sqlite-native): avoid cloning cached read chunks #4634
• perf(sqlite-native): gate kv operation labels #4633
• perf(sqlite-native): reuse read cache for partial writes #4632
• perf(sqlite-native): remove delete file existence probe #4631
• fix(sqlite-native): gate native read cache behind env var #4630 👈 (View in Graphite)
• fix(pegboard): isolate runner config dc lookup failures #4625 : 1 other dependent PR (#4626 )
• docs(pegboard): note runner config upsert split write risk #4624
• fix(pegboard): unpack global runner config keys correctly #4623
• fix(pegboard-gateway): enforce tunnel message state #4622
• fix(pegboard-runner): clear terminal tunnel routes #4621
• fix(pegboard): skip protocol version keys in runner pool backfill #4620
• fix(pegboard): create missing normal runner configs #4619
• fix(epoxy): reuse cached optimistic value #4618
• main

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

[claude]

Review: fix(sqlite-native): gate native read cache behind env var

Overview

This PR adds an opt-in environment variable (RIVETKIT_SQLITE_NATIVE_READ_CACHE) to enable the native SQLite VFS read cache, which is disabled by default. The motivation is correct: the WASM VFS intentionally has no read cache (it relies solely on SQLite's pager cache), and the native VFS should match that behavior by default to avoid divergent performance characteristics between the two backends.

Correctness

The critical path is safe. In SQLITE_FCNTL_COMMIT_ATOMIC_WRITE, the KV write (ctx.kv_put) happens before the dirty buffer is moved into the read cache. When the cache is disabled, the KV write still completes and state.dirty_buffer is correctly cleared via std::mem::take. No data loss.

Issues

Minor: unnecessary allocation when cache is disabled

In the commit handler (vfs.rs:920):

let flushed: Vec<_> = std::mem::take(&mut state.dirty_buffer).into_iter().collect();
if let Some(read_cache) = state.read_cache.as_mut() {
    for (chunk_index, data) in flushed { ... }
}

When read_cache is None, flushed is allocated and immediately dropped. Consider moving the allocation inside the if let and clearing directly in the else branch:

if let Some(read_cache) = state.read_cache.as_mut() {
    for (chunk_index, data) in std::mem::take(&mut state.dirty_buffer) {
        let key = kv::get_chunk_key(file.file_tag, chunk_index);
        read_cache.insert(key.to_vec(), data);
    }
} else {
    state.dirty_buffer.clear();
}

This avoids a gratuitous heap allocation on every commit when the cache is disabled (the common production path).

Minor: comment style doesn't match conventions

From CLAUDE.md: "Write comments as normal, complete sentences."

/// Opt-in flag for the native read cache. Disabled by default to match the WASM VFS.
const READ_CACHE_ENV_VAR: &str = "RIVETKIT_SQLITE_NATIVE_READ_CACHE";

"Opt-in flag for the native read cache" is a sentence fragment. Suggested: "Name of the environment variable that enables the native read cache. Disabled by default to match the WASM VFS."

Minor: redundant field

VfsContext.read_cache_enabled: bool is populated from read_cache_enabled() which already uses a OnceLock<bool> for one-time initialization. The field is only used once in kv_vfs_open to pass to KvFileState::new. Calling read_cache_enabled() directly in kv_vfs_open would be equally cheap and remove the field. Minor point — the current approach is clear and explicit.

Positive Notes

• OnceLock<bool> with a function-local static is the correct Rust idiom for lazy once-initialized globals. Clean.
• read_cache_enabled.then(HashMap::new) is idiomatic and concise.
• Accepting "1" | "true" | "yes" | "on" (case-insensitive) is user-friendly and consistent with common Unix tooling conventions.
• The comment cleanup throughout the diff follows project conventions (no em dashes, clear prose).
• The VfsContext field approach correctly captures the env var at VFS construction time, so it is read exactly once per process.

PR Description

The checklist is empty. Please fill in the "How Has This Been Tested?" section — even noting that this was validated by running with the env var set and exercising SQLite reads/writes would be helpful for reviewers.