Take the filing action, not the whole rule

Deleting a folder removed every rule that filed into it, along with
whatever else those rules did. A rule that filed into Work, marked read
and stopped processing lost the marking and the stopping too, and
deleting a folder says nothing about whether those were still wanted.

Only the fileinto action goes now. A rule left with nothing to do is
still removed, because it has nothing to do; a rule filing into two
folders keeps the one that still exists. The toast says which happened.

Verified against the running app with two rules aimed at the same
folder, one filing only and one filing and marking read: the first was
removed, the second kept its markread, and the script stored on the
server agrees.

web/src/lib/__tests__/sieveFolders.test.ts

@@ -1,5 +1,5 @@
 import { describe, expect, it } from "vitest";
-import { retargetRules, dropRulesForFolders } from "../sieveFolders";
+import { retargetRules, detachFolders } from "../sieveFolders";
 import { newRule, type SieveRule } from "../sieve";
 
 /**
@@ -56,30 +56,53 @@ describe("retargetRules", () => {
   });
 });
 
-describe("dropRulesForFolders", () => {
-  it("removes a rule whose destination is gone", () => {
+describe("detachFolders", () => {
+  it("removes only the filing action, leaving the rest of the rule doing its job", () => {
+    const rules = [rule("news", fileinto("Newsletters", "mb1", [{ type: "markread" }, { type: "stop" }]))];
+    const out = detachFolders(rules, [{ id: "mb1", path: "Newsletters" }]);
+    expect(out.removed).toEqual([]);
+    expect(out.edited).toHaveLength(1);
+    expect(out.rules[0]!.actions.map((a) => a.type)).toEqual(["markread", "stop"]);
+  });
+
+  it("removes the rule when filing was all it did", () => {
     const rules = [rule("news", fileinto("Newsletters", "mb1")), rule("keep", fileinto("Archive", "mb9"))];
-    const out = dropRulesForFolders(rules, [{ id: "mb1", path: "Newsletters" }]);
+    const out = detachFolders(rules, [{ id: "mb1", path: "Newsletters" }]);
     expect(out.removed.map((r) => r.name)).toEqual(["news"]);
     expect(out.rules.map((r) => r.name)).toEqual(["keep"]);
   });
 
-  it("removes rules for a deleted folder's children too", () => {
-    const rules = [rule("a", fileinto("Work", "mb1")), rule("b", fileinto("Work/Invoices", "mb2"))];
-    const out = dropRulesForFolders(rules, [{ id: "mb1", path: "Work" }, { id: "mb2", path: "Work/Invoices" }]);
-    expect(out.rules).toEqual([]);
-    expect(out.removed).toHaveLength(2);
+  it("handles a deleted folder's children too", () => {
+    const rules = [
+      rule("a", fileinto("Work", "mb1")),
+      rule("b", fileinto("Work/Invoices", "mb2", [{ type: "flag" }])),
+    ];
+    const out = detachFolders(rules, [{ id: "mb1", path: "Work" }, { id: "mb2", path: "Work/Invoices" }]);
+    expect(out.removed.map((r) => r.name)).toEqual(["a"]);
+    expect(out.rules.map((r) => r.name)).toEqual(["b"]);
+    expect(out.rules[0]!.actions.map((a) => a.type)).toEqual(["flag"]);
   });
 
   it("still finds the rule when only the path matches", () => {
     const rules = [rule("news", fileinto("Newsletters"))];
-    expect(dropRulesForFolders(rules, [{ id: "mb1", path: "NEWSLETTERS" }]).removed).toHaveLength(1);
+    expect(detachFolders(rules, [{ id: "mb1", path: "NEWSLETTERS" }]).removed).toHaveLength(1);
+  });
+
+  it("keeps a second filing action aimed somewhere that still exists", () => {
+    const rules = [rule("both", [
+      { type: "fileinto", mailbox: "Newsletters", mailboxId: "mb1" },
+      { type: "fileinto", mailbox: "Archive", mailboxId: "mb9", copy: true },
+    ])];
+    const out = detachFolders(rules, [{ id: "mb1", path: "Newsletters" }]);
+    expect(out.removed).toEqual([]);
+    expect(out.rules[0]!.actions).toEqual([{ type: "fileinto", mailbox: "Archive", mailboxId: "mb9", copy: true }]);
   });
 
   it("leaves the list untouched when nothing matches", () => {
     const rules = [rule("keep", fileinto("Archive", "mb9"))];
-    const out = dropRulesForFolders(rules, [{ id: "mb1", path: "Newsletters" }]);
+    const out = detachFolders(rules, [{ id: "mb1", path: "Newsletters" }]);
     expect(out.rules).toBe(rules);
+    expect(out.edited).toEqual([]);
     expect(out.removed).toEqual([]);
   });
 });

web/src/lib/sieveFolders.ts

@@ -47,16 +47,35 @@ export function retargetRules(rules: SieveRule[], moves: Array<FolderRef & { new
 }
 
 /**
- * Drops rules that file into folders which no longer exist.
+ * Takes the deleted folders out of the rules that filed into them.
  *
- * The whole rule goes, not just its fileinto action: a rule whose destination
- * has been deleted has no destination, and leaving it behind to match mail and
- * do nothing is worse than removing it. Rules that merely mention the folder in
- * some other action are left alone.
+ * Only the `fileinto` action goes. A rule that also marks read, flags, or stops
+ * processing keeps doing those things — deleting a folder says nothing about
+ * whether the rest of the rule was still wanted. A rule left with no actions at
+ * all has nothing to do, so that one goes.
  */
-export function dropRulesForFolders(rules: SieveRule[], gone: FolderRef[]): { rules: SieveRule[]; removed: SieveRule[] } {
-  if (!gone.length) return { rules, removed: [] };
-  const removed = rules.filter((r) => gone.some((ref) => filesInto(r, ref)));
-  if (!removed.length) return { rules, removed: [] };
-  return { rules: rules.filter((r) => !removed.includes(r)), removed };
+export function detachFolders(rules: SieveRule[], gone: FolderRef[]): { rules: SieveRule[]; edited: SieveRule[]; removed: SieveRule[] } {
+  if (!gone.length) return { rules, edited: [], removed: [] };
+  const targets = (a: SieveRule["actions"][number]) =>
+    a.type === "fileinto" && gone.some((ref) => a.mailboxId === ref.id || a.mailbox.toLowerCase() === ref.path.toLowerCase());
+
+  const edited: SieveRule[] = [];
+  const removed: SieveRule[] = [];
+  const next: SieveRule[] = [];
+  for (const rule of rules) {
+    if (!rule.actions.some(targets)) {
+      next.push(rule);
+      continue;
+    }
+    const actions = rule.actions.filter((a) => !targets(a));
+    if (!actions.length) {
+      removed.push(rule);
+      continue;
+    }
+    const trimmed = { ...rule, actions };
+    edited.push(trimmed);
+    next.push(trimmed);
+  }
+  if (!edited.length && !removed.length) return { rules, edited: [], removed: [] };
+  return { rules: next, edited, removed };
 }

web/src/store/mail.ts

@@ -1069,16 +1069,18 @@ async function followFolders(before: FolderRef[]): Promise<void> {
       else gone.push(ref);
     }
 
-    const { retargetRules, dropRulesForFolders } = await import("@/lib/sieveFolders");
+    const { retargetRules, detachFolders } = await import("@/lib/sieveFolders");
     const retargeted = retargetRules(rules, moves);
-    const dropped = dropRulesForFolders(retargeted.rules, gone);
-    if (!retargeted.changed && !dropped.removed.length) return;
+    const detached = detachFolders(retargeted.rules, gone);
+    if (!retargeted.changed && !detached.edited.length && !detached.removed.length) return;
 
-    await useSieve.getState().saveRules(dropped.rules);
+    await useSieve.getState().saveRules(detached.rules);
     const { toast } = await import("@/ui/toast");
+    const plural = (n: number) => (n === 1 ? "" : "s");
     const said: string[] = [];
-    if (retargeted.changed) said.push(`${retargeted.changed} filter rule${retargeted.changed === 1 ? "" : "s"} updated`);
-    if (dropped.removed.length) said.push(`${dropped.removed.length} filter rule${dropped.removed.length === 1 ? "" : "s"} removed: ${dropped.removed.map((r) => `“${r.name}”`).join(", ")}`);
+    if (retargeted.changed) said.push(`${retargeted.changed} filter rule${plural(retargeted.changed)} updated`);
+    if (detached.edited.length) said.push(`${detached.edited.length} filter rule${plural(detached.edited.length)} no longer file${detached.edited.length === 1 ? "s" : ""} there`);
+    if (detached.removed.length) said.push(`${detached.removed.length} filter rule${plural(detached.removed.length)} removed, having nothing left to do: ${detached.removed.map((r) => `“${r.name}”`).join(", ")}`);
     toast.show(said.join(" · "), { duration: 8000 });
   } catch (err) {
     const { toast } = await import("@/ui/toast");