feat(connections): unify channel/app integration connect lifecycle - #1185
Open
realcodesiman wants to merge 2 commits into
Open
realcodesiman wants to merge 2 commits into
realcodesiman wants to merge 2 commits into
Conversation
Replace ~22 per-provider connect/disconnect action pairs and three cookie-based OAuth picker flows with one connection-providers / connections / connect-sessions resource family, backed by a new server-side ConnectSession table instead of pending-auth cookies. The same registry and business services now serve the builder UI (private oRPC), the public API (workspace tokens), and MCP (generated from the shared OpenAPI document), so every IntegrationType is connectable and discoverable through one consistent surface. Legacy per-provider public lifecycle routes remain but are marked deprecated.
…eview Addresses the code-review findings on the Connection-lifecycle unification (unify channel/app integration connect lifecycle, commit f437105): Critical: - ConnectSession.create writes targets/claimedTargetIds/resultConnectionIds/ results explicitly (drizzle-kit never inlines a sql template default for jsonb/array columns) - every OAuth connect was NOT NULL-violating on insert. - markOffline(reason) branches on instanceof AuthException: a revoked token still marks unhealthy, but a transient refresh failure now degrades (quota held) instead of releasing the channel's quota slot. - Messenger/Instagram/InstagramFacebook disconnect actions now transition the Connection row (not just Inbox), fixing a quota double-charge on connect -> UI-disconnect -> reconnect. - Telegram plus the 8 api_key marketing providers gained a live fromCredentials validator; api/smtp/webchat/chatbotx are explicitly gated unavailableReason "notImplemented" pending their sourceId identity model instead of silently 500ing. Important: - Telegram's Connection.sourceId derives from the bot token's numeric prefix instead of a colliding "workspace" constant. - Disconnect teardown failures are surfaced onto Connection.lastError; the satellite-row delete and FSM transition share one db.transaction. - Telegram's verify() returns a ConnectionHealth failure instead of throwing. - drip/activeCampaign/getResponse's isRevokedTokenError TODO stubs now classify real 401/403 responses. - ConnectionStateService.transition consumes quota before the status write (never needs a rollback) and throws instead of silently skipping a required quota edge with no resolvable owner. - completeAuthorization's exchangeCode catch now records exchange_failed instead of overloading provider_denied (real denial is filtered upstream by the OAuth callback's own error query-param check). - Messenger's exchangeCode/listCandidates carry the real callbackUrl as redirectUrl instead of an empty string. - /connect is public (the ConnectSession completion page never requires a builder session). Simplifications: - packages/connections/src/service.ts split into internal.ts, lifecycle.ts, credentials.ts, and connect-session-flow.ts (each under the 800-line cap), deduplicating the revive-or-insert-then-transition block into upsertConnectionRow. - Shared isUnauthorizedStatusError SDK helper replaces duplicated 401/403 duck-typing across 7 marketing providers. - Shared verifyMetaToken(label) factory for Messenger/Instagram-Facebook. - .agents/rules/data-access.md documents packages/connections as the registry-aware tier above packages/business. Plus test coverage for every fix above (ordering, auth-safety, scope enforcement on the public API, the inbox-status write guard, registry invariants) and five stale comments and docs corrected.
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
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
connection-providers/connections/connect-sessionsresource family.ConnectSessiontable (packages/database/src/schema/connect-session.ts) replacing the pending-auth cookies, so OAuth flows no longer depend on browser cookies and can be resumed/polled via/connect/[sessionId].IntegrationTypeis connectable and discoverable through one consistent surface. Legacy per-provider public lifecycle routes remain reachable but are markeddeprecated.Changes
packages/connectionspackage: provider registry + connection service.packages/business/src/connectionandpackages/business/src/connect-session: connection state/adapter services and session lifecycle service.Connection,ConnectSession(packages/database/drizzle/20260914163422_consolidate_connection_scope,.../20260914170910_add_connection_tables), plus a backfill script (packages/database/scripts/backfill-connections.ts).apps/builder/src/features/connections(actions, oRPC private/public API, schemas) andapps/builder/src/app/connect/[sessionId]UI for the unified OAuth completion/handoff flow.purge-expired-connect-sessions.apps/builder/src/lib/facebook-pending-auth.ts(cookie-based pending-auth) and its tests, superseded byConnectSession.Test plan
pnpm lint/ typecheck (ran automatically via pre-commit hooks — passed)pnpm --filter builder check-typesconnectionsendpoints via a workspace tokenpackages/databasemigrations apply cleanly (db:migrate— requires explicit approval per project rules)🤖 Generated with Claude Code