Skip to content

fix: isolate sqlcommenter plugins from each other#28861

Merged
aqrln merged 2 commits into
mainfrom
aqrln-knowvqmzvvqx
Dec 19, 2025
Merged

fix: isolate sqlcommenter plugins from each other#28861
aqrln merged 2 commits into
mainfrom
aqrln-knowvqmzvvqx

Conversation

@aqrln

@aqrln aqrln commented Dec 5, 2025

Copy link
Copy Markdown
Member

Make sure one sqcommenter plugin can't poison the context passed to all subsequent ones.

Summary by CodeRabbit

  • Tests

    • Added comprehensive tests validating SQL commenter context isolation and immutability across top-level, deeply nested properties, and arrays for both single and compacted queries.
  • Bug Fixes

    • Ensure each plugin receives an isolated copy of the SQL commenter context so mutations in one plugin do not affect others or shared state.

✏️ Tip: You can customize this high-level summary in your review settings.

@aqrln aqrln added this to the 7.2.0 milestone Dec 5, 2025
@coderabbitai

coderabbitai Bot commented Dec 5, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

Replaces passing the original SqlCommenter context to plugins with a deep clone per plugin using klona, and adds tests verifying that plugin-visible context mutations do not affect other plugins or the original context.

Changes

Cohort / File(s) Summary
SqlCommenter runtime change
packages/client-engine-runtime/src/sql-commenter.ts
Imports klona and updates applySqlCommenters to pass klona(context) (a deep clone) to each plugin; updates JSDoc to reflect cloning behavior.
SqlCommenter tests
packages/client-engine-runtime/src/sql-commenter.test.ts
Adds multiple tests ensuring each plugin receives an isolated copy of the context: top-level, deeply nested, and array mutations in one plugin do not affect other plugins or the original context.
Runtime dependency
packages/client-engine-runtime/package.json
Adds klona (version 2.0.6) to dependencies.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix: isolate sqlcommenter plugins from each other' directly and clearly describes the main change: ensuring SQL commenter plugins are isolated from each other through context cloning to prevent mutation leakage.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch aqrln-knowvqmzvvqx

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Comment thread packages/client-engine-runtime/src/sql-commenter.test.ts Outdated
Comment thread packages/client-engine-runtime/src/sql-commenter.ts Outdated
@github-actions

github-actions Bot commented Dec 5, 2025

Copy link
Copy Markdown
Contributor

size-limit report 📦

Path Size
packages/client/runtime/index-browser.js 2.23 KB (0%)
packages/client/runtime/index-browser.d.ts 3.28 KB (0%)
packages/cli/build/index.js 2.51 MB (0%)
packages/client/prisma-client-0.0.0.tgz 10.24 MB (+0.06% 🔺)
packages/cli/prisma-0.0.0.tgz 6.71 MB (0%)
packages/bundle-size/da-workers-libsql/output.tgz 805.66 KB (+0.04% 🔺)
packages/bundle-size/da-workers-neon/output.tgz 859.72 KB (+0.04% 🔺)
packages/bundle-size/da-workers-pg/output.tgz 852.64 KB (+0.04% 🔺)
packages/bundle-size/da-workers-planetscale/output.tgz 805.22 KB (+0.04% 🔺)
packages/bundle-size/da-workers-d1/output.tgz 788.56 KB (+0.04% 🔺)

Comment thread packages/client-engine-runtime/src/sql-commenter.ts Outdated
aqrln added 2 commits December 9, 2025 12:43
Make sure one sqcommenter plugin can't poison the context passed to all
subsequent ones.
@aqrln aqrln force-pushed the aqrln-knowvqmzvvqx branch from 006e8e8 to 58842d2 Compare December 9, 2025 12:01
@aqrln

aqrln commented Dec 9, 2025

Copy link
Copy Markdown
Member Author

@jacek-prisma I changed the code to clone the context using klona instead of freezing it. This seems simpler and safer, handles all edge cases, and addresses your and coderabbit's comments.

@aqrln aqrln merged commit d7a32bd into main Dec 19, 2025
255 checks passed
@aqrln aqrln deleted the aqrln-knowvqmzvvqx branch December 19, 2025 12:18
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.

2 participants