From c14183065b98d652c52784916f0b80a67f2ac1c1 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 17 Aug 2026 15:26:20 -0700 Subject: [PATCH 1/4] fix(forking): stop a parent re-pick blanking a dependent's stored target value MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A dependent selector (a sheet under a spreadsheet, a label under a mailbox) is invalidated when its parent is re-picked, because the stored child no longer exists under the new parent. That invalidation was recorded by writing an empty string into the in-session override map — the same value the user's own "clear this field" produces. The two are not the same thing, and the map is submitted verbatim and written into the target workflow's configuration, so an invalidated field cleared the target's real stored value. The sharpest case is an undo. Re-pick a parent away from its original target, then back. The parent nets out unchanged, so nothing is remapped and the remap's own clearing pass never runs — but the child is still blank, and that blank lands on a value the user never touched, with nothing in the UI showing it happened. Record the invalidation with a distinct marker instead. It reads as blank in the selector, the in-block chain context, and the Sync gate, so a required invalidated field still blocks Sync and still renders; but it is omitted from the submitted payload rather than sent as empty, so no override is written and the target keeps what it had. A blank the user picked themselves is still submitted and still clears the target. Also: skip the cascade entirely when a re-pick selects the value the field already had, since the selector fires its change handler either way. Fork file copy: a file whose name is already taken in a reused target folder is de-duplicated with the same allocator the ordinary upload path uses, rather than colliding with the folder-name unique index and being dropped from the fork with its blob deleted. Adds hook-level coverage for the submitted payload, which had none. --- .../fork-sync/dependent-value.test.ts | 164 +++++++- .../components/fork-sync/dependent-value.ts | 84 +++- .../components/fork-sync/fork-sync-view.tsx | 19 +- .../fork-sync/use-fork-sync.test.tsx | 385 ++++++++++++++++++ .../components/fork-sync/use-fork-sync.ts | 25 +- .../lib/copy/copy-files.test.ts | 347 +++++++++++++++- .../workspace-forking/lib/copy/copy-files.ts | 54 ++- 7 files changed, 1051 insertions(+), 27 deletions(-) create mode 100644 apps/sim/ee/workspace-forking/components/fork-sync/use-fork-sync.test.tsx diff --git a/apps/sim/ee/workspace-forking/components/fork-sync/dependent-value.test.ts b/apps/sim/ee/workspace-forking/components/fork-sync/dependent-value.test.ts index 20a1c079ab9..d8aaff78b57 100644 --- a/apps/sim/ee/workspace-forking/components/fork-sync/dependent-value.test.ts +++ b/apps/sim/ee/workspace-forking/components/fork-sync/dependent-value.test.ts @@ -5,12 +5,15 @@ import { describe, expect, it } from 'vitest' import type { ForkDependentReconfig } from '@/lib/api/contracts/workspace-fork' import { applyDependentRepick, + DEPENDENT_CLEARED_BY_PARENT, dependentKey, effectiveCopyDependentValue, effectiveDependentValue, getActionableDependentFields, getDisplayedDependentFields, + isDependentClearedByParent, isDependentConfigurationActionable, + submittedDependentValue, } from '@/ee/workspace-forking/components/fork-sync/dependent-value' const field = (overrides: Partial = {}): ForkDependentReconfig => ({ @@ -143,15 +146,39 @@ describe('applyDependentRepick', () => { expect(next).toEqual({ [dependentKey(site)]: 'site-new', - [dependentKey(drive)]: '', - [dependentKey(spreadsheet)]: '', - [dependentKey(sheet)]: '', + [dependentKey(drive)]: DEPENDENT_CLEARED_BY_PARENT, + [dependentKey(spreadsheet)]: DEPENDENT_CLEARED_BY_PARENT, + [dependentKey(sheet)]: DEPENDENT_CLEARED_BY_PARENT, [dependentKey(unrelated)]: 'still-keep-me', }) expect(effectiveDependentValue(drive, next, false)).toBe('') expect(effectiveCopyDependentValue(sheet, next)).toBe('') }) + it('re-picking the value the field already had leaves its descendants alone', () => { + const spreadsheet = field({ + subBlockKey: 'spreadsheetId', + currentValue: 'sheet-doc', + providesContextKey: 'spreadsheetId', + }) + const range = field({ + subBlockKey: 'range', + currentValue: 'Sheet1!A1:D', + consumesContextKeys: ['spreadsheetId'], + }) + + const next = applyDependentRepick( + {}, + spreadsheet, + [spreadsheet, range], + 'sheet-doc', + effectiveDependentValue(spreadsheet, {}, false) + ) + + expect(next).toEqual({ [dependentKey(spreadsheet)]: 'sheet-doc' }) + expect(effectiveDependentValue(range, next, false)).toBe('Sheet1!A1:D') + }) + it('only changes the selected field when it provides no selector context', () => { const leaf = field({ subBlockKey: 'issueKey', currentValue: 'ISSUE-1' }) const unrelated = field({ subBlockKey: 'label', currentValue: 'keep-me' }) @@ -204,12 +231,96 @@ describe('applyDependentRepick', () => { ) ).toEqual({ [dependentKey(projectOne)]: 'P1-NEW', - [dependentKey(issueOne)]: '', + [dependentKey(issueOne)]: DEPENDENT_CLEARED_BY_PARENT, [dependentKey(issueTwo)]: 'P2-1', }) }) }) +describe('submittedDependentValue', () => { + const mappedParent = { copying: false, parentChanged: false } + + it('omits an optional descendant a parent re-pick blanked, so the target keeps its value', () => { + const spreadsheet = field({ + subBlockKey: 'spreadsheetId', + currentValue: 'doc-old', + providesContextKey: 'spreadsheetId', + }) + const sheet = field({ + subBlockKey: 'sheetName', + currentValue: 'Sheet1', + required: true, + consumesContextKeys: ['spreadsheetId'], + }) + const range = field({ + subBlockKey: 'range', + currentValue: 'A1:D50', + required: false, + consumesContextKeys: ['spreadsheetId'], + }) + + const next = applyDependentRepick( + {}, + spreadsheet, + [spreadsheet, sheet, range], + 'doc-new', + effectiveDependentValue(spreadsheet, {}, false) + ) + + expect(isDependentClearedByParent(range, next)).toBe(true) + expect(submittedDependentValue(range, next, mappedParent)).toBeUndefined() + expect(submittedDependentValue(sheet, next, mappedParent)).toBeUndefined() + expect(submittedDependentValue(spreadsheet, next, mappedParent)).toBe('doc-new') + }) + + it('submits an empty value the user picked themselves, so an explicit clear still clears', () => { + const label = field({ subBlockKey: 'label', currentValue: 'INBOX' }) + + const next = applyDependentRepick({}, label, [label], '', 'INBOX') + + expect(isDependentClearedByParent(label, next)).toBe(false) + expect(submittedDependentValue(label, next, mappedParent)).toBe('') + }) + + it('submits a re-picked descendant once the user chooses a replacement', () => { + const spreadsheet = field({ + subBlockKey: 'spreadsheetId', + currentValue: 'doc-old', + providesContextKey: 'spreadsheetId', + }) + const range = field({ + subBlockKey: 'range', + currentValue: 'A1:D50', + consumesContextKeys: ['spreadsheetId'], + }) + + const cleared = applyDependentRepick( + {}, + spreadsheet, + [spreadsheet, range], + 'doc-new', + effectiveDependentValue(spreadsheet, {}, false) + ) + const repicked = applyDependentRepick( + cleared, + range, + [spreadsheet, range], + 'A1:Z', + effectiveDependentValue(range, cleared, false) + ) + + expect(submittedDependentValue(range, repicked, mappedParent)).toBe('A1:Z') + }) + + it('falls back to the stored value under an unchanged parent and to the source when copying', () => { + const untouched = field({ subBlockKey: 'label', currentValue: 'INBOX' }) + const copied = field({ subBlockKey: 'documentSelector', currentValue: '', sourceValue: 'doc' }) + + expect(submittedDependentValue(untouched, {}, mappedParent)).toBe('INBOX') + expect(submittedDependentValue(copied, {}, { copying: true, parentChanged: false })).toBe('doc') + }) +}) + describe('isDependentConfigurationActionable', () => { it('hides stored values when the mapped parent is unchanged', () => { expect( @@ -281,6 +392,51 @@ describe('isDependentConfigurationActionable', () => { ).toBe(true) }) + it('shows a required field that a parent re-pick blanked (it blocks Sync)', () => { + const spreadsheet = field({ + subBlockKey: 'spreadsheetId', + currentValue: 'doc-old', + providesContextKey: 'spreadsheetId', + }) + const sheet = field({ + subBlockKey: 'sheetName', + required: true, + currentValue: 'Sheet1', + consumesContextKeys: ['spreadsheetId'], + }) + const next = applyDependentRepick( + {}, + spreadsheet, + [spreadsheet, sheet], + 'doc-new', + effectiveDependentValue(spreadsheet, {}, false) + ) + + // The sync gate reads the same blank the selector shows, so the field gates and is visible. + expect(effectiveDependentValue(sheet, next, false)).toBe('') + expect( + isDependentConfigurationActionable(sheet, next, { + parentResolved: true, + parentChanged: false, + copying: false, + }) + ).toBe(true) + }) + + it('shows a required field the user emptied themselves (it blocks Sync)', () => { + const sheet = field({ subBlockKey: 'sheetName', required: true, currentValue: 'Sheet1' }) + const next = applyDependentRepick({}, sheet, [sheet], '', 'Sheet1') + + expect(effectiveDependentValue(sheet, next, false)).toBe('') + expect( + isDependentConfigurationActionable(sheet, next, { + parentResolved: true, + parentChanged: false, + copying: false, + }) + ).toBe(true) + }) + it('hides dependents until their parent is resolved', () => { expect( isDependentConfigurationActionable( diff --git a/apps/sim/ee/workspace-forking/components/fork-sync/dependent-value.ts b/apps/sim/ee/workspace-forking/components/fork-sync/dependent-value.ts index 9ae7ab40d84..08ce8100e20 100644 --- a/apps/sim/ee/workspace-forking/components/fork-sync/dependent-value.ts +++ b/apps/sim/ee/workspace-forking/components/fork-sync/dependent-value.ts @@ -10,18 +10,46 @@ function sameDependencyScope(left: ForkDependentReconfig, right: ForkDependentRe } /** - * Store a dependent re-pick and clear every selector transitively scoped by it. Empty-string - * overrides are intentional: an absent override means "fall back to the stored value", while a - * changed provider makes every stored descendant stale for both mapped and copied parents. + * Marker stored for a descendant whose value an in-session parent re-pick invalidated, kept + * distinct from the user's own empty pick. It reads as blank everywhere it is consumed - the + * selector, the in-block chain context, and the sync gate - but is never submitted: the user + * has not chosen a replacement, so no override is written for it. A `''` the user picked + * themselves IS submitted and does clear the target. + * + * What the omission buys differs by path. On Save nothing rewrites the target draft, so its + * stored value survives outright - including across an undo (re-pick away, then back), where + * the parent nets out unchanged so `clearDependentsOnRemap` never fires and an explicit `''` + * would land on a value nobody touched. On Sync the written state is source-derived and + * `clearDependentsOnRemap` already blanks every top-level dependent of a remapped parent, so + * omission preserves nothing there; what it prevents is an explicit `''` reaching the fields + * that pass does not cover - nested `tools[i].param` values in particular, which + * `applyNestedToolOverrides` would otherwise blank. + * + * The escaped NUL prefix keeps it disjoint from every real selector value (ids, names, label + * paths) - no selector can produce one, so it can never collide with a genuine pick. + */ +export const DEPENDENT_CLEARED_BY_PARENT = '\u0000fork-sync:cleared-by-parent' + +/** + * Store a dependent re-pick and invalidate every selector transitively scoped by it, marking + * each with `DEPENDENT_CLEARED_BY_PARENT`: a changed provider makes every stored descendant + * stale for both mapped and copied parents, but only the fields the user reviews themselves + * get written to the target. + * + * `previousValue` is the field's effective value before the pick. `onChange` fires even when + * the user re-selects the value the field already had, and that pick moves no scope, so nothing + * below it went stale - the cascade is skipped. Omit it only where no prior value is known. */ export function applyDependentRepick( reconfig: Record, changedField: ForkDependentReconfig, blockFields: ForkDependentReconfig[], - value: string + value: string, + previousValue?: string ): Record { const changedKey = dependentKey(changedField) const nextState = { ...reconfig, [changedKey]: value } + if (previousValue !== undefined && previousValue === value) return nextState if (!changedField.providesContextKey) return nextState const pendingContextKeys = [changedField.providesContextKey] @@ -41,7 +69,7 @@ export function applyDependentRepick( } visitedFields.add(fieldKey) - nextState[fieldKey] = '' + nextState[fieldKey] = DEPENDENT_CLEARED_BY_PARENT if (field.providesContextKey) pendingContextKeys.push(field.providesContextKey) } } @@ -51,9 +79,11 @@ export function applyDependentRepick( /** * The value sent + displayed for a dependent: the user's in-session re-pick if present, else the - * stored value (`currentValue`). Blank when the parent target changed in-session, since the old - * stored value was for the previous parent and won't resolve against the new one. Shared by the - * sync gate + payload build and the per-block selector so the rule can't drift between them. + * stored value (`currentValue`). Blank when the parent target changed in-session, or when an + * in-block parent re-pick invalidated it, since the old stored value was for the previous parent + * and won't resolve against the new one. Shared by the sync gate and the per-block selector so + * the rule can't drift between them. What gets SUBMITTED is `submittedDependentValue`, which + * additionally omits the fields the user has not reviewed. */ export function effectiveDependentValue( field: ForkDependentReconfig, @@ -61,6 +91,7 @@ export function effectiveDependentValue( parentChanged: boolean ): string { const repicked = reconfig[dependentKey(field)] + if (repicked === DEPENDENT_CLEARED_BY_PARENT) return '' if (repicked !== undefined) return repicked return parentChanged ? '' : field.currentValue } @@ -78,10 +109,41 @@ export function effectiveCopyDependentValue( reconfig: Record ): string { const repicked = reconfig[dependentKey(field)] + if (repicked === DEPENDENT_CLEARED_BY_PARENT) return '' if (repicked !== undefined) return repicked return field.currentValue || field.sourceValue } +/** + * Whether an in-block parent re-pick invalidated this field and the user has not re-picked it + * since. Such a field shows blank but is not submitted: blanking the target on the user's behalf + * would destroy a stored value they never chose to clear. + */ +export function isDependentClearedByParent( + field: ForkDependentReconfig, + reconfig: Record +): boolean { + return reconfig[dependentKey(field)] === DEPENDENT_CLEARED_BY_PARENT +} + +/** + * The value this dependent contributes to the submitted mapping, or `undefined` when it must be + * OMITTED so the target keeps what it already stores. Only a field the user reviewed is written: + * an explicit empty pick clears the target, while a value merely invalidated by a parent re-pick + * is left alone (it is on screen and blank, and if it is required the sync gate blocks until the + * user picks one - `effectiveDependentValue` reports it as blank). + */ +export function submittedDependentValue( + field: ForkDependentReconfig, + reconfig: Record, + state: { copying: boolean; parentChanged: boolean } +): string | undefined { + if (isDependentClearedByParent(field, reconfig)) return undefined + return state.copying + ? effectiveCopyDependentValue(field, reconfig) + : effectiveDependentValue(field, reconfig, state.parentChanged) +} + export interface DependentConfigurationState { parentResolved: boolean parentChanged: boolean @@ -92,6 +154,12 @@ export interface DependentConfigurationState { * Whether a dependent selector needs to be shown. A changed or copied parent requires review * because its children resolve in a different scope. An unchanged mapping only needs a selector * when a required value is missing; its stored values are already valid and sync-ready. + * + * A field a parent re-pick invalidated reads as blank through `effectiveDependentValue`, so a + * REQUIRED one stays on screen here and keeps gating Sync. An optional one drops out of the + * default view - `getDisplayedDependentFields` brings it back under explicit edit mode - and + * hiding it loses nothing, because `submittedDependentValue` omits it from the payload rather + * than blanking the target. */ export function isDependentConfigurationActionable( field: ForkDependentReconfig, diff --git a/apps/sim/ee/workspace-forking/components/fork-sync/fork-sync-view.tsx b/apps/sim/ee/workspace-forking/components/fork-sync/fork-sync-view.tsx index a0cc46362e0..8a04c531553 100644 --- a/apps/sim/ee/workspace-forking/components/fork-sync/fork-sync-view.tsx +++ b/apps/sim/ee/workspace-forking/components/fork-sync/fork-sync-view.tsx @@ -204,10 +204,11 @@ function DependentSelector({ reconfig, setReconfig, }: DependentSelectorProps) { - const effectiveValue = (f: ForkDependentReconfig) => + const effectiveValueIn = (f: ForkDependentReconfig, state: Record) => copying - ? effectiveCopyDependentValue(f, reconfig) - : effectiveDependentValue(f, reconfig, parentChanged) + ? effectiveCopyDependentValue(f, state) + : effectiveDependentValue(f, state, parentChanged) + const effectiveValue = (f: ForkDependentReconfig) => effectiveValueIn(f, reconfig) const { providedValues, providedContextKeys } = blockChainState(block, field, effectiveValue) // Disabled until every in-block parent it depends on has a value, so a child never queries // a stale upstream value. @@ -230,7 +231,17 @@ function DependentSelector({ enabled={parentValue !== '' && ready} value={effectiveValue(field)} onChange={(value) => - setReconfig((current) => applyDependentRepick(current, field, block.fields, value)) + setReconfig((current) => + // The pre-pick value comes from the state being updated, so re-selecting the value + // already shown is recognised as the no-op it is and leaves descendants intact. + applyDependentRepick( + current, + field, + block.fields, + value, + effectiveValueIn(field, current) + ) + ) } title={field.title} /> diff --git a/apps/sim/ee/workspace-forking/components/fork-sync/use-fork-sync.test.tsx b/apps/sim/ee/workspace-forking/components/fork-sync/use-fork-sync.test.tsx new file mode 100644 index 00000000000..6bb7799a36e --- /dev/null +++ b/apps/sim/ee/workspace-forking/components/fork-sync/use-fork-sync.test.tsx @@ -0,0 +1,385 @@ +/** + * @vitest-environment jsdom + * + * Coverage for the payload {@link useForkSync} actually submits, not just the helpers it calls. + * `dependent-value.test.ts` proves `submittedDependentValue` omits a field an in-block parent + * re-pick invalidated; nothing there proves the hook USES it. Reverting the hook back to + * `dependentValueFor` re-introduces the P0 - a `{ value: '' }` written over the target's stored + * configuration - while every unit test stays green, so these tests assert on the submitted + * bodies and on the derived `dirty` / `syncDisabled` gates instead. + */ +import { act, type ReactNode } from 'react' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { + ForkDependentReconfig, + ForkMappingEntry, + UpdateForkMappingBody, +} from '@/lib/api/contracts/workspace-fork' + +const { + mockUseForkMapping, + mockUseForkDiff, + mockUpdateMutate, + mockUpdateMutateAsync, + mockPromote, +} = vi.hoisted(() => ({ + mockUseForkMapping: vi.fn(), + mockUseForkDiff: vi.fn(), + mockUpdateMutate: vi.fn(), + mockUpdateMutateAsync: vi.fn(), + mockPromote: vi.fn(), +})) + +vi.mock('@sim/emcn', () => ({ + toast: { success: vi.fn(), error: vi.fn(), warning: vi.fn() }, +})) + +vi.mock('@/ee/workspace-forking/hooks/workspace-fork', () => ({ + useForkMapping: mockUseForkMapping, + useForkDiff: mockUseForkDiff, + useUpdateForkMapping: () => ({ + mutate: mockUpdateMutate, + mutateAsync: mockUpdateMutateAsync, + isPending: false, + }), + usePromoteFork: () => ({ mutateAsync: mockPromote }), +})) + +import { + applyDependentRepick, + DEPENDENT_CLEARED_BY_PARENT, + dependentKey, +} from '@/ee/workspace-forking/components/fork-sync/dependent-value' +import { + type ForkSyncController, + useForkSync, +} from '@/ee/workspace-forking/components/fork-sync/use-fork-sync' + +const WORKSPACE_ID = 'ws-child' +const OTHER_WORKSPACE_ID = 'ws-parent' +const WORKFLOW_ID = 'wf-1' +const BLOCK_ID = 'block-1' + +/** The mapped credential every dependent below hangs off; already mapped, so nothing re-picks it. */ +const CREDENTIAL_ENTRY: ForkMappingEntry = { + kind: 'credential', + resourceType: 'credential', + sourceId: 'cred-src', + sourceLabel: 'Google (source)', + targetId: 'cred-tgt', + suggested: false, + required: true, + sourceDeleted: false, + candidates: [], + candidatesTruncated: false, +} + +function dependent(overrides: Partial): ForkDependentReconfig { + return { + parentKind: 'credential', + parentSourceId: CREDENTIAL_ENTRY.sourceId, + parentContextKey: 'oauthCredential', + targetWorkflowId: WORKFLOW_ID, + targetBlockId: BLOCK_ID, + blockName: 'Google Sheets', + subBlockKey: 'field', + selectorKey: 'sheet', + title: 'Field', + currentValue: '', + sourceValue: '', + required: false, + consumesContextKeys: [], + context: {}, + ...overrides, + } +} + +/** The in-block provider the user re-picks; its change invalidates `CHILD`. */ +const PARENT_FIELD = dependent({ + subBlockKey: 'spreadsheetId', + title: 'Spreadsheet', + currentValue: 'sheet-1', + providesContextKey: 'spreadsheetId', +}) + +/** Optional descendant of `PARENT_FIELD` - the field the P0 used to blank in the target. */ +const CHILD_FIELD = dependent({ + subBlockKey: 'sheetName', + title: 'Sheet', + currentValue: 'Tab A', + consumesContextKeys: ['spreadsheetId'], +}) + +/** Unrelated field the user clears themselves; an intentional clear must still be submitted. */ +const CLEARED_FIELD = dependent({ + subBlockKey: 'folderId', + title: 'Folder', + currentValue: 'folder-9', +}) + +/** Untouched field, submitted with its stored value so the "full stored mapping" contract holds. */ +const UNTOUCHED_FIELD = dependent({ + subBlockKey: 'labelId', + title: 'Label', + currentValue: 'label-3', +}) + +const DEPENDENTS = [PARENT_FIELD, CHILD_FIELD, CLEARED_FIELD, UNTOUCHED_FIELD] + +function diffData(dependentReconfigs: ForkDependentReconfig[]) { + return { + sourceWorkspaceId: WORKSPACE_ID, + targetWorkspaceId: OTHER_WORKSPACE_ID, + workflows: [], + dependentReconfigs, + resourceUsages: [], + copyableUnmapped: [], + clearedRefs: [], + triggerMappings: [], + retiringTriggerUrls: [], + excludedSourceWorkflows: [], + excludedTargetWorkflows: [], + mcpReauthServerIds: [], + inlineSecretSources: [], + } +} + +const mountedRoots: Root[] = [] + +function renderForkSync(): { get: () => ForkSyncController } { + ;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true + const container = document.createElement('div') + const root = createRoot(container) + mountedRoots.push(root) + let result: ForkSyncController | undefined + + function Probe() { + result = useForkSync({ + workspaceId: WORKSPACE_ID, + otherWorkspaceId: OTHER_WORKSPACE_ID, + otherWorkspaceName: 'Parent', + direction: 'push', + enabled: true, + }) + return null + } + + act(() => { + root.render(() as ReactNode) + }) + + return { + get: () => { + if (!result) throw new Error('hook result is not ready') + return result + }, + } +} + +/** The `dependentValues` the last Save sent, or `undefined` when it sent none. */ +function savedDependentValues(): UpdateForkMappingBody['dependentValues'] { + expect(mockUpdateMutate).toHaveBeenCalledTimes(1) + const [variables] = mockUpdateMutate.mock.calls[0] as [{ body: UpdateForkMappingBody }] + return variables.body.dependentValues +} + +/** The `dependentValues` the last Sync promoted. */ +function promotedDependentValues(): UpdateForkMappingBody['dependentValues'] { + expect(mockPromote).toHaveBeenCalledTimes(1) + const [variables] = mockPromote.mock.calls[0] as [ + { body: { dependentValues?: UpdateForkMappingBody['dependentValues'] } }, + ] + return variables.body.dependentValues +} + +function valueFor( + submitted: UpdateForkMappingBody['dependentValues'], + field: ForkDependentReconfig +): string | undefined { + return submitted?.find((entry) => entry.subBlockKey === field.subBlockKey)?.value +} + +beforeEach(() => { + vi.clearAllMocks() + mockUseForkMapping.mockReturnValue({ + data: { entries: [CREDENTIAL_ENTRY] }, + isLoading: false, + isError: false, + error: null, + isPlaceholderData: false, + }) + mockUseForkDiff.mockReturnValue({ + data: diffData(DEPENDENTS), + isError: false, + error: null, + isPlaceholderData: false, + }) + mockUpdateMutateAsync.mockResolvedValue({ success: true, updated: 1 }) + mockPromote.mockResolvedValue({ + promoteRunId: 'run-1', + blockers: [], + unmappedRequired: [], + droppedReferences: [], + triggerUrlChanges: [], + deployFailed: 0, + }) +}) + +afterEach(() => { + act(() => { + for (const root of mountedRoots.splice(0)) root.unmount() + }) +}) + +describe('useForkSync dependent payload', () => { + it('omits a dependent an in-block parent re-pick invalidated, so the target keeps its stored value', () => { + const { get } = renderForkSync() + + act(() => { + get().setReconfig((prev) => + applyDependentRepick(prev, PARENT_FIELD, DEPENDENTS, 'sheet-2', 'sheet-1') + ) + }) + + expect(get().reconfig[dependentKey(CHILD_FIELD)]).toBe(DEPENDENT_CLEARED_BY_PARENT) + + act(() => get().save()) + + const submitted = savedDependentValues() + expect(submitted?.map((entry) => entry.subBlockKey)).toEqual([ + PARENT_FIELD.subBlockKey, + CLEARED_FIELD.subBlockKey, + UNTOUCHED_FIELD.subBlockKey, + ]) + expect(valueFor(submitted, PARENT_FIELD)).toBe('sheet-2') + expect(valueFor(submitted, UNTOUCHED_FIELD)).toBe('label-3') + }) + + it('submits a field the user cleared themselves, so an intentional clear still clears the target', () => { + const { get } = renderForkSync() + + act(() => { + get().setReconfig((prev) => ({ ...prev, [dependentKey(CLEARED_FIELD)]: '' })) + }) + act(() => get().save()) + + const submitted = savedDependentValues() + expect(submitted?.map((entry) => entry.subBlockKey)).toContain(CLEARED_FIELD.subBlockKey) + expect(valueFor(submitted, CLEARED_FIELD)).toBe('') + }) + + it('leaves the editor clean when only markers remain, so an undone re-pick is not savable', () => { + const { get } = renderForkSync() + + act(() => { + get().setReconfig((prev) => + applyDependentRepick(prev, PARENT_FIELD, DEPENDENTS, 'sheet-2', 'sheet-1') + ) + }) + expect(get().dirty).toBe(true) + + // The user puts the provider back to its stored value. Their own re-pick is no longer a + // change, and the marker it left on the child contributes nothing to the payload - so + // nothing is savable, even though `reconfig` is not empty. + act(() => { + get().setReconfig((prev) => + applyDependentRepick(prev, PARENT_FIELD, DEPENDENTS, 'sheet-1', 'sheet-2') + ) + }) + + expect(get().reconfig[dependentKey(CHILD_FIELD)]).toBe(DEPENDENT_CLEARED_BY_PARENT) + expect(get().dirty).toBe(false) + + act(() => get().save()) + expect(mockUpdateMutate).not.toHaveBeenCalled() + }) + + it('keeps Sync blocked while a REQUIRED dependent is marked by a parent re-pick', () => { + const requiredChild = { ...CHILD_FIELD, required: true } + mockUseForkDiff.mockReturnValue({ + data: diffData([PARENT_FIELD, requiredChild]), + isError: false, + error: null, + isPlaceholderData: false, + }) + const { get } = renderForkSync() + + expect(get().syncDisabled).toBe(false) + + act(() => { + get().setReconfig((prev) => + applyDependentRepick( + prev, + PARENT_FIELD, + [PARENT_FIELD, requiredChild], + 'sheet-2', + 'sheet-1' + ) + ) + }) + + expect(get().syncDisabled).toBe(true) + expect(get().syncDisabledReason).toBe('Reconfigure all required fields first') + }) + + it('omits the marked dependent from the promote payload too', async () => { + const { get } = renderForkSync() + + act(() => { + get().setReconfig((prev) => + applyDependentRepick(prev, PARENT_FIELD, DEPENDENTS, 'sheet-2', 'sheet-1') + ) + }) + await act(async () => { + await get().sync() + }) + + expect(promotedDependentValues()?.map((entry) => entry.subBlockKey)).not.toContain( + CHILD_FIELD.subBlockKey + ) + }) +}) + +describe('useForkSync post-sync reset', () => { + it('drops the in-session re-picks once the sync commits them', async () => { + const { get } = renderForkSync() + + act(() => { + get().setReconfig((prev) => + applyDependentRepick(prev, PARENT_FIELD, DEPENDENTS, 'sheet-2', 'sheet-1') + ) + }) + expect(get().dirty).toBe(true) + + await act(async () => { + await get().sync() + }) + + expect(get().reconfig).toEqual({}) + expect(get().dirty).toBe(false) + }) + + it('keeps the in-session re-picks when the sync is refused by the server gate', async () => { + mockPromote.mockResolvedValue({ + promoteRunId: null, + blockers: [{ kind: 'credential', sourceId: 'cred-src' }], + unmappedRequired: [], + droppedReferences: [], + triggerUrlChanges: [], + deployFailed: 0, + }) + const { get } = renderForkSync() + + act(() => { + get().setReconfig((prev) => + applyDependentRepick(prev, PARENT_FIELD, DEPENDENTS, 'sheet-2', 'sheet-1') + ) + }) + await act(async () => { + await get().sync() + }) + + expect(get().reconfig[dependentKey(PARENT_FIELD)]).toBe('sheet-2') + }) +}) diff --git a/apps/sim/ee/workspace-forking/components/fork-sync/use-fork-sync.ts b/apps/sim/ee/workspace-forking/components/fork-sync/use-fork-sync.ts index c28d29c371b..f2bcc58b301 100644 --- a/apps/sim/ee/workspace-forking/components/fork-sync/use-fork-sync.ts +++ b/apps/sim/ee/workspace-forking/components/fork-sync/use-fork-sync.ts @@ -34,6 +34,8 @@ import { dependentKey, effectiveCopyDependentValue, effectiveDependentValue, + isDependentClearedByParent, + submittedDependentValue, } from '@/ee/workspace-forking/components/fork-sync/dependent-value' import { forkDyingTriggerUrls, @@ -685,7 +687,11 @@ export function useForkSync(params: { // effective value: re-pick, stored, or blank-after-change) or copy-selected (re-pick, stored, // or the source reference; promote translates a source document id to its copied counterpart // at write time). The server persists this verbatim as the stored mapping; fields whose - // parent is unresolved are omitted (they can't be configured). This is the whole "what's in + // parent is unresolved are omitted (they can't be configured), as are fields an in-block + // parent re-pick invalidated and the user never re-picked - submitting those blank writes an + // explicit `''` override into the target draft (see `applyDependentOverrides`), which on the + // paths `clearDependentsOnRemap` does not cover would clear a value nobody chose to clear. + // `DEPENDENT_CLEARED_BY_PARENT` documents which paths those are. This is the whole "what's in // the mapping goes in" contract, shared by Save and Sync so the two persist identically. const buildDependentValues = () => dependentReconfigs.flatMap((field) => { @@ -693,12 +699,17 @@ export function useForkSync(params: { if (!parent) return [] const resolution = resolutionFor(parent) if (resolution === 'unresolved') return [] + const value = submittedDependentValue(field, reconfig, { + copying: resolution === 'copied', + parentChanged: shouldReconfigureEntry(parent, targets), + }) + if (value === undefined) return [] return [ { workflowId: field.targetWorkflowId, blockId: field.targetBlockId, subBlockKey: field.subBlockKey, - value: dependentValueFor(field, parent, resolution), + value, }, ] }) @@ -723,9 +734,12 @@ export function useForkSync(params: { // A dependent re-pick that differs from its stored value also dirties the editor. A re-pick // under a changed parent is covered by `targetsDirty` (the parent override is the change). + // A field merely invalidated by a parent re-pick is not a change of its own - it contributes + // nothing to the payload, so it must not make an otherwise untouched editor savable. const reconfigDirty = useMemo( () => dependentReconfigs.some((field) => { + if (isDependentClearedByParent(field, reconfig)) return false const repicked = reconfig[dependentKey(field)] return repicked !== undefined && repicked !== field.currentValue }), @@ -922,6 +936,13 @@ export function useForkSync(params: { return } + // The run committed the in-session choices: the mapping entries and dependent values are + // stored, so keeping the overrides would leave every touched field's card on screen against + // a freshly refetched diff - and a `DEPENDENT_CLEARED_BY_PARENT` marker would keep rendering + // blank over the value the sync just stored. Drop them exactly as a successful Save does. + setTargets({}) + setReconfig({}) + const target = otherWorkspaceName || 'the workspace' const label = direction === 'pull' ? `Pulled from "${target}"` : `Pushed to "${target}"` // A sync only commits once every reference is mapped/copied and every required dependent diff --git a/apps/sim/ee/workspace-forking/lib/copy/copy-files.test.ts b/apps/sim/ee/workspace-forking/lib/copy/copy-files.test.ts index 622c01cf582..9f833f565e1 100644 --- a/apps/sim/ee/workspace-forking/lib/copy/copy-files.test.ts +++ b/apps/sim/ee/workspace-forking/lib/copy/copy-files.test.ts @@ -4,21 +4,75 @@ import { folder as folderTable } from '@sim/db/schema' import { dbChainMockFns, + type MockCondition, resetDbChainMock, storageServiceMock, storageServiceMockFns, } from '@sim/testing' import { beforeEach, describe, expect, it, vi } from 'vitest' +/** The `workspace_files` columns {@link fileRows} enforces its unique indexes on. */ +interface WorkspaceFileRow { + id: string + key: string + workspaceId: string | null + folderId?: string | null + context: string + originalName: string + deletedAt: Date | null + [column: string]: unknown +} + +/** + * `fileRows` is the shared stand-in for the `workspace_files` table: the mocked name + * allocator reads it exactly as the real one queries the DB, and the insert simulation in + * `executeForkFileBlobCopies collisions` enforces the same unique indexes against it. One + * store, so the allocator and the index can never disagree the way two fixtures would. + */ const { + fileRows, + mockAllocateUniqueWorkspaceFileName, mockCopyWorkspaceFileSecretProvenanceInTx, mockIncrementStorageUsageInTx, mockResolveStorageBillingContext, -} = vi.hoisted(() => ({ - mockCopyWorkspaceFileSecretProvenanceInTx: vi.fn(), - mockIncrementStorageUsageInTx: vi.fn(), - mockResolveStorageBillingContext: vi.fn(), -})) +} = vi.hoisted(() => { + const fileRows: WorkspaceFileRow[] = [] + const withCopySuffix = (name: string, n: number) => { + const lastDot = name.lastIndexOf('.') + return lastDot > 0 && lastDot < name.length - 1 + ? `${name.slice(0, lastDot)} (${n})${name.slice(lastDot)}` + : `${name} (${n})` + } + return { + fileRows, + /** + * Mirrors `allocateUniqueWorkspaceFileName`: the taken-name probe matches the columns + * of `workspace_files_workspace_folder_name_active_unique`. + */ + mockAllocateUniqueWorkspaceFileName: vi.fn( + async (workspaceId: string, baseName: string, folderId?: string | null) => { + const taken = (name: string) => + fileRows.some( + (row) => + row.deletedAt === null && + row.context === 'workspace' && + row.workspaceId === workspaceId && + (row.folderId ?? null) === (folderId ?? null) && + row.originalName === name + ) + if (!taken(baseName)) return baseName + for (let n = 1; n <= 1000; n++) { + const candidate = withCopySuffix(baseName, n) + if (!taken(candidate)) return candidate + } + throw new Error(`A file named "${baseName}" already exists in this workspace`) + } + ), + mockCopyWorkspaceFileSecretProvenanceInTx: vi.fn(), + mockIncrementStorageUsageInTx: vi.fn(), + mockResolveStorageBillingContext: vi.fn(), + } +}) vi.mock('@/lib/uploads/core/storage-service', () => storageServiceMock) vi.mock('@/lib/billing/storage', () => ({ @@ -26,6 +80,7 @@ vi.mock('@/lib/billing/storage', () => ({ resolveStorageBillingContext: mockResolveStorageBillingContext, })) vi.mock('@/lib/uploads/contexts/workspace/workspace-file-manager', () => ({ + allocateUniqueWorkspaceFileName: mockAllocateUniqueWorkspaceFileName, generateWorkspaceFileKey: vi.fn( (workspaceId: string, fileName: string) => `workspace/${workspaceId}/generated-${fileName}` ), @@ -63,6 +118,7 @@ describe('executeForkFileBlobCopies storage accounting', () => { beforeEach(() => { vi.clearAllMocks() resetDbChainMock() + fileRows.length = 0 storageServiceMockFns.mockHeadObject.mockResolvedValue(null) storageServiceMockFns.mockDownloadFile.mockResolvedValue(Buffer.from('blob-bytes')) storageServiceMockFns.mockUploadFile.mockResolvedValue({ key: 'workspace/child-ws/target' }) @@ -194,6 +250,287 @@ describe('executeForkFileBlobCopies storage accounting', () => { }) }) +/** Rejects a predicate the harness does not model, rather than letting it match everything. */ +function unsupportedPredicate(detail: string): never { + throw new Error(`Unsupported predicate in test harness: ${detail}`) +} + +/** The nested clauses of an `and`/`or` node, or a throw when the node carries none. */ +function predicateClauses(node: MockCondition): unknown[] { + if (!Array.isArray(node.conditions)) + unsupportedPredicate(`${String(node.type)} without conditions`) + return node.conditions +} + +/** + * The row key a predicate node references. The mocked schema tables are column-name maps, so a + * column reference is the column name itself; anything else is a shape this harness cannot read. + */ +function predicateColumn(node: MockCondition, field: 'left' | 'column'): string { + const column = node[field] + if (typeof column !== 'string') + unsupportedPredicate(`${String(node.type)} with a non-column ${field}`) + return column +} + +/** + * Evaluate a mocked drizzle predicate against a row. Real predicate reading, so a chain that + * ignores its `where` clause cannot pass these tests by echoing a fixture back. + */ +function matchesPredicate(row: Record, predicate: unknown): boolean { + if (!predicate) return true + if (typeof predicate !== 'object') unsupportedPredicate(typeof predicate) + const node = predicate as MockCondition + switch (node.type) { + case 'and': + return predicateClauses(node).every((clause) => matchesPredicate(row, clause)) + case 'or': + return predicateClauses(node).some((clause) => matchesPredicate(row, clause)) + case 'eq': + return row[predicateColumn(node, 'left')] === node.right + case 'isNull': { + const value = row[predicateColumn(node, 'column')] + return value === null || value === undefined + } + case 'inArray': { + if (!Array.isArray(node.values)) unsupportedPredicate('inArray without values') + return node.values.includes(row[predicateColumn(node, 'column')]) + } + default: + return unsupportedPredicate(String(node.type)) + } +} + +/** Awaitable stand-in for a drizzle select result, supporting `.limit`/`.for`/`.orderBy`. */ +interface MockSelectResult extends PromiseLike { + catch: Promise['catch'] + finally: Promise['finally'] + limit: (count: number) => MockSelectResult + for: () => MockSelectResult + orderBy: () => MockSelectResult +} + +function selectResult(rows: WorkspaceFileRow[]): MockSelectResult { + const settled = Promise.resolve(rows) + const builder: MockSelectResult = { + then: (onFulfilled, onRejected) => settled.then(onFulfilled, onRejected), + catch: (onRejected) => settled.catch(onRejected), + finally: (onFinally) => settled.finally(onFinally), + limit: (count: number) => selectResult(rows.slice(0, count)), + for: () => builder, + orderBy: () => builder, + } + return builder +} + +/** + * Postgres-faithful `workspace_files` writes against {@link fileRows}: `key` is guarded by + * `workspace_files_key_active_unique` and `(workspace_id, coalesce(folder_id, ''), + * original_name)` by `workspace_files_workspace_folder_name_active_unique`. A bare + * `onConflictDoNothing()` absorbs BOTH plus the primary key (the shipped bug); one targeted at + * the primary key absorbs only a replay of the same row and lets a real name clash raise. + */ +function installFileTableSimulation(): void { + dbChainMockFns.where.mockImplementation((predicate: unknown) => + selectResult(fileRows.filter((row) => matchesPredicate(row, predicate))) + ) + dbChainMockFns.values.mockImplementation((row: WorkspaceFileRow) => { + const attemptInsert = (conflictTarget: unknown) => { + const pkConflict = fileRows.some((existing) => existing.id === row.id) + const activeConflict = fileRows.some( + (existing) => + existing.deletedAt === null && + (existing.key === row.key || + (existing.context === 'workspace' && + existing.workspaceId === row.workspaceId && + (existing.folderId ?? null) === (row.folderId ?? null) && + existing.originalName === row.originalName)) + ) + if (conflictTarget === undefined ? pkConflict || activeConflict : pkConflict) { + return Promise.resolve([]) + } + if (activeConflict) { + return Promise.reject( + Object.assign( + new Error( + 'duplicate key value violates unique constraint ' + + '"workspace_files_workspace_folder_name_active_unique"' + ), + { code: '23505' } + ) + ) + } + fileRows.push({ ...row }) + return Promise.resolve([{ id: row.id }]) + } + return { + onConflictDoNothing: (config?: { target?: unknown }) => { + dbChainMockFns.onConflictDoNothing(config) + return { returning: () => attemptInsert(config?.target) } + }, + onConflictDoUpdate: () => ({ returning: () => attemptInsert(undefined) }), + returning: () => attemptInsert('no-conflict-clause'), + } + }) +} + +describe('executeForkFileBlobCopies target name collisions', () => { + const collidingTask = () => + makeTask({ + fileName: 'budget.xlsx', + sourceKey: 'workspace/src-ws/source-budget.xlsx', + targetKey: 'workspace/child-ws/target-budget.xlsx', + contentType: 'application/vnd.ms-excel', + targetFolderId: 'target-reports', + }) + + beforeEach(() => { + vi.clearAllMocks() + resetDbChainMock() + fileRows.length = 0 + storageServiceMockFns.mockHeadObject.mockResolvedValue(null) + storageServiceMockFns.mockDownloadFile.mockResolvedValue(Buffer.from('blob-bytes')) + storageServiceMockFns.mockUploadFile.mockResolvedValue({ + key: 'workspace/child-ws/target-budget.xlsx', + }) + mockResolveStorageBillingContext.mockResolvedValue({ + workspaceId: 'child-ws', + billedAccountUserId: 'target-payer', + billingEntity: { type: 'user', id: 'target-payer' }, + plan: 'pro', + customStorageLimitGB: null, + }) + mockIncrementStorageUsageInTx.mockResolvedValue(100) + installFileTableSimulation() + }) + + it('keeps a file whose name is already taken in the reused target folder', async () => { + // The target already holds `Reports/budget.xlsx`; the fork mirrors `Reports` onto it. + fileRows.push({ + id: 'pre-existing', + key: 'workspace/child-ws/pre-existing-budget.xlsx', + workspaceId: 'child-ws', + folderId: 'target-reports', + context: 'workspace', + originalName: 'budget.xlsx', + deletedAt: null, + }) + + const result = await executeForkFileBlobCopies([collidingTask()], 'test') + + expect(result).toEqual({ copied: 1, failed: 0, failedTargetKeys: [] }) + // Non-destructive: the copy lands beside the target's own file, in the mirrored folder. + expect(fileRows).toHaveLength(2) + expect(fileRows.find((row) => row.id === 'pre-existing')?.originalName).toBe('budget.xlsx') + expect(fileRows.find((row) => row.id === 'target-file-1')).toMatchObject({ + key: 'workspace/child-ws/target-budget.xlsx', + workspaceId: 'child-ws', + folderId: 'target-reports', + originalName: 'budget (1).xlsx', + deletedAt: null, + }) + // The blob backing the surviving row must never be swept. + expect(storageServiceMockFns.mockDeleteFile).not.toHaveBeenCalled() + // The de-duplication probe is scoped to the index's exact tuple, folder included. + expect(mockAllocateUniqueWorkspaceFileName).toHaveBeenCalledWith( + 'child-ws', + 'budget.xlsx', + 'target-reports' + ) + expect(mockIncrementStorageUsageInTx).toHaveBeenCalledTimes(1) + }) + + it('absorbs only a primary-key conflict, so a name conflict can never be mistaken for a replay', async () => { + await executeForkFileBlobCopies([collidingTask()], 'test') + + expect(dbChainMockFns.onConflictDoNothing).toHaveBeenCalledWith({ target: 'id' }) + }) + + it('copies a non-colliding file into the mirrored folder unchanged', async () => { + fileRows.push({ + id: 'pre-existing', + key: 'workspace/child-ws/pre-existing-forecast.xlsx', + workspaceId: 'child-ws', + folderId: 'target-reports', + context: 'workspace', + originalName: 'forecast.xlsx', + deletedAt: null, + }) + + const result = await executeForkFileBlobCopies([collidingTask()], 'test') + + expect(result).toEqual({ copied: 1, failed: 0, failedTargetKeys: [] }) + expect(fileRows.find((row) => row.id === 'target-file-1')).toMatchObject({ + folderId: 'target-reports', + originalName: 'budget.xlsx', + }) + expect(storageServiceMockFns.mockDeleteFile).not.toHaveBeenCalled() + }) + + it('de-duplicates each same-named copy against the rows earlier tasks already landed', async () => { + // Two source files share a name inside the folder the target already has a `budget.xlsx` in. + // Each allocation must see the row the previous task committed, so the suffixes advance + // instead of every task racing for the same `budget (1).xlsx`. + fileRows.push({ + id: 'pre-existing', + key: 'workspace/child-ws/pre-existing-budget.xlsx', + workspaceId: 'child-ws', + folderId: 'target-reports', + context: 'workspace', + originalName: 'budget.xlsx', + deletedAt: null, + }) + + const result = await executeForkFileBlobCopies( + [ + collidingTask(), + makeTask({ + fileName: 'budget.xlsx', + sourceKey: 'workspace/src-ws/source-budget-2.xlsx', + targetKey: 'workspace/child-ws/target-budget-2.xlsx', + contentType: 'application/vnd.ms-excel', + targetFolderId: 'target-reports', + targetFileId: 'target-file-2', + }), + ], + 'test' + ) + + expect(result).toEqual({ copied: 2, failed: 0, failedTargetKeys: [] }) + expect(fileRows.map((row) => row.originalName)).toEqual([ + 'budget.xlsx', + 'budget (1).xlsx', + 'budget (2).xlsx', + ]) + expect(storageServiceMockFns.mockDeleteFile).not.toHaveBeenCalled() + }) + + it('replays to the same end state without duplicating the copy or failing differently', async () => { + fileRows.push({ + id: 'pre-existing', + key: 'workspace/child-ws/pre-existing-budget.xlsx', + workspaceId: 'child-ws', + folderId: 'target-reports', + context: 'workspace', + originalName: 'budget.xlsx', + deletedAt: null, + }) + + const first = await executeForkFileBlobCopies([collidingTask()], 'test') + const afterFirst = structuredClone(fileRows) + const replay = await executeForkFileBlobCopies([collidingTask()], 'test') + + expect(first).toEqual({ copied: 1, failed: 0, failedTargetKeys: [] }) + // The replay resolves the existing copy instead of re-copying: still one success, no failure. + expect(replay).toEqual({ copied: 1, failed: 0, failedTargetKeys: [] }) + expect(fileRows).toEqual(afterFirst) + expect(storageServiceMockFns.mockUploadFile).toHaveBeenCalledTimes(1) + expect(dbChainMockFns.transaction).toHaveBeenCalledTimes(1) + expect(mockIncrementStorageUsageInTx).toHaveBeenCalledTimes(1) + expect(storageServiceMockFns.mockDeleteFile).not.toHaveBeenCalled() + }) +}) + describe('planForkFileCopies', () => { it('plans deterministic target metadata without inserting an active row before blob copy', async () => { const sourceMeta = { diff --git a/apps/sim/ee/workspace-forking/lib/copy/copy-files.ts b/apps/sim/ee/workspace-forking/lib/copy/copy-files.ts index cfa4888170d..d84745adc05 100644 --- a/apps/sim/ee/workspace-forking/lib/copy/copy-files.ts +++ b/apps/sim/ee/workspace-forking/lib/copy/copy-files.ts @@ -9,7 +9,10 @@ import { resolveStorageBillingContext, } from '@/lib/billing/storage' import type { DbOrTx } from '@/lib/db/types' -import { generateWorkspaceFileKey } from '@/lib/uploads/contexts/workspace/workspace-file-manager' +import { + allocateUniqueWorkspaceFileName, + generateWorkspaceFileKey, +} from '@/lib/uploads/contexts/workspace/workspace-file-manager' import { copyWorkspaceFileSecretProvenanceInTx } from '@/lib/uploads/contexts/workspace/workspace-file-secret-provenance' import { deleteFile, @@ -112,6 +115,31 @@ async function getFinalizedFileCopies( return new Map(active.map((row) => [row.id, { key: row.key, workspaceId: row.workspaceId }])) } +/** + * Pick the child row's `original_name`, de-duplicating against the partial unique index + * `workspace_files_workspace_folder_name_active_unique` on + * `(workspace_id, coalesce(folder_id, ''), original_name)`. + * + * Since the fork copy started preserving folder structure, a target folder reused by + * {@link resolveForkFolderMapping} can already hold a file of the same name (parent has + * `Reports/budget.xlsx`, the child pushes its own `Reports/budget.xlsx`). Reusing the source + * name would violate that index, so the copy lands as `budget (1).xlsx` instead: both files + * survive, the copy stays visible in the fork under the mirrored folder, and its blob is kept. + * + * Advisory only. The lookup runs outside the finalize transaction (and so cannot see it), so a + * concurrent upload can still claim the name in between. The index stays the authority: such a + * task fails loudly into `failedTargetKeys` instead of being silently dropped. + */ +async function resolveTargetOriginalName(task: BlobCopyTask): Promise { + // The index is partial on durable workspace files; no other context can collide on it. + if (task.context !== 'workspace') return task.fileName + return allocateUniqueWorkspaceFileName( + task.workspaceId, + task.fileName, + task.targetFolderId ?? null + ) +} + /** * Plan child metadata identities and blob copies without inserting an active * `workspace_files` row. The child workspace and source mappings are committed @@ -216,6 +244,10 @@ export async function planForkFileCopies(params: { * `file-upload` references pointing at the now-missing object. A failed task has no * active target metadata; an object uploaded before a failed finalization is deleted * best-effort outside the transaction. + * + * A copy whose name is already taken inside its mirrored target folder is de-duplicated + * (`budget (1).xlsx`) rather than dropped - see {@link resolveTargetOriginalName}. Those + * still count as `copied`. */ export async function executeForkFileBlobCopies( blobTasks: BlobCopyTask[], @@ -292,6 +324,7 @@ export async function executeForkFileBlobCopies( } const billingContext = await resolveStorageBillingContext(task.workspaceId) + const targetOriginalName = await resolveTargetOriginalName(task) await db.transaction(async (tx) => { const [inserted] = await tx .insert(workspaceFiles) @@ -303,16 +336,21 @@ export async function executeForkFileBlobCopies( folderId: task.targetFolderId ?? null, context: task.context, chatId: null, - originalName: task.fileName, + originalName: targetOriginalName, displayName: task.displayName, contentType: task.contentType, size: task.size, deletedAt: null, uploadedAt: new Date(), }) - .onConflictDoNothing() + // Targeted at the primary key so ONLY a replay of this same task is absorbed. A + // bare `onConflictDoNothing()` also swallows the `(workspace_id, folder_id, + // original_name)` unique index, whose conflicting row has a DIFFERENT id - the + // recovery below then finds nothing and drops a file that merely shares a name. + .onConflictDoNothing({ target: workspaceFiles.id }) .returning({ id: workspaceFiles.id }) + // Reachable only on a primary-key conflict, so the row is addressable by id. if (!inserted) { const [current] = await tx .select({ @@ -346,7 +384,7 @@ export async function executeForkFileBlobCopies( folderId: task.targetFolderId ?? null, context: task.context, chatId: null, - originalName: task.fileName, + originalName: targetOriginalName, displayName: task.displayName, contentType: task.contentType, size: task.size, @@ -371,6 +409,14 @@ export async function executeForkFileBlobCopies( await incrementStorageUsageForBillingContextInTx(tx, billingContext, task.size) }) copied += 1 + if (targetOriginalName !== task.fileName) { + logger.warn(`[${requestId}] Copied file renamed to avoid a target name collision`, { + targetKey: task.targetKey, + folderId: task.targetFolderId ?? null, + from: task.fileName, + to: targetOriginalName, + }) + } } catch (error) { failedTargetKeys.push(task.targetKey) logger.warn(`[${requestId}] Failed to copy file blob during fork`, { From b4bc9bfdab9e4922d5746cbc2daa2ada77229b7c Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Mon, 17 Aug 2026 15:48:46 -0700 Subject: [PATCH 2/4] fix(forking): preserve dependent-chain semantics --- .../fork-sync/dependent-value.test.ts | 137 +++++++------ .../components/fork-sync/dependent-value.ts | 181 ++++++++++-------- .../components/fork-sync/fork-sync-view.tsx | 25 ++- .../fork-sync/use-fork-sync.test.tsx | 137 ++++++++++--- .../components/fork-sync/use-fork-sync.ts | 37 ++-- .../lib/remap/remap-references.test.ts | 50 +++++ 6 files changed, 370 insertions(+), 197 deletions(-) diff --git a/apps/sim/ee/workspace-forking/components/fork-sync/dependent-value.test.ts b/apps/sim/ee/workspace-forking/components/fork-sync/dependent-value.test.ts index d8aaff78b57..077175f1bd6 100644 --- a/apps/sim/ee/workspace-forking/components/fork-sync/dependent-value.test.ts +++ b/apps/sim/ee/workspace-forking/components/fork-sync/dependent-value.test.ts @@ -5,15 +5,13 @@ import { describe, expect, it } from 'vitest' import type { ForkDependentReconfig } from '@/lib/api/contracts/workspace-fork' import { applyDependentRepick, - DEPENDENT_CLEARED_BY_PARENT, dependentKey, effectiveCopyDependentValue, effectiveDependentValue, getActionableDependentFields, getDisplayedDependentFields, - isDependentClearedByParent, isDependentConfigurationActionable, - submittedDependentValue, + isDependentInvalidated, } from '@/ee/workspace-forking/components/fork-sync/dependent-value' const field = (overrides: Partial = {}): ForkDependentReconfig => ({ @@ -34,6 +32,11 @@ const field = (overrides: Partial = {}): ForkDependentRec ...overrides, }) +const mappedRepickContext = (previousValue: string) => ({ + previousValue, + baselineValueFor: (dependent: ForkDependentReconfig) => dependent.currentValue, +}) + describe('dependentKey', () => { it('keys by target workflow + block + subblock', () => { expect( @@ -141,14 +144,15 @@ describe('applyDependentRepick', () => { previous, site, [site, drive, spreadsheet, sheet, unrelated], - 'site-new' + 'site-new', + mappedRepickContext('site-old') ) expect(next).toEqual({ [dependentKey(site)]: 'site-new', - [dependentKey(drive)]: DEPENDENT_CLEARED_BY_PARENT, - [dependentKey(spreadsheet)]: DEPENDENT_CLEARED_BY_PARENT, - [dependentKey(sheet)]: DEPENDENT_CLEARED_BY_PARENT, + [dependentKey(drive)]: null, + [dependentKey(spreadsheet)]: null, + [dependentKey(sheet)]: null, [dependentKey(unrelated)]: 'still-keep-me', }) expect(effectiveDependentValue(drive, next, false)).toBe('') @@ -172,10 +176,10 @@ describe('applyDependentRepick', () => { spreadsheet, [spreadsheet, range], 'sheet-doc', - effectiveDependentValue(spreadsheet, {}, false) + mappedRepickContext(effectiveDependentValue(spreadsheet, {}, false)) ) - expect(next).toEqual({ [dependentKey(spreadsheet)]: 'sheet-doc' }) + expect(next).toEqual({}) expect(effectiveDependentValue(range, next, false)).toBe('Sheet1!A1:D') }) @@ -188,7 +192,8 @@ describe('applyDependentRepick', () => { { [dependentKey(unrelated)]: 'still-keep-me' }, leaf, [leaf, unrelated], - 'ISSUE-2' + 'ISSUE-2', + mappedRepickContext('ISSUE-1') ) ).toEqual({ [dependentKey(leaf)]: 'ISSUE-2', @@ -227,97 +232,105 @@ describe('applyDependentRepick', () => { previous, projectOne, [projectOne, issueOne, projectTwo, issueTwo], - 'P1-NEW' + 'P1-NEW', + mappedRepickContext('INBOX') ) ).toEqual({ [dependentKey(projectOne)]: 'P1-NEW', - [dependentKey(issueOne)]: DEPENDENT_CLEARED_BY_PARENT, + [dependentKey(issueOne)]: null, [dependentKey(issueTwo)]: 'P2-1', }) }) -}) -describe('submittedDependentValue', () => { - const mappedParent = { copying: false, parentChanged: false } - - it('omits an optional descendant a parent re-pick blanked, so the target keeps its value', () => { + it('restores the stored chain when a provider is changed and then returned to baseline', () => { const spreadsheet = field({ subBlockKey: 'spreadsheetId', currentValue: 'doc-old', providesContextKey: 'spreadsheetId', }) - const sheet = field({ - subBlockKey: 'sheetName', - currentValue: 'Sheet1', - required: true, - consumesContextKeys: ['spreadsheetId'], - }) const range = field({ subBlockKey: 'range', currentValue: 'A1:D50', - required: false, consumesContextKeys: ['spreadsheetId'], }) - const next = applyDependentRepick( + const changed = applyDependentRepick( {}, spreadsheet, - [spreadsheet, sheet, range], + [spreadsheet, range], 'doc-new', - effectiveDependentValue(spreadsheet, {}, false) + mappedRepickContext('doc-old') + ) + const restored = applyDependentRepick( + changed, + spreadsheet, + [spreadsheet, range], + 'doc-old', + mappedRepickContext('doc-new') ) - expect(isDependentClearedByParent(range, next)).toBe(true) - expect(submittedDependentValue(range, next, mappedParent)).toBeUndefined() - expect(submittedDependentValue(sheet, next, mappedParent)).toBeUndefined() - expect(submittedDependentValue(spreadsheet, next, mappedParent)).toBe('doc-new') + expect(changed).toEqual({ + [dependentKey(spreadsheet)]: 'doc-new', + [dependentKey(range)]: null, + }) + expect(restored).toEqual({}) + expect(effectiveDependentValue(range, restored, false)).toBe('A1:D50') }) - it('submits an empty value the user picked themselves, so an explicit clear still clears', () => { + it('keeps an intentional empty pick distinct from automatic invalidation', () => { const label = field({ subBlockKey: 'label', currentValue: 'INBOX' }) - const next = applyDependentRepick({}, label, [label], '', 'INBOX') + const next = applyDependentRepick({}, label, [label], '', mappedRepickContext('INBOX')) - expect(isDependentClearedByParent(label, next)).toBe(false) - expect(submittedDependentValue(label, next, mappedParent)).toBe('') + expect(isDependentInvalidated(label, next)).toBe(false) + expect(next[dependentKey(label)]).toBe('') + expect(effectiveDependentValue(label, next, false)).toBe('') }) - it('submits a re-picked descendant once the user chooses a replacement', () => { + it('does not restore an Excel sheet while its drive still differs from baseline', () => { + const drive = field({ + subBlockKey: 'driveId', + currentValue: 'drive-old', + providesContextKey: 'driveId', + }) const spreadsheet = field({ subBlockKey: 'spreadsheetId', - currentValue: 'doc-old', + currentValue: 'workbook-old', + consumesContextKeys: ['driveId'], providesContextKey: 'spreadsheetId', }) - const range = field({ - subBlockKey: 'range', - currentValue: 'A1:D50', - consumesContextKeys: ['spreadsheetId'], + const sheet = field({ + subBlockKey: 'sheetName', + currentValue: 'Sheet1', + consumesContextKeys: ['driveId', 'spreadsheetId'], }) - const cleared = applyDependentRepick( + const driveChanged = applyDependentRepick( {}, + drive, + [drive, spreadsheet, sheet], + 'drive-new', + mappedRepickContext('drive-old') + ) + const spreadsheetRepicked = applyDependentRepick( + driveChanged, spreadsheet, - [spreadsheet, range], - 'doc-new', - effectiveDependentValue(spreadsheet, {}, false) + [drive, spreadsheet, sheet], + 'workbook-new', + mappedRepickContext('') ) - const repicked = applyDependentRepick( - cleared, - range, - [spreadsheet, range], - 'A1:Z', - effectiveDependentValue(range, cleared, false) + const spreadsheetRestored = applyDependentRepick( + spreadsheetRepicked, + spreadsheet, + [drive, spreadsheet, sheet], + 'workbook-old', + mappedRepickContext('workbook-new') ) - expect(submittedDependentValue(range, repicked, mappedParent)).toBe('A1:Z') - }) - - it('falls back to the stored value under an unchanged parent and to the source when copying', () => { - const untouched = field({ subBlockKey: 'label', currentValue: 'INBOX' }) - const copied = field({ subBlockKey: 'documentSelector', currentValue: '', sourceValue: 'doc' }) - - expect(submittedDependentValue(untouched, {}, mappedParent)).toBe('INBOX') - expect(submittedDependentValue(copied, {}, { copying: true, parentChanged: false })).toBe('doc') + expect(spreadsheetRestored).toEqual({ + [dependentKey(drive)]: 'drive-new', + [dependentKey(sheet)]: null, + }) }) }) @@ -409,7 +422,7 @@ describe('isDependentConfigurationActionable', () => { spreadsheet, [spreadsheet, sheet], 'doc-new', - effectiveDependentValue(spreadsheet, {}, false) + mappedRepickContext(effectiveDependentValue(spreadsheet, {}, false)) ) // The sync gate reads the same blank the selector shows, so the field gates and is visible. @@ -425,7 +438,7 @@ describe('isDependentConfigurationActionable', () => { it('shows a required field the user emptied themselves (it blocks Sync)', () => { const sheet = field({ subBlockKey: 'sheetName', required: true, currentValue: 'Sheet1' }) - const next = applyDependentRepick({}, sheet, [sheet], '', 'Sheet1') + const next = applyDependentRepick({}, sheet, [sheet], '', mappedRepickContext('Sheet1')) expect(effectiveDependentValue(sheet, next, false)).toBe('') expect( diff --git a/apps/sim/ee/workspace-forking/components/fork-sync/dependent-value.ts b/apps/sim/ee/workspace-forking/components/fork-sync/dependent-value.ts index 08ce8100e20..f23111dc4e3 100644 --- a/apps/sim/ee/workspace-forking/components/fork-sync/dependent-value.ts +++ b/apps/sim/ee/workspace-forking/components/fork-sync/dependent-value.ts @@ -1,5 +1,8 @@ import type { ForkDependentReconfig } from '@/lib/api/contracts/workspace-fork' +/** In-session dependent values. `null` means an upstream selector invalidated the field. */ +export type DependentReconfigState = Record + /** Stable key for a per-target dependent re-pick (target workflow + block + subblock). */ export function dependentKey(dependent: ForkDependentReconfig): string { return `${dependent.targetWorkflowId}:${dependent.targetBlockId}:${dependent.subBlockKey}` @@ -10,50 +13,62 @@ function sameDependencyScope(left: ForkDependentReconfig, right: ForkDependentRe } /** - * Marker stored for a descendant whose value an in-session parent re-pick invalidated, kept - * distinct from the user's own empty pick. It reads as blank everywhere it is consumed - the - * selector, the in-block chain context, and the sync gate - but is never submitted: the user - * has not chosen a replacement, so no override is written for it. A `''` the user picked - * themselves IS submitted and does clear the target. - * - * What the omission buys differs by path. On Save nothing rewrites the target draft, so its - * stored value survives outright - including across an undo (re-pick away, then back), where - * the parent nets out unchanged so `clearDependentsOnRemap` never fires and an explicit `''` - * would land on a value nobody touched. On Sync the written state is source-derived and - * `clearDependentsOnRemap` already blanks every top-level dependent of a remapped parent, so - * omission preserves nothing there; what it prevents is an explicit `''` reaching the fields - * that pass does not cover - nested `tools[i].param` values in particular, which - * `applyNestedToolOverrides` would otherwise blank. - * - * The escaped NUL prefix keeps it disjoint from every real selector value (ids, names, label - * paths) - no selector can produce one, so it can never collide with a genuine pick. + * Index the selector fields that provide each dependency context within a top-level block or + * nested tool instance. The fork diff has already normalized canonical basic/advanced fields + * into these context keys, so this is the same graph the selectors use to resolve their options. */ -export const DEPENDENT_CLEARED_BY_PARENT = '\u0000fork-sync:cleared-by-parent' +function indexContextProviders( + fields: ForkDependentReconfig[] +): Map> { + const providersByScope = new Map>() + for (const field of fields) { + if (!field.providesContextKey) continue + let providers = providersByScope.get(field.dependencyScope) + if (!providers) { + providers = new Map() + providersByScope.set(field.dependencyScope, providers) + } + providers.set(field.providesContextKey, field) + } + return providersByScope +} + +interface DependentRepickContext { + /** Effective value displayed immediately before this selection. */ + previousValue: string + /** Value each field falls back to when it has no in-session override. */ + baselineValueFor: (field: ForkDependentReconfig) => string +} /** - * Store a dependent re-pick and invalidate every selector transitively scoped by it, marking - * each with `DEPENDENT_CLEARED_BY_PARENT`: a changed provider makes every stored descendant - * stale for both mapped and copied parents, but only the fields the user reviews themselves - * get written to the target. + * Apply one selector choice and invalidate its transitive descendants within the same dependency + * scope. Automatic invalidation is represented by `null`, not `''`, so it can be distinguished + * from a user's intentional clear while the editor is open. * - * `previousValue` is the field's effective value before the pick. `onChange` fires even when - * the user re-selects the value the field already had, and that pick moves no scope, so nothing - * below it went stale - the cascade is skipped. Omit it only where no prior value is known. + * Returning a provider to its baseline restores every descendant whose complete provider chain + * has also returned to baseline. A descendant remains invalidated when another one of its + * providers is still changed. This makes A -> B -> A a true undo without reviving a value under + * a genuinely different scope. */ export function applyDependentRepick( - reconfig: Record, + reconfig: DependentReconfigState, changedField: ForkDependentReconfig, blockFields: ForkDependentReconfig[], value: string, - previousValue?: string -): Record { + context: DependentRepickContext +): DependentReconfigState { const changedKey = dependentKey(changedField) - const nextState = { ...reconfig, [changedKey]: value } - if (previousValue !== undefined && previousValue === value) return nextState + const baselineValue = context.baselineValueFor(changedField) + const nextState = { ...reconfig } + if (value === baselineValue) delete nextState[changedKey] + else nextState[changedKey] = value + + if (context.previousValue === value) return nextState if (!changedField.providesContextKey) return nextState const pendingContextKeys = [changedField.providesContextKey] const visitedFields = new Set([changedKey]) + const descendants: ForkDependentReconfig[] = [] for (let index = 0; index < pendingContextKeys.length; index += 1) { const contextKey = pendingContextKeys[index] if (!contextKey) continue @@ -69,11 +84,53 @@ export function applyDependentRepick( } visitedFields.add(fieldKey) - nextState[fieldKey] = DEPENDENT_CLEARED_BY_PARENT + descendants.push(field) if (field.providesContextKey) pendingContextKeys.push(field.providesContextKey) } } + for (const field of descendants) nextState[dependentKey(field)] = null + if (value !== baselineValue) return nextState + + const providersByScope = indexContextProviders(blockFields) + const fieldAndProvidersAtBaseline = ( + field: ForkDependentReconfig, + visiting: Set + ): boolean => { + const fieldKey = dependentKey(field) + if (visiting.has(fieldKey)) return false + const fieldValue = nextState[fieldKey] + const effectiveFieldValue = + fieldValue === undefined ? context.baselineValueFor(field) : fieldValue + if (effectiveFieldValue !== context.baselineValueFor(field)) return false + + visiting.add(fieldKey) + const providers = providersByScope.get(field.dependencyScope) + const providerChainAtBaseline = field.consumesContextKeys.every((contextKey) => { + const provider = providers?.get(contextKey) + return !provider || fieldAndProvidersAtBaseline(provider, visiting) + }) + visiting.delete(fieldKey) + return providerChainAtBaseline + } + + for (let pass = 0; pass < descendants.length; pass += 1) { + let restored = false + for (const field of descendants) { + const fieldKey = dependentKey(field) + if (nextState[fieldKey] !== null) continue + const providers = providersByScope.get(field.dependencyScope) + const providerChainAtBaseline = field.consumesContextKeys.every((contextKey) => { + const provider = providers?.get(contextKey) + return !provider || fieldAndProvidersAtBaseline(provider, new Set()) + }) + if (!providerChainAtBaseline) continue + delete nextState[fieldKey] + restored = true + } + if (!restored) break + } + return nextState } @@ -82,16 +139,16 @@ export function applyDependentRepick( * stored value (`currentValue`). Blank when the parent target changed in-session, or when an * in-block parent re-pick invalidated it, since the old stored value was for the previous parent * and won't resolve against the new one. Shared by the sync gate and the per-block selector so - * the rule can't drift between them. What gets SUBMITTED is `submittedDependentValue`, which - * additionally omits the fields the user has not reviewed. + * the rule cannot drift between them. The payload submits the same effective blank so stale + * values are removed from top-level fields and nested Agent tool parameters alike. */ export function effectiveDependentValue( field: ForkDependentReconfig, - reconfig: Record, + reconfig: DependentReconfigState, parentChanged: boolean ): string { const repicked = reconfig[dependentKey(field)] - if (repicked === DEPENDENT_CLEARED_BY_PARENT) return '' + if (repicked === null) return '' if (repicked !== undefined) return repicked return parentChanged ? '' : field.currentValue } @@ -106,42 +163,24 @@ export function effectiveDependentValue( */ export function effectiveCopyDependentValue( field: ForkDependentReconfig, - reconfig: Record + reconfig: DependentReconfigState ): string { const repicked = reconfig[dependentKey(field)] - if (repicked === DEPENDENT_CLEARED_BY_PARENT) return '' + if (repicked === null) return '' if (repicked !== undefined) return repicked return field.currentValue || field.sourceValue } /** - * Whether an in-block parent re-pick invalidated this field and the user has not re-picked it - * since. Such a field shows blank but is not submitted: blanking the target on the user's behalf - * would destroy a stored value they never chose to clear. + * Whether an in-block provider change invalidated this field and the user has not re-picked it. + * It reads and submits as empty while the provider remains changed, preventing a stale top-level + * or nested Agent tool value from surviving under the new scope. */ -export function isDependentClearedByParent( +export function isDependentInvalidated( field: ForkDependentReconfig, - reconfig: Record + reconfig: DependentReconfigState ): boolean { - return reconfig[dependentKey(field)] === DEPENDENT_CLEARED_BY_PARENT -} - -/** - * The value this dependent contributes to the submitted mapping, or `undefined` when it must be - * OMITTED so the target keeps what it already stores. Only a field the user reviewed is written: - * an explicit empty pick clears the target, while a value merely invalidated by a parent re-pick - * is left alone (it is on screen and blank, and if it is required the sync gate blocks until the - * user picks one - `effectiveDependentValue` reports it as blank). - */ -export function submittedDependentValue( - field: ForkDependentReconfig, - reconfig: Record, - state: { copying: boolean; parentChanged: boolean } -): string | undefined { - if (isDependentClearedByParent(field, reconfig)) return undefined - return state.copying - ? effectiveCopyDependentValue(field, reconfig) - : effectiveDependentValue(field, reconfig, state.parentChanged) + return reconfig[dependentKey(field)] === null } export interface DependentConfigurationState { @@ -158,12 +197,11 @@ export interface DependentConfigurationState { * A field a parent re-pick invalidated reads as blank through `effectiveDependentValue`, so a * REQUIRED one stays on screen here and keeps gating Sync. An optional one drops out of the * default view - `getDisplayedDependentFields` brings it back under explicit edit mode - and - * hiding it loses nothing, because `submittedDependentValue` omits it from the payload rather - * than blanking the target. + * hiding it is safe because the effective blank is still submitted to prevent a stale value. */ export function isDependentConfigurationActionable( field: ForkDependentReconfig, - reconfig: Record, + reconfig: DependentReconfigState, state: DependentConfigurationState ): boolean { if (!state.parentResolved) return false @@ -178,22 +216,13 @@ export function isDependentConfigurationActionable( */ export function getActionableDependentFields( fields: ForkDependentReconfig[], - reconfig: Record, + reconfig: DependentReconfigState, state: DependentConfigurationState ): ForkDependentReconfig[] { const actionable = new Set( fields.filter((field) => isDependentConfigurationActionable(field, reconfig, state)) ) - const providersByScope = new Map>() - for (const field of fields) { - if (!field.providesContextKey) continue - let providers = providersByScope.get(field.dependencyScope) - if (!providers) { - providers = new Map() - providersByScope.set(field.dependencyScope, providers) - } - providers.set(field.providesContextKey, field) - } + const providersByScope = indexContextProviders(fields) const pending = Array.from(actionable) for (let index = 0; index < pending.length; index += 1) { @@ -217,7 +246,7 @@ export function getActionableDependentFields( */ export function getDisplayedDependentFields( fields: ForkDependentReconfig[], - reconfig: Record, + reconfig: DependentReconfigState, state: DependentConfigurationState, showConfigured: boolean ): ForkDependentReconfig[] { diff --git a/apps/sim/ee/workspace-forking/components/fork-sync/fork-sync-view.tsx b/apps/sim/ee/workspace-forking/components/fork-sync/fork-sync-view.tsx index 8a04c531553..9a27dbf54eb 100644 --- a/apps/sim/ee/workspace-forking/components/fork-sync/fork-sync-view.tsx +++ b/apps/sim/ee/workspace-forking/components/fork-sync/fork-sync-view.tsx @@ -36,6 +36,7 @@ import { DependentFieldSelector } from '@/ee/workspace-forking/components/fork-s import { applyDependentRepick, type DependentConfigurationState, + type DependentReconfigState, dependentKey, effectiveCopyDependentValue, effectiveDependentValue, @@ -109,7 +110,7 @@ interface WorkflowDependents { function groupDependentsByWorkflow( workflows: ForkResourceUsage['workflows'], dependents: ForkDependentReconfig[], - reconfig: Record, + reconfig: DependentReconfigState, state: DependentConfigurationState, showConfigured: boolean ): WorkflowDependents[] { @@ -181,8 +182,8 @@ interface DependentSelectorProps { copying: boolean workspaceId: string sourceWorkspaceId: string - reconfig: Record - setReconfig: Dispatch>> + reconfig: DependentReconfigState + setReconfig: Dispatch> } /** @@ -204,10 +205,11 @@ function DependentSelector({ reconfig, setReconfig, }: DependentSelectorProps) { - const effectiveValueIn = (f: ForkDependentReconfig, state: Record) => + const effectiveValueIn = (f: ForkDependentReconfig, state: DependentReconfigState) => copying ? effectiveCopyDependentValue(f, state) : effectiveDependentValue(f, state, parentChanged) + const baselineValueFor = (f: ForkDependentReconfig) => effectiveValueIn(f, {}) const effectiveValue = (f: ForkDependentReconfig) => effectiveValueIn(f, reconfig) const { providedValues, providedContextKeys } = blockChainState(block, field, effectiveValue) // Disabled until every in-block parent it depends on has a value, so a child never queries @@ -234,13 +236,10 @@ function DependentSelector({ setReconfig((current) => // The pre-pick value comes from the state being updated, so re-selecting the value // already shown is recognised as the no-op it is and leaves descendants intact. - applyDependentRepick( - current, - field, - block.fields, - value, - effectiveValueIn(field, current) - ) + applyDependentRepick(current, field, block.fields, value, { + previousValue: effectiveValueIn(field, current), + baselineValueFor, + }) ) } title={field.title} @@ -257,8 +256,8 @@ interface DependentWorkflowCardProps { copying: boolean workspaceId: string sourceWorkspaceId: string - reconfig: Record - setReconfig: Dispatch>> + reconfig: DependentReconfigState + setReconfig: Dispatch> } /** diff --git a/apps/sim/ee/workspace-forking/components/fork-sync/use-fork-sync.test.tsx b/apps/sim/ee/workspace-forking/components/fork-sync/use-fork-sync.test.tsx index 6bb7799a36e..bd1fdf37319 100644 --- a/apps/sim/ee/workspace-forking/components/fork-sync/use-fork-sync.test.tsx +++ b/apps/sim/ee/workspace-forking/components/fork-sync/use-fork-sync.test.tsx @@ -2,11 +2,8 @@ * @vitest-environment jsdom * * Coverage for the payload {@link useForkSync} actually submits, not just the helpers it calls. - * `dependent-value.test.ts` proves `submittedDependentValue` omits a field an in-block parent - * re-pick invalidated; nothing there proves the hook USES it. Reverting the hook back to - * `dependentValueFor` re-introduces the P0 - a `{ value: '' }` written over the target's stored - * configuration - while every unit test stays green, so these tests assert on the submitted - * bodies and on the derived `dirty` / `syncDisabled` gates instead. + * The tests distinguish a genuine provider change, which must clear unresolved descendants, + * from a provider undo, which must restore them before Save or Sync derives its payload. */ import { act, type ReactNode } from 'react' import { createRoot, type Root } from 'react-dom/client' @@ -48,7 +45,6 @@ vi.mock('@/ee/workspace-forking/hooks/workspace-fork', () => ({ import { applyDependentRepick, - DEPENDENT_CLEARED_BY_PARENT, dependentKey, } from '@/ee/workspace-forking/components/fork-sync/dependent-value' import { @@ -127,6 +123,11 @@ const UNTOUCHED_FIELD = dependent({ const DEPENDENTS = [PARENT_FIELD, CHILD_FIELD, CLEARED_FIELD, UNTOUCHED_FIELD] +const mappedRepickContext = (previousValue: string) => ({ + previousValue, + baselineValueFor: (field: ForkDependentReconfig) => field.currentValue, +}) + function diffData(dependentReconfigs: ForkDependentReconfig[]) { return { sourceWorkspaceId: WORKSPACE_ID, @@ -233,26 +234,34 @@ afterEach(() => { }) describe('useForkSync dependent payload', () => { - it('omits a dependent an in-block parent re-pick invalidated, so the target keeps its stored value', () => { + it('submits an effective blank for an optional dependent invalidated by a real provider change', () => { const { get } = renderForkSync() act(() => { get().setReconfig((prev) => - applyDependentRepick(prev, PARENT_FIELD, DEPENDENTS, 'sheet-2', 'sheet-1') + applyDependentRepick( + prev, + PARENT_FIELD, + DEPENDENTS, + 'sheet-2', + mappedRepickContext('sheet-1') + ) ) }) - expect(get().reconfig[dependentKey(CHILD_FIELD)]).toBe(DEPENDENT_CLEARED_BY_PARENT) + expect(get().reconfig[dependentKey(CHILD_FIELD)]).toBeNull() act(() => get().save()) const submitted = savedDependentValues() expect(submitted?.map((entry) => entry.subBlockKey)).toEqual([ PARENT_FIELD.subBlockKey, + CHILD_FIELD.subBlockKey, CLEARED_FIELD.subBlockKey, UNTOUCHED_FIELD.subBlockKey, ]) expect(valueFor(submitted, PARENT_FIELD)).toBe('sheet-2') + expect(valueFor(submitted, CHILD_FIELD)).toBe('') expect(valueFor(submitted, UNTOUCHED_FIELD)).toBe('label-3') }) @@ -269,26 +278,35 @@ describe('useForkSync dependent payload', () => { expect(valueFor(submitted, CLEARED_FIELD)).toBe('') }) - it('leaves the editor clean when only markers remain, so an undone re-pick is not savable', () => { + it('restores the dependent chain when the provider re-pick is undone', () => { const { get } = renderForkSync() act(() => { get().setReconfig((prev) => - applyDependentRepick(prev, PARENT_FIELD, DEPENDENTS, 'sheet-2', 'sheet-1') + applyDependentRepick( + prev, + PARENT_FIELD, + DEPENDENTS, + 'sheet-2', + mappedRepickContext('sheet-1') + ) ) }) expect(get().dirty).toBe(true) - // The user puts the provider back to its stored value. Their own re-pick is no longer a - // change, and the marker it left on the child contributes nothing to the payload - so - // nothing is savable, even though `reconfig` is not empty. act(() => { get().setReconfig((prev) => - applyDependentRepick(prev, PARENT_FIELD, DEPENDENTS, 'sheet-1', 'sheet-2') + applyDependentRepick( + prev, + PARENT_FIELD, + DEPENDENTS, + 'sheet-1', + mappedRepickContext('sheet-2') + ) ) }) - expect(get().reconfig[dependentKey(CHILD_FIELD)]).toBe(DEPENDENT_CLEARED_BY_PARENT) + expect(get().reconfig).toEqual({}) expect(get().dirty).toBe(false) act(() => get().save()) @@ -314,30 +332,89 @@ describe('useForkSync dependent payload', () => { PARENT_FIELD, [PARENT_FIELD, requiredChild], 'sheet-2', - 'sheet-1' + mappedRepickContext('sheet-1') ) ) }) expect(get().syncDisabled).toBe(true) expect(get().syncDisabledReason).toBe('Reconfigure all required fields first') + + act(() => { + get().setReconfig((prev) => + applyDependentRepick( + prev, + PARENT_FIELD, + [PARENT_FIELD, requiredChild], + 'sheet-1', + mappedRepickContext('sheet-2') + ) + ) + }) + + expect(get().reconfig).toEqual({}) + expect(get().syncDisabled).toBe(false) }) - it('omits the marked dependent from the promote payload too', async () => { + it('submits the invalidated dependent as blank in the promote payload too', async () => { const { get } = renderForkSync() act(() => { get().setReconfig((prev) => - applyDependentRepick(prev, PARENT_FIELD, DEPENDENTS, 'sheet-2', 'sheet-1') + applyDependentRepick( + prev, + PARENT_FIELD, + DEPENDENTS, + 'sheet-2', + mappedRepickContext('sheet-1') + ) ) }) await act(async () => { await get().sync() }) - expect(promotedDependentValues()?.map((entry) => entry.subBlockKey)).not.toContain( - CHILD_FIELD.subBlockKey - ) + expect(valueFor(promotedDependentValues(), CHILD_FIELD)).toBe('') + }) + + it('clears a stale nested Agent tool child when its provider changes', async () => { + const project = dependent({ + subBlockKey: 'tools[0].projectId', + dependencyScope: 'tools[0]', + currentValue: 'project-old', + providesContextKey: 'projectId', + }) + const issue = dependent({ + subBlockKey: 'tools[0].issueKey', + dependencyScope: 'tools[0]', + currentValue: 'OLD-1', + consumesContextKeys: ['projectId'], + }) + mockUseForkDiff.mockReturnValue({ + data: diffData([project, issue]), + isError: false, + error: null, + isPlaceholderData: false, + }) + const { get } = renderForkSync() + + act(() => { + get().setReconfig((prev) => + applyDependentRepick( + prev, + project, + [project, issue], + 'project-new', + mappedRepickContext('project-old') + ) + ) + }) + await act(async () => { + await get().sync() + }) + + expect(valueFor(promotedDependentValues(), project)).toBe('project-new') + expect(valueFor(promotedDependentValues(), issue)).toBe('') }) }) @@ -347,7 +424,13 @@ describe('useForkSync post-sync reset', () => { act(() => { get().setReconfig((prev) => - applyDependentRepick(prev, PARENT_FIELD, DEPENDENTS, 'sheet-2', 'sheet-1') + applyDependentRepick( + prev, + PARENT_FIELD, + DEPENDENTS, + 'sheet-2', + mappedRepickContext('sheet-1') + ) ) }) expect(get().dirty).toBe(true) @@ -373,7 +456,13 @@ describe('useForkSync post-sync reset', () => { act(() => { get().setReconfig((prev) => - applyDependentRepick(prev, PARENT_FIELD, DEPENDENTS, 'sheet-2', 'sheet-1') + applyDependentRepick( + prev, + PARENT_FIELD, + DEPENDENTS, + 'sheet-2', + mappedRepickContext('sheet-1') + ) ) }) await act(async () => { diff --git a/apps/sim/ee/workspace-forking/components/fork-sync/use-fork-sync.ts b/apps/sim/ee/workspace-forking/components/fork-sync/use-fork-sync.ts index f2bcc58b301..932b6a5a3f0 100644 --- a/apps/sim/ee/workspace-forking/components/fork-sync/use-fork-sync.ts +++ b/apps/sim/ee/workspace-forking/components/fork-sync/use-fork-sync.ts @@ -31,11 +31,11 @@ import { isForkRequiredComplete, } from '@/ee/workspace-forking/components/fork-sync/copy-reconciliation' import { + type DependentReconfigState, dependentKey, effectiveCopyDependentValue, effectiveDependentValue, - isDependentClearedByParent, - submittedDependentValue, + isDependentInvalidated, } from '@/ee/workspace-forking/components/fork-sync/dependent-value' import { forkDyingTriggerUrls, @@ -147,8 +147,8 @@ export interface ForkSyncController { */ sourceWorkspaceId: string /** In-session dependent re-picks, keyed by `dependentKey`. */ - reconfig: Record - setReconfig: Dispatch>> + reconfig: DependentReconfigState + setReconfig: Dispatch> /** Keys the backend offers as copy candidates, for the entry rows' "Copy instead" affordance. */ copyableKeys: ReadonlySet /** Copyables actually selected for copy (visible + checked), keyed `${kind}:${sourceId}`. */ @@ -288,7 +288,7 @@ export function useForkSync(params: { // `dependentKey`. Folded into the full effective set sent on save/sync, which the server // persists as the stored mapping - so the selection survives every future sync without // re-picking. - const [reconfig, setReconfig] = useState>({}) + const [reconfig, setReconfig] = useState({}) // Referenced-but-unmapped resources the user chose to copy into the target (keyed by // `${kind}:${sourceId}`); default-selected once the diff loads. Selected ones are copied on // sync so their references resolve to the copy instead of being cleared. @@ -687,29 +687,23 @@ export function useForkSync(params: { // effective value: re-pick, stored, or blank-after-change) or copy-selected (re-pick, stored, // or the source reference; promote translates a source document id to its copied counterpart // at write time). The server persists this verbatim as the stored mapping; fields whose - // parent is unresolved are omitted (they can't be configured), as are fields an in-block - // parent re-pick invalidated and the user never re-picked - submitting those blank writes an - // explicit `''` override into the target draft (see `applyDependentOverrides`), which on the - // paths `clearDependentsOnRemap` does not cover would clear a value nobody chose to clear. - // `DEPENDENT_CLEARED_BY_PARENT` documents which paths those are. This is the whole "what's in - // the mapping goes in" contract, shared by Save and Sync so the two persist identically. + // parent is unresolved are omitted because they can't be configured. A field invalidated by + // an in-block provider re-pick is submitted as `''`: required fields gate Sync until re-picked, + // while optional/LLM-fillable fields must land empty rather than retain a source value scoped + // to the old provider. This is the whole "what's in the mapping goes in" contract, shared by + // Save and Sync so the two persist identically. const buildDependentValues = () => dependentReconfigs.flatMap((field) => { const parent = entryForDependent(field) if (!parent) return [] const resolution = resolutionFor(parent) if (resolution === 'unresolved') return [] - const value = submittedDependentValue(field, reconfig, { - copying: resolution === 'copied', - parentChanged: shouldReconfigureEntry(parent, targets), - }) - if (value === undefined) return [] return [ { workflowId: field.targetWorkflowId, blockId: field.targetBlockId, subBlockKey: field.subBlockKey, - value, + value: dependentValueFor(field, parent, resolution), }, ] }) @@ -734,12 +728,12 @@ export function useForkSync(params: { // A dependent re-pick that differs from its stored value also dirties the editor. A re-pick // under a changed parent is covered by `targetsDirty` (the parent override is the change). - // A field merely invalidated by a parent re-pick is not a change of its own - it contributes - // nothing to the payload, so it must not make an otherwise untouched editor savable. + // An automatically invalidated field is covered by the provider override that caused it; it + // must not independently dirty an editor after that provider has been restored to baseline. const reconfigDirty = useMemo( () => dependentReconfigs.some((field) => { - if (isDependentClearedByParent(field, reconfig)) return false + if (isDependentInvalidated(field, reconfig)) return false const repicked = reconfig[dependentKey(field)] return repicked !== undefined && repicked !== field.currentValue }), @@ -938,8 +932,7 @@ export function useForkSync(params: { // The run committed the in-session choices: the mapping entries and dependent values are // stored, so keeping the overrides would leave every touched field's card on screen against - // a freshly refetched diff - and a `DEPENDENT_CLEARED_BY_PARENT` marker would keep rendering - // blank over the value the sync just stored. Drop them exactly as a successful Save does. + // a freshly refetched diff. Drop them exactly as a successful Save does. setTargets({}) setReconfig({}) diff --git a/apps/sim/ee/workspace-forking/lib/remap/remap-references.test.ts b/apps/sim/ee/workspace-forking/lib/remap/remap-references.test.ts index 4f050b71c5d..0334e30e1d5 100644 --- a/apps/sim/ee/workspace-forking/lib/remap/remap-references.test.ts +++ b/apps/sim/ee/workspace-forking/lib/remap/remap-references.test.ts @@ -1117,6 +1117,56 @@ describe('applyDependentOverrides', () => { expect(tools[0].params.folder).toBe('Label_99') }) + it('applies an invalidated nested child as empty so it cannot survive under a new provider', () => { + vi.mocked(getBlock).mockImplementation((type) => { + if (type === 'agent') return blockWith([{ id: 'tools', title: 'Tools', type: 'tool-input' }]) + if (type === 'jira') { + return blockWith([ + { id: 'credential', title: 'Credential', type: 'oauth-input' }, + { + id: 'projectId', + title: 'Project', + type: 'project-selector', + dependsOn: ['credential'], + selectorKey: 'jira.projects', + }, + { + id: 'issueKey', + title: 'Issue', + type: 'issue-selector', + dependsOn: ['projectId'], + selectorKey: 'jira.issues', + }, + ]) + } + return undefined as unknown as BlockConfig + }) + const subBlocks: SubBlockRecord = { + tools: entry('tools', 'tool-input', [ + { + type: 'jira', + title: 'Jira', + params: { credential: 'c-new', projectId: 'project-old', issueKey: 'OLD-1' }, + }, + ]), + } + + const result = applyDependentOverrides( + subBlocks, + 'agent', + new Map([ + ['tools[0].projectId', 'project-new'], + ['tools[0].issueKey', ''], + ]) + ) + const tools = ( + result.tools as { + value: Array<{ params: { projectId: string; issueKey: string } }> + } + ).value + expect(tools[0].params).toMatchObject({ projectId: 'project-new', issueKey: '' }) + }) + it('rejects a nested override for a non-allowlisted tool param', () => { vi.mocked(getBlock).mockImplementation((type) => { if (type === 'agent') return blockWith([{ id: 'tools', title: 'Tools', type: 'tool-input' }]) From d636d5e7071533573081f67994cbc9f7215f72d6 Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Mon, 17 Aug 2026 16:17:13 -0700 Subject: [PATCH 3/4] fix(forking): preserve edits during fork sync --- .../fork-sync/use-fork-sync.test.tsx | 100 ++++++++++++++++-- .../components/fork-sync/use-fork-sync.ts | 16 +-- .../lib/copy/copy-files.test.ts | 3 + .../workspace-forking/lib/copy/copy-files.ts | 6 +- 4 files changed, 109 insertions(+), 16 deletions(-) diff --git a/apps/sim/ee/workspace-forking/components/fork-sync/use-fork-sync.test.tsx b/apps/sim/ee/workspace-forking/components/fork-sync/use-fork-sync.test.tsx index bd1fdf37319..dbf6f10d502 100644 --- a/apps/sim/ee/workspace-forking/components/fork-sync/use-fork-sync.test.tsx +++ b/apps/sim/ee/workspace-forking/components/fork-sync/use-fork-sync.test.tsx @@ -123,6 +123,23 @@ const UNTOUCHED_FIELD = dependent({ const DEPENDENTS = [PARENT_FIELD, CHILD_FIELD, CLEARED_FIELD, UNTOUCHED_FIELD] +const SUCCESSFUL_PROMOTE_RESULT = { + promoteRunId: 'run-1', + blockers: [], + unmappedRequired: [], + droppedReferences: [], + triggerUrlChanges: [], + deployFailed: 0, +} + +function createDeferred() { + let resolve!: (value: T) => void + const promise = new Promise((resolvePromise) => { + resolve = resolvePromise + }) + return { promise, resolve } +} + const mappedRepickContext = (previousValue: string) => ({ previousValue, baselineValueFor: (field: ForkDependentReconfig) => field.currentValue, @@ -217,14 +234,7 @@ beforeEach(() => { isPlaceholderData: false, }) mockUpdateMutateAsync.mockResolvedValue({ success: true, updated: 1 }) - mockPromote.mockResolvedValue({ - promoteRunId: 'run-1', - blockers: [], - unmappedRequired: [], - droppedReferences: [], - triggerUrlChanges: [], - deployFailed: 0, - }) + mockPromote.mockResolvedValue(SUCCESSFUL_PROMOTE_RESULT) }) afterEach(() => { @@ -471,4 +481,78 @@ describe('useForkSync post-sync reset', () => { expect(get().reconfig[dependentKey(PARENT_FIELD)]).toBe('sheet-2') }) + + it('keeps a newer target mapping selected while Sync was in flight', async () => { + const pendingPromote = createDeferred() + mockPromote.mockReturnValue(pendingPromote.promise) + const { get } = renderForkSync() + let syncPromise!: Promise + + act(() => { + syncPromise = get().sync() + }) + await act(async () => Promise.resolve()) + expect(mockPromote).toHaveBeenCalledTimes(1) + + act(() => get().setTarget(CREDENTIAL_ENTRY, 'cred-newer')) + pendingPromote.resolve(SUCCESSFUL_PROMOTE_RESULT) + await act(async () => syncPromise) + + expect(get().targetFor(CREDENTIAL_ENTRY)).toBe('cred-newer') + expect(get().dirty).toBe(true) + }) + + it('keeps a newer dependent re-pick made while Sync was in flight', async () => { + const pendingPromote = createDeferred() + mockPromote.mockReturnValue(pendingPromote.promise) + const { get } = renderForkSync() + let syncPromise!: Promise + + act(() => { + syncPromise = get().sync() + }) + await act(async () => Promise.resolve()) + expect(mockPromote).toHaveBeenCalledTimes(1) + + act(() => { + get().setReconfig((current) => ({ + ...current, + [dependentKey(PARENT_FIELD)]: 'sheet-newer', + })) + }) + pendingPromote.resolve(SUCCESSFUL_PROMOTE_RESULT) + await act(async () => syncPromise) + + expect(get().reconfig[dependentKey(PARENT_FIELD)]).toBe('sheet-newer') + expect(get().dirty).toBe(true) + }) + + it('keeps newer mapping edits made while Save was in flight', () => { + let finishSave: (() => void) | undefined + mockUpdateMutate.mockImplementation((_variables, options) => { + finishSave = options.onSuccess + }) + const { get } = renderForkSync() + + act(() => { + get().setReconfig((current) => ({ + ...current, + [dependentKey(PARENT_FIELD)]: 'sheet-submitted', + })) + }) + act(() => get().save()) + + act(() => { + get().setTarget(CREDENTIAL_ENTRY, 'cred-newer') + get().setReconfig((current) => ({ + ...current, + [dependentKey(PARENT_FIELD)]: 'sheet-newer', + })) + }) + act(() => finishSave?.()) + + expect(get().targetFor(CREDENTIAL_ENTRY)).toBe('cred-newer') + expect(get().reconfig[dependentKey(PARENT_FIELD)]).toBe('sheet-newer') + expect(get().dirty).toBe(true) + }) }) diff --git a/apps/sim/ee/workspace-forking/components/fork-sync/use-fork-sync.ts b/apps/sim/ee/workspace-forking/components/fork-sync/use-fork-sync.ts index 932b6a5a3f0..61aadc7bce5 100644 --- a/apps/sim/ee/workspace-forking/components/fork-sync/use-fork-sync.ts +++ b/apps/sim/ee/workspace-forking/components/fork-sync/use-fork-sync.ts @@ -744,6 +744,8 @@ export function useForkSync(params: { const save = () => { if (!otherWorkspaceId || !dirty || updateMapping.isPending) return + const submittedTargets = targets + const submittedReconfig = reconfig updateMapping.mutate( { workspaceId, @@ -759,8 +761,8 @@ export function useForkSync(params: { }, { onSuccess: () => { - setTargets({}) - setReconfig({}) + setTargets((current) => (current === submittedTargets ? {} : current)) + setReconfig((current) => (current === submittedReconfig ? {} : current)) toast.success('Mapping saved') }, onError: (error) => toast.error(getErrorMessage(error, 'Failed to save mapping')), @@ -836,6 +838,8 @@ export function useForkSync(params: { const sync = async () => { if (!otherWorkspaceId) return setSubmitting(true) + const submittedTargets = targets + const submittedReconfig = reconfig // Capture every payload from the state at confirm time, before any await - the page's // controls stay mounted during the run (unlike the old modal, which blocked its UI), so a // mid-flight edit must not leak into the promote body. @@ -931,10 +935,10 @@ export function useForkSync(params: { } // The run committed the in-session choices: the mapping entries and dependent values are - // stored, so keeping the overrides would leave every touched field's card on screen against - // a freshly refetched diff. Drop them exactly as a successful Save does. - setTargets({}) - setReconfig({}) + // stored. Drop only the exact snapshots it submitted; edits made while the request was in + // flight were not committed by this run and must remain available for the next Save/Sync. + setTargets((current) => (current === submittedTargets ? {} : current)) + setReconfig((current) => (current === submittedReconfig ? {} : current)) const target = otherWorkspaceName || 'the workspace' const label = direction === 'pull' ? `Pulled from "${target}"` : `Pushed to "${target}"` diff --git a/apps/sim/ee/workspace-forking/lib/copy/copy-files.test.ts b/apps/sim/ee/workspace-forking/lib/copy/copy-files.test.ts index 9f833f565e1..76f2bb315aa 100644 --- a/apps/sim/ee/workspace-forking/lib/copy/copy-files.test.ts +++ b/apps/sim/ee/workspace-forking/lib/copy/copy-files.test.ts @@ -382,6 +382,7 @@ describe('executeForkFileBlobCopies target name collisions', () => { targetKey: 'workspace/child-ws/target-budget.xlsx', contentType: 'application/vnd.ms-excel', targetFolderId: 'target-reports', + displayName: 'budget.xlsx', }) beforeEach(() => { @@ -427,6 +428,7 @@ describe('executeForkFileBlobCopies target name collisions', () => { workspaceId: 'child-ws', folderId: 'target-reports', originalName: 'budget (1).xlsx', + displayName: 'budget (1).xlsx', deletedAt: null, }) // The blob backing the surviving row must never be swept. @@ -463,6 +465,7 @@ describe('executeForkFileBlobCopies target name collisions', () => { expect(fileRows.find((row) => row.id === 'target-file-1')).toMatchObject({ folderId: 'target-reports', originalName: 'budget.xlsx', + displayName: 'budget.xlsx', }) expect(storageServiceMockFns.mockDeleteFile).not.toHaveBeenCalled() }) diff --git a/apps/sim/ee/workspace-forking/lib/copy/copy-files.ts b/apps/sim/ee/workspace-forking/lib/copy/copy-files.ts index d84745adc05..22ff96868a3 100644 --- a/apps/sim/ee/workspace-forking/lib/copy/copy-files.ts +++ b/apps/sim/ee/workspace-forking/lib/copy/copy-files.ts @@ -325,6 +325,8 @@ export async function executeForkFileBlobCopies( const billingContext = await resolveStorageBillingContext(task.workspaceId) const targetOriginalName = await resolveTargetOriginalName(task) + const targetDisplayName = + targetOriginalName === task.fileName ? task.displayName : targetOriginalName await db.transaction(async (tx) => { const [inserted] = await tx .insert(workspaceFiles) @@ -337,7 +339,7 @@ export async function executeForkFileBlobCopies( context: task.context, chatId: null, originalName: targetOriginalName, - displayName: task.displayName, + displayName: targetDisplayName, contentType: task.contentType, size: task.size, deletedAt: null, @@ -385,7 +387,7 @@ export async function executeForkFileBlobCopies( context: task.context, chatId: null, originalName: targetOriginalName, - displayName: task.displayName, + displayName: targetDisplayName, contentType: task.contentType, size: task.size, deletedAt: null, From 0739aa94788281ce23d37d228ca319dec8e85ea4 Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Mon, 17 Aug 2026 17:51:16 -0700 Subject: [PATCH 4/4] fix(workflows): clear stale dependent inputs in Mothership edits --- .../connector-selector-field.tsx | 2 +- .../[id]/hooks/use-connector-config-fields.ts | 2 +- .../components/combobox/combobox.tsx | 2 +- .../components/dropdown/dropdown.tsx | 2 +- .../components/tool-input/param-dependents.ts | 6 +- .../workflow-search-replace.tsx | 9 +- .../workflow-block/workflow-block.tsx | 2 +- apps/sim/blocks/utils.test.ts | 93 ------- apps/sim/blocks/utils.ts | 34 --- .../lib/mapping/dependent-reconfigs.ts | 8 +- .../lib/remap/remap-references.ts | 13 +- apps/sim/hooks/use-collaborative-workflow.ts | 2 +- .../workflow/edit-workflow/operations.test.ts | 256 +++++++++++++++--- .../workflow/edit-workflow/operations.ts | 50 +++- .../search-replace/dependencies.test.ts | 24 -- .../workflows/search-replace/dependencies.ts | 33 --- .../lib/workflows/search-replace/indexer.ts | 4 +- .../workflows/subblocks/dependencies.test.ts | 137 ++++++++++ .../lib/workflows/subblocks/dependencies.ts | 66 +++++ 19 files changed, 492 insertions(+), 253 deletions(-) delete mode 100644 apps/sim/lib/workflows/search-replace/dependencies.test.ts delete mode 100644 apps/sim/lib/workflows/search-replace/dependencies.ts create mode 100644 apps/sim/lib/workflows/subblocks/dependencies.test.ts create mode 100644 apps/sim/lib/workflows/subblocks/dependencies.ts diff --git a/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/connector-selector-field/connector-selector-field.tsx b/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/connector-selector-field/connector-selector-field.tsx index 6cdd14d27ef..4252b6ec357 100644 --- a/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/connector-selector-field/connector-selector-field.tsx +++ b/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/connector-selector-field/connector-selector-field.tsx @@ -5,11 +5,11 @@ import { ChipCombobox, type ComboboxOption } from '@sim/emcn' import { Loader } from '@sim/emcn/icons' import { SEARCH_DEBOUNCE_MS } from '@/lib/url-state' import { SELECTOR_CONTEXT_FIELDS } from '@/lib/workflows/subblocks/context' +import { getDependsOnFields } from '@/lib/workflows/subblocks/dependencies' import type { ConfigFieldMap, ConfigFieldValue, } from '@/app/workspace/[workspaceId]/knowledge/[id]/hooks/use-connector-config-fields' -import { getDependsOnFields } from '@/blocks/utils' import type { ConnectorConfigField } from '@/connectors/types' import { getSelectorDefinition } from '@/hooks/selectors/registry' import type { SelectorContext, SelectorKey } from '@/hooks/selectors/types' diff --git a/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/hooks/use-connector-config-fields.ts b/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/hooks/use-connector-config-fields.ts index 955891811d2..69723548992 100644 --- a/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/hooks/use-connector-config-fields.ts +++ b/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/hooks/use-connector-config-fields.ts @@ -1,7 +1,7 @@ 'use client' import { useCallback, useMemo, useState } from 'react' -import { getDependsOnFields } from '@/blocks/utils' +import { getDependsOnFields } from '@/lib/workflows/subblocks/dependencies' import type { ConnectorConfigField, ConnectorMeta } from '@/connectors/types' export type ConfigFieldValue = string | string[] diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/combobox/combobox.tsx b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/combobox/combobox.tsx index bea443e2f7d..f8ebccf4356 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/combobox/combobox.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/combobox/combobox.tsx @@ -2,6 +2,7 @@ import { memo, useCallback, useEffect, useMemo, useRef, useState } from 'react' import { Combobox, type ComboboxOption, cn } from '@sim/emcn' import { Plus } from '@sim/emcn/icons' import { useReactFlow } from 'reactflow' +import { getDependsOnFields } from '@/lib/workflows/subblocks/dependencies' import { SandboxCreateModal } from '@/app/workspace/[workspaceId]/settings/components/sandboxes/components/sandbox-create-modal' import type { SandboxLanguage } from '@/app/workspace/[workspaceId]/settings/components/sandboxes/utils' import { shouldClearMissingOption } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/combobox/missing-option' @@ -13,7 +14,6 @@ import { useSubBlockValue } from '@/app/workspace/[workspaceId]/w/[workflowId]/c import { useActiveSearchTarget } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/providers/active-search-target-provider' import { useAccessibleReferencePrefixes } from '@/app/workspace/[workspaceId]/w/[workflowId]/hooks/use-accessible-reference-prefixes' import type { SubBlockConfig } from '@/blocks/types' -import { getDependsOnFields } from '@/blocks/utils' import { usePermissionConfig } from '@/hooks/use-permission-config' import { useSubBlockStore } from '@/stores/workflows/subblock/store' diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/dropdown/dropdown.tsx b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/dropdown/dropdown.tsx index 996fbf7f3f4..b14dfc42c4e 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/dropdown/dropdown.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/dropdown/dropdown.tsx @@ -6,6 +6,7 @@ import { NO_DENIED_OPERATIONS, OPERATION_SUBBLOCK_ID, } from '@/lib/permission-groups/operation-access' +import { getDependsOnFields } from '@/lib/workflows/subblocks/dependencies' import { formatDisplayText } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/formatted-text' import { getWorkflowSearchLabelHighlight } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/workflow-search-highlight' import { useFetchedOptions } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/hooks/use-fetched-options' @@ -13,7 +14,6 @@ import { useSubBlockValue } from '@/app/workspace/[workspaceId]/w/[workflowId]/c import { useActiveSearchTarget } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/providers/active-search-target-provider' import { getBlock } from '@/blocks/registry' import type { SubBlockConfig } from '@/blocks/types' -import { getDependsOnFields } from '@/blocks/utils' import { ResponseBlockHandler } from '@/executor/handlers/response/response-handler' import { useOperationAccess } from '@/hooks/use-operation-access' import { useWorkflowStore } from '@/stores/workflows/workflow/store' diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/param-dependents.ts b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/param-dependents.ts index a860066f290..77e44462cf0 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/param-dependents.ts +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/param-dependents.ts @@ -1,10 +1,10 @@ -import { getWorkflowSearchDependentClears } from '@/lib/workflows/search-replace/dependencies' +import { getTransitiveSubBlockDependents } from '@/lib/workflows/subblocks/dependencies' import { getBlock } from '@/blocks/registry' /** * Clear every TRANSITIVE `dependsOn` descendant of `changedParamId` in a nested tool's params, * mirroring the top-level block clear (`use-collaborative-workflow`). Reuses the shared - * {@link getWorkflowSearchDependentClears} walk - transitive BFS plus canonical-pair expansion, so a + * {@link getTransitiveSubBlockDependents} walk - transitive BFS plus canonical-pair expansion, so a * basic OR advanced member change clears the dependent - so both surfaces clear identically. Only * descendants that currently hold a non-empty value are reset to `''`; the changed param itself and * non-descendants are untouched. Returns the same reference when nothing changed. @@ -16,7 +16,7 @@ export function clearDependentToolParams( ): Record { const subBlocks = getBlock(toolType)?.subBlocks ?? [] let next: Record | null = null - for (const { subBlockId } of getWorkflowSearchDependentClears(subBlocks, changedParamId)) { + for (const { subBlockId } of getTransitiveSubBlockDependents(subBlocks, [changedParamId])) { if (!params[subBlockId]) continue next ??= { ...params } next[subBlockId] = '' diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/search-replace/workflow-search-replace.tsx b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/search-replace/workflow-search-replace.tsx index be31940d6ab..90d773f7933 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/search-replace/workflow-search-replace.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/search-replace/workflow-search-replace.tsx @@ -5,7 +5,6 @@ import { Button, cn, Input, toast } from '@sim/emcn' import { ChevronDown, ChevronRight, ChevronUp, X } from '@sim/emcn/icons' import { useParams } from 'next/navigation' import { useShallow } from 'zustand/react/shallow' -import { getWorkflowSearchDependentClears } from '@/lib/workflows/search-replace/dependencies' import { indexWorkflowSearchMatches } from '@/lib/workflows/search-replace/indexer' import { buildWorkflowSearchReplacePlan } from '@/lib/workflows/search-replace/replacements' import { @@ -20,6 +19,7 @@ import { import { getWorkflowSearchBlocks } from '@/lib/workflows/search-replace/state' import { WORKFLOW_SEARCH_SUBFLOW_FIELD_IDS } from '@/lib/workflows/search-replace/subflow-fields' import type { WorkflowSearchReplaceSubflowUpdate } from '@/lib/workflows/search-replace/types' +import { getTransitiveSubBlockDependents } from '@/lib/workflows/subblocks/dependencies' import { useRegisterGlobalCommands } from '@/app/workspace/[workspaceId]/providers/global-commands-provider' import { useUserPermissionsContext } from '@/app/workspace/[workspaceId]/providers/workspace-permissions-provider' import { createCommand } from '@/app/workspace/[workspaceId]/utils/commands-utils' @@ -474,10 +474,9 @@ export function WorkflowSearchReplace() { const blockConfig = block ? getBlock(block.type) : null if (!blockConfig?.subBlocks) continue - const dependentClears = getWorkflowSearchDependentClears( - blockConfig.subBlocks, - update.subBlockId - ) + const dependentClears = getTransitiveSubBlockDependents(blockConfig.subBlocks, [ + update.subBlockId, + ]) for (const clear of dependentClears) { const alreadyUpdated = batchUpdates.some( (candidate) => diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/workflow-block/workflow-block.tsx b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/workflow-block/workflow-block.tsx index 3babe49ba7a..bc9714ea200 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/workflow-block/workflow-block.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/workflow-block/workflow-block.tsx @@ -50,6 +50,7 @@ import { import { resolveSelectedTriggerId } from '@/lib/workflows/blocks/canvas-trigger-sentence' import { calculateWorkflowBlockDimensions } from '@/lib/workflows/blocks/deterministic-dimensions' import { getConditionRows, getRouterRows } from '@/lib/workflows/dynamic-handle-topology' +import { getDependsOnFields } from '@/lib/workflows/subblocks/dependencies' import { getDisplayValue, hasDisplayableRowValue, @@ -96,7 +97,6 @@ import { SELECTOR_TYPES_HYDRATION_REQUIRED, type SubBlockConfig, } from '@/blocks/types' -import { getDependsOnFields } from '@/blocks/utils' import { useKnowledgeBase } from '@/hooks/kb/use-knowledge' import { useCustomTools } from '@/hooks/queries/custom-tools' import { useDeployWorkflow } from '@/hooks/queries/deployments' diff --git a/apps/sim/blocks/utils.test.ts b/apps/sim/blocks/utils.test.ts index fd71606f4ae..1cf444caa07 100644 --- a/apps/sim/blocks/utils.test.ts +++ b/apps/sim/blocks/utils.test.ts @@ -67,13 +67,10 @@ vi.mock('@/lib/oauth/utils', () => ({ getScopesForService: vi.fn(() => []), })) -import type { SubBlockConfig } from '@/blocks/types' import { BUILT_IN_TOOL_TYPES, getApiKeyCondition, - getDependsOnFields, getSerializedModelProviderId, - getSubBlocksDependingOnChange, parseOptionalBooleanInput, parseOptionalJsonInput, parseOptionalNumberInput, @@ -377,96 +374,6 @@ describe('parseOptionalBooleanInput', () => { }) }) -describe('getDependsOnFields', () => { - it('returns an empty array when dependsOn is unset', () => { - expect(getDependsOnFields(undefined)).toEqual([]) - }) - - it('returns array dependencies unchanged', () => { - expect(getDependsOnFields(['credential', 'projectId'])).toEqual(['credential', 'projectId']) - }) - - it('flattens all and any dependencies', () => { - expect(getDependsOnFields({ all: ['credential'], any: ['teamId', 'manualTeamId'] })).toEqual([ - 'credential', - 'teamId', - 'manualTeamId', - ]) - }) -}) - -describe('getSubBlocksDependingOnChange', () => { - it('finds direct dependents of a changed subblock', () => { - const subBlocks: SubBlockConfig[] = [ - { id: 'provider', title: 'Provider', type: 'dropdown' }, - { id: 'model', title: 'Model', type: 'dropdown', dependsOn: ['provider'] }, - { id: 'prompt', title: 'Prompt', type: 'long-input' }, - ] - - expect( - getSubBlocksDependingOnChange(subBlocks, 'provider').map((subBlock) => subBlock.id) - ).toEqual(['model']) - }) - - it('matches dependents through canonical basic and advanced siblings', () => { - const subBlocks: SubBlockConfig[] = [ - { - id: 'channel', - title: 'Channel', - type: 'channel-selector', - canonicalParamId: 'channelId', - mode: 'basic', - }, - { - id: 'manualChannel', - title: 'Channel ID', - type: 'short-input', - canonicalParamId: 'channelId', - mode: 'advanced', - }, - { - id: 'messageId', - title: 'Message ID', - type: 'short-input', - dependsOn: ['channelId'], - }, - { - id: 'threadTs', - title: 'Thread Timestamp', - type: 'short-input', - dependsOn: ['otherField'], - }, - ] - - expect( - getSubBlocksDependingOnChange(subBlocks, 'manualChannel').map((subBlock) => subBlock.id) - ).toEqual(['messageId']) - expect( - getSubBlocksDependingOnChange(subBlocks, 'channel').map((subBlock) => subBlock.id) - ).toEqual(['messageId']) - }) - - it('matches object-form dependencies when any listed dependency changes', () => { - const subBlocks: SubBlockConfig[] = [ - { id: 'credential', title: 'Credential', type: 'oauth-input' }, - { id: 'teamId', title: 'Team', type: 'short-input' }, - { - id: 'projectId', - title: 'Project', - type: 'short-input', - dependsOn: { all: ['credential'], any: ['teamId'] }, - }, - ] - - expect( - getSubBlocksDependingOnChange(subBlocks, 'credential').map((subBlock) => subBlock.id) - ).toEqual(['projectId']) - expect( - getSubBlocksDependingOnChange(subBlocks, 'teamId').map((subBlock) => subBlock.id) - ).toEqual(['projectId']) - }) -}) - describe('getSerializedModelProviderId', () => { const resolver = vi.mocked(getProviderFromModel) diff --git a/apps/sim/blocks/utils.ts b/apps/sim/blocks/utils.ts index 909e42a01f4..60e4efad52f 100644 --- a/apps/sim/blocks/utils.ts +++ b/apps/sim/blocks/utils.ts @@ -8,7 +8,6 @@ import { } from '@/lib/core/config/env-flags' import { getScopesForService } from '@/lib/oauth/utils' import { containsReference } from '@/lib/workflows/sanitization/references' -import { buildCanonicalIndex } from '@/lib/workflows/subblocks/visibility' import type { SubBlockConfig } from '@/blocks/types' import { getBaseModelProviders, @@ -112,39 +111,6 @@ export function getPiModelOptions() { }) } -/** - * Gets all dependency fields as a flat array. - * Handles both simple array format and object format with all/any fields. - */ -export function getDependsOnFields(dependsOn: SubBlockConfig['dependsOn']): string[] { - if (!dependsOn) return [] - if (Array.isArray(dependsOn)) return dependsOn - return [...(dependsOn.all || []), ...(dependsOn.any || [])] -} - -/** - * Finds subblocks that depend on a changed field, accounting for canonical pairs. - */ -export function getSubBlocksDependingOnChange( - allSubBlocks: SubBlockConfig[], - changedSubBlockId: string -): SubBlockConfig[] { - const canonicalIndex = buildCanonicalIndex(allSubBlocks) - const canonicalId = canonicalIndex.canonicalIdBySubBlockId[changedSubBlockId] - const group = canonicalId ? canonicalIndex.groupsById[canonicalId] : undefined - const changedFields = new Set([changedSubBlockId]) - - if (canonicalId) changedFields.add(canonicalId) - if (group?.basicId) changedFields.add(group.basicId) - for (const advancedId of group?.advancedIds || []) { - changedFields.add(advancedId) - } - - return allSubBlocks.filter((subBlock) => - getDependsOnFields(subBlock.dependsOn).some((field) => changedFields.has(field)) - ) -} - function getProviderFromStore(model: string): string | null { const { providers } = useProvidersStore.getState() const normalized = model.toLowerCase() diff --git a/apps/sim/ee/workspace-forking/lib/mapping/dependent-reconfigs.ts b/apps/sim/ee/workspace-forking/lib/mapping/dependent-reconfigs.ts index 9d52ec1ce21..f050018cec4 100644 --- a/apps/sim/ee/workspace-forking/lib/mapping/dependent-reconfigs.ts +++ b/apps/sim/ee/workspace-forking/lib/mapping/dependent-reconfigs.ts @@ -1,12 +1,15 @@ import { isRecordLike } from '@sim/utils/object' import type { ForkDependentReconfig, ForkResourceUsage } from '@/lib/api/contracts/workspace-fork' import { coerceObjectArray } from '@/lib/workflows/persistence/remap-internal-ids' -import { getWorkflowSearchDependentClears } from '@/lib/workflows/search-replace/dependencies' import { getToolInputParamConfigs } from '@/lib/workflows/search-replace/indexer' import { buildSelectorContextFromBlock, SELECTOR_CONTEXT_FIELDS, } from '@/lib/workflows/subblocks/context' +import { + getDependsOnFields, + getTransitiveSubBlockDependents, +} from '@/lib/workflows/subblocks/dependencies' import { buildCanonicalIndex, buildSubBlockValues, @@ -18,7 +21,6 @@ import { import { resolveToolParamRequired } from '@/lib/workflows/tool-input/param-visibility' import { getBlock } from '@/blocks/registry' import type { SubBlockConfig } from '@/blocks/types' -import { getDependsOnFields } from '@/blocks/utils' import type { ForkBlockIdResolver } from '@/ee/workspace-forking/lib/remap/block-identity' import { toScannerBlocks } from '@/ee/workspace-forking/lib/remap/reference-scan' import { @@ -163,7 +165,7 @@ function emitAnchoredDependents(params: EmitAnchoredParams): void { if (typeof value === 'string' && value) context[key] = value } - for (const clear of getWorkflowSearchDependentClears(config.subBlocks, anchorCfg.id)) { + for (const clear of getTransitiveSubBlockDependents(config.subBlocks, [anchorCfg.id])) { const dependent = configById.get(clear.subBlockId) if (!dependent?.id || !dependent.selectorKey) continue // Skip fields gated off by their `condition` - a selector under a now-inactive diff --git a/apps/sim/ee/workspace-forking/lib/remap/remap-references.ts b/apps/sim/ee/workspace-forking/lib/remap/remap-references.ts index 1a71e90e60b..ae83c7c6f15 100644 --- a/apps/sim/ee/workspace-forking/lib/remap/remap-references.ts +++ b/apps/sim/ee/workspace-forking/lib/remap/remap-references.ts @@ -10,13 +10,17 @@ import { type SubBlockRecord, } from '@/lib/workflows/persistence/remap-internal-ids' import { CREDENTIAL_SUBBLOCK_IDS } from '@/lib/workflows/persistence/utils' -import { getWorkflowSearchDependentClears } from '@/lib/workflows/search-replace/dependencies' import { getToolInputParamConfigs } from '@/lib/workflows/search-replace/indexer' import { getWorkflowSearchSubBlockResourceDefinition, parseWorkflowSearchSubBlockResources, type StructuredWorkflowSearchResourceKind, } from '@/lib/workflows/search-replace/resources/registry' +import { + getDependsOnFields, + getSubBlocksDependingOnChange, + getTransitiveSubBlockDependents, +} from '@/lib/workflows/subblocks/dependencies' import { buildCanonicalIndex, buildSubBlockValues, @@ -36,7 +40,6 @@ import { import type { ParsedStoredTool } from '@/lib/workflows/tool-input/types' import { getBlock } from '@/blocks/registry' import type { SubBlockConfig } from '@/blocks/types' -import { getDependsOnFields, getSubBlocksDependingOnChange } from '@/blocks/utils' import { collectForkFileUploadKeys, remapForkFileUploadValue, @@ -393,7 +396,7 @@ interface ToolBlockRemapOptions { * fields (handled by the callers / the workflow id map), not block params, so they * pass through here untouched. Returns a new tool object only when something changed. * After remapping, dependent params (via `dependsOn`) of any changed resource are - * cleared with the same {@link getWorkflowSearchDependentClears} walk search-replace + * cleared with the same {@link getTransitiveSubBlockDependents} walk search-replace * uses, so a child scoped to the old parent isn't left stale. */ export function remapToolBlockResources( @@ -592,7 +595,7 @@ export function remapToolBlockResources( const parentCfg = configBySubBlockId.get(subBlockId) const parentRemappedNonEmpty = isNonEmptyValue(readParam(parentCfg, subBlockId)) const parentCopied = parentRemappedNonEmpty && copyRemappedSubBlockIds.has(subBlockId) - for (const clear of getWorkflowSearchDependentClears(toolBlockSubBlocks, subBlockId)) { + for (const clear of getTransitiveSubBlockDependents(toolBlockSubBlocks, [subBlockId])) { const dependentCfg = configBySubBlockId.get(clear.subBlockId) // A verbatim manual-parent dependent is never cleared, even when reachable from a // second (remapped) parent. @@ -1136,7 +1139,7 @@ export function clearDependentsOnRemap( } } - // Same BFS as `getWorkflowSearchDependentClears`, with each preserved dependent's subtree + // Same BFS as `getTransitiveSubBlockDependents`, with each preserved dependent's subtree // pruned (skipping it keeps its own dependents - e.g. a tool's arguments - out of the clear // set). A dependent under an ACTIVE MANUAL parent is verbatim by policy (the manual value is // never remapped), so it is pruned the same way. diff --git a/apps/sim/hooks/use-collaborative-workflow.ts b/apps/sim/hooks/use-collaborative-workflow.ts index 87e37394c0e..75516648bc1 100644 --- a/apps/sim/hooks/use-collaborative-workflow.ts +++ b/apps/sim/hooks/use-collaborative-workflow.ts @@ -25,10 +25,10 @@ import { type WorkflowSearchSubflowFieldId, workflowSearchSubflowFieldMatchesExpected, } from '@/lib/workflows/search-replace/subflow-fields' +import { getSubBlocksDependingOnChange } from '@/lib/workflows/subblocks/dependencies' import { isSyntheticToolSubBlockId } from '@/lib/workflows/tool-input/synthetic-subblocks' import { useSocket } from '@/app/workspace/providers/socket-provider' import { getBlock } from '@/blocks' -import { getSubBlocksDependingOnChange } from '@/blocks/utils' import { invalidateDeploymentQueries } from '@/hooks/queries/deployments' import { useUndoRedo } from '@/hooks/use-undo-redo' import { diff --git a/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/operations.test.ts b/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/operations.test.ts index 666d5c4e973..30312b308e9 100644 --- a/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/operations.test.ts +++ b/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/operations.test.ts @@ -4,22 +4,23 @@ import { describe, expect, it, vi } from 'vitest' import { applyOperationsToWorkflowState } from './engine' -vi.mock('@/blocks/registry', () => ({ - getAllBlocks: () => [ - { +vi.mock('@/blocks/registry', () => { + const blocks: Record = { + condition: { type: 'condition', name: 'Condition', subBlocks: [{ id: 'conditions', type: 'condition-input' }], }, - { + agent: { type: 'agent', name: 'Agent', subBlocks: [ { id: 'systemPrompt', type: 'long-input' }, { id: 'model', type: 'combobox' }, + { id: 'tools', type: 'tool-input' }, ], }, - { + function: { type: 'function', name: 'Function', subBlocks: [ @@ -27,34 +28,54 @@ vi.mock('@/blocks/registry', () => ({ { id: 'language', type: 'dropdown' }, ], }, - ], - getBlock: (type: string) => { - const blocks: Record = { - condition: { - type: 'condition', - name: 'Condition', - subBlocks: [{ id: 'conditions', type: 'condition-input' }], - }, - agent: { - type: 'agent', - name: 'Agent', - subBlocks: [ - { id: 'systemPrompt', type: 'long-input' }, - { id: 'model', type: 'combobox' }, - ], - }, - function: { - type: 'function', - name: 'Function', - subBlocks: [ - { id: 'code', type: 'code' }, - { id: 'language', type: 'dropdown' }, - ], - }, - } - return blocks[type] || undefined - }, -})) + jira: { + type: 'jira', + name: 'Jira', + tools: { access: ['jira_get_issue'] }, + subBlocks: [ + { id: 'credential', type: 'oauth-input' }, + { + id: 'projectId', + type: 'project-selector', + canonicalParamId: 'projectId', + mode: 'basic', + dependsOn: ['credential'], + }, + { + id: 'manualProjectId', + type: 'short-input', + canonicalParamId: 'projectId', + mode: 'advanced', + dependsOn: ['credential'], + }, + { + id: 'issueKey', + type: 'file-selector', + canonicalParamId: 'issueKey', + mode: 'basic', + dependsOn: ['projectId'], + }, + { + id: 'manualIssueKey', + type: 'short-input', + canonicalParamId: 'issueKey', + mode: 'advanced', + dependsOn: ['projectId'], + }, + { + id: 'transitionId', + type: 'short-input', + dependsOn: ['issueKey'], + }, + ], + }, + } + + return { + getAllBlocks: () => Object.values(blocks), + getBlock: (type: string) => blocks[type], + } +}) vi.mock('@/lib/integrations/availability.server', () => ({ isIntegrationDeploymentAvailableForVisibility: () => true, @@ -189,6 +210,175 @@ function makeNestedLoopWorkflow() { } } +function makeDependentWorkflow() { + return { + blocks: { + 'jira-1': { + id: 'jira-1', + type: 'jira', + name: 'Jira 1', + position: { x: 0, y: 0 }, + enabled: true, + subBlocks: { + credential: { id: 'credential', type: 'oauth-input', value: 'credential-old' }, + projectId: { id: 'projectId', type: 'project-selector', value: 'PROJECT-OLD' }, + manualProjectId: { + id: 'manualProjectId', + type: 'short-input', + value: '', + }, + issueKey: { id: 'issueKey', type: 'file-selector', value: 'OLD-123' }, + manualIssueKey: { id: 'manualIssueKey', type: 'short-input', value: '' }, + transitionId: { id: 'transitionId', type: 'short-input', value: 'transition-old' }, + }, + outputs: {}, + data: { + canonicalModes: { + projectId: 'basic', + issueKey: 'basic', + }, + }, + }, + }, + edges: [], + loops: {}, + parallels: {}, + } +} + +describe('handleEditOperation dependent inputs', () => { + it('clears omitted descendants transitively when a parent changes', () => { + const { state } = applyOperationsToWorkflowState(makeDependentWorkflow(), [ + { + operation_type: 'edit', + block_id: 'jira-1', + params: { inputs: { projectId: 'PROJECT-NEW' } }, + }, + ]) + + expect(state.blocks['jira-1'].subBlocks.projectId.value).toBe('PROJECT-NEW') + expect(state.blocks['jira-1'].subBlocks.issueKey.value).toBe('') + expect(state.blocks['jira-1'].subBlocks.transitionId.value).toBe('') + }) + + it('preserves explicitly supplied descendants and clears only their omitted descendants', () => { + const { state } = applyOperationsToWorkflowState(makeDependentWorkflow(), [ + { + operation_type: 'edit', + block_id: 'jira-1', + params: { + inputs: { + projectId: 'PROJECT-NEW', + issueKey: 'NEW-456', + }, + }, + }, + ]) + + expect(state.blocks['jira-1'].subBlocks.projectId.value).toBe('PROJECT-NEW') + expect(state.blocks['jira-1'].subBlocks.issueKey.value).toBe('NEW-456') + expect(state.blocks['jira-1'].subBlocks.transitionId.value).toBe('') + }) + + it('does not clear descendants when the submitted parent is unchanged', () => { + const { state } = applyOperationsToWorkflowState(makeDependentWorkflow(), [ + { + operation_type: 'edit', + block_id: 'jira-1', + params: { inputs: { projectId: 'PROJECT-OLD' } }, + }, + ]) + + expect(state.blocks['jira-1'].subBlocks.issueKey.value).toBe('OLD-123') + expect(state.blocks['jira-1'].subBlocks.transitionId.value).toBe('transition-old') + }) + + it('uses canonical advanced inputs as dependency changes', () => { + const { state } = applyOperationsToWorkflowState(makeDependentWorkflow(), [ + { + operation_type: 'edit', + block_id: 'jira-1', + params: { inputs: { manualProjectId: 'PROJECT-MANUAL' } }, + }, + ]) + + expect(state.blocks['jira-1'].subBlocks.manualProjectId.value).toBe('PROJECT-MANUAL') + expect(state.blocks['jira-1'].data.canonicalModes.projectId).toBe('advanced') + expect(state.blocks['jira-1'].subBlocks.issueKey.value).toBe('') + expect(state.blocks['jira-1'].subBlocks.transitionId.value).toBe('') + }) + + it('clears active manual descendants when their authoring context changes', () => { + const workflow = makeDependentWorkflow() + const jira = workflow.blocks['jira-1'] + jira.subBlocks.projectId.value = '' + jira.subBlocks.manualProjectId.value = 'PROJECT-MANUAL-OLD' + jira.subBlocks.issueKey.value = '' + jira.subBlocks.manualIssueKey.value = 'OLD-123' + jira.data.canonicalModes.projectId = 'advanced' + jira.data.canonicalModes.issueKey = 'advanced' + + const { state } = applyOperationsToWorkflowState(workflow, [ + { + operation_type: 'edit', + block_id: 'jira-1', + params: { inputs: { credential: 'credential-new' } }, + }, + ]) + + expect(state.blocks['jira-1'].subBlocks.manualProjectId.value).toBe('') + expect(state.blocks['jira-1'].subBlocks.manualIssueKey.value).toBe('') + expect(state.blocks['jira-1'].subBlocks.transitionId.value).toBe('') + }) + + it('replaces nested agent tool params instead of retaining omitted dependents', () => { + const workflow = { + blocks: { + 'agent-1': { + id: 'agent-1', + type: 'agent', + name: 'Agent 1', + position: { x: 0, y: 0 }, + enabled: true, + subBlocks: { + tools: { + id: 'tools', + type: 'tool-input', + value: [ + { + type: 'jira', + params: { projectId: 'PROJECT-OLD', issueKey: 'OLD-123' }, + }, + ], + }, + }, + outputs: {}, + data: {}, + }, + }, + edges: [], + loops: {}, + parallels: {}, + } + + const { state } = applyOperationsToWorkflowState(workflow, [ + { + operation_type: 'edit', + block_id: 'agent-1', + params: { + inputs: { + tools: [{ type: 'jira', params: { projectId: 'PROJECT-NEW' } }], + }, + }, + }, + ]) + + expect(state.blocks['agent-1'].subBlocks.tools.value[0].params).toEqual({ + projectId: 'PROJECT-NEW', + }) + }) +}) + describe('handleEditOperation nestedNodes merge', () => { it('preserves existing child block IDs when editing a loop with nestedNodes', () => { const workflow = makeLoopWorkflow() diff --git a/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/operations.ts b/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/operations.ts index eda45aa2e6c..e2e8bb65f9a 100644 --- a/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/operations.ts +++ b/apps/sim/lib/copilot/tools/server/workflow/edit-workflow/operations.ts @@ -1,5 +1,9 @@ import { createLogger } from '@sim/logger' +import { isRecordLike } from '@sim/utils/object' +import { isEqual } from 'es-toolkit' import { isValidKey } from '@/lib/workflows/sanitization/key-validation' +import { getTransitiveSubBlockDependents } from '@/lib/workflows/subblocks/dependencies' +import { isNonEmptyValue } from '@/lib/workflows/subblocks/visibility' import { TriggerUtils } from '@/lib/workflows/triggers/triggers' import { getBlock } from '@/blocks/registry' import { normalizeName, RESERVED_BLOCK_NAMES } from '@/executor/constants' @@ -425,6 +429,14 @@ export function handleEditOperation(op: EditWorkflowOperation, ctx: OperationCon if (params?.inputs) { if (!block.subBlocks) block.subBlocks = {} + const previousSubBlockValues = new Map( + Object.entries(block.subBlocks).map(([key, subBlock]: [string, any]) => [ + key, + structuredClone(subBlock?.value), + ]) + ) + const explicitInputKeys = new Set() + // Validate inputs against block configuration const validationResult = validateInputsForBlock(block.type, params.inputs, block_id) validationErrors.push(...validationResult.errors) @@ -439,6 +451,7 @@ export function handleEditOperation(op: EditWorkflowOperation, ctx: OperationCon if (TRIGGER_RUNTIME_SUBBLOCK_IDS.includes(key)) { return } + explicitInputKeys.add(key) let sanitizedValue = normalizeSubblockValue(key, value) sanitizedValue = normalizeConditionRouterIds(block_id, key, sanitizedValue) @@ -467,12 +480,7 @@ export function handleEditOperation(op: EditWorkflowOperation, ctx: OperationCon } } else { const existingValue = block.subBlocks[key].value - const valuesEqual = - typeof existingValue === 'object' || typeof sanitizedValue === 'object' - ? JSON.stringify(existingValue) === JSON.stringify(sanitizedValue) - : existingValue === sanitizedValue - - if (!valuesEqual) { + if (!isEqual(existingValue, sanitizedValue)) { block.subBlocks[key].value = sanitizedValue } } @@ -481,9 +489,12 @@ export function handleEditOperation(op: EditWorkflowOperation, ctx: OperationCon if ( Object.hasOwn(params.inputs, 'triggerConfig') && block.subBlocks.triggerConfig && - typeof block.subBlocks.triggerConfig.value === 'object' + isRecordLike(block.subBlocks.triggerConfig.value) ) { applyTriggerConfigToBlockSubblocks(block, block.subBlocks.triggerConfig.value) + for (const key of Object.keys(block.subBlocks.triggerConfig.value)) { + explicitInputKeys.add(key) + } } // Update loop/parallel configuration in block.data (strict validation) @@ -538,11 +549,26 @@ export function handleEditOperation(op: EditWorkflowOperation, ctx: OperationCon const editBlockConfig = getBlock(block.type) if (editBlockConfig) { - updateCanonicalModesForInputs( - block, - Object.keys(validationResult.validInputs), - editBlockConfig - ) + updateCanonicalModesForInputs(block, [...explicitInputKeys], editBlockConfig) + + const changedInputKeys = editBlockConfig.subBlocks + .filter((subBlock) => { + const currentSubBlock = block.subBlocks[subBlock.id] + const existed = previousSubBlockValues.has(subBlock.id) + if (!existed || !currentSubBlock) return existed !== Boolean(currentSubBlock) + return !isEqual(previousSubBlockValues.get(subBlock.id), currentSubBlock.value) + }) + .map((subBlock) => subBlock.id) + + for (const { subBlockId } of getTransitiveSubBlockDependents( + editBlockConfig.subBlocks, + changedInputKeys + )) { + if (explicitInputKeys.has(subBlockId)) continue + const dependent = block.subBlocks[subBlockId] + if (!dependent || !isNonEmptyValue(dependent.value)) continue + dependent.value = '' + } } } diff --git a/apps/sim/lib/workflows/search-replace/dependencies.test.ts b/apps/sim/lib/workflows/search-replace/dependencies.test.ts deleted file mode 100644 index f1e2505fa75..00000000000 --- a/apps/sim/lib/workflows/search-replace/dependencies.test.ts +++ /dev/null @@ -1,24 +0,0 @@ -/** - * @vitest-environment node - */ -import { describe, expect, it } from 'vitest' -import { getWorkflowSearchDependentClears } from '@/lib/workflows/search-replace/dependencies' -import type { SubBlockConfig } from '@/blocks/types' - -describe('getWorkflowSearchDependentClears', () => { - it('returns transitive dependents without cycling', () => { - const subBlocks: SubBlockConfig[] = [ - { id: 'credential', title: 'Credential', type: 'oauth-input' }, - { id: 'project', title: 'Project', type: 'project-selector', dependsOn: ['credential'] }, - { id: 'issue', title: 'Issue', type: 'file-selector', dependsOn: ['project'] }, - { id: 'assignee', title: 'Assignee', type: 'user-selector', dependsOn: ['issue'] }, - { id: 'unrelated', title: 'Unrelated', type: 'short-input' }, - ] - - expect(getWorkflowSearchDependentClears(subBlocks, 'credential')).toEqual([ - { subBlockId: 'project', reason: 'project depends on credential' }, - { subBlockId: 'issue', reason: 'issue depends on project' }, - { subBlockId: 'assignee', reason: 'assignee depends on issue' }, - ]) - }) -}) diff --git a/apps/sim/lib/workflows/search-replace/dependencies.ts b/apps/sim/lib/workflows/search-replace/dependencies.ts deleted file mode 100644 index 5059ebc29f2..00000000000 --- a/apps/sim/lib/workflows/search-replace/dependencies.ts +++ /dev/null @@ -1,33 +0,0 @@ -import type { SubBlockConfig } from '@/blocks/types' -import { getSubBlocksDependingOnChange } from '@/blocks/utils' - -export interface DependentClear { - subBlockId: string - reason: string -} - -export function getWorkflowSearchDependentClears( - allSubBlocks: SubBlockConfig[], - changedSubBlockId: string -): DependentClear[] { - const clears: DependentClear[] = [] - const visited = new Set([changedSubBlockId]) - const queue = [changedSubBlockId] - - while (queue.length > 0) { - const currentSubBlockId = queue.shift() - if (!currentSubBlockId) continue - - for (const subBlock of getSubBlocksDependingOnChange(allSubBlocks, currentSubBlockId)) { - if (!subBlock.id || visited.has(subBlock.id)) continue - visited.add(subBlock.id) - clears.push({ - subBlockId: subBlock.id, - reason: `${subBlock.id} depends on ${currentSubBlockId}`, - }) - queue.push(subBlock.id) - } - } - - return clears -} diff --git a/apps/sim/lib/workflows/search-replace/indexer.ts b/apps/sim/lib/workflows/search-replace/indexer.ts index 25c2e3ca7b3..5fc1a9ffdfc 100644 --- a/apps/sim/lib/workflows/search-replace/indexer.ts +++ b/apps/sim/lib/workflows/search-replace/indexer.ts @@ -3,7 +3,6 @@ import { DEFAULT_SUBBLOCK_TYPE } from '@sim/workflow-persistence/subblocks' import type { SubBlockType } from '@sim/workflow-types/blocks' import { isWorkflowBlockProtected } from '@sim/workflow-types/workflow' import { COMPARISON_OPERATORS, LOGICAL_OPERATORS } from '@/lib/table/query-builder/constants' -import { getWorkflowSearchDependentClears } from '@/lib/workflows/search-replace/dependencies' import { getSearchableJsonStringLeaves, isSearchableJsonValueSubBlock, @@ -26,6 +25,7 @@ import type { } from '@/lib/workflows/search-replace/types' import { pathToKey, walkStringValues } from '@/lib/workflows/search-replace/value-walker' import { SELECTOR_CONTEXT_FIELDS } from '@/lib/workflows/subblocks/context' +import { getTransitiveSubBlockDependents } from '@/lib/workflows/subblocks/dependencies' import { resolveStoredToolName } from '@/lib/workflows/subblocks/display' import { buildCanonicalIndex, @@ -784,7 +784,7 @@ export function getToolInputParamConfigs({ ) const allToolSubBlocks = blockConfig?.subBlocks ?? subBlocksResult.subBlocks const getDependentValuePaths = (changedSubBlockId: string): WorkflowSearchValuePath[] => - getWorkflowSearchDependentClears(allToolSubBlocks, changedSubBlockId).map((clear) => [ + getTransitiveSubBlockDependents(allToolSubBlocks, [changedSubBlockId]).map((clear) => [ 'params', clear.subBlockId, ]) diff --git a/apps/sim/lib/workflows/subblocks/dependencies.test.ts b/apps/sim/lib/workflows/subblocks/dependencies.test.ts new file mode 100644 index 00000000000..0a8842ea976 --- /dev/null +++ b/apps/sim/lib/workflows/subblocks/dependencies.test.ts @@ -0,0 +1,137 @@ +/** + * @vitest-environment node + */ +import { describe, expect, it } from 'vitest' +import { + getDependsOnFields, + getSubBlocksDependingOnChange, + getTransitiveSubBlockDependents, +} from '@/lib/workflows/subblocks/dependencies' +import type { SubBlockConfig } from '@/blocks/types' + +describe('getDependsOnFields', () => { + it('returns an empty array when dependsOn is unset', () => { + expect(getDependsOnFields(undefined)).toEqual([]) + }) + + it('returns array dependencies unchanged', () => { + expect(getDependsOnFields(['credential', 'projectId'])).toEqual(['credential', 'projectId']) + }) + + it('flattens all and any dependencies', () => { + expect(getDependsOnFields({ all: ['credential'], any: ['teamId', 'manualTeamId'] })).toEqual([ + 'credential', + 'teamId', + 'manualTeamId', + ]) + }) +}) + +describe('getSubBlocksDependingOnChange', () => { + it('finds direct dependents of a changed subblock', () => { + const subBlocks: SubBlockConfig[] = [ + { id: 'provider', title: 'Provider', type: 'dropdown' }, + { id: 'model', title: 'Model', type: 'dropdown', dependsOn: ['provider'] }, + { id: 'prompt', title: 'Prompt', type: 'long-input' }, + ] + + expect( + getSubBlocksDependingOnChange(subBlocks, 'provider').map((subBlock) => subBlock.id) + ).toEqual(['model']) + }) + + it('matches dependents through canonical basic and advanced siblings', () => { + const subBlocks: SubBlockConfig[] = [ + { + id: 'channel', + title: 'Channel', + type: 'channel-selector', + canonicalParamId: 'channelId', + mode: 'basic', + }, + { + id: 'manualChannel', + title: 'Channel ID', + type: 'short-input', + canonicalParamId: 'channelId', + mode: 'advanced', + }, + { + id: 'messageId', + title: 'Message ID', + type: 'short-input', + dependsOn: ['channelId'], + }, + { + id: 'threadTs', + title: 'Thread Timestamp', + type: 'short-input', + dependsOn: ['otherField'], + }, + ] + + expect( + getSubBlocksDependingOnChange(subBlocks, 'manualChannel').map((subBlock) => subBlock.id) + ).toEqual(['messageId']) + expect( + getSubBlocksDependingOnChange(subBlocks, 'channel').map((subBlock) => subBlock.id) + ).toEqual(['messageId']) + }) + + it('matches object-form dependencies when any listed dependency changes', () => { + const subBlocks: SubBlockConfig[] = [ + { id: 'credential', title: 'Credential', type: 'oauth-input' }, + { id: 'teamId', title: 'Team', type: 'short-input' }, + { + id: 'projectId', + title: 'Project', + type: 'short-input', + dependsOn: { all: ['credential'], any: ['teamId'] }, + }, + ] + + expect( + getSubBlocksDependingOnChange(subBlocks, 'credential').map((subBlock) => subBlock.id) + ).toEqual(['projectId']) + expect( + getSubBlocksDependingOnChange(subBlocks, 'teamId').map((subBlock) => subBlock.id) + ).toEqual(['projectId']) + }) +}) + +describe('getTransitiveSubBlockDependents', () => { + it('returns transitive dependents without cycling', () => { + const subBlocks: SubBlockConfig[] = [ + { id: 'credential', title: 'Credential', type: 'oauth-input' }, + { id: 'project', title: 'Project', type: 'project-selector', dependsOn: ['credential'] }, + { id: 'issue', title: 'Issue', type: 'file-selector', dependsOn: ['project'] }, + { id: 'assignee', title: 'Assignee', type: 'user-selector', dependsOn: ['issue'] }, + { id: 'unrelated', title: 'Unrelated', type: 'short-input' }, + ] + + expect(getTransitiveSubBlockDependents(subBlocks, ['credential'])).toEqual([ + { subBlockId: 'project', reason: 'project depends on credential' }, + { subBlockId: 'issue', reason: 'issue depends on project' }, + { subBlockId: 'assignee', reason: 'assignee depends on issue' }, + ]) + }) + + it('walks multiple changed roots once', () => { + const subBlocks: SubBlockConfig[] = [ + { id: 'credential', title: 'Credential', type: 'oauth-input' }, + { id: 'domain', title: 'Domain', type: 'short-input' }, + { + id: 'project', + title: 'Project', + type: 'project-selector', + dependsOn: ['credential', 'domain'], + }, + { id: 'issue', title: 'Issue', type: 'file-selector', dependsOn: ['project'] }, + ] + + expect(getTransitiveSubBlockDependents(subBlocks, ['credential', 'domain'])).toEqual([ + { subBlockId: 'project', reason: 'project depends on credential' }, + { subBlockId: 'issue', reason: 'issue depends on project' }, + ]) + }) +}) diff --git a/apps/sim/lib/workflows/subblocks/dependencies.ts b/apps/sim/lib/workflows/subblocks/dependencies.ts new file mode 100644 index 00000000000..59f2030c0b9 --- /dev/null +++ b/apps/sim/lib/workflows/subblocks/dependencies.ts @@ -0,0 +1,66 @@ +import { buildCanonicalIndex } from '@/lib/workflows/subblocks/visibility' +import type { SubBlockConfig } from '@/blocks/types' + +export interface DependentSubBlock { + subBlockId: string + reason: string +} + +/** Flattens array and all/any dependency declarations into their referenced field IDs. */ +export function getDependsOnFields(dependsOn: SubBlockConfig['dependsOn']): string[] { + if (!dependsOn) return [] + if (Array.isArray(dependsOn)) return dependsOn + return [...(dependsOn.all || []), ...(dependsOn.any || [])] +} + +/** Finds direct dependents while treating canonical basic/advanced siblings as one field. */ +export function getSubBlocksDependingOnChange( + allSubBlocks: SubBlockConfig[], + changedSubBlockId: string +): SubBlockConfig[] { + const canonicalIndex = buildCanonicalIndex(allSubBlocks) + const canonicalId = canonicalIndex.canonicalIdBySubBlockId[changedSubBlockId] + const group = canonicalId ? canonicalIndex.groupsById[canonicalId] : undefined + const changedFields = new Set([changedSubBlockId]) + + if (canonicalId) changedFields.add(canonicalId) + if (group?.basicId) changedFields.add(group.basicId) + for (const advancedId of group?.advancedIds || []) { + changedFields.add(advancedId) + } + + return allSubBlocks.filter((subBlock) => + getDependsOnFields(subBlock.dependsOn).some((field) => changedFields.has(field)) + ) +} + +/** + * Returns every transitive `dependsOn` descendant of the changed subblocks. + * Canonical basic/advanced siblings are treated as one logical field by the + * shared block dependency resolver. + */ +export function getTransitiveSubBlockDependents( + allSubBlocks: SubBlockConfig[], + changedSubBlockIds: Iterable +): DependentSubBlock[] { + const dependents: DependentSubBlock[] = [] + const visited = new Set(changedSubBlockIds) + const queue = [...visited] + + while (queue.length > 0) { + const currentSubBlockId = queue.shift() + if (!currentSubBlockId) continue + + for (const subBlock of getSubBlocksDependingOnChange(allSubBlocks, currentSubBlockId)) { + if (!subBlock.id || visited.has(subBlock.id)) continue + visited.add(subBlock.id) + dependents.push({ + subBlockId: subBlock.id, + reason: `${subBlock.id} depends on ${currentSubBlockId}`, + }) + queue.push(subBlock.id) + } + } + + return dependents +}