diff --git a/components/mail/mail-app.tsx b/components/mail/mail-app.tsx index 74561b685..5a28842ed 100644 --- a/components/mail/mail-app.tsx +++ b/components/mail/mail-app.tsx @@ -56,7 +56,7 @@ import { debug } from "@/lib/debug"; import { playNotificationSound } from "@/lib/notification-sound"; import { cn, getMailboxPath } from "@/lib/utils"; import { localizeMailboxName } from "@/lib/mailbox-label"; -import { KEYWORD_PREFIX, KEYWORD_PREFIX_LEGACY, groupEmailsByThread, threadKeyFor } from "@/lib/thread-utils"; +import { groupEmailsByThread, threadKeyFor, toggleTagKeywords } from "@/lib/thread-utils"; import { resolveThreadRoute } from "@/lib/thread-routing"; import { ErrorBoundary, @@ -1589,19 +1589,25 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { const markAsReadDelay = useSettingsStore.getState().markAsReadDelay; debug.log('email', '[Mark as Read] Delay setting:', markAsReadDelay, 'ms for email:', selectedEmail.id); + // The server may refuse (a shared folder without the right to set seen); + // the message then just stays unread. + const markRead = () => markAsRead(client, selectedEmail.id, true).catch((error) => { + debug.warn('email', '[Mark as Read] Failed:', error); + }); + if (markAsReadDelay === -1) { // Never mark as read automatically debug.log('email', '[Mark as Read] Never mode - email will stay unread'); } else if (markAsReadDelay === 0) { // Mark as read instantly debug.log('email', '[Mark as Read] Instant mode - marking as read now'); - markAsRead(client, selectedEmail.id, true); + void markRead(); } else { // Mark as read after delay debug.log('email', '[Mark as Read] Delayed mode - will mark as read in', markAsReadDelay, 'ms'); markAsReadTimeoutRef.current = setTimeout(() => { debug.log('email', '[Mark as Read] Timeout fired - marking as read now'); - markAsRead(client, selectedEmail.id, true); + void markRead(); markAsReadTimeoutRef.current = null; }, markAsReadDelay); } @@ -2426,33 +2432,12 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { if (!client) return; try { - // Remove any existing tag keywords const email = emails.find(e => e.id === emailId); if (!email) return; - const keywords = { ...email.keywords }; - - if (tagId === null) { - // Remove all tag keywords - Object.keys(keywords).forEach(key => { - if (key.startsWith(KEYWORD_PREFIX) || key.startsWith(KEYWORD_PREFIX_LEGACY)) { - keywords[key] = false; - } - }); - } else { - // Both prefixes name the same tag when read, so taking one off has to - // clear whichever spellings are actually set. - const activeKeys = [KEYWORD_PREFIX + tagId, KEYWORD_PREFIX_LEGACY + tagId] - .filter(key => keywords[key]); - if (activeKeys.length > 0) { - activeKeys.forEach(key => { - keywords[key] = false; - }); - } else { - // Add the tag without disturbing others - keywords[KEYWORD_PREFIX + tagId] = true; - } - } + // The whole keyword map is written, so it may only hold keywords that + // are set: a removed tag is left out rather than sent as false. + const keywords = toggleTagKeywords(email.keywords, tagId); // Routes the write to the email's own account, then patches the email in // place so the list keeps its scroll/pagination state instead of being @@ -2465,7 +2450,10 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { // Refresh tag counts fetchTagCounts(client); } catch (error) { + // The message keeps its old tags. Stalwart 1.0 refuses keywords over + // 128 bytes and more than 100 on one message. console.error("Failed to set tag:", error); + toast.error(t('notifications.tag_failed')); } }; diff --git a/components/settings/__tests__/keyword-settings.test.tsx b/components/settings/__tests__/keyword-settings.test.tsx index 9a24d114f..12bc04bdd 100644 --- a/components/settings/__tests__/keyword-settings.test.tsx +++ b/components/settings/__tests__/keyword-settings.test.tsx @@ -163,8 +163,10 @@ describe('KeywordSettings', () => { expect(screen.getByText('$label:work/clients')).toBeInTheDocument(); }); - it('rejects a path that would exceed the keyword length limit', () => { - const deepId = 'a'.repeat(240); + it('rejects a path that would exceed the 128-byte keyword limit of Stalwart', () => { + // `$label:` + 110 + `/overflowing-name` is 134 bytes: fine by the RFC's + // 255, refused by Stalwart 1.0 and cut short by 0.16. + const deepId = 'a'.repeat(110); useSettingsStore.setState({ emailKeywords: [{ id: deepId, label: 'Deep', color: 'blue' }], nestedTags: true, diff --git a/lib/__tests__/jmap-cross-account-move.test.ts b/lib/__tests__/jmap-cross-account-move.test.ts index 462abe5b8..b310bf39a 100644 --- a/lib/__tests__/jmap-cross-account-move.test.ts +++ b/lib/__tests__/jmap-cross-account-move.test.ts @@ -3,7 +3,7 @@ import { JMAPClient } from '../jmap/client'; type Call = [string, Record, string]; -function clientWithCopyResult(copyResult: Record) { +function clientWithCopyResult(copyResult: Record, keywords: Record = { $seen: true }) { const client = new JMAPClient('https://jmap.example.com', 'user@example.com', 'pass'); Object.assign(client, { accountId: 'me' }); const calls: Call[] = []; @@ -11,7 +11,7 @@ function clientWithCopyResult(copyResult: Record) { .mockImplementation(async (reqCalls: Call[]) => { const [method, args, id] = reqCalls[0]; calls.push(reqCalls[0]); - if (method === 'Email/get') return { methodResponses: [['Email/get', { list: [{ id: 'm1', keywords: { $seen: true } }] }, id]] }; + if (method === 'Email/get') return { methodResponses: [['Email/get', { list: [{ id: 'm1', keywords }] }, id]] }; if (method === 'Email/copy') return { methodResponses: [['Email/copy', copyResult, id]] }; if (method === 'Email/set') return { methodResponses: [['Email/set', { destroyed: args.destroy }, id]] }; throw new Error(`unexpected ${method}`); @@ -34,4 +34,17 @@ describe('copyEmailAcrossAccounts', () => { await expect(client.copyEmailAcrossAccounts('m1', 'me', 'grp', 'inbox')).rejects.toThrow('overQuota'); expect(calls.some(([m]) => m === 'Email/set')).toBe(false); }); + + // Stalwart 1.0 builds the copy's keywords, and from them its hasAttachment, + // only from what the create sends. + it('carries the attachment keyword over to the copy on Stalwart 1.0', async () => { + const { client, calls } = clientWithCopyResult( + { created: { c: { id: 'new1' } } }, + { $seen: true, '$label:work': true, $hasattachment: true }, + ); + await client.copyEmailAcrossAccounts('m1', 'me', 'grp', 'inbox'); + + const create = (calls.find(([m]) => m === 'Email/copy')![1].create as Record>).c; + expect(create.keywords).toEqual({ $seen: true, '$label:work': true, $hasattachment: true }); + }); }); diff --git a/lib/__tests__/jmap-keyword-account.test.ts b/lib/__tests__/jmap-keyword-account.test.ts index cdf4c6956..4bd4504cc 100644 --- a/lib/__tests__/jmap-keyword-account.test.ts +++ b/lib/__tests__/jmap-keyword-account.test.ts @@ -1,5 +1,6 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { JMAPClient } from '../jmap/client'; +import { jmapErrorProperties, jmapErrorType } from '../jmap/jmap-error'; // Regression coverage for the shared-account keyword write path (#281): keyword // mutations (tags, $answered/$forwarded) on a unified-inbox message must target @@ -82,6 +83,15 @@ describe('Email/set clears keywords with null, not false', () => { beforeEach(() => vi.restoreAllMocks()); afterEach(() => vi.restoreAllMocks()); + it('updateEmailKeywords writes a full map of true values only', async () => { + // Local copies carry `$seen: false` after marking unread; RFC 8621 allows + // only true in a keyword map. + const client = createClient(); + const { captured } = mockEmailSet(); + await client.updateEmailKeywords('email-x', { $seen: false, $flagged: false, '$label:work': true, $pinned: true }); + expect(patchFor(captured[0], 'email-x')).toEqual({ keywords: { '$label:work': true, $pinned: true } }); + }); + it('markAsRead(false) removes $seen with null', async () => { const client = createClient(); const { captured } = mockEmailSet(); @@ -169,3 +179,100 @@ describe('spam / not-spam patch keywords alongside the move (#850)', () => { expect(patch['keywords/$seen']).toBeUndefined(); }); }); + +// Stalwart refuses a keyword per message in Email/set `notUpdated`, not as a +// request error, so a writer that only awaited the request reported success +// and the UI showed a tag that was gone after a reload. 1.0 refuses keywords +// over 128 bytes and more than 100 keywords on one message; 0.16 accepts both +// (it cuts long keywords to 128 characters). +describe('keyword writes report what the server refused', () => { + beforeEach(() => vi.restoreAllMocks()); + afterEach(() => vi.restoreAllMocks()); + + const TOO_LONG = { + type: 'invalidProperties', + description: 'Keywords cannot be longer than 128 bytes.', + properties: ['keywords'], + }; + const TOO_MANY = { + type: 'tooManyKeywords', + description: 'An email cannot have more than 100 keywords.', + properties: ['keywords'], + }; + + /** Answers every Email/set with `result` for the ids it names. */ + function mockEmailSetResult(result: (ids: string[]) => Record) { + const captured: JMAPMethodCall[] = []; + vi.spyOn(globalThis, 'fetch').mockImplementation(async (_url, init) => { + const body = JSON.parse((init as { body: string }).body) as { methodCalls: JMAPMethodCall[] }; + captured.push(...body.methodCalls); + const methodResponses = body.methodCalls.map((call) => { + const ids = Object.keys((call[1].update ?? {}) as Record); + return ['Email/set', { accountId: call[1].accountId, newState: 's2', ...result(ids) }, call[2]]; + }); + return new Response(JSON.stringify({ methodResponses }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }); + }); + return captured; + } + + const refuseAll = (error: Record) => (ids: string[]) => ({ + notUpdated: Object.fromEntries(ids.map((id) => [id, error])), + }); + const acceptAll = (ids: string[]) => ({ updated: Object.fromEntries(ids.map((id) => [id, null])) }); + + const writers: Array<[string, (client: JMAPClient) => Promise]> = [ + ['updateEmailKeywords', (c) => c.updateEmailKeywords('m1', { [`$label:${'x'.repeat(130)}`]: true })], + ['setKeyword', (c) => c.setKeyword('m1', `$label:${'x'.repeat(130)}`)], + ['removeKeyword', (c) => c.removeKeyword('m1', '$label:work')], + ['batchUpdateKeywords', (c) => c.batchUpdateKeywords(['m1'], { [`keywords/$label:${'x'.repeat(130)}`]: true })], + ['toggleStar', (c) => c.toggleStar('m1', true)], + ['markAsRead', (c) => c.markAsRead('m1', true)], + ['batchMarkAsRead', (c) => c.batchMarkAsRead(['m1'], true)], + ]; + + it.each(writers)('%s rejects with the SetError type on Stalwart 1.0', async (_name, write) => { + mockEmailSetResult(refuseAll(TOO_LONG)); + const error = await write(createClient()).then(() => null, (e: unknown) => e); + expect(error).toBeInstanceOf(Error); + expect((error as Error).message).toContain('Keywords cannot be longer than 128 bytes.'); + expect(jmapErrorType(error)).toBe('invalidProperties'); + expect(jmapErrorProperties(error)).toEqual(['keywords']); + }); + + it.each(writers)('%s resolves when Stalwart 0.16 accepts the write', async (_name, write) => { + mockEmailSetResult(acceptAll); + await expect(write(createClient())).resolves.toBeUndefined(); + }); + + it('reports too many keywords on one message (Stalwart 1.0)', async () => { + mockEmailSetResult(refuseAll(TOO_MANY)); + const error = await createClient().setKeyword('m1', '$label:one-more').then(() => null, (e: unknown) => e); + expect(jmapErrorType(error)).toBe('tooManyKeywords'); + expect((error as Error).message).toContain('An email cannot have more than 100 keywords.'); + }); + + it('reports a method-level refusal with its type', async () => { + vi.spyOn(globalThis, 'fetch').mockResolvedValue(new Response(JSON.stringify({ + methodResponses: [['error', { type: 'accountReadOnly' }, '0']], + }), { status: 200 })); + const error = await createClient().toggleStar('m1', false).then(() => null, (e: unknown) => e); + expect(jmapErrorType(error)).toBe('accountReadOnly'); + }); + + it('writes every batch before reporting a refused message', async () => { + const client = createClient(); + Object.assign(client, { capabilities: { 'urn:ietf:params:jmap:core': { maxObjectsInSet: 1 } } }); + const captured = mockEmailSetResult((ids) => (ids[0] === 'm1' ? refuseAll(TOO_LONG)(ids) : acceptAll(ids))); + + await expect(client.batchUpdateKeywords(['m1', 'm2', 'm3'], { 'keywords/$label:work': true })) + .rejects.toThrow('Keywords cannot be longer than 128 bytes.'); + expect(captured.map((call) => Object.keys(call[1].update as Record))).toEqual([['m1'], ['m2'], ['m3']]); + + captured.length = 0; + await expect(client.batchMarkAsRead(['m1', 'm2'], true)).rejects.toThrow(/mark as read/); + expect(captured).toHaveLength(2); + }); +}); diff --git a/lib/__tests__/jmap-keyword-discovery.test.ts b/lib/__tests__/jmap-keyword-discovery.test.ts index 08b27e84c..29596d140 100644 --- a/lib/__tests__/jmap-keyword-discovery.test.ts +++ b/lib/__tests__/jmap-keyword-discovery.test.ts @@ -273,4 +273,17 @@ describe('JMAPClient.discoverKeywords', () => { ]); expect(result).toMatchObject({ scanned: 1, total: 1, complete: true }); }); + + it('leaves the attachment keywords Stalwart 1.0 sets on every message out of the fallback labels', async () => { + const client = await connectedClient(); + recordRequests(() => page(['m1', 'm2'], [ + { '$label:work': true, $hasattachment: true }, + { $seen: true, $hasnoattachment: true }, + ], 2)); + + const result = await client.getKeywords(); + + expect(result.keywords).toEqual({ '$label:work': 1, $seen: 1 }); + expect(result.labels.map((label) => label.id)).toEqual(['$label:work', '$seen']); + }); }); diff --git a/lib/__tests__/keyword-nesting.test.ts b/lib/__tests__/keyword-nesting.test.ts index 888c45ad5..c5e8f667e 100644 --- a/lib/__tests__/keyword-nesting.test.ts +++ b/lib/__tests__/keyword-nesting.test.ts @@ -1,6 +1,7 @@ import { describe, it, expect } from "vitest"; import { MAX_KEYWORD_ID_LENGTH, + MAX_KEYWORD_LENGTH, buildKeywordTree, composeKeywordId, countKeywordNodes, @@ -96,9 +97,17 @@ describe("hasChildKeywords", () => { }); describe("MAX_KEYWORD_ID_LENGTH", () => { - it("leaves room for the `$label:` prefix within the 255-character keyword limit", () => { - expect(MAX_KEYWORD_ID_LENGTH).toBe(248); - expect("$label:".length + MAX_KEYWORD_ID_LENGTH).toBe(255); + // Stalwart 1.0 refuses a keyword over 128 bytes; 0.16 cuts it to 128 + // characters, which detaches it from its tag. + it("leaves room for the `$label:` prefix within Stalwart's 128-byte keyword limit", () => { + expect(MAX_KEYWORD_LENGTH).toBe(128); + expect(MAX_KEYWORD_ID_LENGTH).toBe(121); + expect("$label:".length + MAX_KEYWORD_ID_LENGTH).toBe(128); + }); + + it("counts bytes, which a composed id always has as many of as characters", () => { + const id = composeKeywordId("work/clients", "Ärger & Überstunden"); + expect(new TextEncoder().encode(`$label:${id}`).length).toBe(`$label:${id}`.length); }); }); diff --git a/lib/__tests__/message-list-tabs-store.test.ts b/lib/__tests__/message-list-tabs-store.test.ts index e61367dcf..5203bc828 100644 --- a/lib/__tests__/message-list-tabs-store.test.ts +++ b/lib/__tests__/message-list-tabs-store.test.ts @@ -1,12 +1,17 @@ // Contract tests for the plugin-facing message-list category tabs // (api.tabs.set → stores/message-list-tabs-store.ts). -import { describe, it, expect, beforeEach } from 'vitest'; +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import { useMessageListTabsStore, validateTabsConfig, resolveTabFilter, } from '@/stores/message-list-tabs-store'; +import { useEmailStore } from '@/stores/email-store'; +import { messageListTabHooks } from '@/lib/plugin-hooks'; +import { jmapError } from '@/lib/jmap/jmap-error'; +import type { IJMAPClient } from '@/lib/jmap/client-interface'; +import type { Email } from '@/lib/jmap/types'; import type { MessageListTab } from '@/lib/plugin-types'; const GMAIL_STYLE = { @@ -59,6 +64,17 @@ describe('validateTabsConfig', () => { })).toThrow(/reserved/); }); + // Stalwart 1.0 sets one of these on every message and derives hasAttachment + // from it; a tab move would add or strip it. + it.each(['$hasattachment', '$hasnoattachment', '$HasAttachment'])( + 'rejects the server-set attachment keyword %s (Stalwart 1.0)', + (keyword) => { + expect(() => validateTabsConfig({ + tabs: [{ id: 'a', label: 'A', keyword }, { id: 'b', label: 'B' }], + })).toThrow(/reserved keyword "\$has(no)?attachment"/); + }, + ); + it('rejects duplicate ids, multiple default tabs, and single-tab configs', () => { expect(() => validateTabsConfig({ tabs: [{ id: 'a', label: 'A' }, { id: 'a', label: 'B', keyword: '$x' }], @@ -166,3 +182,59 @@ describe('store registration', () => { expect(useMessageListTabsStore.getState().getCategoryFilter('inbox')).toEqual({ hasKeyword: '$category-promotions' }); }); }); + +describe('categorizeEmails', () => { + const email = (id: string, accountId?: string) => ({ + id, threadId: `t-${id}`, keywords: { $seen: true }, mailboxIds: { inbox: true }, + from: [{ email: `${id}@example.com` }], accountId, + }) as unknown as Email; + + beforeEach(() => { + useEmailStore.setState({ emails: [email('m1'), email('m2', 'shared')], mailboxes: [], selectedMailbox: '' }); + useMessageListTabsStore.getState().registerTabs('category-tabs', GMAIL_STYLE); + }); + + afterEach(() => { + vi.restoreAllMocks(); + useEmailStore.setState({ emails: [] }); + }); + + it('patches the category keywords and moves the messages locally', async () => { + const batchUpdateKeywords = vi.fn().mockResolvedValue(undefined); + const emitted = vi.spyOn(messageListTabHooks.onEmailCategorize, 'emit'); + + await expect(useMessageListTabsStore.getState() + .categorizeEmails({ batchUpdateKeywords } as unknown as IJMAPClient, ['m1'], 'promotions')).resolves.toBe(true); + + expect(batchUpdateKeywords).toHaveBeenCalledWith( + ['m1'], + { 'keywords/$category-updates': null, 'keywords/$category-promotions': true }, + undefined, + ); + expect(useEmailStore.getState().emails[0].keywords).toEqual({ $seen: true, '$category-promotions': true }); + expect(emitted).toHaveBeenCalledTimes(1); + }); + + it('rejects when Stalwart 1.0 refuses the keyword, leaving the refused messages as they were', async () => { + // What batchUpdateKeywords throws for a `notUpdated` entry on 1.0, here + // for a message that already has 100 keywords. + const refusal = jmapError('Failed to update keywords: An email cannot have more than 100 keywords.', { + type: 'tooManyKeywords', description: 'An email cannot have more than 100 keywords.', properties: ['keywords'], + }); + const batchUpdateKeywords = vi.fn(async (_ids: string[], _patch: unknown, accountId?: string) => { + if (accountId === 'shared') throw refusal; + }); + const emitted = vi.spyOn(messageListTabHooks.onEmailCategorize, 'emit'); + + await expect(useMessageListTabsStore.getState() + .categorizeEmails({ batchUpdateKeywords } as unknown as IJMAPClient, ['m1', 'm2'], 'promotions')).rejects.toBe(refusal); + + // Both accounts were written; only the one the server accepted moved. + expect(batchUpdateKeywords).toHaveBeenCalledTimes(2); + const [m1, m2] = useEmailStore.getState().emails; + expect(m1.keywords).toEqual({ $seen: true, '$category-promotions': true }); + expect(m2.keywords).toEqual({ $seen: true }); + // No per-sender override is recorded for a move that did not happen. + expect(emitted).not.toHaveBeenCalled(); + }); +}); diff --git a/lib/__tests__/plugin-api-gates.test.ts b/lib/__tests__/plugin-api-gates.test.ts index b267c495f..074048138 100644 --- a/lib/__tests__/plugin-api-gates.test.ts +++ b/lib/__tests__/plugin-api-gates.test.ts @@ -304,4 +304,30 @@ describe('native keyword extension API', () => { expect(error).toMatch(/No active session/); } }); + + // Stalwart 1.0 derives hasAttachment from these, so a plugin adding or + // removing one would misreport the message. + it('keeps plugins off the attachment keywords Stalwart 1.0 sets itself', async () => { + const setKeyword = vi.fn(); + const removeKeyword = vi.fn(); + const updateEmailKeywords = vi.fn(); + useAuthStore.setState({ client: { setKeyword, removeKeyword, updateEmailKeywords } as never }); + const writer = plugin({ permissions: ['email:write'], grantedPermissions: ['email:write'] }); + + for (const method of ['jmap.setKeyword', 'jmap.removeKeyword', 'email.setKeyword', 'email.removeKeyword']) { + for (const keyword of ['$hasattachment', '$hasnoattachment', '$HasAttachment']) { + expect(await gateError(writer, method, ['email-1', keyword])).toMatch(/is set by the server/); + } + } + expect(await gateError(writer, 'jmap.setKeywords', ['email-1', { '$seen': true, '$hasattachment': true }])) + .toMatch(/"\$hasattachment" is set by the server/); + + expect(setKeyword).not.toHaveBeenCalled(); + expect(removeKeyword).not.toHaveBeenCalled(); + expect(updateEmailKeywords).not.toHaveBeenCalled(); + + // Other keywords still go through. + await dispatchApiCall(writer, 'jmap.setKeyword', ['email-1', '$label:work']); + expect(setKeyword).toHaveBeenCalledWith('email-1', '$label:work', undefined); + }); }); diff --git a/lib/__tests__/server-keywords.test.ts b/lib/__tests__/server-keywords.test.ts new file mode 100644 index 000000000..bf4f62846 --- /dev/null +++ b/lib/__tests__/server-keywords.test.ts @@ -0,0 +1,66 @@ +import { describe, it, expect } from 'vitest'; +import type { Email } from '@/lib/jmap/types'; +import { generateEmailSource } from '@/lib/email-source'; +import { emailToReadView } from '@/lib/plugin-projection'; +import { isServerSetKeyword } from '@/lib/jmap/server-keywords'; + +// Stalwart 1.0 puts `$hasattachment` or `$hasnoattachment` on every message and +// derives hasAttachment from it. Where keywords are listed they would only +// repeat hasAttachment, so they are left out; 0.16 never sets them. + +function makeEmail(keywords: Record, hasAttachment: boolean): Email { + return { + id: 'm1', + threadId: 't1', + mailboxIds: { inbox: true }, + keywords, + size: 2048, + receivedAt: '2026-09-25T10:00:00Z', + from: [{ name: 'Ann', email: 'ann@example.org' }], + to: [{ name: '', email: 'bob@example.org' }], + subject: 'Report', + preview: 'See attached', + hasAttachment, + textBody: [{ partId: '1', blobId: 'b1', size: 12, type: 'text/plain' }], + htmlBody: [], + bodyValues: { '1': { value: 'See attached', isEncodingProblem: false, isTruncated: false } }, + } as unknown as Email; +} + +describe('isServerSetKeyword', () => { + it('matches the two attachment keywords in any case, and nothing else', () => { + expect(isServerSetKeyword('$hasattachment')).toBe(true); + expect(isServerSetKeyword('$hasnoattachment')).toBe(true); + expect(isServerSetKeyword('$HasAttachment')).toBe(true); + expect(isServerSetKeyword('$seen')).toBe(false); + expect(isServerSetKeyword('$label:hasattachment')).toBe(false); + }); +}); + +describe('server-set keywords in listings', () => { + it('leaves them out of the view-source keyword line on Stalwart 1.0', () => { + const source = generateEmailSource(makeEmail({ $seen: true, '$label:work': true, $hasattachment: true }, true)); + expect(source).toContain('Has Attachment: Yes\n'); + expect(source).toContain('Keywords: $seen, $label:work\n'); + expect(source).not.toContain('$hasattachment'); + }); + + it('drops the keyword line when the attachment keyword is the only one on Stalwart 1.0', () => { + const source = generateEmailSource(makeEmail({ $hasnoattachment: true }, false)); + expect(source).toContain('Has Attachment: No\n'); + expect(source).not.toContain('Keywords:'); + }); + + it('keeps the plugin projection keywords to what the user set, with hasAttachment intact', () => { + const view = emailToReadView(makeEmail({ $seen: true, $hasattachment: true, $flagged: false }, true)); + expect(view.keywords).toEqual(['$seen']); + expect(view.hasAttachment).toBe(true); + expect(view.source).not.toContain('$hasattachment'); + }); + + it('lists keywords unchanged on Stalwart 0.16, which never sets them', () => { + const email = makeEmail({ $seen: true, '$label:work': true }, true); + expect(emailToReadView(email).keywords).toEqual(['$seen', '$label:work']); + expect(generateEmailSource(email)).toContain('Keywords: $seen, $label:work\n'); + }); +}); diff --git a/lib/__tests__/thread-utils.test.ts b/lib/__tests__/thread-utils.test.ts index 095142a7a..ea1acbd5a 100644 --- a/lib/__tests__/thread-utils.test.ts +++ b/lib/__tests__/thread-utils.test.ts @@ -10,6 +10,7 @@ import { getThreadTagIds, threadKeyFor, threadIdFromKey, + toggleTagKeywords, } from '../thread-utils'; import type { Email, ThreadGroup } from '../jmap/types'; @@ -306,6 +307,41 @@ describe('getEmailTagIds', () => { }); }); +// The result is written as the message's whole keyword map, where RFC 8621 +// allows only true values: a removed tag must be missing, not false. +describe('toggleTagKeywords', () => { + it('adds a tag without disturbing the other keywords', () => { + expect(toggleTagKeywords({ $seen: true, '$label:red': true }, 'work')) + .toEqual({ $seen: true, '$label:red': true, '$label:work': true }); + }); + + it('takes a set tag off by leaving its key out, under either prefix', () => { + const next = toggleTagKeywords({ $seen: true, '$label:red': true, '$color:red': true, '$label:work': true }, 'red'); + expect(next).toEqual({ $seen: true, '$label:work': true }); + expect(Object.values(next).every((value) => value === true)).toBe(true); + }); + + it('clears every tag, whatever the prefix, and keeps the rest', () => { + expect(toggleTagKeywords({ $flagged: true, '$label:red': true, '$color:blue': true }, null)) + .toEqual({ $flagged: true }); + }); + + it('drops keywords the local copy only marks false', () => { + // Marking a message unread or unstarred leaves these behind locally. + expect(toggleTagKeywords({ $seen: false, $flagged: false, '$label:red': false }, 'red')) + .toEqual({ '$label:red': true }); + }); + + it('keeps the server-set attachment keyword Stalwart 1.0 reports', () => { + expect(toggleTagKeywords({ $hasattachment: true }, 'work')).toEqual({ $hasattachment: true, '$label:work': true }); + }); + + it('starts from nothing when the message has no keywords', () => { + expect(toggleTagKeywords(undefined, 'work')).toEqual({ '$label:work': true }); + expect(toggleTagKeywords(undefined, null)).toEqual({}); + }); +}); + describe('getEmailTagId', () => { it('returns label from $label: keyword', () => { expect(getEmailTagId({ '$label:red': true, $seen: true })).toBe('red'); diff --git a/lib/email-source.ts b/lib/email-source.ts index 7a80a2f12..efc9e4e85 100644 --- a/lib/email-source.ts +++ b/lib/email-source.ts @@ -4,6 +4,7 @@ // exact same text the UI shows. Pure: depends only on the passed `email`. import type { Email } from '@/lib/jmap/types'; +import { isServerSetKeyword } from '@/lib/jmap/server-keywords'; import { formatFileSize } from '@/lib/utils'; export function generateEmailSource(email: Email): string { @@ -66,8 +67,9 @@ export function generateEmailSource(email: Email): string { source += `Size: ${formatFileSize(email.size)}\n`; source += `Has Attachment: ${email.hasAttachment ? 'Yes' : 'No'}\n`; if (email.keywords) { + // Stalwart 1.0's attachment keywords only restate the line above. const keywords = Object.entries(email.keywords) - .filter(([_, v]) => v) + .filter(([k, v]) => v && !isServerSetKeyword(k)) .map(([k]) => k) .join(', '); if (keywords) source += `Keywords: ${keywords}\n`; diff --git a/lib/jmap/client.ts b/lib/jmap/client.ts index 870edbbe1..c877665f5 100644 --- a/lib/jmap/client.ts +++ b/lib/jmap/client.ts @@ -19,6 +19,7 @@ import { fileNodeDepth } from "./filenode-depth"; import { contactFromWire, contactToWire } from "./contact-wire"; import { SchedulingDeniedError } from "./scheduling-error"; import { jmapError, jmapErrorType, jmapSetError, type JmapErrorInfo } from "./jmap-error"; +import { isServerSetKeyword } from "./server-keywords"; export { SchedulingDeniedError }; import { acceptedFileName, fileNameRulesFrom, type FileNameRules } from "@/lib/file-name-rules"; @@ -2346,7 +2347,12 @@ export class JMAPClient implements IJMAPClient { if (!supportsKeywordGet) { const scan = await this.discoverKeywords(options); - const labels: KeywordInfo[] = Object.entries(scan.keywords).map(([id, total]) => ({ + // Stalwart 1.0 sets an attachment keyword on every message; it is not a + // label anyone applied. + const keywords = Object.fromEntries( + Object.entries(scan.keywords).filter(([id]) => !isServerSetKeyword(id)), + ); + const labels: KeywordInfo[] = Object.entries(keywords).map(([id, total]) => ({ id, name: id.startsWith('$label:') ? id.slice('$label:'.length) : id, color: null, @@ -2355,7 +2361,7 @@ export class JMAPClient implements IJMAPClient { isProviderLabel: false, source: 'message', })); - return { ...scan, labels }; + return { ...scan, keywords, labels }; } if (options?.signal?.aborted) { @@ -2571,10 +2577,16 @@ export class JMAPClient implements IJMAPClient { } } + // The keyword writers below check the Email/set outcome: the server refuses + // a keyword per message in `notUpdated`, not as a request error. Stalwart + // 1.0 refuses keywords over 128 bytes (invalidProperties) and more than 100 + // keywords on one message (tooManyKeywords); a read-only shared folder + // refuses any. Unchecked, the UI showed the change until the next reload. + async markAsRead(emailId: string, read: boolean = true, accountId?: string): Promise { const targetAccountId = accountId || this.accountId; - await this.request([ + const response = await this.request([ ["Email/set", { accountId: targetAccountId, update: { @@ -2584,21 +2596,28 @@ export class JMAPClient implements IJMAPClient { }, }, "0"], ]); + this.assertEmailSetSucceeded(response, read ? "mark as read" : "mark as unread"); } async batchMarkAsRead(emailIds: string[], read: boolean = true, accountId?: string): Promise { if (emailIds.length === 0) return; + const responses: JMAPResponse[] = []; for (const batch of batched(emailIds, this.getMaxObjectsInSet())) { const updates = Object.fromEntries(batch.map(id => [id, { "keywords/$seen": read ? true : null }])); - await this.request([ + responses.push(await this.request([ ["Email/set", { accountId: accountId || this.accountId, update: updates }, "0"], - ]); + ])); + } + // Every batch is sent before a refusal is reported: one refused message + // must not leave the rest of the selection unchanged. + for (const response of responses) { + this.assertEmailSetSucceeded(response, read ? "mark as read" : "mark as unread"); } } async toggleStar(emailId: string, starred: boolean, accountId?: string): Promise { - await this.request([ + const response = await this.request([ ["Email/set", { accountId: accountId || this.accountId, update: { @@ -2608,23 +2627,29 @@ export class JMAPClient implements IJMAPClient { }, }, "0"], ]); + this.assertEmailSetSucceeded(response, starred ? "star the email" : "unstar the email"); } async updateEmailKeywords(emailId: string, keywords: Record, accountId?: string): Promise { - await this.request([ + // Every value of a full keyword map must be true (RFC 8621 section + // 4.1.1). Callers start from local copies, where marking a message unread + // or unstarred leaves `$seen: false` / `$flagged: false` behind. + const keywordSet = Object.fromEntries(Object.entries(keywords).filter(([, value]) => value === true)); + const response = await this.request([ ["Email/set", { accountId: accountId || this.accountId, update: { [emailId]: { - keywords, + keywords: keywordSet, }, }, }, "0"], ]); + this.assertEmailSetSucceeded(response, "update keywords"); } async setKeyword(emailId: string, keyword: string, accountId?: string): Promise { - await this.request([ + const response = await this.request([ ["Email/set", { accountId: accountId || this.accountId, update: { @@ -2634,10 +2659,11 @@ export class JMAPClient implements IJMAPClient { }, }, "0"], ]); + this.assertEmailSetSucceeded(response, `set ${keyword}`); } async removeKeyword(emailId: string, keyword: string, accountId?: string): Promise { - await this.request([ + const response = await this.request([ ["Email/set", { accountId: accountId || this.accountId, update: { @@ -2647,6 +2673,7 @@ export class JMAPClient implements IJMAPClient { }, }, "0"], ]); + this.assertEmailSetSucceeded(response, `remove ${keyword}`); } /** @@ -2656,11 +2683,15 @@ export class JMAPClient implements IJMAPClient { */ async batchUpdateKeywords(emailIds: string[], patch: Record, accountId?: string): Promise { if (emailIds.length === 0 || Object.keys(patch).length === 0) return; + const responses: JMAPResponse[] = []; for (const batch of batched(emailIds, this.getMaxObjectsInSet())) { const update = Object.fromEntries(batch.map(id => [id, { ...patch }])); - await this.request([ + responses.push(await this.request([ ["Email/set", { accountId: accountId || this.accountId, update }, "0"], - ]); + ])); + } + for (const response of responses) { + this.assertEmailSetSucceeded(response, "update keywords"); } } @@ -2797,9 +2828,9 @@ export class JMAPClient implements IJMAPClient { private assertEmailSetSucceeded(response: JMAPResponse, action: string): void { const [name, result] = response.methodResponses?.[0] ?? []; if (name === "error") { - throw new Error(`Failed to ${action}: ${result?.description || result?.type || 'unknown error'}`); + throw jmapError(`Failed to ${action}: ${result?.description || result?.type || 'unknown error'}`, result); } - const failed: Record = { + const failed: Record = { ...(result?.notUpdated ?? {}), ...(result?.notDestroyed ?? {}), }; @@ -2807,10 +2838,11 @@ export class JMAPClient implements IJMAPClient { if (failedIds.length === 0) return; const first = failed[failedIds[0]]; const reason = first?.description || first?.type || 'unknown error'; - throw new Error( + throw jmapError( failedIds.length === 1 ? `Failed to ${action}: ${reason}` : `Failed to ${action} ${failedIds.length} email(s); first error: ${reason}`, + first, ); } @@ -9251,7 +9283,9 @@ export class JMAPClient implements IJMAPClient { destMailboxId: string, ): Promise { // Email/copy drops keywords unless the create sets them, so carry the - // source's over — otherwise the moved message shows up as unread. + // source's over — otherwise the moved message shows up as unread. All of + // them: Stalwart 1.0 also takes the copy's `$hasattachment` / + // `$hasnoattachment`, and with them its hasAttachment, only from here. const srcResp = await this.request([ ["Email/get", { accountId: fromAccountId, ids: [emailId], properties: ["keywords"] }, "0"], ]); diff --git a/lib/jmap/server-keywords.ts b/lib/jmap/server-keywords.ts new file mode 100644 index 000000000..24b6e9267 --- /dev/null +++ b/lib/jmap/server-keywords.ts @@ -0,0 +1,18 @@ +/** + * Keywords the server sets on its own. + * + * Stalwart 1.0 puts `$hasattachment` or `$hasnoattachment` on every message it + * stores and derives Email `hasAttachment` (the property and the filter) from + * them alone. A patch can still add or remove them, which flips + * `hasAttachment`. So they are not tags: they are hidden wherever keywords are + * listed, and no plugin or category tab may add or remove them. A copy must + * carry them over, though: Email/copy takes a message's keywords only from the + * request, and a copy sent without them loses its attachment flag. Stalwart + * 0.16 never sets them. + */ +export const SERVER_SET_KEYWORDS: ReadonlySet = new Set(['$hasattachment', '$hasnoattachment']); + +/** True for a keyword the server sets itself (keywords are case-insensitive). */ +export function isServerSetKeyword(keyword: string): boolean { + return SERVER_SET_KEYWORDS.has(keyword.toLowerCase()); +} diff --git a/lib/keyword-nesting.ts b/lib/keyword-nesting.ts index 57a4fc9b0..e8e2eef8c 100644 --- a/lib/keyword-nesting.ts +++ b/lib/keyword-nesting.ts @@ -11,7 +11,8 @@ * RFC 8621 section 4.1.1 allows a keyword of 1-255 characters from the ASCII * range %x21-%x7e minus `( ) { ] % * " \`, so the separator is legal but the * length of a deep id is not free - `MAX_KEYWORD_ID_LENGTH` is the budget a - * composed id has to stay within. + * composed id has to stay within. Stalwart caps keywords well below the RFC + * (see `MAX_KEYWORD_LENGTH`). * * Turning any of this into text for the screen lives in `keyword-format`. */ @@ -21,8 +22,13 @@ import { KEYWORD_PREFIX } from "./thread-utils"; /** Separates parent from child inside a tag id. */ export const KEYWORD_SEPARATOR = "/"; -/** Longest keyword a JMAP server has to accept (RFC 8621, section 4.1.1). */ -export const MAX_KEYWORD_LENGTH = 255; +/** + * Longest keyword Stalwart keeps, in bytes (the RFC allows 255). 1.0 refuses a + * longer new keyword ("Keywords cannot be longer than 128 bytes.") and 0.16 + * cuts it to 128 characters, which silently detaches it from its tag. Tag + * keywords are ASCII, so bytes and characters are the same count. + */ +export const MAX_KEYWORD_LENGTH = 128; /** What is left for the id once the `$label:` prefix is spent. */ export const MAX_KEYWORD_ID_LENGTH = MAX_KEYWORD_LENGTH - KEYWORD_PREFIX.length; diff --git a/lib/plugin-projection.ts b/lib/plugin-projection.ts index 865df43e3..40dbf9590 100644 --- a/lib/plugin-projection.ts +++ b/lib/plugin-projection.ts @@ -5,6 +5,7 @@ import type { Email } from '@/lib/jmap/types'; import type { EmailReadView } from '@/lib/plugin-types'; import { generateEmailSource } from '@/lib/email-source'; +import { isServerSetKeyword } from '@/lib/jmap/server-keywords'; // Resolve a message's plain-text body from its JMAP body parts. Plugins that // translate or scan content need the real body, not just the short `preview` @@ -57,7 +58,8 @@ export function emailToReadView(email: Email): EmailReadView { text: plainTextFromEmail(email), headers: email.headers, source: generateEmailSource(email), - keywords: Object.keys(email.keywords || {}).filter(k => email.keywords[k]), + // Stalwart 1.0's attachment keywords are what `hasAttachment` says. + keywords: Object.keys(email.keywords || {}).filter(k => email.keywords[k] && !isServerSetKeyword(k)), auth: email.authenticationResults, }; } diff --git a/lib/plugin-sandbox/host-api.ts b/lib/plugin-sandbox/host-api.ts index 6393177e4..f423bfe0d 100644 --- a/lib/plugin-sandbox/host-api.ts +++ b/lib/plugin-sandbox/host-api.ts @@ -21,6 +21,7 @@ import type { MessageListTabsConfig } from '../plugin-types'; import { apiFetch, getPathPrefix } from '../browser-navigation'; import { reportUploadProgress } from '../upload-progress'; import { DEFAULT_KEYWORD_SCAN_LIMIT } from '../jmap/client'; +import { isServerSetKeyword } from '../jmap/server-keywords'; import { suggestKeywordColor } from '../keyword-discovery'; import { MAX_KEYWORD_LENGTH } from '../keyword-nesting'; import { KEYWORD_PREFIX } from '../thread-utils'; @@ -994,6 +995,11 @@ function assertPluginKeyword(keyword: unknown): string { if (typeof keyword !== 'string' || !PLUGIN_KEYWORD_RE.test(keyword)) { throw new Error(`Invalid JMAP keyword "${String(keyword)}"`); } + // Stalwart 1.0 derives hasAttachment from these; adding or removing one + // would misreport the message. + if (isServerSetKeyword(keyword)) { + throw new Error(`JMAP keyword "${keyword}" is set by the server`); + } return keyword; } diff --git a/lib/thread-utils.ts b/lib/thread-utils.ts index a3476c6ec..bb78a4eaf 100644 --- a/lib/thread-utils.ts +++ b/lib/thread-utils.ts @@ -231,6 +231,37 @@ export function getEmailTagIds(keywords: Record | undefined): s return [...tags]; } +/** + * The complete keyword map after toggling one tag, or after clearing every tag + * (`tagId` null), for writing as a whole. Every value in such a map must be + * true (RFC 8621 section 4.1.1), so a tag comes off by leaving its key out, + * and so do keywords the local copy only marks false (`$seen: false` after + * marking a message unread). Both prefixes name the same tag, so taking a tag + * off clears whichever spellings are set. + */ +export function toggleTagKeywords( + keywords: Record | undefined, + tagId: string | null, +): Record { + const next: Record = {}; + for (const [key, value] of Object.entries(keywords ?? {})) { + if (value === true) next[key] = true; + } + if (tagId === null) { + for (const key of Object.keys(next)) { + if (key.startsWith(KEYWORD_PREFIX) || key.startsWith(KEYWORD_PREFIX_LEGACY)) delete next[key]; + } + return next; + } + const activeKeys = [KEYWORD_PREFIX + tagId, KEYWORD_PREFIX_LEGACY + tagId].filter((key) => next[key]); + if (activeKeys.length > 0) { + for (const key of activeKeys) delete next[key]; + } else { + next[KEYWORD_PREFIX + tagId] = true; + } + return next; +} + /** * Gets the first tag id set on a message, if any. * Reads both the current $label: prefix and the legacy $color: prefix. diff --git a/stores/__tests__/email-store-cross-account-move.test.ts b/stores/__tests__/email-store-cross-account-move.test.ts index fcfdb9c26..5390ca62d 100644 --- a/stores/__tests__/email-store-cross-account-move.test.ts +++ b/stores/__tests__/email-store-cross-account-move.test.ts @@ -1,10 +1,14 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; import { useEmailStore } from '../email-store'; +import { useAuthStore } from '../auth-store'; import type { Email, Mailbox } from '@/lib/jmap/types'; import type { IJMAPClient } from '@/lib/jmap/client-interface'; type Store = ReturnType; +// The routing tests replace it with a spy; the carry-over test needs the real one. +const realCrossAccountMoveEmails = useEmailStore.getState().crossAccountMoveEmails; + function makeMailbox(overrides: Partial): Mailbox { return { id: 'inbox', @@ -100,3 +104,32 @@ describe('email-store moveToMailboxCrossAware', () => { ); }); }); + +describe('email-store crossAccountMoveEmails between two servers', () => { + it('imports the message with every keyword, the Stalwart 1.0 attachment keyword included', async () => { + const blob = new Blob(['From: x@example.com\r\n\r\nhi']); + const keywords = { $seen: true, '$label:work': true, $hasattachment: true }; + const source = { + getEmail: vi.fn().mockResolvedValue({ id: 'e1', blobId: 'blob-1', keywords }), + fetchBlob: vi.fn().mockResolvedValue(blob), + deleteEmail: vi.fn().mockResolvedValue(undefined), + }; + const dest = { importRawEmail: vi.fn().mockResolvedValue('imported-1') }; + useAuthStore.setState({ + activeAccountId: 'login-A', + getClientForAccount: ((id: string) => (id === 'login-A' ? source : id === 'login-B' ? dest : undefined)) as never, + } as never); + useEmailStore.setState({ + emails: [makeEmail('e1', 'srv-inbox-A')], + crossAccountMoveEmails: realCrossAccountMoveEmails, + // The background folder refresh after the move is not under test. + fetchMailboxes: vi.fn().mockResolvedValue(undefined) as unknown as Store['fetchMailboxes'], + fetchAccountMailboxes: vi.fn().mockResolvedValue(undefined) as unknown as Store['fetchAccountMailboxes'], + }); + + await useEmailStore.getState().crossAccountMoveEmails(new Map([['login-A', ['e1']]]), 'login-B', 'inbox-B'); + + expect(dest.importRawEmail).toHaveBeenCalledWith(blob, { 'inbox-B': true }, keywords, undefined); + expect(source.deleteEmail).toHaveBeenCalledWith('e1', undefined); + }); +}); diff --git a/stores/__tests__/email-store-shared-folder-keywords.test.ts b/stores/__tests__/email-store-shared-folder-keywords.test.ts index 97659d646..5a494cb47 100644 --- a/stores/__tests__/email-store-shared-folder-keywords.test.ts +++ b/stores/__tests__/email-store-shared-folder-keywords.test.ts @@ -3,6 +3,7 @@ import { useEmailStore } from '../email-store'; import { useAuthStore } from '../auth-store'; import type { Email, Mailbox } from '@/lib/jmap/types'; import type { IJMAPClient } from '@/lib/jmap/client-interface'; +import { jmapError } from '@/lib/jmap/jmap-error'; // Regression coverage for keyword writes (tags, $pinned, $answered/$forwarded, // $mdnsent) performed while viewing a shared/group mailbox DIRECTLY from the @@ -115,6 +116,19 @@ describe('non-unified shared-folder keyword routing', () => { ); }); + it('setEmailKeywords keeps the old tags when Stalwart 1.0 refuses the keyword', async () => { + // What updateEmailKeywords throws for a `notUpdated` entry on 1.0 (0.16 + // accepts the write and cuts the keyword to 128 characters instead). + const refusal = jmapError('Failed to update keywords: Keywords cannot be longer than 128 bytes.', { + type: 'invalidProperties', properties: ['keywords'], + }); + vi.mocked(activeClient.updateEmailKeywords).mockRejectedValueOnce(refusal); + + await expect(useEmailStore.getState().setEmailKeywords(activeClient, 'e1', { [`$label:${'x'.repeat(130)}`]: true })) + .rejects.toBe(refusal); + expect(useEmailStore.getState().emails.find(e => e.id === 'e1')?.keywords).toEqual({}); + }); + it('setEmailKeywords patches local state so the tag shows immediately', async () => { await useEmailStore.getState().setEmailKeywords(activeClient, 'e1', { '$label:work': true }); diff --git a/stores/email-store.ts b/stores/email-store.ts index b333c06ec..aa9a9ce00 100644 --- a/stores/email-store.ts +++ b/stores/email-store.ts @@ -2671,6 +2671,8 @@ export const useEmailStore = create((set, get) => ({ throw new Error('Source email has no raw blob to copy'); } const blob = await sourceClient.fetchBlob(full.blobId, undefined, undefined, sourceJmapAccountId); + // Every keyword, the server's attachment ones included (see + // lib/jmap/server-keywords.ts). const keywords: Record = { ...(full.keywords ?? {}) }; await destClient.importRawEmail(blob, { [destMailboxId]: true }, keywords, destJmapAccountId); await sourceClient.deleteEmail(emailId, sourceJmapAccountId); diff --git a/stores/message-list-tabs-store.ts b/stores/message-list-tabs-store.ts index 5a95ba890..fc4575cfa 100644 --- a/stores/message-list-tabs-store.ts +++ b/stores/message-list-tabs-store.ts @@ -16,15 +16,19 @@ import type { MessageListTab, MessageListTabsConfig } from '@/lib/plugin-types'; import { MAX_MESSAGE_LIST_TABS } from '@/lib/plugin-types'; import { messageListTabHooks } from '@/lib/plugin-hooks'; import { keywordPointer } from '@/lib/jmap/patch-pointer'; +import { SERVER_SET_KEYWORDS } from '@/lib/jmap/server-keywords'; import { useEmailStore } from './email-store'; // ─── Validation ────────────────────────────────────────────── // System keywords a tab may never claim: filtering the inbox by these would -// hijack read/star/pin semantics, and `$label:` keywords belong to user tags. +// hijack read/star/pin semantics, moving mail between tabs would add and +// remove the server's own attachment keywords (flipping `hasAttachment`), and +// `$label:` keywords belong to user tags. const RESERVED_KEYWORDS = new Set([ '$seen', '$flagged', '$draft', '$answered', '$forwarded', '$recent', '$junk', '$notjunk', '$phishing', '$pinned', + ...SERVER_SET_KEYWORDS, ]); const TAB_ID_RE = /^[a-z0-9][a-z0-9_-]{0,31}$/i; @@ -336,31 +340,45 @@ export const useMessageListTabsStore = create()((set, get) list.push(id); byAccount.set(acct, list); } + // A refused write (a read-only shared folder; Stalwart 1.0 also refuses a + // 101st keyword on a message) fails the move: the plugin's promise rejects + // with the server's reason and the categorize hooks stay quiet. The other + // accounts are still written, and their messages still move locally. + const written: string[] = []; + let failure: unknown = null; for (const [acct, ids] of byAccount) { - await client.batchUpdateKeywords(ids, patch, acct); + try { + await client.batchUpdateKeywords(ids, patch, acct); + written.push(...ids); + } catch (err) { + failure ??= err; + } } // Optimistic local update: fix keywords in place, and drop moved messages // from a keyword-filtered view they no longer belong to. (Query-based // active tabs are left as-is — membership there is server-derived.) - const idSet = new Set(emailIds); + const idSet = new Set(written); const activeTab = tabs.find((t) => t.id === get().activeTabId); const shouldDrop = !!activeTab && !activeTab.query && activeTab.id !== toTabId && get().isEnabledForRole( emailState.mailboxes.find((mb) => mb.id === emailState.selectedMailbox)?.role ?? null, ); - useEmailStore.setState((state) => ({ - emails: state.emails - .map((e) => { - if (!idSet.has(e.id)) return e; - const keywords = { ...e.keywords }; - for (const k of keywordsRemoved) delete keywords[k]; - if (keywordAdded) keywords[keywordAdded] = true; - return { ...e, keywords }; - }) - .filter((e) => !shouldDrop || !idSet.has(e.id)), - })); + if (idSet.size > 0) { + useEmailStore.setState((state) => ({ + emails: state.emails + .map((e) => { + if (!idSet.has(e.id)) return e; + const keywords = { ...e.keywords }; + for (const k of keywordsRemoved) delete keywords[k]; + if (keywordAdded) keywords[keywordAdded] = true; + return { ...e, keywords }; + }) + .filter((e) => !shouldDrop || !idSet.has(e.id)), + })); + } + if (failure) throw failure; void messageListTabHooks.onEmailCategorize.emit(ctx); return true; },