Skip to content

Commit 90b0089

Browse files
committed
fix(workflows): harden the version diff after review
Comparison: keep basic/advanced mode changes visible, include a tool's permission, server and implementation fields in list diffs, include input field defaults, read checkbox records as records, show whitespace-only edits, cap word and line diffing so a pathological prompt cannot stall the pane, slot deleted branches back at their old position, never reuse a ghost edge id, and mask secret-looking keys at any depth (including key/value table rows) with a name-based fallback when a block definition is unknown. Change list: memoized cards that only re-render when their own selection changes, nested cards for blocks inside an added or removed container, a shared sign map, muted tokens that exist, hover and focus treatment, scroll edge fades, and a shared skeleton for both hosts. Fork preview: the before side is the target draft the sync overwrites, read in one snapshot and scoped to the target workspace; variables and their assignments are re-keyed by unique name; a create reports no target id; the change rows are a discriminated union; the query key sits under the fork diff keys so a sync invalidates it; a direction switch closes the preview.
1 parent b02ec19 commit 90b0089

29 files changed

Lines changed: 745 additions & 286 deletions

File tree

‎apps/sim/app/api/workspaces/[id]/fork/workflow-diff/route.test.ts‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ import { OrchestrationError } from '@/lib/core/orchestration/types'
1919
const mocks = vi.hoisted(() => ({
2020
loadSourceDeployedStates: vi.fn(),
2121
loadTargetDraftState: vi.fn(),
22-
readDeployedState: vi.fn(),
2322
loadForkBlockMap: vi.fn(),
2423
computeForkPromotePlan: vi.fn(),
2524
}))
@@ -31,7 +30,6 @@ vi.mock('@/ee/workspace-forking/lib/lineage/lineage', () => workspaceForkingLine
3130
vi.mock('@/ee/workspace-forking/lib/copy/deploy-bridge', () => ({
3231
loadSourceDeployedStates: mocks.loadSourceDeployedStates,
3332
loadTargetDraftState: mocks.loadTargetDraftState,
34-
readDeployedState: mocks.readDeployedState,
3533
}))
3634
vi.mock('@/ee/workspace-forking/lib/mapping/block-map-store', () => ({
3735
loadForkBlockMap: mocks.loadForkBlockMap,
@@ -113,6 +111,18 @@ describe('fork workflow-diff route', () => {
113111
expect(mocks.loadSourceDeployedStates).not.toHaveBeenCalled()
114112
})
115113

114+
it('rejects workspaces that are not a direct fork edge without reading state', async () => {
115+
workspaceForkingLineageMockFns.mockResolveForkEdge.mockResolvedValue(null)
116+
117+
const response = await GET(
118+
request({ otherWorkspaceId: 'parent', direction: 'push', sourceWorkflowId: 'wf-src' }),
119+
routeContext
120+
)
121+
122+
expect(response.status).toBe(400)
123+
expect(mocks.loadSourceDeployedStates).not.toHaveBeenCalled()
124+
})
125+
116126
it('maps a workflow outside the sync plan to 404', async () => {
117127
const response = await GET(
118128
request({ otherWorkspaceId: 'parent', direction: 'push', sourceWorkflowId: 'foreign' }),
@@ -130,7 +140,7 @@ describe('fork workflow-diff route', () => {
130140

131141
expect(response.status).toBe(200)
132142
await expect(response.json()).resolves.toEqual({
133-
targetWorkflowId: 'wf-tgt',
143+
targetWorkflowId: null,
134144
before: null,
135145
after: emptyState,
136146
beforeLabel: 'Ask Biz (current)',

‎apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/components/general/components/compare-versions-modal.tsx‎

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,13 @@ import {
77
ChipModal,
88
ChipModalBody,
99
ChipModalHeader,
10-
Skeleton,
1110
} from '@sim/emcn'
11+
import { ArrowRight } from '@sim/emcn/icons'
1212
import type { WorkflowDeploymentVersionResponse } from '@/lib/workflows/persistence/utils'
1313
import { formatVersionLabel } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/components/general/format-version-label'
1414
import { useDraftWorkflowState } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/hooks/use-draft-workflow-state'
1515
import {
16-
CHANGE_LIST_WIDTH_CLASS,
16+
WorkflowDiffSkeleton,
1717
WorkflowDiffView,
1818
} from '@/app/workspace/[workspaceId]/w/components/workflow-diff'
1919
import { useDeploymentVersionState } from '@/hooks/queries/workflows'
@@ -103,7 +103,7 @@ export function CompareVersionsModal({
103103
srTitle='Compare versions'
104104
aria-describedby={descriptionId}
105105
size='full'
106-
className='h-[92vh] [&>div]:h-full'
106+
className='h-[84vh] [&>div]:h-full'
107107
>
108108
<ChipModalHeader onClose={() => onOpenChange(false)}>
109109
<div className='flex items-center gap-2'>
@@ -114,7 +114,7 @@ export function CompareVersionsModal({
114114
onChange={(value) => setBase(valueToSide(value))}
115115
align='start'
116116
/>
117-
<span className='text-[var(--text-muted)]'>→</span>
117+
<ArrowRight className='size-[12px] shrink-0 text-[var(--text-icon)]' />
118118
<ChipDropdown
119119
options={options}
120120
value={sideToValue(target)}
@@ -132,12 +132,7 @@ export function CompareVersionsModal({
132132
{loadError.message || 'Could not load one of the versions.'}
133133
</div>
134134
) : isLoading || !baseState || !targetState ? (
135-
<div className='flex h-full gap-0'>
136-
<Skeleton className='h-full flex-1 rounded-none' />
137-
<Skeleton
138-
className={`h-full ${CHANGE_LIST_WIDTH_CLASS} rounded-none border-[var(--border)] border-l`}
139-
/>
140-
</div>
135+
<WorkflowDiffSkeleton />
141136
) : (
142137
<WorkflowDiffView baseState={baseState} targetState={targetState} />
143138
)}

‎apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/components/general/general.tsx‎

Lines changed: 10 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,8 @@ import { Preview, PreviewWorkflow } from '@/app/workspace/[workspaceId]/w/compon
2222
import { useDeploymentVersionState, useRevertToVersion } from '@/hooks/queries/workflows'
2323
import { useWorkflowRegistry } from '@/stores/workflows/registry/store'
2424
import type { WorkflowState } from '@/stores/workflows/workflow/types'
25-
import { resolveComparePair } from './compare-pair'
26-
import { type CompareSide, CompareVersionsModal, Versions } from './components'
25+
import { type ComparePair, resolveComparePair } from './compare-pair'
26+
import { CompareVersionsModal, Versions } from './components'
2727
import { formatVersionLabel } from './format-version-label'
2828

2929
const logger = createLogger('GeneralDeploy')
@@ -65,21 +65,9 @@ export function GeneralDeploy({
6565
onLoadDeploymentBlocked,
6666
}: GeneralDeployProps) {
6767
const expandedPreviewDescriptionId = useId()
68-
const [comparePair, setComparePair] = useState<{ base: CompareSide; target: CompareSide } | null>(
69-
null
70-
)
68+
const [comparePair, setComparePair] = useState<ComparePair | null>(null)
7169
const [selectedVersion, setSelectedVersion] = useState<number | null>(null)
7270
const [showActiveDespiteSelection, setShowActiveDespiteSelection] = useState(false)
73-
const activeVersion = versions.find((v) => v.isActive)?.version ?? null
74-
75-
/**
76-
* Opens a comparison with the older version on the left. Comparing the live
77-
* version (or any version when nothing is live) shows it against the draft,
78-
* which is what a redeploy would ship.
79-
*/
80-
const handleCompareVersion = (version: number) => {
81-
setComparePair(resolveComparePair(version, activeVersion))
82-
}
8371
const previewMode: PreviewMode =
8472
selectedVersion !== null && !showActiveDespiteSelection ? 'selected' : 'active'
8573
const [showLoadDialog, setShowLoadDialog] = useState(false)
@@ -93,6 +81,12 @@ export function GeneralDeploy({
9381
workflowId: string
9482
version: number
9583
} | null>(null)
84+
const activeVersion = versions.find((v) => v.isActive)?.version ?? null
85+
86+
/** See resolveComparePair for which two sides a version opens against. */
87+
const handleCompareVersion = (version: number) => {
88+
setComparePair(resolveComparePair(version, activeVersion))
89+
}
9690

9791
const selectedVersionInfo = versions.find((v) => v.version === selectedVersion)
9892
const versionToPromoteInfo = versions.find((v) => v.version === versionToPromote?.version)
@@ -225,7 +219,7 @@ export function GeneralDeploy({
225219
<button
226220
type='button'
227221
onClick={() => handleCompareVersion(activeVersion)}
228-
className='text-[var(--warning)] text-caption underline-offset-2 hover-hover:underline focus-visible:underline focus-visible:outline-none'
222+
className='text-[var(--text-primary)] text-small underline-offset-2 hover-hover:underline focus-visible:underline focus-visible:outline-none'
229223
>
230224
View changes
231225
</button>

‎apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/hooks/use-draft-workflow-state.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import type { WorkflowState } from '@/stores/workflows/workflow/types'
1212
* cleanly against any of them.
1313
*
1414
* The merge walks every block on every store change, so a consumer that has
15-
* nothing to compare against yet passes `enabled: false` and gets null
15+
* nothing to compare against yet passes `false` as `enabled` and gets null
1616
* without paying for it.
1717
*/
1818
export function useDraftWorkflowState(

‎apps/sim/app/workspace/[workspaceId]/w/components/preview/components/preview-workflow/components/block/block.tsx‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -738,20 +738,20 @@ function WorkflowPreviewBlockInner({ data }: NodeProps<WorkflowPreviewBlockNode>
738738
)
739739
}
740740

741-
/**
742-
* Custom comparison function for React.memo optimization.
743-
* Uses fast-path primitive comparison before shallow comparing subBlockValues.
744-
* @param prevProps - Previous render props
745-
* @param nextProps - Next render props
746-
* @returns True if render should be skipped (props are equal)
747-
*/
748741
/** Same changed-field list, by identity first so the common unchanged case costs nothing. */
749742
function sameFields(prev: string[] | undefined, next: string[] | undefined): boolean {
750743
if (prev === next) return true
751744
if (!prev || !next || prev.length !== next.length) return false
752745
return prev.every((field, index) => field === next[index])
753746
}
754747

748+
/**
749+
* Custom comparison function for React.memo optimization.
750+
* Uses fast-path primitive comparison before shallow comparing subBlockValues.
751+
* @param prevProps - Previous render props
752+
* @param nextProps - Next render props
753+
* @returns True if render should be skipped (props are equal)
754+
*/
755755
function shouldSkipPreviewBlockRender(
756756
prevProps: NodeProps<WorkflowPreviewBlockNode>,
757757
nextProps: NodeProps<WorkflowPreviewBlockNode>

‎apps/sim/app/workspace/[workspaceId]/w/components/preview/components/preview-workflow/components/subflow/subflow.tsx‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
'use client'
22

33
import { memo } from 'react'
4+
import { cn } from '@sim/emcn'
45
import { SubflowNodeView } from '@sim/workflow-renderer'
56
import type { Node, NodeProps } from '@xyflow/react'
67
import type { BlockDiffStatus } from '@/lib/workflows/comparison'
@@ -51,12 +52,12 @@ function WorkflowPreviewSubflowInner({ data, id }: NodeProps<WorkflowPreviewSubf
5152
onSelect={() => undefined}
5253
/>
5354
)
54-
if (data.diffStatus !== 'removed') return view
55-
/* A removed container fades like a removed card; its children ghost themselves. */
55+
if (!data.diffStatus) return view
56+
/* Same label as a card; a removed container fades like a removed card, its children ghost themselves. */
5657
return (
57-
<div className='relative opacity-45'>
58-
<DiffStatusLabel status='removed' />
59-
{view}
58+
<div className='relative'>
59+
<DiffStatusLabel status={data.diffStatus} />
60+
<div className={cn(data.diffStatus === 'removed' && 'opacity-45')}>{view}</div>
6061
</div>
6162
)
6263
}

‎apps/sim/app/workspace/[workspaceId]/w/components/workflow-diff/components/change-list/binding-change-row.tsx‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
'use client'
22

3+
import { OverflowText } from '@sim/emcn'
34
import { ArrowRight } from '@sim/emcn/icons'
45
import { resolveFieldLabel } from '@/lib/workflows/comparison/resolve-values'
56
import {
@@ -28,7 +29,7 @@ export function BindingChangeRow({ blockType, field, oldValue, newValue }: Bindi
2829

2930
return (
3031
<div className='flex min-w-0 items-center gap-2 text-caption'>
31-
<span className='w-[112px] shrink-0 truncate text-[var(--text-tertiary)]'>{label}</span>
32+
<OverflowText label={label} className='w-[112px] shrink-0 text-[var(--text-tertiary)]' />
3233
{kind === 'secret' ? (
3334
<span className='text-[var(--text-muted)]'>Differs between workspaces</span>
3435
) : (

0 commit comments

Comments
 (0)