fix: detect runtime crashes in e2e harness and preserve retry evidence

A non-zero act exit with no parseable job failure was classified
transient and retried up to five times. A real runtime crash (cascade
CLI or act) emits a goroutine dump that corrupts act's --json stream, so
the job-failure parse misses and a genuine crash is misclassified as a
flake, burning the retry budget while the CI log expires before the
stack trace can be read.

Detect Go-runtime crash signatures (panic:, fatal error:, a goroutine
dump header, a fatal signal) in act's raw output and treat them as
definitive, non-transient failures so they surface immediately with
their reason. Anchor on real runtime signatures so a benign log line
mentioning the word panic does not misfire.

On retry-budget exhaustion, persist the last attempt's full act
stdout/stderr to a per scenario+attempt file under e2e/_artifacts and
reference its path in the returned error, so the next genuine failure
keeps its stack-trace origin.

Signed-off-by: Joshua Temple <joshua.temple@stablekernel.com>

Author: joshua-temple

e2e/.gitignore

@@ -1,3 +1,6 @@
 # Test artifacts
 *.log
 coverage.out
+
+# Captured act stdout/stderr from exhausted scenario retries
+_artifacts/

e2e/harness/act.go

@@ -368,6 +368,13 @@ func (a *ActRunner) RunWorkflowFromRepo(ctx context.Context, opts RunOpts) (*Ext
 			Jobs:       make(map[string]*JobResultExtended),
 			Logs:       logs.String(),
 		}
+		// The parse fallback bypasses ParseActOutput's crash detection, so run
+		// it directly here against the raw logs: a crash dump is the most likely
+		// reason the JSON stream failed to parse, and it must not be lost.
+		if crashed, reason := detectCrashSignature(cleanedLogs); crashed {
+			result.Crashed = true
+			result.CrashReason = reason
+		}
 	}
 
 	normalizeWorkflowResult(result, opts.WorkflowPath, exitCode)
@@ -384,19 +391,37 @@ func (a *ActRunner) RunWorkflowFromRepo(ctx context.Context, opts RunOpts) (*Ext
 // up as a green-but-empty scenario (#25).
 func normalizeWorkflowResult(result *ExtendedWorkflowResult, workflowPath string, exitCode int) {
 	if exitCode != 0 {
-		// ExecError means act could NOT run the workflow to a conclusion: a
-		// genuine act/docker transport or exec hiccup where no job reached a
-		// conclusion. It must NOT cover the case where act ran the workflow and
-		// a job genuinely concluded "failure" - that is a real, deterministic
-		// defect and retrying it would mask a real failure as transient.
-		//
-		// So only tag ExecError when the non-zero exit is unaccompanied by any
-		// parsed job-level failure. If a job concluded "failure" (or the
-		// reconciled conclusion is already "failure"), this was a real outcome.
-		execError := !hasJobFailure(result)
+		// Classify BEFORE forcing the conclusion to "failure": hasJobFailure
+		// keys off result.Conclusion, so overwriting it first would make every
+		// non-zero exit look like a real job failure and defeat the transient
+		// path.
+		switch {
+		case result != nil && result.Crashed:
+			// A Go-runtime crash (cascade CLI or act) corrupts the --json stream
+			// so no "Job failed" event parses. Left to the ExecError heuristic
+			// below it would look like a job-less hiccup and be retried away as a
+			// transient flake, burning the retry budget and letting the stack
+			// trace expire. A crash is a definitive defect: keep ExecError false
+			// so the scenario runner does NOT retry it, and surface the crash
+			// reason.
+			result.ExecError = false
+			result.Error = fmt.Sprintf("workflow crashed: %s", result.CrashReason)
+		case hasJobFailure(result):
+			// act ran the workflow and a job genuinely concluded "failure". That
+			// is a real, deterministic defect, not a transport hiccup, so it must
+			// not be retried.
+			result.ExecError = false
+			result.Error = "workflow execution failed"
+		default:
+			// A non-zero exit with no parsed job failure and no crash signature
+			// is a benign act/docker transport or exec hiccup: tag it transient
+			// so the scenario runner may retry from a clean slate.
+			result.ExecError = true
+			result.Error = "workflow execution failed"
+		}
+		// A non-zero exit is always a failure; set the conclusion after
+		// classification so hasJobFailure above saw the pre-exit conclusion.
 		result.Conclusion = "failure"
-		result.Error = "workflow execution failed"
-		result.ExecError = execError
 	}
 
 	if workflowPath != "" && len(result.Jobs) == 0 && result.Conclusion != "failure" {

e2e/harness/crash_test.go

@@ -0,0 +1,146 @@
+package harness
+
+import (
+	"strings"
+	"testing"
+)
+
+// goroutineDump is a representative Go runtime panic + goroutine dump as it
+// appears interleaved in act's stdout/stderr when the cascade CLI (or act
+// itself) crashes mid-run. The dump corrupts act's --json stream, so no
+// "Job failed" event is parseable and the run would otherwise be misclassified
+// as a transient flake and retried away.
+const goroutineDump = `time="2026-06-13T00:00:00Z" level=info msg="⭐ Run Main cascade orchestrate"
+panic: runtime error: invalid memory address or nil pointer dereference
+[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x10a3f20]
+
+goroutine 1 [running]:
+github.com/stablekernel/cascade/internal/orchestrate.(*Planner).Plan(0x0)
+	/cascade/internal/orchestrate/planner.go:142 +0x1a0
+main.main()
+	/cascade/main.go:33 +0x9c
+`
+
+// TestDetectCrashSignature_GoPanicAndGoroutineDump verifies a real Go-runtime
+// crash signature in raw act output is detected so it can be treated as a
+// definitive failure rather than a transient flake.
+func TestDetectCrashSignature_GoPanicAndGoroutineDump(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name        string
+		logs        string
+		wantCrash   bool
+		wantReasony string // substring expected in the reason when wantCrash
+	}{
+		{
+			name:        "panic plus goroutine dump",
+			logs:        goroutineDump,
+			wantCrash:   true,
+			wantReasony: "panic",
+		},
+		{
+			name:        "fatal error at line start",
+			logs:        "some log\nfatal error: concurrent map writes\n\ngoroutine 7 [running]:\n",
+			wantCrash:   true,
+			wantReasony: "fatal error",
+		},
+		{
+			name:        "goroutine dump with runtime frame",
+			logs:        "goroutine 42 [running]:\nruntime.gopanic(0x1, 0x2)\n\t/usr/local/go/src/runtime/panic.go:884\n",
+			wantCrash:   true,
+			wantReasony: "goroutine",
+		},
+		{
+			name:        "SIGSEGV signal line",
+			logs:        "unexpected\n[signal SIGSEGV: segmentation violation code=0x1 addr=0x0]\n",
+			wantCrash:   true,
+			wantReasony: "SIGSEGV",
+		},
+		{
+			name:      "benign log mentioning the word panic in prose is not a crash",
+			logs:      `{"jobID":"deploy","msg":"Do not panic: rollback is automatic","level":"info"}`,
+			wantCrash: false,
+		},
+		{
+			name:      "benign log mentioning goroutine count in prose is not a crash",
+			logs:      `{"jobID":"deploy","msg":"started 8 goroutines for the worker pool","level":"info"}`,
+			wantCrash: false,
+		},
+		{
+			name:      "ordinary successful output has no crash signature",
+			logs:      `{"jobID":"build","jobResult":"success","msg":"🏁 Job succeeded","level":"info"}`,
+			wantCrash: false,
+		},
+	}
+
+	for _, tt := range tests {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			crashed, reason := detectCrashSignature(tt.logs)
+			if crashed != tt.wantCrash {
+				t.Fatalf("detectCrashSignature crashed = %v, want %v (reason=%q)", crashed, tt.wantCrash, reason)
+			}
+			if tt.wantCrash {
+				if reason == "" {
+					t.Fatalf("expected a non-empty crash reason when crashed")
+				}
+				if tt.wantReasony != "" && !strings.Contains(strings.ToLower(reason), strings.ToLower(tt.wantReasony)) {
+					t.Fatalf("crash reason %q does not mention %q", reason, tt.wantReasony)
+				}
+			}
+		})
+	}
+}
+
+// TestParseActOutput_SetsCrashFields verifies the parser surfaces a runtime
+// crash on the result so the classifier downstream can act on it.
+func TestParseActOutput_SetsCrashFields(t *testing.T) {
+	t.Parallel()
+
+	result, err := ParseActOutput(goroutineDump)
+	if err != nil {
+		t.Fatalf("ParseActOutput returned error: %v", err)
+	}
+	if !result.Crashed {
+		t.Fatalf("expected Crashed=true for a panic+goroutine dump")
+	}
+	if result.CrashReason == "" {
+		t.Fatalf("expected a non-empty CrashReason")
+	}
+}
+
+// TestNormalizeWorkflowResult_CrashIsNotTransient is the core safety guarantee:
+// a non-zero act exit carrying a Go-runtime crash signature must be classified
+// as a REAL (non-transient) failure so the scenario runner does not retry it as
+// a flake and the stack trace surfaces.
+func TestNormalizeWorkflowResult_CrashIsNotTransient(t *testing.T) {
+	t.Parallel()
+
+	result := &ExtendedWorkflowResult{
+		Conclusion: "success",
+		Jobs:       map[string]*JobResultExtended{},
+		Logs:       goroutineDump,
+		Crashed:    true,
+		CrashReason: "panic: runtime error: invalid memory address or nil pointer " +
+			"dereference",
+	}
+	normalizeWorkflowResult(result, ".github/workflows/orchestrate.yaml", 2)
+
+	if result.Conclusion != "failure" {
+		t.Fatalf("Conclusion = %q, want failure", result.Conclusion)
+	}
+	if result.ExecError {
+		t.Fatalf("a crash must NOT be tagged as a transient ExecError")
+	}
+	if !strings.Contains(strings.ToLower(result.Error), "crash") {
+		t.Fatalf("error %q should describe the crash", result.Error)
+	}
+
+	// And it must not be retried by the classifier.
+	failErr := workflowFailureError("orchestrate", result)
+	if IsTransientWorkflowError(failErr) {
+		t.Fatalf("a crash failure must not be classified transient: %v", failErr)
+	}
+}

e2e/harness/parser.go

@@ -3,6 +3,7 @@ package harness
 import (
 	"bufio"
 	"encoding/json"
+	"regexp"
 	"strings"
 	"time"
 )
@@ -37,6 +38,58 @@ type ExtendedWorkflowResult struct {
 	// mismatch (which must fail deterministically). A run that parsed real job
 	// events and concluded "failure" leaves ExecError false.
 	ExecError bool
+	// Crashed is true when act's raw output carries a Go-runtime crash
+	// signature (a panic, a goroutine dump, a SIGSEGV/SIGABRT, or a fatal
+	// error) from the cascade CLI or act itself. A crash corrupts act's --json
+	// stream so no "Job failed" event is parseable, which would otherwise let a
+	// genuine crash masquerade as a transient flake and be retried away. When
+	// Crashed is true the run is a definitive failure, NOT a transient one.
+	Crashed bool
+	// CrashReason is the first matched crash-signature line, captured so the
+	// failure surfaces the stack-trace origin rather than a generic message.
+	CrashReason string
+}
+
+// goroutineDumpRE matches the header of a Go goroutine dump, e.g.
+// "goroutine 1 [running]:". This is the anchored runtime signature emitted on a
+// panic or fatal error; it does not match prose that merely contains the word
+// "goroutine".
+var goroutineDumpRE = regexp.MustCompile(`(?m)^goroutine \d+ \[`)
+
+// signalCrashRE matches a Go runtime fatal-signal line, e.g.
+// "[signal SIGSEGV: segmentation violation ...]" or a SIGABRT report.
+var signalCrashRE = regexp.MustCompile(`signal SIG(SEGV|ABRT|BUS|FPE|ILL)`)
+
+// detectCrashSignature reports whether raw act stdout/stderr carries a Go
+// runtime crash signature, returning the first matched signature line as the
+// reason. It anchors on real runtime signatures (a goroutine dump header, a
+// "panic:" or "fatal error:" at the start of a line, a fatal signal report) so
+// it does not misfire on benign occurrences of the words "panic" or
+// "goroutine" inside an ordinary log line (e.g. a deploy script's prose).
+func detectCrashSignature(logs string) (bool, string) {
+	if logs == "" {
+		return false, ""
+	}
+
+	scanner := bufio.NewScanner(strings.NewReader(logs))
+	// A crash dump line can be long (a deeply nested stack frame); raise the
+	// scanner's token limit so a long frame cannot truncate detection.
+	scanner.Buffer(make([]byte, 0, 64*1024), 1024*1024)
+	for scanner.Scan() {
+		line := scanner.Text()
+		trimmed := strings.TrimSpace(line)
+		switch {
+		case strings.HasPrefix(trimmed, "panic:"):
+			return true, trimmed
+		case strings.HasPrefix(trimmed, "fatal error:"):
+			return true, trimmed
+		case goroutineDumpRE.MatchString(line):
+			return true, trimmed
+		case signalCrashRE.MatchString(line):
+			return true, trimmed
+		}
+	}
+	return false, ""
 }
 
 // JobResultExtended contains detailed result of a single job
@@ -141,5 +194,13 @@ func ParseActOutput(output string) (*ExtendedWorkflowResult, error) {
 		}
 	}
 
+	// A Go-runtime crash (cascade CLI or act) can corrupt the --json stream so
+	// no "Job failed" event parses; flag it from the raw logs so the classifier
+	// treats it as a definitive failure rather than a transient flake.
+	if crashed, reason := detectCrashSignature(output); crashed {
+		result.Crashed = true
+		result.CrashReason = reason
+	}
+
 	return result, scanner.Err()
 }

e2e/harness/scenario_retry.go

@@ -2,8 +2,12 @@ package harness
 
 import (
 	"context"
+	"errors"
 	"fmt"
+	"os"
 	"os/exec"
+	"path/filepath"
+	"regexp"
 	"testing"
 	"time"
 )
@@ -30,6 +34,45 @@ var scenarioRetryBackoff = 5 * time.Second
 // path stays a single, testable seam over the docker CLI).
 var pruneBetweenAttempts = pruneDockerNetworks
 
+// artifactDir is where the raw act stdout/stderr of an exhausted scenario is
+// persisted so a future CI failure yields the stack-trace origin instead of an
+// expired log. It is a var so unit tests can redirect it to a temp dir. The
+// default lives under the e2e module so captured logs are easy to find and are
+// covered by e2e/.gitignore (which ignores *.log).
+var artifactDir = "_artifacts"
+
+// artifactNameUnsafe matches any run of characters that are not safe in a
+// cross-platform filename. The scenario name (which can contain spaces and
+// slashes) is sanitized through it so each artifact path is a single, safe
+// filename.
+var artifactNameUnsafe = regexp.MustCompile(`[^A-Za-z0-9._-]+`)
+
+// persistAttemptEvidence writes the raw act logs carried by err to a per
+// scenario+attempt artifact file and returns its path. It is best effort: if no
+// logs are available or the write fails, it returns "" and the caller proceeds
+// without an artifact reference (the scenario result must never be masked by an
+// artifact-write failure). The filename is unique per scenario+attempt so
+// parallel scenarios never collide.
+func persistAttemptEvidence(scenario string, attempt int, err error) string {
+	var we *workflowError
+	if !errors.As(err, &we) {
+		return ""
+	}
+	logs := we.actLogs()
+	if logs == "" {
+		return ""
+	}
+	if mkErr := os.MkdirAll(artifactDir, 0o755); mkErr != nil {
+		return ""
+	}
+	safe := artifactNameUnsafe.ReplaceAllString(scenario, "_")
+	path := filepath.Join(artifactDir, fmt.Sprintf("%s-attempt%d.log", safe, attempt))
+	if wrErr := os.WriteFile(path, []byte(logs), 0o644); wrErr != nil {
+		return ""
+	}
+	return path
+}
+
 // logger is the minimal logging surface a scenario attempt needs. *testing.T
 // satisfies it, and unit tests can supply a fake to assert on retry logging.
 type logger interface {
@@ -81,6 +124,15 @@ func runScenarioWithRetry(ctx context.Context, log logger, name string, attempt
 		log.Logf("scenario %q: exhausted %d attempts; last failure was transient: %v",
 			name, scenarioRetryAttempts, err)
 	}
+
+	// The budget is exhausted. Persist the last attempt's raw act stdout/stderr
+	// so the stack-trace origin survives the CI log's retention window, and
+	// reference the artifact path in the returned error.
+	if path := persistAttemptEvidence(name, scenarioRetryAttempts, lastErr); path != "" {
+		log.Logf("scenario %q: wrote last-attempt act log to %s", name, path)
+		return fmt.Errorf("scenario %q failed after %d attempts (act log: %s): %w",
+			name, scenarioRetryAttempts, path, lastErr)
+	}
 	return fmt.Errorf("scenario %q failed after %d attempts: %w", name, scenarioRetryAttempts, lastErr)
 }