Skip to content

Commit 48f72a2

Browse files
j15zicecrasher321
andauthored
feat(copilot): let Run agent cancel workflow runs (#7151)
* feat(copilot): execute workflow run cancellations * fix(copilot): route run cancellations through internal API * fix(copilot): share workflow cancellation boundary * fix(copilot): honor aborts before cancellation commit * fix(copilot): reconcile failed cancellation rollbacks * fix(copilot): honor workflow group cancellation commit * test(copilot): include workflow cancellation approval * improvement(copilot): simplify cancellation lifecycle * improvement(workflows): unify run cancellation * fix(workflows): preserve v2 terminal cancellation responses * fix(workflows): align cancellation adapter test * fix(workflows): reconcile replacement resume cancellation * fix(workflows): confirm every active resume stop * fix(copilot): require workflow id for cancellation * fix(workflows): validate cancellation terminal writes * fix(workflows): cancel runs by execution id * fix(workflows): reconcile failed abort rollbacks * fix(workflows): restore rejected cancellation staging * fix(workflows): preserve terminal runs during cancellation * fix(execution): stop terminal-race resumes before cleanup --------- Co-authored-by: Vikhyath Mondreti <vikhyath@simstudio.ai>
1 parent 1404e65 commit 48f72a2

31 files changed

Lines changed: 3328 additions & 3323 deletions

apps/docs/openapi-v2-workflows.json

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9997,7 +9997,7 @@
99979997
"description": "Whether a paused execution was cancelled."
99989998
},
99999999
"reason": {
10000-
"description": "Machine-readable cancellation outcome, present on every cancellation including full successes. `recorded` is the success value. `already_cancelled`, `already_completed`, and `already_failed` mean the run had already reached that terminal state, so nothing was cancelled and `durablyRecorded` is false. `redis_unavailable` and `redis_write_failed` mean the distributed cancellation signal was not written, so an already-running execution may not observe the cancellation. `paused_event_publish_failed` and `paused_database_cancel_failed` name the failing step for a paused run.",
10000+
"description": "Machine-readable cancellation outcome, present on every cancellation including full successes. `recorded` and `queue_cancelled` are successful cancellation values. `already_cancelled`, `already_completed`, and `already_failed` mean the run had already reached that terminal state, so nothing was cancelled and `durablyRecorded` is false. The remaining values identify a degraded or incomplete cancellation step.",
1000110001
"type": "string",
1000210002
"enum": [
1000310003
"recorded",
@@ -10007,7 +10007,10 @@
1000710007
"redis_unavailable",
1000810008
"redis_write_failed",
1000910009
"paused_event_publish_failed",
10010-
"paused_database_cancel_failed"
10010+
"paused_database_cancel_failed",
10011+
"queue_cancelled",
10012+
"active_resume_signal_failed",
10013+
"cancellation_not_finalized"
1001110014
]
1001210015
}
1001310016
},

apps/sim/app/api/v2/workflows/[workflowId]/runs/[runId]/cancel/route.test.ts

Lines changed: 38 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,14 @@ import {
1111
} from '@sim/testing'
1212
import { NextRequest } from 'next/server'
1313
import { beforeEach, describe, expect, it, vi } from 'vitest'
14+
import { WorkflowRunAlreadyTerminalError } from '@/lib/execution/workflow-run-already-terminal-error'
1415

1516
const mocks = vi.hoisted(() => ({
1617
cancel: vi.fn(),
17-
capture: vi.fn(),
1818
}))
1919

2020
vi.mock('@/lib/api/server/routes/v2-api-key-auth', () => v2ApiKeyAuthModuleMock)
2121
vi.mock('@/lib/core/rate-limiter', () => v2RateLimiterModuleMock)
22-
vi.mock('@/lib/posthog/server', () => ({ captureServerEvent: mocks.capture }))
2322
vi.mock('@/lib/workflows/application/cancel-run', () => ({
2423
cancelWorkflowRun: { operation: { id: 'workflows.runs.cancel' }, execute: mocks.cancel },
2524
}))
@@ -91,18 +90,10 @@ describe('POST /api/v2/workflows/[workflowId]/runs/[runId]/cancel', () => {
9190
})
9291
})
9392

94-
/**
95-
* The published outcome of a cancel against a run that had already finished.
96-
* `durablyRecorded: true` here is the defect this suite pins: nothing was
97-
* written, so a caller reconciling on that flag would trust a write that never
98-
* happened.
99-
*/
100-
it.each([
101-
['cancelled', 'already_cancelled'],
102-
['completed', 'already_completed'],
103-
['failed', 'already_failed'],
104-
])('reports a terminal %s run as a no-op the caller can tell apart', async (_status, reason) => {
105-
mocks.cancel.mockResolvedValue(serviceResult({ success: true, durablyRecorded: false, reason }))
93+
it('reports an already-cancelled run as an idempotent no-op', async () => {
94+
mocks.cancel.mockResolvedValue(
95+
serviceResult({ success: true, durablyRecorded: false, reason: 'already_cancelled' })
96+
)
10697

10798
const response = await POST(request(), context)
10899

@@ -111,7 +102,39 @@ describe('POST /api/v2/workflows/[workflowId]/runs/[runId]/cancel', () => {
111102
success: true,
112103
runId: RUN_ID,
113104
durablyRecorded: false,
114-
reason,
105+
reason: 'already_cancelled',
115106
})
116107
})
108+
109+
it.each([
110+
['completed', 'already_completed'],
111+
['failed', 'already_failed'],
112+
] as const)(
113+
'preserves the v2 terminal no-op response when a standalone run is already %s',
114+
async (executionStatus, reason) => {
115+
mocks.cancel.mockRejectedValue(
116+
new WorkflowRunAlreadyTerminalError({
117+
executionId: RUN_ID,
118+
executionStatus,
119+
redisAvailable: true,
120+
locallyAborted: false,
121+
})
122+
)
123+
124+
const response = await POST(request(), context)
125+
126+
expect(response.status).toBe(200)
127+
await expect(response.json()).resolves.toEqual({
128+
data: {
129+
success: true,
130+
runId: RUN_ID,
131+
redisAvailable: true,
132+
durablyRecorded: false,
133+
locallyAborted: false,
134+
pausedCancelled: false,
135+
reason,
136+
},
137+
})
138+
}
139+
)
117140
})
Lines changed: 2 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import { v2CancelWorkflowRunContract } from '@/lib/api/contracts/v2/workflows'
22
import { defineV2JsonRoute, v2ApiKeyAuth, v2RateLimits } from '@/lib/api/server/routes'
3-
import { captureServerEvent } from '@/lib/posthog/server'
43
import { v2WorkflowErrorPolicies } from '@/lib/workflows/api'
54
import { cancelWorkflowRun } from '@/lib/workflows/application/cancel-run'
65
import { workflowOperations } from '@/lib/workflows/application/operations'
@@ -13,8 +12,8 @@ export const POST = defineV2JsonRoute({
1312
auth: v2ApiKeyAuth,
1413
operation: workflowOperations.cancelRun,
1514
rateLimit: v2RateLimits.publicApi,
16-
errorPolicy: v2WorkflowErrorPolicies.concealRunAuthorization,
17-
mapInput: ({ params }) => ({ workflowId: params.workflowId, runId: params.runId }),
15+
errorPolicy: v2WorkflowErrorPolicies.cancelRun,
16+
mapInput: ({ params }) => ({ runId: params.runId }),
1817
useCase: cancelWorkflowRun,
1918
present: (result) => ({
2019
data: {
@@ -27,21 +26,4 @@ export const POST = defineV2JsonRoute({
2726
reason: result.reason,
2827
},
2928
}),
30-
/**
31-
* Reports a cancellation, so it needs the run to have actually been
32-
* cancelled. `success` alone no longer implies that: a cancel against an
33-
* already-terminal run satisfies the request without writing anything, and
34-
* reports `success: true` with `durablyRecorded: false`. Requiring both also
35-
* keeps the event off a cancellation that reached the row but failed its
36-
* paused reconciliation, which reports the inverse pair.
37-
*/
38-
onSuccess: ({ principal, result }) => {
39-
if (!result.success || !result.durablyRecorded || principal.kind !== 'personal_api_key') return
40-
captureServerEvent(
41-
principal.userId,
42-
'workflow_execution_cancelled',
43-
{ workflow_id: result.workflowId, workspace_id: result.workspaceId },
44-
{ groups: { workspace: result.workspaceId } }
45-
)
46-
},
4729
})

apps/sim/app/api/v2/workflows/[workflowId]/runs/[runId]/route.test.ts

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -14,16 +14,13 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'
1414

1515
const mocks = vi.hoisted(() => ({
1616
cancel: vi.fn(),
17-
capture: vi.fn(),
1817
readRun: vi.fn(),
1918
authorizeReadRun: vi.fn(),
2019
}))
2120

2221
vi.mock('@/lib/api/server/routes/v2-api-key-auth', () => v2ApiKeyAuthModuleMock)
2322
vi.mock('@/lib/core/rate-limiter', () => v2RateLimiterModuleMock)
2423

25-
vi.mock('@/lib/posthog/server', () => ({ captureServerEvent: mocks.capture }))
26-
2724
vi.mock('@/lib/workflows/application/read-workflow-run', () => ({
2825
readWorkflowRun: {
2926
operation: { id: 'workflows.runs.read' },
@@ -362,15 +359,14 @@ describe('v2 run detail and cancel adapters', () => {
362359
})
363360
expect(mocks.cancel).toHaveBeenCalledWith({
364361
principal,
365-
input: { workflowId: 'workflow-1', runId: 'run-1' },
362+
input: { runId: 'run-1' },
366363
request: expect.anything(),
367364
})
368365
expect(v2RouteMocks.operationRate).toHaveBeenCalledTimes(2)
369366
expect(v2RouteMocks.operationRate).toHaveBeenCalledWith(
370367
'v2:workflows.runs.cancel:api-key:key-1',
371368
expect.anything()
372369
)
373-
expect(mocks.capture).not.toHaveBeenCalled()
374370
})
375371

376372
it('keeps cancellation request-rate admission separate from run control', async () => {
@@ -399,13 +395,17 @@ describe('v2 run detail and cancel adapters', () => {
399395
code: 'FORBIDDEN',
400396
message: 'Insufficient workspace permissions',
401397
})
402-
expect(mocks.capture).not.toHaveBeenCalled()
403398
})
404399

405-
it('projects cancellation analytics only after a successful personal-key result', async () => {
400+
it('passes a personal-key principal to the cancellation use case', async () => {
401+
const personalPrincipal = {
402+
kind: 'personal_api_key' as const,
403+
userId: 'key-user',
404+
keyId: 'personal-key',
405+
}
406406
v2RouteMocks.authenticate.mockResolvedValueOnce({
407407
...auth,
408-
principal: { kind: 'personal_api_key', userId: 'key-user', keyId: 'personal-key' },
408+
principal: personalPrincipal,
409409
rateLimitSubjectIds: ['api-key:personal-key', 'user:key-user'],
410410
keyType: 'personal',
411411
})
@@ -415,12 +415,10 @@ describe('v2 run detail and cancel adapters', () => {
415415
})
416416

417417
expect(response.status).toBe(200)
418-
expect(mocks.capture).toHaveBeenCalledOnce()
419-
expect(mocks.capture).toHaveBeenCalledWith(
420-
'key-user',
421-
'workflow_execution_cancelled',
422-
{ workflow_id: 'workflow-1', workspace_id: 'workspace-1' },
423-
{ groups: { workspace: 'workspace-1' } }
424-
)
418+
expect(mocks.cancel).toHaveBeenCalledWith({
419+
principal: personalPrincipal,
420+
input: { runId: 'run-1' },
421+
request: expect.anything(),
422+
})
425423
})
426424
})

0 commit comments

Comments
 (0)