diff --git a/apps/sim/app/api/workspaces/[id]/fork/diff/route.ts b/apps/sim/app/api/workspaces/[id]/fork/diff/route.ts index caf75870d29..72b0d7dd435 100644 --- a/apps/sim/app/api/workspaces/[id]/fork/diff/route.ts +++ b/apps/sim/app/api/workspaces/[id]/fork/diff/route.ts @@ -10,6 +10,7 @@ import { loadTargetDraftSubBlocks } from '@/ee/workspace-forking/lib/copy/copy-w import { listForkExcludedDeployedWorkflows, loadSourceDeployedStates, + loadTargetWebhookPathsByBlock, } from '@/ee/workspace-forking/lib/copy/deploy-bridge' import { assertCanPromote } from '@/ee/workspace-forking/lib/lineage/authz' import { loadForkBlockMap } from '@/ee/workspace-forking/lib/mapping/block-map-store' @@ -27,6 +28,10 @@ import { collectForkClearedRefCandidates, } from '@/ee/workspace-forking/lib/promote/cleared-refs' import { computeForkPromotePlan } from '@/ee/workspace-forking/lib/promote/promote-plan' +import { + buildForkTriggerPlan, + resolveForkTriggerPaths, +} from '@/ee/workspace-forking/lib/promote/trigger-urls' import { buildForkBlockIdResolver } from '@/ee/workspace-forking/lib/remap/block-identity' import { readTargetDraftDependentValue } from '@/ee/workspace-forking/lib/remap/remap-references' @@ -173,6 +178,36 @@ export const GET = withRouteHandler( }) ) + // Trigger URLs this sync decides in the target - the "we had to re-paste the Slack Request + // URL again" case, surfaced as an editable pairing before the overwrite instead of discovered + // after it. The preview reports the plan's DEFAULT resolution; the user's picks ride the + // promote call, where the same plan is rebuilt and validated against them. + const triggerPlan = buildForkTriggerPlan({ + items: plan.items, + sourceStates, + resolveBlockId, + targetWebhooks: await loadTargetWebhookPathsByBlock(db, allTargetIds), + }) + const { changes: triggerUrlChanges } = resolveForkTriggerPaths(triggerPlan) + // Every trigger that HAS a public URL, plus every one whose URL is up for decision - not just + // the decisions, so the section reads as a standing statement of each URL rather than an alert. + // + // A trigger with neither is deliberately absent: whether a block will serve a URL at all is + // only knowable from its webhook row, and a schedule / chat / manual / poller trigger never + // gets one. Claiming "gets a new URL" for those would be a straight lie, and no declarative + // flag separates them - `polling` is set on 10 of the trigger defs, while `webhook` is set on + // 345 including `slack_oauth`, which routes by `routingKey` with a NULL path. + const triggerMappings = triggerPlan.slots + .filter((slot) => slot.ownPath !== null || slot.adoptablePaths.length > 0) + .map((slot) => ({ + sourceBlockId: slot.sourceBlockId, + blockName: slot.blockName, + workflowName: slot.workflowName, + ownPath: slot.ownPath, + adoptablePaths: slot.adoptablePaths, + defaultAdoptPath: slot.defaultAdoptPath, + })) + const toRef = (reference: (typeof plan.unmappedRequired)[number]) => ({ kind: reference.kind, sourceId: reference.sourceId, @@ -224,6 +259,8 @@ export const GET = withRouteHandler( resourceUsages: collectForkResourceUsages(plan.items, sourceStates), copyableUnmapped: plan.copyableUnmapped, clearedRefs, + triggerUrlChanges, + triggerMappings, }) } ) diff --git a/apps/sim/app/api/workspaces/[id]/fork/promote/route.ts b/apps/sim/app/api/workspaces/[id]/fork/promote/route.ts index cbf6c0fb23b..a06dd6300f9 100644 --- a/apps/sim/app/api/workspaces/[id]/fork/promote/route.ts +++ b/apps/sim/app/api/workspaces/[id]/fork/promote/route.ts @@ -25,7 +25,14 @@ export const POST = withRouteHandler( const parsed = await parseRequest(promoteForkContract, req, context) if (!parsed.success) return parsed.response const { id } = parsed.data.params - const { otherWorkspaceId, direction, dependentValues, copyResources } = parsed.data.body + const { + otherWorkspaceId, + direction, + dependentValues, + copyResources, + dropReferences, + triggerMappings, + } = parsed.data.body const auth = await assertCanPromote(id, otherWorkspaceId, direction, session.user.id) @@ -38,6 +45,8 @@ export const POST = withRouteHandler( actorName: session.user.name ?? undefined, dependentValues, copyResources, + dropReferences, + triggerMappings, requestId, }) @@ -52,6 +61,8 @@ export const POST = withRouteHandler( blockers: result.blockers, needsConfiguration: result.needsConfiguration, clearedOptional: result.clearedOptional, + droppedReferences: result.droppedReferences, + triggerUrlChanges: result.triggerUrlChanges, } if (result.blocked) { @@ -91,7 +102,9 @@ export const POST = withRouteHandler( status: result.deployFailed > 0 || result.needsConfiguration.length > 0 || - result.clearedOptional.length > 0 + result.clearedOptional.length > 0 || + result.droppedReferences.length > 0 || + result.triggerUrlChanges.length > 0 ? 'completed_with_warnings' : 'completed', message: direction === 'pull' ? `Pulled from "${otherName}"` : `Pushed to "${otherName}"`, @@ -110,6 +123,8 @@ export const POST = withRouteHandler( archivedNames: result.archivedNames, needsConfiguration: result.needsConfiguration, clearedOptional: result.clearedOptional, + droppedReferences: result.droppedReferences.length, + triggerUrlChanges: result.triggerUrlChanges.length, }, }).catch((error) => logger.error(`[${requestId}] Failed to record sync activity`, { diff --git a/apps/sim/ee/workspace-forking/components/fork-sync/cleared-refs-list.ts b/apps/sim/ee/workspace-forking/components/fork-sync/cleared-refs-list.ts index d80da2d03f0..0e928112356 100644 --- a/apps/sim/ee/workspace-forking/components/fork-sync/cleared-refs-list.ts +++ b/apps/sim/ee/workspace-forking/components/fork-sync/cleared-refs-list.ts @@ -58,14 +58,20 @@ export function splitForkClearedRefs(visibleRefs: ForkClearedRef[]): { return { blockers, informational } } -/** Human label per blocker kind for the resolution copy (singular, lowercase mid-sentence). */ -const BLOCKER_KIND_LABEL: Record = { +/** + * Human label per remap kind for the resolution copy (singular, lowercase mid-sentence). Shared + * with the Mappings section's source-deleted note so both phrase the same resolution identically. + * `credential` is reachable only from a mapping entry - credentials gate through the required + * check, never through the cleared-ref blockers. + */ +export const FORK_RESOURCE_KIND_LABEL: Record = { table: 'table', 'knowledge-base': 'knowledge base', file: 'file', 'custom-tool': 'custom tool', skill: 'skill', 'mcp-server': 'MCP server', + credential: 'credential', } /** @@ -79,7 +85,7 @@ export function forkBlockerResolution(ref: ForkClearedRef): string | null { case 'unmapped-copyable': return 'map it to a target or select it for copy' case 'source-deleted': - return `deleted in the source — map it to an existing ${BLOCKER_KIND_LABEL[ref.kind] ?? 'resource'} in the target` + return `deleted in the source — map it to an existing ${FORK_RESOURCE_KIND_LABEL[ref.kind] ?? 'resource'} in the target` case 'workflow-missing': return `deploy "${ref.sourceLabel}" in the source or remove the reference` } diff --git a/apps/sim/ee/workspace-forking/components/fork-sync/copy-reconciliation.ts b/apps/sim/ee/workspace-forking/components/fork-sync/copy-reconciliation.ts index 1cb90b2bc8a..f1896d9d19c 100644 --- a/apps/sim/ee/workspace-forking/components/fork-sync/copy-reconciliation.ts +++ b/apps/sim/ee/workspace-forking/components/fork-sync/copy-reconciliation.ts @@ -83,38 +83,39 @@ export function forkParentResolution( } /** - * Whether every required reference is satisfied - it has a mapping target OR is selected for copy. - * The server accepts a copy as resolving a required ref (promote.ts `willResolve`), so the client - * gate must too. No double-count: a mapped copyable is excluded from the copy candidates, so the two - * branches are mutually exclusive. + * Whether every required reference is satisfied - it has a mapping target, or its key is in + * `satisfiedKeys` (selected for copy, or acknowledged as a dropped source-deleted reference). + * The server accepts both as resolving a required ref, so the client gate must too. No + * double-count: a mapped copyable is excluded from the copy candidates, and a droppable reference + * is source-deleted, so it has no copy candidate either. */ export function isForkRequiredComplete( entries: ForkMappingEntry[], targets: Record, - copyingKeys: ReadonlySet + satisfiedKeys: ReadonlySet ): boolean { return entries.every( (entry) => !entry.required || effectiveForkTarget(entry, targets) !== '' || - copyingKeys.has(forkRefKey(entry)) + satisfiedKeys.has(forkRefKey(entry)) ) } /** - * Whether any reference in a kind is required AND still unmapped AND not selected for copy - drives - * the mapping summary's amber "pending" badge. Mirrors {@link isForkRequiredComplete}'s satisfied rule. + * Whether any reference in a kind is required AND still unmapped AND not satisfied another way - + * drives the mapping summary's amber "pending" badge. Mirrors {@link isForkRequiredComplete}. */ export function forkRequiredPending( items: ForkMappingEntry[], targets: Record, - copyingKeys: ReadonlySet + satisfiedKeys: ReadonlySet ): boolean { return items.some( (entry) => entry.required && effectiveForkTarget(entry, targets) === '' && - !copyingKeys.has(forkRefKey(entry)) + !satisfiedKeys.has(forkRefKey(entry)) ) } 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 0423b05b3f8..a3ab75a55d2 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 @@ -4,6 +4,7 @@ import { type Dispatch, Fragment, type SetStateAction, useMemo, useState } from import { Badge, ChevronDown, + Chip, ChipCombobox, ChipSwitch, CollapsibleCard, @@ -18,6 +19,7 @@ import type { ForkDependentReconfig, ForkMappingEntry, ForkResourceUsage, + ForkTriggerMapping, } from '@/lib/api/contracts/workspace-fork' import { SettingsEmptyState } from '@/app/workspace/[workspaceId]/settings/components/settings-empty-state' import { SettingsSection } from '@/app/workspace/[workspaceId]/settings/components/settings-section/settings-section' @@ -25,7 +27,10 @@ import { FileKindRow, ResourceKindRow, } from '@/ee/workspace-forking/components/fork-resource-picker/fork-resource-picker' -import { forkBlockerResolution } from '@/ee/workspace-forking/components/fork-sync/cleared-refs-list' +import { + FORK_RESOURCE_KIND_LABEL, + forkBlockerResolution, +} from '@/ee/workspace-forking/components/fork-sync/cleared-refs-list' import { forkRefKey } from '@/ee/workspace-forking/components/fork-sync/copy-reconciliation' import { DependentFieldSelector } from '@/ee/workspace-forking/components/fork-sync/dependent-field-selector' import { @@ -39,7 +44,9 @@ import type { ForkSyncController, } from '@/ee/workspace-forking/components/fork-sync/use-fork-sync' import type { ForkDirection } from '@/ee/workspace-forking/hooks/workspace-fork' +import { forkSyncBlockerReasonFor } from '@/ee/workspace-forking/lib/promote/sync-blockers' import type { SelectorKey } from '@/hooks/selectors/types' +import { buildWebhookTriggerUrl } from '@/triggers/webhook-url' /** * Copyable kinds as expandable rows in the "Copy resources" section, ordered + labeled to match @@ -65,6 +72,12 @@ const COPYABLE_KIND_SECTIONS: ReadonlyArray<{ */ const NEW_COPY_VALUE = '__new_copy__' +/** + * Sentinel option value for "New URL" - the trigger mints a fresh public URL instead of taking + * over a retiring one. Sent as `adoptPath: null`. + */ +const NEW_TRIGGER_URL_VALUE = '__new_trigger_url__' + /** Fixed target-picker width so every mapping row's control lines up as one column (mirrors General). */ const MAPPING_TARGET_TRIGGER_CLASS = 'w-[240px] flex-shrink-0' @@ -390,6 +403,13 @@ function MappingEntry({ controller, group, entry }: MappingEntryProps) { /> + {entry.sourceDeleted ? ( +

+ Deleted in the source — its name can't be shown. Map it to an existing{' '} + {FORK_RESOURCE_KIND_LABEL[entry.kind] ?? 'resource'} in the target, or fix the reference + in the source and redeploy. +

+ ) : null} {entry.candidatesTruncated ? (

More options than shown — search by name. @@ -561,6 +581,84 @@ function CopyKindSections({ controller, byKind }: CopyKindSectionsProps) { ) } +interface TriggerMappingRowProps { + controller: ForkSyncController + mapping: ForkTriggerMapping +} + +/** + * One arriving trigger's URL decision: take over a URL that is retiring in the same target + * workflow, or mint a new one. + * + * Keyed and labelled by BLOCK NAME rather than the raw path - it is one block to one webhook URL, + * and the name is what the user recognises. Adopting keeps the external caller (a Slack Request + * URL, a provider subscription) working with no re-registration at all. + */ +function TriggerMappingRow({ controller, mapping }: TriggerMappingRowProps) { + // A trigger that already serves a URL keeps it, so the row states the URL and offers no + // control. Only a trigger the sync would give a NEW URL has something to decide. + const decidable = mapping.ownPath === null && mapping.adoptablePaths.length > 0 + const chosen = + mapping.sourceBlockId in controller.triggerAdoptions + ? controller.triggerAdoptions[mapping.sourceBlockId] + : (mapping.defaultAdoptPath ?? '') + const resultingPath = mapping.ownPath ?? (chosen === '' ? null : chosen) + + return ( +

+
+ {/* One inner span, so the name and its "in " suffix share a normal inline flow: + `Label` is inline-flex, and a flex container DISCARDS whitespace-only children, which + eats the separating space (and leaves `truncate` with no text run to clip). */} + +
+ {decidable ? ( + ({ + label: + mapping.adoptablePaths.length === 1 + ? 'Keep existing URL' + : `Keep …${path.slice(-12)}`, + value: path, + })), + { label: 'Generate new URL', value: NEW_TRIGGER_URL_VALUE }, + ]} + value={chosen === '' ? NEW_TRIGGER_URL_VALUE : chosen} + onChange={(value) => + controller.setTriggerAdoption( + mapping.sourceBlockId, + value === NEW_TRIGGER_URL_VALUE ? '' : value + ) + } + placeholder='Generate new URL' + /> + ) : ( +

Unchanged

+ )} +
+
+

+ {resultingPath ? ( + {buildWebhookTriggerUrl(resultingPath)} + ) : ( + 'Gets a new URL on sync — register it with the calling service afterwards.' + )} +

+
+ ) +} + interface ForkSyncViewProps { controller: ForkSyncController onDirectionChange: (direction: ForkDirection) => void @@ -574,7 +672,10 @@ interface ForkSyncViewProps { */ export function ForkSyncView({ controller, onDirectionChange }: ForkSyncViewProps) { const detailsError = controller.errorMessage ?? controller.diffErrorMessage - const headsUp = controller.mcpReauthCount > 0 || controller.inlineSecretCount > 0 + const headsUp = + controller.mcpReauthCount > 0 || + controller.inlineSecretCount > 0 || + controller.triggerUrlChanges.length > 0 // Excluded workflows render greyed in the change list. Orient each name's tooltip // to WHERE it is excluded (that's the only place it can be re-included): the sync's @@ -694,6 +795,20 @@ export function ForkSyncView({ controller, onDirectionChange }: ForkSyncViewProp target workspace. ) : null} + {controller.triggerUrlChanges.map((change) => ( +
+ + A webhook URL in {change.workflowName} + {' '} + stops being served — anything calling it will stop working. + + {buildWebhookTriggerUrl(change.path)} + +
+ ))} ) : null} @@ -722,6 +837,20 @@ export function ForkSyncView({ controller, onDirectionChange }: ForkSyncViewProp ) : null} + {controller.triggerMappings.length > 0 ? ( + +
+ {controller.triggerMappings.map((mapping) => ( + + ))} +
+
+ ) : null} + {controller.hasVisibleCopyables ? (
@@ -743,18 +872,51 @@ export function ForkSyncView({ controller, onDirectionChange }: ForkSyncViewProp ) : null} {controller.blockingRefs.length > 0 ? ( - + 1 ? ( + Drop all deleted + ) : undefined + } + >
- {controller.blockingRefs.map((ref, index) => ( -
- {ref.blockLabel} would lose{' '} - {ref.fieldLabel} in{' '} - {ref.workflowName} — {forkBlockerResolution(ref)} -
- ))} + {controller.blockingRefs.map((ref, index) => { + const dropKey = `${ref.kind}:${ref.sourceId}` + const uses = controller.blockingUsesByResource.get(dropKey) ?? 1 + return ( +
+ + {ref.blockLabel} would lose{' '} + {ref.fieldLabel} in{' '} + {ref.workflowName} — {forkBlockerResolution(ref)} + + {/* Only a source-deleted reference can be dropped: an unmapped copyable can still + be copied and a missing workflow can still be deployed, so neither is a dead + end the user should be able to accept away. + + One control per RESOURCE, not per row: the resource is gone, so the sync + clears every field naming it (the remapper's clear resolves by reference, not + by field). Rendering a Drop on each row would imply a per-field choice the + write path cannot honour, so later rows for the same id state the scope + instead. */} + {forkSyncBlockerReasonFor(ref) !== + 'source-deleted' ? null : controller.firstBlockingRowForResource.get(dropKey) === + index ? ( + controller.toggleDroppedRef(ref.kind, ref.sourceId, true)}> + {uses > 1 ? `Drop from ${uses} fields` : 'Drop'} + + ) : ( + + same reference + + )} +
+ ) + })}
) : null} @@ -762,16 +924,30 @@ export function ForkSyncView({ controller, onDirectionChange }: ForkSyncViewProp {controller.dependentClears.length > 0 ? (
- {controller.dependentClears.map((ref, index) => ( -
- {ref.blockLabel} will lose{' '} - {ref.fieldLabel} in{' '} - {ref.workflowName} -
- ))} + {controller.dependentClears.map((ref, index) => { + const droppedKey = `${ref.kind}:${ref.sourceId}` + const dropped = controller.droppedRefs.has(droppedKey) + return ( +
+ + {ref.blockLabel} will lose{' '} + {ref.fieldLabel} in{' '} + {ref.workflowName} + {dropped ? ' — dropped' : ''} + + {dropped ? ( + controller.toggleDroppedRef(ref.kind, ref.sourceId, false)} + > + Undo + + ) : null} +
+ ) + })}

Re-pick these in the target after the sync. 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 15f70c53e4d..54b3669a249 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 @@ -9,6 +9,8 @@ import type { ForkDependentReconfig, ForkMappingEntry, ForkResourceUsage, + ForkTriggerMapping, + ForkTriggerUrlChange, ForkWorkflowChange, } from '@/lib/api/contracts/workspace-fork' import { @@ -40,6 +42,7 @@ import { usePromoteFork, useUpdateForkMapping, } from '@/ee/workspace-forking/hooks/workspace-fork' +import { forkSyncBlockerReasonFor } from '@/ee/workspace-forking/lib/promote/sync-blockers' /** * The mapping kinds that can be a standalone mapping entry. `knowledge-document` is excluded: @@ -92,6 +95,12 @@ export interface ForkKindSummary { export interface ForkSyncController { direction: ForkDirection otherWorkspaceName: string + /** + * The workspace this sync WRITES, named for user-facing copy: the other workspace on push, + * "this workspace" on pull. Derived once here so every surface that names the target - the + * overwrite confirm, the Trigger URLs heading - says the same thing. + */ + targetWorkspaceName: string isLoading: boolean isError: boolean errorMessage: string | null @@ -140,6 +149,25 @@ export interface ForkSyncController { /** The raw copy selection (visible-ness not applied), for per-kind selected-id derivation. */ copySelected: ReadonlySet toggleCopyKeys: (keys: string[], checked: boolean) => void + /** + * Source-deleted references the user accepted losing in the target, keyed `${kind}:${sourceId}`. + * In-session only - an acknowledgment is a decision about this sync, never a stored mapping. + */ + droppedRefs: ReadonlySet + /** Toggle one acknowledgment; the row leaves "Blocking sync" for "Will be cleared". */ + toggleDroppedRef: (kind: string, sourceId: string, dropped: boolean) => void + /** Accept losing every source-deleted blocker at once - the volume is the point. */ + dropAllDeletedRefs: () => void + /** Source-deleted blockers still awaiting a decision, for the bulk affordance. */ + droppableBlockerCount: number + /** + * How many blocking rows name each resource, keyed `${kind}:${sourceId}`. A drop is inherently + * resource-scoped - the remapper clears by reference, not by field - so the row that offers the + * control states how many fields it covers rather than implying a per-field choice. + */ + blockingUsesByResource: ReadonlyMap + /** Index of the row that owns each resource's Drop control, so it renders exactly once. */ + firstBlockingRowForResource: ReadonlyMap /** Visible copy candidates split by referenced-ness, grouped per kind for the section rows. */ referencedByKind: ReadonlyMap unreferencedByKind: ReadonlyMap @@ -152,6 +180,16 @@ export interface ForkSyncController { workflowChanges: ForkWorkflowChange[] /** Names of target workflows this sync archives, for the confirm modal. */ archivedWorkflowNames: string[] + /** Public trigger URLs this sync would stop serving in the target (warn before overwriting). */ + triggerUrlChanges: ForkTriggerUrlChange[] + /** Arriving triggers whose URL is a choice: keep a retiring one, or mint a new one. */ + triggerMappings: ForkTriggerMapping[] + /** + * The chosen adoption per source trigger block. A key present with a path adopts it; present + * with `''` mints a new URL; absent takes the server's `defaultAdoptPath`. + */ + triggerAdoptions: Readonly> + setTriggerAdoption: (sourceBlockId: string, path: string) => void /** Names of deployed SOURCE workflows marked "Exclude from sync" - never sent. */ excludedSourceWorkflows: string[] /** Names of mapped TARGET workflows marked "Exclude from sync" - never replaced or archived. */ @@ -239,6 +277,16 @@ export function useForkSync(params: { // sync so their references resolve to the copy instead of being cleared. const [copySelected, setCopySelected] = useState>(new Set()) const [copyDefaulted, setCopyDefaulted] = useState(false) + // Source-deleted references the user explicitly accepted losing in the target (keyed by + // `${kind}:${sourceId}`). In-session only, like `copySelected` - an acknowledgment is a decision + // about THIS sync, never a stored mapping. The server re-checks that each source really is gone + // before honouring one. + const [droppedRefs, setDroppedRefs] = useState>(new Set()) + // Which retiring public URL each arriving trigger takes over, keyed by SOURCE block id. Session + // state like the two above: the choice is about THIS sync, and once it lands the adopted path is + // stored in the target block's `triggerPath`, so later syncs preserve it with no input at all. + // `''` is the explicit "mint a new URL" choice, distinct from an absent key (take the default). + const [triggerAdoptions, setTriggerAdoptions] = useState>({}) const [submitting, setSubmitting] = useState(false) // Drop every in-session choice when the direction (or edge) changes - the mapping set, @@ -248,6 +296,8 @@ export function useForkSync(params: { setReconfig({}) setCopySelected(new Set()) setCopyDefaulted(false) + setDroppedRefs(new Set()) + setTriggerAdoptions({}) }, [direction, otherWorkspaceId]) const mapping = useForkMapping({ workspaceId, otherWorkspaceId, direction, enabled }) @@ -266,6 +316,10 @@ export function useForkSync(params: { [diff.data?.copyableUnmapped] ) const clearedRefs = useMemo(() => diff.data?.clearedRefs ?? [], [diff.data?.clearedRefs]) + const triggerMappings = useMemo( + () => diff.data?.triggerMappings ?? [], + [diff.data?.triggerMappings] + ) // Keys the backend offers as copy candidates, so the entry rows show a "Copy instead" // affordance only for those - clearing a name-match suggestion returns the ref to the copy @@ -298,6 +352,16 @@ export function useForkSync(params: { [visibleCopyables, copySelected] ) + /** + * Keys that no longer need a mapping target: selected for copy, or an acknowledged drop. Kept + * separate from `copyingKeys` so a dropped reference is never counted as "copied" in the + * per-kind badge. + */ + const satisfiedKeys = useMemo(() => { + if (droppedRefs.size === 0) return copyingKeys + return new Set([...copyingKeys, ...droppedRefs]) + }, [copyingKeys, droppedRefs]) + // Group the visible copy candidates by kind so each renders as its own expandable section // (chevron + tri-state select-all + count), matching the fork picker. Referenced and // unreferenced candidates group separately: unreferenced ones (used by no synced workflow) @@ -448,7 +512,7 @@ export function useForkSync(params: { // A required reference is satisfied when it has a mapping target OR the user selected it for // copy (the server accepts a copy as resolving a required ref). See `isForkRequiredComplete`. - const requiredComplete = isForkRequiredComplete(entries, targets, copyingKeys) + const requiredComplete = isForkRequiredComplete(entries, targets, satisfiedKeys) // Every required dependent whose parent is RESOLVED must have a value before sync. Under a // mapped parent the user re-picks against the target; under a copy-resolved parent the field @@ -494,8 +558,20 @@ export function useForkSync(params: { const mapped = entry ? (targets[key] ?? entry.targetId ?? '') !== '' : false return mapped || copyingKeys.has(key) } - return splitForkClearedRefs(selectVisibleClearedRefs(clearedRefs, isResolved)) - }, [clearedRefs, entriesByParent, targets, copyingKeys]) + const { blockers, informational } = splitForkClearedRefs( + selectVisibleClearedRefs(clearedRefs, isResolved) + ) + if (droppedRefs.size === 0) return { blockers, informational } + // An acknowledged drop stops blocking and moves into the informational "Will be cleared" + // list, mirroring the server: it filters the same entries out of its own gate, but only + // after re-checking that each source really is gone. + const dropped = blockers.filter((ref) => droppedRefs.has(`${ref.kind}:${ref.sourceId}`)) + if (dropped.length === 0) return { blockers, informational } + return { + blockers: blockers.filter((ref) => !droppedRefs.has(`${ref.kind}:${ref.sourceId}`)), + informational: [...informational, ...dropped], + } + }, [clearedRefs, entriesByParent, targets, copyingKeys, droppedRefs]) // Per-kind status for the Mappings summary: "Fully mapped" or "n/total mapped", flagged when // a REQUIRED target is still missing (which blocks Sync). Reads the effective @@ -510,7 +586,7 @@ export function useForkSync(params: { const copied = group.items.filter((entry) => copyingKeys.has(entryKey(entry))).length // Mirror the Sync gate: a required ref selected for copy is satisfied, so it is not // "pending". - const requiredPending = forkRequiredPending(group.items, targets, copyingKeys) + const requiredPending = forkRequiredPending(group.items, targets, satisfiedKeys) const reconfigPending = reconfigPendingByKind.has(group.kind) return { kind: group.kind, total, mapped, copied, requiredPending, reconfigPending } }) @@ -665,9 +741,51 @@ export function useForkSync(params: { ) } + const toggleDroppedRef = (kind: string, sourceId: string, dropped: boolean) => { + const key = `${kind}:${sourceId}` + setDroppedRefs((prev) => { + const next = new Set(prev) + if (dropped) next.add(key) + else next.delete(key) + return next + }) + } + + // Only `source-deleted` blockers are droppable: an unmapped-copyable can be copied and a + // missing workflow can be deployed, so neither is a dead end the user should be able to accept. + const droppableBlockerKeys = useMemo( + () => + blockingRefs + .filter((ref) => forkSyncBlockerReasonFor(ref) === 'source-deleted') + .map((ref) => `${ref.kind}:${ref.sourceId}`), + [blockingRefs] + ) + + const dropAllDeletedRefs = () => { + setDroppedRefs((prev) => new Set([...prev, ...droppableBlockerKeys])) + } + + // Blocking rows indexed by the resource they name, so the Drop control renders once per resource + // and can state how many fields it covers - matching what the sync actually does. + const { blockingUsesByResource, firstBlockingRowForResource } = useMemo(() => { + const uses = new Map() + const firstRow = new Map() + blockingRefs.forEach((ref, index) => { + const key = `${ref.kind}:${ref.sourceId}` + uses.set(key, (uses.get(key) ?? 0) + 1) + if (!firstRow.has(key)) firstRow.set(key, index) + }) + return { blockingUsesByResource: uses, firstBlockingRowForResource: firstRow } + }, [blockingRefs]) + + const setTriggerAdoption = (sourceBlockId: string, path: string) => { + setTriggerAdoptions((prev) => ({ ...prev, [sourceBlockId]: path })) + } + const discard = () => { setTargets({}) setReconfig({}) + setTriggerAdoptions({}) } const sync = async () => { @@ -685,6 +803,27 @@ export function useForkSync(params: { const selectedCopyables = visibleCopyables.filter((candidate) => copySelected.has(forkRefKey(candidate)) ) + // Acknowledged drops, captured at confirm time like every other payload. The server honours + // one only after re-checking that the source resource is genuinely gone. + const dropReferences = Array.from(droppedRefs).map((key) => { + const separator = key.indexOf(':') + return { + kind: key.slice(0, separator) as ForkMappingEntry['kind'], + sourceId: key.slice(separator + 1), + } + }) + // Only the choices that DIFFER from the server's default need sending - an untouched row is + // already what the server would pick, so an empty list means "the preview, as shown". + const triggerMappingOverrides = triggerMappings + .filter( + (mapping) => + mapping.sourceBlockId in triggerAdoptions && + (triggerAdoptions[mapping.sourceBlockId] || null) !== mapping.defaultAdoptPath + ) + .map((mapping) => ({ + sourceBlockId: mapping.sourceBlockId, + adoptPath: triggerAdoptions[mapping.sourceBlockId] || null, + })) try { await updateMapping.mutateAsync({ workspaceId, @@ -716,6 +855,10 @@ export function useForkSync(params: { // existing store is left untouched. ...(dependentValues !== null ? { dependentValues } : {}), ...(selectedCopyables.length > 0 ? { copyResources } : {}), + ...(dropReferences.length > 0 ? { dropReferences } : {}), + ...(triggerMappingOverrides.length > 0 + ? { triggerMappings: triggerMappingOverrides } + : {}), }, }) @@ -751,11 +894,26 @@ export function useForkSync(params: { // Activity entry (needsConfiguration/clearedOptional are recorded there) and a // needs-config workflow visibly stays undeployed. Deploy FAILURES remain a real, // actionable outcome, so they keep a warning. + const dropped = result.droppedReferences.length + // Naming the dropped count is the point of making the drop explicit: the fields really are + // blank in the target now, and the server reports only the acknowledgments it honoured. + const droppedSuffix = + dropped > 0 ? ` ${dropped} deleted reference${dropped === 1 ? '' : 's'} dropped.` : '' + // A dead webhook URL fails silently and externally - nothing in the app breaks - so the one + // moment the user can act on it is right after the sync that killed it. + const deadUrls = result.triggerUrlChanges.length + const urlSuffix = + deadUrls > 0 + ? ` ${deadUrls} webhook URL${deadUrls === 1 ? '' : 's'} stopped being served — re-register ${deadUrls === 1 ? 'it' : 'them'}.` + : '' + const suffix = `${droppedSuffix}${urlSuffix}` if (result.deployFailed > 0) { const n = result.deployFailed toast.warning( - `${label}, but ${n} workflow${n === 1 ? '' : 's'} failed to deploy — open and redeploy ${n === 1 ? 'it' : 'them'}.` + `${label}, but ${n} workflow${n === 1 ? '' : 's'} failed to deploy — open and redeploy ${n === 1 ? 'it' : 'them'}.${suffix}` ) + } else if (suffix !== '') { + toast.warning(`${label}.${suffix}`) } else { toast.success(label) } @@ -769,6 +927,7 @@ export function useForkSync(params: { return { direction, otherWorkspaceName, + targetWorkspaceName: direction === 'push' ? otherWorkspaceName : 'this workspace', isLoading: enabled && mapping.isLoading, isError: mapping.isError, errorMessage: mapping.isError ? getErrorMessage(mapping.error, 'Failed to load mapping') : null, @@ -793,6 +952,12 @@ export function useForkSync(params: { copyingKeys, copySelected, toggleCopyKeys, + droppedRefs, + toggleDroppedRef, + dropAllDeletedRefs, + droppableBlockerCount: droppableBlockerKeys.length, + blockingUsesByResource, + firstBlockingRowForResource, referencedByKind, unreferencedByKind, hasVisibleCopyables: visibleCopyables.length > 0, @@ -800,6 +965,10 @@ export function useForkSync(params: { dependentClears, workflowChanges, archivedWorkflowNames, + triggerUrlChanges: diff.data?.triggerUrlChanges ?? [], + triggerMappings, + triggerAdoptions, + setTriggerAdoption, excludedSourceWorkflows: diff.data?.excludedSourceWorkflows ?? [], excludedTargetWorkflows: diff.data?.excludedTargetWorkflows ?? [], mcpReauthCount: diff.data?.mcpReauthServerIds.length ?? 0, diff --git a/apps/sim/ee/workspace-forking/components/forks.tsx b/apps/sim/ee/workspace-forking/components/forks.tsx index 6cc9789acd1..c8582141d6b 100644 --- a/apps/sim/ee/workspace-forking/components/forks.tsx +++ b/apps/sim/ee/workspace-forking/components/forks.tsx @@ -46,6 +46,7 @@ import { } from '@/ee/workspace-forking/hooks/workspace-fork' import { useWorkspaceCreationPolicy, useWorkspacesQuery } from '@/hooks/queries/workspace' import { useSettingsNavigation } from '@/hooks/use-settings-navigation' +import { buildWebhookTriggerUrl } from '@/triggers/webhook-url' /** Explains a disabled lineage action whose target workspace the viewer cannot open. */ const NO_ACCESS_TOOLTIP = "You don't have access to this workspace" @@ -152,7 +153,7 @@ function ForkSyncDetailView({ }, ] - const targetWorkspaceName = direction === 'push' ? otherWorkspaceName : 'this workspace' + const targetWorkspaceName = controller.targetWorkspaceName return ( <> @@ -224,6 +225,36 @@ function ForkSyncDetailView({ ) : null}

) : null} + {/* A dead trigger URL is only discoverable after the fact, when the external caller goes + quiet - so it belongs in the confirm, next to the other irreversible consequences. */} + {controller.triggerUrlChanges.length > 0 ? ( +
+

+ {controller.triggerUrlChanges.length === 1 ? 'A webhook URL' : 'Webhook URLs'} in{' '} + {targetWorkspaceName} will stop being served — + anything calling {controller.triggerUrlChanges.length === 1 ? 'it' : 'them'} breaks + until you re-register: +

+ {controller.triggerUrlChanges.slice(0, ARCHIVED_PREVIEW_LIMIT).map((change) => ( + // Naming the URL, not just its workflow: several URLs in one workflow would render + // as identical lines, and this confirm is the last point before they stop serving. +
+ {change.workflowName} + + {buildWebhookTriggerUrl(change.path)} + +
+ ))} + {controller.triggerUrlChanges.length > ARCHIVED_PREVIEW_LIMIT ? ( +
+ and {controller.triggerUrlChanges.length - ARCHIVED_PREVIEW_LIMIT} more +
+ ) : null} +
+ ) : null} ) diff --git a/apps/sim/ee/workspace-forking/lib/copy/copy-workflows.test.ts b/apps/sim/ee/workspace-forking/lib/copy/copy-workflows.test.ts index 330fdb2a025..44913d6381c 100644 --- a/apps/sim/ee/workspace-forking/lib/copy/copy-workflows.test.ts +++ b/apps/sim/ee/workspace-forking/lib/copy/copy-workflows.test.ts @@ -354,3 +354,81 @@ describe('copyWorkflowStateIntoTarget canonicalModes reindex propagation', () => } ) }) + +describe('copyWorkflowStateIntoTarget webhook path pinning', () => { + const sourceState = { + blocks: { + 'blk-src': { + id: 'blk-src', + type: 'slack', + name: 'Slack', + // The SOURCE's own path, written back into its draft after its deploy. Copying it would + // point the target at the source's URL, so the sanitizer strips it. + subBlocks: { triggerPath: { id: 'triggerPath', type: 'short-input', value: 'src-path' } }, + outputs: {}, + enabled: true, + }, + }, + edges: [], + loops: {}, + parallels: {}, + variables: {}, + } as never + + const baseParams = { + targetWorkflowId: 'wf-tgt', + targetWorkspaceId: 'ws-target', + userId: 'target-user', + mode: 'replace' as const, + now: new Date('2026-07-01'), + sourceState, + sourceMeta: { name: 'Prod', description: null, folderId: null, sortOrder: 0 }, + workflowIdMap: new Map(), + folderIdMap: new Map(), + nameRegistry: buildWorkflowNameRegistry([]), + resolveBlockId: (_targetWorkflowId: string, sourceBlockId: string) => `tgt-${sourceBlockId}`, + } + + /** `replace` mode updates the existing target workflow row; stub just that chain. */ + const stubTx = () => + ({ + update: () => ({ set: () => ({ where: () => Promise.resolve() }) }), + }) as unknown as DbOrTx + + function writtenSubBlocks() { + const state = mockSaveWorkflowToNormalizedTables.mock.calls.at(-1)?.[1] as { + blocks: Record }> + } + return state.blocks['tgt-blk-src'].subBlocks ?? {} + } + + it("pins the TARGET's live webhook path so a sync never moves a URL already in the wild", async () => { + mockSaveWorkflowToNormalizedTables.mockResolvedValue({ success: true }) + await copyWorkflowStateIntoTarget({ + ...baseParams, + tx: stubTx(), + triggerPathByBlockId: new Map([['tgt-blk-src', 'parent-live-path']]), + }) + expect(writtenSubBlocks().triggerPath?.value).toBe('parent-live-path') + }) + + /** + * The adoption case: the arriving trigger has a different target block id (re-created in the + * source), and the resolver handed it the URL retiring in the same target workflow. + */ + it('writes an ADOPTED path onto a trigger block that serves no webhook of its own', async () => { + mockSaveWorkflowToNormalizedTables.mockResolvedValue({ success: true }) + await copyWorkflowStateIntoTarget({ + ...baseParams, + tx: stubTx(), + triggerPathByBlockId: new Map([['tgt-blk-src', 'retiring-slack-path']]), + }) + expect(writtenSubBlocks().triggerPath?.value).toBe('retiring-slack-path') + }) + + it('leaves the path unset when the target block serves no webhook yet (derives as before)', async () => { + mockSaveWorkflowToNormalizedTables.mockResolvedValue({ success: true }) + await copyWorkflowStateIntoTarget({ ...baseParams, tx: stubTx() }) + expect(writtenSubBlocks().triggerPath).toBeUndefined() + }) +}) diff --git a/apps/sim/ee/workspace-forking/lib/copy/copy-workflows.ts b/apps/sim/ee/workspace-forking/lib/copy/copy-workflows.ts index 0e6ac294fb2..2d6fec8dbb2 100644 --- a/apps/sim/ee/workspace-forking/lib/copy/copy-workflows.ts +++ b/apps/sim/ee/workspace-forking/lib/copy/copy-workflows.ts @@ -362,6 +362,13 @@ export interface CopyWorkflowStateParams { * creation, where every id is derived fresh. */ resolveBlockId?: ForkBlockIdResolver + /** + * The resolved public webhook path per TARGET block id - the block's own live path, or a + * retiring one it adopts (see `resolveForkTriggerPaths`). Pinned into the target block's + * `triggerPath` so the URL stops being a derivation of the block id this sync assigns. + * Omitted on fork creation, where the child has no webhooks yet. + */ + triggerPathByBlockId?: ReadonlyMap requestId?: string } @@ -394,6 +401,7 @@ export async function copyWorkflowStateIntoTarget( dependentOverrides, nameRegistry, resolveBlockId, + triggerPathByBlockId, requestId = 'unknown', } = params @@ -438,6 +446,19 @@ export async function copyWorkflowStateIntoTarget( const sourceSubBlocks = (block.subBlocks ?? {}) as unknown as SubBlockRecord const sanitizedSource = sanitizeSubBlocksForDuplicate(sourceSubBlocks) let subBlocks: SubBlockRecord = sanitizedSource + // The sanitizer strips `triggerPath` (the SOURCE's URL must never be copied). Pin the + // TARGET's resolved path back in - the one this block already serves, or a retiring one it + // adopts - so the URL stops being a derivation of the block id this sync assigns. Otherwise + // any later change to that id silently re-points a URL external systems already call (a + // Slack Request URL, a provider subscription). With no resolved path the field stays empty + // and derives as before, so a first-time sync is unchanged. + const resolvedTriggerPath = triggerPathByBlockId?.get(newBlockId) + if (resolvedTriggerPath) { + subBlocks = { + ...subBlocks, + triggerPath: { id: 'triggerPath', type: 'short-input', value: resolvedTriggerPath }, + } + } // Tracks the block's live `canonicalModes` through this pass, so a `tool-input` reindex // (a dropped custom-tool/MCP entry shifts later tools' array positions) is visible to every // later step below that resolves a nested tool's basic/advanced mode - not just the final diff --git a/apps/sim/ee/workspace-forking/lib/copy/deploy-bridge.ts b/apps/sim/ee/workspace-forking/lib/copy/deploy-bridge.ts index 8bfc338bf20..759c48347d1 100644 --- a/apps/sim/ee/workspace-forking/lib/copy/deploy-bridge.ts +++ b/apps/sim/ee/workspace-forking/lib/copy/deploy-bridge.ts @@ -1,11 +1,12 @@ import { db, runOutsideTransactionContext } from '@sim/db' -import { workflow, workflowDeploymentVersion } from '@sim/db/schema' +import { webhook, workflow, workflowDeploymentVersion } from '@sim/db/schema' import { createLogger } from '@sim/logger' -import { and, eq, exists, inArray, isNull, sql } from 'drizzle-orm' +import { and, eq, exists, inArray, isNotNull, isNull, sql } from 'drizzle-orm' import type { DbOrTx } from '@/lib/db/types' import { loadDeployedWorkflowState } from '@/lib/workflows/persistence/utils' import { ForkError } from '@/ee/workspace-forking/lib/lineage/authz' import type { Variable, WorkflowState } from '@/stores/workflows/workflow/types' +import { isInternalTriggerProvider, isPollingWebhookProvider } from '@/triggers/constants' const logger = createLogger('WorkspaceForkDeployBridge') @@ -227,3 +228,83 @@ export async function readDeployedState( } }) } + +/** A live, path-based webhook on a target block: the URL it serves and the workflow it belongs to. */ +export interface ForkTargetWebhook { + path: string + workflowId: string + /** + * The provider the path is served under. An inbound request is authenticated and parsed as this + * provider, so a URL is only meaningfully transferable to a trigger of the SAME provider. + */ + provider: string | null +} + +/** + * The public webhook path each target trigger block currently serves on, keyed by block id. + * + * A webhook's path defaults to its block id (`triggerPath || block.id`, see + * `lib/webhooks/deploy.ts`), so the target's URL has always been a *derivation* of an id the + * sync itself assigns. Reading the live path lets the copy pin it back into the target block's + * own `triggerPath`, turning the URL into stored data - the sync then cannot move a URL that + * external systems (a Slack Request URL, a provider subscription) are already pointing at. + * + * Only rows serving a PUBLIC URL are returned. Three families are excluded, because preserving + * their path would be meaningless and offering it for adoption actively wrong: + * - shared-app providers (the native Slack trigger) route by `routingKey` with a NULL path; + * - polling providers ({@link isPollingWebhookProvider}) keep a webhook row as state, but Sim + * pulls from the provider - nothing external calls the path; + * - internal providers ({@link isInternalTriggerProvider}) register a path that the public + * trigger route deliberately rejects, so it is not an endpoint either. + * + * Scoped to each workflow's ACTIVE deployment version, exactly as inbound delivery resolves a + * path (`lib/webhooks/processor.ts`). A workflow keeps non-archived webhook rows from previous + * versions too (`lib/webhooks/deploy.ts` reads "ALL webhooks for this workflow (all versions)" + * before narrowing to the current one), so an unscoped read would return several rows per block + * and pick a stale path arbitrarily - pinning a URL nothing is actually serving, which is the + * precise failure this function exists to prevent. + */ +export async function loadTargetWebhookPathsByBlock( + executor: DbOrTx, + targetWorkflowIds: string[] +): Promise> { + if (targetWorkflowIds.length === 0) return new Map() + const rows = await executor + .select({ + blockId: webhook.blockId, + path: webhook.path, + workflowId: webhook.workflowId, + provider: webhook.provider, + }) + .from(webhook) + .innerJoin( + workflowDeploymentVersion, + and( + eq(workflowDeploymentVersion.workflowId, webhook.workflowId), + eq(workflowDeploymentVersion.isActive, true), + eq(workflowDeploymentVersion.id, webhook.deploymentVersionId) + ) + ) + .where( + and( + inArray(webhook.workflowId, targetWorkflowIds), + isNull(webhook.archivedAt), + isNotNull(webhook.blockId), + isNotNull(webhook.path) + ) + ) + const byBlock = new Map() + for (const row of rows) { + if (!row.blockId || !row.path) continue + if (isPollingWebhookProvider(row.provider ?? '') || isInternalTriggerProvider(row.provider)) { + continue + } + // One live path-based row per block within a version - `path_deployment_unique` enforces it. + byBlock.set(row.blockId, { + path: row.path, + workflowId: row.workflowId, + provider: row.provider, + }) + } + return byBlock +} diff --git a/apps/sim/ee/workspace-forking/lib/mapping/mapping-service.test.ts b/apps/sim/ee/workspace-forking/lib/mapping/mapping-service.test.ts index 4efb1d0778a..2a7fda4ead2 100644 --- a/apps/sim/ee/workspace-forking/lib/mapping/mapping-service.test.ts +++ b/apps/sim/ee/workspace-forking/lib/mapping/mapping-service.test.ts @@ -4,24 +4,63 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' import type { ForkRemapKind } from '@/ee/workspace-forking/lib/remap/remap-references' -const { mockFilterExisting, mockGetCredentialProviders, mockGetEnvKeys } = vi.hoisted(() => ({ +const { + mockFilterExisting, + mockGetCredentialProviders, + mockGetEnvKeys, + mockLoadLabels, + mockListCandidates, + mockClassifyCredential, + mockListDeployedWorkflows, + mockReadDeployedState, + mockScanWorkflowReferences, + mockDetectCascade, +} = vi.hoisted(() => ({ mockFilterExisting: vi.fn(), mockGetCredentialProviders: vi.fn(), mockGetEnvKeys: vi.fn(), + mockLoadLabels: vi.fn(), + mockListCandidates: vi.fn(), + mockClassifyCredential: vi.fn(), + mockListDeployedWorkflows: vi.fn(), + mockReadDeployedState: vi.fn(), + mockScanWorkflowReferences: vi.fn(), + mockDetectCascade: vi.fn(), })) vi.mock('@/ee/workspace-forking/lib/mapping/resources', () => ({ - listForkResourceCandidates: vi.fn(), - classifyCredentialResourceType: vi.fn(), + listForkResourceCandidates: mockListCandidates, + classifyCredentialResourceType: mockClassifyCredential, getWorkspaceEnvKeys: mockGetEnvKeys, filterExistingForkTargets: mockFilterExisting, getCredentialProvidersByIds: mockGetCredentialProviders, + loadForkResourceLabels: mockLoadLabels, CANDIDATE_LIMIT: 1000, })) +vi.mock('@/ee/workspace-forking/lib/copy/deploy-bridge', () => ({ + listDeployedWorkflows: mockListDeployedWorkflows, + readDeployedState: mockReadDeployedState, +})) + +vi.mock('@/ee/workspace-forking/lib/mapping/cascade', () => ({ + detectForkCascadeReferences: mockDetectCascade, +})) + +vi.mock('@/ee/workspace-forking/lib/remap/remap-references', () => ({ + scanWorkflowReferences: mockScanWorkflowReferences, +})) + +vi.mock('@/ee/workspace-forking/lib/remap/reference-scan', () => ({ + toScannerBlocks: vi.fn((state: unknown) => state), +})) + +import { workflow, workspaceForkResourceMap } from '@sim/db/schema' +import { queueTableRows, resetDbChainMock } from '@sim/testing' import { ForkError } from '@/ee/workspace-forking/lib/lineage/authz' import { findDuplicateTargetEntry, + getForkMappingView, suggestTarget, validateForkMappingTargets, } from '@/ee/workspace-forking/lib/mapping/mapping-service' @@ -149,16 +188,32 @@ describe('validateForkMappingTargets', () => { ).resolves.toBeUndefined() }) - it('rejects a credential whose source is not a credential in the source workspace', async () => { + /** + * A source credential that no longer exists is exactly what the mapping editor asks the user + * to resolve (`sourceDeleted`), so the save must accept it - rejecting made it the one kind of + * reference the UI told you to map and the server refused. Access propagation is not driven + * from here: `propagateCredentialAccess` re-validates both sides inside the promote tx. + */ + it('accepts a credential whose source no longer exists in the source workspace', async () => { mockFilterExisting.mockResolvedValue({ credential: new Set(['cred-tgt']) }) mockGetCredentialProviders.mockImplementation(async (_db: unknown, workspaceId: string) => workspaceId === 'ws-source' - ? new Map() // cred-foreign is not in the source + ? new Map() // cred-deleted is gone from the source : new Map([['cred-tgt', 'google-email']]) ) await expect( validateForkMappingTargets('ws-source', 'ws-target', [ - { resourceType: 'oauth_credential', sourceId: 'cred-foreign', targetId: 'cred-tgt' }, + { resourceType: 'oauth_credential', sourceId: 'cred-deleted', targetId: 'cred-tgt' }, + ]) + ).resolves.toBeUndefined() + }) + + it('still rejects a target that does not exist, even when the source is gone', async () => { + mockFilterExisting.mockResolvedValue({ credential: new Set() }) + mockGetCredentialProviders.mockImplementation(async () => new Map()) + await expect( + validateForkMappingTargets('ws-source', 'ws-target', [ + { resourceType: 'oauth_credential', sourceId: 'cred-deleted', targetId: 'cred-foreign' }, ]) ).rejects.toBeInstanceOf(ForkError) }) @@ -246,3 +301,109 @@ describe('suggestTarget', () => { expect(suggestTarget('table', ' Orders ', undefined, [cand('t1', 'orders')])).toBe('t1') }) }) + +describe('getForkMappingView', () => { + const edge = { parentWorkspaceId: 'ws-parent', childWorkspaceId: 'ws-child' } as never + const emptyCandidates = { + credential: [], + 'env-var': [], + table: [], + 'knowledge-base': [], + 'mcp-server': [], + 'custom-tool': [], + skill: [], + 'knowledge-document': [], + file: [], + } + + /** Pull: parent is the source, child the target — the direction the raw-id rows showed up in. */ + function pullView(overrides: { workflowRows?: unknown[] } = {}) { + // The real `getEdgeMappingRows` runs; this row is the workflow identity pair it returns. + queueTableRows(workspaceForkResourceMap, [ + { + id: 'map-1', + childWorkspaceId: 'ws-child', + resourceType: 'workflow', + parentResourceId: 'wf-parent', + childResourceId: 'wf-child', + }, + ]) + queueTableRows( + workflow, + overrides.workflowRows ?? [{ id: 'wf-child', forkSyncExcluded: false }] + ) + return getForkMappingView({ + edge, + sourceWorkspaceId: 'ws-parent', + targetWorkspaceId: 'ws-child', + }) + } + + beforeEach(() => { + vi.clearAllMocks() + resetDbChainMock() + mockGetEnvKeys.mockResolvedValue(new Set()) + mockListCandidates.mockResolvedValue(emptyCandidates) + mockListDeployedWorkflows.mockResolvedValue([{ id: 'wf-parent', name: 'Prod' }]) + mockReadDeployedState.mockResolvedValue({ blocks: {} }) + mockScanWorkflowReferences.mockReturnValue({ + references: [ + { kind: 'table', sourceId: 'tbl_live', subBlockKey: 'tableSelector', required: false }, + { kind: 'table', sourceId: 'tbl_gone', subBlockKey: 'tableSelector', required: false }, + ], + }) + mockDetectCascade.mockResolvedValue({ references: [] }) + mockFilterExisting.mockResolvedValue({}) + mockGetCredentialProviders.mockResolvedValue(new Map()) + mockClassifyCredential.mockResolvedValue('oauth_credential') + mockLoadLabels.mockResolvedValue({ table: new Map([['tbl_live', 'Orders']]) }) + }) + + it('labels a live source resource by name and flags a deleted one', async () => { + const { entries } = await pullView() + expect(entries).toEqual([ + expect.objectContaining({ + sourceId: 'tbl_live', + sourceLabel: 'Orders', + sourceDeleted: false, + }), + expect.objectContaining({ + sourceId: 'tbl_gone', + sourceLabel: 'tbl_gone', + sourceDeleted: true, + }), + ]) + }) + + /** + * The label lookup must be by exact id, never the display-capped candidate list — otherwise a + * workspace past CANDIDATE_LIMIT renders live resources as raw ids, indistinguishable from + * deleted ones. Pinned by asserting the exact ids are what gets looked up. + */ + it('looks source labels up by exact id, not through the capped candidate list', async () => { + await pullView() + expect(mockLoadLabels).toHaveBeenCalledWith(expect.anything(), 'ws-parent', { + table: new Set(['tbl_live', 'tbl_gone']), + }) + expect(mockListCandidates).toHaveBeenCalledTimes(1) + expect(mockListCandidates).toHaveBeenCalledWith(expect.anything(), 'ws-child') + }) + + it('skips a source workflow whose target is excluded from sync', async () => { + const { entries } = await pullView({ + workflowRows: [{ id: 'wf-child', forkSyncExcluded: true }], + }) + expect(entries).toEqual([]) + expect(mockReadDeployedState).not.toHaveBeenCalled() + }) + + it('still scans when the excluded flag is on an unrelated target workflow', async () => { + const { entries } = await pullView({ + workflowRows: [ + { id: 'wf-child', forkSyncExcluded: false }, + { id: 'wf-other', forkSyncExcluded: true }, + ], + }) + expect(entries).toHaveLength(2) + }) +}) diff --git a/apps/sim/ee/workspace-forking/lib/mapping/mapping-service.ts b/apps/sim/ee/workspace-forking/lib/mapping/mapping-service.ts index 64f3ca2d91a..8ad23f68d0f 100644 --- a/apps/sim/ee/workspace-forking/lib/mapping/mapping-service.ts +++ b/apps/sim/ee/workspace-forking/lib/mapping/mapping-service.ts @@ -1,4 +1,6 @@ import { db } from '@sim/db' +import { workflow } from '@sim/db/schema' +import { and, eq, isNull } from 'drizzle-orm' import type { ForkMappableResourceType, ForkMappingEntry } from '@/lib/api/contracts/workspace-fork' import type { DbOrTx } from '@/lib/db/types' import { @@ -25,7 +27,9 @@ import { getCredentialProvidersByIds, getWorkspaceEnvKeys, listForkResourceCandidates, + loadForkResourceLabels, } from '@/ee/workspace-forking/lib/mapping/resources' +import { resolveForkExcludedTargetId } from '@/ee/workspace-forking/lib/promote/promote-plan' import { toScannerBlocks } from '@/ee/workspace-forking/lib/remap/reference-scan' import { type ForkReference, @@ -66,13 +70,16 @@ export async function getForkMappingView( const { edge, sourceWorkspaceId, targetWorkspaceId } = params const sourceIsParent = sourceWorkspaceId === edge.parentWorkspaceId - const [mappingRows, targetEnvKeys, sourceEnvKeys, sourceCandidates, targetCandidates] = + const [mappingRows, targetEnvKeys, sourceEnvKeys, targetCandidates, targetWorkflows] = await Promise.all([ getEdgeMappingRows(db, edge.childWorkspaceId), getWorkspaceEnvKeys(db, targetWorkspaceId), getWorkspaceEnvKeys(db, sourceWorkspaceId), - listForkResourceCandidates(db, sourceWorkspaceId), listForkResourceCandidates(db, targetWorkspaceId), + db + .select({ id: workflow.id, forkSyncExcluded: workflow.forkSyncExcluded }) + .from(workflow) + .where(and(eq(workflow.workspaceId, targetWorkspaceId), isNull(workflow.archivedAt))), ]) const resolver = buildForkResolver(mappingRows, { sourceIsParent, targetEnvKeys, sourceEnvKeys }) @@ -93,11 +100,30 @@ export async function getForkMappingView( if (key) resourceTypeBySourceId.set(key, row.resourceType) } + // The workflow identity map + the target's live/excluded sets, so this view scans exactly the + // workflows a sync would write. Without the exclusion filter a source whose target is marked + // "Exclude from sync" still contributed blocking mapping entries the sync could never act on. + const identityMap = new Map() + for (const row of mappingRows) { + if (row.resourceType !== 'workflow' || row.childResourceId == null) continue + if (sourceIsParent) identityMap.set(row.parentResourceId, row.childResourceId) + else identityMap.set(row.childResourceId, row.parentResourceId) + } + const targetActiveIds = new Set(targetWorkflows.map((w) => w.id)) + const excludedTargetIds = new Set( + targetWorkflows.filter((w) => w.forkSyncExcluded).map((w) => w.id) + ) + // Scan one deployed workflow state at a time and merge deduped references, so // peak memory stays at a single workflow state rather than all of them at once. const deployedWorkflows = await listDeployedWorkflows(db, sourceWorkspaceId) const referenceByKey = new Map() for (const wf of deployedWorkflows) { + if ( + resolveForkExcludedTargetId(wf.id, identityMap, targetActiveIds, excludedTargetIds) !== null + ) { + continue + } const state = await readDeployedState(wf.id, sourceWorkspaceId) if (!state) continue for (const reference of scanWorkflowReferences(toScannerBlocks(state), () => null).references) { @@ -116,6 +142,25 @@ export async function getForkMappingView( } const references: ForkReference[] = Array.from(referenceByKey.values()) + // Source-side labels and credential providers, both looked up by EXACT ID (never the capped + // candidate list). A capped lookup made a live resource past `CANDIDATE_LIMIT` render as a raw + // id, indistinguishable from a deleted one - and, for a credential, silently dropped the + // provider filter so the picker offered every provider's credentials. Resolved here, an id + // missing from `sourceLabels` means exactly one thing: it no longer exists in the source. + const sourceIdsByKind: Partial>> = {} + for (const reference of references) { + if (reference.kind === 'env-var' || reference.kind === 'knowledge-document') continue + ;(sourceIdsByKind[reference.kind] ??= new Set()).add(reference.sourceId) + } + const [sourceLabels, sourceProviders] = await Promise.all([ + loadForkResourceLabels(db, sourceWorkspaceId, sourceIdsByKind), + getCredentialProvidersByIds( + db, + sourceWorkspaceId, + Array.from(sourceIdsByKind.credential ?? []) + ), + ]) + // First pass: resolve each reference's stored target + the data to build its entry, // collecting stored target ids so existence is checked by exact id (cap-free) - a // valid mapping to a target past the display cap must be RETAINED, not shown unmapped. @@ -123,6 +168,7 @@ export async function getForkMappingView( reference: ForkReference resourceType: ForkMappableResourceType sourceLabel: string + sourceDeleted: boolean sourceProviderId: string | undefined candidates: ForkResourceCandidate[] storedTargetId: string | null @@ -147,11 +193,16 @@ export async function getForkMappingView( : nonCredentialForkKindToResourceType(reference.kind) } - const sourceCandidate = sourceCandidates[reference.kind].find( - (c) => c.id === reference.sourceId - ) - const sourceLabel = sourceCandidate?.label ?? reference.sourceId - const sourceProviderId = sourceCandidate?.providerId + // An env var IS its own name, so it can never be "deleted but referenced" here - a `{{KEY}}` + // absent from the source workspace was already skipped above as a personal secret. + const sourceLabel = + reference.kind === 'env-var' + ? reference.sourceId + : (sourceLabels[reference.kind]?.get(reference.sourceId) ?? reference.sourceId) + const sourceDeleted = + reference.kind !== 'env-var' && + !(sourceLabels[reference.kind]?.has(reference.sourceId) ?? false) + const sourceProviderId = sourceProviders.get(reference.sourceId) ?? undefined // A credential reference only maps to a target credential of the SAME OAuth // provider - a Gmail (google-email) reference must never offer a Google Calendar // credential. Non-credential kinds carry no provider, so their full list stands. @@ -169,6 +220,7 @@ export async function getForkMappingView( reference, resourceType, sourceLabel, + sourceDeleted, sourceProviderId, candidates, storedTargetId, @@ -212,6 +264,7 @@ export async function getForkMappingView( resourceType: p.resourceType, sourceId: p.reference.sourceId, sourceLabel: p.sourceLabel, + sourceDeleted: p.sourceDeleted, targetId, suggested, // Every entry here is a reference a synced workflow actually carries, and a sync is @@ -414,16 +467,18 @@ export async function validateForkMappingTargets( } if (kind === 'credential') { - // The source must be a real credential in the source workspace. A foreign id - // (not present) would skip the provider check and let a crafted mapping drive - // cross-workspace credential-access propagation on promote. - if (!sourceProviders.has(entry.sourceId)) { - throw new ForkError( - `Source credential "${entry.sourceId}" is not a credential in the source workspace`, - 400 - ) - } + // A source credential that no longer exists in the source workspace is EXPECTED here: the + // mapping editor deliberately lists such references (`sourceDeleted`) because mapping the + // dead id to a live target is the documented way to unblock the sync. Rejecting the save + // made that the one kind you could not resolve - the UI told you to map it and the server + // refused. Accepting it is safe: the target is still proven to belong to the target + // workspace above, and credential-ACCESS propagation is not driven from here - promote's + // `propagateCredentialAccess` re-validates BOTH sides inside its transaction and skips any + // pair whose source is not a live credential of the source workspace. const sourceProviderId = sourceProviders.get(entry.sourceId) + if (sourceProviderId === undefined) continue + // With a live source, the target must share its OAuth provider - a Gmail reference can + // never be pointed at a Google Calendar credential. const targetProviderId = targetProviders.get(targetId) ?? null if (sourceProviderId && targetProviderId !== sourceProviderId) { throw new ForkError( diff --git a/apps/sim/ee/workspace-forking/lib/mapping/resources.ts b/apps/sim/ee/workspace-forking/lib/mapping/resources.ts index 362849b5e10..8216d5c4a0a 100644 --- a/apps/sim/ee/workspace-forking/lib/mapping/resources.ts +++ b/apps/sim/ee/workspace-forking/lib/mapping/resources.ts @@ -240,21 +240,27 @@ export async function listForkResourceCandidates( } } +/** One live resource, by exact id. `label` is absent for kinds looked up by id only. */ +interface ForkResourceRow { + id: string + label?: string +} + /** - * Given mapped target ids grouped by kind, return the subset that still EXISTS in the - * target workspace (same archived/deleted filters as `listForkResourceCandidates`). - * Used at promote time so a mapping whose target was deleted after it was saved - * resolves as unmapped (surfaced/cleared) instead of writing a dead id into the - * promoted workflow. Queries the exact ids (not the capped candidate list) so a valid - * target is never wrongly dropped, and only the DB-backed kinds are checked - env-var - * existence is handled by the resolver's `targetEnvKeys`, and `file`/`workflow` are - * resolved by other paths. + * Look up the given ids, grouped by kind, in one workspace and return the rows that still EXIST + * (same archived/deleted filters as `listForkResourceCandidates`). Queries the exact ids - NOT + * the capped candidate list - so a resource sitting past `CANDIDATE_LIMIT` is never mistaken for + * a missing one. Only the DB-backed kinds are checked: env-var existence is handled by the + * resolver's `targetEnvKeys`, and `file`/`workflow` are resolved by other paths. + * + * Backs both {@link filterExistingForkTargets} (existence) and {@link loadForkResourceLabels} + * (display names), so the two can never disagree about what "exists" means. */ -export async function filterExistingForkTargets( +async function loadForkResourceRows( executor: DbOrTx, workspaceId: string, idsByKind: Partial>> -): Promise>>> { +): Promise>> { const ids = (kind: ForkRemapKind): string[] => { const set = idsByKind[kind] return set && set.size > 0 ? Array.from(set) : [] @@ -272,9 +278,9 @@ export async function filterExistingForkTargets( const [creds, tables, kbs, docs, servers, tools, skills, files] = await Promise.all([ credIds.length === 0 - ? Promise.resolve([] as Array<{ id: string }>) + ? Promise.resolve([] as ForkResourceRow[]) : executor - .select({ id: credential.id }) + .select({ id: credential.id, label: credential.displayName }) .from(credential) .where( and( @@ -284,15 +290,15 @@ export async function filterExistingForkTargets( ) ), tableIds.length === 0 - ? Promise.resolve([] as Array<{ id: string }>) + ? Promise.resolve([] as ForkResourceRow[]) : tableCandidatesQuery(executor, workspaceId, tableIds), kbIds.length === 0 - ? Promise.resolve([] as Array<{ id: string }>) + ? Promise.resolve([] as ForkResourceRow[]) : knowledgeBaseCandidatesQuery(executor, workspaceId, kbIds), // Documents are validated through a KB join (they are not a standalone candidate kind), so // this existence check stays inline rather than sharing a per-kind candidate query. docIds.length === 0 - ? Promise.resolve([] as Array<{ id: string }>) + ? Promise.resolve([] as ForkResourceRow[]) : executor .select({ id: document.id }) .from(document) @@ -307,29 +313,75 @@ export async function filterExistingForkTargets( ) ), mcpIds.length === 0 - ? Promise.resolve([] as Array<{ id: string }>) + ? Promise.resolve([] as ForkResourceRow[]) : mcpServerCandidatesQuery(executor, workspaceId, mcpIds), toolIds.length === 0 - ? Promise.resolve([] as Array<{ id: string }>) + ? Promise.resolve([] as ForkResourceRow[]) : customToolCandidatesQuery(executor, workspaceId, toolIds), skillIds.length === 0 - ? Promise.resolve([] as Array<{ id: string }>) + ? Promise.resolve([] as ForkResourceRow[]) : skillCandidatesQuery(executor, workspaceId, skillIds), fileKeys.length === 0 - ? Promise.resolve([] as Array<{ id: string }>) + ? Promise.resolve([] as ForkResourceRow[]) : fileCandidatesQuery(executor, workspaceId, fileKeys), ]) + const result: Partial> = {} + if (credIds.length > 0) result.credential = creds + if (tableIds.length > 0) result.table = tables + if (kbIds.length > 0) result['knowledge-base'] = kbs + if (docIds.length > 0) result['knowledge-document'] = docs + if (mcpIds.length > 0) result['mcp-server'] = servers + if (toolIds.length > 0) result['custom-tool'] = tools + if (skillIds.length > 0) result.skill = skills + // `fileCandidatesQuery` exposes the storage key under `id`, so file rows key by `r.id`. + if (fileKeys.length > 0) result.file = files + return result +} + +/** + * Given mapped target ids grouped by kind, return the subset that still EXISTS in the target + * workspace. Used at promote time so a mapping whose target was deleted after it was saved + * resolves as unmapped (surfaced/cleared) instead of writing a dead id into the promoted + * workflow, and by the cleared-ref collector pointed at the SOURCE workspace to flag a + * reference whose resource is gone. + */ +export async function filterExistingForkTargets( + executor: DbOrTx, + workspaceId: string, + idsByKind: Partial>> +): Promise>>> { + const rows = await loadForkResourceRows(executor, workspaceId, idsByKind) const result: Partial>> = {} - if (credIds.length > 0) result.credential = new Set(creds.map((r) => r.id)) - if (tableIds.length > 0) result.table = new Set(tables.map((r) => r.id)) - if (kbIds.length > 0) result['knowledge-base'] = new Set(kbs.map((r) => r.id)) - if (docIds.length > 0) result['knowledge-document'] = new Set(docs.map((r) => r.id)) - if (mcpIds.length > 0) result['mcp-server'] = new Set(servers.map((r) => r.id)) - if (toolIds.length > 0) result['custom-tool'] = new Set(tools.map((r) => r.id)) - if (skillIds.length > 0) result.skill = new Set(skills.map((r) => r.id)) - // `fileCandidatesQuery` exposes the storage key under `id`, so file existence keys by `r.id`. - if (fileKeys.length > 0) result.file = new Set(files.map((r) => r.id)) + for (const [kind, kindRows] of Object.entries(rows) as Array< + [ForkRemapKind, ForkResourceRow[]] + >) { + result[kind] = new Set(kindRows.map((row) => row.id)) + } + return result +} + +/** + * Display names for the given ids, grouped by kind, looked up by exact id in one workspace. + * + * The mapping view labels each scanned reference with this rather than with the capped + * `listForkResourceCandidates` output: a workspace past `CANDIDATE_LIMIT` would otherwise render + * a perfectly live resource as a raw id, indistinguishable from one that was actually deleted. + * With this, an id absent from the returned map means exactly one thing - the resource no longer + * exists in that workspace - which is what `ForkMappingEntry.sourceDeleted` reports. + */ +export async function loadForkResourceLabels( + executor: DbOrTx, + workspaceId: string, + idsByKind: Partial>> +): Promise>>> { + const rows = await loadForkResourceRows(executor, workspaceId, idsByKind) + const result: Partial>> = {} + for (const [kind, kindRows] of Object.entries(rows) as Array< + [ForkRemapKind, ForkResourceRow[]] + >) { + result[kind] = new Map(kindRows.map((row) => [row.id, row.label ?? row.id])) + } return result } diff --git a/apps/sim/ee/workspace-forking/lib/promote/cleared-refs.test.ts b/apps/sim/ee/workspace-forking/lib/promote/cleared-refs.test.ts index 0487a6082c9..e988d239cec 100644 --- a/apps/sim/ee/workspace-forking/lib/promote/cleared-refs.test.ts +++ b/apps/sim/ee/workspace-forking/lib/promote/cleared-refs.test.ts @@ -957,7 +957,7 @@ describe('collectForkSyncBlockers', () => { mockLoadCopyableLabels.mockResolvedValue( new Map([['table:tbl-src', { label: 'Orders', parentId: null, parentLabel: null }]]) ) - const blockers = await collectForkSyncBlockers( + const { blockers } = await collectForkSyncBlockers( baseParams({ sourceStates: new Map([ [ @@ -987,7 +987,7 @@ describe('collectForkSyncBlockers', () => { blockWith([{ id: 'tbl', title: 'Table', type: 'table-selector' }]) ) const { executor, select } = makeExecutor() - const blockers = await collectForkSyncBlockers( + const { blockers } = await collectForkSyncBlockers( baseParams({ executor, sourceStates: new Map([ @@ -1014,7 +1014,7 @@ describe('collectForkSyncBlockers', () => { ) mockFilterExisting.mockResolvedValue({ 'mcp-server': new Set(['srv-1']) }) const { executor } = makeExecutor([[{ id: 'srv-1', name: 'Internal Tools' }]]) - const blockers = await collectForkSyncBlockers( + const { blockers } = await collectForkSyncBlockers( baseParams({ executor, sourceStates: new Map([ @@ -1037,6 +1037,58 @@ describe('collectForkSyncBlockers', () => { ]) }) + /** + * The drop hatch. `sourceDeleted` is re-derived here from the source workspace inside the + * promote transaction, so the acknowledgment is only ever honoured against a reference that is + * genuinely gone - a crafted payload can never drop a working one. + */ + it('honours a drop acknowledgment for a source-deleted reference', async () => { + vi.mocked(getBlock).mockReturnValue( + blockWith([{ id: 'kb', title: 'Knowledge Base', type: 'knowledge-base-selector' }]) + ) + mockFilterExisting.mockResolvedValue({ 'knowledge-base': new Set() }) + const { blockers, appliedDrops } = await collectForkSyncBlockers( + baseParams({ + sourceStates: new Map([ + [ + 'wf-src', + stateWith('knowledge', 'KB Block', { + kb: { type: 'knowledge-base-selector', value: 'kb-gone' }, + }), + ], + ]), + droppedReferences: [{ kind: 'knowledge-base', sourceId: 'kb-gone' }], + }) + ) + expect(blockers).toEqual([]) + expect(appliedDrops).toEqual([{ kind: 'knowledge-base', sourceId: 'kb-gone' }]) + }) + + it('ignores a drop acknowledgment for a reference whose source is still live', async () => { + vi.mocked(getBlock).mockReturnValue( + blockWith([{ id: 'kb', title: 'Knowledge Base', type: 'knowledge-base-selector' }]) + ) + // The source row still exists, so the reference is an unmapped-copyable, not source-deleted. + mockFilterExisting.mockResolvedValue({ 'knowledge-base': new Set(['kb-live']) }) + const { blockers, appliedDrops } = await collectForkSyncBlockers( + baseParams({ + sourceStates: new Map([ + [ + 'wf-src', + stateWith('knowledge', 'KB Block', { + kb: { type: 'knowledge-base-selector', value: 'kb-live' }, + }), + ], + ]), + droppedReferences: [{ kind: 'knowledge-base', sourceId: 'kb-live' }], + }) + ) + expect(blockers).toEqual([ + expect.objectContaining({ sourceId: 'kb-live', reason: 'unmapped-copyable' }), + ]) + expect(appliedDrops).toEqual([]) + }) + it('blocks a source-deleted reference (source-deleted) - no exemption, resolvable by mapping', async () => { vi.mocked(getBlock).mockReturnValue( blockWith([{ id: 'kb', title: 'Knowledge Base', type: 'knowledge-base-selector' }]) @@ -1044,7 +1096,7 @@ describe('collectForkSyncBlockers', () => { // The liveness check reports the source row gone; the copy loader (live rows only) misses, // so the label falls back to the id. mockFilterExisting.mockResolvedValue({ 'knowledge-base': new Set() }) - const blockers = await collectForkSyncBlockers( + const { blockers } = await collectForkSyncBlockers( baseParams({ sourceStates: new Map([ [ @@ -1066,7 +1118,7 @@ describe('collectForkSyncBlockers', () => { ]) // Mapping the dead id to a live target resolves it (the resolver never checks source // liveness - a mapping row whose source row is gone still resolves). - const resolved = await collectForkSyncBlockers( + const { blockers: resolved } = await collectForkSyncBlockers( baseParams({ sourceStates: new Map([ [ @@ -1095,7 +1147,7 @@ describe('collectForkSyncBlockers', () => { targetActiveIds: new Set(['wf-child-tgt']), items: [{ sourceWorkflowId: 'wf-src', targetWorkflowId: 'wf-tgt' }], }) - const blockers = await collectForkSyncBlockers( + const { blockers } = await collectForkSyncBlockers( baseParams({ executor, sourceStates: new Map([ @@ -1132,7 +1184,7 @@ describe('collectForkSyncBlockers', () => { targetActiveIds: new Set(['wf-child-tgt']), items: [{ sourceWorkflowId: 'wf-src', targetWorkflowId: 'wf-tgt' }], }) - const blockers = await collectForkSyncBlockers( + const { blockers } = await collectForkSyncBlockers( baseParams({ executor, sourceStates: new Map([ @@ -1167,8 +1219,8 @@ describe('collectForkSyncBlockers', () => { ], ]) - const freshScan = await collectForkSyncBlockers(baseParams({ sourceStates })) - const reusedPlan = await collectForkSyncBlockers( + const { blockers: freshScan } = await collectForkSyncBlockers(baseParams({ sourceStates })) + const { blockers: reusedPlan } = await collectForkSyncBlockers( baseParams({ sourceStates, planUnmapped: [{ kind: 'table', sourceId: 'tbl-src' }], @@ -1179,7 +1231,7 @@ describe('collectForkSyncBlockers', () => { // unchanged either way. const overlayResolver: ForkReferenceResolver = (kind, id) => kind === 'custom-tool' && id === 'ct-unreferenced' ? 'ct-copy' : null - const withIrrelevantCopy = await collectForkSyncBlockers( + const { blockers: withIrrelevantCopy } = await collectForkSyncBlockers( baseParams({ sourceStates, resolver: overlayResolver, @@ -1206,7 +1258,7 @@ describe('collectForkSyncBlockers', () => { blockWith([{ id: 'tbl', title: 'Table', type: 'table-selector' }]) ) const { executor, select } = makeExecutor() - const blockers = await collectForkSyncBlockers( + const { blockers } = await collectForkSyncBlockers( baseParams({ executor, sourceStates: new Map([ @@ -1230,7 +1282,7 @@ describe('collectForkSyncBlockers', () => { blockWith([{ id: 'tbl', title: 'Table', type: 'table-selector' }]) ) const { executor, select } = makeExecutor() - const blockers = await collectForkSyncBlockers( + const { blockers } = await collectForkSyncBlockers( baseParams({ executor, sourceStates: new Map([ @@ -1259,7 +1311,7 @@ describe('collectForkSyncBlockers', () => { blockWith([{ id: 'target', title: 'Workflow', type: 'workflow-selector' }]) ) const { executor } = makeExecutor([[{ id: 'wf-child', name: 'Child Flow' }]]) - const blockers = await collectForkSyncBlockers( + const { blockers } = await collectForkSyncBlockers( baseParams({ executor, sourceStates: new Map([ @@ -1289,7 +1341,7 @@ describe('collectForkSyncBlockers', () => { blockWith([{ id: 'workflowIds', title: 'Workflows', type: 'dropdown', multiSelect: true }]) ) const { executor } = makeExecutor([[{ id: 'wf-watched', name: 'Watched Workflow' }]]) - const blockers = await collectForkSyncBlockers( + const { blockers } = await collectForkSyncBlockers( baseParams({ executor, sourceStates: new Map([ @@ -1351,7 +1403,7 @@ describe('collectForkSyncBlockers', () => { parallels: {}, variables: {}, } as unknown as WorkflowState - const blockers = await collectForkSyncBlockers( + const { blockers } = await collectForkSyncBlockers( baseParams({ executor, sourceStates: new Map([['wf-src', state]]), @@ -1376,7 +1428,7 @@ describe('collectForkSyncBlockers', () => { ]) ) const { executor, select } = makeExecutor() - const blockers = await collectForkSyncBlockers( + const { blockers } = await collectForkSyncBlockers( baseParams({ executor, items: [{ ...replaceItem, mode: 'create' as const }], diff --git a/apps/sim/ee/workspace-forking/lib/promote/cleared-refs.ts b/apps/sim/ee/workspace-forking/lib/promote/cleared-refs.ts index 521111dfde4..540c2e10e84 100644 --- a/apps/sim/ee/workspace-forking/lib/promote/cleared-refs.ts +++ b/apps/sim/ee/workspace-forking/lib/promote/cleared-refs.ts @@ -300,6 +300,34 @@ export async function annotateForkClearedRefSourceLiveness( ) } +/** + * Narrow a caller's drop acknowledgments to the ones the server will actually honour: a reference + * whose kind can block at all, and whose resource is genuinely gone from the SOURCE workspace. + * + * Both promote gates consult this, so "which drops count" is decided once. The unmapped gate runs + * FIRST and would otherwise reject a dropped required reference before the cleared-ref gate ever + * got to honour it - making Drop unusable for exactly the required references it exists for. It + * cannot simply subtract the raw acknowledgments there either: an unmapped reference of a + * non-blocking kind (credential, env-var) never re-blocks downstream, so an unverified subtraction + * would let a crafted payload skip the required gate entirely. + */ +export async function verifyForkDropAcknowledgments( + executor: DbOrTx, + sourceWorkspaceId: string, + acknowledged: ReadonlyArray<{ kind: ForkRemapKind; sourceId: string }> | undefined +): Promise> { + const droppable = (acknowledged ?? []).filter( + (entry) => !CLEARED_REF_EXCLUDED_KINDS.has(entry.kind) + ) + if (droppable.length === 0) return [] + const idsByKind: Partial>> = {} + for (const entry of droppable) { + ;(idsByKind[entry.kind] ??= new Set()).add(entry.sourceId) + } + const liveByKind = await filterExistingForkTargets(executor, sourceWorkspaceId, idsByKind) + return droppable.filter((entry) => !(liveByKind[entry.kind]?.has(entry.sourceId) ?? false)) +} + /** Upper bound on the blockers a gate failure reports, so the error body stays sane. */ const FORK_SYNC_BLOCKER_LIMIT = 100 @@ -376,24 +404,60 @@ export async function collectForkSyncBlockers( * rows) when one does. Omit to always collect from scratch. */ planUnmapped?: ReadonlyArray> + /** + * References the user explicitly acknowledged dropping. Applied ONLY where the source + * resource is actually gone, judged by the liveness annotation below - which reads the + * source workspace inside this same transaction - so an acknowledgment for a still-live + * reference is ignored and keeps blocking. + */ + droppedReferences?: ReadonlyArray<{ kind: ForkRemapKind; sourceId: string }> } -): Promise { - const { executor, sourceWorkspaceId, planUnmapped, ...collectParams } = params - if (planUnmapped && !hasForkSyncBlockerCandidates(planUnmapped, collectParams)) return [] +): Promise<{ + blockers: ForkSyncBlocker[] + /** The acknowledgments that were actually honoured, for post-sync reporting. */ + appliedDrops: Array<{ kind: ForkRemapKind; sourceId: string }> +}> { + const { executor, sourceWorkspaceId, planUnmapped, droppedReferences, ...collectParams } = params + const empty = { blockers: [] as ForkSyncBlocker[], appliedDrops: [] } + if (planUnmapped && !hasForkSyncBlockerCandidates(planUnmapped, collectParams)) return empty const candidates = collectForkClearedRefCandidates({ ...collectParams, sourceLabels: new Map(), sourceWorkflowNames: new Map(), }) - if (!candidates.some((ref) => ref.cause === 'reference' || ref.cause === 'workflow')) return [] + if (!candidates.some((ref) => ref.cause === 'reference' || ref.cause === 'workflow')) return empty const annotated = await annotateForkClearedRefSourceLiveness( executor, sourceWorkspaceId, candidates ) - const blocking = selectForkSyncBlockingRefs(annotated).slice(0, FORK_SYNC_BLOCKER_LIMIT) - if (blocking.length === 0) return [] + + const acknowledged = new Set( + (droppedReferences ?? []).map((entry) => `${entry.kind}:${entry.sourceId}`) + ) + const appliedDropKeys = new Set() + const afterDrops = + acknowledged.size === 0 + ? annotated + : annotated.filter((ref) => { + const key = `${ref.kind}:${ref.sourceId}` + // `sourceDeleted` is set only on `reference`-cause entries, so this can never drop a + // dependent- or workflow-cause blocker, nor a reference whose source is still live. + if (ref.cause !== 'reference' || !ref.sourceDeleted || !acknowledged.has(key)) return true + appliedDropKeys.add(key) + return false + }) + const appliedDrops = Array.from(appliedDropKeys).map((key) => { + const separator = key.indexOf(':') + return { + kind: key.slice(0, separator) as ForkRemapKind, + sourceId: key.slice(separator + 1), + } + }) + + const blocking = selectForkSyncBlockingRefs(afterDrops).slice(0, FORK_SYNC_BLOCKER_LIMIT) + if (blocking.length === 0) return { blockers: [], appliedDrops } // Best-effort display labels (failure path only). Copyable kinds go through the shared label // loader (live rows only - a deleted source keeps its id label); MCP servers are read without @@ -434,7 +498,10 @@ export async function collectForkSyncBlockers( return copyableLabels.get(`${ref.kind}:${ref.sourceId}`)?.label ?? ref.sourceLabel } - return toForkSyncBlockers( - blocking.map(({ ref, reason }) => ({ ref: { ...ref, sourceLabel: labelFor(ref) }, reason })) - ) + return { + blockers: toForkSyncBlockers( + blocking.map(({ ref, reason }) => ({ ref: { ...ref, sourceLabel: labelFor(ref) }, reason })) + ), + appliedDrops, + } } diff --git a/apps/sim/ee/workspace-forking/lib/promote/promote-plan.ts b/apps/sim/ee/workspace-forking/lib/promote/promote-plan.ts index 79932823041..c666c345c30 100644 --- a/apps/sim/ee/workspace-forking/lib/promote/promote-plan.ts +++ b/apps/sim/ee/workspace-forking/lib/promote/promote-plan.ts @@ -140,6 +140,27 @@ export function buildPromoteWorkflowIdMap(params: { * reported in `excludedTargets` instead of written - the target side of the * "Exclude from sync" contract. Pure - split from the DB reads so it is unit-testable. */ +/** + * The target this source would write, when that target is live AND marked "Exclude from sync" - + * the target side of the exclusion contract, which makes the sync skip the source entirely. + * Returns null when the source is not excluded. + * + * Shared by the plan builder and by `getForkMappingView`, so the Mappings section can never list + * references carried only by a workflow the sync provably never touches. Such an entry would be + * unresolvable-looking (its "Used in" list is plan-scoped, so it renders empty) yet still block + * Sync, because every mapping entry is `required`. + */ +export function resolveForkExcludedTargetId( + sourceWorkflowId: string, + identityMap: ReadonlyMap, + targetActiveIds: ReadonlySet, + excludedTargetIds: ReadonlySet +): string | null { + const mappedTargetId = identityMap.get(sourceWorkflowId) + if (!mappedTargetId || !targetActiveIds.has(mappedTargetId)) return null + return excludedTargetIds.has(mappedTargetId) ? mappedTargetId : null +} + export function buildForkPromotePlanItems(params: { deployedSourceWorkflows: DeployedWorkflowSummary[] sourceStateIds: ReadonlySet @@ -164,16 +185,22 @@ export function buildForkPromotePlanItems(params: { for (const source of deployedSourceWorkflows) { if (!sourceStateIds.has(source.id)) continue - const mappedTargetId = identityMap.get(source.id) - const activeTargetId = - mappedTargetId && targetActiveIds.has(mappedTargetId) ? mappedTargetId : null - if (activeTargetId && excludedTargetIds.has(activeTargetId)) { + const excludedTargetId = resolveForkExcludedTargetId( + source.id, + identityMap, + targetActiveIds, + excludedTargetIds + ) + if (excludedTargetId !== null) { excludedTargets.push({ - id: activeTargetId, - name: targetNameById.get(activeTargetId) ?? source.name, + id: excludedTargetId, + name: targetNameById.get(excludedTargetId) ?? source.name, }) continue } + const mappedTargetId = identityMap.get(source.id) + const activeTargetId = + mappedTargetId && targetActiveIds.has(mappedTargetId) ? mappedTargetId : null items.push({ sourceWorkflowId: source.id, targetWorkflowId: activeTargetId ?? generateId(), diff --git a/apps/sim/ee/workspace-forking/lib/promote/promote.test.ts b/apps/sim/ee/workspace-forking/lib/promote/promote.test.ts index fd5f5035892..c44dd9c9e2a 100644 --- a/apps/sim/ee/workspace-forking/lib/promote/promote.test.ts +++ b/apps/sim/ee/workspace-forking/lib/promote/promote.test.ts @@ -20,6 +20,8 @@ const { mockCreateTransform, mockSumForkCopyBytes, mockAssertForkStorageHeadroom, + mockLoadTargetWebhookPaths, + mockVerifyDrops, } = vi.hoisted(() => ({ mockComputePlan: vi.fn(), mockBuildCopySelection: vi.fn(), @@ -36,6 +38,8 @@ const { mockCreateTransform: vi.fn(), mockSumForkCopyBytes: vi.fn(), mockAssertForkStorageHeadroom: vi.fn(), + mockLoadTargetWebhookPaths: vi.fn(), + mockVerifyDrops: vi.fn(), })) vi.mock('@/lib/workflows/deployment-outbox', () => ({ @@ -68,6 +72,7 @@ vi.mock('@/ee/workspace-forking/lib/copy/storage-quota', () => ({ vi.mock('@/ee/workspace-forking/lib/copy/deploy-bridge', () => ({ getActiveDeploymentVersionNumbers: vi.fn(async () => new Map()), loadSourceDeployedStates: mockLoadSourceDeployedStates, + loadTargetWebhookPathsByBlock: mockLoadTargetWebhookPaths, })) vi.mock('@/ee/workspace-forking/lib/lineage/lineage', () => ({ acquireForkEdgeLock: vi.fn(), @@ -104,6 +109,7 @@ vi.mock('@/ee/workspace-forking/lib/mapping/mapping-store', () => ({ })) vi.mock('@/ee/workspace-forking/lib/promote/cleared-refs', () => ({ collectForkSyncBlockers: mockCollectBlockers, + verifyForkDropAcknowledgments: mockVerifyDrops, })) vi.mock('@/ee/workspace-forking/lib/promote/copy-unmapped', () => ({ // Faithful mirror of the real overlay so a copy's id maps resolve through the augmented @@ -152,6 +158,7 @@ vi.mock('@/lib/workspaces/permissions/utils', () => ({ })) import { db } from '@sim/db' +import { getBlock } from '@/blocks/registry' import { copyWorkflowStateIntoTarget } from '@/ee/workspace-forking/lib/copy/copy-workflows' import { reconcileForkDependentValues } from '@/ee/workspace-forking/lib/mapping/dependent-value-store' import { promoteFork } from '@/ee/workspace-forking/lib/promote/promote' @@ -246,7 +253,7 @@ beforeEach(() => { willResolve: new Set(), }) mockHasCopySelection.mockReturnValue(false) - mockCollectBlockers.mockResolvedValue([]) + mockCollectBlockers.mockResolvedValue({ blockers: [], appliedDrops: [] }) mockLoadBlockMap.mockResolvedValue(new Map()) mockBuildBlockIdResolver.mockReturnValue((_wf: string, blockId: string) => blockId) mockResolveFolderMapping.mockResolvedValue(new Map()) @@ -255,6 +262,9 @@ beforeEach(() => { mockCreateTransform.mockReturnValue((subBlocks: unknown) => subBlocks) mockSumForkCopyBytes.mockResolvedValue(0) mockAssertForkStorageHeadroom.mockResolvedValue(undefined) + mockLoadTargetWebhookPaths.mockResolvedValue(new Map()) + // Default: no acknowledgments, so the unmapped gate behaves exactly as before. + mockVerifyDrops.mockResolvedValue([]) }) describe('promoteFork gates', () => { @@ -321,8 +331,56 @@ describe('promoteFork gates', () => { expect(mockUpsertPromoteRun).not.toHaveBeenCalled() }) + /** + * A source-deleted reference on a REQUIRED field sits in `unmappedRequired`, and that gate runs + * before the cleared-ref gate that honours drops. Without subtracting verified drops here, Drop + * was inert for exactly the references it exists to unblock - the sync still failed with + * "map all required ... first". + */ + it('lets a VERIFIED drop clear the unmapped gate for a required reference', async () => { + mockComputePlan.mockResolvedValue( + makePlan({ + unmappedRequired: [ + { kind: 'table', sourceId: 'tbl-gone', subBlockKey: 'tableSelector', required: true }, + ], + }) + ) + mockVerifyDrops.mockResolvedValue([{ kind: 'table', sourceId: 'tbl-gone' }]) + + const result = await promoteFork({ + ...promoteParams(), + dropReferences: [{ kind: 'table', sourceId: 'tbl-gone' }], + }) + + expect(result.blocked).toBeNull() + // The SAME verified set reaches the cleared-ref gate, so one liveness check governs both. + expect(mockCollectBlockers).toHaveBeenCalledWith( + expect.objectContaining({ droppedReferences: [{ kind: 'table', sourceId: 'tbl-gone' }] }) + ) + }) + + /** An acknowledgment the server refuses (source still live) must not weaken the required gate. */ + it('keeps blocking when the acknowledgment fails verification', async () => { + mockComputePlan.mockResolvedValue( + makePlan({ + unmappedRequired: [ + { kind: 'table', sourceId: 'tbl-live', subBlockKey: 'tableSelector', required: true }, + ], + }) + ) + mockVerifyDrops.mockResolvedValue([]) + + const result = await promoteFork({ + ...promoteParams(), + dropReferences: [{ kind: 'table', sourceId: 'tbl-live' }], + }) + + expect(result.blocked).toBe('unmapped') + expect(mockCollectBlockers).not.toHaveBeenCalled() + }) + it('blocks with the structured blocker list when references would clear, writing NOTHING', async () => { - mockCollectBlockers.mockResolvedValue([BLOCKER]) + mockCollectBlockers.mockResolvedValue({ blockers: [BLOCKER], appliedDrops: [] }) const result = await promoteFork(promoteParams()) @@ -624,3 +682,106 @@ describe('promoteFork dependent values', () => { ) }) }) + +describe('promoteFork trigger URLs', () => { + beforeEach(() => { + // A block only holds a public URL when its config declares a `useWebhookUrl` field, so the + // fixture has to look like a webhook trigger to the shared predicate. + vi.mocked(getBlock).mockReturnValue({ + category: 'triggers', + subBlocks: [{ id: 'triggerWebhookUrl', useWebhookUrl: true }], + } as never) + }) + + const triggerState = { + blocks: { + 'blk-new': { + id: 'blk-new', + // The REAL slack_webhook trigger id, so the provider check resolves against the actual + // registry - adoption only pairs a URL with a trigger of the SAME provider. + type: 'slack_webhook', + name: 'Slack messages', + triggerMode: true, + subBlocks: {}, + outputs: {}, + enabled: true, + }, + }, + edges: [], + loops: {}, + parallels: {}, + variables: {}, + } + + function arrangeReCreatedTrigger() { + const item = { + sourceWorkflowId: 'wf-src', + targetWorkflowId: 'wf-tgt', + targetName: 'Flow', + mode: 'replace' as const, + sourceMeta: { name: 'Flow', description: null, folderId: null, sortOrder: 0 }, + } + mockComputePlan.mockResolvedValue(makePlan({ items: [item] })) + mockLoadSourceDeployedStates.mockResolvedValue({ + deployedWorkflows: [], + sourceStates: new Map([['wf-src', triggerState]]), + }) + // The old trigger block ('blk-old') serves the live URL and is NOT in the source any more: + // the user deleted and re-added the trigger, so the sync writes 'blk-new' instead. + mockLoadTargetWebhookPaths.mockResolvedValue( + new Map([['blk-old', { path: 'live-slack-path', workflowId: 'wf-tgt', provider: 'slack' }]]) + ) + vi.mocked(copyWorkflowStateIntoTarget).mockResolvedValue({ + targetWorkflowId: 'wf-tgt', + mode: 'replace', + name: 'Flow', + blocksCount: 1, + edgesCount: 0, + subflowsCount: 0, + clearedDependents: [], + blockIdMapping: new Map(), + }) + } + + /** + * The reported bug, at the promote level: pushing a workflow whose Slack trigger was re-created + * used to hand the parent a brand-new webhook URL, forcing a re-paste into Slack every sync. + */ + it('hands the retiring URL to the arriving trigger instead of minting a new one', async () => { + arrangeReCreatedTrigger() + + const result = await promoteFork(promoteParams()) + + expect(result.blocked).toBeNull() + const writeParams = vi.mocked(copyWorkflowStateIntoTarget).mock.calls[0][0] + expect(writeParams.triggerPathByBlockId?.get('blk-new')).toBe('live-slack-path') + // Adopted, so nothing needs re-registering externally. + expect(result.triggerUrlChanges).toEqual([]) + }) + + it('reports the URL as lost when the caller explicitly opts into a new one', async () => { + arrangeReCreatedTrigger() + + const result = await promoteFork({ + ...promoteParams(), + triggerMappings: [{ sourceBlockId: 'blk-new', adoptPath: null }], + }) + + const writeParams = vi.mocked(copyWorkflowStateIntoTarget).mock.calls[0][0] + expect(writeParams.triggerPathByBlockId?.size).toBe(0) + expect(result.triggerUrlChanges).toEqual([{ workflowName: 'Flow', path: 'live-slack-path' }]) + }) + + /** The server re-derives the adoptable set, so a stale or crafted path is never honoured. */ + it('ignores a mapping naming a path the plan does not offer', async () => { + arrangeReCreatedTrigger() + + await promoteFork({ + ...promoteParams(), + triggerMappings: [{ sourceBlockId: 'blk-new', adoptPath: 'someone-elses-path' }], + }) + + const writeParams = vi.mocked(copyWorkflowStateIntoTarget).mock.calls[0][0] + expect(writeParams.triggerPathByBlockId?.size).toBe(0) + }) +}) diff --git a/apps/sim/ee/workspace-forking/lib/promote/promote.ts b/apps/sim/ee/workspace-forking/lib/promote/promote.ts index 5de3872f0f4..22a25a9e42c 100644 --- a/apps/sim/ee/workspace-forking/lib/promote/promote.ts +++ b/apps/sim/ee/workspace-forking/lib/promote/promote.ts @@ -33,6 +33,7 @@ import { import { getActiveDeploymentVersionNumbers, loadSourceDeployedStates, + loadTargetWebhookPathsByBlock, } from '@/ee/workspace-forking/lib/copy/deploy-bridge' import { assertForkStorageHeadroom, @@ -63,7 +64,10 @@ import { upsertEdgeMappings, } from '@/ee/workspace-forking/lib/mapping/mapping-store' import { getMcpServerMetaByIds } from '@/ee/workspace-forking/lib/mapping/resources' -import { collectForkSyncBlockers } from '@/ee/workspace-forking/lib/promote/cleared-refs' +import { + collectForkSyncBlockers, + verifyForkDropAcknowledgments, +} from '@/ee/workspace-forking/lib/promote/cleared-refs' import { augmentForkResolver, buildPromoteCopySelection, @@ -78,6 +82,12 @@ import { type PromoteRunWorkflowSnapshot, upsertPromoteRun, } from '@/ee/workspace-forking/lib/promote/promote-run-store' +import { + buildForkTriggerPlan, + type ForkTriggerMappingInput, + type ForkTriggerUrlChange, + resolveForkTriggerPaths, +} from '@/ee/workspace-forking/lib/promote/trigger-urls' import { buildForkBlockIdResolver } from '@/ee/workspace-forking/lib/remap/block-identity' import { createForkSubBlockTransform, @@ -119,6 +129,17 @@ export interface PromoteForkParams { * plan's copyable candidates, so an arbitrary id is ignored. */ copyResources?: PromoteCopyResources + /** + * References the caller explicitly acknowledged dropping, so the sync clears them in the target + * instead of blocking. Honoured only where the source resource is genuinely gone (re-derived + * in-transaction), so a live reference can never be dropped by a crafted payload. + */ + dropReferences?: Array<{ kind: ForkRemapKind; sourceId: string }> + /** + * Which retiring public URL each arriving trigger takes over. Re-validated in-transaction + * against the adoptable set the plan derives, so an entry the plan does not offer is ignored. + */ + triggerMappings?: ForkTriggerMappingInput[] requestId?: string } @@ -159,6 +180,16 @@ export interface PromoteForkResult { * behavior is never silent. */ clearedOptional: Array<{ workflowName: string; blocks: string[] }> + /** + * Source-deleted references the user acknowledged dropping that this sync actually cleared in + * the target. Only entries whose source was verified gone in-transaction appear here. + */ + droppedReferences: Array<{ kind: ForkRemapKind; sourceId: string }> + /** + * Public trigger URLs this sync stopped serving in the target - a URL that retired with no + * arriving trigger adopting it. Whatever calls it externally has to be repointed. + */ + triggerUrlChanges: ForkTriggerUrlChange[] } function collectCredentialPairs(plan: ForkPromotePlan): Array<[string, string]> { @@ -307,6 +338,10 @@ interface PromoteTxApplied { needsConfiguration: Array<{ workflowId: string; workflowName: string; blocks: string[] }> /** Per-workflow optional dependents a parent change cleared (surfaced, not gated). */ clearedOptional: Array<{ workflowName: string; blocks: string[] }> + /** Acknowledged source-deleted references this sync cleared instead of blocking on. */ + droppedReferences: Array<{ kind: ForkRemapKind; sourceId: string }> + /** Public trigger URLs this sync stopped serving (nothing adopted them). */ + triggerUrlChanges: ForkTriggerUrlChange[] /** Heavy content for resources copied into the target this sync, filled best-effort post-commit. */ copyContentPlan: ForkContentPlan | null /** Serialized in-content maps for the post-commit skill-body rewrite (paired with the plan). */ @@ -442,10 +477,23 @@ export async function promoteFork(params: PromoteForkParams): Promise `${entry.kind}:${entry.sourceId}`)) // plan.unmappedRequired is already references.filter(resolver == null).filter(required), so // subtracting the refs the copy will resolve is equivalent to re-scanning the predicate. const postCopyUnmappedRequired = plan.unmappedRequired.filter( - (reference) => !willResolve.has(`${reference.kind}:${reference.sourceId}`) + (reference) => + !willResolve.has(`${reference.kind}:${reference.sourceId}`) && + !droppedKeys.has(`${reference.kind}:${reference.sourceId}`) ) if (postCopyUnmappedRequired.length > 0) { return { @@ -477,9 +525,10 @@ export async function promoteFork(params: PromoteForkParams): Promise = [] const gateResolver: ForkReferenceResolver = (kind, sourceId) => willResolve.has(`${kind}:${sourceId}`) ? sourceId : plan.resolver(kind, sourceId) - const blockers = await collectForkSyncBlockers({ + const { blockers, appliedDrops } = await collectForkSyncBlockers({ executor: tx, sourceWorkspaceId, items: plan.items, @@ -488,10 +537,12 @@ export async function promoteFork(params: PromoteForkParams): Promise 0) { return { blocked: 'cleared-refs', blockers } } + droppedReferences = appliedDrops // Resolve the source->target folder map BEFORE the copy so the folders already exist in the // target and the copy can rewrite `sim:folder/` references inside copied skill / markdown @@ -638,6 +689,22 @@ export async function promoteFork(params: PromoteForkParams): Promise item.targetWorkflowId) + ), + }) + const { pathByTargetBlockId: triggerPathByBlockId, changes: triggerUrlChanges } = + resolveForkTriggerPaths(triggerPlan, params.triggerMappings) + const updatedSnapshots: PromoteRunWorkflowSnapshot[] = [] const createdTargetIds: string[] = [] const writtenItems: typeof plan.items = [] @@ -672,6 +739,7 @@ export async function promoteFork(params: PromoteForkParams): Promise): WorkflowState { + return { + blocks: Object.fromEntries( + Object.entries(blocks).map(([id, block]) => [ + id, + { id, type: block.type, name: block.name, subBlocks: {}, outputs: {}, enabled: true }, + ]) + ), + edges: [], + loops: {}, + parallels: {}, + } as unknown as WorkflowState +} + +const item = { + sourceWorkflowId: 'wf-src', + targetWorkflowId: 'wf-tgt', + targetName: 'Prod', + mode: 'replace' as const, + sourceMeta: { + name: 'Prod', + description: null, + folderId: null, + sortOrder: 0, + isPublicApi: false, + }, +} + +/** Identity resolver: the source block keeps its id in the target (the stable-pairing case). */ +const identityResolver = (_targetWorkflowId: string, sourceBlockId: string) => sourceBlockId + +function webhooks(entries: Array<[string, ForkTargetWebhook]>) { + return new Map(entries) +} + +function run( + blocks: Record, + targetWebhooks: Map, + overrides?: ForkTriggerMappingInput[] +) { + const plan = buildForkTriggerPlan({ + items: [item], + sourceStates: new Map([['wf-src', stateWith(blocks)]]), + resolveBlockId: identityResolver, + targetWebhooks, + }) + return { plan, ...resolveForkTriggerPaths(plan, overrides) } +} + +/** + * Blocks use the REAL `slack_webhook` trigger id, so provider resolution runs against the actual + * trigger registry (provider `slack`) rather than a mock that could drift from it. + */ +describe('fork trigger URLs', () => { + beforeEach(() => { + vi.clearAllMocks() + vi.mocked(getBlock).mockReturnValue(TRIGGER_BLOCK as never) + }) + + it('pins a trigger that keeps its target identity to its own path, reporting no change', () => { + const { pathByTargetBlockId, changes, plan } = run( + { blk: { type: 'slack_webhook', name: 'Slack' } }, + webhooks([['blk', { path: 'custom-path', workflowId: 'wf-tgt', provider: 'slack' }]]) + ) + expect(changes).toEqual([]) + expect(pathByTargetBlockId.get('blk')).toBe('custom-path') + // Nothing to decide: the block already serves a URL, so it offers no alternatives. + expect(plan.slots[0].adoptablePaths).toEqual([]) + }) + + /** + * The reported bug: a trigger deleted and re-added in the source re-keys the target block, which + * used to mint a new URL. The single arriving trigger now adopts the retiring URL instead. + */ + it('adopts a retiring URL onto the single arriving trigger that replaces it', () => { + const { pathByTargetBlockId, changes, plan } = run( + { blk2: { type: 'slack_webhook', name: 'Slack v2' } }, + webhooks([['blk1', { path: 'blk1', workflowId: 'wf-tgt', provider: 'slack' }]]) + ) + expect(plan.slots[0].defaultAdoptPath).toBe('blk1') + expect(pathByTargetBlockId.get('blk2')).toBe('blk1') + // Adopted means still served — there is nothing for the user to re-register. + expect(changes).toEqual([]) + }) + + it('reports a removal when the trigger is gone from the source entirely', () => { + vi.mocked(getBlock).mockReturnValue({ ...TRIGGER_BLOCK, category: 'blocks' } as never) + const { pathByTargetBlockId, changes } = run( + { fn: { type: 'function', name: 'Fn' } }, + webhooks([['blk1', { path: 'blk1', workflowId: 'wf-tgt', provider: 'slack' }]]) + ) + expect(changes).toEqual([{ workflowName: 'Prod', path: 'blk1' }]) + expect(pathByTargetBlockId.size).toBe(0) + }) + + it('does not guess a pairing when several URLs retire at once', () => { + const { pathByTargetBlockId, changes, plan } = run( + { blk3: { type: 'slack_webhook', name: 'Slack' } }, + webhooks([ + ['blk1', { path: 'blk1', workflowId: 'wf-tgt', provider: 'slack' }], + ['blk2', { path: 'blk2', workflowId: 'wf-tgt', provider: 'slack' }], + ]) + ) + expect(plan.slots[0].defaultAdoptPath).toBeNull() + // Both are offered, so the user can resolve the ambiguity; neither is taken by default. + expect(plan.slots[0].adoptablePaths).toEqual(['blk1', 'blk2']) + expect(pathByTargetBlockId.size).toBe(0) + expect(changes.map((change) => change.path)).toEqual(['blk1', 'blk2']) + }) + + it('honours an explicit pick when the pairing is ambiguous', () => { + const { pathByTargetBlockId, changes } = run( + { blk3: { type: 'slack_webhook', name: 'Slack' } }, + webhooks([ + ['blk1', { path: 'blk1', workflowId: 'wf-tgt', provider: 'slack' }], + ['blk2', { path: 'blk2', workflowId: 'wf-tgt', provider: 'slack' }], + ]), + [{ sourceBlockId: 'blk3', adoptPath: 'blk2' }] + ) + expect(pathByTargetBlockId.get('blk3')).toBe('blk2') + expect(changes.map((change) => change.path)).toEqual(['blk1']) + }) + + it('lets an explicit null override the default and mint a new URL', () => { + const { pathByTargetBlockId, changes } = run( + { blk2: { type: 'slack_webhook', name: 'Slack v2' } }, + webhooks([['blk1', { path: 'blk1', workflowId: 'wf-tgt', provider: 'slack' }]]), + [{ sourceBlockId: 'blk2', adoptPath: null }] + ) + expect(pathByTargetBlockId.size).toBe(0) + expect(changes).toEqual([{ workflowName: 'Prod', path: 'blk1' }]) + }) + + /** A crafted payload must not be able to move a URL the plan never offered. */ + it('ignores an override naming a path this slot does not offer', () => { + const { pathByTargetBlockId } = run( + { blk2: { type: 'slack_webhook', name: 'Slack v2' } }, + webhooks([['blk1', { path: 'blk1', workflowId: 'wf-tgt', provider: 'slack' }]]), + [{ sourceBlockId: 'blk2', adoptPath: 'a-path-from-another-workspace' }] + ) + expect(pathByTargetBlockId.size).toBe(0) + }) + + it('never lets two triggers adopt the same path', () => { + const { pathByTargetBlockId } = run( + { + blk2: { type: 'slack_webhook', name: 'Slack A' }, + blk3: { type: 'slack_webhook', name: 'Slack B' }, + }, + webhooks([['blk1', { path: 'blk1', workflowId: 'wf-tgt', provider: 'slack' }]]), + [ + { sourceBlockId: 'blk2', adoptPath: 'blk1' }, + { sourceBlockId: 'blk3', adoptPath: 'blk1' }, + ] + ) + expect(Array.from(pathByTargetBlockId.values())).toEqual(['blk1']) + expect(pathByTargetBlockId.get('blk2')).toBe('blk1') + }) + + /** + * A path is authenticated and parsed as its provider. Handing a GitHub URL to an arriving Slack + * trigger would keep the endpoint alive while every request failed signature verification — and + * the sync would have reported the URL as preserved, so nobody would go looking. + */ + it('never offers a retiring URL from a DIFFERENT provider', () => { + const { plan, pathByTargetBlockId, changes } = run( + { blk2: { type: 'slack_webhook', name: 'Slack v2' } }, + webhooks([['blk1', { path: 'blk1', workflowId: 'wf-tgt', provider: 'github' }]]) + ) + expect(plan.slots[0].adoptablePaths).toEqual([]) + expect(plan.slots[0].defaultAdoptPath).toBeNull() + expect(pathByTargetBlockId.size).toBe(0) + // Still reported as lost, so the GitHub subscription's owner is told it stopped serving. + expect(changes).toEqual([{ workflowName: 'Prod', path: 'blk1' }]) + }) + + it('pairs only within the matching provider when several URLs retire', () => { + const { plan, pathByTargetBlockId } = run( + { blk3: { type: 'slack_webhook', name: 'Slack' } }, + webhooks([ + ['blk1', { path: 'blk1', workflowId: 'wf-tgt', provider: 'github' }], + ['blk2', { path: 'blk2', workflowId: 'wf-tgt', provider: 'slack' }], + ]) + ) + // Only the same-provider URL is a candidate, which makes the pairing unambiguous again. + expect(plan.slots[0].adoptablePaths).toEqual(['blk2']) + expect(pathByTargetBlockId.get('blk3')).toBe('blk2') + }) + + /** + * Adoption is scoped to one target workflow because `webhook_path_claim` ownership is + * per-workflow: taking a path from another workflow would be an ownership transfer the claim + * layer refuses, so it must never be offered. + */ + it('never offers a path owned by a different workflow', () => { + const { plan, pathByTargetBlockId, changes } = run( + { blk: { type: 'slack_webhook', name: 'Slack' } }, + webhooks([['other', { path: 'other', workflowId: 'wf-elsewhere', provider: 'slack' }]]) + ) + expect(plan.slots[0].adoptablePaths).toEqual([]) + expect(pathByTargetBlockId.size).toBe(0) + expect(changes).toEqual([]) + }) + + it('skips a non-trigger block arriving on a target block with no webhook', () => { + vi.mocked(getBlock).mockReturnValue({ ...TRIGGER_BLOCK, category: 'blocks' } as never) + const { plan } = run( + { fn: { type: 'function', name: 'Fn' } }, + webhooks([['blk1', { path: 'blk1', workflowId: 'wf-tgt', provider: 'slack' }]]) + ) + expect(plan.slots).toEqual([]) + }) + + /** + * A poller or schedule trigger has no public URL, so handing it a retiring one would point an + * external caller at a path its provider never serves. + */ + it('never offers a retiring URL to a trigger that serves no public URL', () => { + vi.mocked(getBlock).mockReturnValue(URL_LESS_TRIGGER_BLOCK as never) + const { plan, pathByTargetBlockId, changes } = run( + { poller: { type: 'gmail', name: 'Gmail poller' } }, + webhooks([['blk1', { path: 'blk1', workflowId: 'wf-tgt', provider: 'slack' }]]) + ) + expect(plan.slots).toEqual([]) + expect(pathByTargetBlockId.size).toBe(0) + // The URL still retires and is still reported - it just has no eligible adopter. + expect(changes).toEqual([{ workflowName: 'Prod', path: 'blk1' }]) + }) +}) diff --git a/apps/sim/ee/workspace-forking/lib/promote/trigger-urls.ts b/apps/sim/ee/workspace-forking/lib/promote/trigger-urls.ts new file mode 100644 index 00000000000..e02b46198ce --- /dev/null +++ b/apps/sim/ee/workspace-forking/lib/promote/trigger-urls.ts @@ -0,0 +1,195 @@ +import type { ForkTargetWebhook } from '@/ee/workspace-forking/lib/copy/deploy-bridge' +import type { ForkPromotePlanItem } from '@/ee/workspace-forking/lib/promote/promote-plan' +import type { ForkBlockIdResolver } from '@/ee/workspace-forking/lib/remap/block-identity' +import type { BlockState, WorkflowState } from '@/stores/workflows/workflow/types' +import { blockAdvertisesWebhookUrl, resolveBlockTriggerProvider } from '@/triggers/webhook-url' + +/** + * A public trigger URL a sync stops serving in the target. + * + * Only URLs that genuinely go away are reported: one an arriving trigger adopts keeps serving the + * same path, so it is not a change. Whatever calls this path externally - a Slack Request URL, a + * provider subscription - stops being called and has to be repointed by hand. + */ +export interface ForkTriggerUrlChange { + workflowName: string + path: string +} + +/** + * One arriving trigger block whose public URL this sync decides. + * + * A target webhook's path is `triggerPath || block.id`, and a sync assigns target block ids from + * the SOURCE's block identity - so a trigger re-created in the source re-keys its target block and + * moves the URL. `ownPath` is the stable case (the block already serves a URL, which is pinned + * back verbatim); `adoptablePaths` is the decision, listing URLs retiring in the SAME target + * workflow that this block can take over instead of minting a new one. + */ +export interface ForkTriggerSlot { + sourceBlockId: string + targetBlockId: string + blockName: string + workflowName: string + /** The path this block already serves. Pinned as-is; there is no decision to make. */ + ownPath: string | null + /** Retiring paths in the same target workflow this block could take over instead. */ + adoptablePaths: string[] + /** The unambiguous pairing (exactly one retiring URL, exactly one arriving trigger). */ + defaultAdoptPath: string | null +} + +/** Every trigger decision a sync makes, plus the URLs it would retire. */ +export interface ForkTriggerPlan { + slots: ForkTriggerSlot[] + /** Live target webhooks on blocks this sync will not write - their URLs stop being served. */ + retiring: Array<{ path: string; workflowName: string }> +} + +/** A caller's explicit choice of which retiring URL an arriving trigger takes over. */ +export interface ForkTriggerMappingInput { + sourceBlockId: string + /** A path from that slot's `adoptablePaths`, or null to mint a new URL. */ + adoptPath: string | null +} + +/** + * Work out, per target workflow, which trigger URLs retire and which arriving triggers could + * take them over. + * + * Adoption is deliberately scoped to a SINGLE target workflow. `webhook_path_claim` ownership is + * per-workflow (`claimWebhookPath` conflicts only against a *different* workflow), so moving a + * path between blocks of the same workflow re-uses a claim that workflow already holds and can + * never conflict. Offering a path from another workflow would be a genuine ownership transfer, + * which the claim layer refuses by design - so it is never a candidate. + * + * Pure over the pre-read source states, so the preview and the write agree by construction. + */ +export function buildForkTriggerPlan(params: { + items: ForkPromotePlanItem[] + sourceStates: Map + resolveBlockId: ForkBlockIdResolver + targetWebhooks: ReadonlyMap +}): ForkTriggerPlan { + const { items, sourceStates, resolveBlockId, targetWebhooks } = params + + const liveByWorkflow = new Map< + string, + Array<{ blockId: string; path: string; provider: string | null }> + >() + for (const [blockId, row] of targetWebhooks) { + const entry = { blockId, path: row.path, provider: row.provider } + const list = liveByWorkflow.get(row.workflowId) + if (list) list.push(entry) + else liveByWorkflow.set(row.workflowId, [entry]) + } + + const slots: ForkTriggerSlot[] = [] + const retiring: ForkTriggerPlan['retiring'] = [] + + for (const item of items) { + const sourceState = sourceStates.get(item.sourceWorkflowId) + if (!sourceState) continue + + const sourceByTargetBlockId = new Map() + for (const [sourceBlockId, block] of Object.entries(sourceState.blocks)) { + sourceByTargetBlockId.set(resolveBlockId(item.targetWorkflowId, sourceBlockId), { + sourceBlockId, + block, + }) + } + + // A live webhook on a block this sync will not write: its URL stops being served. + const live = liveByWorkflow.get(item.targetWorkflowId) ?? [] + const retired = live.filter((row) => !sourceByTargetBlockId.has(row.blockId)) + for (const row of retired) { + retiring.push({ path: row.path, workflowName: item.sourceMeta.name }) + } + + const arriving: ForkTriggerSlot[] = [] + for (const [targetBlockId, { sourceBlockId, block }] of sourceByTargetBlockId) { + // Only a block that advertises a public URL can hold one. Handing a retiring URL to a + // poller or a shared-app trigger would point an external caller at a path its provider + // never serves - so those are not candidates, and never appear as rows. + if (!blockAdvertisesWebhookUrl(block)) continue + const ownPath = targetWebhooks.get(targetBlockId)?.path ?? null + // Only a retiring URL of the SAME provider is adoptable. A path is authenticated and parsed + // as its provider, so handing a GitHub URL to a Slack trigger would keep the endpoint alive + // while every request failed signature verification - and the sync would have reported the + // URL as preserved, so nobody would go looking. + const provider = resolveBlockTriggerProvider(block) + arriving.push({ + sourceBlockId, + targetBlockId, + blockName: block.name, + workflowName: item.sourceMeta.name, + ownPath, + // A block already serving a URL keeps it; only a block without one is a candidate to + // adopt, so offering it a second URL would just be a way to break the first. + adoptablePaths: + ownPath === null && provider !== null + ? retired.filter((row) => row.provider === provider).map((row) => row.path) + : [], + defaultAdoptPath: null, + }) + } + + // Default only the unambiguous pairing, and only within one provider: with several retiring or + // several arriving, guessing which new trigger replaces which old URL would silently point an + // external caller at the wrong workflow branch - the user picks instead. + const adopters = arriving.filter((slot) => slot.adoptablePaths.length > 0) + if (adopters.length === 1 && adopters[0].adoptablePaths.length === 1) { + adopters[0].defaultAdoptPath = adopters[0].adoptablePaths[0] + } + slots.push(...arriving) + } + + return { slots, retiring } +} + +/** + * Resolve every trigger block's final path, applying the caller's explicit choices over the + * plan's defaults, and report the URLs that still retire. + * + * An override is honoured only for a path the slot actually offered (same target workflow, still + * retiring), and each path can be adopted once - so a crafted payload can neither move a URL + * across workflows nor point two triggers at one path (which the unique webhook path index would + * reject at deploy time anyway, failing the whole sync). + */ +export function resolveForkTriggerPaths( + plan: ForkTriggerPlan, + overrides: readonly ForkTriggerMappingInput[] = [] +): { + /** Target block id -> the path to pin into its `triggerPath`. */ + pathByTargetBlockId: Map + changes: ForkTriggerUrlChange[] +} { + const overrideBySourceBlockId = new Map( + overrides.map((entry) => [entry.sourceBlockId, entry.adoptPath]) + ) + + const pathByTargetBlockId = new Map() + const adopted = new Set() + + for (const slot of plan.slots) { + if (slot.ownPath !== null) { + pathByTargetBlockId.set(slot.targetBlockId, slot.ownPath) + continue + } + const requested = overrideBySourceBlockId.has(slot.sourceBlockId) + ? overrideBySourceBlockId.get(slot.sourceBlockId)! + : slot.defaultAdoptPath + if (requested === null || requested === undefined) continue + if (!slot.adoptablePaths.includes(requested)) continue + if (adopted.has(requested)) continue + adopted.add(requested) + pathByTargetBlockId.set(slot.targetBlockId, requested) + } + + const changes: ForkTriggerUrlChange[] = [] + for (const row of plan.retiring) { + // An adopted path keeps serving the same URL, so it is not a change to warn about. + if (adopted.has(row.path)) continue + changes.push({ workflowName: row.workflowName, path: row.path }) + } + return { pathByTargetBlockId, changes } +} 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 dbc1a95ac2a..10d16ab6841 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 @@ -1300,6 +1300,64 @@ describe('canonical mode policy (fork/promote)', () => { expect(scan.references).toEqual([]) }) + /** + * Every shipped canonical pair's advanced member is a plain `short-input`, which carries no + * resource definition — so the "advanced is user-owned, verbatim" policy has never been + * exercised against an advanced member that IS a resource selector. Pin it here so the + * policy holds by enforcement rather than by the accident of the current block configs. + */ + const selectorPairBlock = () => + blockWith([ + { + id: 'tableSelector', + title: 'Table', + type: 'table-selector', + canonicalParamId: 'tableId', + mode: 'basic', + }, + { + id: 'advancedTableSelector', + title: 'Table (advanced)', + type: 'table-selector', + canonicalParamId: 'tableId', + mode: 'advanced', + }, + ]) + + it('advanced mode: a selector-typed manual member is neither remapped nor detected', () => { + vi.mocked(getBlock).mockReturnValue(selectorPairBlock()) + const resolveTable = (kind: string, id: string) => + kind === 'table' && id === 'tbl-manual' ? 'tbl-copy' : null + const transform = createForkBootstrapTransform(resolveTable as never) + const result = transform( + { + tableSelector: entry('tableSelector', 'table-selector', 'tbl-basic'), + advancedTableSelector: entry('advancedTableSelector', 'table-selector', 'tbl-manual'), + }, + 'table', + { tableId: 'advanced' } + ) + expect(result.advancedTableSelector.value).toBe('tbl-manual') + expect(result.tableSelector.value).toBe('') + + const scan = scanWorkflowReferences( + [ + { + id: 'b1', + name: 'Table', + type: 'table', + subBlocks: { + tableSelector: entry('tableSelector', 'table-selector', 'tbl-basic'), + advancedTableSelector: entry('advancedTableSelector', 'table-selector', 'tbl-manual'), + }, + canonicalModes: { tableId: 'advanced' }, + }, + ], + () => null + ) + expect(scan.references).toEqual([]) + }) + it('does not detect a condition-hidden subblock (its value never executes)', () => { vi.mocked(getBlock).mockReturnValue( blockWith([ 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 60b2663f57a..d419669f296 100644 --- a/apps/sim/ee/workspace-forking/lib/remap/remap-references.ts +++ b/apps/sim/ee/workspace-forking/lib/remap/remap-references.ts @@ -859,14 +859,20 @@ export function remapForkSubBlocks( // under a MANUAL (advanced-active) parent passes through verbatim; a condition-hidden // subblock is rewritten but never detected. const dormant = gates.isDormantMember(subBlockKey) - const verbatimManualDependent = !dormant && gates.isManualParentDependent(subBlockKey) - const detectionSkipped = - dormant || verbatimManualDependent || gates.isConditionHidden(subBlockKey) + // Verbatim (user-owned: never remapped, never a mapping requirement) covers the ACTIVE + // advanced member itself as well as every dependent scoped to it. `clearDependentsOnRemap` + // already spares an active manual member from a parent remap; naming it here applies the + // same policy on the detect/rewrite side, which until now held only because every shipped + // pair's advanced member is a plain `short-input` carrying no resource definition. + const verbatimManual = + !dormant && + (gates.isActiveManualMember(subBlockKey) || gates.isManualParentDependent(subBlockKey)) + const detectionSkipped = dormant || verbatimManual || gates.isConditionHidden(subBlockKey) if (dormant && isNonEmptyValue(value)) { value = '' } - if (definition && forkKind && subBlockType && !verbatimManualDependent) { + if (definition && forkKind && subBlockType && !verbatimManual) { const parsed = parseWorkflowSearchSubBlockResources(value, { type: subBlockType as SubBlockType, }) diff --git a/apps/sim/lib/api/contracts/workspace-fork.test.ts b/apps/sim/lib/api/contracts/workspace-fork.test.ts index 9a72c30fc0f..03ebc5d2326 100644 --- a/apps/sim/lib/api/contracts/workspace-fork.test.ts +++ b/apps/sim/lib/api/contracts/workspace-fork.test.ts @@ -8,6 +8,7 @@ import { forkMappableResourceTypeSchema, getForkDiffContract, getWorkspaceBackgroundWorkQuerySchema, + promoteForkBodySchema, updateForkExcludedWorkflowsBodySchema, updateForkMappingBodySchema, } from '@/lib/api/contracts/workspace-fork' @@ -187,6 +188,54 @@ describe('getForkDiffContract response excluded-workflow lists', () => { const parsed = getForkDiffContract.response.schema.parse(baseDiffResponse) expect(parsed.excludedSourceWorkflows).toEqual([]) expect(parsed.excludedTargetWorkflows).toEqual([]) + expect(parsed.triggerUrlChanges).toEqual([]) + expect(parsed.triggerMappings).toEqual([]) + }) + + it('carries every trigger, whether or not its URL is up for decision', () => { + const parsed = getForkDiffContract.response.schema.parse({ + ...baseDiffResponse, + triggerMappings: [ + // Already serving a URL: informational, no choice offered. + { + sourceBlockId: 'blk-stable', + blockName: 'Prod intake', + workflowName: 'ITSM intake', + ownPath: 'prod-live-path', + adoptablePaths: [], + defaultAdoptPath: null, + }, + // Arriving without one, with a retiring URL it can take over. + { + sourceBlockId: 'blk-new', + blockName: 'Slack messages', + workflowName: 'ITSM intake', + ownPath: null, + adoptablePaths: ['live-slack-path'], + defaultAdoptPath: 'live-slack-path', + }, + ], + triggerUrlChanges: [{ workflowName: 'ITSM intake', path: 'dead-path' }], + }) + expect(parsed.triggerMappings[0].ownPath).toBe('prod-live-path') + expect(parsed.triggerMappings[0].adoptablePaths).toEqual([]) + expect(parsed.triggerMappings[1].defaultAdoptPath).toBe('live-slack-path') + expect(parsed.triggerUrlChanges[0].path).toBe('dead-path') + }) + + it('accepts a trigger mapping choice on the promote body, including "new URL"', () => { + const parsed = promoteForkBodySchema.parse({ + otherWorkspaceId: 'ws-other', + direction: 'push', + triggerMappings: [ + { sourceBlockId: 'blk-a', adoptPath: 'keep-this-path' }, + { sourceBlockId: 'blk-b', adoptPath: null }, + ], + }) + expect(parsed.triggerMappings).toEqual([ + { sourceBlockId: 'blk-a', adoptPath: 'keep-this-path' }, + { sourceBlockId: 'blk-b', adoptPath: null }, + ]) }) it('carries the lists when present', () => { diff --git a/apps/sim/lib/api/contracts/workspace-fork.ts b/apps/sim/lib/api/contracts/workspace-fork.ts index 6a62bb4c15d..5546100fdcf 100644 --- a/apps/sim/lib/api/contracts/workspace-fork.ts +++ b/apps/sim/lib/api/contracts/workspace-fork.ts @@ -212,6 +212,14 @@ export const forkMappingEntrySchema = z.object({ /** True when `targetId` is an unconfirmed auto-suggestion (no persisted mapping yet). */ suggested: z.boolean(), required: z.boolean(), + /** + * True when the referenced resource no longer exists in the SOURCE workspace, so `sourceLabel` + * falls back to the raw id. Checked by exact id (never the capped candidate list), so this is + * unambiguous: a live resource always resolves its name, however many the workspace has. Such a + * reference cannot be offered for copy - there is nothing to copy - so the resolutions are + * mapping it to a live target, fixing the block in the source, or dropping it. + */ + sourceDeleted: z.boolean(), candidates: z.array(forkMappingCandidateSchema), /** * True when the target workspace has more candidates of this kind than the picker @@ -486,6 +494,53 @@ export const getForkDiffQuerySchema = z.object({ otherWorkspaceId: workspaceIdSchema, direction: forkDirectionSchema, }) +/** + * A public trigger URL a sync would stop serving in the target. Surfaced before the overwrite is + * confirmed, because the external system calling it - a Slack Request URL, a provider webhook + * subscription - has to be repointed by hand afterwards. + */ +export const forkTriggerUrlChangeSchema = z.object({ + workflowName: z.string(), + /** The path that stops being served. A URL an arriving trigger adopts is not reported here. */ + path: z.string(), +}) +export type ForkTriggerUrlChange = z.output + +/** + * One trigger block in this sync that has a public webhook URL, or whose URL is up for decision. + * + * Both cases get an entry, not just the decisions, so the Trigger URLs section reads as a + * standing statement of each URL rather than an alert that appears only when something is wrong. + * A trigger already serving one reports it as `ownPath` and keeps it - `adoptablePaths` is empty + * and the row is informational. A trigger arriving without one lists the URLs retiring in the + * SAME target workflow; picking one hands that live URL to the new block, so the external caller + * - a Slack Request URL, a provider webhook subscription - keeps working untouched. + * + * Triggers with neither are absent, because whether a block serves a URL at all is only knowable + * from its webhook row: a schedule, chat, manual or poller trigger never gets one, and no + * declarative flag on the trigger definition separates them cleanly. + */ +export const forkTriggerMappingSchema = z.object({ + /** The SOURCE block id - stable across the sync, and what a chosen mapping is keyed by. */ + sourceBlockId: z.string(), + blockName: z.string(), + workflowName: z.string(), + /** + * The URL path this trigger already serves in the target, which the sync preserves verbatim. + * Null when the target block has no webhook yet, i.e. the sync decides its URL. + */ + ownPath: z.string().nullable(), + /** + * Retiring URLs in the same target workflow this block may take over instead of minting a new + * one. Always empty when `ownPath` is set: a trigger that already serves a URL keeps it, and + * offering it a second one would only be a way to abandon the first. + */ + adoptablePaths: z.array(z.string()), + /** The pre-selected pairing: unambiguous only when one URL retires and one trigger arrives. */ + defaultAdoptPath: z.string().nullable(), +}) +export type ForkTriggerMapping = z.output + export const getForkDiffContract = defineRouteContract({ method: 'GET', path: '/api/workspaces/[id]/fork/diff', @@ -548,6 +603,13 @@ export const getForkDiffContract = defineRouteContract({ * always clear (informational). */ clearedRefs: z.array(forkClearedRefSchema), + /** + * Public trigger URLs this sync would stop serving in the target. Defaulted so a new client + * tolerates an old server's response during rollout. + */ + triggerUrlChanges: z.array(forkTriggerUrlChangeSchema).default([]), + /** Arriving trigger blocks whose URL this sync decides, with their adoptable alternatives. */ + triggerMappings: z.array(forkTriggerMappingSchema).default([]), }), }, }) @@ -598,6 +660,27 @@ export const promoteForkBodySchema = z.object({ dependentValues: z.array(forkDependentValueEntrySchema).max(2000).optional(), /** Referenced-but-unmapped resources to copy into the target before the sync gate (U17). */ copyResources: promoteCopyResourcesSchema.optional(), + /** + * References the user explicitly acknowledged dropping, so the sync may clear them in the + * target instead of blocking. Honoured ONLY for a reference whose resource no longer exists in + * the source workspace - the server re-derives that liveness inside the promote transaction and + * ignores an acknowledgment for anything still live, so a working reference can never be + * dropped and the zero-cleared-refs invariant relaxes only where the source is already broken. + */ + dropReferences: z + .array(z.object({ kind: forkRemapKindSchema, sourceId: z.string().min(1) })) + .max(2000) + .optional(), + /** + * Which retiring public URL each arriving trigger takes over, overriding the unambiguous + * default. `adoptPath: null` means "mint a new URL for this trigger". The server re-derives the + * adoptable set inside the promote transaction and ignores a path that slot did not offer, so a + * URL can never be moved between workflows (which the per-workflow path claim forbids anyway). + */ + triggerMappings: z + .array(z.object({ sourceBlockId: z.string().min(1), adoptPath: z.string().min(1).nullable() })) + .max(500) + .optional(), }) export const promoteForkContract = defineRouteContract({ method: 'POST', @@ -624,6 +707,20 @@ export const promoteForkContract = defineRouteContract({ needsConfiguration: z.array(forkNeedsConfigurationSchema), /** Workflows whose optional dependent fields a swap cleared (surfaced, not gated). */ clearedOptional: z.array(forkNeedsConfigurationSchema), + /** + * Acknowledged source-deleted references this sync cleared in the target instead of + * blocking on. Only entries whose source was verified gone in-transaction appear here, so + * an acknowledgment the server refused is visibly absent. + */ + droppedReferences: z + .array(z.object({ kind: forkRemapKindSchema, sourceId: z.string() })) + .default([]), + /** + * Public trigger URLs this sync stopped serving, because no arriving trigger adopted them. + * Reported after the fact so the post-sync toast can name what needs re-registering. + * Defaulted alongside the rest, so an old server's response still parses. + */ + triggerUrlChanges: z.array(forkTriggerUrlChangeSchema).default([]), }), }, }) @@ -683,6 +780,10 @@ export const backgroundWorkMetadataSchema = z needsConfiguration: z.array(forkNeedsConfigurationSchema).optional(), /** Workflows whose optional dependent fields a sync cleared (FYI, non-blocking). */ clearedOptional: z.array(forkNeedsConfigurationSchema).optional(), + /** How many source-deleted references the operator explicitly dropped in this sync. */ + droppedReferences: z.number().int().optional(), + /** How many public trigger URLs this sync stopped serving. */ + triggerUrlChanges: z.number().int().optional(), }) .nullable() export const backgroundWorkItemSchema = z.object({ diff --git a/apps/sim/lib/webhooks/deploy.test.ts b/apps/sim/lib/webhooks/deploy.test.ts index da0f6f4a4f0..4aa780ceeac 100644 --- a/apps/sim/lib/webhooks/deploy.test.ts +++ b/apps/sim/lib/webhooks/deploy.test.ts @@ -10,7 +10,12 @@ import type { BlockState } from '@/stores/workflows/workflow/types' // deploy.ts pulls in the trigger/block/provider registries at module load; none are exercised by // buildProviderConfig (a pure function), so stub them to keep this unit test fast and isolated. -vi.mock('@/blocks', () => ({ getBlock: vi.fn() })) +const { mockGetBlock } = vi.hoisted(() => ({ mockGetBlock: vi.fn() })) +// `deploy.ts` reads the registry through `@/blocks`, while the trigger-id resolution it now +// shares (`@/triggers/webhook-url`) reads `@/blocks/registry`. Point both specifiers at ONE spy +// so a test configuring the block config governs the whole path, not half of it. +vi.mock('@/blocks', () => ({ getBlock: mockGetBlock })) +vi.mock('@/blocks/registry', () => ({ getBlock: mockGetBlock })) vi.mock('@/triggers', () => ({ getTrigger: vi.fn(), isTriggerValid: vi.fn(() => true) })) vi.mock('@/lib/webhooks/providers', () => ({ getProviderHandler: vi.fn() })) vi.mock('@/lib/webhooks/provider-subscriptions', () => ({ diff --git a/apps/sim/lib/webhooks/deploy.ts b/apps/sim/lib/webhooks/deploy.ts index b5e04b99df2..52e3e5244d2 100644 --- a/apps/sim/lib/webhooks/deploy.ts +++ b/apps/sim/lib/webhooks/deploy.ts @@ -32,12 +32,12 @@ import { refreshAccessTokenIfNeeded, resolveOAuthAccountId, } from '@/app/api/auth/oauth/utils' -import { getBlock } from '@/blocks' import type { SubBlockConfig } from '@/blocks/types' import type { BlockState } from '@/stores/workflows/workflow/types' import { getTrigger, isTriggerValid } from '@/triggers' import { SYSTEM_SUBBLOCK_IDS } from '@/triggers/constants' import { SIM_SUBSCRIBED_EVENTS } from '@/triggers/slack/shared' +import { resolveBlockTriggerId } from '@/triggers/webhook-url' const logger = createLogger('DeployWebhookSync') @@ -75,7 +75,7 @@ export async function validateTriggerWebhookConfigForDeploy( const triggerBlocks = Object.values(blocks || {}).filter((b) => b && b.enabled !== false) for (const block of triggerBlocks) { - const triggerId = resolveTriggerId(block) + const triggerId = resolveBlockTriggerId(block) if (!triggerId || !isTriggerValid(triggerId)) continue const triggerDef = getTrigger(triggerId) @@ -172,43 +172,6 @@ function isFieldRequired( return evalCond(condition, subBlockValues) } -function resolveTriggerId(block: BlockState): string | undefined { - const blockConfig = getBlock(block.type) - - if (blockConfig?.category === 'triggers' && isTriggerValid(block.type)) { - return block.type - } - - if (!block.triggerMode) { - return undefined - } - - const selectedTriggerId = getSubBlockValue(block, 'selectedTriggerId') - if (typeof selectedTriggerId === 'string' && isTriggerValid(selectedTriggerId)) { - return selectedTriggerId - } - - const storedTriggerId = getSubBlockValue(block, 'triggerId') - if (typeof storedTriggerId === 'string' && isTriggerValid(storedTriggerId)) { - return storedTriggerId - } - - if (blockConfig?.triggers?.enabled) { - const configuredTriggerId = - typeof selectedTriggerId === 'string' ? selectedTriggerId : undefined - if (configuredTriggerId && isTriggerValid(configuredTriggerId)) { - return configuredTriggerId - } - - const available = blockConfig.triggers?.available?.[0] - if (available && isTriggerValid(available)) { - return available - } - } - - return undefined -} - function getConfigValue(block: BlockState, subBlock: SubBlockConfig): unknown { const fieldValue = getSubBlockValue(block, subBlock.id) @@ -372,7 +335,7 @@ export async function resolveWebhookConfigForBlock(input: { userId: string requestId: string }): Promise { - const triggerId = resolveTriggerId(input.block) + const triggerId = resolveBlockTriggerId(input.block) if (!triggerId || !isTriggerValid(triggerId)) return null const triggerDef = getTrigger(triggerId) diff --git a/apps/sim/lib/workflows/credentials/credential-extractor.test.ts b/apps/sim/lib/workflows/credentials/credential-extractor.test.ts new file mode 100644 index 00000000000..7f609589d0e --- /dev/null +++ b/apps/sim/lib/workflows/credentials/credential-extractor.test.ts @@ -0,0 +1,96 @@ +/** + * @vitest-environment node + */ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { + EXPORT_PRESERVED_RESOURCE_TYPES, + sanitizeForExport, +} from '@/lib/workflows/credentials/credential-extractor' +import { WORKFLOW_SEARCH_SUBBLOCK_RESOURCE_TYPES } from '@/lib/workflows/search-replace/resources/registry' +import { getBlock } from '@/blocks/registry' +import type { WorkflowState } from '@/stores/workflows/workflow/types' + +function stateWithSubBlock(type: string, value: unknown): Partial { + return { + blocks: { + b1: { + id: 'b1', + type: 'test-block', + name: 'Test', + position: { x: 0, y: 0 }, + subBlocks: { field: { id: 'field', type, value } }, + outputs: {}, + enabled: true, + }, + }, + } as unknown as Partial +} + +function sanitizedValue(type: string, value: unknown): unknown { + vi.mocked(getBlock).mockReturnValue({ + name: 'Test', + description: '', + subBlocks: [{ id: 'field', title: 'Field', type }], + outputs: {}, + } as never) + const sanitized = sanitizeForExport(stateWithSubBlock(type, value)) + return sanitized.blocks?.b1?.subBlocks?.field?.value +} + +describe('export sanitizer resource coverage', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + /** + * The drift guard. Adding a selector to the resource registry without deciding how export + * should treat it fails here rather than silently shipping a workspace-scoped id to another + * workspace — which is exactly how raw `tbl_…` table ids used to escape. + */ + it.each( + WORKFLOW_SEARCH_SUBBLOCK_RESOURCE_TYPES.filter( + (type) => !EXPORT_PRESERVED_RESOURCE_TYPES.has(type) + ) + )('clears %s on export', (type) => { + expect(sanitizedValue(type, 'res-id-123')).toBeNull() + }) + + it('clears a table-selector id, the omission that leaked ids across workspaces', () => { + expect(sanitizedValue('table-selector', 'tbl_239e870374c14d4a89923175a7b10648')).toBeNull() + }) + + /** + * Nothing on the import path remaps workflow references — `import-export.ts` extracts each + * workflow independently under a fresh id — so a preserved reference names a workflow that does + * not exist in the target, bundle or not. + */ + it('clears workflow-selector, since import never remaps the id it names', () => { + expect(sanitizedValue('workflow-selector', 'wf-123')).toBeNull() + }) + + it('still clears oauth-input, via the credential rule rather than the workspace rule', () => { + expect(sanitizedValue('oauth-input', 'cred-123')).toBeNull() + }) + + it('leaves an ordinary field untouched', () => { + expect(sanitizedValue('short-input', 'plain text')).toBe('plain text') + }) + + it('clears tableId by key on a block with no registry config', () => { + vi.mocked(getBlock).mockReturnValue(undefined as never) + const sanitized = sanitizeForExport({ + blocks: { + b1: { + id: 'b1', + type: 'unknown-block', + name: 'Test', + position: { x: 0, y: 0 }, + subBlocks: { tableId: { id: 'tableId', type: 'short-input', value: 'tbl_abc' } }, + outputs: {}, + enabled: true, + }, + }, + } as unknown as Partial) + expect(sanitized.blocks?.b1?.subBlocks?.tableId?.value).toBeNull() + }) +}) diff --git a/apps/sim/lib/workflows/credentials/credential-extractor.ts b/apps/sim/lib/workflows/credentials/credential-extractor.ts index 9540c5891d7..8e9e99bd9c1 100644 --- a/apps/sim/lib/workflows/credentials/credential-extractor.ts +++ b/apps/sim/lib/workflows/credentials/credential-extractor.ts @@ -1,3 +1,4 @@ +import { WORKFLOW_SEARCH_SUBBLOCK_RESOURCE_TYPES } from '@/lib/workflows/search-replace/resources/registry' import { buildCanonicalIndex, buildSubBlockValues, @@ -28,27 +29,49 @@ export interface CredentialRequirement { required: boolean } -// Workspace-specific subblock types that should be cleared -const WORKSPACE_SPECIFIC_TYPES = new Set([ - 'knowledge-base-selector', +/** + * Resource-selector types NOT cleared by the workspace rule below. Everything else the resource + * registry knows about IS cleared, so the two lists can never drift apart again — the previous + * hand-written copy had silently omitted `table-selector`, `mcp-tool-selector`, `user-selector` + * and `sheet-selector`, which is how raw `tbl_…` ids reached other workspaces through an export. + * + * Every id in an export is workspace-scoped, and nothing on the import path remaps them: + * `import-export.ts` extracts each workflow independently and assigns it a fresh id, so a + * preserved reference points at a workflow that does not exist in the target — including inside a + * multi-workflow bundle, where the sibling it named was itself re-created under a new id. Clearing + * is therefore the only correct treatment for every id-bearing selector. + */ +export const EXPORT_PRESERVED_RESOURCE_TYPES: ReadonlySet = new Set([ + // Cleared by the dedicated `oauth-input` branch in `sanitizeWorkflowForSharing`, so excluding it + // here only avoids clearing it twice - it never survives an export. + 'oauth-input', +]) + +/** + * Sub-block types holding a reference scoped to this workspace, or to a credential that is itself + * cleared on export. Derived from the canonical resource registry plus the name/slot-based + * knowledge fields, which carry no resource id and therefore no registry entry. + */ +const WORKSPACE_SPECIFIC_TYPES: ReadonlySet = new Set([ + ...WORKFLOW_SEARCH_SUBBLOCK_RESOURCE_TYPES.filter( + (type) => !EXPORT_PRESERVED_RESOURCE_TYPES.has(type) + ), 'knowledge-tag-filters', - 'document-selector', 'document-tag-entry', - 'file-selector', // Workspace files - 'file-upload', // Uploaded files in workspace - 'project-selector', // Workspace-specific projects - 'channel-selector', // Workspace-specific channels - 'folder-selector', // User-specific folders - 'mcp-server-selector', // User-specific MCP servers ]) -// Field IDs that are workspace-specific +/** + * Field IDs that are workspace-specific, for the fallback pass over blocks with no registry + * config (and over legacy `block.data`). Keyed by sub-block / canonical param id, which the + * type-keyed registry above cannot supply, so this list stays explicit. + */ const WORKSPACE_SPECIFIC_FIELDS = new Set([ 'knowledgeBaseId', 'tagFilters', 'documentTags', 'documentId', 'fileId', + 'tableId', 'projectId', 'channelId', 'folderId', diff --git a/apps/sim/lib/workflows/persistence/duplicate.test.ts b/apps/sim/lib/workflows/persistence/duplicate.test.ts index c2a09c242ae..1952c164dec 100644 --- a/apps/sim/lib/workflows/persistence/duplicate.test.ts +++ b/apps/sim/lib/workflows/persistence/duplicate.test.ts @@ -166,6 +166,11 @@ describe('duplicateWorkflow ordering', () => { subBlocks: { triggerPath: { id: 'triggerPath', type: 'short-input', value: 'old-webhook-path' }, webhookId: { id: 'webhookId', type: 'short-input', value: 'old-webhook-id' }, + triggerConfig: { + id: 'triggerConfig', + type: 'trigger-config', + value: { tableSelector: 'tbl_stale' }, + }, webhookUrlDisplay: { id: 'webhookUrlDisplay', type: 'short-input', @@ -217,6 +222,10 @@ describe('duplicateWorkflow ordering', () => { expect(copiedSubBlocks.triggerPath).toBeUndefined() expect(copiedSubBlocks.webhookId).toBeUndefined() expect(copiedSubBlocks.webhookUrlDisplay).toBeUndefined() + // The aggregate must not ride along: `populateTriggerFieldsFromConfig` re-seeds any empty + // trigger field from it on load, so carrying it would resurrect the source's resource ids in + // the copy right after the remapper cleared or remapped them. + expect(copiedSubBlocks.triggerConfig).toBeUndefined() expect(copiedSubBlocks.variables.value[0].variableId).not.toBe('old-var-id') expect(copiedSubBlocks.variables.value[0].variableName).toBe('customerName') expect(insertedBlocks?.[0].locked).toBe(false) diff --git a/apps/sim/lib/workflows/sanitization/json-sanitizer.ts b/apps/sim/lib/workflows/sanitization/json-sanitizer.ts index 812dbb62f6f..1f05840615e 100644 --- a/apps/sim/lib/workflows/sanitization/json-sanitizer.ts +++ b/apps/sim/lib/workflows/sanitization/json-sanitizer.ts @@ -2,14 +2,11 @@ import { isRecordLike, sortObjectKeysDeep } from '@sim/utils/object' import type { Edge } from 'reactflow' import { getBaseUrl } from '@/lib/core/utils/urls' import { sanitizeWorkflowForSharing } from '@/lib/workflows/credentials/credential-extractor' -import { - buildSubBlockValues, - evaluateSubBlockCondition, -} from '@/lib/workflows/subblocks/visibility' import { getBlock } from '@/blocks/registry' import type { BlockState, Loop, Parallel, WorkflowState } from '@/stores/workflows/workflow/types' import { generateLoopBlocks, generateParallelBlocks } from '@/stores/workflows/workflow/utils' import { TRIGGER_WEBHOOK_URL_FIELD } from '@/triggers/constants' +import { blockAdvertisesWebhookUrl } from '@/triggers/webhook-url' /** * Sanitized workflow state for copilot (removes all UI-specific data) @@ -345,21 +342,7 @@ function sanitizeSubBlocks( * read time, never stored, and rejected on write by `edit_workflow` validation. */ function resolveTriggerWebhookUrl(blockId: string, block: BlockState): string | null { - const blockConfig = getBlock(block.type) - if (!blockConfig) return null - - const actsAsTrigger = blockConfig.category === 'triggers' || block.triggerMode === true - if (!actsAsTrigger) return null - - // A webhook-URL display subblock (`useWebhookUrl`) marks a webhook-based trigger. - // Multi-trigger blocks namespace one per trigger id, each gated by a condition on - // selectedTriggerId — only count a field active for the current values, so a block - // configured with a polling trigger doesn't advertise a webhook URL. - const values = buildSubBlockValues(block.subBlocks || {}) - const hasActiveWebhookUrlField = blockConfig.subBlocks.some( - (sb) => sb.useWebhookUrl === true && evaluateSubBlockCondition(sb.condition, values) - ) - if (!hasActiveWebhookUrlField) return null + if (!blockAdvertisesWebhookUrl(block)) return null const triggerPath = block.subBlocks?.triggerPath?.value const path = typeof triggerPath === 'string' && triggerPath.length > 0 ? triggerPath : blockId diff --git a/apps/sim/lib/workflows/search-replace/resources/registry.test.ts b/apps/sim/lib/workflows/search-replace/resources/registry.test.ts new file mode 100644 index 00000000000..db92f519c40 --- /dev/null +++ b/apps/sim/lib/workflows/search-replace/resources/registry.test.ts @@ -0,0 +1,72 @@ +/** + * @vitest-environment node + */ +import { describe, expect, it } from 'vitest' +import { + getWorkflowSearchSubBlockResourceDefinition, + parseWorkflowSearchSubBlockResources, + workflowSearchResourceValueContains, +} from '@/lib/workflows/search-replace/resources/registry' + +const TABLE_SUB_BLOCK = { type: 'table-selector' } as const + +function replaceTableValue(value: unknown, rawValue: string, replacement: string) { + const definition = getWorkflowSearchSubBlockResourceDefinition(TABLE_SUB_BLOCK) + if (!definition) throw new Error('table-selector is not a registered resource selector') + return definition.codec.replace(value, rawValue, replacement) +} + +/** + * `parse` and `contains` split on commas and trim, so a value carrying stray whitespace is + * detected as a reference. `replace` must agree, or the reference becomes permanently stuck: + * it is reported as needing a mapping, yet neither remapping nor clearing can ever touch it. + */ +describe('scalar resource codec whitespace handling', () => { + const padded = ' tbl_239e870374c14d4a89923175a7b10648 ' + const rawValue = 'tbl_239e870374c14d4a89923175a7b10648' + + it('detects a padded single value as a reference', () => { + const parsed = parseWorkflowSearchSubBlockResources(padded, TABLE_SUB_BLOCK) + expect(parsed.map((reference) => reference.rawValue)).toEqual([rawValue]) + expect( + workflowSearchResourceValueContains( + { subBlockType: 'table-selector', rawValue } as Parameters< + typeof workflowSearchResourceValueContains + >[0], + padded + ) + ).toBe(true) + }) + + it('remaps a padded single value to its target', () => { + expect(replaceTableValue(padded, rawValue, 'tbl_target')).toEqual({ + success: true, + nextValue: 'tbl_target', + }) + }) + + it('clears a padded single value when the reference is unresolved', () => { + expect(replaceTableValue(padded, rawValue, '')).toEqual({ success: true, nextValue: '' }) + }) + + it('leaves a non-matching single value untouched', () => { + expect(replaceTableValue(' tbl_other ', rawValue, 'tbl_target')).toEqual({ + success: true, + nextValue: ' tbl_other ', + }) + }) + + it('still remaps an unpadded single value', () => { + expect(replaceTableValue(rawValue, rawValue, 'tbl_target')).toEqual({ + success: true, + nextValue: 'tbl_target', + }) + }) + + it('still remaps one entry of a padded multi-value list', () => { + expect(replaceTableValue(` ${rawValue} , tbl_other `, rawValue, 'tbl_target')).toEqual({ + success: true, + nextValue: 'tbl_target,tbl_other', + }) + }) +}) diff --git a/apps/sim/lib/workflows/search-replace/resources/registry.ts b/apps/sim/lib/workflows/search-replace/resources/registry.ts index 8215636f339..37f19619ea4 100644 --- a/apps/sim/lib/workflows/search-replace/resources/registry.ts +++ b/apps/sim/lib/workflows/search-replace/resources/registry.ts @@ -135,7 +135,10 @@ function replaceCommaResourceValue( } return { success: true, nextValue } } - const nextValue = shouldReplace(value) ? replacement : value + // Compare the TRIMMED token, matching what `parse` and `contains` produced. Comparing the + // raw string made a padded single value (`" tbl_abc"`) unmatchable here even though it was + // detected as a reference, so it could never be remapped nor cleared and stuck forever. + const nextValue = shouldReplace(parts[0]) ? replacement : value if (targetOccurrenceIndex !== undefined && !replaced) { return { success: false, reason: 'Target resource changed since search' } } @@ -353,6 +356,16 @@ const WORKFLOW_SEARCH_SUBBLOCK_RESOURCES: Partial< 'project-selector': { kind: 'selector-resource', codec: scalarResourceCodec }, } +/** + * Every sub-block type that carries a resource reference. This registry is the single source of + * truth for "does this field hold an id scoped to a workspace or a credential", so consumers that + * need that answer derive it from here rather than keeping a parallel hand-written list — see + * `sanitizeWorkflowForSharing`, whose hand-maintained copy had silently omitted `table-selector`. + */ +export const WORKFLOW_SEARCH_SUBBLOCK_RESOURCE_TYPES = Object.keys( + WORKFLOW_SEARCH_SUBBLOCK_RESOURCES +) as SubBlockType[] + export function getWorkflowSearchResourceKindDefinition( kind: WorkflowSearchMatchKind ): WorkflowSearchResourceKindDefinition | null { diff --git a/apps/sim/triggers/webhook-url.test.ts b/apps/sim/triggers/webhook-url.test.ts new file mode 100644 index 00000000000..099fdb68cfb --- /dev/null +++ b/apps/sim/triggers/webhook-url.test.ts @@ -0,0 +1,161 @@ +/** + * @vitest-environment node + */ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { getBlock } from '@/blocks/registry' +import type { BlockState } from '@/stores/workflows/workflow/types' +import { + INTERNAL_TRIGGER_PROVIDERS, + isInternalTriggerProvider, + isPollingWebhookProvider, + POLLING_PROVIDERS, +} from '@/triggers/constants' +import { TRIGGER_REGISTRY } from '@/triggers/registry' +import { blockAdvertisesWebhookUrl } from '@/triggers/webhook-url' + +function block(overrides: Partial = {}): BlockState { + return { + id: 'blk', + type: 'slack', + name: 'Slack', + subBlocks: {}, + outputs: {}, + enabled: true, + ...overrides, + } as unknown as BlockState +} + +describe('blockAdvertisesWebhookUrl', () => { + beforeEach(() => vi.clearAllMocks()) + + it('is true for a trigger block with an unconditional webhook-URL field', () => { + vi.mocked(getBlock).mockReturnValue({ + category: 'triggers', + subBlocks: [{ id: 'triggerWebhookUrl', useWebhookUrl: true }], + } as never) + expect(blockAdvertisesWebhookUrl(block())).toBe(true) + }) + + it('is false for a trigger block that declares no webhook-URL field (poller, schedule, chat)', () => { + vi.mocked(getBlock).mockReturnValue({ + category: 'triggers', + subBlocks: [{ id: 'cron' }], + } as never) + expect(blockAdvertisesWebhookUrl(block())).toBe(false) + }) + + it('is false for a non-trigger block, even one whose config declares a URL field', () => { + vi.mocked(getBlock).mockReturnValue({ + category: 'blocks', + subBlocks: [{ id: 'triggerWebhookUrl', useWebhookUrl: true }], + } as never) + expect(blockAdvertisesWebhookUrl(block())).toBe(false) + }) + + it('is true for a tool block flipped into trigger mode', () => { + vi.mocked(getBlock).mockReturnValue({ + category: 'blocks', + subBlocks: [{ id: 'triggerWebhookUrl', useWebhookUrl: true }], + } as never) + expect(blockAdvertisesWebhookUrl(block({ triggerMode: true } as never))).toBe(true) + }) + + /** + * The case a trigger-definition flag cannot express: one block hosts several triggers, and only + * some of them serve a URL. Reading the ACTIVE condition is what keeps a block currently set to + * the polling trigger from claiming a URL its webhook sibling would have. + */ + it('honours the selectedTriggerId condition on a multi-trigger block', () => { + vi.mocked(getBlock).mockReturnValue({ + category: 'triggers', + subBlocks: [ + { + id: 'webhookUrl', + useWebhookUrl: true, + condition: { field: 'selectedTriggerId', value: 'service_webhook' }, + }, + ], + } as never) + const pollingBlock = block({ + subBlocks: { selectedTriggerId: { id: 'selectedTriggerId', value: 'service_poller' } }, + } as never) + const webhookBlock = block({ + subBlocks: { selectedTriggerId: { id: 'selectedTriggerId', value: 'service_webhook' } }, + } as never) + expect(blockAdvertisesWebhookUrl(pollingBlock)).toBe(false) + expect(blockAdvertisesWebhookUrl(webhookBlock)).toBe(true) + }) + + it('is false when the block type is not in the registry', () => { + vi.mocked(getBlock).mockReturnValue(undefined as never) + expect(blockAdvertisesWebhookUrl(block())).toBe(false) + }) +}) + +/** + * The two provider registries and the per-subblock `useWebhookUrl` marker must agree on which + * triggers serve a public URL. `POLLING_PROVIDERS` is already pinned against `polling: true` in + * `constants.test.ts`; this closes the remaining gap - a trigger whose events Sim pulls, or whose + * path the public route rejects, must never also advertise a URL to paste into a provider console. + */ +describe('provider registries agree with the webhook-URL marker', () => { + it('no polling or internal trigger declares a webhook-URL sub-block', () => { + const offenders = Object.values(TRIGGER_REGISTRY) + .filter( + (trigger) => + isPollingWebhookProvider(trigger.provider) || isInternalTriggerProvider(trigger.provider) + ) + .filter((trigger) => trigger.subBlocks.some((subBlock) => subBlock.useWebhookUrl === true)) + .map((trigger) => `${trigger.id} (provider: ${trigger.provider})`) + + expect( + offenders, + 'A polling/internal trigger advertising a webhook URL would offer a path nothing external can call' + ).toEqual([]) + }) + + it('keeps both registries non-empty, so neither guard can pass vacuously', () => { + expect(POLLING_PROVIDERS.size).toBeGreaterThan(0) + expect(INTERNAL_TRIGGER_PROVIDERS.size).toBeGreaterThan(0) + }) +}) + +/** + * Slack ships BOTH delivery families, so it is the sharpest test of the marker - and the trigger + * the fork sync's URL preservation exists for. `slack_webhook` is path-based and its URL is what + * a user pastes into a Slack app's Request URL; `slack_oauth` arrives on a shared endpoint routed + * by `routingKey`, so `lib/webhooks/deploy.ts` nulls its path and there is no URL to preserve. + * + * The block configs spread these exact arrays (`blocks/blocks/slack.ts` `...getTrigger(...) + * .subBlocks`), so asserting on the trigger definitions is asserting on what the predicate reads. + */ +describe('Slack: both delivery families classify correctly', () => { + function slackBlock(triggerId: 'slack_webhook' | 'slack_oauth'): BlockState { + vi.mocked(getBlock).mockReturnValue({ + category: 'triggers', + subBlocks: TRIGGER_REGISTRY[triggerId].subBlocks, + } as never) + return block({ type: triggerId === 'slack_webhook' ? 'slack' : 'slack_v2' }) + } + + it('slack_webhook advertises a URL, so the fork sync can preserve it', () => { + expect(blockAdvertisesWebhookUrl(slackBlock('slack_webhook'))).toBe(true) + }) + + it('slack_oauth does NOT, so it is never offered a URL it cannot serve', () => { + expect(blockAdvertisesWebhookUrl(slackBlock('slack_oauth'))).toBe(false) + }) + + /** + * The URL field must stay UNCONDITIONAL on the single-trigger Slack block. A `selectedTriggerId` + * condition would evaluate false there (no dropdown ⇒ no value), silently dropping Slack from + * the Trigger URLs section - the one trigger this feature was built for. + */ + it('slack_webhook gates its URL field on nothing', () => { + const urlField = TRIGGER_REGISTRY.slack_webhook.subBlocks.find( + (subBlock) => subBlock.useWebhookUrl === true + ) + expect(urlField).toBeDefined() + expect(urlField?.condition).toBeUndefined() + }) +}) diff --git a/apps/sim/triggers/webhook-url.ts b/apps/sim/triggers/webhook-url.ts new file mode 100644 index 00000000000..289e928a9cf --- /dev/null +++ b/apps/sim/triggers/webhook-url.ts @@ -0,0 +1,110 @@ +import { getBaseUrl } from '@/lib/core/utils/urls' +import { + buildSubBlockValues, + evaluateSubBlockCondition, +} from '@/lib/workflows/subblocks/visibility' +import { getBlock } from '@/blocks/registry' +import type { BlockState } from '@/stores/workflows/workflow/types' +import { getTrigger, isTriggerValid } from '@/triggers' + +/** The public URL an external system POSTs to for a given webhook path. */ +export function buildWebhookTriggerUrl(path: string): string { + return `${getBaseUrl()}/api/webhooks/trigger/${path}` +} + +function subBlockValue(block: BlockState, subBlockId: string): unknown { + return block.subBlocks?.[subBlockId]?.value +} + +/** + * The trigger a block deploys as, or undefined when it is not acting as one. + * + * A dedicated trigger block IS its trigger; a tool block flipped into trigger mode names one via + * `selectedTriggerId` / `triggerId`, falling back to the first trigger its config declares + * available. Single-sourced here because the webhook deploy path and anything reasoning about a + * block's delivery must agree on the answer - two resolutions would let a block deploy as one + * trigger while another layer classified it as a different one. + */ +export function resolveBlockTriggerId(block: BlockState): string | undefined { + const blockConfig = getBlock(block.type) + + if (blockConfig?.category === 'triggers' && isTriggerValid(block.type)) { + return block.type + } + + if (!block.triggerMode) { + return undefined + } + + const selectedTriggerId = subBlockValue(block, 'selectedTriggerId') + if (typeof selectedTriggerId === 'string' && isTriggerValid(selectedTriggerId)) { + return selectedTriggerId + } + + const storedTriggerId = subBlockValue(block, 'triggerId') + if (typeof storedTriggerId === 'string' && isTriggerValid(storedTriggerId)) { + return storedTriggerId + } + + if (blockConfig?.triggers?.enabled) { + const configuredTriggerId = + typeof selectedTriggerId === 'string' ? selectedTriggerId : undefined + if (configuredTriggerId && isTriggerValid(configuredTriggerId)) { + return configuredTriggerId + } + + const available = blockConfig.triggers?.available?.[0] + if (available && isTriggerValid(available)) { + return available + } + } + + return undefined +} + +/** + * The webhook provider a block's events arrive under, or null when it is not a trigger. + * + * This is the identity an inbound request is verified against: a path served by a `slack` webhook + * authenticates Slack's signature and parses Slack's event shape. Two triggers of DIFFERENT + * providers can therefore never share a URL meaningfully, however similar they look. + */ +export function resolveBlockTriggerProvider(block: BlockState): string | null { + const triggerId = resolveBlockTriggerId(block) + if (!triggerId || !isTriggerValid(triggerId)) return null + return getTrigger(triggerId).provider ?? null +} + +/** + * Whether this block advertises a public webhook URL - one an external system POSTs to at + * `/api/webhooks/trigger/`. + * + * The marker is the `useWebhookUrl` sub-block: the field that renders the copyable URL in the + * block's own config. If the UI shows a URL for a block, that is a URL someone could have pasted + * into Slack or a provider console; if it does not, there is nothing external pointing at it. + * That makes this the right question for anything reasoning about "would changing this break a + * caller" - which is why the copilot's read view and the fork sync both ask it here rather than + * re-deriving it. + * + * Deliberately NOT derived from the trigger definition. Neither declarative flag separates the + * families cleanly: `polling` is set on 8 of the trigger defs while several pollers omit it, and + * `webhook` is set on ~345 including `slack_oauth`, which routes by `routingKey` on a shared + * endpoint and has no per-workflow URL at all. + * + * Condition-aware on purpose: a multi-trigger block namespaces one URL field per trigger id, each + * gated on `selectedTriggerId`, so a block currently configured with a POLLING trigger correctly + * reports false even though its config declares a URL field for a sibling trigger. + */ +export function blockAdvertisesWebhookUrl(block: BlockState): boolean { + const blockConfig = getBlock(block.type) + if (!blockConfig) return false + + const actsAsTrigger = blockConfig.category === 'triggers' || block.triggerMode === true + if (!actsAsTrigger) return false + + const values = buildSubBlockValues(block.subBlocks || {}) + return blockConfig.subBlocks.some( + (subBlock) => + subBlock.useWebhookUrl === true && evaluateSubBlockCondition(subBlock.condition, values) + ) +}