Skip to content

feat(rivetkit): support nested actions - #5459

Merged
NathanFlurry merged 3 commits into
mainfrom
nested-actions
Jul 22, 2026
Merged

feat(rivetkit): support nested actions#5459
NathanFlurry merged 3 commits into
mainfrom
nested-actions

Conversation

@NathanFlurry

Copy link
Copy Markdown
Member
  • Add nested action definitions with dotted low-level action names and typed client access
  • Validate nested action schemas and cover server/client behavior with tests
  • Document nested actions and add a reusable technical code-image renderer

@railway-app
railway-app Bot temporarily deployed to rivet-frontend / rivet-pr-5459 July 22, 2026 06:09 Destroyed
@NathanFlurry
NathanFlurry merged commit 98b956b into main Jul 22, 2026
10 of 18 checks passed
@NathanFlurry
NathanFlurry deleted the nested-actions branch July 22, 2026 06:09
@claude

claude Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Review: feat(rivetkit): support nested actions

Overall the nested-action flattening design (actions.ts) is clean, and the collision/validation logic is well tested. Two correctness issues below, one of which looks like it will break an existing test.

1. High: client action proxy now breaks .bind() / .call() / .apply() on every action, not just nested ones

rivetkit-typescript/packages/rivetkit/src/client/client.ts:523-536

const actionPath = (name: string): ActorActionFunction => {
	...
	method = new Proxy(
		(...args: unknown[]) => handle.action({ name, args }),
		{
			get(target, prop: string | symbol) {
				if (typeof prop === "symbol")
					return Reflect.get(target, prop);
				if (prop === "then") return undefined;
				return actionPath(`${name}.${prop}`);
			},
		},
	) as ActorActionFunction;
	...
};

Previously, a top-level action returned a plain arrow function (method = (...args) => target.action({ name: prop, args })), so Function.prototype members worked normally. Now every action accessor (nested or not) goes through actionPath, whose get trap treats every string property except "then" as another nested action segment. This means someAction.bind, .call, .apply, .name, .length, .toString, etc. no longer return the real function/property — they return a new action-path proxy for e.g. "someAction.bind".

Concretely: connection.getCounts.bind(connection) no longer returns a bound function. .bind resolves to a callable proxy for the (nonexistent) action "getCounts.bind", and the immediate (connection) call invokes it, dispatching handle.action({ name: "getCounts.bind", args: [connection] }) right there and returning a Promise, not a function. This exact pattern already exists in the test suite and will break:

rivetkit-typescript/packages/rivetkit/tests/driver/actor-sleep-db.test.ts:401

await waitForAction(
	connection.getCounts.bind(connection),
	...
);

waitForAction expects action: () => Promise<T> and later does await action() — but action will now be a Promise, not a function, so this throws action is not a function (or similar) instead of polling.

Fix: have the inner proxy's get trap fall back to Reflect.get(target, prop) for properties that already exist on the underlying function (mirroring the prop in target check the outer proxy already does), and only treat unknown string properties as nested action segments.

2. Medium: flattenActionInputSchemas reads schemas[name] without an own-property check, so prototype-named actions can pick up a bogus inherited "schema"

rivetkit-typescript/packages/rivetkit/src/actor/actions.ts:29-34

const nestedSchema = lookupNestedSchema(schemas, path);
const flatSchema = schemas[name];
if (
	nestedSchema !== undefined &&
	flatSchema !== undefined &&
	nestedSchema !== flatSchema
) { ... }

lookupNestedSchema correctly uses Object.hasOwn(value, segment) when walking nested paths, but flatSchema = schemas[name] is a raw bracket access. If an action name collides with an Object.prototype member — toString, constructor, valueOf, hasOwnProperty, __proto__, etc. — and actionInputSchemas is a plain object that doesn't itself define that key, schemas[name] resolves to the inherited value (e.g. Object.prototype.toString, a function) instead of undefined. That gets treated as a real schema (flattened[name] = schema), and would later fail with a confusing error (e.g. schema.parse is not a function) wherever it's consumed, rather than being treated as "no schema provided."

This is notable because the PR does explicitly support and test prototype-colliding action names for flattenActionHandlers ("supports action names that overlap object prototype keys"), so the same care should extend to flattenActionInputSchemas. Suggested fix: const flatSchema = Object.hasOwn(schemas, name) ? schemas[name] : undefined;.

Minor / nit

  • definition.ts:169-170 calls flattenActionHandlers(config.actions) and discards the result right after ActorConfigSchema.parse(input), but zActionTree's superRefine (config.ts) already calls flattenActionHandlers during .parse() and would have thrown a ZodError first on any invalid tree. The extra call in definition.ts is unreachable/redundant (the flattenActionInputSchemas call right after it is still needed, since actionInputSchemas isn't validated by the zod tree).
  • The plain-object detection logic is duplicated between isRecord in actions.ts and the inline .custom() predicate for zActionTree in config.ts. Consider importing isRecord from actions.ts to avoid drift.

Test coverage

Good coverage for the new flattening/collision/type behavior (nested-actions.test.ts, driver test in action-features.test.ts). Missing coverage for the regression in finding #1 — a test exercising .bind()/.call() on a returned action handle would have caught it, and would be a good regression test to add alongside the fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant