fix: Locking during run ensure calls

[yhakbar]

Description

This addresses review coments from #5308

TODOs

Read the Gruntwork contribution guidelines.

• I authored this code entirely myself
• I am submitting code based on open source software (e.g. MIT, MPL-2.0, Apache)]
• I am adding or upgrading a dependency or adapted code and confirm it has a compatible open source license
• Update the docs.
• Run the relevant tests successfully, including pre-commit checks.
• Include release notes. If this PR is backward incompatible, include a migration guide.

Release Notes (draft)

Added / Removed / Updated [X].

Migration Guide

Summary by CodeRabbit

• Bug Fixes

  • Fixed thread-safety issue in run modification that could have caused data corruption during concurrent operations.

• Refactor

  • Improved internal code clarity with variable naming updates to reduce confusion in discovery context handling.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Review	Updated (UTC)
terragrunt-docs	Ready	Preview, Comment	Jan 7, 2026 3:12pm

[coderabbitai]

📝 Walkthrough

Walkthrough

Changes introduce thread-safety improvements to the EnsureRun function by acquiring run-level mutex locks before applying EndOptions, and correct variable names to avoid shadowing the outer ctx variable in discovery context operations across multiple runner modules.

Changes

Cohort / File(s)	Summary
Thread-safety fix internal/report/report.go	EnsureRun now acquires run.mu and defers unlock before applying EndOptions to existing runs, preventing data races during concurrent Run field modifications
Variable naming refactor internal/runner/common/unit_runner.go, internal/runner/runnerpool/runner.go	Renamed shadowing ctx variable to discoveryCtx in discovery context retrieval blocks to avoid variable shadowing; preserves all conditional logic and behavior

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• fix: Fixing report run duplication error #5252: Modifies EnsureRun function signature in internal/report/report.go alongside logging behavior changes
• chore: mark as completed runner-pool, reports and provider cache #4900: Contains overlapping changes to internal/report/report.go and runner module variable naming patterns

Suggested reviewers

• denis256
• ThisGuyCodes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description is largely incomplete. While it references the related issue #5308, it lacks substantive details about the changes, and most template sections (release notes, migration guide) remain unfilled with placeholder content.	Expand the description with details about the locking changes, complete the release notes with actual content, and clarify whether this is a backward-incompatible change requiring a migration guide.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'fix: Locking during run ensure calls' clearly and directly summarizes the main change—adding mutex locking during run ensure operations to prevent data races.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

---

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 06a2384 and a7ad4e2.

📒 Files selected for processing (3)

• internal/report/report.go
• internal/runner/common/unit_runner.go
• internal/runner/runnerpool/runner.go

🧰 Additional context used

📓 Path-based instructions (1)

**/*.go

⚙️ CodeRabbit configuration file

Review the Go code for quality and correctness. Make sure that the Go code follows best practices, is performant, and is easy to understand and maintain.

Files:

• internal/report/report.go
• internal/runner/runnerpool/runner.go
• internal/runner/common/unit_runner.go

🧬 Code graph analysis (2)

internal/runner/runnerpool/runner.go (2)

internal/component/component.go (1)

• DiscoveryContext (67-73)

internal/report/report.go (1)

• WithDiscoveryWorkingDir (332-336)

internal/runner/common/unit_runner.go (3)

internal/component/unit.go (1)

• Unit (22-34)

internal/component/component.go (1)

• DiscoveryContext (67-73)

internal/report/report.go (1)

• WithDiscoveryWorkingDir (332-336)

🔇 Additional comments (4)

internal/report/report.go (1)

182-184: LGTM! Critical thread-safety fix.

Acquiring run.mu before applying EndOptions to an existing Run prevents data races during concurrent EnsureRun calls. This pattern aligns with the existing locking in EndRun (lines 244-245) and properly protects Run field mutations.

internal/runner/runnerpool/runner.go (2)

485-486: LGTM! Fixes variable shadowing.

Renaming to discoveryCtx eliminates shadowing of the ctx context.Context parameter (line 441). The previous code masked the context parameter with a local variable of type *DiscoveryContext, which could cause subtle bugs if the context was referenced later in the scope.

---

580-581: LGTM! Fixes variable shadowing.

Consistent with the fix at lines 485-486, this eliminates shadowing of the ctx context.Context parameter, preventing potential confusion between context.Context and *DiscoveryContext types.

internal/runner/common/unit_runner.go (1)

64-65: LGTM! Fixes dangerous variable shadowing.

Renaming to discoveryCtx prevents shadowing of the ctx context.Context parameter (line 45). This is particularly important here since ctx is actively used at lines 78 and 80 for context propagation. The previous shadowing could have caused subtle bugs if the discovery context assignment accidentally masked the real context in a refactoring scenario.

---

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