fix: resolve --all flag prompting for component (issue #1945)#1976
Merged
fix: resolve --all flag prompting for component (issue #1945)#1976
Conversation
Fixed a critical bug where terraform commands with --all, --affected, --query, or --components flags would incorrectly prompt for component selection, then error when a component was selected. Root cause was applyOptionsToInfo being called after resolveAndPromptForArgs, preventing the prompting logic from detecting multi-component flags. - Reorder flag application to occur before interactive prompting - Add regression tests to prevent similar bugs in future - Improve UX: show success messages when no components affected/matched - Change internal logging from INFO to DEBUG for dry-run mode - Add user-facing dry-run progress with ui.Successf (e.g., "✓ Would plan `vpc` in `plat-ue2-dev` (dry run)") - Markdown styling with backticks highlights component and stack names in cyan Fixes #1945 Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Dependency Review✅ No vulnerabilities or license issues found.Scanned FilesNone |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1976 +/- ##
==========================================
+ Coverage 75.22% 75.24% +0.01%
==========================================
Files 785 785
Lines 72455 72460 +5
==========================================
+ Hits 54503 54521 +18
+ Misses 14461 14452 -9
+ Partials 3491 3487 -4
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Contributor
📝 WalkthroughWalkthroughApply CLI multi-component flags to the info struct before interactive prompting; skip component prompts when bulk flags are present; change component processing to return (bool, error); add dry-run short-circuit and user-facing "no matches/affected" success messages; expand tests and test-cases for multi-component flags, dry-run, and no-match behavior. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as User/CLI
participant Info as Info Struct
participant Prompt as Prompt Logic
participant Exec as Component Processor
participant UI as UI
CLI->>Info: apply CLI options (--all/--affected/--query/--components)
Info->>Prompt: hasMultiComponentFlags()?
alt multi-component flags present
Note over Prompt: Skip interactive component prompt
Prompt->>Exec: Determine target components (all/affected/query/list)
else no multi-component flags
Prompt->>CLI: prompt for component selection
CLI->>Info: apply user selection
Info->>Exec: use selected component
end
Exec->>Exec: For each candidate: evaluate query, check metadata, check dry-run
alt query not satisfied or metadata/disabled/abstract
Exec->>UI: log skip (debug)
Exec-->>Exec: return processed=false
else dry-run
Exec->>UI: ui.Successf("Would ... (dry run)")
Exec-->>Exec: return processed=true
else execute terraform
Exec->>Exec: ExecuteTerraform(...)
Exec-->>Exec: return processed=true
end
Exec->>UI: if no components processed -> ui.Success("No components affected/matched")
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
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.go (1)
194-223: Dry-run currently skips dependents in affected flow.
The earlyreturnprevents the dependency traversal, so--affected --include-dependents --dry-runonly reports the root component. Keep the dry-run message, but continue recursion.🔧 Suggested fix (keep recursion in dry-run)
// Show user-facing progress for dry-run mode. if info.DryRun { msg := fmt.Sprintf("Would %s `%s` in `%s` (dry run)", info.SubCommand, affectedComponent, affectedStack) if args.IncludeDependents && parentComponent != "" && parentStack != "" { msg = fmt.Sprintf("Would %s `%s` in `%s` (dependency of `%s` in `%s`) (dry run)", info.SubCommand, affectedComponent, affectedStack, parentComponent, parentStack) } _ = ui.Successf("%s", msg) - return nil - } - - // Execute the terraform command for the affected component. - err := ExecuteTerraform(*info) - if err != nil { - return err + } else { + // Execute the terraform command for the affected component. + if err := ExecuteTerraform(*info); err != nil { + return err + } }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
cmd/terraform/utils.gocmd/terraform/utils_test.gointernal/exec/terraform_affected.gointernal/exec/terraform_query.gointernal/exec/terraform_utils.gointernal/exec/terraform_utils_test.go
🧰 Additional context used
📓 Path-based instructions (5)
**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*.go: Use Viper for managing configuration, environment variables, and flags in CLI commands
Use interfaces for external dependencies to facilitate mocking and consider using testify/mock for creating mock implementations
All code must pass golangci-lint checks
Follow Go's error handling idioms: use meaningful error messages, wrap errors with context usingfmt.Errorf("context: %w", err), and consider using custom error types for domain-specific errors
Follow standard Go coding style: usegofmtandgoimportsto format code, prefer short descriptive variable names, use kebab-case for command-line flags, and snake_case for environment variables
Document all exported functions, types, and methods following Go's documentation conventions
Document complex logic with inline comments in Go code
Support configuration via files, environment variables, and flags following the precedence order: flags > environment variables > config file > defaults
Provide clear error messages to users, include troubleshooting hints when appropriate, and log detailed errors for debugging
**/*.go: All comments must end with periods (enforced bygodotlinter) in Go code
Organize imports into three groups separated by blank lines, sorted alphabetically: Go stdlib, 3rd-party (NOT cloudposse/atmos), then Atmos packages with maintained aliases (cfg,log,u,errUtils)
All errors MUST be wrapped using static errors defined inerrors/errors.go- useerrors.Joinfor combining errors,fmt.Errorfwith%wfor context, anderrors.Is()for error checking
Never manually create mocks - usego.uber.org/mock/mockgenwith//go:generatedirectives in Go code
Keep files small and focused - under 600 lines with one cmd/impl per file, co-locate tests, never use//revive:disable:file-length-limit
Use colors frompkg/ui/theme/colors.gofor all UI theming in Go code
Code must be compatible with Linux, macOS, and Windows - use SDKs over binaries, usefilepath.Join()instead of h...
Files:
internal/exec/terraform_query.gocmd/terraform/utils.gointernal/exec/terraform_affected.gointernal/exec/terraform_utils_test.gointernal/exec/terraform_utils.gocmd/terraform/utils_test.go
**/{pkg,internal,cmd}/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
Add
defer perf.Track(atmosConfig, "pkg.FuncName")()plus blank line to all public functions, usingnilif no atmosConfig param - exceptions: trivial getters/setters, command constructors, simple factories, functions delegating to tracked functions
Files:
internal/exec/terraform_query.gocmd/terraform/utils.gointernal/exec/terraform_affected.gointernal/exec/terraform_utils_test.gointernal/exec/terraform_utils.gocmd/terraform/utils_test.go
cmd/**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
cmd/**/*.go: Use Cobra's recommended command structure with a root command and subcommands, implementing each command in a separate file undercmd/directory
Provide comprehensive help text for all commands and flags, include examples in command help, and follow Go's documentation conventions in Cobra command definitions
Provide meaningful feedback to users and include progress indicators for long-running operations in CLI commands
Files:
cmd/terraform/utils.gocmd/terraform/utils_test.go
**/*_test.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*_test.go: Every new feature must include comprehensive unit tests targeting >80% code coverage for all packages
Use table-driven tests for testing multiple scenarios in Go
Include integration tests for command flows and test CLI end-to-end when possible with test fixturesPrefer unit tests with mocks over integration tests - use interfaces and dependency injection for testability, generate mocks with
go.uber.org/mock/mockgen, use table-driven tests, target >80% coverage
Files:
internal/exec/terraform_utils_test.gocmd/terraform/utils_test.go
cmd/**/*_test.go
📄 CodeRabbit inference engine (CLAUDE.md)
Always use
cmd.NewTestKit(t)for cmd tests to auto-clean RootCmd state
Files:
cmd/terraform/utils_test.go
🧠 Learnings (46)
📓 Common learnings
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: docs/prd/tool-dependencies-integration.md:58-64
Timestamp: 2025-12-13T06:07:37.766Z
Learning: cloudposse/atmos: For PRD docs (docs/prd/*.md), markdownlint issues like MD040/MD010/MD034 can be handled in a separate documentation cleanup commit and should not block the current PR.
Learnt from: osterman
Repo: cloudposse/atmos PR: 727
File: internal/exec/terraform.go:114-118
Timestamp: 2024-10-21T17:51:53.976Z
Learning: When `atmos terraform clean --everything` is used without specifying a component and without the `--force` flag, prompt the user for confirmation before deleting all components. Use the `--force` flag to skip the confirmation prompt.
Learnt from: aknysh
Repo: cloudposse/atmos PR: 944
File: go.mod:206-206
Timestamp: 2025-01-17T00:18:57.769Z
Learning: For indirect dependencies with license compliance issues in the cloudposse/atmos repository, the team prefers to handle them in follow-up PRs rather than blocking the current changes, as these issues often require deeper investigation of the dependency tree.
Learnt from: Listener430
Repo: cloudposse/atmos PR: 934
File: tests/fixtures/scenarios/docs-generate/README.md.gotmpl:99-118
Timestamp: 2025-01-25T03:51:57.689Z
Learning: For the cloudposse/atmos repository, changes to template contents should be handled in dedicated PRs and are typically considered out of scope for PRs focused on other objectives.
Learnt from: osterman
Repo: cloudposse/atmos PR: 1498
File: website/src/components/Screengrabs/atmos-terraform-metadata--help.html:25-55
Timestamp: 2025-10-07T00:25:16.333Z
Learning: In Atmos CLI, subcommands inherit flags from their parent commands via Cobra's command inheritance. For example, `atmos terraform metadata --help` shows `--affected` and related flags inherited from the parent `terraform` command (defined in cmd/terraform.go), even though the metadata subcommand doesn't explicitly define these flags. This is expected Cobra behavior and auto-generated help screengrabs accurately reflect this inheritance.
Learnt from: osterman
Repo: cloudposse/atmos PR: 1599
File: internal/exec/terraform.go:394-402
Timestamp: 2025-10-10T23:51:36.597Z
Learning: In Atmos (internal/exec/terraform.go), when adding OpenTofu-specific flags like `--var-file` for `init`, do not gate them based on command name (e.g., checking if `info.Command == "tofu"` or `info.Command == "opentofu"`) because command names don't reliably indicate the actual binary being executed (symlinks, aliases). Instead, document the OpenTofu requirement in code comments and documentation, trusting users who enable the feature (e.g., `PassVars`) to ensure their terraform command points to an OpenTofu binary.
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 955
File: tests/snapshots/TestCLICommands_atmos_validate_editorconfig_--help.stdout.golden:0-0
Timestamp: 2025-01-19T15:49:15.593Z
Learning: In future commits, the help text for Atmos CLI commands should be limited to only show component and stack parameters for commands that actually use them. This applies to the example usage section in command help text.
Learnt from: osterman
Repo: cloudposse/atmos PR: 768
File: website/docs/cheatsheets/vendoring.mdx:70-70
Timestamp: 2024-11-12T13:06:56.194Z
Learning: In `atmos vendor pull --everything`, the `--everything` flag uses the TTY for TUI but is not interactive.
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1068
File: tests/snapshots/TestCLICommands_atmos_terraform_help.stdout.golden:59-64
Timestamp: 2025-02-18T13:13:11.497Z
Learning: For Atmos CLI help text, angle brackets in command examples and flag descriptions should be escaped using HTML entities (e.g., `<component>`) rather than converted to backticks or other markdown formatting.
Learnt from: osterman
Repo: cloudposse/atmos PR: 1813
File: cmd/terraform/shell.go:28-73
Timestamp: 2025-12-13T03:21:35.786Z
Learning: In Atmos, when calling cfg.InitCliConfig, you must first populate the schema.ConfigAndStacksInfo struct with global flag values using flags.ParseGlobalFlags(cmd, v) rather than passing an empty struct. The LoadConfig function (pkg/config/load.go) reads config selection fields (AtmosConfigFilesFromArg, AtmosConfigDirsFromArg, BasePath, ProfilesFromArg) directly from the ConfigAndStacksInfo struct, NOT from Viper. Passing an empty struct causes config selection flags (--base-path, --config, --config-path, --profile) to be silently ignored. Correct pattern: parse flags → populate struct → call InitCliConfig. See cmd/terraform/plan_diff.go for reference implementation.
Learnt from: osterman
Repo: cloudposse/atmos PR: 1533
File: pkg/config/load.go:585-637
Timestamp: 2025-09-27T20:50:20.564Z
Learning: In the cloudposse/atmos repository, command merging prioritizes precedence over display ordering. Help commands are displayed lexicographically regardless of internal array order, so the mergeCommandArrays function focuses on ensuring the correct precedence chain (top-level file wins) rather than maintaining specific display order.
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1466
File: cmd/markdown/atmos_toolchain_aliases.md:2-4
Timestamp: 2025-09-13T16:39:20.007Z
Learning: In the cloudposse/atmos repository, CLI documentation files in cmd/markdown/ follow a specific format that uses " $ atmos command" (with leading space and dollar sign prompt) in code blocks. This is the established project convention and should not be changed to comply with standard markdownlint rules MD040 and MD014.
📚 Learning: 2024-10-31T19:25:41.298Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 727
File: internal/exec/terraform_clean.go:233-235
Timestamp: 2024-10-31T19:25:41.298Z
Learning: When specifying color values in functions like `confirmDeleteTerraformLocal` in `internal/exec/terraform_clean.go`, avoid hardcoding color values. Instead, use predefined color constants or allow customization through configuration settings to improve accessibility and user experience across different terminals and themes.
Applied to files:
internal/exec/terraform_query.gocmd/terraform/utils.gointernal/exec/terraform_affected.gointernal/exec/terraform_utils_test.gointernal/exec/terraform_utils.gocmd/terraform/utils_test.go
📚 Learning: 2024-11-24T19:13:10.287Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 727
File: internal/exec/terraform_clean.go:407-416
Timestamp: 2024-11-24T19:13:10.287Z
Learning: In `internal/exec/terraform_clean.go`, when `getStackTerraformStateFolder` returns an error in the `handleCleanSubCommand` function, the error is logged, and the process continues without returning the error.
Applied to files:
internal/exec/terraform_query.gointernal/exec/terraform_utils_test.gointernal/exec/terraform_utils.go
📚 Learning: 2024-11-12T03:16:02.910Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 775
File: internal/exec/template_funcs_component.go:157-159
Timestamp: 2024-11-12T03:16:02.910Z
Learning: In the Go code for `componentFunc` in `internal/exec/template_funcs_component.go`, the function `cleanTerraformWorkspace` does not return errors, and it's acceptable if the file does not exist. Therefore, error handling for `cleanTerraformWorkspace` is not needed.
Applied to files:
internal/exec/terraform_query.gointernal/exec/terraform_affected.gointernal/exec/terraform_utils_test.gointernal/exec/terraform_utils.go
📚 Learning: 2024-12-07T16:19:01.683Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 825
File: internal/exec/terraform.go:30-30
Timestamp: 2024-12-07T16:19:01.683Z
Learning: In `internal/exec/terraform.go`, skipping stack validation when help flags are present is not necessary.
Applied to files:
internal/exec/terraform_query.gocmd/terraform/utils.gointernal/exec/terraform_utils_test.gointernal/exec/terraform_utils.gocmd/terraform/utils_test.go
📚 Learning: 2025-09-29T02:20:11.636Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1540
File: internal/exec/validate_component.go:117-118
Timestamp: 2025-09-29T02:20:11.636Z
Learning: The ValidateComponent function in internal/exec/validate_component.go had its componentSection parameter type refined from `any` to `map[string]any` without adding new parameters. This is a type safety improvement, not a signature change requiring call site updates.
Applied to files:
internal/exec/terraform_query.gointernal/exec/terraform_utils_test.gointernal/exec/terraform_utils.gocmd/terraform/utils_test.go
📚 Learning: 2025-06-23T02:14:30.937Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1327
File: cmd/terraform.go:111-117
Timestamp: 2025-06-23T02:14:30.937Z
Learning: In cmd/terraform.go, flags for the DescribeAffected function are added dynamically at runtime when info.Affected is true. This is intentional to avoid exposing internal flags like "file", "format", "verbose", "include-spacelift-admin-stacks", "include-settings", and "upload" in the terraform command interface, while still providing them for the shared DescribeAffected function used by both `atmos describe affected` and `atmos terraform apply --affected`.
Applied to files:
internal/exec/terraform_query.gocmd/terraform/utils.gointernal/exec/terraform_affected.gointernal/exec/terraform_utils.gocmd/terraform/utils_test.go
📚 Learning: 2024-10-27T04:41:49.199Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 727
File: internal/exec/terraform_clean.go:215-223
Timestamp: 2024-10-27T04:41:49.199Z
Learning: In `internal/exec/terraform_clean.go`, the function `determineCleanPath` is necessary and should not be removed.
Applied to files:
internal/exec/terraform_query.gointernal/exec/terraform_utils.go
📚 Learning: 2024-11-02T15:35:09.958Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 759
File: internal/exec/terraform.go:366-368
Timestamp: 2024-11-02T15:35:09.958Z
Learning: In `internal/exec/terraform.go`, the workspace cleaning code under both the general execution path and within the `case "init":` block is intentionally duplicated because the code execution paths are different. The `.terraform/environment` file should be deleted before executing `terraform init` in both scenarios to ensure a clean state.
Applied to files:
internal/exec/terraform_query.go
📚 Learning: 2024-10-27T04:28:40.966Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 727
File: internal/exec/terraform_clean.go:155-175
Timestamp: 2024-10-27T04:28:40.966Z
Learning: In the `CollectDirectoryObjects` function in `internal/exec/terraform_clean.go`, recursive search through all subdirectories is not needed.
Applied to files:
internal/exec/terraform_query.go
📚 Learning: 2024-10-28T01:51:30.811Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 727
File: internal/exec/terraform_clean.go:329-332
Timestamp: 2024-10-28T01:51:30.811Z
Learning: In the Atmos Go code, when deleting directories or handling file paths (e.g., in `terraform_clean.go`), always resolve the absolute path using `filepath.Abs` and use the logger `u.LogWarning` for logging messages instead of using `fmt.Printf`.
Applied to files:
internal/exec/terraform_query.gointernal/exec/terraform_affected.gointernal/exec/terraform_utils.go
📚 Learning: 2025-09-13T18:06:07.674Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1466
File: toolchain/list.go:39-42
Timestamp: 2025-09-13T18:06:07.674Z
Learning: In the cloudposse/atmos repository, for UI messages in the toolchain package, use utils.PrintfMessageToTUI instead of log.Error or fmt.Fprintln(os.Stderr, ...). Import pkg/utils with alias "u" to follow the established pattern.
Applied to files:
internal/exec/terraform_query.gointernal/exec/terraform_affected.gointernal/exec/terraform_utils.go
📚 Learning: 2024-11-13T21:37:07.852Z
Learnt from: Cerebrovinny
Repo: cloudposse/atmos PR: 764
File: internal/exec/describe_stacks.go:289-295
Timestamp: 2024-11-13T21:37:07.852Z
Learning: In the `internal/exec/describe_stacks.go` file of the `atmos` project written in Go, avoid extracting the stack name handling logic into a helper function within the `ExecuteDescribeStacks` method, even if the logic appears duplicated.
Applied to files:
internal/exec/terraform_query.go
📚 Learning: 2024-12-17T07:08:41.288Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 863
File: internal/exec/yaml_func_terraform_output.go:34-38
Timestamp: 2024-12-17T07:08:41.288Z
Learning: In the `processTagTerraformOutput` function within `internal/exec/yaml_func_terraform_output.go`, parameters are separated by spaces and do not contain spaces. Therefore, using `strings.Fields()` for parsing is acceptable, and there's no need to handle parameters with spaces.
Applied to files:
internal/exec/terraform_query.gointernal/exec/terraform_utils_test.go
📚 Learning: 2025-12-21T04:10:29.030Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1891
File: internal/exec/describe_affected.go:468-468
Timestamp: 2025-12-21T04:10:29.030Z
Learning: In Go, package-level declarations (constants, variables, types, and functions) are visible to all files in the same package without imports. During reviews in cloudposse/atmos (and similar Go codebases), before suggesting to declare a new identifier, first check if it already exists in another file of the same package. If it exists, you can avoid adding a new declaration; if not, proceed with a proper package-level declaration.
Applied to files:
internal/exec/terraform_query.gocmd/terraform/utils.gointernal/exec/terraform_affected.gointernal/exec/terraform_utils_test.gointernal/exec/terraform_utils.gocmd/terraform/utils_test.go
📚 Learning: 2025-10-10T23:51:36.597Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1599
File: internal/exec/terraform.go:394-402
Timestamp: 2025-10-10T23:51:36.597Z
Learning: In Atmos (internal/exec/terraform.go), when adding OpenTofu-specific flags like `--var-file` for `init`, do not gate them based on command name (e.g., checking if `info.Command == "tofu"` or `info.Command == "opentofu"`) because command names don't reliably indicate the actual binary being executed (symlinks, aliases). Instead, document the OpenTofu requirement in code comments and documentation, trusting users who enable the feature (e.g., `PassVars`) to ensure their terraform command points to an OpenTofu binary.
Applied to files:
cmd/terraform/utils.gointernal/exec/terraform_utils.gocmd/terraform/utils_test.go
📚 Learning: 2025-01-09T22:37:01.004Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 914
File: cmd/terraform_commands.go:260-265
Timestamp: 2025-01-09T22:37:01.004Z
Learning: In the terraform commands implementation (cmd/terraform_commands.go), the direct use of `os.Args[2:]` for argument handling is intentionally preserved to avoid extensive refactoring. While it could be improved to use cobra's argument parsing, such changes should be handled in a dedicated PR to maintain focus and minimize risk.
Applied to files:
cmd/terraform/utils.gocmd/terraform/utils_test.go
📚 Learning: 2025-12-13T03:21:27.620Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1813
File: cmd/terraform/shell.go:28-73
Timestamp: 2025-12-13T03:21:27.620Z
Learning: In Atmos, when initializing CLI config via cfg.InitCliConfig, always first populate the ConfigAndStacksInfo struct with global flag values by calling flags.ParseGlobalFlags(cmd, v) before LoadConfig. LoadConfig (pkg/config/load.go) reads config selection fields (AtmosConfigFilesFromArg, AtmosConfigDirsFromArg, BasePath, ProfilesFromArg) from the ConfigAndStacksInfo struct, not from Viper. Passing an empty struct causes the --base-path, --config, --config-path, and --profile flags to be ignored. Recommended pattern: parse flags → populate struct → call InitCliConfig. See cmd/terraform/plan_diff.go for reference implementation.
Applied to files:
cmd/terraform/utils.gocmd/terraform/utils_test.go
📚 Learning: 2025-10-07T00:25:16.333Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1498
File: website/src/components/Screengrabs/atmos-terraform-metadata--help.html:25-55
Timestamp: 2025-10-07T00:25:16.333Z
Learning: In Atmos CLI, subcommands inherit flags from their parent commands via Cobra's command inheritance. For example, `atmos terraform metadata --help` shows `--affected` and related flags inherited from the parent `terraform` command (defined in cmd/terraform.go), even though the metadata subcommand doesn't explicitly define these flags. This is expected Cobra behavior and auto-generated help screengrabs accurately reflect this inheritance.
Applied to files:
cmd/terraform/utils.go
📚 Learning: 2025-10-11T19:11:58.965Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1599
File: internal/exec/terraform.go:0-0
Timestamp: 2025-10-11T19:11:58.965Z
Learning: For terraform apply interactivity checks in Atmos (internal/exec/terraform.go), use stdin TTY detection (e.g., `IsTTYSupportForStdin()` or checking `os.Stdin`) to determine if user prompts are possible. This is distinct from stdout/stderr TTY checks used for output display (like TUI rendering). User input requires stdin to be a TTY; output display requires stdout/stderr to be a TTY.
Applied to files:
cmd/terraform/utils.go
📚 Learning: 2025-09-05T14:57:37.360Z
Learnt from: RoseSecurity
Repo: cloudposse/atmos PR: 1448
File: cmd/ansible.go:26-28
Timestamp: 2025-09-05T14:57:37.360Z
Learning: The Atmos codebase uses a consistent pattern for commands that delegate to external tools: `PersistentFlags().Bool("", false, doubleDashHint)` where doubleDashHint provides help text about using double dashes to separate Atmos options from native command arguments. This pattern is used across terraform, packer, helmfile, atlantis, aws, and ansible commands.
Applied to files:
cmd/terraform/utils.go
📚 Learning: 2024-12-07T16:16:13.038Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 825
File: internal/exec/helmfile_generate_varfile.go:28-31
Timestamp: 2024-12-07T16:16:13.038Z
Learning: In `internal/exec/helmfile_generate_varfile.go`, the `--help` command (`./atmos helmfile generate varfile --help`) works correctly without requiring stack configurations, and the only change needed was to make `ProcessCommandLineArgs` exportable by capitalizing its name.
Applied to files:
cmd/terraform/utils.gointernal/exec/terraform_utils_test.gointernal/exec/terraform_utils.gocmd/terraform/utils_test.go
📚 Learning: 2025-12-13T04:37:40.435Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1686
File: cmd/toolchain/get.go:23-40
Timestamp: 2025-12-13T04:37:40.435Z
Learning: In Go CLI command files using Cobra, constrain the subcommand to accept at most one positional argument (MaximumNArgs(1)) so it supports both listing all items (zero args) and fetching a specific item (one arg). Define and parse flags with a standard parser (e.g., flags.NewStandardParser()) and avoid binding flags to Viper (no viper.BindEnv/BindPFlag). This promotes explicit argument handling and predictable flag behavior across command files.
Applied to files:
cmd/terraform/utils.gocmd/terraform/utils_test.go
📚 Learning: 2025-07-05T20:59:02.914Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1363
File: internal/exec/template_utils.go:18-18
Timestamp: 2025-07-05T20:59:02.914Z
Learning: In the Atmos project, gomplate v4 is imported with a blank import (`_ "github.com/hairyhenderson/gomplate/v4"`) alongside v3 imports to resolve AWS SDK version conflicts. V3 uses older AWS SDK versions that conflict with newer AWS modules used by Atmos. A full migration to v4 requires extensive refactoring due to API changes and should be handled in a separate PR.
Applied to files:
internal/exec/terraform_affected.gointernal/exec/terraform_utils.go
📚 Learning: 2026-01-04T00:55:21.720Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-04T00:55:21.720Z
Learning: Applies to **/{pkg,internal,cmd}/**/*.go : Add `defer perf.Track(atmosConfig, "pkg.FuncName")()` plus blank line to all public functions, using `nil` if no atmosConfig param - exceptions: trivial getters/setters, command constructors, simple factories, functions delegating to tracked functions
Applied to files:
internal/exec/terraform_affected.gointernal/exec/terraform_utils.go
📚 Learning: 2026-01-04T00:55:21.720Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-04T00:55:21.720Z
Learning: Applies to **/*.go : Organize imports into three groups separated by blank lines, sorted alphabetically: Go stdlib, 3rd-party (NOT cloudposse/atmos), then Atmos packages with maintained aliases (`cfg`, `log`, `u`, `errUtils`)
Applied to files:
internal/exec/terraform_affected.gointernal/exec/terraform_utils.go
📚 Learning: 2025-11-09T19:06:58.470Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1752
File: pkg/profile/list/formatter_table.go:27-29
Timestamp: 2025-11-09T19:06:58.470Z
Learning: In the cloudposse/atmos repository, performance tracking with `defer perf.Track()` is enforced on all functions via linting, including high-frequency utility functions, formatters, and renderers. This is a repository-wide policy to maintain consistency and avoid making case-by-case judgment calls about which functions should have profiling.
Applied to files:
internal/exec/terraform_affected.gointernal/exec/terraform_utils.go
📚 Learning: 2025-02-06T13:38:07.216Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 984
File: internal/exec/copy_glob.go:0-0
Timestamp: 2025-02-06T13:38:07.216Z
Learning: The `u.LogTrace` function in the `cloudposse/atmos` repository accepts `atmosConfig` as its first parameter, followed by the message string.
Applied to files:
internal/exec/terraform_affected.gointernal/exec/terraform_utils.go
📚 Learning: 2024-12-11T18:40:12.808Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 844
File: cmd/helmfile.go:37-37
Timestamp: 2024-12-11T18:40:12.808Z
Learning: In the atmos project, `cliConfig` is initialized within the `cmd` package in `root.go` and can be used in other command files.
Applied to files:
internal/exec/terraform_affected.gointernal/exec/terraform_utils.go
📚 Learning: 2024-10-23T21:36:40.262Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 740
File: cmd/cmd_utils.go:340-359
Timestamp: 2024-10-23T21:36:40.262Z
Learning: In the Go codebase for Atmos, when reviewing functions like `checkAtmosConfig` in `cmd/cmd_utils.go`, avoid suggesting refactoring to return errors instead of calling `os.Exit` if such changes would significantly increase the scope due to the need to update multiple call sites.
Applied to files:
internal/exec/terraform_affected.gointernal/exec/terraform_utils.go
📚 Learning: 2024-12-08T14:26:16.972Z
Learnt from: pkbhowmick
Repo: cloudposse/atmos PR: 828
File: pkg/schema/schema.go:98-100
Timestamp: 2024-12-08T14:26:16.972Z
Learning: The `ListConfig` columns array in the `Components` struct can be empty.
Applied to files:
internal/exec/terraform_affected.go
📚 Learning: 2025-12-10T18:32:51.237Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1808
File: cmd/terraform/backend/backend_delete_test.go:9-23
Timestamp: 2025-12-10T18:32:51.237Z
Learning: In cmd subpackages (e.g., cmd/terraform/backend/), tests cannot use cmd.NewTestKit(t) due to Go's test visibility rules (NewTestKit is in a parent package test file). These tests only need TestKit if they execute commands through RootCmd or modify RootCmd state. Structural tests that only verify command structure/flags without touching RootCmd don't require TestKit cleanup.
Applied to files:
internal/exec/terraform_utils_test.go
📚 Learning: 2024-11-30T22:07:08.610Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 810
File: internal/exec/yaml_func_terraform_output.go:35-40
Timestamp: 2024-11-30T22:07:08.610Z
Learning: In the Go function `processTagTerraformOutput` in `internal/exec/yaml_func_terraform_output.go`, parameters cannot contain spaces. The code splits the input by spaces, and if the parameters contain spaces, `len(parts) != 3` will fail and show an error to the user.
Applied to files:
internal/exec/terraform_utils_test.go
📚 Learning: 2025-10-03T18:02:08.535Z
Learnt from: Benbentwo
Repo: cloudposse/atmos PR: 1475
File: internal/exec/terraform.go:269-272
Timestamp: 2025-10-03T18:02:08.535Z
Learning: In internal/exec/terraform.go, when auth.TerraformPreHook fails, the error is logged but execution continues. This is a deliberate design choice to allow Terraform commands to proceed even if authentication setup fails, rather than failing fast.
Applied to files:
internal/exec/terraform_utils_test.go
📚 Learning: 2025-12-13T03:21:35.786Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1813
File: cmd/terraform/shell.go:28-73
Timestamp: 2025-12-13T03:21:35.786Z
Learning: In Atmos, when calling cfg.InitCliConfig, you must first populate the schema.ConfigAndStacksInfo struct with global flag values using flags.ParseGlobalFlags(cmd, v) rather than passing an empty struct. The LoadConfig function (pkg/config/load.go) reads config selection fields (AtmosConfigFilesFromArg, AtmosConfigDirsFromArg, BasePath, ProfilesFromArg) directly from the ConfigAndStacksInfo struct, NOT from Viper. Passing an empty struct causes config selection flags (--base-path, --config, --config-path, --profile) to be silently ignored. Correct pattern: parse flags → populate struct → call InitCliConfig. See cmd/terraform/plan_diff.go for reference implementation.
Applied to files:
internal/exec/terraform_utils_test.go
📚 Learning: 2024-10-31T19:23:45.538Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 727
File: internal/exec/terraform.go:65-66
Timestamp: 2024-10-31T19:23:45.538Z
Learning: The variable `shouldCheckStack` in `ExecuteTerraform` controls whether validation is performed.
Applied to files:
internal/exec/terraform_utils_test.go
📚 Learning: 2025-04-11T22:06:46.999Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1147
File: internal/exec/validate_schema.go:42-57
Timestamp: 2025-04-11T22:06:46.999Z
Learning: The "ExecuteAtmosValidateSchemaCmd" function in internal/exec/validate_schema.go has been reviewed and confirmed to have acceptable cognitive complexity despite static analysis warnings. The function uses a clean structure with only three if statements for error handling and delegates complex operations to helper methods.
Applied to files:
internal/exec/terraform_utils_test.go
📚 Learning: 2024-12-03T05:18:49.169Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 810
File: internal/exec/terraform_utils.go:40-213
Timestamp: 2024-12-03T05:18:49.169Z
Learning: In the context of the Atmos project, it's acceptable for functions like `execTerraformOutput` to remain as single functions if they perform a single purpose, such as retrieving Terraform outputs for a component in a stack, even if the function is lengthy.
Applied to files:
internal/exec/terraform_utils_test.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*_test.go : Include integration tests for command flows and test CLI end-to-end when possible with test fixtures
Applied to files:
cmd/terraform/utils_test.go
📚 Learning: 2025-12-10T18:32:43.260Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1808
File: cmd/terraform/backend/backend_delete_test.go:9-23
Timestamp: 2025-12-10T18:32:43.260Z
Learning: In cmd subpackages, tests should avoid using NewTestKit(t) unless tests actually interact with RootCmd (e.g., execute commands via RootCmd or modify RootCmd state). For structural tests that only verify command structure/flags without touching RootCmd, TestKit cleanup is unnecessary.
Applied to files:
cmd/terraform/utils_test.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*_test.go : Every new feature must include comprehensive unit tests targeting >80% code coverage for all packages
Applied to files:
cmd/terraform/utils_test.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*_test.go : Use table-driven tests for testing multiple scenarios in Go
Applied to files:
cmd/terraform/utils_test.go
📚 Learning: 2025-02-09T14:38:53.443Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 992
File: cmd/cmd_utils.go:0-0
Timestamp: 2025-02-09T14:38:53.443Z
Learning: Error handling for RegisterFlagCompletionFunc in AddStackCompletion is not required as the errors are non-critical for tab completion functionality.
Applied to files:
cmd/terraform/utils_test.go
📚 Learning: 2025-02-18T13:18:53.146Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1068
File: cmd/vendor_pull.go:31-31
Timestamp: 2025-02-18T13:18:53.146Z
Learning: Error checking is not required for cobra.Command.RegisterFlagCompletionFunc calls as these are static configurations done at init time.
Applied to files:
cmd/terraform/utils_test.go
📚 Learning: 2025-01-09T22:27:25.538Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 914
File: cmd/validate_stacks.go:20-23
Timestamp: 2025-01-09T22:27:25.538Z
Learning: The validate commands in Atmos can have different help handling implementations. Specifically, validate_component.go and validate_stacks.go are designed to handle help requests differently, with validate_stacks.go including positional argument checks while validate_component.go does not.
Applied to files:
cmd/terraform/utils_test.go
📚 Learning: 2025-02-07T19:21:38.028Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 992
File: cmd/vendor_pull.go:31-31
Timestamp: 2025-02-07T19:21:38.028Z
Learning: The cobra.Command.RegisterFlagCompletionFunc method (as of cobra v1.8.1) never returns an error. It only initializes an internal map and stores the completion function, always returning nil. Error handling for this method call is unnecessary.
Applied to files:
cmd/terraform/utils_test.go
🧬 Code graph analysis (3)
internal/exec/terraform_query.go (2)
pkg/logger/log.go (1)
Debug(24-26)pkg/ui/formatter.go (1)
Success(204-211)
internal/exec/terraform_affected.go (1)
pkg/ui/formatter.go (1)
Success(204-211)
cmd/terraform/utils_test.go (1)
pkg/schema/schema.go (1)
ConfigAndStacksInfo(732-829)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Acceptance Tests (windows)
- GitHub Check: Acceptance Tests (linux)
- GitHub Check: Summary
🔇 Additional comments (16)
internal/exec/terraform_affected.go (2)
3-9: UI import is appropriate here.
Nice setup to support the new success messaging.
106-109: Clear UX for empty affected set.
This early success message keeps the flow tidy.cmd/terraform/utils.go (1)
227-230: Good fix: apply options before prompting.
This ordering correctly blocks prompts when multi-component flags are set.internal/exec/terraform_query.go (3)
20-22: Consistent debug-level logging.
Keeps internal verbosity aligned across modes.
40-48: Processed-count tracking is solid.
Good use of the new processed flag to drive reporting.
54-57: Nice empty-result success message.
Improves clarity when the query matches nothing.cmd/terraform/utils_test.go (1)
319-455: Solid regression coverage for multi-component prompts.
These cases directly protect against issue#1945.internal/exec/terraform_utils.go (2)
10-15: UI import fits the new dry-run messaging.
All good here.
286-342: Processed-flag flow looks good.
Clear filtering and dry-run messaging with the new return value.internal/exec/terraform_utils_test.go (7)
231-296: LGTM!The test cases properly adapt to the new
(bool, error)return signature ofprocessTerraformComponent. Each scenario correctly validates:
processed=falsewhen filtering out abstract, disabled, or query-unsatisfied componentsprocessed=truewhen actually executing- The
calledflag confirms whetherExecuteTerraformwas invoked
1147-1160: Good defensive handling for gomonkey platform limitations.The skip conditions gracefully handle cases where gomonkey's function patching fails due to compiler optimizations or platform differences. This prevents false test failures in CI.
392-398: Pattern is correct for unsetting env vars.Since
t.Setenvdoesn't support truly unsetting environment variables (only setting them to empty), the manualos.Unsetenvwitht.Cleanuprestoration is the right approach here.
1309-1314: Verify this case-sensitivity is intentional.The test documents that
--upload-status=FALSEreturnstruebecause only lowercase"false"is recognized. This could be confusing for users. Consider whether case-insensitive comparison would be more user-friendly in the actual implementation.
1355-1476: LGTM!Solid test coverage for
TF_CLI_ARGSparsing including the JSON variable edge case. Thefloat64comment on line 1400 helps future readers understand Go's JSON unmarshaling behavior.
1189-1222: Good use of dry-run mode in benchmark.Using
DryRun: trueprevents actual terraform execution while still measuring the function's overhead. The gomonkey setup adds some noise but is necessary for the test to function.
93-151: LGTM!Integration test with proper guards: skipping in short mode, requiring git remote and terraform binary. The
t.Skipfon error is pragmatic - prevents flaky CI failures from environment issues while still running when conditions are met.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@cmd/terraform/utils_test.go`:
- Around line 319-382: The test function TestHasMultiComponentFlags duplicates
an existing test with the same name and causes a redeclaration build error;
rename this test function (e.g., to TestHasMultiComponentFlags_Table or
TestHasMultiComponentFlags_New) or merge its table cases into the existing
TestHasMultiComponentFlags, and ensure any t.Run subtests remain unchanged;
locate the function by the symbol TestHasMultiComponentFlags and the helper
under test hasMultiComponentFlags to update the declaration consistently across
the file.
Add tests to increase patch coverage for terraform_utils.go and terraform_query.go: - TestProcessTerraformComponent/dry_run: covers dry-run branch with ui.Successf - TestExecuteTerraformAffectedComponentInDepOrder/dry_run_with_parent_component_info: covers parent component info in dry-run message - TestExecuteTerraformQueryNoMatches: covers "No components matched" success path Also fix issues from merge with main: - Remove _ = from ui.Success/ui.Successf calls (functions no longer return values) - Remove duplicate TestHasMultiComponentFlags function in cmd/terraform/utils_test.go Coverage improvements: - ExecuteTerraformQuery: 75% → 84.2% - executeTerraformAffectedComponentInDepOrder: 96% - processTerraformComponent: 88.5% Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
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)
466-483: Guard against gomonkey patch failures in the execute subtest.If patching fails, the real
ExecuteTerraformmay run and make this test flaky.Proposed diff
info := schema.ConfigAndStacksInfo{SubCommand: "plan"} processed, err := processTerraformComponent(&atmosConfig, &info, stack, component, section, logFunc) + if !called { + t.Skip("gomonkey function mocking failed (likely due to compiler optimizations or platform issues)") + } assert.NoError(t, err) assert.True(t, processed) assert.True(t, called) })
🧹 Nitpick comments (1)
internal/exec/terraform_utils_test.go (1)
259-276: PreferErrorIsfor wrapped errors.Equality is brittle if the production path wraps errors;
ErrorIskeeps the test resilient.Proposed diff
- assert.Error(t, err) - assert.Equal(t, expectedErr, err) + assert.ErrorIs(t, err, expectedErr)
aknysh
approved these changes
Jan 21, 2026
|
These changes were released in v1.204.1-rc.4. |
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
--all,--affected,--query, or--componentsflags would incorrectly prompt for component selection, then errorapplyOptionsToInfobeing called afterresolveAndPromptForArgs, preventing prompting logic from detecting multi-component flagswhy
The
--allflag must be applied to the info struct BEFORE checking whether to show interactive prompts. Otherwise, the prompting logic cannot determine that multi-component flags are set and incorrectly prompts for a component, which then conflicts with the multi-component flag validation.references
closes #1945
Summary by CodeRabbit
New Features
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.