diff --git a/specs/mail/spam-classification.md b/specs/mail/spam-classification.md index 2bdb958..daac4e2 100644 --- a/specs/mail/spam-classification.md +++ b/specs/mail/spam-classification.md @@ -28,7 +28,8 @@ paths behaved differently, and neither behaviour was chosen: race the label application and drop real mail. But the only label check downstream was the self-echo filter. **A message Google had already put in `SPAM` became an ordinary `active` conversation in the inbox.** Google's verdict was computed, delivered to us, - and thrown away. + and thrown away. The same held for `TRASH` — mail the operator's own filter or own hand + had already deleted still opened live support work (§3.1's `TRASH` note). An operator who connects a Gmail mailbox reasonably expects Gmail's spam filtering to still apply. It did not. @@ -62,37 +63,94 @@ transport's own conclusion across the provider boundary. Three states: | Value | Meaning | |---|---| -| `'spam'` | The provider affirmatively classified the message as junk. | +| `'spam'` | The source mailbox has already discarded this message as unwanted — its classifier called it junk, **or** the operator threw it away. | | `'clean'` | The provider classified it and did not call it junk. | | `'unknown'` | No verdict available — the provider does not classify, or omitted it on this delivery. | Omitted entirely by a transport with no concept of a verdict; ingest reads an absent field exactly as `'unknown'`. +**`'spam'` is broader than its name and this is deliberate (§7, D7).** It covers two +different things: a *classifier verdict* ("this is junk") and an *operator action* ("I +deleted this"). They are not the same claim — one is a machine's guess, the other is a +person's decision, and the person's is the stronger of the two. They are collapsed into +one value because the only thing either one is allowed to change is identical: the status a +brand-new conversation is filed under. Nothing downstream branches on which of the two +produced the verdict, so distinguishing them on the wire would be a field nobody reads. + +Two consequences worth stating rather than discovering. §3.2's header scoring must not read +`'spam'` as evidence that a *classifier* fired — it is not necessarily one. And a message +the operator deleted lands in the Spam folder, which is a slight vocabulary stretch, paid +knowingly: it is the folder for mail that is not live work, everything in it is fully +stored and readable, and a reply reopens it to `active` (§4.2) whichever path filed it. + `'clean'` and `'unknown'` produce the same outcome today (§4.1) but are kept distinct on the wire: "we asked and it said no" is evidence, "we have no idea" is not, and §3.2's header signals will need to tell them apart — header scoring should defer to an explicit `'clean'` and should not defer to silence. +**Each transport must earn `'clean'`; no transport inherits it.** The table above defines +`'clean'` as an *affirmative* judgment, and "the provider sent us something and none of it +said spam" is not, on its own, that judgment. The two collapse only where the transport +classifies **every** message it delivers, so that the absence of a junk marker is itself +the verdict. That happens to hold for Gmail (below), which is why its implementation reads +a bare absence of `SPAM` as `'clean'` — but it holds *because of a property of Gmail*, not +because absence generally means clean. A transport that classifies only some of its mail, +or that marks junk only above a confidence threshold, must report `'unknown'` for the +unmarked remainder. Getting this wrong is invisible today, because §4.1 files `'clean'` and +`'unknown'` identically; it becomes load-bearing the moment §3.2's header scoring ships and +starts standing down in the presence of an explicit `'clean'`. + **Per transport:** - **Gmail** (built, `spamVerdictOf` in `src/mail/gmail-reconcile.ts`): the system `SPAM` - label ⇒ `'spam'`; any other non-empty label set ⇒ `'clean'`; an empty/absent `labelIds` - ⇒ `'unknown'`, because the history client documents that Gmail does not guarantee the - field is populated. This runs **after** the self-echo filter, so our own outbound reply - that Gmail happened to file as junk is skipped entirely rather than filed as a spam - conversation. + **or** `TRASH` label ⇒ `'spam'`; any other non-empty label set ⇒ `'clean'`; an + empty/absent `labelIds` ⇒ `'unknown'`, because the history client documents that Gmail + does not guarantee the field is populated. This runs **after** the self-echo filter, so + our own outbound reply that Gmail happened to file as junk — or that the operator + deleted — is skipped entirely rather than filed as a spam conversation. + + **`TRASH` is the operator-action half of the widened `'spam'` above.** A message the + operator's own mailbox threw away reaches us for exactly the reason a `SPAM`-labeled one + does: `history.list` is an unfiltered delta stream, and the client set-unions + `labelsAdded` without ever removing a label. So both `['TRASH']` — a Gmail filter that + deletes on arrival — and `['INBOX','TRASH']` — a manual delete inside the reconcile + window — arrive intact. Without this they fell through to `'clean'` and opened a live + conversation for mail the operator had already discarded. Note this is a *stronger* + signal than the classifier half, not a weaker one: `SPAM` is Google guessing, `TRASH` is + usually the operator deciding. + + Reading a bare absence of `SPAM` as `'clean'` is licensed by the rule above and by one + specific fact: **Gmail classifies every message it accepts**, so a delivered message + carrying labels but not `SPAM` has been assessed and cleared. This is the whole + justification — it is not a default, and the next transport does not get it for free + (§7, D6). **This is best-effort, not complete, and the boundary is worth stating precisely.** `history.list` is a delta stream, not a snapshot. The history client requests `labelAdded` alongside `messageAdded` and set-unions the label deltas (`src/providers/adapters/gmail/history.ts`), so a message that arrives labeled `INBOX` and is classified `SPAM` moments later — **within the same reconcile window** — is - correctly seen as spam. What is *not* covered: a `SPAM` label applied **after** the - window's `history.list` snapshot. That message has already been ingested as `active`, - and the next reconcile deliberately ignores a `labelsAdded` record for an id it did not - itself newly add (otherwise any re-labeling of any old message would manufacture an - ingest). So it stays `active`. + correctly seen as spam. What is *not* covered: a `SPAM` or `TRASH` label applied + **after** the window's `history.list` snapshot. That message has already been ingested + as `active`, and the next reconcile deliberately ignores a `labelsAdded` record for an + id it did not itself newly add (otherwise any re-labeling of any old message would + manufacture an ingest). So it stays `active`. Deleting a message you already saw arrive + in Helpthread therefore does not retroactively file it — that is §5's problem. + + **The mirror case: a label REMOVED inside the window is not seen either (§7, D8).** The + history client requests `messageAdded` and `labelAdded` — deliberately *not* + `labelRemoved`, because unioning deltas fails toward ingesting, which is the safe + direction for invariant #1. The cost is that `labelIds` here is the set of labels a + message has *ever* carried in this window, not its state now. So an operator who deletes + a just-arrived message and immediately undoes it leaves `TRASH` in the union, and the + conversation is filed `spam` despite sitting in their Inbox. The same overapproximation + has always applied to `SPAM` (mark-as-junk, then not-junk), but `TRASH` extends it to a + one-click action Gmail actively offers to undo, so the exposure is materially larger. + Accepted rather than fixed: reading `labelRemoved` would re-open the SENT/INBOX + split-delta race the union was introduced to close, and the misfile is bounded and + self-correcting — the message is fully stored, sits readable in the Spam folder, and any + reply reopens it to `active` (§4.2). Revisit with §5, not at intake. This is not a regression — before this spec, *every* spam message stayed `active` — but it means the feature must not be described as "Gmail's spam filtering now applies." @@ -247,6 +305,10 @@ source. Anything marked **INFERRED** is the author's judgment and has not been a | D3 | An Agent's "not spam" correction is **never** written back to the operator's Gmail | Maintainer, 2026-08-02: "no writeback" | | D4 | Header scoring requires one strong or two weak signals; a lone weak signal never classifies | ⚠️ INFERRED — a conservative default, not a measured threshold | | D5 | `Auto-Submitted` is a loop-guard concern, not a spam signal | ⚠️ INFERRED | +| D6 | Gmail's "labels present, none of them `SPAM`" keeps reporting `'clean'` rather than `'unknown'`; §3.1 states the justification instead of the code changing | Maintainer, 2026-08-02: chose "Keep code, tighten §3.1" over narrowing the code, on the question of whether `'clean'` is honest for a message with no `SPAM` label | +| D7 | A `TRASH`-labeled Gmail message is filed as `spam`, sharing one verdict value with classifier-flagged junk rather than getting a separate signal and status | Maintainer, 2026-08-02: chose "file as `spam`" over a separate provider signal filed as `closed`, and over leaving it `active`, once told the writeback hazard behind the original recommendation was void per D3 | +| D7a | A message carrying **both** `SPAM` and `TRASH` is filed as `spam` | Follows mechanically from D7 — the two map to one value, so the combination needs no rule of its own | +| D8 | A message deleted and then restored inside one reconcile window is filed `spam` anyway; the label union is not corrected by reading `labelRemoved` | ⚠️ INFERRED — surfaced by adversarial review of D7. Accepted because the alternative re-opens the split-delta race the union closed, and the misfile is stored, visible and reply-reversible. Not put to the maintainer as its own question | **One-way door:** none, in what is built or specified. The only candidate was writing classification state back into the operator's own mailbox, and D3 closed that door rather diff --git a/src/mail/gmail-reconcile.test.ts b/src/mail/gmail-reconcile.test.ts index 4e2be63..18e754c 100644 --- a/src/mail/gmail-reconcile.test.ts +++ b/src/mail/gmail-reconcile.test.ts @@ -767,6 +767,116 @@ describe('createGmailReconcileHandler', () => { expect(getRawMessage).not.toHaveBeenCalled() expect(setCalls).toEqual([{ mailboxId: MAILBOX_ID, historyId: 'cursor-2' }]) }) + + // --- TRASH: the operator-action half of the verdict (§3.1, §7 D7) ------ + // TRASH is not a classifier verdict — it is the operator's own mailbox + // having discarded the message, by a delete-on-arrival filter or by + // hand. It reaches this handler for the same reason SPAM does (the + // delta stream is unfiltered and the client never REMOVES a label), and + // before D7 it fell through to 'clean' and opened live support work for + // mail the operator had already thrown away. + + it("a TRASH-labeled message — a filter that deleted on arrival — carries providerSpamVerdict: 'spam'", async () => { + const { store: watchStateStore, setCalls } = fakeWatchStateStore({ [MAILBOX_ID]: 'cursor-1' }) + const historyClient = fakeHistoryClient({ + listResult: { + kind: 'ok', + messages: taggedMsgs([['trashed-1', ['TRASH']]]), + newHistoryId: 'cursor-2', + }, + rawMessages: { 'trashed-1': { rawBytes: textBytes('raw'), receivedAt: new Date() } }, + }) + const ingest = vi.fn(async (raw: RawInboundMessage) => storedOutcome(raw)) + + const handler = createGmailReconcileHandler( + baseDeps({ watchStateStore, ingest, createHistoryClient: () => historyClient }), + ) + + const result = await handler(job()) + + expect(result).toEqual({ kind: 'ack' }) + // Still never dropped (inbound-ingestion.md §1 invariant #3) — it is + // filed, not discarded, and a reply reopens it (§4.2). + expect(ingest).toHaveBeenCalledTimes(1) + const raw = ingest.mock.calls[0][0] as RawInboundMessage + expect(raw.providerMessageId).toBe('trashed-1') + expect(raw.providerSpamVerdict).toBe('spam') + expect(setCalls).toEqual([{ mailboxId: MAILBOX_ID, historyId: 'cursor-2' }]) + }) + + // The manual-delete shape. The history client set-unions labelsAdded + // and never removes a label, so a message that arrived in INBOX and was + // deleted inside the same reconcile window reaches us carrying BOTH. + // INBOX must not out-vote TRASH — this is the case that regressed to + // 'clean' before D7. + it("INBOX + TRASH — deleted by hand inside the window — is 'spam', not 'clean'", async () => { + const { store: watchStateStore } = fakeWatchStateStore({ [MAILBOX_ID]: 'cursor-1' }) + const historyClient = fakeHistoryClient({ + listResult: { + kind: 'ok', + messages: taggedMsgs([['trashed-2', ['INBOX', 'TRASH']]]), + newHistoryId: 'cursor-2', + }, + rawMessages: { 'trashed-2': { rawBytes: textBytes('raw'), receivedAt: new Date() } }, + }) + const ingest = vi.fn(async (raw: RawInboundMessage) => storedOutcome(raw)) + + const handler = createGmailReconcileHandler( + baseDeps({ watchStateStore, ingest, createHistoryClient: () => historyClient }), + ) + + await handler(job()) + + expect((ingest.mock.calls[0][0] as RawInboundMessage).providerSpamVerdict).toBe('spam') + }) + + // §7 D7a: both labels map to the one value, so the combination needs no + // rule of its own. Asserted so that stays true if either branch moves. + it("SPAM + TRASH — junk the operator then deleted — is 'spam'", async () => { + const { store: watchStateStore } = fakeWatchStateStore({ [MAILBOX_ID]: 'cursor-1' }) + const historyClient = fakeHistoryClient({ + listResult: { + kind: 'ok', + messages: taggedMsgs([['trashed-3', ['SPAM', 'TRASH']]]), + newHistoryId: 'cursor-2', + }, + rawMessages: { 'trashed-3': { rawBytes: textBytes('raw'), receivedAt: new Date() } }, + }) + const ingest = vi.fn(async (raw: RawInboundMessage) => storedOutcome(raw)) + + const handler = createGmailReconcileHandler( + baseDeps({ watchStateStore, ingest, createHistoryClient: () => historyClient }), + ) + + await handler(job()) + + expect((ingest.mock.calls[0][0] as RawInboundMessage).providerSpamVerdict).toBe('spam') + }) + + it('a TRASH-labeled message that is ALSO a self-echo is still skipped — our own deleted reply is not a spam conversation', async () => { + const { store: watchStateStore, setCalls } = fakeWatchStateStore({ [MAILBOX_ID]: 'cursor-1' }) + const getRawMessage = vi.fn(async () => null) + const historyClient: GmailHistoryClient = { + listAddedMessageIds: async () => ({ + kind: 'ok', + messages: taggedMsgs([['echo-trashed-1', ['SENT', 'TRASH']]]), + newHistoryId: 'cursor-2', + }), + getRawMessage, + } + const ingest = vi.fn(async (raw: RawInboundMessage) => storedOutcome(raw)) + + const handler = createGmailReconcileHandler( + baseDeps({ watchStateStore, ingest, createHistoryClient: () => historyClient }), + ) + + const result = await handler(job()) + + expect(result).toEqual({ kind: 'ack' }) + expect(ingest).not.toHaveBeenCalled() + expect(getRawMessage).not.toHaveBeenCalled() + expect(setCalls).toEqual([{ mailboxId: MAILBOX_ID, historyId: 'cursor-2' }]) + }) }) it('a 404-expired cursor pauses the mailbox, does not advance the cursor, and acks', async () => { diff --git a/src/mail/gmail-reconcile.ts b/src/mail/gmail-reconcile.ts index ae22d2a..414687d 100644 --- a/src/mail/gmail-reconcile.ts +++ b/src/mail/gmail-reconcile.ts @@ -692,10 +692,33 @@ function isSelfEchoMessage(labelIds: string[]): boolean { * `SENT`+`SPAM` self-echo (our own reply that Gmail happened to file as * junk) is never ingested at all rather than being filed as a spam * conversation. + * + * ## `TRASH` counts too, and the field's name undersells why + * + * `TRASH` is NOT a classifier verdict — it is the operator's own mailbox + * having thrown the message away, either by hand or by a filter of theirs + * that deletes on arrival. It reaches us for the same reason `SPAM` does: + * `history.list` is a delta stream over the whole mailbox, and the client + * set-unions `labelsAdded` without ever REMOVING a label, so both + * `['TRASH']` (a filter that deleted at delivery) and `['INBOX','TRASH']` + * (deleted by hand inside the reconcile window) arrive here intact. Before + * this, both fell through to `'clean'` and became live support work in an + * inbox — mail the operator had already discarded. + * + * Mapping it onto `'spam'` deliberately widens what that value asserts: + * not "a classifier called this junk" but "the source mailbox has already + * discarded this as unwanted." specs/mail/spam-classification.md §3.1 + * carries the widened definition, and §7's D7 records that collapsing the + * two into one value was a decision, not an oversight. The properties that + * make it safe are the ones `'spam'` already had: nothing is dropped + * (inbound-ingestion.md §1), the message is fully parsed and stored, an + * Agent can see it in the Spam folder, and a reply reopens it to `active` + * (agent-inbox-v1.md §4a) — so a message trashed in error self-heals the + * moment the sender writes back. */ function spamVerdictOf(labelIds: string[]): ProviderSpamVerdict { if (labelIds.length === 0) return 'unknown' - return labelIds.includes('SPAM') ? 'spam' : 'clean' + return labelIds.includes('SPAM') || labelIds.includes('TRASH') ? 'spam' : 'clean' } /**