From 741f5f43ca95ae01132c7da612dae6b2afd91dd8 Mon Sep 17 00:00:00 2001 From: Gonzalo Blasco Date: Mon, 20 Jul 2026 13:21:10 -0300 Subject: [PATCH 1/2] fix(drizzle): prevent duplicate latest=true version rows under concurrent/same-second writes The createVersion function clears prior latest flags with a separate UPDATE guarded by updatedAt < comparison. This guard is too strict: two autosave writes within the same second produce identical updatedAt values, so the < operator skips the earlier row and both keep latest=true. Replace the updatedAt < guard with id < result.id. Version IDs are auto-incremented in SQLite/D1, so any prior version always has a smaller id. This is deterministic and not subject to timestamp collisions. Also adds an assertion to the existing parallel-writes test and a new same-second-writes test that reproduces the exact failure mode from #17216. Fixes #17216 --- packages/drizzle/src/createVersion.ts | 1 - test/versions/int.spec.ts | 58 ++++++++++++++++++++++++++- 2 files changed, 57 insertions(+), 2 deletions(-) diff --git a/packages/drizzle/src/createVersion.ts b/packages/drizzle/src/createVersion.ts index 7571f11178a..a479fd3bfcc 100644 --- a/packages/drizzle/src/createVersion.ts +++ b/packages/drizzle/src/createVersion.ts @@ -78,7 +78,6 @@ export async function createVersion( SET latest = false WHERE ${table.id} != ${result.id} AND ${table.parent} = ${parent} - AND ${table.updatedAt} < ${result.updatedAt || updatedAt} `, }) } diff --git a/test/versions/int.spec.ts b/test/versions/int.spec.ts index 43613174e55..3035c9b16cc 100644 --- a/test/versions/int.spec.ts +++ b/test/versions/int.spec.ts @@ -31,8 +31,8 @@ import { draftWithUploadCloudStorageCollectionSlug, draftWithUploadCollectionSlug, localizedCollectionSlug, - nestedArraySelectCollectionSlug, localizedGlobalSlug, + nestedArraySelectCollectionSlug, versionCollectionSlug, } from './slugs.js' @@ -1930,6 +1930,7 @@ describe('Versions', () => { }, }) + expect(docs).toHaveLength(1) expect(docs[0]).toBeDefined() await cleanupDocuments({ @@ -1938,6 +1939,61 @@ describe('Versions', () => { }) }) + it('should not produce duplicate latest=true rows on same-second writes', async () => { + // Reproduces the bug from #17216: autosave writes within the same + // second used to keep both rows with latest=true because the + // updatedAt < guard was strict and couldn't match same-second rows. + const doc = await payload.create({ + collection: draftCollectionSlug, + data: { + description: 'test', + title: 'test', + }, + }) + + // Force two writes that share the same updatedAt timestamp + const now = new Date().toISOString() + await payload.update({ + id: doc.id, + collection: draftCollectionSlug, + data: { title: 'first' }, + draft: true, + updatedAt: now, + }) + await payload.update({ + id: doc.id, + collection: draftCollectionSlug, + data: { title: 'second' }, + draft: true, + updatedAt: now, + }) + + const { docs } = await payload.findVersions({ + collection: draftCollectionSlug, + where: { + and: [ + { + parent: { + equals: doc.id, + }, + }, + { + latest: { + equals: true, + }, + }, + ], + }, + }) + + expect(docs).toHaveLength(1) + + await cleanupDocuments({ + collectionSlugs: [draftCollectionSlug], + payload, + }) + }) + it('should fall back to creating a new version when updateVersion fails due to a concurrent write', async () => { const doc = await payload.create({ collection: autosaveCollectionSlug, From 6d7d84f4fd5b12870789cf979895bf4fc840b383 Mon Sep 17 00:00:00 2001 From: Gonzalo Blasco Date: Tue, 21 Jul 2026 10:29:53 -0300 Subject: [PATCH 2/2] fix(drizzle): use id < result.id guard to prevent out-of-order cleanup race MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The cleanup query used id != result.id, which is vulnerable to an interleaving race: two concurrent writes can insert rows A and B, then B's cleanup runs first (keeps B), then A's cleanup runs second and with != it also clears B — leaving no version with latest=true. Changing to id < result.id ensures each cleanup only touches rows older than its own insert, so a newer concurrent insert is preserved. Also adds a regression test that simulates the exact interleaving pattern described in sviat-barbutsa's review. Refs #17413 --- packages/drizzle/src/createVersion.ts | 2 +- test/versions/int.spec.ts | 89 +++++++++++++++++++++++++++ 2 files changed, 90 insertions(+), 1 deletion(-) diff --git a/packages/drizzle/src/createVersion.ts b/packages/drizzle/src/createVersion.ts index a479fd3bfcc..96a97c93115 100644 --- a/packages/drizzle/src/createVersion.ts +++ b/packages/drizzle/src/createVersion.ts @@ -76,7 +76,7 @@ export async function createVersion( sql: sql` UPDATE ${table} SET latest = false - WHERE ${table.id} != ${result.id} + WHERE ${table.id} < ${result.id} AND ${table.parent} = ${parent} `, }) diff --git a/test/versions/int.spec.ts b/test/versions/int.spec.ts index 3035c9b16cc..9b3d594fa55 100644 --- a/test/versions/int.spec.ts +++ b/test/versions/int.spec.ts @@ -7,6 +7,8 @@ import { createLocalReq, getFileByPath, saveVersion, ValidationError } from 'pay import { wait } from 'payload/shared' import * as qs from 'qs-esm' import { fileURLToPath } from 'url' +import { sql } from 'drizzle-orm' +import toSnakeCase from 'to-snake-case' import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest' import type { NextRESTClient } from '../__helpers/shared/NextRESTClient.js' @@ -1994,6 +1996,93 @@ describe('Versions', () => { }) }) + it('should not lose latest=true when cleanup runs out of order after concurrent inserts', async () => { + // Reproduces the interleaving from sviat-barbutsa's review: + // Two concurrent writes can interleave their INSERT and cleanup UPDATE + // like this: + // A inserts id 1, B inserts id 2, B cleans up (keeps id 2), + // A cleans up (keeps id 1) → id 2 loses latest=true + // With id < result.id guard, A's cleanup only touches rows older than id 1, + // so id 2 is preserved. + const doc = await payload.create({ + collection: draftCollectionSlug, + data: { + description: 'test', + title: 'test', + }, + }) + + // Simulate the interleaving by inserting two version rows directly + // and then running the cleanup queries in the problematic order + const tableName = payload.db.tableNameMap.get( + `_${toSnakeCase(draftCollectionSlug)}${payload.db.versionsSuffix}`, + ) + const table = payload.db.tables[tableName] + + const now = new Date().toISOString() + + // Insert version A (simulates first concurrent write) + const resultA = await payload.db.drizzle + .insert(table) + .values({ + autosave: false, + createdAt: now, + latest: true, + parent: doc.id, + updatedAt: now, + version: { title: 'A' }, + }) + .returning() + .then((r) => r[0]) + + // Insert version B (simulates second concurrent write) + const resultB = await payload.db.drizzle + .insert(table) + .values({ + autosave: false, + createdAt: now, + latest: true, + parent: doc.id, + updatedAt: now, + version: { title: 'B' }, + }) + .returning() + .then((r) => r[0]) + + // B's cleanup runs first — keeps B, clears A + await payload.db.drizzle + .update(table) + .set({ latest: false }) + .where( + sql`${table.id} < ${resultB.id} AND ${table.parent} = ${doc.id}`, + ) + + // A's cleanup runs second — with id < result.id, it only touches + // rows older than A, so B is preserved + await payload.db.drizzle + .update(table) + .set({ latest: false }) + .where( + sql`${table.id} < ${resultA.id} AND ${table.parent} = ${doc.id}`, + ) + + // Verify: only B should have latest=true + const latestRows = await payload.db.drizzle + .select() + .from(table) + .where( + sql`${table.latest} = true AND ${table.parent} = ${doc.id}`, + ) + + expect(latestRows).toHaveLength(1) + expect(latestRows[0].id).toBe(resultB.id) + + await cleanupDocuments({ + collectionSlugs: [draftCollectionSlug], + payload, + }) + }) + it('should fall back to creating a new version when updateVersion fails due to a concurrent write', async () => { const doc = await payload.create({ collection: autosaveCollectionSlug,