fix(runner): prevent data race during rate limiter initialization

[AdamMagued]

Proposed changes

Fixes #2653

When initializing the runner rate limiter, the limiter struct value was previously copied via dereference (*ratelimit.New(...)). Because ratelimit.Limiter spawns a background goroutine that mutates atomic fields, copying the value leads to a data race.

This change stores *ratelimit.Limiter as a pointer on the Runner struct directly, avoiding struct copy races while keeping the limiter reference valid. Nil checks are also added to Close() and Take() calls to guard against uninitialized states.

Proof

Added concurrent initialization unit tests (TestRunner_RateLimiter_Initialization) verifying unlimited, per-second, and per-minute rate limiter configurations as well as nil safety. Tested using go test -race with zero race warnings.

Checklist

• Pull request is created against the dev branch
• All checks passed (lint, unit/integration/regression tests etc.) with my changes
• I have added tests that prove my fix is effective or that my feature works
• I have added necessary documentation (if appropriate)

Summary by CodeRabbit

• Bug Fixes
  • Improved runner reliability across unlimited, per-second, and per-minute rate-limit configurations. Runners can now be created concurrently and shut down safely, including when no rate limiter is configured. This helps prevent errors during startup, request handling, and cleanup in these configurations.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: b67c6623-38de-4c8e-9160-5b47a1fdeda2

---

📥 Commits

Reviewing files that changed from the base of the PR and between 10b2453 and 2ecd7ba.

---

📒 Files selected for processing (2)

• runner/runner.go
• runner/runner_test.go

---

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

---

---

Walkthrough

The runner now stores rate limiters by pointer instead of copying them into a value field. It checks for a non-nil limiter before stopping it or taking tokens in request and probe paths. Tests cover concurrent initialization and closing with a nil limiter.

Changes

Runner rate limiter handling

Layer / File(s)	Summary
Retain the active limiter runner/runner.go, runner/runner_test.go	Runner stores the pointer returned by each rate-limit constructor. Concurrent tests check initialization, token acquisition, and closure for unlimited, per-second, and per-minute configurations.
Guard limiter operations runner/runner.go, runner/runner_test.go	Close and request and probe paths call limiter methods only when the pointer is non-nil. A test checks that Close does not panic when the limiter is nil.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: mzack9999

Merge Risk: ⚪ Minimal · up to 2ecd7

The rate-limiter change appears mergeable after normal checks.

Pre-merge checks | 4 | 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: preventing a data race during rate limiter initialization in the runner.
Linked Issues check		Issue #2653 requires runner.New to retain the live limiter pointer and avoid copying active limiter state. Runner.ratelimiter is now *ratelimit.Limiter, and all three constructor branches assign…
Out of Scope Changes check		The changes are limited to runner limiter storage, limiter use and shutdown guards, and focused initialization tests. The nil guards support the changed pointer representation and the tests cover nil …

---

---

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

---

---

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

_{A rabbit watched the limiter run,
Its live pointer shared with everyone.
“No copied counters,” it said with delight,
“And nil checks keep each call just right.”
Then it hopped off beneath the moon.}

_{Comment @coderabbitai help to get the list of available commands.}