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

  • Ensures cloned/derived clients preserve the active transaction context so transactional operations behave correctly.

• Tests

  • Added end-to-end tests verifying transaction isolation and rollback when using client helper methods inside transactions.

[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: 527583d7-ee36-44b6-b6cd-ec2ffdfb8b80

📥 Commits

Reviewing files that changed from the base of the PR and between abeac6f and 0a46d48.

📒 Files selected for processing (1)

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

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

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

---

📝 Walkthrough

Walkthrough

Client cloning now preserves active transaction context by reusing the base client's Kysely transaction instance when creating new ClientImpl clones. Five end-to-end tests were added to verify transactional helper APIs maintain isolation and rollback behavior inside a transaction.

Changes

Cohort / File(s)	Summary
Client transaction propagation packages/orm/src/client/client-impl.ts	ClientImpl constructor updated to conditionally reuse the transaction-bound Kysely instance from baseClient.$qb when cloning inside a transaction, preserving transaction context. Minor TypeScript signature formatting change in prepareArgsForExtResult.
E2E transaction tests tests/e2e/orm/client-api/transaction.test.ts	Added five end-to-end tests exercising transactional helper APIs ($unuseAll, $unuse('nonexistent'), $use(...), $setAuth(undefined), $setOptions(...)) inside client.$transaction with forced rollback, verifying rollback and transaction isolation are preserved.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐇 I hopped into the transaction den,
Found clones that slipped outside again.
I tied the thread of Kysely tight,
So every query stays in flight.
🥕✨ Rollbacks safe beneath moonlight.

🚥 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 describes the main fix: preserving transaction state in helper methods like $use, $unuse, and $unuseAll, which is the core change in this PR.
Linked Issues check	✅ Passed	The PR implementation aligns with #2494 requirements: it preserves transaction state in derived clients by reusing the transaction-bound Kysely instance, and includes E2E tests verifying the fix works correctly.
Out of Scope Changes check	✅ Passed	All changes are scoped to fixing transaction preservation in ClientImpl and adding corresponding E2E tests; no unrelated modifications are present.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.}

[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

[docloulou]

This fix #2493 ?
´´´
client.$setAuth(...) returns a new ZenStackClient instance that re-triggers entity mutation hooks, causing infinite recursion when the hook performs mutations on the same model it listens to.
´´´