From af5fac6baa5a41ed03a063f9546e9e9cdc3dc738 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Tue, 25 Aug 2026 19:36:52 +0200 Subject: [PATCH] fix: flip $junk/$notjunk keywords on spam and not-spam #850 --- integration/tests/06-actions.spec.ts | 20 +++++++++++-- lib/__tests__/jmap-keyword-account.test.ts | 35 ++++++++++++++++++++++ lib/demo/demo-client.ts | 8 ++++- lib/jmap/client.ts | 12 +++++++- 4 files changed, 70 insertions(+), 5 deletions(-) diff --git a/integration/tests/06-actions.spec.ts b/integration/tests/06-actions.spec.ts index 86c7cb2ac..ccaef229e 100644 --- a/integration/tests/06-actions.spec.ts +++ b/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); }); }); diff --git a/lib/__tests__/jmap-keyword-account.test.ts b/lib/__tests__/jmap-keyword-account.test.ts index 0ee4b49b3..cdf4c6956 100644 --- a/lib/__tests__/jmap-keyword-account.test.ts +++ b/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(); + }); +}); diff --git a/lib/demo/demo-client.ts b/lib/demo/demo-client.ts index af1a5c3d9..ee58c70cf 100644 --- a/lib/demo/demo-client.ts +++ b/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 { 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(); } diff --git a/lib/jmap/client.ts b/lib/jmap/client.ts index f5e510aff..58c190b79 100644 --- a/lib/jmap/client.ts +++ b/lib/jmap/client.ts @@ -2436,7 +2436,13 @@ export class JMAPClient implements IJMAPClient { ? junkMailbox.originalId : junkMailbox.id; - const patch: Record = { 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 = { + 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"],