fix(envoy-client): fail sent vfs sqlite requests on disconnect instead of stalling until expiry

[MasterPtato]

No description provided.

[MasterPtato]

Stack for rivet-dev/actors

• #5685 fix(universaldb): stop postgres commits from starving the co …
• #5670 fix(rivetkit): surface native runtime load failures instead …
• #5667 fix(envoy-client): fail sent vfs sqlite requests on disconne … 👈
• #5666 fix(rivetkit-core): report actor crashes as a sleep intent i …
• #5668 fix(envoy-client): ack start commands immediately instead of …
• main

Get stack: forklift get 5667
Push local edits: forklift submit
Merge when ready: forklift merge 5667

_{change snwrsykw}

[claude]

Review

Small, targeted fix: VFS SQLite requests now get failed with an indeterminate-result error on ConnClose, mirroring the existing remote-exec/execute path, instead of stalling for the full KV_EXPIRE_MS cleanup window. The approach is sound and closely matches the established pattern (fail_sent_remote_sqlite_requests_with_indeterminate_result).

Finding: stale per-test doc comment contradicts the rewritten test body (engine/sdks/rust/envoy-client/tests/sqlite_vfs_disconnect_stall.rs:191-204)

The module-level doc comment at the top of the file was correctly rewritten to describe the fix, but the function-level doc comment directly above sent_vfs_request_fails_immediately_on_disconnect (lines 191-204) still describes the old, bug-present behavior:

'5. Assert: the remote-execute oneshot resolves immediately... The VFS get_pages oneshot is STILL pending.'
'6. Verify that ONLY the cleanup path can release the VFS request... Now the VFS oneshot resolves with the sqlite request timed out error.'

This directly contradicts the actual test body below it (lines 284-315), which now asserts the VFS oneshot resolves immediately with RemoteSqliteIndeterminateResultError, and that nothing is left for the cleanup tick to reap. Worth updating this comment block in the same change so it doesn't mislead the next reader into thinking the fix isn't actually working.

Minor / non-blocking

RemoteSqliteIndeterminateResultError's Display impl hardcodes 'remote sqlite {operation} result is indeterminate after envoy disconnect' (utils.rs:41-49), but it is now also raised for local VFS requests (get_pages, commit, etc.) via the new function. Logs/errors for a lost local VFS commit will read 'remote sqlite commit result is indeterminate...', which could be confusing when triaging since these are not 'remote' SQL requests. Not a functional issue since nothing appears to pattern-match on the message text, just a naming mismatch worth a follow-up if it causes confusion during incident debugging.

Other notes

Metrics/inflight-gauge bookkeeping (sqlite_requests_inflight.dec()) is symmetric with the increment site and matches the existing remote-path convention. It does not increment a dedicated disconnect-failure counter; only the timeout-cleanup path does that, which is consistent with existing behavior and not a new issue. The negative-control test for unsent requests is unaffected since the new function filters on sent, so it correctly does not need to call the new function. The design tradeoff around ambiguous commit results being adjudicated later by the head-fence mechanism, rather than resolved here, is reasonable and well documented in the module comment. No security or trust-boundary concerns: this is actor-local, already-trusted-peer plumbing per the engine's trust model.

🤖 Generated with Claude Code