Skip to content

Commit 5308deb

Browse files
committed
Fix regression
1 parent 57a91e9 commit 5308deb

4 files changed

Lines changed: 131 additions & 14 deletions

File tree

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

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import {
2020
} from '@sim/testing'
2121
import { NextRequest } from 'next/server'
2222
import { beforeEach, describe, expect, it, vi } from 'vitest'
23+
import { getWorkflowResponseDataSchema } from '@/lib/api/contracts/workflows'
2324

2425
const mockLoadWorkflowFromNormalizedTables =
2526
workflowsPersistenceUtilsMockFns.mockLoadWorkflowFromNormalizedTables
@@ -183,6 +184,55 @@ describe('Workflow By ID API Route', () => {
183184
expect(data.data.id).toBe('workflow-123')
184185
})
185186

187+
it('omits null workflow description from state metadata so response validates', async () => {
188+
const mockWorkflow = {
189+
id: 'workflow-null-description',
190+
userId: 'user-123',
191+
name: 'No Description Workflow',
192+
description: null,
193+
workspaceId: 'workspace-456',
194+
folderId: null,
195+
sortOrder: 0,
196+
color: '#3972F6',
197+
lastSynced: new Date(),
198+
createdAt: new Date(),
199+
updatedAt: new Date(),
200+
isDeployed: false,
201+
deployedAt: null,
202+
isPublicApi: false,
203+
locked: false,
204+
runCount: 0,
205+
lastRunAt: null,
206+
archivedAt: null,
207+
variables: {},
208+
}
209+
210+
mockGetSession({ user: { id: 'user-123' } })
211+
mockGetWorkflowById.mockResolvedValue(mockWorkflow)
212+
mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValue({
213+
allowed: true,
214+
status: 200,
215+
workflow: mockWorkflow,
216+
workspacePermission: 'admin',
217+
})
218+
mockLoadWorkflowFromNormalizedTables.mockResolvedValue({
219+
blocks: {},
220+
edges: [],
221+
loops: {},
222+
parallels: {},
223+
})
224+
225+
const req = new NextRequest('http://localhost:3000/api/workflows/workflow-null-description')
226+
const params = Promise.resolve({ id: 'workflow-null-description' })
227+
228+
const response = await GET(req, { params })
229+
const data = await response.json()
230+
231+
expect(response.status).toBe(200)
232+
expect(data.data.state.metadata).toEqual({ name: 'No Description Workflow' })
233+
expect(getWorkflowResponseDataSchema.safeParse(data.data).success).toBe(true)
234+
})
235+
186236
it.concurrent('should allow access when user has workspace permissions', async () => {
187237
const mockWorkflow = {
188238
id: 'workflow-123',

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

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,12 @@ export const GET = withRouteHandler(
108108
stampedVariables[variableId] = { ...variable, workflowId }
109109
}
110110
}
111+
const workflowStateMetadata = {
112+
name: responseWorkflowData.name,
113+
...(typeof responseWorkflowData.description === 'string'
114+
? { description: responseWorkflowData.description }
115+
: {}),
116+
}
111117

112118
if (snapshot.normalizedData) {
113119
const finalWorkflowData = {
@@ -120,10 +126,7 @@ export const GET = withRouteHandler(
120126
lastSaved: Date.now(),
121127
isDeployed: responseWorkflowData.isDeployed || false,
122128
deployedAt: responseWorkflowData.deployedAt,
123-
metadata: {
124-
name: responseWorkflowData.name,
125-
description: responseWorkflowData.description,
126-
},
129+
metadata: workflowStateMetadata,
127130
},
128131
variables: stampedVariables,
129132
}
@@ -145,10 +148,7 @@ export const GET = withRouteHandler(
145148
lastSaved: Date.now(),
146149
isDeployed: responseWorkflowData.isDeployed || false,
147150
deployedAt: responseWorkflowData.deployedAt,
148-
metadata: {
149-
name: responseWorkflowData.name,
150-
description: responseWorkflowData.description,
151-
},
151+
metadata: workflowStateMetadata,
152152
},
153153
variables: stampedVariables,
154154
}

apps/sim/lib/copilot/request/handlers/handlers.test.ts

Lines changed: 62 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,16 @@ const { isSimExecuted, executeTool, ensureHandlersRegistered } = vi.hoisted(() =
1212
ensureHandlersRegistered: vi.fn(),
1313
}))
1414

15-
const { upsertAsyncToolCall, markAsyncToolRunning, completeAsyncToolCall } = vi.hoisted(() => ({
16-
upsertAsyncToolCall: vi.fn(),
17-
markAsyncToolRunning: vi.fn(),
18-
completeAsyncToolCall: vi.fn(),
15+
const { upsertAsyncToolCall, markAsyncToolRunning, completeAsyncToolCall, markAsyncToolDelivered } =
16+
vi.hoisted(() => ({
17+
upsertAsyncToolCall: vi.fn(),
18+
markAsyncToolRunning: vi.fn(),
19+
completeAsyncToolCall: vi.fn(),
20+
markAsyncToolDelivered: vi.fn(),
21+
}))
22+
23+
const { waitForToolCompletion } = vi.hoisted(() => ({
24+
waitForToolCompletion: vi.fn(),
1925
}))
2026

2127
vi.mock('@/lib/copilot/tool-executor', () => ({
@@ -34,16 +40,20 @@ vi.mock('@/lib/copilot/async-runs/repository', () => ({
3440
createRunCheckpoint: vi.fn(),
3541
getAsyncToolCall: vi.fn(),
3642
markAsyncToolStatus: vi.fn(),
37-
markAsyncToolDelivered: vi.fn(),
3843
listAsyncToolCallsForRun: vi.fn(),
3944
getAsyncToolCalls: vi.fn(),
4045
claimCompletedAsyncToolCall: vi.fn(),
4146
releaseCompletedAsyncToolClaim: vi.fn(),
4247
upsertAsyncToolCall,
4348
markAsyncToolRunning,
49+
markAsyncToolDelivered,
4450
completeAsyncToolCall,
4551
}))
4652

53+
vi.mock('@/lib/copilot/request/tools/client', () => ({
54+
waitForToolCompletion,
55+
}))
56+
4757
import {
4858
MothershipStreamV1AsyncToolRecordStatus,
4959
MothershipStreamV1EventType,
@@ -68,6 +78,8 @@ describe('sse-handlers tool lifecycle', () => {
6878
upsertAsyncToolCall.mockResolvedValue(null)
6979
markAsyncToolRunning.mockResolvedValue(null)
7080
completeAsyncToolCall.mockResolvedValue(null)
81+
markAsyncToolDelivered.mockResolvedValue(null)
82+
waitForToolCompletion.mockResolvedValue(null)
7183
context = {
7284
chatId: undefined,
7385
messageId: 'msg-1',
@@ -236,6 +248,51 @@ describe('sse-handlers tool lifecycle', () => {
236248
expect(updated?.result?.output).toBe('done')
237249
})
238250

251+
it('marks background client workflow tools delivered after synthetic result emission', async () => {
252+
waitForToolCompletion.mockResolvedValueOnce({
253+
status: 'background',
254+
data: { detached: true },
255+
})
256+
const onEvent = vi.fn()
257+
258+
await sseHandlers.tool(
259+
{
260+
type: MothershipStreamV1EventType.tool,
261+
payload: {
262+
toolCallId: 'tool-background',
263+
toolName: 'run_workflow',
264+
arguments: { workflowId: 'workflow-1' },
265+
executor: MothershipStreamV1ToolExecutor.client,
266+
mode: MothershipStreamV1ToolMode.async,
267+
phase: MothershipStreamV1ToolPhase.call,
268+
},
269+
} satisfies StreamEvent,
270+
context,
271+
execContext,
272+
{ onEvent, interactive: true, timeout: 1000 }
273+
)
274+
275+
await sleep(0)
276+
await Promise.allSettled(context.pendingToolPromises.values())
277+
278+
expect(markAsyncToolDelivered).toHaveBeenCalledWith('tool-background')
279+
expect(onEvent).toHaveBeenCalledWith(
280+
expect.objectContaining({
281+
type: MothershipStreamV1EventType.tool,
282+
payload: expect.objectContaining({
283+
toolCallId: 'tool-background',
284+
phase: MothershipStreamV1ToolPhase.result,
285+
status: MothershipStreamV1ToolOutcome.skipped,
286+
success: true,
287+
output: { detached: true },
288+
}),
289+
})
290+
)
291+
expect(context.toolCalls.get('tool-background')?.status).toBe(
292+
MothershipStreamV1ToolOutcome.skipped
293+
)
294+
})
295+
239296
it('does not add hidden tool calls to content blocks', async () => {
240297
executeTool.mockResolvedValueOnce({ success: true, output: { skill: 'ok' } })
241298

apps/sim/lib/copilot/request/handlers/tool.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { createLogger } from '@sim/logger'
22
import { getErrorMessage, toError } from '@sim/utils/errors'
3-
import { upsertAsyncToolCall } from '@/lib/copilot/async-runs/repository'
3+
import { ASYNC_TOOL_CONFIRMATION_STATUS } from '@/lib/copilot/async-runs/lifecycle'
4+
import { markAsyncToolDelivered, upsertAsyncToolCall } from '@/lib/copilot/async-runs/repository'
45
import { STREAM_TIMEOUT_MS } from '@/lib/copilot/constants'
56
import {
67
MothershipStreamV1AsyncToolRecordStatus,
@@ -457,6 +458,15 @@ async function dispatchToolExecution(
457458
span.setAttribute(TraceAttr.ToolOutcome, completion.status)
458459
}
459460
handleClientCompletion(toolCall, toolCallId, completion)
461+
if (completion?.status === ASYNC_TOOL_CONFIRMATION_STATUS.background) {
462+
await markAsyncToolDelivered(toolCallId).catch((err) => {
463+
logger.warn(`Failed to mark background ${scopeLabel}tool delivered`, {
464+
toolCallId,
465+
toolName,
466+
error: toError(err).message,
467+
})
468+
})
469+
}
460470
await emitSyntheticToolResult(toolCallId, toolCall.name, completion, options)
461471
return (
462472
completion ?? {

0 commit comments

Comments
 (0)