fix: preorder tree display, audit before commit, deeper cancellation checks (PR review)

- status.go: Use text path in recursive CTE for correct preorder traversal
- cancel.go: Emit audit events inside transaction via EmitTx before commit
- engine.go: Check cancellation status before each retry attempt
- engine.go: Guard updateStep against overwriting cancelled status
- workflow_connector_test.go: Assert specific depth error substring

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: michaelmcnees

packages/engine/internal/cli/cancel.go

@@ -97,19 +97,20 @@ func newCancelCommand() *cobra.Command {
 				return fmt.Errorf("cancelling step executions: %w", err)
 			}
 
-			if err := tx.Commit(); err != nil {
-				return fmt.Errorf("committing cancellation: %w", err)
-			}
-
-			// Emit audit events for each cancelled execution.
-			auditor := &audit.PostgresEmitter{DB: database}
+			// Emit audit events inside the transaction so they commit atomically.
 			for _, id := range cancelledIDs {
-				_ = auditor.Emit(cmd.Context(), audit.Event{
+				if err := audit.EmitTx(cmd.Context(), tx, audit.Event{
 					Timestamp: time.Now(),
 					Actor:     "cli",
 					Action:    audit.ActionExecutionCancelled,
 					Resource:  audit.Resource{Type: "workflow_execution", ID: id},
-				})
+				}); err != nil {
+					return fmt.Errorf("emitting audit event for %s: %w", id, err)
+				}
+			}
+
+			if err := tx.Commit(); err != nil {
+				return fmt.Errorf("committing cancellation: %w", err)
 			}
 
 			fmt.Fprintf(cmd.OutOrStdout(), "Cancelled execution %s (%d executions affected)\n", execID, len(cancelledIDs))

packages/engine/internal/cli/status.go

@@ -101,16 +101,19 @@ func newStatusCommand() *cobra.Command {
 			}
 
 			// Query for full descendant execution tree using recursive CTE.
+			// Build a text path for correct preorder traversal ordering of nested trees.
 			childRows, err := database.QueryContext(cmd.Context(),
 				`WITH RECURSIVE tree AS (
-					SELECT id, workflow_name, workflow_version, status, 1 as depth
+					SELECT id, workflow_name, workflow_version, status, 1 as depth,
+					       id::text as path
 					FROM workflow_executions WHERE parent_execution_id = $1
 					UNION ALL
-					SELECT e.id, e.workflow_name, e.workflow_version, e.status, t.depth + 1
+					SELECT e.id, e.workflow_name, e.workflow_version, e.status, t.depth + 1,
+					       t.path || '/' || e.id::text
 					FROM workflow_executions e
 					JOIN tree t ON e.parent_execution_id = t.id
 				)
-				SELECT id, workflow_name, workflow_version, status, depth FROM tree ORDER BY depth, id`, execID,
+				SELECT id, workflow_name, workflow_version, status, depth, path FROM tree ORDER BY path`, execID,
 			)
 			if err == nil {
 				defer childRows.Close()
@@ -121,11 +124,12 @@ func newStatusCommand() *cobra.Command {
 					Version  int
 					Status   string
 					Depth    int
+					Path     string
 				}
 				var children []childExec
 				for childRows.Next() {
 					var c childExec
-					if err := childRows.Scan(&c.ID, &c.Workflow, &c.Version, &c.Status, &c.Depth); err == nil {
+					if err := childRows.Scan(&c.ID, &c.Workflow, &c.Version, &c.Status, &c.Depth, &c.Path); err == nil {
 						children = append(children, c)
 					}
 				}

packages/engine/internal/engine/engine.go

@@ -622,6 +622,12 @@ func (e *Engine) executeStepLogic(ctx context.Context, execID string, step workf
 	var lastErr error
 
 	for attempt := 1; attempt <= maxAttempts; attempt++ {
+		// Check if the execution has been cancelled before each retry attempt.
+		var execStatus string
+		if err := e.DB.QueryRowContext(ctx, "SELECT status FROM workflow_executions WHERE id = $1", execID).Scan(&execStatus); err == nil && execStatus == "cancelled" {
+			return nil, fmt.Errorf("execution cancelled")
+		}
+
 		// Create a fresh artifacts scratch dir per attempt.
 		if len(step.Artifacts) > 0 && e.Storage != nil {
 			if artifactsDir != "" {
@@ -1055,9 +1061,10 @@ func (e *Engine) updateStep(ctx context.Context, execID, stepName, status string
 		completedAt = time.Now()
 	}
 
+	// Don't overwrite a cancelled step.
 	_, err = e.DB.ExecContext(ctx,
 		`UPDATE step_executions SET status = $1, output = $2, error = $3, completed_at = $4, continue_on_error = $5, updated_at = NOW()
-		 WHERE execution_id = $6 AND step_name = $7 AND attempt = 1`,
+		 WHERE execution_id = $6 AND step_name = $7 AND attempt = 1 AND status != 'cancelled'`,
 		status, outputJSON, errorVal, completedAt, continueOnError, execID, stepName,
 	)
 	if err != nil {

packages/engine/internal/engine/workflow_connector_test.go

@@ -3,6 +3,7 @@ package engine
 import (
 	"context"
 	"encoding/json"
+	"strings"
 	"testing"
 )
 
@@ -227,6 +228,9 @@ steps:
 	if callDeep.Error == "" {
 		t.Error("call-deep error should be non-empty")
 	}
+	if !strings.Contains(callDeep.Error, "depth") {
+		t.Errorf("expected depth limit error, got: %s", callDeep.Error)
+	}
 	t.Logf("depth limit error: %s", callDeep.Error)
 }