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>
This commit is contained in:
Vikhyath Mondreti
2026-08-19 21:20:12 -07:00
parent d702e598ca
commit 1f423ab5c4
4 changed files with 155 additions and 33 deletions
@@ -25,6 +25,7 @@ import type { ForkBlockIdResolver } from '@/ee/workspace-forking/lib/remap/block
import { toScannerBlocks } from '@/ee/workspace-forking/lib/remap/reference-scan'
import {
createCanonicalModeGates,
reconfigurableDependentIds,
scanWorkflowReferences,
} from '@/ee/workspace-forking/lib/remap/remap-references'
import type { WorkflowState } from '@/stores/workflows/workflow/types'
@@ -48,12 +49,6 @@ interface ReconfigItem {
* intentionally excluded: their tool dependent has no `selectorKey` and a separate
* (non-`useSelectorOptions`) stack, so it falls back to the needs-config surfacing.
*/
/**
* Dependent sub-block types the modal renders as a free-text field rather than a picker.
* They carry no options to fetch, so they need no selector just somewhere to type.
*/
const TEXT_DEPENDENT_TYPES = new Set<string>(['short-input', 'long-input'])
const PARENT_ANCHORS: ReadonlyArray<{
subBlockType: string
parentKind: ForkDependentReconfig['parentKind']
@@ -136,22 +131,9 @@ function emitAnchoredDependents(params: EmitAnchoredParams): void {
const canonicalIndex = buildCanonicalIndex(config.subBlocks)
const gates = createCanonicalModeGates(config.subBlocks, values, canonicalModes)
const configById = new Map(config.subBlocks.filter((cfg) => cfg.id).map((cfg) => [cfg.id, cfg]))
// Text members of a canonical pair whose basic side IS a selector. The pair already
// represents the field: its selector member is offered, and the manual member is verbatim by
// policy (`clearDependentsOnRemap` never clears it), so offering it too would show the same
// concept twice and invite writing into the inactive half.
const canonicalWithSelector = new Set(
config.subBlocks
.filter((cfg) => cfg.canonicalParamId && cfg.selectorKey)
.map((cfg) => cfg.canonicalParamId)
)
const canonicalPairMembers = new Set(
config.subBlocks
.filter(
(cfg) => cfg.id && cfg.canonicalParamId && canonicalWithSelector.has(cfg.canonicalParamId)
)
.map((cfg) => cfg.id as string)
)
// Shared with `applyDependentOverrides`, so what the modal offers is exactly what the sync
// can write back — the two encoded this rule separately once and drifted.
const reconfigurableIds = reconfigurableDependentIds(config.subBlocks)
// A field could hang off two anchors (or be reachable via two paths); emit it once.
const seen = new Set<string>()
@@ -198,10 +180,7 @@ function emitAnchoredDependents(params: EmitAnchoredParams): void {
// transitive dependent of a remapped parent on EVERY sync (a credential mapped across
// environments changes value each time), so a field the modal never offered was
// re-emptied on every push and could not be fixed by setting it in the target either.
if (!dependent?.id) continue
const isTextDependent =
TEXT_DEPENDENT_TYPES.has(dependent.type) && !canonicalPairMembers.has(dependent.id)
if (!dependent.selectorKey && !isTextDependent) continue
if (!dependent?.id || !reconfigurableIds.has(dependent.id)) continue
// Skip fields gated off by their `condition` - a selector under a now-inactive
// operation (e.g. a move-only label while the block reads) isn't in play. We do
// NOT require a source value: an active selector the source left empty is still
@@ -1,10 +1,13 @@
/**
* @vitest-environment node
*/
import { describe, expect, it } from 'vitest'
import { describe, expect, it, vi } from 'vitest'
import { getBlock } from '@/blocks/registry'
import {
applyDependentOverrides,
customBlockInputStorageKey,
type ForkReferenceResolver,
reconfigurableDependentIds,
remapForkBlockType,
replaceCustomBlockInputs,
scanWorkflowReferences,
@@ -195,3 +198,77 @@ describe('replaceCustomBlockInputs target carry-over', () => {
expect(result.workflowId).toEqual({ value: 'wf-prod' })
})
})
describe('reconfigurableDependentIds', () => {
const SUBS = [
{ id: 'credential', type: 'oauth-input' },
{ id: 'issueType', type: 'short-input', dependsOn: ['credential'] },
{ id: 'notes', type: 'long-input', dependsOn: ['credential'] },
{
id: 'labelId',
type: 'file-selector',
dependsOn: ['credential'],
selectorKey: 'gmail.labels',
},
{
id: 'projectId',
type: 'project-selector',
dependsOn: ['credential'],
selectorKey: 'jira.projects',
canonicalParamId: 'projectId',
},
{
id: 'manualProjectId',
type: 'short-input',
dependsOn: ['credential'],
canonicalParamId: 'projectId',
},
{ id: 'watchColumns', type: 'dropdown', dependsOn: ['credential'] },
{ id: 'standalone', type: 'short-input' },
]
it('offers selector-backed and plain text dependents', () => {
const allowed = reconfigurableDependentIds(SUBS)
expect([...allowed].sort()).toEqual(['issueType', 'labelId', 'notes', 'projectId'])
})
it('excludes the manual half of a selector-backed canonical pair', () => {
expect(reconfigurableDependentIds(SUBS).has('manualProjectId')).toBe(false)
})
it('excludes a dependent the modal can render no control for', () => {
// A `dropdown` with no selector has options to fetch and no way to fetch them here.
expect(reconfigurableDependentIds(SUBS).has('watchColumns')).toBe(false)
})
it('excludes a field that depends on nothing', () => {
expect(reconfigurableDependentIds(SUBS).has('standalone')).toBe(false)
})
it('is the SAME set the sync actually writes back', () => {
// The collector offers these and `applyDependentOverrides` writes them. Encoding the rule
// twice is what let text dependents be collected, stored, gated on — then silently dropped
// on apply, leaving the field wiped on every push with nowhere for the value to go.
vi.mocked(getBlock).mockReturnValue({ type: 'jira', subBlocks: SUBS } as never)
const applied = applyDependentOverrides(
{
issueType: { value: 'old' },
labelId: { value: 'old' },
manualProjectId: { value: 'keep-me' },
watchColumns: { value: 'keep-me' },
},
'jira',
new Map([
['issueType', 'Bug'],
['labelId', 'LABEL_1'],
['manualProjectId', 'hacked'],
['watchColumns', 'hacked'],
])
)
expect(applied.issueType).toEqual({ value: 'Bug' })
expect(applied.labelId).toEqual({ value: 'LABEL_1' })
// Not offered, so not writable — an override naming one must not slip through.
expect(applied.manualProjectId).toEqual({ value: 'keep-me' })
expect(applied.watchColumns).toEqual({ value: 'keep-me' })
})
})
@@ -1629,6 +1629,48 @@ function applyNestedToolOverrides(
* set a parent/credential field (bypassing mapping validation) or inject a bogus subblock.
* Returns a new record only when something applied.
*/
/** Sub-block types the fork sync modal renders as a free-text field rather than a picker. */
export const TEXT_DEPENDENT_TYPES = new Set<string>(['short-input', 'long-input'])
/**
* The dependents of a remapped parent that the sync modal can offer AND the sync can apply.
*
* ONE definition on purpose. The collector and the apply side each encoded this rule separately
* and drifted the moment text fields were added: they were collected, stored, and gated on by
* the Sync button, then dropped here because the allowlist still demanded a `selectorKey`. The
* field stayed wiped on every push and the typed value went nowhere.
*
* A text member of a canonical pair whose basic side is a selector is excluded: the pair is
* already represented by its selector member, and the manual member is verbatim by policy.
*/
export function reconfigurableDependentIds(
subBlocks: ReadonlyArray<{
id?: string
type?: string
dependsOn?: unknown
selectorKey?: string
canonicalParamId?: string
}>
): Set<string> {
const canonicalWithSelector = new Set(
subBlocks
.filter((cfg) => cfg.canonicalParamId && cfg.selectorKey)
.map((cfg) => cfg.canonicalParamId)
)
const allowed = new Set<string>()
for (const cfg of subBlocks) {
if (!cfg.id || !cfg.dependsOn) continue
if (cfg.selectorKey) {
allowed.add(cfg.id)
continue
}
if (!TEXT_DEPENDENT_TYPES.has(cfg.type ?? '')) continue
if (cfg.canonicalParamId && canonicalWithSelector.has(cfg.canonicalParamId)) continue
allowed.add(cfg.id)
}
return allowed
}
export function applyDependentOverrides(
subBlocks: SubBlockRecord,
blockType: string,
@@ -1637,12 +1679,10 @@ export function applyDependentOverrides(
const config = getBlock(blockType)
if (!config || overrides.size === 0) return subBlocks
const allowedTopLevel = new Set<string>()
const allowedTopLevel = reconfigurableDependentIds(config.subBlocks)
const toolInputIds = new Set<string>()
for (const cfg of config.subBlocks) {
if (!cfg.id) continue
if (cfg.dependsOn && cfg.selectorKey) allowedTopLevel.add(cfg.id)
if (cfg.type === 'tool-input') toolInputIds.add(cfg.id)
if (cfg.id && cfg.type === 'tool-input') toolInputIds.add(cfg.id)
}
const nestedByTool = new Map<string, Array<{ index: number; paramId: string; value: string }>>()
@@ -1,8 +1,13 @@
import { useMemo } from 'react'
import { useCallback, useMemo } from 'react'
import { useQueries } from '@tanstack/react-query'
import { buildSelectorContextFromBlock } from '@/lib/workflows/subblocks/context'
import { summarizeNames } from '@/lib/workflows/subblocks/display'
import type { SubBlockConfig } from '@/blocks/types'
import { getSelectorDefinition } from '@/hooks/selectors/registry'
import type { SelectorContext } from '@/hooks/selectors/types'
import { useWorkflowRegistry } from '@/stores/workflows/registry/store'
import { useSubBlockStore } from '@/stores/workflows/subblock/store'
import { useWorkflowStore } from '@/stores/workflows/workflow/store'
export const DYNAMIC_SUBBLOCK_OPTION_STALE_TIME = 30 * 1000
@@ -49,6 +54,27 @@ export function useDynamicSubBlockOptionDisplayName({
// per-block resolver any more, so a selector without one simply renders the raw id.
const definition = subBlock?.selectorKey ? getSelectorDefinition(subBlock.selectorKey) : undefined
const fetchById = definition?.fetchById
/**
* The block's own values, the same context the canvas builds. A `workspaceId`-only context
* silently fails every selector scoped by a sibling `workspace.credentialGroupProviders`
* needs the group before it can name a provider, so the card fell back to raw ids.
*/
const buildResolverContext = useCallback((): SelectorContext => {
const activeWorkflowId = useWorkflowRegistry.getState().activeWorkflowId
const block = blockId ? useWorkflowStore.getState().blocks[blockId] : undefined
if (!block?.type || !blockId) return { workspaceId }
const live = activeWorkflowId
? (useSubBlockStore.getState().workflowValues[activeWorkflowId]?.[blockId] ?? {})
: {}
const merged: Record<string, { value?: unknown }> = { ...(block.subBlocks ?? {}) }
for (const [id, value] of Object.entries(live)) merged[id] = { ...merged[id], value }
return buildSelectorContextFromBlock(block.type, merged, {
workflowId: activeWorkflowId ?? undefined,
workspaceId,
canonicalModes: block.data?.canonicalModes,
})
}, [blockId, workspaceId])
const canResolve = Boolean(blockId && fetchById && optionIds.length > 0)
const queries = useQueries({
@@ -61,7 +87,7 @@ export function useDynamicSubBlockOptionDisplayName({
}
return fetchById({
key: definition.key,
context: { workspaceId },
context: buildResolverContext(),
detailId: optionId,
signal,
})