fix: composite pre/post conditions read the job status (#1242)

A composite action's child run context starts with empty step results, so a nested action's `post-if: success()` saw the composite's own result and still ran after the job had failed, which let `actions/cache` save caches from failed jobs.

The `job` context now always comes from the job's run context. Only `success()` and `failure()` in a composite's own main steps read the composite's result, matching GitHub Actions.

Assisted-by: Claude Code:claude-fable-5-1
Reviewed-on: https://gitea.com/gitea/runner/pulls/1242
Reviewed-by: bircni <bircni@icloud.com>
Co-authored-by: silverwind <me@silverwind.io>

act/runner/action_composite_test.go

@@ -117,3 +117,51 @@ func TestAppendUniqueMasksNoExponentialGrowth(t *testing.T) {
 
 	assert.Equal(t, []string{"secret"}, parentMasks)
 }
+
+func TestCompositeStepIfReadsJobStatusExceptInItsOwnMainSteps(t *testing.T) {
+	failStep := func(rc *RunContext) {
+		rc.StepResults["failed"] = &model.StepResult{Conclusion: model.StepStatusFailure}
+	}
+	tests := []struct {
+		name   string
+		setup  func(job, composite *RunContext)
+		nested bool
+		stage  stepStage
+		want   map[string]bool
+	}{
+		{"post-if reads the failed job", func(job, _ *RunContext) { failStep(job) }, false, stepStagePost, map[string]bool{"success()": false, "failure()": true, "job.status == 'failure'": true}},
+		{"main if ignores the failed job", func(job, _ *RunContext) { failStep(job) }, false, stepStageMain, map[string]bool{"success()": true, "failure()": false}},
+		{"main if reads the composite's own failure", func(_, composite *RunContext) { failStep(composite) }, false, stepStageMain, map[string]bool{"success()": false, "failure()": true}},
+		{"nested main if ignores the outer composite's failure", func(_, composite *RunContext) { failStep(composite) }, true, stepStageMain, map[string]bool{"success()": true}},
+		{"main if sees the cancelled job", func(job, _ *RunContext) { job.markCancelled() }, false, stepStageMain, map[string]bool{"cancelled()": true, "success()": false}},
+		{"post-if sees the cancelled job", func(job, _ *RunContext) { job.markCancelled() }, false, stepStagePost, map[string]bool{"cancelled()": true, "success()": false}},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			job := makeTestRC(t, "runs-on: ubuntu-latest")
+			composite := newTestCompositeRunContext(t, job)
+			rc := composite
+			if tt.nested {
+				rc = newTestCompositeRunContext(t, composite)
+			}
+			tt.setup(job, composite)
+			step := &stepRun{RunContext: rc}
+			for expr, want := range tt.want {
+				enabled, err := isStepEnabled(t.Context(), expr, step, tt.stage)
+				require.NoError(t, err)
+				assert.Equal(t, want, enabled, expr)
+			}
+		})
+	}
+}
+
+func newTestCompositeRunContext(t *testing.T, parent *RunContext) *RunContext {
+	composite, err := newCompositeRunContext(t.Context(), parent, &stepActionRemote{
+		Step:       &model.Step{},
+		RunContext: parent,
+		action:     &model.Action{},
+		env:        map[string]string{},
+	}, "/action")
+	require.NoError(t, err)
+	return composite
+}

act/runner/expression.go

@@ -127,15 +127,15 @@ var hashfiles string
 
 // NewStepExpressionEvaluator creates a new evaluator with the `inputs` of the enclosing workflow or composite action
 func (rc *RunContext) NewStepExpressionEvaluator(ctx context.Context, step step) *ExpressionEvaluator {
-	return rc.newStepExpressionEvaluator(ctx, step, rc.actionInputs)
+	return rc.newStepExpressionEvaluator(ctx, step, rc.actionInputs, rc.getJobContext())
 }
 
 // NewActionInputsExpressionEvaluator creates a new evaluator with the step's own with: values as `inputs`
 func (rc *RunContext) NewActionInputsExpressionEvaluator(ctx context.Context, step step) *ExpressionEvaluator {
-	return rc.newStepExpressionEvaluator(ctx, step, inputsFromEnv(*step.getEnv()))
+	return rc.newStepExpressionEvaluator(ctx, step, inputsFromEnv(*step.getEnv()), rc.getJobContext())
 }
 
-func (rc *RunContext) newStepExpressionEvaluator(ctx context.Context, step step, stepInputs map[string]any) *ExpressionEvaluator {
+func (rc *RunContext) newStepExpressionEvaluator(ctx context.Context, step step, stepInputs map[string]any, jobContext *model.JobContext) *ExpressionEvaluator {
 	// todo: cleanup EvaluationEnvironment creation
 	job := rc.Run.Job()
 	strategy := make(map[string]any)
@@ -157,7 +157,7 @@ func (rc *RunContext) newStepExpressionEvaluator(ctx context.Context, step step,
 	ee := &exprparser.EvaluationEnvironment{
 		Github:   step.getGithubContext(ctx),
 		Env:      *step.getEnv(),
-		Job:      rc.getJobContext(),
+		Job:      jobContext,
 		Steps:    rc.getStepsContext(),
 		Secrets:  getWorkflowSecrets(rc),
 		Vars:     getWorkflowVars(ctx, rc),

act/runner/run_context.go

@@ -1339,21 +1339,26 @@ func trimToLen(s string, l int) string {
 	return s
 }
 
-func (rc *RunContext) getJobContext() *model.JobContext {
-	jobStatus := "success"
+// ownStatus is the result of this context's own steps, which for a composite action is its action status
+func (rc *RunContext) ownStatus() string {
 	if rc.jobFailed {
-		jobStatus = "failure"
+		return "failure"
 	}
 	for _, stepStatus := range rc.StepResults {
 		if stepStatus.Conclusion == model.StepStatusFailure {
-			jobStatus = "failure"
-			break
+			return "failure"
 		}
 	}
+	return "success"
+}
+
+func (rc *RunContext) getJobContext() *model.JobContext {
+	job := rc.topLevelRunContext()
+	jobStatus := job.ownStatus()
 	// A cancelled run takes precedence over success/failure so cancelled() is true and
 	// success()/failure() are false, matching GitHub Actions: on cancellation only
 	// always() and cancelled() steps run.
-	if rc.jobCancelled {
+	if job.jobCancelled {
 		jobStatus = "cancelled"
 	}
 
@@ -1361,17 +1366,17 @@ func (rc *RunContext) getJobContext() *model.JobContext {
 		Status:   jobStatus,
 		Services: map[string]model.JobService{}, // an empty map, never null
 	}
-	if rc.jobContainerID != "" {
-		jobContext.Container.ID = rc.jobContainerID
-		jobContext.Container.Network = rc.jobNetworkName
+	if job.jobContainerID != "" {
+		jobContext.Container.ID = job.jobContainerID
+		jobContext.Container.Network = job.jobNetworkName
 	}
-	for _, svc := range rc.serviceContainers {
+	for _, svc := range job.serviceContainers {
 		if svc.info == nil {
 			continue
 		}
 		jobContext.Services[svc.name] = model.JobService{
 			ID:      svc.info.ID,
-			Network: rc.jobNetworkName,
+			Network: job.jobNetworkName,
 			Ports:   svc.info.Ports,
 		}
 	}

act/runner/step.go

@@ -299,7 +299,12 @@ func isStepEnabled(ctx context.Context, expr string, step step, stage stepStage)
 		defaultStatusCheck = exprparser.DefaultStatusCheckSuccess
 	}
 
-	runStep, err := EvalBool(ctx, rc.NewStepExpressionEvaluator(ctx, step), expr, defaultStatusCheck)
+	// success() and failure() in a composite's own main steps read the composite's result, other stages the job's
+	jobContext := rc.getJobContext()
+	if rc.Parent != nil && stage == stepStageMain && jobContext.Status != "cancelled" {
+		jobContext.Status = rc.ownStatus()
+	}
+	runStep, err := EvalBool(ctx, rc.newStepExpressionEvaluator(ctx, step, rc.actionInputs, jobContext), expr, defaultStatusCheck)
 	if err != nil {
 		return false, fmt.Errorf("if-expression %q evaluation failed: %s", expr, err)
 	}