fix: driver test suite & sqlite reliability

[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-4559 environment in rivet-frontend

Service	Status	Web	Updated (UTC)
kitchen-sink	❌ Build Failed (View Logs)	Web	Apr 7, 2026 at 5:08 pm
frontend-cloud	❌ Build Failed (View Logs)	Web	Apr 7, 2026 at 1:35 am
frontend-inspector	❌ Build Failed (View Logs)	Web	Apr 7, 2026 at 1:35 am
mcp-hub	✅ Success (View Logs)	Web	Apr 5, 2026 at 11:57 am
website	❌ Build Failed (View Logs)	Web	Apr 5, 2026 at 11:57 am
ladle	❌ Build Failed (View Logs)	Web	Apr 5, 2026 at 11:57 am

[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

• chore: remove non-engine drivers #4574 : 2 dependent PRs (#4581 , #4582 )
• feat: dynamic actors #4397
• fix: driver test suite & sqlite reliability #4559 👈 (View in Graphite)
• fix: remove serverless token #4572
• fix: misc token fixes #4569
• fix(pb): clean up actor stop decision handling #4566
• fix(envoy): use global instance, add signal handlers #4565
• chore: misc fixes, add pb snapshot test #4561
• fix(cache): make in memory cache global #4560
• main

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

[claude]

PR Review: fix: driver test suite & sqlite reliability

Overall Assessment: Solid multi-fix PR addressing real bugs across the envoy client, KV channel, native SQLite adapter, WebSocket handling, and AgentOS process callbacks. The fixes are well-targeted and the test infrastructure improvements are valuable. A few issues warrant attention before landing.

---

High Priority Issues

Bug: NativeSqliteDatabase.close() error handling is incorrect

The close() method catches an error, calls #recoverFromStaleKvChannel, but then re-checks isStaleKvChannelError on the original error variable -- which was already checked inside the recovery method. If recovery throws, the original error is lost. On close, recovery is also questionable since the database is being torn down anyway. The fix should swallow stale-channel errors without recovery:

async close(): Promise<void> {
    try {
        await this.#module.closeDatabase(this.#nativeDb);
    } catch (error) {
        if (!isStaleKvChannelError(error)) {
            throw error;
        }
        // Stale channel on close is expected; swallow and continue teardown.
    }
}

Bug: open_actors still uses Arc<Mutex<HashSet>>

CLAUDE.md says to prefer scc::HashMap over Arc<Mutex<HashMap<...>>>. The open_actors set in engine/packages/pegboard-kv-channel/src/lib.rs is Arc<Mutex<HashSet<String>>>, locked on every incoming message per connection. This should be replaced with Arc<scc::HashSet<String>>.

Design concern: single-writer lock removal changes the documented concurrency contract

CLAUDE.md states: "The KV channel enforces single-writer locks per actor." The removal of KvChannelState.actor_locks eliminates this guarantee. Either update CLAUDE.md to document where the invariant now lives (e.g., at the SQLite VFS layer) or restore enforcement at a different layer. The current state leaves the documented guarantee and the implementation out of sync.

---

Medium Priority Issues

Duplicate named-parameter extraction with a behavioral difference

extractNamedSqliteParameters and the binding-resolution helpers exist independently in both native-adapter.ts and native-sqlite.ts. The two implementations differ: native-sqlite.ts deduplicates repeated parameter names via a seen Set, while native-adapter.ts does not. SQL like WHERE a = :x AND b = :x will behave differently between the two paths. One implementation should be authoritative and shared.

Fragile string-regex stale-channel detection

The pattern /kv channel (?:connection closed|shut down)/i in native-adapter.ts matches on free-text error messages from the Rust native addon. Any phrasing change on the Rust side silently breaks reconnection. A typed error code or sentinel property on the error object would be more robust.

Misleading fallback warning in open-database.ts

If nativeSqliteAvailable() returns true but ctx.nativeSqliteConfig is falsy (driver forgot to configure it), the warning says "native SQLite not available" when the addon is actually available but unconfigured. The two conditions should produce distinct warning messages.

---

Minor Issues

• INITIAL_SLEEP_TIMEOUT_MS = 250 in actor-driver.ts has no comment explaining why 250 ms is the floor or what wake scenario this guards against.
• The open_actors comment was simplified but now omits context. The set gates KV ops and enforces MAX_ACTORS_PER_CONNECTION, not just cleanup on disconnect.
• Module-level shared state in engine tests (sharedNamespacePromise, etc.) relies on describe.sequential ordering with no explanatory comment.
• destroyNamespaceActors outer loop is serial over actor names. Collecting all actor IDs in parallel first would speed up cleanup.

---

What Is Correct and Well-Done

• Pegboard-envoy copy-paste fix: reading last_ping_ts_key instead of create_ts_key twice is a clear correctness fix.
• Envoy lostTimeout return value fix: the outer variable was never updated when cleared inside handleConnMessage/handleConnClose.
• connHandle.abort() in envoy cleanup: without aborting, the connection coroutine could outlive the envoy loop.
• Conn.disconnect() single-flight fix: wrapping in #disconnectPromise prevents double-disconnect races idiomatically.
• Non-awaited action processing in WebSocket (router-websocket-endpoints.ts): the added comment clearly explains why actions are intentionally not awaited while subscription updates remain serial.
• broadcastProcessEvent catching ActorStopping in agent-os/actor/process.ts: correct pattern for shutdown-safe callbacks.
• openActorDatabase unification: the shared call eliminates a class of mode-specific bugs where raw and Drizzle paths diverged.
• actor_request_task cache change from single cached_actor to HashMap<String, (Id, String)>: correctly handles a connection multiplexing multiple actors.
• Removing the committed .node binary and turbo.json for sqlite-native: correct housekeeping.
• Legacy v2-format value removal in epoxy/src/ops/kv/read_value.rs: safe cleanup.

---

Summary

Priority	Item
High	Fix NativeSqliteDatabase.close() error handling logic
High	Clarify/restore single-writer actor lock guarantee and update CLAUDE.md
High	Replace Arc<Mutex<HashSet>> with Arc<scc::HashSet> in pegboard-kv-channel
Medium	Deduplicate extractNamedSqliteParameters between native-adapter.ts and native-sqlite.ts
Medium	Replace string-regex stale-channel detection with a typed error
Medium	Fix misleading fallback warning in open-database.ts
Low	Document INITIAL_SLEEP_TIMEOUT_MS = 250
Low	Parallelize destroyNamespaceActors outer loop

Generated with Claude Code