Skip to content

Commit 889d5d3

Browse files
fix(workflows): close application composition gaps
1 parent f61edde commit 889d5d3

24 files changed

Lines changed: 2314 additions & 761 deletions
Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { createMockRequest } from '@sim/testing'
5+
import { beforeEach, describe, expect, it, vi } from 'vitest'
6+
7+
const mocks = vi.hoisted(() => ({
8+
activate: vi.fn(),
9+
parseRequest: vi.fn(),
10+
read: vi.fn(),
11+
session: vi.fn(),
12+
update: vi.fn(),
13+
}))
14+
15+
vi.mock('@/lib/api/server', () => ({
16+
getValidationErrorMessage: vi.fn(),
17+
parseRequest: mocks.parseRequest,
18+
}))
19+
20+
vi.mock('@/lib/api/server/routes', () => ({
21+
InternalUnauthenticatedError: class InternalUnauthenticatedError extends Error {},
22+
internalSessionAuth: { authenticate: mocks.session },
23+
}))
24+
25+
vi.mock('@/lib/core/utils/with-route-handler', () => ({
26+
withRouteHandler: (handler: unknown) => handler,
27+
}))
28+
29+
vi.mock('@/lib/workflows/application/deployments', () => ({
30+
activateWorkflowVersion: { execute: mocks.activate },
31+
updateWorkflowVersion: { execute: mocks.update },
32+
}))
33+
34+
vi.mock('@/lib/workflows/application/read-workflow-version', () => ({
35+
readWorkflowVersion: { execute: mocks.read },
36+
}))
37+
38+
import { PATCH } from '@/app/api/workflows/[id]/deployments/[version]/route'
39+
40+
describe('workflow deployment version PATCH', () => {
41+
beforeEach(() => {
42+
vi.clearAllMocks()
43+
mocks.session.mockResolvedValue({ kind: 'session', userId: 'user-1', sessionId: 'session-1' })
44+
mocks.activate.mockResolvedValue({
45+
deployedAt: new Date('2026-01-01T00:00:00Z'),
46+
warnings: undefined,
47+
activeDeployment: null,
48+
latestDeploymentAttempt: null,
49+
name: 'Release 2',
50+
description: 'Production',
51+
})
52+
mocks.update.mockResolvedValue({ name: 'Release 2', description: 'Production' })
53+
})
54+
55+
it('sends activation and optional metadata through one application command', async () => {
56+
mocks.parseRequest.mockResolvedValue({
57+
success: true,
58+
data: {
59+
params: { id: 'workflow-1', version: 2 },
60+
body: { isActive: true, name: 'Release 2', description: 'Production' },
61+
},
62+
})
63+
64+
const response = await PATCH(
65+
createMockRequest(
66+
'PATCH',
67+
undefined,
68+
{},
69+
'http://localhost/api/workflows/workflow-1/deployments/2'
70+
),
71+
{ params: Promise.resolve({ id: 'workflow-1', version: '2' }) }
72+
)
73+
74+
expect(response.status).toBe(200)
75+
expect(await response.json()).toMatchObject({
76+
success: true,
77+
name: 'Release 2',
78+
description: 'Production',
79+
})
80+
expect(mocks.activate).toHaveBeenCalledWith(
81+
expect.objectContaining({
82+
input: expect.objectContaining({
83+
workflowId: 'workflow-1',
84+
version: 2,
85+
name: 'Release 2',
86+
description: 'Production',
87+
}),
88+
})
89+
)
90+
expect(mocks.update).not.toHaveBeenCalled()
91+
})
92+
93+
it('keeps metadata-only edits on the existing update-version operation', async () => {
94+
mocks.parseRequest.mockResolvedValue({
95+
success: true,
96+
data: {
97+
params: { id: 'workflow-1', version: 2 },
98+
body: { isActive: false, name: 'Release 2' },
99+
},
100+
})
101+
102+
const response = await PATCH(
103+
createMockRequest(
104+
'PATCH',
105+
undefined,
106+
{},
107+
'http://localhost/api/workflows/workflow-1/deployments/2'
108+
),
109+
{ params: Promise.resolve({ id: 'workflow-1', version: '2' }) }
110+
)
111+
112+
expect(response.status).toBe(200)
113+
expect(mocks.update).toHaveBeenCalledOnce()
114+
expect(mocks.activate).not.toHaveBeenCalled()
115+
})
116+
})

apps/sim/app/api/workflows/[id]/deployments/[version]/route.ts

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -89,25 +89,13 @@ export const PATCH = withRouteHandler(
8989
transition: 'activate',
9090
requestId,
9191
analytics: 'human',
92+
name,
93+
description,
9294
},
9395
request,
9496
})
9597

96-
let updatedName: string | null | undefined
97-
let updatedDescription: string | null | undefined
9898
if (name !== undefined || description !== undefined) {
99-
const updated = await updateWorkflowVersion.execute({
100-
principal,
101-
input: {
102-
workflowId: id,
103-
version: versionNum,
104-
name,
105-
description,
106-
},
107-
request,
108-
})
109-
updatedName = updated.name
110-
updatedDescription = updated.description
11199
logger.info(
112100
`[${requestId}] Updated deployment version ${version} metadata during activation`,
113101
{ name, description }
@@ -120,8 +108,8 @@ export const PATCH = withRouteHandler(
120108
warnings: activateResult.warnings,
121109
activeDeployment: activateResult.activeDeployment ?? null,
122110
latestDeploymentAttempt: activateResult.latestDeploymentAttempt ?? null,
123-
...(updatedName !== undefined && { name: updatedName }),
124-
...(updatedDescription !== undefined && { description: updatedDescription }),
111+
...(name !== undefined && { name: activateResult.name ?? null }),
112+
...(description !== undefined && { description: activateResult.description ?? null }),
125113
})
126114
}
127115

apps/sim/lib/copilot/tools/handlers/deployment/deploy.test.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
* @vitest-environment node
33
*/
44
import { dbChainMockFns, resetDbChainMock } from '@sim/testing'
5+
import { getErrorMessage } from '@sim/utils/errors'
56
import { beforeEach, describe, expect, it, vi } from 'vitest'
67

78
const {
@@ -25,7 +26,7 @@ const {
2526
vi.mock('@/lib/copilot/application/execute-workflow-use-case', () => ({
2627
executeCopilotWorkflowUseCase: mockExecuteCopilotWorkflowUseCase,
2728
messageForCopilotWorkflowError: (error: unknown, fallback: string) =>
28-
error instanceof Error ? error.message : fallback,
29+
getErrorMessage(error, fallback),
2930
}))
3031

3132
vi.mock('@/lib/workflows/orchestration', () => ({

apps/sim/lib/copilot/tools/handlers/deployment/manage.test.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
workflowsOrchestrationMock,
99
workflowsOrchestrationMockFns,
1010
} from '@sim/testing'
11+
import { getErrorMessage } from '@sim/utils/errors'
1112
import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest'
1213
import type { ExecutionContext } from '@/lib/copilot/request/types'
1314

@@ -21,7 +22,7 @@ const { ensureWorkflowAccessMock, checkNeedsRedeploymentMock, mockExecuteCopilot
2122
vi.mock('@/lib/copilot/application/execute-workflow-use-case', () => ({
2223
executeCopilotWorkflowUseCase: mockExecuteCopilotWorkflowUseCase,
2324
messageForCopilotWorkflowError: (error: unknown, fallback: string) =>
24-
error instanceof Error ? error.message : fallback,
25+
getErrorMessage(error, fallback),
2526
}))
2627

2728
const performRevertToVersionMock = workflowsOrchestrationMockFns.mockPerformRevertToVersion

0 commit comments

Comments
 (0)