Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions apps/docs/openapi-v2-workflows.json
Original file line number Diff line number Diff line change
Expand Up @@ -9997,7 +9997,7 @@
"description": "Whether a paused execution was cancelled."
},
"reason": {
"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.",
"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.",
"type": "string",
"enum": [
"recorded",
Expand All @@ -10007,7 +10007,10 @@
"redis_unavailable",
"redis_write_failed",
"paused_event_publish_failed",
"paused_database_cancel_failed"
"paused_database_cancel_failed",
"queue_cancelled",
"active_resume_signal_failed",
"cancellation_not_finalized"
]
}
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,14 @@ import {
} from '@sim/testing'
import { NextRequest } from 'next/server'
import { beforeEach, describe, expect, it, vi } from 'vitest'
import { WorkflowRunAlreadyTerminalError } from '@/lib/execution/workflow-run-already-terminal-error'

const mocks = vi.hoisted(() => ({
cancel: vi.fn(),
capture: vi.fn(),
}))

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

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

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

Expand All @@ -111,7 +102,39 @@ describe('POST /api/v2/workflows/[workflowId]/runs/[runId]/cancel', () => {
success: true,
runId: RUN_ID,
durablyRecorded: false,
reason,
reason: 'already_cancelled',
})
})

it.each([
['completed', 'already_completed'],
['failed', 'already_failed'],
] as const)(
'preserves the v2 terminal no-op response when a standalone run is already %s',
async (executionStatus, reason) => {
mocks.cancel.mockRejectedValue(
new WorkflowRunAlreadyTerminalError({
executionId: RUN_ID,
executionStatus,
redisAvailable: true,
locallyAborted: false,
})
)

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

expect(response.status).toBe(200)
await expect(response.json()).resolves.toEqual({
data: {
success: true,
runId: RUN_ID,
redisAvailable: true,
durablyRecorded: false,
locallyAborted: false,
pausedCancelled: false,
reason,
},
})
}
)
})
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { v2CancelWorkflowRunContract } from '@/lib/api/contracts/v2/workflows'
import { defineV2JsonRoute, v2ApiKeyAuth, v2RateLimits } from '@/lib/api/server/routes'
import { captureServerEvent } from '@/lib/posthog/server'
import { v2WorkflowErrorPolicies } from '@/lib/workflows/api'
import { cancelWorkflowRun } from '@/lib/workflows/application/cancel-run'
import { workflowOperations } from '@/lib/workflows/application/operations'
Expand All @@ -13,7 +12,7 @@ export const POST = defineV2JsonRoute({
auth: v2ApiKeyAuth,
operation: workflowOperations.cancelRun,
rateLimit: v2RateLimits.publicApi,
errorPolicy: v2WorkflowErrorPolicies.concealRunAuthorization,
errorPolicy: v2WorkflowErrorPolicies.cancelRun,
mapInput: ({ params }) => ({ workflowId: params.workflowId, runId: params.runId }),
useCase: cancelWorkflowRun,
present: (result) => ({
Expand All @@ -27,21 +26,4 @@ export const POST = defineV2JsonRoute({
reason: result.reason,
},
}),
/**
* Reports a cancellation, so it needs the run to have actually been
* cancelled. `success` alone no longer implies that: a cancel against an
* already-terminal run satisfies the request without writing anything, and
* reports `success: true` with `durablyRecorded: false`. Requiring both also
* keeps the event off a cancellation that reached the row but failed its
* paused reconciliation, which reports the inverse pair.
*/
onSuccess: ({ principal, result }) => {
if (!result.success || !result.durablyRecorded || principal.kind !== 'personal_api_key') return
captureServerEvent(
principal.userId,
'workflow_execution_cancelled',
{ workflow_id: result.workflowId, workspace_id: result.workspaceId },
{ groups: { workspace: result.workspaceId } }
)
},
})
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,13 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'

const mocks = vi.hoisted(() => ({
cancel: vi.fn(),
capture: vi.fn(),
readRun: vi.fn(),
authorizeReadRun: vi.fn(),
}))

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

vi.mock('@/lib/posthog/server', () => ({ captureServerEvent: mocks.capture }))

vi.mock('@/lib/workflows/application/read-workflow-run', () => ({
readWorkflowRun: {
operation: { id: 'workflows.runs.read' },
Expand Down Expand Up @@ -370,7 +367,6 @@ describe('v2 run detail and cancel adapters', () => {
'v2:workflows.runs.cancel:api-key:key-1',
expect.anything()
)
expect(mocks.capture).not.toHaveBeenCalled()
})

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

it('projects cancellation analytics only after a successful personal-key result', async () => {
it('passes a personal-key principal to the cancellation use case', async () => {
const personalPrincipal = {
kind: 'personal_api_key' as const,
userId: 'key-user',
keyId: 'personal-key',
}
v2RouteMocks.authenticate.mockResolvedValueOnce({
...auth,
principal: { kind: 'personal_api_key', userId: 'key-user', keyId: 'personal-key' },
principal: personalPrincipal,
rateLimitSubjectIds: ['api-key:personal-key', 'user:key-user'],
keyType: 'personal',
})
Expand All @@ -415,12 +415,10 @@ describe('v2 run detail and cancel adapters', () => {
})

expect(response.status).toBe(200)
expect(mocks.capture).toHaveBeenCalledOnce()
expect(mocks.capture).toHaveBeenCalledWith(
'key-user',
'workflow_execution_cancelled',
{ workflow_id: 'workflow-1', workspace_id: 'workspace-1' },
{ groups: { workspace: 'workspace-1' } }
)
expect(mocks.cancel).toHaveBeenCalledWith({
principal: personalPrincipal,
input: { workflowId: 'workflow-1', runId: 'run-1' },
request: expect.anything(),
})
})
})
Loading
Loading