fix(rivetkit): only trigger onStateChange and inspector updates for actor state changes (#3393)

Author: NathanFlurry

rivetkit-typescript/packages/rivetkit/src/actor/instance.ts

@@ -53,7 +53,7 @@ import type {
 import { processMessage } from "./protocol/old";
 import { CachedSerializer } from "./protocol/serde";
 import { Schedule } from "./schedule";
-import { DeadlineError, deadline } from "./utils";
+import { DeadlineError, deadline, isConnStatePath, isStatePath } from "./utils";
 
 export const PERSIST_SYMBOL = Symbol("persist");
 
@@ -698,7 +698,6 @@ export class ActorInstance<S, CP, CS, V, I, DB extends AnyDatabaseProvider> {
 		// Set raw persist object
 		this.#persistRaw = target;
 
-		// TODO: Only validate this for conn state
 		// TODO: Allow disabling in production
 		// If this can't be proxied, return raw value
 		if (target === null || typeof target !== "object") {
@@ -732,35 +731,46 @@ export class ActorInstance<S, CP, CS, V, I, DB extends AnyDatabaseProvider> {
 				_previousValue: any,
 				_applyData: any,
 			) => {
-				if (path !== "state" && !path.startsWith("state.")) {
-					return;
+				const actorStatePath = isStatePath(path);
+				const connStatePath = isConnStatePath(path);
+
+				// Validate CBOR serializability for state changes
+				if (actorStatePath || connStatePath) {
+					let invalidPath = "";
+					if (
+						!isCborSerializable(
+							value,
+							(invalidPathPart) => {
+								invalidPath = invalidPathPart;
+							},
+							"",
+						)
+					) {
+						throw new errors.InvalidStateType({
+							path: path + (invalidPath ? `.${invalidPath}` : ""),
+						});
+					}
 				}
 
-				let invalidPath = "";
-				if (
-					!isCborSerializable(
-						value,
-						(invalidPathPart) => {
-							invalidPath = invalidPathPart;
-						},
-						"",
-					)
-				) {
-					throw new errors.InvalidStateType({
-						path: path + (invalidPath ? `.${invalidPath}` : ""),
-					});
-				}
+				this.#rLog.debug({
+					msg: "onChange triggered, setting persistChanged=true",
+					path,
+				});
 				this.#persistChanged = true;
 
-				// Inform the inspector about state changes
-				this.inspector.emitter.emit(
-					"stateUpdated",
-					this.#persist.state,
-				);
+				// Inform the inspector about state changes (only for state path)
+				if (actorStatePath) {
+					this.inspector.emitter.emit(
+						"stateUpdated",
+						this.#persist.state,
+					);
+				}
 
 				// Call onStateChange if it exists
+				//
 				// Skip if we're already inside onStateChange to prevent infinite recursion
 				if (
+					actorStatePath &&
 					this.#config.onStateChange &&
 					this.#ready &&
 					!this.#isInOnStateChange
@@ -806,6 +816,8 @@ export class ActorInstance<S, CP, CS, V, I, DB extends AnyDatabaseProvider> {
 			this.#rLog.info({
 				msg: "actor restoring",
 				connections: persistData.connections.length,
+				hibernatableWebSockets:
+					persistData.hibernatableWebSocket.length,
 			});
 
 			// Set initial state
@@ -1868,6 +1880,13 @@ export class ActorInstance<S, CP, CS, V, I, DB extends AnyDatabaseProvider> {
 	async saveState(opts: SaveStateOptions) {
 		this.#assertReady(opts.allowStoppingState);
 
+		this.#rLog.debug({
+			msg: "saveState called",
+			persistChanged: this.#persistChanged,
+			allowStoppingState: opts.allowStoppingState,
+			immediate: opts.immediate,
+		});
+
 		if (this.#persistChanged) {
 			if (opts.immediate) {
 				// Save immediately
@@ -2075,7 +2094,8 @@ export class ActorInstance<S, CP, CS, V, I, DB extends AnyDatabaseProvider> {
 		for (const connection of this.#connections.values()) {
 			promises.push(connection.disconnect());
 
-			// TODO: Figure out how to abort HTTP requests on shutdown
+			// TODO: Figure out how to abort HTTP requests on shutdown. This
+			// might already be handled by the engine runner tunnel shutdown.
 		}
 
 		// Wait for any background tasks to finish, with timeout

rivetkit-typescript/packages/rivetkit/src/actor/utils.test.ts

@@ -0,0 +1,48 @@
+import { describe, expect, test } from "vitest";
+import { isConnStatePath, isStatePath } from "./utils";
+
+describe("isStatePath", () => {
+	test("matches exact state", () => {
+		expect(isStatePath("state")).toBe(true);
+	});
+
+	test("matches nested state paths", () => {
+		expect(isStatePath("state.foo")).toBe(true);
+		expect(isStatePath("state.foo.bar")).toBe(true);
+	});
+
+	test("does not match other paths", () => {
+		expect(isStatePath("connections")).toBe(false);
+		expect(isStatePath("stateX")).toBe(false);
+		expect(isStatePath("mystate")).toBe(false);
+	});
+});
+
+describe("isConnStatePath", () => {
+	test("matches connection state paths", () => {
+		expect(isConnStatePath("connections.0.state")).toBe(true);
+		expect(isConnStatePath("connections.123.state")).toBe(true);
+	});
+
+	test("matches nested connection state paths", () => {
+		expect(isConnStatePath("connections.0.state.foo")).toBe(true);
+		expect(isConnStatePath("connections.5.state.bar.baz")).toBe(true);
+	});
+
+	test("does not match non-state connection paths", () => {
+		expect(isConnStatePath("connections.0.params")).toBe(false);
+		expect(isConnStatePath("connections.0.token")).toBe(false);
+		expect(isConnStatePath("connections.0")).toBe(false);
+	});
+
+	test("does not match other paths", () => {
+		expect(isConnStatePath("state")).toBe(false);
+		expect(isConnStatePath("connections")).toBe(false);
+		expect(isConnStatePath("other.0.state")).toBe(false);
+	});
+
+	test("does not match malformed paths", () => {
+		expect(isConnStatePath("connections.state")).toBe(false);
+		expect(isConnStatePath("connections.0.stateX")).toBe(false);
+	});
+});

rivetkit-typescript/packages/rivetkit/src/actor/utils.ts

@@ -104,3 +104,26 @@ export function generateRandomString(length = 32) {
 	}
 	return result;
 }
+
+/**
+ * Checks if a path is an actor state path within the persisted actor data.
+ */
+export function isStatePath(path: string): boolean {
+	return path === "state" || path.startsWith("state.");
+}
+
+/**
+ * Checks if a path is a connection state path within the persisted actor data.
+ */
+export function isConnStatePath(path: string): boolean {
+	if (!path.startsWith("connections.")) {
+		return false;
+	}
+	const stateIndex = path.indexOf(".state", 12); // Start after "connections."
+	if (stateIndex === -1) {
+		return false;
+	}
+	const afterState = stateIndex + 6; // ".state".length = 6
+	// Check if ".state" is followed by end of string or "."
+	return path.length === afterState || path[afterState] === ".";
+}