Skip to content

fix(mail): skip self-echoed Gmail reconcile sends and drafts (HT-50) #53

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
zaridan merged 3 commits into from
Jul 17, 2026
Merged

fix(mail): skip self-echoed Gmail reconcile sends and drafts (HT-50) #53

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c258d7d
fix(mail): skip self-echoed sends in Gmail reconcile (HT-50)
zaridan Jul 17, 2026
62cd76a
fix(mail): close HT-50 review findings — DRAFT skip, labelsAdded hard…
zaridan Jul 17, 2026
81802b6
fix(mail): merge labelsAdded top-level labelIds delta, not message sn…
zaridan Jul 17, 2026
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
74 changes: 64 additions & 10 deletions specs/mail/gmail-push.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,25 +98,74 @@ Then, from the resolved mailbox and its stored cursor:
- `users.history.list?startHistoryId=<stored cursor>` — enumerate `messagesAdded` since
**our stored cursor**, not the notification's `historyId` (which is the *new* watermark:
starting from it returns nothing, because there are no changes newer than the current
state — the stored cursor is the source of truth). Page through all results.
- For each new message id: `users.messages.get?format=raw` → the raw RFC822 bytes. `raw` is
mandatory; a parsed/`full` fetch would reintroduce the second-parser problem
(inbound-ingestion.md §1).
state — the stored cursor is the source of truth). Page through all results. Each
`messagesAdded` entry carries the message's `labelIds` alongside its id — see "The
self-echo filter" below for what that is for.
- **Skip the mailbox's own outbound sends before fetching them (HT-50; "the self-echo
filter", below).**
- For each remaining new message id: `users.messages.get?format=raw` → the raw RFC822
bytes. `raw` is mandatory; a parsed/`full` fetch would reintroduce the second-parser
problem (inbound-ingestion.md §1).
- Hand each message to the ingest pipeline as `{ raw, mailboxId, providerMessageId =
<Gmail message id>, receivedAt }` (inbound-ingestion.md §2–3). **The transport never
parses — and therefore never extracts attachments.** Parsing the MIME and writing
attachments to the `BlobStore` is the pipeline's job (inbound-ingestion.md §2–3),
downstream of the single `parseInboundEmail` call; the transport only moves raw bytes.

### The self-echo filter (HT-50)

**Live-proven failure (2026-07-17, first HT-44 live run):** `history.list` does not
distinguish "a message that arrived" from "a message this mailbox just sent" — Gmail
surfaces the mailbox's own outbound sends as `messagesAdded` entries exactly like a
genuine inbound message. Reconcile ingested the desk's own just-sent reply as a brand-new
`from help@resonantiq.app` conversation; every Agent reply was spawning a ghost
conversation. This is a transport-level gap, not a mail-semantics one — it belongs here,
not in inbound-ingestion.md's own (separate) loop-suppression rule (§5 there), which
guards a different case (a verifiable Message-ID/reply-token correlation showing OUR mail
bounced or was auto-answered) and runs downstream, inside the pipeline.

**Fix:** before `messages.get`/ingest, skip a `messagesAdded` entry whose `labelIds`
contain `SENT` and do **not** contain `INBOX`, **or** whose `labelIds` contain `DRAFT`.
A message with both `SENT` and `INBOX` — the self-addressed edge case, an Agent emailing
the shared mailbox itself — is **not** skipped: Gmail gives no other signal to tell that
case apart from a genuine customer message at the transport layer, and getting this wrong
in the drop direction would silently lose a real message forever (invariant #1). The
`DRAFT` check is safe in that same drop direction with no such ambiguity: genuine inbound
mail can never carry the system `DRAFT` label, so it exists purely to stop an Agent's
in-progress Gmail-UI compose or reply — which Gmail autosaves as a new `DRAFT`-labeled
message id on every keystroke pause, each one surfacing in `history.list` before the
Agent ever sends anything — from being ingested as a half-written "customer" message or
spawning a ghost conversation. The skip happens **before** any `inbound_deliveries` ledger
row is created, and does **not** disturb the cursor: a skipped message is treated exactly
like the existing "deleted between list and get" 404 case (a message that contributes no
outcome to the batch), so §4's cursor-advance rule proceeds unaffected and nothing is ever
leased or left `in-progress` on a skipped message's behalf.

**Alternative considered and rejected:** track the Gmail message id `users.messages.send`
returns and skip exactly those ids on reconcile. More precise for sends issued through
Helpthread's own send path, but it misses an Agent replying **directly from the Gmail web
UI** — that reply carries no id this engine ever minted, so an id-tracking filter would
let it through and produce the identical ghost conversation. `SENT`-without-`INBOX`
catches both origins, because Gmail applies the `SENT` label identically regardless of
which client sent the mail.

## 4. The cursor: monotonic, transactional with persistence

Each mailbox stores a `historyId` cursor (HT-36). Its one rule: **it advances only after
the ingest pipeline confirms every message in the batch is `stored` or `suppressed`**
(inbound-ingestion.md §4). A crash mid-batch leaves the cursor where it was; the next
notification (or the §6 reconciliation sweep) re-lists from there, re-fetches, and the
pipeline dedups on `(mailboxId, providerMessageId)`. Advancing the cursor *before*
persistence would silently drop any message that failed to store — the one outcome
invariant #1 forbids — so we always bias to re-fetch, never to skip.
the ingest pipeline confirms every message HANDED TO IT is `stored` or `suppressed`**
(inbound-ingestion.md §4) — "the batch" here is scoped to messages the pipeline actually
received, not every `messagesAdded` entry `history.list` returned. Two kinds of entry are
filtered out before ever reaching the pipeline and so contribute no outcome for this rule
to inspect: a message deleted between `history.list` and the raw fetch (§3, "deleted
between list and get"), and a self-echo/draft skipped by §3's self-echo filter. Neither is
a gap in this rule — both are terminal, ledger-free non-events decided entirely on
transport metadata before `ingest` is ever called, so there is nothing for them to be
`stored`/`suppressed` INTO. A crash mid-batch (of messages that WERE handed to the
pipeline) leaves the cursor where it was; the next notification (or the §6 reconciliation
sweep) re-lists from there, re-fetches, and the pipeline dedups on `(mailboxId,
providerMessageId)`. Advancing the cursor *before* persistence would silently drop any
message that failed to store — the one outcome invariant #1 forbids — so we always bias
to re-fetch, never to skip.

## 5. Expired history cursor — the dangerous case, and a dogfood decision

Expand Down Expand Up @@ -255,6 +304,11 @@ Against a **faked** Gmail API + Pub/Sub push (no cloud):
- A duplicate push (same `historyId`) → no duplicate ingestion (dedup, inbound-ingestion.md §4).
- A mid-batch failure → the cursor does not advance past the unstored message.
- A 404 on `history.list` → the mailbox is paused and flagged, no resync attempted.
- A `messagesAdded` entry labeled `SENT` without `INBOX` (the mailbox's own outbound send,
or an Agent's direct Gmail-UI reply) → skipped before `messages.get`/ingest, no
`inbound_deliveries` row created, cursor still advances past it (HT-50, "the self-echo
filter" above). A self-addressed entry labeled both `SENT` and `INBOX` → still ingested
normally.
Comment on lines +307 to +311

@coderabbitai coderabbitai Bot Jul 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Add a DRAFT case to the acceptance criteria.

The filter also skips DRAFT messages, but this release-gate section only verifies SENT without INBOX. Add an acceptance case confirming that drafts avoid messages.get/ingest while the cursor still advances.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@specs/mail/gmail-push.md` around lines 307 - 311, Add an acceptance criterion
near the existing self-echo filter case documenting that a `messagesAdded` entry
labeled `DRAFT` is skipped before `messages.get`/ingest, creates no
`inbound_deliveries` row, and still advances the cursor past it.

- The daily reconciliation sweep re-lists from the stored cursor and ingests a message a
*dropped* push never delivered — with no duplication of messages push already delivered.

Expand Down
Loading
Loading