fix(execution): report a workflow-group cancellation as the write it performed

Cancelling a workflow-group run whose log had already been cancelled, but whose
cell sidecar still needed reconciliation, durably cancelled that sidecar and
then reported `already_cancelled` with `durablyRecorded: false` — because the
terminal-status shortcut answered from the entry snapshot alone and never asked
what this request had written. The analytics event, which now gates on that
field, stopped firing for a cancellation that really happened.

The outcome a cancel reports is the same question whichever path answers it, so
there is now one vocabulary for it rather than one the direct claim tracked and
one the group transition did not. Every group result maps to that outcome
through a total map, so a new group result cannot compile without deciding what
it wrote, and the reclassification leads with whether this request wrote at all.

A group transition that reports itself already cancelled is deliberately mapped
as unknown rather than as a no-op: it leaves the sidecar alone but still
terminalizes a log that was active, and the result does not say which happened.
That costs nothing today, because the only snapshot that would reclassify proves
the log was already terminal.

Author: waleedlatif1

apps/sim/lib/execution/cancel-workflow-execution.test.ts

@@ -319,6 +319,109 @@ describe('cancelWorkflowExecution', () => {
     expect(mockReleaseExecutionSlot).toHaveBeenCalledWith('execution-1')
   })
 
+  /**
+   * A workflow-group log that is already `cancelled` can still own a cell
+   * sidecar left in `error`, and reconciling it to `cancelled` is a durable
+   * write this request performed. The terminal entry snapshot cannot see that
+   * work, so it must not reinterpret the outcome as a no-op — the API would
+   * otherwise tell the caller nothing changed and drop the cancellation event.
+   */
+  it('reports a durable write when a cancelled group run still had its sidecar reconciled', async () => {
+    mockResolveWorkflowExecutionOwnership.mockResolvedValue({
+      belongsToWorkflow: true,
+      workflowGroupWorkspaceId: 'workspace-1',
+      priorStatus: 'cancelled',
+    })
+    const cancelled = {
+      kind: 'cancelled' as const,
+      tableId: 'table-1',
+      rowId: 'row-1',
+      groupId: 'group-1',
+    }
+    mockCancelWorkflowGroupExecution.mockResolvedValue(cancelled)
+
+    const result = await cancelWorkflowExecution(INPUT)
+
+    expect(result).toMatchObject({ success: true, durablyRecorded: true, reason: 'recorded' })
+    expect(mockPublishWorkflowGroupCancellationEvent).toHaveBeenCalledWith(cancelled, 'execution-1')
+  })
+
+  /**
+   * The group path terminalizes the workflow log itself when the cell sidecar
+   * is already gone, so that outcome is a durable write too.
+   */
+  it('reports a durable write when the group path cancels a run whose sidecar is gone', async () => {
+    mockResolveWorkflowExecutionOwnership.mockResolvedValue({
+      belongsToWorkflow: true,
+      workflowGroupWorkspaceId: 'workspace-1',
+      priorStatus: 'running',
+    })
+    mockCancelWorkflowGroupExecution.mockResolvedValue({ kind: 'cancelled_without_sidecar' })
+
+    const result = await cancelWorkflowExecution(INPUT)
+
+    expect(result).toMatchObject({ success: true, durablyRecorded: true, reason: 'recorded' })
+    expect(mockUpdateSet).not.toHaveBeenCalled()
+  })
+
+  /**
+   * The mirror case: a group run that was already terminal and whose sidecar was
+   * already `cancelled` leaves both records untouched, so it still reports the
+   * state it observed rather than a durable write.
+   */
+  it('reports a group run that changed nothing as a no-op', async () => {
+    mockResolveWorkflowExecutionOwnership.mockResolvedValue({
+      belongsToWorkflow: true,
+      workflowGroupWorkspaceId: 'workspace-1',
+      priorStatus: 'cancelled',
+    })
+    mockCancelWorkflowGroupExecution.mockResolvedValue({
+      kind: 'already_cancelled',
+      tableId: 'table-1',
+      rowId: 'row-1',
+      groupId: 'group-1',
+    })
+
+    const result = await cancelWorkflowExecution(INPUT)
+
+    expect(result).toMatchObject({
+      success: true,
+      durablyRecorded: false,
+      reason: 'already_cancelled',
+    })
+  })
+
+  /**
+   * The lost-race re-read applies to the group path as well: an entry snapshot
+   * can still read `running` when the sidecar-less transition finds the log
+   * already `cancelled` and writes nothing.
+   */
+  it('reports a group run that lost the race to another cancel as a no-op', async () => {
+    mockResolveWorkflowExecutionOwnership
+      .mockResolvedValueOnce({
+        belongsToWorkflow: true,
+        workflowGroupWorkspaceId: 'workspace-1',
+        priorStatus: 'running',
+      })
+      .mockResolvedValueOnce({
+        belongsToWorkflow: true,
+        workflowGroupWorkspaceId: 'workspace-1',
+        priorStatus: 'cancelled',
+      })
+    mockCancelWorkflowGroupExecution.mockResolvedValue({
+      kind: 'already_cancelled_without_sidecar',
+    })
+
+    const result = await cancelWorkflowExecution(INPUT)
+
+    expect(result).toMatchObject({
+      success: true,
+      durablyRecorded: false,
+      reason: 'already_cancelled',
+    })
+    expect(mockResolveWorkflowExecutionOwnership).toHaveBeenCalledTimes(2)
+  })
+
   it.each([
     [{ kind: 'conflict' as const, status: 'completed' }, 'cannot be cancelled while completed'],
     [{ kind: 'not_workflow_group' as const }, 'no longer the active table execution'],

apps/sim/lib/execution/cancel-workflow-execution.ts

@@ -18,6 +18,7 @@ import {
   cancelWorkflowGroupExecution,
   type PublishableWorkflowGroupCancellation,
   publishWorkflowGroupCancellationEvent,
+  type WorkflowGroupExecutionCancellationResult,
 } from '@/lib/table/workflow-group-cancellation'
 import { WORKFLOW_EXECUTION_JOB_ID_PREFIX } from '@/lib/workflows/executor/execution-job-ids'
 import { resolveWorkflowExecutionOwnership } from '@/lib/workflows/executor/execution-queries'
@@ -79,24 +80,53 @@ function toTerminalExecutionStatus(
 }
 
 /**
- * What the direct terminal log claim did: moved the run to `cancelled` here and
- * now, matched no row, or never ran — the workflow-group and paused paths write
- * their terminal row elsewhere, and a failed statement knows nothing either way.
+ * What this request's own terminal claim did: moved the run to `cancelled` here
+ * and now, provably matched no row, or ran on a path that cannot tell. Every
+ * path that can terminalize the run — the direct log claim and the
+ * workflow-group transition — answers in this one vocabulary, so the report can
+ * ask a single question: did this request durably write?
  */
-type DirectTerminalWriteOutcome = 'applied' | 'no_row' | 'unknown'
+type TerminalWriteOutcome = 'applied' | 'no_row' | 'unknown'
+
+/**
+ * What a returned workflow-group transition durably wrote. `cancelled` claims
+ * the cell sidecar and `cancelled_without_sidecar` terminalizes the workflow log
+ * itself, so both are writes this request performed — including the reconciling
+ * cancel of a sidecar left in `error` behind an already-`cancelled` log.
+ * `already_cancelled_without_sidecar` found the log already `cancelled` and
+ * touched nothing. `already_cancelled` left the sidecar alone but may still have
+ * terminalized an active workflow log, which the result does not distinguish, so
+ * it cannot claim either way. `conflict` and `not_workflow_group` never reach
+ * the report — both throw above — and are mapped only to keep this map total.
+ */
+const WORKFLOW_GROUP_TERMINAL_WRITES = {
+  cancelled: 'applied',
+  cancelled_without_sidecar: 'applied',
+  already_cancelled: 'unknown',
+  already_cancelled_without_sidecar: 'no_row',
+  conflict: 'unknown',
+  not_workflow_group: 'unknown',
+} as const satisfies Record<WorkflowGroupExecutionCancellationResult['kind'], TerminalWriteOutcome>
 
 /**
  * Names the terminal state the cancel could not move, or `null` when it did
  * real work or when this path cannot tell — in which case the caller keeps the
  * undifferentiated report rather than guessing.
  *
- * The status read at entry is not enough on its own: a run that finishes
- * between that read and the claim leaves a stale non-terminal snapshot behind a
- * cancel that wrote nothing. The claim's own row count settles that, and a
- * plain post-read cannot: after a successful cancel the row reads `cancelled`
- * too, so the state has to be attributed to whoever wrote it. A claim that
- * moved no row against a non-terminal snapshot re-reads the row it lost the
- * race to, through the same ownership query the entry read came from.
+ * A request that durably wrote is never a no-op, whatever the entry snapshot
+ * said. A run can be terminal at entry and still owe this request a real write:
+ * a workflow-group run whose log is already `cancelled` can carry a sidecar left
+ * in `error`, and reconciling it is a durable cancellation that the entry
+ * snapshot cannot see.
+ *
+ * The status read at entry is not enough on its own in the other direction
+ * either: a run that finishes between that read and the claim leaves a stale
+ * non-terminal snapshot behind a cancel that wrote nothing. The claim's own row
+ * count settles that, and a plain post-read cannot: after a successful cancel
+ * the row reads `cancelled` too, so the state has to be attributed to whoever
+ * wrote it. A claim that moved no row against a non-terminal snapshot re-reads
+ * the row it lost the race to, through the same ownership query the entry read
+ * came from.
  *
  * Purely observational — it gates no effect, and a read failure falls back to
  * the undifferentiated report rather than failing the cancel.
@@ -105,10 +135,11 @@ async function resolveTerminalNoOpReason(
   executionId: string,
   workflowId: string,
   priorTerminalStatus: TerminalExecutionStatus | null,
-  directTerminalWrite: DirectTerminalWriteOutcome
+  terminalWrite: TerminalWriteOutcome
 ): Promise<CancelWorkflowExecutionReason | null> {
+  if (terminalWrite === 'applied') return null
   if (priorTerminalStatus !== null) return TERMINAL_NO_OP_REASONS[priorTerminalStatus]
-  if (directTerminalWrite !== 'no_row') return null
+  if (terminalWrite !== 'no_row') return null
   try {
     const { priorStatus } = await resolveWorkflowExecutionOwnership(executionId, workflowId)
     const terminalStatus = toTerminalExecutionStatus(priorStatus)
@@ -441,9 +472,10 @@ export async function cancelWorkflowExecution(
    * The claim's row count is read back only to report it — `returning` changes
    * what the statement returns, never the row it writes or the rows it matches.
    */
-  let directTerminalWrite: DirectTerminalWriteOutcome = 'unknown'
-  if (
-    groupCancellation === null &&
+  let terminalWrite: TerminalWriteOutcome = 'unknown'
+  if (groupCancellation !== null) {
+    terminalWrite = WORKFLOW_GROUP_TERMINAL_WRITES[groupCancellation.kind]
+  } else if (
     (cancellation.durablyRecorded || queuedJobCancelled || locallyAborted) &&
     !pausedCancelled
   ) {
@@ -459,7 +491,7 @@ export async function cancelWorkflowExecution(
           )
         )
         .returning({ id: workflowExecutionLogs.id })
-      directTerminalWrite = claimedRows.length > 0 ? 'applied' : 'no_row'
+      terminalWrite = claimedRows.length > 0 ? 'applied' : 'no_row'
     } catch (dbError) {
       logger.warn('Failed to update execution log status directly', {
         executionId,
@@ -510,16 +542,12 @@ export async function cancelWorkflowExecution(
    * run — cancelled, or force-failed with paused state left behind — can still
    * carry real paused-HITL reconciliation work, and a genuine failure there owes
    * the caller the step that failed, not a no-op. So only an otherwise-clean
-   * `recorded` is a candidate, whatever the prior status was.
+   * `recorded` is a candidate, whatever the prior status was — and only when
+   * this request wrote nothing durable on any path.
    */
   const terminalNoOpReason =
     reason === 'recorded' && !pausedCancelled
-      ? await resolveTerminalNoOpReason(
-          executionId,
-          workflowId,
-          priorTerminalStatus,
-          directTerminalWrite
-        )
+      ? await resolveTerminalNoOpReason(executionId, workflowId, priorTerminalStatus, terminalWrite)
       : null
 
   return {