From b7cdaec5fa1b51dc6b980fcd0f39590d75762971 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 31 Aug 2026 16:21:39 -0700 Subject: [PATCH 1/2] fix(settings): restore recovery and test isolation --- .../hooks/use-workflow-execution.test.tsx | 297 +++++++++--------- .../ee/sso/components/sso-settings.test.tsx | 45 +++ apps/sim/ee/sso/components/sso-settings.tsx | 27 +- 3 files changed, 221 insertions(+), 148 deletions(-) diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/hooks/use-workflow-execution.test.tsx b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/hooks/use-workflow-execution.test.tsx index 4eab4ada664..110e77c5ac3 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/hooks/use-workflow-execution.test.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/hooks/use-workflow-execution.test.tsx @@ -414,151 +414,26 @@ describe('useWorkflowExecution cancellation', () => { }) }) -describe('useWorkflowExecution attachment uploads', () => { - beforeEach(() => { - vi.clearAllMocks() - mockEndScopedExecution.mockReset().mockReturnValue(true) - terminalStoreState._hasHydrated = false - executionStoreState.workflowExecutions.set('workflow-1', idleExecution) - executionStoreState.getWorkflowExecution.mockReturnValue(idleExecution) - executionStoreState.getCurrentExecutionId.mockReturnValue(null) - mockAdoptScopedExecution.mockReturnValue(undefined) - mockLoadExecutionPointer.mockResolvedValue(null) - mockReconnect.mockResolvedValue(undefined) - mockResolveStartCandidates.mockReturnValue([]) - mockSelectBestTrigger.mockReturnValue([]) - vi.stubGlobal('fetch', mockFetch) - mockUploadInternalFileSession.mockRejectedValue( - new Error('Workspace file storage limit exceeded') - ) - mockFetch.mockResolvedValue( - new Response(JSON.stringify({ error: 'Workspace file storage limit exceeded' }), { - status: 413, - headers: { 'Content-Type': 'application/json' }, - }) - ) - mockExecute.mockResolvedValue(undefined) - mockExecuteFromBlock.mockResolvedValue(undefined) - workflowStoreState.edges.length = 0 - }) - - afterEach(() => { - vi.unstubAllGlobals() - }) - - it('does not execute and reports the exact server error when an explicit attachment fails', async () => { - const { result, unmount } = renderWorkflowExecutionHook() - const contextFile = new File(['context'], 'context.txt', { type: 'text/plain' }) - const file = new File(['report'], 'report.pdf', { type: 'application/pdf' }) - let uploadError: unknown - - mockUploadInternalFileSession.mockResolvedValueOnce({ - id: 'attachment-context', - key: 'executions/context.txt', - url: '/uploads/context.txt', - name: contextFile.name, - size: contextFile.size, - type: contextFile.type, - context: 'execution', - }) - - await act(async () => { - try { - await result().handleRunWorkflow({ - input: 'Summarize this report', - conversationId: 'conversation-1', - files: [ - { - name: contextFile.name, - size: contextFile.size, - type: contextFile.type, - file: contextFile, - }, - { - name: file.name, - size: file.size, - type: file.type, - file, - }, - ], - }) - } catch (error) { - uploadError = error - } - }) - - expect(uploadError).toBeInstanceOf(WorkflowAttachmentUploadError) - expect((uploadError as Error).message).toBe( - 'Failed to upload report.pdf: Workspace file storage limit exceeded' - ) - expect(mockExecute).not.toHaveBeenCalled() - - unmount() - }) - - it('returns uploaded metadata without mutating or leaking local input into execution', async () => { - const { result, unmount } = renderWorkflowExecutionHook() - const file = new File(['diagram'], 'diagram.png', { type: 'image/png' }) - const workflowInput = { - input: 'Describe this diagram', - conversationId: 'conversation-1', - files: [ - { - name: file.name, - size: file.size, - type: file.type, - file, - }, - ], - } - let runResult: unknown - - mockUploadInternalFileSession.mockResolvedValueOnce({ - id: 'attachment-diagram', - key: 'execution/diagram.png', - url: '/api/files/serve/execution%2Fdiagram.png', - name: file.name, - size: file.size, - type: file.type, - context: 'execution', - }) - - await act(async () => { - runResult = await result().handleRunWorkflow(workflowInput) - await drainStream(runResult) - }) - - expect(isChatWorkflowRunResult(runResult)).toBe(true) - if (!isChatWorkflowRunResult(runResult)) { - throw new Error('Expected a chat workflow run result') - } - expect(runResult.uploadedAttachments).toEqual([ - expect.objectContaining({ - name: 'diagram.png', - url: '/api/files/serve/execution%2Fdiagram.png', - size: file.size, - type: 'image/png', - key: 'execution/diagram.png', - }), - ]) - expect(workflowInput.files[0].file).toBe(file) - expect(mockExecute).toHaveBeenCalledWith( - expect.objectContaining({ - input: expect.objectContaining({ - input: 'Describe this diagram', - conversationId: 'conversation-1', - files: [ - expect.objectContaining({ - name: 'diagram.png', - url: '/api/files/serve/execution%2Fdiagram.png', - }), - ], - }), - }) - ) +function resetWorkflowExecutionTestState() { + vi.clearAllMocks() + mockBeginScopedExecution.mockReset().mockReturnValue({}) + mockAdoptScopedExecution.mockReset().mockReturnValue(undefined) + mockEndScopedExecution.mockReset().mockReturnValue(true) + mockLoadExecutionPointer.mockReset().mockResolvedValue(null) + mockReconnect.mockReset().mockResolvedValue(undefined) + mockResolveStartCandidates.mockReset().mockReturnValue([]) + mockSelectBestTrigger.mockReset().mockReturnValue([]) + mockExecute.mockReset().mockResolvedValue(undefined) + mockExecuteFromBlock.mockReset().mockResolvedValue(undefined) + terminalStoreState._hasHydrated = false + executionStoreState.workflowExecutions.set('workflow-1', idleExecution) + executionStoreState.getWorkflowExecution.mockReturnValue(idleExecution) + executionStoreState.getCurrentExecutionId.mockReturnValue(null) + workflowStoreState.edges.length = 0 +} - unmount() - }) +describe('useWorkflowExecution lifecycle ownership', () => { + beforeEach(resetWorkflowExecutionTestState) it('does not let an overlapping run without lifecycle ownership end the active run', async () => { const persistenceExecution = {} @@ -776,6 +651,140 @@ describe('useWorkflowExecution attachment uploads', () => { unmount() }) +}) + +describe('useWorkflowExecution attachment uploads', () => { + beforeEach(() => { + resetWorkflowExecutionTestState() + vi.stubGlobal('fetch', mockFetch) + mockUploadInternalFileSession.mockRejectedValue( + new Error('Workspace file storage limit exceeded') + ) + mockFetch.mockResolvedValue( + new Response(JSON.stringify({ error: 'Workspace file storage limit exceeded' }), { + status: 413, + headers: { 'Content-Type': 'application/json' }, + }) + ) + }) + + afterEach(() => { + vi.unstubAllGlobals() + }) + + it('does not execute and reports the exact server error when an explicit attachment fails', async () => { + const { result, unmount } = renderWorkflowExecutionHook() + const contextFile = new File(['context'], 'context.txt', { type: 'text/plain' }) + const file = new File(['report'], 'report.pdf', { type: 'application/pdf' }) + let uploadError: unknown + + mockUploadInternalFileSession.mockResolvedValueOnce({ + id: 'attachment-context', + key: 'executions/context.txt', + url: '/uploads/context.txt', + name: contextFile.name, + size: contextFile.size, + type: contextFile.type, + context: 'execution', + }) + + await act(async () => { + try { + await result().handleRunWorkflow({ + input: 'Summarize this report', + conversationId: 'conversation-1', + files: [ + { + name: contextFile.name, + size: contextFile.size, + type: contextFile.type, + file: contextFile, + }, + { + name: file.name, + size: file.size, + type: file.type, + file, + }, + ], + }) + } catch (error) { + uploadError = error + } + }) + + expect(uploadError).toBeInstanceOf(WorkflowAttachmentUploadError) + expect((uploadError as Error).message).toBe( + 'Failed to upload report.pdf: Workspace file storage limit exceeded' + ) + expect(mockExecute).not.toHaveBeenCalled() + + unmount() + }) + + it('returns uploaded metadata without mutating or leaking local input into execution', async () => { + const { result, unmount } = renderWorkflowExecutionHook() + const file = new File(['diagram'], 'diagram.png', { type: 'image/png' }) + const workflowInput = { + input: 'Describe this diagram', + conversationId: 'conversation-1', + files: [ + { + name: file.name, + size: file.size, + type: file.type, + file, + }, + ], + } + let runResult: unknown + + mockUploadInternalFileSession.mockResolvedValueOnce({ + id: 'attachment-diagram', + key: 'execution/diagram.png', + url: '/api/files/serve/execution%2Fdiagram.png', + name: file.name, + size: file.size, + type: file.type, + context: 'execution', + }) + + await act(async () => { + runResult = await result().handleRunWorkflow(workflowInput) + await drainStream(runResult) + }) + + expect(isChatWorkflowRunResult(runResult)).toBe(true) + if (!isChatWorkflowRunResult(runResult)) { + throw new Error('Expected a chat workflow run result') + } + expect(runResult.uploadedAttachments).toEqual([ + expect.objectContaining({ + name: 'diagram.png', + url: '/api/files/serve/execution%2Fdiagram.png', + size: file.size, + type: 'image/png', + key: 'execution/diagram.png', + }), + ]) + expect(workflowInput.files[0].file).toBe(file) + expect(mockExecute).toHaveBeenCalledWith( + expect.objectContaining({ + input: expect.objectContaining({ + input: 'Describe this diagram', + conversationId: 'conversation-1', + files: [ + expect.objectContaining({ + name: 'diagram.png', + url: '/api/files/serve/execution%2Fdiagram.png', + }), + ], + }), + }) + ) + + unmount() + }) it('uses only projected live thinking without changing normal settle behavior', async () => { mockExecute.mockImplementationOnce(async (options) => { diff --git a/apps/sim/ee/sso/components/sso-settings.test.tsx b/apps/sim/ee/sso/components/sso-settings.test.tsx index 259c0dd6d66..fa978f82c54 100644 --- a/apps/sim/ee/sso/components/sso-settings.test.tsx +++ b/apps/sim/ee/sso/components/sso-settings.test.tsx @@ -3,6 +3,7 @@ */ import { act, type ChangeEventHandler, type ReactNode } from 'react' import { resetEnvFlagsMock, setEnvFlags } from '@sim/testing' +import { getErrorMessage } from '@sim/utils/errors' import { createRoot, type Root } from 'react-dom/client' import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest' @@ -98,6 +99,24 @@ vi.mock('@/components/settings/save-discard-actions', () => ({ vi.mock('@/app/workspace/[workspaceId]/settings/components/settings-empty-state', () => ({ SettingsEmptyState: ({ children }: { children?: ReactNode }) =>
{children}
, + SettingsQueryErrorState: ({ + error, + fallback, + isRetrying, + onRetry, + }: { + error: unknown + fallback: string + isRetrying: boolean + onRetry: () => void + }) => ( +
+ {getErrorMessage(error, fallback)} + +
+ ), })) vi.mock('@/app/workspace/[workspaceId]/settings/components/settings-panel', () => ({ @@ -198,7 +217,9 @@ beforeEach(() => { mockUseOrganizationBilling.mockReturnValue({ data: { data: { subscriptionPlan: 'enterprise' } }, error: null, + isFetching: false, isLoading: false, + refetch: vi.fn(), }) mockUseConfigureSSO.mockReturnValue({ isPending: false, @@ -207,7 +228,9 @@ beforeEach(() => { mockUseSSOProviders.mockImplementation(({ organizationId }: { organizationId: string }) => ({ data: { providers: [provider(organizationId)] }, error: null, + isFetching: false, isLoading: false, + refetch: vi.fn(), })) }) @@ -234,16 +257,38 @@ describe('SSO organization transitions', () => { }) it('shows a billing failure instead of an Enterprise upsell', () => { + const refetch = vi.fn() mockUseOrganizationBilling.mockReturnValue({ data: undefined, error: new Error('Billing entitlement failed'), + isFetching: false, isLoading: false, + refetch, }) renderSso('org-a') expect(container).toHaveTextContent('Billing entitlement failed') expect(container).not.toHaveTextContent('available on Enterprise plans only') + act(() => findButton('Try again')?.click()) + expect(refetch).toHaveBeenCalledOnce() + }) + + it('retries an initial provider failure without leaving the page', () => { + const refetch = vi.fn() + mockUseSSOProviders.mockReturnValue({ + data: undefined, + error: new Error('Provider lookup failed'), + isFetching: false, + isLoading: false, + refetch, + }) + + renderSso('org-a') + + expect(container).toHaveTextContent('Provider lookup failed') + act(() => findButton('Try again')?.click()) + expect(refetch).toHaveBeenCalledOnce() }) }) diff --git a/apps/sim/ee/sso/components/sso-settings.tsx b/apps/sim/ee/sso/components/sso-settings.tsx index 11c21257c63..9c28f955ff3 100644 --- a/apps/sim/ee/sso/components/sso-settings.tsx +++ b/apps/sim/ee/sso/components/sso-settings.tsx @@ -27,7 +27,10 @@ import { isEnterprise } from '@/lib/billing/plan-helpers' import { isBillingEnabled } from '@/lib/core/config/env-flags' import { REDACTED_MARKER } from '@/lib/core/security/redaction' import { getBaseUrl } from '@/lib/core/utils/urls' -import { SettingsEmptyState } from '@/app/workspace/[workspaceId]/settings/components/settings-empty-state' +import { + SettingsEmptyState, + SettingsQueryErrorState, +} from '@/app/workspace/[workspaceId]/settings/components/settings-empty-state' import { SettingsPanel } from '@/app/workspace/[workspaceId]/settings/components/settings-panel' import { SettingsSection } from '@/app/workspace/[workspaceId]/settings/components/settings-section/settings-section' import { useSettingsUnsavedGuard } from '@/app/workspace/[workspaceId]/settings/hooks/use-settings-unsaved-guard' @@ -233,13 +236,17 @@ function OrganizationSsoSettings({ organizationId }: SSOProps) { const { data: organizationBillingData, isLoading: isLoadingOrganizationBilling, + isFetching: isFetchingOrganizationBilling, error: organizationBillingError, + refetch: refetchOrganizationBilling, } = useOrganizationBilling(organizationId) const { data: providersData, isLoading: isLoadingProviders, + isFetching: isFetchingProviders, error: providersError, + refetch: refetchProviders, } = useSSOProviders({ organizationId }) const providers = providersData?.providers || [] @@ -291,9 +298,21 @@ function OrganizationSsoSettings({ organizationId }: SSOProps) { (isBillingEnabled && organizationBillingData === undefined ? organizationBillingError : null) if (loadingError) { return ( - - {getErrorMessage(loadingError, 'Failed to load Single Sign-On settings')} - + { + if (providersData === undefined && providersError) void refetchProviders() + if ( + isBillingEnabled && + organizationBillingData === undefined && + organizationBillingError + ) { + void refetchOrganizationBilling() + } + }} + /> ) } From 726a7b7f1a4ddc936c85c5bb68cdf51d42e5ee5d Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 31 Aug 2026 16:29:44 -0700 Subject: [PATCH 2/2] fix(settings): scope SSO retry state to failed queries --- .../ee/sso/components/sso-settings.test.tsx | 20 +++++++++++++++++ apps/sim/ee/sso/components/sso-settings.tsx | 22 +++++++++---------- 2 files changed, 30 insertions(+), 12 deletions(-) diff --git a/apps/sim/ee/sso/components/sso-settings.test.tsx b/apps/sim/ee/sso/components/sso-settings.test.tsx index fa978f82c54..bf290898ac1 100644 --- a/apps/sim/ee/sso/components/sso-settings.test.tsx +++ b/apps/sim/ee/sso/components/sso-settings.test.tsx @@ -258,6 +258,14 @@ describe('SSO organization transitions', () => { it('shows a billing failure instead of an Enterprise upsell', () => { const refetch = vi.fn() + const refetchProviders = vi.fn() + mockUseSSOProviders.mockReturnValue({ + data: { providers: [provider('org-a')] }, + error: null, + isFetching: true, + isLoading: false, + refetch: refetchProviders, + }) mockUseOrganizationBilling.mockReturnValue({ data: undefined, error: new Error('Billing entitlement failed'), @@ -270,12 +278,22 @@ describe('SSO organization transitions', () => { expect(container).toHaveTextContent('Billing entitlement failed') expect(container).not.toHaveTextContent('available on Enterprise plans only') + expect(findButton('Try again')).not.toBeDisabled() act(() => findButton('Try again')?.click()) expect(refetch).toHaveBeenCalledOnce() + expect(refetchProviders).not.toHaveBeenCalled() }) it('retries an initial provider failure without leaving the page', () => { const refetch = vi.fn() + const refetchBilling = vi.fn() + mockUseOrganizationBilling.mockReturnValue({ + data: { data: { subscriptionPlan: 'enterprise' } }, + error: null, + isFetching: true, + isLoading: false, + refetch: refetchBilling, + }) mockUseSSOProviders.mockReturnValue({ data: undefined, error: new Error('Provider lookup failed'), @@ -287,8 +305,10 @@ describe('SSO organization transitions', () => { renderSso('org-a') expect(container).toHaveTextContent('Provider lookup failed') + expect(findButton('Try again')).not.toBeDisabled() act(() => findButton('Try again')?.click()) expect(refetch).toHaveBeenCalledOnce() + expect(refetchBilling).not.toHaveBeenCalled() }) }) diff --git a/apps/sim/ee/sso/components/sso-settings.tsx b/apps/sim/ee/sso/components/sso-settings.tsx index 9c28f955ff3..cc171a75f90 100644 --- a/apps/sim/ee/sso/components/sso-settings.tsx +++ b/apps/sim/ee/sso/components/sso-settings.tsx @@ -293,24 +293,22 @@ function OrganizationSsoSettings({ organizationId }: SSOProps) { return null } - const loadingError = - (providersData === undefined ? providersError : null) ?? - (isBillingEnabled && organizationBillingData === undefined ? organizationBillingError : null) + const providersLoadingError = providersData === undefined ? providersError : null + const organizationBillingLoadingError = + isBillingEnabled && organizationBillingData === undefined ? organizationBillingError : null + const loadingError = providersLoadingError ?? organizationBillingLoadingError if (loadingError) { return ( { - if (providersData === undefined && providersError) void refetchProviders() - if ( - isBillingEnabled && - organizationBillingData === undefined && - organizationBillingError - ) { - void refetchOrganizationBilling() - } + if (providersLoadingError) void refetchProviders() + if (organizationBillingLoadingError) void refetchOrganizationBilling() }} /> )