fix(actions): nil pointer dereference in concurrency during PR creation #137

2026-05-23T23:20:28Z

jmiller commented

2026-05-23 23:20:28 +00:00

Summary

Fix nil pointer dereference panic in EvaluateRunConcurrencyFillModel when InsertRun passes nil for the attempt parameter (concurrency.go:39)
Fix data loss where run.ConcurrencyGroup was never populated for DB persistence because the function only wrote to the nil attempt
Fix TestEvaluateRunConcurrency_RunIDFallback which had wrong argument count (5 vs 6) and didn't test the attempt path

Closes #136

Root cause

InsertRun (run.go:91) called EvaluateRunConcurrencyFillModel(ctx, run, nil, ...), but the function wrote to attempt.ConcurrencyGroup at concurrency.go:39 without checking for nil. This crashed the server during the post-creation notification phase when creating PRs via the API on repos with workflow-level concurrency configured.

Changes

File	Change
`services/actions/concurrency.go`	Write concurrency fields to both `run` and `attempt` (with nil guard)
`services/actions/run.go`	Create `ActionRunAttempt` struct before calling evaluate, reuse for `PrepareToStartRunWithConcurrency`
`services/actions/context_test.go`	Fix function call to match 6-param signature, add attempt assertions

Test plan

Deploy to dev environment and create a PR via API on a repo with workflow concurrency configured
Verify API returns HTTP 201 with valid JSON (no panic)
Verify the concurrency group is correctly persisted in the DB
Verify existing TestEvaluateRunConcurrency_RunIDFallback passes
Verify TestPrepareRunAndInsert_ExpressionsSeeRunID passes

?? Generated with Claude Code

## Summary - Fix nil pointer dereference panic in `EvaluateRunConcurrencyFillModel` when `InsertRun` passes `nil` for the `attempt` parameter (concurrency.go:39) - Fix data loss where `run.ConcurrencyGroup` was never populated for DB persistence because the function only wrote to the nil attempt - Fix `TestEvaluateRunConcurrency_RunIDFallback` which had wrong argument count (5 vs 6) and didn't test the attempt path Closes #136 ## Root cause `InsertRun` (run.go:91) called `EvaluateRunConcurrencyFillModel(ctx, run, nil, ...)`, but the function wrote to `attempt.ConcurrencyGroup` at concurrency.go:39 without checking for nil. This crashed the server during the post-creation notification phase when creating PRs via the API on repos with workflow-level concurrency configured. ## Changes | File | Change | |------|--------| | `services/actions/concurrency.go` | Write concurrency fields to both `run` and `attempt` (with nil guard) | | `services/actions/run.go` | Create `ActionRunAttempt` struct before calling evaluate, reuse for `PrepareToStartRunWithConcurrency` | | `services/actions/context_test.go` | Fix function call to match 6-param signature, add attempt assertions | ## Test plan - [ ] Deploy to dev environment and create a PR via API on a repo with workflow concurrency configured - [ ] Verify API returns HTTP 201 with valid JSON (no panic) - [ ] Verify the concurrency group is correctly persisted in the DB - [ ] Verify existing `TestEvaluateRunConcurrency_RunIDFallback` passes - [ ] Verify `TestPrepareRunAndInsert_ExpressionsSeeRunID` passes ?? Generated with [Claude Code](https://claude.com/claude-code)

jmiller added 1 commit 2026-05-23 23:20:29 +00:00

fix(actions): nil pointer dereference in concurrency during PR creation

Branch Policy Check / Verify merge target (pull_request) Failing after 1s

Details

a804ebcf09

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) <noreply@anthropic.com>

jmiller merged commit 7a06e44e24 into main

2026-05-24 03:26:43 +00:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: MokoConsulting/MokoGitea#137