Code Review: chore: move ActorDefinition and ActorInstance to interfaces

This is a well-structured architectural refactor. Introducing a minimal ActorInstance interface that both StaticActorInstance and DynamicActorInstance implement is sound and enables the engine to handle dynamic actors alongside static ones without leaking internal implementation details. The approach is largely correct and coherent. A few concerns below.

Architecture and Design

DynamicActorDefinition.instantiate() throws at runtime. The method is required by the ActorDefinition interface contract but deliberately throws:

instantiate(): ActorInstance<...> {
    throw new Error(
        "dynamic actor definitions are instantiated by the runtime driver",
    );
}

This violates the Liskov Substitution Principle. The guards in actor-driver.ts and global-state.ts correctly check isDynamicActorDefinition before calling instantiate(), but nothing at the type level prevents a future code path from calling instantiate() on an AnyActorDefinition and hitting this unexpected throw. Consider either narrowing the return type signature or splitting DynamicActorDefinition away from the ActorDefinition interface entirely.

Three copies of loadStaticActor. The same guard helper appears independently in router-endpoints.ts, router-websocket-endpoints.ts, and router.ts. All three call actorDriver.loadActor, assert the result is a StaticActorInstance, and throw if not. Additionally, error types are inconsistent: router-endpoints.ts throws errors.InternalError (the proper domain error) while the other two throw a plain new Error(...). This should be unified into a shared utility with consistent error handling.

ExtractActorState, ExtractActorConnParams, ExtractActorConnState now hardcode StaticActorInstance. These utility types only return non-never for StaticActorInstance. If the intent is that extraction only makes sense for static actors, the type constraint should be A extends AnyStaticActorInstance rather than the misleading A extends AnyActorInstance.

Potential Bugs

Race condition in #runnerDynamicWebSocket message flush. The flush runs:

proxyToActorWs.addEventListener("open", () => {
    actorWebSocketReady = true;
    void flushPendingMessages();
});

flushPendingMessages is not awaited, and the outer websocket.addEventListener("message", ...) handler also calls runtime.forwardIncomingWebSocketMessage without coordination. If messages arrive while flushPendingMessages is draining the queue (after actorWebSocketReady = true but before clearing all pending entries), a concurrent incoming message will also try to forward immediately. This can cause messages to be delivered out of order.

Silent discard of messages to a stopping actor. In the engine WebSocket handler, messages to a stopping dynamic actor are silently dropped with return. For hibernatable WebSocket paths, the ACK is never sent for the dropped message. If the gateway waits on an ACK before delivering the next message, this will stall the connection without a clean close. It would be safer to close the WebSocket explicitly when discarding due to a stopping actor.

dynamicGetHibernatingWebSocketsEnvelope silently drops connections when symbol lookup fails. The isolate code accesses conn[CONN_STATE_MANAGER_SYMBOL]. If the isolate's require("rivetkit") resolves a different symbol than the one used internally (e.g., due to module caching or version mismatch), the lookup returns undefined with no warning, silently omitting connections from the hibernating snapshot.

Code Quality

(session.websocket as any).triggerMessage(...) casts. The as any casts suggest the VirtualWebSocket type accessible from host-runtime.ts may not yet have the updated triggerMessage signature (with the new rivetMessageIndex parameter). If the shared package type is now properly typed, these casts should be removed.

DynamicActorConfigInput may be missing noSleep. The old dynamicActor implementation set noSleep: true on the host-side placeholder actor. The new DynamicActorConfigInput only exposes GlobalActorOptionsInput, and noSleep lives in InstanceActorOptionsBaseSchema, so it cannot be passed through. Worth confirming this was intentionally dropped rather than accidentally omitted.

Inconsistent indentation in several new files. There are a few spots with misaligned tab depths in mod.ts (around db.createClient), host-runtime.ts (in #sendWebSocketMessage and openWebSocket array arguments), and driver-engine-dynamic.test.ts (the describe callback body). These look like carry-over formatting artifacts.

Security

No new attack surface is introduced. The hibernation metadata serialization (base64 of ArrayBuffer across the isolate boundary) is handled correctly, including the byteOffset fix when slicing from a Node.js Buffer.

Test Coverage

The new driver-engine-dynamic.test.ts covers the main happy paths well (URL-loaded source, action invocation, KV, WebSocket ping/pong, sleep/wake, alarms). A few gaps worth noting:

• No test for the hibernatable WebSocket path (isHibernatable: true / isRestoringHibernatable: true flow).
• No test for the error path when the dynamic runtime fails to start (verifying cleanup from #dynamicRuntimes).
• No test verifying that messages buffered while actorWebSocketReady = false are flushed in order.
• The readWebSocketJson helper has a hardcoded 5-second timeout with no per-test override, which may be fragile on slow CI for the sleep/alarm tests.