From a804ebcf0942f4794cee7eef537ecc32c6e7c482 Mon Sep 17 00:00:00 2001 From: Jonathan Miller Date: Sat, 23 May 2026 18:18:06 -0500 Subject: [PATCH] fix(actions): nil pointer dereference in concurrency during PR creation InsertRun passed nil for the attempt parameter to EvaluateRunConcurrencyFillModel, which then dereferenced the nil pointer at concurrency.go:39 when writing ConcurrencyGroup and ConcurrencyCancel fields. This caused a server panic whenever a PR was created via the API on a repo with workflow-level concurrency configured. The fix: - Creates an ActionRunAttempt struct in InsertRun before calling EvaluateRunConcurrencyFillModel, and reuses it for PrepareToStartRunWithConcurrency - Updates EvaluateRunConcurrencyFillModel to write concurrency fields to both the run (for DB persistence) and the attempt (for in-memory concurrency checks), with a nil guard on the attempt - Fixes TestEvaluateRunConcurrency_RunIDFallback which had the wrong argument count and was not testing the attempt path Closes #136 Co-Authored-By: Claude Opus 4.6 (1M context) --- services/actions/concurrency.go | 9 +++++++-- services/actions/context_test.go | 8 ++++++-- services/actions/run.go | 5 +++-- 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/services/actions/concurrency.go b/services/actions/concurrency.go index e1ec549930..7eb4bc1f6b 100644 --- a/services/actions/concurrency.go +++ b/services/actions/concurrency.go @@ -35,11 +35,16 @@ func EvaluateRunConcurrencyFillModel(ctx context.Context, run *actions_model.Act } } - var err error - attempt.ConcurrencyGroup, attempt.ConcurrencyCancel, err = jobparser.EvaluateConcurrency(wfRawConcurrency, "", nil, actionsRunCtx, jobResults, vars, inputs) + concurrencyGroup, concurrencyCancel, err := jobparser.EvaluateConcurrency(wfRawConcurrency, "", nil, actionsRunCtx, jobResults, vars, inputs) if err != nil { return fmt.Errorf("evaluate concurrency: %w", err) } + run.ConcurrencyGroup = concurrencyGroup + run.ConcurrencyCancel = concurrencyCancel + if attempt != nil { + attempt.ConcurrencyGroup = concurrencyGroup + attempt.ConcurrencyCancel = concurrencyCancel + } return nil } diff --git a/services/actions/context_test.go b/services/actions/context_test.go index 4ade67111c..7805df1eee 100644 --- a/services/actions/context_test.go +++ b/services/actions/context_test.go @@ -31,11 +31,15 @@ func TestEvaluateRunConcurrency_RunIDFallback(t *testing.T) { CancelInProgress: "true", } - assert.NoError(t, EvaluateRunConcurrencyFillModel(ctx, runA, expr, nil, nil)) - assert.NoError(t, EvaluateRunConcurrencyFillModel(ctx, runB, expr, nil, nil)) + attemptA := &actions_model.ActionRunAttempt{RunID: runA.ID, RepoID: runA.RepoID} + attemptB := &actions_model.ActionRunAttempt{RunID: runB.ID, RepoID: runB.RepoID} + assert.NoError(t, EvaluateRunConcurrencyFillModel(ctx, runA, attemptA, expr, nil, nil)) + assert.NoError(t, EvaluateRunConcurrencyFillModel(ctx, runB, attemptB, expr, nil, nil)) assert.Contains(t, runA.ConcurrencyGroup, "791") assert.Contains(t, runB.ConcurrencyGroup, "792") + assert.Equal(t, runA.ConcurrencyGroup, attemptA.ConcurrencyGroup) + assert.Equal(t, runB.ConcurrencyGroup, attemptB.ConcurrencyGroup) assert.NotEqual(t, runA.ConcurrencyGroup, runB.ConcurrencyGroup) } diff --git a/services/actions/run.go b/services/actions/run.go index 306d1d2e92..efae87fb5c 100644 --- a/services/actions/run.go +++ b/services/actions/run.go @@ -88,10 +88,11 @@ func InsertRun(ctx context.Context, run *actions_model.ActionRun, content []byte } if wfRawConcurrency != nil { - if err := EvaluateRunConcurrencyFillModel(ctx, run, nil, wfRawConcurrency, vars, inputs); err != nil { + attempt := &actions_model.ActionRunAttempt{RunID: run.ID, RepoID: run.RepoID} + if err := EvaluateRunConcurrencyFillModel(ctx, run, attempt, wfRawConcurrency, vars, inputs); err != nil { return fmt.Errorf("EvaluateRunConcurrencyFillModel: %w", err) } - run.Status, _, err = PrepareToStartRunWithConcurrency(ctx, &actions_model.ActionRunAttempt{RunID: run.ID}) + run.Status, _, err = PrepareToStartRunWithConcurrency(ctx, attempt) if err != nil { return err } -- 2.52.0