Skip to content

Commit 2d53b4a

Browse files
committed
fix(workspace-forking): keep the dormant surface classified as it was before scoping
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.
1 parent da73fd0 commit 2d53b4a

14 files changed

Lines changed: 277 additions & 10 deletions

File tree

apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/tool-input.tsx

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -533,6 +533,8 @@ export const ToolInput = memo(function ToolInput({
533533
for (const [toolIndex, tool] of selectedTools.entries()) {
534534
const blockConfig = allBlocks.find((b: { type: string }) => b.type === tool.type)
535535
if (!blockConfig?.subBlocks) continue
536+
// canonical-index-unscoped: a nested tool resolves against `tool.params`, which only ever
537+
// holds action-surface values — a tool is never invoked in trigger mode.
536538
const toolCanonical = buildCanonicalIndex(blockConfig.subBlocks)
537539
const scopedOverrides = scopeCanonicalModesForTool(
538540
canonicalModeOverrides,
@@ -1779,7 +1781,8 @@ export const ToolInput = memo(function ToolInput({
17791781
: null
17801782

17811783
const toolCanonicalIndex: CanonicalIndex | null = toolBlock?.subBlocks
1782-
? buildCanonicalIndex(toolBlock.subBlocks)
1784+
? // canonical-index-unscoped: nested tool params are always the action surface
1785+
buildCanonicalIndex(toolBlock.subBlocks)
17831786
: null
17841787

17851788
const toolContextValues = toolCanonicalIndex

apps/sim/ee/workspace-forking/lib/mapping/dependent-reconfigs.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,8 @@ function emitAnchoredDependents(params: EmitAnchoredParams): void {
135135
})
136136
const scanSubBlocks = getSelectorContextSubBlocks(config.subBlocks, values, triggerMode)
137137
const canonicalIndex = buildCanonicalIndex(scanSubBlocks)
138+
// canonical-index-unscoped: `scanSubBlocks` is already narrowed to the active surface by
139+
// `getSelectorContextSubBlocks` above, so scoping again here would be a no-op.
138140
const gates = createCanonicalModeGates(scanSubBlocks, values, canonicalModes)
139141
const configById = new Map(scanSubBlocks.filter((cfg) => cfg.id).map((cfg) => [cfg.id, cfg]))
140142
// Shared with `applyDependentOverrides`, so what the modal offers is exactly what the sync

apps/sim/ee/workspace-forking/lib/remap/remap-references.test.ts

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -825,6 +825,46 @@ describe('canonical-mode gates on a mixed action/trigger block', () => {
825825
expect(gates.isActiveManualMember('triggerSiteId')).toBe(false)
826826
})
827827

828+
it('leaves the DORMANT action surface classified exactly as before scoping', () => {
829+
vi.mocked(getBlock).mockReturnValue(mixedSurfaceBlock())
830+
const config = getBlock('webflow') as BlockConfig
831+
const scoped = createCanonicalModeGates(config.subBlocks, values, { siteId: 'advanced' }, true)
832+
833+
// Scoping decides membership for LIVE fields only. The action surface's own values are still
834+
// real keys in the block's value map, and the remap loop reads `isDormantMember` to decide
835+
// both whether to clear a value and whether to skip detecting it. Answering "not a member"
836+
// here would stop clearing them AND start detecting them, turning a stale action selector on
837+
// a trigger-mode block into a mapping requirement that can block a sync.
838+
expect(scoped.isDormantMember('siteSelector')).toBe(true)
839+
expect(scoped.isActiveManualMember('manualSiteId')).toBe(true)
840+
841+
// Identical to what the unscoped gates answered for those same keys before the fix.
842+
const legacy = createCanonicalModeGates(config.subBlocks, values, { siteId: 'advanced' }, false)
843+
for (const key of ['siteSelector', 'manualSiteId']) {
844+
expect(scoped.isDormantMember(key)).toBe(legacy.isDormantMember(key))
845+
expect(scoped.isActiveManualMember(key)).toBe(legacy.isActiveManualMember(key))
846+
}
847+
})
848+
849+
it('does not turn a dormant action credential into a detected reference', () => {
850+
vi.mocked(getBlock).mockReturnValue(mixedSurfaceBlock())
851+
const subBlocks: SubBlockRecord = {
852+
siteSelector: { type: 'project-selector', value: 'source-workspace-site' },
853+
manualSiteId: { type: 'short-input', value: 'stale-manual-site' },
854+
triggerSiteId: { type: 'dropdown', value: 'site-live' },
855+
}
856+
const result = remapForkSubBlocks(subBlocks, () => null, 'promote', {
857+
blockType: 'webflow',
858+
canonicalModes: { siteId: 'advanced' },
859+
triggerMode: true,
860+
})
861+
// The dormant basic member is cleared and never becomes a promote blocker, exactly as it did
862+
// before surface scoping — while the live trigger field survives.
863+
expect(result.subBlocks.siteSelector.value).toBe('')
864+
expect(result.unmapped.some((ref) => ref.subBlockKey === 'siteSelector')).toBe(false)
865+
expect(result.subBlocks.triggerSiteId.value).toBe('site-live')
866+
})
867+
828868
it('still gates the action surface normally', () => {
829869
vi.mocked(getBlock).mockReturnValue(mixedSurfaceBlock())
830870
const config = getBlock('webflow') as BlockConfig

apps/sim/ee/workspace-forking/lib/remap/remap-references.ts

Lines changed: 35 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -472,20 +472,45 @@ export function createCanonicalModeGates(
472472
if (!configSubBlocks || configSubBlocks.length === 0) return NO_GATES
473473
const surfaceSubBlocks = getCanonicalSubBlocksForSurface(configSubBlocks, triggerSurface)
474474
const canonicalIndex = buildCanonicalIndex(surfaceSubBlocks)
475+
// canonical-index-unscoped: the fallback for keys the ACTIVE surface does not define — see
476+
// `indexFor`. Scoping decides membership for live fields only; a dormant surface's own values
477+
// keep the classification they had before scoping existed.
478+
const fullIndex = buildCanonicalIndex(configSubBlocks)
475479
const configByBaseKey = new Map(
476-
surfaceSubBlocks.filter((cfg) => cfg.id).map((cfg) => [cfg.id, cfg])
480+
configSubBlocks.filter((cfg) => cfg.id).map((cfg) => [cfg.id, cfg])
477481
)
478482
const conditionValues = { ...values }
479-
for (const [canonicalId, group] of Object.entries(canonicalIndex.groupsById)) {
480-
if (conditionValues[canonicalId] === undefined) {
481-
conditionValues[canonicalId] = resolveActiveCanonicalValue(group, values, canonicalModes)
483+
for (const index of [canonicalIndex, fullIndex]) {
484+
for (const [canonicalId, group] of Object.entries(index.groupsById)) {
485+
if (conditionValues[canonicalId] === undefined) {
486+
conditionValues[canonicalId] = resolveActiveCanonicalValue(group, values, canonicalModes)
487+
}
482488
}
483489
}
484490

491+
/**
492+
* The index that owns a key.
493+
*
494+
* The scoped index answers for anything the active surface defines — that is the fix: a trigger
495+
* field sharing a `canonicalParamId` with an action pair gets its OWN group instead of being
496+
* read as a stranded member of the action pair's.
497+
*
498+
* Everything else falls back to the whole array, deliberately. A dormant surface's values are
499+
* still real keys in the block's value map, and the remap loop reads `isDormantMember` to decide
500+
* both whether to CLEAR a value and whether to skip detecting it as a reference. Answering
501+
* "not a member" for them would stop clearing them AND start detecting them, turning a stale
502+
* action selector on a trigger-mode block into a mapping requirement that can block a sync.
503+
* Scoping is meant to stop live fields being misread, not to re-classify dormant ones.
504+
*/
505+
const indexFor = (key: string) =>
506+
canonicalIndex.canonicalIdBySubBlockId[key] || canonicalIndex.groupsById[key]
507+
? canonicalIndex
508+
: fullIndex
509+
485510
const groupFor = (memberOrCanonicalId: string) => {
486-
const canonicalId =
487-
canonicalIndex.canonicalIdBySubBlockId[memberOrCanonicalId] ?? memberOrCanonicalId
488-
const group = canonicalIndex.groupsById[canonicalId]
511+
const index = indexFor(memberOrCanonicalId)
512+
const canonicalId = index.canonicalIdBySubBlockId[memberOrCanonicalId] ?? memberOrCanonicalId
513+
const group = index.groupsById[canonicalId]
489514
return group && isCanonicalPair(group) ? group : undefined
490515
}
491516
const baseKeyOf = (subBlockKey: string) => subBlockKey.replace(/_\d+$/, '')
@@ -500,7 +525,7 @@ export function createCanonicalModeGates(
500525
isDormantMember: (subBlockKey) => {
501526
const baseKey = baseKeyOf(subBlockKey)
502527
const group = groupFor(baseKey)
503-
if (!group || !canonicalIndex.canonicalIdBySubBlockId[baseKey]) return false
528+
if (!group || !indexFor(baseKey).canonicalIdBySubBlockId[baseKey]) return false
504529
return isAdvancedActiveGroup(baseKey) !== group.advancedIds.includes(baseKey)
505530
},
506531
isActiveManualMember: (subBlockKey) => {
@@ -645,6 +670,7 @@ export function remapToolBlockResources(
645670
tool.type
646671
)
647672
const toolBlockSubBlocks = (opts.blockConfigs?.[tool.type] ?? getBlock(tool.type))?.subBlocks
673+
// canonical-index-unscoped: a nested tool's params are always the action surface
648674
const gates = createCanonicalModeGates(toolBlockSubBlocks, toolValues, scopedModes)
649675

650676
// Clear DORMANT member keys first: a stale inactive value must not survive the copy (and must
@@ -1456,6 +1482,7 @@ function collectClearedToolParamDependents(
14561482
// A DORMANT canonical member's cleared slot is not a lost configuration (only the pair's
14571483
// active member executes). Modes resolve like the tool-input UI: tool-scoped overrides,
14581484
// then the value heuristic over the merged params.
1485+
// canonical-index-unscoped: a nested tool's params are always the action surface
14591486
const gates = createCanonicalModeGates(
14601487
toolConfig.subBlocks,
14611488
mergedValues,

apps/sim/lib/copilot/tools/server/workflow/edit-workflow/builders.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -263,6 +263,8 @@ export function updateCanonicalModesForInputs(
263263
): void {
264264
if (!blockConfig.subBlocks?.length) return
265265

266+
// canonical-index-unscoped: structural only — this maps written input ids to the mode they
267+
// imply and reads no values, so neither surface can shadow the other.
266268
const canonicalIndex = buildCanonicalIndex(blockConfig.subBlocks)
267269
const canonicalModeUpdates: Record<string, 'basic' | 'advanced'> = {}
268270

apps/sim/lib/webhooks/deploy.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,8 @@ export function buildProviderConfig(
209209
Object.entries(block.subBlocks || {}).map(([key, value]) => [key, { value: value.value }])
210210
)
211211

212+
// canonical-index-unscoped: a trigger DEFINITION's subblocks are the trigger surface by
213+
// construction — this never sees the host block's action fields.
212214
const canonicalIndex = buildCanonicalIndex(triggerDef.subBlocks)
213215
const satisfiedCanonicalIds = new Set<string>()
214216
const filledSubBlockIds = new Set<string>()

apps/sim/lib/workflows/blocks/canvas-sentence-validation.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,8 @@ function buildBlockIndex(config: ValidatableBlockConfig): BlockIndex {
228228
return {
229229
subBlocks: config.subBlocks,
230230
byId: groupSubBlocksById(config.subBlocks),
231+
// canonical-index-unscoped: `resolveVisibility` returns `hidden` for every trigger-mode
232+
// subblock as its first check, so only action subblocks ever reach this index.
231233
canonical: buildCanonicalIndex(config.subBlocks),
232234
seededValues: getSeededSubBlockValues(config),
233235
}

apps/sim/lib/workflows/migrations/subblock-migrations.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -535,6 +535,9 @@ export function backfillCanonicalModes(blocks: Record<string, BlockState>): {
535535
continue
536536
}
537537

538+
// canonical-index-unscoped: the backfill writes a mode only for canonical PAIRS, whose two
539+
// members always sit on the same surface — a cross-surface alias joins an existing group
540+
// rather than forming a pair, so scoping cannot change what gets backfilled.
538541
const canonicalIndex = buildCanonicalIndex(blockConfig.subBlocks)
539542
const pairs = Object.values(canonicalIndex.groupsById).filter(isCanonicalPair)
540543
if (pairs.length === 0) {

apps/sim/lib/workflows/search-replace/indexer.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -790,6 +790,7 @@ export function getToolInputParamConfigs({
790790
})
791791
}
792792

793+
// canonical-index-unscoped: a nested tool's params are always the action surface
793794
const toolCanonicalIndex = buildCanonicalIndex(
794795
blockConfig?.subBlocks ?? subBlocksResult.subBlocks
795796
)

apps/sim/providers/utils.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -786,7 +786,9 @@ export async function transformBlockTool(
786786
const userProvidedParams = block.params || {}
787787

788788
const canonicalGroups: CanonicalGroup[] = blockDef?.subBlocks
789-
? Object.values(buildCanonicalIndex(blockDef.subBlocks).groupsById).filter(isCanonicalPair)
789+
? // canonical-index-unscoped: an agent tool resolves against `block.params`, which only ever
790+
// holds action-surface values — a tool is never invoked in trigger mode.
791+
Object.values(buildCanonicalIndex(blockDef.subBlocks).groupsById).filter(isCanonicalPair)
790792
: []
791793

792794
const resolvedResourceParams = resolveCanonicalResourceParams(

0 commit comments

Comments
 (0)