From a05b8fab25e2d8adcbed68444386f83e858296e4 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 30 Sep 2026 18:23:21 -0700 Subject: [PATCH 1/2] fix(integrations): close connection recovery edge cases --- .github/workflows/desktop-e2e.yml | 2 + apps/desktop/e2e/source-connect.spec.ts | 138 ++++++++++++++++++ .../callback/route.test.ts | 18 +++ .../slack-managed-users/callback/route.ts | 2 +- .../api/desktop/source-connect/route.test.ts | 32 ++++ .../app/api/desktop/source-connect/route.ts | 1 + apps/sim/hooks/queries/slack-search.ts | 2 +- .../use-search-integration-connection.ts | 1 + .../fixtures/desktop-source-connect.tsx | 38 ++++- 9 files changed, 230 insertions(+), 4 deletions(-) create mode 100644 apps/sim/app/api/credential-groups/slack-managed-users/callback/route.test.ts create mode 100644 apps/sim/app/api/desktop/source-connect/route.test.ts diff --git a/.github/workflows/desktop-e2e.yml b/.github/workflows/desktop-e2e.yml index 14ebd277e2d..c9d4f92a539 100644 --- a/.github/workflows/desktop-e2e.yml +++ b/.github/workflows/desktop-e2e.yml @@ -17,6 +17,8 @@ on: - 'apps/sim/app/desktop/connect/**' - 'apps/sim/app/credential-groups/**' - 'apps/sim/hooks/queries/slack-search.ts' + - 'apps/sim/hooks/queries/personal-search-integrations.ts' + - 'apps/sim/hooks/use-search-integration-connection.ts' - 'apps/sim/hooks/use-github-installation-setup.ts' - 'apps/sim/app/o/**/integrations/indexed/use-member-enrollment.ts' - 'apps/sim/lib/api/contracts/desktop-source-connect.ts' diff --git a/apps/desktop/e2e/source-connect.spec.ts b/apps/desktop/e2e/source-connect.spec.ts index c260cec768d..ba6d9cca2e1 100644 --- a/apps/desktop/e2e/source-connect.spec.ts +++ b/apps/desktop/e2e/source-connect.spec.ts @@ -54,6 +54,10 @@ test('source authorization returns to its desktop screen and refreshes live', as const githubInventorySessions: string[] = [] let nativeCredentialVisible = false let installed = false + let holdSlackStart = false + let canceledSlackRequests = 0 + const personalAttempts = new Map() + let personalInventoryFailed = false let javascript = '' let stylesheet = '' let origin = '' @@ -174,9 +178,82 @@ test('source authorization returns to its desktop screen and refreshes live', as const state = generateShortId(32) attempts.set(state, session) startSessions.push(session) + if (holdSlackStart) { + response.on('close', () => { + if (!response.writableEnded) canceledSlackRequests++ + }) + return + } json({ authorizationUrl: `${origin}/provider?state=${state}` }) return } + if (path === '/api/knowledge/sim-search/personal-integrations') { + if (request.method === 'POST') { + const { oauthCompletionId } = await body() + personalAttempts.set(oauthCompletionId, { session, completed: false }) + json({ + success: true, + data: { url: `${origin}/personal-provider?completionId=${oauthCompletionId}` }, + }) + } else if (personalInventoryFailed) { + json({ error: 'Inventory temporarily unavailable' }, 503) + } else { + const attempt = personalAttempts.get(url.searchParams.get('completionId') ?? '') + const connected = attempt?.completed === true + json({ + success: true, + data: { + completedCredentialId: connected ? 'fixture-personal-account' : null, + connections: connected + ? [ + { + name: 'Slack', + providerId: 'slack', + connectorType: 'slack', + description: '', + accounts: [ + { + credentialId: 'fixture-personal-account', + displayName: 'Fixture', + status: 'connected', + action: null, + }, + ], + connectionStatus: 'connected', + action: null, + }, + ] + : [], + available: [ + { + name: 'Slack', + description: '', + target: { + type: 'link', + provider: 'slack', + connectorType: 'slack', + connectionMode: 'live', + optionId: 'fixture-option', + }, + }, + ], + nextCursor: null, + }, + }) + } + return + } + if (path === '/personal-callback') { + const completionId = url.searchParams.get('completionId') ?? '' + const attempt = personalAttempts.get(completionId) + if (!attempt || attempt.session !== session) { + json({ error: 'Wrong attempt' }, 403) + return + } + attempt.completed = true + redirect(`/credential-groups/complete?completionId=${completionId}`) + return + } if (path === '/api/knowledge/slack/oauth/callback') { const state = url.searchParams.get('state') ?? '' callbackSessions.push(session) @@ -298,6 +375,12 @@ test('source authorization returns to its desktop screen and refreshes live', as ) return } + if (path === '/personal-provider') { + response.end( + `Authorize personal Search` + ) + return + } if (path === '/github-provider') { response.end( `Authorize GitHub` @@ -570,6 +653,61 @@ test('source authorization returns to its desktop screen and refreshes live', as await expect(web).toHaveURL(`${origin}/o/fixture-organization/integrations`) await expect(web.getByLabel('Account count')).toHaveText('1') }) + await check('canceling Slack setup aborts the pending web HTTP request', async () => { + holdSlackStart = true + const starts = startSessions.length + try { + await web.getByRole('button', { name: 'Connect Slack', exact: true }).click() + await expect.poll(() => startSessions.length).toBe(starts + 1) + await web.getByRole('button', { name: 'Cancel Slack request', exact: true }).click() + await expect.poll(() => canceledSlackRequests).toBe(1) + await expect(web.getByLabel('Connection', { exact: true })).toHaveText('error') + } finally { + holdSlackStart = false + } + }) + await check( + 'desktop Search preserves pending receipts after inventory failure and allows cancellation/retry', + async () => { + const previousOpens = (await opened()).length + await page.getByRole('button', { name: 'Connect personal Search', exact: true }).click() + await expect.poll(async () => (await opened()).length).toBe(previousOpens + 1) + await external.goto((await opened())[previousOpens]) + await external.getByRole('link', { name: 'Authorize personal Search' }).waitFor() + personalInventoryFailed = true + await external.getByRole('link', { name: 'Authorize personal Search' }).click() + await expect(external).toHaveURL(`${origin}/desktop/done?kind=connect`) + await expect(page.getByLabel('Personal Search inventory error')).toHaveText( + 'Inventory temporarily unavailable' + ) + await expect( + page.getByRole('button', { name: 'Connect personal Search', exact: true }) + ).toBeEnabled() + const receipt = () => + page.evaluate(() => { + const entry = Object.entries(localStorage).find(([key]) => + key.startsWith('sim.search-connection.') + ) + return entry ? (JSON.parse(entry[1]).completionId as string) : null + }) + const pendingReceipt = await receipt() + expect(pendingReceipt).toBeTruthy() + await page.getByRole('button', { name: 'Connect personal Search', exact: true }).click() + expect(await receipt()).toBe(pendingReceipt) + await expect(page.getByLabel('Personal Search pending')).toHaveText('true') + await page.getByRole('button', { name: 'Cancel personal Search', exact: true }).click() + await expect(page.getByLabel('Personal Search pending')).toHaveText('false') + personalInventoryFailed = false + await page.getByRole('button', { name: 'Retry personal inventory', exact: true }).click() + await page.getByRole('button', { name: 'Connect personal Search', exact: true }).click() + await expect.poll(async () => (await opened()).length).toBe(previousOpens + 2) + expect(await receipt()).not.toBe(pendingReceipt) + await external.goto((await opened())[previousOpens + 1]) + await external.getByRole('link', { name: 'Authorize personal Search' }).click() + await expect(page.getByLabel('Personal Search connected')).toHaveText('true') + await expect(page.getByLabel('Personal Search pending')).toHaveText('false') + } + ) await page.screenshot({ path: test.info().outputPath('source-connect-desktop.png') }) } finally { mkdirSync(dirname(reportPath), { recursive: true }) diff --git a/apps/sim/app/api/credential-groups/slack-managed-users/callback/route.test.ts b/apps/sim/app/api/credential-groups/slack-managed-users/callback/route.test.ts new file mode 100644 index 00000000000..9212f0f743b --- /dev/null +++ b/apps/sim/app/api/credential-groups/slack-managed-users/callback/route.test.ts @@ -0,0 +1,18 @@ +import { authMockFns } from '@sim/testing/mocks/auth.mock' +import { createMockRequest } from '@sim/testing/mocks/request.mock' +import { expect, it } from 'vitest' +import { GET } from '@/app/api/credential-groups/slack-managed-users/callback/route' + +it('preserves sign-in recovery when the managed Slack callback loses its session', async () => { + authMockFns.mockGetSession.mockResolvedValueOnce(null) + const response = await GET( + createMockRequest({ + url: 'http://localhost/api/credential-groups/slack-managed-users/callback?state=fixture-state&code=fixture-code', + }) + ) + expect(response.status).toBe(303) + const location = new URL(response.headers.get('location')!) + expect(location.pathname).toBe('/credential-groups/slack-complete') + expect(location.searchParams.get('state')).toBe('fixture-state') + expect(location.searchParams.get('reason')).toBe('signin_required') +}) diff --git a/apps/sim/app/api/credential-groups/slack-managed-users/callback/route.ts b/apps/sim/app/api/credential-groups/slack-managed-users/callback/route.ts index 06bc2743e34..3299729d20b 100644 --- a/apps/sim/app/api/credential-groups/slack-managed-users/callback/route.ts +++ b/apps/sim/app/api/credential-groups/slack-managed-users/callback/route.ts @@ -41,7 +41,7 @@ export const GET = withRouteHandler(async (request: NextRequest) => { ok: false, message: 'Sign in to Sim to complete this Slack setup.', state: rawState, - reason: 'unauthenticated', + reason: 'signin_required', }) } const parsed = await parseRequest(slackCredentialGroupConfigurationCallbackContract, request, {}) diff --git a/apps/sim/app/api/desktop/source-connect/route.test.ts b/apps/sim/app/api/desktop/source-connect/route.test.ts new file mode 100644 index 00000000000..1f3a404b136 --- /dev/null +++ b/apps/sim/app/api/desktop/source-connect/route.test.ts @@ -0,0 +1,32 @@ +import { authMockFns } from '@sim/testing/mocks/auth.mock' +import { rateLimiterMock, rateLimiterMockFns } from '@sim/testing/mocks/rate-limiter.mock' +import { createMockRequest } from '@sim/testing/mocks/request.mock' +import { expect, it, vi } from 'vitest' + +vi.mock('@/lib/core/rate-limiter', () => rateLimiterMock) + +import { POST } from '@/app/api/desktop/source-connect/route' + +it.each([true, false])( + 'rejects an oversized desktop request before JSON decoding (declared length: %s)', + async (declaredLength) => { + authMockFns.mockGetSession.mockResolvedValueOnce({ + user: { id: 'fixture-user' }, + session: { id: 'fixture-session' }, + }) + rateLimiterMockFns.mockEnforceUserRateLimit.mockResolvedValueOnce(null) + const rawBody = ' '.repeat(64 * 1024 + 1) + const response = await POST( + createMockRequest({ + method: 'POST', + url: 'http://localhost/api/desktop/source-connect', + rawBody, + headers: { + 'content-type': 'application/json', + ...(declaredLength ? { 'content-length': String(rawBody.length) } : {}), + }, + }) + ) + expect(response.status).toBe(413) + } +) diff --git a/apps/sim/app/api/desktop/source-connect/route.ts b/apps/sim/app/api/desktop/source-connect/route.ts index 518b0f4e88f..e44102784b3 100644 --- a/apps/sim/app/api/desktop/source-connect/route.ts +++ b/apps/sim/app/api/desktop/source-connect/route.ts @@ -13,6 +13,7 @@ export const POST = defineInternalJsonRoute({ operation: createDesktopSourceRequest.operation, rateLimit: internalRateLimits.user({ bucketName: 'desktop-source-connect' }), errorPolicy: internalOrchestrationErrorPolicy, + parseOptions: { maxBodyBytes: 64 * 1024 }, mapInput: ({ body }) => ({ requestId: body.requestId, payload: JSON.stringify(body.request) }), useCase: createDesktopSourceRequest, staticResponseHeaders: { 'Cache-Control': 'no-store' }, diff --git a/apps/sim/hooks/queries/slack-search.ts b/apps/sim/hooks/queries/slack-search.ts index 030c529a924..39a02552e3c 100644 --- a/apps/sim/hooks/queries/slack-search.ts +++ b/apps/sim/hooks/queries/slack-search.ts @@ -48,7 +48,7 @@ export function useStartSlackSearchOAuth() { await connectDesktopSource({ kind: 'slack-search', body }, signal) return null } - return requestJson(startSlackSearchOAuthContract, { body }) + return requestJson(startSlackSearchOAuthContract, { body, signal }) }, onSettled: (_data, _error, input) => Promise.all([ diff --git a/apps/sim/hooks/use-search-integration-connection.ts b/apps/sim/hooks/use-search-integration-connection.ts index 63f974d1b8d..ed7ba243072 100644 --- a/apps/sim/hooks/use-search-integration-connection.ts +++ b/apps/sim/hooks/use-search-integration-connection.ts @@ -160,6 +160,7 @@ export function useSearchIntegrationConnection({ return } const desktop = isDesktopApp() + if (desktop && pending) return const tab = desktop ? null : window.open('about:blank', '_blank', 'width=600,height=700') if (!desktop && !tab) { setLocalError('Allow pop-ups for this site to connect your account.') diff --git a/apps/sim/scripts/fixtures/desktop-source-connect.tsx b/apps/sim/scripts/fixtures/desktop-source-connect.tsx index 747e5bcdf94..14f5539dff3 100644 --- a/apps/sim/scripts/fixtures/desktop-source-connect.tsx +++ b/apps/sim/scripts/fixtures/desktop-source-connect.tsx @@ -15,6 +15,7 @@ import { } from '@/hooks/queries/organization-accounts' import { useSlackSearchInstallations, useStartSlackSearchOAuth } from '@/hooks/queries/slack-search' import { useGitHubInstallationSetup } from '@/hooks/use-github-installation-setup' +import { useSearchIntegrationConnection } from '@/hooks/use-search-integration-connection' const NO_CONNECTIONS = new Set() const MEMBERSHIP_KEYS: readonly (readonly string[])[] = [] @@ -32,7 +33,20 @@ function SourceConnectFixture() { organizationId: 'fixture-organization', onConnected: setGithubCredential, }) + const slackAbort = useRef(null) const connection = useStartSlackSearchOAuth() + const personal = useSearchIntegrationConnection({ + organizationId: 'fixture-organization', + userId: 'fixture-user', + controlId: 'fixture-search-card', + target: { + type: 'link', + provider: 'slack', + connectorType: 'slack', + connectionMode: 'live', + optionId: 'fixture-option', + }, + }) const inventory = useSlackSearchInstallations('fixture-organization') return (
@@ -72,17 +86,37 @@ function SourceConnectFixture() { + + + + + {String(personal.pending)} + {personal.inventoryError} + {String(personal.connected)} From f6731c8a8e6dc1bae06b7e6d3a6d71209fef2549 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 30 Sep 2026 18:34:58 -0700 Subject: [PATCH 2/2] fix(integrations): verify persisted connection outcomes --- apps/desktop/e2e/source-connect.spec.ts | 33 ++++++++++++------- .../fixtures/desktop-source-connect.tsx | 3 -- 2 files changed, 21 insertions(+), 15 deletions(-) diff --git a/apps/desktop/e2e/source-connect.spec.ts b/apps/desktop/e2e/source-connect.spec.ts index ba6d9cca2e1..c918a0bcfb7 100644 --- a/apps/desktop/e2e/source-connect.spec.ts +++ b/apps/desktop/e2e/source-connect.spec.ts @@ -58,6 +58,7 @@ test('source authorization returns to its desktop screen and refreshes live', as let canceledSlackRequests = 0 const personalAttempts = new Map() let personalInventoryFailed = false + let personalInventoryFailures = 0 let javascript = '' let stylesheet = '' let origin = '' @@ -196,6 +197,7 @@ test('source authorization returns to its desktop screen and refreshes live', as data: { url: `${origin}/personal-provider?completionId=${oauthCompletionId}` }, }) } else if (personalInventoryFailed) { + personalInventoryFailures++ json({ error: 'Inventory temporarily unavailable' }, 503) } else { const attempt = personalAttempts.get(url.searchParams.get('completionId') ?? '') @@ -661,7 +663,7 @@ test('source authorization returns to its desktop screen and refreshes live', as await expect.poll(() => startSessions.length).toBe(starts + 1) await web.getByRole('button', { name: 'Cancel Slack request', exact: true }).click() await expect.poll(() => canceledSlackRequests).toBe(1) - await expect(web.getByLabel('Connection', { exact: true })).toHaveText('error') + await expect(web.getByRole('button', { name: 'Connect Slack', exact: true })).toBeEnabled() } finally { holdSlackStart = false } @@ -677,9 +679,7 @@ test('source authorization returns to its desktop screen and refreshes live', as personalInventoryFailed = true await external.getByRole('link', { name: 'Authorize personal Search' }).click() await expect(external).toHaveURL(`${origin}/desktop/done?kind=connect`) - await expect(page.getByLabel('Personal Search inventory error')).toHaveText( - 'Inventory temporarily unavailable' - ) + await expect.poll(() => personalInventoryFailures).toBeGreaterThan(0) await expect( page.getByRole('button', { name: 'Connect personal Search', exact: true }) ).toBeEnabled() @@ -688,24 +688,33 @@ test('source authorization returns to its desktop screen and refreshes live', as const entry = Object.entries(localStorage).find(([key]) => key.startsWith('sim.search-connection.') ) - return entry ? (JSON.parse(entry[1]).completionId as string) : null + if (!entry) return null + const attempt: { completionId: string; status: string; credentialId?: string } = + JSON.parse(entry[1]) + return attempt }) const pendingReceipt = await receipt() - expect(pendingReceipt).toBeTruthy() + expect(pendingReceipt).toMatchObject({ status: 'pending' }) await page.getByRole('button', { name: 'Connect personal Search', exact: true }).click() - expect(await receipt()).toBe(pendingReceipt) - await expect(page.getByLabel('Personal Search pending')).toHaveText('true') + expect(await receipt()).toEqual(pendingReceipt) await page.getByRole('button', { name: 'Cancel personal Search', exact: true }).click() - await expect(page.getByLabel('Personal Search pending')).toHaveText('false') + await expect + .poll(receipt) + .toMatchObject({ completionId: pendingReceipt?.completionId, status: 'failed' }) personalInventoryFailed = false await page.getByRole('button', { name: 'Retry personal inventory', exact: true }).click() await page.getByRole('button', { name: 'Connect personal Search', exact: true }).click() await expect.poll(async () => (await opened()).length).toBe(previousOpens + 2) - expect(await receipt()).not.toBe(pendingReceipt) + const retryReceipt = await receipt() + expect(retryReceipt).toMatchObject({ status: 'pending' }) + expect(retryReceipt?.completionId).not.toBe(pendingReceipt?.completionId) await external.goto((await opened())[previousOpens + 1]) await external.getByRole('link', { name: 'Authorize personal Search' }).click() - await expect(page.getByLabel('Personal Search connected')).toHaveText('true') - await expect(page.getByLabel('Personal Search pending')).toHaveText('false') + await expect.poll(receipt).toMatchObject({ + completionId: retryReceipt?.completionId, + status: 'connected', + credentialId: 'fixture-personal-account', + }) } ) await page.screenshot({ path: test.info().outputPath('source-connect-desktop.png') }) diff --git a/apps/sim/scripts/fixtures/desktop-source-connect.tsx b/apps/sim/scripts/fixtures/desktop-source-connect.tsx index 14f5539dff3..0e4dfe86046 100644 --- a/apps/sim/scripts/fixtures/desktop-source-connect.tsx +++ b/apps/sim/scripts/fixtures/desktop-source-connect.tsx @@ -114,9 +114,6 @@ function SourceConnectFixture() { - {String(personal.pending)} - {personal.inventoryError} - {String(personal.connected)}