Skip to content

Commit 0f55477

Browse files
committed
fix(forks): offer "View changes" only on workflows the sync changes
- Sync details load each replaced target's draft and diff it against the projected source, using the same projection and "has changes" rule as the per-workflow preview, so a row and its preview never disagree. Rows the sync would not change read "No changes" and offer no comparison. - Preview buttons wait until the list matches the selected direction. - Mask JSON-encoded secrets in text and scalar fields, and say "A masked value changed" when masking hides the only difference. - Show role changes on agent messages, highlight a router's changed Context on the canvas, and keep ghost edges under live ones into containers. - Added or removed loops and parallels list their iteration settings. - Size ghost containers the way the preview draws them. - "Order changed" only when items moved; CRLF and CR read as line breaks; collapsing unchanged lines closes every fold; selecting a nested block opens its container card; the version pickers have distinct names. - Drop redundant mock resets flagged by check:test-patterns.
1 parent 43feafc commit 0f55477

27 files changed

Lines changed: 672 additions & 206 deletions

File tree

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,6 @@ function request(query: Record<string, string>) {
6262

6363
describe('fork workflow-diff route', () => {
6464
beforeEach(() => {
65-
vi.clearAllMocks()
6665
mockGetSession.mockResolvedValue({ user: { id: 'user-1' }, session: { id: 'session-1' } })
6766
mockAuthorizeWorkspaceOperation.mockResolvedValue(undefined)
6867
workspaceForkingLineageMockFns.mockResolveForkEdge.mockResolvedValue({

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,12 @@ export type CompareSide = { kind: 'draft' } | { kind: 'version'; version: number
2323

2424
const DRAFT_OPTION_VALUE = 'draft'
2525

26+
/** The visible label of the selected option, so each picker's accessible name says what it holds. */
27+
function optionLabel(options: ChipDropdownOption[], value: string): string {
28+
const label = options.find((option) => option.value === value)?.label
29+
return typeof label === 'string' ? label : ''
30+
}
31+
2632
function sideToValue(side: CompareSide): string {
2733
return side.kind === 'draft' ? DRAFT_OPTION_VALUE : String(side.version)
2834
}
@@ -113,13 +119,15 @@ export function CompareVersionsModal({
113119
value={sideToValue(base)}
114120
onChange={(value) => setBase(valueToSide(value))}
115121
align='start'
122+
aria-label={`Compare from ${optionLabel(options, sideToValue(base))}`}
116123
/>
117124
<ArrowRight className='size-[12px] shrink-0 text-[var(--text-icon)]' />
118125
<ChipDropdown
119126
options={options}
120127
value={sideToValue(target)}
121128
onChange={(value) => setTarget(valueToSide(value))}
122129
align='start'
130+
aria-label={`Compare to ${optionLabel(options, sideToValue(target))}`}
123131
/>
124132
</div>
125133
</ChipModalHeader>

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -632,6 +632,7 @@ function WorkflowPreviewBlockInner({ data }: NodeProps<WorkflowPreviewBlockNode>
632632
value={lightweight ? undefined : getDisplayValue(rawValues.context)}
633633
workflowMap={workflowMap}
634634
workflowLabelsReady={workflowLabelsReady}
635+
changed={changedFieldSet.has('context')}
635636
/>
636637
{routerRows.map((route, index) => (
637638
<SubBlockRow

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

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -585,7 +585,17 @@ export function PreviewWorkflow({
585585
* whose handle matches no mounted handle, so without this the preview
586586
* renders the cards with no lines between them.
587587
*/
588-
return normalizeWorkflowEdgeHandles(workflowState.edges).map((edge) => {
588+
/*
589+
* Ghosts go first: edges sharing a z-index paint in array order, and a
590+
* Loop/Parallel target gives its live and removed edges the same one, so
591+
* order is what keeps a ghost under the live line into that container.
592+
*/
593+
const ordered = normalizeWorkflowEdgeHandles(workflowState.edges)
594+
const edgesInPaintOrder = [
595+
...ordered.filter((edge) => edgeDiffStatus?.[edge.id] === 'removed'),
596+
...ordered.filter((edge) => edgeDiffStatus?.[edge.id] !== 'removed'),
597+
]
598+
return edgesInPaintOrder.map((edge) => {
589599
const status = getEdgeExecutionStatus(edge)
590600
const isErrorEdge = edge.sourceHandle === 'error'
591601
const isGhost = edgeDiffStatus?.[edge.id] === 'removed'

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

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ import {
2929
splitEnvironmentBindings,
3030
} from '@/app/workspace/[workspaceId]/w/components/workflow-diff/utils'
3131
import { BlockTile } from '@/blocks/block-tile'
32-
import type { BlockState } from '@/stores/workflows/workflow/types'
32+
import type { BlockState, WorkflowState } from '@/stores/workflows/workflow/types'
3333

3434
const STATUS_BADGE_VARIANT: Record<BlockDiffStatus, 'green' | 'amber' | 'red'> = {
3535
added: 'green',
@@ -42,6 +42,11 @@ interface ChangeListProps {
4242
/** The two sides, so cards can show an added or removed block's fields and detect moves */
4343
baseBlocks: Record<string, BlockState>
4444
targetBlocks: Record<string, BlockState>
45+
/** Each side's loop and parallel configs, so an added or removed container shows what it runs */
46+
containers: {
47+
base: Pick<WorkflowState, 'loops' | 'parallels'>
48+
target: Pick<WorkflowState, 'loops' | 'parallels'>
49+
}
4550
selectedBlockId: string | null
4651
onSelectBlock: Dispatch<SetStateAction<string | null>>
4752
/**
@@ -67,13 +72,14 @@ export function ChangeList({
6772
summary,
6873
baseBlocks,
6974
targetBlocks,
75+
containers,
7076
selectedBlockId,
7177
onSelectBlock,
7278
environmentBindings = false,
7379
}: ChangeListProps) {
7480
const entries = useMemo(
75-
() => listBlockChanges(summary, baseBlocks, targetBlocks),
76-
[summary, baseBlocks, targetBlocks]
81+
() => listBlockChanges(summary, baseBlocks, targetBlocks, containers),
82+
[summary, baseBlocks, targetBlocks, containers]
7783
)
7884
const blocks = useMemo(() => ({ ...baseBlocks, ...targetBlocks }), [baseBlocks, targetBlocks])
7985
const cardRefs = useRef<Map<string, HTMLDivElement>>(null)
@@ -224,7 +230,13 @@ const BlockCard = memo(function BlockCard({
224230
nested = false,
225231
}: BlockCardProps) {
226232
const [collapsed, setCollapsed] = useState(false)
233+
const [seenSelection, setSeenSelection] = useState(selectedBlockId)
227234
const selected = selectedBlockId === entry.id
235+
/* Selecting a block nested in this card on the canvas opens the card so its row can show. */
236+
if (seenSelection !== selectedBlockId) {
237+
setSeenSelection(selectedBlockId)
238+
if (selectedBlockId !== null && !selected) setCollapsed(false)
239+
}
228240
const block = blocks[entry.id]
229241
const setCardRef = useCallback(
230242
(node: HTMLDivElement | null) => registerCard(entry.id, node),
@@ -235,9 +247,7 @@ const BlockCard = memo(function BlockCard({
235247
const fields =
236248
entry.status === 'modified'
237249
? entry.changes
238-
: block
239-
? listOneSidedFields(block, entry.status)
240-
: []
250+
: [...entry.changes, ...(block ? listOneSidedFields(block, entry.status) : [])]
241251
return environmentBindings
242252
? splitEnvironmentBindings(entry.type, fields)
243253
: { logic: fields, bindings: [] }

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

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,20 @@ export function FieldChangeRow({ blockType, field, oldValue, newValue }: FieldCh
4444
: (ENGINE_FIELD_LABELS[field] ?? resolveFieldLabel(blockType, field))
4545
/* A field its definition never titled comes back as the raw id; humanize it. */
4646
const label = resolvedLabel === field ? formatParameterLabel(field) : resolvedLabel
47+
const textual = kind === 'text' || kind === 'json'
48+
const scalar = kind === 'scalar' || kind === 'toggle'
49+
const oldText = textual
50+
? toDiffText(oldValue)
51+
: scalar
52+
? formatScalar(blockType, field, oldValue)
53+
: ''
54+
const newText = textual
55+
? toDiffText(newValue)
56+
: scalar
57+
? formatScalar(blockType, field, newValue)
58+
: ''
59+
/* The summary saw a change the masked text cannot show, so the change is inside a secret. */
60+
const maskedOnly = (textual || (scalar && !oneSided)) && oldText === newText
4761

4862
return (
4963
<div className='flex flex-col gap-1.5'>
@@ -53,9 +67,10 @@ export function FieldChangeRow({ blockType, field, oldValue, newValue }: FieldCh
5367
{isBlankValue(oldValue) ? 'Set' : isBlankValue(newValue) ? 'Cleared' : 'Value changed'}
5468
</span>
5569
)}
56-
{(kind === 'text' || kind === 'json') && (
57-
<TextDiff oldText={toDiffText(oldValue)} newText={toDiffText(newValue)} />
70+
{maskedOnly && (
71+
<span className='text-[var(--text-secondary)] text-small'>A masked value changed</span>
5872
)}
73+
{textual && !maskedOnly && <TextDiff oldText={oldText} newText={newText} />}
5974
{kind === 'messages' && <MessagesDiff oldValue={oldValue} newValue={newValue} />}
6075
{kind === 'list' && (
6176
<KeyedListDiff
@@ -65,22 +80,17 @@ export function FieldChangeRow({ blockType, field, oldValue, newValue }: FieldCh
6580
newValue={newValue}
6681
/>
6782
)}
68-
{(kind === 'scalar' || kind === 'toggle') &&
83+
{scalar &&
84+
!maskedOnly &&
6985
(oneSided ? (
7086
<ValueChip
7187
tone={isBlankValue(oldValue) ? 'added' : 'removed'}
7288
text={formatScalar(blockType, field, isBlankValue(oldValue) ? newValue : oldValue)}
7389
/>
7490
) : wordDiff ? (
75-
<InlineDiff
76-
oldText={formatScalar(blockType, field, oldValue)}
77-
newText={formatScalar(blockType, field, newValue)}
78-
/>
91+
<InlineDiff oldText={oldText} newText={newText} />
7992
) : (
80-
<OldNewPair
81-
oldText={formatScalar(blockType, field, oldValue)}
82-
newText={formatScalar(blockType, field, newValue)}
83-
/>
93+
<OldNewPair oldText={oldText} newText={newText} />
8494
))}
8595
</div>
8696
)
@@ -112,7 +122,10 @@ function MessagesDiff({ oldValue, newValue }: MessagesDiffProps) {
112122
{slots.map((slot) => (
113123
<div key={slot.index} className='flex flex-col gap-1'>
114124
<span className='text-[var(--text-muted)] text-caption capitalize'>
115-
{slot.next?.role ?? slot.old?.role} message
125+
{slot.old && slot.next && slot.old.role !== slot.next.role
126+
? `${slot.old.role} → ${slot.next.role}`
127+
: (slot.next?.role ?? slot.old?.role)}{' '}
128+
message
116129
{!slot.old && ' (added)'}
117130
{!slot.next && ' (removed)'}
118131
</span>

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { TextDiff } from '@/app/workspace/[workspaceId]/w/components/workflow-di
99
import {
1010
describeListItems,
1111
isPositionalListField,
12+
listOrderChanged,
1213
pairListItems,
1314
toItemList,
1415
} from '@/app/workspace/[workspaceId]/w/components/workflow-diff/utils'
@@ -29,11 +30,14 @@ export function KeyedListDiff({ blockType, field, oldValue, newValue }: KeyedLis
2930
const oldItems = describeListItems(blockType, field, toItemList(oldValue) ?? [])
3031
const newItems = describeListItems(blockType, field, toItemList(newValue) ?? [])
3132
const rows = pairListItems(oldItems, newItems, isPositionalListField(blockType, field))
33+
const reordered = rows.length === 0 && listOrderChanged(oldItems, newItems)
3234

3335
return (
3436
<div className='flex flex-col gap-1.5'>
3537
{rows.length === 0 && (
36-
<span className='text-[var(--text-tertiary)] text-small'>Order changed</span>
38+
<span className='text-[var(--text-tertiary)] text-small'>
39+
{reordered ? 'Order changed' : 'Same items, stored differently'}
40+
</span>
3741
)}
3842
{rows.map((row, index) => (
3943
<div key={`${row.kind}-${row.label}-${index}`} className='flex items-start gap-2'>

‎apps/sim/app/workspace/[workspaceId]/w/components/workflow-diff/components/change-list/text-diff-lines.test.ts‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,4 +160,16 @@ describe('buildDiffRows', () => {
160160
expect(lines.map((line) => line.kind)).toEqual(['removed', 'added'])
161161
expect(lines.every((line) => line.parts === undefined)).toBe(true)
162162
})
163+
it('reads CRLF and bare CR endings as line breaks', () => {
164+
const rows = buildDiffRows('a\r\nb\rc', 'a\nb\nd')
165+
166+
expect(
167+
rows.map((row) => (row.type === 'line' ? [row.line.kind, row.line.text] : row.type))
168+
).toEqual([
169+
['context', 'a'],
170+
['context', 'b'],
171+
['removed', 'c'],
172+
['added', 'd'],
173+
])
174+
})
163175
})

‎apps/sim/app/workspace/[workspaceId]/w/components/workflow-diff/components/change-list/text-diff-lines.ts‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,10 @@ export function markWordChanges(lines: DiffLine[]): DiffLine[] {
9696
return out
9797
}
9898

99+
function normalizeLineEndings(value: string): string {
100+
return value.replace(/\r\n?/g, '\n')
101+
}
102+
99103
export function splitLines(value: string): string[] {
100104
const lines = value.split('\n')
101105
if (lines.length > 1 && lines[lines.length - 1] === '') lines.pop()
@@ -158,7 +162,10 @@ export function capOneSided(lines: DiffLine[]): DiffRow[] {
158162
* the one-sided cap, or a single summary row when the bodies are too long to
159163
* diff inline.
160164
*/
161-
export function buildDiffRows(oldText: string, newText: string): DiffRow[] {
165+
export function buildDiffRows(rawOld: string, rawNew: string): DiffRow[] {
166+
/* Stored text may carry CRLF or bare CR endings; diff them as the lines they display as. */
167+
const oldText = normalizeLineEndings(rawOld)
168+
const newText = normalizeLineEndings(rawNew)
162169
const oldLines = splitLines(oldText).length
163170
const newLines = splitLines(newText).length
164171
if (oldLines + newLines > MAX_DIFF_LINES) return [{ type: 'oversized', oldLines, newLines }]

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,11 @@ export function TextDiff({ oldText, newText }: TextDiffProps) {
7373
{foldCount > MANY_FOLDS && (
7474
<button
7575
type='button'
76-
onClick={() => setExpandedAll((value) => !value)}
76+
onClick={() => {
77+
/* One switch for every fold: collapsing must also close folds opened one by one. */
78+
setExpanded(new Set())
79+
setExpandedAll((value) => !value)
80+
}}
7781
className='flex w-full items-center justify-between border-[var(--border)] border-b bg-[var(--surface-2)] px-2 py-0.5 text-left font-sans text-[var(--text-tertiary)] text-caption transition-colors hover-hover:text-[var(--text-secondary)] focus-visible:bg-[var(--surface-4)] focus-visible:outline-none'
7882
>
7983
<span>

0 commit comments

Comments
 (0)