Support parent-scoped multi-file stacks - #2787
Merged
Merged
Conversation
Contributor
|
Tip Atmos Pro
No affected stacks workflow was detected for this pull request. 
|
Erik Osterman (Cloud Posse) (osterman)
temporarily deployed
to
screengrabs
July 23, 2026 03:20 — with 
GitHub Actions
Inactive
GitHub Actions
Inactive
Dependency Review✅ No vulnerabilities or license issues found.Scanned FilesNone |
Erik Osterman (Cloud Posse) (osterman)
marked this pull request as ready for review
July 23, 2026 05:16
Erik Osterman (Cloud Posse) (osterman)
requested review from
a team
as code owners
July 23, 2026 05:16
Erik Osterman (Cloud Posse) (osterman)
added
the
patch
Erik Osterman (Cloud Posse) (osterman)
temporarily deployed
to
screengrabs
July 23, 2026 05:18 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
Erik Osterman (Cloud Posse) (osterman)
temporarily deployed
to
screengrabs
July 23, 2026 05:21 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
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:
📝 WalkthroughWalkthroughAtmos now groups top-level manifests by logical stack identity while preserving parent-scoped resolution. Equivalent duplicate components use a canonical source; conflicting configurations and peer-only inheritance produce explicit errors. Tests, fixtures, PRD, blog, and roadmap entries cover the behavior. ChangesTop-Level Stack Composition
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant StackManifest
participant findComponentInStacks
participant ProcessComponentConfig
participant ValidateStacks
StackManifest->>findComponentInStacks: provide candidate parent manifest
findComponentInStacks->>ProcessComponentConfig: resolve parent-scoped component configuration
ProcessComponentConfig-->>findComponentInStacks: resolved component candidate
findComponentInStacks->>ValidateStacks: provide matching component configurations
ValidateStacks-->>findComponentInStacks: equivalent result or duplicate error
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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: 5
🧹 Nitpick comments (4)
internal/exec/validate_stacks.go (1)
232-232: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTerminate the new comment with a period.
As per coding guidelines, all Go comments must end with periods.
🤖 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/validate_stacks.go` at line 232, Update the new comment describing the merge context to end with a period, preserving its existing wording.Source: Coding guidelines
internal/exec/stack_processor_utils.go (3)
586-651: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winWrap errors with context before returning.
logicalStackIdentity(line 591) andm.DeepCopyMap(line 627) errors are returned raw, with no indication of which stack file failed. As per coding guidelines: "wrap errors, useerrors.Joinfor multiple errors,%wfor string context...".♻️ Proposed fix
identity, err := logicalStackIdentity(atmosConfig, results[i].stackFileName, results[i].deepMergedConfig) if err != nil { - return err + return fmt.Errorf("resolving logical stack identity for %q: %w", results[i].stackFileName, err) }deepMergedConfig, err := m.DeepCopyMap(results[index].deepMergedConfig) if err != nil { - return err + return fmt.Errorf("copying deep-merged config for %q: %w", results[index].stackFileName, err) }🤖 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/stack_processor_utils.go` around lines 586 - 651, Update addPeerComponentsForInheritance to wrap errors from logicalStackIdentity and m.DeepCopyMap with stack-file context before returning, using %w so the original errors remain inspectable. Identify the failing stack via results[i].stackFileName or results[index].stackFileName in the respective loops.Source: Coding guidelines
791-849: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAvoid deep-copying the whole stack config just to preserve component ownership.
originalConfigs[i]deep-copies the entiredeepMergedConfig(vars, settings, env, locals, imports, everything), butretainOwnedComponentsonly ever readsoriginalConfig[cfg.ComponentsSectionName]. For stacks with largevars/settingssections this is unnecessary CPU/memory overhead on every file, on top of the deep copy already performed insideaddPeerComponentsForInheritance.⚡ Proposed fix
originalConfigs := make([]map[string]any, count) for i := range processedResults { - originalConfig, err := m.DeepCopyMap(processedResults[i].deepMergedConfig) - if err != nil { - return nil, nil, nil, err + components, _ := processedResults[i].deepMergedConfig[cfg.ComponentsSectionName].(map[string]any) + originalComponents, err := m.DeepCopyMap(map[string]any{cfg.ComponentsSectionName: components}) + if err != nil { + return nil, nil, nil, err } - originalConfigs[i] = originalConfig + originalConfigs[i] = originalComponents }🤖 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/stack_processor_utils.go` around lines 791 - 849, Replace the full deep-copy loop that builds originalConfigs with a copy containing only each processed result’s component ownership section required by retainOwnedComponents. Preserve the existing indexing and error behavior, and keep retainOwnedComponents(finalConfig, originalConfigs[i]) unchanged unless needed to consume the reduced configuration.
549-868: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winBreak up
stack_processor_utils.goto stay under the file-length guideline.This file is 3,049 lines, well above the stated
below 600 linescap. Move the peer-inheritance helpers and related composition logic into a focused helper/test file, such asstack_processor_composition.go, and keep this file near or under the 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 `@internal/exec/stack_processor_utils.go` around lines 549 - 868, Split the peer-inheritance and composition logic out of stack_processor_utils.go into a focused file such as stack_processor_composition.go, moving logicalStackIdentity, addPeerComponentsForInheritance, and retainOwnedComponents together with any required imports and tests. Update ProcessYAMLConfigFiles to continue calling these helpers without changing behavior, and reduce stack_processor_utils.go to the stated below-600-line guideline.Source: Coding guidelines
🤖 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 `@docs/prd/top-level-stack-composition.md`:
- Around line 16-22: Update the ordered-list continuation indentation in the
logical stack requirements, especially items 1–5 and the corresponding section
around items 86–95, from 3 spaces to 4 spaces. Preserve the text and list
structure while ensuring continuation lines use indentation compatible with the
editorconfig.
In `@internal/exec/stack_processor_utils.go`:
- Around line 653-674: Update retainOwnedComponents so it still processes
finalComponents when originalConfig has no components section; only return early
when finalComponents is unavailable. Treat a nil originalComponents map as empty
while checking each componentType, allowing all injected peer components to be
removed when the manifest originally owned none.
- Around line 565-580: Update logicalStackIdentity to handle a nil atmosConfig
before accessing atmosConfig.Stacks or other configuration fields, returning the
appropriate stack-file-based identity for that case. Preserve the existing
manifest-name, name-template, and name-pattern precedence when atmosConfig is
non-nil.
In `@internal/exec/top_level_stack_composition_test.go`:
- Around line 13-16: Update initTopLevelStackCompositionConfig to construct the
fixture directory with filepath.Join using the existing parent, tests, fixtures,
scenarios, and fixture components, then pass that result to t.Chdir; add the
necessary filepath import.
In `@internal/exec/validate_stacks.go`:
- Around line 398-413: Update the validation errors in the stack manifest lookup
flow around stackConfig, components, componentTypeConfig, and componentConfig to
use suitable sentinel errors from errors/errors.go, adding them if necessary.
Wrap each sentinel with the existing manifest or component context so callers
can classify failures with errors.Is() while retaining the dynamic details.
---
Nitpick comments:
In `@internal/exec/stack_processor_utils.go`:
- Around line 586-651: Update addPeerComponentsForInheritance to wrap errors
from logicalStackIdentity and m.DeepCopyMap with stack-file context before
returning, using %w so the original errors remain inspectable. Identify the
failing stack via results[i].stackFileName or results[index].stackFileName in
the respective loops.
- Around line 791-849: Replace the full deep-copy loop that builds
originalConfigs with a copy containing only each processed result’s component
ownership section required by retainOwnedComponents. Preserve the existing
indexing and error behavior, and keep retainOwnedComponents(finalConfig,
originalConfigs[i]) unchanged unless needed to consume the reduced
configuration.
- Around line 549-868: Split the peer-inheritance and composition logic out of
stack_processor_utils.go into a focused file such as
stack_processor_composition.go, moving logicalStackIdentity,
addPeerComponentsForInheritance, and retainOwnedComponents together with any
required imports and tests. Update ProcessYAMLConfigFiles to continue calling
these helpers without changing behavior, and reduce stack_processor_utils.go to
the stated below-600-line guideline.
In `@internal/exec/validate_stacks.go`:
- Line 232: Update the new comment describing the merge context to end with a
period, preserving its existing wording.
🪄 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 Plus
Run ID: 3c37e459-1dae-4957-81b7-96375a7d1d08
📒 Files selected for processing (30)
docs/prd/top-level-stack-composition.mderrors/errors.gointernal/exec/describe_component.gointernal/exec/stack_processor_utils.gointernal/exec/top_level_stack_composition_test.gointernal/exec/utils.gointernal/exec/validate_stacks.gotests/fixtures/scenarios/top-level-stack-composition-conflict/atmos.yamltests/fixtures/scenarios/top-level-stack-composition-conflict/components/terraform/mock/main.tftests/fixtures/scenarios/top-level-stack-composition-conflict/stacks/catalog/shared.yamltests/fixtures/scenarios/top-level-stack-composition-conflict/stacks/parents/01-network.yamltests/fixtures/scenarios/top-level-stack-composition-conflict/stacks/parents/02-platform.yamltests/fixtures/scenarios/top-level-stack-composition-explicit-name/atmos.yamltests/fixtures/scenarios/top-level-stack-composition-explicit-name/components/terraform/mock/main.tftests/fixtures/scenarios/top-level-stack-composition-explicit-name/stacks/catalog/shared.yamltests/fixtures/scenarios/top-level-stack-composition-explicit-name/stacks/parents/01-network.yamltests/fixtures/scenarios/top-level-stack-composition-explicit-name/stacks/parents/02-platform.yamltests/fixtures/scenarios/top-level-stack-composition-isolated-inheritance/atmos.yamltests/fixtures/scenarios/top-level-stack-composition-isolated-inheritance/components/terraform/mock/main.tftests/fixtures/scenarios/top-level-stack-composition-isolated-inheritance/stacks/parents/01-network.yamltests/fixtures/scenarios/top-level-stack-composition-isolated-inheritance/stacks/parents/02-platform.yamltests/fixtures/scenarios/top-level-stack-composition-parent-scope/atmos.yamltests/fixtures/scenarios/top-level-stack-composition-parent-scope/components/terraform/mock/main.tftests/fixtures/scenarios/top-level-stack-composition-parent-scope/stacks/parents/01-network.yamltests/fixtures/scenarios/top-level-stack-composition-parent-scope/stacks/parents/02-platform.yamltests/fixtures/scenarios/top-level-stack-composition/atmos.yamltests/fixtures/scenarios/top-level-stack-composition/components/terraform/mock/main.tftests/fixtures/scenarios/top-level-stack-composition/stacks/catalog/shared.yamltests/fixtures/scenarios/top-level-stack-composition/stacks/parents/01-network.yamltests/fixtures/scenarios/top-level-stack-composition/stacks/parents/02-platform.yaml
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2787 +/- ##
=======================================
Coverage ? 81.84%
=======================================
Files ? 1793
Lines ? 173241
Branches ? 0
=======================================
Hits ? 141787
Misses ? 23656
Partials ? 7798
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Erik Osterman (Cloud Posse) (osterman)
temporarily deployed
to
screengrabs
July 23, 2026 16:40 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
Erik Osterman (Cloud Posse) (osterman)
added
minor
Erik Osterman (Cloud Posse) (osterman)
temporarily deployed
to
screengrabs
July 25, 2026 23:25 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
|
Note Release Documentation Complete ✅
Thank you! |
Erik Osterman (Cloud Posse) (osterman)
temporarily deployed
to
screengrabs
July 25, 2026 23:33 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
Erik Osterman (Cloud Posse) (osterman)
temporarily deployed
to
screengrabs
July 27, 2026 16:08 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
Erik Osterman (Cloud Posse) (osterman)
temporarily deployed
to
screengrabs
July 27, 2026 22:46 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
Erik Osterman (Cloud Posse) (osterman)
temporarily deployed
to
screengrabs
July 28, 2026 01:04 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
checkComponentStackMap validates terraform and helmfile component types with dedicated sentinel errors; every other component type falls through to the generic ErrInvalidComponentsSection default case, which was untested and showed zero coverage on the newly added lines in internal/exec/validate_stacks.go.
Erik Osterman (Cloud Posse) (osterman)
temporarily deployed
to
screengrabs
July 28, 2026 02:39 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
Andriy Knysh (aknysh)
temporarily deployed
to
screengrabs
July 28, 2026 18:13 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
Andriy Knysh (aknysh)
approved these changes
Jul 29, 2026
Contributor
|
Tip Atmos Pro
No affected stacks workflow was detected for this pull request.
|
|
These changes were released in v1.225.0-rc.1. |
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
2 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.
what
metadata.inheritsgraph self-contained while canonicalizing equivalent imported duplicates by lexical parent path.why
references
docs/prd/top-level-stack-composition.mdSummary by CodeRabbit
New Features
Bug Fixes
Documentation