feat(channels): connect several pages and numbers in one flow (#1107)

Author: viethung0o0

.agents/skills/integration-channel/SKILL.md

@@ -20,7 +20,8 @@ description: >-
 7. [Post-Creation Verification](#post-creation-verification) — lint, install, build
 8. [Platform Credentials](#platform-credentials-only-if-needed) — optional OAuth app credentials
 9. [Webhook Flow](#webhook-flow)
-10. [Existing Integrations Reference](#existing-integrations-reference)
+10. [Multi-Account Pickers](#multi-account-pickers)
+11. [Existing Integrations Reference](#existing-integrations-reference)
 
 ---
 
@@ -547,6 +548,64 @@ branded domain, so a webhook on the reseller domain silently fails. See
 `features/integration-whatsapp/actions/webhook-url.ts` and `docs/tenancy.md`
 (provider-console registration runbook).
 
+## Multi-Account Pickers
+
+Channels whose provider returns a list of connectable accounts — Messenger
+pages, Instagram-via-Facebook accounts, WhatsApp phone numbers — share one
+implementation instead of each building its own picker:
+`apps/builder/src/features/channel-connect/` (`ConnectSelectionForm` +
+`useConnectFlow` + `ConnectManyDialog` + `CoexistStep`).
+
+- **Single-account server core + oRPC route** — one plain server function per
+  provider id (e.g. `actions/connect-page.ts`'s `connectMessengerPage`),
+  returning `ConnectActionResult<TOutcome>` (`{ kind: "outcome", outcome }` or
+  `{ kind: "sessionError", code }`) — never a thrown exception the client has
+  to classify, and never a 500 (the route would be unclassifiable). Build the
+  three outcome literals with `lib/connect-action-outcomes.ts`'s
+  `notSelectableOutcome` / `duplicatedOutcome` / `connectedOutcome`, wrap
+  best-effort follow-ups (branding, tag scan, …) in `runConnectFollowUps`,
+  and convert the core's outer catch with `toConnectActionFailure`. Expose it
+  as `POST /api/channels/<channel>/connect` from the feature's `api/`
+  folder (`authorizedAPI`, ids-only input, registered through the feature's
+  `api/index.ts`) — **not** a server action: Next serializes server actions
+  from one browser, so the picker's batch could only connect one account at a
+  time. Add a server action only for a form that genuinely needs one (as
+  WhatsApp's top-level connect form does), delegating to the same core.
+- **`resolveConnectSession`** (`lib/resolve-connect-session.ts`) — reads the
+  pending-auth cookie or signup session for both legs (initial provider list
+  fetch and the per-id connect call); returns the same session-error codes
+  the outcome wire type carries.
+- **Client side** — every picker posts through `lib/connect-client.ts`'s
+  `connectViaApi` (path from `CONNECT_CHANNEL_REGISTRY[channel].connectPath`),
+  which turns any transport failure into the batch's own `failed`/`unknown`
+  outcome. `useConnectFlow` runs a single pick inline (button spinner) and
+  fans 2+ picks out through `ConnectManyDialog`'s status list,
+  `CONNECT_CONCURRENCY` at a time.
+  On a coexist-eligible channel (`isCoexistChannel`,
+  `packages/utils/channel.ts`) the "sync existing history" opt-in is a
+  **per-row switch in the picker** (`CoexistRowSwitch` /
+  `CoexistOptionsPanel`), not a step: each row's call runs right after that
+  row connects, via `lib/coexist-client.ts`'s `setCoexist`
+  (`useConnectBatch`'s `afterConnect` for a batch, inline in `useConnectFlow`
+  for a single pick). A channel with its own picker form gets the same rule
+  by calling `hooks/use-coexist-selection.tsx`'s `useCoexistSelection` —
+  don't re-derive `coexistIds ⊆ selectedIds` by hand. The dialog's Continue
+  skips every channel extra step on the session errors in
+  `SESSION_ERRORS_SKIPPING_EXTRA_STEPS` (`lib/row-status.ts`), whose routes
+  `workspaceAuthorizedMidddleware` would deny anyway.
+  `CoexistStep`/`CoexistPopup` survive only for WhatsApp's
+  manual/auto-select direct path.
+- **New channel registration** — add one entry to
+  `lib/registry.ts`'s `CONNECT_CHANNEL_REGISTRY`, typed
+  `satisfies Record<ConnectPickerChannel, ConnectChannelConfig>` so a missing
+  channel fails to compile. That file is the only place in
+  `channel-connect` allowed to hard-code a channel name.
+- **Row/warning copy** — `lib/row-status.ts` (`ROW_STATUS`,
+  `REASON_MESSAGE_KEYS`, `WARNING_MESSAGE_KEYS`,
+  `SESSION_ERROR_MESSAGE_KEYS`) is the single source of i18n keys for every
+  row state, failure reason, and outcome warning shown in the dialog — reuse
+  these, don't add a channel-local copy of the same labels.
+
 ## Existing Integrations Reference
 
 | Integration         | Auth type | Platform credentials? | Notes                                                                                                                              |

AGENTS.md

@@ -116,6 +116,9 @@ For automatic context injection on every prompt, add the hook to your **own** `.
 
 - Schema and migrations: **`packages/database`** (Drizzle). Use the **drizzle-database** skill in `.agents/skills/drizzle-database/SKILL.md` for migrations and query patterns.
 - **Migration safety:** Never run or apply `db:migrate` automatically. Generate and inspect migration SQL when needed, then wait for explicit user approval before applying it. This applies even when a plan lists `db:migrate` as a verification step.
+- **Drift guard (no database needed, runs in `pnpm lint`):** `pnpm --filter @chatbotx.io/database db:check-drift` runs `drizzle-kit generate` into a throwaway probe folder and fails if any SQL would be emitted — i.e. `src/schema` and the migration snapshot chain disagree. It is registered as that package's `lint` script, so `pnpm lint` covers it. The probe folder is swept on exit and on signals, and `run-migrations.mjs` refuses to apply a leftover one.
+- **Database-backed tests (opt-in):** `pnpm --filter @chatbotx.io/database test:db` runs `packages/database/__tests__/integration/`, which reads `information_schema.columns` from a real Postgres. They skip themselves under plain `pnpm test` (the vitest preset points `DATABASE_URL` at a non-routable sentinel) and insert no rows.
+- **A drizzle `.default()` is not proof of a database default.** `.default(...)` makes the column optional in `$inferInsert` and makes drizzle emit the bare `DEFAULT` keyword for an omitted key — it never inlines the value. drizzle-kit serializes `jsonb().default(sql`[]`)` as *no* default, so schema, snapshot and database all agree the column has none while TypeScript still calls it optional: omitting the key is a NOT NULL violation at runtime, and `db:check-drift` cannot see it. 19 columns are in that state today; they are pinned in `__tests__/integration/schema-default-parity.test.ts`, and every insert path for one of them must write the value explicitly.
 
 ### Workers & queues
 

apps/builder/__tests__/channel-connect-credential-consistency.test.ts

@@ -9,46 +9,120 @@ import { beforeEach, describe, expect, test, vi } from "vitest"
 // request host. Those legs run post-relay on the broker or branded host
 // interchangeably, so a host-derived completion leg could silently pick a
 // different OAuth app than the one the start leg authorized against,
-// breaking the token exchange. This test pins that each completion leg
-// forwards `parsedInput.workspaceId` into `resolvePlatformOwnerId`/
-// `resolveOwnerForWorkspace` unchanged, rather than re-deriving it.
+// breaking the token exchange.
+//
+// Messenger's `connectMessengerPage` (plan §2.4/§4.7) gets its `workspaceId`
+// from the encrypted, httpOnly pending-auth cookie — never client input —
+// so this test pins that it forwards the COOKIE's workspaceId into
+// `resolvePlatformOwnerId`, and that a schema-invalid/missing cookie is
+// rejected with a `sessionError` before the resolver is ever called.
+// Instagram's two legs (phase 4) now go through the same
+// `resolveConnectSession` helper, so they get the identical treatment: the
+// wire payload carries only `igId`, never `workspaceId`.
 // ---------------------------------------------------------------------------
 
-const { mockResolvePlatformOwnerId, mockResolveForOwner } = vi.hoisted(() => ({
+const {
+  mockResolvePlatformOwnerId,
+  mockResolveForOwner,
+  mockReadPendingAuth,
+  mockWorkspaceFind,
+  mockIsMember,
+} = vi.hoisted(() => ({
   mockResolvePlatformOwnerId: vi.fn(async () => "resolved-owner-1"),
   mockResolveForOwner: vi.fn(async () => undefined),
+  mockReadPendingAuth: vi.fn(
+    async (): Promise<{
+      userToken: string
+      workspaceId: string
+      referer: string
+      version: string
+      expiresAt: number
+    } | null> => ({
+      userToken: "user-token-1",
+      workspaceId: "ws-1",
+      referer: "/channels/create",
+      version: "v23.0",
+      expiresAt: Date.now() + 600_000,
+    }),
+  ),
+  mockWorkspaceFind: vi.fn(async () => ({
+    id: "ws-1",
+    ownerId: "owner-1",
+  })),
+  mockIsMember: vi.fn(async () => true),
 }))
 
 // A passthrough action-client chain: `.inputSchema()`/`.action()` just
 // return their handler so the test can call it directly with a hand-built
 // `{ ctx, parsedInput }`, without instantiating the real safe-action /
 // next-safe-action machinery. Mirrors the pattern in
 // `instagram-facebook-settings-actions.test.ts`.
-vi.mock("@/lib/safe-action", () => {
-  const chain: Record<string, unknown> = {}
-  chain.inputSchema = () => chain
-  chain.action = (handler: unknown) => handler
-  return { authActionClient: chain }
-})
-
 vi.mock("@/lib/platform-credential-owner", () => ({
   resolvePlatformOwnerId: mockResolvePlatformOwnerId,
 }))
 
+// Bypassed entirely — this test is about credential-owner resolution, not
+// the trial/MAC gate (covered by `messenger-select-page-action.test.ts`).
+vi.mock("@/lib/workspace/authorize-workspace-access", () => ({
+  checkWorkspaceOwnerAccess: vi.fn(async () => null),
+  workspaceAccessDenialException: vi.fn(
+    (reason: string) => new Error(`denied:${reason}`),
+  ),
+}))
+
 vi.mock("@chatbotx.io/business", () => ({
   platformCredentialService: { resolveForOwner: mockResolveForOwner },
-  workspaceService: { create: vi.fn() },
-  resolveTenantSettings: vi.fn(),
+  workspaceService: {
+    create: vi.fn(),
+    find: mockWorkspaceFind,
+  },
+  workspaceMemberService: { isMember: mockIsMember },
+  resolveTenantSettings: vi.fn(async () => ({ appUrl: "https://app.test" })),
   updateInstagramIntegrationUserInfo: vi.fn(),
   updateMessengerIntegrationUserInfo: vi.fn(),
+  messengerIntegrationService: {
+    findConnectedPageIds: vi.fn(async () => new Set<string>()),
+    connectPage: vi.fn(),
+    updateUserInfo: vi.fn(),
+  },
+  instagramIntegrationService: {
+    findConnectedIgIds: vi.fn(async () => new Set<string>()),
+    connectAccount: vi.fn(),
+    updateUserInfo: vi.fn(),
+  },
   tagSyncService: { enqueueChannelScan: vi.fn() },
   userQuotaService: { getAccessState: vi.fn(async () => ({ blocked: false })) },
   connectChannelIntegration: vi.fn(),
+  buildContext: vi.fn(async () => ({})),
 }))
 
-vi.mock("@chatbotx.io/business/errors", () => ({
-  ChatbotXException: class ChatbotXException extends Error {},
-}))
+// The REAL session/item-outcome mapping table — `resolveConnectSession`
+// (called by `connectMessengerPage`) throws genuine exceptions from the
+// (also real, below) `@chatbotx.io/business/errors`, so this file lets the
+// real mapping classify them instead of re-implementing that table as a
+// second source of truth that could silently drift from production.
+vi.mock("@chatbotx.io/business/inbox/connect-outcome", async (importOriginal) =>
+  importOriginal(),
+)
+
+vi.mock("@chatbotx.io/business/errors", () => {
+  class ChatbotXException extends Error {
+    code?: string
+    constructor(message: string, code?: string) {
+      super(message)
+      this.code = code
+    }
+  }
+  return {
+    ChatbotXException,
+    connectSessionExpiredException: (message: string) =>
+      new ChatbotXException(message, "connectSessionExpired"),
+    notWorkspaceMemberException: () =>
+      new ChatbotXException("not a member", "notWorkspaceMember"),
+    credentialMissingException: (message: string) =>
+      new ChatbotXException(message, "credentialMissing"),
+  }
+})
 
 vi.mock("@chatbotx.io/database/client", () => ({
   db: { transaction: vi.fn(async () => undefined) },
@@ -67,6 +141,17 @@ vi.mock("@chatbotx.io/database/schema", async (importOriginal) => {
 
 vi.mock("@chatbotx.io/integration-messenger", () => ({
   integration: { runChannelHandler: vi.fn() },
+  getUserPages: vi.fn(async () => ({
+    pages: [
+      {
+        id: "p1",
+        name: "Page",
+        access_token: "page-token",
+        isConnectable: true,
+      },
+    ],
+    bmLookupFailed: false,
+  })),
 }))
 vi.mock("@chatbotx.io/integration-messenger/apis/page", () => ({
   exchangeLongLivedToken: vi.fn(),
@@ -107,7 +192,8 @@ vi.mock("@/lib/facebook-pending-auth", () => ({
   FB_MESSENGER_PENDING_AUTH_COOKIE: "fb_messenger_pending_auth",
   FB_INSTAGRAM_FACEBOOK_PENDING_AUTH_COOKIE:
     "fb_instagram_facebook_pending_auth",
-  readPendingAuth: vi.fn(async () => null),
+  FB_INSTAGRAM_PENDING_AUTH_COOKIE: "fb_instagram_pending_auth",
+  readPendingAuth: mockReadPendingAuth,
 }))
 vi.mock("@/lib/integration-user-info", () => ({
   persistIntegrationUserInfo: vi.fn(),
@@ -116,23 +202,16 @@ vi.mock("@/lib/log", () => ({
   logger: { warn: vi.fn(), error: vi.fn(), info: vi.fn() },
 }))
 
-const { selectPageAction } = await import(
-  "../src/features/integration-messenger/actions/select-page.action"
+const { connectMessengerPage } = await import(
+  "../src/features/integration-messenger/actions/connect-page"
 )
-const { selectAccountAction } = await import(
-  "../src/features/integration-instagram/actions/select-account.action"
+const { connectInstagramAccount } = await import(
+  "../src/features/integration-instagram/actions/connect-account"
 )
-const { selectFacebookAccountAction } = await import(
-  "../src/features/integration-instagram/actions/select-account-facebook.action"
+const { connectInstagramAccountViaFacebook } = await import(
+  "../src/features/integration-instagram/actions/connect-account-facebook"
 )
 
-type ActionHandler = (args: {
-  parsedInput: Record<string, unknown>
-  ctx: { user: { id: string } }
-}) => Promise<unknown>
-
-const call = (action: unknown) => action as ActionHandler
-
 describe("channel connect completion legs never re-derive the credential owner from the host", () => {
   beforeEach(() => {
     vi.clearAllMocks()
@@ -141,36 +220,43 @@ describe("channel connect completion legs never re-derive the credential owner f
     // resolver call — exactly the point this test needs to observe, without
     // running the rest of the (heavily mocked) connect transaction.
     mockResolveForOwner.mockResolvedValue(undefined)
+    mockReadPendingAuth.mockResolvedValue({
+      userToken: "user-token-1",
+      workspaceId: "ws-1",
+      referer: "/channels/create",
+      version: "v23.0",
+      expiresAt: Date.now() + 600_000,
+    })
+    mockWorkspaceFind.mockResolvedValue({ id: "ws-1", ownerId: "owner-1" })
+    mockIsMember.mockResolvedValue(true)
   })
 
-  test("select-page.action (messenger) forwards workspaceId unchanged", async () => {
-    await call(selectPageAction)({
-      parsedInput: { workspaceId: "ws-1", pageId: "p1", pageName: "Page" },
-      ctx: { user: { id: "user-1" } },
-    }).catch(() => undefined)
+  test("connectMessengerPage resolves the credential owner from the pending-auth cookie's workspaceId", async () => {
+    await connectMessengerPage({ userId: "user-1", pageId: "p1" }).catch(
+      () => undefined,
+    )
 
     expect(mockResolvePlatformOwnerId).toHaveBeenCalledWith({
       userId: "user-1",
       workspaceId: "ws-1",
     })
   })
 
-  test("select-account.action (instagram) forwards workspaceId unchanged", async () => {
-    await call(selectAccountAction)({
-      parsedInput: { workspaceId: "ws-1", igId: "ig1", igName: "IG" },
-      ctx: { user: { id: "user-1" } },
-    }).catch(() => undefined)
+  test("connectInstagramAccount resolves the credential owner from the pending-auth cookie's workspaceId", async () => {
+    await connectInstagramAccount({ userId: "user-1", igId: "ig1" }).catch(
+      () => undefined,
+    )
 
     expect(mockResolvePlatformOwnerId).toHaveBeenCalledWith({
       userId: "user-1",
       workspaceId: "ws-1",
     })
   })
 
-  test("select-account-facebook.action (instagram via Facebook) forwards workspaceId unchanged", async () => {
-    await call(selectFacebookAccountAction)({
-      parsedInput: { workspaceId: "ws-1", igId: "ig1", igName: "IG" },
-      ctx: { user: { id: "user-1" } },
+  test("connectInstagramAccountViaFacebook resolves the credential owner from the pending-auth cookie's workspaceId", async () => {
+    await connectInstagramAccountViaFacebook({
+      userId: "user-1",
+      igId: "ig1",
     }).catch(() => undefined)
 
     expect(mockResolvePlatformOwnerId).toHaveBeenCalledWith({
@@ -179,15 +265,16 @@ describe("channel connect completion legs never re-derive the credential owner f
     })
   })
 
-  test("no-workspace-yet connects (first channel ever) forward a nullish workspaceId, not a guessed one", async () => {
-    await call(selectPageAction)({
-      parsedInput: { workspaceId: undefined, pageId: "p1", pageName: "Page" },
-      ctx: { user: { id: "user-1" } },
-    }).catch(() => undefined)
+  test("connectMessengerPage never calls the resolver when the pending-auth cookie is missing/schema-invalid", async () => {
+    mockReadPendingAuth.mockResolvedValue(null)
 
-    expect(mockResolvePlatformOwnerId).toHaveBeenCalledWith({
+    const result = await connectMessengerPage({
       userId: "user-1",
-      workspaceId: undefined,
+      pageId: "p1",
     })
+
+    expect(result).toEqual({ kind: "sessionError", code: "sessionExpired" })
+    expect(mockResolvePlatformOwnerId).not.toHaveBeenCalled()
+    expect(mockWorkspaceFind).not.toHaveBeenCalled()
   })
 })