fix(execution): have a workflow-group cancellation report the writes it made

Three review findings landed on the same reporting logic, each a different face
of one cause: the caller could not see what the group transaction had written, so
it inferred. It inferred from an entry snapshot, then from the returned kind, and
the remaining blind spot was the kind that covers two different transactions —
a repair that terminalizes an active log, and a genuine no-op — which left a
cancel that wrote nothing still claiming a durable write when it lost a race.

The transaction now reports both writes it can make, each read from that
statement's own returning row and recorded immediately before the throw that
already depended on it, so the report cannot drift from the write. The caller
derives its outcome from those rather than from the kind, and the kind is back to
naming the situation instead of standing in for the work.

The group path can now always answer whether it wrote. The only remaining
unknown is the direct claim when its update throws or is never attempted, which
genuinely has no row count to report.

Author: waleedlatif1

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

@@ -101,6 +101,16 @@ vi.mock('@/lib/workflows/executor/human-in-the-loop-manager', () => ({
 
 import { cancelWorkflowExecution } from '@/lib/execution/cancel-workflow-execution'
 
+/**
+ * The durable writes a workflow-group transition reports back. The transaction
+ * updates the workflow log only, the cell sidecar only, or both, so a single
+ * `kind` cannot answer whether this request wrote anything.
+ */
+const NO_WRITES = { workflowLogTerminalized: false, sidecarCancelled: false } as const
+const LOG_WRITE = { workflowLogTerminalized: true, sidecarCancelled: false } as const
+const SIDECAR_WRITE = { workflowLogTerminalized: false, sidecarCancelled: true } as const
+const BOTH_WRITES = { workflowLogTerminalized: true, sidecarCancelled: true } as const
+
 const INPUT = {
   executionId: 'execution-1',
   workflowId: 'workflow-1',
@@ -303,6 +313,7 @@ describe('cancelWorkflowExecution', () => {
       tableId: 'table-1',
       rowId: 'row-1',
       groupId: 'group-1',
+      writes: BOTH_WRITES,
     }
     mockCancelWorkflowGroupExecution.mockResolvedValue(cancelled)
 
@@ -337,6 +348,7 @@ describe('cancelWorkflowExecution', () => {
       tableId: 'table-1',
       rowId: 'row-1',
       groupId: 'group-1',
+      writes: SIDECAR_WRITE,
     }
     mockCancelWorkflowGroupExecution.mockResolvedValue(cancelled)
 
@@ -356,7 +368,10 @@ describe('cancelWorkflowExecution', () => {
       workflowGroupWorkspaceId: 'workspace-1',
       priorStatus: 'running',
     })
-    mockCancelWorkflowGroupExecution.mockResolvedValue({ kind: 'cancelled_without_sidecar' })
+    mockCancelWorkflowGroupExecution.mockResolvedValue({
+      kind: 'cancelled_without_sidecar',
+      writes: LOG_WRITE,
+    })
 
     const result = await cancelWorkflowExecution(INPUT)
 
@@ -380,6 +395,71 @@ describe('cancelWorkflowExecution', () => {
       tableId: 'table-1',
       rowId: 'row-1',
       groupId: 'group-1',
+      writes: NO_WRITES,
+    })
+
+    const result = await cancelWorkflowExecution(INPUT)
+
+    expect(result).toMatchObject({
+      success: true,
+      durablyRecorded: false,
+      reason: 'already_cancelled',
+    })
+  })
+
+  /**
+   * The same `already_cancelled` kind covers a transition that left the sidecar
+   * alone but still terminalized an active workflow log. That log write is
+   * durable, so the outcome must stay `recorded` and must not re-read a state
+   * this request itself wrote.
+   */
+  it('reports a durable write when a group run only repaired its workflow log', async () => {
+    mockResolveWorkflowExecutionOwnership.mockResolvedValue({
+      belongsToWorkflow: true,
+      workflowGroupWorkspaceId: 'workspace-1',
+      priorStatus: 'running',
+    })
+    mockCancelWorkflowGroupExecution.mockResolvedValue({
+      kind: 'already_cancelled',
+      tableId: 'table-1',
+      rowId: 'row-1',
+      groupId: 'group-1',
+      writes: LOG_WRITE,
+    })
+
+    const result = await cancelWorkflowExecution(INPUT)
+
+    expect(result).toMatchObject({ success: true, durablyRecorded: true, reason: 'recorded' })
+    expect(mockResolveWorkflowExecutionOwnership).toHaveBeenCalledOnce()
+  })
+
+  /**
+   * The lost race the sidecar-bearing kind used to hide: a concurrent cancel
+   * terminalized both records between the entry snapshot and this transaction,
+   * which then found the sidecar already `cancelled` and the log already
+   * `cancelled` and wrote nothing. A non-terminal entry snapshot cannot catch
+   * that, so the transition's own report of having written nothing is what
+   * forces the re-read — otherwise the request would claim a durable write and
+   * fire the v2 cancel analytics gate on a no-op.
+   */
+  it('reports a group run that lost the race with its sidecar already cancelled 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',
+      tableId: 'table-1',
+      rowId: 'row-1',
+      groupId: 'group-1',
+      writes: NO_WRITES,
     })
 
     const result = await cancelWorkflowExecution(INPUT)
@@ -389,6 +469,7 @@ describe('cancelWorkflowExecution', () => {
       durablyRecorded: false,
       reason: 'already_cancelled',
     })
+    expect(mockResolveWorkflowExecutionOwnership).toHaveBeenCalledTimes(2)
   })
 
   /**
@@ -410,6 +491,7 @@ describe('cancelWorkflowExecution', () => {
       })
     mockCancelWorkflowGroupExecution.mockResolvedValue({
       kind: 'already_cancelled_without_sidecar',
+      writes: NO_WRITES,
     })
 
     const result = await cancelWorkflowExecution(INPUT)
@@ -423,8 +505,14 @@ describe('cancelWorkflowExecution', () => {
   })
 
   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'],
+    [
+      { kind: 'conflict' as const, status: 'completed', writes: NO_WRITES },
+      'cannot be cancelled while completed',
+    ],
+    [
+      { kind: 'not_workflow_group' as const, writes: NO_WRITES },
+      'no longer the active table execution',
+    ],
   ])(
     'releases the reservation before reporting a refused workflow-group cell claim as a conflict',
     async (outcome, message) => {
@@ -444,8 +532,8 @@ describe('cancelWorkflowExecution', () => {
   )
 
   it.each([
-    [{ kind: 'conflict' as const, status: 'completed' }],
-    [{ kind: 'not_workflow_group' as const }],
+    [{ kind: 'conflict' as const, status: 'completed', writes: NO_WRITES }],
+    [{ kind: 'not_workflow_group' as const, writes: NO_WRITES }],
   ])(
     'keeps the reservation held when a refused claim follows a paused cancellation',
     async (outcome) => {
@@ -462,8 +550,8 @@ describe('cancelWorkflowExecution', () => {
   )
 
   it.each([
-    [{ kind: 'conflict' as const, status: 'completed' }],
-    [{ kind: 'not_workflow_group' as const }],
+    [{ kind: 'conflict' as const, status: 'completed', writes: NO_WRITES }],
+    [{ kind: 'not_workflow_group' as const, writes: NO_WRITES }],
   ])(
     'keeps the reservation held when a refused claim follows a failed cancellation',
     async (outcome) => {

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

@@ -18,7 +18,7 @@ import {
   cancelWorkflowGroupExecution,
   type PublishableWorkflowGroupCancellation,
   publishWorkflowGroupCancellationEvent,
-  type WorkflowGroupExecutionCancellationResult,
+  type WorkflowGroupCancellationWrites,
 } 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'
@@ -85,28 +85,24 @@ function toTerminalExecutionStatus(
  * 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?
+ *
+ * Only the direct claim ever answers `unknown`, and only when it could not run
+ * or its statement failed. The workflow-group transition always knows: it
+ * reports the writes it performed.
  */
 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.
+ * Reads a workflow-group transition's durability off the writes it reported
+ * rather than off its `kind`. Terminalizing the workflow log and cancelling the
+ * cell sidecar are each a durable write this request performed, and a single
+ * `kind` covers both a transition that did one of them and one that did
+ * neither: `already_cancelled` leaves a sidecar that was already `cancelled`
+ * alone, but may still have terminalized an active workflow log.
  */
-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>
+function toTerminalWriteOutcome(writes: WorkflowGroupCancellationWrites): TerminalWriteOutcome {
+  return writes.workflowLogTerminalized || writes.sidecarCancelled ? 'applied' : 'no_row'
+}
 
 /**
  * Names the terminal state the cancel could not move, or `null` when it did
@@ -474,7 +470,7 @@ export async function cancelWorkflowExecution(
    */
   let terminalWrite: TerminalWriteOutcome = 'unknown'
   if (groupCancellation !== null) {
-    terminalWrite = WORKFLOW_GROUP_TERMINAL_WRITES[groupCancellation.kind]
+    terminalWrite = toTerminalWriteOutcome(groupCancellation.writes)
   } else if (
     (cancellation.durablyRecorded || queuedJobCancelled || locallyAborted) &&
     !pausedCancelled

apps/sim/lib/table/workflow-group-cancellation.test.ts

@@ -23,6 +23,8 @@ const OPTIONS = {
   executionId: 'execution-1',
 }
 
+const NO_WRITES = { workflowLogTerminalized: false, sidecarCancelled: false } as const
+
 const ACTIVE_TARGET = {
   tableId: 'table-1',
   rowId: 'row-1',
@@ -64,6 +66,7 @@ describe('cancelWorkflowGroupExecution', () => {
       rowId: 'row-1',
       groupId: 'group-1',
       blockErrors: { 'block-1': 'Provider failed' },
+      writes: { workflowLogTerminalized: true, sidecarCancelled: true },
     })
 
     expect(dbChainMockFns.transaction).toHaveBeenCalledOnce()
@@ -165,6 +168,7 @@ describe('cancelWorkflowGroupExecution', () => {
 
     await expect(cancelWorkflowGroupExecution(OPTIONS)).resolves.toEqual({
       kind: 'not_workflow_group',
+      writes: NO_WRITES,
     })
 
     expect(dbChainMockFns.update).not.toHaveBeenCalled()
@@ -178,6 +182,7 @@ describe('cancelWorkflowGroupExecution', () => {
 
     await expect(cancelWorkflowGroupExecution(OPTIONS)).resolves.toEqual({
       kind: 'not_workflow_group',
+      writes: NO_WRITES,
     })
 
     expect(dbChainMockFns.update).not.toHaveBeenCalled()
@@ -197,6 +202,7 @@ describe('cancelWorkflowGroupExecution', () => {
 
     await expect(cancelWorkflowGroupExecution(OPTIONS)).resolves.toEqual({
       kind: 'not_workflow_group',
+      writes: NO_WRITES,
     })
 
     expect(dbChainMockFns.update).not.toHaveBeenCalled()
@@ -216,6 +222,7 @@ describe('cancelWorkflowGroupExecution', () => {
 
     await expect(cancelWorkflowGroupExecution(OPTIONS)).resolves.toEqual({
       kind: 'cancelled_without_sidecar',
+      writes: { workflowLogTerminalized: true, sidecarCancelled: false },
     })
 
     expect(dbChainMockFns.update).toHaveBeenCalledOnce()
@@ -249,6 +256,7 @@ describe('cancelWorkflowGroupExecution', () => {
 
     await expect(cancelWorkflowGroupExecution(OPTIONS)).resolves.toEqual({
       kind: 'already_cancelled_without_sidecar',
+      writes: NO_WRITES,
     })
 
     expect(dbChainMockFns.update).not.toHaveBeenCalled()
@@ -269,6 +277,7 @@ describe('cancelWorkflowGroupExecution', () => {
 
     await expect(cancelWorkflowGroupExecution(OPTIONS)).resolves.toEqual({
       kind: 'not_workflow_group',
+      writes: NO_WRITES,
     })
 
     expect(dbChainMockFns.update).not.toHaveBeenCalled()
@@ -288,6 +297,7 @@ describe('cancelWorkflowGroupExecution', () => {
     await expect(cancelWorkflowGroupExecution(OPTIONS)).resolves.toEqual({
       kind: 'conflict',
       status: 'completed',
+      writes: NO_WRITES,
     })
 
     expect(dbChainMockFns.update).not.toHaveBeenCalled()
@@ -318,6 +328,7 @@ describe('cancelWorkflowGroupExecution', () => {
     await expect(cancelWorkflowGroupExecution(OPTIONS)).resolves.toEqual({
       kind: 'conflict',
       status: 'completed',
+      writes: NO_WRITES,
     })
 
     expect(dbChainMockFns.update).not.toHaveBeenCalled()
@@ -335,6 +346,7 @@ describe('cancelWorkflowGroupExecution', () => {
       rowId: 'row-1',
       groupId: 'group-1',
       blockErrors: { 'block-1': 'Provider failed' },
+      writes: NO_WRITES,
     })
 
     expect(dbChainMockFns.update).not.toHaveBeenCalled()
@@ -353,6 +365,7 @@ describe('cancelWorkflowGroupExecution', () => {
       rowId: 'row-1',
       groupId: 'group-1',
       blockErrors: { 'block-1': 'Provider failed' },
+      writes: { workflowLogTerminalized: false, sidecarCancelled: true },
     })
 
     expect(dbChainMockFns.update).toHaveBeenCalledOnce()
@@ -371,6 +384,7 @@ describe('cancelWorkflowGroupExecution', () => {
       rowId: 'row-1',
       groupId: 'group-1',
       blockErrors: { 'block-1': 'Provider failed' },
+      writes: { workflowLogTerminalized: false, sidecarCancelled: true },
     })
 
     const sidecarUpdateValues = collectConditionValues(dbChainMockFns.where.mock.calls[2]?.[0])
@@ -395,6 +409,7 @@ describe('cancelWorkflowGroupExecution', () => {
     await expect(cancelWorkflowGroupExecution(OPTIONS)).resolves.toEqual({
       kind: 'conflict',
       status: 'error',
+      writes: NO_WRITES,
     })
 
     expect(dbChainMockFns.update).not.toHaveBeenCalled()
@@ -413,6 +428,7 @@ describe('cancelWorkflowGroupExecution', () => {
       rowId: 'row-1',
       groupId: 'group-1',
       blockErrors: { 'block-1': 'Provider failed' },
+      writes: { workflowLogTerminalized: true, sidecarCancelled: false },
     })
 
     expect(dbChainMockFns.update).toHaveBeenCalledOnce()
@@ -429,6 +445,7 @@ describe('cancelWorkflowGroupExecution', () => {
       await expect(cancelWorkflowGroupExecution(OPTIONS)).resolves.toEqual({
         kind: 'conflict',
         status,
+        writes: NO_WRITES,
       })
 
       expect(dbChainMockFns.update).not.toHaveBeenCalled()