From fac8b3b4eb4247468cc44014d6acace26f91cf68 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 11 Aug 2026 21:17:12 -0700 Subject: [PATCH] fix(api): widen the cancel-execution reason contract to what the route emits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../[executionId]/cancel/route.test.ts | 19 +++++++++++- .../executions/[executionId]/cancel/route.ts | 31 ++++++++++++++----- apps/sim/lib/api/contracts/logs.ts | 27 ---------------- apps/sim/lib/api/contracts/workflows.test.ts | 21 +++++++++++++ apps/sim/lib/api/contracts/workflows.ts | 31 +++++++++++++++---- .../execution/cancel-workflow-execution.ts | 7 +++-- 6 files changed, 92 insertions(+), 44 deletions(-) diff --git a/apps/sim/app/api/workflows/[id]/executions/[executionId]/cancel/route.test.ts b/apps/sim/app/api/workflows/[id]/executions/[executionId]/cancel/route.test.ts index 6cc2723dec4..a8591a5f2ca 100644 --- a/apps/sim/app/api/workflows/[id]/executions/[executionId]/cancel/route.test.ts +++ b/apps/sim/app/api/workflows/[id]/executions/[executionId]/cancel/route.test.ts @@ -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) => { + 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', { diff --git a/apps/sim/app/api/workflows/[id]/executions/[executionId]/cancel/route.ts b/apps/sim/app/api/workflows/[id]/executions/[executionId]/cancel/route.ts index a9cc8962df3..9848732d8d3 100644 --- a/apps/sim/app/api/workflows/[id]/executions/[executionId]/cancel/route.ts +++ b/apps/sim/app/api/workflows/[id]/executions/[executionId]/cancel/route.ts @@ -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: diff --git a/apps/sim/lib/api/contracts/logs.ts b/apps/sim/lib/api/contracts/logs.ts index 06b0f5c4467..b269d8c75f5 100644 --- a/apps/sim/lib/api/contracts/logs.ts +++ b/apps/sim/lib/api/contracts/logs.ts @@ -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 -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 export type WorkflowStats = z.output export type DashboardStatsResponse = z.output export type ExecutionSnapshotData = z.output -export type CancelWorkflowExecutionResponse = z.output 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, - }, -}) diff --git a/apps/sim/lib/api/contracts/workflows.test.ts b/apps/sim/lib/api/contracts/workflows.test.ts index 05b7834b68f..50fa869865b 100644 --- a/apps/sim/lib/api/contracts/workflows.test.ts +++ b/apps/sim/lib/api/contracts/workflows.test.ts @@ -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) + } + }) }) diff --git a/apps/sim/lib/api/contracts/workflows.ts b/apps/sim/lib/api/contracts/workflows.ts index bb1c7f9524b..acaa7cc344c 100644 --- a/apps/sim/lib/api/contracts/workflows.ts +++ b/apps/sim/lib/api/contracts/workflows.ts @@ -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 + const resumeWorkflowExecutionContextResponseSchema = z .object({ status: z.enum(['queued', 'started']).optional(), diff --git a/apps/sim/lib/execution/cancel-workflow-execution.ts b/apps/sim/lib/execution/cancel-workflow-execution.ts index 79ed19d0823..1ca55620762 100644 --- a/apps/sim/lib/execution/cancel-workflow-execution.ts +++ b/apps/sim/lib/execution/cancel-workflow-execution.ts @@ -41,9 +41,12 @@ async function cancelActiveWorkflowJob(executionId: string): Promise { } /** - * 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'