diff --git a/apps/sim/app/api/workflows/[id]/deployed/route.test.ts b/apps/sim/app/api/workflows/[id]/deployed/route.test.ts new file mode 100644 index 00000000000..a8854c4afdf --- /dev/null +++ b/apps/sim/app/api/workflows/[id]/deployed/route.test.ts @@ -0,0 +1,202 @@ +/** + * Tests for the workflow deployed-state API route. + * Covers internal-JWT authorization (acting user required + workspace read + * permission) and the unchanged session path. + * + * @vitest-environment node + */ + +import { + workflowAuthzMockFns, + workflowsPersistenceUtilsMock, + workflowsPersistenceUtilsMockFns, + workflowsUtilsMock, + workflowsUtilsMockFns, +} from '@sim/testing' +import { NextRequest } from 'next/server' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { mockVerifyInternalToken } = vi.hoisted(() => ({ + mockVerifyInternalToken: vi.fn(), +})) + +vi.mock('@/lib/auth/internal', () => ({ + verifyInternalToken: mockVerifyInternalToken, +})) + +vi.mock('@/lib/workflows/persistence/utils', () => workflowsPersistenceUtilsMock) + +vi.mock('@/lib/workflows/utils', () => workflowsUtilsMock) + +import { GET } from './route' + +const mockAuthorizeWorkflowByWorkspacePermission = + workflowAuthzMockFns.mockAuthorizeWorkflowByWorkspacePermission +const mockLoadDeployedWorkflowState = workflowsPersistenceUtilsMockFns.mockLoadDeployedWorkflowState +const mockValidateWorkflowPermissions = workflowsUtilsMockFns.mockValidateWorkflowPermissions + +const DEPLOYED_STATE = { + blocks: { 'block-1': { id: 'block-1', type: 'starter' } }, + edges: [], + loops: {}, + parallels: {}, + variables: {}, +} + +function createRequest(options?: { bearerToken?: string }) { + const headers: Record = {} + if (options?.bearerToken) { + headers.Authorization = `Bearer ${options.bearerToken}` + } + return new NextRequest('http://localhost:3000/api/workflows/workflow-123/deployed', { headers }) +} + +const routeParams = () => ({ params: Promise.resolve({ id: 'workflow-123' }) }) + +describe('GET /api/workflows/[id]/deployed', () => { + beforeEach(() => { + vi.clearAllMocks() + mockVerifyInternalToken.mockResolvedValue({ valid: false }) + mockLoadDeployedWorkflowState.mockResolvedValue(DEPLOYED_STATE) + }) + + describe('internal JWT path', () => { + it('returns 200 when the token carries a user with read permission', async () => { + mockVerifyInternalToken.mockResolvedValue({ valid: true, userId: 'user-123' }) + mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValue({ + allowed: true, + status: 200, + workflow: { id: 'workflow-123', workspaceId: 'workspace-456' }, + workspacePermission: 'read', + }) + + const response = await GET(createRequest({ bearerToken: 'internal-token' }), routeParams()) + + expect(response.status).toBe(200) + const data = await response.json() + expect(data.deployedState).toEqual(DEPLOYED_STATE) + expect(mockAuthorizeWorkflowByWorkspacePermission).toHaveBeenCalledWith({ + workflowId: 'workflow-123', + userId: 'user-123', + action: 'read', + }) + expect(mockValidateWorkflowPermissions).not.toHaveBeenCalled() + }) + + it('returns 403 when the acting user lacks read permission', async () => { + mockVerifyInternalToken.mockResolvedValue({ valid: true, userId: 'user-123' }) + mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValue({ + allowed: false, + status: 403, + message: 'Unauthorized: Access denied to read this workflow', + workflow: { id: 'workflow-123', workspaceId: 'workspace-456' }, + workspacePermission: null, + }) + + const response = await GET(createRequest({ bearerToken: 'internal-token' }), routeParams()) + + expect(response.status).toBe(403) + const data = await response.json() + expect(data.error).toBe('Unauthorized: Access denied to read this workflow') + expect(mockLoadDeployedWorkflowState).not.toHaveBeenCalled() + }) + + it('returns 403 when the token carries no acting user (fail closed)', async () => { + mockVerifyInternalToken.mockResolvedValue({ valid: true, userId: undefined }) + + const response = await GET(createRequest({ bearerToken: 'internal-token' }), routeParams()) + + expect(response.status).toBe(403) + const data = await response.json() + expect(data.error).toBe('Forbidden') + expect(mockAuthorizeWorkflowByWorkspacePermission).not.toHaveBeenCalled() + expect(mockLoadDeployedWorkflowState).not.toHaveBeenCalled() + }) + + it('returns 404 when the workflow does not exist', async () => { + mockVerifyInternalToken.mockResolvedValue({ valid: true, userId: 'user-123' }) + mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValue({ + allowed: false, + status: 404, + message: 'Workflow not found', + workflow: null, + workspacePermission: null, + }) + + const response = await GET(createRequest({ bearerToken: 'internal-token' }), routeParams()) + + expect(response.status).toBe(404) + const data = await response.json() + expect(data.error).toBe('Workflow not found') + expect(mockLoadDeployedWorkflowState).not.toHaveBeenCalled() + }) + }) + + describe('session path', () => { + it('returns 200 when session permissions validate', async () => { + mockValidateWorkflowPermissions.mockResolvedValue({ + error: null, + session: { user: { id: 'user-123' } }, + workflow: { id: 'workflow-123' }, + }) + + const response = await GET(createRequest(), routeParams()) + + expect(response.status).toBe(200) + const data = await response.json() + expect(data.deployedState).toEqual(DEPLOYED_STATE) + expect(mockValidateWorkflowPermissions).toHaveBeenCalledWith( + 'workflow-123', + expect.any(String), + 'read' + ) + expect(mockAuthorizeWorkflowByWorkspacePermission).not.toHaveBeenCalled() + }) + + it('propagates validateWorkflowPermissions errors unchanged', async () => { + mockValidateWorkflowPermissions.mockResolvedValue({ + error: { message: 'Unauthorized', status: 401 }, + session: null, + workflow: null, + }) + + const response = await GET(createRequest(), routeParams()) + + expect(response.status).toBe(401) + const data = await response.json() + expect(data.error).toBe('Unauthorized') + }) + + it('falls back to session validation when the bearer token is not a valid internal token', async () => { + mockVerifyInternalToken.mockResolvedValue({ valid: false }) + mockValidateWorkflowPermissions.mockResolvedValue({ + error: { message: 'Unauthorized', status: 401 }, + session: null, + workflow: null, + }) + + const response = await GET(createRequest({ bearerToken: 'not-internal' }), routeParams()) + + expect(response.status).toBe(401) + expect(mockValidateWorkflowPermissions).toHaveBeenCalled() + expect(mockAuthorizeWorkflowByWorkspacePermission).not.toHaveBeenCalled() + }) + }) + + it('returns null deployedState when loading the snapshot fails', async () => { + mockVerifyInternalToken.mockResolvedValue({ valid: true, userId: 'user-123' }) + mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValue({ + allowed: true, + status: 200, + workflow: { id: 'workflow-123', workspaceId: 'workspace-456' }, + workspacePermission: 'admin', + }) + mockLoadDeployedWorkflowState.mockRejectedValue(new Error('no active deployment')) + + const response = await GET(createRequest({ bearerToken: 'internal-token' }), routeParams()) + + expect(response.status).toBe(200) + const data = await response.json() + expect(data.deployedState).toBeNull() + }) +}) diff --git a/apps/sim/app/api/workflows/[id]/deployed/route.ts b/apps/sim/app/api/workflows/[id]/deployed/route.ts index d68f2b6d6ed..60e8feaf7ec 100644 --- a/apps/sim/app/api/workflows/[id]/deployed/route.ts +++ b/apps/sim/app/api/workflows/[id]/deployed/route.ts @@ -1,4 +1,5 @@ import { createLogger } from '@sim/logger' +import { authorizeWorkflowByWorkspacePermission } from '@sim/platform-authz/workflow' import type { NextRequest, NextResponse } from 'next/server' import { getDeployedWorkflowStateContract } from '@/lib/api/contracts/deployments' import { parseRequest } from '@/lib/api/server' @@ -19,6 +20,18 @@ function addNoCacheHeaders(response: NextResponse): NextResponse { return response } +/** + * GET /api/workflows/[id]/deployed + * Returns the active deployed state snapshot for a workflow. + * + * Internal (server-to-server) calls must carry the acting user in the internal + * JWT payload (`generateInternalToken(userId)` — the executor's + * `buildAuthHeaders(ctx.userId)` always embeds it) and are authorized as that + * user with the same workspace-read semantics as the sibling + * `/api/workflows/[id]` route. Internal calls without a user id are rejected + * (fail closed). Session calls are authorized via + * `validateWorkflowPermissions` as before. + */ export const GET = withRouteHandler( async (request: NextRequest, context: { params: Promise<{ id: string }> }) => { const requestId = generateRequestId() @@ -29,14 +42,39 @@ export const GET = withRouteHandler( try { const authHeader = request.headers.get('authorization') let isInternalCall = false + let internalCallUserId: string | undefined if (authHeader?.startsWith('Bearer ')) { const token = authHeader.split(' ')[1] const verification = await verifyInternalToken(token) isInternalCall = verification.valid + internalCallUserId = verification.userId } - if (!isInternalCall) { + if (isInternalCall) { + if (!internalCallUserId) { + logger.warn(`[${requestId}] Internal call without acting user denied for workflow ${id}`) + return addNoCacheHeaders(createErrorResponse('Forbidden', 403)) + } + + const authorization = await authorizeWorkflowByWorkspacePermission({ + workflowId: id, + userId: internalCallUserId, + action: 'read', + }) + if (!authorization.workflow) { + logger.warn(`[${requestId}] Workflow ${id} not found for internal call`) + return addNoCacheHeaders(createErrorResponse('Workflow not found', 404)) + } + if (!authorization.allowed) { + logger.warn( + `[${requestId}] Internal call user ${internalCallUserId} denied read access to workflow ${id}` + ) + return addNoCacheHeaders( + createErrorResponse(authorization.message || 'Access denied', authorization.status) + ) + } + } else { const { error } = await validateWorkflowPermissions(id, requestId, 'read') if (error) { const response = createErrorResponse(error.message, error.status) diff --git a/apps/sim/app/api/workflows/[id]/execute/route.ts b/apps/sim/app/api/workflows/[id]/execute/route.ts index 246a62ab694..4cd544c27bb 100644 --- a/apps/sim/app/api/workflows/[id]/execute/route.ts +++ b/apps/sim/app/api/workflows/[id]/execute/route.ts @@ -527,6 +527,7 @@ async function handleExecutePost( startBlockId, stopAfterBlockId, runFromBlock: rawRunFromBlock, + parentWorkspaceId, } = validation.data const triggerBlockId = parsedTriggerBlockId ?? startBlockId @@ -642,6 +643,7 @@ async function handleExecutePost( stopAfterBlockId: _stopAfterBlockId, runFromBlock: _runFromBlock, workflowId: _workflowId, // Also exclude workflowId used for internal JWT auth + parentWorkspaceId: _parentWorkspaceId, ...rest } = body return Object.keys(rest).length > 0 ? rest : validatedInput @@ -729,6 +731,25 @@ async function handleExecutePost( ) } + /** + * Workflow-in-workflow invocations (e.g. the agent `workflow_executor` + * tool) declare the parent execution's workspace. Reject execution when + * the target workflow lives in a different workspace so a stale or + * foreign workflow id cannot silently execute with the parent's context. + * The error intentionally omits the target's workspace id. + */ + if (parentWorkspaceId && workflowAuthorization.workflow?.workspaceId !== parentWorkspaceId) { + reqLogger.warn('Blocked cross-workspace child workflow execution', { + parentWorkspaceId, + }) + return NextResponse.json( + { + error: `Child workflow ${workflowId} belongs to a different workspace and cannot be executed`, + }, + { status: 403 } + ) + } + if (req.signal.aborted) { return clientCancelledResponse() } 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 e717958e081..7f2d2a4eb9a 100644 --- a/apps/sim/app/api/workspaces/[id]/fork/diff/route.ts +++ b/apps/sim/app/api/workspaces/[id]/fork/diff/route.ts @@ -19,7 +19,10 @@ import { loadForkDependentValues, } from '@/lib/workspaces/fork/mapping/dependent-value-store' import { listForkResourceCandidates } from '@/lib/workspaces/fork/mapping/resources' -import { collectForkClearedRefCandidates } from '@/lib/workspaces/fork/promote/cleared-refs' +import { + annotateForkClearedRefSourceLiveness, + collectForkClearedRefCandidates, +} from '@/lib/workspaces/fork/promote/cleared-refs' import { computeForkPromotePlan } from '@/lib/workspaces/fork/promote/promote-plan' import { buildForkBlockIdResolver } from '@/lib/workspaces/fork/remap/block-identity' import { readTargetDraftDependentValue } from '@/lib/workspaces/fork/remap/remap-references' @@ -127,15 +130,21 @@ export const GET = withRouteHandler( sourceLabels.set(`${kind}:${candidate.id}`, candidate.label) } const sourceWorkflowNames = new Map(sourceWorkflowRows.map((row) => [row.id, row.name])) - const clearedRefs = collectForkClearedRefCandidates({ - items: plan.items, - sourceStates, - resolver: plan.resolver, - workflowIdMap: plan.workflowIdMap, - resolveBlockId, - sourceLabels, - sourceWorkflowNames, - }) + // Annotate each reference-cause entry's source liveness so the client can phrase the blocker + // reason (a deleted source can't be copied - it must be mapped to a live target resource). + const clearedRefs = await annotateForkClearedRefSourceLiveness( + db, + auth.sourceWorkspaceId, + collectForkClearedRefCandidates({ + items: plan.items, + sourceStates, + resolver: plan.resolver, + workflowIdMap: plan.workflowIdMap, + resolveBlockId, + sourceLabels, + sourceWorkflowNames, + }) + ) const toRef = (reference: (typeof plan.unmappedRequired)[number]) => ({ kind: reference.kind, 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 6b6228620eb..7cfadf34bed 100644 --- a/apps/sim/app/api/workspaces/[id]/fork/promote/route.ts +++ b/apps/sim/app/api/workspaces/[id]/fork/promote/route.ts @@ -48,6 +48,7 @@ export const POST = withRouteHandler( redeployed: result.redeployed, deployFailed: result.deployFailed, unmappedRequired: result.unmappedRequired, + blockers: result.blockers, needsConfiguration: result.needsConfiguration, clearedOptional: result.clearedOptional, } diff --git a/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workspace-header/components/fork-resource-picker/fork-resource-picker.tsx b/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workspace-header/components/fork-resource-picker/fork-resource-picker.tsx index 964aadd907e..82ffbf88cae 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workspace-header/components/fork-resource-picker/fork-resource-picker.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workspace-header/components/fork-resource-picker/fork-resource-picker.tsx @@ -1,8 +1,7 @@ 'use client' import { useId, useMemo, useState } from 'react' -import { Checkbox, ChevronDown, ChipInput, cn } from '@sim/emcn' -import { Search } from 'lucide-react' +import { Checkbox, ChevronDown, cn } from '@sim/emcn' import { ForkFileTree, type ForkFlatFile, @@ -15,14 +14,11 @@ export interface ForkResourcePickerItem { label: string } -/** Show the inline search once a kind has more entries than fit comfortably. */ -const SEARCH_THRESHOLD = 8 - interface ResourceKindRowProps { label: string items: ForkResourcePickerItem[] selected: Set - /** Toggle the given ids on/off. Used for select-all over the currently-VISIBLE (filtered) subset. */ + /** Toggle the given ids on/off. Used by the select-all header checkbox. */ onToggleMany: (ids: string[], checked: boolean) => void onToggleItem: (id: string, checked: boolean) => void disabled?: boolean @@ -30,10 +26,10 @@ interface ResourceKindRowProps { /** * One expandable resource kind in the fork / sync copy picker: a tri-state "select all" header - * (count of selected / total) plus, when expanded, a searchable scrollable list of individual - * resources so the user can copy a specific subset. Shared by the fork modal's "Copy resources" - * and the sync modal's "Copy resources" so the two surfaces stay identical. Files nest in a - * folder tree instead - use {@link FileKindRow}. + * (count of selected / total) plus, when expanded, a scrollable list of individual resources so + * the user can copy a specific subset. Shared by the fork modal's "Copy resources" and the sync + * modal's "Copy resources" so the two surfaces stay identical. Files nest in a folder tree + * instead - use {@link FileKindRow}. */ export function ResourceKindRow({ label, @@ -44,19 +40,10 @@ export function ResourceKindRow({ disabled = false, }: ResourceKindRowProps) { const [expanded, setExpanded] = useState(false) - const [query, setQuery] = useState('') const fieldId = useId() - const filtered = useMemo(() => { - const trimmed = query.trim().toLowerCase() - if (!trimmed) return items - return items.filter((item) => item.label.toLowerCase().includes(trimmed)) - }, [items, query]) - - // Count + header state + select-all are scoped to the VISIBLE (filtered) items so a search never - // selects or counts hidden ones. With no filter, `filtered === items`, so behavior is unchanged. - const total = filtered.length - const selectedCount = filtered.reduce((count, item) => count + (selected.has(item.id) ? 1 : 0), 0) + const total = items.length + const selectedCount = items.reduce((count, item) => count + (selected.has(item.id) ? 1 : 0), 0) const headerState = selectedCount === 0 ? false : selectedCount === total ? true : 'indeterminate' return ( @@ -68,7 +55,7 @@ export function ResourceKindRow({ checked={headerState} onCheckedChange={() => onToggleMany( - filtered.map((item) => item.id), + items.map((item) => item.id), headerState !== true ) } @@ -92,46 +79,32 @@ export function ResourceKindRow({ {expanded ? ( -
- {total > SEARCH_THRESHOLD ? ( - setQuery(event.target.value)} - placeholder={`Search ${label.toLowerCase()}`} - disabled={disabled} - /> - ) : null} -
- {filtered.map((item) => { - const isChecked = selected.has(item.id) - const itemId = `${fieldId}-${item.id}` - return ( - - ) - })} - {filtered.length === 0 ? ( -

No matches

- ) : null} -
+
+ {items.map((item) => { + const isChecked = selected.has(item.id) + const itemId = `${fieldId}-${item.id}` + return ( + + ) + })}
) : null}
diff --git a/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workspace-header/components/promote-workspace-modal/cleared-refs-list.test.ts b/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workspace-header/components/promote-workspace-modal/cleared-refs-list.test.ts index 677c6c8f4fc..c8311b2bd1b 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workspace-header/components/promote-workspace-modal/cleared-refs-list.test.ts +++ b/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workspace-header/components/promote-workspace-modal/cleared-refs-list.test.ts @@ -3,7 +3,11 @@ */ import { describe, expect, it } from 'vitest' import type { ForkClearedRef } from '@/lib/api/contracts/workspace-fork' -import { selectVisibleClearedRefs } from '@/app/workspace/[workspaceId]/w/components/sidebar/components/workspace-header/components/promote-workspace-modal/cleared-refs-list' +import { + forkBlockerResolution, + selectVisibleClearedRefs, + splitForkClearedRefs, +} from '@/app/workspace/[workspaceId]/w/components/sidebar/components/workspace-header/components/promote-workspace-modal/cleared-refs-list' type ReferenceRef = Extract type WorkflowRef = Extract @@ -20,8 +24,9 @@ const base = { const referenceRef = ( kind: ReferenceRef['kind'], sourceId: string, - fieldLabel = 'Field' -): ReferenceRef => ({ ...base, fieldLabel, cause: 'reference', kind, sourceId }) + fieldLabel = 'Field', + sourceDeleted = false +): ReferenceRef => ({ ...base, fieldLabel, cause: 'reference', kind, sourceId, sourceDeleted }) const workflowRef = (sourceId: string, fieldLabel = 'Workflow'): WorkflowRef => ({ ...base, @@ -118,3 +123,47 @@ describe('selectVisibleClearedRefs', () => { ).toEqual([workflowReference]) }) }) + +describe('splitForkClearedRefs', () => { + it('splits reference/workflow causes into blockers and dependents into informational', () => { + const tableReference = referenceRef('table', 'tbl-1') + const workflowReference = workflowRef('wf-other') + const labelDependent = dependentRef('credential', 'cred-1', 'Label') + const { blockers, informational } = splitForkClearedRefs([ + tableReference, + workflowReference, + labelDependent, + ]) + expect(blockers).toEqual([tableReference, workflowReference]) + expect(informational).toEqual([labelDependent]) + }) + + it('treats an unmapped MCP server and a source-deleted reference as blockers', () => { + const mcpReference = referenceRef('mcp-server', 'srv-1') + const deletedReference = referenceRef('skill', 'sk-gone', 'Skill', true) + const { blockers, informational } = splitForkClearedRefs([mcpReference, deletedReference]) + expect(blockers).toEqual([mcpReference, deletedReference]) + expect(informational).toEqual([]) + }) +}) + +describe('forkBlockerResolution', () => { + it('phrases each blocker reason with its actionable resolution', () => { + expect(forkBlockerResolution(referenceRef('table', 'tbl-1'))).toBe( + 'map it to a target or select it for copy' + ) + expect(forkBlockerResolution(referenceRef('mcp-server', 'srv-1'))).toBe( + 'map it to an MCP server in the target workspace' + ) + expect(forkBlockerResolution(referenceRef('knowledge-base', 'kb-gone', 'KB', true))).toBe( + 'deleted in the source — map it to an existing knowledge base in the target' + ) + expect(forkBlockerResolution(workflowRef('wf-other', 'Workflow'))).toBe( + 'deploy "Source" in the source or remove the reference' + ) + }) + + it('returns null for non-blocking dependent entries', () => { + expect(forkBlockerResolution(dependentRef('credential', 'cred-1'))).toBeNull() + }) +}) diff --git a/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workspace-header/components/promote-workspace-modal/cleared-refs-list.ts b/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workspace-header/components/promote-workspace-modal/cleared-refs-list.ts index 3f0bba304fe..d55a460fa07 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workspace-header/components/promote-workspace-modal/cleared-refs-list.ts +++ b/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workspace-header/components/promote-workspace-modal/cleared-refs-list.ts @@ -1,4 +1,5 @@ import type { ForkClearedRef } from '@/lib/api/contracts/workspace-fork' +import { forkSyncBlockerReasonFor } from '@/lib/workspaces/fork/promote/sync-blockers' /** Whether a resource is resolved by the current selection (mapped to a target OR selected for copy). */ export type ClearedRefResolvedPredicate = (kind: string, sourceId: string) => boolean @@ -37,3 +38,51 @@ export function selectVisibleClearedRefs( return true }) } + +/** + * Split the visible would-clear entries into sync BLOCKERS (cause `reference`/`workflow` - the + * sync is disabled while any remain) and the informational remainder (`dependent` entries, owned + * by the reconfigure flow - they clear but never block). Pure, so the modal's gate and the two + * sections stay one testable rule. + */ +export function splitForkClearedRefs(visibleRefs: ForkClearedRef[]): { + blockers: ForkClearedRef[] + informational: ForkClearedRef[] +} { + const blockers: ForkClearedRef[] = [] + const informational: ForkClearedRef[] = [] + for (const ref of visibleRefs) { + if (forkSyncBlockerReasonFor(ref)) blockers.push(ref) + else informational.push(ref) + } + return { blockers, informational } +} + +/** Human label per blocker kind for the resolution copy (singular, lowercase mid-sentence). */ +const BLOCKER_KIND_LABEL: Record = { + table: 'table', + 'knowledge-base': 'knowledge base', + file: 'file', + 'custom-tool': 'custom tool', + skill: 'skill', + 'mcp-server': 'MCP server', +} + +/** + * The actionable resolution line for a blocking entry, phrased for "{block} would lose {field} + * in {workflow} - {resolution}". Null for non-blocking (dependent) entries. + */ +export function forkBlockerResolution(ref: ForkClearedRef): string | null { + const reason = forkSyncBlockerReasonFor(ref) + if (!reason) return null + switch (reason) { + case 'unmapped-copyable': + return 'map it to a target or select it for copy' + case 'unmapped-mcp-server': + return 'map it to an MCP server in the target workspace' + case 'source-deleted': + return `deleted in the source — map it to an existing ${BLOCKER_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/app/workspace/[workspaceId]/w/components/sidebar/components/workspace-header/components/promote-workspace-modal/components/resource-reconfigure.tsx b/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workspace-header/components/promote-workspace-modal/components/resource-reconfigure.tsx index 5c7c6eedba5..075d46dc7e4 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workspace-header/components/promote-workspace-modal/components/resource-reconfigure.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workspace-header/components/promote-workspace-modal/components/resource-reconfigure.tsx @@ -3,7 +3,6 @@ import { type Dispatch, type SetStateAction, useMemo, useState } from 'react' import { ChevronDown, cn } from '@sim/emcn' import type { ForkDependentReconfig, ForkResourceUsage } from '@/lib/api/contracts/workspace-fork' -import { SettingsSection } from '@/app/workspace/[workspaceId]/settings/components/settings-section/settings-section' import { DependentFieldSelector } from '@/app/workspace/[workspaceId]/w/components/sidebar/components/workspace-header/components/promote-workspace-modal/components/dependent-field-selector' import { dependentKey, @@ -57,8 +56,8 @@ interface ResourceReconfigureProps { * Always-on per-resource reconfigure listing: every workflow the resource is used in, each a * chevron row that expands to its blocks + dependent selectors so the user can (re)configure * them at any time - not only right after a target swap. A workflow with nothing configurable - * (a secret/file, or a credential with no dependent selector here) renders greyed and - * non-expandable with a tooltip, so the usage is still visible. + * (a secret/file, or a credential with no dependent selector here) renders as a plain + * non-interactive row without a chevron, with a tooltip, so the usage is still visible. */ export function ResourceReconfigure({ workflows, @@ -88,24 +87,26 @@ export function ResourceReconfigure({ }, [workflows, dependents]) if (workflows.length === 0) return null + // Muted caption label over the list (no divider) so this reads as subordinate to the + // resource-name section header above, mirroring the "Recent runs" listing in + // mothership-view's resource-content. return ( -
- -
- {workflowBlocks.map((workflow) => ( - - ))} -
-
+
+ Workflows +
+ {workflowBlocks.map((workflow) => ( + + ))} +
) } @@ -120,7 +121,7 @@ interface ReconfigWorkflowRowProps { setReconfig: Dispatch>> } -/** One workflow row: a chevron header (greyed + non-expandable when nothing to configure). */ +/** One workflow row: a chevron header (a plain non-interactive row when nothing to configure). */ function ReconfigWorkflowRow({ workflowName, blocks, @@ -141,28 +142,31 @@ function ReconfigWorkflowRow({ return (
- {/* Chevron styling mirrors the Activity panel's collapsible rows exactly. A greyed, - non-expandable row uses a native title tooltip to explain why. */} - + {/* Chevron styling mirrors the Activity panel's collapsible rows exactly. A row with + nothing to configure renders as muted plain text (no chevron, not a button) with a + native title tooltip explaining why it isn't expandable. */} + {configurable ? ( + + ) : ( +
+ {workflowName} +
+ )} {configurable && open ? blocks.map((block) => ( ): ForkCopyableUnmappe label: 'KB', parentId: null, parentLabel: null, + referenced: true, ...overrides, }) @@ -82,6 +85,23 @@ describe('copy-vs-map reconciliation', () => { }) }) +describe('forkDefaultCopySelection', () => { + it('seeds every referenced candidate and leaves unreferenced ones unselected', () => { + const selection = forkDefaultCopySelection([ + copyable({ kind: 'knowledge-base', sourceId: 'kb-1', referenced: true }), + copyable({ kind: 'table', sourceId: 'tbl-new', referenced: false }), + copyable({ kind: 'file', sourceId: 'workspace/SRC/new.png', referenced: false }), + ]) + expect(selection).toEqual(new Set(['knowledge-base:kb-1'])) + }) + + it('seeds nothing when every candidate is unreferenced', () => { + expect( + forkDefaultCopySelection([copyable({ kind: 'skill', sourceId: 'sk-new', referenced: false })]) + ).toEqual(new Set()) + }) +}) + describe('isForkRequiredComplete', () => { it('a required ref is satisfied by a mapping target', () => { const entries = [ @@ -100,12 +120,47 @@ describe('isForkRequiredComplete', () => { expect(isForkRequiredComplete(entries, {}, new Set())).toBe(false) }) + it('a referenced MCP server (map-only, required) blocks until mapped - copy cannot satisfy it', () => { + const entries = [ + entry({ kind: 'mcp-server', resourceType: 'mcp_server', sourceId: 'srv-1', required: true }), + ] + // MCP servers are never copy candidates, so the copy set can't contain them; only a + // mapping target resolves the entry. + expect(isForkRequiredComplete(entries, {}, new Set())).toBe(false) + expect(isForkRequiredComplete(entries, { 'mcp-server:srv-1': 'srv-tgt' }, new Set())).toBe(true) + }) + + it('a source-deleted referenced resource (required, no copy candidate) blocks until mapped', () => { + // A deleted source is dropped from the copy candidates (its label lookup fails), so the + // only resolution is mapping the dead id to a live target resource. + const entries = [ + entry({ kind: 'table', resourceType: 'table', sourceId: 'tbl-gone', required: true }), + ] + expect(isForkRequiredComplete(entries, {}, new Set())).toBe(false) + expect(isForkRequiredComplete(entries, { 'table:tbl-gone': 'tbl-live' }, new Set())).toBe(true) + }) + it('optional refs never block', () => { const entries = [entry({ kind: 'table', sourceId: 't1', required: false })] expect(isForkRequiredComplete(entries, {}, new Set())).toBe(true) }) }) +describe('forkRequiredKindsLabel', () => { + it('names credentials and secrets by kind, together or alone', () => { + expect(forkRequiredKindsLabel(new Set(['credential', 'env-var']))).toBe( + 'credentials and secrets' + ) + expect(forkRequiredKindsLabel(new Set(['credential']))).toBe('credentials') + expect(forkRequiredKindsLabel(new Set(['env-var']))).toBe('secrets') + }) + + it('falls back to "references" for any other (or empty) kind set', () => { + expect(forkRequiredKindsLabel(new Set(['table']))).toBe('references') + expect(forkRequiredKindsLabel(new Set())).toBe('references') + }) +}) + describe('forkRequiredPending', () => { it('is true when a required ref is neither mapped nor selected for copy', () => { const items = [entry({ kind: 'credential', sourceId: 'c1', required: true })] diff --git a/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workspace-header/components/promote-workspace-modal/copy-reconciliation.ts b/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workspace-header/components/promote-workspace-modal/copy-reconciliation.ts index f1707ad1a3b..afea9cd5010 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workspace-header/components/promote-workspace-modal/copy-reconciliation.ts +++ b/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workspace-header/components/promote-workspace-modal/copy-reconciliation.ts @@ -35,6 +35,20 @@ export function forkVisibleCopyables( return copyableUnmapped.filter((candidate) => !mappedKeys.has(forkRefKey(candidate))) } +/** + * The copy selection seeded once the diff settles: every REFERENCED candidate (deselecting one + * clears its references, so the common case needs no clicks). Unreferenced candidates - used by + * no synced workflow - start unselected: copying them is opt-in, so scratch data created in the + * source is never pushed by surprise. + */ +export function forkDefaultCopySelection(copyableUnmapped: ForkCopyableUnmapped[]): Set { + const keys = new Set() + for (const candidate of copyableUnmapped) { + if (candidate.referenced) keys.add(forkRefKey(candidate)) + } + return keys +} + /** Keys of the visible copy candidates actually selected for copy. */ export function forkCopyingKeys( visibleCopyables: ForkCopyableUnmapped[], @@ -83,3 +97,20 @@ export function forkRequiredPending( !copyingKeys.has(forkRefKey(entry)) ) } + +/** + * Human label for the kinds still failing the required gate, for "Map all required {label} first" + * messaging - shared by the Sync button's disabled tooltip (client gate) and the server gate's + * failure toast so both name the obstacle identically. Credentials and secrets are named + * explicitly: they are the map-only kinds that fail the required gate WITHOUT also appearing in + * the cleared-ref blockers (the collector excludes them), so they need their own wording. Any + * other kind falls back to "references". + */ +export function forkRequiredKindsLabel(kinds: ReadonlySet): string { + const credentials = kinds.has('credential') + const secrets = kinds.has('env-var') + if (credentials && secrets) return 'credentials and secrets' + if (credentials) return 'credentials' + if (secrets) return 'secrets' + return 'references' +} diff --git a/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workspace-header/components/promote-workspace-modal/promote-workspace-modal.tsx b/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workspace-header/components/promote-workspace-modal/promote-workspace-modal.tsx index c2186faff72..9e5129e6dc3 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workspace-header/components/promote-workspace-modal/promote-workspace-modal.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workspace-header/components/promote-workspace-modal/promote-workspace-modal.tsx @@ -11,6 +11,7 @@ import { ChipModalFooter, type ChipModalFooterSlotAction, ChipModalHeader, + cn, toast, } from '@sim/emcn' import { getErrorMessage } from '@sim/utils/errors' @@ -28,13 +29,19 @@ import { FileKindRow, ResourceKindRow, } from '@/app/workspace/[workspaceId]/w/components/sidebar/components/workspace-header/components/fork-resource-picker/fork-resource-picker' -import { selectVisibleClearedRefs } from '@/app/workspace/[workspaceId]/w/components/sidebar/components/workspace-header/components/promote-workspace-modal/cleared-refs-list' +import { + forkBlockerResolution, + selectVisibleClearedRefs, + splitForkClearedRefs, +} from '@/app/workspace/[workspaceId]/w/components/sidebar/components/workspace-header/components/promote-workspace-modal/cleared-refs-list' import { ResourceReconfigure } from '@/app/workspace/[workspaceId]/w/components/sidebar/components/workspace-header/components/promote-workspace-modal/components/resource-reconfigure' import { effectiveForkTarget, forkCopyingKeys, + forkDefaultCopySelection, forkMappedCopyableKeys, forkRefKey, + forkRequiredKindsLabel, forkRequiredPending, forkVisibleCopyables, isForkRequiredComplete, @@ -127,7 +134,7 @@ const MAPPING_SECTION: Record forkRefKey(candidate) /** Sentinel option value for the editor's "Copy instead" entry - handled via onSelect, never sent. */ const COPY_INSTEAD_VALUE = '__copy_instead__' +/** Archived-workflow names shown in the sync confirm before truncating to "and X more". */ +const ARCHIVED_PREVIEW_LIMIT = 5 + interface EdgeOption { value: string label: string @@ -278,27 +288,33 @@ export function PromoteWorkspaceModal({ // 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. Files nest in a folder ▸ - // file tree inside their section; every other kind is a flat searchable list. - const copyablesByKind = useMemo(() => { - const groups = new Map() + // file tree inside their section; every other kind is a flat list. Referenced and unreferenced + // candidates group separately: unreferenced ones (used by no synced workflow) render under a + // muted "Not used by any workflow" grouping and default to unselected. + const { referencedByKind, unreferencedByKind } = useMemo(() => { + const referenced = new Map() + const unreferenced = new Map() for (const candidate of visibleCopyables) { + const groups = candidate.referenced ? referenced : unreferenced const list = groups.get(candidate.kind) if (list) list.push(candidate) else groups.set(candidate.kind, [candidate]) } - return groups + return { referencedByKind: referenced, unreferencedByKind: unreferenced } }, [visibleCopyables]) - // Default every copyable referenced resource to "copy" once the diff loads, so the common case + // Default every REFERENCED copyable resource to "copy" once the diff loads, so the common case // (bring the referenced resources along) needs no clicks; the user can deselect to clear instead. - // Seed ONLY from a settled diff for the current direction: on a direction switch the reset clears - // `copyDefaulted`, but `useForkDiff` keeps the previous direction's payload (placeholderData) until - // the new fetch resolves - seeding from it would latch against stale keys and leave the real - // copyables unchecked, clearing their references on Sync. + // Unreferenced candidates start unselected (see `forkDefaultCopySelection`) - copying them is + // opt-in since nothing references them. Seed ONLY from a settled diff for the current direction: + // on a direction switch the reset clears `copyDefaulted`, but `useForkDiff` keeps the previous + // direction's payload (placeholderData) until the new fetch resolves - seeding from it would + // latch against stale keys and leave the real copyables unchecked, clearing their references + // on Sync. useEffect(() => { if (!open || diff.isPlaceholderData || copyableUnmapped.length === 0 || copyDefaulted) return setCopyDefaulted(true) - setCopySelected(new Set(copyableUnmapped.map(copyableKey))) + setCopySelected(forkDefaultCopySelection(copyableUnmapped)) }, [open, diff.isPlaceholderData, copyableUnmapped, copyDefaulted]) // Group dependents by their parent (kind:sourceId) once, so each mapping entry below gets a @@ -422,18 +438,20 @@ export function PromoteWorkspaceModal({ } } - // The references this sync will blank, reactively narrowed to the current selection. A resource + // The references this sync would blank, reactively narrowed to the current selection. A resource // is "resolved" once it has a mapping target OR is selected for copy - the same predicate drives // a `reference` (its own resource) and a `dependent` (its PARENT resource), so mapping or copying - // a parent KB makes its child document drop off. `workflow` refs always clear (not resolvable here). - const clearedRefsToShow = useMemo(() => { + // a parent KB makes its child document drop off. Then split: `reference`/`workflow` entries are + // BLOCKERS (Sync stays disabled while any remain - mirroring the server's zero-cleared-refs + // gate); `dependent` entries stay informational (the reconfigure flow owns them). + const { blockers: blockingRefs, informational: dependentClears } = useMemo(() => { const isResolved = (kind: string, sourceId: string) => { const key = `${kind}:${sourceId}` const entry = entriesByParent.get(key) const mapped = entry ? (targets[key] ?? entry.targetId ?? '') !== '' : false return mapped || copyingKeys.has(key) } - return selectVisibleClearedRefs(clearedRefs, isResolved) + return splitForkClearedRefs(selectVisibleClearedRefs(clearedRefs, isResolved)) }, [clearedRefs, entriesByParent, targets, copyingKeys]) // Per-kind status for the overview listing: "Fully mapped" or "n/total mapped", @@ -461,6 +479,14 @@ export function PromoteWorkspaceModal({ } }) + // Kinds whose required gate is still failing, so the Sync tooltip can name the actual + // obstacle. An unmapped credential/secret is NEVER a cleared-ref blocker (the collector + // excludes required kinds), so the required gate must not borrow the blocker message - + // it would point at a "Blocking sync" section that isn't rendered. + const pendingRequiredKinds = new Set( + kindSummaries.filter((summary) => summary.requiredPending).map((summary) => summary.kind) + ) + // Step 0 is the overview; each subsequent step edits one resource kind, entered via // "Edit mappings". Reconfigure cards render inline under the changed mapping (not as // their own steps) so the credential/KB context stays visible. `safeStep` guards @@ -479,8 +505,18 @@ export function PromoteWorkspaceModal({ mapping.isPlaceholderData || !diff.data || diff.isPlaceholderData + // Zero-blockers invariant (mirrors the server gate): Sync stays disabled while ANY reference + // would clear in a synced target workflow. `requiredComplete` covers the mapping entries + // (credentials/secrets and unresolved resource refs); `blockingRefs` additionally covers + // workflow-to-workflow references, which have no mapping entry to resolve. + const syncBlocked = blockingRefs.length > 0 const syncDisabled = - submitting || !otherWorkspaceId || !requiredComplete || !reconfigComplete || dataPending + submitting || + !otherWorkspaceId || + !requiredComplete || + !reconfigComplete || + syncBlocked || + dataPending const headsUp = (diff.data?.mcpReauthServerIds.length ?? 0) > 0 || (diff.data?.inlineSecretSources.length ?? 0) > 0 @@ -553,20 +589,22 @@ export function PromoteWorkspaceModal({ }) if (!result.promoteRunId) { + if (result.blockers.length > 0) { + // The server's authoritative gate re-found would-clear references (something changed + // between the preview and Sync). The mutation's settled invalidation refetches the + // diff, so the refreshed blocker list is already on its way in. + const count = result.blockers.length + toast.error( + `Sync blocked: ${count} reference${count === 1 ? '' : 's'} would break in the target. Review the updated list and try again.` + ) + return + } if (result.unmappedRequired.length > 0) { // Name the actual blocking kinds rather than always blaming credentials: the server // blocks on required REFERENCES (credentials and/or secrets); required dependents are // gated client-side before this runs (see the Sync button's disabled tooltip). const kinds = new Set(result.unmappedRequired.map((reference) => reference.kind)) - const what = - kinds.has('credential') && kinds.has('env-var') - ? 'credentials and secrets' - : kinds.has('credential') - ? 'credentials' - : kinds.has('env-var') - ? 'secrets' - : 'references' - toast.error(`Map all required ${what} first`) + toast.error(`Map all required ${forkRequiredKindsLabel(kinds)} first`) return } toast.error('Sync did not complete') @@ -631,6 +669,83 @@ export function PromoteWorkspaceModal({ ) }, [diff.data?.workflows]) + // Target workflows this sync archives (their source was deleted), named in the confirm modal so + // the overwrite warning is concrete - the push-to-parent case is the high-stakes one, so the + // target workspace is named explicitly there. + const archivedWorkflowNames = useMemo( + () => + workflowChanges + .filter((change) => change.action === 'archive') + .map((change) => change.currentName), + [workflowChanges] + ) + const targetWorkspaceName = + direction === 'push' ? (parent?.name ?? 'the parent workspace') : 'this workspace' + + // One expandable row per copyable kind present in `byKind` - shared by the referenced group + // and the unreferenced "Not used by any workflow" group so both render exactly like the fork + // picker (files as a folder tree, every other kind flat). + const renderCopyKindSections = ( + byKind: ReadonlyMap + ) => + COPYABLE_KIND_SECTIONS.map((section) => { + const candidates = byKind.get(section.kind) + if (!candidates || candidates.length === 0) return null + // The picker rows track item ids; copy selection is keyed `${kind}:${id}` + // (matching `copyableKey`), so derive the per-kind selected-id subset and + // re-prefix on toggle. + const selectedIds = new Set( + candidates + .filter((candidate) => copySelected.has(copyableKey(candidate))) + .map((candidate) => candidate.sourceId) + ) + const toggleMany = (ids: string[], checked: boolean) => + setCopySelected((prev) => { + const next = new Set(prev) + for (const id of ids) { + const key = `${section.kind}:${id}` + if (checked) next.add(key) + else next.delete(key) + } + return next + }) + const toggleAll = (selectAll: boolean) => + toggleMany( + candidates.map((candidate) => candidate.sourceId), + selectAll + ) + return section.kind === 'file' ? ( + ({ + id: candidate.sourceId, + label: candidate.label, + folderId: candidate.parentId, + folderName: candidate.parentLabel, + }))} + selected={selectedIds} + onToggleAll={toggleAll} + onToggleItem={(id, checked) => toggleMany([id], checked)} + onToggleMany={toggleMany} + disabled={submitting} + /> + ) : ( + ({ + id: candidate.sourceId, + label: candidate.label, + }))} + selected={selectedIds} + onToggleMany={toggleMany} + onToggleItem={(id, checked) => toggleMany([id], checked)} + disabled={submitting} + /> + ) + }) + // Right-cluster action sitting immediately left of the primary. The overview pairs // "Edit mappings" with Sync (entering the step walk); every editing step pairs Back // with Next (or with Sync on the last step). Back out of step 1 lands on the @@ -782,10 +897,31 @@ export function PromoteWorkspaceModal({ ) : null} - {clearedRefsToShow.length > 0 ? ( + {syncBlocked ? ( + +
+ {blockingRefs.map((ref, index) => ( +
+ {ref.blockLabel} would lose{' '} + {ref.fieldLabel} in{' '} + {ref.workflowName} — {forkBlockerResolution(ref)} +
+ ))} +
+

+ Sync is blocked while any of these remain, so every synced workflow stays fully + operational in the target. +

+
+ ) : null} + + {dependentClears.length > 0 ? (
- {clearedRefsToShow.map((ref, index) => ( + {dependentClears.map((ref, index) => (

- Map or copy a reference to keep it. Fields that reference another workflow, or - that hang off a remapped credential or knowledge base, are cleared regardless. + Fields that hang off a remapped credential or knowledge base are cleared — + re-pick them in the target after the sync.

) : null} @@ -806,67 +942,33 @@ export function PromoteWorkspaceModal({ {visibleCopyables.length > 0 ? (
- {COPYABLE_KIND_SECTIONS.map((section) => { - const candidates = copyablesByKind.get(section.kind) - if (!candidates || candidates.length === 0) return null - // The picker rows track item ids; copy selection is keyed `${kind}:${id}` - // (matching `copyableKey`), so derive the per-kind selected-id subset and - // re-prefix on toggle. - const selectedIds = new Set( - candidates - .filter((candidate) => copySelected.has(copyableKey(candidate))) - .map((candidate) => candidate.sourceId) - ) - const toggleMany = (ids: string[], checked: boolean) => - setCopySelected((prev) => { - const next = new Set(prev) - for (const id of ids) { - const key = `${section.kind}:${id}` - if (checked) next.add(key) - else next.delete(key) - } - return next - }) - const toggleAll = (selectAll: boolean) => - toggleMany( - candidates.map((candidate) => candidate.sourceId), - selectAll - ) - return section.kind === 'file' ? ( - ({ - id: candidate.sourceId, - label: candidate.label, - folderId: candidate.parentId, - folderName: candidate.parentLabel, - }))} - selected={selectedIds} - onToggleAll={toggleAll} - onToggleItem={(id, checked) => toggleMany([id], checked)} - onToggleMany={toggleMany} - disabled={submitting} - /> - ) : ( - ({ - id: candidate.sourceId, - label: candidate.label, - }))} - selected={selectedIds} - onToggleMany={toggleMany} - onToggleItem={(id, checked) => toggleMany([id], checked)} - disabled={submitting} - /> - ) - })} -

- These referenced resources aren't in the target yet. Selected ones are copied - during the sync; deselected ones have their references cleared. -

+ {referencedByKind.size > 0 ? ( + <> + {renderCopyKindSections(referencedByKind)} +

+ These referenced resources aren't in the target yet. Selected ones are + copied during the sync; a deselected one blocks the sync until it's mapped + or selected again. +

+ + ) : null} + {unreferencedByKind.size > 0 ? ( + <> +
0 && 'mt-1' + )} + > + Not used by any workflow +
+ {renderCopyKindSections(unreferencedByKind)} +

+ These aren't referenced by any synced workflow. Selected ones are copied + during the sync; deselected ones are simply left out. +

+ + ) : null}
) : null} @@ -882,17 +984,7 @@ export function PromoteWorkspaceModal({ ? takenTargetOwners(currentGroup.items, targets, entry) : EMPTY_TARGET_OWNERS return ( - - * - - ) : undefined - } - > + ) : null} {/* Always-on: every workflow this resource is used in, each expandable to - its blocks + dependent selectors (greyed when nothing to configure). */} + its blocks + dependent selectors (a plain row when nothing to configure). */} setConfirmSyncOpen(true), disabled: syncDisabled, - disabledTooltip: !requiredComplete - ? 'Map all required secrets first' - : !reconfigComplete - ? 'Reconfigure all required fields first' - : dataPending - ? 'Loading sync details…' - : undefined, + // Priority mirrors the resolution flow: clear the blockers, map the required + // resources, reconfigure their dependents - each failing gate names ITS + // obstacle (an unmapped credential/secret is a required-mapping failure, not + // a cleared-ref blocker; see `pendingRequiredKinds`). + disabledTooltip: syncBlocked + ? 'Resolve every blocking reference first — map it, copy it, or fix it in the source' + : !requiredComplete + ? `Map all required ${forkRequiredKindsLabel(pendingRequiredKinds)} first` + : !reconfigComplete + ? 'Reconfigure all required fields first' + : dataPending + ? 'Loading sync details…' + : undefined, } } /> @@ -993,7 +1091,29 @@ export function PromoteWorkspaceModal({ pending: submitting, pendingLabel: 'Syncing...', }} - /> + > + {archivedWorkflowNames.length > 0 ? ( +
+

+ Will be archived in {targetWorkspaceName}{' '} + (deleted in the source): +

+ {archivedWorkflowNames.slice(0, ARCHIVED_PREVIEW_LIMIT).map((name, index) => ( +
+ {name} +
+ ))} + {archivedWorkflowNames.length > ARCHIVED_PREVIEW_LIMIT ? ( +
+ and {archivedWorkflowNames.length - ARCHIVED_PREVIEW_LIMIT} more +
+ ) : null} +
+ ) : null} + ) } diff --git a/apps/sim/executor/handlers/workflow/workflow-handler.test.ts b/apps/sim/executor/handlers/workflow/workflow-handler.test.ts index 2823d3d1383..1ff65759ed1 100644 --- a/apps/sim/executor/handlers/workflow/workflow-handler.test.ts +++ b/apps/sim/executor/handlers/workflow/workflow-handler.test.ts @@ -5,6 +5,21 @@ import { WorkflowBlockHandler } from '@/executor/handlers/workflow/workflow-hand import type { ExecutionContext } from '@/executor/types' import type { SerializedBlock } from '@/serializer/types' +const { mockExecutorExecute, mockCreateSnapshot } = vi.hoisted(() => ({ + mockExecutorExecute: vi.fn(), + mockCreateSnapshot: vi.fn(), +})) + +vi.mock('@/executor', () => ({ + Executor: class { + execute = mockExecutorExecute + }, +})) + +vi.mock('@/lib/logs/execution/snapshot/service', () => ({ + snapshotService: { createSnapshotWithDeduplication: mockCreateSnapshot }, +})) + vi.mock('@/lib/auth/internal', () => ({ generateInternalToken: vi.fn().mockResolvedValue('test-token'), })) @@ -161,6 +176,119 @@ describe('WorkflowBlockHandler', () => { }) }) + describe('workspace containment', () => { + const inputs = { workflowId: 'child-workflow-id' } + + it('should fail a cross-workspace child in the draft loader path', async () => { + const ctx = { ...mockContext, workspaceId: 'workspace-parent' } + + mockFetch.mockResolvedValueOnce({ + ok: true, + json: () => + Promise.resolve({ + data: { + name: 'Foreign Workflow', + workspaceId: 'workspace-other', + state: { blocks: {}, edges: [], loops: {}, parallels: {} }, + }, + }), + }) + + await expect(handler.execute(ctx, mockBlock, inputs)).rejects.toThrow( + 'Child workflow child-workflow-id belongs to a different workspace and cannot be executed' + ) + expect(mockCreateSnapshot).not.toHaveBeenCalled() + expect(mockExecutorExecute).not.toHaveBeenCalled() + }) + + it('should fail a cross-workspace child in the deployed loader path', async () => { + const ctx = { + ...mockContext, + workspaceId: 'workspace-parent', + isDeployedContext: true, + } + + mockFetch.mockImplementation(async (url: unknown) => { + if (String(url).includes('/deployed')) { + return { + ok: true, + json: () => + Promise.resolve({ + data: { + deployedState: { blocks: {}, edges: [], loops: {}, parallels: {} }, + }, + }), + } + } + return { + ok: true, + json: () => + Promise.resolve({ + data: { + name: 'Foreign Workflow', + workspaceId: 'workspace-other', + variables: {}, + }, + }), + } + }) + + await expect(handler.execute(ctx, mockBlock, inputs)).rejects.toThrow( + 'Child workflow child-workflow-id belongs to a different workspace and cannot be executed' + ) + expect(mockCreateSnapshot).not.toHaveBeenCalled() + expect(mockExecutorExecute).not.toHaveBeenCalled() + }) + + it('should execute a same-workspace child as before', async () => { + const ctx = { ...mockContext, workspaceId: 'workspace-parent' } + + mockFetch.mockResolvedValueOnce({ + ok: true, + json: () => + Promise.resolve({ + data: { + name: 'Child Workflow', + workspaceId: 'workspace-parent', + state: { blocks: {}, edges: [], loops: {}, parallels: {} }, + }, + }), + }) + mockCreateSnapshot.mockResolvedValue({ snapshot: { id: 'snapshot-1' } }) + mockExecutorExecute.mockResolvedValue({ success: true, output: { data: 'ok' } }) + + const result = await handler.execute(ctx, mockBlock, inputs) + + expect(result).toMatchObject({ + success: true, + childWorkflowId: 'child-workflow-id', + childWorkflowName: 'Child Workflow', + childWorkflowSnapshotId: 'snapshot-1', + result: { data: 'ok' }, + }) + expect(mockExecutorExecute).toHaveBeenCalledWith('child-workflow-id') + }) + + it('should fail closed when the executing context has no workspace', async () => { + mockFetch.mockResolvedValueOnce({ + ok: true, + json: () => + Promise.resolve({ + data: { + name: 'Child Workflow', + workspaceId: 'workspace-parent', + state: { blocks: {}, edges: [], loops: {}, parallels: {} }, + }, + }), + }) + + await expect(handler.execute(mockContext, mockBlock, inputs)).rejects.toThrow( + 'Cannot execute child workflow child-workflow-id: executing context has no workspace' + ) + expect(mockExecutorExecute).not.toHaveBeenCalled() + }) + }) + describe('loadChildWorkflow', () => { it('should return null for 404 responses', async () => { const workflowId = 'non-existent-workflow' diff --git a/apps/sim/executor/handlers/workflow/workflow-handler.ts b/apps/sim/executor/handlers/workflow/workflow-handler.ts index 2a8d3d73c31..eea6b4a3004 100644 --- a/apps/sim/executor/handlers/workflow/workflow-handler.ts +++ b/apps/sim/executor/handlers/workflow/workflow-handler.ts @@ -109,6 +109,8 @@ export class WorkflowBlockHandler implements BlockHandler { throw new Error(`Child workflow ${workflowId} not found`) } + this.assertChildWorkflowInWorkspace(workflowId, childWorkflow.workspaceId, ctx.workspaceId) + childWorkflowName = childWorkflow.name || 'Unknown Workflow' logger.info( @@ -324,6 +326,34 @@ export class WorkflowBlockHandler implements BlockHandler { return { chain, rootError: rootError.trim() || 'Unknown error' } } + /** + * Ensures the child workflow belongs to the same workspace as the executing + * context before any child execution starts. Blocks silent cross-workspace + * execution (e.g. a manual workflow id still pointing at the source + * workspace after a fork), which would otherwise run the foreign workflow + * with the parent workspace's environment and billing. Fails closed when the + * executing context carries no workspace id: every server execution path + * populates it via execution-core, so a missing value indicates a context + * that must not silently bypass the check. The error message intentionally + * omits the foreign workspace id. + */ + private assertChildWorkflowInWorkspace( + childWorkflowId: string, + childWorkspaceId: string | null | undefined, + parentWorkspaceId: string | undefined + ): void { + if (!parentWorkspaceId) { + throw new Error( + `Cannot execute child workflow ${childWorkflowId}: executing context has no workspace` + ) + } + if (childWorkspaceId !== parentWorkspaceId) { + throw new Error( + `Child workflow ${childWorkflowId} belongs to a different workspace and cannot be executed` + ) + } + } + private async loadChildWorkflow(workflowId: string, userId?: string) { const headers = await buildAuthHeaders(userId) const url = buildAPIUrl(`/api/workflows/${workflowId}`) @@ -378,6 +408,7 @@ export class WorkflowBlockHandler implements BlockHandler { return { name: workflowData.name, + workspaceId: (workflowData.workspaceId ?? null) as string | null, serializedState: serializedWorkflow, variables: workflowVariables, workflowState: workflowStateWithVariables, @@ -461,6 +492,7 @@ export class WorkflowBlockHandler implements BlockHandler { return { name: childName, + workspaceId: (wfData?.workspaceId ?? null) as string | null, serializedState: serializedWorkflow, variables: workflowVariables, workflowState: workflowStateWithVariables, diff --git a/apps/sim/lib/api/contracts/workflows.ts b/apps/sim/lib/api/contracts/workflows.ts index 828d91db17b..72bb635cf71 100644 --- a/apps/sim/lib/api/contracts/workflows.ts +++ b/apps/sim/lib/api/contracts/workflows.ts @@ -1,4 +1,5 @@ import { z } from 'zod' +import { workspaceIdSchema } from '@/lib/api/contracts/primitives' import { defineRouteContract } from '@/lib/api/contracts/types' const subBlockValuesSchema = z.record(z.string(), z.record(z.string(), z.unknown())) @@ -343,6 +344,13 @@ export const executeWorkflowBodySchema = z.object({ startBlockId: z.string().optional(), stopAfterBlockId: z.string().optional(), runFromBlock: executeWorkflowRunFromBlockSchema.optional(), + /** + * Workspace of the parent execution when this call is a workflow-in-workflow + * invocation (e.g. the agent `workflow_executor` tool). When present, the + * route rejects execution of a workflow that lives in a different workspace. + * Direct API callers omit it and are unaffected. + */ + parentWorkspaceId: workspaceIdSchema.optional(), }) export type ExecuteWorkflowBody = z.input diff --git a/apps/sim/lib/api/contracts/workspace-fork.ts b/apps/sim/lib/api/contracts/workspace-fork.ts index cf7e2430dfa..ec1280b9d15 100644 --- a/apps/sim/lib/api/contracts/workspace-fork.ts +++ b/apps/sim/lib/api/contracts/workspace-fork.ts @@ -331,24 +331,33 @@ const forkClearedRefBaseSchema = z.object({ }) /** - * A reference in a synced source workflow that WILL be blanked in the target by this sync, with the + * A reference in a synced source workflow that this sync would blank in the target, with the * labels to phrase it as "{blockLabel} will lose {fieldLabel} in workflow {workflowName}". A * discriminated union on `cause` so clients narrow exhaustively (only `dependent` carries the parent * fields): - * - `reference`: an unmapped remappable resource (`kind`) - drops off the list once the user maps - * OR copies it (matched to a mapping entry by `${kind}:${sourceId}`). - * - `workflow`: a `workflow-selector`/`workflow_input` ref to a workflow not in the target - - * always cleared (cannot be fixed in the modal). - * - `dependent`: a create-target dependent selector a remapped parent clears. Carries the parent - * (`parentKind`/`parentSourceId`); when the child follows its parent (a document under a knowledge - * base) the client drops it once that parent is mapped/copied, else it stays (credential label / - * table column). + * - `reference`: an unmapped remappable resource (`kind`). BLOCKS the sync until the user maps it + * OR selects it for copy (matched to a mapping entry by `${kind}:${sourceId}`); the entry drops + * off the blocker list once resolved. + * - `workflow`: a `workflow-selector`/`workflow_input` ref to a workflow not carried into the + * target. BLOCKS the sync; resolved outside the modal (deploy the referenced workflow in the + * source, or remove/fix the reference). + * - `dependent`: a create-target dependent selector a remapped parent clears. NOT a blocker (the + * reconfigure flow owns dependents). Carries the parent (`parentKind`/`parentSourceId`); when the + * child follows its parent (a document under a knowledge base) the client drops it once that + * parent is mapped/copied, else it stays (credential label / table column). */ export const forkClearedRefSchema = z.discriminatedUnion('cause', [ forkClearedRefBaseSchema.extend({ cause: z.literal('reference'), /** The unmapped remappable resource (never `workflow`). */ kind: forkRemapKindSchema, + /** + * True when the referenced resource no longer exists (deleted/archived) in the SOURCE + * workspace, so it cannot be offered for copy - the resolution is mapping the dead source id + * to a live target resource, or fixing the source workflow. Collected as `false` and + * annotated post-collection by the source-liveness check (`annotateForkClearedRefSourceLiveness`). + */ + sourceDeleted: z.boolean(), }), forkClearedRefBaseSchema.extend({ cause: z.literal('workflow'), @@ -365,6 +374,42 @@ export const forkClearedRefSchema = z.discriminatedUnion('cause', [ ]) export type ForkClearedRef = z.output +/** + * Why a would-clear reference blocks the sync, so clients can phrase the resolution: + * - `unmapped-copyable`: a live copyable-kind resource (table / KB / file / custom tool / skill) + * with no target mapping - resolve by mapping it or selecting it for copy. + * - `unmapped-mcp-server`: a live external MCP server with no target mapping - resolve by mapping + * (MCP servers are never copied; create one in the target first if none exists). + * - `source-deleted`: the referenced resource was deleted in the source - resolve by mapping the + * dead id to an existing live target resource, or by fixing/archiving the source workflow. + * - `workflow-missing`: a cross-workflow reference to a workflow not carried into the target - + * resolve by deploying the referenced workflow in the source, or removing the reference. + */ +export const forkSyncBlockerReasonSchema = z.enum([ + 'unmapped-copyable', + 'unmapped-mcp-server', + 'source-deleted', + 'workflow-missing', +]) +export type ForkSyncBlockerReason = z.output + +/** + * One reference that blocked a promote at the server gate (the authoritative in-tx re-check of + * the would-clear set). Mirrors the cleared-ref labels so the client can phrase each blocker; + * `kind` is `workflow` for cross-workflow references. `sourceLabel` may fall back to `sourceId` + * (the gate skips display-label loading); the modal's refreshed diff carries the labeled list. + */ +export const forkSyncBlockerSchema = z.object({ + workflowName: z.string(), + blockLabel: z.string(), + fieldLabel: z.string(), + kind: z.union([forkRemapKindSchema, z.literal('workflow')]), + sourceId: z.string(), + sourceLabel: z.string(), + reason: forkSyncBlockerReasonSchema, +}) +export type ForkSyncBlocker = z.output + export const getForkDiffQuerySchema = z.object({ otherWorkspaceId: workspaceIdSchema, direction: forkDirectionSchema, @@ -395,11 +440,13 @@ export const getForkDiffContract = defineRouteContract({ /** Every workflow each mapped resource is used in, for the always-on reconfigure listing. */ resourceUsages: z.array(forkResourceUsageSchema), /** - * Referenced resources with no target mapping that the sync can copy into the target - * (fork-style), so the user can copy instead of mapping each one by hand. Default-selected - * in the modal; documents under a selected knowledge base are copied automatically. - * `parentId`/`parentLabel` carry the folder grouping for file entries (id + name); they - * are null for non-file kinds and for files at the workspace root. + * Copyable resources with no target mapping that the sync can copy into the target + * (fork-style). `referenced: true` entries are referenced by the synced workflows and + * default-selected in the modal (deselecting one clears its references); `referenced: false` + * entries exist in the source but are used by no synced workflow and default-unselected + * (skipping one breaks nothing). Documents under a selected knowledge base are copied + * automatically. `parentId`/`parentLabel` carry the folder grouping for file entries + * (id + name); they are null for non-file kinds and for files at the workspace root. */ copyableUnmapped: z.array( z.object({ @@ -408,6 +455,8 @@ export const getForkDiffContract = defineRouteContract({ label: z.string(), parentId: z.string().nullable(), parentLabel: z.string().nullable(), + /** Whether any synced workflow references this resource (drives the copy default). */ + referenced: z.boolean(), }) ), /** @@ -453,9 +502,10 @@ export const forkDependentValueEntrySchema = z.object({ export type ForkDependentValueEntry = z.input /** - * Source resource ids (by kind) the user chose to copy into the target before the sync gate, - * for referenced-but-unmapped resources. Each kind's documents under a copied knowledge base - * are discovered + copied automatically (the user selects only the parent resources). + * Source resource ids (by kind) the user chose to copy into the target before the sync gate - + * unmapped resources, whether referenced by the synced workflows or not. Each kind's documents + * under a copied knowledge base are discovered + copied automatically (the user selects only + * the parent resources). */ export const promoteCopyResourcesSchema = z.object({ knowledgeBases: forkResourceIdList, @@ -495,6 +545,12 @@ export const promoteForkContract = defineRouteContract({ redeployed: z.number().int(), deployFailed: z.number().int(), unmappedRequired: z.array(forkUnmappedReferenceSchema), + /** + * References the sync would have cleared, so it was blocked without writing (the + * authoritative in-tx gate; non-empty only when `promoteRunId` is empty). Normally the + * client blocks first - this fires only when the state changed between preview and Sync. + */ + blockers: z.array(forkSyncBlockerSchema), /** Workflows whose required dependent fields the target must re-pick post-sync. */ needsConfiguration: z.array(forkNeedsConfigurationSchema), /** Workflows whose optional dependent fields a swap cleared (surfaced, not gated). */ diff --git a/apps/sim/lib/workflows/persistence/remap-internal-ids.test.ts b/apps/sim/lib/workflows/persistence/remap-internal-ids.test.ts index d6261bc01c5..7cdc22f39b2 100644 --- a/apps/sim/lib/workflows/persistence/remap-internal-ids.test.ts +++ b/apps/sim/lib/workflows/persistence/remap-internal-ids.test.ts @@ -121,8 +121,10 @@ describe('remapWorkflowReferencesInSubBlocks', () => { // The `inputMapping` belongs to the ACTIVE canonical mode's workflow only. resolveCanonicalMode // picks the active mode (block.data.canonicalModes override, else the value heuristic); the wipe - // fires iff the ACTIVE mode's workflowId was removed by the remap. clearUnmapped: true throughout. - it('keeps inputMapping: active basic valid + dormant advanced stale (no override)', () => { + // fires iff the ACTIVE mode's workflow was removed by the remap. Only the SELECTOR is ever + // remapped/cleared - the manual member passes through verbatim - so an active-advanced (manual) + // mode never wipes inputMapping. clearUnmapped: true throughout. + it('keeps inputMapping: active basic valid + dormant advanced manual preserved (no override)', () => { const subBlocks: SubBlockRecord = { workflowId: { id: 'workflowId', type: 'workflow-selector', value: 'wf-src' }, manualWorkflowId: { id: 'manualWorkflowId', type: 'short-input', value: 'wf-unknown' }, @@ -130,11 +132,12 @@ describe('remapWorkflowReferencesInSubBlocks', () => { } const result = remapWorkflowReferencesInSubBlocks(subBlocks, map, { clearUnmapped: true }) expect(result.workflowId.value).toBe('wf-dst') - expect(result.manualWorkflowId.value).toBe('') + // Manual member is user-owned: preserved verbatim (never cleared), even while dormant. + expect(result.manualWorkflowId.value).toBe('wf-unknown') expect(result.inputMapping.value).toBe('{"a":"b"}') }) - it('wipes inputMapping: active advanced stale (canonicalModes override) + dormant basic valid', () => { + it('keeps inputMapping: active advanced manual preserved (canonicalModes override) + dormant basic remapped', () => { const subBlocks: SubBlockRecord = { workflowId: { id: 'workflowId', type: 'workflow-selector', value: 'wf-src' }, manualWorkflowId: { id: 'manualWorkflowId', type: 'short-input', value: 'wf-unknown' }, @@ -144,12 +147,14 @@ describe('remapWorkflowReferencesInSubBlocks', () => { clearUnmapped: true, canonicalModes: { workflowId: 'advanced' }, }) + // Active advanced manual is preserved, so its inputMapping survives; the dormant basic selector + // still remaps. expect(result.workflowId.value).toBe('wf-dst') - expect(result.manualWorkflowId.value).toBe('') - expect(result.inputMapping.value).toBe('') + expect(result.manualWorkflowId.value).toBe('wf-unknown') + expect(result.inputMapping.value).toBe('{"a":"b"}') }) - it('wipes inputMapping: active basic stale (heuristic) + dormant advanced valid', () => { + it('wipes inputMapping: active basic selector cleared (heuristic) + dormant advanced manual preserved', () => { const subBlocks: SubBlockRecord = { workflowId: { id: 'workflowId', type: 'workflow-selector', value: 'wf-unknown' }, manualWorkflowId: { id: 'manualWorkflowId', type: 'short-input', value: 'wf-src' }, @@ -157,22 +162,24 @@ describe('remapWorkflowReferencesInSubBlocks', () => { } const result = remapWorkflowReferencesInSubBlocks(subBlocks, map, { clearUnmapped: true }) expect(result.workflowId.value).toBe('') - expect(result.manualWorkflowId.value).toBe('wf-dst') + // Manual preserved verbatim (not remapped to wf-dst); the active basic selector clearing is what + // wipes inputMapping. + expect(result.manualWorkflowId.value).toBe('wf-src') expect(result.inputMapping.value).toBe('') }) - it('wipes inputMapping: active advanced stale + basic empty (heuristic)', () => { + it('keeps inputMapping: active advanced manual preserved + basic empty (heuristic)', () => { const subBlocks: SubBlockRecord = { workflowId: { id: 'workflowId', type: 'workflow-selector', value: '' }, manualWorkflowId: { id: 'manualWorkflowId', type: 'short-input', value: 'wf-unknown' }, inputMapping: { id: 'inputMapping', type: 'input-mapping', value: '{"a":"b"}' }, } const result = remapWorkflowReferencesInSubBlocks(subBlocks, map, { clearUnmapped: true }) - expect(result.manualWorkflowId.value).toBe('') - expect(result.inputMapping.value).toBe('') + expect(result.manualWorkflowId.value).toBe('wf-unknown') + expect(result.inputMapping.value).toBe('{"a":"b"}') }) - it('keeps inputMapping: both modes valid', () => { + it('keeps inputMapping: both modes valid (selector remapped, manual preserved)', () => { const subBlocks: SubBlockRecord = { workflowId: { id: 'workflowId', type: 'workflow-selector', value: 'wf-src' }, manualWorkflowId: { id: 'manualWorkflowId', type: 'short-input', value: 'sub-src' }, @@ -180,27 +187,27 @@ describe('remapWorkflowReferencesInSubBlocks', () => { } const result = remapWorkflowReferencesInSubBlocks(subBlocks, map, { clearUnmapped: true }) expect(result.workflowId.value).toBe('wf-dst') - expect(result.manualWorkflowId.value).toBe('sub-dst') + expect(result.manualWorkflowId.value).toBe('sub-src') expect(result.inputMapping.value).toBe('{"a":"b"}') }) - it('remaps the advanced-mode manualWorkflowId override', () => { + it('does not remap the advanced manualWorkflowId (manual is user-owned)', () => { const subBlocks: SubBlockRecord = { manualWorkflowId: { id: 'manualWorkflowId', type: 'short-input', value: 'wf-src' }, } const result = remapWorkflowReferencesInSubBlocks(subBlocks, map) - expect(result.manualWorkflowId.value).toBe('wf-dst') + expect(result.manualWorkflowId.value).toBe('wf-src') }) - it('remaps a comma-separated manualWorkflowIds list', () => { + it('does not remap the manual comma-separated manualWorkflowIds list (manual is user-owned)', () => { const subBlocks: SubBlockRecord = { manualWorkflowIds: { id: 'manualWorkflowIds', type: 'short-input', value: 'wf-src, sub-src' }, } const result = remapWorkflowReferencesInSubBlocks(subBlocks, map) - expect(result.manualWorkflowIds.value).toBe('wf-dst,sub-dst') + expect(result.manualWorkflowIds.value).toBe('wf-src, sub-src') }) - it('drops unmapped ids from a manualWorkflowIds list when clearUnmapped is set', () => { + it('preserves the manual manualWorkflowIds list verbatim even under clearUnmapped', () => { const subBlocks: SubBlockRecord = { manualWorkflowIds: { id: 'manualWorkflowIds', @@ -209,7 +216,47 @@ describe('remapWorkflowReferencesInSubBlocks', () => { }, } const result = remapWorkflowReferencesInSubBlocks(subBlocks, map, { clearUnmapped: true }) - expect(result.manualWorkflowIds.value).toBe('wf-dst') + expect(result.manualWorkflowIds.value).toBe('wf-src,wf-unknown') + }) + + // The advanced manual field is user-owned: ANY free-form value - env ref, literal id, tag, or + // arbitrary text - is preserved verbatim under clearUnmapped (active advanced), and its sibling + // inputMapping is never wiped. One passthrough covers every free-form edge case at once. + it.each([ + ['env ref', '{{MY_WORKFLOW_ID}}'], + ['literal source-workspace id', 'wf-unknown'], + ['block-output tag', ''], + ['arbitrary text', 'not an id at all'], + ])('preserves a manual %s value and its inputMapping (active advanced)', (_label, value) => { + const subBlocks: SubBlockRecord = { + workflowId: { id: 'workflowId', type: 'workflow-selector', value: '' }, + manualWorkflowId: { id: 'manualWorkflowId', type: 'short-input', value }, + inputMapping: { id: 'inputMapping', type: 'input-mapping', value: '{"a":"b"}' }, + } + const result = remapWorkflowReferencesInSubBlocks(subBlocks, map, { clearUnmapped: true }) + expect(result.manualWorkflowId.value).toBe(value) + expect(result.inputMapping.value).toBe('{"a":"b"}') + }) + + // The one behavioral change vs. selector handling: a literal source-workspace id typed into the + // MANUAL field that WOULD map to a copied target is left AS-IS (not remapped), because manual is + // user-owned - while the SELECTOR with the same id still remaps to the copied target. + it('leaves a mapped literal id in the manual field as-is while the selector remaps it', () => { + const manualSubBlocks: SubBlockRecord = { + manualWorkflowId: { id: 'manualWorkflowId', type: 'short-input', value: 'wf-src' }, + } + expect( + remapWorkflowReferencesInSubBlocks(manualSubBlocks, map, { clearUnmapped: true }) + .manualWorkflowId.value + ).toBe('wf-src') + + const selectorSubBlocks: SubBlockRecord = { + workflowId: { id: 'workflowId', type: 'workflow-selector', value: 'wf-src' }, + } + expect( + remapWorkflowReferencesInSubBlocks(selectorSubBlocks, map, { clearUnmapped: true }).workflowId + .value + ).toBe('wf-dst') }) it('remaps a multi-select workflowSelector array', () => { @@ -220,11 +267,64 @@ describe('remapWorkflowReferencesInSubBlocks', () => { expect(result.workflowSelector.value).toEqual(['wf-dst', 'sub-dst']) }) - // create-fork scopes its workflow id map to the workflows ACTUALLY copied (deployed state - // loaded). With BOTH the parent (`wf-src`) and child (`sub-src`) workflows copied, every - // reference variety must remap to the child id (NOT clear), even under fork-create's - // clearUnmapped policy - the explicit "both deployed and copied" guard. - it('remaps every reference variety when both referenced workflows are copied (clearUnmapped)', () => { + it('clears unmapped ids from the structured workflowSelector list under clearUnmapped', () => { + const subBlocks: SubBlockRecord = { + workflowSelector: { + id: 'workflowSelector', + type: 'dropdown', + value: ['wf-src', 'wf-unknown'], + }, + } + const result = remapWorkflowReferencesInSubBlocks(subBlocks, map, { clearUnmapped: true }) + expect(result.workflowSelector.value).toEqual(['wf-dst']) + }) + + // The sim workspace-event trigger's workflow filter: a multi-select `dropdown` with baseKey + // `workflowIds` whose options are workspace workflow ids - a structured (selector-sourced) + // list, remapped exactly like `workflowSelector`. + it('remaps the workspace-event trigger workflowIds dropdown list', () => { + const subBlocks: SubBlockRecord = { + workflowIds: { id: 'workflowIds', type: 'dropdown', value: ['wf-src', 'sub-src'] }, + } + const result = remapWorkflowReferencesInSubBlocks(subBlocks, map) + expect(result.workflowIds.value).toEqual(['wf-dst', 'sub-dst']) + }) + + it('drops unmapped ids from the workflowIds dropdown under clearUnmapped', () => { + const subBlocks: SubBlockRecord = { + workflowIds: { id: 'workflowIds', type: 'dropdown', value: ['wf-src', 'wf-unknown'] }, + } + const result = remapWorkflowReferencesInSubBlocks(subBlocks, map, { clearUnmapped: true }) + expect(result.workflowIds.value).toEqual(['wf-dst']) + }) + + // The TYPE gate: the legacy logs block's `workflowIds` is a free-form short-input (manual, + // user-owned), so it must pass through verbatim even though its baseKey matches. + it('leaves a short-input workflowIds (legacy logs block, user-owned) untouched under clearUnmapped', () => { + const subBlocks: SubBlockRecord = { + workflowIds: { id: 'workflowIds', type: 'short-input', value: 'wf-src,wf-unknown' }, + } + const result = remapWorkflowReferencesInSubBlocks(subBlocks, map, { clearUnmapped: true }) + expect(result.workflowIds.value).toBe('wf-src,wf-unknown') + }) + + // The baseKey gate: dropdowns whose baseKey is neither `workflowSelector` nor `workflowIds` + // (event pickers, status filters, ...) hold non-workflow values and are never rewritten. + it('leaves other dropdowns untouched (only workflow-list baseKeys are remapped)', () => { + const subBlocks: SubBlockRecord = { + eventType: { id: 'eventType', type: 'dropdown', value: 'wf-src' }, + level: { id: 'level', type: 'dropdown', value: ['wf-src'] }, + } + const result = remapWorkflowReferencesInSubBlocks(subBlocks, map, { clearUnmapped: true }) + expect(result.eventType.value).toBe('wf-src') + expect(result.level.value).toEqual(['wf-src']) + }) + + // create-fork scopes its workflow id map to the workflows ACTUALLY copied (deployed state loaded). + // With BOTH `wf-src` and `sub-src` copied, the SELECTOR varieties remap to the child ids; the + // free-form MANUAL varieties (`manualWorkflowId`, `manualWorkflowIds`) are user-owned and pass + // through verbatim, even under fork-create's clearUnmapped policy. + it('remaps selector varieties and preserves manual varieties when both workflows are copied (clearUnmapped)', () => { const subBlocks: SubBlockRecord = { selector: { id: 'selector', type: 'workflow-selector', value: 'wf-src' }, inputMapping: { id: 'inputMapping', type: 'input-mapping', value: '{"a":"b"}' }, @@ -243,17 +343,19 @@ describe('remapWorkflowReferencesInSubBlocks', () => { const result = remapWorkflowReferencesInSubBlocks(subBlocks, map, { clearUnmapped: true }) expect(result.selector.value).toBe('wf-dst') expect(result.inputMapping.value).toBe('{"a":"b"}') - expect(result.manualWorkflowId.value).toBe('sub-dst') - expect(result.manualWorkflowIds.value).toBe('wf-dst,sub-dst') + // Manual varieties pass through verbatim (not remapped to the child ids). + expect(result.manualWorkflowId.value).toBe('sub-src') + expect(result.manualWorkflowIds.value).toBe('wf-src, sub-src') expect(result.workflowSelector.value).toEqual(['wf-dst', 'sub-dst']) const tools = result.tools.value as Array<{ type: string; params?: { workflowId?: string } }> expect(tools[0].params?.workflowId).toBe('sub-dst') expect(tools[1]).toEqual({ type: 'custom-tool', customToolId: 'ct-1' }) }) - // A deployed source workflow whose state failed to load is excluded from the scoped fork map, - // so a copied workflow's reference to it clears (never dangles at a never-created child id). - it('clears references to a deployed-but-uncopied workflow absent from the scoped map', () => { + // A deployed source workflow whose state failed to load is excluded from the scoped fork map, so a + // copied workflow's SELECTOR reference to it clears (never dangles at a never-created child id). The + // free-form manual list is user-owned and preserved verbatim. + it('clears selector references to a deployed-but-uncopied workflow (manual list preserved)', () => { const subBlocks: SubBlockRecord = { selector: { id: 'selector', type: 'workflow-selector', value: 'wf-uncopied' }, inputMapping: { id: 'inputMapping', type: 'input-mapping', value: '{"a":"b"}' }, @@ -271,7 +373,7 @@ describe('remapWorkflowReferencesInSubBlocks', () => { const result = remapWorkflowReferencesInSubBlocks(subBlocks, map, { clearUnmapped: true }) expect(result.selector.value).toBe('') expect(result.inputMapping.value).toBe('') - expect(result.manualWorkflowIds.value).toBe('wf-dst') + expect(result.manualWorkflowIds.value).toBe('wf-src,wf-uncopied') expect(result.tools.value as unknown[]).toHaveLength(0) }) }) diff --git a/apps/sim/lib/workflows/persistence/remap-internal-ids.ts b/apps/sim/lib/workflows/persistence/remap-internal-ids.ts index d21d12c6e1e..5a42bcd4fce 100644 --- a/apps/sim/lib/workflows/persistence/remap-internal-ids.ts +++ b/apps/sim/lib/workflows/persistence/remap-internal-ids.ts @@ -126,17 +126,24 @@ export function remapVariableIdsInSubBlocks( } /** - * Rewrite cross-workflow references through a workflow id map: single - * `workflow-selector` / `manualWorkflowId` values, multi-workflow lists - * (`workflowSelector` multi-select + comma-separated `manualWorkflowIds`, as used - * by the logs block), and `workflow_input` sub-workflow tools nested in a - * `tool-input` array (an agent calling another workflow as a tool). + * Rewrite cross-workflow references through a workflow id map. Only SELECTOR-sourced (structured) + * references are remapped/cleared: the basic `workflow-selector` value, the multi-select + * `workflowSelector` list (logs block), the workspace-event trigger's multi-select `workflowIds` + * dropdown (its options are workspace workflow ids), and `workflow_input` sub-workflow tools + * nested in a `tool-input` array (an agent picking another workflow as a tool - its + * `params.workflowId` comes from the workflow picker, never free-form input). * - * `clearUnmapped` controls the cross-workspace case: fork/promote pass `true` so a - * reference to a workflow that wasn't copied is cleared/dropped rather than left - * pointing at the source workspace (a silent cross-workspace execution). Same- - * workspace duplication leaves it `false` to preserve references to untouched - * sibling workflows. + * The advanced, free-form MANUAL fields (`manualWorkflowId`, comma-separated `manualWorkflowIds`) + * are user-owned and pass through VERBATIM - mirroring `manualCredential` in the fork remap - so a + * hand-typed value (an env ref `{{VAR}}`, a `` tag, a literal id, or arbitrary text) + * is never rewritten or cleared. The `workflowIds` handling is gated on subblock TYPE `dropdown` + * for the same reason: the legacy logs block's `workflowIds` is a free-form `short-input` + * (user-owned, verbatim), and only the workspace-event trigger uses a `workflowIds` dropdown. + * + * `clearUnmapped` controls the cross-workspace case for those selector references: fork/promote pass + * `true` so a selector pointing at a workflow that wasn't copied is cleared/dropped rather than left + * pointing at the source workspace (a silent cross-workspace execution). Same-workspace duplication + * leaves it `false` to preserve references to untouched sibling workflows. */ export function remapWorkflowReferencesInSubBlocks( subBlocks: SubBlockRecord, @@ -152,7 +159,8 @@ export function remapWorkflowReferencesInSubBlocks( } // The `workflowId` canonical pair: basic `workflow-selector` + advanced `manualWorkflowId`. Capture // each key (by type/baseKey, regardless of value) and its ORIGINAL value so the inputMapping wipe - // below can decide on the ACTIVE mode's disposition via `resolveCanonicalMode`. + // below can decide on the ACTIVE mode's disposition via `resolveCanonicalMode`. Only the basic + // selector is ever remapped; the advanced manual member is captured for mode resolution only. let basicId: string | undefined let basicValue = '' let advancedId: string | undefined @@ -168,15 +176,23 @@ export function remapWorkflowReferencesInSubBlocks( advancedId = key advancedValue = typeof subBlock.value === 'string' ? subBlock.value : '' } + // Remap only the SELECTOR member; the manual `manualWorkflowId` passes through verbatim. if ( - (subBlock.type === 'workflow-selector' || baseKey === 'manualWorkflowId') && + subBlock.type === 'workflow-selector' && typeof subBlock.value === 'string' && subBlock.value ) { updated[key] = { ...subBlock, value: remapScalar(subBlock.value) } continue } - if (baseKey === 'manualWorkflowIds' || baseKey === 'workflowSelector') { + // Remap only the STRUCTURED multi-workflow lists: the logs block's `workflowSelector` and + // the workspace-event trigger's `workflowIds` dropdown. The latter is gated on TYPE + // `dropdown` so the legacy logs block's `workflowIds` short-input (manual, user-owned) + // passes through verbatim, as does the manual comma-separated `manualWorkflowIds`. + if ( + baseKey === 'workflowSelector' || + (subBlock.type === 'dropdown' && baseKey === 'workflowIds') + ) { const remapped = remapWorkflowIdList(subBlock.value, workflowIdMap, clearUnmapped) if (remapped !== subBlock.value) { updated[key] = { ...subBlock, value: remapped } diff --git a/apps/sim/lib/workspaces/fork/copy/cleanup-failed.ts b/apps/sim/lib/workspaces/fork/copy/cleanup-failed.ts index 5f557f68f7a..936a777a82d 100644 --- a/apps/sim/lib/workspaces/fork/copy/cleanup-failed.ts +++ b/apps/sim/lib/workspaces/fork/copy/cleanup-failed.ts @@ -75,6 +75,14 @@ function clearFailedSubBlockReferences( * when a reference-clear phase threw - placeholders were then NOT dropped - and `cleared` is 0 in * that case, so the report never claims references it did not actually clear. On success `cleared` * is the count of failed resources whose references were cleared. + * + * Storage accounting: this cleanup never decrements storage usage because it never removes + * anything that was counted. Copied file blobs are the only counted copies (incremented in + * `executeForkFileBlobCopies` only after the blob lands), and a failed file's blob never + * landed - its metadata row is intentionally left re-uploadable, and nothing was charged. The + * dropped table/KB/document placeholders are DB rows the upload path never counts, and any KB + * blobs copied before their KB failed are left in storage (rows only are dropped here) but + * uncounted - mirroring the KB upload path, which never counts KB blobs. */ export async function clearFailedForkResourceReferences(params: { childWorkspaceId: string diff --git a/apps/sim/lib/workspaces/fork/copy/copy-files.test.ts b/apps/sim/lib/workspaces/fork/copy/copy-files.test.ts new file mode 100644 index 00000000000..e0b5c3c0e58 --- /dev/null +++ b/apps/sim/lib/workspaces/fork/copy/copy-files.test.ts @@ -0,0 +1,158 @@ +/** + * @vitest-environment node + */ +import { storageServiceMock, storageServiceMockFns } from '@sim/testing' +import { omit } from '@sim/utils/object' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { mockIncrementStorageUsage } = vi.hoisted(() => ({ + mockIncrementStorageUsage: vi.fn(), +})) + +vi.mock('@/lib/uploads/core/storage-service', () => storageServiceMock) +vi.mock('@/lib/billing/storage', () => ({ + incrementStorageUsage: mockIncrementStorageUsage, +})) +vi.mock('@/lib/uploads/contexts/workspace/workspace-file-manager', () => ({ + generateWorkspaceFileKey: vi.fn( + (workspaceId: string, fileName: string) => `workspace/${workspaceId}/generated-${fileName}` + ), +})) + +import type { DbOrTx } from '@/lib/db/types' +import { + type BlobCopyTask, + executeForkFileBlobCopies, + planForkFileCopies, +} from '@/lib/workspaces/fork/copy/copy-files' + +function makeTask(overrides: Partial = {}): BlobCopyTask { + return { + sourceKey: 'workspace/src-ws/source-a.txt', + targetKey: 'workspace/child-ws/target-a.txt', + context: 'workspace', + fileName: 'a.txt', + contentType: 'text/plain', + size: 100, + userId: 'user-1', + workspaceId: 'child-ws', + ...overrides, + } +} + +describe('executeForkFileBlobCopies storage accounting', () => { + beforeEach(() => { + vi.clearAllMocks() + storageServiceMockFns.mockHeadObject.mockResolvedValue(null) + storageServiceMockFns.mockDownloadFile.mockResolvedValue(Buffer.from('blob-bytes')) + storageServiceMockFns.mockUploadFile.mockResolvedValue({ key: 'workspace/child-ws/target' }) + mockIncrementStorageUsage.mockResolvedValue(undefined) + }) + + it('charges the initiating user exactly once per landed blob, by the metadata row size', async () => { + const tasks = [ + makeTask({ targetKey: 'workspace/child-ws/t1', size: 100 }), + makeTask({ targetKey: 'workspace/child-ws/t2', size: 200, fileName: 'b.txt' }), + ] + + const result = await executeForkFileBlobCopies(tasks, 'test') + + expect(result).toEqual({ copied: 2, failed: 0, failedTargetKeys: [] }) + expect(mockIncrementStorageUsage).toHaveBeenCalledTimes(2) + expect(mockIncrementStorageUsage).toHaveBeenNthCalledWith(1, 'user-1', 100, 'child-ws') + expect(mockIncrementStorageUsage).toHaveBeenNthCalledWith(2, 'user-1', 200, 'child-ws') + }) + + it('skips an already-existing target blob without re-copying or re-charging (replayed run)', async () => { + storageServiceMockFns.mockHeadObject.mockResolvedValue({ size: 100 }) + + const result = await executeForkFileBlobCopies([makeTask()], 'test') + + expect(result).toEqual({ copied: 1, failed: 0, failedTargetKeys: [] }) + expect(storageServiceMockFns.mockDownloadFile).not.toHaveBeenCalled() + expect(storageServiceMockFns.mockUploadFile).not.toHaveBeenCalled() + expect(mockIncrementStorageUsage).not.toHaveBeenCalled() + }) + + it('never charges a failed copy (the blob did not land)', async () => { + storageServiceMockFns.mockDownloadFile.mockRejectedValue(new Error('source gone')) + + const result = await executeForkFileBlobCopies([makeTask()], 'test') + + expect(result).toEqual({ + copied: 0, + failed: 1, + failedTargetKeys: ['workspace/child-ws/target-a.txt'], + }) + expect(mockIncrementStorageUsage).not.toHaveBeenCalled() + }) + + it('treats a tracking failure as best-effort - the copy still counts as landed', async () => { + mockIncrementStorageUsage.mockRejectedValue(new Error('billing hiccup')) + + const result = await executeForkFileBlobCopies([makeTask()], 'test') + + expect(result).toEqual({ copied: 1, failed: 0, failedTargetKeys: [] }) + expect(storageServiceMockFns.mockUploadFile).toHaveBeenCalledTimes(1) + }) + + it('skips the charge for a legacy payload enqueued before size existed', async () => { + // Simulates a Trigger.dev payload serialized by a pre-`size` deploy (rolling upgrade). + const legacyTask = omit(makeTask(), ['size']) as BlobCopyTask + + const result = await executeForkFileBlobCopies([legacyTask], 'test') + + expect(result.copied).toBe(1) + expect(mockIncrementStorageUsage).not.toHaveBeenCalled() + }) +}) + +describe('planForkFileCopies', () => { + it('carries the source metadata size onto each blob task and the child row', async () => { + const sourceMeta = { + id: 'wf_src1', + key: 'workspace/src-ws/1-abc-a.txt', + userId: 'uploader-1', + workspaceId: 'src-ws', + folderId: 'folder-1', + context: 'workspace', + chatId: null, + originalName: 'a.txt', + displayName: null, + contentType: 'text/plain', + size: 4321, + deletedAt: null, + uploadedAt: new Date('2026-01-01'), + updatedAt: new Date('2026-01-01'), + } + const inserted: Array> = [] + const tx = { + select: vi.fn(() => ({ from: () => ({ where: () => Promise.resolve([sourceMeta]) }) })), + insert: vi.fn(() => ({ + values: (row: Record) => { + inserted.push(row) + return Promise.resolve() + }, + })), + } as unknown as DbOrTx + + const result = await planForkFileCopies({ + tx, + sourceWorkspaceId: 'src-ws', + childWorkspaceId: 'child-ws', + userId: 'user-1', + fileIds: ['wf_src1'], + now: new Date('2026-02-01'), + }) + + expect(result.blobTasks).toHaveLength(1) + expect(result.blobTasks[0]).toMatchObject({ + sourceKey: 'workspace/src-ws/1-abc-a.txt', + targetKey: 'workspace/child-ws/generated-a.txt', + size: 4321, + userId: 'user-1', + workspaceId: 'child-ws', + }) + expect(inserted[0]).toMatchObject({ size: 4321, workspaceId: 'child-ws' }) + }) +}) diff --git a/apps/sim/lib/workspaces/fork/copy/copy-files.ts b/apps/sim/lib/workspaces/fork/copy/copy-files.ts index e7e8f5080eb..828aa16513a 100644 --- a/apps/sim/lib/workspaces/fork/copy/copy-files.ts +++ b/apps/sim/lib/workspaces/fork/copy/copy-files.ts @@ -3,9 +3,10 @@ import { createLogger } from '@sim/logger' import { getErrorMessage } from '@sim/utils/errors' import { generateId } from '@sim/utils/id' import { and, eq, inArray, isNull, or } from 'drizzle-orm' +import { incrementStorageUsage } from '@/lib/billing/storage' import type { DbOrTx } from '@/lib/db/types' import { generateWorkspaceFileKey } from '@/lib/uploads/contexts/workspace/workspace-file-manager' -import { downloadFile, uploadFile } from '@/lib/uploads/core/storage-service' +import { downloadFile, headObject, uploadFile } from '@/lib/uploads/core/storage-service' import type { StorageContext } from '@/lib/uploads/shared/types' import { MAX_FILE_SIZE } from '@/lib/uploads/utils/validation' import { @@ -30,6 +31,13 @@ export interface BlobCopyTask { context: StorageContext fileName: string contentType: string + /** + * Byte size from the source metadata row - the child `workspace_files` row was inserted + * with this same size, so the storage-usage increment after a successful blob copy + * charges exactly the bytes the row advertises (matching the upload path, where the + * incremented bytes always equal the row's `size`). + */ + size: number userId: string workspaceId: string } @@ -127,6 +135,7 @@ export async function planForkFileCopies(params: { context: meta.context as StorageContext, fileName: meta.originalName, contentType: meta.contentType, + size: meta.size, userId, workspaceId: childWorkspaceId, }) @@ -145,6 +154,17 @@ export async function planForkFileCopies(params: { * blob's child storage key is returned in `failedTargetKeys` so the caller can clear the * `file-upload` references pointing at the now-missing object (the metadata row is left in * place, so the user can still re-upload the blob). + * + * Storage accounting: each blob that actually lands increments the initiating user's + * storage usage by the metadata row's size - the copied bytes are charged exactly as if + * the file had been uploaded to the target workspace. The increment cannot double-count: + * the content-copy job is at-most-once by config (`maxAttempts: 1`), each task increments + * only after its own successful upload, and the target-existence skip below means a + * manually replayed run neither re-copies nor re-charges a blob a prior attempt landed. + * Like the upload path, a tracking failure is logged and never fails the copy - and is + * never retried, so a landed blob whose increment failed stays uncounted (a manual replay + * skips it without charging). Accepted trade-off, matching the platform's upload paths: + * storage may undercount, but a user is never charged twice or for bytes that didn't land. */ export async function executeForkFileBlobCopies( blobTasks: BlobCopyTask[], @@ -155,6 +175,18 @@ export async function executeForkFileBlobCopies( const failedTargetKeys: string[] = [] for (const task of blobTasks) { try { + // Replay guard: target keys are freshly generated per fork/sync, so an existing + // object can only mean an earlier attempt already landed this exact copy. Skip + // without incrementing - a replay must never double-charge, so if the prior + // attempt's best-effort increment failed those bytes stay uncounted (the same + // accepted undercount as a tracking failure on the upload path). `headObject` + // returns null on local storage, where the copy is simply repeated (same bytes + // to the same key). + const existing = await headObject(task.targetKey, task.context) + if (existing) { + copied += 1 + continue + } const buffer = await downloadFile({ key: task.sourceKey, context: task.context, @@ -187,6 +219,17 @@ export async function executeForkFileBlobCopies( }, }) copied += 1 + // The typeof guard covers payloads enqueued before `size` existed (rolling deploy). + if (typeof task.size === 'number' && task.size > 0) { + try { + await incrementStorageUsage(task.userId, task.size, task.workspaceId) + } catch (storageError) { + logger.error(`[${requestId}] Failed to update storage tracking for copied file blob`, { + targetKey: task.targetKey, + error: getErrorMessage(storageError), + }) + } + } } catch (error) { failedTargetKeys.push(task.targetKey) logger.warn(`[${requestId}] Failed to copy file blob during fork`, { diff --git a/apps/sim/lib/workspaces/fork/copy/copy-resources.test.ts b/apps/sim/lib/workspaces/fork/copy/copy-resources.test.ts index a1ff7fb971d..e33be42700f 100644 --- a/apps/sim/lib/workspaces/fork/copy/copy-resources.test.ts +++ b/apps/sim/lib/workspaces/fork/copy/copy-resources.test.ts @@ -10,8 +10,15 @@ import { } from '@sim/testing' import { beforeEach, describe, expect, it, vi } from 'vitest' +const { mockIncrementStorageUsage } = vi.hoisted(() => ({ + mockIncrementStorageUsage: vi.fn(), +})) + vi.mock('@sim/db', () => dbChainMock) vi.mock('@/lib/uploads/core/storage-service', () => storageServiceMock) +vi.mock('@/lib/billing/storage', () => ({ + incrementStorageUsage: mockIncrementStorageUsage, +})) import type { DbOrTx } from '@/lib/db/types' import { @@ -115,6 +122,24 @@ describe('copyForkResourceContent', () => { }) }) + it('#1 never touches storage accounting for copied KB document blobs (mirrors the KB upload path)', async () => { + // The normal KB upload path never increments `storage_used_bytes` (and embeddings are + // uncounted DB rows), so a fork-copied KB blob must not be charged either - copied KB + // bytes are only headroom-checked pre-fork, exactly like the multipart-initiate check. + dbChainMockFns.limit.mockResolvedValueOnce([sourceDoc]) + + const result = await copyForkResourceContent({ + contentPlan: basePlan({ + knowledgeBases: [{ sourceId: 'src-kb', childId: 'child-kb', documentIdMap: {} }], + }), + requestId: 'test', + }) + + expect(result.copied).toBe(1) + expect(storageServiceMockFns.mockUploadFile).toHaveBeenCalledTimes(1) + expect(mockIncrementStorageUsage).not.toHaveBeenCalled() + }) + it('#4 re-reads a copied skill body post-commit and rewrites it via db.update (never from payload)', async () => { // The body is no longer carried in the plan - the content phase keyset-re-reads the child row. dbChainMockFns.limit.mockResolvedValueOnce([ @@ -355,6 +380,101 @@ describe('copyForkResourceContainers skill copy', () => { }) }) +describe('copyForkResourceContainers knowledge-base tag definitions', () => { + /** Sequential tx mock: each select resolves the next queued row set; inserts are captured per call. */ + function makeKbTx(selects: Array>>) { + let call = 0 + const inserts: Array>> = [] + const tx = { + select: () => ({ + from: () => ({ where: () => Promise.resolve(selects[call++] ?? []) }), + }), + insert: () => ({ + values: (rows: Array>) => { + inserts.push(rows) + return Promise.resolve() + }, + }), + } + return { tx: tx as unknown as DbOrTx, inserts } + } + + const kbSelection = { + customTools: [], + skills: [], + workflowMcpServers: [], + tables: [], + knowledgeBases: ['kb-1'], + } + + const sourceBase = { id: 'kb-1', name: 'Docs KB', workspaceId: 'src-ws', deletedAt: null } + + it('copies the source KB tag definitions to the child KB with fresh ids (other columns verbatim)', async () => { + const { tx, inserts } = makeKbTx([ + [sourceBase], + [ + { + id: 'tag-1', + knowledgeBaseId: 'kb-1', + tagSlot: 'tag1', + displayName: 'Category', + fieldType: 'text', + }, + { + id: 'tag-2', + knowledgeBaseId: 'kb-1', + tagSlot: 'boolean1', + displayName: 'Reviewed', + fieldType: 'boolean', + }, + ], + ]) + + const result = await copyForkResourceContainers({ + tx, + sourceWorkspaceId: 'src-ws', + childWorkspaceId: 'child-ws', + userId: 'user-1', + now: new Date(), + selection: kbSelection, + workflowIdMap: new Map(), + }) + + const childKbId = result.idMap.get('knowledge_base')?.get('kb-1') + expect(childKbId).toBeTruthy() + // insert #0 is the KB row; insert #1 is the tag-definition batch. + expect(inserts).toHaveLength(2) + const tagRows = inserts[1] + expect(tagRows).toHaveLength(2) + for (const row of tagRows) { + expect(row.knowledgeBaseId).toBe(childKbId) + expect(row.id).not.toBe('tag-1') + expect(row.id).not.toBe('tag-2') + } + expect(tagRows.map((row) => [row.tagSlot, row.displayName, row.fieldType])).toEqual([ + ['tag1', 'Category', 'text'], + ['boolean1', 'Reviewed', 'boolean'], + ]) + }) + + it('no-ops the tag-definition copy for a KB with zero definitions', async () => { + const { tx, inserts } = makeKbTx([[sourceBase], []]) + + await copyForkResourceContainers({ + tx, + sourceWorkspaceId: 'src-ws', + childWorkspaceId: 'child-ws', + userId: 'user-1', + now: new Date(), + selection: kbSelection, + workflowIdMap: new Map(), + }) + + // Only the KB row itself is inserted - no empty tag-definition insert. + expect(inserts).toHaveLength(1) + }) +}) + describe('planForkMappedKbDocumentCopies', () => { const sourceRow = (id: string, knowledgeBaseId: string) => ({ id, diff --git a/apps/sim/lib/workspaces/fork/copy/copy-resources.ts b/apps/sim/lib/workspaces/fork/copy/copy-resources.ts index 5f78426b2b8..a6b3779c4ce 100644 --- a/apps/sim/lib/workspaces/fork/copy/copy-resources.ts +++ b/apps/sim/lib/workspaces/fork/copy/copy-resources.ts @@ -4,6 +4,7 @@ import { document, embedding, knowledgeBase, + knowledgeBaseTagDefinitions, skill, userTableDefinitions, userTableRows, @@ -24,6 +25,7 @@ import type { ForkMappingUpsert, ForkResourceType, } from '@/lib/workspaces/fork/mapping/mapping-store' +import type { ForkBlockIdResolver } from '@/lib/workspaces/fork/remap/block-identity' import { type ForkContentRefMaps, rewriteForkContentRefs, @@ -83,6 +85,13 @@ export interface CopyResourcesParams { * plan resolver); omitted by fork-create, which preserves env names verbatim (no rewrite). */ resolveEnvName?: (key: string) => string | null | undefined + /** + * Resolve a source block id to its target block id for copied tables' workflow-group + * `outputs[].blockId`. Promote passes the SAME persisted-pair resolver its workflow writes + * use (on push the parent keeps its ORIGINAL block ids, never the derive); fork-create + * omits it, defaulting to the deterministic derive (a fresh child has no pairs). + */ + resolveBlockId?: ForkBlockIdResolver } export interface ForkContentPlanEntry { @@ -199,8 +208,9 @@ type SkillSkeletonInsert = Omit & { conten /** * Copy the selected resources' **container rows** into the child workspace inside * the fork transaction: custom tools, skills, and MCP server configs (each a - * single row), plus table definitions and knowledge-base rows (without their bulk - * rows / documents / embeddings). This keeps the fork transaction bounded to + * single row), plus table definitions and knowledge-base rows with their tag + * definitions (bounded per KB) but without their bulk rows / documents / + * embeddings. This keeps the fork transaction bounded to * O(selected resources) single-row writes. The heavy content (table rows, KB * documents + embeddings) is returned as a {@link ForkContentPlan} for * {@link copyForkResourceContent} to copy best-effort after commit. Secrets are @@ -359,7 +369,8 @@ export async function copyForkResourceContainers( const childTableId = generateId() const remappedSchema = remapForkTableWorkflowGroups( definition.schema as TableSchema, - workflowIdMap + workflowIdMap, + params.resolveBlockId ) inserts.push({ ...definition, @@ -414,6 +425,34 @@ export async function copyForkResourceContainers( } if (inserts.length > 0) await tx.insert(knowledgeBase).values(inserts) + // Copy each source KB's tag definitions to its child so tagged documents keep a working tag + // schema: the copied documents carry tag VALUES in their slot columns, and both tag-filter + // search and documentTags writes resolve display names through these definition rows (a copy + // without them 400s / throws on every defined tag). Fresh ids, child KB id, all other columns + // verbatim - nothing persists a tag-definition id (workflow state, documents, and fork + // mappings all reference tags by display name / slot), so no id map is recorded. + if (kbEntryBySourceId.size > 0) { + const tagDefinitions = await tx + .select() + .from(knowledgeBaseTagDefinitions) + .where( + inArray(knowledgeBaseTagDefinitions.knowledgeBaseId, Array.from(kbEntryBySourceId.keys())) + ) + const tagDefinitionInserts: (typeof knowledgeBaseTagDefinitions.$inferInsert)[] = [] + for (const definition of tagDefinitions) { + const childKbId = kbEntryBySourceId.get(definition.knowledgeBaseId)?.childId + if (!childKbId) continue + tagDefinitionInserts.push({ + ...definition, + id: generateId(), + knowledgeBaseId: childKbId, + }) + } + if (tagDefinitionInserts.length > 0) { + await tx.insert(knowledgeBaseTagDefinitions).values(tagDefinitionInserts) + } + } + // Pre-create placeholder document rows for the documents the copied workflows // reference, at child ids generated inside the transaction, so each // `document-selector` reference can be remapped to a valid copied document rather diff --git a/apps/sim/lib/workspaces/fork/copy/copy-workflows.test.ts b/apps/sim/lib/workspaces/fork/copy/copy-workflows.test.ts index e141d65e7b4..74b7ade28f9 100644 --- a/apps/sim/lib/workspaces/fork/copy/copy-workflows.test.ts +++ b/apps/sim/lib/workspaces/fork/copy/copy-workflows.test.ts @@ -1,8 +1,22 @@ /** * @vitest-environment node */ -import { describe, expect, it } from 'vitest' -import { buildWorkflowNameRegistry } from '@/lib/workspaces/fork/copy/copy-workflows' +import { describe, expect, it, vi } from 'vitest' +import type { DbOrTx } from '@/lib/db/types' + +const { mockSaveWorkflowToNormalizedTables } = vi.hoisted(() => ({ + mockSaveWorkflowToNormalizedTables: vi.fn(), +})) + +vi.mock('@/lib/workflows/persistence/utils', () => ({ + saveWorkflowToNormalizedTables: mockSaveWorkflowToNormalizedTables, +})) + +import { + buildWorkflowNameRegistry, + copyWorkflowStateIntoTarget, + resolveForkFolderMapping, +} from '@/lib/workspaces/fork/copy/copy-workflows' describe('buildWorkflowNameRegistry', () => { it('reports a name as taken by another workflow in the same folder', () => { @@ -60,3 +74,222 @@ describe('buildWorkflowNameRegistry', () => { expect(reg.isTaken('f1', 'Dup', 'w1')).toBe(false) }) }) + +interface FolderRow { + id: string + name: string + userId: string + workspaceId: string + parentId: string | null + color: string | null + isExpanded: boolean + locked: boolean + sortOrder: number + createdAt: Date + updatedAt: Date + archivedAt: Date | null +} + +function folderRow(id: string, name: string, parentId: string | null = null): FolderRow { + return { + id, + name, + userId: 'source-user', + workspaceId: 'ws-source', + parentId, + color: '#6B7280', + isExpanded: true, + locked: false, + sortOrder: 0, + createdAt: new Date('2026-01-01'), + updatedAt: new Date('2026-01-01'), + archivedAt: null, + } +} + +/** + * Transaction stub for {@link resolveForkFolderMapping}: the first awaited select resolves + * the source folders, the second the target folders, and inserted rows are captured. + */ +function buildFolderTx(sourceFolders: FolderRow[], targetFolders: FolderRow[] = []) { + const insertedRows: FolderRow[] = [] + const selects = [sourceFolders, targetFolders] + let selectIndex = 0 + const tx = { + select: () => ({ + from: () => ({ + where: () => Promise.resolve(selects[selectIndex++] ?? []), + }), + }), + insert: () => ({ + values: (rows: FolderRow[]) => { + insertedRows.push(...rows) + return Promise.resolve() + }, + }), + } as unknown as DbOrTx + return { tx, insertedRows } +} + +function resolveMapping(params: { + tx: DbOrTx + contentFolderIds: ReadonlyArray +}): Promise> { + return resolveForkFolderMapping({ + tx: params.tx, + sourceWorkspaceId: 'ws-source', + targetWorkspaceId: 'ws-target', + userId: 'target-user', + now: new Date('2026-07-01'), + contentFolderIds: params.contentFolderIds, + }) +} + +describe('resolveForkFolderMapping', () => { + it('keeps the full ancestor chain of a nested folder holding a copied workflow', async () => { + const { tx, insertedRows } = buildFolderTx([ + folderRow('A', 'Alpha'), + folderRow('B', 'Beta', 'A'), + folderRow('C', 'Gamma', 'B'), + ]) + + const map = await resolveMapping({ tx, contentFolderIds: ['C'] }) + + expect(map.size).toBe(3) + expect(insertedRows).toHaveLength(3) + const byName = new Map(insertedRows.map((row) => [row.name, row])) + expect(byName.get('Alpha')?.parentId).toBeNull() + expect(byName.get('Beta')?.parentId).toBe(map.get('A')) + expect(byName.get('Gamma')?.parentId).toBe(map.get('B')) + for (const row of insertedRows) { + expect(row.workspaceId).toBe('ws-target') + expect(row.userId).toBe('target-user') + expect(row.locked).toBe(false) + expect(['A', 'B', 'C']).not.toContain(row.id) + } + }) + + it('prunes an empty sibling subtree while keeping the occupied folder', async () => { + const { tx, insertedRows } = buildFolderTx([ + folderRow('A', 'Occupied'), + folderRow('D', 'Empty parent'), + folderRow('E', 'Empty child', 'D'), + ]) + + const map = await resolveMapping({ tx, contentFolderIds: ['A'] }) + + expect(insertedRows).toHaveLength(1) + expect(insertedRows[0].name).toBe('Occupied') + expect(map.has('A')).toBe(true) + expect(map.has('D')).toBe(false) + expect(map.has('E')).toBe(false) + }) + + it('prunes a root-level empty folder when the copied workflows live at root', async () => { + const { tx, insertedRows } = buildFolderTx([folderRow('F', 'Never used')]) + + const map = await resolveMapping({ tx, contentFolderIds: [null, null] }) + + expect(insertedRows).toHaveLength(0) + expect(map.size).toBe(0) + }) + + it('creates no folders when nothing is copied into any folder', async () => { + const { tx, insertedRows } = buildFolderTx([ + folderRow('A', 'Alpha'), + folderRow('B', 'Beta', 'A'), + ]) + + const map = await resolveMapping({ tx, contentFolderIds: [] }) + + expect(insertedRows).toHaveLength(0) + expect(map.size).toBe(0) + }) + + it('reuses an existing target folder for a kept folder instead of duplicating it', async () => { + const existing = { ...folderRow('T1', 'Shared'), workspaceId: 'ws-target' } + const { tx, insertedRows } = buildFolderTx([folderRow('G', 'Shared')], [existing]) + + const map = await resolveMapping({ tx, contentFolderIds: ['G'] }) + + expect(insertedRows).toHaveLength(0) + expect(map.get('G')).toBe('T1') + }) + + it('maps a pruned folder onto a matching existing target folder without creating it', async () => { + const existing = { ...folderRow('T1', 'Prior sync'), workspaceId: 'ws-target' } + const { tx, insertedRows } = buildFolderTx([folderRow('P', 'Prior sync')], [existing]) + + const map = await resolveMapping({ tx, contentFolderIds: [] }) + + expect(insertedRows).toHaveLength(0) + expect(map.get('P')).toBe('T1') + }) + + it('never root-aliases a pruned nested folder onto a same-named root target folder', async () => { + // Source X is nested under unmatched P; the target's root-level "X" is unrelated. + const existing = { ...folderRow('T-root-x', 'X'), workspaceId: 'ws-target' } + const { tx, insertedRows } = buildFolderTx( + [folderRow('P', 'Parent'), folderRow('X', 'X', 'P')], + [existing] + ) + + const map = await resolveMapping({ tx, contentFolderIds: [] }) + + expect(insertedRows).toHaveLength(0) + expect(map.size).toBe(0) + }) + + it('creates a kept child under a reused existing parent folder', async () => { + const existingParent = { ...folderRow('T-parent', 'Parent'), workspaceId: 'ws-target' } + const { tx, insertedRows } = buildFolderTx( + [folderRow('P', 'Parent'), folderRow('C', 'Child', 'P')], + [existingParent] + ) + + const map = await resolveMapping({ tx, contentFolderIds: ['C'] }) + + expect(map.get('P')).toBe('T-parent') + expect(insertedRows).toHaveLength(1) + expect(insertedRows[0].name).toBe('Child') + expect(insertedRows[0].parentId).toBe('T-parent') + }) +}) + +describe('copyWorkflowStateIntoTarget folder fallback', () => { + it('places a copied workflow at the target root when its source folder has no mapping', async () => { + mockSaveWorkflowToNormalizedTables.mockResolvedValue({ success: true }) + const insertedWorkflows: Array> = [] + const tx = { + insert: () => ({ + values: (row: Record) => { + insertedWorkflows.push(row) + return Promise.resolve() + }, + }), + } as unknown as DbOrTx + + const result = await copyWorkflowStateIntoTarget({ + tx, + targetWorkflowId: 'wf-child', + targetWorkspaceId: 'ws-target', + userId: 'target-user', + mode: 'create', + now: new Date('2026-07-01'), + sourceState: { blocks: {}, edges: [], loops: {}, parallels: {}, variables: {} }, + sourceMeta: { + name: 'Orphaned placement', + description: null, + folderId: 'folder-with-no-mapping', + sortOrder: 0, + }, + workflowIdMap: new Map(), + folderIdMap: new Map(), + nameRegistry: buildWorkflowNameRegistry([]), + }) + + expect(insertedWorkflows).toHaveLength(1) + expect(insertedWorkflows[0].folderId).toBeNull() + expect(result.name).toBe('Orphaned placement') + }) +}) diff --git a/apps/sim/lib/workspaces/fork/copy/copy-workflows.ts b/apps/sim/lib/workspaces/fork/copy/copy-workflows.ts index ecb386d0d09..f1d7be3ecbb 100644 --- a/apps/sim/lib/workspaces/fork/copy/copy-workflows.ts +++ b/apps/sim/lib/workspaces/fork/copy/copy-workflows.ts @@ -45,13 +45,25 @@ interface ResolveForkFolderMappingParams { targetWorkspaceId: string userId: string now: Date + /** + * Source folder ids that will directly hold copied content (workflows); null entries + * (root-placed content) are ignored. A source folder is copied into the target only when + * its subtree contains at least one of these, so a fork/sync never creates folders that + * would end up empty. Copied workspace FILES never influence this set: they live in the + * separate `workspace_file_folders` entity and are flattened to root by the copy. + */ + contentFolderIds: ReadonlyArray } /** - * Mirror the source workspace's folder tree into the target workspace, creating - * folders as needed and reusing target folders that already match by name within - * the same (mapped) parent. Returns a map from source folder id to target folder - * id so copied workflows can be placed in the corresponding folder. + * Mirror into the target workspace the part of the source folder tree that will actually + * receive copied content: the folders in `contentFolderIds` plus their ancestor chains (so + * nesting stays intact). Target folders that already match by name within the same (mapped) + * parent are reused instead of duplicated. Folders whose subtree holds no copied content are + * pruned - never created - though a pruned folder still maps onto an existing target folder + * when one matches, so previously-synced content refs keep resolving. Returns a map from + * source folder id to target folder id; a copied workflow whose folder is absent from the + * map is placed at the target's root (see {@link copyWorkflowStateIntoTarget}). */ export async function resolveForkFolderMapping({ tx, @@ -59,6 +71,7 @@ export async function resolveForkFolderMapping({ targetWorkspaceId, userId, now, + contentFolderIds, }: ResolveForkFolderMappingParams): Promise> { const map = new Map() @@ -71,6 +84,20 @@ export async function resolveForkFolderMapping({ if (sourceFolders.length === 0) return map + const byId = new Map(sourceFolders.map((folder) => [folder.id, folder])) + + // Kept = folders that directly hold copied content plus every ancestor; everything else + // would be empty in the target and is pruned. A dangling (archived) parent ends the walk, + // matching the re-root fallback below. + const kept = new Set() + for (const folderId of contentFolderIds) { + let current = folderId ? byId.get(folderId) : undefined + while (current && !kept.has(current.id)) { + kept.add(current.id) + current = current.parentId ? byId.get(current.parentId) : undefined + } + } + const targetFolders = await tx .select() .from(workflowFolder) @@ -83,7 +110,6 @@ export async function resolveForkFolderMapping({ targetByKey.set(`${folder.parentId ?? ''}::${folder.name}`, folder.id) } - const byId = new Map(sourceFolders.map((folder) => [folder.id, folder])) const ordered: typeof sourceFolders = [] const seen = new Set() const visit = (folder: (typeof sourceFolders)[number]) => { @@ -97,13 +123,20 @@ export async function resolveForkFolderMapping({ const newFolders: (typeof sourceFolders)[number][] = [] for (const folder of ordered) { + const isKept = kept.has(folder.id) const mappedParentId = folder.parentId ? (map.get(folder.parentId) ?? null) : null const key = `${mappedParentId ?? ''}::${folder.name}` const existing = targetByKey.get(key) if (existing) { - map.set(folder.id, existing) + // A pruned folder may still MAP onto an existing target folder, but only when its + // parent chain actually resolved: an unmapped pruned parent aliases the key to root + // level, which could match an unrelated same-named root folder. + if (isKept || !folder.parentId || map.has(folder.parentId)) { + map.set(folder.id, existing) + } continue } + if (!isKept) continue const newFolderId = generateId() map.set(folder.id, newFolderId) targetByKey.set(key, newFolderId) diff --git a/apps/sim/lib/workspaces/fork/copy/storage-quota.test.ts b/apps/sim/lib/workspaces/fork/copy/storage-quota.test.ts new file mode 100644 index 00000000000..04f0ef050f5 --- /dev/null +++ b/apps/sim/lib/workspaces/fork/copy/storage-quota.test.ts @@ -0,0 +1,140 @@ +/** + * @vitest-environment node + */ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { mockCheckStorageQuota } = vi.hoisted(() => ({ + mockCheckStorageQuota: vi.fn(), +})) + +vi.mock('@/lib/billing/storage', () => ({ + checkStorageQuota: mockCheckStorageQuota, +})) + +/** + * Minimal stand-in for the domain error so this unit test never loads the authz module's + * billing/feature-flag import chain. Shape-compatible with the real `ForkError`. + */ +vi.mock('@/lib/workspaces/fork/lineage/authz', () => ({ + ForkError: class ForkError extends Error { + statusCode: number + constructor(message: string, statusCode = 400) { + super(message) + this.name = 'ForkError' + this.statusCode = statusCode + } + }, +})) + +import type { DbOrTx } from '@/lib/db/types' +import { + assertForkStorageHeadroom, + sumForkCopyBytes, +} from '@/lib/workspaces/fork/copy/storage-quota' +import { ForkError } from '@/lib/workspaces/fork/lineage/authz' + +/** + * Fake executor resolving one aggregate row per query, in call order. Supports both sum + * shapes: `select().from().where()` (files) and `select().from().innerJoin().where()` (KB + * documents joined to their live KB row). + */ +function makeExecutor(totals: Array) { + let call = 0 + const next = () => Promise.resolve([{ total: totals[call++] ?? 0 }]) + const select = vi.fn(() => ({ + from: () => ({ + where: next, + innerJoin: () => ({ where: next }), + }), + })) + return { executor: { select } as unknown as DbOrTx, select } +} + +describe('sumForkCopyBytes', () => { + it('adds the workspace-file and KB-document byte sums', async () => { + const { executor, select } = makeExecutor([300, 700]) + + const bytes = await sumForkCopyBytes(executor, 'src-ws', { + fileIds: ['wf-1'], + knowledgeBaseIds: ['kb-1'], + }) + + expect(bytes).toBe(1000) + expect(select).toHaveBeenCalledTimes(2) + }) + + it('coerces driver string aggregates (bigint sums) to numbers', async () => { + const { executor } = makeExecutor(['1024']) + + const bytes = await sumForkCopyBytes(executor, 'src-ws', { fileKeys: ['workspace/src/k1'] }) + + expect(bytes).toBe(1024) + }) + + it('runs no query for an empty selection', async () => { + const { executor, select } = makeExecutor([]) + + const bytes = await sumForkCopyBytes(executor, 'src-ws', { + fileIds: [], + fileKeys: [], + knowledgeBaseIds: [], + }) + + expect(bytes).toBe(0) + expect(select).not.toHaveBeenCalled() + }) + + it('skips the file query when only KBs are selected (and vice versa)', async () => { + const { executor, select } = makeExecutor([555]) + + const bytes = await sumForkCopyBytes(executor, 'src-ws', { knowledgeBaseIds: ['kb-1'] }) + + expect(bytes).toBe(555) + expect(select).toHaveBeenCalledTimes(1) + }) +}) + +describe('assertForkStorageHeadroom', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it('never consults the quota helper for zero bytes', async () => { + await assertForkStorageHeadroom({ userId: 'user-1', bytes: 0 }) + expect(mockCheckStorageQuota).not.toHaveBeenCalled() + }) + + it('resolves when the scope has headroom', async () => { + mockCheckStorageQuota.mockResolvedValue({ allowed: true, currentUsage: 10, limit: 100 }) + + await expect( + assertForkStorageHeadroom({ userId: 'user-1', bytes: 50 }) + ).resolves.toBeUndefined() + expect(mockCheckStorageQuota).toHaveBeenCalledWith('user-1', 50) + }) + + it("throws a 413 ForkError carrying the upload path's quota message when over quota", async () => { + mockCheckStorageQuota.mockResolvedValue({ + allowed: false, + currentUsage: 99, + limit: 100, + error: 'Storage limit exceeded. Used: 10.50GB, Limit: 10GB', + }) + + const rejection = expect(assertForkStorageHeadroom({ userId: 'user-1', bytes: 50 })).rejects + await rejection.toBeInstanceOf(ForkError) + await rejection.toMatchObject({ + statusCode: 413, + message: + 'Not enough storage to copy the selected resources. Storage limit exceeded. Used: 10.50GB, Limit: 10GB', + }) + }) + + it('falls back to a generic storage message when the quota helper omits one', async () => { + mockCheckStorageQuota.mockResolvedValue({ allowed: false, currentUsage: 0, limit: 0 }) + + await expect(assertForkStorageHeadroom({ userId: 'user-1', bytes: 1 })).rejects.toThrow( + 'Not enough storage to copy the selected resources. Storage limit exceeded' + ) + }) +}) diff --git a/apps/sim/lib/workspaces/fork/copy/storage-quota.ts b/apps/sim/lib/workspaces/fork/copy/storage-quota.ts new file mode 100644 index 00000000000..91a250f1b5d --- /dev/null +++ b/apps/sim/lib/workspaces/fork/copy/storage-quota.ts @@ -0,0 +1,126 @@ +import { document, knowledgeBase, workspaceFiles } from '@sim/db/schema' +import { and, eq, inArray, isNotNull, isNull, or, sql } from 'drizzle-orm' +import { checkStorageQuota } from '@/lib/billing/storage' +import type { DbOrTx } from '@/lib/db/types' +import { ForkError } from '@/lib/workspaces/fork/lineage/authz' + +/** Resource ids whose blob bytes a fork/sync copy would duplicate into the target. */ +export interface ForkCopyBytesSelection { + /** Workspace files selected by `workspace_files.id` (the fork modal's picker shape). */ + fileIds?: string[] + /** Workspace files selected by storage key (the sync copy selection shape). */ + fileKeys?: string[] + /** Knowledge bases whose live documents' stored blobs would be re-keyed into the target. */ + knowledgeBaseIds?: string[] +} + +/** + * Byte total of the workspace-file blobs a copy selection would duplicate. Applies the + * same row filters as `planForkFileCopies` (source workspace, durable `workspace` + * context, non-deleted, id/key selectors OR'd), so the sum covers exactly the rows the + * copy would plan. + */ +async function sumWorkspaceFileBytes( + executor: DbOrTx, + sourceWorkspaceId: string, + fileIds: string[], + fileKeys: string[] +): Promise { + if (fileIds.length === 0 && fileKeys.length === 0) return 0 + const selectors = [ + fileIds.length > 0 ? inArray(workspaceFiles.id, fileIds) : undefined, + fileKeys.length > 0 ? inArray(workspaceFiles.key, fileKeys) : undefined, + ].filter((clause): clause is NonNullable => clause !== undefined) + const rows = await executor + .select({ total: sql`coalesce(sum(${workspaceFiles.size}), 0)` }) + .from(workspaceFiles) + .where( + and( + selectors.length === 1 ? selectors[0] : or(...selectors), + eq(workspaceFiles.workspaceId, sourceWorkspaceId), + eq(workspaceFiles.context, 'workspace'), + isNull(workspaceFiles.deletedAt) + ) + ) + // `sum()` comes back as a string (bigint) from the driver; coerce explicitly. + return Number(rows[0]?.total ?? 0) +} + +/** + * Byte total of the KB document blobs the selected knowledge bases would re-key into the + * target. Scoped to live KBs in the source workspace (mirroring the container copy) and + * to LIVE documents with an internal blob: external/`data:` documents have a null + * `storageKey` (no blob is duplicated), and embeddings are DB rows the upload path never + * counts, so neither contributes bytes here. + */ +async function sumKbDocumentBytes( + executor: DbOrTx, + sourceWorkspaceId: string, + knowledgeBaseIds: string[] +): Promise { + if (knowledgeBaseIds.length === 0) return 0 + const rows = await executor + .select({ total: sql`coalesce(sum(${document.fileSize}), 0)` }) + .from(document) + .innerJoin(knowledgeBase, eq(document.knowledgeBaseId, knowledgeBase.id)) + .where( + and( + inArray(knowledgeBase.id, knowledgeBaseIds), + eq(knowledgeBase.workspaceId, sourceWorkspaceId), + isNull(knowledgeBase.deletedAt), + isNull(document.deletedAt), + isNull(document.archivedAt), + isNotNull(document.storageKey) + ) + ) + return Number(rows[0]?.total ?? 0) +} + +/** + * Byte total a fork/sync copy selection would duplicate into the target: selected + * workspace-file blobs plus the selected knowledge bases' stored document blobs. Sizes + * come from the metadata rows (`workspace_files.size`, `document.file_size`) - no blob + * reads. Both sums scope to the source workspace with the same filters the copy itself + * applies, so an id that is not actually copyable can only over-count (block), never + * under-count. + */ +export async function sumForkCopyBytes( + executor: DbOrTx, + sourceWorkspaceId: string, + selection: ForkCopyBytesSelection +): Promise { + const fileBytes = await sumWorkspaceFileBytes( + executor, + sourceWorkspaceId, + selection.fileIds ?? [], + selection.fileKeys ?? [] + ) + const kbBytes = await sumKbDocumentBytes( + executor, + sourceWorkspaceId, + selection.knowledgeBaseIds ?? [] + ) + return fileBytes + kbBytes +} + +/** + * Assert the initiating user's storage scope has headroom for `bytes` of copied blobs, + * using the exact quota helper the upload path uses (`checkStorageQuota`, which resolves + * the org-pooled vs personal scope from the user's subscription and always allows when + * billing is disabled). Over quota throws a {@link ForkError} (413, matching the upload + * routes' storage-limit status) carrying the upload path's quota error message, so the + * fork/sync modals surface the same user-facing text an over-quota upload would. + */ +export async function assertForkStorageHeadroom(params: { + userId: string + bytes: number +}): Promise { + const { userId, bytes } = params + if (bytes <= 0) return + const quota = await checkStorageQuota(userId, bytes) + if (quota.allowed) return + throw new ForkError( + `Not enough storage to copy the selected resources. ${quota.error ?? 'Storage limit exceeded'}`, + 413 + ) +} diff --git a/apps/sim/lib/workspaces/fork/create-fork.test.ts b/apps/sim/lib/workspaces/fork/create-fork.test.ts new file mode 100644 index 00000000000..c2f1ee680f8 --- /dev/null +++ b/apps/sim/lib/workspaces/fork/create-fork.test.ts @@ -0,0 +1,187 @@ +/** + * @vitest-environment node + */ +import { dbChainMock, dbChainMockFns, resetDbChainMock } from '@sim/testing' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { + mockSumForkCopyBytes, + mockAssertForkStorageHeadroom, + mockLoadSourceDeployedStates, + mockPlanForkFileCopies, + mockCopyForkResourceContainers, + mockStartBackgroundWork, + mockFinishBackgroundWork, + mockScheduleForkContentCopy, +} = vi.hoisted(() => ({ + mockSumForkCopyBytes: vi.fn(), + mockAssertForkStorageHeadroom: vi.fn(), + mockLoadSourceDeployedStates: vi.fn(), + mockPlanForkFileCopies: vi.fn(), + mockCopyForkResourceContainers: vi.fn(), + mockStartBackgroundWork: vi.fn(), + mockFinishBackgroundWork: vi.fn(), + mockScheduleForkContentCopy: vi.fn(), +})) + +vi.mock('@sim/db', () => dbChainMock) +vi.mock('@/lib/workflows/defaults', () => ({ + buildDefaultWorkflowArtifacts: vi.fn(() => ({ workflowState: {} })), +})) +vi.mock('@/lib/workflows/persistence/utils', () => ({ + saveWorkflowToNormalizedTables: vi.fn(), +})) +vi.mock('@/lib/workspaces/fork/background-work/store', () => ({ + startBackgroundWork: mockStartBackgroundWork, + finishBackgroundWork: mockFinishBackgroundWork, +})) +vi.mock('@/lib/workspaces/fork/copy/content-copy-runner', () => ({ + hasForkContentToCopy: vi.fn(() => false), + scheduleForkContentCopy: mockScheduleForkContentCopy, + serializeContentRefMaps: vi.fn(() => ({})), +})) +vi.mock('@/lib/workspaces/fork/copy/copy-files', () => ({ + planForkFileCopies: mockPlanForkFileCopies, +})) +vi.mock('@/lib/workspaces/fork/copy/copy-resources', () => ({ + copyForkResourceContainers: mockCopyForkResourceContainers, +})) +vi.mock('@/lib/workspaces/fork/copy/storage-quota', () => ({ + sumForkCopyBytes: mockSumForkCopyBytes, + assertForkStorageHeadroom: mockAssertForkStorageHeadroom, +})) +vi.mock('@/lib/workspaces/fork/copy/copy-workflows', () => ({ + copyWorkflowStateIntoTarget: vi.fn(), + loadWorkflowNameRegistry: vi.fn(async () => new Map()), + resolveForkFolderMapping: vi.fn(async () => new Map()), +})) +vi.mock('@/lib/workspaces/fork/copy/deploy-bridge', () => ({ + loadSourceDeployedStates: mockLoadSourceDeployedStates, +})) +vi.mock('@/lib/workspaces/fork/lineage/lineage', () => ({ + setForkLockTimeout: vi.fn(), +})) +vi.mock('@/lib/workspaces/fork/mapping/block-map-store', () => ({ + reconcileForkBlockPairs: vi.fn(), + toForkBlockPairs: vi.fn(() => []), +})) +vi.mock('@/lib/workspaces/fork/mapping/mapping-store', () => ({ + seedEdgeMappings: vi.fn(), +})) +vi.mock('@/lib/workspaces/fork/remap/fork-bootstrap', () => ({ + createForkBootstrapTransform: vi.fn(() => (subBlocks: unknown) => subBlocks), +})) +vi.mock('@/lib/workspaces/fork/remap/reference-scan', () => ({ + collectReferencedDocumentIds: vi.fn(() => new Set()), +})) +vi.mock('@/lib/workspaces/policy', () => ({ + WORKSPACE_MODE: { + PERSONAL: 'personal', + ORGANIZATION: 'organization', + GRANDFATHERED_SHARED: 'grandfathered_shared', + }, +})) + +import { createFork } from '@/lib/workspaces/fork/create-fork' + +const SOURCE = { id: 'src-ws', name: 'Parent' } as never +const POLICY = { + organizationId: null, + workspaceMode: 'personal', + billedAccountUserId: null, +} as never + +function forkParams(selection?: { + files?: string[] + knowledgeBases?: string[] +}): Parameters[0] { + return { + source: SOURCE, + policy: POLICY, + userId: 'user-1', + name: 'My Fork', + selection: { + files: selection?.files ?? [], + tables: [], + knowledgeBases: selection?.knowledgeBases ?? [], + customTools: [], + skills: [], + workflowMcpServers: [], + }, + requestId: 'test', + } +} + +describe('createFork storage headroom gate', () => { + beforeEach(() => { + vi.clearAllMocks() + resetDbChainMock() + mockSumForkCopyBytes.mockResolvedValue(0) + mockAssertForkStorageHeadroom.mockResolvedValue(undefined) + mockLoadSourceDeployedStates.mockResolvedValue({ + deployedWorkflows: [], + sourceStates: new Map(), + }) + mockPlanForkFileCopies.mockResolvedValue({ + keyMap: new Map(), + idMap: new Map(), + blobTasks: [], + }) + mockCopyForkResourceContainers.mockResolvedValue({ + idMap: new Map(), + mappingEntries: [], + contentPlan: { + sourceWorkspaceId: 'src-ws', + childWorkspaceId: 'child-ws', + userId: 'user-1', + tables: [], + knowledgeBases: [], + skills: [], + documents: [], + }, + names: { + tables: [], + knowledgeBases: [], + customTools: [], + skills: [], + workflowMcpServers: [], + }, + }) + mockStartBackgroundWork.mockResolvedValue('status-1') + mockFinishBackgroundWork.mockResolvedValue(undefined) + }) + + it('fails an over-quota fork BEFORE any read or write, with the storage error', async () => { + mockSumForkCopyBytes.mockResolvedValue(999_999) + mockAssertForkStorageHeadroom.mockRejectedValue( + new Error( + 'Not enough storage to copy the selected resources. Storage limit exceeded. Used: 10.50GB, Limit: 10GB' + ) + ) + + await expect( + createFork(forkParams({ files: ['wf-1'], knowledgeBases: ['kb-1'] })) + ).rejects.toThrow('Not enough storage to copy the selected resources') + + expect(mockAssertForkStorageHeadroom).toHaveBeenCalledWith({ userId: 'user-1', bytes: 999_999 }) + // Nothing was read, created, or recorded: the fork failed before all of it. + expect(mockLoadSourceDeployedStates).not.toHaveBeenCalled() + expect(dbChainMockFns.transaction).not.toHaveBeenCalled() + expect(mockStartBackgroundWork).not.toHaveBeenCalled() + }) + + it('proceeds under quota, summing exactly the selected files + knowledge bases', async () => { + mockSumForkCopyBytes.mockResolvedValue(500) + + const result = await createFork(forkParams({ files: ['wf-1'], knowledgeBases: ['kb-1'] })) + + expect(result.workspace.name).toBe('My Fork') + expect(result.workflowsCopied).toBe(0) + expect(mockSumForkCopyBytes).toHaveBeenCalledWith(expect.anything(), 'src-ws', { + fileIds: ['wf-1'], + knowledgeBaseIds: ['kb-1'], + }) + expect(mockAssertForkStorageHeadroom).toHaveBeenCalledWith({ userId: 'user-1', bytes: 500 }) + expect(dbChainMockFns.transaction).toHaveBeenCalledTimes(1) + }) +}) diff --git a/apps/sim/lib/workspaces/fork/create-fork.ts b/apps/sim/lib/workspaces/fork/create-fork.ts index 0e4fce8e5de..37230f4ddd9 100644 --- a/apps/sim/lib/workspaces/fork/create-fork.ts +++ b/apps/sim/lib/workspaces/fork/create-fork.ts @@ -29,6 +29,10 @@ import { resolveForkFolderMapping, } from '@/lib/workspaces/fork/copy/copy-workflows' import { loadSourceDeployedStates } from '@/lib/workspaces/fork/copy/deploy-bridge' +import { + assertForkStorageHeadroom, + sumForkCopyBytes, +} from '@/lib/workspaces/fork/copy/storage-quota' import { buildForkWorkflowIdMap } from '@/lib/workspaces/fork/copy/workflow-id-map' import { setForkLockTimeout } from '@/lib/workspaces/fork/lineage/lineage' import { @@ -111,6 +115,16 @@ export async function createFork(params: CreateForkParams): Promise child folder id map: remaps folder references in the copied workflows below and // feeds the post-commit content-ref rewrite (`sim:folder/` mentions in skill/file bodies). + // Scoped to the folders that will actually receive a copied workflow (plus ancestors): a + // fork copies only DEPLOYED workflows, so folders holding none would be created empty in + // the child and are pruned instead. Copied files don't extend this set - they use the + // separate workspace-file-folder entity and land at the child's root. const folderIdMap = await resolveForkFolderMapping({ tx, sourceWorkspaceId: source.id, targetWorkspaceId: childWorkspaceId, userId, now, + contentFolderIds: deployedWorkflows + .filter((wf) => workflowIdMap.has(wf.id)) + .map((wf) => wf.folderId), }) const resourceResult = await copyForkResourceContainers({ diff --git a/apps/sim/lib/workspaces/fork/mapping/mapping-service.ts b/apps/sim/lib/workspaces/fork/mapping/mapping-service.ts index 7a9f4ad3115..a2f05cea3f4 100644 --- a/apps/sim/lib/workspaces/fork/mapping/mapping-service.ts +++ b/apps/sim/lib/workspaces/fork/mapping/mapping-service.ts @@ -205,7 +205,12 @@ export async function getForkMappingView( sourceLabel: p.sourceLabel, targetId, suggested, - required: p.reference.required, + // Every entry here is a reference a synced workflow actually carries, and a sync is + // blocked while ANY reference would clear - so every entry is required. Copyable kinds + // (table / KB / file / custom tool / skill) also satisfy the gate by being selected for + // copy; map-only kinds (credential / env-var / MCP server) and source-deleted resources + // (no copy candidate) must be mapped. + required: true, candidates, // The full (unfiltered) target list for this kind hit the cap, so the picker is // showing a partial list - the UI tells the user to refine. diff --git a/apps/sim/lib/workspaces/fork/mapping/resources.test.ts b/apps/sim/lib/workspaces/fork/mapping/resources.test.ts index b67ce351f22..a29094c217e 100644 --- a/apps/sim/lib/workspaces/fork/mapping/resources.test.ts +++ b/apps/sim/lib/workspaces/fork/mapping/resources.test.ts @@ -5,6 +5,7 @@ import { dbChainMock, dbChainMockFns, resetDbChainMock } from '@sim/testing' import { beforeEach, describe, expect, it } from 'vitest' import type { DbOrTx } from '@/lib/db/types' import { + listForkCopyableSourceResources, listForkResourceCandidates, loadForkCopyableResourceLabels, } from '@/lib/workspaces/fork/mapping/resources' @@ -49,6 +50,75 @@ describe('listForkResourceCandidates', () => { }) }) +describe('listForkCopyableSourceResources', () => { + beforeEach(() => { + resetDbChainMock() + }) + + it('lists every sync-copyable kind, files keyed by storage key with folder grouping', async () => { + // The grouped queries resolve in Promise.all array order, each ending in `.limit()`: + // files (with folder), tables, knowledge bases, custom tools, skills. + dbChainMockFns.limit + .mockResolvedValueOnce([ + { + id: 'file-row-1', + key: 'workspace/SRC/a.png', + label: 'a.png', + folderId: 'fld-1', + folderName: 'Images', + }, + { + id: 'file-row-2', + key: 'workspace/SRC/root.txt', + label: 'root.txt', + folderId: null, + folderName: null, + }, + ]) + .mockResolvedValueOnce([{ id: 'tbl-1', label: 'Table One' }]) + .mockResolvedValueOnce([{ id: 'kb-1', label: 'KB One' }]) + .mockResolvedValueOnce([{ id: 'ct-1', label: 'Tool One' }]) + .mockResolvedValueOnce([{ id: 'sk-1', label: 'Skill One' }]) + + const result = await listForkCopyableSourceResources(executor, 'ws-src') + + expect(result).toEqual([ + // Files are addressed by STORAGE KEY (matching `file-upload` references + the promote copy + // selection), never by `workspace_files.id`, and carry their folder grouping. + { + kind: 'file', + sourceId: 'workspace/SRC/a.png', + label: 'a.png', + parentId: 'fld-1', + parentLabel: 'Images', + }, + { + kind: 'file', + sourceId: 'workspace/SRC/root.txt', + label: 'root.txt', + parentId: null, + parentLabel: null, + }, + { kind: 'table', sourceId: 'tbl-1', label: 'Table One', parentId: null, parentLabel: null }, + { + kind: 'knowledge-base', + sourceId: 'kb-1', + label: 'KB One', + parentId: null, + parentLabel: null, + }, + { + kind: 'custom-tool', + sourceId: 'ct-1', + label: 'Tool One', + parentId: null, + parentLabel: null, + }, + { kind: 'skill', sourceId: 'sk-1', label: 'Skill One', parentId: null, parentLabel: null }, + ]) + }) +}) + describe('loadForkCopyableResourceLabels', () => { beforeEach(() => { resetDbChainMock() diff --git a/apps/sim/lib/workspaces/fork/mapping/resources.ts b/apps/sim/lib/workspaces/fork/mapping/resources.ts index 23b0bf1cd38..1a9702e2da9 100644 --- a/apps/sim/lib/workspaces/fork/mapping/resources.ts +++ b/apps/sim/lib/workspaces/fork/mapping/resources.ts @@ -17,7 +17,7 @@ import { and, count, eq, exists, inArray, isNull, sql } from 'drizzle-orm' import type { ForkCopyableKind } from '@/lib/api/contracts/workspace-fork' import type { DbOrTx } from '@/lib/db/types' import type { ForkResourceType } from '@/lib/workspaces/fork/mapping/mapping-store' -import type { ForkRemapKind } from '@/lib/workspaces/fork/remap/remap-references' +import type { ForkMcpServerMeta, ForkRemapKind } from '@/lib/workspaces/fork/remap/remap-references' export interface ForkResourceCandidate { id: string @@ -329,6 +329,32 @@ export async function filterExistingForkTargets( return result } +/** + * Identity metadata (`name`/`url`) for the given MCP server ids in a workspace, looked up by + * exact id (no candidate cap, same deleted filter as the candidates). Promote uses it for the + * MAPPED TARGET servers so remapped tool-input entries rewrite their embedded server metadata + * from the target row (see {@link ForkMcpServerMeta}) - one bounded `inArray` read per sync, + * never per-entry. An id absent from the map no longer exists; its entries are left as-is. + */ +export async function getMcpServerMetaByIds( + executor: DbOrTx, + workspaceId: string, + ids: string[] +): Promise> { + if (ids.length === 0) return new Map() + const rows = await executor + .select({ id: mcpServers.id, name: mcpServers.name, url: mcpServers.url }) + .from(mcpServers) + .where( + and( + eq(mcpServers.workspaceId, workspaceId), + isNull(mcpServers.deletedAt), + inArray(mcpServers.id, ids) + ) + ) + return new Map(rows.map((row) => [row.id, { name: row.name, url: row.url ?? null }])) +} + /** * Provider id for each given credential id in a workspace, looked up by exact id (no * candidate cap). Presence in the returned map means the credential exists in the @@ -451,6 +477,66 @@ export interface ForkCopyableLabel { parentLabel: string | null } +/** + * One copyable resource in the sync SOURCE workspace, keyed the way the promote copy addresses + * it: files by STORAGE KEY (matching `file-upload` references + `planForkFileCopies`), every + * other kind by row id. `parentId`/`parentLabel` carry a file's folder grouping (null for + * non-file kinds and root files). + */ +export interface ForkCopyableSourceResource { + kind: ForkCopyableKind + sourceId: string + label: string + parentId: string | null + parentLabel: string | null +} + +/** + * Every copyable-kind resource in the sync source workspace (same archived/deleted filters and + * per-kind {@link CANDIDATE_LIMIT} cap as the copy picker), as sync-copy candidate entries. The + * promote plan filters these down to the UNREFERENCED-and-unmapped set it offers for copy + * alongside the referenced candidates. Covers exactly the sync-copyable kinds + * (`forkCopyableKindSchema`): workflow-publishing MCP servers are fork-copy-only (their copies + * are not recorded in the fork resource map, so a sync copy could never be idempotent) and + * external MCP servers / credentials / env vars are never copied. + */ +export async function listForkCopyableSourceResources( + executor: DbOrTx, + sourceWorkspaceId: string +): Promise { + const [files, tables, kbs, tools, skills] = await Promise.all([ + fileCandidatesWithFolderQuery(executor, sourceWorkspaceId), + tableCandidatesQuery(executor, sourceWorkspaceId), + knowledgeBaseCandidatesQuery(executor, sourceWorkspaceId), + customToolCandidatesQuery(executor, sourceWorkspaceId), + skillCandidatesQuery(executor, sourceWorkspaceId), + ]) + const flat = ( + kind: ForkCopyableKind, + rows: Array<{ id: string; label: string }> + ): ForkCopyableSourceResource[] => + rows.map((row) => ({ + kind, + sourceId: row.id, + label: row.label, + parentId: null, + parentLabel: null, + })) + return [ + ...files.map((row) => ({ + kind: 'file' as const, + sourceId: row.key, + label: row.label, + parentId: row.folderId, + parentLabel: row.folderName, + })), + ...flat('table', tables), + ...flat('knowledge-base', kbs), + ...flat('custom-tool', tools), + ...flat('skill', skills), + ] +} + /** * Labels (by exact id) for the copyable resource kinds referenced-but-unmapped at promote time, * scoped to the source workspace and the same archived/deleted filters as the copy picker. A diff --git a/apps/sim/lib/workspaces/fork/promote/cleared-refs.test.ts b/apps/sim/lib/workspaces/fork/promote/cleared-refs.test.ts index 69f3adb8d01..278f421b111 100644 --- a/apps/sim/lib/workspaces/fork/promote/cleared-refs.test.ts +++ b/apps/sim/lib/workspaces/fork/promote/cleared-refs.test.ts @@ -1,7 +1,7 @@ /** * @vitest-environment node */ -import { describe, expect, it, vi } from 'vitest' +import { beforeEach, describe, expect, it, vi } from 'vitest' import type { SubBlockConfig } from '@/blocks/types' // The reference indexer resolves a tool's params via the tool registry; stub it so loading the @@ -19,7 +19,30 @@ vi.mock('@/tools/params', () => ({ formatParameterLabel: (label: string) => label, })) -import { collectForkClearedRefCandidates } from '@/lib/workspaces/fork/promote/cleared-refs' +// The liveness annotation + gate label loading go through the mapping resource helpers; mocked so +// each case controls which source ids read as still-alive and which labels resolve. +const { mockFilterExisting, mockLoadCopyableLabels } = vi.hoisted(() => ({ + mockFilterExisting: vi.fn(), + mockLoadCopyableLabels: vi.fn(), +})) +vi.mock('@/lib/workspaces/fork/mapping/resources', () => ({ + filterExistingForkTargets: mockFilterExisting, + loadForkCopyableResourceLabels: mockLoadCopyableLabels, + getWorkspaceEnvKeys: vi.fn(), + listForkCopyableSourceResources: vi.fn(), + listForkResourceCandidates: vi.fn(), + getCredentialProvidersByIds: vi.fn(), + classifyCredentialResourceType: vi.fn(), + CANDIDATE_LIMIT: 1000, +})) + +import type { DbOrTx } from '@/lib/db/types' +import { + annotateForkClearedRefSourceLiveness, + collectForkClearedRefCandidates, + collectForkSyncBlockers, +} from '@/lib/workspaces/fork/promote/cleared-refs' +import { buildPromoteWorkflowIdMap } from '@/lib/workspaces/fork/promote/promote-plan' import { buildForkBlockIdResolver, deriveForkBlockId, @@ -114,6 +137,8 @@ describe('collectForkClearedRefCandidates', () => { sourceId: 'kb-src', sourceLabel: 'Docs KB', cause: 'reference', + // Collected as false; source liveness is annotated afterwards (DB check). + sourceDeleted: false, }, ]) }) @@ -271,7 +296,7 @@ describe('collectForkClearedRefCandidates', () => { expect(result).toEqual([]) }) - it('collapses the workflowId pair to the active member: a dormant basic selector is not a false cleared-ref', () => { + it('collapses the workflowId pair to the active member: manual active emits nothing, basic selector still clears', () => { vi.mocked(getBlock).mockReturnValue( blockWith([ { @@ -290,7 +315,16 @@ describe('collectForkClearedRefCandidates', () => { }, ]) ) - // Advanced mode active; the dormant basic selector holds a stale, uncopied id. + const item = { + sourceWorkflowId: 'wf-src', + targetWorkflowId: 'wf-tgt', + mode: 'replace' as const, + sourceMeta: { name: 'Caller' }, + } + + // Advanced (manual) mode active; the dormant basic selector holds a stale, uncopied id. The + // manual member is user-owned (never cleared) and the dormant basic selector is collapsed away, + // so NO workflow cleared-ref rows even when nothing is carried into the target. const advancedState = { blocks: { 'block-1': { @@ -309,34 +343,274 @@ describe('collectForkClearedRefCandidates', () => { parallels: {}, variables: {}, } as unknown as WorkflowState + const manualActive = collectForkClearedRefCandidates( + params({ + items: [item], + sourceStates: new Map([['wf-src', advancedState]]), + sourceWorkflowNames: new Map([['wf-active', 'Active Workflow']]), + }) + ) + expect(manualActive.filter((ref) => ref.cause === 'workflow')).toEqual([]) + + // Active BASIC selector path unbroken: an uncopied selector value still produces a workflow row. + const basicState = { + blocks: { + 'block-1': { + id: 'block-1', + type: 'workflow', + name: 'Caller', + data: { canonicalModes: { workflowId: 'basic' } }, + subBlocks: { + workflowId: { type: 'workflow-selector', value: 'wf-basic' }, + manualWorkflowId: { type: 'short-input', value: 'wf-active' }, + }, + }, + }, + edges: [], + loops: {}, + parallels: {}, + variables: {}, + } as unknown as WorkflowState + const basicActive = collectForkClearedRefCandidates( + params({ + items: [item], + sourceStates: new Map([['wf-src', basicState]]), + sourceWorkflowNames: new Map([['wf-basic', 'Basic Workflow']]), + }) + ) + const workflowRows = basicActive.filter((ref) => ref.cause === 'workflow') + expect(workflowRows).toHaveLength(1) + expect(workflowRows[0].sourceId).toBe('wf-basic') + }) + + it('collapses the workflowIds pair to the active member: a stale dormant workflowSelector array emits nothing, active basic still clears', () => { + vi.mocked(getBlock).mockReturnValue( + blockWith([ + { + id: 'workflowSelector', + title: 'Workflows', + type: 'dropdown', + canonicalParamId: 'workflowIds', + mode: 'basic', + }, + { + id: 'manualWorkflowIds', + title: 'Workflow IDs', + type: 'short-input', + canonicalParamId: 'workflowIds', + mode: 'advanced', + }, + ]) + ) const item = { sourceWorkflowId: 'wf-src', targetWorkflowId: 'wf-tgt', mode: 'replace' as const, - sourceMeta: { name: 'Caller' }, + sourceMeta: { name: 'Logs' }, } + const stateWithModes = (canonicalModes: Record): WorkflowState => + ({ + blocks: { + 'block-1': { + id: 'block-1', + type: 'logs', + name: 'Logs', + data: { canonicalModes }, + subBlocks: { + // Switching to advanced does NOT clear the dormant basic selector, so a stale + // non-empty array persists here. + workflowSelector: { type: 'dropdown', value: ['wf-stale-1', 'wf-stale-2'] }, + manualWorkflowIds: { type: 'short-input', value: 'wf-manual' }, + }, + }, + }, + edges: [], + loops: {}, + parallels: {}, + variables: {}, + }) as unknown as WorkflowState - // Active advanced workflow carried into the target: the dormant basic must NOT produce a row. - const carried = collectForkClearedRefCandidates( + // Advanced mode active: the dormant selector's stale, unmapped ids must NOT surface as + // workflow cleared-refs (they would be unresolvable sync blockers - the modal can't map them). + const advancedActive = collectForkClearedRefCandidates( params({ items: [item], - sourceStates: new Map([['wf-src', advancedState]]), - workflowIdMap: new Map([['wf-active', 'wf-active-child']]), + sourceStates: new Map([['wf-src', stateWithModes({ workflowIds: 'advanced' })]]), + workflowIdMap: new Map(), }) ) - expect(carried.filter((ref) => ref.cause === 'workflow')).toEqual([]) + expect(advancedActive.filter((ref) => ref.cause === 'workflow')).toEqual([]) - // The ACTIVE member still produces a row when it is not carried (active path unbroken). - const cleared = collectForkClearedRefCandidates( + // Active BASIC selector path unbroken: the same unmapped ids still emit one row each. + const basicActive = collectForkClearedRefCandidates( params({ items: [item], - sourceStates: new Map([['wf-src', advancedState]]), - sourceWorkflowNames: new Map([['wf-active', 'Active Workflow']]), + sourceStates: new Map([['wf-src', stateWithModes({ workflowIds: 'basic' })]]), + workflowIdMap: new Map(), }) ) - const workflowRows = cleared.filter((ref) => ref.cause === 'workflow') - expect(workflowRows).toHaveLength(1) - expect(workflowRows[0].sourceId).toBe('wf-active') + const workflowRows = basicActive.filter((ref) => ref.cause === 'workflow') + expect(workflowRows.map((ref) => ref.sourceId)).toEqual(['wf-stale-1', 'wf-stale-2']) + }) + + // The sim workspace-event trigger's workflow filter: a multi-select `dropdown` with baseKey + // `workflowIds` (options are workspace workflow ids). Uncarried ids are dropped by the remap, + // so they must surface as workflow-cause cleared refs / sync blockers. + it('emits workflow refs for the workspace-event trigger workflowIds dropdown (uncarried ids)', () => { + vi.mocked(getBlock).mockReturnValue( + blockWith([{ id: 'workflowIds', title: 'Workflows', type: 'dropdown', multiSelect: true }]) + ) + const result = collectForkClearedRefCandidates( + params({ + items: [ + { + sourceWorkflowId: 'wf-src', + targetWorkflowId: 'wf-tgt', + mode: 'replace', + sourceMeta: { name: 'Alerts' }, + }, + ], + sourceStates: new Map([ + [ + 'wf-src', + stateWith('sim_workspace_event', 'Workspace Events', { + workflowIds: { type: 'dropdown', value: ['wf-watched', 'wf-carried'] }, + }), + ], + ]), + workflowIdMap: new Map([['wf-carried', 'wf-carried-tgt']]), + sourceWorkflowNames: new Map([['wf-watched', 'Watched Workflow']]), + }) + ) + expect(result).toEqual([ + { + targetWorkflowId: 'wf-tgt', + workflowName: 'Alerts', + blockId: targetBlockId, + blockLabel: 'Workspace Events', + fieldLabel: 'Workflows', + kind: 'workflow', + sourceId: 'wf-watched', + sourceLabel: 'Watched Workflow', + cause: 'workflow', + }, + ]) + }) + + it('emits nothing for the trigger workflowIds when every watched workflow is carried', () => { + vi.mocked(getBlock).mockReturnValue( + blockWith([{ id: 'workflowIds', title: 'Workflows', type: 'dropdown', multiSelect: true }]) + ) + const result = collectForkClearedRefCandidates( + params({ + items: [ + { + sourceWorkflowId: 'wf-src', + targetWorkflowId: 'wf-tgt', + mode: 'replace', + sourceMeta: { name: 'Alerts' }, + }, + ], + sourceStates: new Map([ + [ + 'wf-src', + stateWith('sim_workspace_event', 'Workspace Events', { + workflowIds: { type: 'dropdown', value: ['wf-a', 'wf-b'] }, + }), + ], + ]), + workflowIdMap: new Map([ + ['wf-a', 'wf-a-tgt'], + ['wf-b', 'wf-b-tgt'], + ]), + }) + ) + expect(result).toEqual([]) + }) + + // The TYPE gate: the legacy logs block's `workflowIds` is a free-form short-input (manual, + // user-owned, never remapped/cleared), so it must not emit workflow cleared-refs. + it('does not treat the legacy logs short-input workflowIds as a workflow reference', () => { + vi.mocked(getBlock).mockReturnValue( + blockWith([{ id: 'workflowIds', title: 'Workflow IDs', type: 'short-input' }]) + ) + const result = collectForkClearedRefCandidates( + params({ + items: [ + { + sourceWorkflowId: 'wf-src', + targetWorkflowId: 'wf-tgt', + mode: 'replace', + sourceMeta: { name: 'Logs' }, + }, + ], + sourceStates: new Map([ + [ + 'wf-src', + stateWith('logs', 'Logs', { + workflowIds: { type: 'short-input', value: 'wf-a,wf-b' }, + }), + ], + ]), + workflowIdMap: new Map(), + }) + ) + expect(result.filter((ref) => ref.cause === 'workflow')).toEqual([]) + }) + + it('does not emit manual manualWorkflowIds values as workflow cleared-refs', () => { + vi.mocked(getBlock).mockReturnValue( + blockWith([ + { + id: 'workflowSelector', + title: 'Workflows', + type: 'dropdown', + canonicalParamId: 'workflowIds', + mode: 'basic', + }, + { + id: 'manualWorkflowIds', + title: 'Workflow IDs', + type: 'short-input', + canonicalParamId: 'workflowIds', + mode: 'advanced', + }, + ]) + ) + // Active advanced manual list holds uncopied ids: user-owned, so never a workflow cleared-ref. + const state = { + blocks: { + 'block-1': { + id: 'block-1', + type: 'logs', + name: 'Logs', + data: { canonicalModes: { workflowIds: 'advanced' } }, + subBlocks: { + workflowSelector: { type: 'dropdown', value: [] }, + manualWorkflowIds: { type: 'short-input', value: 'wf-a,wf-b' }, + }, + }, + }, + edges: [], + loops: {}, + parallels: {}, + variables: {}, + } as unknown as WorkflowState + const result = collectForkClearedRefCandidates( + params({ + items: [ + { + sourceWorkflowId: 'wf-src', + targetWorkflowId: 'wf-tgt', + mode: 'replace', + sourceMeta: { name: 'Logs' }, + }, + ], + sourceStates: new Map([['wf-src', state]]), + workflowIdMap: new Map(), + }) + ) + expect(result.filter((ref) => ref.cause === 'workflow')).toEqual([]) }) it('emits a configured create-target dependent a remapped parent will clear (cause dependent)', () => { @@ -526,3 +800,542 @@ describe('collectForkClearedRefCandidates', () => { expect(result).toEqual([]) }) }) + +const replaceItem = { + sourceWorkflowId: 'wf-src', + targetWorkflowId: 'wf-tgt', + mode: 'replace' as const, + sourceMeta: { name: 'Caller' }, +} + +/** Fake executor whose select chains resolve queued row sets in call order. */ +function makeExecutor(rowSets: unknown[][] = []) { + let call = 0 + const select = vi.fn(() => ({ + from: vi.fn(() => ({ + where: vi.fn(() => Promise.resolve(rowSets[call++] ?? [])), + })), + })) + return { executor: { select } as unknown as DbOrTx, select } +} + +describe('annotateForkClearedRefSourceLiveness', () => { + beforeEach(() => { + mockFilterExisting.mockReset() + mockFilterExisting.mockResolvedValue({}) + }) + + const referenceRef = (kind: 'table' | 'knowledge-base', sourceId: string) => ({ + targetWorkflowId: 'wf-tgt', + workflowName: 'Caller', + blockId: 'b1', + blockLabel: 'Block', + fieldLabel: 'Field', + kind, + sourceId, + sourceLabel: sourceId, + cause: 'reference' as const, + sourceDeleted: false, + }) + + it('flags deleted sources and leaves live ones (checked against the SOURCE workspace)', async () => { + mockFilterExisting.mockResolvedValue({ table: new Set(['tbl-live']) }) + const { executor } = makeExecutor() + const result = await annotateForkClearedRefSourceLiveness(executor, 'src-ws', [ + referenceRef('table', 'tbl-live'), + referenceRef('table', 'tbl-gone'), + ]) + expect(mockFilterExisting).toHaveBeenCalledWith(executor, 'src-ws', { + table: new Set(['tbl-live', 'tbl-gone']), + }) + expect(result.map((ref) => (ref.cause === 'reference' ? ref.sourceDeleted : null))).toEqual([ + false, + true, + ]) + }) + + it('no-ops with zero queries when there are no reference-cause entries', async () => { + const { executor } = makeExecutor() + const workflowRef = { + targetWorkflowId: 'wf-tgt', + workflowName: 'Caller', + blockId: 'b1', + blockLabel: 'Block', + fieldLabel: 'Workflow', + kind: 'workflow' as const, + sourceId: 'wf-other', + sourceLabel: 'Other', + cause: 'workflow' as const, + } + const result = await annotateForkClearedRefSourceLiveness(executor, 'src-ws', [workflowRef]) + expect(result).toEqual([workflowRef]) + expect(mockFilterExisting).not.toHaveBeenCalled() + }) +}) + +describe('collectForkSyncBlockers', () => { + beforeEach(() => { + mockFilterExisting.mockReset() + mockLoadCopyableLabels.mockReset() + mockFilterExisting.mockResolvedValue({}) + mockLoadCopyableLabels.mockResolvedValue(new Map()) + }) + + const baseParams = (overrides: Partial[0]>) => ({ + executor: makeExecutor().executor, + sourceWorkspaceId: 'src-ws', + items: [replaceItem], + sourceStates: new Map(), + resolver: (() => null) as ForkReferenceResolver, + workflowIdMap: new Map(), + resolveBlockId, + ...overrides, + }) + + it('blocks an unmapped referenced copyable (unmapped-copyable) with its loaded label', async () => { + vi.mocked(getBlock).mockReturnValue( + blockWith([{ id: 'tbl', title: 'Table', type: 'table-selector' }]) + ) + mockFilterExisting.mockResolvedValue({ table: new Set(['tbl-src']) }) + mockLoadCopyableLabels.mockResolvedValue( + new Map([['table:tbl-src', { label: 'Orders', parentId: null, parentLabel: null }]]) + ) + const blockers = await collectForkSyncBlockers( + baseParams({ + sourceStates: new Map([ + [ + 'wf-src', + stateWith('table', 'Table Block', { + tbl: { type: 'table-selector', value: 'tbl-src' }, + }), + ], + ]), + }) + ) + expect(blockers).toEqual([ + { + workflowName: 'Caller', + blockLabel: 'Table Block', + fieldLabel: 'Table', + kind: 'table', + sourceId: 'tbl-src', + sourceLabel: 'Orders', + reason: 'unmapped-copyable', + }, + ]) + }) + + it('passes with ZERO queries when the resolver maps/copy-resolves every reference', async () => { + vi.mocked(getBlock).mockReturnValue( + blockWith([{ id: 'tbl', title: 'Table', type: 'table-selector' }]) + ) + const { executor, select } = makeExecutor() + const blockers = await collectForkSyncBlockers( + baseParams({ + executor, + sourceStates: new Map([ + [ + 'wf-src', + stateWith('table', 'Table Block', { + tbl: { type: 'table-selector', value: 'tbl-src' }, + }), + ], + ]), + // The promote gate overlays the copy selection onto the plan resolver; a mapped OR + // copy-selected reference resolves non-null and never reaches the blocker list. + resolver: (kind, id) => (kind === 'table' && id === 'tbl-src' ? 'tbl-copy' : null), + }) + ) + expect(blockers).toEqual([]) + expect(mockFilterExisting).not.toHaveBeenCalled() + expect(select).not.toHaveBeenCalled() + }) + + it('blocks an unmapped external MCP server (unmapped-mcp-server), named via the source read', async () => { + vi.mocked(getBlock).mockReturnValue( + blockWith([{ id: 'server', title: 'Server', type: 'mcp-server-selector' }]) + ) + mockFilterExisting.mockResolvedValue({ 'mcp-server': new Set(['srv-1']) }) + const { executor } = makeExecutor([[{ id: 'srv-1', name: 'Internal Tools' }]]) + const blockers = await collectForkSyncBlockers( + baseParams({ + executor, + sourceStates: new Map([ + [ + 'wf-src', + stateWith('mcp', 'MCP Block', { + server: { type: 'mcp-server-selector', value: 'srv-1' }, + }), + ], + ]), + }) + ) + expect(blockers).toEqual([ + expect.objectContaining({ + kind: 'mcp-server', + sourceId: 'srv-1', + sourceLabel: 'Internal Tools', + reason: 'unmapped-mcp-server', + }), + ]) + }) + + 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' }]) + ) + // 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( + baseParams({ + sourceStates: new Map([ + [ + 'wf-src', + stateWith('knowledge', 'KB Block', { + kb: { type: 'knowledge-base-selector', value: 'kb-gone' }, + }), + ], + ]), + }) + ) + expect(blockers).toEqual([ + expect.objectContaining({ + kind: 'knowledge-base', + sourceId: 'kb-gone', + sourceLabel: 'kb-gone', + reason: 'source-deleted', + }), + ]) + // 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( + baseParams({ + sourceStates: new Map([ + [ + 'wf-src', + stateWith('knowledge', 'KB Block', { + kb: { type: 'knowledge-base-selector', value: 'kb-gone' }, + }), + ], + ]), + resolver: (kind, id) => (kind === 'knowledge-base' && id === 'kb-gone' ? 'kb-tgt' : null), + }) + ) + expect(resolved).toEqual([]) + }) + + it('blocks a workflow reference that would clear (workflow-missing), named via the source read', async () => { + vi.mocked(getBlock).mockReturnValue( + blockWith([{ id: 'target', title: 'Workflow', type: 'workflow-selector' }]) + ) + const { executor } = makeExecutor([[{ id: 'wf-child', name: 'Child Flow' }]]) + // The child was deleted in the source: not an item, not in the identity map -> the map + // built by buildPromoteWorkflowIdMap misses and the reference would clear. + const workflowIdMap = buildPromoteWorkflowIdMap({ + identityMap: new Map([['wf-child', 'wf-child-tgt']]), + existingSourceIds: new Set(), + targetActiveIds: new Set(['wf-child-tgt']), + items: [{ sourceWorkflowId: 'wf-src', targetWorkflowId: 'wf-tgt' }], + }) + const blockers = await collectForkSyncBlockers( + baseParams({ + executor, + sourceStates: new Map([ + [ + 'wf-src', + stateWith('workflow_caller', 'Run Subflow', { + target: { type: 'workflow-selector', value: 'wf-child' }, + }), + ], + ]), + workflowIdMap, + }) + ) + expect(blockers).toEqual([ + expect.objectContaining({ + kind: 'workflow', + sourceId: 'wf-child', + sourceLabel: 'Child Flow', + reason: 'workflow-missing', + }), + ]) + }) + + it('does NOT block a previously-synced, source-undeployed child (its mapping still resolves)', async () => { + vi.mocked(getBlock).mockReturnValue( + blockWith([{ id: 'target', title: 'Workflow', type: 'workflow-selector' }]) + ) + const { executor, select } = makeExecutor() + // The child exists in the source (merely undeployed, so not an item this push) and its + // mapped target is still active: the identity seed repoints the reference, nothing clears. + const workflowIdMap = buildPromoteWorkflowIdMap({ + identityMap: new Map([['wf-child', 'wf-child-tgt']]), + existingSourceIds: new Set(['wf-child']), + targetActiveIds: new Set(['wf-child-tgt']), + items: [{ sourceWorkflowId: 'wf-src', targetWorkflowId: 'wf-tgt' }], + }) + const blockers = await collectForkSyncBlockers( + baseParams({ + executor, + sourceStates: new Map([ + [ + 'wf-src', + stateWith('workflow_caller', 'Run Subflow', { + target: { type: 'workflow-selector', value: 'wf-child' }, + }), + ], + ]), + workflowIdMap, + }) + ) + expect(blockers).toEqual([]) + expect(select).not.toHaveBeenCalled() + }) + + it('returns identical blockers via the reused-plan path and a fresh scan, incl. an irrelevant copy selection', async () => { + vi.mocked(getBlock).mockReturnValue( + blockWith([{ id: 'tbl', title: 'Table', type: 'table-selector' }]) + ) + mockFilterExisting.mockResolvedValue({ table: new Set(['tbl-src']) }) + mockLoadCopyableLabels.mockResolvedValue( + new Map([['table:tbl-src', { label: 'Orders', parentId: null, parentLabel: null }]]) + ) + const sourceStates = new Map([ + [ + 'wf-src', + stateWith('table', 'Table Block', { + tbl: { type: 'table-selector', value: 'tbl-src' }, + }), + ], + ]) + + const freshScan = await collectForkSyncBlockers(baseParams({ sourceStates })) + const reusedPlan = await collectForkSyncBlockers( + baseParams({ + sourceStates, + planUnmapped: [{ kind: 'table', sourceId: 'tbl-src' }], + }) + ) + // An irrelevant copy selection: the overlay resolver resolves a candidate no synced block + // references. The plan lists it as unmapped, the overlay resolves it, and the blockers are + // unchanged either way. + const overlayResolver: ForkReferenceResolver = (kind, id) => + kind === 'custom-tool' && id === 'ct-unreferenced' ? 'ct-copy' : null + const withIrrelevantCopy = await collectForkSyncBlockers( + baseParams({ + sourceStates, + resolver: overlayResolver, + planUnmapped: [ + { kind: 'table', sourceId: 'tbl-src' }, + { kind: 'custom-tool', sourceId: 'ct-unreferenced' }, + ], + }) + ) + + expect(freshScan).toEqual([ + expect.objectContaining({ kind: 'table', sourceId: 'tbl-src', reason: 'unmapped-copyable' }), + ]) + expect(reusedPlan).toEqual(freshScan) + expect(withIrrelevantCopy).toEqual(freshScan) + }) + + it('skips the per-block reference re-scan when the plan reports nothing unmapped', async () => { + // Deliberately inconsistent inputs: the state carries an unmapped table ref a fresh scan + // WOULD flag, but the supplied plan data says nothing is unmapped. The empty result proves + // the reused-plan shortcut skipped the re-scan entirely (in production the plan is computed + // over the same states inside the same tx, so the inputs can never actually diverge). + vi.mocked(getBlock).mockReturnValue( + blockWith([{ id: 'tbl', title: 'Table', type: 'table-selector' }]) + ) + const { executor, select } = makeExecutor() + const blockers = await collectForkSyncBlockers( + baseParams({ + executor, + sourceStates: new Map([ + [ + 'wf-src', + stateWith('table', 'Table Block', { + tbl: { type: 'table-selector', value: 'tbl-src' }, + }), + ], + ]), + planUnmapped: [], + }) + ) + expect(blockers).toEqual([]) + expect(mockFilterExisting).not.toHaveBeenCalled() + expect(select).not.toHaveBeenCalled() + }) + + it('short-circuits with zero scans/queries when the copy overlay resolves every plan-unmapped ref', async () => { + vi.mocked(getBlock).mockReturnValue( + blockWith([{ id: 'tbl', title: 'Table', type: 'table-selector' }]) + ) + const { executor, select } = makeExecutor() + const blockers = await collectForkSyncBlockers( + baseParams({ + executor, + sourceStates: new Map([ + [ + 'wf-src', + stateWith('table', 'Table Block', { + tbl: { type: 'table-selector', value: 'tbl-src' }, + }), + ], + ]), + // The plan saw the ref unmapped; the gate resolver (plan resolver + copy-selection + // overlay) resolves it, so no blocking candidate can exist. + resolver: (kind, id) => (kind === 'table' && id === 'tbl-src' ? 'tbl-copy' : null), + planUnmapped: [{ kind: 'table', sourceId: 'tbl-src' }], + }) + ) + expect(blockers).toEqual([]) + expect(mockFilterExisting).not.toHaveBeenCalled() + expect(select).not.toHaveBeenCalled() + }) + + it('still blocks on a would-clear workflow reference through the reused-plan path', async () => { + // Workflow refs are not in the plan's reference scan, so the shortcut walks them separately: + // an uncarried ref must still trigger the full collection and emit workflow-missing. + vi.mocked(getBlock).mockReturnValue( + blockWith([{ id: 'target', title: 'Workflow', type: 'workflow-selector' }]) + ) + const { executor } = makeExecutor([[{ id: 'wf-child', name: 'Child Flow' }]]) + const blockers = await collectForkSyncBlockers( + baseParams({ + executor, + sourceStates: new Map([ + [ + 'wf-src', + stateWith('workflow_caller', 'Run Subflow', { + target: { type: 'workflow-selector', value: 'wf-child' }, + }), + ], + ]), + planUnmapped: [], + }) + ) + expect(blockers).toEqual([ + expect.objectContaining({ + kind: 'workflow', + sourceId: 'wf-child', + reason: 'workflow-missing', + }), + ]) + }) + + it('blocks on an uncarried workspace-event trigger workflowIds entry through the reused-plan path', async () => { + // Trigger workflow filters are not in the plan's reference scan, so the shortcut's light + // workflow-ref walk must detect them and trigger the full collection. + vi.mocked(getBlock).mockReturnValue( + blockWith([{ id: 'workflowIds', title: 'Workflows', type: 'dropdown', multiSelect: true }]) + ) + const { executor } = makeExecutor([[{ id: 'wf-watched', name: 'Watched Workflow' }]]) + const blockers = await collectForkSyncBlockers( + baseParams({ + executor, + sourceStates: new Map([ + [ + 'wf-src', + stateWith('sim_workspace_event', 'Workspace Events', { + workflowIds: { type: 'dropdown', value: ['wf-watched'] }, + }), + ], + ]), + planUnmapped: [], + }) + ) + expect(blockers).toEqual([ + expect.objectContaining({ + kind: 'workflow', + sourceId: 'wf-watched', + sourceLabel: 'Watched Workflow', + reason: 'workflow-missing', + }), + ]) + }) + + it('never blocks on a dormant workflowSelector array (advanced manual mode active)', async () => { + vi.mocked(getBlock).mockReturnValue( + blockWith([ + { + id: 'workflowSelector', + title: 'Workflows', + type: 'dropdown', + canonicalParamId: 'workflowIds', + mode: 'basic', + }, + { + id: 'manualWorkflowIds', + title: 'Workflow IDs', + type: 'short-input', + canonicalParamId: 'workflowIds', + mode: 'advanced', + }, + ]) + ) + const { executor, select } = makeExecutor() + const state = { + blocks: { + 'block-1': { + id: 'block-1', + type: 'logs', + name: 'Logs', + data: { canonicalModes: { workflowIds: 'advanced' } }, + subBlocks: { + workflowSelector: { type: 'dropdown', value: ['wf-stale'] }, + manualWorkflowIds: { type: 'short-input', value: 'wf-manual' }, + }, + }, + }, + edges: [], + loops: {}, + parallels: {}, + variables: {}, + } as unknown as WorkflowState + const blockers = await collectForkSyncBlockers( + baseParams({ + executor, + sourceStates: new Map([['wf-src', state]]), + planUnmapped: [], + }) + ) + expect(blockers).toEqual([]) + expect(select).not.toHaveBeenCalled() + }) + + it('never blocks on dependent-cause entries (create-target dependents stay informational)', async () => { + vi.mocked(getBlock).mockReturnValue( + blockWith([ + { id: 'credential', title: 'Credential', type: 'oauth-input' }, + { + id: 'folder', + title: 'Label', + type: 'folder-selector', + dependsOn: ['credential'], + selectorKey: 'gmail.labels', + }, + ]) + ) + const { executor, select } = makeExecutor() + const blockers = await collectForkSyncBlockers( + baseParams({ + executor, + items: [{ ...replaceItem, mode: 'create' as const }], + sourceStates: new Map([ + [ + 'wf-src', + stateWith('gmail', 'Gmail', { + credential: { value: 'cred-src' }, + folder: { value: 'INBOX' }, + }), + ], + ]), + }) + ) + expect(blockers).toEqual([]) + expect(select).not.toHaveBeenCalled() + expect(mockFilterExisting).not.toHaveBeenCalled() + }) +}) diff --git a/apps/sim/lib/workspaces/fork/promote/cleared-refs.ts b/apps/sim/lib/workspaces/fork/promote/cleared-refs.ts index e6922731275..cb31fef852b 100644 --- a/apps/sim/lib/workspaces/fork/promote/cleared-refs.ts +++ b/apps/sim/lib/workspaces/fork/promote/cleared-refs.ts @@ -1,4 +1,11 @@ -import type { ForkClearedRef } from '@/lib/api/contracts/workspace-fork' +import { mcpServers, workflow } from '@sim/db/schema' +import { and, eq, inArray } from 'drizzle-orm' +import type { + ForkClearedRef, + ForkCopyableKind, + ForkSyncBlocker, +} from '@/lib/api/contracts/workspace-fork' +import type { DbOrTx } from '@/lib/db/types' import { coerceObjectArray, isRecord, @@ -12,8 +19,18 @@ import { resolveCanonicalMode, } from '@/lib/workflows/subblocks/visibility' import { collectForkDependentReconfigs } from '@/lib/workspaces/fork/mapping/dependent-reconfigs' +import { + filterExistingForkTargets, + loadForkCopyableResourceLabels, +} from '@/lib/workspaces/fork/mapping/resources' +import { isForkCopyableKind } from '@/lib/workspaces/fork/promote/promote-plan' +import { + selectForkSyncBlockingRefs, + toForkSyncBlockers, +} from '@/lib/workspaces/fork/promote/sync-blockers' import type { ForkBlockIdResolver } from '@/lib/workspaces/fork/remap/block-identity' import { + type ForkReference, type ForkReferenceResolver, type ForkRemapKind, REQUIRED_KINDS, @@ -24,10 +41,12 @@ import type { WorkflowState } from '@/stores/workflows/workflow/types' /** * Remappable kinds excluded from the `reference` cleared-ref list. REQUIRED kinds (credential, - * env-var) are BLOCKERS - they gate Sync and are resolved by mapping, never silently cleared - so - * they must not read as "will be cleared" (a credential is also preserved by name once mapped, an - * env-var always). `knowledge-document` follows its parent KB - a document under an unmapped KB is - * implied by the KB's own cleared-ref entry, and under a mapped/copied KB it is auto-copied. + * env-var) gate Sync through the kind-level required gate with their own messaging, so they must + * not double-report here (a credential is also preserved by name once mapped, an env-var always). + * `knowledge-document` follows its parent KB - a document under an unmapped KB is implied by the + * KB's own cleared-ref entry, and under a mapped/copied KB it is auto-copied. Every other kind's + * entry IS a sync blocker (cause `reference`/`workflow`): a sync proceeds only when zero + * references would clear. */ const CLEARED_REF_EXCLUDED_KINDS = new Set([...REQUIRED_KINDS, 'knowledge-document']) @@ -59,10 +78,14 @@ function baseSubBlockId(key: string): string { } /** - * Cross-workflow references (`workflow-selector`, advanced `manualWorkflowId(s)`, multi-select - * `workflowSelector`, nested `workflow_input` tools) in a block's subBlocks. Mirrors the detection - * in {@link remapWorkflowReferencesInSubBlocks} so the cleared-ref list flags exactly the refs that - * remap would clear. Returns one entry per referenced workflow id with its owning subblock key. + * Cross-workflow references (`workflow-selector`, multi-select `workflowSelector`, the + * workspace-event trigger's multi-select `workflowIds` dropdown, nested `workflow_input` tools) + * in a block's subBlocks. Mirrors the detection in + * {@link remapWorkflowReferencesInSubBlocks} so the cleared-ref list flags exactly the refs that + * remap would clear - the free-form manual fields (`manualWorkflowId`, `manualWorkflowIds`) are + * user-owned and never remapped/cleared, so they are intentionally excluded (the `workflowIds` + * branch is gated on TYPE `dropdown` because the legacy logs block's `workflowIds` is a manual + * `short-input`). Returns one entry per referenced workflow id with its owning subblock key. */ function collectForkWorkflowReferences( subBlocks: SubBlockRecord, @@ -70,29 +93,42 @@ function collectForkWorkflowReferences( canonicalModes: CanonicalModeOverrides | undefined ): Array<{ workflowId: string; subBlockKey: string }> { const out: Array<{ workflowId: string; subBlockKey: string }> = [] - // Collapse the `workflowId` canonical pair (basic `workflow-selector` + advanced `manualWorkflowId`) - // to its ACTIVE member: only the active mode is serialized, so a dormant stale member is not a ref - // that would be cleared (mirrors remap-internal-ids.ts). Undefined mode -> emit both (legacy/no-pair). - const workflowGroup = config - ? buildCanonicalIndex(config.subBlocks).groupsById.workflowId - : undefined - const workflowMode = - workflowGroup && isCanonicalPair(workflowGroup) - ? resolveCanonicalMode(workflowGroup, buildSubBlockValues(subBlocks), canonicalModes) - : undefined + // Collapse each canonical pair to its ACTIVE member: only the selector members are + // remapped/cleared (the advanced `manualWorkflowId`/`manualWorkflowIds` are user-owned and + // preserved verbatim), so a DORMANT member's stale value is not a ref that would be cleared - + // it must not become an unresolvable sync blocker. Mirrors `isDormantCanonicalMember` in + // remap-references.ts: the lookup is per subblock key, so the scalar `workflowId` pair, the + // deployments block's scalar `workflowSelector` pair, and the logs block's multi-select + // `workflowSelector` (`workflowIds` group) all resolve through their OWN group. A missing + // config or a non-pair member is never skipped (legacy/no-pair states keep emitting). + const canonicalIndex = config ? buildCanonicalIndex(config.subBlocks) : undefined + const values = canonicalIndex ? buildSubBlockValues(subBlocks) : {} + const isDormantCanonicalMember = (key: string): boolean => { + if (!canonicalIndex) return false + const baseKey = baseSubBlockId(key) + const canonicalId = canonicalIndex.canonicalIdBySubBlockId[baseKey] + const group = canonicalId ? canonicalIndex.groupsById[canonicalId] : undefined + if (!group || !isCanonicalPair(group)) return false + const activeMode = resolveCanonicalMode(group, values, canonicalModes) + return (activeMode === 'advanced') !== group.advancedIds.includes(baseKey) + } for (const [key, subBlock] of Object.entries(subBlocks)) { if (!subBlock || typeof subBlock !== 'object') continue const baseKey = baseSubBlockId(key) if ( - (subBlock.type === 'workflow-selector' || baseKey === 'manualWorkflowId') && + subBlock.type === 'workflow-selector' && typeof subBlock.value === 'string' && subBlock.value ) { - // Skip the dormant member of the pair (the active mode owns the reference). - const isAdvancedMember = baseKey === 'manualWorkflowId' - if (workflowMode && (workflowMode === 'advanced') !== isAdvancedMember) continue + // Only the SELECTOR is remapped/cleared; the manual member is user-owned and preserved + // verbatim, so skip the dormant selector when advanced/manual mode is active. + if (isDormantCanonicalMember(key)) continue out.push({ workflowId: subBlock.value, subBlockKey: key }) - } else if (baseKey === 'manualWorkflowIds' || baseKey === 'workflowSelector') { + } else if ( + baseKey === 'workflowSelector' || + (subBlock.type === 'dropdown' && baseKey === 'workflowIds') + ) { + if (isDormantCanonicalMember(key)) continue const ids = Array.isArray(subBlock.value) ? subBlock.value : typeof subBlock.value === 'string' @@ -158,10 +194,16 @@ export function collectForkClearedRefCandidates( config?.subBlocks.find((cfg) => cfg.id === baseSubBlockId(subBlockKey))?.title ?? subBlockKey - // Cause `reference`: unmapped remappable resource refs (per block/field). + // Cause `reference`: unmapped remappable resource refs (per block/field). `blockType` + + // `canonicalModes` gate detection to the ACTIVE canonical member, matching the plan's + // reference scan - a dormant member's stale value is not a real reference, so it must not + // become a blocker with no mapping entry to resolve it. `sourceDeleted` starts false; the + // caller annotates it via {@link annotateForkClearedRefSourceLiveness} (DB check). const scan = remapForkSubBlocks(subBlocks, resolver, 'promote', { blockId: targetBlockId, blockName: blockLabel, + blockType: block.type, + canonicalModes: block.data?.canonicalModes, }) for (const ref of scan.unmapped) { if (CLEARED_REF_EXCLUDED_KINDS.has(ref.kind)) continue @@ -175,6 +217,7 @@ export function collectForkClearedRefCandidates( sourceId: ref.sourceId, sourceLabel: labelFor(ref.kind, ref.sourceId), cause: 'reference', + sourceDeleted: false, }) } @@ -231,3 +274,169 @@ export function collectForkClearedRefCandidates( return out } + +/** + * Fill each `reference`-cause entry's `sourceDeleted` flag by checking whether its resource still + * exists (not deleted/archived) in the SOURCE workspace. Reuses {@link filterExistingForkTargets} + * - a per-kind, exact-id (cap-free) liveness check with the canonical archived/deleted filters - + * pointed at the source workspace instead of a target. One batched round per kind present; a + * no-op (zero queries) when no reference-cause entries exist. Files check by storage key, matching + * how `file` references are recorded. + */ +export async function annotateForkClearedRefSourceLiveness( + executor: DbOrTx, + sourceWorkspaceId: string, + clearedRefs: ForkClearedRef[] +): Promise { + const idsByKind: Partial>> = {} + for (const ref of clearedRefs) { + if (ref.cause !== 'reference') continue + ;(idsByKind[ref.kind] ??= new Set()).add(ref.sourceId) + } + if (Object.keys(idsByKind).length === 0) return clearedRefs + const liveByKind = await filterExistingForkTargets(executor, sourceWorkspaceId, idsByKind) + return clearedRefs.map((ref) => + ref.cause === 'reference' + ? { ...ref, sourceDeleted: !(liveByKind[ref.kind]?.has(ref.sourceId) ?? false) } + : ref + ) +} + +/** Upper bound on the blockers a gate failure reports, so the error body stays sane. */ +const FORK_SYNC_BLOCKER_LIMIT = 100 + +/** + * Cheap existence check for blocking gate candidates, reusing the plan's already-computed scan + * output instead of re-running the full per-block reference scan: + * - `reference` cause: the collector detects references with the same per-block scan + * ({@link remapForkSubBlocks}) over the same source states the plan already ran, so a + * candidate exists iff some plan-unmapped reference of a non-excluded kind still resolves to + * null through the gate resolver. The gate resolver only ADDS resolutions on top of the plan + * resolver (promote's copy-selection overlay), so filtering the plan's unmapped set through it + * yields exactly the gate's unmapped set. The plan's cascade-only additions (env-var / + * credential) are excluded kinds and never contribute. + * - `workflow` cause: cross-workflow refs are not part of the plan's scan, so walk the blocks + * with the (much lighter) workflow-reference detection only, against the same workflowIdMap + * predicate the collector applies. + * `dependent`-cause candidates never block (see {@link forkSyncBlockerReasonFor}), so they are + * not checked. + */ +function hasForkSyncBlockerCandidates( + planUnmapped: ReadonlyArray>, + params: Pick< + CollectForkClearedRefsParams, + 'items' | 'sourceStates' | 'resolver' | 'workflowIdMap' + > +): boolean { + const { items, sourceStates, resolver, workflowIdMap } = params + const hasReferenceCandidate = planUnmapped.some( + (reference) => + !CLEARED_REF_EXCLUDED_KINDS.has(reference.kind) && + resolver(reference.kind, reference.sourceId) == null + ) + if (hasReferenceCandidate) return true + for (const item of items) { + const state = sourceStates.get(item.sourceWorkflowId) + if (!state) continue + for (const block of Object.values(state.blocks)) { + // double-cast-allowed: a WorkflowState block's SubBlockState entries are structurally + // SubBlockRecord entries but lack the open index signature SubBlockRecord declares + const subBlocks = (block.subBlocks ?? {}) as unknown as SubBlockRecord + const workflowRefs = collectForkWorkflowReferences( + subBlocks, + getBlock(block.type), + block.data?.canonicalModes + ) + if (workflowRefs.some((ref) => !workflowIdMap.has(ref.workflowId))) return true + } + } + return false +} + +/** + * The authoritative would-clear gate input for a promote: collect the cleared-ref candidates for + * the sync (against the caller's resolver, which must already account for the copy selection), + * keep the blocking causes (`reference` / `workflow` - dependents stay with the reconfigure + * flow), annotate source liveness, and return them as wire {@link ForkSyncBlocker}s with + * best-effort labels. The happy path (nothing would clear) costs ZERO queries - the collection is + * pure over the pre-read source states - and, when `planUnmapped` is supplied, ZERO re-scans of + * the blocks the plan already scanned; liveness + label reads (and the full candidate collection, + * for identical per-block/field blocker rows) run only when something blocks. Truncated to + * {@link FORK_SYNC_BLOCKER_LIMIT} entries. + */ +export async function collectForkSyncBlockers( + params: Omit & { + executor: DbOrTx + sourceWorkspaceId: string + /** + * The plan's unmapped references (`unmappedRequired` + `unmappedOptional`), when the caller + * computed the plan over the SAME `items`/`sourceStates` inside the same transaction AND the + * gate `resolver` only augments the plan's resolver (never un-resolves a plan-mapped ref) - + * promote's copy-selection overlay satisfies both. Enables the happy-path shortcut via + * {@link hasForkSyncBlockerCandidates}: the full per-block reference scan the plan already + * ran is skipped when no blocking candidate can exist, and re-run (for byte-identical blocker + * rows) when one does. Omit to always collect from scratch. + */ + planUnmapped?: ReadonlyArray> + } +): Promise { + const { executor, sourceWorkspaceId, planUnmapped, ...collectParams } = params + if (planUnmapped && !hasForkSyncBlockerCandidates(planUnmapped, collectParams)) return [] + const candidates = collectForkClearedRefCandidates({ + ...collectParams, + sourceLabels: new Map(), + sourceWorkflowNames: new Map(), + }) + if (!candidates.some((ref) => ref.cause === 'reference' || ref.cause === 'workflow')) return [] + + const annotated = await annotateForkClearedRefSourceLiveness( + executor, + sourceWorkspaceId, + candidates + ) + const blocking = selectForkSyncBlockingRefs(annotated).slice(0, FORK_SYNC_BLOCKER_LIMIT) + if (blocking.length === 0) return [] + + // 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 + // the deleted filter so a source-deleted server still names itself; workflow names label the + // `workflow`-cause entries. + const copyableIdsByKind: Partial> = {} + const mcpIds: string[] = [] + const workflowIds: string[] = [] + for (const { ref } of blocking) { + if (ref.cause === 'workflow') workflowIds.push(ref.sourceId) + else if (ref.kind === 'mcp-server') mcpIds.push(ref.sourceId) + else if (isForkCopyableKind(ref.kind)) (copyableIdsByKind[ref.kind] ??= []).push(ref.sourceId) + } + const [copyableLabels, mcpRows, workflowRows] = await Promise.all([ + loadForkCopyableResourceLabels(executor, sourceWorkspaceId, copyableIdsByKind), + mcpIds.length === 0 + ? Promise.resolve([] as Array<{ id: string; name: string }>) + : executor + .select({ id: mcpServers.id, name: mcpServers.name }) + .from(mcpServers) + .where( + and(eq(mcpServers.workspaceId, sourceWorkspaceId), inArray(mcpServers.id, mcpIds)) + ), + workflowIds.length === 0 + ? Promise.resolve([] as Array<{ id: string; name: string }>) + : executor + .select({ id: workflow.id, name: workflow.name }) + .from(workflow) + .where( + and(eq(workflow.workspaceId, sourceWorkspaceId), inArray(workflow.id, workflowIds)) + ), + ]) + const mcpNames = new Map(mcpRows.map((row) => [row.id, row.name])) + const workflowNames = new Map(workflowRows.map((row) => [row.id, row.name])) + const labelFor = (ref: ForkClearedRef): string => { + if (ref.cause === 'workflow') return workflowNames.get(ref.sourceId) ?? ref.sourceLabel + if (ref.kind === 'mcp-server') return mcpNames.get(ref.sourceId) ?? ref.sourceLabel + return copyableLabels.get(`${ref.kind}:${ref.sourceId}`)?.label ?? ref.sourceLabel + } + + return toForkSyncBlockers( + blocking.map(({ ref, reason }) => ({ ref: { ...ref, sourceLabel: labelFor(ref) }, reason })) + ) +} diff --git a/apps/sim/lib/workspaces/fork/promote/copy-unmapped.test.ts b/apps/sim/lib/workspaces/fork/promote/copy-unmapped.test.ts index 91e0e74341c..4737665694f 100644 --- a/apps/sim/lib/workspaces/fork/promote/copy-unmapped.test.ts +++ b/apps/sim/lib/workspaces/fork/promote/copy-unmapped.test.ts @@ -53,16 +53,55 @@ import { isForkCopyableKind } from '@/lib/workspaces/fork/promote/promote-plan' import type { ForkRemapKind } from '@/lib/workspaces/fork/remap/remap-references' const candidates: ForkCopyableUnmapped[] = [ - { kind: 'knowledge-base', sourceId: 'kb-1', label: 'KB One', parentId: null, parentLabel: null }, - { kind: 'table', sourceId: 'tbl-1', label: 'Table One', parentId: null, parentLabel: null }, - { kind: 'custom-tool', sourceId: 'ct-1', label: 'Tool One', parentId: null, parentLabel: null }, - { kind: 'skill', sourceId: 'sk-1', label: 'Skill One', parentId: null, parentLabel: null }, + { + kind: 'knowledge-base', + sourceId: 'kb-1', + label: 'KB One', + parentId: null, + parentLabel: null, + referenced: true, + }, + { + kind: 'table', + sourceId: 'tbl-1', + label: 'Table One', + parentId: null, + parentLabel: null, + referenced: true, + }, + { + kind: 'custom-tool', + sourceId: 'ct-1', + label: 'Tool One', + parentId: null, + parentLabel: null, + referenced: true, + }, + { + kind: 'skill', + sourceId: 'sk-1', + label: 'Skill One', + parentId: null, + parentLabel: null, + referenced: true, + }, { kind: 'file', sourceId: 'workspace/SRC/a.png', label: 'a.png', parentId: 'fld-1', parentLabel: 'Images', + referenced: true, + }, + // An UNREFERENCED candidate (new in the source, used by no synced workflow): selectable for + // copy exactly like a referenced one - the server treats the two identically. + { + kind: 'table', + sourceId: 'tbl-unref', + label: 'Scratch table', + parentId: null, + parentLabel: null, + referenced: false, }, ] @@ -76,7 +115,6 @@ describe('buildPromoteCopySelection', () => { expect(selection.tables).toEqual(['tbl-1']) expect(selection.customTools).toEqual(['ct-1']) expect(selection.skills).toEqual(['sk-1']) - expect(selection.workflowMcpServers).toEqual([]) expect(willResolve.has('knowledge-base:kb-1')).toBe(true) expect(willResolve.has('skill:sk-1')).toBe(true) }) @@ -106,12 +144,31 @@ describe('buildPromoteCopySelection', () => { expect(willResolve.size).toBe(0) }) + it('accepts an UNREFERENCED candidate exactly like a referenced one', () => { + // The client keeps unreferenced candidates default-unselected, but once the user opts in the + // server validates + copies them through the same path. Its willResolve key matches no + // unmapped reference (nothing references it), so the pre-copy gate is unaffected. + const { selection, willResolve } = buildPromoteCopySelection( + { tables: ['tbl-unref'] }, + candidates + ) + expect(selection.tables).toEqual(['tbl-unref']) + expect(willResolve.has('table:tbl-unref')).toBe(true) + }) + it('copy-vs-map: maps win - a mapped resource is absent from the candidates, so a copy request for it is dropped', () => { // Reconciliation precedence at the server boundary: a resource the user mapped resolves to a // target, so the plan never lists it in `copyableUnmapped`. Even if a (stale) client still // requests it for copy, only the genuinely-unmapped candidates survive - the map wins. const onlyTableUnmapped: ForkCopyableUnmapped[] = [ - { kind: 'table', sourceId: 'tbl-1', label: 'Table One', parentId: null, parentLabel: null }, + { + kind: 'table', + sourceId: 'tbl-1', + label: 'Table One', + parentId: null, + parentLabel: null, + referenced: true, + }, ] const { selection, willResolve } = buildPromoteCopySelection( // kb-1 + the file were mapped (so absent from candidates); only the table remains copyable. @@ -137,7 +194,6 @@ describe('hasPromoteCopySelection', () => { hasPromoteCopySelection({ customTools: [], skills: [], - workflowMcpServers: [], tables: [], knowledgeBases: ['kb-1'], files: [], @@ -147,7 +203,6 @@ describe('hasPromoteCopySelection', () => { hasPromoteCopySelection({ customTools: [], skills: [], - workflowMcpServers: [], tables: [], knowledgeBases: [], files: [], @@ -157,7 +212,6 @@ describe('hasPromoteCopySelection', () => { hasPromoteCopySelection({ customTools: [], skills: [], - workflowMcpServers: [], tables: [], knowledgeBases: [], files: ['workspace/SRC/file.png'], @@ -229,6 +283,9 @@ describe('copyPromoteUnmappedResources - files + folder content-refs', () => { const tx = {} as DbOrTx // Only edge.childWorkspaceId is read by the copy path. const edge = { childWorkspaceId: 'edge-child' } as unknown as ForkEdge + // The promote-built persisted-pair resolver; the copy must forward it verbatim so copied + // tables' workflow-group outputs land on the same block ids the workflow writes assign. + const resolveBlockId = (workflowId: string, blockId: string) => `${workflowId}:${blockId}` beforeEach(() => { vi.clearAllMocks() @@ -287,7 +344,6 @@ describe('copyPromoteUnmappedResources - files + folder content-refs', () => { selection: { customTools: [], skills: [], - workflowMcpServers: [], tables: [], knowledgeBases: [], files: ['workspace/SRC/a.png'], @@ -295,6 +351,7 @@ describe('copyPromoteUnmappedResources - files + folder content-refs', () => { workflowIdMap: new Map(), folderIdMap: new Map([['fld-src', 'fld-dst']]), resolver: () => null, + resolveBlockId, referencedDocumentIds: [], }) @@ -323,6 +380,64 @@ describe('copyPromoteUnmappedResources - files + folder content-refs', () => { expect(result.contentRefMaps.fileIds).toEqual({ 'file-src': 'file-dst' }) }) + it('persists container mapping entries for copied resources (idempotency for unreferenced copies)', async () => { + // An UNREFERENCED table selected for copy flows through the same container pipeline; its + // mapping row is what makes the next sync resolve the copy instead of re-offering it. + mockCopyForkResourceContainers.mockResolvedValue({ + idMap: new Map([['table', new Map([['tbl-unref', 'tbl-copy']])]]), + mappingEntries: [ + { resourceType: 'table', parentResourceId: 'tbl-unref', childResourceId: 'tbl-copy' }, + ], + contentPlan: { + sourceWorkspaceId: 'src-ws', + childWorkspaceId: 'target-ws', + userId: 'user-1', + tables: [{ sourceId: 'tbl-unref', childId: 'tbl-copy' }], + knowledgeBases: [], + skills: [], + documents: [], + }, + names: { + tables: ['Scratch table'], + knowledgeBases: [], + customTools: [], + skills: [], + workflowMcpServers: [], + }, + }) + mockPlanForkFileCopies.mockResolvedValue({ + keyMap: new Map(), + idMap: new Map(), + blobTasks: [], + }) + + await copyPromoteUnmappedResources({ + tx, + edge, + sourceWorkspaceId: 'src-ws', + targetWorkspaceId: 'target-ws', + direction: 'pull', + userId: 'user-1', + now: new Date(), + selection: { + customTools: [], + skills: [], + tables: ['tbl-unref'], + knowledgeBases: [], + files: [], + }, + workflowIdMap: new Map(), + folderIdMap: new Map(), + resolver: () => null, + resolveBlockId, + referencedDocumentIds: [], + }) + + expect(mockUpsertEdgeMappings).toHaveBeenCalledWith(tx, 'edge-child', 'user-1', [ + { resourceType: 'table', parentResourceId: 'tbl-unref', childResourceId: 'tbl-copy' }, + ]) + }) + it('threads the plan-provided referencedDocumentIds into both doc-copy paths (no in-tx re-scan)', async () => { await copyPromoteUnmappedResources({ tx, @@ -335,7 +450,6 @@ describe('copyPromoteUnmappedResources - files + folder content-refs', () => { selection: { customTools: [], skills: [], - workflowMcpServers: [], tables: [], knowledgeBases: ['kb-1'], files: [], @@ -343,13 +457,22 @@ describe('copyPromoteUnmappedResources - files + folder content-refs', () => { workflowIdMap: new Map(), folderIdMap: new Map(), resolver: () => null, + resolveBlockId, // The doc ids come straight from the promote plan's references; the copy must forward them, // not re-scan every source workflow state inside the locked tx. referencedDocumentIds: ['doc-1', 'doc-2'], }) expect(mockCopyForkResourceContainers).toHaveBeenCalledWith( - expect.objectContaining({ referencedDocumentIds: ['doc-1', 'doc-2'] }) + expect.objectContaining({ + referencedDocumentIds: ['doc-1', 'doc-2'], + // Workflow-publishing MCP servers are fork-create-only; a sync always passes the + // shared pipeline's slot empty (PromoteCopySelection has no such field). + selection: expect.objectContaining({ workflowMcpServers: [] }), + // The promote-built block-id resolver reaches the table remap unchanged, so copied + // tables' workflow-group outputs use the persisted-pair ids, not the derive. + resolveBlockId, + }) ) expect(mockPlanForkMappedKbDocumentCopies).toHaveBeenCalledWith( expect.objectContaining({ referencedDocumentIds: ['doc-1', 'doc-2'] }) diff --git a/apps/sim/lib/workspaces/fork/promote/copy-unmapped.ts b/apps/sim/lib/workspaces/fork/promote/copy-unmapped.ts index f0080b1d46c..cf1d33d56c8 100644 --- a/apps/sim/lib/workspaces/fork/promote/copy-unmapped.ts +++ b/apps/sim/lib/workspaces/fork/promote/copy-unmapped.ts @@ -21,16 +21,20 @@ import { resourceTypeToForkKind, upsertEdgeMappings, } from '@/lib/workspaces/fork/mapping/mapping-store' +import type { ForkBlockIdResolver } from '@/lib/workspaces/fork/remap/block-identity' import type { ForkReferenceResolver, ForkRemapKind, } from '@/lib/workspaces/fork/remap/remap-references' -/** The source ids selected for copy at promote, validated against the plan's copyable candidates. */ +/** + * The source ids selected for copy at promote, validated against the plan's copyable + * candidates. Exactly the sync-copyable kinds (`forkCopyableKindSchema`): workflow-publishing + * MCP servers are fork-create-only (never a promote copy candidate), so they have no slot here. + */ export interface PromoteCopySelection { customTools: string[] skills: string[] - workflowMcpServers: string[] tables: string[] knowledgeBases: string[] /** Workspace files to copy, identified by storage key (not `workspace_files.id`). */ @@ -54,10 +58,11 @@ export const FORK_COPYABLE_KIND_TO_SELECTION_KEY: Record< } /** - * Intersect the user's requested copy with the plan's actual copyable candidates, so a sync can - * only copy resources that are genuinely referenced-but-unmapped + still exist in the source (a - * crafted request can never copy an arbitrary resource). Returns the validated selection plus the - * set of `${kind}:${sourceId}` references the copy will resolve, for the pre-copy sync gate. + * Intersect the user's requested copy with the plan's actual copyable candidates (referenced or + * not, always unmapped + still existing in the source), so a crafted request can never copy an + * arbitrary resource. Returns the validated selection plus the set of `${kind}:${sourceId}` + * references the copy will resolve, for the pre-copy sync gate - an unreferenced candidate's key + * simply matches no reference there, which is harmless. */ export function buildPromoteCopySelection( requested: PromoteCopyResources | undefined, @@ -72,7 +77,6 @@ export function buildPromoteCopySelection( const selection: PromoteCopySelection = { customTools: [], skills: [], - workflowMcpServers: [], tables: [], knowledgeBases: [], files: [], @@ -140,8 +144,8 @@ export interface PromoteCopyResult { } /** - * Copy the referenced-but-unmapped resources a sync brings into the target (reusing the fork copy - * pipeline), then persist the source<->target id map in the direction the edge expects: a pull + * Copy the selected unmapped resources (referenced or not) a sync brings into the target (reusing + * the fork copy pipeline), then persist the source<->target id map in the direction the edge expects: a pull * fills the existing `(parent, child=null)` row (fill-null), a push replaces any prior * `(parent, child)` row keyed on the source child resource (delete-then-insert). This covers: * - the user-selected copyable containers (KB / table / custom-tool / skill) and workspace files, @@ -168,6 +172,12 @@ export async function copyPromoteUnmappedResources(params: { folderIdMap: Map /** Base resolver (persisted mappings + env identity), used to detect already-mapped KBs (U-docs). */ resolver: ForkReferenceResolver + /** + * The SAME block-id resolver the sync's workflow writes use (persisted pairs preferred over + * derive), so copied tables' workflow-group `outputs[].blockId` point at the blocks the sync + * actually writes - on push the parent keeps its ORIGINAL block ids, never the derive. + */ + resolveBlockId: ForkBlockIdResolver /** * Knowledge-document ids the synced workflows reference, already scanned once in the promote * plan and threaded in so the copy doesn't re-scan every source state inside the locked tx. @@ -188,6 +198,7 @@ export async function copyPromoteUnmappedResources(params: { workflowIdMap, folderIdMap, resolver, + resolveBlockId, referencedDocumentIds, } = params @@ -200,7 +211,9 @@ export async function copyPromoteUnmappedResources(params: { selection: { customTools: selection.customTools, skills: selection.skills, - workflowMcpServers: selection.workflowMcpServers, + // Workflow-publishing MCP servers are fork-create-only (never a sync-copy candidate); + // the shared copy pipeline still takes the slot, so pass it empty. + workflowMcpServers: [], tables: selection.tables, knowledgeBases: selection.knowledgeBases, }, @@ -209,6 +222,7 @@ export async function copyPromoteUnmappedResources(params: { // A sync can rename env vars, so a copied custom tool's `code` must have its `{{ENV}}` refs // rewritten through the same plan resolver that remaps subblock-value env refs. resolveEnvName: (key) => resolver('env-var', key), + resolveBlockId, }) // Copy the selected workspace files (keyed by storage key) - metadata inserts in the tx, blob diff --git a/apps/sim/lib/workspaces/fork/promote/promote-plan.test.ts b/apps/sim/lib/workspaces/fork/promote/promote-plan.test.ts index bf3d07b1791..f01e4d44a35 100644 --- a/apps/sim/lib/workspaces/fork/promote/promote-plan.test.ts +++ b/apps/sim/lib/workspaces/fork/promote/promote-plan.test.ts @@ -2,11 +2,15 @@ * @vitest-environment node */ import { describe, expect, it } from 'vitest' -import type { ForkCopyableLabel } from '@/lib/workspaces/fork/mapping/resources' +import type { + ForkCopyableLabel, + ForkCopyableSourceResource, +} from '@/lib/workspaces/fork/mapping/resources' import { assembleForkCopyableUnmapped, buildPromoteWorkflowIdMap, collectForkCopyableIdsByKind, + collectForkUnreferencedCopyables, } from '@/lib/workspaces/fork/promote/promote-plan' import type { ForkReference } from '@/lib/workspaces/fork/remap/remap-references' @@ -135,8 +139,16 @@ describe('assembleForkCopyableUnmapped', () => { label: 'Docs KB', parentId: null, parentLabel: null, + referenced: true, + }, + { + kind: 'file', + sourceId: 'fk-1', + label: 'a.png', + parentId: 'fld-1', + parentLabel: 'Folder', + referenced: true, }, - { kind: 'file', sourceId: 'fk-1', label: 'a.png', parentId: 'fld-1', parentLabel: 'Folder' }, ]) }) @@ -152,3 +164,93 @@ describe('assembleForkCopyableUnmapped', () => { expect(result).toEqual([]) }) }) + +describe('collectForkUnreferencedCopyables', () => { + const source = ( + kind: ForkCopyableSourceResource['kind'], + sourceId: string, + label = sourceId + ): ForkCopyableSourceResource => ({ kind, sourceId, label, parentId: null, parentLabel: null }) + + const referencedCandidate = (kind: ForkCopyableSourceResource['kind'], sourceId: string) => ({ + kind, + sourceId, + label: sourceId, + parentId: null, + parentLabel: null, + referenced: true, + }) + + it('emits an unmapped source resource no synced workflow references, flagged referenced: false', () => { + const result = collectForkUnreferencedCopyables( + [source('table', 'tbl-new', 'Scratch table')], + [], + () => null + ) + expect(result).toEqual([ + { + kind: 'table', + sourceId: 'tbl-new', + label: 'Scratch table', + parentId: null, + parentLabel: null, + referenced: false, + }, + ]) + }) + + it('dedupes against the referenced candidate set (a referenced resource is never double-listed)', () => { + const result = collectForkUnreferencedCopyables( + [source('knowledge-base', 'kb-1'), source('knowledge-base', 'kb-new')], + [referencedCandidate('knowledge-base', 'kb-1')], + () => null + ) + expect(result.map((candidate) => candidate.sourceId)).toEqual(['kb-new']) + }) + + it('excludes a resource with a persisted mapping (idempotency: a prior copy is never re-offered)', () => { + // A resource copied by a prior sync resolves through its workspace_fork_resource_map row. + const result = collectForkUnreferencedCopyables( + [source('skill', 'sk-copied'), source('skill', 'sk-new')], + [], + (kind, sourceId) => (kind === 'skill' && sourceId === 'sk-copied' ? 'sk-target' : null) + ) + expect(result.map((candidate) => candidate.sourceId)).toEqual(['sk-new']) + }) + + it('does not confuse the same id across kinds when deduping or resolving', () => { + const result = collectForkUnreferencedCopyables( + [source('table', 'shared-id'), source('skill', 'shared-id')], + [referencedCandidate('table', 'shared-id')], + () => null + ) + expect(result).toHaveLength(1) + expect(result[0]).toMatchObject({ kind: 'skill', sourceId: 'shared-id', referenced: false }) + }) + + it('carries a file candidate keyed by storage key with its folder grouping', () => { + const result = collectForkUnreferencedCopyables( + [ + { + kind: 'file', + sourceId: 'workspace/SRC/new.png', + label: 'new.png', + parentId: 'fld-1', + parentLabel: 'Images', + }, + ], + [], + () => null + ) + expect(result).toEqual([ + { + kind: 'file', + sourceId: 'workspace/SRC/new.png', + label: 'new.png', + parentId: 'fld-1', + parentLabel: 'Images', + referenced: false, + }, + ]) + }) +}) diff --git a/apps/sim/lib/workspaces/fork/promote/promote-plan.ts b/apps/sim/lib/workspaces/fork/promote/promote-plan.ts index e4d271b736a..32289c204fd 100644 --- a/apps/sim/lib/workspaces/fork/promote/promote-plan.ts +++ b/apps/sim/lib/workspaces/fork/promote/promote-plan.ts @@ -13,8 +13,10 @@ import { } from '@/lib/workspaces/fork/mapping/mapping-store' import { type ForkCopyableLabel, + type ForkCopyableSourceResource, filterExistingForkTargets, getWorkspaceEnvKeys, + listForkCopyableSourceResources, loadForkCopyableResourceLabels, } from '@/lib/workspaces/fork/mapping/resources' import { toScannerBlocks } from '@/lib/workspaces/fork/remap/reference-scan' @@ -61,10 +63,13 @@ export interface ForkPromotePlan { /** Review-only descriptions of inline secrets that cannot be id-mapped. */ inlineSecretSources: string[] /** - * Referenced-but-unmapped resources of copyable kinds that still exist in the source, so a - * sync can copy them into the target instead of requiring a manual mapping (U15). Documents - * are auto-copied with their parent KB and are not listed here. `parentId`/`parentLabel` carry - * a file's folder grouping (null for non-file kinds and root files), for the nested picker. + * Unmapped resources of copyable kinds that still exist in the source, so a sync can copy + * them into the target instead of requiring a manual mapping (U15). `referenced: true` + * entries are referenced by the synced workflows (default-selected in the modal - skipping + * one clears its references); `referenced: false` entries are used by no synced workflow + * (default-unselected - skipping one breaks nothing). Documents are auto-copied with their + * parent KB and are not listed here. `parentId`/`parentLabel` carry a file's folder grouping + * (null for non-file kinds and root files), for the nested picker. */ copyableUnmapped: Array<{ kind: ForkCopyableKind @@ -72,6 +77,7 @@ export interface ForkPromotePlan { label: string parentId: string | null parentLabel: string | null + referenced: boolean }> willUpdate: number willCreate: number @@ -136,10 +142,10 @@ export function collectForkCopyableIdsByKind( } /** - * Assemble {@link ForkPromotePlan.copyableUnmapped} from the unmapped references and the loaded - * source labels: each copyable reference whose label resolved becomes a copy candidate; one whose - * label is missing (the resource no longer exists in the source) is dropped. Pure - split from the - * DB label load so it is unit-testable. + * Assemble the REFERENCED slice of {@link ForkPromotePlan.copyableUnmapped} from the unmapped + * references and the loaded source labels: each copyable reference whose label resolved becomes a + * copy candidate; one whose label is missing (the resource no longer exists in the source) is + * dropped. Pure - split from the DB label load so it is unit-testable. */ export function assembleForkCopyableUnmapped( unmappedReferences: ForkReference[], @@ -156,12 +162,36 @@ export function assembleForkCopyableUnmapped( label: entry.label, parentId: entry.parentId, parentLabel: entry.parentLabel, + referenced: true, }, ] : [] }) } +/** + * Assemble the UNREFERENCED slice of {@link ForkPromotePlan.copyableUnmapped}: every copyable + * resource in the source workspace that no synced workflow references (not in the referenced + * candidate set) and that has no target mapping for this edge (the resolver returns null). A + * previously-copied resource resolves through its persisted `workspace_fork_resource_map` row, + * so a re-sync never re-offers it (idempotency). Pure - split from the DB source listing so it + * is unit-testable. + */ +export function collectForkUnreferencedCopyables( + sourceResources: ForkCopyableSourceResource[], + referencedCopyables: ForkPromotePlan['copyableUnmapped'], + resolver: ForkReferenceResolver +): ForkPromotePlan['copyableUnmapped'] { + const referencedKeys = new Set( + referencedCopyables.map((candidate) => `${candidate.kind}:${candidate.sourceId}`) + ) + return sourceResources.flatMap((resource) => { + if (referencedKeys.has(`${resource.kind}:${resource.sourceId}`)) return [] + if (resolver(resource.kind, resource.sourceId) != null) return [] + return [{ ...resource, referenced: false }] + }) +} + /** * Compute everything a promote needs without mutating. Only the source's * **deployed** workflows participate; each plan item carries the source's active @@ -330,7 +360,15 @@ export async function computeForkPromotePlan(params: { sourceWorkspaceId, collectForkCopyableIdsByKind(allUnmapped) ) - const copyableUnmapped = assembleForkCopyableUnmapped(allUnmapped, copyableLabels) + const referencedCopyables = assembleForkCopyableUnmapped(allUnmapped, copyableLabels) + // Also offer the source's UNREFERENCED copyable resources with no target mapping (e.g. newly + // created since the fork), default-unselected in the modal. Mapped ones (including everything + // a prior sync copied) resolve non-null and drop out, so a re-sync never re-offers a copy. + const sourceCopyables = await listForkCopyableSourceResources(executor, sourceWorkspaceId) + const copyableUnmapped = [ + ...referencedCopyables, + ...collectForkUnreferencedCopyables(sourceCopyables, referencedCopyables, resolver), + ] const willUpdate = items.filter((i) => i.mode === 'replace').length const willCreate = items.filter((i) => i.mode === 'create').length diff --git a/apps/sim/lib/workspaces/fork/promote/promote.test.ts b/apps/sim/lib/workspaces/fork/promote/promote.test.ts new file mode 100644 index 00000000000..e9b2a4f7d5b --- /dev/null +++ b/apps/sim/lib/workspaces/fork/promote/promote.test.ts @@ -0,0 +1,401 @@ +/** + * @vitest-environment node + */ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { ForkSyncBlocker } from '@/lib/api/contracts/workspace-fork' + +const { + mockComputePlan, + mockBuildCopySelection, + mockHasCopySelection, + mockCopyUnmapped, + mockCollectBlockers, + mockLoadBlockMap, + mockBuildBlockIdResolver, + mockResolveFolderMapping, + mockUpsertPromoteRun, + mockLoadSourceDeployedStates, + mockGetUsersWithPermissions, + mockGetMcpServerMeta, + mockCreateTransform, + mockSumForkCopyBytes, + mockAssertForkStorageHeadroom, +} = vi.hoisted(() => ({ + mockComputePlan: vi.fn(), + mockBuildCopySelection: vi.fn(), + mockHasCopySelection: vi.fn(), + mockCopyUnmapped: vi.fn(), + mockCollectBlockers: vi.fn(), + mockLoadBlockMap: vi.fn(), + mockBuildBlockIdResolver: vi.fn(), + mockResolveFolderMapping: vi.fn(), + mockUpsertPromoteRun: vi.fn(), + mockLoadSourceDeployedStates: vi.fn(), + mockGetUsersWithPermissions: vi.fn(), + mockGetMcpServerMeta: vi.fn(), + mockCreateTransform: vi.fn(), + mockSumForkCopyBytes: vi.fn(), + mockAssertForkStorageHeadroom: vi.fn(), +})) + +vi.mock('@/lib/workflows/deployment-outbox', () => ({ + enqueueWorkflowUndeploySideEffects: vi.fn(), + processWorkflowDeploymentOutboxEvent: vi.fn(), +})) +vi.mock('@/lib/workflows/orchestration/deploy', () => ({ + performFullDeploy: vi.fn(async () => ({ success: true })), +})) +vi.mock('@/lib/workflows/persistence/utils', () => ({ + undeployWorkflow: vi.fn(async () => ({ success: true })), +})) +vi.mock('@/lib/workspaces/fork/background-work/store', () => ({ + startBackgroundWork: vi.fn(), +})) +vi.mock('@/lib/workspaces/fork/copy/content-copy-runner', () => ({ + hasForkContentToCopy: vi.fn(() => false), + scheduleForkContentCopy: vi.fn(), +})) +vi.mock('@/lib/workspaces/fork/copy/copy-workflows', () => ({ + copyWorkflowStateIntoTarget: vi.fn(), + loadTargetDraftSubBlocks: vi.fn(async () => new Map()), + loadWorkflowNameRegistry: vi.fn(async () => new Map()), + resolveForkFolderMapping: mockResolveFolderMapping, +})) +vi.mock('@/lib/workspaces/fork/copy/storage-quota', () => ({ + sumForkCopyBytes: mockSumForkCopyBytes, + assertForkStorageHeadroom: mockAssertForkStorageHeadroom, +})) +vi.mock('@/lib/workspaces/fork/copy/deploy-bridge', () => ({ + getActiveDeploymentVersionNumbers: vi.fn(async () => new Map()), + loadSourceDeployedStates: mockLoadSourceDeployedStates, +})) +vi.mock('@/lib/workspaces/fork/lineage/lineage', () => ({ + acquireForkEdgeLock: vi.fn(), + acquireForkTargetLock: vi.fn(), + setForkLockTimeout: vi.fn(), +})) +vi.mock('@/lib/workspaces/fork/mapping/block-map-store', () => ({ + loadForkBlockMap: mockLoadBlockMap, + reconcileForkBlockPairs: vi.fn(), + toForkBlockPairs: vi.fn(() => []), +})) +vi.mock('@/lib/workspaces/fork/mapping/dependent-value-store', () => ({ + loadForkDependentValues: vi.fn(async () => []), + reconcileForkDependentValues: vi.fn(), +})) +vi.mock('@/lib/workspaces/fork/mapping/mapping-store', () => ({ + deleteWorkflowIdentityByIds: vi.fn(), + upsertEdgeMappings: vi.fn(), +})) +vi.mock('@/lib/workspaces/fork/promote/cleared-refs', () => ({ + collectForkSyncBlockers: mockCollectBlockers, +})) +vi.mock('@/lib/workspaces/fork/promote/copy-unmapped', () => ({ + augmentForkResolver: vi.fn((base) => base), + buildPromoteCopySelection: mockBuildCopySelection, + copyPromoteUnmappedResources: mockCopyUnmapped, + hasPromoteCopySelection: mockHasCopySelection, +})) +vi.mock('@/lib/workspaces/fork/promote/promote-plan', () => ({ + computeForkPromotePlan: mockComputePlan, +})) +vi.mock('@/lib/workspaces/fork/promote/promote-run-store', () => ({ + upsertPromoteRun: mockUpsertPromoteRun, +})) +vi.mock('@/lib/workspaces/fork/mapping/resources', () => ({ + getMcpServerMetaByIds: mockGetMcpServerMeta, +})) +vi.mock('@/lib/workspaces/fork/remap/block-identity', () => ({ + buildForkBlockIdResolver: mockBuildBlockIdResolver, +})) +vi.mock('@/lib/workspaces/fork/remap/remap-references', () => ({ + createForkSubBlockTransform: mockCreateTransform, +})) +vi.mock('@/lib/workspaces/fork/socket', () => ({ + notifyForkWorkflowChanged: vi.fn(), +})) +vi.mock('@/lib/workspaces/permissions/utils', () => ({ + getUsersWithPermissions: mockGetUsersWithPermissions, +})) + +import { db } from '@sim/db' +import { promoteFork } from '@/lib/workspaces/fork/promote/promote' +import type { ForkPromotePlan } from '@/lib/workspaces/fork/promote/promote-plan' + +const EDGE = { childWorkspaceId: 'child-ws', parentWorkspaceId: 'parent-ws' } + +const EMPTY_SELECTION = { + customTools: [], + skills: [], + tables: [], + knowledgeBases: [], + files: [], +} + +function makePlan(overrides: Partial = {}): ForkPromotePlan { + return { + childWorkspaceId: EDGE.childWorkspaceId, + sourceWorkspaceId: 'src-ws', + targetWorkspaceId: 'tgt-ws', + direction: 'push', + resolver: () => null, + items: [], + workflowIdMap: new Map(), + archivedTargetIds: [], + archivedTargets: [], + references: [], + unmappedRequired: [], + unmappedOptional: [], + mcpReauthServerIds: [], + inlineSecretSources: [], + copyableUnmapped: [], + willUpdate: 0, + willCreate: 0, + willArchive: 0, + ...overrides, + } +} + +const BLOCKER: ForkSyncBlocker = { + workflowName: 'Caller', + blockLabel: 'Table Block', + fieldLabel: 'Table', + kind: 'table', + sourceId: 'tbl-1', + sourceLabel: 'Orders', + reason: 'unmapped-copyable', +} + +function promoteParams() { + return { + edge: EDGE as never, + sourceWorkspaceId: 'src-ws', + targetWorkspaceId: 'tgt-ws', + direction: 'push' as const, + userId: 'user-1', + } +} + +describe('promoteFork gates', () => { + beforeEach(() => { + vi.clearAllMocks() + vi.mocked(db.transaction).mockImplementation( + async (cb: (tx: unknown) => unknown) => cb({}) as never + ) + mockGetUsersWithPermissions.mockResolvedValue([]) + mockLoadSourceDeployedStates.mockResolvedValue({ + deployedWorkflows: [], + sourceStates: new Map(), + }) + mockComputePlan.mockResolvedValue(makePlan()) + mockBuildCopySelection.mockReturnValue({ + selection: EMPTY_SELECTION, + willResolve: new Set(), + }) + mockHasCopySelection.mockReturnValue(false) + mockCollectBlockers.mockResolvedValue([]) + mockLoadBlockMap.mockResolvedValue(new Map()) + mockBuildBlockIdResolver.mockReturnValue((_wf: string, blockId: string) => blockId) + mockResolveFolderMapping.mockResolvedValue(new Map()) + mockUpsertPromoteRun.mockResolvedValue('run-1') + mockGetMcpServerMeta.mockResolvedValue(new Map()) + mockCreateTransform.mockReturnValue((subBlocks: unknown) => subBlocks) + mockSumForkCopyBytes.mockResolvedValue(0) + mockAssertForkStorageHeadroom.mockResolvedValue(undefined) + }) + + it('blocks an over-quota copy selection before any lock, read, or write', async () => { + mockSumForkCopyBytes.mockResolvedValue(999_999) + mockAssertForkStorageHeadroom.mockRejectedValue( + new Error( + 'Not enough storage to copy the selected resources. Storage limit exceeded. Used: 10.50GB, Limit: 10GB' + ) + ) + + await expect( + promoteFork({ + ...promoteParams(), + copyResources: { files: ['workspace/src-ws/key-1'], knowledgeBases: ['kb-1'] }, + }) + ).rejects.toThrow('Not enough storage to copy the selected resources') + + expect(mockAssertForkStorageHeadroom).toHaveBeenCalledWith({ userId: 'user-1', bytes: 999_999 }) + // Fails fast: no source-state loads, no locked transaction, no writes of any kind. + expect(mockLoadSourceDeployedStates).not.toHaveBeenCalled() + expect(db.transaction).not.toHaveBeenCalled() + expect(mockUpsertPromoteRun).not.toHaveBeenCalled() + }) + + it('sums the requested copy selection bytes against the SOURCE workspace (files by key, KBs by id)', async () => { + await promoteFork({ + ...promoteParams(), + copyResources: { + files: ['workspace/src-ws/key-1'], + knowledgeBases: ['kb-1'], + tables: ['tbl-1'], + }, + }) + + expect(mockSumForkCopyBytes).toHaveBeenCalledTimes(1) + expect(mockSumForkCopyBytes).toHaveBeenCalledWith(expect.anything(), 'src-ws', { + fileKeys: ['workspace/src-ws/key-1'], + knowledgeBaseIds: ['kb-1'], + }) + }) + + it('blocks on unmapped required credentials/secrets BEFORE the cleared-refs gate runs', async () => { + mockComputePlan.mockResolvedValue( + makePlan({ + unmappedRequired: [ + { kind: 'credential', sourceId: 'c1', subBlockKey: 'credential', required: true }, + ], + }) + ) + + const result = await promoteFork(promoteParams()) + + expect(result.blocked).toBe('unmapped') + expect(result.unmappedRequired).toEqual([ + { kind: 'credential', sourceId: 'c1', required: true, blockName: undefined }, + ]) + expect(result.blockers).toEqual([]) + expect(mockCollectBlockers).not.toHaveBeenCalled() + expect(mockResolveFolderMapping).not.toHaveBeenCalled() + expect(mockUpsertPromoteRun).not.toHaveBeenCalled() + }) + + it('blocks with the structured blocker list when references would clear, writing NOTHING', async () => { + mockCollectBlockers.mockResolvedValue([BLOCKER]) + + const result = await promoteFork(promoteParams()) + + expect(result.blocked).toBe('cleared-refs') + expect(result.blockers).toEqual([BLOCKER]) + expect(result.promoteRunId).toBe('') + expect(result.updated).toBe(0) + expect(result.created).toBe(0) + expect(result.archived).toBe(0) + // Blocked before the first write: no folder creation, no resource copy, no undo point. + expect(mockResolveFolderMapping).not.toHaveBeenCalled() + expect(mockCopyUnmapped).not.toHaveBeenCalled() + expect(mockUpsertPromoteRun).not.toHaveBeenCalled() + }) + + it('evaluates the gate against the plan resolver overlaid with the copy selection', async () => { + const planResolver = vi.fn(() => 'plan-resolved') + mockComputePlan.mockResolvedValue(makePlan({ resolver: planResolver })) + mockBuildCopySelection.mockReturnValue({ + selection: EMPTY_SELECTION, + willResolve: new Set(['table:t1']), + }) + + await promoteFork(promoteParams()) + + expect(mockCollectBlockers).toHaveBeenCalledTimes(1) + const gateParams = mockCollectBlockers.mock.calls[0][0] + // A copy-selected reference resolves through the overlay (never hits the plan resolver); + // everything else falls through to the plan's persisted-mapping resolver. + expect(gateParams.resolver('table', 't1')).toBe('t1') + expect(planResolver).not.toHaveBeenCalled() + expect(gateParams.resolver('table', 't2')).toBe('plan-resolved') + expect(planResolver).toHaveBeenCalledWith('table', 't2') + }) + + it('threads the SAME block-id resolver into the gate and the resource copy as the workflow writes', async () => { + // Copied tables' workflow-group outputs must land on the block ids the sync actually writes + // (persisted pairs preferred over derive), so the copy receives the resolver built from the + // loaded block map - the identical instance the cleared-refs gate uses. + const resolver = (_workflowId: string, blockId: string) => `pair-${blockId}` + mockBuildBlockIdResolver.mockReturnValue(resolver) + mockHasCopySelection.mockReturnValue(true) + mockCopyUnmapped.mockResolvedValue({ + contentPlan: { + sourceWorkspaceId: 'src-ws', + childWorkspaceId: 'tgt-ws', + userId: 'user-1', + tables: [], + knowledgeBases: [], + skills: [], + documents: [], + }, + copyIdMapByKind: new Map(), + contentRefMaps: {}, + blobTasks: [], + }) + + await promoteFork(promoteParams()) + + expect(mockCopyUnmapped).toHaveBeenCalledTimes(1) + expect(mockCopyUnmapped.mock.calls[0][0].resolveBlockId).toBe(resolver) + expect(mockCollectBlockers.mock.calls[0][0].resolveBlockId).toBe(resolver) + }) + + it('proceeds when zero references would clear (empty blocker list)', async () => { + const plan = makePlan() + mockComputePlan.mockResolvedValue(plan) + + const result = await promoteFork(promoteParams()) + + expect(result.blocked).toBeNull() + expect(result.blockers).toEqual([]) + expect(result.promoteRunId).toBe('run-1') + expect(mockCollectBlockers).toHaveBeenCalledWith( + expect.objectContaining({ + sourceWorkspaceId: 'src-ws', + items: plan.items, + workflowIdMap: plan.workflowIdMap, + }) + ) + expect(mockUpsertPromoteRun).toHaveBeenCalledTimes(1) + }) + + it("threads the plan's unmapped references into the gate so it can reuse the plan's scan", async () => { + const unmappedOptional = [ + { kind: 'table' as const, sourceId: 'tbl-1', subBlockKey: 'tbl', required: false }, + ] + mockComputePlan.mockResolvedValue(makePlan({ unmappedOptional })) + + await promoteFork(promoteParams()) + + expect(mockCollectBlockers).toHaveBeenCalledWith( + expect.objectContaining({ planUnmapped: unmappedOptional }) + ) + }) + + it('batch-loads the mapped TARGET MCP server rows and threads them into the subblock transform', async () => { + // Two references resolving to the SAME target and one unmapped: the read must cover the + // distinct mapped target ids only (one bounded query, unmapped ids dropped). + const resolver = (kind: string, id: string) => { + if (kind !== 'mcp-server') return null + if (id === 'srv-a' || id === 'srv-b') return 'srv-tgt' + return null + } + mockComputePlan.mockResolvedValue( + makePlan({ + resolver, + references: [ + { kind: 'mcp-server', sourceId: 'srv-a', subBlockKey: 'tools', required: false }, + { kind: 'mcp-server', sourceId: 'srv-b', subBlockKey: 'server', required: false }, + { kind: 'mcp-server', sourceId: 'srv-unmapped', subBlockKey: 'tools', required: false }, + ], + }) + ) + mockGetMcpServerMeta.mockResolvedValue( + new Map([['srv-tgt', { name: 'Target Server', url: 'https://target.example/mcp' }]]) + ) + + await promoteFork(promoteParams()) + + expect(mockGetMcpServerMeta).toHaveBeenCalledTimes(1) + expect(mockGetMcpServerMeta).toHaveBeenCalledWith(expect.anything(), 'tgt-ws', ['srv-tgt']) + // The transform receives a lookup resolving the TARGET id to its row metadata, so remapped + // tool-input entries rewrite their embedded serverUrl/serverName from the target server. + expect(mockCreateTransform).toHaveBeenCalledTimes(1) + const [, transformOptions] = mockCreateTransform.mock.calls[0] + expect(transformOptions.resolveMcpServerMeta('srv-tgt')).toEqual({ + name: 'Target Server', + url: 'https://target.example/mcp', + }) + expect(transformOptions.resolveMcpServerMeta('srv-unknown')).toBeUndefined() + }) +}) diff --git a/apps/sim/lib/workspaces/fork/promote/promote.ts b/apps/sim/lib/workspaces/fork/promote/promote.ts index ad6e5d792c6..c475e8f7d25 100644 --- a/apps/sim/lib/workspaces/fork/promote/promote.ts +++ b/apps/sim/lib/workspaces/fork/promote/promote.ts @@ -4,7 +4,7 @@ import { createLogger } from '@sim/logger' import { getErrorMessage } from '@sim/utils/errors' import { generateId } from '@sim/utils/id' import { and, eq, inArray } from 'drizzle-orm' -import type { PromoteCopyResources } from '@/lib/api/contracts/workspace-fork' +import type { ForkSyncBlocker, PromoteCopyResources } from '@/lib/api/contracts/workspace-fork' import type { DbOrTx } from '@/lib/db/types' import { enqueueWorkflowUndeploySideEffects, @@ -31,6 +31,10 @@ import { getActiveDeploymentVersionNumbers, loadSourceDeployedStates, } from '@/lib/workspaces/fork/copy/deploy-bridge' +import { + assertForkStorageHeadroom, + sumForkCopyBytes, +} from '@/lib/workspaces/fork/copy/storage-quota' import { acquireForkEdgeLock, acquireForkTargetLock, @@ -53,6 +57,8 @@ import { type ForkMappingUpsert, upsertEdgeMappings, } from '@/lib/workspaces/fork/mapping/mapping-store' +import { getMcpServerMetaByIds } from '@/lib/workspaces/fork/mapping/resources' +import { collectForkSyncBlockers } from '@/lib/workspaces/fork/promote/cleared-refs' import { augmentForkResolver, buildPromoteCopySelection, @@ -71,6 +77,7 @@ import { buildForkBlockIdResolver } from '@/lib/workspaces/fork/remap/block-iden import { createForkSubBlockTransform, type ForkReference, + type ForkReferenceResolver, } from '@/lib/workspaces/fork/remap/remap-references' import { notifyForkWorkflowChanged } from '@/lib/workspaces/fork/socket' import { getUsersWithPermissions } from '@/lib/workspaces/permissions/utils' @@ -99,9 +106,10 @@ export interface PromoteForkParams { value: string }> /** - * Referenced-but-unmapped resources (by source id) the caller chose to copy into the target - * before the sync gate. Validated against the plan's copyable candidates, so an arbitrary id is - * ignored. Each copied resource's references then resolve to the new copy instead of blocking. + * Unmapped resources (by source id) the caller chose to copy into the target before the sync + * gate - referenced ones (their references then resolve to the new copy instead of blocking) + * and unreferenced ones (new in the source, brought along untouched). Validated against the + * plan's copyable candidates, so an arbitrary id is ignored. */ copyResources?: PromoteCopyResources requestId?: string @@ -120,7 +128,14 @@ export interface PromoteForkResult { */ deployFailed: number unmappedRequired: Array> - blocked: 'unmapped' | null + /** + * References the sync would have cleared in the target, so it was blocked without writing + * (`blocked: 'cleared-refs'`). The authoritative in-tx re-check of the diff's would-clear + * preview: normally the client blocks first, so a non-empty list means the state changed + * between preview and Sync. + */ + blockers: ForkSyncBlocker[] + blocked: 'unmapped' | 'cleared-refs' | null /** Names of the workflows the sync changed, by action, for the activity report. */ updatedNames: string[] createdNames: string[] @@ -256,10 +271,9 @@ async function propagateCredentialAccess( } } -interface PromoteTxBlocked { - blocked: 'unmapped' - unmappedRequired: PromoteForkResult['unmappedRequired'] -} +type PromoteTxBlocked = + | { blocked: 'unmapped'; unmappedRequired: PromoteForkResult['unmappedRequired'] } + | { blocked: 'cleared-refs'; blockers: ForkSyncBlocker[] } interface PromoteTxApplied { blocked: null @@ -329,7 +343,9 @@ function groupDependentOverrides( * propagated, and every promoted target is deployed. The plan is computed inside * the edge lock so concurrent promotes serialize. A sync always force-replaces the * target's deployed state (the modal confirms the overwrite up front); it blocks - * without mutating only when required references are unmapped. + * without mutating when required references (credentials / secrets) are unmapped OR + * when any reference would clear in a synced target workflow (the zero-cleared-refs + * gate - every reference must be mapped, selected for copy, or carried by the sync). */ export async function promoteFork(params: PromoteForkParams): Promise { const { edge, sourceWorkspaceId, targetWorkspaceId, direction, userId } = params @@ -352,6 +368,19 @@ export async function promoteFork(params: PromoteForkParams): Promise m.userId) // Read the source's deployed workflows + states BEFORE the transaction so these @@ -382,10 +411,10 @@ export async function promoteFork(params: PromoteForkParams): Promise target fully operational). Evaluated + // against the plan resolver overlaid with the validated copy selection (a selected copy + // resolves its references), BEFORE any write. Authoritative versus the diff's unlocked + // preview - state drift between preview and Sync re-blocks here (TOCTOU) - and it makes the + // in-tx remap's clear-unresolved behavior an unreachable defense-in-depth backstop. The + // plan's unmapped references are threaded through so the gate's happy path reuses the plan's + // scan (computed moments earlier over the same states, inside this same locked tx) instead of + // re-running the full per-block reference scan; the scan re-runs only when something blocks. + const gateResolver: ForkReferenceResolver = (kind, sourceId) => + willResolve.has(`${kind}:${sourceId}`) ? sourceId : plan.resolver(kind, sourceId) + const blockers = await collectForkSyncBlockers({ + executor: tx, + sourceWorkspaceId, + items: plan.items, + sourceStates, + resolver: gateResolver, + workflowIdMap: plan.workflowIdMap, + resolveBlockId, + planUnmapped: [...plan.unmappedRequired, ...plan.unmappedOptional], + }) + if (blockers.length > 0) { + return { blocked: 'cleared-refs', blockers } + } + // 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 // bodies (the post-commit content rewrite reads this map). Idempotent: it reuses target - // folders that already match by name within the same mapped parent. + // folders that already match by name within the same mapped parent. Creation is scoped to + // the folders that will hold a synced workflow (plus ancestors) - a folder whose subtree + // syncs nothing is never created empty in the target, though it still maps onto a matching + // existing target folder so prior syncs' refs keep resolving. const folderIdMap = await resolveForkFolderMapping({ tx, sourceWorkspaceId, targetWorkspaceId, userId, now, + contentFolderIds: plan.items.map((item) => item.sourceMeta.folderId), }) let resolver = plan.resolver @@ -445,6 +512,9 @@ export async function promoteFork(params: PromoteForkParams): Promise reference.kind === 'mcp-server') + .map((reference) => resolver('mcp-server', reference.sourceId)) + .filter((targetId): targetId is string => targetId != null) + ), + ] + const mcpServerMetaById = await getMcpServerMetaByIds( + tx, + targetWorkspaceId, + mappedMcpServerTargetIds + ) + + const transform = createForkSubBlockTransform(resolver, { + resolveMcpServerMeta: (targetServerId) => mcpServerMetaById.get(targetServerId), + }) // Batch every prior-version read (replace + archive targets) into one query before any // write, so the locked apply phase doesn't do N round-trips. Reads are pre-write, so @@ -491,13 +583,8 @@ export async function promoteFork(params: PromoteForkParams): Promise +type DependentRef = Extract + +const base = { + targetWorkflowId: 'wf-tgt', + workflowName: 'Workflow', + blockId: 'block-1', + blockLabel: 'Block', + fieldLabel: 'Field', + sourceLabel: 'Source', +} + +const referenceRef = ( + kind: ReferenceRef['kind'], + sourceId: string, + sourceDeleted = false +): ReferenceRef => ({ ...base, cause: 'reference', kind, sourceId, sourceDeleted }) + +const workflowRef = (sourceId: string): ForkClearedRef => ({ + ...base, + cause: 'workflow', + kind: 'workflow', + sourceId, +}) + +const dependentRef = (parentKind: DependentRef['parentKind']): DependentRef => ({ + ...base, + cause: 'dependent', + kind: parentKind, + sourceId: 'parent-src', + parentKind, + parentSourceId: 'parent-src', +}) + +describe('forkSyncBlockerReasonFor', () => { + it('maps a live unmapped copyable-kind reference to unmapped-copyable (map or copy)', () => { + for (const kind of ['table', 'knowledge-base', 'file', 'custom-tool', 'skill'] as const) { + expect(forkSyncBlockerReasonFor(referenceRef(kind, 'src-1'))).toBe('unmapped-copyable') + } + }) + + it('maps a live unmapped MCP server to unmapped-mcp-server (map-only; no copy option)', () => { + expect(forkSyncBlockerReasonFor(referenceRef('mcp-server', 'srv-1'))).toBe( + 'unmapped-mcp-server' + ) + }) + + it('maps a source-deleted reference of ANY kind to source-deleted (no exemption)', () => { + expect(forkSyncBlockerReasonFor(referenceRef('table', 'tbl-gone', true))).toBe('source-deleted') + expect(forkSyncBlockerReasonFor(referenceRef('mcp-server', 'srv-gone', true))).toBe( + 'source-deleted' + ) + expect(forkSyncBlockerReasonFor(referenceRef('file', 'workspace/SRC/gone.png', true))).toBe( + 'source-deleted' + ) + }) + + it('maps a workflow-cause entry to workflow-missing', () => { + expect(forkSyncBlockerReasonFor(workflowRef('wf-other'))).toBe('workflow-missing') + }) + + it('never blocks a dependent-cause entry (the reconfigure flow owns dependents)', () => { + expect(forkSyncBlockerReasonFor(dependentRef('credential'))).toBeNull() + expect(forkSyncBlockerReasonFor(dependentRef('knowledge-base'))).toBeNull() + }) + + it('defensively ignores kinds the collector excludes (credential / env-var / document)', () => { + // These never reach the cleared list (excluded by the collector); if one leaked, the + // kind-level required gate owns credentials/env-vars, so this path must not double-block. + expect(forkSyncBlockerReasonFor(referenceRef('credential', 'c1'))).toBeNull() + expect(forkSyncBlockerReasonFor(referenceRef('env-var', 'KEY'))).toBeNull() + expect(forkSyncBlockerReasonFor(referenceRef('knowledge-document', 'doc-1'))).toBeNull() + }) +}) + +describe('selectForkSyncBlockingRefs / toForkSyncBlockers', () => { + it('keeps reference + workflow causes with their reasons and drops dependents', () => { + const refs: ForkClearedRef[] = [ + referenceRef('table', 'tbl-1'), + referenceRef('mcp-server', 'srv-1'), + referenceRef('skill', 'sk-gone', true), + workflowRef('wf-other'), + dependentRef('credential'), + ] + const blocking = selectForkSyncBlockingRefs(refs) + expect(blocking.map(({ ref, reason }) => [ref.sourceId, reason])).toEqual([ + ['tbl-1', 'unmapped-copyable'], + ['srv-1', 'unmapped-mcp-server'], + ['sk-gone', 'source-deleted'], + ['wf-other', 'workflow-missing'], + ]) + }) + + it('maps blocking entries to the wire blocker shape', () => { + const blocking = selectForkSyncBlockingRefs([referenceRef('table', 'tbl-1')]) + expect(toForkSyncBlockers(blocking)).toEqual([ + { + workflowName: 'Workflow', + blockLabel: 'Block', + fieldLabel: 'Field', + kind: 'table', + sourceId: 'tbl-1', + sourceLabel: 'Source', + reason: 'unmapped-copyable', + }, + ]) + }) +}) diff --git a/apps/sim/lib/workspaces/fork/promote/sync-blockers.ts b/apps/sim/lib/workspaces/fork/promote/sync-blockers.ts new file mode 100644 index 00000000000..b5d2bbed6c2 --- /dev/null +++ b/apps/sim/lib/workspaces/fork/promote/sync-blockers.ts @@ -0,0 +1,65 @@ +import { + type ForkClearedRef, + type ForkSyncBlocker, + type ForkSyncBlockerReason, + forkCopyableKindSchema, +} from '@/lib/api/contracts/workspace-fork' + +/** + * Pure sync-blocker taxonomy, shared by the server gate (promote) and the modal's blocker + * rendering. A sync is allowed only when ZERO references would clear in any synced target + * workflow; every would-clear entry of cause `reference` or `workflow` is a blocker with an + * actionable reason. `dependent`-cause entries are NOT blockers - the dependent/reconfigure + * flow owns them (its own required gating), and a credential-anchored dependent clears on any + * parent remap, so blocking on it would be unresolvable. + */ + +/** Copyable kinds derived from the wire contract, so the reason split can never drift. */ +const COPYABLE_BLOCKER_KINDS: ReadonlySet = new Set(forkCopyableKindSchema.options) + +/** + * The blocker reason for a would-clear entry, or null when the entry does not block + * (`dependent` cause, and - defensively - any kind the cleared-ref collector excludes): + * - `workflow` cause -> `workflow-missing` (deploy the referenced workflow in the source, or + * remove the reference). + * - `reference` + source deleted -> `source-deleted` (map the dead id to a live target + * resource, or fix/archive the source workflow). + * - `reference` + external MCP server -> `unmapped-mcp-server` (map it; MCP servers are never + * copied). + * - `reference` + copyable kind -> `unmapped-copyable` (map it or select it for copy). + */ +export function forkSyncBlockerReasonFor(ref: ForkClearedRef): ForkSyncBlockerReason | null { + if (ref.cause === 'workflow') return 'workflow-missing' + if (ref.cause !== 'reference') return null + if (ref.sourceDeleted) return 'source-deleted' + if (ref.kind === 'mcp-server') return 'unmapped-mcp-server' + if (COPYABLE_BLOCKER_KINDS.has(ref.kind)) return 'unmapped-copyable' + // Credential / env-var / knowledge-document never reach the cleared list (excluded by the + // collector; the first two gate via the kind-level required gate, documents follow their KB). + return null +} + +/** The would-clear entries that BLOCK the sync, paired with their reason. */ +export function selectForkSyncBlockingRefs( + clearedRefs: ForkClearedRef[] +): Array<{ ref: ForkClearedRef; reason: ForkSyncBlockerReason }> { + return clearedRefs.flatMap((ref) => { + const reason = forkSyncBlockerReasonFor(ref) + return reason ? [{ ref, reason }] : [] + }) +} + +/** Map blocking entries to the wire {@link ForkSyncBlocker} shape of the promote gate error. */ +export function toForkSyncBlockers( + blocking: Array<{ ref: ForkClearedRef; reason: ForkSyncBlockerReason }> +): ForkSyncBlocker[] { + return blocking.map(({ ref, reason }) => ({ + workflowName: ref.workflowName, + blockLabel: ref.blockLabel, + fieldLabel: ref.fieldLabel, + kind: ref.kind, + sourceId: ref.sourceId, + sourceLabel: ref.sourceLabel, + reason, + })) +} diff --git a/apps/sim/lib/workspaces/fork/remap/remap-references.test.ts b/apps/sim/lib/workspaces/fork/remap/remap-references.test.ts index 849273501aa..d549381d74a 100644 --- a/apps/sim/lib/workspaces/fork/remap/remap-references.test.ts +++ b/apps/sim/lib/workspaces/fork/remap/remap-references.test.ts @@ -25,6 +25,7 @@ import { applyDependentOverrides, clearDependentsOnRemap, collectClearedDependents, + createForkSubBlockTransform, parseNestedDependentKey, readTargetDraftDependentValue, remapForkSubBlocks, @@ -413,6 +414,263 @@ describe('createForkBootstrapTransform document-selector remap', () => { }) }) +describe('MCP block server remap follows the tool selection (optimistic verbatim)', () => { + // Shape of the real MCP block: tool depends on server, arguments depend on tool. + const mcpBlock = () => + blockWith([ + { id: 'server', title: 'MCP Server', type: 'mcp-server-selector', required: true }, + { + id: 'tool', + title: 'Tool', + type: 'mcp-tool-selector', + required: true, + dependsOn: ['server'], + }, + { id: 'arguments', title: '', type: 'mcp-dynamic-args', dependsOn: ['tool'] }, + ]) + const mcpSubBlocks = (): SubBlockRecord => ({ + server: { id: 'server', type: 'mcp-server-selector', value: 'mcp-src1' }, + tool: { id: 'tool', type: 'mcp-tool-selector', value: 'mcp-src1-search_docs' }, + arguments: { id: 'arguments', type: 'mcp-dynamic-args', value: '{"query":"hello"}' }, + }) + const mapServer = (kind: string, id: string) => + kind === 'mcp-server' && id === 'mcp-src1' ? 'mcp-tgt9' : null + + it('sync transform: keeps the tool (embedded server id swapped, name verbatim) and its arguments', () => { + // The same transform serves BOTH create- and replace-mode sync targets, so a freshly + // created target deploys with the tool intact instead of an empty required field. + vi.mocked(getBlock).mockReturnValue(mcpBlock()) + const transform = createForkSubBlockTransform(mapServer) + const result = transform(mcpSubBlocks(), 'mcp') + expect(result.server.value).toBe('mcp-tgt9') + expect(result.tool.value).toBe('mcp-tgt9-search_docs') + expect(result.arguments.value).toBe('{"query":"hello"}') + }) + + it('keeps a bare tool name (no embedded server id) verbatim under the remapped server', () => { + vi.mocked(getBlock).mockReturnValue(mcpBlock()) + const subBlocks = mcpSubBlocks() + subBlocks.tool = { id: 'tool', type: 'mcp-tool-selector', value: 'search_docs' } + const transform = createForkSubBlockTransform(mapServer) + const result = transform(subBlocks, 'mcp') + expect(result.server.value).toBe('mcp-tgt9') + expect(result.tool.value).toBe('search_docs') + expect(result.arguments.value).toBe('{"query":"hello"}') + }) + + it('sync transform: an UNMAPPED server is cleared and still clears tool + arguments (defense-in-depth)', () => { + // The zero-cleared-refs gate blocks a sync before this state can persist; the remap's + // clear-unresolved backstop must still never leave a tool under a cleared server. + vi.mocked(getBlock).mockReturnValue(mcpBlock()) + const transform = createForkSubBlockTransform(() => null) + const result = transform(mcpSubBlocks(), 'mcp') + expect(result.server.value).toBe('') + expect(result.tool.value).toBe('') + expect(result.arguments.value).toBe('') + }) + + it('fork-create: servers are not copied, so the reference clears and dependents clear with it', () => { + vi.mocked(getBlock).mockReturnValue(mcpBlock()) + const transform = createForkBootstrapTransform(() => null) + const result = transform(mcpSubBlocks(), 'mcp') + expect(result.server.value).toBe('') + expect(result.tool.value).toBe('') + expect(result.arguments.value).toBe('') + }) + + it('remap layer: the tool follow-rewrite is not registered as a remapped parent key', () => { + // Only `server` may drive dependent clears; the followed tool must not (its own + // dependent - arguments - is preserved with it). + const result = remapForkSubBlocks(mcpSubBlocks(), mapServer, 'promote') + expect(result.subBlocks.tool.value).toBe('mcp-tgt9-search_docs') + expect(result.remappedKeys).toEqual(new Set(['server'])) + }) + + it('clearDependentsOnRemap: exemption applies ONLY to the mcp tool selector, not other kinds', () => { + // A knowledge-base parent remapped to a non-empty target still clears its + // document-selector dependent (regression guard for the mcp-only exemption). + vi.mocked(getBlock).mockReturnValue( + blockWith([ + { id: 'knowledgeBaseId', title: 'KB', type: 'knowledge-base-selector' }, + { + id: 'documentId', + title: 'Doc', + type: 'document-selector', + dependsOn: ['knowledgeBaseId'], + }, + ]) + ) + const result = clearDependentsOnRemap( + { + knowledgeBaseId: { + id: 'knowledgeBaseId', + type: 'knowledge-base-selector', + value: 'kb-dst', + }, + documentId: { id: 'documentId', type: 'document-selector', value: 'doc-src' }, + }, + 'knowledge', + new Set(['knowledgeBaseId']) + ) + expect(result.documentId.value).toBe('') + }) + + it('clearDependentsOnRemap: preserve holds when a SECOND remapped key also reaches the tool selector', () => { + // Synthetic config (no registry block wires this today): the tool selector hangs off BOTH a + // remapped mcp-server parent (preserve) and another remapped parent (no preserve). The + // selector-keyed preserve must win over the other key's clear, in either key order, while + // the other key's own non-exempt dependent still clears. + vi.mocked(getBlock).mockReturnValue( + blockWith([ + { id: 'server', title: 'MCP Server', type: 'mcp-server-selector' }, + { id: 'knowledgeBaseId', title: 'KB', type: 'knowledge-base-selector' }, + { + id: 'tool', + title: 'Tool', + type: 'mcp-tool-selector', + dependsOn: ['server', 'knowledgeBaseId'], + }, + { id: 'arguments', title: '', type: 'mcp-dynamic-args', dependsOn: ['tool'] }, + { + id: 'documentId', + title: 'Doc', + type: 'document-selector', + dependsOn: ['knowledgeBaseId'], + }, + ]) + ) + const subBlocks = (): SubBlockRecord => ({ + server: { id: 'server', type: 'mcp-server-selector', value: 'mcp-tgt9' }, + knowledgeBaseId: { id: 'knowledgeBaseId', type: 'knowledge-base-selector', value: 'kb-dst' }, + tool: { id: 'tool', type: 'mcp-tool-selector', value: 'mcp-tgt9-search_docs' }, + arguments: { id: 'arguments', type: 'mcp-dynamic-args', value: '{"query":"hello"}' }, + documentId: { id: 'documentId', type: 'document-selector', value: 'doc-src' }, + }) + for (const keys of [ + ['server', 'knowledgeBaseId'], + ['knowledgeBaseId', 'server'], + ]) { + const result = clearDependentsOnRemap(subBlocks(), 'mcp', new Set(keys)) + expect(result.tool.value).toBe('mcp-tgt9-search_docs') + expect(result.arguments.value).toBe('{"query":"hello"}') + expect(result.documentId.value).toBe('') + } + }) + + it('clearDependentsOnRemap: a CLEARED server alongside another remapped key still clears the tool', () => { + // Same two-key config, but the server was cleared (unmapped): no preserve applies anywhere, + // so the tool and its arguments clear as ordinary dependents. + vi.mocked(getBlock).mockReturnValue( + blockWith([ + { id: 'server', title: 'MCP Server', type: 'mcp-server-selector' }, + { id: 'knowledgeBaseId', title: 'KB', type: 'knowledge-base-selector' }, + { + id: 'tool', + title: 'Tool', + type: 'mcp-tool-selector', + dependsOn: ['server', 'knowledgeBaseId'], + }, + { id: 'arguments', title: '', type: 'mcp-dynamic-args', dependsOn: ['tool'] }, + ]) + ) + const result = clearDependentsOnRemap( + { + server: { id: 'server', type: 'mcp-server-selector', value: '' }, + knowledgeBaseId: { + id: 'knowledgeBaseId', + type: 'knowledge-base-selector', + value: 'kb-dst', + }, + tool: { id: 'tool', type: 'mcp-tool-selector', value: 'mcp-src1-search_docs' }, + arguments: { id: 'arguments', type: 'mcp-dynamic-args', value: '{"query":"hello"}' }, + }, + 'mcp', + new Set(['server', 'knowledgeBaseId']) + ) + expect(result.tool.value).toBe('') + expect(result.arguments.value).toBe('') + }) +}) + +describe('tool-input MCP entry server remap rewrites embedded server metadata', () => { + const toolInputSubBlocks = (params: Record): SubBlockRecord => ({ + tools: { + id: 'tools', + type: 'tool-input', + value: [{ type: 'mcp', title: 'search', toolId: 'mcp-src1-search', params }], + }, + }) + const entryParams = () => ({ + serverId: 'mcp-src1', + serverUrl: 'https://old.example/mcp', + toolName: 'search', + serverName: 'Old Server', + }) + const mapServer = (kind: string, id: string) => + kind === 'mcp-server' && id === 'mcp-src1' ? 'mcp-tgt9' : null + + it('rewrites serverUrl/serverName from the mapped TARGET row; tool name verbatim, toolId rebuilt', () => { + const result = remapForkSubBlocks(toolInputSubBlocks(entryParams()), mapServer, 'promote', { + resolveMcpServerMeta: (targetServerId) => + targetServerId === 'mcp-tgt9' + ? { name: 'New Server', url: 'https://new.example/mcp' } + : undefined, + }) + const [tool] = result.subBlocks.tools.value as Array<{ + toolId: string + params: Record + }> + expect(tool.params).toEqual({ + serverId: 'mcp-tgt9', + serverUrl: 'https://new.example/mcp', + toolName: 'search', + serverName: 'New Server', + }) + expect(tool.toolId).toBe('mcp-tgt9-search') + }) + + it('drops the stale serverUrl when the target server has no url', () => { + const result = remapForkSubBlocks(toolInputSubBlocks(entryParams()), mapServer, 'promote', { + resolveMcpServerMeta: () => ({ name: 'New Server', url: null }), + }) + const [tool] = result.subBlocks.tools.value as Array<{ params: Record }> + expect(tool.params).toEqual({ + serverId: 'mcp-tgt9', + toolName: 'search', + serverName: 'New Server', + }) + }) + + it('without a meta resolver (scan-only callers) the id remaps and metadata is left as-is', () => { + const result = remapForkSubBlocks(toolInputSubBlocks(entryParams()), mapServer, 'promote') + const [tool] = result.subBlocks.tools.value as Array<{ + toolId: string + params: Record + }> + expect(tool.params).toEqual({ + serverId: 'mcp-tgt9', + serverUrl: 'https://old.example/mcp', + toolName: 'search', + serverName: 'Old Server', + }) + expect(tool.toolId).toBe('mcp-tgt9-search') + }) + + it('threads the meta resolver through the sync transform', () => { + // Transform-level check: promote passes the batch-loaded target rows via options. + vi.mocked(getBlock).mockReturnValue( + blockWith([{ id: 'tools', title: 'Tools', type: 'tool-input' }]) + ) + const transform = createForkSubBlockTransform(mapServer, { + resolveMcpServerMeta: () => ({ name: 'New Server', url: 'https://new.example/mcp' }), + }) + const result = transform(toolInputSubBlocks(entryParams()), 'agent') + const [tool] = result.tools.value as Array<{ params: Record }> + expect(tool.params.serverUrl).toBe('https://new.example/mcp') + expect(tool.params.serverName).toBe('New Server') + }) +}) + describe('clearDependentsOnRemap canonical-pair gating', () => { const kbCanonicalBlock = () => blockWith([ diff --git a/apps/sim/lib/workspaces/fork/remap/remap-references.ts b/apps/sim/lib/workspaces/fork/remap/remap-references.ts index dc4e9c269bf..fe645f8b9ad 100644 --- a/apps/sim/lib/workspaces/fork/remap/remap-references.ts +++ b/apps/sim/lib/workspaces/fork/remap/remap-references.ts @@ -1,5 +1,6 @@ import { createLogger } from '@sim/logger' import { getErrorMessage } from '@sim/utils/errors' +import { omit } from '@sim/utils/object' import type { SubBlockType } from '@sim/workflow-types/blocks' import type { z } from 'zod' import type { forkRemapKindSchema } from '@/lib/api/contracts/workspace-fork' @@ -33,6 +34,7 @@ import { } from '@/lib/workspaces/fork/remap/remap-files' import { getBlock } from '@/blocks/registry' import type { SubBlockConfig } from '@/blocks/types' +import { getSubBlocksDependingOnChange } from '@/blocks/utils' /** * Resource kinds the fork remapper rewrites across workspaces, derived from the @@ -82,9 +84,11 @@ export const REGISTRY_KIND_TO_FORK_KIND: Partial< // map when its referenced document was copied into the fork; an unmapped document (its // parent KB wasn't copied, or the doc wasn't copyable) resolves to null and is cleared, // and `clearDependentsOnRemap` still clears it as a `knowledgeBaseId` dependent when the -// parent KB itself is unmapped. `mcp-tool-selector` is cleared by `dependsOn` when its -// `mcp-server-selector` parent is remapped - the tool list is server-scoped and may -// differ in the target. +// parent KB itself is unmapped. `mcp-tool-selector` follows its `mcp-server-selector` +// parent's remap: mapping asserts the servers are equivalent, so the tool SELECTION is +// kept (its embedded server id swapped to the target's, the tool name verbatim - see +// {@link remapForkSubBlocks}) and `clearDependentsOnRemap` exempts it. When the server +// is CLEARED (unmapped / fork-create) the tool still clears as a dependent. /** Matches `{{ENV_KEY}}` references inside subblock values; shared with cascade detection. */ export const ENV_REF_PATTERN = /\{\{\s*([A-Za-z_][A-Za-z0-9_]*)\s*\}\}/g @@ -128,6 +132,22 @@ export type ForkReferenceResolver = ( sourceId: string ) => string | null | undefined +/** Identity metadata of a mapped TARGET MCP server row (url is null for url-less transports). */ +export interface ForkMcpServerMeta { + name: string + url: string | null +} + +/** + * Resolves a mapped TARGET MCP server id to its row metadata, so a remapped tool-input + * entry's embedded `serverUrl`/`serverName` are rewritten from the target server instead + * of carrying the source server's (which would show a false "URL changed" stale badge in + * the target UI). Undefined when the row is unknown - the entry's metadata is then left + * as-is. Threaded by promote (which batch-loads the mapped targets); scan-only callers + * omit it because they never persist the remapped value. + */ +export type ForkMcpServerMetaResolver = (targetServerId: string) => ForkMcpServerMeta | undefined + export interface ForkReference { kind: ForkRemapKind sourceId: string @@ -153,6 +173,8 @@ export interface RemapForkContext { blockType?: string /** Canonical-mode overrides (`block.data.canonicalModes`), picking the active member per pair. */ canonicalModes?: CanonicalModeOverrides + /** Target MCP server row lookup for rewriting remapped tool-input entries' server metadata. */ + resolveMcpServerMeta?: ForkMcpServerMetaResolver } function remapEnvInValue( @@ -349,6 +371,8 @@ interface ForkToolInputOptions { /** Fork-create drops unresolved tools / clears params; promote keeps + records. */ clearUnresolved: boolean record?: (kind: ForkRemapKind, sourceId: string, mapped: boolean) => void + /** Target MCP server row lookup for rewriting a remapped MCP entry's server metadata. */ + resolveMcpServerMeta?: ForkMcpServerMetaResolver } /** @@ -394,10 +418,22 @@ function remapForkToolInputValue( changed = true const toolName = typeof tool.params.toolName === 'string' ? tool.params.toolName : undefined + let nextParams: Record = { ...tool.params, serverId: target } + // The entry embeds the server's identity metadata (`serverUrl`/`serverName`); rewrite + // it from the mapped TARGET row so the target UI never flags a false "URL changed" + // stale badge against the source server's url (a url-less target drops the stale key). + // The tool NAME stays verbatim - mapping asserts server equivalence; a name missing on + // the target degrades to the existing tool_not_found badge / runtime skip. Without a + // meta resolver (scan-only callers) the metadata is left as-is. + const meta = opts.resolveMcpServerMeta?.(target) + if (meta) { + nextParams = { ...omit(nextParams, ['serverUrl']), serverName: meta.name } + if (meta.url) nextParams.serverUrl = meta.url + } return [ { ...tool, - params: { ...tool.params, serverId: target }, + params: nextParams, toolId: toolName ? createMcpToolId(target, toolName) : tool.toolId, }, ] @@ -476,6 +512,8 @@ export function remapForkSubBlocks( const references = new Map() const unmapped = new Map() const remappedKeys = new Set() + /** MCP server ids remapped to a DIFFERENT mapped target this pass (source id -> target id). */ + const mcpServerRemaps = new Map() const recordReference = (key: string, reference: ForkReference, mapped: boolean) => { if (mode !== 'promote') return @@ -542,6 +580,7 @@ export function remapForkSubBlocks( if (!isDormant) recordReference(`${forkKind}:${ref.rawValue}`, reference, mapped) if (mapped) { if (target !== ref.rawValue) { + if (forkKind === 'mcp-server') mcpServerRemaps.set(ref.rawValue, target) const replaceResult = definition.codec.replace(value, ref.rawValue, target) if (replaceResult.success) value = replaceResult.nextValue } @@ -591,7 +630,11 @@ export function remapForkSubBlocks( ) value = subBlockType === 'tool-input' - ? remapForkToolInputValue(value, resolve, { clearUnresolved, record }) + ? remapForkToolInputValue(value, resolve, { + clearUnresolved, + record, + resolveMcpServerMeta: context?.resolveMcpServerMeta, + }) : remapForkSkillInputValue(value, resolve, { clearUnresolved, record }) } @@ -618,6 +661,32 @@ export function remapForkSubBlocks( result[subBlockKey] = { ...subBlock, value } } + // An MCP block's tool SELECTION follows its server's remap instead of clearing: the stored + // value embeds the server id (`mcp--`), so swap the embedded id for the + // mapped target's and keep the tool NAME verbatim - mapping asserts the servers are + // equivalent, mirroring how tool-input MCP entries keep their tool name. A value that does + // not embed a remapped server id (a bare tool name) is already server-agnostic and kept + // as-is. Deliberately NOT added to `remappedKeys`: the selection is preserved, so its own + // dependents (the tool's arguments) must be preserved with it, and `clearDependentsOnRemap` + // exempts the selector under a remapped (non-cleared) server parent. + if (mcpServerRemaps.size > 0) { + for (const [subBlockKey, subBlock] of Object.entries(result)) { + if (!subBlock || typeof subBlock !== 'object') continue + if (subBlock.type !== 'mcp-tool-selector') continue + const toolValue = subBlock.value + if (typeof toolValue !== 'string' || !toolValue) continue + for (const [sourceServerId, targetServerId] of mcpServerRemaps) { + const sourcePrefix = createMcpToolId(sourceServerId, '') + if (!toolValue.startsWith(sourcePrefix)) continue + result[subBlockKey] = { + ...subBlock, + value: createMcpToolId(targetServerId, toolValue.slice(sourcePrefix.length)), + } + break + } + } + } + return { subBlocks: result, references: Array.from(references.values()), @@ -629,11 +698,17 @@ export function remapForkSubBlocks( /** * Clear every subblock whose `dependsOn` parent was remapped to a different * target this pass, so a child scoped to the old parent (a KB's document, a - * Slack channel, a sheet tab) never carries a stale id into the target. Reuses - * the search-replace dependent-clear walk (canonical-pair aware, transitive over - * `dependsOn` chains) so fork/promote and in-editor search-replace clear - * identically. Children of an unchanged parent are preserved; a no-op for - * unknown block types or when nothing was remapped. + * Slack channel, a sheet tab) never carries a stale id into the target. Uses + * the same dependent walk as search-replace (canonical-pair aware, transitive + * over `dependsOn` chains) so fork/promote and in-editor search-replace clear + * identically - with ONE remap-specific exemption: an `mcp-tool-selector` under + * an `mcp-server-selector` parent that was REMAPPED to a mapped target (its + * post-remap value is non-empty) is preserved along with its own dependents + * (the tool's arguments), because mapping asserts the servers are equivalent + * and {@link remapForkSubBlocks} already followed the selection onto the target + * server. A CLEARED server (unmapped / fork-create) still clears its dependents. + * Children of an unchanged parent are preserved; a no-op for unknown block + * types or when nothing was remapped. */ export function clearDependentsOnRemap( subBlocks: SubBlockRecord, @@ -661,11 +736,50 @@ export function clearDependentsOnRemap( return (mode === 'advanced') !== group.advancedIds.includes(baseKey) } + // The exemption's parent test: an mcp-server selector whose POST-remap value is non-empty was + // remapped to a mapped target (a cleared one is empty), so its tool selection is preserved. + const configTypeById = new Map( + config.subBlocks.filter((cfg) => cfg.id).map((cfg) => [cfg.id, cfg.type]) + ) + const isRemappedMcpServerParent = (key: string): boolean => { + if (configTypeById.get(key.replace(/_\d+$/, '')) !== 'mcp-server-selector') return false + const parent = subBlocks[key] + return parent && typeof parent === 'object' ? isNonEmptyValue(parent.value) : false + } + + // The preserve decision is hoisted out of the per-key walk and keyed on the SELECTOR (not on + // which remapped key reaches it): `toClear` is a union across per-key BFS passes (each with its + // own `visited`), so an in-loop exemption holds only against the exempting key - a second + // remapped key (or a longer dependsOn path) reaching the same tool selector would re-add it. + // Unreachable with today's registry (the tool selector's only dependsOn parent is its server), + // but this makes the exemption independent of key order and path by construction. + const preservedMcpToolSelectors = new Set() + for (const key of remappedKeys) { + if (isDormantCanonicalMember(key) || !isRemappedMcpServerParent(key)) continue + for (const dependent of getSubBlocksDependingOnChange(config.subBlocks, key)) { + if (dependent.id && dependent.type === 'mcp-tool-selector') { + preservedMcpToolSelectors.add(dependent.id) + } + } + } + + // Same BFS as `getWorkflowSearchDependentClears`, with the preserved tool selector's subtree + // pruned (skipping it keeps its own dependents - the arguments - out of the clear set too). const toClear = new Set() for (const key of remappedKeys) { if (isDormantCanonicalMember(key)) continue - for (const clear of getWorkflowSearchDependentClears(config.subBlocks, key)) { - if (!remappedKeys.has(clear.subBlockId)) toClear.add(clear.subBlockId) + const visited = new Set([key]) + const queue = [key] + while (queue.length > 0) { + const current = queue.shift() + if (!current) continue + for (const dependent of getSubBlocksDependingOnChange(config.subBlocks, current)) { + if (!dependent.id || visited.has(dependent.id)) continue + if (preservedMcpToolSelectors.has(dependent.id)) continue + visited.add(dependent.id) + if (!remappedKeys.has(dependent.id)) toClear.add(dependent.id) + queue.push(dependent.id) + } } } @@ -975,14 +1089,20 @@ export function remapSubBlocks( /** A `copyWorkflowStateIntoTarget` subBlock transform that rewrites references via the resolver. */ export function createForkSubBlockTransform( - resolve: ForkReferenceResolver + resolve: ForkReferenceResolver, + options?: { + /** Mapped-target MCP server rows, so remapped tool-input entries rewrite their server metadata. */ + resolveMcpServerMeta?: ForkMcpServerMetaResolver + } ): ( subBlocks: SubBlockRecord, blockType: string, canonicalModes?: CanonicalModeOverrides ) => SubBlockRecord { return (subBlocks, blockType, canonicalModes) => { - const result = remapSubBlocks(subBlocks, resolve) + const result = remapSubBlocks(subBlocks, resolve, { + resolveMcpServerMeta: options?.resolveMcpServerMeta, + }) return clearDependentsOnRemap(result.subBlocks, blockType, result.remappedKeys, canonicalModes) } } diff --git a/apps/sim/lib/workspaces/fork/remap/remap-table-groups.test.ts b/apps/sim/lib/workspaces/fork/remap/remap-table-groups.test.ts index e05b8b00971..c49ee410b03 100644 --- a/apps/sim/lib/workspaces/fork/remap/remap-table-groups.test.ts +++ b/apps/sim/lib/workspaces/fork/remap/remap-table-groups.test.ts @@ -3,7 +3,10 @@ */ import { describe, expect, it } from 'vitest' import type { TableSchema } from '@/lib/table/types' -import { deriveForkBlockId } from '@/lib/workspaces/fork/remap/block-identity' +import { + buildForkBlockIdResolver, + deriveForkBlockId, +} from '@/lib/workspaces/fork/remap/block-identity' import { remapForkTableWorkflowGroups } from '@/lib/workspaces/fork/remap/remap-table-groups' describe('remapForkTableWorkflowGroups', () => { @@ -28,6 +31,36 @@ describe('remapForkTableWorkflowGroups', () => { expect(result.columns[0].workflowGroupId).toBe('g1') }) + // Promote threads its persisted-pair resolver: a paired block resolves to the pair's target id + // (on push, the parent's ORIGINAL id - never the derive); an unpaired block falls back to the + // derive, matching the workflow write path. + it('prefers a provided block-id resolver (persisted pair) over the derive, deriving unpaired blocks', () => { + const map = new Map([['src-wf', 'child-wf']]) + const schema: TableSchema = { + columns: [], + workflowGroups: [ + { + id: 'g1', + workflowId: 'src-wf', + outputs: [ + { blockId: 'src-block', path: 'out', columnName: 'col_1' }, + { blockId: 'src-unpaired', path: 'out2', columnName: 'col_2' }, + ], + }, + ], + } + const resolver = buildForkBlockIdResolver(true, { + parentToChild: new Map([ + ['src-block', { targetBlockId: 'original-parent-block', targetWorkflowId: 'child-wf' }], + ]), + childToParent: new Map(), + }) + const result = remapForkTableWorkflowGroups(schema, map, resolver) + const outputs = result.workflowGroups?.[0].outputs + expect(outputs?.[0].blockId).toBe('original-parent-block') + expect(outputs?.[1].blockId).toBe(deriveForkBlockId('child-wf', 'src-unpaired')) + }) + it('drops a group whose backing workflow was not copied and clears its column wiring', () => { const schema: TableSchema = { columns: [{ id: 'col_1', name: 'Out', type: 'string', workflowGroupId: 'g1' }], diff --git a/apps/sim/lib/workspaces/fork/remap/remap-table-groups.ts b/apps/sim/lib/workspaces/fork/remap/remap-table-groups.ts index 9eb00d914c3..1d278f2b2ad 100644 --- a/apps/sim/lib/workspaces/fork/remap/remap-table-groups.ts +++ b/apps/sim/lib/workspaces/fork/remap/remap-table-groups.ts @@ -1,19 +1,28 @@ import type { TableSchema } from '@/lib/table/types' -import { deriveForkBlockId } from '@/lib/workspaces/fork/remap/block-identity' +import { + deriveForkBlockId, + type ForkBlockIdResolver, +} from '@/lib/workspaces/fork/remap/block-identity' /** * Remap the workflow/block references embedded in a copied table's schema so its * workflow groups keep working in the child workspace. `workflowGroups[].workflowId` * is rewritten through the source→child workflow identity map, and each - * `outputs[].blockId` is rewritten to the deterministic forked block id (matching - * `copyWorkflowStateIntoTarget`). Manual groups whose backing workflow was not + * `outputs[].blockId` is rewritten through `resolveBlockId` - which MUST be the + * same resolver that assigns the target workflows' block ids, or the outputs + * point at nonexistent blocks. Fork-create omits it and defaults to the + * deterministic {@link deriveForkBlockId} (a fresh child has no persisted + * pairs, matching `copyWorkflowStateIntoTarget`); promote passes its + * persisted-pair resolver (a push keeps the parent's ORIGINAL block ids, which + * never equal the derive). Manual groups whose backing workflow was not * copied are dropped, and any columns wired to a dropped group have their * `workflowGroupId` cleared. Enrichment groups (empty `workflowId`) and column * ids are left untouched. */ export function remapForkTableWorkflowGroups( schema: TableSchema, - workflowIdMap: Map + workflowIdMap: Map, + resolveBlockId: ForkBlockIdResolver = deriveForkBlockId ): TableSchema { const groups = schema.workflowGroups ?? [] if (groups.length === 0) return schema @@ -33,7 +42,7 @@ export function remapForkTableWorkflowGroups( outputs: group.outputs.map((output) => ({ ...output, blockId: output.blockId - ? deriveForkBlockId(childWorkflowId, output.blockId) + ? resolveBlockId(childWorkflowId, output.blockId) : output.blockId, })), }, diff --git a/apps/sim/tools/workflow/executor.test.ts b/apps/sim/tools/workflow/executor.test.ts index f1c5ca56bd5..90b08329e1a 100644 --- a/apps/sim/tools/workflow/executor.test.ts +++ b/apps/sim/tools/workflow/executor.test.ts @@ -52,6 +52,39 @@ describe('workflowExecutorTool', () => { }) }) + it.concurrent('should declare parentWorkspaceId when the context has a workspace', () => { + const params = { + workflowId: 'test-workflow-id', + inputMapping: { name: 'Test' }, + _context: { workspaceId: 'workspace-parent' }, + } + + const result = buildBody(params) + + expect(result).toEqual({ + input: { name: 'Test' }, + triggerType: 'workflow', + useDraftState: true, + parentWorkspaceId: 'workspace-parent', + }) + }) + + it.concurrent('should omit parentWorkspaceId when the context has no workspace', () => { + const params = { + workflowId: 'test-workflow-id', + inputMapping: { name: 'Test' }, + _context: { isDeployedContext: true }, + } + + const result = buildBody(params) + + expect(result).toEqual({ + input: { name: 'Test' }, + triggerType: 'workflow', + useDraftState: false, + }) + }) + it.concurrent('should parse JSON string inputMapping (UI-provided via tool-input)', () => { const params = { workflowId: 'test-workflow-id', diff --git a/apps/sim/tools/workflow/executor.ts b/apps/sim/tools/workflow/executor.ts index 95bfa1ff52b..6abc33eed29 100644 --- a/apps/sim/tools/workflow/executor.ts +++ b/apps/sim/tools/workflow/executor.ts @@ -44,10 +44,12 @@ export const workflowExecutorTool: ToolConfig< } // Use draft state for manual runs (not deployed), deployed state for deployed runs const isDeployedContext = params._context?.isDeployedContext + const parentWorkspaceId = params._context?.workspaceId return { input: inputData, triggerType: 'workflow', useDraftState: !isDeployedContext, + ...(parentWorkspaceId ? { parentWorkspaceId } : {}), } }, }, diff --git a/scripts/check-api-validation-contracts.ts b/scripts/check-api-validation-contracts.ts index 9f5d387d0a1..4858463d4a5 100644 --- a/scripts/check-api-validation-contracts.ts +++ b/scripts/check-api-validation-contracts.ts @@ -25,7 +25,7 @@ const BOUNDARY_POLICY_BASELINE = { clientHookRawFetches: 0, clientSameOriginApiFetches: 0, doubleCasts: 8, - rawJsonReads: 21, + rawJsonReads: 8, untypedResponses: 0, annotationsMissingReason: 0, } as const