Skip to content

Commit a2828a3

Browse files
committed
fix webhook stability issues + drift detection removal
1 parent 8a968e4 commit a2828a3

19 files changed

Lines changed: 654 additions & 120 deletions

File tree

apps/sim/app/api/workspaces/[id]/fork/diff/route.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,10 @@ import { getSession } from '@/lib/auth'
66
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
77
import { loadSourceDeployedStates } from '@/lib/workspaces/fork/copy/deploy-bridge'
88
import { assertCanPromote } from '@/lib/workspaces/fork/lineage/authz'
9+
import { loadForkBlockMap } from '@/lib/workspaces/fork/mapping/block-map-store'
910
import { collectForkDependentReconfigs } from '@/lib/workspaces/fork/mapping/dependent-reconfigs'
1011
import { computeForkPromotePlan } from '@/lib/workspaces/fork/promote/promote-plan'
12+
import { buildForkBlockIdResolver } from '@/lib/workspaces/fork/remap/block-identity'
1113

1214
export const GET = withRouteHandler(
1315
async (req: NextRequest, context: { params: Promise<{ id: string }> }) => {
@@ -36,6 +38,13 @@ export const GET = withRouteHandler(
3638
sourceStates,
3739
})
3840

41+
// Resolve dependent-reconfig target block ids through the SAME persisted block map the
42+
// sync will use, so a re-pick the modal keys by target block id lands on the block the
43+
// promote actually writes (on push that's the parent's original id, not a derived one).
44+
const sourceIsParent = auth.sourceWorkspaceId === auth.edge.parentWorkspaceId
45+
const blockMap = await loadForkBlockMap(db, auth.edge.childWorkspaceId)
46+
const resolveBlockId = buildForkBlockIdResolver(sourceIsParent, blockMap)
47+
3948
const toRef = (reference: (typeof plan.unmappedRequired)[number]) => ({
4049
kind: reference.kind,
4150
sourceId: reference.sourceId,
@@ -81,8 +90,7 @@ export const GET = withRouteHandler(
8190
unmappedOptional: plan.unmappedOptional.map(toRef),
8291
mcpReauthServerIds: plan.mcpReauthServerIds,
8392
inlineSecretSources: plan.inlineSecretSources,
84-
dependentReconfigs: collectForkDependentReconfigs(plan.items, sourceStates),
85-
drift: plan.drift,
93+
dependentReconfigs: collectForkDependentReconfigs(plan.items, sourceStates, resolveBlockId),
8694
})
8795
}
8896
)

apps/sim/app/api/workspaces/[id]/fork/promote/route.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ export const POST = withRouteHandler(
2525
const parsed = await parseRequest(promoteForkContract, req, context)
2626
if (!parsed.success) return parsed.response
2727
const { id } = parsed.data.params
28-
const { otherWorkspaceId, direction, force, dependentOverrides } = parsed.data.body
28+
const { otherWorkspaceId, direction, dependentOverrides } = parsed.data.body
2929

3030
const auth = await assertCanPromote(id, otherWorkspaceId, direction, session.user.id)
3131

@@ -34,7 +34,6 @@ export const POST = withRouteHandler(
3434
sourceWorkspaceId: auth.sourceWorkspaceId,
3535
targetWorkspaceId: auth.targetWorkspaceId,
3636
direction,
37-
force,
3837
userId: session.user.id,
3938
dependentOverrides,
4039
requestId,
@@ -50,7 +49,6 @@ export const POST = withRouteHandler(
5049
unmappedRequired: result.unmappedRequired,
5150
needsConfiguration: result.needsConfiguration,
5251
clearedOptional: result.clearedOptional,
53-
drift: result.drift,
5452
}
5553

5654
if (result.blocked) {

apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workspace-header/components/promote-workspace-modal/promote-workspace-modal.tsx

Lines changed: 14 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ interface EdgeOption {
128128
* picks a direction and lists each resource kind's mapping status, then Sync.
129129
* "Edit mappings" steps through every kind (Back/Next, each source a
130130
* settings-style section + full-width target) to set or review targets before
131-
* landing back on Sync - with a force-confirm on drift. The durable record of
131+
* landing back on Sync - which always confirms the overwrite first. The durable record of
132132
* every sync is the Activity log in Manage Forks, so this modal just closes on
133133
* success.
134134
*/
@@ -173,7 +173,7 @@ export function PromoteWorkspaceModal({
173173
// Wizard step: 0 is the overview; 1..N edit one resource kind each, entered via
174174
// "Edit mappings". Backing out of step 1 returns to the overview.
175175
const [step, setStep] = useState(0)
176-
const [confirmDriftOpen, setConfirmDriftOpen] = useState(false)
176+
const [confirmSyncOpen, setConfirmSyncOpen] = useState(false)
177177
const [submitting, setSubmitting] = useState(false)
178178

179179
useEffect(() => {
@@ -289,7 +289,7 @@ export function PromoteWorkspaceModal({
289289
(diff.data?.mcpReauthServerIds.length ?? 0) > 0 ||
290290
(diff.data?.inlineSecretSources.length ?? 0) > 0
291291

292-
const runPromote = async (force: boolean) => {
292+
const runPromote = async () => {
293293
if (!otherWorkspaceId) return
294294
setSubmitting(true)
295295
try {
@@ -328,7 +328,6 @@ export function PromoteWorkspaceModal({
328328
body: {
329329
otherWorkspaceId,
330330
direction,
331-
force,
332331
...(dependentOverrides.length > 0 ? { dependentOverrides } : {}),
333332
},
334333
})
@@ -338,10 +337,6 @@ export function PromoteWorkspaceModal({
338337
toast.error('Map all required credentials and secrets first')
339338
return
340339
}
341-
if (result.drift) {
342-
setConfirmDriftOpen(true)
343-
return
344-
}
345340
toast.error('Sync did not complete')
346341
return
347342
}
@@ -638,7 +633,7 @@ export function PromoteWorkspaceModal({
638633
? { label: 'Next', onClick: () => setStep(safeStep + 1), disabled: submitting }
639634
: {
640635
label: submitting ? 'Working...' : 'Sync',
641-
onClick: () => void runPromote(false),
636+
onClick: () => setConfirmSyncOpen(true),
642637
disabled: syncDisabled,
643638
disabledTooltip: !requiredComplete
644639
? 'Map all required secrets first'
@@ -651,20 +646,20 @@ export function PromoteWorkspaceModal({
651646
</ChipModal>
652647

653648
<ChipConfirmModal
654-
open={confirmDriftOpen}
655-
onOpenChange={setConfirmDriftOpen}
656-
srTitle='Force sync'
657-
title='Target has changed'
649+
open={confirmSyncOpen}
650+
onOpenChange={setConfirmSyncOpen}
651+
srTitle='Sync workspace'
652+
title='Overwrite target workspace'
658653
text={[
659-
'The target workspace was modified since the last sync. Force syncing will ',
660-
{ text: 'overwrite those changes', bold: true },
661-
'. Continue?',
654+
'The target may have been modified since the last sync. Syncing will ',
655+
{ text: 'overwrite any changes', bold: true },
656+
' there. Continue?',
662657
]}
663658
confirm={{
664-
label: 'Force sync',
659+
label: 'Sync',
665660
onClick: () => {
666-
setConfirmDriftOpen(false)
667-
void runPromote(true)
661+
setConfirmSyncOpen(false)
662+
void runPromote()
668663
},
669664
pending: submitting,
670665
pendingLabel: 'Syncing...',

apps/sim/lib/api/contracts/workspace-fork.ts

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -283,7 +283,6 @@ export const getForkDiffContract = defineRouteContract({
283283
inlineSecretSources: z.array(z.string()),
284284
/** Configured selector fields per parent (credential/KB), for the pre-sync reconfigure. */
285285
dependentReconfigs: z.array(forkDependentReconfigSchema),
286-
drift: z.boolean(),
287286
}),
288287
},
289288
})
@@ -320,7 +319,6 @@ export type ForkDependentOverride = z.input<typeof forkDependentOverrideSchema>
320319
export const promoteForkBodySchema = z.object({
321320
otherWorkspaceId: workspaceIdSchema,
322321
direction: forkDirectionSchema,
323-
force: z.boolean().optional().default(false),
324322
/** Pre-sync re-picks for dependent fields whose credential the user swapped. */
325323
dependentOverrides: z.array(forkDependentOverrideSchema).max(2000).optional(),
326324
})
@@ -343,7 +341,6 @@ export const promoteForkContract = defineRouteContract({
343341
needsConfiguration: z.array(forkNeedsConfigurationSchema),
344342
/** Workflows whose optional dependent fields a swap cleared (surfaced, not gated). */
345343
clearedOptional: z.array(forkNeedsConfigurationSchema),
346-
drift: z.boolean(),
347344
}),
348345
},
349346
})

apps/sim/lib/workspaces/fork/copy/copy-workflows.ts

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,10 @@ import {
1212
sanitizeSubBlocksForDuplicate,
1313
} from '@/lib/workflows/persistence/remap-internal-ids'
1414
import { saveWorkflowToNormalizedTables } from '@/lib/workflows/persistence/utils'
15-
import { deriveForkBlockId } from '@/lib/workspaces/fork/remap/block-identity'
15+
import {
16+
deriveForkBlockId,
17+
type ForkBlockIdResolver,
18+
} from '@/lib/workspaces/fork/remap/block-identity'
1619
import {
1720
applyDependentOverrides,
1821
collectClearedDependents,
@@ -257,6 +260,11 @@ export interface CopyWorkflowResult {
257260
* surfaces optional ones so a cleared filter never broadens behavior silently.
258261
*/
259262
clearedDependents: NeedsConfigurationField[]
263+
/**
264+
* Source block id -> assigned target block id, so the caller can persist the
265+
* block-identity pairs (see `recordForkBlockPairs`) that keep promotes reversible.
266+
*/
267+
blockIdMapping: Map<string, string>
260268
}
261269

262270
export interface CopyWorkflowStateParams {
@@ -299,6 +307,13 @@ export interface CopyWorkflowStateParams {
299307
* query per workflow inside the tx. Build once per copy loop via {@link loadWorkflowNameRegistry}.
300308
*/
301309
nameRegistry: WorkflowNameRegistry
310+
/**
311+
* Resolve each source block to its target block id, reusing the persisted counterpart
312+
* when one exists so a push keeps the parent's original block ids (and webhook URLs)
313+
* instead of re-deriving them (see {@link buildForkBlockIdResolver}). Omitted on fork
314+
* creation, where every id is derived fresh.
315+
*/
316+
resolveBlockId?: ForkBlockIdResolver
302317
requestId?: string
303318
}
304319

@@ -330,6 +345,7 @@ export async function copyWorkflowStateIntoTarget(
330345
targetCurrentBlocks,
331346
dependentOverrides,
332347
nameRegistry,
348+
resolveBlockId,
333349
requestId = 'unknown',
334350
} = params
335351

@@ -345,7 +361,12 @@ export async function copyWorkflowStateIntoTarget(
345361

346362
const blockIdMapping = new Map<string, string>()
347363
for (const oldBlockId of Object.keys(sourceState.blocks)) {
348-
blockIdMapping.set(oldBlockId, deriveForkBlockId(targetWorkflowId, oldBlockId))
364+
blockIdMapping.set(
365+
oldBlockId,
366+
resolveBlockId
367+
? resolveBlockId(targetWorkflowId, oldBlockId)
368+
: deriveForkBlockId(targetWorkflowId, oldBlockId)
369+
)
349370
}
350371

351372
const newBlocks: Record<string, BlockState> = {}
@@ -526,5 +547,6 @@ export async function copyWorkflowStateIntoTarget(
526547
edgesCount: newEdges.length,
527548
subflowsCount: Object.keys(newLoops).length + Object.keys(newParallels).length,
528549
clearedDependents,
550+
blockIdMapping,
529551
}
530552
}

apps/sim/lib/workspaces/fork/create-fork.ts

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,11 @@ import {
3030
} from '@/lib/workspaces/fork/copy/copy-workflows'
3131
import { loadSourceDeployedStates } from '@/lib/workspaces/fork/copy/deploy-bridge'
3232
import { setForkLockTimeout } from '@/lib/workspaces/fork/lineage/lineage'
33+
import {
34+
type ForkBlockPair,
35+
reconcileForkBlockPairs,
36+
toForkBlockPairs,
37+
} from '@/lib/workspaces/fork/mapping/block-map-store'
3338
import {
3439
type ForkMappingUpsert,
3540
type ForkResourceType,
@@ -212,12 +217,18 @@ export async function createFork(params: CreateForkParams): Promise<CreateForkRe
212217
const nameRegistry = await loadWorkflowNameRegistry(tx, childWorkspaceId)
213218

214219
let workflowsCopied = 0
220+
// Seed the block-identity map (parent block -> derived child block) so a later push of
221+
// this fork resolves each child block back to the parent's ORIGINAL id instead of
222+
// re-deriving and re-keying the parent's webhook URLs.
223+
const blockPairs: ForkBlockPair[] = []
224+
const sourceWorkflowIds: string[] = []
215225
for (const wf of deployedWorkflows) {
216226
const sourceState = sourceStates.get(wf.id)
217227
if (!sourceState) continue
218-
await copyWorkflowStateIntoTarget({
228+
const targetWorkflowId = workflowIdMap.get(wf.id)!
229+
const copyResult = await copyWorkflowStateIntoTarget({
219230
tx,
220-
targetWorkflowId: workflowIdMap.get(wf.id)!,
231+
targetWorkflowId,
221232
targetWorkspaceId: childWorkspaceId,
222233
userId,
223234
mode: 'create',
@@ -235,9 +246,13 @@ export async function createFork(params: CreateForkParams): Promise<CreateForkRe
235246
nameRegistry,
236247
requestId,
237248
})
249+
// Creation copies parent -> child, so the source side is the parent.
250+
blockPairs.push(...toForkBlockPairs(copyResult.blockIdMapping, true, wf.id, targetWorkflowId))
251+
sourceWorkflowIds.push(wf.id)
238252
workflowsCopied += 1
239253
forkedWorkflowNames.push(wf.name)
240254
}
255+
await reconcileForkBlockPairs(tx, childWorkspaceId, true, sourceWorkflowIds, blockPairs)
241256

242257
// A fork carries only DEPLOYED workflows. When the source has none (e.g. it was
243258
// itself just forked and never redeployed), seed a default workflow so the child
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { describe, expect, it } from 'vitest'
5+
import { toForkBlockPairs } from '@/lib/workspaces/fork/mapping/block-map-store'
6+
7+
describe('toForkBlockPairs', () => {
8+
const mapping = new Map([
9+
['src-1', 'tgt-1'],
10+
['src-2', 'tgt-2'],
11+
])
12+
13+
it('orients source->target as parent->child on pull/create (source = parent)', () => {
14+
expect(toForkBlockPairs(mapping, true, 'wf-parent', 'wf-child')).toEqual([
15+
{
16+
parentWorkflowId: 'wf-parent',
17+
childWorkflowId: 'wf-child',
18+
parentBlockId: 'src-1',
19+
childBlockId: 'tgt-1',
20+
},
21+
{
22+
parentWorkflowId: 'wf-parent',
23+
childWorkflowId: 'wf-child',
24+
parentBlockId: 'src-2',
25+
childBlockId: 'tgt-2',
26+
},
27+
])
28+
})
29+
30+
it('orients source->target as child->parent on push (source = child)', () => {
31+
expect(toForkBlockPairs(mapping, false, 'wf-child', 'wf-parent')).toEqual([
32+
{
33+
parentWorkflowId: 'wf-parent',
34+
childWorkflowId: 'wf-child',
35+
parentBlockId: 'tgt-1',
36+
childBlockId: 'src-1',
37+
},
38+
{
39+
parentWorkflowId: 'wf-parent',
40+
childWorkflowId: 'wf-child',
41+
parentBlockId: 'tgt-2',
42+
childBlockId: 'src-2',
43+
},
44+
])
45+
})
46+
47+
it('returns an empty list for an empty mapping', () => {
48+
expect(toForkBlockPairs(new Map(), true, 'wf-parent', 'wf-child')).toEqual([])
49+
})
50+
})

0 commit comments

Comments
 (0)