diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/tool-input.tsx b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/tool-input.tsx index 6807d20e44b..9b28cfb9797 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/tool-input.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/tool-input.tsx @@ -533,6 +533,8 @@ export const ToolInput = memo(function ToolInput({ for (const [toolIndex, tool] of selectedTools.entries()) { const blockConfig = allBlocks.find((b: { type: string }) => b.type === tool.type) if (!blockConfig?.subBlocks) continue + // canonical-index-unscoped: a nested tool resolves against `tool.params`, which only ever + // holds action-surface values — a tool is never invoked in trigger mode. const toolCanonical = buildCanonicalIndex(blockConfig.subBlocks) const scopedOverrides = scopeCanonicalModesForTool( canonicalModeOverrides, @@ -1779,7 +1781,8 @@ export const ToolInput = memo(function ToolInput({ : null const toolCanonicalIndex: CanonicalIndex | null = toolBlock?.subBlocks - ? buildCanonicalIndex(toolBlock.subBlocks) + ? // canonical-index-unscoped: nested tool params are always the action surface + buildCanonicalIndex(toolBlock.subBlocks) : null const toolContextValues = toolCanonicalIndex diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/hooks/use-canonical-sub-block-value.ts b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/hooks/use-canonical-sub-block-value.ts index 3cb5fc25bfc..9bec924dcd2 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/hooks/use-canonical-sub-block-value.ts +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/hooks/use-canonical-sub-block-value.ts @@ -1,7 +1,10 @@ import { useCallback, useMemo } from 'react' import { isEqual } from 'es-toolkit' import { useStoreWithEqualityFn } from 'zustand/traditional' -import { buildCanonicalIndex, resolveDependencyValue } from '@/lib/workflows/subblocks/visibility' +import { + buildCanonicalIndexForSurface, + resolveDependencyValue, +} from '@/lib/workflows/subblocks/visibility' import { getBlock } from '@/blocks/registry' import { useWorkflowRegistry } from '@/stores/workflows/registry/store' import { useSubBlockStore } from '@/stores/workflows/subblock/store' @@ -22,9 +25,10 @@ export function useCanonicalSubBlockValue( const activeWorkflowId = useWorkflowRegistry((s) => s.activeWorkflowId) const blockState = useWorkflowStore((state) => state.blocks[blockId]) const blockConfig = blockState?.type ? getBlock(blockState.type) : null + const triggerSurface = blockState?.triggerMode === true const canonicalIndex = useMemo( - () => buildCanonicalIndex(blockConfig?.subBlocks || []), - [blockConfig?.subBlocks] + () => buildCanonicalIndexForSurface(blockConfig?.subBlocks || [], triggerSurface), + [blockConfig?.subBlocks, triggerSurface] ) const canonicalModeOverrides = blockState?.data?.canonicalModes diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/hooks/use-depends-on-gate.ts b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/hooks/use-depends-on-gate.ts index 54576b7c819..142b45339ac 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/hooks/use-depends-on-gate.ts +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/hooks/use-depends-on-gate.ts @@ -4,7 +4,7 @@ import { useCallback, useMemo } from 'react' import { isEqual } from 'es-toolkit' import { useStoreWithEqualityFn } from 'zustand/traditional' import { - buildCanonicalIndex, + buildCanonicalIndexForSurface, isNonEmptyValue, normalizeDependencyValue, parseDependsOn, @@ -41,9 +41,15 @@ export function useDependsOnGate( : blockState?.type ? getBlock(blockState.type) : null + /** + * A nested tool's params are always the ACTION surface — `dependencyBlockType` means + * `blockConfig` describes the tool, not the host block, so the host's trigger mode says + * nothing about which of the tool's fields are live. + */ + const triggerSurface = !dependencyBlockType && blockState?.triggerMode === true const canonicalIndex = useMemo( - () => buildCanonicalIndex(blockConfig?.subBlocks || []), - [blockConfig?.subBlocks] + () => buildCanonicalIndexForSurface(blockConfig?.subBlocks || [], triggerSurface), + [blockConfig?.subBlocks, triggerSurface] ) const canonicalModeOverrides = blockState?.data?.canonicalModes diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/editor.tsx b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/editor.tsx index d536009b0dd..fb4c2b6ed31 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/editor.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/editor.tsx @@ -23,11 +23,11 @@ import { isRetryEligibleBlock } from '@/lib/workflows/blocks/retry-eligibility' import { buildCanonicalIndex, evaluateSubBlockCondition, + getCanonicalSubBlocksForSurface, hasAdvancedValues, isCanonicalPair, isStandaloneAdvancedMode, resolveCanonicalMode, - shouldUseSubBlockForTriggerModeCanonicalIndex, } from '@/lib/workflows/subblocks/visibility' import { useUserPermissionsContext } from '@/app/workspace/[workspaceId]/providers/workspace-permissions-provider' import { @@ -157,11 +157,10 @@ export function Editor() { isEqual ) - const subBlocksForCanonical = useMemo(() => { - const subBlocks = blockConfig?.subBlocks || [] - if (!triggerMode) return subBlocks - return subBlocks.filter(shouldUseSubBlockForTriggerModeCanonicalIndex) - }, [blockConfig?.subBlocks, triggerMode]) + const subBlocksForCanonical = useMemo( + () => getCanonicalSubBlocksForSurface(blockConfig?.subBlocks || [], triggerMode), + [blockConfig?.subBlocks, triggerMode] + ) const canonicalIndex = useMemo( () => buildCanonicalIndex(subBlocksForCanonical), diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/hooks/use-editor-subblock-layout.ts b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/hooks/use-editor-subblock-layout.ts index 6ca9b69470d..d9feec0800e 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/hooks/use-editor-subblock-layout.ts +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/hooks/use-editor-subblock-layout.ts @@ -1,13 +1,12 @@ import { useCallback, useMemo } from 'react' import { - buildCanonicalIndex, + buildCanonicalIndexForSurface, evaluateSubBlockCondition, isSubBlockFeatureEnabled, isSubBlockHidden, isSubBlockVisibleForMode, isSubBlockVisibleForTriggerMode, isToolInputOnlySubBlock, - shouldUseSubBlockForTriggerModeCanonicalIndex, } from '@/lib/workflows/subblocks/visibility' import type { BlockConfig, SubBlockConfig } from '@/blocks/types' import { usePermissionConfig } from '@/hooks/use-permission-config' @@ -48,7 +47,8 @@ export function useEditorSubblockLayout( config?.subBlocks || [], blockId, activeWorkflowId, - blockDataFromStore?.canonicalModes + blockDataFromStore?.canonicalModes, + displayTriggerMode ) return useMemo(() => { @@ -102,10 +102,7 @@ export function useEditorSubblockLayout( {} ) - const subBlocksForCanonical = displayTriggerMode - ? (config.subBlocks || []).filter(shouldUseSubBlockForTriggerModeCanonicalIndex) - : config.subBlocks || [] - const canonicalIndex = buildCanonicalIndex(subBlocksForCanonical) + const canonicalIndex = buildCanonicalIndexForSurface(config.subBlocks || [], displayTriggerMode) const effectiveAdvanced = displayAdvancedMode const canonicalModeOverrides = blockData?.canonicalModes 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 bcda9aeb776..b2fa0c090e2 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 @@ -65,6 +65,7 @@ import { } from '@/lib/workflows/subblocks/display' import { buildCanonicalIndex, + buildCanonicalIndexForSurface, hasAdvancedValues, resolveDependencyValue, } from '@/lib/workflows/subblocks/visibility' @@ -805,14 +806,18 @@ export const WorkflowBlock = memo(function WorkflowBlock({ ]) } - const canonicalIndex = useMemo(() => buildCanonicalIndex(config.subBlocks), [config.subBlocks]) + const canonicalIndex = useMemo( + () => buildCanonicalIndexForSurface(config.subBlocks, displayTriggerMode), + [config.subBlocks, displayTriggerMode] + ) const canonicalModeOverrides = currentStoreBlock?.data?.canonicalModes const hiddenByReactiveCondition = useReactiveConditions( config.subBlocks, id, activeWorkflowId, - canonicalModeOverrides + canonicalModeOverrides, + displayTriggerMode ) const subBlockRowsData = useMemo(() => { @@ -859,7 +864,6 @@ export const WorkflowBlock = memo(function WorkflowBlock({ const displayableSubBlocks = getCardSubBlocks(config, { advanced: effectiveAdvanced, values: rawValues, - canonicalIndex, canonicalModeOverrides, triggerMode: effectiveTrigger, hiddenIds: hiddenByReactiveCondition, diff --git a/apps/sim/app/workspace/[workspaceId]/w/components/preview/components/preview-editor/preview-editor.tsx b/apps/sim/app/workspace/[workspaceId]/w/components/preview/components/preview-editor/preview-editor.tsx index 6bb5e293f16..a16ea864787 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/components/preview/components/preview-editor/preview-editor.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/components/preview/components/preview-editor/preview-editor.tsx @@ -30,7 +30,7 @@ import { useParams } from 'next/navigation' import { ReactFlowProvider } from 'reactflow' import { extractReferencePrefixes } from '@/lib/workflows/sanitization/references' import { - buildCanonicalIndex, + buildCanonicalIndexForSurface, evaluateSubBlockCondition, hasAdvancedValues, isSubBlockFeatureEnabled, @@ -1055,9 +1055,10 @@ function PreviewEditorContent({ }, {}) }, [subBlockValues]) + const effectiveTrigger = block.triggerMode === true const canonicalIndex = useMemo( - () => buildCanonicalIndex(blockConfig?.subBlocks || []), - [blockConfig?.subBlocks] + () => buildCanonicalIndexForSurface(blockConfig?.subBlocks || [], effectiveTrigger), + [blockConfig?.subBlocks, effectiveTrigger] ) const isSubflow = block.type === 'loop' || block.type === 'parallel' @@ -1118,7 +1119,6 @@ function PreviewEditorContent({ hasAdvancedValues(blockConfig.subBlocks, rawValues, canonicalIndex) const isPureTriggerBlock = blockConfig.triggers?.enabled && blockConfig.category === 'triggers' - const effectiveTrigger = block.triggerMode === true const visibleSubBlocks = blockConfig.subBlocks.filter((subBlock) => { if (subBlock.hidden || subBlock.hideFromPreview) return false diff --git a/apps/sim/app/workspace/[workspaceId]/w/components/preview/components/preview-workflow/components/block/block.tsx b/apps/sim/app/workspace/[workspaceId]/w/components/preview/components/preview-workflow/components/block/block.tsx index a83ce1bba88..399f9b53c06 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/components/preview/components/preview-workflow/components/block/block.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/components/preview/components/preview-workflow/components/block/block.tsx @@ -28,7 +28,7 @@ import { resolveWorkflowSelectionLabel, } from '@/lib/workflows/subblocks/display' import { - buildCanonicalIndex, + buildCanonicalIndexForSurface, evaluateSubBlockCondition, isSubBlockFeatureEnabled, isSubBlockVisibleForMode, @@ -237,10 +237,11 @@ function WorkflowPreviewBlockInner({ data }: NodeProps } = data const blockConfig = getBlock(type) + const effectiveTrigger = isTrigger || type === 'starter' const canonicalIndex = useMemo( - () => buildCanonicalIndex(blockConfig?.subBlocks || []), - [blockConfig?.subBlocks] + () => buildCanonicalIndexForSurface(blockConfig?.subBlocks || [], effectiveTrigger), + [blockConfig?.subBlocks, effectiveTrigger] ) const rawValues = useMemo(() => { @@ -267,7 +268,6 @@ function WorkflowPreviewBlockInner({ data }: NodeProps if (!blockConfig?.subBlocks) return [] const isPureTriggerBlock = blockConfig.triggers?.enabled && blockConfig.category === 'triggers' - const effectiveTrigger = isTrigger || type === 'starter' return blockConfig.subBlocks.filter((subBlock) => { if (subBlock.hidden) return false @@ -308,8 +308,7 @@ function WorkflowPreviewBlockInner({ data }: NodeProps blockConfig?.subBlocks, blockConfig?.triggers?.enabled, blockConfig?.category, - type, - isTrigger, + effectiveTrigger, canonicalIndex, rawValues, canvasPresentation, @@ -348,7 +347,6 @@ function WorkflowPreviewBlockInner({ data }: NodeProps * lightweight mode, which has no values to resolve chips from. */ const sentenceSegments = useMemo(() => { - const effectiveTrigger = isTrigger || type === 'starter' if (lightweight || !blockConfig) return null if (type === 'condition' || type === 'router_v2' || type === 'starter') return null @@ -374,7 +372,7 @@ function WorkflowPreviewBlockInner({ data }: NodeProps (subBlockId) => availableIds.has(subBlockId), (subBlockId) => onCardById.get(subBlockId) ?? null ) - }, [lightweight, blockConfig, type, isTrigger, visibleSubBlocks, onCardById, rawValues]) + }, [lightweight, blockConfig, type, effectiveTrigger, visibleSubBlocks, onCardById, rawValues]) /** * Compute condition rows for condition blocks. diff --git a/apps/sim/ee/workspace-forking/lib/copy/copy-workflows.ts b/apps/sim/ee/workspace-forking/lib/copy/copy-workflows.ts index 0ecb17df411..11696d24ee5 100644 --- a/apps/sim/ee/workspace-forking/lib/copy/copy-workflows.ts +++ b/apps/sim/ee/workspace-forking/lib/copy/copy-workflows.ts @@ -520,11 +520,21 @@ export async function copyWorkflowStateIntoTarget( let activeCanonicalModes: CanonicalModeOverrides | undefined = ( block.data as { canonicalModes?: Record } | undefined )?.canonicalModes + // A mixed action/trigger block shares one `canonicalModes` key across both surfaces, so the + // remap has to know which surface is live: without it a trigger field reads as a dormant + // member of the action pair and the remap clears the value. + const blockTriggerMode = block.triggerMode === true if (transformSubBlocks) { - subBlocks = transformSubBlocks(subBlocks, block.type, activeCanonicalModes, (next) => { - activeCanonicalModes = next - updatedData = { ...updatedData, canonicalModes: next } as BlockData - }) + subBlocks = transformSubBlocks( + subBlocks, + block.type, + activeCanonicalModes, + (next) => { + activeCanonicalModes = next + updatedData = { ...updatedData, canonicalModes: next } as BlockData + }, + blockTriggerMode + ) } if (varIdMapping.size > 0) { subBlocks = remapVariableIdsInSubBlocks(subBlocks, varIdMapping) @@ -565,7 +575,8 @@ export async function copyWorkflowStateIntoTarget( block.name, targetCurrent.subBlocks, subBlocks, - activeCanonicalModes + activeCanonicalModes, + blockTriggerMode ) ) } 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 d311a4160a0..f0401b4c9ad 100644 --- a/apps/sim/ee/workspace-forking/lib/mapping/dependent-reconfigs.ts +++ b/apps/sim/ee/workspace-forking/lib/mapping/dependent-reconfigs.ts @@ -135,6 +135,8 @@ function emitAnchoredDependents(params: EmitAnchoredParams): void { }) const scanSubBlocks = getSelectorContextSubBlocks(config.subBlocks, values, triggerMode) const canonicalIndex = buildCanonicalIndex(scanSubBlocks) + // canonical-index-unscoped: `scanSubBlocks` is already narrowed to the active surface by + // `getSelectorContextSubBlocks` above, so scoping again here would be a no-op. const gates = createCanonicalModeGates(scanSubBlocks, values, canonicalModes) const configById = new Map(scanSubBlocks.filter((cfg) => cfg.id).map((cfg) => [cfg.id, cfg])) // Shared with `applyDependentOverrides`, so what the modal offers is exactly what the sync diff --git a/apps/sim/ee/workspace-forking/lib/promote/cleared-refs.ts b/apps/sim/ee/workspace-forking/lib/promote/cleared-refs.ts index 71d1a4fe2b7..d777af432a0 100644 --- a/apps/sim/ee/workspace-forking/lib/promote/cleared-refs.ts +++ b/apps/sim/ee/workspace-forking/lib/promote/cleared-refs.ts @@ -89,7 +89,8 @@ function baseSubBlockId(key: string): string { function collectForkWorkflowReferences( subBlocks: SubBlockRecord, config: ReturnType, - canonicalModes: CanonicalModeOverrides | undefined + canonicalModes: CanonicalModeOverrides | undefined, + triggerMode: boolean ): Array<{ workflowId: string; subBlockKey: string }> { const out: Array<{ workflowId: string; subBlockKey: string }> = [] // Collapse each canonical pair to its ACTIVE member and skip condition-hidden fields: only a @@ -103,7 +104,8 @@ function collectForkWorkflowReferences( const gates = createCanonicalModeGates( config?.subBlocks, buildSubBlockValues(subBlocks), - canonicalModes + canonicalModes, + triggerMode ) const detectionSkipped = (key: string) => gates.isDormantMember(key) || gates.isConditionHidden(key) @@ -199,6 +201,7 @@ export function collectForkClearedRefCandidates( blockName: blockLabel, blockType: block.type, canonicalModes: block.data?.canonicalModes, + triggerMode: block.triggerMode === true, }) for (const ref of scan.unmapped) { if (CLEARED_REF_EXCLUDED_KINDS.has(ref.kind)) continue @@ -245,7 +248,8 @@ export function collectForkClearedRefCandidates( for (const wfRef of collectForkWorkflowReferences( subBlocks, config, - block.data?.canonicalModes + block.data?.canonicalModes, + block.triggerMode === true )) { if (workflowIdMap.has(wfRef.workflowId)) continue out.push({ @@ -397,7 +401,8 @@ function hasForkSyncBlockerCandidates( const workflowRefs = collectForkWorkflowReferences( subBlocks, getBlock(block.type), - block.data?.canonicalModes + block.data?.canonicalModes, + block.triggerMode === true ) if (workflowRefs.some((ref) => !workflowIdMap.has(ref.workflowId))) return true } diff --git a/apps/sim/ee/workspace-forking/lib/remap/fork-bootstrap.ts b/apps/sim/ee/workspace-forking/lib/remap/fork-bootstrap.ts index cba64544532..48ac7f9f67e 100644 --- a/apps/sim/ee/workspace-forking/lib/remap/fork-bootstrap.ts +++ b/apps/sim/ee/workspace-forking/lib/remap/fork-bootstrap.ts @@ -22,7 +22,7 @@ export type ForkCopyResolver = (kind: ForkRemapKind, sourceId: string) => string * the child defines the key). */ export function createForkBootstrapTransform(resolveCopied: ForkCopyResolver): SubBlockTransform { - return (subBlocks, blockType, canonicalModes, onCanonicalModesChanged) => { + return (subBlocks, blockType, canonicalModes, onCanonicalModesChanged, triggerMode) => { // Every resolution at fork-create IS a copy (the resolver is the copy id map), so all // remapped keys carry copy provenance - copy-faithful dependents (column picks) survive. // `blockType`/`canonicalModes` activate the mode policy: active basic remaps, active @@ -30,6 +30,7 @@ export function createForkBootstrapTransform(resolveCopied: ForkCopyResolver): S const result = remapForkSubBlocks(subBlocks, resolveCopied, 'create', { blockType, canonicalModes, + triggerMode, isCopiedTarget: (kind, sourceId) => resolveCopied(kind, sourceId) != null, }) if (result.canonicalModes) onCanonicalModesChanged?.(result.canonicalModes) @@ -38,7 +39,8 @@ export function createForkBootstrapTransform(resolveCopied: ForkCopyResolver): S blockType, result.remappedKeys, result.canonicalModes ?? canonicalModes, - result.copyRemappedKeys + result.copyRemappedKeys, + triggerMode ) } } diff --git a/apps/sim/ee/workspace-forking/lib/remap/reference-scan.ts b/apps/sim/ee/workspace-forking/lib/remap/reference-scan.ts index cf0e9190cbb..80c493319b8 100644 --- a/apps/sim/ee/workspace-forking/lib/remap/reference-scan.ts +++ b/apps/sim/ee/workspace-forking/lib/remap/reference-scan.ts @@ -12,6 +12,7 @@ interface ScannerBlock { type: string subBlocks: unknown canonicalModes?: CanonicalModeOverrides + triggerMode?: boolean } /** @@ -45,6 +46,7 @@ export function toScannerBlocks(state: WorkflowState): ScannerBlock[] { type: block.type, subBlocks: block.subBlocks as unknown, canonicalModes: block.data?.canonicalModes, + triggerMode: block.triggerMode, })) } 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 c703eeafce1..dc5bbd80d9a 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 @@ -35,6 +35,7 @@ import { applyDependentOverrides, clearDependentsOnRemap, collectClearedDependents, + createCanonicalModeGates, createForkSubBlockTransform, type ForkReferenceResolver, parseNestedDependentKey, @@ -777,6 +778,118 @@ describe('clearDependentsOnRemap canonical-pair gating', () => { }) }) +describe('canonical-mode gates on a mixed action/trigger block', () => { + /** + * Webflow's shape: an action pair plus a trigger alias sharing one `canonicalParamId` under a + * DIFFERENT id. Both surfaces live in one `subBlocks` array and share one `canonicalModes` key. + */ + const mixedSurfaceBlock = () => + blockWith([ + { + id: 'siteSelector', + title: 'Site', + type: 'project-selector', + canonicalParamId: 'siteId', + mode: 'basic', + }, + { + id: 'manualSiteId', + title: 'Site ID', + type: 'short-input', + canonicalParamId: 'siteId', + mode: 'advanced', + }, + { + id: 'triggerSiteId', + title: 'Site', + type: 'dropdown', + canonicalParamId: 'siteId', + mode: 'trigger', + }, + ]) + + const values = { + siteSelector: '', + manualSiteId: 'stale-manual-site', + triggerSiteId: 'site-live', + } + + it('does not call a live trigger field dormant when the shared mode is advanced', () => { + vi.mocked(getBlock).mockReturnValue(mixedSurfaceBlock()) + const config = getBlock('webflow') as BlockConfig + // Configured as an action with the manual Site ID, then switched to trigger mode. The mode key + // is shared, so unscoped the trigger field reads as a dormant member of the action pair — and + // a fork CLEARS dormant members, silently wiping the trigger's configured site. + const gates = createCanonicalModeGates(config.subBlocks, values, { siteId: 'advanced' }, true) + expect(gates.isDormantMember('triggerSiteId')).toBe(false) + expect(gates.isActiveManualMember('triggerSiteId')).toBe(false) + }) + + it('leaves the DORMANT action surface classified exactly as before scoping', () => { + vi.mocked(getBlock).mockReturnValue(mixedSurfaceBlock()) + const config = getBlock('webflow') as BlockConfig + const scoped = createCanonicalModeGates(config.subBlocks, values, { siteId: 'advanced' }, true) + + // Scoping decides membership for LIVE fields only. The action surface's own values are still + // real keys in the block's value map, and the remap loop reads `isDormantMember` to decide + // both whether to clear a value and whether to skip detecting it. Answering "not a member" + // here would stop clearing them AND start detecting them, turning a stale action selector on + // a trigger-mode block into a mapping requirement that can block a sync. + expect(scoped.isDormantMember('siteSelector')).toBe(true) + expect(scoped.isActiveManualMember('manualSiteId')).toBe(true) + + // Identical to what the unscoped gates answered for those same keys before the fix. + const legacy = createCanonicalModeGates(config.subBlocks, values, { siteId: 'advanced' }, false) + for (const key of ['siteSelector', 'manualSiteId']) { + expect(scoped.isDormantMember(key)).toBe(legacy.isDormantMember(key)) + expect(scoped.isActiveManualMember(key)).toBe(legacy.isActiveManualMember(key)) + } + }) + + it('does not turn a dormant action credential into a detected reference', () => { + vi.mocked(getBlock).mockReturnValue(mixedSurfaceBlock()) + const subBlocks: SubBlockRecord = { + siteSelector: { type: 'project-selector', value: 'source-workspace-site' }, + manualSiteId: { type: 'short-input', value: 'stale-manual-site' }, + triggerSiteId: { type: 'dropdown', value: 'site-live' }, + } + const result = remapForkSubBlocks(subBlocks, () => null, 'promote', { + blockType: 'webflow', + canonicalModes: { siteId: 'advanced' }, + triggerMode: true, + }) + // The dormant basic member is cleared and never becomes a promote blocker, exactly as it did + // before surface scoping — while the live trigger field survives. + expect(result.subBlocks.siteSelector.value).toBe('') + expect(result.unmapped.some((ref) => ref.subBlockKey === 'siteSelector')).toBe(false) + expect(result.subBlocks.triggerSiteId.value).toBe('site-live') + }) + + it('still gates the action surface normally', () => { + vi.mocked(getBlock).mockReturnValue(mixedSurfaceBlock()) + const config = getBlock('webflow') as BlockConfig + const gates = createCanonicalModeGates(config.subBlocks, values, { siteId: 'advanced' }, false) + // Basic is dormant while advanced is active; the manual member is the live one. + expect(gates.isDormantMember('siteSelector')).toBe(true) + expect(gates.isActiveManualMember('manualSiteId')).toBe(true) + }) + + it("keeps a trigger-mode block's live field through the fork remap", () => { + vi.mocked(getBlock).mockReturnValue(mixedSurfaceBlock()) + const subBlocks: SubBlockRecord = { + siteSelector: { type: 'project-selector', value: '' }, + manualSiteId: { type: 'short-input', value: 'stale-manual-site' }, + triggerSiteId: { type: 'dropdown', value: 'site-live' }, + } + const result = remapForkSubBlocks(subBlocks, () => null, 'create', { + blockType: 'webflow', + canonicalModes: { siteId: 'advanced' }, + triggerMode: true, + }) + expect(result.subBlocks.triggerSiteId.value).toBe('site-live') + }) +}) + describe('scanWorkflowReferences canonical-pair detection', () => { const credBlock = () => blockWith([ 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 13ceef044d1..916acd6ecdf 100644 --- a/apps/sim/ee/workspace-forking/lib/remap/remap-references.ts +++ b/apps/sim/ee/workspace-forking/lib/remap/remap-references.ts @@ -26,6 +26,7 @@ import { buildSubBlockValues, type CanonicalModeOverrides, evaluateSubBlockCondition, + getCanonicalSubBlocksForSurface, isCanonicalPair, isNonEmptyValue, reindexCanonicalModesByPosition, @@ -228,7 +229,9 @@ export type SubBlockTransform = ( subBlocks: SubBlockRecord, blockType: string, canonicalModes?: CanonicalModeOverrides, - onCanonicalModesChanged?: (next: CanonicalModeOverrides) => void + onCanonicalModesChanged?: (next: CanonicalModeOverrides) => void, + /** The block's trigger mode, scoping the canonical index (see {@link createCanonicalModeGates}). */ + triggerMode?: boolean ) => SubBlockRecord /** @@ -451,28 +454,63 @@ const NO_GATES: CanonicalModeGates = { * augmented with each pair's ACTIVE value under its canonical id, mirroring how the serializer * exposes params to conditions. With no configs (unknown block type) every gate is a no-op: * everything is detected and nothing passes through, the conservative default. + * + * `triggerSurface` scopes the index to the block's active surface. Without it, a trigger field + * sharing a `canonicalParamId` with an action pair (`triggerSiteId` under `siteId`, + * `triggerCredentials` under `oauthCredential`) is read as a member of THAT pair, and since it is + * neither its `basicId` nor in its `advancedIds`, `isDormantMember` answers `true` the moment the + * shared mode resolves to advanced — which a fork acts on by CLEARING the value. Pass a caller + * that has already narrowed its configs (the dependent scan) `false`; scoping twice is harmless + * but the flag should describe what the caller actually did. */ export function createCanonicalModeGates( configSubBlocks: SubBlockConfig[] | undefined, values: Record, - canonicalModes?: CanonicalModeOverrides + canonicalModes?: CanonicalModeOverrides, + triggerSurface = false ): CanonicalModeGates { if (!configSubBlocks || configSubBlocks.length === 0) return NO_GATES - const canonicalIndex = buildCanonicalIndex(configSubBlocks) + const surfaceSubBlocks = getCanonicalSubBlocksForSurface(configSubBlocks, triggerSurface) + const canonicalIndex = buildCanonicalIndex(surfaceSubBlocks) + // canonical-index-unscoped: the fallback for keys the ACTIVE surface does not define — see + // `indexFor`. Scoping decides membership for live fields only; a dormant surface's own values + // keep the classification they had before scoping existed. + const fullIndex = buildCanonicalIndex(configSubBlocks) const configByBaseKey = new Map( configSubBlocks.filter((cfg) => cfg.id).map((cfg) => [cfg.id, cfg]) ) const conditionValues = { ...values } - for (const [canonicalId, group] of Object.entries(canonicalIndex.groupsById)) { - if (conditionValues[canonicalId] === undefined) { - conditionValues[canonicalId] = resolveActiveCanonicalValue(group, values, canonicalModes) + for (const index of [canonicalIndex, fullIndex]) { + for (const [canonicalId, group] of Object.entries(index.groupsById)) { + if (conditionValues[canonicalId] === undefined) { + conditionValues[canonicalId] = resolveActiveCanonicalValue(group, values, canonicalModes) + } } } + /** + * The index that owns a key. + * + * The scoped index answers for anything the active surface defines — that is the fix: a trigger + * field sharing a `canonicalParamId` with an action pair gets its OWN group instead of being + * read as a stranded member of the action pair's. + * + * Everything else falls back to the whole array, deliberately. A dormant surface's values are + * still real keys in the block's value map, and the remap loop reads `isDormantMember` to decide + * both whether to CLEAR a value and whether to skip detecting it as a reference. Answering + * "not a member" for them would stop clearing them AND start detecting them, turning a stale + * action selector on a trigger-mode block into a mapping requirement that can block a sync. + * Scoping is meant to stop live fields being misread, not to re-classify dormant ones. + */ + const indexFor = (key: string) => + canonicalIndex.canonicalIdBySubBlockId[key] || canonicalIndex.groupsById[key] + ? canonicalIndex + : fullIndex + const groupFor = (memberOrCanonicalId: string) => { - const canonicalId = - canonicalIndex.canonicalIdBySubBlockId[memberOrCanonicalId] ?? memberOrCanonicalId - const group = canonicalIndex.groupsById[canonicalId] + const index = indexFor(memberOrCanonicalId) + const canonicalId = index.canonicalIdBySubBlockId[memberOrCanonicalId] ?? memberOrCanonicalId + const group = index.groupsById[canonicalId] return group && isCanonicalPair(group) ? group : undefined } const baseKeyOf = (subBlockKey: string) => subBlockKey.replace(/_\d+$/, '') @@ -487,7 +525,7 @@ export function createCanonicalModeGates( isDormantMember: (subBlockKey) => { const baseKey = baseKeyOf(subBlockKey) const group = groupFor(baseKey) - if (!group || !canonicalIndex.canonicalIdBySubBlockId[baseKey]) return false + if (!group || !indexFor(baseKey).canonicalIdBySubBlockId[baseKey]) return false return isAdvancedActiveGroup(baseKey) !== group.advancedIds.includes(baseKey) }, isActiveManualMember: (subBlockKey) => { @@ -521,6 +559,12 @@ export interface RemapForkContext { blockType?: string /** Canonical-mode overrides (`block.data.canonicalModes`), picking the active member per pair. */ canonicalModes?: CanonicalModeOverrides + /** + * Whether the block is in TRIGGER mode, scoping the canonical index to that surface. A mixed + * action/trigger block shares one mode key across both surfaces, so without this a trigger + * field reads as a dormant member of the action pair and its value is cleared. + */ + triggerMode?: boolean /** Target MCP server row lookup for rewriting remapped tool-input entries' server metadata. */ resolveMcpServerMeta?: ForkMcpServerMetaResolver /** @@ -626,6 +670,7 @@ export function remapToolBlockResources( tool.type ) const toolBlockSubBlocks = (opts.blockConfigs?.[tool.type] ?? getBlock(tool.type))?.subBlocks + // canonical-index-unscoped: a nested tool's params are always the action surface const gates = createCanonicalModeGates(toolBlockSubBlocks, toolValues, scopedModes) // Clear DORMANT member keys first: a stale inactive value must not survive the copy (and must @@ -1047,7 +1092,8 @@ export function remapForkSubBlocks( const gates = createCanonicalModeGates( context?.blockType ? getBlock(context.blockType)?.subBlocks : undefined, buildSubBlockValues(subBlocks), - context?.canonicalModes + context?.canonicalModes, + context?.triggerMode === true ) for (const [subBlockKey, subBlock] of Object.entries(subBlocks)) { @@ -1276,7 +1322,9 @@ export function clearDependentsOnRemap( remappedKeys: ReadonlySet, canonicalModes?: CanonicalModeOverrides, /** Keys remapped via a COPY (see {@link RemapSubBlocksResult.copyRemappedKeys}). */ - copyRemappedKeys?: ReadonlySet + copyRemappedKeys?: ReadonlySet, + /** The block's trigger mode, scoping the canonical index (see {@link createCanonicalModeGates}). */ + triggerMode?: boolean ): SubBlockRecord { if (remappedKeys.size === 0) return subBlocks const config = getBlock(blockType) @@ -1290,7 +1338,8 @@ export function clearDependentsOnRemap( const gates = createCanonicalModeGates( config.subBlocks, buildSubBlockValues(subBlocks), - canonicalModes + canonicalModes, + triggerMode === true ) // The exemption's parent test: an mcp-server selector whose POST-remap value is non-empty was @@ -1433,6 +1482,7 @@ function collectClearedToolParamDependents( // A DORMANT canonical member's cleared slot is not a lost configuration (only the pair's // active member executes). Modes resolve like the tool-input UI: tool-scoped overrides, // then the value heuristic over the merged params. + // canonical-index-unscoped: a nested tool's params are always the action surface const gates = createCanonicalModeGates( toolConfig.subBlocks, mergedValues, @@ -1498,7 +1548,9 @@ export function collectClearedDependents( blockName: string, targetCurrentSubBlocks: SubBlockRecord, mergedSubBlocks: SubBlockRecord, - canonicalModes?: CanonicalModeOverrides + canonicalModes?: CanonicalModeOverrides, + /** The block's trigger mode, scoping the canonical index (see {@link createCanonicalModeGates}). */ + triggerMode?: boolean ): NeedsConfigurationField[] { const config = getBlock(blockType) if (!config) return [] @@ -1506,7 +1558,12 @@ export function collectClearedDependents( const mergedValues = buildSubBlockValues(mergedSubBlocks) // A DORMANT canonical member the merge cleared is not a lost configuration - only the pair's // active member executes, so an inactive slot must never demand a re-pick. - const gates = createCanonicalModeGates(config.subBlocks, mergedValues, canonicalModes) + const gates = createCanonicalModeGates( + config.subBlocks, + mergedValues, + canonicalModes, + triggerMode === true + ) const fields: NeedsConfigurationField[] = [] for (const cfg of config.subBlocks) { if (!cfg.id) continue @@ -1755,10 +1812,11 @@ export function createForkSubBlockTransform( isCopiedTarget?: (kind: ForkRemapKind, sourceId: string) => boolean } ): SubBlockTransform { - return (subBlocks, blockType, canonicalModes, onCanonicalModesChanged) => { + return (subBlocks, blockType, canonicalModes, onCanonicalModesChanged, triggerMode) => { const result = remapSubBlocks(subBlocks, resolve, { blockType, canonicalModes, + triggerMode, resolveMcpServerMeta: options?.resolveMcpServerMeta, isCopiedTarget: options?.isCopiedTarget, }) @@ -1768,7 +1826,8 @@ export function createForkSubBlockTransform( blockType, result.remappedKeys, result.canonicalModes ?? canonicalModes, - result.copyRemappedKeys + result.copyRemappedKeys, + triggerMode ) } } @@ -1792,6 +1851,8 @@ export function scanWorkflowReferences( subBlocks: unknown /** `block.data.canonicalModes`, picking the active member per canonical pair for detection. */ canonicalModes?: CanonicalModeOverrides + /** The block's trigger mode, scoping the canonical index (see {@link createCanonicalModeGates}). */ + triggerMode?: boolean }>, resolve: ForkReferenceResolver ): WorkflowReferenceScan { @@ -1822,6 +1883,7 @@ export function scanWorkflowReferences( blockName: block.name, blockType: block.type, canonicalModes: block.canonicalModes, + triggerMode: block.triggerMode, }) for (const reference of blockResult.references) { const key = `${reference.kind}:${reference.sourceId}` diff --git a/apps/sim/hooks/use-reactive-conditions.ts b/apps/sim/hooks/use-reactive-conditions.ts index d5323a8b85f..3cf2933c785 100644 --- a/apps/sim/hooks/use-reactive-conditions.ts +++ b/apps/sim/hooks/use-reactive-conditions.ts @@ -1,6 +1,9 @@ import { useCallback, useMemo } from 'react' import type { CanonicalModeOverrides } from '@/lib/workflows/subblocks/visibility' -import { buildCanonicalIndex, resolveDependencyValue } from '@/lib/workflows/subblocks/visibility' +import { + buildCanonicalIndexForSurface, + resolveDependencyValue, +} from '@/lib/workflows/subblocks/visibility' import type { SubBlockConfig } from '@/blocks/types' import { useWorkspaceCredential } from '@/hooks/queries/credentials' import { EMPTY_BLOCK_SUBBLOCK_VALUES, useSubBlockStore } from '@/stores/workflows/subblock/store' @@ -15,12 +18,22 @@ export function useReactiveConditions( subBlocks: SubBlockConfig[], blockId: string, activeWorkflowId: string | null, - canonicalModeOverrides?: CanonicalModeOverrides + canonicalModeOverrides?: CanonicalModeOverrides, + triggerSurface = false ): Set { const reactiveSubBlock = useMemo(() => subBlocks.find((sb) => sb.reactiveCondition), [subBlocks]) const reactiveCond = reactiveSubBlock?.reactiveCondition - const canonicalIndex = useMemo(() => buildCanonicalIndex(subBlocks), [subBlocks]) + /** + * Scoped so a trigger-mode block watches its own credential. The only shipped reactive + * condition (`SERVICE_ACCOUNT_SUBBLOCKS`) watches `oauthCredential`, which on Gmail, Drive, + * Sheets, Forms and Calendar spans both surfaces under different ids — unscoped, trigger mode + * resolves it to the dormant action credential and fetches the wrong one. + */ + const canonicalIndex = useMemo( + () => buildCanonicalIndexForSurface(subBlocks, triggerSurface), + [subBlocks, triggerSurface] + ) // Resolve watchFields through canonical index to get the active credential value const watchedCredentialId = useSubBlockStore( diff --git a/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/builders.ts b/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/builders.ts index 218f82b83b0..28fd029958a 100644 --- a/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/builders.ts +++ b/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/builders.ts @@ -263,6 +263,8 @@ export function updateCanonicalModesForInputs( ): void { if (!blockConfig.subBlocks?.length) return + // canonical-index-unscoped: structural only — this maps written input ids to the mode they + // imply and reads no values, so neither surface can shadow the other. const canonicalIndex = buildCanonicalIndex(blockConfig.subBlocks) const canonicalModeUpdates: Record = {} diff --git a/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/validation.ts b/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/validation.ts index 7d87fbdba63..da3890beecc 100644 --- a/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/validation.ts +++ b/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/validation.ts @@ -11,6 +11,7 @@ import { getSkillById } from '@/lib/workflows/skills/operations' import { buildCanonicalIndex, buildSubBlockValues, + getCanonicalSubBlocksForSurface, isCanonicalPair, resolveCanonicalMode, } from '@/lib/workflows/subblocks/visibility' @@ -1034,11 +1035,18 @@ function collectSelectorFields( const blockConfig = getBlock(blockType) if (!blockConfig) continue - const canonicalIndex = buildCanonicalIndex(blockConfig.subBlocks) + // Scoped to the block's active surface: a trigger field sharing a `canonicalParamId` with an + // action pair matches neither of its members, so an unscoped index skipped every trigger + // selector as "inactive" while still validating the dormant action ones. + const activeSubBlocks = getCanonicalSubBlocksForSurface( + blockConfig.subBlocks, + blockData.triggerMode === true + ) + const canonicalIndex = buildCanonicalIndex(activeSubBlocks) const allValues = buildSubBlockValues(blockData.subBlocks || {}) const canonicalModeOverrides = blockData.data?.canonicalModes - for (const subBlockConfig of blockConfig.subBlocks) { + for (const subBlockConfig of activeSubBlocks) { if (!SELECTOR_TYPES.has(subBlockConfig.type)) continue // oauth-input credentials are only validated when explicitly requested diff --git a/apps/sim/lib/webhooks/deploy.ts b/apps/sim/lib/webhooks/deploy.ts index 7882f04d709..16843b7ce44 100644 --- a/apps/sim/lib/webhooks/deploy.ts +++ b/apps/sim/lib/webhooks/deploy.ts @@ -209,6 +209,8 @@ export function buildProviderConfig( Object.entries(block.subBlocks || {}).map(([key, value]) => [key, { value: value.value }]) ) + // canonical-index-unscoped: a trigger DEFINITION's subblocks are the trigger surface by + // construction — this never sees the host block's action fields. const canonicalIndex = buildCanonicalIndex(triggerDef.subBlocks) const satisfiedCanonicalIds = new Set() const filledSubBlockIds = new Set() diff --git a/apps/sim/lib/workflows/autolayout/utils.ts b/apps/sim/lib/workflows/autolayout/utils.ts index 9bb11f18835..bccedd3cdd6 100644 --- a/apps/sim/lib/workflows/autolayout/utils.ts +++ b/apps/sim/lib/workflows/autolayout/utils.ts @@ -28,7 +28,7 @@ import { calculateWorkflowBlockDimensions } from '@/lib/workflows/blocks/determi import { getConditionRows, getRouterRows } from '@/lib/workflows/dynamic-handle-topology' import { getDisplayValue, hasDisplayableRowValue } from '@/lib/workflows/subblocks/display' import { - buildCanonicalIndex, + buildCanonicalIndexForSurface, buildSubBlockValues, type CanonicalModeOverrides, evaluateSubBlockCondition, @@ -198,9 +198,9 @@ function getVisiblePreviewSubBlocks(block: BlockState): { rawValues.__canonicalModes = canonicalModeOverrides } - const canonicalIndex = buildCanonicalIndex(blockConfig.subBlocks) const effectiveAdvanced = Boolean(block.advancedMode) const effectiveTrigger = Boolean(block.triggerMode) + const canonicalIndex = buildCanonicalIndexForSurface(blockConfig.subBlocks, effectiveTrigger) const isPureTriggerBlock = blockConfig.triggers?.enabled && blockConfig.category === 'triggers' const visibleSubBlocks = blockConfig.subBlocks.filter((subBlock) => { diff --git a/apps/sim/lib/workflows/blocks/canvas-card-fields.test.ts b/apps/sim/lib/workflows/blocks/canvas-card-fields.test.ts new file mode 100644 index 00000000000..a8a744854d6 --- /dev/null +++ b/apps/sim/lib/workflows/blocks/canvas-card-fields.test.ts @@ -0,0 +1,111 @@ +/** + * @vitest-environment node + * + * The card's subblock set, checked against every real block rather than a fixture. + * + * `@/blocks/registry` is globally mocked for import cost, so these read `BLOCK_REGISTRY` + * directly — the point of the sweep is that it sees the actual shipped block definitions. + */ +import { describe, expect, it } from 'vitest' +import { getCardSubBlocks } from '@/lib/workflows/blocks/canvas-card-fields' +import { + buildCanonicalIndex, + isCanonicalPair, + shouldUseSubBlockForTriggerModeCanonicalIndex, +} from '@/lib/workflows/subblocks/visibility' +import { BLOCK_REGISTRY } from '@/blocks/registry-maps' +import type { BlockConfig } from '@/blocks/types' + +/** + * Blocks that are an action AND a trigger. Both surfaces live in one `subBlocks` array — the + * block's own fields, then its trigger's spread in after them — which is what makes the two + * able to interfere. + */ +const MIXED_SURFACE_BLOCKS = Object.values(BLOCK_REGISTRY).filter( + (block) => block.triggers?.enabled && block.category !== 'triggers' +) + +/** + * A trigger-mode block with every trigger field filled, carrying the block-creation default + * (`buildDefaultCanonicalModes`) of `'basic'` for every canonical pair. + */ +function triggerModeState(block: BlockConfig) { + const values: Record = {} + for (const subBlock of block.subBlocks) { + if (shouldUseSubBlockForTriggerModeCanonicalIndex(subBlock)) { + values[subBlock.id] = 'configured-value' + } + } + // A block offering several triggers conditions its fields on which one the user picked. + const firstTrigger = block.triggers?.available?.[0] + if (firstTrigger) values.selectedTriggerId = firstTrigger + + const canonicalModeOverrides: Record = {} + for (const group of Object.values(buildCanonicalIndex(block.subBlocks).groupsById)) { + if (isCanonicalPair(group)) canonicalModeOverrides[group.canonicalId] = 'basic' + } + return { values, canonicalModeOverrides } +} + +function triggerCardIds(block: BlockConfig, subBlocks = block.subBlocks): string[] { + const { values, canonicalModeOverrides } = triggerModeState(block) + return getCardSubBlocks( + { subBlocks, category: block.category, triggers: block.triggers }, + { advanced: false, values, canonicalModeOverrides, triggerMode: true } + ).map((subBlock) => subBlock.id) +} + +describe('getCardSubBlocks', () => { + it('finds mixed action/trigger blocks to check', () => { + expect(MIXED_SURFACE_BLOCKS.length).toBeGreaterThan(0) + }) + + /** + * The invariant: a trigger card is a function of the trigger surface ALONE. Dropping the + * block's action fields — which trigger mode never renders anyway — must not change what the + * card shows. + * + * It used to, because the card indexed canonical groups over the whole array. The two surfaces + * collide in both directions: by shared `canonicalParamId` under different ids (Webflow's + * `triggerSiteId` joining the action `siteId` pair, eight blocks' `triggerCredentials` joining + * `oauthCredential`) and by shared id (Airtable's trigger `baseId`/`tableId` inheriting the + * action pair's group). Either way the trigger field matched neither the group's `basicId` nor + * its `advancedIds`, so `isSubBlockVisibleForMode` dropped it — while the editor panel, which + * did scope its index, showed the same field. Users configured fields the canvas then refused + * to display, and the autolayout height estimate lost the rows too. + * + * Comparing against the same function on a reduced config, rather than a hand-written expected + * set, is deliberate: a second model of "what is on a card" is what this module exists to + * prevent, and it would drift the moment an unrelated filter changed. + */ + describe.each(MIXED_SURFACE_BLOCKS.map((block) => [block.type, block] as const))( + '%s in trigger mode', + (_type, block) => { + it('shows the same fields whether or not the action surface is present', () => { + const triggerOnly = block.subBlocks.filter(shouldUseSubBlockForTriggerModeCanonicalIndex) + expect(triggerCardIds(block)).toEqual(triggerCardIds(block, triggerOnly)) + }) + } + ) + + it('keeps a canonical pair on the trigger surface collapsed to its active member', () => { + // Google Calendar's trigger owns a real pair (`calendarId` + `trigger-advanced` + // `manualCalendarId`), so scoping must not flatten it into two visible rows. + const googleCalendar = BLOCK_REGISTRY.google_calendar + const onCard = triggerCardIds(googleCalendar) + expect(onCard).toContain('calendarId') + expect(onCard).not.toContain('manualCalendarId') + }) + + it('shows a trigger field whose canonical id is also an action pair', () => { + const onCard = triggerCardIds(BLOCK_REGISTRY.webflow) + expect(onCard).toEqual(expect.arrayContaining(['triggerCredentials', 'triggerSiteId'])) + }) + + it('shows a trigger field whose id is also an action pair member', () => { + // Airtable's trigger declares plain `baseId`/`tableId`, ids the action surface already uses + // as the advanced members of its `baseId`/`tableId` pairs. + const onCard = triggerCardIds(BLOCK_REGISTRY.airtable) + expect(onCard).toEqual(expect.arrayContaining(['baseId', 'tableId'])) + }) +}) diff --git a/apps/sim/lib/workflows/blocks/canvas-card-fields.ts b/apps/sim/lib/workflows/blocks/canvas-card-fields.ts index a5a32cca877..8dfb5de1260 100644 --- a/apps/sim/lib/workflows/blocks/canvas-card-fields.ts +++ b/apps/sim/lib/workflows/blocks/canvas-card-fields.ts @@ -1,5 +1,5 @@ import { - type CanonicalIndex, + buildCanonicalIndexForSurface, type CanonicalModeOverrides, evaluateSubBlockCondition, isSubBlockFeatureEnabled, @@ -29,9 +29,15 @@ export interface CardFieldsOptions { advanced: boolean /** Current subblock values, for conditions and canonical mode resolution. */ values: Record - canonicalIndex: CanonicalIndex canonicalModeOverrides?: CanonicalModeOverrides - /** The card is showing the block as a trigger, which swaps the subblock set. */ + /** + * The card is showing the block as a trigger, which swaps the subblock set — and with it the + * canonical index, which this derives rather than accepts. A caller that supplied its own could + * hand in one built for the other surface, and every caller did: a mixed action/trigger block's + * trigger fields all collapse into the action pairs they share a `canonicalParamId` with, so + * `isSubBlockVisibleForMode` matched none of them and dropped Airtable's Base ID and Table ID, + * Webflow's Site and Collection, and eight blocks' trigger Credentials off the card entirely. + */ triggerMode?: boolean /** Ids an async reactive condition has hidden; runtime-only, empty in checks. */ hiddenIds?: ReadonlySet @@ -75,13 +81,13 @@ export function getCardSubBlocks( const { advanced, values, - canonicalIndex, canonicalModeOverrides, triggerMode = false, hiddenIds, titleOperationSubBlockId, } = options + const canonicalIndex = buildCanonicalIndexForSurface(config.subBlocks, triggerMode) const isPureTriggerBlock = Boolean(config.triggers?.enabled && config.category === 'triggers') return config.subBlocks.filter((subBlock) => { diff --git a/apps/sim/lib/workflows/blocks/canvas-sentence-render.ts b/apps/sim/lib/workflows/blocks/canvas-sentence-render.ts index e5c1d47471c..99183cd32a3 100644 --- a/apps/sim/lib/workflows/blocks/canvas-sentence-render.ts +++ b/apps/sim/lib/workflows/blocks/canvas-sentence-render.ts @@ -3,7 +3,6 @@ import { getSeededSubBlockValues, } from '@/lib/workflows/blocks/canvas-card-fields' import { type CardSelector, resolveCanvasSentence } from '@/lib/workflows/blocks/canvas-sentence' -import { buildCanonicalIndex } from '@/lib/workflows/subblocks/visibility' import type { BlockConfig, SubBlockConfig } from '@/blocks/types' /** @@ -76,7 +75,6 @@ export function renderSentenceReadings( for (const subBlock of getCardSubBlocks(config, { advanced: false, values, - canonicalIndex: buildCanonicalIndex(config.subBlocks), triggerMode: card.mode === 'trigger', })) { if (!onCardById.has(subBlock.id)) onCardById.set(subBlock.id, subBlock) diff --git a/apps/sim/lib/workflows/blocks/canvas-sentence-validation.ts b/apps/sim/lib/workflows/blocks/canvas-sentence-validation.ts index 7351f45a8a2..7f0ce41f925 100644 --- a/apps/sim/lib/workflows/blocks/canvas-sentence-validation.ts +++ b/apps/sim/lib/workflows/blocks/canvas-sentence-validation.ts @@ -228,6 +228,8 @@ function buildBlockIndex(config: ValidatableBlockConfig): BlockIndex { return { subBlocks: config.subBlocks, byId: groupSubBlocksById(config.subBlocks), + // canonical-index-unscoped: `resolveVisibility` returns `hidden` for every trigger-mode + // subblock as its first check, so only action subblocks ever reach this index. canonical: buildCanonicalIndex(config.subBlocks), seededValues: getSeededSubBlockValues(config), } diff --git a/apps/sim/lib/workflows/migrations/subblock-migrations.ts b/apps/sim/lib/workflows/migrations/subblock-migrations.ts index 62aa01aa471..f6edd10576e 100644 --- a/apps/sim/lib/workflows/migrations/subblock-migrations.ts +++ b/apps/sim/lib/workflows/migrations/subblock-migrations.ts @@ -535,6 +535,9 @@ export function backfillCanonicalModes(blocks: Record): { continue } + // canonical-index-unscoped: the backfill writes a mode only for canonical PAIRS, whose two + // members always sit on the same surface — a cross-surface alias joins an existing group + // rather than forming a pair, so scoping cannot change what gets backfilled. const canonicalIndex = buildCanonicalIndex(blockConfig.subBlocks) const pairs = Object.values(canonicalIndex.groupsById).filter(isCanonicalPair) if (pairs.length === 0) { diff --git a/apps/sim/lib/workflows/search-replace/indexer.ts b/apps/sim/lib/workflows/search-replace/indexer.ts index af65cacffb0..8eb9e60b37e 100644 --- a/apps/sim/lib/workflows/search-replace/indexer.ts +++ b/apps/sim/lib/workflows/search-replace/indexer.ts @@ -30,6 +30,7 @@ import { getTransitiveSubBlockDependents } from '@/lib/workflows/subblocks/depen import { resolveStoredToolName } from '@/lib/workflows/subblocks/display' import { buildCanonicalIndex, + buildCanonicalIndexForSurface, buildSubBlockValues, type CanonicalModeOverrides, evaluateSubBlockCondition, @@ -41,7 +42,6 @@ import { parseDependsOn, resolveDependencyValue, scopeCanonicalModesForTool, - shouldUseSubBlockForTriggerModeCanonicalIndex, } from '@/lib/workflows/subblocks/visibility' import { isSyntheticToolSubBlockId } from '@/lib/workflows/tool-input/synthetic-subblocks' import { type ParsedStoredTool, parseStoredToolInputValue } from '@/lib/workflows/tool-input/types' @@ -790,6 +790,7 @@ export function getToolInputParamConfigs({ }) } + // canonical-index-unscoped: a nested tool's params are always the action surface const toolCanonicalIndex = buildCanonicalIndex( blockConfig?.subBlocks ?? subBlocksResult.subBlocks ) @@ -1287,11 +1288,11 @@ export function indexWorkflowSearchMatches( for (const block of Object.values(workflow.blocks)) { const blockConfig = blockConfigs[block.type] ?? getBlock(block.type) const subBlockConfigs = blockConfig?.subBlocks ?? [] - const canonicalSubBlockConfigs = block.triggerMode - ? subBlockConfigs.filter(shouldUseSubBlockForTriggerModeCanonicalIndex) - : subBlockConfigs const configsById = new Map(subBlockConfigs.map((subBlock) => [subBlock.id, subBlock])) - const canonicalIndex = buildCanonicalIndex(canonicalSubBlockConfigs) + const canonicalIndex = buildCanonicalIndexForSurface( + subBlockConfigs, + Boolean(block.triggerMode) + ) const subBlockValues = buildSubBlockValues(block.subBlocks ?? {}) const canonicalModes = getSearchCanonicalModes(block) const protectedByLock = isWorkflowBlockProtected(block.id, workflow.blocks) diff --git a/apps/sim/lib/workflows/subblocks/visibility.test.ts b/apps/sim/lib/workflows/subblocks/visibility.test.ts index 8d6a87224b1..da470499192 100644 --- a/apps/sim/lib/workflows/subblocks/visibility.test.ts +++ b/apps/sim/lib/workflows/subblocks/visibility.test.ts @@ -2,8 +2,11 @@ * @vitest-environment node */ import { describe, expect, it } from 'vitest' +import type { SubBlockConfig } from '@/blocks/types' import { + buildCanonicalIndexForSurface, evaluateSubBlockCondition, + getCanonicalSubBlocksForSurface, reindexToolCanonicalModes, scopeCanonicalModesForTool, } from './visibility' @@ -237,6 +240,21 @@ describe('scopeCanonicalModesForTool', () => { expect(scopeCanonicalModesForTool(overrides, 0, 'table')).toEqual({ tableId: 'basic' }) }) + it.concurrent('keeps legacy modes for canonical ids the user has not re-toggled', () => { + // Toggles are written one key at a time, so the first toggle on a legacy tool leaves a map + // holding both formats. Returning only the index-scoped side reverted every canonical id the + // user had not yet touched back to basic. + const overrides = { + 'table:tableId': 'advanced' as const, + 'table:conflictColumn': 'advanced' as const, + '0:conflictColumn': 'basic' as const, + } + expect(scopeCanonicalModesForTool(overrides, 0, 'table')).toEqual({ + tableId: 'advanced', + conflictColumn: 'basic', + }) + }) + it.concurrent('does not fall back when no legacyToolType is given', () => { expect(scopeCanonicalModesForTool({ 'table:tableId': 'advanced' }, 0)).toBeUndefined() }) @@ -326,3 +344,48 @@ describe('reindexToolCanonicalModes', () => { expect(result).toBeUndefined() }) }) + +describe('canonical index scoping by surface', () => { + /** Webflow's shape: an action pair and a trigger alias sharing one `canonicalParamId`. */ + const MIXED: SubBlockConfig[] = [ + { id: 'siteSelector', type: 'dropdown', canonicalParamId: 'siteId', mode: 'basic' }, + { id: 'manualSiteId', type: 'short-input', canonicalParamId: 'siteId', mode: 'advanced' }, + { id: 'triggerSiteId', type: 'dropdown', canonicalParamId: 'siteId', mode: 'trigger' }, + ] as SubBlockConfig[] + + it.concurrent('keeps the whole array on the action surface', () => { + expect(getCanonicalSubBlocksForSurface(MIXED, false)).toBe(MIXED) + }) + + it.concurrent('keeps only trigger members on the trigger surface', () => { + expect(getCanonicalSubBlocksForSurface(MIXED, true).map((s) => s.id)).toEqual(['triggerSiteId']) + }) + + it.concurrent('makes the trigger alias its own group rather than a stranded member', () => { + // Unscoped, `triggerSiteId` joins the action pair and matches neither side of it, so every + // group-relative question about it answers for the dormant surface. + const unscoped = buildCanonicalIndexForSurface(MIXED, false).groupsById.siteId + expect(unscoped.basicId).toBe('siteSelector') + expect(unscoped.advancedIds).toEqual(['manualSiteId']) + + const scoped = buildCanonicalIndexForSurface(MIXED, true).groupsById.siteId + expect(scoped.basicId).toBe('triggerSiteId') + expect(scoped.advancedIds).toEqual([]) + }) + + it.concurrent('preserves a pair that lives entirely on the trigger surface', () => { + const triggerPair: SubBlockConfig[] = [ + { id: 'calendarId', type: 'dropdown', canonicalParamId: 'calId', mode: 'trigger' }, + { + id: 'manualCalendarId', + type: 'short-input', + canonicalParamId: 'calId', + mode: 'trigger-advanced', + }, + ] as SubBlockConfig[] + + const group = buildCanonicalIndexForSurface(triggerPair, true).groupsById.calId + expect(group.basicId).toBe('calendarId') + expect(group.advancedIds).toEqual(['manualCalendarId']) + }) +}) diff --git a/apps/sim/lib/workflows/subblocks/visibility.ts b/apps/sim/lib/workflows/subblocks/visibility.ts index c5fc887f086..c7924bd8532 100644 --- a/apps/sim/lib/workflows/subblocks/visibility.ts +++ b/apps/sim/lib/workflows/subblocks/visibility.ts @@ -124,6 +124,45 @@ export function buildCanonicalIndex(subBlocks: SubBlockConfig[]): CanonicalIndex return { groupsById, canonicalIdBySubBlockId } } +/** + * The subblocks that define a block's canonical groups on the surface it is being rendered or + * resolved on. + * + * A block that is both an action and a trigger holds ONE `subBlocks` array: its own fields plus + * its trigger's, spread in after them. Those two sets routinely share a `canonicalParamId` while + * using DIFFERENT ids — Webflow's `siteSelector`/`manualSiteId` (action) and `triggerSiteId` + * (trigger) are all `siteId`. Indexed together they collapse into one group whose `basicId` + * belongs to the other surface, so the trigger member matches neither `basicId` nor `advancedIds` + * and every group-relative question about it answers wrong: {@link isSubBlockVisibleForMode} hides + * it outright, and {@link resolveDependencyValue} answers with the dormant surface's stale value. + * + * The serializer is deliberately exempt and keeps the unscoped index: `shouldSerializeSubBlock` + * drops the inactive surface's members BEFORE the canonical collapse reads them, so it resolves + * against a value map the dormant surface cannot appear in. That filter-then-resolve ordering is + * the whole reason execution has always been correct here. Every other caller resolves against the + * block's FULL value map, so for them the scoping has to live in the index instead. + * + * Only the trigger surface is filtered. The action surface keeps the whole array because a trigger + * member is already excluded by each caller's own trigger-mode filter, and because dropping it + * would also drop the `canonicalIdBySubBlockId` entry that lets a legacy alias still resolve + * through {@link resolveDependencyValue}. Mirrors `getSelectorContextSubBlocks`. + */ +export function getCanonicalSubBlocksForSurface( + subBlocks: SubBlockConfig[], + triggerSurface: boolean +): SubBlockConfig[] { + if (!triggerSurface) return subBlocks + return subBlocks.filter(shouldUseSubBlockForTriggerModeCanonicalIndex) +} + +/** {@link buildCanonicalIndex} over {@link getCanonicalSubBlocksForSurface}'s active set. */ +export function buildCanonicalIndexForSurface( + subBlocks: SubBlockConfig[], + triggerSurface: boolean +): CanonicalIndex { + return buildCanonicalIndex(getCanonicalSubBlocksForSurface(subBlocks, triggerSurface)) +} + /** * Resolve if a canonical group is a swap pair (basic + advanced). */ @@ -281,10 +320,15 @@ function extractPrefixedModes( * `type` — so that two tool entries of the SAME type (e.g. two Table tools on one Agent block) get * independent canonical modes instead of colliding on a shared `${toolType}:${canonicalId}` key. * - * Falls back to the legacy `${legacyToolType}:` prefix (the pre-instance-scoping format) when no - * index-scoped key matches, so an override saved before this scoping change isn't silently dropped - - * it keeps applying (type-shared, the old behavior) until the user re-toggles it explicitly, at which - * point it's rewritten under the new index-scoped key. + * The legacy `${legacyToolType}:` prefix (the pre-instance-scoping format) is the BASELINE, with + * index-scoped entries layered over it per canonical id, so an override saved before this scoping + * change isn't silently dropped - it keeps applying (type-shared, the old behavior) until the user + * re-toggles that specific canonical id, at which point the new index-scoped key wins for it alone. + * + * Merging per key rather than preferring one map wholesale is what keeps a PARTIALLY re-toggled + * tool intact. Toggles are written one key at a time (`setBlockCanonicalMode`), so the first toggle + * on a legacy tool produces a map holding both formats; returning only the index-scoped side there + * would silently revert every canonical id the user had not yet re-toggled back to basic. * * Returns `undefined` when there are no overrides, no `toolIndex`, and no legacy match. */ @@ -296,8 +340,9 @@ export function scopeCanonicalModesForTool( if (!overrides) return undefined const scoped = toolIndex !== undefined ? extractPrefixedModes(overrides, `${toolIndex}:`) : undefined - if (scoped) return scoped - return legacyToolType ? extractPrefixedModes(overrides, `${legacyToolType}:`) : undefined + const legacy = legacyToolType ? extractPrefixedModes(overrides, `${legacyToolType}:`) : undefined + if (!scoped) return legacy + return legacy ? { ...legacy, ...scoped } : scoped } const INDEX_SCOPED_KEY = /^(\d+):(.+)$/ @@ -466,7 +511,19 @@ export function isSubBlockVisibleForTriggerMode( } /** - * Resolve the dependency value for a dependsOn key, honoring canonical swaps. + * Resolve what a `dependsOn` key currently points at, honoring canonical swaps. + * + * Deliberately PERMISSIVE, unlike {@link resolveActiveCanonicalValue}: it falls back across the + * pair and then scans the group's other members, so a dependant stays satisfied whenever the group + * holds a usable value anywhere. That is the right answer for a gate ("is my parent chosen yet?") + * and the wrong answer for a value read ("what is live?") - use `resolveActiveCanonicalValue` for + * the latter, which is why the two differ. + * + * Pass a {@link buildCanonicalIndexForSurface} index. The member scan predates surface scoping and + * was how a trigger alias (`triggerCredentials` under an action `oauthCredential` group) used to be + * found at all; a scoped index now makes that alias the group's own `basicId`, so the scan is left + * only as the fallback for state the mode backfill has not reached. Handing it an UNSCOPED index on + * a trigger-mode block puts the dormant action surface back in scan range. */ export function resolveDependencyValue( dependencyKey: string, diff --git a/apps/sim/providers/utils.ts b/apps/sim/providers/utils.ts index 136014d2712..ebff63516ff 100644 --- a/apps/sim/providers/utils.ts +++ b/apps/sim/providers/utils.ts @@ -786,7 +786,9 @@ export async function transformBlockTool( const userProvidedParams = block.params || {} const canonicalGroups: CanonicalGroup[] = blockDef?.subBlocks - ? Object.values(buildCanonicalIndex(blockDef.subBlocks).groupsById).filter(isCanonicalPair) + ? // canonical-index-unscoped: an agent tool resolves against `block.params`, which only ever + // holds action-surface values — a tool is never invoked in trigger mode. + Object.values(buildCanonicalIndex(blockDef.subBlocks).groupsById).filter(isCanonicalPair) : [] const resolvedResourceParams = resolveCanonicalResourceParams( diff --git a/apps/sim/scripts/canvas-sentence-spec.ts b/apps/sim/scripts/canvas-sentence-spec.ts index 4e70d2c5c3b..63e2ef8f10e 100644 --- a/apps/sim/scripts/canvas-sentence-spec.ts +++ b/apps/sim/scripts/canvas-sentence-spec.ts @@ -117,6 +117,8 @@ function visibleOperations(subBlock: SubBlockConfig): string[] | 'all' | 'unknow return main.filter((id) => allowed.has(id)) } +// canonical-index-unscoped: structural only — the spec lists a canonical group whole and +// resolves no values, so neither surface can shadow the other. const canonicalIndex = buildCanonicalIndex(config.subBlocks) /* A sentence can never usefully name these: the operation selector supplies the diff --git a/apps/sim/scripts/check-canvas-sentences.ts b/apps/sim/scripts/check-canvas-sentences.ts index 109e98c6259..130485f6221 100644 --- a/apps/sim/scripts/check-canvas-sentences.ts +++ b/apps/sim/scripts/check-canvas-sentences.ts @@ -39,7 +39,6 @@ import { validateBlockSentences, validateTriggerSentence, } from '@/lib/workflows/blocks/canvas-sentence-validation' -import { buildCanonicalIndex } from '@/lib/workflows/subblocks/visibility' import { getBlockRegistry } from '@/blocks/registry' import type { BlockConfig, SubBlockConfig } from '@/blocks/types' import { TRIGGER_REGISTRY } from '@/triggers/registry' @@ -94,7 +93,6 @@ function paintsOnEmptyCard( for (const subBlock of getCardSubBlocks(config, { advanced: false, values, - canonicalIndex: buildCanonicalIndex(config.subBlocks), triggerMode: card.mode === 'trigger', })) { if (!onCardById.has(subBlock.id)) onCardById.set(subBlock.id, subBlock) diff --git a/apps/sim/serializer/field-analysis.test.ts b/apps/sim/serializer/field-analysis.test.ts index 6123810cde6..8d4c023b04e 100644 --- a/apps/sim/serializer/field-analysis.test.ts +++ b/apps/sim/serializer/field-analysis.test.ts @@ -214,4 +214,76 @@ describe('extractBlockParams', () => { expect(params.credential).toBeUndefined() expect(params.manualCredential).toBeUndefined() }) + + describe('legacy advancedMode', () => { + /** + * `advancedMode` is a block flag the editor stopped writing in #6458, but stored workflows + * still carry it. It means "the ADVANCED member of a pair wins", which only has meaning for a + * group that has one. + */ + const nonPair = () => + config([ + { + id: 'personalApiKey', + title: 'Personal API Key', + type: 'short-input', + canonicalParamId: 'apiKey', + required: true, + }, + ]) + + it('keeps a canonical group that has no advanced member', () => { + svcConfig.value = nonPair() + + const params = extractBlockParams( + block({ + type: 'svc', + advancedMode: true, + subBlocks: { personalApiKey: { value: 'phx_secret' } }, + }) + ) + + // Regression: forcing 'advanced' on a group with no advanced member left `chosen` + // undefined while the source-id sweep still deleted `personalApiKey`, so the block + // serialized with neither key and failed at run time on a field the user had filled. + expect(params.apiKey).toBe('phx_secret') + expect(params.personalApiKey).toBeUndefined() + }) + + it('does not report the surviving value as a missing required field', () => { + const cfg = nonPair() + const state = block({ + type: 'svc', + advancedMode: true, + subBlocks: { personalApiKey: { value: 'phx_secret' } }, + }) + + expect( + collectBlockFieldIssues(state, cfg, extractBlockParams(state)).missingRequiredFields + ).toEqual([]) + }) + + it('still selects the advanced member of a real pair', () => { + svcConfig.value = config([ + { id: 'sel', title: 'Table', type: 'dropdown', canonicalParamId: 'tableId', mode: 'basic' }, + { + id: 'manualId', + title: 'Table ID', + type: 'short-input', + canonicalParamId: 'tableId', + mode: 'advanced', + }, + ]) + + const params = extractBlockParams( + block({ + type: 'svc', + advancedMode: true, + subBlocks: { sel: { value: 'basic-value' }, manualId: { value: 'advanced-value' } }, + }) + ) + + expect(params.tableId).toBe('advanced-value') + }) + }) }) diff --git a/apps/sim/serializer/index.ts b/apps/sim/serializer/index.ts index 5c4a6a53dc4..7c1293b5011 100644 --- a/apps/sim/serializer/index.ts +++ b/apps/sim/serializer/index.ts @@ -511,6 +511,10 @@ export function extractBlockParams(block: BlockState): Record { isCustomBlock && blockConfig.subBlocks.some((config) => !RESERVED_PARAMS.has(config.id)) const isTriggerContext = block.triggerMode ?? false const isTriggerCategory = blockConfig.category === 'triggers' + // The serializer filters BEFORE it resolves, which is why execution has always been correct + // here even unscoped — see `getCanonicalSubBlocksForSurface`. + // canonical-index-unscoped: `shouldSerializeSubBlock` drops the inactive surface's members, and + // the collapse below reads `params` (the filtered map), never `allValues`. const canonicalIndex = buildCanonicalIndex(blockConfig.subBlocks) const allValues = buildSubBlockValues(block.subBlocks) @@ -586,10 +590,15 @@ export function extractBlockParams(block: BlockState): Record { Object.values(canonicalIndex.groupsById).forEach((group) => { const { basicValue, advancedValue } = getCanonicalValues(group, params) const hasExplicitOverride = canonicalModeOverrides?.[group.canonicalId] != null - const pairMode = - hasExplicitOverride || !legacyAdvancedMode - ? resolveCanonicalMode(group, allValues, canonicalModeOverrides) - : 'advanced' + // Legacy `advancedMode: true` (a block flag the editor no longer writes) means "the advanced + // member of a PAIR wins". A group with no advanced member has nothing for it to select, so it + // must resolve normally - forcing 'advanced' there leaves `chosen` undefined while the sourceIds + // sweep below still deletes the basic member, dropping the value the block actually holds. + // Mirrors the `isCanonicalPair` guard `shouldSerializeSubBlock` already applies upstream. + const legacyAdvancedWins = legacyAdvancedMode && !hasExplicitOverride && isCanonicalPair(group) + const pairMode = legacyAdvancedWins + ? 'advanced' + : resolveCanonicalMode(group, allValues, canonicalModeOverrides) const chosen = pairMode === 'advanced' ? advancedValue : basicValue const sourceIds = [group.basicId, ...group.advancedIds].filter(Boolean) as string[] @@ -648,6 +657,8 @@ export function collectBlockFieldIssues( const displayAdvancedOptions = block.advancedMode ?? false const isTriggerContext = block.triggerMode ?? false const isTriggerCategory = blockConfig.category === 'triggers' + // canonical-index-unscoped: same filter-then-resolve ordering as `extractBlockParams`, and a + // trigger-mode block returns above before reaching here at all. const canonicalIndex = buildCanonicalIndex(blockConfig.subBlocks || []) const canonicalModeOverrides = block.data?.canonicalModes const allValues = buildSubBlockValues(block.subBlocks) diff --git a/package.json b/package.json index 0807e5a7447..29dfdd76971 100644 --- a/package.json +++ b/package.json @@ -33,6 +33,7 @@ "check:cli-api": "bun run scripts/generate-v2-cli-api.ts --check", "generate:cli-docs": "bun run scripts/generate-cli-docs.ts", "check:cli-docs": "bun run scripts/generate-cli-docs.ts --check", + "check:canonical-index": "bun run scripts/check-canonical-index-surface.ts", "check:cron-parity": "bun run scripts/check-cron-parity.ts", "check:api-validation:strict": "bun run scripts/check-api-validation-contracts.ts --check --enforce-boundary-baseline", "check:realtime-prune": "bun run scripts/check-realtime-prune-graph.ts", diff --git a/scripts/check-canonical-index-surface.ts b/scripts/check-canonical-index-surface.ts new file mode 100644 index 00000000000..43d7ac491b1 --- /dev/null +++ b/scripts/check-canonical-index-surface.ts @@ -0,0 +1,182 @@ +#!/usr/bin/env bun +/** + * Asserts that no code decides a canonical group's membership without saying which SURFACE it + * means. + * + * A block that is both an action and a trigger holds ONE `subBlocks` array: its own fields plus + * its trigger's, spread in after them. The two sets routinely share a `canonicalParamId` under + * DIFFERENT ids — Webflow's `siteSelector`/`manualSiteId` (action) and `triggerSiteId` (trigger) + * are all `siteId`. Indexed together they collapse into one group whose `basicId` belongs to the + * other surface, so the trigger member matches neither `basicId` nor `advancedIds` and every + * group-relative question about it answers for the dormant surface: the canvas card hid it, the + * `dependsOn` gate resolved it to a stale action value, and the fork remap classified it a + * dormant member and CLEARED it. + * + * That shipped three separate times, in three subsystems, each found by hand. + * `buildCanonicalIndexForSurface` makes the correct thing one call, but nothing stopped the next + * caller from reaching for `blockConfig.subBlocks` again — which is what this audit is for. A + * site that genuinely means the whole array says so in an annotation, so the reasoning lives at + * the call instead of being re-derived by the next reader. + * + * The two guarded functions declare their surface differently, so each gets the rule that fits: + * + * - `buildCanonicalIndex` takes the member set directly, so a first argument reading `.subBlocks` + * off a config is unscoped by construction. A call taking an already-narrowed local + * (`contextConfigs`, `activeSubBlocks`, a `getCanonicalSubBlocksForSurface` result) is + * self-evidently fine and is not flagged. + * - `createCanonicalModeGates` scopes internally from a trailing `triggerSurface` argument, so + * what matters is whether the caller passed one at all — omitting it silently means "action". + */ +import { spawnSync } from 'node:child_process' +import path from 'node:path' + +const ROOT = path.resolve(import.meta.dir, '..') + +/** Placed on the line above a call that deliberately fixes one surface. */ +const ANNOTATION = 'canonical-index-unscoped:' + +/** + * Files that hold the guarded names without calling them. + * + * `buildCanonicalIndexForSurface` has to call the raw `buildCanonicalIndex`, so its defining + * module is exempt rather than annotated. This audit's own source carries both names as string + * literals to search for — without the exemption it flags itself, which is not hypothetical: it + * passed locally while the file was still untracked and failed the moment it was committed and + * `git ls-files` started listing it. + */ +const NOT_CALLERS = new Set([ + 'apps/sim/lib/workflows/subblocks/visibility.ts', + 'scripts/check-canonical-index-surface.ts', +]) + +/** The `triggerSurface` argument's position in `createCanonicalModeGates`. */ +const GATES_SURFACE_ARG_COUNT = 4 + +/** Preceding non-empty lines searched for the annotation, matching the repo's other boundary annotations. */ +const ANNOTATION_LOOKBACK = 3 + +interface Offender { + file: string + line: number + detail: string +} + +/** The call's top-level arguments, via a balanced scan so a multi-line call still parses. */ +function callArguments(source: string, openParenIndex: number): string[] | null { + const args: string[] = [] + let depth = 0 + let start = openParenIndex + 1 + for (let i = openParenIndex; i < source.length; i++) { + const char = source[i] + if (char === '(' || char === '[' || char === '{') depth++ + else if (char === ')' || char === ']' || char === '}') { + depth-- + if (depth === 0) { + const tail = source.slice(start, i).trim() + if (tail.length > 0 || args.length > 0) args.push(tail) + return args + } + } else if (char === ',' && depth === 1) { + args.push(source.slice(start, i).trim()) + start = i + 1 + } + } + return null +} + +/** + * Whether an annotation with a non-empty reason sits on one of the three preceding non-empty + * lines, matching `check-api-validation-contracts.ts`. Deliberately not "three preceding COMMENT + * lines": a guarded call inside a multi-line expression (a ternary arm, a chained `Object.values`) + * is not adjacent to its own comment, and an annotation that cannot be placed is one nobody writes. + */ +function hasAnnotation(lines: string[], line: number): boolean { + let seen = 0 + for (let i = line - 2; i >= 0 && seen < ANNOTATION_LOOKBACK; i--) { + const text = lines[i].trim() + if (text.length === 0) continue + const at = text.indexOf(ANNOTATION) + if (at !== -1) return text.slice(at + ANNOTATION.length).trim().length > 0 + seen++ + } + return false +} + +const listed = spawnSync('git', ['ls-files', '-z', '--', '*.ts', '*.tsx'], { + cwd: ROOT, + encoding: 'buffer', + maxBuffer: 256 * 1024 * 1024, +}) + +if (listed.status !== 0) { + console.error(`Canonical-index audit failed: \`git ls-files\` exited ${listed.status}.`) + process.exit(1) +} + +const files = listed.stdout + .toString('utf8') + .split('\0') + .filter((entry) => entry.length > 0 && !entry.includes('.test.') && !NOT_CALLERS.has(entry)) + +const offenders: Offender[] = [] +let scanned = 0 +let annotated = 0 + +for (const file of files) { + const source = await Bun.file(path.join(ROOT, file)).text() + const hasIndex = source.includes('buildCanonicalIndex(') + const hasGates = source.includes('createCanonicalModeGates(') + if (!hasIndex && !hasGates) continue + const lines = source.split('\n') + + const inspect = (call: string, verdict: (args: string[]) => string | null) => { + for (const match of source.matchAll(new RegExp(`\\b${call}\\(`, 'g'))) { + const args = callArguments(source, match.index + match[0].length - 1) + if (args === null) continue + scanned++ + const problem = verdict(args) + if (problem === null) continue + const line = source.slice(0, match.index).split('\n').length + if (hasAnnotation(lines, line)) { + annotated++ + continue + } + offenders.push({ file, line, detail: `${call}(…) — ${problem}` }) + } + } + + if (hasIndex) { + inspect('buildCanonicalIndex', (args) => + /\.subBlocks\b/.test(args[0] ?? '') + ? `indexes a config's whole \`subBlocks\`: ${(args[0] ?? '').replace(/\s+/g, ' ')}` + : null + ) + } + if (hasGates) { + inspect('createCanonicalModeGates', (args) => + args.length < GATES_SURFACE_ARG_COUNT + ? `omits the \`triggerSurface\` argument, so it silently gates as the action surface` + : null + ) + } +} + +if (offenders.length > 0) { + console.error( + `Canonical-index surface audit failed: ${offenders.length} call(s) decide canonical group\n` + + 'membership without declaring which surface they mean.\n\n' + + offenders.map((o) => ` ${o.file}:${o.line}\n ${o.detail}`).join('\n\n') + + '\n\n On a block that is both an action and a trigger, both surfaces live in one `subBlocks`\n' + + ' array and routinely share a `canonicalParamId` under different ids. Indexed together, a\n' + + ' trigger field joins the action pair and matches neither side of it — so it gets hidden,\n' + + " resolved to the dormant surface's value, or cleared as a dormant member.\n\n" + + ' Pass the surface — `buildCanonicalIndexForSurface(subBlocks, triggerSurface)`, or the\n' + + ' trailing argument on `createCanonicalModeGates`. When the surface is provably constant\n' + + ` at the call, say why instead:\n\n // ${ANNOTATION} nested tool params are always the action surface\n` + ) + process.exit(1) +} + +console.log( + `Canonical-index surface audit passed (${scanned} call(s) checked, ${annotated} annotated).` +)