Fix component menu to filter abstract and disabled components#1977
Fix component menu to filter abstract and disabled components#1977
Conversation
…us and shell completion When running 'atmos terraform plan -s stack-name' without specifying a component, the interactive "Choose a component" menu now correctly filters out: - Abstract components (metadata.type: abstract) that cannot be deployed - Disabled components (metadata.enabled: false) that cannot be deployed - Components not in the specified stack when --stack is provided This filtering also applies to shell tab completion. Added helper functions isComponentDeployable() and filterDeployableComponents() to centralize the filtering logic. Updated PromptForComponent() to accept a stack parameter for stack-scoped filtering. All callers updated to pass the stack parameter. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Dependency Review✅ No vulnerabilities or license issues found.Scanned Files
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1977 +/- ##
==========================================
+ Coverage 74.73% 74.91% +0.17%
==========================================
Files 777 777
Lines 71213 71302 +89
==========================================
+ Hits 53221 53413 +192
+ Misses 14526 14408 -118
- Partials 3466 3481 +15
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
📝 WalkthroughWalkthroughThis PR makes component selection stack-aware across Terraform CLI flows by changing PromptForComponent to accept a stack parameter, adds stack-filtered component completion and deployability checks, updates callers to pass/validate stacks, and adds extensive tests plus documentation and a roadmap entry. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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}
- Add DI variables to cmd/terraform/shared/prompt.go for testability - Add comprehensive tests for listTerraformComponents, listTerraformComponentsForStack, listStacksForComponent, listAllStacks, and componentsArgCompletionWithStack - Add tests for utils.go helper functions: isMultiComponentExecution, hasMultiComponentFlags, hasNonAffectedMultiFlags, hasSingleComponentFlags, handlePathResolutionError, handleInteractiveComponentStackSelection - Extract common mock setup to helper function to avoid duplication - Increases cmd/terraform/shared coverage from 30% to 87.9% Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
github-actions
bot
added
size/xl
|
Warning This PR exceeds the recommended limit of 1,000 lines.Large PRs are difficult to review and may be rejected due to their size. Please verify that this PR does not address multiple issues. |
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)
cmd/terraform/shared/prompt.go (1)
182-324: Honor global config flags during stack/component listing.
These helpers initializeConfigAndStacksInfoas empty, which can ignore--config,--config-path,--base-path, and--profileduring prompts/completions, leading to options from the wrong config. Please thread parsed global flags intoinitCliConfig(e.g., viaflags.ParseGlobalFlagson the callingcmd) before listing stacks/components. Based on learnings, ...
🤖 Fix all issues with AI agents
In `@website/src/data/roadmap.js`:
- Line 193: The roadmap entry for 'Smarter component selection filtering'
(object with label 'Smarter component selection filtering', pr: 1977, changelog:
'component-selection-filtering') is marked shipped for q1-2026 but the quarters
state still lists q4-2025 as current and q1-2026 as planned; update the quarters
data to mark q4-2025 completed and set q1-2026 as current, and if PR 1977 is
classified as minor/major, increment the DX initiative progress value in the
same roadmap data structure to reflect the shipped milestone.
🧹 Nitpick comments (2)
cmd/terraform/shared/prompt_test.go (1)
326-524: Good coverage of deployability rules; add cmd.NewTestKit(t) for cmd/ test isolation.
These are solid, but per cmd test hygiene, initializecmd.NewTestKit(t)to auto-clean RootCmd state and avoid cross-test leakage. As per coding guidelines, ...cmd/terraform/utils_test.go (1)
345-395: Multi-component execution tests are solid; add cmd.NewTestKit(t).
Please initializecmd.NewTestKit(t)for cmd/ tests to ensure RootCmd cleanup between tests. As per coding guidelines, ...
- Mark Q4 2025 as completed - Mark Q1 2026 as current Addresses CodeRabbit feedback about quarter status alignment with shipped Q1 2026 milestone. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
When running `atmos terraform plan --stack <invalid>` without a component, the command would exit silently. Now it validates the stack exists and shows a helpful error with available stacks. Changes: - Add ValidateStackExists() to cmd/terraform/shared/prompt.go - Add stack validation in handleInteractiveComponentStackSelection() - Add WithCausef() method to ErrorBuilder for formatted cause messages - Add comprehensive tests for both features Example error output: Error: invalid stack: stack `demo` does not exist Explanation: The specified stack was not found in the configuration Hint: Available stacks: acme-west-dev, acme-west-prod, acme-west-staging Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
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)
cmd/terraform/shared/prompt_test.go (1)
294-296: Godot: comment line missing trailing period.
Add a period to satisfy the comment-style linter.🩹 Suggested fix
- // This tests the branching logic based on args, not the actual completion values - // since those require config initialization. + // This tests the branching logic based on args, not the actual completion values since those require config initialization.
🤖 Fix all issues with AI agents
In `@cmd/terraform/shared/prompt.go`:
- Around line 183-191: listTerraformComponents currently builds an empty
schema.ConfigAndStacksInfo and calls initCliConfig, which ignores global CLI
selectors (--base-path, --config, --config-path, --profile) and returns wrong
stacks/components; fix by plumbing parsed global flags into all
listing/validation paths: call flags.ParseGlobalFlags to populate a
schema.ConfigAndStacksInfo and pass that populated struct into initCliConfig (or
InitCliConfig) instead of an empty one, and update signatures/usages of
listTerraformComponents, listTerraformComponentsForStack,
listStacksForComponent, listAllStacks and ValidateStackExists to accept and use
the ConfigAndStacksInfo (or reuse an existing cliConfig variable) so the global
selectors are honored during stack/component enumeration and validation.
🧹 Nitpick comments (1)
errors/builder_test.go (1)
473-510: Consider a table-driven structure for the multi-scenario test.
Keeps the cases uniform and aligns with the repo’s test style. As per coding guidelines, prefer table-driven tests for multiple scenarios.♻️ Proposed refactor
func TestErrorBuilder_WithCausef(t *testing.T) { - t.Run("creates formatted cause error", func(t *testing.T) { - stackName := "dev-us-east-1" - - err := Build(ErrInvalidStack). - WithCausef("stack `%s` does not exist", stackName). - Err() - - assert.Error(t, err) - assert.True(t, errors.Is(err, ErrInvalidStack)) - assert.Contains(t, err.Error(), "stack `dev-us-east-1` does not exist") - }) - - t.Run("works with multiple format arguments", func(t *testing.T) { - err := Build(ErrInvalidComponent). - WithCausef("component `%s` not found in stack `%s`", "vpc", "prod"). - Err() - - assert.Error(t, err) - assert.True(t, errors.Is(err, ErrInvalidComponent)) - assert.Contains(t, err.Error(), "component `vpc` not found in stack `prod`") - }) - - t.Run("chains with other builder methods", func(t *testing.T) { - err := Build(ErrInvalidStack). - WithCausef("stack `%s` does not exist", "demo"). - WithExplanation("The stack was not found"). - WithHint("Check your configuration"). - Err() - - assert.Error(t, err) - assert.True(t, errors.Is(err, ErrInvalidStack)) - assert.Contains(t, err.Error(), "stack `demo` does not exist") - - // Verify hint is present. - hints := errors.GetAllHints(err) - assert.Contains(t, hints, "Check your configuration") - }) + type testCase struct { + name string + build func() error + wantIs error + wantContains string + assertExtra func(t *testing.T, err error) + } + + cases := []testCase{ + { + name: "creates formatted cause error", + build: func() error { + stackName := "dev-us-east-1" + return Build(ErrInvalidStack). + WithCausef("stack `%s` does not exist", stackName). + Err() + }, + wantIs: ErrInvalidStack, + wantContains: "stack `dev-us-east-1` does not exist", + }, + { + name: "works with multiple format arguments", + build: func() error { + return Build(ErrInvalidComponent). + WithCausef("component `%s` not found in stack `%s`", "vpc", "prod"). + Err() + }, + wantIs: ErrInvalidComponent, + wantContains: "component `vpc` not found in stack `prod`", + }, + { + name: "chains with other builder methods", + build: func() error { + return Build(ErrInvalidStack). + WithCausef("stack `%s` does not exist", "demo"). + WithExplanation("The stack was not found"). + WithHint("Check your configuration"). + Err() + }, + wantIs: ErrInvalidStack, + wantContains: "stack `demo` does not exist", + assertExtra: func(t *testing.T, err error) { + hints := errors.GetAllHints(err) + assert.Contains(t, hints, "Check your configuration") + }, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + err := tc.build() + assert.Error(t, err) + if tc.wantIs != nil { + assert.True(t, errors.Is(err, tc.wantIs)) + } + if tc.wantContains != "" { + assert.Contains(t, err.Error(), tc.wantContains) + } + if tc.assertExtra != nil { + tc.assertExtra(t, err) + } + }) + } }
|
These changes were released in v1.204.1-rc.2. |
2 participants
what
why
Previously, users would see all components from all stacks in the interactive menu, including abstract base components that serve as templates. This was confusing because abstract components cannot be deployed. The fix ensures only deployable, valid components appear in interactive prompts and shell completion, improving user experience and preventing errors.
references
Fixes component selection menu to correctly filter out non-deployable components.
Summary by CodeRabbit
New Features
Improvements
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.