fix: report refused tags and re-check the sort order after a server change

Stalwart 1.0 refuses keywords over 128 bytes and more than 100
keywords on one message, per message in Email/set `notUpdated`; a
read-only shared folder refuses any keyword on every version. The app
stopped a large selection at the first refused batch, "mark folder as
read" ignored refusals, and the list's swipe, bulk and tag actions let
the rejection go unhandled, so nothing was said. The keyword sort
polarity verdict was stored per server address for a week, so a server
reinstalled as 1.0 (which sorts as the RFC says, where 0.16.8-0.16.22
invert) kept the old reading.

- Email/set writes send every batch before reporting the first refused
  message, and "mark folder as read" marks the rest, then reports the
  refusal. The list shows "Tagging failed" or "Failed to update email"
  for swipe, bulk and tag actions, and the viewer's automatic
  mark-as-read puts the message back to unread when it is refused.
- The sort polarity verdict is keyed by the server fingerprint as well
  (stored as `keyword-sort-polarity:v2`), so a replaced or upgraded
  server is probed afresh. An upgrade that leaves the capabilities
  alone, such as 0.16.22 to 0.16.23, still waits out the week.
- Tests pin that Stalwart 1.0's `$hasattachment` / `$hasnoattachment`
  never show as tags or as tags to recover; the app lists no other
  keywords, and tag ids stay within 128 bytes by construction.
This commit is contained in:
Linus Rath
2026-09-25 20:59:19 +02:00
parent 42751a6b29
commit 2e26ff7b64
8 changed files with 285 additions and 21 deletions
@@ -0,0 +1,131 @@
import { describe, it, expect, vi, beforeEach } from 'vitest';
// Stalwart refuses a keyword per message in Email/set `notUpdated`, not as a
// request error. 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); a read-only shared folder refuses any on both.
vi.mock('../jmap-client', () => ({
jmapClient: {
accountId: 'f',
request: vi.fn(),
getMaxObjectsInGet: vi.fn(() => 500),
getMaxObjectsInSet: vi.fn(() => 500),
},
}));
import { jmapClient } from '../jmap-client';
import { markMailboxAsRead, patchKeywordsForEmails } from '../email';
import { isJMAPSetError, JMAPSetError } from '../jmap-result';
const mockRequest = jmapClient.request as ReturnType<typeof vi.fn>;
const mockMaxInSet = jmapClient.getMaxObjectsInSet as ReturnType<typeof vi.fn>;
// Exactly what Stalwart 1.0 answers (webmail d7004b38 captured these).
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'],
};
type Call = [string, Record<string, unknown>, string];
/** Answers each Email/set with `result(ids)` and each Email/query with `query()`. */
function serve(result: (ids: string[]) => Record<string, unknown>, query?: () => string[]) {
const sent: Call[] = [];
mockRequest.mockImplementation(async (calls: Call[]) => {
sent.push(...calls);
return {
methodResponses: calls.map(([name, args, id]) => {
if (name === 'Email/query') return [name, { accountId: 'f', ids: query?.() ?? [] }, id];
const ids = Object.keys((args.update ?? {}) as Record<string, unknown>);
return [name, { accountId: 'f', oldState: 'sfy', newState: 'sfz', ...result(ids) }, id];
}),
};
});
return sent;
}
const refuse = (error: Record<string, unknown>, which: (id: string) => boolean = () => true) => (ids: string[]) => ({
updated: Object.fromEntries(ids.filter((id) => !which(id)).map((id) => [id, null])),
notUpdated: Object.fromEntries(ids.filter(which).map((id) => [id, error])),
});
const accept = (ids: string[]) => ({ updated: Object.fromEntries(ids.map((id) => [id, null])) });
beforeEach(() => {
vi.clearAllMocks();
mockMaxInSet.mockReturnValue(500);
});
describe('keyword writes report what the server refused', () => {
it('rejects a tag over 128 bytes with the SetError on Stalwart 1.0', async () => {
serve(refuse(TOO_LONG));
const err = await patchKeywordsForEmails(['m1'], { [`$label:${'x'.repeat(130)}`]: true }).catch((e) => e);
expect(isJMAPSetError(err)).toBe(true);
expect(err).toMatchObject({ type: 'invalidProperties', properties: ['keywords'] });
expect((err as Error).message).toContain('Keywords cannot be longer than 128 bytes.');
});
it('rejects one keyword too many on Stalwart 1.0', async () => {
serve(refuse(TOO_MANY));
await expect(patchKeywordsForEmails(['m1'], { '$label:one-more': true }))
.rejects.toMatchObject({ type: 'tooManyKeywords' });
});
it('resolves when Stalwart 0.16 takes the write', async () => {
serve(accept);
await expect(patchKeywordsForEmails(['m1', 'm2'], { $flagged: true })).resolves.toBeUndefined();
});
it('writes every batch before reporting a refused message', async () => {
mockMaxInSet.mockReturnValue(1);
const sent = serve(refuse(TOO_MANY, (id) => id === 'm1'));
await expect(patchKeywordsForEmails(['m1', 'm2', 'm3'], { '$label:work': true }))
.rejects.toThrow(JMAPSetError);
expect(sent.map(([, args]) => Object.keys(args.update as Record<string, unknown>))).toEqual([['m1'], ['m2'], ['m3']]);
});
});
describe('markMailboxAsRead', () => {
it('marks what it can and then reports the refused messages (read-only shared folder)', async () => {
// The refused message stays unread and comes back in the next page.
const unread = ['m1', 'm2', 'm3'];
const sent = serve(
(ids) => {
const result = refuse({ type: 'forbidden', description: 'You are not allowed to modify this message.' }, (id) => id === 'm2')(ids);
for (const id of Object.keys(result.updated)) unread.splice(unread.indexOf(id), 1);
return result;
},
() => [...unread],
);
mockMaxInSet.mockReturnValue(3);
const err = await markMailboxAsRead('a').catch((e) => e);
expect(isJMAPSetError(err)).toBe(true);
expect(err).toMatchObject({ type: 'forbidden' });
// Two rounds: m1..m3, then the refused m2 alone, which ends the loop.
expect(sent.filter(([name]) => name === 'Email/set')).toHaveLength(2);
expect(unread).toEqual(['m2']);
});
it('returns the count when every message is marked (Stalwart 0.16 and 1.0)', async () => {
const unread = ['m1', 'm2'];
serve(
(ids) => {
for (const id of ids) unread.splice(unread.indexOf(id), 1);
return accept(ids);
},
() => [...unread],
);
expect(await markMailboxAsRead('a')).toBe(2);
});
});
+29 -4
View File
@@ -104,7 +104,10 @@ function stateWentBack(accountId: string, sinceState: string, newState: unknown)
* Run one `Email/set` per `maxObjectsInSet`-sized slice of `update` /
* `destroy`, checking every slice for `notUpdated` / `notDestroyed`. Over-limit
* requests fail whole (RFC 8620 §3.6.1), so a long multi-select or a big
* offline replay must be split before it is sent.
* offline replay must be split before it is sent. Every slice is sent before
* the first refusal is thrown: a refused message (Stalwart 1.0 refuses
* keywords over 128 bytes and more than 100 keywords on one message) must not
* leave the rest of a selection unchanged.
*/
async function emailSetBatched(
accountId: string,
@@ -113,21 +116,30 @@ async function emailSetBatched(
): Promise<{ updated: string[]; destroyed: string[] }> {
const out = { updated: [] as string[], destroyed: [] as string[] };
const size = maxInSet();
let refusal: unknown = null;
const check = (body: Record<string, any>, ids: string[]) => {
try {
assertSetResult(body, ids, what);
} catch (err) {
refusal ??= err;
}
};
const updateEntries = Object.entries(args.update ?? {});
for (const slice of batched(updateEntries, size)) {
const res = await jmapClient.request([
['Email/set', { accountId, update: Object.fromEntries(slice) }, '0'],
]);
const body = requireMethodResult(res, '0', 'Email/set');
assertSetResult(body, slice.map(([id]) => id), what);
check(body, slice.map(([id]) => id));
out.updated.push(...Object.keys(body.updated ?? {}));
}
for (const slice of batched(args.destroy ?? [], size)) {
const res = await jmapClient.request([['Email/set', { accountId, destroy: slice }, '0']]);
const body = requireMethodResult(res, '0', 'Email/set');
assertSetResult(body, slice, what);
check(body, slice);
out.destroyed.push(...((body.destroyed as string[] | undefined) ?? []));
}
if (refusal) throw refusal;
return out;
}
@@ -339,11 +351,17 @@ export async function emptyMailbox(mailboxId: string, accountIdOverride?: string
return totalDestroyed;
}
/** Set `$seen` on every unread message in a folder. Returns the count. */
/**
* Set `$seen` on every unread message in a folder. Returns the count. Throws
* the first refusal once the rest are marked: a refused message stays unread
* (a shared folder without the right to set it, a message at Stalwart 1.0's
* 100-keyword limit), and the folder would otherwise look done.
*/
export async function markMailboxAsRead(mailboxId: string, accountIdOverride?: string): Promise<number> {
const accountId = accountIdOverride ?? jmapClient.accountId;
const pageSize = Math.min(500, maxInSet());
let total = 0;
let refusal: unknown = null;
for (;;) {
const res = await jmapClient.request([
['Email/query', {
@@ -359,8 +377,15 @@ export async function markMailboxAsRead(mailboxId: string, accountIdOverride?: s
const body = requireMethodResult(setRes, '0', 'Email/set');
const marked = Object.keys(body.updated ?? {}).length;
total += marked;
try {
assertSetResult(body, ids, 'email');
} catch (err) {
refusal ??= err;
}
// Refused messages come back in every page; stop once only they are left.
if (marked === 0 || ids.length < pageSize) break;
}
if (refusal) throw refusal;
return total;
}