Skip to content

fix(api): widen the cancel-execution reason contract to what the route emits - #6592

Merged
waleedlatif1 merged 1 commit into
stagingfrom
fix/cancel-reason-contract
Aug 12, 2026
Merged

fix(api): widen the cancel-execution reason contract to what the route emits#6592
waleedlatif1 merged 1 commit into
stagingfrom
fix/cancel-reason-contract

Conversation

@waleedlatif1

@waleedlatif1 waleedlatif1 commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • cancelWorkflowExecutionReasonSchema enumerated only the five reasons the cancellation service produces, but the internal cancel route mints four more (queue_cancelled, already_cancelled, active_resume_signal_failed, cancellation_not_finalized). requestJson validates every 2xx against the contract, so a stop that genuinely applied threw on the client — killing pollForTerminalExecution and rolling back the optimistic cancelling row in the logs list
  • Regression vs main, introduced by improvement(external-endpoints): v2 versions with clean signatures + updated docs based on openapi spec #5273, which tightened reason from z.string().optional() to the enum without touching the route. Not on main, so it never reached prod
  • Kept the service enum narrow for the public v2 contract — cancelWorkflowRun delegates wholly to the service and structurally cannot emit the other four, and the v2 OpenAPI schema documents the enum value by value. Added a superset enum for the internal contract instead. check:openapi confirms the published spec is unchanged
  • Routed the route's seven success bodies through one contract-typed constructor, so an unknown reason is now a compile error rather than a runtime surprise
  • Deleted the dead duplicate cancelWorkflowExecutionContract in contracts/logs.ts — zero importers, and a stricter wrong copy (narrow enum and reason required), waiting to reintroduce this exact bug

Type of Change

  • Bug fix

Testing

  • route.test.ts now drives every case through a wrapper that parses each success body against the contract, so all 47 tests are contract-checked
  • Verified the tests can fail: reverting the enum turns 11 red, including "treats an already-cancelled execution as an idempotent success" — the user-visible bug
  • Verified the compile-time guard: an invented reason fails typecheck at a site no test exercises (paused_database_cancel_failed appears in zero tests, so the runtime wrapper alone did not cover it)
  • 2137 tests pass across lib/api, app/api/workflows, hooks/queries, lib/execution, lib/logs
  • check:api-validation:strict, check:openapi, and bun run lint all pass

Follow-up (not in this PR)

968 of 1098 route.ts files export a raw withRouteHandler, and 637 of those already parseRequest a contract while never validating their response — this route was one instance. The tractable lever is a ratcheted counter in scripts/check-api-validation-contracts.ts for that class. Migrating this route to defineInternalJsonRoute is separately blocked: no builder auth policy covers checkHybridAuth, and its ~600 lines of orchestration have no application use case yet.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@waleedlatif1
fix(api): widen the cancel-execution reason contract to what the rout…
fac8b3b 
…e emits

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.
@vercel

vercel Bot commented Aug 12, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Aug 12, 2026 4:17am

Request Review

@cursor

cursor Bot commented Aug 12, 2026 •
edited
Loading

Copy link
Copy Markdown

PR Summary

Low Risk
Contract/schema alignment only; cancellation behavior is unchanged and the published v2 OpenAPI enum stays the same. Widening the internal response enum is the safe direction for client validation.

Overview
Fixes a client-side validation bug where successful cancel responses were rejected because the contract only allowed the five cancellation-service reasons, while the internal cancel route also emits queue_cancelled, already_cancelled, active_resume_signal_failed, and cancellation_not_finalized.

Splits the reason vocabulary: the public v2 schema stays on the narrow service enum, and the internal contract uses a superset. Route success bodies now go through a typed cancellationOutcome helper, and the route tests parse every 2xx against the contract. Also removes a dead, stricter duplicate cancel contract from contracts/logs.ts.

Reviewed by Cursor Bugbot for commit fac8b3b. Configure here.

@greptile-apps

greptile-apps Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR aligns the internal cancellation response contract with all outcomes already emitted by its route while preserving the narrower public v2 API vocabulary.

  • Adds a dedicated internal superset of cancellation reasons.
  • Routes cancellation response bodies through a contract-typed constructor.
  • Adds runtime contract checks to route tests and verifies separation from the public enum.
  • Removes the unused duplicate cancellation contract from the logs contract module.

Confidence Score: 5/5

The PR appears safe to merge; the internal and public cancellation contracts match their respective reachable response vocabularies.

All internal route response shapes conform to the widened schema, the public v2 endpoint remains constrained to service-produced reasons, and removal of the duplicate logs contract leaves no unresolved consumers.

Important Files Changed
Filename Overview
apps/sim/app/api/workflows/[id]/executions/[executionId]/cancel/route.ts Centralizes seven cancellation response sites behind a typed constructor; all reachable response reasons and required fields conform to the internal contract.
apps/sim/lib/api/contracts/workflows.ts Adds a nine-value internal reason schema while retaining the service’s five-value schema for the public v2 cancellation contract.
apps/sim/app/api/workflows/[id]/executions/[executionId]/cancel/route.test.ts Wraps route calls so every successful test response is parsed against the client-enforced contract.
apps/sim/lib/api/contracts/workflows.test.ts Verifies that internal-only reasons remain excluded from the public cancellation enum.
apps/sim/lib/api/contracts/logs.ts Removes an unused duplicate cancellation contract with no remaining importers.
apps/sim/lib/execution/cancel-workflow-execution.ts Clarifies that the service reason type defines the complete vocabulary reachable through the public v2 path.

Reviews (1): Last reviewed commit: "fix(api): widen the cancel-execution rea..." | Re-trigger Greptile

@waleedlatif1
waleedlatif1 merged commit a1a05c6 into staging Aug 12, 2026
30 checks passed
@waleedlatif1
waleedlatif1 deleted the fix/cancel-reason-contract branch August 12, 2026 04:22
@waleedlatif1 waleedlatif1 mentioned this pull request Aug 12, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@waleedlatif1