feat(inbound): Gmail history sync + raw message fetch (HT-41) - #40
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (12)
🚧 Files skipped from review as they are similar to previous changes (9)
📝 WalkthroughWalkthroughChangesAdds Gmail history and raw-message retrieval, mailbox watch-state persistence, and a queue reconciliation handler. The flow handles pagination, cursor expiry, deleted messages, token failures, retries, terminal ingest outcomes, oversized payload storage, and mailbox lifecycle transitions. Gmail reconciliation flow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Queue
participant ReconcileHandler
participant MailboxStore
participant GmailHistoryClient
participant Ingest
participant GmailWatchStateStore
Queue->>ReconcileHandler: Deliver GmailReconcileJob
ReconcileHandler->>MailboxStore: getMailboxById
ReconcileHandler->>GmailHistoryClient: listAddedMessageIds
GmailHistoryClient-->>ReconcileHandler: Added IDs and newHistoryId
ReconcileHandler->>GmailHistoryClient: getRawMessage
ReconcileHandler->>Ingest: Ingest message content
ReconcileHandler->>GmailWatchStateStore: setCursor
ReconcileHandler-->>Queue: Return ack or retry
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/mail/gmail-reconcile.test.ts`:
- Around line 634-646: Remove the test named “the default createHistoryClient
wires the real Gmail history client…” from the Gmail reconciliation tests. Do
not add replacement coverage; the existing paused-mailbox short-circuit test
already covers this behavior, and createHistoryClient is required and supplied
by baseDeps().
In `@src/providers/adapters/gmail/history.ts`:
- Around line 249-266: Validate body.raw as well-formed base64url in the
createGmailHistoryClient response handling before calling Buffer.from, rejecting
malformed values rather than allowing truncated RFC822 data; preserve the
existing missing/empty validation and error context. Add a malformed-base64url
fixture and assertion in src/providers/adapters/gmail/history.test.ts:211-225
covering the rejection behavior.
In `@src/store/gmail-watch-state.ts`:
- Around line 56-62: Make setCursor in src/store/gmail-watch-state.ts accept the
expected current cursor and perform a compare-and-set update, returning whether
the persisted cursor still matched and was advanced. In
src/mail/gmail-reconcile.ts, pass the cursor used for listing and treat a failed
CAS as a stale job. In src/store/gmail-watch-state.test.ts, add an
overlapping-worker test confirming a stale write cannot replace the newer
watermark; preserve existing mail semantics and provide fixture-based
equivalence or explicit justification for any affected 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: 3385d8cc-21a9-4394-b92b-b3e0f4745295
📒 Files selected for processing (12)
src/api/gmail-webhook.test.tssrc/api/index.test.tssrc/mail/gmail-reconcile.test.tssrc/mail/gmail-reconcile.tssrc/providers/adapters/gmail/history.test.tssrc/providers/adapters/gmail/history.tssrc/providers/adapters/gmail/index.tssrc/store/gmail-watch-state.test.tssrc/store/gmail-watch-state.tssrc/store/index.tssrc/store/mailboxes.test.tssrc/store/mailboxes.ts
The consumer of the reconcile job HT-39 enqueues: history.list from the mailbox's STORED cursor -> messages.get?format=raw -> the HT-37 ingest pipeline -> transactional cursor advance. Raw bytes only: no MIME parse, no attachment extraction (charter parse-once boundary). A 404-expired cursor pauses the mailbox for manual rebaseline; the cursor advances only when every message is terminally ledgered (stored/suppressed/dead-letter), never past unpersisted mail. - src/providers/adapters/gmail/history.ts history.list + messages.get?format=raw client - src/store/gmail-watch-state.ts per-mailbox history cursor (upsert) - src/mail/gmail-reconcile.ts QueueMessageHandler<GmailReconcileJob> - src/store/mailboxes.ts getMailboxById + markPaused createHistoryClient is injected (no default) so engine core never imports a concrete adapter (src/providers/README.md), matching HT-39's verifySignature. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
51a12ca to
9b27451
Compare
Implements HT-41 [H] — Gmail history sync + raw message fetch, under the HT-33 epic. This is the consumer of the reconcile job HT-39 enqueues: it turns a mailbox's stored cursor into the raw RFC822 bytes of everything added since, and feeds them to the HT-37 ingest pipeline. No new spec — gmail-push.md §3–§5 (written in HT-34) is the contract.
What's here
src/mail/gmail-reconcile.ts—createGmailReconcileHandler: theQueueMessageHandler<GmailReconcileJob>. Re-checks mailbox status → acquires a token → reads the stored cursor (never the notification'shistoryId) →history.list→messages.get?format=raweach id →ingest→ advances the cursor.src/providers/adapters/gmail/history.ts—createGmailHistoryClient:listAddedMessageIds(paginated, id-deduped, 404→{kind:'expired'}) andgetRawMessage(base64url→bytes, 404→null). Mirrorssender.ts(injectablefetch,AbortSignal.timeout, token never logged, throw-on-unexpected-non-2xx).src/store/gmail-watch-state.ts—createGmailWatchStateStore: the per-mailboxhistory_idcursor.setCursorupserts (the baseline row is normally seeded bywatch(), HT-42).src/store/mailboxes.ts— addsgetMailboxById+markPaused(the 404-expired transition).Sacred boundaries (charter §2) — held
messages.get?format=raw→Buffer.from(raw, 'base64url')→ handed off untouched to the pipeline's singleparseInboundEmail. (This is why the Jira ticket's stale "attachments to blob store" line was not implemented — attachments are the pipeline's job, deferred to HT-46; doing them here would force a second parser and break parse-once.)failed/in-progressblocks the advance and the whole batch retries (dedup makes re-fetch free). Worst case is redundant work, never a skipped message.dead-letteradvances the cursor — gmail-push.md §4's prose names onlystored/suppressed, but the ledger also has a terminaldead-letterstate. Blocking the cursor on it would wedge the mailbox forever on one poison message (and starve every healthy message behind it in history order). Sincedead-letteris durably recorded (never-drop holds), the handler treats it as cursor-advancing too. Documented at length ingmail-reconcile.ts's module doc. This extends the spec's literal wording — want a spec footnote, or is the reasoning sound as-is?DEFAULT_MAX_INLINE_RAW_BYTES = 1 MB— not spec'd. Raw messages ≤1 MB go toingestinline; larger ones are written to theBlobStorefirst (mailbox-namespaced keyinbound/raw/{mailboxId}/{messageId}) and handed over as ablobRef— the ticket's "OOM guard," applied to the raw message (not attachments), which is exactly whatRawMessageContent.blobRefwas designed for. Injected/retunable.Review notes (mine)
I read the correctness-critical surface end-to-end rather than trusting the implementer's summary. One real defect found and fixed:
createHistoryClientto the concretecreateGmailHistoryClient, which meant engine core did a runtimeimportof an adapter — againstsrc/providers/README.md's rule and contrary to HT-39's ownverifySignature(required, no default). Fixed:createHistoryClientis now a required injected dependency, the concrete client wired at the composition root (HT-43); only the interface type is imported (type-only, erased at runtime).Verified: cursor never advances past unpersisted mail; 404→pause; token never in a log line or thrown error; base64url decode matches
sender.ts's encode side. Gates:typecheck+biomeclean, 582/582 tests pass locally.Scope
Handler + client + cursor store only. Out:
watch()arm/renewal + the initial baseline cursor (HT-42), the daily reconciliation sweep + per-mailbox lease (HT-42), and the composition-root queue-consumer HTTP wiring (HT-43). Acceptance is against a faked Gmail API per gmail-push.md §8.🤖 Generated with Claude Code
Summary by CodeRabbit