diff --git a/app/api/dev-jmap/[...path]/route.ts b/app/api/dev-jmap/[...path]/route.ts index d2d4ca958..e5cc03c2e 100644 --- a/app/api/dev-jmap/[...path]/route.ts +++ b/app/api/dev-jmap/[...path]/route.ts @@ -1,4 +1,5 @@ import { NextRequest, NextResponse } from 'next/server'; +import { pointerTokenValue } from '@/lib/jmap/patch-pointer'; /** * Mock JMAP server for local development. @@ -1725,7 +1726,9 @@ function handleEmailSet(args: MethodArgs, callId: string): MethodResult { // Patch-style keyword updates: "keywords/$seen", "keywords/$flagged", etc. for (const [key, value] of Object.entries(changes)) { if (key.startsWith('keywords/')) { - const keyword = key.slice('keywords/'.length); + // The key is a JSON Pointer, so a nested tag arrives escaped + // (`$label:work~1clients`) and has to be read back to its real name. + const keyword = pointerTokenValue(key.slice('keywords/'.length)); if (value) { email.keywords[keyword] = true; } else { diff --git a/components/settings/keyword-settings.tsx b/components/settings/keyword-settings.tsx index d55317caa..d106d22b8 100644 --- a/components/settings/keyword-settings.tsx +++ b/components/settings/keyword-settings.tsx @@ -574,9 +574,15 @@ export function KeywordSettings() { try { const oldJmapKeyword = `$label:${oldId}`; const newJmapKeyword = `$label:${keyword.id}`; - await client.migrateKeyword(oldJmapKeyword, newJmapKeyword); + const migration = await client.migrateKeyword(oldJmapKeyword, newJmapKeyword); renameKeyword(oldId, keyword); fetchTagCounts(client); + if (migration.refused > 0) { + // Some mail kept the old keyword. The definition still follows the + // messages that did move; the rest is what the warning is about. + const toastModule = await import('sonner'); + toastModule.toast.warning(t("migration_error")); + } } catch (error) { console.error("Failed to migrate keyword:", error); const toastModule = await import('sonner'); diff --git a/lib/__tests__/jmap-keyword-pointer.test.ts b/lib/__tests__/jmap-keyword-pointer.test.ts new file mode 100644 index 000000000..02b0b483f --- /dev/null +++ b/lib/__tests__/jmap-keyword-pointer.test.ts @@ -0,0 +1,358 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { JMAPClient } from '../jmap/client'; +import { keywordPointer, pointerToken, pointerTokenValue } from '../jmap/patch-pointer'; + +// A nested tag's id contains a slash (`work/clients`), so its keyword is +// `$label:work/clients`. PatchObject keys are JSON Pointers (RFC 8620 section +// 5.3), where an unescaped slash starts a new path segment - the server reads +// `keywords/$label:work/clients` as "the `clients` member of the `$label:work` +// keyword", which is not a boolean and is rejected or ignored. Nested tags can +// therefore only be written with the slash escaped as `~1`. + +function createClient(): JMAPClient { + const client = new JMAPClient('https://jmap.example.com', 'user@example.com', 'pass'); + Object.assign(client, { + apiUrl: 'https://jmap.example.com/api', + accountId: 'primary-account', + username: 'user@example.com', + }); + return client; +} + +interface JMAPMethodCall { + 0: string; + 1: Record; + 2: string; +} + +/** + * Answers Email/query with one id and then nothing - the walk ends on an empty + * page - and Email/set with a plain success. + */ +function mockJmap() { + const captured: JMAPMethodCall[] = []; + let queried = false; + const fetchSpy = vi.spyOn(globalThis, 'fetch'); + fetchSpy.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) => { + if (call[0] !== 'Email/query') return ['Email/set', { updated: { m1: null } }, call[2]]; + const ids = queried ? [] : ['m1']; + queried = true; + return ['Email/query', { ids }, call[2]]; + }); + return new Response(JSON.stringify({ methodResponses }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }); + }); + return { captured, fetchSpy }; +} + +function patchOf(captured: JMAPMethodCall[], emailId = 'm1'): Record { + const set = captured.find((call) => call[0] === 'Email/set'); + const update = set?.[1].update as Record> | undefined; + return update?.[emailId] ?? {}; +} + +describe('pointerToken', () => { + it('escapes a slash as ~1', () => { + expect(pointerToken('work/clients')).toBe('work~1clients'); + }); + + it('escapes a tilde as ~0, and does so before slashes', () => { + expect(pointerToken('a~/b')).toBe('a~0~1b'); + expect(pointerToken('a~1b')).toBe('a~01b'); + }); + + it('leaves a token with nothing to escape alone', () => { + expect(keywordPointer('$label:red')).toBe('keywords/$label:red'); + }); +}); + +describe('pointerTokenValue', () => { + it('reads back what pointerToken wrote', () => { + for (const value of ['work/clients', 'a~/b', 'a~1b', '$label:red', '~0/~1']) { + expect(pointerTokenValue(pointerToken(value))).toBe(value); + } + }); + + it('reads ~1 before ~0, so an escaped tilde stays literal', () => { + expect(pointerTokenValue('~01')).toBe('~1'); + }); +}); + +describe('keyword writes escape the JSON Pointer', () => { + beforeEach(() => vi.restoreAllMocks()); + afterEach(() => vi.restoreAllMocks()); + + it('setKeyword escapes a nested tag keyword', async () => { + const { captured } = mockJmap(); + await createClient().setKeyword('m1', '$label:work/clients'); + expect(patchOf(captured)).toEqual({ 'keywords/$label:work~1clients': true }); + }); + + it('removeKeyword escapes a nested tag keyword', async () => { + const { captured } = mockJmap(); + await createClient().removeKeyword('m1', '$label:work/clients'); + expect(patchOf(captured)).toEqual({ 'keywords/$label:work~1clients': null }); + }); + + it('migrateKeyword escapes both the old and the new keyword', async () => { + const { captured } = mockJmap(); + await createClient().migrateKeyword('$label:work/clients', '$label:work/acme'); + expect(patchOf(captured)).toEqual({ + 'keywords/$label:work~1clients': null, + 'keywords/$label:work~1acme': true, + }); + }); + + it('leaves a flat keyword unchanged', async () => { + const { captured } = mockJmap(); + await createClient().setKeyword('m1', '$label:red'); + expect(patchOf(captured)).toEqual({ 'keywords/$label:red': true }); + }); +}); + +describe('migrateKeyword reports a server that refused', () => { + beforeEach(() => vi.restoreAllMocks()); + afterEach(() => vi.restoreAllMocks()); + + /** + * Answers queries from `pages` in order (an empty page ends the walk) and + * every Email/set with `setResult`. A page given as an error tuple is + * returned as the method response itself. + */ + function mockRun( + pages: (string[] | ['error', Record])[], + setResult: Record | ['error', Record] = { updated: {} }, + ) { + let page = 0; + vi.spyOn(globalThis, 'fetch').mockImplementation(async (_url, init) => { + const body = JSON.parse((init as { body: string }).body) as { methodCalls: JMAPMethodCall[] }; + const methodResponses = body.methodCalls.map((call) => { + if (call[0] === 'Email/query') { + const next = pages[Math.min(page, pages.length - 1)]; + page++; + if (Array.isArray(next) && next[0] === 'error') { + return ['error', next[1] as Record, call[2]]; + } + return ['Email/query', { ids: next as string[] }, call[2]]; + } + if (Array.isArray(setResult) && setResult[0] === 'error') { + return ['error', setResult[1] as Record, call[2]]; + } + return ['Email/set', setResult as Record, call[2]]; + }); + return new Response(JSON.stringify({ methodResponses }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }); + }); + } + + it('throws when the query for tagged mail fails instead of reading it as none', async () => { + mockRun([['error', { type: 'serverFail', description: 'boom' }]]); + await expect(createClient().migrateKeyword('$label:red', '$label:iso')).rejects.toThrow('boom'); + }); + + it('throws when the update fails outright', async () => { + mockRun([['m1'], []], ['error', { type: 'serverFail', description: 'no writes today' }]); + await expect(createClient().migrateKeyword('$label:red', '$label:iso')).rejects.toThrow( + 'no writes today', + ); + }); + + it('counts only the messages the server actually updated', async () => { + mockRun([['m1', 'm2'], []], { updated: { m1: null }, notUpdated: { m2: { type: 'notFound' } } }); + await expect(createClient().migrateKeyword('$label:red', '$label:iso')).resolves.toEqual({ + migrated: 1, + refused: 0, + }); + }); + + it('reports a message refused for a reason other than being gone', async () => { + mockRun([['m1', 'm2'], []], { updated: { m1: null }, notUpdated: { m2: { type: 'forbidden' } } }); + // Throwing would abandon m1, which this same call already migrated. + await expect(createClient().migrateKeyword('$label:red', '$label:iso')).resolves.toEqual({ + migrated: 1, + refused: 1, + }); + }); + + it('keeps what it migrated when a later batch fails outright', async () => { + // One message per Email/set, so the first write lands before the second + // fails: throwing would strand m1 under the new keyword with the tag + // definition still on the old one. + const client = createClient(); + Object.assign(client, { capabilities: { 'urn:ietf:params:jmap:core': { maxObjectsInSet: 1 } } }); + let set = 0; + vi.spyOn(globalThis, 'fetch').mockImplementation(async (_url, init) => { + const body = JSON.parse((init as { body: string }).body) as { methodCalls: JMAPMethodCall[] }; + const methodResponses = body.methodCalls.map((call) => { + if (call[0] === 'Email/query') { + return ['Email/query', { ids: set === 0 ? ['m1', 'm2'] : [], queryState: 's1' }, call[2]]; + } + set++; + return set === 1 + ? ['Email/set', { updated: { m1: null } }, call[2]] + : ['error', { type: 'serverFail', description: 'gave up' }, call[2]]; + }); + return new Response(JSON.stringify({ methodResponses }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }); + }); + + await expect(client.migrateKeyword('$label:red', '$label:iso')).resolves.toEqual({ + migrated: 1, + refused: 1, + }); + }); + + it('throws when nothing at all could be migrated', async () => { + mockRun([['m1'], []], { notUpdated: { m1: { type: 'forbidden' } } }); + await expect(createClient().migrateKeyword('$label:red', '$label:iso')).rejects.toThrow( + 'forbidden', + ); + }); + + it('restarts the walk when the query state changes under it', async () => { + // Positions only line up within one queryState. The first page belongs to a + // state that is gone by the next request; a walk that kept its position (and + // the ids it had collected) would migrate mail the tag is no longer on and + // miss the mail it is. + const current = ['m1', 'm2']; + let queries = 0; + const touched: string[] = []; + vi.spyOn(globalThis, 'fetch').mockImplementation(async (_url, init) => { + const body = JSON.parse((init as { body: string }).body) as { methodCalls: JMAPMethodCall[] }; + const methodResponses = body.methodCalls.map((call) => { + if (call[0] === 'Email/query') { + queries++; + if (queries === 1) { + return ['Email/query', { ids: ['stale1', 'stale2'], queryState: 's1' }, call[2]]; + } + const position = call[1].position as number; + const limit = call[1].limit as number; + return ['Email/query', { ids: current.slice(position, position + limit), queryState: 's2' }, call[2]]; + } + const update = call[1].update as Record; + touched.push(...Object.keys(update)); + return ['Email/set', { updated: Object.fromEntries(Object.keys(update).map((id) => [id, null])) }, call[2]]; + }); + return new Response(JSON.stringify({ methodResponses }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }); + }); + + await expect(createClient().migrateKeyword('$label:red', '$label:iso')).resolves.toEqual({ + migrated: 2, + refused: 0, + }); + expect(touched).toEqual(['m1', 'm2']); + }); + + it('keeps migrating later batches after one message is refused', async () => { + // A refusal is per message: the mail behind it in the queue is unaffected, + // and skipping it would leave far more behind than the server refused. + const client = createClient(); + Object.assign(client, { capabilities: { 'urn:ietf:params:jmap:core': { maxObjectsInSet: 1 } } }); + const touched: string[] = []; + let queried = false; + vi.spyOn(globalThis, 'fetch').mockImplementation(async (_url, init) => { + const body = JSON.parse((init as { body: string }).body) as { methodCalls: JMAPMethodCall[] }; + const methodResponses = body.methodCalls.map((call) => { + if (call[0] === 'Email/query') { + const ids = queried ? [] : ['m1', 'm2']; + queried = true; + return ['Email/query', { ids, queryState: 's1' }, call[2]]; + } + const id = Object.keys(call[1].update as Record)[0]; + touched.push(id); + return id === 'm1' + ? ['Email/set', { notUpdated: { m1: { type: 'forbidden' } } }, call[2]] + : ['Email/set', { updated: { [id]: null } }, call[2]]; + }); + return new Response(JSON.stringify({ methodResponses }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }); + }); + + await expect(client.migrateKeyword('$label:red', '$label:iso')).resolves.toEqual({ + migrated: 1, + refused: 1, + }); + expect(touched).toEqual(['m1', 'm2']); + }); + + it('gives up before writing anything when the mailbox will not hold still', async () => { + // Each query reports a new state, so no page ever belongs with the last. + // Mixing them would retag mail the tag has since been taken off. + let queries = 0; + const sets: string[] = []; + vi.spyOn(globalThis, 'fetch').mockImplementation(async (_url, init) => { + const body = JSON.parse((init as { body: string }).body) as { methodCalls: JMAPMethodCall[] }; + const methodResponses = body.methodCalls.map((call) => { + if (call[0] === 'Email/query') { + queries++; + return ['Email/query', { ids: [`m${queries}`], queryState: `s${queries}` }, call[2]]; + } + sets.push(call[2]); + return ['Email/set', { updated: {} }, call[2]]; + }); + return new Response(JSON.stringify({ methodResponses }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }); + }); + + await expect(createClient().migrateKeyword('$label:red', '$label:iso')).rejects.toThrow( + /kept changing/, + ); + expect(sets).toEqual([]); + }); + + it('ends the walk when a page repeats ids instead of paginating', async () => { + // A server that answers every position with the same page would otherwise + // be paged forever. + mockRun([['m1']]); + await expect(createClient().migrateKeyword('$label:red', '$label:iso')).resolves.toEqual({ + migrated: 1, + refused: 0, + }); + }); + + it('keeps paging when the server returns fewer ids than the limit asked for', async () => { + // RFC 8620 section 5.5 lets a server clamp the limit. Treating a short page + // as the last one would silently migrate only the first page. + const pages = [['m1', 'm2'], ['m3'], []]; + let query = 0; + vi.spyOn(globalThis, 'fetch').mockImplementation(async (_url, init) => { + const body = JSON.parse((init as { body: string }).body) as { methodCalls: JMAPMethodCall[] }; + const methodResponses = body.methodCalls.map((call) => { + if (call[0] === 'Email/query') { + const ids = pages[Math.min(query, pages.length - 1)]; + query++; + return ['Email/query', { ids }, call[2]]; + } + const update = call[1].update as Record; + const updated = Object.fromEntries(Object.keys(update).map((id) => [id, null])); + return ['Email/set', { updated }, call[2]]; + }); + return new Response(JSON.stringify({ methodResponses }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }); + }); + + await expect(createClient().migrateKeyword('$label:red', '$label:iso')).resolves.toEqual({ + migrated: 3, + refused: 0, + }); + expect(query).toBe(3); + }); +}); diff --git a/lib/demo/demo-client.ts b/lib/demo/demo-client.ts index 25acd36ac..d9133b5f4 100644 --- a/lib/demo/demo-client.ts +++ b/lib/demo/demo-client.ts @@ -1,4 +1,4 @@ -import type { IJMAPClient, KeywordDiscoveryResult } from '@/lib/jmap/client-interface'; +import type { IJMAPClient, KeywordDiscoveryResult, KeywordMigration } from '@/lib/jmap/client-interface'; import type { Email, Mailbox, StateChange, AccountStates, Thread, Identity, EmailAddress, ContactCard, AddressBook, VacationResponse, Calendar, CalendarEvent, CalendarEventFilter, CalendarTask, FileNode, ScheduledEmail, SendEmailResult, SharedAccount } from '@/lib/jmap/types'; import type { SieveScript, SieveCapabilities } from '@/lib/jmap/sieve-types'; import { getDemoData, type DemoData } from './demo-data'; @@ -382,16 +382,16 @@ export class DemoJMAPClient implements IJMAPClient { } } - async migrateKeyword(oldKeyword: string, newKeyword: string): Promise { - let count = 0; + async migrateKeyword(oldKeyword: string, newKeyword: string): Promise { + let migrated = 0; for (const email of this.data.emails) { if (email.keywords[oldKeyword]) { delete email.keywords[oldKeyword]; email.keywords[newKeyword] = true; - count++; + migrated++; } } - return count; + return { migrated, refused: 0 }; } async deleteEmail(emailId: string): Promise { diff --git a/lib/jmap/client-interface.ts b/lib/jmap/client-interface.ts index d38348f28..961028843 100644 --- a/lib/jmap/client-interface.ts +++ b/lib/jmap/client-interface.ts @@ -1,6 +1,17 @@ import type { Email, Mailbox, StateChange, AccountStates, Thread, Identity, EmailAddress, ContactCard, AddressBook, AddressBookRights, VacationResponse, Calendar, CalendarRights, CalendarEvent, CalendarEventFilter, CalendarTask, FileNode, FileNodeRights, Principal, PushSubscription, ScheduledEmail, SendEmailResult, SharedAccount } from "./types"; import type { SieveScript, SieveCapabilities } from "./sieve-types"; +/** What `migrateKeyword` managed to do. */ +export interface KeywordMigration { + /** Messages now carrying the new keyword. */ + migrated: number; + /** + * Messages the server refused to update for a reason other than being gone. + * Those still carry the old keyword. + */ + refused: number; +} + export interface KeywordInfo { id: string; name: string; @@ -153,7 +164,13 @@ export interface IJMAPClient { removeKeyword(emailId: string, keyword: string, accountId?: string): Promise; /** Apply one `keywords/` patch fragment (true=add, null=remove) to many messages in a single Email/set. */ batchUpdateKeywords(emailIds: string[], patch: Record, accountId?: string): Promise; - migrateKeyword(oldKeyword: string, newKeyword: string): Promise; + /** + * Rewrite one keyword to another on every message that carries it, in + * batches. Fails outright only when the server refuses the whole call; + * per-message refusals are reported in the result, since the messages + * migrated alongside them stay migrated. + */ + migrateKeyword(oldKeyword: string, newKeyword: string): Promise; deleteEmail(emailId: string, accountId?: string): Promise; moveToTrash(emailId: string, trashMailboxId: string, accountId?: string, markAsRead?: boolean): Promise; batchDeleteEmails(emailIds: string[], accountId?: string): Promise; diff --git a/lib/jmap/client.ts b/lib/jmap/client.ts index ec42ecc7b..a164a0adf 100644 --- a/lib/jmap/client.ts +++ b/lib/jmap/client.ts @@ -1,8 +1,9 @@ import type { Email, Mailbox, StateChange, AccountStates, Thread, Identity, EmailAddress, ContactCard, AddressBook, AddressBookRights, VacationResponse, Calendar, CalendarRights, CalendarEvent, CalendarEventFilter, CalendarTask, FileNode, FileNodeFilter, FileNodeRights, Principal, PushSubscription, EmailSubmission, ScheduledEmail, SendEmailResult, SharedAccount } from "./types"; import type { SieveScript, SieveCapabilities } from "./sieve-types"; -import type { IJMAPClient, KeywordDiscoveryResult, KeywordInfo } from "./client-interface"; +import type { IJMAPClient, KeywordDiscoveryResult, KeywordInfo, KeywordMigration } from "./client-interface"; import { toWildcardQuery } from "./search-utils"; import { batched, itemsPerRequest } from "./request-limits"; +import { keywordPointer } from "./patch-pointer"; import { debug } from "@/lib/debug"; import { normalizeCalendarEventLike } from "@/lib/calendar-event-normalization"; import { findTasksOnlyCalendarIds, isTaskLikeObject, type ScannedCalendarObject } from "@/lib/calendar-component-detection"; @@ -1756,7 +1757,7 @@ export class JMAPClient implements IJMAPClient { accountId: accountId || this.accountId, update: { [emailId]: { - [`keywords/${keyword}`]: true, + [keywordPointer(keyword)]: true, }, }, }, "0"], @@ -1769,7 +1770,7 @@ export class JMAPClient implements IJMAPClient { accountId: accountId || this.accountId, update: { [emailId]: { - [`keywords/${keyword}`]: null, + [keywordPointer(keyword)]: null, }, }, }, "0"], @@ -1791,11 +1792,18 @@ export class JMAPClient implements IJMAPClient { } } - async migrateKeyword(oldKeyword: string, newKeyword: string): Promise { + async migrateKeyword(oldKeyword: string, newKeyword: string): Promise { // Query all email IDs that have the old keyword + const seen = new Set(); const allIds: string[] = []; let position = 0; const batchSize = 100; + // Positions only line up within one queryState (RFC 8620 section 5.5). + // Mail arriving mid-walk shifts every later page, so a changed state + // restarts the walk - bounded, since a busy mailbox could change forever. + let queryState: string | undefined; + let restarts = 0; + const maxRestarts = 3; while (true) { const response = await this.request([ @@ -1807,35 +1815,106 @@ export class JMAPClient implements IJMAPClient { }, "0"], ]); - const queryResult = response.methodResponses?.[0]?.[1]; - const ids: string[] = queryResult?.ids || []; - allIds.push(...ids); + const [queryMethod, queryResult] = response.methodResponses?.[0] || []; + if (queryMethod === "error") { + // Reading a refusal as "no messages carry the tag" would report a + // clean migration of mail that still has the old keyword. + throw new Error(queryResult?.description || "Failed to list emails with the keyword"); + } + const state = queryResult?.queryState as string | undefined; + if (queryState === undefined) { + queryState = state; + } else if (state !== undefined && state !== queryState) { + if (restarts >= maxRestarts) { + // Carrying on would mix ids from states that no longer agree, and + // retagging a message the tag has since been taken off is worse than + // not retagging at all. Nothing has been written yet, so this is a + // clean failure the caller can simply repeat. + throw new Error("The mailbox kept changing while listing the messages to retag"); + } + restarts++; + queryState = state; + seen.clear(); + allIds.length = 0; + position = 0; + continue; + } - if (ids.length < batchSize) break; + const ids: string[] = queryResult?.ids || []; + const fresh = ids.filter(id => !seen.has(id)); + for (const id of fresh) seen.add(id); + allIds.push(...fresh); + + // A server may return fewer ids than the limit asked for (RFC 8620 + // section 5.5 lets it clamp), so a short page does not end the walk. + // A page carrying nothing new under an unchanged state does: the walk + // is not advancing, and continuing would loop for as long as the server + // keeps answering. Position counts the server's own page, not the ids + // this walk had not seen before. + if (fresh.length === 0) break; position += ids.length; } - if (allIds.length === 0) return 0; + if (allIds.length === 0) return { migrated: 0, refused: 0 }; // Batch update: remove old keyword, add new keyword using per-property patches + let migrated = 0; + let refusals = 0; + // A whole batch the server refused to take: it will not take the next one + // either. A single message it refused: the rest of the mail is unaffected, + // and only the reason is worth keeping. + let stopped: Error | null = null; + let refusal: Error | null = null; for (const batch of batched(allIds, this.getMaxObjectsInSet())) { + if (stopped) { + refusals += batch.length; + continue; + } + const update: Record> = {}; for (const id of batch) { update[id] = { - [`keywords/${oldKeyword}`]: null, - [`keywords/${newKeyword}`]: true, + [keywordPointer(oldKeyword)]: null, + [keywordPointer(newKeyword)]: true, }; } - await this.request([ + const response = await this.request([ ["Email/set", { accountId: this.accountId, update, }, "0"], ]); + + const [setMethod, setResult] = response.methodResponses?.[0] || []; + if (setMethod === "error") { + stopped = new Error(setResult?.description || "Failed to migrate keyword"); + refusals += batch.length; + continue; + } + // Email/set applies per message, so a refusal is never all-or-nothing: + // throwing here would abandon messages this very call already migrated. + // Report instead - a message deleted since the query (notFound) is + // nothing to report, any other refusal leaves the old keyword in place + // on mail the caller would otherwise treat as migrated. + const notUpdated = (setResult?.notUpdated || {}) as Record; + const failed = Object.entries(notUpdated); + const refused = failed.filter(([, error]) => error?.type !== 'notFound'); + if (refused.length > 0) { + debug.warn('keywords', 'Keyword migration refused for', refused.length, 'message(s):', refused[0][1]); + refusal = refusal || new Error(refused[0][1]?.description || refused[0][1]?.type || "Failed to migrate keyword"); + } + refusals += refused.length; + migrated += batch.length - failed.length; } - return allIds.length; + // Nothing moved: the caller must not rename the tag over messages that all + // kept the old keyword. Something moved: that is a partial migration the + // caller reports, not an error - the messages that did move are migrated + // and renaming the definition is what keeps them named. + if (migrated === 0 && (stopped || refusal)) throw (stopped || refusal) as Error; + + return { migrated, refused: refusals }; } async deleteEmail(emailId: string, accountId?: string): Promise { diff --git a/lib/jmap/patch-pointer.ts b/lib/jmap/patch-pointer.ts new file mode 100644 index 000000000..af860631b --- /dev/null +++ b/lib/jmap/patch-pointer.ts @@ -0,0 +1,29 @@ +/** + * Naming one member of an object in a JMAP PatchObject. + * + * A PatchObject key is a JSON Pointer (RFC 8620 section 5.3), so a slash in the + * member's own name starts a new path segment unless it is escaped. That bites + * keywords: a nested tag is stored as `$label:work/clients`, and the unescaped + * pointer `keywords/$label:work/clients` asks for the `clients` member of the + * `$label:work` keyword - not a boolean, so the patch is rejected or ignored + * and the tag never lands. + */ + +/** One JSON Pointer reference token: `~` -> `~0`, `/` -> `~1` (RFC 6901). */ +export function pointerToken(value: string): string { + return value.replace(/~/g, '~0').replace(/\//g, '~1'); +} + +/** + * The member name a reference token stands for: the inverse of + * `pointerToken`. `~1` is read before `~0`, per RFC 6901 - the other order + * would turn the escaped `~1` of `~01` back into a slash. + */ +export function pointerTokenValue(token: string): string { + return token.replace(/~1/g, '/').replace(/~0/g, '~'); +} + +/** The PatchObject key that sets or clears the keyword `keyword`. */ +export function keywordPointer(keyword: string): string { + return `keywords/${pointerToken(keyword)}`; +} diff --git a/stores/message-list-tabs-store.ts b/stores/message-list-tabs-store.ts index 7b0e74157..8bae86d2f 100644 --- a/stores/message-list-tabs-store.ts +++ b/stores/message-list-tabs-store.ts @@ -15,6 +15,7 @@ import type { Email } from '@/lib/jmap/types'; 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 { useEmailStore } from './email-store'; // ─── Validation ────────────────────────────────────────────── @@ -296,8 +297,8 @@ export const useMessageListTabsStore = create()((set, get) // One PatchObject per message: drop every other category keyword, add the // target's (removals of absent keywords are no-ops per RFC 8620 §5.3). const patch: Record = {}; - for (const k of keywordsRemoved) patch[`keywords/${k}`] = null; - if (keywordAdded) patch[`keywords/${keywordAdded}`] = true; + for (const k of keywordsRemoved) patch[keywordPointer(k)] = null; + if (keywordAdded) patch[keywordPointer(keywordAdded)] = true; // Group by owning account so shared-mailbox messages patch correctly. const byAccount = new Map();