Repository navigation
[codex] enable terraform plan scheduler concurrency - #2468
Merged
Andriy Knysh (aknysh) merged 20 commits intoJun 1, 2026
Merged
Andriy Knysh (aknysh) merged 20 commits into
Andriy Knysh (aknysh) merged 20 commits into
Conversation
Contributor
|
Tip Atmos Pro
No affected stacks workflow was detected for this pull request. 
|
Dependency Review✅ No vulnerabilities or license issues found.Scanned FilesNone |
This was referenced May 21, 2026
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## codex/dag-terraform-graph-bulk-path #2468 +/- ##
=======================================================================
+ Coverage 78.61% 78.68% +0.07%
=======================================================================
Files 1185 1186 +1
Lines 113476 114106 +630
=======================================================================
+ Hits 89210 89787 +577
- Misses 19337 19355 +18
- Partials 4929 4964 +35
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Mikhail Shirkov (shirkevich)
force-pushed
the
codex/dag-terraform-plan-concurrency
branch
from
May 22, 2026 10:21
58d9f1f to
71df0d8
Compare
Mikhail Shirkov (shirkevich)
force-pushed
the
codex/dag-terraform-graph-bulk-path
branch
from
May 25, 2026 07:53
1f0197e to
3d1c53c
Compare
Contributor
|
💥 This pull request now has conflicts. Could you fix it Mikhail Shirkov (@shirkevich)? 🙏 |
Mikhail Shirkov (shirkevich)
force-pushed
the
codex/dag-terraform-plan-concurrency
branch
from
May 25, 2026 07:56
c4ec7b7 to
524307c
Compare
Mikhail Shirkov (shirkevich)
marked this pull request as ready for review
May 26, 2026 18:07
Mikhail Shirkov (shirkevich)
force-pushed
the
codex/dag-terraform-graph-bulk-path
branch
from
May 27, 2026 18:13
3d1c53c to
713f3ff
Compare
Mikhail Shirkov (shirkevich)
force-pushed
the
codex/dag-terraform-plan-concurrency
branch
from
May 27, 2026 18:13
524307c to
e004395
Compare
Collaborator
Author
|
CodeRabbit (@coderabbitai) review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
💤 Files with no reviewable changes (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdds plan/apply/destroy CLI flags and schema fields; introduces TerraformExecution/TerraformExecutionResult and context-aware per-component execution with streaming/capture and hooks; implements LinePrefixWriter; refactors scheduler adapter for concurrency, grouped/streamed output, change detection, and deterministic plan-summary JSON; skips Terraform runtime dirs in sync/hash; updates/extends tests, fixtures, and snapshots. ChangesTerraform concurrent plan execution with output control
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
cmd/terraform/options_test.go (1)
36-55: ⚡ Quick winCover the other new plan option fields in these tests.
You added checks for
MaxConcurrency, butPlanLogOrder,PlanHideNoChanges, andPlanSummaryFileare also new parse/propagation paths and currently unasserted.Proposed test additions
@@ setup: func(v *viper.Viper) { @@ v.Set("max-concurrency", 4) + v.Set("log-order", "grouped") + v.Set("hide-no-changes", true) + v.Set("execution-summary-file", "/tmp/summary.json") }, expected: &TerraformRunOptions{ @@ MaxConcurrency: 4, + PlanLogOrder: "grouped", + PlanHideNoChanges: true, + PlanSummaryFile: "/tmp/summary.json", }, @@ assert.Equal(t, tt.expected.MaxConcurrency, result.MaxConcurrency, "MaxConcurrency should match") + assert.Equal(t, tt.expected.PlanLogOrder, result.PlanLogOrder, "PlanLogOrder should match") + assert.Equal(t, tt.expected.PlanHideNoChanges, result.PlanHideNoChanges, "PlanHideNoChanges should match") + assert.Equal(t, tt.expected.PlanSummaryFile, result.PlanSummaryFile, "PlanSummaryFile should match") @@ opts: &TerraformRunOptions{ @@ MaxConcurrency: 4, + PlanLogOrder: "grouped", + PlanHideNoChanges: true, + PlanSummaryFile: "/tmp/summary.json", }, checkInfo: func(t *testing.T, info *schema.ConfigAndStacksInfo) { @@ assert.Equal(t, 4, info.MaxConcurrency) + assert.Equal(t, "grouped", info.TerraformPlanLogOrder) + assert.True(t, info.TerraformPlanHideNoChanges) + assert.Equal(t, "/tmp/summary.json", info.TerraformPlanSummaryFile) },As per coding guidelines "
**/*_test.go: Every new feature must include comprehensive unit tests targeting >80% code coverage for all packages."Also applies to: 95-102, 257-257, 390-400
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/terraform/options_test.go` around lines 36 - 55, Update the unit test in options_test.go to assert the newly-parsed plan options on TerraformRunOptions: set the input flags in the test's v.Set calls for "plan-log-order", "plan-hide-no-changes", and "plan-summary-file" and add the corresponding expected fields PlanLogOrder, PlanHideNoChanges, and PlanSummaryFile to the expected TerraformRunOptions struct (alongside the already-added MaxConcurrency). Ensure you cover both positive and any alternate test cases referenced (lines around 95-102, 257, and 390-400) so the parser and propagation of PlanLogOrder, PlanHideNoChanges, and PlanSummaryFile are exercised and validated.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/io/line_prefix_writer.go`:
- Around line 58-60: Flush()/flushCompleteLinesLocked() currently clear w.buffer
before calling writeLine, which loses buffered data on write failure; change the
flow so you only remove bytes from w.buffer after writeLine returns success (or
restore the buffer on error). Specifically, in flushCompleteLinesLocked() and
the similar block around lines 69-72, build the line to write without truncating
w.buffer, call writeLine(line), and only on nil error advance/truncate w.buffer
(or reassign the remaining slice) so failed writes leave w.buffer intact for
retry.
---
Nitpick comments:
In `@cmd/terraform/options_test.go`:
- Around line 36-55: Update the unit test in options_test.go to assert the
newly-parsed plan options on TerraformRunOptions: set the input flags in the
test's v.Set calls for "plan-log-order", "plan-hide-no-changes", and
"plan-summary-file" and add the corresponding expected fields PlanLogOrder,
PlanHideNoChanges, and PlanSummaryFile to the expected TerraformRunOptions
struct (alongside the already-added MaxConcurrency). Ensure you cover both
positive and any alternate test cases referenced (lines around 95-102, 257, and
390-400) so the parser and propagation of PlanLogOrder, PlanHideNoChanges, and
PlanSummaryFile are exercised and validated.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 713f88ad-040b-418e-8a89-e2187359761d
📒 Files selected for processing (15)
cmd/terraform/options.gocmd/terraform/options_test.gocmd/terraform/plan.gocmd/terraform/utils.gointernal/exec/terraform_query.gointernal/exec/terraform_utils_test.gopkg/io/line_prefix_writer.gopkg/io/line_prefix_writer_test.gopkg/provisioner/workdir/fs.gopkg/provisioner/workdir/fs_test.gopkg/scheduler/adapters/terraform.gopkg/scheduler/adapters/terraform_test.gopkg/schema/schema.gotests/snapshots/TestCLICommands_config_alias_tp_--help_shows_terraform_plan_help.stdout.goldentests/snapshots/TestCLICommands_tf_plan_help_shows_inherited_stack_flag.stdout.golden
Mikhail Shirkov (shirkevich)
force-pushed
the
codex/dag-terraform-graph-bulk-path
branch
from
May 27, 2026 19:08
713f3ff to
aeb50f6
Compare
Contributor
|
💥 This pull request now has conflicts. Could you fix it Mikhail Shirkov (@shirkevich)? 🙏 |
Mikhail Shirkov (shirkevich)
force-pushed
the
codex/dag-terraform-plan-concurrency
branch
from
May 27, 2026 19:20
e004395 to
6914f4d
Compare
Collaborator
Author
|
CodeRabbit (@coderabbitai) review |
Collaborator
Author
|
CodeRabbit (@coderabbitai) review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/scheduler/adapters/terraform.go (1)
1-1244: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy liftFile exceeds length limit.
This file is 1244 lines, more than double the 600-line guideline. Consider splitting into focused modules (e.g., separate files for graph building, output handling, execution, and summary generation).
As per coding guidelines: "Keep files small and focused (<600 lines); use one cmd/impl per file; co-locate tests; never use
//revive:disable:file-length-limit"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/scheduler/adapters/terraform.go` around lines 1 - 1244, The file is too large; split it into focused modules: extract graph-building logic (functions/types: BuildTerraformGraph, addTerraformDependencies, terraformDependencies, parseLegacyDependsOn*, walkTerraformComponents, sortedStackNames, sortedComponentNames, sortedGraphNodeIDs, sortedCopy, containsString, terraformNodeID, cloneTerraformNodeMetadata) into a graph.go; move scheduling/dispatch logic (TerraformDispatcher type and methods Dispatch, lockTerraformResource, shouldSkipByQuery, selectedTerraformNodeIDs, matchesTerraformSelection, evaluateTerraformQuery, processedCount, terraformNodeLabel) into dispatcher.go; move output/logging concerns (type terraformOutput and methods newTerraformOutput, captureOutput, nodeWriters, openNodeLogFiles, combineWriters, closeTerraformLogFiles, finishNode, replayGroupedOutput, writeGroupedOutputMarker, safeTerraformLogName, terraformOutputCommand, terraformLogDir, terraformPlanHideNoChangesEnabled, terraformPlanHideNoChanges constants) into output.go; move summary/timing (terraformSummary types, terraformNodeTiming/timings and methods Start/Complete/Get, writeTerraformSummary) into summary.go; and leave small shared utilities and locks (terraformResourceLocks, newTerraformResourceLocks, Lock, terraformResourceKey, componentInfoPath, componentField, metadataComponent, terraformPlanHasNoChanges, terraformPlanChangedError, terraformExitCode, terraformPlanChanged, effectiveTerraformMaxConcurrency, supportsTerraformConcurrency, containsTerraformFlag, hasTerraformAutoApprove*, requiresTerraformAutoApprove) in utils.go; ensure package remains adapters, adjust file-level imports to compile, keep function signatures unchanged and test/build after moving.
🧹 Nitpick comments (2)
pkg/scheduler/adapters/terraform.go (2)
358-420: ⚡ Quick winConsider extracting node execution setup into a helper.
The
Dispatchfunction is ~63 lines. To maintain the funlen limit, consider extracting the execution preparation block (lines 377-398) into aprepareNodeExecutionhelper.As per coding guidelines: "Golangci-lint enforces funlen: lines: 60, statements: 40"
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/scheduler/adapters/terraform.go` around lines 358 - 420, The Dispatch method in TerraformDispatcher is too long; extract the node execution preparation block that builds nodeInfo, acquires the unlock via lockTerraformResource, constructs TerraformExecution (including setting Context, Info, Stdout, Stderr, Flush, CaptureOutput) and returns the execution and unlock function into a new helper prepareNodeExecution (or prepareTerraformExecution) that returns (TerraformExecution, func(), map[string]string, error) or similar; update Dispatch to call prepareNodeExecution before calling d.executor and keep subsequent logic (execResult handling, Flush, d.output.finishNode, outcome computation) unchanged, ensuring you preserve setting nodeInfo.Component/ComponentFromArg/Stack/StackFromArg and usage of d.output.nodeWriters and d.lockTerraformResource.
96-166: ⚡ Quick winConsider extracting validation logic into a helper function.
The
ExecuteTerraformfunction is ~70 lines and may exceed thefunlenlimit. Consider extracting the validation block (lines 99-107) into avalidateTerraformOptionshelper to keep this orchestrator flat.As per coding guidelines: "Golangci-lint enforces funlen: lines: 60, statements: 40; when refactoring high-complexity functions, extract blocks into named helper functions and keep orchestrator as flat linear pipeline"
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/scheduler/adapters/terraform.go` around lines 96 - 166, Extract the initial validation block in ExecuteTerraform into a new helper function validateTerraformOptions(opts TerraformOptions) error that checks opts.AtmosConfig, opts.Info, and opts.Executor and returns the same formatted errors (using errUtils.ErrInvalidConfig) so ExecuteTerraform becomes a flat orchestrator; replace the inline checks (currently lines checking opts.AtmosConfig, opts.Info, opts.Executor) with a single call to validateTerraformOptions at the top of ExecuteTerraform and update any callers/tests if needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmd/terraform/plan.go`:
- Line 130: PlanCompatFlags() registers compat flags for the terraform plan
command but there is no mapping for the legacy --hide-no-changes, causing
unknown-flag errors; update the compat mapping in cmd/terraform/compat_flags.go
to translate the old flag into the new flags.WithStringSliceFlag("hide", ...)
form (e.g., map "--hide-no-changes" => "--hide no-changes") or alternatively
register an alias flag that forwards to the "hide" slice, and update the command
help/docs to mention the alias; locate symbols PlanCompatFlags,
flags.WithStringSliceFlag("hide", ...), and the compat mapping in
compat_flags.go to implement the translation/alias.
---
Outside diff comments:
In `@pkg/scheduler/adapters/terraform.go`:
- Around line 1-1244: The file is too large; split it into focused modules:
extract graph-building logic (functions/types: BuildTerraformGraph,
addTerraformDependencies, terraformDependencies, parseLegacyDependsOn*,
walkTerraformComponents, sortedStackNames, sortedComponentNames,
sortedGraphNodeIDs, sortedCopy, containsString, terraformNodeID,
cloneTerraformNodeMetadata) into a graph.go; move scheduling/dispatch logic
(TerraformDispatcher type and methods Dispatch, lockTerraformResource,
shouldSkipByQuery, selectedTerraformNodeIDs, matchesTerraformSelection,
evaluateTerraformQuery, processedCount, terraformNodeLabel) into dispatcher.go;
move output/logging concerns (type terraformOutput and methods
newTerraformOutput, captureOutput, nodeWriters, openNodeLogFiles,
combineWriters, closeTerraformLogFiles, finishNode, replayGroupedOutput,
writeGroupedOutputMarker, safeTerraformLogName, terraformOutputCommand,
terraformLogDir, terraformPlanHideNoChangesEnabled, terraformPlanHideNoChanges
constants) into output.go; move summary/timing (terraformSummary types,
terraformNodeTiming/timings and methods Start/Complete/Get,
writeTerraformSummary) into summary.go; and leave small shared utilities and
locks (terraformResourceLocks, newTerraformResourceLocks, Lock,
terraformResourceKey, componentInfoPath, componentField, metadataComponent,
terraformPlanHasNoChanges, terraformPlanChangedError, terraformExitCode,
terraformPlanChanged, effectiveTerraformMaxConcurrency,
supportsTerraformConcurrency, containsTerraformFlag, hasTerraformAutoApprove*,
requiresTerraformAutoApprove) in utils.go; ensure package remains adapters,
adjust file-level imports to compile, keep function signatures unchanged and
test/build after moving.
---
Nitpick comments:
In `@pkg/scheduler/adapters/terraform.go`:
- Around line 358-420: The Dispatch method in TerraformDispatcher is too long;
extract the node execution preparation block that builds nodeInfo, acquires the
unlock via lockTerraformResource, constructs TerraformExecution (including
setting Context, Info, Stdout, Stderr, Flush, CaptureOutput) and returns the
execution and unlock function into a new helper prepareNodeExecution (or
prepareTerraformExecution) that returns (TerraformExecution, func(),
map[string]string, error) or similar; update Dispatch to call
prepareNodeExecution before calling d.executor and keep subsequent logic
(execResult handling, Flush, d.output.finishNode, outcome computation)
unchanged, ensuring you preserve setting
nodeInfo.Component/ComponentFromArg/Stack/StackFromArg and usage of
d.output.nodeWriters and d.lockTerraformResource.
- Around line 96-166: Extract the initial validation block in ExecuteTerraform
into a new helper function validateTerraformOptions(opts TerraformOptions) error
that checks opts.AtmosConfig, opts.Info, and opts.Executor and returns the same
formatted errors (using errUtils.ErrInvalidConfig) so ExecuteTerraform becomes a
flat orchestrator; replace the inline checks (currently lines checking
opts.AtmosConfig, opts.Info, opts.Executor) with a single call to
validateTerraformOptions at the top of ExecuteTerraform and update any
callers/tests if needed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 87eeada7-80d4-43c6-bca4-1e21a9d0ef67
📒 Files selected for processing (36)
cmd/terraform/apply.gocmd/terraform/destroy.gocmd/terraform/options.gocmd/terraform/options_test.gocmd/terraform/plan.gocmd/terraform/utils.gointernal/exec/terraform_all.gointernal/exec/terraform_query.gopkg/scheduler/adapters/terraform.gopkg/scheduler/adapters/terraform_test.gopkg/schema/schema.gotests/cli_auth_console_test.gotests/cli_auth_login_provider_test.gotests/cli_describe_identity_test.gotests/cli_double_hyphen_test.gotests/cli_identity_flag_test.gotests/cli_interactive_test.gotests/cli_plugin_cache_test.gotests/cli_profile_test.gotests/cli_skip_init_test.gotests/cli_terraform_test.gotests/cli_test.gotests/cli_workdir_test.gotests/fixtures/scenarios/terraform-floci-dag/atmos.yamltests/fixtures/scenarios/terraform-floci-dag/components/terraform/alias-shared/main.tftests/fixtures/scenarios/terraform-floci-dag/components/terraform/bucket-marker/main.tftests/fixtures/scenarios/terraform-floci-dag/components/terraform/final-marker/main.tftests/fixtures/scenarios/terraform-floci-dag/components/terraform/queue-marker/main.tftests/fixtures/scenarios/terraform-floci-dag/components/terraform/seed/main.tftests/fixtures/scenarios/terraform-floci-dag/components/terraform/topic-marker/main.tftests/fixtures/scenarios/terraform-floci-dag/stacks/deploy/local.yamltests/snapshots/TestCLICommands_atmos_terraform_apply_--help.stdout.goldentests/snapshots/TestCLICommands_atmos_terraform_apply_help.stdout.goldentests/snapshots/TestCLICommands_config_alias_tp_--help_shows_terraform_plan_help.stdout.goldentests/snapshots/TestCLICommands_tf_plan_help_shows_inherited_stack_flag.stdout.goldentests/terraform_floci_dag_test.go
✅ Files skipped from review due to trivial changes (4)
- tests/snapshots/TestCLICommands_atmos_terraform_apply_help.stdout.golden
- tests/snapshots/TestCLICommands_config_alias_tp_--help_shows_terraform_plan_help.stdout.golden
- tests/snapshots/TestCLICommands_atmos_terraform_apply_--help.stdout.golden
- tests/snapshots/TestCLICommands_tf_plan_help_shows_inherited_stack_flag.stdout.golden
🚧 Files skipped from review as they are similar to previous changes (4)
- pkg/schema/schema.go
- cmd/terraform/options.go
- internal/exec/terraform_query.go
- pkg/scheduler/adapters/terraform_test.go
Erik Osterman (Cloud Posse) (osterman)
approved these changes
May 30, 2026
* Route terraform affected through scheduler * test(terraform): add floci affected scheduler validation * Add Terraform scheduler failure mode flags * Support Git worktreeConfig repositories * [autocommit] formatting fixes * Address CodeRabbit affected scheduler feedback * Use a single Terraform failure mode flag * Format Floci affected DAG test calls * Document affected scheduler helpers * Fix worktreeConfig test cleanup on Windows * Increase affected scheduler test coverage --------- Co-authored-by: atmos-pro[bot] <173522224+atmos-pro[bot]@users.noreply.github.com>
github-actions
Bot
added
size/xl
Member
|
Mikhail Shirkov (@shirkevich) please resolve the conflicts |
Contributor
|
💥 This pull request now has conflicts. Could you fix it Mikhail Shirkov (@shirkevich)? 🙏 |
…ath' into codex/fix-pr2468-conflict # Conflicts: # errors/errors.go # pkg/git/git.go # pkg/scheduler/adapters/terraform.go
Collaborator
Author
|
CodeRabbit (@coderabbitai) review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/exec/terraform_utils_test.go (1)
244-319:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winGuard the new gomonkey tests on darwin/arm64.
These tests use
gomonkeybut don't carry the Apple Silicon skip that this file already applies later. On macOS ARM64 that can SIGBUS the test binary instead of producing a clean skip.Suggested guard.
+ if runtime.GOOS == "darwin" && runtime.GOARCH == "arm64" { + t.Skip("gomonkey binary patching is not supported on macOS ARM64") + }Also applies to: 321-435, 437-542
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/exec/terraform_utils_test.go` around lines 244 - 319, Add a runtime guard at the top of each gomonkey-based test in this file (including TestExecuteTerraformQueryRoutesThroughSchedulerAdapter and the two other tests that use gomonkey) to skip on Apple Silicon macOS: check if runtime.GOOS == "darwin" && runtime.GOARCH == "arm64" and call t.Skip with a short message; this prevents SIGBUS on macOS/arm64 while leaving the rest of the test logic (patches.ApplyFunc, ExecuteTerraformQuery, scheduleradapters.ExecuteTerraform, etc.) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmd/terraform/apply.go`:
- Around line 124-130: The apply command is exposing an apply-specific
concurrency flag that should be omitted for this PR; remove the
flags.WithIntFlag("max-concurrency", "", 1, ...) entry and the corresponding
flags.WithEnvVars("max-concurrency", "ATMOS_TERRAFORM_APPLY_MAX_CONCURRENCY")
line from the flags list in cmd/terraform/apply.go (where the apply command
flags are registered) so that --max-concurrency and its env var are not wired
into terraform apply until apply concurrency is implemented end-to-end.
In `@pkg/git/git.go`:
- Around line 247-260: The current worktreeConfigTolerantStorer.Config mutates
and returns the real repository config after calling
removeWorktreeConfigExtension, which causes callers (e.g., GetRepoConfig ->
repo.Storer.SetConfig) to persist the stripped extension back to the repo. Fix
by returning a copy instead of the original: in
worktreeConfigTolerantStorer.Config() clone or deep-copy the config.Config
returned by s.Storer.Config(), call removeWorktreeConfigExtension on that copy,
and return the modified copy; alternatively implement a corresponding SetConfig
wrapper that preserves/reapplies extensions if cloning is not feasible. Ensure
you reference worktreeConfigTolerantStorer.Config, s.Storer.Config,
removeWorktreeConfigExtension and repo.Storer.SetConfig when making the change.
- Around line 264-272: In openWorktreeConfigTolerantRepo, do not discard the
native-git failure from gitRepositoryPaths when it returns an error; instead
return a wrapped error combining the gitRepositoryPaths error with the original
go-git error (use errors.Join or wrap with the static errors from
errors/errors.go per project guidelines) so callers can see both contexts;
update the return in the gitRepositoryPaths error branch to return nil and
errors.Join(err, originalErr) (and apply the same change to the similar call
site around lines 313-320 that also discards the gitRepositoryPaths error).
---
Outside diff comments:
In `@internal/exec/terraform_utils_test.go`:
- Around line 244-319: Add a runtime guard at the top of each gomonkey-based
test in this file (including
TestExecuteTerraformQueryRoutesThroughSchedulerAdapter and the two other tests
that use gomonkey) to skip on Apple Silicon macOS: check if runtime.GOOS ==
"darwin" && runtime.GOARCH == "arm64" and call t.Skip with a short message; this
prevents SIGBUS on macOS/arm64 while leaving the rest of the test logic
(patches.ApplyFunc, ExecuteTerraformQuery, scheduleradapters.ExecuteTerraform,
etc.) unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 92471425-e1a1-4c89-a30f-1d78a9997757
📒 Files selected for processing (20)
cmd/terraform/apply.gocmd/terraform/destroy.gocmd/terraform/options.gocmd/terraform/options_test.gocmd/terraform/plan.gocmd/terraform/utils.goerrors/errors.gogo.modinternal/exec/terraform_affected.gointernal/exec/terraform_utils_test.gopkg/git/git.gopkg/git/git_test.gopkg/scheduler/adapters/terraform.gopkg/scheduler/adapters/terraform_test.gopkg/schema/schema.gotests/snapshots/TestCLICommands_atmos_terraform_apply_--help.stdout.goldentests/snapshots/TestCLICommands_atmos_terraform_apply_help.stdout.goldentests/snapshots/TestCLICommands_config_alias_tp_--help_shows_terraform_plan_help.stdout.goldentests/snapshots/TestCLICommands_tf_plan_help_shows_inherited_stack_flag.stdout.goldentests/terraform_floci_dag_test.go
💤 Files with no reviewable changes (8)
- tests/snapshots/TestCLICommands_atmos_terraform_apply_help.stdout.golden
- tests/snapshots/TestCLICommands_atmos_terraform_apply_--help.stdout.golden
- tests/snapshots/TestCLICommands_tf_plan_help_shows_inherited_stack_flag.stdout.golden
- tests/snapshots/TestCLICommands_config_alias_tp_--help_shows_terraform_plan_help.stdout.golden
- tests/terraform_floci_dag_test.go
- pkg/schema/schema.go
- pkg/scheduler/adapters/terraform_test.go
- pkg/scheduler/adapters/terraform.go
🚧 Files skipped from review as they are similar to previous changes (5)
- cmd/terraform/plan.go
- cmd/terraform/destroy.go
- cmd/terraform/options.go
- cmd/terraform/options_test.go
- cmd/terraform/utils.go
Member
|
Mikhail Shirkov (@shirkevich) thanks, please address the comments |
Collaborator
Author
|
CodeRabbit (@coderabbitai) review |
Contributor
✅ Actions performedReview triggered.
|
Andriy Knysh (aknysh)
approved these changes
Jun 1, 2026
Andriy Knysh (aknysh)
merged commit Jun 1, 2026
45c562f
into
codex/dag-terraform-graph-bulk-path
53 checks passed
Contributor
|
Tip Atmos Pro
No affected stacks workflow was detected for this pull request.
|
Andriy Knysh (aknysh)
added a commit
that referenced
this pull request
Jun 6, 2026
* Add process and I/O execution foundation * Address process I/O review feedback * Address CodeRabbit rereview feedback * Rename output pipeline API * add dag scheduler core * Address CodeRabbit scheduler feedback * Ensure scheduler aggregates non-success statuses * route terraform bulk execution through scheduler * fix credential store concurrent initialization * serialize terraform aliases sharing component paths * remove temporary terraform concurrency override * route terraform all through graph scheduler * Address CodeRabbit Terraform adapter feedback * Address CodeRabbit graph adapter feedback * [autocommit] formatting fixes * Preserve dependency-order log in scheduler path * Apply gofumpt wrapping to scheduler tests * [codex] enable terraform plan scheduler concurrency (#2468) * enable terraform plan scheduler concurrency * complete terraform plan concurrency output semantics * fix pre-commit formatting * update terraform plan help snapshots * allow workdir aliases to run concurrently * Skip Terraform runtime dirs in workdir sync * Add Terraform summary node timings * Add coverage for Terraform plan concurrency paths * Allow Terraform aliases without workdir * Address CodeRabbit plan concurrency feedback * Test concurrent line prefix writes * Clarify OpenTofu workdir runtime dirs * Document Terraform scheduler helpers * Restore Terraform all dependency order message * Normalize Terraform resource key test expectation * PR5: Concurrent Terraform apply and destroy (#2474) * enable terraform apply destroy concurrency * test(terraform): add floci apply destroy validation * Address CodeRabbit apply destroy feedback * Use portable Floci fixture logging * Apply Floci test formatting * Format terraform run helper * Respect false auto-approve flag values * Address Terraform adapter review nits * Format Floci DAG test calls * Address Floci test review nitpicks * Fix test comment punctuation * Use extensible plan hide flag * Route Terraform affected through scheduler (#2519) * Route terraform affected through scheduler * test(terraform): add floci affected scheduler validation * Add Terraform scheduler failure mode flags * Support Git worktreeConfig repositories * [autocommit] formatting fixes * Address CodeRabbit affected scheduler feedback * Use a single Terraform failure mode flag * Format Floci affected DAG test calls * Document affected scheduler helpers * Fix worktreeConfig test cleanup on Windows * Increase affected scheduler test coverage --------- Co-authored-by: atmos-pro[bot] <173522224+atmos-pro[bot]@users.noreply.github.com> * Address CodeRabbit follow-ups for plan concurrency --------- Co-authored-by: atmos-pro[bot] <173522224+atmos-pro[bot]@users.noreply.github.com> * docs: add terraform dag release notes * docs: polish terraform dag changelog * style: gofumpt terraform environment setup * fix(terraform): include captured output in scheduler errors * fix(terraform): address review feedback * chore(terraform): address coderabbit nitpicks --------- Co-authored-by: Erik Osterman (Cloud Posse) <erik@cloudposse.com> Co-authored-by: atmos-pro[bot] <173522224+atmos-pro[bot]@users.noreply.github.com> Co-authored-by: Andriy Knysh <aknysh@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
--max-concurrencywiring for graph-backed Terraform bulk execution--max-concurrency 1plan; non-plan subcommands remain behaviorally sequential in PR40clean,2changes, non-zero failure--log-order=streamfor line-buffered, labeled live output--log-order=groupedfor per-component grouped replay--hide-no-changesto suppress grouped replay for no-op plans.atmos/logs/terraform/plan--execution-summary-filefor deterministic JSON execution summaries with per-nodestarted_at,finished_at, andduration_msfields.terraformandterraform.tfstate.dStacking
This PR is stacked on #2466 and targets
codex/dag-terraform-graph-bulk-path.Scope
atmos terraform plan --all --max-concurrency Natmos terraform plan --components ... --max-concurrency Natmos terraform plan --query ... --max-concurrency NOut of scope:
--affectedapply,deploy, ordestroyNotes
Concurrency remains opt-in. The default value is
1, which preserves the sequential behavior established in PR 3.provision.workdir.enabled=trueis not required to runplan,apply, ordestroy. In PR4, concurrent execution is enabled only forplan; non-plan subcommands continue to resolve through the same graph-backed path with effective sequential execution.Concurrent plan mode still requires a non-interactive identity value when
--max-concurrency > 1, but it no longer rejects components that do not enable workdir provisioning. Instead, the adapter locks by execution resource:This keeps the default/non-workdir Terraform behavior debuggable while allowing safe parallelism across independent component directories.
--hide-no-changesimplies grouped output because live stream output cannot be hidden after it has already been written. The no-change detector strips ANSI before matching Terraform/OpenTofu no-change messages.Validation
Additional local validation after the no-workdir locking update:
Adapter package coverage after the update: 80.7%.
Downstream smoke test
Built
build/atmosfrom this branch and ran anonymized private downstream smoke tests with an explicit identity.Workdir-enabled full
terraform plan --allmatrix, rerun after confirming no competing Terraform process was active:--max-concurrencyNotes:
fatal error: concurrent map writesviper.(*Viper).BindEnvauth panic.terraformprovider cache copy failure after the workdir sync fix--hide-no-changeswas manually validated downstream and correctly suppresses no-op plan groupsAdditional workdir-enabled alias smoke, using three logical aliases that share one physical Terraform component directory:
--max-concurrencyAdditional workdir-disabled validation, after removing the downstream default workdir setting and rebuilding this branch:
terraform plan --all, c8, grouped + hide no changesterraform plan --all, c8, grouped + execution summaryNo-workdir findings:
provision.workdir.enabled=truepreflight errorconcurrent map writesSummary by CodeRabbit
New Features
Improvements
Tests