From 2ff39d503449b42496844400261a54a889e350f4 Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Sat, 22 Aug 2026 15:14:39 -0700 Subject: [PATCH 1/4] fix(workflows): scope the canonical sub-block index to the active surface MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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, so indexing them together collapses a trigger field into an action pair whose `basicId` it can never be. Every group-relative question about that field then answers for the dormant surface. The serializer was never affected: `shouldSerializeSubBlock` drops the inactive surface before the canonical collapse reads it, so it resolves against a value map the dormant surface cannot appear in. Every other caller resolves against the block's full value map, so the scoping has to live in the index. - add `getCanonicalSubBlocksForSurface` / `buildCanonicalIndexForSurface`, and move the three sites that already had the filter inline onto them - `getCardSubBlocks` derives its own index instead of accepting one; it already took `triggerMode`, and accepting an index is what let all three callers pass one built for the other surface - scope the remaining consumers that resolve against a full value map: the canvas card, autolayout, both preview surfaces, the dependsOn gate, the canonical value hook, reactive conditions, and the copilot selector lint - keep a canonical group with no advanced member out of the legacy `advancedMode` path, which deleted its basic member and republished nothing - merge legacy type-scoped tool modes as a baseline under index-scoped ones, so the first re-toggle stops reverting the ids the user has not touched --- .../hooks/use-canonical-sub-block-value.ts | 10 +- .../sub-block/hooks/use-depends-on-gate.ts | 12 +- .../panel/components/editor/editor.tsx | 11 +- .../hooks/use-editor-subblock-layout.ts | 11 +- .../workflow-block/workflow-block.tsx | 10 +- .../preview-editor/preview-editor.tsx | 8 +- .../components/block/block.tsx | 14 +-- apps/sim/hooks/use-reactive-conditions.ts | 19 ++- .../workflow/edit-workflow/validation.ts | 12 +- apps/sim/lib/workflows/autolayout/utils.ts | 4 +- .../blocks/canvas-card-fields.test.ts | 111 ++++++++++++++++++ .../workflows/blocks/canvas-card-fields.ts | 14 ++- .../blocks/canvas-sentence-render.ts | 2 - .../lib/workflows/search-replace/indexer.ts | 10 +- .../workflows/subblocks/visibility.test.ts | 63 ++++++++++ .../sim/lib/workflows/subblocks/visibility.ts | 71 +++++++++-- apps/sim/scripts/check-canvas-sentences.ts | 2 - apps/sim/serializer/field-analysis.test.ts | 72 ++++++++++++ apps/sim/serializer/index.ts | 13 +- 19 files changed, 404 insertions(+), 65 deletions(-) create mode 100644 apps/sim/lib/workflows/blocks/canvas-card-fields.test.ts 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/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/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/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/search-replace/indexer.ts b/apps/sim/lib/workflows/search-replace/indexer.ts index af65cacffb0..10a2fafad07 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' @@ -1287,11 +1287,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/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..e8ad9ce077a 100644 --- a/apps/sim/serializer/index.ts +++ b/apps/sim/serializer/index.ts @@ -586,10 +586,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[] From da73fd0f6b62719696266a17f184a2c3101009c7 Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Sat, 22 Aug 2026 15:24:53 -0700 Subject: [PATCH 2/4] fix(workspace-forking): scope the canonical gates to the block's active surface MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `createCanonicalModeGates` indexed a block's whole `subBlocks` array, so on a mixed action/trigger block a trigger field sharing a `canonicalParamId` with an action pair was read as a member of THAT pair. Being neither its `basicId` nor in its `advancedIds`, `isDormantMember` answered true the moment the shared mode resolved to advanced — and a fork acts on that by clearing the value, so a configured trigger field was silently wiped on fork/sync. Reachable without any explicit toggle: a block configured as an action with a manual id and then switched to trigger mode leaves the pair's value heuristic resolving to advanced on its own. - `createCanonicalModeGates` takes the surface and scopes its index - thread `triggerMode` through `RemapForkContext`, `SubBlockTransform`, `clearDependentsOnRemap`, `collectClearedDependents`, the reference scanners, and the promote cleared-ref collectors - nested tool params and the dependent scan are unchanged: a tool is always the action surface, and the dependent scan already narrows its configs --- .../lib/copy/copy-workflows.ts | 21 ++++-- .../lib/promote/cleared-refs.ts | 13 +++- .../lib/remap/fork-bootstrap.ts | 6 +- .../lib/remap/reference-scan.ts | 2 + .../lib/remap/remap-references.test.ts | 73 +++++++++++++++++++ .../lib/remap/remap-references.ts | 57 ++++++++++++--- 6 files changed, 150 insertions(+), 22 deletions(-) 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/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..f6a257bfd22 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,78 @@ 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('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..005aebf84c1 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,16 +454,26 @@ 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) const configByBaseKey = new Map( - configSubBlocks.filter((cfg) => cfg.id).map((cfg) => [cfg.id, cfg]) + surfaceSubBlocks.filter((cfg) => cfg.id).map((cfg) => [cfg.id, cfg]) ) const conditionValues = { ...values } for (const [canonicalId, group] of Object.entries(canonicalIndex.groupsById)) { @@ -521,6 +534,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 /** @@ -1047,7 +1066,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 +1296,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 +1312,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 @@ -1498,7 +1521,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 +1531,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 +1785,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 +1799,8 @@ export function createForkSubBlockTransform( blockType, result.remappedKeys, result.canonicalModes ?? canonicalModes, - result.copyRemappedKeys + result.copyRemappedKeys, + triggerMode ) } } @@ -1792,6 +1824,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 +1856,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}` From 2d53b4ae4550d3fa13114866a09db3b3a52b65ab Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Sat, 22 Aug 2026 15:52:18 -0700 Subject: [PATCH 3/4] fix(workspace-forking): keep the dormant surface classified as it was before scoping MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Surface scoping decides canonical membership for the ACTIVE surface. Applying it to every key also re-classified the dormant surface's own values: they stopped being dormant members, which meant the remap no longer cleared them AND started detecting them as references — turning a stale action selector on a trigger-mode block into a mapping requirement that can block promote/sync. The gates now pick the index per key: the scoped one for anything the active surface defines (the fix — a trigger field gets its own group instead of being read as a stranded member of an action pair), the whole array for everything else, which is byte-for-byte the pre-scoping behavior. Also adds `check:canonical-index`, an audit that fails any call building a canonical index off a config's whole `subBlocks`, or calling the fork gates without a surface, unless annotated with why. This defect shipped three times in three subsystems; the 14 sites that legitimately mean one fixed surface now say so at the call. --- .../components/tool-input/tool-input.tsx | 5 +- .../lib/mapping/dependent-reconfigs.ts | 2 + .../lib/remap/remap-references.test.ts | 40 ++++ .../lib/remap/remap-references.ts | 43 ++++- .../server/workflow/edit-workflow/builders.ts | 2 + apps/sim/lib/webhooks/deploy.ts | 2 + .../blocks/canvas-sentence-validation.ts | 2 + .../migrations/subblock-migrations.ts | 3 + .../lib/workflows/search-replace/indexer.ts | 1 + apps/sim/providers/utils.ts | 4 +- apps/sim/scripts/canvas-sentence-spec.ts | 2 + apps/sim/serializer/index.ts | 6 + package.json | 1 + scripts/check-canonical-index-surface.ts | 174 ++++++++++++++++++ 14 files changed, 277 insertions(+), 10 deletions(-) create mode 100644 scripts/check-canonical-index-surface.ts 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/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/remap/remap-references.test.ts b/apps/sim/ee/workspace-forking/lib/remap/remap-references.test.ts index f6a257bfd22..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 @@ -825,6 +825,46 @@ describe('canonical-mode gates on a mixed action/trigger block', () => { 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 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 005aebf84c1..916acd6ecdf 100644 --- a/apps/sim/ee/workspace-forking/lib/remap/remap-references.ts +++ b/apps/sim/ee/workspace-forking/lib/remap/remap-references.ts @@ -472,20 +472,45 @@ export function createCanonicalModeGates( if (!configSubBlocks || configSubBlocks.length === 0) return NO_GATES 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( - surfaceSubBlocks.filter((cfg) => cfg.id).map((cfg) => [cfg.id, cfg]) + 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+$/, '') @@ -500,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) => { @@ -645,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 @@ -1456,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, 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/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/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 10a2fafad07..8eb9e60b37e 100644 --- a/apps/sim/lib/workflows/search-replace/indexer.ts +++ b/apps/sim/lib/workflows/search-replace/indexer.ts @@ -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 ) 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/serializer/index.ts b/apps/sim/serializer/index.ts index e8ad9ce077a..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) @@ -653,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..cfc0c5aff71 --- /dev/null +++ b/scripts/check-canonical-index-surface.ts @@ -0,0 +1,174 @@ +#!/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:' + +/** + * Where the surface-scoping primitives are defined. `buildCanonicalIndexForSurface` has to call + * the raw `buildCanonicalIndex`, so the defining module is exempt rather than annotated. + */ +const DEFINING_MODULE = 'apps/sim/lib/workflows/subblocks/visibility.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.') && entry !== DEFINING_MODULE) + +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).` +) From 4a083b9e2ceb78d538d700a7b433b848d32c0898 Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Sat, 22 Aug 2026 16:22:45 -0700 Subject: [PATCH 4/4] fix(audits): stop the canonical-index guard flagging its own source MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The audit holds `buildCanonicalIndex(` and `createCanonicalModeGates(` as string literals to search for, and its own regex matched them — the arg-count rule then fired on the literal. It passed locally only because the file was still untracked when it ran, so `git ls-files` did not list it; committing it made the audit scan itself and fail CI on the first run. Exempts the audit's own source alongside the module that defines the primitives. Verified the guard still fails on both regression shapes after the exemption. --- scripts/check-canonical-index-surface.ts | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/scripts/check-canonical-index-surface.ts b/scripts/check-canonical-index-surface.ts index cfc0c5aff71..43d7ac491b1 100644 --- a/scripts/check-canonical-index-surface.ts +++ b/scripts/check-canonical-index-surface.ts @@ -36,10 +36,18 @@ const ROOT = path.resolve(import.meta.dir, '..') const ANNOTATION = 'canonical-index-unscoped:' /** - * Where the surface-scoping primitives are defined. `buildCanonicalIndexForSurface` has to call - * the raw `buildCanonicalIndex`, so the defining module is exempt rather than annotated. + * 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 DEFINING_MODULE = 'apps/sim/lib/workflows/subblocks/visibility.ts' +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 @@ -108,7 +116,7 @@ if (listed.status !== 0) { const files = listed.stdout .toString('utf8') .split('\0') - .filter((entry) => entry.length > 0 && !entry.includes('.test.') && entry !== DEFINING_MODULE) + .filter((entry) => entry.length > 0 && !entry.includes('.test.') && !NOT_CALLERS.has(entry)) const offenders: Offender[] = [] let scanned = 0