mirror of
https://github.com/simstudioai/sim.git
synced 2026-09-24 15:45:35 +08:00
fix(api): widen the cancel-execution reason contract to what the route emits (#6592)
The internal cancel route mints four outcomes the cancellation service never produces — queue_cancelled, already_cancelled, active_resume_signal_failed and cancellation_not_finalized — but the contract enumerated only the five service reasons. requestJson validates every 2xx against that contract, so a stop that genuinely applied threw on the client. Keep the service enum narrow for the public v2 contract, which delegates wholly to the service and cannot emit the other four, and validate the internal route against a superset. Route the seven success bodies through one contract-typed constructor so drift is a compile error, and delete the dead duplicate cancel contract left behind in contracts/logs.ts.
This commit is contained in:
@@ -112,7 +112,24 @@ vi.mock('@/lib/execution/event-buffer', () => ({
|
||||
}),
|
||||
}))
|
||||
|
||||
import { POST } from './route'
|
||||
import { cancelWorkflowExecutionContract } from '@/lib/api/contracts/workflows'
|
||||
import { POST as cancelExecution } from './route'
|
||||
|
||||
/**
|
||||
* Drives the route and validates every success body against the contract that
|
||||
* `requestJson` enforces on the client. The route builds its responses by hand
|
||||
* rather than through a declarative builder, so nothing else checks that the
|
||||
* two agree — and a `reason` the contract omits makes the client throw on a
|
||||
* cancellation that actually applied. Routing every case in this suite through
|
||||
* here covers each `NextResponse.json` shape the route can return.
|
||||
*/
|
||||
const POST = async (...args: Parameters<typeof cancelExecution>) => {
|
||||
const response = await cancelExecution(...args)
|
||||
if (response.status < 400) {
|
||||
cancelWorkflowExecutionContract.response.schema.parse(await response.clone().json())
|
||||
}
|
||||
return response
|
||||
}
|
||||
|
||||
const makeRequest = () =>
|
||||
new NextRequest('http://localhost/api/workflows/wf-1/executions/ex-1/cancel', {
|
||||
|
||||
@@ -6,7 +6,10 @@ import { toError } from '@sim/utils/errors'
|
||||
import { sleep } from '@sim/utils/helpers'
|
||||
import { and, eq, inArray } from 'drizzle-orm'
|
||||
import { type NextRequest, NextResponse } from 'next/server'
|
||||
import { cancelWorkflowExecutionContract } from '@/lib/api/contracts/workflows'
|
||||
import {
|
||||
type CancelWorkflowExecutionResponse,
|
||||
cancelWorkflowExecutionContract,
|
||||
} from '@/lib/api/contracts/workflows'
|
||||
import { parseRequest } from '@/lib/api/server'
|
||||
import { WORKSPACE_KEY_SCOPE_DENIED } from '@/lib/api-key/policy-messages'
|
||||
import { checkHybridAuth } from '@/lib/auth/hybrid'
|
||||
@@ -34,6 +37,18 @@ const logger = createLogger('CancelExecutionAPI')
|
||||
const PAUSED_CANCELLATION_DB_ATTEMPTS = 3
|
||||
const PAUSED_CANCELLATION_DB_RETRY_MS = 200
|
||||
|
||||
/**
|
||||
* Builds the single success shape this route returns. The route hand-builds its
|
||||
* responses rather than going through a declarative builder, so nothing else
|
||||
* checks that they satisfy the contract the client validates against — and
|
||||
* several outcomes the route resolves itself (a still-queued run, an
|
||||
* already-cancelled run) ride on `success: true`, where a rejected body turns a
|
||||
* cancellation that worked into a client-side failure.
|
||||
*/
|
||||
function cancellationOutcome(body: CancelWorkflowExecutionResponse) {
|
||||
return NextResponse.json(body)
|
||||
}
|
||||
|
||||
async function cancelQueuedExecutionJobs(
|
||||
workflowId: string,
|
||||
executionId: string,
|
||||
@@ -397,7 +412,7 @@ export const POST = withRouteHandler(
|
||||
workspaceId ? { groups: { workspace: workspaceId } } : undefined
|
||||
)
|
||||
|
||||
return NextResponse.json({
|
||||
return cancellationOutcome({
|
||||
success: true,
|
||||
executionId,
|
||||
redisAvailable: cancellation.reason !== 'redis_unavailable',
|
||||
@@ -524,7 +539,7 @@ export const POST = withRouteHandler(
|
||||
const pausedReconciliationSucceeded =
|
||||
exactStopSatisfied &&
|
||||
(!hasPausedCancellation || (cancellationEventPublished && pausedCancelled))
|
||||
return NextResponse.json({
|
||||
return cancellationOutcome({
|
||||
success: pausedReconciliationSucceeded,
|
||||
executionId,
|
||||
redisAvailable: requiresCancellationEvent ? cancellationEventPublished : true,
|
||||
@@ -597,7 +612,7 @@ export const POST = withRouteHandler(
|
||||
})
|
||||
})
|
||||
await clearStopSignalMarkers(stopSummary)
|
||||
return NextResponse.json({
|
||||
return cancellationOutcome({
|
||||
success: false,
|
||||
executionId,
|
||||
redisAvailable: stopSummary.cancellation.reason !== 'redis_unavailable',
|
||||
@@ -658,7 +673,7 @@ export const POST = withRouteHandler(
|
||||
})
|
||||
})
|
||||
await clearStopSignalMarkers(stopSummary)
|
||||
return NextResponse.json({
|
||||
return cancellationOutcome({
|
||||
success: false,
|
||||
executionId,
|
||||
redisAvailable: stopSummary.cancellation.reason !== 'redis_unavailable',
|
||||
@@ -669,7 +684,7 @@ export const POST = withRouteHandler(
|
||||
})
|
||||
}
|
||||
} else if (!effectivePausedCancellationPath) {
|
||||
return NextResponse.json({
|
||||
return cancellationOutcome({
|
||||
success: false,
|
||||
executionId,
|
||||
redisAvailable: stopSummary.cancellation.reason !== 'redis_unavailable',
|
||||
@@ -801,7 +816,7 @@ export const POST = withRouteHandler(
|
||||
await PauseResumeManager.clearPausedCancellationIntent(executionId, workflowId)
|
||||
}
|
||||
}
|
||||
return NextResponse.json({
|
||||
return cancellationOutcome({
|
||||
success: false,
|
||||
executionId,
|
||||
redisAvailable: stopSummary.cancellation.reason !== 'redis_unavailable',
|
||||
@@ -963,7 +978,7 @@ export const POST = withRouteHandler(
|
||||
? 'queue_cancelled'
|
||||
: stopSummary.cancellation.reason
|
||||
|
||||
return NextResponse.json({
|
||||
return cancellationOutcome({
|
||||
success,
|
||||
executionId,
|
||||
redisAvailable:
|
||||
|
||||
@@ -1,7 +1,6 @@
|
||||
import { z } from 'zod'
|
||||
import { userFileSchema } from '@/lib/api/contracts/primitives'
|
||||
import { defineRouteContract } from '@/lib/api/contracts/types'
|
||||
import { cancelWorkflowExecutionReasonSchema } from '@/lib/api/contracts/workflows'
|
||||
|
||||
const comparisonOperatorSchema = z.enum(['=', '>', '<', '>=', '<=', '!='])
|
||||
|
||||
@@ -13,11 +12,6 @@ export const executionIdParamsSchema = z.object({
|
||||
executionId: z.string().min(1),
|
||||
})
|
||||
|
||||
export const cancelWorkflowExecutionParamsSchema = z.object({
|
||||
id: z.string().min(1, 'Invalid workflow ID'),
|
||||
executionId: z.string().min(1, 'Invalid execution ID'),
|
||||
})
|
||||
|
||||
const logFilterQuerySchema = z.object({
|
||||
workspaceId: z.string(),
|
||||
level: z.string().optional(),
|
||||
@@ -353,21 +347,10 @@ export const triggersQuerySchema = z.object({
|
||||
})
|
||||
export type TriggersQuery = z.output<typeof triggersQuerySchema>
|
||||
|
||||
export const cancelWorkflowExecutionResponseSchema = z.object({
|
||||
success: z.boolean(),
|
||||
executionId: z.string(),
|
||||
redisAvailable: z.boolean(),
|
||||
durablyRecorded: z.boolean(),
|
||||
locallyAborted: z.boolean(),
|
||||
pausedCancelled: z.boolean(),
|
||||
reason: cancelWorkflowExecutionReasonSchema,
|
||||
})
|
||||
|
||||
export type SegmentStats = z.output<typeof segmentStatsSchema>
|
||||
export type WorkflowStats = z.output<typeof workflowStatsSchema>
|
||||
export type DashboardStatsResponse = z.output<typeof dashboardStatsResponseSchema>
|
||||
export type ExecutionSnapshotData = z.output<typeof executionSnapshotDataSchema>
|
||||
export type CancelWorkflowExecutionResponse = z.output<typeof cancelWorkflowExecutionResponseSchema>
|
||||
|
||||
export const listLogsContract = defineRouteContract({
|
||||
method: 'GET',
|
||||
@@ -424,13 +407,3 @@ export const getExecutionSnapshotContract = defineRouteContract({
|
||||
schema: executionSnapshotDataSchema,
|
||||
},
|
||||
})
|
||||
|
||||
export const cancelWorkflowExecutionContract = defineRouteContract({
|
||||
method: 'POST',
|
||||
path: '/api/workflows/[id]/executions/[executionId]/cancel',
|
||||
params: cancelWorkflowExecutionParamsSchema,
|
||||
response: {
|
||||
mode: 'json',
|
||||
schema: cancelWorkflowExecutionResponseSchema,
|
||||
},
|
||||
})
|
||||
|
||||
@@ -1,7 +1,9 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import {
|
||||
cancelWorkflowExecutionReasonSchema,
|
||||
executeWorkflowBodySchema,
|
||||
getWorkflowResponseDataSchema,
|
||||
internalCancelWorkflowExecutionReasonSchema,
|
||||
updateWorkflowBodySchema,
|
||||
workflowListItemSchema,
|
||||
} from '@/lib/api/contracts/workflows'
|
||||
@@ -127,4 +129,23 @@ describe('workflow contracts', () => {
|
||||
expect(forkPolicySchema.parse({ forkSyncExcluded: true }).forkSyncExcluded).toBe(true)
|
||||
expect(forkPolicySchema.parse({}).forkSyncExcluded).toBe(false)
|
||||
})
|
||||
|
||||
/**
|
||||
* The v2 cancel endpoint presents `cancelWorkflowRun`'s result unchanged, and
|
||||
* that use case delegates wholly to the cancellation service — so it cannot
|
||||
* emit the outcomes the internal route resolves for itself. Folding those into
|
||||
* the service enum would publish four reasons v2 never returns, because the
|
||||
* v2 contract documents this enum value by value.
|
||||
*/
|
||||
it('keeps internal-only cancellation reasons out of the enum v2 publishes', () => {
|
||||
for (const reason of [
|
||||
'queue_cancelled',
|
||||
'already_cancelled',
|
||||
'active_resume_signal_failed',
|
||||
'cancellation_not_finalized',
|
||||
]) {
|
||||
expect(internalCancelWorkflowExecutionReasonSchema.options).toContain(reason)
|
||||
expect(cancelWorkflowExecutionReasonSchema.options).not.toContain(reason)
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
@@ -661,11 +661,12 @@ export const workflowExecutionStatusQuerySchema = z.object({
|
||||
})
|
||||
|
||||
/**
|
||||
* Full cancellation-outcome vocabulary — mirrors
|
||||
* `CancelWorkflowExecutionReason` in `lib/execution/cancel-workflow-execution`
|
||||
* (contracts stay import-clean of server modules). The paused-HITL path emits
|
||||
* the two `paused_*` values; a narrower copy of this enum previously lived in
|
||||
* `contracts/logs.ts` and made the client reject those responses.
|
||||
* Cancellation outcomes produced by the cancellation service, and so the whole
|
||||
* vocabulary the public v2 endpoint can return — `cancelWorkflowRun` delegates
|
||||
* its outcome to that service. Mirrors `CancelWorkflowExecutionReason` in
|
||||
* `lib/execution/cancel-workflow-execution` (contracts stay import-clean of
|
||||
* server modules). Keeping the internal route's extra outcomes out of here is
|
||||
* what stops the published v2 schema advertising reasons v2 cannot emit.
|
||||
*/
|
||||
export const cancelWorkflowExecutionReasonSchema = z.enum([
|
||||
'recorded',
|
||||
@@ -675,6 +676,22 @@ export const cancelWorkflowExecutionReasonSchema = z.enum([
|
||||
'paused_database_cancel_failed',
|
||||
])
|
||||
|
||||
/**
|
||||
* The internal route's vocabulary. It resolves four outcomes before the service
|
||||
* is ever reached: `queue_cancelled` (the run was still queued, so no execution
|
||||
* log row existed), `already_cancelled` (reconciling a run already cancelled),
|
||||
* and the two stop-signal failures. Several ride on `success: true` responses,
|
||||
* so validating them against the service enum makes `requestJson` reject
|
||||
* cancellations that genuinely applied.
|
||||
*/
|
||||
export const internalCancelWorkflowExecutionReasonSchema = z.enum([
|
||||
...cancelWorkflowExecutionReasonSchema.options,
|
||||
'queue_cancelled',
|
||||
'already_cancelled',
|
||||
'active_resume_signal_failed',
|
||||
'cancellation_not_finalized',
|
||||
])
|
||||
|
||||
const cancelWorkflowExecutionResponseSchema = z.object({
|
||||
success: z.boolean(),
|
||||
executionId: z.string(),
|
||||
@@ -682,9 +699,11 @@ const cancelWorkflowExecutionResponseSchema = z.object({
|
||||
durablyRecorded: z.boolean(),
|
||||
locallyAborted: z.boolean(),
|
||||
pausedCancelled: z.boolean(),
|
||||
reason: cancelWorkflowExecutionReasonSchema.optional(),
|
||||
reason: internalCancelWorkflowExecutionReasonSchema.optional(),
|
||||
})
|
||||
|
||||
export type CancelWorkflowExecutionResponse = z.output<typeof cancelWorkflowExecutionResponseSchema>
|
||||
|
||||
const resumeWorkflowExecutionContextResponseSchema = z
|
||||
.object({
|
||||
status: z.enum(['queued', 'started']).optional(),
|
||||
|
||||
@@ -41,9 +41,12 @@ async function cancelActiveWorkflowJob(executionId: string): Promise<boolean> {
|
||||
}
|
||||
|
||||
/**
|
||||
* Cancellation outcome vocabulary. `recorded`/`redis_unavailable`/
|
||||
* Cancellation outcome vocabulary produced by this service, and so the whole
|
||||
* vocabulary the public v2 endpoint can return. `recorded`/`redis_unavailable`/
|
||||
* `redis_write_failed` come from the Redis record step; the two `paused_*`
|
||||
* values from the paused-HITL path.
|
||||
* values from the paused-HITL path. The internal cancel route resolves further
|
||||
* outcomes on top of these — see `internalCancelWorkflowExecutionReasonSchema`
|
||||
* in `lib/api/contracts/workflows`.
|
||||
*/
|
||||
export type CancelWorkflowExecutionReason =
|
||||
| 'recorded'
|
||||
|
||||
Reference in New Issue
Block a user