Skip to content

Commit 455ae13

Browse files
committed
chore(forking): move the fork-sync changes to their own PR
The dependent-omission fix and the fork file-copy de-duplication are reviewed separately in #6787. They are the only changes here that overlap #6776, and they carry their own design tradeoff, so they should not ride along with the unrelated audit fixes in this PR.
1 parent f4c5ef7 commit 455ae13

7 files changed

Lines changed: 27 additions & 1051 deletions

File tree

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

Lines changed: 4 additions & 160 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,12 @@ import { describe, expect, it } from 'vitest'
55
import type { ForkDependentReconfig } from '@/lib/api/contracts/workspace-fork'
66
import {
77
applyDependentRepick,
8-
DEPENDENT_CLEARED_BY_PARENT,
98
dependentKey,
109
effectiveCopyDependentValue,
1110
effectiveDependentValue,
1211
getActionableDependentFields,
1312
getDisplayedDependentFields,
14-
isDependentClearedByParent,
1513
isDependentConfigurationActionable,
16-
submittedDependentValue,
1714
} from '@/ee/workspace-forking/components/fork-sync/dependent-value'
1815

1916
const field = (overrides: Partial<ForkDependentReconfig> = {}): ForkDependentReconfig => ({
@@ -146,39 +143,15 @@ describe('applyDependentRepick', () => {
146143

147144
expect(next).toEqual({
148145
[dependentKey(site)]: 'site-new',
149-
[dependentKey(drive)]: DEPENDENT_CLEARED_BY_PARENT,
150-
[dependentKey(spreadsheet)]: DEPENDENT_CLEARED_BY_PARENT,
151-
[dependentKey(sheet)]: DEPENDENT_CLEARED_BY_PARENT,
146+
[dependentKey(drive)]: '',
147+
[dependentKey(spreadsheet)]: '',
148+
[dependentKey(sheet)]: '',
152149
[dependentKey(unrelated)]: 'still-keep-me',
153150
})
154151
expect(effectiveDependentValue(drive, next, false)).toBe('')
155152
expect(effectiveCopyDependentValue(sheet, next)).toBe('')
156153
})
157154

158-
it('re-picking the value the field already had leaves its descendants alone', () => {
159-
const spreadsheet = field({
160-
subBlockKey: 'spreadsheetId',
161-
currentValue: 'sheet-doc',
162-
providesContextKey: 'spreadsheetId',
163-
})
164-
const range = field({
165-
subBlockKey: 'range',
166-
currentValue: 'Sheet1!A1:D',
167-
consumesContextKeys: ['spreadsheetId'],
168-
})
169-
170-
const next = applyDependentRepick(
171-
{},
172-
spreadsheet,
173-
[spreadsheet, range],
174-
'sheet-doc',
175-
effectiveDependentValue(spreadsheet, {}, false)
176-
)
177-
178-
expect(next).toEqual({ [dependentKey(spreadsheet)]: 'sheet-doc' })
179-
expect(effectiveDependentValue(range, next, false)).toBe('Sheet1!A1:D')
180-
})
181-
182155
it('only changes the selected field when it provides no selector context', () => {
183156
const leaf = field({ subBlockKey: 'issueKey', currentValue: 'ISSUE-1' })
184157
const unrelated = field({ subBlockKey: 'label', currentValue: 'keep-me' })
@@ -231,96 +204,12 @@ describe('applyDependentRepick', () => {
231204
)
232205
).toEqual({
233206
[dependentKey(projectOne)]: 'P1-NEW',
234-
[dependentKey(issueOne)]: DEPENDENT_CLEARED_BY_PARENT,
207+
[dependentKey(issueOne)]: '',
235208
[dependentKey(issueTwo)]: 'P2-1',
236209
})
237210
})
238211
})
239212

240-
describe('submittedDependentValue', () => {
241-
const mappedParent = { copying: false, parentChanged: false }
242-
243-
it('omits an optional descendant a parent re-pick blanked, so the target keeps its value', () => {
244-
const spreadsheet = field({
245-
subBlockKey: 'spreadsheetId',
246-
currentValue: 'doc-old',
247-
providesContextKey: 'spreadsheetId',
248-
})
249-
const sheet = field({
250-
subBlockKey: 'sheetName',
251-
currentValue: 'Sheet1',
252-
required: true,
253-
consumesContextKeys: ['spreadsheetId'],
254-
})
255-
const range = field({
256-
subBlockKey: 'range',
257-
currentValue: 'A1:D50',
258-
required: false,
259-
consumesContextKeys: ['spreadsheetId'],
260-
})
261-
262-
const next = applyDependentRepick(
263-
{},
264-
spreadsheet,
265-
[spreadsheet, sheet, range],
266-
'doc-new',
267-
effectiveDependentValue(spreadsheet, {}, false)
268-
)
269-
270-
expect(isDependentClearedByParent(range, next)).toBe(true)
271-
expect(submittedDependentValue(range, next, mappedParent)).toBeUndefined()
272-
expect(submittedDependentValue(sheet, next, mappedParent)).toBeUndefined()
273-
expect(submittedDependentValue(spreadsheet, next, mappedParent)).toBe('doc-new')
274-
})
275-
276-
it('submits an empty value the user picked themselves, so an explicit clear still clears', () => {
277-
const label = field({ subBlockKey: 'label', currentValue: 'INBOX' })
278-
279-
const next = applyDependentRepick({}, label, [label], '', 'INBOX')
280-
281-
expect(isDependentClearedByParent(label, next)).toBe(false)
282-
expect(submittedDependentValue(label, next, mappedParent)).toBe('')
283-
})
284-
285-
it('submits a re-picked descendant once the user chooses a replacement', () => {
286-
const spreadsheet = field({
287-
subBlockKey: 'spreadsheetId',
288-
currentValue: 'doc-old',
289-
providesContextKey: 'spreadsheetId',
290-
})
291-
const range = field({
292-
subBlockKey: 'range',
293-
currentValue: 'A1:D50',
294-
consumesContextKeys: ['spreadsheetId'],
295-
})
296-
297-
const cleared = applyDependentRepick(
298-
{},
299-
spreadsheet,
300-
[spreadsheet, range],
301-
'doc-new',
302-
effectiveDependentValue(spreadsheet, {}, false)
303-
)
304-
const repicked = applyDependentRepick(
305-
cleared,
306-
range,
307-
[spreadsheet, range],
308-
'A1:Z',
309-
effectiveDependentValue(range, cleared, false)
310-
)
311-
312-
expect(submittedDependentValue(range, repicked, mappedParent)).toBe('A1:Z')
313-
})
314-
315-
it('falls back to the stored value under an unchanged parent and to the source when copying', () => {
316-
const untouched = field({ subBlockKey: 'label', currentValue: 'INBOX' })
317-
const copied = field({ subBlockKey: 'documentSelector', currentValue: '', sourceValue: 'doc' })
318-
319-
expect(submittedDependentValue(untouched, {}, mappedParent)).toBe('INBOX')
320-
expect(submittedDependentValue(copied, {}, { copying: true, parentChanged: false })).toBe('doc')
321-
})
322-
})
323-
324213
describe('isDependentConfigurationActionable', () => {
325214
it('hides stored values when the mapped parent is unchanged', () => {
326215
expect(
@@ -392,51 +281,6 @@ describe('isDependentConfigurationActionable', () => {
392281
).toBe(true)
393282
})
394283

395-
it('shows a required field that a parent re-pick blanked (it blocks Sync)', () => {
396-
const spreadsheet = field({
397-
subBlockKey: 'spreadsheetId',
398-
currentValue: 'doc-old',
399-
providesContextKey: 'spreadsheetId',
400-
})
401-
const sheet = field({
402-
subBlockKey: 'sheetName',
403-
required: true,
404-
currentValue: 'Sheet1',
405-
consumesContextKeys: ['spreadsheetId'],
406-
})
407-
const next = applyDependentRepick(
408-
{},
409-
spreadsheet,
410-
[spreadsheet, sheet],
411-
'doc-new',
412-
effectiveDependentValue(spreadsheet, {}, false)
413-
)
414-
415-
// The sync gate reads the same blank the selector shows, so the field gates and is visible.
416-
expect(effectiveDependentValue(sheet, next, false)).toBe('')
417-
expect(
418-
isDependentConfigurationActionable(sheet, next, {
419-
parentResolved: true,
420-
parentChanged: false,
421-
copying: false,
422-
})
423-
).toBe(true)
424-
})
425-
426-
it('shows a required field the user emptied themselves (it blocks Sync)', () => {
427-
const sheet = field({ subBlockKey: 'sheetName', required: true, currentValue: 'Sheet1' })
428-
const next = applyDependentRepick({}, sheet, [sheet], '', 'Sheet1')
429-
430-
expect(effectiveDependentValue(sheet, next, false)).toBe('')
431-
expect(
432-
isDependentConfigurationActionable(sheet, next, {
433-
parentResolved: true,
434-
parentChanged: false,
435-
copying: false,
436-
})
437-
).toBe(true)
438-
})
439-
440284
it('hides dependents until their parent is resolved', () => {
441285
expect(
442286
isDependentConfigurationActionable(

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

Lines changed: 8 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -10,46 +10,18 @@ function sameDependencyScope(left: ForkDependentReconfig, right: ForkDependentRe
1010
}
1111

1212
/**
13-
* Marker stored for a descendant whose value an in-session parent re-pick invalidated, kept
14-
* distinct from the user's own empty pick. It reads as blank everywhere it is consumed - the
15-
* selector, the in-block chain context, and the sync gate - but is never submitted: the user
16-
* has not chosen a replacement, so no override is written for it. A `''` the user picked
17-
* themselves IS submitted and does clear the target.
18-
*
19-
* What the omission buys differs by path. On Save nothing rewrites the target draft, so its
20-
* stored value survives outright - including across an undo (re-pick away, then back), where
21-
* the parent nets out unchanged so `clearDependentsOnRemap` never fires and an explicit `''`
22-
* would land on a value nobody touched. On Sync the written state is source-derived and
23-
* `clearDependentsOnRemap` already blanks every top-level dependent of a remapped parent, so
24-
* omission preserves nothing there; what it prevents is an explicit `''` reaching the fields
25-
* that pass does not cover - nested `tools[i].param` values in particular, which
26-
* `applyNestedToolOverrides` would otherwise blank.
27-
*
28-
* The escaped NUL prefix keeps it disjoint from every real selector value (ids, names, label
29-
* paths) - no selector can produce one, so it can never collide with a genuine pick.
30-
*/
31-
export const DEPENDENT_CLEARED_BY_PARENT = '\u0000fork-sync:cleared-by-parent'
32-
33-
/**
34-
* Store a dependent re-pick and invalidate every selector transitively scoped by it, marking
35-
* each with `DEPENDENT_CLEARED_BY_PARENT`: a changed provider makes every stored descendant
36-
* stale for both mapped and copied parents, but only the fields the user reviews themselves
37-
* get written to the target.
38-
*
39-
* `previousValue` is the field's effective value before the pick. `onChange` fires even when
40-
* the user re-selects the value the field already had, and that pick moves no scope, so nothing
41-
* below it went stale - the cascade is skipped. Omit it only where no prior value is known.
13+
* Store a dependent re-pick and clear every selector transitively scoped by it. Empty-string
14+
* overrides are intentional: an absent override means "fall back to the stored value", while a
15+
* changed provider makes every stored descendant stale for both mapped and copied parents.
4216
*/
4317
export function applyDependentRepick(
4418
reconfig: Record<string, string>,
4519
changedField: ForkDependentReconfig,
4620
blockFields: ForkDependentReconfig[],
47-
value: string,
48-
previousValue?: string
21+
value: string
4922
): Record<string, string> {
5023
const changedKey = dependentKey(changedField)
5124
const nextState = { ...reconfig, [changedKey]: value }
52-
if (previousValue !== undefined && previousValue === value) return nextState
5325
if (!changedField.providesContextKey) return nextState
5426

5527
const pendingContextKeys = [changedField.providesContextKey]
@@ -69,7 +41,7 @@ export function applyDependentRepick(
6941
}
7042

7143
visitedFields.add(fieldKey)
72-
nextState[fieldKey] = DEPENDENT_CLEARED_BY_PARENT
44+
nextState[fieldKey] = ''
7345
if (field.providesContextKey) pendingContextKeys.push(field.providesContextKey)
7446
}
7547
}
@@ -79,19 +51,16 @@ export function applyDependentRepick(
7951

8052
/**
8153
* The value sent + displayed for a dependent: the user's in-session re-pick if present, else the
82-
* stored value (`currentValue`). Blank when the parent target changed in-session, or when an
83-
* in-block parent re-pick invalidated it, since the old stored value was for the previous parent
84-
* and won't resolve against the new one. Shared by the sync gate and the per-block selector so
85-
* the rule can't drift between them. What gets SUBMITTED is `submittedDependentValue`, which
86-
* additionally omits the fields the user has not reviewed.
54+
* stored value (`currentValue`). Blank when the parent target changed in-session, since the old
55+
* stored value was for the previous parent and won't resolve against the new one. Shared by the
56+
* sync gate + payload build and the per-block selector so the rule can't drift between them.
8757
*/
8858
export function effectiveDependentValue(
8959
field: ForkDependentReconfig,
9060
reconfig: Record<string, string>,
9161
parentChanged: boolean
9262
): string {
9363
const repicked = reconfig[dependentKey(field)]
94-
if (repicked === DEPENDENT_CLEARED_BY_PARENT) return ''
9564
if (repicked !== undefined) return repicked
9665
return parentChanged ? '' : field.currentValue
9766
}
@@ -109,41 +78,10 @@ export function effectiveCopyDependentValue(
10978
reconfig: Record<string, string>
11079
): string {
11180
const repicked = reconfig[dependentKey(field)]
112-
if (repicked === DEPENDENT_CLEARED_BY_PARENT) return ''
11381
if (repicked !== undefined) return repicked
11482
return field.currentValue || field.sourceValue
11583
}
11684

117-
/**
118-
* Whether an in-block parent re-pick invalidated this field and the user has not re-picked it
119-
* since. Such a field shows blank but is not submitted: blanking the target on the user's behalf
120-
* would destroy a stored value they never chose to clear.
121-
*/
122-
export function isDependentClearedByParent(
123-
field: ForkDependentReconfig,
124-
reconfig: Record<string, string>
125-
): boolean {
126-
return reconfig[dependentKey(field)] === DEPENDENT_CLEARED_BY_PARENT
127-
}
128-
129-
/**
130-
* The value this dependent contributes to the submitted mapping, or `undefined` when it must be
131-
* OMITTED so the target keeps what it already stores. Only a field the user reviewed is written:
132-
* an explicit empty pick clears the target, while a value merely invalidated by a parent re-pick
133-
* is left alone (it is on screen and blank, and if it is required the sync gate blocks until the
134-
* user picks one - `effectiveDependentValue` reports it as blank).
135-
*/
136-
export function submittedDependentValue(
137-
field: ForkDependentReconfig,
138-
reconfig: Record<string, string>,
139-
state: { copying: boolean; parentChanged: boolean }
140-
): string | undefined {
141-
if (isDependentClearedByParent(field, reconfig)) return undefined
142-
return state.copying
143-
? effectiveCopyDependentValue(field, reconfig)
144-
: effectiveDependentValue(field, reconfig, state.parentChanged)
145-
}
146-
14785
export interface DependentConfigurationState {
14886
parentResolved: boolean
14987
parentChanged: boolean
@@ -154,12 +92,6 @@ export interface DependentConfigurationState {
15492
* Whether a dependent selector needs to be shown. A changed or copied parent requires review
15593
* because its children resolve in a different scope. An unchanged mapping only needs a selector
15694
* when a required value is missing; its stored values are already valid and sync-ready.
157-
*
158-
* A field a parent re-pick invalidated reads as blank through `effectiveDependentValue`, so a
159-
* REQUIRED one stays on screen here and keeps gating Sync. An optional one drops out of the
160-
* default view - `getDisplayedDependentFields` brings it back under explicit edit mode - and
161-
* hiding it loses nothing, because `submittedDependentValue` omits it from the payload rather
162-
* than blanking the target.
16395
*/
16496
export function isDependentConfigurationActionable(
16597
field: ForkDependentReconfig,

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

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -204,11 +204,10 @@ function DependentSelector({
204204
reconfig,
205205
setReconfig,
206206
}: DependentSelectorProps) {
207-
const effectiveValueIn = (f: ForkDependentReconfig, state: Record<string, string>) =>
207+
const effectiveValue = (f: ForkDependentReconfig) =>
208208
copying
209-
? effectiveCopyDependentValue(f, state)
210-
: effectiveDependentValue(f, state, parentChanged)
211-
const effectiveValue = (f: ForkDependentReconfig) => effectiveValueIn(f, reconfig)
209+
? effectiveCopyDependentValue(f, reconfig)
210+
: effectiveDependentValue(f, reconfig, parentChanged)
212211
const { providedValues, providedContextKeys } = blockChainState(block, field, effectiveValue)
213212
// Disabled until every in-block parent it depends on has a value, so a child never queries
214213
// a stale upstream value.
@@ -231,17 +230,7 @@ function DependentSelector({
231230
enabled={parentValue !== '' && ready}
232231
value={effectiveValue(field)}
233232
onChange={(value) =>
234-
setReconfig((current) =>
235-
// The pre-pick value comes from the state being updated, so re-selecting the value
236-
// already shown is recognised as the no-op it is and leaves descendants intact.
237-
applyDependentRepick(
238-
current,
239-
field,
240-
block.fields,
241-
value,
242-
effectiveValueIn(field, current)
243-
)
244-
)
233+
setReconfig((current) => applyDependentRepick(current, field, block.fields, value))
245234
}
246235
title={field.title}
247236
/>

0 commit comments

Comments
 (0)