[evgenovalov]

Problem

Inside db.$transaction(async (tx) => ...), a client derived from tx runs in the transaction (since #2497), but its queries still go through the transaction client's Kysely-level machinery:

• tx.$unuse('policy') and tx.$unuseAll() still enforce access policies, and still run every other onKyselyQuery / onEntityMutation hook of tx.
• tx.$use(plugin) never runs the new plugin's onKyselyQuery / onEntityMutation hooks.
• tx.$setAuth(user) evaluates policies with tx's auth instead of user. This applies both ways: switching to a narrower user inside a transaction keeps the wider user's access.
• Options changed with $setOptions are not applied by the executor (e.g. diagnostics, useCompactAliasNames).

ORM-level hooks (onQuery, onProcedure), plugin client members and ext results already worked, because they are read from the derived client itself.

Cause

#2497 made the ClientImpl constructor reuse baseClient.$qb when the base client is a transaction (client-impl.ts:159-165). That Kysely Transaction runs on the ZenStackQueryExecutor that was built for the original transaction client. The executor reads onKyselyQuery hooks (zenstack-query-executor.ts:209), entity mutation hooks (:358, :399) and options from the client it is bound to, and passes that client, with its $auth, to the hooks (:220, :273). The executor that the constructor builds for the derived client was never used.

The #2497 tests only check rollback, and use $unuse('nonexistent'), so they could not catch this.

Fix

A client derived from a transaction client keeps its own ZenStackQueryExecutor, bound to itself, and is wrapped in a Kysely Transaction. So isTransaction stays true, and nested $transaction or internal safeTransaction calls do not open a new transaction.

The executor gets the transaction's connection from a small TransactionConnectionProvider, created by $transaction and held by its transaction client:

• It hands out the transaction's connection (through Kysely's transaction executor, i.e. its SingleConnectionProvider).
• Once the $transaction callback has settled, it throws Transaction is already committed / Transaction is already rolled back, like Kysely's own transaction executor. It records the outcome before Kysely commits and releases the connection, and switches to "rolled back" if the commit fails (Kysely then rolls back). So a derived client kept past the transaction fails loudly instead of running on a connection that is back in the pool.

Clients derived from a derived client reach the same provider through their base's executor. This covers $use, $unuse, $unuseAll, $setAuth, $setOptions and $setInputValidation, chains of them, and nested or sequential $transaction calls on a derived client. withExecutor and the clients built for hooks by createClientForConnection are unchanged.

Alternatives considered:

• Keep sharing baseClient.$qb and look up the issuing client per query: the executor is shared, and Kysely has no per-query context to tell it which client issued a query.
• Reuse Kysely's own guard: NotCommittedOrRolledBackAssertingExecutor is internal and not exported. Its withConnectionProvider / withPlugin keep the state, but they delegate to ZenStackQueryExecutor methods that bind the new executor to a clone of the original client.
• Switch $transaction to startTransaction() to read ControlledTransaction.isCommitted / isRolledBack: this rewrites how transactions are started and ended for a state flag.
• Check the driver's per-connection transaction flag: a pooled connection can be inside another request's transaction by then.
• Go back to a fresh pool connection (before fix: preserve transaction state in $use, $unuse, and $unuseAll #2497): the derived client leaves the transaction again.

Tests

tests/e2e/orm/client-api/transaction.test.ts, next to the #2497 tests:

• tx.$unuse('policy') creates a row that the policy rejects. The row is visible inside the transaction and rolled back with it, and tx itself still rejects the same create.
• tx.$setAuth(u2) creates u2's row and is rejected for u1's row, while tx (auth u1) can still create u1's row.
• tx.$use(plugin) runs the plugin's onKyselyQuery and onEntityMutation hooks, and tx itself does not.
• A derived client, and one derived from it, throws Transaction is already committed after a commit and Transaction is already rolled back after a rollback.

The first three fail on dev. The last one passes on dev, and fails without the provider's check: the derived client then reads from the released connection.

Results:

• transaction.test.ts passes on SQLite, PostgreSQL and MySQL 8.4.
• tests/regression, packages/server and packages/plugins/soft-delete pass on SQLite and PostgreSQL.
• tests/e2e passes on SQLite and PostgreSQL apart from failures unrelated to this change: orm/client-api/timezone/mysql-timezone.test.ts needs a MySQL server even on the SQLite and PostgreSQL runs, and failed when none was running.
• packages/auth-adapters/better-auth: test/cli-generate.test.ts fails the same way on dev, because the better-auth CLI is not installed locally.
• packages/server on PostgreSQL: the beforeAll of test/openapi/*-openapi.test.ts sometimes fails with database ... is being accessed by other users. It reads a stale currentTestName, so consecutive suites drop the database the previous suite's still-open client uses. The file passes on re-run, on this branch and on dev.

🤖 Generated with Claude Code

https://claude.ai/code/session_01RwcqsRcacQHX18hXKbJN4n

Summary by CodeRabbit

• Bug Fixes
  • Transaction-derived clients now consistently use the active transaction, including when created from another derived client.
  • Attempts to use a transaction-derived client after the transaction commits or rolls back are rejected.
  • Interactive transactions now report callback and commit failures correctly.
• New Features
  • Derived transaction clients support policy configuration, updated authentication values, and query or entity-mutation hooks.

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: zenstackhq/zenstack
• Branch: dev
• File: .coderabbit.yaml
• Commit: d44e489
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: a0a82381-7ea6-4234-a89e-38530b05e0ad

Commits

Reviewing files that changed from the base of the PR and between d44e489 and be8ab11.

Files selected for processing: 2

• packages/orm/src/client/client-impl.ts
• tests/e2e/orm/client-api/transaction.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

Walkthrough

Interactive transactions now use a transaction-scoped connection provider. Derived clients reuse that provider, construct their own Transaction executor, and reject queries after transaction completion. Tests cover derived-client policy and auth changes, plugin hooks, and query rejection after commit or rollback.

Changes: Interactive transaction-derived clients

Layer / File(s)	Summary
Transaction-scoped connection lifecycle packages/orm/src/client/client-impl.ts	A connection provider captures the transaction executor during the callback, tracks completion, and rejects connection requests before the transaction starts or after it ends. Interactive transactions run through the provider.
Derived-client execution and behavior packages/orm/src/client/client-impl.ts, tests/e2e/orm/client-api/transaction.test.ts	Derived transaction clients use the base client’s provider and construct their own Transaction executor. Tests cover policy and auth behavior, plugin hooks, and query rejection after commit or rollback.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

Possibly related PRs

• zenstackhq/zenstack#2895: Changes the entity-mutation hook contract and selection. This PR tests mutation hooks installed on a derived transaction client.

Suggested reviewers: ymc9

Merge Risk: Minimal · up to be8ab

No actionable merge-blocking issue remains after normal checks.

Pre-merge checks: 5

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly describes the main change: derived clients from transaction clients now use their own plugins and authentication context.
Docstring Coverage		No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 2 files.

Finishing Touches

Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

• X
• Mastodon
• Reddit
• LinkedIn