Skip to content

Commit aa2e5f5

Browse files
committed
fix(workflows): confirm every active resume stop
1 parent 161f18d commit aa2e5f5

2 files changed

Lines changed: 69 additions & 6 deletions

File tree

apps/sim/lib/execution/cancel-workflow-execution.test.ts

Lines changed: 48 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -803,6 +803,7 @@ describe('cancelWorkflowExecution', () => {
803803
mockMarkExecutionCancelled
804804
.mockResolvedValueOnce({ durablyRecorded: false, reason: 'redis_unavailable' })
805805
.mockResolvedValueOnce({ durablyRecorded: true, reason: 'recorded' })
806+
.mockResolvedValueOnce({ durablyRecorded: true, reason: 'recorded' })
806807
mockCompletePausedCancellation.mockResolvedValueOnce(true)
807808

808809
const response = await POST(makeRequest(), makeParams())
@@ -822,7 +823,10 @@ describe('cancelWorkflowExecution', () => {
822823
expect(mockMarkExecutionCancelled).toHaveBeenNthCalledWith(1, 'resume-ex-1', {
823824
executionDeadlineAt: null,
824825
})
825-
expect(mockMarkExecutionCancelled).toHaveBeenNthCalledWith(2, 'resume-ex-2', {
826+
expect(mockMarkExecutionCancelled).toHaveBeenNthCalledWith(2, 'resume-ex-1', {
827+
executionDeadlineAt: null,
828+
})
829+
expect(mockMarkExecutionCancelled).toHaveBeenNthCalledWith(3, 'resume-ex-2', {
826830
executionDeadlineAt: null,
827831
})
828832
expect(mockCompletePausedCancellation).toHaveBeenCalledWith('ex-1', 'wf-1')
@@ -842,6 +846,7 @@ describe('cancelWorkflowExecution', () => {
842846
.mockResolvedValueOnce({ durablyRecorded: false, reason: 'redis_unavailable' })
843847
.mockResolvedValueOnce({ durablyRecorded: false, reason: 'redis_unavailable' })
844848
.mockResolvedValueOnce({ durablyRecorded: true, reason: 'recorded' })
849+
.mockResolvedValueOnce({ durablyRecorded: true, reason: 'recorded' })
845850
mockCompletePausedCancellation.mockResolvedValueOnce(true)
846851

847852
const response = await POST(makeRequest(), makeParams())
@@ -864,12 +869,53 @@ describe('cancelWorkflowExecution', () => {
864869
expect(mockMarkExecutionCancelled).toHaveBeenNthCalledWith(2, 'resume-ex-1', {
865870
executionDeadlineAt: null,
866871
})
867-
expect(mockMarkExecutionCancelled).toHaveBeenNthCalledWith(3, 'resume-ex-2', {
872+
expect(mockMarkExecutionCancelled).toHaveBeenNthCalledWith(3, 'resume-ex-1', {
873+
executionDeadlineAt: null,
874+
})
875+
expect(mockMarkExecutionCancelled).toHaveBeenNthCalledWith(4, 'resume-ex-2', {
868876
executionDeadlineAt: null,
869877
})
870878
expect(mockCompletePausedCancellation).toHaveBeenCalledWith('ex-1', 'wf-1')
871879
})
872880

881+
it('does not treat replacement queue cancellation as confirmation of the original stop', async () => {
882+
mockStagePausedCancellation
883+
.mockResolvedValueOnce({ kind: 'active_resume', target: ACTIVE_RESUME_TARGET })
884+
.mockResolvedValueOnce({
885+
kind: 'active_resume',
886+
target: REPLACEMENT_ACTIVE_RESUME_TARGET,
887+
})
888+
mockGetActiveResumeCancellationTarget
889+
.mockResolvedValueOnce(REPLACEMENT_ACTIVE_RESUME_TARGET)
890+
.mockResolvedValueOnce(REPLACEMENT_ACTIVE_RESUME_TARGET)
891+
mockRollbackActiveResumeCancellation.mockResolvedValueOnce(false)
892+
mockCancelByExecution.mockResolvedValue(1)
893+
mockMarkExecutionCancelled
894+
.mockResolvedValueOnce({ durablyRecorded: false, reason: 'redis_unavailable' })
895+
.mockResolvedValueOnce({ durablyRecorded: false, reason: 'redis_unavailable' })
896+
.mockResolvedValueOnce({ durablyRecorded: true, reason: 'recorded' })
897+
898+
const response = await POST(makeRequest(), makeParams())
899+
900+
expect(response.status).toBe(200)
901+
await expect(response.json()).resolves.toMatchObject({
902+
success: false,
903+
pausedCancelled: false,
904+
reason: 'active_resume_signal_failed',
905+
})
906+
expect(mockMarkExecutionCancelled).toHaveBeenNthCalledWith(1, 'resume-ex-1', {
907+
executionDeadlineAt: null,
908+
})
909+
expect(mockMarkExecutionCancelled).toHaveBeenNthCalledWith(2, 'resume-ex-1', {
910+
executionDeadlineAt: null,
911+
})
912+
expect(mockMarkExecutionCancelled).toHaveBeenNthCalledWith(3, 'resume-ex-2', {
913+
executionDeadlineAt: null,
914+
})
915+
expect(mockWriteTerminalEvent).not.toHaveBeenCalled()
916+
expect(mockCompletePausedCancellation).not.toHaveBeenCalled()
917+
})
918+
873919
it('returns success when a paused HITL execution is cancelled directly in the database', async () => {
874920
mockStagePausedCancellation.mockResolvedValue({ kind: 'idle' })
875921
mockCompletePausedCancellation.mockResolvedValue(true)

apps/sim/lib/execution/cancel-workflow-execution.ts

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -181,11 +181,14 @@ async function didActiveResumeStop(
181181
target: ActiveResumeCancellationTarget,
182182
signal: ExecutionStopSignalResult
183183
): Promise<boolean> {
184-
if (signal.accepted) return true
184+
if (signal.cancellation.durablyRecorded || signal.locallyAborted) return true
185185
const currentTarget = await PauseResumeManager.getActiveResumeCancellationTarget(
186186
executionId,
187187
workflowId
188188
)
189+
if (currentTarget?.resumeEntryId === target.resumeEntryId && signal.queueJobsCancelled > 0) {
190+
return true
191+
}
189192
if (currentTarget && currentTarget.resumeEntryId !== target.resumeEntryId) {
190193
logger.warn('A replacement resume became active while cancellation was staged', {
191194
executionId,
@@ -707,6 +710,7 @@ export async function cancelWorkflowExecution({
707710
pausedCancellationStage.kind === 'active_resume' ? pausedCancellationStage.target : null
708711
let activeResumeEntryId = activeResumeTarget?.resumeEntryId ?? null
709712
let activeResumeSignalAccepted = false
713+
const activeResumeTargetsNeedingStopConfirmation: ActiveResumeCancellationTarget[] = []
710714

711715
if (activeResumeTarget && !isWorkflowGroupExecution) {
712716
activeResumeSignalAccepted = await signalAndRecordActiveResumeStop({
@@ -728,6 +732,7 @@ export async function cancelWorkflowExecution({
728732
await clearStopSignalMarkers(stopSummary)
729733
return activeResumeSignalFailureResult(executionId, stopSummary)
730734
}
735+
activeResumeTargetsNeedingStopConfirmation.push(activeResumeTarget)
731736
}
732737
} else if (!effectivePausedCancellationPath && !isWorkflowGroupExecution) {
733738
const signal = await signalExecutionStop({
@@ -778,6 +783,7 @@ export async function cancelWorkflowExecution({
778783
await clearStopSignalMarkers(stopSummary)
779784
return activeResumeSignalFailureResult(executionId, stopSummary)
780785
}
786+
activeResumeTargetsNeedingStopConfirmation.push(activeResumeTarget)
781787
}
782788
} else if (!effectivePausedCancellationPath) {
783789
return {
@@ -934,6 +940,19 @@ export async function cancelWorkflowExecution({
934940

935941
let pauseReconciliationFailed = false
936942
let activeResumeSignalFailed = false
943+
for (const target of activeResumeTargetsNeedingStopConfirmation) {
944+
const stopConfirmed = await signalAndRecordActiveResumeStop({
945+
workflowId,
946+
executionId,
947+
executionDeadlineAt: execution.executionDeadlineAt,
948+
target,
949+
summary: stopSummary,
950+
})
951+
if (target.resumeEntryId === activeResumeEntryId) {
952+
activeResumeSignalAccepted = stopConfirmed
953+
}
954+
activeResumeSignalFailed = activeResumeSignalFailed || !stopConfirmed
955+
}
937956
if (isWorkflowGroupExecution) {
938957
if (activeResumeTarget) {
939958
activeResumeSignalAccepted = await signalAndRecordActiveResumeStop({
@@ -975,9 +994,7 @@ export async function cancelWorkflowExecution({
975994
summary: stopSummary,
976995
})
977996
}
978-
activeResumeSignalFailed = !activeResumeSignalAccepted
979-
} else {
980-
activeResumeSignalFailed = false
997+
activeResumeSignalFailed = activeResumeSignalFailed || !activeResumeSignalAccepted
981998
}
982999
}
9831000
} catch (error) {

0 commit comments

Comments
 (0)