fix(email): preserve read state and remove source on cross-account move

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

@@ -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<void> {
-    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<void> {
+    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);
   });
 });

lib/jmap/client.ts

@@ -6400,12 +6400,18 @@ export class JMAPClient implements IJMAPClient {
     toAccountId: string,
     destMailboxId: string,
   ): Promise<string> {
+    // 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;
   }