Skip to content

Commit 7b2fa42

Browse files
Vikhyath Mondreticlaude
Vikhyath Mondreti
and
claude
committed
fix(workspace-forking): namespace, type, and single-source a custom block's configured inputs
Four review findings, all real, three of them the same root cause: the dependent store holds a plain string keyed only by (target workflow, block, sub-block), and that key carried none of what applying the value correctly needs. The key now carries the TARGET TYPE and the field's declared TYPE. Target type, because remapping a block to A, configuring it, then remapping to B would otherwise pre-fill and submit A's value into any field id the two happened to share — a different workflow's field of the same name. Namespacing makes that structurally impossible instead of a rule to remember. Field type, because the canvas stores a boolean input as a real boolean (its sub-block is a `switch`), so a stored `'false'` written as text is truthy to the child workflow. The apply side reads the type off the key and restores it, and the modal offers a switch rather than a text field. `object`/`array` stay strings: they are authored as JSON and parsed by the executor. Separately, the carve-out that keeps a custom block's stored value alive through its always-true `parentChanged` was applied at the render site, so the modal showed the stored value while the Sync gate and the submitted payload still saw blank — required fields looked filled but kept Sync disabled, and optional ones submitted empty and wiped the stored mapping. It now lives in `effectiveDependentValue`, the one place all three read through. Field-type-to-control selection moves out of the component into its own module, where it sits beside the boolean round-trip constants it has to agree with. Reported by Greptile and Cursor Bugbot on #6871. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 5a92fcd commit 7b2fa42

9 files changed

Lines changed: 242 additions & 47 deletions

File tree

‎apps/sim/ee/workspace-forking/components/fork-sync/custom-block-input-control.ts‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
/**
2+
* Which control the sync modal renders for a repointed custom block's input, derived from the
3+
* field type its Start block declares.
4+
*
5+
* Mirrors `subBlockTypeForField` in `@/blocks/custom/build-config`, which decides the same thing
6+
* for the canvas — a field the user configures here must read and behave the way it will once
7+
* the block is open in the editor.
8+
*/
9+
export type CustomBlockInputControl = 'switch' | 'textarea' | 'input'
10+
11+
export function customBlockInputControl(fieldType: string | undefined): CustomBlockInputControl {
12+
switch (fieldType) {
13+
// Stored as a real boolean on the canvas (its sub-block is a `switch`), so it must be
14+
// toggled here rather than typed — a text field would persist the string `'true'`.
15+
case 'boolean':
16+
return 'switch'
17+
// Authored as JSON and parsed by the executor before the child receives it.
18+
case 'object':
19+
case 'array':
20+
return 'textarea'
21+
default:
22+
return 'input'
23+
}
24+
}
25+
26+
/**
27+
* The two string values a boolean input round-trips through the string-valued dependent store.
28+
* `replaceCustomBlockInputs` turns them back into a real boolean on apply, because the canvas
29+
* stores a `switch` sub-block as one.
30+
*/
31+
export const CUSTOM_BLOCK_BOOLEAN_TRUE = 'true'
32+
export const CUSTOM_BLOCK_BOOLEAN_FALSE = 'false'
33+
34+
/** Segments for a boolean input, in the order a switch reads them. */
35+
export const CUSTOM_BLOCK_BOOLEAN_OPTIONS = [
36+
{ value: CUSTOM_BLOCK_BOOLEAN_TRUE, label: 'True' },
37+
{ value: CUSTOM_BLOCK_BOOLEAN_FALSE, label: 'False' },
38+
] as const

‎apps/sim/ee/workspace-forking/components/fork-sync/dependent-value.test.ts‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,27 @@ describe('effectiveDependentValue', () => {
5555
expect(effectiveDependentValue(field({ currentValue: 'INBOX' }), {}, false)).toBe('INBOX')
5656
})
5757

58+
it('keeps a custom-block input\'s stored value even though its parent always "changed"', () => {
59+
// These fields exist BECAUSE the block's type was repointed, so `parentChanged` is always
60+
// true — but the stored value is the user's configuration for that exact target (the
61+
// storage key namespaces it by target type), not a stale pick against an old parent.
62+
// Blanking it here desyncs the rendered value from the Sync gate and the submitted
63+
// payload: required fields look filled but keep Sync disabled, and optional ones submit
64+
// empty and wipe the stored mapping.
65+
const customBlockField = field({
66+
parentKind: 'custom-block',
67+
parentSourceId: 'custom_block_uat0001',
68+
currentValue: 'configured for the target',
69+
})
70+
71+
expect(effectiveDependentValue(customBlockField, {}, true)).toBe('configured for the target')
72+
})
73+
74+
it('still blanks a custom-block input the user explicitly cleared', () => {
75+
const f = field({ parentKind: 'custom-block', currentValue: 'stored' })
76+
expect(effectiveDependentValue(f, { [dependentKey(f)]: null }, true)).toBe('')
77+
})
78+
5879
it('returns blank when the parent changed (the stored value no longer resolves)', () => {
5980
expect(effectiveDependentValue(field({ currentValue: 'INBOX' }), {}, true)).toBe('')
6081
})

‎apps/sim/ee/workspace-forking/components/fork-sync/dependent-value.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,13 @@ export function effectiveDependentValue(
150150
const repicked = reconfig[dependentKey(field)]
151151
if (repicked === null) return ''
152152
if (repicked !== undefined) return repicked
153-
return parentChanged ? '' : field.currentValue
153+
// A custom block's inputs exist BECAUSE its type was repointed, so `parentChanged` is always
154+
// true for them — but their stored value IS the user's configuration for that exact target
155+
// (the storage key namespaces it by target type), not a stale pick against an old parent, so
156+
// it must survive. The rule lives here rather than at the render site so the displayed value,
157+
// the Sync gate, and the submitted payload can never disagree about what a field holds.
158+
if (parentChanged && field.parentKind !== 'custom-block') return ''
159+
return field.currentValue
154160
}
155161

156162
/**

‎apps/sim/ee/workspace-forking/components/fork-sync/fork-sync-view.tsx‎

Lines changed: 33 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,12 @@ import {
3333
forkBlockerResolution,
3434
} from '@/ee/workspace-forking/components/fork-sync/cleared-refs-list'
3535
import { forkRefKey } from '@/ee/workspace-forking/components/fork-sync/copy-reconciliation'
36+
import {
37+
CUSTOM_BLOCK_BOOLEAN_FALSE,
38+
CUSTOM_BLOCK_BOOLEAN_OPTIONS,
39+
CUSTOM_BLOCK_BOOLEAN_TRUE,
40+
customBlockInputControl,
41+
} from '@/ee/workspace-forking/components/fork-sync/custom-block-input-control'
3642
import { DependentFieldSelector } from '@/ee/workspace-forking/components/fork-sync/dependent-field-selector'
3743
import {
3844
applyDependentRepick,
@@ -91,9 +97,6 @@ const NEW_TRIGGER_URL_VALUE = '__new_trigger_url__'
9197
*/
9298
const MAPPING_TARGET_TRIGGER_CLASS = 'w-[380px] flex-shrink-0'
9399

94-
/** Custom-block input types whose value is JSON, so the field needs a textarea, not a line. */
95-
const CUSTOM_BLOCK_JSON_FIELD_TYPES = new Set(['object', 'array'])
96-
97100
interface DependentBlock {
98101
targetBlockId: string
99102
blockName: string
@@ -209,16 +212,13 @@ function DependentSelector({
209212
reconfig,
210213
setReconfig,
211214
}: DependentSelectorProps) {
212-
// A custom block's inputs exist BECAUSE its type was repointed, so `parentChanged` is
213-
// always true for them — but unlike a re-picked selector their stored value is the user's
214-
// own configuration for the target and must pre-fill, not blank out.
215+
// `effectiveDependentValue` owns the custom-block carve-out, so the value shown here is the
216+
// same one the Sync gate and the submitted payload see.
215217
const isCustomBlockInput = field.parentKind === 'custom-block'
216218
const effectiveValueIn = (f: ForkDependentReconfig, state: DependentReconfigState) =>
217-
isCustomBlockInput
218-
? effectiveDependentValue(f, state, false)
219-
: copying
220-
? effectiveCopyDependentValue(f, state)
221-
: effectiveDependentValue(f, state, parentChanged)
219+
copying && !isCustomBlockInput
220+
? effectiveCopyDependentValue(f, state)
221+
: effectiveDependentValue(f, state, parentChanged)
222222
const baselineValueFor = (f: ForkDependentReconfig) => effectiveValueIn(f, {})
223223
const effectiveValue = (f: ForkDependentReconfig) => effectiveValueIn(f, reconfig)
224224
if (isCustomBlockInput) {
@@ -228,17 +228,29 @@ function DependentSelector({
228228
// is JSON, matching how `subBlockTypeForField` renders them on the canvas.
229229
const setValue = (value: string) =>
230230
setReconfig((current) => ({ ...current, [dependentKey(field)]: value }))
231-
const shared = {
232-
title: field.title,
233-
required: field.required,
234-
value: effectiveValue(field),
235-
onChange: setValue,
231+
const value = effectiveValue(field)
232+
const shared = { title: field.title, required: field.required }
233+
switch (customBlockInputControl(field.fieldType)) {
234+
case 'switch':
235+
return (
236+
<ChipModalField {...shared} type='custom'>
237+
<ChipSwitch
238+
options={CUSTOM_BLOCK_BOOLEAN_OPTIONS}
239+
value={
240+
value === CUSTOM_BLOCK_BOOLEAN_TRUE
241+
? CUSTOM_BLOCK_BOOLEAN_TRUE
242+
: CUSTOM_BLOCK_BOOLEAN_FALSE
243+
}
244+
onChange={setValue}
245+
aria-label={field.title}
246+
/>
247+
</ChipModalField>
248+
)
249+
case 'textarea':
250+
return <ChipModalField {...shared} type='textarea' value={value} onChange={setValue} />
251+
default:
252+
return <ChipModalField {...shared} type='input' value={value} onChange={setValue} />
236253
}
237-
return CUSTOM_BLOCK_JSON_FIELD_TYPES.has(field.fieldType ?? '') ? (
238-
<ChipModalField {...shared} type='textarea' />
239-
) : (
240-
<ChipModalField {...shared} type='input' />
241-
)
242254
}
243255

244256
const { providedValues, providedContextKeys } = blockChainState(block, field, effectiveValue)

‎apps/sim/ee/workspace-forking/lib/copy/copy-workflows.test.ts‎

Lines changed: 60 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -580,7 +580,9 @@ describe('copyWorkflowStateIntoTarget custom-block remap', () => {
580580
...baseParams,
581581
tx: stubTx(),
582582
transformBlockType: (type) => (type === UAT ? PROD : type),
583-
dependentOverrides: new Map([['tgt-blk-cb', new Map([['field-prod-x', 'prod value X']])]]),
583+
dependentOverrides: new Map([
584+
['tgt-blk-cb', new Map([[`${PROD}::string::field-prod-x`, 'prod value X']])],
585+
]),
584586
})
585587

586588
const subBlocks = writtenBlock().subBlocks ?? {}
@@ -602,8 +604,8 @@ describe('copyWorkflowStateIntoTarget custom-block remap', () => {
602604
[
603605
'tgt-blk-cb',
604606
new Map([
605-
['workflowId', 'crafted'],
606-
['field-prod-x', 'ok'],
607+
[`${PROD}::string::workflowId`, 'crafted'],
608+
[`${PROD}::string::field-prod-x`, 'ok'],
607609
]),
608610
],
609611
]),
@@ -623,12 +625,66 @@ describe('copyWorkflowStateIntoTarget custom-block remap', () => {
623625
...baseParams,
624626
tx: stubTx(),
625627
transformBlockType: (type) => type,
626-
dependentOverrides: new Map([['tgt-blk-cb', new Map([['field-prod-x', 'must not apply']])]]),
628+
dependentOverrides: new Map([
629+
['tgt-blk-cb', new Map([[`${PROD}::string::field-prod-x`, 'must not apply']])],
630+
]),
627631
})
628632

629633
const subBlocks = writtenBlock().subBlocks ?? {}
630634
expect(writtenBlock().type).toBe(UAT)
631635
expect(subBlocks['field-uat-a']?.value).toBe('uat value A')
632636
expect(subBlocks['field-prod-x']).toBeUndefined()
633637
})
638+
639+
it('ignores values stored for a DIFFERENT target, so a second remap starts clean', async () => {
640+
// Map to A, configure it, then remap to B. A field id present on both would otherwise
641+
// carry A's value into B — a different workflow's field that happens to share a name.
642+
mockSaveWorkflowToNormalizedTables.mockResolvedValue({ success: true })
643+
const OTHER = 'custom_block_other99'
644+
645+
await copyWorkflowStateIntoTarget({
646+
...baseParams,
647+
tx: stubTx(),
648+
transformBlockType: (type) => (type === UAT ? PROD : type),
649+
dependentOverrides: new Map([
650+
[
651+
'tgt-blk-cb',
652+
new Map([
653+
[`${OTHER}::string::shared-field`, 'value from the previous target'],
654+
[`${PROD}::string::shared-field`, 'value for this target'],
655+
]),
656+
],
657+
]),
658+
})
659+
660+
expect(writtenBlock().subBlocks?.['shared-field']?.value).toBe('value for this target')
661+
})
662+
663+
it('restores a boolean input as a real boolean, not the string it was stored as', async () => {
664+
// The dependent store holds strings, but a boolean field's sub-block is a `switch` and the
665+
// canvas stores it as a boolean — `'false'` left as text is truthy to the child workflow.
666+
mockSaveWorkflowToNormalizedTables.mockResolvedValue({ success: true })
667+
668+
await copyWorkflowStateIntoTarget({
669+
...baseParams,
670+
tx: stubTx(),
671+
transformBlockType: (type) => (type === UAT ? PROD : type),
672+
dependentOverrides: new Map([
673+
[
674+
'tgt-blk-cb',
675+
new Map([
676+
[`${PROD}::boolean::flag-on`, 'true'],
677+
[`${PROD}::boolean::flag-off`, 'false'],
678+
[`${PROD}::string::text`, 'true'],
679+
]),
680+
],
681+
]),
682+
})
683+
684+
const subBlocks = writtenBlock().subBlocks ?? {}
685+
expect(subBlocks['flag-on']?.value).toBe(true)
686+
expect(subBlocks['flag-off']?.value).toBe(false)
687+
// A string field whose value happens to read "true" stays a string.
688+
expect(subBlocks.text?.value).toBe('true')
689+
})
634690
})

‎apps/sim/ee/workspace-forking/lib/copy/copy-workflows.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -562,7 +562,7 @@ export async function copyWorkflowStateIntoTarget(
562562
// wholesale because ALL of a custom block's inputs are reconfigurable, not a `dependsOn`
563563
// subset. `applyDependentOverrides` above is a no-op for them: it allowlists on
564564
// `dependsOn` + `selectorKey`, which no custom-block input has.
565-
subBlocks = replaceCustomBlockInputs(subBlocks, blockOverrides)
565+
subBlocks = replaceCustomBlockInputs(subBlocks, blockOverrides, nextBlockType)
566566
}
567567

568568
newBlocks[newBlockId] = {

‎apps/sim/ee/workspace-forking/lib/mapping/custom-block-reconfigs.test.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,12 @@ describe('collectForkCustomBlockReconfigs', () => {
6060
const out = await collectForkCustomBlockReconfigs({ ...baseParams, resolve: swapResolver })
6161

6262
expect(out).toHaveLength(2)
63-
expect(out.map((f) => f.subBlockKey)).toEqual(['field-a', 'field-b'])
63+
// Namespaced by TARGET type and FIELD type: re-pointing the block again cannot reuse these
64+
// values, and the apply side can restore a boolean's real type without a second lookup.
65+
expect(out.map((f) => f.subBlockKey)).toEqual([
66+
`${PROD}::string::field-a`,
67+
`${PROD}::object::field-b`,
68+
])
6469
expect(out[0]).toMatchObject({
6570
parentKind: 'custom-block',
6671
// Joined to its own mapping row on (parentKind, parentSourceId) — the SOURCE type is

‎apps/sim/ee/workspace-forking/lib/mapping/custom-block-reconfigs.ts‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,10 @@ import type { ForkDependentReconfig } from '@/lib/api/contracts/workspace-fork'
44
import { resolveCustomBlockToolBinding } from '@/lib/workflows/custom-blocks/operations'
55
import { isCustomBlockType } from '@/blocks/custom/build-config'
66
import type { ForkBlockIdResolver } from '@/ee/workspace-forking/lib/remap/block-identity'
7-
import type { ForkReferenceResolver } from '@/ee/workspace-forking/lib/remap/remap-references'
7+
import {
8+
customBlockInputStorageKey,
9+
type ForkReferenceResolver,
10+
} from '@/ee/workspace-forking/lib/remap/remap-references'
811
import type { WorkflowState } from '@/stores/workflows/workflow/types'
912

1013
const logger = createLogger('ForkCustomBlockReconfigs')
@@ -97,10 +100,13 @@ export async function collectForkCustomBlockReconfigs(
97100
const binding = bindingByType.get(swap.targetType)
98101
if (!binding) continue
99102
for (const field of binding.inputFields) {
100-
// Sub-blocks are keyed by the field's stable id, falling back to its name for legacy
101-
// fields with none — the same rule `buildCustomBlockConfig` keys its sub-blocks on, so
102-
// a stored value lands on the sub-block the canvas will read it from.
103-
const subBlockKey = field.id ?? field.name
103+
// The canvas keys a custom block's sub-blocks by the field's stable id, falling back to
104+
// its name for legacy fields with none. The STORAGE key namespaces that by the target
105+
// type and field type, so re-pointing the block again cannot reuse this target's values
106+
// and the apply side can restore a boolean's real type — see
107+
// `customBlockInputStorageKey`.
108+
const fieldId = field.id ?? field.name
109+
const subBlockKey = customBlockInputStorageKey(swap.targetType, field.type, fieldId)
104110
out.push({
105111
parentKind: 'custom-block',
106112
parentSourceId: swap.sourceType,
@@ -115,7 +121,7 @@ export async function collectForkCustomBlockReconfigs(
115121
// something else. The diff overlays the stored value on top of this.
116122
currentValue: '',
117123
sourceValue: '',
118-
required: binding.requiredInputIds.includes(subBlockKey),
124+
required: binding.requiredInputIds.includes(fieldId),
119125
consumesContextKeys: [],
120126
context: {},
121127
})

0 commit comments

Comments
 (0)