fix(attachments): purge R2 attachments on no-JS bulk email delete

The form-based bulk-delete fallback removed KV entries but left R2
attachments orphaned. Extract a shared deleteAttachmentsForEmails helper
and use it across single, JSON bulk, and form bulk delete paths.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

src/routes/admin.test.ts

@@ -728,6 +728,79 @@ describe("Admin Routes", () => {
           .length;
         expect(indicatorCount).toBe(1);
       });
+
+      it("form-based bulk-delete also removes R2 attachments", async () => {
+        const r2Env = createMockEnv({ withR2: true }) as unknown as Env;
+        const bucket = r2Env.ATTACHMENT_BUCKET as unknown as {
+          put: (k: string, v: string) => Promise<void>;
+          _has: (k: string) => boolean;
+        };
+        await bucket.put("att-1", "data1");
+        await bucket.put("att-2", "data2");
+
+        const loginForm = new FormData();
+        loginForm.append("password", "test-password");
+        const loginRes = await testApp.request(
+          "/admin/login",
+          { method: "POST", body: loginForm },
+          r2Env,
+        );
+        const authCookie = (loginRes.headers.get("Set-Cookie") as string).split(
+          ";",
+        )[0];
+
+        const feedId = "bulk-r2-feed";
+        await r2Env.EMAIL_STORAGE.put(
+          "feeds:list",
+          JSON.stringify({ feeds: [{ id: feedId, title: "F" }] }),
+        );
+        const emailKey = `feed:${feedId}:1`;
+        await r2Env.EMAIL_STORAGE.put(
+          emailKey,
+          JSON.stringify({
+            subject: "x",
+            from: "a@b.c",
+            content: "<p>x</p>",
+            receivedAt: 1,
+            headers: {},
+            attachments: [{ id: "att-1" }, { id: "att-2" }],
+          }),
+        );
+        await r2Env.EMAIL_STORAGE.put(
+          `feed:${feedId}:metadata`,
+          JSON.stringify({
+            emails: [
+              {
+                key: emailKey,
+                subject: "x",
+                receivedAt: 1,
+                attachmentIds: ["att-1", "att-2"],
+              },
+            ],
+          }),
+        );
+
+        const form = new FormData();
+        form.append("emailKeys", emailKey);
+        const res = await testApp.request(
+          `/admin/feeds/${feedId}/emails/bulk-delete`,
+          {
+            method: "POST",
+            headers: {
+              Cookie: authCookie,
+              Origin: "https://test.getmynews.app",
+            },
+            body: form,
+          },
+          r2Env,
+        );
+
+        expect(res.status).toBe(302);
+        expect(res.headers.get("Location")).toContain("bulkDeleted");
+        expect(await r2Env.EMAIL_STORAGE.get(emailKey, "json")).toBeNull();
+        expect(bucket._has("att-1")).toBe(false);
+        expect(bucket._has("att-2")).toBe(false);
+      });
     });
   });
 

src/routes/admin/emails.tsx

@@ -8,9 +8,11 @@ import {
 } from "../../types";
 import { logger } from "../../lib/logger";
 import { Layout, clampText } from "./ui";
-import { deleteKeysWithConcurrency } from "./helpers";
+import {
+  deleteAttachmentsForEmails,
+  deleteKeysWithConcurrency,
+} from "./helpers";
 import { feedRssUrl, feedAtomUrl, feedEmailAddress } from "../../utils/urls";
-import { getAttachmentBucket } from "../../utils/attachments";
 import { emailsPageScript } from "../../scripts/generated/emails-page";
 
 type AppEnv = { Bindings: Env };
@@ -632,10 +634,11 @@ emailsRouter.post("/emails/:emailKey/delete", async (c) => {
     const feedMetadata = (await emailStorage.get(feedMetadataKey, {
       type: "json",
     })) as FeedMetadata | null;
-    const attachmentIds =
-      feedMetadata?.emails.find((e) => e.key === emailKey)?.attachmentIds ?? [];
 
     await emailStorage.delete(emailKey);
+    await deleteAttachmentsForEmails(env, feedMetadata?.emails ?? [], [
+      emailKey,
+    ]);
 
     if (feedMetadata) {
       feedMetadata.emails = feedMetadata.emails.filter(
@@ -644,13 +647,6 @@ emailsRouter.post("/emails/:emailKey/delete", async (c) => {
       await emailStorage.put(feedMetadataKey, JSON.stringify(feedMetadata));
     }
 
-    const attachmentBucket = getAttachmentBucket(env);
-    if (attachmentBucket && attachmentIds.length > 0) {
-      await Promise.allSettled(
-        attachmentIds.map((id) => attachmentBucket.delete(id)),
-      );
-    }
-
     if (wantsJson) return c.json({ ok: true, emailKey, feedId });
     return c.redirect(`/admin/feeds/${feedId}/emails`);
   } catch (error) {
@@ -714,13 +710,10 @@ emailsRouter.post("/feeds/:feedId/emails/bulk-delete", async (c) => {
       }
 
       const candidates = emailKeys.filter((key) => allowedKeys.has(key));
-      const candidateSet = new Set(candidates);
-      const r2AttachmentIds = feedMetadata.emails
-        .filter((e) => candidateSet.has(e.key))
-        .flatMap((e) => e.attachmentIds ?? []);
 
       const { ok: deletedOk, failed: failedEmailKeys } =
         await deleteKeysWithConcurrency(emailStorage, candidates, 35);
+      await deleteAttachmentsForEmails(env, feedMetadata.emails, candidates);
 
       const deletedSet = new Set(deletedOk);
       feedMetadata.emails = feedMetadata.emails.filter(
@@ -728,13 +721,6 @@ emailsRouter.post("/feeds/:feedId/emails/bulk-delete", async (c) => {
       );
       await emailStorage.put(feedMetadataKey, JSON.stringify(feedMetadata));
 
-      const attachmentBucket = getAttachmentBucket(env);
-      if (attachmentBucket && r2AttachmentIds.length > 0) {
-        await Promise.allSettled(
-          r2AttachmentIds.map((id) => attachmentBucket.delete(id)),
-        );
-      }
-
       return c.json({
         ok: failedEmailKeys.length === 0,
         deletedEmailKeys: deletedOk,
@@ -752,11 +738,13 @@ emailsRouter.post("/feeds/:feedId/emails/bulk-delete", async (c) => {
       return c.redirect(`/admin/feeds/${feedId}/emails?message=bulkDeleteNoop`);
 
     const candidates = emailKeys.filter((key) => allowedKeys.has(key));
+
     const { ok: deletedOk } = await deleteKeysWithConcurrency(
       emailStorage,
       candidates,
       35,
     );
+    await deleteAttachmentsForEmails(env, feedMetadata.emails, candidates);
 
     const deletedSet = new Set(deletedOk);
     feedMetadata.emails = feedMetadata.emails.filter(

src/routes/admin/helpers.ts

@@ -1,6 +1,34 @@
-import { EmailData, FeedList, FeedListItem, FeedMetadata } from "../../types";
+import {
+  EmailData,
+  EmailMetadata,
+  Env,
+  FeedList,
+  FeedListItem,
+  FeedMetadata,
+} from "../../types";
 import { FEEDS_LIST_KEY } from "../../config/constants";
 import { logger } from "../../lib/logger";
+import { getAttachmentBucket } from "../../utils/attachments";
+
+// Delete the R2 attachments belonging to the given email keys. Call before the
+// emails are removed from feed metadata, while `emails` still carries their
+// attachmentIds.
+export async function deleteAttachmentsForEmails(
+  env: Env,
+  emails: EmailMetadata[],
+  keys: Iterable<string>,
+): Promise<void> {
+  const keySet = new Set(keys);
+  const attachmentIds = emails
+    .filter((e) => keySet.has(e.key))
+    .flatMap((e) => e.attachmentIds ?? []);
+  if (attachmentIds.length === 0) return;
+
+  const bucket = getAttachmentBucket(env);
+  if (!bucket) return;
+
+  await Promise.allSettled(attachmentIds.map((id) => bucket.delete(id)));
+}
 
 export async function deleteKeysWithConcurrency(
   emailStorage: KVNamespace,