Skip to content

Commit 2a3e35f

Browse files
mzxchandramzxchandra
committed
fix(workflows): bound the sync change check and tighten diff rendering
- The sync list's per-workflow change check measures the target drafts in one query first and skips itself past the fork state limit, then reads drafts a few at a time and drops each after comparing, instead of holding them all. - Mask keys with a secret word anywhere in them (secretAccessKey, aws_secret_access_key) while credential references stay readable. - Inline diffs show whitespace-only changes. - Opened folds reset when the compared bodies change. - Ghost collision boxes use the canvas's own block measurement.
1 parent 0f55477 commit 2a3e35f

13 files changed

Lines changed: 321 additions & 88 deletions

File tree

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

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
import { BLOCK_DIMENSIONS, CONTAINER_DIMENSIONS } from '@sim/workflow-renderer'
2+
import { estimateBlockDimensions } from '@/app/workspace/[workspaceId]/w/[workflowId]/utils'
3+
import type { BlockState } from '@/stores/workflows/workflow/types'
4+
5+
/**
6+
* The size a block is drawn at on the read-only canvas: stored values when
7+
* present, clamped to the minimums, else the type's estimate. Shared with the
8+
* comparison overlay so its ghost collision boxes match what is drawn.
9+
*/
10+
export function getPreviewBlockDimensions(block: BlockState): { width: number; height: number } {
11+
if (block.type === 'loop' || block.type === 'parallel') {
12+
return {
13+
width: block.data?.width
14+
? Math.max(block.data.width, CONTAINER_DIMENSIONS.MIN_WIDTH)
15+
: CONTAINER_DIMENSIONS.DEFAULT_WIDTH,
16+
height: block.data?.height
17+
? Math.max(block.data.height, CONTAINER_DIMENSIONS.MIN_HEIGHT)
18+
: CONTAINER_DIMENSIONS.DEFAULT_HEIGHT,
19+
}
20+
}
21+
22+
if (block.height) {
23+
return {
24+
width: BLOCK_DIMENSIONS.FIXED_WIDTH,
25+
height: Math.max(block.height, BLOCK_DIMENSIONS.MIN_HEIGHT),
26+
}
27+
}
28+
29+
return estimateBlockDimensions(block.type)
30+
}

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

Lines changed: 2 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ import '@xyflow/react/dist/style.css'
1717
import { cn } from '@sim/emcn'
1818
import { createLogger } from '@sim/logger'
1919
import {
20-
BLOCK_DIMENSIONS,
2120
BLOCK_Z_BASE,
2221
CANVAS_Z_INDEX_MODE,
2322
CONTAINER_CHILD_Z_BASE,
@@ -31,41 +30,16 @@ import {
3130
import { normalizeWorkflowEdgeHandles } from '@sim/workflow-types/workflow'
3231
import type { BlockDiffStatus, EdgeDiffStatus } from '@/lib/workflows/comparison'
3332
import { WorkflowEdge } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/workflow-edge/workflow-edge'
34-
import {
35-
estimateBlockDimensions,
36-
SUBFLOW_CHILD_NODE_CLASS,
37-
} from '@/app/workspace/[workspaceId]/w/[workflowId]/utils'
33+
import { SUBFLOW_CHILD_NODE_CLASS } from '@/app/workspace/[workspaceId]/w/[workflowId]/utils'
3834
import { PreviewBlock } from '@/app/workspace/[workspaceId]/w/components/preview/components/preview-workflow/components/block'
3935
import { PreviewSubflow } from '@/app/workspace/[workspaceId]/w/components/preview/components/preview-workflow/components/subflow'
36+
import { getPreviewBlockDimensions } from '@/app/workspace/[workspaceId]/w/components/preview/components/preview-workflow/preview-dimensions'
4037
import { useWorkflowMap } from '@/hooks/queries/workflows'
4138
import type { BlockState, WorkflowState } from '@/stores/workflows/workflow/types'
4239
import '@/app/workspace/[workspaceId]/w/components/preview/components/preview-workflow/preview-workflow.css'
4340

4441
const logger = createLogger('PreviewWorkflow')
4542

46-
/** Gets block dimensions, using stored values or defaults. */
47-
function getPreviewBlockDimensions(block: BlockState): { width: number; height: number } {
48-
if (block.type === 'loop' || block.type === 'parallel') {
49-
return {
50-
width: block.data?.width
51-
? Math.max(block.data.width, CONTAINER_DIMENSIONS.MIN_WIDTH)
52-
: CONTAINER_DIMENSIONS.DEFAULT_WIDTH,
53-
height: block.data?.height
54-
? Math.max(block.data.height, CONTAINER_DIMENSIONS.MIN_HEIGHT)
55-
: CONTAINER_DIMENSIONS.DEFAULT_HEIGHT,
56-
}
57-
}
58-
59-
if (block.height) {
60-
return {
61-
width: BLOCK_DIMENSIONS.FIXED_WIDTH,
62-
height: Math.max(block.height, BLOCK_DIMENSIONS.MIN_HEIGHT),
63-
}
64-
}
65-
66-
return estimateBlockDimensions(block.type)
67-
}
68-
6943
/** Calculates container dimensions from child block positions. */
7044
function calculateContainerDimensions(
7145
containerId: string,

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

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
import { useMemo, useState } from 'react'
44
import { cn } from '@sim/emcn'
5-
import { diffWords } from 'diff'
5+
import { diffWordsWithSpace } from 'diff'
66
import {
77
buildDiffRows,
88
type DiffLine,
@@ -66,6 +66,13 @@ export function TextDiff({ oldText, newText }: TextDiffProps) {
6666
const rows = useMemo(() => buildDiffRows(oldText, newText), [oldText, newText])
6767
const [expanded, setExpanded] = useState<Set<number>>(() => new Set())
6868
const [expandedAll, setExpandedAll] = useState(false)
69+
/* Fold indexes belong to one pair of bodies; a new comparison starts folded again. */
70+
const [foldsFor, setFoldsFor] = useState(rows)
71+
if (foldsFor !== rows) {
72+
setFoldsFor(rows)
73+
setExpanded(new Set())
74+
setExpandedAll(false)
75+
}
6976
const foldCount = rows.filter((row) => row.type === 'fold').length
7077

7178
return (
@@ -161,7 +168,8 @@ interface InlineDiffProps {
161168
* on a green tint.
162169
*/
163170
export function InlineDiff({ oldText, newText }: InlineDiffProps) {
164-
const parts = useMemo(() => diffWords(oldText, newText), [oldText, newText])
171+
/* Whitespace counts, so a value that differs only by a space still shows where. */
172+
const parts = useMemo(() => diffWordsWithSpace(oldText, newText), [oldText, newText])
165173
return (
166174
<span className='whitespace-pre-wrap break-words text-[var(--text-primary)] text-small'>
167175
{parts.map((part, index) => (

‎apps/sim/app/workspace/[workspaceId]/w/components/workflow-diff/utils.test.ts‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -340,6 +340,26 @@ describe('maskSecretsDeep', () => {
340340
)
341341
})
342342

343+
it('masks keys with a secret word anywhere in them and keeps credential references readable', () => {
344+
expect(
345+
maskSecretsDeep({
346+
secretValue: 'a',
347+
awsSecretAccessKey: 'b',
348+
aws_secret_access_key: 'c',
349+
passwordHash: 'd',
350+
credentialId: 'cred-1',
351+
secretaryName: 'Ada',
352+
})
353+
).toEqual({
354+
secretValue: '•••',
355+
awsSecretAccessKey: '•••',
356+
aws_secret_access_key: '•••',
357+
passwordHash: '•••',
358+
credentialId: 'cred-1',
359+
secretaryName: 'Ada',
360+
})
361+
})
362+
343363
it('masks inside a JSON-encoded string the way tool params are stored', () => {
344364
const encoded = JSON.stringify({ Authorization: 'Bearer x', Accept: 'json' })
345365
expect(maskSecretsDeep({ headers: encoded })).toEqual({

‎apps/sim/app/workspace/[workspaceId]/w/components/workflow-diff/utils.ts‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -519,10 +519,29 @@ interface OneSidedField {
519519
*/
520520
const SECRET_KEY_PATTERN =
521521
/^(auth|authorization|bearer|cookie|pwd)$|(token|secret|password|passphrase|credential|api[_-]?key|private[_-]?key|authorization)$/i
522+
/**
523+
* Words that make a key secret wherever they sit in it: `secretAccessKey`,
524+
* `aws_secret_access_key`, `passwordHash`. `credential` is not one of them, so
525+
* `credentialId` (a reference, not a secret) stays readable.
526+
*/
527+
const SECRET_WORDS = new Set(['secret', 'password', 'passphrase', 'passwd', 'pwd'])
522528
const MASKED_VALUE = '•••'
523529

530+
/** Splits `awsSecretAccessKey`, `client-secret` and `API_KEY` into lowercase words. */
531+
function keyWords(key: string): string[] {
532+
return key
533+
.replace(/([a-z0-9])([A-Z])/g, '$1 $2')
534+
.replace(/([A-Z])([A-Z][a-z])/g, '$1 $2')
535+
.split(/[^A-Za-z0-9]+/)
536+
.filter(Boolean)
537+
.map((word) => word.toLowerCase())
538+
}
539+
524540
function isSecretKey(key: string): boolean {
525-
return SECRET_KEY_PATTERN.test(key.trim())
541+
const trimmed = key.trim()
542+
return (
543+
SECRET_KEY_PATTERN.test(trimmed) || keyWords(trimmed).some((word) => SECRET_WORDS.has(word))
544+
)
526545
}
527546

528547
/**

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {
1616
omitPresentationChanges,
1717
} from '@/lib/workflows/comparison'
1818
import { PreviewWorkflow } from '@/app/workspace/[workspaceId]/w/components/preview'
19+
import { getPreviewBlockDimensions } from '@/app/workspace/[workspaceId]/w/components/preview/components/preview-workflow/preview-dimensions'
1920
import { ChangeList } from '@/app/workspace/[workspaceId]/w/components/workflow-diff/components/change-list'
2021
import type { WorkflowState } from '@/stores/workflows/workflow/types'
2122

@@ -69,7 +70,7 @@ export function WorkflowDiffView({
6970
[targetState, baseState]
7071
)
7172
const overlay = useMemo(
72-
() => buildWorkflowDiffOverlay(summary, baseState, targetState),
73+
() => buildWorkflowDiffOverlay(summary, baseState, targetState, getPreviewBlockDimensions),
7374
[summary, baseState, targetState]
7475
)
7576
const containers = useMemo(

‎apps/sim/ee/workspace-forking/application/sync-details.ts‎

Lines changed: 11 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import { loadTargetDraftSubBlocks } from '@/ee/workspace-forking/lib/copy/copy-w
1414
import {
1515
listForkExcludedDeployedWorkflows,
1616
loadSourceDeployedStates,
17-
loadTargetDraftStates,
1817
loadTargetWebhookPathsByBlock,
1918
} from '@/ee/workspace-forking/lib/copy/deploy-bridge'
2019
import { loadForkBlockMap } from '@/ee/workspace-forking/lib/mapping/block-map-store'
@@ -27,10 +26,7 @@ import {
2726
collectForkClearedRefCandidates,
2827
} from '@/ee/workspace-forking/lib/promote/cleared-refs'
2928
import { computeForkPromotePlan } from '@/ee/workspace-forking/lib/promote/promote-plan'
30-
import {
31-
projectSyncSource,
32-
syncChangesWorkflow,
33-
} from '@/ee/workspace-forking/lib/promote/sync-preview'
29+
import { listUnchangedSyncSources } from '@/ee/workspace-forking/lib/promote/sync-preview'
3430
import { buildForkTriggerPlan } from '@/ee/workspace-forking/lib/promote/trigger-urls'
3531
import { buildForkBlockIdResolver } from '@/ee/workspace-forking/lib/remap/block-identity'
3632

@@ -95,7 +91,7 @@ export const getWorkspaceSyncDetails = defineForkUseCase({
9591
sourceCandidates,
9692
sourceWorkflowRows,
9793
excludedSourceWorkflows,
98-
targetDraftStates,
94+
unchangedSourceIds,
9995
] = await Promise.all([
10096
loadForkDependentValues(db, auth.edge.childWorkspaceId, allTargetIds),
10197
loadTargetDraftSubBlocks(db, replaceTargetIds),
@@ -107,8 +103,14 @@ export const getWorkspaceSyncDetails = defineForkUseCase({
107103
.where(eq(workflow.workspaceId, auth.sourceWorkspaceId)),
108104
// Deployed-but-excluded source workflows, so the preview can show what a sync skips.
109105
listForkExcludedDeployedWorkflows(db, auth.sourceWorkspaceId),
110-
// Whole drafts, so each row can say whether the sync changes anything in it.
111-
loadTargetDraftStates(replaceTargetIds, auth.targetWorkspaceId),
106+
// Rows the sync would not change offer no comparison. Same projection and rule as the
107+
// per-workflow preview, so a row and its preview never disagree.
108+
listUnchangedSyncSources({
109+
items: plan.items,
110+
sourceStates,
111+
targetWorkspaceId: auth.targetWorkspaceId,
112+
resolveBlockId,
113+
}),
112114
])
113115
const storedByKey = new Map(
114116
storedValues.map((entry) => [
@@ -250,24 +252,6 @@ export const getWorkspaceSyncDetails = defineForkUseCase({
250252
blockName: reference.blockName,
251253
})
252254

253-
// A replaced workflow whose draft already matches what the sync writes changes nothing, so its
254-
// row offers no comparison. Same projection and rule as the per-workflow preview, so the row
255-
// and the preview never disagree. A draft that failed to load counts as changed.
256-
const changedSourceIds = new Set<string>()
257-
for (const item of plan.items) {
258-
const sourceState = sourceStates.get(item.sourceWorkflowId)
259-
const before =
260-
item.mode === 'replace' ? (targetDraftStates.get(item.targetWorkflowId) ?? null) : null
261-
const changed =
262-
!sourceState ||
263-
(item.mode === 'replace' && !before) ||
264-
syncChangesWorkflow(
265-
before,
266-
projectSyncSource(sourceState, before, item.targetWorkflowId, resolveBlockId)
267-
)
268-
if (changed) changedSourceIds.add(item.sourceWorkflowId)
269-
}
270-
271255
// Orient the mapping around the workspace the modal is open in (`id`): show the
272256
// caller's workflow name first, the sync partner's second, so renames are legible.
273257
const currentIsSource = auth.sourceWorkspaceId === id
@@ -289,7 +273,7 @@ export const getWorkspaceSyncDetails = defineForkUseCase({
289273
currentName: currentIsSource ? item.sourceMeta.name : targetName,
290274
otherName: currentIsSource ? targetName : item.sourceMeta.name,
291275
sourceWorkflowId: item.sourceWorkflowId,
292-
hasChanges: changedSourceIds.has(item.sourceWorkflowId),
276+
hasChanges: !unchangedSourceIds.has(item.sourceWorkflowId),
293277
}
294278
}),
295279
...plan.archivedTargets.map((target) => ({

‎apps/sim/ee/workspace-forking/lib/copy/deploy-bridge.ts‎

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { db, runOutsideTransactionContext } from '@sim/db'
2-
import { webhook, workflow, workflowDeploymentVersion } from '@sim/db/schema'
2+
import { webhook, workflow, workflowBlocks, workflowDeploymentVersion } from '@sim/db/schema'
33
import { createLogger } from '@sim/logger'
44
import { and, eq, exists, inArray, isNotNull, isNull, sql } from 'drizzle-orm'
55
import type { DbOrTx } from '@/lib/db/types'
@@ -391,23 +391,39 @@ export async function loadTargetDraftState(
391391
}
392392

393393
/**
394-
* {@link loadTargetDraftState} for many workflows, a few at a time. A workflow
395-
* that is not in `workspaceId` is left out of the result.
394+
* Serialized size of the blocks in these workflows' drafts, from one query, so
395+
* a caller can decide whether reading the drafts in full is affordable before
396+
* it reads any of them.
396397
*/
397-
export async function loadTargetDraftStates(
398+
export async function measureTargetDraftBytes(workflowIds: string[]): Promise<number> {
399+
if (workflowIds.length === 0) return 0
400+
const [row] = await db
401+
.select({
402+
bytes: sql<string>`coalesce(sum(octet_length(${workflowBlocks.subBlocks}::text)), 0)`,
403+
})
404+
.from(workflowBlocks)
405+
.where(inArray(workflowBlocks.workflowId, workflowIds))
406+
return Number(row?.bytes ?? 0)
407+
}
408+
409+
/**
410+
* {@link loadTargetDraftState} for many workflows, a few at a time, handing
411+
* each draft to `visit` and keeping none of them, so memory stays at one batch
412+
* however many workflows there are. A workflow not in `workspaceId` is skipped.
413+
*/
414+
export async function forEachTargetDraft(
398415
workflowIds: string[],
399-
workspaceId: string
400-
): Promise<Map<string, WorkflowState>> {
401-
const states = new Map<string, WorkflowState>()
416+
workspaceId: string,
417+
visit: (workflowId: string, draft: WorkflowState) => void
418+
): Promise<void> {
402419
for (let i = 0; i < workflowIds.length; i += READ_CONCURRENCY) {
403420
const batch = workflowIds.slice(i, i + READ_CONCURRENCY)
404421
const loaded = await Promise.all(batch.map((id) => loadTargetDraftState(id, workspaceId)))
405422
batch.forEach((id, index) => {
406-
const state = loaded[index]
407-
if (state) states.set(id, state)
423+
const draft = loaded[index]
424+
if (draft) visit(id, draft)
408425
})
409426
}
410-
return states
411427
}
412428

413429
/**

0 commit comments

Comments
 (0)