From 15ad783848e16c01d188fee705335b061c55703b Mon Sep 17 00:00:00 2001 From: Stefan Hildebrandt <695494+hildebrandttk@users.noreply.github.com> Date: Tue, 21 Jul 2026 07:58:42 +0200 Subject: [PATCH 1/3] fix(email): make the "Move to" context menu work across accounts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Moving a message to a folder in another account (own ↔ delegated/shared) via the "Move to" context menu was a no-op — the handlers always issued a single-account Email/set, which can't move between JMAP accounts. Drag-and-drop already routed these correctly; the context menu never did. Add moveToMailboxCrossAware: it detects a cross-account destination (own and shared mailboxes both carry accountId) and routes through the drag-and-drop crossAccountMoveEmails pipeline, else falls back to the single-account move. Fix the pipeline for delegated folders too: a client can't stage a blob in a delegated account (Blob/upload → blobNotFound), so importing into a shared folder failed. When one client reaches both accounts, use a server-side JMAP Email/copy (+ destroy original) instead of blob copy+import; the blob path is kept only for separate cross-server login accounts. Adds client.copyEmailAcrossAccounts. Unit tests for the dispatch; the two 08-shared-moves specs are un-pinned. Full docker integration suite green (37 passed). --- app/(main)/[locale]/page.tsx | 5 +- integration/tests/08-shared-moves.spec.ts | 11 +- lib/demo/demo-client.ts | 1 + lib/jmap/client-interface.ts | 8 ++ lib/jmap/client.ts | 26 +++++ .../email-store-cross-account-move.test.ts | 102 ++++++++++++++++++ stores/email-store.ts | 93 +++++++++++++++- 7 files changed, 234 insertions(+), 12 deletions(-) create mode 100644 stores/__tests__/email-store-cross-account-move.test.ts diff --git a/app/(main)/[locale]/page.tsx b/app/(main)/[locale]/page.tsx index 547b66dc..7345bb80 100644 --- a/app/(main)/[locale]/page.tsx +++ b/app/(main)/[locale]/page.tsx @@ -282,6 +282,7 @@ export default function Home() { toggleStar, setEmailKeywordsLocal, moveToMailbox, + moveToMailboxCrossAware, moveThreadToMailbox, searchEmails, searchQuery, @@ -3229,7 +3230,7 @@ export default function Home() { }} onMoveToMailbox={async (emailId, mailboxId) => { if (client) { - await moveToMailbox(client, emailId, mailboxId); + await moveToMailboxCrossAware(client, emailId, mailboxId); } }} onMarkAsSpam={async (email) => { @@ -3499,7 +3500,7 @@ export default function Home() { selectedMailbox={selectedMailbox} onMoveToMailbox={async (mailboxId) => { if (client && selectedEmail) { - await moveToMailbox(client, selectedEmail.id, mailboxId); + await moveToMailboxCrossAware(client, selectedEmail.id, mailboxId); } }} className={isMobile ? "flex-1" : undefined} diff --git a/integration/tests/08-shared-moves.spec.ts b/integration/tests/08-shared-moves.spec.ts index b508d795..0ac079ff 100644 --- a/integration/tests/08-shared-moves.spec.ts +++ b/integration/tests/08-shared-moves.spec.ts @@ -77,12 +77,9 @@ test.describe('Shared-folder moves', () => { expect(await ja.findEmailBySubject(s, teamB), 'message left TeamB').toBeFalsy(); }); - // KNOWN LIMITATION (documented via test.fail): the "Move to" submenu offers a - // shared folder as a destination for an own-account message, but clicking it - // does NOT relocate the message across the account boundary — it stays put. - // Same in reverse (shared -> own). If cross-account moves get implemented, - // these will start passing; flip them back to plain tests then. - test.fail('own account -> shared folder', async ({ page }) => { + // The "Move to" submenu relocates a message across the account boundary + // (own ↔ shared folder) via copy+delete, matching drag-and-drop. + test('own account -> shared folder', async ({ page }) => { const s = subj('mv-own2sh'); await sendMail({ from: carol.email, authPass: carol.password, to: carol.email, subject: s, body: 'x' }); await jc.waitForEmail(s); @@ -100,7 +97,7 @@ test.describe('Shared-folder moves', () => { expect(await ja.findEmailBySubject(s, teamA), 'message in shared TeamA').toBeTruthy(); }); - test.fail('shared folder -> own account', async ({ page }) => { + test('shared folder -> own account', async ({ page }) => { const s = subj('mv-sh2own'); await seedInto(teamA, s); diff --git a/lib/demo/demo-client.ts b/lib/demo/demo-client.ts index 76aa0104..79b5fc67 100644 --- a/lib/demo/demo-client.ts +++ b/lib/demo/demo-client.ts @@ -1094,6 +1094,7 @@ export class DemoJMAPClient implements IJMAPClient { // ── S/MIME raw-email helpers ────────────────────────────────── async importRawEmail(): Promise { return generateDemoId('email'); } + async copyEmailAcrossAccounts(): Promise { return generateDemoId('email'); } async submitEmail(): Promise { /* no-op */ } async submitRawEmail(blob: Blob, identityId: string, diff --git a/lib/jmap/client-interface.ts b/lib/jmap/client-interface.ts index aef7cc44..35e6f1ad 100644 --- a/lib/jmap/client-interface.ts +++ b/lib/jmap/client-interface.ts @@ -345,4 +345,12 @@ export interface IJMAPClient { // ── S/MIME raw-email helpers ────────────────────────────────── importRawEmail(blob: Blob, mailboxIds: Record, keywords?: Record, accountId?: string): Promise; submitEmail(emailId: string, identityId: string): Promise; + /** + * Server-side move of one email across accounts reachable through THIS client + * (JMAP `Email/copy` + destroy-original). Used for delegated/shared folders, + * where the two accounts share a client but a client can't stage a blob in a + * delegated account (so the blob copy+import path doesn't work). Returns the + * new email id in the destination account. + */ + copyEmailAcrossAccounts(emailId: string, fromAccountId: string, toAccountId: string, destMailboxId: string): Promise; } diff --git a/lib/jmap/client.ts b/lib/jmap/client.ts index d0071b6b..cddecc5b 100644 --- a/lib/jmap/client.ts +++ b/lib/jmap/client.ts @@ -6394,6 +6394,32 @@ export class JMAPClient implements IJMAPClient { * a shared mailbox owned by another user). When omitted, falls back to the * client's own primary account. */ + async copyEmailAcrossAccounts( + emailId: string, + fromAccountId: string, + toAccountId: string, + destMailboxId: string, + ): Promise { + const response = await this.request([ + ["Email/copy", { + fromAccountId, + accountId: toAccountId, + create: { c: { id: emailId, mailboxIds: { [destMailboxId]: true } } }, + onSuccessDestroyOriginal: true, + }, "0"], + ]); + const res = response.methodResponses?.[0]?.[1]; + const err = res?.notCreated?.c; + if (err) { + throw new Error(err.description || err.type || "Failed to copy email across accounts"); + } + const id = res?.created?.c?.id; + if (!id) { + throw new Error("Email/copy succeeded but no ID returned"); + } + return id; + } + async importRawEmail( blob: Blob, mailboxIds: Record, diff --git a/stores/__tests__/email-store-cross-account-move.test.ts b/stores/__tests__/email-store-cross-account-move.test.ts new file mode 100644 index 00000000..fcfdb9c2 --- /dev/null +++ b/stores/__tests__/email-store-cross-account-move.test.ts @@ -0,0 +1,102 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { useEmailStore } from '../email-store'; +import type { Email, Mailbox } from '@/lib/jmap/types'; +import type { IJMAPClient } from '@/lib/jmap/client-interface'; + +type Store = ReturnType; + +function makeMailbox(overrides: Partial): Mailbox { + return { + id: 'inbox', + name: 'Inbox', + sortOrder: 0, + totalEmails: 0, + unreadEmails: 0, + totalThreads: 0, + unreadThreads: 0, + myRights: { + mayReadItems: true, mayAddItems: true, mayRemoveItems: true, maySetSeen: true, + maySetKeywords: true, mayCreateChild: true, mayRename: true, mayDelete: true, maySubmit: true, + }, + isSubscribed: true, + isShared: false, + ...overrides, + }; +} + +function makeEmail(id: string, mailboxServerId: string): Email { + return { + id, threadId: `t-${id}`, mailboxIds: { [mailboxServerId]: true }, keywords: {}, + size: 100, receivedAt: new Date().toISOString(), + from: [{ name: 'X', email: 'x@example.com' }], to: [{ name: 'Y', email: 'y@example.com' }], + subject: id, preview: '', hasAttachment: false, textBody: [], htmlBody: [], bodyValues: {}, + }; +} + +// Own account (JMAP acct "jmap-A", reached via local account "local-A") and a +// delegated/shared folder owned by another JMAP account ("jmap-B"). +const ownInbox = makeMailbox({ id: 'inbox-A', role: 'inbox', accountId: 'jmap-A', originalId: 'srv-inbox-A' }); +const ownArchive = makeMailbox({ id: 'archive-A', role: 'archive', accountId: 'jmap-A', originalId: 'srv-archive-A' }); +const sharedTeamA = makeMailbox({ id: 'jmap-B:srv-teamA', name: 'TeamA', accountId: 'jmap-B', originalId: 'srv-teamA', isShared: true }); + +describe('email-store moveToMailboxCrossAware', () => { + let crossSpy: ReturnType; + let moveSpy: ReturnType; + const client = {} as IJMAPClient; + + beforeEach(() => { + const email = makeEmail('e1', 'srv-inbox-A'); + email.accountId = 'local-A'; + crossSpy = vi.fn().mockResolvedValue(undefined); + moveSpy = vi.fn().mockResolvedValue(undefined); + useEmailStore.setState({ + emails: [email], + mailboxes: [ownInbox, ownArchive, sharedTeamA], + selectedMailbox: 'inbox-A', + viewingAccountId: 'local-A', + isUnifiedView: false, + accountMailboxes: {}, + crossAccountMoveEmails: crossSpy as unknown as Store['crossAccountMoveEmails'], + moveToMailbox: moveSpy as unknown as Store['moveToMailbox'], + }); + }); + + it('routes an own → shared (cross-account) move through crossAccountMoveEmails', async () => { + await useEmailStore.getState().moveToMailboxCrossAware(client, 'e1', 'jmap-B:srv-teamA'); + + expect(moveSpy).not.toHaveBeenCalled(); + // copy into the owner's (jmap-B) TeamA via the viewer's client, using the + // destination's raw server id; source is own, so no source override. + expect(crossSpy).toHaveBeenCalledWith( + new Map([['local-A', ['e1']]]), + 'local-A', + 'srv-teamA', + 'jmap-B', + undefined, + ); + }); + + it('routes a same-account move through the single-account moveToMailbox', async () => { + await useEmailStore.getState().moveToMailboxCrossAware(client, 'e1', 'archive-A'); + + expect(crossSpy).not.toHaveBeenCalled(); + expect(moveSpy).toHaveBeenCalledWith(client, 'e1', 'archive-A'); + }); + + it('reverse: shared → own also routes cross-account (source override set)', async () => { + const email = makeEmail('e2', 'srv-teamA'); + email.accountId = 'local-A'; + useEmailStore.setState({ emails: [email], selectedMailbox: 'jmap-B:srv-teamA' }); + + await useEmailStore.getState().moveToMailboxCrossAware(client, 'e2', 'inbox-A'); + + expect(moveSpy).not.toHaveBeenCalled(); + expect(crossSpy).toHaveBeenCalledWith( + new Map([['local-A', ['e2']]]), + 'local-A', + 'srv-inbox-A', + undefined, // dest (own) not shared + 'jmap-B', // source shared → override to owner account + ); + }); +}); diff --git a/stores/email-store.ts b/stores/email-store.ts index 14ca81d2..785a4966 100644 --- a/stores/email-store.ts +++ b/stores/email-store.ts @@ -161,6 +161,14 @@ interface EmailStore { deleteEmail: (client: IJMAPClient, emailId: string, forceDelete?: boolean) => Promise; markAsRead: (client: IJMAPClient, emailId: string, read: boolean) => Promise; moveToMailbox: (client: IJMAPClient, emailId: string, mailboxId: string) => Promise; + /** + * Move a single email, routing across the account boundary when the + * destination folder is owned by a different JMAP account (a delegated/shared + * mailbox, or a different connected account) — the "Move to" context-menu + * equivalent of what drag-and-drop already does. Falls back to the plain + * single-account `moveToMailbox` when source and destination share an account. + */ + moveToMailboxCrossAware: (client: IJMAPClient, emailId: string, mailboxId: string) => Promise; moveEmailsToMailbox: (client: IJMAPClient, emailIds: string[], mailboxId: string) => Promise; moveThreadToMailbox: (client: IJMAPClient, emailId: string, mailboxId: string) => Promise; /** @@ -424,6 +432,23 @@ function resolveEmailActionContext( }; } +/** + * Local account id ("user@host") whose connected client owns `mailbox`. + * `mailbox.accountId` is the JMAP server's opaque id; map it back to a local + * client id, falling back to the viewing/active account — a delegated/shared + * folder has no separately-connected client, it's reached through the viewer's. + * Mirrors resolveDestAccountId in use-mailbox-drop.ts. + */ +function resolveDestLocalAccountId(mailbox: Mailbox): string | null { + const jmapId = mailbox.accountId; + if (jmapId) { + for (const [localId, client] of useAuthStore.getState().getAllConnectedClients()) { + if (client.getAccountId() === jmapId) return localId; + } + } + return useEmailStore.getState().viewingAccountId ?? useAuthStore.getState().activeAccountId; +} + /** * Builds the `UnifiedAccountClient[]` list used by every unified fan-out * action (browse, load-more, search). Each entry has a JMAP client plus a @@ -1595,6 +1620,54 @@ export const useEmailStore = create((set, get) => ({ } }, + moveToMailboxCrossAware: async (client, emailId, destinationMailboxId) => { + const state = get(); + const email = state.emails.find((e) => e.id === emailId); + if (!email) return; + + const { mailboxes } = resolveEmailActionContext(email, client); + const find = (id: string) => + mailboxes.find((mb) => mb.id === id) ?? state.mailboxes.find((mb) => mb.id === id); + const destMailbox = find(destinationMailboxId); + // A context-menu move acts on the visible list, so the source folder is the + // one currently open. + const sourceMailbox = find(state.selectedMailbox ?? ''); + + // Cross-account when the two folders live in different JMAP accounts (both + // own and shared mailboxes carry accountId, so this catches own↔shared too). + const isCrossAccount = + !!destMailbox && + !!sourceMailbox?.accountId && + !!destMailbox.accountId && + sourceMailbox.accountId !== destMailbox.accountId; + + if (!isCrossAccount) { + await get().moveToMailbox(client, emailId, destinationMailboxId); + return; + } + + const destAccountId = resolveDestLocalAccountId(destMailbox!); + const sourceAccountId = + email.accountId ?? state.viewingAccountId ?? useAuthStore.getState().activeAccountId; + if (!destAccountId || !sourceAccountId) { + // Can't resolve the local endpoints — fall back rather than drop the mail. + await get().moveToMailbox(client, emailId, destinationMailboxId); + return; + } + + // JMAP has no cross-account move: copy the raw message into the destination + // account's mailbox, then delete the original (crossAccountMoveEmails). The + // *Jmap* overrides target the owner account when a shared folder is reached + // through another user's client. + await get().crossAccountMoveEmails( + new Map([[sourceAccountId, [emailId]]]), + destAccountId, + destMailbox!.originalId ?? destMailbox!.id, + destMailbox!.isShared ? destMailbox!.accountId : undefined, + sourceMailbox?.isShared ? sourceMailbox.accountId : undefined, + ); + }, + moveToMailbox: async (client, emailId, destinationMailboxId) => { try { const email = get().emails.find(e => e.id === emailId); @@ -1763,9 +1836,23 @@ export const useEmailStore = create((set, get) => ({ // the source clean in the happy path. const results = await Promise.allSettled( emailIds.map(async (emailId) => { - // When the source is a delegated/shared mailbox, the email, - // its blob, and the destroy all live in the owner's JMAP - // account, not the source client's primary one. + // Delegated/shared folders: one client reaches both accounts, so a + // server-side Email/copy moves the message. A client can't stage a + // blob in a *delegated* account (blobNotFound), so the blob + // copy+import path below is only valid across separate login + // clients/servers. + if (sourceClient === destClient) { + await sourceClient.copyEmailAcrossAccounts( + emailId, + sourceJmapAccountId ?? sourceClient.getAccountId(), + destJmapAccountId ?? destClient.getAccountId(), + destMailboxId, + ); + return emailId; + } + // Separate clients (cross-server multi-account): the email, its + // blob, and the destroy all live in the owner's JMAP account, not + // the source client's primary one. const full = await sourceClient.getEmail(emailId, sourceJmapAccountId); if (!full?.blobId) { throw new Error('Source email has no raw blob to copy'); From 6248bb9825784635805035a2d6f7b8155f71e837 Mon Sep 17 00:00:00 2001 From: Stefan Hildebrandt <695494+hildebrandttk@users.noreply.github.com> Date: Tue, 21 Jul 2026 19:58:25 +0200 Subject: [PATCH 2/3] fix(email): preserve read state and remove source on cross-account move MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Moving a message across the account boundary (own ↔ shared folder, or between two owners' shared folders) left the original in the source folder and showed the moved copy as unread. Same-account moves were fine. Cause (verified against a live Stalwart): - Email/copy drops keywords unless the create sets them, so the copy lost $seen and arrived unread. - onSuccessDestroyOriginal is unreliable — the implicit destroy reports notFound and leaves the original behind (flaky), so the move duplicated. Fix (copyEmailAcrossAccounts): read the source keywords and carry them into the Email/copy create, then destroy the original with an explicit Email/set on the source account instead of onSuccessDestroyOriginal. Tests: 08-shared-moves now asserts the source is gone and the read state survives on every cross-account case, and adds a cross-owner shared → shared move (alice's folder → bob's folder). Confirmed red on the old code (3 cross-account cases fail), green with the fix. --- integration/tests/08-shared-moves.spec.ts | 82 +++++++++++++++++------ lib/jmap/client.ts | 23 ++++++- 2 files changed, 83 insertions(+), 22 deletions(-) diff --git a/integration/tests/08-shared-moves.spec.ts b/integration/tests/08-shared-moves.spec.ts index 0ac079ff..32901562 100644 --- a/integration/tests/08-shared-moves.spec.ts +++ b/integration/tests/08-shared-moves.spec.ts @@ -13,39 +13,50 @@ import { /** * Moving mail across the own-account / shared-folder boundary, in both - * directions, and between two shared folders. The move is driven from the list - * context menu's "Move to" submenu; the authoritative check is the server-side - * mailbox the message ends up in, with the reliably-updating (own-account) - * counters checked in the UI too. + * directions, and between two shared folders (same owner and across owners). + * The move is driven from the list context menu's "Move to" submenu; the + * authoritative check is the server-side mailbox the message ends up in. Each + * cross-account case also asserts the source copy is gone (no duplicate) and + * the read state survives the move (Email/copy drops keywords unless carried). */ -const { alice, carol } = ACCOUNTS; +const { alice, bob, carol } = ACCOUNTS; const subj = (l: string) => `IT ${l} ${Date.now()}`; test.describe('Shared-folder moves', () => { - let ja: JmapClient; // owner + let ja: JmapClient; // owner A + let jb: JmapClient; // owner B (cross-owner shared → shared) let jc: JmapClient; // grantee let teamA: string; let teamB: string; + let teamC: string; // owned by bob test.beforeEach(async () => { ja = await JmapClient.connect(alice.email, alice.password); + jb = await JmapClient.connect(bob.email, bob.password); jc = await JmapClient.connect(carol.email, carol.password); await ja.reset(); + await jb.reset(); await jc.reset(); teamA = await ja.createSharedFolder('TeamA', carol.email); teamB = await ja.createSharedFolder('TeamB', carol.email); + teamC = await jb.createSharedFolder('TeamC', carol.email); }); - async function seedInto(mailboxId: string, subject: string, owner = ja): Promise { - const acct = owner === ja ? alice : carol; + // Seed a message into a mailbox and mark it read, so a lost $seen after the + // move is observable as the moved copy coming back unread. + async function seedRead(mailboxId: string, subject: string, owner = ja): Promise { + const acct = owner === ja ? alice : owner === jb ? bob : carol; await sendMail({ from: acct.email, authPass: acct.password, to: acct.email, subject, body: 'x' }); const m = await owner.waitForEmail(subject); await owner.moveEmail(m.id, mailboxId); + await owner.setSeen(m.id, true); } - test('shared folder A -> shared folder B', async ({ page }) => { + const seenOf = (m: any) => Boolean(m?.keywords?.$seen); + + test('shared folder A -> shared folder B (same owner)', async ({ page }) => { const s = subj('mv-a2b'); - await seedInto(teamA, s); + await seedRead(teamA, s); await login(page, carol); await expandSharedFolders(page, alice.email); @@ -56,13 +67,15 @@ test.describe('Shared-folder moves', () => { await moveEmailTo(page, s, dest); await page.waitForTimeout(1500); - expect(await ja.findEmailBySubject(s, teamB), 'message in TeamB').toBeTruthy(); + const inB = await ja.findEmailBySubject(s, teamB); + expect(inB, 'message in TeamB').toBeTruthy(); expect(await ja.findEmailBySubject(s, teamA), 'message left TeamA').toBeFalsy(); + expect(seenOf(inB), 'read state kept').toBe(true); }); - test('shared folder B -> shared folder A', async ({ page }) => { + test('shared folder B -> shared folder A (same owner)', async ({ page }) => { const s = subj('mv-b2a'); - await seedInto(teamB, s); + await seedRead(teamB, s); await login(page, carol); await expandSharedFolders(page, alice.email); @@ -73,8 +86,32 @@ test.describe('Shared-folder moves', () => { await moveEmailTo(page, s, dest); await page.waitForTimeout(1500); - expect(await ja.findEmailBySubject(s, teamA), 'message in TeamA').toBeTruthy(); + const inA = await ja.findEmailBySubject(s, teamA); + expect(inA, 'message in TeamA').toBeTruthy(); expect(await ja.findEmailBySubject(s, teamB), 'message left TeamB').toBeFalsy(); + expect(seenOf(inA), 'read state kept').toBe(true); + }); + + // Cross-owner shared → shared: source is alice's account, destination is bob's, + // so this exercises the true cross-account Email/copy + destroy path. + test('shared folder (owner A) -> shared folder (owner B)', async ({ page }) => { + const s = subj('mv-a2c'); + await seedRead(teamA, s); + + await login(page, carol); + await expandSharedFolders(page, alice.email); + await expandSharedFolders(page, bob.email); + const dest = await folderMailboxId(page, { name: 'TeamC', shared: true }); + await openFolder(page, { name: 'TeamA', shared: true }); + await forceSync(page); + + await moveEmailTo(page, s, dest); + await page.waitForTimeout(2000); + + const inC = await jb.findEmailBySubject(s, teamC); + expect(inC, 'message in bob TeamC').toBeTruthy(); + expect(await ja.findEmailBySubject(s, teamA), 'message left alice TeamA').toBeFalsy(); + expect(seenOf(inC), 'read state kept').toBe(true); }); // The "Move to" submenu relocates a message across the account boundary @@ -82,7 +119,8 @@ test.describe('Shared-folder moves', () => { test('own account -> shared folder', async ({ page }) => { const s = subj('mv-own2sh'); await sendMail({ from: carol.email, authPass: carol.password, to: carol.email, subject: s, body: 'x' }); - await jc.waitForEmail(s); + const own = await jc.waitForEmail(s); + await jc.setSeen(own.id, true); await login(page, carol); await expandSharedFolders(page, alice.email); @@ -93,13 +131,15 @@ test.describe('Shared-folder moves', () => { await moveEmailTo(page, s, dest); await page.waitForTimeout(2000); - // Expected (once supported): the message moves to the owner's shared TeamA. - expect(await ja.findEmailBySubject(s, teamA), 'message in shared TeamA').toBeTruthy(); + const inTeam = await ja.findEmailBySubject(s, teamA); + expect(inTeam, 'message in shared TeamA').toBeTruthy(); + expect(await jc.findEmailBySubject(s), 'message left own account').toBeFalsy(); + expect(seenOf(inTeam), 'read state kept').toBe(true); }); test('shared folder -> own account', async ({ page }) => { const s = subj('mv-sh2own'); - await seedInto(teamA, s); + await seedRead(teamA, s); await login(page, carol); await expandSharedFolders(page, alice.email); @@ -110,7 +150,9 @@ test.describe('Shared-folder moves', () => { await moveEmailTo(page, s, dest); await page.waitForTimeout(2000); - // Expected (once supported): the message arrives in carol's own Inbox. - expect(await jc.findEmailBySubject(s), 'message in own account').toBeTruthy(); + const inOwn = await jc.findEmailBySubject(s); + expect(inOwn, 'message in own account').toBeTruthy(); + expect(await ja.findEmailBySubject(s, teamA), 'message left shared TeamA').toBeFalsy(); + expect(seenOf(inOwn), 'read state kept').toBe(true); }); }); diff --git a/lib/jmap/client.ts b/lib/jmap/client.ts index cddecc5b..ca18ff54 100644 --- a/lib/jmap/client.ts +++ b/lib/jmap/client.ts @@ -6400,12 +6400,18 @@ export class JMAPClient implements IJMAPClient { toAccountId: string, 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. + const srcResp = await this.request([ + ["Email/get", { accountId: fromAccountId, ids: [emailId], properties: ["keywords"] }, "0"], + ]); + const keywords = srcResp.methodResponses?.[0]?.[1]?.list?.[0]?.keywords ?? {}; + const response = await this.request([ ["Email/copy", { fromAccountId, accountId: toAccountId, - create: { c: { id: emailId, mailboxIds: { [destMailboxId]: true } } }, - onSuccessDestroyOriginal: true, + create: { c: { id: emailId, mailboxIds: { [destMailboxId]: true }, keywords } }, }, "0"], ]); const res = response.methodResponses?.[0]?.[1]; @@ -6417,6 +6423,19 @@ export class JMAPClient implements IJMAPClient { if (!id) { throw new Error("Email/copy succeeded but no ID returned"); } + + // onSuccessDestroyOriginal is unreliable on Stalwart (implicit destroy reports + // notFound and leaves the original behind), so remove the source explicitly. + const delResp = await this.request([ + ["Email/set", { accountId: fromAccountId, destroy: [emailId] }, "0"], + ]); + const notDestroyed = delResp.methodResponses?.[0]?.[1]?.notDestroyed?.[emailId]; + if (notDestroyed) { + throw new Error( + notDestroyed.description || notDestroyed.type || + "Copied email but failed to remove the original from the source folder", + ); + } return id; } From b48b6e0871e7e0ef3445d4b7039430e0b040c0d0 Mon Sep 17 00:00:00 2001 From: Stefan Hildebrandt <695494+hildebrandttk@users.noreply.github.com> Date: Tue, 21 Jul 2026 21:02:40 +0200 Subject: [PATCH 3/3] fix(email): defer source removal on cross-account move to Stalwart The explicit Email/set destroy workaround for the duplicate-on-move bug is removed now that the root cause is filed upstream (support.stalw.art #1150: onSuccessDestroyOriginal destroys the copy's create-id instead of the source id). copyEmailAcrossAccounts keeps requesting onSuccessDestroyOriginal, so the move self-heals once Stalwart ships the fix. Kept: the keyword-preservation fix (carry the source keywords into Email/copy) so the moved message keeps its read state. Tests: 08-shared-moves still asserts delivery + read-state on every cross-account case; the source-removal checks are re-pinned test.fail, scoped to a nested describe, until #1150 is fixed. Suite green (5 pass, 3 expected-fail). --- integration/tests/08-shared-moves.spec.ts | 126 ++++++++++++---------- lib/jmap/client.ts | 18 +--- 2 files changed, 72 insertions(+), 72 deletions(-) diff --git a/integration/tests/08-shared-moves.spec.ts b/integration/tests/08-shared-moves.spec.ts index 32901562..258e1084 100644 --- a/integration/tests/08-shared-moves.spec.ts +++ b/integration/tests/08-shared-moves.spec.ts @@ -15,12 +15,18 @@ import { * Moving mail across the own-account / shared-folder boundary, in both * directions, and between two shared folders (same owner and across owners). * The move is driven from the list context menu's "Move to" submenu; the - * authoritative check is the server-side mailbox the message ends up in. Each - * cross-account case also asserts the source copy is gone (no duplicate) and - * the read state survives the move (Email/copy drops keywords unless carried). + * authoritative check is the server-side mailbox the message ends up in. + * + * Each cross-account case asserts delivery *and* that the read state survives + * (Email/copy drops keywords unless carried). Removing the source, however, is + * currently blocked by a Stalwart bug — onSuccessDestroyOriginal destroys the + * copy's create-id instead of the source id, so the original is left behind + * (support.stalw.art #1150). Those source-removal checks are pinned test.fail + * until Stalwart ships the fix; same-account moves (Email/set) are unaffected. */ const { alice, bob, carol } = ACCOUNTS; const subj = (l: string) => `IT ${l} ${Date.now()}`; +type FolderSel = Parameters[1]; test.describe('Shared-folder moves', () => { let ja: JmapClient; // owner A @@ -54,18 +60,28 @@ test.describe('Shared-folder moves', () => { const seenOf = (m: any) => Boolean(m?.keywords?.$seen); + // Log in as carol, reveal the relevant shared owners, and move `subject` from + // `source` to `dest` via the context menu. + async function uiMove( + page: import('@playwright/test').Page, + opts: { subject: string; owners: string[]; source: FolderSel; dest: FolderSel }, + ): Promise { + await login(page, carol); + for (const o of opts.owners) await expandSharedFolders(page, o); + const destId = await folderMailboxId(page, opts.dest); + await openFolder(page, opts.source); + await forceSync(page); + await moveEmailTo(page, opts.subject, destId); + await page.waitForTimeout(2000); + } + + const inbox: FolderSel = { role: 'inbox', shared: false }; + const shared = (name: string): FolderSel => ({ name, shared: true }); + test('shared folder A -> shared folder B (same owner)', async ({ page }) => { const s = subj('mv-a2b'); await seedRead(teamA, s); - - await login(page, carol); - await expandSharedFolders(page, alice.email); - const dest = await folderMailboxId(page, { name: 'TeamB', shared: true }); - await openFolder(page, { name: 'TeamA', shared: true }); - await forceSync(page); - - await moveEmailTo(page, s, dest); - await page.waitForTimeout(1500); + await uiMove(page, { subject: s, owners: [alice.email], source: shared('TeamA'), dest: shared('TeamB') }); const inB = await ja.findEmailBySubject(s, teamB); expect(inB, 'message in TeamB').toBeTruthy(); @@ -76,15 +92,7 @@ test.describe('Shared-folder moves', () => { test('shared folder B -> shared folder A (same owner)', async ({ page }) => { const s = subj('mv-b2a'); await seedRead(teamB, s); - - await login(page, carol); - await expandSharedFolders(page, alice.email); - const dest = await folderMailboxId(page, { name: 'TeamA', shared: true }); - await openFolder(page, { name: 'TeamB', shared: true }); - await forceSync(page); - - await moveEmailTo(page, s, dest); - await page.waitForTimeout(1500); + await uiMove(page, { subject: s, owners: [alice.email], source: shared('TeamB'), dest: shared('TeamA') }); const inA = await ja.findEmailBySubject(s, teamA); expect(inA, 'message in TeamA').toBeTruthy(); @@ -92,67 +100,67 @@ test.describe('Shared-folder moves', () => { expect(seenOf(inA), 'read state kept').toBe(true); }); - // Cross-owner shared → shared: source is alice's account, destination is bob's, - // so this exercises the true cross-account Email/copy + destroy path. - test('shared folder (owner A) -> shared folder (owner B)', async ({ page }) => { + // Cross-account cases: delivery + read state must hold (our fix); removing the + // source is pinned test.fail below (Stalwart #1150). + test('cross-owner shared -> shared: delivers and keeps read state', async ({ page }) => { const s = subj('mv-a2c'); await seedRead(teamA, s); - - await login(page, carol); - await expandSharedFolders(page, alice.email); - await expandSharedFolders(page, bob.email); - const dest = await folderMailboxId(page, { name: 'TeamC', shared: true }); - await openFolder(page, { name: 'TeamA', shared: true }); - await forceSync(page); - - await moveEmailTo(page, s, dest); - await page.waitForTimeout(2000); + await uiMove(page, { subject: s, owners: [alice.email, bob.email], source: shared('TeamA'), dest: shared('TeamC') }); const inC = await jb.findEmailBySubject(s, teamC); expect(inC, 'message in bob TeamC').toBeTruthy(); - expect(await ja.findEmailBySubject(s, teamA), 'message left alice TeamA').toBeFalsy(); expect(seenOf(inC), 'read state kept').toBe(true); }); - // The "Move to" submenu relocates a message across the account boundary - // (own ↔ shared folder) via copy+delete, matching drag-and-drop. - test('own account -> shared folder', async ({ page }) => { + test('own account -> shared folder: delivers and keeps read state', async ({ page }) => { const s = subj('mv-own2sh'); await sendMail({ from: carol.email, authPass: carol.password, to: carol.email, subject: s, body: 'x' }); const own = await jc.waitForEmail(s); await jc.setSeen(own.id, true); - - await login(page, carol); - await expandSharedFolders(page, alice.email); - const dest = await folderMailboxId(page, { name: 'TeamA', shared: true }); - await openFolder(page, { role: 'inbox', shared: false }); - await forceSync(page); - - await moveEmailTo(page, s, dest); - await page.waitForTimeout(2000); + await uiMove(page, { subject: s, owners: [alice.email], source: inbox, dest: shared('TeamA') }); const inTeam = await ja.findEmailBySubject(s, teamA); expect(inTeam, 'message in shared TeamA').toBeTruthy(); - expect(await jc.findEmailBySubject(s), 'message left own account').toBeFalsy(); expect(seenOf(inTeam), 'read state kept').toBe(true); }); - test('shared folder -> own account', async ({ page }) => { + test('shared folder -> own account: delivers and keeps read state', async ({ page }) => { const s = subj('mv-sh2own'); await seedRead(teamA, s); - - await login(page, carol); - await expandSharedFolders(page, alice.email); - const dest = await folderMailboxId(page, { role: 'inbox', shared: false }); - await openFolder(page, { name: 'TeamA', shared: true }); - await forceSync(page); - - await moveEmailTo(page, s, dest); - await page.waitForTimeout(2000); + await uiMove(page, { subject: s, owners: [alice.email], source: shared('TeamA'), dest: inbox }); const inOwn = await jc.findEmailBySubject(s); expect(inOwn, 'message in own account').toBeTruthy(); - expect(await ja.findEmailBySubject(s, teamA), 'message left shared TeamA').toBeFalsy(); expect(seenOf(inOwn), 'read state kept').toBe(true); }); + + // Pinned failing: Stalwart's onSuccessDestroyOriginal leaves the original in + // place on a cross-account copy (support.stalw.art #1150). Un-pin once fixed + // upstream (our copyEmailAcrossAccounts already requests the destroy). + test.describe('source is removed after a cross-account move', () => { + test.fail(true, 'blocked by Stalwart #1150 (onSuccessDestroyOriginal destroys wrong id)'); + + test('cross-owner shared -> shared', async ({ page }) => { + const s = subj('rm-a2c'); + await seedRead(teamA, s); + await uiMove(page, { subject: s, owners: [alice.email, bob.email], source: shared('TeamA'), dest: shared('TeamC') }); + expect(await ja.findEmailBySubject(s, teamA), 'original left alice TeamA').toBeFalsy(); + }); + + test('own account -> shared folder', async ({ page }) => { + const s = subj('rm-own2sh'); + await sendMail({ from: carol.email, authPass: carol.password, to: carol.email, subject: s, body: 'x' }); + const own = await jc.waitForEmail(s); + await jc.setSeen(own.id, true); + await uiMove(page, { subject: s, owners: [alice.email], source: inbox, dest: shared('TeamA') }); + expect(await jc.findEmailBySubject(s), 'original left own account').toBeFalsy(); + }); + + test('shared folder -> own account', async ({ page }) => { + const s = subj('rm-sh2own'); + await seedRead(teamA, s); + await uiMove(page, { subject: s, owners: [alice.email], source: shared('TeamA'), dest: inbox }); + expect(await ja.findEmailBySubject(s, teamA), 'original left shared TeamA').toBeFalsy(); + }); + }); }); diff --git a/lib/jmap/client.ts b/lib/jmap/client.ts index ca18ff54..771eaed2 100644 --- a/lib/jmap/client.ts +++ b/lib/jmap/client.ts @@ -6407,11 +6407,16 @@ export class JMAPClient implements IJMAPClient { ]); const keywords = srcResp.methodResponses?.[0]?.[1]?.list?.[0]?.keywords ?? {}; + // onSuccessDestroyOriginal is the spec-correct way to remove the source, but + // Stalwart currently destroys the copy's create-id instead of the source id, + // so the original is left behind — a duplicate on every cross-account move. + // Reported upstream (support.stalw.art #1150); this self-heals once fixed. const response = await this.request([ ["Email/copy", { fromAccountId, accountId: toAccountId, create: { c: { id: emailId, mailboxIds: { [destMailboxId]: true }, keywords } }, + onSuccessDestroyOriginal: true, }, "0"], ]); const res = response.methodResponses?.[0]?.[1]; @@ -6423,19 +6428,6 @@ export class JMAPClient implements IJMAPClient { if (!id) { throw new Error("Email/copy succeeded but no ID returned"); } - - // onSuccessDestroyOriginal is unreliable on Stalwart (implicit destroy reports - // notFound and leaves the original behind), so remove the source explicitly. - const delResp = await this.request([ - ["Email/set", { accountId: fromAccountId, destroy: [emailId] }, "0"], - ]); - const notDestroyed = delResp.methodResponses?.[0]?.[1]?.notDestroyed?.[emailId]; - if (notDestroyed) { - throw new Error( - notDestroyed.description || notDestroyed.type || - "Copied email but failed to remove the original from the source folder", - ); - } return id; }