Skip to content

refactor(polling): consolidate polling services into provider handler pattern#4035

Merged
waleedlatif1 merged 4 commits intostagingfrom
waleedlatif1/refactor-polling-services
Apr 8, 2026
Merged

refactor(polling): consolidate polling services into provider handler pattern#4035
waleedlatif1 merged 4 commits intostagingfrom
waleedlatif1/refactor-polling-services

Conversation

@waleedlatif1
Copy link
Copy Markdown
Collaborator

@waleedlatif1 waleedlatif1 commented Apr 8, 2026

Summary

  • Eliminates self-POST anti-pattern: Adds processPolledWebhookEvent() to processor.ts for direct in-process webhook execution, removing HTTP round-trips that caused Lambda 403/timeout errors under load
  • Extracts shared boilerplate: markWebhookFailed/Success, fetchActiveWebhooks, runWithConcurrency, resolveOAuthCredential, updateWebhookProviderConfig — previously duplicated across all 4 services (~30% of each file)
  • Consolidates routes: Replaces 4 identical route files with a single dynamic [provider] route. Helm cron paths (/api/webhooks/poll/gmail, etc.) resolve unchanged via Next.js dynamic routing
  • Standardizes concurrency: All providers now use CONCURRENCY = 10 (IMAP was previously 5)

New structure (mirrors lib/webhooks/providers/)

lib/webhooks/polling/
├── types.ts          # PollingProviderHandler interface
├── registry.ts       # Handler lookup + VALID_POLLING_PROVIDERS
├── utils.ts          # 6 shared utilities
├── orchestrator.ts   # pollProvider() entry point
├── index.ts          # Barrel exports
├── gmail.ts          # Gmail handler
├── outlook.ts        # Outlook handler
├── imap.ts           # IMAP handler
└── rss.ts            # RSS handler

app/api/webhooks/poll/
└── [provider]/route.ts   # Single dynamic route

No infra changes needed

  • Helm cron job paths remain /api/webhooks/poll/{gmail,outlook,imap,rss} — dynamic route matches identically
  • Lock keys (${provider}-polling-lock) produce the same strings as before
  • maxDuration = 180, dynamic = 'force-dynamic', verifyCronAuth — all preserved

Test plan

  • bun run type-check passes (only pre-existing Athena errors)
  • Verify no dangling imports: grep -r "polling-service" apps/sim/ returns nothing
  • Deploy to staging — Lambda cron hits /api/webhooks/poll/gmail etc. and returns 200
  • Verify distributed locks prevent concurrent polling
  • Verify Gmail historyId tracking, Outlook folder filtering, IMAP UID tracking, RSS GUID dedup all work correctly

… pattern

Eliminate self-POST anti-pattern and extract shared boilerplate from 4 polling
services into a clean handler registry mirroring lib/webhooks/providers/.

- Add processPolledWebhookEvent() to processor.ts for direct in-process webhook
  execution, removing HTTP round-trips that caused Lambda 403/timeout errors
- Extract shared utilities (markWebhookFailed/Success, fetchActiveWebhooks,
  runWithConcurrency, resolveOAuthCredential, updateWebhookProviderConfig)
- Create PollingProviderHandler interface with per-provider implementations
- Consolidate 4 identical route files into single dynamic [provider] route
- Standardize concurrency to 10 across all providers
- No infra changes needed — Helm cron paths resolve via dynamic route

Co-Authored-By: Claude Opus 4.6 <[email protected]>
@cursor
Copy link
Copy Markdown

cursor bot commented Apr 8, 2026

PR Summary

Medium Risk
Touches polling execution and webhook dispatch by replacing HTTP self-calls with in-process processing and shared orchestration, so regressions could impact webhook triggering, state updates, or concurrency/locking behavior across providers.

Overview
Refactors polling to a provider handler + orchestrator model (lib/webhooks/polling/*), replacing per-provider polling services with gmail/imap/outlook/rss handlers registered via registry.ts and executed through pollProvider() with shared concurrency and DB utilities.

Consolidates API endpoints by replacing separate /api/webhooks/poll/{provider} routes with a single dynamic app/api/webhooks/poll/[provider]/route.ts, including provider validation and per-provider Redis lock keys.

Eliminates the polling self-POST pattern by adding processPolledWebhookEvent() in lib/webhooks/processor.ts, so polling handlers dispatch webhook executions directly (with admission control, deployment block checks, and queueing) instead of calling /api/webhooks/trigger/... over HTTP.

Reviewed by Cursor Bugbot for commit 4a1000c. Configure here.

@vercel
Copy link
Copy Markdown

vercel bot commented Apr 8, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Apr 8, 2026 3:48am

Request Review

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Apr 8, 2026

Greptile Summary

This PR is a well-scoped structural refactor that consolidates four near-identical polling services (Gmail, Outlook, IMAP, RSS) into a unified handler pattern under lib/webhooks/polling/, and replaces four duplicate route files with a single dynamic [provider] route. The primary behavioral improvement is the elimination of the self-POST anti-pattern via the new processPolledWebhookEvent() function in processor.ts, which routes polled events directly in-process.

Key changes:

  • New lib/webhooks/polling/ module: types.ts (strategy interface), registry.ts (handler lookup + VALID_POLLING_PROVIDERS), utils.ts (6 shared helpers extracted from per-provider duplication), orchestrator.ts (entry point), and one handler file per provider.
  • processor.ts: Adds processPolledWebhookEvent() that admits, preprocesses, checks block existence, handles billing, and dispatches to BullMQ or inline job queue — bypassing the HTTP round-trip.
  • app/api/webhooks/poll/[provider]/route.ts: Single dynamic route replacing 4 identical files; auth check correctly precedes provider validation; lock TTL tied to maxDuration.
  • Deleted files: gmail-polling-service.ts, rss-polling-service.ts, and the 3 individual route files.
  • All prior review feedback (archived webhook filtering, auth-before-enumeration ordering, idempotency import path) has been addressed in this version.

Confidence Score: 5/5

Safe to merge — all prior P1 concerns are resolved, no new correctness or security issues introduced.

All previously raised P1 findings (archived webhook filtering, auth-before-enumeration, idempotency import path) are addressed. Remaining findings are P2: a discarded preprocessing error detail, duplicated local interfaces in processor.ts, and verbose logger type annotations in private helpers. None affect runtime correctness.

apps/sim/lib/webhooks/processor.ts — minor observability gap in preprocessResult error handling and interface duplication.

Vulnerabilities

No security issues identified. The dynamic [provider] route correctly places verifyCronAuth before the VALID_POLLING_PROVIDERS check, preventing provider enumeration by unauthenticated callers. IMAP host validation (validateDatabaseHost) and RSS URL validation (validateUrlWithDNS + secureFetchWithPinnedIP) are preserved. OAuth credential resolution in resolveOAuthCredential uses the existing token-refresh flow unchanged. Admission-ticket acquisition and release in processPolledWebhookEvent follow the correct guard pattern (early return before try/finally when no ticket is issued).

Important Files Changed

Filename Overview
apps/sim/app/api/webhooks/poll/[provider]/route.ts Consolidates 4 identical route files into one dynamic route; auth check correctly precedes provider validation; lock key and TTL preserved correctly.
apps/sim/lib/webhooks/polling/utils.ts Extracts shared boilerplate (markWebhookFailed/Success, fetchActiveWebhooks with archivedAt guards, runWithConcurrency, resolveOAuthCredential, updateWebhookProviderConfig) that was previously duplicated across all 4 providers.
apps/sim/lib/webhooks/polling/orchestrator.ts Clean entry point delegating to handler registry and shared concurrency runner; correct per-webhook requestId scoping.
apps/sim/lib/webhooks/processor.ts Adds processPolledWebhookEvent() to bypass self-POST; preprocessResult.error is discarded without logging; PolledWebhookRecord/PolledWorkflowRecord duplicate types in polling/types.ts.
apps/sim/lib/webhooks/polling/gmail.ts Ports Gmail polling logic faithfully; uses ReturnType<typeof import('@sim/logger').createLogger> instead of Logger type in private helpers — minor style inconsistency.
apps/sim/lib/webhooks/polling/types.ts Clean strategy interface for PollingProviderHandler; WebhookRecord/WorkflowRecord types well-defined but not reused in processor.ts.
apps/sim/lib/webhooks/polling/registry.ts Simple handler lookup with VALID_POLLING_PROVIDERS derived from registry keys — adding a new provider only requires one entry here.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    CRON["Helm Cron\n/api/webhooks/poll/{provider}"]
    ROUTE["[provider]/route.ts\nverifyCronAuth → VALID_POLLING_PROVIDERS\nacquireLock"]
    ORCHESTRATOR["orchestrator.ts\npollProvider()"]
    REGISTRY["registry.ts\ngetPollingHandler()"]
    UTILS_FETCH["utils.ts\nfetchActiveWebhooks()"]
    CONCURRENCY["utils.ts\nrunWithConcurrency(CONCURRENCY=10)"]
    GMAIL["gmail.ts\ngmailPollingHandler"]
    OUTLOOK["outlook.ts\noutlookPollingHandler"]
    IMAP["imap.ts\nimapPollingHandler"]
    RSS["rss.ts\nrssPollingHandler"]
    IDEMPOTENCY["pollingIdempotency\n(per item)"]
    PROCESSOR["processor.ts\nprocessPolledWebhookEvent()"]
    ADMIT["tryAdmit() ticket"]
    PREPROCESS["checkWebhookPreprocessing()"]
    DISPATCH["BullMQ / inline job queue\nwebhook-execution"]
    CRON --> ROUTE
    ROUTE --> ORCHESTRATOR
    ORCHESTRATOR --> REGISTRY
    ORCHESTRATOR --> UTILS_FETCH
    UTILS_FETCH --> CONCURRENCY
    CONCURRENCY --> GMAIL & OUTLOOK & IMAP & RSS
    GMAIL & OUTLOOK & IMAP & RSS --> IDEMPOTENCY
    IDEMPOTENCY --> PROCESSOR
    PROCESSOR --> ADMIT
    ADMIT --> PREPROCESS
    PREPROCESS --> DISPATCH
Loading

Reviews (2): Last reviewed commit: "fix(polling): use literal for maxDuratio..." | Re-trigger Greptile

…e casts

- Widen processPolledWebhookEvent body param to accept object, eliminating
  `as unknown as Record<string, unknown>` double casts in all 4 handlers
- Extract LOCK_TTL_SECONDS constant in route, tying maxDuration and lock TTL
  to a single value

Co-Authored-By: Claude Opus 4.6 <[email protected]>
- Add archivedAt filters to fetchActiveWebhooks query, matching
  findWebhookAndWorkflow in processor.ts to prevent polling archived
  webhooks/workflows
- Move provider validation after auth check to prevent provider
  enumeration by unauthenticated callers
- Fix inconsistent pollingIdempotency import path in outlook.ts to
  match other handlers

Co-Authored-By: Claude Opus 4.6 <[email protected]>
@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@cursor review

Next.js requires segment config exports to be statically analyzable
literals. Using a variable reference caused build failure.

Co-Authored-By: Claude Opus 4.6 <[email protected]>
Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 4a1000c. Configure here.

@waleedlatif1 waleedlatif1 merged commit 086b7d9 into staging Apr 8, 2026
11 checks passed
@waleedlatif1 waleedlatif1 deleted the waleedlatif1/refactor-polling-services branch April 8, 2026 03:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant