diff --git a/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/connector-selector-field/connector-selector-field.tsx b/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/connector-selector-field/connector-selector-field.tsx index 6cdd14d27ef..4252b6ec357 100644 --- a/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/connector-selector-field/connector-selector-field.tsx +++ b/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/connector-selector-field/connector-selector-field.tsx @@ -5,11 +5,11 @@ import { ChipCombobox, type ComboboxOption } from '@sim/emcn' import { Loader } from '@sim/emcn/icons' import { SEARCH_DEBOUNCE_MS } from '@/lib/url-state' import { SELECTOR_CONTEXT_FIELDS } from '@/lib/workflows/subblocks/context' +import { getDependsOnFields } from '@/lib/workflows/subblocks/dependencies' import type { ConfigFieldMap, ConfigFieldValue, } from '@/app/workspace/[workspaceId]/knowledge/[id]/hooks/use-connector-config-fields' -import { getDependsOnFields } from '@/blocks/utils' import type { ConnectorConfigField } from '@/connectors/types' import { getSelectorDefinition } from '@/hooks/selectors/registry' import type { SelectorContext, SelectorKey } from '@/hooks/selectors/types' diff --git a/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/hooks/use-connector-config-fields.ts b/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/hooks/use-connector-config-fields.ts index 955891811d2..69723548992 100644 --- a/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/hooks/use-connector-config-fields.ts +++ b/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/hooks/use-connector-config-fields.ts @@ -1,7 +1,7 @@ 'use client' import { useCallback, useMemo, useState } from 'react' -import { getDependsOnFields } from '@/blocks/utils' +import { getDependsOnFields } from '@/lib/workflows/subblocks/dependencies' import type { ConnectorConfigField, ConnectorMeta } from '@/connectors/types' export type ConfigFieldValue = string | string[] diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/combobox/combobox.tsx b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/combobox/combobox.tsx index bea443e2f7d..f8ebccf4356 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/combobox/combobox.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/combobox/combobox.tsx @@ -2,6 +2,7 @@ import { memo, useCallback, useEffect, useMemo, useRef, useState } from 'react' import { Combobox, type ComboboxOption, cn } from '@sim/emcn' import { Plus } from '@sim/emcn/icons' import { useReactFlow } from 'reactflow' +import { getDependsOnFields } from '@/lib/workflows/subblocks/dependencies' import { SandboxCreateModal } from '@/app/workspace/[workspaceId]/settings/components/sandboxes/components/sandbox-create-modal' import type { SandboxLanguage } from '@/app/workspace/[workspaceId]/settings/components/sandboxes/utils' import { shouldClearMissingOption } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/combobox/missing-option' @@ -13,7 +14,6 @@ import { useSubBlockValue } from '@/app/workspace/[workspaceId]/w/[workflowId]/c import { useActiveSearchTarget } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/providers/active-search-target-provider' import { useAccessibleReferencePrefixes } from '@/app/workspace/[workspaceId]/w/[workflowId]/hooks/use-accessible-reference-prefixes' import type { SubBlockConfig } from '@/blocks/types' -import { getDependsOnFields } from '@/blocks/utils' import { usePermissionConfig } from '@/hooks/use-permission-config' import { useSubBlockStore } from '@/stores/workflows/subblock/store' diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/dropdown/dropdown.tsx b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/dropdown/dropdown.tsx index 996fbf7f3f4..b14dfc42c4e 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/dropdown/dropdown.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/dropdown/dropdown.tsx @@ -6,6 +6,7 @@ import { NO_DENIED_OPERATIONS, OPERATION_SUBBLOCK_ID, } from '@/lib/permission-groups/operation-access' +import { getDependsOnFields } from '@/lib/workflows/subblocks/dependencies' import { formatDisplayText } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/formatted-text' import { getWorkflowSearchLabelHighlight } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/workflow-search-highlight' import { useFetchedOptions } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/hooks/use-fetched-options' @@ -13,7 +14,6 @@ import { useSubBlockValue } from '@/app/workspace/[workspaceId]/w/[workflowId]/c import { useActiveSearchTarget } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/providers/active-search-target-provider' import { getBlock } from '@/blocks/registry' import type { SubBlockConfig } from '@/blocks/types' -import { getDependsOnFields } from '@/blocks/utils' import { ResponseBlockHandler } from '@/executor/handlers/response/response-handler' import { useOperationAccess } from '@/hooks/use-operation-access' import { useWorkflowStore } from '@/stores/workflows/workflow/store' diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/param-dependents.ts b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/param-dependents.ts index a860066f290..77e44462cf0 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/param-dependents.ts +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/param-dependents.ts @@ -1,10 +1,10 @@ -import { getWorkflowSearchDependentClears } from '@/lib/workflows/search-replace/dependencies' +import { getTransitiveSubBlockDependents } from '@/lib/workflows/subblocks/dependencies' import { getBlock } from '@/blocks/registry' /** * Clear every TRANSITIVE `dependsOn` descendant of `changedParamId` in a nested tool's params, * mirroring the top-level block clear (`use-collaborative-workflow`). Reuses the shared - * {@link getWorkflowSearchDependentClears} walk - transitive BFS plus canonical-pair expansion, so a + * {@link getTransitiveSubBlockDependents} walk - transitive BFS plus canonical-pair expansion, so a * basic OR advanced member change clears the dependent - so both surfaces clear identically. Only * descendants that currently hold a non-empty value are reset to `''`; the changed param itself and * non-descendants are untouched. Returns the same reference when nothing changed. @@ -16,7 +16,7 @@ export function clearDependentToolParams( ): Record { const subBlocks = getBlock(toolType)?.subBlocks ?? [] let next: Record | null = null - for (const { subBlockId } of getWorkflowSearchDependentClears(subBlocks, changedParamId)) { + for (const { subBlockId } of getTransitiveSubBlockDependents(subBlocks, [changedParamId])) { if (!params[subBlockId]) continue next ??= { ...params } next[subBlockId] = '' diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/search-replace/workflow-search-replace.tsx b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/search-replace/workflow-search-replace.tsx index be31940d6ab..90d773f7933 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/search-replace/workflow-search-replace.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/search-replace/workflow-search-replace.tsx @@ -5,7 +5,6 @@ import { Button, cn, Input, toast } from '@sim/emcn' import { ChevronDown, ChevronRight, ChevronUp, X } from '@sim/emcn/icons' import { useParams } from 'next/navigation' import { useShallow } from 'zustand/react/shallow' -import { getWorkflowSearchDependentClears } from '@/lib/workflows/search-replace/dependencies' import { indexWorkflowSearchMatches } from '@/lib/workflows/search-replace/indexer' import { buildWorkflowSearchReplacePlan } from '@/lib/workflows/search-replace/replacements' import { @@ -20,6 +19,7 @@ import { import { getWorkflowSearchBlocks } from '@/lib/workflows/search-replace/state' import { WORKFLOW_SEARCH_SUBFLOW_FIELD_IDS } from '@/lib/workflows/search-replace/subflow-fields' import type { WorkflowSearchReplaceSubflowUpdate } from '@/lib/workflows/search-replace/types' +import { getTransitiveSubBlockDependents } from '@/lib/workflows/subblocks/dependencies' import { useRegisterGlobalCommands } from '@/app/workspace/[workspaceId]/providers/global-commands-provider' import { useUserPermissionsContext } from '@/app/workspace/[workspaceId]/providers/workspace-permissions-provider' import { createCommand } from '@/app/workspace/[workspaceId]/utils/commands-utils' @@ -474,10 +474,9 @@ export function WorkflowSearchReplace() { const blockConfig = block ? getBlock(block.type) : null if (!blockConfig?.subBlocks) continue - const dependentClears = getWorkflowSearchDependentClears( - blockConfig.subBlocks, - update.subBlockId - ) + const dependentClears = getTransitiveSubBlockDependents(blockConfig.subBlocks, [ + update.subBlockId, + ]) for (const clear of dependentClears) { const alreadyUpdated = batchUpdates.some( (candidate) => diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/workflow-block/workflow-block.tsx b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/workflow-block/workflow-block.tsx index 3babe49ba7a..bc9714ea200 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/workflow-block/workflow-block.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/workflow-block/workflow-block.tsx @@ -50,6 +50,7 @@ import { import { resolveSelectedTriggerId } from '@/lib/workflows/blocks/canvas-trigger-sentence' import { calculateWorkflowBlockDimensions } from '@/lib/workflows/blocks/deterministic-dimensions' import { getConditionRows, getRouterRows } from '@/lib/workflows/dynamic-handle-topology' +import { getDependsOnFields } from '@/lib/workflows/subblocks/dependencies' import { getDisplayValue, hasDisplayableRowValue, @@ -96,7 +97,6 @@ import { SELECTOR_TYPES_HYDRATION_REQUIRED, type SubBlockConfig, } from '@/blocks/types' -import { getDependsOnFields } from '@/blocks/utils' import { useKnowledgeBase } from '@/hooks/kb/use-knowledge' import { useCustomTools } from '@/hooks/queries/custom-tools' import { useDeployWorkflow } from '@/hooks/queries/deployments' diff --git a/apps/sim/blocks/utils.test.ts b/apps/sim/blocks/utils.test.ts index fd71606f4ae..1cf444caa07 100644 --- a/apps/sim/blocks/utils.test.ts +++ b/apps/sim/blocks/utils.test.ts @@ -67,13 +67,10 @@ vi.mock('@/lib/oauth/utils', () => ({ getScopesForService: vi.fn(() => []), })) -import type { SubBlockConfig } from '@/blocks/types' import { BUILT_IN_TOOL_TYPES, getApiKeyCondition, - getDependsOnFields, getSerializedModelProviderId, - getSubBlocksDependingOnChange, parseOptionalBooleanInput, parseOptionalJsonInput, parseOptionalNumberInput, @@ -377,96 +374,6 @@ describe('parseOptionalBooleanInput', () => { }) }) -describe('getDependsOnFields', () => { - it('returns an empty array when dependsOn is unset', () => { - expect(getDependsOnFields(undefined)).toEqual([]) - }) - - it('returns array dependencies unchanged', () => { - expect(getDependsOnFields(['credential', 'projectId'])).toEqual(['credential', 'projectId']) - }) - - it('flattens all and any dependencies', () => { - expect(getDependsOnFields({ all: ['credential'], any: ['teamId', 'manualTeamId'] })).toEqual([ - 'credential', - 'teamId', - 'manualTeamId', - ]) - }) -}) - -describe('getSubBlocksDependingOnChange', () => { - it('finds direct dependents of a changed subblock', () => { - const subBlocks: SubBlockConfig[] = [ - { id: 'provider', title: 'Provider', type: 'dropdown' }, - { id: 'model', title: 'Model', type: 'dropdown', dependsOn: ['provider'] }, - { id: 'prompt', title: 'Prompt', type: 'long-input' }, - ] - - expect( - getSubBlocksDependingOnChange(subBlocks, 'provider').map((subBlock) => subBlock.id) - ).toEqual(['model']) - }) - - it('matches dependents through canonical basic and advanced siblings', () => { - const subBlocks: SubBlockConfig[] = [ - { - id: 'channel', - title: 'Channel', - type: 'channel-selector', - canonicalParamId: 'channelId', - mode: 'basic', - }, - { - id: 'manualChannel', - title: 'Channel ID', - type: 'short-input', - canonicalParamId: 'channelId', - mode: 'advanced', - }, - { - id: 'messageId', - title: 'Message ID', - type: 'short-input', - dependsOn: ['channelId'], - }, - { - id: 'threadTs', - title: 'Thread Timestamp', - type: 'short-input', - dependsOn: ['otherField'], - }, - ] - - expect( - getSubBlocksDependingOnChange(subBlocks, 'manualChannel').map((subBlock) => subBlock.id) - ).toEqual(['messageId']) - expect( - getSubBlocksDependingOnChange(subBlocks, 'channel').map((subBlock) => subBlock.id) - ).toEqual(['messageId']) - }) - - it('matches object-form dependencies when any listed dependency changes', () => { - const subBlocks: SubBlockConfig[] = [ - { id: 'credential', title: 'Credential', type: 'oauth-input' }, - { id: 'teamId', title: 'Team', type: 'short-input' }, - { - id: 'projectId', - title: 'Project', - type: 'short-input', - dependsOn: { all: ['credential'], any: ['teamId'] }, - }, - ] - - expect( - getSubBlocksDependingOnChange(subBlocks, 'credential').map((subBlock) => subBlock.id) - ).toEqual(['projectId']) - expect( - getSubBlocksDependingOnChange(subBlocks, 'teamId').map((subBlock) => subBlock.id) - ).toEqual(['projectId']) - }) -}) - describe('getSerializedModelProviderId', () => { const resolver = vi.mocked(getProviderFromModel) diff --git a/apps/sim/blocks/utils.ts b/apps/sim/blocks/utils.ts index 909e42a01f4..60e4efad52f 100644 --- a/apps/sim/blocks/utils.ts +++ b/apps/sim/blocks/utils.ts @@ -8,7 +8,6 @@ import { } from '@/lib/core/config/env-flags' import { getScopesForService } from '@/lib/oauth/utils' import { containsReference } from '@/lib/workflows/sanitization/references' -import { buildCanonicalIndex } from '@/lib/workflows/subblocks/visibility' import type { SubBlockConfig } from '@/blocks/types' import { getBaseModelProviders, @@ -112,39 +111,6 @@ export function getPiModelOptions() { }) } -/** - * Gets all dependency fields as a flat array. - * Handles both simple array format and object format with all/any fields. - */ -export function getDependsOnFields(dependsOn: SubBlockConfig['dependsOn']): string[] { - if (!dependsOn) return [] - if (Array.isArray(dependsOn)) return dependsOn - return [...(dependsOn.all || []), ...(dependsOn.any || [])] -} - -/** - * Finds subblocks that depend on a changed field, accounting for canonical pairs. - */ -export function getSubBlocksDependingOnChange( - allSubBlocks: SubBlockConfig[], - changedSubBlockId: string -): SubBlockConfig[] { - const canonicalIndex = buildCanonicalIndex(allSubBlocks) - const canonicalId = canonicalIndex.canonicalIdBySubBlockId[changedSubBlockId] - const group = canonicalId ? canonicalIndex.groupsById[canonicalId] : undefined - const changedFields = new Set([changedSubBlockId]) - - if (canonicalId) changedFields.add(canonicalId) - if (group?.basicId) changedFields.add(group.basicId) - for (const advancedId of group?.advancedIds || []) { - changedFields.add(advancedId) - } - - return allSubBlocks.filter((subBlock) => - getDependsOnFields(subBlock.dependsOn).some((field) => changedFields.has(field)) - ) -} - function getProviderFromStore(model: string): string | null { const { providers } = useProvidersStore.getState() const normalized = model.toLowerCase() diff --git a/apps/sim/ee/workspace-forking/components/fork-sync/dependent-value.test.ts b/apps/sim/ee/workspace-forking/components/fork-sync/dependent-value.test.ts index 20a1c079ab9..077175f1bd6 100644 --- a/apps/sim/ee/workspace-forking/components/fork-sync/dependent-value.test.ts +++ b/apps/sim/ee/workspace-forking/components/fork-sync/dependent-value.test.ts @@ -11,6 +11,7 @@ import { getActionableDependentFields, getDisplayedDependentFields, isDependentConfigurationActionable, + isDependentInvalidated, } from '@/ee/workspace-forking/components/fork-sync/dependent-value' const field = (overrides: Partial = {}): ForkDependentReconfig => ({ @@ -31,6 +32,11 @@ const field = (overrides: Partial = {}): ForkDependentRec ...overrides, }) +const mappedRepickContext = (previousValue: string) => ({ + previousValue, + baselineValueFor: (dependent: ForkDependentReconfig) => dependent.currentValue, +}) + describe('dependentKey', () => { it('keys by target workflow + block + subblock', () => { expect( @@ -138,20 +144,45 @@ describe('applyDependentRepick', () => { previous, site, [site, drive, spreadsheet, sheet, unrelated], - 'site-new' + 'site-new', + mappedRepickContext('site-old') ) expect(next).toEqual({ [dependentKey(site)]: 'site-new', - [dependentKey(drive)]: '', - [dependentKey(spreadsheet)]: '', - [dependentKey(sheet)]: '', + [dependentKey(drive)]: null, + [dependentKey(spreadsheet)]: null, + [dependentKey(sheet)]: null, [dependentKey(unrelated)]: 'still-keep-me', }) expect(effectiveDependentValue(drive, next, false)).toBe('') expect(effectiveCopyDependentValue(sheet, next)).toBe('') }) + it('re-picking the value the field already had leaves its descendants alone', () => { + const spreadsheet = field({ + subBlockKey: 'spreadsheetId', + currentValue: 'sheet-doc', + providesContextKey: 'spreadsheetId', + }) + const range = field({ + subBlockKey: 'range', + currentValue: 'Sheet1!A1:D', + consumesContextKeys: ['spreadsheetId'], + }) + + const next = applyDependentRepick( + {}, + spreadsheet, + [spreadsheet, range], + 'sheet-doc', + mappedRepickContext(effectiveDependentValue(spreadsheet, {}, false)) + ) + + expect(next).toEqual({}) + expect(effectiveDependentValue(range, next, false)).toBe('Sheet1!A1:D') + }) + it('only changes the selected field when it provides no selector context', () => { const leaf = field({ subBlockKey: 'issueKey', currentValue: 'ISSUE-1' }) const unrelated = field({ subBlockKey: 'label', currentValue: 'keep-me' }) @@ -161,7 +192,8 @@ describe('applyDependentRepick', () => { { [dependentKey(unrelated)]: 'still-keep-me' }, leaf, [leaf, unrelated], - 'ISSUE-2' + 'ISSUE-2', + mappedRepickContext('ISSUE-1') ) ).toEqual({ [dependentKey(leaf)]: 'ISSUE-2', @@ -200,14 +232,106 @@ describe('applyDependentRepick', () => { previous, projectOne, [projectOne, issueOne, projectTwo, issueTwo], - 'P1-NEW' + 'P1-NEW', + mappedRepickContext('INBOX') ) ).toEqual({ [dependentKey(projectOne)]: 'P1-NEW', - [dependentKey(issueOne)]: '', + [dependentKey(issueOne)]: null, [dependentKey(issueTwo)]: 'P2-1', }) }) + + it('restores the stored chain when a provider is changed and then returned to baseline', () => { + const spreadsheet = field({ + subBlockKey: 'spreadsheetId', + currentValue: 'doc-old', + providesContextKey: 'spreadsheetId', + }) + const range = field({ + subBlockKey: 'range', + currentValue: 'A1:D50', + consumesContextKeys: ['spreadsheetId'], + }) + + const changed = applyDependentRepick( + {}, + spreadsheet, + [spreadsheet, range], + 'doc-new', + mappedRepickContext('doc-old') + ) + const restored = applyDependentRepick( + changed, + spreadsheet, + [spreadsheet, range], + 'doc-old', + mappedRepickContext('doc-new') + ) + + expect(changed).toEqual({ + [dependentKey(spreadsheet)]: 'doc-new', + [dependentKey(range)]: null, + }) + expect(restored).toEqual({}) + expect(effectiveDependentValue(range, restored, false)).toBe('A1:D50') + }) + + it('keeps an intentional empty pick distinct from automatic invalidation', () => { + const label = field({ subBlockKey: 'label', currentValue: 'INBOX' }) + + const next = applyDependentRepick({}, label, [label], '', mappedRepickContext('INBOX')) + + expect(isDependentInvalidated(label, next)).toBe(false) + expect(next[dependentKey(label)]).toBe('') + expect(effectiveDependentValue(label, next, false)).toBe('') + }) + + it('does not restore an Excel sheet while its drive still differs from baseline', () => { + const drive = field({ + subBlockKey: 'driveId', + currentValue: 'drive-old', + providesContextKey: 'driveId', + }) + const spreadsheet = field({ + subBlockKey: 'spreadsheetId', + currentValue: 'workbook-old', + consumesContextKeys: ['driveId'], + providesContextKey: 'spreadsheetId', + }) + const sheet = field({ + subBlockKey: 'sheetName', + currentValue: 'Sheet1', + consumesContextKeys: ['driveId', 'spreadsheetId'], + }) + + const driveChanged = applyDependentRepick( + {}, + drive, + [drive, spreadsheet, sheet], + 'drive-new', + mappedRepickContext('drive-old') + ) + const spreadsheetRepicked = applyDependentRepick( + driveChanged, + spreadsheet, + [drive, spreadsheet, sheet], + 'workbook-new', + mappedRepickContext('') + ) + const spreadsheetRestored = applyDependentRepick( + spreadsheetRepicked, + spreadsheet, + [drive, spreadsheet, sheet], + 'workbook-old', + mappedRepickContext('workbook-new') + ) + + expect(spreadsheetRestored).toEqual({ + [dependentKey(drive)]: 'drive-new', + [dependentKey(sheet)]: null, + }) + }) }) describe('isDependentConfigurationActionable', () => { @@ -281,6 +405,51 @@ describe('isDependentConfigurationActionable', () => { ).toBe(true) }) + it('shows a required field that a parent re-pick blanked (it blocks Sync)', () => { + const spreadsheet = field({ + subBlockKey: 'spreadsheetId', + currentValue: 'doc-old', + providesContextKey: 'spreadsheetId', + }) + const sheet = field({ + subBlockKey: 'sheetName', + required: true, + currentValue: 'Sheet1', + consumesContextKeys: ['spreadsheetId'], + }) + const next = applyDependentRepick( + {}, + spreadsheet, + [spreadsheet, sheet], + 'doc-new', + mappedRepickContext(effectiveDependentValue(spreadsheet, {}, false)) + ) + + // The sync gate reads the same blank the selector shows, so the field gates and is visible. + expect(effectiveDependentValue(sheet, next, false)).toBe('') + expect( + isDependentConfigurationActionable(sheet, next, { + parentResolved: true, + parentChanged: false, + copying: false, + }) + ).toBe(true) + }) + + it('shows a required field the user emptied themselves (it blocks Sync)', () => { + const sheet = field({ subBlockKey: 'sheetName', required: true, currentValue: 'Sheet1' }) + const next = applyDependentRepick({}, sheet, [sheet], '', mappedRepickContext('Sheet1')) + + expect(effectiveDependentValue(sheet, next, false)).toBe('') + expect( + isDependentConfigurationActionable(sheet, next, { + parentResolved: true, + parentChanged: false, + copying: false, + }) + ).toBe(true) + }) + it('hides dependents until their parent is resolved', () => { expect( isDependentConfigurationActionable( diff --git a/apps/sim/ee/workspace-forking/components/fork-sync/dependent-value.ts b/apps/sim/ee/workspace-forking/components/fork-sync/dependent-value.ts index 9ae7ab40d84..f23111dc4e3 100644 --- a/apps/sim/ee/workspace-forking/components/fork-sync/dependent-value.ts +++ b/apps/sim/ee/workspace-forking/components/fork-sync/dependent-value.ts @@ -1,5 +1,8 @@ import type { ForkDependentReconfig } from '@/lib/api/contracts/workspace-fork' +/** In-session dependent values. `null` means an upstream selector invalidated the field. */ +export type DependentReconfigState = Record + /** Stable key for a per-target dependent re-pick (target workflow + block + subblock). */ export function dependentKey(dependent: ForkDependentReconfig): string { return `${dependent.targetWorkflowId}:${dependent.targetBlockId}:${dependent.subBlockKey}` @@ -10,22 +13,62 @@ function sameDependencyScope(left: ForkDependentReconfig, right: ForkDependentRe } /** - * Store a dependent re-pick and clear every selector transitively scoped by it. Empty-string - * overrides are intentional: an absent override means "fall back to the stored value", while a - * changed provider makes every stored descendant stale for both mapped and copied parents. + * Index the selector fields that provide each dependency context within a top-level block or + * nested tool instance. The fork diff has already normalized canonical basic/advanced fields + * into these context keys, so this is the same graph the selectors use to resolve their options. + */ +function indexContextProviders( + fields: ForkDependentReconfig[] +): Map> { + const providersByScope = new Map>() + for (const field of fields) { + if (!field.providesContextKey) continue + let providers = providersByScope.get(field.dependencyScope) + if (!providers) { + providers = new Map() + providersByScope.set(field.dependencyScope, providers) + } + providers.set(field.providesContextKey, field) + } + return providersByScope +} + +interface DependentRepickContext { + /** Effective value displayed immediately before this selection. */ + previousValue: string + /** Value each field falls back to when it has no in-session override. */ + baselineValueFor: (field: ForkDependentReconfig) => string +} + +/** + * Apply one selector choice and invalidate its transitive descendants within the same dependency + * scope. Automatic invalidation is represented by `null`, not `''`, so it can be distinguished + * from a user's intentional clear while the editor is open. + * + * Returning a provider to its baseline restores every descendant whose complete provider chain + * has also returned to baseline. A descendant remains invalidated when another one of its + * providers is still changed. This makes A -> B -> A a true undo without reviving a value under + * a genuinely different scope. */ export function applyDependentRepick( - reconfig: Record, + reconfig: DependentReconfigState, changedField: ForkDependentReconfig, blockFields: ForkDependentReconfig[], - value: string -): Record { + value: string, + context: DependentRepickContext +): DependentReconfigState { const changedKey = dependentKey(changedField) - const nextState = { ...reconfig, [changedKey]: value } + const baselineValue = context.baselineValueFor(changedField) + const nextState = { ...reconfig } + if (value === baselineValue) delete nextState[changedKey] + else nextState[changedKey] = value + + if (context.previousValue === value) return nextState if (!changedField.providesContextKey) return nextState const pendingContextKeys = [changedField.providesContextKey] const visitedFields = new Set([changedKey]) + const descendants: ForkDependentReconfig[] = [] for (let index = 0; index < pendingContextKeys.length; index += 1) { const contextKey = pendingContextKeys[index] if (!contextKey) continue @@ -41,26 +84,71 @@ export function applyDependentRepick( } visitedFields.add(fieldKey) - nextState[fieldKey] = '' + descendants.push(field) if (field.providesContextKey) pendingContextKeys.push(field.providesContextKey) } } + for (const field of descendants) nextState[dependentKey(field)] = null + if (value !== baselineValue) return nextState + + const providersByScope = indexContextProviders(blockFields) + const fieldAndProvidersAtBaseline = ( + field: ForkDependentReconfig, + visiting: Set + ): boolean => { + const fieldKey = dependentKey(field) + if (visiting.has(fieldKey)) return false + const fieldValue = nextState[fieldKey] + const effectiveFieldValue = + fieldValue === undefined ? context.baselineValueFor(field) : fieldValue + if (effectiveFieldValue !== context.baselineValueFor(field)) return false + + visiting.add(fieldKey) + const providers = providersByScope.get(field.dependencyScope) + const providerChainAtBaseline = field.consumesContextKeys.every((contextKey) => { + const provider = providers?.get(contextKey) + return !provider || fieldAndProvidersAtBaseline(provider, visiting) + }) + visiting.delete(fieldKey) + return providerChainAtBaseline + } + + for (let pass = 0; pass < descendants.length; pass += 1) { + let restored = false + for (const field of descendants) { + const fieldKey = dependentKey(field) + if (nextState[fieldKey] !== null) continue + const providers = providersByScope.get(field.dependencyScope) + const providerChainAtBaseline = field.consumesContextKeys.every((contextKey) => { + const provider = providers?.get(contextKey) + return !provider || fieldAndProvidersAtBaseline(provider, new Set()) + }) + if (!providerChainAtBaseline) continue + delete nextState[fieldKey] + restored = true + } + if (!restored) break + } + return nextState } /** * The value sent + displayed for a dependent: the user's in-session re-pick if present, else the - * stored value (`currentValue`). Blank when the parent target changed in-session, since the old - * stored value was for the previous parent and won't resolve against the new one. Shared by the - * sync gate + payload build and the per-block selector so the rule can't drift between them. + * stored value (`currentValue`). Blank when the parent target changed in-session, or when an + * in-block parent re-pick invalidated it, since the old stored value was for the previous parent + * and won't resolve against the new one. Shared by the sync gate and the per-block selector so + * the rule cannot drift between them. The payload submits the same effective blank so stale + * values are removed from top-level fields and nested Agent tool parameters alike. */ export function effectiveDependentValue( field: ForkDependentReconfig, - reconfig: Record, + reconfig: DependentReconfigState, parentChanged: boolean ): string { const repicked = reconfig[dependentKey(field)] + if (repicked === null) return '' if (repicked !== undefined) return repicked return parentChanged ? '' : field.currentValue } @@ -75,13 +163,26 @@ export function effectiveDependentValue( */ export function effectiveCopyDependentValue( field: ForkDependentReconfig, - reconfig: Record + reconfig: DependentReconfigState ): string { const repicked = reconfig[dependentKey(field)] + if (repicked === null) return '' if (repicked !== undefined) return repicked return field.currentValue || field.sourceValue } +/** + * Whether an in-block provider change invalidated this field and the user has not re-picked it. + * It reads and submits as empty while the provider remains changed, preventing a stale top-level + * or nested Agent tool value from surviving under the new scope. + */ +export function isDependentInvalidated( + field: ForkDependentReconfig, + reconfig: DependentReconfigState +): boolean { + return reconfig[dependentKey(field)] === null +} + export interface DependentConfigurationState { parentResolved: boolean parentChanged: boolean @@ -92,10 +193,15 @@ export interface DependentConfigurationState { * Whether a dependent selector needs to be shown. A changed or copied parent requires review * because its children resolve in a different scope. An unchanged mapping only needs a selector * when a required value is missing; its stored values are already valid and sync-ready. + * + * A field a parent re-pick invalidated reads as blank through `effectiveDependentValue`, so a + * REQUIRED one stays on screen here and keeps gating Sync. An optional one drops out of the + * default view - `getDisplayedDependentFields` brings it back under explicit edit mode - and + * hiding it is safe because the effective blank is still submitted to prevent a stale value. */ export function isDependentConfigurationActionable( field: ForkDependentReconfig, - reconfig: Record, + reconfig: DependentReconfigState, state: DependentConfigurationState ): boolean { if (!state.parentResolved) return false @@ -110,22 +216,13 @@ export function isDependentConfigurationActionable( */ export function getActionableDependentFields( fields: ForkDependentReconfig[], - reconfig: Record, + reconfig: DependentReconfigState, state: DependentConfigurationState ): ForkDependentReconfig[] { const actionable = new Set( fields.filter((field) => isDependentConfigurationActionable(field, reconfig, state)) ) - const providersByScope = new Map>() - for (const field of fields) { - if (!field.providesContextKey) continue - let providers = providersByScope.get(field.dependencyScope) - if (!providers) { - providers = new Map() - providersByScope.set(field.dependencyScope, providers) - } - providers.set(field.providesContextKey, field) - } + const providersByScope = indexContextProviders(fields) const pending = Array.from(actionable) for (let index = 0; index < pending.length; index += 1) { @@ -149,7 +246,7 @@ export function getActionableDependentFields( */ export function getDisplayedDependentFields( fields: ForkDependentReconfig[], - reconfig: Record, + reconfig: DependentReconfigState, state: DependentConfigurationState, showConfigured: boolean ): ForkDependentReconfig[] { diff --git a/apps/sim/ee/workspace-forking/components/fork-sync/fork-sync-view.tsx b/apps/sim/ee/workspace-forking/components/fork-sync/fork-sync-view.tsx index a0cc46362e0..9a27dbf54eb 100644 --- a/apps/sim/ee/workspace-forking/components/fork-sync/fork-sync-view.tsx +++ b/apps/sim/ee/workspace-forking/components/fork-sync/fork-sync-view.tsx @@ -36,6 +36,7 @@ import { DependentFieldSelector } from '@/ee/workspace-forking/components/fork-s import { applyDependentRepick, type DependentConfigurationState, + type DependentReconfigState, dependentKey, effectiveCopyDependentValue, effectiveDependentValue, @@ -109,7 +110,7 @@ interface WorkflowDependents { function groupDependentsByWorkflow( workflows: ForkResourceUsage['workflows'], dependents: ForkDependentReconfig[], - reconfig: Record, + reconfig: DependentReconfigState, state: DependentConfigurationState, showConfigured: boolean ): WorkflowDependents[] { @@ -181,8 +182,8 @@ interface DependentSelectorProps { copying: boolean workspaceId: string sourceWorkspaceId: string - reconfig: Record - setReconfig: Dispatch>> + reconfig: DependentReconfigState + setReconfig: Dispatch> } /** @@ -204,10 +205,12 @@ function DependentSelector({ reconfig, setReconfig, }: DependentSelectorProps) { - const effectiveValue = (f: ForkDependentReconfig) => + const effectiveValueIn = (f: ForkDependentReconfig, state: DependentReconfigState) => copying - ? effectiveCopyDependentValue(f, reconfig) - : effectiveDependentValue(f, reconfig, parentChanged) + ? effectiveCopyDependentValue(f, state) + : effectiveDependentValue(f, state, parentChanged) + const baselineValueFor = (f: ForkDependentReconfig) => effectiveValueIn(f, {}) + const effectiveValue = (f: ForkDependentReconfig) => effectiveValueIn(f, reconfig) const { providedValues, providedContextKeys } = blockChainState(block, field, effectiveValue) // Disabled until every in-block parent it depends on has a value, so a child never queries // a stale upstream value. @@ -230,7 +233,14 @@ function DependentSelector({ enabled={parentValue !== '' && ready} value={effectiveValue(field)} onChange={(value) => - setReconfig((current) => applyDependentRepick(current, field, block.fields, value)) + setReconfig((current) => + // The pre-pick value comes from the state being updated, so re-selecting the value + // already shown is recognised as the no-op it is and leaves descendants intact. + applyDependentRepick(current, field, block.fields, value, { + previousValue: effectiveValueIn(field, current), + baselineValueFor, + }) + ) } title={field.title} /> @@ -246,8 +256,8 @@ interface DependentWorkflowCardProps { copying: boolean workspaceId: string sourceWorkspaceId: string - reconfig: Record - setReconfig: Dispatch>> + reconfig: DependentReconfigState + setReconfig: Dispatch> } /** diff --git a/apps/sim/ee/workspace-forking/components/fork-sync/use-fork-sync.test.tsx b/apps/sim/ee/workspace-forking/components/fork-sync/use-fork-sync.test.tsx new file mode 100644 index 00000000000..dbf6f10d502 --- /dev/null +++ b/apps/sim/ee/workspace-forking/components/fork-sync/use-fork-sync.test.tsx @@ -0,0 +1,558 @@ +/** + * @vitest-environment jsdom + * + * Coverage for the payload {@link useForkSync} actually submits, not just the helpers it calls. + * The tests distinguish a genuine provider change, which must clear unresolved descendants, + * from a provider undo, which must restore them before Save or Sync derives its payload. + */ +import { act, type ReactNode } from 'react' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { + ForkDependentReconfig, + ForkMappingEntry, + UpdateForkMappingBody, +} from '@/lib/api/contracts/workspace-fork' + +const { + mockUseForkMapping, + mockUseForkDiff, + mockUpdateMutate, + mockUpdateMutateAsync, + mockPromote, +} = vi.hoisted(() => ({ + mockUseForkMapping: vi.fn(), + mockUseForkDiff: vi.fn(), + mockUpdateMutate: vi.fn(), + mockUpdateMutateAsync: vi.fn(), + mockPromote: vi.fn(), +})) + +vi.mock('@sim/emcn', () => ({ + toast: { success: vi.fn(), error: vi.fn(), warning: vi.fn() }, +})) + +vi.mock('@/ee/workspace-forking/hooks/workspace-fork', () => ({ + useForkMapping: mockUseForkMapping, + useForkDiff: mockUseForkDiff, + useUpdateForkMapping: () => ({ + mutate: mockUpdateMutate, + mutateAsync: mockUpdateMutateAsync, + isPending: false, + }), + usePromoteFork: () => ({ mutateAsync: mockPromote }), +})) + +import { + applyDependentRepick, + dependentKey, +} from '@/ee/workspace-forking/components/fork-sync/dependent-value' +import { + type ForkSyncController, + useForkSync, +} from '@/ee/workspace-forking/components/fork-sync/use-fork-sync' + +const WORKSPACE_ID = 'ws-child' +const OTHER_WORKSPACE_ID = 'ws-parent' +const WORKFLOW_ID = 'wf-1' +const BLOCK_ID = 'block-1' + +/** The mapped credential every dependent below hangs off; already mapped, so nothing re-picks it. */ +const CREDENTIAL_ENTRY: ForkMappingEntry = { + kind: 'credential', + resourceType: 'credential', + sourceId: 'cred-src', + sourceLabel: 'Google (source)', + targetId: 'cred-tgt', + suggested: false, + required: true, + sourceDeleted: false, + candidates: [], + candidatesTruncated: false, +} + +function dependent(overrides: Partial): ForkDependentReconfig { + return { + parentKind: 'credential', + parentSourceId: CREDENTIAL_ENTRY.sourceId, + parentContextKey: 'oauthCredential', + targetWorkflowId: WORKFLOW_ID, + targetBlockId: BLOCK_ID, + blockName: 'Google Sheets', + subBlockKey: 'field', + selectorKey: 'sheet', + title: 'Field', + currentValue: '', + sourceValue: '', + required: false, + consumesContextKeys: [], + context: {}, + ...overrides, + } +} + +/** The in-block provider the user re-picks; its change invalidates `CHILD`. */ +const PARENT_FIELD = dependent({ + subBlockKey: 'spreadsheetId', + title: 'Spreadsheet', + currentValue: 'sheet-1', + providesContextKey: 'spreadsheetId', +}) + +/** Optional descendant of `PARENT_FIELD` - the field the P0 used to blank in the target. */ +const CHILD_FIELD = dependent({ + subBlockKey: 'sheetName', + title: 'Sheet', + currentValue: 'Tab A', + consumesContextKeys: ['spreadsheetId'], +}) + +/** Unrelated field the user clears themselves; an intentional clear must still be submitted. */ +const CLEARED_FIELD = dependent({ + subBlockKey: 'folderId', + title: 'Folder', + currentValue: 'folder-9', +}) + +/** Untouched field, submitted with its stored value so the "full stored mapping" contract holds. */ +const UNTOUCHED_FIELD = dependent({ + subBlockKey: 'labelId', + title: 'Label', + currentValue: 'label-3', +}) + +const DEPENDENTS = [PARENT_FIELD, CHILD_FIELD, CLEARED_FIELD, UNTOUCHED_FIELD] + +const SUCCESSFUL_PROMOTE_RESULT = { + promoteRunId: 'run-1', + blockers: [], + unmappedRequired: [], + droppedReferences: [], + triggerUrlChanges: [], + deployFailed: 0, +} + +function createDeferred() { + let resolve!: (value: T) => void + const promise = new Promise((resolvePromise) => { + resolve = resolvePromise + }) + return { promise, resolve } +} + +const mappedRepickContext = (previousValue: string) => ({ + previousValue, + baselineValueFor: (field: ForkDependentReconfig) => field.currentValue, +}) + +function diffData(dependentReconfigs: ForkDependentReconfig[]) { + return { + sourceWorkspaceId: WORKSPACE_ID, + targetWorkspaceId: OTHER_WORKSPACE_ID, + workflows: [], + dependentReconfigs, + resourceUsages: [], + copyableUnmapped: [], + clearedRefs: [], + triggerMappings: [], + retiringTriggerUrls: [], + excludedSourceWorkflows: [], + excludedTargetWorkflows: [], + mcpReauthServerIds: [], + inlineSecretSources: [], + } +} + +const mountedRoots: Root[] = [] + +function renderForkSync(): { get: () => ForkSyncController } { + ;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true + const container = document.createElement('div') + const root = createRoot(container) + mountedRoots.push(root) + let result: ForkSyncController | undefined + + function Probe() { + result = useForkSync({ + workspaceId: WORKSPACE_ID, + otherWorkspaceId: OTHER_WORKSPACE_ID, + otherWorkspaceName: 'Parent', + direction: 'push', + enabled: true, + }) + return null + } + + act(() => { + root.render(() as ReactNode) + }) + + return { + get: () => { + if (!result) throw new Error('hook result is not ready') + return result + }, + } +} + +/** The `dependentValues` the last Save sent, or `undefined` when it sent none. */ +function savedDependentValues(): UpdateForkMappingBody['dependentValues'] { + expect(mockUpdateMutate).toHaveBeenCalledTimes(1) + const [variables] = mockUpdateMutate.mock.calls[0] as [{ body: UpdateForkMappingBody }] + return variables.body.dependentValues +} + +/** The `dependentValues` the last Sync promoted. */ +function promotedDependentValues(): UpdateForkMappingBody['dependentValues'] { + expect(mockPromote).toHaveBeenCalledTimes(1) + const [variables] = mockPromote.mock.calls[0] as [ + { body: { dependentValues?: UpdateForkMappingBody['dependentValues'] } }, + ] + return variables.body.dependentValues +} + +function valueFor( + submitted: UpdateForkMappingBody['dependentValues'], + field: ForkDependentReconfig +): string | undefined { + return submitted?.find((entry) => entry.subBlockKey === field.subBlockKey)?.value +} + +beforeEach(() => { + vi.clearAllMocks() + mockUseForkMapping.mockReturnValue({ + data: { entries: [CREDENTIAL_ENTRY] }, + isLoading: false, + isError: false, + error: null, + isPlaceholderData: false, + }) + mockUseForkDiff.mockReturnValue({ + data: diffData(DEPENDENTS), + isError: false, + error: null, + isPlaceholderData: false, + }) + mockUpdateMutateAsync.mockResolvedValue({ success: true, updated: 1 }) + mockPromote.mockResolvedValue(SUCCESSFUL_PROMOTE_RESULT) +}) + +afterEach(() => { + act(() => { + for (const root of mountedRoots.splice(0)) root.unmount() + }) +}) + +describe('useForkSync dependent payload', () => { + it('submits an effective blank for an optional dependent invalidated by a real provider change', () => { + const { get } = renderForkSync() + + act(() => { + get().setReconfig((prev) => + applyDependentRepick( + prev, + PARENT_FIELD, + DEPENDENTS, + 'sheet-2', + mappedRepickContext('sheet-1') + ) + ) + }) + + expect(get().reconfig[dependentKey(CHILD_FIELD)]).toBeNull() + + act(() => get().save()) + + const submitted = savedDependentValues() + expect(submitted?.map((entry) => entry.subBlockKey)).toEqual([ + PARENT_FIELD.subBlockKey, + CHILD_FIELD.subBlockKey, + CLEARED_FIELD.subBlockKey, + UNTOUCHED_FIELD.subBlockKey, + ]) + expect(valueFor(submitted, PARENT_FIELD)).toBe('sheet-2') + expect(valueFor(submitted, CHILD_FIELD)).toBe('') + expect(valueFor(submitted, UNTOUCHED_FIELD)).toBe('label-3') + }) + + it('submits a field the user cleared themselves, so an intentional clear still clears the target', () => { + const { get } = renderForkSync() + + act(() => { + get().setReconfig((prev) => ({ ...prev, [dependentKey(CLEARED_FIELD)]: '' })) + }) + act(() => get().save()) + + const submitted = savedDependentValues() + expect(submitted?.map((entry) => entry.subBlockKey)).toContain(CLEARED_FIELD.subBlockKey) + expect(valueFor(submitted, CLEARED_FIELD)).toBe('') + }) + + it('restores the dependent chain when the provider re-pick is undone', () => { + const { get } = renderForkSync() + + act(() => { + get().setReconfig((prev) => + applyDependentRepick( + prev, + PARENT_FIELD, + DEPENDENTS, + 'sheet-2', + mappedRepickContext('sheet-1') + ) + ) + }) + expect(get().dirty).toBe(true) + + act(() => { + get().setReconfig((prev) => + applyDependentRepick( + prev, + PARENT_FIELD, + DEPENDENTS, + 'sheet-1', + mappedRepickContext('sheet-2') + ) + ) + }) + + expect(get().reconfig).toEqual({}) + expect(get().dirty).toBe(false) + + act(() => get().save()) + expect(mockUpdateMutate).not.toHaveBeenCalled() + }) + + it('keeps Sync blocked while a REQUIRED dependent is marked by a parent re-pick', () => { + const requiredChild = { ...CHILD_FIELD, required: true } + mockUseForkDiff.mockReturnValue({ + data: diffData([PARENT_FIELD, requiredChild]), + isError: false, + error: null, + isPlaceholderData: false, + }) + const { get } = renderForkSync() + + expect(get().syncDisabled).toBe(false) + + act(() => { + get().setReconfig((prev) => + applyDependentRepick( + prev, + PARENT_FIELD, + [PARENT_FIELD, requiredChild], + 'sheet-2', + mappedRepickContext('sheet-1') + ) + ) + }) + + expect(get().syncDisabled).toBe(true) + expect(get().syncDisabledReason).toBe('Reconfigure all required fields first') + + act(() => { + get().setReconfig((prev) => + applyDependentRepick( + prev, + PARENT_FIELD, + [PARENT_FIELD, requiredChild], + 'sheet-1', + mappedRepickContext('sheet-2') + ) + ) + }) + + expect(get().reconfig).toEqual({}) + expect(get().syncDisabled).toBe(false) + }) + + it('submits the invalidated dependent as blank in the promote payload too', async () => { + const { get } = renderForkSync() + + act(() => { + get().setReconfig((prev) => + applyDependentRepick( + prev, + PARENT_FIELD, + DEPENDENTS, + 'sheet-2', + mappedRepickContext('sheet-1') + ) + ) + }) + await act(async () => { + await get().sync() + }) + + expect(valueFor(promotedDependentValues(), CHILD_FIELD)).toBe('') + }) + + it('clears a stale nested Agent tool child when its provider changes', async () => { + const project = dependent({ + subBlockKey: 'tools[0].projectId', + dependencyScope: 'tools[0]', + currentValue: 'project-old', + providesContextKey: 'projectId', + }) + const issue = dependent({ + subBlockKey: 'tools[0].issueKey', + dependencyScope: 'tools[0]', + currentValue: 'OLD-1', + consumesContextKeys: ['projectId'], + }) + mockUseForkDiff.mockReturnValue({ + data: diffData([project, issue]), + isError: false, + error: null, + isPlaceholderData: false, + }) + const { get } = renderForkSync() + + act(() => { + get().setReconfig((prev) => + applyDependentRepick( + prev, + project, + [project, issue], + 'project-new', + mappedRepickContext('project-old') + ) + ) + }) + await act(async () => { + await get().sync() + }) + + expect(valueFor(promotedDependentValues(), project)).toBe('project-new') + expect(valueFor(promotedDependentValues(), issue)).toBe('') + }) +}) + +describe('useForkSync post-sync reset', () => { + it('drops the in-session re-picks once the sync commits them', async () => { + const { get } = renderForkSync() + + act(() => { + get().setReconfig((prev) => + applyDependentRepick( + prev, + PARENT_FIELD, + DEPENDENTS, + 'sheet-2', + mappedRepickContext('sheet-1') + ) + ) + }) + expect(get().dirty).toBe(true) + + await act(async () => { + await get().sync() + }) + + expect(get().reconfig).toEqual({}) + expect(get().dirty).toBe(false) + }) + + it('keeps the in-session re-picks when the sync is refused by the server gate', async () => { + mockPromote.mockResolvedValue({ + promoteRunId: null, + blockers: [{ kind: 'credential', sourceId: 'cred-src' }], + unmappedRequired: [], + droppedReferences: [], + triggerUrlChanges: [], + deployFailed: 0, + }) + const { get } = renderForkSync() + + act(() => { + get().setReconfig((prev) => + applyDependentRepick( + prev, + PARENT_FIELD, + DEPENDENTS, + 'sheet-2', + mappedRepickContext('sheet-1') + ) + ) + }) + await act(async () => { + await get().sync() + }) + + expect(get().reconfig[dependentKey(PARENT_FIELD)]).toBe('sheet-2') + }) + + it('keeps a newer target mapping selected while Sync was in flight', async () => { + const pendingPromote = createDeferred() + mockPromote.mockReturnValue(pendingPromote.promise) + const { get } = renderForkSync() + let syncPromise!: Promise + + act(() => { + syncPromise = get().sync() + }) + await act(async () => Promise.resolve()) + expect(mockPromote).toHaveBeenCalledTimes(1) + + act(() => get().setTarget(CREDENTIAL_ENTRY, 'cred-newer')) + pendingPromote.resolve(SUCCESSFUL_PROMOTE_RESULT) + await act(async () => syncPromise) + + expect(get().targetFor(CREDENTIAL_ENTRY)).toBe('cred-newer') + expect(get().dirty).toBe(true) + }) + + it('keeps a newer dependent re-pick made while Sync was in flight', async () => { + const pendingPromote = createDeferred() + mockPromote.mockReturnValue(pendingPromote.promise) + const { get } = renderForkSync() + let syncPromise!: Promise + + act(() => { + syncPromise = get().sync() + }) + await act(async () => Promise.resolve()) + expect(mockPromote).toHaveBeenCalledTimes(1) + + act(() => { + get().setReconfig((current) => ({ + ...current, + [dependentKey(PARENT_FIELD)]: 'sheet-newer', + })) + }) + pendingPromote.resolve(SUCCESSFUL_PROMOTE_RESULT) + await act(async () => syncPromise) + + expect(get().reconfig[dependentKey(PARENT_FIELD)]).toBe('sheet-newer') + expect(get().dirty).toBe(true) + }) + + it('keeps newer mapping edits made while Save was in flight', () => { + let finishSave: (() => void) | undefined + mockUpdateMutate.mockImplementation((_variables, options) => { + finishSave = options.onSuccess + }) + const { get } = renderForkSync() + + act(() => { + get().setReconfig((current) => ({ + ...current, + [dependentKey(PARENT_FIELD)]: 'sheet-submitted', + })) + }) + act(() => get().save()) + + act(() => { + get().setTarget(CREDENTIAL_ENTRY, 'cred-newer') + get().setReconfig((current) => ({ + ...current, + [dependentKey(PARENT_FIELD)]: 'sheet-newer', + })) + }) + act(() => finishSave?.()) + + expect(get().targetFor(CREDENTIAL_ENTRY)).toBe('cred-newer') + expect(get().reconfig[dependentKey(PARENT_FIELD)]).toBe('sheet-newer') + expect(get().dirty).toBe(true) + }) +}) diff --git a/apps/sim/ee/workspace-forking/components/fork-sync/use-fork-sync.ts b/apps/sim/ee/workspace-forking/components/fork-sync/use-fork-sync.ts index c28d29c371b..61aadc7bce5 100644 --- a/apps/sim/ee/workspace-forking/components/fork-sync/use-fork-sync.ts +++ b/apps/sim/ee/workspace-forking/components/fork-sync/use-fork-sync.ts @@ -31,9 +31,11 @@ import { isForkRequiredComplete, } from '@/ee/workspace-forking/components/fork-sync/copy-reconciliation' import { + type DependentReconfigState, dependentKey, effectiveCopyDependentValue, effectiveDependentValue, + isDependentInvalidated, } from '@/ee/workspace-forking/components/fork-sync/dependent-value' import { forkDyingTriggerUrls, @@ -145,8 +147,8 @@ export interface ForkSyncController { */ sourceWorkspaceId: string /** In-session dependent re-picks, keyed by `dependentKey`. */ - reconfig: Record - setReconfig: Dispatch>> + reconfig: DependentReconfigState + setReconfig: Dispatch> /** Keys the backend offers as copy candidates, for the entry rows' "Copy instead" affordance. */ copyableKeys: ReadonlySet /** Copyables actually selected for copy (visible + checked), keyed `${kind}:${sourceId}`. */ @@ -286,7 +288,7 @@ export function useForkSync(params: { // `dependentKey`. Folded into the full effective set sent on save/sync, which the server // persists as the stored mapping - so the selection survives every future sync without // re-picking. - const [reconfig, setReconfig] = useState>({}) + const [reconfig, setReconfig] = useState({}) // Referenced-but-unmapped resources the user chose to copy into the target (keyed by // `${kind}:${sourceId}`); default-selected once the diff loads. Selected ones are copied on // sync so their references resolve to the copy instead of being cleared. @@ -685,8 +687,11 @@ export function useForkSync(params: { // effective value: re-pick, stored, or blank-after-change) or copy-selected (re-pick, stored, // or the source reference; promote translates a source document id to its copied counterpart // at write time). The server persists this verbatim as the stored mapping; fields whose - // parent is unresolved are omitted (they can't be configured). This is the whole "what's in - // the mapping goes in" contract, shared by Save and Sync so the two persist identically. + // parent is unresolved are omitted because they can't be configured. A field invalidated by + // an in-block provider re-pick is submitted as `''`: required fields gate Sync until re-picked, + // while optional/LLM-fillable fields must land empty rather than retain a source value scoped + // to the old provider. This is the whole "what's in the mapping goes in" contract, shared by + // Save and Sync so the two persist identically. const buildDependentValues = () => dependentReconfigs.flatMap((field) => { const parent = entryForDependent(field) @@ -723,9 +728,12 @@ export function useForkSync(params: { // A dependent re-pick that differs from its stored value also dirties the editor. A re-pick // under a changed parent is covered by `targetsDirty` (the parent override is the change). + // An automatically invalidated field is covered by the provider override that caused it; it + // must not independently dirty an editor after that provider has been restored to baseline. const reconfigDirty = useMemo( () => dependentReconfigs.some((field) => { + if (isDependentInvalidated(field, reconfig)) return false const repicked = reconfig[dependentKey(field)] return repicked !== undefined && repicked !== field.currentValue }), @@ -736,6 +744,8 @@ export function useForkSync(params: { const save = () => { if (!otherWorkspaceId || !dirty || updateMapping.isPending) return + const submittedTargets = targets + const submittedReconfig = reconfig updateMapping.mutate( { workspaceId, @@ -751,8 +761,8 @@ export function useForkSync(params: { }, { onSuccess: () => { - setTargets({}) - setReconfig({}) + setTargets((current) => (current === submittedTargets ? {} : current)) + setReconfig((current) => (current === submittedReconfig ? {} : current)) toast.success('Mapping saved') }, onError: (error) => toast.error(getErrorMessage(error, 'Failed to save mapping')), @@ -828,6 +838,8 @@ export function useForkSync(params: { const sync = async () => { if (!otherWorkspaceId) return setSubmitting(true) + const submittedTargets = targets + const submittedReconfig = reconfig // Capture every payload from the state at confirm time, before any await - the page's // controls stay mounted during the run (unlike the old modal, which blocked its UI), so a // mid-flight edit must not leak into the promote body. @@ -922,6 +934,12 @@ export function useForkSync(params: { return } + // The run committed the in-session choices: the mapping entries and dependent values are + // stored. Drop only the exact snapshots it submitted; edits made while the request was in + // flight were not committed by this run and must remain available for the next Save/Sync. + setTargets((current) => (current === submittedTargets ? {} : current)) + setReconfig((current) => (current === submittedReconfig ? {} : current)) + const target = otherWorkspaceName || 'the workspace' const label = direction === 'pull' ? `Pulled from "${target}"` : `Pushed to "${target}"` // A sync only commits once every reference is mapped/copied and every required dependent diff --git a/apps/sim/ee/workspace-forking/lib/copy/copy-files.test.ts b/apps/sim/ee/workspace-forking/lib/copy/copy-files.test.ts index 622c01cf582..76f2bb315aa 100644 --- a/apps/sim/ee/workspace-forking/lib/copy/copy-files.test.ts +++ b/apps/sim/ee/workspace-forking/lib/copy/copy-files.test.ts @@ -4,21 +4,75 @@ import { folder as folderTable } from '@sim/db/schema' import { dbChainMockFns, + type MockCondition, resetDbChainMock, storageServiceMock, storageServiceMockFns, } from '@sim/testing' import { beforeEach, describe, expect, it, vi } from 'vitest' +/** The `workspace_files` columns {@link fileRows} enforces its unique indexes on. */ +interface WorkspaceFileRow { + id: string + key: string + workspaceId: string | null + folderId?: string | null + context: string + originalName: string + deletedAt: Date | null + [column: string]: unknown +} + +/** + * `fileRows` is the shared stand-in for the `workspace_files` table: the mocked name + * allocator reads it exactly as the real one queries the DB, and the insert simulation in + * `executeForkFileBlobCopies collisions` enforces the same unique indexes against it. One + * store, so the allocator and the index can never disagree the way two fixtures would. + */ const { + fileRows, + mockAllocateUniqueWorkspaceFileName, mockCopyWorkspaceFileSecretProvenanceInTx, mockIncrementStorageUsageInTx, mockResolveStorageBillingContext, -} = vi.hoisted(() => ({ - mockCopyWorkspaceFileSecretProvenanceInTx: vi.fn(), - mockIncrementStorageUsageInTx: vi.fn(), - mockResolveStorageBillingContext: vi.fn(), -})) +} = vi.hoisted(() => { + const fileRows: WorkspaceFileRow[] = [] + const withCopySuffix = (name: string, n: number) => { + const lastDot = name.lastIndexOf('.') + return lastDot > 0 && lastDot < name.length - 1 + ? `${name.slice(0, lastDot)} (${n})${name.slice(lastDot)}` + : `${name} (${n})` + } + return { + fileRows, + /** + * Mirrors `allocateUniqueWorkspaceFileName`: the taken-name probe matches the columns + * of `workspace_files_workspace_folder_name_active_unique`. + */ + mockAllocateUniqueWorkspaceFileName: vi.fn( + async (workspaceId: string, baseName: string, folderId?: string | null) => { + const taken = (name: string) => + fileRows.some( + (row) => + row.deletedAt === null && + row.context === 'workspace' && + row.workspaceId === workspaceId && + (row.folderId ?? null) === (folderId ?? null) && + row.originalName === name + ) + if (!taken(baseName)) return baseName + for (let n = 1; n <= 1000; n++) { + const candidate = withCopySuffix(baseName, n) + if (!taken(candidate)) return candidate + } + throw new Error(`A file named "${baseName}" already exists in this workspace`) + } + ), + mockCopyWorkspaceFileSecretProvenanceInTx: vi.fn(), + mockIncrementStorageUsageInTx: vi.fn(), + mockResolveStorageBillingContext: vi.fn(), + } +}) vi.mock('@/lib/uploads/core/storage-service', () => storageServiceMock) vi.mock('@/lib/billing/storage', () => ({ @@ -26,6 +80,7 @@ vi.mock('@/lib/billing/storage', () => ({ resolveStorageBillingContext: mockResolveStorageBillingContext, })) vi.mock('@/lib/uploads/contexts/workspace/workspace-file-manager', () => ({ + allocateUniqueWorkspaceFileName: mockAllocateUniqueWorkspaceFileName, generateWorkspaceFileKey: vi.fn( (workspaceId: string, fileName: string) => `workspace/${workspaceId}/generated-${fileName}` ), @@ -63,6 +118,7 @@ describe('executeForkFileBlobCopies storage accounting', () => { beforeEach(() => { vi.clearAllMocks() resetDbChainMock() + fileRows.length = 0 storageServiceMockFns.mockHeadObject.mockResolvedValue(null) storageServiceMockFns.mockDownloadFile.mockResolvedValue(Buffer.from('blob-bytes')) storageServiceMockFns.mockUploadFile.mockResolvedValue({ key: 'workspace/child-ws/target' }) @@ -194,6 +250,290 @@ describe('executeForkFileBlobCopies storage accounting', () => { }) }) +/** Rejects a predicate the harness does not model, rather than letting it match everything. */ +function unsupportedPredicate(detail: string): never { + throw new Error(`Unsupported predicate in test harness: ${detail}`) +} + +/** The nested clauses of an `and`/`or` node, or a throw when the node carries none. */ +function predicateClauses(node: MockCondition): unknown[] { + if (!Array.isArray(node.conditions)) + unsupportedPredicate(`${String(node.type)} without conditions`) + return node.conditions +} + +/** + * The row key a predicate node references. The mocked schema tables are column-name maps, so a + * column reference is the column name itself; anything else is a shape this harness cannot read. + */ +function predicateColumn(node: MockCondition, field: 'left' | 'column'): string { + const column = node[field] + if (typeof column !== 'string') + unsupportedPredicate(`${String(node.type)} with a non-column ${field}`) + return column +} + +/** + * Evaluate a mocked drizzle predicate against a row. Real predicate reading, so a chain that + * ignores its `where` clause cannot pass these tests by echoing a fixture back. + */ +function matchesPredicate(row: Record, predicate: unknown): boolean { + if (!predicate) return true + if (typeof predicate !== 'object') unsupportedPredicate(typeof predicate) + const node = predicate as MockCondition + switch (node.type) { + case 'and': + return predicateClauses(node).every((clause) => matchesPredicate(row, clause)) + case 'or': + return predicateClauses(node).some((clause) => matchesPredicate(row, clause)) + case 'eq': + return row[predicateColumn(node, 'left')] === node.right + case 'isNull': { + const value = row[predicateColumn(node, 'column')] + return value === null || value === undefined + } + case 'inArray': { + if (!Array.isArray(node.values)) unsupportedPredicate('inArray without values') + return node.values.includes(row[predicateColumn(node, 'column')]) + } + default: + return unsupportedPredicate(String(node.type)) + } +} + +/** Awaitable stand-in for a drizzle select result, supporting `.limit`/`.for`/`.orderBy`. */ +interface MockSelectResult extends PromiseLike { + catch: Promise['catch'] + finally: Promise['finally'] + limit: (count: number) => MockSelectResult + for: () => MockSelectResult + orderBy: () => MockSelectResult +} + +function selectResult(rows: WorkspaceFileRow[]): MockSelectResult { + const settled = Promise.resolve(rows) + const builder: MockSelectResult = { + then: (onFulfilled, onRejected) => settled.then(onFulfilled, onRejected), + catch: (onRejected) => settled.catch(onRejected), + finally: (onFinally) => settled.finally(onFinally), + limit: (count: number) => selectResult(rows.slice(0, count)), + for: () => builder, + orderBy: () => builder, + } + return builder +} + +/** + * Postgres-faithful `workspace_files` writes against {@link fileRows}: `key` is guarded by + * `workspace_files_key_active_unique` and `(workspace_id, coalesce(folder_id, ''), + * original_name)` by `workspace_files_workspace_folder_name_active_unique`. A bare + * `onConflictDoNothing()` absorbs BOTH plus the primary key (the shipped bug); one targeted at + * the primary key absorbs only a replay of the same row and lets a real name clash raise. + */ +function installFileTableSimulation(): void { + dbChainMockFns.where.mockImplementation((predicate: unknown) => + selectResult(fileRows.filter((row) => matchesPredicate(row, predicate))) + ) + dbChainMockFns.values.mockImplementation((row: WorkspaceFileRow) => { + const attemptInsert = (conflictTarget: unknown) => { + const pkConflict = fileRows.some((existing) => existing.id === row.id) + const activeConflict = fileRows.some( + (existing) => + existing.deletedAt === null && + (existing.key === row.key || + (existing.context === 'workspace' && + existing.workspaceId === row.workspaceId && + (existing.folderId ?? null) === (row.folderId ?? null) && + existing.originalName === row.originalName)) + ) + if (conflictTarget === undefined ? pkConflict || activeConflict : pkConflict) { + return Promise.resolve([]) + } + if (activeConflict) { + return Promise.reject( + Object.assign( + new Error( + 'duplicate key value violates unique constraint ' + + '"workspace_files_workspace_folder_name_active_unique"' + ), + { code: '23505' } + ) + ) + } + fileRows.push({ ...row }) + return Promise.resolve([{ id: row.id }]) + } + return { + onConflictDoNothing: (config?: { target?: unknown }) => { + dbChainMockFns.onConflictDoNothing(config) + return { returning: () => attemptInsert(config?.target) } + }, + onConflictDoUpdate: () => ({ returning: () => attemptInsert(undefined) }), + returning: () => attemptInsert('no-conflict-clause'), + } + }) +} + +describe('executeForkFileBlobCopies target name collisions', () => { + const collidingTask = () => + makeTask({ + fileName: 'budget.xlsx', + sourceKey: 'workspace/src-ws/source-budget.xlsx', + targetKey: 'workspace/child-ws/target-budget.xlsx', + contentType: 'application/vnd.ms-excel', + targetFolderId: 'target-reports', + displayName: 'budget.xlsx', + }) + + beforeEach(() => { + vi.clearAllMocks() + resetDbChainMock() + fileRows.length = 0 + storageServiceMockFns.mockHeadObject.mockResolvedValue(null) + storageServiceMockFns.mockDownloadFile.mockResolvedValue(Buffer.from('blob-bytes')) + storageServiceMockFns.mockUploadFile.mockResolvedValue({ + key: 'workspace/child-ws/target-budget.xlsx', + }) + mockResolveStorageBillingContext.mockResolvedValue({ + workspaceId: 'child-ws', + billedAccountUserId: 'target-payer', + billingEntity: { type: 'user', id: 'target-payer' }, + plan: 'pro', + customStorageLimitGB: null, + }) + mockIncrementStorageUsageInTx.mockResolvedValue(100) + installFileTableSimulation() + }) + + it('keeps a file whose name is already taken in the reused target folder', async () => { + // The target already holds `Reports/budget.xlsx`; the fork mirrors `Reports` onto it. + fileRows.push({ + id: 'pre-existing', + key: 'workspace/child-ws/pre-existing-budget.xlsx', + workspaceId: 'child-ws', + folderId: 'target-reports', + context: 'workspace', + originalName: 'budget.xlsx', + deletedAt: null, + }) + + const result = await executeForkFileBlobCopies([collidingTask()], 'test') + + expect(result).toEqual({ copied: 1, failed: 0, failedTargetKeys: [] }) + // Non-destructive: the copy lands beside the target's own file, in the mirrored folder. + expect(fileRows).toHaveLength(2) + expect(fileRows.find((row) => row.id === 'pre-existing')?.originalName).toBe('budget.xlsx') + expect(fileRows.find((row) => row.id === 'target-file-1')).toMatchObject({ + key: 'workspace/child-ws/target-budget.xlsx', + workspaceId: 'child-ws', + folderId: 'target-reports', + originalName: 'budget (1).xlsx', + displayName: 'budget (1).xlsx', + deletedAt: null, + }) + // The blob backing the surviving row must never be swept. + expect(storageServiceMockFns.mockDeleteFile).not.toHaveBeenCalled() + // The de-duplication probe is scoped to the index's exact tuple, folder included. + expect(mockAllocateUniqueWorkspaceFileName).toHaveBeenCalledWith( + 'child-ws', + 'budget.xlsx', + 'target-reports' + ) + expect(mockIncrementStorageUsageInTx).toHaveBeenCalledTimes(1) + }) + + it('absorbs only a primary-key conflict, so a name conflict can never be mistaken for a replay', async () => { + await executeForkFileBlobCopies([collidingTask()], 'test') + + expect(dbChainMockFns.onConflictDoNothing).toHaveBeenCalledWith({ target: 'id' }) + }) + + it('copies a non-colliding file into the mirrored folder unchanged', async () => { + fileRows.push({ + id: 'pre-existing', + key: 'workspace/child-ws/pre-existing-forecast.xlsx', + workspaceId: 'child-ws', + folderId: 'target-reports', + context: 'workspace', + originalName: 'forecast.xlsx', + deletedAt: null, + }) + + const result = await executeForkFileBlobCopies([collidingTask()], 'test') + + expect(result).toEqual({ copied: 1, failed: 0, failedTargetKeys: [] }) + expect(fileRows.find((row) => row.id === 'target-file-1')).toMatchObject({ + folderId: 'target-reports', + originalName: 'budget.xlsx', + displayName: 'budget.xlsx', + }) + expect(storageServiceMockFns.mockDeleteFile).not.toHaveBeenCalled() + }) + + it('de-duplicates each same-named copy against the rows earlier tasks already landed', async () => { + // Two source files share a name inside the folder the target already has a `budget.xlsx` in. + // Each allocation must see the row the previous task committed, so the suffixes advance + // instead of every task racing for the same `budget (1).xlsx`. + fileRows.push({ + id: 'pre-existing', + key: 'workspace/child-ws/pre-existing-budget.xlsx', + workspaceId: 'child-ws', + folderId: 'target-reports', + context: 'workspace', + originalName: 'budget.xlsx', + deletedAt: null, + }) + + const result = await executeForkFileBlobCopies( + [ + collidingTask(), + makeTask({ + fileName: 'budget.xlsx', + sourceKey: 'workspace/src-ws/source-budget-2.xlsx', + targetKey: 'workspace/child-ws/target-budget-2.xlsx', + contentType: 'application/vnd.ms-excel', + targetFolderId: 'target-reports', + targetFileId: 'target-file-2', + }), + ], + 'test' + ) + + expect(result).toEqual({ copied: 2, failed: 0, failedTargetKeys: [] }) + expect(fileRows.map((row) => row.originalName)).toEqual([ + 'budget.xlsx', + 'budget (1).xlsx', + 'budget (2).xlsx', + ]) + expect(storageServiceMockFns.mockDeleteFile).not.toHaveBeenCalled() + }) + + it('replays to the same end state without duplicating the copy or failing differently', async () => { + fileRows.push({ + id: 'pre-existing', + key: 'workspace/child-ws/pre-existing-budget.xlsx', + workspaceId: 'child-ws', + folderId: 'target-reports', + context: 'workspace', + originalName: 'budget.xlsx', + deletedAt: null, + }) + + const first = await executeForkFileBlobCopies([collidingTask()], 'test') + const afterFirst = structuredClone(fileRows) + const replay = await executeForkFileBlobCopies([collidingTask()], 'test') + + expect(first).toEqual({ copied: 1, failed: 0, failedTargetKeys: [] }) + // The replay resolves the existing copy instead of re-copying: still one success, no failure. + expect(replay).toEqual({ copied: 1, failed: 0, failedTargetKeys: [] }) + expect(fileRows).toEqual(afterFirst) + expect(storageServiceMockFns.mockUploadFile).toHaveBeenCalledTimes(1) + expect(dbChainMockFns.transaction).toHaveBeenCalledTimes(1) + expect(mockIncrementStorageUsageInTx).toHaveBeenCalledTimes(1) + expect(storageServiceMockFns.mockDeleteFile).not.toHaveBeenCalled() + }) +}) + describe('planForkFileCopies', () => { it('plans deterministic target metadata without inserting an active row before blob copy', async () => { const sourceMeta = { diff --git a/apps/sim/ee/workspace-forking/lib/copy/copy-files.ts b/apps/sim/ee/workspace-forking/lib/copy/copy-files.ts index cfa4888170d..22ff96868a3 100644 --- a/apps/sim/ee/workspace-forking/lib/copy/copy-files.ts +++ b/apps/sim/ee/workspace-forking/lib/copy/copy-files.ts @@ -9,7 +9,10 @@ import { resolveStorageBillingContext, } from '@/lib/billing/storage' import type { DbOrTx } from '@/lib/db/types' -import { generateWorkspaceFileKey } from '@/lib/uploads/contexts/workspace/workspace-file-manager' +import { + allocateUniqueWorkspaceFileName, + generateWorkspaceFileKey, +} from '@/lib/uploads/contexts/workspace/workspace-file-manager' import { copyWorkspaceFileSecretProvenanceInTx } from '@/lib/uploads/contexts/workspace/workspace-file-secret-provenance' import { deleteFile, @@ -112,6 +115,31 @@ async function getFinalizedFileCopies( return new Map(active.map((row) => [row.id, { key: row.key, workspaceId: row.workspaceId }])) } +/** + * Pick the child row's `original_name`, de-duplicating against the partial unique index + * `workspace_files_workspace_folder_name_active_unique` on + * `(workspace_id, coalesce(folder_id, ''), original_name)`. + * + * Since the fork copy started preserving folder structure, a target folder reused by + * {@link resolveForkFolderMapping} can already hold a file of the same name (parent has + * `Reports/budget.xlsx`, the child pushes its own `Reports/budget.xlsx`). Reusing the source + * name would violate that index, so the copy lands as `budget (1).xlsx` instead: both files + * survive, the copy stays visible in the fork under the mirrored folder, and its blob is kept. + * + * Advisory only. The lookup runs outside the finalize transaction (and so cannot see it), so a + * concurrent upload can still claim the name in between. The index stays the authority: such a + * task fails loudly into `failedTargetKeys` instead of being silently dropped. + */ +async function resolveTargetOriginalName(task: BlobCopyTask): Promise { + // The index is partial on durable workspace files; no other context can collide on it. + if (task.context !== 'workspace') return task.fileName + return allocateUniqueWorkspaceFileName( + task.workspaceId, + task.fileName, + task.targetFolderId ?? null + ) +} + /** * Plan child metadata identities and blob copies without inserting an active * `workspace_files` row. The child workspace and source mappings are committed @@ -216,6 +244,10 @@ export async function planForkFileCopies(params: { * `file-upload` references pointing at the now-missing object. A failed task has no * active target metadata; an object uploaded before a failed finalization is deleted * best-effort outside the transaction. + * + * A copy whose name is already taken inside its mirrored target folder is de-duplicated + * (`budget (1).xlsx`) rather than dropped - see {@link resolveTargetOriginalName}. Those + * still count as `copied`. */ export async function executeForkFileBlobCopies( blobTasks: BlobCopyTask[], @@ -292,6 +324,9 @@ export async function executeForkFileBlobCopies( } const billingContext = await resolveStorageBillingContext(task.workspaceId) + const targetOriginalName = await resolveTargetOriginalName(task) + const targetDisplayName = + targetOriginalName === task.fileName ? task.displayName : targetOriginalName await db.transaction(async (tx) => { const [inserted] = await tx .insert(workspaceFiles) @@ -303,16 +338,21 @@ export async function executeForkFileBlobCopies( folderId: task.targetFolderId ?? null, context: task.context, chatId: null, - originalName: task.fileName, - displayName: task.displayName, + originalName: targetOriginalName, + displayName: targetDisplayName, contentType: task.contentType, size: task.size, deletedAt: null, uploadedAt: new Date(), }) - .onConflictDoNothing() + // Targeted at the primary key so ONLY a replay of this same task is absorbed. A + // bare `onConflictDoNothing()` also swallows the `(workspace_id, folder_id, + // original_name)` unique index, whose conflicting row has a DIFFERENT id - the + // recovery below then finds nothing and drops a file that merely shares a name. + .onConflictDoNothing({ target: workspaceFiles.id }) .returning({ id: workspaceFiles.id }) + // Reachable only on a primary-key conflict, so the row is addressable by id. if (!inserted) { const [current] = await tx .select({ @@ -346,8 +386,8 @@ export async function executeForkFileBlobCopies( folderId: task.targetFolderId ?? null, context: task.context, chatId: null, - originalName: task.fileName, - displayName: task.displayName, + originalName: targetOriginalName, + displayName: targetDisplayName, contentType: task.contentType, size: task.size, deletedAt: null, @@ -371,6 +411,14 @@ export async function executeForkFileBlobCopies( await incrementStorageUsageForBillingContextInTx(tx, billingContext, task.size) }) copied += 1 + if (targetOriginalName !== task.fileName) { + logger.warn(`[${requestId}] Copied file renamed to avoid a target name collision`, { + targetKey: task.targetKey, + folderId: task.targetFolderId ?? null, + from: task.fileName, + to: targetOriginalName, + }) + } } catch (error) { failedTargetKeys.push(task.targetKey) logger.warn(`[${requestId}] Failed to copy file blob during fork`, { diff --git a/apps/sim/ee/workspace-forking/lib/mapping/dependent-reconfigs.ts b/apps/sim/ee/workspace-forking/lib/mapping/dependent-reconfigs.ts index 9d52ec1ce21..f050018cec4 100644 --- a/apps/sim/ee/workspace-forking/lib/mapping/dependent-reconfigs.ts +++ b/apps/sim/ee/workspace-forking/lib/mapping/dependent-reconfigs.ts @@ -1,12 +1,15 @@ import { isRecordLike } from '@sim/utils/object' import type { ForkDependentReconfig, ForkResourceUsage } from '@/lib/api/contracts/workspace-fork' import { coerceObjectArray } from '@/lib/workflows/persistence/remap-internal-ids' -import { getWorkflowSearchDependentClears } from '@/lib/workflows/search-replace/dependencies' import { getToolInputParamConfigs } from '@/lib/workflows/search-replace/indexer' import { buildSelectorContextFromBlock, SELECTOR_CONTEXT_FIELDS, } from '@/lib/workflows/subblocks/context' +import { + getDependsOnFields, + getTransitiveSubBlockDependents, +} from '@/lib/workflows/subblocks/dependencies' import { buildCanonicalIndex, buildSubBlockValues, @@ -18,7 +21,6 @@ import { import { resolveToolParamRequired } from '@/lib/workflows/tool-input/param-visibility' import { getBlock } from '@/blocks/registry' import type { SubBlockConfig } from '@/blocks/types' -import { getDependsOnFields } from '@/blocks/utils' import type { ForkBlockIdResolver } from '@/ee/workspace-forking/lib/remap/block-identity' import { toScannerBlocks } from '@/ee/workspace-forking/lib/remap/reference-scan' import { @@ -163,7 +165,7 @@ function emitAnchoredDependents(params: EmitAnchoredParams): void { if (typeof value === 'string' && value) context[key] = value } - for (const clear of getWorkflowSearchDependentClears(config.subBlocks, anchorCfg.id)) { + for (const clear of getTransitiveSubBlockDependents(config.subBlocks, [anchorCfg.id])) { const dependent = configById.get(clear.subBlockId) if (!dependent?.id || !dependent.selectorKey) continue // Skip fields gated off by their `condition` - a selector under a now-inactive diff --git a/apps/sim/ee/workspace-forking/lib/remap/remap-references.test.ts b/apps/sim/ee/workspace-forking/lib/remap/remap-references.test.ts index 4f050b71c5d..0334e30e1d5 100644 --- a/apps/sim/ee/workspace-forking/lib/remap/remap-references.test.ts +++ b/apps/sim/ee/workspace-forking/lib/remap/remap-references.test.ts @@ -1117,6 +1117,56 @@ describe('applyDependentOverrides', () => { expect(tools[0].params.folder).toBe('Label_99') }) + it('applies an invalidated nested child as empty so it cannot survive under a new provider', () => { + vi.mocked(getBlock).mockImplementation((type) => { + if (type === 'agent') return blockWith([{ id: 'tools', title: 'Tools', type: 'tool-input' }]) + if (type === 'jira') { + return blockWith([ + { id: 'credential', title: 'Credential', type: 'oauth-input' }, + { + id: 'projectId', + title: 'Project', + type: 'project-selector', + dependsOn: ['credential'], + selectorKey: 'jira.projects', + }, + { + id: 'issueKey', + title: 'Issue', + type: 'issue-selector', + dependsOn: ['projectId'], + selectorKey: 'jira.issues', + }, + ]) + } + return undefined as unknown as BlockConfig + }) + const subBlocks: SubBlockRecord = { + tools: entry('tools', 'tool-input', [ + { + type: 'jira', + title: 'Jira', + params: { credential: 'c-new', projectId: 'project-old', issueKey: 'OLD-1' }, + }, + ]), + } + + const result = applyDependentOverrides( + subBlocks, + 'agent', + new Map([ + ['tools[0].projectId', 'project-new'], + ['tools[0].issueKey', ''], + ]) + ) + const tools = ( + result.tools as { + value: Array<{ params: { projectId: string; issueKey: string } }> + } + ).value + expect(tools[0].params).toMatchObject({ projectId: 'project-new', issueKey: '' }) + }) + it('rejects a nested override for a non-allowlisted tool param', () => { vi.mocked(getBlock).mockImplementation((type) => { if (type === 'agent') return blockWith([{ id: 'tools', title: 'Tools', type: 'tool-input' }]) diff --git a/apps/sim/ee/workspace-forking/lib/remap/remap-references.ts b/apps/sim/ee/workspace-forking/lib/remap/remap-references.ts index 1a71e90e60b..ae83c7c6f15 100644 --- a/apps/sim/ee/workspace-forking/lib/remap/remap-references.ts +++ b/apps/sim/ee/workspace-forking/lib/remap/remap-references.ts @@ -10,13 +10,17 @@ import { type SubBlockRecord, } from '@/lib/workflows/persistence/remap-internal-ids' import { CREDENTIAL_SUBBLOCK_IDS } from '@/lib/workflows/persistence/utils' -import { getWorkflowSearchDependentClears } from '@/lib/workflows/search-replace/dependencies' import { getToolInputParamConfigs } from '@/lib/workflows/search-replace/indexer' import { getWorkflowSearchSubBlockResourceDefinition, parseWorkflowSearchSubBlockResources, type StructuredWorkflowSearchResourceKind, } from '@/lib/workflows/search-replace/resources/registry' +import { + getDependsOnFields, + getSubBlocksDependingOnChange, + getTransitiveSubBlockDependents, +} from '@/lib/workflows/subblocks/dependencies' import { buildCanonicalIndex, buildSubBlockValues, @@ -36,7 +40,6 @@ import { import type { ParsedStoredTool } from '@/lib/workflows/tool-input/types' import { getBlock } from '@/blocks/registry' import type { SubBlockConfig } from '@/blocks/types' -import { getDependsOnFields, getSubBlocksDependingOnChange } from '@/blocks/utils' import { collectForkFileUploadKeys, remapForkFileUploadValue, @@ -393,7 +396,7 @@ interface ToolBlockRemapOptions { * fields (handled by the callers / the workflow id map), not block params, so they * pass through here untouched. Returns a new tool object only when something changed. * After remapping, dependent params (via `dependsOn`) of any changed resource are - * cleared with the same {@link getWorkflowSearchDependentClears} walk search-replace + * cleared with the same {@link getTransitiveSubBlockDependents} walk search-replace * uses, so a child scoped to the old parent isn't left stale. */ export function remapToolBlockResources( @@ -592,7 +595,7 @@ export function remapToolBlockResources( const parentCfg = configBySubBlockId.get(subBlockId) const parentRemappedNonEmpty = isNonEmptyValue(readParam(parentCfg, subBlockId)) const parentCopied = parentRemappedNonEmpty && copyRemappedSubBlockIds.has(subBlockId) - for (const clear of getWorkflowSearchDependentClears(toolBlockSubBlocks, subBlockId)) { + for (const clear of getTransitiveSubBlockDependents(toolBlockSubBlocks, [subBlockId])) { const dependentCfg = configBySubBlockId.get(clear.subBlockId) // A verbatim manual-parent dependent is never cleared, even when reachable from a // second (remapped) parent. @@ -1136,7 +1139,7 @@ export function clearDependentsOnRemap( } } - // Same BFS as `getWorkflowSearchDependentClears`, with each preserved dependent's subtree + // Same BFS as `getTransitiveSubBlockDependents`, with each preserved dependent's subtree // pruned (skipping it keeps its own dependents - e.g. a tool's arguments - out of the clear // set). A dependent under an ACTIVE MANUAL parent is verbatim by policy (the manual value is // never remapped), so it is pruned the same way. diff --git a/apps/sim/hooks/use-collaborative-workflow.ts b/apps/sim/hooks/use-collaborative-workflow.ts index 87e37394c0e..75516648bc1 100644 --- a/apps/sim/hooks/use-collaborative-workflow.ts +++ b/apps/sim/hooks/use-collaborative-workflow.ts @@ -25,10 +25,10 @@ import { type WorkflowSearchSubflowFieldId, workflowSearchSubflowFieldMatchesExpected, } from '@/lib/workflows/search-replace/subflow-fields' +import { getSubBlocksDependingOnChange } from '@/lib/workflows/subblocks/dependencies' import { isSyntheticToolSubBlockId } from '@/lib/workflows/tool-input/synthetic-subblocks' import { useSocket } from '@/app/workspace/providers/socket-provider' import { getBlock } from '@/blocks' -import { getSubBlocksDependingOnChange } from '@/blocks/utils' import { invalidateDeploymentQueries } from '@/hooks/queries/deployments' import { useUndoRedo } from '@/hooks/use-undo-redo' import { diff --git a/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/operations.test.ts b/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/operations.test.ts index 666d5c4e973..30312b308e9 100644 --- a/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/operations.test.ts +++ b/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/operations.test.ts @@ -4,22 +4,23 @@ import { describe, expect, it, vi } from 'vitest' import { applyOperationsToWorkflowState } from './engine' -vi.mock('@/blocks/registry', () => ({ - getAllBlocks: () => [ - { +vi.mock('@/blocks/registry', () => { + const blocks: Record = { + condition: { type: 'condition', name: 'Condition', subBlocks: [{ id: 'conditions', type: 'condition-input' }], }, - { + agent: { type: 'agent', name: 'Agent', subBlocks: [ { id: 'systemPrompt', type: 'long-input' }, { id: 'model', type: 'combobox' }, + { id: 'tools', type: 'tool-input' }, ], }, - { + function: { type: 'function', name: 'Function', subBlocks: [ @@ -27,34 +28,54 @@ vi.mock('@/blocks/registry', () => ({ { id: 'language', type: 'dropdown' }, ], }, - ], - getBlock: (type: string) => { - const blocks: Record = { - condition: { - type: 'condition', - name: 'Condition', - subBlocks: [{ id: 'conditions', type: 'condition-input' }], - }, - agent: { - type: 'agent', - name: 'Agent', - subBlocks: [ - { id: 'systemPrompt', type: 'long-input' }, - { id: 'model', type: 'combobox' }, - ], - }, - function: { - type: 'function', - name: 'Function', - subBlocks: [ - { id: 'code', type: 'code' }, - { id: 'language', type: 'dropdown' }, - ], - }, - } - return blocks[type] || undefined - }, -})) + jira: { + type: 'jira', + name: 'Jira', + tools: { access: ['jira_get_issue'] }, + subBlocks: [ + { id: 'credential', type: 'oauth-input' }, + { + id: 'projectId', + type: 'project-selector', + canonicalParamId: 'projectId', + mode: 'basic', + dependsOn: ['credential'], + }, + { + id: 'manualProjectId', + type: 'short-input', + canonicalParamId: 'projectId', + mode: 'advanced', + dependsOn: ['credential'], + }, + { + id: 'issueKey', + type: 'file-selector', + canonicalParamId: 'issueKey', + mode: 'basic', + dependsOn: ['projectId'], + }, + { + id: 'manualIssueKey', + type: 'short-input', + canonicalParamId: 'issueKey', + mode: 'advanced', + dependsOn: ['projectId'], + }, + { + id: 'transitionId', + type: 'short-input', + dependsOn: ['issueKey'], + }, + ], + }, + } + + return { + getAllBlocks: () => Object.values(blocks), + getBlock: (type: string) => blocks[type], + } +}) vi.mock('@/lib/integrations/availability.server', () => ({ isIntegrationDeploymentAvailableForVisibility: () => true, @@ -189,6 +210,175 @@ function makeNestedLoopWorkflow() { } } +function makeDependentWorkflow() { + return { + blocks: { + 'jira-1': { + id: 'jira-1', + type: 'jira', + name: 'Jira 1', + position: { x: 0, y: 0 }, + enabled: true, + subBlocks: { + credential: { id: 'credential', type: 'oauth-input', value: 'credential-old' }, + projectId: { id: 'projectId', type: 'project-selector', value: 'PROJECT-OLD' }, + manualProjectId: { + id: 'manualProjectId', + type: 'short-input', + value: '', + }, + issueKey: { id: 'issueKey', type: 'file-selector', value: 'OLD-123' }, + manualIssueKey: { id: 'manualIssueKey', type: 'short-input', value: '' }, + transitionId: { id: 'transitionId', type: 'short-input', value: 'transition-old' }, + }, + outputs: {}, + data: { + canonicalModes: { + projectId: 'basic', + issueKey: 'basic', + }, + }, + }, + }, + edges: [], + loops: {}, + parallels: {}, + } +} + +describe('handleEditOperation dependent inputs', () => { + it('clears omitted descendants transitively when a parent changes', () => { + const { state } = applyOperationsToWorkflowState(makeDependentWorkflow(), [ + { + operation_type: 'edit', + block_id: 'jira-1', + params: { inputs: { projectId: 'PROJECT-NEW' } }, + }, + ]) + + expect(state.blocks['jira-1'].subBlocks.projectId.value).toBe('PROJECT-NEW') + expect(state.blocks['jira-1'].subBlocks.issueKey.value).toBe('') + expect(state.blocks['jira-1'].subBlocks.transitionId.value).toBe('') + }) + + it('preserves explicitly supplied descendants and clears only their omitted descendants', () => { + const { state } = applyOperationsToWorkflowState(makeDependentWorkflow(), [ + { + operation_type: 'edit', + block_id: 'jira-1', + params: { + inputs: { + projectId: 'PROJECT-NEW', + issueKey: 'NEW-456', + }, + }, + }, + ]) + + expect(state.blocks['jira-1'].subBlocks.projectId.value).toBe('PROJECT-NEW') + expect(state.blocks['jira-1'].subBlocks.issueKey.value).toBe('NEW-456') + expect(state.blocks['jira-1'].subBlocks.transitionId.value).toBe('') + }) + + it('does not clear descendants when the submitted parent is unchanged', () => { + const { state } = applyOperationsToWorkflowState(makeDependentWorkflow(), [ + { + operation_type: 'edit', + block_id: 'jira-1', + params: { inputs: { projectId: 'PROJECT-OLD' } }, + }, + ]) + + expect(state.blocks['jira-1'].subBlocks.issueKey.value).toBe('OLD-123') + expect(state.blocks['jira-1'].subBlocks.transitionId.value).toBe('transition-old') + }) + + it('uses canonical advanced inputs as dependency changes', () => { + const { state } = applyOperationsToWorkflowState(makeDependentWorkflow(), [ + { + operation_type: 'edit', + block_id: 'jira-1', + params: { inputs: { manualProjectId: 'PROJECT-MANUAL' } }, + }, + ]) + + expect(state.blocks['jira-1'].subBlocks.manualProjectId.value).toBe('PROJECT-MANUAL') + expect(state.blocks['jira-1'].data.canonicalModes.projectId).toBe('advanced') + expect(state.blocks['jira-1'].subBlocks.issueKey.value).toBe('') + expect(state.blocks['jira-1'].subBlocks.transitionId.value).toBe('') + }) + + it('clears active manual descendants when their authoring context changes', () => { + const workflow = makeDependentWorkflow() + const jira = workflow.blocks['jira-1'] + jira.subBlocks.projectId.value = '' + jira.subBlocks.manualProjectId.value = 'PROJECT-MANUAL-OLD' + jira.subBlocks.issueKey.value = '' + jira.subBlocks.manualIssueKey.value = 'OLD-123' + jira.data.canonicalModes.projectId = 'advanced' + jira.data.canonicalModes.issueKey = 'advanced' + + const { state } = applyOperationsToWorkflowState(workflow, [ + { + operation_type: 'edit', + block_id: 'jira-1', + params: { inputs: { credential: 'credential-new' } }, + }, + ]) + + expect(state.blocks['jira-1'].subBlocks.manualProjectId.value).toBe('') + expect(state.blocks['jira-1'].subBlocks.manualIssueKey.value).toBe('') + expect(state.blocks['jira-1'].subBlocks.transitionId.value).toBe('') + }) + + it('replaces nested agent tool params instead of retaining omitted dependents', () => { + const workflow = { + blocks: { + 'agent-1': { + id: 'agent-1', + type: 'agent', + name: 'Agent 1', + position: { x: 0, y: 0 }, + enabled: true, + subBlocks: { + tools: { + id: 'tools', + type: 'tool-input', + value: [ + { + type: 'jira', + params: { projectId: 'PROJECT-OLD', issueKey: 'OLD-123' }, + }, + ], + }, + }, + outputs: {}, + data: {}, + }, + }, + edges: [], + loops: {}, + parallels: {}, + } + + const { state } = applyOperationsToWorkflowState(workflow, [ + { + operation_type: 'edit', + block_id: 'agent-1', + params: { + inputs: { + tools: [{ type: 'jira', params: { projectId: 'PROJECT-NEW' } }], + }, + }, + }, + ]) + + expect(state.blocks['agent-1'].subBlocks.tools.value[0].params).toEqual({ + projectId: 'PROJECT-NEW', + }) + }) +}) + describe('handleEditOperation nestedNodes merge', () => { it('preserves existing child block IDs when editing a loop with nestedNodes', () => { const workflow = makeLoopWorkflow() diff --git a/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/operations.ts b/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/operations.ts index eda45aa2e6c..e2e8bb65f9a 100644 --- a/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/operations.ts +++ b/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/operations.ts @@ -1,5 +1,9 @@ import { createLogger } from '@sim/logger' +import { isRecordLike } from '@sim/utils/object' +import { isEqual } from 'es-toolkit' import { isValidKey } from '@/lib/workflows/sanitization/key-validation' +import { getTransitiveSubBlockDependents } from '@/lib/workflows/subblocks/dependencies' +import { isNonEmptyValue } from '@/lib/workflows/subblocks/visibility' import { TriggerUtils } from '@/lib/workflows/triggers/triggers' import { getBlock } from '@/blocks/registry' import { normalizeName, RESERVED_BLOCK_NAMES } from '@/executor/constants' @@ -425,6 +429,14 @@ export function handleEditOperation(op: EditWorkflowOperation, ctx: OperationCon if (params?.inputs) { if (!block.subBlocks) block.subBlocks = {} + const previousSubBlockValues = new Map( + Object.entries(block.subBlocks).map(([key, subBlock]: [string, any]) => [ + key, + structuredClone(subBlock?.value), + ]) + ) + const explicitInputKeys = new Set() + // Validate inputs against block configuration const validationResult = validateInputsForBlock(block.type, params.inputs, block_id) validationErrors.push(...validationResult.errors) @@ -439,6 +451,7 @@ export function handleEditOperation(op: EditWorkflowOperation, ctx: OperationCon if (TRIGGER_RUNTIME_SUBBLOCK_IDS.includes(key)) { return } + explicitInputKeys.add(key) let sanitizedValue = normalizeSubblockValue(key, value) sanitizedValue = normalizeConditionRouterIds(block_id, key, sanitizedValue) @@ -467,12 +480,7 @@ export function handleEditOperation(op: EditWorkflowOperation, ctx: OperationCon } } else { const existingValue = block.subBlocks[key].value - const valuesEqual = - typeof existingValue === 'object' || typeof sanitizedValue === 'object' - ? JSON.stringify(existingValue) === JSON.stringify(sanitizedValue) - : existingValue === sanitizedValue - - if (!valuesEqual) { + if (!isEqual(existingValue, sanitizedValue)) { block.subBlocks[key].value = sanitizedValue } } @@ -481,9 +489,12 @@ export function handleEditOperation(op: EditWorkflowOperation, ctx: OperationCon if ( Object.hasOwn(params.inputs, 'triggerConfig') && block.subBlocks.triggerConfig && - typeof block.subBlocks.triggerConfig.value === 'object' + isRecordLike(block.subBlocks.triggerConfig.value) ) { applyTriggerConfigToBlockSubblocks(block, block.subBlocks.triggerConfig.value) + for (const key of Object.keys(block.subBlocks.triggerConfig.value)) { + explicitInputKeys.add(key) + } } // Update loop/parallel configuration in block.data (strict validation) @@ -538,11 +549,26 @@ export function handleEditOperation(op: EditWorkflowOperation, ctx: OperationCon const editBlockConfig = getBlock(block.type) if (editBlockConfig) { - updateCanonicalModesForInputs( - block, - Object.keys(validationResult.validInputs), - editBlockConfig - ) + updateCanonicalModesForInputs(block, [...explicitInputKeys], editBlockConfig) + + const changedInputKeys = editBlockConfig.subBlocks + .filter((subBlock) => { + const currentSubBlock = block.subBlocks[subBlock.id] + const existed = previousSubBlockValues.has(subBlock.id) + if (!existed || !currentSubBlock) return existed !== Boolean(currentSubBlock) + return !isEqual(previousSubBlockValues.get(subBlock.id), currentSubBlock.value) + }) + .map((subBlock) => subBlock.id) + + for (const { subBlockId } of getTransitiveSubBlockDependents( + editBlockConfig.subBlocks, + changedInputKeys + )) { + if (explicitInputKeys.has(subBlockId)) continue + const dependent = block.subBlocks[subBlockId] + if (!dependent || !isNonEmptyValue(dependent.value)) continue + dependent.value = '' + } } } diff --git a/apps/sim/lib/workflows/search-replace/dependencies.test.ts b/apps/sim/lib/workflows/search-replace/dependencies.test.ts deleted file mode 100644 index f1e2505fa75..00000000000 --- a/apps/sim/lib/workflows/search-replace/dependencies.test.ts +++ /dev/null @@ -1,24 +0,0 @@ -/** - * @vitest-environment node - */ -import { describe, expect, it } from 'vitest' -import { getWorkflowSearchDependentClears } from '@/lib/workflows/search-replace/dependencies' -import type { SubBlockConfig } from '@/blocks/types' - -describe('getWorkflowSearchDependentClears', () => { - it('returns transitive dependents without cycling', () => { - const subBlocks: SubBlockConfig[] = [ - { id: 'credential', title: 'Credential', type: 'oauth-input' }, - { id: 'project', title: 'Project', type: 'project-selector', dependsOn: ['credential'] }, - { id: 'issue', title: 'Issue', type: 'file-selector', dependsOn: ['project'] }, - { id: 'assignee', title: 'Assignee', type: 'user-selector', dependsOn: ['issue'] }, - { id: 'unrelated', title: 'Unrelated', type: 'short-input' }, - ] - - expect(getWorkflowSearchDependentClears(subBlocks, 'credential')).toEqual([ - { subBlockId: 'project', reason: 'project depends on credential' }, - { subBlockId: 'issue', reason: 'issue depends on project' }, - { subBlockId: 'assignee', reason: 'assignee depends on issue' }, - ]) - }) -}) diff --git a/apps/sim/lib/workflows/search-replace/dependencies.ts b/apps/sim/lib/workflows/search-replace/dependencies.ts deleted file mode 100644 index 5059ebc29f2..00000000000 --- a/apps/sim/lib/workflows/search-replace/dependencies.ts +++ /dev/null @@ -1,33 +0,0 @@ -import type { SubBlockConfig } from '@/blocks/types' -import { getSubBlocksDependingOnChange } from '@/blocks/utils' - -export interface DependentClear { - subBlockId: string - reason: string -} - -export function getWorkflowSearchDependentClears( - allSubBlocks: SubBlockConfig[], - changedSubBlockId: string -): DependentClear[] { - const clears: DependentClear[] = [] - const visited = new Set([changedSubBlockId]) - const queue = [changedSubBlockId] - - while (queue.length > 0) { - const currentSubBlockId = queue.shift() - if (!currentSubBlockId) continue - - for (const subBlock of getSubBlocksDependingOnChange(allSubBlocks, currentSubBlockId)) { - if (!subBlock.id || visited.has(subBlock.id)) continue - visited.add(subBlock.id) - clears.push({ - subBlockId: subBlock.id, - reason: `${subBlock.id} depends on ${currentSubBlockId}`, - }) - queue.push(subBlock.id) - } - } - - return clears -} diff --git a/apps/sim/lib/workflows/search-replace/indexer.ts b/apps/sim/lib/workflows/search-replace/indexer.ts index 25c2e3ca7b3..5fc1a9ffdfc 100644 --- a/apps/sim/lib/workflows/search-replace/indexer.ts +++ b/apps/sim/lib/workflows/search-replace/indexer.ts @@ -3,7 +3,6 @@ import { DEFAULT_SUBBLOCK_TYPE } from '@sim/workflow-persistence/subblocks' import type { SubBlockType } from '@sim/workflow-types/blocks' import { isWorkflowBlockProtected } from '@sim/workflow-types/workflow' import { COMPARISON_OPERATORS, LOGICAL_OPERATORS } from '@/lib/table/query-builder/constants' -import { getWorkflowSearchDependentClears } from '@/lib/workflows/search-replace/dependencies' import { getSearchableJsonStringLeaves, isSearchableJsonValueSubBlock, @@ -26,6 +25,7 @@ import type { } from '@/lib/workflows/search-replace/types' import { pathToKey, walkStringValues } from '@/lib/workflows/search-replace/value-walker' import { SELECTOR_CONTEXT_FIELDS } from '@/lib/workflows/subblocks/context' +import { getTransitiveSubBlockDependents } from '@/lib/workflows/subblocks/dependencies' import { resolveStoredToolName } from '@/lib/workflows/subblocks/display' import { buildCanonicalIndex, @@ -784,7 +784,7 @@ export function getToolInputParamConfigs({ ) const allToolSubBlocks = blockConfig?.subBlocks ?? subBlocksResult.subBlocks const getDependentValuePaths = (changedSubBlockId: string): WorkflowSearchValuePath[] => - getWorkflowSearchDependentClears(allToolSubBlocks, changedSubBlockId).map((clear) => [ + getTransitiveSubBlockDependents(allToolSubBlocks, [changedSubBlockId]).map((clear) => [ 'params', clear.subBlockId, ]) diff --git a/apps/sim/lib/workflows/subblocks/dependencies.test.ts b/apps/sim/lib/workflows/subblocks/dependencies.test.ts new file mode 100644 index 00000000000..0a8842ea976 --- /dev/null +++ b/apps/sim/lib/workflows/subblocks/dependencies.test.ts @@ -0,0 +1,137 @@ +/** + * @vitest-environment node + */ +import { describe, expect, it } from 'vitest' +import { + getDependsOnFields, + getSubBlocksDependingOnChange, + getTransitiveSubBlockDependents, +} from '@/lib/workflows/subblocks/dependencies' +import type { SubBlockConfig } from '@/blocks/types' + +describe('getDependsOnFields', () => { + it('returns an empty array when dependsOn is unset', () => { + expect(getDependsOnFields(undefined)).toEqual([]) + }) + + it('returns array dependencies unchanged', () => { + expect(getDependsOnFields(['credential', 'projectId'])).toEqual(['credential', 'projectId']) + }) + + it('flattens all and any dependencies', () => { + expect(getDependsOnFields({ all: ['credential'], any: ['teamId', 'manualTeamId'] })).toEqual([ + 'credential', + 'teamId', + 'manualTeamId', + ]) + }) +}) + +describe('getSubBlocksDependingOnChange', () => { + it('finds direct dependents of a changed subblock', () => { + const subBlocks: SubBlockConfig[] = [ + { id: 'provider', title: 'Provider', type: 'dropdown' }, + { id: 'model', title: 'Model', type: 'dropdown', dependsOn: ['provider'] }, + { id: 'prompt', title: 'Prompt', type: 'long-input' }, + ] + + expect( + getSubBlocksDependingOnChange(subBlocks, 'provider').map((subBlock) => subBlock.id) + ).toEqual(['model']) + }) + + it('matches dependents through canonical basic and advanced siblings', () => { + const subBlocks: SubBlockConfig[] = [ + { + id: 'channel', + title: 'Channel', + type: 'channel-selector', + canonicalParamId: 'channelId', + mode: 'basic', + }, + { + id: 'manualChannel', + title: 'Channel ID', + type: 'short-input', + canonicalParamId: 'channelId', + mode: 'advanced', + }, + { + id: 'messageId', + title: 'Message ID', + type: 'short-input', + dependsOn: ['channelId'], + }, + { + id: 'threadTs', + title: 'Thread Timestamp', + type: 'short-input', + dependsOn: ['otherField'], + }, + ] + + expect( + getSubBlocksDependingOnChange(subBlocks, 'manualChannel').map((subBlock) => subBlock.id) + ).toEqual(['messageId']) + expect( + getSubBlocksDependingOnChange(subBlocks, 'channel').map((subBlock) => subBlock.id) + ).toEqual(['messageId']) + }) + + it('matches object-form dependencies when any listed dependency changes', () => { + const subBlocks: SubBlockConfig[] = [ + { id: 'credential', title: 'Credential', type: 'oauth-input' }, + { id: 'teamId', title: 'Team', type: 'short-input' }, + { + id: 'projectId', + title: 'Project', + type: 'short-input', + dependsOn: { all: ['credential'], any: ['teamId'] }, + }, + ] + + expect( + getSubBlocksDependingOnChange(subBlocks, 'credential').map((subBlock) => subBlock.id) + ).toEqual(['projectId']) + expect( + getSubBlocksDependingOnChange(subBlocks, 'teamId').map((subBlock) => subBlock.id) + ).toEqual(['projectId']) + }) +}) + +describe('getTransitiveSubBlockDependents', () => { + it('returns transitive dependents without cycling', () => { + const subBlocks: SubBlockConfig[] = [ + { id: 'credential', title: 'Credential', type: 'oauth-input' }, + { id: 'project', title: 'Project', type: 'project-selector', dependsOn: ['credential'] }, + { id: 'issue', title: 'Issue', type: 'file-selector', dependsOn: ['project'] }, + { id: 'assignee', title: 'Assignee', type: 'user-selector', dependsOn: ['issue'] }, + { id: 'unrelated', title: 'Unrelated', type: 'short-input' }, + ] + + expect(getTransitiveSubBlockDependents(subBlocks, ['credential'])).toEqual([ + { subBlockId: 'project', reason: 'project depends on credential' }, + { subBlockId: 'issue', reason: 'issue depends on project' }, + { subBlockId: 'assignee', reason: 'assignee depends on issue' }, + ]) + }) + + it('walks multiple changed roots once', () => { + const subBlocks: SubBlockConfig[] = [ + { id: 'credential', title: 'Credential', type: 'oauth-input' }, + { id: 'domain', title: 'Domain', type: 'short-input' }, + { + id: 'project', + title: 'Project', + type: 'project-selector', + dependsOn: ['credential', 'domain'], + }, + { id: 'issue', title: 'Issue', type: 'file-selector', dependsOn: ['project'] }, + ] + + expect(getTransitiveSubBlockDependents(subBlocks, ['credential', 'domain'])).toEqual([ + { subBlockId: 'project', reason: 'project depends on credential' }, + { subBlockId: 'issue', reason: 'issue depends on project' }, + ]) + }) +}) diff --git a/apps/sim/lib/workflows/subblocks/dependencies.ts b/apps/sim/lib/workflows/subblocks/dependencies.ts new file mode 100644 index 00000000000..59f2030c0b9 --- /dev/null +++ b/apps/sim/lib/workflows/subblocks/dependencies.ts @@ -0,0 +1,66 @@ +import { buildCanonicalIndex } from '@/lib/workflows/subblocks/visibility' +import type { SubBlockConfig } from '@/blocks/types' + +export interface DependentSubBlock { + subBlockId: string + reason: string +} + +/** Flattens array and all/any dependency declarations into their referenced field IDs. */ +export function getDependsOnFields(dependsOn: SubBlockConfig['dependsOn']): string[] { + if (!dependsOn) return [] + if (Array.isArray(dependsOn)) return dependsOn + return [...(dependsOn.all || []), ...(dependsOn.any || [])] +} + +/** Finds direct dependents while treating canonical basic/advanced siblings as one field. */ +export function getSubBlocksDependingOnChange( + allSubBlocks: SubBlockConfig[], + changedSubBlockId: string +): SubBlockConfig[] { + const canonicalIndex = buildCanonicalIndex(allSubBlocks) + const canonicalId = canonicalIndex.canonicalIdBySubBlockId[changedSubBlockId] + const group = canonicalId ? canonicalIndex.groupsById[canonicalId] : undefined + const changedFields = new Set([changedSubBlockId]) + + if (canonicalId) changedFields.add(canonicalId) + if (group?.basicId) changedFields.add(group.basicId) + for (const advancedId of group?.advancedIds || []) { + changedFields.add(advancedId) + } + + return allSubBlocks.filter((subBlock) => + getDependsOnFields(subBlock.dependsOn).some((field) => changedFields.has(field)) + ) +} + +/** + * Returns every transitive `dependsOn` descendant of the changed subblocks. + * Canonical basic/advanced siblings are treated as one logical field by the + * shared block dependency resolver. + */ +export function getTransitiveSubBlockDependents( + allSubBlocks: SubBlockConfig[], + changedSubBlockIds: Iterable +): DependentSubBlock[] { + const dependents: DependentSubBlock[] = [] + const visited = new Set(changedSubBlockIds) + const queue = [...visited] + + while (queue.length > 0) { + const currentSubBlockId = queue.shift() + if (!currentSubBlockId) continue + + for (const subBlock of getSubBlocksDependingOnChange(allSubBlocks, currentSubBlockId)) { + if (!subBlock.id || visited.has(subBlock.id)) continue + visited.add(subBlock.id) + dependents.push({ + subBlockId: subBlock.id, + reason: `${subBlock.id} depends on ${currentSubBlockId}`, + }) + queue.push(subBlock.id) + } + } + + return dependents +}