donotmerge: rivetkit test

[abcxff]

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

Service	Status	Web	Updated (UTC)
website	😴 Sleeping (View Logs)	Web	May 7, 2026 at 1:37 am
frontend-cloud	❌ Build Failed (View Logs)	Web	May 7, 2026 at 1:29 am
ladle	❌ Build Failed (View Logs)	Web	May 7, 2026 at 1:29 am
frontend-inspector	❌ Build Failed (View Logs)	Web	May 7, 2026 at 1:29 am
mcp-hub	✅ Success (View Logs)	Web	May 7, 2026 at 1:27 am
kitchen-sink	❌ Build Failed (View Logs)	Web	May 7, 2026 at 1:27 am

[abcxff]

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

• donotmerge: rivetkit test #4991 👈 (View in Graphite)
• [slop]fix(rivetkit): surface raw error messages instead of generic #4985
• [slop]fix(rivetkit): remove async drizzle db proxy #4984 : 1 other dependent PR (#4986 )
• fix(rivetkit-napi): demote bridge error decode log from warn to debug #4983
• fix(rivetkit): use keepAwake for websocket callback tracking to prevent c.vars crash after grace deadline #4978 : 1 other dependent PR (#4990 )
• main

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

[claude]

Code Review

Note: This is a draft PR marked donotmerge: rivetkit test — treating this as exploratory tracing/observability work.

Overview

This PR adds structured tracing::trace! instrumentation across the actor/envoy message loops and introduces manual Debug impls for ToActor, ToEnvoyMessage, and DispatchCommand that intentionally redact payload bytes and channel senders. It also re-categorizes some log levels in the NAPI bridge.

---

What's Good

• Exhaustive match arms in all manual Debug impls — correctly follows the project rule of no _ => fallthrough on enums. Adding a new variant will produce a compile error.
• finish_non_exhaustive() used appropriately — payload bytes, response channels, and other non-Debug fields are cleanly hidden while key metadata fields (IDs, names, timestamps) remain visible.
• #[derive(Debug)] on SqliteRequest — straightforward and correct since all its inner types already implement Debug.
• Error level split in anyhow_to_bridge_rivet_error_payload — routing public errors to debug and internal errors to error is the right distinction. Dropping has_metadata is fine since metadata = ?error.metadata() already conveys presence/absence.
• Structured fields — all new logs use structured fields rather than format strings (e.g. ?actor_id not "actor_id: {actor_id}"). Consistent with CLAUDE.md.

---

Issues / Suggestions

1. Redundant per-arm trace logs in envoy_loop

The general tracing::trace!(?msg, "received to envoy message") already fires for every message including its full debug output. The subsequent per-arm traces for SendEvents (?events) and KvRequest (?actor_id, ?data) duplicate what's already in ?msg. Consider removing the per-arm traces, or removing the general one and keeping only targeted per-arm logs so readers can filter by specific message kind.

2. KV data field potentially logs large payloads

tracing::trace!(?actor_id, ?data, "handling kv request") — and ?data also appears via ?msg at the top of the loop. Depending on data's type, this could be large (page data, value blobs). Trace-level is appropriate, but worth confirming the Debug output of data is bounded or summarized rather than dumping raw bytes.

3. keep_awake promise rejection downgrade: warn → debug

Both sites in actor_context.rs now log at debug. If user code in a keep_awake promise throws an unexpected error, it will be invisible in default log configurations. Consider whether there's a distinction to be made (e.g. debug for cancellation/abort, warn for actual unexpected rejections), or at minimum add a comment explaining why debug is appropriate here.

4. "serialize state save tick" label is imprecise

tracing::trace!("serialize state save tick") for the inspector branch reads ambiguously — it sounds like a state-save tick rather than an inspector-serialize tick. Something like "inspector serialize state tick" would be clearer and distinct from the adjacent "state save tick".

5. No actor ID in most trace logs

Most of the new trace calls (e.g. in the ActorTask select loop) don't include actor_id as a field. If the enclosing span carries it this is fine, but if not, filtering these traces by actor in production tooling will be difficult. Worth verifying the span context is always present.

---

Checklist Items Not Addressed (expected for a debug/observability PR)

• No new tests — acceptable here since logging paths aren't easily unit-tested.
• PR description template is unfilled — fine for a draft/test PR.

---

Overall this is clean instrumentation work. The main actionable items are the redundant per-arm traces (#1), the potential large-payload concern for KV data (#2), and the keep_awake log level rationale (#3).