fix(mail): skip self-echoed Gmail reconcile sends and drafts (HT-50) - #53
Merged
Conversation
📝 WalkthroughWalkthroughGmail history results now include message labels, allowing reconciliation to skip outbound self-echoes and drafts before fetching or ingesting them. History pagination folds label updates, while reconciliation tests cover filtering, cursor advancement, normal ingestion, and existing lease behavior. ChangesGmail self-echo filtering
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant GmailHistory
participant ReconcileOneMailbox
participant RawMessageClient
participant IngestPipeline
GmailHistory->>ReconcileOneMailbox: return added messages with labelIds
ReconcileOneMailbox->>ReconcileOneMailbox: apply isSelfEchoMessage
ReconcileOneMailbox->>RawMessageClient: fetch non-skipped message
RawMessageClient->>IngestPipeline: provide RawInboundMessage
IngestPipeline-->>ReconcileOneMailbox: return ingest outcome
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with 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.
Inline comments:
In `@src/providers/adapters/gmail/history.ts`:
- Around line 189-194: Update src/providers/adapters/gmail/history.ts:189-194 to
model labelsAdded.labelIds at the top level of each label-added record, then
update the history folding logic at
src/providers/adapters/gmail/history.ts:293-307 to merge those labels into the
already tracked entry rather than reading message.labelIds or overwriting
existing labels. Adjust the related history definitions/documentation at
src/providers/adapters/gmail/history.ts:47-92, add coverage for the top-level
label delta in src/providers/adapters/gmail/history.test.ts:139-220, and update
the reconciliation handling at src/mail/gmail-reconcile.ts:208-220 and behavior
documentation at specs/mail/gmail-push.md:101-103 as needed to reflect the
corrected merge behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: bc815879-a7ac-4df3-9725-f95ac63caafc
📒 Files selected for processing (6)
specs/mail/gmail-push.mdsrc/mail/gmail-reconcile.test.tssrc/mail/gmail-reconcile.tssrc/providers/adapters/gmail/history.test.tssrc/providers/adapters/gmail/history.tssrc/providers/adapters/gmail/index.ts
Live-proven failure (first HT-44 run): history.list surfaces the mailbox's own outbound sends as messagesAdded entries indistinguishable from genuine inbound mail, so reconcile ingested an Agent's just-sent reply as a new ghost conversation. Surface each history record's labelIds through the Gmail history adapter (GmailHistoryClient.listAddedMessageIds now returns AddedGmailMessage[] instead of bare ids) and skip a message in the reconcile handler when it carries SENT without INBOX, before any messages.get/ingest call — so no inbound_deliveries row is ever created and the cursor still advances past it exactly like the existing "deleted between list and get" case. A self-addressed message (both SENT and INBOX) is deliberately still ingested, since Gmail gives no other signal to distinguish it from a real customer message. A message-id-tracking alternative was considered and rejected: it would miss an Agent replying directly from the Gmail web UI, which the label filter catches. specs/mail/gmail-push.md documents the filter and the rejected alternative; inbound-ingestion.md is unchanged since its pipeline contract (parse/thread/ ingest) is untouched — the filter runs entirely on transport metadata before any of that. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ening (HT-50) Review round 2 findings, all addressed: - Skip DRAFT-labeled history entries too (src/mail/gmail-reconcile.ts's isSelfEchoMessage), not just SENT-without-INBOX. Gmail autosaves an Agent's in-progress Gmail-UI reply as a new DRAFT-labeled message id on every pause, each surfacing in history.list before anything is sent; the prior filter left these to ingest as ghost/half-written "customer" messages. Safe in the drop direction with no ambiguity: genuine inbound mail can never carry the system DRAFT label. - Harden src/providers/adapters/gmail/history.ts's listAddedMessageIds against the (unverified-live, but plausible) case where Gmail records a self-addressed send's SENT and INBOX labels as two SEPARATE history records instead of one messagesAdded snapshot carrying both. Now also requests the labelAdded history type and folds a later labelsAdded record's label snapshot into an id already tracked via messagesAdded in the same listed window, so a split SENT-then-INBOX ordering can no longer cause a silent permanent drop (invariant #1). Live verification against the dogfood mailbox remains open and is called out in the module doc as such — this hardening does not depend on nor confirm the assumption, it removes the failure mode either way. - specs/mail/gmail-push.md: document the DRAFT skip; amend §4's cursor rule to scope "the batch" to messages actually handed to the ingest pipeline, closing the contradiction with §3's self-echo filter (which was already described as cursor-neutral but never reconciled against §4's literal wording). - Full `npx vitest run` on this branch: 44 files, 868 tests, all passing (exit 0) — the previously-unattributed 18 failures from review round 1 do not reproduce; no comparison against origin/main was needed since this run itself is clean evidence. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…apshot (HT-50) CodeRabbit round 3: Gmail's HistoryLabelAdded schema carries the added labels in the record's TOP-LEVEL labelIds field, beside `message` — the embedded message's own labelIds is not guaranteed to be populated in history records. The folding logic was reading message.labelIds and OVERWRITING the tracked entry, so an omitted embedded snapshot would clobber a real ['SENT'] down to [] (losing the label the self-echo filter keys on and defeating the very INBOX-visibility hardening the fold exists for). Now merges (set union) the schema-guaranteed top-level delta into the tracked entry and never reads the embedded snapshot: merging can only ADD labels, so a later INBOX becomes visible without ever losing the earlier SENT. Still fails open — an absent delta leaves the snapshot intact, biasing toward ingesting, never toward dropping (invariant #1). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
zaridan
force-pushed
the
fix/ht-50-reconcile-self-echo-skip
branch
from
July 17, 2026 03:45
4fdb658 to
81802b6
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with 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.
Inline comments:
In `@specs/mail/gmail-push.md`:
- Around line 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.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 278e7469-08a6-45f2-8fbc-43a1a3e24521
📒 Files selected for processing (6)
specs/mail/gmail-push.mdsrc/mail/gmail-reconcile.test.tssrc/mail/gmail-reconcile.tssrc/providers/adapters/gmail/history.test.tssrc/providers/adapters/gmail/history.tssrc/providers/adapters/gmail/index.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- src/providers/adapters/gmail/index.ts
- src/providers/adapters/gmail/history.test.ts
- src/mail/gmail-reconcile.ts
- src/providers/adapters/gmail/history.ts
- src/mail/gmail-reconcile.test.ts
Comment on lines
+307
to
+311
| - 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. |
There was a problem hiding this comment.
📐 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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
First live HT-44 run exposed a real failure: Gmail's
history.listsurfaces the mailbox's own outbound sends asmessagesAddedentries indistinguishable from genuine inbound mail, so reconcile ingested an Agent's just-sent reply as a brand-new ghost conversation.This PR fixes that at the transport layer, before any ingest happens:
GmailHistoryClient.listAddedMessageIds(src/providers/adapters/gmail/history.ts) now surfaces each history record'slabelIds(returnsAddedGmailMessage[]instead of bare ids).src/mail/gmail-reconcile.ts) skips a message that carriesSENTwithoutINBOX— before anymessages.get/ingest call — so noinbound_deliveriesrow is ever created, and the cursor still advances past it exactly like the existing "deleted between list and get" case.SENTandINBOX) is deliberately still ingested — Gmail gives no other signal to distinguish it from a real customer message. A message-id-tracking alternative was considered and rejected: it would miss an Agent replying directly from the Gmail web UI, which the label filter catches.DRAFT-labeled message id on every pause, each surfacing inhistory.listbefore anything is sent. The prior filter left these to ingest as ghost/half-written "customer" messages. Fixed by also skippingDRAFT-labeled history entries inisSelfEchoMessage— safe in the drop direction with no ambiguity, since genuine inbound mail can never carry the systemDRAFTlabel.listAddedMessageIdsnow also requests thelabelAddedhistory type and folds a laterlabelsAddedrecord's label snapshot into an id already tracked viamessagesAddedin the same listed window. This closes an (unverified-live, but plausible) failure mode where Gmail records a self-addressed send'sSENTandINBOXlabels as two separate history records instead of onemessagesAddedsnapshot carrying both — which could otherwise cause a silent, permanent drop (invariant HT-3: clean-room protocol doc #1: never lose an inbound message).specs/mail/gmail-push.mddocuments the DRAFT skip, the rejected message-id-tracking alternative, and amends §4's cursor rule to scope "the batch" to messages actually handed to the ingest pipeline — closing a contradiction with §3's self-echo filter (previously described as cursor-neutral but never reconciled against §4's literal wording).inbound-ingestion.mdis unchanged since its pipeline contract (parse/thread/ingest) is untouched — the filter runs entirely on transport metadata before any of that.Design decisions (flagging for sign-off)
SENTandINBOXlabels — so it's treated as inbound. Flagging in case that's not the desired default.labelsAddedfolding) is unverified against a live Gmail mailbox. It defends against a plausible-but-unconfirmed API behavior (SENT and INBOX landing as two separate history records). The module doc calls this out explicitly. It doesn't depend on nor confirm the assumption — it closes the failure mode either way — but the maintainer should know live confirmation is still open.Review — 7 adversarial findings (5 actionable), fixes applied
Two review rounds were run against this change. Round 2 findings (all addressed, see
296b755):isSelfEchoMessageto skipDRAFTalongsideSENT-without-INBOX.listAddedMessageIdswas vulnerable to a split SENT/INBOX history ordering silently dropping a message — hardened by requestinglabelAddedhistory type and folding later label snapshots into already-tracked ids.specs/mail/gmail-push.mdhad an internal contradiction between §3 (self-echo filter framed as cursor-neutral) and §4 (literal wording of "the batch") — reconciled.Verification
Ran for real, in the assigned worktree only (
/Users/tjbaker/Projects/helpthread-worktrees/fix-ht-50-reconcile-self-echo-skip):npm install(fresh, sincenode_moduleswasn't present) — succeeded.npm run typecheck(tsc --noEmit) — exit 0, no errors.npm run lint(biome check .) — one formatting violation (a single-quoted string containing an apostrophe) auto-fixed viabiome check --write; re-run exit 0, no remaining violations.npx vitest run— exit 0: 44 files, 868 tests, all passing.git status --short— clean tree.Independent gate exit codes: typecheck 0, lint 0, tests 0, clean tree.
Jira: https://resonantiq.atlassian.net/browse/HT-50
🤖 Generated with Claude Code
Summary by CodeRabbit