Skip to content

Commit 1f423ab

Browse files
icecrasher321claude
andcommitted
fix(workspace-forking): actually apply text dependents, and resolve labels against the full selector context
Both from Bugbot; both real, both mine. **Text dependents never persisted.** `applyDependentOverrides` allowlisted `dependsOn && selectorKey`, so the plain text fields the collector started emitting were offered in the modal, stored, and gated on by the Sync button — then dropped on apply. The field stayed wiped on every push and the typed value went nowhere, which is the exact treadmill the feature existed to end. The cause was the rule being written twice. `reconfigurableDependentIds` is now the single definition of "a dependent the modal can offer AND the sync can write back", used by the collector and by the apply side. A test asserts the two agree by round-tripping through `applyDependentOverrides`, and fails against the old allowlist. **Provider labels stayed raw ids.** `useDynamicSubBlockOptionDisplayName` called `fetchById` with a `workspaceId`-only context, which silently fails any selector scoped by a sibling — `workspace.credentialGroupProviders` needs the group before it can name a provider, so the `fetchById` restored last round returned null every time. It now builds the block's real context with `buildSelectorContextFromBlock`, the same one the canvas uses. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent d702e59 commit 1f423ab

4 files changed

Lines changed: 155 additions & 33 deletions

File tree

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

Lines changed: 5 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import type { ForkBlockIdResolver } from '@/ee/workspace-forking/lib/remap/block
2525
import { toScannerBlocks } from '@/ee/workspace-forking/lib/remap/reference-scan'
2626
import {
2727
createCanonicalModeGates,
28+
reconfigurableDependentIds,
2829
scanWorkflowReferences,
2930
} from '@/ee/workspace-forking/lib/remap/remap-references'
3031
import type { WorkflowState } from '@/stores/workflows/workflow/types'
@@ -48,12 +49,6 @@ interface ReconfigItem {
4849
* intentionally excluded: their tool dependent has no `selectorKey` and a separate
4950
* (non-`useSelectorOptions`) stack, so it falls back to the needs-config surfacing.
5051
*/
51-
/**
52-
* Dependent sub-block types the modal renders as a free-text field rather than a picker.
53-
* They carry no options to fetch, so they need no selector — just somewhere to type.
54-
*/
55-
const TEXT_DEPENDENT_TYPES = new Set<string>(['short-input', 'long-input'])
56-
5752
const PARENT_ANCHORS: ReadonlyArray<{
5853
subBlockType: string
5954
parentKind: ForkDependentReconfig['parentKind']
@@ -136,22 +131,9 @@ function emitAnchoredDependents(params: EmitAnchoredParams): void {
136131
const canonicalIndex = buildCanonicalIndex(config.subBlocks)
137132
const gates = createCanonicalModeGates(config.subBlocks, values, canonicalModes)
138133
const configById = new Map(config.subBlocks.filter((cfg) => cfg.id).map((cfg) => [cfg.id, cfg]))
139-
// Text members of a canonical pair whose basic side IS a selector. The pair already
140-
// represents the field: its selector member is offered, and the manual member is verbatim by
141-
// policy (`clearDependentsOnRemap` never clears it), so offering it too would show the same
142-
// concept twice and invite writing into the inactive half.
143-
const canonicalWithSelector = new Set(
144-
config.subBlocks
145-
.filter((cfg) => cfg.canonicalParamId && cfg.selectorKey)
146-
.map((cfg) => cfg.canonicalParamId)
147-
)
148-
const canonicalPairMembers = new Set(
149-
config.subBlocks
150-
.filter(
151-
(cfg) => cfg.id && cfg.canonicalParamId && canonicalWithSelector.has(cfg.canonicalParamId)
152-
)
153-
.map((cfg) => cfg.id as string)
154-
)
134+
// Shared with `applyDependentOverrides`, so what the modal offers is exactly what the sync
135+
// can write back — the two encoded this rule separately once and drifted.
136+
const reconfigurableIds = reconfigurableDependentIds(config.subBlocks)
155137
// A field could hang off two anchors (or be reachable via two paths); emit it once.
156138
const seen = new Set<string>()
157139

@@ -198,10 +180,7 @@ function emitAnchoredDependents(params: EmitAnchoredParams): void {
198180
// transitive dependent of a remapped parent on EVERY sync (a credential mapped across
199181
// environments changes value each time), so a field the modal never offered was
200182
// re-emptied on every push and could not be fixed by setting it in the target either.
201-
if (!dependent?.id) continue
202-
const isTextDependent =
203-
TEXT_DEPENDENT_TYPES.has(dependent.type) && !canonicalPairMembers.has(dependent.id)
204-
if (!dependent.selectorKey && !isTextDependent) continue
183+
if (!dependent?.id || !reconfigurableIds.has(dependent.id)) continue
205184
// Skip fields gated off by their `condition` - a selector under a now-inactive
206185
// operation (e.g. a move-only label while the block reads) isn't in play. We do
207186
// NOT require a source value: an active selector the source left empty is still

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

Lines changed: 78 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,13 @@
11
/**
22
* @vitest-environment node
33
*/
4-
import { describe, expect, it } from 'vitest'
4+
import { describe, expect, it, vi } from 'vitest'
5+
import { getBlock } from '@/blocks/registry'
56
import {
7+
applyDependentOverrides,
68
customBlockInputStorageKey,
79
type ForkReferenceResolver,
10+
reconfigurableDependentIds,
811
remapForkBlockType,
912
replaceCustomBlockInputs,
1013
scanWorkflowReferences,
@@ -195,3 +198,77 @@ describe('replaceCustomBlockInputs target carry-over', () => {
195198
expect(result.workflowId).toEqual({ value: 'wf-prod' })
196199
})
197200
})
201+
202+
describe('reconfigurableDependentIds', () => {
203+
const SUBS = [
204+
{ id: 'credential', type: 'oauth-input' },
205+
{ id: 'issueType', type: 'short-input', dependsOn: ['credential'] },
206+
{ id: 'notes', type: 'long-input', dependsOn: ['credential'] },
207+
{
208+
id: 'labelId',
209+
type: 'file-selector',
210+
dependsOn: ['credential'],
211+
selectorKey: 'gmail.labels',
212+
},
213+
{
214+
id: 'projectId',
215+
type: 'project-selector',
216+
dependsOn: ['credential'],
217+
selectorKey: 'jira.projects',
218+
canonicalParamId: 'projectId',
219+
},
220+
{
221+
id: 'manualProjectId',
222+
type: 'short-input',
223+
dependsOn: ['credential'],
224+
canonicalParamId: 'projectId',
225+
},
226+
{ id: 'watchColumns', type: 'dropdown', dependsOn: ['credential'] },
227+
{ id: 'standalone', type: 'short-input' },
228+
]
229+
230+
it('offers selector-backed and plain text dependents', () => {
231+
const allowed = reconfigurableDependentIds(SUBS)
232+
expect([...allowed].sort()).toEqual(['issueType', 'labelId', 'notes', 'projectId'])
233+
})
234+
235+
it('excludes the manual half of a selector-backed canonical pair', () => {
236+
expect(reconfigurableDependentIds(SUBS).has('manualProjectId')).toBe(false)
237+
})
238+
239+
it('excludes a dependent the modal can render no control for', () => {
240+
// A `dropdown` with no selector has options to fetch and no way to fetch them here.
241+
expect(reconfigurableDependentIds(SUBS).has('watchColumns')).toBe(false)
242+
})
243+
244+
it('excludes a field that depends on nothing', () => {
245+
expect(reconfigurableDependentIds(SUBS).has('standalone')).toBe(false)
246+
})
247+
248+
it('is the SAME set the sync actually writes back', () => {
249+
// The collector offers these and `applyDependentOverrides` writes them. Encoding the rule
250+
// twice is what let text dependents be collected, stored, gated on — then silently dropped
251+
// on apply, leaving the field wiped on every push with nowhere for the value to go.
252+
vi.mocked(getBlock).mockReturnValue({ type: 'jira', subBlocks: SUBS } as never)
253+
const applied = applyDependentOverrides(
254+
{
255+
issueType: { value: 'old' },
256+
labelId: { value: 'old' },
257+
manualProjectId: { value: 'keep-me' },
258+
watchColumns: { value: 'keep-me' },
259+
},
260+
'jira',
261+
new Map([
262+
['issueType', 'Bug'],
263+
['labelId', 'LABEL_1'],
264+
['manualProjectId', 'hacked'],
265+
['watchColumns', 'hacked'],
266+
])
267+
)
268+
expect(applied.issueType).toEqual({ value: 'Bug' })
269+
expect(applied.labelId).toEqual({ value: 'LABEL_1' })
270+
// Not offered, so not writable — an override naming one must not slip through.
271+
expect(applied.manualProjectId).toEqual({ value: 'keep-me' })
272+
expect(applied.watchColumns).toEqual({ value: 'keep-me' })
273+
})
274+
})

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

Lines changed: 44 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1629,6 +1629,48 @@ function applyNestedToolOverrides(
16291629
* set a parent/credential field (bypassing mapping validation) or inject a bogus subblock.
16301630
* Returns a new record only when something applied.
16311631
*/
1632+
/** Sub-block types the fork sync modal renders as a free-text field rather than a picker. */
1633+
export const TEXT_DEPENDENT_TYPES = new Set<string>(['short-input', 'long-input'])
1634+
1635+
/**
1636+
* The dependents of a remapped parent that the sync modal can offer AND the sync can apply.
1637+
*
1638+
* ONE definition on purpose. The collector and the apply side each encoded this rule separately
1639+
* and drifted the moment text fields were added: they were collected, stored, and gated on by
1640+
* the Sync button, then dropped here because the allowlist still demanded a `selectorKey`. The
1641+
* field stayed wiped on every push and the typed value went nowhere.
1642+
*
1643+
* A text member of a canonical pair whose basic side is a selector is excluded: the pair is
1644+
* already represented by its selector member, and the manual member is verbatim by policy.
1645+
*/
1646+
export function reconfigurableDependentIds(
1647+
subBlocks: ReadonlyArray<{
1648+
id?: string
1649+
type?: string
1650+
dependsOn?: unknown
1651+
selectorKey?: string
1652+
canonicalParamId?: string
1653+
}>
1654+
): Set<string> {
1655+
const canonicalWithSelector = new Set(
1656+
subBlocks
1657+
.filter((cfg) => cfg.canonicalParamId && cfg.selectorKey)
1658+
.map((cfg) => cfg.canonicalParamId)
1659+
)
1660+
const allowed = new Set<string>()
1661+
for (const cfg of subBlocks) {
1662+
if (!cfg.id || !cfg.dependsOn) continue
1663+
if (cfg.selectorKey) {
1664+
allowed.add(cfg.id)
1665+
continue
1666+
}
1667+
if (!TEXT_DEPENDENT_TYPES.has(cfg.type ?? '')) continue
1668+
if (cfg.canonicalParamId && canonicalWithSelector.has(cfg.canonicalParamId)) continue
1669+
allowed.add(cfg.id)
1670+
}
1671+
return allowed
1672+
}
1673+
16321674
export function applyDependentOverrides(
16331675
subBlocks: SubBlockRecord,
16341676
blockType: string,
@@ -1637,12 +1679,10 @@ export function applyDependentOverrides(
16371679
const config = getBlock(blockType)
16381680
if (!config || overrides.size === 0) return subBlocks
16391681

1640-
const allowedTopLevel = new Set<string>()
1682+
const allowedTopLevel = reconfigurableDependentIds(config.subBlocks)
16411683
const toolInputIds = new Set<string>()
16421684
for (const cfg of config.subBlocks) {
1643-
if (!cfg.id) continue
1644-
if (cfg.dependsOn && cfg.selectorKey) allowedTopLevel.add(cfg.id)
1645-
if (cfg.type === 'tool-input') toolInputIds.add(cfg.id)
1685+
if (cfg.id && cfg.type === 'tool-input') toolInputIds.add(cfg.id)
16461686
}
16471687

16481688
const nestedByTool = new Map<string, Array<{ index: number; paramId: string; value: string }>>()

apps/sim/hooks/queries/dynamic-subblock-options.ts

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,13 @@
1-
import { useMemo } from 'react'
1+
import { useCallback, useMemo } from 'react'
22
import { useQueries } from '@tanstack/react-query'
3+
import { buildSelectorContextFromBlock } from '@/lib/workflows/subblocks/context'
34
import { summarizeNames } from '@/lib/workflows/subblocks/display'
45
import type { SubBlockConfig } from '@/blocks/types'
56
import { getSelectorDefinition } from '@/hooks/selectors/registry'
7+
import type { SelectorContext } from '@/hooks/selectors/types'
8+
import { useWorkflowRegistry } from '@/stores/workflows/registry/store'
9+
import { useSubBlockStore } from '@/stores/workflows/subblock/store'
10+
import { useWorkflowStore } from '@/stores/workflows/workflow/store'
611

712
export const DYNAMIC_SUBBLOCK_OPTION_STALE_TIME = 30 * 1000
813

@@ -49,6 +54,27 @@ export function useDynamicSubBlockOptionDisplayName({
4954
// per-block resolver any more, so a selector without one simply renders the raw id.
5055
const definition = subBlock?.selectorKey ? getSelectorDefinition(subBlock.selectorKey) : undefined
5156
const fetchById = definition?.fetchById
57+
58+
/**
59+
* The block's own values, the same context the canvas builds. A `workspaceId`-only context
60+
* silently fails every selector scoped by a sibling — `workspace.credentialGroupProviders`
61+
* needs the group before it can name a provider, so the card fell back to raw ids.
62+
*/
63+
const buildResolverContext = useCallback((): SelectorContext => {
64+
const activeWorkflowId = useWorkflowRegistry.getState().activeWorkflowId
65+
const block = blockId ? useWorkflowStore.getState().blocks[blockId] : undefined
66+
if (!block?.type || !blockId) return { workspaceId }
67+
const live = activeWorkflowId
68+
? (useSubBlockStore.getState().workflowValues[activeWorkflowId]?.[blockId] ?? {})
69+
: {}
70+
const merged: Record<string, { value?: unknown }> = { ...(block.subBlocks ?? {}) }
71+
for (const [id, value] of Object.entries(live)) merged[id] = { ...merged[id], value }
72+
return buildSelectorContextFromBlock(block.type, merged, {
73+
workflowId: activeWorkflowId ?? undefined,
74+
workspaceId,
75+
canonicalModes: block.data?.canonicalModes,
76+
})
77+
}, [blockId, workspaceId])
5278
const canResolve = Boolean(blockId && fetchById && optionIds.length > 0)
5379

5480
const queries = useQueries({
@@ -61,7 +87,7 @@ export function useDynamicSubBlockOptionDisplayName({
6187
}
6288
return fetchById({
6389
key: definition.key,
64-
context: { workspaceId },
90+
context: buildResolverContext(),
6591
detailId: optionId,
6692
signal,
6793
})

0 commit comments

Comments
 (0)