Skip to content

Commit 165acd5

Browse files
fix: cancel.go CTE dedup, ErrNoRows distinction, step audit events; test assertion upgrade
- Extract repeated WITH RECURSIVE children CTE into childrenCTE constant - Distinguish sql.ErrNoRows from other DB errors in zero-update path - Collect cancelled step IDs via RETURNING and emit audit events for each - Replace t.Logf with t.Fatalf in depth-limit test so regressions fail the test Co-authored-by: Michael McNees <michaelmcnees@users.noreply.github.com>
1 parent 8655bcd commit 165acd5

2 files changed

Lines changed: 44 additions & 17 deletions

File tree

packages/engine/internal/cli/cancel.go

Lines changed: 43 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package cli
22

33
import (
4+
"database/sql"
45
"fmt"
56
"time"
67

@@ -10,6 +11,14 @@ import (
1011
"github.com/spf13/cobra"
1112
)
1213

14+
// childrenCTE is a recursive CTE that selects an execution and all its descendants.
15+
const childrenCTE = `WITH RECURSIVE children AS (
16+
SELECT id FROM workflow_executions WHERE id = $1
17+
UNION ALL
18+
SELECT e.id FROM workflow_executions e
19+
JOIN children c ON e.parent_execution_id = c.id
20+
)`
21+
1322
func newCancelCommand() *cobra.Command {
1423
return &cobra.Command{
1524
Use: "cancel <execution-id>",
@@ -39,12 +48,7 @@ func newCancelCommand() *cobra.Command {
3948
// Cancel the execution and all child executions recursively.
4049
// Use RETURNING id to capture which executions were cancelled.
4150
rows, err := tx.QueryContext(cmd.Context(),
42-
`WITH RECURSIVE children AS (
43-
SELECT id FROM workflow_executions WHERE id = $1
44-
UNION ALL
45-
SELECT e.id FROM workflow_executions e
46-
JOIN children c ON e.parent_execution_id = c.id
47-
)
51+
childrenCTE+`
4852
UPDATE workflow_executions SET status = 'cancelled', completed_at = NOW(), updated_at = NOW()
4953
WHERE id IN (SELECT id FROM children) AND status IN ('pending', 'running', 'queued')
5054
RETURNING id`,
@@ -74,28 +78,41 @@ func newCancelCommand() *cobra.Command {
7478
err := tx.QueryRowContext(cmd.Context(),
7579
`SELECT status FROM workflow_executions WHERE id = $1`, execID,
7680
).Scan(&status)
77-
if err != nil {
81+
if err == sql.ErrNoRows {
7882
return fmt.Errorf("execution %q not found", execID)
7983
}
84+
if err != nil {
85+
return fmt.Errorf("checking execution status: %w", err)
86+
}
8087
fmt.Fprintf(cmd.OutOrStdout(), "Execution %s is already %s\n", execID, status)
8188
return nil
8289
}
8390

84-
// Also mark any running/pending steps in the tree as cancelled.
85-
_, err = tx.ExecContext(cmd.Context(),
86-
`WITH RECURSIVE children AS (
87-
SELECT id FROM workflow_executions WHERE id = $1
88-
UNION ALL
89-
SELECT e.id FROM workflow_executions e
90-
JOIN children c ON e.parent_execution_id = c.id
91-
)
91+
// Also mark any running/pending steps in the tree as cancelled,
92+
// collecting their IDs for audit events.
93+
stepRows, err := tx.QueryContext(cmd.Context(),
94+
childrenCTE+`
9295
UPDATE step_executions SET status = 'cancelled', completed_at = NOW(), updated_at = NOW()
93-
WHERE execution_id IN (SELECT id FROM children) AND status IN ('pending', 'running')`,
96+
WHERE execution_id IN (SELECT id FROM children) AND status IN ('pending', 'running')
97+
RETURNING id`,
9498
execID,
9599
)
96100
if err != nil {
97101
return fmt.Errorf("cancelling step executions: %w", err)
98102
}
103+
var cancelledStepIDs []string
104+
for stepRows.Next() {
105+
var id string
106+
if err := stepRows.Scan(&id); err != nil {
107+
stepRows.Close()
108+
return fmt.Errorf("scanning cancelled step id: %w", err)
109+
}
110+
cancelledStepIDs = append(cancelledStepIDs, id)
111+
}
112+
stepRows.Close()
113+
if err := stepRows.Err(); err != nil {
114+
return fmt.Errorf("iterating cancelled step ids: %w", err)
115+
}
99116

100117
// Emit audit events inside the transaction so they commit atomically.
101118
for _, id := range cancelledIDs {
@@ -108,6 +125,16 @@ func newCancelCommand() *cobra.Command {
108125
return fmt.Errorf("emitting audit event for %s: %w", id, err)
109126
}
110127
}
128+
for _, id := range cancelledStepIDs {
129+
if err := audit.EmitTx(cmd.Context(), tx, audit.Event{
130+
Timestamp: time.Now(),
131+
Actor: "cli",
132+
Action: audit.ActionExecutionCancelled,
133+
Resource: audit.Resource{Type: "step_execution", ID: id},
134+
}); err != nil {
135+
return fmt.Errorf("emitting audit event for step %s: %w", id, err)
136+
}
137+
}
111138

112139
if err := tx.Commit(); err != nil {
113140
return fmt.Errorf("committing cancellation: %w", err)

packages/engine/internal/engine/workflow_connector_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ steps:
180180

181181
// With maxDepth=1 and parentDepth=0, childDepth=1 which does not exceed max, so this succeeds.
182182
if result.Status != "completed" {
183-
t.Logf("result status=%s, error=%s", result.Status, result.Error)
183+
t.Fatalf("expected completed status for non-exceeding depth, got status=%q error=%q", result.Status, result.Error)
184184
}
185185

186186
// Now test the actual depth limit: create an execution at depth 1 and max=1.

0 commit comments

Comments
 (0)