fix: prevent duplicate file deletions in atmos terraform clean by osterman · Pull Request #1510 · cloudposse/atmos

osterman · 2025-09-24T20:03:20Z

what

Fixed duplicate file deletion attempts in atmos terraform clean command that occur when multiple stacks reference the same component
Added deduplication logic to prevent the same component path from being processed multiple times
Added unit test to verify the fix

why

Users were experiencing intermittent errors where atmos terraform clean would attempt to delete the same files twice:

✓ Deleted project-name/components/terraform/vnet-elements/.terraform/
✓ Deleted project-name/components/terraform/vnet-elements/vnet-elements-eastus-prd-vnet-elements.terraform.tfvars.json
✓ Deleted project-name/components/terraform/vnet-elements/vnet-elements-eastus-prd-vnet-elements.planfile
✓ Deleted project-name/components/terraform/vnet-elements/.terraform.lock.hcl
✓ Deleted project-name/components/terraform/vnet-elements/backend.tf.json
x Cannot delete project-name/components/terraform/vnet-elements/.terraform/: path does not exist
x Cannot delete project-name/components/terraform/vnet-elements/vnet-elements-eastus-prd-vnet-elements.terraform.tfvars.json: path does not exist
x Cannot delete project-name/components/terraform/vnet-elements/vnet-elements-eastus-prd-vnet-elements.planfile: path does not exist
x Cannot delete project-name/components/terraform/vnet-elements/.terraform.lock.hcl: path does not exist
x Cannot delete project-name/components/terraform/vnet-elements/backend.tf.json: path does not exist

This happened because when multiple stacks (e.g., dev, staging, prod) reference the same component (e.g., vnet-elements), the getAllStacksComponentsPaths function would collect duplicate component paths. The clean command would then try to delete the same files multiple times - succeeding on the first attempt and failing on subsequent attempts.

The fix ensures each unique component path is only processed once, regardless of how many stacks reference it.

references

Fixes user-reported issue with duplicate deletion attempts in atmos terraform clean

🤖 Generated with Claude Code

Summary by CodeRabbit

Bug Fixes
- Deduplicates component paths aggregated across multiple stacks to avoid repeated processing, reduce redundant logs, and improve stability during cleanup.
Tests
- Added unit, integration, and end-to-end tests verifying deduplication when multiple stacks reference the same components.
- Expanded deletion tests to cover file/directory removals, handling of missing files, refusal of symlink deletions, and ensuring no duplicate deletions occur.
- Included a skipped placeholder for a future filesystem-dependent integration test.

When multiple stacks reference the same component, the getAllStacksComponentsPaths function would return duplicate component paths. This caused the clean command to attempt deleting the same files multiple times, resulting in "path does not exist" errors on the second attempt. The fix deduplicates component paths before processing deletions by tracking unique paths in a map. - Added deduplication logic to getAllStacksComponentsPaths function - Added test to verify multiple stacks with same component don't create duplicates - Fixes intermittent errors during terraform clean operations Resolves issue where users see duplicate deletion attempts with errors like: ✓ Deleted component/.terraform/ x Cannot delete component/.terraform/: path does not exist 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>

coderabbitai · 2025-09-24T20:06:24Z

📝 Walkthrough

Walkthrough

Adds deduplication when aggregating component paths across stacks and introduces unit, integration, and end-to-end tests verifying deduplication, collection, deletion behavior, and refusal to delete symlinks. One unit test is skipped pending filesystem setup.

Changes

Cohort / File(s)	Summary of changes
Deduplication logic `internal/exec/terraform_clean_util.go`	Added local `uniquePaths` tracking to ensure aggregated component paths across stacks are de-duplicated; preserved function signatures and existing error handling.
Unit tests — dedup detection `internal/exec/terraform_clean_duplicate_test.go`	New unit tests verifying `getAllStacksComponentsPaths` returns deduplicated paths; counts/logs duplicates and includes a skipped placeholder requiring filesystem setup.
Integration tests — deletion & collection `internal/exec/terraform_clean_integration_test.go`	New integration-style tests for `CollectComponentsDirectoryObjects` (duplicate handling) and `DeletePathTerraform` covering non-existent files, file deletion, directory deletion, and refusing to delete symlinks.
End-to-end test — dedup + delete `internal/exec/terraform_clean_e2e_test.go`	New e2e test that creates temp component structure with shared components across stacks, asserts deduplication, verifies collection counts, performs deletions, and asserts expected files/dirs removed with zero errors.

Sequence Diagram(s)

sequenceDiagram
  participant TestRunner
  participant Cleaner as getAllStacksComponentsPaths
  participant Stacks as stacksMap
  participant Unique as uniquePaths (map)

  Note over TestRunner,Cleaner: Request deduplicated component paths
  TestRunner->>Cleaner: getAllStacksComponentsPaths(stacksMap)
  Cleaner->>Stacks: iterate stacks -> components list
  loop for each component path
    Cleaner->>Unique: seen := uniquePaths[path]?
    alt not seen
      Unique-->>Cleaner: mark seen (uniquePaths[path]=true)
      Cleaner-->>TestRunner: append path to results
    else seen
      Note over Cleaner,Unique: skip duplicate path
    end
  end
  Cleaner-->>TestRunner: return deduplicated paths

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Fix atmos terraform clean command for Terraform components in sub-directories #1195 — Touches terraform-clean utilities and component-path aggregation/deduplication (strong overlap in scope).

Suggested reviewers

aknysh

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title clearly summarizes the primary change by stating that the PR fixes duplicate file deletions in the atmos terraform clean command, which directly matches the addition of deduplication logic and related tests. It is concise, specific to the key functionality, and omits irrelevant details. The inclusion of the “fix:” prefix follows convention and highlights the nature of the change without extraneous information. Scanning the history for this title would quickly inform a reviewer of the intended fix.

✨ Finishing touches

📝 Generate Docstrings

🧪 Generate unit tests

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch fix-clean

📜 Recent 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.

📥 Commits

Reviewing files that changed from the base of the PR and between a5bd148 and 8be2981.

📒 Files selected for processing (1)

internal/exec/terraform_clean_duplicate_test.go (1 hunks)

🧰 Additional context used

📓 Path-based instructions (3)

**/*_test.go

📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)

**/*_test.go: Every new feature must include comprehensive unit tests
Test both happy paths and error conditions
Use table-driven tests for multiple scenarios

**/*_test.go: Test file naming symmetry: test files mirror implementation structure with _test.go suffix
Unit tests should focus on pure functions and use table-driven tests
ALWAYS use t.Skipf() instead of t.Skip() and provide a clear reason for skipped tests
NEVER use t.Skipf() without a reason

Files:

internal/exec/terraform_clean_duplicate_test.go

**/*.go

📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)

**/*.go: All code must pass golangci-lint checks
Follow Go error handling idioms and use meaningful error messages
Wrap errors with context using fmt.Errorf("context: %w", err)
Consider custom error types for domain-specific errors
Follow standard Go coding style; run gofmt and goimports
Use snake_case for environment variables
Document complex logic with inline comments

**/*.go: All comments must end with periods - comments should be complete sentences (enforced by golangci-lint's godot linter)
All errors MUST be wrapped using static errors defined in errors/errors.go; never use dynamic errors directly
Always wrap errors with the static error first and add context-specific details after the static error; use %w verb to preserve error chain
ALWAYS use viper.BindEnv() for environment variable binding, and EVERY env var MUST have an ATMOS_ alternative
Distinguish between logging and UI output: only use logging for system/debug info, and use stderr/UI for prompts, progress, errors, and data for piping should go to stdout
Most text UI MUST go to stderr; only data/results go to stdout for proper piping; use utils.PrintfMessageToTUI() for UI messages
Atmos MUST work on Linux, macOS, and Windows—write portable implementations and prefer SDKs over calling binaries when available
ALWAYS compile after making changes; verify no compilation errors; run tests after successful compile; fix compilation errors immediately before proceeding
Write a test to reproduce the bug before fixing, run the test and confirm failure, fix the bug iteratively, and verify fix doesn't break existing tests

Files:

internal/exec/terraform_clean_duplicate_test.go

internal/exec/**.go

📄 CodeRabbit inference engine (CLAUDE.md)

Prefer extending existing utilities in internal/exec/ and pkg/ over creating new ones—search for existing methods before implementation

Files:

internal/exec/terraform_clean_duplicate_test.go

🧠 Learnings (7)

📚 Learning: 2024-11-02T15:35:09.958Z

Learnt from: aknysh
PR: cloudposse/atmos#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_clean_duplicate_test.go

📚 Learning: 2024-10-27T04:41:49.199Z

Learnt from: haitham911
PR: cloudposse/atmos#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_clean_duplicate_test.go

📚 Learning: 2024-10-27T04:28:40.966Z

Learnt from: haitham911
PR: cloudposse/atmos#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_clean_duplicate_test.go

📚 Learning: 2024-11-12T03:16:02.910Z

Learnt from: aknysh
PR: cloudposse/atmos#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_clean_duplicate_test.go

📚 Learning: 2024-10-30T13:25:45.965Z

Learnt from: osterman
PR: cloudposse/atmos#727
File: internal/exec/terraform_clean.go:405-412
Timestamp: 2024-10-30T13:25:45.965Z
Learning: In `internal/exec/terraform_clean.go`, when appending `stackFolders` to `folders` in the `handleCleanSubCommand` function, it's unnecessary to check if `stackFolders` is nil before appending, because in Go, appending a nil slice is safe and does not cause a panic.

Applied to files:

internal/exec/terraform_clean_duplicate_test.go

📚 Learning: 2024-11-24T19:13:10.287Z

Learnt from: haitham911
PR: cloudposse/atmos#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_clean_duplicate_test.go

📚 Learning: 2025-09-23T02:30:42.362Z

Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to **/*_test.go : Test both happy paths and error conditions

Applied to files:

internal/exec/terraform_clean_duplicate_test.go

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)

GitHub Check: Run pre-commit hooks
GitHub Check: Build (macos-latest, macos)
GitHub Check: Build (windows-latest, windows)
GitHub Check: Analyze (go)
GitHub Check: Lint (golangci)
GitHub Check: Summary

🔇 Additional comments (1)

internal/exec/terraform_clean_duplicate_test.go (1)

76-97: Solid coverage for dedupe behavior.

Matching the unique-count assertion with the membership checks closes the gap the earlier review spotted and keeps the regression net tight. Nice work.

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.

❤️ Share

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

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (1)

internal/exec/terraform_clean_duplicate_test.go (1)
48-68: Also assert we kept every expected component.

The duplicate check will fail without the fix, but we could still pass while silently dropping a unique component. Let's pin the expected set so we catch that too.
 	paths := getAllStacksComponentsPaths(stacksMap)
 	require.NotEmpty(t, paths)
+	require.ElementsMatch(t, []string{"vnet-elements", "database", "app"}, paths)

📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 9f67c80 and d9b53f3.

📒 Files selected for processing (2)

internal/exec/terraform_clean_duplicate_test.go (1 hunks)
internal/exec/terraform_clean_util.go (1 hunks)

🧰 Additional context used

📓 Path-based instructions (4)

**/*.go

📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)

**/*.go: All code must pass golangci-lint checks
Follow Go error handling idioms and use meaningful error messages
Wrap errors with context using fmt.Errorf("context: %w", err)
Consider custom error types for domain-specific errors
Follow standard Go coding style; run gofmt and goimports
Use snake_case for environment variables
Document complex logic with inline comments

**/*.go: All comments must end with periods - comments should be complete sentences (enforced by golangci-lint's godot linter)
All errors MUST be wrapped using static errors defined in errors/errors.go; never use dynamic errors directly
Always wrap errors with the static error first and add context-specific details after the static error; use %w verb to preserve error chain
ALWAYS use viper.BindEnv() for environment variable binding, and EVERY env var MUST have an ATMOS_ alternative
Distinguish between logging and UI output: only use logging for system/debug info, and use stderr/UI for prompts, progress, errors, and data for piping should go to stdout
Most text UI MUST go to stderr; only data/results go to stdout for proper piping; use utils.PrintfMessageToTUI() for UI messages
Atmos MUST work on Linux, macOS, and Windows—write portable implementations and prefer SDKs over calling binaries when available
ALWAYS compile after making changes; verify no compilation errors; run tests after successful compile; fix compilation errors immediately before proceeding
Write a test to reproduce the bug before fixing, run the test and confirm failure, fix the bug iteratively, and verify fix doesn't break existing tests

Files:

internal/exec/terraform_clean_util.go
internal/exec/terraform_clean_duplicate_test.go

**/!(*_test).go

📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)

Document all exported functions, types, and methods with Go doc comments

Files:

internal/exec/terraform_clean_util.go

internal/exec/**.go

📄 CodeRabbit inference engine (CLAUDE.md)

Prefer extending existing utilities in internal/exec/ and pkg/ over creating new ones—search for existing methods before implementation

Files:

internal/exec/terraform_clean_util.go
internal/exec/terraform_clean_duplicate_test.go

**/*_test.go

📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)

**/*_test.go: Every new feature must include comprehensive unit tests
Test both happy paths and error conditions
Use table-driven tests for multiple scenarios

**/*_test.go: Test file naming symmetry: test files mirror implementation structure with _test.go suffix
Unit tests should focus on pure functions and use table-driven tests
ALWAYS use t.Skipf() instead of t.Skip() and provide a clear reason for skipped tests
NEVER use t.Skipf() without a reason

Files:

internal/exec/terraform_clean_duplicate_test.go

🧠 Learnings (9)

📚 Learning: 2024-10-27T04:41:49.199Z

Learnt from: haitham911
PR: cloudposse/atmos#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_clean_util.go
internal/exec/terraform_clean_duplicate_test.go

📚 Learning: 2024-10-27T04:28:40.966Z

Learnt from: haitham911
PR: cloudposse/atmos#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_clean_util.go
internal/exec/terraform_clean_duplicate_test.go

📚 Learning: 2025-04-26T15:54:10.506Z

Learnt from: haitham911
PR: cloudposse/atmos#1195
File: internal/exec/terraform_clean.go:99-99
Timestamp: 2025-04-26T15:54:10.506Z
Learning: The error variable `ErrRelPath` is defined in `internal/exec/terraform_clean_util.go` and is used across files in the `exec` package, including in `terraform_clean.go`. This is part of an approach to standardize error handling in the codebase.

Applied to files:

internal/exec/terraform_clean_util.go
internal/exec/terraform_clean_duplicate_test.go

📚 Learning: 2024-11-02T15:35:09.958Z

Learnt from: aknysh
PR: cloudposse/atmos#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_clean_util.go
internal/exec/terraform_clean_duplicate_test.go

📚 Learning: 2024-11-12T03:16:02.910Z

Learnt from: aknysh
PR: cloudposse/atmos#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_clean_duplicate_test.go

📚 Learning: 2024-11-24T19:13:10.287Z

Learnt from: haitham911
PR: cloudposse/atmos#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_clean_duplicate_test.go

📚 Learning: 2024-10-30T13:25:45.965Z

Learnt from: osterman
PR: cloudposse/atmos#727
File: internal/exec/terraform_clean.go:405-412
Timestamp: 2024-10-30T13:25:45.965Z
Learning: In `internal/exec/terraform_clean.go`, when appending `stackFolders` to `folders` in the `handleCleanSubCommand` function, it's unnecessary to check if `stackFolders` is nil before appending, because in Go, appending a nil slice is safe and does not cause a panic.

Applied to files:

internal/exec/terraform_clean_duplicate_test.go

📚 Learning: 2025-09-23T02:30:42.362Z

Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to **/*_test.go : Test both happy paths and error conditions

Applied to files:

internal/exec/terraform_clean_duplicate_test.go

📚 Learning: 2024-11-13T21:37:07.852Z

Learnt from: Cerebrovinny
PR: cloudposse/atmos#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_clean_duplicate_test.go

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)

GitHub Check: Build (windows-latest, windows)
GitHub Check: Run pre-commit hooks
GitHub Check: Analyze (go)
GitHub Check: Lint (golangci)
GitHub Check: Summary

🔇 Additional comments (1)

internal/exec/terraform_clean_util.go (1)

18-32: Dedup filter looks solid.

Thanks for centralizing the uniqueness check here; this keeps downstream logic unchanged while preventing repeat deletions.

codecov · 2025-09-24T20:12:20Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 57.02%. Comparing base (2d3498e) to head (00498c8).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1510      +/-   ##
==========================================
+ Coverage   56.94%   57.02%   +0.08%     
==========================================
  Files         284      284              
  Lines       30286    30292       +6     
==========================================
+ Hits        17247    17275      +28     
+ Misses      11213    11193      -20     
+ Partials     1826     1824       -2

Flag	Coverage Δ
unittests	`57.02% <100.00%> (+0.08%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

- Added integration tests to verify DeletePathTerraform behavior - Added end-to-end test simulating multiple stacks referencing same component - Tests confirm files are deleted only once even when multiple stacks reference the same component - Tests verify our deduplication fix prevents duplicate deletion attempts

coderabbitai

Actionable comments posted: 2

📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between d9b53f3 and 6c342d2.

📒 Files selected for processing (2)

internal/exec/terraform_clean_e2e_test.go (1 hunks)
internal/exec/terraform_clean_integration_test.go (1 hunks)

🧰 Additional context used

📓 Path-based instructions (3)

**/*_test.go

📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)

**/*_test.go: Every new feature must include comprehensive unit tests
Test both happy paths and error conditions
Use table-driven tests for multiple scenarios

**/*_test.go: Test file naming symmetry: test files mirror implementation structure with _test.go suffix
Unit tests should focus on pure functions and use table-driven tests
ALWAYS use t.Skipf() instead of t.Skip() and provide a clear reason for skipped tests
NEVER use t.Skipf() without a reason

Files:

internal/exec/terraform_clean_integration_test.go
internal/exec/terraform_clean_e2e_test.go

**/*.go

📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)

**/*.go: All code must pass golangci-lint checks
Follow Go error handling idioms and use meaningful error messages
Wrap errors with context using fmt.Errorf("context: %w", err)
Consider custom error types for domain-specific errors
Follow standard Go coding style; run gofmt and goimports
Use snake_case for environment variables
Document complex logic with inline comments

**/*.go: All comments must end with periods - comments should be complete sentences (enforced by golangci-lint's godot linter)
All errors MUST be wrapped using static errors defined in errors/errors.go; never use dynamic errors directly
Always wrap errors with the static error first and add context-specific details after the static error; use %w verb to preserve error chain
ALWAYS use viper.BindEnv() for environment variable binding, and EVERY env var MUST have an ATMOS_ alternative
Distinguish between logging and UI output: only use logging for system/debug info, and use stderr/UI for prompts, progress, errors, and data for piping should go to stdout
Most text UI MUST go to stderr; only data/results go to stdout for proper piping; use utils.PrintfMessageToTUI() for UI messages
Atmos MUST work on Linux, macOS, and Windows—write portable implementations and prefer SDKs over calling binaries when available
ALWAYS compile after making changes; verify no compilation errors; run tests after successful compile; fix compilation errors immediately before proceeding
Write a test to reproduce the bug before fixing, run the test and confirm failure, fix the bug iteratively, and verify fix doesn't break existing tests

Files:

internal/exec/terraform_clean_integration_test.go
internal/exec/terraform_clean_e2e_test.go

internal/exec/**.go

📄 CodeRabbit inference engine (CLAUDE.md)

Prefer extending existing utilities in internal/exec/ and pkg/ over creating new ones—search for existing methods before implementation

Files:

internal/exec/terraform_clean_integration_test.go
internal/exec/terraform_clean_e2e_test.go

🧠 Learnings (10)

📚 Learning: 2024-11-02T15:35:09.958Z

Learnt from: aknysh
PR: cloudposse/atmos#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_clean_integration_test.go
internal/exec/terraform_clean_e2e_test.go

📚 Learning: 2024-10-27T04:41:49.199Z

Learnt from: haitham911
PR: cloudposse/atmos#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_clean_integration_test.go
internal/exec/terraform_clean_e2e_test.go

📚 Learning: 2024-10-27T04:28:40.966Z

Learnt from: haitham911
PR: cloudposse/atmos#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_clean_integration_test.go
internal/exec/terraform_clean_e2e_test.go

📚 Learning: 2024-11-12T03:16:02.910Z

Learnt from: aknysh
PR: cloudposse/atmos#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_clean_integration_test.go
internal/exec/terraform_clean_e2e_test.go

📚 Learning: 2025-04-26T15:54:10.506Z

Learnt from: haitham911
PR: cloudposse/atmos#1195
File: internal/exec/terraform_clean.go:99-99
Timestamp: 2025-04-26T15:54:10.506Z
Learning: The error variable `ErrRelPath` is defined in `internal/exec/terraform_clean_util.go` and is used across files in the `exec` package, including in `terraform_clean.go`. This is part of an approach to standardize error handling in the codebase.

Applied to files:

internal/exec/terraform_clean_integration_test.go
internal/exec/terraform_clean_e2e_test.go

📚 Learning: 2025-09-23T03:44:59.111Z

Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-23T03:44:59.111Z
Learning: Applies to tests/**/*.go : Shared test utilities should be placed in the tests/ directory for integration tests

Applied to files:

internal/exec/terraform_clean_integration_test.go

📚 Learning: 2024-11-24T19:13:10.287Z

Learnt from: haitham911
PR: cloudposse/atmos#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_clean_integration_test.go
internal/exec/terraform_clean_e2e_test.go

📚 Learning: 2025-09-23T03:44:59.111Z

Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-23T03:44:59.111Z
Learning: Applies to cmd/*.go : Co-locate tests with implementation (_test.go alongside .go files in cmd/)

Applied to files:

internal/exec/terraform_clean_integration_test.go

📚 Learning: 2025-09-23T02:30:42.362Z

Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to **/*_test.go : Test both happy paths and error conditions

Applied to files:

internal/exec/terraform_clean_integration_test.go

📚 Learning: 2024-10-30T13:25:45.965Z

Learnt from: osterman
PR: cloudposse/atmos#727
File: internal/exec/terraform_clean.go:405-412
Timestamp: 2024-10-30T13:25:45.965Z
Learning: In `internal/exec/terraform_clean.go`, when appending `stackFolders` to `folders` in the `handleCleanSubCommand` function, it's unnecessary to check if `stackFolders` is nil before appending, because in Go, appending a nil slice is safe and does not cause a panic.

Applied to files:

internal/exec/terraform_clean_e2e_test.go

🧬 Code graph analysis (2)

internal/exec/terraform_clean_integration_test.go (2)

internal/exec/terraform_clean_util.go (1)

CollectComponentsDirectoryObjects (70-82)

internal/exec/terraform_clean.go (1)

DeletePathTerraform (278-301)

internal/exec/terraform_clean_e2e_test.go (2)

internal/exec/terraform_clean_util.go (1)

CollectComponentsDirectoryObjects (70-82)

internal/exec/terraform_clean.go (1)

DeletePathTerraform (278-301)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)

GitHub Check: Build (macos-latest, macos)
GitHub Check: Build (windows-latest, windows)
GitHub Check: Analyze (go)
GitHub Check: Run pre-commit hooks
GitHub Check: Lint (golangci)
GitHub Check: Summary

internal/exec/terraform_clean_e2e_test.go

internal/exec/terraform_clean_integration_test.go

All inline comments must end with periods per golangci-lint godot linter requirements.

coderabbitai

Actionable comments posted: 1

📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 6c342d2 and a5bd148.

📒 Files selected for processing (3)

internal/exec/terraform_clean_duplicate_test.go (1 hunks)
internal/exec/terraform_clean_e2e_test.go (1 hunks)
internal/exec/terraform_clean_integration_test.go (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (2)

internal/exec/terraform_clean_integration_test.go
internal/exec/terraform_clean_e2e_test.go

🧰 Additional context used

📓 Path-based instructions (3)

**/*_test.go

📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)

**/*_test.go: Every new feature must include comprehensive unit tests
Test both happy paths and error conditions
Use table-driven tests for multiple scenarios

**/*_test.go: Test file naming symmetry: test files mirror implementation structure with _test.go suffix
Unit tests should focus on pure functions and use table-driven tests
ALWAYS use t.Skipf() instead of t.Skip() and provide a clear reason for skipped tests
NEVER use t.Skipf() without a reason

Files:

internal/exec/terraform_clean_duplicate_test.go

**/*.go

📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)

**/*.go: All code must pass golangci-lint checks
Follow Go error handling idioms and use meaningful error messages
Wrap errors with context using fmt.Errorf("context: %w", err)
Consider custom error types for domain-specific errors
Follow standard Go coding style; run gofmt and goimports
Use snake_case for environment variables
Document complex logic with inline comments

**/*.go: All comments must end with periods - comments should be complete sentences (enforced by golangci-lint's godot linter)
All errors MUST be wrapped using static errors defined in errors/errors.go; never use dynamic errors directly
Always wrap errors with the static error first and add context-specific details after the static error; use %w verb to preserve error chain
ALWAYS use viper.BindEnv() for environment variable binding, and EVERY env var MUST have an ATMOS_ alternative
Distinguish between logging and UI output: only use logging for system/debug info, and use stderr/UI for prompts, progress, errors, and data for piping should go to stdout
Most text UI MUST go to stderr; only data/results go to stdout for proper piping; use utils.PrintfMessageToTUI() for UI messages
Atmos MUST work on Linux, macOS, and Windows—write portable implementations and prefer SDKs over calling binaries when available
ALWAYS compile after making changes; verify no compilation errors; run tests after successful compile; fix compilation errors immediately before proceeding
Write a test to reproduce the bug before fixing, run the test and confirm failure, fix the bug iteratively, and verify fix doesn't break existing tests

Files:

internal/exec/terraform_clean_duplicate_test.go

internal/exec/**.go

📄 CodeRabbit inference engine (CLAUDE.md)

Prefer extending existing utilities in internal/exec/ and pkg/ over creating new ones—search for existing methods before implementation

Files:

internal/exec/terraform_clean_duplicate_test.go

🧠 Learnings (10)

📚 Learning: 2024-11-02T15:35:09.958Z

Learnt from: aknysh
PR: cloudposse/atmos#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_clean_duplicate_test.go

📚 Learning: 2024-10-27T04:41:49.199Z

Learnt from: haitham911
PR: cloudposse/atmos#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_clean_duplicate_test.go

📚 Learning: 2024-10-27T04:28:40.966Z

Learnt from: haitham911
PR: cloudposse/atmos#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_clean_duplicate_test.go

📚 Learning: 2024-11-12T03:16:02.910Z

Learnt from: aknysh
PR: cloudposse/atmos#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_clean_duplicate_test.go

📚 Learning: 2024-10-30T13:25:45.965Z

Learnt from: osterman
PR: cloudposse/atmos#727
File: internal/exec/terraform_clean.go:405-412
Timestamp: 2024-10-30T13:25:45.965Z
Learning: In `internal/exec/terraform_clean.go`, when appending `stackFolders` to `folders` in the `handleCleanSubCommand` function, it's unnecessary to check if `stackFolders` is nil before appending, because in Go, appending a nil slice is safe and does not cause a panic.

Applied to files:

internal/exec/terraform_clean_duplicate_test.go

📚 Learning: 2024-11-24T19:13:10.287Z

Learnt from: haitham911
PR: cloudposse/atmos#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_clean_duplicate_test.go

📚 Learning: 2025-04-26T15:54:10.506Z

Learnt from: haitham911
PR: cloudposse/atmos#1195
File: internal/exec/terraform_clean.go:99-99
Timestamp: 2025-04-26T15:54:10.506Z
Learning: The error variable `ErrRelPath` is defined in `internal/exec/terraform_clean_util.go` and is used across files in the `exec` package, including in `terraform_clean.go`. This is part of an approach to standardize error handling in the codebase.

Applied to files:

internal/exec/terraform_clean_duplicate_test.go

📚 Learning: 2024-12-07T16:19:01.683Z

Learnt from: aknysh
PR: cloudposse/atmos#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_clean_duplicate_test.go

📚 Learning: 2025-09-23T02:30:42.362Z

Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to **/*_test.go : Test both happy paths and error conditions

Applied to files:

internal/exec/terraform_clean_duplicate_test.go

📚 Learning: 2024-11-13T21:37:07.852Z

Learnt from: Cerebrovinny
PR: cloudposse/atmos#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_clean_duplicate_test.go

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)

GitHub Check: Build (windows-latest, windows)
GitHub Check: Lint (golangci)
GitHub Check: Analyze (go)
GitHub Check: Run pre-commit hooks
GitHub Check: Summary

internal/exec/terraform_clean_duplicate_test.go

…erved - Define expected unique components upfront (vnet-elements, database, app) - Assert the exact count of unique components matches expected - Verify each expected component is present in the result - Ensure no unexpected components are added during deduplication - Makes test more robust by verifying deduplication preserves all valid components

github-actions · 2025-09-25T17:57:47Z

These changes were released in v1.191.1-test.0.

osterman requested a review from a team as a code owner September 24, 2025 20:03

github-actions bot added the size/s Small size PR label Sep 24, 2025

coderabbitai bot reviewed Sep 24, 2025

View reviewed changes

coderabbitai bot previously approved these changes Sep 24, 2025

View reviewed changes

osterman added the patch A minor, backward compatible change label Sep 24, 2025

osterman dismissed coderabbitai[bot]’s stale review via 6c342d2 September 24, 2025 21:24

github-actions bot added size/m Medium size PR and removed size/s Small size PR labels Sep 24, 2025

coderabbitai bot requested changes Sep 24, 2025

View reviewed changes

internal/exec/terraform_clean_e2e_test.go Outdated Show resolved Hide resolved

internal/exec/terraform_clean_integration_test.go Outdated Show resolved Hide resolved

fix: add periods to inline comments for godot linter compliance

a5bd148

All inline comments must end with periods per golangci-lint godot linter requirements.

coderabbitai bot requested changes Sep 24, 2025

View reviewed changes

internal/exec/terraform_clean_duplicate_test.go Show resolved Hide resolved

coderabbitai bot approved these changes Sep 24, 2025

View reviewed changes

Merge branch 'main' into fix-clean

9a16b09

aknysh added no-release Do not create a new release (wait for additional code changes) and removed patch A minor, backward compatible change labels Sep 24, 2025

aknysh approved these changes Sep 24, 2025

View reviewed changes

Merge branch 'main' into fix-clean

00498c8

aknysh merged commit 423e592 into main Sep 24, 2025
52 checks passed

aknysh deleted the fix-clean branch September 24, 2025 23:03

coderabbitai bot mentioned this pull request Sep 26, 2025

fix: normalize path separators in terraform clean output #1523

Merged

coderabbitai bot mentioned this pull request Oct 4, 2025

clean user files set by user #1381

Open

coderabbitai bot mentioned this pull request Oct 14, 2025

Improve test quality: fix tests without assertions and convert skip-only tests #1621

Merged

osterman mentioned this pull request Oct 19, 2025

Refactor exit code handling and error reporting #1669

Merged

coderabbitai bot mentioned this pull request Dec 29, 2025

Add generate section inheritance and auto-generation support #1878

Merged

coderabbitai bot mentioned this pull request Jan 3, 2026

feat: Add native Terraform provider caching with XDG support #1882

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: prevent duplicate file deletions in atmos terraform clean#1510

fix: prevent duplicate file deletions in atmos terraform clean#1510
aknysh merged 6 commits intomainfrom
fix-clean

osterman commented Sep 24, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Sep 24, 2025 •

edited

Loading

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related PRs

Suggested reviewers

Uh oh!

coderabbitai bot left a comment

Uh oh!

codecov bot commented Sep 24, 2025 •

edited

Loading

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Sep 25, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

osterman commented Sep 24, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

what

why

references

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Sep 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related PRs

Suggested reviewers

Pre-merge checks and finishing touches

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Sep 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Sep 25, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

osterman commented Sep 24, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Sep 24, 2025 •

edited

Loading

codecov bot commented Sep 24, 2025 •

edited

Loading