fix: leave trash and spam out of tag views and tag counts, and count tags with Keyword/get where the server can (bulwarkmail/webmail#1156)

src/api/__tests__/live-stalwart-mail.test.ts

@@ -41,6 +41,7 @@ import {
 } from '../email';
 import { senderWarningOf } from '../../lib/email-headers';
 import { scheduledEmailIdsInThread } from '../thread-scheduled';
+import { fetchTagCounts, KEYWORDS_CAPABILITY } from '../tag-counts';
 import { uploadBytes } from '../blob';
 import { getIdentities, updateIdentity } from '../identity';
 import { isJMAPSetError } from '../jmap-result';
@@ -466,5 +467,28 @@ live('live Stalwart: mail, contacts, search (0.16 and 1.0)', () => {
       const [filed] = await getFullEmails([result.emailId!]);
       expect((await scheduledEmailIdsInThread(filed.threadId)).size).toBe(0);
     }, 90_000);
+
+    it('leaves Trash out of the tag counts (bulwarkmail/webmail#1156)', async (ctx) => {
+      const boxes = await getMailboxes();
+      const trash = boxes.find((b) => b.role === 'trash')!.id;
+      const tag = 'readerpass';
+      const keyword = `$label:${tag}`;
+      const live = await importMessage(scratch, `tag live ${mailTag}`, 'a');
+      const gone = await importMessage(trash, `tag deleted ${mailTag}`, 'b', { $seen: true });
+      // Set afterwards: 0.16 drops the keywords Email/import is given.
+      await patchKeywordsForEmails([live, gone], { [keyword]: true });
+      const flags = await getEmailFlags([live]);
+      if (!flags.list[0]?.keywords?.[keyword]) {
+        // Seen on the 0.16.27 test server: Email/set answers "updated" but
+        // keeps only system keywords for this account. Nothing to count.
+        ctx.skip('the server did not keep the tag keyword');
+        return;
+      }
+      const all = await eventually(() => fetchTagCounts([tag]), ([c]) => c.total === 2);
+      expect(all).toEqual([{ id: tag, total: 2, unread: 1 }]);
+      expect(await fetchTagCounts([tag], [undefined], { '': [trash] })).toEqual([{ id: tag, total: 1, unread: 1 }]);
+      // Stalwart answers Keyword/get for no keyword counters (no capability).
+      expect(client.currentSession?.capabilities?.[KEYWORDS_CAPABILITY]).toBeUndefined();
+    }, 60_000);
   });
 });

src/api/__tests__/tag-counts.test.ts

@@ -5,11 +5,13 @@ vi.mock('../jmap-client', () => ({
     accountId: 'c',
     request: vi.fn(),
     getMaxCallsInRequest: vi.fn(() => 16),
+    currentSession: { capabilities: {} as Record<string, unknown> },
+    hasAccountCapability: vi.fn(() => true),
   },
 }));
 
 import { jmapClient } from '../jmap-client';
-import { fetchTagCounts } from '../tag-counts';
+import { fetchTagCounts, KEYWORDS_CAPABILITY } from '../tag-counts';
 
 const mockRequest = jmapClient.request as ReturnType<typeof vi.fn>;
 const mockMaxCalls = jmapClient.getMaxCallsInRequest as ReturnType<typeof vi.fn>;
@@ -30,6 +32,7 @@ function answer(totals: Record<string, { total: number; unread: number }>, faili
 beforeEach(() => {
   vi.clearAllMocks();
   mockMaxCalls.mockReturnValue(16);
+  jmapClient.currentSession!.capabilities = {};
 });
 
 describe('fetchTagCounts', () => {
@@ -39,11 +42,11 @@ describe('fetchTagCounts', () => {
     expect(await fetchTagCounts(['red'])).toEqual([{ id: 'red', total: 2, unread: 1 }]);
     const calls = mockRequest.mock.calls[0][0];
     expect(calls).toEqual([
-      ['Email/query', { accountId: 'c', filter: { hasKeyword: '$label:red' }, limit: 0, calculateTotal: true }, 't0:red'],
+      ['Email/query', { accountId: 'c', filter: { hasKeyword: '$label:red' }, limit: 1, calculateTotal: true }, 't0:red'],
       ['Email/query', {
         accountId: 'c',
         filter: { operator: 'AND', conditions: [{ hasKeyword: '$label:red' }, { notKeyword: '$seen' }] },
-        limit: 0,
+        limit: 1,
         calculateTotal: true,
       }, 'u0:red'],
     ]);
@@ -77,4 +80,53 @@ describe('fetchTagCounts', () => {
 
     expect(await fetchTagCounts(['red'], [undefined, 'team'])).toEqual([{ id: 'red', total: 2, unread: 1 }]);
   });
+
+  // The tag view leaves Trash and Junk out; its badge counted them (#1156).
+  it("leaves each account's Trash and Junk out of the counts", async () => {
+    mockRequest.mockImplementation(async (calls: Array<[string, Record<string, unknown>, string]>) => ({
+      methodResponses: calls.map(([, , callId]) => ['Email/query', { ids: [], total: 1 }, callId]),
+    }));
+
+    await fetchTagCounts(['red'], [undefined, 'team'], { '': ['trash', 'junk'], team: ['t-trash'] });
+
+    const filters = mockRequest.mock.calls[0][0].map((c: unknown[]) => (c[1] as { filter: unknown }).filter);
+    expect(filters).toEqual([
+      { operator: 'AND', conditions: [{ hasKeyword: '$label:red' }, { inMailboxOtherThan: ['trash', 'junk'] }] },
+      { operator: 'AND', conditions: [{ hasKeyword: '$label:red' }, { notKeyword: '$seen' }, { inMailboxOtherThan: ['trash', 'junk'] }] },
+      { operator: 'AND', conditions: [{ hasKeyword: '$label:red' }, { inMailboxOtherThan: ['t-trash'] }] },
+      { operator: 'AND', conditions: [{ hasKeyword: '$label:red' }, { notKeyword: '$seen' }, { inMailboxOtherThan: ['t-trash'] }] },
+    ]);
+  });
+
+  // One Keyword/get where the server counts keywords (#1399), else the queries.
+  it('asks Keyword/get when the server counts keywords, and falls back to queries when it fails', async () => {
+    jmapClient.currentSession!.capabilities = { [KEYWORDS_CAPABILITY]: { supportsCounts: true } };
+    mockRequest.mockImplementation(async (calls: Array<[string, Record<string, unknown>, string]>) => {
+      if (calls[0][0] === 'Keyword/get') {
+        return { methodResponses: [['Keyword/get', { list: [{ id: '$LABEL:Red', total: 7, unread: 3 }] }, '0']] };
+      }
+      return { methodResponses: calls.map(([, , callId]) => ['Email/query', { ids: [], total: 1 }, callId]) };
+    });
+
+    expect(await fetchTagCounts(['red', 'blue'])).toEqual([
+      { id: 'red', total: 7, unread: 3 },
+      { id: 'blue', total: 0, unread: 0 },
+    ]);
+    expect(mockRequest).toHaveBeenCalledTimes(1);
+    expect(mockRequest.mock.calls[0][0][0]).toEqual(['Keyword/get', { accountId: 'c', ids: ['$label:red', '$label:blue'] }, '0']);
+
+    mockRequest.mockClear();
+    mockRequest.mockImplementation(async (calls: Array<[string, Record<string, unknown>, string]>) => (
+      calls[0][0] === 'Keyword/get'
+        ? { methodResponses: [['error', { type: 'unknownMethod' }, '0']] }
+        : { methodResponses: calls.map(([, , callId]) => ['Email/query', { ids: [], total: 2 }, callId]) }
+    ));
+    expect(await fetchTagCounts(['red'])).toEqual([{ id: 'red', total: 2, unread: 2 }]);
+
+    // Without supportsCounts nothing asks Keyword/get.
+    jmapClient.currentSession!.capabilities = { [KEYWORDS_CAPABILITY]: {} };
+    mockRequest.mockClear();
+    await fetchTagCounts(['red']);
+    expect(mockRequest.mock.calls[0][0][0][0]).toBe('Email/query');
+  });
 });

src/api/tag-counts.ts

@@ -1,5 +1,8 @@
 import { jmapClient } from './jmap-client';
-import type { JMAPMethodCall } from './types';
+import { CAPABILITIES, type JMAPMethodCall } from './types';
+
+/** Bulwark's keyword counters (webmail lib/jmap/client.ts KEYWORDS_CAPABILITY). */
+export const KEYWORDS_CAPABILITY = 'https://bulwarkmail.com/ns/jmap/keywords';
 
 export interface TagCount {
   /** Tag id (the part after `$label:`). */
@@ -17,33 +20,61 @@ export interface TagCount {
  * allows. Port of the webmail's `fetchTagCounts` (stores/email-store.ts). A
  * call that fails, such as for an account that can't be reached, counts 0
  * rather than sinking the rest.
+ *
+ * `excluded` names each account's Trash and Junk (key '' = the own account):
+ * the counts leave them out like the tag view does (webmail #1156). An
+ * account whose server counts keywords itself (Bulwark's keywords capability
+ * with `supportsCounts`) is asked once with Keyword/get instead (#1399); its
+ * counters cover the whole account, as the webmail's do.
  */
 export async function fetchTagCounts(
   tagIds: string[],
   accountIds: Array<string | undefined> = [undefined],
+  excluded: Record<string, string[]> = {},
 ): Promise<TagCount[]> {
   if (tagIds.length === 0) return [];
   const perRequest = Math.max(2, jmapClient.getMaxCallsInRequest());
   const counts = new Map<string, TagCount>(tagIds.map((id) => [id, { id, total: 0, unread: 0 }]));
 
+  const queried: Array<string | undefined> = [];
+  for (const override of accountIds) {
+    const counted = countsKeywords(override ?? jmapClient.accountId)
+      ? await tagCountsFromKeywords(tagIds, override ?? jmapClient.accountId)
+      : null;
+    if (!counted) {
+      queried.push(override);
+      continue;
+    }
+    for (const c of counted) {
+      const entry = counts.get(c.id)!;
+      entry.total += c.total;
+      entry.unread += c.unread;
+    }
+  }
+
   const calls: JMAPMethodCall[] = [];
   const targets = new Map<string, { id: string; kind: 'total' | 'unread' }>();
-  accountIds.forEach((override, index) => {
+  queried.forEach((override, index) => {
     const accountId = override ?? jmapClient.accountId;
+    const skip = excluded[override ?? ''] ?? [];
+    const exclusion = skip.length > 0 ? [{ inMailboxOtherThan: skip }] : [];
     for (const id of tagIds) {
       const keyword = `$label:${id}`;
       targets.set(`t${index}:${id}`, { id, kind: 'total' });
       calls.push(['Email/query', {
         accountId,
-        filter: { hasKeyword: keyword },
-        limit: 0,
+        filter: exclusion.length > 0
+          ? { operator: 'AND', conditions: [{ hasKeyword: keyword }, ...exclusion] }
+          : { hasKeyword: keyword },
+        // 1, not 0: Stalwart takes 0 as "no limit" and returns every id.
+        limit: 1,
         calculateTotal: true,
       }, `t${index}:${id}`]);
       targets.set(`u${index}:${id}`, { id, kind: 'unread' });
       calls.push(['Email/query', {
         accountId,
-        filter: { operator: 'AND', conditions: [{ hasKeyword: keyword }, { notKeyword: '$seen' }] },
-        limit: 0,
+        filter: { operator: 'AND', conditions: [{ hasKeyword: keyword }, { notKeyword: '$seen' }, ...exclusion] },
+        limit: 1,
         calculateTotal: true,
       }, `u${index}:${id}`]);
     }
@@ -63,3 +94,30 @@ export async function fetchTagCounts(
   }
   return tagIds.map((id) => counts.get(id)!);
 }
+
+/** Whether the server reports every keyword's total and unread in Keyword/get. */
+function countsKeywords(accountId: string): boolean {
+  const capability = jmapClient.currentSession?.capabilities?.[KEYWORDS_CAPABILITY] as { supportsCounts?: unknown } | undefined;
+  return capability?.supportsCounts === true && jmapClient.hasAccountCapability(KEYWORDS_CAPABILITY, accountId);
+}
+
+/** Counts from the server's keyword counters, or null so the caller queries instead. */
+async function tagCountsFromKeywords(tagIds: string[], accountId: string): Promise<TagCount[] | null> {
+  try {
+    const res = await jmapClient.request(
+      [['Keyword/get', { accountId, ids: tagIds.map((id) => `$label:${id}`) }, '0']],
+      [CAPABILITIES.CORE, KEYWORDS_CAPABILITY],
+    );
+    const [name, body] = res.methodResponses?.[0] ?? [];
+    if (name !== 'Keyword/get' || !Array.isArray(body?.list)) return null;
+    // Keywords are case-insensitive (RFC 8621 4.1.1).
+    const byId = new Map((body.list as Array<{ id: string; total?: number; unread?: number }>)
+      .map((k) => [k.id.toLowerCase(), k]));
+    return tagIds.map((id) => {
+      const k = byId.get(`$label:${id}`.toLowerCase());
+      return { id, total: k?.total ?? 0, unread: k?.unread ?? 0 };
+    });
+  } catch {
+    return null;
+  }
+}

src/components/SidebarDrawer.tsx

@@ -15,7 +15,7 @@ import {
 import { spacing, radius, typography, type ThemePalette } from '../theme/tokens';
 import { useColors } from '../theme/colors';
 import { useAnimDuration } from '../theme/dynamic';
-import { useEmailStore, spannedAccounts } from '../stores/email-store';
+import { useEmailStore, spannedAccounts, trashAndJunkIds } from '../stores/email-store';
 import { useAuthStore } from '../stores/auth-store';
 import { useAccountStore } from '../stores/account-store';
 import { useSettingsStore } from '../stores/settings-store';
@@ -462,8 +462,10 @@ export default function SidebarDrawer({ visible, onClose, docked = false }: Side
   React.useEffect(() => {
     if (!visible || !tagsExpanded || keywordDefs.length === 0 || !activeAccountId || !jmapClient.isConnected) return;
     const accounts = tagAccountsKey.split('|').map((id) => id || undefined);
-    void ensureTagCounts(activeAccountId, keywordDefs.map((k) => k.id), accounts);
-  }, [visible, tagsExpanded, keywordDefs, tagAccountsKey, activeAccountId, tagCountsGeneration, ensureTagCounts]);
+    // Trash and Junk stay out of the counts as they do of the tag view (#1156).
+    const excluded = Object.fromEntries(accounts.map((id) => [id ?? '', trashAndJunkIds(mailboxes, id)]));
+    void ensureTagCounts(activeAccountId, keywordDefs.map((k) => k.id), accounts, excluded);
+  }, [visible, tagsExpanded, keywordDefs, tagAccountsKey, mailboxes, activeAccountId, tagCountsGeneration, ensureTagCounts]);
 
   // ── Folder actions (long-press) ───────────────────────────────────────
   const refFor = (mb: Mailbox) => ({

src/stores/__tests__/email-store-all-folders.test.ts

@@ -626,6 +626,8 @@ describe('tag view across the own and the team account (#1038)', () => {
         mail('o1', 'inbox', 20, 'own red 1', { keywords: { [RED]: true } }),
         mail('o2', 'archive', 10, 'own red 2', { keywords: { [RED]: true, $seen: true } }),
         mail('o3', 'inbox', 21, 'own plain'),
+        // Deleted, but it keeps its keywords: not listed under the tag (#1156).
+        mail('o4', 'trash', 24, 'own red deleted', { keywords: { [RED]: true } }),
       ],
       team: [
         mail('m1', 't-inbox', 22, 'team red 1', { keywords: { [RED]: true } }),
@@ -647,9 +649,10 @@ describe('tag view across the own and the team account (#1038)', () => {
     expect(ids()).toEqual(['m1', 'o1', 'm2', 'o2']);
     expect(server.current!.requests).toHaveLength(1);
     const queries = server.current!.callsOf('Email/query');
+    // Each account leaves its own Trash and Junk out (webmail #1156).
     expect(queries.map(([, a]) => [a.accountId, a.filter])).toEqual([
-      ['c', { hasKeyword: RED }],
-      ['team', { hasKeyword: RED }],
+      ['c', { operator: 'AND', conditions: [{ hasKeyword: RED }, { inMailboxOtherThan: ['trash', 'junk'] }] }],
+      ['team', { operator: 'AND', conditions: [{ hasKeyword: RED }, { inMailboxOtherThan: ['t-trash', 't-junk'] }] }],
     ]);
   });
 
@@ -669,8 +672,8 @@ describe('tag view across the own and the team account (#1038)', () => {
 
     const last = server.current!.requests.at(-1)!.filter(([n]) => n === 'Email/query');
     expect(last.map(([, a]) => a.filter)).toEqual([
-      { operator: 'AND', conditions: [{ text: 'red 2' }, { hasKeyword: RED }] },
-      { operator: 'AND', conditions: [{ text: 'red 2' }, { hasKeyword: RED }] },
+      { operator: 'AND', conditions: [{ text: 'red 2' }, { hasKeyword: RED }, { inMailboxOtherThan: ['trash', 'junk'] }] },
+      { operator: 'AND', conditions: [{ text: 'red 2' }, { hasKeyword: RED }, { inMailboxOtherThan: ['t-trash', 't-junk'] }] },
     ]);
   });
 
@@ -689,7 +692,7 @@ describe('tag view across the own and the team account (#1038)', () => {
         const last = server.current!.requests.at(-1)!.filter(([n]) => n === 'Email/query');
         expect(last[0][1].filter).toEqual({
           operator: 'AND',
-          conditions: [{ text: '"red 2"' }, { text: 'notes' }, { hasKeyword: RED }],
+          conditions: [{ text: '"red 2"' }, { text: 'notes' }, { hasKeyword: RED }, { inMailboxOtherThan: ['trash', 'junk'] }],
         });
       });
       await vi.waitFor(() => expect(useEmailStore.getState().loading).toBe(false));

src/stores/email-store.ts

@@ -303,9 +303,12 @@ export function searchSkipsTrashAndJunk(
   filters: EmailFilters,
   openRole?: string | null,
 ): boolean {
-  if (filters.keyword) return false;
   if (filters.folder === SCOPE_EXCEPT_TRASH_JUNK) return true;
   if (filters.folder) return false;
+  // A tag view leaves Trash and Junk out, as Gmail's label views do: a
+  // deleted message keeps its keywords and stayed listed under the tag
+  // (webmail #1156). It is still in the Trash folder itself.
+  if (filters.keyword) return true;
   return effectiveFolderScope(searchQuery, filters, openRole) === 'all';
 }
 
@@ -626,7 +629,7 @@ export function spansAccounts(
 }
 
 // The raw JMAP ids of one account's Trash and Junk (undefined = the user's own).
-function trashAndJunkIds(mailboxes: Mailbox[], accountId: string | undefined): string[] {
+export function trashAndJunkIds(mailboxes: Mailbox[], accountId: string | undefined): string[] {
   return accountMailboxes(mailboxes, accountId)
     .filter((m) => isTrashOrJunkRole(m.role))
     .map((m) => m.originalId ?? m.id);

src/stores/tag-counts-store.ts

@@ -20,7 +20,13 @@ interface TagCountsState {
   /** `generation` the counts were fetched at. */
   fetchedAt: number;
   /** Fetch the counts unless the cached ones are current. */
-  ensure: (login: string, tagIds: string[], accountIds: Array<string | undefined>) => Promise<void>;
+  ensure: (
+    login: string,
+    tagIds: string[],
+    accountIds: Array<string | undefined>,
+    /** Each account's Trash and Junk ids (key '' = the own account), left out of the counts. */
+    excluded?: Record<string, string[]>,
+  ) => Promise<void>;
   /** An Email change arrived: the next `ensure` fetches again. */
   invalidate: () => void;
 }
@@ -35,8 +41,8 @@ export const useTagCountsStore = create<TagCountsState>((set, get) => ({
   generation: 0,
   fetchedAt: -1,
 
-  ensure: (login, tagIds, accountIds) => {
-    const key = `${login}|${accountIds.map((id) => id ?? '').join(',')}|${tagIds.join(',')}`;
+  ensure: (login, tagIds, accountIds, excluded = {}) => {
+    const key = `${login}|${accountIds.map((id) => id ?? '').join(',')}|${tagIds.join(',')}|${JSON.stringify(excluded)}`;
     if (get().login !== login) set({ counts: {}, login, key: null });
     const { generation } = get();
     if (get().key === key && get().fetchedAt === generation) return Promise.resolve();
@@ -44,7 +50,7 @@ export const useTagCountsStore = create<TagCountsState>((set, get) => ({
     const seq = ++lastSeq;
     const promise = (async () => {
       try {
-        const list = await fetchTagCounts(tagIds, accountIds);
+        const list = await fetchTagCounts(tagIds, accountIds, excluded);
         // A later ensure (another login, other tags) supersedes this one.
         if (inflight?.seq !== seq || get().login !== login) return;
         set({ counts: Object.fromEntries(list.map((c) => [c.id, c])), key, fetchedAt: generation });