Skip to content

Commit 9a18b8e

Browse files
committed
fix(workflows): scope selection reset to picked versions and test outcomes, not mock calls
- Key the comparison view by the picked version pair instead of resetting on state identity, so live draft updates no longer clear the selection. - Agent message changes hidden by masking say so, like other fields. - Fork preview tests assert results, with mocks that answer only the right arguments, instead of asserting mock calls.
1 parent a1f6f07 commit 9a18b8e

6 files changed

Lines changed: 58 additions & 60 deletions

File tree

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

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,6 @@ describe('fork workflow-diff route', () => {
9292
)
9393

9494
expect(response.status).toBe(403)
95-
expect(mocks.loadSourceDeployedWorkflow).not.toHaveBeenCalled()
9695
})
9796

9897
it('rejects a request without the source workflow id', async () => {
@@ -102,7 +101,6 @@ describe('fork workflow-diff route', () => {
102101
)
103102

104103
expect(response.status).toBe(400)
105-
expect(mocks.loadSourceDeployedWorkflow).not.toHaveBeenCalled()
106104
})
107105

108106
it('rejects workspaces that are not a direct fork edge without reading state', async () => {
@@ -114,7 +112,6 @@ describe('fork workflow-diff route', () => {
114112
)
115113

116114
expect(response.status).toBe(400)
117-
expect(mocks.loadSourceDeployedWorkflow).not.toHaveBeenCalled()
118115
})
119116

120117
it('maps a workflow outside the sync plan to 404', async () => {
@@ -152,7 +149,7 @@ describe('fork workflow-diff route', () => {
152149
expect(response.status).toBe(404)
153150
})
154151

155-
it('reads the target draft and only its block pairs when the sync replaces a workflow', async () => {
152+
it('returns the target draft as before when the sync replaces a workflow', async () => {
156153
const targetState = {
157154
...emptyState,
158155
blocks: {},
@@ -164,19 +161,17 @@ describe('fork workflow-diff route', () => {
164161
mode: 'replace',
165162
sourceMeta: { name: 'Support Agent' },
166163
})
167-
mocks.loadTargetDraftState.mockResolvedValue(targetState)
164+
/* Only the replaced target in the target workspace has a draft; any other read is a 404. */
165+
mocks.loadTargetDraftState.mockImplementation(async (id: string, workspaceId: string) =>
166+
id === 'wf-tgt' && workspaceId === 'parent' ? targetState : null
167+
)
168168

169169
const response = await GET(
170170
request({ otherWorkspaceId: 'parent', direction: 'push', sourceWorkflowId: 'wf-src' }),
171171
routeContext
172172
)
173173

174174
expect(response.status).toBe(200)
175-
expect(mocks.loadTargetDraftState).toHaveBeenCalledWith('wf-tgt', 'parent')
176-
expect(mocks.loadForkBlockMap).toHaveBeenCalledWith(expect.anything(), 'child', {
177-
side: 'parent',
178-
workflowId: 'wf-tgt',
179-
})
180175
await expect(response.json()).resolves.toMatchObject({
181176
targetWorkflowId: 'wf-tgt',
182177
before: targetState,

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,12 @@ export function CompareVersionsModal({
142142
) : isLoading || !baseState || !targetState ? (
143143
<WorkflowDiffSkeleton />
144144
) : (
145-
<WorkflowDiffView baseState={baseState} targetState={targetState} />
145+
/* One view per picked pair, so selection and folds start fresh when either side changes. */
146+
<WorkflowDiffView
147+
key={`${sideToValue(base)}:${sideToValue(target)}`}
148+
baseState={baseState}
149+
targetState={targetState}
150+
/>
146151
)}
147152
</ChipModalBody>
148153
</ChipModal>

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

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -113,9 +113,17 @@ function MessagesDiff({ oldValue, newValue }: MessagesDiffProps) {
113113
index,
114114
old: oldMessages[index],
115115
next: newMessages[index],
116-
})).filter(
117-
(slot) => slot.old?.content !== slot.next?.content || slot.old?.role !== slot.next?.role
118-
)
116+
}))
117+
.filter(
118+
(slot) => slot.old?.content !== slot.next?.content || slot.old?.role !== slot.next?.role
119+
)
120+
.map((slot) => {
121+
const oldText = toDiffText(slot.old?.content)
122+
const newText = toDiffText(slot.next?.content)
123+
/* The content changed but masking hides where, as with any other field. */
124+
const maskedOnly = oldText === newText && slot.old?.content !== slot.next?.content
125+
return { ...slot, oldText, newText, maskedOnly }
126+
})
119127

120128
return (
121129
<div className='flex flex-col gap-2'>
@@ -129,10 +137,11 @@ function MessagesDiff({ oldValue, newValue }: MessagesDiffProps) {
129137
{!slot.old && ' (added)'}
130138
{!slot.next && ' (removed)'}
131139
</span>
132-
<TextDiff
133-
oldText={toDiffText(slot.old?.content)}
134-
newText={toDiffText(slot.next?.content)}
135-
/>
140+
{slot.maskedOnly ? (
141+
<span className='text-[var(--text-secondary)] text-small'>A masked value changed</span>
142+
) : (
143+
<TextDiff oldText={slot.oldText} newText={slot.newText} />
144+
)}
136145
</div>
137146
))}
138147
</div>

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

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -62,12 +62,6 @@ export function WorkflowDiffView({
6262
environmentBindings = false,
6363
}: WorkflowDiffViewProps) {
6464
const [selectedBlockId, setSelectedBlockId] = useState<string | null>(null)
65-
/* A selection belongs to one comparison; picking another version starts unselected. */
66-
const [selectionFor, setSelectionFor] = useState({ baseState, targetState })
67-
if (selectionFor.baseState !== baseState || selectionFor.targetState !== targetState) {
68-
setSelectionFor({ baseState, targetState })
69-
setSelectedBlockId(null)
70-
}
7165
const [listElement, setListElement] = useState<HTMLDivElement | null>(null)
7266
const listEdges = useScrollEdges(listElement)
7367

‎apps/sim/ee/workspace-forking/application/sync-workflow-diff.test.ts‎

Lines changed: 30 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -79,10 +79,10 @@ function planItem(overrides: Record<string, unknown> = {}) {
7979
}
8080
}
8181

82-
/** The source loader, answering for `wf-src` only, as the real one answers for sync sources only. */
82+
/** The source loader, answering for `wf-src` in the child only, as the real one answers for sync sources only. */
8383
function sourceIs(sourceState: WorkflowState | null) {
84-
mocks.loadSourceDeployedWorkflow.mockImplementation(async (_workspaceId: string, id: string) =>
85-
id === 'wf-src' && sourceState
84+
mocks.loadSourceDeployedWorkflow.mockImplementation(async (workspaceId: string, id: string) =>
85+
workspaceId === 'child' && id === 'wf-src' && sourceState
8686
? { summary: { id, name: 'Support Agent' }, state: sourceState }
8787
: null
8888
)
@@ -118,13 +118,27 @@ describe('getWorkspaceSyncWorkflowDiff', () => {
118118
allowPersonalApiKeys: false,
119119
}))
120120
workspaceForkingLineageMockFns.mockResolveForkEdge.mockResolvedValue(edge)
121-
mocks.loadForkBlockMap.mockResolvedValue({
122-
parentToChild: new Map(),
123-
childToParent: new Map([['b1', { targetBlockId: 'mapped-b1', targetWorkflowId: 'wf-tgt' }]]),
124-
})
121+
/* Pairs come back only for a read scoped to the target workflow, so a mapped id proves the scope. */
122+
mocks.loadForkBlockMap.mockImplementation(
123+
async (_db: unknown, child: string, scope?: { side: string; workflowId: string }) => ({
124+
parentToChild: new Map(),
125+
childToParent:
126+
child === 'child' && scope?.side === 'parent' && scope.workflowId === 'wf-tgt'
127+
? new Map([['b1', { targetBlockId: 'mapped-b1', targetWorkflowId: 'wf-tgt' }]])
128+
: new Map(),
129+
})
130+
)
125131
sourceIs(state({ b1: {}, b2: {} }, [{ id: 'e', source: 'b1', target: 'b2' }]))
126-
mocks.resolveForkPlanItem.mockResolvedValue(planItem())
127-
mocks.loadTargetDraftState.mockResolvedValue(state({ 'mapped-b1': {} }))
132+
/* The item only resolves for the child-to-parent orientation both directions use here. */
133+
mocks.resolveForkPlanItem.mockImplementation(
134+
async (params: { sourceWorkspaceId: string; targetWorkspaceId: string }) =>
135+
params.sourceWorkspaceId === 'child' && params.targetWorkspaceId === 'parent'
136+
? planItem()
137+
: null
138+
)
139+
mocks.loadTargetDraftState.mockImplementation(async (id: string, workspaceId: string) =>
140+
id === 'wf-tgt' && workspaceId === 'parent' ? state({ 'mapped-b1': {} }) : null
141+
)
128142
})
129143

130144
it('re-keys the source through the block map and derives ids for unmapped blocks', async () => {
@@ -143,11 +157,6 @@ describe('getWorkspaceSyncWorkflowDiff', () => {
143157
afterLabel: 'Support Agent (deployed)',
144158
})
145159
expect(result.before).toEqual(state({ 'mapped-b1': {} }))
146-
expect(mocks.loadTargetDraftState).toHaveBeenCalledWith('wf-tgt', 'parent')
147-
expect(mocks.loadForkBlockMap).toHaveBeenCalledWith(expect.anything(), 'child', {
148-
side: 'parent',
149-
workflowId: 'wf-tgt',
150-
})
151160
})
152161

153162
it('re-keys source variables and their assignments to the target ids by unique name', async () => {
@@ -196,8 +205,6 @@ describe('getWorkspaceSyncWorkflowDiff', () => {
196205
expect(result.before).toBeNull()
197206
expect(result.targetWorkflowId).toBeNull()
198207
expect(result.beforeLabel).toBe('Support Agent (current)')
199-
expect(mocks.loadTargetDraftState).not.toHaveBeenCalled()
200-
expect(mocks.loadForkBlockMap).not.toHaveBeenCalled()
201208
})
202209

203210
it('fails instead of guessing when a replaced target cannot be loaded', async () => {
@@ -210,35 +217,29 @@ describe('getWorkspaceSyncWorkflowDiff', () => {
210217
await expect(run({ sourceWorkflowId: 'foreign' })).rejects.toMatchObject({
211218
code: 'not_found',
212219
})
213-
expect(mocks.resolveForkPlanItem).not.toHaveBeenCalled()
214-
expect(mocks.loadTargetDraftState).not.toHaveBeenCalled()
215220
})
216221

217222
it('rejects a source whose target is excluded from sync, as the promote skips it', async () => {
218223
mocks.resolveForkPlanItem.mockResolvedValue(null)
219224

220225
await expect(run()).rejects.toMatchObject({ code: 'not_found' })
221-
expect(mocks.loadTargetDraftState).not.toHaveBeenCalled()
222226
})
223227

224228
it('refuses two admin workspaces that are not a direct fork edge', async () => {
225229
workspaceForkingLineageMockFns.mockResolveForkEdge.mockResolvedValue(null)
226230

227231
await expect(run()).rejects.toMatchObject({ code: 'validation' })
228-
expect(mocks.loadSourceDeployedWorkflow).not.toHaveBeenCalled()
229232
})
230233

231234
it('reads the other side as the source on a pull and pairs through the child block map', async () => {
232-
await run({ workspaceId: 'parent', otherWorkspaceId: 'child', direction: 'pull' })
233-
234-
expect(mocks.loadSourceDeployedWorkflow).toHaveBeenCalledWith('child', 'wf-src')
235-
expect(mocks.resolveForkPlanItem).toHaveBeenCalledWith(
236-
expect.objectContaining({ sourceWorkspaceId: 'child', targetWorkspaceId: 'parent' })
237-
)
238-
expect(mocks.loadForkBlockMap).toHaveBeenCalledWith(expect.anything(), 'child', {
239-
side: 'parent',
240-
workflowId: 'wf-tgt',
235+
const result = await run({
236+
workspaceId: 'parent',
237+
otherWorkspaceId: 'child',
238+
direction: 'pull',
241239
})
240+
241+
expect(result.targetWorkflowId).toBe('wf-tgt')
242+
expect(Object.keys(result.after.blocks)).toContain('mapped-b1')
242243
})
243244

244245
it('refuses when the caller is not an admin on the other side', async () => {
@@ -251,6 +252,5 @@ describe('getWorkspaceSyncWorkflowDiff', () => {
251252
)
252253

253254
await expect(run()).rejects.toMatchObject({ code: 'forbidden' })
254-
expect(mocks.loadSourceDeployedWorkflow).not.toHaveBeenCalled()
255255
})
256256
})

‎apps/sim/ee/workspace-forking/lib/promote/sync-preview.test.ts‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -118,13 +118,9 @@ describe('listUnchangedSyncSources', () => {
118118
})
119119

120120
expect([...unchanged]).toEqual(['s-same'])
121-
expect(mocks.forEachTargetDraft).toHaveBeenCalledWith(
122-
['t-same', 't-diff'],
123-
'ws-t',
124-
expect.any(Function)
125-
)
126121
})
127122

123+
/* `s-same` would be listed if any draft were read, so an empty result shows none was. */
128124
it('reads no drafts and treats everything as changed when the drafts exceed the limit', async () => {
129125
mocks.measureTargetDraftBytes.mockResolvedValue(5000)
130126

@@ -136,6 +132,5 @@ describe('listUnchangedSyncSources', () => {
136132
})
137133

138134
expect(unchanged.size).toBe(0)
139-
expect(mocks.forEachTargetDraft).not.toHaveBeenCalled()
140135
})
141136
})

0 commit comments

Comments
 (0)