feat(inbound): Gmail watch() renewal cron + reconciliation sweep (HT-42) by zaridan · Pull Request #42 · Helpthread/helpthread

zaridan · 2026-07-14T21:33:02Z

HT-42 — Gmail `watch()` renewal cron + reconciliation sweep

A daily maintenance pass over active Gmail mailboxes. Gmail push watch() silently expires every ~7 days; push is also best-effort (Gmail may drop/delay notifications). This closes both gaps.

Structured like the HT-16 delivery worker: a plain injected-deps function (runGmailWatchMaintenance), not a SchedulerProvider adapter — the Vercel-cron schedule declaration and the queue-consumer wiring are composition-root concerns deferred to HT-43 (no such adapter is wired yet). Per active mailbox, failure-isolated:

Re-arm watch() and store the fresh expiration via GmailWatchStateStore.setWatchExpiration — which updates watch_expiration only, never the cursor. A renewal's fresh historyId is ahead of the stored cursor, so overwriting it would silently skip un-reconciled mail (invariant HT-3: clean-room protocol doc #1). A test asserts history_id is preserved across renewal.
Enqueue a bounded reconciliation sweep — one reconcile job per active mailbox onto HT-41's existing GMAIL_RECONCILE_TOPIC. Enqueued with no dedupeKey, so a daily sweep of a quiet mailbox isn't suppressed as a duplicate; idempotent ingest ((mailboxId, providerMessageId)) makes the redundant work safe.

New surface: MailboxStore.listActiveMailboxes, GmailWatchStateStore.setWatchExpiration, src/mail/gmail-watch-maintenance.ts. No migration.

Scope decision (worth a look)

The Jira ticket scopes HT-42 to the renewal cron and says "NOT on the ingest path"; gmail-push.md §6 bundles renewal + a reconciliation sweep + a per-mailbox reconciliation lease. I consulted a second engineering opinion (Codex) and landed on: renewal + sweep here, defer only the lease (→ new ticket HT-48). Rationale: the sweep is the safety net that makes best-effort push acceptable under invariant #1 — on a quiet mailbox a dropped push isn't "caught by the next push" (there may be none), so mail could sit unseen indefinitely; and the sweep is a small add that reuses HT-41 (no new ingest code, satisfying the ticket's intent). The lease is pure efficiency (§6's own words: "not a correctness one") + needs a migration → HT-48. §6 updated to re-attribute the lease.

Spec refinement I made (judgment call — flagging it)

gmail-push.md §6 said "on watch() failure → mark needs-reconnect." Since HT-38, the token layer owns the dead-grant→needs_reconnect transition (via invalid_grant) and catches every dead grant within the ~1h access-token cache lifetime. So auto-marking needs_reconnect on a generic watch() failure would halt a healthy mailbox on a transient Gmail blip. This PR instead treats a valid-token watch() failure as transient (retry next daily run — the 7-day expiry leaves margin) and lets the token layer own needs_reconnect. I updated §6 to reflect this. The dead-grant outcome is unchanged; only where it's decided moved.

Verified (watched, not claimed)

Sacred: renewal calls setWatchExpiration only (grep-confirmed — never seedBaseline/setCursor); type-only adapter import with injected createWatchClient (adapter-boundary rule); no token in any log or thrown error; failure-isolated per mailbox.
Gates re-run by me: typecheck ✓, lint ✓ (166 files), test ✓ (35 files / 692 tests, +16 new).

Deferred / unchanged boundaries

Schedule declaration + queue-consumer wiring → HT-43; reconciliation lease → HT-48; live end-to-end proof → HT-44. Exercised entirely against fakes (fake watch client, fake queue, in-memory stores) — no cloud.

🤖 Generated with Claude Code

Summary by CodeRabbit

New Features
- Added daily Gmail watch renewal and bounded reconciliation sweep for active mailboxes.
- Preserves the stored history cursor when updating watch expiration to avoid message gaps.
Bug Fixes
- Improved per-mailbox failure handling: token/watch issues are isolated so one mailbox doesn’t block others; reconnect needs are set by the token layer for dead grants.
Tests / Documentation
- Expanded maintenance and state-store test coverage, including new watch-expiration behavior.
- Updated the Gmail inbound push maintenance spec to reflect the refined renewal/reconciliation and failure semantics.

A daily maintenance pass over active Gmail mailboxes (a plain injected-deps function like the HT-16 delivery worker; the SchedulerProvider/Vercel-cron schedule is composition-root wiring, deferred to HT-43). Per active mailbox, failure-isolated: - Re-arm users.watch() (Gmail push silently expires every ~7 days) and store the fresh expiration via GmailWatchStateStore.setWatchExpiration, which updates watch_expiration ONLY and never the cursor — a renewal's fresh historyId is ahead of the stored cursor, so overwriting it would silently skip un-reconciled mail (charter invariant #1). - Enqueue a bounded reconciliation sweep (one reconcile job per active mailbox onto HT-41's GMAIL_RECONCILE_TOPIC) — the safety net for dropped best-effort pushes, so a quiet mailbox whose last push was dropped never sits stale indefinitely. No dedupeKey, so daily sweeps of a quiet mailbox aren't suppressed; idempotent ingest makes the redundant work safe. Failure handling: the OAuth token layer (HT-38) owns needs_reconnect via invalid_grant; a valid-token watch() failure is treated as transient (retried next daily run) rather than halting a healthy mailbox. Re-arm and sweep are independent. One mailbox's failure never stops the batch. New: MailboxStore.listActiveMailboxes, GmailWatchStateStore.setWatchExpiration, src/mail/gmail-watch-maintenance.ts. No migration. Scope: renewal + sweep only, per the ticket's "NOT on the ingest path" (the sweep reuses HT-41, adds no new ingest code). The reconciliation lease from gmail-push.md §6 is deferred to HT-48 (pure efficiency, not correctness); §6 updated to re-attribute it and refine the watch()-failure handling. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

zaridan · 2026-07-14T21:49:25Z

@coderabbitai review

coderabbitai · 2026-07-14T21:49:35Z

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai · 2026-07-14T21:49:43Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: a7f80b1c-60ac-4008-89a5-c1a8dc7fe38f

📥 Commits

Reviewing files that changed from the base of the PR and between b1b0da4 and f04f878.

📒 Files selected for processing (2)

src/mail/gmail-watch-maintenance.test.ts
src/mail/gmail-watch-maintenance.ts

🚧 Files skipped from review as they are similar to previous changes (2)

src/mail/gmail-watch-maintenance.test.ts
src/mail/gmail-watch-maintenance.ts

📝 Walkthrough

Walkthrough

Changes

Gmail watch maintenance

Layer / File(s)	Summary
Mailbox and watch-state persistence contracts `src/store/mailboxes.ts`, `src/store/gmail-watch-state.ts`, `src/store/*.test.ts`	Adds active-mailbox listing and watch-expiration updates that preserve stored history cursors.
Daily renewal and reconciliation orchestration `src/mail/gmail-watch-maintenance.ts`, `src/mail/gmail-watch-maintenance.test.ts`, `specs/mail/gmail-push.md`	Adds per-mailbox watch renewal, token failure classification, cursor-based reconciliation enqueueing, structured logging, and transient watch-failure handling.
Interface-compatible test fixtures `src/api/*.test.ts`, `src/mail/gmail-connect.test.ts`, `src/mail/gmail-reconcile.test.ts`	Extends existing test doubles with the new mailbox-store and watch-state methods.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Maintenance
  participant MailboxStore
  participant OAuth
  participant Gmail
  participant WatchState
  participant Queue
  Maintenance->>MailboxStore: List active mailboxes
  Maintenance->>OAuth: Acquire access token
  Maintenance->>Gmail: Renew watch registration
  Gmail-->>WatchState: Store expiration
  Maintenance->>WatchState: Read history cursor
  Maintenance->>Queue: Enqueue reconciliation job

Possibly related PRs

Helpthread/helpthread#37: Provides the OAuth token-layer needs_reconnect behavior consumed by maintenance.
Helpthread/helpthread#40: Provides the Gmail reconciliation handler for jobs enqueued by maintenance.
Helpthread/helpthread#41: Extends the shared Gmail watch-state persistence contract used by renewal and baseline handling.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: a Gmail watch renewal cron plus reconciliation sweep for HT-42.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch feat/ht-42-watch-renewal-sweep

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

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/mail/gmail-watch-maintenance.ts`:
- Around line 228-260: Reuse the access token obtained during Step 1 when
invoking watch() in the maintenance flow, rather than calling getAccessToken
again through createWatchClient. Update the surrounding logic in the maintenance
function to pass the validated token into the watch client while preserving
existing failure handling, and add a regression test asserting one token-service
call per mailbox.

🪄 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: 1f396720-aba1-46f3-bf9d-6a53ef0a9f83

📥 Commits

Reviewing files that changed from the base of the PR and between 83f8ebb and 05fd4c1.

📒 Files selected for processing (11)

specs/mail/gmail-push.md
src/api/gmail-webhook.test.ts
src/api/index.test.ts
src/mail/gmail-connect.test.ts
src/mail/gmail-reconcile.test.ts
src/mail/gmail-watch-maintenance.test.ts
src/mail/gmail-watch-maintenance.ts
src/store/gmail-watch-state.test.ts
src/store/gmail-watch-state.ts
src/store/mailboxes.test.ts
src/store/mailboxes.ts

…n (HT-42 review) Addresses CodeRabbit on PR #42. Acquire the access token once per mailbox and reuse it for the single watch() call, instead of probing getAccessToken in step 1 and letting the watch client fetch it a second time. Beyond dropping a redundant token-service call, this classifies token-acquisition failures in one place — a watch() failure is now unambiguously a watch-API failure, not a token refresh that raced revocation mid-call and got mislabeled transient. (gmail-reconcile.ts keeps the getAccessToken closure because its multi-page, long-running client must not carry a token that goes stale mid-run; a single watch() call has no such need.) Adds a regression test asserting exactly one token-service call per mailbox. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

zaridan · 2026-07-14T22:01:16Z

Addressed in b1b0da4. Fixed — good catch. Step 1 now acquires the access token once and reuses it for the single watch() call, instead of probing getAccessToken and letting the watch client re-fetch it. Beyond dropping the redundant token-service call, this classifies token-acquisition failures in one place: a watch() failure is now unambiguously a watch-API failure, never a token refresh that raced revocation mid-call and got mislabeled transient. (gmail-reconcile.ts deliberately keeps the getAccessToken closure because its multi-page, long-running client must not carry a token that goes stale mid-run — a single watch() call has no such need.) Added a regression test asserting exactly one token-service call per mailbox. Gates green: typecheck ✓, lint ✓, test ✓ (35 files / 693 tests).

…n (HT-42 review) Addresses CodeRabbit on PR #42. Acquire the access token once per mailbox and reuse it for the single watch() call, instead of probing getAccessToken in step 1 and letting the watch client fetch it a second time. Beyond dropping a redundant token-service call, this classifies token-acquisition failures in one place — a watch() failure is now unambiguously a watch-API failure, not a token refresh that raced revocation mid-call and got mislabeled transient. (gmail-reconcile.ts keeps the getAccessToken closure because its multi-page, long-running client must not carry a token that goes stale mid-run; a single watch() call has no such need.) Adds a regression test asserting exactly one token-service call per mailbox. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

coderabbitai Bot reviewed Jul 14, 2026

View reviewed changes

Comment thread src/mail/gmail-watch-maintenance.ts Outdated

zaridan force-pushed the feat/ht-42-watch-renewal-sweep branch from b1b0da4 to f04f878 Compare July 14, 2026 22:08

zaridan merged commit 946c86d into main Jul 14, 2026
5 checks passed

zaridan deleted the feat/ht-42-watch-renewal-sweep branch July 14, 2026 22:15

coderabbitai Bot mentioned this pull request Jul 21, 2026

feat(mail): drop the Pub/Sub requirement — scheduled fetch as primary intake (HT-94) #107

Merged

coderabbitai Bot mentioned this pull request Aug 5, 2026

docs: cut the remaining module doc-comments for concision #193

Merged

Navigation Menu

Uh oh!

feat(inbound): Gmail watch() renewal cron + reconciliation sweep (HT-42) - #42

feat(inbound): Gmail watch() renewal cron + reconciliation sweep (HT-42)#42
zaridan merged 2 commits into
mainfrom
feat/ht-42-watch-renewal-sweep

zaridan commented Jul 14, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

zaridan commented Jul 14, 2026

Uh oh!

coderabbitai Bot commented Jul 14, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Jul 14, 2026 •

edited

Loading

Walkthrough

Changes

Sequence Diagram(s)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

zaridan commented Jul 14, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

zaridan commented Jul 14, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

HT-42 — Gmail watch() renewal cron + reconciliation sweep

Scope decision (worth a look)

Spec refinement I made (judgment call — flagging it)

Verified (watched, not claimed)

Deferred / unchanged boundaries

Summary by CodeRabbit

Uh oh!

zaridan commented Jul 14, 2026

Uh oh!

coderabbitai Bot commented Jul 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented Jul 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

zaridan commented Jul 14, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

zaridan commented Jul 14, 2026 •

edited by coderabbitai Bot

Loading

HT-42 — Gmail `watch()` renewal cron + reconciliation sweep

coderabbitai Bot commented Jul 14, 2026 •

edited

Loading

coderabbitai Bot commented Jul 14, 2026 •

edited

Loading