Skip to content

chore: bump Kysely to 0.29#2626

Draft
ymc9 wants to merge 3 commits intodevfrom
fix/better-auth-type-compat
Draft

chore: bump Kysely to 0.29#2626
ymc9 wants to merge 3 commits intodevfrom
fix/better-auth-type-compat

Conversation

@ymc9
Copy link
Copy Markdown
Member

@ymc9 ymc9 commented Apr 28, 2026

Summary

  • @better-auth/core gets two separate pnpm store instances because @better-auth/cli pulls in @better-auth/telemetry, which creates a different peer dependency resolution context
  • Importing BetterAuthOptions and Where directly from @better-auth/core in adapter.ts resolved to the wrong instance — incompatible with the types used internally by better-auth/adapters's createAdapterFactory
  • Fix by importing those types from better-auth instead, which uses the same @better-auth/core instance as createAdapterFactory
  • Also adds a pnpm override to pin @better-auth/core to 1.4.19 to prevent future drift

Test plan

  • pnpm --filter @zenstackhq/better-auth build completes without TypeScript errors
  • Run @zenstackhq/better-auth tests: pnpm --filter @zenstackhq/better-auth test

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Chores

    • Pinned @better-auth/core to 1.4.19 and retained the existing lodash override to ensure consistent installs.
    • Updated workspace dependency pin for Kysely to 0.29.0-rc.0 to standardize resolved versions across the workspace.
  • Refactor

    • Minor internal type annotation cleanup with no change to public APIs.
  • Bug Fixes

    • Improved ORM traversal to support additional schema operation types for better compatibility without API changes.

…-auth/core instances

pnpm creates two instances of @better-auth/core due to different peer
dependency contexts (@better-auth/cli pulls in @better-auth/telemetry which
creates a separate resolution). Importing BetterAuthOptions and Where
directly from @better-auth/core resolved to the wrong instance, making
those types incompatible with the ones used internally by better-auth/adapters.

Fix by importing BetterAuthOptions and Where from better-auth instead,
which resolves to the same instance that createAdapterFactory uses.
Also pin @better-auth/core via pnpm override to prevent further drift.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 28, 2026

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: d35dcd52-07ce-41eb-96e6-79212f5fa9fc

📥 Commits

Reviewing files that changed from the base of the PR and between 3944687 and 55beba0.

📒 Files selected for processing (1)
  • packages/orm/src/utils/kysely-utils.ts

📝 Walkthrough

Walkthrough

Updated pnpm overrides to pin @better-auth/core@1.4.19 while preserving the existing lodash override, changed workspace Kysely pin to 0.29.0-rc.0, adjusted type imports and removed an explicit return type in the better-auth adapter, and added three Kysely AST visitor methods to the ORM utils.

Changes

Cohort / File(s) Summary
Dependency overrides
package.json
Added a pnpm override pinning @better-auth/core to 1.4.19; retained the existing lodash override rule.
Workspace manifest
pnpm-workspace.yaml
Changed workspace Kysely entry from ~0.28.16 to 0.29.0-rc.0.
Auth adapter
packages/auth-adapters/better-auth/src/adapter.ts
Switched type imports from @better-auth/core to better-auth and removed the explicit DBAdapter<BetterAuthOptions> return type on the adapter factory (relying on TypeScript inference).
ORM Kysely utils
packages/orm/src/utils/kysely-utils.ts
Added AlterTypeNode, AddValueNode, and RenameValueNode imports and corresponding visitAlterType, visitAddValue, and visitRenameValue methods to DefaultOperationNodeVisitor that call defaultVisit.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I nibble pins and tidy imports tonight,
I hop through versions till the locks are tight,
New visitors learn how ASTs behave,
Types gently inferred — no need to be brave,
A fluffy commit, and the build feels light.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Title check ⚠️ Warning The title 'chore: bump Kysely to 0.29' is overly narrow and misleading. While the PR does bump Kysely, the primary changes involve fixing @better-auth type compatibility issues with imports and pnpm overrides. The Kysely bump is secondary. Revise the title to reflect the main objective: 'fix: resolve @better-auth type compatibility issues' or 'chore: fix @better-auth imports and add dependency overrides' to better represent the changeset's primary purpose.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/better-auth-type-compat

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

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

@ymc9 ymc9 changed the title fix(better-auth): resolve type incompatibility from duplicate @better-auth/core instances chore: bump Kysely to 0.29 Apr 28, 2026
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Apr 28, 2026

Claude finished @ymc9's task —— View job


PR Review: Better Auth Type Compatibility Fix

Code Review Analysis

This PR addresses a legitimate dependency resolution issue with Better Auth that causes type incompatibilities. Here's my comprehensive review:

Code Quality & Best Practices

Excellent Solution: The fix correctly addresses the root cause by importing types from the unified better-auth package rather than directly from @better-auth/core. This ensures type consistency with the createAdapterFactory function.

Clean Changes:

  • Line 1 in adapter.ts: Consolidated imports from better-auth instead of separate packages
  • Removed unused DBAdapter import that was causing type conflicts
  • Simplified return type annotation on line 215 (removing explicit DBAdapter constraint)

Dependency Management

Smart Prevention: The pnpm override in package.json ("@better-auth/core": "1.4.19") prevents future version drift that could reintroduce this issue. This is a proactive approach to dependency management.

Consistent Versioning: The override aligns with the devDependency versions already specified in the Better Auth package.

Bug Fix Assessment

Root Cause Correctly Identified: The issue stems from @better-auth/cli pulling in @better-auth/telemetry, which creates a separate peer dependency resolution context for @better-auth/core. This results in two different instances with incompatible types.

Appropriate Solution: Using the unified better-auth package ensures the same @better-auth/core instance is used throughout the dependency tree.

Security Considerations

No Security Issues: The changes only modify import sources and don't introduce any security vulnerabilities. The pnpm override actually improves security by ensuring consistent dependency versions.

⚠️ Performance Considerations

Minimal Impact: The import consolidation has negligible performance impact. The pnpm override may slightly increase bundle size if it prevents deduplication, but this is necessary for type safety.

⚠️ Test Coverage Assessment

Test Plan Provided: The PR description includes appropriate testing steps:

  • Build verification for the package
  • Running the package test suite

Recommendation: Consider adding a regression test to detect future Better Auth version conflicts, possibly in the CI pipeline.

📋 Minor Suggestions

  1. Documentation: Consider adding a comment explaining the import strategy to prevent future refactoring back to @better-auth/core
  2. Monitoring: Set up alerts for Better Auth version updates to review the pnpm override necessity

🎯 Overall Assessment

This is a well-executed fix that:

  • ✅ Addresses the root cause correctly
  • ✅ Implements appropriate preventive measures
  • ✅ Maintains code quality and type safety
  • ✅ Uses industry best practices for dependency management

Recommendation: Approve - This fix properly resolves the type compatibility issue with a clean implementation and appropriate safeguards.

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Apr 28, 2026

Claude finished @ymc9's task —— View job


PR Review: Better Auth Type Compatibility Fix

Todo List

  • Analyze changed files and understand the type compatibility issue
  • Review code quality and best practices
  • Check for potential bugs or security concerns
  • Assess performance implications
  • Evaluate test coverage approach
  • Provide comprehensive review feedback

Code Review Analysis

This PR addresses a legitimate dependency resolution issue with Better Auth that causes type incompatibilities. Here's my comprehensive review:

Code Quality & Best Practices

Excellent Solution: The fix correctly addresses the root cause by importing types from the unified better-auth package rather than directly from @better-auth/core. This ensures type consistency with the createAdapterFactory function.

Clean Changes:

  • packages/auth-adapters/better-auth/src/adapter.ts:1 - Consolidated imports from better-auth instead of separate packages
  • Removed unused DBAdapter import that was causing type conflicts
  • Simplified return type annotation on line 215 (removing explicit DBAdapter constraint)

Dependency Management

Smart Prevention: The pnpm override in package.json:67 ("@better-auth/core": "1.4.19") prevents future version drift that could reintroduce this issue. This is a proactive approach to dependency management.

Consistent Versioning: The override aligns with the devDependency versions already specified in the Better Auth package.

Bug Fix Assessment

Root Cause Correctly Identified: The issue stems from @better-auth/cli pulling in @better-auth/telemetry, which creates a separate peer dependency resolution context for @better-auth/core. This results in two different instances with incompatible types.

Appropriate Solution: Using the unified better-auth package ensures the same @better-auth/core instance is used throughout the dependency tree.

Security Considerations

No Security Issues: The changes only modify import sources and don't introduce any security vulnerabilities. The pnpm override actually improves security by ensuring consistent dependency versions.

⚠️ Performance Considerations

Minimal Impact: The import consolidation has negligible performance impact. The pnpm override may slightly increase bundle size if it prevents deduplication, but this is necessary for type safety.

⚠️ Test Coverage Assessment

Test Plan Provided: The PR description includes appropriate testing steps:

  • Build verification for the package
  • Running the package test suite

Recommendation: Consider adding a regression test to detect future Better Auth version conflicts, possibly in the CI pipeline.

📋 Minor Suggestions

  1. Documentation: Consider adding a comment explaining the import strategy to prevent future refactoring back to @better-auth/core
  2. Monitoring: Set up alerts for Better Auth version updates to review the pnpm override necessity

🎯 Overall Assessment

This is a well-executed fix that:

  • ✅ Addresses the root cause correctly
  • ✅ Implements appropriate preventive measures
  • ✅ Maintains code quality and type safety
  • ✅ Uses industry best practices for dependency management

Recommendation: Approve - This fix properly resolves the type compatibility issue with a clean implementation and appropriate safeguards.


Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
package.json (1)

17-17: ⚠️ Potential issue | 🟡 Minor

Pin packageManager to the repo-required pnpm version.

Line 17 uses pnpm@10.33.0, but this repo policy requires pnpm@10.12.1.

As per coding guidelines "Use pnpm for package management with version pinned to pnpm@10.12.1."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@package.json` at line 17, The packageManager field is pinned to pnpm@10.33.0
but repo policy requires pnpm@10.12.1; update the packageManager value (the
"packageManager" JSON key in package.json) to "pnpm@10.12.1" so the repository
uses the required pinned pnpm version.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@package.json`:
- Line 17: The packageManager field is pinned to pnpm@10.33.0 but repo policy
requires pnpm@10.12.1; update the packageManager value (the "packageManager"
JSON key in package.json) to "pnpm@10.12.1" so the repository uses the required
pinned pnpm version.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 69d5e636-ec09-45fa-b68f-9f8bb1a8b7a1

📥 Commits

Reviewing files that changed from the base of the PR and between d30ebad and 6d425fd.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (2)
  • package.json
  • packages/auth-adapters/better-auth/src/adapter.ts

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Apr 28, 2026

Claude finished @ymc9's task —— View job


PR Review: Better Auth Type Compatibility Fix

Todo List

  • Read and analyze all changed files
  • Review code quality and best practices
  • Check for potential bugs or security concerns
  • Assess performance implications
  • Evaluate test coverage approach
  • Provide comprehensive review feedback

Code Review Analysis

This PR addresses a legitimate dependency resolution issue with Better Auth that causes type incompatibilities. Here's my comprehensive review:

Code Quality & Best Practices

Excellent Solution: The fix correctly addresses the root cause by importing types from the unified better-auth package rather than directly from @better-auth/core. This ensures type consistency with the createAdapterFactory function.

Clean Changes:

  • packages/auth-adapters/better-auth/src/adapter.ts:1 - Consolidated imports from better-auth instead of separate @better-auth/core packages
  • Removed unused DBAdapter import that was causing type conflicts
  • Simplified return type annotation on line 215 (removing explicit DBAdapter<BetterAuthOptions> constraint)
  • Added comprehensive Kysely node visitor utilities in packages/orm/src/utils/kysely-utils.ts:107-422

Dependency Management

Smart Prevention: The pnpm override in package.json:67 ("@better-auth/core": "1.4.19") prevents future version drift that could reintroduce this issue. This is a proactive approach to dependency management.

Consistent Versioning: The override aligns with the devDependency versions already specified in the Better Auth package.

Kysely Upgrade: Updated workspace dependency to kysely: 0.29.0-rc.0 in pnpm-workspace.yaml:18, which includes the new node types being used in the utils file.

Bug Fix Assessment

Root Cause Correctly Identified: The issue stems from @better-auth/cli pulling in @better-auth/telemetry, which creates a separate peer dependency resolution context for @better-auth/core. This results in two different instances with incompatible types.

Appropriate Solution: Using the unified better-auth package ensures the same @better-auth/core instance is used throughout the dependency tree.

Security Considerations

No Security Issues: The changes only modify import sources and don't introduce any security vulnerabilities. The pnpm override actually improves security by ensuring consistent dependency versions.

⚠️ Performance Considerations

Minimal Impact: The import consolidation has negligible performance impact. The pnpm override may slightly increase bundle size if it prevents deduplication, but this is necessary for type safety.

Kysely Utils Enhancement: The new DefaultOperationNodeVisitor class provides a comprehensive foundation for AST processing, which could improve query optimization capabilities.

⚠️ Minor Issues Found

Package Manager Version Mismatch: package.json:17 specifies "packageManager": "pnpm@10.33.0" but according to CLAUDE.md, the repo policy requires pnpm@10.12.1. This should be corrected for consistency.

⚠️ Test Coverage Assessment

Test Plan Provided: The PR description includes appropriate testing steps:

  • Build verification for the package
  • Running the package test suite

Recommendation: Consider adding a regression test to detect future Better Auth version conflicts, possibly in the CI pipeline.

📋 Minor Suggestions

  1. Documentation: Consider adding a comment explaining the import strategy to prevent future refactoring back to @better-auth/core
  2. Package Manager: Update packageManager field to match repo policy (pnpm@10.12.1)
  3. Monitoring: Set up alerts for Better Auth version updates to review the pnpm override necessity

🎯 Overall Assessment

This is a well-executed fix that:

  • ✅ Addresses the root cause correctly
  • ✅ Implements appropriate preventive measures
  • ✅ Maintains code quality and type safety
  • ✅ Uses industry best practices for dependency management
  • ✅ Adds valuable Kysely utilities for future development

Recommendation: Approve with minor corrections - This fix properly resolves the type compatibility issue with a clean implementation and appropriate safeguards. The only concern is the package manager version mismatch which should be addressed.


@ymc9 ymc9 marked this pull request as draft April 29, 2026 00:33
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