fix: flip $junk/$notjunk keywords on spam and not-spam #850

integration/tests/06-actions.spec.ts

@@ -92,6 +92,9 @@ test.describe('Inbox message actions', () => {
     const junk = await jmap.mailboxByRole('junk');
     const found = await jmap.findEmailBySubject(s, junk!.id);
     expect(found, 'spammed message is in Junk on the server').toBeTruthy();
+    // Filed *and* flagged: other JMAP clients read the keyword, not the folder.
+    expect(found.keywords?.$junk, 'spammed message carries $junk').toBe(true);
+    expect(found.keywords?.$notjunk, 'spammed message has no $notjunk').toBeFalsy();
   });
 
   test('spam then not-spam round-trips the message back out of Junk', async ({ page }) => {
@@ -103,6 +106,13 @@ test.describe('Inbox message actions', () => {
     await emailContextAction(page, s, 'ctx-spam');
     await expectFolderCountsSynced(page, { role: 'junk' }, { total: 1 });
 
+    // Precondition for the keyword assertions below: the message is flagged
+    // $junk before "not spam" runs, so a stale flag would be observable.
+    const junk = await jmap.mailboxByRole('junk');
+    const inbox = await jmap.mailboxByRole('inbox');
+    const spammed = await jmap.findEmailBySubject(s, junk!.id);
+    expect(spammed?.keywords?.$junk, 'message flagged $junk before not-spam').toBe(true);
+
     // Open Junk, then mark not-spam.
     await openFolder(page, { role: 'junk' });
     await expectEmailVisible(page, s);
@@ -113,9 +123,13 @@ test.describe('Inbox message actions', () => {
     // authoritative server state rather than the Junk badge, whose reconcile
     // can stall under heavy concurrent load.)
     await expect(emailItem(page, s)).toHaveCount(0);
-    const junk = await jmap.mailboxByRole('junk');
-    const inbox = await jmap.mailboxByRole('inbox');
     expect(await jmap.findEmailBySubject(s, junk!.id), 'message no longer in Junk').toBeFalsy();
-    expect(await jmap.findEmailBySubject(s, inbox!.id), 'message back in Inbox').toBeTruthy();
+    const restored = await jmap.findEmailBySubject(s, inbox!.id);
+    expect(restored, 'message back in Inbox').toBeTruthy();
+
+    // The move alone is not enough: a message sitting in the Inbox with $junk
+    // still set is junk to every other JMAP client. (#850)
+    expect(restored.keywords?.$junk, 'restored message no longer carries $junk').toBeFalsy();
+    expect(restored.keywords?.$notjunk, 'restored message carries $notjunk').toBe(true);
   });
 });

lib/__tests__/jmap-keyword-account.test.ts

@@ -134,3 +134,38 @@ describe('Email/set clears keywords with null, not false', () => {
     expect(patchFor(captured[0], 'email-x')['keywords/$flagged']).toBe(true);
   });
 });
+
+// "Not spam" used to patch only mailboxIds, so a message restored to the Inbox
+// kept its $junk keyword and stayed flagged as junk for every other JMAP client.
+// Both directions now move the message and flip $junk/$notjunk in one patch.
+describe('spam / not-spam patch keywords alongside the move (#850)', () => {
+  beforeEach(() => vi.restoreAllMocks());
+  afterEach(() => vi.restoreAllMocks());
+
+  it('undoSpam clears $junk (null) and sets $notjunk with the move', async () => {
+    const client = createClient();
+    const { captured } = mockEmailSet();
+    await client.undoSpam('email-x', 'inbox-id', 'shared-account');
+    expect(captured[0][0]).toBe('Email/set');
+    expect(captured[0][1].accountId).toBe('shared-account');
+    const patch = patchFor(captured[0], 'email-x');
+    expect(patch.mailboxIds).toEqual({ 'inbox-id': true });
+    expect(patch['keywords/$junk']).toBeNull();
+    expect(patch['keywords/$junk']).not.toBe(false);
+    expect(patch['keywords/$notjunk']).toBe(true);
+  });
+
+  it('markAsSpam sets $junk and clears $notjunk (null) with the move', async () => {
+    const client = createClient();
+    vi.spyOn(client, 'getMailboxes').mockResolvedValue([
+      { id: 'junk-id', role: 'junk', name: 'Junk', isShared: false } as never,
+    ]);
+    const { captured } = mockEmailSet();
+    await client.markAsSpam('email-x');
+    const patch = patchFor(captured[0], 'email-x');
+    expect(patch.mailboxIds).toEqual({ 'junk-id': true });
+    expect(patch['keywords/$junk']).toBe(true);
+    expect(patch['keywords/$notjunk']).toBeNull();
+    expect(patch['keywords/$seen']).toBeUndefined();
+  });
+});

lib/demo/demo-client.ts

@@ -505,6 +505,8 @@ export class DemoJMAPClient implements IJMAPClient {
     const junkMb = this.data.mailboxes.find(m => m.role === 'junk');
     if (email && junkMb) {
       email.mailboxIds = { [junkMb.id]: true };
+      email.keywords.$junk = true;
+      delete email.keywords.$notjunk;
       if (markAsRead) email.keywords.$seen = true;
     }
     this.recalcMailboxCounts();
@@ -512,7 +514,11 @@ export class DemoJMAPClient implements IJMAPClient {
 
   async undoSpam(emailId: string, originalMailboxId: string): Promise<void> {
     const email = this.data.emails.find(e => e.id === emailId);
-    if (email) email.mailboxIds = { [originalMailboxId]: true };
+    if (email) {
+      email.mailboxIds = { [originalMailboxId]: true };
+      delete email.keywords.$junk;
+      email.keywords.$notjunk = true;
+    }
     this.recalcMailboxCounts();
   }
 

lib/jmap/client.ts

@@ -2436,7 +2436,13 @@ export class JMAPClient implements IJMAPClient {
       ? junkMailbox.originalId
       : junkMailbox.id;
 
-    const patch: Record<string, unknown> = { mailboxIds: { [mailboxId]: true } };
+    // Move + flag in one patch so other JMAP clients (and the server's spam
+    // classifier) see the message as junk, not just filed in Junk. (#850)
+    const patch: Record<string, unknown> = {
+      mailboxIds: { [mailboxId]: true },
+      "keywords/$junk": true,
+      "keywords/$notjunk": null,
+    };
     if (markAsRead) patch["keywords/$seen"] = true;
 
     await this.request([
@@ -2456,6 +2462,10 @@ export class JMAPClient implements IJMAPClient {
         update: {
           [emailId]: {
             mailboxIds: { [originalMailboxId]: true },
+            // Moving alone leaves $junk set, so the message shows up in the
+            // Inbox while still flagged as junk for every other client. (#850)
+            "keywords/$junk": null,
+            "keywords/$notjunk": true,
           },
         },
       }, "0"],