From a99571e95b9b370706ad9c82906aaa33e2bb83cb Mon Sep 17 00:00:00 2001 From: Gustavo Valverde Date: Thu, 11 Jun 2026 22:35:33 -0700 Subject: [PATCH] fix(org): make invitation acceptance idempotent --- .changeset/fix-invite-accept-race.md | 6 + .../src/plugins/organization/adapter.ts | 24 +- .../organization/routes/crud-invites.test.ts | 220 +++++++++++- .../organization/routes/crud-invites.ts | 331 ++++++++++++------ .../src/plugins/organization/team.test.ts | 16 +- .../kysely-adapter/src/bun-sqlite-dialect.ts | 12 +- packages/kysely-adapter/src/dialect.ts | 9 +- .../kysely-adapter/src/node-sqlite-dialect.ts | 12 +- .../src/sqlite-dialect-mutations.test.ts | 100 +++++- 9 files changed, 596 insertions(+), 134 deletions(-) create mode 100644 .changeset/fix-invite-accept-race.md diff --git a/.changeset/fix-invite-accept-race.md b/.changeset/fix-invite-accept-race.md new file mode 100644 index 0000000000..86680d267f --- /dev/null +++ b/.changeset/fix-invite-accept-race.md @@ -0,0 +1,6 @@ +--- +"better-auth": patch +"@better-auth/kysely-adapter": patch +--- + +Fix organization invitation acceptance retries so concurrent accepts converge on one member and accepted invitations can be replayed safely. Direct SQLite/MySQL/Postgres Kysely databases now use real adapter transactions where supported, and SQLite update/delete mutations no longer report stale insert IDs. diff --git a/packages/better-auth/src/plugins/organization/adapter.ts b/packages/better-auth/src/plugins/organization/adapter.ts index d9579efddb..ea9050f58d 100644 --- a/packages/better-auth/src/plugins/organization/adapter.ts +++ b/packages/better-auth/src/plugins/organization/adapter.ts @@ -16,6 +16,7 @@ import type { InferOrganization, InferTeam, InvitationInput, + InvitationStatus, Member, MemberInput, OrganizationInput, @@ -940,8 +941,9 @@ export const getOrgAdapter = ( }): Promise< { status: "added"; member: TeamMember } | { status: "limitReached" } > => { - return runWithTransaction(baseAdapter, async () => { - const adapter = await getCurrentAdapter(baseAdapter); + const addTeamMember = async ( + adapter: Awaited>, + ) => { const existing = await adapter.findOne({ model: "teamMember", where: [ @@ -950,14 +952,14 @@ export const getOrgAdapter = ( ], }); if (existing) { - return { status: "added", member: existing }; + return { status: "added" as const, member: existing }; } const count = await adapter.count({ model: "teamMember", where: [{ field: "teamId", value: data.teamId }], }); if (count >= data.maximumMembersPerTeam) { - return { status: "limitReached" }; + return { status: "limitReached" as const }; } const member = await adapter.create, TeamMember>( { @@ -969,7 +971,15 @@ export const getOrgAdapter = ( }, }, ); - return { status: "added", member }; + return { status: "added" as const, member }; + }; + + const currentAdapter = await getCurrentAdapter(baseAdapter); + if (currentAdapter !== baseAdapter) { + return await addTeamMember(currentAdapter); + } + return await runWithTransaction(baseAdapter, async () => { + return await addTeamMember(await getCurrentAdapter(baseAdapter)); }); }, removeTeamMember: async (data: { teamId: string; userId: string }) => { @@ -1133,13 +1143,13 @@ export const getOrgAdapter = ( }, updateInvitation: async (data: { invitationId: string; - status: "pending" | "accepted" | "canceled" | "rejected"; + status: InvitationStatus; /** * Only transition when the invitation is currently in this status. The * guarded update is atomic, so a concurrent caller racing the same * transition gets `null` instead of both proceeding. */ - fromStatus?: "pending"; + fromStatus?: InvitationStatus; }) => { const adapter = await getCurrentAdapter(baseAdapter); const where = [{ field: "id", value: data.invitationId }]; diff --git a/packages/better-auth/src/plugins/organization/routes/crud-invites.test.ts b/packages/better-auth/src/plugins/organization/routes/crud-invites.test.ts index 0478c1d835..4de9be842b 100644 --- a/packages/better-auth/src/plugins/organization/routes/crud-invites.test.ts +++ b/packages/better-auth/src/plugins/organization/routes/crud-invites.test.ts @@ -1,5 +1,5 @@ import type { BetterAuthOptions, GenerateIdFn } from "@better-auth/core"; -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; import { getTestInstance } from "../../../test-utils/test-instance"; import { organizationClient } from "../client"; import { organization } from "../organization"; @@ -372,6 +372,224 @@ describe("organization invitation recipient ownership gates", async () => { }); }); +describe("organization invitation acceptance idempotency", async () => { + const INVITEE_EMAIL = "invitee-idempotent@example.com"; + const CONCURRENT_INVITEE_EMAIL = "invitee-concurrent@example.com"; + const PASSWORD = "test-password-123"; + + const wait = (ms: number) => + new Promise((resolve) => setTimeout(resolve, ms)); + + /** + * @see https://github.com/better-auth/better-auth/issues/9154 + */ + it("replays an accepted invitation after expiration without creating another member", async () => { + const { client, signInWithTestUser, signInWithUser, cookieSetter, db } = + await getTestInstance( + { + databaseHooks: { + user: { + create: { + before: async (user) => ({ + data: { ...user, emailVerified: true }, + }), + }, + }, + }, + plugins: [ + organization({ + async sendInvitationEmail() {}, + }), + ], + }, + { clientOptions: { plugins: [organizationClient()] } }, + ); + + const { headers: ownerHeaders } = await signInWithTestUser(); + const createdOrganization = await client.organization.create({ + name: "Replay Org", + slug: "replay-org", + fetchOptions: { + headers: ownerHeaders, + onSuccess: cookieSetter(ownerHeaders), + }, + }); + const invitation = await client.organization.inviteMember({ + organizationId: createdOrganization.data!.id, + email: INVITEE_EMAIL, + role: "member", + fetchOptions: { headers: ownerHeaders }, + }); + const invitationId = String(invitation.data!.id); + + await client.signUp.email({ + email: INVITEE_EMAIL, + password: PASSWORD, + name: "Invitee", + }); + const { headers: inviteeHeaders, res: invitee } = await signInWithUser( + INVITEE_EMAIL, + PASSWORD, + ); + + const firstAccept = await client.organization.acceptInvitation({ + invitationId, + fetchOptions: { headers: inviteeHeaders }, + }); + expect(firstAccept.error).toBeNull(); + expect(firstAccept.data?.invitation.status).toBe("accepted"); + + await db.update({ + model: "invitation", + where: [{ field: "id", value: invitationId }], + update: { expiresAt: new Date(Date.now() - 1000) }, + }); + + const replay = await client.organization.acceptInvitation({ + invitationId, + fetchOptions: { headers: inviteeHeaders }, + }); + + expect(replay.error).toBeNull(); + expect(replay.data?.invitation.status).toBe("accepted"); + expect(replay.data?.member.id).toBe(firstAccept.data?.member.id); + + const members = await db.findMany<{ id: string }>({ + model: "member", + where: [ + { field: "organizationId", value: createdOrganization.data!.id }, + { field: "userId", value: invitee.user.id }, + ], + }); + expect(members).toHaveLength(1); + }); + + /** + * @see https://github.com/better-auth/better-auth/issues/9154 + */ + it("replays concurrent accepts while creating the member once", async () => { + let inviteeUserId = ""; + let delayedInviteeMemberCreates = 0; + const { + auth, + client, + signInWithTestUser, + signInWithUser, + cookieSetter, + db, + } = await getTestInstance( + { + databaseHooks: { + user: { + create: { + before: async (user) => ({ + data: { ...user, emailVerified: true }, + }), + }, + }, + }, + plugins: [ + organization({ + async sendInvitationEmail() {}, + }), + ], + }, + { clientOptions: { plugins: [organizationClient()] } }, + ); + + const { headers: ownerHeaders } = await signInWithTestUser(); + const createdOrganization = await client.organization.create({ + name: "Concurrent Replay Org", + slug: "concurrent-replay-org", + fetchOptions: { + headers: ownerHeaders, + onSuccess: cookieSetter(ownerHeaders), + }, + }); + const invitation = await client.organization.inviteMember({ + organizationId: createdOrganization.data!.id, + email: CONCURRENT_INVITEE_EMAIL, + role: "member", + fetchOptions: { headers: ownerHeaders }, + }); + const invitationId = String(invitation.data!.id); + + await client.signUp.email({ + email: CONCURRENT_INVITEE_EMAIL, + password: PASSWORD, + name: "Concurrent Invitee", + }); + const { headers: inviteeHeaders, res: invitee } = await signInWithUser( + CONCURRENT_INVITEE_EMAIL, + PASSWORD, + ); + inviteeUserId = invitee.user.id; + + const ctx = await auth.$context; + const originalTransaction = ctx.adapter.transaction.bind(ctx.adapter); + const transactionSpy = vi + .spyOn(ctx.adapter, "transaction") + .mockImplementation(async (callback) => { + return await originalTransaction(async (transactionAdapter) => { + const originalCreate = + transactionAdapter.create.bind(transactionAdapter); + const createSpy = vi + .spyOn(transactionAdapter, "create") + .mockImplementation(async (createArgs) => { + if ( + createArgs.model === "member" && + createArgs.data.userId === inviteeUserId + ) { + delayedInviteeMemberCreates++; + await wait(50); + } + return await originalCreate(createArgs); + }); + + try { + return await callback(transactionAdapter); + } finally { + createSpy.mockRestore(); + } + }); + }); + + try { + const acceptResponses = await Promise.all([ + client.organization.acceptInvitation({ + invitationId, + fetchOptions: { headers: inviteeHeaders }, + }), + client.organization.acceptInvitation({ + invitationId, + fetchOptions: { headers: inviteeHeaders }, + }), + ]); + + for (const acceptResponse of acceptResponses) { + expect(acceptResponse.error).toBeNull(); + expect(acceptResponse.data?.invitation.status).toBe("accepted"); + } + expect( + new Set(acceptResponses.map((response) => response.data?.member.id)) + .size, + ).toBe(1); + expect(delayedInviteeMemberCreates).toBe(1); + } finally { + transactionSpy.mockRestore(); + } + + const members = await db.findMany<{ id: string }>({ + model: "member", + where: [ + { field: "organizationId", value: createdOrganization.data!.id }, + { field: "userId", value: invitee.user.id }, + ], + }); + expect(members).toHaveLength(1); + }); +}); + /** * An invitation's teamId must be scoped to the invitation's organization at * creation AND acceptance, and team read endpoints must verify organization diff --git a/packages/better-auth/src/plugins/organization/routes/crud-invites.ts b/packages/better-auth/src/plugins/organization/routes/crud-invites.ts index f13fa61183..7e90085ebb 100644 --- a/packages/better-auth/src/plugins/organization/routes/crud-invites.ts +++ b/packages/better-auth/src/plugins/organization/routes/crud-invites.ts @@ -658,12 +658,90 @@ export const acceptInvitation = (options: O) => const invitation = await adapter.findInvitationById( ctx.body.invitationId, ); + type AcceptedInvitation = NonNullable; + type AcceptedInvitationMember = Member & + InferAdditionalFieldsFromPluginOptions<"member", O, false>; - if ( - !invitation || - invitation.expiresAt < new Date() || - invitation.status !== "pending" - ) { + const findAcceptedInvitationMember = async ( + acceptedInvitation: AcceptedInvitation, + ) => { + return await ctx.context.adapter.findOne({ + model: "member", + where: [ + { + field: "userId", + value: session.user.id, + }, + { + field: "organizationId", + value: acceptedInvitation.organizationId, + }, + ], + }); + }; + + const restoreAcceptedInvitationTeamSession = async ( + acceptedInvitation: AcceptedInvitation, + ) => { + if ( + !ctx.context.orgOptions.teams?.enabled || + !("teamId" in acceptedInvitation) || + !acceptedInvitation.teamId + ) { + return; + } + + const teamIds = (acceptedInvitation.teamId as string).split(","); + if (teamIds.length !== 1) { + return; + } + + const teamId = teamIds[0]!; + const teamMember = await adapter.findTeamMember({ + teamId, + userId: session.user.id, + }); + if (!teamMember) { + return; + } + + const updatedSession = await adapter.setActiveTeam( + session.session.token, + teamId, + ctx, + ); + + await setSessionCookie(ctx, { + session: updatedSession, + user: session.user, + }); + }; + + const completeAcceptedInvitationReplay = async ( + acceptedInvitation: AcceptedInvitation, + ) => { + const member = await findAcceptedInvitationMember(acceptedInvitation); + if (!member) { + throw APIError.from( + "BAD_REQUEST", + ORGANIZATION_ERROR_CODES.FAILED_TO_RETRIEVE_INVITATION, + ); + } + + await restoreAcceptedInvitationTeamSession(acceptedInvitation); + await adapter.setActiveOrganization( + session.session.token, + acceptedInvitation.organizationId, + ctx, + ); + + return ctx.json({ + invitation: acceptedInvitation, + member, + }); + }; + + if (!invitation) { throw APIError.from( "BAD_REQUEST", ORGANIZATION_ERROR_CODES.INVITATION_NOT_FOUND, @@ -697,6 +775,20 @@ export const acceptInvitation = (options: O) => ); } + if (invitation.status === "accepted") { + return await completeAcceptedInvitationReplay(invitation); + } + + if ( + invitation.expiresAt < new Date() || + invitation.status !== "pending" + ) { + throw APIError.from( + "BAD_REQUEST", + ORGANIZATION_ERROR_CODES.INVITATION_NOT_FOUND, + ); + } + const membershipLimit = ctx.context.orgOptions?.membershipLimit || 100; const membersCount = await adapter.countMembers({ organizationId: invitation.organizationId, @@ -733,118 +825,137 @@ export const acceptInvitation = (options: O) => }); } - // Claim the invitation atomically so only one concurrent accept wins the - // pending -> accepted transition; the guarded update is a single statement - // and atomic on every adapter. The membership work then runs in a - // transaction so it is all-or-nothing where the adapter supports it, and if - // it fails the claim is released back to pending so the invitee can retry - // instead of being stranded as accepted with no membership. - const acceptedI = await adapter.updateInvitation({ - invitationId: ctx.body.invitationId, - status: "accepted", - fromStatus: "pending", + let didClaimInvitation = false; + const acceptance = await runWithTransaction( + ctx.context.adapter, + async () => { + // Claim the invitation inside the transaction so transactional adapters + // do not expose an accepted invitation before the member row exists. + const acceptedI = await adapter.updateInvitation({ + invitationId: ctx.body.invitationId, + status: "accepted", + fromStatus: "pending", + }); + if (!acceptedI) { + return { status: "replay" as const }; + } + didClaimInvitation = true; + + if ( + ctx.context.orgOptions.teams && + ctx.context.orgOptions.teams.enabled && + "teamId" in acceptedI && + acceptedI.teamId + ) { + const teamIds = (acceptedI.teamId as string).split(","); + const onlyOne = teamIds.length === 1; + + for (const teamId of teamIds) { + // Confirm the team still belongs to the accepted invitation's + // organization before adding the member. This keeps team + // membership consistent with the invitation's organization, + // including for older invitations and for teams that were + // moved or removed between invite and accept. + const team = await adapter.findTeamById({ + teamId, + organizationId: acceptedI.organizationId, + }); + if (!team) { + throw APIError.from( + "BAD_REQUEST", + ORGANIZATION_ERROR_CODES.TEAM_NOT_FOUND, + ); + } + + const maximumMembersPerTeam = await resolveMaximumMembersPerTeam( + ctx.context.orgOptions.teams, + { + teamId, + organizationId: acceptedI.organizationId, + session, + }, + ); + if (maximumMembersPerTeam !== undefined) { + const result = await adapter.addTeamMemberWithLimit({ + teamId, + userId: session.user.id, + maximumMembersPerTeam, + }); + if (result.status === "limitReached") { + throw APIError.from( + "FORBIDDEN", + ORGANIZATION_ERROR_CODES.TEAM_MEMBER_LIMIT_REACHED, + ); + } + } else { + await adapter.findOrCreateTeamMember({ + teamId: teamId, + userId: session.user.id, + }); + } + } + + if (onlyOne) { + const teamId = teamIds[0]!; + const updatedSession = await adapter.setActiveTeam( + session.session.token, + teamId, + ctx, + ); + + await setSessionCookie(ctx, { + session: updatedSession, + user: session.user, + }); + } + } + + const createdMember = await adapter.createMember({ + organizationId: acceptedI.organizationId, + userId: session.user.id, + role: acceptedI.role, + createdAt: new Date(), + }); + + await adapter.setActiveOrganization( + session.session.token, + acceptedI.organizationId, + ctx, + ); + + return { + status: "accepted" as const, + invitation: acceptedI, + member: createdMember, + }; + }, + ).catch(async (error) => { + // The membership work failed; release the claim so the invitation is + // pending again and the invitee can retry. + if (didClaimInvitation) { + await adapter.updateInvitation({ + invitationId: ctx.body.invitationId, + status: "pending", + fromStatus: "accepted", + }); + } + throw error; }); - if (!acceptedI) { - // Another request already accepted this invitation. + + if (acceptance.status === "replay") { + const latestInvitation = await adapter.findInvitationById( + ctx.body.invitationId, + ); + if (latestInvitation?.status === "accepted") { + return await completeAcceptedInvitationReplay(latestInvitation); + } throw APIError.from( "BAD_REQUEST", ORGANIZATION_ERROR_CODES.INVITATION_NOT_FOUND, ); } - const member = await runWithTransaction(ctx.context.adapter, async () => { - if ( - ctx.context.orgOptions.teams && - ctx.context.orgOptions.teams.enabled && - "teamId" in acceptedI && - acceptedI.teamId - ) { - const teamIds = (acceptedI.teamId as string).split(","); - const onlyOne = teamIds.length === 1; - - for (const teamId of teamIds) { - // Confirm the team still belongs to the accepted invitation's - // organization before adding the member. This keeps team - // membership consistent with the invitation's organization, - // including for older invitations and for teams that were - // moved or removed between invite and accept. - const team = await adapter.findTeamById({ - teamId, - organizationId: acceptedI.organizationId, - }); - if (!team) { - throw APIError.from( - "BAD_REQUEST", - ORGANIZATION_ERROR_CODES.TEAM_NOT_FOUND, - ); - } - - const maximumMembersPerTeam = await resolveMaximumMembersPerTeam( - ctx.context.orgOptions.teams, - { - teamId, - organizationId: acceptedI.organizationId, - session, - }, - ); - if (maximumMembersPerTeam !== undefined) { - const result = await adapter.addTeamMemberWithLimit({ - teamId, - userId: session.user.id, - maximumMembersPerTeam, - }); - if (result.status === "limitReached") { - throw APIError.from( - "FORBIDDEN", - ORGANIZATION_ERROR_CODES.TEAM_MEMBER_LIMIT_REACHED, - ); - } - } else { - await adapter.findOrCreateTeamMember({ - teamId: teamId, - userId: session.user.id, - }); - } - } - - if (onlyOne) { - const teamId = teamIds[0]!; - const updatedSession = await adapter.setActiveTeam( - session.session.token, - teamId, - ctx, - ); - - await setSessionCookie(ctx, { - session: updatedSession, - user: session.user, - }); - } - } - - const createdMember = await adapter.createMember({ - organizationId: acceptedI.organizationId, - userId: session.user.id, - role: acceptedI.role, - createdAt: new Date(), - }); - - await adapter.setActiveOrganization( - session.session.token, - acceptedI.organizationId, - ctx, - ); - - return createdMember; - }).catch(async (error) => { - // The membership work failed; release the claim so the invitation is - // pending again and the invitee can retry. - await adapter.updateInvitation({ - invitationId: ctx.body.invitationId, - status: "pending", - }); - throw error; - }); + const { invitation: acceptedI, member } = acceptance; if (options?.organizationHooks?.afterAcceptInvitation) { await options?.organizationHooks.afterAcceptInvitation({ diff --git a/packages/better-auth/src/plugins/organization/team.test.ts b/packages/better-auth/src/plugins/organization/team.test.ts index a2e81aaff1..c4688fee8e 100644 --- a/packages/better-auth/src/plugins/organization/team.test.ts +++ b/packages/better-auth/src/plugins/organization/team.test.ts @@ -1653,7 +1653,10 @@ describe("accept-invitation validates team capacity before adding the member", a expect(orgMembers.some((m) => m.userId === userId)).toBe(false); }); - it("accepts only once when the same invitation is accepted concurrently", async () => { + /** + * @see https://github.com/better-auth/better-auth/issues/9154 + */ + it("replays the accepted member when the same invitation is accepted concurrently", async () => { const team = await auth.api.createTeam({ headers: owner.headers, body: { name: "Concurrent Team", organizationId: org.id }, @@ -1674,10 +1677,13 @@ describe("accept-invitation validates team capacity before adding the member", a }), ]); - // One request wins the claim; the other observes the invitation is no - // longer pending and is rejected, never running the side effects twice. - expect(results.filter((r) => r.status === "fulfilled")).toHaveLength(1); - expect(results.filter((r) => r.status === "rejected")).toHaveLength(1); + const acceptedResults = results.flatMap((result) => + result.status === "fulfilled" ? [result.value] : [], + ); + expect(acceptedResults).toHaveLength(2); + expect( + new Set(acceptedResults.map((result) => result.member.id)).size, + ).toBe(1); const orgMembers = await ctx.adapter.findMany<{ userId: string }>({ model: "member", diff --git a/packages/kysely-adapter/src/bun-sqlite-dialect.ts b/packages/kysely-adapter/src/bun-sqlite-dialect.ts index fd6bfd1a6f..3639fb43ec 100644 --- a/packages/kysely-adapter/src/bun-sqlite-dialect.ts +++ b/packages/kysely-adapter/src/bun-sqlite-dialect.ts @@ -143,15 +143,19 @@ class BunSqliteConnection implements DatabaseConnection { }); } + const firstSqlToken = sql.trimStart().split(/\s+/, 1)[0]?.toLowerCase(); + const shouldReturnInsertId = + firstSqlToken === "insert" || firstSqlToken === "replace"; const { changes, lastInsertRowid } = stmt.run(...params); + const insertId = + typeof lastInsertRowid === "bigint" + ? lastInsertRowid + : BigInt(lastInsertRowid); return Promise.resolve({ rows: [], numAffectedRows: BigInt(changes), - insertId: - typeof lastInsertRowid === "bigint" - ? lastInsertRowid - : BigInt(lastInsertRowid), + ...(shouldReturnInsertId ? { insertId } : {}), }); } diff --git a/packages/kysely-adapter/src/dialect.ts b/packages/kysely-adapter/src/dialect.ts index 489757b598..70c6e7d5eb 100644 --- a/packages/kysely-adapter/src/dialect.ts +++ b/packages/kysely-adapter/src/dialect.ts @@ -83,6 +83,7 @@ export const createKyselyAdapter = async (config: BetterAuthOptions) => { } let dialect: Dialect | undefined = undefined; + let transaction: boolean | undefined = undefined; const databaseType = getKyselyDatabaseType(db); @@ -94,17 +95,20 @@ export const createKyselyAdapter = async (config: BetterAuthOptions) => { dialect = new SqliteDialect({ database: db, }); + transaction = true; } if ("getConnection" in db) { // @ts-expect-error - mysql2/promise dialect = new MysqlDialect(db); + transaction = true; } if ("connect" in db) { dialect = new PostgresDialect({ pool: db, }); + transaction = true; } if ("fileControl" in db) { @@ -112,6 +116,7 @@ export const createKyselyAdapter = async (config: BetterAuthOptions) => { dialect = new BunSqliteDialect({ database: db, }); + transaction = true; } if ("createSession" in db) { @@ -141,6 +146,7 @@ export const createKyselyAdapter = async (config: BetterAuthOptions) => { dialect = new NodeSqliteDialect({ database: db, }); + transaction = true; } } @@ -150,11 +156,12 @@ export const createKyselyAdapter = async (config: BetterAuthOptions) => { dialect = new D1SqliteDialect({ database: db, }); + transaction = false; } return { kysely: dialect ? new Kysely({ dialect }) : null, databaseType, - transaction: undefined, + transaction, }; }; diff --git a/packages/kysely-adapter/src/node-sqlite-dialect.ts b/packages/kysely-adapter/src/node-sqlite-dialect.ts index e10a3d6c3a..4067f4e651 100644 --- a/packages/kysely-adapter/src/node-sqlite-dialect.ts +++ b/packages/kysely-adapter/src/node-sqlite-dialect.ts @@ -143,15 +143,19 @@ class NodeSqliteConnection implements DatabaseConnection { }); } + const firstSqlToken = sql.trimStart().split(/\s+/, 1)[0]?.toLowerCase(); + const shouldReturnInsertId = + firstSqlToken === "insert" || firstSqlToken === "replace"; const { changes, lastInsertRowid } = stmt.run(...params); + const insertId = + typeof lastInsertRowid === "bigint" + ? lastInsertRowid + : BigInt(lastInsertRowid); return Promise.resolve({ rows: [], numAffectedRows: BigInt(changes), - insertId: - typeof lastInsertRowid === "bigint" - ? lastInsertRowid - : BigInt(lastInsertRowid), + ...(shouldReturnInsertId ? { insertId } : {}), }); } diff --git a/packages/kysely-adapter/src/sqlite-dialect-mutations.test.ts b/packages/kysely-adapter/src/sqlite-dialect-mutations.test.ts index 190467b4d3..1a5a2c87bf 100644 --- a/packages/kysely-adapter/src/sqlite-dialect-mutations.test.ts +++ b/packages/kysely-adapter/src/sqlite-dialect-mutations.test.ts @@ -1,9 +1,11 @@ import type { SQLInputValue } from "node:sqlite"; import { DatabaseSync } from "node:sqlite"; -import type { Generated } from "kysely"; -import { Kysely } from "kysely"; +import type { BetterAuthOptions } from "@better-auth/core"; +import type { Dialect, Generated } from "kysely"; +import { CompiledQuery, Kysely } from "kysely"; import { describe, expect, it } from "vitest"; import { BunSqliteDialect } from "./bun-sqlite-dialect"; +import { createKyselyAdapter } from "./dialect"; import { NodeSqliteDialect } from "./node-sqlite-dialect"; interface UsersTable { @@ -52,7 +54,54 @@ function asBunLikeDatabase(db: DatabaseSync) { } as unknown as ConstructorParameters[0]["database"]; } +async function expectNoStaleInsertIdForUpdateOrDelete(dialect: Dialect) { + const driver = dialect.createDriver(); + await driver.init(); + const connection = await driver.acquireConnection(); + + try { + await connection.executeQuery( + CompiledQuery.raw( + "CREATE TABLE users (id INTEGER PRIMARY KEY AUTOINCREMENT, name TEXT, value TEXT)", + ), + ); + const insert = await connection.executeQuery( + CompiledQuery.raw( + "INSERT INTO users (name, value) VALUES ('alice', 'first')", + ), + ); + expect(insert.insertId).toBe(1n); + + const update = await connection.executeQuery( + CompiledQuery.raw( + "UPDATE users SET value = 'second' WHERE name = 'alice'", + ), + ); + expect(update.numAffectedRows).toBe(1n); + expect(update.insertId).toBeUndefined(); + + const remove = await connection.executeQuery( + CompiledQuery.raw("DELETE FROM users WHERE name = 'alice'"), + ); + expect(remove.numAffectedRows).toBe(1n); + expect(remove.insertId).toBeUndefined(); + } finally { + await driver.releaseConnection(connection); + await driver.destroy(); + } +} + describe("NodeSqliteDialect mutation metadata", () => { + it("enables real transactions for direct node:sqlite databases", async () => { + const sqlite = new DatabaseSync(":memory:"); + const adapter = await createKyselyAdapter({ + database: sqlite, + } satisfies BetterAuthOptions); + + expect(adapter.transaction).toBe(true); + await adapter.kysely?.destroy(); + }); + it("binds multiple parameters and surfaces insert/update/delete metadata", async () => { const sqlite = new DatabaseSync(":memory:"); createSchema(sqlite); @@ -108,6 +157,45 @@ describe("NodeSqliteDialect mutation metadata", () => { await db.destroy(); }); + + it("does not report stale insertId for update and delete", async () => { + await expectNoStaleInsertIdForUpdateOrDelete( + new NodeSqliteDialect({ database: new DatabaseSync(":memory:") }), + ); + }); +}); + +describe("D1SqliteDialect transaction metadata", () => { + it("keeps interactive transactions disabled for D1", async () => { + const d1LikeDatabase = { + batch() { + return Promise.resolve([]); + }, + exec() { + return Promise.resolve(); + }, + prepare() { + return { + bind() { + return { + all() { + return Promise.resolve({ + results: [], + meta: {}, + }); + }, + }; + }, + }; + }, + } as unknown as BetterAuthOptions["database"]; + const adapter = await createKyselyAdapter({ + database: d1LikeDatabase, + } satisfies BetterAuthOptions); + + expect(adapter.transaction).toBe(false); + await adapter.kysely?.destroy(); + }); }); describe("BunSqliteDialect mutation metadata", () => { @@ -165,4 +253,12 @@ describe("BunSqliteDialect mutation metadata", () => { await db.destroy(); }); + + it("does not report stale insertId for update and delete", async () => { + await expectNoStaleInsertIdForUpdateOrDelete( + new BunSqliteDialect({ + database: asBunLikeDatabase(new DatabaseSync(":memory:")), + }), + ); + }); });