fix(execution): report cancellation durability truthfully

[PlaneInABottle]

Summary

• make the cancel-execution response reflect whether the Redis-backed cancellation marker was durably recorded
• return durability metadata (durablyRecorded, reason) and avoid misleading success logging when persistence fails
• fix the targeted cancellation tests by hoisting Vitest mocks in cancellation.test.ts

Problem / current behavior

• the cancel-execution API previously implied success after attempting cancellation even when the Redis-backed marker was not durably recorded
• when Redis was unavailable or the write failed, callers and operators could read the response/logging as a successful cancellation even though no durable cancellation marker existed
• the targeted cancellation tests also needed a follow-up fix so the mocked module state was initialized correctly under Vitest

Proposed fix / behavior after

• update the cancellation helper to return structured durability results instead of a bare boolean
• make the cancel route set success based on whether the cancellation marker was durably recorded, and include durablyRecorded plus a failure reason in the response
• downgrade the non-durable path from misleading success reporting to warning-level reporting
• hoist the Vitest mocks in apps/sim/lib/execution/cancellation.test.ts so the targeted tests reliably validate the success and failure outcomes

Why this matters

• prevents false-positive cancellation reporting
• makes Redis availability/write failures visible and actionable to API consumers and operators
• preserves regression coverage for the durability contract this branch introduces

Files changed

• apps/sim/app/api/workflows/[id]/executions/[executionId]/cancel/route.ts
• apps/sim/app/api/workflows/[id]/executions/[executionId]/cancel/route.test.ts
• apps/sim/lib/execution/cancellation.ts
• apps/sim/lib/execution/cancellation.test.ts

Tests run

• cd /Users/Y_ALTAY1/Documents/simWoot/sim/apps/sim && bun run test -- lib/execution/cancellation.test.ts 'app/api/workflows/[id]/executions/[executionId]/cancel/route.test.ts'

Notes / non-goals / follow-ups

• this PR is intentionally scoped to truthful cancellation durability reporting plus the follow-up test-only fix
• it does not redesign broader cancellation orchestration, retries, or worker-side handling beyond surfacing whether the Redis-backed marker was durably written
• the second commit on this branch is test-only and limited to hoisting Vitest mocks in cancellation.test.ts

[cursor]

PR Summary

Medium Risk
Changes the cancel API response contract and the markExecutionCancelled return type, which may affect existing callers that assumed a boolean or unconditional success. Logic is straightforward but touches execution cancellation and operational signaling (logging/redis failure modes).

Overview
Makes execution cancellation reporting truthful about Redis durability. markExecutionCancelled now returns a structured result (durablyRecorded + reason) instead of a boolean, distinguishing redis_unavailable vs redis_write_failed.

Updates the cancel-execution route to set success based on durable recording, return the new metadata (durablyRecorded, reason, derived redisAvailable), and emit a warning when the cancellation marker cannot be persisted. Adds/updates Vitest coverage for both the helper and the API route across success and failure scenarios.

^{Written by Cursor Bugbot for commit 4f22a7e. Configure here.}

[vercel]

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

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
docs	Skipped		Mar 12, 2026 11:25pm

[icecrasher321]

@PlaneInABottle please make this point towards staging as well

[greptile-apps]

Greptile Summary

This PR improves cancellation durability reporting by refactoring markExecutionCancelled to return a structured discriminated union (ExecutionCancellationRecordResult) instead of a bare boolean, and propagates that result through the cancel API route so callers can distinguish between "durably recorded", "Redis unavailable", and "Redis write failed" outcomes. The fix is well-scoped and the core cancellation.ts changes are clean; however, one semantic issue exists in the route response.

Key changes:

• cancellation.ts: markExecutionCancelled now returns { durablyRecorded, reason } instead of boolean; the exported discriminated union type is a good contract
• route.ts: success is now gated on durablyRecorded, and a reason field surfaces the failure mode — but redisAvailable is incorrectly set to cancellation.durablyRecorded, conflating "is Redis reachable" with "did the write succeed"
• route.test.ts / cancellation.test.ts: New test files with vi.hoisted mocks correctly cover all three outcome variants, though the route tests mirror the redisAvailable bug

Confidence Score: 3/5

• Safe to merge with a minor fix — the redisAvailable field in the response incorrectly reports false when Redis is reachable but the write fails, misleading API consumers.
• The core logic in cancellation.ts is correct and well-typed. The route improvement is meaningful (truthful success flag, reason field). The only defect is that redisAvailable is aliased to durablyRecorded rather than being derived from reason !== 'redis_unavailable', producing a self-contradictory response body in the redis_write_failed path. The tests reinforce this incorrect behaviour, so the bug would persist without a targeted fix.
• apps/sim/app/api/workflows/[id]/executions/[executionId]/cancel/route.ts (line 52) and the corresponding assertion in route.test.ts (lines 77 and 99).

Important Files Changed

Filename	Overview
apps/sim/app/api/workflows/[id]/executions/[executionId]/cancel/route.ts	Durability reporting upgraded to use structured result; however redisAvailable is incorrectly aliased to durablyRecorded, misreporting Redis availability when the write fails after a successful connection.
apps/sim/lib/execution/cancellation.ts	Clean refactor: markExecutionCancelled now returns a discriminated union ExecutionCancellationRecordResult with durablyRecorded and typed reason; all three paths (unavailable, success, write error) are correctly handled.
apps/sim/app/api/workflows/[id]/executions/[executionId]/cancel/route.test.ts	New test file with good coverage of all three cancellation outcomes; the redisAvailable: false expectation for the redis_write_failed case mirrors the bug in the route, so it will pass but test incorrect behaviour.
apps/sim/lib/execution/cancellation.test.ts	New test file; mocks are properly hoisted with vi.hoisted, covering all three result variants of the refactored markExecutionCancelled.

Sequence Diagram

sequenceDiagram
    participant Client
    participant CancelRoute as POST /cancel
    participant markExecutionCancelled
    participant Redis

    Client->>CancelRoute: POST (workflowId, executionId)
    CancelRoute->>CancelRoute: checkHybridAuth
    CancelRoute->>CancelRoute: authorizeWorkflowByWorkspacePermission
    CancelRoute->>markExecutionCancelled: markExecutionCancelled(executionId)

    alt Redis client is null
        markExecutionCancelled-->>CancelRoute: { durablyRecorded: false, reason: 'redis_unavailable' }
    else Redis client exists
        markExecutionCancelled->>Redis: SET execution:cancel:{id} 1 EX 3600
        alt Write succeeds
            Redis-->>markExecutionCancelled: OK
            markExecutionCancelled-->>CancelRoute: { durablyRecorded: true, reason: 'recorded' }
        else Write throws
            Redis-->>markExecutionCancelled: Error
            markExecutionCancelled-->>CancelRoute: { durablyRecorded: false, reason: 'redis_write_failed' }
        end
    end

    CancelRoute-->>Client: 200 { success, executionId, redisAvailable, durablyRecorded, reason }

Loading

_{Last reviewed commit: 47dadaa}

[PlaneInABottle]

@icecrasher321 Done — the PR has been retargeted to staging.

[icecrasher321]

bugbot run

[cursor]

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR