Skip to content

Consolidate edge functions#2855

Open
malek10xdev wants to merge 64 commits into
mainfrom
consolidate-edge-functions
Open

Consolidate edge functions#2855
malek10xdev wants to merge 64 commits into
mainfrom
consolidate-edge-functions

Conversation

@malek10xdev

@malek10xdev malek10xdev commented Jun 29, 2026

Copy link
Copy Markdown
Collaborator

Summary

Consolidates 3 edge function migrations into a single PR: export-contacts (CSV, VCard, Google Contacts), enrich (3 engines), and imap-boxes.

Edge function env vars

The following env vars are now required in supabase/functions/.env.dev for local dev. In production, SUPABASE_URL, SUPABASE_ANON_KEY, and SUPABASE_SERVICE_ROLE_KEY are auto-injected.

Var Purpose Required
SUPABASE_URL, SUPABASE_ANON_KEY, SUPABASE_SERVICE_ROLE_KEY Supabase runtime Yes (local)
ENRICHLAYER_URL, ENRICHLAYER_API_KEY EnrichLayer engine Optional
THEDIG_URL, THEDIG_API_KEY TheDig engine Optional
VOILANORBERT_URL, VOILANORBERT_USERNAME, VOILANORBERT_API_KEY VoilaNorbert (async) Optional
OPENWA_API_URL, OPENWA_API_KEY, OPENWA_BASE_URL, OPENWA_WEBHOOK_SECRET WhatsApp automation Optional
REDIS_URL Rate limiter (falls back to in-memory) Optional
LOG_LEVEL, DENO_ENV, APP_NAME, FALLBACK_SENDER_ENABLED Runtime config Optional

See supabase/functions/.env.example for the full template with annotations.

Auth

The enrich function uses verify_jwt = false at the gateway. Auth is per-route via authMiddleware (accepts user JWT or service-role key). The /webhook/:id route is unauthenticated — uses token + id in body for auth (like backend webhooks).

Test infrastructure

Added backend/src/mocks/enrichment/endpoints/thedig.ts (TheDig mock) and server-with-thedig.ts (mock server variant). The default mock server (server.ts) is unchanged; the new variant is for local enrichment testing.

enrich_contacts RPC requires user_id in every contact record.
The enrich function was sending enriched data without user_id,
causing 'All records in p_contacts_data must contain user_id field' error.

Also adds enrich function config to supabase/config.toml for
Docker edge runtime registration.
Add a new POST /api/imap/boxes endpoint to the emails-fetcher that
retrieves the IMAP folder tree for a given email. The endpoint:
- Validates userId and email input
- Fetches IMAP credentials from the mining source service
- Opens an IMAP connection via ImapConnectionProvider
- Uses ImapBoxesFetcher to build the IMAP folder tree
- Properly handles connection cleanup in finally block
Replace the backend API call (/api/imap/boxes) with a direct Supabase
edge function invocation (imap/boxes). Only requires email now, since
the edge function resolves the user ID from the auth token.
Migrate the IMAP edge function from raw Deno.serve to Hono framework.
Add POST /boxes endpoint that proxies IMAP folder listing requests to
the emails-fetcher microservice. Refactor /detect to use Hono routing.
Remove the old backend IMAP controller, routes, and service for listing
IMAP boxes. This functionality has been migrated to:
- The supabase/functions/imap edge function (proxies to emails-fetcher)
- The micro-services/emails-fetcher microservice (actual IMAP logic)
…or, make MiningSource.id required

- Conflicts #1, #2, #3: Take deletion (enrich edge function replaces backend enrichment layer)
- Conflict #4: Keep MiningSource.id required (fix frontend callers if needed)
@github-actions

github-actions Bot commented Jun 29, 2026

Copy link
Copy Markdown

Coverage Report

Passed

Commit: 6a791e4

Summary

Name Stmts Branch Funcs Lines
🟡 Total 40.9% 39% 46.4% 41.3%
All files
Name Stmts Branch Funcs Lines
🔴 backend/src/app.ts 0% 0% 0% 0%
🔴 backend/src/controllers/contacts.controller.ts 0% 100% 0% 0%
🔴 backend/src/db/pg/PgContacts.ts 35.1% 6.3% 47.8% 33.6%
🔴 backend/src/mocks/enrichment/server-with-thedig.ts 0% 100% 0% 0%
🔴 backend/src/mocks/enrichment/endpoints/thedig.ts 0% 0% 0% 0%
🔴 backend/src/routes/contacts.routes.ts 0% 100% 0% 0%
No coverage changes
Name Stmts Branch Funcs Lines
🔴 backend/src/app.ts 0% 0% 0% 0%
🔴 backend/src/controllers/contacts.controller.ts 0% 100% 0% 0%
🔴 backend/src/db/pg/PgContacts.ts 35.1% 6.3% 47.8% 33.6%
🔴 backend/src/mocks/enrichment/server-with-thedig.ts 0% 100% 0% 0%
🔴 backend/src/mocks/enrichment/endpoints/thedig.ts 0% 0% 0% 0%
🔴 backend/src/routes/contacts.routes.ts 0% 100% 0% 0%

Generated by Test Coverage Reporter for commit 6a791e4

- Add passive-mining to config.toml with verify_jwt=false (C3)
- Remove dead service-role branch from mixedAuth — verify_jwt=true blocks it at gateway (C1)
- Remove authorizeUser Express dead code from middlewares.ts (M3)
- Make rate limiter graceful when REDIS_URL missing — returns no-op instead of throwing (C2)
- Remove dead getExportedContacts/getNonExportedContacts/registerExportedContacts from edge ContactsClient (M1)
…proper type

Introduce a PopulatedTree intersection type that captures the
post-construction state of FlatTree nodes (total and cumulativeTotal
are always set by createFlatTreeFromImap). Use the narrowed type as
the return type of createFlatTreeFromImap and the parameter type of
buildFinalTree, eliminating the need for non-null assertions inside
the loop body.

Removes two skipcq: JS-0339 suppressions.
…bers

Each ExportStrategy implementation (CsvExport, VCardExport,
GoogleContactsExport) held no instance state - the export logic is
pure data transformation. Per DeepSource JS-0105, methods that do not
use 'this' should be static.

Changes:
- Promote 'type' and 'export' to static members on each strategy class
- Remove the 'implements ExportStrategy<T>' clauses; TypeScript's
  structural typing verifies assignability against the interface
  without requiring the class to expose the contract as instance
  members
- Update ExportFactory to register classes (not instances) so the
  factory's lookup matches the new contract
- Update tests to call static methods (CsvExport.export(...) instead
  of new CsvExport().export(...))
- Add missing static 'ExportType' import in the unsupported-type test
- Consolidate the duplicated ExportStrategy interface and remove the
  unused BaseExportStrategy abstract class in formats/strategy.ts;
  strategy.ts now only re-exports types from types.ts

Removes three skipcq: JS-0105 suppressions.
Per DeepSource JS-0105, methods that do not reference 'this' should be
static. Each engine's validity check is a pure function over the
contact - no instance state is consulted - so promote 'isValid' on
EnrichLayer, TheDig, and Voilanorbert to static class members.

The 'name'/'isSync'/'isAsync' metadata is also exposed as static
readonly, and the instance properties delegate to the statics so the
existing 'instance.name' references in parseResult/logging keep
working without churn.

EnrichLayer's placeholder enrichAsync (which throws because the engine
is sync-only) is rewritten as an instance method that references
'this.constructor.name' in the error message, giving the lint a real
'use of this' rather than a suppression.

The Enricher now stores { ctor, instance } pairs so validity filtering
can invoke the static isValid on the class while instance-bound
methods (enrichSync, parseResult, enrichAsync on TheDig/Voilanorbert)
still operate on per-engine state (e.g. injected API clients).

The 'implements Engine' clauses are dropped in favor of structural
typing; Engine is now reserved for instance members and EngineClass
captures the static-side contract.

Removes four skipcq: JS-0105 suppressions.
After the PopulatedTree intersection was introduced, box.parent was
typed as FlatTree | undefined even though pathMap only stores
PopulatedTree values - TypeScript does not narrow recursive
references. Add an asPopulated type guard and a self-referential
parent?: PopulatedTree / children?: PopulatedTree declaration so
buildFinalTree can read parent.cumulativeTotal without re-introducing
non-null assertions.

This is a follow-up to 473dc27 (the PopulatedTree type was correct
in isolation but missed the parent/children recursive narrowing).
…ble, matching backend

- Use RateLimiterMemory/RateLimiterRedis wrapped in RateLimiterQueue
- Fall back to in-memory when REDIS_URL is unset
- Preserve public API (QuotaType, withRateLimit, TokenBucketRateLimiter)
- Bounded retry on RateLimiterRes with msBeforeNext delay
- Shared edge createLogger instead of winston
- export-contacts/deno.json: added rate-limiter-flexible import map
Reviews #2 + #3 from PR #2855 code review:

Review #2: Port backend enrichment engines (no rewrite from scratch)
- engine.ts: port of Engine interface (EngineResponse, EngineResult, Person)
- enricher.ts: port of Enricher orchestrator
- enrich-layer.ts, thedig.ts, voilanorbert.ts: per-engine ports
- validation.ts: undefinedIfEmpty/undefinedIfFalsy helpers
- engines.ts: barrel re-export for backward compat
- Adapted: axios -> fetch, winston -> createLogger, qs -> URLSearchParams,
  ENV -> Deno.env.get, backend rate-limiter -> edge rate-limiter-flexible

Review #3: Extract SQL ops into client classes (mirrors backend pattern)
- db-types.ts: TaskStatus, TaskType, EnrichTask, EngagementType, etc.
- tasks-client.ts: port of SupabaseTasks
- engagements-client.ts: port of Engagements
- enrichments-client.ts: port of Enrichments (main task lifecycle)
- enrichment-helpers.ts: getContactsToEnrich, getEnrichmentCache, enrichFromCache
- index.ts: 522 lines of inline SQL -> delegations to new clients (-250 net LOC)
- Constructor injection: new EnrichmentsClient(supabase, logger)
- No module-scoped globals
- Dropped billing hooks (not used in edge)
- Preserved sameAs/alternateName/telephone join mappings for enrich_contacts RPC
- Preserved ENRICH engagement registration
@malek10xdev
malek10xdev force-pushed the consolidate-edge-functions branch from bd50b33 to 0f8f48a Compare July 3, 2026 12:37
…or protected routes

The /webhook/:id route uses token+id-based auth (like backend webhooks)
and should not require a JWT at the Supabase gateway.

- verify_jwt = false on the enrich function
- authMiddleware applied to /person and /person/bulk routes (accepts service role OR user JWT)
- /webhook/:id route stays unauthenticated at gateway level
- Re-tested: auth flows work, webhook accessible without JWT
@malek10xdev
malek10xdev force-pushed the consolidate-edge-functions branch from 0f8f48a to 4ef0490 Compare July 3, 2026 12:39
- Prettier: format new mock server files (thedig.ts, server-with-thedig.ts)
- DeepSource MINOR (JS-0246): convert string concat to template literals in phone.ts
- DeepSource CRITICAL: false positive on local Supabase demo JWTs in
  supabase/functions/.env.dev and .env.example. Added env files to
  .deepsource.toml exclude_patterns to suppress.
The rate limiter (token-bucket over Redis or in-memory) throttles
outbound calls to EnrichLayer, TheDig, VoilaNorbert, and Google
People API. Until now, if the underlying store (Redis) became
unreachable at runtime, the limiter would throw and the
enrich/export request would fail.

This change adds two safe wrappers:
- TokenBucketRateLimiter.removeTokensSafe() — never throws
- withRateLimitSafe() — never throws

Both log a warning and proceed without throttling when the limiter
fails. This matches the user's intent: rate limiting is a safety
guardrail for OUTBOUND calls, not a hard gate.

Updated callers:
- enrich/services/enrich-layer.ts (1 use)
- enrich/services/thedig.ts (2 uses)
- enrich/services/voilanorbert.ts (1 use)
- export-contacts/formats/google/contacts-api.ts (2 uses)

Also adds supabase/functions/_shared/rate-limiter.test.ts covering
the safe wrappers, bucket refill, and per-key isolation.

Closes tasks 5 + 6 of .rate-limiting-deferred.md (graceful
fallback + tests).
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