fix: count the forwards one message can collect against the redirect limit

The rule editor added up the forwards of all enabled rules against the
server's redirect limit, which applies per message. Rules run in order,
and a rule that stops ends the script for its messages, so a rule that
forwards and stops locked forwarding in every other rule, although no
message can reach both.

Count the most forwards one message can collect instead, and warn above
the rule list when the order, a switch or a drag lets one message
collect more than the limit.

Builds on the stop fix: without it, "Stop processing" after "Delete
silently" or "Reject" writes no stop.

components/filters/filter-rule-modal.tsx

@@ -19,6 +19,7 @@ import type { Mailbox } from "@/lib/jmap/types";
 import { buildMailboxTree, flattenMailboxTree, type MailboxNode, generateUUID, cn } from "@/lib/utils";
 import type { RuleSuggestion } from "@/lib/filters/quick-rules";
 import { retroactiveSupport } from "@/lib/filters/retroactive";
+import { ruleForwards, ruleStops } from "@/lib/filters/forward-limit";
 import { useSettingsStore } from "@/stores/settings-store";
 import { useKeywordFormat } from "@/hooks/use-keyword-format";
 
@@ -36,8 +37,13 @@ interface FilterRuleModalProps {
   mailboxes: Mailbox[];
   /** Server cap on redirects per message (Sieve `maxNumberRedirects`). */
   maxRedirects?: number | null;
-  /** Forward actions in the other enabled rules, which share that cap. */
-  otherForwards?: number;
+  /**
+   * Forwards a message can have collected when it reaches this rule, and
+   * the most it can still collect below it when the rule does not stop
+   * (see forwardsAround in lib/filters/forward-limit.ts).
+   */
+  forwardsBefore?: number;
+  forwardsAfter?: number;
   onSave: (rule: FilterRule, options?: { applyToExisting: boolean }) => void;
   onClose: () => void;
 }
@@ -107,7 +113,8 @@ export function FilterRuleModal({
   offerApplyToExisting = false,
   mailboxes,
   maxRedirects,
-  otherForwards = 0,
+  forwardsBefore = 0,
+  forwardsAfter = 0,
   onSave,
   onClose,
 }: FilterRuleModalProps) {
@@ -159,7 +166,11 @@ export function FilterRuleModal({
     return "";
   }, [mailboxPathMap]);
 
-  const forwardCount = actions.filter((a) => a.type === "forward").length;
+  // What counts is the most forwards one message can collect: those of the
+  // rules above that let it go on, this rule's, and those below unless this
+  // rule stops.
+  const forwardCount = ruleForwards({ actions });
+  const otherForwards = forwardsBefore + (ruleStops({ actions, stopProcessing }) ? 0 : forwardsAfter);
   const forwardLimit = typeof maxRedirects === "number" && maxRedirects > 0 ? maxRedirects : null;
   const forwardOverLimit = forwardLimit !== null && forwardCount + otherForwards > forwardLimit;
 

components/settings/__tests__/filter-settings-forward-limit.test.tsx

@@ -0,0 +1,153 @@
+import { fireEvent, render, screen, waitFor, within } from '@testing-library/react';
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
+import React from 'react';
+import type { FilterAction, FilterRule, SieveCapabilities } from '@/lib/jmap/sieve-types';
+import { useAuthStore } from '@/stores/auth-store';
+import { useEmailStore } from '@/stores/email-store';
+import { useFilterStore } from '@/stores/filter-store';
+import { FilterSettings } from '../filter-settings';
+
+// The redirect limit on the settings page: the real page and rule dialog,
+// with a server that allows one redirect per message, as Stalwart does.
+
+vi.mock('@/stores/toast-store', () => ({ toast: { error: vi.fn(), success: vi.fn(), info: vi.fn() } }));
+
+const CAPABILITIES: SieveCapabilities = {
+  implementation: 'Stalwart',
+  maxSizeScript: 102400,
+  sieveExtensions: ['fileinto', 'mailbox', 'imap4flags', 'copy'],
+  notificationMethods: [],
+  externalLists: [],
+  maxNumberRedirects: 1,
+};
+
+const forward: FilterAction = { type: 'forward', value: 'colleague@example.com' };
+const rule = (id: string, name: string, actions: FilterAction[], stopProcessing: boolean): FilterRule => ({
+  id,
+  name,
+  enabled: true,
+  matchType: 'all',
+  conditions: [{ field: 'from', comparator: 'contains', value: 'reports@vendor.example' }],
+  actions,
+  stopProcessing,
+});
+// Forwards to a colleague, deletes silently, and stops processing.
+const vendor = (stops = true) => rule('vendor', 'Vendor reports', [forward, { type: 'discard' }], stops);
+const everyoneElse = rule('rest', 'Everyone else', [{ type: 'forward', value: 'office@example.com' }], false);
+
+const initial = {
+  auth: useAuthStore.getState(),
+  email: useEmailStore.getState(),
+  filter: useFilterStore.getState(),
+};
+
+beforeEach(() => {
+  useAuthStore.setState({ client: { updateSieveScript: vi.fn(async () => {}), getSieveAccountId: () => 'primary' } as never });
+  useEmailStore.setState({ fetchMailboxes: vi.fn(async () => {}) as never });
+});
+
+afterEach(() => {
+  useAuthStore.setState(initial.auth, true);
+  useEmailStore.setState(initial.email, true);
+  useFilterStore.setState(initial.filter, true);
+});
+
+function openSettings(rules: FilterRule[]) {
+  useFilterStore.setState({
+    rules,
+    isSupported: true,
+    isLoading: false,
+    isOpaque: false,
+    activeScriptId: 'script-1',
+    selectedAccountId: 'primary',
+    sieveCapabilities: CAPABILITIES,
+    selectAccount: vi.fn(async () => {}),
+  });
+  render(<FilterSettings />);
+}
+
+const forwardOption = (dialog: ReturnType<typeof within>) =>
+  dialog.getAllByRole('combobox', { name: 'actions' })[0].querySelector('option[value="forward"]') as HTMLOptionElement;
+
+describe('the redirect limit on the settings page', () => {
+  it('offers forwarding in a new rule below one that forwards and stops', () => {
+    openSettings([vendor()]);
+    fireEvent.click(screen.getByText('add_rule'));
+    const dialog = within(screen.getByRole('dialog'));
+    expect(forwardOption(dialog).disabled).toBe(false);
+    expect(screen.queryByText('forward_limit')).toBeNull();
+  });
+
+  it('keeps forwarding locked below a rule that forwards and lets messages go on', () => {
+    openSettings([vendor(false)]);
+    fireEvent.click(screen.getByText('add_rule'));
+    expect(forwardOption(within(screen.getByRole('dialog'))).disabled).toBe(true);
+  });
+
+  it('says so in the list when the order lets one message collect too many', () => {
+    openSettings([everyoneElse, vendor()]);
+    expect(screen.getByText('forward_limit')).toBeInTheDocument();
+  });
+
+  it('says nothing in the list when the rule that stops comes first', () => {
+    openSettings([vendor(), everyoneElse]);
+    expect(screen.queryByText('forward_limit')).toBeNull();
+  });
+
+  it('says so in the list once a rule is switched on that lets one message collect too many', async () => {
+    openSettings([{ ...everyoneElse, enabled: false }, vendor()]);
+    expect(screen.queryByText('forward_limit')).toBeNull();
+    fireEvent.click(within(screen.getByRole('list', { name: 'rule_list' })).getAllByRole('switch')[0]);
+    // Saved, not put back.
+    await waitFor(() => expect(useFilterStore.getState().rawScript).toContain('redirect "office@example.com"'));
+    expect(screen.getByText('forward_limit')).toBeInTheDocument();
+  });
+
+  it('says so in the list once a rule is moved where one message collects too many', async () => {
+    openSettings([vendor(), everyoneElse]);
+    expect(screen.queryByText('forward_limit')).toBeNull();
+    const [vendorRow, restRow] = within(screen.getByRole('list', { name: 'rule_list' })).getAllByRole('listitem');
+    const dataTransfer = { setData: vi.fn(), effectAllowed: '', dropEffect: '' };
+    fireEvent.dragStart(restRow, { dataTransfer });
+    fireEvent.dragOver(vendorRow, { dataTransfer });
+    fireEvent.drop(vendorRow, { dataTransfer });
+    await waitFor(() => expect(useFilterStore.getState().rawScript).toMatch(/# Rule: Everyone else[\s\S]*# Rule: Vendor reports/));
+    expect(screen.getByText('forward_limit')).toBeInTheDocument();
+  });
+
+  it('counts the forward of an edited rule once', () => {
+    openSettings([vendor(), everyoneElse]);
+    fireEvent.click(screen.getByText('Everyone else'));
+    const dialog = within(screen.getByRole('dialog'));
+    expect(dialog.queryByText('forward_limit')).toBeNull();
+  });
+
+  it('does not count a forward behind a Stop action', () => {
+    const office: FilterAction = { type: 'forward', value: 'office@example.com' };
+    openSettings([rule('cut', 'Cut short', [forward, { type: 'stop' }, office], false)]);
+    fireEvent.click(screen.getByText('Cut short'));
+    expect(within(screen.getByRole('dialog')).queryByText('forward_limit')).toBeNull();
+  });
+
+  it('puts a new rule above the rules managed elsewhere', () => {
+    const archive: FilterAction = { type: 'forward', value: 'archive@example.org' };
+    openSettings([{ ...rule('ext', 'Managed elsewhere', [archive], true), origin: 'external' }]);
+    fireEvent.click(screen.getByText('add_rule'));
+    const dialog = within(screen.getByRole('dialog'));
+    // A message the new rule matches still reaches the one below it ...
+    expect(forwardOption(dialog).disabled).toBe(true);
+    // ... unless the new rule stops.
+    fireEvent.click(dialog.getByLabelText('stop_processing'));
+    expect(forwardOption(dialog).disabled).toBe(false);
+  });
+
+  it('warns in the dialog once the rule above stops no more', () => {
+    openSettings([vendor(), everyoneElse]);
+    fireEvent.click(screen.getByText('Vendor reports'));
+    const dialog = within(screen.getByRole('dialog'));
+    expect(dialog.queryByText('forward_limit')).toBeNull();
+    // Without its stop, a vendor report also reaches the rule below.
+    fireEvent.click(dialog.getByLabelText('stop_processing'));
+    expect(dialog.getByText('forward_limit')).toBeInTheDocument();
+  });
+});

components/settings/filter-settings.tsx

@@ -13,6 +13,7 @@ import { useSettingsStore } from "@/stores/settings-store";
 import { toast } from "@/stores/toast-store";
 import type { FilterRule } from "@/lib/jmap/sieve-types";
 import type { Mailbox } from "@/lib/jmap/types";
+import { forwardsAround, worstCaseForwards } from "@/lib/filters/forward-limit";
 import { useVacationStore } from "@/stores/vacation-store";
 import { useManagedAccountStore } from "@/stores/managed-account-store";
 import {
@@ -248,6 +249,20 @@ export function FilterSettings() {
   const [dragOverIndex, setDragOverIndex] = useState<number | null>(null);
   const draggedIndexRef = useRef<number | null>(null);
 
+  // A message can collect only so many forwards on this server. The rules run
+  // in the order listed, and one that stops ends the script for its messages,
+  // so the order counts (see lib/filters/forward-limit.ts).
+  const redirectLimit = sieveCapabilities?.maxNumberRedirects;
+  const tooManyForwards =
+    typeof redirectLimit === "number" && redirectLimit > 0 && worstCaseForwards(rules) > redirectLimit;
+  // The edited rule where it is; a new one goes below Bulwark's own rules.
+  const editedIndex = editingRule ? rules.findIndex((r) => r.id === editingRule.id) : -1;
+  const forwardsOfEdited = forwardsAround(
+    rules,
+    editedIndex >= 0 ? editedIndex : rules.filter((r) => !isReadonlyRule(r)).length,
+    editedIndex >= 0,
+  );
+
   useEffect(() => {
     if (client && isSupported) {
       // Always pass a concrete account id (managed shared account, or the
@@ -516,6 +531,12 @@ export function FilterSettings() {
           </div>
         )}
 
+        {!isOpaque && tooManyForwards && (
+          <p className="text-xs text-amber-600 dark:text-amber-400">
+            {t("forward_limit", { count: redirectLimit })}
+          </p>
+        )}
+
         {!isOpaque && rules.length > 0 && (
           <div className="space-y-1" role="list" aria-label={t("rule_list")}>
             {rules.map((rule, index) => {
@@ -702,9 +723,8 @@ export function FilterSettings() {
           rule={editingRule}
           mailboxes={mailboxes}
           maxRedirects={sieveCapabilities?.maxNumberRedirects}
-          otherForwards={rules
-            .filter((r) => r.enabled && r.id !== editingRule?.id)
-            .reduce((n, r) => n + r.actions.filter((a) => a.type === "forward").length, 0)}
+          forwardsBefore={forwardsOfEdited.before}
+          forwardsAfter={forwardsOfEdited.after}
           onSave={handleSaveRule}
           onClose={() => {
             setShowRuleModal(false);

lib/filters/__tests__/forward-limit.test.ts

@@ -0,0 +1,162 @@
+import { describe, expect, it } from 'vitest';
+import type { FilterAction, FilterRule } from '@/lib/jmap/sieve-types';
+import { generateScript } from '@/lib/sieve/generator';
+import { forwardsAround, ruleForwards, ruleStops, worstCaseForwards } from '../forward-limit';
+
+const forward: FilterAction = { type: 'forward', value: 'x@example.com' };
+const rule = (actions: FilterAction[], stopProcessing = false, enabled = true): FilterRule => ({
+  id: `r-${Math.random()}`,
+  name: 'R',
+  enabled,
+  matchType: 'all',
+  conditions: [{ field: 'from', comparator: 'contains', value: 'a' }],
+  actions,
+  stopProcessing,
+});
+
+// "Forward to a colleague, delete silently, stop processing", and a rule for
+// every other message that forwards too.
+const vendor = rule([forward, { type: 'discard' }], true);
+const everyoneElse = rule([forward]);
+
+describe('the forwards of one rule', () => {
+  it('counts those ahead of a Stop action', () => {
+    expect(ruleForwards(rule([forward, forward]))).toBe(2);
+    expect(ruleForwards(rule([forward, { type: 'stop' }, forward]))).toBe(1);
+    expect(ruleForwards(rule([{ type: 'mark_read' }]))).toBe(0);
+  });
+
+  it('knows a rule stops by its checkbox or a Stop action', () => {
+    expect(ruleStops(rule([forward], true))).toBe(true);
+    expect(ruleStops(rule([forward, { type: 'stop' }]))).toBe(true);
+    // Delete silently and Reject alone do not end the script.
+    expect(ruleStops(rule([forward, { type: 'discard' }]))).toBe(false);
+    expect(ruleStops(rule([{ type: 'reject', value: 'no' }]))).toBe(false);
+  });
+});
+
+describe('the most forwards one message can collect', () => {
+  it('stays at one when a rule that forwards stops ahead of another that forwards', () => {
+    expect(worstCaseForwards([vendor, everyoneElse])).toBe(1);
+  });
+
+  it('is two once the rule for every message comes first', () => {
+    // A message for the colleague meets both: the second forward would be skipped.
+    expect(worstCaseForwards([everyoneElse, vendor])).toBe(2);
+  });
+
+  it('adds up the rules that let a message go on', () => {
+    expect(worstCaseForwards([everyoneElse, rule([forward]), rule([forward])])).toBe(3);
+    expect(worstCaseForwards([rule([forward], true), rule([forward, forward], true)])).toBe(2);
+  });
+
+  it('leaves out rules that are off', () => {
+    expect(worstCaseForwards([rule([forward], false, false), vendor])).toBe(1);
+  });
+
+  it('is none without forwards', () => {
+    expect(worstCaseForwards([])).toBe(0);
+    expect(worstCaseForwards([rule([{ type: 'mark_read' }], true)])).toBe(0);
+  });
+});
+
+describe('the forwards around one rule', () => {
+  it('takes nothing from a rule above that stops, for a new rule below it', () => {
+    expect(forwardsAround([vendor], 1, false)).toEqual({ before: 0, after: 0 });
+  });
+
+  it('takes the forwards of a rule above that lets messages go on', () => {
+    const going = rule([forward, { type: 'discard' }]);
+    expect(forwardsAround([going], 1, false)).toEqual({ before: 1, after: 0 });
+  });
+
+  it('takes the worst case below an edited rule, without the rule itself', () => {
+    expect(forwardsAround([vendor, everyoneElse], 0, true)).toEqual({ before: 0, after: 1 });
+    expect(forwardsAround([everyoneElse, vendor], 1, true)).toEqual({ before: 1, after: 0 });
+  });
+
+  it('puts a new rule at the index, keeping the rule there below it', () => {
+    expect(forwardsAround([everyoneElse, vendor], 1, false)).toEqual({ before: 1, after: 1 });
+  });
+});
+
+describe('the count and the script', () => {
+  it('agree on the forwards a rule runs and whether it stops', () => {
+    const variants: FilterAction[][] = [
+      [forward],
+      [forward, { type: 'discard' }],
+      [forward, { type: 'reject', value: 'no' }],
+      [forward, { type: 'move', value: 'Archive' }],
+      [{ type: 'mark_read' }, forward, forward],
+      [forward, { type: 'stop' }, forward],
+      [{ type: 'stop' }, forward],
+    ];
+    for (const actions of variants) {
+      for (const stopProcessing of [false, true]) {
+        const r = rule(actions, stopProcessing);
+        const lines = generateScript([r]).split('# Rule:')[1].split('\n').map((line) => line.trim());
+        const stop = lines.indexOf('stop;');
+        const runs = stop === -1 ? lines : lines.slice(0, stop);
+        const label = `${actions.map((a) => a.type).join(', ')}${stopProcessing ? ', stop processing' : ''}`;
+        expect(runs.filter((line) => line.startsWith('redirect ')).length, label).toBe(ruleForwards(r));
+        expect(stop !== -1, label).toBe(ruleStops(r));
+      }
+    }
+  });
+});
+
+describe('against every set of rules a message can match', () => {
+  // A message that matches the rules in `matched`: the forwards it collects,
+  // and the rules that run for it.
+  function walk(rules: FilterRule[], matched: Set<number>): { forwards: number; ran: Set<number> } {
+    let forwards = 0;
+    const ran = new Set<number>();
+    for (let i = 0; i < rules.length; i++) {
+      if (!rules[i].enabled || !matched.has(i)) continue;
+      ran.add(i);
+      forwards += ruleForwards(rules[i]);
+      if (ruleStops(rules[i])) break;
+    }
+    return { forwards, ran };
+  }
+  // The most forwards one message can collect; with `runs`, one the rule
+  // there runs for.
+  function mostCollected(rules: FilterRule[], runs?: number): number {
+    let most = 0;
+    for (let mask = 0; mask < 1 << rules.length; mask++) {
+      const matched = new Set(rules.map((_, i) => i).filter((i) => mask & (1 << i)));
+      const { forwards, ran } = walk(rules, matched);
+      if (runs === undefined || ran.has(runs)) most = Math.max(most, forwards);
+    }
+    return most;
+  }
+
+  // Fixed seed: the same rule lists on every run.
+  let seed = 7;
+  const random = () => (seed = (seed * 16807) % 2147483647) / 2147483647;
+  const pool: FilterAction[][] = [
+    [], [forward], [forward, forward], [forward, { type: 'discard' }],
+    [forward, { type: 'stop' }], [{ type: 'stop' }, forward], [{ type: 'mark_read' }],
+  ];
+  const lists = Array.from({ length: 300 }, () =>
+    Array.from({ length: Math.floor(random() * 7) }, () =>
+      rule(pool[Math.floor(random() * pool.length)], random() < 0.4, random() < 0.8)));
+
+  it('finds the most forwards one message can collect', () => {
+    for (const rules of lists) expect(worstCaseForwards(rules)).toBe(mostCollected(rules));
+  });
+
+  it('finds the most a message the rule runs for can collect, for rules in place and new ones', () => {
+    for (const rules of lists) {
+      for (let i = 0; i < rules.length; i++) {
+        const r = rules[i];
+        if (!r.enabled) continue;
+        const { before, after } = forwardsAround(rules, i, true);
+        expect(before + ruleForwards(r) + (ruleStops(r) ? 0 : after)).toBe(mostCollected(rules, i));
+        // The same rule about to be put there, rather than there already.
+        const others = rules.filter((_, j) => j !== i);
+        expect(forwardsAround(others, i, false)).toEqual({ before, after });
+      }
+    }
+  });
+});

lib/filters/forward-limit.ts

@@ -0,0 +1,63 @@
+import type { FilterRule } from '@/lib/jmap/sieve-types';
+
+/*
+ * A server lets one message trigger only so many redirects
+ * (maxNumberRedirects, 1 for user scripts on Stalwart by default) and skips
+ * the ones over that. The rules run in order, and a rule that stops ("Stop
+ * processing", or the Stop action) ends the script for the messages it
+ * matches: the forwards of the rules below it never add to its own. What
+ * counts is the most forwards a single message can collect, not how many
+ * forwards there are.
+ */
+
+type RuleForwards = Pick<FilterRule, 'enabled' | 'actions' | 'stopProcessing'>;
+
+/** The forwards of a rule that run: those ahead of a Stop action. */
+export function ruleForwards(rule: Pick<FilterRule, 'actions'>): number {
+  let forwards = 0;
+  for (const action of rule.actions) {
+    if (action.type === 'stop') break;
+    if (action.type === 'forward') forwards++;
+  }
+  return forwards;
+}
+
+/** The rule ends the script for the messages it matches. */
+export function ruleStops(rule: Pick<FilterRule, 'actions' | 'stopProcessing'>): boolean {
+  return rule.stopProcessing || rule.actions.some((action) => action.type === 'stop');
+}
+
+/** The most forwards one message can collect going through `rules`, in their order. */
+export function worstCaseForwards(rules: RuleForwards[]): number {
+  // From the rules so far that let the message go on.
+  let open = 0;
+  let worst = 0;
+  for (const rule of rules) {
+    if (!rule.enabled) continue;
+    const forwards = ruleForwards(rule);
+    if (ruleStops(rule)) worst = Math.max(worst, open + forwards);
+    else open += forwards;
+  }
+  return Math.max(worst, open);
+}
+
+/**
+ * For the rule at `index` of `rules` (in the order they run): the forwards a
+ * message can have collected when it reaches the rule, and the most it can
+ * still collect below it when the rule lets it go on. `replaces`: the rule
+ * is at `index` already, rather than about to be put there.
+ */
+export function forwardsAround(
+  rules: RuleForwards[],
+  index: number,
+  replaces: boolean,
+): { before: number; after: number } {
+  // A rule above that stops lets only the messages it does not match go on,
+  // so its forwards never meet this rule's.
+  const before = rules
+    .slice(0, index)
+    .filter((rule) => rule.enabled && !ruleStops(rule))
+    .reduce((n, rule) => n + ruleForwards(rule), 0);
+  const after = worstCaseForwards(rules.slice(replaces ? index + 1 : index));
+  return { before, after };
+}