fix(sqlite-native): restore kv error hook

[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 👈 (View in Graphite)
• 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
• 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]

PR Review: fix(sqlite-native): restore kv error hook

Overview

Well-scoped and coherent bug fix. The core logic is sound: KV errors were being silently dropped at the VFS boundary, and this PR adds a proper channel to surface them from Rust to JavaScript with a useful, actionable error message. The implementation is generally correct.

---

Issues

wrapNativeStorageError is not annotated as @returns {never}

In wrapper.js, wrapNativeStorageError always throws unconditionally but is not typed or documented as such. At each call site, the code is slightly misleading - the exec path has an implicit undefined return after the catch block. At runtime this is fine (the function always throws), but it is a readability hazard. Consider annotating with JSDoc @returns {never} or restructuring so each call site ends with an explicit throw.

No tests for the new error-surfacing path

No new tests are added for:

• xGetLastError VFS callback returning the correct message.
• wrapNativeStorageError producing the enriched error with the KV error cause.
• clear_last_error correctly resetting state on successful operations.

This is a behavior regression fix, so test coverage here would be especially valuable.

Pre-existing debug console.log statements in wrapper.js

There are several debug logs that appear to predate this PR but are still present and should be cleaned up.

---

Positive Notes

• The last_error mutex accessor methods (clear_last_error, set_last_error, clone_last_error, take_last_error) are consistent and handle mutex-poisoned state with tracing::warn! rather than panicking, which is appropriate for code inside a VFS callback.
• report_kv_error centralizes error handling cleanly: it sets the last error and calls on_error in one place.
• clear_last_error on success correctly prevents stale errors from bleeding across queries.
• Using clone_last_error in xGetLastError (non-consuming, SQLite may call multiple times) vs. take_last_error in the JS-facing path (consuming, JS reads once) is a thoughtful distinction.
• The typeof nativeDb.takeLastKvError === "function" guard in wrapNativeStorageError is a good defensive check.
• The config.notGlobal ?? false fix (from || false) is correct: false || false would have incorrectly coerced an explicit false through the || short-circuit.
• The read_cache.as_ref().and_then(...) and value.as_deref() fixes are correct and appropriate.

---

Security Note

The error message written to the SQLite error buffer originates from the envoy KV error. If the envoy can return attacker-controlled strings in errors, those strings would be exposed to JavaScript callers via takeLastKvError(). Low risk given this is internal infrastructure, but worth being aware of.

---

Performance

The Mutex<Option<String>> for last_error adds negligible overhead per KV operation given the operations already cross async task boundaries via block_on.