Skip to content

Commit f9dddb4

Browse files
committed
fix(copilot): release failed workflow claims
1 parent 586f210 commit f9dddb4

4 files changed

Lines changed: 115 additions & 1 deletion

File tree

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

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ const {
4343
mockInitializeExecutionStreamMeta,
4444
mockReleaseExecutionIdClaim,
4545
mockReleaseExecutionSlot,
46+
mockReleaseWorkflowToolExecutionClaim,
4647
mockRequireBillingAttributionHeader,
4748
mockValidatePublicApiAllowed,
4849
} = vi.hoisted(() => ({
@@ -67,6 +68,7 @@ const {
6768
mockInitializeExecutionStreamMeta: vi.fn(),
6869
mockReleaseExecutionIdClaim: vi.fn(),
6970
mockReleaseExecutionSlot: vi.fn(),
71+
mockReleaseWorkflowToolExecutionClaim: vi.fn(),
7072
mockRequireBillingAttributionHeader: vi.fn(),
7173
mockValidatePublicApiAllowed: vi.fn(),
7274
}))
@@ -119,6 +121,7 @@ vi.mock('@/lib/copilot/async-runs/repository', () => ({
119121
claimWorkflowToolExecution: mockClaimWorkflowToolExecution,
120122
getAsyncToolCall: mockGetAsyncToolCall,
121123
getRunSegment: mockGetRunSegment,
124+
releaseWorkflowToolExecutionClaim: mockReleaseWorkflowToolExecutionClaim,
122125
}))
123126

124127
vi.mock('@/lib/execution/event-buffer', () => ({
@@ -435,6 +438,7 @@ describe('workflow execute async route', () => {
435438
expect(response.status).toBe(200)
436439
expect(streamCompleted).toBe(false)
437440
expect(mockClaimWorkflowToolExecution).toHaveBeenCalledWith('copilot-tool-1', 'execution-123')
441+
expect(mockReleaseWorkflowToolExecutionClaim).not.toHaveBeenCalled()
438442
expect(loggingSessionMockFns.mockSetTrustedExecutionCorrelation).toHaveBeenCalledWith({
439443
executionId: 'execution-123',
440444
requestId: 'req-12345678',
@@ -466,9 +470,44 @@ describe('workflow execute async route', () => {
466470
expect(loggingSessionMockFns.mockSetTrustedExecutionCorrelation).not.toHaveBeenCalled()
467471
expect(mockPreprocessExecution).not.toHaveBeenCalled()
468472
expect(mockExecuteWorkflowCore).not.toHaveBeenCalled()
473+
expect(mockReleaseWorkflowToolExecutionClaim).not.toHaveBeenCalled()
469474
expect(mockReleaseExecutionIdClaim).toHaveBeenCalled()
470475
})
471476

477+
it('releases a bound Copilot workflow claim when preprocessing rejects the run', async () => {
478+
mockPreprocessExecution.mockResolvedValueOnce({
479+
success: false,
480+
error: { message: 'Not admitted', statusCode: 402 },
481+
})
482+
483+
const response = await POST(createBoundCopilotExecutionRequest(), {
484+
params: Promise.resolve({ id: 'workflow-1' }),
485+
})
486+
487+
expect(response.status).toBe(402)
488+
expect(mockReleaseWorkflowToolExecutionClaim).toHaveBeenCalledWith(
489+
'copilot-tool-1',
490+
'execution-123'
491+
)
492+
expect(mockReleaseExecutionIdClaim).toHaveBeenCalled()
493+
})
494+
495+
it('retains a bound Copilot workflow claim when preprocessing created a durable error log', async () => {
496+
mockPreprocessExecution.mockResolvedValueOnce({
497+
success: false,
498+
error: { message: 'Not admitted', statusCode: 402 },
499+
})
500+
mockHasDurableExecutionOwner.mockResolvedValueOnce(true)
501+
502+
const response = await POST(createBoundCopilotExecutionRequest(), {
503+
params: Promise.resolve({ id: 'workflow-1' }),
504+
})
505+
506+
expect(response.status).toBe(402)
507+
expect(mockReleaseWorkflowToolExecutionClaim).not.toHaveBeenCalled()
508+
expect(mockReleaseExecutionIdClaim).not.toHaveBeenCalled()
509+
})
510+
472511
it('binds a workflow execution after its page-hide confirmation detached the waiter', async () => {
473512
mockGetAsyncToolCall.mockResolvedValueOnce({
474513
toolCallId: 'copilot-tool-1',

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

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import {
2424
claimWorkflowToolExecution,
2525
getAsyncToolCall,
2626
getRunSegment,
27+
releaseWorkflowToolExecutionClaim,
2728
} from '@/lib/copilot/async-runs/repository'
2829
import { isWorkflowToolName, resolveWorkflowToolTargetId } from '@/lib/copilot/tools/workflow-tools'
2930
import { admissionRejectedResponse, tryAdmit } from '@/lib/core/admission/gate'
@@ -621,6 +622,8 @@ async function handleExecutePost(
621622
let executionId = ''
622623
let executionIdClaim: ExecutionIdClaim | null = null
623624
let executionIdClaimCommitted = false
625+
let workflowToolClaimAcquired = false
626+
let copilotToolCallId: string | undefined
624627

625628
try {
626629
const auth = await checkHybridAuth(req, { requireWorkflowId: false })
@@ -758,13 +761,14 @@ async function handleExecutePost(
758761
workflowStateOverride,
759762
deploymentVersionId: admittedDeploymentVersionId,
760763
executionId: rawBodyExecutionId,
761-
copilotToolCallId,
764+
copilotToolCallId: parsedCopilotToolCallId,
762765
triggerBlockId: parsedTriggerBlockId,
763766
startBlockId,
764767
stopAfterBlockId,
765768
runFromBlock: rawRunFromBlock,
766769
parentWorkspaceId,
767770
} = validation.data
771+
copilotToolCallId = parsedCopilotToolCallId
768772
const triggerBlockId = parsedTriggerBlockId ?? startBlockId
769773
const streamHeader = req.headers.get('X-Stream-Response') === 'true'
770774
const enableSSE = streamHeader || streamParam === true
@@ -1160,6 +1164,7 @@ async function handleExecutePost(
11601164
{ status: 409 }
11611165
)
11621166
}
1167+
workflowToolClaimAcquired = true
11631168
}
11641169

11651170
const loggingSession = new LoggingSession(
@@ -2387,6 +2392,18 @@ async function handleExecutePost(
23872392
}
23882393
}
23892394

2395+
if (copilotToolCallId && workflowToolClaimAcquired && !executionIdClaimCommitted) {
2396+
try {
2397+
await releaseWorkflowToolExecutionClaim(copilotToolCallId, executionId)
2398+
} catch (error) {
2399+
reqLogger.warn('Failed to release pre-start Copilot workflow tool claim', {
2400+
error: toError(error).message,
2401+
executionId,
2402+
copilotToolCallId,
2403+
})
2404+
}
2405+
}
2406+
23902407
if (executionIdClaim && !executionIdClaimCommitted) {
23912408
try {
23922409
await releaseExecutionIdClaim(executionIdClaim)

apps/sim/lib/copilot/async-runs/repository.test.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
detachAsyncToolCall,
1313
getClaimedWorkflowExecutionId,
1414
recordToolPermissionDecision,
15+
releaseWorkflowToolExecutionClaim,
1516
replaceTerminalAsyncToolCallResult,
1617
upsertAsyncToolCall,
1718
} from './repository'
@@ -161,6 +162,29 @@ describe('async tool repository single-row semantics', () => {
161162
await expect(claimWorkflowToolExecution('workflow-tool', 'execution-2')).resolves.toBeNull()
162163
})
163164

165+
it('releases a matching pre-start workflow claim without changing its lifecycle status', async () => {
166+
dbChainMockFns.returning.mockResolvedValueOnce([
167+
{
168+
toolCallId: 'workflow-tool',
169+
status: 'delivered',
170+
claimedBy: null,
171+
},
172+
])
173+
174+
const result = await releaseWorkflowToolExecutionClaim('workflow-tool', 'execution-1')
175+
176+
expect(result).toMatchObject({
177+
toolCallId: 'workflow-tool',
178+
status: 'delivered',
179+
claimedBy: null,
180+
})
181+
expect(dbChainMockFns.set).toHaveBeenCalledWith({
182+
claimedBy: null,
183+
claimedAt: null,
184+
updatedAt: expect.any(Date),
185+
})
186+
})
187+
164188
it('detaches a bound workflow waiter without releasing its execution claim', async () => {
165189
dbChainMockFns.returning.mockResolvedValueOnce([
166190
{

apps/sim/lib/copilot/async-runs/repository.ts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -424,6 +424,40 @@ export async function claimWorkflowToolExecution(toolCallId: string, executionId
424424
)
425425
}
426426

427+
export async function releaseWorkflowToolExecutionClaim(toolCallId: string, executionId: string) {
428+
const claimedBy = `${WORKFLOW_EXECUTION_CLAIM_PREFIX}${executionId}`
429+
return withDbSpan(
430+
TraceSpan.CopilotAsyncRunsReleaseClaim,
431+
'UPDATE',
432+
'copilot_async_tool_calls',
433+
{
434+
[TraceAttr.ToolCallId]: toolCallId,
435+
[TraceAttr.CopilotAsyncToolClaimedBy]: claimedBy,
436+
},
437+
async () => {
438+
const [row] = await db
439+
.update(copilotAsyncToolCalls)
440+
.set({
441+
claimedBy: null,
442+
claimedAt: null,
443+
updatedAt: new Date(),
444+
})
445+
.where(
446+
and(
447+
eq(copilotAsyncToolCalls.toolCallId, toolCallId),
448+
eq(copilotAsyncToolCalls.claimedBy, claimedBy),
449+
inArray(copilotAsyncToolCalls.status, [
450+
ASYNC_TOOL_STATUS.running,
451+
ASYNC_TOOL_STATUS.delivered,
452+
])
453+
)
454+
)
455+
.returning()
456+
return row ?? null
457+
}
458+
)
459+
}
460+
427461
/**
428462
* Atomically claims a pending client tool exactly once. Native browser actions
429463
* use this before crossing the Electron boundary so a replayed renderer event

0 commit comments

Comments
 (0)