Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 74 additions & 12 deletions specs/mail/spam-classification.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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."
Expand Down Expand Up @@ -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
Expand Down
110 changes: 110 additions & 0 deletions src/mail/gmail-reconcile.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down
25 changes: 24 additions & 1 deletion src/mail/gmail-reconcile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
}

/**
Expand Down