Skip to content

fix: only treat a crash with a cascade frame as non-transient #147

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
joshua-temple merged 1 commit into from
Jun 13, 2026
Merged

fix: only treat a crash with a cascade frame as non-transient #147

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
180 changes: 158 additions & 22 deletions e2e/harness/crash_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,11 @@ import (
)

// 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.
// appears interleaved in act's stdout/stderr when the cascade CLI crashes
// mid-run. The dump corrupts act's --json stream, so no "Job failed" event is
// parseable. It carries a real cascade stack frame
// (internal/orchestrate.(*Planner).Plan), so it is a definitive cascade crash
// and must not be retried away as a transient flake.
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]
Expand All @@ -21,10 +22,59 @@ 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) {
// actOnlyPanicDump is a panic whose goroutine dump contains ONLY nektos/act and
// stdlib frames, with no cascade frame. act can itself panic on an
// infrastructure path (a docker transport hiccup, a teardown race); such a
// crash is not a cascade defect and a retry should absorb it, so it must NOT be
// classified as a (non-transient) cascade crash.
const actOnlyPanicDump = `time="2026-06-13T00:00:00Z" level=info msg="⭐ Run Set up job"
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x9f1d40]

goroutine 84 [running]:
github.com/nektos/act/pkg/runner.(*runnerImpl).NewPlanExecutor.func1(0x0)
/go/pkg/mod/github.com/nektos/act/pkg/runner/runner.go:201 +0x120
net/http.(*conn).serve(0xc0001a4000, {0x1, 0x2})
/usr/local/go/src/net/http/server.go:2092 +0x4f4
internal/poll.(*FD).Read(0xc0000b8000, {0xc0001, 0x10, 0x10})
/usr/local/go/src/internal/poll/fd_unix.go:164 +0x29a
`

// bareActGoroutineDump is the actual shape act prints on a benign teardown:
// monitorJobCancellation under cancel-in-progress dumps every goroutine with a
// "goroutine N [wselect]:" header and NO panic / fatal error / signal trigger.
// On CI run 27480827218 this flaked Hotfix_Refusal_Guards step 2 even though the
// scenario passes 4/4 locally with zero retries. It is pure act-infra noise and
// must stay transient.
const bareActGoroutineDump = `time="2026-06-13T00:00:00Z" level=info msg="⭐ Run Set up job"
goroutine 470 [wselect]:
github.com/nektos/act/pkg/runner.(*runnerImpl).monitorJobCancellation.func1()
/go/pkg/mod/github.com/nektos/act/pkg/runner/runner.go:312 +0x88
created by github.com/nektos/act/pkg/runner.(*runnerImpl).monitorJobCancellation
/go/pkg/mod/github.com/nektos/act/pkg/runner/runner.go:305 +0x14c

goroutine 12 [chan receive]:
github.com/nektos/act/pkg/container.(*containerReference).Exec.func2()
/go/pkg/mod/github.com/nektos/act/pkg/container/docker_run.go:401 +0x40
`

// signalWithCascadeFrame is a fatal signal whose dump carries a cascade frame
// but no leading "panic:" line. The signal report is itself a crash trigger, so
// paired with a cascade frame it is a definitive cascade crash.
const signalWithCascadeFrame = `time="2026-06-13T00:00:00Z" level=info msg="⭐ Run Main cascade promote"
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x10b4c10]

goroutine 1 [running]:
github.com/stablekernel/cascade/cmd/cascade.run({0x0, 0x0})
/cascade/cmd/cascade/main.go:88 +0x210
`

// TestDetectCrashSignature verifies a crash is classified (non-transient) ONLY
// when a real runtime trigger (panic/fatal error/signal) is paired with a real
// cascade stack frame. An act/docker/stdlib-only dump, or a bare goroutine dump
// with no trigger, stays uncaught here so the scenario runner treats it as a
// transient flake and retries it.
func TestDetectCrashSignature(t *testing.T) {
t.Parallel()

tests := []struct {
Expand All @@ -34,28 +84,47 @@ func TestDetectCrashSignature_GoPanicAndGoroutineDump(t *testing.T) {
wantReasony string // substring expected in the reason when wantCrash
}{
{
name: "panic plus goroutine dump",
name: "panic plus goroutine dump with cascade frame is a crash",
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",
name: "fatal error at line start with cascade frame is a crash",
logs: "some log\nfatal error: concurrent map writes\n\ngoroutine 7 [running]:\ngithub.com/stablekernel/cascade/internal/state.(*Store).Write(0x0)\n\t/cascade/internal/state/store.go:51 +0x40\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",
name: "SIGSEGV signal line with cascade frame is a crash",
logs: signalWithCascadeFrame,
wantCrash: true,
wantReasony: "goroutine",
wantReasony: "SIGSEGV",
},
{
name: "SIGSEGV signal line",
logs: "unexpected\n[signal SIGSEGV: segmentation violation code=0x1 addr=0x0]\n",
wantCrash: true,
wantReasony: "SIGSEGV",
name: "panic with only act and stdlib frames is not a cascade crash",
logs: actOnlyPanicDump,
wantCrash: false,
},
{
name: "bare act goroutine dump with no trigger is not a crash",
logs: bareActGoroutineDump,
wantCrash: false,
},
{
name: "fatal error with no cascade frame is not a cascade crash",
logs: "fatal error: concurrent map writes\n\ngoroutine 7 [running]:\nruntime.gopanic(0x1, 0x2)\n\t/usr/local/go/src/runtime/panic.go:884\n",
wantCrash: false,
},
{
name: "cascade frame present but no trigger is not a crash",
logs: "goroutine 9 [running]:\ngithub.com/stablekernel/cascade/internal/orchestrate.(*Planner).Plan(0x0)\n\t/cascade/internal/orchestrate/planner.go:142\n",
wantCrash: false,
},
{
name: "git-url prose mentioning the cascade module path is not a crash",
logs: `{"jobID":"deploy","msg":"cloning github.com/stablekernel/cascade/internal at v1.2.3","level":"info"}`,
wantCrash: false,
},
{
name: "benign log mentioning the word panic in prose is not a crash",
Expand Down Expand Up @@ -104,17 +173,44 @@ func TestParseActOutput_SetsCrashFields(t *testing.T) {
t.Fatalf("ParseActOutput returned error: %v", err)
}
if !result.Crashed {
t.Fatalf("expected Crashed=true for a panic+goroutine dump")
t.Fatalf("expected Crashed=true for a panic+goroutine dump with a cascade frame")
}
if result.CrashReason == "" {
t.Fatalf("expected a non-empty CrashReason")
}
}

// TestParseActOutput_ActOnlyDumpIsNotCrash verifies an act/docker-only panic or
// a bare act goroutine dump does NOT set the crash fields, so the run falls
// through to the transient path and is retried as an infrastructure flake.
func TestParseActOutput_ActOnlyDumpIsNotCrash(t *testing.T) {
t.Parallel()

for _, tt := range []struct {
name string
logs string
}{
{name: "act-only panic", logs: actOnlyPanicDump},
{name: "bare act goroutine dump", logs: bareActGoroutineDump},
} {
tt := tt
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
result, err := ParseActOutput(tt.logs)
if err != nil {
t.Fatalf("ParseActOutput returned error: %v", err)
}
if result.Crashed {
t.Fatalf("expected Crashed=false for an act-only dump (reason=%q)", result.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.
// a non-zero act exit carrying a cascade 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()

Expand Down Expand Up @@ -144,3 +240,43 @@ func TestNormalizeWorkflowResult_CrashIsNotTransient(t *testing.T) {
t.Fatalf("a crash failure must not be classified transient: %v", failErr)
}
}

// TestNormalizeWorkflowResult_ActOnlyDumpIsTransient is the sibling guarantee:
// a non-zero act exit whose logs carry only an act/docker dump (no cascade
// frame) is NOT flagged Crashed by the parser, so it falls through to the
// transient ExecError path and the scenario runner retries it as an infra
// flake.
func TestNormalizeWorkflowResult_ActOnlyDumpIsTransient(t *testing.T) {
t.Parallel()

for _, tt := range []struct {
name string
logs string
}{
{name: "act-only panic", logs: actOnlyPanicDump},
{name: "bare act goroutine dump", logs: bareActGoroutineDump},
} {
tt := tt
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
// Mirror the production path: the parser decides Crashed, then
// normalizeWorkflowResult classifies a non-zero exit.
result, err := ParseActOutput(tt.logs)
if err != nil {
t.Fatalf("ParseActOutput returned error: %v", err)
}
normalizeWorkflowResult(result, ".github/workflows/orchestrate.yaml", 2)

if result.Conclusion != "failure" {
t.Fatalf("Conclusion = %q, want failure", result.Conclusion)
}
if !result.ExecError {
t.Fatalf("an act-only dump must be tagged transient (ExecError) so it is retried")
}
failErr := workflowFailureError("orchestrate", result)
if !IsTransientWorkflowError(failErr) {
t.Fatalf("an act-only dump failure must be classified transient: %v", failErr)
}
})
}
}
48 changes: 35 additions & 13 deletions e2e/harness/parser.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,27 +50,51 @@ type ExtendedWorkflowResult struct {
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)`)

// cascadeFrameRE matches a Go stack-trace frame that belongs to cascade's own
// code, e.g. "github.com/stablekernel/cascade/internal/orchestrate.(*Planner).Plan(...)"
// or "github.com/stablekernel/cascade/cmd/cascade.run(...)". It is deliberately
// strict: the package path must be under cascade's internal or cmd tree AND be
// followed by a "." and a function/receiver identifier, so it matches a real
// stack frame and not git-URL prose, the bare word "cascade", stdlib packages
// such as "internal/poll", or vendored "internal" packages like
// "go.opentelemetry.io/otel/internal/global".
var cascadeFrameRE = regexp.MustCompile(`stablekernel/cascade/(internal|cmd)[^\s]*\.[A-Za-z0-9_]`)

// 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).
// runtime crash originating in cascade's own code, returning the matched crash
// trigger line as the reason.
//
// A run is classified as a cascade crash only when BOTH conditions hold:
//
// 1. A real crash TRIGGER is present: a line beginning (trimmed) with "panic:"
// or "fatal error:", or a fatal-signal report ("signal SIGSEGV" and
// friends).
// 2. The dump contains a real cascade STACK FRAME (see cascadeFrameRE).
//
// This pairing is intentional. act routinely prints full goroutine dumps on its
// own cancellation/teardown paths (for example monitorJobCancellation under
// cancel-in-progress emits a bare "goroutine N [...]:" dump with no panic and
// only nektos/act frames). A bare goroutine dump, or a panic that carries only
// act/docker/stdlib frames, is act-infrastructure noise that a retry can absorb,
// not a cascade defect. Requiring both a trigger and a cascade frame keeps a
// genuine cascade-CLI panic a definitive failure while letting infra flakes stay
// transient. A bare goroutine-dump header is therefore NOT a trigger.
func detectCrashSignature(logs string) (bool, string) {
if logs == "" {
return false, ""
}

// First, require a real cascade stack frame anywhere in the dump. Without a
// cascade frame the crash (if any) is act/docker/stdlib only, which is
// infrastructure noise a retry should absorb.
if !cascadeFrameRE.MatchString(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.
Expand All @@ -83,8 +107,6 @@ func detectCrashSignature(logs string) (bool, string) {
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
}
Expand Down
Loading