Skip to content

Commit 592e2d5

Browse files
committed
merge: graft the fork-sync omission fix onto #6776
#6776 landed on staging as a competing fix for the same fork-sync defects this branch addressed. Take its work wholesale and keep only the part of ours it does not cover. Kept from upstream (#6776): the `sameDependencyScope` cascade guard, `getDisplayedDependentFields` with the "Edit configuration" chip, per-scope provider indexing, the `blockChainState(block, field, ...)` scope filter, and the `dependent-reconfigs` / `workspace-fork` contract changes that emit `dependencyScope` and per-scope context keys for nested tool params. Kept from this branch: `DEPENDENT_CLEARED_BY_PARENT` and `submittedDependentValue`. The scope guard decides WHICH descendants a re-pick invalidates; the sentinel decides HOW an invalidated one is represented, and those are different layers. Without the sentinel the cascade still writes `''` into the reconfig map and `buildDependentValues` still submits it, so a hidden optional dependent's stored target value is destroyed by a parent re-pick the user never applied to it. The scope guard does not close this: two top-level block subblocks both have `dependencyScope === undefined`, so it is a no-op there. #6776 also widens the exposure by emitting context keys for nested tool params that previously could never be cascaded onto. Also kept: the `previousValue` no-op guard in `applyDependentRepick` (a separate bug - re-selecting the value a field already shows must not invalidate its descendants) and the post-sync reset in `use-fork-sync`. Dropped from this branch: the sticky-visibility predicate in `isDependentConfigurationActionable` and its three tests. It and upstream's edit chip are two mechanisms for one visibility problem, and it is unnecessary - a marked REQUIRED field reads as `''` through `effectiveDependentValue`, so the existing `required && value === ''` arm keeps it on screen and keeps it gating Sync. Only marked OPTIONAL fields drop out of the default view, and those are omitted from the payload, so hiding them costs nothing; the edit chip brings them back. Reconciled the cascade assertions in `dependent-value.test.ts` to the sentinel, including upstream's nested-tool-instance case.
2 parents 0c2cb08 + 746a449 commit 592e2d5

16 files changed

Lines changed: 583 additions & 237 deletions

File tree

apps/docs/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@
2727
"fumadocs-mdx": "14.3.2",
2828
"fumadocs-openapi": "10.8.1",
2929
"fumadocs-ui": "16.8.5",
30-
"next": "16.2.12",
30+
"next": "16.3.1",
3131
"next-themes": "^0.4.6",
3232
"react": "19.2.4",
3333
"react-dom": "19.2.4",

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

Lines changed: 117 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
effectiveCopyDependentValue,
1111
effectiveDependentValue,
1212
getActionableDependentFields,
13+
getDisplayedDependentFields,
1314
isDependentClearedByParent,
1415
isDependentConfigurationActionable,
1516
submittedDependentValue,
@@ -177,6 +178,63 @@ describe('applyDependentRepick', () => {
177178
expect(next).toEqual({ [dependentKey(spreadsheet)]: 'sheet-doc' })
178179
expect(effectiveDependentValue(range, next, false)).toBe('Sheet1!A1:D')
179180
})
181+
182+
it('only changes the selected field when it provides no selector context', () => {
183+
const leaf = field({ subBlockKey: 'issueKey', currentValue: 'ISSUE-1' })
184+
const unrelated = field({ subBlockKey: 'label', currentValue: 'keep-me' })
185+
186+
expect(
187+
applyDependentRepick(
188+
{ [dependentKey(unrelated)]: 'still-keep-me' },
189+
leaf,
190+
[leaf, unrelated],
191+
'ISSUE-2'
192+
)
193+
).toEqual({
194+
[dependentKey(leaf)]: 'ISSUE-2',
195+
[dependentKey(unrelated)]: 'still-keep-me',
196+
})
197+
})
198+
199+
it('does not clear a descendant belonging to another nested tool instance', () => {
200+
const projectOne = field({
201+
subBlockKey: 'tools[0].projectId',
202+
dependencyScope: 'tools[0]',
203+
providesContextKey: 'projectId',
204+
})
205+
const issueOne = field({
206+
subBlockKey: 'tools[0].issueKey',
207+
dependencyScope: 'tools[0]',
208+
consumesContextKeys: ['projectId'],
209+
})
210+
const projectTwo = field({
211+
subBlockKey: 'tools[1].projectId',
212+
dependencyScope: 'tools[1]',
213+
providesContextKey: 'projectId',
214+
})
215+
const issueTwo = field({
216+
subBlockKey: 'tools[1].issueKey',
217+
dependencyScope: 'tools[1]',
218+
consumesContextKeys: ['projectId'],
219+
})
220+
const previous = {
221+
[dependentKey(issueOne)]: 'P1-1',
222+
[dependentKey(issueTwo)]: 'P2-1',
223+
}
224+
225+
expect(
226+
applyDependentRepick(
227+
previous,
228+
projectOne,
229+
[projectOne, issueOne, projectTwo, issueTwo],
230+
'P1-NEW'
231+
)
232+
).toEqual({
233+
[dependentKey(projectOne)]: 'P1-NEW',
234+
[dependentKey(issueOne)]: DEPENDENT_CLEARED_BY_PARENT,
235+
[dependentKey(issueTwo)]: 'P2-1',
236+
})
237+
})
180238
})
181239

182240
describe('submittedDependentValue', () => {
@@ -261,23 +319,6 @@ describe('submittedDependentValue', () => {
261319
expect(submittedDependentValue(untouched, {}, mappedParent)).toBe('INBOX')
262320
expect(submittedDependentValue(copied, {}, { copying: true, parentChanged: false })).toBe('doc')
263321
})
264-
265-
it('only changes the selected field when it provides no selector context', () => {
266-
const leaf = field({ subBlockKey: 'issueKey', currentValue: 'ISSUE-1' })
267-
const unrelated = field({ subBlockKey: 'label', currentValue: 'keep-me' })
268-
269-
expect(
270-
applyDependentRepick(
271-
{ [dependentKey(unrelated)]: 'still-keep-me' },
272-
leaf,
273-
[leaf, unrelated],
274-
'ISSUE-2'
275-
)
276-
).toEqual({
277-
[dependentKey(leaf)]: 'ISSUE-2',
278-
[dependentKey(unrelated)]: 'still-keep-me',
279-
})
280-
})
281322
})
282323

283324
describe('isDependentConfigurationActionable', () => {
@@ -351,47 +392,6 @@ describe('isDependentConfigurationActionable', () => {
351392
).toBe(true)
352393
})
353394

354-
it('keeps a required field on screen after the user fills it in', () => {
355-
const issue = field({ subBlockKey: 'issueKey', required: true, currentValue: '' })
356-
const picked = applyDependentRepick({}, issue, [issue], 'ISSUE-7', '')
357-
358-
expect(
359-
isDependentConfigurationActionable(issue, picked, {
360-
parentResolved: true,
361-
parentChanged: false,
362-
copying: false,
363-
})
364-
).toBe(true)
365-
})
366-
367-
it('keeps an optional descendant on screen once a parent re-pick blanked it', () => {
368-
const spreadsheet = field({
369-
subBlockKey: 'spreadsheetId',
370-
currentValue: 'doc-old',
371-
providesContextKey: 'spreadsheetId',
372-
})
373-
const range = field({
374-
subBlockKey: 'range',
375-
currentValue: 'A1:D50',
376-
consumesContextKeys: ['spreadsheetId'],
377-
})
378-
const next = applyDependentRepick(
379-
{},
380-
spreadsheet,
381-
[spreadsheet, range],
382-
'doc-new',
383-
effectiveDependentValue(spreadsheet, {}, false)
384-
)
385-
386-
expect(
387-
isDependentConfigurationActionable(range, next, {
388-
parentResolved: true,
389-
parentChanged: false,
390-
copying: false,
391-
})
392-
).toBe(true)
393-
})
394-
395395
it('shows a required field that a parent re-pick blanked (it blocks Sync)', () => {
396396
const spreadsheet = field({
397397
subBlockKey: 'spreadsheetId',
@@ -503,35 +503,6 @@ describe('getActionableDependentFields', () => {
503503
).toEqual(['spreadsheetId', 'sheetName'])
504504
})
505505

506-
it("keeps the block's only required field (and its provider) after the user fills it in", () => {
507-
const spreadsheet = field({
508-
subBlockKey: 'spreadsheetId',
509-
title: 'Spreadsheet',
510-
currentValue: 'doc-target',
511-
providesContextKey: 'spreadsheetId',
512-
})
513-
const sheet = field({
514-
subBlockKey: 'sheetName',
515-
title: 'Sheet',
516-
currentValue: '',
517-
required: true,
518-
consumesContextKeys: ['spreadsheetId'],
519-
})
520-
const fields = [spreadsheet, sheet]
521-
522-
// The card is on screen because the required sheet is missing; the user picks one.
523-
expect(
524-
getActionableDependentFields(fields, {}, unchangedMappedParent).map((f) => f.subBlockKey)
525-
).toEqual(['spreadsheetId', 'sheetName'])
526-
const picked = applyDependentRepick({}, sheet, fields, 'Sheet1', '')
527-
528-
// The block still has configurable fields, so its workflow card cannot unmount underneath
529-
// the user - who would otherwise be unable to see or change what they just picked.
530-
expect(
531-
getActionableDependentFields(fields, picked, unchangedMappedParent).map((f) => f.subBlockKey)
532-
).toEqual(['spreadsheetId', 'sheetName'])
533-
})
534-
535506
it('walks transitive providers and leaves unrelated optional fields hidden', () => {
536507
const unrelated = field({
537508
subBlockKey: 'optionalLabel',
@@ -567,4 +538,63 @@ describe('getActionableDependentFields', () => {
567538
).map((dependent) => dependent.subBlockKey)
568539
).toEqual(['siteId', 'driveId', 'spreadsheetId'])
569540
})
541+
542+
it('finds a required child provider only within the same nested tool instance', () => {
543+
const projectOne = field({
544+
subBlockKey: 'tools[0].projectId',
545+
dependencyScope: 'tools[0]',
546+
providesContextKey: 'projectId',
547+
})
548+
const projectTwo = field({
549+
subBlockKey: 'tools[1].projectId',
550+
dependencyScope: 'tools[1]',
551+
providesContextKey: 'projectId',
552+
})
553+
const issueOne = field({
554+
subBlockKey: 'tools[0].issueKey',
555+
dependencyScope: 'tools[0]',
556+
currentValue: '',
557+
required: true,
558+
consumesContextKeys: ['projectId'],
559+
})
560+
561+
expect(
562+
getActionableDependentFields(
563+
[projectOne, projectTwo, issueOne],
564+
{},
565+
unchangedMappedParent
566+
).map((dependent) => dependent.subBlockKey)
567+
).toEqual(['tools[0].projectId', 'tools[0].issueKey'])
568+
})
569+
})
570+
571+
describe('getDisplayedDependentFields', () => {
572+
const unchangedMappedParent = {
573+
parentResolved: true,
574+
parentChanged: false,
575+
copying: false,
576+
}
577+
578+
it('reveals configured and optional fields only after the explicit edit action', () => {
579+
const configuredRequired = field({ subBlockKey: 'projectId', required: true })
580+
const optional = field({ subBlockKey: 'issueKey', currentValue: '', required: false })
581+
582+
expect(
583+
getDisplayedDependentFields([configuredRequired, optional], {}, unchangedMappedParent, false)
584+
).toEqual([])
585+
expect(
586+
getDisplayedDependentFields([configuredRequired, optional], {}, unchangedMappedParent, true)
587+
).toEqual([configuredRequired, optional])
588+
})
589+
590+
it('never shows selectors before their parent mapping is resolved', () => {
591+
expect(
592+
getDisplayedDependentFields(
593+
[field()],
594+
{},
595+
{ ...unchangedMappedParent, parentResolved: false },
596+
true
597+
)
598+
).toEqual([])
599+
})
570600
})

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

Lines changed: 40 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,10 @@ export function dependentKey(dependent: ForkDependentReconfig): string {
55
return `${dependent.targetWorkflowId}:${dependent.targetBlockId}:${dependent.subBlockKey}`
66
}
77

8+
function sameDependencyScope(left: ForkDependentReconfig, right: ForkDependentReconfig): boolean {
9+
return left.dependencyScope === right.dependencyScope
10+
}
11+
812
/**
913
* Marker stored for a descendant whose value an in-session parent re-pick invalidated, kept
1014
* distinct from the user's own empty pick. It reads as blank everywhere it is consumed - the
@@ -47,7 +51,13 @@ export function applyDependentRepick(
4751

4852
for (const field of blockFields) {
4953
const fieldKey = dependentKey(field)
50-
if (visitedFields.has(fieldKey) || !field.consumesContextKeys.includes(contextKey)) continue
54+
if (
55+
!sameDependencyScope(changedField, field) ||
56+
visitedFields.has(fieldKey) ||
57+
!field.consumesContextKeys.includes(contextKey)
58+
) {
59+
continue
60+
}
5161

5262
visitedFields.add(fieldKey)
5363
nextState[fieldKey] = DEPENDENT_CLEARED_BY_PARENT
@@ -136,10 +146,11 @@ export interface DependentConfigurationState {
136146
* because its children resolve in a different scope. An unchanged mapping only needs a selector
137147
* when a required value is missing; its stored values are already valid and sync-ready.
138148
*
139-
* Anything the session already touched stays shown - the user picked it, cleared it, or a parent
140-
* re-pick invalidated it. Re-deriving pure satisfaction would pull the field (and, when it is its
141-
* block's only one, the whole workflow card) out from under the user the instant they filled it
142-
* in, and would hide a descendant a parent re-pick just blanked.
149+
* A field a parent re-pick invalidated reads as blank through `effectiveDependentValue`, so a
150+
* REQUIRED one stays on screen here and keeps gating Sync. An optional one drops out of the
151+
* default view - `getDisplayedDependentFields` brings it back under explicit edit mode - and
152+
* hiding it loses nothing, because `submittedDependentValue` omits it from the payload rather
153+
* than blanking the target.
143154
*/
144155
export function isDependentConfigurationActionable(
145156
field: ForkDependentReconfig,
@@ -148,7 +159,6 @@ export function isDependentConfigurationActionable(
148159
): boolean {
149160
if (!state.parentResolved) return false
150161
if (state.parentChanged || state.copying) return true
151-
if (reconfig[dependentKey(field)] !== undefined) return true
152162
return field.required && effectiveDependentValue(field, reconfig, false) === ''
153163
}
154164

@@ -165,17 +175,23 @@ export function getActionableDependentFields(
165175
const actionable = new Set(
166176
fields.filter((field) => isDependentConfigurationActionable(field, reconfig, state))
167177
)
168-
const providersByContextKey = new Map<string, ForkDependentReconfig>()
178+
const providersByScope = new Map<string | undefined, Map<string, ForkDependentReconfig>>()
169179
for (const field of fields) {
170-
if (field.providesContextKey) providersByContextKey.set(field.providesContextKey, field)
180+
if (!field.providesContextKey) continue
181+
let providers = providersByScope.get(field.dependencyScope)
182+
if (!providers) {
183+
providers = new Map()
184+
providersByScope.set(field.dependencyScope, providers)
185+
}
186+
providers.set(field.providesContextKey, field)
171187
}
172188

173189
const pending = Array.from(actionable)
174190
for (let index = 0; index < pending.length; index += 1) {
175191
const field = pending[index]
176192
if (!field) continue
177193
for (const contextKey of field.consumesContextKeys) {
178-
const provider = providersByContextKey.get(contextKey)
194+
const provider = providersByScope.get(field.dependencyScope)?.get(contextKey)
179195
if (!provider || actionable.has(provider)) continue
180196
actionable.add(provider)
181197
pending.push(provider)
@@ -184,3 +200,18 @@ export function getActionableDependentFields(
184200

185201
return fields.filter((field) => actionable.has(field))
186202
}
203+
204+
/**
205+
* Fields rendered in the mapping UI. Required missing fields remain visible by default; an
206+
* explicit edit action reveals every active selector under a resolved parent without changing
207+
* which fields gate Sync.
208+
*/
209+
export function getDisplayedDependentFields(
210+
fields: ForkDependentReconfig[],
211+
reconfig: Record<string, string>,
212+
state: DependentConfigurationState,
213+
showConfigured: boolean
214+
): ForkDependentReconfig[] {
215+
if (!state.parentResolved) return []
216+
return showConfigured ? fields : getActionableDependentFields(fields, reconfig, state)
217+
}

0 commit comments

Comments
 (0)