Skip to content

Commit 77c3858

Browse files
committed
fix(workflows): compare deployment states after the snapshot transaction commits
checkNeedsRedeployment ran the full workflow diff inside its REPEATABLE READ transaction, holding a pooled connection idle in transaction during CPU work. Reads (active version, normalized draft, variables, credential migrations) stay in the snapshot; the pure diff now runs after commit.
1 parent 9e4a27e commit 77c3858

2 files changed

Lines changed: 90 additions & 4 deletions

File tree

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { workflowDeploymentVersion } from '@sim/db'
5+
import { createMinimalWorkflowState } from '@sim/testing/factories/workflow.factory'
6+
import {
7+
dbChainMock,
8+
dbChainMockFns,
9+
queueTableRows,
10+
resetDbChainMock,
11+
} from '@sim/testing/mocks/database.mock'
12+
import {
13+
workflowsPersistenceUtilsMock,
14+
workflowsPersistenceUtilsMockFns,
15+
} from '@sim/testing/mocks/workflows-persistence-utils.mock'
16+
import { beforeEach, describe, expect, it, vi } from 'vitest'
17+
18+
vi.mock('@/lib/workflows/persistence/utils', () => workflowsPersistenceUtilsMock)
19+
20+
import { checkNeedsRedeployment } from '@/lib/workflows/deployment-status'
21+
22+
const WORKFLOW_ID = 'wf-1'
23+
24+
describe('checkNeedsRedeployment', () => {
25+
let transactionOpen: boolean
26+
let draftReadWhileTransactionOpen: boolean
27+
28+
beforeEach(() => {
29+
resetDbChainMock()
30+
transactionOpen = false
31+
draftReadWhileTransactionOpen = false
32+
33+
dbChainMockFns.transaction.mockImplementation(async (cb: (tx: unknown) => unknown) => {
34+
transactionOpen = true
35+
try {
36+
return await cb(dbChainMock.db)
37+
} finally {
38+
transactionOpen = false
39+
}
40+
})
41+
42+
queueTableRows(workflowDeploymentVersion, [
43+
{ id: 'version-1', state: { blocks: {} }, workspaceId: 'ws-1' },
44+
])
45+
46+
const deployed = createMinimalWorkflowState()
47+
const draft = createMinimalWorkflowState()
48+
draft.blocks.agent1.subBlocks.prompt.value = 'Edited after deploy'
49+
const draftBlocks = draft.blocks
50+
51+
workflowsPersistenceUtilsMockFns.mockMaterializeDeploymentState.mockResolvedValue({
52+
...deployed,
53+
parallels: {},
54+
variables: {},
55+
})
56+
workflowsPersistenceUtilsMockFns.mockLoadWorkflowDeploymentSnapshot.mockResolvedValue({
57+
edges: draft.edges,
58+
loops: {},
59+
parallels: {},
60+
variables: {},
61+
get blocks() {
62+
if (transactionOpen) draftReadWhileTransactionOpen = true
63+
return draftBlocks
64+
},
65+
})
66+
})
67+
68+
it('diffs the draft against the deployment only after the snapshot transaction commits', async () => {
69+
await expect(checkNeedsRedeployment(WORKFLOW_ID)).resolves.toBe(true)
70+
expect(draftReadWhileTransactionOpen).toBe(false)
71+
})
72+
73+
it('reports no redeploy when the workflow has no active deployment', async () => {
74+
resetDbChainMock()
75+
await expect(checkNeedsRedeployment(WORKFLOW_ID)).resolves.toBe(false)
76+
})
77+
})

‎apps/sim/lib/workflows/deployment-status.ts‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,15 @@ import type { WorkflowState } from '@/stores/workflows/workflow/types'
1919
* caller is what let this surface compare a raw jsonb blob against a normalized
2020
* draft, so that the server and the client answered the same question
2121
* differently for the same workflow.
22+
*
23+
* Every read — the active version row, the normalized draft, its variables and
24+
* the credential rows the block migrations consult — runs in one REPEATABLE READ
25+
* transaction so both operands come from the same snapshot. The diff does not:
26+
* it is pure CPU over values already read, and running it after commit returns
27+
* the pooled connection instead of holding it idle in transaction.
2228
*/
2329
export async function checkNeedsRedeployment(workflowId: string): Promise<boolean> {
24-
return db.transaction(async (tx) => {
30+
const snapshot = await db.transaction(async (tx) => {
2531
await tx.execute(sql`SET TRANSACTION ISOLATION LEVEL REPEATABLE READ`)
2632
/*
2733
* `workspaceId` is selected here, in this transaction, rather than left for
@@ -53,10 +59,10 @@ export async function checkNeedsRedeployment(workflowId: string): Promise<boolea
5359
.limit(1)
5460

5561
/* The inner join guarantees a workspace row; a null id means unusable data. */
56-
if (!active?.state || !active.workspaceId) return false
62+
if (!active?.state || !active.workspaceId) return null
5763

5864
const currentState = await loadWorkflowDeploymentSnapshot(workflowId, tx)
59-
if (!currentState) return false
65+
if (!currentState) return null
6066

6167
const deployedState = await materializeDeploymentState(
6268
workflowId,
@@ -65,6 +71,9 @@ export async function checkNeedsRedeployment(workflowId: string): Promise<boolea
6571
tx
6672
)
6773

68-
return hasWorkflowChanged(currentState, deployedState as WorkflowState)
74+
return { currentState, deployedState }
6975
})
76+
77+
if (!snapshot) return false
78+
return hasWorkflowChanged(snapshot.currentState, snapshot.deployedState as WorkflowState)
7079
}

0 commit comments

Comments
 (0)