fix: Prevent foreign key violation when permanently deleting a team (#12527)

The attachment cleanup loop used findAllInBatches, which advances an
OFFSET each iteration. Because the callback deletes each batch, the
remaining rows shift backwards and the advancing offset skips over them,
leaving attachments that still reference the team. team.destroy() then
failed with attachments_teamId_fkey.

Page from offset 0 until no attachments remain, and remove the now
redundant per-user attachment delete so the loop is the single
authoritative cleanup.

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

server/commands/teamPermanentDeleter.test.ts

@@ -139,6 +139,47 @@ describe("teamPermanentDeleter", () => {
     ).toEqual(0);
   });
 
+  it("should destroy attachments spanning multiple batches", async () => {
+    const team = await buildTeam({
+      deletedAt: subDays(new Date(), 90),
+    });
+    const user = await buildUser({ teamId: team.id });
+    const document = await buildDocument({
+      teamId: team.id,
+      userId: user.id,
+    });
+
+    // More than a single batch (batchLimit is 100) to ensure no attachments are
+    // skipped while deleting, which would otherwise leave rows referencing the
+    // team and cause a foreign key violation when the team is destroyed.
+    await Promise.all(
+      Array.from({ length: 150 }, () =>
+        buildAttachment({
+          teamId: team.id,
+          userId: user.id,
+          documentId: document.id,
+        })
+      )
+    );
+
+    await teamPermanentDeleter(team);
+
+    expect(
+      await Attachment.count({
+        where: {
+          teamId: team.id,
+        },
+      })
+    ).toEqual(0);
+    expect(
+      await Team.count({
+        where: {
+          id: team.id,
+        },
+      })
+    ).toEqual(0);
+  });
+
   it("should destroy imports", async () => {
     const team = await buildTeam({
       deletedAt: subDays(new Date(), 90),

server/commands/teamPermanentDeleter.ts

@@ -40,21 +40,25 @@ async function teamPermanentDeleter(team: Team) {
   );
   const teamId = team.id;
 
-  await Attachment.findAllInBatches<Attachment>(
-    {
+  // Attachments are destroyed as individual instances (rather than a bulk
+  // delete) so the BeforeDestroy hook runs and removes the associated file from
+  // storage. We cannot use findAllInBatches with an advancing offset here –
+  // deleting a batch shifts the remaining rows backwards, so advancing the
+  // offset would skip records and leave attachments that still reference the
+  // team, causing a foreign key violation when the team itself is destroyed.
+  // Instead we repeatedly fetch and delete the first batch until none remain.
+  let attachments: Attachment[];
+  do {
+    attachments = await Attachment.findAll<Attachment>({
       where: {
         teamId,
       },
-      batchLimit: 100,
-    },
-    async (attachments, options) => {
+      limit: 100,
+    });
+
+    if (attachments.length > 0) {
       await sequelize.transaction(async (transaction) => {
-        Logger.info(
-          "commands",
-          `Deleting attachments ${options.offset} – ${
-            (options.offset || 0) + (options?.limit || 0)
-          }…`
-        );
+        Logger.info("commands", `Deleting ${attachments.length} attachments…`);
         await Promise.all(
           attachments.map((attachment) =>
             attachment.destroy({
@@ -64,7 +68,7 @@ async function teamPermanentDeleter(team: Team) {
         );
       });
     }
-  );
+  } while (attachments.length > 0);
 
   // Destroy user-relation models
   await User.findAllInBatches<User>(
@@ -85,14 +89,6 @@ async function teamPermanentDeleter(team: Team) {
           force: true,
           transaction,
         });
-        await Attachment.destroy({
-          where: {
-            teamId,
-            userId: userIds,
-          },
-          force: true,
-          transaction,
-        });
         await ApiKey.destroy({
           where: {
             userId: userIds,