fix: preserve transaction state in $use, $unuse, and $unuseAll

[pkudinov]

Fixes #2494

Problem

Calling $use(), $unuse(), or $unuseAll() on a transaction client returns a new client that runs queries outside the active transaction.

The constructor always creates a fresh Kysely instance from kyselyProps (this.kysely = new Kysely(this.kyselyProps)), but the transaction state lives in txClient.kysely which was set to the Kysely Transaction object after construction. The new client never receives this.

Fix

After constructing the new client in $use(), $unuse(), and $unuseAll(), propagate the transaction Kysely instance:

if (this.kysely.isTransaction) {
    newClient.kysely = this.kysely;
}

Tests

Three new tests in tests/e2e/orm/client-api/transaction.test.ts verify that $unuseAll(), $unuse(), and $use() on a transaction client stay in the same transaction by checking that writes roll back when the transaction fails.

Summary by CodeRabbit

• Bug Fixes

  • Fixed preservation of active transaction context when creating cloned client instances.

• Tests

  • Added end-to-end tests confirming transaction isolation and rollback behavior when using client cloning helper methods.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 03883985-44ad-45ba-8437-387aef995fd7

📥 Commits

Reviewing files that changed from the base of the PR and between 03736ed and ce30ea2.

📒 Files selected for processing (2)

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

✅ Files skipped from review due to trivial changes (1)

• packages/orm/src/client/client-impl.ts

🚧 Files skipped from review as they are similar to previous changes (1)

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

---

📝 Walkthrough

Walkthrough

Client cloning methods ($use, $unuse, $unuseAll) now propagate the active Kysely transaction instance to newly created ClientImpl instances when this.kysely.isTransaction is true. Three end-to-end tests were added to verify transaction isolation and rollback behavior when these helpers are used inside transactions.

Changes

Cohort / File(s)	Summary
Client transaction propagation packages/orm/src/client/client-impl.ts	When cloning a client via $use(), $unuse(), or $unuseAll(), the new ClientImpl now receives this.kysely if the current client is inside a transaction (this.kysely.isTransaction), preserving transaction context.
E2E transaction tests tests/e2e/orm/client-api/transaction.test.ts	Added three end-to-end tests exercising $use(), $unuse(), and $unuseAll() inside a transaction: each performs a write, forces a rollback by throwing, asserts the transaction rejects, and verifies no data was persisted.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐇 I hopped through code and found a seam,
Where clones would wander out of theme.
I tucked the Kysely thread right in,
So transactions keep their careful spin.
♻️🥕 Hops for safe rollbacks and peaceful checks.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately and concisely describes the main fix: preserving transaction state in three specific helper methods ($use, $unuse, $unuseAll).
Linked Issues check	✅ Passed	The PR fully implements the suggested fix from issue #2494: it adds transaction state preservation to $use, $unuse, and $unuseAll, and includes comprehensive E2E tests verifying the fix works correctly.
Out of Scope Changes check	✅ Passed	All changes are directly related to fixing the transaction isolation issue in #2494; no out-of-scope modifications detected.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

📝 Coding Plan

• Generate coding plan for human review comments

---

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

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Tip

CodeRabbit can approve the review once all CodeRabbit's comments are resolved.

Enable the reviews.request_changes_workflow setting to automatically approve the review once all CodeRabbit's comments are resolved.

[coderabbitai]

🧹 Nitpick comments (1)

tests/e2e/orm/client-api/transaction.test.ts (1)

136-148: Consider using a valid plugin structure.

The plugin object uses handle, which isn't a recognized plugin hook based on the ClientImpl implementation (which looks for onQuery, onProcedure, client, result). While this works as a no-op plugin (unrecognized properties are ignored), using the correct interface would make the test more representative:

🔧 Suggested improvement

         it('$use preserves transaction isolation', async () => {
             await expect(
                 client.$transaction(async (tx) => {
-                    await (tx as any).$use({ id: 'noop', handle: (_node: any, proceed: any) => proceed(_node) }).user.create({
+                    await (tx as any).$use({ id: 'noop' }).user.create({
                         data: { email: 'u1@test.com' },
                     });

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/e2e/orm/client-api/transaction.test.ts` around lines 136 - 148, The
test uses client.$use with a plugin object that defines a nonstandard "handle"
property; update the plugin to use a valid plugin hook (e.g., onQuery or
onProcedure) so it matches the ClientImpl plugin shape and remains a
no-op—replace the object passed to (tx as any).$use (id: 'noop', handle: ...)
with one that provides a recognized hook like onQuery/onProcedure that simply
calls proceed, keeping the id 'noop' and the overall behavior under
client.$transaction intact.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@tests/e2e/orm/client-api/transaction.test.ts`:
- Around line 136-148: The test uses client.$use with a plugin object that
defines a nonstandard "handle" property; update the plugin to use a valid plugin
hook (e.g., onQuery or onProcedure) so it matches the ClientImpl plugin shape
and remains a no-op—replace the object passed to (tx as any).$use (id: 'noop',
handle: ...) with one that provides a recognized hook like onQuery/onProcedure
that simply calls proceed, keeping the id 'noop' and the overall behavior under
client.$transaction intact.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 4a081a18-ec8d-4f80-8906-472b677ced06

📥 Commits

Reviewing files that changed from the base of the PR and between 00768de and 03736ed.

📒 Files selected for processing (2)

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